docs: end-user artifact verification guide#1357
Conversation
|
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 (3)
📝 WalkthroughWalkthroughA new end-user documentation guide Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 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 |
mchmarny
left a comment
There was a problem hiding this comment.
Welcome to AICR, @ld-singh — this is exactly the consolidation the verification docs needed. The trust level table with explicit ordering and the "max auto-detects" explanation are particularly useful; that model is non-obvious from the CLI help alone.
All four anchor links (#aicr-verify, #aicr-evidence-verify, #aicr-trust-update, #aicr-recipe-verify-catalog) check out against cli-reference.md. The offline/air-gapped section correctly reflects the two unsupported paths and links the tracking issues. One inline concern on the JSON-for-CI case statement — the pipeline form is fragile under set -o pipefail which is common in CI; see the comment.
Three mechanics items before merge:
- CI hasn't run. This is a fork PR, so only labeling workflows fired — the test/lint/lychee anchor-check suite needs a maintainer to approve the workflow run.
- Branch is behind
main. Rebase + squash + sign needed. - Missing
theme/*label.theme/supply-chainfits here.
ArangoGutierrez
left a comment
There was a problem hiding this comment.
Welcome, @ld-singh — really nice first contribution. The trust-level table and the honest air-gapped / private-Sigstore limitations make this genuinely useful, and the verification flows are correct. I checked the links (CI has not run yet): the three tracking issues (#1154, #1149, #1153) and all four cli-reference.md anchors resolve.
No objection to merging as-is and handling the set -o pipefail nit from @mchmarny's comment in a follow-up — that example fails closed (it aborts rather than passing an invalid bundle through), so deferring it is safe. Tracked in #1364 so it does not get lost.
Two notes, neither blocking the content: this is an advisory approval — docs/** is owned by aicr-write, so it does not satisfy the code-owner gate (an aicr-write member still needs to approve). And the fork CI (test / lint / lychee / docs build) should run green before merge, since lychee is the only anchor-link net.
|
Thanks for this, @ld-singh! Before it can merge, the commit-signing gate needs all commits to be cryptographically signed (
(The merge commit AICR uses cryptographic signing to satisfy DCO — the project doesn't use 1. Set up signing (one-time, skip if already configured)SSH signing is the simplest path if you already push over SSH: git config --global gpg.format ssh
git config --global user.signingkey ~/.ssh/id_ed25519.pub # your public key
git config --global commit.gpgsign trueThen add that same key as a Signing key under GitHub → Settings → SSH and GPG keys (a signing key is a separate entry from an auth key, even if it's the same key). Prefer GPG?gpg --quick-gen-key "Your Name <your@email>" # only if you don't have a key yet
git config --global user.signingkey <KEY_ID>
git config --global commit.gpgsign true
gpg --armor --export <KEY_ID> # paste into GitHub → GPG keys2. Re-sign the commit and pushYour branch has a merge commit sitting on top of the unsigned one, so the cleanest fix is to rebase onto git fetch origin
git rebase --exec 'git commit --amend --no-edit -S' origin/main
git push --force-with-leaseIf you'd rather sign just the single commit interactively: git rebase -i origin/main # mark f76a5e2c as 'edit'
git commit --amend --no-edit -S
git rebase --continue
git push --force-with-lease3. Verify before pushinggit log --show-signature -1You should see a Happy to help if you hit any snags with the signing setup. |
8def53c to
24f29df
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/user/artifact-verification.md (1)
208-214:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix shell pipeline pattern that breaks with
set -o pipefail(duplicate of past feedback).The command substitution using a pipeline with
aicr evidence verifywill fail in scripts withset -o pipefailenabled (common in CI). When the command exits non-zero, the pipeline status propagates and aborts the script before thecasestatement runs. Write to a file first and read from it instead:-case "$(aicr evidence verify recipes/evidence/<recipe>.yaml --format json | jq '.exit')" in +aicr evidence verify recipes/evidence/<recipe>.yaml --format json -o result.json || true +case "$(jq '.exit' result.json)" inThe
|| trueabsorbs the non-zero exit so the script continues, andjqreads from the file rather than a potentially-disrupted pipe.🤖 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/artifact-verification.md` around lines 208 - 214, The shell pipeline in the command substitution using `aicr evidence verify recipes/evidence/<recipe>.yaml --format json | jq '.exit'` will break when `set -o pipefail` is enabled because a non-zero exit from the command propagates through the pipeline and aborts the script before the case statement executes. Fix this by writing the output of `aicr evidence verify` to a temporary file with `|| true` appended to absorb non-zero exit codes, then read from that file with `jq '.exit'` instead of relying on the pipe. This ensures the script continues execution regardless of the aicr command's exit status.
🤖 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/user/artifact-verification.md`:
- Around line 208-214: The shell pipeline in the command substitution using
`aicr evidence verify recipes/evidence/<recipe>.yaml --format json | jq '.exit'`
will break when `set -o pipefail` is enabled because a non-zero exit from the
command propagates through the pipeline and aborts the script before the case
statement executes. Fix this by writing the output of `aicr evidence verify` to
a temporary file with `|| true` appended to absorb non-zero exit codes, then
read from that file with `jq '.exit'` instead of relying on the pipe. This
ensures the script continues execution regardless of the aicr command's exit
status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: bcb7f159-c9c4-43a2-99d1-4660a028cdf0
📒 Files selected for processing (3)
docs/index.ymldocs/user/artifact-verification.mddocs/user/index.md
|
Thanks for the thorough review and the warm welcome, @mchmarny @ArangoGutierrez. Much appreciated for a first contribution! I have re-signed the commit and force-pushed: the branch is now a single commit ( |
24f29df to
fb07ef0
Compare
|
🌿 Preview your docs: https://nvidia-preview-docs-artifact-verification.docs.buildwithfern.com/aicr |
Summary
Add a consumer-facing guide,
docs/user/artifact-verification.md, covering how to verifyaicrartifacts (deployment bundles and recipe-evidence bundles) across public-trust, KMS-key/PEM, and offline deployment shapes.Motivation / Context
The V1 acceptance bar calls for a documented verification path that works in air-gapped, private, and public-trust deployments. Verification knowledge was scattered across
aicr verifyhelp and signing-side docs; this consolidates it into one end-user guide with copy-pasteable commands.Fixes: #1157
Related: #1149, #1153, #1154
Type of Change
Component(s) Affected
docs/,examples/)Implementation Notes
docs/user/artifact-verification.md: what-can-be-verified table, trust levels, public-trust verification, min-trust enforcement, KMS-key and local-PEM verification, offline/air-gapped considerations, recipe-evidence verification, JSON-for-CI gating, and troubleshooting.docs/index.yml) and the user-section index (docs/user/index.md)..claude/skills/creating-slide-decks/skeleton.htmlgains the standard license header (added automatically bymake license; required for a clean gate).Testing
make qualifypasses: unit tests (race + coverage), lint (Go/YAML/license/agents-sync/docs-filenames/docs-mdx/bom-pinning), 22/22 e2e chainsaw tests, and vulnerability scan. Doc anchors verified againstcli-reference.mdand internal headings.Risk Assessment
Rollout notes: N/A
Checklist
make testwith-race)make lint)git commit -S)