Skip to content

feat(disk-hygiene): add model-invocable read-only audit skill - #5590

Merged
kyle-sexton merged 20 commits into
mainfrom
feat/5516-disk-hygiene-audit-skill
Sep 30, 2026
Merged

kyle-sexton merged 20 commits into
mainfrom
feat/5516-disk-hygiene-audit-skill

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Refs: #5516

Hold: do not merge

The do-not-merge label is applied. Owner condition 4 on #5516 says to probe the guard ask on apply --execute under bypassPermissions before the skill ships. Lane c was not run: the auto-mode classifier denied the launch script and the denial was not worked around. Auto mode, this machine's default, was not probed either. Release the hold only after the owner runs lane c (claude -p --permission-mode bypassPermissions --tools Bash, same scratch target and prompt as lanes a and b) or accepts the gap in a comment here or on #5516. If lane c fails open, condition 4 says fall back to A2 for that lane.

Second owner question: the argument-free kill-switch probe is not matched by the engine-gate filter, so a delegated audit in default mode stops at step 1 without a user allow rule for the probe. Accept that, or add the allow rule.

Merge order: #5571 first, then this PR (see Related).

Summary

Adds /disk-hygiene:audit, a model-invocable, read-only skill that runs the kill-switch probe and one engine scan and reports the snapshot. It runs no preview or apply; removal stays a separate /disk-hygiene:clean run a person invokes. clean is unchanged. Ships together with #5571 (#5520).

Fix

  • skills/audit/SKILL.md and evals/evals.json (three delegated-audit cases and one negative case where "clean up my disk / delete these" must route to /disk-hygiene:clean).
  • The scan template takes a <project-dir> placeholder (a literal absolute path, optional) in place of ${CLAUDE_PROJECT_DIR}, and the skill tells the parent to replace every ${...} token and <placeholder> in the fan-out worker brief with the probe's hook_python and data_root before spawning, since a worker cannot expand tokens.
  • The inline --plugin-dir data_root: null gotcha carries a claim, basis, as-of and recheck record. The null-data_root eval no longer expects a guard denial to relay: no engine call is submitted in that case.
  • audit is registered in scripts/skill-leaf-name-registry.txt with disk-hygiene as its 17th owner and a rationale paragraph.
  • README lists the skill; plugin 0.30.0 to 0.35.0 (main took 0.31.0 to 0.34.1 for other disk-hygiene changes while this PR was open) with a CHANGELOG entry linked to disk-hygiene: model-invocable read-only audit skill for orchestrators and subagents #5516; cheat sheet regenerated.
  • Audit-only (toggle off) still runs the read-only scan and drops the removal handoff, instead of stopping. The owner decision is silent on this; the owner may overrule.

Verification

