From f762043b08a4fed697438bebcc33c3e9074d4b1f Mon Sep 17 00:00:00 2001 From: Tatiana Bradley Date: Thu, 25 May 2023 11:11:56 -0400 Subject: [PATCH] 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 Run-TryBot: Tatiana Bradley Reviewed-by: Damien Neil --- all_test.go | 9 ++-- cmd/vulnreport/main.go | 87 ++++++++++++++++------------------ internal/report/cve5.go | 13 ++--- internal/report/cve5_test.go | 9 ++-- internal/report/osv.go | 14 +++--- internal/report/osv_test.go | 8 ++-- internal/report/report_test.go | 2 +- 7 files changed, 70 insertions(+), 72 deletions(-) diff --git a/all_test.go b/all_test.go index b690938b..57ba7d3f 100644 --- a/all_test.go +++ b/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) diff --git a/cmd/vulnreport/main.go b/cmd/vulnreport/main.go index a841f351..669bc097 100644 --- a/cmd/vulnreport/main.go +++ b/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 } diff --git a/internal/report/cve5.go b/internal/report/cve5.go index 8dfb7ef3..8517fec5 100644 --- a/internal/report/cve5.go +++ b/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 { diff --git a/internal/report/cve5_test.go b/internal/report/cve5_test.go index 4ed02ace..4db3c7bd 100644 --- a/internal/report/cve5_test.go +++ b/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) } } diff --git a/internal/report/osv.go b/internal/report/osv.go index 47284f92..3b21359b 100644 --- a/internal/report/osv.go +++ b/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. diff --git a/internal/report/osv_test.go b/internal/report/osv_test.go index a2c1d97b..479295f0 100644 --- a/internal/report/osv_test.go +++ b/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) } } diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 7606fc2b..1e5ad7d5 100644 --- a/internal/report/report_test.go +++ b/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