diff --git a/plugins/disk-hygiene/.claude-plugin/plugin.json b/plugins/disk-hygiene/.claude-plugin/plugin.json index 20908772d..821862d59 100644 --- a/plugins/disk-hygiene/.claude-plugin/plugin.json +++ b/plugins/disk-hygiene/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "disk-hygiene", - "version": "0.17.7", + "version": "0.17.8", "description": "Context-aware disk hygiene for arbitrary directory trees: inventories orphaned and temporary artifacts, classifies evidence into review tiers, and offers exact-path cleanup only after a fresh safety preview and explicit per-tier approval. The target is read-only by default; OS-managed paths, links and mount points, VCS-tracked content, changed entries, and live-handle uncertainty fail closed.", "author": { "name": "Melodic Software", diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index 630f2428a..6580585df 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -3,6 +3,43 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.17.8] + +### Fixed + +- **Wired hooks launch in shell form, restoring the destructive guard on Windows (#1416).** + `0.17.6` moved both `hooks/hooks.json` registrations onto `"command": "bash"` + `args` to + resolve a real Python 3 interpreter (#1504) — and in doing so reintroduced the exact launch + failure #1006 had already fixed for the skill-frontmatter hook. Exec form (`args` present) is + a bare `PATH` lookup, and on Windows `bash` resolves to the WSL relay `System32\bash.exe` + before Git Bash: `execvpe(/bin/bash) failed: No such file or directory`. A hook that fails to + launch is a **non-blocking** error, so `destructive_guard.py` never ran and the PreToolUse + gate silently enforced nothing on every such host. Both registrations now name + `run-python-hook.sh` directly with `"shell": "bash"` and no `args`, which Claude Code routes + through Git Bash instead of a `PATH` lookup. Every `${CLAUDE_PLUGIN_ROOT}` / + `${CLAUDE_PLUGIN_DATA}` placeholder is double-quoted, so the argv is byte-identical to the + exec-form vector across paths containing spaces. The #1504 Python-resolution behaviour is + unchanged — only the launch mechanism moves. `hooks/run-python-hook.test.sh` previously + asserted `.command == "bash"`, encoding the defect as the contract; it now asserts the + portability property (launcher named in `command`, no `args`, `shell: bash`, every + placeholder quoted). +- **Security records now assess the shell-form launch instead of asserting the old exec form.** + The README trust-surface record and `skills/clean/reference/safety-model.md` still bounded the + plugin-level hook by "exec form (no shell)" — a safety claim the same change disproved, so the + plugin's own security assessment reasoned from a false premise. Both now state what shell form + does and does not guarantee: the command string is a fixed literal in the plugin's own + `hooks.json` with no model-, repo-, or session-supplied interpolation, whose only substituted + values are Claude Code's own double-quoted `${CLAUDE_PLUGIN_ROOT}`/`${CLAUDE_PLUGIN_DATA}` + placeholders — verified byte-identical to the exec-form argv for roots containing spaces and + backslashes — while noting that those placeholders are substituted textually before bash parses + the result, so the quoting bounds whitespace and backslashes rather than every shell + metacharacter. The invariant is now maintained by `hooks/run-python-hook.test.sh` and the + form-agnostic `test_hygiene.py` hook helpers rather than being structural (repo-wide gate: #2569). + `skills/clean/SKILL.md` split its single launch bullet per surface — the wired gate resolves + Python through `run-python-hook.sh`, while the skill-scoped belt is the one still exec-form on a + bare `python3` (#2568) — and `safety-model.md` dropped a stale claim that the `Stop` detector + shares the guard's `python3` lookup and leaves that vector unreported, which #1504 already closed. + ## [0.17.7] ### Changed diff --git a/plugins/disk-hygiene/README.md b/plugins/disk-hygiene/README.md index 2a0122f44..368ef9a12 100644 --- a/plugins/disk-hygiene/README.md +++ b/plugins/disk-hygiene/README.md @@ -47,11 +47,16 @@ at preview. Backups remain the recovery boundary for user data. - Python 3.11+ available on `PATH` is required for scanning, validation, the skill-scoped guard, and cleanup (the floor's single origin is the `MIN_PYTHON` constant in `skills/clean/scripts/hygiene.py`; `/disk-hygiene:setup check` derives the enforced value from - there, so treat the number printed here as a convenience copy). Claude Code launches the guard in - shell-free exec form; guarded engine calls must use the same absolute interpreter reported by that - guard, so Bash aliases and functions cannot replace it. Both wired hooks register as `bash` - invoking `hooks/run-python-hook.sh`, so `bash` must resolve on `PATH` (Git Bash on Windows) before - the launcher can resolve Python (#1504). The guard registers on two surfaces: a + there, so treat the number printed here as a convenience copy). Claude Code launches the + skill-scoped guard in shell-free exec form; guarded engine calls must use the same absolute + interpreter reported by that guard, so Bash aliases and functions cannot replace it. Both wired + hooks register in **shell form** — the `command` string names `hooks/run-python-hook.sh` directly + with `"shell": "bash"` and no `args` — so Claude Code routes them through Git Bash itself instead + of resolving `bash` on `PATH`. Registering them in exec form as `"command": "bash"` is the + regression #1416 tracks: on Windows that bare `PATH` lookup finds the WSL relay + `System32\bash.exe` before Git Bash, the launch fails, and a failed hook launch is non-blocking — + so the guard silently enforces nothing. The launcher then resolves Python (#1504). The guard + registers on two surfaces: a plugin-level **engine gate** (`hooks/hooks.json`) that acts only on commands referencing the engine — deferring everything else instantly — and enforces the kill switch and data-root authority; and the skill-scoped **belt** inside the `clean` skill's context, which adds the @@ -190,22 +195,42 @@ hand-cleaning the zone. or unreadable value fails closed to enabled. The one residual a hook cannot read is a value supplied only via a session `--settings` file. The skill's own kill-switch probe + skill-content value remain a defense-in-depth honoring layer over the guard. -- **Trust-surface record (0.7.0; updated 0.9.0):** the plugin-level `hooks/hooks.json` PreToolUse +- **Trust-surface record (0.7.0; updated 0.17.8):** the plugin-level `hooks/hooks.json` PreToolUse registration is a NEW trust surface (a hook that launches in every consumer session), added deliberately for guard-enforced audit-only mode and data-root authority (#1106 decision, Option E — - split registration). Its blast radius is bounded by design: exec form (no shell), bundled - standard-library script only, instant no-output deferral for any command not referencing the engine, - and no new capability beyond what the skill-scoped deployment already did during active cleanup. - Known costs, accepted: one `python3` launch per Bash/PowerShell call, and on a machine where - `python3` resolves to the Windows Store alias stub the launch fails on every call (tracked with - remediation detection in #1110). **0.9.0 delta:** the gate no longer carries a `${user_config.*}` + split registration). Its blast radius is bounded by design: a fixed launch string authored in the + plugin's own `hooks.json` (see the 0.17.8 delta for exactly what that bounds now that the string + reaches a shell), bundled standard-library scripts only, instant no-output deferral for any command + not referencing the engine, and no new capability beyond what the skill-scoped deployment already + did during active cleanup. Known costs, accepted: one launcher shell plus one Python launch per + Bash/PowerShell call, and on a machine where no Python 3 interpreter resolves at all the gate fails + open on every call — the `Stop` detector emits a `systemMessage` for that case, so the blind spot is + visible rather than silent (#1110, #1504). **0.9.0 delta:** the gate no longer carries a `${user_config.*}` argument (which, unset, dropped the whole hook and left the gate inert on a default install); it now registers unconditionally and resolves the kill switch by **reading** the user `settings.json` and the platform managed-settings.json. The added trust surface is that settings-file *read* — bounded to a single `pluginConfigs` value, from the user file (located from `${CLAUDE_PLUGIN_ROOT}`) and the root-owned managed file at its fixed system path, no write. Both are the plugin's own documented CC config, sanctioned by the acceptance review's operator-home carve-out (criterion 4). This entry is the - plugin-acceptance review delta for the change. A direct `hygiene.py` invocation outside that skill does not read the toggle and + plugin-acceptance review delta for the change. **0.17.8 delta (launch form):** both wired hooks now + register in **shell form** — the `command` string names `hooks/run-python-hook.sh` with + `"shell": "bash"` and no `args` — because exec form's bare `PATH` lookup for `bash` resolved to the + WSL relay on Windows and the guard silently never launched (#1416). Stated plainly: a shell now + parses the launch string, so "no shell involved" is no longer what bounds this surface. What bounds + it instead is that the string is a **fixed literal** in the plugin's own `hooks.json` with no model-, + repo-, or session-supplied text interpolated into it; the only values substituted are Claude Code's + own `${CLAUDE_PLUGIN_ROOT}` and `${CLAUDE_PLUGIN_DATA}`, and each is double-quoted, so the shell's + re-tokenization reproduces the exec-form argument vector byte-for-byte — verified for both hooks + against roots containing spaces and backslashes. The limits of that quoting belong in the record + too: Claude Code substitutes those placeholders *textually* before bash parses the result, so the + double quotes bound whitespace and backslashes but would not neutralize a `$` or a backtick inside a + substituted value (both placeholders resolve under Claude Code's own install and data roots). The + invariant is therefore **maintained by test**, not structural — `hooks/run-python-hook.test.sh` + asserts the launcher is named in `command`, `args` is absent, `shell: bash` is declared, and every + placeholder is double-quoted, and `test_hygiene.py`'s hook helpers are form-agnostic so a shell-form + entry can never make an assertion vacuously green. Interpolating anything beyond those two + placeholders into the command string would open a live injection surface; a repo-wide CI gate for + this defect class is proposed in #2569. A direct `hygiene.py` invocation outside that skill does not read the toggle and answers only to the engine's own preview/approval-token gate. The toggle can only narrow the destructive surface, never widen it (see [the safety model](skills/clean/reference/safety-model.md) for the degraded-mode detail). No credentials. Policy comes from an explicit invocation diff --git a/plugins/disk-hygiene/hooks/hooks.json b/plugins/disk-hygiene/hooks/hooks.json index 843084347..725a9a883 100644 --- a/plugins/disk-hygiene/hooks/hooks.json +++ b/plugins/disk-hygiene/hooks/hooks.json @@ -6,17 +6,8 @@ "hooks": [ { "type": "command", - "command": "bash", - "args": [ - "${CLAUDE_PLUGIN_ROOT}/hooks/run-python-hook.sh", - "${CLAUDE_PLUGIN_ROOT}/skills/clean/scripts/destructive_guard.py", - "--mode", - "engine-gate", - "--plugin-root", - "${CLAUDE_PLUGIN_ROOT}", - "--authorized-data-root", - "${CLAUDE_PLUGIN_DATA}" - ], + "command": "\"${CLAUDE_PLUGIN_ROOT}\"/hooks/run-python-hook.sh \"${CLAUDE_PLUGIN_ROOT}\"/skills/clean/scripts/destructive_guard.py --mode engine-gate --plugin-root \"${CLAUDE_PLUGIN_ROOT}\" --authorized-data-root \"${CLAUDE_PLUGIN_DATA}\"", + "shell": "bash", "timeout": 60 } ] @@ -27,13 +18,8 @@ "hooks": [ { "type": "command", - "command": "bash", - "args": [ - "${CLAUDE_PLUGIN_ROOT}/hooks/run-python-hook.sh", - "${CLAUDE_PLUGIN_ROOT}/skills/clean/scripts/guard_launch_monitor.py", - "--data-root", - "${CLAUDE_PLUGIN_DATA}" - ], + "command": "\"${CLAUDE_PLUGIN_ROOT}\"/hooks/run-python-hook.sh \"${CLAUDE_PLUGIN_ROOT}\"/skills/clean/scripts/guard_launch_monitor.py --data-root \"${CLAUDE_PLUGIN_DATA}\"", + "shell": "bash", "timeout": 20, "statusMessage": "Checking for silent destructive-guard failures..." } diff --git a/plugins/disk-hygiene/hooks/run-python-hook.sh b/plugins/disk-hygiene/hooks/run-python-hook.sh index 7eecdb179..09bab23eb 100755 --- a/plugins/disk-hygiene/hooks/run-python-hook.sh +++ b/plugins/disk-hygiene/hooks/run-python-hook.sh @@ -2,12 +2,21 @@ # Launch disk-hygiene wired hooks through a Python 3 interpreter resolved # independently of a bare `python3` on PATH (#1504). # -# Claude Code registers plugin hooks in exec form: the `command` field is -# resolved on PATH with no shell. When `python3` is absent, broken, or resolves -# to the zero-length WindowsApps App Execution Alias stub, both the guard and its -# Stop detector died the same way — the detector could not observe the guard's -# fail-open. This launcher is registered as `bash` (available in Git Bash on -# Windows) and resolves a real interpreter before exec'ing the target script. +# When `python3` is absent, broken, or resolves to the zero-length WindowsApps +# App Execution Alias stub, both the guard and its Stop detector died the same +# way — the detector could not observe the guard's fail-open. This launcher +# resolves a real interpreter before exec'ing the target script. +# +# hooks.json invokes this file in SHELL FORM — the `command` string names this +# script by path and carries its arguments, with no `args` key. Claude Code +# routes shell form through Git Bash on Windows, resolved by Claude Code itself. +# It must NOT be registered in exec form as `"command": "bash"` + `args`: exec +# form is a bare PATH lookup, and on Windows `bash` resolves to the WSL relay +# `System32\bash.exe` before Git Bash, which fails with +# `execvpe(/bin/bash) failed` (#1006, regressed by #1504). A hook that fails to +# launch is a non-blocking error, so the guard silently enforces nothing. +# Every path placeholder in the command string must stay double-quoted; the +# shell re-tokenizes the string, and plugin roots contain spaces. # # When no interpreter resolves: # * guard_launch_monitor.py — emit a once-per-run systemMessage on stdout diff --git a/plugins/disk-hygiene/hooks/run-python-hook.test.sh b/plugins/disk-hygiene/hooks/run-python-hook.test.sh index 8ec059cd4..58c1a6adf 100755 --- a/plugins/disk-hygiene/hooks/run-python-hook.test.sh +++ b/plugins/disk-hygiene/hooks/run-python-hook.test.sh @@ -33,7 +33,14 @@ assert_contains() { fi } -# --- launcher is executable and hooks.json wires bash + this script --- +# --- hooks.json wires this launcher in portable shell form --- +# +# These assert the PORTABILITY PROPERTY, not a literal spelling. The previous +# revision asserted `.command == "bash"` with the script in `.args`, which +# encoded the #1006 defect as the contract: exec form (`args` present) resolves +# `command` as a bare PATH lookup, and on Windows `bash` finds the WSL relay +# `System32\bash.exe` before Git Bash. The launch fails, and a failed hook +# launch is non-blocking — so the guard silently enforced nothing. HOOKS_JSON="$SCRIPT_DIR/hooks.json" if ! command -v jq >/dev/null 2>&1; then echo "SKIP: jq required" >&2 @@ -41,16 +48,35 @@ if ! command -v jq >/dev/null 2>&1; then fi for hook_name in destructive_guard.py guard_launch_monitor.py; do - command_line="$(jq -r --arg target "$hook_name" ' + entry="$(jq -c --arg target "$hook_name" ' .hooks | to_entries[] | .value[]? | .hooks[]? | - select(.args[]? | contains($target)) | .command + select(.command | contains($target)) ' "$HOOKS_JSON" | head -n1)" - assert_eq "hooks.json command for $hook_name is bash" "bash" "$command_line" - launcher_arg="$(jq -r --arg target "$hook_name" ' - .hooks | to_entries[] | .value[]? | .hooks[]? | - select(.args[]? | contains($target)) | .args[0] - ' "$HOOKS_JSON" | head -n1)" - assert_contains "hooks.json args[0] for $hook_name is the launcher" "run-python-hook.sh" "$launcher_arg" + if [[ -z "$entry" ]]; then + fail "hooks.json has no command hook referencing $hook_name" + fi + + command_line="$(jq -r '.command' <<<"$entry")" + assert_contains "hooks.json command for $hook_name invokes the launcher" \ + "run-python-hook.sh" "$command_line" + + # Shell form only: `args` present would switch Claude Code to exec form, where + # `command` is a bare PATH lookup and `shell` is ignored. + assert_eq "hooks.json entry for $hook_name omits args (shell form)" \ + "null" "$(jq -r '.args // "null" | if type == "array" then "present" else . end' <<<"$entry")" + + # Explicit `shell: bash`. Shell form otherwise falls back to PowerShell on a + # Windows host with no Git Bash detected, which cannot run a .sh launcher. + assert_eq "hooks.json entry for $hook_name declares shell bash" \ + "bash" "$(jq -r '.shell // ""' <<<"$entry")" + + # Every path placeholder must be double-quoted: the shell re-tokenizes the + # command string, and plugin roots routinely contain spaces. + unquoted="$(grep -oE '(^|[^"])\$\{CLAUDE_PLUGIN_(ROOT|DATA)\}|\$\{CLAUDE_PLUGIN_(ROOT|DATA)\}([^"]|$)' <<<"$command_line" || true)" + if [[ -n "$unquoted" ]]; then + fail "hooks.json command for $hook_name has an unquoted path placeholder: $unquoted" + fi + pass "hooks.json command for $hook_name double-quotes every path placeholder" done # --- monitor mode without python emits systemMessage JSON --- diff --git a/plugins/disk-hygiene/skills/clean/SKILL.md b/plugins/disk-hygiene/skills/clean/SKILL.md index bb85076c9..4abcc6487 100644 --- a/plugins/disk-hygiene/skills/clean/SKILL.md +++ b/plugins/disk-hygiene/skills/clean/SKILL.md @@ -57,7 +57,7 @@ unless bounded with `--max-depth` or confirmed with `--confirmed-large-scan`. `${CLAUDE_PLUGIN_ROOT}` — not the environment), so in audit-only mode it denies both mutation lanes it gates outright — the Bash engine `apply` and the PowerShell deletion belt alike. Running the probe still matters so you can state the configured value accurately and stop before proposing work the guard - would deny; the guard is the backstop, not the sole enforcer. The hook runs in shell-free exec form and reports its absolute Python + would deny; the guard is the backstop, not the sole enforcer. The guard reports its absolute Python interpreter and the authorized `--data-root` value in denial guidance. Use that exact interpreter path as `` for every engine call; bare `python`/`python3` is rejected because Bash aliases and functions can @@ -353,13 +353,23 @@ sparse files, hard links, compression, and delayed allocation affect it. `settings.json`, located from the `${CLAUDE_PLUGIN_ROOT}` both receive — so both honor a configured `false`, register unconditionally, and fail closed to enabled when the value is absent or unreadable. Verdicts are idempotent where both fire. -- The guard hook launches in exec form via `python3`, resolved on `PATH` with no shell (`python3`, - not bare `python`, because stock macOS and many Linux distros ship only `python3` and a legacy - `python` 2.x would crash the guard on modern syntax). Enforcement is therefore only as strong as - that resolution: on a host where `python3` does not resolve to an interpreter meeting the - engine's `MIN_PYTHON` floor the PreToolUse - launch fails, and Claude Code treats a failed hook launch as a non-blocking error, so the guard - does not intercept there. Concretely, the exposure is the manual PowerShell deletion lane: engine +- The two surfaces launch differently, and only one depends on a bare `python3` on `PATH`. The + **plugin-level engine gate** (and its `Stop` detector) register in **shell form**: the `command` + string names `hooks/run-python-hook.sh` with `"shell": "bash"` and no `args`, so Claude Code routes + them through its own Git Bash instead of a `PATH` lookup for `bash` (#1416), and the launcher then + resolves a real interpreter — `python3`, then `python`, then `py -3`, rejecting the zero-length + `WindowsApps` alias stub (#1504). A shell parses that launch string, but it is a fixed literal whose + only substituted values are Claude Code's own quoted `${CLAUDE_PLUGIN_ROOT}`/`${CLAUDE_PLUGIN_DATA}` + placeholders. When no interpreter resolves the gate still fails open, but the `Stop` detector emits + a `systemMessage` naming the blind spot, so it is visible rather than silent. +- This **skill's frontmatter belt** still launches in exec form via `python3`, resolved on `PATH` with + no shell (`python3`, not bare `python`, because stock macOS and many Linux distros ship only + `python3` and a legacy `python` 2.x would crash the guard on modern syntax). Its enforcement is + therefore only as strong as that single lookup: on a host where `python3` does not resolve to an + interpreter meeting the engine's `MIN_PYTHON` floor the PreToolUse launch fails, and Claude Code + treats a failed hook launch as a non-blocking error, so the belt does not intercept there + (converting this surface is tracked in #2568). Concretely, the exposure is the manual PowerShell + deletion lane: engine `apply` is unsupported on Windows and macOS and elsewhere runs only behind the guard's own `ask`, so no silent auto-delete path opens, but the guard's PowerShell belt that turns a deletion spelling into a final human prompt is lost. The backstops that remain are the per-path human approval the diff --git a/plugins/disk-hygiene/skills/clean/reference/safety-model.md b/plugins/disk-hygiene/skills/clean/reference/safety-model.md index 7fb80fd48..76c943749 100644 --- a/plugins/disk-hygiene/skills/clean/reference/safety-model.md +++ b/plugins/disk-hygiene/skills/clean/reference/safety-model.md @@ -191,8 +191,9 @@ destination, a truncated `Out-File` target, or an entire volume. TODO(#387): extend the flagged set to those spellings. **Kill-switch enforcement (since 0.9.0): both surfaces resolve it by reading user settings.** The guard -registers on two surfaces — the **plugin-level engine gate** (`hooks/hooks.json`, exec form, -`--mode engine-gate`) and the **skill-scoped belt** (the clean skill's frontmatter hook) — and both +registers on two surfaces — the **plugin-level engine gate** (`hooks/hooks.json`, shell form through +`hooks/run-python-hook.sh`, `--mode engine-gate`; see "Hook launch form" below) and the +**skill-scoped belt** (the clean skill's frontmatter hook, still exec form) — and both resolve `disk_hygiene_enabled` the same single way: by reading it from `pluginConfigs` in the `settings.json` files, through the shared `lib/killswitch_config.py` reader (the same read the setup skill's `kill_switch_probe.py` reports). Neither surface takes the value from the process environment. @@ -251,6 +252,28 @@ Even when the switch resolves enabled, the PowerShell lane is a raised bar, not mutation spelling passes it, so the engine's own containment, revalidation, and platform gates remain the deletion authority. +**Hook launch form, and what it does and does not bound (since 0.17.8, #1416).** The two wired hooks +— the engine gate on `PreToolUse` and its detector on `Stop` — register in **shell form**: the +`command` string names `hooks/run-python-hook.sh` with `"shell": "bash"` and no `args`. Exec form was +not viable: it is a bare `PATH` lookup, and on Windows `bash` resolves to the WSL relay +`System32\bash.exe` before Git Bash, so the launch died and — a failed hook launch being non-blocking +— the guard silently enforced nothing. Shell form is resolved by Claude Code itself, which routes it +through its own Git Bash. The security consequence is stated plainly rather than glossed: a shell now +parses the launch string, so "no shell is involved" is no longer the bound. What bounds it instead is +that the string is a **fixed literal** in the plugin's own `hooks.json`, with no model-, repo-, or +session-supplied text interpolated into it; the only substituted values are Claude Code's own +`${CLAUDE_PLUGIN_ROOT}` and `${CLAUDE_PLUGIN_DATA}` placeholders, each double-quoted, so the shell's +re-tokenization reproduces the exec-form argument vector byte-for-byte — verified for both hooks +against roots containing spaces and backslashes. The limit of that quoting is part of the model too: +the runtime substitutes those placeholders *textually* before bash parses the result, so the double +quotes bound whitespace and backslashes but would not neutralize a `$` or a backtick inside a +substituted value (both resolve under Claude Code's own install and data roots). The invariant is +therefore **maintained by test**, not structural — `hooks/run-python-hook.test.sh` asserts the +launcher is named in `command`, `args` is absent, `shell: bash` is declared, and every placeholder is +quoted, and `test_hygiene.py`'s hook helpers read either launch form so a shell-form entry cannot make +an assertion vacuously green. The skill-scoped belt is a separate surface and still launches in exec +form via `python3` (#2568). + **Guard launch/runtime failures are now surfaced, not silently indistinguishable from approval (since 0.9.5, #1416).** A `PreToolUse` hook that fails to launch, or launches and then exits non-zero, denies nothing — Claude Code treats a non-blocking hook result as approval, so "the guard denied nothing because @@ -268,13 +291,17 @@ on any transcript-read or parse error so it can never itself become the reason a this does **not** cover: repo-hygiene ships its own, structurally different guard, verified working independently and out of scope here; the detector's command-substring filter matches only `destructive_guard.py` invocations, so a renamed or unrelated guard script is invisible to it the same -way it is invisible to the engine gate's own coverage marker (see above); it never retroactively +way it is invisible to the engine gate's own coverage marker (see above); and it never retroactively scans a prior session's transcript — only the transcript named by the current `Stop` event's own -`transcript_path`; and it is wired with the same literal `python3` command as the guard it watches, -so the interpreter-resolution fail-open the plugin README documents (the WindowsApps -`python3.exe` alias stub, or a missing/broken `python3`) takes the detector down with the guard and -leaves that one vector unreported. Closing that requires a launcher whose availability does not -depend on the same lookup, which is tracked separately (#1504). +`transcript_path`. Interpreter resolution is no longer one of those gaps: since #1504 both wired hooks +launch through the shared `hooks/run-python-hook.sh`, which tries `python3`, then `python`, then +`py -3`, rejects the zero-length `WindowsApps` alias stub, and — in monitor mode — emits the +`systemMessage` itself when nothing resolves, so a host with no usable Python reports the blind spot +instead of hiding it. What the two still share is that launcher and the shell that starts it: both are +registered in shell form (`"shell": "bash"`, since 0.17.8), so a host where Claude Code cannot start a +bash shell at all takes the guard and its detector down together with nothing left to report it. That +residual is why the registration shape is asserted by `hooks/run-python-hook.test.sh` and verified as +step 1 of `/disk-hygiene:setup check`. A depth-limited scan records every directory it declined to enter in `truncated_paths`. Truncated directories have no captured descendant set, so the preview blocks them (and anything beneath them) diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py index 7b3453935..3a8c1c721 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py @@ -9,6 +9,7 @@ import math import os import re +import shlex import shutil import stat import subprocess @@ -4206,8 +4207,37 @@ def test_skill_hook_passes_plugin_root_flag_matching_constant(self) -> None: self.assertEqual(guard._PLUGIN_ROOT_PLACEHOLDER, args[flag_index + 1]) @staticmethod - def _engine_gate_hook_args() -> list[str]: - """Return the plugin-level engine-gate hook's `args` from hooks.json.""" + def _hook_argv(hook: dict) -> list[str]: + """Effective argument vector for a command hook, in EITHER launch form. + + Selecting on ``args`` alone would silently match nothing once a hook + moves to shell form, turning every assertion built on it into a vacuous + pass — the bug-as-contract shape that let the dead engine gate ship. + Exec form (``args`` present) spawns ``command`` with ``args`` verbatim; + shell form hands the whole ``command`` string to a shell, so ``shlex`` + reproduces the argv the shell builds, quotes and all. + """ + if hook.get("args"): + return [hook["command"], *hook["args"]] + return shlex.split(hook.get("command", "")) + + @classmethod + def _guard_argv_from_hook(cls, hook: dict, script_name: str) -> list[str]: + """Argv the guard script itself receives, launcher indirection removed. + + Both wired hooks route through ``hooks/run-python-hook.sh``, so the + launcher and its own path lead the vector; the guard's argv starts at + its script path. + """ + argv = cls._hook_argv(hook) + for index, token in enumerate(argv): + if token.endswith(script_name): + return argv[index:] + raise AssertionError(f"{script_name} not found in hook argv: {argv}") + + @classmethod + def _engine_gate_hook_args(cls) -> list[str]: + """Return the plugin-level engine-gate hook's guard argv from hooks.json.""" hooks_path = SCRIPT_DIR.parents[2] / "hooks" / "hooks.json" config = json.loads(hooks_path.read_text(encoding="utf-8")) entries = config["hooks"]["PreToolUse"] @@ -4215,11 +4245,10 @@ def _engine_gate_hook_args() -> list[str]: hook for entry in entries for hook in entry.get("hooks", []) - if hook.get("args") - and any("destructive_guard.py" in arg for arg in hook["args"]) + if any("destructive_guard.py" in token for token in cls._hook_argv(hook)) ] assert len(commands) == 1, commands - return commands[0]["args"] + return cls._guard_argv_from_hook(commands[0], "destructive_guard.py") def test_engine_gate_hook_resolves_kill_switch_from_plugin_root_not_user_config( self, @@ -4250,8 +4279,10 @@ def test_engine_gate_hook_resolves_kill_switch_from_plugin_root_not_user_config( def test_skill_hook_interpreter_is_python3_and_resolves(self) -> None: """Lock the guard's launch interpreter and prove it resolves. - The PreToolUse hook runs in exec form, so `command` is resolved on PATH - with no shell. Bare `python` is absent on stock macOS and many Linux + The skill-scoped PreToolUse hook runs in exec form, so `command` is + resolved on PATH with no shell (the two wired hooks in ``hooks/hooks.json`` + are shell form and resolve Python through ``run-python-hook.sh`` instead; + converting this surface is tracked in #2568). Bare `python` is absent on stock macOS and many Linux distros (and a legacy 2.x would crash the guard), which fails the launch open — the guard never intercepts. The static half locks the config at `python3`. The runtime half is the "interpreter actually resolves" probe: @@ -4777,8 +4808,9 @@ def test_declared_hook_timeouts_match_the_watchdog_ceiling(self) -> None: hook.get("timeout") for entry in config["hooks"]["PreToolUse"] for hook in entry.get("hooks", []) - if hook.get("args") - and any("destructive_guard.py" in arg for arg in hook["args"]) + if any( + "destructive_guard.py" in token for token in self._hook_argv(hook) + ) ] self.assertEqual([guard._DECLARED_HOOK_TIMEOUT_SECONDS], declared) diff --git a/plugins/disk-hygiene/skills/setup/SKILL.md b/plugins/disk-hygiene/skills/setup/SKILL.md index 1e2477718..1f02089b2 100644 --- a/plugins/disk-hygiene/skills/setup/SKILL.md +++ b/plugins/disk-hygiene/skills/setup/SKILL.md @@ -25,9 +25,11 @@ report a PASS/FAIL/INFO table with one remediation line per FAIL. When the plugin's toggle is disabled, every prerequisite absence downgrades from FAIL to INFO — a deliberately disabled plugin is not broken. Report the probes informationally and note that re-enabling restores the FAIL semantics. One exception: every step-1 and step-2 failure stays -FAIL with the toggle disabled. Audit-only mode is *enforced by* the guard, both guard surfaces -launch through the literal name `python3`, and a guard that never runs can neither read nor -enforce the configured `false` — so the fail-open is most dangerous in exactly this +FAIL with the toggle disabled. Audit-only mode is *enforced by* the guard, every guard surface +depends on a Python 3 interpreter resolving (the wired hooks through +`hooks/run-python-hook.sh`, the skill-scoped belt through the literal name `python3`), and a +guard that never runs can neither read nor enforce the configured `false` — so the fail-open +is most dangerous in exactly this configuration. That covers every non-`ok` alias-probe verdict (`store-alias-stub`, `indeterminate`, `not-found`), a nominally `ok` resolution whose version probe then fails to launch at all — a corrupt or zero-length binary outside `WindowsApps`, a broken shim, a @@ -38,11 +40,16 @@ the guard's own source: Python 3.6, for example, rejects the guard's non-blocking — the same silent fail-open through a different door. Unproven guard execution fails closed like every other guard-relevant unknown in this plugin. -1. **Bash launcher on `PATH`** — both wired hooks in `hooks/hooks.json` register as the literal - command `bash` before `hooks/run-python-hook.sh` resolves Python. FAIL if `command -v bash` is - empty; on Windows ensure Git Bash (or another real `bash.exe`) precedes stub paths on `PATH`. - When bash is missing, neither the guard nor the Stop detector can launch — the same blind spot - the launcher exists to surface. +1. **Shell-form launcher registration** — both wired hooks in `hooks/hooks.json` must name + `hooks/run-python-hook.sh` directly in `command`, with `"shell": "bash"` and **no `args`**, so + Claude Code routes them through its own Git Bash rather than a `PATH` lookup. FAIL if either + registration carries an `args` key or sets `command` to a bare interpreter name such as `bash`: + that is exec form, which on Windows resolves `bash` to the WSL relay `System32\bash.exe` and + fails to launch — and a failed hook launch is non-blocking, so the guard silently enforces + nothing (#1416). Do **not** report this as a `PATH`-ordering problem: shell form is resolved by + Claude Code, so reordering `PATH` neither causes nor fixes it. Also FAIL if the launcher is + missing or not executable. Report a missing Git Bash on Windows as an environment prerequisite + (shell form falls back to PowerShell there, which cannot run a `.sh`), not as a `PATH` fix. 2. **Python floor on `PATH`** — the interpreter used by scanning, validation, the guard, and cleanup. (The guard registers on two surfaces: a plugin-level engine gate that acts only on engine-referencing commands, and the skill-scoped belt inside the diff --git a/plugins/disk-hygiene/skills/setup/evals/evals.json b/plugins/disk-hygiene/skills/setup/evals/evals.json new file mode 100644 index 000000000..15d6bbe5f --- /dev/null +++ b/plugins/disk-hygiene/skills/setup/evals/evals.json @@ -0,0 +1,77 @@ +{ + "skill_name": "setup", + "evals": [ + { + "id": 1, + "name": "bare-invocation-runs-read-only-check", + "prompt": "/disk-hygiene:setup", + "expected_output": "Runs the read-only check action (the default) and reports a PASS/FAIL/INFO table with one remediation line per FAIL, writing nothing.", + "files": [], + "expectations": [ + "Defaults to check when no argument is given", + "Reads the clean skill and its bundled scripts as the source of truth for prerequisites rather than reciting SKILL.md", + "Makes no configuration change and never prompts when the action is given" + ] + }, + { + "id": 2, + "name": "apply-is-guidance-only", + "prompt": "/disk-hygiene:setup apply", + "expected_output": "Runs the check first, then points at each remediation without writing any consumer-project configuration.", + "files": [], + "expectations": [ + "Runs check before offering remediation", + "Writes nothing — the plugin owns no consumer-project configuration", + "Names the native userConfig toggle as the only tunable" + ] + }, + { + "id": 3, + "name": "exec-form-wired-hook-registration-fails-step-one", + "prompt": "/disk-hygiene:setup check on a machine whose hooks/hooks.json registers the wired hooks as \"command\": \"bash\" with an args array.", + "expected_output": "Step 1 FAILs on the registration shape itself: exec form resolves `bash` on PATH, which on Windows finds the WSL relay System32\\bash.exe, the launch fails, and a failed hook launch is non-blocking so the guard silently enforces nothing.", + "files": [], + "expectations": [ + "Fails on the presence of an args key or a bare interpreter name in command, not on the machine's environment", + "Does NOT report the problem as a PATH-ordering issue and does NOT recommend reordering PATH or setting CLAUDE_CODE_GIT_BASH_PATH", + "Explains that shell form is resolved by Claude Code, so PATH order neither causes nor fixes it" + ] + }, + { + "id": 4, + "name": "missing-git-bash-reported-as-prerequisite-not-path-fix", + "prompt": "/disk-hygiene:setup check on a Windows machine with no Git Bash installed.", + "expected_output": "Reports the absent Git Bash as an environment prerequisite, because shell form falls back to PowerShell there and PowerShell cannot run the .sh launcher.", + "files": [], + "expectations": [ + "Names Git Bash as a prerequisite for shell-form hook routing on Windows", + "Does not prescribe a PATH reordering remedy", + "Still reports the wired-hook registration shape as correct when it is" + ] + }, + { + "id": 5, + "name": "disabled-toggle-keeps-guard-launch-failures-at-fail", + "prompt": "/disk-hygiene:setup check with disk_hygiene_enabled set to false and python3 resolving to the zero-length WindowsApps alias stub.", + "expected_output": "Most prerequisite absences downgrade to INFO under a disabled toggle, but every step-1 and step-2 failure stays FAIL — a guard that never launches cannot read or enforce the configured false, so the fail-open is most dangerous in exactly this configuration.", + "files": [], + "expectations": [ + "Does not downgrade a step-1 or step-2 failure to INFO because the plugin is disabled", + "Treats the store-alias-stub alias-probe verdict as a guard-launch failure", + "Notes that re-enabling restores FAIL semantics for the downgraded probes" + ] + }, + { + "id": 6, + "name": "python-below-floor-fails-closed", + "prompt": "/disk-hygiene:setup check where python3 launches successfully but reports version 3.6.", + "expected_output": "FAILs on the parsed MIN_PYTHON floor from skills/clean/scripts/hygiene.py rather than accepting a successful launch as proof the guard can run.", + "files": [], + "expectations": [ + "Derives the required version from the MIN_PYTHON constant, not a copy in the README", + "Treats a launching-but-too-old interpreter as unproven guard execution and fails closed", + "Explains that a version probe proves only that something executes" + ] + } + ] +}