fix(tool_risk): rate git -c <exec key>=… high, sharing repo_scan's predicate

.github/workflows/ci.yml

@@ -785,6 +785,7 @@ jobs:
             tests/test_openclaw_detection_real.py \
             tests/test_repo_scan_daemon_wiring.py \
             tests/test_guard_workspace_kinds.py \
+            tests/test_tool_risk.py \
             tests/test_no_shadowed_module_functions.py \
             tests/test_hooks_claude_code.py \
             tests/test_hook_ownership_quoted_launcher.py \

clawmetry/git_config_exec.py

@@ -0,0 +1,115 @@
+"""Which git config settings make git execute a program.
+
+One list, in a leaf, because two modules ask the same question and a copy in
+each is a copy that drifts. Neither format overlaps the other: literal exec
+keys (``core.hookspath``) and value-dependent ones (``alias.*``) are resolved
+by the same predicate so both callers rate identically.
+
+* :mod:`clawmetry.repo_scan` asks it of a ``.git/config`` on disk -- that is
+  the whole basis of the ``repo_config_exec`` finding;
+* :mod:`clawmetry.tool_risk` asks it of ``git -c <key>=<value>`` on a command
+  line, which is the same arbitrary code execution passed a different way.
+
+They disagreed: the scanner rated a poisoned ``core.hooksPath`` critical while
+the classifier rated the identical key on the command line ``medium``, and a
+``min_risk: high`` policy therefore held neither (clawmetry-pro#244).
+
+This module imports only ``re``. It exists so ``tool_risk`` can share the
+knowledge without importing ``repo_scan``: ``tool_risk`` is the leaf that
+``approvals`` and the routes import, and it stays one by depending on this
+constant module rather than on a scanner.
+
+It is a PREDICATE, not a name set. Executability is partly value-dependent --
+``alias.x`` is a shell command only when its value starts with ``!`` -- so a
+name-only export would either miss that or promote every benign alias.
+
+NOT here, deliberately: ``protocol.ext.allow``. It names no program; it enables
+the ``ext::`` transport so the command arrives in the remote URL instead. A
+caller that can see a whole command line handles it separately.
+"""
+from __future__ import annotations
+
+import re
+
+#: Git config keys whose VALUE is a program git runs.
+_EXEC_KEYS = (
+    "core.fsmonitor",
+    "core.hookspath",
+    "core.sshcommand",
+    "core.editor",
+    "core.pager",
+    "core.askpass",
+    "sequence.editor",
+    "credential.helper",
+    "uploadpack.packobjectshook",
+    "diff.external",
+    "gpg.program",
+    "init.templatedir",
+)
+
+#: Key shapes whose value is a program. ``alias.*`` is value-dependent.
+_EXEC_KEY_PATTERNS = (
+    re.compile(r"^filter\..+\.(clean|smudge|process)$"),
+    re.compile(r"^diff\..+\.(command|textconv)$"),
+    re.compile(r"^merge\..+\.driver$"),
+    re.compile(r"^alias\..+$"),          # only flagged when the value starts "!"
+)
+
+#: Values that are ordinary tools rather than payloads.
+_KNOWN_GOOD_PREFIXES = (
+    ("git-lfs", "clean"), ("git-lfs", "smudge"), ("git-lfs", "filter-process"),
+    ("git", "lfs"),
+    ("cat",), ("true",), ("false",),
+    ("rustfmt",), ("gofmt",), ("black",), ("prettier",),
+    ("less",), ("more",), ("delta",), ("diff-so-fancy",),
+)
+
+#: A value that chains, substitutes or redirects is never known-good, whatever
+#: it starts with -- ``git-lfs clean -- %f; curl attacker`` starts with git-lfs.
+#: Covers: sequence (;, &&, ||, &), pipe (|), substitution ($, `), redirect
+#: (>, <), comment injection (#), history expansion (!), subshell/brace grouping
+#: ( ) { }, and CR (\r) which some shells treat as a command separator.
+_SHELL_METACHARS = re.compile(r"[;&|`$><\n\r!#(){}]")
+
+
+def executes(full_key: str, value: str = "") -> bool:
+    """Does setting ``full_key`` to ``value`` make git run a program?
+
+    ``value`` defaults to empty for callers that see the key but not the value
+    (``git --config-env=k=ENVVAR`` names an environment variable). That is
+    conservative on purpose: literal exec keys still match, value-dependent
+    ones do not fire on a value we cannot see.
+    """
+    key = str(full_key or "").strip().lower()
+    val = str(value or "")
+    if key in _EXEC_KEYS:
+        return True
+    for rx in _EXEC_KEY_PATTERNS:
+        if rx.match(key):
+            if key.startswith("alias."):
+                return val.strip().startswith("!")
+            return True
+    return False
+
+
+def value_known_good(value: str) -> bool:
+    """Is this value a recognised ordinary tool rather than a payload?
+
+    ``core.pager=less`` executes by definition and is an entirely ordinary
+    thing to type, so a classifier that promotes on the key alone reports
+    noise on a common command.
+    """
+    if _SHELL_METACHARS.search(value or ""):
+        return False
+    tokens = str(value or "").split()
+    # An empty value sets the key to nothing, which runs nothing. Matches
+    # repo_scan's long-standing behaviour exactly -- this module was extracted
+    # from it, and a semantic change here would silently move every
+    # repo_config_exec verdict.
+    if not tokens:
+        return True
+    lowered = [t.lower() for t in tokens]
+    for prefix in _KNOWN_GOOD_PREFIXES:
+        if lowered[:len(prefix)] == list(prefix):
+            return True
+    return False

clawmetry/repo_scan.py

@@ -38,6 +38,8 @@ class of attack where the agent chooses nothing at all — GitSpawn being the
 import re
 from typing import Optional
 
+from clawmetry import git_config_exec as _gce
+
 #: Every file a scan READS, relative to the workspace. Declared here so the
 #: daemon's cache stamp cannot miss one: the stamp is what decides whether a
 #: repo is re-scanned, so a file the scanner reads but the stamp ignores means
@@ -70,40 +72,16 @@ class of attack where the agent chooses nothing at all — GitSpawn being the
 
 # Git config keys whose VALUE is a program git will execute. Section+key, lowered.
 # Sourced from git-config(1); the wildcard forms cover per-name subsections.
-_EXEC_KEYS = (
-    "core.fsmonitor",
-    "core.hookspath",
-    "core.sshcommand",
-    "core.editor",
-    "core.pager",
-    "core.askpass",
-    "sequence.editor",
-    "credential.helper",
-    "uploadpack.packobjectshook",
-    "diff.external",
-    "gpg.program",
-    "init.templatedir",
-)
-_EXEC_KEY_PATTERNS = (
-    re.compile(r"^filter\..+\.(clean|smudge|process)$"),
-    re.compile(r"^diff\..+\.(command|textconv)$"),
-    re.compile(r"^merge\..+\.driver$"),
-    re.compile(r"^alias\..+$"),          # only flagged when the value starts "!"
-)
+_EXEC_KEYS = _gce._EXEC_KEYS
+_EXEC_KEY_PATTERNS = _gce._EXEC_KEY_PATTERNS
 
 # Commands that legitimately appear in these keys in ordinary repositories.
 # Matched against the value's leading tokens, so `git-lfs clean -- %f` is
 # recognised while `git-lfs clean; curl evil` is not.
-_KNOWN_GOOD_PREFIXES = (
-    ("git-lfs", "clean"), ("git-lfs", "smudge"), ("git-lfs", "filter-process"),
-    ("git", "lfs"),
-    ("cat",), ("true",), ("false",),
-    ("rustfmt",), ("gofmt",), ("black",), ("prettier",),
-    ("less",), ("more",), ("delta",), ("diff-so-fancy",),
-)
+_KNOWN_GOOD_PREFIXES = _gce._KNOWN_GOOD_PREFIXES
 # A value that chains, substitutes or redirects is never "known good", whatever
 # it starts with — `git-lfs clean -- %f; curl attacker` starts with git-lfs.
-_SHELL_METACHARS = re.compile(r"[;&|`$><\n]|\$\(|\|\|")
+_SHELL_METACHARS = _gce._SHELL_METACHARS
 
 _TASKS_AUTORUN = re.compile(r'"runOn"\s*:\s*"folderOpen"', re.I)
 
@@ -121,16 +99,7 @@ def _sketch(value: str, limit: int = 80) -> str:
 
 
 def _is_known_good(value: str) -> bool:
-    if _SHELL_METACHARS.search(value or ""):
-        return False
-    tokens = str(value).split()
-    if not tokens:
-        return True
-    lowered = [t.lower() for t in tokens]
-    for prefix in _KNOWN_GOOD_PREFIXES:
-        if lowered[:len(prefix)] == list(prefix):
-            return True
-    return False
+    return _gce.value_known_good(value)
 
 
 # Hook managers that legitimately point core.hooksPath at the working tree.
@@ -289,16 +258,44 @@ def _parse_git_config(text: str) -> list:
     return out
 
 
+def git_config_value_known_good(value: str) -> bool:
+    """Is this config VALUE a recognised, ordinary tool rather than a payload?
+
+    Shared with :mod:`clawmetry.tool_risk` for the same reason as
+    :func:`git_config_executes`: ``core.pager=less`` executes by definition and
+    is an entirely ordinary thing to type, so a classifier that promotes it on
+    the key alone reports noise on a common command. A value that chains,
+    substitutes or redirects is never known-good whatever it starts with.
+    """
+    return _is_known_good(str(value or ""))
+
+
+def git_config_executes(full_key: str, value: str = "") -> bool:
+    """Does setting this git config key to this value make git run a program?
+
+    The single source of truth for "this git config executes", shared with
+    :mod:`clawmetry.tool_risk` so the on-disk scanner and the tool-stream
+    classifier cannot disagree about the same fact. It is a PREDICATE, not a
+    name set, because executability is partly value-dependent: ``alias.x`` is
+    a shell command only when its value starts with ``!``, and a name-only
+    export would either miss that or promote every benign alias.
+
+    ``value`` defaults to empty for callers that can see the key but not the
+    value (``git --config-env=k=ENVVAR`` names an environment variable, not
+    the program). That is deliberately conservative: literal exec keys still
+    match, and the value-dependent ones do not fire on an unknown value.
+
+    NOTE: this does NOT cover ``protocol.ext.allow``. That key names no
+    program — it enables the ``ext::`` transport so the command comes from the
+    URL argument instead. Callers that see a command line must handle it
+    separately; treating it as an exec key here would be wrong.
+    """
+    return _key_is_executable(str(full_key or "").strip().lower(),
+                              str(value or ""))
+
+
 def _key_is_executable(full_key: str, value: str) -> bool:
-    if full_key in _EXEC_KEYS:
-        return True
-    for rx in _EXEC_KEY_PATTERNS:
-        if rx.match(full_key):
-            # A git alias is only a shell command when it starts with "!".
-            if full_key.startswith("alias."):
-                return value.strip().startswith("!")
-            return True
-    return False
+    return _gce.executes(full_key, value)
 
 
 def _finding(kind: str, severity: str, title: str, detail: str,