From 4f8acbcaf2c3eb68e5f0f2fac114b3723e00cd97 Mon Sep 17 00:00:00 2001 From: Edward Cheng-I Wu <132531341+Imbad0202@users.noreply.github.com> Date: Thu, 18 Jun 2026 11:56:04 +0800 Subject: [PATCH] fix(#454): Windows Python hook portability + graceful no-Python degradation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PreToolUse write-scope guard was wired as a bare `python3 ".../ars_write_scope_guard.py"`. On Windows `python3` is commonly a 0-byte Microsoft Store alias stub (exit 49, no output), so the hook failed before the guard's own fail-safes ran and spammed the hook log every call. A new cross-platform launcher `hooks/run_guard.sh` (POSIX sh; `hooks.json` invokes it via bash) finds a real interpreter β€” `py -3` / `python3` / `python`, each verified by a marker probe that must exit 0 AND print the marker (a stub is skipped) β€” then runs the guard as a supervised, time-bounded subprocess. Plan A graceful degradation: if no real interpreter is found or the guard subprocess misbehaves, the launcher emits a valid pass-through hook JSON and exits 0, never blocking and staying silent on stderr on the degraded paths. Hardened across a two-model dual-track review (codex + gemini, both POSIX-reproduced): the no-`timeout` watchdog fallback's stdin/stdout/timeout handling, a pid-reuse race (done-file handshake), a predictable-/tmp symlink fail-open (mktemp failure degrades to pass-through), and a gameable CI exec assertion (line-anchored). 76 tests (21 launcher + 55 guard); README documents the Git Bash prerequisite on Windows. Verified against the reporter's confirmed environment (py -3 real, python3 an exit-49 stub): the stub is skipped, the guard runs via py -3, and the decision is forwarded. Closes #454. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) --- .gitattributes | 6 + .github/workflows/spec-consistency.yml | 47 +- CHANGELOG.md | 5 + README.md | 1 + ...-windows-python-hook-portability-design.md | 123 +++++ hooks/hooks.json | 2 +- hooks/run_guard.sh | 233 +++++++++ scripts/_ci_pytest_manifest.toml | 4 + scripts/test_run_guard_launcher.py | 467 ++++++++++++++++++ 9 files changed, 881 insertions(+), 7 deletions(-) create mode 100644 .gitattributes create mode 100644 docs/design/2026-06-17-454-windows-python-hook-portability-design.md create mode 100755 hooks/run_guard.sh create mode 100644 scripts/test_run_guard_launcher.py diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..ce94d2d3 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,6 @@ +# Shell scripts MUST stay LF, even on Windows checkouts. The PreToolUse hook launcher +# (hooks/run_guard.sh) and the SessionStart announce hook (scripts/announce-ars-loaded.sh) +# are executed via Git Bash on Windows; a CRLF checkout would break them on the hot path +# (#454). Scope the rule to *.sh so existing line-ending behavior for other files is +# untouched. +*.sh text eol=lf diff --git a/.github/workflows/spec-consistency.yml b/.github/workflows/spec-consistency.yml index 84360474..e4a2b937 100644 --- a/.github/workflows/spec-consistency.yml +++ b/.github/workflows/spec-consistency.yml @@ -402,9 +402,11 @@ jobs: - name: Validate ARS plugin hooks.json wires the write-scope guard # Pins hooks/hooks.json to (a) be valid JSON, (b) carry a PreToolUse entry whose - # matcher covers the structured write tools + Bash, and (c) invoke the guard - # script via ${CLAUDE_PLUGIN_ROOT}. Catches an accidental hook-config regression - # that would silently disable the guard. + # matcher covers the structured write tools + Bash, (c) invoke the cross-platform + # launcher hooks/run_guard.sh via ${CLAUDE_PLUGIN_ROOT}, AND (d) confirm the launcher + # actually execs the guard script (so the launcher->guard chain can't be silently + # severed by a future edit). The hook went via run_guard.sh in #454: a bare `python3` + # command broke on Windows where python3 is a 0-byte Microsoft Store alias stub. run: | python3 - <<'PY' import json @@ -416,9 +418,42 @@ jobs: for tool in ("Write", "Edit", "MultiEdit", "Bash"): assert tool in matcher, f"PreToolUse matcher missing {tool!r}: {matcher!r}" cmds = " ".join(hk.get("command", "") for hk in entry.get("hooks", [])) - assert "ars_write_scope_guard.py" in cmds, "PreToolUse does not invoke the guard script" - assert "${CLAUDE_PLUGIN_ROOT}" in cmds, "guard command must use ${CLAUDE_PLUGIN_ROOT}" - print("hooks.json PreToolUse write-scope guard wiring OK") + assert "hooks/run_guard.sh" in cmds, "PreToolUse does not invoke the run_guard.sh launcher" + assert "${CLAUDE_PLUGIN_ROOT}" in cmds, "hook command must use ${CLAUDE_PLUGIN_ROOT}" + # The launcher must still chain to the guard script β€” pin it so the chain stays intact. + # P2-f: do NOT use a bare substring (a COMMENT mentioning the guard would false-pass and + # the launcher->guard chain could be severed while CI stays green). Skip whole-line + # comments, then require non-comment EXECUTABLE shapes: (1) the guard path is ASSIGNED + # from the launcher's own location, and (2) that assigned path is EXEC'd in the guard + # call site's command-substitution. This is a fast smoke pin, not full semantic proof β€” + # the authoritative launcher->guard exec verification is scripts/test_run_guard_launcher.py + # (run via the CI pytest manifest), which actually runs the launcher and asserts a real + # `deny` is forwarded. So we keep the static check tight enough to catch an obvious + # severance (commenting out the exec line) without trying to out-parse the shell: + # the exec match is bound to the GUARD_OUT=$( ... run_bounded ... "$GUARD" ) call-site + # shape. The assignment line is ANCHORED at line start (^\s*) so an inline comment like + # `ls # GUARD_OUT=$(run_bounded "$GUARD")` can't satisfy it, and the run_bounded/"$GUARD" + # tokens are confined to a single command substitution (no `)` between them) so + # `GUARD_OUT=$( echo run_bounded "$GUARD" )` doesn't false-pass either (gemini r6 P2, + # tightening the codex r6 P2 fix). + import re + guard_assigned = guard_run = False + for raw in open("hooks/run_guard.sh"): + if raw.lstrip().startswith("#"): + continue # whole-line comment β€” never load-bearing + # (1) GUARD=...ars_write_scope_guard.py (assignment, derived from $0 dir) + if re.search(r'\bGUARD=.*ars_write_scope_guard\.py', raw): + guard_assigned = True + # (2) the guard call site, anchored at line start; run_bounded must be the command + # run inside the substitution (only a pipe `|` may precede it), and "$GUARD" its + # argument β€” all within one $( ... ) (the [^)]* forbids a closing paren between). + if re.search(r'^\s*GUARD_OUT=\$\([^)]*\|\s*run_bounded\b[^)]*"\$GUARD"', raw): + guard_run = True + assert guard_assigned, \ + "run_guard.sh has no non-comment GUARD=...ars_write_scope_guard.py assignment β€” chain severed" + assert guard_run, \ + 'run_guard.sh has no anchored GUARD_OUT=$(... | run_bounded ... "$GUARD") call site β€” launcher->guard exec chain severed' + print("hooks.json PreToolUse write-scope guard wiring OK (via run_guard.sh launcher)") PY - name: Run v3.9.4 temporal verification lint (#135) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eb90a5d..666677d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed + +- **Windows Python hook portability + graceful no-Python degradation (#454).** The `PreToolUse` write-scope guard was wired as a bare `python3 ".../ars_write_scope_guard.py"`. On Windows `python3` is commonly a 0-byte Microsoft Store App Execution Alias stub, so the hook errored before the guard's own fail-safes could run and spammed the hook log every call. A new cross-platform launcher `hooks/run_guard.sh` (POSIX sh; `hooks.json` now invokes it via `bash`) finds a REAL interpreter β€” `py -3` / `python3` / `python`, each verified by a marker probe that must exit 0 AND print the marker (a stub that prints then exits non-zero is rejected) β€” then runs the guard as a supervised, time-bounded subprocess. **Plan A graceful degradation** (the guard is optional v3.10 hardening; ARS core needs no Python): if no real interpreter is found OR the guard subprocess misbehaves (non-zero, timeout, empty, or non-JSON / missing-key output, validated by a real `json.load` not a substring grep), the launcher emits a valid pass-through hook JSON and exits 0 β€” it never exits non-zero and stays silent on stderr on these degraded paths (PreToolUse is a hot path; per-call stderr is the spam #454 is about). Healthy-guard stderr advisories are relayed. New `scripts/test_run_guard_launcher.py` (21 tests, run from a temp plugin layout so the guard is always resolved from the launcher's own `../scripts/` β€” no production env back door); the `hooks.json` CI assertion now requires a line-anchored non-comment guard assignment AND the `GUARD_OUT=$(... | run_bounded ... "$GUARD")` exec call site rather than a bare filename substring (a comment or an `echo`-wrapped decoy no longer false-passes). New `.gitattributes` pins `*.sh eol=lf`. README documents the Git Bash prerequisite (without it Claude Code falls back to PowerShell, which cannot run the `.sh` launcher, so the guard is inactive and the hook logs per call instead of no-opping quietly). + - **Real-use findings**: a two-model dual-track implementation review (codex + gemini, both POSIX-reproduced) hardened the launcher far beyond the original wiring fix, and the cross-model split was load-bearing β€” each model caught real bugs the other missed. Round 5 (codex) found the marker probe ignored exit status, the guard ran unbounded, and the JSON check was a substring grep. Round 6, once the tests exercised the REAL watchdog on a host with neither `timeout` nor `setsid`, found three fail-open bugs the back-door tests had masked: the no-`timeout` fallback fed the guard an EMPTY stdin (a real `deny` was silently lost β€” the guard was dead on any timeout-less host), an un-reapable orphan grandchild could wedge the `$(...)` capture, and the watchdog could false-report a timeout for a command that finished within the bound. The independent **gemini track** then refuted codex's first race fix (a successful `kill` does NOT prove the child is still alive β€” after `wait` reaps it the pid can be RECYCLED, so a blind kill could hit an innocent process and still false-flag a timeout) and added two fail-open findings codex missed: a predictable `/tmp` fallback when `mktemp` fails is a symlink-attack surface whose redirect failure reads as a broken guard, and the CI exec assertion was still gameable by an inline comment or an `echo`-wrapped call. Final state: stdin stashed on fd 3, stdout captured via a private temp file, timeout decided by a **done-file handshake** (the parent disarms the watchdog before reaping it; the watchdog kills/flags only while the done-file is absent β€” no pid-reuse race), `mktemp` failure degrades to pass-through instead of a guessable path, and the CI assertion is line-anchored. Orphan-grandchild leakage in the doubly-degraded no-`timeout`/no-`setsid` path, and a multi-megabyte payload held in a shell variable, are documented as accepted trade-offs (the real probe and guard spawn no grandchildren and ordinary hook payloads are small; the robust alternatives add temp-file lifecycle / symlink surface to a hot path). + ### Added - **Diff/patch revision mode β€” Slice B revision-mode adoption (#89 Item 7, spec #390, sub-issue #424).** The MVP ship-gate slice: `academic-paper` revision mode now runs **anchorize β†’ patch β†’ deterministic apply β†’ finalizer** instead of full re-emission. `draft_writer_agent` gains the `## Patch-Document Revision Emission (#390)` contract (patch document as a `phase6_*/revision_patch_round.json` sidecar β€” hashes copied from the block manifest, never computed; `[PATCH-ESCALATION-REQUIRED:]` pre-drafting escalation tag; retry-once; provisional Schema 8 items with mechanical fields left to the orchestrator). `pipeline_orchestrator_agent` gains `## Revision-Round Patch Sequencing (#390)` (five normative steps with a no-rewrite window between manifest generation and apply; two-layer escalation gate with the MANDATORY checkpoint wording; never auto-fallback to full re-emission; escalated rounds re-anchorize under a new ID generation and stamp `mode: full_reemission_escalated`; `preserved_ratio` surfaced next to the #389 round-trip count). Schema 8 `ResponseItem` gains optional `change_block_ids` (orchestrator-populated from the apply report, Β§3.5 role split). New protocol doc `academic-paper/references/revision_patch_protocol.md` (exact Mode B commands, exit codes, apply report as a required re-review input, marker lifecycle). Two recorded ship decisions land as a spec Β§0 amendment with cross-model concurrence: **`touched_ratio` threshold = 0.6** (now the apply-script CLI default, strict `>`, 1.0 disables) and the **`insert_after` heading-anchor exemption** (anchoring on a heading no longer flags when the inserted text carries no headings; heading-bearing text still flags). Β§10 open items closed the verified way: `formatter_agent` gains `## ARS Marker Stripping (#390)` (all marker kinds stripped from converted final outputs only AFTER marker-dependent gates; working drafts keep markers) and `word_count_conventions.md` gains the strip-``-before-count rule (first-party check found NEITHER rule previously existed β€” the spec's "expectation" had nothing to point at); max single-op `new_text` size folded into the existing triggers (no separate cap). New lint `scripts/check_390_revision_patch_discipline.py` (8 invariants: writer/orchestrator/SKILL/Schema 8/protocol-doc/marker-rules block-scoped literals, threshold value lock, spec-example schema validation) + 30 mutation tests, wired into `spec-consistency.yml` + the pytest manifest. diff --git a/README.md b/README.md index 165852f8..8d3a88a0 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,7 @@ The architecture doc supersedes the sprawling pipeline description that used to - [Claude Code](https://docs.claude.com/en/docs/claude-code/setup) (latest; plugin packaging requires recent versions) - `ANTHROPIC_API_KEY` exported, or set on first `claude` run - *Optional:* Pandoc for DOCX, tectonic + Source Han Serif TC for APA 7.0 PDF (Markdown output works without either) +- *Optional (real Python):* The core skills (research / write / review) need no Python β€” they are prompt-driven. A **real Python interpreter** is needed only for: the `PreToolUse` write-scope guard (optional subagent hardening β€” if no real Python is found it cleanly no-ops and the guard is simply inactive; core skills are unaffected), plus a few opt-in features that shell out to Python (revision-patch mode, the submission-package verifier, and the `/ars-cache-invalidate` / `/ars-mark-read` / `/ars-unmark-read` commands). On Windows, note that `python3` is often a non-functional Microsoft Store placeholder rather than real Python; install Python from python.org (or via `winget`) so the launcher can find a real interpreter. The guard launcher is a POSIX shell script and `hooks.json` invokes it through `bash`, so on Windows it needs **Git Bash** (bundled with Git for Windows). With Git Bash present, a missing real Python degrades cleanly (the guard no-ops, silently). Without Git Bash, Claude Code falls back to PowerShell, which cannot run the `.sh` launcher at all: the guard is inactive and the `PreToolUse` hook will log an error per call rather than no-op quietly (accepted degradation β€” the guard is optional and never blocks your writes, but the hook noise is the trade-off until Git Bash is installed). **Plugin install (v3.7.0+, recommended):** diff --git a/docs/design/2026-06-17-454-windows-python-hook-portability-design.md b/docs/design/2026-06-17-454-windows-python-hook-portability-design.md new file mode 100644 index 00000000..a0b34ca3 --- /dev/null +++ b/docs/design/2026-06-17-454-windows-python-hook-portability-design.md @@ -0,0 +1,123 @@ +# #454 β€” Windows Python hook portability + graceful no-Python degradation + +**Status**: design (awaiting user review) +**Issue**: #454 (`ncwuguo`) β€” `ars_write_scope_guard.py` crashes with exit 49, empty stderr, on Windows alongside RTK +**Branch**: `fix/454-windows-python-hook-portability` +**Date**: 2026-06-17 + +## 1. Problem + +The PreToolUse write-scope guard hook is registered as: + +```json +{ "type": "command", "command": "python3 \"${CLAUDE_PLUGIN_ROOT}/scripts/ars_write_scope_guard.py\"" } +``` + +On the reporter's Windows machine every Bash/Write/Edit tool call makes this hook fail (exitCode 49, empty stderr, log spam). The guard's own Python only ever `return 0`s, so the failure is at the **interpreter-launch layer, before Python runs**: `python3` on Windows is commonly a 0-byte Microsoft Store App Execution Alias stub, not real Python. + +### Empirically established (first-party, Windows 11 VM) +- On this Windows class, `python3`/`python`/`py` are 0-byte Store alias stubs under `…\AppData\Local\Microsoft\WindowsApps\`, not real Python. (CONFIRMED on VM.) +- The exact "exit 49 + empty stderr" signature was NOT reproduced (`prlctl exec` runs as SYSTEM, not the interactive user; the alias's real behavior only fires in the user's interactive session). The exact emitter of 49 remains UNCONFIRMED β€” but the root-cause CLASS (hook hardcodes a `python3` that is a non-functional alias) is established. + +### Ground truth (official Claude Code hook docs, verified this session) +- Shell-form hook runs via `sh -c` (macOS/Linux), Git Bash (Windows), or **PowerShell when Git Bash isn't installed**. +- `${CLAUDE_PLUGIN_ROOT}` is substituted by Claude Code itself before the shell β€” the unexpanded value in the user's log is display text, not the executed command. (So "variable didn't expand" is NOT the bug.) +- Exit codes: `0` = no block (normal permission flow); `2` = blocks the tool; **any other non-zero (1, 49…) = NON-BLOCKING error β€” the action proceeds**, first stderr line shows as a hook-error notice, full stderr to debug log. +- Hooks for one event run **in parallel**; one hook's failure does NOT short-circuit siblings. (So ARS's failing hook does NOT serially "break" RTK β€” that earlier claim was wrong; RTK's hook runs regardless. ARS's only real harm is its own log spam.) +- No per-OS conditional command field. Exec form (`args` array) bypasses the shell but does NOT fix a wrong interpreter. + +## 2. The deciding fact: ARS core does not require Python + +Verified by repo inspection + independent codex/gemini fact-check: +- **Core skill use (research / write / review) requires NO Python.** README prerequisites list only Claude Code + API key; there is a `requirements-dev.txt` but no `requirements.txt`. SKILL.md/agent files are prompt/markdown that Claude reads. +- The guard hook is the **sole auto-running Python at user runtime**, and is an **optional security hardening layer** added in v3.10 (#134), not a core feature. +- **Nuance (codex catch β€” must stay honest):** ARS is not 100% prompt-only. These *opt-in / advanced* features DO execute real Python when the user invokes them: + - revision mode: `scripts/ars_anchorize_draft.py`, `scripts/ars_apply_revision_patch.py` + - pipeline submission verifier: `scripts/verify_submission_package.py` + - slash commands: `/ars-cache-invalidate`, `/ars-mark-read`, `/ars-unmark-read` + These are user-triggered (not auto-running hooks), so they fail visibly and the user can choose to install Python. They are **out of scope for this fix** (tracked as follow-up, Β§6). + +### Consequence β†’ Plan A (graceful degradation) +Forcing a Python install on a user whose ARS usage never needed Python (Plan B: fail-closed / exit 2) would turn an optional hardening layer into a global prerequisite, contradicting the setup docs and ARS's established "don't assume the environment has X; degrade if it's missing" principle (cf. #413 symlinkβ†’materialized copies for Windows). Both codex and gemini independently endorsed Plan A. + +## 3. Design + +### 3.1 A launcher that finds real Python (the fix for the bug body) +Add `hooks/run_guard.sh` (POSIX sh, Bash 3.2 compatible, same style/shape as the existing `scripts/announce-ars-loaded.sh`). Responsibilities: + +1. **Resolve the plugin root from the launcher's OWN path, not `${CLAUDE_PLUGIN_ROOT}` (codex P1).** CC substitutes `${CLAUDE_PLUGIN_ROOT}` into the hook *command text* before the shell, but that does NOT guarantee the variable is exported into the launcher's environment. So the launcher computes the guard path relative to itself: `SELF_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"; GUARD="$SELF_DIR/../scripts/ars_write_scope_guard.py"`. This must work when `CLAUDE_PLUGIN_ROOT` is unset AND when the plugin path contains spaces (both are test cases, Β§3.5). +2. Detect a REAL Python interpreter by trying candidates in order. **Each candidate is a command + fixed args, NOT a single quoted string (codex P1):** `py -3` (command `py`, arg `-3`), then `python3`, then `python`. Implement as parallel positional sets, e.g. iterate over `"py -3" "python3" "python"` and split into `cmd`/`rest` via `set -- $candidate` so `py` is invoked with `-3` as a real argument β€” never as an executable literally named `py -3`. +3. For each candidate, VERIFY it actually executes by running a marker probe β€” ` -c "import sys; print('ARS_PY_OK')"` β€” and requiring **both exit 0 AND the exact marker `ARS_PY_OK` on stdout**. A 0-byte Store stub fails to execute / prints nothing β†’ skipped. Probe stdout/stderr suppressed except the marker check. Each probe wrapped in `timeout` when available (see Β§3.3 for the no-`timeout` watchdog). +4. First verified interpreter runs the guard **as a supervised subprocess, NOT a bare `exec` (codex/gemini P1 β€” see Β§3.2.1):** capture the guard's stdout and exit code; decide what to emit based on whether the guard produced a clean result. Stdin is forwarded to the guard. + +### 3.2 No-Python posture (Plan A) +If NO candidate verifies (no real Python on the machine): the launcher emits a valid pass-through hook JSON `{"hookSpecificOutput":{"hookEventName":"PreToolUse"}}` and **exits 0**, and is SILENT on stderr (see "stderr discipline" below). + +Rationale (grounded in Β§1 ground truth + Β§2 fact): +- A non-zero (non-2) exit blocks nothing anyway (GT) β€” "fail closed via nonzero" is an illusion that only spams logs. +- Exit 2 (true block) would hard-lock a user out of all inspected writes/Bash for an environment gap β€” wrong for an optional hardening layer on a Python-free core (Β§2). +- So pass-through + exit 0: no spam, no block. + +**stderr discipline (resolves codex P2 β€” the earlier "stderr advisory + no spam" was self-contradictory):** PreToolUse is a hot path firing on every inspected tool call. Any stderr the launcher writes on the no-Python / guard-broke paths would, if CC surfaces or logs it, repeat on EVERY call β€” i.e. exactly the log spam #454 is about. Therefore the launcher writes **nothing to stderr on these degraded paths**; it stays silent and exits 0. The user-facing surface for "guard not active" is the docs note (Β§3.4), NOT a per-call stderr line. (A future once-per-session SessionStart advisory could surface it without hot-path spam β€” noted as optional follow-up Β§6, not built here.) + +### 3.2.1 Found-Python-but-guard-broke posture (decision: do NOT block) +Distinct from "no Python" (an environment gap, not ARS's fault): here a real Python WAS found but the guard subprocess does not return a clean result β€” it crashes (non-zero exit), the guard script is missing, or it emits invalid/empty stdout instead of the expected hook JSON. This is an ARS-side defect (a broken guard), so: + +- **The launcher emits valid pass-through JSON + exits 0 β€” it does NOT block (exit 2).** Maintainer decision: an ARS bug in our own hardening layer must not hard-lock the user out of their writes/Bash. Same degradation philosophy as no-Python: when the optional guard cannot produce a trustworthy decision, fall through to the normal permission flow rather than wedging the session. +- Two sub-cases the launcher must distinguish from a real decision: + - guard exits 0 with valid hook JSON on stdout (deny OR pass-through) β†’ forward that stdout verbatim, exit 0. (Normal path; note the guard ALWAYS exits 0 and signals deny via `permissionDecision:"deny"` in JSON β€” see Β§3.5 test-language fix.) + - guard exits non-zero, or stdout is empty / not valid JSON / lacks `hookSpecificOutput` β†’ treat as broken: emit the canonical pass-through JSON, exit 0, stay silent on stderr (per Β§3.2 discipline). +- Note the existing guard already self-degrades cleanly for an unreadable manifest (`ars_write_scope_guard.py:472` emits pass-through + exit 0), so that case never reaches the "broken" branch. The launcher's broken-branch handles the cases the guard itself can't (syntax error, missing file, crash before render). +- Honest cost: a genuinely broken guard is silently inactive (no per-call surface, per Β§3.2). This is the accepted trade-off of the maintainer's "don't block on our own bug" decision; the docs note (Β§3.4) plus CI lints (which would catch a guard syntax error pre-ship) are the compensating controls. + +### 3.3 Bash 3.2 / cross-platform constraints +- POSIX sh only; no bashisms requiring bash 4+. Mirrors `announce-ars-loaded.sh` (which README notes runs on macOS stock bash with no `brew install bash`). +- **Probe timeout / no-`timeout` watchdog (codex P2).** Each marker probe runs under `timeout s` when a `timeout` binary is present. When `timeout` is ABSENT, the launcher must NOT run the probe unbounded (a broken-but-non-stub interpreter that hangs would then hang EVERY PreToolUse call). Fallback watchdog: run the probe in the background, sleep a short bound, and kill the probe if still alive (`probe & pid=$!; ( sleep ; kill "$pid" 2>/dev/null ) & ...; wait "$pid"`), in POSIX-sh-portable form. If even that proves unreliable on a target shell, the launcher caps total candidates tried and bounds its own wall-clock; it must never hang the hot path. Tests cover both with- and without-`timeout` hosts AND a hanging candidate (Β§3.5). +- The single `.sh` launcher covers macOS, Linux, and Windows-with-Git-Bash. **On Windows WITHOUT Git Bash, this is NOT a clean degradation (codex P1 correction).** CC falls back to PowerShell, which runs the hook command `bash ".../run_guard.sh"`; if `bash` is absent from PATH, PowerShell errors, and per GT every inspected tool call becomes a NON-BLOCKING hook error β€” i.e. the guard is inactive AND the session is hook-error noisy (a different, milder spam than #454, but still noise). We ACCEPT this degradation for now (guard is optional hardening) but the spec states it honestly rather than calling it clean. Mitigations considered and deferred: a `.ps1` twin, a compiled binary (both rejected as disproportionate, Β§5); documenting Git Bash as the Windows prerequisite for the guard to be active (Β§3.4 docs note + Β§6 follow-up to quiet the PowerShell-no-bash noise). + +### 3.4 hooks.json + docs +- `hooks.json` PreToolUse command changes from `python3 "…ars_write_scope_guard.py"` to `bash "${CLAUDE_PLUGIN_ROOT}/hooks/run_guard.sh"` (same shape as the announce hook). +- README / docs/SETUP.md gain a short note: the write-scope guard needs a real Python interpreter to be active; if none is found it cleanly no-ops and core (Python-free) skills are unaffected. Plus the honest list (Β§2 nuance): revision / submission-verify / those 3 slash commands need real Python. + +### 3.5 Tests (close the codex-flagged blind spot) +Existing tests invoke the guard via `[sys.executable, guard_path]` β€” they never exercise interpreter resolution. Add launch-layer tests for `run_guard.sh` (and register the new test file in `scripts/_ci_pytest_manifest.toml`, see Β§3.6): + +**Terminology fix (gemini/codex):** the guard ALWAYS exits 0 and signals a denial via `"permissionDecision":"deny"` in its stdout JSON (`render_hook_output`), never via a non-zero exit. All tests below assert on the forwarded JSON payload, not on exit codes-as-decision. + +- **No real Python on PATH** β€” temp PATH dir holding only non-executing `python3`/`python`/`py` stubs (0-byte files or scripts printing nothing) so the marker probe fails for every candidate β†’ launcher emits canonical pass-through JSON + exit 0, **silent on stderr** (Β§3.2). Run with AND without a `timeout` binary on PATH. +- **Real Python present, in-scope write** β†’ launcher forwards the guard's pass-through JSON (no `permissionDecision`). +- **Real Python present, out-of-scope Bucket A write** β†’ launcher forwards the guard's `permissionDecision:"deny"` JSON verbatim. +- **Stub-then-real ordering** (first candidate a stub, a later one real) β†’ skips the stub, uses the real one, decision forwarded. +- **`py -3` arg handling** β†’ assert the launcher invokes `py` with `-3` as an argument (e.g. a fake `py` on PATH that records its argv and only succeeds when given `-3`), not a command literally named `py -3`. +- **`CLAUDE_PLUGIN_ROOT` unset** β†’ launcher still resolves the guard from its own path and works (covers codex P1). +- **Plugin path containing spaces** β†’ launcher resolves and runs correctly (no word-splitting break). +- **Found-Python-but-guard-broke (Β§3.2.1)** β†’ point the launcher at a guard that (a) exits non-zero, (b) prints nothing, (c) prints invalid JSON β†’ in all three the launcher emits canonical pass-through JSON + exit 0 (does NOT block, does NOT spam). This is the gemini/codex P1 fail-open-on-crash case, resolved per the maintainer's "don't block on our own bug" decision. +- **Hanging candidate** β†’ a fake interpreter that sleeps longer than the probe bound β†’ launcher kills it and moves on within the bound (no hot-path hang), on both with/without-`timeout` hosts. +- **Infra self-protection** β†’ a Bucket A subagent payload writing to `hooks/run_guard.sh` is denied (confirms `hooks/*.sh` glob covers the new file end-to-end through the launcher). +- (POSIX host simulation only; an actual Windows repro is still needed to confirm the Store-alias path, `py.exe -3` under Git Bash, Git Bash path conversion, CRLF, and the no-Git-Bash PowerShell fallback β€” noted as a known test-environment limit, not blocking ship but tracked Β§6.) + +### 3.6 Seams to update (cross-file, dual-track flagged β€” all verified against the actual files) +- **CI hook-wiring assertion**: `.github/workflows/spec-consistency.yml` (~line 419, verified) asserts `"ars_write_scope_guard.py" in cmds` directly in hooks.json's PreToolUse command β€” this WILL break. Update it to assert the PreToolUse command now wires `run_guard.sh`, AND add an assertion that `hooks/run_guard.sh` references/execs `ars_write_scope_guard.py` (so the launcherβ†’guard chain stays pinned and a future edit can't silently sever it). +- **CI pytest manifest (codex β€” newly surfaced, easy to miss)**: new test files are only run if listed in `scripts/_ci_pytest_manifest.toml` (the manifest runner at workflow line ~68; `check_ci_pytest_manifest.py` only validates listed entries). Add the new launch-layer test file to the manifest alongside the existing guard test entry (~line 149) β€” otherwise the new tests silently don't run in CI. +- **Infra self-protection**: `INFRA_PROTECTED_GLOBS` in `ars_write_scope_guard.py` (line 66, verified) already includes `hooks/*.sh` and matching is segment-aware (line 181), so `hooks/run_guard.sh` is auto-protected β€” no list change needed. Add the Β§3.5 test asserting a subagent write to it is denied (lock it in). +- **`.gitattributes` (codex β€” note it's a NEW file, not a confirmation)**: the repo currently has NO `.gitattributes`. Add one declaring `*.sh text eol=lf` (and the new launcher specifically) so a Windows CRLF checkout can't break the hot-path hook. Since it's new, confirm it doesn't disturb existing line-ending assumptions for other tracked files (scope the rule to `*.sh` to be safe). +- **Executable bit**: `run_guard.sh` committed with `+x` (matches `announce-ars-loaded.sh`). +- **Version/changelog discipline**: this repo enforces version-consistency lints. A hook-wiring + new-file change of this size should land a CHANGELOG entry and any version bump the repo's release discipline requires; check `check_version_consistency` / changelog lints before PR (per repo convention, not a guess about which version). + +## 4. What this does NOT change +- The guard's `evaluate_decision` logic is untouched (it was never the bug). +- No per-OS branching (impossible per GT), no `.ps1` twin, no compiled binary. +- No change to the SessionStart announce hook (it's `bash` too; same no-Git-Bash caveat, but it's a context-injection nicety, not a security control β€” tracked as follow-up if desired, Β§6). + +## 5. Rejected alternatives +- **exec form + hardcoded `node`/`python`**: collapses to the same bug class (a hardcoded interpreter that may be absent; `node` is NOT guaranteed on PATH for native-binary CC installs β€” codex verified against setup docs). +- **`.sh` + `.ps1` twin (universal launcher)**: solves no-Git-Bash Windows, but disproportionate maintenance shape for an OPTIONAL hardening layer; under Plan A, no-Git-Bash Windows degrading to "guard inactive" is acceptable. +- **Compiled native binary (Go/Rust)**: truly zero-dependency, but saddles a prompt/Python repo with a permanent cross-platform build+sign toolchain. Both reviewers called it overkill. +- **Plan B (fail-closed exit 2 for Bucket A when no Python)**: rejected per Β§2 β€” turns optional hardening into a global Python prerequisite on a Python-free core. +- **Fail-closed (exit 2) when Python IS found but the guard crashes**: considered (gemini argued for it: a present-but-broken security control should hard-block). Rejected by maintainer decision (Β§3.2.1): an ARS-side bug in our own optional hardening layer must not lock the user out of their writes/Bash. The guard-broke path degrades the same way as no-Python (pass-through + exit 0). Compensating controls: pre-ship CI lints catch a broken guard; the Β§3.4 docs note sets expectations. This is a deliberate availability-over-strictness call for THIS layer, not a general posture. + +## 6. Follow-up (separate issues, not this PR) +- Harden the OTHER hardcoded-`python`/`python3` user-runtime call sites (revision scripts, submission verifier, the 3 slash commands) the same way, or document the Python requirement at those touch points. +- SessionStart announce hook: same no-Git-Bash-Windows caveat (cosmetic, non-security). +- CI Windows runner to actually exercise the Store-alias path (currently no Windows CI). +- Reply to codex repo #31 (`ncyunju`, Windows symlink/skill-registration) β€” same "ARS assumes a Windows capability" family. diff --git a/hooks/hooks.json b/hooks/hooks.json index a60cb315..6a5a3165 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -16,7 +16,7 @@ "hooks": [ { "type": "command", - "command": "python3 \"${CLAUDE_PLUGIN_ROOT}/scripts/ars_write_scope_guard.py\"" + "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/run_guard.sh\"" } ] } diff --git a/hooks/run_guard.sh b/hooks/run_guard.sh new file mode 100755 index 00000000..5abce3ef --- /dev/null +++ b/hooks/run_guard.sh @@ -0,0 +1,233 @@ +#!/bin/sh +# version: 1.0.0 +# +# ARS write-scope guard LAUNCHER β€” PreToolUse hook (#454 Windows portability fix). +# +# WHY THIS EXISTS: the guard hook used to be wired as `python3 ".../ars_write_scope_guard.py"` +# directly. On Windows, `python3` is commonly a 0-byte Microsoft Store App Execution Alias +# stub (not real Python); invoking it non-interactively fails BEFORE the guard's Python runs, +# so none of the guard's own fail-safes apply β€” it just errors and spams the hook log (#454). +# +# This launcher finds a REAL Python (skipping stubs), then runs the guard as a SUPERVISED +# subprocess. Design: docs/design/2026-06-17-454-windows-python-hook-portability-design.md. +# +# POSTURE (Plan A β€” graceful degradation; the guard is OPTIONAL v3.10 hardening and ARS core +# needs no Python): if no real Python is found, OR the guard subprocess misbehaves, the +# launcher emits a valid PASS-THROUGH hook JSON and exits 0. It NEVER exits non-zero on these +# degraded paths (a non-2 exit blocks nothing anyway and only spams logs; exit 2 would +# hard-lock the user out of all writes/Bash for an environment gap or an ARS-side bug β€” wrong +# for an optional layer). It stays SILENT on stderr on degraded paths: PreToolUse is a hot +# path, so any per-call stderr IS the spam #454 is about. +# +# Bash 3.2 / POSIX sh compatible (same constraint as scripts/announce-ars-loaded.sh). On +# Windows this runs under Git Bash; with no Git Bash, CC falls back to PowerShell which can't +# run this .sh β€” the guard is then inactive (accepted degradation, see spec Β§3.3). + +# Canonical pass-through output: no permissionDecision => falls back to the normal permission +# flow (NEVER emit "allow" β€” that would skip every other permission rule). +PASS_THROUGH='{"hookSpecificOutput":{"hookEventName":"PreToolUse"}}' + +emit_passthrough_and_exit() { + printf '%s\n' "$PASS_THROUGH" + exit 0 +} + +# --- Resolve the guard script from THIS launcher's own location (codex P1) --------------- +# CC substitutes ${CLAUDE_PLUGIN_ROOT} into the hook COMMAND text before the shell, but does +# NOT guarantee it as an env var inside this script. So compute the guard path from $0. +# (No production env override: the guard path is ALWAYS derived from the launcher's own +# location. Tests that need a broken/alternate guard run the launcher from a temp plugin +# layout, so there is no production back door β€” P2-e.) +# shellcheck disable=SC1007 # `CDPATH= cd` is intentional: clear CDPATH for this one cd only +SELF_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) || emit_passthrough_and_exit +GUARD="$SELF_DIR/../scripts/ars_write_scope_guard.py" + +# --- Read the payload from stdin ONCE (we must replay it to the guard subprocess) --------- +# Known trade-off (gemini round-6 P2, accepted): the payload is held in a shell variable and +# replayed with `printf '%s'`. `printf` is a builtin so it is NOT bound by ARG_MAX, and the +# stripped trailing newline is irrelevant to the guard's JSON parse, so for ordinary hook +# payloads this is safe. A multi-megabyte Write payload on a POSIX shell that caps variable +# length is the narrow case this does not cover; buffering to a private temp file would handle +# it but adds another temp-file lifecycle (and symlink surface) to a hot path, so it is left as +# documented degradation rather than fixed speculatively. +PAYLOAD=$(cat) + +# --- Marker probe: does this candidate run real Python? ---------------------------------- +# A candidate is "real" iff the probe exits 0 AND prints the exact marker on stdout. A 0-byte +# Store stub fails to execute / prints nothing, so it is skipped. We bound each probe so a +# broken-but-hanging interpreter can't wedge the hot path (spec Β§3.3): prefer `timeout` when +# present, else a portable process-group watchdog. +MARKER=ARS_PY_OK +# Per-candidate (and guard) wall-clock bound, seconds. A small ops knob; validated to be a +# bare integer so it can't smuggle anything into the `timeout`/`sleep` args. Default 3. +PROBE_BOUND=${ARS_PROBE_BOUND:-3} +case "$PROBE_BOUND" in + ''|*[!0-9]*) PROBE_BOUND=3 ;; +esac + +# ARS_GUARD_FORCE_WATCHDOG=1 forces the no-`timeout` watchdog path even on hosts that HAVE +# `timeout`. This is a test/debug switch: it only changes the BOUNDING MECHANISM (timeout +# binary vs process-group watchdog), never the security decision β€” both paths enforce the +# same wall-clock bound and the same pass-through-on-overrun posture. Honored if set; safe +# to leave unset (the default prefers the `timeout` binary when present). +have_timeout() { + [ -z "${ARS_GUARD_FORCE_WATCHDOG:-}" ] && command -v timeout >/dev/null 2>&1 +} + +# Reserved exit status meaning "the bounded command was killed for overrunning its bound". +TIMEOUT_STATUS=124 + +# Run "$@" with a wall-clock bound; its stdout flows to OUR stdout (capture via $(...)). +# Returns the command's real exit status, or $TIMEOUT_STATUS if it overran the bound. +# Stdin for "$@" is whatever the caller arranges (we redirect it per call site) β€” the guard +# call site PIPES the payload in, so the bounded command MUST keep that stdin. +run_bounded() { + if have_timeout; then + # GNU timeout exits 124 on timeout β€” normalize to our sentinel for a uniform caller. + timeout "${PROBE_BOUND}s" "$@" + _st=$? + [ "$_st" -eq 124 ] && return "$TIMEOUT_STATUS" + return "$_st" + fi + # No `timeout` binary: a portable background-and-watchdog fallback. Several POSIX subtleties + # bite here, all of which broke a naive version (#454 dual-track): + # + # 1. STDIN: a backgrounded (`&`) command's stdin defaults to /dev/null, which would feed + # the GUARD an EMPTY payload (guard reads nothing -> pass-through -> a real `deny` is + # LOST and the guard is dead on any timeout-less host). We stash this function's stdin + # on fd 3 and redirect the job's stdin from it (`<&3`) so the piped payload arrives. + # 2. STDOUT: capturing the job's stdout through the `$(...)` pipe directly would WEDGE the + # pipe β€” if the job spawns a grandchild we can't reap (no `setsid` to group-kill), that + # orphan keeps the pipe's write end open and `$(...)` blocks until it dies (the whole + # point of the bound). So the job writes stdout to a temp file; we `cat` it back after + # reaping the direct child. The orphan may linger but can no longer wedge the hot path. + # 3. KILL: with `setsid` we new-pgid the job and signal the whole group (negative pid) so + # TERM-ignoring children die too; without it we can only reach the direct pid, so a + # grandchild a hung interpreter spawned can be ORPHANED (it no longer wedges the hot + # path per (2), but it lingers). There is no fully portable POSIX reap without process + # groups / setsid / job-control, and inventing one would add fragility for a doubly- + # degraded case (no `timeout` AND no `setsid` AND a broken interpreter that forks β€” the + # real marker probe and guard never spawn grandchildren). Accepted; prefer `timeout` + # (path A) and `setsid` where present (codex round-6 P2). + # 4. TIMEOUT DETECTION via a DONE-FILE handshake, not the exit code (a TERM'd `sh` wrapper + # does not reliably surface 143/137) and not "did the kill succeed" (gemini round-6 P1 + # refuted that: after `wait` reaps the child its pid is freed, and if the OS recycles it + # before the parent disarms the watchdog, a blind `kill` would land on an INNOCENT + # unrelated process β€” succeeding, so it would both false-flag a timeout AND signal the + # wrong pid). Instead the parent writes a done-file the instant `wait` returns; the + # watchdog kills (and flags) ONLY if that file is still absent when its sleep elapses. + # So "child finished in time" and "child overran" are decided by the handshake, never by + # racing a pid. mktemp failure must NOT fall back to a predictable /tmp path: a local + # attacker could pre-create it as a symlink, our redirect would fail, and the launcher + # would treat that as a broken guard and fail OPEN (gemini round-6 P1). If we can't get a + # private temp, degrade safely to pass-through instead of writing a guessable path. + _rb_out=$(mktemp 2>/dev/null) || emit_passthrough_and_exit + _rb_fired=$(mktemp 2>/dev/null) || { rm -f "$_rb_out" 2>/dev/null; emit_passthrough_and_exit; } + _rb_done=$(mktemp 2>/dev/null) || { rm -f "$_rb_out" "$_rb_fired" 2>/dev/null; emit_passthrough_and_exit; } + rm -f "$_rb_fired" 2>/dev/null # absent = watchdog has not fired + rm -f "$_rb_done" 2>/dev/null # absent = parent has not yet signalled completion + if command -v setsid >/dev/null 2>&1; then + { setsid "$@" >"$_rb_out" <&3 3<&- & } 3<&0 + else + { "$@" >"$_rb_out" <&3 3<&- & } 3<&0 + fi + _cmd_pid=$! + ( sleep "$PROBE_BOUND" + # If the parent already signalled completion, the child finished within the bound: do NOT + # kill (its pid may have been recycled to an innocent process) and do NOT flag a timeout. + if [ ! -f "$_rb_done" ]; then + # Real overrun: kill the whole process group (negative pid) when we can, else the bare + # pid, and record the timeout. Re-check the done-file before the hard KILL too. + kill -TERM "-$_cmd_pid" 2>/dev/null || kill -TERM "$_cmd_pid" 2>/dev/null + : >"$_rb_fired" + sleep 1 + if [ ! -f "$_rb_done" ]; then + kill -KILL "-$_cmd_pid" 2>/dev/null || kill -KILL "$_cmd_pid" 2>/dev/null + fi + fi + ) & + _watch_pid=$! + wait "$_cmd_pid" 2>/dev/null + _st=$? + : >"$_rb_done" # DISARM the watchdog before reaping it β€” closes the pid-reuse race window + kill "$_watch_pid" 2>/dev/null + wait "$_watch_pid" 2>/dev/null + cat "$_rb_out" 2>/dev/null # replay captured stdout to OUR stdout (for the $(...) caller) + if [ -f "$_rb_fired" ]; then + rm -f "$_rb_out" "$_rb_fired" "$_rb_done" 2>/dev/null + return "$TIMEOUT_STATUS" + fi + rm -f "$_rb_out" "$_rb_fired" "$_rb_done" 2>/dev/null + return "$_st" +} + +# Echo the first candidate ("cmd args") that verifies as REAL Python, or nothing. +# A candidate qualifies ONLY if the probe exits 0 AND prints exactly the marker on stdout +# (P1-a: a stub that prints the marker but exits non-zero must be rejected). Candidates in +# order; `py -3` first (the Windows launcher). +find_real_python() { + for cand in "py -3" "python3" "python"; do + # shellcheck disable=SC2086 # intentional word-split: "py -3" -> py with arg -3 + set -- $cand + cmd=$1 + command -v "$cmd" >/dev/null 2>&1 || continue + probe_out=$(run_bounded "$@" -c "import sys; sys.stdout.write('$MARKER')" /dev/null) + probe_status=$? + if [ "$probe_status" -eq 0 ] && [ "$probe_out" = "$MARKER" ]; then + printf '%s' "$cand" + return 0 + fi + done + return 1 +} + +REAL_PY=$(find_real_python) || emit_passthrough_and_exit +[ -n "$REAL_PY" ] || emit_passthrough_and_exit + +# is_valid_hook_json: true iff $1 parses as a JSON object containing a top-level +# "hookSpecificOutput" key. Uses the REAL Python we already found (no jq dependency, P1-c: +# substring grep false-accepts e.g. `not json "hookSpecificOutput"`). Reads candidate on stdin. +is_valid_hook_json() { + # shellcheck disable=SC2086 # $REAL_PY is "py -3" or "python3" β€” intentional split + set -- $REAL_PY + printf '%s' "$GUARD_OUT" | "$@" -c ' +import sys, json +try: + d = json.load(sys.stdin) +except Exception: + sys.exit(1) +sys.exit(0 if isinstance(d, dict) and "hookSpecificOutput" in d else 1) +' >/dev/null 2>&1 +} + +# --- Supervise the guard subprocess (P1-b: time-bound it too; P1-c: validate JSON) -------- +# Run the guard with the found interpreter, replaying the captured payload on its stdin, +# under the SAME wall-clock bound as the probes so a hung guard can't wedge the hot path. +# Decide what to emit: +# * guard exits 0 AND stdout is a JSON object with hookSpecificOutput -> forward verbatim. +# * anything else (non-zero, timeout, empty, non-JSON, missing key) -> guard is BROKEN; +# per the maintainer decision (Β§3.2.1) degrade to pass-through + exit 0, never block. +# shellcheck disable=SC2086 # $REAL_PY is "py -3" or "python3" β€” intentional split +set -- $REAL_PY +# Capture the guard's stderr to a temp so we can RELAY it on the success path (P2-h: the guard +# has its own no-silent advisories β€” absent agent_type / schema drift / unreadable manifest β€” +# that must surface; the launcher only suppresses stderr on its OWN degraded paths). On the +# broken path we drop it, since broken-guard noise on every hot-path call is the #454 spam. +# No predictable /tmp fallback: a guessable path is a symlink-attack surface, and a redirect +# onto an attacker-owned symlink fails -> we'd read that as a broken guard and fail OPEN +# (gemini round-6 P1). If mktemp can't give us a private file, degrade safely to pass-through. +GUARD_ERR=$(mktemp 2>/dev/null) || emit_passthrough_and_exit +GUARD_OUT=$(printf '%s' "$PAYLOAD" | run_bounded "$@" "$GUARD" 2>"$GUARD_ERR") +GUARD_STATUS=$? + +if [ "$GUARD_STATUS" -eq 0 ] && is_valid_hook_json; then + # Healthy guard decision: relay its stderr advisories, then forward its JSON verbatim. + [ -s "$GUARD_ERR" ] && cat "$GUARD_ERR" >&2 + rm -f "$GUARD_ERR" 2>/dev/null + printf '%s\n' "$GUARD_OUT" + exit 0 +fi + +# Guard broke / timed out / produced invalid output. Degrade, don't block, don't spam. +rm -f "$GUARD_ERR" 2>/dev/null +emit_passthrough_and_exit diff --git a/scripts/_ci_pytest_manifest.toml b/scripts/_ci_pytest_manifest.toml index 04bcbe41..52062b1a 100644 --- a/scripts/_ci_pytest_manifest.toml +++ b/scripts/_ci_pytest_manifest.toml @@ -150,6 +150,10 @@ path = "scripts/test_eval_harness_workflow.py" id = "v3.10-134-write-scope-guard" path = "scripts/test_ars_write_scope_guard.py" +[[pytest]] +id = "454-write-scope-guard-launcher" +path = "scripts/test_run_guard_launcher.py" + [[pytest]] id = "v3.10-134-write-scope-lint-mutation" path = "scripts/test_check_v3_10_134_write_scope.py" diff --git a/scripts/test_run_guard_launcher.py b/scripts/test_run_guard_launcher.py new file mode 100644 index 00000000..a77db1d2 --- /dev/null +++ b/scripts/test_run_guard_launcher.py @@ -0,0 +1,467 @@ +"""Launch-layer tests for hooks/run_guard.sh (#454 Windows Python hook portability). + +The existing test_ars_write_scope_guard.py drives the guard via [sys.executable, guard] +β€” it never exercises INTERPRETER RESOLUTION, which is the entire #454 bug class (a Windows +`python3` that is a 0-byte Microsoft Store alias stub, not real Python). These tests cover +the launcher's job: find a REAL python (skipping stubs), run the guard as a SUPERVISED +subprocess, and degrade to pass-through+exit-0 (never block, never spam) when no real python +is found OR the guard subprocess is broken. + +Design: docs/design/2026-06-17-454-windows-python-hook-portability-design.md (Β§3.1–3.6). + +Mechanism: each test builds a temp `bin/` dir, puts fake `py`/`python3`/`python` programs +in it, and runs the launcher with PATH=that bin (plus a real python for the cases that need +one). A "stub" is a program that exits non-zero / prints nothing (mimicking the 0-byte Store +alias as the marker probe sees it). The launcher must: + - emit canonical pass-through JSON {"hookSpecificOutput":{"hookEventName":"PreToolUse"}} + and exit 0 when no real python verifies, or when the guard subprocess misbehaves; + - forward the guard's stdout JSON verbatim (deny via permissionDecision:"deny", or + pass-through) when a real python runs the guard cleanly; + - stay SILENT on stderr on the degraded paths (PreToolUse is a hot path β€” any per-call + stderr is the log spam #454 is about). + +NOTE (honest limitation): this is a POSIX-host simulation. The real Windows Store-alias path, +`py.exe -3` under Git Bash, Git Bash path conversion, CRLF, and the no-Git-Bash PowerShell +fallback still need a Windows repro (tracked in the spec Β§6). +""" + +import json +import os +import shutil +import stat +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +LAUNCHER = os.path.join(REPO_ROOT, "hooks", "run_guard.sh") +GUARD = os.path.join(REPO_ROOT, "scripts", "ars_write_scope_guard.py") +PASS_THROUGH = {"hookSpecificOutput": {"hookEventName": "PreToolUse"}} + +# A real python to hand the launcher when a test needs the guard to actually run. +REAL_PY = sys.executable + + +def _write_exec(path, body): + """Write an executable script at `path` with `body` (a /bin/sh script).""" + with open(path, "w") as fh: + fh.write(body) + os.chmod(path, os.stat(path).st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + + +def _stub(path): + """A non-executing stub: prints nothing, exits 1 β€” what the marker probe sees for a + 0-byte Microsoft Store alias when invoked non-interactively.""" + _write_exec(path, "#!/bin/sh\nexit 1\n") + + +def _fake_real_python(path, real=REAL_PY): + """A fake `python3`/`python`/`py` that behaves like a real interpreter by delegating to + an actual python. Handles the marker probe (`-c ...`) and running the guard script.""" + _write_exec(path, f'#!/bin/sh\nexec "{real}" "$@"\n') + + +# System dirs that hold sh/cat/grep/command/mktemp/sleep/kill/timeout. bin_dir goes FIRST +# on PATH so our injected fake py/python3/python (and any stub that shadows the system one) +# win interpreter resolution, while the launcher's own shell utilities still resolve. +_SYS_PATH = "/usr/bin:/bin:/usr/sbin:/sbin" +_SH = shutil.which("sh") or "/bin/sh" +_LAUNCHER_TIMEOUT = 45 # generous so a (bounded) hanging-candidate test can't false-fail + + +def _run_launcher(bin_dir, payload, extra_env=None, launcher=LAUNCHER): + """Run the launcher with PATH = bin_dir + system dirs (bin_dir first, so injected + interpreters shadow the real ones). Returns (exit_code, stdout, stderr). + + `launcher` defaults to the repo's real launcher; pass an alternate path to run a copy from + a temp plugin layout (the only way to exercise a broken/alternate guard now that the + ARS_GUARD_PATH_FOR_TEST back door is gone β€” the guard is ALWAYS resolved from the + launcher's own ../scripts/, P2-e).""" + path = bin_dir + os.pathsep + _SYS_PATH + env = { + "PATH": path, + # Short probe bound keeps the suite fast; a hanging-candidate is killed in ~1s. + "ARS_PROBE_BOUND": "1", + # The launcher must resolve the guard from its OWN path, not any var (codex P1). + # We deliberately do NOT set CLAUDE_PLUGIN_ROOT in most tests. + } + if extra_env: + env.update(extra_env) + proc = subprocess.run( + [_SH, launcher], + input=json.dumps(payload), + env=env, + capture_output=True, + text=True, + timeout=_LAUNCHER_TIMEOUT, + ) + return proc.returncode, proc.stdout, proc.stderr + + +def _make_plugin_layout(base, guard_body): + """Build a temp plugin layout under `base`: hooks/run_guard.sh (real launcher copy) + + scripts/ars_write_scope_guard.py (a guard whose body is `guard_body`) + the manifest the + real guard needs. Returns the path to the copied launcher. + + This is how a test runs an ALTERNATE/broken guard without a production back door: the + launcher resolves the guard from its OWN ../scripts/, so we plant the guard there (P2-e).""" + os.makedirs(os.path.join(base, "hooks")) + os.makedirs(os.path.join(base, "scripts")) + launcher_copy = os.path.join(base, "hooks", "run_guard.sh") + shutil.copy(LAUNCHER, launcher_copy) + with open(os.path.join(base, "scripts", "ars_write_scope_guard.py"), "w") as fh: + fh.write(guard_body) + # The real guard reads this manifest; copy it so an unmodified-guard layout also works. + manifest = os.path.join(os.path.dirname(GUARD), "ars_phase_scope_manifest.json") + shutil.copy(manifest, os.path.join(base, "scripts", "ars_phase_scope_manifest.json")) + return launcher_copy + + +def _bucket_a_payload(workspace, file_path): + """A Bucket A subagent Write payload (research_architect_agent is Bucket A).""" + return { + "tool_name": "Write", + "cwd": workspace, + "agent_type": "research_architect_agent", + "tool_input": {"file_path": file_path, "content": "x"}, + } + + +class LauncherNoPythonTest(unittest.TestCase): + """No real python anywhere -> pass-through + exit 0 + silent (Plan A, spec Β§3.2).""" + + def _assert_clean_passthrough(self, code, out, err): + self.assertEqual(code, 0, f"must exit 0, got {code}; stderr={err!r}") + self.assertEqual(json.loads(out), PASS_THROUGH, f"must emit canonical pass-through; got {out!r}") + self.assertEqual(err.strip(), "", f"must be SILENT on hot-path stderr; got {err!r}") + + def test_only_silent_stubs_on_path(self): + # All three candidates are stubs that exit 1 and print nothing (the marker probe sees + # exactly what a 0-byte Store alias produces). bin_dir is FIRST on PATH so these + # shadow any real system python. + with tempfile.TemporaryDirectory() as bin_dir: + for name in ("py", "python3", "python"): + _stub(os.path.join(bin_dir, name)) + code, out, err = _run_launcher(bin_dir, {"tool_name": "Bash", "tool_input": {"command": "ls"}}) + self._assert_clean_passthrough(code, out, err) + + def test_stubs_that_print_error_to_stderr(self): + # A different stub variant: exits 1 AND writes a Windows-style error to stderr (closer + # to "is not recognized" / Store-stub noise). The marker check is on STDOUT, so these + # are still rejected. Confirms the launcher keys on the stdout marker, not exit text. + with tempfile.TemporaryDirectory() as bin_dir: + for name in ("py", "python3", "python"): + _write_exec(os.path.join(bin_dir, name), + "#!/bin/sh\necho 'not a real python' 1>&2\nexit 9\n") + code, out, err = _run_launcher(bin_dir, {"tool_name": "Write", "tool_input": {"file_path": "/x", "content": "y"}}) + self._assert_clean_passthrough(code, out, err) + + def test_marker_printed_but_nonzero_exit_rejected(self): + """P1-a: a candidate that PRINTS the exact marker on stdout but EXITS NON-ZERO must be + rejected (a half-broken interpreter, or a stub that echoes then fails). The probe keys + on `exit 0 AND marker`, not the marker alone -> no real python -> pass-through.""" + with tempfile.TemporaryDirectory() as bin_dir: + for name in ("py", "python3", "python"): + # Print the marker to stdout (what a REAL probe would print) but exit 9. + # Must still be rejected because the exit status is non-zero. + _write_exec(os.path.join(bin_dir, name), + "#!/bin/sh\nprintf 'ARS_PY_OK'\nexit 9\n") + code, out, err = _run_launcher(bin_dir, {"tool_name": "Bash", "tool_input": {"command": "ls"}}) + self._assert_clean_passthrough(code, out, err) + + def test_no_timeout_binary_present(self): + """Launcher must still work (and not hang) when the `timeout` binary is bypassed β€” + forces the process-group watchdog fallback via ARS_GUARD_FORCE_WATCHDOG.""" + with tempfile.TemporaryDirectory() as bin_dir: + for name in ("py", "python3", "python"): + _stub(os.path.join(bin_dir, name)) + code, out, err = _run_launcher(bin_dir, {"tool_name": "Bash", "tool_input": {"command": "ls"}}, + extra_env={"ARS_GUARD_FORCE_WATCHDOG": "1"}) + self._assert_clean_passthrough(code, out, err) + + +class LauncherRealPythonForwardsTest(unittest.TestCase): + """A real python is found -> launcher runs the guard and forwards its JSON decision.""" + + def test_in_scope_write_passthrough_forwarded(self): + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + _fake_real_python(os.path.join(bin_dir, "python3")) + # research_architect_agent's allowed dir β€” an in-scope write should pass through. + # Use the agent's actual allowed glob area; a clearly in-scope path: workspace root file + # owned by main session is simplest, but we want the guard to RUN and say pass-through. + payload = {"tool_name": "Write", "cwd": ws, + "tool_input": {"file_path": os.path.join(ws, "notes.md"), "content": "x"}} + code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws}) + self.assertEqual(code, 0) + decision = json.loads(out) + self.assertNotIn("permissionDecision", decision["hookSpecificOutput"]) + + def test_out_of_scope_bucket_a_deny_forwarded(self): + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + _fake_real_python(os.path.join(bin_dir, "python3")) + # Bucket A agent writing outside its allowed globs -> guard denies via JSON. + payload = _bucket_a_payload(ws, os.path.join(ws, "totally_out_of_scope_dir", "x.md")) + code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws}) + self.assertEqual(code, 0, f"guard always exits 0; got {code} err={err!r}") + decision = json.loads(out) + self.assertEqual(decision["hookSpecificOutput"].get("permissionDecision"), "deny", + f"out-of-scope Bucket A write must be denied via forwarded JSON; got {out!r}") + + def test_bash_denied_for_bucket_a_forwarded(self): + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + _fake_real_python(os.path.join(bin_dir, "python3")) + payload = {"tool_name": "Bash", "cwd": ws, "agent_type": "research_architect_agent", + "tool_input": {"command": "rm -rf /"}} + code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws}) + self.assertEqual(code, 0) + decision = json.loads(out) + self.assertEqual(decision["hookSpecificOutput"].get("permissionDecision"), "deny") + + +class LauncherStubSkipOrderingTest(unittest.TestCase): + """First candidate is a stub, a later one is real -> skip the stub, use the real one.""" + + def test_py_stub_python3_real(self): + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + _stub(os.path.join(bin_dir, "py")) # py -3 fails (stub) + _fake_real_python(os.path.join(bin_dir, "python3")) # python3 is real + payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md")) + code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws}) + self.assertEqual(code, 0) + decision = json.loads(out) + self.assertEqual(decision["hookSpecificOutput"].get("permissionDecision"), "deny", + "must skip the py stub and use the real python3") + + +class LauncherPyDashThreeArgTest(unittest.TestCase): + """`py -3` must be invoked as command `py` with arg `-3`, not an executable named `py -3`.""" + + def test_py_invoked_with_dash_three(self): + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + argv_log = os.path.join(bin_dir, "py_argv.log") + # fake `py` that only succeeds when given -3 as the first arg; logs argv. + _write_exec(os.path.join(bin_dir, "py"), ( + "#!/bin/sh\n" + f'echo "$@" >> "{argv_log}"\n' + 'if [ "$1" = "-3" ]; then shift; exec "' + REAL_PY + '" "$@"; fi\n' + "exit 1\n" + )) + payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md")) + code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws}) + self.assertEqual(code, 0) + decision = json.loads(out) + self.assertEqual(decision["hookSpecificOutput"].get("permissionDecision"), "deny") + # Confirm py was actually called with -3 (not as a literal "py -3" command, which + # would have produced no log line at all because no such command exists). + self.assertTrue(os.path.exists(argv_log), "py was never invoked") + with open(argv_log) as fh: + first = fh.readline().strip() + self.assertTrue(first.startswith("-3"), f"py must be called with -3 first; got {first!r}") + + +class LauncherSelfResolveTest(unittest.TestCase): + """Launcher resolves the guard from its OWN path; works with CLAUDE_PLUGIN_ROOT unset + and with a plugin path containing spaces (codex P1).""" + + def test_claude_plugin_root_unset(self): + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + _fake_real_python(os.path.join(bin_dir, "python3")) + payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md")) + # _run_launcher deliberately does not set CLAUDE_PLUGIN_ROOT. + code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws}) + self.assertEqual(code, 0) + self.assertEqual(json.loads(out)["hookSpecificOutput"].get("permissionDecision"), "deny") + + def test_plugin_path_with_spaces(self): + # Copy the launcher + guard into a path containing spaces, run from there. + with tempfile.TemporaryDirectory() as base, tempfile.TemporaryDirectory() as bin_dir, \ + tempfile.TemporaryDirectory() as ws: + spaced = os.path.join(base, "plugin dir with spaces") + os.makedirs(os.path.join(spaced, "hooks")) + os.makedirs(os.path.join(spaced, "scripts")) + shutil.copy(LAUNCHER, os.path.join(spaced, "hooks", "run_guard.sh")) + shutil.copy(GUARD, os.path.join(spaced, "scripts", "ars_write_scope_guard.py")) + # the guard imports nothing repo-local except its manifest, which lives in scripts/ + manifest = os.path.join(os.path.dirname(GUARD), "ars_phase_scope_manifest.json") + shutil.copy(manifest, os.path.join(spaced, "scripts", "ars_phase_scope_manifest.json")) + _fake_real_python(os.path.join(bin_dir, "python3")) + payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md")) + env = {"PATH": bin_dir + os.pathsep + _SYS_PATH, "CLAUDE_PROJECT_DIR": ws, + "ARS_PROBE_BOUND": "1"} + proc = subprocess.run([_SH, os.path.join(spaced, "hooks", "run_guard.sh")], + input=json.dumps(payload), env=env, capture_output=True, + text=True, timeout=_LAUNCHER_TIMEOUT) + self.assertEqual(proc.returncode, 0, f"stderr={proc.stderr!r}") + self.assertEqual(json.loads(proc.stdout)["hookSpecificOutput"].get("permissionDecision"), + "deny", "launcher must resolve guard via its own path even under spaces") + + +class LauncherGuardBrokeTest(unittest.TestCase): + """Found real python BUT the guard subprocess misbehaves -> pass-through + exit 0, + do NOT block (maintainer decision Β§3.2.1), do NOT spam stderr. + + Mechanism (P2-e): the launcher resolves the guard from its OWN ../scripts/ β€” no test-only + path override exists in production. So each test plants a broken guard in a temp plugin + layout (real launcher copy + broken scripts/ars_write_scope_guard.py) and runs that copy.""" + + def _run_with_broken_guard(self, guard_body): + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as base: + _fake_real_python(os.path.join(bin_dir, "python3")) + launcher_copy = _make_plugin_layout(base, guard_body) + return _run_launcher( + bin_dir, {"tool_name": "Bash", "tool_input": {"command": "ls"}}, + launcher=launcher_copy, + ) + + def test_guard_exits_nonzero(self): + code, out, err = self._run_with_broken_guard("import sys\nsys.exit(3)\n") + self.assertEqual(code, 0, f"must not propagate guard crash as block/error; err={err!r}") + self.assertEqual(json.loads(out), PASS_THROUGH) + self.assertEqual(err.strip(), "") + + def test_guard_prints_nothing(self): + code, out, err = self._run_with_broken_guard("pass\n") # exits 0, no stdout + self.assertEqual(code, 0) + self.assertEqual(json.loads(out), PASS_THROUGH) + self.assertEqual(err.strip(), "") + + def test_guard_prints_invalid_json(self): + code, out, err = self._run_with_broken_guard("print('not json at all')\n") + self.assertEqual(code, 0) + self.assertEqual(json.loads(out), PASS_THROUGH) + self.assertEqual(err.strip(), "") + + def test_guard_prints_substring_not_json(self): + """P1-c: stdout CONTAINS the literal 'hookSpecificOutput' but is NOT valid JSON. A + substring grep would false-accept and forward garbage; the real json.load parse must + reject it and degrade to pass-through.""" + code, out, err = self._run_with_broken_guard( + "print('not json but mentions \"hookSpecificOutput\" in prose')\n" + ) + self.assertEqual(code, 0, f"non-JSON substring match must NOT be forwarded; err={err!r}") + self.assertEqual(json.loads(out), PASS_THROUGH) + self.assertEqual(err.strip(), "") + + def test_guard_script_missing(self): + # A plugin layout with the launcher but NO scripts/ars_write_scope_guard.py: the + # launcher resolves a guard path that doesn't exist -> the run_bounded exec fails -> + # broken-path degrade. Build the layout, then delete the guard the helper planted. + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as base: + _fake_real_python(os.path.join(bin_dir, "python3")) + launcher_copy = _make_plugin_layout(base, "pass\n") + os.remove(os.path.join(base, "scripts", "ars_write_scope_guard.py")) + code, out, err = _run_launcher( + bin_dir, {"tool_name": "Bash", "tool_input": {"command": "ls"}}, + launcher=launcher_copy, + ) + self.assertEqual(code, 0, f"missing guard -> pass-through, not block; err={err!r}") + self.assertEqual(json.loads(out), PASS_THROUGH) + + +class LauncherHangingCandidateTest(unittest.TestCase): + """A candidate interpreter that hangs must be killed within a bound, then move on β€” + the hot path must never hang (spec Β§3.3 watchdog).""" + + def test_hanging_py_then_real_python3(self): + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + # py hangs on the marker probe; python3 is real. Launcher must kill py's probe + # and fall through to python3 within the test's 30s subprocess timeout. + _write_exec(os.path.join(bin_dir, "py"), "#!/bin/sh\nsleep 60\n") + _fake_real_python(os.path.join(bin_dir, "python3")) + payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md")) + code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws}) + self.assertEqual(code, 0, f"must not hang/error on a hanging candidate; err={err!r}") + self.assertEqual(json.loads(out)["hookSpecificOutput"].get("permissionDecision"), "deny", + "must kill the hanging py and use the real python3") + + def test_hanging_py_killed_via_watchdog_path(self): + """P2-d: same hanging-candidate scenario, but ARS_GUARD_FORCE_WATCHDOG=1 forces the + no-`timeout` process-group watchdog instead of the `timeout` binary. Confirms the + watchdog (setsid + kill -TERM/-KILL on the negative pid) actually reaps a hung probe + so the launcher still falls through to the real python3 within the bound. A candidate + that hangs AND spawns a child grandprocess exercises the process-GROUP kill.""" + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + # py spawns a long-sleeping grandchild then waits β€” a bare-pid TERM would leak the + # child; the process-group kill must take down the whole group. + _write_exec(os.path.join(bin_dir, "py"), + "#!/bin/sh\nsleep 60 &\nwait\n") + _fake_real_python(os.path.join(bin_dir, "python3")) + payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md")) + code, out, err = _run_launcher(bin_dir, payload, + extra_env={"CLAUDE_PROJECT_DIR": ws, + "ARS_GUARD_FORCE_WATCHDOG": "1"}) + self.assertEqual(code, 0, f"watchdog must reap the hung probe; err={err!r}") + self.assertEqual(json.loads(out)["hookSpecificOutput"].get("permissionDecision"), "deny", + "watchdog must kill the hanging py and fall through to python3") + + +class LauncherWatchdogRobustnessTest(unittest.TestCase): + """Round-6 gemini track: the no-`timeout` watchdog must not fail OPEN under (a) a private- + temp allocation failure or (b) the pid-reuse race that would false-report a timeout.""" + + def test_mktemp_failure_degrades_to_passthrough_not_fail_open(self): + """gemini P1: if `mktemp` can't hand out a private temp, the launcher must NOT fall back + to a predictable /tmp path (symlink-attack surface whose redirect failure would read as a + broken guard and fail OPEN). It must degrade to a clean pass-through: exit 0, canonical + JSON, silent. We force mktemp to fail by shadowing it on PATH and force the watchdog path + (where the temp files live) via ARS_GUARD_FORCE_WATCHDOG.""" + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + _fake_real_python(os.path.join(bin_dir, "python3")) + _write_exec(os.path.join(bin_dir, "mktemp"), "#!/bin/sh\nexit 1\n") # mktemp always fails + # A Bucket A out-of-scope write would normally be DENIED β€” proving we don't fail open, + # the launcher must NOT emit deny here (it can't run the guard without a temp), it must + # cleanly pass through instead of erroring or blocking. + payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md")) + code, out, err = _run_launcher(bin_dir, payload, + extra_env={"CLAUDE_PROJECT_DIR": ws, + "ARS_GUARD_FORCE_WATCHDOG": "1"}) + self.assertEqual(code, 0, f"mktemp failure must not crash/block; err={err!r}") + self.assertEqual(json.loads(out), PASS_THROUGH, + "mktemp failure must degrade to canonical pass-through, never a " + "predictable-temp path that fails open") + self.assertEqual(err.strip(), "", f"degraded path must be silent; got {err!r}") + + def test_fast_guard_not_falsely_timed_out_on_watchdog_path(self): + """gemini P1 (pid-reuse race): on the forced watchdog path, a guard that answers WELL + within the bound must have its real decision forwarded β€” the done-file handshake must not + let the watchdog false-flag a timeout (which would fail open to pass-through). Run the + deny case several times; a single false-timeout would surface as a pass-through.""" + with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws: + _fake_real_python(os.path.join(bin_dir, "python3")) + payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md")) + for i in range(5): + code, out, err = _run_launcher(bin_dir, payload, + extra_env={"CLAUDE_PROJECT_DIR": ws, + "ARS_GUARD_FORCE_WATCHDOG": "1"}) + self.assertEqual(code, 0, f"run {i}: err={err!r}") + self.assertEqual(json.loads(out)["hookSpecificOutput"].get("permissionDecision"), + "deny", + f"run {i}: fast guard decision must be forwarded, not lost to a " + f"false timeout; got {out!r}") + + +class LauncherInfraProtectionTest(unittest.TestCase): + """A Bucket A subagent writing to hooks/run_guard.sh itself must be denied β€” confirms + the hooks/*.sh infra glob covers the new launcher end-to-end through the launcher.""" + + def test_subagent_write_to_launcher_denied(self): + with tempfile.TemporaryDirectory() as bin_dir: + _fake_real_python(os.path.join(bin_dir, "python3")) + # plugin_root defaults to the repo root (launcher's ../). Target the real launcher + # path inside this repo so it resolves inside plugin_root and matches hooks/*.sh. + payload = { + "tool_name": "Write", + "cwd": REPO_ROOT, + "agent_type": "research_architect_agent", + "tool_input": {"file_path": LAUNCHER, "content": "evil"}, + } + code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PLUGIN_ROOT": REPO_ROOT}) + self.assertEqual(code, 0) + self.assertEqual(json.loads(out)["hookSpecificOutput"].get("permissionDecision"), "deny", + "writing the launcher itself must be infra-denied") + + +if __name__ == "__main__": + unittest.main()