From 3dd730203fdc341f5599c2221b82c408019e9940 Mon Sep 17 00:00:00 2001 From: Jonathan Zhang Date: Tue, 28 Jul 2026 09:17:06 -0700 Subject: [PATCH 1/2] feat(automerge): refuse auto-merge on non-default bases, with a selftest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit whois-api-llc/wxa-jake-ai#1027 (2026-07-28): a PR opened against another OPEN PR's head branch squash-merged 68 seconds after creation, growing that PR's diff from 6 files to 8 AFTER its 4-round codex review completed. The human is then asked to click merge on a diff nobody reviewed. Branch rulesets are conventionally scoped to the default branch (`ref_name: ~DEFAULT_BRANCH`), so a feature-branch base has NO required status checks and `gh pr merge --auto` merges on the spot. That is what makes this a gate rather than a routing preference: every other protection in this lane is latency-shaped — the caller-side classification gate, this workflow's own hold read — and silently assumes there is a wait to lose. Remove the wait and an unguarded arm always wins. Policy: * base == default branch -> unchanged, and costs no API calls * base is another open PR's head -> refused, NOT liftable by the opt-in * base != default branch -> refused without the new `nonmain_base_optin_label` (default `auto-merge-nonmain`) Fail-closed throughout: an unreadable PR listing or label set is an unknown base state and must never read as "no refusal". A refusal also revokes an existing arm, and the always() error-revoke now counts a failed base read. The arm re-binds to the base (and, on a non-default base, re-lists open heads and re-checks the opt-in) immediately before arming, because all three mutate WITHOUT moving the head SHA that `--match-head-commit` binds — and opening the parent PR fires no event for the child at all. selftest/test_automerge_base_gate.sh extracts the shipped bash and runs it against a stubbed `gh`, following test_automerge_hold_gate.sh conventions: 15 behavioural cases plus structural pins (pagination, non-aggregating jq, same-repo head filter, and no pre-arm rejection that exits without disarming). Verified non-vacuous — every structural pin fails against origin/main. Auto-Merge-Risk: high Co-Authored-By: Claude Opus 5 --- .github/workflows/claude-author-automerge.yml | 249 +++++++++- selftest/README.md | 10 + selftest/test_automerge_base_gate.sh | 436 ++++++++++++++++++ selftest/test_workflow_guards.py | 1 + 4 files changed, 695 insertions(+), 1 deletion(-) create mode 100644 selftest/test_automerge_base_gate.sh diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index c352cc5..67539e9 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -127,6 +127,11 @@ on: required: false type: string default: "manual-merge" + nonmain_base_optin_label: + description: "Label that opts a PR based on a NON-default branch into auto-merge. Non-default bases refuse by default: branch rulesets are conventionally scoped to the default branch, so a feature-branch base has no required checks and `--auto` merges on the spot (wxa-jake-ai#1027). This label does NOT lift the unconditional refusal when the base is another OPEN PR's head. Set empty to refuse every non-default base with no opt-in path." + required: false + type: string + default: "auto-merge-nonmain" codex_check_name: description: "Status-check name for Codex Review. When this check is SUCCESS on a Claude-authored PR, the risk-tier check is bypassed (Option B). Set empty to disable Codex-trusted bypass." required: false @@ -623,6 +628,168 @@ jobs: exit 1 fi + # Base-branch gate. Runs AFTER the hold so a durable hold always wins + # the reason line, and BEFORE the risk/bypass lane so no bypass label + # and no Codex pass can talk past it. + # + # whois-api-llc/wxa-jake-ai#1027 (2026-07-28): a PR opened against + # another OPEN PR's head branch squash-merged 68 seconds after it was + # created, growing that PR's diff from 6 files to 8 files AFTER its + # 4-round codex review had completed — the human is then asked to + # click merge on a diff nobody reviewed. Branch rulesets are + # conventionally scoped to the default branch (`ref_name: + # ~DEFAULT_BRANCH`), so a feature-branch base has NO required checks + # and `gh pr merge --auto` merges on the spot instead of waiting. + # + # That instant merge is what makes this a security gap rather than a + # routing quirk: every other protection in this lane is latency-shaped + # (a caller-side classification gate, this workflow's own hold read) + # and silently assumes there is a wait to lose. Remove the wait and + # the unguarded arm always wins. In #1027 the losing disarm started 7 + # seconds after the merge had already landed. + # + # Policy: the default branch is the only ref the ruleset protects, so + # it is the only base where "auto-merge" means "merge after required + # checks and review". Other bases refuse unless a human opts in + # per-PR. A base that is itself an open PR's head refuses outright — + # not overridable — because that is the review-scope-expansion vector. + # + # Fail-closed: an unreadable PR listing or label set is an UNKNOWN + # base state, which must never read as "no refusal". + - name: Check base branch + id: base_gate + if: | + steps.detect.outputs.claude_authored == '1' && + steps.classifier_verdict.outputs.blocked != '1' && + steps.hold.outputs.hold != '1' + env: + GH_TOKEN: ${{ github.token }} + PR: ${{ github.event.pull_request.number }} + BASE_REF: ${{ github.event.pull_request.base.ref }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + OPTIN_LABEL: ${{ inputs.nonmain_base_optin_label }} + run: | + set -euo pipefail + retry() { local i out; for i in 1 2 3; do if out=$("$@"); then printf '%s' "$out"; return 0; fi; [ "$i" -lt 3 ] && sleep $((i * 5)); done; return 1; } + refuse=0 + reason="" + + if [ -z "$BASE_REF" ] || [ -z "$DEFAULT_BRANCH" ]; then + echo "::error::empty base ref ('$BASE_REF') or default branch ('$DEFAULT_BRANCH') — cannot establish the base is protected; refusing to arm auto-merge (fail closed)." + exit 1 + fi + + # The open-PR-head check below is deliberately scoped to NON-default + # bases. Review raised the `main -> release` train shape, where the + # default branch is itself an open PR's head; applying the check + # there would refuse auto-merge for EVERY PR in every such repo for + # as long as that release PR stays open — a fleet-wide liveness + # regression. The trade is wrong in both directions: this invariant + # exists to stop merges into an UNPROTECTED branch someone is + # reviewing as a fixed diff, and the default branch is neither — + # it is ruleset-protected, and a release train PR is by + # construction expected to accumulate whatever lands on it. + if [ "$BASE_REF" != "$DEFAULT_BRANCH" ]; then + # Listed with a NON-aggregating jq: `gh api --paginate` emits one + # JSON array per page, so an aggregating filter would compute a + # per-page answer and print one line per page (the multi-page + # defect pinned in test_automerge_hold_gate.sh). This streams + # every page's refs into one newline-separated list. + # + # Same-repo heads only. A fork PR's `.head.ref` is just a branch + # name in the fork, so an unrelated fork branch called `develop` + # would otherwise read as "the parent PR's head" and refuse a + # legitimate opted-in merge (codex P2). `.head.repo` is null when + # a fork is deleted; in jq that yields null, which compares + # unequal and is filtered out — the safe direction. + open_heads=$(retry gh api "/repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" \ + --paginate --jq '.[] | select(.head.repo.full_name == env.GITHUB_REPOSITORY) | .head.ref') || { + echo "::error::could not list open PRs after 3 attempts — cannot rule out that '$BASE_REF' is another open PR's head; refusing to arm auto-merge (fail closed). Re-run when the API recovers, or merge manually." + exit 1 + } + if printf '%s\n' "$open_heads" | grep -qxF "$BASE_REF"; then + # Deliberately NOT liftable by OPTIN_LABEL: opting into a + # plain integration branch is a routing choice; opting into + # rewriting an under-review PR's diff is not one a label + # should be able to make. + refuse=1 + reason="base '$BASE_REF' is the head branch of another open PR — merging would expand that PR's diff after its review" + elif [ -n "$OPTIN_LABEL" ]; then + labels=$(retry gh api "/repos/${GITHUB_REPOSITORY}/issues/${PR}/labels?per_page=100" \ + --paginate --jq '.[].name') || { + echo "::error::could not read PR labels after 3 attempts — cannot confirm a '$OPTIN_LABEL' opt-in; refusing to arm auto-merge (fail closed)." + exit 1 + } + if printf '%s\n' "$labels" | grep -qxF "$OPTIN_LABEL"; then + echo "::notice::base '$BASE_REF' is not the default branch, but '$OPTIN_LABEL' opts in explicitly." + else + refuse=1 + reason="base '$BASE_REF' is not the default branch ('$DEFAULT_BRANCH') and the '$OPTIN_LABEL' opt-in label is absent" + fi + else + refuse=1 + reason="base '$BASE_REF' is not the default branch ('$DEFAULT_BRANCH') and the non-default-base opt-in is disabled" + fi + fi + + { + echo "refuse=$refuse" + echo "reason=$reason" + # Published so the arm can re-bind to the routing THIS run + # validated — see the re-read in "Enable auto-merge". + echo "base_ref=$BASE_REF" + } >> "$GITHUB_OUTPUT" + if [ "$refuse" -eq 1 ]; then + echo "::notice::Auto-merge withheld: $reason. Manual merge remains available." + else + echo "Base '$BASE_REF' cleared." + fi + + # Same idempotent revoke as the hold label: a base refusal must also + # disarm an arm placed by an earlier run (or by hand) — "refusing to + # arm" alone is not fail-closed for a PR that is already armed. + - name: Revoke auto-merge on base-gate refusal + if: | + steps.detect.outputs.claude_authored == '1' && + steps.base_gate.outputs.refuse == '1' + env: + GH_TOKEN: ${{ github.token }} + PR_URL: ${{ github.event.pull_request.html_url }} + PR: ${{ github.event.pull_request.number }} + GATE_BASE_REF: ${{ steps.base_gate.outputs.base_ref }} + run: | + set -euo pipefail + # Ownership guard, mirroring the error-revoke's head-moved check. + # This verdict describes the base THIS run evaluated. Retargeting + # from a refused base onto an allowed one fires no event of its + # own, but any later subscribed event (a push, a label) starts a + # newer run that can validly arm — and this older run would then + # strand the PR by revoking a decision made from current state. + # Skip only on a POSITIVE differing read; if the read fails, revoke + # anyway (fail toward safety — a rare stale disarm self-heals on + # the next event, an unreviewed merge does not). + # + # This is deliberately NOT symmetric with the pre-arm disarm in + # "Enable auto-merge", which stays unconditional on a base change. + # There, the run HOLDS a live read proving the arm it is about to + # place was validated against a base that is no longer current, and + # nothing re-gates a retarget. Here the VERDICT is the stale + # artifact, and whichever run placed any live arm validated the + # base itself at arm time. + now_base=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}" --jq .base.ref 2>/dev/null || echo "") + if [ -n "$now_base" ] && [ -n "$GATE_BASE_REF" ] && [ "$now_base" != "$GATE_BASE_REF" ]; then + echo "::notice::base changed ('$GATE_BASE_REF' → '$now_base') since this run's gate — this refusal is stale and a newer run owns the arm decision; skipping revoke." + exit 0 + fi + gh pr merge --disable-auto "$PR_URL" 2>&1 || true + state=$(gh pr view "$PR_URL" --json autoMergeRequest \ + --jq 'if .autoMergeRequest == null then "OFF" else "ON" end') + echo "Auto-merge state after base-gate revoke: $state" + if [ "$state" = "ON" ]; then + echo "::error::auto-merge still ON after --disable-auto — investigate" + exit 1 + fi + # Option A: explicit human approval via label. # Run BEFORE the risk-tier path-scan so a PR the scan can't # classify can still bypass-merge. Historically that meant any @@ -844,7 +1011,8 @@ jobs: if: | steps.detect.outputs.claude_authored == '1' && steps.classifier_verdict.outputs.blocked != '1' && - steps.hold.outputs.hold != '1' && ( + steps.hold.outputs.hold != '1' && + steps.base_gate.outputs.refuse != '1' && ( steps.risk.outputs.risky != '1' || steps.bypass_label.outputs.bypass == '1' || steps.bypass_codex.outputs.bypass == '1' @@ -865,8 +1033,86 @@ jobs: BYPASS_LABEL: ${{ steps.bypass_label.outputs.bypass }} BYPASS_CODEX: ${{ steps.bypass_codex.outputs.bypass }} USING_PAT: ${{ secrets.automerge_pat != '' && '1' || '0' }} + GATE_BASE_REF: ${{ steps.base_gate.outputs.base_ref }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + OPTIN_LABEL: ${{ inputs.nonmain_base_optin_label }} run: | set -euo pipefail + # Re-bind the base-gate verdict to live state immediately before + # arming. `--match-head-commit` below binds the arm to the head + # SHA, but the BASE and the opt-in LABEL both mutate WITHOUT moving + # that SHA — so a run that cleared the gate, followed by a retarget + # onto an unprotected branch (or removal of the opt-in label), + # would arm against routing nothing ever validated and, with no + # required checks on that base, merge instantly. Refusing on ANY + # change is stricter than re-running the base rules and needs no + # second open-PR listing: the retarget's own event re-gates from + # scratch. + # + # Every rejection below DISARMS before exiting. Declining to call + # `gh pr merge --auto` is not enough when an earlier run already + # armed this PR: the base-gate refusal-revoke did not fire (that + # run's gate passed) and the always() error-revoke does not fire + # either when this step exits 0. Without the disarm, the change + # would be detected and then ignored, the stale arm merging anyway. + disarm_then_exit() { + gh pr merge --disable-auto "$PR_URL" 2>&1 || true + state=$(gh pr view "$PR_URL" --json autoMergeRequest \ + --jq 'if .autoMergeRequest == null then "OFF" else "ON" end' 2>/dev/null || echo "UNKNOWN") + echo "Auto-merge state after pre-arm revalidation revoke: $state" + if [ "$state" != "OFF" ]; then + echo "::error::auto-merge is '$state' after --disable-auto — investigate" + exit 1 + fi + exit "$1" + } + now_base=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}" --jq .base.ref) || { + echo "::error::could not re-read the base ref before arming — refusing to arm (fail closed)." + disarm_then_exit 1 + } + # NOTE — do NOT "fix" this into an ownership guard that skips the + # disarm when the base moved (proposed as a stale-run concern in + # review). That reasoning assumes a retarget starts a newer run + # which owns the decision. It does not: a base change emits + # `pull_request: edited`, and no caller subscribes to `edited` + # (types are opened/synchronize/reopened/ready_for_review/labeled). + # So a retarget fires NOTHING, and skipping the disarm would leave + # an arm validated against the OLD base live on the new one — with + # no required checks there, it merges immediately. That is the + # incident, rebuilt. Disarming can strand a PR until its next event + # (or a human click); that cost is bounded, visible, and recoverable, + # and an unreviewed merge is none of those. + if [ "$now_base" != "$GATE_BASE_REF" ]; then + echo "::notice::base changed ('$GATE_BASE_REF' → '$now_base') since this run's gate read — not arming, and disarming: no event fires on a retarget, so an arm validated against '$GATE_BASE_REF' must not survive onto '$now_base'." + disarm_then_exit 0 + fi + if [ "$now_base" != "$DEFAULT_BRANCH" ]; then + # Re-list open heads. Opening the PARENT PR fires NO event for + # this child, so nothing would re-gate it: a parent opened + # between this run's gate read and here would let the child merge + # into a branch that is now itself under review — precisely the + # #1027 shape, reassembled from the other side. + now_heads=$(gh api "/repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" \ + --paginate --jq '.[] | select(.head.repo.full_name == env.GITHUB_REPOSITORY) | .head.ref') || { + echo "::error::could not re-list open PRs before arming a non-default base — refusing to arm (fail closed)." + disarm_then_exit 1 + } + if printf '%s\n' "$now_heads" | grep -qxF "$now_base"; then + echo "::notice::base '$now_base' became the head branch of an open PR since this run's gate read — not arming." + disarm_then_exit 0 + fi + if [ -n "$OPTIN_LABEL" ]; then + optin=$(gh api "/repos/${GITHUB_REPOSITORY}/issues/${PR}/labels?per_page=100" \ + --paginate --jq '.[].name') || { + echo "::error::could not re-read labels before arming a non-default base — refusing to arm (fail closed)." + disarm_then_exit 1 + } + if ! printf '%s\n' "$optin" | grep -qxF "$OPTIN_LABEL"; then + echo "::notice::'$OPTIN_LABEL' is no longer present and the base is not the default branch — not arming." + disarm_then_exit 0 + fi + fi + fi if [ "$BYPASS_LABEL" = "1" ]; then echo "Enabling auto-merge ($METHOD) — detection: $REASON; risk bypassed via approval label" elif [ "$BYPASS_CODEX" = "1" ]; then @@ -1041,6 +1287,7 @@ jobs: startsWith(github.event.pull_request.head.ref, 'claude/'))) && (steps.classifier_verdict.outcome == 'failure' || steps.hold.outcome == 'failure' || + steps.base_gate.outcome == 'failure' || steps.risk.outcome == 'failure' || steps.setup_node.outcome == 'failure' || steps.checkout.outcome == 'failure' || diff --git a/selftest/README.md b/selftest/README.md index 4d575fa..3bbcb84 100644 --- a/selftest/README.md +++ b/selftest/README.md @@ -43,6 +43,16 @@ arbitrary helper scripts; that's a different kind of repo. review went red. Requiring a recognizable CLEAN phrase failed the same way. The accepted cost — asserted explicitly — is that a finding phrased outside both forms is missed. +- `test_automerge_base_gate.sh` — auto-merge may only target the ref a + branch ruleset actually protects. Rulesets are conventionally scoped to + the default branch (`ref_name: ~DEFAULT_BRANCH`), so a feature-branch base + has no required checks and `--auto` merges on the spot; every other + protection in that lane is latency-shaped and assumes a wait to lose. + wxa-jake-ai#1027 merged into another open PR's head 68s after opening and + grew that PR's reviewed diff from 6 files to 8. Pins: default base is free + (no API calls), another open PR's head refuses unconditionally, other + non-default bases need the opt-in label, and every unreadable input + refuses. - `test_pr_files_listing.sh` — no reusable may fetch changed files via `gh pr diff` (HTTP 406 past 20k diff lines); pins the paginated files-API idiom instead. diff --git a/selftest/test_automerge_base_gate.sh b/selftest/test_automerge_base_gate.sh new file mode 100644 index 0000000..8682354 --- /dev/null +++ b/selftest/test_automerge_base_gate.sh @@ -0,0 +1,436 @@ +#!/usr/bin/env bash +# Behavioral test for the base-branch gate in claude-author-automerge.yml — +# the step that keeps auto-merge pointed at the one ref a branch ruleset +# actually protects. +# +# Incident context (whois-api-llc/wxa-jake-ai#1027, 2026-07-28): a PR opened +# against ANOTHER OPEN PR's head branch was squash-merged 68 seconds after +# creation. It grew that PR's diff from 6 files to 8 AFTER its 4-round codex +# review had completed, so the human being asked to click merge on the parent +# would approve two files nobody reviewed. +# +# The mechanism is what makes this a gate and not a preference. Branch +# rulesets are conventionally scoped to the default branch (`ref_name: +# ~DEFAULT_BRANCH`), so a feature-branch base has NO required status checks: +# `gh pr merge --auto` merges on the spot instead of waiting. Every other +# protection in this lane is latency-shaped — the caller-side classification +# gate, this workflow's own hold read — and silently assumes there is a wait +# to lose. In #1027 the disarm that would have caught it started 7 seconds +# AFTER the merge landed. Remove the wait and the unguarded arm always wins, +# which is why "the hold label protects main-targeted PRs" was true only by +# accident of timing. +# +# Policy pinned here: +# +# * base == default branch → no refusal, and NO API calls (the +# overwhelmingly common path stays free) +# * base is another open PR's head → refuse, NOT liftable by the opt-in +# label (the review-scope-expansion +# vector above) +# * base != default branch → refuse unless the opt-in label is +# present +# * unreadable anything → refuse (fail closed) +# +# This test EXTRACTS the step's bash from the workflow YAML (the shipped +# script, not a mirrored copy) and executes it against a stubbed `gh`: +# +# 1. base == default branch ⇒ no refusal… +# 2. …and not one API call is made. +# 3. #1027 replay: base is another open PR's head ⇒ refuse. +# 4. the opt-in label does NOT lift that refusal. +# 5. non-default base, not another PR's head, no opt-in ⇒ refuse. +# 6. non-default base + opt-in label ⇒ allowed. +# 7. near-miss opt-in label names do not opt in (grep -qxF exact match). +# 8. a base that is a SUBSTRING of an open head does not refuse. +# 9. MULTI-PAGE: the matching head arrives on a later page ⇒ refuse. +# 10. opt-in input empty ⇒ every non-default base refuses; labels unread. +# 11. FAIL CLOSED: open-PR listing fails 3× ⇒ nonzero exit. +# 12. FAIL CLOSED: label read fails 3× ⇒ nonzero exit. +# 13. FAIL CLOSED: empty base ref ⇒ nonzero exit. +# 14. FAIL CLOSED: empty default branch ⇒ nonzero exit. +# 15. open-PR listing fails twice then succeeds ⇒ retry recovers. +# +# Structural pins: the extracted run block contains no `${{ }}` interpolation +# (extraction-safe AND injection-safe); the open-PR listing paginates with a +# NON-aggregating jq (an aggregating filter prints one verdict per page — the +# defect pinned in test_automerge_hold_gate.sh case 7); that listing is +# filtered to same-repo heads (a fork PR's `.head.ref` is only a branch name +# in the fork, so an unrelated fork branch of the same name would read as the +# parent PR's head and refuse an opted-in merge); the enable step +# consults the gate; a refusal revokes an existing arm; and the always() +# error-revoke counts a failed gate read as a gate error, so a fail-closed +# read also disarms a PR armed by an earlier run. +# +# Run from the repo root: +# bash selftest/test_automerge_base_gate.sh +set -euo pipefail + +WF=.github/workflows/claude-author-automerge.yml +failed=0 +T=$(mktemp -d) +trap 'rm -rf "$T"' EXIT + +# --------------------------------------------------------------------------- +# 0a. Structural pins — wiring this test cannot see from the extracted bash. +# --------------------------------------------------------------------------- +if grep -q "steps.base_gate.outputs.refuse != '1'" "$WF"; then + echo "✓ enable step consults the base gate" +else + echo "✗ enable step does not consult steps.base_gate.outputs.refuse — the gate computes a verdict nothing reads" + failed=1 +fi +if grep -q "steps.base_gate.outcome == 'failure'" "$WF"; then + echo "✓ error-revoke counts a failed base-gate read as a gate error (stale arm disarmed)" +else + echo "✗ error-revoke omits steps.base_gate.outcome == 'failure' — a fail-closed read leaves a stale arm alive" + failed=1 +fi +if grep -q 'name: Revoke auto-merge on base-gate refusal' "$WF"; then + echo "✓ a base-gate refusal revokes an already-armed auto-merge" +else + echo "✗ no revoke step for a base-gate refusal — refusing to arm is not fail-closed for an already-armed PR" + failed=1 +fi +# The base and the opt-in label mutate WITHOUT moving the head SHA, so +# --match-head-commit alone does not bind this verdict. A stale run that +# cleared the gate before a retarget would otherwise arm against an +# unprotected base and — having no required checks there — merge instantly. +enable_block=$(awk '/name: Enable auto-merge/{f=1} f' "$WF") +if echo "$enable_block" | grep -q 'could not re-read the base ref before arming'; then + echo "✓ the arm re-binds to the base this run's gate validated" +else + echo "✗ the arm does not revalidate the base — a retarget between the gate read and the arm merges under unvalidated routing" + failed=1 +fi +if echo "$enable_block" | grep -q 'is no longer present and the base is not the default branch'; then + echo "✓ the arm re-checks the opt-in label on a non-default base" +else + echo "✗ the arm does not re-check the opt-in label — removing it before the arm still merges on a non-default base" + failed=1 +fi +if echo "$enable_block" | grep -q 'became the head branch of an open PR since'; then + echo "✓ the arm re-lists open heads (a parent PR opened after the gate fires no event for this child)" +else + echo "✗ the arm does not re-list open heads — a parent PR opened between gate and arm still merges the child into it" + failed=1 +fi +# Every PRE-ARM rejection must DISARM, not merely decline to re-arm: the PR +# may already be armed by an earlier run, whose own gate legitimately passed. +# Scoped to the revalidation region (everything before the arm itself) — the +# head-moved branch AFTER the arm deliberately does NOT revoke, because that +# run's knowledge is stale and its revoke could land after the newer head's +# run validly armed. See that branch's own comment. +# The helper's own body legitimately contains `exit 1` (the "still ON after +# --disable-auto" escalation), so strip the function before scanning for +# bare exits in the rejection paths. +prearm=$(echo "$enable_block" \ + | awk '/gh pr merge --auto/{exit} {print}' \ + | awk '/disarm_then_exit\(\) \{/{inf=1} inf && /^ *\}$/{inf=0; next} !inf') +if echo "$enable_block" | grep -q 'disarm_then_exit()' \ + && ! echo "$prearm" | grep -qE '^ *exit [01]$'; then + echo "✓ every pre-arm rejection disarms an existing arm (no bare exit-without-disarm)" +else + echo "✗ a pre-arm rejection exits without disarming — an arm placed by an earlier run survives the rejection" + failed=1 +fi + +# --------------------------------------------------------------------------- +# 0b. Extract the base-gate step's run block — the shipped bash. +# --------------------------------------------------------------------------- +awk ' + /^ - name: Check base branch$/ { in_step=1 } + in_step && /^ run: \|/ { in_run=1; next } + in_run { + if ($0 ~ /^ / || $0 == "") { sub(/^ /, ""); print } + else { exit } + } +' "$WF" > "$T/base.sh" + +if ! grep -q 'BASE_REF' "$T/base.sh" || ! grep -q 'GITHUB_OUTPUT' "$T/base.sh"; then + echo "✗ could not extract the base-gate run block from $WF" + exit 1 +fi +echo "✓ extracted base-gate step ($(wc -l < "$T/base.sh" | tr -d ' ') lines)" + +if grep -q '${{' "$T/base.sh"; then + echo "✗ base-gate run block contains \${{ }} interpolation — untestable standalone and an injection-risk pattern; use env vars" + failed=1 +else + echo "✓ base-gate run block is interpolation-free (env vars only)" +fi + +if grep -q 'pulls?state=open&per_page=100' "$T/base.sh" \ + && grep -A1 'pulls?state=open&per_page=100' "$T/base.sh" | grep -q -- '--paginate'; then + echo "✓ open-PR listing paginates (a base branch past page 1 cannot read as unlisted)" +else + echo "✗ open-PR listing does not paginate with an explicit page size" + failed=1 +fi +# The filter streams `.head.ref` per page; an aggregating filter +# (map/add/any/reduce) would emit one answer PER PAGE — the multi-page defect +# from the hold gate. +if grep -q -- '\.head\.ref' "$T/base.sh" \ + && ! grep -Eq -- "--jq '[^']*(map\(|add|reduce |any\()" "$T/base.sh"; then + echo "✓ open-PR listing uses a non-aggregating jq (safe under --paginate)" +else + echo "✗ open-PR listing must stream .head.ref — an aggregating filter yields one answer per page under --paginate" + failed=1 +fi +# Fork PRs share the head-ref namespace: a fork branch named like this repo's +# base would read as "the parent PR's head" and refuse a legitimately +# opted-in merge. Compare full_name, not just the ref. (Codex round 1, P2.) +if grep -q 'head.repo.full_name == env.GITHUB_REPOSITORY' "$T/base.sh"; then + echo "✓ open-PR listing is filtered to same-repo heads (fork refs cannot collide)" +else + echo "✗ open-PR listing does not filter on .head.repo.full_name — an unrelated fork branch of the same name reads as the parent PR's head" + failed=1 +fi + +# --------------------------------------------------------------------------- +# 0c. Stubs. `gh` dispatches on the requested URL and logs every call; +# `sleep` no-ops the retry backoffs. Output is post-`--jq` shaped, as the +# real `gh api --jq` would emit. Knobs (env): +# STUB_OPEN_HEADS — newline-separated open-PR head refs +# STUB_OPENPR_FAIL_TIMES — first N open-PR listings fail +# STUB_LABELS — newline-separated label names +# STUB_LABELS_FAIL_TIMES — first N label reads fail +# --------------------------------------------------------------------------- +mkdir -p "$T/bin" +cat > "$T/bin/gh" <<'STUB' +#!/usr/bin/env bash +args="$*" +echo "$args" >> "$CALLS_LOG" +bump() { local f="$1" n; n=$(cat "$f" 2>/dev/null || echo 0); n=$((n + 1)); echo "$n" > "$f"; echo "$n"; } +case "$args" in + *"pulls?state=open"*) + n=$(bump "$STUB_DIR/openpr-attempts") + if [ "$n" -le "${STUB_OPENPR_FAIL_TIMES:-0}" ]; then + echo "gh: Internal Server Error (HTTP 500)" >&2 + exit 1 + fi + printf '%s\n' "${STUB_OPEN_HEADS:-}" + ;; + *issues/*/labels*) + n=$(bump "$STUB_DIR/labels-attempts") + if [ "$n" -le "${STUB_LABELS_FAIL_TIMES:-0}" ]; then + echo "gh: Internal Server Error (HTTP 500)" >&2 + exit 1 + fi + printf '%s\n' "${STUB_LABELS:-}" + ;; + *) + echo "gh-stub: unexpected call: $args" >&2 + exit 64 + ;; +esac +STUB +chmod +x "$T/bin/gh" +printf '#!/usr/bin/env bash\nexit 0\n' > "$T/bin/sleep" +chmod +x "$T/bin/sleep" + +# --------------------------------------------------------------------------- +# Runner + assertions. +# --------------------------------------------------------------------------- +reset_case() { + STUB_OPEN_HEADS=""; STUB_OPENPR_FAIL_TIMES=0 + STUB_LABELS=""; STUB_LABELS_FAIL_TIMES=0 + CASE_BASE_REF="main"; CASE_DEFAULT_BRANCH="main" + CASE_OPTIN_LABEL="auto-merge-nonmain" +} + +run_gate() { + OUT_FILE="$T/gh-output.txt" + CALLS_LOG="$T/calls.log" + : > "$OUT_FILE" + : > "$CALLS_LOG" + rm -f "$T/openpr-attempts" "$T/labels-attempts" + set +e + GATE_LOG=$(cd "$T" && \ + PATH="$T/bin:$PATH" \ + GITHUB_REPOSITORY="acme/fixture" PR=1027 \ + BASE_REF="$CASE_BASE_REF" DEFAULT_BRANCH="$CASE_DEFAULT_BRANCH" \ + OPTIN_LABEL="$CASE_OPTIN_LABEL" \ + GITHUB_OUTPUT="$OUT_FILE" GH_TOKEN=stub \ + STUB_DIR="$T" CALLS_LOG="$CALLS_LOG" \ + STUB_OPEN_HEADS="$STUB_OPEN_HEADS" \ + STUB_OPENPR_FAIL_TIMES="$STUB_OPENPR_FAIL_TIMES" \ + STUB_LABELS="$STUB_LABELS" STUB_LABELS_FAIL_TIMES="$STUB_LABELS_FAIL_TIMES" \ + bash base.sh 2>&1) + GATE_RC=$? + set -e +} + +out_get() { grep "^$1=" "$OUT_FILE" | tail -1 | cut -d= -f2- || true; } + +# expect_gate [reason-substring] +expect_gate() { + local desc="$1" want="$2" needle="${3:-}" got + if [ "$GATE_RC" != "0" ]; then + echo "✗ $desc — step exited rc=$GATE_RC (want 0). Output:" + echo "$GATE_LOG" | sed 's/^/ /' + failed=1 + return + fi + got=$(out_get refuse) + if [ "$got" != "$want" ]; then + echo "✗ $desc — got refuse='$got' (want '$want'). Output:" + echo "$GATE_LOG" | sed 's/^/ /' + failed=1 + return + fi + if [ -n "$needle" ] && ! out_get reason | grep -q "$needle"; then + echo "✗ $desc — reason '$(out_get reason)' does not mention '$needle'" + failed=1 + return + fi + echo "✓ $desc" +} + +# expect_fail_closed +expect_fail_closed() { + local desc="$1" needle="$2" + if [ "$GATE_RC" -ne 0 ] && echo "$GATE_LOG" | grep -q "$needle"; then + echo "✓ $desc" + else + echo "✗ $desc — want nonzero rc + '$needle'; got rc=$GATE_RC. Output:" + echo "$GATE_LOG" | sed 's/^/ /' + failed=1 + fi +} + +# expect_calls +expect_calls() { + local desc="$1" needle="$2" want="$3" got + got=$(grep -c "$needle" "$CALLS_LOG" || true) + if [ "$got" = "$want" ]; then + echo "✓ $desc" + else + echo "✗ $desc — $needle called $got time(s), want $want. Calls:" + sed 's/^/ /' "$CALLS_LOG" + failed=1 + fi +} + +echo "" +echo "— behavioral cases —" + +# 1/2. The default branch is the protected ref: no refusal, and the common +# path must not spend API calls to learn that. +reset_case +run_gate +expect_gate "base == default branch ⇒ no refusal" 0 +expect_calls " … and no API call is made on the default-branch path" "" 0 + +# 3. THE #1027 REPLAY. +reset_case +CASE_BASE_REF="fix/qa1104-credit-guard-residuals" +STUB_OPEN_HEADS=$(printf '%s\n' 'fix/qa1104-estimate-shape-filter' 'fix/qa1104-credit-guard-residuals') +run_gate +expect_gate "#1027 replay: base is another open PR's head ⇒ refuse" 1 "head branch of another open PR" +expect_calls " … and the label read is skipped (that refusal is unconditional)" "/labels" 0 + +# 4. The opt-in label must NOT lift the open-PR-head refusal — opting into a +# plain integration branch is a routing choice; opting into rewriting an +# under-review PR's diff is not one a label should be able to make. +reset_case +CASE_BASE_REF="fix/parent" +STUB_OPEN_HEADS="fix/parent" +STUB_LABELS=$(printf '%s\n' 'risk:standard' 'auto-merge-nonmain') +run_gate +expect_gate "opt-in label does NOT lift the open-PR-head refusal" 1 "head branch of another open PR" + +# 5. Plain non-default base, no opt-in. +reset_case +CASE_BASE_REF="integration/v2" +STUB_OPEN_HEADS="something/else" +run_gate +expect_gate "non-default base without the opt-in label ⇒ refuse" 1 "not the default branch" + +# 6. Plain non-default base WITH opt-in. +reset_case +CASE_BASE_REF="integration/v2" +STUB_OPEN_HEADS="something/else" +STUB_LABELS=$(printf '%s\n' 'risk:standard' 'auto-merge-nonmain') +run_gate +expect_gate "non-default base WITH the opt-in label ⇒ allowed" 0 + +# 7. Near-miss opt-in label names must not opt in. +reset_case +CASE_BASE_REF="integration/v2" +STUB_OPEN_HEADS="something/else" +STUB_LABELS=$(printf '%s\n' 'auto-merge-nonmain-please' 'not-auto-merge-nonmain' 'Auto-Merge-Nonmain') +run_gate +expect_gate "near-miss opt-in label names (prefix/suffix/case) do not opt in" 1 "not the default branch" + +# 8. Substring must not count as a match — grep -qxF is whole-line. +reset_case +CASE_BASE_REF="fix/parent" +STUB_OPEN_HEADS=$(printf '%s\n' 'fix/parent-2' 'prefix/fix/parent') +run_gate +expect_gate "a base that is only a SUBSTRING of an open head does not refuse as one" 1 "not the default branch" + +# 9. MULTI-PAGE: `gh api --paginate` concatenates pages; the matching head +# arrives after the first page's worth of refs. +reset_case +CASE_BASE_REF="fix/parent" +STUB_OPEN_HEADS=$(printf '%s\n' 'a/1' 'b/2' 'c/3' 'fix/parent') +run_gate +expect_gate "matching head later in a paginated listing ⇒ refuse" 1 "head branch of another open PR" + +# 10. Opt-in disabled by the caller ⇒ no escape hatch at all, labels unread. +reset_case +CASE_BASE_REF="integration/v2" +CASE_OPTIN_LABEL="" +STUB_OPEN_HEADS="something/else" +STUB_LABELS="auto-merge-nonmain" +run_gate +expect_gate "opt-in input empty ⇒ every non-default base refuses" 1 "opt-in is disabled" +expect_calls " … and the labels endpoint is never called" "/labels" 0 + +# 11. FAIL CLOSED: cannot rule out that the base is another PR's head. +reset_case +CASE_BASE_REF="integration/v2" +STUB_OPENPR_FAIL_TIMES=3 +run_gate +expect_fail_closed "unreadable open-PR listing (HTTP 500 ×3) fails closed" "could not list open PRs" + +# 12. FAIL CLOSED: cannot confirm the opt-in. +reset_case +CASE_BASE_REF="integration/v2" +STUB_OPEN_HEADS="something/else" +STUB_LABELS_FAIL_TIMES=3 +run_gate +expect_fail_closed "unreadable label set (HTTP 500 ×3) fails closed" "could not read PR labels" + +# 13/14. FAIL CLOSED on a missing base or default branch. An absent +# `github.event.repository.default_branch` would otherwise compare +# unequal to every base and silently refuse-or-allow on noise. +reset_case +CASE_BASE_REF="" +run_gate +expect_fail_closed "empty base ref fails closed" "empty base ref" +reset_case +CASE_DEFAULT_BRANCH="" +run_gate +expect_fail_closed "empty default branch fails closed" "empty base ref" + +# 15. Retry recovers. +reset_case +CASE_BASE_REF="fix/parent" +STUB_OPEN_HEADS="fix/parent" +STUB_OPENPR_FAIL_TIMES=2 +run_gate +expect_gate "open-PR listing fails ×2 then succeeds ⇒ retry recovers, refusal honored" 1 "head branch of another open PR" +if [ "$(cat "$T/openpr-attempts" 2>/dev/null)" = "3" ]; then + echo "✓ … exactly 3 open-PR listing attempts made" +else + echo "✗ … expected 3 open-PR listing attempts, got '$(cat "$T/openpr-attempts" 2>/dev/null)'" + failed=1 +fi + +echo "" +if [ "$failed" -gt 0 ]; then + echo "FAIL: base-branch gate case(s) regressed." + exit 1 +fi +echo "OK: all base-branch gate cases pass." diff --git a/selftest/test_workflow_guards.py b/selftest/test_workflow_guards.py index 90baf50..810107f 100644 --- a/selftest/test_workflow_guards.py +++ b/selftest/test_workflow_guards.py @@ -22,6 +22,7 @@ @pytest.mark.parametrize( "script", [ + "selftest/test_automerge_base_gate.sh", "selftest/test_automerge_hold_gate.sh", "selftest/test_automerge_risk_patterns.sh", "selftest/test_automerge_riskfile_gate.sh", From 0a5c2dedcdfd84d17d30bba0282a525bc2712ce2 Mon Sep 17 00:00:00 2001 From: Jonathan Zhang Date: Tue, 28 Jul 2026 09:22:18 -0700 Subject: [PATCH 2/2] fix(automerge): match labels via here-string, not a pipe into grep -q MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `printf '%s\n' "$x" | grep -qxF "$y"` under `set -o pipefail` can report non-zero even on a MATCH: grep -q exits the moment it matches, and the writer can take SIGPIPE, which pipefail then surfaces as pipeline failure. In the base gate that direction fails OPEN — an opt-in label reading as absent merely over-refuses, but the same shape guards the open-PR-head check, where a missed match is an unreviewed merge. Inert at today's input sizes (label and head lists are far smaller than the 64KB pipe buffer, so the writer never blocks and never signals — the selftest passes either way), but the safety margin is "the list stayed small", which is not a property worth depending on. A here-string has no pipeline and therefore no pipefail interaction at all. Scoped to the code this branch introduces. The pre-existing hold step keeps its `echo | grep` form: it is proven, test_automerge_hold_gate.sh pins that exact shipped bash, and changing it is a drive-by. Auto-Merge-Risk: high Co-Authored-By: Claude Opus 5 --- .github/workflows/claude-author-automerge.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index 67539e9..86947d8 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -707,7 +707,7 @@ jobs: echo "::error::could not list open PRs after 3 attempts — cannot rule out that '$BASE_REF' is another open PR's head; refusing to arm auto-merge (fail closed). Re-run when the API recovers, or merge manually." exit 1 } - if printf '%s\n' "$open_heads" | grep -qxF "$BASE_REF"; then + if grep -qxF "$BASE_REF" <<<"$open_heads"; then # Deliberately NOT liftable by OPTIN_LABEL: opting into a # plain integration branch is a routing choice; opting into # rewriting an under-review PR's diff is not one a label @@ -720,7 +720,7 @@ jobs: echo "::error::could not read PR labels after 3 attempts — cannot confirm a '$OPTIN_LABEL' opt-in; refusing to arm auto-merge (fail closed)." exit 1 } - if printf '%s\n' "$labels" | grep -qxF "$OPTIN_LABEL"; then + if grep -qxF "$OPTIN_LABEL" <<<"$labels"; then echo "::notice::base '$BASE_REF' is not the default branch, but '$OPTIN_LABEL' opts in explicitly." else refuse=1 @@ -1097,7 +1097,7 @@ jobs: echo "::error::could not re-list open PRs before arming a non-default base — refusing to arm (fail closed)." disarm_then_exit 1 } - if printf '%s\n' "$now_heads" | grep -qxF "$now_base"; then + if grep -qxF "$now_base" <<<"$now_heads"; then echo "::notice::base '$now_base' became the head branch of an open PR since this run's gate read — not arming." disarm_then_exit 0 fi @@ -1107,7 +1107,7 @@ jobs: echo "::error::could not re-read labels before arming a non-default base — refusing to arm (fail closed)." disarm_then_exit 1 } - if ! printf '%s\n' "$optin" | grep -qxF "$OPTIN_LABEL"; then + if ! grep -qxF "$OPTIN_LABEL" <<<"$optin"; then echo "::notice::'$OPTIN_LABEL' is no longer present and the base is not the default branch — not arming." disarm_then_exit 0 fi