mirror of
https://github.com/anchore/syft.git
synced 2026-10-30 14:02:26 +01:00
fix: emit archive-to-archive CONTAINS edges and drop self-loops
the archive's `CONTAINS` edges were built from every coordinate in the sub-pipeline's scratch SBOM, which gets one of two things wrong: - with the default `FilesOwnedByPackageSelection`, nothing records the nested archives as files, so the archive-to-archive chain was missing and grandchild packages hung off coordinates with no incoming edge - the scratch SBOM also holds coordinates from the parent's frame, most notably unknowns recorded at the archive's own coordinates (e.g. a jar with no manifest). Those turned into `x.jar CONTAINS x.jar` edges now go to the nested archives found in it (always, whatever the file selection), plus coordinates whose `ArchivePath` is this archive's own chain, i.e. files that are actually inside it. The walk reuses that one discovery pass for the recursion. Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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...)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user