diff --git a/.gitea/scripts/review-check.sh b/.gitea/scripts/review-check.sh new file mode 100755 index 00000000..858e6c41 --- /dev/null +++ b/.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 diff --git a/.gitea/workflows/qa-review.yml b/.gitea/workflows/qa-review.yml new file mode 100644 index 00000000..3bc343b9 --- /dev/null +++ b/.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 ()` — 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 diff --git a/.gitea/workflows/security-review.yml b/.gitea/workflows/security-review.yml new file mode 100644 index 00000000..2243d9ca --- /dev/null +++ b/.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