Add exit codes and resolution strictness options to CLI scan command (#332)

* Add exit codes and resolution strictness options to CLI scan command

Signed-off-by: bpatra3 <bpatra3@opentext.com>

* Implement proper error handling and exit codes for scan resolution failures

Signed-off-by: bpatra3 <bpatra3@opentext.com>

---------

Signed-off-by: bpatra3 <bpatra3@opentext.com>

Author: biswajit-patra-dev

README.md

@@ -74,6 +74,29 @@ Once you've installed the CLI, you're ready to scan your project. You can scan a
 
 When the scan is complete, you will see the total number of vulnerabilities found and a list of automation rules that have been evaluated. Read more about automations [here](https://debricked.com/docs/automation/automation-overview.html#automation-overview).
 
+### Exit codes
+| Code | Meaning |
+| ---- | ------- |
+| 0    | The scan completed and no triggered automation rule failed the pipeline |
+| 1    | The scan failed. This covers triggered automation rules configured to fail the pipeline, resolution failures (see below), and errors such as a bad access token or an unreachable service |
+| 3    | The scan completed, but some (not all) dependency files failed to resolve. Only produced by `--resolution-strictness=3` |
+
+Failed resolution of dependency files affects the scan and its exit code according to
+`--resolution-strictness` (default `1`):
+
+| Level | Meaning |
+| ----- | ------- |
+| 0     | Always continue the scan, even if any or all files failed to resolve |
+| 1     | Exit with code 1 if all files failed to resolve, otherwise continue the scan |
+| 2     | Exit with code 1 if any file failed to resolve, otherwise continue the scan |
+| 3     | Exit with code 1 if all files failed to resolve. If some but not all files failed to resolve, complete the scan and then exit with code 3 |
+
+A resolution failure typically means the relevant package manager is not installed or not on the
+`PATH` (for example `mvn` or `composer`).
+
+If Debricked's scan queue is long, the CLI stops polling for progress and exits with code 1, having
+printed a link to the results. Pass `--pass-on-timeout` to exit 0 in that case instead.
+
 ### Docker
 To make a scan directly through Docker based on your current working directory, you can use the following command:
 ```sh

internal/cmd/scan/scan.go

@@ -8,7 +8,9 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/debricked/cli/internal/cmd/cmderror"
 	"github.com/debricked/cli/internal/file"
+	"github.com/debricked/cli/internal/resolution"
 	"github.com/debricked/cli/internal/scan"
 	"github.com/fatih/color"
 	"github.com/spf13/cobra"
@@ -36,6 +38,7 @@ var passOnDowntime bool
 var regenerate int
 var repositoryName string
 var repositoryUrl string
+var resolutionStrictness int
 var verbose bool
 var versionHint bool
 var sbom string
@@ -72,6 +75,7 @@ const (
 	TagCommitAsReleaseEnv           = "TAG_COMMIT_AS_RELEASE"
 	ExperimentalFlag                = "experimental"
 	GenerateCommitNameFlag          = "generate-commit-name"
+	ResolutionStrictnessFlag        = "resolution-strictness"
 )
 
 var scanCmdError error
@@ -162,9 +166,21 @@ $ debricked scan . --inclusion '**/node_modules/**'`)
 		}, "\n")
 	cmd.Flags().BoolVar(&verbose, VerboseFlag, true, verboseDoc)
 	cmd.Flags().BoolVar(&debug, DebugFlag, false, "write all debug output to stderr")
-	cmd.Flags().BoolVarP(&passOnDowntime, PassOnTimeOut, "p", false, "pass scan if there is a service access timeout")
+	cmd.Flags().BoolVarP(&passOnDowntime, PassOnTimeOut, "p", false, "pass scan if there is a service access timeout, or if the scan is still queued once progress polling gives up")
 	cmd.Flags().BoolVar(&noResolve, NoResolveFlag, false, `disables resolution of manifest files that lack lock files. Resolving manifest files enables more accurate dependency scanning since the whole dependency tree will be analysed.
 For example, if there is a "go.mod" in the target path, its dependencies are going to get resolved onto a lock file, and latter scanned.`)
+	resolutionStrictnessDoc := strings.Join(
+		[]string{
+			"Allows you to configure how failed resolution of manifest files affects the scan and its exit code.\n",
+			"Strictness Level | Meaning",
+			"---------------- | -------",
+			"0                | Always continue the scan, even if any or all files failed to resolve",
+			"1 (default)      | Exit with code 1 if all files failed to resolve, otherwise continue the scan",
+			"2                | Exit with code 1 if any file failed to resolve, otherwise continue the scan",
+			"3                | Exit with code 1 if all files failed to resolve. If some but not all files failed to resolve, complete the scan and then exit with code 3",
+			"\nExample:\n$ debricked scan . --resolution-strictness=3",
+		}, "\n")
+	cmd.Flags().IntVar(&resolutionStrictness, ResolutionStrictnessFlag, int(resolution.FailIfAllFail), resolutionStrictnessDoc)
 	cmd.Flags().BoolVar(&noFingerprint, NoFingerprintFlag, false, "Toggle fingerprinting for undeclared component identification. Can be run as a standalone command [fingerprint] with more granular options.")
 	cmd.Flags().BoolVar(&callgraph, CallGraphFlag, false, `Enables call graph generation during scan.`)
 	cmd.Flags().StringVar(&javaCallgraphEngine, JavaCallgraphEngineFlag, "soot", "Java call graph engine to use during scan callgraph generation: soot or sootup.")
@@ -232,6 +248,11 @@ func RunE(s *scan.IScanner) func(_ *cobra.Command, args []string) error {
 			tagCommitAsRelease = viper.GetBool(TagCommitAsReleaseFlag)
 		}
 
+		strictness, err := resolution.GetStrictnessLevel(viper.GetInt(ResolutionStrictnessFlag))
+		if err != nil {
+			return err
+		}
+
 		options := scan.DebrickedOptions{
 			Path:                        path,
 			Resolve:                     !viper.GetBool(NoResolveFlag),
@@ -261,14 +282,16 @@ func RunE(s *scan.IScanner) func(_ *cobra.Command, args []string) error {
 			MinFingerprintContentLength: viper.GetInt(MinFingerprintContentLengthFlag),
 			TagCommitAsRelease:          tagCommitAsRelease,
 			Experimental:                viper.GetBool(ExperimentalFlag),
+			ResolutionStrictness:        strictness,
 		}
 		if s != nil {
 			scanCmdError = (*s).Scan(options)
 		} else {
 			scanCmdError = errors.New("scanner was nil")
 		}
 
-		if scanCmdError == scan.FailPipelineErr {
+		var cmdErr cmderror.CommandError
+		if scanCmdError == scan.FailPipelineErr || scanCmdError == scan.LongQueueErr || errors.As(scanCmdError, &cmdErr) {
 			cmd.SilenceUsage = true
 			cmd.SilenceErrors = true
 

internal/cmd/scan/scan_test.go

@@ -1,8 +1,11 @@
 package scan
 
 import (
+	"errors"
 	"testing"
 
+	"github.com/debricked/cli/internal/cmd/cmderror"
+	"github.com/debricked/cli/internal/resolution"
 	"github.com/debricked/cli/internal/scan"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -26,6 +29,7 @@ func TestNewScanCmd(t *testing.T) {
 		JavaCallgraphEngineFlag:      "",
 		CallGraphUploadTimeoutFlag:   "",
 		CallGraphGenerateTimeoutFlag: "",
+		ResolutionStrictnessFlag:     "",
 	}
 	flags := cmd.Flags()
 	for name, shorthand := range flagAssertions {
@@ -83,11 +87,124 @@ func TestRunEFailPipelineErr(t *testing.T) {
 
 	err := runE(cmd, nil)
 
-	assert.Error(t, err, scan.FailPipelineErr)
+	assert.ErrorIs(t, err, scan.FailPipelineErr)
 	assert.True(t, cmd.SilenceUsage, "failed to assert that usage was silenced")
 	assert.True(t, cmd.SilenceErrors, "failed to assert that errors were silenced")
 }
 
+func TestRunELongQueueErr(t *testing.T) {
+	var s scan.IScanner
+	mock := &scannerMock{}
+	mock.setErr(scan.LongQueueErr)
+	s = mock
+	runE := RunE(&s)
+	cmd := &cobra.Command{}
+
+	err := runE(cmd, nil)
+
+	assert.ErrorIs(t, err, scan.LongQueueErr)
+	assert.True(t, cmd.SilenceUsage, "failed to assert that usage was silenced")
+	assert.True(t, cmd.SilenceErrors, "failed to assert that errors were silenced")
+}
+
+func TestRunECommandError(t *testing.T) {
+	var s scan.IScanner
+	mock := &scannerMock{}
+	cmdErr := cmderror.CommandError{Code: 3, Err: errors.New("partial resolution failure")}
+	mock.setErr(cmdErr)
+	s = mock
+	runE := RunE(&s)
+	cmd := &cobra.Command{}
+
+	err := runE(cmd, nil)
+
+	var gotCmdErr cmderror.CommandError
+	assert.True(t, errors.As(err, &gotCmdErr), "expected CommandError to be preserved")
+	assert.Equal(t, 3, gotCmdErr.Code, "expected exit code 3 to be preserved")
+	assert.True(t, cmd.SilenceUsage, "failed to assert that usage was silenced")
+	assert.True(t, cmd.SilenceErrors, "failed to assert that errors were silenced")
+}
+
+func TestRunEResolutionStrictness(t *testing.T) {
+	cases := []struct {
+		name     string
+		flag     interface{}
+		expected resolution.StrictnessLevel
+	}{
+		{name: "default", flag: nil, expected: resolution.FailIfAllFail},
+		{name: "no fail", flag: 0, expected: resolution.NoFail},
+		{name: "fail if all fail", flag: 1, expected: resolution.FailIfAllFail},
+		{name: "fail if any fail", flag: 2, expected: resolution.FailIfAnyFail},
+		{name: "fail or warn", flag: 3, expected: resolution.FailOrWarn},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			viper.Reset()
+			if c.flag != nil {
+				viper.Set(ResolutionStrictnessFlag, c.flag)
+			} else {
+				// Mirror the flag default that PreRun would have bound.
+				viper.SetDefault(ResolutionStrictnessFlag, int(resolution.FailIfAllFail))
+			}
+			defer viper.Reset()
+
+			var s scan.IScanner
+			mock := &scannerMock{}
+			s = mock
+			runE := RunE(&s)
+
+			err := runE(&cobra.Command{}, nil)
+
+			assert.NoError(t, err)
+			options, ok := mock.options.(scan.DebrickedOptions)
+			assert.True(t, ok, "failed to assert that scan options were passed")
+			assert.Equal(t, c.expected, options.ResolutionStrictness)
+		})
+	}
+}
+
+// "debricked files find" binds DEBRICKED_STRICT to the global viper key
+// "strict". The scan flag must not share that key, or setting the env var for
+// one command would silently change resolution behaviour in the other.
+func TestRunEStrictEnvDoesNotAffectResolutionStrictness(t *testing.T) {
+	viper.Reset()
+	viper.SetEnvPrefix("DEBRICKED")
+	viper.AutomaticEnv()
+	viper.MustBindEnv("strict")
+	viper.SetDefault(ResolutionStrictnessFlag, int(resolution.FailIfAllFail))
+	t.Setenv("DEBRICKED_STRICT", "3")
+	defer viper.Reset()
+
+	var s scan.IScanner
+	mock := &scannerMock{}
+	s = mock
+	runE := RunE(&s)
+
+	err := runE(&cobra.Command{}, nil)
+
+	assert.NoError(t, err)
+	options, ok := mock.options.(scan.DebrickedOptions)
+	assert.True(t, ok, "failed to assert that scan options were passed")
+	assert.Equal(t, resolution.FailIfAllFail, options.ResolutionStrictness)
+}
+
+func TestRunEInvalidResolutionStrictness(t *testing.T) {
+	viper.Reset()
+	viper.Set(ResolutionStrictnessFlag, 4)
+	defer viper.Reset()
+
+	var s scan.IScanner
+	mock := &scannerMock{}
+	s = mock
+	runE := RunE(&s)
+
+	err := runE(&cobra.Command{}, nil)
+
+	assert.ErrorContains(t, err, "invalid strictness level: 4")
+	assert.Nil(t, mock.options, "failed to assert that the scan was not started")
+}
+
 func TestRunEError(t *testing.T) {
 	runE := RunE(nil)
 	err := runE(nil, []string{"."})
@@ -102,9 +219,13 @@ func TestPreRun(t *testing.T) {
 
 type scannerMock struct {
 	err error
+	// options records the options of the most recent Scan call.
+	options scan.IOptions
 }
 
-func (s *scannerMock) Scan(_ scan.IOptions) error {
+func (s *scannerMock) Scan(o scan.IOptions) error {
+	s.options = o
+
 	return s.err
 }
 

internal/resolution/resolver.go

@@ -30,6 +30,11 @@ const (
 	FailOrWarn
 )
 
+// WarnExitCode is produced by FailOrWarn when some, but not all, files failed to
+// resolve. It is the only resolution exit code that lets a scan run to
+// completion - every other non-zero code aborts it.
+const WarnExitCode = 3
+
 func GetStrictnessLevel(level int) (StrictnessLevel, error) {
 	switch level {
 	case 0:
@@ -141,7 +146,7 @@ func (r Resolver) failOrWarnLogic(errorCount, jobCount int) (int, error) {
 		return 1, nil
 	}
 
-	return 3, nil
+	return WarnExitCode, nil
 }
 
 func (r Resolver) Resolve(paths []string, options IOptions) (IResolution, error) {

internal/resolution/testdata/resolver_mock.go

@@ -9,14 +9,17 @@ import (
 )
 
 type ResolverMock struct {
-	Err   error
-	files []string
+	Err error
+	// Options records the options of the most recent Resolve call.
+	Options resolution.IOptions
+	files   []string
 }
 
 func (r *ResolverMock) SetNpmPreferred(_ bool) {
 }
 
-func (r *ResolverMock) Resolve(_ []string, _ resolution.IOptions) (resolution.IResolution, error) {
+func (r *ResolverMock) Resolve(_ []string, options resolution.IOptions) (resolution.IResolution, error) {
+	r.Options = options
 	for _, f := range r.files {
 		createdFile, err := os.Create(f)
 		if err != nil {