From 60daa0c2883dbd7fcfd7c6b0218de504a35b0b3f Mon Sep 17 00:00:00 2001 From: Rex P Date: Mon, 28 Oct 2024 15:34:11 +1100 Subject: [PATCH] Address PR comments --- cmd/osv-scanner/__snapshots__/main_test.snap | 4 +- internal/image/layer.go | 3 + internal/lockfilescalibr/translation.go | 76 +++++++++----------- pkg/osvscanner/osvscanner.go | 1 - 4 files changed, 37 insertions(+), 47 deletions(-) diff --git a/cmd/osv-scanner/__snapshots__/main_test.snap b/cmd/osv-scanner/__snapshots__/main_test.snap index ed223f742a..e270b2c8ba 100755 --- a/cmd/osv-scanner/__snapshots__/main_test.snap +++ b/cmd/osv-scanner/__snapshots__/main_test.snap @@ -2284,7 +2284,7 @@ stat /path/to/my:project/package-lock.json: no such file or directory --- [TestRun_LockfileWithExplicitParseAs/files_that_error_on_parsing_stop_parsable_files_from_being_checked - 2] -(extracting as Cargo.lock) could not extract from /fixtures/locks-insecure/my-package-lock.json: toml: line 1: expected '.' or '=', but got '{' instead +(extracting as rust/Cargolock) could not extract from /fixtures/locks-insecure/my-package-lock.json: toml: line 1: expected '.' or '=', but got '{' instead --- @@ -2342,7 +2342,7 @@ No issues found --- [TestRun_LockfileWithExplicitParseAs/parse-as_takes_priority,_even_if_it's_wrong - 2] -(extracting as package-lock.json) could not extract from "/fixtures/locks-many/yarn.lock": invalid character '#' looking for beginning of value +(extracting as javascript/packagelockjson) could not extract from "/fixtures/locks-many/yarn.lock": invalid character '#' looking for beginning of value --- diff --git a/internal/image/layer.go b/internal/image/layer.go index 949c009a73..539eb949f1 100644 --- a/internal/image/layer.go +++ b/internal/image/layer.go @@ -144,6 +144,9 @@ var _ fs.StatFS = Layer{} var _ fs.ReadDirFS = Layer{} func (filemap Layer) getFileNode(path string) (*FileNode, error) { + // We expect all paths queried to be absolute paths rooted at the container root + // However, scalibr uses paths without a prepending /, because the paths are relative to Root + // Root will always be '/' for container scanning, so prepend with / if necessary. if !filepath.IsAbs(path) { path = filepath.Join("/", path) } diff --git a/internal/lockfilescalibr/translation.go b/internal/lockfilescalibr/translation.go index 6c0597b1ff..f56c754c51 100644 --- a/internal/lockfilescalibr/translation.go +++ b/internal/lockfilescalibr/translation.go @@ -83,20 +83,7 @@ func ExtractWithExtractor(ctx context.Context, localPath string, ext filesystem. return nil, err } - si, err := createScanInput(localPath, info) - if err != nil { - return nil, err - } - inv, err := ext.Extract(ctx, si) - if err != nil { - return nil, fmt.Errorf("(extracting as %s) %w", ext.Name(), err) - } - - for i := range inv { - inv[i].Extractor = ext - } - - return inv, nil + return extractWithExtractor(ctx, localPath, info, ext) } // Extract attempts to extract the file at the given path @@ -120,27 +107,7 @@ func Extract(ctx context.Context, localPath string, extractAs string) ([]*extrac } if extractAs != "" { - for _, ext := range lockfileExtractors { - if lockfileExtractorMapping[extractAs] == ext.Name() { - si, err := createScanInput(localPath, info) - if err != nil { - return nil, err - } - - inv, err := ext.Extract(ctx, si) - if err != nil { - return nil, fmt.Errorf("(extracting as %s) %w", extractAs, err) - } - - for i := range inv { - inv[i].Extractor = ext - } - - return inv, nil - } - } - - return nil, fmt.Errorf("%w, requested %s", ErrExtractorNotFound, extractAs) + return extractAsSpecific(ctx, extractAs, localPath, info) } output := []*extractor.Inventory{} @@ -149,19 +116,12 @@ func Extract(ctx context.Context, localPath string, extractAs string) ([]*extrac for _, ext := range lockfileExtractors { if ext.FileRequired(localPath, info) { extractorFound = true - si, err := createScanInput(localPath, info) - if err != nil { - return nil, err - } - inv, err := ext.Extract(ctx, si) + inv, err := extractWithExtractor(ctx, localPath, info, ext) if err != nil { - return nil, fmt.Errorf("(extracting as %s) %w", ext.Name(), err) + return nil, err } - for i := range inv { - inv[i].Extractor = ext - } output = append(output, inv...) } } @@ -181,6 +141,34 @@ func Extract(ctx context.Context, localPath string, extractAs string) ([]*extrac return output, nil } +// Use the extractor specified by extractAs string key +func extractAsSpecific(ctx context.Context, extractAs string, localPath string, info fs.FileInfo) ([]*extractor.Inventory, error) { + for _, ext := range lockfileExtractors { + if lockfileExtractorMapping[extractAs] == ext.Name() { + return extractWithExtractor(ctx, localPath, info, ext) + } + } + + return nil, fmt.Errorf("%w, requested %s", ErrExtractorNotFound, extractAs) +} + +func extractWithExtractor(ctx context.Context, localPath string, info fs.FileInfo, ext filesystem.Extractor) ([]*extractor.Inventory, error) { + si, err := createScanInput(localPath, info) + if err != nil { + return nil, err + } + + inv, err := ext.Extract(ctx, si) + if err != nil { + return nil, fmt.Errorf("(extracting as %s) %w", ext.Name(), err) + } + + for i := range inv { + inv[i].Extractor = ext + } + return inv, nil +} + func createScanInput(path string, fileInfo fs.FileInfo) (*filesystem.ScanInput, error) { reader, err := os.Open(path) if err != nil { diff --git a/pkg/osvscanner/osvscanner.go b/pkg/osvscanner/osvscanner.go index c9ab42a0c0..468c98bcb0 100644 --- a/pkg/osvscanner/osvscanner.go +++ b/pkg/osvscanner/osvscanner.go @@ -374,7 +374,6 @@ func scanLockfile(r reporter.Reporter, path string, parseAs string, transitiveAc // used by lockfile.Parse to avoid false-positives when scanning projects switch parseAs { case "apk-installed": - // inventories, err := apkinstalled.Extractor{}.Extract(context.Background(), &si) inventories, err = lockfilescalibr.ExtractWithExtractor(context.Background(), path, apk.New(apk.DefaultConfig())) case "dpkg-status": inventories, err = lockfilescalibr.ExtractWithExtractor(context.Background(), path, dpkg.New(dpkg.DefaultConfig()))