feat(guardrails): check repository files a shell command changed

[kyle-sexton]

Closes #6674

Summary

block-hook-bypass.sh keys on command shape, so a script file run by an interpreter (node x.js, python3 x.py) writes repository files that no Write/Edit guard sees. This adds check-bash-file-changes, which gates the outcome: every repository file a Bash or PowerShell command changed goes through the same guards a Write or Edit of it runs. Steps 1 and 3 of the issue; step 2 (narrowing block-hook-bypass.sh) is #6689, and block-hook-bypass.sh is unchanged here.

Fix

• hooks/check-bash-file-changes.mjs, one Node script with two modes, exec form (command: node), no bash on the common path:
  • snapshot (PreToolUse, Bash|PowerShell): finds the repository holding the call's cwd by walking up to a .git entry (no process), runs one git status --porcelain -z --untracked-files=all, and stores each dirty path's size and mtime under CLAUDE_PLUGIN_DATA, keyed by session and tool_use_id. Outside a git repository it starts nothing and writes nothing.
  • check (PostToolUse and PostToolUseFailure, since a Bash command that exits non-zero fires the latter): runs git status again; a path new to it, or whose size or mtime moved, is changed. Each one runs through the guards of the PreToolUse Write|Edit row, read from hooks.json so the two cannot drift (today secret-pattern-detection, hardcoded-path-check, block-windows-drive-tmp): a new file as a Write of its content, a tracked file as an Edit of the lines git diff adds against the index (hook-precision rule 1). A guard's block message reaches Claude as the PostToolUse block reason, or as additionalContext after a failed command. The command already ran, so this reports and does not deny.
• Fails open everywhere: git missing or failing, no snapshot, malformed stdin, a timeout, a binary or >1 MiB file. Git runs with GIT_OPTIONAL_LOCKS=0, so it never takes the index lock a concurrent git command needs. At most 20 files and 40 s of guard runs per call.
• Kill switch bash_file_change_check_enabled (default on). README guard table, kill-switch table and generated options reference updated; changelog fragment added.
• Two .performance/ratchets.json counters for the always-on path (snapshot, and a check after a command that changed nothing), ceiling 3 each (node plus one git). No strace on this host, so the ceilings are derived, not measured; the setup and check clauses were dry-run with a stand-in strace.
• Settings hook, not a mod: ADR 0052 keeps a hook a settings hook where it must run with mods off, as a guard must.

Verification

• hooks/check-bash-file-changes.test.sh: 23 assertions pass. The issue's step 3 case: node <scratch script> writes a new repo file with a hardcoded Linux home path, and the check's reason contains the exact stderr that run-guards.sh gives a Write of that content (and the Edit-path message for an appended tracked file, and a secret). Plus PostToolUseFailure, the PowerShell tool, and stay-quiet cases: no change, benign change, an already-dirty file the command did not touch, a gitignored file, a non-git cwd, no snapshot, kill switch, malformed stdin.
• Mutation check: disabling the size/mtime comparison fails the already-dirty stay-quiet case; scanning a tracked file's whole content instead of its added lines fails the Edit-path case.
• scripts/affected-tests.sh --run: all 6 selected suites pass (run-guards, coverage-manifest, abort-boundary, require-jq-*).
• check-hook-exec-form.sh, check-hooks-description.sh, check-hook-userconfig-argv.sh, check-hook-slow-shapes.sh, check-killswitch-hoist.sh, claude plugin validate plugins/guardrails: pass. shellcheck and shfmt clean.
• git status in this repository (about 8,000 files) takes about 30 ms.

Not verified: the hook on Windows (the Windows lane runs a fixed list of suites and this one is not on it), and the strace counts above.

Related

• fix(guardrails): narrow or retire block-hook-bypass now that shell file changes are checked #6689 (step 2: narrow or retire block-hook-bypass.sh)
• The testing plugin's test-scan-bash.sh reads tool_response.bashEditDiff, which Claude Code adds only when bashEditDiffEnabled is set; this hook needs no setting.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EFqTQRnCGywRxhv4nw4o4V

