From a41a00bbe10797d57cdf8bb330d6fa813ccd508e Mon Sep 17 00:00:00 2001 From: Jonathan Zhang Date: Sun, 9 Aug 2026 22:19:10 -1000 Subject: [PATCH] ci(safe-paths): honor risk-tier paths instead of merging past the decline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit topcoder1/inbox_superpilot#215 merged a tests-only PR touching web/tests/e2e/auth/signup.spec.ts twelve minutes after claude-author-automerge posted "Auto-merge blocked — risk-tier paths touched. Manual click-merge required." That comment is still on the merged PR. The two gates never disagreed in any resolvable sense. claude-author-automerge cannot block a merge — it can only decline to ARM one — and safe-paths-automerge is the only workflow that arms a docs/tests diff, and the only one that can revoke a standing arm. So the decline was an abstention, and the permissive gate won by default. The failure is not that the wrong gate won; it is that a gate announced an enforcement it never had, leaving a merged PR carrying an audit trail that asserts a human approval that never happened. Adds a TIER 2 risk-tier hold to safe-paths-automerge.yml, generalizing the docs/legal override already there (wxa_vpn#1268): safe-by-glob is not safe-by-content. The `*.spec.ts` premise "passing tests can't break runtime" is true about RUNTIME and false about DETECTION — deleting the assertion that catches a lockout is how the lockout ships in a LATER PR, laundered through a green suite. Scoping, both directions: - The hold runs ONLY in the branch where every changed file is safe-by-glob, i.e. where this workflow would otherwise arm. Hoisting it beside the tier-1 override looks equivalent and is not: it would emit a revoke-triggering reason on PRs this workflow never arms — a dependabot bump of .github/workflows/** matches the risk patterns — and the revoke step would then disarm dependabot-auto-merge's legitimate arm. Pinned structurally in test_workflow_guards.py. - The hold is released by the caller's risk_bypass_label, the same one-click label claude-author-automerge's blocked-PR comment advertises. Ignoring it would kill the advertised escape hatch. docs/legal stays absolute and is NOT releasable by the label: a label click is not evidence anyone read the clause. The pattern list is copied VERBATIM from claude-author-automerge.yml and a drift guard re-derives both and fails if they diverge — two gates that disagree about what is risky is how this reopens. Deliberately NOT narrowing the auth pattern away from test roots. That looked like the fix, but the coverage it would remove is arbitrary: matching is anchored on directory SEGMENTS, so auth/signup.spec.ts trips while auth.spec.ts, auth-flows.spec.ts and login-flow.spec.ts do not, and on #215 itself the Stripe payment spec sailed through while the auth spec tripped. Narrowing would trade a real hold for a coverage gap that is already the majority case. The gap is real but is a separate decision about who reviews auth-test edits; corpus entry added so a future "narrow the false positive" change has to confront it. Evidence: - #215's exact four files against origin/main's classify block => all_safe=1 (it auto-merged); against this branch => all_safe=0, reason=risk-tier-hold. - 17 new behavioral cases + drift guard; existing 17 override cases unchanged; 66 pytest selftests pass; BB corpus selftest 179 cases pass; shellcheck clean; both selftests pass under macOS bash 3.2.57. Co-Authored-By: Claude Opus 5 --- .github/workflows/safe-paths-automerge.yml | 164 ++++++++++++++++- selftest/risk_patterns_corpus.txt | 7 + selftest/test_safe_paths_risk_tier_hold.sh | 197 +++++++++++++++++++++ selftest/test_workflow_guards.py | 50 ++++++ 4 files changed, 415 insertions(+), 3 deletions(-) create mode 100755 selftest/test_safe_paths_risk_tier_hold.sh diff --git a/.github/workflows/safe-paths-automerge.yml b/.github/workflows/safe-paths-automerge.yml index da774ff..f9e54aa 100644 --- a/.github/workflows/safe-paths-automerge.yml +++ b/.github/workflows/safe-paths-automerge.yml @@ -36,6 +36,16 @@ on: required: false type: string default: "" + risk_bypass_label: + description: | + Label that releases the risk-tier hold below. Must match the + caller's claude-author-automerge `risk_bypass_label` — that + workflow's blocked-PR comment advertises this exact label as the + one-click bypass, so a mismatch would leave the advertised + escape hatch dead here. + required: false + type: string + default: "auto-merge-approved" secrets: automerge_pat: description: >- @@ -78,6 +88,7 @@ jobs: REPO: ${{ github.repository }} PR: ${{ github.event.pull_request.number }} EXTRA_GLOBS: ${{ inputs.extra_safe_globs }} + BYPASS_LABEL: ${{ inputs.risk_bypass_label }} run: | set -euo pipefail # Changed-file names via the paginated files API — `gh pr diff @@ -88,8 +99,8 @@ jobs: --paginate --jq '.[].filename') # NOTE the 3000-file truncation check lives AFTER the override # block below, not here. Both exit early with all_safe=0, but only - # the override also emits reason=unsafe-override, which is what - # triggers the revoke step. Returning on truncation first would + # the overrides also emit a revoke-triggering reason + # (unsafe-override / risk-tier-hold). Returning on truncation first would # skip the override entirely, so an already-armed PR updated to # 3000+ files including docs/legal/** would keep its arm and merge # the legal change. (Codex review round 8 P2.) Checking the @@ -120,8 +131,81 @@ jobs: # it decides on diff content alone — so a repo can list # docs/legal/** as `sensitive:` and still have it merged by this # workflow. Both gates are required; neither subsumes the other. + # + # TIER 1 — ABSOLUTE. Deliberately NOT released by the bypass + # label: a contractual clause needs the human who merges it to + # have read it, and a label click is not that. unsafe_overrides='^docs/legal/' + # TIER 2 — RISK-TIER HOLD (lesson 2026-08-10, below). + # + # Why this has to live in THIS workflow: claude-author-automerge + # cannot block a merge. It can only decline to ARM one. This + # workflow is the only one that arms auto-merge on a docs/tests + # diff and the only one that can revoke a standing arm — so a + # risk-tier decline over there is a NO-OP unless it is honored + # here. The two gates never disagreed in any resolvable sense: + # one abstained while the other acted. + # + # Incident, topcoder1/inbox_superpilot#215: a tests-only PR + # touching web/tests/e2e/auth/signup.spec.ts drew "Auto-merge + # blocked — risk-tier paths touched. Manual click-merge required" + # from claude-author-automerge, then merged here 12 minutes + # later. That comment is still on the merged PR. A gate that + # announces a block it cannot enforce is worse than no gate: it + # leaves an audit trail asserting a human approval that never + # happened. + # + # The premise the `*.spec.ts` safe glob rests on — "passing tests + # can't break runtime" — is true about RUNTIME and false about + # DETECTION. Deleting the assertion that catches a lockout is how + # the lockout ships in a LATER PR, laundered through a green + # suite. Same shape as the docs/legal case: a defect CI cannot + # catch, and the safe-by-glob argument does not reach it. + # + # Released by the caller's risk_bypass_label — the same one-click + # label claude-author-automerge's blocked-PR comment advertises. + # Holding without honoring that label would kill the advertised + # escape hatch. + # + # COPIED VERBATIM from the `patterns=` block in + # claude-author-automerge.yml. selftest/test_safe_paths_risk_tier_hold.sh + # re-derives both and fails on drift — edit both or neither. + risk_tier_overrides='^(.*/)?(auth|login|signin|signup|logout|session[s]?|oauth|oauth2|sso|jwt|mfa|totp|webauthn|passkey)(/|\.(py|go|ts|tsx|js|jsx)$|$) + ^(.*/)?secret[s]?(/|\.(py|go|ts|tsx|js|jsx)$|$) + ^(.*/)?\.env($|\..*) + ^(.*/)?keychain.* + ^(.*/)?credentials.* + ^(.*/)?migrations(/|$) + .*\.sql$ + ^(.*/)?(billing|payment[s]?|pricing|invoice[s]?|subscription[s]?|checkout|refund[s]?)(/|\.(py|go|ts|tsx|js|jsx)$|$) + ^docs/legal/.* + (^|/)naf(/|\.(py|go|ts|js)$) + (^|/)main\.go$ + (^|/)Dockerfile(\..*)?$ + ^docker-compose.*\.ya?ml$ + (^|/)docker/docker-compose.*\.ya?ml$ + ^\.github/workflows/.* + ^templates/ci-workflows/.* + ^\.github/actions?/.* + ^\.github/scripts/.* + (^|/)action\.ya?ml$ + ^\.github/dependabot\.ya?ml$ + ^\.github/risk-paths\.yml$ + ^\.github/CODEOWNERS$ + ^infra/iam/.* + ^infra/(deploy|terraform|pulumi|k8s|cloudformation|ansible|digitalocean|scanner-id)/.* + ^infra/nginx.* + ^infra/.*\.(service|slice|timer|tf|hcl|sh)$ + ^terraform/.* + ^pulumi/.* + ^k8s/.* + ^fly\.toml$ + ^deploy/.* + (^|/)deploy.*\.(sh|ya?ml)$ + (^|/)ml/predictor\.(py|go|ts|js)$ + (^|/)ml/retrain\.(py|go|ts|js)$' + # A rename reports ONLY its destination in `.filename`; the source # path lives in `.previous_filename`. Without this, moving # docs/legal/aup.md to docs/archive/aup.md — which removes the @@ -159,6 +243,7 @@ jobs: exit 0 fi + # Deferred from above: the listing endpoint caps at 3000 files. # Past that we can't prove every changed file is safe → defer # (same stance as the empty-diff branch below). @@ -219,6 +304,68 @@ jobs: echo "Unsafe files:" printf "%b" "$unsafe_files" else + # TIER 2 — RISK-TIER HOLD. Evaluated ONLY here, in the branch + # where every changed file is safe-by-glob and this workflow + # would otherwise arm. Placing it earlier (beside the tier-1 + # override) looked equivalent and is not: it would emit a + # revoke-triggering reason on PRs this workflow never arms — a + # dependabot bump of `.github/workflows/**` matches the risk + # patterns — and the revoke step would then disarm a SIBLING + # workflow's legitimate auto-merge. Scoping the hold to the + # would-arm branch keeps the blast radius exactly the target + # set: diffs that are 100% docs/tests AND touch a risk path. + # + # Scans `renamed_from` too, so moving an auth spec OUT of + # auth/ (destination safe-by-glob, source risk-tier) still + # holds — same rename-bypass reasoning as tier 1. + # + # One `grep -E -f` pass rather than the nested per-pattern loop + # used above: tier 1 is a single pattern, tier 2 is 34, and at + # the 3000-file cap the nested form would fork ~100k greps. + # Matching FILES is all the decision needs; the per-pattern + # attribution below runs only over the (few) hits, so the log + # stays as actionable as the sibling's blocked-PR comment. + risk_pat_file=$(mktemp) + printf '%s\n' "$risk_tier_overrides" \ + | sed -e 's/^[[:space:]]*//' -e '/^$/d' > "$risk_pat_file" + risk_hits=$(printf '%s\n%s\n' "$changed" "$renamed_from" \ + | sed '/^$/d' | grep -E -f "$risk_pat_file" || true) + rm -f "$risk_pat_file" + + if [ -n "$risk_hits" ]; then + annotated="" + while IFS= read -r f; do + [ -z "$f" ] && continue + while IFS= read -r p; do + p="${p#"${p%%[![:space:]]*}"}" + [ -z "$p" ] && continue + if echo "$f" | grep -Eq "$p"; then + annotated="$annotated$f (matched: $p)\n" + break + fi + done <<< "$risk_tier_overrides" + done <<< "$risk_hits" + + # Read labels LIVE, not from the event payload: the bypass is + # applied by a human AFTER the blocked-PR comment appears, so + # a payload captured at `opened`/`synchronize` never shows it. + # Fail CLOSED — an unreadable label list holds the PR, which + # matches this workflow's stance everywhere else. + bypass_label="${BYPASS_LABEL:-auto-merge-approved}" + pr_labels=$(gh api "repos/${REPO}/pulls/${PR}" --jq '.labels[].name' 2>/dev/null || echo "") + if printf '%s\n' "$pr_labels" | grep -Fxq "$bypass_label"; then + echo "::notice::Risk-tier paths present but '$bypass_label' is applied — hold released." + printf "%b" "$annotated" + else + echo "all_safe=0" >> "$GITHUB_OUTPUT" + echo "reason=risk-tier-hold" >> "$GITHUB_OUTPUT" + echo "::warning::Diff is docs/tests-only but touches risk-tier paths — holding for a human. Apply '$bypass_label' to release." + echo "Risk-tier hits:" + printf "%b" "$annotated" + exit 0 + fi + fi + echo "all_safe=1" >> "$GITHUB_OUTPUT" echo "All $(echo "$changed" | wc -l | tr -d ' ') changed files are in safe paths." fi @@ -243,9 +390,19 @@ jobs: # claude-author-automerge.yml also fails closed on truncation — so this # does not fight a workflow that would legitimately arm. # (Codex review round 10 P2.) + # `risk-tier-hold` joins the revoke set for the same reason as the + # override: GitHub PRESERVES an auto-merge request across pushes, so + # "PR touches only kb/*.spec.ts (armed here) → push adds + # auth/signup.spec.ts" would otherwise keep the arm and merge the + # auth-test change the hold exists to stop. Both are positive "this + # must not auto-merge" verdicts, unlike the other all_safe=0 branches + # which mean "defer to another workflow". Releasing the hold via the + # bypass label sets no reason at all, so an operator who applies the + # label never trips this step. (Lesson 2026-08-10, inbox_superpilot#215.) - name: Revoke auto-merge on an unsafe-override path if: >- steps.classify.outputs.reason == 'unsafe-override' || + steps.classify.outputs.reason == 'risk-tier-hold' || steps.classify.outputs.reason == 'file-list-truncated' env: # Match the enable step's token: an arm placed by the PAT is @@ -254,6 +411,7 @@ jobs: PR_URL: ${{ github.event.pull_request.html_url }} PR: ${{ github.event.pull_request.number }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} + CLASSIFY_REASON: ${{ steps.classify.outputs.reason }} run: | set -euo pipefail # Ownership guard, mirroring claude-author-automerge.yml: if the @@ -273,7 +431,7 @@ jobs: echo "Auto-merge already off — nothing to revoke." exit 0 fi - echo "::warning::Diff touches an unsafe-override path (customer-facing legal wording) while auto-merge is armed — revoking." + echo "::warning::Diff touches a path this workflow must not auto-merge (reason=${CLASSIFY_REASON:-unknown}) while auto-merge is armed — revoking." # Fail CLOSED: only a verified OFF is success. A swallowed # `--disable-auto` failure or an unreadable verify both leave the # PR possibly still armed, and treating that as success would let diff --git a/selftest/risk_patterns_corpus.txt b/selftest/risk_patterns_corpus.txt index 5e3335c..8f8017e 100644 --- a/selftest/risk_patterns_corpus.txt +++ b/selftest/risk_patterns_corpus.txt @@ -33,6 +33,13 @@ RISKY: services/mfa/verify.py RISKY: app/totp/generate.go RISKY: lib/webauthn/register.go RISKY: internal/passkey/store.go +# A spec file under a test root is STILL risky — the segment anchor makes no +# exception for tests/, and safe-paths-automerge.yml's tier-2 hold now depends +# on that. topcoder1/inbox_superpilot#215 (2026-08-10) merged a change to this +# exact path unattended; "narrowing the false positive" by anchoring the auth +# pattern away from test roots would silently reopen it. Change this line only +# with a deliberate decision about who reviews auth-test edits. +RISKY: web/tests/e2e/auth/signup.spec.ts # ── RISKY: Go entrypoints — wxa-mcp-server#193 gap ───────────────────────── RISKY: main.go diff --git a/selftest/test_safe_paths_risk_tier_hold.sh b/selftest/test_safe_paths_risk_tier_hold.sh new file mode 100755 index 0000000..65a34be --- /dev/null +++ b/selftest/test_safe_paths_risk_tier_hold.sh @@ -0,0 +1,197 @@ +#!/usr/bin/env bash +# Behavioral test for safe-paths-automerge.yml's TIER 2 risk-tier hold — +# the gate that stops a 100%-docs/tests diff from auto-merging when it +# touches a risk-tier path. +# +# Incident (topcoder1/inbox_superpilot#215, 2026-08-10): a tests-only PR +# touching web/tests/e2e/auth/signup.spec.ts drew "Auto-merge blocked — +# risk-tier paths touched. Manual click-merge required" from +# claude-author-automerge.yml, and then merged HERE 12 minutes later. The +# two workflows never disagreed in any resolvable sense: claude-author- +# automerge cannot block a merge, only decline to ARM one, and this +# workflow is the only one that arms a docs/tests diff. The decline was an +# abstention, not a veto — so the permissive gate won silently and the +# merged PR kept an audit trail claiming a human had approved it. +# +# Pins: +# 1. The incident shape holds (all_safe=0, reason=risk-tier-hold). +# 2. The bypass label RELEASES the hold — claude-author-automerge's +# blocked-PR comment advertises that exact one-click path, so a hold +# that ignored it would kill the advertised escape hatch. +# 3. docs/legal/** (tier 1) is NOT releasable by the label. +# 4. The hold fires ONLY in the would-arm branch. A diff carrying a +# non-safe file must keep reason empty — emitting a revoke-triggering +# reason there would make the revoke step disarm a SIBLING workflow's +# legitimate arm (a dependabot bump of .github/workflows/** matches +# the risk patterns and is armed by dependabot-auto-merge.yml). +# 5. Ordinary docs/tests still auto-merge — the carve-out this workflow +# exists to provide must not regress. +# 6. The tier-2 pattern list has not drifted from claude-author- +# automerge.yml's `patterns=` block. +# +# The classify block is EXTRACTED from the workflow YAML and executed, so +# this exercises the shipped bash rather than a mirrored copy that can drift. +# +# Run from the repo root: +# bash selftest/test_safe_paths_risk_tier_hold.sh +set -euo pipefail + +failed=0 +T=$(mktemp -d) +trap 'rm -rf "$T"' EXIT + +WF=.github/workflows/safe-paths-automerge.yml +SIB=.github/workflows/claude-author-automerge.yml + +awk ' + /^[[:space:]]*id: classify[[:space:]]*$/ { in_step = 1 } + in_step && /^[[:space:]]*run: \|[[:space:]]*$/ { grab = 1; next } + grab && /^ - name:/ { exit } + grab { print } +' "$WF" | sed -e 's/^ //' > "$T/classify.sh" + +if ! grep -q 'risk_tier_overrides=' "$T/classify.sh"; then + echo "FAIL: could not extract the classify block from $WF (no risk_tier_overrides found)" + exit 1 +fi + +# Stub `gh`: the shipped block makes THREE distinct calls — the changed-file +# listing (.filename), the rename sources (.previous_filename), and the live +# label read (.labels[].name). Discriminate on the --jq expression so each +# returns its own fixture; a stub returning one list for all three would make +# the bypass-label and rename cases pass vacuously. +mkdir -p "$T/bin" +cat > "$T/bin/gh" <<'STUB' +#!/usr/bin/env bash +for a in "$@"; do + case "$a" in + *previous_filename*) cat "$FAKE_RENAMES"; exit 0 ;; + *labels*) cat "$FAKE_LABELS"; exit 0 ;; + esac +done +cat "$FAKE_FILES" +STUB +chmod +x "$T/bin/gh" +export PATH="$T/bin:$PATH" + +export GH_TOKEN=stub REPO=owner/repo PR=1 EXTRA_GLOBS="" BYPASS_LABEL="auto-merge-approved" + +# Reset after every run_case so they never leak between cases. +RENAMED_FROM="" +LABELS="" + +# run_case ... +run_case() { + local name="$1" want_safe="$2" want_reason="$3" + shift 3 + printf '%s\n' "$@" > "$T/files.txt" + export FAKE_FILES="$T/files.txt" + printf '%s' "$RENAMED_FROM" > "$T/renames.txt" + export FAKE_RENAMES="$T/renames.txt" + printf '%s' "$LABELS" > "$T/labels.txt" + export FAKE_LABELS="$T/labels.txt" + RENAMED_FROM="" + LABELS="" + : > "$T/gh_output" + export GITHUB_OUTPUT="$T/gh_output" + + if ! bash "$T/classify.sh" > "$T/log" 2>&1; then + echo "FAIL[$name]: classify block exited non-zero" + sed 's/^/ /' "$T/log" + failed=1 + return + fi + + local got_safe got_reason + got_safe=$(grep -E '^all_safe=' "$T/gh_output" | tail -1 | cut -d= -f2) + got_reason=$(grep -E '^reason=' "$T/gh_output" | tail -1 | cut -d= -f2- || true) + [ -n "$got_reason" ] || got_reason="-" + + if [ "$got_safe" != "$want_safe" ] || \ + { [ "$want_reason" != "-" ] && [ "$got_reason" != "$want_reason" ]; }; then + echo "FAIL[$name]: all_safe=$got_safe reason=$got_reason, want $want_safe/$want_reason (files: $*)" + failed=1 + return + fi + echo "ok[$name] all_safe=$got_safe reason=$got_reason" +} + +# 1. The incident case, exactly as it merged. +run_case "incident-auth-spec" 0 risk-tier-hold "web/tests/e2e/auth/signup.spec.ts" +run_case "incident-pr215-full" 0 risk-tier-hold \ + "web/tests/e2e/auth/signup.spec.ts" \ + "web/tests/e2e/kb/knowledge-base.spec.ts" \ + "web/tests/e2e/marketing/landing.spec.ts" \ + "web/tests/e2e/stripe/stripe-integration.spec.ts" + +# Other risk directories under a test root hold the same way. +run_case "risk-session-dir" 0 risk-tier-hold "web/tests/e2e/session/expiry.spec.ts" +run_case "risk-billing-dir" 0 risk-tier-hold "web/tests/e2e/checkout/pay.spec.ts" +# A .sql fixture under tests/ is safe-by-glob but risk-tier by content. +run_case "risk-sql-fixture" 0 risk-tier-hold "tests/fixtures/seed.sql" + +# 2. The bypass label releases the hold. +LABELS="auto-merge-approved" +run_case "bypass-releases-hold" 1 - "web/tests/e2e/auth/signup.spec.ts" +# An unrelated label must NOT release it. +LABELS="dependencies" +run_case "unrelated-label-holds" 0 risk-tier-hold "web/tests/e2e/auth/signup.spec.ts" + +# 3. Tier 1 is absolute — the label does not release customer-facing legal +# wording. A label click is not evidence anyone read the clause. +LABELS="auto-merge-approved" +run_case "legal-not-bypassable" 0 unsafe-override "docs/legal/acceptable-use-policy.md" + +# 4. The hold is scoped to the would-arm branch. These carry a non-safe file, +# so this workflow no-ops — reason MUST stay empty or the revoke step +# would disarm whatever sibling legitimately armed the PR. +run_case "workflow-bump-no-reason" 0 - ".github/workflows/ci.yml" +run_case "workflow-bump-mixed-no-reason" 0 - ".github/workflows/ci.yml" "docs/changelog.md" +run_case "auth-source-no-reason" 0 - "src/auth/login.ts" + +# 5. Regression guard: the carve-out still works. +run_case "plain-docs" 1 - "docs/architecture.md" +run_case "plain-tests" 1 - "tests/test_a.py" "tests/test_b.py" +run_case "specs-no-risk-dir" 1 - \ + "web/tests/e2e/kb/knowledge-base.spec.ts" \ + "web/tests/e2e/marketing/landing.spec.ts" \ + "web/tests/e2e/stripe/stripe-integration.spec.ts" + +# 6. Rename bypass: relocating an auth spec OUT of auth/ reports only the +# destination in .filename. The destination is safe-by-glob, so without +# scanning previous_filename the move would auto-merge. +RENAMED_FROM="web/tests/e2e/auth/signup.spec.ts" +run_case "rename-auth-out" 0 risk-tier-hold "web/tests/e2e/misc/signup2.spec.ts" +# A rename with no risk path on either end stays safe. +RENAMED_FROM="web/tests/e2e/kb/old.spec.ts" +run_case "rename-benign" 1 - "web/tests/e2e/kb/new.spec.ts" + +# 7. DRIFT GUARD. The tier-2 list is a verbatim copy of the sibling gate's +# `patterns=` block. If they diverge, the two gates disagree about what +# is risky and this whole fix silently develops holes. +python3 - "$WF" "$SIB" <<'PY' || failed=1 +import re, sys +wf, sib = (open(p).read() for p in sys.argv[1:3]) +def block(text, name): + m = re.search(r"^ +%s='(.*?)'\n" % name, text, re.S | re.M) + if not m: + sys.exit("FAIL[drift-guard]: could not locate %s=' block" % name) + return [l.strip() for l in m.group(1).splitlines() if l.strip()] +a, b = block(wf, "risk_tier_overrides"), block(sib, "patterns") +if a != b: + only_wf = [p for p in a if p not in b] + only_sib = [p for p in b if p not in a] + print("FAIL[drift-guard]: tier-2 list has drifted from claude-author-automerge.yml") + for p in only_wf: + print(" only in safe-paths-automerge.yml: %s" % p) + for p in only_sib: + print(" only in claude-author-automerge.yml: %s" % p) + sys.exit(1) +print("ok[drift-guard] %d patterns identical in both gates" % len(a)) +PY + +if [ "$failed" -ne 0 ]; then + echo "FAILED" + exit 1 +fi +echo "All safe-paths risk-tier-hold cases passed." diff --git a/selftest/test_workflow_guards.py b/selftest/test_workflow_guards.py index 736b901..a7f9bf7 100644 --- a/selftest/test_workflow_guards.py +++ b/selftest/test_workflow_guards.py @@ -35,6 +35,7 @@ "selftest/test_prettier_scope_failsafe.sh", "selftest/test_prettier_symlink_filter.sh", "selftest/test_ruff_ruleset_warning.sh", + "selftest/test_safe_paths_risk_tier_hold.sh", "selftest/test_safe_paths_unsafe_overrides.sh", ], ) @@ -277,6 +278,55 @@ def test_safe_paths_never_automerges_customer_facing_legal(): ) +def test_safe_paths_honors_risk_tier_and_scopes_the_hold(): + """safe-paths must honor risk-tier paths, and only where it would arm. + + 2026-08-10, topcoder1/inbox_superpilot#215: claude-author-automerge + posted "Auto-merge blocked — risk-tier paths touched. Manual click-merge + required" for web/tests/e2e/auth/signup.spec.ts, and safe-paths merged + the PR 12 minutes later. The two never disagreed in a resolvable sense — + claude-author-automerge can only decline to ARM, never block, and + safe-paths is the only workflow that arms a docs/tests diff. So the + decline was an abstention and the permissive gate won silently. + + Behavior is pinned by selftest/test_safe_paths_risk_tier_hold.sh, + including a drift guard against the sibling's pattern list. This asserts + the wiring that test cannot see. + """ + text = (WORKFLOWS_DIR / "safe-paths-automerge.yml").read_text() + + assert "risk_tier_overrides='" in text, ( + "safe-paths-automerge.yml must carry the risk-tier pattern list — " + "without it a tests-only auth change auto-merges past the sibling's " + "decline (inbox_superpilot#215)" + ) + assert "steps.classify.outputs.reason == 'risk-tier-hold'" in text, ( + "the revoke must fire on risk-tier-hold too — GitHub preserves an " + "auto-merge arm across pushes, so a PR armed on a clean revision " + "keeps its arm when a later push adds an auth spec" + ) + assert "BYPASS_LABEL: ${{ inputs.risk_bypass_label }}" in text, ( + "the hold must honor the bypass label — claude-author-automerge's " + "blocked-PR comment advertises it as the one-click release, and a " + "hold that ignored it would leave that escape hatch dead" + ) + + # STRUCTURAL: the risk scan must sit AFTER the safe-glob verdict, in the + # branch where every changed file is safe and this workflow would arm. + # Hoisting it up beside the tier-1 override looks equivalent and is not: + # it would emit a revoke-triggering reason on PRs this workflow never + # arms — a dependabot bump of .github/workflows/** matches the risk + # patterns — and the revoke step would disarm dependabot-auto-merge's + # legitimate arm. + unsafe_branch = text.index('if [ -n "$unsafe_files" ]') + risk_scan = text.index("risk_hits=$(") + assert risk_scan > unsafe_branch, ( + "the risk-tier scan must run in the all-files-safe branch, after the " + "safe-glob check — running it earlier makes the revoke step disarm " + "sibling workflows on PRs safe-paths never arms" + ) + + def test_no_global_git_config_writes_in_workflows(): """No reusable may write to global git config.