Skip to content

Commit

Permalink
Improve SARIF path handling and severity (#686)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kzantow
kzantow committed Mar 22, 2022
1 parent d40fb77 commit b2e66d3
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 106 deletions.
31 changes: 22 additions & 9 deletions grype/presenter/sarif/presenter.go
View file
Expand Up @@ -3,6 +3,7 @@ package sarif
import (
"fmt"
"io"
"path/filepath"
"strings"

"github.com/owenrumney/go-sarif/sarif"
Expand Down Expand Up @@ -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()
}
Expand All @@ -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
Expand All @@ -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),
Expand All @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down
199 changes: 106 additions & 93 deletions grype/presenter/sarif/presenter_test.go
View file
Expand Up @@ -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)
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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)

Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterDir.golden
View file
Expand Up @@ -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"
}
},
{
Expand All @@ -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"
}
}
]
Expand Down
4 changes: 2 additions & 2 deletions grype/presenter/sarif/test-fixtures/snapshot/TestSarifPresenterImage.golden
View file
Expand Up @@ -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"
}
},
{
Expand All @@ -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"
}
}
]
Expand Down

0 comments on commit b2e66d3

Please sign in to comment.