Skip to content

Verify release attestation offline against published bundle asset - #421

Open
evandowning wants to merge 8 commits into
mainfrom
worktree-offline-attestation-verify
Open

Verify release attestation offline against published bundle asset#421
evandowning wants to merge 8 commits into
mainfrom
worktree-offline-attestation-verify

Conversation

@evandowning

@evandowning evandowning commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Problem

curl -fsSL …/install.sh | bash fails for users outside the trailofbits org:

Loading attestations from GitHub API failed. Error: HTTP 403: Resource protected by
organization SAML enforcement

install.sh and coop update both ran gh attestation verify … --repo trailofbits/coop,
which fetches the Sigstore bundle from the GitHub attestations API. gh always attaches its
stored credential and refuses to run without one. Once a token is attached, trailofbits SAML
enforcement rejects it unless it carries an SSO session for the org, so the request 403s even
though the data is public. The affected user cannot fix it if they have no SSO access. The
failure is unrelated to artifact integrity — the SHA256SUMS check already passed before
verification aborted.

The same code path also fails for anyone who has gh installed but is not logged in: gh
exits 4 ("please run gh auth login") and the install is refused outright. That is a larger
population than SAML users, and their only escape today is uninstalling gh, which silently
downgrades them to checksum-only. Coverage goes up here, not just error quality.

Fix

Publish the provenance bundle as a release asset (attestations.jsonl) and verify it
with gh attestation verify --bundle, which makes no attestations-API call and needs no
credential. Both install paths stop depending on the caller's GitHub identity for the
verification step.

--bundle is credential-free, not offline: gh still fetches the Sigstore trust root from
tuf-repo.github.com unless --custom-trusted-root is supplied, and downloading the bundle
asset itself still uses gh/GITHUB_TOKEN when one exists.

  • .github/workflows/release.yml — give the attest step an id, normalize its bundle
    output to one-per-line with jq -c (runs on the runner only; not a user-facing dep), and
    publish attestations.jsonl alongside the tarballs and SHA256SUMS.
  • install.sh — download the bundle asset via the existing download_asset (gh → curl
    fallback) and pass --bundle. Both verification paths print a confirmation on success.
  • src/update.rs — same treatment for coop update: look up the attestations.jsonl
    asset in the release metadata, download it, and pass --bundle. The lookup is skipped when
    gh is absent or COOP_UPDATE_API_BASE_URL is overridden, so an update whose attestation
    step is a no-op does no pointless work.
  • Docs — README leads with the credential-free recipe; RELEASING.md adds the asset to
    the release checklist and pins VERSION in the credentials-stripped smoke test;
    docs/trust-model.md records that --bundle changes transport, not the guarantee.

Releases without the asset still install

Verification falls back to the API path — exactly what every release did before — when a
release publishes no bundle, or when the bundle download fails. Failing closed instead would
have broken the documented curl … | bash one-liner for every user between merge and the
next release (README serves install.sh from main, and latest is v0.5.4, which has no
asset), and would have broken VERSION=v0.5.x pinned installs permanently. So the bundle is
a strict improvement and never a regression.

install.sh and update.rs treat "no asset published" and "asset download failed" the same
way, because for the client they are the same situation: no bundle to read.

A bundle that downloads but fails to verify is refused, not retried through the API. To be
clear about what that does and does not buy: it is not stricter on integrity — a
substituted bundle fails --bundle and would then verify correctly against the genuine
attestation, and a tampered artifact fails both checks. The reason is diagnostic: a bundle
that downloaded and will not verify means a broken download or a gh that cannot read it,
and silently switching transports would hide both.

Verification strength is unchanged

This is not a softening of the update-verification chain. Verified against v0.5.4 with
credentials stripped (GH_CONFIG_DIR empty, GH_TOKEN/GITHUB_TOKEN unset):

Check Result
correct artifact + correct bundle, zero credentials exit 0
multi-subject bundle (what the release actually produces) exit 0
tampered tarball (one byte appended) exit 1 — integrity enforced
wrong --repo (trailofbits/not-coop) exit 1 — identity enforced
corrupt/truncated bundle exit 1 — install refused (see above for why not retried)
raw API JSON passed to --bundle exit 1

The bundle is signed and verified against Sigstore's trust root, and what defeats a
substituted bundle is the subject-digest bindinggh digests the artifact and requires
a matching subject. --repo pins the certificate's source repository, not the signer: any
workflow in trailofbits/coop with id-token: write + attestations: write satisfies it.
The API path is keyed by digest against the same repo-scoped store and is equally unpinned,
so fetching an already-signed object over an unauthenticated download changes the transport
and nothing else.

