Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions .github/workflows/package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,32 @@ jobs:
with:
python-version: "3.13"

- name: Verify PyPI description and version
run: python scripts/verify_pypi_metadata.py --version "${{ needs.build.outputs.version }}"
- name: Download reviewed distributions
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
name: base-cli-dist-${{ github.run_id }}
path: dist

- name: Download release metadata
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
name: base-cli-release-metadata-${{ github.run_id }}
path: dist

- name: Verify PyPI metadata and published bytes
run: |
python scripts/verify_pypi_metadata.py \
--version "${{ needs.build.outputs.version }}" \
--artifacts-dir dist \
--evidence-output dist/PYPI-BYTE-VERIFICATION.json

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness: the new "Upload PyPI byte verification evidence" step has no if: always(), so Actions skips it whenever the preceding verification step fails — meaning the evidence artifact is never produced on exactly the failure path (a PyPI byte/hash mismatch) it exists to document. When scripts/verify_pypi_metadata.py detects a mismatch, it writes the evidence file and then raises SystemExit(1); that marks the step failed, and the next step (no if: condition) is skipped by default — losing the forensic evidence, contradicting this PR's stated goal, and inconsistent with this repo's own convention in .github/workflows/tests.yml (~lines 121-127, 194-200), which pairs if: always() with if-no-files-found: ignore for exactly this "keep evidence even on failure" case. Naively adding just if: always() isn't quite right either — if the script crashes before writing any evidence file (e.g. _fetch() exhausts retries), the upload step's current if-no-files-found: error would then surface a misleading second "no files found" failure masking the real root cause; pair it with if-no-files-found: ignore like the existing convention does.


- name: Upload PyPI byte verification evidence
if: ${{ always() }}
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: base-cli-pypi-byte-evidence-${{ github.run_id }}
path: dist/PYPI-BYTE-VERIFICATION.json
if-no-files-found: ignore

release:
name: Create GitHub Release
Expand Down
68 changes: 68 additions & 0 deletions scripts/verify_pypi_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
from pathlib import Path
from typing import Any

try:
from .release_metadata_helpers import sha256_file
except ImportError: # pragma: no cover - direct script execution
from release_metadata_helpers import sha256_file


def _normalized(value: str) -> str:
return value.replace("\r\n", "\n").strip()
Expand Down Expand Up @@ -40,6 +45,61 @@ def validate_metadata(metadata: dict[str, Any], version: str, readme: str) -> li
return errors


def validate_artifact_hashes(metadata: dict[str, Any], artifacts_dir: Path) -> tuple[list[str], dict[str, Any]]:
"""Compare PyPI file hashes with the reviewed distribution and SHA256SUMS."""

errors: list[str] = []
artifacts = sorted((*artifacts_dir.glob("*.whl"), *artifacts_dir.glob("*.tar.gz")))
reviewed = {path.name: sha256_file(path) for path in artifacts}
urls = metadata.get("urls")
published: dict[str, str] = {}
if not isinstance(urls, list):
errors.append("PyPI response has no file URL list")
else:
for entry in urls:
if not isinstance(entry, dict):
errors.append("PyPI file URL entry is not an object")
continue
filename = entry.get("filename")
digest = (entry.get("digests") or {}).get("sha256") if isinstance(entry.get("digests"), dict) else None
if not isinstance(filename, str) or not isinstance(digest, str):
errors.append("PyPI file URL entry has no filename and SHA-256 digest")
continue
published[filename] = digest
if set(published) != set(reviewed):
errors.append(
f"PyPI files differ from reviewed artifacts: expected {sorted(reviewed)!r}, observed {sorted(published)!r}"
)
for filename, digest in reviewed.items():
if published.get(filename) != digest:
errors.append(f"PyPI SHA-256 for {filename} does not match the reviewed artifact")

checksums_path = artifacts_dir / "SHA256SUMS"
checksums: dict[str, str] = {}
if not checksums_path.is_file():
errors.append("reviewed SHA256SUMS is missing")
else:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarity bug: this error message swaps the semantic labels — it prints the locally-reviewed hash set as "expected" and PyPI's actual response as "observed", backwards from how a reader would interpret those words during a CI failure (expected {sorted(observed)!r}, observed {sorted(published)!r}, where observed actually holds the locally-built/reviewed artifact hashes and published holds PyPI's response). A maintainer debugging a genuine mismatch could misattribute which side is the trusted baseline.

for line in checksums_path.read_text(encoding="utf-8").splitlines():
fields = line.split(maxsplit=1)
if len(fields) == 2:
checksums[fields[1]] = fields[0]
if checksums != reviewed:
errors.append("reviewed SHA256SUMS does not match the reviewed artifacts")

evidence = {
"package": "base-cli",
"version": metadata.get("info", {}).get("version") if isinstance(metadata.get("info"), dict) else None,
"files": [
{"filename": filename, "sha256": reviewed[filename], "pypi_sha256": published.get(filename)}
for filename in sorted(reviewed)
],
"sha256sums_verified": checksums == reviewed,
"result": not errors,
"errors": errors,
}
return errors, evidence


def _fetch(url: str, attempts: int, delay: float) -> dict[str, Any]:
last_error: Exception | None = None
for attempt in range(1, attempts + 1):
Expand All @@ -62,9 +122,17 @@ def main() -> None:
parser.add_argument("--readme", type=Path, default=Path("README.md"))
parser.add_argument("--attempts", type=int, default=12)
parser.add_argument("--delay", type=float, default=5.0)
parser.add_argument("--artifacts-dir", type=Path)
parser.add_argument("--evidence-output", type=Path)
args = parser.parse_args()
metadata = _fetch(f"https://pypi.org/pypi/base-cli/{args.version}/json", args.attempts, args.delay)
errors = validate_metadata(metadata, args.version, args.readme.read_text(encoding="utf-8"))
evidence: dict[str, Any] | None = None
if args.artifacts_dir is not None:
artifact_errors, evidence = validate_artifact_hashes(metadata, args.artifacts_dir)
errors.extend(artifact_errors)
if args.evidence_output is not None and evidence is not None:
args.evidence_output.write_text(json.dumps(evidence, indent=2, sort_keys=True) + "\n", encoding="utf-8")
if errors:
for error in errors:
print(f"PyPI metadata validation failed: {error}")
Expand Down
3 changes: 3 additions & 0 deletions tests/test_package_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ def test_package_workflow_gates_tag_publication_on_release_documentation() -> No
assert "python scripts/validate_release_docs.py" in workflow
assert "scripts/verify_pypi_metadata.py" in workflow
assert "name: Verify published PyPI metadata" in workflow
assert "--artifacts-dir dist" in workflow
assert "PYPI-BYTE-VERIFICATION.json" in workflow
assert "if: ${{ always() }}" in workflow
assert "needs: [build, smoke, provenance, publish, attest, verify_pypi]" in workflow


Expand Down
59 changes: 58 additions & 1 deletion tests/test_verify_pypi_metadata.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
from __future__ import annotations

import hashlib
import sys
import unittest
from pathlib import Path
from tempfile import TemporaryDirectory

sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
from scripts.verify_pypi_metadata import validate_metadata
from scripts.verify_pypi_metadata import validate_artifact_hashes, validate_metadata

README = """\
# base-cli
Expand All @@ -29,6 +31,61 @@ def test_rejects_prepublication_description(self) -> None:
errors = validate_metadata(metadata, "1.2.3", README)
self.assertTrue(any("pre-publication wording" in error for error in errors))

def test_verifies_pypi_file_hashes_against_reviewed_artifacts_and_checksums(self) -> None:
with TemporaryDirectory() as directory:
root = Path(directory)
wheel = root / "base_cli-1.2.3-py3-none-any.whl"
sdist = root / "base_cli-1.2.3.tar.gz"
wheel.write_bytes(b"wheel")
sdist.write_bytes(b"sdist")
checksums = "\n".join(
f"{hashlib.sha256(path.read_bytes()).hexdigest()} {path.name}" for path in (wheel, sdist)
)
(root / "SHA256SUMS").write_text(checksums + "\n", encoding="utf-8")
metadata = {
"info": {"version": "1.2.3"},
"urls": [
{"filename": path.name, "digests": {"sha256": hashlib.sha256(path.read_bytes()).hexdigest()}}
for path in (wheel, sdist)
],
}
errors, evidence = validate_artifact_hashes(metadata, root)

self.assertEqual(errors, [])
self.assertTrue(evidence["result"])

def test_rejects_pypi_hash_mismatch(self) -> None:
with TemporaryDirectory() as directory:
root = Path(directory)
wheel = root / "base_cli-1.2.3-py3-none-any.whl"
wheel.write_bytes(b"wheel")
digest = hashlib.sha256(wheel.read_bytes()).hexdigest()
(root / "SHA256SUMS").write_text(f"{digest} {wheel.name}\n", encoding="utf-8")
metadata = {
"info": {"version": "1.2.3"},
"urls": [{"filename": wheel.name, "digests": {"sha256": "0" * 64}}],
}

errors, evidence = validate_artifact_hashes(metadata, root)

self.assertTrue(any("does not match" in error for error in errors))
self.assertFalse(evidence["result"])

def test_preserves_reviewed_hashes_when_pypi_urls_are_malformed(self) -> None:
with TemporaryDirectory() as directory:
root = Path(directory)
wheel = root / "base_cli-1.2.3-py3-none-any.whl"
wheel.write_bytes(b"wheel")
digest = hashlib.sha256(wheel.read_bytes()).hexdigest()
(root / "SHA256SUMS").write_text(f"{digest} {wheel.name}\n", encoding="utf-8")

errors, evidence = validate_artifact_hashes({"info": {"version": "1.2.3"}, "urls": {}}, root)

self.assertTrue(any("no file URL list" in error for error in errors))
self.assertEqual(evidence["files"], [{"filename": wheel.name, "pypi_sha256": None, "sha256": digest}])
self.assertEqual(evidence["errors"], errors)
self.assertFalse(evidence["result"])


if __name__ == "__main__":
unittest.main()
Loading