Skip to content

Commit

Permalink
Simplify return codes to return 1 if any vulnerability related error (#…
Browse files Browse the repository at this point in the history
…677)

Fixes #676 

> - Return 0 if there are no findings or errors.
> - Return 1 if there are any findings (license or vulns).
> - Return 128 if no packages are found.
  • Loading branch information
another-rex authored Nov 27, 2023
1 parent 8c5634d commit aa3ca89
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 62 deletions.
13 changes: 1 addition & 12 deletions cmd/osv-reporter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,8 @@ func run(args []string, stdout, stderr io.Writer) int {

// if vulnerability exists it should return error
if len(diffVulns.Results) > 0 {
// If any vulnerabilities are called, then we return VulnerabilitiesFoundErr
for _, vf := range diffVulns.Flatten() {
if vf.GroupInfo.IsCalled() {
return osvscanner.VulnerabilitiesFoundErr
}
}
// Otherwise return OnlyUncalledVulnerabilitiesFoundErr
return osvscanner.OnlyUncalledVulnerabilitiesFoundErr
return osvscanner.VulnerabilitiesFoundErr
}

return nil
Expand All @@ -184,11 +178,6 @@ func run(args []string, stdout, stderr io.Writer) int {
return 1
}

if errors.Is(err, osvscanner.OnlyUncalledVulnerabilitiesFoundErr) {
// TODO: Discuss whether to have a different exit code now that running call analysis is not default
return 2
}

if errors.Is(err, osvscanner.NoPackagesFoundErr) {
tableReporter.PrintError("No package sources found, --help for usage information.\n")
return 128
Expand Down
22 changes: 3 additions & 19 deletions cmd/osv-scanner/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,10 @@ func run(args []string, stdout, stderr io.Writer) int {
},
}, r)

issueResultErr := errors.Join(
osvscanner.VulnerabilitiesFoundErr,
osvscanner.OnlyUncalledVulnerabilitiesFoundErr,
osvscanner.LicenseViolationsErr,
osvscanner.VulnerabilitiesFoundAndLicenseViolationsErr,
osvscanner.OnlyUncalledVulnerabilitiesFoundAndLicenseViolationsErr,
)
if err != nil && !errors.Is(issueResultErr, err) {
if err != nil && !errors.Is(err, osvscanner.VulnerabilitiesFoundErr) {
return err
}

if errPrint := r.PrintResult(&vulnResult); errPrint != nil {
return fmt.Errorf("failed to write output: %w", errPrint)
}
Expand All @@ -204,17 +198,7 @@ func run(args []string, stdout, stderr io.Writer) int {
}
switch {
case errors.Is(err, osvscanner.VulnerabilitiesFoundErr):
return 0b0001 // 1
case errors.Is(err, osvscanner.OnlyUncalledVulnerabilitiesFoundErr):
// TODO: Discuss whether to have a different exit code
// now that running call analysis is not default.
return 0b0010 // 2
case errors.Is(err, osvscanner.LicenseViolationsErr):
return 0b0100 // 4
case errors.Is(err, osvscanner.VulnerabilitiesFoundAndLicenseViolationsErr):
return 0b0101 // 5
case errors.Is(err, osvscanner.OnlyUncalledVulnerabilitiesFoundAndLicenseViolationsErr):
return 0b0110 // 6
return 1
case errors.Is(err, osvscanner.NoPackagesFoundErr):
r.PrintError("No package sources found, --help for usage information.\n")
return 128
Expand Down
12 changes: 6 additions & 6 deletions cmd/osv-scanner/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,7 @@ func TestRun_Licenses(t *testing.T) {
{
name: "No vulnerabilities but contains license violations",
args: []string{"", "--experimental-licenses", "", "./fixtures/locks-many"},
wantExitCode: 4,
wantExitCode: 1,
wantStdout: `
Scanning dir ./fixtures/locks-many
Scanned <rootdir>/fixtures/locks-many/Gemfile.lock file and found 1 package
Expand All @@ -1171,7 +1171,7 @@ func TestRun_Licenses(t *testing.T) {
{
name: "No vulnerabilities but contains license violations markdown",
args: []string{"", "--experimental-licenses", "", "--format=markdown", "./fixtures/locks-many"},
wantExitCode: 4,
wantExitCode: 1,
wantStdout: `Scanning dir ./fixtures/locks-many
Scanned <rootdir>/fixtures/locks-many/Gemfile.lock file and found 1 package
Scanned <rootdir>/fixtures/locks-many/alpine.cdx.xml as CycloneDX SBOM and found 15 packages
Expand All @@ -1193,7 +1193,7 @@ Filtered 2 vulnerabilities from output
{
name: "Vulnerabilities and license violations",
args: []string{"", "--experimental-licenses", "", "--config=./fixtures/osv-scanner-empty-config.toml", "./fixtures/locks-many/package-lock.json"},
wantExitCode: 5,
wantExitCode: 1,
wantStdout: `
Scanning dir ./fixtures/locks-many/package-lock.json
Scanned <rootdir>/fixtures/locks-many/package-lock.json file and found 1 package
Expand All @@ -1213,7 +1213,7 @@ Filtered 2 vulnerabilities from output
{
name: "Vulnerabilities and license violations with allowlist",
args: []string{"", "--experimental-licenses", "MIT", "--config=./fixtures/osv-scanner-empty-config.toml", "./fixtures/locks-many/package-lock.json"},
wantExitCode: 5,
wantExitCode: 1,
wantStdout: `
Scanning dir ./fixtures/locks-many/package-lock.json
Scanned <rootdir>/fixtures/locks-many/package-lock.json file and found 1 package
Expand Down Expand Up @@ -1248,7 +1248,7 @@ Filtered 2 vulnerabilities from output
{
name: "Some packages with license violations and show-all-packages in json",
args: []string{"", "--format=json", "--experimental-licenses", "MIT", "--experimental-all-packages", "./fixtures/locks-licenses/package-lock.json"},
wantExitCode: 4,
wantExitCode: 1,
wantStdout: `
{
"results": [
Expand Down Expand Up @@ -1318,7 +1318,7 @@ Filtered 2 vulnerabilities from output
{
name: "Some packages with license violations in json",
args: []string{"", "--format=json", "--experimental-licenses", "MIT", "./fixtures/locks-licenses/package-lock.json"},
wantExitCode: 4,
wantExitCode: 1,
wantStdout: `
{
"results": [
Expand Down
32 changes: 7 additions & 25 deletions pkg/osvscanner/osvscanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,21 +59,17 @@ type ExperimentalScannerActions struct {
//nolint:errname,stylecheck // Would require version major bump to change
var NoPackagesFoundErr = errors.New("no packages found in scan")

// VulnerabilitiesFoundErr includes both vulnerabilities being found or license violations being found,
// however, will not be raised if only uncalled vulnerabilities are found.
//
//nolint:errname,stylecheck // Would require version major bump to change
var VulnerabilitiesFoundErr = errors.New("vulnerabilities found")

// Deprecated: This error is no longer returned, check the results to determine if this is the case
//
//nolint:errname,stylecheck // Would require version bump to change
var OnlyUncalledVulnerabilitiesFoundErr = errors.New("only uncalled vulnerabilities found")

//nolint:errname,stylecheck // Would require version bump to change
var LicenseViolationsErr = errors.New("license violations found")

//nolint:errname,stylecheck // Would require version bump to change
var VulnerabilitiesFoundAndLicenseViolationsErr = errors.New("vulnerabilities found and license violations found")

//nolint:errname,stylecheck // Would require version bump to change
var OnlyUncalledVulnerabilitiesFoundAndLicenseViolationsErr = errors.New("only uncalled vulnerabilities found and license violations found")

var (
vendoredLibNames = map[string]struct{}{
"3rdparty": {},
Expand Down Expand Up @@ -827,25 +823,11 @@ func DoScan(actions ScannerActions, r reporter.Reporter) (models.VulnerabilityRe
}
onlyUncalledVuln = onlyUncalledVuln && vuln

switch {
case !vuln && !onlyUncalledVuln && !licenseViolation:
if (!vuln || onlyUncalledVuln) && !licenseViolation {
// There is no error.
return results, nil
case vuln && !onlyUncalledVuln && !licenseViolation:
} else {
return results, VulnerabilitiesFoundErr
case !vuln && onlyUncalledVuln && !licenseViolation:
// Impossible state.
panic("internal error: uncalled vulnerabilities exist but no vulnerabilities exist")
case vuln && onlyUncalledVuln && !licenseViolation:
return results, OnlyUncalledVulnerabilitiesFoundErr
case !vuln && !onlyUncalledVuln && licenseViolation:
return results, LicenseViolationsErr
case vuln && !onlyUncalledVuln && licenseViolation:
return results, VulnerabilitiesFoundAndLicenseViolationsErr
case !vuln && onlyUncalledVuln && licenseViolation:
panic("internal error: uncalled vulnerabilities exist but no vulnerabilities exist")
case vuln && onlyUncalledVuln && licenseViolation:
return results, OnlyUncalledVulnerabilitiesFoundAndLicenseViolationsErr
}
}

Expand Down

0 comments on commit aa3ca89

Please sign in to comment.