From b2e66d368b3785b607c7f78221f67b9cfd299130 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Tue, 22 Mar 2022 10:48:44 -0400 Subject: [PATCH] Improve SARIF path handling and severity (#686) --- grype/presenter/sarif/presenter.go | 31 ++- grype/presenter/sarif/presenter_test.go | 199 ++++++++++-------- .../snapshot/TestSarifPresenterDir.golden | 4 +- .../snapshot/TestSarifPresenterImage.golden | 4 +- 4 files changed, 132 insertions(+), 106 deletions(-) diff --git a/grype/presenter/sarif/presenter.go b/grype/presenter/sarif/presenter.go index a85b7f417a9..56051037f8a 100644 --- a/grype/presenter/sarif/presenter.go +++ b/grype/presenter/sarif/presenter.go @@ -3,6 +3,7 @@ package sarif import ( "fmt" "io" + "path/filepath" "strings" "github.com/owenrumney/go-sarif/sarif" @@ -152,7 +153,7 @@ func (pres *Presenter) helpText(m match.Match, link string) *sarif.MultiformatMe // packagePath attempts to get the relative path of the package to the "scan root" func (pres *Presenter) packagePath(p pkg.Package) string { if len(p.Locations) > 0 { - return locationPath(p.Locations[0]) + return pres.locationPath(p.Locations[0]) } return pres.inputPath() } @@ -169,13 +170,26 @@ func (pres *Presenter) inputPath() string { return inputPath } -// locationPath returns a path for the location -func locationPath(l source.Location) string { +// locationPath returns a path for the location, relative to the cwd +func (pres *Presenter) locationPath(l source.Location) string { path := l.RealPath if l.VirtualPath != "" { path = l.VirtualPath } - return strings.TrimPrefix(path, "./") + in := pres.inputPath() + path = strings.TrimPrefix(path, "./") + // trimmed off any ./ and accounted for dir:. for both path and input path + if pres.srcMetadata != nil { + switch pres.srcMetadata.Scheme { + case source.DirectoryScheme: + if filepath.IsAbs(path) || in == "" { + return path + } + // return a path relative to the cwd, if it's not absolute + return fmt.Sprintf("%s/%s", in, path) + } + } + return path } // locations the locations array is a single "physical" location with potentially multiple logical locations @@ -188,7 +202,7 @@ func (pres *Presenter) locations(m match.Match) []*sarif.Location { case source.ImageScheme: img := pres.srcMetadata.ImageMetadata.UserInput for _, l := range m.Package.Locations { - trimmedPath := strings.TrimPrefix(locationPath(l), "/") + trimmedPath := strings.TrimPrefix(pres.locationPath(l), "/") logicalLocations = append(logicalLocations, &sarif.LogicalLocation{ FullyQualifiedName: sp(fmt.Sprintf("%s@%s:/%s", img, l.FileSystemID, trimmedPath)), Name: sp(l.RealPath), @@ -203,13 +217,12 @@ func (pres *Presenter) locations(m match.Match) []*sarif.Location { case source.FileScheme: for _, l := range m.Package.Locations { logicalLocations = append(logicalLocations, &sarif.LogicalLocation{ - FullyQualifiedName: sp(fmt.Sprintf("%s:/%s", pres.srcMetadata.Path, locationPath(l))), + FullyQualifiedName: sp(fmt.Sprintf("%s:/%s", pres.srcMetadata.Path, pres.locationPath(l))), Name: sp(l.RealPath), }) } case source.DirectoryScheme: - // Get a friendly relative location as well as possible - physicalLocation = strings.TrimPrefix(physicalLocation, pres.inputPath()) + // DirectoryScheme is already handled, with input prepended if needed } return []*sarif.Location{ @@ -302,7 +315,7 @@ func (pres *Presenter) securitySeverityValue(m match.Match) string { // this corresponds directly to the CVSS score, so we return this if we have it score := pres.cvssScore(m.Vulnerability) if score > 0 { - return fmt.Sprintf("%f", score) + return fmt.Sprintf("%.1f", score) } severity := vulnerability.ParseSeverity(meta.Severity) switch severity { diff --git a/grype/presenter/sarif/presenter_test.go b/grype/presenter/sarif/presenter_test.go index c839319ae02..f7b33737dfc 100644 --- a/grype/presenter/sarif/presenter_test.go +++ b/grype/presenter/sarif/presenter_test.go @@ -118,10 +118,10 @@ func createImagePresenter(t *testing.T) *Presenter { return pres } -func createDirPresenter(t *testing.T) *Presenter { +func createDirPresenter(t *testing.T, path string) *Presenter { matches, packages := createResults() - s, err := source.NewFromDirectory("/some/path") + s, err := source.NewFromDirectory(path) if err != nil { t.Fatal(err) } @@ -131,101 +131,114 @@ func createDirPresenter(t *testing.T) *Presenter { return pres } -func Test_locations(t *testing.T) { - pres := createDirPresenter(t) - - // Check . - - pres.srcMetadata = &source.Metadata{ - Scheme: source.DirectoryScheme, - Path: ".", - } - assert.Equal(t, "", pres.inputPath()) - - path := pres.packagePath(pkg.Package{ - Locations: []source.Location{ - { - Coordinates: source.Coordinates{ - RealPath: "/bin/exe", - }, - VirtualPath: "./exe", - }, +func Test_locationPath(t *testing.T) { + tests := []struct { + name string + path string + scheme source.Scheme + real string + virtual string + expected string + }{ + { + name: "dir:.", + scheme: source.DirectoryScheme, + path: ".", + real: "/home/usr/file", + virtual: "file", + expected: "file", }, - }) - - assert.Equal(t, "exe", path) - - path = pres.packagePath(pkg.Package{ - Locations: []source.Location{ - { - Coordinates: source.Coordinates{ - RealPath: "/bin/exe", - }, - }, + { + name: "dir:./", + scheme: source.DirectoryScheme, + path: "./", + real: "/home/usr/file", + virtual: "file", + expected: "file", }, - }) - - assert.Equal(t, "/bin/exe", path) - - // check ./ - - pres.srcMetadata = &source.Metadata{ - Scheme: source.DirectoryScheme, - Path: "./", - } - assert.Equal(t, "", pres.inputPath()) - - path = pres.packagePath(pkg.Package{ - Locations: []source.Location{ - { - Coordinates: source.Coordinates{ - RealPath: "/bin/exe", - }, - }, + { + name: "dir:./someplace", + scheme: source.DirectoryScheme, + path: "./someplace", + real: "/home/usr/file", + virtual: "file", + expected: "someplace/file", }, - }) - - assert.Equal(t, "/bin/exe", path) - - path = pres.packagePath(pkg.Package{ - Locations: []source.Location{ - { - Coordinates: source.Coordinates{ - RealPath: "/bin/exe", - }, - VirtualPath: "exe", - }, + { + name: "dir:/someplace", + scheme: source.DirectoryScheme, + path: "/someplace", + real: "file", + expected: "/someplace/file", + }, + { + name: "dir:/someplace symlink", + scheme: source.DirectoryScheme, + path: "/someplace", + real: "/someplace/usr/file", + virtual: "file", + expected: "/someplace/file", + }, + { + name: "dir:/someplace absolute", + scheme: source.DirectoryScheme, + path: "/someplace", + real: "/usr/file", + expected: "/usr/file", + }, + { + name: "file:/someplace/file", + scheme: source.FileScheme, + path: "/someplace/file", + real: "/usr/file", + expected: "/usr/file", + }, + { + name: "file:/someplace/file relative", + scheme: source.FileScheme, + path: "/someplace/file", + real: "file", + expected: "file", + }, + { + name: "image", + scheme: source.ImageScheme, + path: "alpine:latest", + real: "/etc/file", + expected: "/etc/file", + }, + { + name: "image symlink", + scheme: source.ImageScheme, + path: "alpine:latest", + real: "/etc/elsewhere/file", + virtual: "/etc/file", + expected: "/etc/file", }, - }) - - assert.Equal(t, "exe", path) - - // Check relative path - - pres.srcMetadata = &source.Metadata{ - Scheme: source.DirectoryScheme, - Path: "./file", - } - assert.Equal(t, "file", pres.inputPath()) - - // Check absolute path: - - pres.srcMetadata = &source.Metadata{ - Scheme: source.DirectoryScheme, - Path: "/usr", } - assert.Equal(t, "/usr", pres.inputPath()) - path = pres.packagePath(pkg.Package{ - Locations: []source.Location{ - { - VirtualPath: "/usr/bin/exe", - }, - }, - }) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + pres := createDirPresenter(t, test.path) + pres.srcMetadata = &source.Metadata{ + Scheme: test.scheme, + Path: test.path, + } - assert.Equal(t, "/usr/bin/exe", path) + path := pres.packagePath(pkg.Package{ + Locations: []source.Location{ + { + Coordinates: source.Coordinates{ + RealPath: test.real, + }, + VirtualPath: test.virtual, + }, + }, + }) + assert.Equal(t, test.expected, path) + }) + } } func Test_imageToSarifReport(t *testing.T) { @@ -257,7 +270,7 @@ func Test_imageToSarifReport(t *testing.T) { } func Test_dirToSarifReport(t *testing.T) { - pres := createDirPresenter(t) + pres := createDirPresenter(t, "/abs/path") s, err := pres.toSarifReport() assert.NoError(t, err) @@ -275,13 +288,13 @@ func Test_dirToSarifReport(t *testing.T) { assert.Equal(t, "CVE-1999-0001-package-1", *result.RuleID) assert.Len(t, result.Locations, 1) location := result.Locations[0] - assert.Equal(t, "etc/pkg-1", *location.PhysicalLocation.ArtifactLocation.URI) + assert.Equal(t, "/abs/path/etc/pkg-1", *location.PhysicalLocation.ArtifactLocation.URI) result = run.Results[1] assert.Equal(t, "CVE-1999-0002-package-2", *result.RuleID) assert.Len(t, result.Locations, 1) location = result.Locations[0] - assert.Equal(t, "pkg-2", *location.PhysicalLocation.ArtifactLocation.URI) + assert.Equal(t, "/abs/path/pkg-2", *location.PhysicalLocation.ArtifactLocation.URI) } func TestSarifPresenterImage(t *testing.T) { @@ -311,7 +324,7 @@ func TestSarifPresenterImage(t *testing.T) { func TestSarifPresenterDir(t *testing.T) { var buffer bytes.Buffer - pres := createDirPresenter(t) + pres := createDirPresenter(t, ".") // run presenter err := pres.Present(&buffer) diff --git a/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden b/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden index d8881f66d2c..867042e4e06 100644 --- a/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden +++ b/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden @@ -24,7 +24,7 @@ "markdown": "**Vulnerability CVE-1999-0001**\n| Severity | Package | Version | Fix Version | Type | Location | Data Namespace | Link |\n| --- | --- | --- | --- | --- | --- | --- | --- |\n| low | package-1 | 1.0.1 | | deb | etc/pkg-1 | source-1 | CVE-1999-0001 |\n" }, "properties": { - "security-severity": "4.000000" + "security-severity": "4.0" } }, { @@ -42,7 +42,7 @@ "markdown": "**Vulnerability CVE-1999-0002**\n| Severity | Package | Version | Fix Version | Type | Location | Data Namespace | Link |\n| --- | --- | --- | --- | --- | --- | --- | --- |\n| critical | package-2 | 2.0.1 | | deb | pkg-2 | source-2 | CVE-1999-0002 |\n" }, "properties": { - "security-severity": "1.000000" + "security-severity": "1.0" } } ] diff --git a/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterImage.golden b/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterImage.golden index f4dc1345e9b..cfcfa38717a 100644 --- a/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterImage.golden +++ b/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterImage.golden @@ -24,7 +24,7 @@ "markdown": "**Vulnerability CVE-1999-0001**\n| Severity | Package | Version | Fix Version | Type | Location | Data Namespace | Link |\n| --- | --- | --- | --- | --- | --- | --- | --- |\n| low | package-1 | 1.0.1 | | deb | etc/pkg-1 | source-1 | CVE-1999-0001 |\n" }, "properties": { - "security-severity": "4.000000" + "security-severity": "4.0" } }, { @@ -42,7 +42,7 @@ "markdown": "**Vulnerability CVE-1999-0002**\n| Severity | Package | Version | Fix Version | Type | Location | Data Namespace | Link |\n| --- | --- | --- | --- | --- | --- | --- | --- |\n| critical | package-2 | 2.0.1 | | deb | pkg-2 | source-2 | CVE-1999-0002 |\n" }, "properties": { - "security-severity": "1.000000" + "security-severity": "1.0" } } ]