From 2fa7282abca8dee6fe8a35bddea02b861565ce71 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Wed, 21 Oct 2026 11:39:55 -0400 Subject: [PATCH] fix: let the last archive entry at a path win When an archive held two entries at one path, the resolver kept the first, and this was documented as intended. Everything that actually unpacks the archive keeps the last: `tar` (and `tar -r` appends updates), OCI layer application, and the legacy java path's `ContentsFromZip`. So a jar could carry a `pom.properties` reporting 2.17.1 ahead of one installing 2.14.0, and the SBOM would show the former. - a later regular file, link or other non-directory entry now replaces the earlier one, refunding its in-memory bytes - a file/directory clash at one path still keeps the first, since swapping kinds would mean re-parenting what is already under it Signed-off-by: Alex Goodman --- internal/archive/README.md | 4 ++-- internal/archive/resolver.go | 35 ++++++++++++++++++++++++++++--- internal/archive/resolver_test.go | 32 +++++++++++++++++++++++++--- 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/internal/archive/README.md b/internal/archive/README.md index ef7d9dbaf..c58df8d27 100644 --- a/internal/archive/README.md +++ b/internal/archive/README.md @@ -67,8 +67,8 @@ PATH_MAX are refused, and nesting stops at the task's Entries are indexed in a stereoscope `filetree.FileTree` + `filetree.Index` and searched with `filetree.SearchContext`, the same index the directory and image resolvers use. Paths are archive-relative with no leading slash; a leading slash on a query is tolerated. Directories implied -by entry paths exist for `HasPath` but are never files. The first entry at a path wins over later -duplicates. +by entry paths exist for `HasPath` but are never files. The last entry at a path wins over earlier +duplicates, as it does on extraction; a file and a directory at one path keep the first. Links resolve inside the archive only, as the file tree resolves them: a hard link names its target from the archive root. A lookup matching a link and its target returns one location, with diff --git a/internal/archive/resolver.go b/internal/archive/resolver.go index 5f0cd9286..fbd499937 100644 --- a/internal/archive/resolver.go +++ b/internal/archive/resolver.go @@ -99,13 +99,17 @@ func approxIndexBytes(hdr tar.Header) int64 { // add stores and indexes one entry, reading its content in full now since archive readers are // sequential. Reaching a limit stores nothing for this entry and returns ErrDiskLimitReached; entries -// added before it remain usable. The first entry at a path wins. +// added before it remain usable. The last entry at a path wins, as it does when the archive is +// extracted (tar appends updates, an OCI layer applies in order, and java's legacy zip reading +// overwrites too), so a stale earlier copy, such as an older pom.properties, can't stand in for what is +// really there. func (r *Resolver) add(hdr tar.Header, content io.Reader) error { entryPath := path.Clean("/" + hdr.Name) if entryPath == "/" { return nil } - if exists, res, _ := r.tree.File(stereoscopeFile.Path(entryPath)); exists && res.HasReference() { + replacing, ok := r.replace(entryPath, hdr) + if !ok { return nil } @@ -158,12 +162,37 @@ func (r *Resolver) add(hdr tar.Header, content io.Reader) error { log.WithFields("entry", hdr.Name, "error", err).Trace("skipping archive entry") return nil } - if hdr.Typeflag != tar.TypeDir { + if hdr.Typeflag != tar.TypeDir && !replacing { r.entries++ } return nil } +// replace clears the way for an entry at a path that is already held, reporting whether it did and +// false when the entry should be dropped. A later non-directory entry replaces an earlier one, refunding +// what it held. A file and a directory at one path keep the first, since swapping kinds would mean +// re-parenting what is already beneath it. +func (r *Resolver) replace(entryPath string, hdr tar.Header) (replacing, ok bool) { + exists, res, _ := r.tree.File(stereoscopeFile.Path(entryPath)) + if !exists || !res.HasReference() { + return false, true + } + earlier, err := r.index.Get(*res.Reference) + if err != nil || earlier.IsDir() || hdr.Typeflag == tar.TypeDir { + return false, false + } + // a removed node's index entry is left behind, but searches only answer with references in the tree + if err := r.tree.RemovePath(stereoscopeFile.Path(entryPath)); err != nil { + log.WithFields("entry", hdr.Name, "error", err).Trace("unable to replace archive entry") + return false, false + } + if b := r.content[res.ID()]; b != nil { + r.discard(b) + } + delete(r.content, res.ID()) + return true, true +} + func (r *Resolver) store(rooted string, hdr tar.Header, b *blob) error { ref, err := r.addToTree(rooted, hdr) if err != nil { diff --git a/internal/archive/resolver_test.go b/internal/archive/resolver_test.go index 33299df36..e39331f4f 100644 --- a/internal/archive/resolver_test.go +++ b/internal/archive/resolver_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + stereoscopeFile "github.com/anchore/stereoscope/pkg/file" "github.com/anchore/syft/syft/file" ) @@ -83,15 +84,40 @@ func TestResolver_add_indexMakesRoomBySpillingHeldContent(t *testing.T) { assert.Equal(t, strings.Repeat("a", 100), readStore(t, r)["a.txt"].body) } -func TestResolver_add_firstEntryAtAPathWins(t *testing.T) { +func TestResolver_add_lastEntryAtAPathWins(t *testing.T) { r, _ := resolverIn(t, memCharge(1<<20)) require.NoError(t, r.add(regularHeader("a.txt", 5), bytes.NewReader([]byte("first")))) require.NoError(t, r.add(regularHeader("a.txt", 6), bytes.NewReader([]byte("second")))) + require.NoError(t, r.add(regularHeader("b.txt", 1), bytes.NewReader([]byte("b")))) + require.NoError(t, r.add(tar.Header{Name: "b.txt", Typeflag: tar.TypeSymlink, Linkname: "a.txt"}, nil)) r.finish() - assert.Equal(t, "first", readStore(t, r)["a.txt"].body) - assert.Equal(t, 1, r.entries) + assert.Equal(t, "second", readStore(t, r)["a.txt"].body) + assert.Equal(t, "a.txt", readStore(t, r)["b.txt"].linkTarget, "a later entry may change kind") + assert.Equal(t, 2, r.entries) + assert.Equal(t, int64(len("second")), r.heldInMemory(), "the earlier copy is refunded") + + locations, err := r.FilesByGlob("**/a.txt") + require.NoError(t, err) + require.Len(t, locations, 1) + reader, err := r.FileContentsByLocation(locations[0]) + require.NoError(t, err) + assert.Equal(t, "second", readEntry(t, reader.(entryReader).ReaderAtSeeker)) +} + +func TestResolver_add_fileAndDirectoryAtOnePathKeepsTheFirst(t *testing.T) { + r, _ := resolverIn(t, memCharge(1<<20)) + + require.NoError(t, r.add(regularHeader("a/b", 1), bytes.NewReader([]byte("x")))) + require.NoError(t, r.add(regularHeader("a", 4), bytes.NewReader([]byte("file")))) + require.NoError(t, r.add(regularHeader("c", 4), bytes.NewReader([]byte("file")))) + require.NoError(t, r.add(tar.Header{Name: "c", Typeflag: tar.TypeDir, Mode: 0o755}, nil)) + r.finish() + + got := readStore(t, r) + assert.Equal(t, stereoscopeFile.TypeDirectory, got["a"].kind) + assert.Equal(t, "file", got["c"].body) } func TestResolver_pathsAndContents(t *testing.T) {