Review follow-ups (fd28783)

  • install.sh failure messaging — the credential explanation is now gated on gh
    actually reporting a 403 / SAML / gh auth login symptom. Previously a network error, a
    gh too old for the command, or a genuine provenance mismatch was all described as an SSO
    problem and pointed the user at a different release. Exercised with a stubbed gh across
    six outcomes: the explanation appears for the two credential cases and for neither the
    network error nor the "no matching attestations found" mismatch.
  • Symmetric fallback — a failed bundle download in update.rs falls back to the API
    instead of failing the update, matching install.sh. fetch_attestation_bundle is now
    infallible and returns Option<PathBuf> directly.
  • Tripwire hardening — the asset-name test asserts on the gh release create line, not
    just on the file containing the string. Confirmed it now fails when the asset is dropped
    from publication while the jq step still creates it; before, that regression stayed green.
  • release.yml step nameNormalize attestation bundle, which is what the step does;
    publishing happens in gh release create.
  • Deferred: making the API fallback itself credential-free (so pre-bundle releases also
    verify with no credential) is filed as Make the attestation-API fallback credential-free for pre-bundle releases #423 rather than grown into this PR.

Review follow-ups (1d649b4)

Second round; all 13 inline threads addressed.

  • Carry the fallback reasonfetch_attestation_bundle's Option<PathBuf> collapsed four
    outcomes and the error text then re-derived a reason from the surviving None, so a failed
    download reported "the release publishes no attestations.jsonl" on a release that publishes
    it. Replaced with a Provenance enum that carries the reason; the failure branch is now unit-
    tested in both directions.
  • Split the decision from the IObundle_decision(release, api_overridden, gh_present) is
    pure and covered over all four outcomes. This was the only new decision logic in the diff and
    had no coverage of any kind: src/update.rs is whole-module excluded in .cargo/mutants.toml
    and integration-update.sh short-circuits it via COOP_UPDATE_API_BASE_URL. It also removes
    the guard that duplicated verify_attestation's own skip checks by convention.
  • Reject empty bundlesjq -c . exits 0 on empty input, so the normalize step could publish
    a 0-byte asset green, blocking every gh-equipped install of an immutable release (and, on gh
    before 2.56.0, reporting success having verified nothing). Guarded in release.yml with [ -s ]
    and in both clients, which now fall back to the API rather than pass an empty file to --bundle.
  • Harden the installer tripwireinstall.sh names the asset literally only in its BUNDLE=
    declaration, so installer.contains(BUNDLE_ASSET) survived deleting the download and the
    --bundle verify. Now keyed on those lines; each of the three assertions was confirmed to fail
    on the regression it names, where the old check stayed green.
  • Say what was observed — the installer's silenced probe cannot tell "not published" from a
    failed download, so the fallback line no longer asserts the first. The gated credential
    explanation also moved to stderr, alongside the gh output it explains and the die that
    follows. RELEASING.md's release-acceptance step carries the same correction.
  • Docs — "offline" narrowed to "no attestations-API call, no credential" in all five places;
    --repo documented as pinning the certificate's source repository rather than the signer; the
    SSO/403 rationale reduced from four copies to docs/trust-model.md plus a short one in the
    standalone install.sh. docs/ARCHITECTURE.md updated for the third asset (raised out-of-diff).

Gates: shellcheck, bash -n, cargo fmt --check, cargo clippy --all-targets --all-features -D warnings clean; cargo test 1065 passed; ./tests/integration-update.sh 5 passed.
actionlint reports only the pre-existing SC2086 on main in the unrelated Package step.

Review follow-ups (ff03f8a)

Third round; all 5 inline threads addressed.

  • The bundle fetch is now credential-free. It went through download_asset, which
    prefers gh and then falls back to curl carrying $GITHUB_TOKEN — so on a
    SAML-restricted token the bundle download was the one step in the chain that could
    still 403, and when it did, verification fell back to the API and failed with exactly
    the original error. Both clients now fetch the asset with a bare unauthenticated
    request: download_bundle in install.sh, curl_download(url, dest, None) in
    update.rs. download_asset is unchanged for the tarball and SHA256SUMS, where a
    credential is harmless.
  • Credential attribution corrected. The attestations store is anonymously readable
    for a public repo; what needs a credential is gh attestation verify without
    --bundle, which refuses to run unless gh is logged in and then attaches its token.
    The API path was described as the thing requiring the credential in README, install.sh,
    update.rs (the Provenance::Api doc and three tracing messages), and
    docs/trust-model.md. All corrected.
  • The two transports are not interchangeable, and trust-model.md now says so instead
    of concluding "the transport change costs nothing here". The --bundle path accepts a
    superset (any correctly-signed bundle in a release needs only contents: write, while
    registering one in the attestation store needs attestations: write) and is unrevocable
    (Fulcio certificates carry no CRL/OCSP and gh consults no revocation source, so a
    saved bundle keeps verifying after the attestation is deleted). The conclusion is
    unchanged; it now rests on the subject-digest binding rather than on equivalence.
    --repo is also documented as constraining the signer SAN to that repo, just not to a
    specific workflow file or ref.
  • Installer tripwire hardened again. It keyed on download_asset && $BUNDLE, so it
    would have stayed green on the change above. It now asserts the call site uses
    download_bundle and that the helper's body is a bare curl with no GITHUB_TOKEN or
    gh release; each assertion was confirmed to fail on the regression it names.
  • release.yml comment corrected (commit 001326e). It claimed an empty asset would
    leave clients failing with "no API fallback", which contradicts the clients in this PR —
    both fall back on it. The unwarned risk is the opposite: a non-empty degenerate bundle,
    since a whitespace-only file passes [ -s ] and len() > 0 alike and gh's JSON-Lines
    loader then yields zero attestations, reported as success by gh before 2.56.0.

Gates: bash -n install.sh clean; cargo fmt --check, cargo clippy --all-targets --all-features -D warnings clean; cargo test 1065 passed; ./tests/integration-update.sh
5 passed. shellcheck and actionlint are unavailable in this environment and were not
re-run for this round.

Testing

  • shellcheck install.sh, bash -n install.sh — clean. actionlint reports only the
    pre-existing SC2086 on main in the unrelated Package step.
  • cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings,
    cargo test (1012 passed) — clean. ./tests/integration-update.sh — 5 passed.
  • End-to-end install.sh with zero credentials against a v0.5.4 release with the bundle
    asset shimmed in: installs cleanly. Corrupt bundle: refuses. No bundle: falls back to the
    API and reports the SSO cause on failure.
  • New unit tests pin the gh attestation verify argument list (with and without --bundle)
    and guard the attestations.jsonl name against drift between release.yml, install.sh,
    and update.rs. Both were confirmed to fail when the behavior is reverted.

Stage 2 (a real release carrying the asset) can only run after the next release publishes
attestations.jsonl.

🤖 Generated with Claude Code

install.sh ran `gh attestation verify --repo`, which fetches the Sigstore
bundle from the GitHub attestations API. `gh` always attaches its stored
credential, so a token without an SSO session for the trailofbits org 403s
on public data — external users cannot install even though the artifact is
public and its SHA256SUMS check already passed.

Publish the provenance bundle as a release asset (attestations.jsonl) and
verify it offline with `gh attestation verify --bundle`, which makes no API
call and needs no credential. The --repo identity constraint and the artifact
digest match are still enforced, so transporting the already-signed bundle
over an unauthenticated download does not weaken the guarantee.

- release.yml: give the attest step an id, normalize its bundle output to
  one-per-line with `jq -c`, and publish attestations.jsonl with the release.
- install.sh: download the bundle asset and pass --bundle; releases before
  the asset existed fail closed with a clear message.
- README.md / RELEASING.md: document the offline path and the credential-free
  manual recipe; strip credentials in the release smoke test.

src/update.rs has the identical bug (coop update shells out to the same
API-fetching verify) and needs the same fix in a follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@evandowning evandowning self-assigned this Jul 30, 2026
Review of #421 found two problems with the installer change.

install.sh failed closed when a release published no attestations.jsonl.
Since README serves the installer from main, merging would have broken
`curl … | bash` for every user until the next release: latest is v0.5.4,
which has no bundle asset, and the error told users to "install a newer
version" that does not exist. It also broke `VERSION=v0.5.x` pinned
installs permanently. Fall back to the API path in that case instead —
exactly what every release did before — so the bundle is a strict
improvement and never a regression. A bundle that fails to verify still
refuses the install; only a missing bundle falls back.

