fix(validate): never emit evidence in --no-cluster dry-run mode by njhensley · Pull Request #1810 · NVIDIA/aicr

njhensley · 2026-07-17T23:15:36Z

Summary

validate --no-cluster (an offline dry-run) still emitted and signed a full evidence bundle when the config set spec.validate.evidence.attestation.*, writing attestation.intoto.jsonl into the shared evidence output dir. In the UAT flow this lets the prep phase's leftover signature get packed into the authoritative run-tag artifact, which breaks aicr evidence verify and the Evidence: Ingest job with a signed-subject/pulled-digest mismatch.

Motivation / Context

The UAT emit output dir is attestation.out: ./evidence, shared across phases in the runner workspace.

prep runs validate --no-cluster (dry-run, all checks skipped). It still honored the config's evidence block, signed a bundle to the aicr content-hash tag (…-59266d32abc3, digest d3466f4c…), and left attestation.intoto.jsonl (subject = d3466f4c…) in ./evidence/.
conformance then runs the authoritative validate --phase all, which re-packs ./evidence/ into the run-<id> artifact (2a486d77…). The closed-world pack sweeps in prep's stale attestation.intoto.jsonl, so the run-tag artifact embeds a signature for a foreign subject (d3466f4c…, not 2a486d77…).
Normally the conformance emit's own Sigstore referrer overwrites the staged attestation at verify time, masking this. On the failing run the referrer push hit GHCR timeouts and never attached, so MaterializeBundle found no referrer to overwrite the embedded stale file, and VerifySignature read the embedded d3466f4c attestation → mismatch.

Directly confirmed against the registry:

oras pull …@sha256:2a486d77… → the artifact embeds attestation.intoto.jsonl, and its DSSE payload signs subject d3466f4c… (prep's content-tag ref).
oras discover …@sha256:2a486d77… → zero referrers (the conformance signature never attached).

This is why it is intermittent / "new": it needs prep's stale attestation (present on every emit-enabled config) and the conformance referrer failing to attach (a GHCR write hiccup). Healthy runs overwrite the stale file via the referrer and pass — which is why the H100/release cells are green. Observed on run 29611408651 (verify) and 29619695743 (ingest). An offline dry-run over an all-skipped run attests to nothing, so it should never sign or write an attestation in the first place.

Fixes: N/A
Related: #1793 (@main verify regression since #1758 closed-world pack)

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Refactoring (no functional changes)
Build/CI/tooling

Component(s) Affected

CLI (cmd/aicr, pkg/cli)
Docs/examples (docs/, examples/)
Other: UAT harness (tests/uat/lib/phases.sh)

Implementation Notes

Two layers so a dry-run can never leave a signed attestation behind:

CLI (authoritative): evidenceConfigForRunMode drops the recipe-evidence config whenever --no-cluster is set, so an offline dry-run can never sign, push, or write attestation.intoto.jsonl, regardless of config. Logs once when it suppresses emission.
UAT harness (belt-and-suspenders): the prep phase validates against a config copy with spec.validate.evidence stripped (mirroring the readiness-gate strip already used in the conformance phase). Keeps prep safe even against an older released aicr that predates the CLI guard.

With prep no longer emitting, ./evidence/ holds no stale attestation for the conformance pack to embed, so the run-tag artifact carries only its own signature. Producer-side fix — no change needed in verify/ingest.

Possible follow-up (out of scope): have the emit exclude any pre-existing attestation.intoto.jsonl from the pack, and/or have MaterializeBundle prefer the referrer over an embedded attestation — defense-in-depth against any stray signed file in the output dir.

Testing

go build ./pkg/cli/...
go test ./pkg/cli/ -count=1            # ok (incl. new TestEvidenceConfigForRunMode)
golangci-lint run -c .golangci.yaml ./pkg/cli/...   # 0 issues
shellcheck -x tests/uat/lib/phases.sh  # clean (SC1091 info only)

Added TestEvidenceConfigForRunMode covering live-cluster pass-through, --no-cluster suppression, and nil-in/nil-out. Verified the mechanism against the live registry with oras pull / oras discover (see Motivation).

Risk Assessment

Low — Isolated change, well-tested, easy to revert

Rollout notes: Behavior change is limited to --no-cluster runs, which are dry-runs that should not have been producing evidence; live-cluster validate (the only path that emits authoritative evidence) is unchanged. Backwards compatible.

Checklist

Tests pass locally (make test with -race) — ran go test ./pkg/cli/
Linter passes (make lint) — golangci-lint 0 issues on pkg/cli
I did not skip/disable tests to make CI green
I added/updated tests for new functionality
I updated docs if user-facing behavior changed
Changes follow existing patterns in the codebase
Commits are cryptographically signed (git commit -S)

github-actions · 2026-07-17T23:19:01Z

🌿 Preview your docs: https://nvidia-preview-fix-no-cluster-skip-evidence-emit.docs.buildwithfern.com/aicr

coderabbitai · 2026-07-17T23:20:22Z

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 5dba56dd-53bf-42b7-9c8a-6b240759395e

📥 Commits

Reviewing files that changed from the base of the PR and between 168d1d3 and 198e45c.

📒 Files selected for processing (6)

docs/user/cli-reference.md
pkg/cli/validate.go
pkg/cli/validate_evidence.go
pkg/cli/validate_evidence_test.go
pkg/cli/validate_test.go
tests/uat/lib/phases.sh

📝 Walkthrough

Walkthrough

aicr validate now suppresses recipe-evidence configuration during --no-cluster runs, rejects incompatible CNCF submission mode, and preserves evidence handling for live-cluster runs. Tests cover both behaviors, while UAT preparation and CLI documentation reflect offline-mode semantics.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: theme/validation

Suggested reviewers: lockwobr

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main behavior change: suppressing evidence emission in no-cluster dry-run mode.
Description check	✅ Passed	The description is detailed and directly matches the changeset, covering the bug, motivation, and implementation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 3

🤖 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`:
- Line 835: Update the no-cluster CLI documentation to match runValidation’s
actual behavior: either ensure noCluster also suppresses evidenceDir-driven
conformance artifacts, or narrow the description to state that only
recipe-attestation bundles and related options are ignored. Keep the documented
behavior consistent with the implementation.

In `@pkg/cli/validate.go`:
- Line 761: Update the --cncf-submission flow in pkg/cli/validate.go around
runCNCFSubmission and evidenceConfigForRunMode so --cncf-submission --no-cluster
cannot reach the live-cluster evidence collector; either reject the combination
or pass noCluster into runCNCFSubmission and honor it. In
docs/user/cli-reference.md, narrow the --no-cluster description to match the
supported --evidence-dir behavior.

In `@tests/uat/lib/phases.sh`:
- Around line 183-191: Update the temporary config setup in the deployment
validation flow to register a scoped cleanup trap immediately after prep_config
is created with mktemp. Ensure the trap removes prep_config on command failure,
interruption, or normal completion, and avoid relying solely on the later
explicit rm command.

🪄 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: fc4c2150-81f5-49b4-bc4f-b158ea84363a

📥 Commits

Reviewing files that changed from the base of the PR and between 6f9050b and 037d5ce.

📒 Files selected for processing (5)

docs/user/cli-reference.md
pkg/cli/validate.go
pkg/cli/validate_evidence.go
pkg/cli/validate_evidence_test.go
tests/uat/lib/phases.sh

github-actions · 2026-07-17T23:22:05Z

Recipe evidence check

No leaf overlays affected by this PR.

This gate is warning-only and never blocks merge.

njhensley · 2026-07-17T23:43:35Z

Addressed the CodeRabbit review (all 3 findings) in the amended commit:

--cncf-submission --no-cluster (Major): now rejected in the flag-combination validation — the CNCF collector short-circuits straight to the live cluster, bypassing the noCluster handling, so the combination fails closed with ErrCodeInvalidRequest instead of silently contacting the cluster. Added a table case to TestValidateCmd_CNCFSubmissionFlagValidation.
Docs claim too broad (Minor): narrowed the --no-cluster row to state that only the recipe-evidence attestation is suppressed (not signed/pushed); --evidence-dir CNCF conformance markdown is still rendered locally.
Temp config leak (Minor): phase_prep now captures the validate rc and removes the temp config on the failure path too (mirrors the vrc pattern in phase_conformance).

go build/go test ./pkg/cli/ (incl. new cases), golangci-lint (0 issues), and shellcheck -x all pass locally.

coderabbitai

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/cli/validate_test.go`:
- Around line 208-213: Extend the “cncf-submission with no-cluster” validation
test to assert the returned error has code ErrCodeInvalidRequest, not only the
expected message. Add a separate case where no-cluster is enabled through
resolved configuration, and verify it produces the same validation error and
code.

🪄 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: 326af2e3-b12e-4986-81e3-12d97b285466

📥 Commits

Reviewing files that changed from the base of the PR and between 0f48a8f and 168d1d3.

📒 Files selected for processing (6)

docs/user/cli-reference.md
pkg/cli/validate.go
pkg/cli/validate_evidence.go
pkg/cli/validate_evidence_test.go
pkg/cli/validate_test.go
tests/uat/lib/phases.sh

A --no-cluster validate is an offline dry-run that reports every check as "skipped", yet it still honored spec.validate.evidence.attestation.{out,push} and built, signed, and wrote attestation.intoto.jsonl into the evidence output dir (and pushed the bundle). In the UAT flow the emit output dir (attestation.out: ./evidence) is shared across phases in the runner workspace. The prep phase's dry-run therefore left a signed attestation.intoto.jsonl (subject = prep's content-hash OCI digest) in ./evidence/. The later conformance-phase emit re-packs ./evidence/ into the run-<id> artifact; the closed-world pack sweeps in that stale attestation, so the run-tag artifact embeds a signature for a FOREIGN subject (prep's digest, not the run-tag digest). Normally the conformance emit's own Sigstore referrer overwrites the staged attestation at verify time and masks this; when that referrer push fails to attach (observed: GHCR timeouts), `aicr evidence verify` (and the Evidence: Ingest job) fall back to the embedded stale attestation and fail with "signed subject digest (<prep digest>) does not match pulled artifact digest (<run-tag digest>)". Confirmed by `oras pull` of the run-tag artifact: it embeds attestation.intoto.jsonl signing the prep digest, and the run-tag artifact has zero referrers. Fix in two layers: - CLI: evidenceConfigForRunMode drops the recipe-evidence config when --no-cluster is set, so an offline dry-run can never sign or push an attestation regardless of config. Emitting evidence over an all-skipped run attests to nothing. - UAT: the prep phase validates against a config copy with spec.validate.evidence stripped (mirroring the readiness-gate strip in the conformance phase), keeping prep safe even against an older released aicr that predates the CLI guard. With prep no longer emitting, no attestation.intoto.jsonl is left in ./evidence/ for the conformance pack to embed, so the run-tag artifact carries only its own signature. Documents the behavior on the --no-cluster row of the CLI reference. - flag-combination tests assert ErrCodeInvalidRequest (structured code), not just the message text. Signed-off-by: Nathan Hensley <nhensley@nvidia.com> Review (CodeRabbit) follow-ups: - Reject --cncf-submission --no-cluster: the behavioral evidence collector short-circuits to the live cluster, bypassing the noCluster handling, so the combination must fail closed rather than silently contact the cluster. - Narrow the --no-cluster doc to what actually changes (recipe-evidence attestation is not signed/pushed); CNCF conformance markdown via --evidence-dir is still rendered locally. - prep temp config cleanup runs on every exit path via a subshell + EXIT trap (matches the signing subshell in phases.sh), not just the normal path. Signed-off-by: Nathan Hensley <nhensley@nvidia.com>

njhensley requested a review from a team as a code owner July 17, 2026 23:15

njhensley added the theme/supply-chain SLSA, SBOM, Sigstore, and provenance verification label Jul 17, 2026

njhensley self-assigned this Jul 17, 2026

github-actions Bot added area/tests area/docs area/cli size/M labels Jul 17, 2026

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread docs/user/cli-reference.md Outdated

Comment thread pkg/cli/validate.go

Comment thread tests/uat/lib/phases.sh Outdated

njhensley force-pushed the fix/no-cluster-skip-evidence-emit branch 2 times, most recently from 8682ef3 to 0f48a8f Compare July 17, 2026 23:42

njhensley force-pushed the fix/no-cluster-skip-evidence-emit branch from 0f48a8f to 168d1d3 Compare July 17, 2026 23:48

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread pkg/cli/validate_test.go

njhensley force-pushed the fix/no-cluster-skip-evidence-emit branch from 168d1d3 to 198e45c Compare July 17, 2026 23:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(validate): never emit evidence in --no-cluster dry-run mode#1810

fix(validate): never emit evidence in --no-cluster dry-run mode#1810
njhensley wants to merge 1 commit into
NVIDIA:mainfrom
njhensley:fix/no-cluster-skip-evidence-emit

njhensley commented Jul 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

Reviews paused

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

njhensley commented Jul 17, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

njhensley commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation / Context

Type of Change

Component(s) Affected

Implementation Notes

Testing

Risk Assessment

Checklist

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

coderabbitai Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 17, 2026

Recipe evidence check

Uh oh!

njhensley commented Jul 17, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

njhensley commented Jul 17, 2026 •

edited

Loading

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading