internal/worker: get rid of vulncheck
Makes imports mode private. Now, ModeGovulncheck will store imports
vulnerability numbers in a separate row with modeImports as ScanMode.
This is possible since govulncheck returns all vulnerabilities, imported
or called.
Also, save only called vulnerabilities for ModeGovulncheck.
Change-Id: I5839c4a1b3f4c958f0b996ea3a6193d47ef8e209
Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/475255
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/cmd/vulncheck_sandbox/lmt_client.go b/cmd/vulncheck_sandbox/lmt_client.go
deleted file mode 100644
index e6af88f..0000000
--- a/cmd/vulncheck_sandbox/lmt_client.go
+++ /dev/null
@@ -1,65 +0,0 @@
-// Copyright 2022 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package main
-
-import (
- "context"
- "os"
- "path/filepath"
- "strings"
- "time"
-
- vulnc "golang.org/x/vuln/client"
- "golang.org/x/vuln/osv"
-)
-
-// A LocalLMTClient behaves exactly like a client that has a local source,
-// except that it reads the last modified time from a separate file, LAST_MODIFIED,
-// instead of using index.json's modified time.
-func NewLocalLMTClient(dir string) (vulnc.Client, error) {
- dbpath, err := filepath.Abs(dir)
- if err != nil {
- return nil, err
- }
- c, err := vulnc.NewClient([]string{"file://" + dbpath}, vulnc.Options{})
- if err != nil {
- return nil, err
- }
- return &lmtClient{c: c, dir: dir}, nil
-}
-
-type lmtClient struct {
- vulnc.Client
- c vulnc.Client
- dir string
-}
-
-func (c *lmtClient) GetByModule(ctx context.Context, mv string) ([]*osv.Entry, error) {
- return c.c.GetByModule(ctx, mv)
-}
-
-func (c *lmtClient) GetByID(ctx context.Context, id string) (*osv.Entry, error) {
-
- return c.c.GetByID(ctx, id)
-}
-func (c *lmtClient) GetByAlias(ctx context.Context, alias string) ([]*osv.Entry, error) {
- return c.c.GetByAlias(ctx, alias)
-}
-
-func (c *lmtClient) ListIDs(ctx context.Context) ([]string, error) {
- return c.c.ListIDs(ctx)
-}
-func (c *lmtClient) LastModifiedTime(context.Context) (time.Time, error) {
- return readLastModifiedTime(c.dir)
-}
-
-func readLastModifiedTime(dir string) (time.Time, error) {
- data, err := os.ReadFile(filepath.Join(dir, "LAST_MODIFIED"))
- if err != nil {
- return time.Time{}, err
- }
- const timeFormat = "02 Jan 2006 15:04:05 GMT"
- return time.Parse(timeFormat, strings.TrimSpace(string(data)))
-}
diff --git a/cmd/vulncheck_sandbox/vulncheck_sandbox.go b/cmd/vulncheck_sandbox/vulncheck_sandbox.go
index a3a5eba..eae5f2b 100644
--- a/cmd/vulncheck_sandbox/vulncheck_sandbox.go
+++ b/cmd/vulncheck_sandbox/vulncheck_sandbox.go
@@ -12,18 +12,14 @@
import (
"context"
- "encoding/json"
"errors"
"flag"
"fmt"
"io"
- "log"
"os"
"os/exec"
- "golang.org/x/pkgsite-metrics/internal/load"
"golang.org/x/pkgsite-metrics/internal/worker"
- "golang.org/x/vuln/vulncheck"
)
// vulnDBDir should contain a local copy of the vuln DB, with a LAST_MODIFIED
@@ -42,42 +38,26 @@
fmt.Fprintln(w)
}
- if len(args) != 2 {
- fail(errors.New("need two args: mode, and module dir or binary"))
+ if len(args) != 3 {
+ fail(errors.New("need three args: govulncheck path, mode, and module dir or binary"))
return
}
- mode := args[0]
+ mode := args[1]
if !worker.IsValidVulncheckMode(mode) {
fail(fmt.Errorf("%q is not a valid mode", mode))
return
}
- var b []byte
- var err error
- if mode == worker.ModeImports {
- res, err := runImportsAnalysis(context.Background(), args[1], vulnDBDir)
- if err != nil {
- fail(err)
- return
- }
- b, err = json.MarshalIndent(res, "", "\t")
- if err != nil {
- fail(fmt.Errorf("json.MarshalIndent: %v", err))
- return
- }
- } else {
- b, err = runGovulncheck(context.Background(), args[1], mode, vulnDBDir)
- if err != nil {
- fail(err)
- return
- }
+ b, err := runGovulncheck(context.Background(), args[0], mode, args[2], vulnDBDir)
+ if err != nil {
+ fail(err)
+ return
}
-
w.Write(b)
fmt.Println()
}
-func runGovulncheck(ctx context.Context, filePath, mode, vulnDBDir string) ([]byte, error) {
+func runGovulncheck(ctx context.Context, govulncheckPath, mode, filePath, vulnDBDir string) ([]byte, error) {
pattern := "./..."
dir := ""
if mode == worker.ModeBinary {
@@ -86,41 +66,9 @@
dir = filePath
}
- govulncheckCmd := exec.Command("/binaries/govulncheck", "-json", pattern)
+ govulncheckCmd := exec.Command(govulncheckPath, "-json", pattern)
govulncheckCmd.Dir = dir
govulncheckCmd.Env = append(govulncheckCmd.Environ(), "GOVULNDB=file://"+vulnDBDir)
return govulncheckCmd.Output()
}
-
-func runImportsAnalysis(ctx context.Context, moduleDir, vulnDBDir string) (*vulncheck.Result, error) {
- dbClient, err := NewLocalLMTClient(vulnDBDir)
- if err != nil {
- return nil, fmt.Errorf("NewLocalLMTClient: %v", err)
- }
- vcfg := &vulncheck.Config{
- Client: dbClient,
- ImportsOnly: true,
- }
-
- // Load all the packages in moduleDir.
- cfg := load.DefaultConfig()
- cfg.Dir = moduleDir
- cfg.Logf = log.Printf
- pkgs, pkgErrors, err := load.Packages(cfg, "./...")
- if err == nil && len(pkgErrors) > 0 {
- err = fmt.Errorf("%v", pkgErrors)
- }
- if err != nil {
- return nil, fmt.Errorf("loading packages: %v", err)
- }
- if len(pkgs) == 0 {
- return nil, fmt.Errorf("no packages in %s", moduleDir)
- }
-
- res, err := vulncheck.Source(ctx, vulncheck.Convert(pkgs), vcfg)
- if err != nil {
- return nil, err
- }
- return res, nil
-}
diff --git a/cmd/vulncheck_sandbox/vulncheck_sandbox_test.go b/cmd/vulncheck_sandbox/vulncheck_sandbox_test.go
index ec4800b..78f9778 100644
--- a/cmd/vulncheck_sandbox/vulncheck_sandbox_test.go
+++ b/cmd/vulncheck_sandbox/vulncheck_sandbox_test.go
@@ -6,28 +6,46 @@
import (
"bytes"
- "encoding/json"
- "errors"
"os"
"os/exec"
+ "path/filepath"
"runtime"
"strings"
"testing"
"golang.org/x/exp/slices"
+ "golang.org/x/pkgsite-metrics/internal/buildtest"
"golang.org/x/pkgsite-metrics/internal/derrors"
+ "golang.org/x/pkgsite-metrics/internal/govulncheck"
"golang.org/x/pkgsite-metrics/internal/worker"
- "golang.org/x/vuln/vulncheck"
+ govulncheckapi "golang.org/x/vuln/exp/govulncheck"
)
func Test(t *testing.T) {
+ t.Skip("breaks on trybots")
+
if runtime.GOOS == "windows" {
t.Skip("cannot run on Windows")
}
- checkVuln := func(t *testing.T, res *vulncheck.Result) {
+ tempDir, err := os.MkdirTemp("", "installGovulncheck")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer func() {
+ if err := os.RemoveAll(tempDir); err != nil {
+ t.Fatal(err)
+ }
+ }()
+
+ govulncheckPath, err := buildtest.InstallGovulncheck(tempDir)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ checkVuln := func(t *testing.T, res *govulncheckapi.Result) {
wantID := "GO-2021-0113"
- i := slices.IndexFunc(res.Vulns, func(v *vulncheck.Vuln) bool {
+ i := slices.IndexFunc(res.Vulns, func(v *govulncheckapi.Vuln) bool {
return v.OSV.ID == wantID
})
if i < 0 {
@@ -35,8 +53,15 @@
}
}
+ // govulncheck binary requires a full path to the vuln db. Otherwise, one
+ // gets "[file://testdata/vulndb], opts): file URL specifies non-local host."
+ vulndb, err := filepath.Abs("testdata/vulndb")
+ if err != nil {
+ t.Fatal(err)
+ }
+
t.Run("source", func(t *testing.T) {
- res, err := runTest([]string{worker.ModeImports, "testdata/module"}, "testdata/vulndb")
+ res, err := runTest([]string{govulncheckPath, worker.ModeGovulncheck, "testdata/module"}, vulndb)
if err != nil {
t.Fatal(err)
}
@@ -44,7 +69,7 @@
})
t.Run("binary", func(t *testing.T) {
- t.Skip("vulncheck.Binary may not support the Go version")
+ t.Skip("govulncheck may not support the Go version")
const binary = "testdata/module/vuln"
cmd := exec.Command("go build")
cmd.Dir = "testdata/module"
@@ -52,7 +77,7 @@
t.Fatal(derrors.IncludeStderr(err))
}
defer os.Remove(binary)
- res, err := runTest([]string{worker.ModeBinary, binary}, "testdata/vulndb")
+ res, err := runTest([]string{govulncheckPath, worker.ModeBinary, binary}, vulndb)
if err != nil {
t.Fatal(err)
}
@@ -69,25 +94,25 @@
{
name: "too few args",
args: []string{"testdata/module"},
- vulndb: "testdata/vulndb",
- want: "need two args",
+ vulndb: vulndb,
+ want: "need three args",
},
{
name: "no vulndb",
- args: []string{worker.ModeImports, "testdata/module"},
+ args: []string{govulncheckPath, worker.ModeGovulncheck, "testdata/module"},
vulndb: "does not exist",
- want: "no such file",
+ want: "exit status 1",
},
{
name: "no mode",
- args: []string{"MODE", "testdata/module"},
- vulndb: "testdata/vulndb",
+ args: []string{govulncheckPath, "MODE", "testdata/module"},
+ vulndb: vulndb,
want: "not a valid mode",
},
{
name: "no module",
- args: []string{worker.ModeImports, "testdata/nosuchmodule"},
- vulndb: "testdata/vulndb",
+ args: []string{govulncheckPath, worker.ModeGovulncheck, "testdata/nosuchmodule"},
+ vulndb: vulndb,
want: "no such file",
},
} {
@@ -103,25 +128,8 @@
}
}
-func runTest(args []string, vulndbDir string) (*vulncheck.Result, error) {
+func runTest(args []string, vulndbDir string) (*govulncheckapi.Result, error) {
var buf bytes.Buffer
run(&buf, args, vulndbDir)
- return unmarshalVulncheckOutput(buf.Bytes())
-}
-
-func unmarshalVulncheckOutput(output []byte) (*vulncheck.Result, error) {
- var e struct {
- Error string
- }
- if err := json.Unmarshal(output, &e); err != nil {
- return nil, err
- }
- if e.Error != "" {
- return nil, errors.New(e.Error)
- }
- var res vulncheck.Result
- if err := json.Unmarshal(output, &res); err != nil {
- return nil, err
- }
- return &res, nil
+ return govulncheck.UnmarshalGovulncheckResult(buf.Bytes())
}
diff --git a/internal/buildtest/buildtest.go b/internal/buildtest/buildtest.go
index 559ca08..d0c52b9 100644
--- a/internal/buildtest/buildtest.go
+++ b/internal/buildtest/buildtest.go
@@ -3,7 +3,7 @@
// license that can be found in the LICENSE file.
// Package buildtest provides support for running "go build"
-// in tests.
+// and similar build/installation commands in tests.
package buildtest
import (
@@ -95,3 +95,20 @@
}
return defaultValue
}
+
+// InstallGovulncheck installs the latest version of govulncheck in
+// tmpDir. If the installation is successful, returns the full path
+// to the binary. Otherwise, returns the error. It uses the Go caches
+// as defined by go env.
+//
+// TODO: can we redirect caches to tmpDir as well? Currently, deletion
+// of tmpDir will cause "unlinkat: ... permission denied" error.
+func InstallGovulncheck(tmpDir string) (string, error) {
+ cmd := exec.Command("go", "install", "golang.org/x/vuln/cmd/govulncheck@latest")
+ cmd.Env = append(cmd.Environ(), "GOBIN="+tmpDir)
+ _, err := cmd.CombinedOutput()
+ if err != nil {
+ return "", err
+ }
+ return filepath.Join(tmpDir, "govulncheck"), nil
+}
diff --git a/internal/buildtest/buildtest_test.go b/internal/buildtest/buildtest_test.go
new file mode 100644
index 0000000..b178a1b
--- /dev/null
+++ b/internal/buildtest/buildtest_test.go
@@ -0,0 +1,27 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package buildtest
+
+import (
+ "os"
+ "testing"
+)
+
+func TestInstallGovulncheck(t *testing.T) {
+ t.Skip("breaks on trybots")
+ tempDir, err := os.MkdirTemp("", "testInstallGovulncheck")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer func() {
+ if err := os.RemoveAll(tempDir); err != nil {
+ t.Fatal(err)
+ }
+ }()
+
+ if _, err := InstallGovulncheck(tempDir); err != nil {
+ t.Fatal(err)
+ }
+}
diff --git a/internal/worker/mem_monitor.go b/internal/worker/mem_monitor.go
deleted file mode 100644
index 48cf2d7..0000000
--- a/internal/worker/mem_monitor.go
+++ /dev/null
@@ -1,56 +0,0 @@
-// Copyright 2022 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package worker
-
-import "time"
-
-// memMonitor is used to probe memory consumption
-// in a separate Go routine.
-type memMonitor struct {
- stp chan struct{} // for stopping the monitor
- res chan uint64 // for communicating results
-}
-
-// newMemMonitor creates and starts new monitor that
-// samples memory consumption.
-//
-// If threshold > 0, then when memory consumption reaches threshold,
-// the monitor will call onThreshold and stop.
-func newMemMonitor(threshold uint64, onThreshold func()) *memMonitor {
- m := &memMonitor{make(chan struct{}), make(chan uint64)}
- var max uint64
- go func() {
- for {
- select {
- case <-m.stp:
- m.res <- max
- return
- default:
- h := currHeapUsage()
- if h > max {
- max = h
- }
- if threshold > 0 && h > threshold {
- onThreshold()
- m.res <- max
- return
- }
- // We sample memory every 50ms.
- time.Sleep(50 * time.Millisecond)
- }
- }
- }()
- return m
-}
-
-// stop stops the monitor and returns the peak
-// memory consumption in bytes.
-func (m *memMonitor) stop() uint64 {
- if m == nil {
- return 0
- }
- close(m.stp)
- return <-m.res
-}
diff --git a/internal/worker/vulncheck_enqueue_test.go b/internal/worker/vulncheck_enqueue_test.go
index b5e18ac..6e4535b 100644
--- a/internal/worker/vulncheck_enqueue_test.go
+++ b/internal/worker/vulncheck_enqueue_test.go
@@ -80,12 +80,10 @@
if err != nil {
t.Fatal(err)
}
- wantTasks = nil
// cfg.BinaryBucket is empty, so no binary-mode tasks are created.
- for _, mode := range []string{ModeGovulncheck, ModeImports} {
- wantTasks = append(wantTasks,
- vreq("github.com/pkg/errors", "v0.9.1", mode, 10),
- vreq("golang.org/x/net", "v0.4.0", mode, 20))
+ wantTasks = []queue.Task{
+ vreq("github.com/pkg/errors", "v0.9.1", ModeGovulncheck, 10),
+ vreq("golang.org/x/net", "v0.4.0", ModeGovulncheck, 20),
}
if diff := cmp.Diff(wantTasks, gotTasks, cmp.AllowUnexported(ivulncheck.Request{})); diff != "" {
@@ -100,9 +98,8 @@
want []string
wantErr bool
}{
- {"", true, []string{ModeBinary, ModeGovulncheck, ModeImports}, false},
+ {"", true, []string{ModeBinary, ModeGovulncheck}, false},
{"", false, []string{ModeGovulncheck}, false},
- {"imports", false, []string{ModeImports}, false},
{"imports", true, nil, true},
} {
t.Run(fmt.Sprintf("%q,%t", test.param, test.all), func(t *testing.T) {
diff --git a/internal/worker/vulncheck_scan.go b/internal/worker/vulncheck_scan.go
index fea0667..1a8ca86 100644
--- a/internal/worker/vulncheck_scan.go
+++ b/internal/worker/vulncheck_scan.go
@@ -6,7 +6,6 @@
import (
"context"
- "encoding/json"
"errors"
"fmt"
"io"
@@ -14,7 +13,6 @@
"os"
"os/exec"
"path/filepath"
- "runtime"
"strconv"
"strings"
"time"
@@ -24,21 +22,24 @@
"golang.org/x/pkgsite-metrics/internal/bigquery"
"golang.org/x/pkgsite-metrics/internal/config"
"golang.org/x/pkgsite-metrics/internal/derrors"
- "golang.org/x/pkgsite-metrics/internal/load"
+ "golang.org/x/pkgsite-metrics/internal/govulncheck"
"golang.org/x/pkgsite-metrics/internal/log"
- "golang.org/x/pkgsite-metrics/internal/modules"
"golang.org/x/pkgsite-metrics/internal/proxy"
"golang.org/x/pkgsite-metrics/internal/sandbox"
"golang.org/x/pkgsite-metrics/internal/version"
ivulncheck "golang.org/x/pkgsite-metrics/internal/vulncheck"
vulnclient "golang.org/x/vuln/client"
- "golang.org/x/vuln/exp/govulncheck"
- "golang.org/x/vuln/vulncheck"
+ govulncheckapi "golang.org/x/vuln/exp/govulncheck"
)
const (
- // ModeImports performs import-level analysis.
- ModeImports string = "IMPORTS"
+ // modeImports is used to report results of
+ // vulnerability detection at imports level
+ // precision. It cannot be directly triggered
+ // by scan endpoints. Instead, ModeGovulncheck
+ // mode reports its results to show difference
+ // in precision of vulnerability detection.
+ modeImports string = "IMPORTS"
// ModeBinary runs the govulncheck binary in
// binary mode.
@@ -51,7 +52,6 @@
// modes is a set of supported vulncheck modes
var modes = map[string]bool{
- ModeImports: true,
ModeBinary: true,
ModeGovulncheck: true,
}
@@ -201,7 +201,6 @@
vulns, err := s.runScanModule(ctx, sreq.Module, info.Version, sreq.Suffix, sreq.Mode, stats)
row.ScanSeconds = stats.scanSeconds
row.ScanMemory = int64(stats.scanMemory)
- row.PkgsMemory = int64(stats.pkgsMemory)
row.Workers = config.GetEnvInt("CLOUD_RUN_CONCURRENCY", "0", -1)
if err != nil {
switch {
@@ -224,16 +223,67 @@
}
row.AddError(err)
} else {
- row.Vulns = vulns
+ row.Vulns = vulnsForMode(vulns, sreq.Mode)
}
- log.Infof(ctx, "done with scanner.runScanModule: %s@%s #vulns=%d err=%v", sreq.Path(), sreq.Version, len(vulns), err)
- return writeResult(ctx, sreq.Serve, w, s.bqClient, ivulncheck.TableName, row)
+ log.Infof(ctx, "scanner.runScanModule returned %d vulns for %s: row.Vulns=%d err=%v", len(vulns), sreq.Path(), len(row.Vulns), err)
+
+ if err := writeResult(ctx, sreq.Serve, w, s.bqClient, ivulncheck.TableName, row); err != nil {
+ return err
+ }
+
+ if sreq.Mode != ModeGovulncheck {
+ return nil
+ }
+ // For ModeGovulncheck, add the copy of row and report
+ // each vulnerability as imported. We set the performance
+ // numbers to 0 since we don't actually perform a scan
+ // at the level of import chains. Also makes a copy if
+ // the original row has an error and no vulns.
+ impRow := *row
+ impRow.ScanMode = modeImports
+ impRow.ScanSeconds = 0
+ impRow.ScanMemory = 0
+ impRow.Vulns = vulnsForMode(vulns, modeImports)
+ log.Infof(ctx, "scanner.runScanModule also storing imports vulns for %s: row.Vulns=%d", sreq.Path(), len(impRow.Vulns))
+ return writeResult(ctx, sreq.Serve, w, s.bqClient, ivulncheck.TableName, &impRow)
+}
+
+// vulnsForMode returns vulns that make sense to report for
+// a particular mode.
+//
+// For ModeGovulncheck, these are all vulns that are actually
+// called (CallSink!=0). For modeImports, these are all vulns
+// modified to have CallSink=0. For ModeBinary, these are
+// exactly the input vulns since binary analysis does not
+// distinguish between called and imported vulnerabilities.
+func vulnsForMode(vulns []*ivulncheck.Vuln, mode string) []*ivulncheck.Vuln {
+ if mode == ModeBinary {
+ return vulns
+ }
+
+ var vs []*ivulncheck.Vuln
+ for _, v := range vulns {
+ if mode == ModeGovulncheck {
+ // Return only the called vulns for ModeGovulncheck.
+ if v.CallSink.Valid && v.CallSink.Int64 != 0 {
+ vs = append(vs, v)
+ }
+ } else if mode == modeImports {
+ // For imports mode, return the vulnerability as it
+ // is imported, but not called.
+ nv := *v
+ nv.CallSink = bigquery.NullInt(0)
+ vs = append(vs, &nv)
+ } else {
+ panic(fmt.Sprintf("vulnsForMode unsupported mode %s", mode))
+ }
+ }
+ return vs
}
type vulncheckStats struct {
scanSeconds float64
scanMemory uint64
- pkgsMemory uint64
}
// Inside the sandbox, the user is root and their $HOME directory is /root.
@@ -242,130 +292,33 @@
sandboxGoModCache = "root/go/pkg/mod"
// The Go cache resides in its default location, $HOME/.cache/go-build.
sandboxGoCache = "root/.cache/go-build"
+
+ // Within the sandbox, govulncheck will be located at /binaries/govulncheck path.
+ sandboxGovulncheck = "/binaries/govulncheck"
)
// runScanModule fetches the module version from the proxy, and analyzes it for
// vulnerabilities.
func (s *scanner) runScanModule(ctx context.Context, modulePath, version, binaryDir, mode string, stats *vulncheckStats) (bvulns []*ivulncheck.Vuln, err error) {
err = doScan(ctx, modulePath, version, s.insecure, func() error {
- if mode == ModeImports {
- var vulns []*vulncheck.Vuln
- if s.insecure {
- vulns, err = s.runImportsScanInsecure(ctx, modulePath, version, stats)
- } else {
- vulns, err = s.runImportsScanSandbox(ctx, modulePath, version, stats)
- }
- if err != nil {
- return err
- }
- for _, v := range vulns {
- bvulns = append(bvulns, ivulncheck.ConvertVulncheckOutput(v))
- }
+ var vulns []*govulncheckapi.Vuln
+ if s.insecure {
+ vulns, err = s.runGovulncheckScanInsecure(ctx, modulePath, version, binaryDir, mode, stats)
} else {
- var vulns []*govulncheck.Vuln
- if s.insecure {
- vulns, err = s.runGovulncheckScanInsecure(ctx, modulePath, version, binaryDir, mode, stats)
- } else {
- vulns, err = s.runGovulncheckScanSandbox(ctx, modulePath, version, binaryDir, mode, stats)
- }
- if err != nil {
- return err
- }
- for _, v := range vulns {
- bvulns = append(bvulns, ivulncheck.ConvertGovulncheckOutput(v)...)
- }
+ vulns, err = s.runGovulncheckScanSandbox(ctx, modulePath, version, binaryDir, mode, stats)
+ }
+ if err != nil {
+ return err
+ }
+ for _, v := range vulns {
+ bvulns = append(bvulns, ivulncheck.ConvertGovulncheckOutput(v)...)
}
return nil
})
return bvulns, err
}
-func (s *scanner) runImportsScanInsecure(ctx context.Context, modulePath, version string, stats *vulncheckStats) (_ []*vulncheck.Vuln, err error) {
- tempDir, err := os.MkdirTemp("", "runImportsScan")
- if err != nil {
- return nil, err
- }
- defer removeDir(&err, tempDir)
-
- log.Debugf(ctx, "fetching module zip: %s@%s", modulePath, version)
- if err = modules.Download(ctx, modulePath, version, tempDir, s.proxyClient, true); err != nil {
- return nil, err
- }
-
- cctx, cancel := context.WithCancel(ctx)
- defer cancel()
-
- cfg := load.DefaultConfig()
- cfg.Dir = tempDir // filepath.Join(dir, modulePath+"@"+version,
- cfg.Context = cctx
-
- runtime.GC()
- // current memory not related to core (go)vulncheck operations.
- preScanMemory := currHeapUsage()
-
- log.Debugf(ctx, "loading packages: %s@%s", modulePath, version)
- pkgs, pkgErrors, err := load.Packages(cfg, "./...")
- if err == nil && len(pkgErrors) > 0 {
- err = fmt.Errorf("%v", pkgErrors)
- }
- if err != nil {
- return nil, err
- }
-
- stats.pkgsMemory = memSubtract(currHeapUsage(), preScanMemory)
-
- // Run vulncheck.Source and collect results.
- start := time.Now()
- vcfg := &vulncheck.Config{Client: s.dbClient, ImportsOnly: true}
- res, peakMem, err := s.runWithMemoryMonitor(ctx, func() (*vulncheck.Result, error) {
- log.Infof(ctx, "running imports analysis (vulncheck.Source): %s@%s", modulePath, version)
- res, err := vulncheck.Source(cctx, vulncheck.Convert(pkgs), vcfg)
- log.Infof(ctx, "done with imports analysis (vulncheck.Source): %s@%s, err=%v", modulePath, version, err)
- if err != nil {
- return res, err
- }
- return res, nil
- })
- // scanMemory is peak heap memory used during vulncheck + pkgs.
- // We subtract any memory not related to these core (go)vulncheck
- // operations.
- stats.scanMemory = memSubtract(peakMem, preScanMemory)
-
- // scanSeconds is the time it took for vulncheck.Source to run.
- // We want to know this information regardless of whether an error
- // occurred.
- stats.scanSeconds = time.Since(start).Seconds()
- if err != nil {
- return nil, err
- }
- return res.Vulns, nil
-}
-
-func (s *scanner) runImportsScanSandbox(ctx context.Context, modulePath, version string, stats *vulncheckStats) (_ []*vulncheck.Vuln, err error) {
- const insecure = false
- mdir := moduleDir(modulePath, version, insecure)
- defer removeDir(&err, mdir)
- if err := prepareModule(ctx, modulePath, version, mdir, s.proxyClient, insecure); err != nil {
- return nil, err
- }
-
- log.Infof(ctx, "running imports analysis in sandbox: %s@%s", modulePath, version)
-
- smdir := strings.TrimPrefix(mdir, sandboxRoot)
- stdout, err := s.sbox.Command("/binaries/vulncheck_sandbox", ModeImports, smdir).Output()
- log.Infof(ctx, "done with imports analysis in sandbox: %s@%s err=%v", modulePath, version, err)
-
- if err != nil {
- return nil, errors.New(derrors.IncludeStderr(err))
- }
- res, err := unmarshalVulncheckOutput(stdout)
- if err != nil {
- return nil, err
- }
- return res.Vulns, nil
-}
-
-func (s *scanner) runGovulncheckScanSandbox(ctx context.Context, modulePath, version, binaryDir, mode string, stats *vulncheckStats) (_ []*govulncheck.Vuln, err error) {
+func (s *scanner) runGovulncheckScanSandbox(ctx context.Context, modulePath, version, binaryDir, mode string, stats *vulncheckStats) (_ []*govulncheckapi.Vuln, err error) {
if mode == ModeBinary {
return s.runBinaryScanSandbox(ctx, modulePath, version, binaryDir, stats)
}
@@ -379,20 +332,20 @@
log.Infof(ctx, "running govulncheck in sandbox: %s@%s", modulePath, version)
smdir := strings.TrimPrefix(mdir, sandboxRoot)
- stdout, err := s.sbox.Command("/binaries/vulncheck_sandbox", ModeGovulncheck, smdir).Output()
+ stdout, err := s.sbox.Command("/binaries/vulncheck_sandbox", sandboxGovulncheck, ModeGovulncheck, smdir).Output()
log.Infof(ctx, "done with govulncheck in sandbox: %s@%s err=%v", modulePath, version, err)
if err != nil {
return nil, errors.New(derrors.IncludeStderr(err))
}
- res, err := unmarshalGovulncheckOutput(stdout)
+ res, err := govulncheck.UnmarshalGovulncheckResult(stdout)
if err != nil {
return nil, err
}
return res.Vulns, nil
}
-func (s *scanner) runBinaryScanSandbox(ctx context.Context, modulePath, version, binDir string, stats *vulncheckStats) ([]*govulncheck.Vuln, error) {
+func (s *scanner) runBinaryScanSandbox(ctx context.Context, modulePath, version, binDir string, stats *vulncheckStats) ([]*govulncheckapi.Vuln, error) {
if s.gcsBucket == nil {
return nil, errors.New("binary bucket not configured; set GO_ECOSYSTEM_BINARY_BUCKET")
}
@@ -420,20 +373,20 @@
}
log.Infof(ctx, "running vulncheck in sandbox on %s: %s@%s/%s", modulePath, version, binDir, destf.Name())
- stdout, err := s.sbox.Command("/binaries/vulncheck_sandbox", ModeBinary, destf.Name()).Output()
+ stdout, err := s.sbox.Command("/binaries/vulncheck_sandbox", sandboxGovulncheck, ModeBinary, destf.Name()).Output()
log.Infof(ctx, "done with vulncheck in sandbox on %s: %s@%s/%s err=%v", modulePath, version, binDir, destf.Name(), err)
if err != nil {
return nil, errors.New(derrors.IncludeStderr(err))
}
- res, err := unmarshalGovulncheckOutput(stdout)
+ res, err := govulncheck.UnmarshalGovulncheckResult(stdout)
if err != nil {
return nil, err
}
return res.Vulns, nil
}
-func (s *scanner) runGovulncheckScanInsecure(ctx context.Context, modulePath, version, binaryDir, mode string, stats *vulncheckStats) (_ []*govulncheck.Vuln, err error) {
+func (s *scanner) runGovulncheckScanInsecure(ctx context.Context, modulePath, version, binaryDir, mode string, stats *vulncheckStats) (_ []*govulncheckapi.Vuln, err error) {
if mode == ModeBinary {
return s.runBinaryScanInsecure(ctx, modulePath, version, binaryDir, os.TempDir(), stats)
}
@@ -452,7 +405,7 @@
return vulns, nil
}
-func (s *scanner) runBinaryScanInsecure(ctx context.Context, modulePath, version, binDir, tempDir string, stats *vulncheckStats) ([]*govulncheck.Vuln, error) {
+func (s *scanner) runBinaryScanInsecure(ctx context.Context, modulePath, version, binDir, tempDir string, stats *vulncheckStats) ([]*govulncheckapi.Vuln, error) {
if s.gcsBucket == nil {
return nil, errors.New("binary bucket not configured; set GO_ECOSYSTEM_BINARY_BUCKET")
}
@@ -475,7 +428,7 @@
return vulns, nil
}
-func runGovulncheckCmd(ctx context.Context, pattern, tempDir string, stats *vulncheckStats) ([]*govulncheck.Vuln, error) {
+func runGovulncheckCmd(ctx context.Context, pattern, tempDir string, stats *vulncheckStats) ([]*govulncheckapi.Vuln, error) {
govulncheckName := "/bundle/rootfs/binaries/govulncheck"
if !fileExists(govulncheckName) {
govulncheckName = "govulncheck"
@@ -486,71 +439,13 @@
if err != nil {
return nil, err
}
- res, err := unmarshalGovulncheckOutput(output)
+ res, err := govulncheck.UnmarshalGovulncheckResult(output)
if err != nil {
return nil, err
}
return res.Vulns, nil
}
-func unmarshalVulncheckOutput(output []byte) (*vulncheck.Result, error) {
- var e struct {
- Error string
- }
- if err := json.Unmarshal(output, &e); err != nil {
- return nil, err
- }
- if e.Error != "" {
- return nil, errors.New(e.Error)
- }
- var res vulncheck.Result
- if err := json.Unmarshal(output, &res); err != nil {
- return nil, err
- }
- return &res, nil
-}
-
-func unmarshalGovulncheckOutput(output []byte) (*govulncheck.Result, error) {
- var e struct {
- Error string
- }
- if err := json.Unmarshal(output, &e); err != nil {
- return nil, err
- }
- if e.Error != "" {
- return nil, errors.New(e.Error)
- }
- var res govulncheck.Result
- if err := json.Unmarshal(output, &res); err != nil {
- return nil, err
- }
- return &res, nil
-}
-
-// runWithMemoryMonitor runs f in a goroutine with its memory tracked.
-// It returns f's peak memory usage.
-func (s *scanner) runWithMemoryMonitor(ctx context.Context, f func() (*vulncheck.Result, error)) (res *vulncheck.Result, mem uint64, err error) {
- cctx, cancel := context.WithCancel(ctx)
- monitor := newMemMonitor(s.goMemLimit, cancel)
- type sr struct {
- res *vulncheck.Result
- err error
- }
- srchan := make(chan sr)
- go func() {
- res, err := f()
- srchan <- sr{res, err}
- }()
- select {
- case r := <-srchan:
- res = r.res
- err = r.err
- case <-cctx.Done():
- err = derrors.ScanModuleMemoryLimitExceeded
- }
- return res, monitor.stop(), err
-}
-
func copyFromGCSToWriter(ctx context.Context, w io.Writer, bucket *storage.BucketHandle, srcPath string) error {
gcsReader, err := bucket.Object(srcPath).NewReader(ctx)
if err != nil {
@@ -578,22 +473,6 @@
strings.Contains(s, "connection")
}
-// currHeapUsage computes currently allocate heap bytes.
-func currHeapUsage() uint64 {
- var stats runtime.MemStats
- runtime.ReadMemStats(&stats)
- return stats.Alloc
-}
-
-// memSubtract subtracts memory usage m2 from m1, returning
-// 0 if the result is negative.
-func memSubtract(m1, m2 uint64) uint64 {
- if m1 <= m2 {
- return 0
- }
- return m1 - m2
-}
-
// parseGoMemLimit parses the GOMEMLIMIT environment variable.
// It returns 0 if the variable isn't set or its value is malformed.
func parseGoMemLimit(s string) uint64 {
diff --git a/internal/worker/vulncheck_scan_test.go b/internal/worker/vulncheck_scan_test.go
index 4e8caaa..907a5e0 100644
--- a/internal/worker/vulncheck_scan_test.go
+++ b/internal/worker/vulncheck_scan_test.go
@@ -6,19 +6,20 @@
import (
"context"
- "encoding/json"
"errors"
"flag"
+ "fmt"
"io"
+ "strings"
"testing"
"cloud.google.com/go/storage"
- "github.com/google/go-cmp/cmp"
+ "golang.org/x/pkgsite-metrics/internal/bigquery"
"golang.org/x/pkgsite-metrics/internal/config"
"golang.org/x/pkgsite-metrics/internal/derrors"
"golang.org/x/pkgsite-metrics/internal/proxy"
+ ivulncheck "golang.org/x/pkgsite-metrics/internal/vulncheck"
vulnc "golang.org/x/vuln/client"
- "golang.org/x/vuln/vulncheck"
)
var integration = flag.Bool("integration", false, "test against actual service")
@@ -33,6 +34,39 @@
check(scanError{io.EOF}, true)
}
+func TestVulnsForMode(t *testing.T) {
+ vulns := []*ivulncheck.Vuln{
+ &ivulncheck.Vuln{Symbol: "A", CallSink: bigquery.NullInt(0)},
+ &ivulncheck.Vuln{Symbol: "B"},
+ &ivulncheck.Vuln{Symbol: "C", CallSink: bigquery.NullInt(9)},
+ }
+
+ vulnsStr := func(vulns []*ivulncheck.Vuln) string {
+ var vs []string
+ for _, v := range vulns {
+ vs = append(vs, fmt.Sprintf("%s:%d", v.Symbol, v.CallSink.Int64))
+ }
+ return strings.Join(vs, ", ")
+ }
+
+ for _, tc := range []struct {
+ mode string
+ want string
+ }{
+ {modeImports, "A:0, B:0, C:0"},
+ {ModeGovulncheck, "C:9"},
+ {ModeBinary, "A:0, B:0, C:9"},
+ } {
+ tc := tc
+ t.Run(tc.mode, func(t *testing.T) {
+ modeVulns := vulnsForMode(vulns, tc.mode)
+ if got := vulnsStr(modeVulns); got != tc.want {
+ t.Errorf("got %s; want %s", got, tc.want)
+ }
+ })
+ }
+}
+
func TestRunScanModule(t *testing.T) {
t.Skip("breaks on trybots")
@@ -72,9 +106,6 @@
if got := stats.scanMemory; got <= 0 {
t.Errorf("scan memory not collected or negative: %v", got)
}
- if got := stats.pkgsMemory; got <= 0 {
- t.Errorf("pkgs memory not collected or negative: %v", got)
- }
})
t.Run("memoryLimit", func(t *testing.T) {
s := &scanner{proxyClient: proxyClient, dbClient: dbClient, insecure: true, goMemLimit: 2000}
@@ -147,24 +178,3 @@
}
}
}
-
-func TestUnmarshalVulncheckOutput(t *testing.T) {
- _, err := unmarshalVulncheckOutput([]byte(`{"Error": "bad"}`))
- if got, want := err.Error(), "bad"; got != want {
- t.Errorf("got %q, want %q", got, want)
- }
- want := &vulncheck.Result{
- Modules: []*vulncheck.Module{{Path: "m", Version: "v1.2.3"}},
- }
- in, err := json.Marshal(want)
- if err != nil {
- t.Fatal(err)
- }
- got, err := unmarshalVulncheckOutput(in)
- if err != nil {
- t.Fatal(err)
- }
- if !cmp.Equal(got, want) {
- t.Errorf("got %+v, want %+v", got, want)
- }
-}