src/update.rs had the same API-only verification, so `coop update` still
403'd for the users this fixes. It now downloads the release's
attestations.jsonl and passes --bundle, with the same fallback. The
bundle is skipped when gh is absent or the API base is overridden, so a
fallible download cannot fail an update whose attestation step is a
no-op anyway.

Also: install.sh's no-gh hint and update.rs's equivalent advised the
credential-requiring recipe; both now show --bundle. README leads with
the offline recipe, RELEASING.md pins VERSION in the smoke test, and
docs/trust-model.md records that --bundle changes transport, not the
guarantee — --repo still pins signer identity and the bundle is signed.

Verified against v0.5.4 with GH_CONFIG_DIR empty and GH_TOKEN/GITHUB_TOKEN
unset: offline bundle path installs (exit 0), corrupt bundle refuses
(exit 1), missing bundle falls back to the API, and a multi-subject
bundle — which is what the release actually produces — verifies.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@hbrodin hbrodin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracking this down — the diagnosis is right and this is a genuinely useful catch. I'm looking into the bigger picture around this separately.

I verified the central claims independently rather than taking the description on faith, against the real v0.5.4 release with gh 2.93.0:

  • The attestations API is readable anonymously (200, 2 attestations). It really is only gh attaching its token that converts public data into a 403 — same class as cli/cli#6675. I also confirmed the obvious workaround does not exist: stripping the token and running the API path exits 4, so --bundle is the only credential-free route.
  • Credential-free --bundle verification works: correct artifact → exit 0 with GH_CONFIG_DIR empty and both token vars unset. Negative controls all fail closed — tampered tarball → 1, wrong --repo → 1, garbage bundle → 1, empty bundle → 1, raw API JSON as --bundle → 1.
  • So the guarantee is unchanged: Sigstore trust root, --repo signer identity, and subject-digest match are all still enforced offline. Agreed this is not a softening of the chain.
  • bundle-path is declared at the exact pinned action SHA, and --bundle documents and accepts JSON Lines — so the jq -c normalization is sound whichever shape the action emits.

One thing the description undersells in the other direction: this fixes a second, independent failure. On main, anyone with gh installed but not logged in gets exit 4 ("please run gh auth login") → die → the install is refused outright. That's a larger population than SAML users, and today their only escape is uninstalling gh, which silently downgrades them to checksum-only. Coverage genuinely goes up here.

Comments inline. The only one I'd want resolved before merge is the failure messaging in install.sh. Worth stating plainly though: the fix is latent — install.sh is served from main and latest is v0.5.4 with no asset, so the reported symptom persists for every user until the next release ships, and permanently for VERSION=v0.5.x pinned installs.

Comment thread install.sh Outdated
Comment thread src/update.rs Outdated
Comment thread src/update.rs Outdated
Comment thread src/update.rs Outdated
Comment thread install.sh
Comment thread .github/workflows/release.yml Outdated
Comment thread install.sh
- install.sh: gate the credential explanation on gh actually reporting a
  403 / SAML / "gh auth login" symptom. A network error, a gh too old for
  the command, or a genuine provenance mismatch previously got described
  as an SSO problem and pointed the user at a different release.
- install.sh: confirm a successful verify on both paths — the offline
  bundle path printed nothing, which read the same as a skip.
- update.rs: a failed bundle download now falls back to the API instead
  of failing the update, matching install.sh. One policy, one behavior.
  fetch_attestation_bundle is infallible, so it returns Option directly.
- update.rs: the asset-name tripwire now asserts on the `gh release
  create` line. Dropping the asset from publication alone left the test
  green while the asset silently stopped shipping.
- install.sh: record why a bundle that fails to verify is refused rather
  than retried — it is not stricter on integrity, it surfaces a broken
  download or an unusable gh.
- Docs: trust-model records the failed-download fallback and the
  refuse-don't-retry rule; RELEASING checks for the positive confirmation
  line.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evandowning

Copy link
Copy Markdown
Contributor Author

Thanks for the independent verification — especially the negative controls and the bundle-path-at-pinned-SHA check. All seven inline comments are addressed in fd28783 and the threads are resolved. Branch also merged up to main (b3da317, the proxy work) in 19a35c3.

Changed

  • Failure messaging (install.sh) — the blocker. gh output is captured, replayed to stderr unconditionally, and the credential explanation is gated on 403 / SAML / gh auth login. Verified with a stubbed gh: the explanation fires for the 403 and the exit-4 no-credential case, and stays silent for a DNS failure and for no matching attestations found. A real provenance mismatch is no longer described as an SSO problem.
  • Symmetric fallback (update.rs) — a failed bundle download falls back to the API instead of failing the update, matching install.sh. fetch_attestation_bundle had no fallible path left, so it returns Option<PathBuf> rather than Result<Option<…>>.
  • Tripwire hardening — the asset-name test asserts on the gh release create line. Confirmed it fails on the exact regression you described (asset dropped from publication while the jq step still creates it); the old assertion stayed green on that edit.
  • Success confirmation — both paths now print one, so the offline path is no longer indistinguishable from a skip. RELEASING.md checks for that positive line instead of the absence of a negative one.
  • Step nameNormalize attestation bundle.
  • Rationale correction — you were right that "refusing is stricter" does not hold. Removed from the description and replaced at the call site with the actual reason (it surfaces a broken download or an unusable gh; it buys nothing on integrity). docs/trust-model.md now states both the failed-download fallback and the refuse-don't-retry rule.

Deferred to #423 — making the API fallback itself credential-free, with your verified curl | jq -c | --bundle recipe. That is the part that reaches latest and VERSION=v0.5.x pinned installs, so it is the real fix for the latency you flagged; kept separate because it changes fallback transport rather than the bundle mechanism.

Gates on the merged tree: shellcheck, bash -n, cargo fmt --check, cargo clippy --all-targets --all-features -D warnings clean; cargo test 1061 passed; ./tests/integration-update.sh 5 passed. src/update.rs is in exclude_globs in .cargo/mutants.toml, so no mutation-scope change is needed for the new code.

@evandowning
evandowning requested a review from hbrodin July 30, 2026 18:40
Comment thread src/update.rs Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread install.sh Outdated
Comment thread src/update.rs Outdated
Comment thread docs/trust-model.md Outdated
Comment thread README.md Outdated
Comment thread src/update.rs Outdated
Comment thread install.sh Outdated
Comment thread src/update.rs Outdated
Comment thread src/update.rs Outdated
Comment thread RELEASING.md Outdated
Comment thread install.sh Outdated
Comment thread .github/workflows/release.yml Outdated
@hbrodin

hbrodin commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

13 findings posted inline.

Three claims I verified directly rather than reasoning about, since the review turns on them:

  • attestations.jsonl appears as a literal in install.sh exactly once — line 13, BUNDLE="attestations.jsonl". Every use site goes through ${BUNDLE}, so the new tripwire's installer.contains(BUNDLE_ASSET) is satisfied by the declaration alone and cannot fail as its message claims.
  • jq -c . on an empty input exits 0 and writes 0 bytes (: > empty.json; jq -c . empty.json → exit 0, 0 bytes), so the normalize step can publish an empty bundle green.
  • src/update.rs is whole-module excluded in .cargo/mutants.toml exclude_globs, which confirms both that no mutants.toml update is owed by this PR and that fetch_attestation_bundle has no mutation coverage either.

Out of diff, so not inline: docs/ARCHITECTURE.md:176-179 still describes the pre-PR asset set — "download the platform tarball + SHA256SUMS, verify the checksum (mandatory), verify Sigstore attestation via gh (best-effort)". The chain now downloads a third asset and the attestation step is no longer a bare gh attestation verify. The paragraph does defer to trust-model.md for the full chain, so it is a one-clause fix, but CLAUDE.md's cross-file-sync rule points at it.

No diff noise. shellcheck is clean on the post-change install.sh; the new workflow step omits shell: exactly like its siblings in the release job and uses the env:-indirection zizmor wants; the test module's #[expect(clippy::unwrap_used, …)] matches every other test module in the repo.

Checked and deliberately not raised: the #421 reference in the BUNDLE_ASSET doc comment (bare #NNN cross-references have clear precedent — #349, #411, #303, #147, anthropics/claude-code#8938); the include_str! note in the tripwire test (added at a reviewer's explicit request in the previous round); the bundle-vs-API strictness rationale at install.sh:134-138 as a design question (reviewer said they were not asking for a change — the inline comment there is scoped narrowly to the failure-cause enumeration omitting digest mismatch); and the credential-free-API-fallback idea, which was explicitly deferred to a follow-up.

All seven prior inline threads were confirmed genuinely addressed at 19a35c3, not just marked resolved.

Coverage: all eight agents ran — review-correctness, review-design, review-conventions, review-security (update-chain / subprocess / credential surfaces), review-api-usage (gh attestation verify, actions/attest-build-provenance v4.1.1, jq), review-tests, review-docs, review-comments. None skipped. 21 raw findings deduplicated to 13 after per-file validation against the post-change tree; 8 dropped as ungrounded, already-handled by repo convention, previously-resolved, or folded into a stronger neighbour.

Second review round on #421.

- `update.rs`: replace `Option<PathBuf>` with `Provenance`, which carries
  why there is no bundle instead of re-deriving it from `None`. A failed
  download no longer reports "the release publishes no attestations.jsonl"
  on a release that publishes it. Split the decision from the IO as
  `bundle_decision(release, api_overridden, gh_present)`, so all four
  outcomes are unit-testable; this also drops the guard that duplicated
  `verify_attestation`'s own skip checks by convention.
- Reject an empty bundle in both clients and in `release.yml`. `jq -c .`
  exits 0 on empty input, so the step could publish a 0-byte asset that
  blocks every `gh`-equipped install of an immutable release; `gh` before
  2.56.0 reports success on one, having verified nothing.
- Harden the asset-name tripwire on the installer half. `install.sh` names
  the asset literally only in its `BUNDLE=` declaration, so the old
  `contains` check survived deletion of the download and the `--bundle`
  verify. Now keyed on those lines.
- `install.sh`: the fallback message no longer states "not published" as
  the only cause of a silenced probe failure, and the credential
  explanation moves to stderr alongside the `gh` output and the `die`.
- Docs: `--bundle` means no attestations-API call and no credential, not
  offline — `gh` still fetches the Sigstore trust root, and the bundle
  download itself uses a credential when one exists. Record that `--repo`
  pins the certificate's source repository, not the signer, and that the
  subject-digest binding is what defeats a substituted bundle. Correct the
  RELEASING.md diagnosis and the `coop update` asset list in ARCHITECTURE.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evandowning

Copy link
Copy Markdown
Contributor Author

All 13 inline findings addressed in 1d649b4 and the threads resolved. Branch was already current with main (b3da317), so no merge this round.

Three of these were bugs in things the previous round added or claimed, which is a fair hit rate on a round that was itself review follow-up:

  • The download-failure path told the user the wrong thing. The Option<PathBuf> I introduced last round to fix the ?-aborts-the-update thread collapsed four outcomes into one None, and the error text re-derived a reason from it — so a failed download reported "the release publishes no attestations.jsonl" on a release that publishes it, contradicting the warn! from moments earlier. Now a Provenance enum carrying the reason, so the wrong one is unrepresentable.
  • The tripwire I hardened last round was still weak on its other half. Your check that attestations.jsonl appears literally in install.sh exactly once was the whole finding: installer.contains(BUNDLE_ASSET) was satisfied by the BUNDLE= declaration, so deleting the download and the --bundle verify left it green. Three behavior-keyed assertions now, each confirmed to fail on the regression it names.
  • jq -c . exits 0 on empty input. The normalize step could publish a 0-byte asset green, which is worse than no asset — every gh-equipped install of an immutable release blocked, with the no-fallback rule I argued for last round firing. Guarded in release.yml and in both clients.

On the two documentation corrections: "offline" and "--repo pins the signer" were both wrong, and both were load-bearing in how I described this change. --bundle is credential-free but still fetches the Sigstore trust root; --repo pins the certificate's source repository, and the subject-digest binding is what actually defeats a substituted bundle. Corrected in the five places you listed, in docs/trust-model.md as the system of record, and in the PR description, which asserted both.

Coverage note. bundle_decision and api_fallback_hint are pure and directly unit-tested, but src/update.rs stays whole-module excluded in .cargo/mutants.toml — un-excluding it to reach two functions would surface the shell-out and network mutants the exclusion exists for. So the new logic has unit coverage, not mutation coverage.

Still deferred to #423: the credential-free API fallback. Unchanged reasoning — it changes fallback transport rather than the bundle mechanism, and it is the piece that reaches latest and VERSION=v0.5.x pinned installs.

Verification for the behavior changes, beyond the gates:

