diff --git a/scripts/release_metadata_helpers.py b/scripts/release_metadata_helpers.py index 584dba4..8918e7e 100644 --- a/scripts/release_metadata_helpers.py +++ b/scripts/release_metadata_helpers.py @@ -6,6 +6,14 @@ from pathlib import Path +def validate_tag_prefix(tag: str) -> str | None: + """Return a validation error when *tag* is not a version tag.""" + + if not tag.startswith("v") or tag == "v": + return f"release tag must be a v-prefixed version, got {tag!r}" + return None + + def sha256_file(path: Path) -> str: """Return the SHA-256 digest of *path* in lowercase hexadecimal form.""" diff --git a/scripts/validate_release_provenance.py b/scripts/validate_release_provenance.py index 5f600bd..e65923b 100644 --- a/scripts/validate_release_provenance.py +++ b/scripts/validate_release_provenance.py @@ -11,13 +11,17 @@ import sys from pathlib import Path +try: + from .release_metadata_helpers import validate_tag_prefix +except ImportError: # pragma: no cover - direct script execution + from release_metadata_helpers import validate_tag_prefix + FULL_SHA = re.compile(r"^[0-9a-f]{40}$") VALID_PUBLISH_TARGETS = {"", "testpypi", "pypi"} def validate_release_provenance( *, - event_name: str, ref_type: str, tag: str, publish_target: str, @@ -38,14 +42,14 @@ def validate_release_provenance( if repository_shallow: errors.append("release provenance cannot be verified from a shallow repository") - production_release = (event_name == "push" and ref_type == "tag") or publish_target == "pypi" tag_release = ref_type == "tag" - if production_release and ref_type != "tag": + if publish_target == "pypi" and ref_type != "tag": errors.append("PyPI publication requires a version tag, not a branch or pull request ref") if tag_release: - if not tag.startswith("v") or tag == "v": - errors.append(f"release tag must be a v-prefixed version, got {tag!r}") + tag_error = validate_tag_prefix(tag) + if tag_error is not None: + errors.append(tag_error) if tag_type != "tag": errors.append("release tag must be an annotated tag; lightweight tags are rejected") if not FULL_SHA.fullmatch(resolved_tag_commit): @@ -74,23 +78,35 @@ def _git(*args: str) -> tuple[int, str]: return completed.returncode, completed.stdout.strip() -def _git_output(*args: str) -> str: - returncode, output = _git(*args) - return output if returncode == 0 else "" +def _git_failure(*args: str) -> str: + return f"git {' '.join(args)} failed; release provenance cannot be verified" + + +def _git_checked(git_errors: list[str], *args: str, allowed_codes: tuple[int, ...] = ()) -> tuple[int, str]: + code, output = _git(*args) + if code != 0 and code not in allowed_codes: + git_errors.append(_git_failure(*args)) + return code, output + + +def _git_required(git_errors: list[str], *args: str) -> str: + code, output = _git_checked(git_errors, *args) + return output if code == 0 else "" def _read_event_flags(event_path: Path) -> tuple[bool, bool, list[str]]: try: - payload = json.loads(event_path.read_text(encoding="utf-8")) + payload: object = json.loads(event_path.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError) as exc: return False, False, [f"could not read the GitHub event payload: {exc}"] + if not isinstance(payload, dict): + return False, False, ["GitHub event payload must be a JSON object"] return bool(payload.get("forced")), bool(payload.get("deleted")), [] def main() -> None: parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--event-name", default=os.environ.get("GITHUB_EVENT_NAME", "")) - parser.add_argument("--event-path", type=Path, default=os.environ.get("GITHUB_EVENT_PATH", "")) + parser.add_argument("--event-path", default=os.environ.get("GITHUB_EVENT_PATH")) parser.add_argument("--ref-type", default=os.environ.get("GITHUB_REF_TYPE", "")) parser.add_argument("--tag", default=os.environ.get("GITHUB_REF_NAME", "")) parser.add_argument("--publish-target", default=os.environ.get("PUBLISH_TARGET", "")) @@ -101,34 +117,58 @@ def main() -> None: tag_type = "" resolved_tag_commit = "" main_reachable = False + git_errors: list[str] = [] if args.ref_type == "tag": tag_ref = f"refs/tags/{args.tag}" - tag_type = _git_output("cat-file", "-t", tag_ref) - resolved_tag_commit = _git_output("rev-parse", "--verify", f"{tag_ref}^{{}}") + tag_type = _git_required(git_errors, "cat-file", "-t", tag_ref) + resolved_tag_commit = _git_required(git_errors, "rev-parse", "--verify", f"{tag_ref}^{{}}") + if resolved_tag_commit: - main_reachable = _git("merge-base", "--is-ancestor", resolved_tag_commit, args.main_ref)[0] == 0 + ancestor_code, _ = _git_checked( + git_errors, + "merge-base", + "--is-ancestor", + resolved_tag_commit, + args.main_ref, + allowed_codes=(1,), + ) + if ancestor_code == 0: + main_reachable = True forced = False deleted = False event_errors: list[str] = [] if args.ref_type == "tag" or args.publish_target == "pypi": - if not args.event_path: + event_path = Path(args.event_path) if args.event_path else None + if event_path is None: event_errors.append("GitHub event payload is required for release provenance validation") else: - forced, deleted, event_errors = _read_event_flags(Path(args.event_path)) - - errors = event_errors + validate_release_provenance( - event_name=args.event_name, - ref_type=args.ref_type, - tag=args.tag, - publish_target=args.publish_target, - source_commit=args.source_commit, - tag_type=tag_type, - resolved_tag_commit=resolved_tag_commit, - main_reachable=main_reachable, - repository_shallow=_git_output("rev-parse", "--is-shallow-repository") == "true", - forced=forced, - deleted=deleted, + forced, deleted, event_errors = _read_event_flags(event_path) + + shallow_output = _git_required(git_errors, "rev-parse", "--is-shallow-repository") + if not shallow_output: + repository_shallow = True + elif shallow_output not in {"true", "false"}: + git_errors.append("git rev-parse returned an unknown shallow-repository state") + repository_shallow = True + else: + repository_shallow = shallow_output == "true" + + errors = ( + event_errors + + git_errors + + validate_release_provenance( + ref_type=args.ref_type, + tag=args.tag, + publish_target=args.publish_target, + source_commit=args.source_commit, + tag_type=tag_type, + resolved_tag_commit=resolved_tag_commit, + main_reachable=main_reachable, + repository_shallow=repository_shallow, + forced=forced, + deleted=deleted, + ) ) if errors: for error in errors: diff --git a/scripts/validate_release_ref.py b/scripts/validate_release_ref.py index 42f4a72..da148f6 100644 --- a/scripts/validate_release_ref.py +++ b/scripts/validate_release_ref.py @@ -9,6 +9,11 @@ import sys from pathlib import Path +try: + from .release_metadata_helpers import validate_tag_prefix +except ImportError: # pragma: no cover - direct script execution + from release_metadata_helpers import validate_tag_prefix + RELEASE_HEADING = re.compile(r"^## \[(?P\d+\.\d+\.\d+)\] - (?P\d{4}-\d{2}-\d{2})$") BULLET = re.compile(r"^\s*[-*+]\s+\S") @@ -16,8 +21,9 @@ def validate_release_ref(version_path: Path, changelog_path: Path, tag: str) -> list[str]: """Return violations for a release ``tag`` and its source files.""" errors: list[str] = [] - if not tag.startswith("v") or tag == "v": - return [f"release tag must be a v-prefixed version, got {tag!r}"] + tag_error = validate_tag_prefix(tag) + if tag_error is not None: + return [tag_error] version = tag[1:] declared = version_path.read_text(encoding="utf-8").strip() if declared != version: diff --git a/tests/test_package_workflow.py b/tests/test_package_workflow.py index d5a75c5..71de8e8 100644 --- a/tests/test_package_workflow.py +++ b/tests/test_package_workflow.py @@ -2,6 +2,8 @@ from pathlib import Path +import yaml + def test_package_workflow_uses_numeric_reproducibility_epoch() -> None: workflow = (Path(__file__).resolve().parents[1] / ".github/workflows/package.yml").read_text(encoding="utf-8") @@ -26,9 +28,11 @@ def test_package_workflow_does_not_replace_published_release_assets() -> None: def test_package_workflow_gates_writes_on_release_provenance() -> None: workflow = (Path(__file__).resolve().parents[1] / ".github/workflows/package.yml").read_text(encoding="utf-8") + jobs = yaml.safe_load(workflow)["jobs"] assert "name: Verify reviewed release provenance" in workflow assert 'git fetch --no-tags --prune origin "refs/heads/main:refs/remotes/origin/main"' in workflow assert "python scripts/validate_release_provenance.py" in workflow - assert "needs: [build, smoke, provenance]" in workflow - assert "needs: [build, smoke, provenance, publish, attest]" in workflow + assert jobs["publish"]["needs"] == ["build", "smoke", "provenance"] + assert jobs["attest"]["needs"] == ["build", "smoke", "provenance"] + assert jobs["release"]["needs"] == ["build", "smoke", "provenance", "publish", "attest"] diff --git a/tests/test_validate_release_provenance.py b/tests/test_validate_release_provenance.py index ce93216..dbcc3a2 100644 --- a/tests/test_validate_release_provenance.py +++ b/tests/test_validate_release_provenance.py @@ -1,10 +1,17 @@ from __future__ import annotations +import contextlib +import io +import json +import os import sys +import tempfile import unittest from pathlib import Path +from unittest import mock sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from scripts import validate_release_provenance as provenance_module from scripts.validate_release_provenance import validate_release_provenance SOURCE = "a" * 40 @@ -13,7 +20,6 @@ def valid(**overrides: object) -> list[str]: values: dict[str, object] = { - "event_name": "push", "ref_type": "tag", "tag": "v1.0.0", "publish_target": "", @@ -47,18 +53,21 @@ def test_rejects_forced_tag_update(self) -> None: errors = valid(forced=True) self.assertTrue(any("forced tag" in error for error in errors)) + def test_rejects_deleted_tag_event(self) -> None: + errors = valid(deleted=True) + self.assertTrue(any("deleted tag" in error for error in errors)) + def test_rejects_shallow_history(self) -> None: errors = valid(repository_shallow=True) self.assertTrue(any("shallow" in error for error in errors)) def test_rejects_pypi_dispatch_from_a_branch(self) -> None: - errors = valid(event_name="workflow_dispatch", ref_type="branch", publish_target="pypi") + errors = valid(ref_type="branch", publish_target="pypi") self.assertTrue(any("requires a version tag" in error for error in errors)) def test_allows_testpypi_branch_rehearsal_with_full_history(self) -> None: self.assertEqual( valid( - event_name="workflow_dispatch", ref_type="branch", tag="", publish_target="testpypi", @@ -69,6 +78,150 @@ def test_allows_testpypi_branch_rehearsal_with_full_history(self) -> None: [], ) + def test_main_requires_event_payload_without_turning_empty_path_into_directory(self) -> None: + def git_success(*args: str) -> tuple[int, str]: + if args[:2] == ("cat-file", "-t"): + return 0, "tag" + if args[:2] == ("rev-parse", "--verify"): + return 0, SOURCE + if args[:2] == ("merge-base", "--is-ancestor"): + return 0, "" + if args == ("rev-parse", "--is-shallow-repository"): + return 0, "false" + raise AssertionError(args) + + stderr = io.StringIO() + environment = { + "GITHUB_EVENT_NAME": "push", + "GITHUB_REF_TYPE": "tag", + "GITHUB_REF_NAME": "v1.0.0", + "GITHUB_SHA": SOURCE, + } + with ( + mock.patch.dict(os.environ, environment, clear=True), + mock.patch.object(sys, "argv", ["validate_release_provenance.py"]), + mock.patch.object(provenance_module, "_git", side_effect=git_success), + contextlib.redirect_stderr(stderr), + self.assertRaises(SystemExit) as raised, + ): + provenance_module.main() + + self.assertEqual(raised.exception.code, 1) + self.assertIn("event payload is required", stderr.getvalue()) + self.assertNotIn("Is a directory", stderr.getvalue()) + + def test_main_reads_deleted_event_payload(self) -> None: + def git_success(*args: str) -> tuple[int, str]: + if args[:2] == ("cat-file", "-t"): + return 0, "tag" + if args[:2] == ("rev-parse", "--verify"): + return 0, SOURCE + if args[:2] == ("merge-base", "--is-ancestor"): + return 0, "" + if args == ("rev-parse", "--is-shallow-repository"): + return 0, "false" + raise AssertionError(args) + + with tempfile.TemporaryDirectory() as directory: + event_path = Path(directory) / "event.json" + event_path.write_text(json.dumps({"deleted": True}), encoding="utf-8") + stderr = io.StringIO() + environment = { + "GITHUB_EVENT_NAME": "push", + "GITHUB_REF_TYPE": "tag", + "GITHUB_REF_NAME": "v1.0.0", + "GITHUB_SHA": SOURCE, + } + with ( + mock.patch.dict(os.environ, environment, clear=True), + mock.patch.object( + sys, + "argv", + ["validate_release_provenance.py", "--event-path", str(event_path)], + ), + mock.patch.object(provenance_module, "_git", side_effect=git_success), + contextlib.redirect_stderr(stderr), + self.assertRaises(SystemExit) as raised, + ): + provenance_module.main() + + self.assertEqual(raised.exception.code, 1) + self.assertIn("deleted tag", stderr.getvalue()) + + def test_main_rejects_non_object_event_payloads(self) -> None: + def git_success(*args: str) -> tuple[int, str]: + if args[:2] == ("cat-file", "-t"): + return 0, "tag" + if args[:2] == ("rev-parse", "--verify"): + return 0, SOURCE + if args[:2] == ("merge-base", "--is-ancestor"): + return 0, "" + if args == ("rev-parse", "--is-shallow-repository"): + return 0, "false" + raise AssertionError(args) + + environment = { + "GITHUB_EVENT_NAME": "push", + "GITHUB_REF_TYPE": "tag", + "GITHUB_REF_NAME": "v1.0.0", + "GITHUB_SHA": SOURCE, + } + for payload in ([1, 2], None, 42, "event"): + with self.subTest(payload=payload), tempfile.TemporaryDirectory() as directory: + event_path = Path(directory) / "event.json" + event_path.write_text(json.dumps(payload), encoding="utf-8") + stderr = io.StringIO() + with ( + mock.patch.dict(os.environ, environment, clear=True), + mock.patch.object(sys, "argv", ["validate_release_provenance.py", "--event-path", str(event_path)]), + mock.patch.object(provenance_module, "_git", side_effect=git_success), + contextlib.redirect_stderr(stderr), + self.assertRaises(SystemExit) as raised, + ): + provenance_module.main() + + self.assertEqual(raised.exception.code, 1) + self.assertIn("event payload must be a JSON object", stderr.getvalue()) + self.assertNotIn("AttributeError", stderr.getvalue()) + + def test_main_fails_closed_when_shallow_state_query_fails(self) -> None: + def git_failure(*args: str) -> tuple[int, str]: + if args[:2] == ("cat-file", "-t"): + return 0, "tag" + if args[:2] == ("rev-parse", "--verify"): + return 0, SOURCE + if args[:2] == ("merge-base", "--is-ancestor"): + return 0, "" + if args == ("rev-parse", "--is-shallow-repository"): + return 128, "" + raise AssertionError(args) + + with tempfile.TemporaryDirectory() as directory: + event_path = Path(directory) / "event.json" + event_path.write_text("{}", encoding="utf-8") + stderr = io.StringIO() + environment = { + "GITHUB_EVENT_NAME": "push", + "GITHUB_REF_TYPE": "tag", + "GITHUB_REF_NAME": "v1.0.0", + "GITHUB_SHA": SOURCE, + } + with ( + mock.patch.dict(os.environ, environment, clear=True), + mock.patch.object( + sys, + "argv", + ["validate_release_provenance.py", "--event-path", str(event_path)], + ), + mock.patch.object(provenance_module, "_git", side_effect=git_failure), + contextlib.redirect_stderr(stderr), + self.assertRaises(SystemExit) as raised, + ): + provenance_module.main() + + self.assertEqual(raised.exception.code, 1) + self.assertIn("is-shallow-repository", stderr.getvalue()) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_validate_release_ref.py b/tests/test_validate_release_ref.py index af0056c..7fcfd47 100644 --- a/tests/test_validate_release_ref.py +++ b/tests/test_validate_release_ref.py @@ -52,6 +52,10 @@ def test_rejects_undated_or_missing_section(self) -> None: errors = self.validate("1.2.3\n", changelog, "v1.2.3") self.assertTrue(any("missing a dated release section" in error for error in errors)) + def test_rejects_non_version_tag(self) -> None: + errors = self.validate("1.2.3\n", VALID_CHANGELOG, "1.2.3") + self.assertEqual(errors, ["release tag must be a v-prefixed version, got '1.2.3'"]) + if __name__ == "__main__": unittest.main()