fix(agent-guard): resolve the git subcommand past global flags, not a fixed argv slice

tools/agent-guard/src/agent_guard/__init__.py

@@ -240,6 +240,53 @@ def _opt_value(argv: list[str], short: str, long: str) -> str | None:
 # the group.
 GH_VALUE_FLAGS = frozenset({"-R", "--repo", "-H", "--hostname"})
 
+# Global `git` options that take a separate value and may appear before the
+# subcommand, e.g. `git -C /path commit ...`. `git` parses these interspersed
+# the same way `gh` does, and the value must be consumed here for the same
+# reason GH_VALUE_FLAGS is: leaving it behind shifts the subcommand one token
+# to the right.
+GIT_VALUE_FLAGS = frozenset(
+    {
+        "-C",
+        "-c",
+        "--exec-path",
+        "--html-path",
+        "--man-path",
+        "--info-path",
+        "--namespace",
+        "--work-tree",
+        "--git-dir",
+        "--super-prefix",
+        "--config-env",
+    }
+)
+
+
+def git_subcommand_index(argv: list[str]) -> int | None:
+    """Index into ``argv`` of the ``git`` subcommand (``\"commit\"``,
+    ``\"push\"``, ...), skipping global flags and their values so
+    ``git -C /tmp commit`` resolves the same as ``git commit``. None if
+    ``argv`` is not a ``git`` call, or if no subcommand can be identified.
+
+    Without this, a guard anchored on a fixed ``argv[:2]`` slice is fail-open
+    for any ordinary global flag before the subcommand (``git -C <dir>
+    commit``, ``git -c core.editor=true commit``, ``git --no-pager commit``),
+    the same bypass ``gh_subcommand`` above already guards against for ``gh``.
+    """
+    if not argv or argv[0] != "git":
+        return None
+    i = 1
+    while i < len(argv):
+        tok = argv[i]
+        if tok.startswith("-") and tok != "-":
+            if "=" not in tok and tok in GIT_VALUE_FLAGS:
+                i += 2
+                continue
+            i += 1
+            continue
+        return i
+    return None
+
 
 def gh_subcommand(argv: list[str]) -> tuple[str, str] | None:
     """For an argv whose first token is ``gh``, return ``(group, sub)`` skipping
@@ -352,7 +399,8 @@ def _repo_flag(argv: list[str]) -> list[str]:
 
 
 def guard_commit_trailer(seg: Segment, cwd: str | None) -> str | None:
-    if seg.argv[:2] != ["git", "commit"]:
+    idx = git_subcommand_index(seg.argv)
+    if idx is None or seg.argv[idx] != "commit":
         return None
     if not re.search(r"co-authored-by:", seg.raw, re.IGNORECASE):
         return None
@@ -367,7 +415,8 @@ def guard_commit_trailer(seg: Segment, cwd: str | None) -> str | None:
 
 
 def guard_empty_rebase(seg: Segment, cwd: str | None) -> str | None:
-    if seg.argv[:2] != ["git", "push"]:
+    idx = git_subcommand_index(seg.argv)
+    if idx is None or seg.argv[idx] != "push":
         return None
     forced = any(
         t in ("-f", "--force") or t == "--force-with-lease" or t.startswith("--force-with-lease=")
@@ -379,7 +428,7 @@ def guard_empty_rebase(seg: Segment, cwd: str | None) -> str | None:
         return None
 
     # Resolve the source ref being pushed: last `src[:dst]` positional, else HEAD.
-    positionals = [t for t in seg.argv[2:] if not t.startswith("-")]
+    positionals = [t for t in seg.argv[idx + 1 :] if not t.startswith("-")]
     src = "HEAD"
     if len(positionals) >= 2:
         src = positionals[1].split(":", 1)[0] or "HEAD"
@@ -467,6 +516,13 @@ def override(self, *names: str) -> bool:
     def gh_subcommand(self) -> tuple[str, str] | None:
         return gh_subcommand(self.argv)
 
+    def git_subcommand(self) -> str | None:
+        """The ``git`` subcommand (``\"commit\"``, ``\"push\"``, ...), skipping
+        global flags and their values the same way :func:`gh_subcommand` does
+        for ``gh``. None if this segment is not a ``git`` call."""
+        idx = git_subcommand_index(self.argv)
+        return self.argv[idx] if idx is not None else None
+
     def opt(self, short: str, long: str) -> str | None:
         return _opt_value(self.argv, short, long)
 
@@ -497,7 +553,14 @@ def command_kinds(seg: Segment) -> set[str]:
         return kinds
     head = seg.argv[0]
     kinds.add(head)
-    if len(seg.argv) > 1 and head in ("git", "gh"):
+    if head == "git":
+        # A TRIGGERS = ["git:commit"] contributed guard must still fire behind
+        # an ordinary global flag (`git -C <dir> commit`); a plain seg.argv[1]
+        # read is the same fail-open shape guard_commit_trailer had.
+        idx = git_subcommand_index(seg.argv)
+        if idx is not None:
+            kinds.add(f"git:{seg.argv[idx]}")
+    elif len(seg.argv) > 1 and head == "gh":
         kinds.add(f"{head}:{seg.argv[1]}")
     return kinds
 

tools/agent-guard/src/agent_guard/guards.d/no_verify_commit.py

@@ -39,7 +39,7 @@
 
 
 def guard(ctx):
-    if ctx.argv[:2] != ["git", "commit"]:
+    if ctx.git_subcommand() != "commit":
         return None
     if not any(tok in ("--no-verify", "-n") for tok in ctx.argv):
         return None

tools/agent-guard/tests/test_guards.py

@@ -93,6 +93,22 @@ def test_commit_coauthor_override():
     assert dispatch('MAGPIE_ALLOW_COAUTHOR=1 git commit -m "x\nCo-Authored-By: a"') is None
 
 
+@pytest.mark.parametrize(
+    "command",
+    [
+        'git -C /tmp commit -m "x\n\nCo-Authored-By: a"',
+        'git -c core.editor=true commit -m "x\n\nCo-Authored-By: a"',
+        'git --no-pager commit -m "x\n\nCo-Authored-By: a"',
+    ],
+)
+def test_commit_coauthor_denied_past_a_global_flag(command):
+    # A global `git` option before the subcommand must not shift it past the
+    # guard's fixed check, the same bypass `gh_subcommand` already guards
+    # against for `gh --repo ... pr create`.
+    reason = dispatch(command)
+    assert reason and "Co-Authored-By" in reason
+
+
 # --------------------------------------------------------------------------- #
 # empty-rebase guard (bundled)
 # --------------------------------------------------------------------------- #
@@ -142,6 +158,16 @@ def test_empty_rebase_override(monkeypatch):
     assert dispatch("MAGPIE_ALLOW_EMPTY_PUSH=1 git push --force origin b:b") is None
 
 
+def test_empty_rebase_denied_past_a_global_flag(monkeypatch):
+    # Same global-flag bypass as test_commit_coauthor_denied_past_a_global_flag,
+    # for the other bundled guard: `git -C <dir> push --force ...` must still
+    # resolve the subcommand as `push`, and the src ref must still be read
+    # relative to the subcommand rather than a fixed argv[2:] slice.
+    monkeypatch.setattr(agent_guard, "_run", fake_run(_push_handler("0")))
+    reason = dispatch("git -C /tmp push --force-with-lease origin mybranch:mybranch")
+    assert reason and "0 commits" in reason
+
+
 # --------------------------------------------------------------------------- #
 # bundled example contributed guard (guards.d/no_verify_commit.py)
 # --------------------------------------------------------------------------- #
@@ -160,6 +186,14 @@ def test_plain_commit_not_blocked_by_no_verify_guard():
     assert dispatch('git commit -m "ordinary commit"') is None
 
 
+def test_no_verify_guard_denied_past_a_global_flag():
+    # The guard's own ctx.argv[:2] check had the same bypass as the two
+    # bundled guards; command_kinds' TRIGGERS = ["git:commit"] pre-filter
+    # had it too, so a global flag used to skip calling guard() at all.
+    reason = dispatch('git -C /tmp commit -m "x" --no-verify')
+    assert reason and "no-verify" in reason
+
+
 # --------------------------------------------------------------------------- #
 # contributed-guard discovery
 # --------------------------------------------------------------------------- #
@@ -332,3 +366,34 @@ def test_abs_path_plain_commit_still_allowed():
 )
 def test_gh_subcommand_consumes_flag_values(argv, expected):
     assert agent_guard.gh_subcommand(argv) == expected
+
+
+# ---------------------------------------------------------------------------
+# git_subcommand_index: the same fail-open shape as gh_subcommand above, for
+# the two BUILTIN_GUARDS (guard_commit_trailer, guard_empty_rebase), which
+# used to anchor on a fixed argv[:2] slice.
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    "argv, expected_index",
+    [
+        (["git", "commit", "-m", "x"], 1),
+        (["git", "push", "origin", "b"], 1),
+        # A global flag before the subcommand: the regression.
+        (["git", "-C", "/tmp", "commit", "-m", "x"], 3),
+        (["git", "-c", "core.editor=true", "commit", "-m", "x"], 3),
+        (["git", "--no-pager", "commit", "-m", "x"], 2),
+        (["git", "--work-tree", "/tmp", "push"], 3),
+        # Several pre-subcommand flags.
+        (["git", "-C", "/tmp", "-c", "a=b", "commit"], 5),
+        # No subcommand at all: only global flags, or nothing after `git`.
+        (["git", "-C", "/tmp"], None),
+        (["git"], None),
+        # Non-git and empty input.
+        (["gh", "pr", "create"], None),
+        ([], None),
+    ],
+)
+def test_git_subcommand_index_consumes_flag_values(argv, expected_index):
+    assert agent_guard.git_subcommand_index(argv) == expected_index