fix(sbom): eight preservation defects review found, and the guard that missed one

Every one reproduced before it was fixed and fails a test after.

Identity and sources. Converting SPDX to CycloneDX adopted the source
namespace into the model, minted a fresh serial because CycloneDX has no
namespace slot, and then suppressed the source link by comparing against
the namespace -- so the export named its source neither way. The
comparison is now against the identity the format actually writes. A
document that asserted nothing about itself is also a source now: dropping
it made a merge of two documents look like a conversion of one and publish
the merged inventory under the other's identity.

A conversion keeps the source's creation time. It adopts the source's
identity, and claiming that identity while stating a different creation
time is two claims that disagree. The fixed-point test pinned the
timestamp, which is exactly what hid this; it pins nothing now.

Component data. A homepage vanished on any CycloneDX hop -- the format has
no homepage field, so it travels as the website reference CycloneDX offers
for the same claim, both ways. A document whose only component is its
primary one lost supplier, description, hashes, CPE and references, since
the ingest fallback never applied assertions. Configured provenance
overwrote a root component's own supplier: one names who supplied the
component, the other who produced the project.

Two components minting one canonical package URL discarded the second
along with everything it asserted. They fold through the SDK's InsertNode
now. The guard that exists to catch exactly this spelled the lookup as
`.Node(node.ID)`, so it never saw `.Node(packageID)` -- widened, which
immediately found a second site in explain.

An ingested organization disappeared even on a CycloneDX round trip: the
export filled the one manufacturer slot from configured provenance alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: bomly-guy

internal/detectors/guards_test.go

@@ -44,7 +44,13 @@ func walkInternalGo(t *testing.T, visit func(path, body string)) {
 func TestNodeInsertionGoesThroughTheSharedHelper(t *testing.T) {
 	// A lookup on the graph followed by an insert, which is the shape that
 	// silently discards the duplicate.
-	lookupThenAdd := regexp.MustCompile(`(?s)\.Node\(node\.ID\).{0,200}?\.AddNode\(`)
+	// Any receiver and any identifier, not the one variable name this rule was
+	// first written against. The regex used to spell the lookup as
+	// `.Node(node.ID)`, so internal/sbom's `.Node(packageID)` walked straight
+	// past it and silently discarded a duplicate component's assertions for as
+	// long as the guard has existed. A guard that only catches the shape you
+	// already fixed is not a guard.
+	lookupThenAdd := regexp.MustCompile(`(?s)\.Node\([A-Za-z_][\w.]*\).{0,200}?\.AddNode\(`)
 
 	var offenders []string
 	walkInternalGo(t, func(path, body string) {

internal/detectors/sbom/detector.go

@@ -100,9 +100,10 @@ func (d Detector) ResolveGraph(_ context.Context, req sdk.DetectionRequest) (sdk
 	graphs := sdk.SingleGraphContainer(depsGraph, detectorkit.InferManifestMetadata(req, evidencePatterns))
 	// What the document said about itself rides the entry it became, so a
 	// later export can restate it instead of crediting only Bomly for a
-	// document Bomly only converted (ADR-0037).
-	if assertions := doc.Assertions; !assertions.IsEmpty() && len(graphs.Entries) == 1 {
-		graphs.Entries[0].Document = &assertions
+	// document Bomly only converted (ADR-0037). The codec decides what that
+	// record contains, including for a document that asserted nothing.
+	if len(graphs.Entries) == 1 {
+		graphs.Entries[0].Document = sbom.DocumentAssertionsFor(doc)
 	}
 
 	logger.Debug("resolved explicit sbom file", zap.String("path", sbomPath), zap.String("format", string(target)))

internal/engine/explain/why.go

@@ -113,16 +113,19 @@ func GraphFromPaths(source *sdk.Graph, paths []Path) (*sdk.Graph, error) {
 	if source == nil {
 		return focused, nil
 	}
+	// Nodes repeat across paths -- a shared dependency is on every path that
+	// reaches it -- so insertion folds rather than skipping. Every witness here
+	// is a clone of one source node, so the fold is a no-op in practice; it is
+	// the shared entry point regardless, because "skipping is fine here" is a
+	// judgement each site otherwise re-makes on its own.
 	for _, path := range paths {
 		for i, ref := range path.Packages {
 			pkg, ok := source.Node(ref.ID)
 			if !ok || pkg == nil {
 				continue
 			}
-			if _, exists := focused.Node(pkg.NodeID()); !exists {
-				if err := focused.AddNode(pkg.CloneNode()); err != nil {
-					return nil, err
-				}
+			if _, err := focused.InsertNode(pkg.CloneNode()); err != nil {
+				return nil, err
 			}
 			if i == 0 {
 				continue
@@ -132,10 +135,8 @@ func GraphFromPaths(source *sdk.Graph, paths []Path) (*sdk.Graph, error) {
 			if !ok || parent == nil {
 				continue
 			}
-			if _, exists := focused.Node(parent.NodeID()); !exists {
-				if err := focused.AddNode(parent.CloneNode()); err != nil {
-					return nil, err
-				}
+			if _, err := focused.InsertNode(parent.CloneNode()); err != nil {
+				return nil, err
 			}
 			if err := focused.AddEdge(parent.NodeID(), pkg.NodeID()); err != nil && !errors.Is(err, sdk.ErrCycleDetected) {
 				return nil, err

internal/sbom/assertions_round_trip_test.go

@@ -48,16 +48,31 @@ func assertPreserved(t *testing.T, where string, component Component) {
 	if !strings.Contains(component.Description, "widgeting") {
 		t.Fatalf("%s: description = %q, want the source's", where, component.Description)
 	}
-	if len(component.Digests) == 0 {
-		t.Fatalf("%s: checksums lost", where)
+	// Values, not presence. A conversion that kept a checksum entry while
+	// changing its algorithm or digest, or kept a reference while relabelling
+	// its type, would satisfy a presence check and still have corrupted the
+	// claim -- and the algorithm names differ between the two formats, which
+	// is exactly where such a slip would hide.
+	var digest Digest
+	for _, candidate := range component.Digests {
+		if strings.EqualFold(strings.ReplaceAll(candidate.Algorithm, "-", ""), "sha256") {
+			digest = candidate
+		}
+	}
+	if digest.Value != "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" {
+		t.Fatalf("%s: sha-256 checksum = %+v, want the source's", where, component.Digests)
 	}
-	if len(component.CPEs) == 0 {
-		t.Fatalf("%s: CPE lost", where)
+	if len(component.CPEs) == 0 || component.CPEs[0] != "cpe:2.3:a:widget:widget:1.0.0:*:*:*:*:*:*:*" {
+		t.Fatalf("%s: CPE = %v, want the source's", where, component.CPEs)
 	}
 	var tracker bool
 	for _, ref := range component.ExternalReferences {
-		if strings.Contains(ref.Locator, "widgets.example/issues") {
-			tracker = true
+		if ref.Locator != "https://widgets.example/issues" {
+			continue
+		}
+		tracker = true
+		if !strings.EqualFold(ref.Type, "issue-tracker") {
+			t.Fatalf("%s: the issue-tracker reference was relabelled %q", where, ref.Type)
 		}
 	}
 	if !tracker {
@@ -147,3 +162,141 @@ func TestIngestLeavesComponentsEligibleForEnrichment(t *testing.T) {
 	}
 	_ = sdk.EcosystemUnknown
 }
+
+// A homepage survives a CycloneDX hop. The format has no homepage field, so it
+// travels as the website reference CycloneDX offers for the same claim -- and
+// used to travel nowhere at all, disappearing on any SPDX-to-CycloneDX
+// conversion.
+func TestHomepageSurvivesCycloneDX(t *testing.T) {
+	const spdxWithHomepage = `{
+  "spdxVersion": "SPDX-2.3", "dataLicense": "CC0-1.0", "SPDXID": "SPDXRef-DOCUMENT",
+  "name": "h", "documentNamespace": "https://h.example/spdx/1",
+  "creationInfo": {"created": "2026-01-01T00:00:00Z", "creators": ["Tool: t"]},
+  "packages": [{
+    "SPDXID": "SPDXRef-w", "name": "widget", "versionInfo": "1.0.0",
+    "homepage": "https://widget.example/",
+    "externalRefs": [{"referenceCategory": "PACKAGE-MANAGER", "referenceType": "purl",
+      "referenceLocator": "pkg:npm/widget@1.0.0"}]
+  }]
+}`
+	doc, _, err := UnmarshalAutoJSON([]byte(spdxWithHomepage))
+	if err != nil {
+		t.Fatalf("ingest: %v", err)
+	}
+	if got := componentNamed(t, doc, "widget").Homepage; got != "https://widget.example/" {
+		t.Fatalf("ingested homepage = %q", got)
+	}
+	graph, err := ToGraph(doc)
+	if err != nil {
+		t.Fatalf("to graph: %v", err)
+	}
+	cyclone, err := MarshalDepGraphJSON(graph, TargetCycloneDX16JSON, BuildOptions{}, EncodeOptions{Pretty: true})
+	if err != nil {
+		t.Fatalf("cyclonedx export: %v", err)
+	}
+	back, _, err := UnmarshalAutoJSON(cyclone)
+	if err != nil {
+		t.Fatalf("re-ingest: %v", err)
+	}
+	if got := componentNamed(t, back, "widget").Homepage; got != "https://widget.example/" {
+		t.Fatalf("homepage after the CycloneDX hop = %q\n%s", got, cyclone)
+	}
+}
+
+// A document whose only component is its primary one keeps that component's
+// assertions. This is legal CycloneDX, and the ingest fallback that handles it
+// used to read half the fields.
+func TestMetadataOnlyComponentKeepsItsAssertions(t *testing.T) {
+	const metadataOnly = `{
+  "bomFormat": "CycloneDX", "specVersion": "1.5", "version": 1,
+  "metadata": {"component": {
+    "bom-ref": "pkg:npm/solo@1.0.0", "type": "application", "name": "solo", "version": "1.0.0",
+    "purl": "pkg:npm/solo@1.0.0", "description": "the only component",
+    "supplier": {"name": "Solo Supply Co"},
+    "hashes": [{"alg": "SHA-256", "content": "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"}]
+  }}
+}`
+	doc, _, err := UnmarshalAutoJSON([]byte(metadataOnly))
+	if err != nil {
+		t.Fatalf("ingest: %v", err)
+	}
+	solo := componentNamed(t, doc, "solo")
+	if solo.Supplier == nil || solo.Supplier.Name != "Solo Supply Co" {
+		t.Errorf("supplier = %+v", solo.Supplier)
+	}
+	if !strings.Contains(solo.Description, "only component") {
+		t.Errorf("description = %q", solo.Description)
+	}
+	if len(solo.Digests) == 0 {
+		t.Error("the checksum was dropped")
+	}
+}
+
+// Two components that mint one canonical package URL fold into one node whose
+// assertions are the union of both, rather than the first one's alone.
+func TestDuplicateComponentsFoldTheirAssertions(t *testing.T) {
+	const duplicated = `{
+  "bomFormat": "CycloneDX", "specVersion": "1.5", "version": 1,
+  "components": [
+    {"bom-ref": "a", "type": "library", "name": "widget", "version": "1.0.0",
+     "purl": "pkg:npm/widget@1.0.0",
+     "externalReferences": [{"type": "issue-tracker", "url": "https://one.example/issues"}]},
+    {"bom-ref": "b", "type": "library", "name": "widget", "version": "1.0.0",
+     "purl": "pkg:npm/widget@1.0.0", "description": "the second says more",
+     "supplier": {"name": "Second Supply Co"},
+     "externalReferences": [{"type": "chat", "url": "https://two.example/chat"}]}
+  ]
+}`
+	doc, _, err := UnmarshalAutoJSON([]byte(duplicated))
+	if err != nil {
+		t.Fatalf("ingest: %v", err)
+	}
+	graph, err := ToGraph(doc)
+	if err != nil {
+		t.Fatalf("to graph: %v", err)
+	}
+	if graph.Size() != 1 {
+		t.Fatalf("size = %d, want the two components folded into one node", graph.Size())
+	}
+	node := graph.DependencyNodes()[0]
+	if !strings.Contains(node.Description, "second says more") {
+		t.Errorf("description = %q, want the second component's -- a gap the first left", node.Description)
+	}
+	if node.Supplier == nil || node.Supplier.Name != "Second Supply Co" {
+		t.Errorf("supplier = %+v, want the second component's", node.Supplier)
+	}
+	var tracker, chat bool
+	for _, ref := range node.ExternalReferences {
+		tracker = tracker || strings.Contains(ref.Locator, "one.example")
+		chat = chat || strings.Contains(ref.Locator, "two.example")
+	}
+	if !tracker || !chat {
+		t.Errorf("references = %+v, want the union of both components'", node.ExternalReferences)
+	}
+}
+
+// A root component's own supplier is not replaced by configured provenance:
+// one names who supplied the component, the other who produced the project.
+func TestConfiguredProvenanceDoesNotOverwriteARootSupplier(t *testing.T) {
+	doc, _, err := UnmarshalAutoJSON([]byte(supplierRichCycloneDX))
+	if err != nil {
+		t.Fatalf("ingest: %v", err)
+	}
+	// The lone component is the document's root.
+	raw, err := MarshalJSON(doc, TargetSPDX23JSON, EncodeOptions{Pretty: true})
+	if err != nil {
+		t.Fatalf("export: %v", err)
+	}
+	if !strings.Contains(string(raw), "Widget Supply Co") {
+		t.Fatalf("the source supplier is missing:\n%s", raw)
+	}
+
+	doc.Provenance = Provenance{Manufacturer: "Operator Ltd"}
+	withProvenance, err := MarshalJSON(doc, TargetSPDX23JSON, EncodeOptions{Pretty: true})
+	if err != nil {
+		t.Fatalf("export with provenance: %v", err)
+	}
+	if !strings.Contains(string(withProvenance), "Widget Supply Co") {
+		t.Errorf("configured provenance overwrote the component's own supplier:\n%s", withProvenance)
+	}
+}

internal/sbom/cyclonedx.go

@@ -68,9 +68,7 @@ func (c cycloneDXCodec) encodeJSON(doc *Document, opts EncodeOptions) ([]byte, e
 		}
 		metadata.Component = &primary
 	}
-	if doc.Provenance.Manufacturer != "" {
-		metadata.Manufacturer = &cdx.OrganizationalEntity{Name: doc.Provenance.Manufacturer}
-	}
+	metadata.Manufacturer = cycloneDXDocumentManufacturer(doc)
 	if authors := cycloneDXDocumentAuthors(doc); len(authors) > 0 {
 		metadata.Authors = &authors
 	}
@@ -163,7 +161,7 @@ func (c cycloneDXCodec) decodeJSON(data []byte) (*Document, error) {
 
 	if len(componentByID) == 0 && bom.Metadata != nil && bom.Metadata.Component != nil {
 		root := bom.Metadata.Component
-		componentByID[root.BOMRef] = Component{
+		component := Component{
 			ID:        root.BOMRef,
 			Name:      root.Name,
 			Org:       root.Group,
@@ -174,6 +172,12 @@ func (c cycloneDXCodec) decodeJSON(data []byte) (*Document, error) {
 			Copyright: root.Copyright,
 			Licenses:  parseCycloneDXLicenses(root.Licenses),
 		}
+		// The same assertions the inventory loop applies. A document whose
+		// only component is its primary one is legal, and reading it with
+		// half the fields was a silent hole: supplier, description, hashes,
+		// CPE and references all stopped here.
+		applyCycloneDXAssertions(&component, *root)
+		componentByID[root.BOMRef] = component
 	}
 
 	components := make([]Component, 0, len(componentByID))
@@ -476,7 +480,7 @@ func cycloneDXComponent(comp Component) cdx.Component {
 	// carries one list, so they concatenate; the emitted set is deduplicated
 	// by the SDK's reference identity before it is written.
 	refs := cycloneDXComponentReferences(comp)
-	refs = append(refs, cycloneDXEmittedReferences(comp.ExternalReferences)...)
+	refs = append(refs, cycloneDXEmittedReferences(cycloneDXComponentAssertedReferences(comp))...)
 	if len(refs) > 0 {
 		component.ExternalReferences = &refs
 	}
@@ -488,6 +492,17 @@ func cycloneDXComponent(comp Component) cdx.Component {
 	return component
 }
 
+// cycloneDXComponentAssertedReferences returns the references a source
+// document asserted about a component, plus the website reference its homepage
+// is carried in.
+func cycloneDXComponentAssertedReferences(comp Component) []sdk.ExternalReference {
+	refs := comp.ExternalReferences
+	if homepage, ok := cycloneDXHomepageReference(comp); ok {
+		refs = sdk.MergeExternalReferences(refs, []sdk.ExternalReference{homepage})
+	}
+	return refs
+}
+
 // cycloneDXLicenses renders a component's licenses into CycloneDX.
 //
 // The format offers three shapes and scores them differently: `license.id` is