diff --git a/plugins/disk-hygiene/.claude-plugin/plugin.json b/plugins/disk-hygiene/.claude-plugin/plugin.json index 5adc898f0e..21ab1e1331 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.12.0", + "version": "0.13.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 adccc0540a..659f8a12dc 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -3,6 +3,57 @@ 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.13.0] + +> Version note: `0.11.0` is claimed by #1804 (PR #1818) and `0.12.0` by #1805 (PR #1819), both open +> against this manifest. This entry takes the next number so the three do not collide; merged in +> issue order the changelog reads contiguously. + +### Fixed + +- **Hint matching is case-insensitive, on every platform (#1806).** `has_protected_name()` casefolds + and `matching_hints()` did not, so on Windows and macOS — where both spellings name the *same* + file — protection was case-robust while discovery was not. Measured against the shipped baseline + before the fix: `Thumbs.db`, `tmp-build`, and `scratch.md` each matched a hint while `thumbs.db`, + `TMP-build`, and `Scratch.md` matched nothing. + + Every glob the engine evaluates now goes through one `glob_matches()` helper — hints, consumer + protection globs, and the protection re-checks in the preview, verify, and apply lanes — so + discovery and protection cannot disagree about what a name is. The protection-side globs move + deliberately rather than by accident, and casefolding is the safe direction for both roles: a + protection glob that matches more can only keep more, and a hint that matches more can only + surface more for triage, since hints are discovery signals and never cleanup verdicts. The helper + uses `fnmatchcase` on casefolded operands rather than `fnmatch`, whose folding follows the host + platform — a matcher whose verdict changes with where the scan runs is not a matcher a protection + can rest on. + +- **Atomic-write staging remnants are hinted as a class, not as one producer's filename (#1806).** + `*.tmp` requires `.tmp` as a *suffix* and `.claude.json.tmp.*` encodes one producer's exact + prefix. Neither matches `.tmp` as an **infix** before a pid and random suffix — the standard + write-temp-then-rename shape — while the producer-specific hint's own `reason` claimed to cover + the class. A scan of one sibling plugin's state directory returned **`hinted_entries: 0` across 63 + entries**, 61 of which were remnants of exactly that shape; they surfaced only because a subagent + read the directory positionally. + + A new `atomic-write-staging-remnant` hint (`*.tmp.*`, ceiling `medium`) covers the class. + `.rate-limits.json.tmp..` and `settings.json.tmp.4` now hint where they previously + matched nothing. The producer-specific hint still fires alongside it, since it carries a narrower + reason and a class hint does not replace that. + +- **The Bash denial text no longer under-reports the allow-list (#1806).** The documented bootstrap + path is to submit a wrong shape so the denial teaches the grammar, and it enumerated four engine + subcommands while omitting the read-only kill-switch probe that `_decide` allows before the + classifier ever runs. A consumer learning the allow-list from the denial never learned the probe + is permitted — and the probe is the step that lets the model state the kill-switch value honestly + instead of assuming the default. The denial now also discloses the bundled engine's own path, + which is the only route left when a rendered body's `${CLAUDE_PLUGIN_ROOT}` arrives unexpanded and + the exact-path identity check denies every guess. + + The enumeration and the grammar are now one list: `classify_exact_engine_command` rejects any + subcommand outside `_ALLOWED_ENGINE_SUBCOMMANDS` before its own dispatch, and both bundled script + paths come from one accessor each, so the message cannot teach a grammar the classifier does not + implement. + ## [0.12.0] > Version note: `0.11.0` is claimed by the cloud-placeholder fix (#1804, PR #1818), which is open diff --git a/plugins/disk-hygiene/skills/clean/reference/baseline-policy.json b/plugins/disk-hygiene/skills/clean/reference/baseline-policy.json index 74b7dad681..e1a40ebc7f 100644 --- a/plugins/disk-hygiene/skills/clean/reference/baseline-policy.json +++ b/plugins/disk-hygiene/skills/clean/reference/baseline-policy.json @@ -102,6 +102,14 @@ "confidence_ceiling": "low", "reason": "Browser partial download; browser process and resume state must be checked" }, + { + "id": "atomic-write-staging-remnant", + "os": ["all"], + "kind": "name_glob", + "pattern": "*.tmp.*", + "confidence_ceiling": "medium", + "reason": "Atomic-write staging remnant: a '.tmp' infix before a pid/random suffix is the standard write-temp-then-rename shape, so the producer crashed or was interrupted before the rename; identify the producer and confirm no live process owns it" + }, { "id": "claude-json-failed-atomic-write", "os": ["all"], diff --git a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py index b65a1fafd7..dc83f738bc 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py +++ b/plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py @@ -269,6 +269,31 @@ def _within_plugin_cache_family(value: str) -> bool: # Different drives on Windows; nothing on another volume is in the tree. return False + +# The subcommands `classify_exact_engine_command` accepts, named once so the +# denial text cannot teach a grammar the classifier does not implement. The +# classifier rejects anything outside this tuple before its own dispatch, so a +# subcommand added to one and not the other fails closed rather than drifting +# (#1806). +_ALLOWED_ENGINE_SUBCOMMANDS = ("scan", "preview", "handoff-verify", "apply") + + +def _engine_script_path() -> Path: + """The one bundled engine path both the classifier and the denial disclose.""" + return Path(__file__).resolve().with_name(_ENGINE_MARKER) + + +def _probe_script_path() -> Path: + """The one bundled kill-switch probe path, likewise shared.""" + return ( + Path(__file__).resolve().parents[2] / "setup" / "scripts" / "kill_switch_probe.py" + ) + + +def _display_path(path: Path) -> str: + """A Bash-friendly absolute spelling of a bundled script path.""" + return os.fspath(path).replace("\\", "/") + # Everything a path we care about cannot contain. Enumerating SHELL syntax here # would be a losing game — an assignment (`engine=hygiene.py`), a list separator # (`PATH=/x:hygiene.py`), and a metacharacter (`foo;hygiene.py`) each glue the @@ -811,8 +836,9 @@ def classify_exact_engine_command(command: str, authority: str | None) -> str | return None if len(tokens) < 3 or not _is_current_python(tokens[0]): return None - expected_script = str(Path(__file__).resolve().with_name("hygiene.py")) - if _script_path_key(tokens[1]) != _script_path_key(expected_script): + if _script_path_key(tokens[1]) != _script_path_key(str(_engine_script_path())): + return None + if tokens[2] not in _ALLOWED_ENGINE_SUBCOMMANDS: return None if tokens[2] == "scan": @@ -904,10 +930,7 @@ def is_exact_kill_switch_probe(command: str) -> bool: tokens = _literal_shell_words(command) if tokens is None or len(tokens) != 2 or not _is_current_python(tokens[0]): return False - expected_script = str( - Path(__file__).resolve().parents[2] / "setup" / "scripts" / "kill_switch_probe.py" - ) - return _script_path_key(tokens[1]) == _script_path_key(expected_script) + return _script_path_key(tokens[1]) == _script_path_key(str(_probe_script_path())) _POWERSHELL_MUTATION_WORDS = re.compile( @@ -1012,8 +1035,13 @@ def _bash_denial_guidance(authority: str | None) -> str: " validated and engine calls fail closed." ) ) + subcommands = ", ".join(_ALLOWED_ENGINE_SUBCOMMANDS[:-1]) return ( - "Disk-hygiene fails closed: Bash is restricted to exact bundled scan, preview, handoff-verify, and apply invocations using the hook's absolute Python interpreter " + "Disk-hygiene fails closed: Bash is restricted to exact bundled " + f"{subcommands}, and {_ALLOWED_ENGINE_SUBCOMMANDS[-1]} invocations of " + f'"{_display_path(_engine_script_path())}", plus the argument-free ' + f'read-only kill-switch probe "{_display_path(_probe_script_path())}" — ' + "all of them using the hook's absolute Python interpreter " f'"{_display_python()}". Bare python/python3 commands are denied because shell functions and aliases can replace them.' + data_sentence + " Use non-Bash read-only tools for supporting inspection." diff --git a/plugins/disk-hygiene/skills/clean/scripts/hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/hygiene.py index c08d884849..d18d2205b9 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/hygiene.py @@ -135,6 +135,25 @@ def os_key() -> str: ) +def glob_matches(subject: str, pattern: str) -> bool: + """Case-insensitive glob match, on every platform. + + One matcher for every glob the engine evaluates — hints, consumer + protection globs, and the protection re-checks in the preview, verify, and + apply lanes — so discovery and protection cannot disagree about what a name + is. `fnmatch.fnmatch` is not that matcher: its case folding follows the host + platform, so its verdict would change with where the scan runs. + + Casefolding is the safe direction for both roles. A protection glob that + matches more can only keep more, and on Windows and macOS the filesystem is + case-insensitive anyway, so a case-sensitive protection glob was a hole + rather than a precision. A hint that matches more can only surface more for + triage — hints are discovery signals, never cleanup verdicts. This also + aligns globs with `has_protected_name`, which has always casefolded. + """ + return fnmatch.fnmatchcase(subject.casefold(), pattern.casefold()) + + def is_within(path: Path, parent: Path) -> bool: try: return os.path.commonpath( @@ -594,7 +613,7 @@ def matching_hints( if "all" not in hint["os"] and current_os not in hint["os"]: continue subject = name if hint["kind"] == "name_glob" else relative - if fnmatch.fnmatchcase(subject, hint["pattern"]): + if glob_matches(subject, hint["pattern"]): matches.append( { "id": hint["id"], @@ -830,7 +849,7 @@ def visit(directory: Path, depth: int = 1) -> int: if path.name.casefold() in VCS_NAMES: repositories.append(path.parent.resolve()) if any( - fnmatch.fnmatchcase(relative, pattern) + glob_matches(relative, pattern) for pattern in policy["additional_protected_path_globs"] ): protections.append("consumer-protected-path") @@ -1508,7 +1527,7 @@ def preview(snapshot: dict[str, Any], plan: dict[str, Any]) -> dict[str, Any]: blockers.extend(hard_protection(current, target, exact_names, known_mounts)) relative_current = current.relative_to(target).as_posix() if any( - fnmatch.fnmatchcase(relative_current, pattern) + glob_matches(relative_current, pattern) for pattern in snapshot.get("policy", {}).get( "additional_protected_path_globs", [] ) @@ -1674,7 +1693,7 @@ def handoff_verify(snapshot: dict[str, Any], approved: list[str]) -> dict[str, A ) relative_current = current.relative_to(target).as_posix() if any( - fnmatch.fnmatchcase(relative_current, pattern) for pattern in globs + glob_matches(relative_current, pattern) for pattern in globs ): contested.add("consumer-protected-path") if not truncated: @@ -1919,7 +1938,7 @@ def apply_plan(snapshot: dict[str, Any], plan: dict[str, Any]) -> dict[str, Any] fresh_mounts, ) if any( - fnmatch.fnmatchcase(relative, pattern) + glob_matches(relative, pattern) for pattern in snapshot.get("policy", {}).get( "additional_protected_path_globs", [] ) diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py index 38f3ec1499..c55a62c54d 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py @@ -130,6 +130,100 @@ def test_policy_can_disable_hints_and_only_add_protection(self) -> None: ) self.assertIn("NTUSER.DAT", policy["protected_exact_names"]) + def test_hints_match_a_name_whatever_its_case(self) -> None: + # Protection casefolds and discovery did not, so on Windows and macOS — + # where both spellings name the SAME file — an entry could be protected + # case-robustly while being invisible to triage. + policy = hygiene.load_policy(None) + for name, expected in ( + ("tmp-build", "common-temp-directory"), + ("TMP-build", "common-temp-directory"), + ("scratch.md", "scratch-artifact"), + ("Scratch.md", "scratch-artifact"), + ("failed-write.tmp", "common-temp-file"), + ("FAILED-WRITE.TMP", "common-temp-file"), + ): + matched = {hint["id"] for hint in hygiene.matching_hints(name, name, policy)} + self.assertIn(expected, matched, name) + + def test_platform_scoped_hints_are_also_case_insensitive(self) -> None: + # Separated from the OS-agnostic rows and run under a pinned os_key: + # `matching_hints` filters by the current OS BEFORE matching, so asserting + # a windows-only hint on a Linux runner tests the OS filter, not the case + # discipline this is about. + policy = hygiene.load_policy(None) + with mock.patch.object(hygiene, "os_key", return_value="windows"): + for name in ("Thumbs.db", "thumbs.db", "THUMBS.DB"): + matched = { + hint["id"] for hint in hygiene.matching_hints(name, name, policy) + } + self.assertIn("windows-explorer-metadata", matched, name) + with mock.patch.object(hygiene, "os_key", return_value="macos"): + for name in (".DS_Store", ".ds_store"): + matched = { + hint["id"] for hint in hygiene.matching_hints(name, name, policy) + } + self.assertIn("macos-finder-metadata", matched, name) + + def test_atomic_write_staging_remnants_are_hinted_as_a_class(self) -> None: + # The producer-specific hint encodes one filename while its own reason + # claims the class. `.tmp` as an INFIX before a pid/random suffix is the + # standard write-temp-then-rename shape, and a scan of one plugin's + # state directory returned zero hinted entries across 63, of which 61 + # were remnants of exactly this shape. + policy = hygiene.load_policy(None) + for name in ( + ".rate-limits.json.tmp.1363789.17391", + "settings.json.tmp.4", + ".claude.json.tmp.9552.9bfba4e83eaa", + ): + matched = {hint["id"] for hint in hygiene.matching_hints(name, name, policy)} + self.assertIn("atomic-write-staging-remnant", matched, name) + # The producer-specific hint still fires alongside it: it carries a + # narrower reason, and a class hint does not replace that. + matched = { + hint["id"] + for hint in hygiene.matching_hints( + ".claude.json.tmp.9552.9bfba4e83eaa", + ".claude.json.tmp.9552.9bfba4e83eaa", + policy, + ) + } + self.assertIn("claude-json-failed-atomic-write", matched) + + def test_consumer_protection_globs_match_whatever_the_case(self) -> None: + # The same matcher serves hints and protection, so this moves with the + # hint change deliberately rather than by accident. Casefolding a + # protection glob can only ever keep more. + with tempfile.TemporaryDirectory() as temporary: + policy_path = Path(temporary) / "policy.json" + policy_path.write_text( + json.dumps( + { + "version": 1, + "disabled_hint_ids": [], + "additional_hints": [], + "additional_protected_path_globs": ["Deliverables/**"], + } + ), + encoding="utf-8", + ) + root = Path(temporary) / "target" + (root / "deliverables").mkdir(parents=True) + (root / "deliverables" / "report.tmp").write_text("x", encoding="utf-8") + snapshot = hygiene.scan_tree( + root.resolve(), hygiene.load_policy(policy_path) + ) + entries = hygiene.entry_map(snapshot) + # `Deliverables/**` matches the descendants, not the directory + # entry itself — the pattern's own semantics, unchanged here. What + # changes is that a `Deliverables` pattern now reaches a + # `deliverables` path. + self.assertIn( + "consumer-protected-path", + entries["deliverables/report.tmp"]["protected_reasons"], + ) + def test_policy_rejects_non_array_boundary_input(self) -> None: with tempfile.TemporaryDirectory() as temporary: policy_path = Path(temporary) / "policy.json" @@ -3073,6 +3167,33 @@ def test_cache_family_narrowing_is_inert_for_a_checkout_install(self) -> None: self.assertIsNone(guard._plugin_cache_family_root()) self.assertFalse(guard._within_plugin_cache_family(str(SCRIPT_DIR))) + def test_bash_denial_names_every_shape_the_classifier_accepts(self) -> None: + # The documented bootstrap path is to submit a wrong shape so the denial + # teaches the grammar. It enumerated four engine subcommands and omitted + # the read-only kill-switch probe, which `_decide` allows before it ever + # reaches the classifier — so a consumer learning the allow-list from + # the denial never learned the probe is permitted, and the probe is the + # step that lets the model state the kill-switch value honestly instead + # of assuming the default. + guidance = guard._bash_denial_guidance("/data/root") + for subcommand in guard._ALLOWED_ENGINE_SUBCOMMANDS: + self.assertIn(subcommand, guidance, subcommand) + self.assertIn("kill_switch_probe.py", guidance) + # The engine's own path: without it, a body whose ${CLAUDE_PLUGIN_ROOT} + # arrived unexpanded leaves no disclosed route to the engine, and the + # exact-path identity check denies every guess. + self.assertIn(guard._display_path(guard._engine_script_path()), guidance) + + def test_classifier_rejects_a_subcommand_outside_the_shared_list(self) -> None: + """The denial text and the grammar are one list, so they cannot drift.""" + python = guard._display_python() + engine = guard._display_path(guard._engine_script_path()) + self.assertIsNone( + guard.classify_exact_engine_command( + f'"{python}" "{engine}" summarize --snapshot s', None + ) + ) + def test_engine_gate_defers_files_whose_name_merely_ends_in_the_marker( self, ) -> None: