diff --git a/internal/task/archive_nesting_test.go b/internal/task/archive_nesting_test.go index b93584633..1e7d604c4 100644 --- a/internal/task/archive_nesting_test.go +++ b/internal/task/archive_nesting_test.go @@ -132,14 +132,14 @@ func Test_mixedFamilyNesting_leafIsCatalogedOnceWithTheFullChain(t *testing.T) { } func Test_mixedFamilyNesting_containsEdgesChainAtEveryLevel(t *testing.T) { - // the edges make the chain navigable in the SBOM and key on coordinates the file catalogers - // recorded, so file cataloging has to be on for the inner archives to have any + // the edges make the chain navigable in the SBOM. The archive-to-archive edges must not depend on + // the file catalogers having recorded the inner archives, since the default file selection does not for _, row := range nestingRows() { t.Run(row.name, func(t *testing.T) { scanDir := t.TempDir() nested := writeNestedArchive(t, scanDir, nestPlan{families: row.families, leaf: markerLeaf()}) - s := runNesting(t, scanDir, 3, defaultLimits(), markerTask(), fileMetadataTask()) + s := runNesting(t, scanDir, 3, defaultLimits(), markerTask()) // each archive's coordinates as seen in its parent; the outermost sits in the scanned // directory, so it carries no filesystem id of its own @@ -178,10 +178,16 @@ func Test_mixedFamilyNesting_containsEdgesChainAtEveryLevel(t *testing.T) { innermost := archiveCoords[len(archiveCoords)-1] assert.True(t, pkgEdges.Has(coordKey(innermost)+" -> marker-pkg"), "expected a CONTAINS edge from %s to the leaf package", nested.paths[len(nested.paths)-1]) - assert.True(t, fileEdges.Has(coordKey(innermost)+" -> "+coordKey(file.Coordinates{ - RealPath: "nested/marker.txt", - ArchivePath: nested.leafFileSystemID(), - })), "expected a CONTAINS edge from the innermost archive to the leaf file") + + // with file cataloging on, files inside an archive get an edge from it too + s = runNesting(t, scanDir, 3, defaultLimits(), markerTask(), fileMetadataTask()) + var leafEdge bool + for _, rel := range s.Relationships { + from, _ := rel.From.(file.Coordinates) + to, _ := rel.To.(file.Coordinates) + leafEdge = leafEdge || (from == innermost && to == file.Coordinates{RealPath: "nested/marker.txt", ArchivePath: nested.leafFileSystemID()}) + } + assert.True(t, leafEdge, "expected a CONTAINS edge from the innermost archive to the leaf file") }) } } @@ -508,9 +514,7 @@ func javaTask() Task { return NewPackageTask(CatalogingFactoryConfig{}, java.NewArchiveCataloger(java.DefaultArchiveCatalogerConfig())) } -// fileMetadataTask records every file, which the CONTAINS chain needs: archive-to-file edges key on -// coordinates the file catalogers recorded, so without them an inner archive has no coordinate to -// hang an edge on. +// fileMetadataTask records every file, so each file inside an archive gets an archive-to-file edge. func fileMetadataTask() Task { return newFileMetadataCatalogerTask(file.AllFilesSelection) } diff --git a/internal/task/archive_tasks.go b/internal/task/archive_tasks.go index 8dfe58476..095aadf36 100644 --- a/internal/task/archive_tasks.go +++ b/internal/task/archive_tasks.go @@ -140,6 +140,10 @@ const unboundedArchiveDepth = 16 // catalog processes every archive in the resolver, recursing into each up to maxDepth. func (c *archiveCataloger) catalog(ctx context.Context, resolver file.Resolver, depth int, builder sbomsync.Builder) error { + return c.catalogCandidates(ctx, resolver, c.discoverArchives(resolver), depth, builder) +} + +func (c *archiveCataloger) catalogCandidates(ctx context.Context, resolver file.Resolver, candidates []archiveCandidate, depth int, builder sbomsync.Builder) error { maxDepth := c.maxDepth if maxDepth < 0 { maxDepth = unboundedArchiveDepth @@ -147,7 +151,7 @@ func (c *archiveCataloger) catalog(ctx context.Context, resolver file.Resolver, if depth >= maxDepth { // java's own recursion is off while this task runs, so an archive left here is otherwise lost // without a trace - for _, candidate := range c.discoverArchives(resolver) { + for _, candidate := range candidates { if candidate.sniffedAsArchive { recordArchiveUnknown(builder, candidate.location.Coordinates, "nested archive not cataloged: the nested archive depth limit was reached") } @@ -155,7 +159,7 @@ func (c *archiveCataloger) catalog(ctx context.Context, resolver file.Resolver, return nil } var errs error - for _, candidate := range c.discoverArchives(resolver) { + for _, candidate := range candidates { if err := ctx.Err(); err != nil { return err } @@ -235,7 +239,8 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi c.progress.Increment() scratch := c.runSubPipeline(ctx, extracted, location.Coordinates) - mergeArchiveResults(location.Coordinates, scratch, builder) + 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 @@ -243,7 +248,7 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi c.slowest, c.slowestPath = took, archivePath } - return c.catalog(ctx, extracted, depth+1, builder) + return c.catalogCandidates(ctx, extracted, children, depth+1, builder) } // a top-level archive and everything nested in it may decompress to archiveTreeRatio times its size, or @@ -359,7 +364,12 @@ func (c *archiveCataloger) discoverArchives(resolver file.Resolver) []archiveCan // 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. -func mergeArchiveResults(archiveCoordinates file.Coordinates, scratch *sbom.SBOM, builder sbomsync.Builder) { +// +// Files inside the archive are those whose ArchivePath is the archive's own chain, 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 { builder.AddPackages(pkgs...) @@ -375,7 +385,16 @@ func mergeArchiveResults(archiveCoordinates file.Coordinates, scratch *sbom.SBOM for _, p := range pkgs { rels = append(rels, artifact.Relationship{From: archiveCoordinates, To: p, Type: artifact.ContainsRelationship}) } + contained := file.NewCoordinateSet() + for _, child := range children { + contained.Add(child.location.Coordinates) + } for _, coordinates := range scratch.AllCoordinates() { + if coordinates.ArchivePath == archivePath { + contained.Add(coordinates) + } + } + for _, coordinates := range contained.ToSlice() { rels = append(rels, artifact.Relationship{From: archiveCoordinates, To: coordinates, Type: artifact.ContainsRelationship}) } rels = append(rels, scratch.Relationships...) diff --git a/internal/task/archive_tasks_test.go b/internal/task/archive_tasks_test.go index 4d5488984..24aa36c2e 100644 --- a/internal/task/archive_tasks_test.go +++ b/internal/task/archive_tasks_test.go @@ -32,6 +32,7 @@ import ( "github.com/anchore/syft/internal/mimetype" "github.com/anchore/syft/internal/sbomsync" "github.com/anchore/syft/internal/unknown" + "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/cataloging" "github.com/anchore/syft/syft/event" "github.com/anchore/syft/syft/event/monitor" @@ -396,6 +397,29 @@ func Test_archiveCataloger_decompressionBudgetIsSharedByATreeAndFreshPerTopLevel assert.Equal(t, 1, fileCounts["/sibling.zip"], "another top-level archive starts with a fresh budget") } +func Test_archiveCataloger_noSelfContainsEdge(t *testing.T) { + // a jar java cannot identify records an unknown at its own coordinates; that must not turn into + // "x.jar CONTAINS x.jar", which a graph walker would loop on + rootDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(rootDir, "x.jar"), makeZip(t, map[string][]byte{"readme.txt": []byte("no manifest")}), 0o600)) + + tsk := newTestTask(t, cataloging.DefaultArchiveSearchConfig().WithMaxDepth(1), javaTask(), fileMetadataTask()) + s := newTestSBOM() + s.Artifacts.FileMetadata = map[file.Coordinates]file.Metadata{} + _ = tsk.Execute(context.Background(), dirTestResolver{dir: rootDir}, sbomsync.NewBuilder(s)) + + require.Contains(t, s.Artifacts.Unknowns, file.Coordinates{RealPath: "/x.jar"}, "the fixture must reproduce the unknown") + var contains int + for _, rel := range s.Relationships { + if rel.Type != artifact.ContainsRelationship { + continue + } + contains++ + assert.NotEqual(t, rel.From.ID(), rel.To.ID(), "self-referential CONTAINS edge: %+v", rel) + } + assert.Positive(t, contains, "the archive still contains its files") +} + func Test_NewArchiveCatalogerTask_dropsItselfFromSubPipeline(t *testing.T) { // a copy of itself in the sub-pipeline would process every nesting level twice cfg := cataloging.DefaultArchiveSearchConfig().WithMaxDepth(2)