feat(evidence): aicr evidence verify (signed OCI bundles)#890
Merged
mchmarny merged 9 commits intoMay 14, 2026
Conversation
…verification Second slice of the evidence verifier. Layers three input forms and the signature step on top of the directory-input verifier that shipped in NVIDIA#879. Input forms (InputFormPointer, InputFormOCI added): - Pointer file (recipes/evidence/<recipe>.yaml) — declares the OCI ref, expected digest, predicateType, signer claims, and Rekor index. The verifier loads it, pulls the artifact at the declared digest, and verifies the pulled bundle's content matches. - OCI ref (registry/repo@sha256:...) — direct OCI pull when callers don't have a pointer file. - Directory — unchanged from the first slice. Signature verification (new step 2): - For directory input, reads attestation.intoto.jsonl from the bundle. - For pointer/OCI input, discovers the Sigstore Bundle as a sibling Referrer artifact on the registry (mediaType application/vnd.dev.sigstore.bundle.v0.3+json) and verifies it without writing the signature to disk — matches cosign's mental model. One fewer round trip, no stray file in the materialized bundle. - Uses sigstore-go for DSSE envelope verification, Fulcio cert chain validation, and Rekor inclusion proof. Extracts the signer's SAN identity and OIDC issuer; surfaces them in the report header. - Unsigned bundles record signature-verify as "skipped" rather than failing — the manifest-hash chain still binds the bundle, the rendered report just marks the signer as unsigned. - Bundle size capped via defaults.MaxSigstoreBundleSize before parse; hostile inputs can't OOM the verifier. Report changes: - Header now surfaces signer identity / issuer / Rekor index when the signature verified, and the bundle digest when known (pointer or OCI input). - Predicate-parse step distinguishes "from verified DSSE payload" (signed) vs "from unsigned statement.intoto.json" (skipped sig). Schema: - Pointer 1.0.x — exactly one entry in attestations; multi-instance (schema 2.0) is reserved. predicateType must be PredicateTypeV1. Signer requires both identity and issuer when present. - Pointer files are capped at 1 MiB (matches MaxRecipePOSTBytes). Docs + demos updated for the new flag, the five-step pipeline, and end-to-end pointer/OCI flows.
…d OCI refs Address three findings from the verify test sweep: 1. Pointer signer claims are now load-bearing (was: decorative). When a pointer file declares signer.identity / signer.issuer / signer.rekorLogIndex, the verifier cross-checks the actual cert SAN, OIDC issuer, and Rekor log index after signature verification. A pointer that names a different identity than the bundle's cert fails at signature-verify with a "pointer claimed X, cert says Y" error. A pointer that claims a signer for an unsigned bundle also fails — the operator's claim and the bundle's reality must agree. 2. Refuse tag-only OCI references by default. Tags are not content-addressable; a registry rewrite (or a push from anyone with write access) could substitute the artifact at the same tag and the verifier would happily verify whatever now sits there. The new --allow-unpinned-tag flag opts back in for one-off debugging. Pointer-driven pulls remain accepted with a tag-only OCI URI as long as pointer.bundle.digest is set — the pointer's digest claim is the pin (already cross-checked against the actual pulled digest). 3. Markdown report now enumerates sub-rows of failed steps. The bundle-tamper test surfaced "manifest inventory check failed for 1 file(s)" with no indication of which file. Add a "Failed check details" section that breaks out each failed step's sub-rows as bullets, naming the offending path / dimension / constraint. Tests cover the new failure modes: pointer-vs-cert identity mismatch, issuer mismatch, Rekor index mismatch, claimed-signer-on-unsigned- bundle, and the tag-only refusal with and without the opt-in flag.
Three cleanups exposed by the verify test sweep: - Remove --no-rekor. The flag only ever works for bundles carrying a separate TSA-signed timestamp, but the emitter signs keyless via Fulcio + Rekor where the Rekor entry IS the timestamp. With --no-rekor on a keyless bundle the verifier had zero timestamp sources and sigstore-go failed the "threshold not met" check — the flag's documented behavior could never succeed against bundles this project produces. Re-introduce if/when the emitter dual-anchors with a TSA. - Distinguish "signature verification failed" from "unsigned bundle" in the report header. The previous code printed "_unsigned bundle_" whenever r.Signer was nil, which was misleading on bundles where the signature step actually ran and failed. Now reads "_signature verification failed (see Verification steps)_" when the signature step status is failed, reserving "_unsigned bundle_" for the no-signature-attached case. - Sanitize sigstore-go's "%!w(<nil>)" format-string leak in sigstore verification error messages. The library's threshold-not-met paths wrap an empty errors.Join with %w, producing user-visible "...: 0 < 1; error: %!w(<nil>)" surface. Strip the artifact so the error reads cleanly. Safe to remove once fixed upstream.
Operators reading the demo were confused why `--push ttl.sh/aicr-demo` worked when ttl.sh requires a tag. Explain that the emitter substitutes `:v1` as a placeholder when the caller omits one — the OCI digest is the canonical address, and the pointer file the emitter writes records both the (tagged) ref and the digest.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds multi-input evidence verification: auto-detects pointer/OCI/directory inputs, materializes bundles via ORAS and OCI Referrers, enforces digest-pinning rules, verifies Sigstore DSSE signatures and extracts signer claims, cross-checks pointer-supplied signer assertions, resolves verified or unsigned predicates, performs manifest-hash inventory checks, and renders structured Markdown/JSON. CLI gains flags for signer pinning, OCI bundle override, and registry transport/security options. Comprehensive tests and an end-to-end demo doc were added. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/evidence/verifier/fetch.go (1)
372-381:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff
newAuthClientmust configure TLS certificate verification, not return nil.The
insecureTLSflag is accepted but ignored. Returningnilcauses two problems:
- Loses Docker credential support (which
auth.DefaultClientwould provide)- Skips TLS configuration entirely—ORAS-go v2 requires custom
http.Clientwithtls.Config{InsecureSkipVerify: true}The correct pattern exists in
pkg/oci/push.go:createAuthClientForHost()(lines 714–726): create a transport, conditionally setInsecureSkipVerify, wrap in an authenticated HTTP client. Apply the same logic here to support both credentials and insecure TLS.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/evidence/verifier/fetch.go` around lines 372 - 381, newAuthClient currently returns nil for insecureTLS which drops docker credential support and skips TLS config; instead mirror the pattern in createAuthClientForHost: build an http.Transport (set TLSClientConfig.InsecureSkipVerify = true when insecureTLS is true), wrap it in an http.Client, and return an auth client that uses that http.Client so credentials from auth.DefaultClient are preserved while allowing opt-in insecure TLS; update newAuthClient to construct and return that configured auth client (referencing newAuthClient and createAuthClientForHost to locate the implementation).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@demos/evidence.md`:
- Around line 116-122: The Exit codes section is inconsistent with the verifier
contract: it states VerifyResult.Exit uses 0/1/2 where 1 means "bundle valid but
checks failed", yet also says the OS exit "collapses to 2" for that case; update
the documentation to pick one definitive behavior and clearly distinguish
current behavior from intended future behavior. Edit the text around
VerifyResult.Exit, the bullet descriptions for `0`, `1`, and `2`, and the
sentence about OS exit collapse so they consistently state whether a JSON
consumer sees Exit=1 while the CLI returns OS exit 1 or 2; mention both the
current behavior (exact mapping used by VerifyResult.Exit and the CLI/OS exit)
and, if different, a clearly labeled "future intent" note describing any planned
collapse to a single OS code. Ensure references to VerifyResult.Exit and the
OS/CLI exit behavior are unambiguous and use neutral, factual language.
- Around line 24-26: Replace the overbroad phrase "any OCI-1.1 registry works"
with a precise list of required registry capabilities (e.g., support for OCI
referrers API or equivalent referrer/discovery endpoints, Sigstore bundle
discovery/referrer listings, ability to store referrers/attachments and write
access for the producer) and add a short note about failure modes (what happens
if a registry lacks referrers or bundle discovery — e.g., signatures/referrers
cannot be discovered or stored, verification will fail and tooling will error
with X/Y behavior). Update the sentence near "any OCI-1.1 registry works" and
the signing/OIDC paragraph so it documents current behavior and required
features rather than promising universal compatibility.
In `@docs/user/cli-reference.md`:
- Line 1900: Add missing blank lines around the Markdown fenced code blocks and
the table to satisfy markdownlint: insert a blank line before and after each
fenced code block marker (e.g., the ```shell fence shown) and ensure there is a
blank line above and below the table block (the contiguous lines using '|' for
columns) so each block is separated by at least one empty line for proper
formatting.
In `@pkg/evidence/verifier/fetch.go`:
- Around line 225-231: The Reference string in the MaterializedBundle is built
using registry + "/" + repo + ":" + refTarget which yields invalid OCI refs for
digest-pinned targets (e.g., "registry/repo:sha256:..."); update the
construction to detect digest-style refTarget (e.g., startsWith "sha256:" or
matches the "<algo>:<hex>" pattern) and use "@" instead of ":" when assembling
Reference (use registry + "/" + repo + "@" + refTarget) while keeping the
current colon usage for tag refs; adjust the code that creates the
MaterializedBundle (Reference field) to branch on refTarget format accordingly.
In `@pkg/evidence/verifier/input.go`:
- Around line 65-66: The OCI-shape detection incorrectly treats tokens "." and
".." as registry-like because they contain dots; update the conditional that
checks the variable first (the token parsed in the OCI detection logic in
pkg/evidence/verifier/input.go) to also reject first == "." and first == ".." so
inputs starting with "./" or "../" are not classified as OCI references. Modify
the if that currently checks !strings.ContainsAny(first, ".:") && first !=
"localhost" to additionally ensure first is not "." or ".." (i.e., treat those
as non-registry tokens).
---
Outside diff comments:
In `@pkg/evidence/verifier/fetch.go`:
- Around line 372-381: newAuthClient currently returns nil for insecureTLS which
drops docker credential support and skips TLS config; instead mirror the pattern
in createAuthClientForHost: build an http.Transport (set
TLSClientConfig.InsecureSkipVerify = true when insecureTLS is true), wrap it in
an http.Client, and return an auth client that uses that http.Client so
credentials from auth.DefaultClient are preserved while allowing opt-in insecure
TLS; update newAuthClient to construct and return that configured auth client
(referencing newAuthClient and createAuthClientForHost to locate the
implementation).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e4500344-e1e5-49e3-8914-7615b90efb7e
📒 Files selected for processing (20)
demos/README.mddemos/evidence.mddocs/user/cli-reference.mdpkg/cli/evidence_verify.gopkg/cli/evidence_verify_test.gopkg/evidence/verifier/crosscheck_test.gopkg/evidence/verifier/doc.gopkg/evidence/verifier/fetch.gopkg/evidence/verifier/fetch_test.gopkg/evidence/verifier/input.gopkg/evidence/verifier/input_test.gopkg/evidence/verifier/pointer.gopkg/evidence/verifier/pointer_test.gopkg/evidence/verifier/referrer_test.gopkg/evidence/verifier/report.gopkg/evidence/verifier/signature.gopkg/evidence/verifier/signature_test.gopkg/evidence/verifier/types.gopkg/evidence/verifier/verify.gopkg/evidence/verifier/verify_test.go
mchmarny
reviewed
May 14, 2026
Member
mchmarny
left a comment
mchmarny
left a comment
There was a problem hiding this comment.
Nice slice — five-step pipeline reads cleanly, the trust-chain story in the package doc is accurate, the test coverage on the deterministic helpers (DSSE parse, identity matcher, cross-check table, referrer fetcher via in-memory fake) is strong, and gating --allow-unpinned-tag behind explicit opt-in is the right default. The pointer-signer cross-check that catches a malicious pointer + valid bundle pairing is a particularly nice touch.
A few things to look at before merge:
- Medium:
discoverAndWriteReferrererrors collapse toslog.Debugat the call site, which means a registry that returns a malformed/oversized referrer manifest looks indistinguishable from "unsigned bundle" — a silent downgrade path. Worth distinguishingErrCodeNotFoundfrom real failures. - Low/security: default identity matcher accepts any Fulcio cert (
.+regex) when no--expected-*flags are passed; aslog.Warnwould surface the missing pin without changing default behavior. - Low:
--allow-unpinned-tagis implemented but not in thedocs/user/cli-reference.mdflag table. - Low:
TestMaterializeBundle_TagOnlyWithAllowFlagreaches the realghcr.ioand can hang up to 2 minutes on flaky networks. - Nit: comment in
materializeFromPointerand the PR description say "non-empty bundle.digest" but the validator requires the strictersha256:prefix.
CI: the Build job (specifically the "Build site (PR)" step) is failing on this head — worth a quick look before requesting re-review. The PR is also behind main and will need a rebase.
Nothing blocks merge after the silent-downgrade and pin-warning items are addressed.
njhensley
commented
May 14, 2026
Member
Author
njhensley
left a comment
njhensley
left a comment
There was a problem hiding this comment.
Thanks for the review. All six inline findings addressed; replies on each thread.
Build site (PR) CI failure — diagnosed and fixed in the same pass: the cli-reference.md section had a relative link ../../demos/evidence.md that VitePress flagged as a dead link (the site only mounts docs/, not demos/). Switched to an absolute GitHub URL matching the convention in docs/README.md, docs/design/00*-*.md, and docs/integrator/recipe-development.md. Local site build now passes (cd site && npm run build → build complete).
Rebase onto main — pending. Will rebase after this fixup commit lands so the rebase only deals with my freshly-committed state. The intervening merges (#886, #888, #889) don't touch pkg/evidence/verifier so it should be clean.
… detection Four review findings from CodeRabbit on PR NVIDIA#890: - materializeOCIRefRequireDigest was joining (registry, repo, target) with ":" unconditionally, producing invalid refs like "ghcr.io/owner/repo:sha256:..." when the target was a digest. The OCI spec uses "@" for digest separators and ":" for tags. Extract formatOCIReference so the rule lives in one place; add a table test covering tag, digest, and localhost-with-port cases. - newAuthClient returned nil for plainHTTP/insecureTLS, throwing away any ambient docker credentials the operator might have configured. Build a proper *auth.Client that loads the docker credential store (best-effort: anonymous fallback when missing) and honors the TLS preferences explicitly. Clone any existing TLS config when flipping InsecureSkipVerify so hardening defaults from defaults.NewHTTPTransport (MinVersion, ciphers) survive. Emits a Warn when TLS verification is disabled — a deliberate operator opt-in should be visible in logs. - DetectInputForm/looksLikeOCIRef classified "./out/bundle" and "../bundle" as OCI references because the leading "." satisfied the "contains '.' or ':'" registry-host heuristic. Reject relative- path tokens explicitly before the registry check; add test cases for both forms. - demos/evidence.md: clarify the registry-referrer requirement (the registry must support OCI 1.1 Referrers API or fallback tag-schema referrers; without either, the Sigstore Bundle can't be attached and the verifier will record signature-verify as skipped even for push-time-signed bundles). Also expand the exit-code section: the CLI collapses library exit codes 1 and 2 to OS exit 2 today; JSON consumers reading the .exit field still see the three-way split.
…first match Three correctness fixes plus an operator-visible warning: - discoverAndWriteReferrer used `return nil` to break out of the Referrers pagination callback after writing the first matching Sigstore Bundle. That only stops the *current* page; subsequent pages would re-enter the loop and overwrite attestation.intoto.jsonl with a later match. Introduce an unexported errReferrerFound sentinel as the stop signal and translate it back to a clean nil at the top. - materializeOCIRefRequireDigest treated *any* referrer-discovery error as "unsigned bundle" and let signature-verify record Skipped. A registry that returns a malformed Referrers response (or a MITM rewriting one) could silently downgrade a signed bundle to "unsigned." Discriminate: only ErrCodeNotFound is the unsigned-bundle path; everything else (malformed manifest, oversized layer, bad JSON) propagates as ErrCodeInvalidRequest so the verifier fails closed. - VerifySignature now slog.Warn-s when the bundle verifies but no signer was pinned via --expected-issuer or --expected-identity- regexp. Without a pin, ANY Fulcio-issued cert from ANY OIDC provider passes the policy — the signature is cryptographically valid but the verifier hasn't said anything about *who* signed. Surfaces the default-trust footgun in the logs. - TestMaterializeBundle_TagOnlyWithAllowFlag previously relied on "no registry available in the test" to fail the OCI pull. That's not portable — DNS for ghcr.io resolves and oras-go could pay the full 2-minute pull timeout. Point the test at 127.0.0.1:1 (no listener) with a 250ms context so connection fails fast and the test stays local-only. - cli-reference.md: document --allow-unpinned-tag and replace the relative demos/evidence.md link with a global GitHub URL (matches the project's Review Output Links convention).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/evidence/verifier/fetch.go`:
- Around line 420-432: When creating a fresh TLS config because
transport.TLSClientConfig is nil, explicitly set a secure MinVersion on the new
config (e.g. tls.VersionTLS12 or tls.VersionTLS13) so hardening defaults from
defaults.NewHTTPTransport are preserved; update the branch that constructs cfg =
&tls.Config{} to assign cfg.MinVersion = tls.VersionTLS12 (or TLS13) before
setting cfg.InsecureSkipVerify and assigning it back to
transport.TLSClientConfig, referencing the transport.TLSClientConfig handling in
this block and the defaults.NewHTTPTransport comment for context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2d4e07ee-9348-4be6-aace-c5562334c879
📒 Files selected for processing (8)
demos/evidence.mddocs/user/cli-reference.mdpkg/evidence/verifier/crosscheck_test.gopkg/evidence/verifier/fetch.gopkg/evidence/verifier/fetch_test.gopkg/evidence/verifier/input.gopkg/evidence/verifier/input_test.gopkg/evidence/verifier/signature.go
…egistry-insecure-tls newAuthClient clones any existing transport TLSClientConfig before flipping InsecureSkipVerify so hardening defaults survive. But defaults.NewHTTPTransport currently leaves TLSClientConfig nil, so we fall through to the synthesized-config branch and would inherit Go's historical client default for MinVersion. Set tls.VersionTLS12 explicitly — it's the project floor everywhere else.
mchmarny
approved these changes
May 14, 2026
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
Second slice of
aicr evidence verify(closes #753): adds Sigstore signature verification, OCI pull with Referrers-API discovery of the attached Sigstore Bundle, pointer-file input form, and the end-to-end producer→consumer demo. The trust handoff is complete after this PR.Motivation / Context
PR 1 (#879) shipped offline directory verification — the receiving end of the bundle integrity chain. The chain was self-consistency only because
statement.intoto.jsonis never signed; the actual signature lives inattestation.intoto.jsonl. This PR closes that gap by verifying the Sigstore-signed Statement and switching predicate extraction to the cryptographically anchored payload, plus the two transport forms most reviewers and CI workflows will actually use (OCI ref + pointer file).Constraint replay (ADR-007's step 5) is a refinement that re-derives constraint outcomes against the bundled snapshot; it's tracked as a follow-up enhancement.
Fixes: #753
Related: ADR-007 PR-B; depends on emitter from #873
Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)cmd/aicrd,pkg/api,pkg/server)pkg/recipe)pkg/bundler,pkg/component/*)pkg/collector,pkg/snapshotter)pkg/validator)pkg/errors,pkg/k8s)docs/,examples/)pkg/evidence/verifier(extends existing package; no API breaks)Implementation Notes
Five-step pipeline (was three in PR 1):
materialize-bundle— directory / pointer / OCI ref dispatch. Pointer and OCI forms pull the artifact via ORAS, then discover the Sigstore Bundle attached as an OCI Referrer (artifact typeapplication/vnd.dev.sigstore.bundle.v0.3+json) and stage it locally asattestation.intoto.jsonl. Referrer is best-effort: an unsigned bundle has no referrer and the signature step records Skipped.signature-verify— sigstore-go verifies the DSSE-wrapped Statement against the Sigstore trusted root (Fulcio cert chain + Rekor entry). Cross-checks the signed subject digest against the pulled artifact's OCI manifest digest. Extracts the verified predicate body from inside the DSSE payload.predicate-parse— prefers the verified predicate from step 2; falls back to the bundle's unsignedstatement.intoto.jsonwhen no signature is attached (PR 1's path).manifest-hash-check— unchanged from PR 1, butpredicate.Manifest.Digestis now cryptographically anchored when the bundle is signed.render-summary.Trust chain when signed:
Tampering anywhere below the signature breaks the chain. The OCI input form locks the signed subject digest to the pulled artifact's OCI manifest digest — a substituted bundle with a stale signature fails.
Pointer signer cross-check. Pointer files can declare
signer.identity/signer.issuer/signer.rekorLogIndex. The orchestrator extracts the actual signer claims from the verified Sigstore Bundle and compares: a pointer that claimsidentity: aliceagainst a bundle signed bymalloryfails loudly with a field-by-field diff. Catches the malicious-pointer scenario where the pointer points at the right OCI bundle but lies about who signed it.Tag-pin enforcement. Tag-only OCI references are refused by default — tags are registry-rewritable and not content-addressable.
--allow-unpinned-tagopts in for one-off debugging. Pointer-driven flows accept tag-formbundle.ocionly when the pointer also carries a non-emptybundle.digest(that digest is the pin and is cross-checked against the pulled artifact).Sigstore error sanitization.
sigstore-go'spkg/verify/tsa.go:88wraps an emptyerrors.Join(nil...)with%w, producing a%!w(<nil>)Go format artifact in the user-visible error.sanitizeSigstoreErrorstrips it before wrapping. Tracked upstream.Header rendering. The Markdown report's Signer line distinguishes "signature verification failed" from "unsigned bundle" — a signed bundle whose signature didn't verify is meaningfully different from a bundle that carries no signature at all.
Failed-step details. The Markdown report appends a "Failed check details" section after the steps table that enumerates per-file sub-rows for any failed step. Markdown tables can't render nested lists; this section is the bullet form.
--no-rekorremoved. The flag was effectively dead code for the current emitter path (cosign keyless → Rekor only): without Rekor and without a TSA timestamp, sigstore-go's threshold check fails with zero verified timestamps. Will come back when the emitter grows TSA support.What this PR does NOT bind
Worth flagging for reviewers — the signature covers the post-resolution
recipe.yamland the predicate body (via the manifest hash chain). It does NOT bind:recipes/components/<name>/values.yamlormanifests/*.yamlcontent. ThecomponentRefsentry stores file paths; the bundler reads the bytes at bundle-time. An edit to those files between attest-time and verify-time silently changes deployment without changing the canonical recipe digest. ADR-007's deferredchainManifestis the answer.The package doc records the depth honestly.
Testing
Validated against a real GB200/EKS bundle (6 nodes, 14 validators across deployment / performance / conformance) signed via cosign keyless + Sigstore + Rekor, published as an OCI Referrer.
Happy-path inputs (all verify clean, exit 0):
aicr evidence verify ./bundle/(unsigned local copy — signature step skips cleanly)…@sha256:f90b…(pulls + discovers Sigstore Bundle Referrer + verifies)Negative-path sweep — six probes against the same bundle:
recipe.yamlsigner.identity--allow-unpinned-tagopt-in-t jsonoutputTwo real security gaps closed (pointer-signer enforcement in #3, digest-pin enforcement in #4) plus one UX gap (#1's "Failed check details" rendering). Three secondary issues surfaced while probing — a
--no-rekorflag that could never succeed against keyless-signed bundles, a_unsigned bundle_header that fired on signature-failure too, and a Go format-string artifact leaking through sigstore-go errors — all addressed in the commit history.Local checks
All green locally:
go test ./pkg/evidence/verifier/... ./pkg/cli/... golangci-lint -c .golangci.yaml run ./pkg/evidence/verifier/... ./pkg/cli/... ./tools/check-docs-mdx ./tools/check-docs-sidebarCoverage highlights:
pointer_test.go— load + validate happy path; rejects unsupported schema, missing recipe, no/multiple attestations, wrong predicate type, bad digest format, oversized file.signature_test.go— sigstore-error sanitizer table; cert-chain-error matcher; identity matcher with and without pinning; absent-file returnsErrUnsignedBundle; nil-bundle errors; pure-function Statement parser with malformed inputs.referrer_test.go— single-layer happy path against an in-memory fake fetcher; rejects multi-layer manifest, oversized manifest, oversized layer (claimed size), malformed JSON.fetch_test.go— parses tag / digest / oci-scheme / invalid forms; directory-input rejects non-bundles.crosscheck_test.go— pointer signer cross-check 10-case table; tag-pin enforcement (tag rejected without flag, accepted with flag);isDigestPinnedhelper.verify_test.go— directory happy path asserts the 5-step transcript; manifest tamper / stray file / unsupported schema / traversal entry / ctx cancellation; predicate-parse failure records under its own step; Markdown header distinguishes passed/skipped/failed signature; Failed-check-details section appears only when needed.Sigstore-actual verification (TLS handshake to Fulcio + Rekor) depends on the trusted root which depends on network access; the existing
pkg/bundler/verifierintegration tests cover the sigstore-go primitives end-to-end. The verifier-side tests cover everything that surrounds them.Risk Assessment
Adds cryptographic verification paths, OCI network I/O, and a new pointer schema dependency. Tag-pin and signer cross-check fail closed by default. Directory input from PR 1 is unchanged. The new transport forms are gated behind explicit user invocation; users who only run
aicr evidence verify ./out/summary-bundlesee no behavior change.Rollout notes: No migration. The directory verify command from PR 1 continues to work. New flags (
--bundle,--expected-issuer,--expected-identity-regexp,--registry-plain-http,--registry-insecure-tls,--allow-unpinned-tag) are additive and default-off.Checklist
make testwith-race)make lint)git commit -S) — GPG signing info