From c83e6044f05bcd8c12805d5105e1e9c52133d3fc Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Wed, 3 Jun 2026 00:44:04 +0000 Subject: [PATCH 1/2] fix(gate): combined refire-token + direct-trigger event fix + auto-fire regression test (gate-fix follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (A) Direct-trigger structural fix — qa-review.yml + security-review.yml: - Replace pull_request_review_approved trigger with pull_request_review types: [submitted] (proven to fire via sop-tier-check.yml live status contexts). - Add job-level if: guard requiring github.event.review.state == 'APPROVED' || 'approved' so only APPROVE reviews run the evaluator; COMMENT / REQUEST_CHANGES are skipped at job level. - Update explicit POST step event guard to pull_request_review. (B) Refire-path token fix — sop-checklist.yml + review-refire-status.sh: - Change explicit POST /statuses to use STATUS_POST_TOKEN (narrow-scoped write:repository token, CTO-granted). - Leave evaluator (review-check.sh + GET /pulls) on SOP_TIER_CHECK_TOKEN || GITHUB_TOKEN (read-only). - review-refire-status.sh now creates a separate post_authfile with STATUS_POST_TOKEN; falls back to GITEA_TOKEN for backward compatibility. (#765 regression test) — test_gate_review_auto_fire.py: - Structural tests asserting qa-review and security-review workflows trigger on pull_request_review submitted, guard on APPROVED state, POST with STATUS_POST_TOKEN, and emit exact BP-required context name. - Structural tests asserting sop-checklist refire steps pass STATUS_POST_TOKEN env var while keeping evaluator on read token. Trust boundary unchanged: BASE ref checkout, no PR-head code execution. Refs: internal#760, internal#765 --- .gitea/scripts/review-refire-status.sh | 10 +- .../tests/test_gate_review_auto_fire.py | 148 ++++++++++++++++++ .gitea/workflows/qa-review.yml | 38 ++--- .gitea/workflows/security-review.yml | 38 ++--- .gitea/workflows/sop-checklist.yml | 12 +- 5 files changed, 201 insertions(+), 45 deletions(-) create mode 100644 .gitea/scripts/tests/test_gate_review_auto_fire.py diff --git a/.gitea/scripts/review-refire-status.sh b/.gitea/scripts/review-refire-status.sh index 0ec2f605e..899faa52f 100755 --- a/.gitea/scripts/review-refire-status.sh +++ b/.gitea/scripts/review-refire-status.sh @@ -17,16 +17,20 @@ CONTEXT="${TEAM}-review / approved (pull_request)" TARGET_URL="https://${GITEA_HOST}/${OWNER}/${NAME}/pulls/${PR_NUMBER}" authfile=$(mktemp) +post_authfile=$(mktemp) prfile=$(mktemp) postfile=$(mktemp) # shellcheck disable=SC2329 # invoked by EXIT trap cleanup() { - rm -f "$authfile" "$prfile" "$postfile" + rm -f "$authfile" "$post_authfile" "$prfile" "$postfile" } trap cleanup EXIT -chmod 600 "$authfile" +chmod 600 "$authfile" "$post_authfile" printf 'header = "Authorization: token %s"\n' "$GITEA_TOKEN" > "$authfile" +# STATUS_POST_TOKEN is narrow-scoped write:repository for explicit status POST. +# Falls back to GITEA_TOKEN for backward compatibility (e.g. local test). +printf 'header = "Authorization: token %s"\n' "${STATUS_POST_TOKEN:-$GITEA_TOKEN}" > "$post_authfile" code=$(curl -sS -o "$prfile" -w '%{http_code}' -K "$authfile" \ "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}") @@ -68,7 +72,7 @@ body=$(jq -nc \ '{state:$state, context:$context, description:$description, target_url:$target_url}') code=$(curl -sS -o "$postfile" -w '%{http_code}' -X POST \ - -K "$authfile" -H "Content-Type: application/json" \ + -K "$post_authfile" -H "Content-Type: application/json" \ -d "$body" \ "${API}/repos/${OWNER}/${NAME}/statuses/${head_sha}") if [ "$code" != "200" ] && [ "$code" != "201" ]; then diff --git a/.gitea/scripts/tests/test_gate_review_auto_fire.py b/.gitea/scripts/tests/test_gate_review_auto_fire.py new file mode 100644 index 000000000..9f9b2823d --- /dev/null +++ b/.gitea/scripts/tests/test_gate_review_auto_fire.py @@ -0,0 +1,148 @@ +"""Regression test #765 — gate auto-fire on real qa/security APPROVED review. + +Validates the structural configuration of qa-review.yml and security-review.yml +so that a real team-member APPROVED review fires the workflow and POSTs the +exact branch-protection-required context name. This is the test #2020's +stale-context failure would have caught. +""" + +from pathlib import Path + +import yaml + +ROOT = Path(__file__).resolve().parents[2] + + +def load_workflow(name: str) -> dict: + with (ROOT / "workflows" / name).open() as f: + return yaml.safe_load(f) + + +def _job_guard_string(workflow: dict) -> str: + """Return the raw job-level `if:` string for the single job.""" + jobs = workflow["jobs"] + # Both qa-review and security-review have exactly one job named "approved". + job = jobs["approved"] + return str(job.get("if", "")) + + +def _post_step(workflow: dict) -> dict: + """Return the explicit POST /statuses step from the job steps list.""" + jobs = workflow["jobs"] + steps = jobs["approved"]["steps"] + for step in steps: + name = step.get("name", "") + if "Post required status context" in name: + return step + raise AssertionError("No explicit POST status step found") + + +class TestQaReviewDirectTrigger: + def test_trigger_is_pull_request_review_submitted(self): + wf = load_workflow("qa-review.yml") + # PyYAML parses bare 'on' as boolean True. + on = wf[True] + assert "pull_request_review" in on, ( + "qa-review must trigger on pull_request_review" + ) + types = on["pull_request_review"].get("types", []) + assert "submitted" in types, ( + "pull_request_review must include 'submitted' type" + ) + + def test_job_guard_requires_approved_state(self): + wf = load_workflow("qa-review.yml") + guard = _job_guard_string(wf) + assert "github.event.review.state == 'APPROVED'" in guard or "github.event.review.state == 'approved'" in guard, ( + "job guard must check review.state for APPROVED" + ) + + def test_post_step_uses_status_post_token(self): + wf = load_workflow("qa-review.yml") + post = _post_step(wf) + env = post.get("env", {}) + assert env.get("GITEA_TOKEN") == "${{ secrets.STATUS_POST_TOKEN }}", ( + "POST step must use STATUS_POST_TOKEN for write-scoped status POST" + ) + + def test_post_step_context_name_exact(self): + """The context POSTed must byte-match the branch-protection requirement.""" + wf = load_workflow("qa-review.yml") + post = _post_step(wf) + run = post.get("run", "") + assert '"qa-review / approved (pull_request_target)"' in run, ( + "POST step must emit exact BP-required context name" + ) + + +class TestSecurityReviewDirectTrigger: + def test_trigger_is_pull_request_review_submitted(self): + wf = load_workflow("security-review.yml") + # PyYAML parses bare 'on' as boolean True. + on = wf[True] + assert "pull_request_review" in on, ( + "security-review must trigger on pull_request_review" + ) + types = on["pull_request_review"].get("types", []) + assert "submitted" in types, ( + "pull_request_review must include 'submitted' type" + ) + + def test_job_guard_requires_approved_state(self): + wf = load_workflow("security-review.yml") + guard = _job_guard_string(wf) + assert "github.event.review.state == 'APPROVED'" in guard or "github.event.review.state == 'approved'" in guard, ( + "job guard must check review.state for APPROVED" + ) + + def test_post_step_uses_status_post_token(self): + wf = load_workflow("security-review.yml") + post = _post_step(wf) + env = post.get("env", {}) + assert env.get("GITEA_TOKEN") == "${{ secrets.STATUS_POST_TOKEN }}", ( + "POST step must use STATUS_POST_TOKEN for write-scoped status POST" + ) + + def test_post_step_context_name_exact(self): + """The context POSTed must byte-match the branch-protection requirement.""" + wf = load_workflow("security-review.yml") + post = _post_step(wf) + run = post.get("run", "") + assert '"security-review / approved (pull_request_target)"' in run, ( + "POST step must emit exact BP-required context name" + ) + + +class TestRefireTokenSeparation: + """The /qa-recheck + /security-recheck backstop must also use STATUS_POST_TOKEN.""" + + def _refire_step(self, workflow_name: str, step_name_keyword: str) -> dict: + wf = load_workflow(workflow_name) + jobs = wf["jobs"] + steps = jobs["review-refire"]["steps"] + for step in steps: + name = step.get("name", "") + if step_name_keyword in name: + return step + raise AssertionError(f"No refire step matching {step_name_keyword!r}") + + def test_qa_refire_uses_status_post_token(self): + step = self._refire_step("sop-checklist.yml", "Refire qa-review") + env = step.get("env", {}) + assert env.get("STATUS_POST_TOKEN") == "${{ secrets.STATUS_POST_TOKEN }}", ( + "qa refire must receive STATUS_POST_TOKEN env var" + ) + # Evaluator stays on read token + assert "SOP_TIER_CHECK_TOKEN" in env.get("GITEA_TOKEN", "") or "GITHUB_TOKEN" in env.get("GITEA_TOKEN", ""), ( + "qa refire evaluator must stay on read-scoped token" + ) + + def test_security_refire_uses_status_post_token(self): + step = self._refire_step("sop-checklist.yml", "Refire security-review") + env = step.get("env", {}) + assert env.get("STATUS_POST_TOKEN") == "${{ secrets.STATUS_POST_TOKEN }}", ( + "security refire must receive STATUS_POST_TOKEN env var" + ) + assert "SOP_TIER_CHECK_TOKEN" in env.get("GITEA_TOKEN", "") or "GITHUB_TOKEN" in env.get("GITEA_TOKEN", ""), ( + "security refire evaluator must stay on read-scoped token" + ) diff --git a/.gitea/workflows/qa-review.yml b/.gitea/workflows/qa-review.yml index 33aee528e..75c68351d 100644 --- a/.gitea/workflows/qa-review.yml +++ b/.gitea/workflows/qa-review.yml @@ -9,19 +9,19 @@ # Triggers on: # - `pull_request_target`: opened, synchronize, reopened # → initial status posts when PR opens / re-pushes -# - `pull_request_review_approved` +# - `pull_request_review` types: [submitted] # → re-evaluate when a team member submits an APPROVE review so # the gate flips immediately (no wait for the next push or -# slash-command). Gitea Actions uses the specific event name -# `pull_request_review_approved` (not the GitHub-style -# `pull_request_review` catch-all). Verified via Gitea source -# code audit (go-gitea/gitea main, modules/webhook/type.go + -# services/actions/notifier.go). The event payload carries -# `review.type="pull_request_review_approved"`; there is no -# `review.state` field. Branch-protection requires the -# `(pull_request_target)` context variant, so the -# pull_request_review_approved path EXPLICITLY POSTS the -# required context via the API. Trust boundary preserved +# slash-command). Verified live: sop-tier-check.yml uses this +# same event and provably fires (produces +# `sop-tier-check / tier-check (pull_request_review)` contexts). +# The job-level `if:` guard checks +# `github.event.review.state == 'APPROVED' || 'approved'` so +# only APPROVE reviews run the evaluator; COMMENT and +# REQUEST_CHANGES are skipped at the job level. +# Branch-protection requires the `(pull_request_target)` +# context variant, so the review-event path EXPLICITLY POSTS +# the required context via the API. Trust boundary preserved # (BASE ref, no PR-head). # - comment refires are handled by `sop-checklist.yml` review-refire job # → `/qa-recheck` slash-command re-evaluates this gate. @@ -97,7 +97,8 @@ name: qa-review on: pull_request_target: types: [opened, synchronize, reopened] - pull_request_review_approved: + pull_request_review: + types: [submitted] permissions: contents: read @@ -114,7 +115,8 @@ jobs: # Comment-triggered refires live in sop-checklist.yml review-refire job. if: | github.event_name == 'pull_request_target' || - github.event_name == 'pull_request_review_approved' + (github.event_name == 'pull_request_review' && + (github.event.review.state == 'APPROVED' || github.event.review.state == 'approved')) runs-on: ubuntu-latest steps: - name: Privilege check (A1.1 — INFORMATIONAL log only, NOT a gate) @@ -174,8 +176,8 @@ jobs: REVIEW_CHECK_STRICT: '0' run: bash .gitea/scripts/review-check.sh - - name: Post required status context on pull_request_review_approved - # Gitea Actions auto-publishes (pull_request_review_approved) context + - name: Post required status context on pull_request_review + # Gitea Actions auto-publishes (pull_request_review) context # for this event, but branch-protection requires (pull_request_target). # We explicitly POST the BP-required context so the gate flips. # Trust boundary: same BASE-ref script result, no PR-head code. @@ -185,7 +187,7 @@ jobs: # for the explicit status POST. Evaluator step stays on # SOP_TIER_CHECK_TOKEN (read-only) per deliberate security # separation: eval computes, POST writes, never the same cred. - if: github.event_name == 'pull_request_review_approved' && always() + if: github.event_name == 'pull_request_review' && always() env: GITEA_TOKEN: ${{ secrets.STATUS_POST_TOKEN }} GITEA_HOST: git.moleculesai.app @@ -211,10 +213,10 @@ jobs: if [ "$EVAL_OUTCOME" = "success" ]; then status_state="success" - description="Approved via pull_request_review_approved trigger" + description="Approved via pull_request_review trigger" else status_state="failure" - description="Review check failed via pull_request_review_approved trigger" + description="Review check failed via pull_request_review trigger" fi body=$(jq -nc \ diff --git a/.gitea/workflows/security-review.yml b/.gitea/workflows/security-review.yml index 0ff5b79b6..35044f308 100644 --- a/.gitea/workflows/security-review.yml +++ b/.gitea/workflows/security-review.yml @@ -7,24 +7,25 @@ # See `qa-review.yml` header for the full A1-α / A1.1 / A4 / A5 design # rationale; everything below is identical in shape. # -# A1-α addendum (internal#760): `pull_request_review_approved` trigger -# added so the security gate flips immediately when a team member submits -# an APPROVE review. Gitea Actions uses the specific event name -# `pull_request_review_approved` (not the GitHub-style `pull_request_review` -# catch-all). Verified via Gitea source code audit (go-gitea/gitea main, -# modules/webhook/type.go + services/actions/notifier.go). The event -# payload carries `review.type="pull_request_review_approved"`; there is -# no `review.state` field. Branch-protection requires the -# `(pull_request_target)` context variant, so the -# pull_request_review_approved path EXPLICITLY POSTS the required context -# via the API. Trust boundary preserved (BASE ref, no PR-head). +# A1-α addendum (internal#760): review-event trigger added so the security +# gate flips immediately when a team member submits an APPROVE review. +# Uses `pull_request_review` types: [submitted] — verified live via +# sop-tier-check.yml which provably fires this event (produces +# `sop-tier-check / tier-check (pull_request_review)` contexts). +# The job-level `if:` guard checks +# `github.event.review.state == 'APPROVED' || 'approved'` so only APPROVE +# reviews run the evaluator; COMMENT and REQUEST_CHANGES are skipped at +# the job level. Branch-protection requires the `(pull_request_target)` +# context variant, so the review-event path EXPLICITLY POSTS the required +# context via the API. Trust boundary preserved (BASE ref, no PR-head). name: security-review on: pull_request_target: types: [opened, synchronize, reopened] - pull_request_review_approved: + pull_request_review: + types: [submitted] permissions: contents: read @@ -41,7 +42,8 @@ jobs: # Comment-triggered refires live in sop-checklist.yml review-refire job. if: | github.event_name == 'pull_request_target' || - github.event_name == 'pull_request_review_approved' + (github.event_name == 'pull_request_review' && + (github.event.review.state == 'APPROVED' || github.event.review.state == 'approved')) runs-on: ubuntu-latest steps: - name: Privilege check (A1.1 — INFORMATIONAL log only, NOT a gate) @@ -87,8 +89,8 @@ jobs: REVIEW_CHECK_STRICT: '0' run: bash .gitea/scripts/review-check.sh - - name: Post required status context on pull_request_review_approved - # Gitea Actions auto-publishes (pull_request_review_approved) context + - name: Post required status context on pull_request_review + # Gitea Actions auto-publishes (pull_request_review) context # for this event, but branch-protection requires (pull_request_target). # We explicitly POST the BP-required context so the gate flips. # Trust boundary: same BASE-ref script result, no PR-head code. @@ -98,7 +100,7 @@ jobs: # for the explicit status POST. Evaluator step stays on # SOP_TIER_CHECK_TOKEN (read-only) per deliberate security # separation: eval computes, POST writes, never the same cred. - if: github.event_name == 'pull_request_review_approved' && always() + if: github.event_name == 'pull_request_review' && always() env: GITEA_TOKEN: ${{ secrets.STATUS_POST_TOKEN }} GITEA_HOST: git.moleculesai.app @@ -124,10 +126,10 @@ jobs: if [ "$EVAL_OUTCOME" = "success" ]; then status_state="success" - description="Approved via pull_request_review_approved trigger" + description="Approved via pull_request_review trigger" else status_state="failure" - description="Review check failed via pull_request_review_approved trigger" + description="Review check failed via pull_request_review trigger" fi body=$(jq -nc \ diff --git a/.gitea/workflows/sop-checklist.yml b/.gitea/workflows/sop-checklist.yml index b56917c71..2310cc0ec 100644 --- a/.gitea/workflows/sop-checklist.yml +++ b/.gitea/workflows/sop-checklist.yml @@ -179,10 +179,10 @@ jobs: - name: Refire qa-review status if: steps.classify.outputs.run_qa == 'true' env: - # RFC_324_TEAM_READ_TOKEN is read-only (team membership read scope only). - # review-refire-status.sh POSTs to /statuses — requires write scope. - # SOP_TIER_CHECK_TOKEN carries write:repository + write:issue + read:organization. + # Evaluator (review-check.sh + GET /pulls) stays on read-scoped token. GITEA_TOKEN: ${{ secrets.SOP_TIER_CHECK_TOKEN || secrets.GITHUB_TOKEN }} + # Explicit POST /statuses uses narrow-scoped write:repository token. + STATUS_POST_TOKEN: ${{ secrets.STATUS_POST_TOKEN }} GITEA_HOST: git.moleculesai.app REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.issue.number }} @@ -198,10 +198,10 @@ jobs: - name: Refire security-review status if: steps.classify.outputs.run_security == 'true' env: - # RFC_324_TEAM_READ_TOKEN is read-only (team membership read scope only). - # review-refire-status.sh POSTs to /statuses — requires write scope. - # SOP_TIER_CHECK_TOKEN carries write:repository + write:issue + read:organization. + # Evaluator (review-check.sh + GET /pulls) stays on read-scoped token. GITEA_TOKEN: ${{ secrets.SOP_TIER_CHECK_TOKEN || secrets.GITHUB_TOKEN }} + # Explicit POST /statuses uses narrow-scoped write:repository token. + STATUS_POST_TOKEN: ${{ secrets.STATUS_POST_TOKEN }} GITEA_HOST: git.moleculesai.app REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.issue.number }} -- 2.52.0 From 1b8b7a7047b6552bad14599c599a10ea175b766b Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Wed, 3 Jun 2026 00:52:07 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(gate):=20CR2=20RC=208337=20=E2=80=94=20?= =?UTF-8?q?refire=20context=20name=20+=20test=20strengthening=20(#2157)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (a) review-refire-status.sh: CONTEXT now posts exact BP-required "(pull_request_target)" instead of bare "(pull_request)". (b) Tests: job_guard_requires_approved_state now asserts BOTH 'APPROVED' and 'approved' case variants are present (not OR). (c) Tests: new test_refire_script_context_is_pull_request_target asserts refire script emits exact (pull_request_target) context. Test count: 10 → 11. Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/review-refire-status.sh | 4 ++- .../tests/test_gate_review_auto_fire.py | 28 ++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/.gitea/scripts/review-refire-status.sh b/.gitea/scripts/review-refire-status.sh index 899faa52f..a68f36b96 100755 --- a/.gitea/scripts/review-refire-status.sh +++ b/.gitea/scripts/review-refire-status.sh @@ -13,7 +13,9 @@ set -euo pipefail OWNER="${REPO%%/*}" NAME="${REPO##*/}" API="https://${GITEA_HOST}/api/v1" -CONTEXT="${TEAM}-review / approved (pull_request)" +# Branch-protection requires the (pull_request_target) context variant. +# The refire path must post the EXACT BP-required name so the gate flips. +CONTEXT="${TEAM}-review / approved (pull_request_target)" TARGET_URL="https://${GITEA_HOST}/${OWNER}/${NAME}/pulls/${PR_NUMBER}" authfile=$(mktemp) diff --git a/.gitea/scripts/tests/test_gate_review_auto_fire.py b/.gitea/scripts/tests/test_gate_review_auto_fire.py index 9f9b2823d..72a650ccc 100644 --- a/.gitea/scripts/tests/test_gate_review_auto_fire.py +++ b/.gitea/scripts/tests/test_gate_review_auto_fire.py @@ -53,8 +53,11 @@ class TestQaReviewDirectTrigger: def test_job_guard_requires_approved_state(self): wf = load_workflow("qa-review.yml") guard = _job_guard_string(wf) - assert "github.event.review.state == 'APPROVED'" in guard or "github.event.review.state == 'approved'" in guard, ( - "job guard must check review.state for APPROVED" + assert "github.event.review.state == 'APPROVED'" in guard, ( + "job guard must check review.state for 'APPROVED'" + ) + assert "github.event.review.state == 'approved'" in guard, ( + "job guard must check review.state for 'approved' (case fallback per #2135)" ) def test_post_step_uses_status_post_token(self): @@ -91,8 +94,11 @@ class TestSecurityReviewDirectTrigger: def test_job_guard_requires_approved_state(self): wf = load_workflow("security-review.yml") guard = _job_guard_string(wf) - assert "github.event.review.state == 'APPROVED'" in guard or "github.event.review.state == 'approved'" in guard, ( - "job guard must check review.state for APPROVED" + assert "github.event.review.state == 'APPROVED'" in guard, ( + "job guard must check review.state for 'APPROVED'" + ) + assert "github.event.review.state == 'approved'" in guard, ( + "job guard must check review.state for 'approved' (case fallback per #2135)" ) def test_post_step_uses_status_post_token(self): @@ -113,6 +119,20 @@ class TestSecurityReviewDirectTrigger: ) +class TestRefireScriptContextName: + """review-refire-status.sh must emit the BP-required (pull_request_target) context.""" + + def test_refire_script_context_is_pull_request_target(self): + script = ROOT / "scripts" / "review-refire-status.sh" + content = script.read_text() + assert 'CONTEXT="${TEAM}-review / approved (pull_request_target)"' in content, ( + "refire script CONTEXT must be the exact BP-required (pull_request_target) variant" + ) + assert 'approved (pull_request)"' not in content, ( + "refire script must NOT post bare (pull_request) context" + ) + + class TestRefireTokenSeparation: """The /qa-recheck + /security-recheck backstop must also use STATUS_POST_TOKEN.""" -- 2.52.0