[vuln] cmd,internal/scan: conslidate error handling for flags
11 views
Skip to first unread message
Gopher Robot (Gerrit)
Apr 17, 2023, 4:25:52 PM4/17/23
to Julie Qiu, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Julie Qiu, Tatiana Bradley, golang-co...@googlegroups.com
Gopher Robot submitted this change.
Approvals:
Gopher Robot: TryBots succeeded
Julie Qiu: Looks good to me, but someone else must approve; Automatically submit change
Tatiana Bradley: Looks good to me, approved
Julie Qiu: Run TryBots
cmd,internal/scan: consolidate error handling for flags
Consolidate error handling for parsing flags to validateConfig. Add
tests for the various error outputs.
For golang/go#58945
Fixes golang/go#59139
Change-Id: Ic43f30b17dc830698bce62a6177c45ee4bfb8567
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/482345
Auto-Submit: Julie Qiu <juli...@google.com>
Run-TryBot: Julie Qiu <ju...@golang.org>
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
Reviewed-by: Julie Qiu <juli...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M cmd/govulncheck/main.go
A cmd/govulncheck/testdata/badmode.ct
A cmd/govulncheck/testdata/binarybadfile.ct
A cmd/govulncheck/testdata/binarymulti.ct
A cmd/govulncheck/testdata/binarynotags.ct
A cmd/govulncheck/testdata/binarynotest.ct
M cmd/govulncheck/testdata/nogomod.ct
M internal/scan/errors.go
M internal/scan/flags.go
M internal/scan/source.go
10 files changed, 89 insertions(+), 53 deletions(-)
diff --git a/cmd/govulncheck/main.go b/cmd/govulncheck/main.go
index 67ef789..eee8aa1 100644
--- a/cmd/govulncheck/main.go
+++ b/cmd/govulncheck/main.go
@@ -20,10 +20,12 @@
switch err {
case flag.ErrHelp:
os.Exit(0)
- case scan.ErrMissingArgPatterns:
- os.Exit(1)
case scan.ErrVulnerabilitiesFound:
os.Exit(3)
+ case scan.ErrNoPatterns:
+ // flag.Usage is printed in the case of this error, so do not print
+ // the actual error message.
+ os.Exit(1)
default:
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
diff --git a/cmd/govulncheck/testdata/badmode.ct b/cmd/govulncheck/testdata/badmode.ct
new file mode 100644
index 0000000..a435b42
--- /dev/null
+++ b/cmd/govulncheck/testdata/badmode.ct
@@ -0,0 +1,4 @@
+# Test of invalid input to -mode
+
+$ govulncheck -mode=invalid ./... --> FAIL 1
+govulncheck: "invalid" is not a valid mode
diff --git a/cmd/govulncheck/testdata/binarybadfile.ct b/cmd/govulncheck/testdata/binarybadfile.ct
new file mode 100644
index 0000000..d742414
--- /dev/null
+++ b/cmd/govulncheck/testdata/binarybadfile.ct
@@ -0,0 +1,4 @@
+# Test of passing a non-file to -mode=binary
+
+$ govulncheck -mode=binary notafile --> FAIL 1
+govulncheck: "notafile" is not a file
diff --git a/cmd/govulncheck/testdata/binarymulti.ct b/cmd/govulncheck/testdata/binarymulti.ct
new file mode 100644
index 0000000..e060de1
--- /dev/null
+++ b/cmd/govulncheck/testdata/binarymulti.ct
@@ -0,0 +1,4 @@
+# Test of trying to analyze multiple binaries
+
+$ govulncheck -mode=binary ${vuln_binary} ${vuln_binary} --> FAIL 1
+govulncheck: only 1 binary can be analyzed at a time
diff --git a/cmd/govulncheck/testdata/binarynotags.ct b/cmd/govulncheck/testdata/binarynotags.ct
new file mode 100644
index 0000000..0a9dab3
--- /dev/null
+++ b/cmd/govulncheck/testdata/binarynotags.ct
@@ -0,0 +1,4 @@
+# Test of trying to run -mode=binary with -tags flag
+
+$ govulncheck -tags=foo -mode=binary ${vuln_binary} --> FAIL 1
+govulncheck: the -tags flag is not supported in binary mode
diff --git a/cmd/govulncheck/testdata/binarynotest.ct b/cmd/govulncheck/testdata/binarynotest.ct
new file mode 100644
index 0000000..15fda5b
--- /dev/null
+++ b/cmd/govulncheck/testdata/binarynotest.ct
@@ -0,0 +1,4 @@
+# Test of trying to run -mode=binary with the -test flag
+
+$ govulncheck -test -mode=binary ${vuln_binary} --> FAIL 1
+govulncheck: the -test flag is not supported in binary mode
diff --git a/cmd/govulncheck/testdata/nogomod.ct b/cmd/govulncheck/testdata/nogomod.ct
index cfcc043..86c94a0 100644
--- a/cmd/govulncheck/testdata/nogomod.ct
+++ b/cmd/govulncheck/testdata/nogomod.ct
@@ -1,10 +1,6 @@
# Test of missing go.mod error message.
$ govulncheck -C ${moddir}/nogomod . --> FAIL 1
-govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.
-
-Using go1.18 and govul...@v0.0.0-00000000000-20000101010101 with
-vulnerability data from testdata/vulndb-v1 (last modified 01 Jan 21 00:00 UTC).
govulncheck: no go.mod file
govulncheck only works Go with modules. Try navigating to your module directory.
diff --git a/internal/scan/errors.go b/internal/scan/errors.go
index 99b8d40..e912c79 100644
--- a/internal/scan/errors.go
+++ b/internal/scan/errors.go
@@ -7,16 +7,23 @@
import (
"errors"
"fmt"
- "os"
"strings"
"golang.org/x/tools/go/packages"
)
var (
- ErrMissingArgPatterns = errors.New("missing any pattern args")
- ErrInvalidArg = errors.New("invalid arg")
+ // ErrVulnerabilitiesFound indicates that vulnerabilities were detected
+ // when running govulncheck. This returns exit status 3 when running
+ // without the -json flag.
ErrVulnerabilitiesFound = errors.New("vulnerabilities found")
+
+ // ErrNoPatterns indicates that no patterns were passed in when running
+ // govulncheck.
+ //
+ // In this case, we assume that the user does not know how to run
+ // govulncheck, and print the usage message with exit status 1.
+ ErrNoPatterns = errors.New("no patterns provided")
)
//lint:file-ignore ST1005 Ignore staticcheck message about error formatting
@@ -27,7 +34,7 @@
used to build govulncheck and the Go version on PATH. Consider rebuilding
govulncheck with the current Go version.`)
- // errNoGoSum indicates that a go.mod file was not found in this module.
+ // errNoGoMod indicates that a go.mod file was not found in this module.
errNoGoMod = errors.New(`no go.mod file
govulncheck only works Go with modules. Try navigating to your module directory.
@@ -50,20 +57,6 @@
return b.String()
}
-// fileExists checks if file path exists. Returns true
-// if the file exists or it cannot prove that it does
-// not exist. Otherwise, returns false.
-func fileExists(path string) bool {
- if _, err := os.Stat(path); err == nil {
- return true
- } else if errors.Is(err, os.ErrNotExist) {
- return false
- }
- // Conservatively return true if os.Stat fails
- // for some other reason.
- return true
-}
-
// isGoVersionMismatchError checks if err is due to mismatch between
// the Go version used to build govulncheck and the one currently
// on PATH.
diff --git a/internal/scan/flags.go b/internal/scan/flags.go
index b536784..ab24e82 100644
--- a/internal/scan/flags.go
+++ b/internal/scan/flags.go
@@ -5,9 +5,11 @@
package scan
import (
+ "errors"
"flag"
"fmt"
"os"
+ "path/filepath"
"golang.org/x/tools/go/buildutil"
"golang.org/x/vuln/internal/vulncheck"
@@ -30,24 +32,16 @@
modeBinary = "binary"
)
-var supportedModes = map[string]bool{
- modeSource: true,
- modeBinary: true,
-}
-
func parseFlags(args []string) (*config, error) {
cfg := &config{}
- var (
- tagsFlag buildutil.TagsFlag
- mode string
- )
+ var tagsFlag buildutil.TagsFlag
flags := flag.NewFlagSet("", flag.ContinueOnError)
flags.BoolVar(&cfg.json, "json", false, "output JSON")
flags.BoolVar(&cfg.verbose, "v", false, "print a full call stack for each vulnerability")
flags.BoolVar(&cfg.test, "test", false, "analyze test files (only valid for source mode)")
flags.StringVar(&cfg.dir, "C", "", "change to dir before running govulncheck")
flags.StringVar(&cfg.db, "db", "https://vuln.go.dev", "vulnerability database URL")
- flags.StringVar(&mode, "mode", modeSource, "supports source or binary")
+ flags.StringVar(&cfg.mode, "mode", modeSource, "supports source or binary")
flags.Var(&tagsFlag, "tags", "comma-separated `list` of build tags")
flags.Usage = func() {
fmt.Fprint(flags.Output(), `Govulncheck is a tool for finding known vulnerabilities.
@@ -67,25 +61,47 @@
cfg.patterns = flags.Args()
if len(cfg.patterns) == 0 {
flags.Usage()
- return nil, ErrMissingArgPatterns
- }
- if _, ok := supportedModes[mode]; !ok {
- return nil, ErrInvalidArg
- }
- cfg.mode = mode
-
- if cfg.mode == modeBinary {
- if len(cfg.patterns) != 1 {
- return nil, ErrInvalidArg
- }
- if !isFile(cfg.patterns[0]) {
- return nil, fmt.Errorf("%q is not a file", cfg.patterns[0])
- }
+ return nil, ErrNoPatterns
}
cfg.tags = tagsFlag
+ if err := validateConfig(cfg); err != nil {
+ return nil, fmt.Errorf("govulncheck: %w", err)
+ }
return cfg, nil
}
+var supportedModes = map[string]bool{
+ modeSource: true,
+ modeBinary: true,
+}
+
+func validateConfig(cfg *config) error {
+ if _, ok := supportedModes[cfg.mode]; !ok {
+ return fmt.Errorf("%q is not a valid mode", cfg.mode)
+ }
+
+ switch cfg.mode {
+ case modeSource:
+ if !fileExists(filepath.Join(cfg.dir, "go.mod")) {
+ return errNoGoMod
+ }
+ case modeBinary:
+ if cfg.test {
+ return fmt.Errorf("the -test flag is not supported in binary mode")
+ }
+ if len(cfg.tags) > 0 {
+ return fmt.Errorf("the -tags flag is not supported in binary mode")
+ }
+ if len(cfg.patterns) != 1 {
+ return fmt.Errorf("only 1 binary can be analyzed at a time")
+ }
+ if !isFile(cfg.patterns[0]) {
+ return fmt.Errorf("%q is not a file", cfg.patterns[0])
+ }
+ }
+ return nil
+}
+
func isFile(path string) bool {
s, err := os.Stat(path)
if err != nil {
@@ -93,3 +109,17 @@
}
return !s.IsDir()
}
+
+// fileExists checks if file path exists. Returns true
+// if the file exists or it cannot prove that it does
+// not exist. Otherwise, returns false.
+func fileExists(path string) bool {
+ if _, err := os.Stat(path); err == nil {
+ return true
+ } else if errors.Is(err, os.ErrNotExist) {
+ return false
+ }
+ // Conservatively return true if os.Stat fails
+ // for some other reason.
+ return true
+}
diff --git a/internal/scan/source.go b/internal/scan/source.go
index 2447d02..2c7b488 100644
--- a/internal/scan/source.go
+++ b/internal/scan/source.go
@@ -9,7 +9,6 @@
"fmt"
"go/ast"
"go/token"
- "path/filepath"
"strconv"
"strings"
@@ -27,10 +26,6 @@
var pkgs []*vulncheck.Package
pkgs, err := loadPackages(cfg, dir)
if err != nil {
- // Try to provide a meaningful and actionable error message.
- if !fileExists(filepath.Join(dir, "go.mod")) {
- return nil, fmt.Errorf("govulncheck: %v", errNoGoMod)
- }
if isGoVersionMismatchError(err) {
return nil, fmt.Errorf("govulncheck: %v\n\n%v", errGoVersionMismatch, err)
}
To view, visit change 482345. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages