Skip to content

fix(disk-hygiene): launch wired hooks in shell form, reviving the dead destructive guard - #2570

Merged
kyle-sexton merged 5 commits into
mainfrom
fix/disk-hygiene-hook-shell-form
Aug 13, 2026
Merged

fix(disk-hygiene): launch wired hooks in shell form, reviving the dead destructive guard#2570
kyle-sexton merged 5 commits into
mainfrom
fix/disk-hygiene-hook-shell-form

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Problem

disk-hygiene's PreToolUse destructive-operation guard has not been running at all on Windows hosts whose PATH resolves bash to the WSL relay. This is a dead safety guard, not log noise: destructive_guard.py never executes, so destructive Bash/PowerShell commands proceed ungated while the operator sees only PreToolUse:Bash hook error lines.

Both registrations in hooks/hooks.json used exec form:

{ "type": "command", "command": "bash", "args": ["${CLAUDE_PLUGIN_ROOT}/hooks/run-python-hook.sh", "..."], "timeout": 60 }

Reproduction

$ where.exe bash            # from PowerShell, i.e. the real Windows PATH
C:\Windows\System32\bash.exe
C:\Users\<user>\AppData\Local\Microsoft\WindowsApps\bash.exe

Git Bash's directory is not on the Windows PATH at all, so the WSL relay wins. Invoking the launcher through it:

$ printf '{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"rm -rf /"}}' \
    | /c/Windows/System32/bash.exe plugins/disk-hygiene/hooks/run-python-hook.sh \
        plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py --mode engine-gate
<3>WSL (31 - Relay) ERROR: CreateProcessCommon:818: execvpe(/bin/bash) failed: No such file or directory
EXIT=1

The only installed WSL distro is docker-desktop, which has no /bin/bash.

Why it is silent

Per the hooks reference:

Exec form runs when args is present. Claude Code resolves command as an executable on PATH and spawns it directly with args as the argument vector, with no shell involved. … No shell tokenization happens on any platform.

On Windows, exec form requires command to resolve to a real executable such as a .exe.

And a hook that cannot start is non-blocking:

A hook that can't start lands in the same non-blocking bucket. … For most hook events, the action proceeds. When you set up a policy hook, watch for this notice on its first run: a mistyped path in settings.json leaves the gate silently disabled.

So "guard never ran" and "guard approved" are indistinguishable from outside.

This is a regression, and the repo already knew the rule

0.17.6 moved both registrations onto "command": "bash" + args to fix Python resolution (#1504) — reintroducing the exact launch failure #1006 had already fixed on the skill-frontmatter surface. #1416 was closed COMPLETED while the guard stayed dead; it is reopened by this PR.

plugins/claude-config/skills/audit/reference/audit-checklist.md Category D already carries this as an error row naming this precise failure — and disk-hygiene shipped it anyway. A checklist a human reads is not a gate; see #2569.

Fix

Shell form, which Claude Code routes through its own Git Bash on Windows rather than a PATH lookup:

Shell form runs when args is absent. The command string is passed to a shell: sh -c on macOS and Linux, Git Bash on Windows, or PowerShell when Git Bash isn't installed. Set the shell field to choose explicitly.

{
  "type": "command",
  "command": "\"${CLAUDE_PLUGIN_ROOT}\"/hooks/run-python-hook.sh \"${CLAUDE_PLUGIN_ROOT}\"/skills/clean/scripts/destructive_guard.py --mode engine-gate --plugin-root \"${CLAUDE_PLUGIN_ROOT}\" --authorized-data-root \"${CLAUDE_PLUGIN_DATA}\"",
  "shell": "bash",
  "timeout": 60
}

The #1504 Python-resolution behaviour is untouched — only the launch mechanism moves.

Design notes

  • Every placeholder is double-quoted, per the page's "In shell form, wrap each placeholder in double quotes." Verified for both hooks that the resulting argv is byte-identical to the exec-form vector, against a CLAUDE_PLUGIN_ROOT and a CLAUDE_PLUGIN_DATA containing spaces and backslashes. The Stop hook was checked explicitly: guard_launch_monitor.py is what emits the systemMessage when no interpreter resolves, so a mangled --data-root there would have broken the detector that makes a dead guard visible.
  • "shell": "bash" is declared explicitly, unlike sibling guardrails which omits it. Shell form otherwise falls back to PowerShell on a Windows host with no Git Bash detected, which cannot run a .sh. This is the spelling claude-config's Category D row names as the fix, and the one fix(repo-hygiene): destructive-guard fail-open on Windows — shell-form hook launch + jq fail-closed degraded mode #1006 used. It is ignored when args is set, so it is meaningful only in shell form.
  • Exec form with a real binary was not viable here. The launcher is a .sh, so it needs bash — and there is no portable spelling of Git Bash's bash.exe to put in command. That is circular, which is why the page's general "prefer exec form for path placeholders" preference (a quoting-safety point) yields to the Windows platform constraint.
  • No environment workaround. CLAUDE_CODE_GIT_BASH_PATH and PATH reordering are explicitly not the fix; shell form is resolved by Claude Code, so PATH order neither causes nor fixes this.

Tests fixed — both encoded the bug as the contract

This is why the defect survived two fix attempts:

  • hooks/run-python-hook.test.sh asserted .command == "bash" and read .args[0]. It now asserts the portability property: launcher named in command, args absent, shell: bash, and every ${CLAUDE_PLUGIN_*} placeholder double-quoted. Verified it fails against the old exec-form JSON.
  • skills/clean/scripts/test_hygiene.py selected guard hooks with if hook.get("args") and …, which matches nothing once a hook moves to shell form — every assertion built on it would have gone vacuously green. Replaced with a form-agnostic _hook_argv() (shlex.split for shell form, [command, *args] for exec form) plus _guard_argv_from_hook().

Audit — full list of instances

Audited every tracked JSON file repo-wide via a recursive walk for type: command + args — which covers the 18 hooks/hooks.json files, any manifest-pointed hook config, and inline hooks objects declared in a plugin.json — plus every SKILL.md and agent-definition YAML frontmatter hooks: block across all 77 plugins.

# Location Shape Status
1 plugins/disk-hygiene/hooks/hooks.jsonPreToolUse exec form, "command": "bash" Fixed here (proven dead)
2 plugins/disk-hygiene/hooks/hooks.jsonStop exec form, "command": "bash" Fixed here (proven dead)
3 plugins/disk-hygiene/skills/clean/SKILL.md frontmatter exec form, "command": "python3" Deferred → #2568 (latent)

No other plugin in the repo has this defect — disk-hygiene is the only one using exec form at all. Every other hook, including all of guardrails, is already quoted shell form.

Why #3 is deferred rather than fixed here: it is latent, not live — on the reporting host python3 resolves to a real binary, so that guard currently works. It would fail only where python3 is the zero-length WindowsApps alias stub. Converting a currently-working safety guard on a premise not verifiable in CI risks killing a live guard, which is the exact harm this PR fixes. #2568 carries the full analysis and the #1014 constraint (skill hooks receive only ${CLAUDE_PLUGIN_ROOT}; --authorized-data-root must not be reintroduced).

Verification

  • run-python-hook.test.sh13/13 PASS; confirmed it fails against the pre-fix hooks.json.
  • Full disk-hygiene suite (5 files, 250 + 23 tests) — no new failures. Three telemetry-sink timing failures reproduce locally on Windows but are confirmed pre-existing on unmodified origin/main via a detached baseline worktree, and the hygiene lane is green on Linux CI.
  • setup shipped no evals/evals.json, which the skill-quality gate requires once its SKILL.md changes; six cases were added and pass both the schema and check-evals-quality.sh.
  • shellcheck -x (repo .shellcheckrc) clean; typos, markdownlint-cli2, check-hook-userconfig-argv.sh, check-shell-portability.sh, and check-changelog-parity.sh (--check, --check-bump, --check-order) all green.
  • Live corroboration: guardrails' quoted shell-form hooks fired and blocked a command during this session on the same machine — the pattern is demonstrably working here.
  • Version bumped 0.17.70.17.8 with a CHANGELOG entry framing it honestly as a regression.

Docs corrected

Both stated the now-disproved premise, and one was actively harmful:

  • README.md claimed the wired hooks "register as bash … so bash must resolve on PATH".
  • skills/setup/SKILL.md preflight item 1 told operators to reorder their Windows PATH so Git Bash precedes stub paths — a check that passes while the guard is dead, prescribing the workaround rather than the fix. It now verifies the registration shape and explicitly says PATH ordering is not the cause.
  • hooks/run-python-hook.sh's header comment asserted the launcher "is registered as bash (available in Git Bash on Windows)" — the exact false assumption. Rewritten to document shell form and warn against reverting to exec form.

Closes #1416

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W

…d guard (#1416)

Both `hooks/hooks.json` registrations used exec form (`"command": "bash"` +
`args`). Per the hooks reference, exec form resolves `command` as an executable
on PATH with no shell — on Windows `bash` finds the WSL relay
`System32\bash.exe` before Git Bash, and the launch dies with
`execvpe(/bin/bash) failed: No such file or directory`. A failed hook launch is
a non-blocking error, so `destructive_guard.py` never ran: the PreToolUse
destructive-operation gate silently enforced nothing.

0.17.6 introduced this while fixing Python resolution (#1504), reintroducing the
exact launch failure #1006 had already fixed on the skill-frontmatter surface.

Both hooks now name `run-python-hook.sh` directly with `"shell": "bash"` and no
`args`, which Claude Code routes through Git Bash instead of a PATH lookup.
Every path placeholder is double-quoted, so the argv is byte-identical to the
exec-form vector across paths containing spaces. The #1504 Python-resolution
behaviour is unchanged — only the launch mechanism moves.

`run-python-hook.test.sh` asserted `.command == "bash"`, encoding the defect as
the contract; it now asserts the portability property. `test_hygiene.py`
selected guard hooks on `args` alone, which would have gone vacuously green on
any shell-form hook — both helpers are now form-agnostic.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W
@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
@claude

claude Bot commented Aug 13, 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 Aug 13, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@github-actions

github-actions Bot commented Aug 13, 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-run the job, or workflow_dispatch this workflow with the PR number, to retry the review. A new push re-triggers this lane only if the caller's pull_request triggers include synchronize (the canonical caller omits it).
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).

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

ℹ️ 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
The setup skill's preflight told operators to reorder their Windows PATH so
Git Bash precedes stub paths — prescribing the environment workaround instead
of the fix, and passing while the guard was dead. It now verifies the
registration shape and states that PATH order neither causes nor fixes this.

Also corrects the toggle-downgrade paragraph, which claimed both guard surfaces
launch through the literal name `python3`; that is now true only of the
skill-scoped belt.

Editing SKILL.md makes evals/evals.json mandatory per the skill-quality gate;
the setup skill shipped none. Adds six cases covering action routing, the
guidance-only apply path, the exec-form registration failure, the Git Bash
prerequisite, the disabled-toggle FAIL floor, and the Python floor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W
@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
@claude

claude Bot commented Aug 13, 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 Aug 13, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

…2570)

The shell-form conversion left the plugin's own trust-surface record and
safety model asserting the disproved premise: README's trust-surface entry
bounded the plugin-level hook by "exec form (no shell)", and safety-model.md
identified the engine gate as exec form. A security record that misdescribes
the invocation shape makes anyone reasoning about the boundary reason from a
false premise.

Both now assess the actual mechanism rather than swapping the words. "No
shell involved" is gone as the bound; what replaces it is stated with its
limits: the command string is a fixed literal in the plugin's own hooks.json
with no model-, repo-, or session-supplied interpolation, and its only
substituted values are Claude Code's own double-quoted ${CLAUDE_PLUGIN_ROOT}
and ${CLAUDE_PLUGIN_DATA} — argv verified byte-identical to the exec-form
vector for roots containing spaces and backslashes. The record also says what
the quoting does NOT do: the runtime substitutes those placeholders textually
before bash parses the result, so double quotes bound whitespace and
backslashes, not every shell metacharacter. The invariant is therefore
maintained by run-python-hook.test.sh plus the form-agnostic test_hygiene.py
hook helpers rather than being structural — the reason #2569 proposes a
repo-wide gate.

Also corrected in the same sweep, beyond the reported finding:

- skills/clean/SKILL.md's single launch bullet asserted exec form, a bare
  python3 PATH lookup, and a silent non-blocking launch failure for "the
  guard hook" — three claims now false for the wired gate and still true for
  the skill-scoped belt. Split per surface, with the belt's residual pointed
  at #2568.
- skills/clean/SKILL.md's "shell-free exec form" clause in the <hook-python>
  guidance was incidental to that point and false for the gate; dropped.
- safety-model.md claimed the Stop detector shares the guard's literal
  python3 lookup and so leaves that vector unreported. #1504 already closed
  that: the shared launcher tries python3/python/py -3, rejects the
  WindowsApps stub, and emits the systemMessage itself. The residual is now
  correctly stated as the shared launcher and the shell that starts it.
- test_hygiene.py's exec-form docstring names the skill-scoped surface it
  actually asserts.

README.md:51 (skill-scoped guard, exec form) and the historical CHANGELOG
entries are accurate for their subjects and left alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W
@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
No content change — rewraps the belt bullet to the file's line width.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W
@claude

claude Bot commented Aug 13, 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 Aug 13, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
The belt bullet asserted a remedy ("moving it to the shared launcher") that
#2568 does not commit to — the #1014 constraint on skill-hook substitution
argues against assuming the wired-hook shape transfers. A safety doc should
name what is tracked, matching the wording already used in test_hygiene.py.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W
@claude

claude Bot commented Aug 13, 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 Aug 13, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
@github-actions github-actions Bot deleted a comment from claude Bot Aug 13, 2026
@claude

claude Bot commented Aug 13, 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 Aug 13, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@kyle-sexton
kyle-sexton merged commit b08516e into main Aug 13, 2026
38 checks passed
@kyle-sexton
kyle-sexton deleted the fix/disk-hygiene-hook-shell-form branch August 13, 2026 16:38
kyle-sexton added a commit that referenced this pull request Aug 13, 2026
#2572)

## Problem

`plugins/disk-hygiene/skills/clean/SKILL.md`'s frontmatter belt was the
**third and last**
exec-form hook registration, deliberately left unconverted by #2570 (row
3 of that PR's audit
table):

