From 42180912fd49d99e05a383b5ee9ecd1d28860269 Mon Sep 17 00:00:00 2001 From: Tim Date: Thu, 27 Aug 2026 16:13:13 -0700 Subject: [PATCH 1/2] ci(release): decide extras by dependence, sign the SBOM, correct the pin comment Follow-up to the release-pipeline repair, from a review of it. Four defects, each verified before changing anything. Pin comment: the bump replaced the 40-character SHA and left the trailing `# v3.0.0` sitting three lines below a new comment block asserting "Pinned at v3.5.0 deliberately". Confirmed against the upstream tags API -- 790bc6be is v3.5.0, f514d46b is v3.0.0 -- so the file contradicted itself. A pin normalizer that trusts the comment rewrites the SHA back to v3.0.0 and silently reinstates the TUF abort the bump exists to avoid. Extras: an empty `extra` makes `extra == "cli"` false, but it makes `extra in "cli"` *true*, because "" is a substring of every string. An extra-gated dependency written that way would be demanded, be correctly absent, and abort the release blaming the venv -- exactly the false-positive class this check was fixed to remove. Extras are now decided by whether the marker's answer depends on `extra` at all, which holds for every operator rather than for `==` alone. Marker exceptions: `Requirement(spec)` was guarded but `.evaluate()` was not, and packaging raises UndefinedComparison and UndefinedEnvironmentName at evaluate time rather than parse time. `a; sys_platform ~= "1.0"` parses cleanly and then raises, aborting with a bare traceback instead of the `::error::` annotation every other failure path here deliberately emits. Both are now caught and routed through fail(). SBOM signature: the document was attached to the release but never passed to the signing step, so the one artifact these checks exist to make trustworthy shipped unauthenticated beside two signed distributions. It is added to `inputs` on the release path only -- the repair path fetches from PyPI and has no SBOM, and a glob matching nothing is fatal to that action. Verified by building a wheel and running the script end to end: 18 components, correct root purl, and `packaging` still absent from the document, so the install-after-generation order still holds. --- .github/workflows/release.yml | 2 +- scripts/sbom_from_wheel.sh | 30 +++++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 01dc4c1a..5b4a2c5a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -175,7 +175,7 @@ jobs: - name: 🔏 Sigstore sign artifacts uses: sigstore/gh-action-sigstore-python@790bc6befb9d733738f18d8f895854b453640ec9 # v3.5.0 with: - inputs: dist/*.whl dist/*.tar.gz + inputs: dist/*.whl dist/*.tar.gz sbom-python.cdx.json - name: ⬆️ Attach artifacts to release env: GH_TOKEN: ${{ github.token }} diff --git a/scripts/sbom_from_wheel.sh b/scripts/sbom_from_wheel.sh index 69c1cf61..e19df291 100755 --- a/scripts/sbom_from_wheel.sh +++ b/scripts/sbom_from_wheel.sh @@ -69,6 +69,7 @@ import re import sys import zipfile +from packaging.markers import UndefinedComparison, UndefinedEnvironmentName from packaging.requirements import InvalidRequirement, Requirement dist, out = pathlib.Path(sys.argv[1]), pathlib.Path(sys.argv[2]) @@ -91,6 +92,26 @@ if wheel is None: raw_name, version = wheel.name.split("-")[:2] expected = normalize(raw_name) + +def installed_here(marker): + """True if a dependency carrying this marker is installed in this venv. + + The wheel above was installed without extras, so anything gated behind one + is legitimately absent. Two probes decide that: if the marker's answer + changes with `extra`, it is extra-gated whatever the operator. Markers + raise at evaluate time, not parse time, so both probes are guarded. + """ + if marker is None: + return True + results = set() + for probe in ("", "\x00no-such-extra"): + try: + results.add(marker.evaluate({"extra": probe})) + except (UndefinedComparison, UndefinedEnvironmentName) as exc: + fail(f"cannot evaluate the environment marker {str(marker)!r}: {exc}") + return False if len(results) > 1 else results.pop() + + # Requires-Dist from the wheel's own metadata, narrowed to what this # environment should actually hold. A marker decides that, so a marker is what # has to be evaluated -- not a substring of one. @@ -104,8 +125,11 @@ expected = normalize(raw_name) # dependency today; the check is fixed here so that the first one added does not # break a release to discover it. # -# An empty `extra` is what makes `extra == "cli"` false: no extra was requested -# when the wheel was installed above. +# Extras are decided by whether the marker's answer depends on `extra` at all, +# not by evaluating it against an empty one. `extra == "cli"` is false for an +# empty extra, but `extra in "cli"` is *true* -- "" is a substring of every +# string -- so the sentinel alone would demand an extra-gated dependency that +# was correctly never installed, and abort the release blaming the venv. requires = set() with zipfile.ZipFile(wheel) as zf: metadata_name = next((n for n in zf.namelist() if n.endswith(".dist-info/METADATA")), None) @@ -119,7 +143,7 @@ with zipfile.ZipFile(wheel) as zf: req = Requirement(spec) except InvalidRequirement as exc: fail(f"{wheel.name} has an unparsable Requires-Dist {spec!r}: {exc}") - if req.marker is not None and not req.marker.evaluate({"extra": ""}): + if not installed_here(req.marker): continue requires.add(normalize(req.name)) From 6b641603c87019e284cd4ea764bc01ca8ca48d13 Mon Sep 17 00:00:00 2001 From: Tim Date: Thu, 27 Aug 2026 21:23:35 -0700 Subject: [PATCH 2/2] ci(release): overlay packaging instead of installing it into the described venv The previous commit installed packaging into the very venv the SBOM describes, after the document was written, on the argument that the ordering made it safe. Ordering keeps packaging out of the document; it does not keep the document true. Where packaging is already in the dependency closure -- verified in four of these repositories -- the install silently replaces the version the document has just recorded, and a project capping `packaging<26` would receive that replacement with every check still green. `uv run --with` layers the dependency onto the interpreter for the single call that needs it. Verified: the check still sees the venv's own site-packages, and the venv is byte-identical afterwards. --- scripts/sbom_from_wheel.sh | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/scripts/sbom_from_wheel.sh b/scripts/sbom_from_wheel.sh index e19df291..520cec85 100755 --- a/scripts/sbom_from_wheel.sh +++ b/scripts/sbom_from_wheel.sh @@ -46,15 +46,16 @@ uvx --from "cyclonedx-bom==${CYCLONEDX_BOM_VERSION}" cyclonedx-py environment "$ --mc-type "$MC_TYPE" \ --output-format json -o "$OUT" -# Installed *after* the SBOM is generated, and that order is load-bearing: this -# venv is the environment the document describes, so anything added to it before -# the previous step would be published as a component of the release. -uv pip install --quiet --python "$VENV/bin/python" "packaging==${PACKAGING_VERSION}" - -# Run under the venv's interpreter rather than the runner's. The check below -# evaluates environment markers, and the only environment whose answers mean -# anything here is the one the SBOM describes. -"$VENV/bin/python" - "$DIST_DIR" "$OUT" <<'PYEOF' +# Overlaid, not installed. This venv is the environment the document describes, +# so installing into it would publish packaging as a component of the release -- +# and where packaging is already in the closure, would silently replace a +# version the document has just recorded. `--with` layers it onto the +# interpreter for this one call and leaves the venv byte-identical, while the +# check still sees the venv's own site-packages: the only environment whose +# marker answers mean anything here. +uv run --no-project --python "$VENV/bin/python" \ + --with "packaging==${PACKAGING_VERSION}" \ + python - "$DIST_DIR" "$OUT" <<'PYEOF' """Complete the root component from the wheel, then verify the SBOM. Every project here declares `dynamic = ["version"]`, so cyclonedx-py reads the