Skip to content

Commit

Permalink
Correct issue with SARIF dir scan relative paths (#682)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kzantow
kzantow committed Mar 21, 2022
1 parent 778ce33 commit 78cd067
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 36 deletions.
43 changes: 28 additions & 15 deletions grype/presenter/sarif/presenter.go
View file
Expand Up @@ -151,34 +151,44 @@ 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.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 {
return ""
}
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 packagePath
return ""
}
return inputPath
}

// locationPath returns a path for the location
func locationPath(l source.Location) string {
path := l.RealPath
if l.VirtualPath != "" {
path = l.VirtualPath
}
return strings.TrimPrefix(path, "./")
}

// 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),
Expand All @@ -193,10 +203,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{
Expand Down
114 changes: 101 additions & 13 deletions grype/presenter/sarif/presenter_test.go
View file
Expand Up @@ -7,7 +7,6 @@ import (
"regexp"
"testing"

"github.com/sergi/go-diff/diffmatchpatch"
"github.com/stretchr/testify/assert"

"github.com/anchore/go-testutils"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -179,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) {
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden
View file
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit 78cd067

Please sign in to comment.