Fix duplicate .att/.sig OCI layers for same-digest type hints

  When a Task declares the same OCI image through multiple type-hint
  formats (IMAGE_URL/IMAGE_DIGEST and IMAGES), Chains produces duplicate
  layers in cosign .att/.sig manifests. This adds dedup at two levels:

  1. Extraction: track seen digests in ExtractOCIImagesFromResults to
     avoid returning the same digest from different type-hint formats
  2. Storage: check existing layers in AttestationStorer and SimpleStorer
     before appending, preventing duplicates from independent signable
     types (TaskRunArtifact + OCIArtifact) storing to the same tag

  Also adds debug logging when existing layer checks fail instead of
  silently swallowing errors.

  Fixes #1596

Author: ab-ghosh

pkg/artifacts/signable.go

@@ -164,6 +164,7 @@ func (oa *OCIArtifact) ExtractObjects(ctx context.Context, obj objects.TektonObj
 func ExtractOCIImagesFromResults(ctx context.Context, results []objects.Result) []interface{} {
 	logger := logging.FromContext(ctx)
 	objs := []interface{}{}
+	seen := map[string]bool{}
 
 	extractor := structuredSignableExtractor{
 		uriSuffix:    OCIImageURLResultName,
@@ -176,6 +177,11 @@ func ExtractOCIImagesFromResults(ctx context.Context, results []objects.Result)
 			logger.Errorf("error getting digest: %v", err)
 			continue
 		}
+		if seen[dgst.DigestStr()] {
+			logger.Debugf("skipping duplicate digest %s", dgst.DigestStr())
+			continue
+		}
+		seen[dgst.DigestStr()] = true
 		objs = append(objs, dgst)
 	}
 
@@ -196,6 +202,11 @@ func ExtractOCIImagesFromResults(ctx context.Context, results []objects.Result)
 				logger.Errorf("error getting digest for img %s: %v", trimmed, err)
 				continue
 			}
+			if seen[dgst.DigestStr()] {
+				logger.Debugf("skipping duplicate digest %s", dgst.DigestStr())
+				continue
+			}
+			seen[dgst.DigestStr()] = true
 			objs = append(objs, dgst)
 		}
 	}

pkg/artifacts/signable_test.go

@@ -188,7 +188,6 @@ func TestExtractOCIImagesFromResults(t *testing.T) {
 	want := []interface{}{
 		createDigest(t, fmt.Sprintf("img1@%s", digest1)),
 		createDigest(t, fmt.Sprintf("img2@%s", digest2)),
-		createDigest(t, fmt.Sprintf("img3@%s", digest1)),
 	}
 	ctx := logtesting.TestContextWithLogger(t)
 	got := ExtractOCIImagesFromResults(ctx, results)
@@ -202,6 +201,24 @@ func TestExtractOCIImagesFromResults(t *testing.T) {
 	}
 }
 
