-
Notifications
You must be signed in to change notification settings - Fork 1
security: gate releases on reviewed main provenance #375
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
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 |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| #!/usr/bin/env python3 | ||
| """Fail closed unless a publication comes from a reviewed main-line commit.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import json | ||
| import os | ||
| import re | ||
| import subprocess | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| 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, | ||
| source_commit: str, | ||
| tag_type: str, | ||
| resolved_tag_commit: str, | ||
| main_reachable: bool, | ||
| repository_shallow: bool, | ||
| forced: bool = False, | ||
| deleted: bool = False, | ||
| ) -> list[str]: | ||
| """Return violations for the source that is about to be published.""" | ||
| errors: list[str] = [] | ||
| if publish_target not in VALID_PUBLISH_TARGETS: | ||
| errors.append(f"unsupported publication target {publish_target!r}") | ||
| if not FULL_SHA.fullmatch(source_commit): | ||
| errors.append("reviewed source commit must be a full 40-character commit SHA") | ||
| 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": | ||
| 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}") | ||
| 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): | ||
| errors.append("release tag must resolve to a full commit SHA") | ||
| elif resolved_tag_commit != source_commit: | ||
| errors.append( | ||
| f"release tag does not resolve to the reviewed source commit ({resolved_tag_commit} != {source_commit})" | ||
| ) | ||
| if not main_reachable: | ||
| errors.append("release tag commit is not reachable from the trusted origin/main history") | ||
| if forced: | ||
| errors.append("forced tag updates are rejected for release publication") | ||
| if deleted: | ||
| errors.append("deleted tag events are rejected for release publication") | ||
|
|
||
| return errors | ||
|
|
||
|
|
||
| def _git(*args: str) -> tuple[int, str]: | ||
| completed = subprocess.run( | ||
| ["git", *args], | ||
| check=False, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| return completed.returncode, completed.stdout.strip() | ||
|
|
||
|
|
||
| def _git_output(*args: str) -> str: | ||
| returncode, output = _git(*args) | ||
| return output if returncode == 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")) | ||
| except (OSError, json.JSONDecodeError) as exc: | ||
| return False, False, [f"could not read the GitHub event payload: {exc}"] | ||
| 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", "")) | ||
|
Contributor
Author
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. Correctness: Failure scenario: |
||
| 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", "")) | ||
| parser.add_argument("--source-commit", default=os.environ.get("GITHUB_SHA", "")) | ||
| parser.add_argument("--main-ref", default="refs/remotes/origin/main") | ||
| args = parser.parse_args() | ||
|
|
||
| tag_type = "" | ||
| resolved_tag_commit = "" | ||
| main_reachable = False | ||
| 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}^{{}}") | ||
| if resolved_tag_commit: | ||
| main_reachable = _git("merge-base", "--is-ancestor", resolved_tag_commit, args.main_ref)[0] == 0 | ||
|
|
||
| forced = False | ||
| deleted = False | ||
| event_errors: list[str] = [] | ||
| if args.ref_type == "tag" or args.publish_target == "pypi": | ||
| if not args.event_path: | ||
| 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", | ||
|
Contributor
Author
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. Correctness (fail-open, inconsistent with Failure scenario: any git-command failure in this step (corrupted checkout, an unsupported flag on a future git version, a transient runner issue) silently waves through the shallow-history gate instead of raising an error, even though this module's own docstring promises "fail closed unless a publication comes from a reviewed main-line commit." Consider treating a non-zero exit here as an error (e.g. |
||
| forced=forced, | ||
| deleted=deleted, | ||
| ) | ||
| if errors: | ||
| for error in errors: | ||
| print(f"release provenance validation failed: {error}", file=sys.stderr) | ||
| raise SystemExit(1) | ||
| print(f"Validated release provenance for {args.source_commit}") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,3 +22,13 @@ def test_package_workflow_does_not_replace_published_release_assets() -> None: | |
| assert '--source-digest "$GITHUB_SHA"' in workflow | ||
| assert '--source-ref "$GITHUB_REF"' in workflow | ||
| assert "--clobber" not in workflow | ||
|
|
||
|
|
||
| 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") | ||
|
|
||
| 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 | ||
|
Contributor
Author
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. Test gap (weak assertion on a security-critical gate): |
||
| assert "needs: [build, smoke, provenance, publish, attest]" in workflow | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import sys | ||
| import unittest | ||
| from pathlib import Path | ||
|
|
||
| sys.path.insert(0, str(Path(__file__).resolve().parents[1])) | ||
| from scripts.validate_release_provenance import validate_release_provenance | ||
|
|
||
| SOURCE = "a" * 40 | ||
| TAG_COMMIT = SOURCE | ||
|
|
||
|
|
||
| def valid(**overrides: object) -> list[str]: | ||
| values: dict[str, object] = { | ||
| "event_name": "push", | ||
| "ref_type": "tag", | ||
| "tag": "v1.0.0", | ||
| "publish_target": "", | ||
| "source_commit": SOURCE, | ||
| "tag_type": "tag", | ||
| "resolved_tag_commit": TAG_COMMIT, | ||
| "main_reachable": True, | ||
| "repository_shallow": False, | ||
| } | ||
| values.update(overrides) | ||
| return validate_release_provenance(**values) # type: ignore[arg-type] | ||
|
|
||
|
|
||
| class ReleaseProvenanceValidationTests(unittest.TestCase): | ||
|
Contributor
Author
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. Test gap: every test in this file calls |
||
| def test_accepts_annotated_tag_on_main(self) -> None: | ||
| self.assertEqual(valid(), []) | ||
|
|
||
| def test_rejects_lightweight_tag(self) -> None: | ||
| errors = valid(tag_type="commit") | ||
| self.assertTrue(any("annotated tag" in error for error in errors)) | ||
|
|
||
| def test_rejects_unmerged_commit(self) -> None: | ||
| errors = valid(main_reachable=False) | ||
| self.assertTrue(any("not reachable" in error for error in errors)) | ||
|
|
||
| def test_rejects_moved_or_mismatched_tag(self) -> None: | ||
| errors = valid(resolved_tag_commit="b" * 40) | ||
| self.assertTrue(any("does not resolve" in error for error in errors)) | ||
|
|
||
| def test_rejects_forced_tag_update(self) -> None: | ||
|
Contributor
Author
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. Test gap (asymmetric coverage): there's a |
||
| errors = valid(forced=True) | ||
| self.assertTrue(any("forced 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") | ||
| 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", | ||
| tag_type="", | ||
| resolved_tag_commit="", | ||
| main_reachable=False, | ||
| ), | ||
| [], | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() | ||
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.
Cleanup (minor):
production_release = (event_name == "push" and ref_type == "tag") or publish_target == "pypi"is only used once, atif production_release and ref_type != "tag":(next line) — which is logically equivalent toif publish_target == "pypi" and ref_type != "tag":, since the first disjunct ofproduction_releasecan never makeref_type != "tag"true (it requiresref_type == "tag"). Not a functional bug, just dead weight that could confuse a future maintainer editing this gate into thinkingevent_name/the first disjunct matters here.