From adba3a4cc2fe9fb68c2cb2c4b971f6a44032621a Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Mon, 21 Mar 2022 11:57:33 -0400 Subject: [PATCH 1/4] Correct issue with SARIF dir scan relative paths Signed-off-by: Keith Zantow --- grype/presenter/sarif/presenter.go | 44 +++++++++++++++++++----------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/grype/presenter/sarif/presenter.go b/grype/presenter/sarif/presenter.go index ec748d8c908..839290b7957 100644 --- a/grype/presenter/sarif/presenter.go +++ b/grype/presenter/sarif/presenter.go @@ -151,34 +151,43 @@ 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 { - inputPath := strings.TrimPrefix(pres.srcMetadata.Path, "./") - if inputPath == "." { - inputPath = "" - } if len(p.Locations) > 0 { - location := p.Locations[0] - packagePath := location.RealPath - if location.VirtualPath != "" { - packagePath = location.VirtualPath - } - if pres.srcMetadata.Scheme == source.DirectoryScheme { - packagePath = fmt.Sprintf("%s/%s", inputPath, packagePath) + return locationPath(p.Locations[0]) + } + return pres.inputPath() +} + +// inputPath returns a friendlier relative path or absolute path depending on the input, not prefixed by . or ./ +func (pres *Presenter) inputPath() string { + if pres.srcMetadata != nil { + inputPath := strings.TrimPrefix(pres.srcMetadata.Path, "./") + if inputPath == "." { + return "" } - return packagePath + return inputPath + } + return "" +} + +// locationPath returns a path for the location +func locationPath(l source.Location) string { + if l.VirtualPath != "" { + return l.VirtualPath } - return inputPath + return l.RealPath } // locations the locations array is a single "physical" location with potentially multiple logical locations func (pres *Presenter) locations(m match.Match) []*sarif.Location { - var logicalLocations []*sarif.LogicalLocation physicalLocation := pres.packagePath(m.Package) - trimmedPath := strings.TrimPrefix(physicalLocation, "/") + + var logicalLocations []*sarif.LogicalLocation switch pres.srcMetadata.Scheme { case source.ImageScheme: img := pres.srcMetadata.ImageMetadata.UserInput for _, l := range m.Package.Locations { + trimmedPath := strings.TrimPrefix(locationPath(l), "/") logicalLocations = append(logicalLocations, &sarif.LogicalLocation{ FullyQualifiedName: sp(fmt.Sprintf("%s@%s:/%s", img, l.FileSystemID, trimmedPath)), Name: sp(l.RealPath), @@ -193,10 +202,13 @@ 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, trimmedPath)), + FullyQualifiedName: sp(fmt.Sprintf("%s:/%s", pres.srcMetadata.Path, locationPath(l))), Name: sp(l.RealPath), }) } + case source.DirectoryScheme: + // Get a friendly relative location as well as possible + physicalLocation = strings.TrimPrefix(physicalLocation, pres.inputPath()) } return []*sarif.Location{ From c09121344e602a8960e05890d876dd60a2d9d7a7 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Mon, 21 Mar 2022 12:22:32 -0400 Subject: [PATCH 2/4] Invert nil check logic Signed-off-by: Keith Zantow --- grype/presenter/sarif/presenter.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/grype/presenter/sarif/presenter.go b/grype/presenter/sarif/presenter.go index 839290b7957..5151fe8393d 100644 --- a/grype/presenter/sarif/presenter.go +++ b/grype/presenter/sarif/presenter.go @@ -159,14 +159,14 @@ func (pres *Presenter) packagePath(p pkg.Package) string { // inputPath returns a friendlier relative path or absolute path depending on the input, not prefixed by . or ./ func (pres *Presenter) inputPath() string { - if pres.srcMetadata != nil { - inputPath := strings.TrimPrefix(pres.srcMetadata.Path, "./") - if inputPath == "." { - return "" - } - return inputPath + if pres.srcMetadata == nil { + return "" } - return "" + inputPath := strings.TrimPrefix(pres.srcMetadata.Path, "./") + if inputPath == "." { + return "" + } + return inputPath } // locationPath returns a path for the location From f16935611a8f39ea18d5b61d5732bb1ea9486192 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Mon, 21 Mar 2022 14:43:08 -0400 Subject: [PATCH 3/4] Add some testing Signed-off-by: Keith Zantow --- grype/presenter/sarif/presenter.go | 5 +- grype/presenter/sarif/presenter_test.go | 110 +++++++++++++++++++++--- 2 files changed, 102 insertions(+), 13 deletions(-) diff --git a/grype/presenter/sarif/presenter.go b/grype/presenter/sarif/presenter.go index 5151fe8393d..a85b7f417a9 100644 --- a/grype/presenter/sarif/presenter.go +++ b/grype/presenter/sarif/presenter.go @@ -171,10 +171,11 @@ func (pres *Presenter) inputPath() string { // locationPath returns a path for the location func locationPath(l source.Location) string { + path := l.RealPath if l.VirtualPath != "" { - return l.VirtualPath + path = l.VirtualPath } - return l.RealPath + return strings.TrimPrefix(path, "./") } // locations the locations array is a single "physical" location with potentially multiple logical locations diff --git a/grype/presenter/sarif/presenter_test.go b/grype/presenter/sarif/presenter_test.go index a85edb8258d..e0bb4aec5a2 100644 --- a/grype/presenter/sarif/presenter_test.go +++ b/grype/presenter/sarif/presenter_test.go @@ -7,7 +7,6 @@ import ( "regexp" "testing" - "github.com/sergi/go-diff/diffmatchpatch" "github.com/stretchr/testify/assert" "github.com/anchore/go-testutils" @@ -132,6 +131,103 @@ 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", + }, + }, + }) + + assert.Equal(t, "exe", path) + + path = pres.packagePath(pkg.Package{ + Locations: []source.Location{ + { + Coordinates: source.Coordinates{ + RealPath: "/bin/exe", + }, + }, + }, + }) + + 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", + }, + }, + }, + }) + + assert.Equal(t, "/bin/exe", path) + + path = pres.packagePath(pkg.Package{ + Locations: []source.Location{ + { + Coordinates: source.Coordinates{ + RealPath: "/bin/exe", + }, + VirtualPath: "exe", + }, + }, + }) + + 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", + }, + }, + }) + + assert.Equal(t, "/usr/bin/exe", path) + +} + func Test_imageToSarifReport(t *testing.T) { pres := createImagePresenter(t) s, err := pres.toSarifReport() @@ -210,11 +306,7 @@ func TestSarifPresenterImage(t *testing.T) { actual = redact(actual) expected = redact(expected) - if !bytes.Equal(expected, actual) { - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(string(expected), string(actual), true) - t.Errorf("mismatched output:\n%s", dmp.DiffPrettyText(diffs)) - } + assert.JSONEq(t, string(expected), string(actual)) } func TestSarifPresenterDir(t *testing.T) { @@ -238,11 +330,7 @@ func TestSarifPresenterDir(t *testing.T) { actual = redact(actual) expected = redact(expected) - if !bytes.Equal(expected, actual) { - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(string(expected), string(actual), true) - t.Errorf("mismatched output:\n%s", dmp.DiffPrettyText(diffs)) - } + assert.JSONEq(t, string(expected), string(actual)) } func redact(s []byte) []byte { From c7f0077aa00b0feeb454413cf64616ae277597ee Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Mon, 21 Mar 2022 14:50:32 -0400 Subject: [PATCH 4/4] Fix tests Signed-off-by: Keith Zantow --- grype/presenter/sarif/presenter_test.go | 4 ++-- .../snapshot/TestSarifPresenterDir.golden | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/grype/presenter/sarif/presenter_test.go b/grype/presenter/sarif/presenter_test.go index e0bb4aec5a2..c839319ae02 100644 --- a/grype/presenter/sarif/presenter_test.go +++ b/grype/presenter/sarif/presenter_test.go @@ -275,13 +275,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, "/some/path/etc/pkg-1", *location.PhysicalLocation.ArtifactLocation.URI) + assert.Equal(t, "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, "/some/path/pkg-2", *location.PhysicalLocation.ArtifactLocation.URI) + assert.Equal(t, "pkg-2", *location.PhysicalLocation.ArtifactLocation.URI) } func TestSarifPresenterImage(t *testing.T) { diff --git a/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden b/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden index 2bab6b8eaa9..d8881f66d2c 100644 --- a/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden +++ b/grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden @@ -20,8 +20,8 @@ }, "helpUri": "https://github.com/anchore/grype", "help": { - "text": "Vulnerability CVE-1999-0001\nSeverity: low\nPackage: package-1\nVersion: 1.0.1\nFix Version: \nType: deb\nLocation: /some/path/etc/pkg-1\nData Namespace: source-1\nLink: CVE-1999-0001", - "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 | /some/path/etc/pkg-1 | source-1 | CVE-1999-0001 |\n" + "text": "Vulnerability CVE-1999-0001\nSeverity: low\nPackage: package-1\nVersion: 1.0.1\nFix Version: \nType: deb\nLocation: etc/pkg-1\nData Namespace: source-1\nLink: CVE-1999-0001", + "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" @@ -38,8 +38,8 @@ }, "helpUri": "https://github.com/anchore/grype", "help": { - "text": "Vulnerability CVE-1999-0002\nSeverity: critical\nPackage: package-2\nVersion: 2.0.1\nFix Version: \nType: deb\nLocation: /some/path/pkg-2\nData Namespace: source-2\nLink: CVE-1999-0002", - "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 | /some/path/pkg-2 | source-2 | CVE-1999-0002 |\n" + "text": "Vulnerability CVE-1999-0002\nSeverity: critical\nPackage: package-2\nVersion: 2.0.1\nFix Version: \nType: deb\nLocation: pkg-2\nData Namespace: source-2\nLink: CVE-1999-0002", + "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" @@ -52,13 +52,13 @@ { "ruleId": "CVE-1999-0001-package-1", "message": { - "text": "The path /some/path/etc/pkg-1 reports package-1 at version 1.0.1 which would result in a vulnerable (deb) package installed" + "text": "The path etc/pkg-1 reports package-1 at version 1.0.1 which would result in a vulnerable (deb) package installed" }, "locations": [ { "physicalLocation": { "artifactLocation": { - "uri": "/some/path/etc/pkg-1" + "uri": "etc/pkg-1" }, "region": { "startLine": 1, @@ -73,13 +73,13 @@ { "ruleId": "CVE-1999-0002-package-2", "message": { - "text": "The path /some/path/pkg-2 reports package-2 at version 2.0.1 which would result in a vulnerable (deb) package installed" + "text": "The path pkg-2 reports package-2 at version 2.0.1 which would result in a vulnerable (deb) package installed" }, "locations": [ { "physicalLocation": { "artifactLocation": { - "uri": "/some/path/pkg-2" + "uri": "pkg-2" }, "region": { "startLine": 1,