Skip to content

Adopting this asks for write access, every secret, and an unpinned ref — and granting less fails with no log #45

Description

@sotashimozono

Read as an outsider would, not found by anything failing. The quickstart's first code block asks
a stranger for three things at once, and the safe answer to any of them is punished.

    permissions:
      contents: write
    uses: QAtlasHub/TestShards.jl/.github/workflows/sharded-tests.yml@main
    secrets: inherit

That reads as: give an unknown organisation's unpinned code write access to your repository and
hand it every secret you have.
Each part is individually defensible and the combination is a
lot to ask before anyone has seen the tool work.

contents: write is needed by two things, neither of which a new adopter runs

needs write when it runs
timings push to the default branch only
steal opt-in, default false

A pull request run with the defaults uses neither. But a called workflow's job cannot request
more permission than the caller granted, so the grant has to be there anyway — and the failure
mode when it is not is startup_failure, with no log to read.

The workflow's own comment already says this:

a job asking for more than the caller granted makes the whole run fail to START
(startup_failure), with no log to read

So the adopter most likely to grant contents: read — the cautious one, reading a workflow from
an org they do not know — is the one who gets an unexplained failure with nothing to debug. The
careful answer is the one that breaks.

secrets: inherit passes everything, for one optional secret

The workflow declares exactly one:

    secrets:
      CODECOV_TOKEN:
        required: false

secrets: inherit hands over every secret the caller has. It is in the first example anyone
reads, and it buys one optional token.

@main is deliberate internally and reads differently from outside

Tracking main across our own repositories is the convention and there are good reasons for it.
An external adopter has a different trust basis: they are pinning to a moving ref and granting
it write and inheriting secrets into it.

Proposed

1. Split the write into its own reusable, so the default adoption is read-only.

  test:
    permissions: { contents: read }
    uses: QAtlasHub/TestShards.jl/.github/workflows/sharded-tests.yml@v1
    with: { shards: 8 }

  timings:
    needs: test
    if: github.event_name == 'push' && github.ref == 'refs/heads/main'
    permissions: { contents: write }
    uses: QAtlasHub/TestShards.jl/.github/workflows/record-timings.yml@v1

Costs the caller one extra job. Buys: the first ask is read-only, the write grant is visible and
scoped to the job that needs it, and steal: true remains available for anyone who grants write
to the test job as well.

There is a second reason to want this. It makes the first-run path the default path — no
ci-timings branch, round-robin, timings accumulating from there. That path is the one every
new adopter starts on and the one this repository exercises least, because QAtlas has had a
timing history for a long time.

2. Drop secrets: inherit from the quickstart. Name CODECOV_TOKEN where coverage upload is
actually wanted. Independent of 1.

3. Publish a moving v1 tag for external callers. Internal repositories keep @main on
purpose; the two audiences have different trust bases, so serving them differently is not an
inconsistency. Independent of 1.

2 and 3 are one-line changes and should not wait on 1.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions