From 9d820c6d94e4ac8de80868e8dc4e94504b4c8b67 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Thu, 24 Sep 2026 13:30:33 -0400 Subject: [PATCH] fix: only look up owned-by-package files in the resolver's own frame with `FilesOwnedByPackageSelection` the file catalogers take every package location in the SBOM and look it up by `RealPath` in the resolver they were given. Inside an archive, the java package's location is the archive itself *in its parent's frame* (`WEB-INF/lib/dep.jar`), so the sub-pipeline looked that path up inside `dep.jar`'s own resolver: a miss, or an unrelated same-named entry by coincidence. At the root, coordinates from inside archives were looked up against the scan root the same way. `coordinatesForSelection` now keeps only coordinates whose `ArchivePath` matches the current frame (the traversal's `ContentsArchivePath()`, or blank at the root). Signed-off-by: Alex Goodman --- internal/archive/traversal.go | 9 +++++++++ internal/task/archive_tasks.go | 5 +++-- internal/task/file_tasks.go | 20 ++++++++++++++++---- internal/task/file_tasks_test.go | 26 +++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/internal/archive/traversal.go b/internal/archive/traversal.go index 6690047b2..d6e3711de 100644 --- a/internal/archive/traversal.go +++ b/internal/archive/traversal.go @@ -22,6 +22,15 @@ type Traversal struct { Digests []file.Digest } +// ContentsArchivePath is the ArchivePath every file inside the archive carries: the chain to it, built +// from real paths. Outside any archive (a nil Traversal) it is blank, as for files at the scan root. +func (t *Traversal) ContentsArchivePath() string { + if t == nil { + return "" + } + return VirtualPath(file.NewLocationFromCoordinates(t.Location.Coordinates)) +} + // VirtualPath returns the colon-delimited chain of archives from the scan root to loc, ending in loc // itself: "app.war:WEB-INF/lib/dep.jar". Outside any archive it is the location's path unchanged. In an // entry name '%' is escaped as "%25" and then ':' as "%3A", so splitting on ':' recovers the archive diff --git a/internal/task/archive_tasks.go b/internal/task/archive_tasks.go index 095aadf36..538bd9221 100644 --- a/internal/task/archive_tasks.go +++ b/internal/task/archive_tasks.go @@ -203,7 +203,8 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi // extracted files keep the filesystem ID of where the archive was found; the nesting chain is // carried separately as the archive path. The chain names real paths, not the link the archive was // reached by, so it is stable and joins to the archive's package location by prefix. - archivePath := archive.VirtualPath(file.NewLocationFromCoordinates(location.Coordinates)) + traversal := &archive.Traversal{Location: location} + archivePath := traversal.ContentsArchivePath() extracted, err := archive.Extract(ctx, archiveContent, location.FileSystemID, archivePath, c.limiter, c.exclusions) if errors.Is(err, archive.ErrDiskLimitReached) { // nothing is released while this archive waits, so skip it rather than block the scan @@ -234,7 +235,7 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi recordArchiveUnknown(builder, location.Coordinates, "archive cataloged from part of its contents: "+extracted.TruncatedReason) } - traversal := &archive.Traversal{Location: location, Digests: extracted.Digests} + traversal.Digests = extracted.Digests ctx = archive.WithTraversal(ctx, traversal) c.progress.Increment() diff --git a/internal/task/file_tasks.go b/internal/task/file_tasks.go index 7af11fcd1..b12202f07 100644 --- a/internal/task/file_tasks.go +++ b/internal/task/file_tasks.go @@ -4,6 +4,7 @@ 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" @@ -39,7 +40,7 @@ func newFileDigestCatalogerTask(selection file.Selection, hashers []crypto.Hash, accessor := builder.(sbomsync.Accessor) - coordinates, ok := coordinatesForSelection(selection, builder.(sbomsync.Accessor)) + coordinates, ok := coordinatesForSelection(ctx, selection, builder.(sbomsync.Accessor)) if !ok { return nil } @@ -70,7 +71,7 @@ func newFileMetadataCatalogerTask(selection file.Selection, tags ...string) Task accessor := builder.(sbomsync.Accessor) - coordinates, ok := coordinatesForSelection(selection, builder.(sbomsync.Accessor)) + coordinates, ok := coordinatesForSelection(ctx, selection, builder.(sbomsync.Accessor)) if !ok { return nil } @@ -141,7 +142,12 @@ 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 -func coordinatesForSelection(selection file.Selection, accessor sbomsync.Accessor) ([]file.Coordinates, bool) { +// +// 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) { if selection == file.AllFilesSelection { return nil, true } @@ -169,7 +175,13 @@ func coordinatesForSelection(selection file.Selection, accessor sbomsync.Accesso } }) - coords := coordinates.ToSlice() + frame := archive.TraversalFromContext(ctx).ContentsArchivePath() + var coords []file.Coordinates + for _, c := range coordinates.ToSlice() { + if c.ArchivePath == frame { + coords = append(coords, c) + } + } if len(coords) == 0 { return nil, false diff --git a/internal/task/file_tasks_test.go b/internal/task/file_tasks_test.go index b24e7e254..06641fb49 100644 --- a/internal/task/file_tasks_test.go +++ b/internal/task/file_tasks_test.go @@ -1,10 +1,13 @@ 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" @@ -96,9 +99,30 @@ func Test_coordinatesForSelection(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - files, ok := coordinatesForSelection(tt.selection, sbomsync.NewBuilder(tt.sbom).(sbomsync.Accessor)) + files, ok := coordinatesForSelection(context.Background(), 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.Coordinates{RealPath: "/app.war"} + inWar := file.Coordinates{RealPath: "WEB-INF/lib/dep.jar", ArchivePath: "/app.war"} + 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") +}