Use package ID from decoded SBOMs when provided

internal/cmptest/location.go

@@ -2,14 +2,15 @@ package cmptest
 
 import (
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 
 	"github.com/anchore/syft/syft/file"
 )
 
 type LocationComparer func(x, y file.Location) bool
 
 func DefaultLocationComparer(x, y file.Location) bool {
-	return cmp.Equal(x.Coordinates, y.Coordinates) && cmp.Equal(x.AccessPath, y.AccessPath)
+	return cmp.Equal(x.Coordinates, y.Coordinates, cmpopts.IgnoreUnexported(file.Coordinates{})) && cmp.Equal(x.AccessPath, y.AccessPath)
 }
 
 func LocationComparerWithoutLayer(x, y file.Location) bool {

syft/format/common/spdxhelpers/to_format_model.go

@@ -268,21 +268,34 @@ func toRootPackage(s source.Description) *spdx.Package {
 }
 
 func toSPDXID(identifiable artifact.Identifiable) spdx.ElementID {
+	id := string(identifiable.ID())
+	if strings.HasPrefix(id, "SPDXRef-") {
+		// this is already an SPDX ID, no need to change it (except for the prefix)
+		return spdx.ElementID(helpers.SanitizeElementID(strings.TrimPrefix(id, "SPDXRef-")))
+	}
 	maxLen := 40
-	id := ""
 	switch it := identifiable.(type) {
 	case pkg.Package:
+		if strings.HasPrefix(id, "Package-") {
+			// this is already an SPDX ID, no need to change it
+			return spdx.ElementID(helpers.SanitizeElementID(id))
+		}
 		switch {
 		case it.Type != "" && it.Name != "":
-			id = fmt.Sprintf("Package-%s-%s-%s", it.Type, it.Name, it.ID())
+			id = fmt.Sprintf("Package-%s-%s-%s", it.Type, it.Name, id)
 		case it.Name != "":
-			id = fmt.Sprintf("Package-%s-%s", it.Name, it.ID())
+			id = fmt.Sprintf("Package-%s-%s", it.Name, id)
 		case it.Type != "":
-			id = fmt.Sprintf("Package-%s-%s", it.Type, it.ID())
+			id = fmt.Sprintf("Package-%s-%s", it.Type, id)
 		default:
-			id = fmt.Sprintf("Package-%s", it.ID())
+			id = fmt.Sprintf("Package-%s", id)
 		}
 	case file.Coordinates:
+		if strings.HasPrefix(id, "File-") {
+			// this is already an SPDX ID, no need to change it. Note: there is no way to reach this case today
+			// from within syft, however, this covers future cases where the ID can be overridden
+			return spdx.ElementID(helpers.SanitizeElementID(id))
+		}
 		p := ""
 		parts := strings.Split(it.RealPath, "/")
 		for i := len(parts); i > 0; i-- {
@@ -296,9 +309,7 @@ func toSPDXID(identifiable artifact.Identifiable) spdx.ElementID {
 			}
 			p = path.Join(part, p)
 		}
-		id = fmt.Sprintf("File-%s-%s", p, it.ID())
-	default:
-		id = string(identifiable.ID())
+		id = fmt.Sprintf("File-%s-%s", p, id)
 	}
 	// NOTE: the spdx library prepend SPDXRef-, so we don't do it here
 	return spdx.ElementID(helpers.SanitizeElementID(id))

syft/format/common/spdxhelpers/to_format_model_test.go

@@ -861,6 +861,34 @@ func Test_toSPDXID(t *testing.T) {
 			},
 			expected: "Package-npm-some-package",
 		},
+		{
+			name: "package with existing SPDX ID",
+			it: func() pkg.Package {
+				p := pkg.Package{
+					Type: pkg.NpmPkg,
+					Name: "some-package",
+				}
+				// SPDXRef- prefix is removed on decode (when everything is working as it should)
+				p.OverrideID("Package-npm-some-package-extra!")
+				return p
+			}(),
+			// note: we still sanitize out the "!" which is not allowed in SPDX IDs
+			expected: "Package-npm-some-package-extra",
+		},
+		{
+			name: "package with existing SPDX Ref",
+			it: func() pkg.Package {
+				p := pkg.Package{
+					Type: pkg.NpmPkg,
+					Name: "some-package",
+				}
+				// someone incorrectly added SPDXRef- prefix
+				p.OverrideID("SPDXRef-Package-npm-some-package-extra!")
+				return p
+			}(),
+			// note: we still sanitize out the "!" which is not allowed in SPDX IDs
+			expected: "Package-npm-some-package-extra",
+		},
 	}
 
 	for _, test := range tests {

syft/format/common/spdxhelpers/to_syft_model.go

@@ -509,7 +509,12 @@ func toSyftPackage(p *spdx.Package) pkg.Package {
 		Metadata: extractMetadata(p, info),
 	}
 
-	sP.SetID()
+	if p.PackageSPDXIdentifier != "" {
+		// always prefer the IDs from the SBOM over derived IDs
+		sP.OverrideID(artifact.ID(p.PackageSPDXIdentifier))
+	} else {
+		sP.SetID()
+	}
 
 	return *sP
 }

syft/format/common/spdxhelpers/to_syft_model_test.go

@@ -664,7 +664,7 @@ func Test_directPackageFiles(t *testing.T) {
 		Packages: []*spdx.Package{
 			{
 				PackageName:           "some-package",
-				PackageSPDXIdentifier: "1",
+				PackageSPDXIdentifier: "1", // important!
 				PackageVersion:        "1.0.5",
 				Files: []*spdx.File{
 					{
@@ -689,7 +689,7 @@ func Test_directPackageFiles(t *testing.T) {
 		Name:    "some-package",
 		Version: "1.0.5",
 	}
-	p.SetID()
+	p.OverrideID("1") // the same as the spdxID on the package element
 	f := file.Location{
 		LocationData: file.LocationData{
 			Coordinates: file.Coordinates{
@@ -730,3 +730,32 @@ func Test_directPackageFiles(t *testing.T) {
 
 	require.Equal(t, s, got)
 }
+
+func Test_useSPDXIdentifierOverDerivedSyftArtifactID(t *testing.T) {
+	doc := &spdx.Document{
+		SPDXVersion: "SPDX-2.3",
+		Packages: []*spdx.Package{
+			{
+				PackageName:           "some-package",
+				PackageSPDXIdentifier: "1", // important!
+				PackageVersion:        "1.0.5",
+				Files: []*spdx.File{
+					{
+						FileName:           "some-file",
+						FileSPDXIdentifier: "2",
+						Checksums: []spdx.Checksum{
+							{
+								Algorithm: "SHA1",
+								Value:     "a8d733c64f9123",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	s, err := ToSyftModel(doc)
+
+	assert.Nil(t, err)
+	assert.NotNil(t, s.Artifacts.Packages.Package("1"))
+}

syft/format/cyclonedxjson/encoder_test.go

@@ -3,6 +3,7 @@ package cyclonedxjson
 import (
 	"bytes"
 	"flag"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -116,6 +117,14 @@ func TestCycloneDxImageEncoder(t *testing.T) {
 func redactor(values ...string) testutil.Redactor {
 	return testutil.NewRedactions().
 		WithValuesRedacted(values...).
+		WithPatternRedactorSpec(
+			testutil.PatternReplacement{
+				// only the source component bom-ref (not package or other component bom-refs)
+				Search:  regexp.MustCompile(`"component": \{[^}]*"bom-ref":\s*"(?P<redact>.+)"[^}]*}`),
+				Groups:  []string{"redact"}, // use the regex to anchore the search, but only replace bytes within the capture group
+				Replace: "redacted",
+			},
+		).
 		WithPatternRedactors(
 			map[string]string{
 				// UUIDs
@@ -126,9 +135,6 @@ func redactor(values ...string) testutil.Redactor {
 
 				// image hashes
 				`sha256:[A-Fa-f0-9]{64}`: `sha256:redacted`,
-
-				// BOM refs
-				`"bom-ref":\s*"[^"]+"`: `"bom-ref":"redacted"`,
 			},
 		)
 }

syft/format/cyclonedxjson/test-fixtures/snapshot/TestCycloneDxDirectoryEncoder.golden

@@ -17,14 +17,14 @@
       ]
     },
     "component": {
-      "bom-ref":"redacted",
+      "bom-ref": "redacted",
       "type": "file",
       "name": "some/path"
     }
   },
   "components": [
     {
-      "bom-ref":"redacted",
+      "bom-ref": "4dd25c6ee16b729a",
       "type": "library",
       "name": "package-1",
       "version": "1.0.1",
@@ -61,7 +61,7 @@
       ]
     },
     {
-      "bom-ref":"redacted",
+      "bom-ref": "pkg:deb/debian/package-2@2.0.1?package-id=39392bb5e270f669",
       "type": "library",
       "name": "package-2",
       "version": "2.0.1",
@@ -91,7 +91,7 @@
       ]
     },
     {
-      "bom-ref":"redacted",
+      "bom-ref": "os:debian@1.2.3",
       "type": "operating-system",
       "name": "debian",
       "version": "1.2.3",

syft/format/cyclonedxjson/test-fixtures/snapshot/TestCycloneDxImageEncoder.golden

@@ -17,15 +17,15 @@
       ]
     },
     "component": {
-      "bom-ref":"redacted",
+      "bom-ref": "redacted",
       "type": "container",
       "name": "user-image-input",
       "version": "sha256:redacted"
     }
   },
   "components": [
     {
-      "bom-ref":"redacted",
+      "bom-ref": "72567175418f73f8",
       "type": "library",
       "name": "package-1",
       "version": "1.0.1",
@@ -66,7 +66,7 @@
       ]
     },
     {
-      "bom-ref":"redacted",
+      "bom-ref": "pkg:deb/debian/package-2@2.0.1?package-id=4b756c6f6fb127a3",
       "type": "library",
       "name": "package-2",
       "version": "2.0.1",
@@ -100,7 +100,7 @@
       ]
     },
     {
-      "bom-ref":"redacted",
+      "bom-ref": "os:debian@1.2.3",
       "type": "operating-system",
       "name": "debian",
       "version": "1.2.3",

syft/format/cyclonedxxml/encoder_test.go

@@ -90,16 +90,24 @@ func TestCycloneDxImageEncoder(t *testing.T) {
 func redactor(values ...string) testutil.Redactor {
 	return testutil.NewRedactions().
 		WithValuesRedacted(values...).
+		WithPatternRedactorSpec(
+			testutil.PatternReplacement{
+				// only the source component bom-ref (not package or other component bom-refs)
+				Search:  regexp.MustCompile(`<component bom-ref="(?P<redact>[^"]*)" type="file">`),
+				Groups:  []string{"redact"}, // use the regex to anchore the search, but only replace bytes within the capture group
+				Replace: "redacted",
+			},
+		).
 		WithPatternRedactors(
 			map[string]string{
 				// dates
 				`([0-9]+)-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])[Tt]([01][0-9]|2[0-3]):([0-5][0-9]):([0-5][0-9]|60)(\.[0-9]+)?(([Zz])|([+|\-]([01][0-9]|2[0-3]):[0-5][0-9]))`: `redacted`,
 
-				// image hashes and BOM refs
+				// image hashes
 				`sha256:[A-Za-z0-9]{64}`: `sha256:redacted`,
 
-				// serial numbers and BOM refs
-				`(serialNumber|bom-ref)="[^"]+"`: `$1="redacted"`,
+				// serial numbers
+				`(serialNumber)="[^"]+"`: `$1="redacted"`,
 			},
 		)
 }

syft/format/cyclonedxxml/test-fixtures/snapshot/TestCycloneDxDirectoryEncoder.golden

@@ -16,7 +16,7 @@
     </component>
   </metadata>
   <components>
-    <component bom-ref="redacted" type="library">
+    <component bom-ref="4dd25c6ee16b729a" type="library">
       <name>package-1</name>
       <version>1.0.1</version>
       <licenses>
@@ -34,7 +34,7 @@
         <property name="syft:location:0:path">/some/path/pkg1</property>
       </properties>
     </component>
-    <component bom-ref="redacted" type="library">
+    <component bom-ref="pkg:deb/debian/package-2@2.0.1?package-id=39392bb5e270f669" type="library">
       <name>package-2</name>
       <version>2.0.1</version>
       <cpe>cpe:2.3:*:some:package:2:*:*:*:*:*:*:*</cpe>
@@ -47,7 +47,7 @@
         <property name="syft:metadata:installedSize">0</property>
       </properties>
     </component>
-    <component bom-ref="redacted" type="operating-system">
+    <component bom-ref="os:debian@1.2.3" type="operating-system">
       <name>debian</name>
       <version>1.2.3</version>
       <description>debian</description>

syft/format/cyclonedxxml/test-fixtures/snapshot/TestCycloneDxImageEncoder.golden

@@ -11,13 +11,13 @@
         </component>
       </components>
     </tools>
-    <component bom-ref="redacted" type="container">
+    <component bom-ref="f28a4ba3ddfdddad" type="container">
       <name>user-image-input</name>
       <version>sha256:redacted</version>
     </component>
   </metadata>
   <components>
-    <component bom-ref="redacted" type="library">
+    <component bom-ref="72567175418f73f8" type="library">
       <name>package-1</name>
       <version>1.0.1</version>
       <licenses>
@@ -36,7 +36,7 @@
         <property name="syft:location:0:path">/somefile-1.txt</property>
       </properties>
     </component>
-    <component bom-ref="redacted" type="library">
+    <component bom-ref="pkg:deb/debian/package-2@2.0.1?package-id=4b756c6f6fb127a3" type="library">
       <name>package-2</name>
       <version>2.0.1</version>
       <cpe>cpe:2.3:*:some:package:2:*:*:*:*:*:*:*</cpe>
@@ -50,7 +50,7 @@
         <property name="syft:metadata:installedSize">0</property>
       </properties>
     </component>
-    <component bom-ref="redacted" type="operating-system">
+    <component bom-ref="os:debian@1.2.3" type="operating-system">
       <name>debian</name>
       <version>1.2.3</version>
       <description>debian</description>