internal/worker: add modules mode to govulncheck pipeline

This is accomplished by using the newest version of govulncheck. The
tool now produces streaming JSON where it emits findings at every level
of precision (module, package, symbol) as it does work.

We thus collect all findings produced by govulncheck and convert them to
Vuln structure right before we save it rows. This simplifies matters. The
ecosystem metrics handler for govulncheck JSON is now trivial. The code
operates on govulncheck.Findings and lets vulnsForMode do the conversion
to Vuln in a single (last) step. Vuln also does not need Called field.

Change-Id: I73651a91b2707d9afd1e667ea4cedb371e763c73
Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/562695
Reviewed-by: Maceo Thompson <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Zvonimir Pavlinovic <[email protected]>

go.mod

@@ -26,9 +26,9 @@ require (
 	golang.org/x/exp/event v0.0.0-20220218215828-6cf2b201936e
 	golang.org/x/mod v0.15.0
 	golang.org/x/net v0.21.0
-	golang.org/x/oauth2 v0.17.0
+	golang.org/x/oauth2 v0.10.0
 	golang.org/x/tools v0.18.0
-	golang.org/x/vuln v1.0.1-0.20230810155601-91424c7c0e1c
+	golang.org/x/vuln v1.0.5-0.20240213143941-dcac2d7cab09
 	google.golang.org/api v0.132.0
 	google.golang.org/genproto/googleapis/api v0.0.0-20230706204954-ccb25ca9f130
 	google.golang.org/grpc v1.56.2

go.sum

@@ -448,8 +448,8 @@ golang.org/x/oauth2 v0.0.0-20210628180205-a41e5a781914/go.mod h1:KelEdhl1UZF7XfJ
 golang.org/x/oauth2 v0.0.0-20210805134026-6f1e6394065a/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
 golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
 golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
-golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ=
-golang.org/x/oauth2 v0.17.0/go.mod h1:OzPDGQiuQMguemayvdylqddI7qcD9lnSDb+1FiwQ5HA=
+golang.org/x/oauth2 v0.10.0 h1:zHCpF2Khkwy4mMB4bv0U37YtJdTGW8jI0glAApi0Kh8=
+golang.org/x/oauth2 v0.10.0/go.mod h1:kTpgurOux7LqtuxjuyZa4Gj2gdezIt/jQtGnNFfypQI=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
@@ -595,8 +595,8 @@ golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
 golang.org/x/tools v0.18.0 h1:k8NLag8AGHnn+PHbl7g43CtqZAwG60vZkLqgyZgIHgQ=
 golang.org/x/tools v0.18.0/go.mod h1:GL7B4CwcLLeo59yx/9UWWuNOW1n3VZ4f5axWfML7Lcg=
-golang.org/x/vuln v1.0.1-0.20230810155601-91424c7c0e1c h1:Tpbe1bL1Wq4yGGhPZhs60B2JPTAOnBAoVBFaAm25PNM=
-golang.org/x/vuln v1.0.1-0.20230810155601-91424c7c0e1c/go.mod h1:V0eyhHwaAaHrt42J9bgrN6rd12f6GU4T0Lu0ex2wDg4=
+golang.org/x/vuln v1.0.5-0.20240213143941-dcac2d7cab09 h1:A64fP6pOn7HymW2o6mact9ciWbn8bAVYuEmoxjCd43Y=
+golang.org/x/vuln v1.0.5-0.20240213143941-dcac2d7cab09/go.mod h1:17esbGXVHp03iSShz13iJUumhcDknaO/tzYo0Oau1TQ=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

internal/govulncheck/govulncheck.go

@@ -107,18 +107,12 @@ func ParseRequest(r *http.Request, prefix string) (*Request, error) {
 // a bigquery vuln.
 func ConvertGovulncheckFinding(f *govulncheckapi.Finding) *Vuln {
 	vulnerableFrame := f.Trace[0]
-	vuln := &Vuln{
+	return &Vuln{
 		ID:          f.OSV,
 		PackagePath: vulnerableFrame.Package,
 		ModulePath:  vulnerableFrame.Module,
 		Version:     vulnerableFrame.Version,
-		Called:      false,
 	}
-	if vulnerableFrame.Function != "" {
-		vuln.Called = true
-	}
-
-	return vuln
 }
 
 const TableName = "govulncheck"
@@ -201,12 +195,6 @@ type Vuln struct {
 	PackagePath string `bigquery:"package_path"`
 	ModulePath  string `bigquery:"module_path"`
 	Version     string `bigquery:"version"`
-	// Called is currently used to differentiate between
-	// called and imported vulnerabilities. We need it
-	// because we don't conduct an imports analysis yet
-	// use the full results of govulncheck source analysis.
-	// It is not part of the bigquery schema.
-	Called bool `bigquery:"-"`
 }
 
 // SchemaVersion changes whenever the govulncheck schema changes.

internal/govulncheck/govulncheck_test.go

@@ -61,7 +61,6 @@ func TestConvertGovulncheckFinding(t *testing.T) {
 				PackagePath: "example.com/repo/module/package",
 				ModulePath:  "example.com/repo/module",
 				Version:     "v0.0.1",
-				Called:      true,
 			},
 		},
 		{
@@ -72,7 +71,6 @@ func TestConvertGovulncheckFinding(t *testing.T) {
 				PackagePath: "example.com/repo/module/package",
 				ModulePath:  "example.com/repo/module",
 				Version:     "v1.0.0",
-				Called:      false,
 			},
 		},
 	}

internal/govulncheck/handler.go

@@ -5,22 +5,18 @@
 package govulncheck
 
 import (
-	"golang.org/x/exp/maps"
 	"golang.org/x/pkgsite-metrics/internal/govulncheckapi"
 	"golang.org/x/pkgsite-metrics/internal/osv"
 )
 
-// NewMetricsHandler returns a handler that returns a set of all findings.
+// NewMetricsHandler returns a handler that returns all findings.
 // For use in the ecosystem metrics pipeline.
 func NewMetricsHandler() *MetricsHandler {
-	m := make(map[string]*govulncheckapi.Finding)
-	return &MetricsHandler{
-		byOSV: m,
-	}
+	return &MetricsHandler{}
 }
 
 type MetricsHandler struct {
-	byOSV map[string]*govulncheckapi.Finding
+	findings []*govulncheckapi.Finding
 }
 
 func (h *MetricsHandler) Config(c *govulncheckapi.Config) error {
@@ -36,16 +32,10 @@ func (h *MetricsHandler) OSV(e *osv.Entry) error {
 }
 
 func (h *MetricsHandler) Finding(finding *govulncheckapi.Finding) error {
-	f, found := h.byOSV[finding.OSV]
-	if !found || f.Trace[0].Function == "" {
-		// If the vuln wasn't called in the first trace, replace it with
-		// the new finding (that way if the vuln is called at any point
-		// it's trace will reflect that, which is needed when converting to bq)
-		h.byOSV[finding.OSV] = finding
-	}
+	h.findings = append(h.findings, finding)
 	return nil
 }
 
 func (h *MetricsHandler) Findings() []*govulncheckapi.Finding {
-	return maps.Values(h.byOSV)
+	return h.findings
 }

internal/govulncheckapi/result.go

@@ -21,12 +21,12 @@ type Message struct {
 	Finding  *Finding   `json:"finding,omitempty"`
 }
 
-// ProtocolVersion is the current protocol version this file implements
-const ProtocolVersion = "v0.1.0"
-
+// Config must occur as the first message of a stream and informs the client
+// about the information used to generate the findings.
+// The only required field is the protocol version.
 type Config struct {
 	// ProtocolVersion specifies the version of the JSON protocol.
-	ProtocolVersion string `json:"protocol_version,omitempty"`
+	ProtocolVersion string `json:"protocol_version"`
 
 	// ScannerName is the name of the tool, for example, govulncheck.
 	//
@@ -48,17 +48,15 @@ type Config struct {
 	// vulnerabilities.
 	GoVersion string `json:"go_version,omitempty"`
 
-	// Consider only vulnerabilities that apply to this OS.
-	GOOS string `json:"goos,omitempty"`
-
-	// Consider only vulnerabilities that apply to this architecture.
-	GOARCH string `json:"goarch,omitempty"`
-
-	// ImportsOnly instructs vulncheck to analyze import chains only.
-	// Otherwise, call chains are analyzed too.
-	ImportsOnly bool `json:"imports_only,omitempty"`
+	// ScanLevel instructs govulncheck to analyze at a specific level of detail.
+	// Valid values include module, package and symbol.
+	ScanLevel ScanLevel `json:"scan_level,omitempty"`
 }
 
+// Progress messages are informational only, intended to allow users to monitor
+// the progress of a long running scan.
+// A stream must remain fully valid and able to be interpreted with all progress
+// messages removed.
 type Progress struct {
 	// A time stamp for the message.
 	Timestamp *time.Time `json:"time,omitempty"`
@@ -67,7 +65,14 @@ type Progress struct {
 	Message string `json:"message,omitempty"`
 }
 
-// Vuln represents a single OSV entry.
+// Finding contains information on a discovered vulnerability. Each vulnerability
+// will likely have multiple findings in JSON mode. This is because govulncheck
+// emits findings as it does work, and therefore could emit one module level,
+// one package level, and potentially multiple symbol level findings depending
+// on scan level.
+// Multiple symbol level findings can be emitted when multiple symbols of the
+// same vuln are called or govulncheck decides to show multiple traces for the
+// same symbol.
 type Finding struct {
 	// OSV is the id of the detected vulnerability.
 	OSV string `json:"osv,omitempty"`
@@ -97,8 +102,10 @@ type Finding struct {
 	// In binary mode, trace will contain a single-frame with no position
 	// information.
 	//
-	// When a package is imported but no vulnerable symbol is called, the trace
-	// will contain a single-frame with no symbol or position information.
+	// For module level source findings, the trace will contain a single-frame
+	// with no symbol, position, or package information. For package level source
+	// findings, the trace will contain a single-frame with no symbol or position
+	// information.
 	Trace []*Frame `json:"trace,omitempty"`
 }
 
@@ -130,11 +137,31 @@ type Frame struct {
 	Position *Position `json:"position,omitempty"`
 }
 
-// Position is a copy of token.Position used to marshal/unmarshal
-// JSON correctly.
+// Position represents arbitrary source position.
 type Position struct {
 	Filename string `json:"filename,omitempty"` // filename, if any
-	Offset   int    `json:"offset"`             // offset, starting at 0
+	Offset   int    `json:"offset"`             // byte offset, starting at 0
 	Line     int    `json:"line"`               // line number, starting at 1
 	Column   int    `json:"column"`             // column number, starting at 1 (byte count)
 }
+
+// ScanLevel represents the detail level at which a scan occurred.
+// This can be necessary to correctly interpret the findings, for instance if
+// a scan is at symbol level and a finding does not have a symbol it means the
+// vulnerability was imported but not called. If the scan however was at
+// "package" level, that determination cannot be made.
+type ScanLevel string
+
+const (
+	ScanLevelModule  = "module"
+	ScanLevelPackage = "package"
+	ScanLevelSymbol  = "symbol"
+)
+
+// WantSymbols can be used to check whether the scan level is one that is able
+// to generate symbol-level findings.
+func (l ScanLevel) WantSymbols() bool { return l == ScanLevelSymbol }
+
+// WantPackages can be used to check whether the scan level is one that is able
+// to generate package-level findings.
+func (l ScanLevel) WantPackages() bool { return l == ScanLevelPackage || l == ScanLevelSymbol }