Răsfoiți Sursa

Harden git invocation and repository hint handling

:house: Remote-Dev: homespace
Octavian Guzu 8 ore în urmă
părinte
comite
debbe686ab

+ 23 - 0
plugins/security-guidance/hooks/gitutil.py

@@ -39,6 +39,29 @@ GIT_CMD = [
     "-c", "core.quotePath=false",
 ]
 
+SAFE_GIT_CONFIG = (
+    ("core.fsmonitor", "false"),
+    ("core.hooksPath", "/dev/null"),
+)
+
+
+def git_config_env(pairs, base=None):
+    base = os.environ if base is None else base
+    try:
+        n = max(0, int(base.get("GIT_CONFIG_COUNT") or 0))
+    except (TypeError, ValueError):
+        n = 0
+    env = {}
+    for i, (k, v) in enumerate(pairs, start=n):
+        env[f"GIT_CONFIG_KEY_{i}"] = k
+        env[f"GIT_CONFIG_VALUE_{i}"] = v
+    env["GIT_CONFIG_COUNT"] = str(n + len(pairs))
+    return env
+
+
+def apply_safe_git_env():
+    os.environ.update(git_config_env(SAFE_GIT_CONFIG))
+
 
 def _git_rev_parse_head(cwd):
     """Return the current HEAD SHA, or None if not a git repo / no commits."""

+ 2 - 0
plugins/security-guidance/hooks/llm.py

@@ -29,6 +29,7 @@ import extensibility
 import review_api
 from _base import debug_log, _record_usage, _record_http_error, _PV, PROVENANCE_TAG, state_dir as _resolve_state_dir  # noqa: F401
 from session_state import with_locked_state
+from gitutil import git_config_env, SAFE_GIT_CONFIG
 
 
 def _inject_agent_sdk_venv_into_syspath(state_dir):
@@ -1123,6 +1124,7 @@ def _agentic_spawn_env() -> Dict[str, str]:
         "GIT_SSH_COMMAND": "/bin/false",
         "GIT_TERMINAL_PROMPT": "0",
         "GIT_OPTIONAL_LOCKS": "0",
+        **git_config_env(SAFE_GIT_CONFIG),
     }
     if os.environ.get("ANTHROPIC_API_KEY"):
         # API key present → blank the OAuth token so API-key auth wins.

+ 15 - 12
plugins/security-guidance/hooks/security_reminder_hook.py

@@ -97,7 +97,7 @@ from session_state import (  # noqa: E402,F401
     load_state, save_state, with_locked_state,
 )
 from gitutil import (  # noqa: E402,F401
-    GIT_CMD,
+    GIT_CMD, apply_safe_git_env,
     _git_rev_parse_head, _find_git_index, _diff_pathspec, _temp_index,
     _git_toplevel, _git_dir, _git_rev_list_range, _git_diff_range,
     _detect_main_branch, _git_reflog_recent_commits, _git_name_only,
@@ -695,7 +695,7 @@ _GIT_COMMIT_RE = re.compile(
     # _GIT_PUSH_RE). Without this, `git -C /repo commit` is silently dropped
     # by the handler — see #2089's secondary finding. The gt branch has no
     # global-option layer to worry about.
-    r'\bgit(?:\s+-[Cc]\s+(?:"[^"]*"|\'[^\']*\'|\S+)|\s+--\S+=\S+'
+    r'\bgit(?:\s+-[Cc]\s+(?:"[^"]*"|\'[^\']*\'|[^\s"\']\S*)|\s+--[^\s=]+=\S+'
     r'|\s+--(?:git-dir|work-tree)\s+\S+)*\s+commit\b'
     r'|\bgt\s+(?:create|modify)\b'
 )
@@ -739,7 +739,7 @@ COMMIT_REVIEW_RATE_WINDOW_S = int(
 # but the bash hook fires on Claude's top-level command so we need to
 # recognize gt submit at the matcher level. See #2048.
 _GIT_PUSH_RE = re.compile(
-    r'(?:\bgit(?:\s+-[cC]\s+(?:"[^"]*"|\'[^\']*\'|\S+)|\s+--\S+=\S+'
+    r'(?:\bgit(?:\s+-[cC]\s+(?:"[^"]*"|\'[^\']*\'|[^\s"\']\S*)|\s+--[^\s=]+=\S+'
     r'|\s+--(?:git-dir|work-tree)\s+\S+)*\s+push\b|\bgt\s+submit\b)'
 )
 
@@ -1177,7 +1177,8 @@ def handle_commit_review_posttooluse(input_data):
         sys.exit(0)
     if repo_res != RES_CWD:
         debug_log(f"Commit review: repo resolved via {repo_res} -> {repo_root!r}")
-        save_repo_hint(session_id, repo_root)
+        if repo_res in (RES_COMMAND, RES_TOUCHED_PATHS):
+            save_repo_hint(session_id, repo_root)
 
     # Pin the review to the exact SHA the Bash command produced, parsed from
     # its stdout. Reviewing HEAD instead is wrong when the commit was made in
@@ -1612,7 +1613,8 @@ def handle_push_sweep_posttooluse(input_data):
         sys.exit(0)
     if repo_res != RES_CWD:
         debug_log(f"Push sweep: repo resolved via {repo_res} -> {repo_root!r}")
-        save_repo_hint(session_id, repo_root)
+        if repo_res in (RES_COMMAND, RES_TOUCHED_PATHS):
+            save_repo_hint(session_id, repo_root)
 
     # Guard: the sweep diffs `base..HEAD` and the agent Reads the working
     # tree, so the pushed ref MUST be HEAD or the review is of the wrong
@@ -1991,13 +1993,8 @@ def handle_stop_hook(input_data):
 
     repo_cwd, repo_res = resolve_repo_root(
         cwd, touched_paths=touched_paths, session_id=session_id)
-    res_metrics = {}
-    if repo_res != RES_CWD:
-        res_metrics = {"cwd_is_repo": False, "repo_resolution": repo_res}
-        if repo_cwd:
-            debug_log(f"Stop hook: repo resolved via {repo_res} -> {repo_cwd!r}")
-            save_repo_hint(session_id, repo_cwd)
-            cwd = repo_cwd
+    res_metrics = ({} if repo_res == RES_CWD
+                   else {"cwd_is_repo": False, "repo_resolution": repo_res})
     if is_subagent and repo_cwd:
         _pd = os.environ.get("CLAUDE_PROJECT_DIR")
         _pd_root = _git_toplevel(_pd) if _pd and os.path.isdir(_pd) else None
@@ -2005,6 +2002,11 @@ def handle_stop_hook(input_data):
             debug_log(f"Stop hook: SubagentStop in {repo_cwd!r}, session repo is {_pd_root!r}")
             v2_metrics = dict(res_metrics)
             _skip(11)
+    if repo_res != RES_CWD and repo_cwd:
+        debug_log(f"Stop hook: repo resolved via {repo_res} -> {repo_cwd!r}")
+        if not is_subagent and repo_res in (RES_COMMAND, RES_TOUCHED_PATHS):
+            save_repo_hint(session_id, repo_cwd)
+        cwd = repo_cwd
 
     review_paths, diff_base, repo_root, untracked, v2_metrics = compute_v2_review_set(
         cwd, baseline_sha, head_at_capture, untracked_at_baseline
@@ -2250,6 +2252,7 @@ def _maybe_bootstrap_agent_sdk_async():
 def main():
     """Main hook function."""
     debug_log(f"Hook called with args: {sys.argv}")
+    apply_safe_git_env()
 
     # Master kill switch — honors ENABLE_SECURITY_REMINDER=0 (legacy) and
     # SECURITY_GUIDANCE_DISABLE=1 (clearer name, no double negative). Emit

+ 88 - 3
plugins/security-guidance/tests/test_repo_resolution.py

@@ -1,15 +1,17 @@
 import json
 import os
+import subprocess
 import threading
 import time
 
 import pytest
 
 from conftest import (
-    HOOKS_DIR, STUB_VULN, VULN_PY, bash_payload, commit_file, edit_payload, git,
-    make_repo, metrics_of, run_hook, stop_payload, ups_payload,
+    GIT_ENV, HOOKS_DIR, STUB_VULN, VULN_PY, bash_payload, commit_file, edit_payload,
+    git, make_repo, metrics_of, run_hook, stop_payload, ups_payload,
 )
 
+import gitutil
 import reporesolve as rr
 import security_reminder_hook as hook
 
@@ -206,6 +208,16 @@ class TestRegexes:
     def test_commit_re_negative(self, cmd):
         assert not hook._GIT_COMMIT_RE.search(cmd)
 
+    @pytest.mark.parametrize("unit", ['-c "a"', "-c 'a'", "--no-a=b", "--a=b=c=d", "--git-dir x"])
+    @pytest.mark.parametrize("regex,verb", [("_GIT_COMMIT_RE", "commit"), ("_GIT_PUSH_RE", "push")])
+    def test_global_option_prefix_is_linear(self, regex, verb, unit):
+        cre = getattr(hook, regex)
+        prefix = "git " + " ".join([unit] * 40)
+        t0 = time.perf_counter()
+        assert not cre.search(prefix + " " + verb + "foo")
+        assert time.perf_counter() - t0 < 0.05
+        assert cre.search(prefix + " " + verb + " -m x")
+
 
 class TestHooksJson:
     def test_matchers_and_events(self):
@@ -229,6 +241,13 @@ def _read_state(env):
         return json.load(f)
 
 
+def _read_state_or_empty(env):
+    d = env["SECURITY_WARNINGS_STATE_DIR"]
+    if not any(e.name.endswith(".json") for e in os.scandir(d)):
+        return {}
+    return _read_state(env)
+
+
 class TestCommitReview:
     def test_cwd_is_repo_unchanged(self, workspace, hook_env, stub_api):
         ws, repo = workspace
@@ -288,6 +307,7 @@ class TestCommitReview:
         m = metrics_of(so)
         assert m.get("skip_reason") is None, m
         assert m["repo_resolution"] == rr.RES_SHA_SCAN and m["files_reviewed"] == 1
+        assert "repo_root_hint" not in _read_state_or_empty(hook_env)
 
     def test_workspace_cwd_sha_scan_via_project_dir(self, workspace, hook_env, tmp_path):
         ws, repo = workspace
@@ -368,6 +388,22 @@ class TestPushSweep:
         assert m.get("skip_reason") != 26
         assert m.get("pushed") == 1
 
+    def test_workspace_cwd_sha_scan_push_leaves_no_hint(self, workspace, hook_env, tmp_path):
+        ws, repo = workspace
+        remote = tmp_path / "remote.git"
+        git(ws, "init", "-q", "--bare", str(remote))
+        git(repo, "remote", "add", "origin", str(remote))
+        git(repo, "push", "-q", "-u", "origin", "main")
+        base = git(repo, "rev-parse", "HEAD").strip()
+        sha, _ = commit_file(repo, "app.py", VULN_PY)
+        git(repo, "push", "-q", "origin", "main")
+        push_stdout = f"To {remote}\n   {base[:7]}..{sha[:7]}  main -> main\n"
+        rc, so, se = run_hook(bash_payload(ws, "git push origin main", push_stdout), hook_env)
+        m = metrics_of(so)
+        assert m["push_sweep"] is True and m["repo_resolution"] == rr.RES_SHA_SCAN, m
+        assert m.get("skip_reason") != 26
+        assert "repo_root_hint" not in _read_state_or_empty(hook_env)
+
     def test_cwd_is_repo_dash_C_other_repo(self, workspace, hook_env, tmp_path):
         ws, repo = workspace
         other = make_repo(ws / "other")
@@ -424,6 +460,7 @@ class TestStop:
         assert after["touched_paths"] == before["touched_paths"] != []
         assert after.get("baseline_sha") == before.get("baseline_sha")
         assert after.get("reviewed_diff_hash")
+        assert "repo_root_hint" not in after
         n_calls = len(stub_api.calls)
         rc, so, se = run_hook(stop_payload(ws), hook_env)
         m2 = metrics_of(so)
@@ -501,6 +538,54 @@ class TestStop:
         m2 = metrics_of(so)
         assert m2.get("skip_reason") is None and m2["files_reviewed"] == 1, m2
 
+    def test_subagent_stop_in_other_worktree_from_workspace_cwd(self, workspace, hook_env, stub_api, tmp_path):
+        ws, repo = workspace
+        wt = tmp_path / "wt"
+        git(repo, "worktree", "add", "-q", "-b", "agent", str(wt))
+        run_hook(ups_payload(ws), hook_env)
+        (wt / "new.py").write_text("import pickle\npickle.loads(b)\n")
+        run_hook(edit_payload(ws, wt / "new.py", "x"), hook_env)
+        env = {**hook_env, "CLAUDE_PROJECT_DIR": str(repo)}
+        rc, so, se = run_hook(stop_payload(ws, event="SubagentStop"), env)
+        m = metrics_of(so)
+        assert m["skip_reason"] == 11 and m["repo_resolution"] == rr.RES_TOUCHED_PATHS, m
+        assert not stub_api.calls
+        assert "repo_root_hint" not in _read_state(hook_env)
+
+    def test_repository_config_cannot_run_programs(self, workspace, hook_env, stub_api, tmp_path):
+        ws, repo = workspace
+        marker = tmp_path / "marker"
+        mon = tmp_path / "mon.sh"
+        mon.write_text(f"#!/bin/sh\necho ran >> '{marker}'\n")
+        mon.chmod(0o755)
+        git(repo, "config", "core.fsmonitor", str(mon))
+        clean = {k: v for k, v in os.environ.items() if not k.startswith("GIT_CONFIG_")}
+        subprocess.run(["git", "status"], cwd=repo, env={**clean, **GIT_ENV}, capture_output=True)
+        assert marker.exists()
+        marker.unlink()
+        run_hook(ups_payload(ws), hook_env)
+        self._touch(ws, repo, hook_env)
+        rc, so, se = run_hook(stop_payload(ws), hook_env)
+        m = metrics_of(so)
+        assert m.get("skip_reason") is None and m["files_reviewed"] == 1, m
+        assert not marker.exists()
+        sha, out = commit_file(repo, "app.py", VULN_PY + "z = 3\n")
+        marker.unlink(missing_ok=True)
+        rc, so, se = run_hook(bash_payload(ws, "cd sub && git commit -m change", out), hook_env)
+        assert metrics_of(so).get("files_reviewed") == 1
+        assert not marker.exists()
+
+    def test_safe_git_env_extends_existing_config_count(self):
+        base = {"GIT_CONFIG_COUNT": "2", "GIT_CONFIG_KEY_0": "a.b", "GIT_CONFIG_VALUE_0": "1",
+                "GIT_CONFIG_KEY_1": "c.d", "GIT_CONFIG_VALUE_1": "2"}
+        env = gitutil.git_config_env(gitutil.SAFE_GIT_CONFIG, base=base)
+        assert env["GIT_CONFIG_COUNT"] == str(2 + len(gitutil.SAFE_GIT_CONFIG))
+        assert "GIT_CONFIG_KEY_0" not in env and "GIT_CONFIG_KEY_1" not in env
+        got = {env[f"GIT_CONFIG_KEY_{i}"]: env[f"GIT_CONFIG_VALUE_{i}"]
+               for i in range(2, int(env["GIT_CONFIG_COUNT"]))}
+        assert got == dict(gitutil.SAFE_GIT_CONFIG)
+        assert gitutil.git_config_env((("x.y", "z"),), base={"GIT_CONFIG_COUNT": "junk"})["GIT_CONFIG_COUNT"] == "1"
+
     def test_subagent_stop_same_repo_with_project_dir_reviews(self, workspace, hook_env, stub_api):
         ws, repo = workspace
         (repo / "pkg").mkdir()
@@ -519,7 +604,7 @@ class TestStop:
         ws, repo = workspace
         run_hook(ups_payload(ws), hook_env)
         self._touch(ws, repo, hook_env)
-        run_hook(stop_payload(ws, event="SubagentStop"), hook_env)
+        run_hook(stop_payload(ws), hook_env)
         (repo / "app.py").write_text(VULN_PY + "\n# more\n")
         run_hook(ups_payload(ws, session_id="s1"), hook_env)
         state = _read_state(hook_env)