Skip to content

chore: block forbidden agent git commands with a PreToolUse hook - #1158

Merged
mforce merged 15 commits into
mainfrom
chore/agent-git-guard
Oct 10, 2026
Merged

mforce merged 15 commits into
mainfrom
chore/agent-git-guard

Conversation

@mforce

@mforce mforce commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Closes #1157

tools/agent-guard/git_guard.py runs before every Bash call in Claude Code and Codex. It blocks the git and gh commands that the rules already forbid agents to run, and tells the agent what to run instead.

How it decides

  1. Text check. It first undoes quotes, backslashes and %-escapes, then asks whether the text mentions git … push|commit, gh … merge|alias, mergePullRequest or AutoMerge. Backslash-newline pairs are removed first, so a continuation inside a word still counts. A $, backtick or glob next to git, gh or a write verb also counts, because it can spell the rest (/usr/bin/gi? push, git $VERB, $'\x70ush'). Braces and parentheses do not count, so @{u}, HEAD@{1}, gh's {owner} placeholders and quoted jq or GraphQL text pass. If none of these match, the command passes untouched. Almost every non-git command takes this path.
  2. Shape. A command that mentions a write must be one simple command, optionally after cd <dir> &&. Newlines, ;, ||, |, &, comments, subshells, braces, if/for/while, heredocs, $(…), backticks, variables, unquoted globs, env assignments and wrappers (env, command, timeout, sudo, bash -c) are refused, not parsed. The message names the allowed forms (git push -u origin <branch>, cd <dir> && git commit -m "…", git commit -F <file> for a long message). Output redirections are refused too, except a trailing 2>&1 or 2>/dev/null.
  3. Directory. cd <dir> && is followed as bash's default cd would (.. by text). An operand that starts with - or ~ is refused, and so is a bare name while CDPATH is set (use ./name or an absolute path). git -C <path> is passed to git's own lookups unchanged, so git resolves symlinks itself.
  4. Rules, applied to that one command:
Blocked Message points to
gh pr merge; a non-GET gh api call to …/pulls/<n>/merge; a literal mergePullRequest/…AutoMerge GraphQL query; gh alias set/import report the PR as ready; only the maintainer merges
a src:dst refspec (HEAD:main, :branch) git branch --show-current, then git push -u origin <that-branch>
--force, -f, +refspec, --mirror, including git's abbreviations (--mir) commit the fix and push normally
--all, --branches, --prune push your own branch
a push whose source is main (named, HEAD/@ on main, or plain git push on main) git switch -c <type>/<topic>
push config that reaches main or forces, on the remote git would pick (positional, branch.<b>.pushRemote, remote.pushDefault, branch.<b>.remote, the sole remote, then origin): push.default=matching; remote.<r>.mirror set to anything but an explicit false; a remote.<r>.push refspec that forces, is the matching refspec :, uses a wildcard or targets main; push.default upstream/tracking with an upstream of main push to a remote without it, or unset the upstream
git -c alias.*|push.*|remote.*|branch.* and unknown git global options run it without the override
git commit with refs/heads/main checked out (read as the full ref, so a tag named main cannot hide it) git switch -c <type>/<topic> first
a failed branch or config lookup, a missing directory, a Bash payload with a missing or non-string command, a missing, empty or non-string cwd, or a non-string or empty tool_name fix it and retry

It allows git push, git push -u origin <current-branch>, --force-with-lease, configured push refspecs to another branch (Paseo PR checkouts use remote.<r>.push = HEAD:refs/heads/<branch>), gh api GET on the merge endpoint (any form: path, full URL, query string), and quoted text in echo/grep.

What it does not see

  • gh or git aliases defined earlier (gh land after an earlier gh alias set land 'pr merge').
  • A GraphQL merge whose query comes from a file (-F query=@q.graphql).
  • Commits made by merge, rebase, cherry-pick or pull. Only git commit is checked.
  • A verb spelled by brace or extglob expansion (git pu{s..s}h, git p@(u)sh). Contrived; counting { and ( as expansions refused common reads (git log @{u}..HEAD, gh api repos/{owner}/{repo}/…), so the table keeps these two as known allows.
  • A configured push destination that the remote resolves as a symbolic ref to main (for example a remote refs/heads/live pointing at main). The guard does no remote lookups.
  • Codex exec_command calls whose workdir differs from the session cwd. Codex leaves workdir out of the hook payload, so the branch is read in the session cwd.
  • A missing script exits 127, which both harnesses treat as a non-blocking error, so it can never block every Bash call.

Harness payloads

Both harnesses send one JSON object on stdin with the shell string in tool_input.command and the session directory in cwd. Both block on exit 2 and pass stderr to the agent.

  • Claude Code. I captured a real payload from 2.1.296 (claude -p --settings with a hook that writes stdin to a file): {"cwd":"/tmp/gg-capture","hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"cd sub && git status --short","description":"…"},…}. Exit 2 behavior is in the hooks reference: "Exit with code 2 to block the action … The message that comes with the block is your hook's stderr."
  • Codex. From source and docs, not a capture. A live capture needed --dangerously-bypass-hook-trust plus a temporary hook entry, and the maintainer declined that run. In codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs, pre_tool_use_payload sends tool_input: {"command": args.cmd} with tool_name Bash, and hook_runtime.rs builds PreToolUseRequest with cwd, turn_id, model and tool_use_id. The Codex hooks docs say: "You can also use exit code 2 and write the blocking reason to stderr."

Notes for review

  • Codex runs the hook only after someone trusts it. Codex stores trust per hook hash, so a new hook entry is skipped until it is approved in /hooks. The graphify entry keeps its index and hash.
  • Agents will notice the shape rule. Chained writes (git add -A && git commit …) and heredoc commit messages are now refused. The agent re-runs the write as its own call, or writes a long message to a file and uses -F. A git or gh read that uses a variable (git -C "$WT" status) is also refused; that false block is deliberate.
  • CI. The table test runs in the classifier-self-test job (no needs, never gated) with the runner's Python 3 and git.

Tests run locally

  • python3 tools/agent-guard/git_guard_test.py: 161 commands, each fed in both payload shapes, plus 23 malformed payloads, another tool's payload, a CDPATH case, and a real git switch -c followed by a separate git commit. All pass.
  • Mutation check: I disabled each rule and parsing step one at a time. Round 1 code at 067a42d8: 47 of 48 mutations turned the table red; the survivor is the git-config read-failure check, which a broken config can never reach because the branch lookup fails first. 149ca647's wider text check: 7 of 7 red. Round 2 checks at 50f9459e: 16 of 16 red (payload fields, redirect suffix, > and bare &, backslash-newline, braces (removed again in 09cea059), cd operands and CDPATH, raw -C, full-ref HEAD, sole-remote fallback, mirror boolean, : refspec, endpoint parsing, GraphQL path). At 09cea059, putting { and ( back turns 18 subtests red, so the new read rows hold the regression.
  • dotnet test filtered to TenancyDocsFreshnessTests and RenameAccountDocsTests (19 passed), plus the ImagePin and CommittedSchemaDocs_CarryNoEnvironmentSpecificContent tracked-file guards (3 passed).

Proof at 09cea059

One blocked command and one allowed push, fed through each harness's configured hook command exactly as written in .claude/settings.json and .codex/hooks.json, with that harness's payload shape (bash -c <hook command>, cwd = worktree, CLAUDE_PROJECT_DIR = worktree).

claude, hook command "$CLAUDE_PROJECT_DIR"/tools/agent-guard/git_guard.py

stdin:  {"session_id": "s", "transcript_path": "/t.jsonl", "cwd": "<worktree>", "permission_mode": "default", "hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": {"command": "git push origin HEAD:main", "description": "push"}, "tool_use_id": "toolu_1"}
exit:   2
stderr: git-guard: `git push origin HEAD:main` is blocked: a src:dst push can land one branch's commits on another branch (#900). Run `git branch --show-current`, then `git push -u origin <that-branch>` (or plain `git push`).

codex, hook command "$(git rev-parse --show-toplevel)"/tools/agent-guard/git_guard.py

stdin:  {"session_id": "s", "turn_id": "t", "cwd": "<worktree>", "transcript_path": null, "model": "gpt-5", "permission_mode": "default", "hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_use_id": "call_1", "tool_input": {"command": "git push origin HEAD:main"}}
exit:   2
stderr: git-guard: `git push origin HEAD:main` is blocked: a src:dst push can land one branch's commits on another branch (#900). Run `git branch --show-current`, then `git push -u origin <that-branch>` (or plain `git push`).

claude, hook command "$CLAUDE_PROJECT_DIR"/tools/agent-guard/git_guard.py

stdin:  {"session_id": "s", "transcript_path": "/t.jsonl", "cwd": "<worktree>", "permission_mode": "default", "hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": {"command": "git push -u origin chore/agent-git-guard", "description": "push"}, "tool_use_id": "toolu_1"}
exit:   0
stderr: <empty>

codex, hook command "$(git rev-parse --show-toplevel)"/tools/agent-guard/git_guard.py

stdin:  {"session_id": "s", "turn_id": "t", "cwd": "<worktree>", "transcript_path": null, "model": "gpt-5", "permission_mode": "default", "hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_use_id": "call_1", "tool_input": {"command": "git push -u origin chore/agent-git-guard"}}
exit:   0
stderr: <empty>
Table test output at 09cea059: 161 commands x 2 payload shapes: {'allow': 82, 'block': 240}, identical verdicts for both shapes
Verdict Session dir Command (Python repr)
allow feature 'git push'
allow feature 'git push -u origin feat/x'
allow feature 'git push -u origin HEAD'
allow feature 'git push --force-with-lease'
allow feature 'git push --force-w origin feat/x'
allow feature 'git push -o ci.skip origin feat/x'
allow feature 'git push paseo'
allow feature 'git push 2>&1'
allow feature 'git push origin feat/x 2>/dev/null'
allow feature "git commit -m 'a > b && c'"
allow feature "git commit -m 'wip'"
allow feature 'git commit -m "subject\n\nbody line"'
allow main 'cd feature && git commit -m x'
allow main 'git -C feature push'
allow main 'git status && git log --oneline -3'
allow main 'git fetch origin main:main'
allow main "git log --grep 'push' --oneline"
allow feature 'gh pr create -R mforce/cluckwork --title x'
allow main 'gh pr view 12 --json state'
allow feature 'gh api repos/mforce/cluckwork/pulls/1158/merge'
allow feature 'gh api -X GET repos/mforce/cluckwork/pulls/1158/merge'
allow feature 'gh api -XGET repos/mforce/cluckwork/pulls/1158/merge'
allow feature "gh api 'repos/mforce/cluckwork/pulls/1158/merge?x=1'"
allow feature "gh api https://api.github.com/graphql -f query='query { viewer { login } }'"
allow main "echo 'git push --force origin HEAD:main'"
allow main "echo ';' git push --force"
allow main "grep -rn 'gh pr merge' docs"
allow main 'ls -la | wc -l'
allow main 'ls *.py | wc -l'
allow tracking 'git rev-parse @{upstream}'
allow tracking 'git log @{u}..HEAD --oneline'
allow tracking 'git show HEAD@{1}'
allow feature 'gh api repos/{owner}/{repo}/pulls/1158'
allow tracking 'git log --format="%h %s (%cr)" | head -20'
allow feature "gh pr view 1158 --json number,title --jq '{number, title}' | cat"
allow feature 'gh api graphql -f query='query { repository(owner:"mforce", name:"cluckwork") { name } }' | cat'
allow main "gh api 'repos/mforce/cluckwork/pulls?state=open' --jq '.[].number'"
block feature 'gh pr merge 12 --squash'
block feature 'gh -R mforce/cluckwork pr merge 12'
block feature 'gh api -X PUT repos/mforce/cluckwork/pulls/12/merge'
block feature 'gh api --method=PUT repos/mforce/cluckwork/pulls/12/merge'
block feature 'gh api -XPUT repos/mforce/cluckwork/pulls/12/merge'
block feature 'gh api repos/mforce/cluckwork/pulls/12/merge -f merge_method=squash'
block feature 'gh api graphql -f query='mutation { mergePullRequest(input: {pullRequestId: "PR_x"}) { clientMutationId } }''
block feature 'gh api https://api.github.com/graphql -f query='mutation { mergePullRequest(input: {pullRequestId: "PR_x"}) { clientMutationId } }''
block feature "gh api /graphql -f query='mutation { mergePullRequest(input: {}) { clientMutationId } }'"
block feature "gh alias set land 'pr merge'"
block feature 'gh alias set land "pr merge" && gh land 1158'
block feature 'gh alias import aliases.yml'
block feature "gh api graphql -f query='mutation { enablePullRequestAutoMerge(input: {}) { clientMutationId } }'"
block feature 'git push origin HEAD:main'
block feature 'git push origin feat/x:feat/y'
block feature 'git push origin :old-branch'
block feature 'git push --force'
block feature 'git push -f origin feat/x'
block feature 'git push -uf origin feat/x'
block feature 'git push origin +feat/x'
block feature 'git push --mirror'
block feature 'git push --all origin'
block feature 'git push --mir origin'
block feature 'git push --al origin'
block feature 'git push --pru origin'
block feature 'git push --forc origin'
block feature 'git push -ofoo origin HEAD:main'
block feature 'git push origin main'
block feature 'git push origin refs/heads/main'
block feature 'git push --delete origin main'
block main 'git push origin @'
block main 'git push'
block main 'git push -u origin HEAD'
block main 'git push origin 2>/dev/null'
block main 'git commit -m x'
block feature 'cd .. && git commit -m x'
block feature 'git -C .. commit -m x'
block feature 'cd new-dir && git push'
block feature 'cd - && git commit -m x'
block feature 'cd -- && git commit -m x'
block main 'cd -P feature && git commit -m x'
block feature 'cd "~" && git commit -m x'
block feature 'cd ~ && git commit -m x'
block feature 'git -C link/.. commit -m x'
block feature 'git -C missing-dir commit -m x'
block plain 'git commit -m x'
block tracking 'git push'
block tracking 'git push -u origin feat/t'
block tracking 'git push origin feat/t'
block tracking 'git push --force-with-lease origin feat/t'
block feature 'git -c remote.origin.push=refs/heads/feat/x:refs/heads/main push -u origin feat/x'
block feature 'git -c remote.origin.push=+refs/heads/feat/x:refs/heads/feat/x push -u origin feat/x'
block feature 'git -c push.default=matching push'
block feature 'git -c remote.origin.mirror=true push'
block feature 'git -c alias.p=push p'
block feature 'git --config-env=push.default=MODE push'
block feature 'git -c remote.origin.pushurl=../other.git push'
block feature 'git push backup'
block feature 'git push mirror'
block matching 'git push'
block solo 'git push'
block feature 'git push matchall'
block feature 'git push mirror2'
allow feature 'git push nomirror'
block tagged 'git commit -m x'
block tagged 'git push'
block feature 'git status &&\ngit push origin HEAD:main'
block feature 'git status;\ngh pr merge 1158'
block feature 'git status\n\ngh pr merge 1158'
block feature 'git status # inspect first\ngit push --force'
block feature 'if true; then git push origin HEAD:main; fi'
block feature 'for branch in main; do git push origin main; done'
block feature '{ git push --force; }'
block feature "bash <<'EOF'\ngh pr merge 1158\nEOF"
block feature 'echo "$(git push --force)"'
block feature 'echo git push --force'
block feature "git commit -F - <<'EOF'\nsubject\nEOF"
block tracking 'git switch main && git commit -m x'
block tracking 'git checkout main && git push'
block main 'git switch -c feat/new && git commit -m x'
block main '(cd feature && git status); git commit -m x'
block main 'cd feature | cat; git commit -m x'
block main 'false && cd feature; git commit -m x'
block feature 'git add -A && git commit -m x'
block feature 'git push 2>&1 | tail -5'
block feature '/usr/bin/env git push --force'
block feature 'command -- gh pr merge 1158'
block feature 'command -p git push --force'
block feature 'timeout --signal TERM 60 git push --force'
block feature 'env -C .. git commit -m x'
block feature 'GIT_DIR=../.git git commit -m x'
block feature "bash -lc 'git push origin feat/x'"
block feature 'git push origin $BRANCH'
block feature 'git push origin \\nHEAD:main'
block main 'git push \\n origin'
block main 'git push # note'
block main 'git status & git commit -m x'
block feature 'git commit -m "unclosed'
block feature 'git pu""sh origin HEAD:main'
block feature "g'i't push --force"
block feature 'git p\ush --force'
block feature "git $'\x70ush' --force"
block feature "gh pr mer''ge 12"
block feature 'git push origin ma?n'
block feature "gh api -X PUT 'repos/mforce/cluckwork/pulls/12/merge?x=1'"
block feature "gh api -X PUT 'repos/mforce/cluckwork/pulls/12/merge#x'"
block feature 'gh api -X PUT repos/mforce/cluckwork/pulls/12/merg%65'
block feature 'gh api -X PUT https://api.github.com/repos/mforce/cluckwork/pulls/12/merge/'
allow feature 'gh api -X PATCH repos/mforce/cluckwork/pulls/12 -f state=open --jq .mergeable'
block feature "rg --pre ./run.sh 'git push' ."
block feature '/usr/bin/gi? push --force'
block feature 'git pu\\nsh --force'
block feature 'g\\nit push --force'
block feature 'gh pr mer\\nge 1158'
block feature 'git com\\nmit -m x'
block feature 'git push 2 > /tmp/push.log main'
block feature 'git push origin 1158 > /tmp/push.log'
block feature 'git push origin feat/x >> /tmp/push.log'
block feature 'git push origin feat/x &> /tmp/push.log'
block feature '$G push --force'
block feature 'git $VERB --force'
block feature 'git -C "$WT" status'
allow feature 'git pu{s..s}h origin HEAD:main'
allow feature 'git p@(u)sh --force'

One guard script runs before every Bash call in Claude Code and Codex. It blocks gh pr merge, src:dst and + refspecs, force pushes, pushes to main and commits on main, and names the command to run instead.
@mforce mforce added this to the Platform hardening milestone Oct 10, 2026
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 (Codex gpt-6-astra, head f6a4e485): 7 product findings, 1 doc finding

Every finding was reproduced by feeding the hook JSON directly. Exit 0 means the command was allowed; nothing was actually pushed or merged.

  1. P1: the tokenizer drops real commands. All of these exit 0:

    • newline-separated chains: git status &&⏎git push origin HEAD:main, git status;⏎gh pr merge 1158;
    • a # comment line before a push;
    • if … then …; fi, for … do …; done and { …; } bodies;
    • bash <<'EOF' heredocs;
    • $(…) and backtick substitutions.

    One false block too: echo ';' git push --force exits 2.

  2. P1: branch and directory state is read once, before the chain runs. git switch main && git commit -m x and git checkout main && git push exit 0. (cd feature && …); git commit and false && cd feature; git commit also exit 0 from main. git switch -c feat/new && git commit exits 2, though it is the recovery the message recommends.

  3. P1: the recommended push can still reach main through config. With an upstream of origin/main and push.default=upstream, git push -u origin feat/t exits 0, and so does --force-with-lease. git -c remote.origin.push=…:refs/heads/main push, -c push.default=matching, -c remote.origin.mirror=true, and a second remote with a HEAD→main mapping also exit 0. A failed @{push} lookup also allows the push.

  4. P2: wrappers. /usr/bin/env git push --force, command -- gh pr merge, command -p git … and timeout --signal TERM 60 git push --force all exit 0. env -C <main-repo> git commit and GIT_DIR=… git commit are checked against the wrong repository.

  5. P2: option forms. git push -ofoo origin HEAD:main treats origin as the -o value. --mir and --al (Git's abbreviations for --mirror and --all) are not recognised.

  6. P2: false block. gh api …/pulls/N/merge with GET is GitHub's read-only "is it merged" check, and exits 2. Only PUT merges.

  7. P2: malformed input fails open. Empty or invalid JSON, {}, a missing or non-string command, and tool_input.cmd all exit 0 without a message. A deeply nested payload exits 1.

  8. P3 (docs): gh alias set land 'pr merge' && gh land N and the GraphQL mergePullRequest mutation are undocumented limits. AGENTS.md should claim only the forms the guard actually covers.

The reviewer confirmed both harness payload contracts against the Claude docs and the Codex source. Both graphify hooks are unchanged.

A command that mentions a git write or PR merge must be one simple command; the guard refuses other shapes instead of parsing them. Pushes resolve the effective remote, push.default, mirror, remote push refspecs and upstream. Short option clusters and long-option prefixes parse like git; gh api allows GET on the merge endpoint; a malformed Bash payload blocks.
The text check undoes quotes, backslashes and %-escapes before matching, and treats $'...' next to git/gh as a write. Unquoted globs are refused. The merge-endpoint match decodes the path and ignores a query or fragment. rg leaves the inert list because --pre runs a program.
A glob or variable can spell git or the verb (/usr/bin/gi? push, $G push, git $VERB), which the text check could not read. Such commands now take the simple-shape path, which refuses them.
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 (Codex gpt-6-astra, head f6a4e485): 7 product findings, 1 doc finding (summary)

Fixed in 4e7dbc5d (guard redesign), 3d12826d (AGENTS.md scope), 067a42d8 (quoting, glob and merge-endpoint differentials found while re-checking) and 149ca647 (a $, backtick or glob beside git or a write verb counts as a write). Head is now 149ca647.

The guard no longer tries to parse bash. If a command's text mentions git push, git commit or a PR merge, the command must be one simple command, optionally after cd <dir> &&. Any other shape is refused with a message that shows the allowed form. Before matching, the text check undoes quotes, backslashes and %-escapes, so pu""sh and merg%65 still count as writes. A $, backtick or glob next to git, gh or a write verb also counts, since it can spell the rest (/usr/bin/gi? push, git $VERB). The cost is a false block on reads such as git -C "$WT" status, with a message saying how to re-run.

  1. Tokenizer (P1). Newlines, ;, ||, |, &, comments, subshells, braces, if/for/while, heredocs, $(…), backticks, variables and unquoted globs are refused whenever the text mentions a write. Every repro is a table row. echo ';' git push --force is now allowed.
  2. Branch state across a chain (P1). The only chain allowed is cd <dir> &&, so the branch is read in the directory where the write runs. Each chain repro is a row. test_switch_then_separate_commit runs git switch -c feat/new for real, then checks that a separate git commit passes.
  3. Config (P1). Each allowed push resolves the remote (positional, branch.<b>.pushRemote, remote.pushDefault, branch.<b>.remote, then origin), push.default, remote.<r>.mirror, remote.<r>.push and the source branch's upstream from git config --list, which includes -c overrides. It blocks matching mode, mirror, a configured push refspec that forces, uses a wildcard or targets main, and push.default upstream/tracking with an upstream of main. -c alias.*|push.*|remote.*|branch.* is refused outright. A failed branch or config lookup blocks. Configured push refspecs that land on another branch stay allowed: Paseo PR checkouts carry remote.<r>.push = HEAD:refs/heads/<branch>, and blocking any configured refspec would stop those normal pushes. The paseo fixture row covers that case.
  4. Wrappers (P2). env, command, timeout, sudo, bash -c and env assignments are refused when the text mentions a write. Only git, gh and a short list of programs that cannot run anything (echo, grep, cat…) pass.
  5. Options (P2). Short clusters stop at -o; its remainder is the value, and the next argument is consumed only when the remainder is empty. Long options resolve git's unambiguous prefixes, so --mir, --al and --pru are blocked, --force-w is allowed, and an ambiguous --forc is refused.
  6. gh api (P2). The effective method decides: -X/--method/-XPUT, or POST when a field or --input is present. GET on …/pulls/N/merge passes. Any other method on a decoded endpoint ending in pulls/<n>/merge (with or without a query, fragment or trailing slash) blocks. A literal mergePullRequest or …AutoMerge GraphQL query also blocks.
  7. Malformed input (P2). Bad JSON, a non-object, a missing or non-string command or cwd, and tool_input.cmd all exit 2 with "unreadable Bash hook payload". The 100,000-deep array does too. A non-Bash event passes.
  8. Docs (P3). AGENTS.md now names only the forms the guard covers, plus what it does not see: gh or git aliases defined earlier, a merge query read from a file, commits made by merge/rebase/cherry-pick/pull, and a Codex workdir. gh alias set itself is now blocked.

Table: 127 commands × 2 payload shapes (Claude, Codex): 56 allow, 198 block, identical verdicts for both shapes, plus the malformed-payload, other-tool and switch-then-commit tests. All pass.

Mutation check: I disabled each rule and parsing step one at a time. At 067a42d8, 47 of 48 mutations turned the table red. The survivor is the git-config read-failure check: a broken config also fails the branch lookup, which blocks first, so no row can tell them apart. Mutating 149ca647's widened text check (7 mutations) turned every one red. Five checks that no mutation could separate from another check were deleted rather than kept: unclosed-quote, unknown short flag, non-object payload, assignment prefix and a redundant endpoint character class.

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 2 (Codex gpt-6-astra, head 149ca647)

All round 1 repros are closed: 40 commands in both formats plus 11 malformed payloads. The table passes: 127 commands × 2 formats.

New findings, each reproduced. The pushes ran only between temporary local repositories, and the GraphQL capture went only to a fake server on this machine.

  1. P1: gh api https://api.github.com/graphql -f query='mutation { mergePullRequest(…) }' exits 0. The GraphQL check matches only the bare graphql endpoint. Normalize full URLs and /graphql.
  2. P1: when only one remote exists and it is not origin, git push falls back to it, but the guard inspects origin. A remote backup with push = HEAD:refs/heads/main updated main. Reproduce Git's single-remote fallback.
  3. P2: remote.origin.push = : (the matching refspec) and remote.origin.mirror = 2 (a valid true) both pass and both update main.
  4. P2: directory resolution differs from what the shell does:
    • With CDPATH set, cd feature && git commit lands in a different repo.
    • git -C link/.. commit is normalized before the symlink is followed.
    • A quoted "~" is expanded, though Bash treats it literally.
    • cd -- is read as a directory, though Bash goes to HOME.
  5. P2: a missing or falsy cwd (null, "", 0, false, [], {}) and a falsy or non-string tool_name still exit 0, because the or os.getcwd() fallback runs before validation.
  6. P2: a numeric argument before > is dropped as a file descriptor. Both git push 2 > log main and git push origin 1158 > log exit 0 and update main.
  7. P2 (contrived): a backslash-newline inside a verb (git pu\⏎sh --force) skips the shape check.
  8. P2 (rare): with a tag also named main, HEAD reads as heads/main, and git commit on main passes.
  9. P2 (contrived): a remote branch that is a symbolic ref to main gets through the remote.<r>.push exception.

The false blocks are as designed: 10 of a 38-command sample of everyday reads were refused (variables, globs, and pipes next to push/commit text).

A Bash payload with a missing, empty or non-string cwd, or a non-string or empty tool_name, now exits 2 instead of falling back to the process directory.
… git guard

A write command may end in exactly 2>&1 or 2>/dev/null; any other output redirection is refused instead of guessing file-descriptor numbers. The text check drops backslash-newline pairs, so a continuation inside git or a verb still counts, and a brace or parenthesis now counts as an expansion (git pu{s..s}h, git p@(u)sh).
… guard

git -C paths go to git's own metadata queries unnormalized, so a symlink followed by .. resolves the way git resolves it. cd refuses option operands, any ~ operand, and, when CDPATH is set, an operand that is neither absolute nor ./ or ../ prefixed.
A push with no remote named falls back to a sole configured remote before origin, as git does. The matching refspec ':' blocks, remote.<name>.mirror uses git's boolean rules (anything but an explicit false blocks), and HEAD is read as its full ref so a tag named main cannot hide the branch.
The endpoint is parsed as a URL and its path decoded, so https://api.github.com/graphql and /graphql get the GraphQL merge check and a REST merge path matches with or without a query, fragment or host.
Without them a missing directory blocked the rows too, so removing the operand refusal stayed green.
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 2 (Codex gpt-6-astra, head 149ca647) (summary)

All nine are fixed or documented. Head is now 50f9459e. Each repro is a table row.

  1. GraphQL URL (P1), eb5ceea2. The gh api endpoint is parsed as a URL and its path decoded before both merge checks. https://api.github.com/graphql and /graphql with mergePullRequest now block, and a GraphQL read on the full URL passes. The REST merge check uses the same path, so a query, fragment or host no longer matters.
  2. Sole remote (P1), 1f399997. With no remote named, the guard picks the remote the way git does: branch.<b>.pushRemote, remote.pushDefault, branch.<b>.remote, the sole configured remote, then origin. New solo fixture: a sole backup remote with push = HEAD:refs/heads/main blocks git push.
  3. : and mirror = 2 (P2), 1f399997. A configured refspec with an empty destination (the matching refspec :) blocks. remote.<r>.mirror blocks unless it is an explicit false (false, no, off, 0, empty). Rows: matchall, mirror2, plus nomirror (mirror = false), which passes.
  4. Directories (P2), 5bfadcfe and 50f9459e. git -C <path> goes to git's own lookups unchanged, so git -C link/.. commit now sees the symlink target's repo on main. cd refuses an operand that starts with - or ~ (cd --, cd -P x, cd "~", cd ~). It also refuses a bare name while CDPATH is set; ./name and absolute paths still work. The -- and ~ rows use real directories with those names, so removing the refusal turns them red.
  5. Payload fields (P2), 1e491293. The os.getcwd() fallback is gone. A missing, empty or non-string cwd, and a non-string or empty tool_name, exit 2 with "unreadable Bash hook payload". All 13 values from the review are in the malformed list.
  6. Redirections (P2), 43a9f0f5. A write command may end in exactly 2>&1 or 2>/dev/null. Any other output redirection is refused, so there is no file-descriptor guessing left. git push 2 > log main and git push origin 1158 > log block. A quoted 'a > b && c' message passes.
  7. Backslash-newline in a word (P2), 43a9f0f5. This cost one line: the text check drops backslash-newline pairs before matching. All four repros block. The same commit adds braces and parentheses to the expansion characters (git pu{s..s}h, git p@(u)sh).
  8. Tag named main (P2), 1f399997. HEAD is read as the full ref and stripped of refs/heads/, so commit and push both see main. The tagged fixture blocks both.
  9. Remote symbolic refs (P2), 01696437. Documented as a limit in the script, AGENTS.md and the PR body. The guard does no remote lookups.

Table: 154 commands × 2 payload shapes: 64 allow, 244 block, with identical verdicts. Also passing: 23 malformed payloads, another tool's payload, a CDPATH case and a real switch-then-commit.

Mutation check on the round 2 checks: 16 of 16 mutations turned the table red. The cd operand mutation first stayed green, because a missing directory also blocked those rows; 50f9459e adds real -- and ~ directories to the fixture.

git_guard.py is 389 lines. The proof and body are updated to 50f9459e.

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 3 (Codex gpt-6-astra, head 50f9459e): one realistic finding

Every round 2 repro is closed, each in both payload formats, and the reviewer confirmed with independent local pushes that the fixtures exercise the real risk. Finding 9 (remote symbolic refs) stays as a documented limit. No new write bypass was found.

  1. P2, realistic: { and ( in EXPANSIONS block common reads.
    • Commands that passed at 149ca647 and fail at 50f9459e: git rev-parse @{upstream}, git log @{u}..HEAD, git show HEAD@{1}, gh api repos/{owner}/{repo}/…, git log --format="%h %s (%cr)" | head, and quoted jq/GraphQL read pipelines.
    • Fix: remove the two characters. The split spellings they caught, pu{s..s}h and p@(u)sh, are contrived and become PR notes, per the maintainer's standing rule. Rolling back only those two characters in memory restored every read listed above, and every verdict in the 38-command sample went back to its round 2 result.

Accepted cost, not a fix request: when push or commit appears anywhere in a command's text, the new redirection rule refuses reads such as git log --grep 'push' > file.

Git reflog and upstream syntax, gh placeholders and quoted jq, format or GraphQL text were refused as possible writes. Brace and extglob spellings of a verb are now a documented limit; backslash-newline handling stays.
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 3 (Codex gpt-6-astra, head 50f9459e): one realistic finding (summary)

Fixed in 09cea059. Head is now 09cea059.

  • { and ( are out of the expansion characters again. The backslash-newline fix stays.
  • New allow rows: git rev-parse @{upstream}, git log @{u}..HEAD --oneline, git show HEAD@{1}, gh api repos/{owner}/{repo}/pulls/1158, git log --format="%h %s (%cr)" | head -20, gh pr view 1158 --json number,title --jq '{number, title}' | cat, and the GraphQL read pipeline from the round 2 sample.
  • git pu{s..s}h … and git p@(u)sh … are now known allows. They sit in a separate "known limits" block of the table and are listed as contrived in the PR body and the script's docstring.
  • Putting { and ( back turns 18 subtests red, so the new rows guard against this regression.

Table: 161 commands × 2 payload shapes: 82 allow, 240 block, with identical verdicts. The malformed-payload, other-tool, CDPATH and switch-then-commit tests pass. The body's limits, proof and table are updated to 09cea059.

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Round 3 fix verified at 09cea059 (coordinator check; no round 4)

  • The delta is the two-character EXPANSIONS rollback, the docstring limit and the new table rows.
  • python3 tools/agent-guard/git_guard_test.py exits 0: 161 commands × 2 payload formats, 82 allow and 240 block.
  • All six reads from round 3 and the GraphQL read pipeline are allowed.
  • In round 2's 38-command false-block sample, 10 are refused, the same 10 as at 149ca647. The regression is gone.

The review loop stops here. Rounds 1–3 confirmed product defects, and every one is fixed or documented. Round 3's only finding was a regression from the round 2 fix, and it is now reverted.

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.

chore: block forbidden agent git commands with a shared PreToolUse hook

1 participant