Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add osv output lockfile + refactor #505

Merged
merged 14 commits into from
Aug 30, 2023
3 changes: 3 additions & 0 deletions pkg/lockfile/extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ func OpenLocalDepFile(path string) (NestedDepFile, error) {
return LocalFile{}, err
}

// Very unlikely to have Abs return an error if the file opens correctly
path, _ = filepath.Abs(path)

return LocalFile{r, path}, nil
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/lockfile/fixtures/osvscannerresults/empty.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"results": []
}
504 changes: 504 additions & 0 deletions pkg/lockfile/fixtures/osvscannerresults/multi-packages-with-vulns.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pkg/lockfile/fixtures/osvscannerresults/not-json.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
this is not valid json! (I think)
21 changes: 21 additions & 0 deletions pkg/lockfile/fixtures/osvscannerresults/one-package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"results": [
{
"source": {
"path": "/path/to/Gemfile.lock",
"type": "lockfile"
},
"packages": [
{
"package": {
"name": "activesupport",
"version": "7.0.7",
"ecosystem": "RubyGems"
},
"vulnerabilities": [],
"groups": []
}
]
}
]
}
15 changes: 9 additions & 6 deletions pkg/lockfile/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ func packageToString(pkg lockfile.PackageDetails) string {
return fmt.Sprintf("%s@%s (%s, %s)", pkg.Name, pkg.Version, pkg.Ecosystem, commit)
}

func hasPackage(packages []lockfile.PackageDetails, pkg lockfile.PackageDetails) bool {
func hasPackage(t *testing.T, packages []lockfile.PackageDetails, pkg lockfile.PackageDetails) bool {
t.Helper()

for _, details := range packages {
if details == pkg {
return true
Expand All @@ -44,7 +46,7 @@ func hasPackage(packages []lockfile.PackageDetails, pkg lockfile.PackageDetails)
func expectPackage(t *testing.T, packages []lockfile.PackageDetails, pkg lockfile.PackageDetails) {
t.Helper()

if !hasPackage(packages, pkg) {
if !hasPackage(t, packages, pkg) {
t.Errorf(
"Expected packages to include %s@%s (%s, %s), but it did not",
pkg.Name,
Expand All @@ -55,11 +57,12 @@ func expectPackage(t *testing.T, packages []lockfile.PackageDetails, pkg lockfil
}
}

func findMissingPackages(actualPackages []lockfile.PackageDetails, expectedPackages []lockfile.PackageDetails) []lockfile.PackageDetails {
func findMissingPackages(t *testing.T, actualPackages []lockfile.PackageDetails, expectedPackages []lockfile.PackageDetails) []lockfile.PackageDetails {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is good by itself along with the hasPackage one, so if you can be bothered I'd favor landing it in its own PR so that this diff is reduced rather than have it wait around for the rest of the changes to get reviewed

t.Helper()
var missingPackages []lockfile.PackageDetails

for _, pkg := range actualPackages {
if !hasPackage(expectedPackages, pkg) {
if !hasPackage(t, expectedPackages, pkg) {
missingPackages = append(missingPackages, pkg)
}
}
Expand All @@ -79,8 +82,8 @@ func expectPackages(t *testing.T, actualPackages []lockfile.PackageDetails, expe
)
}

missingActualPackages := findMissingPackages(actualPackages, expectedPackages)
missingExpectedPackages := findMissingPackages(expectedPackages, actualPackages)
missingActualPackages := findMissingPackages(t, actualPackages, expectedPackages)
missingExpectedPackages := findMissingPackages(t, expectedPackages, actualPackages)

if len(missingActualPackages) != 0 {
for _, unexpectedPackage := range missingActualPackages {
Expand Down
87 changes: 87 additions & 0 deletions pkg/lockfile/osv-vuln-result_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package lockfile_test

import (
"testing"

"github.com/google/osv-scanner/pkg/lockfile"
)

func TestParseOSVScannerResults_FileDoesNotExist(t *testing.T) {
t.Parallel()

packages, err := lockfile.ParseOSVScannerResults("fixtures/osvscannerresults/does-not-exist")

expectErrContaining(t, err, "no such file or directory")
expectPackages(t, packages, []lockfile.PackageDetails{})
}

func TestParseOSVScannerResults_InvalidJSON(t *testing.T) {
t.Parallel()

packages, err := lockfile.ParseOSVScannerResults("fixtures/osvscannerresults/not-json.txt")

expectErrContaining(t, err, "could not extract from")
expectPackages(t, packages, []lockfile.PackageDetails{})
}

func TestParseOSVScannerResults_NoPackages(t *testing.T) {
t.Parallel()

packages, err := lockfile.ParseOSVScannerResults("fixtures/osvscannerresults/empty.json")

if err != nil {
t.Errorf("Got unexpected error: %v", err)
}

expectPackages(t, packages, []lockfile.PackageDetails{})
}

func TestParseOSVScannerResults_OnePackage(t *testing.T) {
t.Parallel()

packages, err := lockfile.ParseOSVScannerResults("fixtures/osvscannerresults/one-package.json")

if err != nil {
t.Errorf("Got unexpected error: %v", err)
}

expectPackages(t, packages, []lockfile.PackageDetails{
{
Name: "activesupport",
Version: "7.0.7",
Ecosystem: lockfile.BundlerEcosystem,
CompareAs: lockfile.BundlerEcosystem,
},
})
}

func TestParseOSVScannerResults_MultiPackages(t *testing.T) {
t.Parallel()

packages, err := lockfile.ParseOSVScannerResults("fixtures/osvscannerresults/multi-packages-with-vulns.json")

if err != nil {
t.Errorf("Got unexpected error: %v", err)
}

expectPackages(t, packages, []lockfile.PackageDetails{
{
Name: "crossbeam-utils",
Version: "0.6.6",
Ecosystem: lockfile.CargoEcosystem,
CompareAs: lockfile.CargoEcosystem,
},
{
Name: "memoffset",
Version: "0.5.6",
Ecosystem: lockfile.CargoEcosystem,
CompareAs: lockfile.CargoEcosystem,
},
{
Name: "smallvec",
Version: "1.6.0",
Ecosystem: lockfile.CargoEcosystem,
CompareAs: lockfile.CargoEcosystem,
},
})
}
55 changes: 55 additions & 0 deletions pkg/lockfile/osv-vuln-results.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package lockfile

import (
"encoding/json"
"fmt"

"github.com/google/osv-scanner/pkg/models"
)

func ParseOSVScannerResults(pathToLockfile string) ([]PackageDetails, error) {
return extractFromFile(pathToLockfile, OSVScannerResultsExtractor{})
}

type OSVScannerResultsExtractor struct{}

func (e OSVScannerResultsExtractor) ShouldExtract(path string) bool {
// The output will always be a custom json file, so don't return a default should extract
return false
}

func (e OSVScannerResultsExtractor) Extract(f DepFile) ([]PackageDetails, error) {
parsedResults := models.VulnerabilityResults{}
err := json.NewDecoder(f).Decode(&parsedResults)

if err != nil {
return nil, fmt.Errorf("could not extract from %s: %w", f.Path(), err)
}

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),
})
}
}

return packages, nil
}

var _ Extractor = OSVScannerResultsExtractor{}

// FromOSVScannerResults attempts to extract packages stored in the OSVScannerResults format
func FromOSVScannerResults(pathToInstalled string) (Lockfile, error) {
packages, err := extractFromFile(pathToInstalled, OSVScannerResultsExtractor{})

return Lockfile{
FilePath: pathToInstalled,
ParsedAs: "osv-scanner-results",
Packages: packages,
}, err
}
1 change: 1 addition & 0 deletions pkg/lockfile/parse-requirements-txt.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func parseRequirementsTxt(f DepFile, requiredAlready map[string]struct{}) ([]Pac
}

detail := parseLine(line)

packages[detail.Name+"@"+detail.Version] = detail
}

Expand Down