[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-10-09T20:34:03.313924Z	1cc088b	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 4m 49s —— View job

---

Code review (/review:code-review)

• Gather context (PR body, changed files, diff)
• Read hooks/check-bash-file-changes.mjs
• Read hooks/check-bash-file-changes.test.sh
• Read hooks/hooks.json diff and exec-bash.mjs/run-guards.sh it depends on
• Read .performance/ratchets.json, README, plugin.json, changelog fragment diffs
• Cross-check against CLAUDE.md/REVIEW.md criteria, docs/conventions/hook-precision/README.md rules 1 and 6, and ADR 0052 (settings hook vs. mod)
• Adversarially re-check candidate findings
• Post summary

Result: no blocking or high-confidence correctness findings

I read check-bash-file-changes.mjs end-to-end (snapshot/check/changedFiles/addedLines/writeGuards/runGuards/report), the hooks.json wiring (PreToolUse snapshot row, PostToolUse/PostToolUseFailure check rows), exec-bash.mjs's resolveBash/readStdin, and the 23-assertion test suite, plus the README/plugin.json/ratchets/changelog-fragment diffs.

Specifically verified against the stated design:

• writeGuards() reads exactly the PreToolUse Write|Edit|NotebookEdit row's guard list (secret-pattern-detection.sh, hardcoded-path-check.sh, block-windows-drive-tmp.sh) from hooks.json, matching the README table and the changelog fragment — no drift between the two lists.
• addedLines() keys each diff by the requested path and only counts + lines after the first @@ (no +++-header parsing), consistent with hook-precision rule 1 (diff-scope Edit checks to the changed hunk) and the already-applied hardening commit (1cc088b) that closed the forged/quoted-header issue a prior security pass found.
• Gitignored files never appear in git status --porcelain output at all, so they're excluded for free — consistent with rule 6.
• Symlinks are excluded via lstatSync (not followed), matching the "link.txt" stay-quiet test.
• snapshotDir() has no shared-temp fallback and no-ops without CLAUDE_PLUGIN_DATA, matching the "another local user could plant a symlink" fix.
• Fail-open paths (missing git, >MAX_STATUS_ENTRIES, oversized/binary files, malformed stdin, missing snapshot, kill switch) all check out against the code and are each covered by a test case.
• CHECK_BUDGET_MS (40s) leaves headroom under the 60s hook timeout for one more GUARD_TIMEOUT_MS (10s) guard run, as the comment claims.
• report()'s decision: "block" for PostToolUse vs. additionalContext for PostToolUseFailure is a deliberate split, not an inconsistency with the advisory-hook convention elsewhere in this plugin (those hooks are pure-advisory everywhere; this one intentionally mirrors the Write/Edit guard's own exit-2 message after the fact).

I did not find a case where the diff-scoping, symlink handling, size/mtime comparison, or guard-list sourcing produces either a false block or a silent miss beyond what the code's own "Scope residuals" comment and the README's "Not covered" list already disclose (background-command writes after the call returns, files past the first 20, >1 MiB/binary files, etc.) — those are documented, intentional trade-offs for a fail-open advisory check, not overlooked bugs.

Not independently verified by me (author-claimed in the PR body, not re-run here): the 23 test-suite assertions, the mutation checks, scripts/affected-tests.sh --run, the hook-contract/exec-form/killswitch-hoist scripts, claude plugin validate, shellcheck/shfmt, and the .performance/ratchets.json ceiling derivations (no strace on this host, as the PR itself notes). Also not exercised: Windows behavior, since the Windows CI lane doesn't run this suite.

Scope note: this PR already carries a prior security-review-driven hardening commit (1cc088b, symlink/temp-fallback/diff-header fixes); any further security-boundary questions belong on the dedicated security-review lane, not this correctness/maintainability lane.

[claude]

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

---

Security review in progress

• Gather context (diff, changed files, CLAUDE.md/AGENTS.md conventions)
• Review check-bash-file-changes.mjs for injection, path-traversal, and trust-boundary issues
• Check instruction-surface deletions against the exception register (hooks.json guard list, block-hook-bypass.sh) — no guard removed or narrowed
• Review hooks.json / plugin.json changes
• Review test file for anything security-relevant
• Adversarially verify 2 candidate findings (producer ≠ verifier) — running now
• Post findings (inline + summary)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1cc088b081

ℹ️ 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]

claude-review has reviewed this pull request through c64e767; a later push is reviewed from there.

[github-actions]

claude-security-review has reviewed this pull request through c64e767; a later push is reviewed from there.

[claude]

Claude finished @kyle-sexton's task in 10m 57s —— View job

---

Security review complete

• Gather context (PR body, custom instructions, review scope)
• Load /review:security-review skill and follow it exactly
• Read .claude-lane/incremental.diff (incremental scope since 1cc088b)
• Review changed files: README.md, check-bash-file-changes.mjs, check-bash-file-changes.test.sh
• Adversarially verify candidate finding with an independent review:lane-verifier agent
• Post findings (inline + summary)

Scope per the lane wrapper: this is the incremental pass, reviewing only what changed since 1cc088b081c1eb13d1f50dde0e7d32bb590af59c — the diff that addresses the two P1 Codex findings (staged-file diff baseline, and write+commit leaving git status clean) by diffing against the pre-command HEAD instead of the index, and adding reflog-based detection of commits made during the command.

Finding (IMPORTANT) — the new write+commit detection is trivially bypassed

Posted inline on committedSince at check-bash-file-changes.mjs#L150-L159.

committedSince walks the HEAD reflog newest→oldest and requires every entry between the old and new HEAD to have a subject starting with "commit", bailing out to false the instant one doesn't. git's reflog subject prefixes are fixed per operation (commit: …, checkout: moving from … to …, reset: moving to …), so a bare git commit is classified correctly — but any later, unrelated HEAD-touching command in the same compound line defeats the whole classification, even a content no-op:

git add secret.txt && git commit -qm "msg" && git reset --soft HEAD

reset --soft HEAD to the already-checked-out value touches neither the index nor the working tree, so git status stays clean, but it still appends a reset: moving to HEAD reflog entry. The loop hits that entry before the real commit entry and returns false → committed = []. With git status already clean, changedFiles() finds nothing either, and check() returns []: the exact write+commit content this PR was built to catch escapes every guard, using two ordinary git commands, no plumbing tricks required. (A sharper variant needs no accidental trailing command at all: git update-ref -m "<any text>" HEAD <sha>, built from git commit-tree, lets the caller set the reflog subject verbatim and defeat the classifier on the very commit that introduces the content.)

I verified both the reset --soft reflog-append behavior and the update-ref -m free-text reflog message via man git-update-ref's "LOGGING UPDATES" section, hand-traced the loop against the actual code, and confirmed via a separate review:lane-verifier agent (producer ≠ verifier) that no other path in changedFiles()/gitStatus() would catch the file. Verdict: CONFIRMED.

Suggest keying detection off the target commit specifically (sha === to) rather than requiring every intervening entry to read as a commit, or excluding reflog entries whose old/new OID is unchanged (a no-op) from the classification, rather than letting a single non-commit entry anywhere in the window invalidate the whole check.

Other changes in scope