Probe results (claude 2.1.285, worktree plugin loaded with --plugin-dir, permission-mode default, fresh scratch target):

  • Lane a, headless default: apply --execute denied by the guard (exact-engine-apply, ask); staging.tmp survived.
  • Lane b, --bg default: parked at a permission prompt on the apply; staging.tmp survived.
  • Lane c, bypassPermissions: not probed. Auto mode (this machine's default) not probed.
  • The argument-free probe is not matched by the engine-gate filter (Bash(*hygiene.py*)), so a default-mode model-invoked session needs a user approval or allow rule to run it. Under --plugin-dir the probe reports data_root null.

Checks run locally on a9f61a8 (after merging origin/main and renumbering 0.34.0 to 0.35.0), all exit 0: check-skill-leaf-names.sh --check and its .test.sh; check-adr-numbers.sh --check; check-changelog-parity.sh --check, --check-order, --check-bump origin/main; validate-plugins.sh; check-changed-skills.sh origin/main; check-skill-count-claims.sh; check-spoke-plugin-root.sh --check; check-docs-naming.sh --check; check-orphaned-fixtures.sh --check; generate-cheatsheet.mjs --check; check-evals-quality.sh and json.tool on the audit evals; hygiene.test.sh (592 tests, 1 skipped) and kill_switch_probe.test.sh. The earlier "all pass" was wrong: on 4c4faf7 CI lint-2 failed skill-leaf-names (fixed by the registry entry) and adr-numbers (a duplicate 0042 on main, fixed by main's renumber, which this branch now includes).

The A2 fallback and the final call on closing #5516 stay with the owner, so this PR uses Refs, not Closes.

Related

#5516; #5520 / #5571 (ships together).

Merge order: #5571 lands first. This PR links its worker brief, which still shows ${CLAUDE_PLUGIN_DATA} and ${CLAUDE_PROJECT_DIR} tokens until #5571 lands. #5571 is stale: its base predates main's 0.29.2 through 0.31.0 changelog entries, so it needs a rebase and a renumber above main's current version before it can merge. disk-hygiene versions on main keep moving, so before this PR merges, merge main and re-run scripts/check-changelog-parity.sh --check-bump origin/main, renumbering above main's then-current version. The repo squash-merges only, so the branch's bump to 0.30.0 commit subject does not reach main.

🤖 Generated with Claude Code

kyle-sexton and others added 5 commits September 30, 2026 01:42
/disk-hygiene:audit runs the engine's scan subcommand only, after the
argument-free kill-switch probe, and reports the snapshot. Removal stays
with /disk-hygiene:clean, which a person invokes. Safety rests on the
plugin-level engine-gate; the skill carries no hooks of its own.

Refs #5516

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The probe reports hook_python as the interpreter it ran under and the
engine-gate does not fire on the probe path, so a bare-python run names
nothing. Say what the probe reports, let the scan's own denial supply
the guard's interpreter, drop the null-data-root denial that never
exists, and drop the unverified claim about the gate reaching subagents.

Refs #5516

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… 0.30.0

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Rebump disk-hygiene to 0.31.0 above main's 0.30.0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

kyle-sexton and others added 3 commits September 30, 2026 09:28
…kill with the delegated-worker contract

Refs #5516

Register disk-hygiene as the 17th owner of the `audit` leaf name and
extend the rationale, so check-skill-leaf-names.sh --check passes.

In the audit skill, replace the literal ${CLAUDE_PROJECT_DIR} in the scan
template with a <project-dir> placeholder filled with a literal absolute
path, tell the parent to fill every token and placeholder in the worker
brief from the probe values before spawning, and give the inline
--plugin-dir data_root null gotcha its claim/basis/as-of/recheck record.
Correct the null-data-root eval, which expected a guard denial that never
occurs because no engine call is submitted, and link the 0.31.0 changelog
entry to #5516.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Main claimed disk-hygiene 0.31.0 for the investigated-entry catalog, so
the audit skill renumbers to 0.32.0 above it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Sep 30, 2026
kyle-sexton and others added 8 commits September 30, 2026 11:07
… tokens that break

Refs #5516

Name ${CLAUDE_PROJECT_DIR} and ${CLAUDE_PLUGIN_DATA} in the scan-template
warning instead of every ${...} token, since the skill's own commands use
${CLAUDE_PLUGIN_ROOT}, and word the leaf-name rationale as joining on its
own grounds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Main claimed disk-hygiene 0.31.1 for the entry-cap error fix, so the
audit skill stays at 0.32.0 above it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Main claimed disk-hygiene 0.32.0 for virtual-disk image protection, so
the audit skill renumbers to 0.33.0 above it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Main added disk-hygiene 0.32.1; the audit skill stays at 0.33.0 above it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Main claimed disk-hygiene 0.33.0 for the partial-totals labeling, so the
audit skill renumbers to 0.34.0 above it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Main added disk-hygiene 0.33.1; the audit skill stays at 0.34.0 above it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-audit-skill

Renumber disk-hygiene to 0.35.0: main now carries 0.34.0 (#5528) and 0.34.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ituted tokens

Refs #5516

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 30, 2026 16:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T17:05:55.735196Z 75bff7a Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 4m 49s —— View job


Security review — /review:security-review

  • Gather context (PR diff, changed files)
  • Apply skip gate
  • Review changed files for security-relevant surface
  • Check instruction-surface deletions against protected-class register
  • Investigate and adversarially verify candidate finding
  • Post findings

Scope. Reviewed the 7 files changed in this PR against origin/main (f66da34
SKILL.md, evals.json, plus doc/registry/version-bump files). No preview/apply
path is added; the new skill runs the engine's read-only scan subcommand only.

Instruction-surface check. This diff adds a new skill file rather than deleting,
narrowing, or softening an existing standing instruction, so the
instruction exception register
lens (guardrail deletions) doesn't apply here — nothing is removed.

Candidate investigated, not confirmed. skills/audit/SKILL.md sets
disable-model-invocation: false
(vs. true on the sibling clean skill), with trigger phrases like "delegate a
disk-hygiene audit" in the description. That raised the question of whether this makes
a filesystem-scan primitive reachable via indirect prompt injection with no human
gate. On verification this doesn't hold up as an issue introduced by this PR: the
plugin-level engine-gate hook (plugins/disk-hygiene/hooks/hooks.json,
unchanged here) already auto-allows any exact hygiene.py scan invocation
regardless of target path or which skill (if any) constructed the command — it's a
session-wide, always-on hook, not something this skill's flag newly unlocks. The
disable-model-invocation flag only gates whether the model auto-loads this
skill's instructions; it doesn't gate the underlying Bash call the guard already
allows. That's pre-existing behavior of destructive_guard.py, out of this diff's
scope, so not reported as a finding here (noting it for the record in case the
owner wants to track it separately — the engine-gate's scan classification
doesn't distinguish an in-project target from something like ~/.ssh).

Other changed files (evals.json, CHANGELOG.md, README.md, plugin.json,
docs/skill-cheat-sheet.md, scripts/skill-leaf-name-registry.txt) are
documentation/registry/version-bump content with no security-relevant surface.

No security issues found in this diff.

Note: the PR's own do-not-merge hold (owner condition 4 on #5516, lane-c
permission probing) is a separate, already-disclosed concern about permission-prompt
ergonomics, not a security defect — outside this lane's scope to adjudicate.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 6m 9s —— View job


Review complete

Ran /review:code-review against origin/main...HEAD (7 files, +204/-2, head 75bff7a). This PR only touches skill/doc content (skills/audit/SKILL.md, evals/evals.json, CHANGELOG.md, README.md, plugin.json, the skill-leaf registry, and the cheat sheet) — no executable code changed.

What I checked

  • Cross-referenced every claim in the new SKILL.md against the actual (untouched) engine code it describes:
    • hooks/hooks.json's engine-gate if filter (Bash(*hygiene.py*)) and destructive_guard.py's _engine_gate_relevant/classify_exact_engine_command — confirmed the "probe isn't adjudicated by the engine-gate, scan is always allowed, only apply is gated" claims are accurate.
    • Every JSON field name the reporting step (children_rollup, reclaimable_local_bytes, size_qualifiers, truncated_paths, hinted_entries, os_autoclean, unwalked_reasons, etc.) against hygiene.py — all exist in the engine's real output vocabulary.
    • The four-part upstream-drift verification record for the --plugin-dir data_root: null claim — present and correctly formatted per .claude/rules/skill-bodies-state-current-rules.md.
    • The ## Next section placement/format — correct per the same rule.
    • All relative doc links (scan-flags.md, safety-model.md, fan-out-worker-brief.md, hygiene.py, MIN_PYTHON) — resolve to real files/symbols.
  • Compared evals/evals.json structure/schema against the sibling clean skill's evals — identical shape, no mismatch.
  • Checked scripts/skill-leaf-name-registry.txt — disk-hygiene inserted in correct alphabetical position in the audit owner list, rationale paragraph is consistent with the skill's actual contract (scan-only, no removal lane).
  • CHANGELOG.md / plugin.json version bump (0.34.1 → 0.35.0) and docs/skill-cheat-sheet.md entry — consistent with each other and correctly alphabetized.

Findings

None that clear this lane's high-signal bar (nothing a linter/static check would catch, and nothing new beyond what the PR body already discloses). The two open gaps called out in the "Hold" section — lane c (bypassPermissions) and auto-mode not probed for apply --execute, and the kill-switch probe not being matched by the engine-gate filter — are accurately described by the skill text itself and are correctly left as owner decisions rather than silently resolved in code; I verified both claims against the guard source and they check out.

No inline comments posted; this PR's do-not-merge label and hold rationale remain valid and are not something this review overrides.

@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: 75bff7ac10

ℹ️ 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/audit/SKILL.md Outdated
Comment thread plugins/disk-hygiene/skills/audit/SKILL.md
Comment thread plugins/disk-hygiene/skills/audit/SKILL.md Outdated
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Lane c result (owner-run, 2026-09-30): the destructive guard's ask held under bypassPermissions.

  • Command: claude -p --model sonnet --effort low --permission-mode bypassPermissions --tools Bash --strict-mcp-config --max-turns 14 --max-budget-usd 3 --plugin-dir <worktree>/plugins/disk-hygiene --settings '{"enabledPlugins":{"disk-hygiene@melodic-software":false}}', same fresh scratch target and prompt template as lanes a and b (setup-lane.sh c), Claude Code 2.1.285.
  • Init event: permissionMode: bypassPermissions, 1 tool, 0 MCP servers.
  • Steps run: kill-switch probe (effective: true), scan (scan-complete, 5 entries), preview (ready-for-explicit-approval, staging.tmp), then apply --execute --confirm-tier medium.
  • The apply --execute call is listed in the result event's permission_denials (1 denial). The tool result was the guard's prompt text: "Confirm this final mutation prompt only if it matches the tier and paths you just approved." The session stopped there (subtype: success, 5 turns, $0.07).
  • staging.tmp survived: target listing after the run was emptydir, staging.tmp, sub.

A first attempt without --strict-mcp-config hit the $3 budget cap after preview and never reached apply (26 MCP servers loaded about 380k tokens per turn); it proved nothing and is superseded by this run.

Not probed: an interactive session in bypassPermissions (where a person could answer the prompt) and auto mode. Condition 4's fail-open fallback (A2) is not triggered by any lane run so far: headless default denied, --bg default parked at the prompt, headless bypassPermissions denied.

Transcript: .work/probe-5516/out-c.jsonl in the #5516 drain worktree.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Owner decision (2026-09-30): release the hold. Condition 4 on #5516 is met by the three probe lanes (headless default denied, --bg default parked, headless bypassPermissions denied; see the lane c comment above). The owner accepts the kill-switch probe prompt in default mode with no user allow rule. Hardening for interactive bypassPermissions and the Windows PowerShell tool is tracked in a follow-up issue.

Resolve the disk-hygiene version and changelog (0.35.0 above 0.34.4) and
address the Codex review on the audit skill: add the verification record to
the permission gotcha, stop probing under bare python, and state that a scan
does not assess locked, needs-elevation, or unverified entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 59s —— View job


Code review complete

  • Gather context (PR diff, changed files)
  • Invoke /review:code-review and follow its instructions
  • Post inline findings
  • Summary comment

Ran /review:code-review against origin/main...HEAD at 45e88b5 (7 files, +213/-2). This lane scopes security findings out (a separate /review:security-review lane already ran on this PR and found nothing); this pass is correctness/maintainability only.

Findings

1 finding, posted inline on plugins/disk-hygiene/skills/audit/SKILL.md:33-38:

The bootstrap fallback ("submit the probe once with bare python: the guard denies that read-only call and names its interpreter") describes a mechanism that cannot occur for this skill. Verified against hooks/hooks.json: the engine-gate's PreToolUse filter is Bash(*hygiene.py*) / PowerShell(*hygiene.py*), which never matches kill_switch_probe.py, and the probe's own docstring says its exit code is always 0 (report-only). The skill's own Gotchas section (line 111) correctly says the probe "is not one of the calls the engine-gate adjudicates" — directly contradicting the earlier claim of a guard denial naming the interpreter. That fallback text is real in the sibling clean/SKILL.md only because clean installs its own skill-level PreToolUse belt hook matching every Bash|PowerShell call (see clean/SKILL.md:5-21); audit/SKILL.md explicitly states "This skill carries no hooks of its own" (line 19), so the fallback appears to have been copied from clean without the mechanism that makes it true there. A model following it literally would wait on a denial that never arrives, or silently accept an unverified interpreter.

Not re-reported

The three Codex findings from the earlier review round (verification record on the permission gotcha, resolving the guard's interpreter before probing, and not claiming unassessed live-state categories) all appear to already be addressed in the current SKILL.md — the Gotchas section now carries full claim/basis/as-of/recheck records, and step 3 now explicitly says a scan "does not assess live handles or elevation" rather than listing empty categories. Not re-flagged.

No other high-signal correctness/maintainability issues found in the remaining changed files (evals/evals.json, CHANGELOG.md, README.md, plugin.json, docs/skill-cheat-sheet.md, scripts/skill-leaf-name-registry.txt) — version bump, changelog, cheat-sheet entry, and registry ordering are all internally consistent.

Note: this PR's do-not-merge hold has since been released by the owner per the thread above; this review doesn't speak to that decision.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 50s —— View job


Security review — /review:security-review

  • Gather context (PR diff, changed files)
  • Apply skip gate
  • Review changed files for security-relevant surface
  • Check instruction-surface deletions against protected-class register
  • Investigate and adversarially verify candidate findings
  • Post findings

Scope. Reviewed all 7 files changed in this PR against origin/main at head
45e88b5
(+213/-2): the new skills/audit/SKILL.md and evals/evals.json, plus
CHANGELOG.md, README.md, plugin.json, docs/skill-cheat-sheet.md, and
scripts/skill-leaf-name-registry.txt. This is a docs/registry-only change —
no executable code is added or modified, and no preview/apply path exists;
the new skill documents the engine's read-only scan subcommand only.

Instruction-surface check. Nothing in this diff deletes, narrows, or softens a
standing instruction — it's purely additive (a new skill file, a new eval fixture,
doc/version-bump entries, and a registry-entry comment). The
instruction exception register
lens doesn't apply.

Verification against the unmodified engine/guard. This head only changes
prose relative to the previously-reviewed commit (75bff7a) — it tightens the
three gotchas Codex flagged (adds the four-part verification record to the
permission gotcha, fixes the "resolve the guard's interpreter before probing"
sequencing, and stops claiming locked/needs-elevation/unverified as
observed-empty categories a scan never produces). I re-checked the
security-relevant claims in
SKILL.md
against the actual (untouched) files they describe:

  • hooks/hooks.json's engine-gate if: "Bash(*hygiene.py*)" / "PowerShell(*hygiene.py*)" matches
    any hygiene.py invocation regardless of which skill built the command — confirmed unchanged,
    so the claim "safety rests on the plugin-level engine-gate" still holds for this new skill.
  • hygiene.py's --sizes-only / --confirmed-large-scan gate (lines ~5005, ~5079) still enforces
    the large-scan confirmation at the engine level regardless of skill prose, so the audit skill
    cannot bypass that control even though its own text doesn't restate the nuance.
  • The UserPromptExpansion matcher (disk-hygiene:clean$) that resolves the guard's interpreter
    fires only for /disk-hygiene:clean, not /disk-hygiene:audit — consistent with why this skill
    needs its own bootstrap probe (step 1) rather than relying on that hook.

Candidate investigated, not confirmed (carried over from the prior review of this
PR).
disable-model-invocation: false plus indirect-invocation trigger phrases in
the description raised the question of whether this newly exposes a filesystem-scan
primitive to indirect prompt injection with no human gate. As before, this doesn't hold
up as an issue introduced by this PR: the engine-gate hook already auto-allows any
exact hygiene.py scan call session-wide, independent of which skill (if any)
constructed it, so the flag doesn't newly unlock anything — it only controls whether
the model auto-loads this skill's instructions. Pre-existing destructive_guard.py
behavior, out of this diff's scope.

Other changed files (evals.json, CHANGELOG.md, README.md, plugin.json,
docs/skill-cheat-sheet.md, scripts/skill-leaf-name-registry.txt) remain
documentation/registry/version-bump content with no security-relevant surface.

No security issues found in this diff.

Note: the PR's do-not-merge hold has since been released by the owner (per the
thread above); that's a permission-prompt-ergonomics decision, not a security defect,
and outside this lane's scope to adjudicate either way.

Comment thread plugins/disk-hygiene/skills/audit/SKILL.md Outdated
kyle-sexton added a commit that referenced this pull request Sep 30, 2026
…5613)

Refs #5609

## Summary

Adds the four-part verification record (claim, basis, as-of, recheck) to
`safety-model.md` for how the PreToolUse `ask` on `hygiene.py apply
--execute` behaves across permission modes. It states the three modes
probed, the modes not probed, and what the official docs and upstream
issues say.

## Fix

Only AC3 of #5609 is met. AC1 (interactive `bypassPermissions` and a
Windows host probe) needs operator-only sessions and is not faked here;
AC2 (a gate decision) depends on AC1 and stays with the owner. The issue
stays open. The plugin is bumped to 0.35.1 with a CHANGELOG entry.

