Repository navigation
fix(sbom): keep wheel purl settings out of upstream identity #1386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -115,7 +115,7 @@ class SbomSettings(pydantic.BaseModel): | |
| """ | ||
|
|
||
| purl_type: PurlType = "pypi" | ||
| """Default purl type for all packages (e.g. ``pypi``, ``generic``)""" | ||
| """Default purl type for wheel package entries (e.g. ``pypi``, ``generic``)""" | ||
|
|
||
| repository_url: AnyUrl | None = None | ||
| """Default purl ``repository_url`` qualifier for all packages | ||
|
|
@@ -333,9 +333,10 @@ class PurlConfig(pydantic.BaseModel): | |
| GENERATED_FROM relationship. Used for packages sourced from | ||
| GitHub/GitLab rather than PyPI. | ||
|
|
||
| When absent, the upstream purl keeps the downstream name, type, and | ||
| namespace but uses the resolved source version instead of a local | ||
| downstream version override. The ``repository_url`` qualifier is omitted. | ||
| When absent, the upstream purl is ``pkg:pypi/<package name>@<resolved | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: This is a user-facing semantics change, so both edited docstrings should carry a |
||
| version>``. Wheel-only name, type, namespace, and version overrides and | ||
| the global ``sbom.purl_type`` do not change it. For non-PyPI sources, | ||
| set ``purl.upstream`` explicitly. | ||
| """ | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,26 +62,21 @@ def _build_upstream_purl( | |
| name: NormalizedName, | ||
| version: Version, | ||
| pbi: PackageBuildInfo, | ||
| sbom_settings: SbomSettings, | ||
| ) -> PackageURL: | ||
| """Build the upstream source package URL. | ||
|
|
||
| If ``upstream`` is set in the per-package ``PurlConfig``, it is | ||
| used as-is. Otherwise, the upstream purl uses the resolved source | ||
| version rather than a downstream wheel version override, and excludes | ||
| the ``repository_url`` qualifier. | ||
| used as-is. Otherwise, the source is identified by its resolved | ||
| PyPI name and version, regardless of downstream wheel purl overrides. | ||
| """ | ||
| pc = pbi.purl_config | ||
| if pc and pc.upstream: | ||
| return PackageURL.from_string(pc.upstream) | ||
|
|
||
| purl_type = pc.type if pc else None | ||
| purl_namespace = pc.namespace if pc else None | ||
| purl_name = pc.name if pc else None | ||
| return PackageURL( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: When pc = pbi.purl_config
if pc and pc.upstream:
return PackageURL.from_string(pc.upstream)
+ if pc and (pc.type or pc.namespace or pc.name):
+ logger.warning(
+ "ignoring purl type/namespace/name overrides for the upstream"
+ " identity of %s; set purl.upstream to describe a non-PyPI source",
+ name,
+ )
return PackageURL( |
||
| type=purl_type or sbom_settings.purl_type, | ||
| namespace=purl_namespace, | ||
| name=purl_name or name, | ||
| type="pypi", | ||
| namespace=None, | ||
| name=name, | ||
| version=str(version), | ||
| ) | ||
|
|
||
|
|
@@ -124,7 +119,6 @@ def generate_sbom( | |
| name=name, | ||
| version=version, | ||
| pbi=pbi, | ||
| sbom_settings=sbom_settings, | ||
| ) | ||
|
|
||
| # Derive the document identity from the downstream purl so the | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |||||||||||||
| import uuid | ||||||||||||||
| from unittest.mock import patch | ||||||||||||||
|
|
||||||||||||||
| import pytest | ||||||||||||||
| from conftest import make_sbom_ctx | ||||||||||||||
| from packaging.requirements import Requirement | ||||||||||||||
| from packaging.version import Version | ||||||||||||||
|
|
@@ -174,6 +175,42 @@ def test_generate_sbom_upstream_uses_source_version_without_upstream_override( | |||||||||||||
| _validate_spdx(doc) | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| @pytest.mark.parametrize( | ||||||||||||||
| ("package_purl", "sbom_type", "expected_wheel"), | ||||||||||||||
| [ | ||||||||||||||
| ({"name": "test-pkg-corp"}, "pypi", "pkg:pypi/test-pkg-corp@0.5.5"), | ||||||||||||||
| ({"type": "generic"}, "pypi", "pkg:generic/test-pkg@0.5.5"), | ||||||||||||||
| ({"namespace": "acme"}, "pypi", "pkg:pypi/acme/test-pkg@0.5.5"), | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor:
Suggested change
|
||||||||||||||
| ({}, "generic", "pkg:generic/test-pkg@0.5.5"), | ||||||||||||||
| ], | ||||||||||||||
| ) | ||||||||||||||
| def test_generate_sbom_upstream_ignores_wheel_purl_overrides( | ||||||||||||||
| tmp_path: pathlib.Path, | ||||||||||||||
| package_purl: dict[str, str], | ||||||||||||||
| sbom_type: str, | ||||||||||||||
| expected_wheel: str, | ||||||||||||||
| ) -> None: | ||||||||||||||
| """Wheel-only purl settings must not rewrite the source identity.""" | ||||||||||||||
| ctx = make_sbom_ctx( | ||||||||||||||
| tmp_path, | ||||||||||||||
| sbom_settings=SbomSettings(purl_type=sbom_type), | ||||||||||||||
| package_overrides={"purl": package_purl}, | ||||||||||||||
| ) | ||||||||||||||
| doc = sbom.generate_sbom( | ||||||||||||||
| ctx=ctx, | ||||||||||||||
| req=Requirement("test-pkg==0.5.5"), | ||||||||||||||
| version=Version("0.5.5"), | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| wheel, upstream = doc["packages"] | ||||||||||||||
| assert wheel["externalRefs"][0]["referenceLocator"] == expected_wheel | ||||||||||||||
| assert upstream["name"] == "test-pkg" | ||||||||||||||
| assert upstream["externalRefs"][0]["referenceLocator"] == ( | ||||||||||||||
| "pkg:pypi/test-pkg@0.5.5" | ||||||||||||||
| ) | ||||||||||||||
| _validate_spdx(doc) | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def test_generate_sbom_namespace_is_unique_per_document( | ||||||||||||||
| tmp_path: pathlib.Path, | ||||||||||||||
| ) -> None: | ||||||||||||||
|
|
@@ -245,9 +282,9 @@ def test_generate_sbom_purl_field_overrides(tmp_path: pathlib.Path) -> None: | |||||||||||||
| assert wheel["externalRefs"][0]["referenceLocator"] == ( | ||||||||||||||
| "pkg:generic/custom-name@1.0.0" | ||||||||||||||
| ) | ||||||||||||||
| # Field overrides carry through to upstream (without qualifiers) | ||||||||||||||
| # Wheel field overrides must not change the resolved source identity. | ||||||||||||||
| assert upstream["externalRefs"][0]["referenceLocator"] == ( | ||||||||||||||
| "pkg:generic/custom-name@1.0.0" | ||||||||||||||
| "pkg:pypi/test-pkg@1.0.0" | ||||||||||||||
| ) | ||||||||||||||
| # Document identity follows the purl name override | ||||||||||||||
| assert doc["name"] == "custom-name-1.0.0" | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor:
purl.versionis also wheel-only (since #1377), and thepurl.upstreamdocstring added in this PR lists all four overrides, so the two docs disagree on the list: