internal/{report,worker}: update display of xrefs

Unify the display of xrefs in the worker and in vulnreport xref.

Call out duplicate aliases more prominently, as they indicate a problem,
whereas module xrefs are informational.

Change-Id: I3898ab1709bb3bfd6aefcfa4aef236af5f270fa7
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/599176
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
This commit is contained in:
Tatiana Bradley 2024-07-17 13:21:42 -04:00
Родитель 2ae4aed38a
Коммит 76c7a5b6fd
8 изменённых файлов: 187 добавлений и 113 удалений

Просмотреть файл

@ -6,9 +6,11 @@ Expected output of test TestXref/found_xrefs
command: "vulnreport xref 4"
-- out --
data/reports/GO-9999-0004.yaml: found xrefs:
Module golang.org/x/tools appears in data/reports/GO-9999-0005.yaml
GO-9999-0004 priority is low
GO-9999-0004: found module xrefs
- golang.org/x/tools appears in 1 other report(s):
- data/reports/GO-9999-0005.yaml (https://github.com/golang/vulndb#5)
GO-9999-0004: priority is low
- golang.org/x/tools has 50 importers (< 100)
-- logs --
info: xref: operating on 1 report(s)

Просмотреть файл

@ -6,7 +6,7 @@ Expected output of test TestXref/no_xrefs
command: "vulnreport xref 1"
-- out --
GO-9999-0001 priority is unknown
GO-9999-0001: priority is unknown
- module golang.org/x/vulndb not found
-- logs --
info: xref: operating on 1 report(s)

Просмотреть файл

@ -7,10 +7,8 @@ package main
import (
"context"
"fmt"
"strings"
"golang.org/x/exp/constraints"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
"golang.org/x/vulndb/cmd/vulnreport/log"
"golang.org/x/vulndb/cmd/vulnreport/priority"
@ -44,13 +42,13 @@ func (x *xref) run(ctx context.Context, input any) (err error) {
r := input.(*yamlReport)
if xrefs := x.xref(r); len(xrefs) > 0 {
log.Outf("%s: found xrefs:%s", r.Filename, xrefs)
log.Out(xrefs)
} else {
log.Infof("%s: no xrefs found", r.Filename)
}
pr, notGo := x.reportPriority(r.Report)
log.Outf("%s priority is %s\n - %s", r.ID, pr.Priority, pr.Reason)
log.Outf("%s: priority is %s\n - %s", r.ID, pr.Priority, pr.Reason)
if notGo != nil {
log.Outf("%s is likely not Go\n - %s", r.ID, notGo.Reason)
}
@ -84,21 +82,9 @@ type xrefer struct {
}
func (x *xrefer) xref(r *yamlReport) string {
out := &strings.Builder{}
matches := x.rc.XRef(r.Report)
delete(matches, r.Filename)
// This sorts as CVEs, GHSAs, and then modules.
for _, fname := range sorted(maps.Keys(matches)) {
for _, id := range sorted(matches[fname]) {
fmt.Fprintf(out, "\n%v appears in %v", id, fname)
if r, ok := x.rc.Report(fname); ok {
if r.IsExcluded() {
fmt.Fprintf(out, " %v", r.Excluded)
}
}
}
}
return out.String()
aliasTitle := fmt.Sprintf("%s: found possible duplicates", r.ID)
moduleTitle := fmt.Sprintf("%s: found module xrefs", r.ID)
return x.rc.XRef(r.Report).ToString(aliasTitle, moduleTitle, "")
}
func (x *xrefer) modulePriority(modulePath string) (*priority.Result, *priority.NotGoResult) {

Просмотреть файл

@ -6,7 +6,10 @@ package report
import (
"context"
"fmt"
"io"
"path/filepath"
"strings"
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing/object"
@ -36,8 +39,8 @@ const (
type Client struct {
byFile map[string]*Report
byIssue map[int]*Report
byAlias map[string][]*Report
byModule map[string][]*Report
byAlias map[string][]*File
byModule map[string][]*File
}
// NewClient returns a Client for accessing the reports in
@ -82,33 +85,91 @@ func (c *Client) List() []*Report {
return maps.Values(c.byFile)
}
// XRef returns cross-references for a report.
// The output, matches, is a map from filenames to aliases (CVE & GHSA IDs)
// and modules (excluding std and cmd).
func (c *Client) XRef(r *Report) (matches map[string][]string) {
mods := make(map[string]bool)
for _, m := range r.Modules {
if mod := m.Module; mod != "" && mod != "std" && mod != "cmd" {
mods[m.Module] = true
type Xrefs struct {
// map from aliases to files
Aliases map[string][]*File
// map from modules to files
Modules map[string][]*File
}
func fprintMap(out io.Writer, m map[string][]*File) {
sortedKeys := func(m map[string][]*File) []string {
s := slices.Clone(maps.Keys(m))
slices.Sort(s)
return s
}
for _, k := range sortedKeys(m) {
fs := m[k]
fmt.Fprintf(out, "- %s appears in %d other report(s):\n", k, len(fs))
for _, f := range fs {
fmt.Fprintf(out, " - %s (https://github.com/golang/vulndb#%d)", f.Filename, f.IssNum)
if f.Report.IsExcluded() {
fmt.Fprintf(out, " %v", f.Report.Excluded)
}
fmt.Fprintf(out, "\n")
}
}
}
func (xs *Xrefs) ToString(aliasTitle, moduleTitle, noneMessage string) string {
if len(xs.Modules) == 0 && len(xs.Aliases) == 0 {
return noneMessage
}
out := &strings.Builder{}
if len(xs.Aliases) != 0 {
fmt.Fprint(out, aliasTitle+"\n")
fprintMap(out, xs.Aliases)
if len(xs.Modules) != 0 {
fmt.Fprintf(out, "\n")
}
}
// matches is a map from filename -> alias/module
matches = make(map[string][]string)
for fname, rr := range c.byFile {
for _, alias := range rr.Aliases() {
if slices.Contains(r.Aliases(), alias) {
matches[fname] = append(matches[fname], alias)
}
}
for _, m := range rr.Modules {
if mods[m.Module] {
k := "Module " + m.Module
matches[fname] = append(matches[fname], k)
if len(xs.Modules) != 0 {
fmt.Fprint(out, moduleTitle+"\n")
fprintMap(out, xs.Modules)
}
return out.String()
}
type File struct {
Filename string
IssNum int
*Report
}
// XRef returns cross-references for a report.
func (c *Client) XRef(r *Report) *Xrefs {
x := &Xrefs{
Aliases: make(map[string][]*File),
Modules: make(map[string][]*File),
}
for _, alias := range r.Aliases() {
for _, f := range c.byAlias[alias] {
if r.ID == f.Report.ID {
continue
}
x.Aliases[alias] = append(x.Aliases[alias], f)
}
}
return matches
for _, m := range r.Modules {
if m.IsFirstParty() {
continue
}
for _, f := range c.byModule[m.Module] {
if r.ID == f.Report.ID {
continue
}
x.Modules[m.Module] = append(x.Modules[m.Module], f)
}
}
return x
}
// Report returns the report with the given filename in vulndb, or
@ -128,13 +189,21 @@ func (c *Client) HasReport(githubID int) (found bool) {
// ReportsByAlias returns a list of reports in vulndb with the given
// alias.
func (c *Client) ReportsByAlias(alias string) []*Report {
return c.byAlias[alias]
var rs []*Report
for _, f := range c.byAlias[alias] {
rs = append(rs, f.Report)
}
return rs
}
// ReportsByModule returns a list of reports in vulndb with the given
// module.
func (c *Client) ReportsByModule(module string) []*Report {
return c.byModule[module]
var rs []*Report
for _, f := range c.byModule[module] {
rs = append(rs, f.Report)
}
return rs
}
// AliasHasReport returns whether the given alias exists in vulndb.
@ -147,8 +216,8 @@ func newClient() *Client {
return &Client{
byIssue: make(map[int]*Report),
byFile: make(map[string]*Report),
byAlias: make(map[string][]*Report),
byModule: make(map[string][]*Report),
byAlias: make(map[string][]*File),
byModule: make(map[string][]*File),
}
}
@ -187,13 +256,19 @@ func (c *Client) addReport(filename string, r *Report) error {
return err
}
f := &File{
Filename: filename,
IssNum: iss,
Report: r,
}
c.byFile[filename] = r
c.byIssue[iss] = r
for _, alias := range r.Aliases() {
c.byAlias[alias] = append(c.byAlias[alias], r)
c.byAlias[alias] = append(c.byAlias[alias], f)
}
for _, m := range r.Modules {
c.byModule[m.Module] = append(c.byModule[m.Module], r)
c.byModule[m.Module] = append(c.byModule[m.Module], f)
}
return nil

Просмотреть файл

@ -42,7 +42,6 @@ var (
Modules: []*Module{
{Module: "example.com/another/module"},
},
GHSAs: []string{
"GHSA-9999-abcd-efgh",
},
@ -55,6 +54,14 @@ var (
},
CVEs: []string{"CVE-9999-0005"},
}
fname6 = "data/reports/GO-9999-0006.yaml"
r6 = Report{
ID: "GO-9999-0006",
Modules: []*Module{
{Module: "example.com/another/module"},
},
GHSAs: []string{"GHSA-9999-abcd-efgh"},
}
txtarFile = filepath.Join("testdata", "repo.txtar")
)
@ -71,7 +78,7 @@ func TestList(t *testing.T) {
}
got := rc.List()
want := []*Report{&r1, &r2, &r4, &r5}
want := []*Report{&r1, &r2, &r4, &r5, &r6}
byID := func(a, b *Report) bool { return a.ID < b.ID }
if diff := cmp.Diff(got, want, cmpopts.SortSlices(byID)); diff != "" {
t.Errorf("mismatch (-got, +want): %s", diff)
@ -91,6 +98,7 @@ func TestXRef(t *testing.T) {
tests := []struct {
name string
r *Report
wantXrefs *Xrefs
wantMatches map[string][]string
}{
{
@ -103,7 +111,10 @@ func TestXRef(t *testing.T) {
ID: "CVE-9999-0003",
},
},
wantMatches: map[string][]string{},
wantXrefs: &Xrefs{
Aliases: map[string][]*File{},
Modules: map[string][]*File{},
},
},
{
name: "Ignores std lib modules",
@ -113,7 +124,10 @@ func TestXRef(t *testing.T) {
},
CVEs: []string{"CVE-9999-0003"},
},
wantMatches: map[string][]string{},
wantXrefs: &Xrefs{
Aliases: map[string][]*File{},
Modules: map[string][]*File{},
},
},
{
name: "Match on CVE (ignores std module)",
@ -123,26 +137,37 @@ func TestXRef(t *testing.T) {
},
CVEs: []string{"CVE-9999-0001"},
},
wantMatches: map[string][]string{
fname1: {"CVE-9999-0001"},
wantXrefs: &Xrefs{
Aliases: map[string][]*File{
"CVE-9999-0001": {
{Filename: fname1, IssNum: 1, Report: &r1},
},
},
Modules: map[string][]*File{},
},
},
{
name: "Match on GHSA & module",
r: &r4,
wantMatches: map[string][]string{
fname4: {
"GHSA-9999-abcd-efgh",
"Module example.com/another/module",
wantXrefs: &Xrefs{
Aliases: map[string][]*File{
"GHSA-9999-abcd-efgh": {
{Filename: fname6, IssNum: 6, Report: &r6},
},
},
Modules: map[string][]*File{
"example.com/another/module": {
{Filename: fname6, IssNum: 6, Report: &r6},
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotMatches := rc.XRef(tt.r)
if diff := cmp.Diff(gotMatches, tt.wantMatches); diff != "" {
t.Errorf("XRef(): matches mismatch (-got, +want): %s", diff)
got := rc.XRef(tt.r)
if diff := cmp.Diff(got, tt.wantXrefs); diff != "" {
t.Errorf("XRef(): mismatch (-got, +want): %s", diff)
}
})
}
@ -198,14 +223,19 @@ func TestNewClient(t *testing.T) {
}
files := map[string]*Report{
fname1: &r1, fname2: &r2, fname4: &r4, fname5: &r5,
fname1: &r1, fname2: &r2, fname4: &r4, fname5: &r5, fname6: &r6,
}
tc, err := NewTestClient(files)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(c, tc, cmp.AllowUnexported(Client{})); diff != "" {
less := func(f1, f2 *File) bool {
return f1.ID < f2.ID
}
if diff := cmp.Diff(c, tc, cmp.AllowUnexported(Client{}),
cmpopts.SortSlices(less)); diff != "" {
t.Errorf("NewClient() / NewTestClient() mismatch (-New, +NewTest): %s", diff)
}
}

7
internal/report/testdata/repo.txtar поставляемый
Просмотреть файл

@ -32,3 +32,10 @@ modules:
- module: example.com/adiff/module
cves:
- CVE-9999-0005
-- data/reports/GO-9999-0006.yaml --
id: GO-9999-0006
modules:
- module: example.com/another/module
ghsas:
- GHSA-9999-abcd-efgh

Просмотреть файл

@ -18,8 +18,6 @@ import (
"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/object"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
"golang.org/x/time/rate"
"golang.org/x/vulndb/internal/cve4"
"golang.org/x/vulndb/internal/cveutils"
@ -181,38 +179,9 @@ func CreateIssues(ctx context.Context, st store.Store, client *issues.Client, pc
// xref returns cross-references for a report: Information about other reports
// for the same CVE, GHSA, or module.
func xref(r *report.Report, rc *report.Client) string {
out := &strings.Builder{}
sorted := func(s []string) []string {
s = slices.Clone(s)
slices.Sort(s)
return s
}
matches := rc.XRef(r)
for _, fname := range sorted(maps.Keys(matches)) {
for _, match := range sorted(matches[fname]) {
// Getting issue number from file name
var appearsIn string
_, _, issueNum, err := report.ParseFilepath(fname)
if err != nil {
appearsIn = fmt.Sprintf("%s (unable to convert file name to issue number, %v)", fname, err)
} else {
appearsIn = strconv.Itoa(issueNum)
}
fmt.Fprintf(out, "- %v appears in issue #%v", match, appearsIn)
if r, ok := rc.Report(fname); ok {
if r.IsExcluded() {
fmt.Fprintf(out, " %v", r.Excluded)
}
}
fmt.Fprintf(out, "\n")
}
}
if len(matches) == 0 {
fmt.Fprint(out, "No existing reports found with this module or alias.")
}
return out.String()
aliasTitle, moduleTitle, noneMessage := "!! Possible duplicate report !!",
"Cross references:", "No existing reports found with this module or alias."
return rc.XRef(r).ToString(aliasTitle, moduleTitle, noneMessage)
}
func createCVEIssues(ctx context.Context, st store.Store, client *issues.Client, pc *proxy.Client, rc *report.Client, limit int) (err error) {
@ -323,8 +292,8 @@ func createGHSAIssues(ctx context.Context, st store.Store, client *issues.Client
func isDuplicate(sa *ghsa.SecurityAdvisory, pc *proxy.Client, rc *report.Client) bool {
r := report.New(sa, pc)
for _, aliases := range rc.XRef(r) {
if slices.Contains(aliases, sa.ID) {
for alias := range rc.XRef(r).Aliases {
if sa.ID == alias {
return true
}
}
@ -451,9 +420,8 @@ Description:
References:{{range .References}}
- {{.Type}}: {{.URL}}{{end}}
Cross references:
{{.Xrefs}}
See [doc/triage.md](https://github.com/golang/vulndb/blob/master/doc/triage.md) for instructions on how to triage this report.
See [doc/quickstart.md](https://github.com/golang/vulndb/blob/master/doc/quickstart.md) for instructions on how to triage this report.
{{if (and .Pre .ReportStr) -}}
{{.Pre}}

Просмотреть файл

@ -302,11 +302,15 @@ a description
References:
- ADVISORY: https://nvd.nist.gov/vuln/detail/CVE-2000-0001
Cross references:
- CVE-2000-0001 appears in issue #2
- Module golang.org/x/vulndb appears in issue #2
!! Possible duplicate report !!
- CVE-2000-0001 appears in 1 other report(s):
- data/reports/GO-9999-0002.yaml (https://github.com/golang/vulndb#2)
See [doc/triage.md](https://github.com/golang/vulndb/blob/master/doc/triage.md) for instructions on how to triage this report.
Cross references:
- golang.org/x/vulndb appears in 1 other report(s):
- data/reports/GO-9999-0002.yaml (https://github.com/golang/vulndb#2)
See [doc/quickstart.md](https://github.com/golang/vulndb/blob/master/doc/quickstart.md) for instructions on how to triage this report.
` + "```" + `
id: GO-ID-PENDING
@ -326,6 +330,7 @@ review_status: UNREVIEWED
` + "```"
if diff := cmp.Diff(unindent(want), got); diff != "" {
t.Errorf("mismatch (-want, +got):\n%s", diff)
t.Errorf("got: %s", got)
}
}
@ -380,10 +385,11 @@ References:
- ADVISORY: https://github.com/advisories/GHSA-xxxx-yyyy-zzzz
- FIX: https://example.com/commit/12345
Cross references:
- GHSA-xxxx-yyyy-zzzz appears in issue #1 EXCLUDED
!! Possible duplicate report !!
- GHSA-xxxx-yyyy-zzzz appears in 1 other report(s):
- data/excluded/GO-9999-0001.yaml (https://github.com/golang/vulndb#1) EXCLUDED
See [doc/triage.md](https://github.com/golang/vulndb/blob/master/doc/triage.md) for instructions on how to triage this report.
See [doc/quickstart.md](https://github.com/golang/vulndb/blob/master/doc/quickstart.md) for instructions on how to triage this report.
` + "```" + `
id: GO-ID-PENDING