Skip to content

Improve signature and attestation detail output for sigstore bundles #3162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
simonbaird wants to merge 2 commits into
base: main
Choose a base branch
from
+146 −23
Draft

Improve signature and attestation detail output for sigstore bundles #3162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eab2fac
Include some v3 bundle signature detail in output
simonbaird Mar 2, 2026
8e59a62
Filter v3 attestations and sigs in output
simonbaird Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 1 addition & 15 deletions features/__snapshots__/task_validate_image.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,7 @@ true
"signatures": [
{
"keyid": "",
"sig": ""
},
{
"keyid": "",
"sig": ""
"sig": "MEUCIBZc+dmgTn8SCx30h9yvCOjsBwj1+aZX0gW53c7TeyuSAiEAp4zWGNHMrjql9NFl/fCmFXnJkgDkOqbN5n7H7mw6aqI="
Copy link
Member Author

@simonbaird simonbaird Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See line 366 in this file for the equivalent output for v2 style sigs.

}
],
"attestations": [
Expand All @@ -268,16 +264,6 @@ true
"sig": "MEUCIQC5bGm4zzbExXBMrZCmqZ98iqUhi8TV/maq/8dJ/c3POAIgCNw+RkeO7PAkT6JDWIvISZ2AjILu9YuPQ0qqfNwCqug="
}
]
},
{
"type": "https://in-toto.io/Statement/v0.1",
"predicateType": "https://sigstore.dev/cosign/sign/v1",
"signatures": [
{
"keyid": "",
"sig": "MEUCID1cJkxyk1oGvXcoAVkDST9A1vfX2gxPEz+LUzN10nDmAiEAxh9rp79yr4fZmAWWOit0dZ5QWK+uYIU8fQVb0/rLIyM="
}
]
}
]
}
Expand Down
153 changes: 145 additions & 8 deletions internal/evaluation_target/application_snapshot_image/application_snapshot_image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,19 @@ func (a *ApplicationSnapshotImage) FetchImageFiles(ctx context.Context) error {
func (a *ApplicationSnapshotImage) ValidateImageSignature(ctx context.Context) error {
opts := a.checkOpts
client := oci.NewClient(ctx)
useBundles := a.hasBundles(ctx)

var sigs []cosignOCI.Signature
var err error

if a.hasBundles(ctx) {
if useBundles {
// For v3 bundles, both image signatures and attestations are stored as
// "attestations" in the unified bundle format. So we use VerifyImageAttestations
// to extract image signatures from the bundle, even though it seems unintuitive.
// This is different from v2 where image signatures and attestations were separate.
//
// The certificate extraction requires different handling for bundles and
// should be addressed in future work to achieve full v2 parity.
opts.NewBundleFormat = true
opts.ClaimVerifier = cosign.IntotoSubjectClaimVerifier
sigs, _, err = client.VerifyImageAttestations(a.reference, &opts)
Expand All @@ -174,11 +182,36 @@ func (a *ApplicationSnapshotImage) ValidateImageSignature(ctx context.Context) e
}

for _, s := range sigs {
es, err := signature.NewEntitySignature(s)
if err != nil {
return err
if useBundles {
// This will appears in the output under "signatures" so filter out
// the sigs that are provenance attestations leaving only the sigs
// that are image signatures. Note: This does seems confusing and
// I'm not 100% sure we're doing the right thing here. Maybe revisit
// once we have a better idea about sigstore bundles and how they're
// handled by cosign itself.
if !isImageSignatureAttestation(s) {
log.Debugf("Skipping non-image signature attestation")
continue
}
Comment on lines +192 to +195
Copy link
Contributor

@qodo-code-review qodo-code-review bot Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Bundle signature filtering broken 🐞 Bug ✓ Correctness

isImageSignatureAttestation() looks for predicateType directly in sig.Payload(), but for
bundles sig.Payload() is a DSSE envelope and predicateType exists only in the base64-decoded
embedded statement. This likely results in skipping all bundle image signatures (empty
image.signatures) and incorrectly including cosign/sign/v1 entries in attestations.
Agent Prompt 
### Issue description
`isImageSignatureAttestation()` currently checks for `predicateType` by JSON-unmarshalling the raw bytes returned from `sig.Payload()`. For bundle entries, `sig.Payload()` is a DSSE envelope JSON containing `payloadType` and a base64-encoded `payload` (the embedded in-toto statement). The DSSE envelope itself does **not** contain `predicateType`, so the check misclassifies bundle entries and breaks signature/attestation filtering.

### Issue Context
- Bundle parsing already assumes `sig.Payload()` is DSSE JSON and uses `attestation.ProvenanceFromBundlePayload()` which base64-decodes the embedded statement.
- Filtering should follow the same model: decode the DSSE envelope, base64-decode its `payload`, then read `predicateType` from the embedded statement.

### Fix Focus Areas
- internal/evaluation_target/application_snapshot_image/application_snapshot_image.go[533-568]
- internal/evaluation_target/application_snapshot_image/application_snapshot_image.go[184-206]
- internal/evaluation_target/application_snapshot_image/application_snapshot_image.go[289-304]
- internal/evaluation_target/application_snapshot_image/application_snapshot_image_test.go[1046-1078]

### Suggested approach
1. Replace `extractPredicateType(payload []byte)` usage for bundle filtering with logic that:
   - Unmarshals `sig.Payload()` into `cosign.AttestationPayload` (or DSSE envelope struct).
   - Base64-decodes `payload.PayLoad`.
   - Extracts `predicateType` from the decoded embedded JSON (your existing `extractPredicateType` can be used *on the decoded payload bytes*).
2. If the DSSE unmarshal/base64 decode fails, return false and emit a debug log that indicates the payload isn’t a DSSE envelope / can’t be decoded.
3. Add a unit test ensuring `isImageSignatureAttestation()` returns true for a bundle DSSE signature whose embedded statement’s predicateType is `https://sigstore.dev/cosign/sign/v1` (you can reuse `createBundleDSSESignature`).
4. Optionally add a higher-level test to ensure ValidateImageSignature(bundle path) populates `a.signatures` (may require controllable bundle detection).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Member Author

@simonbaird simonbaird Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is an actual problem, or just confusion about the multiple different types that are now called a "sig" in cosign. I'm certainly confused about it.

Copy link
Contributor

@qodo-code-review qodo-code-review bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ask❓

I'm not sure if this is an actual problem, or just confusion about the multiple different types that are now called a sig in cosign. I'm certainly confused about it.

Answer:

This is very likely an actual correctness problem (not just naming/confusion), and it stems from the data shape you’re inspecting rather than what cosign happens to call “signatures”.

In your new bundle path you call:

  • VerifyImageAttestations(...) and get back a slice of cosignOCI.Signature (sigs / layers).
  • For bundle entries, sig.Payload() is (typically) the DSSE envelope JSON, not the in-toto statement JSON.
  • predicateType is a field on the in-toto statement (the DSSE envelope’s base64-decoded "payload"), not on the DSSE envelope itself.

But your filter does:

payload, _ := sig.Payload()
return hasPredicateType(payload, cosignSignPredicateType)

and hasPredicateType does a JSON unmarshal looking for a top-level "predicateType". If payload is the DSSE envelope, that unmarshal will succeed but PredicateType will be empty, so isImageSignatureAttestation will return false for everything.

Practical consequences:

  1. In ValidateImageSignature (bundle path), you will likely skip all entries:
    • if !isImageSignatureAttestation(s) { continue }
    • Result: a.signatures ends up empty even when the image is signed via v3 bundle.
  2. In parseAttestationsFromBundles, the inverse filter will likely keep everything:
    • if isImageSignatureAttestation(sig) { continue } never triggers
    • Result: cosign-sign entries may incorrectly land in a.attestations.

So the bot’s concern is not just about terminology—it matches how DSSE/in-toto is structured.

What to do instead (high level):

  • Detect/parse DSSE envelope (e.g., cosign.AttestationPayload or a minimal DSSE struct with PayloadType + Payload fields).
  • Base64-decode the envelope payload.
  • Run your existing extractPredicateType(...) on the decoded bytes, not on the envelope bytes.

One additional nuance: cosign may return some non-DSSE payloads depending on API/version, so you probably want:

  • “Try DSSE parse; if it works, use decoded payload; else fall back to current direct predicateType check”
  • That makes the helper robust across bundle/non-bundle and any oddities.

Given the current code and comments (“not 100% sure we’re doing the right thing”), I’d treat this as a real bug worth fixing + adding a unit test around isImageSignatureAttestation for a DSSE envelope containing a statement with predicateType == https://sigstore.dev/cosign/sign/v1.


// For bundle image signatures produced by cosign v3, the old
// method of accessing the signatures doesn't work. Instead we have
// to extract them from the bundle. And the bundle actually has
// signatures for both the image itself and the attestation.
signatures, err := extractSignaturesFromBundle(s)
if err != nil {
return fmt.Errorf("cannot extract signatures from bundle: %w", err)
}
a.signatures = append(a.signatures, signatures...)
} else {
// For older non-bundle image signatures produced by cosign v2.
// Note that filtering isn't needed, since we have only image sigs here.
es, err := signature.NewEntitySignature(s)
if err != nil {
return err
}
a.signatures = append(a.signatures, es)
}
a.signatures = append(a.signatures, es)
}
return nil
}
Expand All @@ -199,6 +232,11 @@ func (a *ApplicationSnapshotImage) ValidateAttestationSignature(ctx context.Cont
return err
}

// Todo:
// - For the non-bundle code path we actually check the syntax.
// We should do that for bundles as well probably.
// - Doing an early return like thi shere seems untidy, refactor
// maybe?
if useBundles {
return a.parseAttestationsFromBundles(layers)
}
Expand Down Expand Up @@ -251,8 +289,19 @@ func (a *ApplicationSnapshotImage) ValidateAttestationSignature(ctx context.Cont
// parseAttestationsFromBundles extracts attestations from Sigstore bundles.
// Bundle-wrapped layers report an incorrect media type, so we unmarshal the
// DSSE envelope from the raw payload directly.
//
// Note: For v3 bundles, this function filters out image signature attestations
// (https://sigstore.dev/cosign/sign/v1) since those are handled in ValidateImageSignature.
// Only provenance and other attestations are added to the attestations array.
func (a *ApplicationSnapshotImage) parseAttestationsFromBundles(layers []cosignOCI.Signature) error {
for _, sig := range layers {
// For v3 bundles, filter out image signature attestations - those are handled
// in ValidateImageSignature. Only add provenance attestations here.
if isImageSignatureAttestation(sig) {
log.Debugf("Skipping image signature attestation - handled in ValidateImageSignature")
continue
}

payload, err := sig.Payload()
if err != nil {
log.Debugf("Skipping bundle entry: cannot read payload: %v", err)
Expand All @@ -275,8 +324,8 @@ func (a *ApplicationSnapshotImage) parseAttestationsFromBundles(layers []cosignO
if err != nil {
return fmt.Errorf("unable to parse bundle attestation: %w", err)
}
t := att.PredicateType()
log.Debugf("Found bundle attestation with predicateType: %s", t)
log.Debugf("Found bundle attestation with predicateType: %s", att.PredicateType())

a.attestations = append(a.attestations, att)
}
return nil
Expand Down Expand Up @@ -461,7 +510,7 @@ func (a *ApplicationSnapshotImage) WriteInputFile(ctx context.Context) (string,
log.Debugf("Created dir %s", inputDir)
inputJSONPath := path.Join(inputDir, "input.json")

f, err := fs.OpenFile(inputJSONPath, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0644)
f, err := fs.OpenFile(inputJSONPath, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0o644)
if err != nil {
log.Debugf("Problem creating file in %s", inputDir)
return "", nil, err
Expand All @@ -480,3 +529,91 @@ func (a *ApplicationSnapshotImage) WriteInputFile(ctx context.Context) (string,
log.Debugf("Done preparing input file:\n%s", inputJSONPath)
return inputJSONPath, inputJSON, nil
}

// extractPredicateType extracts the predicateType field from a JSON
// payload lazily, i.e. without unmarshalling all the other fields
func extractPredicateType(payload []byte) (string, error) {
var attestation struct {
PredicateType string `json:"predicateType"`
}
if err := json.Unmarshal(payload, &attestation); err != nil {
return "", err
}
return attestation.PredicateType, nil
}

// hasPredicateType checks if a JSON payload has the specified predicate type.
func hasPredicateType(payload []byte, expectedType string) bool {
predicateType, err := extractPredicateType(payload)
if err != nil {
log.Debugf("Cannot parse JSON payload: %v", err)
return false
}
return predicateType == expectedType
}

const cosignSignPredicateType = "https://sigstore.dev/cosign/sign/v1"

// isImageSignatureAttestation checks if a signature from a bundle represents
// an image signature attestation (vs. a provenance attestation).
func isImageSignatureAttestation(sig cosignOCI.Signature) bool {
payload, err := sig.Payload()
if err != nil {
log.Debugf("Cannot read signature payload: %v", err)
return false
}

// Image signature attestations use the cosign/sign predicate type
return hasPredicateType(payload, cosignSignPredicateType)
}

// extractSignaturesFromBundle extracts signature information from a bundle
// image signature attestation, using the same pattern as createEntitySignatures.
//
// TODO: This currently only extracts the signature value from the DSSE envelope.
// Certificate information (keyid, certificate, chain, metadata) is not being
// extracted because it requires different handling for v3 bundles compared to v2.
// Future work should investigate how to access certificate data from bundle
// attestations to achieve full parity with v2 signature output. Also, there might
// be some cosign methods we can use instead of doing it ourselves here.
func extractSignaturesFromBundle(sig cosignOCI.Signature) ([]signature.EntitySignature, error) {
reader, err := sig.Uncompressed()
if err != nil {
return nil, fmt.Errorf("cannot read signature data: %w", err)
}
defer reader.Close()

var attestationPayload cosign.AttestationPayload
if err := json.NewDecoder(reader).Decode(&attestationPayload); err != nil {
return nil, fmt.Errorf("cannot parse DSSE envelope: %w", err)
}

// Create the base EntitySignature from the oci.Signature (for certificate info)
es, err := signature.NewEntitySignature(sig)
if err != nil {
return nil, fmt.Errorf("cannot create base signature: %w", err)
}

var results []signature.EntitySignature
for _, s := range attestationPayload.Signatures {
esNew := es
// The Signature and KeyID can come from two locations, the oci.Signature or
// the cosign.Signature. Prioritize information from oci.Signature, but fallback
// to cosign.Signature when needed (same pattern as createEntitySignatures)
//
// Todo: Actually the above comment might be stale and/or wrong since I belive
// it was copied from similar code in internal/attestation/attestation.go. Let's
// review this later. As far as I can tell this code always produces an empty
// string for the KeyId.
//
if esNew.Signature == "" {
esNew.Signature = s.Sig
}
if esNew.KeyID == "" {
esNew.KeyID = s.KeyID
}
results = append(results, esNew)
}

return results, nil
}