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

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

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 {

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