Resolve PR comments, linter, and update tests

Signed-off-by: Hayden Blauzvern <[email protected]>

cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go

@@ -51,7 +51,7 @@ const altCTLogPublicKeyLocation = "SIGSTORE_CT_LOG_PUBLIC_KEY_FILE"
 // The SCT is a `Signed Certificate Timestamp`, which promises that
 // the certificate issued by Fulcio was also added to the public CT log within
 // some defined time period
-func verifySCT(certPEM, rawSCT []byte, ctx context.Context) error {
+func verifySCT(ctx context.Context, certPEM, rawSCT []byte) error {
 	var pubKeys []crypto.PublicKey
 	rootEnv := os.Getenv(altCTLogPublicKeyLocation)
 	if rootEnv == "" {
@@ -114,7 +114,7 @@ func NewSigner(ctx context.Context, idToken, oidcIssuer, oidcClientID, oidcClien
 	}
 
 	// verify the sct
-	if err := verifySCT(fs.Cert, fs.SCT, ctx); err != nil {
+	if err := verifySCT(ctx, fs.Cert, fs.SCT); err != nil {
 		return nil, errors.Wrap(err, "verifying SCT")
 	}
 	fmt.Fprintln(os.Stderr, "Successfully verified SCT...")

pkg/cosign/tlog.go

@@ -55,7 +55,7 @@ func GetRekorPubs(ctx context.Context) ([]*ecdsa.PublicKey, error) {
 	if err != nil {
 		return nil, err
 	}
-	var publicKeys []*ecdsa.PublicKey
+	publicKeys := make([]*ecdsa.PublicKey, 0, len(targets))
 	for _, t := range targets {
 		rekorPubKey, err := PemToECDSAKey(t.Target)
 		if err != nil {

pkg/cosign/tuf/client.go

@@ -277,7 +277,8 @@ func (t *TUF) GetTargetsByMeta(usage UsageKind, fallbacks []string) ([]TargetFil
 		var scm sigstoreCustomMetadata
 		err := json.Unmarshal(*targetMeta.Custom, &scm)
 		if err != nil {
-			return nil, errors.Wrap(err, "error unmarshaling custom metadata")
+			fmt.Fprintf(os.Stderr, "**Warning** Custom metadata not configured properly for target %s, skipping target\n", name)
+			continue
 		}
 		if scm.Sigstore.Usage == usage {
 			target, err := t.GetTarget(name)

pkg/cosign/tuf/client_test.go

@@ -238,10 +238,10 @@ func TestGetTargetsByMeta(t *testing.T) {
 	}
 
 	tufObj, err := NewFromEnv(ctx)
-	defer tufObj.Close()
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer tufObj.Close()
 	// Fetch a target with no custom metadata.
 	targets, err := tufObj.GetTargetsByMeta(UnknownUsage, []string{"fooNoCustom.txt"})
 	if err != nil {
@@ -256,6 +256,17 @@ func TestGetTargetsByMeta(t *testing.T) {
 	if targets[0].Status != Active {
 		t.Fatalf("target without custom metadata not active, got: %v", targets[0].Status)
 	}
+	// Fetch multiple targets with no custom metadata.
+	targets, err = tufObj.GetTargetsByMeta(UnknownUsage, []string{"fooNoCustom.txt", "fooNoCustomOther.txt"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(targets) != 2 {
+		t.Fatalf("expected two targets without custom metadata, got %d targets", len(targets))
+	}
+	if targets[0].Status != Active || targets[1].Status != Active {
+		t.Fatalf("target without custom metadata not active, got: %v and %v", targets[0].Status, targets[1].Status)
+	}
 	// Fetch targets with custom metadata.
 	targets, err = tufObj.GetTargetsByMeta(Fulcio, []string{"fooNoCustom.txt"})
 	if err != nil {
@@ -271,7 +282,7 @@ func TestGetTargetsByMeta(t *testing.T) {
 	}
 	targetStatuses := []StatusKind{targets[0].Status, targets[1].Status}
 	sort.Slice(targetStatuses, func(i, j int) bool {
-		return i < j
+		return targetStatuses[i] < targetStatuses[j]
 	})
 	expectedTS := []StatusKind{Active, Expired}
 	if !reflect.DeepEqual(targetStatuses, expectedTS) {
@@ -347,6 +358,9 @@ func forceExpirationVersion(t *testing.T, version int) {
 	})
 }
 
+// newTufCustomRepo initializes a TUF repository with root, targets, snapshot, and timestamp roles
+// 4 targets are created to exercise various code paths, including two targets with no custom metadata,
+// one target with custom metadata marked as active, and another with custom metadata marked as expired.
 func newTufCustomRepo(t *testing.T, td string, targetData string) (tuf.LocalStore, *tuf.Repo) {
 	scmActive, err := json.Marshal(&sigstoreCustomMetadata{Sigstore: customMetadata{Usage: Fulcio, Status: Active}})
 	if err != nil {
@@ -370,7 +384,9 @@ func newTufCustomRepo(t *testing.T, td string, targetData string) (tuf.LocalStor
 			t.Error(err)
 		}
 	}
-	for name, scm := range map[string]json.RawMessage{"fooNoCustom.txt": nil, "fooActive.txt": scmActive, "fooExpired.txt": scmExpired} {
+	for name, scm := range map[string]json.RawMessage{
+		"fooNoCustom.txt": nil, "fooNoCustomOther.txt": nil,
+		"fooActive.txt": scmActive, "fooExpired.txt": scmExpired} {
 		targetPath := filepath.Join(td, "staged", "targets", name)
 		if err := os.MkdirAll(filepath.Dir(targetPath), 0755); err != nil {
 			t.Error(err)

pkg/cosign/tuf/status_type.go

@@ -17,8 +17,6 @@ package tuf
 import (
 	"fmt"
 	"strings"
-
-	"github.com/pkg/errors"
 )
 
 type StatusKind int
@@ -42,7 +40,7 @@ func (s StatusKind) String() string {
 func (s StatusKind) MarshalText() ([]byte, error) {
 	str := s.String()
 	if len(str) == 0 {
-		return nil, errors.New(fmt.Sprintf("error while marshalling, int(StatusKind)=%d not valid", int(s)))
+		return nil, fmt.Errorf("error while marshalling, int(StatusKind)=%d not valid", int(s))
 	}
 	return []byte(s.String()), nil
 }
@@ -56,7 +54,7 @@ func (s *StatusKind) UnmarshalText(text []byte) error {
 	case "expired":
 		*s = Expired
 	default:
-		return errors.New(fmt.Sprintf("error while unmarshalling, StatusKind=%v not valid", string(text)))
+		return fmt.Errorf("error while unmarshalling, StatusKind=%v not valid", string(text))
 	}
 	return nil
 }

pkg/cosign/tuf/usage_type.go

@@ -17,8 +17,6 @@ package tuf
 import (
 	"fmt"
 	"strings"
-
-	"github.com/pkg/errors"
 )
 
 type UsageKind int
@@ -44,7 +42,7 @@ func (u UsageKind) String() string {
 func (u UsageKind) MarshalText() ([]byte, error) {
 	str := u.String()
 	if len(str) == 0 {
-		return nil, errors.New(fmt.Sprintf("error while marshalling, int(UsageKind)=%d not valid", int(u)))
+		return nil, fmt.Errorf("error while marshalling, int(UsageKind)=%d not valid", int(u))
 	}
 	return []byte(u.String()), nil
 }
@@ -60,7 +58,7 @@ func (u *UsageKind) UnmarshalText(text []byte) error {
 	case "ctfe":
 		*u = CTFE
 	default:
-		return errors.New(fmt.Sprintf("error while unmarshalling, UsageKind=%v not valid", string(text)))
+		return fmt.Errorf("error while unmarshalling, UsageKind=%v not valid", string(text))
 	}
 	return nil
 }