Skip to content

Commit

Permalink
Fix ReplaceSignatures (#3292)
Browse files Browse the repository at this point in the history
This was correctly replacing the base of the signatures but was also
appending those signatures to the Get() response. So the v1.Image
representation was getting out of sync with the oci.Signatures
representation.

Normally you won't see this because cosign doesn't do multiple
attestations in the same command for the same image, so you'd roundtrip
the signatures through the v1.Image being pushed and pulled, which
corrects the discrepancy.

If you (specifically, me) attempt to attach an attestation multiple
times, the Get() will get very out of sync with the v1.Image and Replace
no longer does what you'd expect because it's operating on incorrect
Get() results instead of directly on the v1.Image representation.

Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
  • Loading branch information
jonjohnsonjr authored Oct 11, 2023
1 parent c827eed commit faa47c7
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 2 deletions.
58 changes: 57 additions & 1 deletion pkg/oci/mutate/mutate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ func TestSignEntity(t *testing.T) {
if al, err := atts.Get(); err != nil {
t.Fatalf("Get() = %v", err)
} else if len(al) != 1 {
t.Errorf("len(Get()) = %d, wanted %d", len(al), i)
t.Errorf("len(Get()) = %d, wanted %d", len(al), 1)
}
}
}
Expand Down Expand Up @@ -424,6 +424,43 @@ func TestSignEntity(t *testing.T) {
}
}
})

t.Run("with replace op (attestation)", func(t *testing.T) {
for _, se := range []oci.SignedEntity{si, sii, sunk} {
orig, err := static.NewAttestation([]byte("blah"))
if err != nil {
t.Fatalf("static.NewAttestation() = %v", err)
}
se, err = AttachAttestationToEntity(se, orig)
if err != nil {
t.Fatalf("AttachAttestationToEntity() = %v", err)
}

ro := &replaceAll{}

for i := 2; i < 10; i++ {
sig, err := static.NewAttestation([]byte(fmt.Sprintf("%d", i)))
if err != nil {
t.Fatalf("static.NewAttestation() = %v", err)
}

se, err = AttachAttestationToEntity(se, sig, WithReplaceOp(ro))
if err != nil {
t.Fatalf("AttachAttestationToEntity() = %v", err)
}

atts, err := se.Attestations()
if err != nil {
t.Fatalf("Attestations() = %v", err)
}
if al, err := atts.Get(); err != nil {
t.Fatalf("Get() = %v", err)
} else if len(al) != 1 {
t.Errorf("len(Get()) = %d, wanted %d", len(al), 1)
}
}
}
})
}

type dupe struct {
Expand All @@ -437,3 +474,22 @@ var _ DupeDetector = (*dupe)(nil)
func (d *dupe) Find(oci.Signatures, oci.Signature) (oci.Signature, error) {
return d.sig, d.err
}

type replaceAll struct {
}

func (r *replaceAll) Replace(signatures oci.Signatures, o oci.Signature) (oci.Signatures, error) {
return &replaceOCISignatures{
Signatures: signatures,
attestations: []oci.Signature{o},
}, nil
}

type replaceOCISignatures struct {
oci.Signatures
attestations []oci.Signature
}

func (r *replaceOCISignatures) Get() ([]oci.Signature, error) {
return r.attestations, nil
}
2 changes: 1 addition & 1 deletion pkg/oci/mutate/signatures.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func ReplaceSignatures(base oci.Signatures) (oci.Signatures, error) {
return &sigAppender{
Image: img,
base: base,
sigs: sigs,
sigs: []oci.Signature{},
}, nil
}

Expand Down

0 comments on commit faa47c7

Please sign in to comment.