Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/disk-hygiene/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
51 changes: 51 additions & 0 deletions plugins/disk-hygiene/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<pid>.<random>` 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
42 changes: 35 additions & 7 deletions plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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."
Expand Down
29 changes: 24 additions & 5 deletions plugins/disk-hygiene/skills/clean/scripts/hygiene.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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", []
)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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", []
)
Expand Down
121 changes: 121 additions & 0 deletions plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down