diff --git a/src/agent_tools/subprocess_tools.py b/src/agent_tools/subprocess_tools.py index 2daf4e2fd..26a5c0270 100644 --- a/src/agent_tools/subprocess_tools.py +++ b/src/agent_tools/subprocess_tools.py @@ -922,42 +922,6 @@ class BashTool: if "/tmp/" in content: isolated_tmp = _isolated_tmp_dir(agent_cwd()) content = content.replace("/tmp/", isolated_tmp.rstrip("/") + "/") - progress_cb = ctx.get("progress_cb") - _subproc_env = ctx.get("subproc_env") - session_id = ctx.get("session_id") - if not IS_WINDOWS and session_id and shutil.which("tmux"): - try: - content, boundary, _confined = _contained_command(content, agent_cwd()) - except containment.ContainmentUnavailable as exc: - return containment.unavailable_tool_result(exc, tool="bash") - stdout, stderr, rc, timed_out = await _run_tmux_bash( - content, - session_id=str(session_id), - cwd=agent_cwd(), - env=_subproc_env, - timeout=DEFAULT_BASH_TIMEOUT, - progress_cb=progress_cb, - ) - if timed_out: - return { - "error": f"bash: timed out after {DEFAULT_BASH_TIMEOUT}s — terminated task shell session", - "exit_code": 124, - "stdout": _truncate(stdout, MAX_OUTPUT_CHARS), - "stderr": _truncate(stderr, MAX_OUTPUT_CHARS), - "tmux_session": _tmux_session_name(str(session_id)), - "containment": boundary, - } - output = stdout.rstrip() - err = stderr.rstrip() - if err: - output = (output + "\nSTDERR: " + err).strip() if output else "STDERR: " + err - return { - "output": _truncate(output, MAX_OUTPUT_CHARS) or "(no output)", - "exit_code": rc or 0, - "tmux_session": _tmux_session_name(str(session_id)), - "containment": boundary, - } - return await _run_owned_command(content, ctx, tool="bash", timeout=DEFAULT_BASH_TIMEOUT) class HostShellTool: diff --git a/src/containment.py b/src/containment.py index 3634a434f..70d6b21c8 100644 --- a/src/containment.py +++ b/src/containment.py @@ -446,6 +446,8 @@ def _write_record(grant: ContainmentGrant) -> None: records[grant.id] = { "id": grant.id, "owner": grant.owner, + "manager_pid": os.getpid(), + "manager_token": process_ownership.start_token(os.getpid()), "mechanism": grant.mechanism, "mode": grant.mode, "workspace": grant.workspace, diff --git a/src/process_reaper.py b/src/process_reaper.py index 706acaeee..762c0bc26 100644 --- a/src/process_reaper.py +++ b/src/process_reaper.py @@ -78,6 +78,11 @@ def reap_containment_grants() -> Dict[str, Any]: # supervisor owns the wall clock and teardown, independently. report["background_kept"] = report.get("background_kept", 0) + 1 continue + if record.get("lifetime") != "cleanup" and record.get("manager_pid") and process_ownership.verify( + record["manager_pid"], record.get("manager_token"), + ) == process_ownership.OWNED: + report["manager_kept"] = report.get("manager_kept", 0) + 1 + continue verdict = process_ownership.verify_record(record) if verdict == process_ownership.GONE: if containment._group_present(record.get("pgid")): @@ -144,6 +149,121 @@ def reap_bg_jobs() -> Dict[str, Any]: return {"seen": 0, "retired": 0, "kept": 0} +def reap_legacy_agent_tmux() -> Dict[str, Any]: + """Retire this runtime's legacy agent shells; a name prefix is not ownership. + + Match the original clean Bash launcher and this runtime's HOME marker on + every pane. Snapshot session/server identities and process start tokens + before teardown; ambiguous sessions remain visible and unsignalled. + """ + import os + import re + import shlex + import shutil + import subprocess + import uuid + from src import containment + from src.constants import DATA_DIR + + report = {"seen": 0, "torn_down": 0, "unverifiable": 0, "failed": 0} + tmux = shutil.which("tmux") + if os.name == "nt" or not tmux: + return report + pattern = "#{session_id}\t#{session_name}\t#{session_created}\t#{pane_pid}\t#{pane_id}\t#{pid}\t#{pane_start_command}" + def snapshot(): + result = subprocess.run([tmux, "list-panes", "-a", "-F", pattern], + capture_output=True, text=True, timeout=5) + if result.returncode: + if not result.stdout and any(message in result.stderr.lower() for message in ("no server", "no sessions", "error connecting")): + return {} + raise RuntimeError("tmux pane discovery failed") + sessions = {} + for line in result.stdout.splitlines(): + fields = line.split("\t", 6) + if len(fields) != 7 or not fields[1].startswith("ody-agent-"): + continue + sessions.setdefault(fields[0], []).append(tuple(fields)) + return {key: sorted(rows) for key, rows in sessions.items()} + + def launcher_is_ours(command): + try: + argv = shlex.split(command) + except ValueError: + return False + if not argv or argv.pop(0) != "env": + return False + env = {} + while argv and "=" in argv[0]: + key, value = argv.pop(0).split("=", 1) + if key not in {"PATH", "VIRTUAL_ENV", "HOME", "TMPDIR", "TERM", "COLUMNS", "LINES"}: + return False + env[key] = value + return argv == ["/bin/bash", "--noprofile", "--norc"] and env.get("HOME") == DATA_DIR + + try: + sessions = snapshot() + for session_id, panes in sessions.items(): + report["seen"] += 1 + if not re.fullmatch(r"\$\d+", session_id) or not all(launcher_is_ours(row[6]) for row in panes): + report["unverifiable"] += 1 + continue + server_pid = int(panes[0][5]) + server_token = process_ownership.start_token(server_pid) + roots = [int(row[3]) for row in panes] + table = process_ownership.process_table() + if not all(pid in table and table[pid].ppid == server_pid and shlex.split(table[pid].command) == [ + "/bin/bash", "--noprofile", "--norc", + ] for pid in roots): + # A stale pane PID can now name a bystander. Its parent and + # current launcher must still match the observed tmux server. + report["unverifiable"] += 1 + continue + targets = process_ownership.descendants(roots, table=table) + identities = {pid: process_ownership.start_token(pid) for pid in targets} + if snapshot().get(session_id) != panes or process_ownership.verify(server_pid, server_token) != process_ownership.OWNED or any( + process_ownership.verify(pid, identities[pid]) != process_ownership.OWNED for pid in roots + ): + report["unverifiable"] += 1 + continue + # Persist every positively identified tree before touching it. A + # failed teardown then remains discoverable even if its pane dies. + tracked = [] + for pid in reversed(targets): + if process_ownership.verify(pid, identities[pid]) != process_ownership.OWNED: + continue + spec = containment.ContainmentSpec(workspace=os.getcwd(), env={}, wall_clock_s=1, + required=frozenset({containment.PROCESS_TREE})) + grant = containment.ContainmentGrant( + id=uuid.uuid4().hex[:12], mechanism="process_group", workspace=spec.workspace, + enforced=frozenset({containment.PROCESS_TREE}), degraded=(), unenforced_required=(), + owner=f"legacy-tmux:{session_id}", mode=containment.MODE_ENFORCING, + spec=spec, pid=pid, pgid=containment._pgid_of(pid), + ) + containment._write_record(grant) + containment._update_record(grant.id, lifetime="cleanup", start_token=identities[pid]) + tracked.append((grant, identities[pid])) + dead = True + for grant, token in tracked: + outcome = containment.release(grant, start_token=token, require_identity=True) + dead = dead and outcome.dead + remaining = snapshot().get(session_id) + if remaining and dead: + # Use the immutable tmux session id, not its reusable name. + if remaining != panes or process_ownership.verify(server_pid, server_token) != process_ownership.OWNED: + dead = False + else: + result = subprocess.run([tmux, "kill-session", "-t", session_id], + capture_output=True, timeout=5) + dead = result.returncode == 0 and session_id not in snapshot() + report["torn_down" if dead else "failed"] += 1 + except Exception: + report["failed"] += 1 + logger.warning("process_reaper: legacy agent tmux cleanup failed", exc_info=True) + if report["unverifiable"]: + logger.warning("process_reaper: left %s legacy tmux sessions without positive ownership", report["unverifiable"]) + return report + + def reap_orphans() -> Dict[str, Any]: """Run both reconciliations. Returns a report; raises nothing. @@ -154,6 +274,7 @@ def reap_orphans() -> Dict[str, Any]: "mechanism": process_ownership.inspection_mechanism(), "grants": reap_containment_grants(), "bg_jobs": reap_bg_jobs(), + "agent_tmux": reap_legacy_agent_tmux(), } if report["mechanism"] == process_ownership.MECHANISM_NONE: logger.error( diff --git a/tests/test_agent_tmux_retirement.py b/tests/test_agent_tmux_retirement.py new file mode 100644 index 000000000..9f4989667 --- /dev/null +++ b/tests/test_agent_tmux_retirement.py @@ -0,0 +1,134 @@ +"""Native Bash does not resurrect tmux; legacy cleanup requires identity.""" +import asyncio +from types import SimpleNamespace + +import pytest + +from src import containment, process_ownership, process_reaper, tool_execution +from src.agent_tools import subprocess_tools +from src.constants import DATA_DIR +from tests.containment_helpers import capture_owned_spawn + + +async def test_a_chat_session_always_uses_the_owned_runner(monkeypatch, tmp_path): + captured = capture_owned_spawn(monkeypatch, tmp_path) + monkeypatch.setattr(tool_execution, "agent_cwd", lambda: str(tmp_path)) + original = subprocess_tools.shutil.which + monkeypatch.setattr(subprocess_tools.shutil, "which", lambda name: "/fake/tmux" if name == "tmux" else original(name)) + async def forbidden(*args, **kwargs): + pytest.fail("native Bash resurrected a persistent tmux shell") + monkeypatch.setattr(subprocess_tools, "_run_tmux_bash", forbidden) + result = await subprocess_tools.BashTool().execute("printf ok", {"session_id": "same-chat"}) + assert result["output"] == "ok" + assert result["teardown"]["dead"] is True + assert "tmux_session" not in result + assert captured["kwargs"].get("start_new_session") is True + + +@pytest.fixture +def legacy(monkeypatch, tmp_path): + import shlex + import subprocess + import shutil + monkeypatch.setattr(containment, "_store_path", lambda: tmp_path / "grants.json") + monkeypatch.setattr(shutil, "which", lambda name: "/usr/bin/tmux" if name == "tmux" else None) + launcher = f"env HOME={shlex.quote(DATA_DIR)} TERM=xterm-256color /bin/bash --noprofile --norc" + row = f"$8\tody-agent-chat\t1234\t4200\t%9\t4100\t{launcher}\n" + state = {"rows": row, "calls": [], "released": []} + def run(argv, **kwargs): + state["calls"].append(argv) + if argv[1] == "list-panes": + return SimpleNamespace(returncode=0, stdout=state["rows"], stderr="") + if argv[1] == "kill-session": + state["rows"] = "" + return SimpleNamespace(returncode=0, stdout="", stderr="") + monkeypatch.setattr(subprocess, "run", run) + monkeypatch.setattr(process_ownership, "process_table", lambda: { + 4200: process_ownership.ProcessInfo(4200, 4100, "/bin/bash --noprofile --norc"), + 4201: process_ownership.ProcessInfo(4201, 4200, "sleep 60"), + }) + monkeypatch.setattr(process_ownership, "start_token", lambda pid: f"token:{pid}") + monkeypatch.setattr(process_ownership, "verify", lambda pid, token: process_ownership.OWNED) + monkeypatch.setattr(containment, "_pgid_of", lambda pid: pid) + def release(grant, **kwargs): + state["released"].append((grant.pid, kwargs)) + return containment.ReleaseOutcome(dead=True, escalated=False) + monkeypatch.setattr(containment, "release", release) + return state + + +def test_legacy_cleanup_checks_home_and_identity_and_kills_children_first(legacy): + report = process_reaper.reap_legacy_agent_tmux() + assert report["torn_down"] == 1 + assert [pid for pid, _ in legacy["released"]] == [4201, 4200] + assert all(options["require_identity"] for _, options in legacy["released"]) + assert legacy["calls"][-2][1:] == ["kill-session", "-t", "$8"] + + +def test_a_name_prefix_alone_never_authorizes_cleanup(legacy): + legacy["rows"] = "$8\tody-agent-chat\t1234\t4200\t%9\t4100\t/bin/bash\n" + report = process_reaper.reap_legacy_agent_tmux() + assert report["unverifiable"] == 1 + assert legacy["released"] == [] + assert all(call[1] != "kill-session" for call in legacy["calls"]) + + +def test_a_recycled_pane_pid_is_never_signalled(legacy, monkeypatch): + monkeypatch.setattr(process_ownership, "verify", lambda pid, token: process_ownership.FOREIGN if pid == 4200 else process_ownership.OWNED) + report = process_reaper.reap_legacy_agent_tmux() + assert report["unverifiable"] == 1 + assert legacy["released"] == [] + + +def test_a_stale_pane_pid_pointing_at_another_parent_is_not_signalled(legacy, monkeypatch): + monkeypatch.setattr(process_ownership, "process_table", lambda: { + 4200: process_ownership.ProcessInfo(4200, 9999, "/bin/bash --noprofile --norc"), + }) + assert process_reaper.reap_legacy_agent_tmux()["unverifiable"] == 1 + assert legacy["released"] == [] + + +def test_a_session_changed_during_discovery_is_never_killed(legacy, monkeypatch): + original = process_ownership.process_table + def table(): + legacy["rows"] = legacy["rows"].replace("1234", "5678") + return original() + monkeypatch.setattr(process_ownership, "process_table", table) + report = process_reaper.reap_legacy_agent_tmux() + assert report["unverifiable"] == 1 + assert legacy["released"] == [] + + +def test_startup_reaper_does_not_kill_a_current_runtime_grant(tmp_path, monkeypatch): + monkeypatch.setattr(containment, "_store_path", lambda: tmp_path / "grants.json") + monkeypatch.setattr(containment, "CONTAINMENT_MODE", containment.MODE_REPORT_ONLY) + grant = containment.acquire(containment.agent_spec(str(tmp_path), {}, 1), owner="active-chat") + monkeypatch.setattr(containment, "reap_record", lambda record: pytest.fail("startup killed current execution")) + assert process_reaper.reap_containment_grants()["manager_kept"] == 1 + containment.release(grant) + + +def test_legacy_cleanup_against_a_private_real_tmux_server(tmp_path, monkeypatch): + import os + import shlex + import shutil + import subprocess + real_tmux = shutil.which("tmux") + if os.name == "nt" or not real_tmux: + pytest.skip("requires POSIX tmux") + socket = str(tmp_path / "tmux.sock") + wrapper = tmp_path / "tmux" + wrapper.write_text(f"#!/bin/sh\nexec {shlex.quote(real_tmux)} -S {shlex.quote(socket)} \"$@\"\n") + wrapper.chmod(0o700) + subprocess.run([real_tmux, "-S", socket, "-f", "/dev/null", "new-session", "-d", "-s", "ody-agent-real", + "env", f"HOME={DATA_DIR}", "TERM=xterm-256color", "/bin/bash", "--noprofile", "--norc"], check=True) + original = shutil.which + monkeypatch.setattr(shutil, "which", lambda name: str(wrapper) if name == "tmux" else original(name)) + monkeypatch.setattr(containment, "_store_path", lambda: tmp_path / "grants.json") + try: + report = process_reaper.reap_legacy_agent_tmux() + assert report["torn_down"] == 1, report + assert subprocess.run([real_tmux, "-S", socket, "has-session", "-t", "ody-agent-real"], capture_output=True).returncode != 0 + assert containment.active_grants() == [] + finally: + subprocess.run([real_tmux, "-S", socket, "kill-server"], capture_output=True) diff --git a/tests/test_orphan_reaping.py b/tests/test_orphan_reaping.py index 38eca2e51..456acf1f9 100644 --- a/tests/test_orphan_reaping.py +++ b/tests/test_orphan_reaping.py @@ -426,6 +426,7 @@ def test_reap_orphans_reports_both_stores_and_the_mechanism( seed_grant() seed_job() verdicts(monkeypatch, {4242: process_ownership.GONE}) + monkeypatch.setattr(process_reaper, "reap_legacy_agent_tmux", lambda: {"seen": 0}) monkeypatch.setattr(containment, "_group_present", lambda _pgid: False) report = process_reaper.reap_orphans() diff --git a/website/configuration-reference.md b/website/configuration-reference.md index dc38e2abd..efa45dd73 100644 --- a/website/configuration-reference.md +++ b/website/configuration-reference.md @@ -75,7 +75,7 @@ The source tree reads **108** `ODYSSEUS_*` variables: 78 an operator may want to | `ODYSSEUS_MAX_VISUAL_EVIDENCE_FRAMES` | `'3'` | `src/agent_loop.py:15361` | How many video frames one tool result may contribute. Clamped to 1-8. | | `ODYSSEUS_MAX_VISUAL_EVIDENCE_IMAGES` | `'1'` | `src/agent_loop.py:15329` | How many images one tool result may contribute to the model turn. Clamped to 1-8. | | `ODYSSEUS_MCP_ALLOWED_COMMANDS` | `''` | `src/agent_tools/admin_tools.py:140` | Security-relevant. Comma-separated allowlist of MCP launcher basenames the agent may start. Empty by default, and the deny list still wins. | -| `ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES` | `''` | `src/agent_tools/subprocess_tools.py:1181` (+1 more) | Security-relevant. Absolute package roots, separated by the platform path separator, exposed to the sandboxed Python tool. Empty exposes none. | +| `ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES` | `''` | `src/agent_tools/subprocess_tools.py:1145` (+1 more) | Security-relevant. Absolute package roots, separated by the platform path separator, exposed to the sandboxed Python tool. Empty exposes none. | | `ODYSSEUS_SCRIPT_HOST` | `'localhost'` | `src/builtin_actions.py:919` | Default host for the run-script action. `localhost`, `127.0.0.1`, `local` and empty run locally; any other value runs over SSH. | | `ODYSSEUS_TOOL_APPROVAL_GATE` | `'0'` | `src/tool_capabilities.py:645` | Security-relevant. Truthy makes tool calls pass through the approval gate. Off by default. |