From a1f7c52d89ec2e9dbec44534392a062b54bf369b Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Tue, 20 Oct 2026 11:15:17 -0400 Subject: [PATCH] fix: unknowns, better full vs local path handling Signed-off-by: Keith Zantow --- internal/archive/extract.go | 40 +++- internal/archive/extract_test.go | 42 ++-- internal/archive/helpers_test.go | 13 +- internal/archive/resolver.go | 63 ++++-- internal/archive/resolver_test.go | 66 ++++-- internal/archive/traversal.go | 56 ++++- internal/task/archive_tasks.go | 194 ++++++++++++------ internal/task/archive_tasks_test.go | 20 +- internal/task/file_tasks.go | 20 +- internal/task/file_tasks_test.go | 26 +-- internal/task/unknowns_tasks.go | 34 ++- internal/task/unknowns_tasks_test.go | 68 ++++++ internal/unknown/path_error.go | 13 +- syft/archive_cataloger_test.go | 86 ++++++++ syft/create_sbom_config.go | 15 +- syft/file/cataloger/filedigest/cataloger.go | 2 +- syft/file/cataloger/filemetadata/cataloger.go | 2 +- syft/file/coordinates.go | 6 +- syft/file/location_test.go | 2 +- .../common/spdxhelpers/to_format_model.go | 36 ++-- .../common/spdxhelpers/to_syft_model.go | 15 +- .../common/spdxhelpers/to_syft_model_v3.go | 10 +- syft/pkg/cataloger/golang/parse_go_mod.go | 11 +- syft/pkg/cataloger/perl/perllocal.go | 2 +- .../python/parse_wheel_egg_metadata.go | 2 +- 25 files changed, 619 insertions(+), 225 deletions(-) create mode 100644 internal/task/unknowns_tasks_test.go diff --git a/internal/archive/extract.go b/internal/archive/extract.go index 592092d67..fc21834b3 100644 --- a/internal/archive/extract.go +++ b/internal/archive/extract.go @@ -16,10 +16,13 @@ import ( "github.com/anchore/syft/syft/file" ) +// ErrUnsupportedFormat is returned for an archive in a format that is not extracted (see supportedExtractor). +var ErrUnsupportedFormat = errors.New("archive format is not supported") + // Extract opens the archive read from r as its own filesystem, holding its bytes and entries within -// the limiter's bounds. It returns nil when the content is not an archive it can read, and -// ErrDiskLimitReached when the archive's own bytes cannot be placed within the disk limit. The caller -// must Cleanup the result. +// the limiter's bounds. It returns nil when the content is not an archive, ErrUnsupportedFormat when it +// is one that is not extracted, and ErrDiskLimitReached when the archive's own bytes cannot be placed +// within the disk limit. The caller must Cleanup the result. // // archiveCoordinates are those of the archive file itself; every location the resolver returns // extends them with its entry path (see file.Coordinates). @@ -43,9 +46,9 @@ func Extract(ctx context.Context, r io.Reader, archiveCoordinates file.Coordinat // the archive's own bytes are needed only until they are extracted and digested defer release() - format := identifyFormat(ctx, path.Base(archiveCoordinates.LocalPath()), content) - if format == nil { - return nil, nil + format, err := identifyFormat(ctx, path.Base(archiveCoordinates.LocalPath()), content) + if format == nil || err != nil { + return nil, err } if err := resolver.extract(ctx, format, content, exclusions); err != nil { @@ -81,20 +84,35 @@ func (r *Resolver) acquire(content io.Reader) (ReaderAtSeeker, func(), error) { // directory, and reading the tar as a zip would lose the rest. The end of the stream is consulted // only when the head identifies nothing supported, which is how a zip appended to a launcher script (a // Spring Boot executable jar) is found, and how a java resource adapter (a zip named .rar) is read. -func identifyFormat(ctx context.Context, name string, content ReaderAtSeeker) archives.Extractor { +func identifyFormat(ctx context.Context, name string, content ReaderAtSeeker) (archives.Extractor, error) { if _, err := content.Seek(0, io.SeekStart); err != nil { - return nil + return nil, nil } format, _, err := intFile.IdentifyArchive(ctx, name, content) if err == nil { if extractor := supportedExtractor(format); extractor != nil { - return extractor + return extractor, nil } } if HasZipEndOfCentralDirectory(content) { - return archives.Zip{} + return archives.Zip{}, nil } - return nil + if err == nil && holdsFiles(format) { + return nil, fmt.Errorf("%w: %s", ErrUnsupportedFormat, format.MediaType()) + } + return nil, nil +} + +// holdsFiles reports whether the format is an archive of files rather than a bare compression format, +// such as a gzipped man page, which holds one stream and is not cataloged as an archive. +func holdsFiles(format archives.Format) bool { + switch f := format.(type) { + case archives.CompressedArchive: + return f.Extraction != nil + case archives.Extractor: + return true + } + return false } // supportedExtractor admits only the zip and tar families, the tar family optionally compressed. Other diff --git a/internal/archive/extract_test.go b/internal/archive/extract_test.go index 488203c9b..e8f92ef69 100644 --- a/internal/archive/extract_test.go +++ b/internal/archive/extract_test.go @@ -33,27 +33,35 @@ func Test_identifyFormat(t *testing.T) { launcher := append([]byte("#!/bin/bash\nexec java -jar \"$0\" \"$@\"\nexit 0\n"), zipBytes(t, sampleFiles)...) tests := []struct { - name string - data []byte - want archives.Extractor + name string + data []byte + want archives.Extractor + unsupported bool }{ - {"test.zip", zipBytes(t, sampleFiles), archives.Zip{}}, - {"test.tar.gz", tarGzBytes(t, sampleFiles), archives.CompressedArchive{}}, - {"plain.txt", []byte("just text"), nil}, + {"test.zip", zipBytes(t, sampleFiles), archives.Zip{}, false}, + {"test.tar.gz", tarGzBytes(t, sampleFiles), archives.CompressedArchive{}, false}, + {"plain.txt", []byte("just text"), nil, false}, // identified by mholt, but not opened until their decoders are deliberately supported - {"test.7z", []byte("7z\xbc\xaf\x27\x1c\x00\x04"), nil}, - {"test.rar", []byte("Rar!\x1a\x07\x01\x00"), nil}, + {"test.7z", []byte("7z\xbc\xaf\x27\x1c\x00\x04"), nil, true}, + {"test.rar", []byte("Rar!\x1a\x07\x01\x00"), nil, true}, + // a bare compression format holds one stream, not files + {"page.1.gz", gzBytes(t, "a man page"), nil, false}, // a java resource adapter archive is a zip that shares rar's extension - {"adapter.rar", zipBytes(t, sampleFiles), archives.Zip{}}, + {"adapter.rar", zipBytes(t, sampleFiles), archives.Zip{}, false}, // the name can be claimed by identification, so the probe alone must not admit prose - {"app.jar", []byte("plain text, no central directory anywhere in it"), nil}, - {"bundle", []byte("plain text, no central directory anywhere in it"), nil}, + {"app.jar", []byte("plain text, no central directory anywhere in it"), nil, false}, + {"bundle", []byte("plain text, no central directory anywhere in it"), nil, false}, // content sniffing types this as a shell script; only its tail says zip - {"app.jar", launcher, archives.Zip{}}, + {"app.jar", launcher, archives.Zip{}, false}, } for _, tt := range tests { t.Run(fmt.Sprintf("%s/%d bytes", tt.name, len(tt.data)), func(t *testing.T) { - got := identifyFormat(ctx, tt.name, bytes.NewReader(tt.data)) + got, err := identifyFormat(ctx, tt.name, bytes.NewReader(tt.data)) + if tt.unsupported { + assert.ErrorIs(t, err, ErrUnsupportedFormat) + } else { + assert.NoError(t, err) + } if tt.want == nil { assert.Nil(t, got) return @@ -227,7 +235,9 @@ func TestResolver_extract_excludedEntriesAreNeverStored(t *testing.T) { r, _ := resolverIn(t, limiter.charge()) content := bytes.NewReader(data) - err := r.extract(context.Background(), identifyFormat(context.Background(), "test.zip", content), content, + format, err := identifyFormat(context.Background(), "test.zip", content) + require.NoError(t, err) + err = r.extract(context.Background(), format, content, NewExclusions([]string{"**/vendor", "**/*.rpm"})) require.NoError(t, err) require.NoError(t, r.Truncated) @@ -314,7 +324,7 @@ func TestExtract_aTruncatedStreamKeepsWhatWasRead(t *testing.T) { assert.Error(t, extracted.Truncated) locations, err := extracted.FilesByGlob("**") require.NoError(t, err) - assert.Equal(t, []string{"a.txt"}, localPaths(locations)) + assert.Equal(t, []string{"/a.txt"}, localPaths(locations)) } func TestExtract_aPanicStillReleasesEverything(t *testing.T) { @@ -536,7 +546,7 @@ func TestExtract_locationsKeepTheTopLevelRealPath(t *testing.T) { require.NoError(t, err) require.Len(t, locations, 1) assert.Equal(t, file.Coordinates{RealPath: "/app.zip", FullPath: "/app.zip!/dir/hello.txt"}, locations[0].Coordinates) - assert.Equal(t, "dir/hello.txt", locations[0].LocalPath()) + assert.Equal(t, "/dir/hello.txt", locations[0].LocalPath()) for _, loc := range []file.Location{ locations[0], diff --git a/internal/archive/helpers_test.go b/internal/archive/helpers_test.go index 387bc2869..bd37a94f6 100644 --- a/internal/archive/helpers_test.go +++ b/internal/archive/helpers_test.go @@ -109,6 +109,16 @@ func zipFromEntries(t testing.TB, entries []testEntry) []byte { return buf.Bytes() } +func gzBytes(t testing.TB, body string) []byte { + t.Helper() + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + _, err := gw.Write([]byte(body)) + require.NoError(t, err) + require.NoError(t, gw.Close()) + return buf.Bytes() +} + func tarGzBytes(t testing.TB, files map[string]string) []byte { return tarGzFromEntries(t, fileEntries(files)) } @@ -216,7 +226,8 @@ func extractBytes(t testing.TB, data []byte, name string, charge *charge) (*Reso t.Helper() r, root := resolverIn(t, charge) content := bytes.NewReader(data) - format := identifyFormat(context.Background(), name, content) + format, err := identifyFormat(context.Background(), name, content) + require.NoError(t, err) require.NotNil(t, format, "fixture must identify as an archive") require.NoError(t, r.extract(context.Background(), format, content, nil)) return r, root diff --git a/internal/archive/resolver.go b/internal/archive/resolver.go index 81a93951b..743c6eb1e 100644 --- a/internal/archive/resolver.go +++ b/internal/archive/resolver.go @@ -9,6 +9,7 @@ import ( "io/fs" "os" "path" + "slices" "sort" "strings" "time" @@ -234,19 +235,39 @@ func entryPath(rooted string) string { // fullPath is how paths leave the resolver: the archive's own full path, then the entry path. func (r *Resolver) fullPath(rooted string) string { - return r.archiveCoordinates.FullNestedPath() + "!/" + entryPath(rooted) + return r.archiveCoordinates.FullNestedPath() + file.ArchivePathSeparator + entryPath(rooted) } // rootedPath is the tree path a query names, which is either form a path takes: a full path is trimmed to // the entry path within this archive, and anything else (a LocalPath) is taken as archive-relative // already. The archive's full path with a trailing "!" (what path.Dir gives for an entry at the root) -// names the root. -func (r *Resolver) rootedPath(userPath string) string { - archivePath := r.archiveCoordinates.FullNestedPath() + "!" - if userPath == archivePath { - return "/" +// names the root. A full path into another archive in the scan names nothing here and reports false. +func (r *Resolver) rootedPath(userPath string) (string, bool) { + if rest, ok := strings.CutPrefix(userPath, r.archiveCoordinates.FullNestedPath()); ok { + if rest == file.ArchivePathSeparator[:1] { + return "/", true + } + if strings.HasPrefix(rest, file.ArchivePathSeparator) { + // keep the separator's slash as the rooted path's + return path.Clean(rest[len(file.ArchivePathSeparator)-1:]), true + } } - return path.Clean("/" + strings.TrimPrefix(userPath, archivePath+"/")) + if r.outside(userPath) { + return "", false + } + if !strings.HasPrefix(userPath, "/") { + userPath = "/" + userPath + } + return path.Clean(userPath), true +} + +// outside reports whether a path not under this archive's full path is a full path from the scan +// nonetheless: every full path in this archive tree starts with the outermost archive's real path and a +// separator. The real path alone is taken as an entry path, so an entry named like the outermost archive +// is found by its LocalPath. +func (r *Resolver) outside(userPath string) bool { + rest, ok := strings.CutPrefix(userPath, r.archiveCoordinates.RealPath) + return ok && strings.HasPrefix(rest, file.ArchivePathSeparator) } // coordinates name an entry by the outermost archive's real path, which never includes a path inside an @@ -272,7 +293,11 @@ func (r *Resolver) location(res stereoscopeFile.Resolution) file.Location { // resolve finds the file a path names, following links, and reports false for directories and // paths the archive never listed. func (r *Resolver) resolve(userPath string) (*stereoscopeFile.Resolution, bool) { - res, err := r.search.SearchByPath(r.rootedPath(userPath), filetree.FollowBasenameLinks) + rooted, ok := r.rootedPath(userPath) + if !ok { + return nil, false + } + res, err := r.search.SearchByPath(rooted, filetree.FollowBasenameLinks) if err != nil || res == nil || !res.HasReference() { return nil, false } @@ -292,7 +317,10 @@ func (r *Resolver) FileContentsByLocation(location file.Location) (io.ReadCloser } func (r *Resolver) FileMetadataByLocation(location file.Location) (file.Metadata, error) { - rooted := r.rootedPath(location.FullNestedPath()) + rooted, ok := r.rootedPath(location.FullNestedPath()) + if !ok { + return file.Metadata{}, fmt.Errorf("no archive entry for path=%q", location.FullNestedPath()) + } exists, res, err := r.tree.File(stereoscopeFile.Path(rooted)) if err != nil || !exists || !res.HasReference() { return file.Metadata{}, fmt.Errorf("no archive entry for path=%q", location.FullNestedPath()) @@ -311,18 +339,19 @@ func (r *Resolver) FileMetadataByLocation(location file.Location) (file.Metadata } func (r *Resolver) HasPath(userPath string) bool { - return r.tree.HasPath(stereoscopeFile.Path(r.rootedPath(userPath))) + rooted, ok := r.rootedPath(userPath) + return ok && r.tree.HasPath(stereoscopeFile.Path(rooted)) } func (r *Resolver) FilesByPath(paths ...string) ([]file.Location, error) { var out []file.Location - seen := stereoscopeFile.NewFileReferenceSet() + var seen []stereoscopeFile.ID // few paths are asked for at once for _, userPath := range paths { res, ok := r.resolve(userPath) - if !ok || seen.Contains(*res.Reference) { + if !ok || slices.Contains(seen, res.ID()) { continue } - seen.Add(*res.Reference) + seen = append(seen, res.ID()) out = append(out, r.location(*res)) } return out, nil @@ -332,7 +361,11 @@ func (r *Resolver) FilesByGlob(patterns ...string) ([]file.Location, error) { var out []file.Location seen := stereoscopeFile.NewFileReferenceSet() for _, pattern := range patterns { - results, err := r.searchByGlob(r.rootedPath(pattern)) + rooted, ok := r.rootedPath(pattern) + if !ok { + continue + } + results, err := r.searchByGlob(rooted) if err != nil { return nil, fmt.Errorf("invalid glob %q: %w", pattern, err) } @@ -387,7 +420,7 @@ func (r *Resolver) FilesByMIMEType(types ...string) ([]file.Location, error) { return out, nil } -// RelativeFileByPath resolves against the archive root, the only frame an archive has. +// RelativeFileByPath resolves against the archive root: an archive has no layers to be relative to. func (r *Resolver) RelativeFileByPath(_ file.Location, userPath string) *file.Location { locations, _ := r.FilesByPath(userPath) if len(locations) == 0 { diff --git a/internal/archive/resolver_test.go b/internal/archive/resolver_test.go index 70dd24fab..33299df36 100644 --- a/internal/archive/resolver_test.go +++ b/internal/archive/resolver_test.go @@ -113,6 +113,34 @@ func TestResolver_pathsAndContents(t *testing.T) { assert.Equal(t, "Manifest-Version: 1.0\n", string(body)) } +func TestResolver_fullPathsOutsideTheArchiveNameNothing(t *testing.T) { + r, _ := resolverIn(t, memCharge(1<<20)) + r.archiveCoordinates = file.Coordinates{RealPath: "/app.war", FullPath: "/app.war!/WEB-INF/lib/dep.jar"} + addAll(t, r, fileEntries(map[string]string{"/app.war": "same name", "/META-INF/MANIFEST.MF": "m"})) + + locations, err := r.FilesByPath("/app.war!/WEB-INF/lib/dep.jar!/META-INF/MANIFEST.MF") + require.NoError(t, err) + assert.Len(t, locations, 1, "a full path in this archive") + + for _, p := range []string{"/app.war!/WEB-INF/lib/dep.jar", "/app.war!/WEB-INF/lib/other.jar!/META-INF/MANIFEST.MF"} { + locations, err = r.FilesByPath(p) + require.NoError(t, err) + assert.Empty(t, locations, "%s is outside the archive", p) + assert.False(t, r.HasPath(p), p) + } + + for _, p := range []string{"app.war", "/app.war"} { + locations, err = r.FilesByPath(p) + require.NoError(t, err) + require.Len(t, locations, 1, "%s is an entry path named like the outermost archive", p) + assert.Equal(t, "/app.war!/WEB-INF/lib/dep.jar!/app.war", locations[0].FullPath, p) + } + + again, err := r.FilesByPath(locations[0].LocalPath()) + require.NoError(t, err) + assert.Equal(t, locations, again, "a LocalPath finds its own entry") +} + func TestResolver_mimeTypesAreSniffedFromContent(t *testing.T) { r := resolverOver(t, 1<<20, map[string]string{ "notes.txt": "just some text", @@ -121,7 +149,7 @@ func TestResolver_mimeTypesAreSniffedFromContent(t *testing.T) { locations, err := r.FilesByMIMEType("text/x-shellscript") require.NoError(t, err) - assert.Equal(t, []string{"run.sh"}, localPaths(locations)) + assert.Equal(t, []string{"/run.sh"}, localPaths(locations)) } func TestResolver_answersAreOrdered(t *testing.T) { @@ -187,11 +215,11 @@ func TestResolver_globsForADirectorysChildren(t *testing.T) { "META-INF/maven/g/a/pom.xml": "deeper", }) - assert.Equal(t, []string{"LICENSE"}, globPaths(t, r, "/*")) - assert.Equal(t, []string{"LICENSE"}, globPaths(t, r, "*")) - assert.Equal(t, []string{"META-INF/LICENSE.txt"}, globPaths(t, r, "/META-INF/*")) - assert.Equal(t, []string{"META-INF/LICENSE.txt"}, globPaths(t, r, "META-INF/*")) - assert.Equal(t, []string{"META-INF/maven/g/a/pom.xml"}, globPaths(t, r, "**/a/*")) + assert.Equal(t, []string{"/LICENSE"}, globPaths(t, r, "/*")) + assert.Equal(t, []string{"/LICENSE"}, globPaths(t, r, "*")) + assert.Equal(t, []string{"/META-INF/LICENSE.txt"}, globPaths(t, r, "/META-INF/*")) + assert.Equal(t, []string{"/META-INF/LICENSE.txt"}, globPaths(t, r, "META-INF/*")) + assert.Equal(t, []string{"/META-INF/maven/g/a/pom.xml"}, globPaths(t, r, "**/a/*")) } func TestResolver_invalidGlobIsAnError(t *testing.T) { @@ -226,7 +254,7 @@ func TestResolver_linksCollapseToOnePath(t *testing.T) { {name: "opt/hard.jar", link: "opt/real.jar", hard: true}, }, pattern: "**/*.jar", - want: []string{"opt/real.jar"}, + want: []string{"/opt/real.jar"}, reason: "one file, so one location - and it is named by the path that holds it", }, { @@ -238,7 +266,7 @@ func TestResolver_linksCollapseToOnePath(t *testing.T) { {name: "opt/m-sym.jar", link: "real.bin"}, }, pattern: "**/*.jar", - want: []string{"opt/real.bin"}, + want: []string{"/opt/real.bin"}, reason: "the content is the target's, so that is the real path the answer carries", }, { @@ -249,7 +277,7 @@ func TestResolver_linksCollapseToOnePath(t *testing.T) { {name: "opt/one-sym.jar", link: "one.jar"}, }, pattern: "**/*.jar", - want: []string{"opt/one.jar", "opt/two.jar"}, + want: []string{"/opt/one.jar", "/opt/two.jar"}, reason: "collapsing is per file, not per pattern", }, { @@ -259,7 +287,7 @@ func TestResolver_linksCollapseToOnePath(t *testing.T) { {name: "opt/gone.jar", link: "missing.jar"}, }, pattern: "**/*.jar", - want: []string{"opt/real.jar"}, + want: []string{"/opt/real.jar"}, reason: "it names no content", }, { @@ -280,7 +308,7 @@ func TestResolver_linksCollapseToOnePath(t *testing.T) { {name: "opt/outer.jar", link: "mid.jar"}, }, pattern: "**/*.jar", - want: []string{"opt/real.jar"}, + want: []string{"/opt/real.jar"}, reason: "following one hop at a time reaches the same file from every path", }, { @@ -300,7 +328,7 @@ func TestResolver_linksCollapseToOnePath(t *testing.T) { {name: "opt/nested/hard.jar", link: "opt/real.jar", hard: true}, }, pattern: "**/*.jar", - want: []string{"opt/real.jar"}, + want: []string{"/opt/real.jar"}, reason: "read as relative it would resolve to opt/nested/opt/real.jar and dangle", }, } @@ -324,7 +352,7 @@ func TestResolver_collapsedLocationReadsTheTarget(t *testing.T) { require.NoError(t, err) require.Len(t, locations, 1) - assert.Equal(t, "opt/real.bin", locations[0].LocalPath(), "content comes from the file") + assert.Equal(t, "/opt/real.bin", locations[0].LocalPath(), "content comes from the file") assert.Equal(t, "outer.jar!/opt/a-sym.jar", locations[0].AccessPath, "the path searched by is the one matched") reader, err := r.FileContentsByLocation(locations[0]) @@ -367,7 +395,7 @@ func TestResolver_danglingLinkHasNoContent(t *testing.T) { locations = append(locations, l) } require.Len(t, locations, 1) - assert.Equal(t, "opt/gone.jar", locations[0].LocalPath()) + assert.Equal(t, "/opt/gone.jar", locations[0].LocalPath()) _, err := r.FileContentsByLocation(locations[0]) assert.Error(t, err, "a link to nothing has no content to read") @@ -394,12 +422,12 @@ func TestResolver_mimeTypeAnswersOnlyWithFiles(t *testing.T) { return localPaths(locations) } - assert.Equal(t, []string{"opt/real.jar"}, byMIME("application/zip"), + assert.Equal(t, []string{"/opt/real.jar"}, byMIME("application/zip"), "the file answers, not the two links to it") - assert.Equal(t, []string{"opt/notes.txt"}, byMIME("text/plain")) + assert.Equal(t, []string{"/opt/notes.txt"}, byMIME("text/plain")) assert.Empty(t, byMIME(""), "a link sniffs to no type, and must not be reachable by asking for that") - assert.Equal(t, []string{"opt/notes.txt", "opt/real.jar"}, byMIME("application/zip", "text/plain")) + assert.Equal(t, []string{"/opt/notes.txt", "/opt/real.jar"}, byMIME("application/zip", "text/plain")) } // AllLocations enumerates what the archive holds rather than searching for files: the file metadata @@ -415,7 +443,7 @@ func TestResolver_allLocationsKeepsEveryPath(t *testing.T) { for l := range r.AllLocations(context.Background()) { got = append(got, l.LocalPath()) } - assert.Equal(t, []string{"opt/gone.jar", "opt/real.jar", "opt/sym.jar"}, got, + assert.Equal(t, []string{"/opt/gone.jar", "/opt/real.jar", "/opt/sym.jar"}, got, "every path the archive names, links included") } @@ -437,7 +465,7 @@ func TestResolver_add_anEntryOverTheCapIsSkippedAndReadNoFurther(t *testing.T) { assert.ErrorContains(t, r.Truncated, "per-entry size cap of 1.0 KiB") locations, err := r.FilesByGlob("**") require.NoError(t, err) - assert.Equal(t, []string{"ok.txt"}, localPaths(locations)) + assert.Equal(t, []string{"/ok.txt"}, localPaths(locations)) } type countingReader struct { diff --git a/internal/archive/traversal.go b/internal/archive/traversal.go index 87e61b62f..de8b0da62 100644 --- a/internal/archive/traversal.go +++ b/internal/archive/traversal.go @@ -2,6 +2,8 @@ package archive import ( "context" + "path" + "slices" "strings" "github.com/anchore/syft/syft/file" @@ -30,6 +32,52 @@ type Traversal struct { Packages []pkg.Package } +// FullPathIn is the full path of the entry at entryPath inside the archive whose full path is +// containerPath, or entryPath itself for a blank containerPath (the scan root). +func FullPathIn(containerPath, entryPath string) string { + if containerPath == "" { + return entryPath + } + if !strings.HasPrefix(entryPath, "/") { + entryPath = "/" + entryPath + } + // cleaning an already clean path allocates nothing + return containerPath + file.ArchivePathSeparator + path.Clean(entryPath)[1:] +} + +// OwnerContainers are the containers a package owns files within: the container of each of its +// locations, each once, as coordinates whose FullPath is the container's full path (blank at the scan +// root). A package without locations owns files at the scan root. See OwnedCoordinates. +func OwnerContainers(p pkg.Package) []file.Coordinates { + var out []file.Coordinates + for _, l := range p.Locations.ToUnorderedSlice() { + c := file.Coordinates{FileSystemID: l.FileSystemID} + if container := ContainerPath(l.Coordinates); container != "" { + c.RealPath, c.FullPath = l.RealPath, container + } + if !slices.Contains(out, c) { + out = append(out, c) + } + } + if len(out) == 0 { + out = append(out, file.Coordinates{}) + } + return out +} + +// OwnedCoordinates are the coordinates of the file at ownedPath within container, one of +// OwnerContainers. At the scan root the path may still need resolving (it may pass through links). +func OwnedCoordinates(container file.Coordinates, ownedPath string) file.Coordinates { + if container.FullPath == "" { + return file.Coordinates{RealPath: ownedPath, FileSystemID: container.FileSystemID} + } + return file.Coordinates{ + RealPath: container.RealPath, + FileSystemID: container.FileSystemID, + FullPath: FullPathIn(container.FullPath, ownedPath), + } +} + // FullTraversalArchivePath is the archive's own full path, which is the ContainerPath of every file inside it. Outside // any archive (a nil Traversal) it is blank, as for files on the scanned filesystem. func (t *Traversal) FullTraversalArchivePath() string { @@ -75,7 +123,10 @@ func VirtualPath(loc file.Location) string { return accessPath } segments := strings.Split(entries, file.ArchivePathSeparator) - segments[len(segments)-1] = strings.TrimPrefix(accessPath, ContainerPath(loc.Coordinates)+file.ArchivePathSeparator) + if rest, ok := strings.CutPrefix(accessPath, ContainerPath(loc.Coordinates)); ok && strings.HasPrefix(rest, file.ArchivePathSeparator) { + accessPath = rest[len(file.ArchivePathSeparator):] + } + segments[len(segments)-1] = accessPath for i, s := range segments { segments[i] = strings.TrimPrefix(escapeEntryPath(s), "/") } @@ -85,6 +136,9 @@ func VirtualPath(loc file.Location) string { var entryPathEscaper = strings.NewReplacer("%", "%25", ":", "%3A") func escapeEntryPath(p string) string { + if !strings.ContainsAny(p, "%:") { + return p + } return entryPathEscaper.Replace(p) } diff --git a/internal/task/archive_tasks.go b/internal/task/archive_tasks.go index fa2e1e430..7ae84f1aa 100644 --- a/internal/task/archive_tasks.go +++ b/internal/task/archive_tasks.go @@ -30,27 +30,33 @@ import ( const ArchiveCatalogerTaskName = "archive-cataloger" -// NewArchiveCatalogerTask returns a task that extracts every archive it finds, runs subPipeline -// against the archive's contents as a standalone filesystem, and records CONTAINS relationships -// from the archive to what was found inside it. It recurses into the archives it extracts up to -// cfg.MaxDepth; the task drops itself from subPipeline so that is the only recursion. +// NewArchiveCatalogerTask returns a task that extracts every archive it finds, runs the same tasks +// against the archive's contents as a standalone filesystem, and records +// CONTAINS relationships from the archive to what was found inside it. It recurses into the archives +// it extracts up to cfg.MaxDepth; the task drops itself from the tasks so that is the only recursion. +// +// Each archive is handled as the scan root is: pkgTasks, then the archives nested in it, then fileTasks, +// which see the packages found in the nested archives (see Traversal.PackageFiles). +// +// The task is the authority on which archives were not cataloged and why; with +// unknowns.IncludeUnexpandedArchives it also reports archives in formats it does not extract. // // exclusions are the scan's exclusion patterns, applied inside each archive. Returns nil when archive // cataloging is disabled (MaxDepth == 0; negative means as deep as unboundedArchiveDepth). -func NewArchiveCatalogerTask(cfg cataloging.ArchiveSearchConfig, subPipeline []Task, exclusions []string) Task { - return newArchiveCatalogerTask(cfg.MaxDepth, archiveLimits(cfg), subPipeline, exclusions) +func NewArchiveCatalogerTask(cfg cataloging.ArchiveSearchConfig, unknowns cataloging.UnknownsConfig, pkgTasks, fileTasks []Task, exclusions []string) Task { + return newArchiveCatalogerTask(cfg.MaxDepth, archiveLimits(cfg), unknowns.IncludeUnexpandedArchives, pkgTasks, fileTasks, exclusions) } func archiveLimits(cfg cataloging.ArchiveSearchConfig) archive.Limits { return archive.Limits{MaxMemoryBytes: cfg.MaxMemoryBytes, MaxDiskBytes: cfg.MaxDiskBytes} } -func newArchiveCatalogerTask(maxDepth int, limits archive.Limits, subPipeline []Task, exclusions []string) Task { +func newArchiveCatalogerTask(maxDepth int, limits archive.Limits, reportUnsupportedUnknowns bool, pkgTasks, fileTasks []Task, exclusions []string) Task { if maxDepth == 0 { return nil } - subPipeline = withoutArchiveCataloger(subPipeline) - if len(subPipeline) == 0 { + pkgTasks, fileTasks = withoutArchiveCataloger(pkgTasks), withoutArchiveCataloger(fileTasks) + if len(pkgTasks)+len(fileTasks) == 0 { return nil } @@ -59,11 +65,13 @@ func newArchiveCatalogerTask(maxDepth int, limits archive.Limits, subPipeline [] ctx = bus.WithCatalogerTaskRegistry(ctx) c := &archiveCataloger{ - maxDepth: maxDepth, - subPipeline: subPipeline, - limiter: archive.NewLimiter(limits), - exclusions: archive.NewExclusions(exclusions), - progress: bus.StartCatalogerTask(ctx, archiveCatalogerProgressInfo(), -1, ""), + maxDepth: maxDepth, + reportUnsupportedUnknowns: reportUnsupportedUnknowns, + pkgTasks: pkgTasks, + fileTasks: fileTasks, + limiter: archive.NewLimiter(limits), + exclusions: archive.NewExclusions(exclusions), + progress: bus.StartCatalogerTask(ctx, archiveCatalogerProgressInfo(), -1, ""), } err := c.catalog(ctx, resolver, 0, builder) c.progress.AtomicStage.Set(fmt.Sprintf("%s archives", humanize.Comma(c.progress.Current()))) @@ -96,10 +104,12 @@ func withoutArchiveCataloger(tasks []Task) []Task { } type archiveCataloger struct { - maxDepth int - subPipeline []Task - limiter *archive.Limiter - exclusions archive.Exclusions + maxDepth int + reportUnsupportedUnknowns bool + pkgTasks []Task + fileTasks []Task + limiter *archive.Limiter + exclusions archive.Exclusions // progress is the one row for the whole walk: its count is how many archives have been entered progress *monitor.TaskProgress @@ -164,6 +174,9 @@ func (c *archiveCataloger) catalogCandidates(ctx context.Context, resolver file. return errs } +// processArchive processes the given candidate archive, catching panics +// +//nolint:funlen func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver file.Resolver, candidate archiveCandidate, depth int, builder sbomsync.Builder) (err error) { // the decoders run on untrusted bytes; a panic in one costs this archive, not the scan defer func() { @@ -200,6 +213,12 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi traversal := &archive.Traversal{Location: location, Parent: archive.TraversalFromContext(ctx)} archivePath := location.FullNestedPath() extracted, err := archive.Extract(ctx, archiveContent, location.Coordinates, c.limiter, c.exclusions) + if errors.Is(err, archive.ErrUnsupportedFormat) { + if c.reportUnsupportedUnknowns { + recordArchiveUnknown(builder, location.Coordinates, "archive not cataloged: "+err.Error()) + } + return nil + } if errors.Is(err, archive.ErrDiskLimitReached) { // nothing is released while this archive waits, so skip it rather than block the scan recordArchiveUnknown(builder, location.Coordinates, "archive skipped: its content would exceed the disk limit") @@ -233,18 +252,33 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi ctx = archive.WithTraversal(ctx, traversal) c.progress.Increment() - scratch := c.runSubPipeline(ctx, extracted, location.Coordinates) - traversal.Packages = packagesAt(scratch, location.Coordinates) - children := c.discoverArchives(extracted) - mergeArchiveResults(location.Coordinates, archivePath, children, scratch, builder) - c.progress.AtomicStage.Set(fmt.Sprintf("%s archives (%s)", humanize.Comma(c.progress.Current()), archivePath)) - // self time stops before descending, so a containing archive is not charged for what it contains - if took := time.Since(started); took > c.slowest { + // everything found in this archive and the archives nested in it, merged into the parent's SBOM once complete + found := &sbom.SBOM{Artifacts: sbom.Artifacts{Packages: pkg.NewCollection()}} + foundBuilder := sbomsync.NewBuilder(found) + runArchiveTasks(ctx, c.pkgTasks, extracted, location.Coordinates, foundBuilder) + direct := found.Artifacts.Packages.Sorted() + traversal.Packages = packagesAt(direct, location.Coordinates) + + // self time excludes descending, so a containing archive is not charged for what it contains + selfTime := time.Since(started) + + // as at the scan root, nested archives are cataloged before this archive's files, so the packages + // describing them (located in this archive) select their files here + children := c.discoverArchives(extracted) + errs := c.catalogCandidates(ctx, extracted, children, depth+1, foundBuilder) + + started = time.Now() + runArchiveTasks(ctx, c.fileTasks, extracted, location.Coordinates, foundBuilder) + addContains(location.Coordinates, archivePath, direct, children, found) + mergeArchiveResults(found, builder) + c.progress.AtomicStage.Set(humanize.Comma(c.progress.Current()) + " archives (" + archivePath + ")") + + if took := selfTime + time.Since(started); took > c.slowest { c.slowest, c.slowestPath = took, archivePath } - return c.catalogCandidates(ctx, extracted, children, depth+1, builder) + return errs } // a top-level archive and everything nested in it may decompress to archiveTreeRatio times its size, or @@ -273,30 +307,37 @@ func (c *archiveCataloger) isAncestor(digests []file.Digest) bool { } // packagesAt returns the packages located at the given coordinates, i.e. those describing the file there. -func packagesAt(s *sbom.SBOM, coordinates file.Coordinates) []pkg.Package { +func packagesAt(pkgs []pkg.Package, coordinates file.Coordinates) []pkg.Package { var out []pkg.Package - for _, p := range s.Artifacts.Packages.Sorted() { - if p.Locations.CoordinateSet().Contains(coordinates) { + for _, p := range pkgs { + if locatedAt(p, coordinates) { out = append(out, p) } } return out } +// locatedAt reports whether one of p's locations is at the given coordinates. +func locatedAt(p pkg.Package, coordinates file.Coordinates) bool { + for _, l := range p.Locations.ToUnorderedSlice() { + if l.Coordinates == coordinates { + return true + } + } + return false +} + func recordArchiveUnknown(builder sbomsync.Builder, coordinates file.Coordinates, reason string) { appendUnknowns(builder, ArchiveCatalogerTaskName, []unknown.CoordinateError{{Coordinates: coordinates, Reason: errors.New(reason)}}) } -// runSubPipeline runs every task against the resolver into a throwaway SBOM, so results can be -// attributed to the containing archive before merging into the shared SBOM. +// runArchiveTasks runs each task against the archive's contents. // // Failures are recorded as RunTask records them at the top level: errors carrying coordinates keep // them, anything else is attributed to the archive. A failure never fails the archive or the scan. -func (c *archiveCataloger) runSubPipeline(ctx context.Context, resolver file.Resolver, archiveCoordinates file.Coordinates) *sbom.SBOM { - scratch := &sbom.SBOM{Artifacts: sbom.Artifacts{Packages: pkg.NewCollection()}} - scratchBuilder := sbomsync.NewBuilder(scratch) - for _, t := range c.subPipeline { - err := runSubTaskSafely(ctx, t, resolver, scratchBuilder) +func runArchiveTasks(ctx context.Context, tasks []Task, resolver file.Resolver, archiveCoordinates file.Coordinates, builder sbomsync.Builder) { + for _, t := range tasks { + err := runSubTaskSafely(ctx, t, resolver, builder) if err == nil { continue } @@ -306,9 +347,8 @@ func (c *archiveCataloger) runSubPipeline(ctx context.Context, resolver file.Res if remaining != nil { unknowns = append(unknowns, unknown.CoordinateError{Coordinates: archiveCoordinates, Reason: remaining}) } - appendUnknowns(scratchBuilder, t.Name(), unknowns) + appendUnknowns(builder, t.Name(), unknowns) } - return scratch } // runSubTaskSafely is runTaskSafely without the task name in a panic's error, since appendUnknowns @@ -345,10 +385,12 @@ var zipArchiveGlobs = []string{ "**/*.egg", "**/*.whl", "**/*.lpkg", "**/*.zap", "**/*.exe", } +var archiveMIMETypes = slices.Sorted(slices.Values(mimetype.ArchiveMIMETypeSet.List())) + // discoverArchives returns every file worth opening: those whose content is an archive, plus those // named like a zip whose tail may still hold one. func (c *archiveCataloger) discoverArchives(resolver file.Resolver) []archiveCandidate { - archives, err := resolver.FilesByMIMEType(mimetype.ArchiveMIMETypeSet.List()...) + archives, err := resolver.FilesByMIMEType(archiveMIMETypes...) if err != nil { log.WithFields("error", err).Debug("unable to list archive files for recursive cataloging") return nil @@ -381,44 +423,78 @@ func (c *archiveCataloger) discoverArchives(resolver file.Resolver) []archiveCan return candidates } -// mergeArchiveResults copies the throwaway SBOM's packages, files and relationships into the shared -// SBOM, adding a CONTAINS relationship from the archive to everything found inside it. -// -// Files inside the archive are those whose container is the archive's own full path, which leaves out the -// archive itself (a java package or an unknown recorded at its coordinates would otherwise become a -// self-loop). The nested archives are always included, so the chain of archives is navigable whatever -// file selection is configured. -func mergeArchiveResults(archiveCoordinates file.Coordinates, archivePath string, children []archiveCandidate, scratch *sbom.SBOM, builder sbomsync.Builder) { - pkgs := scratch.Artifacts.Packages.Sorted() - if len(pkgs) > 0 { +// mergeArchiveResults copies everything found in an archive into its parent's SBOM. +func mergeArchiveResults(found *sbom.SBOM, builder sbomsync.Builder) { + if pkgs := found.Artifacts.Packages.Sorted(); len(pkgs) > 0 { builder.AddPackages(pkgs...) } - if accessor, ok := builder.(sbomsync.Accessor); ok { accessor.WriteToSBOM(func(s *sbom.SBOM) { - mergeFileArtifacts(&s.Artifacts, &scratch.Artifacts) + mergeFileArtifacts(&s.Artifacts, &found.Artifacts) }) } - - var rels []artifact.Relationship - for _, p := range pkgs { - rels = append(rels, artifact.Relationship{From: archiveCoordinates, To: p, Type: artifact.ContainsRelationship}) + if len(found.Relationships) > 0 { + builder.AddRelationships(found.Relationships...) } +} + +// addContains adds a CONTAINS relationship from the archive to each package found directly in it, each +// file directly in it that the SBOM describes, and each nested archive, so the hierarchy is navigable +// whatever file selection is configured. +// +// Files directly in the archive are those whose container is the archive's own full path, which leaves +// out the archive itself (a java package or an unknown recorded at its coordinates would otherwise +// become a self-loop) and the files of nested archives, contained by those. +func addContains(archiveCoordinates file.Coordinates, archivePath string, direct []pkg.Package, children []archiveCandidate, found *sbom.SBOM) { contained := file.NewCoordinateSet() for _, child := range children { contained.Add(child.location.Coordinates) } - for _, coordinates := range scratch.AllCoordinates() { + addInside := func(coordinates file.Coordinates) { if archive.ContainerPath(coordinates) == archivePath { contained.Add(coordinates) } } + eachFileArtifactCoordinates(&found.Artifacts, addInside) + for _, r := range found.Relationships { + if c, ok := r.From.(file.Coordinates); ok { + addInside(c) + } + if c, ok := r.To.(file.Coordinates); ok { + addInside(c) + } + } + + var rels []artifact.Relationship + for _, p := range direct { + rels = append(rels, artifact.Relationship{From: archiveCoordinates, To: p, Type: artifact.ContainsRelationship}) + } for _, coordinates := range contained.ToSlice() { rels = append(rels, artifact.Relationship{From: archiveCoordinates, To: coordinates, Type: artifact.ContainsRelationship}) } - rels = append(rels, scratch.Relationships...) - if len(rels) > 0 { - builder.AddRelationships(rels...) + found.Relationships = append(found.Relationships, rels...) +} + +// eachFileArtifactCoordinates calls fn with the coordinates of every file artifact, once per artifact +// kind describing it. +func eachFileArtifactCoordinates(a *sbom.Artifacts, fn func(file.Coordinates)) { + for c := range a.FileMetadata { + fn(c) + } + for c := range a.FileDigests { + fn(c) + } + for c := range a.FileContents { + fn(c) + } + for c := range a.FileLicenses { + fn(c) + } + for c := range a.Executables { + fn(c) + } + for c := range a.Unknowns { + fn(c) } } diff --git a/internal/task/archive_tasks_test.go b/internal/task/archive_tasks_test.go index 70beefac5..cbdd9bdce 100644 --- a/internal/task/archive_tasks_test.go +++ b/internal/task/archive_tasks_test.go @@ -104,7 +104,7 @@ func Test_archiveCataloger_aPanicCostsOnlyThatArchive(t *testing.T) { func newTestTask(t *testing.T, cfg cataloging.ArchiveSearchConfig, subPipeline ...Task) Task { t.Helper() - tsk := NewArchiveCatalogerTask(cfg, subPipeline, nil) + tsk := NewArchiveCatalogerTask(cfg, cataloging.UnknownsConfig{}, subPipeline, nil, nil) require.NotNil(t, tsk) return tsk } @@ -112,7 +112,7 @@ func newTestTask(t *testing.T, cfg cataloging.ArchiveSearchConfig, subPipeline . // newLimitedTestTask builds the task with the limiter's own limits, where zero forbids the resource. func newLimitedTestTask(t *testing.T, maxDepth int, limits archive.Limits, subPipeline ...Task) Task { t.Helper() - tsk := newArchiveCatalogerTask(maxDepth, limits, subPipeline, nil) + tsk := newArchiveCatalogerTask(maxDepth, limits, false, subPipeline, nil, nil) require.NotNil(t, tsk) return tsk } @@ -236,7 +236,7 @@ func Test_NewArchiveCatalogerTask_gating(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := NewArchiveCatalogerTask(tt.cfg, tt.subPipeline, nil) + got := NewArchiveCatalogerTask(tt.cfg, cataloging.UnknownsConfig{}, tt.subPipeline, nil, nil) if tt.wantTask { require.NotNil(t, got) assert.Equal(t, ArchiveCatalogerTaskName, got.Name()) @@ -362,7 +362,7 @@ func Test_NewArchiveCatalogerTask_dropsItselfFromSubPipeline(t *testing.T) { var ran int countTask := countingTask(&ran) - poisoned := []Task{countTask, NewArchiveCatalogerTask(cfg, []Task{countTask}, nil)} + poisoned := []Task{countTask, NewArchiveCatalogerTask(cfg, cataloging.UnknownsConfig{}, []Task{countTask}, nil, nil)} require.NotNil(t, poisoned[1]) tsk := newTestTask(t, cfg, poisoned...) @@ -371,8 +371,8 @@ func Test_NewArchiveCatalogerTask_dropsItselfFromSubPipeline(t *testing.T) { assert.Equal(t, 2, ran, "two archives, one sub-pipeline task each") t.Run("a sub-pipeline of nothing but itself yields no task", func(t *testing.T) { - only := []Task{NewArchiveCatalogerTask(cfg, []Task{countTask}, nil)} - assert.Nil(t, NewArchiveCatalogerTask(cfg, only, nil)) + only := []Task{NewArchiveCatalogerTask(cfg, cataloging.UnknownsConfig{}, []Task{countTask}, nil, nil)} + assert.Nil(t, NewArchiveCatalogerTask(cfg, cataloging.UnknownsConfig{}, only, nil, nil)) }) } @@ -585,10 +585,10 @@ func Test_archiveCataloger_limitsFallWhenAnArchiveIsReleased(t *testing.T) { func newTestCataloger(maxDepth int, limiter *archive.Limiter, subPipeline ...Task) *archiveCataloger { return &archiveCataloger{ - maxDepth: maxDepth, - subPipeline: subPipeline, - limiter: limiter, - progress: bus.StartCatalogerTask(context.Background(), archiveCatalogerProgressInfo(), -1, ""), + maxDepth: maxDepth, + pkgTasks: subPipeline, + limiter: limiter, + progress: bus.StartCatalogerTask(context.Background(), archiveCatalogerProgressInfo(), -1, ""), } } diff --git a/internal/task/file_tasks.go b/internal/task/file_tasks.go index 758799cba..e82b0ae1e 100644 --- a/internal/task/file_tasks.go +++ b/internal/task/file_tasks.go @@ -4,7 +4,6 @@ import ( "context" "crypto" - "github.com/anchore/syft/internal/archive" "github.com/anchore/syft/internal/sbomsync" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/cataloging/filecataloging" @@ -40,7 +39,7 @@ func newFileDigestCatalogerTask(selection file.Selection, hashers []crypto.Hash, accessor := builder.(sbomsync.Accessor) - coordinates, ok := coordinatesForSelection(ctx, selection, builder.(sbomsync.Accessor)) + coordinates, ok := coordinatesForSelection(selection, builder.(sbomsync.Accessor)) if !ok { return nil } @@ -71,7 +70,7 @@ func newFileMetadataCatalogerTask(selection file.Selection, tags ...string) Task accessor := builder.(sbomsync.Accessor) - coordinates, ok := coordinatesForSelection(ctx, selection, builder.(sbomsync.Accessor)) + coordinates, ok := coordinatesForSelection(selection, builder.(sbomsync.Accessor)) if !ok { return nil } @@ -142,12 +141,7 @@ func newExecutableCatalogerTask(selection file.Selection, cfg executable.Config, // TODO: this should be replaced with a fix that allows passing a coordinate or location iterator to the cataloger // Today internal to both cataloger this functions differently: a slice of coordinates vs a channel of locations -// -// Only coordinates in the resolver's own frame are returned: inside an archive, the files of that archive, -// and at the scan root, files outside any archive. A package's location can be in another frame (a nested -// java package is located at the archive in its parent), and looking that path up here would miss or, -// worse, hit an unrelated file of the same name. -func coordinatesForSelection(ctx context.Context, selection file.Selection, accessor sbomsync.Accessor) ([]file.Coordinates, bool) { +func coordinatesForSelection(selection file.Selection, accessor sbomsync.Accessor) ([]file.Coordinates, bool) { if selection == file.AllFilesSelection { return nil, true } @@ -175,13 +169,7 @@ func coordinatesForSelection(ctx context.Context, selection file.Selection, acce } }) - frame := archive.TraversalFromContext(ctx).FullTraversalArchivePath() - var coords []file.Coordinates - for _, c := range coordinates.ToSlice() { - if archive.ContainerPath(c) == frame { - coords = append(coords, c) - } - } + coords := coordinates.ToSlice() if len(coords) == 0 { return nil, false diff --git a/internal/task/file_tasks_test.go b/internal/task/file_tasks_test.go index 8e7af9cbb..33e81f2ac 100644 --- a/internal/task/file_tasks_test.go +++ b/internal/task/file_tasks_test.go @@ -1,13 +1,10 @@ package task import ( - "context" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/anchore/syft/internal/archive" "github.com/anchore/syft/internal/sbomsync" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" @@ -87,30 +84,9 @@ func Test_coordinatesForSelection(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - files, ok := coordinatesForSelection(context.Background(), tt.selection, sbomsync.NewBuilder(tt.sbom).(sbomsync.Accessor)) + files, ok := coordinatesForSelection(tt.selection, sbomsync.NewBuilder(tt.sbom).(sbomsync.Accessor)) assert.Equal(t, tt.files, files) assert.Equal(t, tt.ok, ok) }) } } - -func Test_coordinatesForSelection_onlyTheResolversFrame(t *testing.T) { - // a package located at an archive in its parent's frame must not send the file catalogers looking up - // that path inside the archive's own resolver - atRoot := file.NewCoordinates("/app.war", "") - inWar := file.Coordinates{RealPath: "/app.war", FullPath: "/app.war!/WEB-INF/lib/dep.jar"} - s := &sbom.SBOM{Artifacts: sbom.Artifacts{Packages: pkg.NewCollection( - pkg.Package{Name: "app", Locations: file.NewLocationSet(file.NewLocationFromCoordinates(atRoot))}, - pkg.Package{Name: "dep", Locations: file.NewLocationSet(file.NewLocationFromCoordinates(inWar))}, - )}} - accessor := sbomsync.NewBuilder(s).(sbomsync.Accessor) - - got, ok := coordinatesForSelection(context.Background(), file.FilesOwnedByPackageSelection, accessor) - require.True(t, ok) - assert.Equal(t, []file.Coordinates{atRoot}, got, "at the scan root") - - ctx := archive.WithTraversal(context.Background(), &archive.Traversal{Location: file.NewLocationFromCoordinates(atRoot)}) - got, ok = coordinatesForSelection(ctx, file.FilesOwnedByPackageSelection, accessor) - require.True(t, ok) - assert.Equal(t, []file.Coordinates{inWar}, got, "inside app.war") -} diff --git a/internal/task/unknowns_tasks.go b/internal/task/unknowns_tasks.go index 01ecfcefe..b2730a312 100644 --- a/internal/task/unknowns_tasks.go +++ b/internal/task/unknowns_tasks.go @@ -2,8 +2,10 @@ package task import ( "context" + "slices" "strings" + "github.com/anchore/syft/internal/archive" intFile "github.com/anchore/syft/internal/file" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/internal/sbomsync" @@ -36,11 +38,20 @@ func (c unknownsLabelerTask) finalize(resolver file.Resolver, s *sbom.SBOM) { hasPackageReference := coordinateReferenceLookup(resolver, s) if c.RemoveWhenPackagesDefined { - for coords := range s.Artifacts.Unknowns { + for coords, reasons := range s.Artifacts.Unknowns { if !hasPackageReference(coords) { continue } - delete(s.Artifacts.Unknowns, coords) + // what the archive cataloger reports is about the archive's contents (left out, cut short), + // which a package describing the archive does not account for + kept := slices.DeleteFunc(reasons, func(r string) bool { + return !strings.HasPrefix(r, formatUnknown("", ArchiveCatalogerTaskName)) + }) + if len(kept) == 0 { + delete(s.Artifacts.Unknowns, coords) + continue + } + s.Artifacts.Unknowns[coords] = kept } } @@ -71,6 +82,7 @@ func formatUnknown(err string, task ...string) string { return strings.Join(task, "/") + ": " + err } +//nolint:gocognit func coordinateReferenceLookup(resolver file.Resolver, s *sbom.SBOM) func(coords file.Coordinates) bool { allPackageCoords := file.NewCoordinateSet() @@ -83,7 +95,25 @@ func coordinateReferenceLookup(resolver file.Resolver, s *sbom.SBOM) func(coords // relationships for these owned files may be disabled, but we always want to include them for p := range s.Artifacts.Packages.Enumerate() { if f, ok := p.Metadata.(pkg.FileOwner); ok { + // owned paths are within the containers the package was found in + var archives []file.Coordinates + atRoot := false + for _, container := range archive.OwnerContainers(p) { + if container.FullPath == "" { + atRoot = true + } else { + archives = append(archives, container) + } + } for _, ownedFilePath := range f.OwnedFiles() { + for _, container := range archives { + // an archive's contents are not in this resolver to follow links in + allPackageCoords.Add(archive.OwnedCoordinates(container, ownedFilePath)) + } + if !atRoot { + continue + } + // resolve these owned files, as they may have symlinks // but coordinates we will test against are always absolute paths locations, err := resolver.FilesByPath(ownedFilePath) diff --git a/internal/task/unknowns_tasks_test.go b/internal/task/unknowns_tasks_test.go new file mode 100644 index 000000000..2466ba044 --- /dev/null +++ b/internal/task/unknowns_tasks_test.go @@ -0,0 +1,68 @@ +package task + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/anchore/syft/syft/cataloging" + "github.com/anchore/syft/syft/file" + "github.com/anchore/syft/syft/pkg" + "github.com/anchore/syft/syft/sbom" +) + +func Test_unknownsLabeler_archiveReasonsSurvivePackages(t *testing.T) { + jar := file.NewCoordinates("/app.jar", "") + p := pkg.Package{Name: "app", Locations: file.NewLocationSet(file.NewLocationFromCoordinates(jar))} + p.SetID() + s := &sbom.SBOM{Artifacts: sbom.Artifacts{ + Packages: pkg.NewCollection(p), + Unknowns: map[file.Coordinates][]string{jar: { + formatUnknown("archive cataloged from part of its contents: too big", ArchiveCatalogerTaskName), + formatUnknown("unable to read manifest", "java-archive-cataloger"), + }}, + }} + + unknownsLabelerTask{cataloging.UnknownsConfig{RemoveWhenPackagesDefined: true}}.finalize(file.NewMockResolverForPaths(), s) + + assert.Equal(t, []string{formatUnknown("archive cataloged from part of its contents: too big", ArchiveCatalogerTaskName)}, s.Artifacts.Unknowns[jar], + "a package describing the archive explains a parse failure, not contents left out") +} + +func Test_unknownsLabeler_ownedFilesAreWithinTheOwnersContainer(t *testing.T) { + atRoot := file.NewCoordinates("/usr/bin/foo", "") + inArchive := file.Coordinates{RealPath: "/rootfs.tar", FullPath: "/rootfs.tar!/usr/bin/foo"} + owner := func(at file.Coordinates) pkg.Package { + p := pkg.Package{ + Name: "owner", + Type: pkg.RpmPkg, + Locations: file.NewLocationSet(file.NewLocationFromCoordinates(at)), + Metadata: pkg.RpmDBEntry{Files: []pkg.RpmFileRecord{{Path: "/usr/bin/foo"}}}, + } + p.SetID() + return p + } + + tests := []struct { + name string + owner file.Coordinates + referenced file.Coordinates + }{ + {name: "an owner at the scan root", owner: file.NewCoordinates("/var/lib/rpm/Packages", ""), referenced: atRoot}, + {name: "an owner inside an archive", owner: file.Coordinates{RealPath: "/rootfs.tar", FullPath: "/rootfs.tar!/var/lib/rpm/Packages"}, referenced: inArchive}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &sbom.SBOM{Artifacts: sbom.Artifacts{ + Packages: pkg.NewCollection(owner(tt.owner)), + Executables: map[file.Coordinates]file.Executable{atRoot: {}, inArchive: {}}, + }} + + unknownsLabelerTask{cataloging.UnknownsConfig{IncludeExecutablesWithoutPackages: true}}.finalize(file.NewMockResolverForPaths("/usr/bin/foo"), s) + + for _, exe := range []file.Coordinates{atRoot, inArchive} { + assert.Equal(t, exe != tt.referenced, len(s.Artifacts.Unknowns[exe]) > 0, exe.FullNestedPath()) + } + }) + } +} diff --git a/internal/unknown/path_error.go b/internal/unknown/path_error.go index dce059563..1c74193dc 100644 --- a/internal/unknown/path_error.go +++ b/internal/unknown/path_error.go @@ -2,6 +2,7 @@ package unknown import ( "regexp" + "strings" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/file" @@ -19,7 +20,7 @@ func ProcessPathErrors(err error) error { if pathErrorRegex.MatchString(errText) { foundPath := pathErrorRegex.ReplaceAllString(err.Error(), "$1") if foundPath != "" { - return New(file.NewLocation(foundPath), err) + return New(file.NewLocationFromCoordinates(pathCoordinates(foundPath)), err) } } unknowns, remainingErrors := ExtractCoordinateErrors(err) @@ -31,3 +32,13 @@ func ProcessPathErrors(err error) error { } return Join(out...) } + +// pathCoordinates names the file at a path from a resolver error, which for a file within an archive is +// a full path: the real path is what precedes the first archive separator. +func pathCoordinates(p string) file.Coordinates { + realPath, _, nested := strings.Cut(p, file.ArchivePathSeparator) + if !nested { + return file.NewCoordinates(p, "") + } + return file.Coordinates{RealPath: realPath, FullPath: p} +} diff --git a/syft/archive_cataloger_test.go b/syft/archive_cataloger_test.go index 27adbdbf4..74616f71e 100644 --- a/syft/archive_cataloger_test.go +++ b/syft/archive_cataloger_test.go @@ -383,6 +383,92 @@ func TestArchiveCataloger_defaultFileSelectionCoversRootJars(t *testing.T) { } } +func TestArchiveCataloger_defaultFileSelectionCoversNestedJars(t *testing.T) { + // a nested jar's package is found inside the jar but located in the archive holding it, so that + // archive's file catalogers must run after it, as the root's do + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "outer.zip"), testutils.Zip(t, map[string][]byte{ + "lib/inner.jar": testutils.Jar(t, "inner", "1.0", nil), + }), 0o644)) + + cfg := DefaultCreateSBOMConfig(). + WithCatalogerSelection(cataloging.NewSelectionRequest().WithDefaults("java")). + WithArchiveConfig(cataloging.DefaultArchiveSearchConfig().WithMaxDepth(2)) + s := scanDirWithExclusions(t, dir, cfg) + + require.Equal(t, []string{"outer.zip!/lib/inner.jar"}, packageLocations(s, "inner")) + jar := file.Coordinates{RealPath: "outer.zip", FullPath: "outer.zip!/lib/inner.jar"} + assert.Contains(t, s.Artifacts.FileMetadata, jar) + assert.Contains(t, s.Artifacts.FileDigests, jar) +} + +func TestArchiveCataloger_archiveUnknowns(t *testing.T) { + dir := t.TempDir() + plain := testutils.Zip(t, map[string]string{"readme.txt": "hi"}) + require.NoError(t, os.WriteFile(filepath.Join(dir, "outer.zip"), testutils.Zip(t, map[string][]byte{ + "lib/inner.jar": testutils.Jar(t, "inner", "1.0", nil), + "lib/plain.zip": plain, + }), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "plain.zip"), plain, 0o644)) + + scan := func(depth int) map[string][]string { + cfg := DefaultCreateSBOMConfig(). + WithCatalogerSelection(cataloging.NewSelectionRequest().WithDefaults("java")). + WithUnknownsConfig(cataloging.DefaultUnknownsConfig()). + WithFilesConfig(filecataloging.DefaultConfig().WithSelection(file.AllFilesSelection)). + WithArchiveConfig(cataloging.DefaultArchiveSearchConfig().WithMaxDepth(depth)) + out := map[string][]string{} + for c, reasons := range scanDirWithExclusions(t, dir, cfg).Artifacts.Unknowns { + out[c.FullNestedPath()] = reasons + } + return out + } + + t.Run("an expanded archive is not reported as not cataloged", func(t *testing.T) { + assert.Empty(t, scan(3)) + }) + + t.Run("an archive past the depth limit is reported once, by the archive cataloger", func(t *testing.T) { + unknowns := scan(1) + assert.NotContains(t, unknowns, "outer.zip") + for _, nested := range []string{"outer.zip!/lib/inner.jar", "outer.zip!/lib/plain.zip"} { + require.Len(t, unknowns[nested], 1, nested) + assert.Contains(t, unknowns[nested][0], "depth limit was reached") + } + }) + + t.Run("an archive is reported as not cataloged when nesting is off", func(t *testing.T) { + assert.Equal(t, []string{"archive not cataloged"}, scan(0)["plain.zip"]) + }) +} + +func TestArchiveCataloger_unsupportedArchiveFormatsAreReported(t *testing.T) { + dir := t.TempDir() + var page bytes.Buffer + gw := gzip.NewWriter(&page) + _, err := gw.Write([]byte("a man page")) + require.NoError(t, err) + require.NoError(t, gw.Close()) + require.NoError(t, os.WriteFile(filepath.Join(dir, "outer.zip"), testutils.Zip(t, map[string][]byte{ + "legacy.7z": []byte("7z\xbc\xaf\x27\x1c\x00\x04"), + "page.1.gz": page.Bytes(), + }), 0o644)) + + cfg := DefaultCreateSBOMConfig(). + WithCatalogerSelection(cataloging.NewSelectionRequest().WithDefaults("java")). + WithUnknownsConfig(cataloging.DefaultUnknownsConfig()). + WithFilesConfig(filecataloging.DefaultConfig().WithSelection(file.AllFilesSelection)). + WithArchiveConfig(cataloging.DefaultArchiveSearchConfig().WithMaxDepth(3)) + unknowns := map[string][]string{} + for c, reasons := range scanDirWithExclusions(t, dir, cfg).Artifacts.Unknowns { + unknowns[c.FullNestedPath()] = reasons + } + + assert.Equal(t, map[string][]string{ + "outer.zip!/legacy.7z": {"archive-cataloger: archive not cataloged: archive format is not supported: application/x-7z-compressed"}, + }, unknowns, "a bare compressed file holds no files, so it is not an archive left uncataloged") +} + func TestArchiveCataloger_javaParentLinkageMatchesTheLegacyPath(t *testing.T) { // a consumer walking java DEPENDENCY_OF edges (the only structure SPDX and CycloneDX keep) must see // the same war-to-jar linkage whether or not the archive task does the recursion diff --git a/syft/create_sbom_config.go b/syft/create_sbom_config.go index 9a0edb062..3df0f89f6 100644 --- a/syft/create_sbom_config.go +++ b/syft/create_sbom_config.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "runtime/debug" - "slices" "strings" "github.com/anchore/syft/internal/log" @@ -202,7 +201,6 @@ func (c *CreateSBOMConfig) makeTaskGroups(src source.Description, exclusions []s environmentTasks := c.environmentTasks() scopeTasks := c.scopeTasks() relationshipsTasks := c.relationshipTasks(src) - unknownTasks := c.unknownsTasks() osFeatureDetectionTasks := c.osFeatureDetectionTasks() pkgTasks, fileTasks, selectionEvidence, err := c.selectTasks(src) @@ -212,7 +210,14 @@ func (c *CreateSBOMConfig) makeTaskGroups(src source.Description, exclusions []s // the same package and file catalogers run against the contents of every archive, after the scan // root and before relationship and unknowns post-processing - archiveTask := task.NewArchiveCatalogerTask(c.Archive, append(slices.Clone(pkgTasks), fileTasks...), exclusions) + archiveTask := task.NewArchiveCatalogerTask(c.Archive, c.Unknowns, pkgTasks, fileTasks, exclusions) + + // the archive cataloger reports the archives it does not expand, which the labeler can only guess at + unknowns := c.Unknowns + if archiveTask != nil { + unknowns.IncludeUnexpandedArchives = false + } + unknownTasks := unknownsTasks(unknowns) // combine the user-provided and configured tasks switch { @@ -466,10 +471,10 @@ func (c *CreateSBOMConfig) environmentTasks() []task.Task { // unknownsTasks returns a set of tasks that perform any necessary post-processing // to identify SBOM elements as unknowns -func (c *CreateSBOMConfig) unknownsTasks() []task.Task { +func unknownsTasks(cfg cataloging.UnknownsConfig) []task.Task { var tasks []task.Task - if t := task.NewUnknownsLabelerTask(c.Unknowns); t != nil { + if t := task.NewUnknownsLabelerTask(cfg); t != nil { tasks = append(tasks, t) } diff --git a/syft/file/cataloger/filedigest/cataloger.go b/syft/file/cataloger/filedigest/cataloger.go index 5617c7eaa..03ebdda78 100644 --- a/syft/file/cataloger/filedigest/cataloger.go +++ b/syft/file/cataloger/filedigest/cataloger.go @@ -42,7 +42,7 @@ func (i *Cataloger) Catalog(ctx context.Context, resolver file.Resolver, coordin locations = intCataloger.AllRegularFiles(ctx, resolver) } else { for _, c := range coordinates { - locs, err := resolver.FilesByPath(c.LocalPath()) + locs, err := resolver.FilesByPath(c.FullNestedPath()) if err != nil { return nil, fmt.Errorf("unable to get file locations for path %q: %w", c.FullNestedPath(), err) } diff --git a/syft/file/cataloger/filemetadata/cataloger.go b/syft/file/cataloger/filemetadata/cataloger.go index ab24a1f40..76dff8d83 100644 --- a/syft/file/cataloger/filemetadata/cataloger.go +++ b/syft/file/cataloger/filemetadata/cataloger.go @@ -34,7 +34,7 @@ func (i *Cataloger) Catalog(ctx context.Context, resolver file.Resolver, coordin go func() { defer close(ch) for _, c := range coordinates { - locs, err := resolver.FilesByPath(c.LocalPath()) + locs, err := resolver.FilesByPath(c.FullNestedPath()) if err != nil { errs = unknown.Append(errs, c, err) continue diff --git a/syft/file/coordinates.go b/syft/file/coordinates.go index 48b86c5bd..c496aad67 100644 --- a/syft/file/coordinates.go +++ b/syft/file/coordinates.go @@ -30,12 +30,12 @@ func (c Coordinates) FullNestedPath() string { return c.FullPath } -// LocalPath is the path within the resolver holding the file: the segment of FullNestedPath after the -// last archive separator, or RealPath for a file not within an archive. +// LocalPath is the rooted path within the resolver holding the file: the segment of FullNestedPath after +// the last archive separator, keeping the separator's slash, or RealPath for a file not within an archive. func (c Coordinates) LocalPath() string { fullPath := c.FullNestedPath() if i := strings.LastIndex(fullPath, ArchivePathSeparator); i >= 0 { - return fullPath[i+len(ArchivePathSeparator):] + return fullPath[i+len(ArchivePathSeparator)-1:] } return fullPath } diff --git a/syft/file/location_test.go b/syft/file/location_test.go index 8ad3ebe20..7ea14de63 100644 --- a/syft/file/location_test.go +++ b/syft/file/location_test.go @@ -105,5 +105,5 @@ func TestCoordinates_ID_unchangedByTopLevelFullRealPath(t *testing.T) { func TestCoordinates_LocalPath(t *testing.T) { assert.Equal(t, "/a.jar", NewCoordinates("/a.jar", "").LocalPath()) assert.Equal(t, "/a.jar", Coordinates{RealPath: "/a.jar"}.LocalPath()) - assert.Equal(t, "META-INF/MANIFEST.MF", Coordinates{RealPath: "/a.war", FullPath: "/a.war!/lib/b.jar!/META-INF/MANIFEST.MF"}.LocalPath()) + assert.Equal(t, "/META-INF/MANIFEST.MF", Coordinates{RealPath: "/a.war", FullPath: "/a.war!/lib/b.jar!/META-INF/MANIFEST.MF"}.LocalPath()) } diff --git a/syft/format/common/spdxhelpers/to_format_model.go b/syft/format/common/spdxhelpers/to_format_model.go index 991e28025..445ca64b8 100644 --- a/syft/format/common/spdxhelpers/to_format_model.go +++ b/syft/format/common/spdxhelpers/to_format_model.go @@ -712,6 +712,26 @@ const ( archivePathCommentPrefix = "archivePath: " ) +// fileComment carries the layer ID and, for a file inside an archive, its archive path. Such a file is +// named by its full path ("/app.war!/WEB-INF/lib/a.jar!/META-INF/MANIFEST.MF"), so a consumer reading +// only the name does not mistake it for a file at that path in the scan root; the entry chain after the +// real path is also kept in the comment so decoding need not guess where it starts. +func fileComment(c file.Coordinates) string { + var entries string + if rest, ok := strings.CutPrefix(c.FullPath, c.RealPath); ok { + entries, _ = strings.CutPrefix(rest, file.ArchivePathSeparator) + } + switch { + case c.FileSystemID != "" && entries != "": + return layerIDCommentPrefix + c.FileSystemID + "\n" + archivePathCommentPrefix + entries + case c.FileSystemID != "": + return layerIDCommentPrefix + c.FileSystemID + case entries != "": + return archivePathCommentPrefix + entries + } + return "" +} + func toFiles(s sbom.SBOM) (results []*spdx.File) { artifacts := s.Artifacts @@ -742,30 +762,16 @@ func toFiles(s sbom.SBOM) (results []*spdx.File) { // TODO: add file classifications (?) and content as a snippet - var comments []string - if c.FileSystemID != "" { - comments = append(comments, layerIDCommentPrefix+c.FileSystemID) - } - - // a file inside an archive is named by its full path ("/app.war!/WEB-INF/lib/a.jar!/META-INF/MANIFEST.MF"), - // so a consumer reading only the name does not mistake it for a file at that path in the scan - // root. The entry chain after the real path is also kept in the comment so decoding need not guess - // where it starts. name := c.FullNestedPath() - if entries, ok := strings.CutPrefix(c.FullNestedPath(), c.RealPath+file.ArchivePathSeparator); ok { - comments = append(comments, archivePathCommentPrefix+entries) - } - relativePath, err := convertAbsoluteToRelative(name) if err != nil { log.Debugf("unable to convert relative path '%s' to absolute path: %s", name, err) relativePath = name } - comment := strings.Join(comments, "\n") results = append(results, &spdx.File{ FileSPDXIdentifier: toSPDXID(c), - FileComment: comment, + FileComment: fileComment(c), // required, no attempt made to determine license information LicenseConcluded: noAssertion, FileCopyrightText: noAssertion, diff --git a/syft/format/common/spdxhelpers/to_syft_model.go b/syft/format/common/spdxhelpers/to_syft_model.go index db7905dbb..ad79dbc01 100644 --- a/syft/format/common/spdxhelpers/to_syft_model.go +++ b/syft/format/common/spdxhelpers/to_syft_model.go @@ -530,18 +530,23 @@ func collectPackageFileRelationships(spdxIDMap map[string]any, doc *spdx.Documen } func toSyftCoordinates(f *spdx.File) file.Coordinates { - c := file.NewCoordinates(f.FileName, "") - for _, line := range strings.Split(f.FileComment, "\n") { + return coordinatesFromFile(f.FileName, f.FileComment, string(f.FileSPDXIdentifier)) +} + +// coordinatesFromFile decodes what fileComment encodes. +func coordinatesFromFile(name, comment, id string) file.Coordinates { + c := file.NewCoordinates(name, "") + for _, line := range strings.Split(comment, "\n") { if v, ok := strings.CutPrefix(line, layerIDCommentPrefix); ok { c.FileSystemID = v } if v, ok := strings.CutPrefix(line, archivePathCommentPrefix); ok { // the name is the full path; the real path is what precedes the entry chain - c.RealPath = strings.TrimSuffix(f.FileName, file.ArchivePathSeparator+v) - c.FullPath = f.FileName + c.RealPath = strings.TrimSuffix(name, file.ArchivePathSeparator+v) + c.FullPath = name } } - if v, ok := strings.CutPrefix(string(f.FileSPDXIdentifier), layerIDCommentPrefix); ok { + if v, ok := strings.CutPrefix(id, layerIDCommentPrefix); ok { c.FileSystemID = v } return c diff --git a/syft/format/common/spdxhelpers/to_syft_model_v3.go b/syft/format/common/spdxhelpers/to_syft_model_v3.go index cd86bf4ca..9ab7a1fb3 100644 --- a/syft/format/common/spdxhelpers/to_syft_model_v3.go +++ b/syft/format/common/spdxhelpers/to_syft_model_v3.go @@ -393,15 +393,7 @@ func v3collectDocRelationships(spdxMap ptrMap[any], doc *spdx.Document) (out []a } func v3toSyftCoordinates(f spdx.AnyFile) file.Coordinates { - const layerIDPrefix = "layerID: " - var fileSystemID string - if strings.HasPrefix(f.GetComment(), layerIDPrefix) { - fileSystemID = strings.TrimPrefix(f.GetComment(), layerIDPrefix) - } - if strings.HasPrefix(f.GetID(), layerIDPrefix) { - fileSystemID = strings.TrimPrefix(f.GetID(), layerIDPrefix) - } - return file.NewCoordinates(f.GetName(), fileSystemID) + return coordinatesFromFile(f.GetName(), f.GetComment(), f.GetID()) } func v3toSyftLocation(f spdx.AnyFile) file.Location { diff --git a/syft/pkg/cataloger/golang/parse_go_mod.go b/syft/pkg/cataloger/golang/parse_go_mod.go index 78bf9dcb5..76780b474 100644 --- a/syft/pkg/cataloger/golang/parse_go_mod.go +++ b/syft/pkg/cataloger/golang/parse_go_mod.go @@ -39,16 +39,13 @@ func newGoModCataloger(opts CatalogerConfig) *goModCataloger { // parseGoModFile takes a go.mod and tries to resolve and lists all packages discovered. func (c *goModCataloger) parseGoModFile(ctx context.Context, resolver file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) (pkgs []pkg.Package, relationships []artifact.Relationship, err error) { + // the go toolchain needs the go.mod on the host: a location without a reference (such as a file within + // an archive) has none, so only the go.mod and go.sum contents are cataloged realPath := string(reader.Location.Reference().RealPath) - if realPath == "" { - // the Reference().RealPath is used to find full paths when using a directory scan, it does not provide correct paths for archive scanning - realPath = reader.LocalPath() - } if windows.HostRunningOnWindows() { // directory resolvers hand back volume-encoded posix paths (/c/...), the go toolchain needs the native path realPath = windows.FromPosix(realPath) } - modDir := filepath.Dir(realPath) digests, err := parseGoSumFile(resolver, reader) if err != nil { log.Debugf("unable to get go.sum: %v", err) @@ -69,12 +66,12 @@ func (c *goModCataloger) parseGoModFile(ctx context.Context, resolver file.Resol var sourceModules map[string]*packages.Module var catalogedModules []pkg.Package - if c.usePackagesLib { + if c.usePackagesLib && realPath != "" { var sourcePackages map[string][]pkgInfo var sourceDependencies map[string][]string var sourceModuleToPkg map[string]artifact.Identifiable - sourcePackages, sourceModules, sourceDependencies, err = c.loadPackages(modDir, reader.Location) + sourcePackages, sourceModules, sourceDependencies, err = c.loadPackages(filepath.Dir(realPath), reader.Location) catalogedModules, sourceModuleToPkg = c.catalogModules(ctx, scanRoot, sourcePackages, sourceModules, reader, digests) relationships = buildModuleRelationships(catalogedModules, sourceDependencies, sourceModuleToPkg) } diff --git a/syft/pkg/cataloger/perl/perllocal.go b/syft/pkg/cataloger/perl/perllocal.go index d46c62613..149785e58 100644 --- a/syft/pkg/cataloger/perl/perllocal.go +++ b/syft/pkg/cataloger/perl/perllocal.go @@ -217,7 +217,7 @@ func parsePerllocal(r io.Reader) []perllocalEntry { func primaryPath(p pkg.Package) string { for _, location := range p.Locations.ToSlice() { if location.Annotations[pkg.EvidenceAnnotationKey] == pkg.PrimaryEvidenceAnnotation { - return location.Path() + return location.LocalPath() } } return "" diff --git a/syft/pkg/cataloger/python/parse_wheel_egg_metadata.go b/syft/pkg/cataloger/python/parse_wheel_egg_metadata.go index 8a0c10cb6..35874ebff 100644 --- a/syft/pkg/cataloger/python/parse_wheel_egg_metadata.go +++ b/syft/pkg/cataloger/python/parse_wheel_egg_metadata.go @@ -48,7 +48,7 @@ func parseWheelOrEggMetadata(locationReader file.LocationReadCloser) (parsedData } // add additional metadata not stored in the egg/wheel metadata file - metadataPath := locationReader.Path() + metadataPath := locationReader.LocalPath() pd.SitePackagesRootPath = determineSitePackagesRootPath(metadataPath) if pd.Licenses != "" || pd.LicenseExpression != "" {