Compare commits

...

1 Commits

Author SHA1 Message Date
Mohamed Hegazy
5212308979 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>
2026-05-28 23:33:14 -07:00
2 changed files with 23 additions and 5 deletions

View File

@@ -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"

View File

@@ -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