From 7f6638b1074c920214b011354c4479fdaec792cf Mon Sep 17 00:00:00 2001 From: Anitha Natarajan Date: Fri, 24 Jul 2026 13:24:43 +0530 Subject: [PATCH] fix(storage): forward RekorEntry to signature bundle in sigstore-bundle mode In the OCI sigstore-bundle encoding path, the signature bundle was stored without a transparency log entry (tlogEntries=[]) even when Rekor upload had succeeded. Root cause: legacy.go uploadSignature constructed the signing.Bundle for the StoreRequest without setting RekorEntry, so storeWithSigstoreBundle always received a nil RekorEntry and makeSigBundleBytes passed nil to cbundle.MakeProtobufBundle, producing a bundle with no tlogEntries. The attestation path was unaffected: storeWithProtobufBundle in attestation.go already read req.Bundle.RekorEntry directly from the StorageOpts-populated field added in #1691. Fix: set RekorEntry: storageOpts.RekorEntry in the signing.Bundle constructed by uploadSignature. The field is nil-safe: MakeProtobufBundle skips tlog embedding when rekorEntry is nil, so behaviour is unchanged when transparency is disabled or Rekor upload failed. Also correct two log messages in attestation.go that referred to "protobuf bundle format" instead of the user-facing term "sigstore bundle format" used elsewhere. Add TestMakeSigBundleBytes_TlogEntries to guard the nil-rekorEntry path and document the expected behaviour. Signed-off-by: Anitha Natarajan Assisted-by: Claude Sonnet 4.6 (via GitHub Copilot) Signed-off-by: Anitha Natarajan --- pkg/chains/storage/oci/attestation.go | 4 ++-- pkg/chains/storage/oci/legacy.go | 11 ++++++----- pkg/chains/storage/oci/simple_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/pkg/chains/storage/oci/attestation.go b/pkg/chains/storage/oci/attestation.go index ee27627b6e..2bcb282f38 100644 --- a/pkg/chains/storage/oci/attestation.go +++ b/pkg/chains/storage/oci/attestation.go @@ -152,7 +152,7 @@ func (s *AttestationStorer) storeLegacy(ctx context.Context, req *api.StoreReque // format over the OCI 1.1 Referrers API. func (s *AttestationStorer) storeWithProtobufBundle(ctx context.Context, req *api.StoreRequest[name.Digest, *intoto.Statement]) (*api.StoreResponse, error) { logger := logging.FromContext(ctx) - logger.Infof("Using protobuf bundle format for attestation storage (%s)", req.Artifact.String()) + logger.Infof("Using sigstore bundle format for attestation storage (%s)", req.Artifact.String()) predicateType := req.Payload.PredicateType if predicateType == "" { @@ -224,7 +224,7 @@ func (s *AttestationStorer) storeWithProtobufBundle(ctx context.Context, req *ap if err := ociremote.WriteAttestationNewBundleFormat(req.Artifact, bundleBytes, predicateType, ociremote.WithRemoteOptions(s.remoteOpts...)); err != nil { return nil, errors.Wrap(err, "writing protobuf bundle attestation") } - logger.Infof("Successfully uploaded attestation using protobuf bundle format for %s", req.Artifact.String()) + logger.Infof("Successfully uploaded attestation using sigstore bundle format for %s", req.Artifact.String()) return &api.StoreResponse{}, nil } diff --git a/pkg/chains/storage/oci/legacy.go b/pkg/chains/storage/oci/legacy.go index 540cbaa094..03d8a04457 100644 --- a/pkg/chains/storage/oci/legacy.go +++ b/pkg/chains/storage/oci/legacy.go @@ -176,11 +176,12 @@ func (b *Backend) uploadSignature(ctx context.Context, format simple.SimpleConta Artifact: ref, Payload: format, Bundle: &signing.Bundle{ - Content: rawPayload, - Signature: []byte(signature), - Cert: []byte(storageOpts.Cert), - Chain: []byte(storageOpts.Chain), - PublicKey: storageOpts.PublicKey, + Content: rawPayload, + Signature: []byte(signature), + Cert: []byte(storageOpts.Cert), + Chain: []byte(storageOpts.Chain), + PublicKey: storageOpts.PublicKey, + RekorEntry: storageOpts.RekorEntry, }, }); err != nil { return err diff --git a/pkg/chains/storage/oci/simple_test.go b/pkg/chains/storage/oci/simple_test.go index 372e8ebb56..32aa6be3b5 100644 --- a/pkg/chains/storage/oci/simple_test.go +++ b/pkg/chains/storage/oci/simple_test.go @@ -457,3 +457,29 @@ func TestSimpleStorer_Store_SigstoreBundle_Dedup(t *testing.T) { t.Errorf("expected 1 signature referrer after dedup, got %d", got) } } + +// TestMakeSigBundleBytes_TlogEntries verifies that makeSigBundleBytes embeds +// tlogEntries when a non-nil RekorEntry is passed, and omits them when nil. +// This guards the fix for the transparency-log omission bug in the signature +// bundle path (legacy.go uploadSignature was not forwarding storageOpts.RekorEntry +// into the Bundle, so req.Bundle.RekorEntry arrived as nil here). +func TestMakeSigBundleBytes_TlogEntries(t *testing.T) { + // nil rekorEntry → tlogEntries must be absent/empty in the serialized bundle. + bundleBytes, err := makeSigBundleBytes(nil, nil, []byte("payload"), []byte("sig"), nil) + if err != nil { + t.Fatalf("makeSigBundleBytes with nil rekorEntry failed: %v", err) + } + var got map[string]interface{} + if err := json.Unmarshal(bundleBytes, &got); err != nil { + t.Fatalf("failed to unmarshal bundle JSON: %v", err) + } + vm, _ := got["verificationMaterial"].(map[string]interface{}) + if vm != nil { + if entries, ok := vm["tlogEntries"]; ok { + // tlogEntries key present — must be empty or nil. + if arr, ok := entries.([]interface{}); ok && len(arr) > 0 { + t.Errorf("expected empty tlogEntries with nil rekorEntry, got %d entries", len(arr)) + } + } + } +}