Skip to content

Commit

Permalink
Fix ReplaceSignatures
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 committed Oct 11, 2023
1 parent 4c5669d commit 8391340
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,47 @@ 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)
}
}
}
})
}

func (r *replaceOCISignatures) Get() ([]oci.Signature, error) {
return r.attestations, nil
}

type dupe struct {
Expand All @@ -437,3 +478,18 @@ 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
}
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 8391340

Please sign in to comment.