diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 4b7d509..5eedeb3 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -4,4 +4,15 @@ set -eu . "$(dirname "$0")/_resolve" + +# Containment in gate.py only runs when RALPH_LOOP is set, and that variable is lost the moment +# anything clears the environment. Ask the detector instead of trusting it: it is fail-closed, so +# containment goes back on unless a human is at an interactive terminal. Humans keep --no-verify. +detector="$(dirname "$0")/../harness/agent_detect.sh" +# Fail closed both ways: if the detector is missing or not executable, assume an agent. +if [ ! -x "$detector" ] || "$detector" >/dev/null; then + RALPH_LOOP=1 + export RALPH_LOOP +fi + exec "$HARNESS" preflight diff --git a/.githooks/pre-push b/.githooks/pre-push index 52ba4ef..f6bb5cc 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -7,4 +7,13 @@ set -eu . "$(dirname "$0")/_resolve" + +# Same reasoning as pre-commit: the detector decides, not the environment variable. +detector="$(dirname "$0")/../harness/agent_detect.sh" +# Fail closed both ways: if the detector is missing or not executable, assume an agent. +if [ ! -x "$detector" ] || "$detector" >/dev/null; then + RALPH_LOOP=1 + export RALPH_LOOP +fi + exec "$HARNESS" gate diff --git a/AGENTS.md b/AGENTS.md index e1c295f..dc02672 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,7 @@ - The quality minimum is code in `harness/gate.py` (forbidden to agents). Leave it unchallenged. - Strengthen tests and coverage. Pass lint, type, and gate checks. - Avoid lint suppressions, type-ignores, skipped/xfail tests, or broad exception swallowing. +- Do not write code that only looks finished: no `...` / `raise NotImplementedError` stubs, no `time.sleep()` to paper over a race. - Never run destructive git commands (`rm -rf`, `git reset --hard`, `git branch -D`) unless the user explicitly asks; verify each risky step. - Never bypass or reconfigure git hooks. diff --git a/README.md b/README.md index 60cc24a..21893ff 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,37 @@ chaotic_continue_statements # abolish unecessary nested continue statements, cl complex_comprehension # no needlessly dense list/set/dict comprehensions, prefer linear code ``` +## Agent detection (containment should not fail open) + +`gate.py` switches every containment check on `RALPH_LOOP`, and `ralph.sh` is what exports it. Lose that variable any way — an agent unsets it, a subshell starts with a cleared environment, an IDE agent that `ralph.sh` never launched — and forbidden-path ejection, the banned-pattern scan and the preferences walk all switch off silently. + +[`harness/agent_detect.sh`](harness/agent_detect.sh) decides the same question without reading it, and both hooks turn containment back on when it says agent. **It is fail-closed**: containment is assumed on, and the only thing that turns it off is positive evidence of a human. + +| signal | catches | misses | +|---|---|---| +| 1. environment marker (`CLAUDECODE`, `CODEX_*`, …) | agents that advertise themselves | most IDE agents | +| 2. agent binary in the process ancestry | a CLI agent that unset its own variables | IDE agents — the chain is `node`/`code`, never an agent binary | +| 3. **no controlling terminal** | everything above, plus IDE agents, CI, scripts | an agent that allocates a pty and adopts it (signals 1–2 cover it) | + +Signal 3 is the one that generalises. A human types `git commit` at a terminal — including the IDE's own integrated terminal — so the process has a *controlling* terminal. An agent spawning git through `child_process`/`subprocess` has none, whatever editor it lives in. + +It has to be the **controlling** terminal, not `[ -t 0 ]`. Git redirects a hook's stdin, so inside a hook — the only place this runs — `[ -t 0 ]` is false for humans too. Measured in a real `pre-commit`: + +| | `[ -t 0 ]` | controlling tty | `/dev/tty` openable | +|---|---|---|---| +| human at a terminal | no | `pts/0` | yes | +| agent, no pty | no | `?` | no | + +Verified end to end by committing a forbidden-path edit through the real hook: as a human it lands, as an agent it is ejected, and with the detector removed it is ejected too (the hooks fail closed if it is missing). + +**Known false positive, in the safe direction:** committing from a GUI button (VS Code's Source Control panel, GitKraken) has no pty either, so a human doing that gets containment. The cost is an unstaged file and a message saying why; `--no-verify` is still the escape hatch. The opposite mistake leaves no trace at all. + +```sh +harness/agent_detect.sh --explain # run inside your editor/agent to see which signals fire +``` + +The marker and process-name lists are short on purpose. Confirmed from this repo's own runs: `RALPH_LOOP`, `CLAUDECODE`, `CLAUDE_CODE_ENTRYPOINT`, and the three `CODEX_*` names. The rest are reported names **not verified here** — `--explain` in your own tool is how you replace a guess with a fact. +
diff --git a/harness/agent_detect.sh b/harness/agent_detect.sh new file mode 100755 index 0000000..98fb96c --- /dev/null +++ b/harness/agent_detect.sh @@ -0,0 +1,156 @@ +#!/usr/bin/env bash +# Answer one question: should containment be ON for this git invocation? +# +# WHY. gate.py switches every containment check on RALPH_LOOP, and ralph.sh is what exports it. Lose +# that variable any way -- an agent unsets it, a subshell starts with a cleared environment, a +# wrapper re-execs, an IDE agent that ralph.sh never launched -- and forbidden-path ejection, the +# banned-pattern scan and the preferences walk all switch off silently. The gate fails open. +# +# THE DESIGN IS FAIL-CLOSED. Containment is assumed ON. The only thing that turns it off is positive +# evidence of a human: an interactive terminal, with no agent fingerprint on it. Everything else -- +# CLI agent, IDE agent, CI, a script, a cron job -- is treated as an agent. This is deliberate. An +# earlier version of this script tried to enumerate agents and switched containment on only for a +# name it recognised, which meant every agent it had not heard of walked straight through. +# +# THREE SIGNALS, checked in order. Any one of them means "agent". +# +# 1. ENVIRONMENT MARKER. Cheap and exact when present, absent for most IDE agents. +# 2. PROCESS ANCESTRY. Catches a CLI agent that unset its own variables, because a process cannot +# remove itself from its own process tree. Useless for an IDE agent: VS Code, Cursor and Zed +# run the agent inside the extension host, so the ancestor chain is `node`/`code`, never an +# agent binary. +# 3. NO CONTROLLING TERMINAL. The one signal that generalises. A human running `git commit` types +# it at a terminal -- including the IDE's own integrated terminal -- so the process has a +# controlling terminal. An agent spawning git through child_process/subprocess has none, +# whatever IDE it lives in. Note CONTROLLING: git redirects a hook's stdin, so `[ -t 0 ]` is +# false for humans too and cannot be used here. See the measurements at signal 3 below. +# +# KNOWN FALSE POSITIVE, and it is the safe direction: committing from a GUI button (VS Code's Source +# Control panel, GitKraken) has no pty either, so a human doing that is treated as an agent and gets +# containment. The cost is an unstaged forbidden file and a message saying so. `--no-verify` remains +# the documented human escape hatch. The opposite mistake -- silently letting an agent through -- +# leaves no trace at all. +# +# Run `harness/agent_detect.sh --explain` inside your own editor, agent or terminal to see exactly +# which signals fire there. That output is how you extend the two lists below for a tool not yet +# named in them. +# +# Exit 0: treat as agent, containment ON. Exit 1: human at an interactive terminal. + +set -uo pipefail + +# Marker variables an agent runtime exports for itself. NOT credentials like ANTHROPIC_API_KEY: a +# human exports those too, and treating them as proof of an agent punishes the wrong person. +# Confirmed present in this repo's own runs: RALPH_LOOP (ralph.sh), CLAUDECODE and +# CLAUDE_CODE_ENTRYPOINT (observed), and the three CODEX_* names (pyproject.toml clears them when +# launching codex). The rest are reported names that have NOT been confirmed here -- they cost +# nothing when absent, and `--explain` in your own tool is how you replace a guess with a fact. +AGENT_ENV_VARS=" +RALPH_LOOP +CLAUDECODE +CLAUDE_CODE_ENTRYPOINT +CLAUDE_CODE_SIMPLE +CODEX_THREAD_ID +CODEX_CONVERSATION_ID +CODEX_SESSION_ID +CURSOR_AGENT +GITHUB_COPILOT_AGENT +AIDER_CHAT +GEMINI_CLI +" + +# Binaries that mean an agent is driving when one appears in the parent chain. Editor names are +# deliberately absent: `code` in the ancestry means an IDE, and an IDE holds humans and agents both. +# Signal 3 is what separates them. +AGENT_PROCESS_NAMES="claude codex copilot agy aider" + +explain="" +if [ "${1:-}" = "--explain" ]; then + explain="yes" +fi + +# ---------------------------------------------------------------- signal 1: environment markers +marker_found="" +for variable in $AGENT_ENV_VARS; do + if [ -n "${!variable:-}" ]; then + marker_found=$variable + break + fi +done + +# ------------------------------------------------------------------- signal 2: process ancestry +# `ps -o ppid=,comm=` is the portable spelling; Linux and macOS agree on it. The walk is +# depth-bounded so a malformed or circular chain can never hang a commit. +ancestor_found="" +chain="" +inspected=$PPID +for _ in $(seq 1 12); do + case "$inspected" in + '' | 0 | 1 | *[!0-9]*) break ;; + esac + line=$(ps -o ppid=,comm= -p "$inspected" 2>/dev/null) || break + [ -n "$line" ] || break + parent=$(echo "$line" | awk '{print $1}') + command_name=$(echo "$line" | awk '{print $2}') + command_name=${command_name##*/} # strip any path, keep the bare binary name + chain="$chain $command_name" + for candidate in $AGENT_PROCESS_NAMES; do + case "$command_name" in + "$candidate" | "$candidate".* | "$candidate"-*) + [ -n "$ancestor_found" ] || ancestor_found=$command_name + ;; + esac + done + inspected=$parent +done + +# ------------------------------------------------------------ signal 3: no controlling terminal +# A human typed this at a terminal, so the process has a CONTROLLING terminal. Anything that spawned +# git programmatically has none, whether it lives in a shell, an editor or CI. +# +# It must be the controlling terminal and NOT `[ -t 0 ]`. Git runs hooks with stdin redirected, so +# inside a hook -- the only place this script is used -- `[ -t 0 ]` is false for a human too, and +# every commit would look like an agent. Measured in a real pre-commit hook: +# human at a terminal: [ -t 0 ] no | controlling tty pts/0 | /dev/tty openable yes +# agent, no pty: [ -t 0 ] no | controlling tty ? | /dev/tty openable no +# +# Two ways of asking, because either can be unavailable: opening /dev/tty is the POSIX question, and +# `ps -o tty=` is the readable one. Either answering "yes" is enough. An agent that allocates a pty +# AND adopts it as its controlling terminal defeats this, which is what signals 1 and 2 are for. +terminal_name=$(ps -o tty= -p $$ 2>/dev/null | tr -d ' ') +interactive="" +if (: < /dev/tty) 2>/dev/null; then + interactive="yes" +fi +case "$terminal_name" in + '' | '?' | '??' | '-') ;; + *) interactive="yes" ;; +esac + +if [ -n "$explain" ]; then + echo "environment marker : ${marker_found:-none of the known names are set}" + echo "agent in ancestry : ${ancestor_found:-none}" + echo "ancestor chain :${chain:- (empty)}" + echo "controlling terminal: ${terminal_name:-none}" + echo "human at a terminal : ${interactive:-no}" + if [ -n "$marker_found" ] || [ -n "$ancestor_found" ] || [ -z "$interactive" ]; then + echo "verdict : AGENT (containment on)" + else + echo "verdict : human (containment off)" + fi +fi + +if [ -n "$marker_found" ]; then + [ -n "$explain" ] || echo "agent detected: environment marker $marker_found" + exit 0 +fi +if [ -n "$ancestor_found" ]; then + [ -n "$explain" ] || echo "agent detected: process ancestor $ancestor_found" + exit 0 +fi +if [ -z "$interactive" ]; then + [ -n "$explain" ] || echo "agent assumed: no controlling terminal (nothing interactive is attached)" + exit 0 +fi + +exit 1 diff --git a/harness/tests/test_agent_detect.py b/harness/tests/test_agent_detect.py new file mode 100644 index 0000000..94ca6da --- /dev/null +++ b/harness/tests/test_agent_detect.py @@ -0,0 +1,127 @@ +"""Tests for harness/agent_detect.sh, which decides whether containment stays on for a commit. + +The script is fail-closed: it exits 0 (treat as agent) unless it can see a human at an interactive +terminal. These tests pin that direction, because the failure that matters is the silent one -- +an agent walking through with containment off leaves no trace, while a human caught by mistake +gets a message and can use --no-verify. + +Every case supplies a stub `ps` on PATH. The suite is normally started by an agent, so the real `ps` +reports that agent in the script's own ancestry and signal 2 fires before the case under test can. +Detaching the child does not help: orphans here re-parent to a subreaper that leads back to the same +agent. `ps` is the seam the script already uses, so replacing it is the honest way in. +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +SCRIPT = Path(__file__).resolve().parents[2] / "harness" / "agent_detect.sh" + +# No agent marker set. PATH is filled in per-case so the stub `ps` is found before the real one. +BARE_ENV = {"PATH": os.environ.get("PATH", "/usr/bin:/bin")} + +# `ps -o ppid=,comm= -p N` answers with a parent of 1, ending the walk after one harmless step. +# `ps -o tty= -p N` answers "?", the no-controlling-terminal marker. Together: no agent, no human. +NO_AGENT_NO_TERMINAL = """#!/bin/sh +case "$*" in + *tty*) echo "?" ;; + *) echo "1 sh" ;; +esac +""" + +# No agent, but a controlling terminal: a human typing at a shell. +NO_AGENT_AT_TERMINAL = """#!/bin/sh +case "$*" in + *tty*) echo "pts/3" ;; + *) echo "1 sh" ;; +esac +""" + +# An agent binary in the ancestry, at a terminal. This is the CLI-agent case. +AGENT_AT_TERMINAL = """#!/bin/sh +case "$*" in + *tty*) echo "pts/3" ;; + *) echo "1 codex" ;; +esac +""" + + +def run( + tmp_path: Path, ps_stub: str, *arguments: str, **markers: str +) -> subprocess.CompletedProcess[str]: + """Run the script against a stub `ps` that decides what ancestry and terminal it sees.""" + stub_directory = tmp_path / "bin" + stub_directory.mkdir(exist_ok=True) + stub = stub_directory / "ps" + stub.write_text(ps_stub) + stub.chmod(0o755) + + environment = {**BARE_ENV, **markers, "PATH": f"{stub_directory}:{BARE_ENV['PATH']}"} + # start_new_session detaches the child from any controlling terminal, so /dev/tty is never + # openable and the stub's `ps -o tty=` answer is the only thing deciding signal 3. Without it a + # developer running pytest from a terminal would get different results than CI does. + return subprocess.run( + [str(SCRIPT), *arguments], + env=environment, + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + check=False, + start_new_session=True, + ) + + +def test_no_terminal_is_treated_as_an_agent(tmp_path: Path) -> None: + """The fail-closed rule, and the only one that catches an IDE agent. + + An editor extension spawns git through child_process with no pty, leaves no agent name in the + process tree, and exports no marker variable. Nothing else in the script would see it. + """ + completed = run(tmp_path, NO_AGENT_NO_TERMINAL) + assert completed.returncode == 0 + assert "no controlling terminal" in completed.stdout + + +def test_interactive_terminal_without_markers_is_a_human(tmp_path: Path) -> None: + """A human typing `git commit` has a pty on stdin and no agent fingerprint: containment off.""" + completed = run(tmp_path, NO_AGENT_AT_TERMINAL) + assert completed.returncode == 1 + assert not completed.stdout + + +def test_agent_in_the_ancestry_beats_an_interactive_terminal(tmp_path: Path) -> None: + """A CLI agent that unset its own variables is still in the process tree it runs in.""" + completed = run(tmp_path, AGENT_AT_TERMINAL) + assert completed.returncode == 0 + assert "process ancestor codex" in completed.stdout + + +def test_marker_variable_beats_an_interactive_terminal(tmp_path: Path) -> None: + """Some agents allocate a pty for their shell tool, so signal 3 alone would not be enough.""" + completed = run(tmp_path, NO_AGENT_AT_TERMINAL, CLAUDECODE="1") + assert completed.returncode == 0 + assert "environment marker CLAUDECODE" in completed.stdout + + +def test_an_empty_marker_variable_does_not_count(tmp_path: Path) -> None: + """`RALPH_LOOP=` exported empty is not a claim that an agent is running.""" + completed = run(tmp_path, NO_AGENT_AT_TERMINAL, RALPH_LOOP="") + assert completed.returncode == 1 + + +def test_explain_reports_every_signal_and_a_verdict(tmp_path: Path) -> None: + """--explain is how someone confirms behaviour in an editor this repo cannot test from here.""" + completed = run(tmp_path, NO_AGENT_NO_TERMINAL, "--explain") + assert completed.returncode == 0 + for label in ("environment marker", "agent in ancestry", "human at a terminal", "verdict"): + assert label in completed.stdout + assert "AGENT" in completed.stdout + + +def test_explain_says_human_when_a_terminal_is_attached(tmp_path: Path) -> None: + """The same report, on the other side of the decision.""" + completed = run(tmp_path, NO_AGENT_AT_TERMINAL, "--explain") + assert completed.returncode == 1 + assert "verdict : human" in completed.stdout diff --git a/pyproject.toml b/pyproject.toml index 27d863e..f1efdb3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -169,7 +169,22 @@ PATTERNS = [ "fmt: skip", "yapf: disable", "complexipy: ignore", - "pragma: no mutate" + "pragma: no mutate", + # Literal-string offenders. These need no AST check: a pattern is zero code and zero tests, and + # it also catches the occurrences inside strings and comments that an AST walk never sees. + "time.sleep(", # blocking wait papering over a race or a flaky test + "__import__", # hides a dependency from the reader and the import graph + "assert_called", # mock interaction asserts: pin HOW code was called, not what it returned + "assert_any_call", + "assert_has_calls", + "assert_not_called", + ".call_count", + ".mock_calls", + # Unresolved merge-conflict markers. Ruff catches these in .py only, as a syntax error, so a + # conflicted .json/.yml/.toml would otherwise sail through. '=======' is omitted on purpose: it + # collides with ASCII banners in code. A real conflict always carries these two. + "<<<<<<<", + ">>>>>>>", ] # ==============================================================================