cmd/vulnreport,internal/report: cleanup filename functions

Now that the ID is in the YAML report, we don't need to call GoID() as
often, and we don't need to pass the Go ID to YAMLFilename or CVEFilename.

Change-Id: I80c161a3be47a54d97837e4d68e789f166c8907b
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/498282
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Damien Neil <dneil@google.com>

all_test.go

@@ -84,7 +84,6 @@ func TestLintReports(t *testing.T) {
 			if len(lints) > 0 {
 				t.Errorf(strings.Join(lints, "\n"))
 			}
-			goID := report.GoID(filename)
 			for _, alias := range r.Aliases() {
 				if report, ok := aliases[alias]; ok {
 					t.Errorf("report %s shares duplicate alias %s with report %s", filename, alias, report)
@@ -94,8 +93,8 @@ func TestLintReports(t *testing.T) {
 			}
 			// Check that a correct OSV file was generated for each YAML report.
 			if r.Excluded == "" {
-				generated := r.ToOSV(goID, time.Time{})
-				osvFilename := report.OSVFilename(goID)
+				generated := r.ToOSV(time.Time{})
+				osvFilename := r.OSVFilename()
 				current, err := report.ReadOSV(osvFilename)
 				if err != nil {
 					t.Fatal(err)
@@ -108,11 +107,11 @@ func TestLintReports(t *testing.T) {
 				}
 			}
 			if r.CVEMetadata != nil {
-				generated, err := r.ToCVE5(goID)
+				generated, err := r.ToCVE5()
 				if err != nil {
 					t.Fatal(err)
 				}
-				cvePath := report.CVEFilename(goID)
+				cvePath := r.CVEFilename()
 				current, err := cveschema5.Read(cvePath)
 				if err != nil {
 					t.Fatal(err)

cmd/vulnreport/main.go

@@ -32,7 +32,6 @@ import (
 	"golang.org/x/exp/slices"
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/vulndb/internal/cvelistrepo"
-	"golang.org/x/vulndb/internal/cveschema5"
 	"golang.org/x/vulndb/internal/database"
 	"golang.org/x/vulndb/internal/derrors"
 	"golang.org/x/vulndb/internal/ghsa"
@@ -367,7 +366,7 @@ func handleExcludedIssue(ctx context.Context, cfg *createCfg, iss *issues.Issue)
 	if err := irun("git", "add", filename); err != nil {
 		return "", fmt.Errorf("git add %s: %v", filename, err)
 	}
-	return filename, nil
+	return r.ID, nil
 }
 
 func createExcluded(ctx context.Context, cfg *createCfg) (err error) {
@@ -398,14 +397,14 @@ func createExcluded(ctx context.Context, cfg *createCfg) (err error) {
 			skipped++
 			continue
 		}
-		filename, err := handleExcludedIssue(ctx, cfg, iss)
+		id, err := handleExcludedIssue(ctx, cfg, iss)
 		if err != nil {
 			fmt.Printf("skipped issue %d due to error: %v\n", iss.Number, err)
 			skipped++
 			continue
 		}
 		successfulIssNums = append(successfulIssNums, fmt.Sprintf("golang/vulndb#%d", iss.Number))
-		successfulGoIDs = append(successfulGoIDs, report.GoID(filename))
+		successfulGoIDs = append(successfulGoIDs, id)
 	}
 	fmt.Printf("Skipped %d issues\n", skipped)
 
@@ -695,13 +694,19 @@ func fix(ctx context.Context, filename string, ghsaClient *ghsa.Client) (err err
 	if err := r.Write(filename); err != nil {
 		return err
 	}
-	goID := report.GoID(filename)
-	if _, err := writeOSV(r, goID); err != nil {
-		return err
+
+	if !r.IsExcluded() {
+		if err := writeOSV(r); err != nil {
+			return err
+		}
 	}
-	if err := writeCVE(r, goID); err != nil {
-		return err
+
+	if r.CVEMetadata != nil {
+		if err := writeCVE(r); err != nil {
+			return err
+		}
 	}
+
 	return nil
 }
 
@@ -874,24 +879,17 @@ func osvCmd(filename string) (err error) {
 	if err != nil {
 		return err
 	}
-	osvFilename, err := writeOSV(r, report.GoID(filename))
-	if err != nil {
-		return err
+	if !r.IsExcluded() {
+		if err := writeOSV(r); err != nil {
+			return err
+		}
+		fmt.Println(r.OSVFilename())
 	}
-	fmt.Println(osvFilename)
 	return nil
 }
 
-func writeOSV(r *report.Report, goID string) (string, error) {
-	if r.Excluded == "" {
-		entry := r.ToOSV(goID, time.Time{})
-		osvFilename := report.OSVFilename(goID)
-		if err := database.WriteJSON(osvFilename, entry, true); err != nil {
-			return "", err
-		}
-		return osvFilename, nil
-	}
-	return "", nil
+func writeOSV(r *report.Report) error {
+	return database.WriteJSON(r.OSVFilename(), r.ToOSV(time.Time{}), true)
 }
 
 func cveCmd(ctx context.Context, filename string) (err error) {
@@ -900,27 +898,20 @@ func cveCmd(ctx context.Context, filename string) (err error) {
 	if err != nil {
 		return err
 	}
-	return writeCVE(r, report.GoID(filename))
+	if r.CVEMetadata != nil {
+		return writeCVE(r)
+	}
+	return nil
 }
 
-// writeCVE takes a report and its Go ID, converts the report
-// into a JSON CVE5 record and writes it to data/cve/v5.
-func writeCVE(r *report.Report, goID string) error {
-	if r.CVEMetadata == nil {
-		return nil
-	}
-	var cve *cveschema5.CVERecord
-	var err error
-
-	cvePath := report.CVEFilename(goID)
-	if cve, err = r.ToCVE5(goID); err != nil {
+// writeCVE converts a report to JSON CVE5 record and writes it to
+// data/cve/v5.
+func writeCVE(r *report.Report) error {
+	cve, err := r.ToCVE5()
+	if err != nil {
 		return err
 	}
-	if err = database.WriteJSON(cvePath, cve, true); err != nil {
-		return err
-	}
-
-	return nil
+	return database.WriteJSON(r.CVEFilename(), cve, true)
 }
 
 func irun(name string, arg ...string) error {
@@ -952,12 +943,11 @@ func commit(ctx context.Context, filename string, ghsaClient *ghsa.Client) (err
 
 	// Find all derived files (OSV and CVE).
 	files := []string{filename}
-	goID := report.GoID(filename)
 	if r.Excluded == "" {
-		files = append(files, report.OSVFilename(goID))
+		files = append(files, r.OSVFilename())
 	}
 	if r.CVEMetadata != nil {
-		files = append(files, report.CVEFilename(goID))
+		files = append(files, r.CVEFilename())
 	}
 
 	// Add the files.
@@ -969,7 +959,7 @@ func commit(ctx context.Context, filename string, ghsaClient *ghsa.Client) (err
 	}
 
 	// Commit the files, allowing the user to edit the default commit message.
-	msg, err := newCommitMsg(r, filename)
+	msg, err := newCommitMsg(r)
 	if err != nil {
 		return err
 	}
@@ -983,8 +973,13 @@ func commit(ctx context.Context, filename string, ghsaClient *ghsa.Client) (err
 	return nil
 }
 
-func newCommitMsg(r *report.Report, filepath string) (string, error) {
-	folder, filename, issueID, err := report.ParseFilepath(filepath)
+func newCommitMsg(r *report.Report) (string, error) {
+	f, err := r.YAMLFilename()
+	if err != nil {
+		return "", err
+	}
+
+	folder, filename, issueID, err := report.ParseFilepath(f)
 	if err != nil {
 		return "", err
 	}

internal/report/cve5.go

@@ -24,8 +24,8 @@ var (
 )
 
 // ToCVE5 creates a CVE in 5.0 format from a YAML report file.
-func (r *Report) ToCVE5(goID string) (_ *cveschema5.CVERecord, err error) {
-	defer derrors.Wrap(&err, "Report.ToCVERecord(%q)", goID)
+func (r *Report) ToCVE5() (_ *cveschema5.CVERecord, err error) {
+	defer derrors.Wrap(&err, "ToCVERecord(%q)", r.ID)
 
 	if len(r.CVEs) > 0 {
 		return nil, errors.New("report has CVE ID is wrong section (should be in cve_metadata for self-issued CVEs)")
@@ -93,8 +93,9 @@ func (r *Report) ToCVE5(goID string) (_ *cveschema5.CVERecord, err error) {
 	for _, ref := range r.References {
 		c.References = append(c.References, cveschema5.Reference{URL: ref.URL})
 	}
-	advisoryLink := GoAdvisory(goID)
-	c.References = append(c.References, cveschema5.Reference{URL: advisoryLink})
+	c.References = append(c.References, cveschema5.Reference{
+		URL: GoAdvisory(r.ID),
+	})
 
 	for _, credit := range r.Credits {
 		c.Credits = append(c.Credits, cveschema5.Credit{
@@ -115,8 +116,8 @@ func (r *Report) ToCVE5(goID string) (_ *cveschema5.CVERecord, err error) {
 	}, nil
 }
 
-func CVEFilename(goID string) string {
-	return filepath.Join(cve5Dir, goID+".json")
+func (r *Report) CVEFilename() string {
+	return filepath.Join(cve5Dir, r.ID+".json")
 }
 
 func versionRangeToVersionRange(versions []VersionRange) []cveschema5.VersionRange {

internal/report/cve5_test.go

@@ -263,12 +263,12 @@ func TestToCVE5(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			got, err := r.ToCVE5(GoID(test.filename))
+			got, err := r.ToCVE5()
 			if err != nil {
-				t.Fatalf("ToCVE5(%s) failed unexpectedly; err=%v", test.filename, err)
+				t.Fatalf("ToCVE5() failed unexpectedly; err=%v", err)
 			}
 			if diff := cmp.Diff(test.want, got); diff != "" {
-				t.Fatalf("ToCVE5(%s): unexpected diffs (-want,+got):\n%v", test.filename, diff)
+				t.Fatalf("ToCVE5(): unexpected diffs (-want,+got):\n%v", diff)
 			}
 		})
 	}
@@ -276,7 +276,8 @@ func TestToCVE5(t *testing.T) {
 
 func TestCVEFilename(t *testing.T) {
 	want := "data/cve/v5/GO-1999-0001.json"
-	if got := CVEFilename("GO-1999-0001"); got != want {
+	r := &Report{ID: "GO-1999-0001"}
+	if got := r.CVEFilename(); got != want {
 		t.Errorf("got %s, want %s", got, want)
 	}
 }

internal/report/osv.go

@@ -30,9 +30,9 @@ var (
 )
 
 // ToOSV creates an osv.Entry for a report.
-// In addition to the report, it takes the ID for the vuln and the time
-// the vuln was last modified.
-func (r *Report) ToOSV(goID string, lastModified time.Time) osv.Entry {
+// lastModified is the time the report should be considered to have
+// been most recently modified.
+func (r *Report) ToOSV(lastModified time.Time) osv.Entry {
 	var credits []osv.Credit
 	for _, credit := range r.Credits {
 		credits = append(credits, osv.Credit{
@@ -46,14 +46,14 @@ func (r *Report) ToOSV(goID string, lastModified time.Time) osv.Entry {
 	}
 
 	entry := osv.Entry{
-		ID:               goID,
+		ID:               r.ID,
 		Published:        osv.Time{Time: r.Published},
 		Modified:         osv.Time{Time: lastModified},
 		Withdrawn:        withdrawn,
 		Details:          trimWhitespace(r.Description),
 		Credits:          credits,
 		SchemaVersion:    SchemaVersion,
-		DatabaseSpecific: &osv.DatabaseSpecific{URL: GoAdvisory(goID)},
+		DatabaseSpecific: &osv.DatabaseSpecific{URL: GoAdvisory(r.ID)},
 	}
 
 	for _, m := range r.Modules {
@@ -69,8 +69,8 @@ func (r *Report) ToOSV(goID string, lastModified time.Time) osv.Entry {
 	return entry
 }
 
-func OSVFilename(goID string) string {
-	return filepath.Join(OSVDir, goID+".json")
+func (r *Report) OSVFilename() string {
+	return filepath.Join(OSVDir, r.ID+".json")
 }
 
 // ReadOSV reads an osv.Entry from a file.

internal/report/osv_test.go

@@ -15,6 +15,7 @@ import (
 
 func TestToOSV(t *testing.T) {
 	r := &Report{
+		ID: "GO-1991-0001",
 		Modules: []*Module{
 			{
 				Module: "example.com/vulnerable/v2",
@@ -200,15 +201,16 @@ func TestToOSV(t *testing.T) {
 		DatabaseSpecific: &osv.DatabaseSpecific{URL: "https://pkg.go.dev/vuln/GO-1991-0001"},
 	}
 
-	gotEntry := r.ToOSV("GO-1991-0001", time.Time{})
+	gotEntry := r.ToOSV(time.Time{})
 	if diff := cmp.Diff(wantEntry, gotEntry, cmp.Comparer(func(a, b time.Time) bool { return a.Equal(b) })); diff != "" {
-		t.Errorf("GenerateOSVEntry returned unexpected entry (-want +got):\n%s", diff)
+		t.Errorf("ToOSV() mismatch (-want +got):\n%s", diff)
 	}
 }
 
 func TestOSVFilename(t *testing.T) {
 	want := "data/osv/GO-1999-0001.json"
-	if got := OSVFilename("GO-1999-0001"); got != want {
+	r := &Report{ID: "GO-1999-0001"}
+	if got := r.OSVFilename(); got != want {
 		t.Errorf("got %s, want %s", got, want)
 	}
 }

internal/report/report_test.go

@@ -46,7 +46,7 @@ func TestUnknownField(t *testing.T) {
 	}
 }
 
-func TestGetYAMLFilename(t *testing.T) {
+func TestYAMLFilename(t *testing.T) {
 	tests := []struct {
 		name string
 		r    *Report