feat(evidence): add `aicr evidence publish` for off-network signing by njhensley · Pull Request #1140 · NVIDIA/aicr

njhensley · 2026-06-01T23:06:12Z

Summary

Add aicr evidence publish <bundle-dir> --push <ref>, which signs, pushes, and writes the pointer for an evidence bundle that aicr validate --emit-attestation (without --push) left on disk — splitting the one-shot path into a cluster-bound leg and a Sigstore-bound leg.

Motivation / Context

The one-shot validate --emit-attestation --push requires one host to reach both the target cluster and Fulcio/Rekor. In practice those live on different networks: validation needs the cluster (often behind a corporate VPN), while keyless signing needs public Sigstore egress (frequently blocked on that same VPN). This change lets the two legs run on different hosts/networks without changing the signed artifact.

Because the predicate — including its baked-in attestedAt timestamp — is read verbatim from the bundle's statement.intoto.json, the published result is byte-for-byte identical to the one-shot output regardless of which host ran which leg.

Fixes: #1130
Related: ADR-007 (docs/design/007-recipe-evidence.md)

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)
API server (cmd/aicrd, pkg/server)
Recipe engine / data (pkg/recipe)
Bundlers (pkg/bundler, pkg/component/*)
Collectors / snapshotter (pkg/collector, pkg/snapshotter)
Validator (pkg/validator)
Core libraries (pkg/errors, pkg/k8s)
Docs/examples (docs/, examples/)
Other: evidence (pkg/evidence/attestation, pkg/evidence/verifier)

Implementation Notes

New command aicr evidence publish and new attestation.Publish entry point. Publish loads a pre-built on-disk bundle, validates the --push ref up front (so a malformed ref doesn't waste a Fulcio cert + Rekor inclusion proof), signs the unsigned statement.intoto.json verbatim, pushes as an OCI artifact, attaches the Sigstore bundle as an OCI referrer, and writes pointer.yaml.
Shared sign+push pipeline. Refactored the sign+push leg out of EmitOptions into a signPushOptions struct so both Emit (in-memory bundle) and Publish (on-disk bundle) drive the identical pipeline without sharing build-only fields (Recipe/Snapshot/PhaseResults).
Shared OIDC resolution. oidcResolveOptionsFromFlags centralizes the --identity-token / COSIGN_IDENTITY_TOKEN / ambient-GHA / --oidc-device-flow precedence chain across validate --push and evidence publish. Token resolution stays deferred until adjacent to signing so Fulcio's nonce-binding window is respected.
Bundle discrimination via HasBundleMarkers (recipe.yaml + manifest.json co-occurrence), accepting either the parent dir or summary-bundle/ itself, mirroring the verifier's directory handling.

Testing

# Targeted gate on affected packages (race + lint + gofmt)
go test -race ./pkg/cli/... ./pkg/evidence/...      # all ok
golangci-lint run -c .golangci.yaml ./pkg/cli/... ./pkg/evidence/...   # 0 issues

Plus a full live end-to-end run against a real EKS GB200 (Ubuntu, Dynamo) cluster:

validate --emit-attestation ./out (no --push) — all phases passed (deployment / performance / conformance), unsigned bundle written.
evidence publish ./out --push ttl.sh/…:72h — pushed, Fulcio keyless-signed (Rekor log index recorded), Sigstore bundle attached as referrer, pointer.yaml populated.
evidence verify confirmed exit 0 across all three input modes (OCI digest ref, pointer file); the tag-only ref was correctly rejected.

Verified the cross-host content-addressability guarantee held: attestedAt from leg 1 was preserved verbatim in the signed output from leg 2.

Note: make qualify's cluster-bound e2e was exercised manually against live hardware rather than via the KWOK path; unit/lint gates run clean.

Risk Assessment

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

Rollout notes: Purely additive — new subcommand + new exported Publish. The existing one-shot validate --emit-attestation --push path is unchanged (the refactor is internal and output-preserving). No migration required.

Checklist

Tests pass locally (make test with -race)
Linter passes (make lint)
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-06-01T23:07:10Z

🌿 Preview your docs: https://nvidia-preview-evidence-decouple-publish.docs.buildwithfern.com/aicr

Decouple the cluster-bound validate step from the Fulcio/Rekor-bound signing step so they can run on different networks. `validate --emit-attestation` (without --push) now leaves an unsigned bundle on disk; `aicr evidence publish <bundle-dir> --push <ref>` then signs, pushes, and writes pointer.yaml from a host with Sigstore egress. The signed artifact is content-addressable: the predicate (including its baked-in attestedAt timestamp) is read verbatim from the bundle's statement.intoto.json, so the result is identical to the one-shot `validate --emit-attestation --push` output regardless of which host ran which leg. Refactor the sign+push pipeline out of EmitOptions into a shared signPushOptions so both Emit (in-memory bundle) and Publish (on-disk bundle) drive the identical path. OIDC token resolution is shared via oidcResolveOptionsFromFlags across validate --push and evidence publish.

coderabbitai · 2026-06-01T23:12:51Z

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: 81723435-d8e5-4ea3-ac6b-84d53dfe4eb5

📥 Commits

Reviewing files that changed from the base of the PR and between 13d0e50 and d971af7.

📒 Files selected for processing (4)

pkg/cli/evidence_publish.go
pkg/cli/evidence_publish_test.go
pkg/evidence/attestation/publish.go
pkg/evidence/attestation/publish_test.go

📝 Walkthrough

Walkthrough

This PR introduces a decoupled evidence publishing workflow. Users can now run aicr validate --emit-attestation without --push on restricted networks to generate unsigned bundles, then run the new aicr evidence publish <bundle-dir> --push <ref> command on networks with Sigstore egress to sign and publish. The implementation adds a Publish orchestration function that loads on-disk bundles, validates their structure and predicates, signs and pushes via the existing pipeline, and writes pointer.yaml. Common OIDC flag handling is extracted into a shared helper, and bundle-marker detection is centralized to reduce duplication.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

[Feature]: Decouple evidence signing/publishing from validate #1130 — Implements the requested decoupled sign+push feature by adding aicr evidence publish CLI and attestation.Publish(ctx, opts) orchestration to operate on on-disk bundles.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main feature addition: a new `aicr evidence publish` command enabling off-network signing workflows.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, covering motivation, implementation details, testing, and risk assessment for the new evidence publish functionality.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

coderabbitai

Actionable comments posted: 5

🤖 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/evidence_publish_test.go`:
- Around line 54-89: The test TestEvidencePublishCmd_RejectsInvalidInvocations
currently only asserts err != nil; update each subtest to assert the returned
error is the expected invalid-request error by using errors.Is(err,
ErrCodeInvalidRequest) (or the package-specific sentinel/typed error) after
calling root.Run, and optionally also assert a case-specific substring in
err.Error() to ensure the failure reason matches the test's `why`; keep
references to root.Run and ErrCodeInvalidRequest so reviewers can find and
change the assertions in the subtests.

In `@pkg/cli/evidence_publish.go`:
- Around line 72-90: Repeated string literals for CLI flag names ("plain-http",
"insecure-tls", "identity-token", "oidc-device-flow", etc.) are causing a
goconst lint failure; define constants (e.g., PlainHTTPFlag, InsecureTLSFlag,
IdentityTokenFlag, OIDCDeviceFlowFlag) near the top of the
pkg/cli/evidence_publish.go file and replace all literal usages in the flag
definitions and any reads/refs with those constants so the name is declared once
and reused everywhere.
- Around line 113-121: Replace the bare return of err from attestation.Publish
with errors.PropagateOrWrap so structured errors are preserved; specifically,
where attestation.Publish(...) currently returns err, return
errors.PropagateOrWrap(err, errors.ErrUnknown, "failed to publish attestation")
(or the appropriate project error code constant instead of errors.ErrUnknown) so
uncoded errors are wrapped and coded errors propagate unchanged.

In `@pkg/cli/evidence.go`:
- Around line 36-38: Update the parent help/description for the "evidence"
command group to remove the phrase implying it's "offline-only" since the group
now includes networked operations like publish; locate the evidence
command/group declaration (e.g., the evidence command variable or function that
sets its Use/Short/Long text such as evidenceCmd or NewEvidenceCmd) and change
the Short/Long/help strings to neutral wording that reflects both offline and
online subcommands (for example "Manage evidence-related commands: digest,
publish, verify") and make the same replacement for the related help string
referenced around the symbol that sets subcommand help on line ~43.

In `@pkg/evidence/attestation/publish.go`:
- Around line 205-210: The readBundlePredicate function currently uses
os.ReadFile to load StatementFilename which is unbounded; change it to open the
file with os.Open, wrap the file reader with io.LimitReader using a named size
constant from pkg/defaults (e.g., DefaultMaxStatementSize) to enforce a maximum
read, and read the limited data (handle EOF vs limit hits as an error); also
ensure the file is closed and propagate errors using the existing errors.Wrap
pattern.

🪄 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: f9475121-80b4-46ae-9ba2-4b92b14c255e

📥 Commits

Reviewing files that changed from the base of the PR and between cfb0cb0 and 5335ea0.

📒 Files selected for processing (12)

demos/evidence.md
docs/design/007-recipe-evidence.md
docs/user/cli-reference.md
pkg/cli/evidence.go
pkg/cli/evidence_publish.go
pkg/cli/evidence_publish_test.go
pkg/cli/validate_evidence.go
pkg/evidence/attestation/emit.go
pkg/evidence/attestation/emit_test.go
pkg/evidence/attestation/publish.go
pkg/evidence/attestation/publish_test.go
pkg/evidence/verifier/fetch.go

mchmarny

Clean architectural split — splits the one-shot validate --emit-attestation --push into a cluster-bound leg and a Sigstore-bound leg without changing the signed artifact, which is exactly the right shape for the corporate-VPN-vs-Sigstore-egress problem. The signPushOptions extraction (so Emit and Publish drive the same pipeline without leaking build-only fields), the oidcResolveOptionsFromFlags consolidation, the shared HasBundleMarkers, and the flag-constant extraction are all good DRY moves that the new command motivated rather than adding incidentally. Up-front push-ref validation (saving a wasted Fulcio cert + Rekor inclusion proof on a malformed ref) and the bounded statement read against MaxAttestationFileBytes match the project's established patterns. CI is green and the description documents a live EKS GB200 end-to-end run.

Two nits inline, both optional — neither blocks merge.

njhensley requested a review from a team as a code owner June 1, 2026 23:06

github-actions Bot added area/docs area/cli labels Jun 1, 2026

njhensley added the enhancement label Jun 1, 2026

github-actions Bot added the size/XL label Jun 1, 2026

njhensley force-pushed the evidence/decouple-publish branch from 5335ea0 to 3d687d2 Compare June 1, 2026 23:12

coderabbitai Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread pkg/cli/evidence_publish_test.go

Comment thread pkg/cli/evidence_publish.go Outdated

Comment thread pkg/cli/evidence_publish.go Outdated

Comment thread pkg/cli/evidence.go

Comment thread pkg/evidence/attestation/publish.go

fix: address coderabbit feedback

13d0e50

mchmarny previously approved these changes Jun 1, 2026

View reviewed changes

Comment thread pkg/evidence/attestation/publish.go Outdated

Comment thread pkg/evidence/attestation/publish.go Outdated

mchmarny assigned njhensley Jun 1, 2026

refactor: clean up error messages, Publish return signature

d971af7

njhensley dismissed mchmarny’s stale review via d971af7 June 1, 2026 23:52

mchmarny approved these changes Jun 2, 2026

View reviewed changes

mchmarny merged commit 1674ea4 into NVIDIA:main Jun 2, 2026
32 checks passed

njhensley deleted the evidence/decouple-publish branch June 2, 2026 14:54

This was referenced Jun 2, 2026

feat(bundle): private Sigstore via --fulcio-url and --rekor-url #1158

Merged

refactor: clarify evidence push tags, enhance push/pull semantics #1167

Closed

fix(evidence): pull by digest and auto-tag pushes instead of :v1 #1168

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(evidence): add `aicr evidence publish` for off-network signing#1140

feat(evidence): add `aicr evidence publish` for off-network signing#1140
mchmarny merged 3 commits into
NVIDIA:mainfrom
njhensley:evidence/decouple-publish

njhensley commented Jun 1, 2026

Uh oh!

github-actions Bot commented Jun 1, 2026

Uh oh!

coderabbitai Bot commented Jun 1, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

Possibly related issues

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mchmarny left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

njhensley commented Jun 1, 2026

Summary

Motivation / Context

Type of Change

Component(s) Affected

Implementation Notes

Testing

Risk Assessment

Checklist

Uh oh!

github-actions Bot commented Jun 1, 2026

Uh oh!

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

Uh oh!

Walkthrough

Estimated code review effort

Possibly related issues

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mchmarny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jun 1, 2026 •

edited

Loading