From d2503b91fa67282f24ab08d9e62cba72f65627ec Mon Sep 17 00:00:00 2001 From: Lucian Behind The Scenes Date: Tue, 29 Sep 2026 18:01:28 +0300 Subject: [PATCH 1/3] fix(autorelease): let lifecycle records merge unattended Protected controls now admits an implementation run's php_bin_ready record bound to its validated merge commit, and accepts record PRs from runs that started on an ancestor of main. The notifier only trusts issues and comments written by github-actions[bot]. --- .github/workflows/protected-controls.yml | 57 ++++++- AUTORELEASE.md | 20 ++- autorelease/_state.py | 72 ++++++++- autorelease/control.py | 1 + scripts/notify-autorelease | 33 +++- tests/test_autorelease.py | 194 +++++++++++++++++++++++ 6 files changed, 362 insertions(+), 15 deletions(-) diff --git a/.github/workflows/protected-controls.yml b/.github/workflows/protected-controls.yml index d12aee2..96673f8 100644 --- a/.github/workflows/protected-controls.yml +++ b/.github/workflows/protected-controls.yml @@ -81,6 +81,7 @@ jobs: from autorelease.control import ( ControlError, validate_completed_event_record, + validate_readiness_event_record, validate_evidence_attestation_predicate, validate_evidence_state_record, ) @@ -136,7 +137,21 @@ jobs: print(f"Protected paths changed by the configured owner {author}.") raise SystemExit(0) - # Both trusted-automation exemptions below bind the whole diff, not just + def started_on_main_history(run_sha): + """Whether a run's start commit is `base` or an ancestor of it. + + Publish and implementation runs merge other records while they run, so + main can move past the commit they started from before they file their + own record. The record is still one commit directly on current main. + """ + if not isinstance(run_sha, str) or not re.fullmatch(r"[0-9a-f]{40}", run_sha): + return False + if run_sha == base: + return True + comparison = api_one(f"repos/{repo}/compare/{run_sha}...{base}") + return comparison.get("status") == "ahead" and comparison.get("behind_by") == 0 + + # The trusted-automation exemptions below bind the whole diff, not just # its protected subset: the watcher writes exactly one file, so any # unprotected passenger riding along is proof this is not that PR. evidence_run = re.fullmatch(r"autorelease/evidence-(\d+)", head_ref) @@ -272,13 +287,51 @@ jobs: and run.get("path") == expected_workflow and run.get("event") in allowed_events and run.get("head_branch") == "main" - and run.get("head_sha") == base and run.get("status") == "in_progress" + and started_on_main_history(run.get("head_sha")) ) if direct_parent and trusted_run: print(f"Protected completed event approved from trusted run {run['id']}.") raise SystemExit(0) + # A lifecycle implementation merges its admitted patch, then files the + # php_bin_ready record for that exact merge commit. The record must sit + # directly on the commit it names, from the implementation run that is + # still in progress, so a new branch needs no owner review to publish. + readiness_run = re.fullmatch(r"autorelease/readiness-(\d+)", head_ref) + if ( + len(files) == 1 + and len(protected) == 1 + and re.fullmatch(r"autorelease-events/[A-Za-z0-9._-]+\.json", protected[0]) + and readiness_run + and author == "github-actions[bot]" + and head_repo.lower() == repo.lower() + ): + commit = api_one(f"repos/{repo}/commits/{head}") + run = api_one(f"repos/{repo}/actions/runs/{readiness_run.group(1)}") + content = api_one(f"repos/{repo}/contents/{protected[0]}?ref={head}") + try: + decoded = base64.b64decode(content["content"].replace("\n", ""), validate=True) + record = json.loads(decoded) + merged_commit = validate_readiness_event_record(record) + except (KeyError, ValueError, json.JSONDecodeError, ControlError) as error: + print(f"Invalid readiness record: {error}", file=sys.stderr) + raise SystemExit(1) from error + expected_filename = record["actionKey"].translate(str.maketrans({":": "-", "/": "-"})) + ".json" + direct_parent = [parent.get("sha") for parent in commit.get("parents", [])] == [base] + trusted_run = ( + protected[0] == f"autorelease-events/{expected_filename}" + and merged_commit == base + and run.get("path") == ".github/workflows/autorelease-implement.yml" + and run.get("event") == "workflow_dispatch" + and run.get("head_branch") == "main" + and run.get("status") == "in_progress" + and started_on_main_history(run.get("head_sha")) + ) + if direct_parent and trusted_run: + print(f"Protected readiness record approved from trusted run {run['id']}.") + raise SystemExit(0) + reviews = api(f"repos/{repo}/pulls/{number}/reviews") approved = any( review.get("state") == "APPROVED" diff --git a/AUTORELEASE.md b/AUTORELEASE.md index f2f4112..166bd47 100644 --- a/AUTORELEASE.md +++ b/AUTORELEASE.md @@ -178,10 +178,10 @@ only after the one before succeeded: watcher, ends the job without a second record, and a pull request or branch an earlier attempt left on the run's own `autorelease/event-` branch is withdrawn before the record is filed afresh. A complete record on main - that names another release stops the job. The rerun can only file the - record while main is still the commit the run was dispatched at, because - `Protected controls` binds a publish run's record to exactly that commit; - once main has moved on, the watcher's record recovery is the path. + that names another release stops the job. `Protected controls` accepts the + record while the run is still in progress and started on current main or + an ancestor of it, so a rerun files the record even after main has moved + on; the watcher's record recovery remains the path once the run has ended. Both install jobs pass their read-only token to `mise-php`, whose GitHub API reads would otherwise be rate limited, and restore no mise cache. Every @@ -315,7 +315,13 @@ from the merged policy with its own deterministic scripts. The readiness and event records then merge on their own: `autorelease-events/`, `autorelease-state/`, and `mise-php`'s `readiness/` sit outside CODEOWNERS precisely so their exact-SHA automation PRs satisfy branch protection without a reviewer, while every protected -control still cannot. Publication waits only on machine facts — matching +control still cannot. `Protected controls` admits each record PR only from +`github-actions[bot]` in this repository, as one file directly on the base +commit, from the exact workflow run named by its branch while that run is in +progress on main: evidence from the watcher, completed events from publish or +the watcher, and a lifecycle's `php_bin_ready` record from the implementation +run, which must name the base commit as its validated merge. A new branch +therefore publishes with no human approval. Publication waits only on machine facts — matching `php_bin_ready` and `mise_ready` records at exact commits. The `mise_ready` record names the mise-php synchronization commit it validated, and the record itself merges on top of it, so the publish job requires the captured mise-php @@ -376,7 +382,9 @@ autorelease-investigation-`): Inspect `autorelease-events/`, generated `support-policy.json`, the reviewed `autorelease/policy-invariants.json`, retained workflow artifacts, and the -event issue marker to reconstruct a decision. `scripts/verify-autorelease-system` +event issue marker to reconstruct a decision. The notifier only trusts issues +and comments written by `github-actions[bot]`: the repository is public, so a +copied marker or fingerprint from anyone else is ignored. `scripts/verify-autorelease-system` writes `autorelease-verification.json` and `autorelease-verification.md` into its `--output` directory; both are per-run artifacts, not checked-in files. diff --git a/autorelease/_state.py b/autorelease/_state.py index 93d309b..abec143 100644 --- a/autorelease/_state.py +++ b/autorelease/_state.py @@ -14,6 +14,7 @@ from ._validation import ( ACTION_KEY_RE, + COMMIT_SHA_RE, SHA256_RE, STABLE_VERSION_RE, ControlError, @@ -68,9 +69,78 @@ def validate_completed_event_record(record: dict[str, Any]) -> None: """Validate a durable event as a complete, contiguous legal transition history.""" require(isinstance(record, dict), "autorelease event must be an object") + require(record.get("state") == "complete", "autorelease event is not complete") + _validate_event_history(record) + + +# The exact field set the implementation run writes for a lifecycle readiness record. +READINESS_RECORD_FIELDS = { + "schemaVersion", + "actionKey", + "classification", + "state", + "history", + "phpBinCommit", + "planDigest", + "supportPolicyDigest", + "policyInvariantsDigest", + "evidenceManifestDigest", + "evidenceDigests", +} + + +def validate_readiness_event_record(record: dict[str, Any]) -> str: + """Validate an implementation run's `php_bin_ready` record and return its merged commit. + + The implementation run writes exactly one transition, `detected` to + `php_bin_ready`, whose only evidence is the validated merge of the admitted + lifecycle patch. That merge commit is what the record vouches for, so the trusted + automation exemption binds it to the pull request's base: the record can only be + accepted directly on top of the commit it names. + """ + require(isinstance(record, dict), "autorelease event must be an object") + require(set(record) == READINESS_RECORD_FIELDS, "readiness record fields changed") + require(record.get("state") == "php_bin_ready", "readiness record is not at php_bin_ready") + classification = record.get("classification") + require(classification in {"new_branch", "branch_eol"}, "readiness record is not a lifecycle action") + require( + record.get("actionKey", "").split(":", 1)[0] == classification, + "readiness record action key does not match its classification", + ) + _validate_event_history(record) + history = record["history"] + require( + len(history) == 1 and history[0]["from"] == "detected" and history[0]["to"] == "php_bin_ready", + "readiness record history is not a single detected to php_bin_ready transition", + ) + evidence = history[0]["evidence"] + require( + len(evidence) == 1 + and set(evidence[0]) == {"kind", "commit", "planDigest"} + and evidence[0]["kind"] == "validated_merge", + "readiness record evidence is not one validated merge", + ) + commit = evidence[0]["commit"] + require(isinstance(commit, str) and bool(COMMIT_SHA_RE.fullmatch(commit)), "readiness merge commit is invalid") + require(record.get("phpBinCommit") == commit, "readiness record names a different php-bin commit") + require(evidence[0]["planDigest"] == record.get("planDigest"), "readiness record plan digest differs") + for field in ("planDigest", "supportPolicyDigest", "policyInvariantsDigest", "evidenceManifestDigest"): + value = record.get(field) + require(isinstance(value, str) and bool(SHA256_RE.fullmatch(value)), f"readiness record {field} is invalid") + digests = record.get("evidenceDigests") + require( + isinstance(digests, list) + and bool(digests) + and all(isinstance(item, str) and SHA256_RE.fullmatch(item) for item in digests), + "readiness record evidence digests are invalid", + ) + return commit + + +def _validate_event_history(record: dict[str, Any]) -> None: + """Require a versioned event whose history is a contiguous chain of legal transitions.""" require(record.get("schemaVersion") == 1, "autorelease event version is invalid") require(bool(ACTION_KEY_RE.fullmatch(record.get("actionKey", ""))), "autorelease event action key is invalid") - require(record.get("state") == "complete", "autorelease event is not complete") history = record.get("history") require(isinstance(history, list) and bool(history), "autorelease event has no transition history") current = history[0].get("from") if isinstance(history[0], dict) else None diff --git a/autorelease/control.py b/autorelease/control.py index 63d3580..4e3b983 100755 --- a/autorelease/control.py +++ b/autorelease/control.py @@ -116,6 +116,7 @@ transition_event, unrecorded_published_release, validate_completed_event_record, + validate_readiness_event_record, watch_decision, ) from autorelease._validation import ( # noqa: E402 diff --git a/scripts/notify-autorelease b/scripts/notify-autorelease index 5fe2c67..f24f628 100755 --- a/scripts/notify-autorelease +++ b/scripts/notify-autorelease @@ -39,6 +39,20 @@ FINGERPRINT_RE = re.compile( ) +# The repository is public, so anyone can open an issue or comment carrying a +# marker. Only what the workflow itself wrote identifies an event issue; anything +# else must neither capture nor silence the owner notification. + + +def written_by_workflow(item: dict) -> bool: + """Whether a `gh issue list` entry or a REST comment was written by GitHub Actions.""" + author = item.get("author") + if isinstance(author, dict): + return author.get("login") == "app/github-actions" and author.get("is_bot") is True + user = item.get("user") or {} + return user.get("login") == "github-actions[bot]" and user.get("type") == "Bot" + + def find_issue(repo: str, action_key: str) -> dict | None: matches: dict[int, dict] = {} for prefix in MARKER_PREFIXES: @@ -54,13 +68,13 @@ def find_issue(repo: str, action_key: str) -> dict | None: "--search", f'"{marker}" in:body', "--json", - "number,body,state,url", + "number,body,state,url,author", "--limit", "100", ) ) for issue in issues: - if marker in issue.get("body", ""): + if marker in issue.get("body", "") and written_by_workflow(issue): matches[issue["number"]] = issue if len(matches) > 1: raise ControlError("multiple issues exist for one action key") @@ -71,11 +85,18 @@ def discover_github_prior(repo: str, action_key: str) -> dict | None: issue = find_issue(repo, action_key) if issue is None: return None - detail = json.loads( - gh("issue", "view", str(issue["number"]), "--repo", repo, "--json", "body,comments") + # REST comments name the author unambiguously (`github-actions[bot]`, type Bot), + # where `gh issue view` reports a bare `github-actions` login. + pages = json.loads( + gh("api", f"repos/{repo}/issues/{issue['number']}/comments", "--paginate", "--slurp") + ) + bodies = [issue.get("body", "")] + bodies.extend( + comment.get("body", "") + for page in pages + for comment in page + if written_by_workflow(comment) ) - bodies = [detail.get("body", "")] - bodies.extend(comment.get("body", "") for comment in detail.get("comments", [])) fingerprints = [ match.group(1) for body in bodies for match in FINGERPRINT_RE.finditer(body) ] diff --git a/tests/test_autorelease.py b/tests/test_autorelease.py index 1b633a1..ace5e82 100644 --- a/tests/test_autorelease.py +++ b/tests/test_autorelease.py @@ -1,3 +1,4 @@ +import base64 import contextlib import hashlib import http.client @@ -60,6 +61,7 @@ validate_archive, validate_completed_event_record, validate_evidence_attestation_predicate, + validate_readiness_event_record, validate_evidence_state_record, validate_recaptured_evidence, validate_release_is_newest_patch, @@ -1718,6 +1720,7 @@ def test_notification_search_covers_current_and_pre_rename_markers(self): "url": f"https://example.invalid/issues/{number}", "state": "CLOSED", "body": f"", + "author": {"login": "app/github-actions", "is_bot": True}, } gh = mock.Mock( side_effect=lambda *arguments, issue=issue, prefix=prefix: json.dumps([issue]) @@ -1728,6 +1731,51 @@ def test_notification_search_covers_current_and_pre_rename_markers(self): found = find_issue("Bigpixelrocket/php-bin", "new_patch:8.5.9") self.assertEqual(issue, found) + def test_notification_ignores_markers_written_by_anyone_but_the_workflow(self): + namespace = runpy.run_path( + str(pathlib.Path(__file__).resolve().parents[1] / "scripts/notify-autorelease") + ) + find_issue = namespace["find_issue"] + discover = namespace["discover_github_prior"] + marker = "" + bot_issue = { + "number": 47, + "url": "https://example.invalid/issues/47", + "state": "OPEN", + "body": f"{marker}\n", + "author": {"login": "app/github-actions", "is_bot": True}, + } + # A public repository lets anyone copy the marker, including a user whose + # login merely resembles the workflow's. + impostors = [ + {**bot_issue, "number": 48, "author": {"login": "someone", "is_bot": False}}, + {**bot_issue, "number": 49, "author": {"login": "github-actions", "is_bot": False}}, + {**bot_issue, "number": 50, "author": {"login": "app/github-actions", "is_bot": False}}, + ] + forged = f"" + comments = [[ + {"body": forged, "user": {"login": "someone", "type": "User"}}, + {"body": forged, "user": {"login": "github-actions", "type": "User"}}, + ]] + + def gh(*arguments): + if arguments[0] == "api": + return json.dumps(comments) + return json.dumps([*impostors, bot_issue]) if "autorelease-action-key" in " ".join(arguments) else "[]" + + with mock.patch.dict(find_issue.__globals__, {"gh": gh}): + self.assertEqual(47, find_issue("Bigpixelrocket/php-bin", "new_patch:8.5.9")["number"]) + prior = discover("Bigpixelrocket/php-bin", "new_patch:8.5.9") + self.assertEqual(f"sha256:{'a' * 64}", prior["fingerprint"]) + + comments[0].append({"body": forged, "user": {"login": "github-actions[bot]", "type": "Bot"}}) + with mock.patch.dict(find_issue.__globals__, {"gh": gh}): + prior = discover("Bigpixelrocket/php-bin", "new_patch:8.5.9") + self.assertEqual(f"sha256:{'b' * 64}", prior["fingerprint"]) + + with mock.patch.dict(find_issue.__globals__, {"gh": lambda *arguments: json.dumps(impostors)}): + self.assertIsNone(find_issue("Bigpixelrocket/php-bin", "new_patch:8.5.9")) + def test_notification_transition_reuses_retained_issue_identity(self): issue = {"number": 10, "url": "https://example.invalid/issues/10", "state": "OPEN"} self.assertEqual(issue, retained_notification_issue({"issue": issue})) @@ -2558,6 +2606,152 @@ def test_protected_controls_pass_owner_authored_changes_before_bot_exemptions(se self.assertLess(protected.index("No protected control path changed."), owner_pass) self.assertLess(owner_pass, protected.index('re.fullmatch(r"autorelease/evidence-')) + def run_protected_controls(self, pr, responses): + """Run the workflow's own evaluator against a fake `gh`, returning (exit code, output).""" + root = pathlib.Path(__file__).resolve().parents[1] + from autorelease.verify import load_workflow + + workflow = load_workflow(root / ".github/workflows/protected-controls.yml") + step = next( + step for step in workflow["jobs"]["protected-controls"]["steps"] + if step.get("name") == "Require exact-head owner approval for protected paths" + ) + script = step["run"].split("<<'PY'\n", 1)[1].rsplit("\nPY", 1)[0] + with tempfile.TemporaryDirectory() as temp: + work = pathlib.Path(temp) + (work / "responses.json").write_text(json.dumps(responses)) + fake = work / "gh" + fake.write_text( + "#!/usr/bin/env python3\n" + "import json, os, sys\n" + "args = sys.argv[1:]\n" + "responses = json.loads(open(os.environ['FAKE_GH_RESPONSES']).read())\n" + "path = args[1]\n" + "if path not in responses:\n" + " sys.exit(f'unexpected gh api {path}')\n" + "body = responses[path]\n" + "print(json.dumps([body] if '--slurp' in args else body))\n" + ) + fake.chmod(0o755) + env = { + **os.environ, + "PATH": f"{work}:{os.environ['PATH']}", + "FAKE_GH_RESPONSES": str(work / "responses.json"), + "PYTHONPATH": str(root), + "REPOSITORY": "Bigpixelrocket/php-bin", + "PR_NUMBER": "7", + "PROTECTED_REVIEWER": "loadinglucian", + **pr, + } + result = subprocess.run( + ["python3", "-c", script], cwd=root, env=env, capture_output=True, text=True + ) + return result.returncode, result.stdout + result.stderr + + def test_protected_controls_admit_exact_implementation_readiness_records(self): + base, head, start = "b" * 40, "c" * 40, "a" * 40 + record = transition_event( + { + "schemaVersion": 1, + "actionKey": "new_branch:8.6", + "classification": "new_branch", + "state": "detected", + "history": [], + "phpBinCommit": base, + "planDigest": "sha256:" + "1" * 64, + "supportPolicyDigest": "sha256:" + "2" * 64, + "policyInvariantsDigest": "sha256:" + "3" * 64, + "evidenceManifestDigest": "sha256:" + "4" * 64, + "evidenceDigests": ["sha256:" + "5" * 64], + }, + "php_bin_ready", + [{"kind": "validated_merge", "commit": base, "planDigest": "sha256:" + "1" * 64}], + ) + self.assertEqual(base, validate_readiness_event_record(record)) + path = "autorelease-events/new_branch-8.6.json" + + def scenario(record=record, files=(path,), author="github-actions[bot]", ref="autorelease/readiness-99", + run_path=".github/workflows/autorelease-implement.yml", run_sha=start, + status="in_progress", compare=("ahead", 0)): + encoded = base64.b64encode(json.dumps(record).encode()).decode() + responses = { + "repos/Bigpixelrocket/php-bin/pulls/7/files": [{"filename": name} for name in files], + f"repos/Bigpixelrocket/php-bin/commits/{head}": {"parents": [{"sha": base}]}, + "repos/Bigpixelrocket/php-bin/actions/runs/99": { + "id": 99, "path": run_path, "event": "workflow_dispatch", + "head_branch": "main", "head_sha": run_sha, "status": status, + }, + f"repos/Bigpixelrocket/php-bin/contents/{path}?ref={head}": {"content": encoded}, + f"repos/Bigpixelrocket/php-bin/compare/{run_sha}...{base}": { + "status": compare[0], "behind_by": compare[1], + }, + "repos/Bigpixelrocket/php-bin/pulls/7/reviews": [], + } + pr = {"HEAD_SHA": head, "BASE_SHA": base, "HEAD_REF": ref, + "HEAD_REPOSITORY": "Bigpixelrocket/php-bin", "PR_AUTHOR": author} + return self.run_protected_controls(pr, responses) + + code, output = scenario() + self.assertEqual(0, code, output) + self.assertIn("Protected readiness record approved from trusted run 99.", output) + # The run may have started on main itself, or on any ancestor of it. + self.assertEqual(0, scenario(run_sha=base)[0]) + + rejected = { + "a diverged start commit": {"compare": ("diverged", 1)}, + "a finished run": {"status": "completed"}, + "another workflow": {"run_path": ".github/workflows/autorelease-watch.yml"}, + "a human author": {"author": "someone"}, + "another branch prefix": {"ref": "autorelease/evidence-99"}, + "a passenger file": {"files": (path, "README.md")}, + "a record for another commit": {"record": {**record, "phpBinCommit": "d" * 40, "history": [ + {**record["history"][0], "evidence": [{**record["history"][0]["evidence"][0], "commit": "d" * 40}]} + ]}}, + "a record naming two commits": {"record": {**record, "phpBinCommit": "d" * 40}}, + "a record not at php_bin_ready": {"record": {**record, "state": "detected"}}, + "a patch action": {"record": {**record, "actionKey": "new_patch:8.6.0", "classification": "new_patch"}}, + "an extra field": {"record": {**record, "note": "x"}}, + } + for label, overrides in rejected.items(): + with self.subTest(label): + code, output = scenario(**overrides) + self.assertNotEqual(0, code, output) + self.assertNotIn("approved from trusted run", output) + code, output = scenario(files=(path.replace("8.6", "8.7"),)) + self.assertNotEqual(0, code, output) + + def test_protected_controls_admit_a_publish_record_after_main_moved(self): + base, head, start = "b" * 40, "c" * 40, "a" * 40 + record = {"schemaVersion": 1, "actionKey": "new_patch:8.5.9", "state": "detected"} + for target in ("php_bin_ready", "release_requested", "released", "public_install_verified", "complete"): + record = transition_event(record, target, [{"kind": "fixture", "value": target}]) + path = "autorelease-events/new_patch-8.5.9.json" + + def scenario(compare): + responses = { + "repos/Bigpixelrocket/php-bin/pulls/7/files": [{"filename": path}], + f"repos/Bigpixelrocket/php-bin/commits/{head}": {"parents": [{"sha": base}]}, + "repos/Bigpixelrocket/php-bin/actions/runs/42": { + "id": 42, "path": ".github/workflows/autorelease-publish.yml", "event": "workflow_dispatch", + "head_branch": "main", "head_sha": start, "status": "in_progress", + }, + f"repos/Bigpixelrocket/php-bin/contents/{path}?ref={head}": { + "content": base64.b64encode(json.dumps(record).encode()).decode(), + }, + f"repos/Bigpixelrocket/php-bin/compare/{start}...{base}": {"status": compare[0], "behind_by": compare[1]}, + "repos/Bigpixelrocket/php-bin/pulls/7/reviews": [], + } + pr = {"HEAD_SHA": head, "BASE_SHA": base, "HEAD_REF": "autorelease/event-42", + "HEAD_REPOSITORY": "Bigpixelrocket/php-bin", "PR_AUTHOR": "github-actions[bot]"} + return self.run_protected_controls(pr, responses) + + code, output = scenario(("ahead", 0)) + self.assertEqual(0, code, output) + self.assertIn("Protected completed event approved from trusted run 42.", output) + for compare in (("behind", 0), ("diverged", 2), ("ahead", 1)): + with self.subTest(compare=compare): + self.assertNotEqual(0, scenario(compare)[0]) + def test_recovered_event_records_use_the_trusted_watcher_branch_prefix(self): root = pathlib.Path(__file__).resolve().parents[1] watcher = (root / ".github/workflows/autorelease-watch.yml").read_text() From 149a1c20c04ce0b2e413db2eed3719b92bd14cd4 Mon Sep 17 00:00:00 2001 From: Lucian Behind The Scenes Date: Tue, 29 Sep 2026 18:13:23 +0300 Subject: [PATCH 2/3] fix(autorelease): bind readiness records to the base policy The readiness exemption now requires the record's policy digests to match the base tree, an uncomparable start commit is rejected with a reason, and a non-string action key fails validation instead of raising. --- .github/workflows/protected-controls.yml | 16 +++++- AUTORELEASE.md | 10 ++-- autorelease/_state.py | 10 ++-- tests/test_autorelease.py | 66 ++++++++++++++++++++---- 4 files changed, 84 insertions(+), 18 deletions(-) diff --git a/.github/workflows/protected-controls.yml b/.github/workflows/protected-controls.yml index 96673f8..7f51703 100644 --- a/.github/workflows/protected-controls.yml +++ b/.github/workflows/protected-controls.yml @@ -80,6 +80,7 @@ jobs: from autorelease.control import ( ControlError, + sha256_file, validate_completed_event_record, validate_readiness_event_record, validate_evidence_attestation_predicate, @@ -148,7 +149,11 @@ jobs: return False if run_sha == base: return True - comparison = api_one(f"repos/{repo}/compare/{run_sha}...{base}") + try: + comparison = api_one(f"repos/{repo}/compare/{run_sha}...{base}") + except subprocess.CalledProcessError: + print(f"Run start commit {run_sha} could not be compared with {base}.", file=sys.stderr) + return False return comparison.get("status") == "ahead" and comparison.get("behind_by") == 0 # The trusted-automation exemptions below bind the whole diff, not just @@ -319,9 +324,18 @@ jobs: raise SystemExit(1) from error expected_filename = record["actionKey"].translate(str.maketrans({":": "-", "/": "-"})) + ".json" direct_parent = [parent.get("sha") for parent in commit.get("parents", [])] == [base] + # This step runs on a checkout of `base`, so the policy digests the + # record carries, the only fields publish and EOL completion act on, + # must describe exactly the tree the record sits on. + policy_bound = ( + record["supportPolicyDigest"] == sha256_file(pathlib.Path("support-policy.json")) + and record["policyInvariantsDigest"] + == sha256_file(pathlib.Path("autorelease/policy-invariants.json")) + ) trusted_run = ( protected[0] == f"autorelease-events/{expected_filename}" and merged_commit == base + and policy_bound and run.get("path") == ".github/workflows/autorelease-implement.yml" and run.get("event") == "workflow_dispatch" and run.get("head_branch") == "main" diff --git a/AUTORELEASE.md b/AUTORELEASE.md index 166bd47..f3c39a9 100644 --- a/AUTORELEASE.md +++ b/AUTORELEASE.md @@ -318,10 +318,12 @@ PRs satisfy branch protection without a reviewer, while every protected control still cannot. `Protected controls` admits each record PR only from `github-actions[bot]` in this repository, as one file directly on the base commit, from the exact workflow run named by its branch while that run is in -progress on main: evidence from the watcher, completed events from publish or -the watcher, and a lifecycle's `php_bin_ready` record from the implementation -run, which must name the base commit as its validated merge. A new branch -therefore publishes with no human approval. Publication waits only on machine facts — matching +progress on main: evidence from the watcher, which must also have started at +exactly the base commit and carry a matching attestation; completed events +from publish or the watcher; and a lifecycle's `php_bin_ready` record from the +implementation run, which must name the base commit as its validated merge and +carry the policy digests of the base tree. A new branch therefore publishes +with no human approval. Publication waits only on machine facts — matching `php_bin_ready` and `mise_ready` records at exact commits. The `mise_ready` record names the mise-php synchronization commit it validated, and the record itself merges on top of it, so the publish job requires the captured mise-php diff --git a/autorelease/_state.py b/autorelease/_state.py index abec143..eda200e 100644 --- a/autorelease/_state.py +++ b/autorelease/_state.py @@ -101,13 +101,13 @@ def validate_readiness_event_record(record: dict[str, Any]) -> str: require(isinstance(record, dict), "autorelease event must be an object") require(set(record) == READINESS_RECORD_FIELDS, "readiness record fields changed") require(record.get("state") == "php_bin_ready", "readiness record is not at php_bin_ready") + _validate_event_history(record) classification = record.get("classification") require(classification in {"new_branch", "branch_eol"}, "readiness record is not a lifecycle action") require( - record.get("actionKey", "").split(":", 1)[0] == classification, + record["actionKey"].split(":", 1)[0] == classification, "readiness record action key does not match its classification", ) - _validate_event_history(record) history = record["history"] require( len(history) == 1 and history[0]["from"] == "detected" and history[0]["to"] == "php_bin_ready", @@ -140,7 +140,11 @@ def validate_readiness_event_record(record: dict[str, Any]) -> str: def _validate_event_history(record: dict[str, Any]) -> None: """Require a versioned event whose history is a contiguous chain of legal transitions.""" require(record.get("schemaVersion") == 1, "autorelease event version is invalid") - require(bool(ACTION_KEY_RE.fullmatch(record.get("actionKey", ""))), "autorelease event action key is invalid") + action_key = record.get("actionKey") + require( + isinstance(action_key, str) and bool(ACTION_KEY_RE.fullmatch(action_key)), + "autorelease event action key is invalid", + ) history = record.get("history") require(isinstance(history, list) and bool(history), "autorelease event has no transition history") current = history[0].get("from") if isinstance(history[0], dict) else None diff --git a/tests/test_autorelease.py b/tests/test_autorelease.py index ace5e82..eefbff1 100644 --- a/tests/test_autorelease.py +++ b/tests/test_autorelease.py @@ -2659,8 +2659,8 @@ def test_protected_controls_admit_exact_implementation_readiness_records(self): "history": [], "phpBinCommit": base, "planDigest": "sha256:" + "1" * 64, - "supportPolicyDigest": "sha256:" + "2" * 64, - "policyInvariantsDigest": "sha256:" + "3" * 64, + "supportPolicyDigest": sha256_file(ROOT / "support-policy.json"), + "policyInvariantsDigest": sha256_file(ROOT / "autorelease/policy-invariants.json"), "evidenceManifestDigest": "sha256:" + "4" * 64, "evidenceDigests": ["sha256:" + "5" * 64], }, @@ -2672,23 +2672,25 @@ def test_protected_controls_admit_exact_implementation_readiness_records(self): def scenario(record=record, files=(path,), author="github-actions[bot]", ref="autorelease/readiness-99", run_path=".github/workflows/autorelease-implement.yml", run_sha=start, - status="in_progress", compare=("ahead", 0)): + status="in_progress", compare=("ahead", 0), head_repo="Bigpixelrocket/php-bin", + run_branch="main", run_event="workflow_dispatch", parents=(base,)): encoded = base64.b64encode(json.dumps(record).encode()).decode() responses = { "repos/Bigpixelrocket/php-bin/pulls/7/files": [{"filename": name} for name in files], - f"repos/Bigpixelrocket/php-bin/commits/{head}": {"parents": [{"sha": base}]}, + f"repos/Bigpixelrocket/php-bin/commits/{head}": {"parents": [{"sha": sha} for sha in parents]}, "repos/Bigpixelrocket/php-bin/actions/runs/99": { - "id": 99, "path": run_path, "event": "workflow_dispatch", - "head_branch": "main", "head_sha": run_sha, "status": status, + "id": 99, "path": run_path, "event": run_event, + "head_branch": run_branch, "head_sha": run_sha, "status": status, }, f"repos/Bigpixelrocket/php-bin/contents/{path}?ref={head}": {"content": encoded}, - f"repos/Bigpixelrocket/php-bin/compare/{run_sha}...{base}": { - "status": compare[0], "behind_by": compare[1], - }, "repos/Bigpixelrocket/php-bin/pulls/7/reviews": [], } + if compare is not None: + responses[f"repos/Bigpixelrocket/php-bin/compare/{run_sha}...{base}"] = { + "status": compare[0], "behind_by": compare[1], + } pr = {"HEAD_SHA": head, "BASE_SHA": base, "HEAD_REF": ref, - "HEAD_REPOSITORY": "Bigpixelrocket/php-bin", "PR_AUTHOR": author} + "HEAD_REPOSITORY": head_repo, "PR_AUTHOR": author} return self.run_protected_controls(pr, responses) code, output = scenario() @@ -2699,6 +2701,14 @@ def scenario(record=record, files=(path,), author="github-actions[bot]", ref="au rejected = { "a diverged start commit": {"compare": ("diverged", 1)}, + "an uncomparable start commit": {"compare": None}, + "a fork": {"head_repo": "someone/php-bin"}, + "a run on another branch": {"run_branch": "feature"}, + "a run started by another event": {"run_event": "push"}, + "a record not directly on base": {"parents": ("d" * 40,)}, + "a merge commit": {"parents": (base, "d" * 40)}, + "a support policy the base does not hold": {"record": {**record, "supportPolicyDigest": "sha256:" + "2" * 64}}, + "invariants the base does not hold": {"record": {**record, "policyInvariantsDigest": "sha256:" + "3" * 64}}, "a finished run": {"status": "completed"}, "another workflow": {"run_path": ".github/workflows/autorelease-watch.yml"}, "a human author": {"author": "someone"}, @@ -2720,6 +2730,42 @@ def scenario(record=record, files=(path,), author="github-actions[bot]", ref="au code, output = scenario(files=(path.replace("8.6", "8.7"),)) self.assertNotEqual(0, code, output) + def test_readiness_record_validator_rejects_every_deviation(self): + commit = "b" * 40 + record = transition_event( + { + "schemaVersion": 1, "actionKey": "branch_eol:8.2:2026-12-31", "classification": "branch_eol", + "state": "detected", "history": [], "phpBinCommit": commit, + "planDigest": "sha256:" + "1" * 64, "supportPolicyDigest": "sha256:" + "2" * 64, + "policyInvariantsDigest": "sha256:" + "3" * 64, "evidenceManifestDigest": "sha256:" + "4" * 64, + "evidenceDigests": ["sha256:" + "5" * 64], + }, + "php_bin_ready", + [{"kind": "validated_merge", "commit": commit, "planDigest": "sha256:" + "1" * 64}], + ) + self.assertEqual(commit, validate_readiness_event_record(record)) + merge = record["history"][0]["evidence"][0] + + def with_evidence(evidence): + return {**record, "history": [{**record["history"][0], "evidence": evidence}]} + + invalid = { + "a non-string action key": {**record, "actionKey": 7}, + "two evidence items": with_evidence([merge, merge]), + "another evidence kind": with_evidence([{**merge, "kind": "published_release"}]), + "an extra evidence field": with_evidence([{**merge, "note": "x"}]), + "a different plan digest": with_evidence([{**merge, "planDigest": "sha256:" + "9" * 64}]), + "a short commit": {**with_evidence([{**merge, "commit": "b" * 7}]), "phpBinCommit": "b" * 7}, + "no evidence digests": {**record, "evidenceDigests": []}, + "a malformed digest": {**record, "supportPolicyDigest": "2" * 64}, + "a classification that disagrees": {**record, "classification": "new_branch"}, + "a second transition": {**record, "history": [*record["history"], record["history"][0]]}, + } + for label, candidate in invalid.items(): + with self.subTest(label): + with self.assertRaises(ControlError): + validate_readiness_event_record(candidate) + def test_protected_controls_admit_a_publish_record_after_main_moved(self): base, head, start = "b" * 40, "c" * 40, "a" * 40 record = {"schemaVersion": 1, "actionKey": "new_patch:8.5.9", "state": "detected"} From d4058b9d308aed24c2294dd295b88cacefb1a29c Mon Sep 17 00:00:00 2001 From: Lucian Behind The Scenes Date: Tue, 29 Sep 2026 18:19:10 +0300 Subject: [PATCH 3/3] test(autorelease): pin the readiness rejection reasons --- .github/workflows/protected-controls.yml | 2 +- tests/test_autorelease.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/protected-controls.yml b/.github/workflows/protected-controls.yml index 7f51703..da0d9ee 100644 --- a/.github/workflows/protected-controls.yml +++ b/.github/workflows/protected-controls.yml @@ -151,7 +151,7 @@ jobs: return True try: comparison = api_one(f"repos/{repo}/compare/{run_sha}...{base}") - except subprocess.CalledProcessError: + except (subprocess.CalledProcessError, json.JSONDecodeError): print(f"Run start commit {run_sha} could not be compared with {base}.", file=sys.stderr) return False return comparison.get("status") == "ahead" and comparison.get("behind_by") == 0 diff --git a/tests/test_autorelease.py b/tests/test_autorelease.py index eefbff1..cf428d2 100644 --- a/tests/test_autorelease.py +++ b/tests/test_autorelease.py @@ -2729,6 +2729,8 @@ def scenario(record=record, files=(path,), author="github-actions[bot]", ref="au self.assertNotIn("approved from trusted run", output) code, output = scenario(files=(path.replace("8.6", "8.7"),)) self.assertNotEqual(0, code, output) + code, output = scenario(compare=None) + self.assertIn("could not be compared with", output) def test_readiness_record_validator_rejects_every_deviation(self): commit = "b" * 40 @@ -2760,6 +2762,11 @@ def with_evidence(evidence): "a malformed digest": {**record, "supportPolicyDigest": "2" * 64}, "a classification that disagrees": {**record, "classification": "new_branch"}, "a second transition": {**record, "history": [*record["history"], record["history"][0]]}, + "a legal but longer history": transition_event( + transition_event({**record, "state": "detected", "history": []}, "blocked", [merge]), + "php_bin_ready", + [merge], + ), } for label, candidate in invalid.items(): with self.subTest(label):