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

5 views
Skip to first unread message

Tatiana Bradley (Gerrit)

unread,
Sep 23, 2022, 12:43:39 PM9/23/22
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Jonathan Amsterdam, Gopher Robot, golang-co...@googlegroups.com

Tatiana Bradley submitted this change.

View Change


Approvals: Jonathan Amsterdam: Looks good to me, approved Tatiana Bradley: Looks good to me, but someone else must approve
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 <tat...@golang.org>
Reviewed-by: Jonathan Amsterdam <j...@google.com>
---
M internal/ghsa/ghsa.go
M internal/ghsa/ghsa_test.go
2 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/internal/ghsa/ghsa.go b/internal/ghsa/ghsa.go
index 0512b9d..9ccb73a 100644
--- a/internal/ghsa/ghsa.go
+++ b/internal/ghsa/ghsa.go
@@ -16,7 +16,7 @@

// 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 @@
// 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 @@
// 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,
diff --git a/internal/ghsa/ghsa_test.go b/internal/ghsa/ghsa_test.go
index 4739bcd..40931e8 100644
--- a/internal/ghsa/ghsa_test.go
+++ b/internal/ghsa/ghsa_test.go
@@ -15,22 +15,29 @@

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")
+ 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")
}
- bytes, err := os.ReadFile(*githubTokenFile)
- if err != nil {
- t.Fatal(err)
- }
- 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 @@
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 @@
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 @@
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)
}

To view, visit change 433135. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: vulndb
Gerrit-Branch: master
Gerrit-Change-Id: I6f12993ff2289e38584e46e2f4382e76f5ad639f
Gerrit-Change-Number: 433135
Gerrit-PatchSet: 4
Gerrit-Owner: Tatiana Bradley <tat...@golang.org>
Gerrit-Reviewer: Gopher Robot <go...@golang.org>
Gerrit-Reviewer: Jonathan Amsterdam <j...@google.com>
Gerrit-Reviewer: Tatiana Bradley <tat...@golang.org>
Gerrit-MessageType: merged
Reply all
Reply to author
Forward
0 new messages