Skip to content

fix(disk-hygiene): require immediate re-enumeration before container-wide manual deletions#1118

Merged
kyle-sexton merged 5 commits into
mainfrom
fix/1108-dh-manual-handoff-toctou
Jul 23, 2026
Merged

fix(disk-hygiene): require immediate re-enumeration before container-wide manual deletions#1118
kyle-sexton merged 5 commits into
mainfrom
fix/1108-dh-manual-handoff-toctou

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Closes #1108 (handoff-inbox item 20260723-021058-disk-hygiene-0-6-4-consumer-audit, finding F2 — cheap process fix; the ambitious handoff-verify subcommand is tracked separately in #1109).

The clean skill's unsupported-platform manual-handoff lane bound container-wide deletion approvals ("empty the Recycle Bin — 1,183 items") to a prose list that goes stale between approval and execution. The 0.6.4 consumer audit hit a live near-miss: four freshly-deleted repo folders landed in the bin seconds after the approved Clear-RecycleBin ran — 30 seconds earlier and they'd have been permanently destroyed under an approval predating their existence.

Changes (SKILL.md, declarative tier):

  • Container-wide operations are forbidden unless the container is re-enumerated immediately before execution and matches the approved count and set exactly; any drift aborts and re-asks — the engine lane's changed-since-scan discipline applied to the manual lane.
  • Recycle Bin / Trash reversibility documented as conditional: bin size caps, policy-disabled bins, and non-NTFS/network volumes silently make removal permanent.

disk-hygiene 0.6.4 → 0.6.5 (+ CHANGELOG entry).

Test plan

  • Finding reproduced against 0.6.4 source before editing (manual-handoff steps had per-path revalidation only; no container-wide rule, unconditional reversibility phrasing)
  • Docs-only change — no engine/script behavior touched; CI gates (hygiene, changelog-parity, skill-quality, markdown lint) verify

Related

🤖 Generated with Claude Code

