fix(hooks): PreToolUse guard remediation — scratch-root default, kill-switch hoist, MCP coverage, two conventions

[kyle-sexton]

Closes #3719

Summary

The 17 PreToolUse guards had never been audited against the current model generation or the current
hooks reference. This ships the actionable half of that audit: the one guard measurably misfiring,
the kill switches that were not saving what they promised, two missing one-line visibility fields, a
content-guard blind spot on the GitHub MCP write tools, and the two policy questions the work had to
settle in order to proceed.

The audit's remaining ablation verdicts are deliberately not acted on here, because they rest on
class reasoning with no firing data. The audit itself lived in the topic's contract slice, pruned
before merge per the topic-docs convention — see Contract slice under Related for the pointer.

One deliberate departure from the topic's locked Brief, stated up front. Its first criterion named
the memory tier (.work/) alongside the harness scratchpad as a shipped block-hook-bypass
exemption. Review showed that exempting the memory tier reopens a content-guard bypass — see Fix 1 —
so it is withdrawn and the criterion is not met by the letter. A criterion that turns out to
introduce a bypass is not one to satisfy as written; the consequence and the routing are recorded
below and in the hook itself.

Fix

1. block-hook-bypass ships one scratch root exempt instead of none (guardrails 0.32.0).
block_hook_bypass_scratch_roots was opt-in and shipped empty, which left the host temp trees —
where the harness's own per-session scratchpad sits — blocked. Measured at five blocks in one day
across four sessions with zero true positives. ADR 0003 clause 4 makes that a wrong SCOPE rather than
a wrong oracle, so the oracle is untouched.

