mirror of
https://github.com/Imbad0202/academic-research-skills.git
synced 2026-09-14 13:51:17 +08:00
fix(#454): Windows Python hook portability + graceful no-Python degradation
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)
This commit is contained in:
Edward Cheng-I Wu
committed by
GitHub
GitHub
parent
88fc003e6a
commit
4f8acbcaf2
@@ -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
|
||||
@@ -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)
|
||||
|
||||
@@ -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<N>.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.
|
||||
|
||||
@@ -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):**
|
||||
|
||||
|
||||
@@ -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 — `<cmd> <args> -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 <N>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 <N>; 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.
|
||||
+1
-1
@@ -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\""
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
Executable
+233
@@ -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 2>/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
|
||||
@@ -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"
|
||||
|
||||
@@ -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()
|
||||
Reference in New Issue
Block a user