fix(backend): use full token chain for all sigstore attestation calls#9307
Merged
fix(backend): use full token chain for all sigstore attestation calls#9307
Conversation
…ken chain Lock-time provenance verification hit GitHub's 60/hour unauthenticated rate limit because three sigstore call sites (aqua, python, ruby) passed env::GITHUB_TOKEN — env vars only — while the github backend used the full resolution chain via github::resolve_token_for_api_url. Users with gh auth or github_tokens.toml but no GITHUB_TOKEN env var saw lock-time requests go out unauthenticated. Concentrate the sigstore_verification surface in a new wrapper at src/github/sigstore.rs. The wrapper resolves tokens internally and exposes token-free signatures, making the asymmetry structurally impossible to reintroduce. All call sites (github, aqua, python, ruby) migrate to the wrapper, and env::GITHUB_TOKEN gets a doc comment steering future authors to the wrapper for HTTP or sigstore use. Unit tests prove the default api.github.com URL, an explicit api.github.com URL, and a GitHub Enterprise base URL each resolve the expected token without hitting the network. Does not address the separate root cause where gh CLI stores OAuth tokens in the macOS Keychain (leaving hosts.yml without oauth_token); that is a stacked follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
… wrapper pub(crate) (auto-fix from review) Three auto-fix findings from review: F-2 (P2): test lock was duplicated in github.rs and github/sigstore.rs tests, so parallel test runs could race on the same env vars. Hoist the lock to a pub(crate) static under #[cfg(test)] so both modules share it. F-3 (P2): sigstore test teardown was manual — a panicking assertion skipped restore_token_env_vars and leaked state into other tests. Replace with a RAII TokenEnvGuard whose Drop restores the snapshot unconditionally. F-5 (P3): pub mod sigstore; made an internal wrapper externally visible for no reason. Tighten to pub(crate) mod sigstore;. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…var token source (review triage) Addresses two human-triaged review findings. F-1 (P1) — Tests only covered env-var token paths; R3 asked for coverage of non-env-var sources too. Add a `#[cfg(test)]` override hook on `resolve_token` that lets the wrapper's test suite seed the `github_tokens.toml` path, and a new test `test_resolve_token_wrapper_uses_github_tokens_toml_source` that asserts the wrapper walks past env vars and reaches the tokens-file source. Also add an `#[ignore]`d regression marker `test_wrapper_resolves_gh_cli_token_from_macos_keychain_after_fix` that documents the gh-CLI-Keychain gap on macOS (the stacked follow-up's scope). Running `cargo test -- --ignored` surfaces it; removing `#[ignore]` and filling in the body becomes part of that follow-up PR. F-4 (P2) — `detect_attestations` had collapsed two distinct error paths (`GitHubSource::with_base_url` failure vs. `fetch_attestations` failure) into a single `AttestationError`, losing the "Failed to create GitHub attestation source" warning. Introduce a local `DetectError` enum with `SourceCreation` / `Fetch` variants so the caller at `src/backend/github.rs:477` can restore both warning messages verbatim. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Contributor
Greptile SummaryThis PR fixes a bug where three sigstore attestation call sites ( Confidence Score: 5/5Safe to merge — the fix is correct, well-tested, and the wrapper pattern makes the bug structurally impossible to reintroduce. All sigstore call sites consistently use the new wrapper which resolves tokens via the full chain. No P0 or P1 issues found. The only minor observation is that the PR description references an #[ignore]d test (test_wrapper_resolves_gh_cli_token_from_macos_keychain_after_fix) that is not present in the reviewed file, but this is a documentation discrepancy, not a code defect. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[aqua.rs / python.rs / ruby.rs / github.rs] -->|calls| B[github::sigstore wrapper]
B --> C[resolve_token_for_wrapper]
C --> D[github::resolve_token_for_api_url]
D --> E[1. env vars: MISE_GITHUB_TOKEN / GITHUB_TOKEN]
D --> F[2. credential_command]
D --> G[3. github_tokens.toml]
D --> H[4. gh CLI hosts.yml]
D --> I[5. git credential fill]
E & F & G & H & I --> J[token resolved]
J --> K{api_url Some or None}
K -->|Some base_url| L[verify_github_attestation_with_base_url]
K -->|None| M[verify_github_attestation]
L & M --> N[sigstore_verification crate]
style B fill:#d4edda,stroke:#28a745
style D fill:#cce5ff,stroke:#004085
Reviews (2): Last reviewed commit: "chore(github): drop macOS Keychain regre..." | Re-trigger Greptile |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized sigstore wrapper module to handle GitHub attestation verification, ensuring consistent token resolution across all call sites and resolving a rate-limiting issue. The aqua, python, and ruby backends have been refactored to use this wrapper, and test infrastructure was updated with a shared mutex to prevent environment variable race conditions. Feedback was provided to implement the source() method for the DetectError type to enhance error diagnostics.
The `cargo clippy --all-features --all-targets -- -D warnings` CI job was
failing on `src/main.rs:255-258`:
std::io::Error::new(std::io::ErrorKind::Other, "user cancelled")
Clippy 1.94+ flags this as `clippy::io_other_error`, preferring
`std::io::Error::other("user cancelled")`. The local clippy invocation
during PR jdx#9307's review phase used `--bin mise`, which does not compile
the test target and therefore did not surface this. The failure is not
introduced by this branch (the offending code came in via jdx#9294), but
the branch can't merge without fixing it.
The sibling `Error::new(ErrorKind::Interrupted, ...)` on the line above
is intentionally left alone — clippy does not flag it (and should not;
`Error::other` is specific to `ErrorKind::Other`).
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Addresses jdx#9307 (comment): `DetectError` wraps `AttestationError` but the `std::error::Error` impl was empty, so callers that walk the error chain (`e.source()`) couldn't reach the inner error. Return the wrapped variant via `source()` per the standard idiom for error-wrapping types. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The `#[ignore]`d test `test_wrapper_resolves_gh_cli_token_from_macos_keychain_after_fix` was added during PR jdx#9307 refinement as a placeholder for a follow-up branch that would add `gh auth token` / macOS Keychain support. The project decision is to not pursue that follow-up. Users whose gh CLI tokens live only in the macOS Keychain have two working paths today: [settings.github] credential_command = 'gh auth token --hostname "$1"' or: [settings.github] use_git_credentials = true Drop the placeholder test and its explanatory doc block; a regression marker for work that won't happen is noise. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
jdx
added a commit
that referenced
this pull request
Apr 23, 2026
Replace the external `sigstore-verification` dependency with a local `mise-sigstore` adapter built on `sigstore-verify` 0.6.5 from sigstore-rust. The wrapper at `src/github/sigstore.rs` and the `vfox` crate are rerouted to the new adapter; mise's call sites are unchanged because the wrapper from PR #9307 already abstracts the underlying verifier. Why a local adapter and not just bumping `sigstore-verification`: the upstream `sigstore-verify` 0.6.x exposes a different (and richer) API surface. The adapter keeps mise's existing helper-style API (`verify_github_attestation`, `verify_cosign_signature`, `verify_slsa_provenance`, `detect_attestations`) so call sites do not need to change. Why we ship a custom TUF root and bypass `TrustedRoot::production()`: the `sigstore-trust-root` crate embeds TUF root v1 (from 2021), which fails verification under `tough` because the `expires` field uses a timezone-offset format that `chrono` normalizes to UTC `Z` on re-serialization, breaking the canonical-JSON byte sequence that the original signature was computed over (0 of 5 signatures verify, threshold is 3). We embed root v12 (the same workaround the original `sigstore` 0.13 crate uses) and pass it via `TufConfig::custom`, so `tough` walks v12 -> v13 -> v14 successfully. The TUF root file is added to `.prettierignore` because reformatting would invalidate its signatures. Verified end-to-end: - cargo check, cargo clippy --all-targets, cargo fmt --all -- --check, taplo fmt - cargo test --bin mise (747 tests pass) - cargo test -p mise-sigstore (4 tests pass) - mise run test:e2e -- test_aqua_github_attestations passes against goreleaser 2.14.1 (live download + TUF fetch + bundle verification) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
jdx
added a commit
that referenced
this pull request
Apr 29, 2026
Replace the external `sigstore-verification` dependency with a local `mise-sigstore` adapter built on `sigstore-verify` 0.6.5 from sigstore-rust. The wrapper at `src/github/sigstore.rs` and the `vfox` crate are rerouted to the new adapter; mise's call sites are unchanged because the wrapper from PR #9307 already abstracts the underlying verifier. Why a local adapter and not just bumping `sigstore-verification`: the upstream `sigstore-verify` 0.6.x exposes a different (and richer) API surface. The adapter keeps mise's existing helper-style API (`verify_github_attestation`, `verify_cosign_signature`, `verify_slsa_provenance`, `detect_attestations`) so call sites do not need to change. Why we ship a custom TUF root and bypass `TrustedRoot::production()`: the `sigstore-trust-root` crate embeds TUF root v1 (from 2021), which fails verification under `tough` because the `expires` field uses a timezone-offset format that `chrono` normalizes to UTC `Z` on re-serialization, breaking the canonical-JSON byte sequence that the original signature was computed over (0 of 5 signatures verify, threshold is 3). We embed root v12 (the same workaround the original `sigstore` 0.13 crate uses) and pass it via `TufConfig::custom`, so `tough` walks v12 -> v13 -> v14 successfully. The TUF root file is added to `.prettierignore` because reformatting would invalidate its signatures. Verified end-to-end: - cargo check, cargo clippy --all-targets, cargo fmt --all -- --check, taplo fmt - cargo test --bin mise (747 tests pass) - cargo test -p mise-sigstore (4 tests pass) - mise run test:e2e -- test_aqua_github_attestations passes against goreleaser 2.14.1 (live download + TUF fetch + bundle verification) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
jdx
added a commit
that referenced
this pull request
May 1, 2026
Replace the external `sigstore-verification` dependency with a local `mise-sigstore` adapter built on `sigstore-verify` 0.6.5 from sigstore-rust. The wrapper at `src/github/sigstore.rs` and the `vfox` crate are rerouted to the new adapter; mise's call sites are unchanged because the wrapper from PR #9307 already abstracts the underlying verifier. Why a local adapter and not just bumping `sigstore-verification`: the upstream `sigstore-verify` 0.6.x exposes a different (and richer) API surface. The adapter keeps mise's existing helper-style API (`verify_github_attestation`, `verify_cosign_signature`, `verify_slsa_provenance`, `detect_attestations`) so call sites do not need to change. Why we ship a custom TUF root and bypass `TrustedRoot::production()`: the `sigstore-trust-root` crate embeds TUF root v1 (from 2021), which fails verification under `tough` because the `expires` field uses a timezone-offset format that `chrono` normalizes to UTC `Z` on re-serialization, breaking the canonical-JSON byte sequence that the original signature was computed over (0 of 5 signatures verify, threshold is 3). We embed root v12 (the same workaround the original `sigstore` 0.13 crate uses) and pass it via `TufConfig::custom`, so `tough` walks v12 -> v13 -> v14 successfully. The TUF root file is added to `.prettierignore` because reformatting would invalidate its signatures. Verified end-to-end: - cargo check, cargo clippy --all-targets, cargo fmt --all -- --check, taplo fmt - cargo test --bin mise (747 tests pass) - cargo test -p mise-sigstore (4 tests pass) - mise run test:e2e -- test_aqua_github_attestations passes against goreleaser 2.14.1 (live download + TUF fetch + bundle verification) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mise lockruns GitHub attestation verification unauthenticated because three of four sigstore call sites — aqua backend (src/backend/aqua.rs:939), python core plugin (src/plugins/core/python.rs:628), ruby core plugin (src/plugins/core/ruby.rs:805) — bypass mise's full token resolution chain by passingenv::GITHUB_TOKEN.as_deref(), which only reads theMISE_GITHUB_TOKEN/GITHUB_API_TOKEN/GITHUB_TOKENenv vars. The fourth call path (github backend) already uses the full chain viagithub::resolve_token_for_api_url. On a repo with many GitHub-sourced tools, the unauthenticated calls exhaust the 60/hour IP rate limit on the secondmise lock.This PR unifies all four call sites through a new wrapper whose signatures structurally prevent re-introducing the bug.
Approach
Introduce a crate-internal wrapper at
src/github/sigstore.rsthat owns everysigstore_verificationimport (verified byrg -l 'use sigstore_verification\b|sigstore_verification::' src/returning exactly that one file). The wrapper's functions (verify_attestation,detect_attestations,verify_slsa_provenance,verify_cosign_signature,verify_cosign_signature_with_key) resolve the token internally viagithub::resolve_token_for_api_url, usinggithub::API_URLas the default. Callers can't pass a token at all.env::GITHUB_TOKENstays — it's still needed for subprocess env-var plumbing incargo.rsandasdf_plugin.rs— and gains a doc comment steering future authors away from HTTP use.A local
DetectErrorenum in the wrapper preserves the original "source creation" vs "API fetch" warning distinction that pre-wrapper code emitted. ItsError::source()implementation lets callers inspect the wrappedAttestationError.Test plan
src/github/sigstore.rscover env-var, explicit and enterpriseapi_url, and the non-env-vargithub_tokens.tomlsource (via a new#[cfg(test)] test_support::TOKENS_FILE_OVERRIDEhook).is_slsa_format_issuetests still pass after theAttestationErrorsignature switch to the re-export.mise run lintpasses locally (runs the clippy check that was failing in CI).mise run cipasses locally (runs format, build, and the full test suite).mise lockon a repo with many github/aqua-backed tools should authenticate all attestation requests when a token source is configured (env var,github_tokens.toml, orcredential_command).🤖 Generated with Claude Code