Skip to content

fix(disk-hygiene): retract false PowerShell non-firing claim, document the real bare-argv drop (#1195) - #1197

Merged
kyle-sexton merged 4 commits into
mainfrom
fix/1195-correct-false-powershell-claims
Jul 24, 2026
Merged

fix(disk-hygiene): retract false PowerShell non-firing claim, document the real bare-argv drop (#1195)#1197
kyle-sexton merged 4 commits into
mainfrom
fix/1195-correct-false-powershell-claims

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Retracts a wrong claim this repo shipped in disk-hygiene 0.8.2. That release documented — in the
safety model — that "PreToolUse guards do not intercept PowerShell-tool commands," scoped behind a
preview caveat. A fresh-session controlled test falsified it:

  • a Bash|PowerShell PreToolUse matcher does fire for the PowerShell tool on 2.1.218,
  • the payload tool_name is literally PowerShell (and CLAUDE_TOOL_NAME is not a hook env var —
    hooks must read tool_name from stdin),
  • a live Set-Content through the PowerShell tool was blocked by guardrails.

No harness firing divergence, no preview limitation. 0.8.2 asserted an un-isolated inference as fact;
this removes it.

The real defect, now documented accurately: hooks/hooks.json passes a bare
${user_config.disk_hygiene_enabled}. Upstream never implemented the declared userConfig default, so
an unset-but-defaulted token is neither substituted nor exported as CLAUDE_PLUGIN_OPTION_*, and its
presence drops the entire hook entry. The plugin-level engine gate has therefore been inert for every
consumer who never explicitly set the key — on Bash and PowerShell alike, which is the real shape of
the reported "PowerShell bypass". The skill-scoped belt carries no such token and is unaffected.

Docs-only retraction (disk-hygiene 0.8.3). The two code fixes — guardrails' PowerShell producer-scan
coverage gap, and a kill-switch delivery channel that doesn't depend on the unimplemented default
are tracked separately.

Test plan

  • Fresh-session controlled test (claude -p subprocesses, probe plugin, token-free control hooks):
    token-carrying hooks vanish when the key is unset while token-free controls fire, and reappear
    unchanged once the key is configured → the bare-argv drop is proven, not inferred. An unset key is
    also not exported to env (CLAUDE_PLUGIN_OPTION_* unset), so an in-script default is mandatory.
  • PowerShell firing verified by payload dump on both tools plus a live PreToolUse:PowerShell block.
  • skill-quality:check disk-hygiene/cleanPASS (0 errors; markdownlint clean; triggers preserved).
  • Grep confirms no residual "does not fire for the PowerShell tool" / preview-caveat text remains.

Related

Closes #1195 (reopened with the falsification recorded; its original diagnosis was wrong).
Upstream gap behind the real defect: anthropics/claude-code #46477 / #39455 / #39827 (userConfig
default declared but never applied to substitution or env export).

…t the real bare-argv drop (#1195)

0.8.2 shipped a safety-model claim that PreToolUse guards do not intercept
PowerShell-tool commands, scoped behind a preview caveat. A fresh-session
controlled test falsified it: a Bash|PowerShell matcher DOES fire for the
PowerShell tool on 2.1.218, payload tool_name is literally "PowerShell", and a
live Set-Content through that tool was blocked. No harness divergence, no preview
limitation — 0.8.2 asserted an un-isolated inference as fact. Removed.

The real defect, now documented: hooks/hooks.json passes a bare
${user_config.disk_hygiene_enabled}, and upstream never implemented the declared
userConfig default, so an unset-but-defaulted token is neither substituted nor
exported to CLAUDE_PLUGIN_OPTION_* and its presence drops the entire hook entry.
The engine gate has therefore never run for any consumer who never set the key —
on Bash and PowerShell alike, which is the real shape of the reported bypass. The
skill-scoped belt carries no such token and is unaffected. Code fix tracked
separately. disk-hygiene 0.8.3.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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


PR Review

  • Gather context (PR body, changed files)
  • Read changed files and diff
  • Review correctness and accuracy of claims
  • Review alignment with CLAUDE.md guidelines
  • Post final review

This is a docs-only correction (disk-hygiene 0.8.3) retracting a false PowerShell safety claim from 0.8.2 and replacing it with an accurate description of the real defect. The changes are internally consistent and well-reasoned.

What Changed

File Change
plugin.json Version bump 0.8.2 → 0.8.3
CHANGELOG.md 0.8.3 entry with detailed retraction + real defect
SKILL.md Replaces PowerShell-non-firing caveat with bare-argv-drop caveat; adds two new gotcha bullets
reference/safety-model.md Same retraction; rewrites the preview-caveat paragraph

Correctness

The retraction is accurate. 0.8.2 asserted that a Bash|PowerShell PreToolUse matcher did not intercept PowerShell-tool commands. The new text correctly reverses that: hooks do fire, tool_name in the payload is literally PowerShell, and the CLAUDE_TOOL_NAME env var note (SKILL.md line 329) is a useful precision — hooks must read from stdin, not a nonexistent env var.

The real defect is described accurately. The bare ${user_config.disk_hygiene_enabled} in hooks/hooks.json causes the whole hook entry to be dropped when the key is unset, because the upstream runtime doesn't implement the declared default. This is consistent with what plugin.json shows: plugin.json line 25 still declares "default": true, confirming the gap is in the runtime, not the manifest.

The skill-scoped belt / plugin-level gate distinction is preserved correctly. Both SKILL.md and safety-model.md consistently note that the skill-scoped belt carries no ${user_config.*} token and is unaffected — this is the right split.

Cross-file consistency: The inline caveat in SKILL.md ("inert until explicitly configured") and the safety-model.md heading ("inert by default") are semantically equivalent and point to the same upstream issue numbers.

Observations (non-blocking)

  1. plugin.json still declares "default": true with no in-manifest hint of the runtime gap. JSON doesn't support comments, so this is not actionable in this PR, but consumers inspecting the manifest alone would not see the caveat. The tracking issue approach (upstream #46477 / #39455 / #39827) is the right workaround.

  2. Upstream issue numbers in the recheck markers (#46477 / #39455 / #39827) are internal to Claude Code's issue tracker and won't be navigable by external consumers. This is acceptable given they're labeled as upstream gaps, but if there's ever a public tracker entry or docs page for this behavior, linking it would improve discoverability.

  3. safety-model.md line 162 still reads "the guard now registers on two surfaces" — the word "now" is holdover language from when that two-surface registration was introduced and is slightly stale. Not introduced by this PR, so out of scope here.

  4. CHANGELOG entry for 0.8.3 has no date appended to the header (consistent with 0.8.2 and earlier entries in this file — no action needed).

Verdict

No correctness bugs or security issues. The documentation now accurately reflects the behavioral reality: PreToolUse hooks do fire for the PowerShell tool, and the plugin-level engine gate has been silently inert for unconfigured consumers since initial deployment. The scope is properly limited to a docs retraction, and the code fix is correctly deferred. Safe to merge.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — disk-hygiene 0.8.3

Scope: Changed files only (plugin.json, CHANGELOG.md, SKILL.md, reference/safety-model.md). This is a docs-only PR.

  • Gather PR context and changed files
  • Read changed files and diff
  • Perform security analysis
  • Post findings

Findings

No security vulnerabilities found in the changed files.

All four files contain only documentation and a version bump. Full analysis below:


plugin.json — version bump 0.8.2 → 0.8.3

permalink

No security-relevant change. The userConfig.disk_hygiene_enabled declaration with "default": true remains as-is, which is correct to keep (consumers who do set the key need a valid declared type/default; the PR body correctly scopes the non-functioning default as an upstream runtime gap, not a manifest error).


CHANGELOG.md — new 0.8.3 entry

Documentation only. No injection surface, no executable content.


SKILL.md and reference/safety-model.md — retraction + real-defect documentation

These are the substantive changes. The updated docs now accurately describe:

  1. The real security limitation: the plugin-level engine gate in hooks/hooks.json drops its entire hook entry when disk_hygiene_enabled is unconfigured, because the declared userConfig default is not implemented upstream. This means for any consumer who never set the key, the engine gate has never run. This is a pre-existing vulnerability being documented — not introduced by this PR.

  2. What is still in force: the skill-scoped belt (frontmatter hook, carries no ${user_config.*} token) is correctly described as unaffected. The description that Bash|PowerShell PreToolUse matchers do fire for the PowerShell tool is accurate and consistent with the controlled test evidence cited in the PR body.

  3. No false sense of security: the caveat in both files is prominent, accurate, and does not overstate residual protections. The skill-scoped belt is the layer that still holds; the docs say so without ambiguity.

One observation worth noting (not a vulnerability in this PR): the documented defect — engine gate inert by default — remains unresolved in the plugin until the upstream userConfig default gap is fixed (#46477 / #39455 / #39827) or the code fix tracked separately is landed. Consumers reading these docs are warned, but users who installed 0.8.2 or earlier and never set disk_hygiene_enabled remain exposed to the inert gate in production. This PR correctly labels that as a separate tracked fix rather than a doc-only mitigation.


Summary: docs-only retraction; no new attack surfaces, no permission widening, no injection risk, no GitHub Actions workflow changes, no secret handling. The updated documentation is more accurate about the true security posture than the 0.8.2 release it replaces.

@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: a28124f2c2

ℹ️ 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/CHANGELOG.md Outdated
…re-argv drop (#1195)

README still told consumers the engine gate "fires in every session" and that
audit-only mode is "guard-enforced ... in every session". Per the proven bare-argv
drop, the gate only registers once disk_hygiene_enabled is explicitly configured;
on a default (unconfigured) install it does not run at all. Both claims scoped to
the configured case with the caveat; the skill self-enforcement is named as the
primary kill-switch honoring on a default install.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — disk-hygiene 0.8.3

  • Gather PR context
  • Read changed files and diff
  • Analyze for security issues
  • Post findings

Scope: Changed files only — plugin.json, CHANGELOG.md, README.md, SKILL.md, reference/safety-model.md. This is a docs-only PR (version bump + documentation corrections).

No security vulnerabilities were introduced by this PR.

Full analysis below.


plugin.json — version bump 0.8.2 → 0.8.3

permalink

Sole change is the version string. No permissions, hooks, scripts, dependencies, or executable content touched.

CHANGELOG.md — new 0.8.3 entry

Documentation only. No injection surface, no executable content.

README.md, SKILL.md, reference/safety-model.md — retraction + real-defect documentation

All three files contain only Markdown prose. No executable content, no secret handling, no access-control logic.

The updated text accurately documents a pre-existing security limitation (the plugin-level engine gate silently dropping for unconfigured consumers) rather than introducing one. Documenting a known security gap is the correct action; it reduces false trust without widening any attack surface.

Specific points checked:

  • No new command injection surfaces. No shell snippets, hook args, or template expressions were added that could be manipulated via user input.
  • No permission widening. No GitHub Actions permissions: keys, settings files, or workflow definitions were touched.
  • No supply-chain changes. No action pins, package versions, or dependency declarations changed.
  • No secret/credential handling. The ${user_config.disk_hygiene_enabled} token described in the docs is a boolean kill-switch flag, not a credential. The docs correctly note it is not exported to env when unset — that is the bug being documented, not a new exposure.
  • No pull_request_target / workflow_run patterns. No workflow files changed at all.
  • The hooks/hooks.json file is not in the diff. The pre-existing bare-argv bug it contains is accurately described but not modified here — the code fix is correctly deferred.

Verdict: docs-only retraction; no new attack surfaces, no permission widening, no injection risk, no GitHub Actions workflow changes, no secret handling. The documentation now more accurately describes the true security posture than 0.8.2 did. Safe to merge from a security standpoint.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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


PR Review

  • Gather context (PR body, changed files, previous reviews)
  • Read the full diff and changed files
  • Review new README.md changes (latest commit 3d658d4)
  • Check cross-file consistency
  • Post final review

This is a docs-only retraction (disk-hygiene 0.8.3) retracting the false PowerShell non-firing claim from 0.8.2 and replacing it with accurate documentation of the real bare-argv drop defect. The latest commit (3d658d4) addresses the Codex reviewer's finding that README.md still carried stale engine-gate claims.

What Changed (all three commits)

File Change
plugin.json Version bump 0.8.2 → 0.8.3
CHANGELOG.md 0.8.3 entry: PowerShell retraction + bare-argv-drop defect
README.md Removes "fires in every session" / "guard-enforced" claims; adds bare-argv-drop caveat in two places
SKILL.md Replaces PowerShell-non-firing caveat with bare-argv-drop caveat; adds new PowerShell-does-fire gotcha
safety-model.md Same retraction; rewrites preview-caveat paragraph

README.md — new commit (3d658d4) ✓

The Codex-flagged stale claims are now corrected:

  • Line 53–54 (permalink): The old "fires in every session" language is gone; the text now says the gate "acts only on commands referencing the engine — deferring everything else instantly" and immediately follows with the bare-argv caveat.
  • Lines 156–165 (permalink): The old "guard-enforced in every session" claim is replaced with an accurate scoped statement: the gate only enforces when the value is explicitly configured false, and the unconfigured default drops the hook entirely.

Both corrections are accurate and consistent with SKILL.md and safety-model.md.

Finding: SKILL.md line 302 still says "fires in every session"

SKILL.md line 302 — in the general gotchas list (the "guard registers twice" bullet) — still reads:

The guard registers twice: a plugin-level engine gate (hooks/hooks.json, --mode engine-gate) that fires in every session, receives the kill switch and data root by plugin-hook substitution...

This is the same stale claim the latest commit correctly purged from README.md. The CHANGELOG.md entry even cites that correcting the "fires in every session" claim was in scope:

README.md (which still claimed the gate "fires in every session" and audit-only is "guard-enforced") corrected

But SKILL.md line 302's identical phrasing was not updated. The new gotcha at lines 319–326 documents the bare-argv drop accurately and contradicts this, so the two bullets are now in direct conflict within the same file. A reader following line 302 and then hitting line 319 gets opposing claims.

Fix this →

Minor observation: README.md line 60 readability

README.md line 60 reads:

...the gate does not run at all, on either tool. The skill-scoped belt below carries no such token and is unaffected. And the skill-scoped belt inside the clean skill's context...

The "And the" is a grammatical seam from the original list structure ("registers on two surfaces: a plugin-level gate… ; and the skill-scoped belt") where the caveat paragraph was inserted. The "And the" now opens what looks like an independent sentence but is actually completing the original list item. Not wrong — a careful reader can follow it — but slightly jarring. Not a blocking issue.

Other files: no issues

  • CHANGELOG.md: accurate summary; mentions the README corrections were in scope (consistent with what was done).
  • safety-model.md: the caveat paragraph is well-structured; skill-scoped belt unaffected claim is correct; PowerShell-does-fire claim is accurate.
  • plugin.json: version bump only.

Out-of-scope note (not blocking)

plugins/disk-hygiene/skills/setup/SKILL.md line 32 also has "fires in every session but acts only on engine-referencing commands" — the same stale family of claims. That file is not in this PR's scope, but it will carry conflicting information until addressed.

Verdict

The SKILL.md line 302 inconsistency is the only material issue: two gotcha bullets in the same section now say opposite things about whether the engine gate fires unconditionally. Everything else is correct. The PR is otherwise a clean, accurate retraction — the README corrections from 3d658d4 fully address the Codex finding. Blocking only on the SKILL.md line 302 fix; the out-of-scope items can be tracked separately.

@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: 3d658d497a

ℹ️ 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
…ine-gate claims (#1195)

Codex review + a full sweep found three more spots that still told consumers the
plugin-level engine gate fires in every session / audit-only is guard-enforced,
contradicting this retraction: clean SKILL.md Gotchas, setup SKILL.md step 1
(direct hygiene.py invocations "meet the engine gate"), and safety-model.md's
kill-switch-reach paragraph. All three scoped to the explicitly-configured case
with the bare-argv-drop caveat, so a default-configured consumer no longer gets
contradictory safety guidance.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — disk-hygiene 0.8.3

Scope: Changed files only — plugin.json, CHANGELOG.md, README.md, skills/clean/SKILL.md, skills/clean/reference/safety-model.md, skills/setup/SKILL.md. This is a docs-only PR (version bump + documentation corrections).

  • Gather PR context
  • Fetch PR diff
  • Read changed files
  • Analyze for security issues
  • Post findings

No security vulnerabilities were introduced by this PR.

Per-file analysis

plugin.json — sole change is version 0.8.20.8.3. No permissions, hooks, scripts, dependencies, or executable content touched. The userConfig.disk_hygiene_enabled declaration with "default": true is unchanged; the PR body correctly identifies the non-functioning default as an upstream runtime gap, not a manifest error.

CHANGELOG.md — documentation only. No injection surface, no executable content.

README.md, skills/clean/SKILL.md, skills/clean/reference/safety-model.md, skills/setup/SKILL.md — all Markdown prose. No executable content, no secret handling, no access-control logic.

Security-relevant observations

The substantive changes accurately document a pre-existing security limitation — not introduce one:

  • The plugin-level engine gate in hooks/hooks.json silently drops for any consumer who never explicitly set disk_hygiene_enabled, because the declared userConfig default is not implemented by the upstream runtime. This means those consumers have never had the engine gate active.
  • The updated docs correctly describe which layer still holds (skill-scoped frontmatter belt) and which is silent (plugin-level engine gate on a default install).
  • No false sense of security is introduced: the caveat is prominent in every affected file and does not overstate residual protections.

Points confirmed as clear:

Check Result
New command injection surfaces None
Permission widening (GitHub Actions, settings) None — no workflow files touched
Supply-chain changes (action pins, dependencies) None
Secret/credential handling None — ${user_config.disk_hygiene_enabled} is a boolean flag, not a credential
pull_request_target / workflow_run patterns Not present — no workflow changes
hooks/hooks.json modified No — the pre-existing bare-argv bug is documented but not changed; the code fix is correctly deferred

Out of scope (pre-existing, not introduced here)

The documented defect — engine gate inert on default installs — remains unresolved in the plugin until the upstream userConfig default gap is closed or the deferred code fix lands. Consumers who installed 0.8.2 or earlier and never set disk_hygiene_enabled remain exposed to the inert gate. This PR correctly labels that as a separate tracked fix and does not falsely claim it is mitigated.


Verdict: docs-only retraction; no new attack surfaces, no permission widening, no injection risk, no GitHub Actions workflow changes, no secret handling. The documentation now more accurately describes the true security posture than 0.8.2 did. Safe to merge from a security standpoint.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — disk-hygiene 0.8.3 (commit b5c4a66)

  • Gather context (PR body, changed files, prior reviews)
  • Read full diff (git diff origin/main...HEAD)
  • Verify previously-flagged SKILL.md line 302 stale claim is resolved
  • Verify previously-flagged skills/setup/SKILL.md:31-34 stale claim is resolved
  • Check cross-file consistency across all 6 changed files
  • Post final review

This commit (b5c4a66) addresses all remaining issues flagged in prior reviews. Full analysis below.

Resolved: SKILL.md line 302 stale claim ✓

The prior review flagged that SKILL.md line 302 still read "fires in every session" while contradicting the new caveat at line 319. That claim is now gone:

The guard registers twice: a plugin-level engine gate (hooks/hooks.json, --mode engine-gate) that receives the kill switch and data root by plugin-hook substitution and defers instantly on any command not referencing the engine

The caveat is appended inline at lines 305–308 — accurate and consistent with the new gotcha bullet at line 322.

Resolved: skills/setup/SKILL.md:31-34 stale claim ✓

The Codex reviewer flagged that the setup SKILL.md still told consumers that direct hygiene.py invocations met the engine gate. setup/SKILL.md line 33 now reads:

Caveat, verified on Claude Code 2.1.218: the engine gate only registers once disk_hygiene_enabled is explicitly configured — its bare ${user_config.*} argument drops the whole hook while the option is unset, so on a default install direct hygiene.py invocations meet NO plugin-level hook

This is accurate and directly reverses the prior misleading description.

CHANGELOG consistency ✓

The 0.8.3 entry states it corrected "the clean and setup SKILL.md files, reference/safety-model.md, and the consumer README.md" — the diff confirms all four were updated in this PR. CHANGELOG is accurate.

Cross-file consistency ✓

All six changed files now consistently assert:

  • Engine gate is inert until disk_hygiene_enabled is explicitly configured
  • Skill-scoped belt carries no ${user_config.*} token and is unaffected
  • Bash|PowerShell PreToolUse hooks do fire for the PowerShell tool
  • Recheck marker #46477 / #39455 / #39827 appears uniformly in CHANGELOG, SKILL.md, and safety-model.md

Minor observation (non-blocking)

safety-model.md line 172 has a prose seam where "Recheck when the upstream gap closes (#46477 / #39455 / #39827)." was inserted inline before "The gate defers instantly (no output)…" without a paragraph break. Readable as-is, but a blank line between them would improve flow. Not a correctness issue.

The "And the" grammatical seam in README.md line ~60 (noted in the prior review as non-blocking) persists — it predates this PR and is out of scope here.

Verdict

All blocking findings from prior reviews are resolved. No correctness bugs, no new attack surfaces, no security regressions. The documentation now accurately reflects the behavioral reality across all six files. Safe to merge.

@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: b5c4a6617d

ℹ️ 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/README.md
…d toggle (#1195)

Review P2: the 0.7.0 trust-surface record still said the plugin-level hook
"launches in every consumer session" and charged one python3 launch per Bash/
PowerShell call — contradictory in the exact default configuration this retraction
corrects. Conditioned on disk_hygiene_enabled being explicitly configured, with the
bare-argv-drop caveat: on a default install the hook does not register or launch, so
neither the trust surface nor its per-call cost applies until the option is set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — disk-hygiene 0.8.3 (commit d58fd60)

  • Gather context (PR body, changed files, prior reviews)
  • Read the diff for the latest commit
  • Verify Codex-flagged trust-surface claim (README.md:161) is resolved
  • Check cross-file consistency across all changed files
  • Post final review

This commit (d58fd60) resolves the final Codex P2 finding flagged against b5c4a66. All prior blocking issues are now addressed.

What d58fd60 changed

Only README.md was touched (12 insertions, 8 deletions). The change targets the 0.7.0 trust-surface record in the plugin-acceptance security review section.

Before: the trust-surface record said the hook "launches in every consumer session" and accepted "one python3 launch per Bash/PowerShell call" as a known cost — both claims unconditionally true, directly contradicting the bare-argv-drop caveat introduced in prior commits.

After:

  • Line 167: "a hook that launches in every consumer session once disk_hygiene_enabled is explicitly configured"
  • Line 173: "one python3 launch per Bash/PowerShell call on a configured install"
  • Lines 175–177: inline caveat block — while disk_hygiene_enabled is unset, the hook does not register or launch, so neither the trust surface nor its per-call cost applies

This is accurate and directly answers the Codex P2.

Cross-file consistency ✓

All six changed files now consistently assert:

Claim Status
Engine gate only registers when disk_hygiene_enabled is explicitly configured ✓ all files
Skill-scoped belt carries no ${user_config.*} token, unaffected by the drop ✓ all files
`Bash PowerShell` PreToolUse hooks DO fire for the PowerShell tool
Trust surface and per-call cost conditional on configured install ✓ README.md (new)
Recheck marker #46477 / #39455 / #39827 ✓ CHANGELOG, SKILL.md, safety-model.md

Carried-forward minor observations (non-blocking, pre-existing)

  1. safety-model.md line 171–172: "Recheck when the upstream gap closes (#46477 / #39455 / #39827)." runs directly into "The gate defers instantly (no output)…" with no paragraph break. Noted in the b5c4a66 review; not introduced here; readable as-is.

  2. README.md line 60: "And the skill-scoped belt…" is a grammatical seam left from where the caveat paragraph was inserted. Noted in prior reviews; out of scope here.

Neither item is a correctness issue.

Verdict

All Codex P2 findings are resolved. No correctness bugs, no security regressions, no new attack surfaces. The documentation now accurately represents the behavioral reality — including the trust-surface and per-call-cost claims — across all six changed files. Safe to merge.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — disk-hygiene 0.8.3

  • Gather PR context
  • Fetch PR diff and read changed files
  • Analyze for security issues
  • Post findings

Scope: Changed files only — plugin.json, CHANGELOG.md, README.md, skills/clean/SKILL.md, skills/clean/reference/safety-model.md, skills/setup/SKILL.md. This is a docs-only PR (version bump + documentation corrections).

No security vulnerabilities were introduced by this PR.

Per-file analysis

plugin.json — sole change is version 0.8.20.8.3. No permissions, hooks, scripts, dependencies, or executable content touched.

CHANGELOG.md — documentation only. No injection surface, no executable content.

README.md, skills/clean/SKILL.md, skills/clean/reference/safety-model.md, skills/setup/SKILL.md — all Markdown prose. No executable content, no secret handling, no access-control logic.

Security checks

Check Result
New command injection surfaces None
Permission widening (GitHub Actions, settings) None — no workflow files touched
Supply-chain changes (action pins, dependencies) None
Secret / credential handling None — ${user_config.disk_hygiene_enabled} is a boolean flag, not a credential
pull_request_target / workflow_run patterns None — no workflow files changed
hooks/hooks.json modified No — the pre-existing bare-argv bug is documented but not changed; the code fix is correctly deferred
Script injection via github context in run: blocks Not applicable — no workflow files changed

Pre-existing defect now documented (not introduced here)

The substantive changes accurately document a pre-existing security limitation rather than introduce one. The plugin-level engine gate in hooks/hooks.json silently drops its entire entry for consumers who never explicitly set disk_hygiene_enabled, because the declared userConfig default is not implemented upstream. This means those consumers have never had the engine gate active — on Bash and PowerShell alike.

This is the correct course of action: documenting a known security gap reduces false trust without widening any attack surface. The remaining protection (skill-scoped frontmatter belt, documented accurately in every updated file) is correctly scoped and unaffected.

Verdict

Docs-only retraction; no new attack surfaces, no permission widening, no injection risk, no GitHub Actions workflow changes, no secret handling. The documentation now more accurately describes the true security posture than 0.8.2 did. Safe to merge from a security standpoint.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

CI classification: all checks green; all Codex review threads resolved after four rounds of caveat completion — every doc that claimed the plugin-level engine gate 'fires in every session' or that audit-only is 'guard-enforced' (clean + setup SKILL.md, safety-model.md, README ×3: two-surface record, config section, trust-surface record) is now conditioned on disk_hygiene_enabled being explicitly configured, with the bare-argv-drop caveat. Docs-only retraction of the incorrect 0.8.2; no code/trust-surface change. Merging (squash).

@kyle-sexton
kyle-sexton merged commit 866a461 into main Jul 24, 2026
25 checks passed
@kyle-sexton
kyle-sexton deleted the fix/1195-correct-false-powershell-claims branch July 24, 2026 00:15
kyle-sexton added a commit that referenced this pull request Jul 24, 2026
…tly (#1242)

## Summary

The `disk_hygiene_enabled` kill switch was **inert on a default
install**. The plugin-level engine-gate hook (`hooks/hooks.json`)
carried a bare `${user_config.disk_hygiene_enabled}` argument; because
the declared userConfig `default` is unimplemented upstream (#46477 /
#39455 / #39827), an unset-but-defaulted token **drops the whole hook
entry**, so the gate never ran. The skill-frontmatter belt could not
receive the value either (skill hooks get neither the `${user_config.*}`
substitution nor `CLAUDE_PLUGIN_OPTION_*`). Audit-only mode therefore
degraded from **deny-outright** to **prompt-gated**.

**Closes #1019.**

## Fix

Both guard surfaces now resolve the toggle by **reading
`disk_hygiene_enabled` directly** from user-scope `pluginConfigs` in
`settings.json`, via a new shared `lib/killswitch_config.py` reader:

- **Located from the tamper-resistant `${CLAUDE_PLUGIN_ROOT}`** both
surfaces already receive (fallback to `CLAUDE_CONFIG_DIR`/`HOME` only
when `--plugin-root` is absent — the report CLI / unit tests).
- **Environment is never consulted** for the toggle or the settings
path: a repo `.claude/settings.json` `env` block reaches hook
subprocesses and carries no provenance. Since CC **2.1.207**
`pluginConfigs` is honored only from user/managed/`--settings` scope
(project/local ignored), so a hostile repo cannot forge it.
- **Fails closed to enabled** on any absent/unreadable/ambiguous read.
- The bare `${user_config.*}` argv is removed from `hooks.json` (fixing
the hook-drop); `kill_switch_probe.py` now delegates to the same reader
(its single-line JSON contract unchanged).

## Design note — supersedes the planned SessionStart + state-file ("C′")

The two enforcement surfaces are the **same script through one resolve
point** (`destructive_guard.py`), so there is nothing to distribute
between sessions or surfaces. A direct read is a **smaller trust
surface** (a settings *read*, no state-file *write*), honors a
mid-session settings change, and needs no session-start timing
dependency. Semantics are unchanged from the locked resolver decision
(read user-scope `pluginConfigs`, ignore env, fail closed to enabled).

## Enforcement reach (accurate)

- **Bash engine invocations** → denied outright in audit-only by the
always-on engine gate, whether or not the clean skill is active.
- **PowerShell deletion spellings** → denied outright by the
skill-scoped belt **while `/disk-hygiene:clean` is active** (the
always-on gate defers on non-engine commands).

## §643 plugin-acceptance security review — PASS

The one new surface is a **settings-file read** (criterion 4). The
playbook's own carve-out sanctions reading the plugin's own documented
`~/.claude` user-global config; it's located from
`${CLAUDE_PLUGIN_ROOT}` with no `../` reach-out into consumer-repo data.
Criterion 1 (hooks) is *reduced* — a `${user_config.*}` arg removed.
Surfaces 2/5/6/7 untouched. Trust-surface delta recorded in `README.md`
+ `CHANGELOG.md`.

## Testing

- `test_hygiene.py`: **179 tests OK** (new `DirectReadKillSwitchTests`
covering the plugin-root channel, `CLAUDE_CONFIG_DIR` fallback,
tamper-resistance, env/argv ignored, fail-closed; new `hooks.json`
contract test locking `--plugin-root` present + bare `${user_config.*}`
gone; guard helpers made hermetic).
- `test_kill_switch_probe.py`: **17 tests OK, unchanged**
(behavior-preserving extraction).
- markdownlint clean; `hooks.json`/`plugin.json` valid.

## Related

- #1195 — the reported "PowerShell bypass" whose real cause (inert
engine gate) this fixes.
- #1197 — disk-hygiene 0.8.3, which documented this defect and left the
code fix to a follow-up (this PR).
- #1106 — the split-registration (Option E) decision that introduced the
plugin-level engine-gate trust surface updated here.
- Upstream userConfig `default` gap this works around: #46477 / #39455 /
#39827 (recheck if resolved).

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <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.

disk-hygiene: PowerShell-tool deletion belt inert on Windows — docs over-promise protection that doesn't fire (preview-tool hook divergence)

1 participant