Skip to content

Commit

Permalink
cmd/vulnreport: fix two bugs in "vulnreport duplicates"
Browse files Browse the repository at this point in the history
- Don't error when an issue is already marked "duplicate"
- Don't mark an issue as a duplicate if it already has a report
(corresponding to the issue itself)

Change-Id: I8909a0963727b070484993fd9e6324ada3c828f5
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/584377
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
tatianab committed May 15, 2024
1 parent f1651ad commit 3b6f478
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
26 changes: 16 additions & 10 deletions cmd/vulnreport/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,15 @@ func argsToIDs(args []string) ([]string, error) {
}

func createReport(ctx context.Context, iss *issues.Issue, pc *proxy.Client, gc *ghsa.Client, ac *genai.GeminiClient, allowClosed bool) (r *report.Report, err error) {
parsed, err := parseGithubIssue(iss, pc, allowClosed)
if iss.HasLabel(labelDuplicate) {
return nil, fmt.Errorf("duplicate issue")
}

if !allowClosed && iss.State == "closed" {
return nil, errors.New("issue is closed")
}

parsed, err := parseGithubIssue(iss, pc)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -270,16 +278,17 @@ func reportFromAliases(ctx context.Context, id, modulePath string, aliases []str
return r
}

func parseGithubIssue(iss *issues.Issue, pc *proxy.Client, allowClosed bool) (*parsedIssue, error) {
const (
labelDuplicate = "duplicate"
labelDirect = "Direct External Report"
)

func parseGithubIssue(iss *issues.Issue, pc *proxy.Client) (*parsedIssue, error) {
parsed := &parsedIssue{
id: iss.NewGoID(),
}

if !allowClosed && iss.State == "closed" {
return nil, errors.New("issue is closed")
}

// Parse labels for excluded and duplicate issues.
// Find any excluded labels.
for _, label := range iss.Labels {
if reason, ok := report.FromLabel(label); ok {
if parsed.excluded == "" {
Expand All @@ -288,9 +297,6 @@ func parseGithubIssue(iss *issues.Issue, pc *proxy.Client, allowClosed bool) (*p
return nil, fmt.Errorf("issue has multiple excluded reasons")
}
}
if label == "duplicate" {
return nil, fmt.Errorf("duplicate issue")
}
}

// Parse elements from GitHub issue title.
Expand Down
13 changes: 12 additions & 1 deletion cmd/vulnreport/duplicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ func (d *duplicates) run(ctx context.Context, issNum string) (err error) {
}
}

parsed, err := parseGithubIssue(iss, d.pc, false)
if iss.HasLabel(labelDuplicate) {
log.Infof("issue #%d is already marked duplicate, skipping", iss.Number)
return
}

parsed, err := parseGithubIssue(iss, d.pc)
if err != nil {
return err
}
Expand All @@ -130,6 +135,12 @@ func (d *duplicates) run(ctx context.Context, issNum string) (err error) {
if err != nil {
fname = r.ID
}
// Skip the report if it corresponds to the issue number.
// (This happens when there is an unsubmitted report for the issue).
_, _, in, _ := report.ParseFilepath(fname)
if in == iss.Number {
continue
}
xrefs = append(xrefs, fname)
}
}
Expand Down
5 changes: 5 additions & 0 deletions internal/issues/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"context"
"fmt"
"net/url"
"slices"
"time"

"github.com/google/go-github/v41/github"
Expand Down Expand Up @@ -212,3 +213,7 @@ func (iss *Issue) NewGoID() string {
}
return fmt.Sprintf("GO-%04d-%04d", year, iss.Number)
}

func (iss *Issue) HasLabel(label string) bool {
return slices.Contains(iss.Labels, label)
}

0 comments on commit 3b6f478

Please sign in to comment.