Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify return codes to return 1 if any vulnerability related error #677

Merged
merged 5 commits into from
Nov 27, 2023
Merged

Simplify return codes to return 1 if any vulnerability related error #677

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7be351f
Flattened a lot of return code errors to single return code 1
another-rex Nov 27, 2023
88c0209
Fixed tests to match
another-rex Nov 27, 2023
06182b0
Fix lints and remove more places the return code should be used
another-rex Nov 27, 2023
caba069
Return 0 if there is only uncalled vulnerabilities
another-rex Nov 27, 2023
621291b
Add extra documentation to vulnerabilitiesFoundErr
another-rex Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions cmd/osv-reporter/main.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
29 changes: 4 additions & 25 deletions pkg/osvscanner/osvscanner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,11 @@ var NoPackagesFoundErr = errors.New("no packages found in scan")
//nolint:errname,stylecheck // Would require version major bump to change
var VulnerabilitiesFoundErr = errors.New("vulnerabilities found")
another-rex marked this conversation as resolved.
Show resolved Hide resolved

// 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
another-rex marked this conversation as resolved.
Show resolved Hide resolved
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 +820,11 @@ func DoScan(actions ScannerActions, r reporter.Reporter) (models.VulnerabilityRe
}
onlyUncalledVuln = onlyUncalledVuln && vuln

switch {
case !vuln && !onlyUncalledVuln && !licenseViolation:
if !vuln && !onlyUncalledVuln && !licenseViolation {
another-rex marked this conversation as resolved.
Show resolved Hide resolved
// 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