Skip to content

Verify release attestation offline against published bundle asset #421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
evandowning wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Verify release attestation offline against published bundle asset #421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
996194d
Verify release attestation offline against published bundle asset
evandowning Jul 30, 2026
9534c09
Verify coop update offline too, and keep old releases installable
evandowning Jul 30, 2026
fd28783
Address review: scope failure messaging, symmetric bundle fallback
evandowning Jul 30, 2026
19a35c3
Merge remote-tracking branch 'origin/main' into worktree-offline-atte…
evandowning Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,18 @@ jobs:
run: sha256sum coop-*.tar.gz > SHA256SUMS

- name: Attest build provenance
id: attest
uses: actions/attest-build-provenance@0f67c3f4856b2e3261c31976d6725780e5e4c373 # v4.1.1
with:
subject-path: "coop-*.tar.gz"

# Normalize to one bundle per line so the published asset has a stable
# shape regardless of whether the action emits a single bundle or several.

@hbrodin hbrodin Jul 30, 2026

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.

The stated rationale describes a shape the pinned action cannot emit, and omits the constraint that actually makes this step necessary.

At actions/attest-build-provenance v4.1.1 (a wrapper over actions/attest v4.1.1), the action writes bundle-path to <mkdtemp>/attestation.json as exactly one JSON.stringify(att.bundle) line, and multiple subject-path matches produce a single attestation referencing all three tarballs rather than several bundles — so "regardless of whether the action emits a single bundle or several" is a case that cannot occur, and the one-bundle-per-line normalization is a no-op on content. (A multi-bundle file would also have worked: gh uses any-match semantics across a .jsonl, erroring only when verifyCount == 0, and sigstore-go matches the artifact digest against every entry of statement.Subject.)

What the step does accomplish, unremarked, is giving the asset a .jsonl extension: gh dispatches on extension and rejects anything else with bundle file extension not supported, must be json or jsonl. The published asset name is therefore load-bearing for --bundle, and a future "simplification" to e.g. attestations.txt would break verification. Worth saying that instead — a plain cp would do the same work.

- name: Normalize attestation bundle
env:
BUNDLE_PATH: ${{ steps.attest.outputs.bundle-path }}
run: jq -c . "$BUNDLE_PATH" > attestations.jsonl

@hbrodin hbrodin Jul 30, 2026

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.

jq -c . exits 0 and writes zero bytes when its input is empty or whitespace-only (confirmed: : > empty.json; jq -c . empty.json → exit 0, 0 bytes), so a degenerate bundle-path yields a green step and a 0-byte attestations.jsonl release asset.

That is worse for clients than the asset being absent, because neither client checks the bundle it got. install.sh's probe succeeds (the file downloads fine), gh attestation verify --bundle <empty> then fails, and the deliberate no-fallback rule fires — die "... refusing to install". coop update fails identically via fetch_attestation_bundle returning Some(dest) on any Ok(()). Every gh-equipped install of that release is blocked with no API fallback, and the version cannot be re-cut. (On gh older than 2.56.0 it fails the other way: cli/cli#9541 fixed empty-bundle handling, and before that an empty JSONL reported success — so the installer would print "Attestation verified offline" having verified nothing.)

The release-notes step three below already models the guard: if [ ! -s RELEASE_NOTES.md ]; then ... exit 1; fi. Suggest jq -c . "$BUNDLE_PATH" > attestations.jsonl && [ -s attestations.jsonl ], and/or a non-empty check before passing --bundle in both clients.


- name: Extract release notes from CHANGELOG
env:
TAG: ${{ github.ref_name }}
Expand All @@ -128,4 +136,4 @@ jobs:
env:
GH_TOKEN: ${{ github.token }}
TAG: ${{ github.ref_name }}
run: gh release create "$TAG" --title "$TAG" --notes-file RELEASE_NOTES.md coop-*.tar.gz SHA256SUMS
run: gh release create "$TAG" --title "$TAG" --notes-file RELEASE_NOTES.md coop-*.tar.gz SHA256SUMS attestations.jsonl
26 changes: 21 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,28 @@ attestation via [`actions/attest-build-provenance`](https://github.com/actions/a
The attestation proves the artifact was built from this repository by the
tagged release workflow.

Both `install.sh` and `coop update` run this verification automatically
when the [GitHub CLI](https://cli.github.com/) is installed. Without `gh`,
they fall back to checksum verification against the release's `SHA256SUMS`
and print a note explaining what was and wasn't verified.
Both `install.sh` and `coop update` run this verification automatically when
the [GitHub CLI](https://cli.github.com/) is installed. Both verify offline
against the `attestations.jsonl` bundle published with the release, so
verification makes no GitHub API call and needs no authentication. Releases

@hbrodin hbrodin Jul 30, 2026

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.

This over-claims relative to docs/trust-model.md:218, which states it precisely ("no attestations-API call"). Verification as a whole does reach GitHub, with a credential when one exists, to obtain the bundle: install.sh's download_asset tries gh release download "$VERSION" --repo "$REPO" ... first and only curls on failure, and coop update routes the bundle through download_asset → select_auth_strategy_from_env(), i.e. AuthStrategy::Gh or AuthStrategy::CurlBearer(token) when GITHUB_TOKEN is set — on top of the api.github.com release-metadata fetch it already performs. Only the gh attestation verify step is credential-free.

That matters for exactly the SSO-less user this PR targets, whose gh release download of the bundle can itself 403. Aligning the README with the trust-model wording keeps the security spec and the user-facing doc from drifting.

published before that asset existed are verified through the GitHub
attestations API instead, which does require a credential authorized for the
`trailofbits` org. Without `gh`, both fall back to checksum verification
against the release's `SHA256SUMS` and print a note explaining what was and
wasn't verified.

To verify a downloaded tarball manually, download `attestations.jsonl` from the
same release and pass `--bundle`:

To verify a downloaded tarball manually:
```sh
gh attestation verify coop-<version>-<triple>.tar.gz --repo trailofbits/coop \
--bundle attestations.jsonl
```

That needs no GitHub credential. Dropping `--bundle` makes `gh` fetch the
attestation from the API instead, which requires one — and fails with
`HTTP 403: Resource protected by organization SAML enforcement` if your token
carries no SSO session for the org:

```sh
gh attestation verify coop-<version>-<triple>.tar.gz --repo trailofbits/coop
Expand Down
16 changes: 14 additions & 2 deletions RELEASING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,23 @@ CI can't run the full VM integration suite or the extra-toolchain checks
This triggers `release.yml`.

9. **Verify the published release.** On the GitHub release page confirm:
- three `coop-vX.Y.Z-<target>.tar.gz` artifacts plus `SHA256SUMS`,
- three `coop-vX.Y.Z-<target>.tar.gz` artifacts plus `SHA256SUMS` and
`attestations.jsonl`,
- the build-provenance attestation is attached,
- the notes match the `## vX.Y.Z` CHANGELOG section.

Then smoke-test the install path (`install.sh`) against the new tag.
Then smoke-test the install path with credentials stripped, so the offline
bundle verification is exercised as an external user sees it. Pin `VERSION`
to the tag you just pushed rather than relying on "latest":

```bash
env -u GH_TOKEN -u GITHUB_TOKEN GH_CONFIG_DIR="$(mktemp -d)" \
VERSION=vX.Y.Z INSTALL_DIR="$(mktemp -d)" bash install.sh
```

The run must print `Attestation verified offline against attestations.jsonl`.
A "No `attestations.jsonl` published" line instead means the asset is missing

@hbrodin hbrodin Jul 30, 2026

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.

The diagnosis states one cause as the only cause. install.sh prints the "No attestations.jsonl published" line whenever the silenced probe download_asset "$BUNDLE" ... > /dev/null 2>&1 returns non-zero — a network blip, a TLS failure, a 5xx, or a gh+curl double failure all produce it exactly like a genuinely missing asset. install.sh's own comment concedes the probe cannot distinguish them.

A maintainer following this step after a transient failure would conclude the release asset is missing — a costly wrong conclusion given releases are immutable and the remedy is a version bump. Suggest: "means the bundle could not be downloaded — confirm the asset on the release page (step 9's first bullet) before concluding it is missing."

and verification fell back to the credential-requiring API path.

## If the tag run fails

Expand Down
18 changes: 13 additions & 5 deletions docs/trust-model.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,19 @@ Self-update (`update.rs`) must preserve, in order:
3. **Mandatory checksum.** The `SHA256SUMS` asset must be present (install is
refused otherwise) and every downloaded tarball is verified against it
(`verify_sha256`, constant-size `Sha256Hash` compare).
4. **Best-effort attestation.** `gh attestation verify --repo trailofbits/coop`
(Sigstore provenance). Skipped with a logged note if `gh` is absent, and
skipped entirely when `COOP_UPDATE_API_BASE_URL` is overridden (test mode).
So provenance is *not* guaranteed on hosts without `gh` — checksum is the
floor.
4. **Best-effort attestation.** `gh attestation verify --repo trailofbits/coop
--bundle attestations.jsonl` (Sigstore provenance), against the bundle asset
downloaded from the same release. `--bundle` makes verification offline: no

@hbrodin hbrodin Jul 30, 2026

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.

Two corrections to this paragraph, in the document that is the checklist future reviewers apply.

"offline" overstates what --bundle does. The no-credential half is right — gh marks the bundle flag with cmdutil.DisableAuthCheckFlag, so no token or gh auth login is needed, true since gh v2.49.0. But --bundle does not make the command network-free: with no --custom-trusted-root, NewLiveSigstoreVerifier builds TUF clients against https://tuf-repo.github.com and the Sigstore public-good CDN with CacheValidity = 1 (one day), and fails with "no valid Sigstore verifiers could be initialized" if neither can be reached. There is no --offline flag; GitHub's own offline-verification doc requires supplying the root out of band via gh attestation trusted-root + --custom-trusted-root. Narrowing the wording to "no attestations-API call, no credential" is accurate and is already enough to justify the change. The same overstatement is in README.md:134-136, install.sh:141, src/update.rs:391-392, and RELEASING.md:100/109.

--repo does not pin the signer. It pins the certificate's source repository: any workflow on any ref in trailofbits/coop holding id-token: write + attestations: write mints a bundle that satisfies --repo trailofbits/coop; gh exposes --signer-workflow / --cert-identity for the actual pin, and neither attestation_verify_args nor install.sh passes one. What defeats a substituted bundle attesting a different artifact is the subject-digest binding (gh digests the artifact and requires a matching subject), not --repo. The "does not weaken the check" conclusion still holds — the API path is keyed by digest against the same repo-scoped store and is equally unpinned as to workflow — but since this diff newly moves the signer material into a release asset, recording the real pin boundary here is worth doing.

Refs: https://cli.github.com/manual/gh_attestation_verify · https://docs.github.com/en/actions/security-for-github-actions/using-artifact-attestations/verifying-attestations-offline

attestations-API call, so no GitHub credential — but it does *not* weaken the
check, because the bundle is signed and `--repo` still pins the signer
identity, so a substituted or tampered bundle fails. A release that publishes
no bundle asset — or a bundle whose download fails — falls back to the API
path (credential required); `update.rs` and `install.sh` treat those two
cases identically. A bundle that downloads but fails to verify is refused
outright, not retried through the API. Skipped with a logged note if `gh` is
absent, and skipped entirely when `COOP_UPDATE_API_BASE_URL` is overridden
(test mode). So provenance is *not* guaranteed on hosts without `gh` —
checksum is the floor.
5. Extraction with `tar -xzf --no-same-owner --no-same-permissions` (path-escape
safe), then an atomic `rename`-over-self.

Expand Down
52 changes: 46 additions & 6 deletions install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ set -euo pipefail

REPO="trailofbits/coop"
BINARY="coop"
BUNDLE="attestations.jsonl"
INSTALL_DIR="${INSTALL_DIR:-${HOME}/.local/bin}"

# --- helpers ----------------------------------------------------------------
Expand Down Expand Up @@ -112,17 +113,56 @@ verify_checksum() {

verify_attestation() {
local file="$1"
if has gh; then
info "Verifying attestation..."
gh attestation verify "$file" --repo "$REPO" \
|| die "Attestation verification failed for $(basename "$file") — refusing to install"
else
if ! has gh; then
info "Note: \`gh\` not installed — skipped cryptographic attestation verification."
info "The download was verified against the published \`SHA256SUMS\` checksum, which"
info "is the same assurance level as most \`curl | bash\` installers. For end-to-end"
info "Sigstore verification, install \`gh\` (https://cli.github.com) and re-run, or"
info "verify manually: \`gh attestation verify <tarball> --repo ${REPO}\`."
info "verify manually: \`gh attestation verify <tarball> --repo ${REPO} \\"
info " --bundle ${BUNDLE}\` against the ${BUNDLE} asset from the same release."
return 0
fi

info "Verifying attestation..."
Comment thread
evandowning marked this conversation as resolved.
# Prefer the bundle published with the release: `gh attestation verify`
# without --bundle queries the GitHub attestations API, and gh always
# attaches its stored token, so a token lacking an SSO session for the org
# 403s on public data. The --repo identity check is enforced either way.
# The probe is silenced: a missing bundle is expected on older releases, so
# curl's bare "404" would read as a hard error.
if download_asset "$BUNDLE" "${TMPDIR}/${BUNDLE}" > /dev/null 2>&1; then
# A failure here is not retried through the API. That is no stricter on

@hbrodin hbrodin Jul 30, 2026

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.

The enumeration omits the security-relevant cause. gh attestation verify --bundle fails precisely when the tarball's digest does not match the bundle's signed subject — a tampered tarball published alongside a matching SHA256SUMS is caught here and nowhere else in the installer. Telling a future reader this failure "means a broken download or a gh that cannot read it" invites them to read the die as an availability nuisance and add the API retry the comment argues against.

Worth adding integrity failure to the list, or trimming the block to the one durable why: not retried through the API, because switching transports on a bundle that downloaded but will not verify would mask the failure.

# integrity — a substituted bundle fails here and would then verify
# correctly against the genuine attestation — but a bundle that
# downloaded and will not verify means a broken download or a `gh` that
# cannot read it, and switching transports would hide both.
gh attestation verify "$file" --repo "$REPO" --bundle "${TMPDIR}/${BUNDLE}" \
|| die "Attestation verification failed for $(basename "$file") — refusing to install"
Comment thread
evandowning marked this conversation as resolved.
info "Attestation verified offline against ${BUNDLE} — no GitHub credential used."
return 0
fi

# Releases published before the bundle asset existed verify through the
# API, exactly as every release did before. That needs a credential
# authorized for the org, so it is the fallback and not the default.
info "No ${BUNDLE} published for ${VERSION} — verifying through the GitHub API instead."

@hbrodin hbrodin Jul 30, 2026

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.

This line asserts as fact something the code has just discarded. It runs on every non-zero exit of the silenced probe download_asset "$BUNDLE" "${TMPDIR}/${BUNDLE}" > /dev/null 2>&1, and download_asset returns non-zero for a 404 and for DNS/TLS failure, a connection reset, a 5xx, a gh release download auth failure, a curl partial transfer, or an unwritable $TMPDIR — with 2>&1 throwing away the evidence that would tell them apart.

On a current release plus a transient blip, the user is told the release publishes no provenance bundle, verification silently moves to the credential-requiring transport, and if their token has no SSO session the gate below then prints ${VERSION} predates the ${BUNDLE} asset — flatly false for that tag. The comment block above this line makes the same claim ("Releases published before the bundle asset existed verify through the API").

src/update.rs keeps the two apart deliberately, with a tracing::info! for the missing asset and a tracing::warn! carrying ({err:#}) for a failed download — so the installer is the weaker of the two, and RELEASING.md:110 builds a release-acceptance check directly on this wording. Suggest stating what was observed: Could not fetch ${BUNDLE} for ${VERSION} (not published, or the download failed) — verifying through the GitHub API instead.

local out
if out="$(gh attestation verify "$file" --repo "$REPO" 2>&1)"; then
info "Attestation verified through the GitHub API."
return 0
fi
printf '%s\n' "$out" >&2
# This path also fails on a network error, a gh too old for the command, or
# a genuine provenance mismatch, so only explain the credential requirement
# when gh actually reported one of its symptoms.
case "$out" in
*403* | *SAML* | *"gh auth login"*)
info "${VERSION} predates the ${BUNDLE} asset, so verification used the GitHub API,"

@hbrodin hbrodin Jul 30, 2026

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.

These three lines go to stdout — info() { printf ' %s\n' "$1"; } — while the captured gh output above them (printf '%s\n' "$out" >&2) and the die below them both go to stderr. For a curl … | bash installer, which is the documented usage, that separates the explanation of the failure from the failure itself whenever the two streams are redirected differently, and lets them interleave out of order on a terminal.

Suggest emitting the explanation on stderr too — a warn-style helper, or printf … >&2 — so the whole failure report stays on one stream.

info "which needs a credential authorized for the org. Installing a release that"
info "publishes ${BUNDLE} avoids the API entirely."
;;
esac
die "Attestation verification failed for $(basename "$file") — refusing to install"
}

# --- main -------------------------------------------------------------------
Expand Down
162 changes: 152 additions & 10 deletions src/update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//! to nudge users when a newer release is available.

use std::env;
use std::ffi::OsString;
use std::fs;
use std::io::IsTerminal as _;
use std::os::unix::fs::PermissionsExt as _;
Expand All @@ -25,6 +26,8 @@ use crate::sha256_hash::Sha256Hash;

const REPO: &str = "trailofbits/coop";
const DEFAULT_API_BASE: &str = "https://api.github.com";
/// Release asset holding the Sigstore provenance bundle, published since #421.
const BUNDLE_ASSET: &str = "attestations.jsonl";
const DEFAULT_CHECK_INTERVAL_HOURS: u64 = 24;

// ── Configuration ────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -383,7 +386,62 @@ fn verify_sha256(file: &Path, expected: &Sha256Hash) -> Result<()> {

// ── Attestation verification (best-effort) ───────────────────────────────────

fn verify_attestation(tarball: &Path) -> Result<()> {
/// Build the `gh attestation verify` argument list.
///
/// With `bundle`, `gh` reads the Sigstore bundle from disk and makes no API

@hbrodin hbrodin Jul 30, 2026

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.

This is the fourth copy of the same SSO/403 rationale in the diff — it is also in install.sh:127-130, README.md:146-149, and docs/trust-model.md item 4. It also hangs on a pure argument-list builder that appends two strings: the paragraph describes gh's network behavior, not this function's.

That matters for drift more than for length, because "always attaches its stored token" is a claim about the current gh implementation; when it changes, four independent copies have to be found and corrected. Suggest keeping the substance in docs/trust-model.md as the system of record and reducing this to the first line plus a pointer. The five-line 403 troubleshooting block at README.md:146-149 — attached to the command the section tells readers not to use — is the one that adds least.

/// call, so verification needs no GitHub credential. Without it, `gh` fetches
/// the bundle from the attestations API and always attaches its stored token —
/// which 403s on public data when that token carries no SSO session for the
/// org. `--repo` pins the signer identity in both cases.
fn attestation_verify_args(tarball: &Path, bundle: Option<&Path>) -> Vec<OsString> {
let mut args: Vec<OsString> = vec![
"attestation".into(),
"verify".into(),
tarball.as_os_str().to_owned(),
"--repo".into(),
REPO.into(),
];
if let Some(bundle) = bundle {
args.push("--bundle".into());
args.push(bundle.as_os_str().to_owned());
}
args
}

/// Download the release's provenance bundle, if it published one.
///
/// `None` means verification falls back to the attestations API, or is skipped

@hbrodin hbrodin Jul 30, 2026

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.

Most of this records the path taken during review rather than the function's contract. "A download that blips is the same situation as a release that never published the asset, and install.sh resolves it the same way" is an argument for if let Err over ?, and it asserts a fact about a sibling shell script that nothing checks — if install.sh's fallback changes, this comment silently lies (the cross-implementation invariant already lives in docs/trust-model.md:220-222). The final sentence narrates the function's own next two lines and then defends them with a truism.

The first sentence plus "None means verification falls back to the attestations API, or is skipped outright — never that the update fails" is the whole contract a caller needs.

/// outright — never that the update fails. A download that blips is the same
/// situation as a release that never published the asset, and `install.sh`
/// resolves it the same way. The download is also skipped when
/// `verify_attestation` would not read the result, so an update whose
/// attestation step is a no-op does no pointless work.
fn fetch_attestation_bundle(release: &Release, dir: &Path) -> Option<PathBuf> {

@hbrodin hbrodin Jul 30, 2026

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.

This is the only genuinely new decision logic in the diff — five outcomes (API-base override, gh absent, asset not published, download failed, success) — and it has no coverage of any kind. No test in mod tests references it or verify_attestation; it is unreachable from tests/integration-update.sh, which exports COOP_UPDATE_API_BASE_URL and so short-circuits the function on its first line; and src/update.rs is whole-module excluded in .cargo/mutants.toml exclude_globs, so no mutation sweep reaches it either. The || on the next line could become &&, or either return None could become Some(dest), and nothing anywhere fails.

It is untestable as written — it probes env, probes $PATH, and shells out. The seam is the one this PR already applied to argv construction one function up: split the decision from the IO, e.g. fn bundle_decision(release, api_overridden, gh_present) -> BundleDecision returning Skip / NoAsset / Fetch(&Asset), unit-tested over all three, leaving fetch_attestation_bundle as the thin download wrapper. That composes with the enum suggested on the hint below, and would give bundle_asset_is_found_on_a_release_that_publishes_it something new to assert against — as written it only exercises the pre-existing Release::find_asset with a different string.

if api_base_overridden() || !command_exists("gh") {
return None;
}
let Some(asset) = release.find_asset(BUNDLE_ASSET) else {
tracing::info!(
"Release {} publishes no {BUNDLE_ASSET} — verifying the attestation through the \
GitHub API, which requires a credential authorized for {REPO}.",
release.tag
);
return None;
};
let dest = dir.join(BUNDLE_ASSET);
if let Err(err) = download_asset(&release.tag, BUNDLE_ASSET, &asset.url, &dest) {
tracing::warn!(
"Failed to download {BUNDLE_ASSET} for release {} ({err:#}) — verifying the \
attestation through the GitHub API, which requires a credential authorized for \
{REPO}.",
release.tag
);
return None;
}
Some(dest)
}

fn verify_attestation(tarball: &Path, bundle: Option<&Path>) -> Result<()> {
// Skip when the API base is overridden — the local test fixture serves
// synthetic artifacts that have no provenance in GitHub's attestation
// API. `warn_if_api_base_overridden` has already surfaced this to the
Expand All @@ -397,21 +455,26 @@ fn verify_attestation(tarball: &Path) -> Result<()> {
The download was verified against the published `SHA256SUMS` checksum, which \
is the same assurance level as most `curl | bash` installers. For end-to-end \
Sigstore verification, install `gh` (https://cli.github.com) and re-run, or \
verify manually: `gh attestation verify <tarball> --repo {}`.",
REPO
verify manually: `gh attestation verify <tarball> --repo {REPO} --bundle \
{BUNDLE_ASSET}` against the {BUNDLE_ASSET} asset from the same release."
);
return Ok(());
}
Cmd::new("gh")
.arg("attestation")
.arg("verify")
.arg(tarball)
.arg("--repo")
.arg(REPO)
.args(attestation_verify_args(tarball, bundle))
.run()
.with_context(|| {
let hint = if bundle.is_some() {

@hbrodin hbrodin Jul 30, 2026

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.

fetch_attestation_bundle collapses four distinct outcomes into Option<PathBuf> — skipped (test mode / no gh), asset not published, download failed, bundle present — and this hint then re-derives a reason from the surviving None, asserting one that can be false.

On the download-failure path (added last round to resolve the ?-aborts-the-update thread) the release does publish attestations.jsonl, yet the error tells the user "the release publishes no attestations.jsonl" — contradicting the tracing::warn!("Failed to download {BUNDLE_ASSET} ...") emitted moments earlier in the same run. The distinction is known at the if let Err(err) above and thrown away before it is needed here.

Returning the decision once as an enum (Offline(PathBuf) / ApiFallback(reason) / Skipped) makes the wrong reason unrepresentable, carries the reason fetch_attestation_bundle already computed for its info!/warn!, and drops the replicated api_base_overridden() || !command_exists("gh") guard that currently duplicates this function's own two skip checks by convention only. It would also make the branch testable — it is pure string composition and untested in both directions today, and it is the only place a user is told a 403 means a missing SSO session rather than a provenance failure.

String::new()
} else {
format!(
" (verified through the GitHub API because the release publishes no \
{BUNDLE_ASSET}; an HTTP 403 here means your GitHub credential has no \
SSO session for the org)"
)
};
format!(
"Attestation verification failed for {} — refusing to install",
"Attestation verification failed for {} — refusing to install{hint}",
tarball.display()
)
})
Expand Down Expand Up @@ -576,7 +639,8 @@ fn perform_update(release: &Release, triple: &str) -> Result<()> {
.with_context(|| format!("{tarball_name} not listed in SHA256SUMS"))?;
verify_sha256(&tarball_path, &expected)?;

verify_attestation(&tarball_path)?;
let bundle_path = fetch_attestation_bundle(release, tmp.path());
verify_attestation(&tarball_path, bundle_path.as_deref())?;

// `--no-same-owner --no-same-permissions` ignore embedded uid/mode metadata.
// `-C <tempdir>` plus modern tar's default refusal of `..`-segmented and absolute
Expand Down Expand Up @@ -924,6 +988,84 @@ mod tests {
verify_sha256(&path, &wrong).unwrap_err();
}

#[test]
fn attestation_verify_args_pin_repo_and_omit_bundle_when_absent() {
let args = attestation_verify_args(Path::new("/tmp/coop.tar.gz"), None);
assert_eq!(
args,
["attestation", "verify", "/tmp/coop.tar.gz", "--repo", REPO]
);
}

#[test]
fn attestation_verify_args_append_bundle_when_present() {
let args = attestation_verify_args(
Path::new("/tmp/coop.tar.gz"),
Some(Path::new("/tmp/attestations.jsonl")),
);
assert_eq!(
args,
[
"attestation",
"verify",
"/tmp/coop.tar.gz",
"--repo",
REPO,
"--bundle",
"/tmp/attestations.jsonl",
]
);
}

#[test]
fn bundle_asset_is_found_on_a_release_that_publishes_it() {
let release: Release = serde_json::from_str(&format!(
r#"{{"tag_name":"v9.9.9","assets":[
{{"name":"SHA256SUMS","browser_download_url":"https://example.com/S"}},
{{"name":"{BUNDLE_ASSET}","browser_download_url":"https://example.com/B"}}
]}}"#
))
.unwrap();
assert_eq!(
release.find_asset(BUNDLE_ASSET).map(|a| a.url.as_str()),
Some("https://example.com/B")
);

// A release predating the asset must fall back, not match something else.
let old: Release = serde_json::from_str(
r#"{"tag_name":"v0.5.4","assets":[
{"name":"SHA256SUMS","browser_download_url":"https://example.com/S"}
]}"#,
)
.unwrap();
assert!(old.find_asset(BUNDLE_ASSET).is_none());
}

/// The asset name is agreed across three files with no compiler link
/// between them. A rename in one silently degrades `coop update` and
/// `install.sh` back to the credential-requiring API path.
///
/// `include_str!` is the tripwire, so moving either file breaks this test
/// as a compile error rather than a named assertion failure.
#[test]
fn bundle_asset_name_matches_release_workflow_and_installer() {
let workflow = include_str!("../.github/workflows/release.yml");
let installer = include_str!("../install.sh");
// The workflow names the asset twice: the `jq` output redirect that
// creates it, and the `gh release create` that publishes it. Only the
// latter makes it reachable by a client, so assert on that line.
assert!(
workflow
.lines()
.any(|l| l.contains("gh release create") && l.contains(BUNDLE_ASSET)),
"release.yml no longer publishes {BUNDLE_ASSET} as a release asset"
);
assert!(
installer.contains(BUNDLE_ASSET),

@hbrodin hbrodin Jul 30, 2026

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.

This assertion cannot detect what its message claims. attestations.jsonl appears as a literal in install.sh exactly once — line 13, BUNDLE="attestations.jsonl" — and every use site goes through ${BUNDLE}. So installer.contains(BUNDLE_ASSET) is satisfied by the variable declaration alone: delete the download_asset "$BUNDLE" "${TMPDIR}/${BUNDLE}" probe and the whole --bundle branch, leave the now-dead assignment behind, and this still passes while asserting "install.sh no longer downloads attestations.jsonl". It would also pass with the name mentioned only in the no-gh help text.

The workflow half was tightened in the previous round to require gh release create on the same line; the installer half needs the equivalent — a line containing both download_asset and BUNDLE, or both gh attestation verify and --bundle — so the tripwire keys on the behavior rather than on a string that survives the behavior's removal.

"install.sh no longer downloads {BUNDLE_ASSET}"
);
}

#[test]
fn parse_sha256sums_returns_first_of_duplicates() {
let content = concat!(
Expand Down