## Verification

- `scripts/check-changelog-parity.sh --check`, `--check-order`,
`--check-bump origin/main`: pass.
- `scripts/validate-plugins.sh`: all manifests and the catalog
validated.
- Docs-only change; no plugin tests apply.

## Related

- Issue #5609
- Probe evidence: PR #5590

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton and others added 2 commits September 30, 2026 14:27
…ootstrap

Merge origin/main, take the plugin to 0.36.0 above main's 0.35.0, and restore
the audit skill's scan-denial path: the audit carries no skill-level belt, so
the probe is not denied. An older hook_python from a bare-python probe no
longer stops the audit; the scan's admission settles the guard's interpreter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep the disk-hygiene version at 0.36.0 above main's 0.35.1 and put this
PR's changelog entry on top.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 38s —— View job


Security review — /review:security-review

  • Gather context (PR diff, changed files)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Check instruction-surface deletions against protected-class register
  • Post findings

Scope. Reviewed all 7 files changed on this PR (origin/main...HEAD, head 15384a6, +214/-2): plugins/disk-hygiene/skills/audit/SKILL.md (new), plugins/disk-hygiene/skills/audit/evals/evals.json (new), CHANGELOG.md, README.md, plugin.json, docs/skill-cheat-sheet.md, scripts/skill-leaf-name-registry.txt. This head is a merge of origin/main plus the 1c4ef37 bootstrap-text fix on top of what the two prior security-review rounds on this PR already covered; no code changed, only prose in SKILL.md moved back to its original (correct) flow.

Instruction-surface check. This diff adds a new skill file; it does not delete, narrow, or soften any existing standing instruction, so the instruction exception register lens (guardrail deletions) doesn't apply.

Re-verified against the unchanged engine/guard (none of this is new since the last round, re-confirmed directly against source at this head rather than taken on faith):

  • hooks/hooks.json — the engine-gate PreToolUse filter is Bash(*hygiene.py*) / PowerShell(*hygiene.py*), unconditional and skill-agnostic. It still auto-admits any exact hygiene.py scan invocation regardless of which skill (if any) built the command, so audit/SKILL.md's disable-model-invocation: false doesn't unlock a capability that a raw Bash call couldn't already reach — it only affects whether this skill's instructions auto-load. Confirmed this is the correct framing (not a bypass) rather than just repeating the prior rounds' conclusion.
  • destructive_guard.classify_exact_engine_command requires the command to tokenize as literal shell words via _literal_shell_words (L173-L185), which rejects the command outright if it contains any shell-expansion/operator character. So a --target value containing injected shell metacharacters (;, `, $(), |, etc.) can't smuggle a second command through the scan call this skill's step 2 constructs — the whole command fails classification and isn't treated as an admitted engine call. This addresses the natural next question raised by a model-invocable skill that interpolates an externally-suggested path into a Bash command; the protection is pre-existing and untouched by this diff.
  • The disable-model-invocation: false + broad trigger-phrase design (reachable via "delegate a disk-hygiene audit", "orchestrator scan...", etc.) was investigated by both prior security-review rounds on this PR as a possible indirect-prompt-injection path to an unattended filesystem scan. I re-traced that reasoning rather than deferring to it and it holds: because the engine-gate already allows scan unconditionally and session-wide, this flag changes reachability of the instructions, not the set of Bash calls the guard admits. No new finding here.

Other changed files (evals.json, CHANGELOG.md, README.md, plugin.json, docs/skill-cheat-sheet.md, scripts/skill-leaf-name-registry.txt) are documentation, version-bump, and registry content with no security-relevant surface — confirmed by reading the actual diffs, not just the file list.

No security issues found in this diff.

Note: this PR's do-not-merge hold was released by the owner earlier in the thread (lane c probed, condition 4 satisfied); that's a permission-prompt-ergonomics decision already worked through in this PR's comments, not a security defect, and outside this lane's scope to relitigate.
(branch)

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 37s —— View job


Code review complete

  • Gather context (PR diff, changed files)
  • Invoke /review:code-review and follow its instructions
  • Post inline findings
  • Summary comment

Ran /review:code-review against origin/main...HEAD at head 15384a6 (7 files, +214/-2). This lane scopes security out (a separate /review:security-review lane already ran twice on this PR and found nothing); this pass is correctness/maintainability only.

What changed since the last review round

The last code-review pass on this lane reviewed 45e88b5 and flagged one correctness issue: the bootstrap fallback claimed the guard denies a bare-python probe call and names its interpreter, which can't happen since audit carries no skill-level belt hook and the engine-gate only matches commands naming hygiene.py (never kill_switch_probe.py). Since then:

  • 1c4ef37 rewrote SKILL.md:33-46 to drop that claim and restore the original flow: the probe runs under bare python to learn hook_python/data_root opportunistically, and it's the subsequent scan call, not the probe, that the engine-gate actually adjudicates — a wrong interpreter there hits classify_exact_engine_command's _is_current_python check, falls through to the not-exact-engine-command deny path, and that denial's _bash_denial_guidance does name the correct interpreter (verified against destructive_guard.py:1307-1308, 2780-2821, 2113-2114). I independently traced this path through the guard source rather than taking the commit message's claim at face value, and it holds up.
  • Two merge commits (45e88b5→1c4ef37, 1c4ef37→15384a6) pulled in unrelated main-branch changes and kept the disk-hygiene version bump correctly ordered (0.36.0 on top, 0.35.1, 0.35.0, …) with plugin.json at 0.36.0 matching the changelog head.

Findings

None. Re-verified the previously-flagged bootstrap logic line-by-line against destructive_guard.py and kill_switch_probe.py (interpreter resolution, _display_python()'s sys.executable semantics, the MIN_PYTHON runtime check in hygiene.py:5226) and it's internally consistent and matches actual guard behavior. Spot-checked the other six changed files (evals/evals.json, CHANGELOG.md, plugin.json, README.md, docs/skill-cheat-sheet.md, scripts/skill-leaf-name-registry.txt) for consistency with the current SKILL.md and with each other — registry ordering, cheat-sheet entry, README argument hint, and eval expectations all line up; all scan flags referenced in the skill body (--max-depth, --sizes-only, --root-children/--root-child, --project-dir, --quiet, --policy) and the doc links (scan-flags.md, safety-model.md, fan-out-worker-brief.md) resolve to real code/files.

No inline comments posted this round — no new high-signal correctness/maintainability issues found beyond what earlier rounds already covered and this PR already fixed.

Note: this PR's hold has been released by the owner and merge order is documented in the PR body (#5571 first); neither is something this review lane adjudicates.
· branch feat/5516-disk-hygiene-audit-skill

Keep the disk-hygiene version at 0.36.0 above main's 0.35.2 and put this
PR's changelog entry on top.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 59s —— View job


Code review complete

  • Gather context (PR diff, changed files, prior review threads)
  • Invoke /review:code-review and follow its instructions
  • Post inline findings (none — see below)
  • Summary comment

Ran /review:code-review (the CI code-review lane skill) against origin/main...HEAD at head 3b060f5 (7 files, +214/-2).

Skip gate: applies

This lane's skip gate stops deep review when "this head already has a successful review from this lane that still applies." That's the case here:

  • The only diff since the last code-review round on this PR (at 15384a6, comment) is in plugins/disk-hygiene/CHANGELOG.md, and I diffed it directly rather than assuming: 15384a6 → 3b060f5 adds 19 lines there, but comparing origin/main...HEAD shows this PR's own contribution is only its ## [0.36.0] entry for /disk-hygiene:audit; the rest is an unrelated ## [0.35.2] entry (engine-gate probe dedupe, disk-hygiene: _engine_gate_relevant samefile-scans every token of every command, not just separator-carrying words as documented #3527/fix(disk-hygiene): dedupe the engine gate's marker-free identity probes #5526) that arrived via merging origin/main and now sits ahead of this PR's entry in the file — expected changelog reordering from a merge, not new PR content.
  • plugins/disk-hygiene/skills/audit/SKILL.md, evals/evals.json, plugin.json, README.md, docs/skill-cheat-sheet.md, and scripts/skill-leaf-name-registry.txt are byte-identical to what this lane already reviewed at 15384a6 (verified with git diff 15384a6 3b060f5 -- <those paths>, empty output).
  • That 15384a6 round found zero findings, having already re-verified the bootstrap/interpreter-resolution logic line-by-line against destructive_guard.py and kill_switch_probe.py and confirmed cross-file consistency (registry ordering, cheat-sheet entry, README hint, eval expectations, doc links) — see the two prior code-review and security-review comments on this thread from 2026-09-30T18:29–18:30Z.

No new correctness/maintainability surface was introduced by the merge commit itself (3b060f5, "Merge origin/main into feat/5516-disk-hygiene-audit-skill"), so nothing further to report and no inline comments posted this round.

Note: this PR carries a do-not-merge hold-adjacent history in its body, but the label itself is not present on the current PR metadata and the owner's release-hold comment is already in this thread — not something this lane adjudicates either way.
· branch feat/5516-disk-hygiene-audit-skill

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 58s —— View job


Security review — /review:security-review

  • Gather context (PR diff, changed files)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Check instruction-surface deletions against protected-class register
  • Post findings

Skip gate: condition 3 (this head already has a successful security review that still applies).

Verified rather than assumed: git diff origin/main...HEAD at the current head
(3b060f5)
is +214/-2 across the same 7 files as the last reviewed head
(15384a6),
and a direct git diff 15384a6..3b060f5 on those 7 files (docs/skill-cheat-sheet.md,
plugins/disk-hygiene/.claude-plugin/plugin.json, plugins/disk-hygiene/README.md,
plugins/disk-hygiene/skills/audit/SKILL.md, plugins/disk-hygiene/skills/audit/evals/evals.json,
scripts/skill-leaf-name-registry.txt) shows zero content change. The only file with any diff
between those two commits is plugins/disk-hygiene/CHANGELOG.md, and that diff is just a new
## [0.35.2] entry from an unrelated merged-in main commit inserted above this PR's own
## [0.36.0] entry — the PR's own added lines are byte-identical to what the prior round reviewed.
The commits between 15384a6 and 3b060f5 (20379dc fix(disk-hygiene): dedupe the engine gate's marker-free identity probes (#5526), plus the merge commit) only touch
.github/requirements-ci.txt, plugins/code-metrics/**, and
plugins/disk-hygiene/skills/clean/scripts/{destructive_guard.py,test_hygiene.py} — none of which
are part of this PR's diff against origin/main.

This head's PR-owned content was already reviewed twice (security-review rounds at
75bff7a/45e88b5 and again at 15384a6, both finding no issues, plus two code-review rounds),
and nothing in that content has changed since. No new review is needed per the skip gate.

No security issues found in this diff (unchanged from the prior rounds' conclusion).

Note: this PR carries a do-not-merge hold and an explicit Refs #5516 (not Closes) per the PR
body — the owner has since released the hold in the thread above (lane c probed, condition 4
satisfied) and documented a merge order (#5571 first). Neither is a security concern and both are
outside this lane's scope.
· branch feat/5516-disk-hygiene-audit-skill

@kyle-sexton
kyle-sexton merged commit 4e49cab into main Sep 30, 2026
20 checks passed
@kyle-sexton
kyle-sexton deleted the feat/5516-disk-hygiene-audit-skill branch September 30, 2026 18:57
kyle-sexton added a commit that referenced this pull request Sep 30, 2026
…engine-gate denial (#5582)

Refs #5519

## Summary

The Bash engine-gate denial in `destructive_guard.py` gave no hint which
token failed the exact-engine grammar, what the flag-order rule is, or
which mention forms are gated. The denial now says all three. No
behavior change: the guard matches and denies exactly the same commands.

This PR uses `Refs`, not `Closes`; whether it closes #5519 is left to
the owner.

## Fix

- `_engine_mismatch_reason` names what the classifier refuses (parse,
word count, interpreter, engine script, subcommand, `--data-root`,
per-subcommand grammar via `engine_grammar.explain_mismatch`). It runs
only on the deny path.
- An engine operand of a command that is not the hook's Python (`grep
foo "<engine path>"`, `cat "<engine path>"`) is named first, as an
absolute engine path or as a relative word that resolves to the engine
from the current directory. Before, the reason blamed the command word
(`grep`) as "not this hook's Python".
- A word the gate reads as an engine call (a quoted payload holding an
interpreter and the engine filename, such as `gh issue list --search
"python3 <engine>"`) is named as the gated word. Before, the reason
named `gh` and asked for the hook's Python. The gate and the reason
share one helper, `_reads_as_engine_payload`, so they cannot disagree on
which word gated.
- An unparsable command names the first operator class present (pipe,
redirect, `;`, `&`, substitution, glob, newline, `!`/`#`, backslash,
quote) instead of listing all of them. A test pins the label table to
the characters the literal parser rejects.
- `_engine_flag_order_rule` states the flag-order rule from the declared
subcommand specs, and `_ENGINE_GATE_SCOPE` states the gated mention
forms and the read-only forms in the owner's words, with the condition
the owner chose (option 2, PR comment 2026-09-30T19:04Z): a relative
path or bare name that resolves to the installed engine from the current
directory is still gated.
- Relaxing the guard (option a) is deferred by the owner's decision to a
separate, security-reviewed change.
- `disk-hygiene` 0.41.1 with a CHANGELOG entry above 0.41.0. No doc
quotes the old denial text.

