You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Bundles four PreToolUse safety guards into one concern-plugin with per-hook HOOK_*_ENABLED kill switches: secret-pattern-detection, hardcoded-path-check, cli-flag-verify, block-no-verify. Bundles path-detection + cli-flag-verify libs via ${CLAUDE_PLUGIN_ROOT}. block-no-verify rewritten to an argv-grammar-faithful gate (sees through quoting, escaping, wrappers, git global options, short-bundles, Windows case) with an honest README residual on shell-expansion bypass. 163 assertions; includes a machine-specific-paths CI exclude for the bundled pattern lib. Clean-tier wave Phase 2.
Note
High Risk
New PreToolUse blocks can stop legitimate writes/commits (secrets, paths, git bypass) and the git-bypass guard is bypassable via shell expansion; mis-tuned allowlists or missing jq change effective protection.
Overview
Adds a new guardrails marketplace plugin that wires four independently toggleable hooks (HOOK_*_ENABLED) into PreToolUse / PostToolUse: blocking secret-pattern writes, hardcoded machine paths, and git hook-bypass Bash commands, plus advisory CLI-flag verification after edits.
Blocking guards exit 2 with stderr guidance; they scope to $CLAUDE_PROJECT_DIR, honor allowlists / git check-ignore, fail open without jq (with a visible notice), and emit opt-in hook-telemetry envelopes that carry category labels only—never secrets, full commands, or matched paths.
block-no-verify is rewritten as an argv-grammar-faithful parser (quoting, wrappers, git -C, core.hooksPath, lefthook env vars, 16KB fail-closed cap); README documents residual shell-expansion bypass.
Bundled lib/path-detection and lib/verification/verify-cli-flag.sh support path scanning and subcommand-aware --help checks; CI excludes the pattern lib from the machine-specific-paths lane so embedded regex bodies do not self-trip.
Catalog, README, hook-telemetry schemas/examples, and extensive *.test.sh contract suites document and lock behavior.
Reviewed by Cursor Bugbot for commit b76d1bb. Bugbot is set up for automated code reviews on this repo. Configure here.
Migrate four medley PreToolUse guard hooks into one bundled marketplace
plugin whose core contract is per-hook kill switches:
- secret-pattern-detection (blocking) — high-confidence secret/credential
patterns in new content; HOOK_SECRET_PATTERN_DETECTION_ENABLED
- hardcoded-path-check (blocking) — machine-specific paths; bundles the
path-detection lib via ${CLAUDE_PLUGIN_ROOT};
HOOK_HARDCODED_PATH_CHECK_ENABLED
- block-no-verify (blocking) — git hook-bypass (--no-verify, core.hooksPath,
LEFTHOOK env prefix); inlines the literal-stripping the byte-identical
hook-utils.sh does not carry; HOOK_BLOCK_NO_VERIFY_ENABLED
- cli-flag-verify (advisory) — hallucinated CLI flags; bundles verify-cli-flag.sh
via ${CLAUDE_PLUGIN_ROOT}; HOOK_CLI_FLAG_VERIFY_ENABLED
Blocking semantics (exit 2 + stderr) preserved identically. Guards emit the
marketplace hook-telemetry envelope (status blocked/ok/skipped) with a
privacy-safe data payload (category labels only — never secrets, matched
paths, or full commands); adds per-hook data schemas + examples and the
Implementers-table rows. hook-utils.sh is a byte-identical copy of the shared
lib. Consumer seams: $CLAUDE_PROJECT_DIR scoping, git check-ignore, and env
overrides. Ships self-contained contract tests (119 assertions) covering each
guard's block/allow verdicts, disabled-path no-op, and telemetry.
Marketplace category: security (first non-formatting concern in the catalog).
…aths lane
The guardrails plugin bundles lib/path-detection/hardcoded-path-patterns.sh,
whose ERE bodies self-match the machine-specific-paths detector. Add an
exclude pathspec to that lane (the ci-workflows action's `exclude` input).
Pinned action SHA untouched.
Confirmed bypass: `git commit "--no-verify"`, `git commit --no-ver'ify'`, and
`git -c "core.hooksPath=/dev/null" commit` all passed — strip_literals deleted
the quoted spans while the shell strips those same quotes before git parses
argv, so the flag reached git unblocked.
Detect the bypass token on a QUOTE-COLLAPSED view of the command (quote
characters removed, content kept — exactly what argv sees), while still gating
on a real `git commit`/`git push` executable resolved from the span-stripped
view (so `echo "git commit --no-verify"` stays a false-positive no-op). Trade-off
is fail-closed: a literal --no-verify token inside a commit message is blocked
(documented in README). Adds regression tests for all three bypass forms plus
quoted-`-n` and single-quoted core.hooksPath. Also fails open with a visible
stderr notice when jq is absent (no silent disable).
…wlist anchoring
- Fail-open visibility: each guard now prints a one-line stderr notice and
exits 0 when jq is absent, instead of silently disabling.
- Telemetry redaction: when a path cannot be made repo-relative, data.file is
reduced to its basename so no absolute path (embedding the developer's
username) lands in a telemetry envelope.
- Allowlist anchoring: the secret-pattern-detection dependency-cache exemptions
(node_modules, .venv) are anchored to a path-segment boundary, so a directory
merely CONTAINING the name (evil_node_modules/, .venv-backup/) is scanned
rather than exempted.
- README: document lefthook-only env-manager coverage (husky/pre-commit not
matched; --no-verify / core.hooksPath checks are manager-agnostic), the
jq fail-open behavior, and the quote-transparent fail-closed trade-off.
Adds secret-suite regression tests for the jq guard, allowlist anchoring
(real segment exempt vs substring impostor scanned), and telemetry redaction.
Round-1 hardened flag DETECTION but left the executable/subcommand GATE on the
span-stripped view, so quoting or escaping the git or commit token still
defeated it. Five argv-verified bypasses passed: `"git" commit --no-verify`,
`git "commit" --no-verify`, `g'i't commit --no-verify`, `git commit
--no-\verify`, `"git" -c core.hooksPath=/dev/null commit`.
Replace the flattened-string regex with an argv-faithful parser: split the
command into top-level segments on UNQUOTED control operators (quote-aware, so
an operator inside quotes does not split), tokenize each segment into argv words
honoring '…', "…" and backslash escapes, then gate on the parsed executable
(basenames to git) and its subcommand (commit/push after any -c k=v / global
options). Bypass tests run over the same argv words. Because quoted whitespace
stays within a word, a --no-verify inside a quoted -m value is a message word,
not a flag — now correctly ALLOWED (the round-1 fail-closed FP is gone).
Adds regression tests for all 5 bypass forms (exit 2) and the negatives that
must stay exit 0 (quoted-arg echo, operator-in-quotes, command-not-found
"git commit" form, flag-in-message-value). README updated to the argv-faithful
claim.
…bundle -n, wrappers, case-fold, linear parser
Round-2 closed the quoting class but left decidable argv-slot gaps. Replace the
gate with a full argv-grammar parser and close them:
- git global options that consume an argument (-C, --git-dir, --work-tree,
--namespace, --super-prefix, --config-env, --attr-source, --exec-path) are
modeled as two-word consumers, so `git -C . commit --no-verify` blocks.
- Executable detection scans every argv word for a git basename (not a single
leading slot), so wrappers with options pass through transparently:
`env -i git …`, `nice git …`, `timeout 5 git …`, `sudo -u x git …`.
- -n is detected in a leading short-option bundle only before the first
argument-taking short (m/F/c/C/t/u/S/G): `-nm msg` / `-vn` block, `-mn`
(m takes value "n") does not.
- Executable match is case-insensitive + `.exe`-stripped on Windows/MSYS only
(OS-gated like hook-utils normalize_path): `GIT` / `git.exe` block on Windows,
stay case-sensitive on POSIX.
- Decidable expansion subset closed in the tokenizer: `$'…'` ANSI-C words are
decoded and backslash-newline continuations collapsed before segmenting.
- Parser is single-pass linear (char array read once) with a 16 KB length cap
that blocks fail-closed above it (obfuscation guard).
README narrows the claim to the true surface and documents the residual: does
NOT evaluate $VAR / $(…) / $IFS — a friction guard, not a sandbox. Adds
regression tests for every form (61 cases, all prior negatives/positives kept).
Review README and docs (telemetry schemas/examples)
Review marketplace.json and CI changes
Synthesize findings and post review
This is a well-engineered plugin. The architecture is sound (shared hook-utils.sh, fast-reject gates, fire-and-forget telemetry, per-hook kill switches), the test coverage is thorough, and the fail-open / fail-closed semantics are correctly chosen per context. I have two correctness bugs in block-no-verify.sh, one documentation inconsistency, and a few minor notes below.
Tracing git commit -m "LEFTHOOK=0": the double-quote parser strips the surrounding quotes, producing the word LEFTHOOK=0. check_segment then sees ["git","commit","-m","LEFTHOOK=0"]. The scan starts at k=0 and LEFTHOOK=0 at k=3 matches the regex → blocked.
The fix is to scope the LEFTHOOK scan to words before the git binary (k=0..gi-1), since that is where shell env-var prefixes live. env LEFTHOOK=0 git commit still fires; commit messages referencing LEFTHOOK values do not.
for((k = gi +1; k < nseg; k++));do
lc="${w[k],,}"
[[ "$lc"==*core.hookspath=* ]] && block "hooksPath" \
Tracing git commit -m "core.hooksPath=/dev/null": the double-quote parser strips quotes, yielding core.hooksPath=/dev/null as a single word. The scan at k=3 (after -m) sees core.hookspath=/dev/null → *core.hookspath=* matches → blocked. A commit message documenting this config option would be wrongly blocked.
Unlike Form 1 (--no-verify), which correctly breaks the scan loop when it hits -m/-F/-C etc., Form 1b has no equivalent guard. The fix is to skip the word immediately following a -m/-F/-c/-C/-t/-u/-S/-G short option when scanning for core.hooksPath=. (The test "quoted core.hooksPath prose (allowed)" passes because it wraps the entire git command inside an echo "…" argument — it doesn't cover a message-value containing core.hooksPath=.)
A Claude Code plugin bundling four PreToolUse safety guards
The table two paragraphs later correctly identifies cli-flag-verify as PostToolUse · Write | Edit, but the opening sentence contradicts it. Suggest: "three blocking PreToolUse guards and one advisory PostToolUse guard."
cli-flag-verify hook timeout vs. cold cache. With 9 default binaries and a 5 s per-binary --help timeout, a cold first run could take up to 45 s against a 30 s hook timeout. In practice most files reference only one or two binaries, and the 24 h cache makes subsequent runs fast, so this is a documentation-level gap rather than a code change. Worth a sentence in the README under Requirements: "cli-flag-verify caches --help output for 24 h; on first run with many binaries the 30 s hook timeout may truncate checking."
tool field is empty string in cli-flag-verify telemetry.cli-flag-verify.sh L281 hardcodes tool:"" while the other hooks populate this from $TOOL (Write/Edit). Since hook::read_file_path consumes stdin without exposing tool_name, the hook would need to buffer stdin first to populate this field. The schema may document it as always empty — but if so, a "tool":"PostToolUse:Write|Edit" constant or the field's removal would be cleaner than a silent empty string.
What's done well
Fast-reject gates in both secret-pattern-detection.sh and hpp::scan_text — single-grep early exit avoids spawning 5-process pipelines on the common clean path. The Windows MSYS2+Defender cost context in the comments is good documentation.
Fire-and-forget telemetry correctly avoids fd1 inheritance. The fd1-inheritance blocker comment ("C1 fd1-inheritance blocker") in hook::emit_telemetry is exactly right.
Argv-grammar-faithful parser — quoting, ANSI-C $'…', backslash-newline continuation, wrappers (env -i, sudo -u x), git global options (-C, --git-dir, --work-tree), short-option bundles — all tested at the correct granularity. The 16 KB fail-closed cap is the right safety valve.
Allowlist path-segment anchoring with the .venv-backup and evil_node_modules regression tests. This is the subtle guard that commonly gets wrong in naive allowlists.
PII safety in telemetry — labels only, no matched path or secret value in envelopes, basename-only fallback when repo-relative path can't be derived. The alice assertion in the test is a good negative.
jq dependency handled gracefully (fail-open with visible stderr notice rather than silent disable). This is the correct tradeoff for a blocking hook.
CI exemption for path-detection patterns is correctly scoped to plugins/guardrails/lib/path-detection/** — narrow enough that the bypass doesn't cover the hook implementations.
Token construction discipline in test files — runtime-assembled from fragments, so ghp_aaa… never appears as a committed literal that would trip gitleaks.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot completed with status skipping (usage limit reached) and did not produce a clean review, so automated review is incomplete. Human review is needed; no reviewers were assigned because the only human repo contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Merge origin/main; keep marketplace entries for markdown-formatter,
bash-lint, biome-format, ruff-format, eol-normalizer, desktop-notification,
and guardrails.
- Scope hardcoded-path-check to CLAUDE_PROJECT_DIR so outside-repo writes
are not blocked.
- Match git only at the segment command position (after env prefixes and
known wrappers) in block-no-verify so diagnostic commands like
echo git commit --trailer "Co-authored-by: Cursor <cursoragent@cursor.com>" --no-verify are allowed.
Co-Authored-By: Claude <noreply@anthropic.com>
Review current state of the PR (post-merge commits)
Check if previously identified bugs were fixed (LEFTHOOK scan scope, core.hooksPath scan, README PreToolUse wording)
Review new changes from merge/Codex feedback commits
Review block-no-verify.sh for correctness
Review other hook implementations for any issues
Synthesize findings and post review
The latest commit (4aca74b) addressed the two Codex-raised issues. The two correctness bugs and the documentation inconsistency from the previous Claude review at 48f73b2remain open.
What was fixed
Codex P2 — resolve_git_index() (command-position guard). The refactor from a bare is_git_bin scan across all words to a proper resolve_git_index() that walks past env-var assignments and known wrappers (env/nice/nohup/sudo/timeout) before testing for git is correct. echo git commit --no-verify now correctly passes. The new test case (echo git commit --no-verify (git as arg, allowed)) covers it.
Codex P2 — hardcoded-path-check.sh scope guard. The CLAUDE_PROJECT_DIR-gated prefix check at lines 61–74 correctly short-circuits on out-of-project writes. The test case (outside CLAUDE_PROJECT_DIR → exit 0) confirms the guard works. Fail-open when CLAUDE_PROJECT_DIR is unset is the right choice.
Bugs still open (from the prior Claude review at 48f73b2)
1. git commit -m "LEFTHOOK=0" — still falsely blocked
# Form 2: hook-manager env-var prefix (commit OR push) — scan every word.for((k =0; k < nseg; k++));do
lc="${w[k],,}"
[[ "$lc"=~ ^lefthook[_a-z0-9]*=(0|false)$ ]] && block "hook-manager-env" ...
done
The loop still runs from k=0 to nseg — all words including those after the git executable. Tracing git commit -m "LEFTHOOK=0": the double-quote parser strips quotes producing the word LEFTHOOK=0; words are ["git","commit","-m","LEFTHOOK=0"]; gi=0; at k=3 the pattern matches LEFTHOOK=0 → wrongly blocked.
The fix is k < gi instead of k < nseg: env-var prefix assignments live before the executable, so scanning past gi is never correct. The intended detection case (LEFTHOOK=0 git commit) still fires because that word is at k=0 which is before gi=1. No test was added for this false positive.
# Form 1b: core.hooksPath assignment (commit OR push) — any word after git.for((k = gi +1; k < nseg; k++));do
lc="${w[k],,}"
[[ "$lc"==*core.hookspath=* ]] && block "hooksPath" ...
done
The loop scans all words after git with no awareness of argument-consuming options. Tracing git commit -m "core.hooksPath=/dev/null": words are ["git","commit","-m","core.hooksPath=/dev/null"]; gi=0; the scan starts at k=1, reaches k=3 where core.hookspath=/dev/null matches *core.hookspath=* → wrongly blocked. A commit message documenting that config option would be blocked.
The fix is to track an in_value flag in the scan: when the word at position k is one of the argument-consuming short options (-m, -F, -c, -C, -t, -u, -S, -G), skip k+1. This mirrors how Form 1 already breaks its scan loop when it encounters these options. No test was added for this false positive.
A Claude Code plugin bundling four PreToolUse safety guards
cli-flag-verify is PostToolUse, as the guard table on line 15 correctly shows. The opening sentence contradicts the table. Suggest: "three blocking PreToolUse guards and one advisory PostToolUse guard."
core.hooksPath false positive on commit message values
❌ Still open
README "four PreToolUse guards" vs PostToolUse reality
❌ Still open
The two false-positive bugs are correctness issues that will block legitimate agent commits. Neither has a regression test. The architecture and the rest of the implementation remain solid.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot completed with status skipping (usage limit reached) and did not produce a clean review, so automated review is incomplete. Human review is needed; no reviewers were assigned because the only human repo contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot completed with status skipping (usage limit reached) and did not produce a clean review, so automated review is incomplete. Human review is needed; no reviewers were assigned because the only human repo contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot completed with status skipping (usage limit reached) and did not produce a clean review, so automated review is incomplete. Human review is needed; no reviewers were assigned because the only human repo contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Resolve marketplace.json to include all main plugins plus guardrails.
Add guardrails catalog row to README alongside actionlint from main.
Co-authored-by: Cursor <cursoragent@cursor.com>
Review current state of block-no-verify.sh (LEFTHOOK scan, core.hooksPath scan, shell keyword bypass, timeout option parsing)
Review README for PreToolUse/PostToolUse accuracy
Review secret-pattern-detection.sh for trailing slash normalization
Check test coverage for open issues
Synthesize findings and post review
Good progress since the prior reviews. Four of the five previously-identified issues are now closed; one correctness bug and one documentation inconsistency remain open.
What's been fixed since the last Claude review
Finding
Status
core.hooksPath false positive on commit message values
✅ Fixed (scoped check to -c/--config/--config-env arg values only — better fix than was suggested)
Shell keywords/builtins (command git …, if git …) bypass
✅ Fixed (command, exec, builtin, !, if, while, etc. now transparent in resolve_git_index)
timeout -s KILL 5 git … bypass (option parsing)
✅ Fixed (-s/--signal/-k/--kill-after correctly consume their argument, then duration is consumed)
# Form 2: hook-manager env-var prefix (commit OR push) — scan every word.for((k =0; k < nseg; k++));do
lc="${w[k],,}"
[[ "$lc"=~ ^lefthook[_a-z0-9]*=(0|false)$ ]] && block "hook-manager-env" \
Tracing git commit -m "LEFTHOOK=0": the double-quote parser strips quotes, producing the word LEFTHOOK=0. check_segment sees ["git","commit","-m","LEFTHOOK=0"] with gi=0. The loop runs k=0..3; at k=3 the word LEFTHOOK=0 matches ^lefthook[_a-z0-9]*=(0|false)$ → wrongly blocked. A commit message documenting lefthook disable syntax would be blocked.
Shell env-var prefix assignments live before the executable — scanning past gi is never correct. The fix is a one-character change:
# was: for ((k = 0; k < nseg; k++)); dofor((k =0; k < gi; k++));do
LEFTHOOK=0 git commit still blocks because that word is at k=0 < gi=1. No test was added for this false positive.
README "four PreToolUse guards" vs PostToolUse reality
❌ Still open
The architecture, the resolve_git_index wrapper handling, the -c/--config scoping for core.hooksPath, and the expanded test suite are all solid. The LEFTHOOK false positive is the one remaining correctness issue that would block legitimate agent commits.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot completed with status skipping (usage limit reached) and did not produce a clean review, so automated review is incomplete. Human review is needed; no reviewers were assigned because the only human repo contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot completed with status skipping (usage limit reached) and did not produce a clean review, so automated review is incomplete. Human review is needed; no reviewers were assigned because the only human repo contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
All prior bugs (resolve_git_index, core.hooksPath, secret-pattern-detection trailing slash, hardcoded-path-check scope)
✅ Fixed (prior rounds)
The SC2249 fix in commit 6955900 is correct: the *) break ;; default in the nice|nohup option-parsing while loop's inner case is unreachable (the while condition already filters to -* tokens), but shellcheck requires the explicit default and the fix is safe.
A Claude Code plugin bundling four PreToolUse safety guards
The guard table at line 15 correctly identifies cli-flag-verify as PostToolUse · Write | Edit. The opening sentence contradicts it — this has been flagged in every prior review round.
One-line fix:
A Claude Code plugin bundling **three blocking PreToolUse guards and one advisory PostToolUse guard** that catch
The implementation is solid. The argv-grammar-faithful parser (resolve_git_index, the Form 1/2 scans, the tokenizer) is correct across quoting, escaping, wrappers, short-option bundles, and edge cases — verified by the 163-assertion test suite. All blocking/fail-open semantics are correctly chosen per context. The one outstanding item is a documentation-only inconsistency in the plugin README opening line.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot completed with status skipping (usage limit reached) and did not produce a clean review, so automated review is incomplete. Human review is needed; no reviewers were assigned because the only human repo contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Second-round Codex findings on #42: handle env split-string operands by
splicing the split words back into the resolver scan (results returned
via RESOLVED_WORDS/RESOLVED_GI globals so the caller matches the
rewritten argv), treat eval as a transparent prefix, and stop
verify-cli-flag.sh option parsing at the first positional so a target
flag named --quiet or --verbose is verified instead of consumed.
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.
Good progress — every correctness bug from prior review rounds is now closed. The latest commit addressed the three remaining Codex findings. One documentation inconsistency remains.
env -S 'git commit --no-verify' now correctly blocked. The fix splices the split-string operand back into the resolver's word array (RESOLVED_WORDS) and restarts the scan at i=0. The nameref (local -n w=RESOLVED_WORDS) means both the resolver and check_segment's copy are updated atomically before returning. All three forms are tested (separated -S val, attached -Sval, long --split-string=val); a non-git split (env -S 'ls -la') correctly passes. Implementation trace is sound — the continue 2 restarts the outer while with the rewritten array; RHS of the array assignment evaluates from the old w before the write, so slice indices are stable. ✅
eval git commit --no-verify is now blocked. eval is treated as a transparent prefix in resolve_git_index, like command/exec/builtin. The comment correctly scopes coverage to the unquoted form — eval 'git commit --no-verify' (single-quoted arg) passes through as one opaque word and is not caught; this is the documented expansion-bypass residual and is consistent with the hook's stated contract. Test added at line 131. ✅
3. verify-cli-flag.sh consuming target flags as verifier options — fixed
The old for arg in "$@" scan consumed every --quiet / --verbose anywhere in argv. The replacement while ... *) break ;; esac done stops at the first non-option positional, so verify-cli-flag.sh --quiet faketool --verbose now verifies --verbose rather than silently enabling verbose mode and verifying faketool alone. Three tests added covering leading option + target --toplevel (exit 0), target --verbose (exit 1, not consumed), and target --quiet (exit 1, not consumed). ✅
Previously fixed, confirmed still correct
Finding
Status
LEFTHOOK false positive on commit message (k < gi upper bound)
✅ Fixed (d7b95aa), test at line 122
git commit -m --no-verify (skip -m value before checking --no-verify)
A Claude Code plugin bundling four PreToolUse safety guards
cli-flag-verify is PostToolUse, as the guard table at line 15 correctly shows. This one-word fix has been flagged in every prior review round. The table's description ("The three blocking guards feed their stderr message back to Claude…") at line 17 already uses the correct framing.
Suggested replacement:
A Claude Code plugin bundling **three blocking PreToolUse guards and one advisory PostToolUse guard** that catch
verify-cli-flag.sh consuming target flags as verifier options
✅ Fixed (b76d1bb)
LEFTHOOK false positive on commit message values
✅ Fixed (d7b95aa)
All prior bugs from earlier review rounds
✅ Fixed
README "four PreToolUse guards" vs PostToolUse reality
❌ Still open (docs only)
The implementation is correct. The argv parser (resolve_git_index, Form 1/2 scans, tokenizer), wrapper handling, env -S splicing, and verifier option scoping are all sound. The single remaining item is a one-line documentation fix in the README opening sentence.
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Not approving: Cursor Bugbot completed with status skipping (usage limit reached) and did not produce a clean review, so automated review is incomplete. Human review is needed; no reviewers were assigned because the only human repo contributor is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
#218)
## Summary
`code-tidying:tidy`'s Phase H ("Ship") told the agent to create the PR
by calling `gh pr create` directly — a bare bypass of
`source-control:pull-request`'s canonical create flow, which resolves
issue-linkage (with an issue-existence check before shipping a `Closes
#N` keyword), assembles the PR body injection-safely (quoted heredoc +
parameter-expansion concat, a defense against shell injection through
prompt-derived text), and gates PR creation on a valid closing keyword
or explicit opt-out before calling `gh pr create` itself.
Closes decisions **#3** and **#42** of the melodic-software issue/PR
consistency initiative's Decisions Log
(`https://claude.ai/code/artifact/232ecdce-8316-4880-8c0a-dc3c7dcf3a63`):
- **#3 (formatting-tidy-canonical-pr-gate-bypass)** — "YES, fix — but
not via direct skill-to-skill coupling... minimize coupling between
`code-tidying:tidy` and `source-control:pull-request`... Likely shape: a
shared script/check both skills call, or a convention-level requirement
enforced by a common gate." This PR fixes the bypass without a hard
cross-plugin dependency (below).
- **#42 (crosscut-skill-content-sync-mechanism)** — "Single shared
reference doc, all authoring skills import it." Applied here for its
first concrete instance (see Design rationale).
## What changed
Phase H now applies the same optional-plugin graceful-degrade pattern
`tidy` already uses elsewhere in its own SKILL.md (Phase C for
`discovery`, Phase D for `work-items`):
- **If `source-control` is installed:** invoke `/pull-request create`,
supplying tidy's own title/body-section content. Its stage-and-commit
step (§2.3) is a no-op here since Phase E already committed the tidyings
atomically — the call goes straight to rebase-check, issue-linkage
resolution, and gated PR creation.
- **If `source-control` isn't installed:** the same invariants apply
inline — verify an issue exists before writing `Closes #N`, assemble the
body via quoted heredoc + concat (never an unquoted `<<EOF`), and refuse
`gh pr create` until the body carries a valid closing keyword or an
explicit `Refs #N` / `No related issue:` opt-out.
## Design rationale (decision #42, recorded as an assumption — no user
available to confirm interactively this session)
Considered extracting the safety invariants into a new shared reference
doc (mirroring the `docs/PLUGIN-ARTIFACT-PROTOCOL.md` → per-plugin
`reference/artifact-protocol.md` precedent, drift-checked by
`validate-plugin-contracts.mjs` and this session's new
`scripts/check-cross-plugin-source-drift.sh`). Rejected for now: every
invariant worth stating is already authoritative in
`pull-request/reference/create.md`, so a copied doc would be pure
duplication with drift risk — and with exactly one non-`source-control`
consumer today, it wouldn't even register as a cluster for the new drift
checker (which requires 2+ plugin copies). A prose convention pointer is
lighter, carries no duplication to drift, and degrades gracefully if
`source-control` is absent — which a hard-copied doc wouldn't buy any
safety over.
This is decision #42's design choice for its *first application*, not a
retrofit of every authoring skill in the marketplace — no other skill is
touched by this PR. Rolling the same pattern out to other PR-creating
skills (if any surface a similar bypass) is deferred, unscoped follow-up
work, tracked in the Decisions Log rather than guessed at here.
## Test plan
- `bash scripts/validate-plugins.sh` — all plugin manifests + strict
catalog validation pass.
- `node scripts/validate-plugin-contracts.mjs` — 16 setup skills + 1342
plugin files checked, clean.
- `npx markdownlint-cli2 plugins/code-tidying/skills/tidy/SKILL.md` — 0
errors.
- Confirmed no other stale bare-`gh pr create` references remain under
`plugins/code-tidying/`.
- No existing test file exercises `tidy`'s `SKILL.md` prose directly
(only `open-pr-count.test.sh`, unrelated to Phase H) — nothing to update
there.
…is active (#1039)
## Summary
`hardcoded-path-check.sh` scanned unconditionally when
`CLAUDE_PROJECT_DIR` was unset — the scope guard's explicit fail-open
branch — contradicting the README "Project scoping" contract ("only
police files under `$CLAUDE_PROJECT_DIR`"). The gitignore escape hatch
was gated on the same variable, so in exactly that case the one
documented per-file exemption was unreachable short of the global kill
switch. Real incident (handoff-inbox producer session): forced `~/`
rewrites onto a machine-local `~/.gitconfig` edited from a no-project
session.
The hook now **skips entirely when no project is active**: a no-project
target is machine-local, not the portable repo artifact this guard
protects. Deliberately different from `secret-pattern-detection`, which
scans even without a resolvable root (secrets are dangerous anywhere;
hardcoded paths only harm portable artifacts). Dead downstream
no-project branches (gitignore-gate condition, `PROJECT_ROOT` fallback)
simplified away. README consumer-seam bullets updated so doc and code
agree.
History check: the fail-open dates to the plugin's initial commit (#42)
with no recorded rationale beyond the inline comment; CHANGELOG 0.9.8
treated a similar prefilter fail-open as a bug.
## Tests
- Red-first: two new no-project skip cases (Write + Edit, incident
shape) failed on the unfixed hook (PASS=40 FAIL=4), green after (PASS=44
FAIL=0).
- Detection/allow cases now set `CLAUDE_PROJECT_DIR` explicitly —
scanning is a within-project behavior; exemption-path tests
(`.claude/hooks`, `.claude/projects`, gitignore seam) set an enclosing
project so each exemption stays exercised rather than masked by the new
skip.
- `shellcheck` clean; `check-changelog-parity --check-bump main` and
`check-changed-skills` pass.
## Versioning
guardrails 0.10.0 → **0.10.1** (patch — behavior fix, no new mechanism)
+ CHANGELOG entry.
## Fresh-docs citation
https://code.claude.com/docs/en/hooks — `${CLAUDE_PROJECT_DIR}`: "the
project root", exported as an env var on spawned hook processes; no
guarantee of presence in no-project sessions.
## Related
- Closes#1038
- Source: handoff-inbox item
`20260722-035839-guardrails-hardcoded-path-check-failopen-scope` (F2 of
that item recorded as working-as-intended decision note on #1038, no
pattern change)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bundles four PreToolUse safety guards into one concern-plugin with per-hook HOOK_*_ENABLED kill switches: secret-pattern-detection, hardcoded-path-check, cli-flag-verify, block-no-verify. Bundles path-detection + cli-flag-verify libs via ${CLAUDE_PLUGIN_ROOT}. block-no-verify rewritten to an argv-grammar-faithful gate (sees through quoting, escaping, wrappers, git global options, short-bundles, Windows case) with an honest README residual on shell-expansion bypass. 163 assertions; includes a machine-specific-paths CI exclude for the bundled pattern lib. Clean-tier wave Phase 2.
Note
High Risk
New PreToolUse blocks can stop legitimate writes/commits (secrets, paths, git bypass) and the git-bypass guard is bypassable via shell expansion; mis-tuned allowlists or missing jq change effective protection.
Overview
Adds a new
guardrailsmarketplace plugin that wires four independently toggleable hooks (HOOK_*_ENABLED) into PreToolUse / PostToolUse: blocking secret-pattern writes, hardcoded machine paths, and git hook-bypass Bash commands, plus advisory CLI-flag verification after edits.Blocking guards exit 2 with stderr guidance; they scope to
$CLAUDE_PROJECT_DIR, honor allowlists /git check-ignore, fail open without jq (with a visible notice), and emit opt-in hook-telemetry envelopes that carry category labels only—never secrets, full commands, or matched paths.block-no-verifyis rewritten as an argv-grammar-faithful parser (quoting, wrappers,git -C,core.hooksPath, lefthook env vars, 16KB fail-closed cap); README documents residual shell-expansion bypass.Bundled
lib/path-detectionandlib/verification/verify-cli-flag.shsupport path scanning and subcommand-aware--helpchecks; CI excludes the pattern lib from the machine-specific-paths lane so embedded regex bodies do not self-trip.Catalog, README, hook-telemetry schemas/examples, and extensive
*.test.shcontract suites document and lock behavior.Reviewed by Cursor Bugbot for commit b76d1bb. Bugbot is set up for automated code reviews on this repo. Configure here.