From aa3ca8914acb6e74f1fbe893de5ed0761a428683 Mon Sep 17 00:00:00 2001 From: Rex P <106129829+another-rex@users.noreply.github.com> Date: Mon, 27 Nov 2023 15:32:29 +1100 Subject: [PATCH] Simplify return codes to return 1 if any vulnerability related error (#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. --- cmd/osv-reporter/main.go | 13 +------------ cmd/osv-scanner/main.go | 22 +++------------------- cmd/osv-scanner/main_test.go | 12 ++++++------ pkg/osvscanner/osvscanner.go | 32 +++++++------------------------- 4 files changed, 17 insertions(+), 62 deletions(-) diff --git a/cmd/osv-reporter/main.go b/cmd/osv-reporter/main.go index 3db93cca361..5c7e8b4354d 100644 --- a/cmd/osv-reporter/main.go +++ b/cmd/osv-reporter/main.go @@ -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 @@ -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 diff --git a/cmd/osv-scanner/main.go b/cmd/osv-scanner/main.go index 03f62c4cd9a..744704249a1 100644 --- a/cmd/osv-scanner/main.go +++ b/cmd/osv-scanner/main.go @@ -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) } @@ -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 diff --git a/cmd/osv-scanner/main_test.go b/cmd/osv-scanner/main_test.go index 05469c15dcf..79f1c25d77d 100644 --- a/cmd/osv-scanner/main_test.go +++ b/cmd/osv-scanner/main_test.go @@ -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 /fixtures/locks-many/Gemfile.lock file and found 1 package @@ -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 /fixtures/locks-many/Gemfile.lock file and found 1 package Scanned /fixtures/locks-many/alpine.cdx.xml as CycloneDX SBOM and found 15 packages @@ -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 /fixtures/locks-many/package-lock.json file and found 1 package @@ -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 /fixtures/locks-many/package-lock.json file and found 1 package @@ -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": [ @@ -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": [ diff --git a/pkg/osvscanner/osvscanner.go b/pkg/osvscanner/osvscanner.go index 38ed2f77149..acda8497312 100644 --- a/pkg/osvscanner/osvscanner.go +++ b/pkg/osvscanner/osvscanner.go @@ -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": {}, @@ -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 } }