check result
tripwire: drop --bundle verify from install.sh FAILED as named (old assertion: green)
tripwire: drop the download_asset "$BUNDLE" probe FAILED as named (old assertion: green)
tripwire: rename BUNDLE= FAILED as named
release.yml step on empty / whitespace / malformed input fails all three (old body: exit 0, 0-byte asset)
installer with a 0-byte bundle falls back to API; --bundle never passed
installer, SAML 403 / gh auth login explanation printed, on stderr with the gh output and the die
installer, DNS failure / no matching attestations found no credential explanation
installer, digest mismatch on the bundle path refused, exit 1

cargo test 1065 passed, ./tests/integration-update.sh 5 passed, cargo fmt --check / cargo clippy --all-targets --all-features -D warnings / shellcheck / bash -n clean.

@evandowning
evandowning requested a review from hbrodin August 4, 2026 13:15
Comment thread install.sh Outdated
Comment thread src/update.rs Outdated
Comment thread README.md Outdated
Comment thread docs/trust-model.md Outdated
Comment thread .github/workflows/release.yml Outdated
@hbrodin

hbrodin commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Evan — this is a long-ish list and I want to say up front why. It's the release-verification path, so a mistake here gets baked into an immutable tag and can't be re-cut. I've been deliberately picky as a result, and that's about the blast radius, not about the work.

I checked the claims in the description rather than taking them on trust, and they held up:

  • --bundle really is credential-free (DisableAuthCheckFlag is attached to that flag and only that flag), and I verified end-to-end with GH_CONFIG_DIR empty and no tokens: correct artifact → exit 0, tampered tarball → exit 1, wrong --repo → exit 1.
  • The .json/.jsonl extension dispatch is real, so the asset name genuinely is load-bearing — a .txt copy of the same bytes is refused.
  • The gh 2.56.0 empty-bundle behaviour and the gh attestation verify handles empty JSONL files cli/cli#9541 reference are both correct.
  • bundle-path really is a single line covering all three subjects, so jq -c reshapes nothing, exactly as you said.

The round-2 fixes all landed. And the premise is still current: cli/cli#11803 (unauthenticated verify) is open, so no newer gh makes this go away on its own.

Two things are worth your time — both posted inline:

  1. The bundle download still goes through gh/GITHUB_TOKEN, so the credential can still bite on the one step this PR is making credential-free. Small fix, and it's the difference between the feature mostly working and fully working.
  2. A couple of doc lines say the attestations API requires a credential. It doesn't — it's anonymously readable, and it's gh that attaches the token. That's good news: it makes Make the attestation-API fallback credential-free for pre-bundle releases #423 considerably easier than it currently looks.

Happy to push those two fixes myself if you'd rather not spin another round on this.


Optional — take or leave

None of these block. Listing once rather than as separate threads:

  • src/update.rs:415 / :451BundleDecision and Provenance model one thing; three of four variants map 1:1. Passing the two bools into resolve_provenance directly would delete the enum, its lifetime and two tests with coverage unchanged. ApiReason's only consumer is a string, and each cause is now stated twice (once in the tracing call, once reworded in api_fallback_hint).
  • src/update.rs:1150 — the tripwire keys on source formatting: it needs gh release create and the asset name on the same physical line, so wrapping line 151 into a block scalar false-fails it, while if: false on the publish step keeps it green.
  • src/update.rs:1090 — the two attestation_verify_args tests assert a literal vec! against the same REPO constant the function interpolates, so the --repo pin can't fail. Spelling the repo literally would fix that.
  • src/update.rs:541 / install.sh:144 — the empty-bundle guards are byte-count tests; a whitespace-only file passes both. Also is_ok_and folds a failed fs::metadata into the "is empty" branch, so a missing file reports the wrong reason.
  • src/update.rs:569 — no success confirmation on the bundle path in the Rust client; install.sh:152 got one, so a successful coop update is silent and reads like a skip.
  • .github/workflows/release.yml:127jq -c . validates JSON syntax, not that the file is a Sigstore bundle. Since bundle-path is already one compact line, cp does the same job. The check that would actually cover it is free and credential-free in the same job: gh attestation verify coop-<triple>.tar.gz --repo ${{ github.repository }} --bundle attestations.jsonl — the tarballs are already in the working directory, and it subsumes both guards plus a subject-digest mismatch.
  • docs/ARCHITECTURE.md:180 — the participial "falling back…" interrupts a comma-joined verb list and pushes "(best-effort)" five clauses from the verb it qualifies; the next line already delegates this detail to trust-model.md.

Not in the diff, so no line to hang them on

  • docs/trust-model.md:60 — the taint-source inventory still reads "tarball + SHA256SUMS". This adds a third downloaded artifact whose path goes into a subprocess argument, and it's the one gated by neither checksum nor attestation.
  • .cargo/mutants.toml:29src/update.rs is still whole-module excluded while this PR adds four pure helpers, so the new tests aren't verified to kill anything.
  • docs/commands.md:824 — the only user-facing description of the update chain not touched here, while README and ARCHITECTURE both were.

Deferred

The signer-identity gap (neither client passes --signer-workflow/--cert-identity, so any workflow in the repo with id-token: write satisfies --repo) is real but is a release-process policy decision, not a defect in this PR — better as its own issue. Note if we do it: --signer-workflow needs gh ≥ 2.51.0 and compiles to a start-anchored-only regex, so release.yml also prefix-matches release.yml.evil; --cert-identity-regex with an explicit $ is the tight form.

On keeping the asset

Worth recording, since it argues for the approach you took over the #423 shortcut: bundle_url from the API can't be used directly — it's Snappy-compressed (I decoded one to confirm) and served from a ~1-hour pre-signed URL. So the anonymous-API route depends on .attestations[].bundle, which GitHub doesn't document, and the anonymous API is 60 req/hr per IP — we exhausted it during this review. The published asset avoids both. If #423 lands, it's the fallback, not the replacement.

@hbrodin

hbrodin commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Filed the signer-identity item as #436 rather than growing this PR — it's a release-process policy decision, not a defect here, and it needs a couple of research questions answered first (notably an audit of the cert identity of every past release, since a strict pin could break VERSION= installs of old tags).

Two things I turned up while writing it up that are worth knowing for this PR:

  • The attestations API returns two attestations per digest for v0.5.4 — one from GitHub's immutable-releases signer (https://dotcom.releases.github.com, covering the tarballs and SHA256SUMS) and one from actions/attest-build-provenance (SAN …/release.yml@refs/tags/v0.5.4, covering the three tarballs). The attestations.jsonl asset carries only the second, since it comes from the attest step's bundle-path.
  • That's fine for verification — nothing here verifies SHA256SUMS itself — but it does mean the asset and the API path carry different contents, which is worth a line in docs/trust-model.md if you're editing it anyway.

evandowning and others added 3 commits August 31, 2026 08:26
The bundle download went through `download_asset`, which prefers `gh` and
then falls back to curl carrying `$GITHUB_TOKEN`. That re-attached the
credential `--bundle` exists to avoid: on a SAML-restricted token the fetch
was the one step in the chain that could still 403, dropping back to the API
path and failing with the original error. Both clients now fetch the asset
with a bare, unauthenticated request — `download_bundle` in `install.sh`,
`curl_download(url, dest, None)` in `update.rs` — so the path is
credential-free end to end, as the README already claimed.

Correct the credential attribution throughout: the attestations API is
anonymously readable for a public repo. What needs a credential is `gh
attestation verify` without `--bundle`, which refuses to run unless logged in
and then attaches its token.

Record the two ways the transports are not interchangeable in
`docs/trust-model.md`: the `--bundle` path accepts a superset (any
correctly-signed bundle in a release needs only `contents: write`, while
registering one in the attestation store needs `attestations: write`), and it
is unrevocable (Fulcio certs carry no CRL/OCSP and `gh` consults no
revocation source, so a saved bundle keeps verifying after the attestation is
deleted). Also state that `--repo` constrains the signer SAN to that repo,
just not to a workflow file or ref.

The installer tripwire keyed on `download_asset`, so it would have stayed
green on both regressions. It now asserts the call site uses
`download_bundle` and that the helper's body is a bare curl with no
`GITHUB_TOKEN` or `gh release`; each assertion was confirmed to fail on the
regression it names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment claimed an empty asset would leave clients failing verification
"with no API fallback". Both clients fall back on it: `install.sh` gates on
`[ -s ]` and `resolve_provenance` returns `ApiReason::EmptyBundle`.

The unwarned risk is the opposite one — a non-empty degenerate bundle. A
whitespace-only file passes `[ -s ]` and `len() > 0` alike, and `gh`'s
JSON-Lines loader then yields zero attestations, which `gh` before 2.56.0
(cli/cli#9541) reports as success. `jq -c .` is what makes that unreachable,
so it has to stay upstream of publication.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…station-verify

# Conflicts:
#	.github/workflows/release.yml
@evandowning
evandowning requested a review from hbrodin August 31, 2026 13:33
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.

2 participants