feat(evidence): --no-sign push and pending/structured-cause verify#1445
Merged
Conversation
mchmarny
added
the
theme/supply-chain
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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)
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: 4
🤖 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/cli/validate.go`:
- Around line 570-576: The `flagNoSign` help text is outdated because it still
points users to `aicr evidence sign`, which is not part of the supported
workflow yet. Update the `cli.BoolFlag` usage string in `validate.go` to
describe only the fork-based CI workflow or a generic later-signing step, and
remove any mention of the deferred command so `aicr validate --help` matches the
actual shipped behavior.
In `@pkg/evidence/attestation/emit.go`:
- Around line 312-319: The unsigned success message in emitOutcome for the
opts.NoSign path is pointing users to a deferred subcommand that is not
available yet. Update the slog.Info text in the emit.go no-sign branch to use
supported fork-based CI workflow wording or a generic “sign later” message,
while keeping the existing reference and digest context intact.
In `@pkg/evidence/verifier/report.go`:
- Around line 206-208: The ExitInvalid verdict headline in report.go is too
specific to integrity checks and should be neutral so it matches all invalid
classes emitted by the verifier. Update the ExitInvalid branch in report.go’s
verdict rendering to use a generic invalid headline, and keep
writeFailureCause(...) for the detailed reason so registry, signature, schema,
and unknown failures are described correctly.
In `@pkg/evidence/verifier/verify.go`:
- Around line 165-170: Delay setting r.Pending in verifyBundle until after
predicate parsing, inventory verification, and the final exit-status decision
are complete. The current early return in verifyBundle marks unsigned bundles as
pending too soon, so later failures can still serialize as pending with a
non-zero exit. Move the pending-signature handling to the final outcome path,
and keep the stepSignature record in sync with the final state so that Pending
is only true for the exit-0 case.
🪄 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: 77642c70-ab2b-4307-97c5-55d77fca123b
📒 Files selected for processing (13)
docs/user/cli-reference.mdpkg/cli/consts.gopkg/cli/evidence_publish.gopkg/cli/validate.gopkg/cli/validate_evidence.gopkg/evidence/attestation/emit.gopkg/evidence/attestation/emit_test.gopkg/evidence/attestation/publish.gopkg/evidence/verifier/failure.gopkg/evidence/verifier/failure_test.gopkg/evidence/verifier/report.gopkg/evidence/verifier/types.gopkg/evidence/verifier/verify.go
Contributor
Coverage Report ✅
Coverage BadgeMerging this branch changes the coverage (1 decrease, 2 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
mchmarny
force-pushed
the
feat/evidence-no-sign-verify-cause
branch
from
fd2ad2b to
a7dcb9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
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/verify.go (1)
129-140: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExit-1 results still drop the structured cause.
When
hasPhaseFailures(pred)flips the result toExitValidPhaseFailures, this path leavesFailureCauseunset. That contradicts the documented verifier contract that non-zero exits carryfailureCause, so JSON/Markdown consumers lose the new machine-readable classification/hint on the exit-1 path. Either assign a dedicated cause here for phase failures or narrow the public contract in the same PR.🤖 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/verify.go` around lines 129 - 140, The ExitValidPhaseFailures path in verify.go is still leaving FailureCause unset, which breaks the verifier’s non-zero exit contract for structured errors. Update the logic around r.Exit and hasPhaseFailures(pred) so the ExitValidPhaseFailures branch also assigns an appropriate FailureCause (or explicitly adjust the documented contract in the same change). Use the existing verifier flow in verify.go and the r.Exit/FailureCause fields to keep the exit-1 result machine-readable for JSON/Markdown consumers.
🤖 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 `@docs/user/cli-reference.md`:
- Around line 2687-2690: Update the `evidence verify` exit-code table so it
matches the verifier contract already described below it: recorded validator
phase failures should be shown as exit 1, and only bundle invalid cases
(signature, integrity, or predicate failure) should be exit 2. Adjust the table
in the CLI reference to align with the `VerifyResult.Exit`/JSON `exit` semantics
and keep the surrounding paragraph consistent with the `evidence verify`
documentation.
In `@pkg/cli/validate.go`:
- Around line 570-574: Update the `flagNoSign` help text in
`buildRecipeEvidenceConfig()` so it explicitly mentions that
`--emit-attestation` is also required for `--no-sign` to have any effect. The
current `Usage` string for `flagNoSign` should describe the full contract,
including that `--push` alone is not sufficient and that evidence emission must
be enabled first. Keep the wording aligned with the existing `--push`/signing
explanation, but add the missing `--emit-attestation` prerequisite so users are
not misled.
In `@pkg/evidence/verifier/failure.go`:
- Around line 50-53: The stepMaterialize fallback in failure classification is
too broad and currently labels all status-less materialize errors as
CauseRegistry. Update the logic in the failure classification path around
stepMaterialize to distinguish true registry transport/availability errors from
local input/path validation cases like missing directories or malformed
references. If there is no registry evidence, default to CauseUnknown instead of
CauseRegistry so failureCause.class stays accurate.
---
Outside diff comments:
In `@pkg/evidence/verifier/verify.go`:
- Around line 129-140: The ExitValidPhaseFailures path in verify.go is still
leaving FailureCause unset, which breaks the verifier’s non-zero exit contract
for structured errors. Update the logic around r.Exit and hasPhaseFailures(pred)
so the ExitValidPhaseFailures branch also assigns an appropriate FailureCause
(or explicitly adjust the documented contract in the same change). Use the
existing verifier flow in verify.go and the r.Exit/FailureCause fields to keep
the exit-1 result machine-readable for JSON/Markdown consumers.
🪄 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: 664a9f08-ebce-4fd2-851b-ae3e77ef5f5b
📒 Files selected for processing (14)
docs/user/cli-reference.mdpkg/cli/consts.gopkg/cli/evidence_publish.gopkg/cli/validate.gopkg/cli/validate_evidence.gopkg/evidence/attestation/emit.gopkg/evidence/attestation/emit_test.gopkg/evidence/attestation/publish.gopkg/evidence/verifier/failure.gopkg/evidence/verifier/failure_test.gopkg/evidence/verifier/report.gopkg/evidence/verifier/types.gopkg/evidence/verifier/verify.gopkg/evidence/verifier/verify_test.go
mchmarny
force-pushed
the
feat/evidence-no-sign-verify-cause
branch
from
a7dcb9e to
62069c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
docs/user/cli-reference.md (1)
832-832: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the full
validate --no-signprerequisite.
validateonly reads this flag after--emit-attestationenables evidence emission, so this row still overstates what--pushalone does. Please align it with the actual contract: requires both--emit-attestationand--push, and is a no-op otherwise.🤖 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 `@docs/user/cli-reference.md` at line 832, The `--no-sign` CLI reference entry currently implies `--push` alone is sufficient, but `validate` only honors it when evidence emission is enabled. Update the documentation text for `--no-sign` in the CLI reference to state the full prerequisite: it requires both `--emit-attestation` and `--push`, and it is a no-op otherwise. Keep the wording aligned with the `validate` command contract and the existing `pointer.yaml`/pending signature behavior.pkg/evidence/attestation/publish.go (1)
49-53: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the deferred
aicr evidence signreference here too.This comment still advertises a command that this PR does not ship. Please keep it aligned with the supported fork-based CI workflow so the code comments don't drift back out of sync.
🤖 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/attestation/publish.go` around lines 49 - 53, The comment on NoSign in the Publish flow still mentions the deferred aicr evidence sign command, which this PR does not support. Update the documentation comment in the Publish-related struct to remove that command reference and keep the wording aligned with the fork-based CI workflow only, preserving the behavior description around unsigned bundle publishing and later signing.
🤖 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/cli/consts.go`:
- Around line 57-61: The comment for flagNoSign is conflating two different
pointer states by saying “rekorLogIndex nil.” Update the documentation near
flagNoSign in consts.go to describe the --no-sign case as an unsigned pointer
where PointerAttestation.Signer is nil, and keep RekorLogIndex == nil reserved
for the separate signed-without-Rekor case. Leave the rest of the flag
description intact but make the state distinction explicit.
In `@pkg/evidence/verifier/failure_test.go`:
- Around line 106-126: Refactor TestWriteVerdict_PendingAndCause in
failure_test.go into table-driven subtests so the pending case and the invalid
FailureCause case each run under their own t.Run. Keep the existing assertions
for RenderMarkdown, ExitValidPassed, ExitInvalid, and the FailureCause fields,
but move them into a shared case list to match the repo’s multi-case test style
and make future verdict variants easier to add.
In `@pkg/evidence/verifier/verify_test.go`:
- Around line 72-110: Convert TestVerify_PendingOnlyWhenExitZero into a
table-driven test using a slice of cases and a shared subtest loop, since both
scenarios validate the same Verify contract. Keep the existing checks around
Verify, ExitValidPassed, ExitInvalid, and Pending, but move the shared setup
through a helper pattern so each case only varies the tampering step and
expected outcomes. Use TestVerify_PendingOnlyWhenExitZero and Verify as the main
anchors when refactoring.
---
Duplicate comments:
In `@docs/user/cli-reference.md`:
- Line 832: The `--no-sign` CLI reference entry currently implies `--push` alone
is sufficient, but `validate` only honors it when evidence emission is enabled.
Update the documentation text for `--no-sign` in the CLI reference to state the
full prerequisite: it requires both `--emit-attestation` and `--push`, and it is
a no-op otherwise. Keep the wording aligned with the `validate` command contract
and the existing `pointer.yaml`/pending signature behavior.
In `@pkg/evidence/attestation/publish.go`:
- Around line 49-53: The comment on NoSign in the Publish flow still mentions
the deferred aicr evidence sign command, which this PR does not support. Update
the documentation comment in the Publish-related struct to remove that command
reference and keep the wording aligned with the fork-based CI workflow only,
preserving the behavior description around unsigned bundle publishing and later
signing.
🪄 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: 79094c92-ebd8-420d-ba2e-61801b80510e
📒 Files selected for processing (14)
docs/user/cli-reference.mdpkg/cli/consts.gopkg/cli/evidence_publish.gopkg/cli/validate.gopkg/cli/validate_evidence.gopkg/evidence/attestation/emit.gopkg/evidence/attestation/emit_test.gopkg/evidence/attestation/publish.gopkg/evidence/verifier/failure.gopkg/evidence/verifier/failure_test.gopkg/evidence/verifier/report.gopkg/evidence/verifier/types.gopkg/evidence/verifier/verify.gopkg/evidence/verifier/verify_test.go
mchmarny
force-pushed
the
feat/evidence-no-sign-verify-cause
branch
from
62069c2 to
575c300
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
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/verify.go (1)
93-104: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStop falling back to the unsigned predicate after a signature failure.
When
stepSignatureCheckreturns(nil, false)because signature verification or pointer-signer cross-check failed,Verifystill callsresolvePredicate, which loadsstatement.intoto.jsonand can populatePredicate,RecipeName, and later passed steps from untrusted bundle bytes. That makes an exit-2 result look partially verified even though the trust anchor already failed. Only use the unsigned fallback for the explicit unsigned/pending path.Suggested guard
verifiedPredicate, pendingSignature := stepSignatureCheck(ctx, r, mat, opts) + if r.Exit == ExitInvalid && !pendingSignature { + record(r, stepRender, StepInformational, "report assembled", nil) + return r, nil + } // Step 3 — predicate parse. Prefer the predicate the signature🤖 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/verify.go` around lines 93 - 104, Stop using the unsigned predicate fallback after a failed signature check. In Verify, after stepSignatureCheck returns nil,false for signature or pointer-signer cross-check failures, do not call resolvePredicate on mat; only allow that fallback when the explicit unsigned/pending path is active. Update the control flow around stepSignatureCheck, resolvePredicate, and the predicate step so untrusted bundle bytes cannot populate Predicate or RecipeName after trust anchoring has already failed.
🤖 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 `@docs/user/cli-reference.md`:
- Around line 2687-2689: The exit code 2 row in the CLI reference is too narrow
and should be expanded to match the verifier’s current invalid-bundle behavior.
Update the description in the exit-code table to reference the broader
failureCause outcomes handled by the verifier, including registry and
schema-class failures in addition to signature, integrity, and predicate
failures. Use the exit-code table entry in the CLI reference to locate the text
and keep the wording aligned with the verifier contract.
- Line 832: The `--no-sign` CLI table entry documents the wrong activation
contract in `docs/user/cli-reference.md`. Update the description to match the
implementation/help text for `validate --no-sign`: it only applies when both
`--emit-attestation` and `--push` are set, and it should be described as a no-op
otherwise. Keep the existing pointer/signing behavior wording, but make the flag
dependency explicit so the docs match the `validate` command behavior.
In `@pkg/evidence/verifier/failure_test.go`:
- Around line 54-56: The test in failure_test.go only asserts that hint-bearing
statuses like 401/403/404 set c.Hint, but it never checks the opposite path for
the generic 500 case. Update the table-driven test around the failure verifier
to explicitly assert that when tt.status is 500 (or any non-actionable status),
c.Hint remains empty, using the existing verifier output field c.Hint and the
tt.wantHint flag to cover both positive and negative branches.
---
Outside diff comments:
In `@pkg/evidence/verifier/verify.go`:
- Around line 93-104: Stop using the unsigned predicate fallback after a failed
signature check. In Verify, after stepSignatureCheck returns nil,false for
signature or pointer-signer cross-check failures, do not call resolvePredicate
on mat; only allow that fallback when the explicit unsigned/pending path is
active. Update the control flow around stepSignatureCheck, resolvePredicate, and
the predicate step so untrusted bundle bytes cannot populate Predicate or
RecipeName after trust anchoring has already failed.
🪄 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: 54d81214-ff20-4e0b-a4a5-d35da27cd9a3
📒 Files selected for processing (14)
docs/user/cli-reference.mdpkg/cli/consts.gopkg/cli/evidence_publish.gopkg/cli/validate.gopkg/cli/validate_evidence.gopkg/evidence/attestation/emit.gopkg/evidence/attestation/emit_test.gopkg/evidence/attestation/publish.gopkg/evidence/verifier/failure.gopkg/evidence/verifier/failure_test.gopkg/evidence/verifier/report.gopkg/evidence/verifier/types.gopkg/evidence/verifier/verify.gopkg/evidence/verifier/verify_test.go
Tune the evidence pilot's CLI/verifier so contributors can split the network-light push leg from the Fulcio-bound signing leg, and so the gate can render actionable verify failures instead of a bare "invalid". Push-without-sign (part of #1434): - Add --no-sign to `validate --emit-attestation` and `evidence publish`. When set with --push, the bundle is pushed content-addressed but unsigned, and the pointer is written with bundle.oci/digest populated and an empty signer block. Skips all OIDC/Fulcio/Rekor work and the identity-disclosure prompt, so it runs where Sigstore egress is blocked. Verify pending + structured cause (#1437 Go half; part of #1434): - evidence verify reports an unsigned pointer (no signer claim) as a non-failing "pending signature" state: exit 0, VerifyResult.Pending true, distinct verdict line. An in-flight PR's unsigned pointer is no longer conflated with "all checks passed" or flagged invalid. - Non-zero exits now carry a structured VerifyResult.FailureCause {class, detail, httpStatus, hint}, classified from the error chain: registry 401/403 -> registry-forbidden (+make-package-public hint), 404 -> not-found, else by step (signature/integrity/schema). Surfaced in --format json and the Markdown verdict (Cause/Hint lines). The sign-existing leg (`aicr evidence sign <pointer>`) that completes #1434 lands as a focused follow-up: it needs registry descriptor resolution and its happy path is registry-bound (not unit-testable), mirroring the existing Publish precedent. Tests: unsigned-pushed outcome -> pointer mapping; failure classifier (403/404/other + per-step) and first-wins; pending/cause rendering.
mchmarny
force-pushed
the
feat/evidence-no-sign-verify-cause
branch
from
575c300 to
c473bab
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
pkg/evidence/attestation/publish.go (1)
49-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the deferred
aicr evidence signreference.Line 51 still documents a subcommand this PR does not ship. Keep this public
PublishOptionscomment aligned with the supported “sign later via the fork-based CI workflow” wording used elsewhere.Suggested fix
- // NoSign pushes the unsigned bundle and writes a pointer with an empty - // signer block instead of signing. The Fulcio/Rekor leg is deferred to - // `aicr evidence sign` (or the fork-based CI workflow). When false, - // Publish signs as before. + // NoSign pushes the unsigned bundle and writes a pointer with an empty + // signer block instead of signing. The Fulcio/Rekor leg is deferred to + // the fork-based CI workflow. When false, Publish signs as before.🤖 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/attestation/publish.go` around lines 49 - 52, The PublishOptions comment in the NoSign documentation still mentions the deferred aicr evidence sign subcommand, which is not part of this PR. Update the comment near NoSign in publish.go to remove that reference and keep it aligned with the supported “sign later via the fork-based CI workflow” wording used elsewhere, so the public API docs only describe shipped behavior.
🤖 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/types.go`:
- Around line 156-157: Update the comment on FailureCause in the verifier types
so it matches the new contract: it should describe that FailureCause is set for
failing results such as ExitInvalid, not only for non-zero exits. Adjust the
field documentation near FailureCause in the types definition to reflect when
failureCause is expected in JSON/YAML output, so callers aren’t misled by the
current nil-for-0/1 wording.
In `@pkg/evidence/verifier/verify_test.go`:
- Around line 77-101: The pending-contract table in verify_test.go only covers
the clean unsigned path and the integrity-failure path, so it misses the
ExitValidPhaseFailures branch. Add a new table case using the same verification
flow that triggers a phase failure and asserts wantExit is
ExitValidPhaseFailures while wantPending remains false, keeping the existing
verify/pending expectations aligned with the verifier behavior.
---
Duplicate comments:
In `@pkg/evidence/attestation/publish.go`:
- Around line 49-52: The PublishOptions comment in the NoSign documentation
still mentions the deferred aicr evidence sign subcommand, which is not part of
this PR. Update the comment near NoSign in publish.go to remove that reference
and keep it aligned with the supported “sign later via the fork-based CI
workflow” wording used elsewhere, so the public API docs only describe shipped
behavior.
🪄 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: 6fc118ad-b689-43f3-96ba-d95c2496f2d6
📒 Files selected for processing (14)
docs/user/cli-reference.mdpkg/cli/consts.gopkg/cli/evidence_publish.gopkg/cli/validate.gopkg/cli/validate_evidence.gopkg/evidence/attestation/emit.gopkg/evidence/attestation/emit_test.gopkg/evidence/attestation/publish.gopkg/evidence/verifier/failure.gopkg/evidence/verifier/failure_test.gopkg/evidence/verifier/report.gopkg/evidence/verifier/types.gopkg/evidence/verifier/verify.gopkg/evidence/verifier/verify_test.go
Address review on PR #1445: - FailureCause field comment: set only for ExitInvalid (2), nil for exit 0 (valid/pending) and exit 1 (phase failures) — removes the ambiguous 'non-zero Exit' wording. - TestVerify_PendingOnlyWhenExitZero: add an exit-1 case via a new buildTestBundleFailedPhase helper, asserting Pending stays false for ExitValidPhaseFailures (the previously-uncovered branch).
lalitadithya
approved these changes
Jun 24, 2026
6 tasks
20 tasks
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
Adds
--no-sign(push-without-sign) tovalidate --emit-attestation/evidence publish, and teachesevidence verifyto report unsigned pointers as a non-failing pending signature state and to emit a structured, classifiedfailureCause(e.g. registry 403) on failures.Motivation / Context
Part of the evidence-gate pilot tuning epic. This is the first of two PRs for the CLI/verifier work: it decouples the network-light push leg from the Fulcio-bound signing leg (so contributors blocked from Sigstore egress can still push), and makes verify failures self-serviceable instead of a bare "invalid".
Fixes: N/A
Related: #1431, #1434 (push-without-sign + verify-pending; sign-existing follows up), #1437 (Go half — structured cause)
Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)pkg/errors,pkg/k8s)docs/,examples/)pkg/evidence/{attestation,verifier}Implementation Notes
--no-signshort-circuitssignAndPushafter the content-addressed push: no artifact statement, no OIDC resolve, no Fulcio/Rekor, no identity-disclosure prompt. The pointer is written withbundle.oci/bundle.digestset and an emptysignerblock.evidence verify: an unsigned pointer with no signer claim setsVerifyResult.Pending(exit 0) — distinct from "all checks passed" and from "invalid". A pointer that claims a signer over an unsigned bundle still fails (cross-check), as before.VerifyResult.FailureCause {class, detail, httpStatus, hint}, classified from the error chain (oraserrcode.ErrorResponsestatus takes precedence over the failing step). Surfaced in--format jsonand the Markdown verdict.aicr evidence sign <pointer>(sign-existing) — it needs registry main-artifact descriptor resolution and its happy path is registry-bound (not unit-testable), mirroring the existingPublishprecedent. That PR completes aicr evidence: two-phase push-without-sign + sign-existing support #1434.Testing
New unit tests: unsigned-pushed outcome → pointer mapping; failure classifier (403/404/other + per-step) with first-wins semantics; pending/cause rendering. Registry-bound happy paths (
--no-signpush, sign, attach) are not unit-testable, consistent with existingPublishcoverage.Risk Assessment
Rollout notes: Backwards-compatible. New
pending/failureCauseJSON fields areomitempty;--no-signdefaults off.Checklist
make testwith-race)make lint)git commit -S)