diff --git a/src/fromager/sbom.py b/src/fromager/sbom.py index 0c41dd98..027d8679 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, @@ -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, }, ], } diff --git a/tests/test_sbom.py b/tests/test_sbom.py index 47e689a6..85f951e3 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,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)