Exempting the temp tree gives up no protection, which is the only reason a default is defensible:
hook::read_file_path, the entry every content guard reads its file through, already declines a
temp-tree file when the project root lies outside that tree. The exemption's width is exactly the
width of the protection it is scoped out of. It is gated on CLAUDE_PROJECT_DIR naming a project
root outside the temp tree, so a temp-rooted project (this repo's own mktemp -d fixtures) still
blocks, and it resolves at run time because the scratchpad path carries a session id.

The memory tier is deliberately NOT a second default. It was, and two reviewers between them
showed why it cannot be: the "gives up no protection" argument does not carry, because
secret-pattern-detection scans a Write to .work/notes.md today (verified: exit 2 on a
PAT-shaped token). Exempting Bash redirects there would let printf '<secret>' >> .work/notes.md
reach disk unscanned while the identical Write stayed blocked — the same bypass Fix 4 closes on the
MCP lane. The safer remedy was taken: withdraw the default rather than widen secret-pattern-detection
on an argument from convention with no firing evidence (ADR 0003). Consequence:
printf '*' >> .work/.gitignore still blocks; that is session-flow's own documented procedure, so
the conflict routes to the skill (#3720 — use Write, which is scanned). The pre-existing asymmetry
between the two content guards on the tier is filed as #3730.

The default is confirmed through symlink resolution before it grants. The lexical compare
alone would exempt a redirect on its spelling, so a symlink under a temp root pointing into the
repository let echo <secret> > /tmp/to-repo/tracked.py through while the direct path blocked
(reproduced, then fixed). Configured roots keep their documented lexical residual because the
"operator naming a root accepts its contents" ground still holds there; a shipped default has no
operator, so it resolves the target or its nearest existing ancestor and re-checks containment. Cost
stays on the grant path. Residual recorded: the check inherits the axis's case-folding, so a symlink
whose real spelling carries capitals is not resolved on a case-sensitive filesystem.

Also resolves a relative redirect target against the payload's .cwd, which the axis previously
refused outright. Still refused when the command carries a cd/pushd/popd or the payload names
no absolute cwd, so the refusal set only shrinks.

2. Every PreToolUse kill switch is read before the library it costs. All 15 shell-implemented
guards read <name>_enabled through hook::check_enabled, which only exists once the 2,684-line
hook-utils.sh is sourced, so a disabled guard parsed the whole library before finding out it had
nothing to do. scripts/check-killswitch-hoist.sh is a new CI gate that fails a guard reversing the
order, with its own suite.

The saving is uneven and the split is stated rather than averaged: four guards run as their own
process and recover ~3.5 ms of ~5.3 ms; the other eleven are sourced into one process by
run-guards.sh, which has already loaded the library, so their hoist buys ~0.05 ms. They are held
to the same rule anyway — one shape is what makes it checkable, and each is invoked standalone by
its own contract test. Inlining trades one definition for fifteen copies, so the gate pins them to
hook::is_enabled's own reading of the env var and fails if that helper changes.

3. Two one-line visibility fields. statusMessage on both disk-hygiene PreToolUse rows (its
Stop row already had one), and a top-level description on 19 of 20 hooks.json files, which all
omitted it. plugins/context-guard/hooks/hooks.json is deliberately excluded — assigned to the
prompt-hooks lane, deferred rather than stalling this PR.

4. The content guards cover the GitHub MCP write lane. secret-pattern-detection and
hardcoded-path-check matched Write|Edit|NotebookEdit only, so a session could be cleared by both
and still push the same secret through mcp__github__push_files or
mcp__github__create_or_update_file — a route with no local file to fix afterwards and no
pre-commit layer. Registered as a NEW hooks.json row rather than by widening the always-on
matcher, so the write path pays nothing for tools it will never see.

mcp__github__delete_file is deliberately not covered: its schema carries no content, so there is
nothing for a content guard to scan and a delete cannot introduce a secret. Three local-only gates
are not applied on the new lane (project scope, git-working-tree, git check-ignore), each because
it is a statement about a local file; applying the scope guard in particular would have skipped
every MCP write, which is a silent hole rather than a scope. Both directions are pinned by tests.

5. Two conventions. ADR 0028 records the A/B/C hook-packaging taxonomy, which forecloses a
fleet-wide "split hooks out of every plugin" that was believed to be policy and turns out to be
unwritten — twelve of the twenty hook-carrying plugins would become empty shells.
docs/conventions/hook-input-rewriting/ records that a PreToolUse hook denies or asks and never
pairs allow with updatedInput.

6. Nineteen plugin patch bumps. The changelog-parity gate refuses a change set that touches a
plugin's shipped files while reusing its published version. The description sweep touched sixteen
plugins for one line each, disk-hygiene for that plus two statusMessages, source-control for
that plus three hoisted kill switches, and context-guard for one hoisted kill switch. Each carries
a patch bump and a release entry naming which of those it was.

Verification

scripts/affected-tests.sh --run on the pruned head selects the shell suites and exits 3, the
documented code for "non-shell suites selected but not run" — the run is recorded in the final
commit-time check below. On the previous head it was 146 shell suites passed, 0 failures. Nine of
the ten non-shell suites were run from their own lanes and pass — eight Python suites and the
autonomy .mjs suite (544 checks); none of their inputs changed since.

One suite could not be run and is not claimed as passing.
plugins/machine-health/.../Test-ClaudeTempRoot.Tests.ps1 needs Pester, which is not installed in
this container, and the repo has no Pester CI lane. It is a transitive selection: this PR touches no
machine-health file and no changed file references it.

The lint lane. It was red on every earlier head; six sub-gates reduced to three causes, each
reproduced locally and fixed: the nineteen missing bumps (Fix 6), five shellcheck notes at the
rcfile's stricter-than-warning severity (all literal single-quoted text, each now carrying a
directive with the reason at the site), one GNU-only sed -i in the hoist suite (rewritten through a
sibling file), and the docs/topics/ slice — which is where the typos and markdown failures
lived and which the contract-slice gate requires pruned. All four changelog-parity modes,
shell-portability against origin/main, shellcheck at rcfile severity, markdownlint and typos over
every changed document, and contract-slice-prune --check-diff are clean locally.

Per-change evidence:

• Repro-first, as the hook-precision convention requires. The stay-quiet block-hook-bypass
  assertions were written first and FAILED against the unmodified hook (expected 0, got 2); the
  adversarial-floor assertions beside them passed before and after. Both review findings on this
  guard were likewise reproduced before being fixed, and each fix is pinned by a discriminating test
  that fails with the fix neutered. Suite 591 → 602, identical with CLAUDE_PROJECT_DIR set in the
  ambient environment, because the run helper now pins it.
• A regression caught by measurement, not by tests. Both content guards ask for
  .tool_input.path, and run-guards.sh's cached hook::jq_fields is all-or-nothing per call — one
  unserved filter sends the whole call to an uncached jq. That cost two extra spawns on every
  Write/Edit, 50 ms → 60 ms, and no suite noticed. Fixed by priming the field: 51 ms → 52 ms, median
  of three 30-run batches per tree. The new lane's own cost (47 ms single-file, 72 ms two-file push)
  is stated in the plugin README per .claude/rules/hook-budget.md.
• Guard suites: secret-pattern-detection 68 → 86, hardcoded-path-check 100 → 118, all 15
  hoisted guards pass, check-killswitch-hoist.test.sh 14/14 including a reversed-order fixture
  that fails the gate and a mutated hook::is_enabled that trips its semantic pin.
• Docs claims checked against the live hooks reference rather than the audit notes, which caught
  two: the note that permissionDecisionReason is hidden from Claude on an ask is contradicted by
  the current reference, and defer is not among the listed permissionDecision values. Neither
  claim is made.
• validate-plugin-contracts.mjs checks 3,123 plugin files clean.

origin/main is merged in. It carried a typos-format spawn-count fix whose absence made that suite
fail at this branch's base — pre-existing, confirmed at the merge-base, and resolved by the merge
(144/0).

The locked Brief (PLAN.md, verbatim at the pre-prune commit)

TLDR

Act on the PreToolUse guard audit recorded in FINDINGS.md: fix the one guard measurably misfiring,
make the kill switches save what they promise, close two visibility gaps, close the MCP coverage
hole, and codify the two conventions the interview established. One draft PR carrying six changes.

Goal

The audit classified 17 PreToolUse guards and found that its ablation verdicts were the least
actionable part of it. The actionable findings were a guard firing four times with zero true
positives, a kill switch recovering 15 percent of a disabled hook's cost instead of 75, two
missing one-line visibility fields, and a content-guard blind spot on the GitHub MCP write
tools. This task ships those four, plus the ADR and convention that stop the next author
re-litigating the two policy questions the interview settled.

Constraints

• Alpha repo, single known consumer. No deprecation shims, no compatibility aliases, no
  migration paths. Breaking changes are acceptable and expected.
• Configurability is non-negotiable. Every guard keeps its *_enabled userConfig boolean,
  read through the CLAUDE_PLUGIN_OPTION_* hook mirror. The operator runs this marketplace
  across several projects and at work; per-hook control is the reason the hoist matters.
• Cross-lane files are announced before they are touched. plugins/*/hooks/hooks.json is
  contended across at least three sessions, and plugins/context-guard/hooks/hooks.json is
  already assigned to the prompt-hooks lane. A contested file defers to a follow-up rather than
  stalling this PR.
• lib/hook-utils.sh and its 17 vendored copies stay untouched. Contended, and a possible
  split of that file is a separate question.
• Validate with scripts/affected-tests.sh --run, never the full suite. A changed file mapping
  to zero suites is an error, not "nothing to run".
• The PR opens as a draft and flips to ready when the work is done. The body satisfies
  .claude/rules/pr-body-contract.md before creation, not after.

Acceptance criteria

1. block_hook_bypass_scratch_roots has a non-empty default covering the harness-designated
   scratchpad and .work/, and block-hook-bypass.test.sh carries a MUST-stay-quiet case that
   FAILS against the unmodified hook and passes after the change.
   (Departed from in review: .work/ withdrawn — see Summary.)
2. All 17 PreToolUse guard scripts read their *_enabled switch before sourcing any library,
   and scripts/check-killswitch-hoist.sh fails on a script that reverses that order.
3. The two plugins/disk-hygiene/hooks/hooks.json PreToolUse rows carry a statusMessage, and
   every uncontested plugins/*/hooks/hooks.json carries a one-line top-level description.
4. secret-pattern-detection and hardcoded-path-check inspect the content written by
   mcp__github__push_files, mcp__github__create_or_update_file and
   mcp__github__delete_file, with contract tests covering each payload shape.
   (delete_file carries no content; recorded as deliberately not covered.)
5. An ADR records the A/B/C hook-packaging taxonomy, and PLUGIN-PHILOSOPHY.md carries a
   pointer row to it.
6. docs/conventions/hook-input-rewriting/README.md states the deny-or-ask posture, names
   updatedInput paired with "ask" as the sanctioned escape hatch, and gives the principle of
   least astonishment as the basis.
7. scripts/affected-tests.sh --run passes and selects a non-empty suite set for changes 1
   through 4.

Captured assumptions

• Placement follows repo precedent, not explicit instruction. ADR 0019 is the packaging
  analogue, and convention docs are owner docs for ongoing disciplines that point up to the
  philosophy. So the taxonomy is an ADR and the rewrite posture is a convention.
• The description text for each hooks.json is the implementer's to draft, one line per
  plugin, describing what that plugin's hook set does.
• The hoist is behavior-preserving for enabled guards. Only the disabled path changes cost.
  Any guard whose kill switch is read after a library that the switch check itself depends on is
  an exception to be surfaced, not silently reordered.
• The A/B/C classification is settled: Class A is 12 plugins where hooks are the plugin,
  Class B is autonomy, context-guard and disk-hygiene where the hook is the feature mechanism,
  Class C is the 5 adjunct-to-skills candidates.

Out of scope

• Executing any Class C plugin split (filed as five issues).
• The ADR 0003 corpus sweep on block-hook-bypass (filed as one issue).
• The PostToolUse half of the kill-switch hoist (sibling lane claude/posttool-hooks-review-ji6rl5).
• Any change to docs/conventions/hook-budget/ (routes to guardrails dispatcher is the whole per-tool-call budget miss after the hook-performance program #3685).
• The remaining ablation verdicts, which need per-guard evidence before any of them moves.

Related

• Contract slice. docs/topics/claude-pretool-validation-hooks-84d7ka/ (PLAN.md, the locked
  Brief above, and FINDINGS.md, the 484-line audit) was pruned in the final commit per the
  topic-docs convention. Pre-prune commit, for best-effort retrieval while GitHub retains the
  object: 19318ff1ba9c5af12a93f34332f0468004c12c27 —
  gh api "repos/melodic-software/claude-code-plugins/contents/docs/topics/claude-pretool-validation-hooks-84d7ka/FINDINGS.md?ref=19318ff1ba9c5af12a93f34332f0468004c12c27".
  Durable outcomes graduated to ADR 0028, docs/conventions/hook-input-rewriting/, and the issues
  below.
• Refs secret-pattern-detection scans the memory tier, which the topic-docs convention designates for raw credentials #3730 — secret-pattern-detection scans the memory tier the topic-docs convention designates
  for raw credentials; the asymmetry review surfaced, filed rather than widened without evidence
• Refs Run the ADR 0003 corpus sweep on block-hook-bypass's remaining default-on scope #3721 — the ADR 0003 corpus sweep on block-hook-bypass's remaining default-on scope, which
  this PR deliberately does not answer
• Refs session-flow prescribes a write command guardrails blocks #3720 — session-flow prescribes a write command guardrails blocks; with the memory-tier
  default withdrawn this is now squarely the skill's to fix
• Refs Evaluate splitting session-flow's SessionStart hook from its 14 skills #3722, Evaluate splitting claude-ops's nine observability hooks from its 12 skills #3723, Evaluate splitting source-control's five PR and worktree gates from its seven skills #3724, Evaluate splitting instruction-placement's index-drift hook from its five skills #3725, Evaluate splitting context-budget's settings-write-ask hook from its two skills #3726 — the five Class C split candidates ADR 0028 gates. The ADR
  records the rule; no split is executed here
• docs/adr/0003-verification-guards-earn-default-on-by-measured-precision.md — clause 4, the
  authority for rescoping rather than deleting, and the evidence bar that kept secret-pattern-detection scans the memory tier, which the topic-docs convention designates for raw credentials #3730 a filing
• docs/adr/0019-share-code-across-plugins-by-vendoring-with-a-sync-gate.md — the packaging
  analogue ADR 0028 is a sibling to
• lib/hook-utils.sh and its 17 vendored copies are untouched, so the sync gate is unaffected. The
  PostToolUse half of the kill-switch hoist belongs to claude/posttool-hooks-review-ji6rl5 under an
  agreed split

🤖 Generated with Claude Code

https://claude.ai/code/session_01TY2eXP6jpKBfH4XSg2b5mj

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-04T20:59:11.298460Z	2df0d85	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

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

[claude]

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

---

Code review in progress

• Gather context (PR body, changed files, CLAUDE.md/AGENTS.md)
• Run git diff origin/main...HEAD and review changed files
• Check skip-gate conditions (not draft, not closed, substantial diff → proceeding)
• Review scratch-root exemption logic in block-hook-bypass.sh
• Review MCP write-lane coverage in secret-pattern-detection.sh / hardcoded-path-check.sh
• Verify kill-switch hoist consistency across all 15 guards + new CI gate
• Review CI wiring, hooks.json changes, and docs/ADR changes
• Post findings as inline comments + summary

[claude]

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

---

Security review complete

• Gather context (PR diff, CLAUDE.md/AGENTS.md, skill instructions)
• Skip-gate check (open PR, security-relevant surface, no prior applicable review)
• Review diff for logic / trust-boundary / injection / secret-handling / Actions security issues
• Adversarially validate candidate findings
• Post findings

Reviewed the diff against origin/main (49 files) per /review:security-review. Scope: .github/workflows/ci.yml (Actions), the block-hook-bypass scratch-root default and cwd-relative-redirect resolution, the kill-switch hoist across 15 guards, the new GitHub MCP write lane on secret-pattern-detection/hardcoded-path-check, and the metadata/doc changes.

One finding posted inline (IMPORTANT): the memory-tier (.work/) default added to block-hook-bypass's scratch-root exemption reopens the exact class of secret-content bypass this PR closes for the GitHub MCP lane. The PR's "gives up no protection" claim holds for the temp-tree default (backed by hook::read_file_path's own temp-root decline, so a Write/Edit there was never content-scanned either) but not for .work/ — secret-pattern-detection.sh has no gitignore or .work/ carve-out and does scan .work/ writes made through Write/Edit today. Since secret-pattern-detection/hardcoded-path-check only match Write|Edit|NotebookEdit (+ the new MCP row), a printf '<secret>' >> .work/notes.md that this PR's own new fixtures confirm is now exempted (block-hook-bypass.test.sh:1519-1528) reaches disk with zero content scanning — previously it was blocked and had to go through the scanned Write path. Details, code citations, and a fix suggestion are in the inline comment on block-hook-bypass.sh.

Everything else reviewed clean:

• ci.yml: additive steps only, no permission/trigger changes, no injection surface.
• The 15 kill-switch hoists all mirror hook::is_enabled's exact semantics (correct default per guard); no fail-open regression.
• The new MCP write lane correctly omits the project-scope/git-working-tree/git check-ignore local-only gates (applying them would have silently skipped every MCP write) and correctly excludes mcp__github__delete_file (no content field).
• The .cwd-relative redirect resolution in block-hook-bypass fails closed on cd/pushd/popd and on a missing/relative cwd, so the fail-closed set only shrinks by targets it can prove are placeable.

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2df0d85439

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

[github-actions]

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