fix(sbom): make package SPDX ids unique per document

src/fromager/sbom.py

@@ -10,6 +10,7 @@
 import json
 import logging
 import pathlib
+import re
 import typing
 import uuid
 from datetime import UTC, datetime
@@ -29,6 +30,19 @@
 SBOM_FILENAME = "fromager.spdx.json"
 
 
+def _spdx_id(kind: str, name: str, version: str, doc_suffix: str) -> str:
+    """Build a package SPDX id unique to this package and document.
+
+    SPDX ids only need to be unique within a document, but tools that read
+    several SBOMs (e.g. syft scanning an environment) merge packages by id.
+    Name and version keep different wheels apart; the per-document suffix
+    keeps apart two builds of the same version that may carry different
+    data. SPDX ids only allow letters, digits, ``.`` and ``-``.
+    """
+    spdx_id = f"SPDXRef-{kind}-{name}-{version}-{doc_suffix}"
+    return re.sub(r"[^A-Za-z0-9.\-]+", "-", spdx_id)
+
+
 def _build_downstream_purl(
     *,
     name: NormalizedName,
@@ -109,6 +123,10 @@ def generate_sbom(
     package entry, its purl, and the document identity use
     ``wheel_version`` when given; the upstream package entry always
     uses ``version``.
+
+    Package SPDX ids include a per-document suffix and are not predictable,
+    so consumers should find the wheel package through the document's
+    DESCRIBES relationship rather than by a fixed id.
     """
     sbom_settings = ctx.settings.sbom_settings
     if sbom_settings is None:
@@ -143,13 +161,19 @@ def generate_sbom(
     # contain URI-hostile characters.
     doc_name = downstream.name
     doc_version = downstream.version or str(wheel_version or version)
+    doc_uuid = uuid.uuid4()
     namespace = (
         f"{sbom_settings.namespace!s}/{quote(doc_name, safe='')}"
-        f"-{quote(doc_version, safe='')}-{uuid.uuid4()}.spdx.json"
+        f"-{quote(doc_version, safe='')}-{doc_uuid}.spdx.json"
     )
 
+    upstream_version = upstream.version or str(version)
+    doc_suffix = doc_uuid.hex[:8]
+    wheel_id = _spdx_id("wheel", doc_name, doc_version, doc_suffix)
+    upstream_id = _spdx_id("upstream", upstream.name, upstream_version, doc_suffix)
+
     wheel_entry: dict[str, typing.Any] = {
-        "SPDXID": "SPDXRef-wheel",
+        "SPDXID": wheel_id,
         "name": doc_name,
         "versionInfo": doc_version,
         "downloadLocation": "NOASSERTION",
@@ -164,9 +188,9 @@ def generate_sbom(
     }
 
     upstream_entry: dict[str, typing.Any] = {
-        "SPDXID": "SPDXRef-upstream",
+        "SPDXID": upstream_id,
         "name": upstream.name,
-        "versionInfo": upstream.version or str(version),
+        "versionInfo": upstream_version,
         "downloadLocation": "NOASSERTION",
         "supplier": "NOASSERTION",
         "externalRefs": [
@@ -193,12 +217,12 @@ def generate_sbom(
             {
                 "spdxElementId": "SPDXRef-DOCUMENT",
                 "relationshipType": "DESCRIBES",
-                "relatedSpdxElement": "SPDXRef-wheel",
+                "relatedSpdxElement": wheel_id,
             },
             {
-                "spdxElementId": "SPDXRef-wheel",
+                "spdxElementId": wheel_id,
                 "relationshipType": "GENERATED_FROM",
-                "relatedSpdxElement": "SPDXRef-upstream",
+                "relatedSpdxElement": upstream_id,
             },
         ],
     }

tests/test_sbom.py

@@ -1,5 +1,6 @@
 import json
 import pathlib
+import re
 import typing
 import uuid
 from unittest.mock import patch
@@ -391,14 +392,97 @@ def test_generate_sbom_relationships(tmp_path: pathlib.Path) -> None:
         version=Version("0.1.0"),
     )
 
+    wheel_id, upstream_id = (p["SPDXID"] for p in doc["packages"])
     rels = doc["relationships"]
     assert len(rels) == 2
     assert rels[0]["spdxElementId"] == "SPDXRef-DOCUMENT"
     assert rels[0]["relationshipType"] == "DESCRIBES"
-    assert rels[0]["relatedSpdxElement"] == "SPDXRef-wheel"
-    assert rels[1]["spdxElementId"] == "SPDXRef-wheel"
+    assert rels[0]["relatedSpdxElement"] == wheel_id
+    assert rels[1]["spdxElementId"] == wheel_id
     assert rels[1]["relationshipType"] == "GENERATED_FROM"
-    assert rels[1]["relatedSpdxElement"] == "SPDXRef-upstream"
+    assert rels[1]["relatedSpdxElement"] == upstream_id
+    _validate_spdx(doc)
+
+
+def test_generate_sbom_spdx_ids_name_package_and_document(
+    tmp_path: pathlib.Path,
+) -> None:
+    """Verify package SPDX ids carry name, version, and a document suffix."""
+    ctx = make_sbom_ctx(tmp_path, sbom_settings=SbomSettings())
+    fixed_uuid = uuid.UUID("12345678-1234-5678-1234-567812345678")
+    with patch("fromager.sbom.uuid.uuid4", return_value=fixed_uuid):
+        doc = sbom.generate_sbom(
+            ctx=ctx,
+            req=Requirement("test==0.1.0"),
+            version=Version("0.1.0"),
+        )
+
+    assert [p["SPDXID"] for p in doc["packages"]] == [
+        "SPDXRef-wheel-test-0.1.0-12345678",
+        "SPDXRef-upstream-test-0.1.0-12345678",
+    ]
+    _validate_spdx(doc)
+
+
+def test_generate_sbom_spdx_ids_differ_between_packages(
+    tmp_path: pathlib.Path,
+) -> None:
+    """Verify SBOMs of different wheels share no package SPDX ids.
+
+    Tools such as syft merge packages from several SBOMs by SPDX id, so
+    shared ids would attribute one wheel's data to another. Both documents
+    get the same UUID so only the name and version can keep the ids apart.
+    """
+    ctx = make_sbom_ctx(tmp_path, sbom_settings=SbomSettings())
+    fixed_uuid = uuid.UUID("12345678-1234-5678-1234-567812345678")
+    with patch("fromager.sbom.uuid.uuid4", return_value=fixed_uuid):
+        docs = [
+            sbom.generate_sbom(ctx=ctx, req=Requirement(req), version=Version(ver))
+            for req, ver in [("idna==3.6", "3.6"), ("packaging==26.3", "26.3")]
+        ]
+
+    ids = [{p["SPDXID"] for p in doc["packages"]} for doc in docs]
+    assert ids[0].isdisjoint(ids[1])
+
+
+def test_generate_sbom_spdx_ids_differ_between_documents(
+    tmp_path: pathlib.Path,
+) -> None:
+    """Verify two SBOMs of the same package share no package SPDX ids.
+
+    Two builds of one version (e.g. for different variants) can carry
+    different data; shared ids would let tools merge them into one package.
+    """
+    ctx = make_sbom_ctx(tmp_path, sbom_settings=SbomSettings())
+    docs = [
+        sbom.generate_sbom(
+            ctx=ctx, req=Requirement("idna==3.6"), version=Version("3.6")
+        )
+        for _ in range(2)
+    ]
+
+    ids = [{p["SPDXID"] for p in doc["packages"]} for doc in docs]
+    assert ids[0].isdisjoint(ids[1])
+
+
+def test_generate_sbom_spdx_ids_are_sanitized(tmp_path: pathlib.Path) -> None:
+    """Verify characters SPDX ids do not allow are replaced."""
+    ctx = make_sbom_ctx(
+        tmp_path,
+        sbom_settings=SbomSettings(),
+        package_overrides={"purl": {"version": "0.5.5+local.1"}},
+    )
+    doc = sbom.generate_sbom(
+        ctx=ctx,
+        req=Requirement("test-pkg==0.5.5"),
+        version=Version("0.5.5"),
+    )
+
+    wheel_id, upstream_id = (p["SPDXID"] for p in doc["packages"])
+    assert re.fullmatch(
+        r"SPDXRef-wheel-test-pkg-0\.5\.5-local\.1-[0-9a-f]{8}", wheel_id
+    )
+    assert re.fullmatch(r"SPDXRef-upstream-test-pkg-0\.5\.5-[0-9a-f]{8}", upstream_id)
     _validate_spdx(doc)