Skip to content

v2.9.1: native deny rules and a small dirty-tree hook replace the per-command guards - #188

Open
pedrohcgs wants to merge 2 commits into
mainfrom
light-hook-core
Open

pedrohcgs wants to merge 2 commits into
mainfrom
light-hook-core

Conversation

@pedrohcgs

Copy link
Copy Markdown
Owner

Summary

The per-command guard hooks slowed everyday work and refused ordinary commands too often. Measured cost: 50–90 ms per hook, mostly Python start-up. The real cost was refusals, each costing a model turn and a retry. This release removes them and keeps the protection that mattered, using Claude Code's own permission rules.

Removed

  • git-guardrails.py and root-of-trust-guard.py, which ran before every Bash call with onFailure: block.
  • context-monitor.py, which ran after every Bash call.
  • The files stay at tag v2.9.0, and the guide says how to restore one.

Added

  • Native deny rules in .claude/settings.json (applied by the owner):

    • git reset --hard, git clean, git push --force and -f (not --force-with-lease), git add -A, --all and ., git checkout -- ., git checkout . and git restore .;
    • reads of .env, *.pem, *.key and credentials*.

    Claude Code checks them itself. They also apply inside ( ) and $(), which closes git-guardrails misses destructive git inside ( … ), $( … ) and backticks #172. They do not match rewritten spellings such as git -C . push, and every doc that mentions them says so.

  • dirty-tree-guard.py:

    • it runs only for git merge, git pull and git rebase (if filters), and refuses while tracked files have uncommitted changes;
    • untracked files, --abort/--continue, and ALLOW_DIRTY_MERGE=1 pass;
    • it fails open, except that a missing hook file refuses those three commands.

Changed

Verification

  • Live, after the settings change, in a scratch repository with a bare remote:
    • every deny rule refused its command, including (git reset --hard) and echo "$(git reset --hard)";
    • every control ran: reset --soft, push --force-with-lease, push --follow-tags, add <files>, restore --staged <file>, checkout -- <file>, merge-base, log --merges;
    • dirty-tree-guard refused a live git merge;
    • the .env rules refused Read, Write, and a shell command that would have written one.
  • Before deleting the old hooks: git -C <scratch> reset --hard ran, so git-guardrails was no longer active.
  • dirty-tree-guard battery: section (j), 16 cases. Six mutants each fail their own cases: check removed, untracked counted, override ignored, GIT_* kept, resume steps not exempt, no real-op check.
  • Grading the grader, re-measured: each remaining decision hook, stubbed to always allow, reddens exactly its own cases (6, 24 and 3).
  • ./scripts/backtest.sh: all 11 gates pass, with 110 battery cases and 130 portability cases. The guide is re-rendered and restamped.
  • Superseded: PR Guards deny an event that lacks the field they read (#186, first half) #187. It made the removed guards stricter, so it will be closed, not merged.

…er-command guards

The owner found the per-command hooks slow and glitchy; the measured cost was not latency
(50-90 ms each, mostly Python start-up) but refusals, each costing a model turn and a retry.

Removed: git-guardrails.py and root-of-trust-guard.py (PreToolUse on every Bash call, with
onFailure: block) and context-monitor.py (PostToolUse on every Bash call). Their files stay at
tag v2.9.0.

Added:
- permissions.deny in .claude/settings.json (applied by the owner): git reset --hard, git clean,
  push --force / -f (not --force-with-lease), add -A / --all / ., checkout -- . / checkout . /
  restore ., plus .env, *.pem, *.key and credentials* reads. Verified live in a scratch repo:
  every rule refused its command, including (git reset --hard) and $(git reset --hard), which
  were open as #172; every control ran.
- .claude/hooks/dirty-tree-guard.py: started only through if filters for git merge / pull /
  rebase; refuses while tracked files have uncommitted changes; untracked files, resume steps
  and ALLOW_DIRTY_MERGE=1 pass; fails open except when its own file is missing. Battery
  section j (16 cases); six mutants each fail their own cases.

Battery 322 -> 110 cases, grading-the-grader re-measured for the remaining decision hooks.
Portability 245 -> 130 (test_rot.py and the removed hooks' cases gone; the Windows code-page
cases moved to dirty-tree-guard). The ledger moves the removed hooks' rows and notes, unedited,
to "Retired at v2.9.1", with their counts reading "at the time". The status line keeps only
Claude Code's own context percentage. README, guide (re-rendered, restamped), TROUBLESHOOTING,
docs/index.html, AGENTS.md, SECURITY.md, confidential-data.md (CLAUDE_STRICT_PATHS is gone)
and four skills updated. CHANGELOG v2.9.1 leads with the upgrade steps for forks that changed
their settings.json; guide and CITATION at 2.9.1.
Copilot AI balanced review requested due to automatic review settings November 7, 2026 04:51
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

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

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-07T04:51:23.482817Z
We couldn’t complete this review. Comment @codex review to try again.
8b4f16c PR opened
ℹ️ 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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

…and launcher patch prepared for the owner

Codex CLI review of PR #188 (the cloud review hit its usage limit), eight findings, all
confirmed against the code:

Fixed here:
- the hook searched raw text, so a --abort in a comment or a -m message exempted a real
  merge, a quoted 'git pull origin' inside a grep was refused, and redirections confused both
  checks. It now tokenizes with shlex (quotes, comments, ; & | ( ), < and > split off but not
  ending the command), re-reads $(...) and backticks outside single quotes, skips heredoc
  bodies, and walks VAR=value, wrappers and git's global options. Found live as well: the hook
  refused one of this session's own commands, a heredoc whose text held a backticked pull.
  Battery j17-j29, 123 cases; fifteen mutants each fail their own cases.
- the locale-decoding portability case used a clean tree, where a fail-open decoding error
  also reads as silence; it now uses a dirty tree and requires a deny (a locale-decoding
  mutant fails it; an unmodified copy passes).

Prepared for the owner (settings.json is owner-edited only), and described in the docs as
the state this PR merges with:
- the deny list missed variants v2.9.0's git-guardrails denied (restore -- ., restore
  --worktree ., checkout HEAD -- ., add -v -A, add -v --all, add file .) and refused two
  ordinary commands (push origin feature--force, reset -- tracked--hard.txt). The new list
  matches whole option words in any position: 30 refused, 24 allowed, 0 wrong in a matcher
  built from the documented rule semantics.
- a registered hook whose file is missing makes python3 exit 2, which refused every command
  the if filter starts the hook for, including any with $VAR. The registration now goes
  through sh -c '[ -f "$1" ] || exit 0; exec python3 "$1"', which the ledger-coverage gate
  already parses.
@pedrohcgs

Copy link
Copy Markdown
Owner Author

Review. The cloud Codex review hit its usage limit, so I ran the Codex CLI locally (codex exec, read-only sandbox) on git diff main...HEAD. It made eight findings. I checked each against the code: all eight are real.

# Finding Status
1 P1: deny rules miss git restore -- ., git restore --worktree ., git checkout HEAD -- . Settings patch below. v2.9.0's git-guardrails denied all three, so this was a regression
2 P2: deny rules miss git add -v -A, git add -v --all, git add file . Settings patch below. Also denied by v2.9.0
3 P2: --abort in a comment or a -m message exempts a real merge Fixed in 6311448. The hook now tokenizes the command like the shell does
4 P2: a quoted 'git pull origin' inside a grep is refused Fixed in 6311448
5 P2: redirections confuse both checks Fixed in 6311448
6 P2: --hard and --force match inside names (feature--force, tracked--hard.txt) Settings patch below. Options are now matched as whole words
7 P2: a missing hook file refuses every command the if filter starts it for (any $VAR, $()) Settings patch below: a launcher that exits 0 when the file is missing
8 P3: the locale test could not fail Fixed in 6311448. It now uses a dirty tree and requires a deny; a locale-decoding mutant fails it

