internal/ghsa: change "ID" in a SecurityAdvisory to mean the GHSA ID

This is a safe change because it will cause future FireStore entries
to be keyed by GHSA, but the worker does not depend on the keys being
any particular format.

Also makes some improvements to the GHSA tests.

Change-Id: I6f12993ff2289e38584e46e2f4382e76f5ad639f
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/433135
Reviewed-by: Tatiana Bradley <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>

Author: tatianab

internal/ghsa/ghsa.go

@@ -16,7 +16,7 @@ import (
 
 // A SecurityAdvisory represents a GitHub security advisory.
 type SecurityAdvisory struct {
-	// The GitHub Security Advisory identifier
+	// The GitHub Security Advisory identifier.
 	ID string
 	// A complete list of identifiers, e.g. CVE numbers.
 	Identifiers []Identifier
@@ -84,7 +84,7 @@ func (s *SecurityAdvisory) PrettyID() string {
 // GitHub's GraphQL schema. The fields must be exported to be populated by
 // Github's Client.Query function.
 type gqlSecurityAdvisory struct {
-	ID              string
+	GhsaID          string
 	Identifiers     []Identifier
 	Summary         string
 	Description     string
@@ -114,13 +114,13 @@ type gqlSecurityAdvisory struct {
 // there are more than 100 vulnerabilities associated with the advisory.
 func (sa *gqlSecurityAdvisory) securityAdvisory() (*SecurityAdvisory, error) {
 	if sa.PublishedAt.After(sa.UpdatedAt) {
-		return nil, fmt.Errorf("%s: published at %s, after updated at %s", sa.ID, sa.PublishedAt, sa.UpdatedAt)
+		return nil, fmt.Errorf("%s: published at %s, after updated at %s", sa.GhsaID, sa.PublishedAt, sa.UpdatedAt)
 	}
 	if sa.Vulnerabilities.PageInfo.HasNextPage {
-		return nil, fmt.Errorf("%s has more than 100 vulns", sa.ID)
+		return nil, fmt.Errorf("%s has more than 100 vulns", sa.GhsaID)
 	}
 	s := &SecurityAdvisory{
-		ID:          sa.ID,
+		ID:          sa.GhsaID,
 		Identifiers: sa.Identifiers,
 		Summary:     sa.Summary,
 		Description: sa.Description,

internal/ghsa/ghsa_test.go

@@ -15,22 +15,29 @@ import (
 
 var githubTokenFile = flag.String("ghtokenfile", "",
 	"path to file containing GitHub access token")
+var githubToken = flag.String("ghtoken", os.Getenv("VULN_GITHUB_ACCESS_TOKEN"), "GitHub access token")
 
 func mustGetAccessToken(t *testing.T) string {
-	if *githubTokenFile == "" {
-		t.Skip("-ghtokenfile not provided")
-	}
-	bytes, err := os.ReadFile(*githubTokenFile)
-	if err != nil {
-		t.Fatal(err)
+	var token string
+	switch {
+	case *githubToken != "":
+		token = *githubToken
+	case *githubTokenFile != "":
+		bytes, err := os.ReadFile(*githubTokenFile)
+		if err != nil {
+			t.Fatal(err)
+		}
+		token = string(bytes)
+	default:
+		t.Skip("neither -ghtokenfile nor -ghtoken provided")
 	}
-	return strings.TrimSpace(string(bytes))
+	return strings.TrimSpace(string(token))
 }
 
 func TestList(t *testing.T) {
 	accessToken := mustGetAccessToken(t)
 	// There were at least three relevant SAs since this date.
-	since := time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)
+	since := time.Date(2022, 9, 1, 0, 0, 0, 0, time.UTC)
 	got, err := List(context.Background(), accessToken, since)
 	if err != nil {
 		t.Fatal(err)
@@ -39,11 +46,6 @@ func TestList(t *testing.T) {
 	if len(got) < want {
 		t.Errorf("got %d, want at least %d", len(got), want)
 	}
-	for _, g := range got {
-		if isCVE(g.Identifiers) {
-			t.Errorf("isCVE true, want false for %+v", g)
-		}
-	}
 }
 
 func TestFetchGHSA(t *testing.T) {
@@ -54,15 +56,7 @@ func TestFetchGHSA(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	want := ghsaID
-	var gotID string
-	for _, id := range got.Identifiers {
-		if id.Type == "GHSA" {
-			gotID = id.Value
-			break
-		}
-	}
-	if gotID != want {
+	if gotID, want := got.ID, ghsaID; gotID != want {
 		t.Errorf("got GHSA with id %q, want %q", got.ID, want)
 	}
 }
@@ -78,17 +72,15 @@ func TestListForCVE(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	var ids []string
-	for _, sa := range got {
-		for _, id := range sa.Identifiers {
-			if id.Type != "GHSA" {
-				continue
-			}
-			ids = append(ids, id.Value)
-			if id.Value == ghsaID {
-				return
-			}
+
+	want := ghsaID
+	if len(got) != 1 {
+		var gotIDs []string
+		for _, sa := range got {
+			gotIDs = append(gotIDs, sa.ID)
 		}
+		t.Errorf("got %v GHSAs %v, want %v", len(got), gotIDs, want)
+	} else if gotID := got[0].ID; gotID != want {
+		t.Errorf("got GHSA %v, want %v", gotID, want)
 	}
-	t.Errorf("got %v GHSAs %v, want %v", len(got), ids, ghsaID)
 }