From 960d25aecba2b5113b3a7a6c9c3db9b71923c751 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 14:14:06 -0700 Subject: [PATCH 01/13] Guard against agent git operations that require a branch-rule bypass The write-guard permitted a direct commit to a protected branch when the authenticated user can bypass the rule, so a plain-looking update silently lands on develop or main under admin bypass. Add Rule 4: inspect the target branch's live rules and deny an operation that would only succeed by bypassing one - a direct update where a pull request is required, a force where history is protected, a delete where deletion is blocked - plus the explicit-bypass flags (gh pr merge --admin, --no-verify). Code-style and config-style develop are told apart by the live rules, not a hardcoded list; the protected-default branches fail closed when the rules cannot be read. Each denial names the bypassed rule and hands the command to the maintainer. 44/44 self-test cases pass; verified end-to-end against live branch rules. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 244 +++++++++++++++++++++- 1 file changed, 235 insertions(+), 9 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index 5ce846e1..3887f9d4 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -5,14 +5,21 @@ classifies the command, and DENIES (with a reason shown to the agent) when a command is a GitHub *write* matching a known-dangerous pattern. Reads and everything that is not a clear write pass through. -Precision over recall by design: it denies the specific shapes that caused the incident, not everything -it cannot parse. A false deny would break the agent, while a missed case still falls under the AGENTS.md -"Repository Boundaries and Write Safety" prose rules. The three denied shapes: +Precision over recall for the write-footgun shapes (1-3): they deny the specific shapes that caused the +incident, not everything unparseable - a false deny would break the agent, and a miss still falls under +the AGENTS.md "Repository Boundaries and Write Safety" prose rules. The branch-bypass rule (4) instead +fails CLOSED on the protected-by-default branches, because the harm there is a silent success under the +maintainer's admin bypass. The denied shapes: 1. a state-changing gh call whose output is discarded or forced to success (>/dev/null, 2>/dev/null, &>/dev/null, || true, || :, || echo) 2. a GraphQL mutation passing a literal GitHub node id (PRRT_/PR_/BOT_/...) instead of a $variable 3. a gh write with an explicit -R/--repo/repos// target outside the checkout's origin + 4. a git operation that would only land by bypassing an active branch rule: a direct push to a branch + whose rules require a pull request, a force-push where history is protected, a delete where deletion + is blocked, or an explicit-bypass flag (`gh pr merge --admin`, `git commit/push --no-verify`). The + branch's live rules are the judge, so a code-style develop is denied and a config-style develop is + allowed with no hardcoded repo list. Run `gh-write-guard.py --selftest` to verify the decision matrix without Claude Code. """ @@ -42,6 +49,23 @@ _GRAPHQL = re.compile(r"\bgh\s+api\b.*\bgraphql\b", re.S) _MUTATION = re.compile(r"\bmutation\b") _GIT_PUSH = re.compile(r"\bgit\s+push\b") +_GIT_COMMIT = re.compile(r"\bgit\s+commit\b") + +# --- Bypass-of-branch-rule detectors (Rule 4) -------------------------------------------------------- +# A git operation is denied when it would only succeed by bypassing an active branch rule - the harm is +# that the maintainer's admin identity CAN bypass, so a plain-looking push silently lands on a protected +# branch. The judgement is made against the branch's *live* rules (self-configuring: a code-style develop +# carries `pull_request` and is denied, a config-style develop does not and is allowed), except for the +# explicit-bypass flags below, which are the bypass by definition and need no query. +# +# Branches that fail CLOSED when their rules cannot be read - protected-by-default across every config. +_PROTECTED_DEFAULT = {"main", "master", "develop"} +# `gh pr merge --admin` overrides required reviews/status checks with admin power. +_GH_ADMIN_MERGE = re.compile(r"\bgh\s+pr\s+merge\b[^\n|&;]*(?:^|\s)--admin\b") +# `--no-verify` skips the local git hooks (signing / lint / pre-push gates). `git commit` also spells it +# `-n`; `git push -n` means --dry-run, so the short form is a bypass only for commit. +_NO_VERIFY_LONG = re.compile(r"(?:^|\s)--no-verify\b") +_COMMIT_SHORT_N = re.compile(r"(?:^|\s)-n\b") # --- Risk-pattern detectors -------------------------------------------------------------------------- # Output-discard / force-success tails. Bare `2>&1` is NOT here: it merges stderr into stdout, leaving @@ -88,12 +112,176 @@ def _origin_owner_repo(cwd): return (m.group(1).lower(), m.group(2).lower()) if m else None -def classify(cmd, cwd=None, origin=None): +def _live_branch_rules(owner, repo, branch): + """Return the set of active rule types on a branch, or None if the query cannot be resolved. + + None (not an empty set) signals "unknown" so the caller can fail closed on a protected-default + branch. An empty set means the branch genuinely has no rules (a feature branch). + """ + try: + r = subprocess.run( + ["gh", "api", f"repos/{owner}/{repo}/rules/branches/{branch}", "--jq", "[.[].type]"], + capture_output=True, text=True, timeout=10, + ) + except Exception: + return None + if r.returncode != 0: + return None + try: + return set(json.loads(r.stdout or "[]")) + except Exception: + return None + + +def _current_push_branch(cwd): + """Resolve the destination branch of a bare `git push` from the branch's configured push target.""" + for args in ( + ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{push}"], + ["rev-parse", "--abbrev-ref", "HEAD"], + ): + try: + r = subprocess.run(["git", "-C", cwd or ".", *args], capture_output=True, text=True, timeout=5) + except Exception: + return None + ref = r.stdout.strip() + if r.returncode == 0 and ref and ref != "HEAD": + return ref.split("/", 1)[1] if "/" in ref else ref + return None + + +# Flags that consume the following token as a value, so the value is not a positional (remote/refspec). +_PUSH_VALUE_FLAGS = {"-o", "--push-option", "--repo", "--receive-pack", "--exec"} + + +def _push_targets(cmd, cwd=None, current_branch=None): + """Parse a `git push` invocation into (op, [branches]). + + op is 'delete' | 'force' | 'update'. current_branch, when given, stands in for the git resolution + of a bare push (the self-test passes it for an offline run). + """ + m = _GIT_PUSH.search(cmd) + seg = cmd[m.end():] + seg = re.split(r"&&|\|\||[;|\n]", seg)[0] + toks = seg.split() + force = delete = False + positionals = [] + i = 0 + while i < len(toks): + t = toks[i] + if t in ("--force", "-f") or t.startswith("--force-with-lease"): + force = True + elif t in ("--delete", "-d"): + delete = True + elif t in _PUSH_VALUE_FLAGS: + i += 1 # skip this flag's value + elif t.startswith("-"): + pass # some other flag (e.g. -u, --tags, --no-verify) + else: + positionals.append(t) + i += 1 + # positionals are [remote, refspec...]; a lone positional is the remote (a bare push). + refspecs = positionals[1:] if len(positionals) >= 2 else [] + branches = [] + for rs in refspecs: + if rs.startswith("+"): + force = True + rs = rs[1:] + if rs.startswith(":"): + delete = True # `:dst` empty-source refspec deletes dst + dst = rs.split(":", 1)[1] if ":" in rs else rs + if dst.startswith("refs/heads/"): + dst = dst[len("refs/heads/"):] + elif dst.startswith("refs/"): + continue # a tag or other non-branch ref + if dst: + branches.append(dst) + if not refspecs and not delete: + b = current_branch if current_branch is not None else _current_push_branch(cwd) + if b: + branches = [b] + op = "delete" if delete else ("force" if force else "update") + return op, branches + + +def _handoff(cmd): + return ( + " The agent must not bypass this - if the bypass is genuinely intended, hand the exact command " + "to Pieter to run in his terminal. See AGENTS.md 'Repository Boundaries and Write Safety' and the " + "Branching Model." + ) + + +def _check_bypass_flags(cmd): + """Deny the explicit-bypass flags: they are a bypass by definition, no branch query needed.""" + bare = _QUOTED_SPAN.sub("", cmd) # a flag mentioned inside a quoted message/body is not a real flag + if _GH_ADMIN_MERGE.search(bare): + return "deny", ( + "This uses `gh pr merge --admin`, which merges past required reviews and status checks using " + "admin power - a bypass of the merge gate. Merge only when the gate is satisfied." + _handoff(cmd) + ) + if _NO_VERIFY_LONG.search(bare) or (_GIT_COMMIT.search(bare) and _COMMIT_SHORT_N.search(bare)): + return "deny", ( + "This uses --no-verify, which skips the git hooks (signing, lint, and pre-push gates). " + "Skipping verification is a bypass; run the command without it." + _handoff(cmd) + ) + return "allow", "" + + +def _check_push_bypass(cmd, cwd, origin, current_branch=None, rules_lookup=None): + """Deny a git push that would only succeed by bypassing an active branch rule.""" + if origin is None: + origin = _origin_owner_repo(cwd) + op, branches = _push_targets(cmd, cwd, current_branch) + for br in branches: + rules = rules_lookup(br) if rules_lookup is not None else ( + _live_branch_rules(origin[0], origin[1], br) if origin else None + ) + if rules is None: + if br in _PROTECTED_DEFAULT: + return "deny", ( + f"Could not read the branch rules for '{br}', a protected-by-default branch " + f"(main/master/develop). Failing closed rather than risk a silent bypass - retry when " + f"the API is reachable." + _handoff(cmd) + ) + continue # an unknown-rules feature/other branch: nothing to bypass, let it through + if op == "update" and "pull_request" in rules: + return "deny", ( + f"This is a direct push to '{br}', whose branch rules require a pull request " + f"(rule: pull_request); it only lands by bypassing that rule with admin power. Use the " + f"protocol path - commit on a feature branch and open a PR (feature -> squash -> develop, " + f"or develop -> merge -> main)." + _handoff(cmd) + ) + if op == "force" and (rules & {"non_fast_forward", "required_linear_history"}): + return "deny", ( + f"This force-pushes '{br}', whose rules forbid rewriting history " + f"(rule: non_fast_forward/required_linear_history). Never force-push a protected branch; " + f"land changes as follow-up commits." + _handoff(cmd) + ) + if op == "delete" and "deletion" in rules: + return "deny", ( + f"This deletes '{br}', whose rules forbid deletion (rule: deletion)." + _handoff(cmd) + ) + return "allow", "" + + +def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None): """Return (decision, reason). decision is 'allow' or 'deny'. origin, when given, is a (owner, repo) tuple used instead of resolving from cwd - the self-test - passes it for a deterministic, offline run. + passes it for a deterministic, offline run. current_branch and rules_lookup are likewise test seams: + current_branch stands in for the git resolution of a bare push, and rules_lookup(branch) stands in + for the live branch-rules query. """ + # Rule 4: a git operation that would only succeed by bypassing an active branch rule. Checked before + # the gh-write gate below, since `git commit --no-verify` is a bypass yet not a GitHub write. + dec, reason = _check_bypass_flags(cmd) + if dec == "deny": + return dec, reason + if _GIT_PUSH.search(cmd): + dec, reason = _check_push_bypass(cmd, cwd, origin, current_branch, rules_lookup) + if dec == "deny": + return dec, reason + if not _is_gh_write(cmd): return "allow", "" @@ -163,7 +351,6 @@ def classify(cmd, cwd=None, origin=None): ("gh api graphql -f query='{repository(owner:\"o\",name:\"r\"){pullRequest(number:1){reviewThreads(first:100){nodes{id}}}}}'", "allow", "graphql READ query"), ("gh pr view 5 --json reviews", "allow", "gh pr view (read)"), ("return 1 2>/dev/null || exit 1", "allow", "shell guard, not a gh write"), - ("git push origin develop", "allow", "normal push (no suppression, no cross-repo)"), ("git commit -m 'x' && git push >/dev/null 2>&1", "deny", "push with discarded output"), ("gh issue comment 5 --body x 2>&1 | tee out.log", "allow", "bare 2>&1 piped to tee is not suppression"), ("gh pr comment 5 --body ok 2>&1", "allow", "bare 2>&1 leaves output visible"), @@ -178,14 +365,53 @@ def classify(cmd, cwd=None, origin=None): ("gh api graphql -f query='mutation{resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"TODO_fixit\"", "allow", "short all-caps token is not a node id"), ] +# Rule-4 (branch-rule bypass) cases. Each carries its own branch->rules map so the run is deterministic +# and offline - the real hook queries the live rules, here rules_lookup is injected. current_branch +# stands in for the git resolution of a bare push. `None` rules mean the query could not be read. +_CODE_RULES = {"deletion", "non_fast_forward", "required_linear_history", "required_signatures", + "pull_request", "required_status_checks", "copilot_code_review"} # code-style develop / any main +_CONFIG_RULES = {"deletion", "non_fast_forward", "required_signatures"} # config-style develop: no pull_request +_GIT_CASES = [ + # (command, current_branch, {branch: rules_set_or_None}, expected_decision, label) + ("git push origin develop", None, {"develop": _CODE_RULES}, "deny", "code-style develop: direct push bypasses pull_request"), + ("git push origin develop", None, {"develop": _CONFIG_RULES}, "allow", "config-style develop: no pull_request rule, direct push allowed"), + ("git push origin main", None, {"main": _CODE_RULES}, "deny", "main: direct push bypasses pull_request"), + ("git push origin feature/x", None, {"feature/x": set()}, "allow", "feature branch: no rules, allowed"), + ("git push -u origin feature/x", None, {"feature/x": set()}, "allow", "feature branch with -u: allowed"), + ("git push origin HEAD:develop", None, {"develop": _CODE_RULES}, "deny", "HEAD:develop refspec resolves to develop"), + ("git push origin abc1234:refs/heads/main", None, {"main": _CODE_RULES}, "deny", "sha:refs/heads/main resolves to main"), + ("git push", "develop", {"develop": _CODE_RULES}, "deny", "bare push resolving to develop"), + ("git push", "feature/x", {"feature/x": set()}, "allow", "bare push resolving to a feature branch"), + ("git push --force origin develop", None, {"develop": _CONFIG_RULES}, "deny", "force-push denied by non_fast_forward even on config develop"), + ("git push --force-with-lease origin feature/x", None, {"feature/x": set()}, "allow", "force-with-lease to a ruleless feature branch"), + ("git push origin +HEAD:develop", None, {"develop": _CODE_RULES}, "deny", "+refspec is a force-push to develop"), + ("git push --delete origin develop", None, {"develop": _CODE_RULES}, "deny", "delete develop denied by deletion rule"), + ("git push origin :develop", None, {"develop": _CODE_RULES}, "deny", "empty-source :develop is a delete"), + ("git push origin develop", None, {"develop": None}, "deny", "develop with unreadable rules: fail closed"), + ("git push origin feature/x", None, {"feature/x": None}, "allow", "feature branch with unreadable rules: fail open"), + ("git commit --no-verify -m x", None, {}, "deny", "commit --no-verify skips the hooks"), + ("git commit -n -m x", None, {}, "deny", "commit -n is --no-verify"), + ("git push --no-verify origin feature/x", None, {"feature/x": set()}, "deny", "push --no-verify is a bypass even on a feature branch"), + ("git push -n origin develop", None, {"develop": _CODE_RULES}, "deny", "push -n is dry-run not no-verify, but the direct push to develop still denies"), + ("gh pr merge 5 --admin --squash", None, {}, "deny", "gh pr merge --admin overrides the merge gate"), + ("git commit -m 'mention --no-verify in the message'", None, {}, "allow", "--no-verify inside a quoted message is not a flag"), +] + def _selftest(): - # Deterministic offline run: pin origin to ptr727/PlexCleaner (the incident repo) so the - # cross-origin case resolves without touching a real checkout. + # Deterministic offline run: pin origin to ptr727/PlexCleaner (the incident repo) so the cross-origin + # case resolves without touching a real checkout. The gh-write cases inject empty rules + a feature + # current-branch so no case reaches the live branch-rules query. origin = ("ptr727", "plexcleaner") ok = True for cmd, want, label in _CASES: - got, _ = classify(cmd, origin=origin) + got, _ = classify(cmd, origin=origin, current_branch="feature/x", rules_lookup=lambda br: set()) + mark = "ok " if got == want else "FAIL" + if got != want: + ok = False + print(f" {mark} [{got:5}] want={want:5} {label}") + for cmd, cur, rmap, want, label in _GIT_CASES: + got, _ = classify(cmd, origin=origin, current_branch=cur, rules_lookup=lambda br, _m=rmap: _m.get(br)) mark = "ok " if got == want else "FAIL" if got != want: ok = False From 3efc519276c3f6e9d2ace2ae0bb8904392b8df6e Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 14:24:21 -0700 Subject: [PATCH 02/13] Stop Rule 4 parsing at redirections and fold line-continuations Copilot review of #449 surfaced two parser gaps: - _push_targets tokenized the segment with split(), so a bare push with a redirect (... >push.log 2>&1) read the redirect as the remote and refspec and skipped the current-branch resolution, missing a protected target. Stop parsing at the first redirection token (> or <). - _GH_ADMIN_MERGE excluded newlines, so a backslash-newline continued admin merge slipped past. Fold backslash-newline continuations to spaces in classify before the bypass checks run. Two self-test cases added for each gap; 46/46 pass. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index 3887f9d4..0eae0984 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -168,6 +168,8 @@ def _push_targets(cmd, cwd=None, current_branch=None): i = 0 while i < len(toks): t = toks[i] + if ">" in t or "<" in t: + break # a redirection operator (>, 2>, >>, <, 2>&1): end of git argv, start of shell syntax if t in ("--force", "-f") or t.startswith("--force-with-lease"): force = True elif t in ("--delete", "-d"): @@ -272,6 +274,10 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) current_branch stands in for the git resolution of a bare push, and rules_lookup(branch) stands in for the live branch-rules query. """ + # Fold shell line-continuations so a multi-line Bash invocation (`gh pr merge 5 \ --admin`) + # parses as one command; only backslash-newline is joined, so a real newline between commands still + # separates them. + cmd = re.sub(r"\\\r?\n", " ", cmd) # Rule 4: a git operation that would only succeed by bypassing an active branch rule. Checked before # the gh-write gate below, since `git commit --no-verify` is a bypass yet not a GitHub write. dec, reason = _check_bypass_flags(cmd) @@ -394,7 +400,10 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("git push --no-verify origin feature/x", None, {"feature/x": set()}, "deny", "push --no-verify is a bypass even on a feature branch"), ("git push -n origin develop", None, {"develop": _CODE_RULES}, "deny", "push -n is dry-run not no-verify, but the direct push to develop still denies"), ("gh pr merge 5 --admin --squash", None, {}, "deny", "gh pr merge --admin overrides the merge gate"), + ("gh pr merge 5 \\\n --admin --squash", None, {}, "deny", "line-continued gh pr merge --admin still caught"), ("git commit -m 'mention --no-verify in the message'", None, {}, "allow", "--no-verify inside a quoted message is not a flag"), + ("git push >push.log 2>&1", "develop", {"develop": _CODE_RULES}, "deny", "redirection tokens are not a branch: bare push to develop still denies"), + ("git push origin develop >push.log 2>&1", None, {"develop": _CODE_RULES}, "deny", "redirect after a real refspec does not hide the develop target"), ] From 7d4642fa119eb5cd7c86737908e8d58dce5aad95 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 14:28:54 -0700 Subject: [PATCH 03/13] Ignore a push mentioned inside a quoted argument Copilot review of #449: the push-bypass gate scanned the raw command, so a `git push ...` appearing only inside a quoted --body/--message (for example a gh issue comment that documents a command) tripped the check and could falsely deny. Run the gate and the target parse on the command with quoted spans removed, matching how the suppression and bypass-flag scans already treat quoted text as non-executable. Self-test case added. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index 0eae0984..1869049a 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -283,8 +283,11 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) dec, reason = _check_bypass_flags(cmd) if dec == "deny": return dec, reason - if _GIT_PUSH.search(cmd): - dec, reason = _check_push_bypass(cmd, cwd, origin, current_branch, rules_lookup) + # Scan for the push on the command with quoted argument values removed, so a `git push ...` that only + # appears inside a --body/--message is text, not an executed push (the pattern the other scans use). + cmd_unquoted = _QUOTED_SPAN.sub("", cmd) + if _GIT_PUSH.search(cmd_unquoted): + dec, reason = _check_push_bypass(cmd_unquoted, cwd, origin, current_branch, rules_lookup) if dec == "deny": return dec, reason @@ -402,6 +405,7 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("gh pr merge 5 --admin --squash", None, {}, "deny", "gh pr merge --admin overrides the merge gate"), ("gh pr merge 5 \\\n --admin --squash", None, {}, "deny", "line-continued gh pr merge --admin still caught"), ("git commit -m 'mention --no-verify in the message'", None, {}, "allow", "--no-verify inside a quoted message is not a flag"), + ("gh issue comment 5 --body \"run: git push origin develop\"", None, {"develop": _CODE_RULES}, "allow", "a git push mentioned inside a quoted body is not an executed push"), ("git push >push.log 2>&1", "develop", {"develop": _CODE_RULES}, "deny", "redirection tokens are not a branch: bare push to develop still denies"), ("git push origin develop >push.log 2>&1", None, {"develop": _CODE_RULES}, "deny", "redirect after a real refspec does not hide the develop target"), ] From 5eb5b27559ebe1ac671bbc7b9162de95c4ec6150 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 14:37:05 -0700 Subject: [PATCH 04/13] Tokenize with shlex, encode the branch, generic maintainer wording Third Copilot round on #449: - _push_targets hand-parsed quotes: a quoted refspec ('HEAD:develop') kept a trailing quote and a fully-quoted refspec was lost when spans were removed upstream. Tokenize with shlex and key off a real git-push argv adjacency, so a quoted refspec is unquoted cleanly and a push named only inside a quoted body forms no adjacency (no target). Removes the span-removal workaround. - _live_branch_rules built the API path from the raw branch name, so a name with a slash (feature/x) split the path and the lookup failed to None. URL encode the branch. - _handoff hardcoded a personal name; use the generic maintainer wording. Self-test now 49 cases (quoted refspec, quoted-mention-before-real-push); verified live: slashed-branch lookup returns a set, quoted refspec denies. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 40 +++++++++++++++-------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index 1869049a..fd586a7f 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -26,8 +26,10 @@ import json import os import re +import shlex import subprocess import sys +from urllib.parse import quote # --- What counts as a GitHub write ------------------------------------------------------------------- # gh subcommands that mutate. `gh api` is handled separately (it needs field/method inspection). @@ -120,7 +122,8 @@ def _live_branch_rules(owner, repo, branch): """ try: r = subprocess.run( - ["gh", "api", f"repos/{owner}/{repo}/rules/branches/{branch}", "--jq", "[.[].type]"], + # quote the branch: a name with `/` (feature/x) would otherwise split the API path. + ["gh", "api", f"repos/{owner}/{repo}/rules/branches/{quote(branch, safe='')}", "--jq", "[.[].type]"], capture_output=True, text=True, timeout=10, ) except Exception: @@ -159,13 +162,23 @@ def _push_targets(cmd, cwd=None, current_branch=None): op is 'delete' | 'force' | 'update'. current_branch, when given, stands in for the git resolution of a bare push (the self-test passes it for an offline run). """ - m = _GIT_PUSH.search(cmd) - seg = cmd[m.end():] - seg = re.split(r"&&|\|\||[;|\n]", seg)[0] - toks = seg.split() + # shlex tokenizes the way the shell does: a quoted refspec (`'HEAD:develop'`) becomes a clean token, + # and a `git push ...` mentioned inside a quoted --body stays a single token, so it never forms the + # `git` `push` argv adjacency below. Fall back to a naive split only if the quoting is unbalanced. + try: + toks = shlex.split(cmd, posix=True) + except ValueError: + toks = cmd.split() + start = None + for k in range(len(toks) - 1): + if toks[k] == "git" and toks[k + 1] == "push": + start = k + 2 + break + if start is None: + return "update", [] # no executable `git push` (only a quoted mention, or `git -C ... push`) force = delete = False positionals = [] - i = 0 + i = start while i < len(toks): t = toks[i] if ">" in t or "<" in t: @@ -208,8 +221,8 @@ def _push_targets(cmd, cwd=None, current_branch=None): def _handoff(cmd): return ( " The agent must not bypass this - if the bypass is genuinely intended, hand the exact command " - "to Pieter to run in his terminal. See AGENTS.md 'Repository Boundaries and Write Safety' and the " - "Branching Model." + "to the maintainer to run in their terminal. See AGENTS.md 'Repository Boundaries and Write " + "Safety' and the Branching Model." ) @@ -283,11 +296,10 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) dec, reason = _check_bypass_flags(cmd) if dec == "deny": return dec, reason - # Scan for the push on the command with quoted argument values removed, so a `git push ...` that only - # appears inside a --body/--message is text, not an executed push (the pattern the other scans use). - cmd_unquoted = _QUOTED_SPAN.sub("", cmd) - if _GIT_PUSH.search(cmd_unquoted): - dec, reason = _check_push_bypass(cmd_unquoted, cwd, origin, current_branch, rules_lookup) + # `_push_targets` tokenizes with shlex and keys off a real `git push` argv adjacency, so a push named + # only inside a quoted argument yields no target - the raw substring is just a cheap pre-filter. + if _GIT_PUSH.search(cmd): + dec, reason = _check_push_bypass(cmd, cwd, origin, current_branch, rules_lookup) if dec == "deny": return dec, reason @@ -406,6 +418,8 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("gh pr merge 5 \\\n --admin --squash", None, {}, "deny", "line-continued gh pr merge --admin still caught"), ("git commit -m 'mention --no-verify in the message'", None, {}, "allow", "--no-verify inside a quoted message is not a flag"), ("gh issue comment 5 --body \"run: git push origin develop\"", None, {"develop": _CODE_RULES}, "allow", "a git push mentioned inside a quoted body is not an executed push"), + ("git push origin 'HEAD:develop'", None, {"develop": _CODE_RULES}, "deny", "a quoted refspec is unquoted by shlex and still resolves to develop"), + ("gh issue comment 5 --body \"first git push\" && git push origin develop", None, {"develop": _CODE_RULES}, "deny", "a quoted mention before a real push does not hide the real target"), ("git push >push.log 2>&1", "develop", {"develop": _CODE_RULES}, "deny", "redirection tokens are not a branch: bare push to develop still denies"), ("git push origin develop >push.log 2>&1", None, {"develop": _CODE_RULES}, "deny", "redirect after a real refspec does not hide the develop target"), ] From 5a1af14e555373b2273f6ef0f1e5ef059c223a89 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 14:44:11 -0700 Subject: [PATCH 05/13] Skip git global options when detecting a push Copilot review of #449: the push detection required a bare git-push token adjacency, so a global option between them (git -C push, git -c k=v push, git --git-dir=... push) dodged Rule 4 entirely and a direct push to a PR-gated branch could slip through. Add _git_push_args(), which skips git's value-taking global options before the subcommand, and route all three sites through it (the push parser, the write classifier, and the pre-filter). The loose pre-filter regex now allows the intervening options. Self-test now 54 cases (-C, -c k=v, --git-dir= forms deny; feature allowed); verified live: git -C ... push origin develop denies. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 66 ++++++++++++++++------- 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index fd586a7f..db799e95 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -50,7 +50,9 @@ _API_FIELD_FLAG = re.compile(r"(?:^|\s)(?:-f|-F|--field|--raw-field|--input)\b") _GRAPHQL = re.compile(r"\bgh\s+api\b.*\bgraphql\b", re.S) _MUTATION = re.compile(r"\bmutation\b") -_GIT_PUSH = re.compile(r"\bgit\s+push\b") +# Loose pre-filter only: matches `git` before `push` even with global options between them +# (git -C push). _git_push_args is the accurate arbiter that confirms an executable push. +_GIT_PUSH = re.compile(r"\bgit\b.*?\bpush\b", re.S) _GIT_COMMIT = re.compile(r"\bgit\s+commit\b") # --- Bypass-of-branch-rule detectors (Rule 4) -------------------------------------------------------- @@ -90,7 +92,7 @@ def _is_gh_write(cmd): - if _GH_WRITE_SUB.search(cmd) or _GIT_PUSH.search(cmd): + if _GH_WRITE_SUB.search(cmd) or _git_push_args(cmd) is not None: return True if _GH_API.search(cmd): if _EXPLICIT_WRITE_METHOD.search(cmd): @@ -154,33 +156,55 @@ def _current_push_branch(cwd): # Flags that consume the following token as a value, so the value is not a positional (remote/refspec). _PUSH_VALUE_FLAGS = {"-o", "--push-option", "--repo", "--receive-pack", "--exec"} +# git global options (before the subcommand) that consume the following token as their value. +_GIT_GLOBAL_VALUE_OPTS = {"-C", "-c", "--git-dir", "--work-tree", "--namespace", "--exec-path", "--config-env"} -def _push_targets(cmd, cwd=None, current_branch=None): - """Parse a `git push` invocation into (op, [branches]). +def _git_push_args(cmd): + """Return the argv following `git [global-options] push`, or None if there is no executable push. - op is 'delete' | 'force' | 'update'. current_branch, when given, stands in for the git resolution - of a bare push (the self-test passes it for an offline run). + shlex tokenizes the way the shell does: a quoted refspec (`'HEAD:develop'`) becomes a clean token and + a `git push` inside a quoted --body stays a single token, so neither forms a bare `git`+`push` argv. + Global options between `git` and `push` (git -C push, git -c k=v push, git --git-dir=... push) + are skipped - missing them would let a direct push slip past Rule 4. Falls back to a naive split only + when the quoting is unbalanced. """ - # shlex tokenizes the way the shell does: a quoted refspec (`'HEAD:develop'`) becomes a clean token, - # and a `git push ...` mentioned inside a quoted --body stays a single token, so it never forms the - # `git` `push` argv adjacency below. Fall back to a naive split only if the quoting is unbalanced. try: toks = shlex.split(cmd, posix=True) except ValueError: toks = cmd.split() - start = None - for k in range(len(toks) - 1): - if toks[k] == "git" and toks[k + 1] == "push": - start = k + 2 - break - if start is None: - return "update", [] # no executable `git push` (only a quoted mention, or `git -C ... push`) + n = len(toks) + i = 0 + while i < n: + if toks[i] != "git": + i += 1 + continue + j = i + 1 + while j < n and toks[j].startswith("-"): + if toks[j] in _GIT_GLOBAL_VALUE_OPTS and "=" not in toks[j]: + j += 2 # this global option consumes the next token as its value + else: + j += 1 + if j < n and toks[j] == "push": + return toks[j + 1:] + i += 1 # this `git` was a different subcommand; keep scanning for another + return None + + +def _push_targets(cmd, cwd=None, current_branch=None): + """Parse a `git push` invocation into (op, [branches]). + + op is 'delete' | 'force' | 'update'. current_branch, when given, stands in for the git resolution + of a bare push (the self-test passes it for an offline run). + """ + args = _git_push_args(cmd) + if args is None: + return "update", [] force = delete = False positionals = [] - i = start - while i < len(toks): - t = toks[i] + i = 0 + while i < len(args): + t = args[i] if ">" in t or "<" in t: break # a redirection operator (>, 2>, >>, <, 2>&1): end of git argv, start of shell syntax if t in ("--force", "-f") or t.startswith("--force-with-lease"): @@ -419,6 +443,10 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("git commit -m 'mention --no-verify in the message'", None, {}, "allow", "--no-verify inside a quoted message is not a flag"), ("gh issue comment 5 --body \"run: git push origin develop\"", None, {"develop": _CODE_RULES}, "allow", "a git push mentioned inside a quoted body is not an executed push"), ("git push origin 'HEAD:develop'", None, {"develop": _CODE_RULES}, "deny", "a quoted refspec is unquoted by shlex and still resolves to develop"), + ("git -C /repo push origin develop", None, {"develop": _CODE_RULES}, "deny", "global option -C before push does not dodge Rule 4"), + ("git -c user.name=x push origin main", None, {"main": _CODE_RULES}, "deny", "global option -c k=v before push does not dodge Rule 4"), + ("git --git-dir=/r/.git push origin develop", None, {"develop": _CODE_RULES}, "deny", "global option --git-dir=... before push does not dodge Rule 4"), + ("git -C /repo push origin feature/x", None, {"feature/x": set()}, "allow", "global options before push to a feature branch: allowed"), ("gh issue comment 5 --body \"first git push\" && git push origin develop", None, {"develop": _CODE_RULES}, "deny", "a quoted mention before a real push does not hide the real target"), ("git push >push.log 2>&1", "develop", {"develop": _CODE_RULES}, "deny", "redirection tokens are not a branch: bare push to develop still denies"), ("git push origin develop >push.log 2>&1", None, {"develop": _CODE_RULES}, "deny", "redirect after a real refspec does not hide the develop target"), From 2398731d628e352da4fdd0053d962726cbd0942b Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 14:49:39 -0700 Subject: [PATCH 06/13] Handle --all/--mirror/--tags pushes in Rule 4 Copilot review of #449: a push with only a remote positional was always treated as a bare push resolving the current branch. That misclassified whole-repo pushes: --all and --mirror update every branch (protected ones included, so a current-branch-only check misses the bypass), while --tags pushes no branch (so resolving one is a false deny). Detect the flags: --all and --mirror target the protected-default branches (--mirror as a force, matching its prune/rewrite), --tags yields no branch target. Non-existent defaults return no rules and are skipped. Self-test now 58 cases. (--all/--mirror scan the default branch names, not arbitrary custom-protected refs - a documented precision-over-recall bound.) Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 33 ++++++++++++++++++----- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index db799e95..6074551e 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -63,7 +63,8 @@ # explicit-bypass flags below, which are the bypass by definition and need no query. # # Branches that fail CLOSED when their rules cannot be read - protected-by-default across every config. -_PROTECTED_DEFAULT = {"main", "master", "develop"} +_PROTECTED_DEFAULT_ORDER = ("main", "master", "develop") +_PROTECTED_DEFAULT = set(_PROTECTED_DEFAULT_ORDER) # `gh pr merge --admin` overrides required reviews/status checks with admin power. _GH_ADMIN_MERGE = re.compile(r"\bgh\s+pr\s+merge\b[^\n|&;]*(?:^|\s)--admin\b") # `--no-verify` skips the local git hooks (signing / lint / pre-push gates). `git commit` also spells it @@ -200,7 +201,7 @@ def _push_targets(cmd, cwd=None, current_branch=None): args = _git_push_args(cmd) if args is None: return "update", [] - force = delete = False + force = delete = push_all = mirror = tags_only = False positionals = [] i = 0 while i < len(args): @@ -211,10 +212,16 @@ def _push_targets(cmd, cwd=None, current_branch=None): force = True elif t in ("--delete", "-d"): delete = True + elif t == "--all": + push_all = True + elif t == "--mirror": + mirror = True + elif t in ("--tags", "--follow-tags"): + tags_only = True elif t in _PUSH_VALUE_FLAGS: i += 1 # skip this flag's value elif t.startswith("-"): - pass # some other flag (e.g. -u, --tags, --no-verify) + pass # some other flag (e.g. -u, --no-verify) else: positionals.append(t) i += 1 @@ -235,9 +242,19 @@ def _push_targets(cmd, cwd=None, current_branch=None): if dst: branches.append(dst) if not refspecs and not delete: - b = current_branch if current_branch is not None else _current_push_branch(cwd) - if b: - branches = [b] + if mirror: + # --mirror force-updates and prunes every ref: treat as a force against the protected defaults + # (a non-existent one just returns no rules and is skipped). + force = True + branches = list(_PROTECTED_DEFAULT_ORDER) + elif push_all: + branches = list(_PROTECTED_DEFAULT_ORDER) # updates every local branch, protected ones included + elif tags_only: + branches = [] # tags only, no branch is updated + else: + b = current_branch if current_branch is not None else _current_push_branch(cwd) + if b: + branches = [b] op = "delete" if delete else ("force" if force else "update") return op, branches @@ -447,6 +464,10 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("git -c user.name=x push origin main", None, {"main": _CODE_RULES}, "deny", "global option -c k=v before push does not dodge Rule 4"), ("git --git-dir=/r/.git push origin develop", None, {"develop": _CODE_RULES}, "deny", "global option --git-dir=... before push does not dodge Rule 4"), ("git -C /repo push origin feature/x", None, {"feature/x": set()}, "allow", "global options before push to a feature branch: allowed"), + ("git push --all origin", None, {"main": _CODE_RULES, "master": set(), "develop": _CODE_RULES}, "deny", "--all updates every branch: a protected default denies"), + ("git push --all origin", None, {"main": set(), "master": set(), "develop": set()}, "allow", "--all where no default branch is protected: allowed"), + ("git push --mirror origin", None, {"main": _CODE_RULES, "master": set(), "develop": _CODE_RULES}, "deny", "--mirror force-prunes every ref: a protected default denies"), + ("git push --tags origin", None, {}, "allow", "--tags pushes tags only, no branch target"), ("gh issue comment 5 --body \"first git push\" && git push origin develop", None, {"develop": _CODE_RULES}, "deny", "a quoted mention before a real push does not hide the real target"), ("git push >push.log 2>&1", "develop", {"develop": _CODE_RULES}, "deny", "redirection tokens are not a branch: bare push to develop still denies"), ("git push origin develop >push.log 2>&1", None, {"develop": _CODE_RULES}, "deny", "redirect after a real refspec does not hide the develop target"), From 54abec397773db1515dc7192d0fc61e88c5d7fe1 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 14:57:25 -0700 Subject: [PATCH 07/13] Tokenize with shell-operator awareness and parse every push Copilot review of #449: the push parser stopped at any token merely containing > or <, so a > inside a quoted option value (--push-option='a>b') ended parsing before the refspec and fell back to current-branch resolution, a potential bypass. Deeper, shlex.split does not isolate operators glued to a token (develop;cmd) and only the first push in a compound was parsed. Tokenize with shlex punctuation_chars so real shell operators are their own tokens while a quoted > stays part of its word, and parse every git push in the command (each argv runs up to the next operator), so develop;cmd, a pipe, and push A && push B are all handled. _push_targets now returns every (op, branch) pair. Self-test 62 cases; verified live: quoted-value and compound pushes deny. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 181 ++++++++++++---------- 1 file changed, 102 insertions(+), 79 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index 6074551e..cf826361 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -93,7 +93,7 @@ def _is_gh_write(cmd): - if _GH_WRITE_SUB.search(cmd) or _git_push_args(cmd) is not None: + if _GH_WRITE_SUB.search(cmd) or _push_arg_lists(cmd): return True if _GH_API.search(cmd): if _EXPLICIT_WRITE_METHOD.search(cmd): @@ -161,20 +161,39 @@ def _current_push_branch(cwd): _GIT_GLOBAL_VALUE_OPTS = {"-C", "-c", "--git-dir", "--work-tree", "--namespace", "--exec-path", "--config-env"} -def _git_push_args(cmd): - """Return the argv following `git [global-options] push`, or None if there is no executable push. +_SHELL_OP_CHARS = set("();<>|&") - shlex tokenizes the way the shell does: a quoted refspec (`'HEAD:develop'`) becomes a clean token and - a `git push` inside a quoted --body stays a single token, so neither forms a bare `git`+`push` argv. - Global options between `git` and `push` (git -C push, git -c k=v push, git --git-dir=... push) - are skipped - missing them would let a direct push slip past Rule 4. Falls back to a naive split only - when the quoting is unbalanced. + +def _shell_tokens(cmd): + """Tokenize like a shell, isolating operator runs (`|`, `&&`, `;`, `>`, `2>&1`, ...) as their own + tokens even when glued to a word - so a `>` inside a quoted value stays part of that token while a + real redirection is separated. Degrades gracefully if the quoting cannot be parsed. """ try: - toks = shlex.split(cmd, posix=True) + lex = shlex.shlex(cmd, posix=True, punctuation_chars=True) + lex.whitespace_split = True + return list(lex) except ValueError: - toks = cmd.split() + try: + return shlex.split(cmd, posix=True) + except ValueError: + return cmd.split() + + +def _is_shell_op(tok): + return tok != "" and all(c in _SHELL_OP_CHARS for c in tok) + + +def _push_arg_lists(cmd): + """Every `git [global-options] push` in the command, each as the argv up to the next shell operator. + + Keying off a real `git`->`push` token sequence (with git's value-taking global options skipped) means + a push named only inside a quoted --body forms no such sequence, and a compound `push A && push B` + yields two independent arg lists so the second push is checked too. + """ + toks = _shell_tokens(cmd) n = len(toks) + out = [] i = 0 while i < n: if toks[i] != "git": @@ -187,76 +206,77 @@ def _git_push_args(cmd): else: j += 1 if j < n and toks[j] == "push": - return toks[j + 1:] - i += 1 # this `git` was a different subcommand; keep scanning for another - return None + k = j + 1 + args = [] + while k < n and not _is_shell_op(toks[k]): + args.append(toks[k]) + k += 1 + out.append(args) + i = k + else: + i += 1 # this `git` was a different subcommand; keep scanning + return out def _push_targets(cmd, cwd=None, current_branch=None): - """Parse a `git push` invocation into (op, [branches]). - - op is 'delete' | 'force' | 'update'. current_branch, when given, stands in for the git resolution - of a bare push (the self-test passes it for an offline run). - """ - args = _git_push_args(cmd) - if args is None: - return "update", [] - force = delete = push_all = mirror = tags_only = False - positionals = [] - i = 0 - while i < len(args): - t = args[i] - if ">" in t or "<" in t: - break # a redirection operator (>, 2>, >>, <, 2>&1): end of git argv, start of shell syntax - if t in ("--force", "-f") or t.startswith("--force-with-lease"): - force = True - elif t in ("--delete", "-d"): - delete = True - elif t == "--all": - push_all = True - elif t == "--mirror": - mirror = True - elif t in ("--tags", "--follow-tags"): - tags_only = True - elif t in _PUSH_VALUE_FLAGS: - i += 1 # skip this flag's value - elif t.startswith("-"): - pass # some other flag (e.g. -u, --no-verify) - else: - positionals.append(t) - i += 1 - # positionals are [remote, refspec...]; a lone positional is the remote (a bare push). - refspecs = positionals[1:] if len(positionals) >= 2 else [] - branches = [] - for rs in refspecs: - if rs.startswith("+"): - force = True - rs = rs[1:] - if rs.startswith(":"): - delete = True # `:dst` empty-source refspec deletes dst - dst = rs.split(":", 1)[1] if ":" in rs else rs - if dst.startswith("refs/heads/"): - dst = dst[len("refs/heads/"):] - elif dst.startswith("refs/"): - continue # a tag or other non-branch ref - if dst: - branches.append(dst) - if not refspecs and not delete: - if mirror: - # --mirror force-updates and prunes every ref: treat as a force against the protected defaults - # (a non-existent one just returns no rules and is skipped). - force = True - branches = list(_PROTECTED_DEFAULT_ORDER) - elif push_all: - branches = list(_PROTECTED_DEFAULT_ORDER) # updates every local branch, protected ones included - elif tags_only: - branches = [] # tags only, no branch is updated - else: - b = current_branch if current_branch is not None else _current_push_branch(cwd) - if b: - branches = [b] - op = "delete" if delete else ("force" if force else "update") - return op, branches + """Parse every push in the command into a list of (op, branch); op is delete | force | update.""" + results = [] + for args in _push_arg_lists(cmd): + force = delete = push_all = mirror = tags_only = False + positionals = [] + i = 0 + while i < len(args): + t = args[i] + if t in ("--force", "-f") or t.startswith("--force-with-lease"): + force = True + elif t in ("--delete", "-d"): + delete = True + elif t == "--all": + push_all = True + elif t == "--mirror": + mirror = True + elif t in ("--tags", "--follow-tags"): + tags_only = True + elif t in _PUSH_VALUE_FLAGS: + i += 1 # skip this flag's value + elif t.startswith("-"): + pass # some other flag (e.g. -u, --no-verify) + else: + positionals.append(t) + i += 1 + # positionals are [remote, refspec...]; a lone positional is the remote (a bare push). + refspecs = positionals[1:] if len(positionals) >= 2 else [] + branches = [] + for rs in refspecs: + if rs.startswith("+"): + force = True + rs = rs[1:] + if rs.startswith(":"): + delete = True # `:dst` empty-source refspec deletes dst + dst = rs.split(":", 1)[1] if ":" in rs else rs + if dst.startswith("refs/heads/"): + dst = dst[len("refs/heads/"):] + elif dst.startswith("refs/"): + continue # a tag or other non-branch ref + if dst: + branches.append(dst) + if not refspecs and not delete: + if mirror: + # --mirror force-updates and prunes every ref: a force against the protected defaults + # (a non-existent one just returns no rules and is skipped). + force = True + branches = list(_PROTECTED_DEFAULT_ORDER) + elif push_all: + branches = list(_PROTECTED_DEFAULT_ORDER) # updates every local branch, protected included + elif tags_only: + branches = [] # tags only, no branch is updated + else: + b = current_branch if current_branch is not None else _current_push_branch(cwd) + if b: + branches = [b] + op = "delete" if delete else ("force" if force else "update") + results.extend((op, br) for br in branches) + return results def _handoff(cmd): @@ -287,8 +307,7 @@ def _check_push_bypass(cmd, cwd, origin, current_branch=None, rules_lookup=None) """Deny a git push that would only succeed by bypassing an active branch rule.""" if origin is None: origin = _origin_owner_repo(cwd) - op, branches = _push_targets(cmd, cwd, current_branch) - for br in branches: + for op, br in _push_targets(cmd, cwd, current_branch): rules = rules_lookup(br) if rules_lookup is not None else ( _live_branch_rules(origin[0], origin[1], br) if origin else None ) @@ -468,6 +487,10 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("git push --all origin", None, {"main": set(), "master": set(), "develop": set()}, "allow", "--all where no default branch is protected: allowed"), ("git push --mirror origin", None, {"main": _CODE_RULES, "master": set(), "develop": _CODE_RULES}, "deny", "--mirror force-prunes every ref: a protected default denies"), ("git push --tags origin", None, {}, "allow", "--tags pushes tags only, no branch target"), + ("git push --push-option='a>b' origin develop", None, {"develop": _CODE_RULES}, "deny", "a > inside a quoted option value is not a redirection: develop still parsed"), + ("git push origin feature/x && git push origin develop", None, {"feature/x": set(), "develop": _CODE_RULES}, "deny", "second push in a compound is checked: develop denies"), + ("git push origin develop && git push origin feature/x", None, {"develop": _CODE_RULES, "feature/x": set()}, "deny", "first push in a compound is checked: develop denies"), + ("git push origin develop | cat", None, {"develop": _CODE_RULES}, "deny", "a pipe ends the push argv: develop still parsed"), ("gh issue comment 5 --body \"first git push\" && git push origin develop", None, {"develop": _CODE_RULES}, "deny", "a quoted mention before a real push does not hide the real target"), ("git push >push.log 2>&1", "develop", {"develop": _CODE_RULES}, "deny", "redirection tokens are not a branch: bare push to develop still denies"), ("git push origin develop >push.log 2>&1", None, {"develop": _CODE_RULES}, "deny", "redirect after a real refspec does not hide the develop target"), From e0fdd5c160f845ef1bc6141b3f899763195f4e57 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 15:02:11 -0700 Subject: [PATCH 08/13] Recognize git invoked by absolute path or .exe Copilot review of #449: the push scan matched only a token exactly equal to "git", and the commit -n short-form detection matched only a bare "git commit", so an absolute-path or .exe invocation (/usr/bin/git push origin develop, git.exe push, /usr/bin/git commit -n) bypassed Rule 4 entirely. Add _is_git_exe(), which matches any token whose basename is git or git.exe, and broaden _GIT_COMMIT to allow a path prefix and .exe suffix. Self-test 65 cases (absolute-path push and commit -n, git.exe push). Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index cf826361..cbdfbdf6 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -53,7 +53,8 @@ # Loose pre-filter only: matches `git` before `push` even with global options between them # (git -C push). _git_push_args is the accurate arbiter that confirms an executable push. _GIT_PUSH = re.compile(r"\bgit\b.*?\bpush\b", re.S) -_GIT_COMMIT = re.compile(r"\bgit\s+commit\b") +# `git commit`, allowing an absolute/relative path and a .exe suffix (/usr/bin/git commit, git.exe commit). +_GIT_COMMIT = re.compile(r"(?:^|\s)\S*?\bgit(?:\.exe)?\s+commit\b", re.I) # --- Bypass-of-branch-rule detectors (Rule 4) -------------------------------------------------------- # A git operation is denied when it would only succeed by bypassing an active branch rule - the harm is @@ -184,6 +185,14 @@ def _is_shell_op(tok): return tok != "" and all(c in _SHELL_OP_CHARS for c in tok) +def _is_git_exe(tok): + """True if the token invokes git, including an absolute/relative path or a .exe suffix + (/usr/bin/git, ./git, C:\\...\\git.exe) - an exact "git" match alone is a bypass path. + """ + base = tok.rsplit("/", 1)[-1].rsplit("\\", 1)[-1].lower() + return base in ("git", "git.exe") + + def _push_arg_lists(cmd): """Every `git [global-options] push` in the command, each as the argv up to the next shell operator. @@ -196,7 +205,7 @@ def _push_arg_lists(cmd): out = [] i = 0 while i < n: - if toks[i] != "git": + if not _is_git_exe(toks[i]): i += 1 continue j = i + 1 @@ -491,6 +500,9 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("git push origin feature/x && git push origin develop", None, {"feature/x": set(), "develop": _CODE_RULES}, "deny", "second push in a compound is checked: develop denies"), ("git push origin develop && git push origin feature/x", None, {"develop": _CODE_RULES, "feature/x": set()}, "deny", "first push in a compound is checked: develop denies"), ("git push origin develop | cat", None, {"develop": _CODE_RULES}, "deny", "a pipe ends the push argv: develop still parsed"), + ("/usr/bin/git push origin develop", None, {"develop": _CODE_RULES}, "deny", "an absolute-path git is still git: direct push denies"), + ("/usr/bin/git commit -n -m x", None, {}, "deny", "absolute-path git commit -n is --no-verify"), + ("git.exe push origin develop", None, {"develop": _CODE_RULES}, "deny", "git.exe is still git: direct push denies"), ("gh issue comment 5 --body \"first git push\" && git push origin develop", None, {"develop": _CODE_RULES}, "deny", "a quoted mention before a real push does not hide the real target"), ("git push >push.log 2>&1", "develop", {"develop": _CODE_RULES}, "deny", "redirection tokens are not a branch: bare push to develop still denies"), ("git push origin develop >push.log 2>&1", None, {"develop": _CODE_RULES}, "deny", "redirect after a real refspec does not hide the develop target"), From 51ea3d28a434af57f5094655121377638b0ccf68 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 15:05:36 -0700 Subject: [PATCH 09/13] Treat --follow-tags as a branch push, not tags-only Copilot review of #449: --follow-tags was grouped with --tags as tags-only, so git push --follow-tags looked like it updated no branch and skipped the checks - but --follow-tags pushes the current branch plus reachable tags. Only --tags is tags-only; --follow-tags falls through to normal bare-push branch resolution. Self-test 66 cases. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index cbdfbdf6..8b03c2e2 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -244,8 +244,8 @@ def _push_targets(cmd, cwd=None, current_branch=None): push_all = True elif t == "--mirror": mirror = True - elif t in ("--tags", "--follow-tags"): - tags_only = True + elif t == "--tags": + tags_only = True # --follow-tags is NOT tags-only: it also pushes the current branch elif t in _PUSH_VALUE_FLAGS: i += 1 # skip this flag's value elif t.startswith("-"): @@ -496,6 +496,7 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("git push --all origin", None, {"main": set(), "master": set(), "develop": set()}, "allow", "--all where no default branch is protected: allowed"), ("git push --mirror origin", None, {"main": _CODE_RULES, "master": set(), "develop": _CODE_RULES}, "deny", "--mirror force-prunes every ref: a protected default denies"), ("git push --tags origin", None, {}, "allow", "--tags pushes tags only, no branch target"), + ("git push --follow-tags origin", "develop", {"develop": _CODE_RULES}, "deny", "--follow-tags also pushes the current branch: resolves develop"), ("git push --push-option='a>b' origin develop", None, {"develop": _CODE_RULES}, "deny", "a > inside a quoted option value is not a redirection: develop still parsed"), ("git push origin feature/x && git push origin develop", None, {"feature/x": set(), "develop": _CODE_RULES}, "deny", "second push in a compound is checked: develop denies"), ("git push origin develop && git push origin feature/x", None, {"develop": _CODE_RULES, "feature/x": set()}, "deny", "first push in a compound is checked: develop denies"), From 1b50ea33576e72ab5e985313e23bd2159db71653 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 15:10:25 -0700 Subject: [PATCH 10/13] Gate --no-verify to git, fix a stale comment reference Copilot review of #449: - --no-verify was denied on any command, so a non-git tool using the same flag name (npm publish --no-verify) false-denied. Gate it to a git commit or git push context. - A comment still named _git_push_args, renamed to _push_arg_lists. Self-test 67 cases (non-git --no-verify allowed). Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index 8b03c2e2..0df86b76 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -51,7 +51,7 @@ _GRAPHQL = re.compile(r"\bgh\s+api\b.*\bgraphql\b", re.S) _MUTATION = re.compile(r"\bmutation\b") # Loose pre-filter only: matches `git` before `push` even with global options between them -# (git -C push). _git_push_args is the accurate arbiter that confirms an executable push. +# (git -C push). _push_arg_lists is the accurate arbiter that confirms an executable push. _GIT_PUSH = re.compile(r"\bgit\b.*?\bpush\b", re.S) # `git commit`, allowing an absolute/relative path and a .exe suffix (/usr/bin/git commit, git.exe commit). _GIT_COMMIT = re.compile(r"(?:^|\s)\S*?\bgit(?:\.exe)?\s+commit\b", re.I) @@ -304,7 +304,13 @@ def _check_bypass_flags(cmd): "This uses `gh pr merge --admin`, which merges past required reviews and status checks using " "admin power - a bypass of the merge gate. Merge only when the gate is satisfied." + _handoff(cmd) ) - if _NO_VERIFY_LONG.search(bare) or (_GIT_COMMIT.search(bare) and _COMMIT_SHORT_N.search(bare)): + # --no-verify / commit -n skip the git hooks, so they only matter for a git commit or push - other + # tools use --no-verify for unrelated things, and denying those would be a false positive. + is_commit = _GIT_COMMIT.search(bare) is not None + is_push = bool(_push_arg_lists(bare)) + long_no_verify = (is_commit or is_push) and _NO_VERIFY_LONG.search(bare) + short_n_commit = is_commit and _COMMIT_SHORT_N.search(bare) # `-n` is --no-verify for commit (push -n is dry-run) + if long_no_verify or short_n_commit: return "deny", ( "This uses --no-verify, which skips the git hooks (signing, lint, and pre-push gates). " "Skipping verification is a bypass; run the command without it." + _handoff(cmd) @@ -486,6 +492,7 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("gh pr merge 5 --admin --squash", None, {}, "deny", "gh pr merge --admin overrides the merge gate"), ("gh pr merge 5 \\\n --admin --squash", None, {}, "deny", "line-continued gh pr merge --admin still caught"), ("git commit -m 'mention --no-verify in the message'", None, {}, "allow", "--no-verify inside a quoted message is not a flag"), + ("npm publish --no-verify", None, {}, "allow", "--no-verify on a non-git command is not a git-hook bypass"), ("gh issue comment 5 --body \"run: git push origin develop\"", None, {"develop": _CODE_RULES}, "allow", "a git push mentioned inside a quoted body is not an executed push"), ("git push origin 'HEAD:develop'", None, {"develop": _CODE_RULES}, "deny", "a quoted refspec is unquoted by shlex and still resolves to develop"), ("git -C /repo push origin develop", None, {"develop": _CODE_RULES}, "deny", "global option -C before push does not dodge Rule 4"), From 2d8f92cd577bcf38b9f6349b5f2d1296ad8a121c Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 15:17:45 -0700 Subject: [PATCH 11/13] Generalize subcommand detection so commit honors git global options Copilot review of #449: the --no-verify/-n check used a regex that matched only a bare "git commit", so git -C commit --no-verify (a global option before the subcommand) bypassed it - the same gap already fixed for push. Generalize the token scanner to _git_subcommand_arglists(cmd, sub) and detect the bypass flag as an actual arg of the commit/push argv, which also drops the three now-unused regexes (_GIT_COMMIT/_NO_VERIFY_LONG/_COMMIT_SHORT_N). Self-test 69 cases (git -C/-c commit -n/--no-verify deny). Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 42 +++++++++++------------ 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index 0df86b76..c58cca53 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -53,8 +53,6 @@ # Loose pre-filter only: matches `git` before `push` even with global options between them # (git -C push). _push_arg_lists is the accurate arbiter that confirms an executable push. _GIT_PUSH = re.compile(r"\bgit\b.*?\bpush\b", re.S) -# `git commit`, allowing an absolute/relative path and a .exe suffix (/usr/bin/git commit, git.exe commit). -_GIT_COMMIT = re.compile(r"(?:^|\s)\S*?\bgit(?:\.exe)?\s+commit\b", re.I) # --- Bypass-of-branch-rule detectors (Rule 4) -------------------------------------------------------- # A git operation is denied when it would only succeed by bypassing an active branch rule - the harm is @@ -68,10 +66,6 @@ _PROTECTED_DEFAULT = set(_PROTECTED_DEFAULT_ORDER) # `gh pr merge --admin` overrides required reviews/status checks with admin power. _GH_ADMIN_MERGE = re.compile(r"\bgh\s+pr\s+merge\b[^\n|&;]*(?:^|\s)--admin\b") -# `--no-verify` skips the local git hooks (signing / lint / pre-push gates). `git commit` also spells it -# `-n`; `git push -n` means --dry-run, so the short form is a bypass only for commit. -_NO_VERIFY_LONG = re.compile(r"(?:^|\s)--no-verify\b") -_COMMIT_SHORT_N = re.compile(r"(?:^|\s)-n\b") # --- Risk-pattern detectors -------------------------------------------------------------------------- # Output-discard / force-success tails. Bare `2>&1` is NOT here: it merges stderr into stdout, leaving @@ -193,12 +187,12 @@ def _is_git_exe(tok): return base in ("git", "git.exe") -def _push_arg_lists(cmd): - """Every `git [global-options] push` in the command, each as the argv up to the next shell operator. +def _git_subcommand_arglists(cmd, sub): + """Every `git [global-options] ` in the command, each as the argv up to the next shell operator. - Keying off a real `git`->`push` token sequence (with git's value-taking global options skipped) means - a push named only inside a quoted --body forms no such sequence, and a compound `push A && push B` - yields two independent arg lists so the second push is checked too. + Keying off a real `git`->`` token sequence (git's value-taking global options skipped, an + absolute-path or .exe git recognized) means the same invocation named inside a quoted --body forms no + such sequence, and a compound ` A && B` yields two independent arg lists so both are seen. """ toks = _shell_tokens(cmd) n = len(toks) @@ -214,7 +208,7 @@ def _push_arg_lists(cmd): j += 2 # this global option consumes the next token as its value else: j += 1 - if j < n and toks[j] == "push": + if j < n and toks[j] == sub: k = j + 1 args = [] while k < n and not _is_shell_op(toks[k]): @@ -227,6 +221,10 @@ def _push_arg_lists(cmd): return out +def _push_arg_lists(cmd): + return _git_subcommand_arglists(cmd, "push") + + def _push_targets(cmd, cwd=None, current_branch=None): """Parse every push in the command into a list of (op, branch); op is delete | force | update.""" results = [] @@ -298,19 +296,19 @@ def _handoff(cmd): def _check_bypass_flags(cmd): """Deny the explicit-bypass flags: they are a bypass by definition, no branch query needed.""" - bare = _QUOTED_SPAN.sub("", cmd) # a flag mentioned inside a quoted message/body is not a real flag - if _GH_ADMIN_MERGE.search(bare): + if _GH_ADMIN_MERGE.search(_QUOTED_SPAN.sub("", cmd)): # a flag inside a quoted body is not a real flag return "deny", ( "This uses `gh pr merge --admin`, which merges past required reviews and status checks using " "admin power - a bypass of the merge gate. Merge only when the gate is satisfied." + _handoff(cmd) ) - # --no-verify / commit -n skip the git hooks, so they only matter for a git commit or push - other - # tools use --no-verify for unrelated things, and denying those would be a false positive. - is_commit = _GIT_COMMIT.search(bare) is not None - is_push = bool(_push_arg_lists(bare)) - long_no_verify = (is_commit or is_push) and _NO_VERIFY_LONG.search(bare) - short_n_commit = is_commit and _COMMIT_SHORT_N.search(bare) # `-n` is --no-verify for commit (push -n is dry-run) - if long_no_verify or short_n_commit: + # --no-verify / commit -n skip the git hooks, so they only matter as an actual arg to a git commit or + # push (other tools use --no-verify for unrelated things; shlex keeps a quoted mention out of the argv). + # `-n` is --no-verify only for commit; `git push -n` is --dry-run. + commit_lists = _git_subcommand_arglists(cmd, "commit") + push_lists = _push_arg_lists(cmd) + commit_bypass = any(("--no-verify" in a) or ("-n" in a) for a in commit_lists) + push_bypass = any("--no-verify" in a for a in push_lists) + if commit_bypass or push_bypass: return "deny", ( "This uses --no-verify, which skips the git hooks (signing, lint, and pre-push gates). " "Skipping verification is a bypass; run the command without it." + _handoff(cmd) @@ -487,6 +485,8 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("git push origin feature/x", None, {"feature/x": None}, "allow", "feature branch with unreadable rules: fail open"), ("git commit --no-verify -m x", None, {}, "deny", "commit --no-verify skips the hooks"), ("git commit -n -m x", None, {}, "deny", "commit -n is --no-verify"), + ("git -C /repo commit -n -m x", None, {}, "deny", "global option before commit -n does not dodge the check"), + ("git -c user.name=x commit --no-verify -m y", None, {}, "deny", "global option before commit --no-verify does not dodge the check"), ("git push --no-verify origin feature/x", None, {"feature/x": set()}, "deny", "push --no-verify is a bypass even on a feature branch"), ("git push -n origin develop", None, {"develop": _CODE_RULES}, "deny", "push -n is dry-run not no-verify, but the direct push to develop still denies"), ("gh pr merge 5 --admin --squash", None, {}, "deny", "gh pr merge --admin overrides the merge gate"), From c63d6dfc6015b326670a096f2462a09e99134861 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 15:22:42 -0700 Subject: [PATCH 12/13] Resolve origin only when a push exists; clarify the fail-closed reason Copilot review of #449: _check_push_bypass resolved origin (a git subprocess) before checking whether any executable push was found, so a command that only mentions git push in a quoted argument still did git work. Compute the targets first and return early when there are none. Also, the fail-closed message assumed API unreachability, but the cause can be missing repo context (not a git checkout) - distinguish the two reasons. Self-test 69 cases, unchanged. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 26 ++++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index c58cca53..e4660b30 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -318,18 +318,28 @@ def _check_bypass_flags(cmd): def _check_push_bypass(cmd, cwd, origin, current_branch=None, rules_lookup=None): """Deny a git push that would only succeed by bypassing an active branch rule.""" - if origin is None: + targets = _push_targets(cmd, cwd, current_branch) + if not targets: + return "allow", "" # only a quoted mention or a non-push git command: no git/API work needed + if rules_lookup is None and origin is None: origin = _origin_owner_repo(cwd) - for op, br in _push_targets(cmd, cwd, current_branch): - rules = rules_lookup(br) if rules_lookup is not None else ( - _live_branch_rules(origin[0], origin[1], br) if origin else None - ) + for op, br in targets: + if rules_lookup is not None: + rules = rules_lookup(br) + elif origin is not None: + rules = _live_branch_rules(origin[0], origin[1], br) + else: + rules = None # no origin to query the rules against if rules is None: if br in _PROTECTED_DEFAULT: + reason = ( + "this checkout's origin repository could not be determined" + if origin is None and rules_lookup is None + else "its branch rules could not be read (the API may be unreachable)" + ) return "deny", ( - f"Could not read the branch rules for '{br}', a protected-by-default branch " - f"(main/master/develop). Failing closed rather than risk a silent bypass - retry when " - f"the API is reachable." + _handoff(cmd) + f"Could not verify '{br}', a protected-by-default branch (main/master/develop), " + f"because {reason}. Failing closed rather than risk a silent bypass." + _handoff(cmd) ) continue # an unknown-rules feature/other branch: nothing to bypass, let it through if op == "update" and "pull_request" in rules: From 6e0529120c52f9dbc3bf9c753a103a420297ea57 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 25 Jul 2026 15:27:16 -0700 Subject: [PATCH 13/13] Skip redirections in argv so leading ones do not hide push targets Copilot review of #449: the argv collector stopped at any shell-operator token, so a redirection placed before the refspec (git push 2>push.log origin develop, which POSIX allows) had its fd digit read as a positional and parsing stopped at >, dropping the real origin develop and falling back to current- branch resolution - a bypass. Distinguish redirections (>, >>, <, >&, and a leading fd digit) which are skipped so args continue, from command separators (|, &&, ;) which end the invocation. Also catch TypeError in _shell_tokens so the tokenizer degrades on a Python without punctuation_chars instead of crashing (which would skip Rule 4 entirely). Self-test 71 cases; verified live: leading-redirection push to develop denies. Co-Authored-By: Claude Opus 4.8 --- host-setup/agent-safety/gh-write-guard.py | 27 ++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index e4660b30..be851d03 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -168,7 +168,7 @@ def _shell_tokens(cmd): lex = shlex.shlex(cmd, posix=True, punctuation_chars=True) lex.whitespace_split = True return list(lex) - except ValueError: + except (ValueError, TypeError): # bad quoting, or punctuation_chars unsupported on old Python try: return shlex.split(cmd, posix=True) except ValueError: @@ -179,6 +179,14 @@ def _is_shell_op(tok): return tok != "" and all(c in _SHELL_OP_CHARS for c in tok) +def _is_redir_op(tok): + return _is_shell_op(tok) and (">" in tok or "<" in tok) # >, >>, <, >&, &> + + +def _is_separator(tok): + return _is_shell_op(tok) and ">" not in tok and "<" not in tok # |, ||, &, &&, ;, (, ) + + def _is_git_exe(tok): """True if the token invokes git, including an absolute/relative path or a .exe suffix (/usr/bin/git, ./git, C:\\...\\git.exe) - an exact "git" match alone is a bypass path. @@ -211,8 +219,19 @@ def _git_subcommand_arglists(cmd, sub): if j < n and toks[j] == sub: k = j + 1 args = [] - while k < n and not _is_shell_op(toks[k]): - args.append(toks[k]) + while k < n: + t = toks[k] + if _is_separator(t): + break # a command separator (|, &&, ;) ends this git invocation + if t.isdigit() and k + 1 < n and _is_redir_op(toks[k + 1]): + k += 1 # a file-descriptor number before a redirection is shell syntax, not git argv + continue + if _is_redir_op(t): + k += 1 # skip the redirection operator and its target token; args continue after it + if k < n and not _is_shell_op(toks[k]): + k += 1 + continue + args.append(t) k += 1 out.append(args) i = k @@ -518,6 +537,8 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None) ("git push origin feature/x && git push origin develop", None, {"feature/x": set(), "develop": _CODE_RULES}, "deny", "second push in a compound is checked: develop denies"), ("git push origin develop && git push origin feature/x", None, {"develop": _CODE_RULES, "feature/x": set()}, "deny", "first push in a compound is checked: develop denies"), ("git push origin develop | cat", None, {"develop": _CODE_RULES}, "deny", "a pipe ends the push argv: develop still parsed"), + ("git push 2>push.log origin develop", None, {"develop": _CODE_RULES}, "deny", "a leading fd redirection is skipped, not a positional: develop still parsed"), + ("git push >log origin develop", None, {"develop": _CODE_RULES}, "deny", "a leading stdout redirection before args does not hide develop"), ("/usr/bin/git push origin develop", None, {"develop": _CODE_RULES}, "deny", "an absolute-path git is still git: direct push denies"), ("/usr/bin/git commit -n -m x", None, {}, "deny", "absolute-path git commit -n is --no-verify"), ("git.exe push origin develop", None, {"develop": _CODE_RULES}, "deny", "git.exe is still git: direct push denies"),