```yaml
- type: command
  command: "python3"
  args: ["${CLAUDE_PLUGIN_ROOT}/skills/clean/scripts/destructive_guard.py", "--plugin-root", "${CLAUDE_PLUGIN_ROOT}"]
  timeout: 60
```

Per the [hooks reference](https://code.claude.com/docs/en/hooks), exec
form (`args` present)
resolves `command` as an executable on `PATH` with no shell, and "On
Windows, exec form requires
`command` to resolve to a real executable such as a `.exe`." On stock
Windows `python3` resolves to
`%LOCALAPPDATA%\Microsoft\WindowsApps\python3.exe` — a **zero-length App
Execution Alias stub**,
not a real executable. The spawn fails, a failed hook launch is
non-blocking, and the belt silently
enforces nothing.

This registration has named a bare interpreter since #215, predating
`hooks/run-python-hook.sh`
(#1504) entirely — the launcher that exists precisely to reject that
stub.

**This one is latent, not dead.** It works wherever `python3` is a real
interpreter, which is the
reporting host. So the risk here is the inverse of #2570's: breaking a
*working* safety guard.
Everything below is scoped to proving that cannot happen.

## Fix

Shell form through the shared launcher — the shape #2570 established, no
third variant:

```yaml
- type: command
  command: '"${CLAUDE_PLUGIN_ROOT}"/hooks/run-python-hook.sh "${CLAUDE_PLUGIN_ROOT}"/skills/clean/scripts/destructive_guard.py --plugin-root "${CLAUDE_PLUGIN_ROOT}"'
  shell: bash
  timeout: 60
```

**On the YAML spelling.** This is the *same value* `hooks.json` carries;
only the file-level escape
differs. JSON needs `\"` escapes, YAML expresses it as a single-quoted
scalar wrapping literal
double quotes. It is not a third shape — `_hook_argv()` tokenizes both
identically.

**Shell form is honored on the skill-frontmatter surface — verified, not
assumed.** #1014 found this
surface more restricted than documented, so this was checked rather than
inferred: `repo-hygiene`'s
`skills/clean/SKILL.md` has shipped a shell-form frontmatter hook with
`shell: bash` since #1006
(`a83d1748`, titled "shell-form hook launch"), and still carries it on
`main` today. A live,
working, same-repo precedent on the exact surface.

**The #1014 constraint is respected.** Verified in code, not from a
summary:
`destructive_guard.py:686-687` resolves `--plugin-root`, rejecting the
literal unexpanded
placeholder, and derives the data root from it via
`_plugin_data_root_from_root`;
`resolve_authorized_data_root`'s own docstring records that "a plugin
`hooks.json` hook can [supply
`${CLAUDE_PLUGIN_DATA}`]; a skill hook cannot." The new command string
therefore substitutes
**only** `${CLAUDE_PLUGIN_ROOT}` — no `${CLAUDE_PLUGIN_DATA}`, no
`${user_config.*}`, and
`--authorized-data-root` is **not** reintroduced. This is asserted by an
existing test that was made
form-agnostic rather than left form-specific.

## Argv equivalence — the load-bearing evidence

**Claim, scoped precisely:** the argv **`destructive_guard.py` itself
receives** is byte-identical
before and after. `argv[0]` necessarily changes — from the interpreter
name `python3` to the
launcher path — because replacing interpreter resolution with the
launcher *is* the fix.

Verified two ways. First with Python's `shlex` (the tokenizer
`test_hygiene.py` uses), then against
**a real bash**, because a claim about how a shell tokenizes should be
proven by a shell. The
launcher path is swapped for an argv dumper so the vector it would
receive is directly observable:

```console
root = C:\Program Files\Claude Code\plug in root
  shell sees: "C:\Program Files\Claude Code\plug in root"/hooks/run-python-hook.sh "C:\Program Files\Claude Code\plug in root"/skills/clean/scripts/destructive_guard.py --plugin-root "C:\Program Files\Claude Code\plug in root"
  argv built by bash (launcher swapped for an argv dumper):
    C:\Program Files\Claude Code\plug in root/skills/clean/scripts/destructive_guard.py
    --plugin-root
    C:\Program Files\Claude Code\plug in root

root = D:\a b\c\d
  argv built by bash (launcher swapped for an argv dumper):
    D:\a b\c\d/skills/clean/scripts/destructive_guard.py
    --plugin-root
    D:\a b\c\d
```

Exactly three tokens after the launcher, matching the exec-form `args`
array element for element,
for a POSIX root and for two Windows roots containing **both spaces and
backslashes**. These are the
same three roots the added test asserts against.

**Why it holds, and why the quoting is required rather than stylistic:**
inside POSIX double quotes a
backslash is literal (it escapes only `$`, `` ` ``, `"`, `\`, newline),
and whitespace does not split.
Remove the double quotes and `C:\Program Files\Claude Code\plug in root`
splits into four argv
entries and the backslashes get eaten — the guard would receive a
truncated `--plugin-root` and lose
its data-root authority. That is exactly why #2570's "double-quote every
placeholder" rule is a
correctness requirement.

## Closest reproducible proxy for "the guard still blocks"

The belt is skill-scoped, so it fires only inside `/disk-hygiene:clean`
and cannot be exercised from
an ordinary session — I am **not** claiming a live in-session
verification of the belt itself.
Instead, the exact new command string was substituted the way Claude
Code substitutes it and driven
end to end:

```console
$ printf '{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"rm -rf /tmp/example"}}' \
    | bash -c '"<root>"/hooks/run-python-hook.sh "<root>"/skills/clean/scripts/destructive_guard.py --plugin-root "<root>"'
{"hookSpecificOutput": {"hookEventName": "PreToolUse", "permissionDecision": "deny",
 "permissionDecisionReason": "Disk-hygiene fails closed: Bash is restricted to exact bundled scan,
 preview, handoff-verify, and apply invocations of ... hygiene.py ..."}}
```

The belt launches through the new string and emits a `deny`. Note for
anyone re-running this: the
belt is **deny-by-default** during active cleanup (it allows only the
exact bundled engine
invocations), so a benign payload such as `echo hi` denies too. Both
denying is the documented
behavior of this surface, not over-blocking introduced here.

Composed with the argv equivalence above, the argument is stronger than
a single live observation:
the guard receives a byte-identical vector, the existing 252-test suite
proves the guard's decisions
*given* that vector, and
`test_skill_hook_launcher_resolves_a_supported_interpreter` proves the
launcher resolves and execs end to end. No behavior change is reachable
— only the launch mechanism
moved.

## What this does NOT fix — stated so it is not overclaimed

`run-python-hook.sh` **exits 0 silently in guard mode** when no
interpreter resolves anywhere on its
ladder (its own test asserts this). So this closes *"the belt cannot
start against the alias stub"*.
It does **not** make the belt fail-closed on a host with no Python at
all — that residual is
unchanged, and is documented as such in `safety-model.md` and the
README.

## Tests — the third bug-as-contract site, and its fix

#2568 named one form-specific helper. There were **three** sites, all of
which encoded the launch
form they were supposed to be checking:

| Site | Old shape | Post-conversion behavior |
| --- | --- | --- |
| `_skill_hook_command_and_args()` | `next(… "args:" in line)` |
**raises `StopIteration`** |
| same helper's `command` read | `.split(":",1)[1].strip().strip('"')` |
silently mis-parses a single-quoted YAML scalar into a string `shlex`
then tokenizes wrongly |
| `test_skill_hook_interpreter_is_python3_and_resolves` |
`assertEqual("python3", interpreter)` | asserts the defect |

The helper is replaced by `_skill_hook()`, which parses the frontmatter
into a **hook mapping** and
feeds it to the *existing* form-agnostic `_hook_argv()` /
`_guard_argv_from_hook()` added by #2570 —
so both surfaces are now asserted through one reading path, in either
form. A `_yaml_flow_scalar()`
helper decodes the three one-line YAML scalar styles rather than
hand-stripping quotes (the suite is
stdlib-only; PyYAML is not a dependency). The interpreter test now
**exercises the launcher's real
resolution ladder** instead of restating it, so the two cannot drift.

**Added — and proven discriminating (requirement 5):**

- `test_skill_hook_registers_in_portable_shell_form` — the same four
portability properties
`run-python-hook.test.sh` asserts for `hooks.json` (launcher named in
`command`, `args` absent,
`shell: bash`, every `${CLAUDE_PLUGIN_*}` double-quoted). That suite is
jq-based and cannot read
YAML frontmatter, which is why this surface is asserted in
`test_hygiene.py`.
**Verified to FAIL against the pre-change frontmatter** (`git stash` of
SKILL.md only):

  ```
  FAIL: test_skill_hook_registers_in_portable_shell_form
  AssertionError: 'hooks/run-python-hook.sh' not found in 'python3' :
    skill hook must launch through the shared launcher: 'python3'
  ```

Note the substitution-allowlist test passes in **both** forms, so it is
explicitly *not* the
  discriminator — its docstring now says so.
- `test_skill_hook_argv_matches_the_exec_form_vector_it_replaced` —
encodes the equivalence proof
above over three roots (POSIX, and two Windows roots with spaces +
backslashes). This one passes
in both forms *by design*: it is the equivalence evidence, not the
regression gate.

## Docs corrected (requirement 6)

Every surface that described this hook's form:

- `README.md` — "Claude Code launches the skill-scoped guard in
shell-free **exec form**"; "the
**wired** hooks resolve Python through `run-python-hook.sh`" (x2, now
all three).
- `skills/clean/reference/safety-model.md` — "(the clean skill's
frontmatter hook, **still exec
form**)"; "The skill-scoped belt is a separate surface and **still
launches in exec form via
`python3`** (#2568)" (sentence's purpose is gone); the "Hook launch
form" section extended from
two hooks to three; "since #1504 **both wired hooks** launch through the
shared launcher".
- `skills/setup/SKILL.md` — "the skill-scoped belt through the **literal
name `python3`**"; preflight
item 1's scope; item 2's "The `clean` guard hook runs the literal
command `python3`".
- `hooks/run-python-hook.sh` header — "Launch disk-hygiene **wired**
hooks"; "**hooks.json** invokes
  this file in SHELL FORM".
- `skills/setup/scripts/python3_alias_probe.py` docstring + operator
message, and
`test_python3_alias_probe.py` docstring — both grounded the check on
"the guard is launched as
  `python3`".

**Also changed — fallout of the conversion, caught in review (thread
from
`chatgpt-codex-connector`, P2):**

`/disk-hygiene:setup check` step 2 verdicts the interpreter **ladder**,
not `python3` alone. My
first pass kept `store-alias-stub` → FAIL and merely re-grounded the
rationale. That was wrong, and
the review found the concrete counter-case: a host with real Python
installed **without "Add to
PATH" but with the `py` launcher** has a stubbed `python3`, a working
`py -3`, and a guard that now
launches on every call — and would have been reported broken and told to
reinstall Python. Before
this PR that FAIL was correct (the belt really did launch as bare
`python3`); the conversion is what
made it a false positive, so fixing it belongs here.

Step 2 now treats the alias probe as **diagnostic input** — it still
exists to classify the first
rung *without executing it*, since a bare `python3 --version` pops the
Store — resolves the ladder
in the launcher's own order skipping stubs, and checks the selected
interpreter against the parsed
`MIN_PYTHON`. This is strictly more accurate in both directions, not a
safety downgrade:

- **FAIL** on an **exhausted** ladder or a below-floor interpreter — the
real fail-open. Both still
  FAIL under a disabled toggle, for the reason already in the file.
- **WARN** when the ladder resolves a supported interpreter but
`python3` is the stub. Guards
launch; the residual is only that a bare `python3` typed by hand still
opens the Store.

The probe's own return values are unchanged (27/27 setup tests still
green) — only the verdict
mapping and the message wording moved.

