From e9121a7933da4cf494d6668448d053bde05ddbab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Prpi=C4=8D?= Date: Wed, 4 Nov 2026 16:34:15 -0500 Subject: [PATCH 1/2] fix(sbom): make package SPDX ids unique per document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every SBOM used the fixed ids `SPDXRef-wheel` and `SPDXRef-upstream`. That is valid SPDX, since ids only need to be unique within a document, but tools that read several SBOMs merge packages by id. Scanning an environment of fromager-built wheels with syft reported a single package carrying the data of all wheels. The ids now include the package name and version plus the first eight hex digits of the document's namespace UUID, e.g. `SPDXRef-wheel-idna-3.6-1f3a9c2e`. Name and version keep different wheels apart; the suffix keeps apart two builds of the same version, such as variant builds with different purls, which syft would otherwise merge into one package. Characters SPDX ids do not allow become `-`. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Martin Prpič --- src/fromager/sbom.py | 34 ++++++++++++++---- tests/test_sbom.py | 86 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 110 insertions(+), 10 deletions(-) diff --git a/src/fromager/sbom.py b/src/fromager/sbom.py index 0c41dd98..d9615903 100644 --- a/src/fromager/sbom.py +++ b/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, @@ -143,13 +157,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 +184,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 +213,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, }, ], } diff --git a/tests/test_sbom.py b/tests/test_sbom.py index 47e689a6..84c50fa2 100644 --- a/tests/test_sbom.py +++ b/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,93 @@ 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", + ] + + +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. + """ + ctx = make_sbom_ctx(tmp_path, sbom_settings=SbomSettings()) + 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) From 4d4e573490fad8b274d115fff4d25e557212a91c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Prpi=C4=8D?= Date: Wed, 4 Nov 2026 20:02:49 -0500 Subject: [PATCH 2/2] test(sbom): address review feedback on SPDX id tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin the UUID in the different-packages test so only name and version keep the ids apart, validate the document in the name/version test, and note in `generate_sbom()` that the wheel package should be found through DESCRIBES. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Martin Prpič --- src/fromager/sbom.py | 4 ++++ tests/test_sbom.py | 14 +++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/fromager/sbom.py b/src/fromager/sbom.py index d9615903..027d8679 100644 --- a/src/fromager/sbom.py +++ b/src/fromager/sbom.py @@ -123,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: diff --git a/tests/test_sbom.py b/tests/test_sbom.py index 84c50fa2..85f951e3 100644 --- a/tests/test_sbom.py +++ b/tests/test_sbom.py @@ -421,6 +421,7 @@ def test_generate_sbom_spdx_ids_name_package_and_document( "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( @@ -429,13 +430,16 @@ def test_generate_sbom_spdx_ids_differ_between_packages( """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. + 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()) - 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")] - ] + 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])