From 4a849b89d99ffc9631f4e4c9017704b0de352372 Mon Sep 17 00:00:00 2001 From: josieang <32358891+josieang@users.noreply.github.com> Date: Wed, 29 Nov 2023 10:33:02 +1100 Subject: [PATCH] add experimental-licenses summary flag (#678) Addressing https://github.com/google/osv-scanner/issues/671#issuecomment-1823770821 --- cmd/osv-scanner/main.go | 32 ++++- cmd/osv-scanner/main_test.go | 89 ++++++++++--- internal/output/table.go | 11 +- pkg/models/results.go | 2 +- pkg/osvscanner/osvscanner.go | 8 +- pkg/osvscanner/vulnerability_result.go | 27 ++-- .../vulnerability_result_internal_test.go | 124 ++++++++++-------- 7 files changed, 188 insertions(+), 105 deletions(-) diff --git a/cmd/osv-scanner/main.go b/cmd/osv-scanner/main.go index 2c8748669f..e5bcfb021d 100644 --- a/cmd/osv-scanner/main.go +++ b/cmd/osv-scanner/main.go @@ -132,9 +132,13 @@ func run(args []string, stdout, stderr io.Writer) int { Name: "experimental-all-packages", Usage: "when json output is selected, prints all packages", }, + &cli.BoolFlag{ + Name: "experimental-licenses-summary", + Usage: "report a license summary, implying the --experimental-all-packages flag", + }, &cli.StringSliceFlag{ Name: "experimental-licenses", - Usage: "report on licenses", + Usage: "report on licenses based on an allowlist", }, }, ArgsUsage: "[directory1 directory2...]", @@ -163,6 +167,17 @@ func run(args []string, stdout, stderr io.Writer) int { } } + if context.Bool("experimental-licenses-summary") && context.IsSet("experimental-licenses") { + return fmt.Errorf("--experimental-licenses-summary and --experimental-licenses flags cannot be set") + } + allowlist := context.StringSlice("experimental-licenses") + if context.IsSet("experimental-licenses") && + (len(allowlist) == 0 || + (len(allowlist) == 1 && allowlist[0] == "")) { + return fmt.Errorf("--experimental-licenses requires at least one value") + } + // TODO: verify that the licenses they passed in are indeed spdx. + if r, err = reporter.New(format, stdout, stderr, termWidth); err != nil { return err } @@ -186,11 +201,16 @@ func run(args []string, stdout, stderr io.Writer) int { DirectoryPaths: context.Args().Slice(), CallAnalysisStates: callAnalysisStates, ExperimentalScannerActions: osvscanner.ExperimentalScannerActions{ - LocalDBPath: context.String("experimental-local-db-path"), - CompareLocally: context.Bool("experimental-local-db"), - CompareOffline: context.Bool("experimental-offline"), - ShowAllPackages: context.Bool("experimental-all-packages"), - ScanLicenses: context.IsSet("experimental-licenses"), + LocalDBPath: context.String("experimental-local-db-path"), + CompareLocally: context.Bool("experimental-local-db"), + CompareOffline: context.Bool("experimental-offline"), + // License summary mode causes all + // packages to appear in the json as + // every package has a license - even + // if it's just the UNKNOWN license. + ShowAllPackages: context.Bool("experimental-all-packages") || + context.Bool("experimental-licenses-summary"), + ScanLicensesSummary: context.Bool("experimental-licenses-summary"), ScanLicensesAllowlist: context.StringSlice("experimental-licenses"), }, }, r) diff --git a/cmd/osv-scanner/main_test.go b/cmd/osv-scanner/main_test.go index 88e4caa990..05afb811b7 100644 --- a/cmd/osv-scanner/main_test.go +++ b/cmd/osv-scanner/main_test.go @@ -353,7 +353,7 @@ func TestRun(t *testing.T) { "results": [], "experimental_config": { "licenses": { - "enabled": false, + "summary": false, "allowlist": null } } @@ -373,7 +373,7 @@ func TestRun(t *testing.T) { "results": [], "experimental_config": { "licenses": { - "enabled": false, + "summary": false, "allowlist": null } } @@ -1075,7 +1075,7 @@ func TestRun_LocalDatabases(t *testing.T) { "results": [], "experimental_config": { "licenses": { - "enabled": false, + "summary": false, "allowlist": null } } @@ -1096,7 +1096,7 @@ func TestRun_LocalDatabases(t *testing.T) { "results": [], "experimental_config": { "licenses": { - "enabled": false, + "summary": false, "allowlist": null } } @@ -1147,9 +1147,9 @@ func TestRun_Licenses(t *testing.T) { t.Parallel() tests := []cliTestCase{ { - name: "No vulnerabilities but contains license violations", - args: []string{"", "--experimental-licenses", "", "./fixtures/locks-many"}, - wantExitCode: 1, + name: "No vulnerabilities with license summary", + args: []string{"", "--experimental-licenses-summary", "./fixtures/locks-many"}, + wantExitCode: 0, wantStdout: ` Scanning dir ./fixtures/locks-many Scanned /fixtures/locks-many/Gemfile.lock file and found 1 package @@ -1172,9 +1172,9 @@ func TestRun_Licenses(t *testing.T) { wantStderr: "", }, { - name: "No vulnerabilities but contains license violations markdown", - args: []string{"", "--experimental-licenses", "", "--format=markdown", "./fixtures/locks-many"}, - wantExitCode: 1, + name: "No vulnerabilities with license summary in markdown", + args: []string{"", "--experimental-licenses-summary", "--format=markdown", "./fixtures/locks-many"}, + wantExitCode: 0, 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 @@ -1194,8 +1194,8 @@ Filtered 2 vulnerabilities from output wantStderr: "", }, { - name: "Vulnerabilities and license violations", - args: []string{"", "--experimental-licenses", "", "--config=./fixtures/osv-scanner-empty-config.toml", "./fixtures/locks-many/package-lock.json"}, + name: "Vulnerabilities and license summary", + args: []string{"", "--experimental-licenses-summary", "--config=./fixtures/osv-scanner-empty-config.toml", "./fixtures/locks-many/package-lock.json"}, wantExitCode: 1, wantStdout: ` Scanning dir ./fixtures/locks-many/package-lock.json @@ -1299,7 +1299,7 @@ Filtered 2 vulnerabilities from output ], "experimental_config": { "licenses": { - "enabled": true, + "summary": false, "allowlist": [ "MIT" ] @@ -1343,7 +1343,7 @@ Filtered 2 vulnerabilities from output ], "experimental_config": { "licenses": { - "enabled": true, + "summary": false, "allowlist": [ "MIT" ] @@ -1404,7 +1404,7 @@ Filtered 2 vulnerabilities from output ], "experimental_config": { "licenses": { - "enabled": true, + "summary": false, "allowlist": [ "MIT", "Apache-2.0" @@ -1418,6 +1418,65 @@ Filtered 2 vulnerabilities from output Scanned /fixtures/locks-licenses/package-lock.json file and found 3 packages `, }, + { + name: "Licenses in summary mode json", + args: []string{"", "--format=json", "--experimental-licenses-summary", "./fixtures/locks-licenses/package-lock.json"}, + wantExitCode: 0, + wantStdout: ` + { + "results": [ + { + "source": { + "path": "/fixtures/locks-licenses/package-lock.json", + "type": "lockfile" + }, + "packages": [ + { + "package": { + "name": "babel", + "version": "6.23.0", + "ecosystem": "npm" + }, + "licenses": [ + "MIT" + ] + }, + { + "package": { + "name": "human-signals", + "version": "5.0.0", + "ecosystem": "npm" + }, + "licenses": [ + "Apache-2.0" + ] + }, + { + "package": { + "name": "ms", + "version": "2.1.3", + "ecosystem": "npm" + }, + "licenses": [ + "MIT" + ] + } + ] + } + ], + "experimental_config": { + "licenses": { + "summary": true, + "allowlist": [] + } + } + } + `, + wantStderr: ` + Scanning dir ./fixtures/locks-licenses/package-lock.json + Scanned /fixtures/locks-licenses/package-lock.json file and found 3 packages + `, + }, } for _, tt := range tests { tt := tt diff --git a/internal/output/table.go b/internal/output/table.go index 3f26023304..eae4154629 100644 --- a/internal/output/table.go +++ b/internal/output/table.go @@ -173,14 +173,13 @@ func MaxSeverity(group models.GroupInfo, pkg models.PackageVulns) string { func licenseTableBuilder(outputTable table.Writer, vulnResult *models.VulnerabilityResults) table.Writer { licenseConfig := vulnResult.ExperimentalAnalysisConfig.Licenses - if !licenseConfig.Enabled { - return outputTable - } - if len(licenseConfig.Allowlist) == 0 { + if licenseConfig.Summary { return licenseSummaryTableBuilder(outputTable, vulnResult) + } else if len(licenseConfig.Allowlist) > 0 { + return licenseViolationsTableBuilder(outputTable, vulnResult) + } else { + return outputTable } - - return licenseViolationsTableBuilder(outputTable, vulnResult) } func licenseSummaryTableBuilder(outputTable table.Writer, vulnResult *models.VulnerabilityResults) table.Writer { diff --git a/pkg/models/results.go b/pkg/models/results.go index f832ebc148..4aba0d5672 100644 --- a/pkg/models/results.go +++ b/pkg/models/results.go @@ -19,7 +19,7 @@ type ExperimentalAnalysisConfig struct { } type ExperimentalLicenseConfig struct { - Enabled bool `json:"enabled"` + Summary bool `json:"summary"` Allowlist []License `json:"allowlist"` } diff --git a/pkg/osvscanner/osvscanner.go b/pkg/osvscanner/osvscanner.go index bd8bdc5ef1..03a7aeddb9 100644 --- a/pkg/osvscanner/osvscanner.go +++ b/pkg/osvscanner/osvscanner.go @@ -48,7 +48,7 @@ type ExperimentalScannerActions struct { CompareLocally bool CompareOffline bool ShowAllPackages bool - ScanLicenses bool + ScanLicensesSummary bool ScanLicensesAllowlist []string LocalDBPath string @@ -786,14 +786,13 @@ func DoScan(actions ScannerActions, r reporter.Reporter) (models.VulnerabilityRe } var licensesResp [][]models.License - if actions.ScanLicenses { + if len(actions.ScanLicensesAllowlist) > 0 || actions.ScanLicensesSummary { licensesResp, err = makeLicensesRequests(filteredScannedPackages) if err != nil { return models.VulnerabilityResults{}, err } } - - results := buildVulnerabilityResults(r, filteredScannedPackages, vulnsResp, licensesResp, actions.CallAnalysisStates, actions.ShowAllPackages, actions.ScanLicenses, actions.ScanLicensesAllowlist) + results := buildVulnerabilityResults(r, filteredScannedPackages, vulnsResp, licensesResp, actions) filtered := filterResults(r, &results, &configManager, actions.ShowAllPackages) if filtered > 0 { @@ -823,6 +822,7 @@ func DoScan(actions ScannerActions, r reporter.Reporter) (models.VulnerabilityRe } } onlyUncalledVuln = onlyUncalledVuln && vuln + licenseViolation = licenseViolation && len(actions.ScanLicensesAllowlist) > 0 if (!vuln || onlyUncalledVuln) && !licenseViolation { // There is no error. diff --git a/pkg/osvscanner/vulnerability_result.go b/pkg/osvscanner/vulnerability_result.go index 6797a9a545..2d20e4e756 100644 --- a/pkg/osvscanner/vulnerability_result.go +++ b/pkg/osvscanner/vulnerability_result.go @@ -21,20 +21,14 @@ func buildVulnerabilityResults( packages []scannedPackage, vulnsResp *osv.HydratedBatchedResponse, licensesResp [][]models.License, - callAnalysisStates map[string]bool, - allPackages bool, - licenses bool, - licensesAllowlist []string, + actions ScannerActions, ) models.VulnerabilityResults { output := models.VulnerabilityResults{ Results: []models.PackageSource{}, } groupedBySource := map[models.SourceInfo][]models.PackageVulns{} - if len(licensesAllowlist) == 1 && licensesAllowlist[0] == "" { - licensesAllowlist = []string{} - } for i, rawPkg := range packages { - includePackage := allPackages + includePackage := actions.ShowAllPackages var pkg models.PackageVulns if rawPkg.Commit != "" { pkg.Package.Commit = rawPkg.Commit @@ -62,10 +56,10 @@ func buildVulnerabilityResults( pkg.Vulnerabilities = vulnsResp.Results[i].Vulns pkg.Groups = grouper.Group(grouper.ConvertVulnerabilityToIDAliases(pkg.Vulnerabilities)) } - if licenses { + if len(actions.ScanLicensesAllowlist) > 0 { pkg.Licenses = licensesResp[i] allowlist := make(map[string]bool) - for _, license := range licensesAllowlist { + for _, license := range actions.ScanLicensesAllowlist { allowlist[strings.ToLower(license)] = true } for _, license := range pkg.Licenses { @@ -77,13 +71,16 @@ func buildVulnerabilityResults( includePackage = true } } + if actions.ScanLicensesSummary { + pkg.Licenses = licensesResp[i] + } if includePackage { groupedBySource[rawPkg.Source] = append(groupedBySource[rawPkg.Source], pkg) } } for source, packages := range groupedBySource { - sourceanalysis.Run(r, source, packages, callAnalysisStates) + sourceanalysis.Run(r, source, packages, actions.CallAnalysisStates) output.Results = append(output.Results, models.PackageSource{ Source: source, Packages: packages, @@ -98,10 +95,10 @@ func buildVulnerabilityResults( return output.Results[i].Source.Path < output.Results[j].Source.Path }) - if licenses { - output.ExperimentalAnalysisConfig.Licenses.Enabled = true - allowlist := make([]models.License, len(licensesAllowlist)) - for i, l := range licensesAllowlist { + if len(actions.ScanLicensesAllowlist) > 0 || actions.ScanLicensesSummary { + output.ExperimentalAnalysisConfig.Licenses.Summary = actions.ScanLicensesSummary + allowlist := make([]models.License, len(actions.ScanLicensesAllowlist)) + for i, l := range actions.ScanLicensesAllowlist { allowlist[i] = models.License(l) } output.ExperimentalAnalysisConfig.Licenses.Allowlist = allowlist diff --git a/pkg/osvscanner/vulnerability_result_internal_test.go b/pkg/osvscanner/vulnerability_result_internal_test.go index 9fa6fc20c5..5781f4ee2e 100644 --- a/pkg/osvscanner/vulnerability_result_internal_test.go +++ b/pkg/osvscanner/vulnerability_result_internal_test.go @@ -13,14 +13,11 @@ import ( func Test_assembleResult(t *testing.T) { t.Parallel() type args struct { - r reporter.Reporter - packages []scannedPackage - vulnsResp *osv.HydratedBatchedResponse - licensesResp [][]models.License - callAnalysisStates map[string]bool - allPackages bool - licenses bool - licensesAllowlist []string + r reporter.Reporter + packages []scannedPackage + vulnsResp *osv.HydratedBatchedResponse + licensesResp [][]models.License + actions ScannerActions } packages := []scannedPackage{ { @@ -84,14 +81,17 @@ func Test_assembleResult(t *testing.T) { }{{ name: "group vulnerabilities", args: args{ - r: &reporter.VoidReporter{}, - packages: packages, - vulnsResp: vulnsResp, - licensesResp: licensesResp, - allPackages: false, - callAnalysisStates: callAnalysisStates, - licenses: false, - licensesAllowlist: nil, + r: &reporter.VoidReporter{}, + packages: packages, + vulnsResp: vulnsResp, + licensesResp: licensesResp, + actions: ScannerActions{ + ExperimentalScannerActions: ExperimentalScannerActions{ + ShowAllPackages: false, + ScanLicensesAllowlist: nil, + }, + CallAnalysisStates: callAnalysisStates, + }, }, want: models.VulnerabilityResults{ Results: []models.PackageSource{ @@ -154,14 +154,17 @@ func Test_assembleResult(t *testing.T) { }, { name: "group vulnerabilities, with all packages included", args: args{ - r: &reporter.VoidReporter{}, - packages: packages, - vulnsResp: vulnsResp, - licensesResp: licensesResp, - allPackages: true, - callAnalysisStates: callAnalysisStates, - licenses: false, - licensesAllowlist: nil, + r: &reporter.VoidReporter{}, + packages: packages, + vulnsResp: vulnsResp, + licensesResp: licensesResp, + actions: ScannerActions{ + ExperimentalScannerActions: ExperimentalScannerActions{ + ShowAllPackages: true, + ScanLicensesAllowlist: nil, + }, + CallAnalysisStates: callAnalysisStates, + }, }, want: models.VulnerabilityResults{ Results: []models.PackageSource{ @@ -230,19 +233,23 @@ func Test_assembleResult(t *testing.T) { }, { name: "group vulnerabilities with licenses", args: args{ - r: &reporter.VoidReporter{}, - packages: packages, - vulnsResp: vulnsResp, - licensesResp: licensesResp, - allPackages: false, - callAnalysisStates: callAnalysisStates, - licenses: true, - licensesAllowlist: nil, + r: &reporter.VoidReporter{}, + packages: packages, + vulnsResp: vulnsResp, + licensesResp: licensesResp, + actions: ScannerActions{ + ExperimentalScannerActions: ExperimentalScannerActions{ + ShowAllPackages: true, + ScanLicensesSummary: true, + ScanLicensesAllowlist: nil, + }, + CallAnalysisStates: callAnalysisStates, + }, }, want: models.VulnerabilityResults{ ExperimentalAnalysisConfig: models.ExperimentalAnalysisConfig{ Licenses: models.ExperimentalLicenseConfig{ - Enabled: true, + Summary: true, Allowlist: []models.License{}, }, }, @@ -274,16 +281,14 @@ func Test_assembleResult(t *testing.T) { Aliases: []string{"CVE-123", "GHSA-123"}, }, }, - Licenses: makeLicenses([]string{"MIT", "0BSD"}), - LicenseViolations: makeLicenses([]string{"MIT", "0BSD"}), + Licenses: makeLicenses([]string{"MIT", "0BSD"}), }, { Package: models.PackageInfo{ Name: "pkg-2", Ecosystem: "npm", Version: "1.0.0", }, - Licenses: makeLicenses([]string{"MIT"}), - LicenseViolations: makeLicenses([]string{"MIT"}), + Licenses: makeLicenses([]string{"MIT"}), }, }, }, @@ -308,8 +313,7 @@ func Test_assembleResult(t *testing.T) { Aliases: []string{"GHSA-456"}, }, }, - Licenses: makeLicenses([]string{"UNKNOWN"}), - LicenseViolations: makeLicenses([]string{"UNKNOWN"}), + Licenses: makeLicenses([]string{"UNKNOWN"}), }, }, }, @@ -318,19 +322,21 @@ func Test_assembleResult(t *testing.T) { }, { name: "group vulnerabilities with license allowlist", args: args{ - r: &reporter.VoidReporter{}, - packages: packages, - vulnsResp: vulnsResp, - licensesResp: licensesResp, - allPackages: false, - callAnalysisStates: callAnalysisStates, - licenses: true, - licensesAllowlist: []string{"MIT", "0BSD"}, + r: &reporter.VoidReporter{}, + packages: packages, + vulnsResp: vulnsResp, + licensesResp: licensesResp, + actions: ScannerActions{ + ExperimentalScannerActions: ExperimentalScannerActions{ + ShowAllPackages: false, + ScanLicensesAllowlist: []string{"MIT", "0BSD"}, + }, + CallAnalysisStates: callAnalysisStates, + }, }, want: models.VulnerabilityResults{ ExperimentalAnalysisConfig: models.ExperimentalAnalysisConfig{ Licenses: models.ExperimentalLicenseConfig{ - Enabled: true, Allowlist: []models.License{models.License("MIT"), models.License("0BSD")}, }, }, @@ -397,19 +403,21 @@ func Test_assembleResult(t *testing.T) { }, { name: "group vulnerabilities, with license allowlist and all packages", args: args{ - r: &reporter.VoidReporter{}, - packages: packages, - vulnsResp: vulnsResp, - licensesResp: licensesResp, - allPackages: true, - callAnalysisStates: callAnalysisStates, - licenses: true, - licensesAllowlist: []string{"MIT", "0BSD"}, + r: &reporter.VoidReporter{}, + packages: packages, + vulnsResp: vulnsResp, + licensesResp: licensesResp, + actions: ScannerActions{ + ExperimentalScannerActions: ExperimentalScannerActions{ + ShowAllPackages: true, + ScanLicensesAllowlist: []string{"MIT", "0BSD"}, + }, + CallAnalysisStates: callAnalysisStates, + }, }, want: models.VulnerabilityResults{ ExperimentalAnalysisConfig: models.ExperimentalAnalysisConfig{ Licenses: models.ExperimentalLicenseConfig{ - Enabled: true, Allowlist: []models.License{models.License("MIT"), models.License("0BSD")}, }, }, @@ -485,7 +493,7 @@ func Test_assembleResult(t *testing.T) { tt := tt // Reinitialize for t.Parallel() t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := buildVulnerabilityResults(tt.args.r, tt.args.packages, tt.args.vulnsResp, tt.args.licensesResp, tt.args.callAnalysisStates, tt.args.allPackages, tt.args.licenses, tt.args.licensesAllowlist); !reflect.DeepEqual(got, tt.want) { + if got := buildVulnerabilityResults(tt.args.r, tt.args.packages, tt.args.vulnsResp, tt.args.licensesResp, tt.args.actions); !reflect.DeepEqual(got, tt.want) { t.Errorf("buildVulnerabilityResults() = %v,\nwant %v", got, tt.want) } })