From cda56d77906af0ad8a59af063c7036bd6d6c7444 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Tue, 22 Sep 2026 13:21:27 -0400 Subject: [PATCH] chore: rewrite top-level archive_cataloger_test to avoid repeated CreateSBOM calls Signed-off-by: Keith Zantow --- syft/archive_cataloger_test.go | 996 ++++++++++++--------------------- 1 file changed, 355 insertions(+), 641 deletions(-) diff --git a/syft/archive_cataloger_test.go b/syft/archive_cataloger_test.go index 973002760..15584f695 100644 --- a/syft/archive_cataloger_test.go +++ b/syft/archive_cataloger_test.go @@ -6,11 +6,9 @@ import ( "bytes" "compress/gzip" "context" - "io" "os" "path/filepath" "sort" - "strings" "sync" "testing" "unicode/utf8" @@ -35,507 +33,328 @@ import ( "github.com/anchore/syft/syft/source/directorysource" ) -// markerCataloger is a stub package cataloger emitting one package per "marker.txt" it finds, so the -// end-to-end tests assert behavior without depending on any real cataloger's fixture format. -type markerCataloger struct{} +// Every CreateSBOM call pays to compile the license scanner, so the end-to-end behavior is asserted +// over one scan of one fixture directory rather than a scan per claim. The fixture holds every shape +// the claims need; the subtests read the one SBOM. +// +// scan root +// +// loose.txt an ordinary file, counted by the file catalogers +// nested/marker.txt excluded by the root-anchored pattern ./nested/marker.txt +// app.zip nested/marker.txt, nested/keep.txt, nested/excluded.txt (**/excluded.txt) +// app/data.zip nested/marker.txt, under a directory +// outer.zip outer/marker.txt +// deeper/inner.zip nested/marker.txt, two levels down +// skip/inner-excluded.zip nested/marker.txt, excluded by **/inner-excluded.zip +// 日本語/café-🎉/unicode.zip nested/marker.txt, under a non-ASCII path +// pkg.rpm a zip holding a marker, excluded by **/*.rpm +// KEPT.RPM the same zip, kept: the pattern is lower-case (a distinct base +// name, so the two coexist on a case-insensitive filesystem) +// app.war WEB-INF/lib/war-lib-2.0.jar +// bundle.zip lib/zip-lib-1.2.jar +// bundle.tar.gz lib/tgz-lib-1.2.jar +var fixtureExclusions = []string{"./nested/marker.txt", "**/excluded.txt", "**/inner-excluded.zip", "**/*.rpm"} -// TestArchiveCataloger_endToEnd scans a directory holding a zip with a marker file inside: the -// package found there must carry the archive's nesting-chain FileSystemID, and a file-level CONTAINS -// relationship must link the archive to it. -func TestArchiveCataloger_endToEnd(t *testing.T) { - scanDir := t.TempDir() - writeTestZip(t, filepath.Join(scanDir, "app.zip"), map[string]string{"nested/marker.txt": "hello"}) - - src, err := directorysource.New(directorysource.Config{Path: scanDir}) - require.NoError(t, err) - t.Cleanup(func() { _ = src.Close() }) - - cfg := DefaultCreateSBOMConfig(). - WithoutCatalogers(). - WithCatalogers(pkgcataloging.NewCatalogerReference(markerCataloger{}, []string{"directory"})) - cfg.Archive.MaxDepth = 1 - - s, err := cfg.Create(context.Background(), src) - require.NoError(t, err) - - pkgs := s.Artifacts.Packages.Sorted() - require.Len(t, pkgs, 1, "expected exactly one package, discovered inside the archive") - found := pkgs[0] - assert.Equal(t, "marker-pkg", found.Name) - - locs := found.Locations.ToSlice() - require.NotEmpty(t, locs) - // the root directory's FSID is empty, so the chain starts with the relative archive path - assert.Equal(t, "app.zip", locs[0].ArchivePath, "package location should carry the archive's FileSystemID") - - // a file-level CONTAINS relationship links the archive file to the discovered package - var hasContains bool - for _, rel := range s.Relationships { - if rel.Type != artifact.ContainsRelationship || rel.To.ID() != found.ID() { - continue - } - coord, ok := rel.From.(file.Coordinates) - if ok && strings.HasSuffix(coord.RealPath, "app.zip") { - hasContains = true - } - } - assert.True(t, hasContains, "expected a CONTAINS relationship from app.zip to the discovered package") -} - -// TestArchiveCataloger_nestedRecursion covers depth-limited recursion and the FileSystemID chain: a -// marker inside inner.zip inside outer.zip carries the full chain. -func TestArchiveCataloger_nestedRecursion(t *testing.T) { - innerZip := buildZipBytes(t, map[string]string{"nested/marker.txt": "hello"}) - scanDir := t.TempDir() - writeTestZipRaw(t, filepath.Join(scanDir, "outer.zip"), map[string][]byte{ - "deeper/inner.zip": innerZip, - }) - - src, err := directorysource.New(directorysource.Config{Path: scanDir}) - require.NoError(t, err) - t.Cleanup(func() { _ = src.Close() }) - - cfg := DefaultCreateSBOMConfig(). - WithoutCatalogers(). - WithCatalogers(pkgcataloging.NewCatalogerReference(markerCataloger{}, []string{"directory"})) - cfg.Archive.MaxDepth = 2 - - s, err := cfg.Create(context.Background(), src) - require.NoError(t, err) - - pkgs := s.Artifacts.Packages.Sorted() - require.Len(t, pkgs, 1) - locs := pkgs[0].Locations.ToSlice() - require.NotEmpty(t, locs) - assert.Equal(t, "outer.zip:deeper/inner.zip", locs[0].ArchivePath, - "nested package location should carry the full archive nesting chain as its FileSystemID") -} - -func TestArchiveCataloger_jarInsideArchiveCatalogedOnce(t *testing.T) { - // the failure mode is a duplicate, not an error: a jar inside a zip can be reached by the task's - // walk and by the java cataloger's own unarchiving, so count rather than assert presence - dep := jarBytes(t, "nested-lib", "2.0", nil) - - tests := []struct { - name string - container string - entry string - }{ - {name: "jar inside a zip", container: "bundle.zip", entry: "lib/nested-lib-2.0.jar"}, - {name: "jar inside a war", container: "app.war", entry: "WEB-INF/lib/nested-lib-2.0.jar"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - scanDir := t.TempDir() - writeTestZipRaw(t, filepath.Join(scanDir, tt.container), map[string][]byte{tt.entry: dep}) - - s := scanDirWith(t, scanDir, javaScanConfig(3)) - - var count int - for _, p := range s.Artifacts.Packages.Sorted() { - if p.Name == "nested-lib" { - count++ - } - } - assert.Equal(t, 1, count, "the nested jar's package must be cataloged exactly once") - }) - } -} - -func TestArchiveCataloger_javaVirtualPathUnchangedByMechanism(t *testing.T) { - // these colon-delimited strings appear in already-published SBOMs, so the package must be reported - // under the same virtual path whichever mechanism reached it - dep := jarBytes(t, "nested-lib", "2.0", nil) - scanDir := t.TempDir() - writeTestZipRaw(t, filepath.Join(scanDir, "app.war"), map[string][]byte{ - "WEB-INF/lib/nested-lib-2.0.jar": dep, - }) - - virtualPaths := func(t *testing.T, depth int) map[string]string { - t.Helper() - s := scanDirWith(t, scanDir, javaScanConfig(depth)) - - out := map[string]string{} - for _, p := range s.Artifacts.Packages.Sorted() { - if metadata, ok := p.Metadata.(pkg.JavaArchive); ok { - out[p.Name] = metadata.VirtualPath - } - } - return out - } - - off := virtualPaths(t, 0) - on := virtualPaths(t, 3) - - require.Contains(t, off, "nested-lib", "the java cataloger's own recursion must find it when the feature is off") - require.Contains(t, on, "nested-lib", "the archive cataloger must find it when the feature is on") - // the compatibility guarantee: the string is in published SBOMs, so a package reached through the - // archive cataloger must carry the same virtual path java's own recursion gave it - assert.Equal(t, off["nested-lib"], on["nested-lib"], - "the nested jar's virtual path must be byte-identical between the two mechanisms") - - // and in colon-delimited form; for a directory source the archive's access path is relative, so the - // chain has no leading separator - assert.Equal(t, "app.war:WEB-INF/lib/nested-lib-2.0.jar", on["nested-lib"]) -} - -func TestArchiveCataloger_archiveContentsDoNotLeakIntoParentTree(t *testing.T) { - scanDir := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(scanDir, "app"), 0o755)) - writeTestZip(t, filepath.Join(scanDir, "app", "data.zip"), map[string]string{"nested/marker.txt": "hello"}) - - s := scanDirWith(t, scanDir, archiveScanConfig(1, markerCataloger{})) - - for _, p := range s.Artifacts.Packages.Sorted() { - for _, loc := range p.Locations.ToSlice() { - // the nested file's path is relative to the archive, and the FileSystemID says which archive. - // A joined or flattened path would smear the archive's contents into the parent's tree, which - // is what the rejected resolver-recursion approach did. - assert.NotContains(t, loc.RealPath, "data.zip/") - assert.NotEqual(t, "/app/nested/marker.txt", loc.RealPath) - assert.Equal(t, "app/data.zip", loc.ArchivePath) - } - } -} - -// there is no archive-specific exclusion setting: a scan pattern's shape decides whether it reaches -// inside an archive -func TestArchiveCataloger_exclusionsAreTheScansOwn(t *testing.T) { +func writeFixture(t *testing.T) string { + t.Helper() + dir := t.TempDir() marker := map[string]string{"nested/marker.txt": "hello"} + markerZip := buildZipBytes(t, marker) - t.Run("a recursive pattern reaches inside archives", func(t *testing.T) { - // the marker is inside the archive, so only an exclusion applied to the archive's own index - // removes it; a filter over the scan root never sees this path - scanDir := t.TempDir() - writeTestZip(t, filepath.Join(scanDir, "app.zip"), marker) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "nested"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "app"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "loose.txt"), []byte("outside"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "nested", "marker.txt"), []byte("at the root"), 0o644)) - s := scanDirWithExclusions(t, scanDir, archiveScanConfig(1, markerCataloger{}), "**/marker.txt") - assert.Empty(t, s.Artifacts.Packages.Sorted(), - "a pattern matching at any depth must exclude a file inside an archive") + writeTestZip(t, filepath.Join(dir, "app.zip"), map[string]string{ + "nested/marker.txt": "hello", + "nested/keep.txt": "hello", + "nested/excluded.txt": "hello", }) - - t.Run("a root-anchored pattern does not reach inside archives", func(t *testing.T) { - // the same relative path exists twice, at the scan root and inside the archive; an anchored - // pattern names the first and says nothing about the second - scanDir := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(scanDir, "nested"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(scanDir, "nested", "marker.txt"), []byte("at the root"), 0o644)) - writeTestZip(t, filepath.Join(scanDir, "app.zip"), marker) - - s := scanDirWithExclusions(t, scanDir, archiveScanConfig(1, markerCataloger{}), "./nested/marker.txt") - - var fsIDs []string - for _, p := range s.Artifacts.Packages.Sorted() { - for _, loc := range p.Locations.ToSlice() { - fsIDs = append(fsIDs, loc.ArchivePath) - } - } - assert.Equal(t, []string{"app.zip"}, fsIDs, - "the file at the scan root must be excluded and the one inside the archive kept") + writeTestZipRaw(t, filepath.Join(dir, "app", "data.zip"), map[string][]byte{"nested/marker.txt": []byte("hello")}) + writeTestZipRaw(t, filepath.Join(dir, "outer.zip"), map[string][]byte{ + "outer/marker.txt": []byte("hello"), + "deeper/inner.zip": markerZip, + "skip/inner-excluded.zip": markerZip, + "日本語/café-🎉/unicode.zip": markerZip, }) + require.NoError(t, os.WriteFile(filepath.Join(dir, "pkg.rpm"), markerZip, 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "KEPT.RPM"), markerZip, 0o644)) - t.Run("excluding an archive's own extension stops it being descended into", func(t *testing.T) { - // the archive is not extracted; it is still an ordinary file, and nothing about it is an error - scanDir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(scanDir, "pkg.rpm"), buildZipBytes(t, marker), 0o644)) - - s := scanDirWithExclusions(t, scanDir, archiveScanConfig(1, markerCataloger{}), "**/*.rpm") - assert.Empty(t, s.Artifacts.Packages.Sorted(), - "an archive whose own path is excluded must not be extracted") + writeTestZipRaw(t, filepath.Join(dir, "app.war"), map[string][]byte{ + "WEB-INF/lib/war-lib-2.0.jar": jarBytes(t, "war-lib", "2.0"), }) - - t.Run("a nested archive is excluded at its own level", func(t *testing.T) { - // outer.zip is cataloged; inner.zip is excluded, so it is absent from outer's own filesystem - // and is neither extracted nor recordable as a file. Asserted through the resolver, since - // absence from the index is upstream of every consumer. - innerZip := buildZipBytes(t, marker) - scanDir := t.TempDir() - writeTestZipRaw(t, filepath.Join(scanDir, "outer.zip"), map[string][]byte{ - "deeper/inner.zip": innerZip, - "outer/marker.txt": []byte("hello"), - }) - - var seen []string - probe := resolverProbeCataloger{ - glob: "**/*.zip", - record: func(paths []string) { - seen = append(seen, paths...) - }, - } - s := scanDirWithExclusions(t, scanDir, archiveScanConfig(2, markerCataloger{}, probe), "**/inner.zip") - - var fsIDs []string - for _, p := range s.Artifacts.Packages.Sorted() { - for _, loc := range p.Locations.ToSlice() { - fsIDs = append(fsIDs, loc.ArchivePath) - } - } - assert.Equal(t, []string{"outer.zip"}, fsIDs, - "the outer archive must still be cataloged and the inner one must contribute nothing") - - for _, path := range seen { - assert.NotContains(t, path, "inner.zip", - "the excluded inner archive must be absent from the outer archive's filesystem") - } - assert.NotEmpty(t, seen, "the probe must have seen the outer archive at the scan root") - }) - - t.Run("matching follows the scan's own case sensitivity", func(t *testing.T) { - // an exclusion behaves the same inside an archive as anywhere else; a caller wanting both - // cases says so in the pattern, as for any other exclusion - scanDir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(scanDir, "PKG.RPM"), buildZipBytes(t, marker), 0o644)) - - kept := scanDirWithExclusions(t, scanDir, archiveScanConfig(1, markerCataloger{}), "**/*.rpm") - assert.NotEmpty(t, kept.Artifacts.Packages.Sorted(), - "a lower-case pattern must not match an upper-case name") - - excluded := scanDirWithExclusions(t, scanDir, archiveScanConfig(1, markerCataloger{}), "**/*.[rR][pP][mM]") - assert.Empty(t, excluded.Artifacts.Packages.Sorted(), - "the case-insensitive intent is expressible as an ordinary pattern") - }) - - t.Run("no exclusions means nothing is excluded", func(t *testing.T) { - scanDir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(scanDir, "pkg.rpm"), buildZipBytes(t, marker), 0o644)) - - s := scanDirWith(t, scanDir, archiveScanConfig(1, markerCataloger{})) - assert.NotEmpty(t, s.Artifacts.Packages.Sorted(), - "with no patterns configured every detected archive stays eligible") - }) - - t.Run("an excluded file is absent from the filesystem, not present and skipped", func(t *testing.T) { - // an excluded entry is not in the archive's index at all, so nothing downstream resolves it by - // path or by glob - scanDir := t.TempDir() - writeTestZip(t, filepath.Join(scanDir, "app.zip"), map[string]string{ - "nested/marker.txt": "hello", - "nested/keep.txt": "hello", - }) - - var seen []string - probe := resolverProbeCataloger{ - glob: "**/*.txt", - record: func(paths []string) { - seen = append(seen, paths...) - }, - } - scanDirWithExclusions(t, scanDir, archiveScanConfig(1, probe), "**/marker.txt") - - assert.Contains(t, seen, "nested/keep.txt") - assert.NotContains(t, seen, "nested/marker.txt", - "an excluded path must not be resolvable from the archive's filesystem at all") + writeTestZipRaw(t, filepath.Join(dir, "bundle.zip"), map[string][]byte{ + "lib/zip-lib-1.2.jar": jarBytes(t, "zip-lib", "1.2"), }) + require.NoError(t, os.WriteFile(filepath.Join(dir, "bundle.tar.gz"), buildTarGzBytes(t, map[string][]byte{ + "lib/tgz-lib-1.2.jar": jarBytes(t, "tgz-lib", "1.2"), + }), 0o644)) + return dir } -// resolverProbeCataloger reports what the resolver it was handed can see, which is how "absent from -// the index" is told from "present and skipped" within a scan. -type resolverProbeCataloger struct { - glob string - record func([]string) -} - -func TestArchiveCataloger_unicodeEntryNamesSurviveToOutput(t *testing.T) { - // entry names reach the FileSystemID and then the encoded SBOM; a mangled one is a corrupt - // identifier rather than a visible failure - innerZip := buildZipBytes(t, map[string]string{"nested/marker.txt": "hello"}) - scanDir := t.TempDir() - writeTestZipRaw(t, filepath.Join(scanDir, "outer.zip"), map[string][]byte{ - "日本語/café-🎉/inner.zip": innerZip, - }) - - s := scanDirWith(t, scanDir, archiveScanConfig(2, markerCataloger{})) - - pkgs := s.Artifacts.Packages.Sorted() - require.Len(t, pkgs, 1) - locs := pkgs[0].Locations.ToSlice() - require.NotEmpty(t, locs) - assert.Equal(t, "outer.zip:日本語/café-🎉/inner.zip", locs[0].ArchivePath) - assert.True(t, utf8.ValidString(locs[0].ArchivePath)) -} - -func TestArchiveCataloger_libraryParity(t *testing.T) { - // the library entry point must behave as the CLI does, and must not silently replace anything the - // consumer set on the package cataloger config - dep := jarBytes(t, "nested-lib", "2.0", nil) - scanDir := t.TempDir() - writeTestZipRaw(t, filepath.Join(scanDir, "bundle.zip"), map[string][]byte{"lib/nested-lib-2.0.jar": dep}) - - countNestedLib := func(t *testing.T, s *sbom.SBOM) int { - t.Helper() - var n int - for _, p := range s.Artifacts.Packages.Sorted() { - if p.Name == "nested-lib" { - n++ - } - } - return n - } - - t.Run("WithArchiveConfig enables the feature and hands java its yield", func(t *testing.T) { - s := scanDirWith(t, scanDir, javaScanConfig(3)) - assert.Equal(t, 1, countNestedLib(t, s), "reached exactly once, via the archive cataloger") - }) - - t.Run("setting only java's own depth neither enables the feature nor breaks java", func(t *testing.T) { - // java's squash-inlined MaxDepth is a config field a consumer can set: it must not affect - // recursion ownership, and must not be overwritten - supplied := java.DefaultArchiveCatalogerConfig() - supplied.ArchiveSearchConfig = supplied.ArchiveSearchConfig.WithMaxDepth(3) - - cfg := javaScanConfig(0).WithPackagesConfig(pkgcataloging.Config{JavaArchive: supplied}) - - s := scanDirWith(t, scanDir, cfg) - // the archive task never ran, so java's own recursion found it - once - assert.Equal(t, 1, countNestedLib(t, s), "java must keep recursing itself") - assert.Equal(t, 3, cfg.Packages.JavaArchive.MaxDepth, "the consumer's value must survive") - }) -} - -func TestArchiveCataloger_noExtractionPathReachesTheSBOM(t *testing.T) { - // covers syft/archive-content-identity#nested-paths-are-archive-relative. AllLocations emitted the - // resolver's own root unrelativized, and filemetadata.Cataloger records every location it is handed, - // so a coordinate whose RealPath was the extraction temp dir reached FileMetadata, merged into the - // shared SBOM, and gained a CONTAINS edge. File cataloging must be on: it is the only path that - // enumerates all locations rather than searching for them. - innerZip := buildZipBytes(t, map[string]string{"nested/marker.txt": "hello"}) - scanDir := t.TempDir() - writeTestZipRaw(t, filepath.Join(scanDir, "outer.zip"), map[string][]byte{"deeper/inner.zip": innerZip}) - - // the resolver normalizes its root with EvalSymlinks, so the scan root it reports is the resolved - // path. On macOS t.TempDir() sits under /var, a symlink to /private/var, so comparing against the - // unresolved value fails there while passing on linux. +func TestArchiveCataloger(t *testing.T) { + scanDir := writeFixture(t) resolvedScanDir, err := filepath.EvalSymlinks(scanDir) require.NoError(t, err) - cfg := archiveScanConfig(2, markerCataloger{}). - WithFilesConfig(filecataloging.Config{Selection: file.AllFilesSelection}) - s := scanDirWith(t, scanDir, cfg) + recorder := &recordingBus{} + bus.Set(recorder) + t.Cleanup(func() { bus.Set(nil) }) - // every path syft can report about this scan, from every table and both ends of every edge - var paths []string - for coords := range s.Artifacts.FileMetadata { - paths = append(paths, coords.RealPath) - } - for _, p := range s.Artifacts.Packages.Sorted() { - for _, loc := range p.Locations.ToSlice() { - paths = append(paths, loc.RealPath, loc.AccessPath) + var ( + probeMu sync.Mutex + seen []string // every .txt and .zip path any resolver handed a cataloger could see + ) + probe := resolverProbeCataloger{globs: []string{"**/*.txt", "**/*.zip"}, record: func(paths []string) { + probeMu.Lock() + defer probeMu.Unlock() + seen = append(seen, paths...) + }} + + // the java catalogers come from the factory, as the CLI builds them: the flag handing recursion + // to the archive task is derived onto the factory's config. Both legacy search settings are off, + // the cell of the matrix that looks like a bug: with the feature on they no longer gate anything + packages := pkgcataloging.DefaultConfig() + packages.JavaArchive.IncludeIndexedArchives = false + packages.JavaArchive.IncludeUnindexedArchives = false + cfg := DefaultCreateSBOMConfig(). + WithCatalogerSelection(cataloging.NewSelectionRequest().WithDefaults("java")). + WithCatalogers( + pkgcataloging.NewAlwaysEnabledCatalogerReference(markerCataloger{}), + pkgcataloging.NewAlwaysEnabledCatalogerReference(probe), + ). + WithPackagesConfig(packages). + WithFilesConfig(filecataloging.DefaultConfig().WithSelection(file.AllFilesSelection)). + WithArchiveConfig(cataloging.DefaultArchiveSearchConfig().WithMaxDepth(3)) + + s := scanDirWithExclusions(t, scanDir, cfg, fixtureExclusions...) + + t.Run("markers are found at every depth under archive-relative paths", func(t *testing.T) { + // the location's ArchivePath is the chain of archives; RealPath is relative to the innermost + assert.Equal(t, []string{ + "KEPT.RPM|nested/marker.txt", + "app.zip|nested/marker.txt", + "app/data.zip|nested/marker.txt", + "outer.zip:deeper/inner.zip|nested/marker.txt", + "outer.zip:日本語/café-🎉/unicode.zip|nested/marker.txt", + "outer.zip|outer/marker.txt", + }, packageLocations(s, "marker-pkg")) + }) + + t.Run("archive contents do not leak into the parent tree", func(t *testing.T) { + for _, loc := range packageLocations(s, "marker-pkg") { + assert.NotContains(t, loc, ".zip/", "a joined path smears the archive's contents into the parent's tree") + assert.NotContains(t, loc, "|/", "a nested path must not be rooted") } - } - for _, rel := range s.Relationships { - for _, end := range []artifact.Identifiable{rel.From, rel.To} { - if coords, ok := end.(file.Coordinates); ok { - paths = append(paths, coords.RealPath) + }) + + t.Run("unicode entry names survive to output", func(t *testing.T) { + for _, loc := range packageLocations(s, "marker-pkg") { + assert.True(t, utf8.ValidString(loc)) + } + }) + + t.Run("a CONTAINS relationship links each archive to what was found inside", func(t *testing.T) { + var containers []string + for _, p := range s.Artifacts.Packages.Sorted() { + if p.Name != "marker-pkg" { + continue + } + for _, rel := range s.Relationships { + if rel.Type != artifact.ContainsRelationship || rel.To.ID() != p.ID() { + continue + } + if coords, ok := rel.From.(file.Coordinates); ok { + containers = append(containers, coords.ArchivePath+"|"+coords.RealPath) + } } } - } - require.NotEmpty(t, paths) + sort.Strings(containers) + assert.Equal(t, []string{ + "outer.zip|deeper/inner.zip", + "outer.zip|日本語/café-🎉/unicode.zip", + "|KEPT.RPM", + "|app.zip", + "|app/data.zip", + "|outer.zip", + }, containers, "the archive file, named in its own parent, contains the package") + }) - for _, p := range paths { - assert.NotContains(t, p, "syft-archive-", - "an extraction directory reached the SBOM at %q", p) - assert.NotEqual(t, "entries", filepath.Base(p), - "the file holding an archive's entries reached the SBOM at %q", p) - if filepath.IsAbs(p) { - // the directory resolver has always reported the scan root absolute: it is the path the - // user asked for. Anything else absolute is a filesystem this feature invented leaking its - // own root. - assert.Equal(t, resolvedScanDir, p, "only the scanned directory itself may be reported absolute") - } - } -} + t.Run("exclusions are the scan's own", func(t *testing.T) { + // there is no archive-specific exclusion setting: a scan pattern's shape decides whether it + // reaches inside an archive, and an excluded entry is absent from the archive's filesystem + // rather than present and skipped + markers := packageLocations(s, "marker-pkg") + assert.NotContains(t, markers, "|nested/marker.txt", "a root-anchored pattern excludes the file at the scan root") + assert.Contains(t, markers, "app.zip|nested/marker.txt", "and says nothing about the same path inside an archive") + assert.NotContains(t, markers, "pkg.rpm|nested/marker.txt", "an archive whose own path is excluded is not extracted") + assert.Contains(t, markers, "KEPT.RPM|nested/marker.txt", "a lower-case pattern does not match an upper-case name") + assert.NotContains(t, markers, "outer.zip:skip/inner-excluded.zip|nested/marker.txt", "a nested archive is excluded at its own level") -func TestArchiveCataloger_repeatedScansAgreeOnNestedPaths(t *testing.T) { - // the reproducibility property the requirement exists for: extraction directories are created fresh - // per run, so anything of theirs reaching output makes two scans of identical input disagree - innerZip := buildZipBytes(t, map[string]string{"nested/marker.txt": "hello"}) - scanDir := t.TempDir() - writeTestZipRaw(t, filepath.Join(scanDir, "outer.zip"), map[string][]byte{"deeper/inner.zip": innerZip}) + assert.Contains(t, seen, "nested/keep.txt") + assert.NotContains(t, seen, "nested/excluded.txt", "a **/ pattern excludes inside an archive, from the index itself") + assert.Contains(t, seen, "deeper/inner.zip", "the probe ran inside outer.zip") + assert.NotContains(t, seen, "skip/inner-excluded.zip", "the excluded inner archive is absent from outer's filesystem") + }) - scan := func(t *testing.T) []string { - t.Helper() - cfg := archiveScanConfig(2, markerCataloger{}). - WithFilesConfig(filecataloging.Config{Selection: file.AllFilesSelection}) - s := scanDirWith(t, scanDir, cfg) + t.Run("a jar inside an archive is cataloged exactly once, under the virtual path java gives it", func(t *testing.T) { + // the failure mode is a duplicate: a jar inside a zip can be reached by the task's walk and by + // the java cataloger's own unarchiving. The colon-delimited virtual path appears in published + // SBOMs, so it must not change with the mechanism; for a directory source the archive's path + // is relative, so the chain has no leading separator + assert.Equal(t, map[string][]string{ + "war-lib": {"app.war:WEB-INF/lib/war-lib-2.0.jar"}, + "zip-lib": {"bundle.zip:lib/zip-lib-1.2.jar"}, + "tgz-lib": {"bundle.tar.gz:lib/tgz-lib-1.2.jar"}, + }, javaVirtualPaths(s), "with both legacy search settings off, every container is still descended") + }) - var out []string + t.Run("no extraction path reaches the SBOM", func(t *testing.T) { + // covers syft/archive-content-identity#nested-paths-are-archive-relative: file cataloging is + // the one path that enumerates every location rather than searching for them, so a resolver + // root that leaked would land in FileMetadata and gain a CONTAINS edge + var paths []string for coords := range s.Artifacts.FileMetadata { - out = append(out, coords.ArchivePath+"|"+coords.RealPath) + paths = append(paths, coords.RealPath) } - sort.Strings(out) - return out - } - - first := scan(t) - require.NotEmpty(t, first) - assert.Equal(t, first, scan(t)) -} - -func TestArchiveCataloger_legacySearchSettingsAreSuperseded(t *testing.T) { - // syft/nested-archive-cataloging#legacy-archive-search-settings-are-superseded. The row that matters - // is nested-on with both settings false: it looks like a bug and is not, so a future change that - // "fixes" it is changing a requirement. - dep := jarBytes(t, "legacy-lib", "1.2", nil) - entry := map[string][]byte{"lib/legacy-lib-1.2.jar": dep} - - containers := map[string][]byte{ - "bundle.zip": buildZipBytesRaw(t, entry), - "bundle.tar.gz": buildTarGzBytes(t, entry), - } - - // with the feature off, java's own wrapped-archive parsers reach the jar, gated by these two - // settings: indexed covers the zip, unindexed covers the tar family - found := func(t *testing.T, container string, depth int, searchSettings *bool) bool { - t.Helper() - scanDir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(scanDir, container), containers[container], 0o644)) - - cfg := javaScanConfig(depth) - if searchSettings != nil { - // these two settings only ever reach the java cataloger's wrapped-archive parser - // registrations, where the CLI's search-indexed-archives and search-unindexed-archives - // land as well - packages := cfg.Packages - packages.JavaArchive.IncludeIndexedArchives = *searchSettings - packages.JavaArchive.IncludeUnindexedArchives = *searchSettings - cfg = cfg.WithPackagesConfig(packages) - } - - for _, p := range scanDirWith(t, scanDir, cfg).Artifacts.Packages.Sorted() { - if p.Name == "legacy-lib" { - return true + for _, p := range s.Artifacts.Packages.Sorted() { + for _, loc := range p.Locations.ToSlice() { + paths = append(paths, loc.RealPath, loc.AccessPath) } } - return false - } + for _, rel := range s.Relationships { + for _, end := range []artifact.Identifiable{rel.From, rel.To} { + if coords, ok := end.(file.Coordinates); ok { + paths = append(paths, coords.RealPath) + } + } + } + require.NotEmpty(t, paths) + for _, p := range paths { + assert.NotContains(t, p, "archive-spill-", "the spill file reached the SBOM at %q", p) + if filepath.IsAbs(p) { + // the directory resolver has always reported the scan root absolute: it is the path the + // user asked for. Anything else absolute is a filesystem this feature invented + assert.Equal(t, resolvedScanDir, p, "only the scanned directory itself may be reported absolute") + } + } + }) - yes, no := true, false + t.Run("file metadata covers every entry of every archive, and nothing else", func(t *testing.T) { + // an exact set is also the reproducibility check: anything of a per-run temp directory reaching + // output would fail it + var got []string + for coords := range s.Artifacts.FileMetadata { + if coords.ArchivePath == "" { + continue + } + got = append(got, coords.ArchivePath+"|"+coords.RealPath) + } + sort.Strings(got) + assert.Equal(t, []string{ + "KEPT.RPM|nested/marker.txt", + "app.war:WEB-INF/lib/war-lib-2.0.jar|META-INF/MANIFEST.MF", + "app.war|WEB-INF/lib/war-lib-2.0.jar", + "app.zip|nested/keep.txt", + "app.zip|nested/marker.txt", + "app/data.zip|nested/marker.txt", + "bundle.tar.gz:lib/tgz-lib-1.2.jar|META-INF/MANIFEST.MF", + "bundle.tar.gz|lib/tgz-lib-1.2.jar", + "bundle.zip:lib/zip-lib-1.2.jar|META-INF/MANIFEST.MF", + "bundle.zip|lib/zip-lib-1.2.jar", + "outer.zip:deeper/inner.zip|nested/marker.txt", + "outer.zip:日本語/café-🎉/unicode.zip|nested/marker.txt", + "outer.zip|deeper/inner.zip", + "outer.zip|outer/marker.txt", + "outer.zip|日本語/café-🎉/unicode.zip", + }, got) + }) - for _, tt := range []struct { - name string - depth int - searchSettings *bool - wantZip bool - wantTarGz bool - }{ - {name: "off, both settings true", depth: 0, searchSettings: &yes, wantZip: true, wantTarGz: true}, - {name: "off, both settings false", depth: 0, searchSettings: &no, wantZip: false, wantTarGz: false}, - {name: "on, both settings true", depth: 2, searchSettings: &yes, wantZip: true, wantTarGz: true}, - // the load-bearing cell - {name: "on, both settings false", depth: 2, searchSettings: &no, wantZip: true, wantTarGz: true}, - // and what real users hit: unindexed defaults to false, so the tar.gz is cataloged with a - // setting whose documented meaning says it should not be - {name: "on, settings untouched", depth: 2, searchSettings: nil, wantZip: true, wantTarGz: true}, - {name: "off, settings untouched", depth: 0, searchSettings: nil, wantZip: true, wantTarGz: false}, - } { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.wantZip, found(t, "bundle.zip", tt.depth, tt.searchSettings), "zip") - assert.Equal(t, tt.wantTarGz, found(t, "bundle.tar.gz", tt.depth, tt.searchSettings), "tar.gz") - }) - } + t.Run("file catalogers publish one progress row counting archive contents", func(t *testing.T) { + // the file catalogers run once over the scan root and again over every archive; each must + // report into one row carrying the total of every run, or the count runs past the total and + // renders as an overshot bar that reads as complete mid-walk + rows := map[string][]*progress.Manual{} + for _, e := range recorder.events { + if e.Type != event.CatalogerTaskStarted { + continue + } + info, ok := e.Source.(monitor.GenericTask) + require.True(t, ok) + tp, ok := e.Value.(*monitor.TaskProgress) + require.True(t, ok) + rows[info.Title.Default] = append(rows[info.Title.Default], tp.Manual) + } + for _, title := range []string{"File digests", "File metadata"} { + published := rows[title] + require.Len(t, published, 1, "%s must publish exactly one row for the whole scan", title) + row := published[0] + assert.Greater(t, row.Current(), int64(len(s.Artifacts.FileMetadata))/2, + "%s must count the files inside the archives, got %d", title, row.Current()) + if row.Size() >= 0 { + assert.Equal(t, row.Current(), row.Size(), "%s total must cover every run that reported into it", title) + } + } + }) } +// TestArchiveCataloger_featureOff is the one comparison that needs a second scan: with the task +// disabled, java's own unarchiving reaches the jars its legacy settings admit, under the same virtual +// path the task gives them. +func TestArchiveCataloger_featureOff(t *testing.T) { + scanDir := writeFixture(t) + + // java's squash-inlined MaxDepth is a config field a consumer can set: it must not enable the + // feature and must not be overwritten + supplied := java.DefaultArchiveCatalogerConfig() + supplied.ArchiveSearchConfig = supplied.ArchiveSearchConfig.WithMaxDepth(3) + cfg := DefaultCreateSBOMConfig(). + WithCatalogerSelection(cataloging.NewSelectionRequest().WithDefaults("java")). + WithPackagesConfig(pkgcataloging.Config{JavaArchive: supplied}) + + s := scanDirWithExclusions(t, scanDir, cfg, fixtureExclusions...) + + // indexed archives (zip, war) are searched by default and unindexed ones (tar.gz) are not, so + // what a user gets without the feature depends on settings the feature makes irrelevant + assert.Equal(t, map[string][]string{ + "war-lib": {"app.war:WEB-INF/lib/war-lib-2.0.jar"}, + "zip-lib": {"bundle.zip:lib/zip-lib-1.2.jar"}, + }, javaVirtualPaths(s), "java's own recursion finds each once, under the virtual path the task also reports") + assert.Equal(t, 3, cfg.Packages.JavaArchive.MaxDepth, "the consumer's value must survive") +} + +// packageLocations lists "archivePath|realPath" for every location of every package with the name. +func packageLocations(s *sbom.SBOM, name string) []string { + var out []string + for _, p := range s.Artifacts.Packages.Sorted() { + if p.Name != name { + continue + } + for _, loc := range p.Locations.ToSlice() { + out = append(out, loc.ArchivePath+"|"+loc.RealPath) + } + } + sort.Strings(out) + return out +} + +// javaVirtualPaths maps each java package name to the virtual paths it was reported under; a name +// with two entries was cataloged twice. +func javaVirtualPaths(s *sbom.SBOM) map[string][]string { + out := map[string][]string{} + for _, p := range s.Artifacts.Packages.Sorted() { + if metadata, ok := p.Metadata.(pkg.JavaArchive); ok { + out[p.Name] = append(out[p.Name], metadata.VirtualPath) + } + } + return out +} + +// markerCataloger is a stub package cataloger emitting one package per "marker.txt" it finds, so the +// end-to-end tests assert behavior without depending on any real cataloger's fixture format. +type markerCataloger struct{} + func (markerCataloger) Name() string { return "marker-cataloger" } func (markerCataloger) Catalog(_ context.Context, resolver file.Resolver) ([]pkg.Package, []artifact.Relationship, error) { @@ -557,13 +376,59 @@ func (markerCataloger) Catalog(_ context.Context, resolver file.Resolver) ([]pkg return pkgs, nil, nil } +// resolverProbeCataloger reports what the resolver it was handed can see, which is how "absent from +// the index" is told from "present and skipped" within a scan. +type resolverProbeCataloger struct { + globs []string + record func([]string) +} + +func (resolverProbeCataloger) Name() string { return "resolver-probe-cataloger" } + +func (c resolverProbeCataloger) Catalog(_ context.Context, resolver file.Resolver) ([]pkg.Package, []artifact.Relationship, error) { + locations, err := resolver.FilesByGlob(c.globs...) + if err != nil { + return nil, nil, err + } + var paths []string + for _, loc := range locations { + paths = append(paths, loc.RealPath) + } + c.record(paths) + return nil, nil, nil +} + +// recordingBus captures the progress rows a scan publishes. +type recordingBus struct { + mu sync.Mutex + events []partybus.Event +} + +func (p *recordingBus) Publish(e partybus.Event) { + p.mu.Lock() + defer p.mu.Unlock() + p.events = append(p.events, e) +} + +// scanDirWithExclusions scans with the given exclusion patterns configured on the source, where +// exclusions live; archive cataloging has no exclusion setting of its own. +func scanDirWithExclusions(t *testing.T, dir string, cfg *CreateSBOMConfig, exclude ...string) *sbom.SBOM { + t.Helper() + src, err := directorysource.New(directorysource.Config{ + Path: dir, + Exclude: source.ExcludeConfig{Paths: exclude}, + }) + require.NoError(t, err) + t.Cleanup(func() { _ = src.Close() }) + + s, err := cfg.Create(context.Background(), src) + require.NoError(t, err) + return s +} + func writeTestZip(t *testing.T, path string, files map[string]string) { t.Helper() - raw := make(map[string][]byte, len(files)) - for k, v := range files { - raw[k] = []byte(v) - } - writeTestZipRaw(t, path, raw) + require.NoError(t, os.WriteFile(path, buildZipBytes(t, files), 0o644)) } func writeTestZipRaw(t *testing.T, path string, files map[string][]byte) { @@ -602,98 +467,7 @@ func buildZipBytesRaw(t *testing.T, files map[string][]byte) []byte { return buf.Bytes() } -// archiveScanConfig builds a CreateSBOMConfig limited to the given catalogers, with nested archive -// cataloging at the given depth (0 disables it). -func archiveScanConfig(depth int, catalogers ...pkg.Cataloger) *CreateSBOMConfig { - refs := make([]pkgcataloging.CatalogerReference, 0, len(catalogers)) - for _, c := range catalogers { - refs = append(refs, pkgcataloging.NewCatalogerReference(c, []string{"directory"})) - } - cfg := DefaultCreateSBOMConfig().WithoutCatalogers().WithCatalogers(refs...) - cfg.Archive.MaxDepth = depth - return cfg -} - -// javaScanConfig builds a config whose java cataloger comes from the cataloging factory rather than -// pre-built: the flag handing recursion to the archive task is derived onto the factory's -// PackagesConfig, so a cataloger passed via WithCatalogers keeps its own config and never yields. -// -// It selects only the java-tagged catalogers, not the whole default set, which includes the RPM -// cataloger and its sqlite driver this test binary does not register. -func javaScanConfig(depth int) *CreateSBOMConfig { - cfg := DefaultCreateSBOMConfig(). - WithCatalogerSelection(cataloging.NewSelectionRequest().WithDefaults("java")) - if depth > 0 { - cfg = cfg.WithArchiveConfig(cataloging.DefaultArchiveSearchConfig().WithMaxDepth(depth)) - } - return cfg -} - -func scanDirWith(t *testing.T, dir string, cfg *CreateSBOMConfig) *sbom.SBOM { - t.Helper() - src, err := directorysource.New(directorysource.Config{Path: dir}) - require.NoError(t, err) - t.Cleanup(func() { _ = src.Close() }) - - s, err := cfg.Create(context.Background(), src) - require.NoError(t, err) - return s -} - -// scanDirWithExclusions scans with the given exclusion patterns configured on the source, where -// exclusions live; archive cataloging has no exclusion setting of its own. -func scanDirWithExclusions(t *testing.T, dir string, cfg *CreateSBOMConfig, exclude ...string) *sbom.SBOM { - t.Helper() - src, err := directorysource.New(directorysource.Config{ - Path: dir, - Exclude: source.ExcludeConfig{Paths: exclude}, - }) - require.NoError(t, err) - t.Cleanup(func() { _ = src.Close() }) - - s, err := cfg.Create(context.Background(), src) - require.NoError(t, err) - return s -} - -func jarBytes(t *testing.T, title, version string, extra map[string][]byte) []byte { - t.Helper() - entries := map[string][]byte{ - "META-INF/MANIFEST.MF": []byte( - "Manifest-Version: 1.0\nImplementation-Title: " + title + "\nImplementation-Version: " + version + "\n", - ), - } - for k, v := range extra { - entries[k] = v - } - return buildZipBytesRaw(t, entries) -} - -func (resolverProbeCataloger) Name() string { return "resolver-probe-cataloger" } - -func (c resolverProbeCataloger) Catalog(_ context.Context, resolver file.Resolver) ([]pkg.Package, []artifact.Relationship, error) { - locations, err := resolver.FilesByGlob(c.glob) - if err != nil { - return nil, nil, err - } - var paths []string - for _, loc := range locations { - paths = append(paths, loc.RealPath) - } - c.record(paths) - return nil, nil, nil -} - func buildTarGzBytes(t *testing.T, files map[string][]byte) []byte { - t.Helper() - var buf bytes.Buffer - gw := gzip.NewWriter(&buf) - writeTarEntries(t, gw, files) - require.NoError(t, gw.Close()) - return buf.Bytes() -} - -func writeTarEntries(t *testing.T, w io.Writer, files map[string][]byte) { t.Helper() names := make([]string, 0, len(files)) for name := range files { @@ -701,7 +475,9 @@ func writeTarEntries(t *testing.T, w io.Writer, files map[string][]byte) { } sort.Strings(names) - tw := tar.NewWriter(w) + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) for _, name := range names { body := files[name] require.NoError(t, tw.WriteHeader(&tar.Header{Name: name, Mode: 0o644, Size: int64(len(body))})) @@ -709,78 +485,16 @@ func writeTarEntries(t *testing.T, w io.Writer, files map[string][]byte) { require.NoError(t, err) } require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + return buf.Bytes() } -// recordingBus captures the progress rows a scan publishes. -type recordingBus struct { - mu sync.Mutex - events []partybus.Event -} - -func (p *recordingBus) Publish(e partybus.Event) { - p.mu.Lock() - defer p.mu.Unlock() - p.events = append(p.events, e) -} - -// TestArchiveCataloger_fileCatalogerProgressCountsArchiveContents scans a directory holding one file -// and a zip holding two more. The file catalogers run once over the scan root and again over the -// archive, and the row each of them publishes must be one row carrying the total of both - a row that -// stops at the scan root reports a number that is wrong by everything inside every archive. -func TestArchiveCataloger_fileCatalogerProgressCountsArchiveContents(t *testing.T) { - scanDir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(scanDir, "loose.txt"), []byte("outside"), 0o600)) - writeTestZip(t, filepath.Join(scanDir, "app.zip"), map[string]string{ - "nested/one.txt": "hello", - "nested/two.txt": "there", +// jarBytes is the smallest jar the java cataloger identifies: a manifest naming the artifact. +func jarBytes(t *testing.T, title, version string) []byte { + t.Helper() + return buildZipBytesRaw(t, map[string][]byte{ + "META-INF/MANIFEST.MF": []byte( + "Manifest-Version: 1.0\nImplementation-Title: " + title + "\nImplementation-Version: " + version + "\n", + ), }) - - recorder := &recordingBus{} - bus.Set(recorder) - t.Cleanup(func() { bus.Set(nil) }) - - src, err := directorysource.New(directorysource.Config{Path: scanDir}) - require.NoError(t, err) - t.Cleanup(func() { _ = src.Close() }) - - cfg := DefaultCreateSBOMConfig(). - WithoutCatalogers(). - WithCatalogers(pkgcataloging.NewCatalogerReference(markerCataloger{}, []string{"directory"})). - WithFilesConfig(filecataloging.DefaultConfig().WithSelection(file.AllFilesSelection)) - cfg.Archive.MaxDepth = 1 - - _, err = cfg.Create(context.Background(), src) - require.NoError(t, err) - - rows := map[string][]*progress.Manual{} - for _, e := range recorder.events { - if e.Type != event.CatalogerTaskStarted { - continue - } - info, ok := e.Source.(monitor.GenericTask) - require.True(t, ok) - prog, ok := e.Value.(progress.StagedProgressable) - require.True(t, ok) - tp, ok := prog.(*monitor.TaskProgress) - require.True(t, ok) - rows[info.Title.Default] = append(rows[info.Title.Default], tp.Manual) - } - - for _, title := range []string{"File digests", "File metadata"} { - published := rows[title] - require.Len(t, published, 1, "%s must publish exactly one row for the whole scan", title) - row := published[0] - assert.GreaterOrEqual(t, row.Current(), int64(4), - "%s must count the two files inside the archive as well as the two beside it, got %d", - title, row.Current()) - - // a re-run reports into the row it already owns, so the row's total has to grow with it. A total - // describing only the scan root leaves the count running past it, which renders as an overshot - // bar and reads as complete (progress.IsCompleted) while the walk is still going. - if row.Size() >= 0 { - assert.Equal(t, row.Current(), row.Size(), - "%s must carry a total covering every run that reported into it, got %d/%d", - title, row.Current(), row.Size()) - } - } }