From 2a61873563f5eb373bdb60c5907b9a86919d3ef7 Mon Sep 17 00:00:00 2001 From: Rex P <106129829+another-rex@users.noreply.github.com> Date: Fri, 8 Sep 2023 15:14:04 +1000 Subject: [PATCH] Fix result scanning (#526) Fix issues with scanning OSV-Scanner Results. - Adds support for adding commit hashes to the OSV-Scanner results, - Allow actually scanning OSV-Scanner results from the cli. --- internal/local/check.go | 13 ++++++++++++ internal/output/table.go | 4 ++-- .../osvscannerresults/one-package-commit.json | 19 +++++++++++++++++ pkg/lockfile/osv-vuln-result_test.go | 16 ++++++++++++++ pkg/lockfile/osv-vuln-results.go | 18 ++++++++++------ pkg/models/results.go | 1 + pkg/osv/osv.go | 21 ++++++++++++------- pkg/osvscanner/osvscanner.go | 2 ++ pkg/osvscanner/vulnerability_result.go | 3 +-- 9 files changed, 79 insertions(+), 18 deletions(-) create mode 100644 pkg/lockfile/fixtures/osvscannerresults/one-package-commit.json diff --git a/internal/local/check.go b/internal/local/check.go index 26dc0c6edf..9d655fb085 100644 --- a/internal/local/check.go +++ b/internal/local/check.go @@ -126,6 +126,19 @@ func MakeRequest(r reporter.Reporter, query osv.BatchedQuery, offline bool, loca continue } + if pkg.Ecosystem == "" { + if pkg.Commit == "" { + // The only time this can happen should be when someone passes in their own OSV-Scanner-Results file. + return nil, fmt.Errorf("ecosystem is empty and there is no commit hash") + } + + // Is a commit based query, skip local scanning + results = append(results, osv.Response{}) + r.PrintText(fmt.Sprintf("Skipping commit scanning for: %s\n", pkg.Commit)) + + continue + } + db, err := loadDBFromCache(pkg.Ecosystem) if err != nil { diff --git a/internal/output/table.go b/internal/output/table.go index 69030bc54b..3eb75230b1 100644 --- a/internal/output/table.go +++ b/internal/output/table.go @@ -108,8 +108,8 @@ func tableBuilderInner(vulnResult *models.VulnerabilityResults, addStyling bool, outputRow = append(outputRow, strings.Join(links, "\n")) outputRow = append(outputRow, MaxSeverity(group, pkg)) - if pkg.Package.Ecosystem == "GIT" { - outputRow = append(outputRow, "GIT", pkg.Package.Version, pkg.Package.Version) + if pkg.Package.Ecosystem == "" && pkg.Package.Commit != "" { + outputRow = append(outputRow, "GIT", pkg.Package.Commit, pkg.Package.Commit) shouldMerge = true } else { outputRow = append(outputRow, pkg.Package.Ecosystem, pkg.Package.Name, pkg.Package.Version) diff --git a/pkg/lockfile/fixtures/osvscannerresults/one-package-commit.json b/pkg/lockfile/fixtures/osvscannerresults/one-package-commit.json new file mode 100644 index 0000000000..044efa3e48 --- /dev/null +++ b/pkg/lockfile/fixtures/osvscannerresults/one-package-commit.json @@ -0,0 +1,19 @@ +{ + "results": [ + { + "source": { + "path": "/path/to/Gemfile.lock", + "type": "lockfile" + }, + "packages": [ + { + "package": { + "commit": "9a6bd55c9d0722cb101fe85a3b22d89e4ff4fe52" + }, + "vulnerabilities": [], + "groups": [] + } + ] + } + ] +} \ No newline at end of file diff --git a/pkg/lockfile/osv-vuln-result_test.go b/pkg/lockfile/osv-vuln-result_test.go index 1317af8f7f..773e6e9266 100644 --- a/pkg/lockfile/osv-vuln-result_test.go +++ b/pkg/lockfile/osv-vuln-result_test.go @@ -55,6 +55,22 @@ func TestParseOSVScannerResults_OnePackage(t *testing.T) { }) } +func TestParseOSVScannerResults_OnePackageCommit(t *testing.T) { + t.Parallel() + + packages, err := lockfile.ParseOSVScannerResults("fixtures/osvscannerresults/one-package-commit.json") + + if err != nil { + t.Errorf("Got unexpected error: %v", err) + } + + expectPackages(t, packages, []lockfile.PackageDetails{ + { + Commit: "9a6bd55c9d0722cb101fe85a3b22d89e4ff4fe52", + }, + }) +} + func TestParseOSVScannerResults_MultiPackages(t *testing.T) { t.Parallel() diff --git a/pkg/lockfile/osv-vuln-results.go b/pkg/lockfile/osv-vuln-results.go index 8931fab884..6e2e202f28 100644 --- a/pkg/lockfile/osv-vuln-results.go +++ b/pkg/lockfile/osv-vuln-results.go @@ -29,12 +29,18 @@ func (e OSVScannerResultsExtractor) Extract(f DepFile) ([]PackageDetails, error) packages := []PackageDetails{} for _, res := range parsedResults.Results { for _, pkg := range res.Packages { - packages = append(packages, PackageDetails{ - Name: pkg.Package.Name, - Ecosystem: Ecosystem(pkg.Package.Ecosystem), - Version: pkg.Package.Version, - CompareAs: Ecosystem(pkg.Package.Ecosystem), - }) + if pkg.Package.Commit != "" { // Prioritize results + packages = append(packages, PackageDetails{ + Commit: pkg.Package.Commit, + }) + } else { + packages = append(packages, PackageDetails{ + Name: pkg.Package.Name, + Ecosystem: Ecosystem(pkg.Package.Ecosystem), + Version: pkg.Package.Version, + CompareAs: Ecosystem(pkg.Package.Ecosystem), + }) + } } } diff --git a/pkg/models/results.go b/pkg/models/results.go index 4bc7d763cb..b649d4c65f 100644 --- a/pkg/models/results.go +++ b/pkg/models/results.go @@ -121,4 +121,5 @@ type PackageInfo struct { Name string `json:"name"` Version string `json:"version"` Ecosystem string `json:"ecosystem"` + Commit string `json:"commit"` } diff --git a/pkg/osv/osv.go b/pkg/osv/osv.go index 8e7e503f74..6cb95afdc1 100644 --- a/pkg/osv/osv.go +++ b/pkg/osv/osv.go @@ -92,14 +92,19 @@ func MakePURLRequest(purl string) *Query { } func MakePkgRequest(pkgDetails lockfile.PackageDetails) *Query { - return &Query{ - Version: pkgDetails.Version, - // API has trouble parsing requests with both commit and Package details filled ins - // Commit: pkgDetails.Commit, - Package: Package{ - Name: pkgDetails.Name, - Ecosystem: string(pkgDetails.Ecosystem), - }, + // API has trouble parsing requests with both commit and Package details filled in + if pkgDetails.Ecosystem == "" && pkgDetails.Commit != "" { + return &Query{ + Commit: pkgDetails.Commit, + } + } else { + return &Query{ + Version: pkgDetails.Version, + Package: Package{ + Name: pkgDetails.Name, + Ecosystem: string(pkgDetails.Ecosystem), + }, + } } } diff --git a/pkg/osvscanner/osvscanner.go b/pkg/osvscanner/osvscanner.go index 5aabd963dc..1351ea7385 100644 --- a/pkg/osvscanner/osvscanner.go +++ b/pkg/osvscanner/osvscanner.go @@ -205,6 +205,8 @@ func scanLockfile(r reporter.Reporter, query *osv.BatchedQuery, path string, par parsedLockfile, err = lockfile.FromApkInstalled(path) case "dpkg-status": parsedLockfile, err = lockfile.FromDpkgStatus(path) + case "osv-scanner-results": + parsedLockfile, err = lockfile.FromOSVScannerResults(path) default: parsedLockfile, err = lockfile.ExtractDeps(f, parseAs) } diff --git a/pkg/osvscanner/vulnerability_result.go b/pkg/osvscanner/vulnerability_result.go index 075b98b9a8..7ad57fc46a 100644 --- a/pkg/osvscanner/vulnerability_result.go +++ b/pkg/osvscanner/vulnerability_result.go @@ -26,8 +26,7 @@ func groupResponseBySource(r reporter.Reporter, query osv.BatchedQuery, resp *os } var pkg models.PackageVulns if query.Commit != "" { - pkg.Package.Version = query.Commit - pkg.Package.Ecosystem = "GIT" + pkg.Package.Commit = query.Commit } else if query.Package.PURL != "" { var err error pkg.Package, err = models.PURLToPackage(query.Package.PURL)