• README.md: documentation-only update describing the new HEAD/reflog-based behavior — no security-relevant content beyond what the finding above already covers.
• check-bash-file-changes.test.sh: new test cases (5c, 5d, 5e) cover the staged-write and commit-then-clean scenarios the Codex findings raised, and a checkout-brings-in-a-branch stay-quiet case — no security issue in the tests themselves, but none of them exercise a trailing no-op/plumbing HEAD move after a real commit, which is why the gap above wasn't caught.
• Reviewed the porcelain v2 -z branch-header parsing (gitStatus) and the --literal-pathspecs/hex-validated base/from/to arguments passed to git diff and git reflog for header-join or argument-injection issues — both check out: branch header lines are individually NUL-terminated under -z (each uses the same eol as entries in git's own porcelain v2 printer), and every value passed to git as a ref is pre-validated against ^[0-9a-f]{40,64}$, so no argument injection or mis-parsed OID.

No GitHub Actions / zizmor-lane findings in this diff (no workflow files changed here).

[claude]

Claude finished @kyle-sexton's task in 5m 25s —— View job

---

Code review (/review:code-review, incremental)

• Gather context (PR body, review scope, incremental diff)
• Read .claude-lane/incremental.diff
• Review plugins/guardrails/hooks/check-bash-file-changes.mjs changes since 1cc088b
• Review plugins/guardrails/hooks/check-bash-file-changes.test.sh changes since 1cc088b
• Review plugins/guardrails/README.md changes since 1cc088b
• Cross-check Codex's two P1 findings against the new commit
• Adversarially verify candidate findings
• Post findings

Result: no blocking or high-confidence correctness findings

This commit (37e5af6) is a targeted fix for the two P1 findings Codex raised on 1cc088b (comment on check-bash-file-changes.mjs:264 and :341): staged-but-uncommitted writes being compared against the now-current index instead of a pre-command baseline, and a write-then-commit sequence leaving git status clean so nothing was reported. I traced both fixes end to end:

• Staged-write fix: addedLines now diffs the working tree against before.head (the pre-command HEAD commit, captured by snapshot) instead of the index, so a git added file is still seen (test 5c).
• Write-then-commit fix: gitStatus switched to --porcelain=v2 --branch to also capture HEAD's oid (L108-L135); check() now detects a moved HEAD and, only when every reflog entry between the old and new HEAD is a commit/commit (amend) action (committedSince, L149-L160), adds the paths those commits touched (committedFiles, name-only diff --diff-filter=AM between the two commits) to the changed set (test 5d).
• Stay-quiet correctness: a checkout/pull/merge that moves HEAD is correctly excluded, since its reflog subject doesn't start with commit — committedSince returns false on the first non-commit entry it meets walking backward from the new HEAD, so a branch switch that brings in someone else's flagged content reports nothing (test 5e, verified by hand-tracing the reflog walk).
• Verified the porcelain=v2 field-count parsing ({1: 8, u: 10, "?": 1} columns before the path) against git's documented record shapes for ordinary, unmerged, and untracked entries, and confirmed -z means git never quotes/escapes the path regardless of core.quotePath, so the fixed-width prefix fields can't shift.
• Checked that a file newly added by an in-command commit (not already dirty, so it never enters the status-based branch) is still diffed and scanned for content — it runs through the Edit shape with old_string: "" and the whole file as new_string, which secret-pattern-detection.sh (via hook::jq_fields … .tool_input.content .tool_input.new_string) and friends scan identically to a Write payload, so there's no detection gap from the tool-name choice.
• Confirmed the new committedFiles/committedSince exports aren't consumed anywhere else, so the addedLines signature change (now 3 args) and gitStatus's new {head, entries} return shape don't break another caller.
• The already-dirty-then-committed overlap case (a file dirty before the command, later git added/committed by it) is swept in by the new commit-based path the same way the existing status-based path already sweeps in pre-existing dirty content — consistent with the header's documented, deliberate over-inclusion for this fail-open advisory check, not a new gap.

I was not able to execute the test suite myself in this sandboxed review lane (the shell invocation was denied), so the 23-assertion pass claim in the PR body, including the new 5c/5d/5e cases, is traced by reading but not independently re-run here.

Nothing here changes the prior full review's conclusions on the rest of the file (guard-list sourcing from hooks.json, fail-open paths, symlink/gitignore handling, budgets) — those are unchanged since 1cc088b and out of this incremental lane's scope.

[claude]

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

---

Code review (/review:code-review, incremental)

• Gather context (PR body, review scope, incremental diff)
• Read .claude-lane/incremental.diff
• Load /review:code-review skill and follow it
• Review check-bash-file-changes.mjs changes since 37e5af6
• Review check-bash-file-changes.test.sh changes since 37e5af6
• Adversarially verify candidate findings (hand-traced the reflog walk against actual git reflog semantics)
• Post findings

Result: one finding

This commit (a01cd21) patches committedSince to skip a reflog entry that left HEAD unchanged (git reset --soft HEAD), closing the specific write+commit bypass the security lane confirmed on the prior commit (37e5af6), and adds test 5d2 for exactly that case.

Posted inline on committedSince:

The fix only catches a literal adjacent same-SHA no-op, not a round trip through a genuinely different HEAD value. git add secret.txt && git commit -qm msg && git checkout -q HEAD^ && git checkout -q - lands HEAD back on the flagged commit (clean git status, same end state as test 5d), but each checkout leg is a real move with a non-commit subject, so the loop's return false fires on the first checkout (i=0) before it ever reaches the real commit entry — committed = [], and the write+commit content escapes every guard again, with no plumbing, just two ordinary git checkout commands. New test 5d2 only exercises the literal reset --soft HEAD shape, so it doesn't catch this. Full write-up and a suggested direction (check sha === from membership independent of intervening non-commit subjects, rather than bailing on the first one) are in the inline comment.

Nothing else in this incremental diff (the gitStatus/porcelain parsing, guard-list sourcing, fail-open paths, etc.) changed since 37e5af6 and is out of this lane's scope.

Branch

[claude]

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

---

Security review in progress

• Gather context (incremental diff, custom instructions, review scope)
• Read .claude-lane/incremental.diff
• Review check-bash-file-changes.mjs changes since 37e5af6
• Review check-bash-file-changes.test.sh changes since 37e5af6
• Check instruction-surface deletions (none in this diff)
• Adversarially verify the reflog no-op fix closes the prior CONFIRMED bypass — running now
• Post findings (inline + summary)

[claude]

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

---

Security review in progress

• Gather context (incremental diff, custom instructions, review scope)
• Read .claude-lane/incremental.diff
• Review check-bash-file-changes.mjs changes since a01cd21
• Review check-bash-file-changes.test.sh changes since a01cd21
• Check instruction-surface deletions (none in this diff)
• Adversarially verify a candidate finding (merge-commit detection gap) — running now
• Post findings (inline + summary)

[claude]

Claude finished @kyle-sexton's task in 5m 11s —— View job

---

Code review (/review:code-review, incremental)

• Gather context (PR body, review scope, incremental diff)
• Read .claude-lane/incremental.diff
• Load /review:code-review skill and follow it
• Review check-bash-file-changes.mjs changes since a01cd21
• Review check-bash-file-changes.test.sh changes since a01cd21
• Adversarially verify candidate findings
• Post findings

Result: one finding

This commit (70112ec) replaces the reflog-text-walking committedSince (defeated by a non-adjacent checkout round trip on the prior commit) with a byte-offset approach: headLog/fileSize record the HEAD reflog file's size at snapshot time, and commitsMade reads only the bytes appended to that file since then, pulling out every line whose subject starts with "commit" regardless of what other (checkout/reset) lines are interleaved. I traced the reported checkout round-trip (git commit && git checkout HEAD^ && git checkout -) against this logic: it no longer bails early on a non-commit entry, so the real commit line is still found no matter how many intervening HEAD moves happen in between. New test 5d3 pins exactly that case, 5d4 covers the initial commit on an unborn branch (previously an explicit residual, now detected since the reflog file springs from size 0), and 5d5 covers a linked worktree's separate logs/HEAD. All three check out against the code.

One gap found in the new committedFiles (used when commitsMade finds appended commits):

• Posted inline on check-bash-file-changes.mjs:198. commitsMade classifies a reflog entry as a "commit" purely by its subject text, and finishing a conflicted merge with an explicit git commit (after git merge --no-commit or after git merge stops on conflicts) produces subject "commit (merge): ..." — which does start with "commit", so that SHA is included. But committedFiles's git log --no-walk=unsorted --name-only --diff-filter=AM <shas> -- suppresses diff/name-only output for any multi-parent commit by default (needs -m/-c/--cc), so that merge commit contributes zero paths. Net effect: a command that writes flagged content and finishes a merge with it inside one Bash/PowerShell call leaves git status clean and check() returns [] — no guard runs. This is a legitimate-use gap (no plumbing or forged reflog text needed, unlike the documented update-ref -m residual), and it contradicts the file header's own claim that merges "name no commit here" — that's true only for an auto-committed, conflict-free git merge (reflog subject "merge <branch>: ...", correctly excluded), not a manually-completed conflicted one. None of the new tests (5d3/5d4/5d5 are all single-parent) exercise it.
  • Suggested fix: add -m (or -c/--cc) to the git log call, or diff each multi-parent commit against its first parent explicitly.
  • Caveat: I could not run git locally in this review sandbox (shell execution was denied) to reproduce end-to-end; this is based on git's documented merge-diff-suppression rule for log/diff-tree and builtin/commit.c's "commit (merge)" reflog-subject logic for a commit made with MERGE_HEAD present, not an executed repro.

Nothing else in this incremental diff (the headLog worktree/submodule path resolution, the byte-offset-vs-rotation guard, changedFiles's merge of status-diff and committed-file paths, test additions) raised a correctness or maintainability concern.

[claude]

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

---

Security review in progress

• Gather context (PR body, custom instructions, review scope)
• Read .claude-lane/incremental.diff (scope: changes since 70112ec — adds --cc to the committedFiles git log call, plus test 5d6 for a resolved merge commit)
• Load /review:security-review skill and follow it
• Review check-bash-file-changes.mjs changes since 70112ec
• Review check-bash-file-changes.test.sh changes since 70112ec
• Adversarially verify candidate findings (dispatched a hunt for a residual merge-detection gap) — running now
• Post findings (inline + summary)

Branch

[claude]

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

---

Code review (/review:code-review, incremental)

• Gather context (PR body, review scope, incremental diff)
• Read .claude-lane/incremental.diff
• Load /review:code-review skill and follow it
• Review check-bash-file-changes.mjs changes since 70112ec
• Review check-bash-file-changes.test.sh changes since 70112ec
• Adversarially verify candidate findings
• Post findings

Result: no blocking or high-confidence correctness findings

This commit (25ca68d) closes the prior finding on 70112ec: a conflicted merge the command resolves and commits (commit (merge) reflog subject, correctly picked up by commitsMade) previously contributed zero paths from committedFiles, because git log --no-walk's default diff mode suppresses output entirely for multi-parent commits.

The fix adds --cc to that git log call. I traced this against git's documented combined-diff semantics:

• --cc (like -c) makes git log/git diff generate a combined diff for a merge commit instead of suppressing it, and further collapses hunks/files that are identical to one parent ("trivially merged") so only paths the merge actually had to reconcile show up.
• For combined diffs, the per-path status git computes for --diff-filter purposes collapses to a single status character (added/modified/deleted relative to the merge result as a whole), so --diff-filter=AM continues to filter correctly rather than comparing against the per-parent multi-letter status used only by --raw format.
• A manually resolved conflict (content that differs from both parents, as in new test 5d6) is "interesting" under --cc and is included; a clean, non-conflicting auto-merge (content identical to one parent) stays excluded — consistent with the file's own documented residual ("a checkout, pull, merge or reset entry brings in others' content... not judged").
• --cc is a no-op for the single-parent commits that can appear in the same commits array, so no regression to the existing 5d/5d2/5d3/5d4/5d5 cases.

New test 5d6 (merge two diverging branches, resolve the conflict with flagged content, commit) exercises exactly the gap the prior finding described, and traces correctly against the fixed code. The g() test helper's added -c rerere.enabled=false is a reasonable defensive guard against a recorded-resolution cache affecting the new conflict scenario, not a functional change to the hook itself.

I was not able to execute git or the test suite in this review sandbox (shell mutations are denied here), so this is a traced-by-reading verification of the fix and the new test, not an executed repro — consistent with the same limitation noted on earlier passes of this lane.

Nothing else changed in this incremental diff (the rest of check-bash-file-changes.mjs and the rest of the test file are unchanged since 70112ec) and is out of this lane's scope.
· Branch

[claude]

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

---

Security review in progress

• Gather context (incremental diff, custom instructions, review scope)
• Read .claude-lane/incremental.diff (scope: changes since 25ca68d — plugins/guardrails/hooks/exec-bash.test.sh only)
• Load /review:security-review skill and follow it
• Read full plugins/guardrails/hooks/exec-bash.test.sh for context
• Check instruction-surface deletions (none; this diff tightens routing checks, doesn't loosen any)
• Apply skip gate
• Post findings

Result: no security issues

This commit (c64e767) only changes plugins/guardrails/hooks/exec-bash.test.sh: it adds a route_problem/native_row helper pair and table-driven cases asserting that the new check-bash-file-changes.mjs rows in hooks.json (the native snapshot/check rows added earlier in this PR) are wired correctly — exact [entry, mode] args, no shell key, matcher Bash|PowerShell, and the right event per mode — and extends the main row-scanning loop to recognize and count those native rows instead of misclassifying them as the exec-bash.mjs dispatcher.

This is test-only code:

• All inputs are literal strings constructed in the test (native_row PreToolUse 'Bash|PowerShell' "[\"$entry\",\"snapshot\"]", etc.) or read from the repository's own hooks.json — nothing here parses untrusted input, so there's no injection surface (command, path, or otherwise) to evaluate.
• jq invocations use --arg/--argjson parameterization throughout (e.g. jq -cn --arg e "$1" --arg m "$2" --argjson a "$3" --argjson x "$extra" '...'), not string-interpolated filters, so no jq-filter-injection concern even if an input were attacker-influenced.
• The change tightens the routing/shape checks this test suite enforces (it now also rejects a native row that carries a launcher flag, sets shell, or targets the wrong event) rather than removing or narrowing any existing assertion — nothing here matches the instruction-surface-deletion lens.
• No secrets, tokens, authorization logic, or trust-boundary crossing is touched by this diff.

Per the skip gate (no security-relevant surface in this change), no findings to report, inline or otherwise.

Nothing else is in scope for this incremental pass — every other file in this PR was reviewed as of 25ca68d480fb329265fd73a08fe5a14bac9e3edc.
· Branch

[claude]

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

---

Code review (/review:code-review, incremental)

• Gather context (PR body, review scope, incremental diff)
• Read .claude-lane/incremental.diff
• Load /review:code-review skill and follow it
• Review plugins/guardrails/hooks/exec-bash.test.sh changes since 25ca68d
• Adversarially verify candidate findings
• Post findings

Result: no blocking or high-confidence correctness findings

This diff is test-only: it extends exec-bash.test.sh's hooks.json-shape assertions to cover the new check-bash-file-changes.mjs rows added by this PR, which run in exec form directly (command: node, no exec-bash.mjs/run-guards.sh wrapper, no bash). The prior shape check (args[0] must be exec-bash.mjs) would have failed on these new native rows, so this update was required for the suite to pass at all.

I traced route_problem() against the actual hooks.json on this head:

• The three real check-bash-file-changes.mjs rows — PreToolUse/Bash|PowerShell → snapshot (L82-91), PostToolUse/Bash|PowerShell → check (L198-212), PostToolUseFailure/Bash|PowerShell → check (L214-229) — each hits the case "$event" branch matching its event, has args == [entry, want] exactly, carries no shell key, and has matcher Bash|PowerShell, so route_problem reports no problem for any of them. That makes native == 3, matching the new assertion.
• Counted the unaffected rows directly from the file: 8 exec-bash.mjs dispatcher rows (Write|Edit|NotebookEdit, the mcp__...push_files matcher, the first Bash|PowerShell/PreToolUse hook, and the 5 if-conditioned PostToolUse/Write|Edit rows) and 1 --require-true workflow row — matching dispatcher -ge 8 and workflow -eq 1 unchanged.
• Hand-traced each of the 6 synthetic "bad" rows against route_problem: wrong entry name, wrong mode for the event (snapshot on PostToolUse, check on PreToolUse), an extra launcher flag before the mode, a shell key, and a matcher of bare Bash instead of Bash|PowerShell — every one is correctly flagged, and the one well-formed synthetic snapshot row is correctly accepted.
• The unrelated skips_on_blocking check later in the file reads hooks.json independently of this change and the new native rows (no --skip-* flag) don't interact with it.

I was not able to execute the shell test (bash exec-bash.test.sh) in this review sandbox — shell execution was denied — so this is a traced-by-reading verification of the new assertions against the real hooks.json, not an executed repro, consistent with this lane's limitation on earlier passes of this PR.

Nothing else in this incremental diff raised a correctness or maintainability concern.
· Branch