From 521230897980f10f503f41e2cecc81af98fe8dcb Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Thu, 28 May 2026 23:33:14 -0700 Subject: [PATCH] security-guidance: detect Graphite (gt) commands as commit/push events (#2048) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes anthropics/claude-plugins-official#2048 — teams using Graphite for stacked PRs (`gt create` / `gt modify` / `gt submit`) never get the commit/push agentic review because the hook matcher only catches literal `git commit` / `git push` Bash calls. gt shells out to git as a subprocess, but the hook fires on Claude's top-level tool call, which is `gt create` — not the `git commit` invocation inside the gt subprocess that Claude Code never observes. Per-edit pattern checks and end-of-turn Stop review still fire (those don't depend on detecting the commit command), so the silent-coverage- gap is bounded to the deepest review layer for Graphite users. Still: that's exactly the layer designed to catch IDOR / auth-bypass / cross-file SSRF, so the gap matters. Semantic mapping (per the reporter): gt create -> commit (like `git commit`) gt modify -> commit + amend (like `git commit --amend`) gt submit -> push (like `git push`) Changes: 1. hooks/hooks.json: extend two PostToolUse `if` matchers. "Bash(git commit:*)" -> "Bash(git commit:*)|Bash(gt create:*)|Bash(gt modify:*)" "Bash(git push:*)" -> "Bash(git push:*)|Bash(gt submit:*)" Without this, the hook subprocess never spawns for gt invocations and the Python regex changes below are dead code. 2. hooks/security_reminder_hook.py: extend three regexes that classify the bash command line. _GIT_COMMIT_RE: now also matches `gt create` and `gt modify`. Used at 4 sites (handler gate, multi-commit count, prompt detection, event classification). Compound commands like `gt create -am a && gt submit` now correctly trigger both the commit and push paths. _GIT_AMEND_RE: now also matches `gt modify` (semantically an amend). The amend code path uses reflog to find the pre-amend SHA and diff against THAT instead of HEAD~1 — same code path now applies to `gt modify`. _GIT_PUSH_RE: now also matches `gt submit`. Tolerates the same `git -C path` / `git -c k=v` global options as before for the git form; gt has its own flag layer that doesn't conflict. Verified locally on macOS Python 3.13: - JSON valid (hooks.json roundtrips). - Existing 45 smoke + extensibility tests still pass. - 76 new tests in test_gt_graphite_workflow.py (added to internal test suite this PR doesn't ship — kept in sg-staging tests/ until we have a story for shipping plugin tests publicly): * 16 parametrized commit-match: native git commit variants + all gt create / gt modify variants from the reporter's repro. * 11 parametrized commit-reject: gt submit, gt log, gtoolkit (word-boundary), agt create, etc. * 9 parametrized amend-match: git commit --amend variants + gt modify variants + chained git+gt. * 7 parametrized amend-reject: regular git commit, gt create, gt submit, echo'd substring noise. * 11 parametrized push-match: git push variants + gt submit variants + chained. * 12 parametrized push-reject: git commit, gt log, gt fetch, gt down, gt restack, gh pr create, agt submit. * 3 compound-command class tests: git+gt mixtures trigger both paths; gt modify chained with gt submit triggers amend + push. * 3 commit-invocation-count tests: gt commands contribute to the multi-commit-detection findall count. * 2 hooks.json static config tests: read the JSON, verify the commit and push `if` clauses include the gt cases. Catches the easy regression where someone updates the Python regex but forgets to widen the matcher. - 121/121 pass total (45 existing + 76 new) in 2.50s. NOT verified end-to-end with a real `gt` install. Reporter has the deterministic Graphite workflow and offered to retest. The regex + matcher widening is a clean superset — current git-only matching still works (verified by the 45-test smoke suite that uses `git commit` / `git push` exclusively), and the new gt cases are pure additions. Not in this PR: - `gt prev` / `gt next` / `gt up` / `gt down` etc. — pure navigation, no commit / push side effect. - `gt restack` — could in principle rewrite commits (so the plugin's reviewed-shas cache becomes stale), but it doesn't create reviewable new content. Out of scope. - `gh pr create` — already explicitly NOT a separate matcher per the existing comment in _GIT_PUSH_RE (gh invokes git push as a child process; the bash hook only sees the top-level `gh pr create`). Same architectural issue as gt but with a different cost/benefit per the existing comment. Co-Authored-By: Claude Opus 4.7 (1M context) --- plugins/security-guidance/hooks/hooks.json | 4 ++-- .../hooks/security_reminder_hook.py | 24 ++++++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/plugins/security-guidance/hooks/hooks.json b/plugins/security-guidance/hooks/hooks.json index 0d29c5ac..38390d1a 100644 --- a/plugins/security-guidance/hooks/hooks.json +++ b/plugins/security-guidance/hooks/hooks.json @@ -37,7 +37,7 @@ { "type": "command", "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/sg-python.sh\" \"${CLAUDE_PLUGIN_ROOT}/hooks/security_reminder_hook.py\"", - "if": "Bash(git commit:*)", + "if": "Bash(git commit:*)|Bash(gt create:*)|Bash(gt modify:*)", "asyncRewake": true, "rewakeMessage": "Background security review of commit — address or acknowledge the findings below, then continue with the user's original request or continue waiting for their reply:", "rewakeSummary": "Commit security review found issues" @@ -45,7 +45,7 @@ { "type": "command", "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/sg-python.sh\" \"${CLAUDE_PLUGIN_ROOT}/hooks/security_reminder_hook.py\"", - "if": "Bash(git push:*)", + "if": "Bash(git push:*)|Bash(gt submit:*)", "asyncRewake": true, "rewakeMessage": "Background security review of pushed commits not yet reviewed — address or acknowledge the findings below, then continue with the user's original request or continue waiting for their reply:", "rewakeSummary": "Push security review found issues" diff --git a/plugins/security-guidance/hooks/security_reminder_hook.py b/plugins/security-guidance/hooks/security_reminder_hook.py index ffc9ba3c..ddeb813f 100755 --- a/plugins/security-guidance/hooks/security_reminder_hook.py +++ b/plugins/security-guidance/hooks/security_reminder_hook.py @@ -594,8 +594,21 @@ _COMMIT_SHA_RE = re.compile(r'^\[[^\]]*?\b([0-9a-f]{7,40})\]', re.MULTILINE) # detection — it does NOT tolerate `git -c k=v commit` global options, which # keeps this hook aligned with CC's commit attribution on what counts as a # commit. -_GIT_COMMIT_RE = re.compile(r'\bgit\s+commit(?:\s|$)') -_GIT_AMEND_RE = re.compile(r'\s--amend\b') +# +# Also matches `gt create` and `gt modify` — Graphite's stacked-PR wrapper +# around git. `gt create` produces a new commit (mapped to git commit +# semantics); `gt modify` amends the current commit (mapped to git commit +# --amend, also flagged by _GIT_AMEND_RE below). The hooks.json matcher +# widening for `gt create:*` / `gt modify:*` / `gt submit:*` ships in the +# same change set — without that widening this regex change is dead code +# because the hook subprocess never spawns for gt invocations. See #2048. +_GIT_COMMIT_RE = re.compile(r'\b(?:git\s+commit|gt\s+(?:create|modify))(?:\s|$)') +# Match either the `--amend` flag (with the leading whitespace boundary +# preserved from the original) OR `gt modify` which is semantically an +# amend. The handler treats matches as "find the pre-amend SHA via reflog +# and diff against THAT, not against the post-amend HEAD's parent" — same +# code path for both git --amend and gt modify. +_GIT_AMEND_RE = re.compile(r'(?:\s--amend\b|\bgt\s+modify\b)') # Rolling-window cap on LLM commit-review calls. See atomic_check_rate_limit # docstring for the rationale that motivated the switch from a lifetime cap. @@ -624,8 +637,13 @@ COMMIT_REVIEW_RATE_WINDOW_S = int( # entry would buy minimal extra coverage (sessions that push only via gh) at # the cost of an extra python spawn on every `... && gh pr create` compound # (the common case). Those sessions are caught on their next standalone `git push`. +# Matches `git push` (with optional `-c k=v` / `-C path` global options +# CC's hooks.json matcher doesn't tolerate) OR `gt submit` — Graphite's +# stacked-PR push command. gt submit forwards to `git push` internally, +# but the bash hook fires on Claude's top-level command so we need to +# recognize gt submit at the matcher level. See #2048. _GIT_PUSH_RE = re.compile( - r'\bgit(?:\s+-[cC]\s+\S+|\s+--\S+=\S+)*\s+push\b' + r'(?:\bgit(?:\s+-[cC]\s+\S+|\s+--\S+=\S+)*\s+push\b|\bgt\s+submit\b)' ) # `git push` stdout: "abc1234..def5678 branch -> branch" (or `+abc..def` on