fix(disk-hygiene): hard-protect cloud-sync placeholders and tenant sync roots - #1818
Conversation
…nc roots The engine's only structural defense against cloud-sync content was is_linkish(), which treats a Windows reparse point as protected. The dominant OneDrive dehydrated-placeholder class carries no reparse bit when read through os.lstat, so the whole subtree was walked and every placeholder was recorded as an ordinary file with empty protected_reasons. Its logical_size is the remote byte count while local occupancy is roughly zero, so the tree also looked like the largest reclaimable win on the volume — and deleting a placeholder propagates the delete to the provider, which for a tenant sync root is the organisation's only copy. Measured before the fix: 1,101 files walked, 872 dehydrated placeholders totalling 13,770,936,008 bytes, 0 of 872 flagged by is_linkish(). hard_protection() now contributes a cloud-placeholder reason from the file attributes it already reads, so the protection reaches scan, preview, handoff-verify, and apply's pre-removal recheck through one predicate. Both flags come from a single lstat per ancestor, so the walk's stat load is unchanged. FILE_ATTRIBUTE_RECALL_ON_OPEN is deliberately excluded, against the obvious reading of the attribute names. Its value 0x00040000 is the same number as FILE_ATTRIBUTE_EA, and RECALL_ON_OPEN appears only in directory enumeration classes while every read here comes from lstat, so through lstat the bit means "has extended attributes". A 412,270-entry sweep of three non-cloud trees found 1,552 fully-local files carrying it — .NET build output and temp .node files — which the engine exists to reclaim. Excluding it leaves zero false positives while the tenant tree still flags correctly. Attribute protection alone is not enough: measurement showed the containing directories carry no cloud attribute at all (the sync root reads 0x31, all 99 subdirectories read plain 0x10), so a fully hydrated sync root would have no protected descendant and stay deletable. Name protection was exact-match and shipped the literal OneDrive only, so OneDrive - <Organization> matched nothing. The baseline gains protected_name_globs holding "OneDrive - *", matched casefolded so the verdict does not depend on the host's case rules, plus Box, Dropbox, Google Drive, and iCloudDrive as exact names. Consumers could not have closed this themselves: additional_protected_path_globs are matched relative to the scan target, so a standing overlay protects such a root only when the target happens to be its parent. Every entry also records file_attributes and a size_qualifiers list, so a placeholder's remote logical_size can never be read as reclaimable local bytes. Additive per-entry trace only; no aggregate's definition changes here. The clean skill's positional-triage rule now reads an entry's own protected_reasons instead of testing membership of protected_exact_names, which would have walked straight past this entry even after the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C
|
Claude finished @kyle-sexton's task in 5m 15s —— View job PR Review: fix(disk-hygiene): hard-protect cloud-sync placeholders
SummaryThis is a well-engineered fix for a genuine critical safety failure — 872 dehydrated OneDrive placeholders totalling ~13 GB were recorded as Findings1.
|
|
Claude finished @kyle-sexton's task in 4m 33s —— View job Security Review — PR #1818
Scope: No critical or important security findingsThe new code paths are purely additive to the protection predicate — they add reasons for blocking deletion, never remove them. None of the additions introduce command execution, subprocess calls, external network access, or user-controlled input that reaches fnmatch or the filesystem in an exploitable way. The following points were examined and found acceptable: Bitmask correctness and OR semantics
return bool(getattr(info, "st_file_attributes", 0) & CLOUD_PLACEHOLDER_ATTRIBUTES)This fires if either flag is set — deliberately broad for a protection predicate (over-protecting is safe; under-protecting is not). Both hex values match Microsoft's documented constants. The excluded Glob pattern injection surface The
except OSError:
return False, FalseAn unreadable path is treated as neither a link nor a cloud placeholder. This is the same behaviour the original
The cache is correct in production: the bundled JSON is a build-time constant and should not change while the process runs. The cache does create a test-isolation hazard when a test mocks
TOCTOU between scan and apply for cloud placeholder state If a file is hydrated (downloaded from cloud to local) after scan and before apply, Casefold platform independence Both sides of the ConclusionNo security vulnerabilities found. The changes are defensively structured: all new predicate paths are strictly additive to protection, user-controlled data never reaches glob compilation or command execution, and the bitmask and glob-matching choices are well-documented and correctly implemented. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b221922ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…x variants A protected name applies at every depth, and a protected directory is never traversed and reports logical_size: 0 — byte-identical to a genuinely empty directory. Over-protection is therefore not free in a reclamation tool: it silently under-reports. The first pass took four sync-root names from the report without weighing that. Box is a common enough directory name in source trees that protecting it at every depth would make ordinary directories untraversable and silently zero-sized. Google Drive is a legacy Backup-and-Sync name: current Google Drive for desktop streams to a virtual drive letter, G: by default on Windows, not to a folder under the user profile (https://support.google.com/drive/answer/13470231). Both are dropped. Dropbox documents its folder as "Dropbox (Personal)" or "Dropbox (<business name>)" as well as the bare name, so the bare exact name alone would miss the documented shapes; the glob "Dropbox (*)" is added alongside it. iCloudDrive is kept on name alone. Its exact default folder name could not be confirmed from an official Apple page in two attempts, and it is recorded as unverified rather than asserted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C
|
Claude finished @kyle-sexton's task in 3m 12s —— View job Review — PR #1818 incremental commit (
|
| Name | Matches | Expected |
|---|---|---|
Dropbox (Personal) |
✓ | ✓ |
Dropbox (Acme Inc) |
✓ | ✓ |
Dropbox (Personal) backup |
✗ | ✗ (not a sync root) |
Dropbox |
via exact name | ✓ |
The bare Dropbox exact name continues to fire on the original install shape. The glob and the exact name are complementary and cover the documented naming space without overlap issues.
3. Box and Google Drive removal — correct judgment
The commit message cites two concrete sources for these removals:
Box: Over-broad becauseBoxis a common directory name in source trees. Box.com's sync client would need the cloud-attribute predicate (already inhard_protection) to catch its placeholders on Windows, not a name protection. Removing it avoids silently zeroing-out legitimate directories namedBox.Google Drive: Current Google Drive for desktop on Windows mounts via a virtual drive letter (G:by default), documented at the linked support page. AGoogle Drivefolder in the user profile belongs to the legacy Backup-and-Sync client, which is no longer distributed.
Both removals are defensible and the CHANGELOG documents the rationale transparently. The note that Box and non-OneDrive roots are covered only by the attribute predicate on Windows (and not at all on Linux/macOS) is honest about residual exposure.
4. iCloudDrive name uncertainty — acknowledged, not worsened
The commit explicitly records that the exact folder name could not be confirmed from an official Apple source. iCloudDrive was already present before this commit; this change does not regress it. A follow-up issue to pin the macOS CloudStorage path (per the prior review's finding 3) is appropriate.
What looks correct
- Glob-in-
protected_name_globsis the right fix surface: Callers ofhas_protected_namecannot suppress the glob check (it fires unconditionally frombaseline_protected_name_globs()), so addingDropbox (*)here gives the same unconditional protection as the earlierOneDrive - *glob. A consumer overlay cannot override it. - CHANGELOG is exemplary: Explicitly records what was rejected and why, cites sources, and states residuals. This is the correct level of transparency for a safety-critical baseline.
- No logic changes in
hygiene.py: This commit touches only the policy JSON and the changelog. The matching logic implemented ine081e5eis identical to what was already tested and confirmed forOneDrive - *; only the policy data changes.
Summary
The Codex finding is correctly addressed. The Box and Google Drive removals are well-reasoned. The only gap worth addressing before merge is a test exercising the Dropbox (*) glob — without it the new protection has no regression anchor. Finding 1 is the one I'd action; findings 2–4 are observations, not blockers.
|
Claude finished @kyle-sexton's task in 3m 13s —— View job Security Review — PR #1818 (Incremental Pass)
This pass covers the incremental changes on top of the prior security review, which already verified bitmask correctness, glob-injection surfaces, No new critical or important security findingsThe second commit ( FindingsSUGGESTION · low confidence —
|
…lity Two properties of the new protection were argued rather than checked, and both are the kind that fail silently. A forged or older snapshot must not be able to weaken name-pattern protection. The patterns are read from the bundled baseline rather than from the snapshot's own policy, so clearing both the entries' protected_reasons and the snapshot policy's protected_name_globs still leaves preview blocking the candidate with baseline-protected-name and withholding the approval token. The exact-name equivalent was already pinned; the pattern path needs its own, because it reads from a different source. A snapshot written by a previous engine carries neither size_qualifiers nor file_attributes. Cached plugin versions linger well past their documented two-week cleanup window, so such a snapshot has to stay previewable rather than raising on a missing key. Stripping both fields still previews clean and still mints a token. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C
|
Claude finished @kyle-sexton's task in 3m 15s —— View job Review — PR #1818 (incremental pass:
|
|
Claude finished @kyle-sexton's task in 5m 13s —— View job Security Review — PR #1818 (final incremental pass:
|
…holder hard_protection's ancestor walk runs up to and INCLUDING the target, so a scan target that itself carries a recall/offline bit marked EVERY entry cloud-placeholder — and scan_tree truncates any directory with protections, so the whole walk collapsed with no diagnostic. That is the same failure the mount-point branch already guards against, for the same reason, so it takes the same exemption: ancestors below the target still contribute, and the target's own entry still reports the reason, so the condition stays visible instead of being swallowed. Also corrects two things the fix's own reporting surface and name list got wrong: SKILL.md section 3 never mentioned size_qualifiers, so an agent could still total a qualified entry's logical_size as reclaimable — the exact misreading the field was added to prevent. It now states that logical_size is reclaimable local bytes only when size_qualifiers is empty, and that qualified bytes are reported separately with their reasons. The iCloud Drive folder name is "iCloud Drive" with a space, per Apple's documented Windows path C:\Users\[username]\iCloud Drive, not "iCloudDrive" — verified against Apple's iCloud for Windows guide rather than assumed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C
|
Claude finished @kyle-sexton's task in 3m 49s —— View job Code Review — PR #1818 (latest commit
|
|
Claude finished @kyle-sexton's task in 2m 55s —— View job Security Review — PR #1818 (incremental pass:
|
|
Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…ve-letter user path The machine-specific-paths hygiene check reads the documented Windows path spelling as a machine-specific absolute path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STASxNiqEm7doy2Z65un4H
|
Claude finished @kyle-sexton's task in 1m 18s —— View job Code Review — PR #1818 (latest commit
|
|
Claude finished @kyle-sexton's task in 31s —— View job Security Review — PR #1818 (incremental pass since
|
…n cache tree (#1819) ## Summary The engine gate defers on a word naming an existing file that is not the bundled engine, so a consumer's own `tools/hygiene.py` is not mistaken for this engine (#1640, #1611). Claude Code keeps a replaced version's directory on disk after an update, so that same escape also covered every **previous version of this engine** sitting beside the current one — each a genuinely different file, each deletion-capable, and each answering to nothing but its own containment once the always-on gate defers. The consequence is a kill-switch bypass, not an unbounded-delete bypass: with `disk_hygiene_enabled: false` the plugin-level gate is the only guard whenever the clean skill is not the active work, and it deferred. The stale engine's own preview, approval-token, and platform blockers still applied. Versions at or below 0.8.1 predate settings-based kill-switch enforcement entirely. ## Fix The escape is refused to any path resolving inside `<plugins>/cache/<marketplace>/<name>`, at both sites that grant it — the literal-parse branch and the `_provably_other_file` test in the unparsable branch. One deviation from the issue's suggestion: the prefix is derived from the **guard module's own `__file__`** rather than from `_plugin_data_root_from_root()`'s `--plugin-root` argv value. `__file__` is the file actually executing, so it needs no argv channel and nothing outside the process can redirect it — the same reasoning that already keeps the kill-switch read off `CLAUDE_CONFIG_DIR`. A `--plugin-dir` checkout carries no such prefix and the narrowing is inert there, which is deliberate: a checkout has no cached siblings, and narrowing on it would gate a contributor's every command naming their own working tree's engine. That inertness is pinned by its own test rather than left implicit. `reference/safety-model.md` gains a paragraph stating the narrowing and its residual, since that document is where the gate's accepted-residual list lives. ## Verification **The reported population, on the audit host** — `~/.claude/plugins/cache/melodic-software/disk-hygiene/`: - 17 version directories, `0.3.0` through `0.10.2`, matching the report exactly. - 16 carry an intact engine; `0.9.4`'s is absent, also as reported. - `0.3.0` is still present, so the plugins reference's "about two weeks before cleanup" does not bound this in practice. **The escape, against the installed 0.10.2 guard** loaded exactly as the hook loads it: all 15 non-current engines resolve, are not `samefile` with the bundled one, and the gate **deferred on an `apply --execute` invocation of every one of them**. Control: 0.10.2's own bundled engine still gates. No engine was executed — the probe only resolves paths, compares identity, and calls the classifier on command strings. **The consequence the issue deliberately left undemonstrated**, isolated rather than run against a live install with the switch off. Two identical synthetic cache layouts (`<base>/plugins/cache/market/disk-hygiene/{0.9.0,0.10.2}/…`), one built with the guard from `origin/main` and one with the fixed guard, asked the same four questions: | Command shape | Before | After | |---|---|---| | Stale sibling engine, plain | defers | **GATES** | | Stale sibling engine, beside an operator | defers | **GATES** | | The install's own bundled engine | GATES | GATES | | A consumer's own engine-named tool outside the cache | defers | defers | The last two rows are the point: the narrowing does not merely move which copy is unguarded, and it does not re-break what #1640 and #1611 fixed. **Gates:** - `bash plugins/disk-hygiene/skills/clean/scripts/hygiene.test.sh` — 226 tests, OK (4 skipped); 3 new tests, which build a real `plugins/cache/…` layout rather than stubbing the predicate, so they exercise the actual derivation. - `bash scripts/check-changelog-parity.sh --check-order` — pass. - `check-skill.sh clean` — PASS, 0 errors. - `markdownlint-cli2` on both changed markdown files — 0 errors; `ruff check` on the guard — clean. **Residual, unchanged:** a *copied* engine carried outside the cache tree is still outside the prefix, as it is outside every identity check the gate makes. That is the copy-evasion class the gate has always accepted. ## Related - Refs #1804 / PR #1818 — the other CRITICAL from the same audit, against the engine rather than the gate. **PR #1818 should merge first**: it claims `0.11.0` on this same manifest, so this PR takes `0.12.0` to avoid a duplicate version. The changelog reads contiguously once both land, and the only conflict is a keep-both-in-order in `CHANGELOG.md`. - Refs #1640, #1611 — the over-gating fixes whose escape this narrows; both behaviours are re-pinned here. Fixes #1805 🤖 Generated with [Claude Code](https://claude.com/claude-code) <https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…nial allow-list (#1820) ## Summary Takes findings **1** and **5** of #1806. The issue bundles seven findings; all seven are verified in [a comment on the issue](#1806 (comment)), and this PR deliberately does **not** close it — five findings remain open, three of them needing a maintainer's call rather than an implementation. ## Fix **Finding 1 — hint matching was misanchored and case-sensitive in the unsafe direction.** `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. All six `fnmatchcase` call sites now go through one `glob_matches()` helper: hints, consumer protection globs, and the protection re-checks in the preview, verify, and apply lanes. That is the part the issue flagged as needing deliberate handling — the protection globs move **with** the hints rather than by accident. 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 casefolds both operands and keeps `fnmatchcase` rather than switching to `fnmatch`, whose folding follows the host platform — a matcher whose verdict changes with where the scan runs is not one a protection can rest on. A new `atomic-write-staging-remnant` hint (`*.tmp.*`, ceiling `medium`) covers the class the producer-specific hint's own `reason` already claimed: `.tmp` as an **infix** before a pid and random suffix, the standard write-temp-then-rename shape. The producer-specific hint still fires alongside it, since it carries a narrower reason. **Finding 5 — the Bash denial text under-reported the allow-list.** It enumerated four engine subcommands and omitted the read-only kill-switch probe that `_decide` allows *before* the classifier ever runs. Since the documented bootstrap path is to submit a wrong shape so the denial teaches the grammar, 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 names the probe and discloses the bundled engine's own path, 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, as suggested: `classify_exact_engine_command` rejects any subcommand outside `_ALLOWED_ENGINE_SUBCOMMANDS` before its own dispatch, and both bundled script paths come from a single accessor each, so the message cannot teach a grammar the classifier does not implement. ## Verification Running `matching_hints()` against the shipped baseline, before and after: | Name | Before | After | |---|---|---| | `.rate-limits.json.tmp.1363789.17391` | none | `atomic-write-staging-remnant` | | `settings.json.tmp.4` | none | `atomic-write-staging-remnant` | | `.claude.json.tmp.9552.9bfba4e83eaa` | `claude-json-failed-atomic-write` | both hints | | `thumbs.db` | none | `windows-explorer-metadata` | | `TMP-build` | none | `common-temp-directory` | | `Scratch.md` | none | `scratch-artifact` | `Thumbs.db`, `tmp-build`, `scratch.md`, and `foo.tmp` are unchanged, so this widens matching without disturbing what already matched. **One correction to the issue's table:** the `.DS_Store` row does not hold on Windows. `macos-finder-metadata` declares `"os": ["macos"]` and `matching_hints()` filters by current OS before matching, so neither spelling hints on a Windows host. That is correct OS filtering, not a defect; the case finding stands on the other five rows. **Gates:** - `bash plugins/disk-hygiene/skills/clean/scripts/hygiene.test.sh` — 228 tests, OK (4 skipped); 5 new tests covering case-insensitive hints, the staging-remnant class, case-insensitive consumer protection globs, the denial text naming every accepted shape, and the classifier rejecting a subcommand outside the shared list. - `bash scripts/check-changelog-parity.sh --check-order` — pass. - `check-skill.sh clean` — PASS, 0 errors; `markdownlint-cli2` — 0 errors; `ruff check` — clean. ## Related - Refs #1806 — **stays open.** This PR takes findings 1 and 5 only. Verified and still open there: finding 2 (byte qualification — its `size_qualifiers` mechanism is landing in PR #1818, and the rest should build on that rather than race it), finding 3 (a `summarize` surface — a new subcommand and a new shape in the guard's grammar, a design call), finding 4 (the `Stop` detector's marker — both remedies change when a safety-observability detector goes quiet, a maintainer call), finding 6 (the probe's environment channel — the probe's allow-list requires exactly two tokens, so the probe, the guard, and `SKILL.md` have to change together), and finding 7 (run-state retention — a durability policy call). - Refs #1804 / PR #1818 and #1805 / PR #1819 — the two CRITICALs from the same audit. All three are open against this manifest, so this PR takes `0.13.0` (`0.11.0` and `0.12.0` are claimed by #1818 and #1819). Merged in issue order the changelog reads contiguously. No linked issue 🤖 Generated with [Claude Code](https://claude.com/claude-code) <https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…1870) ## Summary Completes finding **2** of #1806 (byte accounting cannot express "unknown" / "not reclaimable local bytes"). Findings **1** and **5** already shipped in 0.13.0 via PR #1820. - Truncated directories now record `logical_size: null` with a `not-walked` qualifier instead of looking empty (`0`). - Every entry records `nlink` and cheap `allocated_size` (`st_blocks * 512` on POSIX; null on Windows). - Files with `st_nlink > 1` carry `hardlinked`; sparse files carry `sparse` when the platform exposes the signal. - Snapshot / preview / apply / scan-complete report `reclaimable_local_bytes*` as a figure distinct from the walked logical roll-up; qualified entries are excluded. Closes #1806 ## Related - Refs #1806 — findings **3** (`summarize` surface), **4** (Stop-detector marker amortisation), **6** (probe path provenance vs trusted settings), and **7** (run-state retention / snapshot path containment) need design or coupled-grammar calls; left as follow-ups, not in this PR. - Refs #1804 / PR #1818 — landed the `size_qualifiers` / `file_attributes` mechanism this builds on. - Refs PR #1820 — already closed findings 1 and 5 on this issue. ## Test plan - [x] `python -m unittest -v plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py` — 245 tests, OK (4 skipped) - [x] New coverage: truncated-vs-empty contrast, hardlink exclusion from reclaimable, `nlink`/`allocated_size` shape, protected sync-root `not-walked` - [ ] Do not merge from the worker lane --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Summary
A OneDrive for Business sync root was the single most attractive deletion target in a home audit while reclaiming nothing, and deleting an entry inside it propagates the delete to the organisation's cloud copy. The engine's only structural defense against cloud-sync content was
is_linkish(), and the dominant dehydrated-placeholder class carries no reparse bit when read throughos.lstat— so the whole subtree was walked and every placeholder was recorded as an ordinary file withprotected_reasons: [], carrying its remote byte count as if it were reclaimable local bytes.Verified on the reporting host before any change: 1,101 files walked, 872 dehydrated placeholders totalling 13,770,936,008 bytes, 0 of 872 flagged by
is_linkish(). Full reproduction, including the attribute histogram and theGetFileAttributesWcross-check, is in the verification comment.Fix
1.
hard_protection()contributes acloud-placeholderreason fromFILE_ATTRIBUTE_OFFLINE | FILE_ATTRIBUTE_RECALL_ON_DATA_ACCESS. Placing it there means one predicate coversscan,preview,handoff-verify, andapply's pre-removal recheck at once. It is deliberately independent of the reparse test rather than folded into it — this is precisely the class a reparse test cannot see. Both flags are derived from a singlelstatper ancestor (link_and_cloud_state), so the walk's stat load is unchanged on a scan bounded at 250,000 entries.FILE_ATTRIBUTE_RECALL_ON_OPENis deliberately excluded, correcting the issue's own suggested predicate. Its value0x00040000is the same number asFILE_ATTRIBUTE_EA, and Microsoft documentsRECALL_ON_OPENas appearing "only in directory enumeration classes" while every attribute read here comes fromlstat(File Attribute Constants). Read throughlstatthe bit means "has extended attributes" — see the measured false positives below.2. The baseline gains
protected_name_globs, holdingOneDrive - *, matched casefolded throughfnmatchcaseso the verdict does not depend on the host platform's case rules. This half is not optional: probing the same sync root showed the directories carry no cloud attribute at all (the root reads0x31, all 99 subdirectories read plain0x10), so an attribute predicate protects placeholder files only and a fully hydrated tenant folder would have no protected descendant and stay deletable. The globDropbox (*)and the exact namesDropboxandiCloudDriveship alongside it.The list is deliberately short, because a protected name applies at every depth — a protected directory is never traversed and reports
logical_size: 0, byte-identical to a genuinely empty directory. Over-protection is not free in a reclamation tool; it silently under-reports. Two candidates named in the issue were rejected after checking them:Boxis a common enough directory name in source trees that protecting it at every depth would make ordinary directories untraversable and silently zero-sized, andGoogle Driveis a legacy Backup-and-Sync name — current Google Drive for desktop streams to a virtual drive letter (G:by default on Windows, Drive for desktop settings) rather than a profile folder. Dropbox documents bothDropbox (Personal)andDropbox (<business name>)as folder names, which is why the bare exact name alone was not enough.Consumers could not have closed this themselves:
protected_exact_namesis not overlay-extensible, and an overlay'sadditional_protected_path_globsare matched against a path relative to the scan target, so a standing policy protects such a root only when the target happens to be its parent. Reading the globs from the bundled baseline rather than from the snapshot's policy also means a stale or forged snapshot cannot weaken this protection.3. Every entry records
file_attributesand asize_qualifierslist, so a placeholder's remotelogical_sizecan never be read as reclaimable local bytes. Additive per-entry trace only — no aggregate's definition changes here, since #1806 asks for the reclaimable-bytes figure specifically and the two should not fight.4.
SKILL.mdstep 2's positional-triage rule now reads an entry's ownprotected_reasonsinstead of testing membership ofprotected_exact_names. As written it would have walked straight past a tenant sync root even after this fix.Verification
End-to-end, same tenant tree, engine before vs. after:
protected_reasons: []cloud-placeholder842 rather than 872 because 30 placeholders sit under two
Musicsubtrees an existing name protection already truncates.Depth-1 scan of the user home — the scenario in the report —
OneDrive - <Org>moves fromprotected_reasons: []tobaseline-protected-name, while its siblings are unchanged. Pointing the engine directly at the tenant root now returnsinvalid-or-blocked("protected shell-folder and profile-hive roots are not valid audit targets") instead of scanning it.Negative control, 412,270 entries across three non-cloud trees (a repo checkout,
AppData\Local\Temp,~\.claude):RECALL_ON_OPENincluded (as the issue suggested).nodefilesThose 1,552 entries are fully present on disk; protecting them would block exactly the artifacts this engine exists to reclaim.
Gates:
bash plugins/disk-hygiene/skills/clean/scripts/hygiene.test.sh— 230 tests, OK (4 skipped); 7 new tests added.bash scripts/check-changelog-parity.sh --checkand--check-order— pass.check-skill.sh clean— PASS, 0 errors (the one warning is the pre-existing SKILL.md length soft target).markdownlint-cli2on both changed markdown files — 0 errors.ruff checkon both changed Python files — clean.No cloud-sync placeholder was deleted or hydrated at any point; every probe used
os.walk/lstatonly, and onlyscanwas ever run against the tenant tree.Residual, stated honestly: single Windows 11 host, one tenant. The four non-OneDrive sync roots were confirmed unprotected by name but their file attributes were never sampled, so they are protected on name alone and their placeholder behaviour is unverified. macOS untested.
Related
size_qualifiersset (hardlinked,sparse,not-walked); this PR establishes thesize_qualifiersfield and adds only thecloud-placeholdermember, deliberately leaving aggregate semantics unchanged so the two changes do not conflict.Merge this PR first. Three PRs from the same audit are open against the
disk-hygienemanifest and each claims the next version, so they must merge in issue order or the changelog and manifest disagree:0.11.00.12.00.13.0Merged in that order the changelog reads contiguously and each conflict is a trivial keep-both-in-order in
CHANGELOG.md. Merged out of order, a later version lands above a gap and the earlier PRs conflict in a way that looks like an authoring error rather than an ordering one.Fixes #1804
🤖 Generated with Claude Code
https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C