Skip to content

Commit

Permalink
refactor: Unify where clients and interfaces are initialized, and how…
Browse files Browse the repository at this point in the history
… extractors are defined (#1500)

After some discussions, this PR properly groups where clients,
interfaces, and extractors so that they are all defined in sensible
places that's easy to extend and modify in the future, and improve
readability of the code.

The scanning process is now:
- Given CLI flags -> enable the clients + matchers we are able to.
<sub>(We assume that initialising a client is not an expensive
operation, nor does it make any network requests, so e.g. even if we
don't scan a pomxml file, we can still initialise the Maven
client)</sub>
- Given list of clients -> We build a list of extractors that can
successfully function with the available clients
- Given list of extractors -> Perform the commanded scanning operations
with the list of extractors to return an inventory list
- Given list of inventories -> Perform matching and enrichment with the
list of available matchers.
- Build result into JSON format
- Return error if vuln found.

This PR also removes deprecated errors, renames errors to the proper
format with Err at the front to remove the nolint flags.
  • Loading branch information
another-rex authored Jan 15, 2025
1 parent 17e7a15 commit a6d4ec8
Show file tree
Hide file tree
Showing 10 changed files with 309 additions and 234 deletions.
6 changes: 3 additions & 3 deletions cmd/osv-reporter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func run(args []string, stdout, stderr io.Writer) int {

// if vulnerability exists it should return error
if len(diffVulns.Results) > 0 && failOnVuln && anyIsCalled {
return osvscanner.VulnerabilitiesFoundErr
return osvscanner.ErrVulnerabilitiesFound
}

return nil
Expand All @@ -194,11 +194,11 @@ func run(args []string, stdout, stderr io.Writer) int {
if tableReporter == nil {
tableReporter = reporter.NewTableReporter(stdout, stderr, reporter.InfoLevel, false, 0)
}
if errors.Is(err, osvscanner.VulnerabilitiesFoundErr) {
if errors.Is(err, osvscanner.ErrVulnerabilitiesFound) {
return 1
}

if errors.Is(err, osvscanner.NoPackagesFoundErr) {
if errors.Is(err, osvscanner.ErrNoPackagesFound) {
tableReporter.Errorf("No package sources found, --help for usage information.\n")
return 128
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/osv-scanner/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ func run(args []string, stdout, stderr io.Writer) int {
r = reporter.NewTableReporter(stdout, stderr, reporter.InfoLevel, false, 0)
}
switch {
case errors.Is(err, osvscanner.VulnerabilitiesFoundErr):
case errors.Is(err, osvscanner.ErrVulnerabilitiesFound):
return 1
case errors.Is(err, osvscanner.NoPackagesFoundErr):
case errors.Is(err, osvscanner.ErrNoPackagesFound):
r.Errorf("No package sources found, --help for usage information.\n")
return 128
case errors.Is(err, osvscanner.ErrAPIFailed):
Expand Down
5 changes: 5 additions & 0 deletions cmd/osv-scanner/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,11 @@ func TestRun_OCIImage(t *testing.T) {
args: []string{"", "--experimental-oci-image", "../../internal/image/fixtures/test-node_modules-pnpm-full.tar"},
exit: 1,
},
{
name: "scanning image with go binary",
args: []string{"", "--experimental-oci-image", "../../internal/image/fixtures/test-package-tracing.tar"},
exit: 1,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion cmd/osv-scanner/scan/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func action(context *cli.Context, stdout, stderr io.Writer) (reporter.Reporter,
},
}, r)

if err != nil && !errors.Is(err, osvscanner.VulnerabilitiesFoundErr) {
if err != nil && !errors.Is(err, osvscanner.ErrVulnerabilitiesFound) {
return r, err
}

Expand Down
133 changes: 133 additions & 0 deletions pkg/osvscanner/internal/scanners/extractorbuilder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package scanners

import (
"github.com/google/osv-scalibr/extractor/filesystem"
"github.com/google/osv-scalibr/extractor/filesystem/language/cpp/conanlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/dart/pubspec"
"github.com/google/osv-scalibr/extractor/filesystem/language/dotnet/packageslockjson"
"github.com/google/osv-scalibr/extractor/filesystem/language/erlang/mixlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/golang/gobinary"
"github.com/google/osv-scalibr/extractor/filesystem/language/golang/gomod"
"github.com/google/osv-scalibr/extractor/filesystem/language/java/gradlelockfile"
"github.com/google/osv-scalibr/extractor/filesystem/language/java/gradleverificationmetadataxml"
"github.com/google/osv-scalibr/extractor/filesystem/language/java/pomxml"
"github.com/google/osv-scalibr/extractor/filesystem/language/javascript/packagelockjson"
"github.com/google/osv-scalibr/extractor/filesystem/language/javascript/pnpmlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/javascript/yarnlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/php/composerlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/python/pdmlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/python/pipfilelock"
"github.com/google/osv-scalibr/extractor/filesystem/language/python/poetrylock"
"github.com/google/osv-scalibr/extractor/filesystem/language/python/requirements"
"github.com/google/osv-scalibr/extractor/filesystem/language/r/renvlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/ruby/gemfilelock"
"github.com/google/osv-scalibr/extractor/filesystem/language/rust/cargolock"
"github.com/google/osv-scalibr/extractor/filesystem/os/apk"
"github.com/google/osv-scalibr/extractor/filesystem/os/dpkg"
"github.com/google/osv-scalibr/extractor/filesystem/sbom/cdx"
"github.com/google/osv-scalibr/extractor/filesystem/sbom/spdx"
"github.com/google/osv-scanner/internal/osvdev"
"github.com/google/osv-scanner/internal/resolution/client"
"github.com/google/osv-scanner/internal/resolution/datasource"
"github.com/google/osv-scanner/internal/scalibrextract/filesystem/vendored"
"github.com/google/osv-scanner/internal/scalibrextract/language/java/pomxmlnet"
"github.com/google/osv-scanner/internal/scalibrextract/language/javascript/nodemodules"
"github.com/google/osv-scanner/internal/scalibrextract/vcs/gitrepo"
"github.com/ossf/osv-schema/bindings/go/osvschema"
)

var sbomExtractors = []filesystem.Extractor{
spdx.Extractor{},
cdx.Extractor{},
}

var lockfileExtractors = []filesystem.Extractor{
conanlock.Extractor{},
packageslockjson.Extractor{},
mixlock.Extractor{},
pubspec.Extractor{},
gomod.Extractor{},
gradlelockfile.Extractor{},
gradleverificationmetadataxml.Extractor{},
packagelockjson.Extractor{},
pnpmlock.Extractor{},
yarnlock.Extractor{},
composerlock.Extractor{},
pipfilelock.Extractor{},
pdmlock.Extractor{},
poetrylock.Extractor{},
requirements.Extractor{},
renvlock.Extractor{},
gemfilelock.Extractor{},
cargolock.Extractor{},
}

// BuildLockfileExtractors returns all relevant extractors for lockfile scanning given the required clients
// All clients can be nil, and if nil the extractors requiring those clients will not be returned.
func BuildLockfileExtractors(dependencyClients map[osvschema.Ecosystem]client.DependencyClient, mavenAPIClient *datasource.MavenRegistryAPIClient) []filesystem.Extractor {
extractorsToUse := lockfileExtractors

if dependencyClients[osvschema.EcosystemMaven] != nil && mavenAPIClient != nil {
extractorsToUse = append(extractorsToUse, pomxmlnet.Extractor{
DependencyClient: dependencyClients[osvschema.EcosystemMaven],
MavenRegistryAPIClient: mavenAPIClient,
})
} else {
extractorsToUse = append(extractorsToUse, pomxml.Extractor{})
}

return extractorsToUse
}

// BuildSBOMExtractors returns extractors relevant to SBOM extraction
func BuildSBOMExtractors() []filesystem.Extractor {
return sbomExtractors
}

// BuildWalkerExtractors returns all relevant extractors for directory scanning given the required clients
// All clients can be nil, and if nil the extractors requiring those clients will not be returned.
func BuildWalkerExtractors(
skipGit bool,
osvdevClient *osvdev.OSVClient,
dependencyClients map[osvschema.Ecosystem]client.DependencyClient,
mavenAPIClient *datasource.MavenRegistryAPIClient) []filesystem.Extractor {
relevantExtractors := []filesystem.Extractor{}

if !skipGit {
relevantExtractors = append(relevantExtractors, gitrepo.Extractor{})
}
relevantExtractors = append(relevantExtractors, lockfileExtractors...)
relevantExtractors = append(relevantExtractors, sbomExtractors...)

if osvdevClient != nil {
relevantExtractors = append(relevantExtractors, vendored.Extractor{
ScanGitDir: skipGit,
OSVClient: osvdevClient,
})
}

if dependencyClients[osvschema.EcosystemMaven] != nil && mavenAPIClient != nil {
relevantExtractors = append(relevantExtractors, pomxmlnet.Extractor{
DependencyClient: dependencyClients[osvschema.EcosystemMaven],
MavenRegistryAPIClient: mavenAPIClient,
})
} else {
relevantExtractors = append(relevantExtractors, pomxml.Extractor{})
}

return relevantExtractors
}

// BuildArtifactExtractors returns all relevant extractors for artifact scanning given the required clients
// All clients can be nil, and if nil the extractors requiring those clients will not be returned.
func BuildArtifactExtractors() []filesystem.Extractor {
extractorsToUse := []filesystem.Extractor{
nodemodules.Extractor{},
apk.New(apk.DefaultConfig()),
gobinary.New(gobinary.DefaultConfig()),
// TODO: Add tests for debian containers
dpkg.New(dpkg.DefaultConfig()),
}

return extractorsToUse
}
83 changes: 31 additions & 52 deletions pkg/osvscanner/internal/scanners/lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,6 @@ import (

"github.com/google/osv-scalibr/extractor"
"github.com/google/osv-scalibr/extractor/filesystem"
"github.com/google/osv-scalibr/extractor/filesystem/language/cpp/conanlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/dart/pubspec"
"github.com/google/osv-scalibr/extractor/filesystem/language/dotnet/packageslockjson"
"github.com/google/osv-scalibr/extractor/filesystem/language/erlang/mixlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/golang/gomod"
"github.com/google/osv-scalibr/extractor/filesystem/language/java/gradlelockfile"
"github.com/google/osv-scalibr/extractor/filesystem/language/java/gradleverificationmetadataxml"
"github.com/google/osv-scalibr/extractor/filesystem/language/java/pomxml"
"github.com/google/osv-scalibr/extractor/filesystem/language/javascript/packagelockjson"
"github.com/google/osv-scalibr/extractor/filesystem/language/javascript/pnpmlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/javascript/yarnlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/php/composerlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/python/pdmlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/python/pipfilelock"
"github.com/google/osv-scalibr/extractor/filesystem/language/python/poetrylock"
"github.com/google/osv-scalibr/extractor/filesystem/language/python/requirements"
"github.com/google/osv-scalibr/extractor/filesystem/language/r/renvlock"
"github.com/google/osv-scalibr/extractor/filesystem/language/ruby/gemfilelock"
"github.com/google/osv-scalibr/extractor/filesystem/language/rust/cargolock"
"github.com/google/osv-scalibr/extractor/filesystem/os/apk"
"github.com/google/osv-scalibr/extractor/filesystem/os/dpkg"
"github.com/google/osv-scanner/internal/output"
Expand All @@ -35,27 +16,6 @@ import (
"github.com/google/osv-scanner/pkg/reporter"
)

var lockfileExtractors = []filesystem.Extractor{
conanlock.Extractor{},
packageslockjson.Extractor{},
mixlock.Extractor{},
pubspec.Extractor{},
gomod.Extractor{},
gradlelockfile.Extractor{},
gradleverificationmetadataxml.Extractor{},
packagelockjson.Extractor{},
pnpmlock.Extractor{},
yarnlock.Extractor{},
composerlock.Extractor{},
pipfilelock.Extractor{},
pdmlock.Extractor{},
poetrylock.Extractor{},
requirements.Extractor{},
renvlock.Extractor{},
gemfilelock.Extractor{},
cargolock.Extractor{},
}

var lockfileExtractorMapping = map[string]string{
"pubspec.lock": "dart/pubspec",
"pnpm-lock.yaml": "javascript/pnpmlock",
Expand All @@ -81,28 +41,47 @@ var lockfileExtractorMapping = map[string]string{
"Gemfile.lock": "ruby/gemfilelock",
}

// ScanLockfile will load, identify, and parse the lockfile path passed in, and add the dependencies specified
// ScanSingleFile is similar to ScanSingleFileWithMapping, just without supporting the <lockfileformat>:/path/to/lockfile prefix identifier
func ScanSingleFile(r reporter.Reporter, path string, extractorsToUse []filesystem.Extractor) ([]*extractor.Inventory, error) {
// TODO: Update the logging output to stop referring to SBOMs
path, err := filepath.Abs(path)
if err != nil {
r.Errorf("Failed to resolved path %q with error: %s\n", path, err)
return nil, err
}

invs, err := scalibrextract.ExtractWithExtractors(context.Background(), path, extractorsToUse)
if err != nil {
r.Infof("Failed to parse SBOM %q with error: %s\n", path, err)
return nil, err
}

pkgCount := len(invs)
if pkgCount > 0 {
r.Infof(
"Scanned %s file and found %d %s\n",
path,
pkgCount,
output.Form(pkgCount, "package", "packages"),
)
}

return invs, nil
}

// ScanSingleFileWithMapping will load, identify, and parse the lockfile path passed in, and add the dependencies specified
// within to `query`
//
// TODO(V2 Models): pomExtractor is temporary until V2 Models
func ScanLockfile(r reporter.Reporter, scanArg string, pomExtractor filesystem.Extractor) ([]*extractor.Inventory, error) {
func ScanSingleFileWithMapping(r reporter.Reporter, scanPath string, extractorsToUse []filesystem.Extractor) ([]*extractor.Inventory, error) {
var err error
var inventories []*extractor.Inventory

parseAs, path := parseLockfilePath(scanArg)
parseAs, path := parseLockfilePath(scanPath)

path, err = filepath.Abs(path)
if err != nil {
r.Errorf("Failed to resolved path %q with error: %s\n", path, err)
return nil, err
}
extractorsToUse := lockfileExtractors

if pomExtractor != nil {
extractorsToUse = append(extractorsToUse, pomExtractor)
} else {
extractorsToUse = append(extractorsToUse, pomxml.Extractor{})
}

// special case for the APK and DPKG parsers because they have a very generic name while
// living at a specific location, so they are not included in the map of parsers
Expand Down
45 changes: 0 additions & 45 deletions pkg/osvscanner/internal/scanners/sbom.go

This file was deleted.

Loading

0 comments on commit a6d4ec8

Please sign in to comment.