Emit certificate annotations and fix dedupe for keyless signatures - #234
Merged
Merged
Conversation
container/signer only wrote the plain signature annotation when attaching a cosign signature manifest, so a certificate-bearing (keyless) bundle would silently be misclassified as key-signed by container/verifier, which treats a missing certificate annotation as the key-pair layout. Dedupe also verified against the currently-used public key, which never matches for keyless signing, since each run mints a fresh ephemeral keypair and certificate — so re-signing would append an unbounded number of duplicate layers. attachCosignSignature now takes the full Sigstore bundle and, when its verification material carries a certificate, writes the certificate and transparency-log annotations container/verifier's read side expects, and dedupes by signer identity (certificate SAN + OIDC issuer) instead of key material. The key-signed path is unchanged. This is groundwork for keyless (Fulcio) signing; that flow itself is not implemented here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A certificate-bearing signature with no transparency-log entry (a Fulcio deployment run without Rekor) previously failed to round-trip through RetrieveBundles: the read side unconditionally parsed the absent bundle annotation and discarded the layer. rekorBundleAnnotation also silently produced a broken annotation for a proof-only (Rekor v2) tlog entry, since the classic cosign bundle layout only carries a v1 inclusion promise. And identity-based dedupe trusted an existing layer's certificate match without verifying its signature, letting a corrupted or mismatched layer permanently block a valid re-sign.
rdimitrov
approved these changes
Aug 13, 2026
This was referenced Aug 13, 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
container/signer's attach logic (added in #230) only wrote the plaindev.cosignproject.cosign/signatureannotation, regardless of what kindof Sigstore bundle was being attached.
container/verifier/sigstore.goclassifies a signature layer as key-signed whenever the certificate
annotation is absent — so a certificate-bearing (keyless/Fulcio) bundle
attached through the existing code path would silently be misclassified,
and keyless verification would fail. Dedupe was also broken for that case:
signedByKeyverifies against the public key currently being used tosign, but keyless signing mints a fresh ephemeral keypair (and
certificate) on every run, so it would never dedupe and would append an
unbounded number of duplicate layers on repeated pushes.
This PR fixes both, ahead of a follow-up PR that adds the actual
keyless/Fulcio signing flow on top:
attachCosignSignaturenow takes the full Sigstore bundle (*protobundle.Bundle)instead of a raw signature slice. When the bundle's verification material
carries a certificate, it also writes the
dev.sigstore.cosign/certificate(PEM leaf cert) and
dev.sigstore.cosign/bundle(Rekor transparency-logentry) annotations, matching exactly what
container/verifier/sigstore.goreads back on retrieval.
(certificate SAN + OIDC issuer) against already-attached layers, instead
of key material. The key-signed path (
signedByKey) is unchanged.counts as "already present," not append-vs-replace.
No changes to actual Fulcio/Rekor signing — this PR is attach/dedupe logic
only, working with whatever bundle shape is handed to it.
Type of change
Test plan
go test -race ./container/signer/... ./container/verifier/...passestask lintandtask testpass for the full repotask license-checkpassesNew tests cover:
attachCosignSignatureround-trips through
verifier.RetrieveBundlesand is classified askeyless (
HasCertificate() == true), not key-signed — the regressiontest for the misclassification bug.
entry; the
dev.sigstore.cosign/bundleannotation is written only whena tlog entry is present, and its JSON shape matches what
container/verifier/sigstore.goreads back.issuer) across a fresh ephemeral certificate each "run" collapses to one
layer; a different SAN, or the same SAN under a different OIDC issuer,
appends a new layer instead; the key-signed path's dedupe behavior is
unchanged.
Does this introduce a user-facing change?
No —
container/signer's public API (Signer,Options,Result,SignOCI) is unchanged. This only affects internal attach/dedupe logic,in preparation for a future keyless-signing feature that isn't wired up
yet.
Generated with Claude Code