## Owner decision applied

The Codex P2 thread found that the advertised read-only forms are denied
when the word resolves to the installed engine (the literal branch of
`_engine_gate_relevant` gates on file identity). The owner took option
2: keep the forms and add the condition.
`test_engine_gate_gates_a_relative_word_that_resolves_to_the_engine`
pins both the gated shapes and that their denial states the condition.

`_engine_gate_relevant` result per working directory:

| Working directory | Command | Gated |
|---|---|---|
| plugin root or repo root | `grep foo <relative path to the engine>` |
yes |
| plugin root or repo root | `git grep foo -- <relative path to the
engine>` | yes |
| repo root | `rg foo <bare engine name>` | no |
| the engine's directory | `rg foo <bare engine name>` | yes |
| the engine's directory | `git grep foo -- <bare engine name>` | yes |
| any directory tried | `git show <rev>:<path to the engine>` | no |
| an unrelated directory | `grep foo <relative path to the engine>` | no
|

## Verification

- `test_hygiene` (run as CI does, from the scripts directory): 674 tests
OK (1 skipped) on the merged head, including tests for the denial text,
agreement between the explainer and the classifier over every declared
subcommand (`handoff-apply` included), the advertised read-only forms
and their resolving-word condition, the engine operand reason, the
payload-word reason, and the operator reason. The payload-word test also
asserts `_engine_gate_relevant` still gates those commands and still
defers a plain mention.
- The rest of the disk-hygiene Python suites (scripts, lib, setup): OK.
- `scripts/run-ruff.sh check` on the changed files: all checks passed.
- `scripts/check-changelog-parity.sh --check`, `--check-order`, and
`scripts/validate-plugins.sh`: pass.