**Not changed, deliberately:**

- **`safety-model.md`'s "`${CLAUDE_PLUGIN_ROOT}` — the only substitution
a skill-frontmatter hook
receives"** — still true, still the #1014 constraint, and now
load-bearing for this shape.
- **`scripts/check-hook-userconfig-argv.sh`** — audited for the
vacuous-green risk #2570 found. It
scopes to hook *config JSON* only (`hooks/hooks.json`, manifest-pointed
configs, inline manifest
`hooks`), so SKILL.md frontmatter was never in scope and this change
cannot make it silently pass.
A repo-wide gate covering the frontmatter surface is what **#2569**
proposes; not built here.

## Verification

- **`test_hygiene.py` — 252 tests**, 1 failure.
`check-changed-skills.sh` reports 2 script-test
failures for `clean`. All three are the **pre-existing Windows
telemetry-sink timing failures**
#2570 baselined — confirmed identical on **unmodified `origin/main`**
via a detached baseline
worktree (`250 tests, same single failure` in `test_hygiene.py`; `23
tests, same 2 failures` in
`guard_launch_monitor.test.sh`). Count moves 250 → 252 (two added, one
renamed). The `hygiene`
  lane is green on Linux CI.
- `hooks/run-python-hook.test.sh` — **13/13 PASS**.
- `skills/setup/scripts` — 27/27 PASS.
- `markdownlint-cli2` (5 files) — 0 errors. `typos` — clean. `shellcheck
-x` — clean.
`ruff check` — all checks passed (`ruff format` drift in these files is
pre-existing on `main` and
  is not a CI gate).
