diff --git a/.gitea/scripts/review-refire-status.sh b/.gitea/scripts/review-refire-status.sh index 0ec2f605e..a68f36b96 100755 --- a/.gitea/scripts/review-refire-status.sh +++ b/.gitea/scripts/review-refire-status.sh @@ -13,20 +13,26 @@ 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) +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 +74,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..72a650ccc --- /dev/null +++ b/.gitea/scripts/tests/test_gate_review_auto_fire.py @@ -0,0 +1,168 @@ +"""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, ( + "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): + 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, ( + "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): + 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 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.""" + + 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 }}