ci(release): attest build provenance for release artifacts#574
Conversation
Zious11
left a comment
There was a problem hiding this comment.
Thanks @arcaven — this is a well-constructed contribution: both action SHAs check out against the upstream tags (we verified a2bbfa2… == attest-build-provenance v4.1.0 and the harden-runner pin matches our existing v2.19.4 pin exactly), the permissions are correctly job-scoped, and the fork-transparency intent is appreciated. We researched the placement question against GitHub's docs, SLSA guidance, and real-world pipelines before writing this up, and it points to a design change rather than a polish pass — details below.
Required
1. Move attestation into the build workflow, before upload — not download-and-attest after publish.
GitHub's canonical pattern places attest-build-provenance in the workflow that produced the bytes, before distribution (docs, blog); jdx/mise's own releases attest in-workflow before upload too. Two concrete reasons this matters here:
- The "attest the exact bytes users download" rationale is moot:
gh attestation verify(and mise's verifier) match on sha256 digest, not storage location. An attestation created from workspace artifacts verifies identically against the release-page download — softprops/action-gh-release uploads bytes verbatim, and GitHub now exposesassets[].digestas an independent cross-check. So the post-publish download buys nothing… - …and costs a real window (TOCTOU, CWE-362): the job attests whatever is on the release page at download time. On a signing-enabled fork this race is deterministic —
sign-and-publish.ymlfires on the same workflow completion and--clobbers re-signed darwin assets, so the job can attest bytes users never receive. It also downgrades the claim from builder provenance to consumer re-attestation (weaker SLSA Build L2 semantics — the attesting workflow didn't build the subjects).
Suggested shape: a fan-in job with actions/download-artifact (SHA-pinned, merge-multiple: true) that attests, sitting between the build matrix and the release upload job — or attest per-matrix-job on the built files. Either eliminates the window, drops the GH_TOKEN/contents: read need in that step, and makes the provenance claim exact.
2. Add a fork opt-in gate, matching this repo's release-ops convention.
Every other release-ops job here is gated on a repository variable so forks carry the file verbatim as a no-op (vars.SIGNING_ENABLED, HOMEBREW_TAP_REPO, RELEASE_GAP_FILL_ENABLED, SYNC_UPSTREAM_REPO — see docs/specs/fork-friendly-release-ops.md). As written, the attest job runs unconditionally on any fork's v* tag: it fails hard where attestations aren't available (private forks need GitHub Enterprise Cloud; GHES doesn't support attestations at all; org policies can withhold id-token: write) and publishes fork commit SHAs to the public Rekor transparency log without opt-in. Please gate it (e.g. vars.ATTESTATIONS_ENABLED == 'true') and add the row to the spec's variables table.
3. Bind github.repository through env: rather than inline ${{ }} in the run: block.
Not exploitable here (github.repository is platform-constrained), but this repo enforces the env-binding rule for expressions in shell (see the CWE-77 rule comments in sign-and-publish.yml), and CI review will keep flagging it. One-line change — and it disappears entirely if you adopt the download-artifact design from item 1.
Recommended
4. Exclude .sha256 files from subject-path — the archive attestations already cover integrity; attesting checksum files is circular noise. (release-assets/*.tar.gz + *.zip, or a files list.)
5. Bump to v4.1.1 (current latest, 2026-06-26) while you're in here — SHA-pin it the same way; we'll verify the pin on re-review.
6. Add an inline comment marking the coverage boundary for signed macOS assets: the darwin bare binaries and .pkg/.dmg that sign-and-publish.yml uploads have different digests post-signing, so they need their own attest step in that workflow (after codesign/notarize, before upload). Keeping that out of this PR is a fine scope call — but the boundary should be visible in the code, not just the PR description. A follow-up PR gated on the same variable would complete the story.
7. A short CHANGELOG entry under [Unreleased] > Added — provenance verification is a user-facing capability (gh attestation verify, mise's native verification).
Small notes
set -euo pipefailin any remaining download script so an empty asset dir fails loudly (moot under item 1).- The
!cancelled() && needs.release.result == 'success'guard is fine; the inline comment slightly overstates what it covers (a cancel mid-attest can still interrupt submission) — no change needed.
The mechanics of what you wrote are solid, and with the placement moved in-workflow plus the fork gate this is a clear improvement to the release pipeline. Happy to re-review quickly. This also pairs with #573 — once this lands, the README's attestation paragraph there becomes accurate with a small rewording (commented separately on that PR).
…sted (SHA pins verified; TOCTOU/fork-gate/CWE-77 findings); review artifacts committed
- Fix the attestation sentence: mise's native verification already ships; what is pending is this repo publishing attestations (Zious11#574). Drop the 'SLSA provenance runs by default' overstatement. - Make the mise use / mise.toml examples runnable (Zious11/jira-cli), with one fork-owner substitution sentence. - Narrow the prerelease glob to the only channel this repo cuts (v*-dev.*). - Note mise activate is assumed for the binary to land on PATH. - Add the Windows-has-no-stable-asset-yet caveat (needs prerelease=true). - Give an activation-free xattr fallback via 'mise where'.
…_ENABLED (Zious11#574) Address review: move provenance from download-after-publish to an in-workflow attestation of the built bytes, and make the job fork-safe. - Attest fans in from the immutable `build` artifacts (parallel to `release`, both `needs: build`) instead of `gh release download` after publish. The attested bytes are identical to what softprops uploads verbatim, so there is no publish-then-attest window (TOCTOU/CWE-362) and no consumer re-attestation of possibly-clobbered assets — the claim is builder-side provenance. Kept parallel rather than strictly upstream of `release` so the fork gate can skip `attest` without cascading a skip onto the fork's release. - Gate on `vars.ATTESTATIONS_ENABLED == 'true'`, matching the release-ops convention (SIGNING_ENABLED, RELEASE_GAP_FILL_ENABLED, ...). Forks carry the file verbatim as a no-op; attestations need id-token: write + a public Rekor entry and are unavailable on GHES / private non-GHEC forks. - Drop `.sha256` sidecars from subject-path (archive attestation already covers integrity; attesting checksums is circular). - Bump attest-build-provenance v4.1.0 -> v4.1.1 (SHA-pinned). - Remove the inline ${{ github.repository }} shell interpolation — no download script remains, so the CWE-77 env-binding concern is gone. - Comment the coverage boundary: sign-and-publish.yml re-signs darwin binaries to DIFFERENT digests post-notarization; those need their own attest step in that workflow, out of scope here. - Add CHANGELOG entry, spec variables-table row, and CLAUDE.md gate line.
* docs(readme): document mise install path * docs(readme): address review on mise section (#573) - Fix the attestation sentence: mise's native verification already ships; what is pending is this repo publishing attestations (#574). Drop the 'SLSA provenance runs by default' overstatement. - Make the mise use / mise.toml examples runnable (Zious11/jira-cli), with one fork-owner substitution sentence. - Narrow the prerelease glob to the only channel this repo cuts (v*-dev.*). - Note mise activate is assumed for the binary to land on PATH. - Add the Windows-has-no-stable-asset-yet caveat (needs prerelease=true). - Give an activation-free xattr fallback via 'mise where'.
…-PR triage (D-178) - BC-3.5.006 SATISFIED: spec v1.3.42 (bc-3-issue-write.md); EJ probe confirmed green by nightly run 29398774009 (test_e2e_comment_edit_visibility_merge_semantics ok on develop @ 56d5126); CHANGELOG MERGE-claim now empirically backed. - Docs PR #625 (docs/577-ej-probe-closeout) CI 14/14 green; HELD for human review+merge (independent-review guard — classifier blocked pr-manager and orchestrator reviewer dispatch; session authored the PR; SELF-APPROVAL-GUARD 2nd positive datapoint). - Fresh PR reviews: #573 APPROVE @ 046ee4f (pr-review-2.md); #574 code review APPROVE-pending-rebase + security review APPROVE @ 3c37948 (pr-review-2.md + security-review-2.md; CHANGELOG-only conflict needs contributor rebase). - PR triage: #591 MERGE-READY (9-day soak); #598/#599 HOLD-SOAK-2026-07-16; #612 HOLD-SOAK-2026-07-20; #624 HOLD-SOAK-2026-07-22. - DEC-178: all-dependabot soak broadened to ALL dependabot PRs (cargo + Actions), not just third-party Actions bumps (broadens DEC-133). - Drift items: FACTORY-DISPATCHER-HOOK-TIMEOUT (engine-side, LOW); SELF-APPROVAL-GUARD-2ND-DATAPOINT (positive control, CLOSED). - STATE.md: 341 lines (cap 500); burst-log.md: F5-CONVERGED + F4-COMPLETE step rows archived per keep-5 rule; sidecar-learning.md session timestamps added.
|
Thanks for the rework — this addresses the earlier feedback well. Fresh code and security reviews of head
One thing needed before merge: please rebase onto current Optional nit while you're in there (non-blocking): We'll approve the held workflow run and take a final look once the rebase lands. Note the attest job itself only triggers on |
|
Thanks — the placement critique is right. Reworked in Required
Recommended
Small notes
|
changes made, conflicts resolved best understanding
… contents:read The update-branch merge auto-resolved the CHANGELOG hunk into the already-released 0.6.0-dev.10 section; the entry belongs under the now-empty [Unreleased] > Added per review. Also the optional nit: attest-build-provenance needs only id-token + attestations — the artifact download uses the Actions artifacts API, so contents: read was unused.
Summary
Adds an
attestjob to.github/workflows/release.ymlthat produces aGitHub Artifact Attestation with SLSA build provenance for the release
archives. It fans in from the same immutable
build-job artifacts thatreleaseuploads, so the attestation covers the exact bytes usersdownload. Gated on
vars.ATTESTATIONS_ENABLEDso forks carry it verbatimas a no-op.
Why
mise install github:<owner>/jira-cliand equivalent flows verifyGitHub Artifact Attestations natively when present. Today mise reports
"SLSA provenance: no provenance found" for
jrreleases and falls backto unsigned tarball extraction. Adding provenance closes that gap for
every downstream that verifies (mise,
gh attestation verify, andSigstore-aware clients).
build artifacts and calls
actions/attest-build-provenance. No changesto build matrices, asset names, or the
releasejob itself.so on a fork with the variable set it attests the fork's assets under the
fork's identity; with the variable unset the file is a byte-identical
no-op.
Design decisions
— not download-and-attest after publish. The
attestjob fans in fromthe
buildmatrix's uploaded artifacts (actions/download-artifact,SHA-pinned,
merge-multiple: true) and attests those. Those are the sameimmutable objects
releasedownloads and uploads verbatim (softpropscopies bytes unchanged), so
gh attestation verify/ mise match onsha256 digest regardless of storage location. This makes the provenance
claim builder-side and eliminates the publish-then-attest window
(TOCTOU / CWE-362) — relevant on a signing-enabled fork where
sign-and-publish.ymlfires on the same workflow completion and--clobbers re-signed darwin assets.release, not strictly upstream of it. Both jobsneeds: build. Keepingattestout ofrelease'sneedsmeans thefork gate can skip
attestwithout cascading a skip onto the fork'srelease (a skipped dependency is treated as unmet). Both jobs read the
same immutable build artifacts, so the attested bytes still match the
uploaded bytes. (Open to a linear build→attest→release chain with an
always()-style guard onreleaseif the linear DAG is preferred.)if: vars.ATTESTATIONS_ENABLED == 'true', matchingthe release-ops convention (
SIGNING_ENABLED,HOMEBREW_TAP_REPO,RELEASE_GAP_FILL_ENABLED,SYNC_UPSTREAM_REPO). Attestations needid-token: writeand publish to the public Rekor log; they'reunavailable on GHES and on private forks without GitHub Enterprise Cloud.
Documented in the spec's variables table + CLAUDE.md gate line.
id-token: write,attestations: write,contents: read) — none is inherited.subject-pathcovers*.tar.gz+*.zip; the.sha256sidecars are integrity metadata already covered by the archiveattestation, so attesting them would be circular.
actions/attest-build-provenance@v4.1.1(
0f67c3f4856b2e3261c31976d6725780e5e4c373).harden-runnerat the workflow's current v2.19.4 pin — noversion drift introduced.
What this is not
sign-and-publish.yml. That fork-optional workflowre-signs the darwin binaries and emits
.pkg/.dmgwith differentdigests after notarization. Those need their own attest step there
(after codesign/notarize, before upload), gated on the same variable —
a follow-up PR. The coverage boundary is called out in an inline comment
on the attest step.
backfill-release.ymlorrelease-gap-fill.yml.GitHub Attestations path is sufficient).
Cargo.toml.Test plan
vars.ATTESTATIONS_ENABLED=trueandid-token: writepermitted: cut a tag — the build matrix runs,
attestandreleasebothfan in from the build artifacts,
attestemits provenance for thearchives. Verify with
gh attestation verify <asset> --repo <fork>andmise install github:<fork>/jira-cli@<tag>(mise reports "verify SLSAprovenance: success").
attestis skipped,releaseruns unchanged —the file is a no-op on forks that haven't opted in.