cmd/vulnreport: automatically decide whether to create REVIEWED or UNREVIEWED report
Command vulnreport create now decides whether to generate a REVIEWED
or UNREVIEWED report based on issue's labels.
This can be overridden with flag "-status=<REVIEW_STATUS>". The "unreviewed"
flag is removed.
Change-Id: I8f8b808c6f9bbcaeb0dc176fb6cb875b8f9ccee4
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/587976
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/cmd/issue/main.go b/cmd/issue/main.go
index b1b9cfb..b05d883 100644
--- a/cmd/issue/main.go
+++ b/cmd/issue/main.go
@@ -108,7 +108,7 @@
aliases := []string{arg}
packages := []string{"<placeholder>"}
bodies := []string{fmt.Sprintf("This is a placeholder issue for %q.", arg)}
- if err := publishIssue(ctx, c, packages, aliases, bodies, []string{}); err != nil {
+ if err := publishIssue(ctx, c, packages, aliases, bodies, []string{"first party"}); err != nil {
return err
}
}
diff --git a/cmd/vulnreport/create.go b/cmd/vulnreport/create.go
index 9d0ece0..d1dbff4 100644
--- a/cmd/vulnreport/create.go
+++ b/cmd/vulnreport/create.go
@@ -16,7 +16,7 @@
useAI = flag.Bool("ai", false, "for create, use AI to write draft summary and description when creating report")
populateSymbols = flag.Bool("symbols", false, "for create, attempt to auto-populate symbols")
user = flag.String("user", "", "for create & create-excluded, only consider issues assigned to the given user")
- unreviewed = flag.Bool("unreviewed", false, "for create, create issues in unreviewed state")
+ reviewStatus = flag.String("status", "", "for create, use this review status (REVIEWED or UNREVIEWED) instead of default based on label")
)
type create struct {
diff --git a/cmd/vulnreport/creator.go b/cmd/vulnreport/creator.go
index d7901a8..aa2ef88 100644
--- a/cmd/vulnreport/creator.go
+++ b/cmd/vulnreport/creator.go
@@ -22,6 +22,10 @@
assignee string
created []*report.Report
+ // If non-zero, use this review status
+ // instead of the default for new reports.
+ reviewStatus report.ReviewStatus
+
*fixer
*xrefer
*suggester
@@ -34,6 +38,12 @@
}
c.assignee = user
+ rs, ok := report.ToReviewStatus(*reviewStatus)
+ if !ok {
+ return fmt.Errorf("invalid -status=%s", rs)
+ }
+ c.reviewStatus = rs
+
c.fixer = new(fixer)
c.xrefer = new(xrefer)
if *useAI {
@@ -52,15 +62,6 @@
return fmt.Sprintf("assignee = %q, not %q", iss.Assignee, c.assignee)
}
- if *unreviewed {
- if iss.HasLabel(labelDirect) {
- return "should not create unreviewed report for direct report"
- }
- if iss.HasLabel(labelHighPriority) {
- return "should not create unreviewed report for high priority issue"
- }
- }
-
return c.xrefer.skipReason(iss)
}
@@ -90,14 +91,36 @@
func (c *creator) reportFromIssue(ctx context.Context, iss *issues.Issue) error {
return c.reportFromMeta(ctx, &reportMeta{
- id: iss.NewGoID(),
- excluded: excludedReason(iss),
- modulePath: modulePath(iss),
- aliases: aliases(iss),
- unreviewed: *unreviewed,
+ id: iss.NewGoID(),
+ excluded: excludedReason(iss),
+ modulePath: modulePath(iss),
+ aliases: aliases(iss),
+ reviewStatus: reviewStatusOf(iss, c.reviewStatus),
})
}
+func reviewStatusOf(iss *issues.Issue, reviewStatus report.ReviewStatus) report.ReviewStatus {
+ d := defaultReviewStatus(iss)
+ // If a valid review status is provided, it overrides the priority label.
+ if reviewStatus != 0 {
+ if d != reviewStatus {
+ log.Warnf("issue %d should be %s based on label(s) but this was overridden with the -status=%s flag", iss.Number, d, reviewStatus)
+ }
+ return reviewStatus
+ }
+ return d
+}
+
+func defaultReviewStatus(iss *issues.Issue) report.ReviewStatus {
+ if iss.HasLabel(labelHighPriority) ||
+ iss.HasLabel(labelDirect) ||
+ iss.HasLabel(labelFirstParty) {
+ return report.Reviewed
+ }
+
+ return report.Unreviewed
+}
+
func (c *creator) reportFromMeta(ctx context.Context, meta *reportMeta) error {
// Find the underlying module if the "module" provided is actually a package path.
if module, err := c.pc.FindModule(meta.modulePath); err == nil { // no error
@@ -113,16 +136,11 @@
log.Info("no suitable alias found, creating basic report")
}
- status := report.Reviewed
- if meta.unreviewed {
- status = report.Unreviewed
- }
-
r := report.New(src, c.pc,
report.WithGoID(meta.id),
report.WithModulePath(meta.modulePath),
report.WithAliases(aliases),
- report.WithReviewStatus(status),
+ report.WithReviewStatus(meta.reviewStatus),
)
// Find any additional aliases referenced by the source aliases.
@@ -164,7 +182,7 @@
CVEs: r.CVEs,
GHSAs: r.GHSAs,
}
- case meta.unreviewed:
+ case meta.reviewStatus == report.Unreviewed:
r.Description = ""
addNotes := true
if fixed := c.fix(ctx, r, addNotes); fixed {
@@ -197,6 +215,7 @@
labelSuggestedEdit = "Suggested Edit"
labelTriaged = "triaged"
labelHighPriority = "high priority"
+ labelFirstParty = "first party"
labelPossibleDuplicate = "possible duplicate"
labelPossiblyNotGo = "possibly not Go"
)
@@ -235,11 +254,11 @@
// Data that can be combined with a source vulnerability
// to create a new report.
type reportMeta struct {
- id string
- modulePath string
- aliases []string
- excluded report.ExcludedReason
- unreviewed bool // create basic report with no TODOs and no description
+ id string
+ modulePath string
+ aliases []string
+ excluded report.ExcludedReason
+ reviewStatus report.ReviewStatus
}
const todo = "TODO: "
diff --git a/cmd/vulnreport/unexclude.go b/cmd/vulnreport/unexclude.go
index 118323c..d0cc6be 100644
--- a/cmd/vulnreport/unexclude.go
+++ b/cmd/vulnreport/unexclude.go
@@ -64,10 +64,10 @@
}
if err := u.reportFromMeta(ctx, &reportMeta{
- id: oldR.ID,
- modulePath: modulePath,
- aliases: oldR.Aliases(),
- unreviewed: true,
+ id: oldR.ID,
+ modulePath: modulePath,
+ aliases: oldR.Aliases(),
+ reviewStatus: report.Unreviewed,
}); err != nil {
return err
}
diff --git a/doc/quickstart.md b/doc/quickstart.md
index 34af6bf..61cd97f 100644
--- a/doc/quickstart.md
+++ b/doc/quickstart.md
@@ -49,18 +49,13 @@
### Add standard reports
-1. Issues marked `high priority` need a REVIEWED report, and issues without a priority label need an UNREVIEWED report.
- * To create a reviewed report for issue #NNN, run:
+1. Issues marked `high priority` need a REVIEWED report, and issues without a priority label need an UNREVIEWED report. To create a report for issue #NNN, run:
- ```bash
- $ vulnreport create NNN
- ```
+ ```bash
+ $ vulnreport create NNN
+ ```
- * To create an unreviewed report for issue #NNN, run:
-
- ```bash
- $ vulnreport -unreviewed create NNN
- ```
+ The command knows whether to create a reviewed or unreviewed report based on the issue's label. (To override this decision, use flag `-status=UNREVIEWED` or `-status=REVIEWED`.)
2. Edit the report if needed. For reviewed reports, this follows the standard process. For unreviewed reports, only edit the report if it has lint/fix errors (which will be populated in the notes section).
3. Fix the report and add derived files:
diff --git a/internal/report/report.go b/internal/report/report.go
index 0dfbcb9..3d16c22 100644
--- a/internal/report/report.go
+++ b/internal/report/report.go
@@ -259,6 +259,13 @@
return osv.ReviewStatus(r).String()
}
+func ToReviewStatus(s string) (ReviewStatus, bool) {
+ if rs, ok := osv.ToReviewStatus(s); ok {
+ return ReviewStatus(rs), true
+ }
+ return 0, false
+}
+
func (r ReviewStatus) MarshalYAML() (any, error) {
or := osv.ReviewStatus(r)
if !or.IsValid() {
@@ -270,8 +277,8 @@
func (r *ReviewStatus) UnmarshalYAML(node *yaml.Node) error {
if node.Kind == yaml.ScalarNode {
v := node.Value
- if rs, ok := osv.ToReviewStatus(v); ok {
- *r = ReviewStatus(rs)
+ if rs, ok := ToReviewStatus(v); ok {
+ *r = rs
return nil
}
return fmt.Errorf("UnmarshalYAML: unrecognized review status: %s", v)