- `check-changelog-parity.sh --check`, `--check-bump origin/main`,
`--check-order` — all green.
- `check-hook-userconfig-argv.sh`, `check-shell-portability.sh`,
`check-skill-portability.sh`,
  `check-silent-skips.sh`, `check-discriminating-test-skips.sh`,
  `check-manifest-duplicate-keys.py` — all green.
- All **5** `disk-hygiene` test entry scripts run individually:
`run-python-hook.test.sh`,
`kill_switch_probe.test.sh`, `python3_alias_probe.test.sh` PASS;
`guard_launch_monitor.test.sh`
and `hygiene.test.sh` carry only the three baselined failures above. (I
started
`scripts/run-plugin-tests.sh` for the whole repo but it exceeded its
window and was stopped, so
  I am **not** claiming a full-repo local run — CI covers that lane.)
- `machine-specific-paths` initially failed on the first push: the argv
fixtures used
`C:\Users\<name>\…` and `/home/user/…` roots. A correct catch — the
properties under test are
spaces, backslashes, and a drive-letter shape, none of which need a
home-directory spelling.
Fixtures moved to `/opt/claude/plugins/disk-hygiene`, `C:\Program
Files\Claude Code\plug in root`,
and `D:\a b\c\d`, and the evidence above was re-run against those exact
roots.
- Version bumped `0.17.8` → `0.17.9` with a CHANGELOG entry.
- No `lefthook` config exists in this repo (no `lefthook.yml` /
`.lefthook.yml`), so there is no
  such gate to run here.

Closes #2568

## Related

- #2570 — converted the two wired hooks; deliberately deferred this
instance as row 3 of its audit
- #1416 — the wired-hook instance of this defect class
- #1014 — established the skill-hook substitution allowlist this change
stays inside
- #1006 — the original shell-form fix, and the live precedent for shell
form on this surface
- #1504 — introduced `run-python-hook.sh`, which this belt now finally
routes through
- #2569 — proposed repo-wide CI gate for this defect class

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

https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 13, 2026
An exec-form hook (a hook object carrying `args`) resolves `command` as an
executable through PATH, so a bare name is machine-dependent. On Windows
`bash` resolves to the WSL relay bash.exe under System32 and `python3` to a
zero-length WindowsApps App Execution Alias stub; the launch fails, and a
failed hook launch is a non-blocking error, so a PreToolUse guard wired this
way silently enforces nothing.

The class has shipped three times in disk-hygiene alone: #1006 fixed it,
#1504 reintroduced it while fixing Python resolution, #2570 fixed it again.
The claude-config audit checklist carried it as an `error` row throughout — a
checklist a human reads is not a gate.

Add scripts/check-hook-exec-form.sh, modelled on the sibling
check-hook-userconfig-argv.sh gate: same scope rules for hook config JSON
(default hooks/hooks.json, manifest-pointed paths with the out-of-tree trust
boundary, inline manifest hooks object), extended to the skill/agent YAML
frontmatter `hooks:` blocks that gate does not cover — the surface where the
remaining instance lives. `args` presence is the sole exec-form
discriminator, so shell form with a leading bare `bash` (the #2570 fix) is
never flagged.

The rule is the broad one: no path separator in `command` fails, rather than
a denylist of names already known to burn us — a {bash, sh} denylist would
have waved `python3` straight through. `node` is the one allowlisted bare
name, because no Windows shim or alias stub shadows it and both
docs/PLUGIN-PHILOSOPHY.md and the audit checklist name it as THE
Windows-correct exec-form spelling; the array is pinned by a test so growing
it is a visible diff.

Wire it as its own self-test-first CI job and into the ci-status needs graph,
and point the philosophy doc's Hooks row at the mechanical check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W
kyle-sexton added a commit that referenced this pull request Aug 13, 2026
Three review rounds found four ways past the hand-rolled YAML walk this gate
carried: a quoted key, two escape encodings of the same key (`h` then
`\U00000068`), an alias under the key, and a brace inside an ordinary scalar
misread as flow style. Two were fail-open, two were false positives. That is
not four bugs — it is one: "which spellings does YAML permit" has no natural
end, and a gate that misparses is worse than none, because it either blocks
valid frontmatter or waves through the defect it exists to catch. This one had
done both.

Replace the walk with scripts/check-hook-exec-form-frontmatter.py, which hands
the frontmatter to PyYAML. Quoted and escaped keys arrive already decoded by
the scanner; anchors, aliases, and merge keys resolve; flow and block style are
the same document. All four findings go away by construction rather than by
patch, and the fail-closed refusals they forced go with them — the reader now
refuses only frontmatter that genuinely does not parse.

The JSON surface keeps jq, which parses that format completely. YAML has no
jq, so this surface takes a dependency instead of a parser we would keep
patching. pyyaml is pinned and hash-locked beside every other Python pin, and
resolved exactly the way scripts/run-ruff.sh resolves ruff: a python that can
already import it, else `uv run --with pyyaml==<pin>` reading the same pin,
else exit 1. A missing module never becomes a silent pass over 997 files.

The rule, the allowlist, the JSON reader, and the messages are unchanged: both
readers only report exec-form hooks, and the shell gate decides.

Suite is 54 cases. Against the pre-#2570 tree the gate now names all three
historical instances — both wired hooks and the skill-frontmatter one — and
against main it is clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W
kyle-sexton added a commit that referenced this pull request Aug 13, 2026
)

## Summary

An exec-form hook — a hook object carrying `args` — resolves `command`
as an **executable through PATH**, not as a shell command line. A bare
name is therefore machine-dependent, and on Windows two spellings
resolve to something that is not the interpreter the author meant:
`bash`/`sh` hit the WSL relay `bash.exe` under `System32` (dying with
`execvpe(/bin/bash) failed` when no distro provides `/bin/bash`), and
`python`/`python3`/`py` hit the zero-length WindowsApps App Execution
Alias stub. A failed hook launch is a **non-blocking** error, so a
`PreToolUse` guard wired this way silently enforces nothing.

This class has now shipped **three times in `disk-hygiene` alone**:
#1006 fixed it, #1504 reintroduced it while fixing a different (Python
resolution) bug, #2570 fixed it again — and #1416 was closed `COMPLETED`
while the guard stayed dead through 73 recorded runs, every one a
`hook_non_blocking_error`.
`plugins/claude-config/skills/audit/reference/audit-checklist.md`
Category D has carried this as an `error` row the whole time. A
checklist a human reads is not a gate; nothing structural prevented
reintroduction. This PR is that structure.

### What lands

`scripts/check-hook-exec-form.sh` +
`scripts/check-hook-exec-form.test.sh`, modelled directly on the sibling
`check-hook-userconfig-argv.sh` gate — same `cd`-to-repo-root shape,
same scope rules for hook config JSON, same out-of-tree manifest-path
trust boundary with a visible skip, same self-test-first CI wiring, same
failure-output style.

**Coverage — both declaration surfaces**, because the defect has
appeared in each:

| Surface | Covered |
|---|---|
| `plugins/*/hooks/hooks.json` (default location) | yes |
| Manifest-pointed hook config (`hooks` as a string, or an array of
paths) | yes |
| Inline manifest `hooks` object | yes |
| Skill / agent YAML frontmatter `hooks:` blocks | **yes — new**; the
userconfig-argv gate does not cover this, and it is where the third
instance lived (#2568) |

`args` presence is the **sole** exec-form discriminator. The command
*string* is never searched for interpreter names, so the #2570 fix —
shell form with a leading bare `bash` and `shell: bash`, exactly as
`plugins/repo-hygiene/skills/clean/SKILL.md` now carries it — is not
flagged. That is a named test case, not an accident.

### Two readers, one rule — and why the YAML one is a real parser

The JSON surface is read with `jq`, which parses that format completely.
The frontmatter surface is read by
`scripts/check-hook-exec-form-frontmatter.py`, which hands the document
to **PyYAML**. Both readers only *report* exec-form hooks; the shell
gate owns the rule, so one implementation governs both surfaces.

That split was not the starting point. The frontmatter surface began as
a hand-rolled `awk` walk over block-style YAML, and review found **four
ways past it in three rounds** — a quoted key `"hooks":`, two escape
encodings of the same key (`"hooks":`, then `"\U00000068ooks":`), an
alias under the key, and a brace inside an ordinary scalar misread as
flow style. Two were fail-open, two were false positives. Each fix
enlarged the parser and invited the next case, because *"which spellings
does YAML permit"* has no natural end — and a gate that misparses is
worse than no gate, since it either blocks valid frontmatter or waves
through the very defect it exists to catch. This one had done both.

PyYAML settles the class by construction: quoted and escaped keys arrive
already decoded by the scanner, anchors and aliases and merge keys
resolve, flow and block style are the same document. The fail-closed
refusals those rounds forced went away with it — the reader now refuses
only frontmatter that genuinely does not parse.

The dependency is handled the way this repo already handles a pinned
tool: `pyyaml` is hash-locked in `.github/requirements-ci.txt` beside
every other Python pin, and resolved exactly as `scripts/run-ruff.sh`
resolves ruff — a python that can already import it (the CI path), else
`uv run --with pyyaml==<pin>` reading that same pin (the cross-platform
local path, no global install and no virtualenv ceremony), else exit 1.
A missing module never becomes a silent pass over 997 files.

Scope narrowing that came with it: `hooks` is read only as a
**top-level** frontmatter key, the one position Claude Code loads a
skill or agent hook from. A `hooks:` mapping nested under another key is
data, not a registration.

Relationship to the sibling gate: complementary, non-overlapping.
`check-hook-userconfig-argv.sh` constrains **whether** a
`${user_config.*}` token may appear in a hook config; this gate
constrains **what shape `command` takes** once a hook is in exec form.
Neither subsumes the other.

### The rule, and why this one

**Broad rule, not a denylist:** an exec-form `command` containing no
path separator (`/` or `\`) fails.

A denylist of known-problematic names can only ever hold the spellings
someone was already burned by — and that is *precisely* this defect's
history. A `{bash, sh}` denylist written after #1006 would have waved
`python3` straight through, which is exactly what #2568 is. The failure
is not "these particular names are bad"; it is "a bare `command` is a
PATH lookup whose resolution is a property of the machine, and CI cannot
see the machine". The broad rule names the actual mechanism.

Nothing legitimate is rejected. The tree contains **zero** exec-form
hooks in any `hooks.json` or manifest today, and the alternatives cost
nothing: `${CLAUDE_PLUGIN_ROOT}`-rooted or absolute paths carry a
separator and pass untouched, and shell form has no `args` at all and is
never inspected.

**One allowlisted bare name: `node`.** Not a judgement call about "real
executables on every platform" — that unverifiable judgement is what
failed three times. The admission criterion is mechanical: *a name
qualifies only when no Windows shim, relay, or App Execution Alias stub
shadows it on PATH ahead of the real interpreter.* `bash`/`sh` fail it
(WSL relay). `python`/`python3`/`py` fail it (WindowsApps stubs). `node`
passes — `node.exe` is the only resolution — which is why both
`docs/PLUGIN-PHILOSOPHY.md` (Hooks row) and the claude-config audit
checklist already name `"command": "node", "args": [...]` as **the**
Windows-correct exec-form spelling. Allowlisting it keeps the gate
agreeing with the repo's own documented guidance instead of
contradicting it; a gate that forbids what the philosophy doc recommends
does not get obeyed, it gets exempted. The array is a bash literal in
the script (not a data file) and is **pinned by a test**, so growing it
is a visible code+test diff, never a quiet one-word edit.

### Deviation from the issue's proposal — stated, not silent

The issue proposed *"a documented escape-hatch allowlist file that can
only shrink"* for **file paths**, mirroring
`scripts/hook-userconfig-argv-allowlist.txt`. **This PR does not ship
that file.** Reasoning:

- The sibling gate has a path allowlist because a *sanctioned* use is
foreseeable there — a ratified channel D (`required:true` + argv, no
unset case) adoption. Here no legitimate case exists: shell form and a
rooted path are always available at zero cost, so a path exemption could
only ever grandfather debt.
- A file-path hatch for *this* class is the artifact that would have
kept `bash` alive across #1006#1504. For a defect whose signature is
"the guard is silently off", the escape hatch and the bug are the same
object.
- The name allowlist above already carries the one real exception, with
a mechanical admission criterion CI can restate.

If a genuine case ever appears, adding the file is a small, reviewed
change modelled on the sibling (including its stale-entry guard). I
would rather add it against evidence than ship it empty.

### Also

- New CI job `hook-exec-form-gate` (self-test first, then the gate),
added to the `ci-status` `needs` graph — a gate that is not a required
check is the #1416 failure mode again.
- `docs/PLUGIN-PHILOSOPHY.md` Hooks row now names the failing spellings
and points at the mechanical check, in the repo's own "prose states it
but cannot self-verify" idiom. No behavioural doc change — `"command":
"node"` remains correct, which is the payoff of allowlisting it.

## Test plan

### Red — the gate fails against the shapes that actually shipped

A gate never demonstrated red is not a gate. `3a51996c` is the commit
immediately before #2570 landed, so its tree carries **all three**
historical instances of this defect: the two wired hooks in
`hooks/hooks.json` (both `"command": "bash"` + `args`) and the
skill-frontmatter one (`"command": "python3"` + `args`, which #2568
owned). Reconstruct them into a fixture tree and run the gate:

```bash
t=$(mktemp -d)
mkdir -p "$t/scripts" "$t/.github" "$t/plugins/disk-hygiene/hooks" "$t/plugins/disk-hygiene/skills/clean"
cp scripts/check-hook-exec-form.sh scripts/check-hook-exec-form-frontmatter.py "$t/scripts/"
cp .github/requirements-ci.txt "$t/.github/"
git show 3a51996:plugins/disk-hygiene/hooks/hooks.json > "$t/plugins/disk-hygiene/hooks/hooks.json"
git show 3a51996:plugins/disk-hygiene/skills/clean/SKILL.md > "$t/plugins/disk-hygiene/skills/clean/SKILL.md"
(cd "$t" && bash scripts/check-hook-exec-form.sh; echo "exit=$?")
```

```text
EXEC-FORM HOOK: plugins/disk-hygiene/hooks/hooks.json:.hooks.PreToolUse[0].hooks[0]: exec-form hook (`args` present) with bare command "bash"
EXEC-FORM HOOK: plugins/disk-hygiene/hooks/hooks.json:.hooks.Stop[0].hooks[0]: exec-form hook (`args` present) with bare command "bash"
EXEC-FORM HOOK: plugins/disk-hygiene/skills/clean/SKILL.md:11: exec-form hook (`args` present) with bare command "python3"
exit=1
```

Every instance is named, on both surfaces, with a resolvable location.
Note the JSON paths: the pre-#2570 `hooks` value is an **event-keyed
object**, not an array, so a shape-specific walk would have found one
entry and missed the other.

### Green — the current tree

```console
$ bash scripts/check-hook-exec-form.sh
No exec-form hooks with a bare command name.
exit=0
```

Clean across every JSON surface and all 997 markdown files under
`plugins/`.

This PR was **red by construction** until #2568 landed, on exactly one
file — `plugins/disk-hygiene/skills/clean/SKILL.md:11` — and it
deliberately never touched that file, grandfathered it in a baseline, or
added a path allowlist to clear itself. #2568's fix (PR #2572,
`be72131e`) removed the last instance; this branch is rebased on top of
it. That ordering is the point rather than an inconvenience: this
class's entire history is a guard being switched off with a
plausible-looking justification attached, and a gate that exempts its
own last violation to go green is that same move.

The shape #2572 landed is pinned as a case here too, so the gate can
never start rejecting the fix that unblocked it. So is the
must-stay-green case from #2570:
`plugins/repo-hygiene/skills/clean/SKILL.md` carries shell form with a
leading bare `bash`, and the gate would be **wrong** to flag it — `args`
presence is the sole exec-form discriminator, and the command *string*
is never searched for interpreter names.

### Self-test

`scripts/check-hook-exec-form.test.sh` — 57 cases, all passing locally
and in CI. Fixture-tree pattern copied from the sibling gate's suite.
Coverage:

- the pre-#2570 shape fails and names **both** wired entries; the #2570
shell-form fix passes on both surfaces; the #2572 shape that unblocked
this PR passes
- rooted `${CLAUDE_PLUGIN_ROOT}` command passes; absolute Windows path
passes
- `node` passes; `bash`, `sh`, `python`, `python3`, `py`, `pwsh`, `deno`
each fail; allowlist contents pinned so growing it is a visible
code+test diff
- `args: []` (present but empty) is still exec form
- a `matcher` entry is not itself a hook object; one clean + one dirty
sibling flags only the dirty one
- manifest string-path, manifest array, inline manifest object;
out-of-tree manifest path skipped visibly; unreferenced `hooks/*.json`
not scanned; unparsable manifest does not crash the gate; an MCP
`command`/`args` pair outside the `hooks` key is out of scope
- **every YAML spelling of the key** is one declaration: `hooks:`,
`"hooks":`, `'hooks':`, `hooks :`, `"hooks" :`, `"hooks":`,
`"\U00000068ooks":`, `"\x68ooks":` — each was a live bypass of the walk
this gate used to carry
- **YAML the old walk refused or misread is now simply read**:
flow-style mappings, a whole declaration on the key line, an alias under
the key resolved to its anchor, an anchor in value position, a merge key
expanded (with the explicit-key-wins precedence pinned separately), a
block-scalar `command`, a trailing comment on the key, and braces inside
an ordinary `args` scalar (that last one had been red-lining valid
frontmatter)
- **reported, never resolved by preference**: a duplicate top-level
`hooks` key, a duplicate key inside the hooks declaration, or one
arriving through a `<<` merge. PyYAML keeps the last value and js-yaml
rejects the document, so what would actually run is ambiguous — and a
gate premised on "do not assume how this resolves on the target machine"
must not resolve it toward the reading that clears the file. Not
theoretical: #1492 shipped a duplicate manifest key through a fully
green suite.
- **fail-closed** on the one thing a parser still cannot clear:
unparsable YAML frontmatter, and unparsable hook config JSON
- frontmatter hygiene: line-accurate reporting, block-sequence `args`,
unquoted value with a trailing comment, CRLF, agent frontmatter, a
`hooks:` block in the prose body or a fenced example is not a
declaration, a `hooks:` mapping nested under another key is not a
declaration, and nothing outside the `hooks` key is ever judged (a
folded `description:`, an `&anchor`, and a `<<:` merge in ordinary
frontmatter stay inert)
- a clean tree passes with an explicit positive statement

### Repo gates run locally

`shellcheck` (clean), `shfmt -d` (clean), `actionlint` (clean), `typos`
(clean), `scripts/run-ruff.sh check` on the new Python (clean),
`scripts/check-shell-portability.sh --paths` on both shell files
(clean), `check-silent-skips.sh`, `check-discriminating-test-skips.sh`,
`check-changelog-parity.sh --check`, `check-contract-slice-prune.sh
--check`, `check-orphaned-fixtures.sh --check`,
`check-cross-plugin-source-drift.sh --check`,
`check-contract-clause-coverage.py`, `check-manifest-duplicate-keys.py`.
All three new scripts committed mode `100755`. No `--no-verify`.

### Review

Four rounds from `chatgpt-codex-connector`, seven findings, all
addressed and resolved. Six were code changes; one — "fix the existing
violation or defer requiring the gate" — was answered rather than
applied, because deferring the `ci-status.needs` wiring reproduces the
#1416 shape this PR exists to end. Rounds 2 and 3 are what motivated
replacing the hand-rolled YAML walk with a real parser (three of those
findings were bypasses of a parser that was chasing YAML's spelling
rules); round 4 found the duplicate-key ambiguity above. That reasoning
is in the resolved threads and in the reader's own doc-block.

## Related

- Closes #2569
- #2568 (PR #2572) — was the one remaining violation this gate reported;
landed as `be72131e`, and this branch is rebased on top of it
- #1416 — the wired-hook instance the gate would have caught, closed
`COMPLETED` while the guard was dead
- #1006 — the original fix for this class
- #1504 — the reintroduction
- #2570 — the fix this gate pins

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 14, 2026
… Stop audit (#2580)

## Problem

Claude Code records a hook that fails to launch only as a
`hook_non_blocking_error` transcript attachment — the guarded tool call
proceeds as if approved, and nobody is told. The #1416#2570#2572#2571 chain fixed the disk-hygiene instances and gated the source shape,
but the fleet's only silent-failure detector (`disk-hygiene`'s
`guard_launch_monitor.py`) lives **inside the plugin it watches** and
launches **through the same registration form it watches**. Full-fleet
transcript mining on the incident host (97 transcript files, all
projects) shows what that coupling costs:

| hook | failures | stderr |
| --- | --- | --- |
| `PreToolUse:Bash` — `destructive_guard.py` | 95 | `execvpe(/bin/bash)
failed` (WSL relay) |
| `PreToolUse:PowerShell` — `destructive_guard.py` | 45 | same |
| `Stop` — `guard_launch_monitor.py` (the detector itself) | 23 | same |

And the stale-session window no source-side gate can reach: hook config
loads at session start, so a session running when the #2570 fix landed
on disk (2026-08-13T21:19:56Z) kept executing the dead exec-form config
— **22 further failures after the fix shipped**, latest
2026-08-14T03:53Z, guard and detector both dead, zero operator-visible
signal.

## Fix

`hook-failure-audit.sh`, an eighth `claude-ops` `*-audit` hook,
registered on `Stop`:

- **Decoupled by construction:** lives in a plugin whose hook
registrations have been shell-form `"${CLAUDE_PLUGIN_ROOT}"/hooks/*.sh`
throughout — alive during the entire incident, including the
stale-session window. A defect that kills a watched plugin's launch path
cannot take this detector with it.
- **Bounded cost:** `Stop` cadence (once per turn, per
`guard_launch_monitor.py`'s ADR 0004 / D-12 rationale), transcript-tail
read capped at 2 MB with the truncated first line dropped — O(cap), not
O(session length).
- **Structural matching, never substring:** a record counts only when
top-level `.type == "attachment"` and `.attachment.type ==
"hook_non_blocking_error"`. A `hook_success` whose stdout quotes an
error, and a message record quoting a failure record as a string — both
false-positive shapes hit while mining the incident transcripts — cannot
fire it (both pinned in the contract test).
- **Once per session per distinct failing hook,** re-warning when a
*new* hook starts failing; marker bookkeeping under
`${CLAUDE_PLUGIN_DATA}` degrades toward re-warning, never toward
silence.
- **`systemMessage`** names the failing hooks, counts, a stderr snippet,
the fail-open consequence, and the restart-to-reload remedy for the
stale-session case; **telemetry envelope** carries privacy-safe subjects
(hook names only).
- Advisory: always exit 0; fail-open jq gate with the standard skip
notice; kill switch `hook_failure_audit_enabled`.

Overlap with `guard_launch_monitor.py` is deliberate: that monitor keeps
its guard-specific semantics; a destructive-guard failure may warn
twice. Its own doc block names over-warning as the safe direction for
this class.

## Red-first evidence

`hook-failure-audit.test.sh` was written and run **before** the hook
existed:

```text
FAIL: failure surfaced -> exit 0: expected exit 0, got 127
FAIL: names the dead hook: 'PreToolUse:Bash' not in: bash: .../hook-failure-audit.sh: No such file or directory
...
PASS=4 FAIL=20
```

With the hook in place: `PASS=28 FAIL=0`. The core fixture is a
structural copy of a real incident attachment record (WSL-relay stderr,
session `ac1c95e3`).

## Gates

- `hook-failure-audit.test.sh` — PASS=28 FAIL=0
- `shellcheck` both new files — clean
- `check-hook-exec-form.sh` — clean (the new registration is shell form)
- `check-changelog-parity.sh --check / --check-bump origin/main /
--check-order` — clean (0.31.14 → 0.32.0)
- `check-silent-skips.sh`, `check-hook-userconfig-argv.sh`,
`check-manifest-duplicate-keys.py`, `check-cross-plugin-source-drift.sh`
— clean
- `check-shell-portability.sh origin/main` — clean
- `typos` on all touched files — clean
- lefthook pre-commit suite — passed on commit

## Related

Closes #2577

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

https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

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


Security review skipped

  • Gather context
  • Check skip gate

Per /review:security-review's skip gate: PR #2570 is already merged (not open), so no deep review was performed and no findings are posted.

@github-actions

Copy link
Copy Markdown

Last security-reviewed head: 66757962f1e56993974edf71afa114f0bbbe3f2e. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown

Claude encountered an error —— View job


I'll analyze this and get back to you.

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: the destructive-operation guard has never launched — 73 recorded runs, all hook_non_blocking_error

1 participant