feat(server): non-interactive bundle attestation for /v1/bundle#1891
Open
lockwobr wants to merge 2 commits into
Open
feat(server): non-interactive bundle attestation for /v1/bundle#1891lockwobr wants to merge 2 commits into
lockwobr wants to merge 2 commits into
Conversation
lockwobr
added
the
theme/supply-chain
github-actions
Bot
added
area/ci
area/tests
area/docs
area/bundler
area/api
size/XL
labels
Contributor
Recipe evidence checkNo leaf overlays affected by this PR. This gate is warning-only and never blocks merge. |
Contributor
Coverage Report ✅
Coverage BadgeMerging this branch changes the coverage (2 decrease, 2 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
Contributor
|
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:
📝 WalkthroughWalkthroughAdds server-side bundle attestation through Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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/server/bundle_handler.go`:
- Around line 203-225: Add an authorization check before the attestation setup
in the attestRequested branch, requiring an authorized principal or approved
recipe/source policy before constructing the attester or signing the bundle.
Reject unauthorized requests with the existing error-response mechanism, and
preserve the current signing flow only for requests that pass this authorization
check.
🪄 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: b3245382-984d-4413-8951-497bb1cc31c6
📒 Files selected for processing (19)
.github/workflows/server-kms-e2e.yaml.gitignore.goreleaser.yamlapi/aicr/v1/server.yamldocs/contributor/api-server.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/client/v1/bundle.gopkg/client/v1/bundle_test.gopkg/defaults/timeouts.gopkg/server/bundle_handler.gopkg/server/bundle_handler_test.gopkg/server/serve.gopkg/server/signing.gopkg/server/signing_integration_test.gopkg/server/signing_test.gotests/chainsaw/signing/server-bundle-attestation-vault/README.mdtests/chainsaw/signing/server-bundle-attestation-vault/run.sh
lockwobr
force-pushed
the
feat/server-bundle-attestation
branch
from
July 23, 2026 16:12
8f45f19 to
78c7d7e
Compare
Contributor
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 `@pkg/server/serve.go`:
- Around line 111-114: Update the parseSigningConfig error path in serve to
return the existing err directly instead of wrapping it with errors.Wrap.
Preserve the structured error code and message produced by parseSigningConfig.
In `@pkg/server/signing.go`:
- Around line 66-80: Move the parseTLogUpload call out of the unconditional
start of parseSigningConfig and into the KMS signing branch guarded by the
existing hasKMS check. Preserve error propagation when KMS mode is enabled,
while ignoring malformed or stray AICR_TLOG_UPLOAD values when no signing mode
is configured or keyless mode is selected.
- Around line 221-274: Update loadBinaryAttestation to use
aicrerrors.PropagateOrWrap when handling errors returned by verify(ctx),
preserving existing structured error codes while defaulting uncoded failures to
ErrCodeInternal with the current context message.
🪄 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: 451ed38b-bd88-4855-bbb8-266fa12b404a
📒 Files selected for processing (19)
.github/workflows/server-kms-e2e.yaml.gitignore.goreleaser.yamlapi/aicr/v1/server.yamldocs/contributor/api-server.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/client/v1/bundle.gopkg/client/v1/bundle_test.gopkg/defaults/timeouts.gopkg/server/bundle_handler.gopkg/server/bundle_handler_test.gopkg/server/serve.gopkg/server/signing.gopkg/server/signing_integration_test.gopkg/server/signing_test.gotests/chainsaw/signing/server-bundle-attestation-vault/README.mdtests/chainsaw/signing/server-bundle-attestation-vault/run.sh
lockwobr
force-pushed
the
feat/server-bundle-attestation
branch
2 times, most recently
from
July 23, 2026 16:45
c621912 to
20a021c
Compare
Contributor
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 @.goreleaser.yaml:
- Around line 112-137: Extract the duplicated cosign attest-blob retry and
backoff loop from the aicr and aicrd post-build hooks into a shared script such
as hack/cosign-attest-blob.sh that accepts the bundle path and required signing
inputs. Update both hooks to invoke the shared script while preserving their
distinct bundle destinations and aicrd kodata staging behavior.
In `@pkg/server/signing.go`:
- Around line 227-262: Update defaultBinaryAttestationVerifier to read
attestPath once into a bounded buffer, then reuse that exact buffer for both
verifier.VerifyBinaryAttestation and the returned cached data. Remove the
separate os.Open/io.ReadAll path after verification while preserving the
existing size-limit and error handling behavior.
🪄 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: c24126c7-7fde-4ba2-ae7c-a548451ff56e
📒 Files selected for processing (20)
.github/workflows/server-kms-e2e.yaml.gitignore.goreleaser.yamlTHIRD_PARTY_NOTICES.mdapi/aicr/v1/server.yamldocs/contributor/api-server.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/client/v1/bundle.gopkg/client/v1/bundle_test.gopkg/defaults/timeouts.gopkg/server/bundle_handler.gopkg/server/bundle_handler_test.gopkg/server/serve.gopkg/server/signing.gopkg/server/signing_integration_test.gopkg/server/signing_test.gotests/chainsaw/signing/server-bundle-attestation-vault/README.mdtests/chainsaw/signing/server-bundle-attestation-vault/run.sh
25 tasks
lockwobr
force-pushed
the
feat/server-bundle-attestation
branch
from
July 23, 2026 17:10
20a021c to
9ff88b0
Compare
lockwobr
enabled auto-merge (squash)
July 23, 2026 17:31
mchmarny
requested changes
Jul 23, 2026
mchmarny
left a comment
mchmarny
left a comment
Member
There was a problem hiding this comment.
CHANGES REQUESTED — 2 merge blockers. Reviewed server signing, binary-provenance staging, handler and client wiring, and the KMS E2E at the exact head. Focused race tests, affected-package lint, shell and workflow validation all passed, and the current live E2E executed full mode; the two fail-open paths identified inline still need correction.
Server-side signing for POST /v1/bundle so a bundle can be returned signed via a non-interactive identity, closing the server gap in the supply-chain epic (#1149). - Server signs as itself from env config, never with request-supplied identity. Two modes, fail-fast validated at startup: KMS (AICR_SIGNING_KEY) and keyless against a private Sigstore (AICR_FULCIO_URL + token source). ?attest=true opts a request in. - Tool provenance: the aicrd binary attestation is produced by the release build (goreleaser cosign attest-blob) and shipped per-arch in the image via ko kodata; the server verifies it once at startup, caches it, and embeds it per bundle. Path resolves via AICR_BINARY_ATTESTATION_FILE, KO_DATA_PATH/GOARCH, or next-to-binary; identity pin overridable (org-pinned) via AICR_BINARY_ATTESTATION_IDENTITY_REGEXP. - Additive bundler seam (WithVerifiedBinaryAttestation) + client facade (BundleOptions.BinaryAttestation); CLI path byte-identical. - E2E: Go integration test (real OpenBAO KMS, verified live) + chainsaw server-signing suite + server-kms-e2e CI workflow. - OpenAPI + user/contributor docs updated. - Regenerate THIRD_PARTY_NOTICES.md, which was left stale on main by the direct-push go dep upgrade (091edf8) that skipped `make notices`; folded in here to unblock the notices-freshness gate. Fixes #1150
lockwobr
force-pushed
the
feat/server-bundle-attestation
branch
from
July 23, 2026 22:26
8592c7d to
5e0d8a9
Compare
mchmarny
approved these changes
Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /v1/bundle?attest=truenow returns a signed bundle using a non-interactive, server-owned identity (KMS or private-Sigstore keyless), with tool provenance from the aicrd binary attestation shipped in the image. The server never signs with request-supplied identity.Motivation / Context
The
/v1/bundleendpoint never attested or signed bundles: the handler builtBundleOptionswith noAttester, so a CI pipeline or platform bundling via the API could not get a signed artifact back. This closes the server-side gap in the supply-chain epic. The CLI's default signing path is interactive browser OIDC, a non-starter for a daemon, so this adds non-interactive server signing.Fixes: #1150
Related: #1149
Type of Change
Component(s) Affected
cmd/aicrd,pkg/server)pkg/bundler,pkg/component/*)pkg/errors,pkg/k8s) —pkg/client/v1,pkg/defaultsdocs/,examples/).goreleaser.yaml, newserver-kms-e2eworkflow, chainsaw suiteImplementation Notes
Trust boundary: the server signs as itself. Signing identity comes entirely from server env config, never from the request;
?attest=trueis only a boolean trigger. Request-supplied / on-behalf-of-caller OIDC is explicitly out of scope.Two signing modes (mutually exclusive, fail-fast validated at startup):
AICR_SIGNING_KEY(awskms:///gcpkms:///azurekms:///hashivault://).AICR_FULCIO_URL+ a token source (AICR_IDENTITY_TOKEN_FILE, read fresh per request, or GHA ambient). Shared:AICR_REKOR_URL,AICR_TLOG_UPLOAD(KMS-only),AICR_SIGNING_CONFIG_PATH.Tool provenance (the aicrd binary attestation): the release build (
.goreleaser.yaml) now runscosign attest-blobon the aicrd binary and ships the result per-architecture in the ko image viaKO_DATA_PATH. The server verifies its own attestation once at startup, caches it, and embeds it in each signed bundle. It fails to start if signing is enabled but the attestation is missing or invalid. The attestation path resolves viaAICR_BINARY_ATTESTATION_FILE, thenKO_DATA_PATH/GOARCH, then next-to-binary; the identity pin is overridable (still org-pinned) viaAICR_BINARY_ATTESTATION_IDENTITY_REGEXP, mirroring the CLI's--certificate-identity-regexp.Additive, CLI-safe: the bundler gains
WithVerifiedBinaryAttestationand the client facade gainsBundleOptions.BinaryAttestation. With neither set (the CLI path), behavior is byte-identical: no attest is inferred fromAttester != nil, and the discover-and-verify-per-run path is unchanged.Testing
make testgreen on the changed packages;make lint0 issues;golangci-lint(incl.-tags integration) 0 issues.//go:build integration, skips withoutVAULT_ADDR) verified live against a real OpenBAO Transit KMS: bundle returned signed,bundleAttested=true.tests/chainsaw/signing/server-bundle-attestation-vault/smoke mode passed live (server rejectsattest=truewhen unconfigured with 400; unsigned bundle still returned). The full attested path runs in the newserver-kms-e2e.yamlworkflow (needs GitHub OIDC to attest the binary), the same CI-only limitation as the existing signing e2es.make test, and the chainsaw suite has nochainsaw-test.yaml, so neither is auto-run bymake test/make e2e/make qualify; they do not affect local dev.Note:
tests/releasepolicyfails on local macOS due to a pre-existing environment gap (GNUtimeoutabsent inpublish-homebrew.sh), unrelated to this change; it fails identically onmainand passes in CI. Tracked for a separate portability fix.Risk Assessment
Rollout notes: Server signing is disabled by default (no env config → capability off, current behavior preserved). Turning it on in a real deployment depends on the aicrd binary attestation being present in the image, which this PR wires into the release build. No migration.
Checklist
make testwith-race) — modulo the pre-existing environmentalreleasepolicygap noted abovemake lint)git commit -S)