diff --git a/plugins/disk-hygiene/.claude-plugin/plugin.json b/plugins/disk-hygiene/.claude-plugin/plugin.json index 2c97c83304..30822da735 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.23.0", + "version": "0.23.1", "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 without the complete checkout evidence bundle, 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 4223f5d2a8..9b2ac1ad9d 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -3,6 +3,38 @@ 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.23.1] + +### Changed + +- **The PowerShell engine-gate entry carries `if` filters, matching the Bash entry's + `Bash(*hygiene.py*)`.** Every PowerShell tool call in every session was still launching the + guard to be told it was irrelevant: on a warm interpreter cache that is four `execve` calls + (`bash -c`, the launcher through its `env` shebang, bash, the interpreter) and a 106 KB module + import, counted with `strace -f`. The harness now evaluates the filters before spawning + anything, so a PowerShell call that does not name the engine and does not invoke an interpreter + or call-operator through a variable costs this plugin no process. The 0.21.4 note that a + PowerShell filter "must match every subcommand of a compound command" described allow rules, + not `if`: the harness evaluates `if` through the tool's own permission matcher, and the + PowerShell tool's parses the command AST and runs the hook when any statement, pipeline + element or nested command matches (verified in Claude Code 2.1.258's + `preparePermissionMatcher`: `some` over every collected command, case-insensitive glob; an + unparsable command runs the hook). A mixed line such as `Get-Date; python hygiene.py` + therefore still reaches the guard and is still denied on the PowerShell lane, as are the `|`, + `&&`, newline, CR LF and U+2028 forms. The assignment + `$script = '.../hygiene.py'; python $script scan` is the shape a single + `PowerShell(*hygiene.py*)` filter misses: the matcher evaluates collected command nodes, so + the literal path lives in the assignment and is not part of the later `python $script` + command. Sibling filters `PowerShell(*python*$*)` and `PowerShell(*& $*)` keep that + invocation, and the call-operator form `& $script`, on the guard. No allow/deny decision + changes for a call that reaches the guard. Residuals the filters still cannot see: an engine + reached without its file name in any command node and without an interpreter or call-operator + variable (a symlink or hard link under another name, a Win32 8.3 short name), which the Bash + lane has accepted since 0.21.4; text the PowerShell parser assigns to no command (a comment + naming the engine). The launcher's contract suite gains a kernel-level spawn census + (`strace -f`, skipped where unavailable). The README's hook-budget accounting records the + before and after census. + ## [0.23.0] ### Added diff --git a/plugins/disk-hygiene/README.md b/plugins/disk-hygiene/README.md index 790dc1551c..86c1bee9af 100644 --- a/plugins/disk-hygiene/README.md +++ b/plugins/disk-hygiene/README.md @@ -92,15 +92,7 @@ enforces nothing. The launcher resolves Python itself instead (#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 (since **0.21.4** the gate is registered once per tool: the `Bash` -entry carries the `if` filter `Bash(*hygiene.py*)`, a superset of the gate's own relevance check, -so a Bash command that does not name the engine no longer spawns the Python interpreter to be -deferred, except that a command containing `$()`, a backtick or `$VAR` still spawns it, because -the filter cannot see what the substitution expands to; the `PowerShell` entry carries no `if`, -because an `if` filter is scoped to the tool it names, so a Bash filter would leave every -PowerShell call unguarded, and a PowerShell filter must match every subcommand of a compound -command, which would skip this kill-switch guard silently on a mixed line, so every PowerShell -call still pays the interpreter start); and the skill-scoped **belt** inside the `clean` skill's context, +switch and data-root authority; and the skill-scoped **belt** inside the `clean` skill's context, which adds the deny-by-default Bash and deletion-spelling PowerShell discipline during active cleanup work. Both surfaces resolve the kill switch by reading `disk_hygiene_enabled` from user-scope `pluginConfigs` in `settings.json` (located from `${CLAUDE_PLUGIN_ROOT}`, honored only @@ -303,6 +295,34 @@ measurements below carry the conditions they were taken under. pairs, measured p50 5446 → 1418 ms and p95 16991 → 7874 ms; those absolute values are specific to that contention and are not comparable to the ≈ 190–300 ms figures above, which were taken on a quiet host. Re-measure per the convention's method on a quiet host before citing a new share. + **Superseded in 0.23.1 for every shell call that does not name the engine and does not + invoke it through a variable.** The gate is registered once per tool. Bash carries + `Bash(*hygiene.py*)`. PowerShell carries `PowerShell(*hygiene.py*)` plus + `PowerShell(*python*$*)` and `PowerShell(*& $*)`, because the PowerShell matcher evaluates + collected command nodes and a literal path in `$script = '.../hygiene.py'` is not part of the + later `python $script` (or `& $script`) command. An `if` filter is scoped to the tool it + names, and one `Bash(...)` filter under a `Bash|PowerShell` matcher left every PowerShell + call unguarded. The harness evaluates the filter through the tool's own permission matcher + before it spawns anything, so a Bash or PowerShell call that does not name the engine and + does not invoke an interpreter or call-operator through a variable now costs this plugin + zero processes and zero `execve` calls. Before, on a warm interpreter cache, every + PowerShell call paid four `execve` calls (`bash -c`, the launcher through its `env` + shebang, bash, the interpreter), no fork, and a 106 KB module import, to be told it was + irrelevant; measured with `strace -f` on Linux, where the hook process walled at p50 44 ms + against a `bash -c :` floor of 2 ms (n = 20 per tool), about 22 spawn-equivalents, the cost + class the issue measured as a 2.4 s median on Windows. A call that names the engine, or + invokes python/`&` through a variable, pays that chain unchanged and is judged unchanged. + What the filters still cannot see: for Bash, a command containing `$()`, a backtick or + `$VAR` spawns the guard anyway, because the filter cannot see what the substitution expands + to; for PowerShell, the matcher parses the command and runs the hook when any statement, + pipeline element or nested command matches, so a mixed line such as + `Get-Date; python hygiene.py` still reaches the guard (the every-subcommand rule applies to + allow decisions, not to `if`). Neither filter sees an engine reached without its own file + name in a command node and without an interpreter or call-operator variable, any spelling + such as a symlink or hard link under another name or a Win32 8.3 short name; the gate's + relevance check could catch that case by file identity, and the residual is accepted on + both lanes, as it has been on the Bash lane since 0.21.4, because the engine's own preview + and approval-token containment still answers for it. **0.23.0 delta (local decision record):** the guard now appends one line to `/guard-decisions/decisions.jsonl` on every branch that reaches a verdict. The added trust surface is that one append, to a path under the plugin's own data root and diff --git a/plugins/disk-hygiene/hooks/hooks.json b/plugins/disk-hygiene/hooks/hooks.json index 1972c1b506..2c606707fb 100644 --- a/plugins/disk-hygiene/hooks/hooks.json +++ b/plugins/disk-hygiene/hooks/hooks.json @@ -21,6 +21,23 @@ { "type": "command", "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}\"", + "if": "PowerShell(*hygiene.py*)", + "shell": "bash", + "timeout": 60, + "statusMessage": "Checking the disk-hygiene delete against its authorized roots..." + }, + { + "type": "command", + "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}\"", + "if": "PowerShell(*python*$*)", + "shell": "bash", + "timeout": 60, + "statusMessage": "Checking the disk-hygiene delete against its authorized roots..." + }, + { + "type": "command", + "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}\"", + "if": "PowerShell(*& $*)", "shell": "bash", "timeout": 60, "statusMessage": "Checking the disk-hygiene delete against its authorized roots..." diff --git a/plugins/disk-hygiene/hooks/run-python-hook.test.sh b/plugins/disk-hygiene/hooks/run-python-hook.test.sh index 8ff0975ce7..b9d0c77158 100755 --- a/plugins/disk-hygiene/hooks/run-python-hook.test.sh +++ b/plugins/disk-hygiene/hooks/run-python-hook.test.sh @@ -413,4 +413,49 @@ else echo "SKIP: runnable python3 not available for happy-path probe" >&2 fi +# --- a warm launch creates no process before it execs the interpreter --- +# +# Both engine-gate entries carry an `if` filter, so this launcher now runs only +# for a shell call that names the engine; what such a call pays is the spawn +# chain itself, and a warm launch must add nothing to it. The census is taken at +# the kernel (`strace -f`), not through a PATH shim or `set -x`: a fork that +# never execs (a `$(...)` substitution, a pipeline) is invisible to both, and a +# fork is the unit Windows charges for. Expected on a warm cache: zero +# clone/fork calls, and exactly two execve calls, bash running the launcher and +# the resolved interpreter running the target. Skipped where strace is absent, +# where ptrace is refused, or where `python3` is itself a script (a version +# manager shim), since that shim's own spawns are not this launcher's. +if ! command -v strace >/dev/null 2>&1; then + echo "SKIP: strace not available for the spawn census" >&2 +elif [[ -z "$REAL_PYTHON" ]] || [[ "$(head -c 2 "$REAL_PYTHON" 2>/dev/null)" == "#!" ]]; then + echo "SKIP: python3 is absent or is a shim script; spawn census not exercised" >&2 +elif ! strace -qq -e trace=execve -o /dev/null true >/dev/null 2>&1; then + echo "SKIP: strace cannot trace on this host; spawn census not exercised" >&2 +else + CENSUS_HOME="$PROBE_DIR/census-home" + CENSUS_LOG="$PROBE_DIR/census.strace" + mkdir -p "$CENSUS_HOME" + # Cold launch: resolves and writes the cache record under the census HOME. + HOME="$CENSUS_HOME" bash "$FIXTURE_ROOT/hooks/run-python-hook.sh" \ + "$FIXTURE_TARGET" "$FIXTURE_MARKER" >/dev/null 2>&1 || true + rm -f "$FIXTURE_MARKER" + HOME="$CENSUS_HOME" strace -f -qq -e trace=clone,clone3,fork,vfork,execve \ + -o "$CENSUS_LOG" bash "$FIXTURE_ROOT/hooks/run-python-hook.sh" \ + "$FIXTURE_TARGET" "$FIXTURE_MARKER" >/dev/null 2>&1 || true + assert_eq "the traced warm launch still runs the target" \ + "ran" "$([[ -e "$FIXTURE_MARKER" ]] && printf 'ran' || printf 'skipped')" + # A creation is a clone/fork line that returned a child id; the `unfinished` + # half of a split line is excluded so a creation is counted once. The pattern + # is POSIX ERE: strace writes `clone(`, `clone3(`, `fork(`, `vfork(`; a GNU + # `\b` word boundary is not needed and is not portable. + census_creations="$(grep -E '(clone3?|v?fork)\(' "$CENSUS_LOG" | + grep -v unfinished | grep -cE '= [1-9][0-9]*$' || true)" + census_execs="$(grep -E '^[0-9]+ +execve\(' "$CENSUS_LOG" | + grep -cE '\) = 0$' || true)" + assert_eq "a warm launch creates no process before exec (strace census)" \ + "0" "$census_creations" + assert_eq "a warm launch execs exactly bash and the interpreter (strace census)" \ + "2" "$census_execs" +fi + pass "all run-python-hook contract checks" diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py index cc3f32c8d2..d03b3df2ed 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py @@ -3,6 +3,7 @@ from __future__ import annotations +import fnmatch import importlib.util import io import json @@ -6930,9 +6931,9 @@ def _engine_gate_hook_args(cls) -> list[str]: for hook in entry.get("hooks", []) if any("destructive_guard.py" in token for token in cls._hook_argv(hook)) ] - # One registration per tool (``Bash`` with an ``if`` filter, ``PowerShell`` - # without one), both carrying the same guard argv. - assert len(commands) == 2, commands + # One Bash registration plus the PowerShell filename filter and the + # two variable-invocation filters; every row carries the same guard argv. + assert len(commands) == 4, commands argvs = { tuple(cls._guard_argv_from_hook(hook, "destructive_guard.py")) for hook in commands @@ -6940,27 +6941,170 @@ def _engine_gate_hook_args(cls) -> list[str]: assert len(argvs) == 1, argvs return list(argvs.pop()) + # Glob bodies the engine-gate ``if`` filters carry. Bash uses the filename + # glob only. PowerShell adds interpreter-with-variable and call-operator- + # with-variable globs so an assignment that holds the engine path still + # reaches the guard. + _POWERSHELL_ENGINE_GATE_GLOBS = ( + "*hygiene.py*", + "*python*$*", + "*& $*", + ) + _POWERSHELL_ASSIGNMENT = re.compile(r"^\s*\$[\w:.]+\s*=") + def test_engine_gate_is_registered_once_per_tool(self) -> None: """Lock the per-tool registration shape of the plugin-level engine gate. An ``if`` filter is scoped to the tool it names: under a single ``Bash|PowerShell`` matcher, ``Bash(...)`` filtered every PowerShell call - out of this kill-switch guard. The ``Bash`` entry keeps the filter; the - ``PowerShell`` entry carries none, because a PowerShell filter must match - every subcommand of a compound command and would skip the guard silently - on a mixed line. + out of this kill-switch guard. Each tool therefore has its own matcher + entry. Bash carries the filename glob. PowerShell carries that glob plus + the two variable-invocation globs, because the harness evaluates ``if`` + through the tool's own permission matcher, and the PowerShell tool's + parses the command AST and runs the hook when ANY statement, pipeline + element or nested command matches (Claude Code 2.1.258, the PowerShell + tool's ``preparePermissionMatcher``: ``some`` over every collected + command, case-insensitive glob; an unparsable command runs the hook). + An assignment is not a collected command, so ``$script = '.../hygiene.py'; + python $script`` would miss a filename-only filter. The every-subcommand + rule belongs to allow decisions, not to ``if``. """ hooks_path = SCRIPT_DIR.parents[2] / "hooks" / "hooks.json" config = json.loads(hooks_path.read_text(encoding="utf-8")) - by_matcher = { - entry.get("matcher"): hook - for entry in config["hooks"]["PreToolUse"] - for hook in entry.get("hooks", []) - if any("destructive_guard.py" in token for token in self._hook_argv(hook)) - } + by_matcher: dict[str, list[dict]] = {} + for entry in config["hooks"]["PreToolUse"]: + matcher = entry.get("matcher") + for hook in entry.get("hooks", []): + if any("destructive_guard.py" in token for token in self._hook_argv(hook)): + by_matcher.setdefault(matcher, []).append(hook) self.assertEqual({"Bash", "PowerShell"}, set(by_matcher)) - self.assertTrue(by_matcher["Bash"].get("if", "").startswith("Bash(")) - self.assertNotIn("if", by_matcher["PowerShell"]) + self.assertEqual( + ["Bash(*hygiene.py*)"], + [hook.get("if") for hook in by_matcher["Bash"]], + ) + self.assertEqual( + [ + "PowerShell(*hygiene.py*)", + "PowerShell(*python*$*)", + "PowerShell(*& $*)", + ], + [hook.get("if") for hook in by_matcher["PowerShell"]], + ) + + _IF_STATEMENT_SPLIT = re.compile(r"\r?\n|\u2028|\u2029|;|\|\||&&|\|") + + @classmethod + def _powershell_if_admits(cls, command: str) -> bool: + """Reference of the harness's PowerShell ``if`` across the three globs. + + Mirrors Claude Code 2.1.258: the command is split into statements and + pipeline elements, assignment statements are dropped (the real evaluator + walks the AST and does not treat ``$script = '...'`` as a command node), + each remaining element's text is whitespace-normalised, and each glob is + tried against each element; any match runs the hook. + """ + parts = [] + for part in cls._IF_STATEMENT_SPLIT.split(command): + stripped = re.sub(r"[ \t]+", " ", part.strip()) + if not stripped or cls._POWERSHELL_ASSIGNMENT.match(stripped): + continue + parts.append(stripped) + return any( + fnmatch.fnmatch(part.casefold(), glob.casefold()) + for part in parts + for glob in cls._POWERSHELL_ENGINE_GATE_GLOBS + ) + + def test_powershell_if_filter_skips_only_calls_the_gate_would_defer( + self, + ) -> None: + """A PowerShell call the ``if`` filter skips is one the gate deferred anyway. + + The plugin-level gate acts only on ``_engine_gate_relevant`` commands + and defers everything else with no output BEFORE any deletion spelling + is consulted, so a filter that admits every relevant command changes no + decision. Deletion spellings that never name the engine are the cases + that look like a loss and are not: the engine gate never judged them + (the skill-scoped belt does, and it carries no filter). The zero-width + and split spellings are the marker broken in ways the guard's own token + split also does not read as the engine. + """ + skipped = [ + "Get-ChildItem -Force", + "git status --short", + "Remove-Item -Recurse -Force C:\\temp\\build", + "rm -rf ./node_modules", + "Get-Date; Remove-Item .\\x.log", + "[System.IO.File]::Delete('C:\\temp\\a.txt')", + "python hygiene\u200b.py --scan", + "python hygiene .py", + ] + for command in skipped: + with self.subTest(command=command): + self.assertFalse(self._powershell_if_admits(command)) + self.assertFalse(guard._engine_gate_relevant(command, "PowerShell")) + + def test_powershell_if_filter_admits_every_engine_invocation_shape( + self, + ) -> None: + """A PowerShell call that names the engine still reaches the guard and denies. + + Mixed lines are the case the earlier unfiltered registration feared: a + statement separator, a pipeline, a PowerShell 7 chain operator, a + newline, a CR LF pair, U+2028, the call operator, a nested + ``pwsh -Command`` payload, run-together whitespace and an upper-case + spelling all keep the marker inside a statement the harness matches, + and each still denies on the PowerShell lane. + """ + admitted = [ + "python hygiene.py --scan", + "PYTHON HYGIENE.PY --scan", + "Get-Date; python hygiene.py --scan", + "python hygiene.py --scan | Out-Null", + "Get-Date && python hygiene.py --scan", + "Get-Date\r\npython hygiene.py --scan", + "Get-Date\npython hygiene.py --scan", + "python hygiene.py\u2028Get-Date", + "& python .\\hygiene.py --scan", + 'pwsh -Command "python hygiene.py --scan"', + "python hygiene.py\t--scan", + ] + for command in admitted: + with self.subTest(command=command): + self.assertTrue(self._powershell_if_admits(command)) + self.assertTrue(guard._engine_gate_relevant(command, "PowerShell")) + verdict = guard.powershell_decision(command, True) + self.assertIsNotNone(verdict) + self.assertEqual("deny", verdict[0]) + + def test_powershell_if_filter_admits_variable_script_invocations(self) -> None: + """A script path held in a variable still reaches the guard. + + The PowerShell matcher evaluates collected command nodes, so + ``$script = '.../hygiene.py'; python $script scan`` puts the literal + path in the assignment and not in the later ``python $script`` command. + The filename glob misses that command node; the interpreter-with-variable + and call-operator-with-variable globs keep it on the guard. The unfiltered + gate treats the same payload as ``_engine_gate_relevant`` and denies it. + """ + engine = (SCRIPT_DIR / "hygiene.py").as_posix() + admitted = [ + f"$script = '{engine}'; python $script scan", + f"$script = '{engine}'; python3 $script scan", + f"$script = '{engine}'; & $script scan", + ] + for command in admitted: + with self.subTest(command=command): + self.assertTrue(self._powershell_if_admits(command), command) + self.assertTrue( + guard._engine_gate_relevant(command, "PowerShell"), command + ) + verdict = guard.powershell_decision(command, True) + self.assertIsNotNone(verdict, command) + self.assertEqual("deny", verdict[0], command) + for command_node in ("python $script scan", "& $script scan"): + with self.subTest(command_node=command_node): + self.assertTrue(self._powershell_if_admits(command_node), command_node) def test_engine_gate_hook_resolves_kill_switch_from_plugin_root_not_user_config( self, @@ -7792,8 +7936,10 @@ def test_declared_hook_timeouts_match_the_watchdog_ceiling(self) -> None: for hook in entry.get("hooks", []) if any("destructive_guard.py" in token for token in self._hook_argv(hook)) ] - # One registration per tool, both declaring the same timeout. - self.assertEqual(2, len(declared), declared) + # Bash has one matcher; PowerShell has several (literal engine path + # plus variable-based invocations). Every registration that launches + # the guard must declare the same timeout the watchdog clamp uses. + self.assertGreaterEqual(len(declared), 2, declared) self.assertEqual({guard._DECLARED_HOOK_TIMEOUT_SECONDS}, set(declared)) skill_text = (SCRIPT_DIR.parent / "SKILL.md").read_text(encoding="utf-8")