From a8cfcb979f6aa40928308c23f459360a5f924fc0 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Thu, 10 Sep 2026 22:57:27 +0200 Subject: [PATCH 1/5] Require contents: write for Guard Ready mutations. markPullRequestReadyForReview with GITHUB_TOKEN needs contents: write. Without it GraphQL returns HTTP 200 and leaves isDraft unchanged. --- .github/actions/a38-guard/action.yml | 2 +- docs/a38-guard.md | 2 +- examples/a38-guard.yml | 4 +++- src/agent_cli/pr_lifecycle.py | 13 ++++++++++--- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.github/actions/a38-guard/action.yml b/.github/actions/a38-guard/action.yml index 6ef4fae4..6414ce68 100644 --- a/.github/actions/a38-guard/action.yml +++ b/.github/actions/a38-guard/action.yml @@ -5,7 +5,7 @@ description: > inputs: token: - description: GitHub token (contents read, issues write, pull-requests write, statuses write; actions/checks read for lifecycle, actions write for workflow approval) + description: GitHub token (contents write for markPullRequestReadyForReview, issues write, pull-requests write, statuses write; actions/checks read for lifecycle, actions write for workflow approval) required: true repository: description: owner/name override (workflow_dispatch / standalone) diff --git a/docs/a38-guard.md b/docs/a38-guard.md index da761cd3..342a515a 100644 --- a/docs/a38-guard.md +++ b/docs/a38-guard.md @@ -20,7 +20,7 @@ For the composite action, `A38_RUNTIME_REVISION` is overwritten from `${{ github Standalone execution accepts the same explicit trusted `A38_RUNTIME_REVISION`. Without it, source-checkout fallback is allowed only when the loaded module is exactly `/src/agent_cli/a38_guard.py`, `/.git` belongs to that root, Git reports the same top-level using explicit `--git-dir` and `--work-tree`, and `HEAD` is lowercase 40-hex. The lookup is anchored to the module source root, removes inherited `GIT_*` variables, and never discovers from the current directory or an enclosing consumer checkout. A non-Git packaged install requires the explicit trusted revision; it never guesses `develop` or another moving ref. Closed PRs, ignored events, and empty all-open scans remain successful no-ops and do not need provenance resolution. -The token requires contents read, pull requests write, issues write and statuses write. Publishing the guard comment on a pull request needs `pull-requests: write` for `GITHUB_TOKEN`; `issues: write` alone is not enough and yields 403. Policy migrations also require permission to read collaborators' effective repository permissions. If that API is unavailable, the migration fails closed. Use a dedicated GitHub App or service account with the necessary repository access for external operation. Tokens are taken from `GH_TOKEN` or `GITHUB_TOKEN` and never printed. +The token requires contents write, pull requests write, issues write and statuses write. `markPullRequestReadyForReview` needs `contents: write` on `GITHUB_TOKEN` or it returns HTTP 200 with `isDraft` unchanged (`Resource not accessible by integration`). Publishing the guard comment on a pull request needs `pull-requests: write` for `GITHUB_TOKEN`; `issues: write` alone is not enough and yields 403. Policy migrations also require permission to read collaborators' effective repository permissions. If that API is unavailable, the migration fails closed. Use a dedicated GitHub App or service account with the necessary repository access for external operation. Tokens are taken from `GH_TOKEN` or `GITHUB_TOKEN` and never printed. Actions must actually be available for event-driven operation. When Actions are blocked or unavailable, run the same reconciler on a trusted external host: diff --git a/examples/a38-guard.yml b/examples/a38-guard.yml index 894ed4dc..cff960a3 100644 --- a/examples/a38-guard.yml +++ b/examples/a38-guard.yml @@ -28,10 +28,12 @@ concurrency: cancel-in-progress: false permissions: - contents: read + contents: write issues: write pull-requests: write statuses: write + actions: write + checks: read jobs: guard: diff --git a/src/agent_cli/pr_lifecycle.py b/src/agent_cli/pr_lifecycle.py index 0c6d5aef..cf912ca6 100644 --- a/src/agent_cli/pr_lifecycle.py +++ b/src/agent_cli/pr_lifecycle.py @@ -287,12 +287,19 @@ def _transition(api: Any, node: str, draft: bool) -> Mapping: + "(input: {pullRequestId: $id}) { pullRequest { id isDraft headRefOid baseRefOid } } }") status, data, _ = api.request("POST", "/graphql", body={"query": query, "variables": {"id": node}}, retry=False) pull = _field(data, "data", operation, "pullRequest") - if status != 200 or _field(data, "errors") or _field(pull, "id") != node: - raise GuardError(f"PR lifecycle mutation failed (HTTP {status})") + errors = _field(data, "errors") + detail = "" + if isinstance(errors, list) and errors and isinstance(errors[0], Mapping): + detail = f": {errors[0].get('message') or 'graphql error'}" + if status != 200 or errors or _field(pull, "id") != node: + raise GuardError(f"PR lifecycle mutation failed (HTTP {status}){detail}") if _field(pull, "isDraft") is not draft: if draft and _field(pull, "isDraft") is False: raise LifecycleDraftUnchanged - raise GuardError(f"PR lifecycle mutation failed (HTTP {status})") + raise GuardError( + f"PR lifecycle mutation failed (HTTP {status}): isDraft unchanged; " + "GITHUB_TOKEN needs contents: write for markPullRequestReadyForReview" + ) return pull From f09942a4c010996325e41d8252270a9a17b7fe64 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Thu, 10 Sep 2026 22:58:11 +0200 Subject: [PATCH 2/5] Assert GraphQL Ready failures include the API error message. --- tests/test_pr_lifecycle.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pr_lifecycle.py b/tests/test_pr_lifecycle.py index 3622644c..5dc970f3 100644 --- a/tests/test_pr_lifecycle.py +++ b/tests/test_pr_lifecycle.py @@ -474,7 +474,7 @@ def test_graphql_error_is_not_a_successful_transition(): fake = LifecycleAPI() fake.runs.clear() fake.graphql_error = True - with pytest.raises(GuardError, match="mutation failed"): + with pytest.raises(GuardError, match="denied"): reconcile_pull(fake.api(), REPO, 1) assert not fake.transitions assert all('"phase": "applied"' not in c["body"] for c in fake.comments) From 7977b9d33bfbd329cf4659e032d4895433b6f6e2 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:05:52 +0200 Subject: [PATCH 3/5] Cover unchanged-draft Ready failures and document contents: write. --- docs/a38-guard.md | 3 ++- tests/test_pr_lifecycle.py | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/a38-guard.md b/docs/a38-guard.md index 342a515a..adf5b418 100644 --- a/docs/a38-guard.md +++ b/docs/a38-guard.md @@ -234,7 +234,8 @@ EN/DE comment says the Draft conversion did not take effect. Authorization recor authenticated bot's numeric user ID can supply these records. Dry run performs no writes, including audit comments. -The adopting workflow owns runner routing, `actions` and `checks` read access, +The adopting workflow owns runner routing, `contents: write` for +`markPullRequestReadyForReview`, `actions` and `checks` read access, `pull-requests`/`issues`/`statuses` write access, and `actions: write` for initial workflow approval. The guard authorizes waiting allowlisted fork runs **before** it mutates Ready or Draft, so a failed convert-to-draft cannot skip approval. diff --git a/tests/test_pr_lifecycle.py b/tests/test_pr_lifecycle.py index 5dc970f3..5da8b56a 100644 --- a/tests/test_pr_lifecycle.py +++ b/tests/test_pr_lifecycle.py @@ -28,6 +28,7 @@ def __init__(self): self.graphql_error = False self.graphql_noop_draft = False self.graphql_ready_without_rest = False + self.graphql_noop_ready = False self.mutate_during_transition = False self.fail_comment_once = False @@ -50,6 +51,10 @@ def request_fn(self, method, url, body=None): return 200, {"data": {operation: {"pullRequest": { "id": "PR_example", "isDraft": False, "headRefOid": self.pull["head"]["sha"], "baseRefOid": BASE}}}}, {} + if self.graphql_noop_ready and operation == "markPullRequestReadyForReview": + return 200, {"data": {operation: {"pullRequest": { + "id": "PR_example", "isDraft": True, + "headRefOid": self.pull["head"]["sha"], "baseRefOid": BASE}}}}, {} self.pull["draft"] = operation == "convertPullRequestToDraft" self.transitions.append(self.pull["draft"]) if self.mutate_during_transition: @@ -480,6 +485,17 @@ def test_graphql_error_is_not_a_successful_transition(): assert all('"phase": "applied"' not in c["body"] for c in fake.comments) +def test_ready_mutation_http_200_with_unchanged_draft_fails_closed(): + fake = LifecycleAPI() + fake.pull["draft"] = True + fake.own_authorization() + fake.graphql_noop_ready = True + with pytest.raises(GuardError, match="contents: write"): + reconcile_pull(fake.api(), REPO, 1) + assert not fake.transitions + assert fake.pull["draft"] is True + + def test_graphql_error_still_approves_then_fails_closed(): fake = LifecycleAPI() fake.runs[0].update(status="completed", conclusion="action_required") From ba12f1f7f2067b6aae5682b71694cf2902986b55 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:19:14 +0200 Subject: [PATCH 4/5] Split Ready vs draft isDraft-mismatch errors. --- src/agent_cli/pr_lifecycle.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/agent_cli/pr_lifecycle.py b/src/agent_cli/pr_lifecycle.py index cf912ca6..a8ba398a 100644 --- a/src/agent_cli/pr_lifecycle.py +++ b/src/agent_cli/pr_lifecycle.py @@ -293,13 +293,16 @@ def _transition(api: Any, node: str, draft: bool) -> Mapping: detail = f": {errors[0].get('message') or 'graphql error'}" if status != 200 or errors or _field(pull, "id") != node: raise GuardError(f"PR lifecycle mutation failed (HTTP {status}){detail}") - if _field(pull, "isDraft") is not draft: - if draft and _field(pull, "isDraft") is False: + got = _field(pull, "isDraft") + if got is not draft: + if draft and got is False: raise LifecycleDraftUnchanged - raise GuardError( - f"PR lifecycle mutation failed (HTTP {status}): isDraft unchanged; " - "GITHUB_TOKEN needs contents: write for markPullRequestReadyForReview" - ) + if not draft and got is True: + raise GuardError( + f"PR lifecycle mutation failed (HTTP {status}): isDraft unchanged; " + "GITHUB_TOKEN needs contents: write for markPullRequestReadyForReview" + ) + raise GuardError(f"PR lifecycle mutation failed (HTTP {status}): isDraft={got!r}") return pull From 5443b8ffdf67f714af5e7ecbc8107b036478b9d5 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:58:07 +0200 Subject: [PATCH 5/5] Cover the non-boolean isDraft mismatch in Guard Ready tests --- tests/test_pr_lifecycle.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_pr_lifecycle.py b/tests/test_pr_lifecycle.py index 5da8b56a..a4cbc10d 100644 --- a/tests/test_pr_lifecycle.py +++ b/tests/test_pr_lifecycle.py @@ -29,6 +29,7 @@ def __init__(self): self.graphql_noop_draft = False self.graphql_ready_without_rest = False self.graphql_noop_ready = False + self.graphql_noop_ready_null = False self.mutate_during_transition = False self.fail_comment_once = False @@ -55,6 +56,10 @@ def request_fn(self, method, url, body=None): return 200, {"data": {operation: {"pullRequest": { "id": "PR_example", "isDraft": True, "headRefOid": self.pull["head"]["sha"], "baseRefOid": BASE}}}}, {} + if self.graphql_noop_ready_null and operation == "markPullRequestReadyForReview": + return 200, {"data": {operation: {"pullRequest": { + "id": "PR_example", "isDraft": None, + "headRefOid": self.pull["head"]["sha"], "baseRefOid": BASE}}}}, {} self.pull["draft"] = operation == "convertPullRequestToDraft" self.transitions.append(self.pull["draft"]) if self.mutate_during_transition: @@ -496,6 +501,18 @@ def test_ready_mutation_http_200_with_unchanged_draft_fails_closed(): assert fake.pull["draft"] is True +def test_ready_mutation_non_boolean_is_draft_fails_closed(): + fake = LifecycleAPI() + fake.pull["draft"] = True + fake.own_authorization() + fake.graphql_noop_ready_null = True + with pytest.raises(GuardError, match=r"isDraft=None"): + reconcile_pull(fake.api(), REPO, 1) + assert not fake.transitions + assert fake.pull["draft"] is True + assert all('"phase": "applied"' not in c["body"] for c in fake.comments) + + def test_graphql_error_still_approves_then_fails_closed(): fake = LifecycleAPI() fake.runs[0].update(status="completed", conclusion="action_required")