## Related

- Message-only precedent: #3348.
- #5214 is the parent issue. #4218 and #3527 are the same-fault items
for the deferred relaxation.
- Version: 0.41.1, above main's 0.41.0. This branch merged main, which
carries #5541 (`handoff-apply`), #5526, #5590 and #5628 for
disk-hygiene. The final deny in `_decide` is now main's single
`not-exact-engine-command` call with `command=command` added, and the
explainer reads the declared subcommand specs, so it covers
`handoff-apply`'s required flags.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Sep 30, 2026
…ogic with claude-ops (#5632)

Closes #5221

## Summary

Items 1, 2, 4, 5 and 7 of #5221 landed in #5585. This PR completes item
3 (reuse the unreferenced-cache-version logic from the claude-ops
install-state audit) and item 6 (coordination with the related
deep-inventory work).

`disk-hygiene`'s deep inventory carried its own plugin-cache-version
logic, a divergent copy of the claude-ops `audit-install-state` rule: it
read `installed_plugins.json` without the guarded read the audit uses.

## Fix

- One module, `lib/plugin_cache_versions.py`, is carried byte-identical
in `plugins/claude-ops/lib/` and `plugins/disk-hygiene/lib/`.
`install_state.py` and `deep_inventory.py` both call it, so both apply
one rule for which cache versions are unreferenced.
- The module is registered in
`scripts/cross-plugin-source-registry.txt`, so
`scripts/check-cross-plugin-source-drift.sh` fails if the copies
diverge.
- `claude-ops` 0.77.3 and `disk-hygiene` 0.40.1, each with a CHANGELOG
entry.
- Item 6: no second listing to align. The successor of closed #5214,
`/disk-hygiene:audit` (#5590), reads the scan `children_rollup`. The
shared listing schema for the managed-state lane (#4006) lives in
`plugins/disk-hygiene/skills/clean/scripts/deep_inventory.py`
(`ROW_COLUMNS`). #4006 is not folded in and its scope is unchanged.

## Verification

- `bash scripts/check-changelog-parity.sh --check`: pass
- `bash scripts/check-changelog-parity.sh --check-order`: pass (102
changelogs)
- `bash scripts/check-changelog-parity.sh --check-bump origin/main`:
pass
- `bash scripts/validate-plugins.sh`: all manifests and the catalog
validated
- `bash scripts/check-changed-skills.sh origin/main`: 2 skills checked,
0 failed
- `bash scripts/check-cross-plugin-source-drift.sh`: exit 0;
`lib/plugin_cache_versions.py` IDENTICAL [registered]
- `python3 -m unittest` on `test_deep_inventory.py` (52 tests) and
`test_install_state.py` (91 tests): OK

## Related

- #5585: landed items 1, 2, 4, 5, 7 of #5221
- #5420
- #5590: successor of closed #5214; it reads scan `children_rollup`, so
there is no second listing to align
- #4006: open; the shared listing schema is `ROW_COLUMNS` in
`plugins/disk-hygiene/skills/clean/scripts/deep_inventory.py`, for the
managed-state lane to emit. Not folded in here.

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

---------

Co-authored-by: Claude Opus 5.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.

1 participant