Skip to content

Commit 168d1d3

Browse files
committed
fix(validate): never emit evidence in --no-cluster dry-run mode
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. 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>
1 parent 6f9050b commit 168d1d3

6 files changed

Lines changed: 92 additions & 7 deletions

File tree

docs/user/cli-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,7 @@ aicr validate [flags]
832832
| `--timeout` | | duration | 5m | Timeout for validation Job completion |
833833
| `--no-cleanup` | | bool | false | Skip removal of Job and RBAC resources on completion |
834834
| `--require-gpu` | | bool | false | Require GPU resources on the validation pod |
835-
| `--no-cluster` | | bool | false | Skip cluster access (test mode): skips RBAC and Job deployment, reports checks as skipped |
835+
| `--no-cluster` | | bool | false | Skip cluster access (test mode): skips RBAC and Job deployment, reports checks as skipped. An offline dry-run does not sign or push a recipe-evidence attestation, so `--emit-attestation`/`--push` and `spec.validate.evidence.attestation` are ignored in this mode. Cannot be combined with `--cncf-submission` (that collector requires a live cluster); `--evidence-dir` conformance markdown is still rendered locally |
836836
| `--evidence-dir` | | string | | Directory to write conformance evidence artifacts |
837837
| `--cncf-submission` | | bool | false | Generate CNCF conformance submission artifacts |
838838
| `--feature` | `-f` | string[] | | CNCF evidence-collection feature(s) to scope (repeatable). Valid names: `dra-support`, `gang-scheduling`, `secure-access`, `accelerator-metrics`, `ai-service-metrics`, `inference-gateway`, `robust-operator`, `pod-autoscaling`, `cluster-autoscaling`. Empty selects all features. |

pkg/cli/validate.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,16 @@ Run validation without failing on check errors (informational mode):
625625
if len(features) > 0 && !cncfSubmission {
626626
return errors.New(errors.ErrCodeInvalidRequest, "--feature requires --cncf-submission")
627627
}
628+
// --cncf-submission deploys GPU workloads and collects behavioral
629+
// evidence against the active kube-context, so it is incompatible with
630+
// the offline --no-cluster dry-run. The short-circuit below reaches the
631+
// live-cluster collector directly, bypassing the noCluster handling
632+
// further down, so reject the combination here rather than silently
633+
// contacting the cluster.
634+
if cncfSubmission && boolFlagOrConfig(cmd, "no-cluster", resolved.NoCluster) {
635+
return errors.New(errors.ErrCodeInvalidRequest,
636+
"--cncf-submission cannot be combined with --no-cluster: the behavioral evidence collector requires a live cluster")
637+
}
628638

629639
// Short-circuit: --cncf-submission bypasses normal validation and runs
630640
// the behavioral evidence collector directly. When recipe context is
@@ -758,7 +768,7 @@ Run validation without failing on check errors (informational mode):
758768
"cleanupHint", "kubectl delete clusterrolebinding -l app.kubernetes.io/name=aicr-validator")
759769
}
760770

761-
evidenceCfg := buildRecipeEvidenceConfig(cmd, resolved)
771+
evidenceCfg := evidenceConfigForRunMode(noCluster, buildRecipeEvidenceConfig(cmd, resolved))
762772

763773
return runValidation(ctx, client, rec, snap, validationConfig{
764774
phases: phases,

pkg/cli/validate_evidence.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package cli
1616

1717
import (
1818
"context"
19+
"log/slog"
1920
"os"
2021

2122
"github.com/urfave/cli/v3"
@@ -82,6 +83,25 @@ func buildRecipeEvidenceConfig(cmd *cli.Command, resolved *config.ValidateResolv
8283
}
8384
}
8485

86+
// evidenceConfigForRunMode suppresses recipe-evidence emission in --no-cluster
87+
// mode. An offline dry-run reports every check as "skipped", so an emitted
88+
// attestation would attest to nothing. Worse, when the config sets
89+
// spec.validate.evidence.attestation.{out,push} (as the UAT test configs do), a
90+
// dry-run would sign and push a bundle to the same OCI repo the authoritative
91+
// live-cluster validate later pushes to — leaving two independently-signed
92+
// bundles whose digests differ, which breaks `aicr evidence verify`: the signed
93+
// subject no longer matches the pulled run-tagged artifact. A dry-run must never
94+
// emit signed evidence, so drop the config and log once. Returns cfg unchanged
95+
// for a live-cluster run (including cfg == nil).
96+
func evidenceConfigForRunMode(noCluster bool, cfg *recipeEvidenceConfig) *recipeEvidenceConfig {
97+
if noCluster && cfg != nil {
98+
slog.Warn("skipping evidence emission: --no-cluster is an offline dry-run and must not sign or push an attestation",
99+
"emitAttestation", cfg.OutDir, "push", cfg.Push)
100+
return nil
101+
}
102+
return cfg
103+
}
104+
85105
// oidcResolveOptionsFromFlags builds the keyless-signing OIDC resolution
86106
// options from the shared --identity-token / --oidc-device-flow flag pair
87107
// plus the GitHub Actions ambient-OIDC env vars. Shared by every command

pkg/cli/validate_evidence_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,3 +189,33 @@ func TestBuildRecipeEvidenceConfig(t *testing.T) {
189189
})
190190
}
191191
}
192+
193+
func TestEvidenceConfigForRunMode(t *testing.T) {
194+
cfg := &recipeEvidenceConfig{
195+
OutDir: "/tmp/out",
196+
Push: "ghcr.io/nvidia/aicr-evidence/example",
197+
}
198+
tests := []struct {
199+
name string
200+
noCluster bool
201+
cfg *recipeEvidenceConfig
202+
wantNil bool
203+
}{
204+
{"live cluster keeps emit config", false, cfg, false},
205+
{"no-cluster drops emit config", true, cfg, true},
206+
{"live cluster nil stays nil", false, nil, true},
207+
{"no-cluster nil stays nil", true, nil, true},
208+
}
209+
for _, tt := range tests {
210+
t.Run(tt.name, func(t *testing.T) {
211+
got := evidenceConfigForRunMode(tt.noCluster, tt.cfg)
212+
if (got == nil) != tt.wantNil {
213+
t.Fatalf("evidenceConfigForRunMode() nil = %v, want nil = %v", got == nil, tt.wantNil)
214+
}
215+
// A live-cluster run must pass the config through unchanged.
216+
if !tt.noCluster && tt.cfg != nil && got != tt.cfg {
217+
t.Errorf("live-cluster run mutated the evidence config: got %+v, want %+v", got, tt.cfg)
218+
}
219+
})
220+
}
221+
}

pkg/cli/validate_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ func TestValidateCmd_CNCFSubmissionFlagValidation(t *testing.T) {
205205
wantErr: true,
206206
errContain: "unknown feature",
207207
},
208+
{
209+
name: "cncf-submission with no-cluster",
210+
args: []string{"aicr", "validate", "--cncf-submission", "--evidence-dir", "/tmp/test", "--no-cluster"},
211+
wantErr: true,
212+
errContain: "--cncf-submission cannot be combined with --no-cluster",
213+
},
208214
}
209215

210216
for _, tt := range tests {

tests/uat/lib/phases.sh

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,30 @@ phase_prep() {
170170
echo "::endgroup::"
171171

172172
echo "::group::Validate (dry-run, --no-cluster)"
173-
"${AICR_BIN}" validate \
174-
--config "${config}" \
175-
--phase deployment \
176-
--no-cluster \
177-
--output dry-run.json
173+
# Validate against a copy of the config with spec.validate.evidence stripped so
174+
# the offline dry-run cannot emit/sign/push an evidence bundle. --no-cluster
175+
# reports every check as "skipped", so an attestation would attest to nothing;
176+
# worse, with evidence.attestation.{out,push} set (as UAT configs are) the
177+
# dry-run would sign and push a bundle to the same OCI repo the conformance
178+
# phase's authoritative validate later pushes to, leaving two independently-
179+
# signed bundles and breaking `evidence verify` (signed subject != pulled
180+
# run-tagged digest). Mirrors the readiness-gate strip in phase_conformance.
181+
# Belt-and-suspenders with the CLI's own --no-cluster guard: this keeps prep
182+
# safe even against an older released aicr that lacks that guard.
183+
# Run in a subshell with an EXIT trap so the temp config is removed on every
184+
# exit path (normal return, set -e abort on validate failure, interrupt),
185+
# mirroring the signing subshell below. A non-zero validate rc propagates out
186+
# of the subshell and aborts prep under set -e, as before.
187+
(
188+
prep_config="$(mktemp)"
189+
trap 'rm -f "${prep_config}"' EXIT
190+
yq 'del(.spec.validate.evidence)' "${config}" > "${prep_config}"
191+
"${AICR_BIN}" validate \
192+
--config "${prep_config}" \
193+
--phase deployment \
194+
--no-cluster \
195+
--output dry-run.json
196+
)
178197
test -f dry-run.json
179198
echo "::endgroup::"
180199

0 commit comments

Comments
 (0)