mirror of
https://github.com/anthropics/claude-plugins-official.git
synced 2026-06-13 22:26:03 -03:00
security-guidance: detect Graphite (gt) commands as commit/push events (#2048)
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) <noreply@anthropic.com>
This commit is contained in:
parent
502de97746
commit
5212308979
@ -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"
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user