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 <wagoodman@users.noreply.github.com>

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

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()

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

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")
+}