docs: correct supply-chain, evidence & provenance documentation#1547
Conversation
|
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 updates attestation-verification examples and provenance docs to use explicit repository and signer-workflow inputs instead of Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/design/007-recipe-evidence.md`:
- Around line 19-27: Update the wording in the design doc around the verifier
behavior to qualify the “pending signature” claim: in the section describing the
shipped verifier, make it clear that `pending signature` from the verifier is
only returned after predicate/schema parse and manifest-inventory hash binding
succeed. Use the existing terms in the paragraph (“shipped verifier”, “pending
signature”, “verify”, “unsigned bundle”) and adjust the sentence so unsigned
bundles that fail other validation are described as those failures, not as
`pending signature`.
🪄 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: d19af244-a54f-43ed-92a3-1a3c4a907ecd
📒 Files selected for processing (11)
SECURITY.mddemos/provenance-demo-slides.htmldemos/provenance-demo.shdemos/provenance.mddocs/contributor/maintaining.mddocs/design/007-recipe-evidence.mddocs/design/014-ocp-helm.mddocs/integrator/automation.mddocs/integrator/supply-chain-verification.mddocs/user/artifact-verification.mdpkg/server/doc.go
2097bf2 to
548cfe0
Compare
There was a problem hiding this comment.
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/contributor/api-server.md`:
- Around line 73-75: The API server docs currently imply that the `/` root route
is registered through WithHandler, but the actual registration happens in
configureRootHandler and then it is wrapped through the normal middleware flow.
Reword the root-route description in the contributor docs to distinguish custom
handlers added by WithHandler from the built-in `/` route inserted by
configureRootHandler, while keeping the note that system endpoints like /health,
/ready, and /metrics bypass the chain.
In `@docs/integrator/data-flow.md`:
- Around line 202-212: The matching-criteria description conflicts with the
later RecipeResult field list because it includes platform and node-count as
criteria while the diagram box still only shows service, accelerator, intent,
and os. Update the data-flow section so both places agree: either add platform
and node-count to the RecipeResult list or remove them from the criteria
sentence, and keep the terminology consistent across the surrounding
matching/constraint explanation.
In `@docs/integrator/kubernetes-deployment.md`:
- Around line 515-517: The port-forward example in the Kubernetes deployment
docs can race because curl runs before kubectl port-forward is ready. Update the
snippet around kubectl port-forward and the curl example to wait briefly or
retry until the local 8080 endpoint is available before issuing curl, so the
copy-paste flow is reliable on slower machines.
🪄 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: 134fe4b7-12aa-4d9f-9222-955268ca33ba
📒 Files selected for processing (19)
SECURITY.mddemos/bundle-attestation.mddemos/cuj2-demo.mddemos/end-to-end-cli.mddemos/provenance-demo-slides.htmldemos/provenance-demo.shdemos/provenance.mddocs/contributor/api-server.mddocs/contributor/maintaining.mddocs/contributor/tests.mddocs/design/007-recipe-evidence.mddocs/design/014-ocp-helm.mddocs/integrator/automation.mddocs/integrator/data-flow.mddocs/integrator/kubernetes-deployment.mddocs/integrator/supply-chain-verification.mddocs/user/api-reference.mddocs/user/artifact-verification.mdpkg/server/doc.go
💤 Files with no reviewable changes (1)
- demos/cuj2-demo.md
548cfe0 to
35818fd
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/contributor/api-server.md (1)
73-75: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the root-route attribution precise.
WithHandleronly wires custom handlers; the built-in/route is injected byconfigureRootHandlerand then wrapped like the other handlers. Please reword this so the docs distinguish those paths.♻️ Proposed fix
-System endpoints — `/health`, `/ready`, `/metrics` — bypass the chain entirely. The `/` root route is registered via `WithHandler` and runs the full middleware chain like other application routes. +System endpoints — `/health`, `/ready`, `/metrics` — bypass the chain entirely. The built-in `/` route is injected by `configureRootHandler` and runs the full middleware chain like other application routes.🤖 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/contributor/api-server.md` around lines 73 - 75, The root-route description is attributing `/` to WithHandler, but that symbol only registers custom handlers while configureRootHandler injects the built-in root route and it is then wrapped by the middleware chain. Reword the docs in api-server.md to clearly distinguish the system endpoints (`/health`, `/ready`, `/metrics`) from the root route, and mention configureRootHandler as the source of `/` instead of WithHandler.
🤖 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.
Duplicate comments:
In `@docs/contributor/api-server.md`:
- Around line 73-75: The root-route description is attributing `/` to
WithHandler, but that symbol only registers custom handlers while
configureRootHandler injects the built-in root route and it is then wrapped by
the middleware chain. Reword the docs in api-server.md to clearly distinguish
the system endpoints (`/health`, `/ready`, `/metrics`) from the root route, and
mention configureRootHandler as the source of `/` instead of WithHandler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2fd1d69c-1bf3-4438-8030-10eb9558f97d
📒 Files selected for processing (19)
SECURITY.mddemos/bundle-attestation.mddemos/cuj2-demo.mddemos/end-to-end-cli.mddemos/provenance-demo-slides.htmldemos/provenance-demo.shdemos/provenance.mddocs/contributor/api-server.mddocs/contributor/maintaining.mddocs/contributor/tests.mddocs/design/007-recipe-evidence.mddocs/design/014-ocp-helm.mddocs/integrator/automation.mddocs/integrator/data-flow.mddocs/integrator/kubernetes-deployment.mddocs/integrator/supply-chain-verification.mddocs/user/api-reference.mddocs/user/artifact-verification.mdpkg/server/doc.go
💤 Files with no reviewable changes (1)
- demos/cuj2-demo.md
35818fd to
d522e3a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/integrator/data-flow.md (1)
208-211: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the matching criteria list aligned with the result diagram.
The new sentence again includes
platformandnode-countas matching dimensions, but theRecipeResultbox below still lists onlyservice,accelerator,intent, andos. As per coding guidelines, keep Markdown docs accurate and helpful.🤖 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/integrator/data-flow.md` around lines 208 - 211, The matching criteria text is out of sync with the RecipeResult diagram and currently lists extra dimensions not shown there. Update the markdown in the data-flow doc so the criteria sentence and the RecipeResult box use the same matching fields by keeping the list aligned with the diagram, and reference the RecipeResult section plus the criteria-dimensions description to ensure both stay consistent.Source: Coding guidelines
🤖 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/api-reference.md`:
- Around line 499-516: The `X-Bundle-Files` example in the API reference is
outdated and undercounts the revised bundle tree. Update the header/example in
the docs to reflect the current bundle layout shown in the `README`/bundle tree
description, including the correct total of 11 files and the root-level-only
checksums plus per-component `install.sh` placement, so the documented file list
matches the generated bundle structure.
---
Duplicate comments:
In `@docs/integrator/data-flow.md`:
- Around line 208-211: The matching criteria text is out of sync with the
RecipeResult diagram and currently lists extra dimensions not shown there.
Update the markdown in the data-flow doc so the criteria sentence and the
RecipeResult box use the same matching fields by keeping the list aligned with
the diagram, and reference the RecipeResult section plus the criteria-dimensions
description to ensure both stay consistent.
🪄 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: ee8fba55-c938-40ea-8f92-d5e2bc293a69
📒 Files selected for processing (19)
SECURITY.mddemos/bundle-attestation.mddemos/cuj2-demo.mddemos/end-to-end-cli.mddemos/provenance-demo-slides.htmldemos/provenance-demo.shdemos/provenance.mddocs/contributor/api-server.mddocs/contributor/maintaining.mddocs/contributor/tests.mddocs/design/007-recipe-evidence.mddocs/design/014-ocp-helm.mddocs/integrator/automation.mddocs/integrator/data-flow.mddocs/integrator/kubernetes-deployment.mddocs/integrator/supply-chain-verification.mddocs/user/api-reference.mddocs/user/artifact-verification.mdpkg/server/doc.go
💤 Files with no reviewable changes (1)
- demos/cuj2-demo.md
507203c to
29991f6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/integrator/data-flow.md (1)
202-213: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the query field list consistent.
The new
nodesprojection still isn't reflected in theRecipeResult.criteriabox below, so the section now describes two different field sets. Please update one side to match the other.🤖 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/integrator/data-flow.md` around lines 202 - 213, The query field list is inconsistent because the new nodes projection in the Fingerprint.ToCriteria description is not reflected in the RecipeResult.criteria box. Update the recipe criteria documentation so it matches the projected fields exactly, using the existing Fingerprint.ToCriteria and RecipeResult.criteria sections as the source of truth.
🤖 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/integrator/data-flow.md`:
- Around line 450-468: The markdown section in the template usage docs needs a
blank line before the fenced gotemplate block to satisfy MD031. Update the prose
around the README example so the opening fence is separated by an empty line,
and keep the surrounding `readmeTemplateData` / `deployTemplateData` wording
intact.
In `@docs/integrator/kubernetes-deployment.md`:
- Around line 515-521: The metrics port-forward example can block forever if
kubectl port-forward never succeeds or /metrics is unreachable; update the
snippet to add a bounded timeout around the wait loop and ensure the background
port-forward process is cleaned up on failure. Keep the fix in the kubectl
port-forward/curl example by modifying the wait/kill flow so it exits cleanly
and references the pf_pid handling.
---
Duplicate comments:
In `@docs/integrator/data-flow.md`:
- Around line 202-213: The query field list is inconsistent because the new
nodes projection in the Fingerprint.ToCriteria description is not reflected in
the RecipeResult.criteria box. Update the recipe criteria documentation so it
matches the projected fields exactly, using the existing Fingerprint.ToCriteria
and RecipeResult.criteria sections as the source of truth.
🪄 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: 49dc5ffa-7468-4a38-a465-caa16873911b
📒 Files selected for processing (19)
SECURITY.mddemos/bundle-attestation.mddemos/cuj2-demo.mddemos/end-to-end-cli.mddemos/provenance-demo-slides.htmldemos/provenance-demo.shdemos/provenance.mddocs/contributor/api-server.mddocs/contributor/maintaining.mddocs/contributor/tests.mddocs/design/007-recipe-evidence.mddocs/design/014-ocp-helm.mddocs/integrator/automation.mddocs/integrator/data-flow.mddocs/integrator/kubernetes-deployment.mddocs/integrator/supply-chain-verification.mddocs/user/api-reference.mddocs/user/artifact-verification.mdpkg/server/doc.go
💤 Files with no reviewable changes (1)
- demos/cuj2-demo.md
69e3ccc to
ba11c15
Compare
ba11c15 to
25592a1
Compare
0ef86cf to
a92aaf2
Compare
2552642 to
05a9c98
Compare
05a9c98 to
1a5482d
Compare
12ad086 to
77bc48c
Compare
…l and NVIDIA#1537 examples Post-NVIDIA#1528 documentation-correctness sweep of AICR supply-chain, evidence, provenance, and bundle docs (plus demos, scripts, slide decks), verified against the implementation. Doc/comment-only; the one Go change is a comment. Final review-round closure (round 12): - Ingest verification qualified as implemented-but-currently-failing-closed pending NVIDIA#1505 (the GP2 loader cannot parse the canonical allowlist) — it had been documented as fully operational (artifact guide, evidence.md, ADR-007, maintaining.md). - expected-signer regex example matches its own pointer (validate.yaml@refs/heads/main). - Exit 1/2 documented as structured exit values (both map to OS exit 2; read the JSON .exit via --format json). - community allowlist example includes the required issuer field. - Split signing: only the unsigned subject/predicate + bundle digest are reproducible; signature/cert/identity/time differ per signing host. - Rekor audit commands runnable (extract digest, strip the sha256: prefix); pointer enumeration excludes allowlist.yaml. - Port-forward snippet tracks readiness and preserves the failure exit status. - Checksum wording: inventories the deployment payload (recipe.yaml excluded, attestation files verified separately). - SECURITY supported-version bumped to 0.16.x (v0.16.0 is GA). Earlier rounds: pinned gh attestation verify to --repo/--signer-workflow/--source-ref; anchored cosign identity regexes; qualified SLSA build-level (NVIDIA#1536); reframed recipe evidence as a signer-bound record (NVIDIA#1535); propagated the NVIDIA#1549 checksums scope and NVIDIA#1550 attested/exit semantics; rewrote the data-flow bundler/deployer flow; validated the Policy Controller admission example on demo5 and demoted the Kyverno example (NVIDIA#1537). Signed-off-by: Yuan Chen <[email protected]> Round 13 (propagation closure): - artifact-verification heading no longer says "NVIDIA#1535 resolved" (ingest blocked by NVIDIA#1505). - Split-signing reproducibility corrected in the CLI reference and ADR-007 (only unsigned subject/predicate + OCI digest are stable; signature material varies). - Rekor audit runbook made runnable: quoted POINTER, digest extracted from it, UUID captured from rekor-cli search and passed to rekor-cli get. - Port-forward snippet captures the metrics body in the successful probe and exits nonzero when unreachable (no maskable second curl). - Exit-1 Review Process documents the structured exit: 1 value (OS exit code 2). Signed-off-by: Yuan Chen <[email protected]> Round 14: rekor-cli search --format json returns {"UUIDs":[...]}, so use jq -er .UUIDs[0] (not .[0]) when capturing the entry UUID. Signed-off-by: Yuan Chen <[email protected]>
77bc48c to
cc2b24c
Compare
Summary
A documentation-correctness sweep of AICR's supply-chain, evidence, provenance, and bundle docs (plus the demo guides, scripts, and slide decks), following the #1528 audit. Every change was verified against the implementation. ~34 files; doc/comment-only (the single Go change is a comment in
pkg/server/doc.go).Supply-chain & provenance verification
gh attestation verifypinned to--repo NVIDIA/aicr --signer-workflow …/on-tag.yaml(+--source-ref refs/tags/${TAG}) instead of--owneralone; all cosign--certificate-identity-regexpvalues anchored/escaped (incl.RELEASING.md).gh attestation verifyno longer shown emitting the SBOM (provenance only); image SBOM (signed OCI attestation) vs binary SBOM (separate, unsigned release asset) disambiguated.SECURITY.md,README.md,docs/README.md,installation.md,CONTRIBUTING.md,RELEASING.md, and the provenance deck.SECURITY.mdsupported-version table bumped to 0.16.x (v0.16.0 is GA).SECURITY.mdpinning policy aligned to ADR-006 (gate =bom-pinning-check -strict; exemption map belongs to image-digest pinning; admission verification is roadmap; CycloneDX BOM generated locally).Recipe evidence & maintainer docs
aicr validateresult — not proof the signer ran it or that a cluster physically existed (README,evidence.md,evidence-publishing.md, slides).signer.identity, so sign with a non-personal identity. Pointer paths use the nested<recipe>/<src>/<digest>.yamllayout with a valid full digest.Bundle attestation & trust levels
checksums.txt(recipe.yaml currently excluded; attestation files verified separately).attested= bundle attestation verified but binary attestation missing/external; a failed binary attestation reportsattestedbut exits nonzero; a failed bundle attestation →unknown. CI example isset -e-safe.trust updateoptional).Other
data-flow.mdStage-4/5 rewritten to the real single-DefaultBundler→ selected-deployer flow (5 deployers); Helm-specific layout labeled.SigstoreBundleexample was cluster-tested, does not verify AICR's bundle attestation, and is demoted to a pointer (tracked in Add validated admission-policy examples (Kyverno SigstoreBundle / Policy Controller v0.13+) #1537).rekor-cli --sha, crane-required, port-forward wait+cleanup,run_expect_failreturns nonzero on unexpected success, verify checksums from the bundle root,/ko-app/aicrentrypoint.Motivation / Context
Follow-up to #1528. Surfaced and hardened across multiple review passes.
Issue status
demos/images/*, RELEASING digest verify) tracked in the issue, not claimed fixed here.Type of Change
Component(s) Affected
docs/,demos/,README.md,SECURITY.md,CONTRIBUTING.md,RELEASING.md)pkg/server/doc.go)Testing
Full local
make qualifyis substituted by CI, which runs the identical gate (test-coverage, lint, e2e, scan, license-check) — all green on this head:tests / Test,tests / Lint,tests / E2E,tests / CLI E2E,tests / Security Scan,analyze. This is accepted as themake qualifyequivalent for a doc/comment-only change (the only Go edit is a comment;golangci-lint ./pkg/server/...= 0 issues).The Policy Controller admission example was additionally validated end-to-end on the
aicr-demo5GKE cluster (positive admit + wrong-identity reject).Risk Assessment
Checklist
git commit -S)