-
Notifications
You must be signed in to change notification settings - Fork 377
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: Use lockfile scalibr interface (#1330)
This PR contains all the code required to move to osv-scalibr while making the existing code compile and pass all tests (container tests not passing because of a bug in the scalibr alpine extractor). Changes not mentioned in the following list will be split off in separate PRs which should land before this PR. Those are: - [x] #1337 - [x] #1331 - [x] #1338 - [x] #1341 - [x] #1345 Changes in this PR: - Fixture changes: - Scalibr Python requirements.txt extractor currently doesn't support packages without versions, so added some version strings to the test files - Image package required quite a bit of reworking to successfully update. - Add the ability to iterate through a directory via the pathtree library - Support scalibr FS interface for Layers - Add conversion code to convert inventories from osv-scalibr back to v1's lockfile and Inventory - This is done to minimize snapshot changes. Followup PRs should remove this conversion - Add `internal/lockfilescalibr` package: - `errors.go` adds common extraction errors we want to translate. - `translation.go` adds helper functions and translation logic between osv-scanner v1 extractor names, and osv-scalibr extractor names. Changes in followup PRs: - Delete lockfiles package and migrate everything to use osv-scalibr extractors - Remove conversion code in image --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Gareth Jones <Jones258@Gmail.com> Co-authored-by: Xueqin Cui <72771658+cuixq@users.noreply.github.com> Co-authored-by: Michael Kedar <michaelkedar@google.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Loading branch information
5 people
committed
Nov 29, 2024
1 parent
cf6f568
commit 587bf77
Showing
32 changed files
with
718 additions
and
472 deletions.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
2 changes: 1 addition & 1 deletion
2
cmd/osv-scanner/fixtures/locks-requirements/my-requirements.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| flask | ||
| flask==1.0.0 |
2 changes: 1 addition & 1 deletion
2
cmd/osv-scanner/fixtures/locks-requirements/requirements-dev.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| black | ||
| black==1.0.0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| flask | ||
| flask-cors | ||
| flask==1.0.0 | ||
| flask-cors==1.0.0 | ||
| pandas==0.23.4 |
2 changes: 1 addition & 1 deletion
2
cmd/osv-scanner/fixtures/locks-requirements/the_requirements_for_test.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| pytest | ||
| pytest==1.0.0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,134 +1,113 @@ | ||
| package image | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
| "path" | ||
| "sort" | ||
|
|
||
| "io/fs" | ||
| "strings" | ||
|
|
||
| "github.com/google/osv-scalibr/extractor" | ||
| "github.com/google/osv-scalibr/extractor/filesystem" | ||
| "github.com/google/osv-scalibr/extractor/filesystem/language/golang/gobinary" | ||
| "github.com/google/osv-scalibr/extractor/filesystem/os/apk" | ||
| "github.com/google/osv-scalibr/extractor/filesystem/os/dpkg" | ||
| "github.com/google/osv-scanner/internal/lockfilescalibr" | ||
| "github.com/google/osv-scanner/internal/lockfilescalibr/language/javascript/nodemodules" | ||
| "github.com/google/osv-scanner/pkg/lockfile" | ||
| ) | ||
|
|
||
| // artifactExtractors contains only extractors for artifacts that are important in | ||
| // the final layer of a container image | ||
| var artifactExtractors map[string]lockfile.Extractor = map[string]lockfile.Extractor{ | ||
| "node_modules": lockfile.NodeModulesExtractor{}, | ||
| "apk-installed": lockfile.ApkInstalledExtractor{}, | ||
| "dpkg": lockfile.DpkgStatusExtractor{}, | ||
| "go-binary": lockfile.GoBinaryExtractor{}, | ||
| } | ||
|
|
||
| type extractorPair struct { | ||
| extractor lockfile.Extractor | ||
| name string | ||
| var artifactExtractors []filesystem.Extractor = []filesystem.Extractor{ | ||
| // TODO: Using nodemodules extractor to minimize changes of snapshots | ||
| // After annotations are added, we should switch to using packagejson. | ||
| // packagejson.New(packagejson.DefaultConfig()), | ||
| nodemodules.Extractor{}, | ||
|
|
||
| apk.New(apk.DefaultConfig()), | ||
| gobinary.New(gobinary.DefaultConfig()), | ||
| // TODO: Add tests for debian containers | ||
| dpkg.New(dpkg.DefaultConfig()), | ||
| } | ||
|
|
||
| func findArtifactExtractor(path string) []extractorPair { | ||
| func findArtifactExtractor(path string, fileInfo fs.FileInfo) []filesystem.Extractor { | ||
| // Use ShouldExtract to collect and return a slice of artifactExtractors | ||
| var extractors []extractorPair | ||
| for name, extractor := range artifactExtractors { | ||
| if extractor.ShouldExtract(path) { | ||
| extractors = append(extractors, extractorPair{extractor, name}) | ||
| var extractors []filesystem.Extractor | ||
| for _, extractor := range artifactExtractors { | ||
| if extractor.FileRequired(path, fileInfo) { | ||
| extractors = append(extractors, extractor) | ||
| } | ||
| } | ||
|
|
||
| return extractors | ||
| } | ||
|
|
||
| func extractArtifactDeps(path string, layer *Layer) (lockfile.Lockfile, error) { | ||
| foundExtractors := findArtifactExtractor(path) | ||
| // Note: Output is non deterministic | ||
| func extractArtifactDeps(extractPath string, layer *Layer) ([]*extractor.Inventory, error) { | ||
| pathFileInfo, err := layer.Stat(extractPath) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("attempted to get FileInfo but failed: %w", err) | ||
| } | ||
|
|
||
| scalibrPath := strings.TrimPrefix(extractPath, "/") | ||
| foundExtractors := findArtifactExtractor(scalibrPath, pathFileInfo) | ||
| if len(foundExtractors) == 0 { | ||
| return lockfile.Lockfile{}, fmt.Errorf("%w for %s", lockfile.ErrExtractorNotFound, path) | ||
| return nil, fmt.Errorf("%w for %s", lockfilescalibr.ErrExtractorNotFound, extractPath) | ||
| } | ||
|
|
||
| packages := []lockfile.PackageDetails{} | ||
| inventories := []*extractor.Inventory{} | ||
| var extractedAs string | ||
| for _, extPair := range foundExtractors { | ||
| for _, extractor := range foundExtractors { | ||
| // File has to be reopened per extractor as each extractor moves the read cursor | ||
| f, err := OpenLayerFile(path, layer) | ||
| f, err := layer.Open(extractPath) | ||
| if err != nil { | ||
| return lockfile.Lockfile{}, fmt.Errorf("attempted to open file but failed: %w", err) | ||
| return nil, fmt.Errorf("attempted to open file but failed: %w", err) | ||
| } | ||
|
|
||
| scanInput := &filesystem.ScanInput{ | ||
| FS: layer, | ||
| Path: scalibrPath, | ||
| Root: "/", | ||
| Reader: f, | ||
| Info: pathFileInfo, | ||
| } | ||
|
|
||
| newPackages, err := extPair.extractor.Extract(f) | ||
| newPackages, err := extractor.Extract(context.Background(), scanInput) | ||
| f.Close() | ||
|
|
||
| if err != nil { | ||
| if errors.Is(err, lockfile.ErrIncompatibleFileFormat) { | ||
| continue | ||
| } | ||
|
|
||
| return lockfile.Lockfile{}, fmt.Errorf("(extracting as %s) %w", extPair.name, err) | ||
| return nil, fmt.Errorf("(extracting as %s) %w", extractor.Name(), err) | ||
| } | ||
|
|
||
| extractedAs = extPair.name | ||
| packages = newPackages | ||
| // TODO(rexpan): Determine if it's acceptable to have multiple extractors | ||
| for i := range newPackages { | ||
| newPackages[i].Extractor = extractor | ||
| } | ||
|
|
||
| extractedAs = extractor.Name() | ||
| inventories = newPackages | ||
| // TODO(rexpan): Determine if this it's acceptable to have multiple extractors | ||
| // extract from the same file successfully | ||
| break | ||
| } | ||
|
|
||
| if extractedAs == "" { | ||
| return lockfile.Lockfile{}, fmt.Errorf("%w for %s", lockfile.ErrExtractorNotFound, path) | ||
| return nil, fmt.Errorf("%w for %s", lockfilescalibr.ErrExtractorNotFound, extractPath) | ||
| } | ||
|
|
||
| // Sort to have deterministic output, and to match behavior of lockfile.extractDeps | ||
| sort.Slice(packages, func(i, j int) bool { | ||
| if packages[i].Name == packages[j].Name { | ||
| return packages[i].Version < packages[j].Version | ||
| // Perform any one-off translations here | ||
| for _, inv := range inventories { | ||
| // Scalibr uses go to indicate go compiler version | ||
| // We specifically cares about the stdlib version inside the package | ||
| // so convert the package name from go to stdlib | ||
| if inv.Ecosystem() == "Go" && inv.Name == "go" { | ||
| inv.Name = "stdlib" | ||
| } | ||
|
|
||
| return packages[i].Name < packages[j].Name | ||
| }) | ||
|
|
||
| return lockfile.Lockfile{ | ||
| FilePath: path, | ||
| ParsedAs: extractedAs, | ||
| Packages: packages, | ||
| }, nil | ||
| } | ||
|
|
||
| // A File represents a file that exists in an image | ||
| type File struct { | ||
| *os.File | ||
|
|
||
| layer *Layer | ||
| path string | ||
| } | ||
|
|
||
| func (f File) Open(openPath string) (lockfile.NestedDepFile, error) { | ||
| // use path instead of filepath, because container is always in Unix paths (for now) | ||
| if path.IsAbs(openPath) { | ||
| return OpenLayerFile(openPath, f.layer) | ||
| } | ||
|
|
||
| absPath := path.Join(f.path, openPath) | ||
|
|
||
| return OpenLayerFile(absPath, f.layer) | ||
| } | ||
|
|
||
| func (f File) Path() string { | ||
| return f.path | ||
| } | ||
|
|
||
| func OpenLayerFile(path string, layer *Layer) (File, error) { | ||
| fileNode, err := layer.getFileNode(path) | ||
| if err != nil { | ||
| return File{}, err | ||
| } | ||
|
|
||
| file, err := fileNode.Open() | ||
| if err != nil { | ||
| return File{}, err | ||
| } | ||
|
|
||
| return File{ | ||
| File: file, | ||
| path: path, | ||
| layer: layer, | ||
| }, nil | ||
| return inventories, nil | ||
| } | ||
|
|
||
| var _ lockfile.DepFile = File{} | ||
| var _ lockfile.NestedDepFile = File{} |
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| / # cat /etc/os-release | ||
| NAME="Alpine Linux" | ||
| ID=alpine | ||
| VERSION_ID=3.18.1 | ||
| PRETTY_NAME="Alpine Linux v3.18" | ||
| HOME_URL="https://alpinelinux.org/" | ||
| BUG_REPORT_URL="https://gitlab.alpinelinux.org/alpine/aports/-/issues" |
Oops, something went wrong.