cmd/vulnreport: add support for skipping issues Add new flag `-skip-issues` that is a whitespace-delimited list of GitHub issues to ignore in commands: triage, create, create-excluded, and commit. Change-Id: I93823abfda3d67e51b905b7964f9086ce430ef15 Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/825884 Reviewed-by: Nicholas Husin <husin@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Nicholas Husin <nsh@golang.org>
diff --git a/cmd/vulnreport/commit.go b/cmd/vulnreport/commit.go index 967f562..835d461 100644 --- a/cmd/vulnreport/commit.go +++ b/cmd/vulnreport/commit.go
@@ -12,13 +12,16 @@ "fmt" "path/filepath" "slices" + "strconv" "strings" - "github.com/go-git/go-git/v5" - "golang.org/x/exp/maps" "golang.org/x/vulndb/cmd/vulnreport/log" "golang.org/x/vulndb/internal/gitrepo" "golang.org/x/vulndb/internal/report" + + "golang.org/x/exp/maps" + + "github.com/go-git/go-git/v5" ) var ( @@ -104,6 +107,13 @@ func (c *commit) skip(input any) string { r := input.(*yamlReport) + iss, err := parseReportIssue(r.ID) + if err != nil { + return err.Error() + } + if slices.Contains(skippedIssues, iss) { + return "skipping at user request" + } if c.reviewStatus == 0 { return "" } @@ -117,6 +127,24 @@ return "" } +func parseReportIssue(id string) (int, error) { + parts := strings.Split(id, "-") + if len(parts) != 3 || parts[0] != "GO" { + return 0, fmt.Errorf("id %s is not of the form GO-YYYY-XXX", id) + } + if len(parts[1]) != 4 { + return 0, fmt.Errorf("id %s is not of the form GO-YYYY-XXX", id) + } + if _, err := strconv.Atoi(parts[1]); err != nil { + return 0, fmt.Errorf("id %s is not of the form GO-YYYY-XXX", id) + } + iss, err := strconv.Atoi(parts[2]) + if err != nil { + return 0, fmt.Errorf("id %s is not of the form GO-YYYY-XXX", id) + } + return iss, nil +} + func (c *commit) run(ctx context.Context, input any) error { r := input.(*yamlReport)
diff --git a/cmd/vulnreport/creator.go b/cmd/vulnreport/creator.go index 0344f2b..9d203f7 100644 --- a/cmd/vulnreport/creator.go +++ b/cmd/vulnreport/creator.go
@@ -59,6 +59,10 @@ func (c *creator) skip(input any) string { iss := input.(*issues.Issue) + if slices.Contains(skippedIssues, iss.Number) { + return "skipping at user request" + } + if c.assignee != "" && iss.Assignee != c.assignee { return fmt.Sprintf("assignee = %q, not %q", iss.Assignee, c.assignee) }
diff --git a/cmd/vulnreport/main.go b/cmd/vulnreport/main.go index 21ecd98..5c0bc86 100644 --- a/cmd/vulnreport/main.go +++ b/cmd/vulnreport/main.go
@@ -13,31 +13,51 @@ "log" "os" "runtime/pprof" + "strconv" + "strings" "text/tabwriter" vlog "golang.org/x/vulndb/cmd/vulnreport/log" ) var ( - githubToken = flag.String("ghtoken", "", "GitHub access token (default: value of VULN_GITHUB_ACCESS_TOKEN)") - cpuprofile = flag.String("cpuprofile", "", "write cpuprofile to this file") - quiet = flag.Bool("q", false, "quiet mode (suppress info logs)") - colorize = flag.Bool("color", os.Getenv("NO_COLOR") == "", "show colors in logs") - issueRepo = flag.String("issue-repo", "github.com/golang/vulndb", "repo to locate Github issues") - reportRepo = flag.String("local-repo", ".", "local path to repo to locate YAML reports") + githubToken = flag.String("ghtoken", "", "GitHub access token (default: value of VULN_GITHUB_ACCESS_TOKEN)") + cpuprofile = flag.String("cpuprofile", "", "write cpuprofile to this file") + quiet = flag.Bool("q", false, "quiet mode (suppress info logs)") + colorize = flag.Bool("color", os.Getenv("NO_COLOR") == "", "show colors in logs") + issueRepo = flag.String("issue-repo", "github.com/golang/vulndb", "repo to locate GitHub issues") + reportRepo = flag.String("local-repo", ".", "local path to repo to locate YAML reports") + skippedIssues []int ) func init() { + flag.Func("skip-issues", "for triage, create, create-excluded, and commit, whitespace-delimited list of GitHub issues to skip", func(s string) error { + is, err := parseSkipIssues(s) + if err != nil { + return err + } + skippedIssues = append(skippedIssues, is...) + return nil + }) + out := flag.CommandLine.Output() flag.Usage = func() { - fmt.Fprintf(out, "usage: vulnreport [flags] [cmd] [args]\n\n") + if _, err := fmt.Fprintf(out, "usage: vulnreport [flags] [cmd] [args]\n\n"); err != nil { + panic(err) + } tw := tabwriter.NewWriter(out, 2, 4, 2, ' ', 0) for _, command := range commands { argUsage, desc := command.usage() - fmt.Fprintf(tw, " %s\t%s\t%s\n", command.name(), argUsage, desc) + if _, err := fmt.Fprintf(tw, " %s\t%s\t%s\n", command.name(), argUsage, desc); err != nil { + panic(err) + } } - tw.Flush() - fmt.Fprint(out, "\nsupported flags:\n\n") + if err := tw.Flush(); err != nil { + panic(err) + } + if _, err := fmt.Fprint(out, "\nsupported flags:\n\n"); err != nil { + panic(err) + } flag.PrintDefaults() } } @@ -107,3 +127,15 @@ log.Fatalf("%s: %s", cmdName, err) } } + +func parseSkipIssues(s string) ([]int, error) { + skipped := []int{} + for part := range strings.FieldsSeq(s) { + num, err := strconv.Atoi(part) + if err != nil { + return nil, err + } + skipped = append(skipped, num) + } + return skipped, nil +}
diff --git a/cmd/vulnreport/triage.go b/cmd/vulnreport/triage.go index e082e1a..d907875 100644 --- a/cmd/vulnreport/triage.go +++ b/cmd/vulnreport/triage.go
@@ -89,6 +89,9 @@ func (t *triage) skip(input any) string { iss := input.(*issues.Issue) + if slices.Contains(skippedIssues, iss.Number) { + return "skipping at user request" + } if iss.HasLabel(labelDirect) { return "direct external report" }
diff --git a/cmd/vulnreport/vulnreport_test.go b/cmd/vulnreport/vulnreport_test.go index 602f4f2..bed052b 100644 --- a/cmd/vulnreport/vulnreport_test.go +++ b/cmd/vulnreport/vulnreport_test.go
@@ -5,9 +5,12 @@ package main import ( + "fmt" "testing" + "github.com/google/go-cmp/cmp" "golang.org/x/vulndb/internal/issues" + "golang.org/x/vulndb/internal/report" ) func TestCreate(t *testing.T) { @@ -191,6 +194,250 @@ } } +func TestParseSkipIssues(t *testing.T) { + testCases := []struct { + name string + input string + want []int + wantErr bool + }{ + { + name: "empty string", + input: "", + want: []int{}, + wantErr: false, + }, + { + name: "just whitespace", + input: " \t\n ", + want: []int{}, + wantErr: false, + }, + { + name: "single issue", + input: "100", + want: []int{100}, + wantErr: false, + }, + { + name: "multiple issues", + input: "100 200 300", + want: []int{100, 200, 300}, + wantErr: false, + }, + { + name: "extra whitespace around issue ids", + input: " 100 200 \t 300 ", + want: []int{100, 200, 300}, + wantErr: false, + }, + { + name: "invalid non-integer", + input: "abc", + want: nil, + wantErr: true, + }, + { + name: "mixed valid and invalid", + input: "100 abc 200", + want: nil, + wantErr: true, + }, + { + name: "comma-separated is invalid", + input: "100,200,300", + want: nil, + wantErr: true, + }, + { + name: "floating point number", + input: "12.34", + want: nil, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := parseSkipIssues(tc.input) + if (err != nil) != tc.wantErr { + t.Fatalf("parseSkipIssues(%q) error = %v, wantErr %v", tc.input, err, tc.wantErr) + } + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("parseSkipIssues(%q) mismatch (-want +got):\n%s", tc.input, diff) + } + }) + } +} + +func TestSkipIssues(t *testing.T) { + labelExcluded := report.ExcludedNotGoCode.ToLabel() + + cmds := []struct { + name string + cmd command + input func(issueNum int) any + }{ + { + name: "create (without args)", + cmd: &create{creator: &creator{}, issueParser: &issueParser{}, hasArgs: false}, + input: func(n int) any { + return &issues.Issue{Number: n, State: "open"} + }, + }, + { + name: "create (with args)", + cmd: &create{creator: &creator{}, issueParser: &issueParser{}, hasArgs: true}, + input: func(n int) any { + return &issues.Issue{Number: n, State: "open"} + }, + }, + { + name: "create-excluded", + cmd: &createExcluded{creator: &creator{}}, + input: func(n int) any { + return &issues.Issue{Number: n, State: "open", Labels: []string{labelExcluded}} + }, + }, + { + name: "triage", + cmd: &triage{}, + input: func(n int) any { + return &issues.Issue{Number: n, State: "open"} + }, + }, + { + name: "commit", + cmd: &commit{}, + input: func(n int) any { + return &yamlReport{Report: &report.Report{ID: fmt.Sprintf("GO-2024-%04d", n)}} + }, + }, + } + + testCases := []struct { + name string + skippedIssues []int + issueNum int + want string + }{ + { + name: "issue in skipped list", + skippedIssues: []int{100, 200}, + issueNum: 100, + want: "skipping at user request", + }, + { + name: "second issue in skipped list", + skippedIssues: []int{100, 200}, + issueNum: 200, + want: "skipping at user request", + }, + { + name: "issue not in skipped list", + skippedIssues: []int{100, 200}, + issueNum: 300, + want: "", + }, + { + name: "empty skipped list", + skippedIssues: nil, + issueNum: 100, + want: "", + }, + } + + for _, c := range cmds { + for _, tc := range testCases { + t.Run(c.name+"/"+tc.name, func(t *testing.T) { + oldSkipped := skippedIssues + skippedIssues = tc.skippedIssues + defer func() { skippedIssues = oldSkipped }() + + if got := c.cmd.skip(c.input(tc.issueNum)); got != tc.want { + t.Errorf("%s: skip() = %q, want %q", c.name, got, tc.want) + } + }) + } + } +} + +func TestParseReportIssue(t *testing.T) { + testCases := []struct { + name string + id string + wantIss int + wantErr bool + }{ + { + name: "valid", + id: "GO-2024-0100", + wantIss: 100, + wantErr: false, + }, + { + name: "valid single digit", + id: "GO-2024-1", + wantIss: 1, + wantErr: false, + }, + { + name: "GO-ID-PENDING", + id: "GO-ID-PENDING", + wantIss: 0, + wantErr: true, + }, + { + name: "non-numeric issue", + id: "GO-2024-PENDING", + wantIss: 0, + wantErr: true, + }, + { + name: "too few parts", + id: "GO-100", + wantIss: 0, + wantErr: true, + }, + { + name: "too many parts", + id: "GO-2024-100-1", + wantIss: 0, + wantErr: true, + }, + { + name: "wrong prefix", + id: "CVE-2024-100", + wantIss: 0, + wantErr: true, + }, + { + name: "non-numeric year", + id: "GO-YYYY-100", + wantIss: 0, + wantErr: true, + }, + { + name: "year not 4 digits", + id: "GO-24-100", + wantIss: 0, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gotIss, err := parseReportIssue(tc.id) + if (err != nil) != tc.wantErr { + t.Fatalf("parseReportIssue(%q) err = %v, wantErr %v", tc.id, err, tc.wantErr) + } + if gotIss != tc.wantIss { + t.Errorf("parseReportIssue(%q) = %d, want %d", tc.id, gotIss, tc.wantIss) + } + }) + } +} + func TestCreateExcluded(t *testing.T) { for _, tc := range []*testCase{ // TODO(tatianabradley): add test cases