refactor: apply simplify review on signature fix

- Guard BundleFromDSSEEnvelope against empty DSSE Signatures slice.
- Trim narrative comment on BundleBytesFromEnvelope testhelper.
- Deduplicate attestations_test.go with a shared test envelope helper; add
  explicit no-signatures and no-op cases.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>

Author: migmartri

app/controlplane/pkg/biz/testhelpers/attestation.go

@@ -26,9 +26,8 @@ import (
 	"google.golang.org/protobuf/encoding/protojson"
 )
 
-// BundleBytesFromEnvelope reads a DSSE envelope fixture from disk, wraps it into a
-// Sigstore bundle, and returns its protojson-encoded bytes. Useful to feed envelope
-// test fixtures into APIs that now only accept Sigstore bundles.
+// BundleBytesFromEnvelope reads a DSSE envelope fixture from disk and returns the
+// protojson-encoded bytes of an equivalent Sigstore bundle.
 func BundleBytesFromEnvelope(t *testing.T, path string) []byte {
 	t.Helper()
 	raw, err := os.ReadFile(path)

pkg/attestation/attestations.go

@@ -70,6 +70,9 @@ func DSSEEnvelopeFromBundle(bundle *protobundle.Bundle) *dsse.Envelope {
 }
 
 func BundleFromDSSEEnvelope(dsseEnvelope *dsse.Envelope) (*protobundle.Bundle, error) {
+	if len(dsseEnvelope.Signatures) == 0 {
+		return nil, fmt.Errorf("DSSE envelope has no signatures")
+	}
 	// DSSE Envelope payload and signature are base64 encoded, we need to decode them so they
 	// are not encoded twice when stored as raw bytes in the Sigstore bundle. See #1832.
 	payload, err := base64.StdEncoding.DecodeString(dsseEnvelope.Payload)

pkg/attestation/attestations_test.go

@@ -25,70 +25,58 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// Guards against the double-base64 bug in https://github.com/chainloop-dev/chainloop/issues/1832.
-func TestBundleFromDSSEEnvelopeDecodesSignature(t *testing.T) {
-	rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD}
-	rawPayload := []byte(`{"_type":"statement"}`)
+var (
+	testRawSig     = []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD}
+	testRawPayload = []byte(`{"_type":"statement"}`)
+)
 
-	env := &dsse.Envelope{
+func newTestEnvelope(t *testing.T) *dsse.Envelope {
+	t.Helper()
+	return &dsse.Envelope{
 		PayloadType: "application/vnd.in-toto+json",
-		Payload:     base64.StdEncoding.EncodeToString(rawPayload),
+		Payload:     base64.StdEncoding.EncodeToString(testRawPayload),
 		Signatures: []dsse.Signature{
-			{KeyID: "key-1", Sig: base64.StdEncoding.EncodeToString(rawSig)},
+			{KeyID: "key-1", Sig: base64.StdEncoding.EncodeToString(testRawSig)},
 		},
 	}
+}
 
-	bundle, err := attestation.BundleFromDSSEEnvelope(env)
+// Guards against the double-base64 bug in https://github.com/chainloop-dev/chainloop/issues/1832.
+func TestBundleFromDSSEEnvelopeDecodesSignature(t *testing.T) {
+	bundle, err := attestation.BundleFromDSSEEnvelope(newTestEnvelope(t))
 	require.NoError(t, err)
 
 	gotEnv := bundle.GetDsseEnvelope()
-	assert.Equal(t, rawPayload, gotEnv.GetPayload())
+	assert.Equal(t, testRawPayload, gotEnv.GetPayload())
 	require.Len(t, gotEnv.GetSignatures(), 1)
-	assert.Equal(t, rawSig, gotEnv.GetSignatures()[0].GetSig())
+	assert.Equal(t, testRawSig, gotEnv.GetSignatures()[0].GetSig())
 	assert.Equal(t, "key-1", gotEnv.GetSignatures()[0].GetKeyid())
 }
 
-func TestBundleRoundTripWithFixedSignature(t *testing.T) {
-	rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD}
-	encodedSig := base64.StdEncoding.EncodeToString(rawSig)
-
-	env := &dsse.Envelope{
-		PayloadType: "application/vnd.in-toto+json",
-		Payload:     base64.StdEncoding.EncodeToString([]byte("payload")),
-		Signatures: []dsse.Signature{
-			{KeyID: "key-1", Sig: encodedSig},
-		},
-	}
+func TestBundleFromDSSEEnvelopeNoSignatures(t *testing.T) {
+	env := newTestEnvelope(t)
+	env.Signatures = nil
+	_, err := attestation.BundleFromDSSEEnvelope(env)
+	require.Error(t, err)
+}
 
-	bundle, err := attestation.BundleFromDSSEEnvelope(env)
+func TestFixSignatureInBundleIsNoOpOnFixedBundles(t *testing.T) {
+	bundle, err := attestation.BundleFromDSSEEnvelope(newTestEnvelope(t))
 	require.NoError(t, err)
 
 	before := bundle.GetDsseEnvelope().GetSignatures()[0].GetSig()
 	attestation.FixSignatureInBundle(bundle)
-	assert.Equal(t, before, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig(),
-		"FixSignatureInBundle should be a no-op on properly formed bundles")
-
-	gotEnv := attestation.DSSEEnvelopeFromBundle(bundle)
-	assert.Equal(t, encodedSig, gotEnv.Signatures[0].Sig)
+	assert.Equal(t, before, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig())
 }
 
 func TestFixSignatureInBundleRepairsLegacyBundles(t *testing.T) {
-	rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD}
-	encodedSig := base64.StdEncoding.EncodeToString(rawSig)
-
-	env := &dsse.Envelope{
-		PayloadType: "application/vnd.in-toto+json",
-		Payload:     base64.StdEncoding.EncodeToString([]byte("payload")),
-		Signatures: []dsse.Signature{
-			{KeyID: "key-1", Sig: encodedSig},
-		},
-	}
-	bundle, err := attestation.BundleFromDSSEEnvelope(env)
+	bundle, err := attestation.BundleFromDSSEEnvelope(newTestEnvelope(t))
 	require.NoError(t, err)
 
 	// Simulate the legacy bug: signature is stored as the ASCII bytes of the base64 string.
+	encodedSig := base64.StdEncoding.EncodeToString(testRawSig)
 	bundle.GetDsseEnvelope().GetSignatures()[0].Sig = []byte(encodedSig)
 
 	attestation.FixSignatureInBundle(bundle)
-	assert.Equal(t, rawSig, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig())
+	assert.Equal(t, testRawSig, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig())
 }