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..fa6ae1a 100644 --- a/scripts/validate_release_provenance.py +++ b/scripts/validate_release_provenance.py @@ -11,6 +11,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 + FULL_SHA = re.compile(r"^[0-9a-f]{40}$") VALID_PUBLISH_TARGETS = {"", "testpypi", "pypi"} @@ -38,14 +43,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,9 +79,8 @@ 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 _read_event_flags(event_path: Path) -> tuple[bool, bool, list[str]]: @@ -90,7 +94,7 @@ def _read_event_flags(event_path: Path) -> tuple[bool, bool, list[str]]: 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 +105,64 @@ 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_code, tag_type_output = _git("cat-file", "-t", tag_ref) + if tag_type_code == 0: + tag_type = tag_type_output + else: + git_errors.append(_git_failure("cat-file", "-t", tag_ref)) + + resolved_code, resolved_output = _git("rev-parse", "--verify", f"{tag_ref}^{{}}") + if resolved_code == 0: + resolved_tag_commit = resolved_output + else: + git_errors.append(_git_failure("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("merge-base", "--is-ancestor", resolved_tag_commit, args.main_ref) + if ancestor_code == 0: + main_reachable = True + elif ancestor_code != 1: + git_errors.append(_git_failure("merge-base", "--is-ancestor", resolved_tag_commit, args.main_ref)) 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_code, shallow_output = _git("rev-parse", "--is-shallow-repository") + if shallow_code != 0: + git_errors.append(_git_failure("rev-parse", "--is-shallow-repository")) + 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( + 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=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..18f9b5a 100644 --- a/tests/test_package_workflow.py +++ b/tests/test_package_workflow.py @@ -30,5 +30,10 @@ def test_package_workflow_gates_writes_on_release_provenance() -> None: 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 + + publish_block = workflow.split(" publish:\n", 1)[1].split(" attest:\n", 1)[0] + attest_block = workflow.split(" attest:\n", 1)[1].split(" release:\n", 1)[0] + release_block = workflow.split(" release:\n", 1)[1] + assert "needs: [build, smoke, provenance]" in publish_block + assert "needs: [build, smoke, provenance]" in attest_block + assert "needs: [build, smoke, provenance, publish, attest]" in release_block diff --git a/tests/test_validate_release_provenance.py b/tests/test_validate_release_provenance.py index ce93216..a7f5fba 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 @@ -47,6 +54,10 @@ 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)) @@ -69,6 +80,114 @@ 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_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()