Merge pull request 'feat(ci): add qa-review + security-review checks (RFC#324 Step 1 of 5)' (#535) from infra/rfc-324-workflow-add into main

.gitea/scripts/review-check.sh

@@ -0,0 +1,188 @@
+#!/usr/bin/env bash
+# review-check — evaluate whether a PR satisfies a single team-review gate.
+#
+# RFC#324 Step 1 of 5 — qa-review + security-review check workflows.
+#
+# This is the shared evaluator invoked by:
+#   .gitea/workflows/qa-review.yml      (TEAM=qa,      TEAM_ID=20)
+#   .gitea/workflows/security-review.yml (TEAM=security, TEAM_ID=21)
+#
+# Pass condition (per RFC#324 v1.1 addendum):
+#   ≥ 1 review on the PR where:
+#     • state == APPROVED
+#     • review.dismissed == false
+#     • review.user.login != PR.user.login (non-author)
+#     • review.user.login ∈ team-members
+#
+# Strict mode (default OFF for v1; see RFC trade-off note):
+#   If REVIEW_CHECK_STRICT=1, additionally require review.commit_id == PR.head.sha.
+#   With dismiss_stale_reviews: true at the protection layer, stale reviews
+#   are already dismissed, so the additional commit_id check is belt-and-
+#   suspenders. Keeping it off in v1 simplifies semantics; flip in a follow-up
+#   PR if reviewer telemetry shows residual stale-APPROVE merges.
+#
+# Privilege gate (RFC#324 v1.3 §A1.1 — INFORMATIONAL ONLY):
+#   The /qa-recheck and /security-recheck slash-commands can be triggered
+#   by anyone who can comment on the PR. The workflow's privilege step
+#   logs collaborator-status but does NOT gate execution of this script.
+#   Why this is safe: this evaluator is read-only and idempotent —
+#   reading `pulls/{N}/reviews` and `teams/{id}/members/{u}` can't be
+#   influenced by who triggered the run. If a real team-member APPROVE
+#   exists the gate flips green; otherwise it stays red. A
+#   non-collaborator commenting /qa-recheck cannot manufacture a green
+#   gate. Original (v1.2) design with `if:`-gating of this step was
+#   fail-open (skipped-via-`if:` job still publishes the status as
+#   `success`) — corrected in v1.3 per hongming-pc review 1421.
+#
+# Trust boundary (RFC A4):
+#   This script is loaded from the BASE branch (sourced via .gitea/scripts/
+#   on the workflow's checkout-of-base). It does NOT execute any PR-HEAD
+#   code. It only reads PR review state via the Gitea API.
+#
+# Token scope (RFC A1-α):
+#   The job's own conclusion (exit 0 / exit 1) is what publishes the
+#   `qa-review / approved` / `security-review / approved` status context.
+#   NO `POST /statuses` call here → NO `write:repository` scope on the
+#   token. `read:organization` (for team-membership probe) and
+#   `read:repository` (for PR + reviews) are enough.
+#
+# Required env:
+#   GITEA_TOKEN — least-priv read:repository + read:organization. See note
+#                 below about the team-membership API requiring the token
+#                 owner to be in the queried team (Gitea 1.22.6 quirk).
+#   GITEA_HOST  — e.g. git.moleculesai.app
+#   REPO        — owner/name (from github.repository)
+#   PR_NUMBER   — int (from github.event.pull_request.number or
+#                 github.event.issue.number for issue_comment events)
+#   TEAM        — short team name (qa | security) for log lines
+#   TEAM_ID     — Gitea team id (20=qa, 21=security at time of writing)
+#
+# Optional:
+#   REVIEW_CHECK_DEBUG=1 — per-API-call diagnostic lines
+#   REVIEW_CHECK_STRICT=1 — also require review.commit_id == pr.head.sha
+
+set -euo pipefail
+
+# jq is required for JSON parsing. It is pre-baked into the runner-base
+# image (per RFC#268 workflow-smoke), so the only reason we'd not find it
+# is a broken runner. The previous fallback dance (apt-get + curl to
+# /usr/local/bin/jq) cannot succeed on a uid-1001 rootless runner
+# (#391/#402 + feedback_ci_runner_install_needs_writable_path), so it's
+# dropped. Fail loud with a clear diagnostic rather than attempt an
+# install that physically cannot work.
+if ! command -v jq >/dev/null 2>&1; then
+  echo "::error::jq missing from runner-base image — bake it into the runner image (see RFC#268 workflow-smoke / feedback_ci_runner_install_needs_writable_path). This evaluator cannot run without jq."
+  exit 1
+fi
+
+: "${GITEA_TOKEN:?GITEA_TOKEN required}"
+: "${GITEA_HOST:?GITEA_HOST required}"
+: "${REPO:?REPO required (owner/name)}"
+: "${PR_NUMBER:?PR_NUMBER required}"
+: "${TEAM:?TEAM required (qa|security)}"
+: "${TEAM_ID:?TEAM_ID required (integer)}"
+
+OWNER="${REPO%%/*}"
+NAME="${REPO##*/}"
+API="https://${GITEA_HOST}/api/v1"
+AUTH="Authorization: token ${GITEA_TOKEN}"
+
+debug() {
+  if [ "${REVIEW_CHECK_DEBUG:-}" = "1" ]; then
+    echo "  [debug] $*" >&2
+  fi
+}
+
+echo "::notice::${TEAM}-review evaluating repo=${OWNER}/${NAME} pr=${PR_NUMBER} team_id=${TEAM_ID}"
+
+# --- Fetch the PR (for author + head.sha) ---
+PR_JSON=$(mktemp)
+trap 'rm -f "$PR_JSON" "$REVIEWS_JSON" "$TEAM_PROBE_TMP" 2>/dev/null || true' EXIT
+HTTP_CODE=$(curl -sS -o "$PR_JSON" -w '%{http_code}' \
+  -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}")
+if [ "$HTTP_CODE" != "200" ]; then
+  echo "::error::GET /pulls/${PR_NUMBER} returned HTTP ${HTTP_CODE} (token scope?)"
+  cat "$PR_JSON" >&2
+  exit 1
+fi
+PR_AUTHOR=$(jq -r '.user.login // ""' "$PR_JSON")
+PR_HEAD_SHA=$(jq -r '.head.sha // ""' "$PR_JSON")
+PR_STATE=$(jq -r '.state // ""' "$PR_JSON")
+debug "pr_author=${PR_AUTHOR} pr_head=${PR_HEAD_SHA:0:7} pr_state=${PR_STATE}"
+
+if [ "$PR_STATE" != "open" ]; then
+  echo "::notice::PR ${PR_NUMBER} is ${PR_STATE} — exiting 0 (closed PRs do not gate)"
+  exit 0
+fi
+if [ -z "$PR_AUTHOR" ] || [ -z "$PR_HEAD_SHA" ]; then
+  echo "::error::PR ${PR_NUMBER} missing user.login or head.sha — webhook payload malformed"
+  exit 1
+fi
+
+# --- Fetch all reviews on the PR ---
+REVIEWS_JSON=$(mktemp)
+HTTP_CODE=$(curl -sS -o "$REVIEWS_JSON" -w '%{http_code}' \
+  -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}/reviews")
+if [ "$HTTP_CODE" != "200" ]; then
+  echo "::error::GET /pulls/${PR_NUMBER}/reviews returned HTTP ${HTTP_CODE}"
+  cat "$REVIEWS_JSON" >&2
+  exit 1
+fi
+
+# Filter: state=APPROVED, not-dismissed, non-author. Optionally strict-mode
+# adds commit_id==head.sha (off by default; see header).
+JQ_FILTER='.[]
+  | select(.state == "APPROVED")
+  | select(.dismissed != true)
+  | select(.user.login != $author)'
+if [ "${REVIEW_CHECK_STRICT:-}" = "1" ]; then
+  JQ_FILTER="${JQ_FILTER}
+  | select(.commit_id == \$head)"
+fi
+JQ_FILTER="${JQ_FILTER}
+  | .user.login"
+
+CANDIDATES=$(jq -r --arg author "$PR_AUTHOR" --arg head "$PR_HEAD_SHA" "$JQ_FILTER" "$REVIEWS_JSON" | sort -u)
+debug "candidate non-author approvers: $(echo "$CANDIDATES" | tr '\n' ' ')"
+
+if [ -z "$CANDIDATES" ]; then
+  echo "::error::${TEAM}-review awaiting non-author APPROVE from ${TEAM} team (no candidates yet)"
+  exit 1
+fi
+
+# --- Probe team membership per candidate ---
+# Endpoint: GET /api/v1/teams/{id}/members/{username}
+#   200/204 → is member
+#   403     → token owner is not in this team (Gitea 1.22.6 'Must be a team
+#             member' constraint — see follow-up issue for token-provisioning)
+#   404     → not a member
+TEAM_PROBE_TMP=$(mktemp)
+for U in $CANDIDATES; do
+  CODE=$(curl -sS -o "$TEAM_PROBE_TMP" -w '%{http_code}' \
+    -H "$AUTH" "${API}/teams/${TEAM_ID}/members/${U}")
+  debug "probe ${U} in team ${TEAM} (id=${TEAM_ID}) → HTTP ${CODE}"
+  case "$CODE" in
+    200|204)
+      echo "::notice::${TEAM}-review APPROVED by ${U} (team=${TEAM})"
+      exit 0
+      ;;
+    403)
+      # Token owner is not in the team being probed; the API refuses to
+      # confirm membership. This is the RFC#324 follow-up token-scope gap.
+      # Fail closed — never grant approval on a 403; surface clearly.
+      echo "::error::team-probe for ${U} in ${TEAM} returned 403 (token owner not in ${TEAM} team — RFC#324 token-scope follow-up). Cannot confirm membership; failing closed."
+      cat "$TEAM_PROBE_TMP" >&2
+      exit 1
+      ;;
+    404)
+      debug "${U} not a member of ${TEAM}"
+      ;;
+    *)
+      echo "::warning::team-probe for ${U} in ${TEAM} returned unexpected HTTP ${CODE}"
+      cat "$TEAM_PROBE_TMP" >&2
+      ;;
+  esac
+done
+
+echo "::error::${TEAM}-review awaiting non-author APPROVE from ${TEAM} team (candidates: $(echo "$CANDIDATES" | tr '\n' ',' | sed 's/,$//') — none are in team)"
+exit 1

.gitea/workflows/qa-review.yml

@@ -0,0 +1,159 @@
+# qa-review — non-author APPROVE from the `qa` Gitea team required to merge.
+#
+# RFC#324 Step 1 of 5 (workflow-add). Pairs with `security-review.yml` and the
+# branch-protection flip in Step 2.
+#
+# === DESIGN (RFC#324 v1.1 addendum) ===
+#
+# A1-α (refire mechanism):
+#   Triggers on:
+#     - `pull_request_target`: opened, synchronize, reopened
+#         → initial status posts when PR opens / re-pushes
+#     - `issue_comment`: /qa-recheck slash-command on the PR
+#         → manual re-fire after a QA reviewer clicks APPROVE
+#           (Gitea 1.22.6 doesn't re-fire on pull_request_review, per
+#           go-gitea/gitea#33700 + feedback_pull_request_review_no_refire)
+#   Workflow name = `qa-review` ; job name = `approved`.
+#   The job's own pass/fail conclusion publishes the status context
+#   `qa-review / approved (<event>)` — NO `POST /statuses` call → NO
+#   write:repository token scope needed. Sidesteps internal#321 defect #2.
+#
+# A1.1 (privilege check on slash-comment — INFORMATIONAL ONLY, NOT a gate):
+#   The `issue_comment` event fires for ANY commenter, including
+#   non-collaborators. The original (v1.2) design gated the eval step
+#   behind a collaborator probe → if a non-collaborator commented
+#   /qa-recheck, the eval was `if:`-skipped → the job exited 0 anyway →
+#   the status context published `success` with ZERO real APPROVE.
+#   That was a fail-open: any visitor could green the gate.
+#
+#   RFC#324 v1.3 §A1.1 correction (option b per hongming-pc 1421):
+#   drop privilege-gating of the evaluation entirely. The eval is
+#   read-only and idempotent — it reads `pulls/{N}/reviews` and
+#   `teams/{id}/members/{u}` (both API-side state that a commenter can't
+#   change). Re-running it on a non-collaborator's comment is harmless
+#   AND correct: if a real team-member APPROVE exists, the eval flips
+#   green; if not, it stays red.
+#
+#   We KEEP the privilege step as a `::notice::` log line only — useful
+#   for griefer-spotting (one operator spamming /recheck) without
+#   touching the gate. If rate-limiting is needed later, add it as a
+#   separate concern (time-window throttle, not a privilege gate).
+#
+#   We MUST NOT use `github.event.comment.author_association` (the
+#   field doesn't exist on Gitea 1.22.6 webhook payload — this was
+#   sop-tier-refire's defect #1).
+#
+# A4 (no PR-head checkout under pull_request_target):
+#   We check out the BASE ref explicitly so the review-check.sh script is
+#   loaded from trusted source. We NEVER use `ref: ${{ github.event.pull_request.head.sha }}`.
+#   No PR-head code is executed in the runner. Trust boundary preserved.
+#
+# A5 (real Gitea team):
+#   `qa` team (id=20) verified by orchestrator preflight 2026-05-11; queried
+#   at run time via /api/v1/teams/20/members/{login}.
+#
+# === TOKEN ===
+#
+# The workflow reads PR state, PR reviews, and team membership.
+# Gitea 1.22.6's /api/v1/teams/{id}/members/{u} returns 403 ('Must be a
+# team member') for tokens whose owner is not in that team. The default
+# `secrets.GITHUB_TOKEN` is owned by a workflow-scoped identity that is
+# also not in qa/security teams → also 403.
+#
+# Resolution: a dedicated `RFC_324_TEAM_READ_TOKEN` secret, owned by an
+# identity that IS in both `qa` and `security` teams (Owners-tier
+# claude-ceo-assistant, or a new service-bot added to both teams).
+# Provisioning of this secret is tracked as a follow-up issue (filed by
+# core-devops at PR open).
+#
+# Until that secret is provisioned, the job will exit 1 with a clear
+# 403-on-team-probe error and the `qa-review / approved` status will
+# stay `failure`. This is the correct fail-closed behavior — the gate
+# blocks merge until both (a) a QA team member APPROVEs and (b) the
+# workflow has a token that can confirm their team membership.
+#
+# === SLASH-COMMAND CONTRACT ===
+#
+#   /qa-recheck   — re-evaluate the gate (e.g. after an APPROVE lands)
+#
+# Open to any PR commenter. The eval is read-only and idempotent, so
+# unprivileged refires are harmless (RFC#324 v1.3 §A1.1). Collaborator
+# status is logged for griefer-spotting but does NOT gate execution.
+
+name: qa-review
+
+on:
+  pull_request_target:
+    types: [opened, synchronize, reopened]
+  issue_comment:
+    types: [created]
+
+permissions:
+  contents: read
+  pull-requests: read
+
+jobs:
+  approved:
+    # Gate the job:
+    #   - On pull_request_target events: always run.
+    #   - On issue_comment events: only when it's a PR comment and the body
+    #     contains the slash-command. NO privilege gate at the step level
+    #     (RFC#324 v1.3 §A1.1): a non-collaborator's /qa-recheck is fine
+    #     because the eval is read-only and idempotent — re-running it
+    #     just re-confirms whether a real team-member APPROVE exists.
+    if: |
+      github.event_name == 'pull_request_target' ||
+      (github.event_name == 'issue_comment' &&
+       github.event.issue.pull_request != null &&
+       startsWith(github.event.comment.body, '/qa-recheck'))
+    runs-on: ubuntu-latest
+    steps:
+      - name: Privilege check (A1.1 — INFORMATIONAL log only, NOT a gate)
+        # RFC#324 v1.3 §A1.1: this step does NOT gate subsequent steps.
+        # It exists solely as a log line for griefer-spotting (one
+        # operator spamming /qa-recheck without merit). Re-running the
+        # read-only eval on a non-collaborator comment is harmless;
+        # gating it would be fail-open (skipped steps still publish
+        # `success` for the job's status context).
+        # Only runs on issue_comment events; pull_request_target has
+        # no comment.user.login so the step is a no-op skip there.
+        if: github.event_name == 'issue_comment'
+        env:
+          GITEA_TOKEN: ${{ secrets.RFC_324_TEAM_READ_TOKEN || secrets.GITHUB_TOKEN }}
+        run: |
+          set -euo pipefail
+          login="${{ github.event.comment.user.login }}"
+          code=$(curl -sS -o /dev/null -w '%{http_code}' \
+            -H "Authorization: token ${GITEA_TOKEN}" \
+            "${{ github.server_url }}/api/v1/repos/${{ github.repository }}/collaborators/${login}")
+          if [ "$code" = "204" ]; then
+            echo "::notice::Recheck from ${login} (collaborator=true)"
+          else
+            echo "::notice::Recheck from ${login} (collaborator=false, HTTP ${code}) — proceeding with read-only eval anyway"
+          fi
+
+      - name: Check out BASE ref (A4 — never PR-head)
+        # Loads the review-check.sh script from a trusted ref. For
+        # pull_request_target the default checkout is BASE already; we
+        # set ref explicitly for the issue_comment event too so the
+        # script source is always the default-branch version.
+        # NEVER use ref: ${{ github.event.pull_request.head.sha }} —
+        # that would execute PR-head code with secrets-context.
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2
+        with:
+          ref: ${{ github.event.repository.default_branch }}
+
+      - name: Evaluate qa-review
+        env:
+          GITEA_TOKEN: ${{ secrets.RFC_324_TEAM_READ_TOKEN || secrets.GITHUB_TOKEN }}
+          GITEA_HOST: git.moleculesai.app
+          REPO: ${{ github.repository }}
+          # PR number lives in different places per event:
+          #   pull_request_target → github.event.pull_request.number
+          #   issue_comment       → github.event.issue.number
+          PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }}
+          TEAM: qa
+          TEAM_ID: '20'
+          REVIEW_CHECK_DEBUG: '0'
+          REVIEW_CHECK_STRICT: '0'
+        run: bash .gitea/scripts/review-check.sh

.gitea/workflows/security-review.yml

@@ -0,0 +1,67 @@
+# security-review — non-author APPROVE from the `security` Gitea team
+# required to merge.
+#
+# RFC#324 Step 1 of 5 (workflow-add). Mirror of `qa-review.yml`; differs
+# only in TEAM=security, TEAM_ID=21, and the slash-command name.
+#
+# See `qa-review.yml` header for the full A1-α / A1.1 / A4 / A5 design
+# rationale; everything below is identical in shape.
+
+name: security-review
+
+on:
+  pull_request_target:
+    types: [opened, synchronize, reopened]
+  issue_comment:
+    types: [created]
+
+permissions:
+  contents: read
+  pull-requests: read
+
+jobs:
+  approved:
+    # See qa-review.yml header for full A1-α / A1.1 (v1.3 — informational
+    # log only, NOT a gate) / A4 / A5 design rationale.
+    if: |
+      github.event_name == 'pull_request_target' ||
+      (github.event_name == 'issue_comment' &&
+       github.event.issue.pull_request != null &&
+       startsWith(github.event.comment.body, '/security-recheck'))
+    runs-on: ubuntu-latest
+    steps:
+      - name: Privilege check (A1.1 — INFORMATIONAL log only, NOT a gate)
+        # RFC#324 v1.3 §A1.1: does NOT gate subsequent steps. See
+        # qa-review.yml for full rationale. Eval is read-only/idempotent
+        # so re-running on a non-collaborator comment is harmless.
+        if: github.event_name == 'issue_comment'
+        env:
+          GITEA_TOKEN: ${{ secrets.RFC_324_TEAM_READ_TOKEN || secrets.GITHUB_TOKEN }}
+        run: |
+          set -euo pipefail
+          login="${{ github.event.comment.user.login }}"
+          code=$(curl -sS -o /dev/null -w '%{http_code}' \
+            -H "Authorization: token ${GITEA_TOKEN}" \
+            "${{ github.server_url }}/api/v1/repos/${{ github.repository }}/collaborators/${login}")
+          if [ "$code" = "204" ]; then
+            echo "::notice::Recheck from ${login} (collaborator=true)"
+          else
+            echo "::notice::Recheck from ${login} (collaborator=false, HTTP ${code}) — proceeding with read-only eval anyway"
+          fi
+
+      - name: Check out BASE ref (A4 — never PR-head)
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2
+        with:
+          ref: ${{ github.event.repository.default_branch }}
+
+      - name: Evaluate security-review
+        env:
+          GITEA_TOKEN: ${{ secrets.RFC_324_TEAM_READ_TOKEN || secrets.GITHUB_TOKEN }}
+          GITEA_HOST: git.moleculesai.app
+          REPO: ${{ github.repository }}
+          PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }}
+          TEAM: security
+          TEAM_ID: '21'
+          REVIEW_CHECK_DEBUG: '0'
+          REVIEW_CHECK_STRICT: '0'
+        run: bash .gitea/scripts/review-check.sh