fix: name nested files in SPDX by their archive chain

SPDX wrote `FileName` from `RealPath` only, so two archives' `META-INF/MANIFEST.MF` became two SPDX files with the same name and nothing saying which archive each came from. Worse, a consumer reading just the name would take a nested file for one at that path in the scan root.

- a file inside an archive is now named by its full chain, using the same `:` convention and escaping as `ArchivePath` / java's `VirtualPath`: `app.war:WEB-INF/lib/a.jar:META-INF/MANIFEST.MF`
- `FileComment` gains an `archivePath: ...` line (next to the existing `layerID: ...`, one per line), so decoding recovers `ArchivePath` and the entry's own `RealPath` without guessing where the chain ends
- files outside archives are unchanged

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

internal/archive/traversal.go

@@ -52,12 +52,20 @@ func VirtualPath(loc file.Location) string {
 	return loc.ArchivePath + ":" + strings.TrimPrefix(escapeEntryPath(loc.Path()), "/")
 }
 
-var entryPathEscaper = strings.NewReplacer("%", "%25", ":", "%3A")
+var (
+	entryPathEscaper   = strings.NewReplacer("%", "%25", ":", "%3A")
+	entryPathUnescaper = strings.NewReplacer("%3A", ":", "%25", "%")
+)
 
 func escapeEntryPath(p string) string {
 	return entryPathEscaper.Replace(p)
 }
 
+// UnescapeEntryPath reverses the escaping VirtualPath applies to one segment of the chain.
+func UnescapeEntryPath(p string) string {
+	return entryPathUnescaper.Replace(p)
+}
+
 type traversalCtxKey struct{}
 
 func WithTraversal(ctx context.Context, t *Traversal) context.Context {

syft/format/common/spdxhelpers/to_format_model.go

@@ -14,6 +14,7 @@ import (
 	"github.com/spdx/tools-golang/spdx"
 
 	"github.com/anchore/packageurl-go"
+	"github.com/anchore/syft/internal/archive"
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/internal/mimetype"
 	"github.com/anchore/syft/internal/relationship"
@@ -677,6 +678,12 @@ func lookupRelationship(ty artifact.RelationshipType) (bool, helpers.Relationshi
 	return false, "", ""
 }
 
+// file comments carry what an SPDX file has no field for, one "key: value" per line
+const (
+	layerIDCommentPrefix     = "layerID: "
+	archivePathCommentPrefix = "archivePath: "
+)
+
 func toFiles(s sbom.SBOM) (results []*spdx.File) {
 	artifacts := s.Artifacts
 
@@ -707,17 +714,27 @@ func toFiles(s sbom.SBOM) (results []*spdx.File) {
 
 		// TODO: add file classifications (?) and content as a snippet
 
-		var comment string
+		var comments []string
 		if c.FileSystemID != "" {
-			comment = fmt.Sprintf("layerID: %s", c.FileSystemID)
+			comments = append(comments, layerIDCommentPrefix+c.FileSystemID)
 		}
 
-		relativePath, err := convertAbsoluteToRelative(c.RealPath)
+		// a file inside an archive is named by its full chain ("app.war:WEB-INF/lib/a.jar:META-INF/MANIFEST.MF"),
-		if err != nil {
+		// so a consumer reading only the name does not mistake it for a file at that path in the scan
-			log.Debugf("unable to convert relative path '%s' to absolute path: %s", c.RealPath, err)
+		// root. The archive path is also kept in the comment so decoding need not guess where it ends.
-			relativePath = c.RealPath
+		name := c.RealPath
+		if c.ArchivePath != "" {
+			name = archive.VirtualPath(file.NewLocationFromCoordinates(c))
+			comments = append(comments, archivePathCommentPrefix+c.ArchivePath)
 		}
 
+		relativePath, err := convertAbsoluteToRelative(name)
+		if err != nil {
+			log.Debugf("unable to convert relative path '%s' to absolute path: %s", name, err)
+			relativePath = name
+		}
+		comment := strings.Join(comments, "\n")
+
 		results = append(results, &spdx.File{
 			FileSPDXIdentifier: toSPDXID(c),
 			FileComment:        comment,

syft/format/common/spdxhelpers/to_format_model_test.go

@@ -1131,3 +1131,29 @@ func Test_otherLicenses(t *testing.T) {
 		})
 	}
 }
+
+func Test_toFiles_nestedFilesAreNamedByTheirChainAndRoundTrip(t *testing.T) {
+	// two archives' META-INF/MANIFEST.MF must not come out as two files with the same name and nothing
+	// saying which archive each is in
+	root := file.Coordinates{RealPath: "/etc/os-release", FileSystemID: "sha256:layer"}
+	inA := file.Coordinates{RealPath: "META-INF/MANIFEST.MF", ArchivePath: "/app.war:WEB-INF/lib/a.jar", FileSystemID: "sha256:layer"}
+	inB := file.Coordinates{RealPath: "odd:na%me.txt", ArchivePath: "/app.war:WEB-INF/lib/b.jar"}
+	s := sbom.SBOM{Artifacts: sbom.Artifacts{FileMetadata: map[file.Coordinates]file.Metadata{root: {}, inA: {}, inB: {}}}}
+
+	byName := map[string]*spdx.File{}
+	for _, f := range toFiles(s) {
+		byName[f.FileName] = f
+	}
+	require.Len(t, byName, 3)
+
+	assert.Equal(t, "layerID: sha256:layer", byName["etc/os-release"].FileComment, "files outside archives are unchanged")
+
+	a := byName["app.war:WEB-INF/lib/a.jar:META-INF/MANIFEST.MF"]
+	require.NotNil(t, a)
+	assert.Equal(t, "layerID: sha256:layer\narchivePath: /app.war:WEB-INF/lib/a.jar", a.FileComment)
+	assert.Equal(t, inA, toSyftCoordinates(a))
+
+	b := byName["app.war:WEB-INF/lib/b.jar:odd%3Ana%25me.txt"]
+	require.NotNil(t, b, "names: %v", byName)
+	assert.Equal(t, inB, toSyftCoordinates(b), "the entry path's escaping is undone on decode")
+}

syft/format/common/spdxhelpers/to_syft_model.go

@@ -15,6 +15,7 @@ import (
 	"github.com/spdx/tools-golang/spdx/v2/common"
 
 	"github.com/anchore/packageurl-go"
+	"github.com/anchore/syft/internal/archive"
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/internal/spdxlicense"
 	"github.com/anchore/syft/syft/artifact"
@@ -470,23 +471,28 @@ func collectPackageFileRelationships(spdxIDMap map[string]any, doc *spdx.Documen
 }
 
 func toSyftCoordinates(f *spdx.File) file.Coordinates {
-	const layerIDPrefix = "layerID: "
+	c := file.Coordinates{RealPath: f.FileName}
-	var fileSystemID string
+	for _, line := range strings.Split(f.FileComment, "\n") {
-	if strings.HasPrefix(f.FileComment, layerIDPrefix) {
+		if v, ok := strings.CutPrefix(line, layerIDCommentPrefix); ok {
-		fileSystemID = strings.TrimPrefix(f.FileComment, layerIDPrefix)
+			c.FileSystemID = v
+		}
+		if v, ok := strings.CutPrefix(line, archivePathCommentPrefix); ok {
+			c.ArchivePath = v
+		}
 	}
-	if strings.HasPrefix(string(f.FileSPDXIdentifier), layerIDPrefix) {
+	if v, ok := strings.CutPrefix(string(f.FileSPDXIdentifier), layerIDCommentPrefix); ok {
-		fileSystemID = strings.TrimPrefix(string(f.FileSPDXIdentifier), layerIDPrefix)
+		c.FileSystemID = v
 	}
-	return file.Coordinates{
+	if c.ArchivePath != "" {
-		RealPath:     f.FileName,
+		// the name is the chain; the file's own path is its last, escaped, segment
-		FileSystemID: fileSystemID,
+		c.RealPath = archive.UnescapeEntryPath(f.FileName[strings.LastIndexByte(f.FileName, ':')+1:])
 	}
+	return c
 }
 
 func toSyftLocation(f *spdx.File) file.Location {
-	l := file.NewVirtualLocationFromCoordinates(toSyftCoordinates(f), f.FileName)
+	c := toSyftCoordinates(f)
-	return l
+	return file.NewVirtualLocationFromCoordinates(c, c.RealPath)
 }
 
 func requireAndTrimPrefix(val any, prefix string) string {