The hook rewrite also fixes a false refusal I hit live in this session. The hook refused one of my own commands because a heredoc's text held a backticked git pull. Heredoc bodies and single-quoted backticks are now skipped.

What backs the fixes in 6311448:

  • Battery cases j17–j29; the battery now has 123 cases.
  • Fifteen mutants, each failing its own cases.
  • The deny list in the patch, checked with a matcher built from the documented rule semantics: 30 commands refused, 24 allowed, 0 wrong.

Holding the merge until the patch below is applied. Then I re-run the live check in a scratch repository and merge. The docs on this branch already describe the patched state.

To apply, from the repository root on branch light-hook-core:

git apply <this patch saved to a file>
--- a/.claude/settings.json
+++ b/.claude/settings.json
@@ -11,18 +11,31 @@
       "Read(**)"
     ],
     "deny": [
-      "Bash(git reset *--hard*)",
+      "Bash(git reset --hard *)",
+      "Bash(git reset * --hard)",
+      "Bash(git reset * --hard *)",
       "Bash(git clean *)",
-      "Bash(git push *--force)",
-      "Bash(git push *--force *)",
-      "Bash(git push -f*)",
-      "Bash(git push * -f*)",
-      "Bash(git add -A*)",
-      "Bash(git add --all*)",
+      "Bash(git push --force *)",
+      "Bash(git push * --force)",
+      "Bash(git push * --force *)",
+      "Bash(git push -f *)",
+      "Bash(git push * -f)",
+      "Bash(git push * -f *)",
+      "Bash(git add -A *)",
+      "Bash(git add * -A)",
+      "Bash(git add * -A *)",
+      "Bash(git add --all *)",
+      "Bash(git add * --all)",
+      "Bash(git add * --all *)",
       "Bash(git add . *)",
+      "Bash(git add * .)",
+      "Bash(git add * . *)",
       "Bash(git checkout -- . *)",
       "Bash(git checkout . *)",
+      "Bash(git checkout * .)",
       "Bash(git restore . *)",
+      "Bash(git restore * .)",
+      "Bash(git restore * . *)",
       "Read(.env)",
       "Read(.env.*)",
       "Read(**/.env)",
@@ -49,19 +62,19 @@
           {
             "type": "command",
             "if": "Bash(git merge *)",
-            "command": "python3 \"$CLAUDE_PROJECT_DIR\"/.claude/hooks/dirty-tree-guard.py",
+            "command": "sh -c '[ -f \"$1\" ] || exit 0; exec python3 \"$1\"' dirty-tree-guard \"$CLAUDE_PROJECT_DIR\"/.claude/hooks/dirty-tree-guard.py",
             "timeout": 10
           },
           {
             "type": "command",
             "if": "Bash(git pull *)",
-            "command": "python3 \"$CLAUDE_PROJECT_DIR\"/.claude/hooks/dirty-tree-guard.py",
+            "command": "sh -c '[ -f \"$1\" ] || exit 0; exec python3 \"$1\"' dirty-tree-guard \"$CLAUDE_PROJECT_DIR\"/.claude/hooks/dirty-tree-guard.py",
             "timeout": 10
           },
           {
             "type": "command",
             "if": "Bash(git rebase *)",
-            "command": "python3 \"$CLAUDE_PROJECT_DIR\"/.claude/hooks/dirty-tree-guard.py",
+            "command": "sh -c '[ -f \"$1\" ] || exit 0; exec python3 \"$1\"' dirty-tree-guard \"$CLAUDE_PROJECT_DIR\"/.claude/hooks/dirty-tree-guard.py",
             "timeout": 10
           }
         ]

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

git-guardrails misses destructive git inside ( … ), $( … ) and backticks

2 participants