…wide manual deletions (#1108)

An approval for a container-wide operation (Clear-RecycleBin, emptying the
Trash) was bound to a prose item list that could go stale between approval
and execution: items landing in the container after approval would be
destroyed under an approval that predated their existence — a live near-miss
in the 0.6.4 consumer audit (F2). The manual-handoff lane now forbids
container-wide operations unless the container is re-enumerated immediately
before execution and matches the approved count and set exactly, aborting on
any drift. Also documents that Recycle Bin / Trash reversibility is
conditional (bin size caps, policy-disabled bins, non-NTFS/network volumes
silently make removal permanent). disk-hygiene 0.6.4 -> 0.6.5.

Co-Authored-By: Claude Fable 5 (200k context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2c9af99ff

ℹ️ 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".

Comment thread plugins/disk-hygiene/skills/clean/SKILL.md Outdated
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Orchestrator note (work lane, T3): cross-checked this PR's diff against #1108's three acceptance criteria. Criteria 1 (re-enumeration + abort-on-drift) and 2 (conditional-reversibility caveat) are met. Criterion 3 ("Changed-since-scan threat section cross-references the manual lane") is only satisfied in one direction — the new manual-lane text points to the changed-since-scan discussion, but the existing changed-since-scan/Gotchas bullet (engine-side) doesn't point back at the manual lane. Cheap one-line addition if you want the criterion literally closed; not blocking given the substantive fix (the actual TOCTOU protection) is complete. Flagging for whoever does the post-green review pass on this PR.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

…, not absolute

The specific silent-permanence causes (bin caps, disabled bin, non-NTFS or
network volume) are standard Windows shell behavior but were stated as flat
fact on producer report alone; "can silently make permanent" is the honest
claim strength for what this session could confirm from Microsoft's
SHFileOperation docs (FOF_ALLOWUNDO recycle-vs-permanent split).

Co-Authored-By: Claude Fable 5 (200k context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Re-running the job, or pushing a new commit, will retry the review.

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Re-running the job, or pushing a new commit, will retry the review.

…cross-reference manual lane from engine gotcha

Review findings on PR #1118: (1) Codex P1 — step 3's re-enumeration rule
blesses a container-wide op the PowerShell guard could not intercept
(_POWERSHELL_MUTATION_WORDS lacked clear-recyclebin), so the stated final
exact-list confirmation and audit-only denial never fired for it. Added to
the mutation words with ask + audit-only-deny grid coverage; the broader F4
spellings stay in #1111. (2) Orchestrator AC3 note — the engine-side
changed-since-scan gotcha now points back at the manual lane's
re-enumeration rule, closing the cross-reference in both directions.

Co-Authored-By: Claude Fable 5 (200k context) <noreply@anthropic.com>
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Re the orchestrator T3 note (AC3 one-directional): back-reference added in 5019f46 — the engine-side changed-since-scan gotcha now points at the manual lane's re-enumeration rule, so criterion 3 is closed in both directions. Same commit fixes the Codex P1 (Clear-RecycleBin guard word + tests) and retries the infra-failed automated reviews.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

🔒 babysit-prs lane claiming this PR for on-branch fix work this cycle (amendment-round: 16, safe tier). Will fix clear branch-owned findings and push; will not resolve threads or merge (safe tier).

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5019f4653f

ℹ️ 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".

Comment thread plugins/disk-hygiene/skills/clean/SKILL.md Outdated
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 970cbf9206

ℹ️ 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".

Comment thread plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py
…atch module-qualified deletion cmdlets

Second review round on PR #1118: (1) re-enumeration cannot close the
approval-to-execution window against a live container (items arriving while
the human answers the prompt die under an approval that never saw them) —
the manual lane now forbids container-wide deletion commands and satisfies
"empty the container" by per-item deletion under the lane's per-path
revalidation, so unenumerated arrivals survive. (2) Module-qualified
invocations (Microsoft.PowerShell.Management\Clear-RecycleBin) evaded the
mutation words' lookbehind; a companion pattern now matches module-qualified
Remove-Item / Clear-Content / Clear-RecycleBin, with ask + audit-only-deny
grid coverage. Suite 112 OK.

Co-Authored-By: Claude Fable 5 (200k context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

@kyle-sexton
kyle-sexton merged commit af153a6 into main Jul 23, 2026
25 checks passed
@kyle-sexton
kyle-sexton deleted the fix/1108-dh-manual-handoff-toctou branch July 23, 2026 09:13
kyle-sexton added a commit that referenced this pull request Jul 23, 2026
…nd robocopy purge flags (#1128)

## Summary

Closes #1111 (handoff-inbox item
`20260723-021058-disk-hygiene-0-6-4-consumer-audit`, finding F4
remainder — `Clear-RecycleBin` and module-qualified cmdlets already
landed via #1118's review rounds).

- Instance-method `.Delete()` now raises the guard's final ask prompt
(the .NET pattern required `::` before `delete`, so `$item.Delete()`
executed unflagged — live-observed in the producer audit); denied in
audit-only mode.
- `robocopy` with `/MIR`, `/PURGE`, `/MOV`, `/MOVE` (mass deletion via
mirroring) gated the same way; plain `robocopy /E` copies stay
untouched.
- Truncation family (`Set-Content`, `Out-File`, `New-Item -Force`)
**declined with recorded reason** (CHANGELOG): ordinary file-writing
spellings — ask-tier friction on every write outweighs the raised bar;
design stays raised-bar-not-fail-closed per the guard's documented
philosophy.

disk-hygiene 0.7.0 → 0.7.1.

## Test plan

- [x] Ask-grid + audit-only-deny-grid entries for `.Delete()` (two
spellings), `/MIR`, `/E /PURGE`, `/MOV`
- [x] Negative cases: `robocopy /E` and `Select-Object IsDeleted` defer
- [x] Suite 128 OK

## Related

- #1118 (F4 first tranche via review rounds), #1112 (substring-deny
narrowing, separate)
- Handoff-inbox item:
`20260723-021058-disk-hygiene-0-6-4-consumer-audit` (F4)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 (200k context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 23, 2026
…ation for the manual lane (F2) (#1145)

## Summary

dh **0.8.0** — the disk-hygiene consumer-audit item's largest remaining
finding (F2 ambitious path). Adds a read-only `hygiene.py
handoff-verify` subcommand that brings snapshot binding to Windows/macOS
— the platforms where engine `apply` is unsupported — without adding an
engine deletion lane. Captures most of the declined
Windows-execution-lane (F12) value at a fraction of the risk; #1116's
affirmation records this as the intended alternative.

**Engine** (`hygiene.py`):
- `handoff-verify --snapshot <s> --paths <p> [--data-root <d>]` takes
the snapshot plus the human-approved exact path list (`{"version": 1,
"paths": [...]}` — validated with the same containment rules as plan
candidates: relative, non-root, no traversal, present in snapshot,
non-overlapping) and reruns
identity/reparse/protection/descendant/VCS/handle checks per path
against live state.
- Per-path machine-readable verdicts: `clear` / `drifted` (identity or
captured descendant set changed) / `gone` (no longer present) /
`contested` (protection, VCS, live handle, elevation, or unverifiable
state). **Fail-closed:** every unverifiable condition lands in
`contested`, never `clear`. Exit 0 all-clear, 3 otherwise. **No deletion
capability exists in the subcommand.**
- Platform execution blockers deliberately do not apply — the subcommand
exists exactly where `execution-platform-unsupported` blocks the engine
lane.
- Target-root validation extracted into `resolve_snapshot_target`,
shared with `preview` (behavior there unchanged, full stat identity —
the approval token binds one immutable state). handoff-verify passes
`strict_root_stat=False`: the root is checked with the engine's
established directory object-identity (device/inode/type — the same
check `apply` uses for directories) because the manual lane deletes one
root-level item at a time with a re-verify between items, and the root's
own mtime churn must not invalidate the next verify. A replaced root
still refuses.

**Guard** (`destructive_guard.py`): admits the exact `handoff-verify`
token shape as a read-only invocation — including in audit-only mode
(the kill switch continues to block every deletion lane; verification is
reporting). Malformed shapes (wrong flag order, missing/undeclared
flags, trailing tokens) stay denied. No change to the deny-by-default
posture.

**Docs**: clean SKILL.md's manual-handoff lane now writes
`handoff-paths.json`, runs handoff-verify immediately before deletion,
and acts only on verdict-`clear` paths (per-item, under the PowerShell
guard's final prompt); owner-process absence stays a model-side check
since snapshot entries carry no owner claim. `reference/safety-model.md`
gains a handoff-verify section with the verdict vocabulary table,
fail-closed mapping, and emission-time-only validity of `clear`.

**Contract decisions recorded** (resolved loudly, not silently): the
four verdict classes come from #1109's acceptance criteria; the mapping
choices made here are (a) unverifiable → `contested`, (b) drifted takes
precedence over contested when both apply, (c) root metadata churn
tolerated via object identity while root replacement refuses. All three
documented in safety-model.md and the CHANGELOG.

## Test plan

- [x] 12 new tests: 9 engine (clear read-only + platform-blocker
exclusion, gone, changed-since-approval content drift, new-descendant
drift, live-handle contested, VCS contested, unverified-handle
fail-closed, paths-file validation matrix, `main()` exit codes 0/3) + 3
guard (exact shape allowed, allowed in audit-only mode, malformed shapes
denied)
- [x] Suite passes locally on Windows: 163 tests, OK (4 platform skips)
- [x] `hygiene.test.sh` wrapper → OK; `check-changelog-parity.sh
--check-bump` → pass; `check-changed-skills.sh` → CHECK-SKILL clean:
PASS
- [ ] CI ubuntu lane green

## Related

Closes #1109. Source: handoff-inbox item
`20260723-021058-disk-hygiene-0-6-4-consumer-audit`, finding F2
(ambitious path). Relates to #1116 (F12 decline affirmation — this PR is
the recorded alternative) and #1108/PR #1118 (manual-lane TOCTOU
discipline this extends).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(disk-hygiene): manual-handoff lane — require immediate re-enumeration before container-wide deletions (F2)

1 participant