Skip to content

fix(release): grant the certificate-minting scopes to the signing job only - #1323

Merged
justinjoy merged 1 commit into
mainfrom
fix/1319-job-permissions
Sep 3, 2026
Merged

fix(release): grant the certificate-minting scopes to the signing job only#1323
justinjoy merged 1 commit into
mainfrom
fix/1319-job-permissions

Conversation

@justinjoy

Copy link
Copy Markdown
Collaborator

Closes #1319. Stacked on #1322 (fix/1287-identity-anchor) — review that first.

The bug

release-tag.yml granted id-token: write and attestations: write at workflow level,
so every job that did not re-declare permissions: inherited them. Only release-artifacts
re-declares.

$ grep -n 'permissions:' .github/workflows/release-tag.yml
19:permissions:          # ← contents: read, id-token: write, attestations: write
338:    permissions:      # ← release-artifacts

That matters because job_workflow_ref names the workflow file, not the job. There is
no job discriminator in the SAN or in the GitHub OID certificate extensions, so a
certificate minted by any inheriting job is byte-identical to the signing job's, and no
--certificate-identity-regexp can separate them. #1287 anchored that pattern; this is the
level below it, and the permissions block is the only thing that can bound it — the pattern
structurally cannot.

Impact: a certificate scripts/release/verify-release.sh accepts, over an artifact of
the minting job's choosing, which a consumer following docs/SIGNING.md verifies as
genuine. Two inheriting jobs execute externally-sourced input — downstream fetches and
runs corpora on runs-on: [self-hosted, linux, x64, wirelog-ga], and fuzz runs generated
input.

The fix

Workflow-level permissions: is now contents: read alone. Verified nothing else needed
the removed scopes: every other job runs only actions/checkout@v5 and
actions/upload-artifact@v7, neither of which requires them — corroborated in-repo by
perf-nightly.yml and fuzz-evidence.yml, which already run upload-artifact@v7 under
contents: read on a daily cron.

sanitizers now states permissions: contents: read explicitly. It delegates via uses:,
and the docs are genuinely ambiguous about whether a calling job with no block inherits the
caller's workflow-level mapping or the repository default. The called workflow declares
contents: read itself and can only downgrade what it is passed, so the result is
unchanged either way — stating it makes the ceiling independent of a question neither
reviewer could resolve from documentation.

Tests

Five new assertions in scripts/ci/test-release-signing.sh, written before the fix and
confirmed failing against the unfixed workflow.

They match the scope, not one byte-spelling of it. id-token: 'write', two spaces,
"write", flow-style permissions: {contents: read, id-token: write}, and the value on a
continuation line all parse to the identical grant, and my first attempt — a fixed-string
match — missed every one of them. Three of those would have re-opened this in full,
silently, past a green gate.

The count check is deliberately loose where the positive check is strict: a bare valueless
contents: must fail the assertion that the read scope is granted. Unifying the two
matchers would make that mutation pass green while granting nothing. The comment records
the asymmetry, because it is invisible from the call sites.

refute is new. Unlike assert it has to verify its condition can run: a missing command
exits 127, which assert correctly reads as failure but refute would read as
"successfully refuted" — silently turning every refutation in the file into a pass.

Mutation-tested throughout: workflow-level re-add of each scope in every spelling, a second
job declaring either scope, flow form, continuation form, an emptied mapping, and a
false-positive probe (attestations: read). Local: 307 Ok / 0 Fail / 12 Skipped,
serialized.

What this does not close

#1318. An actor with repository write access can push v1.99.0 carrying their own copy of
release-tag.yml, since on: push: tags: runs the file as it exists at the pushed tag. No
regex over the tag name closes that — the attacker picks the name. This change is
blast-radius reduction against supply-chain and non-adversarial failure, not adversary
exclusion, and the CHANGELOG entry is scoped to the file rather than the system for exactly
that reason.

Review

Independent Reviewer and Critic, two rounds. Both blocked the first candidate. Findings:
the fixed-string scope matching (five evasions, three security-relevant); CHANGELOG.md
still declaring #1319 unclosed, which ships verbatim as the GitHub Release body via
extract-changelog-section.sh; the vacuous refute; a missing attestations counterpart to
the id-token count; and the continuation-line spelling.

One post-approval change: the Reviewer's suggested wording for a reference ambiguity in the
CHANGELOG bullet ((#1318) — see the bullet below (#1319) read as naming the wrong bullet).
Prose only; both gates had called it not worth another round.

#1154 holds: no key, fingerprint, or secret-consuming step anywhere. This strictly reduces
token scope.

@justinjoy
justinjoy force-pushed the fix/1287-identity-anchor branch from 4e38c73 to 0cacae5 Compare September 3, 2026 09:25
@justinjoy
justinjoy force-pushed the fix/1319-job-permissions branch from 403348d to e3b7816 Compare September 3, 2026 09:25
@justinjoy
justinjoy force-pushed the fix/1287-identity-anchor branch from 0cacae5 to 4f0668b Compare September 3, 2026 12:13
Base automatically changed from fix/1287-identity-anchor to main September 3, 2026 14:02
… only

release-tag.yml granted `id-token: write` and `attestations: write` at workflow
level, so every job that did not re-declare `permissions:` inherited them. Only
release-artifacts re-declares.

That matters because job_workflow_ref names the WORKFLOW FILE, not the job.
There is no job discriminator in the SAN or in the GitHub OID extensions, so a
certificate minted by any of those jobs is indistinguishable from the signing
job's and no --certificate-identity-regexp can separate them. #1287 anchored
that pattern; this is the level below it, and it is the only level that can be
bounded here -- the pattern structurally cannot.

The exposure is a certificate verify-release.sh accepts over an artifact of the
minting job's choosing, which a consumer following docs/SIGNING.md verifies as
genuine. Two of the inheriting jobs execute externally-sourced input: downstream
fetches and runs corpora on a self-hosted runner, and fuzz runs generated input.

Workflow-level permissions is now `contents: read` alone. Nothing else needed
the removed scopes: every other job runs only checkout and upload-artifact,
neither of which requires them.

The sanitizers job now states `permissions: contents: read` explicitly. It
delegates via `uses:`, and the docs are ambiguous about whether a calling job
with no block inherits the caller's workflow-level mapping or the repository
default. The called workflow declares contents: read itself and can only
downgrade what it is passed, so the result is unchanged either way; stating it
makes the ceiling independent of the ambiguity.

The contract test gains five assertions: the workflow-level mapping was
extracted at all, it does not grant either minting scope, it still grants
contents: read, and exactly one permissions mapping in the file grants each
scope. They match the SCOPE rather than one byte-spelling of it -- `id-token:
'write'`, two spaces, `"write"`, the flow form, and the value on a continuation
line all parse to the same grant, and a fixed-string match missed every one.
The count check is deliberately loose where the positive check is strict: a
valueless `contents:` must fail the assertion that the read scope is granted.

`refute` is new here. Unlike `assert` it has to check its condition can RUN: a
missing command exits 127, which assert correctly reads as failure but refute
would read as "successfully refuted", silently turning every refutation in the
file into a pass.

This closes what #1287 documented as a residual, so the residual text in
verify-release.sh, sign-artifacts.sh, docs/SIGNING.md and CHANGELOG.md is
updated to say what now bounds it. The CHANGELOG entry is scoped to the file
rather than the system: an actor with write access can still push a tag carrying
their own copy of this workflow, which is #1318 and needs tag protection.

Refs #1319
@justinjoy
justinjoy force-pushed the fix/1319-job-permissions branch from e3b7816 to 7eccfa1 Compare September 3, 2026 14:21
@justinjoy
justinjoy enabled auto-merge (rebase) September 3, 2026 14:22
@justinjoy
justinjoy disabled auto-merge September 3, 2026 14:24
@justinjoy
justinjoy enabled auto-merge (rebase) September 3, 2026 14:24
@justinjoy
justinjoy merged commit 275671d into main Sep 3, 2026
26 checks passed
@justinjoy
justinjoy deleted the fix/1319-job-permissions branch September 3, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

release-tag.yml grants id-token: write at workflow level, so every other job inherits it

1 participant