+func TestExtractOCIImagesFromResults_CrossFormatDedup(t *testing.T) {
+	// Same image appears via IMAGE_URL/IMAGE_DIGEST and IMAGES — should be deduplicated.
+	results := []objects.Result{
+		{Name: "IMAGE_URL", Value: *v1.NewStructuredValues("img1")},
+		{Name: "IMAGE_DIGEST", Value: *v1.NewStructuredValues(digest1)},
+		{Name: "IMAGES", Value: *v1.NewStructuredValues(fmt.Sprintf("img1@%s", digest1))},
+	}
+
+	want := []interface{}{
+		createDigest(t, fmt.Sprintf("img1@%s", digest1)),
+	}
+	ctx := logtesting.TestContextWithLogger(t)
+	got := ExtractOCIImagesFromResults(ctx, results)
+	if !cmp.Equal(got, want, ignore...) {
+		t.Fatalf("expected dedup across type-hint formats, got %s", cmp.Diff(want, got, ignore...))
+	}
+}
+
 func TestExtractSignableTargetFromResults(t *testing.T) {
 	tr := &v1.TaskRun{
 		Status: v1.TaskRunStatus{

pkg/chains/storage/oci/attestation.go

@@ -77,6 +77,25 @@ func (s *AttestationStorer) Store(ctx context.Context, req *api.StoreRequest[nam
 	if err != nil {
 		return nil, err
 	}
+
+	// Check if an attestation with the same digest already exists.
+	newDigest, err := att.Digest()
+	if err != nil {
+		return nil, errors.Wrap(err, "getting new attestation digest")
+	}
+	if existingAtts, err := se.Attestations(); err != nil {
+		logger.Debugf("Could not fetch existing attestations for %s, skipping dedup check: %v", req.Artifact.String(), err)
+	} else if layers, err := existingAtts.Get(); err != nil {
+		logger.Debugf("Could not get attestation layers for %s, skipping dedup check: %v", req.Artifact.String(), err)
+	} else {
+		for _, l := range layers {
+			if d, err := l.Digest(); err == nil && d == newDigest {
+				logger.Infof("Attestation with digest %s already exists for %s, skipping", newDigest, req.Artifact.String())
+				return &api.StoreResponse{}, nil
+			}
+		}
+	}
+
 	newImage, err := mutate.AttachAttestationToEntity(se, att)
 	if err != nil {
 		return nil, err

pkg/chains/storage/oci/attestation_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/google/go-containerregistry/pkg/v1/types"
 	intoto "github.com/in-toto/attestation/go/v1"
+	ociremote "github.com/sigstore/cosign/v2/pkg/oci/remote"
 	"github.com/tektoncd/chains/pkg/chains/signing"
 	"github.com/tektoncd/chains/pkg/chains/storage/api"
 	logtesting "knative.dev/pkg/logging/testing"
@@ -109,3 +110,127 @@ func TestAttestationStorer_Store(t *testing.T) {
 		})
 	}
 }
+
+func TestAttestationStorer_Store_Dedup(t *testing.T) {
+	s := httptest.NewServer(registry.New())
+	defer s.Close()
+	registryName := strings.TrimPrefix(s.URL, "http://")
+
+	img, err := random.Image(1024, 2)
+	if err != nil {
+		t.Fatalf("failed to create random image: %s", err)
+	}
+	imgDigest, err := img.Digest()
+	if err != nil {
+		t.Fatalf("failed to get image digest: %v", err)
+	}
+	ref, err := name.NewDigest(fmt.Sprintf("%s/test/img@%s", registryName, imgDigest))
+	if err != nil {
+		t.Fatalf("failed to parse digest: %v", err)
+	}
+	if err := remote.Write(ref, img); err != nil {
+		t.Fatalf("failed to write image to mock registry: %v", err)
+	}
+
+	storer, err := NewAttestationStorer(WithTargetRepository(ref.Repository))
+	if err != nil {
+		t.Fatalf("failed to create storer: %v", err)
+	}
+
+	ctx := logtesting.TestContextWithLogger(t)
+	req := &api.StoreRequest[name.Digest, *intoto.Statement]{
+		Artifact: ref,
+		Payload:  &intoto.Statement{},
+		Bundle:   &signing.Bundle{Signature: []byte("sig1")},
+	}
+
+	// Store the same attestation twice.
+	if _, err := storer.Store(ctx, req); err != nil {
+		t.Fatalf("first Store() failed: %s", err)
+	}
+	if _, err := storer.Store(ctx, req); err != nil {
+		t.Fatalf("second Store() failed: %s", err)
+	}
+
+	// Verify only one attestation layer exists.
+	se, err := ociremote.SignedEntity(ref)
+	if err != nil {
+		t.Fatalf("failed to get signed entity: %v", err)
+	}
+	atts, err := se.Attestations()
+	if err != nil {
+		t.Fatalf("failed to get attestations: %v", err)
+	}
+	layers, err := atts.Get()
+	if err != nil {
+		t.Fatalf("failed to get attestation layers: %v", err)
+	}
+	if got := len(layers); got != 1 {
+		t.Errorf("expected 1 attestation layer, got %d", got)
+	}
+}
+
+func TestAttestationStorer_Store_DistinctNotDeduped(t *testing.T) {
+	s := httptest.NewServer(registry.New())
+	defer s.Close()
+	registryName := strings.TrimPrefix(s.URL, "http://")
+
+	img, err := random.Image(1024, 2)
+	if err != nil {
+		t.Fatalf("failed to create random image: %s", err)
+	}
+	imgDigest, err := img.Digest()
+	if err != nil {
+		t.Fatalf("failed to get image digest: %v", err)
+	}
+	ref, err := name.NewDigest(fmt.Sprintf("%s/test/img@%s", registryName, imgDigest))
+	if err != nil {
+		t.Fatalf("failed to parse digest: %v", err)
+	}
+	if err := remote.Write(ref, img); err != nil {
+		t.Fatalf("failed to write image to mock registry: %v", err)
+	}
+
+	storer, err := NewAttestationStorer(WithTargetRepository(ref.Repository))
+	if err != nil {
+		t.Fatalf("failed to create storer: %v", err)
+	}
+
+	ctx := logtesting.TestContextWithLogger(t)
+
+	// Store two attestations with different signatures (different layer content).
+	req1 := &api.StoreRequest[name.Digest, *intoto.Statement]{
+		Artifact: ref,
+		Payload:  &intoto.Statement{},
+		Bundle:   &signing.Bundle{Signature: []byte("sig1")},
+	}
+	req2 := &api.StoreRequest[name.Digest, *intoto.Statement]{
+		Artifact: ref,
+		Payload:  &intoto.Statement{},
+		Bundle:   &signing.Bundle{Signature: []byte("sig2")},
+	}
+
+	if _, err := storer.Store(ctx, req1); err != nil {
+		t.Fatalf("first Store() failed: %s", err)
+	}
+	if _, err := storer.Store(ctx, req2); err != nil {
+		t.Fatalf("second Store() failed: %s", err)
+	}
+
+	// Verify both attestation layers are kept.
+	se, err := ociremote.SignedEntity(ref)
+	if err != nil {
+		t.Fatalf("failed to get signed entity: %v", err)
+	}
+	atts, err := se.Attestations()
+	if err != nil {
+		t.Fatalf("failed to get attestations: %v", err)
+	}
+	layers, err := atts.Get()
+	if err != nil {
+		t.Fatalf("failed to get attestation layers: %v", err)
+	}
+	if got := len(layers); got != 2 {
+		t.Errorf("expected 2 distinct attestation layers, got %d", got)
+	}
+}

pkg/chains/storage/oci/simple.go

@@ -74,6 +74,25 @@ func (s *SimpleStorer) Store(ctx context.Context, req *api.StoreRequest[name.Dig
 	if err != nil {
 		return nil, err
 	}
+
+	// Check if a signature with the same payload digest already exists.
+	newDigest, err := sig.Digest()
+	if err != nil {
+		return nil, errors.Wrap(err, "getting new signature digest")
+	}
+	if existingSigs, err := se.Signatures(); err != nil {
+		logger.Debugf("Could not fetch existing signatures for %s, skipping dedup check: %v", req.Artifact.String(), err)
+	} else if layers, err := existingSigs.Get(); err != nil {
+		logger.Debugf("Could not get signature layers for %s, skipping dedup check: %v", req.Artifact.String(), err)
+	} else {
+		for _, l := range layers {
+			if d, err := l.Digest(); err == nil && d == newDigest {
+				logger.Infof("Signature with digest %s already exists, skipping", newDigest)
+				return &api.StoreResponse{}, nil
+			}
+		}
+	}
+
 	// Attach the signature to the entity.
 	newSE, err := mutate.AttachSignatureToEntity(se, sig)
 	if err != nil {

pkg/chains/storage/oci/simple_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/random"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/google/go-containerregistry/pkg/v1/types"
+	ociremote "github.com/sigstore/cosign/v2/pkg/oci/remote"
 	"github.com/tektoncd/chains/pkg/chains/formats/simple"
 	"github.com/tektoncd/chains/pkg/chains/signing"
 	"github.com/tektoncd/chains/pkg/chains/storage/api"
@@ -108,3 +109,127 @@ func TestSimpleStorer_Store(t *testing.T) {
 		})
 	}
 }
+
+func TestSimpleStorer_Store_Dedup(t *testing.T) {
+	s := httptest.NewServer(registry.New())
+	defer s.Close()
+	registryName := strings.TrimPrefix(s.URL, "http://")
+
+	img, err := random.Image(1024, 2)
+	if err != nil {
+		t.Fatalf("failed to create random image: %s", err)
+	}
+	imgDigest, err := img.Digest()
+	if err != nil {
+		t.Fatalf("failed to get image digest: %v", err)
+	}
+	ref, err := name.NewDigest(fmt.Sprintf("%s/test/img@%s", registryName, imgDigest))
+	if err != nil {
+		t.Fatalf("failed to parse digest: %v", err)
+	}
+	if err := remote.Write(ref, img); err != nil {
+		t.Fatalf("failed to write image to mock registry: %v", err)
+	}
+
+	storer, err := NewSimpleStorerFromConfig(WithTargetRepository(ref.Repository))
+	if err != nil {
+		t.Fatalf("failed to create storer: %v", err)
+	}
+
+	ctx := logtesting.TestContextWithLogger(t)
+	req := &api.StoreRequest[name.Digest, simple.SimpleContainerImage]{
+		Artifact: ref,
+		Payload:  simple.NewSimpleStruct(ref),
+		Bundle:   &signing.Bundle{Content: []byte("payload"), Signature: []byte("sig1")},
+	}
+
+	// Store the same signature twice.
+	if _, err := storer.Store(ctx, req); err != nil {
+		t.Fatalf("first Store() failed: %s", err)
+	}
+	if _, err := storer.Store(ctx, req); err != nil {
+		t.Fatalf("second Store() failed: %s", err)
+	}
+
+	// Verify only one signature layer exists.
+	se, err := ociremote.SignedEntity(ref)
+	if err != nil {
+		t.Fatalf("failed to get signed entity: %v", err)
+	}
+	sigs, err := se.Signatures()
+	if err != nil {
+		t.Fatalf("failed to get signatures: %v", err)
+	}
+	layers, err := sigs.Get()
+	if err != nil {
+		t.Fatalf("failed to get signature layers: %v", err)
+	}
+	if got := len(layers); got != 1 {
+		t.Errorf("expected 1 signature layer, got %d", got)
+	}
+}
+
+func TestSimpleStorer_Store_DistinctNotDeduped(t *testing.T) {
+	s := httptest.NewServer(registry.New())
+	defer s.Close()
+	registryName := strings.TrimPrefix(s.URL, "http://")
+
+	img, err := random.Image(1024, 2)
+	if err != nil {
+		t.Fatalf("failed to create random image: %s", err)
+	}
+	imgDigest, err := img.Digest()
+	if err != nil {
+		t.Fatalf("failed to get image digest: %v", err)
+	}
+	ref, err := name.NewDigest(fmt.Sprintf("%s/test/img@%s", registryName, imgDigest))
+	if err != nil {
+		t.Fatalf("failed to parse digest: %v", err)
+	}
+	if err := remote.Write(ref, img); err != nil {
+		t.Fatalf("failed to write image to mock registry: %v", err)
+	}
+
+	storer, err := NewSimpleStorerFromConfig(WithTargetRepository(ref.Repository))
+	if err != nil {
+		t.Fatalf("failed to create storer: %v", err)
+	}
+
+	ctx := logtesting.TestContextWithLogger(t)
+
+	// Store two signatures with different content (different layer digests).
+	req1 := &api.StoreRequest[name.Digest, simple.SimpleContainerImage]{
+		Artifact: ref,
+		Payload:  simple.NewSimpleStruct(ref),
+		Bundle:   &signing.Bundle{Content: []byte("payload1"), Signature: []byte("sig1")},
+	}
+	req2 := &api.StoreRequest[name.Digest, simple.SimpleContainerImage]{
+		Artifact: ref,
+		Payload:  simple.NewSimpleStruct(ref),
+		Bundle:   &signing.Bundle{Content: []byte("payload2"), Signature: []byte("sig2")},
+	}
+
+	if _, err := storer.Store(ctx, req1); err != nil {
+		t.Fatalf("first Store() failed: %s", err)
+	}
+	if _, err := storer.Store(ctx, req2); err != nil {
+		t.Fatalf("second Store() failed: %s", err)
+	}
+
+	// Verify both signature layers are kept.
+	se, err := ociremote.SignedEntity(ref)
+	if err != nil {
+		t.Fatalf("failed to get signed entity: %v", err)
+	}
+	sigs, err := se.Signatures()
+	if err != nil {
+		t.Fatalf("failed to get signatures: %v", err)
+	}
+	layers, err := sigs.Get()
+	if err != nil {
+		t.Fatalf("failed to get signature layers: %v", err)
+	}
+	if got := len(layers); got != 2 {
+		t.Errorf("expected 2 distinct signature layers, got %d", got)
+	}
+}