From bf0623a77b99b50d6ea659671c30612fb17c7ae3 Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:04:12 +0100 Subject: [PATCH] fix(runtime): contain every native Python execution (ODY-143) --- src/agent_tools/subprocess_tools.py | 126 ++------------------ tests/test_native_execution_containment.py | 36 ++++++ tests/test_workspace_artifact_tool_floor.py | 1 + tests/test_workspace_confine.py | 3 +- website/configuration-reference.md | 2 +- 5 files changed, 53 insertions(+), 115 deletions(-) diff --git a/src/agent_tools/subprocess_tools.py b/src/agent_tools/subprocess_tools.py index 1b7178d13..2daf4e2fd 100644 --- a/src/agent_tools/subprocess_tools.py +++ b/src/agent_tools/subprocess_tools.py @@ -811,7 +811,7 @@ async def _run_subprocess_streaming( timed_out, ) -def _owned_spec(cwd: str, env: Optional[dict], timeout: int) -> containment.ContainmentSpec: +def _owned_spec(cwd: str, env: Optional[dict], timeout: int, readonly_extra: tuple = ()) -> containment.ContainmentSpec: """Server-defined boundary shared by the native execution tools.""" readonly = [] for prefix in (sys.prefix, sys.base_prefix): @@ -820,17 +820,18 @@ def _owned_spec(cwd: str, env: Optional[dict], timeout: int) -> containment.Cont readonly.append(prefix) return containment.agent_spec( cwd, dict(os.environ if env is None else env), timeout, - readonly_extra=tuple(dict.fromkeys(readonly)), + readonly_extra=tuple(dict.fromkeys([*readonly, *readonly_extra])), ) -async def _run_owned_command(command, ctx: dict, *, tool: str, timeout: int, argv: bool = False) -> dict: +async def _run_owned_command(command, ctx: dict, *, tool: str, timeout: int, argv: bool = False, + readonly_extra: tuple = ()) -> dict: from src.tool_execution import agent_cwd, _truncate grant = None try: grant = containment.acquire( - _owned_spec(agent_cwd(), ctx.get("subproc_env"), timeout), + _owned_spec(agent_cwd(), ctx.get("subproc_env"), timeout, readonly_extra), owner=str(ctx.get("session_id") or ctx.get("owner") or tool), ) if containment.FILESYSTEM not in grant.enforced: @@ -1206,119 +1207,18 @@ class PythonTool: ), "exit_code": 1, } - # Only create a mount namespace when the submitted code actually - # relies on the public virtual path. Ordinary Python probes and - # scripts should retain the real workspace as os.getcwd(); wrapping - # every invocation would make that stable contract appear as - # ``/workspace`` instead. - needs_virtual_namespace = bool( - WORKSPACE_MOUNT in content - or re.search(r"\b(?:runpy\.run_path|exec\s*\(|importlib\.)", content) - ) 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") - # Generated scripts commonly contain the public `/workspace/...` - # paths shown in the tool contract. Rewriting the inline `-c` body - # cannot repair paths embedded in a script loaded via `runpy`, and a - # process-global `/workspace` symlink would break concurrent tasks. - # Give Python the same per-task namespace Bash receives so both inline - # code and loaded scripts see the stable virtual workspace root. - namespaced_content = _python_with_configured_import_paths( + content = _python_with_configured_import_paths( _python_with_visible_final_expression(content), _subproc_env ) - python_command = shlex.join((sys.executable or "python", "-I", "-c", namespaced_content)) - # Code that explicitly uses the public /workspace path runs inside a - # namespace whose stable cwd is that same bind. Host workspaces under - # /tmp or another unbound parent are intentionally invisible by their - # real path inside the namespace; trying to chdir there makes otherwise - # valid native Python fail before execution. - namespaced = ( - _wrap_workspace_namespace( - python_command, - agent_cwd(), - chdir=WORKSPACE_MOUNT, - interpreter_prefix=sys.prefix, - ) - if needs_virtual_namespace - else None + # All Python code acquires the same server-defined boundary, including + # arithmetic and ordinary imports. Source text never selects a scope. + raw_paths = str((_subproc_env or {}).get("ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES", "")) + roots = tuple(path for path in raw_paths.split(os.pathsep) if path and os.path.isabs(path)) + return await _run_owned_command( + [sys.executable or "python", "-I", "-c", content], ctx, + tool="python", timeout=DEFAULT_PYTHON_TIMEOUT, argv=True, readonly_extra=roots, ) - # The boundary this call actually got, reported either way. Note what - # the gate above means: code that does not mention /workspace and is - # not dynamic gets NO namespace, on every platform including a Linux - # host with working bubblewrap. That is deliberate -- it keeps - # os.getcwd() the real workspace -- but it is also a filesystem - # containment gap wider than the macOS one, and until now nothing said - # so. Reporting it is in scope here; closing it is not: it changes the - # Linux Python path for every call and cannot be verified on a host - # without bwrap. It is the reason enforcing mode cannot be switched on - # yet, because enforcing it as written would refuse ordinary Python on - # a correctly configured host. - boundary_probe = _execution_boundary( - agent_cwd(), wall_clock_s=DEFAULT_PYTHON_TIMEOUT, - ) - confined = namespaced is not None - if not confined and boundary_probe.mode == containment.MODE_ENFORCING: - return containment.unavailable_tool_result( - containment.ContainmentUnavailable( - frozenset({containment.FILESYSTEM}), ALIAS_REWRITE_MECHANISM, - ), - tool="python", - ) - boundary = _filesystem_boundary_block( - boundary_probe.mechanism if confined else ALIAS_REWRITE_MECHANISM, - boundary_probe.mode, - confined=confined, - ) - if namespaced: - proc = await asyncio.create_subprocess_exec( - "/bin/bash", "-lc", namespaced, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - env=_subproc_env, - cwd=agent_cwd(), - ) - else: - # Platforms without a usable namespace still receive the same - # alias contract through a conservative source rewrite. - content = _python_with_configured_import_paths( - _python_with_visible_final_expression( - _replace_workspace_alias(content, agent_cwd()) - ), - _subproc_env, - ) - proc = await asyncio.create_subprocess_exec( - (sys.executable or "python"), "-I", "-c", content, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - env=_subproc_env, - cwd=agent_cwd(), - ) - mark_operation_started('subprocess', pid=proc.pid) - stdout, stderr, rc, timed_out = await _run_subprocess_streaming( - proc, - timeout=DEFAULT_PYTHON_TIMEOUT, - progress_cb=progress_cb, - ) - if timed_out: - return {"error": f"python: timed out after {DEFAULT_PYTHON_TIMEOUT}s — process killed", "exit_code": 124, "stdout": _truncate(stdout, MAX_OUTPUT_CHARS), "stderr": _truncate(stderr, MAX_OUTPUT_CHARS), "containment": boundary} - child_failure = _python_child_runtime_failure(stdout, stderr, rc) - if child_failure: - return { - "error": _truncate( - "python: a child operation failed despite a zero Python exit " - "status:\n" + child_failure, - MAX_OUTPUT_CHARS, - ), - "exit_code": 1, - "stderr": _truncate(stderr, MAX_OUTPUT_CHARS), - "containment": boundary, - } - output = stdout.rstrip() - err = stderr.rstrip() - if err: - output = (output + "\nSTDERR: " + err).strip() if output else "STDERR: " + err - output = _truncate(output, MAX_OUTPUT_CHARS) - return {"output": output or "(no output)", "exit_code": rc or 0, "containment": boundary} diff --git a/tests/test_native_execution_containment.py b/tests/test_native_execution_containment.py index 095addeb1..9c86401f0 100644 --- a/tests/test_native_execution_containment.py +++ b/tests/test_native_execution_containment.py @@ -104,3 +104,39 @@ async def test_blocked_stdin_is_inside_wall_clock(native_boundary): ), timeout=8) assert result.timed_out is True assert result.release.dead is True + + +@pytest.mark.parametrize("source", ["print(1 + 1)", "import os; print(os.getcwd())", + "exec('print(2)')", "print('/workspace')"]) +async def test_python_namespace_is_independent_of_content(source, native_boundary, monkeypatch): + from tests.containment_helpers import capture_owned_spawn + captured = capture_owned_spawn(monkeypatch, native_boundary) + monkeypatch.setattr(containment, "MECHANISMS", (containment.Mechanism( + "bubblewrap", 30, lambda: True, lambda spec: containment.DEFAULT_REQUIRED, + ),)) + monkeypatch.setattr(containment, "CONTAINMENT_MODE", containment.MODE_ENFORCING) + result = await subprocess_tools.PythonTool().execute(source, {}) + assert captured["argv"][0] == "bwrap" + assert "--bind" in captured["argv"] + assert result["containment"]["enforced"] == sorted(containment.DEFAULT_REQUIRED) + assert "-I" in captured["argv"] + + +async def test_ordinary_python_cannot_bypass_unavailable_containment(monkeypatch): + monkeypatch.setattr(containment, "CONTAINMENT_MODE", containment.MODE_ENFORCING) + async def forbidden(*args, **kwargs): + pytest.fail("ordinary Python bypassed required containment") + monkeypatch.setattr(asyncio, "create_subprocess_exec", forbidden) + result = await subprocess_tools.PythonTool().execute("print(1 + 1)", {}) + assert result["containment"]["executed"] is False + + +async def test_python_final_expression_and_opt_in_imports(native_boundary): + package = native_boundary / "packages" + package.mkdir() + (package / "demo.py").write_text("value = 42\n") + result = await subprocess_tools.PythonTool().execute("import demo; demo.value", { + "subproc_env": {**os.environ, "ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES": str(package)}, + }) + assert result["output"] == "42" + assert result["teardown"]["dead"] is True diff --git a/tests/test_workspace_artifact_tool_floor.py b/tests/test_workspace_artifact_tool_floor.py index 897ce541e..447c7b46a 100644 --- a/tests/test_workspace_artifact_tool_floor.py +++ b/tests/test_workspace_artifact_tool_floor.py @@ -2210,6 +2210,7 @@ def test_python_loaded_code_sees_virtual_workspace_alias(monkeypatch, tmp_path): venv.EnvBuilder(with_pip=False).create(environment) monkeypatch.setattr(subprocess_tools, "sys", SimpleNamespace( prefix=str(environment), + base_prefix=sys.base_prefix, executable=str(environment / "bin" / "python"), version_info=sys.version_info, )) diff --git a/tests/test_workspace_confine.py b/tests/test_workspace_confine.py index d033b6481..059366520 100644 --- a/tests/test_workspace_confine.py +++ b/tests/test_workspace_confine.py @@ -287,7 +287,8 @@ async def test_subprocess_cwd_is_workspace_e2e(ws, admin): """python tool runs with cwd = workspace (OS-agnostic probe).""" _, r = await execute_tool_block(_block("python", "import os; print(os.getcwd())"), owner="a", workspace=ws) assert r["exit_code"] == 0 - assert os.path.realpath(r["output"].strip()) == os.path.realpath(ws) + expected_cwd = "/workspace" if "filesystem" in r["containment"]["enforced"] else ws + assert os.path.realpath(r["output"].strip()) == os.path.realpath(expected_cwd) @pytest.mark.asyncio diff --git a/website/configuration-reference.md b/website/configuration-reference.md index bb0474604..dc38e2abd 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:1180` | 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:1181` (+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. |