cmd/govulncheck: fix exit code propagation for wrapped scan errors Previously, exit codes were masked improperly. Ensure that the original error codes are propagated. Fixes golang/go#78694 Cq-Include-Trybots: luci.golang.try:x_vuln-gotip-linux-amd64-longtest Change-Id: Id94878d231272a9a37817ece747692a1ad539cc6 Reviewed-on: https://go-review.googlesource.com/c/vuln/+/786440 Auto-Submit: Ethan Lee <ethanalee@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/cmd/govulncheck/main.go b/cmd/govulncheck/main.go index 73e3370..b15397d 100644 --- a/cmd/govulncheck/main.go +++ b/cmd/govulncheck/main.go
@@ -6,6 +6,7 @@ import ( "context" + "errors" "fmt" "os" @@ -23,11 +24,20 @@ if err == nil { err = cmd.Wait() } - switch err := err.(type) { - case nil: - case interface{ ExitCode() int }: - os.Exit(err.ExitCode()) - default: + if err != nil { + var e interface{ ExitCode() int } + if errors.As(err, &e) { + printErrorToStderr := true + if _, ok := err.(interface{ ExitCode() int }); ok { + // Avoid printing the error to stderr if the exit code error wasn't + // wrapped with another error providing context. + printErrorToStderr = false + } + if printErrorToStderr { + fmt.Fprintln(os.Stderr, err) + } + os.Exit(e.ExitCode()) + } fmt.Fprintln(os.Stderr, err) os.Exit(1) }
diff --git a/cmd/govulncheck/main_test.go b/cmd/govulncheck/main_test.go index 5613de5..1e7c57a 100644 --- a/cmd/govulncheck/main_test.go +++ b/cmd/govulncheck/main_test.go
@@ -7,6 +7,7 @@ import ( "bytes" "context" + "errors" "flag" "fmt" "os" @@ -155,16 +156,27 @@ return nil, err } err := cmd.Wait() - switch e := err.(type) { - case nil: - case interface{ ExitCode() int }: - err = &cmdtest.ExitCodeErr{Msg: err.Error(), Code: e.ExitCode()} - if e.ExitCode() == 0 { - err = nil + if err != nil { + var e interface{ ExitCode() int } + if errors.As(err, &e) { + code := e.ExitCode() + printErrorToStderr := true + if _, ok := err.(interface{ ExitCode() int }); ok { + // Avoid printing the error to stderr if the exit code error wasn't + // wrapped with another error providing context. + printErrorToStderr = false + } + if printErrorToStderr { + fmt.Fprintln(buf, err) + } + err = &cmdtest.ExitCodeErr{Msg: err.Error(), Code: code} + if code == 0 { + err = nil + } + } else { + fmt.Fprintln(buf, err) + err = &cmdtest.ExitCodeErr{Msg: err.Error(), Code: 1} } - default: - fmt.Fprintln(buf, err) - err = &cmdtest.ExitCodeErr{Msg: err.Error(), Code: 1} } sorted := buf if err == nil && isJSONMode(args) {
diff --git a/cmd/govulncheck/testdata/common/testfiles/usage/format.ct b/cmd/govulncheck/testdata/common/testfiles/usage/format.ct index a2747e0..8bd4843 100644 --- a/cmd/govulncheck/testdata/common/testfiles/usage/format.ct +++ b/cmd/govulncheck/testdata/common/testfiles/usage/format.ct
@@ -12,7 +12,7 @@ Use '-show verbose' for more details. # Test of explicit json format -$ govulncheck -C ${moddir}/informational -format json +$ govulncheck -C ${moddir}/informational -format json . { "config": { "protocol_version": "v1.0.0", @@ -25,3 +25,244 @@ "scan_mode": "source" } } +{ + "progress": { + "message": "Fetching vulnerabilities from the database..." + } +} +{ + "progress": { + "message": "Checking the code against the vulnerabilities..." + } +} +{ + "osv": { + "schema_version": "1.3.1", + "id": "GO-2021-0265", + "modified": "2023-04-03T15:57:51Z", + "published": "2022-08-15T18:06:07Z", + "aliases": [ + "CVE-2021-42248", + "CVE-2021-42836", + "GHSA-c9gm-7rfj-8w5h", + "GHSA-ppj4-34rq-v8j9" + ], + "details": "A maliciously crafted path can cause Get and other query functions to consume excessive amounts of CPU and time.", + "affected": [ + { + "package": { + "name": "github.com/tidwall/gjson", + "ecosystem": "Go" + }, + "ranges": [ + { + "type": "SEMVER", + "events": [ + { + "introduced": "0" + }, + { + "fixed": "1.9.3" + } + ] + } + ], + "ecosystem_specific": { + "imports": [ + { + "path": "github.com/tidwall/gjson", + "symbols": [ + "Get", + "GetBytes", + "GetMany", + "GetManyBytes", + "Result.Get", + "parseObject", + "queryMatches" + ] + } + ] + } + } + ], + "references": [ + { + "type": "FIX", + "url": "https://github.com/tidwall/gjson/commit/77a57fda87dca6d0d7d4627d512a630f89a91c96" + }, + { + "type": "WEB", + "url": "https://github.com/tidwall/gjson/issues/237" + }, + { + "type": "WEB", + "url": "https://github.com/tidwall/gjson/issues/236" + }, + { + "type": "WEB", + "url": "https://github.com/tidwall/gjson/commit/590010fdac311cc8990ef5c97448d4fec8f29944" + } + ], + "database_specific": { + "url": "https://pkg.go.dev/vuln/GO-2021-0265" + } + } +} +{ + "finding": { + "osv": "GO-2021-0265", + "fixed_version": "v1.9.3", + "trace": [ + { + "module": "github.com/tidwall/gjson", + "version": "v1.9.2" + } + ] + } +} +{ + "finding": { + "osv": "GO-2021-0265", + "fixed_version": "v1.9.3", + "trace": [ + { + "module": "github.com/tidwall/gjson", + "version": "v1.9.2", + "package": "github.com/tidwall/gjson" + } + ] + } +} +{ + "osv": { + "schema_version": "1.3.1", + "id": "GO-2021-0054", + "modified": "2023-04-03T15:57:51Z", + "published": "2021-04-14T20:04:52Z", + "aliases": [ + "CVE-2020-36067", + "GHSA-p64j-r5f4-pwwx" + ], + "details": "Due to improper bounds checking, maliciously crafted JSON objects can cause an out-of-bounds panic. If parsing user input, this may be used as a denial of service vector.", + "affected": [ + { + "package": { + "name": "github.com/tidwall/gjson", + "ecosystem": "Go" + }, + "ranges": [ + { + "type": "SEMVER", + "events": [ + { + "introduced": "0" + }, + { + "fixed": "1.6.6" + } + ] + } + ], + "ecosystem_specific": { + "imports": [ + { + "path": "github.com/tidwall/gjson", + "symbols": [ + "Result.ForEach", + "unwrap" + ] + } + ] + } + } + ], + "references": [ + { + "type": "FIX", + "url": "https://github.com/tidwall/gjson/commit/bf4efcb3c18d1825b2988603dea5909140a5302b" + }, + { + "type": "WEB", + "url": "https://github.com/tidwall/gjson/issues/196" + } + ], + "credits": [ + { + "name": "@toptotu" + } + ], + "database_specific": { + "url": "https://pkg.go.dev/vuln/GO-2021-0054" + } + } +} +{ + "osv": { + "schema_version": "1.3.1", + "id": "GO-2021-0059", + "modified": "2023-04-03T15:57:51Z", + "published": "2021-04-14T20:04:52Z", + "aliases": [ + "CVE-2020-35380", + "GHSA-w942-gw6m-p62c" + ], + "details": "Due to improper bounds checking, maliciously crafted JSON objects can cause an out-of-bounds panic. If parsing user input, this may be used as a denial of service vector.", + "affected": [ + { + "package": { + "name": "github.com/tidwall/gjson", + "ecosystem": "Go" + }, + "ranges": [ + { + "type": "SEMVER", + "events": [ + { + "introduced": "0" + }, + { + "fixed": "1.6.4" + } + ] + } + ], + "ecosystem_specific": { + "imports": [ + { + "path": "github.com/tidwall/gjson", + "symbols": [ + "Get", + "GetBytes", + "GetMany", + "GetManyBytes", + "Result.Array", + "Result.Get", + "Result.Map", + "Result.Value", + "squash" + ] + } + ] + } + } + ], + "references": [ + { + "type": "FIX", + "url": "https://github.com/tidwall/gjson/commit/f0ee9ebde4b619767ae4ac03e8e42addb530f6bc" + }, + { + "type": "WEB", + "url": "https://github.com/tidwall/gjson/issues/192" + } + ], + "credits": [ + { + "name": "@toptotu" + } + ], + "database_specific": { + "url": "https://pkg.go.dev/vuln/GO-2021-0059" + } + } +}
diff --git a/cmd/govulncheck/testdata/common/testfiles/usage/source_no_packages.ct b/cmd/govulncheck/testdata/common/testfiles/usage/source_no_packages.ct index 27d44a9..e783d31 100644 --- a/cmd/govulncheck/testdata/common/testfiles/usage/source_no_packages.ct +++ b/cmd/govulncheck/testdata/common/testfiles/usage/source_no_packages.ct
@@ -1,5 +1,4 @@ ##### # Test message when there are no packages matching the provided pattern (#59623). -$ govulncheck -show verbose -C ${moddir}/vuln pkg/no-govulncheck/... -No packages matched the provided pattern. -No vulnerabilities found. +$ govulncheck -show verbose -C ${moddir}/vuln pkg/no-govulncheck/... --> FAIL 2 +govulncheck: no packages matched the provided patterns
diff --git a/cmd/govulncheck/testdata/common/testfiles/usage/usage.ct b/cmd/govulncheck/testdata/common/testfiles/usage/usage.ct index ff2c2ae..a04b22b 100644 --- a/cmd/govulncheck/testdata/common/testfiles/usage/usage.ct +++ b/cmd/govulncheck/testdata/common/testfiles/usage/usage.ct
@@ -35,8 +35,10 @@ ##### # Not scanning anything. -$ govulncheck -No vulnerabilities found. +$ govulncheck --> FAIL 2 +govulncheck: no package patterns provided + +To scan the current module, run: govulncheck ./... ##### # Reporting version without scanning anything. @@ -45,5 +47,3 @@ Scanner: govulncheck@v1.0.0 DB: testdata/vulndb-v1 DB updated: 2023-04-03 15:57:51 +0000 UTC - -No vulnerabilities found.
diff --git a/internal/scan/run.go b/internal/scan/run.go index 5f6a641..560a3ad 100644 --- a/internal/scan/run.go +++ b/internal/scan/run.go
@@ -22,9 +22,8 @@ "golang.org/x/vuln/internal/sarif" ) -// RunGovulncheck performs main govulncheck functionality and exits the -// program upon success with an appropriate exit status. Otherwise, -// returns an error. +// RunGovulncheck performs main govulncheck functionality. +// On failure, the returned error wraps an exit code error (see scan.Cmd.Wait). func RunGovulncheck(ctx context.Context, env []string, r io.Reader, stdout io.Writer, stderr io.Writer, args []string) error { cfg := &config{env: env} if err := parseFlags(cfg, stderr, args); err != nil { @@ -110,7 +109,7 @@ // this binary used from the build info. func scannerVersion(cfg *config, bi *debug.BuildInfo) { if bi.Path != "" { - cfg.ScannerName = path.Base(bi.Path) + cfg.ScannerName = strings.TrimSuffix(path.Base(bi.Path), ".test") } if bi.Main.Version != "" && bi.Main.Version != "(devel)" { cfg.ScannerVersion = bi.Main.Version
diff --git a/scan/scan.go b/scan/scan.go index 0aa9975..459fc36 100644 --- a/scan/scan.go +++ b/scan/scan.go
@@ -89,6 +89,16 @@ // Wait waits for the command to exit. The command must have been started by // Start. // +// If the command fails to run or does not complete successfully, the returned +// error wraps an error implementing: +// +// interface { +// ExitCode() int +// } +// +// Callers can use errors.As to retrieve the exit code. Other errors may be +// returned for other problems (e.g. I/O issues). +// // Wait releases any resources associated with the Cmd. func (c *Cmd) Wait() error { if c.done == nil {