cmd/vulnreport/priority: take unexcluded reports into account

When using the existing corpus to prioritize new vulnerabilities,
treat unreviewed unexcluded reports the same as likely-binary excluded
reports.

Change-Id: Id803d05ecd33b4486086acac8ff124977b3725ef
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/592777
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

cmd/vulnreport/priority/priority.go

@@ -69,14 +69,16 @@ func priority(mp string, importers int, sc map[reportState]int) (priority Priori
 	}
 
 	if importers >= highPriority {
+		rev := sc[reviewed]
+		binary := sc[excludedBinary] + sc[unreviewedUnexcluded]
 		getReason := func(conj1, conj2 string) string {
-			return fmt.Sprintf("%s %s reviewed (%d) %s likely-binary excluded reports (%d)",
-				importersStr(">="), conj1, sc[reviewed], conj2, sc[excludedBinary])
+			return fmt.Sprintf("%s %s reviewed (%d) %s likely-binary reports (%d)",
+				importersStr(">="), conj1, rev, conj2, binary)
 		}
 
-		if sc[reviewed] > sc[excludedBinary] {
+		if rev > binary {
 			return High, getReason("and more", "than")
-		} else if sc[reviewed] == sc[excludedBinary] {
+		} else if rev == binary {
 			return High, getReason("and as many", "as")
 		}
 
@@ -98,39 +100,48 @@ type reportState int
 const (
 	unknownReportState reportState = iota
 	reviewed
-	unreviewed
+	unreviewedStandard
+	unreviewedUnexcluded
 	excludedBinary
 	excludedNotGo
 	excludedOther
 )
 
+func state(r *report.Report) reportState {
+	if r.IsExcluded() {
+		switch e := r.Excluded; e {
+		case "NOT_GO_CODE":
+			return excludedNotGo
+		case "EFFECTIVELY_PRIVATE", "NOT_IMPORTABLE", "LEGACY_FALSE_POSITIVE":
+			return excludedBinary
+		case "NOT_A_VULNERABILITY", "DEPENDENT_VULNERABILITY":
+			return excludedOther
+		default:
+			return unknownReportState
+		}
+	}
+
+	switch rs := r.ReviewStatus; rs {
+	case report.Reviewed:
+		return reviewed
+	case report.Unreviewed:
+		if r.Unexcluded != "" {
+			return unreviewedUnexcluded
+		}
+		return unreviewedStandard
+	}
+
+	return unknownReportState
+}
+
 func stateCounts(rs []*report.Report) map[reportState]int {
 	counts := make(map[reportState]int)
 	for _, r := range rs {
-		var state reportState
-		switch {
-		case r.IsExcluded():
-			switch e := r.Excluded; e {
-			case "NOT_GO_CODE":
-				state = excludedNotGo
-			case "EFFECTIVELY_PRIVATE", "NOT_IMPORTABLE", "LEGACY_FALSE_POSITIVE":
-				state = excludedBinary
-			case "NOT_A_VULNERABILITY", "DEPENDENT_VULNERABILITY":
-				state = excludedOther
-			default:
-				panic("unknown excluded reason " + e)
-			}
-		default:
-			switch rs := r.ReviewStatus; rs {
-			case report.Reviewed:
-				state = reviewed
-			case report.Unreviewed:
-				state = unreviewed
-			default:
-				panic("unknown review status " + rs.String())
-			}
+		st := state(r)
+		if st == unknownReportState {
+			panic("could not determine report state for " + r.ID)
 		}
-		counts[state]++
+		counts[st]++
 	}
 	return counts
 }

cmd/vulnreport/priority/priority_test.go

@@ -21,6 +21,10 @@ var (
 	reviewed2 = &report.Report{
 		ReviewStatus: report.Reviewed,
 	}
+	reviewedBinary = &report.Report{
+		ReviewStatus: report.Reviewed,
+		Unexcluded:   "NOT_IMPORTABLE",
+	}
 	unreviewed1 = &report.Report{
 		ReviewStatus: report.Unreviewed,
 	}
@@ -33,6 +37,10 @@ var (
 	binary3 = &report.Report{
 		Excluded: "LEGACY_FALSE_POSITIVE",
 	}
+	unreviewedBinary = &report.Report{
+		ReviewStatus: report.Unreviewed,
+		Unexcluded:   "NOT_IMPORTABLE",
+	}
 	notAVuln1 = &report.Report{
 		Excluded: "NOT_A_VULNERABILITY",
 	}
@@ -75,7 +83,7 @@ func TestAnalyze(t *testing.T) {
 			modulesToImports: map[string]int{"example.com/module": 100},
 			want: &Result{
 				Priority: High,
-				Reason:   "example.com/module has 100 importers (>= 100) and as many reviewed (0) as likely-binary excluded reports (0)",
+				Reason:   "example.com/module has 100 importers (>= 100) and as many reviewed (0) as likely-binary reports (0)",
 			},
 		},
 		{
@@ -85,7 +93,7 @@ func TestAnalyze(t *testing.T) {
 			modulesToImports: map[string]int{"example.com/module": 101},
 			want: &Result{
 				Priority: High,
-				Reason:   "example.com/module has 101 importers (>= 100) and more reviewed (2) than likely-binary excluded reports (1)",
+				Reason:   "example.com/module has 101 importers (>= 100) and more reviewed (2) than likely-binary reports (1)",
 			},
 		},
 		{
@@ -99,7 +107,21 @@ func TestAnalyze(t *testing.T) {
 			modulesToImports: map[string]int{"example.com/module": 101},
 			want: &Result{
 				Priority: Low,
-				Reason:   "example.com/module has 101 importers (>= 100) but fewer reviewed (1) than likely-binary excluded reports (3)",
+				Reason:   "example.com/module has 101 importers (>= 100) but fewer reviewed (1) than likely-binary reports (3)",
+			},
+		},
+		{
+			name:   "unexcluded unreviewed considered binaries",
+			module: "example.com/module",
+			reportsForModule: []*report.Report{
+				reviewed1, reviewedBinary, // reviewed
+				binary1, binary2, unreviewedBinary, // binary
+				unreviewed1, notAVuln1, dependent1, // ignored
+			},
+			modulesToImports: map[string]int{"example.com/module": 101},
+			want: &Result{
+				Priority: Low,
+				Reason:   "example.com/module has 101 importers (>= 100) but fewer reviewed (2) than likely-binary reports (3)",
 			},
 		},
 		{

cmd/vulnreport/testdata/TestTriage/all.txtar

@@ -9,7 +9,7 @@ command: "vulnreport triage "
 issue test-issue-tracker/7 is likely duplicate
   - #7 shares alias(es) CVE-9999-0005 with data/reports/GO-9999-0005.yaml
 issue test-issue-tracker/10 is high priority
-  - golang.org/x/vuln has 101 importers (>= 100) and as many reviewed (0) as likely-binary excluded reports (0)
+  - golang.org/x/vuln has 101 importers (>= 100) and as many reviewed (0) as likely-binary reports (0)
 issue test-issue-tracker/11 is possibly not Go
   - more than 20 percent of reports (1 of 1) with this module are NOT_GO_CODE
 triaged 3 issues: