From 85462df5abd880b18d38a322a682486a5a01ae32 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Fri, 6 Nov 2026 09:46:14 +0000 Subject: [PATCH] fix(kvb-org-operator): run no repo-configured command before the operator's yes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents name the directories of operator rows and can write the repos behind them. Before the operator answered, list and run started git there unsandboxed, and git ran commands the repo config names: core.fsmonitor, the post-index-change hook on an index refresh, filter clean drivers, the pager, external diff and textconv drivers, gpg through log.showSignature, and a lazy fetch of a missing object that runs remote uploadpack through sh. Preview output and task names also reached the terminal raw, so escape sequences could overwrite the displayed command. Every git call before the yes now runs as git -c core.fsmonitor=false -c core.hooksPath=/dev/null -c log.showSignature=false --no-pager --no-optional-locks with GIT_NO_LAZY_FETCH=1, diffs add --no-ext-diff --no-textconv --ignore-submodules=all, a repo whose config names filter commands gets no working-tree preview, and preview output, task, check, table cells and listed commands go through visible(). The approved command itself still runs through $SHELL -ic unchanged. Found by a skeptic while scanning the color change; each hole was reproduced red before the fix. Accepted MEDUSA false positives after kvb-think-criticize (rows in security-accepted.jsonl): MEDUSA-MM-SCAN-058 x3, MEDUSA-GENAI-SCAN-1270 x4, MEDUSA-WM-SCAN-354, MEDUSA-CGS-SCAN-040. 🎯 Fix: 90 % 💥 Break: 3 % 💸 Cost: – ⏱️ Time: – 🛡️ Security: 100 % 🏆 Score: 87 🚦 Gate: ✅ 90/5, cost and time <= 100, security 100 % ``` /home/kevinveenbirkenbach/Repositories/github.com/kevinveenbirkenbach/skills/.worktrees/feature/kvb-org-operator-run-color ``` Co-Authored-By: Claude Opus 5.5 --- skills/kvb-org-operator/scripts/operator.py | 60 +++++++++++++-------- tests/test_operator.py | 51 +++++++++++++++++- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/skills/kvb-org-operator/scripts/operator.py b/skills/kvb-org-operator/scripts/operator.py index b3afb04..c3bc231 100755 --- a/skills/kvb-org-operator/scripts/operator.py +++ b/skills/kvb-org-operator/scripts/operator.py @@ -31,7 +31,8 @@ Args: order. Per row it prints directory, command (control characters made visible), reason and check, then a preview of what the command changes, and asks [y]es / [s]kip / [d]iff / [q]uit; d shows the full - diff. y runs the command in its directory, then sets the row done + diff. A repo whose git config names filter commands gets no + preview. y runs the command in its directory, then sets the row done when its check passes (manual: when the command exits 0). A command the kvb-org-organize watch config calls heavy also shows the memory stage and `queue.py list`; once the config holds the queue keys and @@ -100,6 +101,9 @@ META = re.compile(r"[\s|&;()<>$`\\\"'*?\[\]{}~!#]") NAME = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._+-]*$") COMMIT = re.compile(r"\bcommit\b") HEADER = "| # | Directory | Command | Why | Check |" +# Trip-wire: agents name these repos, and git runs commands their config names (fsmonitor, hooks, pager, external diff, textconv, gpg, filters, lazy fetch) as the operator. +GIT = ["git", "-c", "core.fsmonitor=false", "-c", "core.hooksPath=/dev/null", "-c", "log.showSignature=false", "--no-pager", "--no-optional-locks"] +DIFF = [*GIT, "diff", "--no-ext-diff", "--no-textconv", "--ignore-submodules=all"] def fail(message: str) -> None: @@ -148,11 +152,15 @@ def problem(check: str) -> str | None: return path_problem(arg, kind) +def git_env() -> dict[str, str]: + return {**os.environ, "GIT_NO_LAZY_FETCH": "1"} + + def ahead(repo: str) -> int | None: try: result = subprocess.run( - ["git", "rev-list", "--count", "@{u}..HEAD"], - cwd=repo, capture_output=True, text=True, check=False, + [*GIT, "rev-list", "--count", "@{u}..HEAD"], + cwd=repo, env=git_env(), capture_output=True, text=True, check=False, ) except OSError: return None @@ -191,7 +199,7 @@ def passed(check: str) -> bool: def cell(text: object) -> str: - return " ".join(str(text).split()).replace("|", "\\|") + return visible(" ".join(str(text).split()), "").replace("|", "\\|") def cmd_add(rasci: ModuleType, args: argparse.Namespace) -> int: @@ -269,10 +277,10 @@ def cmd_list(rasci: ModuleType, args: argparse.Namespace) -> int: steps = [(number, row) for number, row in enumerate(listed, 1) if runnable(row)] for number, row in steps: print() - print(f"{number}. {row['task']}") + print(f"{number}. {visible(row['task'], '')}") print("```bash") print(f"cd {shlex.quote(row['directory'])}") - print(row["command"]) + print(visible(row["command"], "")) print("```") if steps: print() @@ -295,7 +303,16 @@ def visible(text: str, keep: str) -> str: def git_tree(directory: str) -> bool: try: return subprocess.run( - ["git", "rev-parse", "--is-inside-work-tree"], cwd=directory, capture_output=True, check=False + [*GIT, "rev-parse", "--is-inside-work-tree"], cwd=directory, env=git_env(), capture_output=True, check=False + ).returncode == 0 + except OSError: + return False + + +def filtered(directory: str) -> bool: + try: + return subprocess.run( + [*GIT, "config", "--get-regexp", r"^filter\."], cwd=directory, env=git_env(), capture_output=True, check=False ).returncode == 0 except OSError: return False @@ -305,23 +322,17 @@ def evidence(check: str, directory: str, full: bool) -> list[tuple[list[str], st kind, _, arg = check.partition(":") if kind == "pushed": if full: - return [(["git", "diff", "@{u}..HEAD"], arg)] - return [ - (["git", "--no-pager", "log", "--oneline", "@{u}..HEAD"], arg), - (["git", "--no-pager", "diff", "--stat", "@{u}..HEAD"], arg), - ] + return [([*DIFF, "@{u}..HEAD"], arg)] + return [([*GIT, "log", "--oneline", "@{u}..HEAD"], arg), ([*DIFF, "--stat", "@{u}..HEAD"], arg)] if kind == "installed": source, _, target = arg.partition("=") return [(["diff", "-ru" if full else "-rq", "--exclude=__pycache__", target, source], directory)] if kind == "absent": return [(["ls", "-la" if full else "-ld", "--", arg], directory)] - if kind == "manual" and git_tree(directory): + if kind == "manual" and git_tree(directory) and not filtered(directory): if full: - return [(["git", "diff", "HEAD"], directory)] - return [ - (["git", "--no-pager", "log", "-1", "--oneline"], directory), - (["git", "--no-pager", "status", "--short"], directory), - ] + return [([*DIFF, "HEAD"], directory)] + return [([*GIT, "log", "-1", "--oneline"], directory), ([*GIT, "status", "--short", "--ignore-submodules=all"], directory)] return [] @@ -331,9 +342,14 @@ def show(commands: list[tuple[list[str], str]]) -> None: for argv, cwd in commands: print(f"$ {shlex.join(argv)}", flush=True) try: - subprocess.run(argv, cwd=cwd, check=False) + done = subprocess.run( + argv, cwd=cwd, env=git_env(), stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, errors="replace", check=False + ) except OSError as error: print(f" {error}", flush=True) + continue + for line in done.stdout.splitlines(): + print(visible(line, "\t"), flush=True) def ask(holdable: bool, read: Callable[[str], str | None]) -> str: @@ -432,7 +448,7 @@ def cmd_granted(rasci: ModuleType, args: argparse.Namespace) -> int: file=sys.stderr) return NOT_RUN row = approved[0] - print(f"\ngranted {row['task']}: the queue lets it run now; its tree may have changed while it waited") + print(f"\ngranted {visible(row['task'], '')}: the queue lets it run now; its tree may have changed while it waited") print(f" directory: {visible(row['directory'], '')}") print(f" command: {visible(row['command'], '')}", flush=True) try: @@ -464,11 +480,11 @@ def cmd_run(rasci: ModuleType, args: argparse.Namespace) -> int: cfg = watch.load_config() for number, row in enumerate(steps, 1): check = "manual" if problem(row["check"]) else row["check"] - print(f"\n[{number}/{len(steps)}] {row['task']}") + print(f"\n[{number}/{len(steps)}] {visible(row['task'], '')}") print(f" directory: {visible(row['directory'], '')}") print(f" command: {visible(row['command'], '')}") print(f" why: {visible(row['why'] or '-', '')}") - print(f" check: {check}", flush=True) + print(f" check: {visible(check, '')}", flush=True) holdable = heavy_row(watch, cfg, row) answer = review(row, holdable, typed) if answer == "q": diff --git a/tests/test_operator.py b/tests/test_operator.py index a78c652..6c098ae 100644 --- a/tests/test_operator.py +++ b/tests/test_operator.py @@ -3,10 +3,12 @@ from __future__ import annotations import json +import os import shutil import subprocess import sys import tempfile +import time import unittest from pathlib import Path @@ -234,11 +236,58 @@ class TestOperator(unittest.TestCase): self._add("push", repo, "false", "branch ahead", f"pushed:{repo}") result = self._feed("y\n") self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("$ git --no-pager log --oneline '@{u}..HEAD'", result.stdout) + self.assertIn("--no-pager --no-optional-locks log --oneline '@{u}..HEAD'", result.stdout) self.assertRegex(result.stdout, r"[0-9a-f]{7,} two") self.assertIn("failed operator/push: exit 1; the row stays open", result.stdout) self.assertEqual(self._open(), ["operator/push"]) + def test_run_preview_runs_no_command_a_repo_config_names(self): + marker = self.base / "ran" + trap = self._repo("trap", upstream=False) + (trap / "keep").write_text("k", encoding="utf-8") + self._git(trap, "add", "keep") + self._git(trap, "commit", "-qm", "keep") + self._git(trap, "config", "core.fsmonitor", f"touch {marker}") + self._git(trap, "config", "diff.external", f"touch {marker}") + hook = trap / ".git" / "hooks" / "post-index-change" + hook.write_text(f"#!/bin/sh\ntouch {marker}\n", encoding="utf-8") + hook.chmod(0o755) + future = time.time() + 1000 + os.utime(trap / "keep", (future, future)) + filtered = self._repo("filtered", upstream=False) + self._git(filtered, "config", "filter.x.clean", f"touch {marker}") + (filtered / ".gitattributes").write_text("* filter=x\n", encoding="utf-8") + for repo in (trap, filtered): + (repo / "one").write_text("changed", encoding="utf-8") + self._add(repo.name, repo, "true", "r", "manual") + result = self._feed("d\ns\nd\ns\n") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(" M one", result.stdout) + self.assertIn("+changed", result.stdout) + self.assertNotIn("?? 2", result.stdout) + self.assertFalse(marker.exists()) + + def test_list_and_preview_never_fetch_a_missing_object(self): + marker = self.base / "fetched" + repo = self._repo("lazy", upstream=True) + self._git(repo, "config", "remote.origin.promisor", "true") + self._git(repo, "config", "remote.origin.uploadpack", f"touch {marker}; git-upload-pack") + (repo / ".git" / "refs" / "remotes" / "origin" / "main").write_text("1" * 40 + "\n", encoding="utf-8") + self._add("lazy", repo, "true", "r", f"pushed:{repo}") + self._out("list") + self.assertEqual(self._feed("s\n").returncode, 0) + self.assertFalse(marker.exists()) + + def test_run_shows_control_characters_of_the_preview(self): + repo = self._repo("spoof", upstream=True) + (repo / "three").write_text("3", encoding="utf-8") + self._git(repo, "add", "three") + self._git(repo, "commit", "-qm", "x\x1b[1A\x1b[2Kfake") + self._add("spoof", repo, "true", "r", f"pushed:{repo}") + out = self._feed("s\n").stdout + self.assertNotIn("\x1b", out) + self.assertIn("x\\x1b[1A\\x1b[2Kfake", out) + def test_decision_rows_stay_out_of_blocks_and_run(self): self._add("ask", self.base, "decide: keep fix B", "operator only", "decision") self._add("go", self.base, "true", "runnable", "manual")