From bd50ada781dde4fd3333c3a4c5a1e6a589502ac9 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:56:49 -0400 Subject: [PATCH 1/6] fix(disk-hygiene): resolve kill switch by reading user settings directly The disk_hygiene_enabled kill switch was inert on a default install: the plugin-level engine-gate hook carried a bare ${user_config.disk_hygiene_enabled} argument, and an unset-but-defaulted userConfig token drops the whole hook entry (upstream default unimplemented, #46477/#39455/#39827). Audit-only mode degraded from deny-outright to prompt-gated. Both guard surfaces now resolve the toggle by reading user-scope pluginConfigs from settings.json through a shared lib/killswitch_config.py reader, located from the tamper-resistant ${CLAUDE_PLUGIN_ROOT} (honored only from user/managed scope since CC 2.1.207, so a repo cannot forge it), ignoring the environment, failing closed to enabled. kill_switch_probe.py now delegates to the same reader. Supersedes the planned SessionStart+state-file delivery: both surfaces are one script through one resolve point, so a direct read is a smaller trust surface (a settings read, no state-file write) and honors mid-session changes. Closes #1019. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../disk-hygiene/.claude-plugin/plugin.json | 2 +- plugins/disk-hygiene/CHANGELOG.md | 40 ++ plugins/disk-hygiene/README.md | 64 ++- plugins/disk-hygiene/hooks/hooks.json | 4 +- plugins/disk-hygiene/lib/killswitch_config.py | 193 ++++++++ plugins/disk-hygiene/skills/clean/SKILL.md | 57 +-- .../skills/clean/reference/safety-model.md | 69 +-- .../skills/clean/scripts/destructive_guard.py | 94 +++- .../skills/clean/scripts/test_hygiene.py | 457 +++++++++++------- plugins/disk-hygiene/skills/setup/SKILL.md | 8 +- .../skills/setup/scripts/kill_switch_probe.py | 177 +------ 11 files changed, 703 insertions(+), 462 deletions(-) create mode 100644 plugins/disk-hygiene/lib/killswitch_config.py diff --git a/plugins/disk-hygiene/.claude-plugin/plugin.json b/plugins/disk-hygiene/.claude-plugin/plugin.json index 16c0c67c78..bc3a76d895 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.8.3", + "version": "0.9.0", "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 50efc66a7d..883947d20e 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -3,6 +3,46 @@ 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.9.0] + +### Fixed + +- **The `disk_hygiene_enabled` kill switch now enforces on both guard surfaces — closing the + inert-by-default engine gate (#1019).** Through 0.8.3 the plugin-level engine gate (`hooks/hooks.json`) + carried a bare `${user_config.disk_hygiene_enabled}` argument. Because the declared userConfig `default` + is unimplemented upstream (#46477 / #39455 / #39827), an unset-but-defaulted token dropped the whole hook + entry, so on a default install the gate never ran; the skill-frontmatter belt could not receive the value + either (skill hooks get neither the `${user_config.*}` substitution nor `CLAUDE_PLUGIN_OPTION_*`). Audit-only + mode therefore degraded from deny-outright to prompt-gated. Both surfaces now resolve the toggle by + **reading it directly** from user-scope `pluginConfigs` in `settings.json`, so a configured `false` is + denied outright on the Bash engine lane and the PowerShell deletion lane, whether or not the clean skill + is active. + +### Changed + +- **Kill-switch delivery is a settings read, not a hook argument or environment variable.** The engine gate + drops its `${user_config.*}` argument (fixing the hook-drop) and both surfaces call the new shared + `lib/killswitch_config.py` reader. The user `settings.json` is located from the tamper-resistant + `${CLAUDE_PLUGIN_ROOT}` both surfaces already receive (falling back to `CLAUDE_CONFIG_DIR`/`HOME` only + when `--plugin-root` is absent, e.g. the report CLI or unit tests). The environment is deliberately not + consulted for the toggle or the settings path: a repo `.claude/settings.json` `env` block reaches hook + subprocesses and carries no provenance, and since Claude Code 2.1.207 `pluginConfigs` is honored only from + user, managed, and `--settings` scope (project/local ignored), so a hostile repo cannot forge the value. + Every absent, unreadable, or ambiguous read fails **closed to enabled**. +- **`kill_switch_probe.py` now delegates to the shared reader** (its behavior and single-line JSON output + contract unchanged) so the report-only probe and the guard resolve the switch one way, not two. +- Docs corrected across `clean`/`setup` `SKILL.md`, `reference/safety-model.md`, and `README.md`: the + "engine gate is inert until configured" and "audit-only reaches only the model, not the guard" caveats + are removed; the guard is again the audit-only backstop. + +### Design note + +- This supersedes the planned SessionStart-hook + state-file delivery ("C′"). Both guard surfaces are the + same script funnelling through one resolve point, so there is nothing to distribute between sessions or + surfaces: a direct read is a smaller trust surface (a settings *read*, no state-file *write*), honors a + mid-session settings change, and needs no session-start timing dependency. Semantics are unchanged from + the locked resolver decision — read user-scope `pluginConfigs`, ignore env, fail closed to enabled. + ## [0.8.3] ### Fixed diff --git a/plugins/disk-hygiene/README.md b/plugins/disk-hygiene/README.md index 96ac7e0567..12956533b8 100644 --- a/plugins/disk-hygiene/README.md +++ b/plugins/disk-hygiene/README.md @@ -51,15 +51,14 @@ at preview. Backups remain the recovery boundary for user data. 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. 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 configured kill switch and - data-root authority through plugin-hook substitution. **Caveat (verified on Claude Code 2.1.218):** - that gate only registers once `disk_hygiene_enabled` is **explicitly configured**. Upstream never - implemented the declared userConfig `default`, so while the option is unset its - `${user_config.disk_hygiene_enabled}` argument is neither substituted nor exported, and its - presence **drops the whole hook entry** — the gate does not run at all, on either tool. The - skill-scoped belt below carries no such token and is unaffected. 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. Hook-lifetime caveat: docs + 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 + 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 from user/managed/`--settings` + scope since Claude Code 2.1.207, so a repo cannot forge it), register unconditionally, and fail + closed to enabled — the earlier bare-`${user_config.*}` argument that dropped the engine gate on a + default install is gone (since 0.9.0). Hook-lifetime caveat: docs scope a skill hook to the component's lifetime, but session-long firing of the belt has been observed on at least one Claude Code build (producer-reported; see issue #1105) — if unrelated commands are denied after a clean run ends, start a new session and see that issue. PreToolUse @@ -153,30 +152,29 @@ hand-cleaning the zone. - **MCP / external trust:** no MCP server, agent, dependency, or third-party service is shipped. - **Configuration:** one non-sensitive `userConfig` boolean (`disk_hygiene_enabled`, default `true`) gating the execution tiers — setting it `false` puts `/disk-hygiene:clean` in audit-only - mode. When the value is **explicitly configured `false`**, the plugin-level engine gate receives it - by exec-form substitution and denies engine invocations outright. **Caveat (verified on Claude Code - 2.1.218):** this holds only for a configured value — because upstream never implemented the declared - userConfig `default`, an *unset* `disk_hygiene_enabled` is neither substituted nor exported, and its - presence in the gate's args drops the whole hook, so on a default (unconfigured) install the engine - gate does not run at all. The skill self-enforcement (kill-switch probe + skill-content value) is - therefore the primary kill-switch honoring on a default install, not a redundant layer; the - skill-scoped belt cannot receive the value either (skill-frontmatter hooks get neither - `${user_config.*}` substitution nor `CLAUDE_PLUGIN_OPTION_*`) and still forces a human prompt before - every mutation. -- **Trust-surface record (0.7.0):** the plugin-level `hooks/hooks.json` PreToolUse registration is a - NEW trust surface (a hook that launches in every consumer session **once `disk_hygiene_enabled` is - explicitly configured** — see the caveat below), 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 **on a configured install**, 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). **Caveat (verified on Claude Code 2.1.218):** while `disk_hygiene_enabled` is - unset, the bare `${user_config.*}` argument drops the whole hook, so on a default install this hook - does not register or launch at all — neither the trust surface nor its per-call cost applies until - the option is configured. 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 + mode. Both guard surfaces resolve the toggle by reading `disk_hygiene_enabled` from user-scope + `pluginConfigs` in `settings.json` (not the process environment). A configured `false` denies Bash + engine invocations outright on the always-on engine gate (whether or not the clean skill is active); + PowerShell deletion spellings are denied outright by the skill-scoped belt while `/disk-hygiene:clean` + is active (the always-on gate defers on non-engine commands). The read is honored only from user, managed, and + `--settings` scope (Claude Code 2.1.207+), so a project or local repo `settings.json` cannot flip + it; the file is located from `${CLAUDE_PLUGIN_ROOT}`, not from repo-redirectable environment. An + absent or unreadable value fails closed to enabled. 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 + 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.*}` + 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`. The + added trust surface is that settings-file *read* — bounded to a single user-scope `pluginConfigs` + value, located from `${CLAUDE_PLUGIN_ROOT}`, no write. 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 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 19e14f5edf..9b4396bcb7 100644 --- a/plugins/disk-hygiene/hooks/hooks.json +++ b/plugins/disk-hygiene/hooks/hooks.json @@ -14,9 +14,7 @@ "--plugin-root", "${CLAUDE_PLUGIN_ROOT}", "--authorized-data-root", - "${CLAUDE_PLUGIN_DATA}", - "--disk-hygiene-enabled", - "${user_config.disk_hygiene_enabled}" + "${CLAUDE_PLUGIN_DATA}" ] } ] diff --git a/plugins/disk-hygiene/lib/killswitch_config.py b/plugins/disk-hygiene/lib/killswitch_config.py new file mode 100644 index 0000000000..6afb1bd508 --- /dev/null +++ b/plugins/disk-hygiene/lib/killswitch_config.py @@ -0,0 +1,193 @@ +#!/usr/bin/env python3 +"""Single source of truth for reading the ``disk_hygiene_enabled`` kill switch. + +Claude Code stores merged plugin options under +``pluginConfigs[].options`` in the user ``settings.json`` and, since +v2.1.207, reads that key back from user settings, the ``--settings`` flag, and +managed settings only — entries in a project's ``.claude/settings.json`` or +``.claude/settings.local.json`` are ignored +(https://code.claude.com/docs/en/plugins-reference, "User configuration"). That +makes the user settings file the one repo-tamper-resistant channel for a +safety toggle, so both the report-only ``kill_switch_probe`` and the +destructive-action guard resolve the switch by reading it here rather than +trusting the process environment (a repo ``settings.json`` ``env`` block reaches +hook subprocesses and carries no provenance). + +``probe()`` returns a full provenance report for the setup skill's honest +"configured vs assumed" reporting; ``resolve_effective()`` is the boolean the +guard needs. Every absent/indeterminate read yields ``effective=True`` — the +switch fails **closed to enabled** (safety on): the guard stays active and gates +every mutation behind a human prompt even when it cannot confirm a configured +value. +""" + +from __future__ import annotations + +import json +import os +import stat +from pathlib import Path + +PLUGIN_NAME = "disk-hygiene" +OPTION_KEY = "disk_hygiene_enabled" + + +def default_settings_path() -> Path: + """The user settings file, honoring ``CLAUDE_CONFIG_DIR`` when relocated.""" + config_dir = os.environ.get("CLAUDE_CONFIG_DIR") + base = Path(config_dir) if config_dir else Path.home() / ".claude" + return base / "settings.json" + + +def _matches_plugin(key: str) -> bool: + return key == PLUGIN_NAME or key.startswith(f"{PLUGIN_NAME}@") + + +def _interpret(value: object) -> bool | None: + """Return the boolean meaning of a stored option value, or None if invalid.""" + if isinstance(value, bool): + return value + if isinstance(value, str): + lowered = value.strip().lower() + if lowered == "true": + return True + if lowered == "false": + return False + return None + + +def _report( + effective: bool, + source: str, + degraded: bool, + detail: str, + settings_path: Path, + entries: list[dict[str, object]], +) -> dict[str, object]: + return { + "effective": effective, + "source": source, + "degraded": degraded, + "detail": detail, + "settings_path": str(settings_path), + "entries": entries, + } + + +def probe(settings_path: Path) -> dict[str, object]: + """Read the effective kill switch from ``settings_path`` with provenance.""" + try: + settings_stat = settings_path.stat() + except FileNotFoundError: + return _report( + True, + "default", + False, + f"No settings file at {settings_path}; the toggle is not configured " + "there and the plugin default (enabled) applies. Managed settings or " + "a --settings flag could still carry a value this probe cannot see.", + settings_path, + [], + ) + except OSError as exc: + return _report( + True, + "indeterminate", + True, + f"Could not inspect {settings_path} ({exc}); assuming the default " + "(enabled). This is an assumption, not the configured value.", + settings_path, + [], + ) + if not stat.S_ISREG(settings_stat.st_mode): + return _report( + True, + "indeterminate", + True, + f"{settings_path} exists but is not a regular file; assuming the " + "default (enabled). This is an assumption, not the configured value.", + settings_path, + [], + ) + try: + settings = json.loads(settings_path.read_text(encoding="utf-8")) + if not isinstance(settings, dict): + raise ValueError("settings root is not a JSON object") + except (OSError, UnicodeDecodeError, ValueError) as exc: + return _report( + True, + "indeterminate", + True, + f"Could not read the configured toggle from {settings_path} ({exc}); " + "assuming the default (enabled). This is an assumption, not the " + "configured value.", + settings_path, + [], + ) + + plugin_configs = settings.get("pluginConfigs") + entries: list[dict[str, object]] = [] + if isinstance(plugin_configs, dict): + for key in sorted(plugin_configs): + if not _matches_plugin(key): + continue + entry = plugin_configs.get(key) + options = entry.get("options") if isinstance(entry, dict) else None + if not isinstance(options, dict) or OPTION_KEY not in options: + continue + entries.append({"key": key, "value": options[OPTION_KEY]}) + + if not entries: + return _report( + True, + "default", + False, + f"{settings_path} carries no {PLUGIN_NAME} {OPTION_KEY} entry; the " + "toggle is not configured there and the plugin default (enabled) " + "applies. Managed settings or a --settings flag could still carry a " + "value this probe cannot see.", + settings_path, + entries, + ) + + interpreted = {entry["key"]: _interpret(entry["value"]) for entry in entries} + values = set(interpreted.values()) + if None in values: + return _report( + True, + "indeterminate", + True, + "A configured value is not a recognizable boolean " + f"({json.dumps({k: e['value'] for k, e in zip(interpreted, entries)})}); " + "assuming the default (enabled). This is an assumption, not the " + "configured value.", + settings_path, + entries, + ) + if len(values) > 1: + return _report( + True, + "indeterminate", + True, + "Multiple disk-hygiene entries disagree on the toggle " + f"({json.dumps(interpreted)}); assuming the default (enabled). This " + "is an assumption, not the configured value.", + settings_path, + entries, + ) + effective = values.pop() + assert effective is not None + return _report( + effective, + "configured", + False, + f"{OPTION_KEY} is configured {str(effective).lower()} in " + f"{settings_path}.", + settings_path, + entries, + ) + + +def resolve_effective(settings_path: Path) -> bool: + """The boolean kill switch: ``probe()``'s effective value, closed to enabled.""" + return bool(probe(settings_path)["effective"]) diff --git a/plugins/disk-hygiene/skills/clean/SKILL.md b/plugins/disk-hygiene/skills/clean/SKILL.md index e7f3b5d057..3eebfe2138 100644 --- a/plugins/disk-hygiene/skills/clean/SKILL.md +++ b/plugins/disk-hygiene/skills/clean/SKILL.md @@ -48,12 +48,13 @@ unless bounded with `--max-depth` or confirmed with `--confirmed-large-scan`. running the bundled probe (the guard allows exactly this argument-free shape): `"" "${CLAUDE_PLUGIN_ROOT}/skills/setup/scripts/kill_switch_probe.py"` and honor the `effective` value it reports; on `degraded: true` proceed as enabled but say the configured - value could not be read. Honoring that value is your responsibility: a skill-frontmatter hook - receives neither the `${user_config.*}` substitution nor the - `CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED` environment variable, so the guard cannot independently - enforce audit-only mode — it stays active and still forces a human prompt before every mutation, - but a configured `false` reaches only you, not the guard. Do not treat the guard as the kill - switch's backstop here. The hook runs in shell-free exec form and reports its absolute Python + value could not be read. The guard now enforces this independently: it resolves the same + `disk_hygiene_enabled` toggle by reading it straight from your user `settings.json` (the read is + shared with this probe, and the settings file is located from the tamper-resistant + `${CLAUDE_PLUGIN_ROOT}` — not the environment), so in audit-only mode it denies every mutation lane + 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 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 @@ -263,13 +264,11 @@ handoff, not an engine plan: The PowerShell guard lane turns deletion spellings into a final human permission prompt (the same bar as the engine apply prompt); confirm that prompt only when the command matches the exact -approved list. Engine invocations from PowerShell stay hard-denied. **Caveat — the plugin-level engine -gate is inert until `disk_hygiene_enabled` is explicitly configured (verified on Claude Code 2.1.218):** -that gate (`hooks/hooks.json`) passes a bare `${user_config.disk_hygiene_enabled}`, and an -unset-but-defaulted userConfig value drops the whole hook entry, so for any consumer who never set the -key the gate never runs — on the Bash tool and the PowerShell tool alike. The skill-scoped belt (this -skill's frontmatter hook) carries no such token and is unaffected; `Bash|PowerShell` PreToolUse hooks do -fire for the PowerShell tool. See `reference/safety-model.md`. +approved list. Engine invocations from PowerShell stay hard-denied. The plugin-level engine gate +(`hooks/hooks.json`) now registers unconditionally and resolves the kill switch itself by reading +`disk_hygiene_enabled` from your user `settings.json`; it no longer carries a `${user_config.*}` +argument, so the unset-default hook-drop that once made it inert on a default install is gone. +`Bash|PowerShell` PreToolUse hooks fire for the PowerShell tool. See `reference/safety-model.md`. Summarize removed paths, logical bytes removed, observed free-space delta, and every skip grouped by `locked`, `changed-or-link`, `protected`, `needs-elevation`, `handle-state-unverified`, or @@ -299,13 +298,13 @@ sparse files, hard links, compression, and delayed allocation affect it. apply shapes using the hook runtime's same absolute executable pass. Shell expansions, globs, splitting/escape forms, operators, redirections, aliases, and exported functions fail closed. - The guard registers twice: a plugin-level engine gate (`hooks/hooks.json`, `--mode engine-gate`) - that receives the kill switch and data root by plugin-hook substitution and defers instantly on any - command not referencing the engine; and this skill's frontmatter belt, which adds the deny-by-default - Bash and deletion-spelling PowerShell discipline while cleanup is the active work. Verdicts are - idempotent where both fire. **Caveat (verified on Claude Code 2.1.218):** the engine gate only - registers once `disk_hygiene_enabled` is explicitly configured — its bare `${user_config.*}` argument - drops the whole hook while the option is unset (upstream never implemented the declared `default`), so - on a default install only the frontmatter belt runs, and only during `clean`. + that receives the data root by plugin-hook substitution and defers instantly on any command not + referencing the engine; and this skill's frontmatter belt, which adds the deny-by-default Bash and + deletion-spelling PowerShell discipline while cleanup is the active work. Both resolve the kill switch + the same single way — reading `disk_hygiene_enabled` from user-scope `pluginConfigs` in + `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 @@ -319,14 +318,16 @@ sparse files, hard links, compression, and delayed allocation affect it. manual-handoff lane already requires and the consumer's baseline permission policy — defense-in-depth lost, not preserved. `/disk-hygiene:setup check` reports whether the interpreter resolves on this machine. -- **The plugin-level engine gate is dropped whenever `disk_hygiene_enabled` is unconfigured (Claude Code - 2.1.218) — distinct from the `python3`-resolution loss above.** `hooks/hooks.json` passes a bare - `${user_config.disk_hygiene_enabled}`; a declared userConfig `default` is not implemented upstream, so an - unset-but-defaulted token is neither substituted nor exported to `CLAUDE_PLUGIN_OPTION_*` and its presence - **drops the whole hook entry**. Fresh-session controlled test: token-carrying hooks vanish while token-free - controls fire, and return once the key is configured. Consequence: for any consumer who never set the key, - this gate has never run — on Bash and PowerShell alike. The skill-scoped belt carries no such token and is - unaffected. Recheck when the upstream `default` gap closes (#46477 / #39455 / #39827). +- **The kill switch is delivered by reading user settings, not by a hook argument (since 0.9.0).** Earlier + versions passed a bare `${user_config.disk_hygiene_enabled}` in `hooks/hooks.json`; a declared userConfig + `default` is not implemented upstream (#46477 / #39455 / #39827), so an unset-but-defaulted token was + neither substituted nor exported to `CLAUDE_PLUGIN_OPTION_*`, and its presence **dropped the whole hook + entry** — making the engine gate inert for any consumer who never set the key. The gate no longer carries + a `${user_config.*}` token; both the gate and the belt resolve `disk_hygiene_enabled` by reading it from + user-scope `pluginConfigs` in `settings.json`. Claude Code honors that key only from user, managed, and + `--settings` scope since 2.1.207 (a project/local `settings.json` is ignored), so a hostile repo cannot + forge it; the file is located from `${CLAUDE_PLUGIN_ROOT}` rather than the environment, which a repo `env` + block could redirect. Absent or unreadable settings fail closed to enabled. - **PreToolUse hooks DO fire for the PowerShell tool** (2.1.218; payload `tool_name` is literally `PowerShell`, confirmed by a live block through that tool). A `Bash|PowerShell` matcher is correct and there is no harness firing divergence — read `tool_name` from the stdin payload, not from an env var diff --git a/plugins/disk-hygiene/skills/clean/reference/safety-model.md b/plugins/disk-hygiene/skills/clean/reference/safety-model.md index 0d3180b73e..046723a1c8 100644 --- a/plugins/disk-hygiene/skills/clean/reference/safety-model.md +++ b/plugins/disk-hygiene/skills/clean/reference/safety-model.md @@ -144,41 +144,42 @@ switch. When the guard sees execution enabled they are downgraded to a final hum when it sees a configured `false` (audit-only mode) they are denied outright, so the kill switch would block deletions on the PowerShell lane too and not only the Bash engine apply. -**Caveat — the plugin-level engine gate is inert by default (verified on Claude Code 2.1.218).** The -gate (`hooks/hooks.json`, exec form) passes a bare `${user_config.disk_hygiene_enabled}`. A declared -userConfig `default` is **not implemented**: an unset-but-defaulted `${user_config.*}` is neither -substituted nor exported as `CLAUDE_PLUGIN_OPTION_*`, and its presence in an exec-form arg **drops the -entire hook entry**. (Fresh-session controlled test: the token-carrying hooks vanish while token-free -control hooks fire, and reappear unchanged once the key is configured.) So for every consumer who never -explicitly set `disk_hygiene_enabled`, this engine gate has never run — on the Bash tool and the -PowerShell tool alike, which is the real shape of the "PowerShell bypass" originally reported. - -The **skill-scoped belt is unaffected** — it carries no `${user_config.*}` token — and PreToolUse hooks -with a `Bash|PowerShell` matcher **do** fire for the PowerShell tool on 2.1.218 (payload `tool_name` is -literally `PowerShell`, confirmed by a live block through that tool). There is no harness firing -divergence. **Recheck** when the upstream `userConfig` `default` gap is fixed (#46477 / #39455 / #39827), -which would let the gate resolve its declared default instead of dropping. - -Kill-switch enforcement is only as reachable as the value is, and the guard now registers on two -surfaces with different reach. The **plugin-level engine gate** (`hooks/hooks.json`, exec form, -`--mode engine-gate`) receives `${user_config.disk_hygiene_enabled}` and `${CLAUDE_PLUGIN_DATA}` -by substitution — channels Claude Code documents for plugin hooks — so **when the value is explicitly -configured** `false` is guard-enforced against every engine invocation, whether or not the clean skill -is active. **Caveat (verified on Claude Code 2.1.218):** that reach exists only for a configured value. -Upstream never implemented the declared userConfig `default`, so while `disk_hygiene_enabled` is unset -its `${user_config.*}` argument is neither substituted nor exported and its presence **drops the whole -engine-gate hook** — on a default install the gate does not register at all, and this surface enforces -nothing (the skill's own kill-switch probe + skill-content value become the only honoring path). Recheck -when the upstream gap closes (#46477 / #39455 / #39827). The gate defers instantly (no output) for any command that does not reference the engine, -so it never taxes unrelated work; its coverage marker is the engine script name, a belt against +**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 +resolve `disk_hygiene_enabled` the same single way: by reading it from user-scope `pluginConfigs` in +`settings.json`, 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. Claude +Code honors that key only from user, managed, and `--settings` scope since 2.1.207 — a project or local +`.claude/settings.json` is ignored — so a hostile repo cannot flip it. The settings file is located from +`${CLAUDE_PLUGIN_ROOT}` (the plugin's true install path, which a repo cannot forge), not from +`CLAUDE_CONFIG_DIR`/`HOME`, which a repo `settings.json` `env` block could redirect. When the value +resolves `false` (audit-only mode), `false` is guard-enforced — denied outright, not merely prompted — but +the two surfaces reach different lanes. The **always-on engine gate** enforces it against every Bash +engine invocation **whether or not the clean skill is active**; it defers (no output) on any command that +does not reference the engine, so it does **not** see PowerShell deletion spellings. Those are enforced by +the **skill-scoped belt** (`powershell_decision`) — denied outright in audit-only — only **while the clean +skill is active**. An absent, unreadable, or ambiguous read fails **closed to enabled**: the guard stays +active and forces a human prompt before every mutation, so an unreadable toggle never silently disables +the guard. + +This replaces the earlier delivery, where the gate carried a bare `${user_config.disk_hygiene_enabled}` +argument. Because the declared userConfig `default` is not implemented upstream +(#46477 / #39455 / #39827), an unset-but-defaulted token was neither substituted nor exported as `CLAUDE_PLUGIN_OPTION_*` and +its presence **dropped the whole engine-gate hook** — so on a default install the gate never ran at all, +the real shape of the "PowerShell bypass" originally reported. Reading settings directly needs no +`default` substitution, so that inert-by-default failure is gone. **Recheck** the tamper and scoping +premises if 2.1.207's user-scope-only `pluginConfigs` behavior changes upstream. + +PreToolUse hooks with a `Bash|PowerShell` matcher fire for the PowerShell tool on 2.1.218 (payload +`tool_name` is literally `PowerShell`, confirmed by a live block through that tool); there is no harness +firing divergence. The gate defers instantly (no output) for any command that does not reference the +engine, so it never taxes unrelated work; its coverage marker is the engine script name, a belt against casual invocation, not an authority (renaming the script evades the gate but not the engine's own -preview/approval-token containment). The **skill-scoped belt** (the clean skill's frontmatter -hook) still receives neither the substitution nor the `CLAUDE_PLUGIN_OPTION_*` environment -variable, so on its surface the guard defaults to enabled; that is now a defense-in-depth -redundancy rather than the only enforcement, and the model additionally reads the substituted -`disk_hygiene_enabled` value from the skill content and self-enforces audit-only. Even when the -switch is reachable, the PowerShell lane is a raised bar, not fail-closed: an unknown mutation -spelling passes it, so the engine's own containment, revalidation, and platform gates remain the +preview/approval-token containment). The model additionally reads the `disk_hygiene_enabled` value from +the skill content and self-enforces audit-only — now defense-in-depth over the guard, not the only path. +Even when the switch resolves enabled, the PowerShell lane is a raised bar, not fail-closed: an unknown +mutation spelling passes it, so the engine's own containment, revalidation, and platform gates remain the deletion authority. A depth-limited scan records every directory it declined to enter in `truncated_paths`. Truncated diff --git a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py index 5d1bccdfda..046f590395 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py +++ b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py @@ -9,6 +9,12 @@ import sys from pathlib import Path +_LIB_DIR = Path(__file__).resolve().parents[3] / "lib" +if str(_LIB_DIR) not in sys.path: + sys.path.insert(0, str(_LIB_DIR)) + +import killswitch_config # noqa: E402 (path set above; plugin-bundled module) + _SHELL_EXPANSION_OR_OPERATOR_CHARS = frozenset("{}$*?[]~`()<>;|&\r\n\t!#") @@ -339,11 +345,6 @@ def resolve_mode() -> str: return value if value in {_MODE_BELT, _MODE_ENGINE_GATE} else _MODE_BELT -_DISK_HYGIENE_ENABLED_FLAG = "--disk-hygiene-enabled" -_DISK_HYGIENE_ENABLED_ENV = "CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED" -_DISK_HYGIENE_ENABLED_PLACEHOLDER = "${user_config.disk_hygiene_enabled}" - - def _argv_flag_value(argv: list[str], flag: str) -> str | None: """Read the runtime-substituted value the frontmatter hook passed for ``flag``. @@ -394,29 +395,74 @@ def resolve_authorized_data_root() -> str | None: return os.environ.get(_CLAUDE_PLUGIN_DATA_ENV) +def _user_settings_path_from_root(plugin_root: str) -> str | None: + """Derive the user ``settings.json`` path from the plugin's install root. + + Claude Code lays a marketplace plugin out at + ``/plugins/cache///``, and the user + settings file is ``/settings.json`` — the sibling of the ``plugins`` + directory. This anchors on the same ``plugins/cache`` marker as + ``_plugin_data_root_from_root`` rather than a fixed depth: the ``plugins`` + directory is the segment before ``cache``, and ``settings.json`` is its + parent's child. A root without that marker yields ``None`` so the caller + falls back to the environment. + """ + parts = Path(plugin_root).parts + for index in range(1, len(parts)): + if ( + parts[index].casefold() == _PLUGIN_CACHE_DIRNAME + and parts[index - 1].casefold() == _PLUGINS_DIRNAME + ): + plugins_dir = Path(*parts[:index]) + return os.fspath(plugins_dir.parent / "settings.json") + return None + + +def _resolve_user_settings_path() -> Path: + """Locate the user settings file that carries the kill switch. + + Precedence, highest first: + + 1. Derived from ``--plugin-root ${CLAUDE_PLUGIN_ROOT}`` — the tamper-resistant + channel. Claude Code substitutes the plugin's true install path, which a + hostile repo cannot forge, so the settings file it points at is the real + user one regardless of any repo-supplied ``CLAUDE_CONFIG_DIR`` / ``HOME``. + 2. ``CLAUDE_CONFIG_DIR`` / ``HOME`` (via + ``killswitch_config.default_settings_path``) — the fallback for non-hook + invocations (the report CLI, unit tests) where ``--plugin-root`` is absent. + """ + plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG) + if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER: + derived = _user_settings_path_from_root(plugin_root) + if derived: + return Path(derived) + return killswitch_config.default_settings_path() + + def resolve_disk_hygiene_enabled() -> bool: - """Resolve the execution kill switch from the hook argv, then the environment. + """Resolve the execution kill switch by reading user-scope ``pluginConfigs``. The kill switch is a safety control: ``false`` is audit-only mode and must - prevent every deletion lane. A host supplies the value either as a - ``--disk-hygiene-enabled`` argv flag or as the - ``CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED`` environment variable; argv wins. - A literal, unsubstituted placeholder or an empty value is treated as absent. - - A skill-frontmatter hook can supply neither reliably: Claude Code substitutes - only ``${CLAUDE_PLUGIN_ROOT}`` into a skill hook's args (so - ``${user_config.disk_hygiene_enabled}`` cannot be passed on argv), and it does - not inject ``CLAUDE_PLUGIN_OPTION_*`` into a skill hook's environment. In that - deployment no channel supplies a value and the guard fails safe to enabled — - it stays active and still gates every mutation behind the final human prompt, - but cannot honor a configured ``false`` by denying outright. Delivering the - kill switch to a skill-scoped guard needs a channel skill hooks do not yet - have; see the plugin's safety model. + prevent every deletion lane. The guard reads ``disk_hygiene_enabled`` straight + out of the user ``settings.json`` (``lib/killswitch_config.py``, the single + reader it shares with the report-only probe) — never the process environment. + Since Claude Code 2.1.207 that key is honored only from user, managed, and + ``--settings`` scope, never a project or local ``settings.json`` + (plugins-reference, "User configuration"), so a hostile repo cannot flip the + switch. The environment is rejected on purpose: a repo ``settings.json`` + ``env`` block reaches hook subprocesses and carries no provenance a hook could + check, so an env-borne toggle (or an env-borne settings path) would reopen the + hole this closes — hence the settings file is located from the tamper-resistant + ``--plugin-root`` first (see ``_resolve_user_settings_path``). + + Every absent, unreadable, or ambiguous read fails **closed to enabled**: the + guard stays active and gates every mutation behind the final human prompt even + when it cannot confirm a configured value. Both registration surfaces reach + this one resolver — the plugin-level engine gate and the skill-frontmatter belt + both run ``main()``, and both receive ``--plugin-root ${CLAUDE_PLUGIN_ROOT}`` — + so the belt needs no environment channel it does not have. """ - from_argv = _argv_flag_value(sys.argv[1:], _DISK_HYGIENE_ENABLED_FLAG) - if from_argv and from_argv != _DISK_HYGIENE_ENABLED_PLACEHOLDER: - return from_argv.strip().lower() != "false" - return os.environ.get(_DISK_HYGIENE_ENABLED_ENV, "true").strip().lower() != "false" + return killswitch_config.resolve_effective(_resolve_user_settings_path()) def _is_authorized_data_root(value: str, authority: str | None) -> bool: diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py index 9a3c305ee2..ec15ec18fb 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py @@ -2186,70 +2186,86 @@ class GuardTests(unittest.TestCase): def python_command() -> str: return guard._display_python() - def run_guard(self, command: str) -> dict[str, object]: - stdin = io.StringIO(json.dumps({"tool_input": {"command": command}})) - stdout = io.StringIO() - with ( - mock.patch("sys.stdin", stdin), - redirect_stdout(stdout), - mock.patch.object(guard.sys, "argv", [str(SCRIPT_DIR / "destructive_guard.py")]), - mock.patch.dict( - "os.environ", {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "true"}, clear=False + def setUp(self) -> None: + # Hermetic kill switch: the guard resolves disk_hygiene_enabled by reading + # the user settings.json. Point it at an isolated CLAUDE_CONFIG_DIR whose + # settings.json we own (absent = enabled default; present-false = + # audit-only), independent of the developer's own ~/.claude/settings.json. + # The plain guard helpers use the CLAUDE_CONFIG_DIR fallback channel rather + # than injecting a --plugin-root, so they never perturb the guard's + # data-root authority resolution. The engine-gate helper (which must pass + # --plugin-root to mirror hooks.json) points it at the fake cache layout + # below, which derives back to the same owned settings.json — so both + # channels are hermetic and consistent no matter where the tests run. + self._cfg = tempfile.TemporaryDirectory() + self.addCleanup(self._cfg.cleanup) + cfg = Path(self._cfg.name) + self._plugin_root = ( + cfg / "plugins" / "cache" / "melodic-software" / "disk-hygiene" / "1.2.3" + ) + self._plugin_root.mkdir(parents=True) + self._settings = cfg / "settings.json" + + def _set_kill_switch(self, enabled: bool) -> None: + if enabled: + self._settings.unlink(missing_ok=True) + return + self._settings.write_text( + json.dumps( + { + "pluginConfigs": { + "disk-hygiene@melodic-software": { + "options": {"disk_hygiene_enabled": False} + } + } + } ), - ): - self.assertEqual(0, guard.main()) - return json.loads(stdout.getvalue()) + encoding="utf-8", + ) - def run_guard_disabled(self, command: str) -> dict[str, object]: - stdin = io.StringIO(json.dumps({"tool_input": {"command": command}})) + def _invoke_guard( + self, command: str, *, tool_name: str = "Bash", enabled: bool = True + ) -> dict[str, object] | None: + self._set_kill_switch(enabled) + argv = [str(SCRIPT_DIR / "destructive_guard.py")] + payload: dict[str, object] = {"tool_input": {"command": command}} + if tool_name: + payload["tool_name"] = tool_name + stdin = io.StringIO(json.dumps(payload)) stdout = io.StringIO() with ( mock.patch("sys.stdin", stdin), redirect_stdout(stdout), - mock.patch.object(guard.sys, "argv", [str(SCRIPT_DIR / "destructive_guard.py")]), + mock.patch.object(guard.sys, "argv", argv), mock.patch.dict( - "os.environ", {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "false"}, clear=False + "os.environ", {"CLAUDE_CONFIG_DIR": self._cfg.name}, clear=False ), ): self.assertEqual(0, guard.main()) - return json.loads(stdout.getvalue()) + value = stdout.getvalue() + return json.loads(value) if value.strip() else None + + def run_guard(self, command: str) -> dict[str, object]: + result = self._invoke_guard(command, enabled=True) + assert result is not None + return result + + def run_guard_disabled(self, command: str) -> dict[str, object]: + result = self._invoke_guard(command, enabled=False) + assert result is not None + return result - def run_guard_enabled_argv( - self, command: str, tool_name: str, enabled_arg: str + def run_guard_tool( + self, command: str, tool_name: str, enabled: bool ) -> dict[str, object] | None: - """Drive the guard with the kill switch supplied via hook argv, not the env. - - Exercises the ``--disk-hygiene-enabled`` argv channel with - CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED absent — the delivery a host that - can substitute ``${user_config.disk_hygiene_enabled}`` (a plugin - ``hooks.json`` hook) would use. The bundled skill-frontmatter hook cannot - supply this value (Claude Code substitutes only ``${CLAUDE_PLUGIN_ROOT}`` - into skill-hook args), so this proves the guard's argv path itself, not the - shipped skill deployment. + """Drive a specific tool lane with the kill switch set via user settings. + + Post-C′ the switch reaches the guard only by reading ``disk_hygiene_enabled`` + out of the user ``settings.json`` (located from ``--plugin-root``); the old + ``--disk-hygiene-enabled`` argv and ``CLAUDE_PLUGIN_OPTION_*`` env channels + are gone. This exercises that one real channel per tool. """ - argv = [ - str(SCRIPT_DIR / "destructive_guard.py"), - "--disk-hygiene-enabled", - enabled_arg, - ] - environment = { - key: value - for key, value in os.environ.items() - if key != "CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED" - } - stdin = io.StringIO( - json.dumps({"tool_name": tool_name, "tool_input": {"command": command}}) - ) - stdout = io.StringIO() - with ( - mock.patch("sys.stdin", stdin), - redirect_stdout(stdout), - mock.patch.object(guard.sys, "argv", argv), - mock.patch.dict("os.environ", environment, clear=True), - ): - self.assertEqual(0, guard.main()) - value = stdout.getvalue() - return json.loads(value) if value.strip() else None + return self._invoke_guard(command, tool_name=tool_name, enabled=enabled) def test_guard_denies_direct_recursive_delete(self) -> None: result = self.run_guard("rm -rf /tmp/example") @@ -2497,32 +2513,28 @@ def test_guard_scan_accepts_optional_policy_and_project_dir(self) -> None: ) def run_guard_engine_gate( - self, command: str, tool_name: str = "Bash", enabled_arg: str = "true" + self, command: str, tool_name: str = "Bash", enabled: bool = True ) -> dict[str, object] | None: """Drive the guard as the plugin-level engine-gate deployment would. - Supplies ``--mode engine-gate`` plus both plugin-level substitution - channels (``--disk-hygiene-enabled`` and ``--authorized-data-root``) on - argv, with the env channel absent — mirroring the shipped - ``hooks/hooks.json`` exec-form registration. Returns None when the guard - deferred with no output. + Supplies ``--mode engine-gate`` plus the ``--plugin-root`` and + ``--authorized-data-root`` substitution channels on argv, mirroring the + shipped ``hooks/hooks.json`` exec-form registration. ``--plugin-root`` + points at the hermetic fake cache layout so the kill switch resolves from + the owned settings.json — the sole post-C′ channel — while the explicit + ``--authorized-data-root`` still drives data-root authority. Returns None + when the guard deferred with no output. """ + self._set_kill_switch(enabled) argv = [ str(SCRIPT_DIR / "destructive_guard.py"), "--mode", "engine-gate", "--plugin-root", - str(SCRIPT_DIR.parent.parent.parent), + os.fspath(self._plugin_root), "--authorized-data-root", str(SCRIPT_DIR / "data-root"), - "--disk-hygiene-enabled", - enabled_arg, ] - environment = { - key: value - for key, value in os.environ.items() - if key != "CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED" - } stdin = io.StringIO( json.dumps({"tool_name": tool_name, "tool_input": {"command": command}}) ) @@ -2531,7 +2543,7 @@ def run_guard_engine_gate( mock.patch("sys.stdin", stdin), redirect_stdout(stdout), mock.patch.object(guard.sys, "argv", argv), - mock.patch.dict("os.environ", environment, clear=True), + mock.patch.dict("os.environ", {}, clear=True), ): self.assertEqual(0, guard.main()) text = stdout.getvalue().strip() @@ -2573,7 +2585,7 @@ def test_engine_gate_catches_interpreter_options_before_the_script(self) -> None """Interpreter options must not slip the kill switch (P1 review).""" script = SCRIPT_DIR / "hygiene.py" result = self.run_guard_engine_gate( - f'/usr/bin/python3 -B "{script}" apply --plan p --token t', "Bash", "false" + f'/usr/bin/python3 -B "{script}" apply --plan p --token t', "Bash", enabled=False ) assert result is not None self.assertEqual("deny", result["hookSpecificOutput"]["permissionDecision"]) @@ -2594,12 +2606,12 @@ def test_engine_gate_catches_wrapper_launchers_of_the_bundled_engine(self) -> No """env / sh -c wrappers around the absolute engine path must gate (P1 review).""" script = SCRIPT_DIR / "hygiene.py" wrapped = self.run_guard_engine_gate( - f'/usr/bin/env "{script}" apply --plan p --token t', "Bash", "false" + f'/usr/bin/env "{script}" apply --plan p --token t', "Bash", enabled=False ) assert wrapped is not None self.assertEqual("deny", wrapped["hookSpecificOutput"]["permissionDecision"]) compound = self.run_guard_engine_gate( - f'sh -c "{script} apply --plan p --token t"', "Bash", "false" + f'sh -c "{script} apply --plan p --token t"', "Bash", enabled=False ) assert compound is not None self.assertEqual("deny", compound["hookSpecificOutput"]["permissionDecision"]) @@ -2615,7 +2627,7 @@ def test_engine_gate_catches_linked_aliases_of_the_bundled_engine(self) -> None: self.skipTest(f"hard links unavailable here: {exc}") posix_alias = str(alias).replace("\\", "/") result = self.run_guard_engine_gate( - f'"{posix_alias}" apply --plan p --token t', "Bash", "false" + f'"{posix_alias}" apply --plan p --token t', "Bash", enabled=False ) assert result is not None self.assertEqual( @@ -2634,7 +2646,7 @@ def test_engine_gate_catches_alias_beside_shell_operator(self) -> None: self.skipTest(f"hard links unavailable here: {exc}") posix_alias = str(alias).replace("\\", "/") result = self.run_guard_engine_gate( - f'"{posix_alias}" apply --plan p --token t && true', "Bash", "false" + f'"{posix_alias}" apply --plan p --token t && true', "Bash", enabled=False ) assert result is not None self.assertEqual( @@ -2681,7 +2693,7 @@ def test_engine_gate_still_gates_engine_beside_consumer_decoy(self) -> None: result = self.run_guard_engine_gate( f'python3 {posix} --help && python3 "{script}" apply --plan p --token t', "Bash", - "false", + enabled=False, ) assert result is not None self.assertEqual( @@ -2693,7 +2705,7 @@ def test_engine_gate_catches_path_resolved_engine_after_env_wrapper(self) -> Non result = self.run_guard_engine_gate( f"env PATH={SCRIPT_DIR}:/usr/bin hygiene.py apply --plan p --token t", "Bash", - "false", + enabled=False, ) assert result is not None self.assertEqual("deny", result["hookSpecificOutput"]["permissionDecision"]) @@ -2703,7 +2715,7 @@ def test_engine_gate_catches_engine_after_wrapper_with_option_operands(self) -> result = self.run_guard_engine_gate( f"env -i PATH=/usr/bin nice -n 10 hygiene.py apply --plan p --token t", "Bash", - "false", + enabled=False, ) assert result is not None self.assertEqual("deny", result["hookSpecificOutput"]["permissionDecision"]) @@ -2712,7 +2724,7 @@ def test_engine_gate_catches_non_cpython_launch_of_bundled_engine(self) -> None: """A relative bundled-engine path gates under ANY launcher (P1 r10).""" with chdir_context(SCRIPT_DIR): result = self.run_guard_engine_gate( - "pypy3 ./hygiene.py apply --plan p --token t", "Bash", "false" + "pypy3 ./hygiene.py apply --plan p --token t", "Bash", enabled=False ) assert result is not None self.assertEqual("deny", result["hookSpecificOutput"]["permissionDecision"]) @@ -2751,48 +2763,18 @@ def test_engine_gate_fails_closed_on_engine_when_disabled(self) -> None: """ script = SCRIPT_DIR / "hygiene.py" result = self.run_guard_engine_gate( - f'python3 "{script}" scan --target t --output s', "Bash", "false" + f'python3 "{script}" scan --target t --output s', "Bash", enabled=False ) assert result is not None self.assertEqual("deny", result["hookSpecificOutput"]["permissionDecision"]) def run_guard_powershell(self, command: str) -> dict[str, object] | None: - stdin = io.StringIO( - json.dumps({"tool_name": "PowerShell", "tool_input": {"command": command}}) - ) - stdout = io.StringIO() - with ( - mock.patch("sys.stdin", stdin), - redirect_stdout(stdout), - mock.patch.object(guard.sys, "argv", [str(SCRIPT_DIR / "destructive_guard.py")]), - mock.patch.dict( - "os.environ", {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "true"}, clear=False - ), - ): - self.assertEqual(0, guard.main()) - value = stdout.getvalue() - return json.loads(value) if value.strip() else None + return self._invoke_guard(command, tool_name="PowerShell", enabled=True) def run_guard_powershell_disabled( self, command: str ) -> dict[str, object] | None: - stdin = io.StringIO( - json.dumps({"tool_name": "PowerShell", "tool_input": {"command": command}}) - ) - stdout = io.StringIO() - with ( - mock.patch("sys.stdin", stdin), - redirect_stdout(stdout), - mock.patch.object(guard.sys, "argv", [str(SCRIPT_DIR / "destructive_guard.py")]), - mock.patch.dict( - "os.environ", - {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "false"}, - clear=False, - ), - ): - self.assertEqual(0, guard.main()) - value = stdout.getvalue() - return json.loads(value) if value.strip() else None + return self._invoke_guard(command, tool_name="PowerShell", enabled=False) def test_guard_scan_accepts_only_hook_authorized_data_root(self) -> None: script = SCRIPT_DIR / "hygiene.py" @@ -2830,7 +2812,7 @@ def test_guard_denies_data_root_without_hook_authority(self) -> None: for key, value in os.environ.items() if key != "CLAUDE_PLUGIN_DATA" } - environment["CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED"] = "true" + environment["CLAUDE_CONFIG_DIR"] = self._cfg.name stdin = io.StringIO(json.dumps({"tool_input": {"command": command}})) stdout = io.StringIO() with ( @@ -2863,7 +2845,7 @@ def run_guard_hook_argv( for key, value in os.environ.items() if key != "CLAUDE_PLUGIN_DATA" } - environment["CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED"] = "true" + environment["CLAUDE_CONFIG_DIR"] = self._cfg.name stdin = io.StringIO(json.dumps({"tool_input": {"command": command}})) stdout = io.StringIO() with ( @@ -2973,7 +2955,7 @@ def run_guard_plugin_root( for key, value in os.environ.items() if key != "CLAUDE_PLUGIN_DATA" } - environment["CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED"] = "true" + environment["CLAUDE_CONFIG_DIR"] = self._cfg.name stdin = io.StringIO(json.dumps({"tool_input": {"command": command}})) stdout = io.StringIO() with ( @@ -3143,54 +3125,6 @@ def test_argv_flag_value_parses_both_arg_spellings(self) -> None: guard._argv_flag_value(["--disk-hygiene-enabled"], "--disk-hygiene-enabled") ) - def test_resolve_disk_hygiene_enabled_precedence(self) -> None: - script = str(SCRIPT_DIR / "destructive_guard.py") - - def drive(argv_tail: list[str], env: dict[str, str]) -> bool: - with ( - mock.patch.object(guard.sys, "argv", [script, *argv_tail]), - mock.patch.dict("os.environ", env, clear=True), - ): - return guard.resolve_disk_hygiene_enabled() - - # Argv is authoritative over the environment, both directions. - self.assertFalse( - drive( - ["--disk-hygiene-enabled", "false"], - {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "true"}, - ), - "a configured false in argv must disable even when the env says true", - ) - self.assertTrue( - drive( - ["--disk-hygiene-enabled", "true"], - {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "false"}, - ) - ) - # Case-insensitive, whitespace-tolerant false. - self.assertFalse(drive(["--disk-hygiene-enabled", " FALSE "], {})) - # A literal, unsubstituted placeholder falls back to the environment. - self.assertFalse( - drive( - ["--disk-hygiene-enabled", "${user_config.disk_hygiene_enabled}"], - {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "false"}, - ), - "an unsubstituted placeholder must fall back to the environment", - ) - # An empty substituted value falls back to the environment. - self.assertFalse( - drive( - ["--disk-hygiene-enabled", ""], - {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "false"}, - ) - ) - # No argv, env supplies the value. - self.assertFalse( - drive([], {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "false"}) - ) - # No channel supplies a value: fail safe to enabled (guard active). - self.assertTrue(drive([], {})) - @staticmethod def _skill_hook_command_and_args() -> tuple[str, list[str]]: """Return the frontmatter guard hook's `command` and parsed `args`.""" @@ -3251,6 +3185,48 @@ def test_skill_hook_passes_plugin_root_flag_matching_constant(self) -> None: flag_index = args.index(guard._PLUGIN_ROOT_FLAG) 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.""" + hooks_path = SCRIPT_DIR.parents[2] / "hooks" / "hooks.json" + config = json.loads(hooks_path.read_text(encoding="utf-8")) + entries = config["hooks"]["PreToolUse"] + commands = [ + 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"]) + ] + assert len(commands) == 1, commands + return commands[0]["args"] + + def test_engine_gate_hook_resolves_kill_switch_from_plugin_root_not_user_config( + self, + ) -> None: + """Lock the engine-gate hook seam the kill-switch read now depends on. + + Post-C′ the plugin-level gate resolves ``disk_hygiene_enabled`` by reading + user settings located from ``--plugin-root ${CLAUDE_PLUGIN_ROOT}`` — so that + flag/token pair is load-bearing for the kill switch, not only the data root. + And the bare ``${user_config.disk_hygiene_enabled}`` argument MUST stay + removed: an unset-but-defaulted userConfig token drops the whole hook entry + (the inert-by-default regression this fix closed). This test fails if either + is reverted in hooks.json. + """ + args = self._engine_gate_hook_args() + self.assertIn(guard._PLUGIN_ROOT_FLAG, args) + flag_index = args.index(guard._PLUGIN_ROOT_FLAG) + self.assertEqual(guard._PLUGIN_ROOT_PLACEHOLDER, args[flag_index + 1]) + self.assertNotIn("--disk-hygiene-enabled", args) + for arg in args: + self.assertNotIn( + "${user_config.", + arg, + "engine-gate hook must carry no ${user_config.*} token — its " + "unset-default form drops the whole hook entry", + ) + def test_skill_hook_interpreter_is_python3_and_resolves(self) -> None: """Lock the guard's launch interpreter and prove it resolves. @@ -3474,14 +3450,16 @@ def test_powershell_deletion_spellings_denied_in_audit_only_mode(self) -> None: command, ) - def test_kill_switch_blocks_every_lane_via_argv_without_env(self) -> None: - """A configured ``false`` reaching the guard only through the - ``--disk-hygiene-enabled`` argv channel — the env var UNSET — must block - deletions on both the PowerShell and Bash lanes. Proves the guard's argv - kill-switch logic for a host that can deliver the value there.""" + def test_kill_switch_blocks_every_lane_when_configured_false(self) -> None: + """A configured ``disk_hygiene_enabled=false`` in user settings must block + deletions on both the PowerShell and Bash lanes. This is the sole delivery + channel post-C′: the guard reads the toggle out of the user settings file + it locates from ``--plugin-root``; there is no argv or env channel.""" script = SCRIPT_DIR / "hygiene.py" - powershell = self.run_guard_enabled_argv( - "Remove-Item -Recurse -Force C:/tmp/example", "PowerShell", "false" + powershell = self.run_guard_tool( + "Remove-Item -Recurse -Force C:/tmp/example", + "PowerShell", + enabled=False, ) assert powershell is not None self.assertEqual( @@ -3491,23 +3469,158 @@ def test_kill_switch_blocks_every_lane_via_argv_without_env(self) -> None: f'"{self.python_command()}" "{script}" apply --execute --snapshot s ' f'--plan p --confirm-tier high --approval-token {"a" * 24} --report r' ) - bash = self.run_guard_enabled_argv(apply_command, "Bash", "false") + bash = self.run_guard_tool(apply_command, "Bash", enabled=False) assert bash is not None self.assertEqual("deny", bash["hookSpecificOutput"]["permissionDecision"]) - def test_kill_switch_enabled_via_argv_without_env_still_gates(self) -> None: - """The argv channel with the env var unset also carries an enabling value: - an ``apply`` is gated (``ask``) rather than denied, confirming the argv - value — not a hardcoded default — drives the decision.""" + def test_kill_switch_enabled_gates_apply_as_ask(self) -> None: + """With the switch enabled (settings absent → default on), an ``apply`` is + gated (``ask``) rather than denied, confirming the resolved toggle — not a + hardcoded deny — drives the decision.""" script = SCRIPT_DIR / "hygiene.py" apply_command = ( f'"{self.python_command()}" "{script}" apply --execute --snapshot s ' f'--plan p --confirm-tier high --approval-token {"a" * 24} --report r' ) - result = self.run_guard_enabled_argv(apply_command, "Bash", "true") + result = self.run_guard_tool(apply_command, "Bash", enabled=True) assert result is not None self.assertEqual("ask", result["hookSpecificOutput"]["permissionDecision"]) +class DirectReadKillSwitchTests(unittest.TestCase): + """The kill switch resolves by reading user-scope pluginConfigs directly. + + Post-C′ the guard ignores the ``--disk-hygiene-enabled`` argv flag and the + ``CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED`` env var (both repo-tamperable + or un-delivered) and instead reads ``disk_hygiene_enabled`` out of the + user's ``settings.json`` ``pluginConfigs``. The settings file is located + from the tamper-resistant ``--plugin-root`` (``${CLAUDE_PLUGIN_ROOT}``) + first, then the ``CLAUDE_CONFIG_DIR``/``HOME`` env fallback. Every + absent/degraded read fails closed to enabled (safety on). + """ + + SCRIPT = str(SCRIPT_DIR / "destructive_guard.py") + + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.config_dir = Path(self.tmp.name) + # Reconstruct the real install layout so --plugin-root derivation finds + # the sibling settings.json: /plugins/cache///. + self.plugin_root = ( + self.config_dir + / "plugins" + / "cache" + / "melodic-software" + / "disk-hygiene" + / "1.2.3" + ) + self.plugin_root.mkdir(parents=True) + self.settings = self.config_dir / "settings.json" + + def write_toggle(self, value: object) -> None: + self.settings.write_text( + json.dumps( + { + "pluginConfigs": { + "disk-hygiene@melodic-software": { + "options": {"disk_hygiene_enabled": value} + } + } + } + ), + encoding="utf-8", + ) + + def resolve(self, argv_tail: list[str], env: dict[str, str]) -> bool: + with ( + mock.patch.object(guard.sys, "argv", [self.SCRIPT, *argv_tail]), + mock.patch.dict("os.environ", env, clear=True), + ): + return guard.resolve_disk_hygiene_enabled() + + def plugin_root_argv(self) -> list[str]: + return ["--plugin-root", os.fspath(self.plugin_root)] + + def test_configured_false_via_plugin_root_channel_disables(self) -> None: + self.write_toggle(False) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + + def test_configured_true_via_plugin_root_channel_stays_enabled(self) -> None: + self.write_toggle(True) + self.assertTrue(self.resolve(self.plugin_root_argv(), {})) + + def test_absent_settings_fails_closed_to_enabled(self) -> None: + # No settings.json written under the derived path. + self.assertTrue(self.resolve(self.plugin_root_argv(), {})) + + def test_degraded_settings_fail_closed_to_enabled(self) -> None: + self.settings.write_text("{not json", encoding="utf-8") + self.assertTrue(self.resolve(self.plugin_root_argv(), {})) + + def test_env_toggle_is_ignored(self) -> None: + # Configured false must hold even though the legacy env var says true... + self.write_toggle(False) + self.assertFalse( + self.resolve( + self.plugin_root_argv(), + {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "true"}, + ) + ) + # ...and a legacy env false cannot force audit-only when unconfigured. + self.settings.unlink() + self.assertTrue( + self.resolve( + self.plugin_root_argv(), + {"CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED": "false"}, + ) + ) + + def test_legacy_argv_flag_is_ignored(self) -> None: + # The dropped --disk-hygiene-enabled flag no longer disables anything. + self.assertTrue( + self.resolve( + [*self.plugin_root_argv(), "--disk-hygiene-enabled", "false"], {} + ) + ) + + def test_config_dir_env_fallback_when_no_plugin_root(self) -> None: + self.write_toggle(False) + self.assertFalse( + self.resolve([], {"CLAUDE_CONFIG_DIR": os.fspath(self.config_dir)}) + ) + + def test_plugin_root_channel_beats_tamperable_config_dir_env(self) -> None: + """A repo-injected CLAUDE_CONFIG_DIR cannot override the real settings. + + ``${CLAUDE_PLUGIN_ROOT}`` is substituted by Claude Code from the plugin's + true install path and carries provenance a repo env block cannot forge; + ``CLAUDE_CONFIG_DIR`` from the environment does not. So the plugin-root + channel must win, or a hostile repo re-opens the kill switch by pointing + the config dir at a settings.json it controls. + """ + self.write_toggle(False) # real user settings: audit-only + evil = Path(self.tmp.name) / "evil" + evil.mkdir() + (evil / "settings.json").write_text( + json.dumps( + { + "pluginConfigs": { + "disk-hygiene@melodic-software": { + "options": {"disk_hygiene_enabled": True} + } + } + } + ), + encoding="utf-8", + ) + self.assertFalse( + self.resolve( + self.plugin_root_argv(), + {"CLAUDE_CONFIG_DIR": os.fspath(evil)}, + ) + ) + + if __name__ == "__main__": unittest.main() diff --git a/plugins/disk-hygiene/skills/setup/SKILL.md b/plugins/disk-hygiene/skills/setup/SKILL.md index 0b5be46a14..fe859bece8 100644 --- a/plugins/disk-hygiene/skills/setup/SKILL.md +++ b/plugins/disk-hygiene/skills/setup/SKILL.md @@ -30,11 +30,9 @@ note that re-enabling restores the FAIL semantics. 1. **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 - `clean` skill's context. Caveat, verified on Claude Code 2.1.218: the engine gate only - registers once `disk_hygiene_enabled` is explicitly configured — its bare - `${user_config.*}` argument drops the whole hook while the option is unset, so on a - default install direct `hygiene.py` invocations meet NO plugin-level hook; the - deny-by-default belt applies only during `clean`.) The required version has one origin: the `MIN_PYTHON` + `clean` skill's context. Both register unconditionally and resolve the kill switch by + reading `disk_hygiene_enabled` from the user `settings.json`; the deny-by-default belt + applies only during `clean`.) The required version has one origin: the `MIN_PYTHON` constant in `${CLAUDE_PLUGIN_ROOT}/skills/clean/scripts/hygiene.py` — parse it from there (`grep -m1 '^MIN_PYTHON' …`) and probe the interpreter against that value; do not recite a version number from this file or the README. FAIL if absent or older, naming diff --git a/plugins/disk-hygiene/skills/setup/scripts/kill_switch_probe.py b/plugins/disk-hygiene/skills/setup/scripts/kill_switch_probe.py index 8c808694c8..5a4d39331d 100755 --- a/plugins/disk-hygiene/skills/setup/scripts/kill_switch_probe.py +++ b/plugins/disk-hygiene/skills/setup/scripts/kill_switch_probe.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Deterministic read of the ``disk_hygiene_enabled`` kill switch. +"""Deterministic read of the ``disk_hygiene_enabled`` kill switch (CLI wrapper). ``${user_config.*}`` body-token expansion in skill content is not reliable enough to carry a safety report: an unexpanded token is indistinguishable from @@ -9,13 +9,14 @@ reports the effective boolean with its provenance, degrading honestly when a definitive read is impossible. +The read itself lives in the plugin-level ``lib/killswitch_config.py`` module so +this report-only probe and the destructive-action guard resolve the switch the +same single way (reuse-or-replace: one reader, not two). This file is the +report-only CLI over that reader. + Report-only: exit code is always 0 and the single-line JSON on stdout is the whole contract. This report is how the ``clean`` skill self-enforces audit-only -mode: a skill-frontmatter hook reaches ``destructive_guard.py`` with neither the -``${user_config.*}`` substitution nor the ``CLAUDE_PLUGIN_OPTION_*`` environment -variable, so the guard cannot read the toggle or deny on it — it still gates -every mutation behind a human prompt, but honoring a configured ``false`` is the -model's responsibility, driven by this probe. +mode when a body token arrives unexpanded. Scope: managed settings and a ``--settings`` flag can also carry ``pluginConfigs`` and are not visible here; the ``detail`` sentence states the @@ -26,167 +27,19 @@ import argparse import json -import os -import stat import sys from pathlib import Path -_PLUGIN_NAME = "disk-hygiene" -_OPTION_KEY = "disk_hygiene_enabled" - - -def default_settings_path() -> Path: - config_dir = os.environ.get("CLAUDE_CONFIG_DIR") - base = Path(config_dir) if config_dir else Path.home() / ".claude" - return base / "settings.json" - - -def _matches_plugin(key: str) -> bool: - return key == _PLUGIN_NAME or key.startswith(f"{_PLUGIN_NAME}@") - - -def _interpret(value: object) -> bool | None: - """Return the boolean meaning of a stored option value, or None if invalid.""" - if isinstance(value, bool): - return value - if isinstance(value, str): - lowered = value.strip().lower() - if lowered == "true": - return True - if lowered == "false": - return False - return None +_LIB_DIR = Path(__file__).resolve().parents[3] / "lib" +if str(_LIB_DIR) not in sys.path: + sys.path.insert(0, str(_LIB_DIR)) +import killswitch_config # noqa: E402 (path set above; plugin-bundled module) -def _report( - effective: bool, - source: str, - degraded: bool, - detail: str, - settings_path: Path, - entries: list[dict[str, object]], -) -> dict[str, object]: - return { - "effective": effective, - "source": source, - "degraded": degraded, - "detail": detail, - "settings_path": str(settings_path), - "entries": entries, - } - - -def probe(settings_path: Path) -> dict[str, object]: - try: - settings_stat = settings_path.stat() - except FileNotFoundError: - return _report( - True, - "default", - False, - f"No settings file at {settings_path}; the toggle is not configured " - "there and the plugin default (enabled) applies. Managed settings or " - "a --settings flag could still carry a value this probe cannot see.", - settings_path, - [], - ) - except OSError as exc: - return _report( - True, - "indeterminate", - True, - f"Could not inspect {settings_path} ({exc}); assuming the default " - "(enabled). This is an assumption, not the configured value.", - settings_path, - [], - ) - if not stat.S_ISREG(settings_stat.st_mode): - return _report( - True, - "indeterminate", - True, - f"{settings_path} exists but is not a regular file; assuming the " - "default (enabled). This is an assumption, not the configured value.", - settings_path, - [], - ) - try: - settings = json.loads(settings_path.read_text(encoding="utf-8")) - if not isinstance(settings, dict): - raise ValueError("settings root is not a JSON object") - except (OSError, UnicodeDecodeError, ValueError) as exc: - return _report( - True, - "indeterminate", - True, - f"Could not read the configured toggle from {settings_path} ({exc}); " - "assuming the default (enabled). This is an assumption, not the " - "configured value.", - settings_path, - [], - ) - - plugin_configs = settings.get("pluginConfigs") - entries: list[dict[str, object]] = [] - if isinstance(plugin_configs, dict): - for key in sorted(plugin_configs): - if not _matches_plugin(key): - continue - entry = plugin_configs.get(key) - options = entry.get("options") if isinstance(entry, dict) else None - if not isinstance(options, dict) or _OPTION_KEY not in options: - continue - entries.append({"key": key, "value": options[_OPTION_KEY]}) - - if not entries: - return _report( - True, - "default", - False, - f"{settings_path} carries no {_PLUGIN_NAME} {_OPTION_KEY} entry; the " - "toggle is not configured there and the plugin default (enabled) " - "applies. Managed settings or a --settings flag could still carry a " - "value this probe cannot see.", - settings_path, - entries, - ) - - interpreted = {entry["key"]: _interpret(entry["value"]) for entry in entries} - values = set(interpreted.values()) - if None in values: - return _report( - True, - "indeterminate", - True, - "A configured value is not a recognizable boolean " - f"({json.dumps({k: e['value'] for k, e in zip(interpreted, entries)})}); " - "assuming the default (enabled). This is an assumption, not the " - "configured value.", - settings_path, - entries, - ) - if len(values) > 1: - return _report( - True, - "indeterminate", - True, - "Multiple disk-hygiene entries disagree on the toggle " - f"({json.dumps(interpreted)}); assuming the default (enabled). This " - "is an assumption, not the configured value.", - settings_path, - entries, - ) - effective = values.pop() - assert effective is not None - return _report( - effective, - "configured", - False, - f"{_OPTION_KEY} is configured {str(effective).lower()} in " - f"{settings_path}.", - settings_path, - entries, - ) +# Re-exported for the setup skill and tests: the read logic and its default +# settings location are the library's, surfaced here unchanged. +default_settings_path = killswitch_config.default_settings_path +probe = killswitch_config.probe def main(argv: list[str] | None = None) -> int: From 23e40d33e1ee41ca687f88f2ed6739bf2a947d16 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 24 Jul 2026 03:07:07 -0400 Subject: [PATCH 2/6] fix(disk-hygiene): drop shebang from killswitch_config library module The exec-bit hygiene gate requires every tracked shebang file to be mode 100755, but the new lib module is imported (no __main__), never executed, and was committed 100644 from Windows. Remove the shebang rather than mark a library executable. Co-Authored-By: Claude Opus 4.8 (1M context) --- plugins/disk-hygiene/lib/killswitch_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/disk-hygiene/lib/killswitch_config.py b/plugins/disk-hygiene/lib/killswitch_config.py index 6afb1bd508..00cb3534fc 100644 --- a/plugins/disk-hygiene/lib/killswitch_config.py +++ b/plugins/disk-hygiene/lib/killswitch_config.py @@ -1,4 +1,3 @@ -#!/usr/bin/env python3 """Single source of truth for reading the ``disk_hygiene_enabled`` kill switch. Claude Code stores merged plugin options under From 266c2a905f1f7e3c7a5c0a3e5096f50b4e65a563 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 24 Jul 2026 03:26:16 -0400 Subject: [PATCH 3/6] fix(disk-hygiene): honor managed settings in kill-switch resolution Addresses a Codex P1: reading only the user settings.json missed the managed and --settings scopes Claude Code also honors for pluginConfigs, so an admin-enforced (managed) disk_hygiene_enabled=false resolved to enabled and the engine gate returned `ask` for `apply` instead of denying it. The shared lib/killswitch_config.py reader now also reads the platform managed-settings.json (macOS /Library/Application Support/ClaudeCode, Linux/WSL /etc/claude-code, Windows %ProgramFiles%\ClaudeCode). As the highest-precedence, non-overridable scope, a value configured there wins over the user file, so an organization can enforce audit-only mode. A value supplied only through a session --settings file (a CLI flag no hook observes) or the managed-settings.d/ drop-in directory remains a documented residual. Co-Authored-By: Claude Opus 4.8 (1M context) --- plugins/disk-hygiene/CHANGELOG.md | 6 ++ plugins/disk-hygiene/README.md | 19 +++-- plugins/disk-hygiene/lib/killswitch_config.py | 48 +++++++++++- plugins/disk-hygiene/skills/clean/SKILL.md | 10 ++- .../skills/clean/reference/safety-model.md | 19 +++-- .../skills/clean/scripts/destructive_guard.py | 23 ++++-- .../skills/clean/scripts/test_hygiene.py | 74 ++++++++++++++++--- 7 files changed, 161 insertions(+), 38 deletions(-) diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index 883947d20e..d49687204a 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -29,6 +29,12 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format fol subprocesses and carries no provenance, and since Claude Code 2.1.207 `pluginConfigs` is honored only from user, managed, and `--settings` scope (project/local ignored), so a hostile repo cannot forge the value. Every absent, unreadable, or ambiguous read fails **closed to enabled**. +- **Managed (enterprise) settings are honored as the highest-precedence scope.** The reader also reads the + platform managed-settings.json (`/Library/Application Support/ClaudeCode/` on macOS, `/etc/claude-code/` + on Linux/WSL, `%ProgramFiles%\ClaudeCode\` on Windows); a value configured there overrides the user file, + so an organization can enforce audit-only mode. The one residual: a value supplied only through a session + `--settings` file (a CLI flag no hook observes) or the `managed-settings.d/` drop-in directory is not + enforced by the guard. - **`kill_switch_probe.py` now delegates to the shared reader** (its behavior and single-line JSON output contract unchanged) so the report-only probe and the guard resolve the switch one way, not two. - Docs corrected across `clean`/`setup` `SKILL.md`, `reference/safety-model.md`, and `README.md`: the diff --git a/plugins/disk-hygiene/README.md b/plugins/disk-hygiene/README.md index 12956533b8..17478e2a42 100644 --- a/plugins/disk-hygiene/README.md +++ b/plugins/disk-hygiene/README.md @@ -158,9 +158,12 @@ hand-cleaning the zone. PowerShell deletion spellings are denied outright by the skill-scoped belt while `/disk-hygiene:clean` is active (the always-on gate defers on non-engine commands). The read is honored only from user, managed, and `--settings` scope (Claude Code 2.1.207+), so a project or local repo `settings.json` cannot flip - it; the file is located from `${CLAUDE_PLUGIN_ROOT}`, not from repo-redirectable environment. An - absent or unreadable value fails closed to enabled. The skill's own kill-switch probe + skill-content - value remain a defense-in-depth honoring layer over the guard. + it; the user file is located from `${CLAUDE_PLUGIN_ROOT}`, not from repo-redirectable environment, and + the managed (enterprise) file at its fixed system path wins as the highest-precedence scope so an org + can enforce audit-only. An absent or unreadable value fails closed to enabled. A value supplied only via + a session `--settings` file or the `managed-settings.d/` drop-in dir is the one residual a hook cannot + read. 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 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 — @@ -171,10 +174,12 @@ hand-cleaning the zone. `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.*}` 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`. The - added trust surface is that settings-file *read* — bounded to a single user-scope `pluginConfigs` - value, located from `${CLAUDE_PLUGIN_ROOT}`, no write. 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 + 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 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/lib/killswitch_config.py b/plugins/disk-hygiene/lib/killswitch_config.py index 00cb3534fc..baddfbf633 100644 --- a/plugins/disk-hygiene/lib/killswitch_config.py +++ b/plugins/disk-hygiene/lib/killswitch_config.py @@ -25,6 +25,7 @@ import json import os import stat +import sys from pathlib import Path PLUGIN_NAME = "disk-hygiene" @@ -38,6 +39,34 @@ def default_settings_path() -> Path: return base / "settings.json" +def managed_settings_path() -> Path | None: + """The enterprise/managed settings file for this platform, or ``None``. + + Managed settings are the highest-precedence scope Claude Code honors for + ``pluginConfigs`` and **cannot be overridden** by user/project/local settings + (settings docs, "Settings precedence"), so an organization can enforce + audit-only mode there. The file lives at a fixed, root-owned system path per + platform, so a repo cannot forge it: + + - macOS: ``/Library/Application Support/ClaudeCode/managed-settings.json`` + - Linux/WSL: ``/etc/claude-code/managed-settings.json`` + - Windows: ``%ProgramFiles%\\ClaudeCode\\managed-settings.json`` (the legacy + ``%ProgramData%`` path is unsupported as of Claude Code v2.1.75) + + Residuals not read here: the ``managed-settings.d/`` drop-in directory, and a + session's ``--settings`` file (a runtime CLI flag a hook cannot observe). A + value supplied only through those is not honored by the guard. + """ + if sys.platform == "darwin": + return Path("/Library/Application Support/ClaudeCode/managed-settings.json") + if sys.platform == "win32": + program_files = os.environ.get("ProgramFiles", r"C:\Program Files") + return Path(program_files) / "ClaudeCode" / "managed-settings.json" + if sys.platform.startswith("linux"): + return Path("/etc/claude-code/managed-settings.json") + return None + + def _matches_plugin(key: str) -> bool: return key == PLUGIN_NAME or key.startswith(f"{PLUGIN_NAME}@") @@ -187,6 +216,21 @@ def probe(settings_path: Path) -> dict[str, object]: ) -def resolve_effective(settings_path: Path) -> bool: - """The boolean kill switch: ``probe()``'s effective value, closed to enabled.""" +def resolve_effective( + settings_path: Path, managed_settings_path: Path | None = None +) -> bool: + """The boolean kill switch, honoring managed precedence, closed to enabled. + + Managed settings are the highest-precedence, non-overridable scope, so an + explicitly *configured* value there wins over the user settings — that is how + an organization enforces audit-only mode. A managed file that is absent, has + no ``disk_hygiene_enabled`` entry, or is unreadable/ambiguous (any source + other than ``configured``) yields no managed verdict and the user settings + decide. Every read is ``probe()``'s effective value, which fails **closed to + enabled**. + """ + if managed_settings_path is not None: + managed = probe(managed_settings_path) + if managed["source"] == "configured": + return bool(managed["effective"]) return bool(probe(settings_path)["effective"]) diff --git a/plugins/disk-hygiene/skills/clean/SKILL.md b/plugins/disk-hygiene/skills/clean/SKILL.md index 3eebfe2138..c7684e7a29 100644 --- a/plugins/disk-hygiene/skills/clean/SKILL.md +++ b/plugins/disk-hygiene/skills/clean/SKILL.md @@ -324,10 +324,12 @@ sparse files, hard links, compression, and delayed allocation affect it. neither substituted nor exported to `CLAUDE_PLUGIN_OPTION_*`, and its presence **dropped the whole hook entry** — making the engine gate inert for any consumer who never set the key. The gate no longer carries a `${user_config.*}` token; both the gate and the belt resolve `disk_hygiene_enabled` by reading it from - user-scope `pluginConfigs` in `settings.json`. Claude Code honors that key only from user, managed, and - `--settings` scope since 2.1.207 (a project/local `settings.json` is ignored), so a hostile repo cannot - forge it; the file is located from `${CLAUDE_PLUGIN_ROOT}` rather than the environment, which a repo `env` - block could redirect. Absent or unreadable settings fail closed to enabled. + `pluginConfigs` in `settings.json`. Claude Code honors that key only from user, managed, and `--settings` + scope since 2.1.207 (a project/local `settings.json` is ignored), so a hostile repo cannot forge it. The + reader reads the **user** file (located from `${CLAUDE_PLUGIN_ROOT}` rather than repo-redirectable + environment) and the **managed** enterprise file (highest precedence — a value there wins, so an org can + enforce audit-only); a session `--settings` file and the `managed-settings.d/` drop-in dir are the + residuals a hook cannot read. Absent or unreadable settings fail closed to enabled. - **PreToolUse hooks DO fire for the PowerShell tool** (2.1.218; payload `tool_name` is literally `PowerShell`, confirmed by a live block through that tool). A `Bash|PowerShell` matcher is correct and there is no harness firing divergence — read `tool_name` from the stdin payload, not from an env var diff --git a/plugins/disk-hygiene/skills/clean/reference/safety-model.md b/plugins/disk-hygiene/skills/clean/reference/safety-model.md index 046723a1c8..870c0b5bcc 100644 --- a/plugins/disk-hygiene/skills/clean/reference/safety-model.md +++ b/plugins/disk-hygiene/skills/clean/reference/safety-model.md @@ -147,13 +147,18 @@ block deletions on the PowerShell lane too and not only the Bash engine apply. **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 -resolve `disk_hygiene_enabled` the same single way: by reading it from user-scope `pluginConfigs` in -`settings.json`, 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. Claude -Code honors that key only from user, managed, and `--settings` scope since 2.1.207 — a project or local -`.claude/settings.json` is ignored — so a hostile repo cannot flip it. The settings file is located from -`${CLAUDE_PLUGIN_ROOT}` (the plugin's true install path, which a repo cannot forge), not from -`CLAUDE_CONFIG_DIR`/`HOME`, which a repo `settings.json` `env` block could redirect. When the value +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. +Claude Code honors that key only from user, managed, and `--settings` scope since 2.1.207 — a project or +local `.claude/settings.json` is ignored — so a hostile repo cannot flip it. The **user** file is located +from `${CLAUDE_PLUGIN_ROOT}` (the plugin's true install path, which a repo cannot forge), not from +`CLAUDE_CONFIG_DIR`/`HOME`, which a repo `settings.json` `env` block could redirect. The **managed** +(enterprise) file at its fixed root-owned system path is read too and, as the highest-precedence +non-overridable scope, an explicitly configured value there **wins over the user file** — so an +organization can enforce audit-only mode. Two honored sources the guard cannot read remain residual: a +session's `--settings` file (a runtime CLI flag no hook observes) and the `managed-settings.d/` drop-in +directory; a value supplied only through those is not enforced. When the value resolves `false` (audit-only mode), `false` is guard-enforced — denied outright, not merely prompted — but the two surfaces reach different lanes. The **always-on engine gate** enforces it against every Bash engine invocation **whether or not the clean skill is active**; it defers (no output) on any command that diff --git a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py index 046f590395..fd688921f3 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py +++ b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py @@ -444,16 +444,22 @@ def resolve_disk_hygiene_enabled() -> bool: The kill switch is a safety control: ``false`` is audit-only mode and must prevent every deletion lane. The guard reads ``disk_hygiene_enabled`` straight - out of the user ``settings.json`` (``lib/killswitch_config.py``, the single + out of the ``settings.json`` files (``lib/killswitch_config.py``, the single reader it shares with the report-only probe) — never the process environment. Since Claude Code 2.1.207 that key is honored only from user, managed, and ``--settings`` scope, never a project or local ``settings.json`` (plugins-reference, "User configuration"), so a hostile repo cannot flip the - switch. The environment is rejected on purpose: a repo ``settings.json`` - ``env`` block reaches hook subprocesses and carries no provenance a hook could - check, so an env-borne toggle (or an env-borne settings path) would reopen the - hole this closes — hence the settings file is located from the tamper-resistant - ``--plugin-root`` first (see ``_resolve_user_settings_path``). + switch. Managed settings are the highest-precedence, non-overridable scope, so + a value configured there wins over the user file — that is how an organization + enforces audit-only mode. The ``--settings`` file is a session CLI flag a hook + cannot observe, the one honored source not read here (see + ``killswitch_config.managed_settings_path`` for the full residual list). The + environment is rejected on purpose: a repo ``settings.json`` ``env`` block + reaches hook subprocesses and carries no provenance a hook could check, so an + env-borne toggle (or an env-borne user-settings path) would reopen the hole + this closes — hence the user settings file is located from the tamper-resistant + ``--plugin-root`` first (see ``_resolve_user_settings_path``), and the managed + file from its fixed root-owned system path. Every absent, unreadable, or ambiguous read fails **closed to enabled**: the guard stays active and gates every mutation behind the final human prompt even @@ -462,7 +468,10 @@ def resolve_disk_hygiene_enabled() -> bool: both run ``main()``, and both receive ``--plugin-root ${CLAUDE_PLUGIN_ROOT}`` — so the belt needs no environment channel it does not have. """ - return killswitch_config.resolve_effective(_resolve_user_settings_path()) + return killswitch_config.resolve_effective( + _resolve_user_settings_path(), + killswitch_config.managed_settings_path(), + ) def _is_authorized_data_root(value: str, authority: str | None) -> bool: diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py index ec15ec18fb..06db5baa9b 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py @@ -2205,6 +2205,9 @@ def setUp(self) -> None: ) self._plugin_root.mkdir(parents=True) self._settings = cfg / "settings.json" + # Managed settings stay absent (points into the temp dir, never written), + # so tests never read a real /etc or Program Files managed-settings.json. + self._managed = cfg / "managed-settings.json" def _set_kill_switch(self, enabled: bool) -> None: if enabled: @@ -2240,6 +2243,11 @@ def _invoke_guard( mock.patch.dict( "os.environ", {"CLAUDE_CONFIG_DIR": self._cfg.name}, clear=False ), + mock.patch.object( + guard.killswitch_config, + "managed_settings_path", + lambda: self._managed, + ), ): self.assertEqual(0, guard.main()) value = stdout.getvalue() @@ -2544,6 +2552,11 @@ def run_guard_engine_gate( redirect_stdout(stdout), mock.patch.object(guard.sys, "argv", argv), mock.patch.dict("os.environ", {}, clear=True), + mock.patch.object( + guard.killswitch_config, + "managed_settings_path", + lambda: self._managed, + ), ): self.assertEqual(0, guard.main()) text = stdout.getvalue().strip() @@ -3517,25 +3530,35 @@ def setUp(self) -> None: ) self.plugin_root.mkdir(parents=True) self.settings = self.config_dir / "settings.json" - - def write_toggle(self, value: object) -> None: - self.settings.write_text( - json.dumps( - { - "pluginConfigs": { - "disk-hygiene@melodic-software": { - "options": {"disk_hygiene_enabled": value} - } + # Managed settings default to absent; managed tests write this file. + self.managed = self.config_dir / "managed-settings.json" + + def _toggle_json(self, value: object) -> str: + return json.dumps( + { + "pluginConfigs": { + "disk-hygiene@melodic-software": { + "options": {"disk_hygiene_enabled": value} } } - ), - encoding="utf-8", + } ) + def write_toggle(self, value: object) -> None: + self.settings.write_text(self._toggle_json(value), encoding="utf-8") + + def write_managed_toggle(self, value: object) -> None: + self.managed.write_text(self._toggle_json(value), encoding="utf-8") + def resolve(self, argv_tail: list[str], env: dict[str, str]) -> bool: with ( mock.patch.object(guard.sys, "argv", [self.SCRIPT, *argv_tail]), mock.patch.dict("os.environ", env, clear=True), + mock.patch.object( + guard.killswitch_config, + "managed_settings_path", + lambda: self.managed, + ), ): return guard.resolve_disk_hygiene_enabled() @@ -3621,6 +3644,35 @@ def test_plugin_root_channel_beats_tamperable_config_dir_env(self) -> None: ) ) + def test_managed_configured_false_overrides_user_true(self) -> None: + # Managed is the highest-precedence, non-overridable scope: an + # organization enforcing audit-only must win over a user-enabled toggle. + self.write_managed_toggle(False) + self.write_toggle(True) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + + def test_managed_configured_true_overrides_user_false(self) -> None: + self.write_managed_toggle(True) + self.write_toggle(False) + self.assertTrue(self.resolve(self.plugin_root_argv(), {})) + + def test_managed_absent_falls_back_to_user(self) -> None: + # No managed file written; the user toggle decides. + self.write_toggle(False) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + + def test_managed_without_toggle_entry_falls_back_to_user(self) -> None: + # Managed file exists but carries no disk-hygiene entry (source=default), + # so it yields no verdict and the user toggle decides. + self.managed.write_text(json.dumps({"pluginConfigs": {}}), encoding="utf-8") + self.write_toggle(False) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + + def test_managed_malformed_falls_back_to_user(self) -> None: + self.managed.write_text("{not json", encoding="utf-8") + self.write_toggle(False) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + if __name__ == "__main__": unittest.main() From 23cad83407350f57f4e65216f9d26190e63d60ba Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 24 Jul 2026 03:35:15 -0400 Subject: [PATCH 4/6] fix(disk-hygiene): harden managed read against env tamper + marketplace masking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Codex findings on the managed-settings read: - P1: the Windows managed path was derived from %ProgramFiles%, which a repo settings `env` block can set for hook subprocesses — letting a repo point the highest-precedence managed lookup at a forged managed-settings.json that force-enables the switch. Hard-code the documented C:\Program Files\ClaudeCode path so no environment value participates. - P2: the pluginConfigs key match was a disk-hygiene@* prefix, so another marketplace's disk-hygiene entry could contradict this install's value and make the read ambiguous (falling back to enabled). The guard now derives its exact @ id from ${CLAUDE_PLUGIN_ROOT} and matches only that; the report-only CLI (which cannot know its marketplace) keeps the prefix match. The session --settings source remains a documented residual: no hook channel exposes the active --settings file path. Co-Authored-By: Claude Opus 4.8 (1M context) --- plugins/disk-hygiene/CHANGELOG.md | 10 ++-- plugins/disk-hygiene/lib/killswitch_config.py | 46 ++++++++++++++----- .../skills/clean/scripts/destructive_guard.py | 33 +++++++++++++ .../skills/clean/scripts/test_hygiene.py | 23 ++++++++++ 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index d49687204a..2bfa2d33a8 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -31,10 +31,12 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format fol Every absent, unreadable, or ambiguous read fails **closed to enabled**. - **Managed (enterprise) settings are honored as the highest-precedence scope.** The reader also reads the platform managed-settings.json (`/Library/Application Support/ClaudeCode/` on macOS, `/etc/claude-code/` - on Linux/WSL, `%ProgramFiles%\ClaudeCode\` on Windows); a value configured there overrides the user file, - so an organization can enforce audit-only mode. The one residual: a value supplied only through a session - `--settings` file (a CLI flag no hook observes) or the `managed-settings.d/` drop-in directory is not - enforced by the guard. + on Linux/WSL, `C:\Program Files\ClaudeCode\` on Windows — a fixed path, not `%ProgramFiles%`-derived, so a + repo `env` block cannot redirect it); a value configured there overrides the user file, so an organization + can enforce audit-only mode. The reader also matches only this install's exact `@` key + (derived from `${CLAUDE_PLUGIN_ROOT}`), so another marketplace's `disk-hygiene` entry cannot mask it. The + one residual: a value supplied only through a session `--settings` file (a CLI flag no hook observes) or + the `managed-settings.d/` drop-in directory is not enforced by the guard. - **`kill_switch_probe.py` now delegates to the shared reader** (its behavior and single-line JSON output contract unchanged) so the report-only probe and the guard resolve the switch one way, not two. - Docs corrected across `clean`/`setup` `SKILL.md`, `reference/safety-model.md`, and `README.md`: the diff --git a/plugins/disk-hygiene/lib/killswitch_config.py b/plugins/disk-hygiene/lib/killswitch_config.py index baddfbf633..90aa9be9ca 100644 --- a/plugins/disk-hygiene/lib/killswitch_config.py +++ b/plugins/disk-hygiene/lib/killswitch_config.py @@ -50,8 +50,16 @@ def managed_settings_path() -> Path | None: - macOS: ``/Library/Application Support/ClaudeCode/managed-settings.json`` - Linux/WSL: ``/etc/claude-code/managed-settings.json`` - - Windows: ``%ProgramFiles%\\ClaudeCode\\managed-settings.json`` (the legacy - ``%ProgramData%`` path is unsupported as of Claude Code v2.1.75) + - Windows: ``C:\\Program Files\\ClaudeCode\\managed-settings.json`` (the + legacy ``%ProgramData%`` path is unsupported as of Claude Code v2.1.75) + + The Windows path is hard-coded, **not** ``%ProgramFiles%``-derived: a repo + ``settings.json`` ``env`` block can set ``ProgramFiles`` for hook subprocesses, + and because managed settings are the highest-precedence scope, an + environment-derived base path would let a repo point this at a forged + ``ClaudeCode/managed-settings.json`` that force-enables the switch. The docs + give this literal absolute path, so trusting it (not the environment) preserves + the tamper-resistance. Residuals not read here: the ``managed-settings.d/`` drop-in directory, and a session's ``--settings`` file (a runtime CLI flag a hook cannot observe). A @@ -60,14 +68,23 @@ def managed_settings_path() -> Path | None: if sys.platform == "darwin": return Path("/Library/Application Support/ClaudeCode/managed-settings.json") if sys.platform == "win32": - program_files = os.environ.get("ProgramFiles", r"C:\Program Files") - return Path(program_files) / "ClaudeCode" / "managed-settings.json" + return Path(r"C:\Program Files\ClaudeCode\managed-settings.json") if sys.platform.startswith("linux"): return Path("/etc/claude-code/managed-settings.json") return None -def _matches_plugin(key: str) -> bool: +def _matches_plugin(key: str, plugin_id: str | None = None) -> bool: + """Match a ``pluginConfigs`` key for this plugin. + + With ``plugin_id`` (the exact ``@`` the guard derives from + its install root) only that key matches — so a second marketplace's + ``disk-hygiene`` entry cannot mask this install's configured value. Without it + (the report-only CLI, which cannot know its marketplace) any ``disk-hygiene`` + or ``disk-hygiene@*`` key matches. + """ + if plugin_id is not None: + return key == plugin_id return key == PLUGIN_NAME or key.startswith(f"{PLUGIN_NAME}@") @@ -102,8 +119,13 @@ def _report( } -def probe(settings_path: Path) -> dict[str, object]: - """Read the effective kill switch from ``settings_path`` with provenance.""" +def probe(settings_path: Path, plugin_id: str | None = None) -> dict[str, object]: + """Read the effective kill switch from ``settings_path`` with provenance. + + ``plugin_id`` narrows the matched ``pluginConfigs`` key to that exact + ``@`` (see ``_matches_plugin``); ``None`` matches any + ``disk-hygiene`` marketplace entry. + """ try: settings_stat = settings_path.stat() except FileNotFoundError: @@ -157,7 +179,7 @@ def probe(settings_path: Path) -> dict[str, object]: entries: list[dict[str, object]] = [] if isinstance(plugin_configs, dict): for key in sorted(plugin_configs): - if not _matches_plugin(key): + if not _matches_plugin(key, plugin_id): continue entry = plugin_configs.get(key) options = entry.get("options") if isinstance(entry, dict) else None @@ -217,7 +239,9 @@ def probe(settings_path: Path) -> dict[str, object]: def resolve_effective( - settings_path: Path, managed_settings_path: Path | None = None + settings_path: Path, + managed_settings_path: Path | None = None, + plugin_id: str | None = None, ) -> bool: """The boolean kill switch, honoring managed precedence, closed to enabled. @@ -230,7 +254,7 @@ def resolve_effective( enabled**. """ if managed_settings_path is not None: - managed = probe(managed_settings_path) + managed = probe(managed_settings_path, plugin_id) if managed["source"] == "configured": return bool(managed["effective"]) - return bool(probe(settings_path)["effective"]) + return bool(probe(settings_path, plugin_id)["effective"]) diff --git a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py index fd688921f3..a055dacb9f 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py +++ b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py @@ -418,6 +418,32 @@ def _user_settings_path_from_root(plugin_root: str) -> str | None: return None +def _plugin_id_from_root(plugin_root: str) -> str | None: + """Return this install's exact ``@`` ``pluginConfigs`` key. + + Claude Code lays a marketplace plugin out at + ``/plugins/cache///`` and keys its options + under ``@``. Deriving that exact key lets the kill-switch + read match only this install, so a second marketplace's ``disk-hygiene`` entry + cannot mask this one's configured value. A root without the ``plugins/cache`` + marker (or missing the name/marketplace segments) yields ``None`` and the read + falls back to matching any ``disk-hygiene`` entry. + """ + parts = Path(plugin_root).parts + for index in range(1, len(parts)): + if ( + parts[index].casefold() == _PLUGIN_CACHE_DIRNAME + and parts[index - 1].casefold() == _PLUGINS_DIRNAME + ): + if index + 2 >= len(parts): + return None + marketplace, name = parts[index + 1], parts[index + 2] + if not marketplace or not name: + return None + return f"{name}@{marketplace}" + return None + + def _resolve_user_settings_path() -> Path: """Locate the user settings file that carries the kill switch. @@ -468,9 +494,16 @@ def resolve_disk_hygiene_enabled() -> bool: both run ``main()``, and both receive ``--plugin-root ${CLAUDE_PLUGIN_ROOT}`` — so the belt needs no environment channel it does not have. """ + plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG) + plugin_id = ( + _plugin_id_from_root(plugin_root) + if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER + else None + ) return killswitch_config.resolve_effective( _resolve_user_settings_path(), killswitch_config.managed_settings_path(), + plugin_id, ) diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py index 06db5baa9b..9b409736b0 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py @@ -3673,6 +3673,29 @@ def test_managed_malformed_falls_back_to_user(self) -> None: self.write_toggle(False) self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + def test_installed_marketplace_id_isolates_from_other_marketplace(self) -> None: + # A second marketplace's disk-hygiene entry must not mask this install's + # configured value: the guard derives its exact @ key + # from --plugin-root (here disk-hygiene@melodic-software) and matches only + # that, rather than aggregating every disk-hygiene@* entry into an + # ambiguous read that would fall back to enabled. + self.settings.write_text( + json.dumps( + { + "pluginConfigs": { + "disk-hygiene@melodic-software": { + "options": {"disk_hygiene_enabled": False} + }, + "disk-hygiene@other-marketplace": { + "options": {"disk_hygiene_enabled": True} + }, + } + } + ), + encoding="utf-8", + ) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + if __name__ == "__main__": unittest.main() From 48f835548c84cd34904a2d86ad5ea15fa915c49c Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 24 Jul 2026 03:45:50 -0400 Subject: [PATCH 5/6] fix(disk-hygiene): read managed-settings.d drop-in kill-switch entries Addresses a Codex P1: an organization can set disk_hygiene_enabled only in the managed managed-settings.d/ drop-in directory, an honored higher-precedence source the reader was ignoring, so an audit-only session could get `ask` instead of `deny`. The reader now merges the primary managed-settings.json with every *.json in the sibling managed-settings.d/ directory (sorted; later files win), mirroring Claude Code's drop-in merge, before falling back to user settings. All at the fixed root-owned system path, so a repo cannot forge them. The session --settings file is now the only honored source a hook cannot read (no env var or payload field exposes its path). Co-Authored-By: Claude Opus 4.8 (1M context) --- plugins/disk-hygiene/CHANGELOG.md | 7 +- plugins/disk-hygiene/README.md | 8 +-- plugins/disk-hygiene/lib/killswitch_config.py | 67 ++++++++++++++++--- plugins/disk-hygiene/skills/clean/SKILL.md | 4 +- .../skills/clean/reference/safety-model.md | 6 +- .../skills/clean/scripts/test_hygiene.py | 25 +++++++ 6 files changed, 95 insertions(+), 22 deletions(-) diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index 2bfa2d33a8..b439a4f885 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -33,10 +33,11 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format fol platform managed-settings.json (`/Library/Application Support/ClaudeCode/` on macOS, `/etc/claude-code/` on Linux/WSL, `C:\Program Files\ClaudeCode\` on Windows — a fixed path, not `%ProgramFiles%`-derived, so a repo `env` block cannot redirect it); a value configured there overrides the user file, so an organization - can enforce audit-only mode. The reader also matches only this install's exact `@` key + can enforce audit-only mode; the sibling `managed-settings.d/` drop-in directory is merged over it + (later files win). The reader also matches only this install's exact `@` key (derived from `${CLAUDE_PLUGIN_ROOT}`), so another marketplace's `disk-hygiene` entry cannot mask it. The - one residual: a value supplied only through a session `--settings` file (a CLI flag no hook observes) or - the `managed-settings.d/` drop-in directory is not enforced by the guard. + one residual: a value supplied only through a session `--settings` file (a runtime CLI flag no hook can + observe) is not enforced by the guard. - **`kill_switch_probe.py` now delegates to the shared reader** (its behavior and single-line JSON output contract unchanged) so the report-only probe and the guard resolve the switch one way, not two. - Docs corrected across `clean`/`setup` `SKILL.md`, `reference/safety-model.md`, and `README.md`: the diff --git a/plugins/disk-hygiene/README.md b/plugins/disk-hygiene/README.md index 17478e2a42..7d42b060cd 100644 --- a/plugins/disk-hygiene/README.md +++ b/plugins/disk-hygiene/README.md @@ -160,10 +160,10 @@ hand-cleaning the zone. `--settings` scope (Claude Code 2.1.207+), so a project or local repo `settings.json` cannot flip it; the user file is located from `${CLAUDE_PLUGIN_ROOT}`, not from repo-redirectable environment, and the managed (enterprise) file at its fixed system path wins as the highest-precedence scope so an org - can enforce audit-only. An absent or unreadable value fails closed to enabled. A value supplied only via - a session `--settings` file or the `managed-settings.d/` drop-in dir is the one residual a hook cannot - read. The skill's own kill-switch probe + skill-content value remain a defense-in-depth honoring layer - over the guard. + can enforce audit-only (the sibling `managed-settings.d/` drop-in directory is merged over it). An absent + 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 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 — diff --git a/plugins/disk-hygiene/lib/killswitch_config.py b/plugins/disk-hygiene/lib/killswitch_config.py index 90aa9be9ca..e50eac9dc9 100644 --- a/plugins/disk-hygiene/lib/killswitch_config.py +++ b/plugins/disk-hygiene/lib/killswitch_config.py @@ -61,9 +61,10 @@ def managed_settings_path() -> Path | None: give this literal absolute path, so trusting it (not the environment) preserves the tamper-resistance. - Residuals not read here: the ``managed-settings.d/`` drop-in directory, and a - session's ``--settings`` file (a runtime CLI flag a hook cannot observe). A - value supplied only through those is not honored by the guard. + The sibling ``managed-settings.d/`` drop-in directory is also read (see + ``_managed_settings_files``). The one honored source a hook cannot read is a + session's ``--settings`` file — a runtime CLI flag no hook observes — so a + value supplied only there is not enforced by the guard. """ if sys.platform == "darwin": return Path("/Library/Application Support/ClaudeCode/managed-settings.json") @@ -238,6 +239,52 @@ def probe(settings_path: Path, plugin_id: str | None = None) -> dict[str, object ) +_MANAGED_DROPIN_DIRNAME = "managed-settings.d" + + +def _managed_settings_files(managed_settings_path: Path) -> list[Path]: + """Managed settings files in Claude Code precedence order (later overrides). + + The primary ``managed-settings.json`` first, then every ``*.json`` in the + sibling ``managed-settings.d/`` drop-in directory in sorted order — Claude + Code merges those drop-ins over the primary file (settings docs, "File-based + managed settings … drop-in directory"). All live at the fixed root-owned + system path, so a repo cannot forge them. + """ + files: list[Path] = [] + if managed_settings_path.is_file(): + files.append(managed_settings_path) + dropin = managed_settings_path.parent / _MANAGED_DROPIN_DIRNAME + if dropin.is_dir(): + files.extend( + sorted( + child + for child in dropin.iterdir() + if child.suffix == ".json" and child.is_file() + ) + ) + return files + + +def _managed_effective( + managed_settings_path: Path, plugin_id: str | None +) -> bool | None: + """The managed-scope verdict, or ``None`` when managed configures no value. + + Reads the primary managed file and the ``managed-settings.d/`` drop-ins in + precedence order; the last file that *configures* ``disk_hygiene_enabled`` + wins (drop-ins override the primary, later drop-ins override earlier), mirroring + Claude Code's merge. A file that is absent, carries no entry, or is + unreadable/ambiguous contributes no verdict. + """ + verdict: bool | None = None + for path in _managed_settings_files(managed_settings_path): + report = probe(path, plugin_id) + if report["source"] == "configured": + verdict = bool(report["effective"]) + return verdict + + def resolve_effective( settings_path: Path, managed_settings_path: Path | None = None, @@ -246,15 +293,15 @@ def resolve_effective( """The boolean kill switch, honoring managed precedence, closed to enabled. Managed settings are the highest-precedence, non-overridable scope, so an - explicitly *configured* value there wins over the user settings — that is how - an organization enforces audit-only mode. A managed file that is absent, has - no ``disk_hygiene_enabled`` entry, or is unreadable/ambiguous (any source - other than ``configured``) yields no managed verdict and the user settings + explicitly *configured* value there — in ``managed-settings.json`` or a + ``managed-settings.d/`` drop-in — wins over the user settings, which is how an + organization enforces audit-only mode. When managed configures no value (all + managed sources absent, entry-less, or unreadable/ambiguous) the user settings decide. Every read is ``probe()``'s effective value, which fails **closed to enabled**. """ if managed_settings_path is not None: - managed = probe(managed_settings_path, plugin_id) - if managed["source"] == "configured": - return bool(managed["effective"]) + managed_verdict = _managed_effective(managed_settings_path, plugin_id) + if managed_verdict is not None: + return managed_verdict return bool(probe(settings_path, plugin_id)["effective"]) diff --git a/plugins/disk-hygiene/skills/clean/SKILL.md b/plugins/disk-hygiene/skills/clean/SKILL.md index c7684e7a29..49e8433d86 100644 --- a/plugins/disk-hygiene/skills/clean/SKILL.md +++ b/plugins/disk-hygiene/skills/clean/SKILL.md @@ -328,8 +328,8 @@ sparse files, hard links, compression, and delayed allocation affect it. scope since 2.1.207 (a project/local `settings.json` is ignored), so a hostile repo cannot forge it. The reader reads the **user** file (located from `${CLAUDE_PLUGIN_ROOT}` rather than repo-redirectable environment) and the **managed** enterprise file (highest precedence — a value there wins, so an org can - enforce audit-only); a session `--settings` file and the `managed-settings.d/` drop-in dir are the - residuals a hook cannot read. Absent or unreadable settings fail closed to enabled. + enforce audit-only, with its `managed-settings.d/` drop-in dir merged over it); a session `--settings` + file is the one honored source a hook cannot read. Absent or unreadable settings fail closed to enabled. - **PreToolUse hooks DO fire for the PowerShell tool** (2.1.218; payload `tool_name` is literally `PowerShell`, confirmed by a live block through that tool). A `Bash|PowerShell` matcher is correct and there is no harness firing divergence — read `tool_name` from the stdin payload, not from an env var diff --git a/plugins/disk-hygiene/skills/clean/reference/safety-model.md b/plugins/disk-hygiene/skills/clean/reference/safety-model.md index 870c0b5bcc..62a9db3482 100644 --- a/plugins/disk-hygiene/skills/clean/reference/safety-model.md +++ b/plugins/disk-hygiene/skills/clean/reference/safety-model.md @@ -156,9 +156,9 @@ from `${CLAUDE_PLUGIN_ROOT}` (the plugin's true install path, which a repo canno `CLAUDE_CONFIG_DIR`/`HOME`, which a repo `settings.json` `env` block could redirect. The **managed** (enterprise) file at its fixed root-owned system path is read too and, as the highest-precedence non-overridable scope, an explicitly configured value there **wins over the user file** — so an -organization can enforce audit-only mode. Two honored sources the guard cannot read remain residual: a -session's `--settings` file (a runtime CLI flag no hook observes) and the `managed-settings.d/` drop-in -directory; a value supplied only through those is not enforced. When the value +organization can enforce audit-only mode; the sibling `managed-settings.d/` drop-in directory is merged +over it (later files win). The one honored source the guard cannot read is a session's `--settings` file +(a runtime CLI flag no hook observes); a value supplied only there is not enforced. When the value resolves `false` (audit-only mode), `false` is guard-enforced — denied outright, not merely prompted — but the two surfaces reach different lanes. The **always-on engine gate** enforces it against every Bash engine invocation **whether or not the clean skill is active**; it defers (no output) on any command that diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py index 9b409736b0..4391935724 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py @@ -3550,6 +3550,11 @@ def write_toggle(self, value: object) -> None: def write_managed_toggle(self, value: object) -> None: self.managed.write_text(self._toggle_json(value), encoding="utf-8") + def write_managed_dropin(self, name: str, value: object) -> None: + dropin = self.config_dir / "managed-settings.d" + dropin.mkdir(exist_ok=True) + (dropin / name).write_text(self._toggle_json(value), encoding="utf-8") + def resolve(self, argv_tail: list[str], env: dict[str, str]) -> bool: with ( mock.patch.object(guard.sys, "argv", [self.SCRIPT, *argv_tail]), @@ -3696,6 +3701,26 @@ def test_installed_marketplace_id_isolates_from_other_marketplace(self) -> None: ) self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + def test_managed_dropin_false_overrides_user(self) -> None: + # A false configured only in the managed drop-in directory must win over a + # user-enabled toggle, just like the primary managed file. + self.write_managed_dropin("10-org-policy.json", False) + self.write_toggle(True) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + + def test_managed_dropin_overrides_primary_managed_file(self) -> None: + # Drop-ins are merged over the primary managed file. + self.write_managed_toggle(True) + self.write_managed_dropin("50-override.json", False) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + + def test_later_managed_dropin_wins_over_earlier(self) -> None: + # Sorted order: 20- overrides 10-. + self.write_managed_dropin("10-first.json", True) + self.write_managed_dropin("20-second.json", False) + self.write_toggle(True) + self.assertFalse(self.resolve(self.plugin_root_argv(), {})) + if __name__ == "__main__": unittest.main() From 429a29c9454c53ac771a455dd8df2ac282e9baf9 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 24 Jul 2026 04:07:11 -0400 Subject: [PATCH 6/6] fix(disk-hygiene): never trust an env-derived settings path in the guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses a Codex P1 on --plugin-dir installs: when the plugin root carries no plugins/cache marker, the guard fell back to CLAUDE_CONFIG_DIR/HOME to locate user settings, and a repo .claude/settings.json env block can inject those into hook subprocesses — pointing the read at a forged settings.json that flips the switch. The guard now locates the user settings file SOLELY from the tamper-resistant ${CLAUDE_PLUGIN_ROOT} cache marker; it never consults the environment. A marker-less (--plugin-dir) root yields no trusted user path, so the user scope is skipped and the switch relies on managed settings (fixed system paths), failing closed to enabled otherwise. Managed enforcement is therefore honored even in --plugin-dir installs. Tests drive the kill switch by patching the guard's resolver directly (no env), which also removes the last CLAUDE_CONFIG_DIR reliance from the guard suite. Co-Authored-By: Claude Opus 4.8 (1M context) --- plugins/disk-hygiene/CHANGELOG.md | 15 ++--- plugins/disk-hygiene/lib/killswitch_config.py | 14 ++++- .../skills/clean/reference/safety-model.md | 7 ++- .../skills/clean/scripts/destructive_guard.py | 28 +++++---- .../skills/clean/scripts/test_hygiene.py | 62 +++++++++++++------ 5 files changed, 81 insertions(+), 45 deletions(-) diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index b439a4f885..5073346dfc 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -22,13 +22,14 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format fol - **Kill-switch delivery is a settings read, not a hook argument or environment variable.** The engine gate drops its `${user_config.*}` argument (fixing the hook-drop) and both surfaces call the new shared - `lib/killswitch_config.py` reader. The user `settings.json` is located from the tamper-resistant - `${CLAUDE_PLUGIN_ROOT}` both surfaces already receive (falling back to `CLAUDE_CONFIG_DIR`/`HOME` only - when `--plugin-root` is absent, e.g. the report CLI or unit tests). The environment is deliberately not - consulted for the toggle or the settings path: a repo `.claude/settings.json` `env` block reaches hook - subprocesses and carries no provenance, and since Claude Code 2.1.207 `pluginConfigs` is honored only from - user, managed, and `--settings` scope (project/local ignored), so a hostile repo cannot forge the value. - Every absent, unreadable, or ambiguous read fails **closed to enabled**. + `lib/killswitch_config.py` reader. The user `settings.json` is located **solely** from the + tamper-resistant `${CLAUDE_PLUGIN_ROOT}` both surfaces receive — the guard never falls back to + `CLAUDE_CONFIG_DIR`/`HOME` for it, because those are environment values a repo `.claude/settings.json` + `env` block can inject into hook subprocesses (carrying no provenance). A marker-less `--plugin-dir` + checkout root leaves no trusted user path, so the user scope is skipped and the switch relies on managed + settings, failing closed to enabled otherwise. Since Claude Code 2.1.207 `pluginConfigs` is honored only + from user, managed, and `--settings` scope (project/local ignored), so a hostile repo cannot forge the + value. Every absent, unreadable, or ambiguous read fails **closed to enabled**. - **Managed (enterprise) settings are honored as the highest-precedence scope.** The reader also reads the platform managed-settings.json (`/Library/Application Support/ClaudeCode/` on macOS, `/etc/claude-code/` on Linux/WSL, `C:\Program Files\ClaudeCode\` on Windows — a fixed path, not `%ProgramFiles%`-derived, so a diff --git a/plugins/disk-hygiene/lib/killswitch_config.py b/plugins/disk-hygiene/lib/killswitch_config.py index e50eac9dc9..7cca9dac4c 100644 --- a/plugins/disk-hygiene/lib/killswitch_config.py +++ b/plugins/disk-hygiene/lib/killswitch_config.py @@ -286,7 +286,7 @@ def _managed_effective( def resolve_effective( - settings_path: Path, + settings_path: Path | None, managed_settings_path: Path | None = None, plugin_id: str | None = None, ) -> bool: @@ -297,11 +297,19 @@ def resolve_effective( ``managed-settings.d/`` drop-in — wins over the user settings, which is how an organization enforces audit-only mode. When managed configures no value (all managed sources absent, entry-less, or unreadable/ambiguous) the user settings - decide. Every read is ``probe()``'s effective value, which fails **closed to - enabled**. + decide. + + ``settings_path`` is ``None`` when the caller has no *trusted* user-settings + location (e.g. a ``--plugin-dir`` install whose root carries no ``plugins/cache`` + marker, where the only remaining locator would be a repo-tamperable environment + variable). In that case the user scope contributes no verdict and, absent a + managed one, the switch fails **closed to enabled**. Every file read is + ``probe()``'s effective value, itself closed to enabled. """ if managed_settings_path is not None: managed_verdict = _managed_effective(managed_settings_path, plugin_id) if managed_verdict is not None: return managed_verdict + if settings_path is None: + return True return bool(probe(settings_path, plugin_id)["effective"]) diff --git a/plugins/disk-hygiene/skills/clean/reference/safety-model.md b/plugins/disk-hygiene/skills/clean/reference/safety-model.md index 62a9db3482..665dfc2f26 100644 --- a/plugins/disk-hygiene/skills/clean/reference/safety-model.md +++ b/plugins/disk-hygiene/skills/clean/reference/safety-model.md @@ -152,8 +152,11 @@ resolve `disk_hygiene_enabled` the same single way: by reading it from `pluginCo skill's `kill_switch_probe.py` reports). Neither surface takes the value from the process environment. Claude Code honors that key only from user, managed, and `--settings` scope since 2.1.207 — a project or local `.claude/settings.json` is ignored — so a hostile repo cannot flip it. The **user** file is located -from `${CLAUDE_PLUGIN_ROOT}` (the plugin's true install path, which a repo cannot forge), not from -`CLAUDE_CONFIG_DIR`/`HOME`, which a repo `settings.json` `env` block could redirect. The **managed** +from `${CLAUDE_PLUGIN_ROOT}` (the plugin's true install path, which a repo cannot forge) and **never** +from `CLAUDE_CONFIG_DIR`/`HOME`, which a repo `settings.json` `env` block could inject. A marker-less +install root (a `--plugin-dir` checkout, whose path has no `plugins/cache` segment) yields no trusted +user-settings path, so the user scope is skipped there and the switch relies on managed settings, failing +closed to enabled otherwise. The **managed** (enterprise) file at its fixed root-owned system path is read too and, as the highest-precedence non-overridable scope, an explicitly configured value there **wins over the user file** — so an organization can enforce audit-only mode; the sibling `managed-settings.d/` drop-in directory is merged diff --git a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py index a055dacb9f..ba3cf6d15c 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py +++ b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py @@ -444,25 +444,27 @@ def _plugin_id_from_root(plugin_root: str) -> str | None: return None -def _resolve_user_settings_path() -> Path: - """Locate the user settings file that carries the kill switch. - - Precedence, highest first: - - 1. Derived from ``--plugin-root ${CLAUDE_PLUGIN_ROOT}`` — the tamper-resistant - channel. Claude Code substitutes the plugin's true install path, which a - hostile repo cannot forge, so the settings file it points at is the real - user one regardless of any repo-supplied ``CLAUDE_CONFIG_DIR`` / ``HOME``. - 2. ``CLAUDE_CONFIG_DIR`` / ``HOME`` (via - ``killswitch_config.default_settings_path``) — the fallback for non-hook - invocations (the report CLI, unit tests) where ``--plugin-root`` is absent. +def _resolve_user_settings_path() -> Path | None: + """Locate the user settings file that carries the kill switch, or ``None``. + + The **only** trusted locator is ``--plugin-root ${CLAUDE_PLUGIN_ROOT}``: Claude + Code substitutes the plugin's true install path, which a hostile repo cannot + forge, and the user settings file is derived from its ``plugins/cache`` marker. + The guard deliberately does **not** fall back to ``CLAUDE_CONFIG_DIR`` / ``HOME``: + those are environment values, and a repo ``.claude/settings.json`` ``env`` block + reaches hook subprocesses, so trusting them would let a repo point the read at a + forged settings file and flip the switch (the exact provenance hole this design + closes). When the plugin root carries no marker — e.g. a ``--plugin-dir`` + checkout install — no trusted user-settings location exists and this returns + ``None``; the caller then relies on managed settings (fixed system paths) and + otherwise fails closed to enabled. """ plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG) if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER: derived = _user_settings_path_from_root(plugin_root) if derived: return Path(derived) - return killswitch_config.default_settings_path() + return None def resolve_disk_hygiene_enabled() -> bool: diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py index 4391935724..f6c6358585 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py @@ -2188,15 +2188,13 @@ def python_command() -> str: def setUp(self) -> None: # Hermetic kill switch: the guard resolves disk_hygiene_enabled by reading - # the user settings.json. Point it at an isolated CLAUDE_CONFIG_DIR whose - # settings.json we own (absent = enabled default; present-false = - # audit-only), independent of the developer's own ~/.claude/settings.json. - # The plain guard helpers use the CLAUDE_CONFIG_DIR fallback channel rather - # than injecting a --plugin-root, so they never perturb the guard's - # data-root authority resolution. The engine-gate helper (which must pass - # --plugin-root to mirror hooks.json) points it at the fake cache layout - # below, which derives back to the same owned settings.json — so both - # channels are hermetic and consistent no matter where the tests run. + # settings.json files. The plain guard helpers patch the guard's own + # `_resolve_user_settings_path` to our owned settings file directly (absent + # = enabled default; present-false = audit-only) and stub the managed path + # to a temp file — so they never touch the developer's real settings and + # never perturb data-root authority resolution. The engine-gate helper + # (which must pass --plugin-root to mirror hooks.json) points it at the fake + # cache layout below, which derives back to the same owned settings.json. self._cfg = tempfile.TemporaryDirectory() self.addCleanup(self._cfg.cleanup) cfg = Path(self._cfg.name) @@ -2240,8 +2238,10 @@ def _invoke_guard( mock.patch("sys.stdin", stdin), redirect_stdout(stdout), mock.patch.object(guard.sys, "argv", argv), - mock.patch.dict( - "os.environ", {"CLAUDE_CONFIG_DIR": self._cfg.name}, clear=False + # Drive the kill switch through the trusted resolver directly (no env, + # no --plugin-root), so data-root authority resolution is untouched. + mock.patch.object( + guard, "_resolve_user_settings_path", lambda: self._settings ), mock.patch.object( guard.killswitch_config, @@ -2825,7 +2825,6 @@ def test_guard_denies_data_root_without_hook_authority(self) -> None: for key, value in os.environ.items() if key != "CLAUDE_PLUGIN_DATA" } - environment["CLAUDE_CONFIG_DIR"] = self._cfg.name stdin = io.StringIO(json.dumps({"tool_input": {"command": command}})) stdout = io.StringIO() with ( @@ -2858,7 +2857,6 @@ def run_guard_hook_argv( for key, value in os.environ.items() if key != "CLAUDE_PLUGIN_DATA" } - environment["CLAUDE_CONFIG_DIR"] = self._cfg.name stdin = io.StringIO(json.dumps({"tool_input": {"command": command}})) stdout = io.StringIO() with ( @@ -2968,7 +2966,6 @@ def run_guard_plugin_root( for key, value in os.environ.items() if key != "CLAUDE_PLUGIN_DATA" } - environment["CLAUDE_CONFIG_DIR"] = self._cfg.name stdin = io.StringIO(json.dumps({"tool_input": {"command": command}})) stdout = io.StringIO() with ( @@ -3506,9 +3503,10 @@ class DirectReadKillSwitchTests(unittest.TestCase): Post-C′ the guard ignores the ``--disk-hygiene-enabled`` argv flag and the ``CLAUDE_PLUGIN_OPTION_DISK_HYGIENE_ENABLED`` env var (both repo-tamperable or un-delivered) and instead reads ``disk_hygiene_enabled`` out of the - user's ``settings.json`` ``pluginConfigs``. The settings file is located - from the tamper-resistant ``--plugin-root`` (``${CLAUDE_PLUGIN_ROOT}``) - first, then the ``CLAUDE_CONFIG_DIR``/``HOME`` env fallback. Every + ``settings.json`` ``pluginConfigs``. The user file is located **solely** from + the tamper-resistant ``--plugin-root`` (``${CLAUDE_PLUGIN_ROOT}``), never an + environment value; a managed policy at its fixed system path overrides it, and + a marker-less (``--plugin-dir``) root leaves no trusted user path. Every absent/degraded read fails closed to enabled (safety on). """ @@ -3612,12 +3610,36 @@ def test_legacy_argv_flag_is_ignored(self) -> None: ) ) - def test_config_dir_env_fallback_when_no_plugin_root(self) -> None: - self.write_toggle(False) - self.assertFalse( + def test_no_plugin_root_ignores_env_config_dir_and_fails_closed_enabled( + self, + ) -> None: + # Without a trusted --plugin-root (e.g. a --plugin-dir checkout install), + # the guard must NOT trust a repo-injectable CLAUDE_CONFIG_DIR: even a + # configured `false` reachable only through the environment is ignored and + # the switch fails closed to enabled, so a repo cannot forge a settings + # path to flip it. + self.write_toggle(False) # config_dir/settings.json + self.assertTrue( self.resolve([], {"CLAUDE_CONFIG_DIR": os.fspath(self.config_dir)}) ) + def test_plugin_dir_install_ignores_env_user_settings_but_honors_managed( + self, + ) -> None: + # A --plugin-dir root carries no plugins/cache marker, so there is no + # trusted user-settings path: a repo-injectable CLAUDE_CONFIG_DIR is + # ignored. A managed policy (fixed system path) is still enforced. + checkout = self.config_dir / "checkout" # marker-less root + checkout.mkdir() + checkout_argv = ["--plugin-root", os.fspath(checkout)] + env = {"CLAUDE_CONFIG_DIR": os.fspath(self.config_dir)} + self.write_toggle(False) # reachable only via env -> ignored + self.write_managed_toggle(False) # managed policy -> honored + self.assertFalse(self.resolve(checkout_argv, env)) + # Without the managed policy, the marker-less install fails closed to enabled. + self.managed.unlink() + self.assertTrue(self.resolve(checkout_argv, env)) + def test_plugin_root_channel_beats_tamperable_config_dir_env(self) -> None: """A repo-injected CLAUDE_CONFIG_DIR cannot override the real settings.