diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index c575f28b..845f98d4 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -518,6 +518,8 @@ compares a truncated run against a full one): The `sandbox` block is optional. When omitted, it defaults to `driver: "tempdir"` with standard Python environment. +The sandbox venv is created with **system site packages**, so `run_command` criteria (and `pre_run`/`post_run`) can import packages the task image installed globally while anything `env_packages` adds still lands in the venv. An isolated venv would shadow the image's interpreter without providing a replacement. + ```yaml sandbox: driver: "tempdir" # Sandbox type ("tempdir" or "docker"); default: "tempdir" diff --git a/src/coder_eval/models/sandbox.py b/src/coder_eval/models/sandbox.py index 2e7f2dbc..b3f3d569 100644 --- a/src/coder_eval/models/sandbox.py +++ b/src/coder_eval/models/sandbox.py @@ -597,7 +597,12 @@ class SandboxConfig(BaseModel): ) python: PythonEnvConfig | None = Field( default_factory=PythonEnvConfig, - description="Python environment config; set to null in YAML (or None in Python) to skip venv creation", + description=( + "Python environment config. A venv is created whenever this block is present, with access to " + "system site packages so criteria can import what the task image installed globally (an isolated " + "venv shadows the interpreter while providing nothing). Set to null in YAML (or None in Python) " + "to skip venv creation, and to opt out of adopting a venv the agent created itself." + ), ) node: NodeEnvConfig | None = Field( default=None, diff --git a/src/coder_eval/sandbox.py b/src/coder_eval/sandbox.py index 8de18944..e9527713 100644 --- a/src/coder_eval/sandbox.py +++ b/src/coder_eval/sandbox.py @@ -406,7 +406,9 @@ def _setup_tempdir(self, target_dir: Path | None = None) -> Path: # Mark mock binaries executable so the agent's PATH can shadow real CLIs self._prepare_mock_path_dirs() - # Set up Python virtual environment (only if python config is provided) + # Set up Python virtual environment (only if python config is provided). + # The venv is created with system site packages -- see _setup_virtualenv + # for why an isolated one was actively harmful. if self.config.python: self._setup_virtualenv() @@ -835,7 +837,29 @@ def _matches_template_include_pattern(self, rel_path: Path, include_patterns: li return False def _setup_virtualenv(self) -> None: - """Create a Python virtual environment in the sandbox.""" + """Create a Python virtual environment in the sandbox, with system site packages. + + ``--system-site-packages`` is load-bearing, not a convenience. An ISOLATED + venv here shadows the interpreter while providing nothing: the sandbox venv + goes on the criterion PATH (``_build_run_command_env``, which governs every + ``run_command`` criterion plus ``pre_run``/``post_run``), so inside a task + image that provisions packages globally, ``python`` resolved to the empty + venv and could not import them while ``pip`` -- which ``uv venv`` does not + place in the venv at all -- fell through to the image's global pip and + reported them present. Measured in a task image: ``import langchain`` raised + ``ModuleNotFoundError`` while ``pip list`` showed ``langchain 1.3.14``. An + agent that tried to verify its own work chased that contradiction for ten + turns and ran out of budget before finishing. + + Note the venv is NOT on the agent's own PATH -- the orchestrator prepends + only ``resolved_mock_path_dirs`` there -- so the contradiction above is a + property of criterion and pre/post-run subprocesses. + + System site packages fixes it in the direction that keeps both halves: the + image's globals stay importable, ``python`` and ``pip`` agree, and installs + still land in the venv (``sys.prefix`` remains the sandbox), so a task's + ``env_packages`` cannot leak into the image. + """ if not self.sandbox_dir: raise RuntimeError("Sandbox directory not initialized") @@ -846,13 +870,16 @@ def _setup_virtualenv(self) -> None: # Check if uv is available subprocess.run(["uv", "--version"], check=True, capture_output=True, timeout=5) # Use uv to create venv - cmd = ["uv", "venv", str(self.venv_dir)] + cmd = ["uv", "venv", "--system-site-packages", str(self.venv_dir)] subprocess.run(cmd, check=True, capture_output=True, text=True, encoding="utf-8", timeout=60) except (subprocess.CalledProcessError, FileNotFoundError): - # Fallback to standard venv if uv is not available + # Fallback to standard venv if uv is not available. The two paths do not + # produce the same artifact -- this one seeds pip, `uv venv` does not -- + # so say which shape this host got rather than leaving it to be inferred. import venv - venv.create(self.venv_dir, with_pip=True) + logger.warning("uv unavailable; created %s with stdlib venv (pip seeded)", self.venv_dir) + venv.create(self.venv_dir, with_pip=True, system_site_packages=True) def _install_packages(self) -> None: """Install required Python packages in the virtual environment.""" diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py index 1ae4e451..f1f2175f 100644 --- a/tests/test_sandbox.py +++ b/tests/test_sandbox.py @@ -486,7 +486,12 @@ def test_sandbox_with_packages(): sandbox = Sandbox(config, task_id="test_packages") try: - sandbox.setup() + sandbox_dir = sandbox.setup() + + # Asking for packages is what earns a venv (see + # test_default_python_config_creates_no_venv_when_nothing_to_install). + assert (sandbox_dir / ".venv").exists() + assert sandbox.venv_dir == sandbox_dir / ".venv" # Test that requests is installed exit_code, stdout, stderr = sandbox.run_command('python -c "import requests; print(requests.__version__)"') @@ -1310,3 +1315,53 @@ def test_absent_when_the_task_declares_no_reference(self, tmp_path): assert "REFERENCE_DIR" not in sb._build_run_command_env() finally: sb.cleanup(preserve=False) + + +def test_default_venv_can_import_system_site_packages(): + """The sandbox venv must not shadow the interpreter it is layered over. + + `SandboxConfig.python` defaults to a `PythonEnvConfig()` instance, so every + task gets a venv. Built ISOLATED, that venv split the toolchain inside a task + image that provisions packages globally: `python` resolved to the venv and + could not import them, while `pip` -- which `uv venv` never places in the venv + -- fell through to the image's global pip and reported them present. Measured + in a task image: `import langchain` raised ModuleNotFoundError while + `pip list` showed `langchain 1.3.14`. + + Asserted through `pyvenv.cfg` rather than a live import so the test is + hermetic: it holds on a host whose base interpreter has nothing installed. + """ + config = SandboxConfig(driver="tempdir") + assert config.python is not None, "default is an instance, not None -- the case this guards" + assert config.python.env_packages == [] + + sandbox = Sandbox(config, task_id="test_system_site_packages") + try: + sandbox_dir = sandbox.setup() + cfg = (sandbox_dir / ".venv" / "pyvenv.cfg").read_text(encoding="utf-8") + assert "include-system-site-packages = true" in cfg.lower() + finally: + sandbox.cleanup() + + +def test_venv_reaches_the_criterion_environment(): + """The venv is only worth creating because criteria run under it. + + `_build_run_command_env` is the single surface that carries it (its sole + caller is `Sandbox.run_command`, i.e. every `run_command` criterion plus + `pre_run`/`post_run`). The agent's own PATH never carries the venv -- the + orchestrator prepends only `resolved_mock_path_dirs` there -- so asserting + the artifact exists says nothing about the effect this change exists for. + """ + sandbox = Sandbox(SandboxConfig(driver="tempdir"), task_id="test_criterion_env") + try: + sandbox_dir = sandbox.setup() + venv_dir = sandbox_dir / ".venv" + assert sandbox.venv_dir == venv_dir + + env = sandbox._build_run_command_env() + assert env["VIRTUAL_ENV"] == str(venv_dir) + scripts_dir = "Scripts" if os.name == "nt" else "bin" + assert env["PATH"].startswith(f"{venv_dir / scripts_dir}{os.pathsep}") + finally: + sandbox.cleanup() diff --git a/tests/test_sandbox_adopt.py b/tests/test_sandbox_adopt.py index 39fafd9b..923aafb2 100644 --- a/tests/test_sandbox_adopt.py +++ b/tests/test_sandbox_adopt.py @@ -88,6 +88,28 @@ def test_adopt_leaves_venv_unset_when_there_is_none(tmp_path: Path) -> None: assert sandbox.venv_dir is None +def test_adopt_ignores_a_venv_when_python_is_null(tmp_path: Path) -> None: + """`python: null` opts out of BOTH halves: setup creates no venv, and adopt + declines to pick up one the agent wrote itself. + + Without this, the false arm of adopt's gate was unexercised across the whole + suite while the field description documents it as user-facing behavior. The + assertion is on the criterion environment, not just `venv_dir`, because that + is the surface the opt-out exists to control. + """ + ws = _workspace(tmp_path) + (ws / ".venv" / "bin").mkdir(parents=True) + sandbox = _sandbox(python=None) + sandbox.adopt(ws) + assert sandbox.venv_dir is None + # `_build_run_command_env` starts from `os.environ.copy()`, so an ambient + # VIRTUAL_ENV from the grader's own shell can be present; what must not + # happen is the harness pointing either variable at the WORKSPACE venv. + env = sandbox._build_run_command_env() + assert env.get("VIRTUAL_ENV") != str(ws.resolve() / ".venv") + assert str(ws.resolve() / ".venv") not in env["PATH"] + + def test_adopt_rejects_the_docker_driver(tmp_path: Path) -> None: """A container workspace is not reachable from the host, so adopting one would silently grade whatever happens to sit at that host path.""" diff --git a/tests/test_sandbox_templates.py b/tests/test_sandbox_templates.py index 12179300..6c737292 100644 --- a/tests/test_sandbox_templates.py +++ b/tests/test_sandbox_templates.py @@ -164,6 +164,7 @@ def test_template_ignores_venv(self, tmp_path): (template_dir / ".venv").mkdir() (template_dir / ".venv" / "bin").mkdir() (template_dir / ".venv" / "bin" / "python").write_text("fake python") + (template_dir / ".venv" / "from_template.marker").write_text("copied") config = SandboxConfig( driver="tempdir", @@ -179,12 +180,16 @@ def test_template_ignores_venv(self, tmp_path): # Verify main.py copied assert (sandbox_path / "main.py").exists() - # Verify .venv from template was NOT copied - # (sandbox creates its own .venv) - venv_bin = sandbox_path / ".venv" / "bin" - if venv_bin.exists(): - # If .venv exists, it should be the sandbox's venv, not the template's - assert not (venv_bin / "python").exists() or (venv_bin / "python").is_symlink() + # The .venv present here is the sandbox's own (setup creates one); + # assert the TEMPLATE's copy did not land, via a marker file the real + # venv can never contain. Unconditional on purpose: guarding this + # behind `if venv_bin.exists()` made the whole check vacuous the + # moment provisioning changed, which is exactly how it went + # unnoticed. The marker is also layout-independent -- asserting on + # `.venv/bin/python` would only work on POSIX, since a real venv puts + # its interpreter in `Scripts/python.exe` on Windows. + assert (sandbox_path / ".venv").exists() + assert not (sandbox_path / ".venv" / "from_template.marker").exists() finally: sandbox.cleanup(preserve=False) diff --git a/tests/test_sandbox_venv_live.py b/tests/test_sandbox_venv_live.py new file mode 100644 index 00000000..fd80ca8e --- /dev/null +++ b/tests/test_sandbox_venv_live.py @@ -0,0 +1,110 @@ +"""Live docker check that the sandbox venv does not shadow a task image's packages. + +Gated: needs a real docker daemon and the `coder-eval-agent` base image. + +This is the one thing the unit tests cannot prove. `tests/test_sandbox.py` asserts +the venv is created with system site packages by reading `pyvenv.cfg`, which is a +property of the flag, not of the outcome. The outcome only exists inside an image +that provisions packages GLOBALLY — the shape every task image has (the framework +image installs with `uv pip install --system`; skillsbench task images do +`RUN pip install ...`). There, an isolated sandbox venv split the toolchain: +`python` resolved to the venv and could not import the image's packages, while +`pip` fell through to the image's global pip and reported them present. + +Measured against this test's own scenario: + + main (isolated venv) python -c "import pydantic" -> exit 1 + with --system-site-packages python -c "import pydantic" -> exit 0 + +`pydantic` is a coder_eval runtime dependency, so the base image already has it +installed globally — no build and no network are needed to reproduce the shape. +""" + +from __future__ import annotations + +import shutil +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + + +BASE_IMAGE = "coder-eval-agent:latest" +REPO_SRC = Path(__file__).resolve().parent.parent / "src" + +pytestmark = [ + pytest.mark.live, + pytest.mark.skipif(sys.platform == "win32", reason="docker driver is POSIX-only"), + pytest.mark.skipif(shutil.which("docker") is None, reason="docker CLI not available"), +] + + +def _docker_daemon_up() -> bool: + try: + return subprocess.run(["docker", "info"], capture_output=True, timeout=15).returncode == 0 + except (subprocess.TimeoutExpired, FileNotFoundError): + return False + + +def _image_present(image: str) -> bool: + return subprocess.run(["docker", "image", "inspect", image], capture_output=True, timeout=30).returncode == 0 + + +PROBE = textwrap.dedent( + """ + from coder_eval.models import SandboxConfig + from coder_eval.sandbox import Sandbox + + sandbox = Sandbox(SandboxConfig(driver="tempdir"), task_id="venv_probe") + try: + sandbox_dir = sandbox.setup() + import_rc, _, _ = sandbox.run_command('python -c "import pydantic"') + _, prefix, _ = sandbox.run_command('python -c "import sys; print(sys.prefix)"') + print(f"IMPORT_RC={import_rc}") + print(f"PREFIX={prefix.strip()}") + print(f"VENV={sandbox.venv_dir}") + finally: + sandbox.cleanup() + """ +) + + +def test_criteria_can_import_the_images_global_packages() -> None: + """A `run_command` criterion must see what the task image installed globally. + + Mounts this checkout's `src/` over the image's copy so the assertion is about + the code under test, not whatever coder_eval version the image was built with. + """ + if not _docker_daemon_up(): + pytest.skip("docker daemon not running") + if not _image_present(BASE_IMAGE): + pytest.skip(f"{BASE_IMAGE} not built locally") + + proc = subprocess.run( + [ + "docker", + "run", + "--rm", + "-v", + f"{REPO_SRC}:/opt/coder_eval/src:ro", + "--entrypoint", + "python3", + BASE_IMAGE, + "-c", + PROBE, + ], + capture_output=True, + text=True, + timeout=300, + ) + assert proc.returncode == 0, f"probe failed:\n{proc.stdout}\n{proc.stderr}" + out = dict(line.split("=", 1) for line in proc.stdout.splitlines() if "=" in line) + + assert out["IMPORT_RC"] == "0", ( + "a criterion could not import a package the image installed globally -- " + f"the sandbox venv is shadowing the image interpreter again:\n{proc.stdout}" + ) + # Isolation still holds: installs land in the sandbox, not the image. + assert out["PREFIX"] == out["VENV"], f"criterion did not run under the sandbox venv:\n{proc.stdout}"