From 7a2c59a7333071b4b5e25ed5faf18e2ae6372cf0 Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Mon, 10 Aug 2026 17:15:52 +0300 Subject: [PATCH 1/7] feat(isolation): isolate evaluated agents by UID and GID --- docker/Dockerfile | 33 ++- docker/coder_eval_claude_agent.sh | 4 + docker/coder_eval_drop_privilege.sh | 20 ++ docs/DOCKER_ISOLATION.md | 53 ++-- src/coder_eval/agents/antigravity_agent.py | 68 +++++- src/coder_eval/agents/claude_code_agent.py | 13 +- src/coder_eval/agents/codex_agent.py | 50 +++- .../cli/run_task_internal_command.py | 30 ++- src/coder_eval/isolation/agent_identity.py | 113 +++++++++ src/coder_eval/isolation/docker_runner.py | 227 +++++++++++++++++- src/coder_eval/models/__init__.py | 16 ++ src/coder_eval/models/container_paths.py | 35 ++- src/coder_eval/models/sandbox.py | 8 + src/coder_eval/orchestrator.py | 34 +++ src/coder_eval/utils.py | 30 +++ tests/test_codex_agent.py | 16 ++ tests/test_docker_build_failure.py | 5 + tests/test_docker_identity_isolation.py | 161 +++++++++++++ tests/test_docker_runner_mounts.py | 25 +- 19 files changed, 869 insertions(+), 72 deletions(-) create mode 100644 docker/coder_eval_claude_agent.sh create mode 100644 docker/coder_eval_drop_privilege.sh create mode 100644 src/coder_eval/isolation/agent_identity.py create mode 100644 tests/test_docker_identity_isolation.py diff --git a/docker/Dockerfile b/docker/Dockerfile index 89717613..a773e521 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -17,14 +17,32 @@ ENV DEBIAN_FRONTEND=noninteractive \ PIP_NO_CACHE_DIR=1 # System deps: git for repo-source templates; curl/ca-certs for HTTPS; -# build-essential because some Python deps (pylint plugins) compile. +# build-essential because some Python deps (pylint plugins) compile; util-linux +# provides setpriv for the agent UID/GID boundary. RUN apt-get update && apt-get install -y --no-install-recommends \ ca-certificates \ curl \ git \ build-essential \ + util-linux \ && rm -rf /var/lib/apt/lists/* +# The harness remains root. Only the evaluated agent is dropped to this +# identity, and it receives no group access to the harness's files. +ARG AGENT_UID=2000 +ARG AGENT_GID=2000 +RUN groupadd --gid ${AGENT_GID} agent \ + && useradd --uid ${AGENT_UID} --gid ${AGENT_GID} \ + --create-home --home-dir /home/agent --shell /usr/sbin/nologin agent \ + && install -d -o agent -g agent -m 0700 /work/agent /home/agent \ + && install -d -o root -g root -m 0700 \ + /opt/coder-eval/grader \ + /opt/coder-eval/grader/input \ + /opt/coder-eval/grader/output \ + /opt/coder-eval/grader/task_dir \ + /opt/coder-eval/grader/references \ + /opt/coder-eval/grader/templates + # Node LTS + the Claude Code CLI, pinned. The agent binary is a dominant # non-model driver of eval results, so it travels with the coder_eval release # tag and is bumped deliberately -- mirrors the codex CLI pin @@ -34,8 +52,20 @@ ARG CLAUDE_CODE_VERSION=2.1.177 RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \ && apt-get install -y --no-install-recommends nodejs \ && rm -rf /var/lib/apt/lists/* \ + && npm config set prefix /usr/local \ && npm install -g @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} +# Every built-in agent backend routes its SDK-owned subprocess through the +# same setpriv policy. Claude's SDK accepts only one executable path, so it +# uses the small backend-specific wrapper below; Codex and Antigravity invoke +# the generic launcher directly. +COPY docker/coder_eval_drop_privilege.sh /usr/local/bin/coder_eval_drop_privilege.sh +COPY docker/coder_eval_claude_agent.sh /usr/local/bin/coder_eval_claude_agent.sh +RUN chmod 0555 \ + /usr/local/bin/coder_eval_drop_privilege.sh \ + /usr/local/bin/coder_eval_claude_agent.sh \ + && test "$(command -v claude)" = "/usr/local/bin/claude" + # uv: matches host sandbox.py's `uv venv` + `uv pip install` fast path RUN curl -LsSf https://astral.sh/uv/install.sh | env UV_INSTALL_DIR=/usr/local/bin sh @@ -81,6 +111,7 @@ RUN coder-eval _run-task-internal --help > /dev/null # AFTER the (billed) run completes via the version field in task.json. ARG CODER_EVAL_VERSION=unknown LABEL org.coder-eval.version="${CODER_EVAL_VERSION}" +LABEL org.coder-eval.agent-isolation="uid-gid-v1" # Stamp the pinned agent binary too, so the host can assert it via # `docker image inspect` before a (billed) run — same rationale as above. LABEL org.coder-eval.claude-code-version="${CLAUDE_CODE_VERSION}" diff --git a/docker/coder_eval_claude_agent.sh b/docker/coder_eval_claude_agent.sh new file mode 100644 index 00000000..1b6be19d --- /dev/null +++ b/docker/coder_eval_claude_agent.sh @@ -0,0 +1,4 @@ +#!/usr/bin/env bash +set -euo pipefail + +exec /usr/local/bin/coder_eval_drop_privilege.sh /usr/local/bin/claude "$@" diff --git a/docker/coder_eval_drop_privilege.sh b/docker/coder_eval_drop_privilege.sh new file mode 100644 index 00000000..9bcf7fd9 --- /dev/null +++ b/docker/coder_eval_drop_privilege.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Execute the evaluated agent under its dedicated identity. The harness invokes +# this as root; every descendant inherits the UID/GID, empty capability sets, +# and no-new-privileges bit. +set -euo pipefail + +if [[ $# -eq 0 ]]; then + echo "coder_eval_drop_privilege: missing command" >&2 + exit 64 +fi + +exec setpriv \ + --reuid=agent \ + --regid=agent \ + --clear-groups \ + --inh-caps=-all \ + --ambient-caps=-all \ + --bounding-set=-all \ + --no-new-privs \ + -- "$@" diff --git a/docs/DOCKER_ISOLATION.md b/docs/DOCKER_ISOLATION.md index d3076229..7f4c8064 100644 --- a/docs/DOCKER_ISOLATION.md +++ b/docs/DOCKER_ISOLATION.md @@ -29,10 +29,18 @@ make docker-image-full Both build `coder-eval-agent:` and tag it `:latest`. -- **`make docker-image`** installs the core package plus **both built-in agents** — claude-code (baked above) and Codex (`--extra codex`, public PyPI). It needs **no credentials** and covers the common case: claude-code or Codex tasks scored with `run_command` / `file_contains` (incl. converted skillsbench tasks). `llm_judge` / `agent_judge` work here too (they route through the run's Anthropic/Bedrock backend). +- **`make docker-image`** installs the core package plus the built-in agents. It needs **no credentials** and carries the `uid-gid-v1` isolation capability used by secure Docker runs. Static file/transcript criteria and `llm_judge` work in protected mode. Privileged dynamic criteria (`run_command`, `uipath_eval`, and `agent_judge`) currently fail closed; see [compatibility limits](#limitations). - **`make docker-image-full`** additionally installs the `uipath` extra. The `uipath` SDK resolves from **public PyPI** (per `uv.lock`), so the build needs **no credentials**. Use this only for tasks that shell out to the in-host `uipath` CLI. (Codex is already in the default image — no extra needed.) -> **Codex sandbox under Docker.** Codex's Landlock-backed `read-only` / `workspace-write` sandboxes can't initialize inside the eval container — their writes/execs fail silently and the agent produces no artifacts (a `score=0` FAILURE with no loud error). The docker runner sets `CODER_EVAL_IN_CONTAINER=1`, and the Codex agent honors it by falling back to `full-access`: the container itself is the trust boundary. Host runs (tempdir) are unaffected — Landlock works there and the marker is unset. So Codex tasks run under `--driver docker` with their natural `acceptEdits` permission mode; no need to set `bypassPermissions` by hand. +> **Codex sandbox under Docker.** Codex's Landlock-backed `read-only` / `workspace-write` sandboxes can't initialize inside the eval container. The runner therefore uses Codex `full-access` inside the agent's own security domain. The boundary is the dedicated Linux agent UID, cleared capabilities, `no_new_privs`, and the protected harness paths—not Landlock and not a root agent process. + +## Agent/grader identity boundary + +`sandbox.docker.agent_isolation` defaults to `true`. The container harness and grader remain root, while every evaluated Claude, Codex, or Antigravity subprocess runs as `agent:agent` (`2000:2000`). + +The agent launcher clears inheritable, ambient, and bounding capabilities and sets `no_new_privs`. Generated work is placed in `/work/agent`. Hidden task data, results, raw task/plugin/reference/template sources, and grader inputs live below root-only `/opt/coder-eval/grader`. Raw source bind mounts remain read-only and are never chmod/chowned; only disposable staging copies and the generated workspace are changed. + +Older/custom images must declare `org.coder-eval.agent-isolation=uid-gid-v1`. A protected run rejects an image without that label before making an LLM call. Images derived with `FROM coder-eval-agent:` inherit it. Runtime-kit injection into an unrelated base does not yet provide the required Linux users and `setpriv` launchers, so it is not compatible with protected mode. ## Running a task in Docker @@ -130,6 +138,14 @@ sandbox: ### Tasks that bring their own base image: the runtime kit (`coder-eval-runtime`) +> **Protected-mode compatibility:** the current runtime kit does not install the +> dedicated identities, `setpriv` launchers, protected directory layout, or the +> `org.coder-eval.agent-isolation=uid-gid-v1` capability label. Because +> `agent_isolation` defaults to `true`, an inject-mode image fails closed at +> preflight. For now, extend `coder-eval-agent:` for protected runs. +> Setting `agent_isolation: false` permits legacy runtime-kit migration but does +> not provide the boundary described on this page. + The `FROM coder-eval-agent` contract above means a task is **rebased** onto the Debian framework image. That breaks tasks whose Dockerfile was written for a different base image (e.g. a Fedora recipe using `dnf`, which doesn't exist on Debian). To keep the task's own base image and build successfully, coder-eval's runtime need to be copied into the task's image. Use `make coder-eval-runtime` first to make the runtime available for copying. @@ -163,7 +179,7 @@ individual targets when you only need one. FROM coder-eval-agent:latest # inherit runtime + entrypoint RUN apt-get update && apt-get install -y --no-install-recommends poppler-utils RUN pip install --no-cache-dir PyMuPDF==1.24.10 -COPY input/ /root/input/ +COPY input/ /work/agent/input/ ``` Behavior: @@ -263,20 +279,19 @@ sandbox: env_passthrough: ["MY_CUSTOM_TOKEN", "ANTHROPIC_API_KEY"] ``` -### `HOME` is forwarded by default +### Agent HOME and Claude state -The default `env_passthrough` includes `HOME` so the in-container `~/.claude` lookup resolves at the same path as on the host (the mount lands at `$HOME/.claude` symmetrically). Practical contract: +In protected mode, the host `HOME` value is not forwarded to the evaluated subprocess. The agent uses `/home/agent`: -- `Path.home()` inside the container returns the host's `HOME` value (e.g. `/Users/you` on macOS). The directory exists in the container because Docker auto-creates it as the mount parent for `~/.claude`. -- `~/.claude` is **not** the host's real dir — the runner makes a throwaway *lean copy* in a tmp dir per task and mounts that copy **read-write** at `$HOME/.claude`. The copy keeps the small set the container needs (auth via `.credentials.json`, `settings.json`, `plugins/`) and **drops heavy or transient per-session state** — `security/` (often hundreds of MB), `projects/`, `cache/`, `file-history/`, `backups/`, `downloads/`, `sessions/`, `telemetry/`, `shell-snapshots/`, `todos/`, `session-env/`, plus the volatile churn dirs the live CLI rewrites. The skip set is a denylist; the authoritative list is `CLAUDE_COPY_IGNORE` in `src/coder_eval/isolation/docker_runner.py` (a test asserts this doc and that constant agree, so the list never silently drifts). The container may write anywhere under `~/.claude`; those writes hit the copy and are discarded when the task ends — the host's real `~/.claude` is never modified. Note the copy includes the OAuth token (`.credentials.json`) and is mounted read-**write**, so the in-container agent can read and tamper with the token *copy* — contained, since the copy is discarded at task end and the host's real dir is untouched. Opt out entirely with `CODER_EVAL_NO_CLAUDE_MOUNT=1`. -- Writes under `$HOME` outside the `~/.claude` mount land in the container's ephemeral rootfs overlay. Don't expect them to persist or to be visible to the host. -- If a tool *detects platform* from `HOME` (e.g. "starts with `/Users/` → macOS"), it will draw the wrong conclusion. Vanishingly rare in practice. +- `~/.claude` is **not** the host's real directory. The runner makes a throwaway *lean copy* and mounts it read-write at `/home/agent/.claude`. The copy keeps the small authentication/settings/plugin set and **drops heavy or transient per-session state** — `security/`, `projects/`, `cache/`, `file-history/`, `backups/`, `downloads/`, `sessions/`, `telemetry/`, `shell-snapshots/`, `todos/`, `session-env/`, plus volatile CLI churn directories. The authoritative skip set is `CLAUDE_COPY_IGNORE` in `docker_runner.py`. Writes affect only the disposable copy. Opt out with `CODER_EVAL_NO_CLAUDE_MOUNT=1`. +- Other writes below `/home/agent` or `/work/agent` are ephemeral until the harness captures the workspace into the protected output mount. +- Harness-only variables such as `SKILLS_REPO_PATH`, `TASK_DIR`, and `CODER_EVAL_*` are removed from agent SDK environments. Required model API credentials remain available. -Remove `HOME` from `env_passthrough` if you don't want this behavior — the container's image-default `HOME=/root` will win, but then the host's OAuth dir is no longer reachable. +When `agent_isolation: false` is explicitly selected for migration, the legacy host-HOME behavior may still apply. That mode is not a security boundary. ## Run directory safety (`--run-dir`) -The host's run dir is bind-mounted **read-write** into the container at the same absolute path (so `task.json` and artifacts land directly on the host filesystem). This makes `--run-dir` load-bearing for isolation: +The host's run dir is bind-mounted read-write at `/opt/coder-eval/grader/output`, below a root-only parent. The agent cannot traverse it; the harness captures `/work/agent` there after the agent lifecycle. This still makes `--run-dir` load-bearing for host safety: - **Do not** point `--run-dir` at a symlink. Docker resolves the source of a bind mount; following a symlink would silently grant the container RW access to a different host location. - **Do not** point `--run-dir` at a sensitive parent (e.g. `$HOME` directly, `/etc`, a repo root). Use a dedicated `runs/` subtree. @@ -286,24 +301,28 @@ The host's run dir is bind-mounted **read-write** into the container at the same | Layer | Location | |---|---| -| Agent process (Claude Code SDK) | inside container | -| Sandbox + per-row criterion checking | inside container | +| Agent process and descendants | container, UID/GID `2000:2000`, `/work/agent` | +| Harness + supported criterion checking | container, root, `/opt/coder-eval/grader` | | **`task.json` serialization** | **container → host bind mount** | | Per-criterion `aggregate()` (P/R/F1, suite thresholds) | host | | Reports, run summary, experiment rollups | host | -`task.json` is the only artifact crossing the boundary. Aggregation reads it via the existing host pipeline unchanged. +`task.json`, logs, and captured workspace artifacts cross through the protected output bind mount. Aggregation reads `task.json` through the existing host pipeline unchanged. ## Limitations -- **Relative template paths**: `template_sources[].path` is resolved to a host absolute path *before* staging, so it won't exist inside the container unless you also forward the parent dir via `sandbox.docker.extra_mounts`. +- **Dynamic privileged graders**: `run_command`, `uipath_eval`, and `agent_judge` are rejected in protected mode until a separate minimal-input grader sandbox exists. This prevents candidate-controlled code from turning a privileged grader into a confused deputy. Migrate to static built-in criteria or explicitly disable isolation only for a trusted transitional run. +- **Custom work directories and extra mounts**: protected mode currently rejects `docker.working_dir` and `docker.extra_mounts` because their agent/private audience is ambiguous. Use the generated `/work/agent` workspace and `template_sources`. +- **Runtime-kit injection**: not yet compatible with protected mode. Extend the current framework image instead. - **No container reuse across tasks**: each task = one fresh container. Adds ~1–3 s startup overhead per task; negligible vs. LLM latency. - **macOS Keychain auth**: not reachable from the container; set `ANTHROPIC_API_KEY` (direct) or Bedrock credentials instead. ## Architecture -The host's `DockerRunner` (`coder_eval/isolation/docker_runner.py`) renders the `docker run` argv, bind-mounts task inputs at `/work/input`, allocates an output dir at `/work/output`, and tails container stdout into `docker.log` in the task's run dir. +The host's `DockerRunner` rewrites host paths to protected container paths, renders `docker run`, and tails container stdout into `docker.log`. Inputs land at `/opt/coder-eval/grader/input`, output at `/opt/coder-eval/grader/output`, the raw task directory at `/opt/coder-eval/grader/task_dir`, and the agent workspace at `/work/agent`. + +Inside the container, the root entrypoint verifies the protected parent. The standard Orchestrator prepares the workspace as root, grants only that generated tree to UID 2000, and launches the selected agent through the shared privilege-drop policy. The host reads the final result from the protected output mount and feeds the existing aggregation pipeline. -Inside the container, the entrypoint invokes `coder-eval _run-task-internal` (hidden subcommand), which loads the staged YAML + context, runs the standard in-process Orchestrator (driver auto-coerced back to `tempdir`), and writes `task.json` to the output mount. Host reads it and feeds the existing aggregation pipeline. +Protected runs use Docker's init reaper and default to a 512-process limit when `limits.max_pids` is not specified. An explicit `max_pids` value takes precedence. Before trusted post-run/finalization begins, the harness stops the SDK, repeatedly kills every remaining UID-2000 process, and fails closed if that UID cannot be emptied. A `result_kind` discriminator on `CriterionResult` ensures `ClassificationCriterionResult` subclasses survive the JSON round-trip — without it, host-side aggregation would silently lose `observed_label`/`expected_label`. diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 4e1475be..49ef119c 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -22,6 +22,9 @@ import contextlib import logging import os +import shlex +import shutil +import tempfile import time from collections.abc import AsyncIterator, Callable from contextlib import AsyncExitStack @@ -39,7 +42,10 @@ TurnTimeoutError, truncate_crash_message, ) +from coder_eval.isolation.agent_identity import agent_isolation_enabled from coder_eval.models import ( + AGENT_HOME, + CONTAINER_DROP_SHIM, AgentKind, AntigravityAgentConfig, ApiRoute, @@ -66,7 +72,11 @@ TurnEndStatus, TurnStartEvent, ) -from coder_eval.utils import expand_env_vars +from coder_eval.utils import ( + AGENT_ENV_SCRUB_PREFIXES, + AGENT_ENV_SCRUB_VARS, + expand_env_vars, +) logger = logging.getLogger(__name__) @@ -215,6 +225,7 @@ def __init__( # Absolute dirs to prepend to PATH so sandbox mock CLIs shadow real ones # for the harness's run_command tool — applied at spawn (see start()). self._env_path_prepend: list[str] = [] + self._drop_shim_dir: Path | None = None # _state / _iteration / _iteration_was_incremented / pending_turn lifecycle # bookkeeping lives on the Agent base class (shared defaults + helpers). self._log = PrefixedAdapter(logger, {"prefix": instance_name}) @@ -223,6 +234,22 @@ def _effective_model(self) -> str: """Resolve the model: task ``agent.model`` > ``ANTIGRAVITY_MODEL`` > default.""" return self.config.model or settings.antigravity_model or _DEFAULT_MODEL + def _stage_localharness_drop_shim(self) -> Path: + """Shadow localharness with a wrapper around the shared setpriv policy.""" + + real = shutil.which("localharness") + if real is None: + raise RuntimeError("agent isolation is enabled but localharness is not available on PATH") + shim_dir = Path(tempfile.mkdtemp(prefix="antigravity-drop-")) + wrapper = shim_dir / "localharness" + wrapper.write_text( + f'#!/usr/bin/env bash\nexec {CONTAINER_DROP_SHIM} {shlex.quote(real)} "$@"\n', + encoding="utf-8", + ) + wrapper.chmod(0o555) + self._drop_shim_dir = shim_dir + return shim_dir + def _resolve_skills_paths(self, plugin_tools_dir: str | None) -> list[str]: """Resolve skill search-path roots for the harness's native ``skills_paths``. @@ -316,6 +343,8 @@ async def start( """ self.working_directory = Path(working_directory) self._env_path_prepend = list(env_path_prepend or []) + if agent_isolation_enabled(): + self._env_path_prepend.insert(0, str(self._stage_localharness_drop_shim())) self._state = AgentState.WORKING try: @@ -391,20 +420,33 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: PATH window, or its harness would inherit another task's mock dirs. """ async with _harness_spawn_lock(): - if not self._env_path_prepend: - yield - return + scrubbed = { + name: os.environ.pop(name) + for name in list(os.environ) + if name in AGENT_ENV_SCRUB_VARS or name.startswith(AGENT_ENV_SCRUB_PREFIXES) + } path_key = next((k for k in os.environ if k.upper() == "PATH"), "PATH") - original = os.environ.get(path_key) - os.environ[path_key] = os.pathsep.join([*self._env_path_prepend, original or ""]) - self._log.debug("PATH prepend for harness spawn: %s", os.pathsep.join(self._env_path_prepend)) + original_path = os.environ.get(path_key) + original_home = os.environ.get("HOME") + if self._env_path_prepend: + os.environ[path_key] = os.pathsep.join([*self._env_path_prepend, original_path or ""]) + self._log.debug("PATH prepend for harness spawn: %s", os.pathsep.join(self._env_path_prepend)) + if agent_isolation_enabled(): + os.environ["HOME"] = AGENT_HOME try: yield finally: - if original is None: - os.environ.pop(path_key, None) - else: - os.environ[path_key] = original + os.environ.update(scrubbed) + if self._env_path_prepend: + if original_path is None: + os.environ.pop(path_key, None) + else: + os.environ[path_key] = original_path + if agent_isolation_enabled(): + if original_home is None: + os.environ.pop("HOME", None) + else: + os.environ["HOME"] = original_home async def communicate( self, @@ -579,6 +621,10 @@ async def _teardown(self) -> None: if stack is not None: with contextlib.suppress(Exception): await stack.aclose() + shim_dir, self._drop_shim_dir = self._drop_shim_dir, None + if shim_dir is not None: + with contextlib.suppress(Exception): + await asyncio.to_thread(shutil.rmtree, shim_dir, ignore_errors=True) class _AntigravityTurnState: diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index 71cb2267..de678f72 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -37,7 +37,10 @@ format_timeout_reason, ) from coder_eval.formatting import format_messages, format_payload +from coder_eval.isolation.agent_identity import agent_isolation_enabled from coder_eval.models import ( + AGENT_HOME, + CONTAINER_CLAUDE_SHIM, AgentKind, ApiRoute, BedrockRoute, @@ -70,7 +73,7 @@ TurnEndStatus, TurnStartEvent, ) -from coder_eval.utils import dump_dataclass, process_plugins +from coder_eval.utils import dump_dataclass, process_plugins, scrub_agent_env_overrides logger = logging.getLogger(__name__) @@ -765,9 +768,11 @@ def _build_sdk_env( Returns: Tuple of (env_vars_dict, model_override_or_None). """ - base_env: dict[str, str] = {} + base_env: dict[str, str] = scrub_agent_env_overrides() if path := os.environ.get("PATH"): base_env["PATH"] = path + if agent_isolation_enabled(): + base_env["HOME"] = AGENT_HOME if path_prepend: prefix = os.pathsep.join(path_prepend) @@ -1199,6 +1204,10 @@ def _build_claude_query( if isinstance(self.config.claude_settings, dict) else self.config.claude_settings, mcp_servers=self._extra_mcp_servers, + # The SDK accepts a single CLI executable path. The baked wrapper + # invokes the real Claude binary through the same setpriv policy as + # the other backends (UID/GID drop, no capabilities, no_new_privs). + cli_path=CONTAINER_CLAUDE_SHIM if agent_isolation_enabled() else None, **self.config.sdk_options, ) diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index c6e4b2d3..68a6262e 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -25,7 +25,10 @@ TurnTimeoutError, truncate_crash_message, ) +from coder_eval.isolation.agent_identity import agent_isolation_enabled, grant_agent_workspace from coder_eval.models import ( + AGENT_HOME, + CONTAINER_DROP_SHIM, AgentKind, ApiRoute, AssistantMessage, @@ -52,7 +55,7 @@ TurnEndStatus, TurnStartEvent, ) -from coder_eval.utils import expand_env_vars +from coder_eval.utils import expand_env_vars, scrub_agent_env_overrides logger = logging.getLogger(__name__) @@ -660,6 +663,7 @@ def __init__( self.working_directory: Path | None = None self._env_path_prepend: list[str] = [] self._login_shell_home: Path | None = None + self._runtime_codex_home: Path | None = None # _state / _iteration / _iteration_was_incremented / pending_turn lifecycle # bookkeeping lives on the Agent base class (shared defaults + helpers). self._log = PrefixedAdapter(logger, {"prefix": instance_name}) @@ -689,6 +693,13 @@ async def start( self.working_directory = Path(working_directory) self._env_path_prepend = list(env_path_prepend or []) self._setup_login_shell_home() + if agent_isolation_enabled(): + if self._login_shell_home is None: + self._login_shell_home = Path(tempfile.mkdtemp(prefix="coder-eval-codex-home-")) + self._runtime_codex_home = Path(AGENT_HOME) / ".codex" + await asyncio.to_thread(self._runtime_codex_home.mkdir, parents=True, exist_ok=True) + grant_agent_workspace(self._login_shell_home) + grant_agent_workspace(self._runtime_codex_home) self._state = AgentState.WORKING try: @@ -696,7 +707,11 @@ async def start( # Build CodexConfig with environment variables for custom API configuration env_override = self._build_codex_env() - config = CodexConfig(env=env_override) if env_override else None + launch_args_override = self._drop_privilege_launch_args() + if env_override is not None or launch_args_override is not None: + config = CodexConfig(env=env_override, launch_args_override=launch_args_override) + else: + config = None # Initialize the Codex client (context manager compatible). Close any # prior client first: start() is driven through execute_with_retry, so @@ -1077,6 +1092,16 @@ def _effective_model(self) -> str | None: """ return self.config.model or settings.codex_model + @staticmethod + def _drop_privilege_launch_args() -> tuple[str, ...] | None: + """Replace the SDK app-server argv with the shared UID-drop launcher.""" + + if not agent_isolation_enabled(): + return None + from codex_cli_bin import bundled_codex_path + + return (CONTAINER_DROP_SHIM, str(bundled_codex_path()), "app-server", "--listen", "stdio://") + def _build_codex_env(self) -> dict[str, str] | None: """Build the environment passed to the Codex app-server. @@ -1091,7 +1116,7 @@ def _build_codex_env(self) -> dict[str, str] | None: (and normalizes the PATH key case-insensitively), so a full PATH value here safely replaces the inherited one. """ - env: dict[str, str] = {} + env: dict[str, str] = scrub_agent_env_overrides() api_key = os.getenv("CODEX_API_KEY") if api_key: env["CODEX_API_KEY"] = api_key @@ -1116,6 +1141,12 @@ def _build_codex_env(self) -> dict[str, str] | None: codex_home = self._codex_home() codex_home.mkdir(parents=True, exist_ok=True) env["CODEX_HOME"] = str(codex_home) + elif agent_isolation_enabled(): + env["HOME"] = AGENT_HOME + env["ZDOTDIR"] = AGENT_HOME + codex_home = self._codex_home() + codex_home.mkdir(parents=True, exist_ok=True) + env["CODEX_HOME"] = str(codex_home) return env if env else None @staticmethod @@ -1157,10 +1188,14 @@ def _setup_login_shell_home(self) -> None: self._cleanup_login_shell_home() if not (self._env_path_prepend and self._login_shell_profiles_supported()): return - original_home = os.environ.get("HOME", "") + # The harness remains root in protected Docker runs. Its HOME and + # ZDOTDIR are private grader state and must never be restored by an + # agent login shell. Use the dedicated agent home as both the runtime + # home and the only profile source in that mode. + original_home = AGENT_HOME if agent_isolation_enabled() else os.environ.get("HOME", "") # Where the user's REAL zsh dotfiles live: their own ZDOTDIR when set, # else their home (zsh's fallback). - original_zdotdir = os.environ.get("ZDOTDIR", "") or original_home + original_zdotdir = AGENT_HOME if agent_isolation_enabled() else os.environ.get("ZDOTDIR", "") or original_home # The profile only ever executes under a POSIX shell, so the PATH # separator is ':' regardless of the host building it. quoted_prepend = shlex.quote(":".join(self._env_path_prepend)) @@ -1766,9 +1801,10 @@ async def _recover_subagent_tool_calls( # Best-effort: a recovery hiccup must never fail the turn. self._log.debug("CodexAgent: sub-agent recovery failed for %s: %s", thread_id, exc) - @staticmethod - def _codex_home() -> Path: + def _codex_home(self) -> Path: """Codex data directory (rollouts live under ``/sessions``).""" + if self._runtime_codex_home is not None: + return self._runtime_codex_home return Path(os.environ.get("CODEX_HOME") or (Path.home() / ".codex")) async def _await_rollout_file(self, home: Path, thread_id: str, *, attempts: int = 20) -> Path | None: diff --git a/src/coder_eval/cli/run_task_internal_command.py b/src/coder_eval/cli/run_task_internal_command.py index 3157e4a7..92e297cd 100644 --- a/src/coder_eval/cli/run_task_internal_command.py +++ b/src/coder_eval/cli/run_task_internal_command.py @@ -1,9 +1,10 @@ """Internal CLI subcommand executed inside the Docker container. Not part of the public CLI surface -- the host's :class:`DockerRunner` -invokes it via ``docker run``. It loads the staged task + context from -``/work/input``, runs one full evaluation cycle in-process (driver=tempdir), -and writes ``task.json`` + ``task.html`` to ``/work/output``. +invokes it via ``docker run``. It loads the staged task + context from the +root-only grader input directory, runs one full evaluation cycle in-process +(driver=tempdir), and writes ``task.json`` + ``task.html`` to the root-only +grader output directory. The container always exits 0 once ``task.json`` is written, even if the task itself failed -- criterion failures are signaled via the final_status @@ -29,6 +30,8 @@ ) from coder_eval.logging_config import setup_logging from coder_eval.models import ( + AGENT_HOME, + CONTAINER_GRADER_DIR, CONTAINER_INPUT_DIR, CONTAINER_OUTPUT_DIR, CONTAINER_TASK_DIR, @@ -82,6 +85,27 @@ def run_task_internal_command( log_level = "DEBUG" if verbose else settings.log_level setup_logging(level=log_level) + from coder_eval.isolation.agent_identity import ( + agent_isolation_enabled, + grant_agent_workspace, + require_isolation_runtime, + ) + + if agent_isolation_enabled(): + require_isolation_runtime() + grader_root = Path(CONTAINER_GRADER_DIR) + grader_stat = grader_root.stat() + if grader_stat.st_uid != 0 or grader_stat.st_mode & 0o077: + raise RuntimeError( + f"protected grader root must be root-owned mode 0700: {grader_root} " + + f"(uid={grader_stat.st_uid}, mode={oct(grader_stat.st_mode & 0o777)})" + ) + claude_state = Path(AGENT_HOME) / ".claude" + if claude_state.exists(): + # This is the disposable host-side copy mounted for the run, never + # the user's real ~/.claude directory. + grant_agent_workspace(claude_state) + # Start the host-heartbeat watchdog: if the host process dies # ungracefully (SIGKILL, Claude-Code Escape, crash) before it can # `docker kill` us, the heartbeat file in output_dir goes stale and diff --git a/src/coder_eval/isolation/agent_identity.py b/src/coder_eval/isolation/agent_identity.py new file mode 100644 index 00000000..db0dbb95 --- /dev/null +++ b/src/coder_eval/isolation/agent_identity.py @@ -0,0 +1,113 @@ +"""Linux identity helpers for the in-container evaluated agent.""" + +from __future__ import annotations + +import contextlib +import os +import signal +import sys +import time +from pathlib import Path + +from coder_eval.models import AGENT_GID, AGENT_UID + + +AGENT_ISOLATION_ENV = "CODER_EVAL_AGENT_ISOLATION" +AGENT_KILL_TIMEOUT_SECONDS = 2.0 + + +def agent_isolation_enabled() -> bool: + """Whether the Docker host requested the UID/GID agent boundary.""" + + return os.environ.get(AGENT_ISOLATION_ENV) == "1" + + +def require_isolation_runtime() -> None: + """Fail closed unless Linux root can perform the requested UID drop.""" + + if not agent_isolation_enabled(): + return + if sys.platform != "linux" or not hasattr(os, "geteuid") or os.geteuid() != 0: + raise RuntimeError("agent UID/GID isolation requires a native Linux container running the harness as root") + + +def grant_agent_workspace(path: Path) -> None: + """Give the unprivileged identity ownership of a generated workspace. + + The caller may pass only disposable sandbox content, never a raw host source + checkout. Symlinks are chowned without following their targets. + """ + + if not agent_isolation_enabled(): + return + require_isolation_runtime() + if not path.is_absolute() or not path.exists(): + raise RuntimeError(f"agent workspace must be an existing absolute path: {path}") + + failures: list[str] = [] + chown = getattr(os, "chown", None) + if chown is None: + raise RuntimeError("agent UID/GID isolation requires os.chown") + + def grant(candidate: Path) -> None: + try: + chown(candidate, AGENT_UID, AGENT_GID, follow_symlinks=False) + except OSError as exc: + failures.append(f"{candidate}: {exc}") + + grant(path) + if path.is_dir() and not path.is_symlink(): + for root_name, dirnames, filenames in os.walk(path, followlinks=False): + root = Path(root_name) + for name in (*dirnames, *filenames): + grant(root / name) + + if failures: + detail = "; ".join(failures[:5]) + raise RuntimeError(f"failed to grant generated workspace to agent uid {AGENT_UID}: {detail}") + + +def _agent_pids() -> list[int]: + """Return processes whose real/effective/saved/fs UID includes the agent.""" + + pids: list[int] = [] + for entry in Path("/proc").iterdir(): + if not entry.name.isdigit(): + continue + try: + status_lines = (entry / "status").read_text(encoding="utf-8").splitlines() + uid_line = next(line for line in status_lines if line.startswith("Uid:")) + uids = [int(value) for value in uid_line.split()[1:]] + except (OSError, StopIteration, ValueError): + continue + if AGENT_UID in uids: + pids.append(int(entry.name)) + return pids + + +def _signal_agent_pids(pids: list[int], sig: signal.Signals) -> None: + for pid in pids: + with contextlib.suppress(OSError, ProcessLookupError): + os.kill(pid, sig) + + +def terminate_agent_processes() -> None: + """Terminate and verify removal of every process owned by the agent UID.""" + + if not agent_isolation_enabled(): + return + require_isolation_runtime() + + _signal_agent_pids(_agent_pids(), signal.SIGTERM) + time.sleep(0.1) + + sigkill = getattr(signal, "SIGKILL", signal.SIGTERM) + deadline = time.monotonic() + AGENT_KILL_TIMEOUT_SECONDS + while pids := _agent_pids(): + _signal_agent_pids(pids, sigkill) + if time.monotonic() >= deadline: + residual = _agent_pids() + if residual: + raise RuntimeError(f"agent UID {AGENT_UID} still owns processes after SIGKILL: {residual[:10]}") + return + time.sleep(0.02) diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index 5185494d..791e28c7 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -26,8 +26,11 @@ from coder_eval.logging_config import DEFAULT_LOG_TAIL_MAX_BYTES from coder_eval.models import ( + AGENT_HOME, + CONTAINER_AGENT_WORK_DIR, CONTAINER_INPUT_DIR, CONTAINER_OUTPUT_DIR, + CONTAINER_TASK_DIR, CONTAINER_WORK_DIR, RESERVED_CONTAINER_DIRS, AgentKind, @@ -64,6 +67,7 @@ # explicitly via `--add-host host.docker.internal:host-gateway`. _DOCKER_HOST_ALIAS = "host.docker.internal" _LOOPBACK_HOSTS = frozenset({"localhost", "127.0.0.1", "::1"}) +DEFAULT_AGENT_ISOLATION_MAX_PIDS = 512 def _rewrite_loopback_for_container(url: str) -> str | None: @@ -256,6 +260,37 @@ def _preflight_image_version(image: str) -> None: ) +def _preflight_agent_isolation_image(image: str) -> None: + """Require an image that contains the declared UID/GID launch boundary.""" + + try: + result = subprocess.run( + [ + "docker", + "image", + "inspect", + "--format", + '{{ index .Config.Labels "org.coder-eval.agent-isolation" }}', + image, + ], + check=True, + capture_output=True, + text=True, + encoding="utf-8", + timeout=10, + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as exc: + raise DockerRunError( + f"cannot verify UID/GID isolation support for image {image!r}; build or pull the image first" + ) from exc + capability = result.stdout.strip() + if capability != "uid-gid-v1": + raise DockerRunError( + f"image {image!r} does not declare org.coder-eval.agent-isolation=uid-gid-v1; " + + "rebuild it from the latest coder-eval-agent image or disable isolation explicitly" + ) + + _CONTAINER_NAME_INVALID = re.compile(r"[^a-zA-Z0-9_.-]") # A leading Windows drive letter (``C:\foo`` / ``c:/foo``). Used so the colon @@ -482,6 +517,10 @@ def __init__( # _build_argv mounts read-write. None when there is no ~/.claude to # forward or the mount is opted out (CODER_EVAL_NO_CLAUDE_MOUNT). self._claude_mount_src: Path | None = None + # Prepared before argv rendering. Raw sources are mounted under the + # root-only grader parent at unrelated container paths. + self._private_source_mounts: list[tuple[Path, str]] = [] + self._host_to_private_paths: dict[str, str] = {} # Resolved in run() (needs the built image for "auto"). Concrete WORKDIR the # agent runs at + copies out from; None = standard artifacts workspace. self._workspace_dir: str | None = None @@ -503,6 +542,7 @@ async def run(self) -> EvaluationResult: dispatcher converts that to an ERROR-status EvaluationResult. """ _preflight() + self._validate_agent_isolation_compatibility() # Resolve the run image: build from a Dockerfile if configured (which # overrides `image`), else use the configured image. The build is # side-effecting, so it runs in a worker thread like the other docker @@ -521,6 +561,8 @@ async def run(self) -> EvaluationResult: # a task-supplied Dockerfile won't carry the org.coder-eval.version label. if not self._docker_config.dockerfile_path: await asyncio.to_thread(_preflight_image_version, image) + if self._docker_config.agent_isolation: + await asyncio.to_thread(_preflight_agent_isolation_image, image) await asyncio.to_thread(self.rt.run_dir.mkdir, parents=True, exist_ok=True) # Docker WORKDIR alignment: resolve the concrete workspace path @@ -528,6 +570,18 @@ async def run(self) -> EvaluationResult: # /root). Forwarded to the in-container orchestrator via the staged context # and rendered as `docker run -w`. None keeps the standard artifacts workspace. self._workspace_dir = await asyncio.to_thread(_resolve_workspace_dir, self._docker_config.working_dir, image) + if self._docker_config.agent_isolation: + if self._workspace_dir is not None: + raise DockerRunError( + "docker.agent_isolation does not yet support docker.working_dir; " + + "use the default generated /work/agent workspace or disable isolation explicitly" + ) + if self._docker_config.extra_mounts: + raise DockerRunError( + "docker.agent_isolation rejects extra_mounts because their agent/private audience is ambiguous; " + + "stage the required input through template_sources or disable isolation explicitly" + ) + self._workspace_dir = CONTAINER_AGENT_WORK_DIR # Stage only the inputs (task YAML + context). The *output* dir is # the host's run_dir itself, bind-mounted at the same path inside @@ -612,9 +666,13 @@ async def _stage_inputs(self, input_dir: Path) -> None: # has since mutated rt.task in-memory (e.g. --model, -D run_limits.max_turns), and the # container needs to see those mutations. task_yaml_in = input_dir / "task.yaml" + task_payload = self.rt.task.model_dump(mode="json") + if self._docker_config.agent_isolation: + await asyncio.to_thread(self._prepare_isolated_sources) + task_payload = self._rewrite_task_paths(task_payload) def _dump_task_yaml() -> str: - return yaml.safe_dump(self.rt.task.model_dump(mode="json"), sort_keys=False) + return yaml.safe_dump(task_payload, sort_keys=False) task_yaml_text = await asyncio.to_thread(_dump_task_yaml) await asyncio.to_thread(task_yaml_in.write_text, task_yaml_text, encoding="utf-8") @@ -637,6 +695,122 @@ def _dump_task_yaml() -> str: ) await asyncio.to_thread((input_dir / "context.json").write_text, context_payload, encoding="utf-8") + def _validate_agent_isolation_compatibility(self) -> None: + """Reject task features whose privileged behavior is not isolated yet.""" + + if not self._docker_config.agent_isolation: + return + agent_type = str(self.rt.task.agent.type) if self.rt.task.agent and self.rt.task.agent.type else "" + supported_agents = { + AgentKind.CLAUDE_CODE.value, + AgentKind.CODEX.value, + AgentKind.ANTIGRAVITY.value, + AgentKind.NONE.value, + } + if agent_type not in supported_agents: + raise DockerRunError( + f"docker.agent_isolation has no verified UID-drop launch seam for agent type {agent_type!r}" + ) + + # These criterion implementations can execute another agent or arbitrary + # task-authored commands in the privileged harness. If that execution + # imports candidate-controlled code, it can act as a confused deputy and + # publish hidden grader bytes. A separate minimal-input grader sandbox is + # required before they can run in protected mode. + unsupported_criteria = sorted( + { + criterion.type + for criterion in self.rt.task.success_criteria + if criterion.type in {"agent_judge", "run_command", "uipath_eval"} + } + ) + if unsupported_criteria: + raise DockerRunError( + "docker.agent_isolation rejects privileged dynamic criteria until they have a separate grader " + + f"sandbox: {unsupported_criteria}. Use static/built-in criteria or explicitly disable isolation." + ) + + def _prepare_isolated_sources(self) -> None: + """Prepare private raw-source mount mappings. + + This method never changes ownership or permissions on a source checkout. + Raw sources remain read-only and are mounted only below the image's + root-owned ``/opt/coder-eval/grader`` directory. + """ + + self._private_source_mounts = [] + self._host_to_private_paths = {} + + task_dir = self.rt.task_file.parent.resolve() if self.rt.task_file else None + if task_dir is not None: + self._host_to_private_paths[str(task_dir)] = CONTAINER_TASK_DIR + + if self.rt.task.agent and self.rt.task.agent.system_prompt_file: + raise DockerRunError( + "docker.agent_isolation requires system_prompt_file to be resolved to inline system_prompt " + + "before container staging" + ) + + from coder_eval.models import TemplateDirSource + + template_index = 0 + for source in self.rt.task.sandbox.template_sources or []: + if not isinstance(source, TemplateDirSource): + continue + host_path = Path(source.path).resolve() + if task_dir is not None and (host_path == task_dir or task_dir in host_path.parents): + continue + self._register_private_mount(host_path, f"/opt/coder-eval/grader/templates/source-{template_index}") + template_index += 1 + + reference = self.rt.task.reference + if reference is not None: + if reference.directory: + self._register_external_private_path( + Path(reference.directory), "/opt/coder-eval/grader/references/directory" + ) + if reference.file: + reference_file = Path(reference.file).resolve() + self._register_external_private_path( + reference_file.parent, "/opt/coder-eval/grader/references/file-parent" + ) + + def _register_external_private_path(self, source: Path, target: str) -> None: + source = source.resolve() + task_dir = self.rt.task_file.parent.resolve() if self.rt.task_file else None + if task_dir is not None and (source == task_dir or task_dir in source.parents): + return + self._register_private_mount(source, target) + + def _register_private_mount(self, source: Path, target: str) -> None: + source = source.resolve() + key = str(source) + if key in self._host_to_private_paths: + return + self._host_to_private_paths[key] = target + self._private_source_mounts.append((source, target)) + + def _rewrite_task_paths(self, payload: dict[str, object]) -> dict[str, object]: + """Rewrite host paths to their protected container mount locations.""" + + replacements = sorted(self._host_to_private_paths.items(), key=lambda item: len(item[0]), reverse=True) + + def rewrite(value: object) -> object: + if isinstance(value, str): + for source, target in replacements: + value = value.replace(source, target) + return value + if isinstance(value, list): + return [rewrite(item) for item in value] + if isinstance(value, dict): + return {key: rewrite(item) for key, item in value.items()} + return value + + rewritten = rewrite(payload) + if not isinstance(rewritten, dict): + raise DockerRunError("internal error rewriting staged task paths") + return rewritten + async def _stream_container_output(self, proc: asyncio.subprocess.Process, log_fh: TextIO) -> int: """Stream the container's stdout, returning its exit code. @@ -1067,7 +1241,7 @@ def _assert_runtime_image(self, image: str, dockerfile: Path) -> None: + "task-specific layers on top. See docs/DOCKER_ISOLATION.md." ) - def _build_argv( + def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirrors docker-run argv self, input_dir: Path, output_dir: Path, *, container_name: str, image: str | None = None ) -> list[str]: cfg = self._docker_config @@ -1078,7 +1252,10 @@ def _build_argv( if image is None: image = cfg.image - argv: list[str] = ["docker", "run", "--rm", "--name", container_name] + # tini as PID 1 reaps orphaned grandchildren. The harness also scans + # and kills the dedicated agent UID before finalization; --init keeps a + # double-forked process from surviving only as an unreapable zombie. + argv: list[str] = ["docker", "run", "--rm", "--init", "--name", container_name] # Pin the framework entrypoint at run time rather than trusting whatever # the task image baked into ENTRYPOINT. This makes the orchestrator launch @@ -1099,6 +1276,8 @@ def _build_argv( argv += ["--cpus", str(self._limits.max_cpus)] if self._limits.max_pids is not None: argv += ["--pids-limit", str(self._limits.max_pids)] + elif cfg.agent_isolation: + argv += ["--pids-limit", str(DEFAULT_AGENT_ISOLATION_MAX_PIDS)] # Forward environment variables: explicit allowlist (optionally extended via env_passthrough_extra). # `--env VAR` (name-only) tells docker to copy the value from our current env at @@ -1113,11 +1292,21 @@ def _build_argv( for env_var in merged_allowlist: # LITELLM_BASE_URL / LITELLM_COST_LOG are forwarded below with a value # rewrite (host alias / absolute mount path), not name-only. - if env_var in ("LITELLM_BASE_URL", "LITELLM_COST_LOG"): + if env_var in ("LITELLM_BASE_URL", "LITELLM_COST_LOG", "SKILLS_REPO_PATH"): + continue + if cfg.agent_isolation and env_var == "HOME": continue if env_var in os.environ: argv += ["--env", env_var] + if cfg.agent_isolation and (skills_repo := os.environ.get("SKILLS_REPO_PATH")): + resolved_skills = str(Path(skills_repo).expanduser().resolve()) + private_skills = self._host_to_private_paths.get(resolved_skills) + if private_skills is not None: + argv += ["--env", f"SKILLS_REPO_PATH={private_skills}"] + else: + logger.debug("Not forwarding unstaged SKILLS_REPO_PATH into protected container: %s", resolved_skills) + # LITELLM_BASE_URL points at a proxy on the HOST. A bridge-network container # can't reach the host's loopback, so rewrite localhost/127.0.0.1 to the # docker host alias and publish that alias (`--add-host`) for Linux parity @@ -1151,6 +1340,8 @@ def _build_argv( # sandbox: Codex's Landlock-backed read-only / workspace-write sandboxes # can't initialize inside a container and otherwise fail writes silently. argv += ["--env", "CODER_EVAL_IN_CONTAINER=1"] + if cfg.agent_isolation: + argv += ["--env", "CODER_EVAL_AGENT_ISOLATION=1"] # Hard-disable telemetry INSIDE the container. The app ships a baked-in # default connection string, so without this the in-container orchestrator @@ -1173,7 +1364,7 @@ def _build_argv( host_task_dir: Path | None = None if self.rt.task_file: host_task_dir = self.rt.task_file.parent.resolve() - argv += ["-v", f"{host_task_dir}:{host_task_dir}:ro"] + argv += ["-v", f"{host_task_dir}:{CONTAINER_TASK_DIR}:ro"] # Forward the host's Claude Code OAuth state so the in-container CLI # inherits the same login as the host. We mount a *throwaway lean copy* # of ~/.claude (made by _prepare_host_mounts) read-WRITE at the host's @@ -1184,7 +1375,11 @@ def _build_argv( # doesn't exist or the mount is opted out (CODER_EVAL_NO_CLAUDE_MOUNT=1). if self._claude_mount_src is not None: host_claude_dir = Path.home() / ".claude" - argv += ["-v", f"{self._claude_mount_src}:{host_claude_dir}"] + claude_target = Path(AGENT_HOME) / ".claude" if cfg.agent_isolation else host_claude_dir + argv += ["-v", f"{self._claude_mount_src}:{claude_target}"] + + for source, target in self._private_source_mounts: + argv += ["-v", f"{source.resolve()}:{target}:ro"] # Auto-mount host paths the task references so they resolve inside # the container at the *same* path they have on the host. @@ -1232,16 +1427,17 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: from coder_eval.models import TemplateDirSource sandbox_cfg = self.rt.task.sandbox - for source in (sandbox_cfg.template_sources or []) if sandbox_cfg else []: - if isinstance(source, TemplateDirSource): - _auto_mount(source.path) + if not cfg.agent_isolation: + for source in (sandbox_cfg.template_sources or []) if sandbox_cfg else []: + if isinstance(source, TemplateDirSource): + _auto_mount(source.path) # Defensive: system_prompt_file is normally inlined into # system_prompt by load_task / experiment resolution, but a variant # could conceivably inject an absolute path that survives. Cover # that path so the in-container Orchestrator can read it. agent_cfg = self.rt.task.agent - if agent_cfg and agent_cfg.system_prompt_file: + if not cfg.agent_isolation and agent_cfg and agent_cfg.system_prompt_file: _auto_mount(agent_cfg.system_prompt_file, dir_only=False) # reference.file / reference.directory: if a task ships absolute @@ -1249,7 +1445,7 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: # ``..``), they must be mounted explicitly. Relative paths under # task_dir are already covered by the symmetric task_dir mount. reference = self.rt.task.reference - if reference is not None: + if not cfg.agent_isolation and reference is not None: _auto_mount(reference.file, dir_only=False) _auto_mount(reference.directory) for mount in cfg.extra_mounts: @@ -1261,7 +1457,12 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: # runs the agent there). NO bind mount targets it -- capture is a copy-out # (see Orchestrator._cleanup), not a mount, so baked inputs/HOME survive. if self._workspace_dir is not None: - _assert_workspace_not_reserved(self._workspace_dir) + # Under isolation the workspace is the framework's OWN agent dir + # (assigned above, not task-authored), and that path is deliberately + # in RESERVED_CONTAINER_DIRS. The assertion guards task/image-supplied + # values, so exempt exactly the isolation-managed constant. + if not (cfg.agent_isolation and self._workspace_dir == CONTAINER_AGENT_WORK_DIR): + _assert_workspace_not_reserved(self._workspace_dir) argv += ["-w", self._workspace_dir] argv += [image] @@ -1271,7 +1472,7 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: argv += ["-v"] argv += ["--output", str(CONTAINER_OUTPUT_DIR)] if host_task_dir is not None: - argv += ["--task-dir", str(host_task_dir)] + argv += ["--task-dir", str(CONTAINER_TASK_DIR)] return argv diff --git a/src/coder_eval/models/__init__.py b/src/coder_eval/models/__init__.py index 51504e92..730f7ef2 100644 --- a/src/coder_eval/models/__init__.py +++ b/src/coder_eval/models/__init__.py @@ -19,6 +19,14 @@ # Container paths (leaf constants; re-exported so consumers obey CE001) from coder_eval.models.container_paths import ( + AGENT_GID, + AGENT_HOME, + AGENT_UID, + AGENT_USERNAME, + CONTAINER_AGENT_WORK_DIR, + CONTAINER_CLAUDE_SHIM, + CONTAINER_DROP_SHIM, + CONTAINER_GRADER_DIR, CONTAINER_INPUT_DIR, CONTAINER_OUTPUT_DIR, CONTAINER_TASK_DIR, @@ -265,6 +273,14 @@ "TemplateSource", # Sandbox "DockerBuildConfig", + "AGENT_GID", + "AGENT_HOME", + "AGENT_UID", + "AGENT_USERNAME", + "CONTAINER_AGENT_WORK_DIR", + "CONTAINER_CLAUDE_SHIM", + "CONTAINER_DROP_SHIM", + "CONTAINER_GRADER_DIR", "CONTAINER_INPUT_DIR", "CONTAINER_OUTPUT_DIR", "CONTAINER_TASK_DIR", diff --git a/src/coder_eval/models/container_paths.py b/src/coder_eval/models/container_paths.py index 0114fe42..35877a98 100644 --- a/src/coder_eval/models/container_paths.py +++ b/src/coder_eval/models/container_paths.py @@ -14,13 +14,36 @@ CONTAINER_WORK_DIR = "/work" -CONTAINER_INPUT_DIR = "/work/input" -CONTAINER_OUTPUT_DIR = "/work/output" -CONTAINER_TASK_DIR = "/work/task_dir" +CONTAINER_AGENT_WORK_DIR = "/work/agent" + +# The evaluated agent shares the container with the trusted harness, but cannot +# traverse this root-owned directory. Hidden task, grader, reference, fixture, +# and result material is mounted below it rather than at agent-readable /work +# paths. +CONTAINER_GRADER_DIR = "/opt/coder-eval/grader" +CONTAINER_INPUT_DIR = f"{CONTAINER_GRADER_DIR}/input" +CONTAINER_OUTPUT_DIR = f"{CONTAINER_GRADER_DIR}/output" +CONTAINER_TASK_DIR = f"{CONTAINER_GRADER_DIR}/task_dir" + +AGENT_UID = 2000 +AGENT_GID = 2000 +AGENT_USERNAME = "agent" +AGENT_HOME = "/home/agent" + +CONTAINER_DROP_SHIM = "/usr/local/bin/coder_eval_drop_privilege.sh" +CONTAINER_CLAUDE_SHIM = "/usr/local/bin/coder_eval_claude_agent.sh" # Paths a task's WORKDIR must never collide with: the container root and every -# framework-owned mount under /work. Consumed by SandboxConfig's working_dir -# validator (models/sandbox.py) and re-asserted host-side in docker_runner. +# framework-owned public or private mount. Consumed by SandboxConfig's +# working_dir validator and re-asserted host-side in docker_runner. RESERVED_CONTAINER_DIRS = frozenset( - {"/", CONTAINER_WORK_DIR, CONTAINER_INPUT_DIR, CONTAINER_OUTPUT_DIR, CONTAINER_TASK_DIR} + { + "/", + CONTAINER_WORK_DIR, + CONTAINER_AGENT_WORK_DIR, + CONTAINER_GRADER_DIR, + CONTAINER_INPUT_DIR, + CONTAINER_OUTPUT_DIR, + CONTAINER_TASK_DIR, + } ) diff --git a/src/coder_eval/models/sandbox.py b/src/coder_eval/models/sandbox.py index d2083d09..d23e547d 100644 --- a/src/coder_eval/models/sandbox.py +++ b/src/coder_eval/models/sandbox.py @@ -195,6 +195,14 @@ class DockerDriverConfig(BaseModel): default="bridge", description="Container network. 'bridge' for tasks needing LLM/pkg access; 'none' for fully sealed runs.", ) + agent_isolation: bool = Field( + default=True, + description=( + "Run the evaluated agent under the image's dedicated unprivileged UID/GID and expose local plugins " + "only through manifest-verified bundles. Enabled by default. Set false only for temporary migration " + "of a trusted task; false is not a secure evaluation boundary." + ), + ) working_dir: str | None = Field( default=None, description=( diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 8640dd7f..36eee20d 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -473,6 +473,10 @@ async def run(self) -> EvaluationResult: # the run as FinalStatus.ERROR; _run_post_run_commands and # _cleanup still execute via the finally block. await self._run_pre_run_commands() + # Trusted pre-run commands may create root-owned files. Re-grant + # only the disposable sandbox before the first model turn; + # hidden harness mounts live outside this tree. + await self._grant_current_sandbox_to_agent() # Enforce task-level timeout via an OS-thread watchdog that # SIGKILLs the in-flight CLI subprocess AND cancels this @@ -597,6 +601,11 @@ def _kill_agent_subprocess_sync() -> None: # awaits below run normally after the CancelledError is caught. teardown_interrupt: BaseException | None = None try: + # Protected Docker runs stop the SDK process and kill any + # same-UID descendants before post-run commands, capture, + # or task.json publication. Background shells must not + # observe trusted finalization or keep using the mock RPC. + await self._stop_isolated_agent_processes() # BEFORE post-run/cleanup: needs the live sandbox to resolve # the agent-aligned `uip`, and post-task tool state on disk. self._refresh_runtime_tool_versions() @@ -1071,6 +1080,10 @@ async def _setup_sandbox() -> Any: assert self.result is not None, "Result not initialized" self.result.sandbox_path = str(sandbox_dir) + # Root prepares templates and dependencies, then hands the generated + # workspace—not any raw task/plugin/reference source—to the agent UID. + await self._grant_current_sandbox_to_agent() + # Determine API routing from settings.api_backend enum self.route = resolve_route(settings) self.eval_route = resolve_evaluation_route(settings, self.route) @@ -2260,6 +2273,27 @@ async def _run_post_run_commands(self) -> None: return await self._run_command_list(self.task.post_run, self.result.post_run_results, "post_run") + async def _stop_isolated_agent_processes(self) -> None: + """Stop the SDK and residual same-UID children before finalization.""" + + from .isolation.agent_identity import agent_isolation_enabled, terminate_agent_processes + + if not agent_isolation_enabled(): + return + if self.agent is not None: + with suppress(Exception): + await self.agent.stop() + await asyncio.to_thread(terminate_agent_processes) + + async def _grant_current_sandbox_to_agent(self) -> None: + """Transfer only the generated sandbox tree to the agent identity.""" + + if self.sandbox is None or self.sandbox.sandbox_dir is None: + return + from .isolation.agent_identity import grant_agent_workspace + + await asyncio.to_thread(grant_agent_workspace, self.sandbox.sandbox_dir) + async def _cleanup(self) -> None: """Clean up all resources.""" # Stop agent diff --git a/src/coder_eval/utils.py b/src/coder_eval/utils.py index 1624dae6..95f2ef93 100644 --- a/src/coder_eval/utils.py +++ b/src/coder_eval/utils.py @@ -86,6 +86,36 @@ def process_plugins( return processed +AGENT_ENV_SCRUB_VARS: tuple[str, ...] = ( + "SKILLS_REPO_PATH", + "TASK_DIR", + # The evaluator's Bedrock credential. No agent needs to INHERIT it: the Claude + # backend sets it explicitly from a resolved BedrockRoute (and blanks it on the + # LiteLLM route), Codex authenticates via CODEX_API_KEY, and Antigravity does not + # use Bedrock. Left inherited it reaches the dropped agent process, where it is + # readable through that process's own environment -- the UID barrier stops + # filesystem access to grading material but cannot hide an agent's own env. + # Scrubbing it also stops an inherited token from silently steering a DirectRoute + # run onto Bedrock (the CLI auto-selects on `process.env.AWS_BEARER_TOKEN_BEDROCK`). + "AWS_BEARER_TOKEN_BEDROCK", +) +AGENT_ENV_SCRUB_PREFIXES: tuple[str, ...] = ("CODER_EVAL_",) + + +def scrub_agent_env_overrides() -> dict[str, str]: + """Mask harness-only variables in SDK subprocess environments. + + Claude and Codex merge their explicit environment over ``os.environ``; + empty-string overrides are therefore the only concurrency-safe removal + mechanism. Antigravity has no environment seam and removes the same names + during its serialized spawn window. + """ + + return { + name: "" for name in os.environ if name in AGENT_ENV_SCRUB_VARS or name.startswith(AGENT_ENV_SCRUB_PREFIXES) + } + + SKIP = object() # Sentinel marking values that serialize_value should drop from the result. diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index 95ee117f..2123fd13 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -18,6 +18,7 @@ CodexAgent, ) from coder_eval.models import AgentConfig, AgentKind, parse_agent_config +from coder_eval.utils import AGENT_ENV_SCRUB_VARS class TestCodexAgentInitialization: @@ -128,6 +129,21 @@ def test_sandbox_is_full_access(self, monkeypatch, mode, in_container, os_name): class TestCodexEnvironmentConfiguration: """Test _build_codex_env: only CODEX_API_KEY travels via env.""" + @pytest.fixture(autouse=True) + def _no_ambient_scrub_vars(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Keep ambient evaluator credentials out of the exact-equality assertions. + + ``_build_codex_env`` starts from ``scrub_agent_env_overrides()``, so any + scrubbed name that the host happens to export (a developer's or CI's + ``AWS_BEARER_TOKEN_BEDROCK``, say) shows up as an extra masking entry and + breaks assertions that are only about what codex itself contributes. The + masking behavior has its own coverage in + ``tests/test_docker_identity_isolation.py``. + """ + + for name in AGENT_ENV_SCRUB_VARS: + monkeypatch.delenv(name, raising=False) + def test_build_codex_env_returns_none_without_key(self, monkeypatch): """No CODEX_API_KEY -> None (base URL alone is not enough).""" monkeypatch.delenv("CODEX_API_KEY", raising=False) diff --git a/tests/test_docker_build_failure.py b/tests/test_docker_build_failure.py index 32f28e0d..31c7d024 100644 --- a/tests/test_docker_build_failure.py +++ b/tests/test_docker_build_failure.py @@ -33,6 +33,11 @@ def _make_runner(run_dir: Path) -> DockerRunner: task_id="suri", description="t", initial_prompt="do", + # A concrete agent type is required now that docker.agent_isolation + # defaults to true: the isolation gate rejects a type with no verified + # UID-drop launch seam before the build runs, and this test covers + # build-failure observability rather than that gate. + agent={"type": "claude-code"}, sandbox=SandboxConfig(driver="docker", docker=DockerDriverConfig(image="x:1", dockerfile_path="/df")), success_criteria=[FileExistsCriterion(description="c", path="out.txt")], ) diff --git a/tests/test_docker_identity_isolation.py b/tests/test_docker_identity_isolation.py new file mode 100644 index 00000000..60a94a6a --- /dev/null +++ b/tests/test_docker_identity_isolation.py @@ -0,0 +1,161 @@ +"""Drift guards for the Linux UID/GID agent boundary.""" + +from __future__ import annotations + +import signal +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from coder_eval.agents.codex_agent import CodexAgent +from coder_eval.isolation.docker_runner import DockerRunError, DockerRunner, _preflight_agent_isolation_image +from coder_eval.models import ( + AGENT_GID, + AGENT_HOME, + AGENT_UID, + AgentKind, + ClaudeCodeAgentConfig, + DockerDriverConfig, + RunCommandCriterion, + SandboxConfig, + TaskDefinition, + parse_agent_config, +) +from coder_eval.utils import scrub_agent_env_overrides + + +REPO_ROOT = Path(__file__).resolve().parents[1] + + +def test_image_identity_literals_and_capability_label_match_models() -> None: + dockerfile = (REPO_ROOT / "docker" / "Dockerfile").read_text(encoding="utf-8") + assert f"ARG AGENT_UID={AGENT_UID}" in dockerfile + assert f"ARG AGENT_GID={AGENT_GID}" in dockerfile + assert 'LABEL org.coder-eval.agent-isolation="uid-gid-v1"' in dockerfile + assert "USER agent" not in dockerfile + assert DockerDriverConfig().agent_isolation is True + + +def test_privilege_launcher_clears_capabilities_and_sets_no_new_privs() -> None: + script = (REPO_ROOT / "docker" / "coder_eval_drop_privilege.sh").read_text(encoding="utf-8") + assert "--inh-caps=-all" in script + assert "--ambient-caps=-all" in script + assert "--bounding-set=-all" in script + assert "--no-new-privs" in script + assert "--clear-groups" in script + + +def test_agent_launcher_targets_only_agent_identity() -> None: + script = (REPO_ROOT / "docker" / "coder_eval_drop_privilege.sh").read_text(encoding="utf-8") + assert "--reuid=agent" in script + assert "--regid=agent" in script + + +def test_agent_environment_scrubs_only_present_harness_paths(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("SKILLS_REPO_PATH", "/private/skills") + monkeypatch.setenv("TASK_DIR", "/private/task") + monkeypatch.setenv("CODER_EVAL_AGENT_ISOLATION", "1") + monkeypatch.setenv("ANTHROPIC_API_KEY", "needed-by-agent") + # Keep the exact-equality assertion below valid on a host that exports it. + monkeypatch.delenv("AWS_BEARER_TOKEN_BEDROCK", raising=False) + + overrides = scrub_agent_env_overrides() + + assert overrides == { + "SKILLS_REPO_PATH": "", + "TASK_DIR": "", + "CODER_EVAL_AGENT_ISOLATION": "", + } + assert "ANTHROPIC_API_KEY" not in overrides + + +def test_agent_environment_scrubs_inherited_bedrock_credential(monkeypatch: pytest.MonkeyPatch) -> None: + """The evaluated agent must not inherit the evaluator's Bedrock token. + + The UID barrier blocks filesystem access to grading material but cannot hide a + process's own environment, so a credential left there is readable by the agent + itself. Claude re-sets this explicitly from a resolved BedrockRoute, so masking + the inherited value costs the Bedrock path nothing. + """ + + monkeypatch.setenv("AWS_BEARER_TOKEN_BEDROCK", "evaluator-only-secret") + monkeypatch.setenv("AWS_REGION", "us-east-1") + + overrides = scrub_agent_env_overrides() + + assert overrides["AWS_BEARER_TOKEN_BEDROCK"] == "" + # The region is not a credential and stays inherited. + assert "AWS_REGION" not in overrides + + +def test_isolated_codex_profiles_never_restore_root_harness_home(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("HOME", "/root") + monkeypatch.setenv("ZDOTDIR", "/root/private-zdot") + monkeypatch.setenv("CODER_EVAL_AGENT_ISOLATION", "1") + monkeypatch.setattr(CodexAgent, "_login_shell_profiles_supported", staticmethod(lambda: True)) + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) + agent._env_path_prepend = ["/work/agent/cli_mocks"] + + agent._setup_login_shell_home() + try: + assert agent._login_shell_home is not None + for profile in (".bash_profile", ".profile", ".zshenv", ".zprofile", ".zshrc"): + content = (agent._login_shell_home / profile).read_text(encoding="utf-8") + assert f"export HOME={AGENT_HOME}" in content + assert "/root" not in content + finally: + agent._cleanup_login_shell_home() + + +def test_agent_teardown_rescans_until_uid_has_no_processes(monkeypatch: pytest.MonkeyPatch) -> None: + from coder_eval.isolation import agent_identity + + scans = iter([[41, 42], [42], []]) + signals: list[tuple[int, signal.Signals]] = [] + monkeypatch.setenv("CODER_EVAL_AGENT_ISOLATION", "1") + monkeypatch.setattr(agent_identity, "require_isolation_runtime", lambda: None) + monkeypatch.setattr(agent_identity, "_agent_pids", lambda: next(scans)) + monkeypatch.setattr( + agent_identity, + "_signal_agent_pids", + lambda pids, sig: signals.extend((pid, sig) for pid in pids), + ) + monkeypatch.setattr(agent_identity.time, "sleep", lambda _seconds: None) + + agent_identity.terminate_agent_processes() + + expected_kill = getattr(signal, "SIGKILL", signal.SIGTERM) + assert signals == [(41, signal.SIGTERM), (42, signal.SIGTERM), (42, expected_kill)] + + +def test_isolation_image_label_preflight_accepts_only_declared_capability(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + "coder_eval.isolation.docker_runner.subprocess.run", + lambda *args, **kwargs: SimpleNamespace(stdout="uid-gid-v1\n"), + ) + _preflight_agent_isolation_image("image:good") + + monkeypatch.setattr( + "coder_eval.isolation.docker_runner.subprocess.run", + lambda *args, **kwargs: SimpleNamespace(stdout="\n"), + ) + with pytest.raises(DockerRunError, match="does not declare"): + _preflight_agent_isolation_image("image:old") + + +def test_isolation_rejects_dynamic_privileged_criterion(tmp_path: Path) -> None: + task = TaskDefinition( + task_id="unsafe-grader", + description="test", + initial_prompt="work", + agent=ClaudeCodeAgentConfig(type=AgentKind.CLAUDE_CODE), + sandbox=SandboxConfig(driver="docker", docker=DockerDriverConfig(agent_isolation=True)), + success_criteria=[RunCommandCriterion(description="unsafe", command="python check.py")], + ) + rt = MagicMock(task=task, task_file=tmp_path / "task.yaml", run_dir=tmp_path / "run") + runner = DockerRunner(rt) + + with pytest.raises(RuntimeError, match="dynamic criteria"): + runner._validate_agent_isolation_compatibility() diff --git a/tests/test_docker_runner_mounts.py b/tests/test_docker_runner_mounts.py index 638d1b69..2788d9d3 100644 --- a/tests/test_docker_runner_mounts.py +++ b/tests/test_docker_runner_mounts.py @@ -4,8 +4,8 @@ omitted, rejection of destinations that shadow framework-owned mounts (``/work``, ``/``), and ``~`` / ``$VAR`` expansion on the source side. -Also covers user/output directory fixes: --user flag on POSIX, output -directory mounted to /work/output, and --output argument using container path. +Also covers protected output and agent-home mounts plus the container-side +``--output`` argument. """ from __future__ import annotations @@ -32,7 +32,7 @@ _sanitize_container_name_component, _validate_extra_mount, ) -from coder_eval.models import FileExistsCriterion, SandboxConfig, TaskDefinition +from coder_eval.models import AGENT_HOME, FileExistsCriterion, SandboxConfig, TaskDefinition # DockerRunner targets Linux containers from POSIX hosts. On Windows the test @@ -137,7 +137,7 @@ def _make_runner(self, run_dir: Path | None = None) -> DockerRunner: return DockerRunner(rt) def test_output_mounted_to_container_output_dir(self): - """Output directory should be mounted to CONTAINER_OUTPUT_DIR (/work/output).""" + """Output directory should be mounted below the protected grader root.""" runner = self._make_runner() with tempfile.TemporaryDirectory() as tmpdir: @@ -380,8 +380,8 @@ class TestClaudeHomeRWCopyMount: ``_prepare_host_mounts`` copies the host ``~/.claude`` (minus heavy per-session state) into a tmp dir and records it on ``_claude_mount_src``; ``_build_argv`` then mounts that copy read-WRITE at - the symmetric ``$HOME/.claude`` path. The old two-layer (``:ro`` parent + - ``session-env`` RW child) scheme is gone. + the dropped agent's dedicated ``/home/agent/.claude`` path. The old + symmetric-host-home and two-layer schemes are gone. """ def _make_runner(self) -> DockerRunner: @@ -432,7 +432,7 @@ def fake_home(self, tmp_path, monkeypatch): monkeypatch.delenv("CODER_EVAL_NO_CLAUDE_MOUNT", raising=False) return home - def test_rw_copy_mounted_at_symmetric_path(self, fake_home, tmp_path): + def test_rw_copy_mounted_at_agent_home(self, fake_home, tmp_path): runner = self._make_runner() staging = tmp_path / "staging" staging.mkdir() @@ -450,10 +450,11 @@ def test_rw_copy_mounted_at_symmetric_path(self, fake_home, tmp_path): mounts = self._volume_mounts(argv) host_claude = fake_home / ".claude" - # Exactly one claude mount: the copy → symmetric path, read-WRITE (no :ro). - assert f"{copy}:{host_claude}" in mounts - claude_mounts = [m for m in mounts if m.endswith(str(host_claude)) or f":{host_claude}" in m] - assert claude_mounts == [f"{copy}:{host_claude}"] + agent_claude = Path(AGENT_HOME) / ".claude" + # Exactly one Claude mount: disposable copy → agent HOME, read-WRITE. + assert f"{copy}:{agent_claude}" in mounts + claude_mounts = [m for m in mounts if m.endswith(str(agent_claude))] + assert claude_mounts == [f"{copy}:{agent_claude}"] # The retired two-layer scheme leaves no trace. assert f"{host_claude}:{host_claude}:ro" not in mounts assert not any("session-env" in m for m in mounts) @@ -674,4 +675,4 @@ def test_argv_reserved_workspace_raises(self): def test_container_paths_reexported_from_docker_runner(self): # Existing importers read CONTAINER_OUTPUT_DIR from docker_runner; keep that working. - assert CONTAINER_OUTPUT_DIR == "/work/output" + assert CONTAINER_OUTPUT_DIR == "/opt/coder-eval/grader/output" From 3c14b2448cca1f5a5d985546abed3345b2707980 Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Wed, 12 Aug 2026 11:26:41 +0300 Subject: [PATCH 2/7] feat(isolation): make docker agent isolation best-effort, downgrading to normal mode when unsupported --- docs/DOCKER_ISOLATION.md | 21 ++-- src/coder_eval/isolation/docker_runner.py | 113 +++++++++++----------- src/coder_eval/models/sandbox.py | 8 +- tests/test_docker_build_failure.py | 7 +- tests/test_docker_identity_isolation.py | 99 ++++++++++++++++--- 5 files changed, 163 insertions(+), 85 deletions(-) diff --git a/docs/DOCKER_ISOLATION.md b/docs/DOCKER_ISOLATION.md index 7f4c8064..c0b6b8e0 100644 --- a/docs/DOCKER_ISOLATION.md +++ b/docs/DOCKER_ISOLATION.md @@ -29,7 +29,7 @@ make docker-image-full Both build `coder-eval-agent:` and tag it `:latest`. -- **`make docker-image`** installs the core package plus the built-in agents. It needs **no credentials** and carries the `uid-gid-v1` isolation capability used by secure Docker runs. Static file/transcript criteria and `llm_judge` work in protected mode. Privileged dynamic criteria (`run_command`, `uipath_eval`, and `agent_judge`) currently fail closed; see [compatibility limits](#limitations). +- **`make docker-image`** installs the core package plus the built-in agents. It needs **no credentials** and carries the `uid-gid-v1` isolation capability used by protected Docker runs. All criterion types work in protected mode: static file/transcript criteria and `llm_judge` run as before, and dynamic criteria (`run_command`, `uipath_eval`, `agent_judge`) execute in the grader phase after the agent identity has been stopped; see [limitations](#limitations). - **`make docker-image-full`** additionally installs the `uipath` extra. The `uipath` SDK resolves from **public PyPI** (per `uv.lock`), so the build needs **no credentials**. Use this only for tasks that shell out to the in-host `uipath` CLI. (Codex is already in the default image — no extra needed.) > **Codex sandbox under Docker.** Codex's Landlock-backed `read-only` / `workspace-write` sandboxes can't initialize inside the eval container. The runner therefore uses Codex `full-access` inside the agent's own security domain. The boundary is the dedicated Linux agent UID, cleared capabilities, `no_new_privs`, and the protected harness paths—not Landlock and not a root agent process. @@ -38,9 +38,11 @@ Both build `coder-eval-agent:` and tag it `:latest`. `sandbox.docker.agent_isolation` defaults to `true`. The container harness and grader remain root, while every evaluated Claude, Codex, or Antigravity subprocess runs as `agent:agent` (`2000:2000`). +Isolation is **best-effort**: when a prerequisite is missing, the run downgrades to the normal single-identity container instead of failing. The downgrade conditions are an agent type without a verified UID-drop launch seam, an image without the `uid-gid-v1` capability label, `docker.working_dir`, and `docker.extra_mounts`. A downgraded run logs one WARNING line ("Running WITHOUT agent isolation") naming the reason — check the task log when the boundary matters. `agent_isolation: false` turns isolation off without a warning. + The agent launcher clears inheritable, ambient, and bounding capabilities and sets `no_new_privs`. Generated work is placed in `/work/agent`. Hidden task data, results, raw task/plugin/reference/template sources, and grader inputs live below root-only `/opt/coder-eval/grader`. Raw source bind mounts remain read-only and are never chmod/chowned; only disposable staging copies and the generated workspace are changed. -Older/custom images must declare `org.coder-eval.agent-isolation=uid-gid-v1`. A protected run rejects an image without that label before making an LLM call. Images derived with `FROM coder-eval-agent:` inherit it. Runtime-kit injection into an unrelated base does not yet provide the required Linux users and `setpriv` launchers, so it is not compatible with protected mode. +A protected run requires the image to declare `org.coder-eval.agent-isolation=uid-gid-v1`; an image without that label runs in normal mode with the downgrade warning. Images derived with `FROM coder-eval-agent:` inherit the label. Runtime-kit injection into an unrelated base does not yet provide the required Linux users and `setpriv` launchers, so it is not compatible with protected mode. ## Running a task in Docker @@ -140,11 +142,10 @@ sandbox: > **Protected-mode compatibility:** the current runtime kit does not install the > dedicated identities, `setpriv` launchers, protected directory layout, or the -> `org.coder-eval.agent-isolation=uid-gid-v1` capability label. Because -> `agent_isolation` defaults to `true`, an inject-mode image fails closed at -> preflight. For now, extend `coder-eval-agent:` for protected runs. -> Setting `agent_isolation: false` permits legacy runtime-kit migration but does -> not provide the boundary described on this page. +> `org.coder-eval.agent-isolation=uid-gid-v1` capability label. An inject-mode +> image therefore runs in normal mode with the downgrade warning. Extend +> `coder-eval-agent:` instead when the run needs the boundary +> described on this page. The `FROM coder-eval-agent` contract above means a task is **rebased** onto the Debian framework image. That breaks tasks whose Dockerfile was written for a @@ -302,7 +303,7 @@ The host's run dir is bind-mounted read-write at `/opt/coder-eval/grader/output` | Layer | Location | |---|---| | Agent process and descendants | container, UID/GID `2000:2000`, `/work/agent` | -| Harness + supported criterion checking | container, root, `/opt/coder-eval/grader` | +| Harness + criterion checking (static and dynamic) | container, root, `/opt/coder-eval/grader` | | **`task.json` serialization** | **container → host bind mount** | | Per-criterion `aggregate()` (P/R/F1, suite thresholds) | host | | Reports, run summary, experiment rollups | host | @@ -311,8 +312,8 @@ The host's run dir is bind-mounted read-write at `/opt/coder-eval/grader/output` ## Limitations -- **Dynamic privileged graders**: `run_command`, `uipath_eval`, and `agent_judge` are rejected in protected mode until a separate minimal-input grader sandbox exists. This prevents candidate-controlled code from turning a privileged grader into a confused deputy. Migrate to static built-in criteria or explicitly disable isolation only for a trusted transitional run. -- **Custom work directories and extra mounts**: protected mode currently rejects `docker.working_dir` and `docker.extra_mounts` because their agent/private audience is ambiguous. Use the generated `/work/agent` workspace and `template_sources`. +- **Dynamic criteria run with grader privileges**: `run_command`, `uipath_eval`, and `agent_judge` execute in the grader phase, as root, after every agent-identity process has been stopped and reaped. The boundary protects grading inputs from the *agent identity during the agent phase*; anything a dynamic criterion invokes (including code the agent wrote) runs with grader privileges and can read the grader mounts. Prefer static built-in criteria where the task allows it. +- **Custom work directories and extra mounts**: `docker.working_dir` and `docker.extra_mounts` have an ambiguous agent/private audience, so setting either downgrades the run to normal mode. Use the generated `/work/agent` workspace and `template_sources` to keep the boundary. - **Runtime-kit injection**: not yet compatible with protected mode. Extend the current framework image instead. - **No container reuse across tasks**: each task = one fresh container. Adds ~1–3 s startup overhead per task; negligible vs. LLM latency. - **macOS Keychain auth**: not reachable from the container; set `ANTHROPIC_API_KEY` (direct) or Bedrock credentials instead. diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index 791e28c7..a0eda8fe 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -260,8 +260,13 @@ def _preflight_image_version(image: str) -> None: ) -def _preflight_agent_isolation_image(image: str) -> None: - """Require an image that contains the declared UID/GID launch boundary.""" +def _image_supports_agent_isolation(image: str) -> bool: + """Whether the image declares the UID/GID launch boundary capability. + + Raises :class:`DockerRunError` only when the capability cannot be checked + at all (image missing, docker broken) -- an unlabeled image is a normal + "no support" answer, not an error. + """ try: result = subprocess.run( @@ -283,12 +288,7 @@ def _preflight_agent_isolation_image(image: str) -> None: raise DockerRunError( f"cannot verify UID/GID isolation support for image {image!r}; build or pull the image first" ) from exc - capability = result.stdout.strip() - if capability != "uid-gid-v1": - raise DockerRunError( - f"image {image!r} does not declare org.coder-eval.agent-isolation=uid-gid-v1; " - + "rebuild it from the latest coder-eval-agent image or disable isolation explicitly" - ) + return result.stdout.strip() == "uid-gid-v1" _CONTAINER_NAME_INVALID = re.compile(r"[^a-zA-Z0-9_.-]") @@ -524,6 +524,12 @@ def __init__( # Resolved in run() (needs the built image for "auto"). Concrete WORKDIR the # agent runs at + copies out from; None = standard artifacts workspace. self._workspace_dir: str | None = None + # Effective isolation for THIS run. Starts from config; run() downgrades + # it to False (with one warning) when a prerequisite is missing -- an + # unsupported agent type, an unlabeled image, working_dir, or + # extra_mounts. Every isolation-conditional site below keys on this + # resolved flag, never on the raw config value. + self._isolation_active: bool = self.rt.task.sandbox.docker.agent_isolation @property def _docker_config(self) -> DockerDriverConfig: @@ -542,7 +548,7 @@ async def run(self) -> EvaluationResult: dispatcher converts that to an ERROR-status EvaluationResult. """ _preflight() - self._validate_agent_isolation_compatibility() + self._resolve_agent_isolation() # Resolve the run image: build from a Dockerfile if configured (which # overrides `image`), else use the configured image. The build is # side-effecting, so it runs in a worker thread like the other docker @@ -561,8 +567,8 @@ async def run(self) -> EvaluationResult: # a task-supplied Dockerfile won't carry the org.coder-eval.version label. if not self._docker_config.dockerfile_path: await asyncio.to_thread(_preflight_image_version, image) - if self._docker_config.agent_isolation: - await asyncio.to_thread(_preflight_agent_isolation_image, image) + if self._isolation_active and not await asyncio.to_thread(_image_supports_agent_isolation, image): + self._downgrade_isolation(f"image {image!r} does not declare org.coder-eval.agent-isolation=uid-gid-v1") await asyncio.to_thread(self.rt.run_dir.mkdir, parents=True, exist_ok=True) # Docker WORKDIR alignment: resolve the concrete workspace path @@ -570,17 +576,9 @@ async def run(self) -> EvaluationResult: # /root). Forwarded to the in-container orchestrator via the staged context # and rendered as `docker run -w`. None keeps the standard artifacts workspace. self._workspace_dir = await asyncio.to_thread(_resolve_workspace_dir, self._docker_config.working_dir, image) - if self._docker_config.agent_isolation: - if self._workspace_dir is not None: - raise DockerRunError( - "docker.agent_isolation does not yet support docker.working_dir; " - + "use the default generated /work/agent workspace or disable isolation explicitly" - ) - if self._docker_config.extra_mounts: - raise DockerRunError( - "docker.agent_isolation rejects extra_mounts because their agent/private audience is ambiguous; " - + "stage the required input through template_sources or disable isolation explicitly" - ) + if self._isolation_active: + # working_dir triggered a downgrade in _resolve_agent_isolation, so + # the resolved workspace here is always the isolation-managed one. self._workspace_dir = CONTAINER_AGENT_WORK_DIR # Stage only the inputs (task YAML + context). The *output* dir is @@ -667,7 +665,7 @@ async def _stage_inputs(self, input_dir: Path) -> None: # container needs to see those mutations. task_yaml_in = input_dir / "task.yaml" task_payload = self.rt.task.model_dump(mode="json") - if self._docker_config.agent_isolation: + if self._isolation_active: await asyncio.to_thread(self._prepare_isolated_sources) task_payload = self._rewrite_task_paths(task_payload) @@ -695,10 +693,29 @@ def _dump_task_yaml() -> str: ) await asyncio.to_thread((input_dir / "context.json").write_text, context_payload, encoding="utf-8") - def _validate_agent_isolation_compatibility(self) -> None: - """Reject task features whose privileged behavior is not isolated yet.""" + def _downgrade_isolation(self, reason: str) -> None: + """Turn agent isolation off for this run and say so once, loudly.""" + + self._isolation_active = False + logger.warning( + "Agent isolation is unavailable for task '%s': %s. Running WITHOUT agent isolation " + + "(normal single-identity container).", + self.rt.task.task_id, + reason, + ) + + def _resolve_agent_isolation(self) -> None: + """Downgrade to a normal run when a config prerequisite is missing. - if not self._docker_config.agent_isolation: + Isolation is best-effort: a missing prerequisite runs the task in the + pre-isolation single-identity container instead of failing the run. + Dynamic criteria (``run_command``, ``uipath_eval``, ``agent_judge``) + never gate isolation -- they execute in the grader phase, after the + agent identity has been stopped and reaped. The image capability label + is checked separately in run() once the image is built. + """ + + if not self._isolation_active: return agent_type = str(self.rt.task.agent.type) if self.rt.task.agent and self.rt.task.agent.type else "" supported_agents = { @@ -708,27 +725,11 @@ def _validate_agent_isolation_compatibility(self) -> None: AgentKind.NONE.value, } if agent_type not in supported_agents: - raise DockerRunError( - f"docker.agent_isolation has no verified UID-drop launch seam for agent type {agent_type!r}" - ) - - # These criterion implementations can execute another agent or arbitrary - # task-authored commands in the privileged harness. If that execution - # imports candidate-controlled code, it can act as a confused deputy and - # publish hidden grader bytes. A separate minimal-input grader sandbox is - # required before they can run in protected mode. - unsupported_criteria = sorted( - { - criterion.type - for criterion in self.rt.task.success_criteria - if criterion.type in {"agent_judge", "run_command", "uipath_eval"} - } - ) - if unsupported_criteria: - raise DockerRunError( - "docker.agent_isolation rejects privileged dynamic criteria until they have a separate grader " - + f"sandbox: {unsupported_criteria}. Use static/built-in criteria or explicitly disable isolation." - ) + self._downgrade_isolation(f"no verified UID-drop launch seam for agent type {agent_type!r}") + elif self._docker_config.working_dir is not None: + self._downgrade_isolation("docker.working_dir has no UID-drop support yet") + elif self._docker_config.extra_mounts: + self._downgrade_isolation("extra_mounts have an ambiguous agent/private audience") def _prepare_isolated_sources(self) -> None: """Prepare private raw-source mount mappings. @@ -1276,7 +1277,7 @@ def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirr argv += ["--cpus", str(self._limits.max_cpus)] if self._limits.max_pids is not None: argv += ["--pids-limit", str(self._limits.max_pids)] - elif cfg.agent_isolation: + elif self._isolation_active: argv += ["--pids-limit", str(DEFAULT_AGENT_ISOLATION_MAX_PIDS)] # Forward environment variables: explicit allowlist (optionally extended via env_passthrough_extra). @@ -1294,12 +1295,12 @@ def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirr # rewrite (host alias / absolute mount path), not name-only. if env_var in ("LITELLM_BASE_URL", "LITELLM_COST_LOG", "SKILLS_REPO_PATH"): continue - if cfg.agent_isolation and env_var == "HOME": + if self._isolation_active and env_var == "HOME": continue if env_var in os.environ: argv += ["--env", env_var] - if cfg.agent_isolation and (skills_repo := os.environ.get("SKILLS_REPO_PATH")): + if self._isolation_active and (skills_repo := os.environ.get("SKILLS_REPO_PATH")): resolved_skills = str(Path(skills_repo).expanduser().resolve()) private_skills = self._host_to_private_paths.get(resolved_skills) if private_skills is not None: @@ -1340,7 +1341,7 @@ def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirr # sandbox: Codex's Landlock-backed read-only / workspace-write sandboxes # can't initialize inside a container and otherwise fail writes silently. argv += ["--env", "CODER_EVAL_IN_CONTAINER=1"] - if cfg.agent_isolation: + if self._isolation_active: argv += ["--env", "CODER_EVAL_AGENT_ISOLATION=1"] # Hard-disable telemetry INSIDE the container. The app ships a baked-in @@ -1375,7 +1376,7 @@ def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirr # doesn't exist or the mount is opted out (CODER_EVAL_NO_CLAUDE_MOUNT=1). if self._claude_mount_src is not None: host_claude_dir = Path.home() / ".claude" - claude_target = Path(AGENT_HOME) / ".claude" if cfg.agent_isolation else host_claude_dir + claude_target = Path(AGENT_HOME) / ".claude" if self._isolation_active else host_claude_dir argv += ["-v", f"{self._claude_mount_src}:{claude_target}"] for source, target in self._private_source_mounts: @@ -1427,7 +1428,7 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: from coder_eval.models import TemplateDirSource sandbox_cfg = self.rt.task.sandbox - if not cfg.agent_isolation: + if not self._isolation_active: for source in (sandbox_cfg.template_sources or []) if sandbox_cfg else []: if isinstance(source, TemplateDirSource): _auto_mount(source.path) @@ -1437,7 +1438,7 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: # could conceivably inject an absolute path that survives. Cover # that path so the in-container Orchestrator can read it. agent_cfg = self.rt.task.agent - if not cfg.agent_isolation and agent_cfg and agent_cfg.system_prompt_file: + if not self._isolation_active and agent_cfg and agent_cfg.system_prompt_file: _auto_mount(agent_cfg.system_prompt_file, dir_only=False) # reference.file / reference.directory: if a task ships absolute @@ -1445,7 +1446,7 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: # ``..``), they must be mounted explicitly. Relative paths under # task_dir are already covered by the symmetric task_dir mount. reference = self.rt.task.reference - if not cfg.agent_isolation and reference is not None: + if not self._isolation_active and reference is not None: _auto_mount(reference.file, dir_only=False) _auto_mount(reference.directory) for mount in cfg.extra_mounts: @@ -1461,7 +1462,7 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: # (assigned above, not task-authored), and that path is deliberately # in RESERVED_CONTAINER_DIRS. The assertion guards task/image-supplied # values, so exempt exactly the isolation-managed constant. - if not (cfg.agent_isolation and self._workspace_dir == CONTAINER_AGENT_WORK_DIR): + if not (self._isolation_active and self._workspace_dir == CONTAINER_AGENT_WORK_DIR): _assert_workspace_not_reserved(self._workspace_dir) argv += ["-w", self._workspace_dir] diff --git a/src/coder_eval/models/sandbox.py b/src/coder_eval/models/sandbox.py index d23e547d..3fdeb102 100644 --- a/src/coder_eval/models/sandbox.py +++ b/src/coder_eval/models/sandbox.py @@ -198,9 +198,11 @@ class DockerDriverConfig(BaseModel): agent_isolation: bool = Field( default=True, description=( - "Run the evaluated agent under the image's dedicated unprivileged UID/GID and expose local plugins " - "only through manifest-verified bundles. Enabled by default. Set false only for temporary migration " - "of a trusted task; false is not a secure evaluation boundary." + "Run the evaluated agent under the image's dedicated unprivileged UID/GID, keeping grading material " + "out of reach of the agent identity. Best-effort: when a prerequisite is missing (agent type without " + "a UID-drop launch seam, an image without the isolation capability label, working_dir, or " + "extra_mounts), the run downgrades to the normal single-identity container with a warning instead of " + "failing. Set false to turn isolation off entirely; false is not a secure evaluation boundary." ), ) working_dir: str | None = Field( diff --git a/tests/test_docker_build_failure.py b/tests/test_docker_build_failure.py index 31c7d024..bb28f46b 100644 --- a/tests/test_docker_build_failure.py +++ b/tests/test_docker_build_failure.py @@ -33,10 +33,9 @@ def _make_runner(run_dir: Path) -> DockerRunner: task_id="suri", description="t", initial_prompt="do", - # A concrete agent type is required now that docker.agent_isolation - # defaults to true: the isolation gate rejects a type with no verified - # UID-drop launch seam before the build runs, and this test covers - # build-failure observability rather than that gate. + # A concrete agent type keeps docker.agent_isolation (default true) + # active instead of downgrading with a warning before the build runs; + # this test covers build-failure observability, not that resolution. agent={"type": "claude-code"}, sandbox=SandboxConfig(driver="docker", docker=DockerDriverConfig(image="x:1", dockerfile_path="/df")), success_criteria=[FileExistsCriterion(description="c", path="out.txt")], diff --git a/tests/test_docker_identity_isolation.py b/tests/test_docker_identity_isolation.py index 60a94a6a..903a7b05 100644 --- a/tests/test_docker_identity_isolation.py +++ b/tests/test_docker_identity_isolation.py @@ -2,7 +2,9 @@ from __future__ import annotations +import logging import signal +import subprocess from pathlib import Path from types import SimpleNamespace from unittest.mock import MagicMock @@ -10,7 +12,7 @@ import pytest from coder_eval.agents.codex_agent import CodexAgent -from coder_eval.isolation.docker_runner import DockerRunError, DockerRunner, _preflight_agent_isolation_image +from coder_eval.isolation.docker_runner import DockerRunError, DockerRunner, _image_supports_agent_isolation from coder_eval.models import ( AGENT_GID, AGENT_HOME, @@ -130,32 +132,105 @@ def test_agent_teardown_rescans_until_uid_has_no_processes(monkeypatch: pytest.M assert signals == [(41, signal.SIGTERM), (42, signal.SIGTERM), (42, expected_kill)] -def test_isolation_image_label_preflight_accepts_only_declared_capability(monkeypatch: pytest.MonkeyPatch) -> None: +def test_isolation_image_capability_check_reports_support_without_failing(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "coder_eval.isolation.docker_runner.subprocess.run", lambda *args, **kwargs: SimpleNamespace(stdout="uid-gid-v1\n"), ) - _preflight_agent_isolation_image("image:good") + assert _image_supports_agent_isolation("image:good") is True monkeypatch.setattr( "coder_eval.isolation.docker_runner.subprocess.run", lambda *args, **kwargs: SimpleNamespace(stdout="\n"), ) - with pytest.raises(DockerRunError, match="does not declare"): - _preflight_agent_isolation_image("image:old") + assert _image_supports_agent_isolation("image:old") is False + def _inspect_fails(*args: object, **kwargs: object) -> SimpleNamespace: + raise subprocess.CalledProcessError(1, "docker") + + monkeypatch.setattr("coder_eval.isolation.docker_runner.subprocess.run", _inspect_fails) + with pytest.raises(DockerRunError, match="cannot verify"): + _image_supports_agent_isolation("image:missing") + + +def _make_runner(tmp_path: Path, task: object) -> DockerRunner: + return DockerRunner(MagicMock(task=task, task_file=tmp_path / "task.yaml", run_dir=tmp_path / "run")) + + +def test_isolation_stays_active_with_dynamic_criteria(tmp_path: Path) -> None: + """run_command / uipath_eval / agent_judge execute in the grader phase and never gate isolation.""" -def test_isolation_rejects_dynamic_privileged_criterion(tmp_path: Path) -> None: task = TaskDefinition( - task_id="unsafe-grader", + task_id="dynamic-grader", description="test", initial_prompt="work", agent=ClaudeCodeAgentConfig(type=AgentKind.CLAUDE_CODE), sandbox=SandboxConfig(driver="docker", docker=DockerDriverConfig(agent_isolation=True)), - success_criteria=[RunCommandCriterion(description="unsafe", command="python check.py")], + success_criteria=[RunCommandCriterion(description="dynamic", command="python check.py")], + ) + runner = _make_runner(tmp_path, task) + + runner._resolve_agent_isolation() + + assert runner._isolation_active is True + + +def test_isolation_downgrades_for_unsupported_agent_type(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + task = SimpleNamespace( + task_id="mystery-agent", + agent=SimpleNamespace(type="mystery"), + sandbox=SimpleNamespace(docker=SimpleNamespace(agent_isolation=True, working_dir=None, extra_mounts=[])), + ) + runner = _make_runner(tmp_path, task) + + with caplog.at_level(logging.WARNING): + runner._resolve_agent_isolation() + + assert runner._isolation_active is False + assert "WITHOUT agent isolation" in caplog.text + + +@pytest.mark.parametrize( + ("docker_kwargs", "reason_fragment"), + [ + ({"working_dir": "/app"}, "working_dir"), + ({"extra_mounts": ["/host/data:/data:ro"]}, "extra_mounts"), + ], +) +def test_isolation_downgrades_for_unsupported_docker_config( + tmp_path: Path, + caplog: pytest.LogCaptureFixture, + docker_kwargs: dict[str, object], + reason_fragment: str, +) -> None: + task = TaskDefinition( + task_id="legacy-config", + description="test", + initial_prompt="work", + agent=ClaudeCodeAgentConfig(type=AgentKind.CLAUDE_CODE), + sandbox=SandboxConfig(driver="docker", docker=DockerDriverConfig(agent_isolation=True, **docker_kwargs)), + success_criteria=[RunCommandCriterion(description="dynamic", command="python check.py")], ) - rt = MagicMock(task=task, task_file=tmp_path / "task.yaml", run_dir=tmp_path / "run") - runner = DockerRunner(rt) + runner = _make_runner(tmp_path, task) + + with caplog.at_level(logging.WARNING): + runner._resolve_agent_isolation() + + assert runner._isolation_active is False + assert reason_fragment in caplog.text + + +def test_explicitly_disabled_isolation_stays_disabled(tmp_path: Path) -> None: + task = TaskDefinition( + task_id="opt-out", + description="test", + initial_prompt="work", + agent=ClaudeCodeAgentConfig(type=AgentKind.CLAUDE_CODE), + sandbox=SandboxConfig(driver="docker", docker=DockerDriverConfig(agent_isolation=False)), + success_criteria=[RunCommandCriterion(description="dynamic", command="python check.py")], + ) + runner = _make_runner(tmp_path, task) + + runner._resolve_agent_isolation() - with pytest.raises(RuntimeError, match="dynamic criteria"): - runner._validate_agent_isolation_compatibility() + assert runner._isolation_active is False From 676679285684067e2f8b70ff5701103b3d3292d1 Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Wed, 12 Aug 2026 15:10:50 +0300 Subject: [PATCH 3/7] fix(isolation): forward SKILLS_REPO_PATH, keep agent_judge privileged, harden downgrade and reap paths --- docs/DOCKER_ISOLATION.md | 4 +- src/coder_eval/agents/antigravity_agent.py | 8 +- src/coder_eval/agents/claude_code_agent.py | 14 +- src/coder_eval/evaluation/sub_agent.py | 5 + src/coder_eval/isolation/agent_identity.py | 7 + src/coder_eval/isolation/docker_runner.py | 70 +++++--- src/coder_eval/models/sandbox.py | 7 +- tests/test_docker_identity_isolation.py | 192 +++++++++++++++++++++ 8 files changed, 276 insertions(+), 31 deletions(-) diff --git a/docs/DOCKER_ISOLATION.md b/docs/DOCKER_ISOLATION.md index c0b6b8e0..5e67d41e 100644 --- a/docs/DOCKER_ISOLATION.md +++ b/docs/DOCKER_ISOLATION.md @@ -38,7 +38,9 @@ Both build `coder-eval-agent:` and tag it `:latest`. `sandbox.docker.agent_isolation` defaults to `true`. The container harness and grader remain root, while every evaluated Claude, Codex, or Antigravity subprocess runs as `agent:agent` (`2000:2000`). -Isolation is **best-effort**: when a prerequisite is missing, the run downgrades to the normal single-identity container instead of failing. The downgrade conditions are an agent type without a verified UID-drop launch seam, an image without the `uid-gid-v1` capability label, `docker.working_dir`, and `docker.extra_mounts`. A downgraded run logs one WARNING line ("Running WITHOUT agent isolation") naming the reason — check the task log when the boundary matters. `agent_isolation: false` turns isolation off without a warning. +Isolation is **best-effort**: when a prerequisite is missing, the run downgrades to the normal single-identity container instead of failing. The downgrade conditions are an agent type without a verified UID-drop launch seam, an image without the `uid-gid-v1` capability label, `docker.working_dir`, `docker.extra_mounts`, and an `agent.system_prompt_file` that survived resolution. A downgraded run logs one WARNING line ("Running WITHOUT agent isolation") naming the reason — check the task log when the boundary matters. `agent_isolation: false` turns isolation off without a warning. + +The `agent_judge` criterion's sub-agent is a trusted grader, not an evaluated agent: it runs with evaluator privileges (no UID drop) after the evaluated agent's identity has been stopped and reaped. The agent launcher clears inheritable, ambient, and bounding capabilities and sets `no_new_privs`. Generated work is placed in `/work/agent`. Hidden task data, results, raw task/plugin/reference/template sources, and grader inputs live below root-only `/opt/coder-eval/grader`. Raw source bind mounts remain read-only and are never chmod/chowned; only disposable staging copies and the generated workspace are changed. diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 49ef119c..38c196e1 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -420,6 +420,10 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: PATH window, or its harness would inherit another task's mock dirs. """ async with _harness_spawn_lock(): + # Latch the flag BEFORE the scrub: the pop below removes + # CODER_EVAL_AGENT_ISOLATION itself, so a later + # agent_isolation_enabled() call would always read False. + isolation_active = agent_isolation_enabled() scrubbed = { name: os.environ.pop(name) for name in list(os.environ) @@ -431,7 +435,7 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: if self._env_path_prepend: os.environ[path_key] = os.pathsep.join([*self._env_path_prepend, original_path or ""]) self._log.debug("PATH prepend for harness spawn: %s", os.pathsep.join(self._env_path_prepend)) - if agent_isolation_enabled(): + if isolation_active: os.environ["HOME"] = AGENT_HOME try: yield @@ -442,7 +446,7 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: os.environ.pop(path_key, None) else: os.environ[path_key] = original_path - if agent_isolation_enabled(): + if isolation_active: if original_home is None: os.environ.pop("HOME", None) else: diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index de678f72..c14c0460 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -671,6 +671,7 @@ def __init__( instance_name: str = "coder", extra_mcp_servers: dict[str, Any] | None = None, cost_log_tags: dict[str, str] | None = None, + isolation_exempt: bool = False, ): """Initialize the Claude Code agent. @@ -693,9 +694,16 @@ def __init__( per turn) stamped into ``ANTHROPIC_CUSTOM_HEADERS`` so a proxy-side cost callback can attribute each call's real cost back to this run. None on Direct/Bedrock. + isolation_exempt: True only for trusted grader-phase instances + (the agent_judge sub-agent). Skips the UID-drop CLI shim and + the agent-HOME redirect that the container's isolation env + flag otherwise applies, so the judge keeps evaluator + privileges over its root-owned sandbox copy. Never expose + this to task YAML. """ self.config = config self.route = route or DirectRoute() + self._isolation_exempt = isolation_exempt self._extra_mcp_servers = extra_mcp_servers or {} # Correlation headers stamped on every SDK->proxy request (LiteLLM route # only), so a proxy-side cost-logging callback can join each call's real @@ -749,6 +757,7 @@ def _build_sdk_env( path_prepend: list[str] | None = None, plugin_tools_dir: str | None = None, cost_log_tags: dict[str, str] | None = None, + isolation_exempt: bool = False, ) -> tuple[dict[str, str], str | None]: """Build SDK environment variables and resolve effective model for the given route. @@ -771,7 +780,7 @@ def _build_sdk_env( base_env: dict[str, str] = scrub_agent_env_overrides() if path := os.environ.get("PATH"): base_env["PATH"] = path - if agent_isolation_enabled(): + if agent_isolation_enabled() and not isolation_exempt: base_env["HOME"] = AGENT_HOME if path_prepend: @@ -1170,6 +1179,7 @@ def _build_claude_query( path_prepend=self._env_path_prepend, plugin_tools_dir=self._plugin_tools_dir, cost_log_tags=cost_log_tags, + isolation_exempt=self._isolation_exempt, ) effective_model = self._resolve_effective_model(self.config.model, env, route_model) @@ -1207,7 +1217,7 @@ def _build_claude_query( # The SDK accepts a single CLI executable path. The baked wrapper # invokes the real Claude binary through the same setpriv policy as # the other backends (UID/GID drop, no capabilities, no_new_privs). - cli_path=CONTAINER_CLAUDE_SHIM if agent_isolation_enabled() else None, + cli_path=(CONTAINER_CLAUDE_SHIM if agent_isolation_enabled() and not self._isolation_exempt else None), **self.config.sdk_options, ) diff --git a/src/coder_eval/evaluation/sub_agent.py b/src/coder_eval/evaluation/sub_agent.py index 88d1bc2e..94fd19eb 100644 --- a/src/coder_eval/evaluation/sub_agent.py +++ b/src/coder_eval/evaluation/sub_agent.py @@ -203,6 +203,11 @@ async def run_async(self, user_msg: str, *, max_turns: int | None, turn_timeout: self._agent_config, route=self._route, extra_mcp_servers=self._extra_mcp_servers, + # The judge is a trusted grader: it must keep evaluator + # privileges (no UID-drop shim, no agent-HOME redirect) so it + # can read the root-owned sandbox copy it grades. It runs + # after the evaluated agent's identity has been reaped. + isolation_exempt=True, ) logger.info( "sub_agent: starting (model=%s, max_turns=%s, allowed_tools=%s)", diff --git a/src/coder_eval/isolation/agent_identity.py b/src/coder_eval/isolation/agent_identity.py index db0dbb95..bd372000 100644 --- a/src/coder_eval/isolation/agent_identity.py +++ b/src/coder_eval/isolation/agent_identity.py @@ -80,6 +80,13 @@ def _agent_pids() -> list[int]: uids = [int(value) for value in uid_line.split()[1:]] except (OSError, StopIteration, ValueError): continue + state_line = next((line for line in status_lines if line.startswith("State:")), "") + if state_line.split()[1:2] == ["Z"]: + # A zombie keeps its Uid: line until reaped, cannot act, and + # SIGKILL cannot remove it -- the container's --init reaper (or + # the parent) collects it. Counting it would fail finalization + # over a process that is already dead. + continue if AGENT_UID in uids: pids.append(int(entry.name)) return pids diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index a0eda8fe..53e7944b 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -730,6 +730,11 @@ def _resolve_agent_isolation(self) -> None: self._downgrade_isolation("docker.working_dir has no UID-drop support yet") elif self._docker_config.extra_mounts: self._downgrade_isolation("extra_mounts have an ambiguous agent/private audience") + elif self.rt.task.agent and self.rt.task.agent.system_prompt_file: + # Normally inlined into system_prompt by load_task / experiment + # resolution; a surviving path is the defensive case the + # non-isolated branch handles by auto-mounting it. + self._downgrade_isolation("system_prompt_file was not resolved to inline system_prompt") def _prepare_isolated_sources(self) -> None: """Prepare private raw-source mount mappings. @@ -745,12 +750,10 @@ def _prepare_isolated_sources(self) -> None: task_dir = self.rt.task_file.parent.resolve() if self.rt.task_file else None if task_dir is not None: self._host_to_private_paths[str(task_dir)] = CONTAINER_TASK_DIR - - if self.rt.task.agent and self.rt.task.agent.system_prompt_file: - raise DockerRunError( - "docker.agent_isolation requires system_prompt_file to be resolved to inline system_prompt " - + "before container staging" - ) + # Alias the unresolved textual form too: staged-YAML strings may + # carry a symlinked prefix (macOS /tmp -> /private/tmp) that the + # resolved key would never match. + self._host_to_private_paths.setdefault(str(self.rt.task_file.parent), CONTAINER_TASK_DIR) from coder_eval.models import TemplateDirSource @@ -758,8 +761,8 @@ def _prepare_isolated_sources(self) -> None: for source in self.rt.task.sandbox.template_sources or []: if not isinstance(source, TemplateDirSource): continue - host_path = Path(source.path).resolve() - if task_dir is not None and (host_path == task_dir or task_dir in host_path.parents): + host_path = Path(source.path) + if task_dir is not None and (host_path.resolve() == task_dir or task_dir in host_path.resolve().parents): continue self._register_private_mount(host_path, f"/opt/coder-eval/grader/templates/source-{template_index}") template_index += 1 @@ -771,24 +774,29 @@ def _prepare_isolated_sources(self) -> None: Path(reference.directory), "/opt/coder-eval/grader/references/directory" ) if reference.file: - reference_file = Path(reference.file).resolve() self._register_external_private_path( - reference_file.parent, "/opt/coder-eval/grader/references/file-parent" + Path(reference.file).parent, "/opt/coder-eval/grader/references/file-parent" ) def _register_external_private_path(self, source: Path, target: str) -> None: - source = source.resolve() task_dir = self.rt.task_file.parent.resolve() if self.rt.task_file else None - if task_dir is not None and (source == task_dir or task_dir in source.parents): + resolved = source.resolve() + if task_dir is not None and (resolved == task_dir or task_dir in resolved.parents): return self._register_private_mount(source, target) def _register_private_mount(self, source: Path, target: str) -> None: + # The rewrite map keys on BOTH the resolved and the raw textual form: + # staged-YAML strings usually match the raw form, while the mount and + # dedup use the resolved one (symlinked prefixes differ between them). + raw_key = str(source) source = source.resolve() key = str(source) if key in self._host_to_private_paths: return self._host_to_private_paths[key] = target + if raw_key != key: + self._host_to_private_paths.setdefault(raw_key, target) self._private_source_mounts.append((source, target)) def _rewrite_task_paths(self, payload: dict[str, object]) -> dict[str, object]: @@ -1289,24 +1297,33 @@ def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirr # here exactly like every other allowlisted var. A flag that only mutated in-process # Settings would be dropped at the container boundary and the in-container Settings # would silently default to DIRECT — downgrading the judge (and agent) route. + # SKILLS_REPO_PATH forwards name-only like any other allowlisted var, + # EXCEPT when isolation privately staged the checkout (template source): + # then the value is rewritten to the staged container path below. In a + # downgraded or plugin-path run the host value passes through, so the + # in-container orchestrator can expand $SKILLS_REPO_PATH plugin paths + # against the auto-mounted host-path plugin dir. The agent subprocess + # itself never sees the variable (scrub_agent_env_overrides). + skills_repo_private: str | None = None + if self._isolation_active and (skills_repo := os.environ.get("SKILLS_REPO_PATH")): + resolved_skills = str(Path(skills_repo).expanduser().resolve()) + skills_repo_private = self._host_to_private_paths.get(resolved_skills) + merged_allowlist = set(cfg.env_passthrough) | set(cfg.env_passthrough_extra) for env_var in merged_allowlist: # LITELLM_BASE_URL / LITELLM_COST_LOG are forwarded below with a value # rewrite (host alias / absolute mount path), not name-only. - if env_var in ("LITELLM_BASE_URL", "LITELLM_COST_LOG", "SKILLS_REPO_PATH"): + if env_var in ("LITELLM_BASE_URL", "LITELLM_COST_LOG"): + continue + if env_var == "SKILLS_REPO_PATH" and skills_repo_private is not None: continue if self._isolation_active and env_var == "HOME": continue if env_var in os.environ: argv += ["--env", env_var] - if self._isolation_active and (skills_repo := os.environ.get("SKILLS_REPO_PATH")): - resolved_skills = str(Path(skills_repo).expanduser().resolve()) - private_skills = self._host_to_private_paths.get(resolved_skills) - if private_skills is not None: - argv += ["--env", f"SKILLS_REPO_PATH={private_skills}"] - else: - logger.debug("Not forwarding unstaged SKILLS_REPO_PATH into protected container: %s", resolved_skills) + if skills_repo_private is not None: + argv += ["--env", f"SKILLS_REPO_PATH={skills_repo_private}"] # LITELLM_BASE_URL points at a proxy on the HOST. A bridge-network container # can't reach the host's loopback, so rewrite localhost/127.0.0.1 to the @@ -1365,7 +1382,12 @@ def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirr host_task_dir: Path | None = None if self.rt.task_file: host_task_dir = self.rt.task_file.parent.resolve() - argv += ["-v", f"{host_task_dir}:{CONTAINER_TASK_DIR}:ro"] + # Isolated runs relocate the task dir below the root-only grader + # tree (the staged YAML is path-rewritten to match). Non-isolated + # runs keep the pre-isolation symmetric host-path mount, because + # their staged YAML still carries the host paths verbatim. + task_dir_target = CONTAINER_TASK_DIR if self._isolation_active else host_task_dir + argv += ["-v", f"{host_task_dir}:{task_dir_target}:ro"] # Forward the host's Claude Code OAuth state so the in-container CLI # inherits the same login as the host. We mount a *throwaway lean copy* # of ~/.claude (made by _prepare_host_mounts) read-WRITE at the host's @@ -1376,7 +1398,9 @@ def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirr # doesn't exist or the mount is opted out (CODER_EVAL_NO_CLAUDE_MOUNT=1). if self._claude_mount_src is not None: host_claude_dir = Path.home() / ".claude" - claude_target = Path(AGENT_HOME) / ".claude" if self._isolation_active else host_claude_dir + # Container-side path: keep it a plain POSIX string. Path() would + # render backslashes on a Windows host and break `docker run -v`. + claude_target = f"{AGENT_HOME}/.claude" if self._isolation_active else str(host_claude_dir) argv += ["-v", f"{self._claude_mount_src}:{claude_target}"] for source, target in self._private_source_mounts: @@ -1473,7 +1497,7 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: argv += ["-v"] argv += ["--output", str(CONTAINER_OUTPUT_DIR)] if host_task_dir is not None: - argv += ["--task-dir", str(CONTAINER_TASK_DIR)] + argv += ["--task-dir", str(CONTAINER_TASK_DIR if self._isolation_active else host_task_dir)] return argv diff --git a/src/coder_eval/models/sandbox.py b/src/coder_eval/models/sandbox.py index 3fdeb102..611ba666 100644 --- a/src/coder_eval/models/sandbox.py +++ b/src/coder_eval/models/sandbox.py @@ -200,9 +200,10 @@ class DockerDriverConfig(BaseModel): description=( "Run the evaluated agent under the image's dedicated unprivileged UID/GID, keeping grading material " "out of reach of the agent identity. Best-effort: when a prerequisite is missing (agent type without " - "a UID-drop launch seam, an image without the isolation capability label, working_dir, or " - "extra_mounts), the run downgrades to the normal single-identity container with a warning instead of " - "failing. Set false to turn isolation off entirely; false is not a secure evaluation boundary." + "a UID-drop launch seam, an image without the isolation capability label, working_dir, extra_mounts, " + "or an unresolved system_prompt_file), the run downgrades to the normal single-identity container " + "with a warning instead of failing. Set false to turn isolation off entirely; false is not a secure " + "evaluation boundary." ), ) working_dir: str | None = Field( diff --git a/tests/test_docker_identity_isolation.py b/tests/test_docker_identity_isolation.py index 903a7b05..cbe1aa15 100644 --- a/tests/test_docker_identity_isolation.py +++ b/tests/test_docker_identity_isolation.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging +import os import signal import subprocess from pathlib import Path @@ -220,6 +221,197 @@ def test_isolation_downgrades_for_unsupported_docker_config( assert reason_fragment in caplog.text +def test_isolation_downgrades_for_unresolved_system_prompt_file( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + task = TaskDefinition( + task_id="stale-prompt-file", + description="test", + initial_prompt="work", + agent=ClaudeCodeAgentConfig(type=AgentKind.CLAUDE_CODE, system_prompt_file="/abs/prompt.md"), + sandbox=SandboxConfig(driver="docker", docker=DockerDriverConfig(agent_isolation=True)), + success_criteria=[RunCommandCriterion(description="dynamic", command="python check.py")], + ) + runner = _make_runner(tmp_path, task) + + with caplog.at_level(logging.WARNING): + runner._resolve_agent_isolation() + + assert runner._isolation_active is False + assert "system_prompt_file" in caplog.text + + +def _make_argv_runner( + tmp_path: Path, + *, + agent_isolation: bool, + env_passthrough_extra: list[str] | None = None, + template_sources: list[object] | None = None, +) -> DockerRunner: + task = TaskDefinition( + task_id="argv-probe", + description="test", + initial_prompt="work", + agent=ClaudeCodeAgentConfig(type=AgentKind.CLAUDE_CODE), + sandbox=SandboxConfig( + driver="docker", + template_sources=template_sources, + docker=DockerDriverConfig( + agent_isolation=agent_isolation, + env_passthrough_extra=env_passthrough_extra or [], + ), + ), + success_criteria=[RunCommandCriterion(description="dynamic", command="python check.py")], + ) + task_dir = tmp_path / "task" + task_dir.mkdir(exist_ok=True) + rt = MagicMock(task=task, task_file=task_dir / "task.yaml", run_dir=tmp_path / "run") + return DockerRunner(rt) + + +def _argv(runner: DockerRunner, tmp_path: Path) -> list[str]: + return runner._build_argv(tmp_path / "in", tmp_path / "out", container_name="probe", image="img:1") + + +def test_skills_repo_path_forwards_name_only_without_isolation(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A downgraded / non-isolated run forwards the allowlisted var like any other.""" + + monkeypatch.setenv("SKILLS_REPO_PATH", str(tmp_path / "skills")) + runner = _make_argv_runner(tmp_path, agent_isolation=False, env_passthrough_extra=["SKILLS_REPO_PATH"]) + + argv = _argv(runner, tmp_path) + + assert "SKILLS_REPO_PATH" in argv + assert not [a for a in argv if a.startswith("SKILLS_REPO_PATH=")] + + +def test_skills_repo_path_forwards_name_only_when_isolated_but_unstaged( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Isolated run without a staged skills checkout (plugin-path pattern): host value passes through.""" + + monkeypatch.setenv("SKILLS_REPO_PATH", str(tmp_path / "skills")) + runner = _make_argv_runner(tmp_path, agent_isolation=True, env_passthrough_extra=["SKILLS_REPO_PATH"]) + runner._prepare_isolated_sources() + + argv = _argv(runner, tmp_path) + + assert "SKILLS_REPO_PATH" in argv + + +def test_skills_repo_path_value_rewritten_when_staged(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + from coder_eval.models import TemplateDirSource + + skills_dir = tmp_path / "skills" + skills_dir.mkdir() + monkeypatch.setenv("SKILLS_REPO_PATH", str(skills_dir)) + runner = _make_argv_runner( + tmp_path, + agent_isolation=True, + env_passthrough_extra=["SKILLS_REPO_PATH"], + template_sources=[TemplateDirSource(type="template_dir", path=str(skills_dir))], + ) + runner._prepare_isolated_sources() + + argv = _argv(runner, tmp_path) + + assert "SKILLS_REPO_PATH=/opt/coder-eval/grader/templates/source-0" in argv + assert "SKILLS_REPO_PATH" not in argv # name-only form must not double-forward + + +def test_claude_mount_target_is_posix_under_isolation(tmp_path: Path) -> None: + runner = _make_argv_runner(tmp_path, agent_isolation=True) + runner._claude_mount_src = tmp_path / "claude-copy" + + argv = _argv(runner, tmp_path) + + claude_mounts = [a for a in argv if a.endswith("/.claude")] + assert claude_mounts and claude_mounts[0].endswith(f":{AGENT_HOME}/.claude") + assert "\\" not in claude_mounts[0].split(":")[-1] + + +def test_task_dir_mount_stays_symmetric_without_isolation(tmp_path: Path) -> None: + runner = _make_argv_runner(tmp_path, agent_isolation=False) + host_task_dir = runner.rt.task_file.parent.resolve() + + argv = _argv(runner, tmp_path) + + assert f"{host_task_dir}:{host_task_dir}:ro" in argv + assert argv[argv.index("--task-dir") + 1] == str(host_task_dir) + + +def test_task_dir_mount_relocated_under_isolation(tmp_path: Path) -> None: + runner = _make_argv_runner(tmp_path, agent_isolation=True) + runner._prepare_isolated_sources() + host_task_dir = runner.rt.task_file.parent.resolve() + + argv = _argv(runner, tmp_path) + + assert f"{host_task_dir}:/opt/coder-eval/grader/task_dir:ro" in argv + assert argv[argv.index("--task-dir") + 1] == "/opt/coder-eval/grader/task_dir" + + +def test_build_sdk_env_isolation_exempt_keeps_evaluator_home(monkeypatch: pytest.MonkeyPatch) -> None: + from coder_eval.agents.claude_code_agent import ClaudeCodeAgent + from coder_eval.models import DirectRoute + + monkeypatch.setenv("CODER_EVAL_AGENT_ISOLATION", "1") + + env_dropped, _ = ClaudeCodeAgent._build_sdk_env(DirectRoute()) + env_exempt, _ = ClaudeCodeAgent._build_sdk_env(DirectRoute(), isolation_exempt=True) + + assert env_dropped["HOME"] == AGENT_HOME + assert env_exempt.get("HOME") != AGENT_HOME + + +def test_agent_pids_skips_zombie_processes(monkeypatch: pytest.MonkeyPatch) -> None: + from coder_eval.isolation import agent_identity + + class FakeStatus: + def __init__(self, text: str) -> None: + self._text = text + + def read_text(self, encoding: str = "utf-8") -> str: + return self._text + + class FakeEntry: + def __init__(self, pid: int, uid: int, state: str) -> None: + self.name = str(pid) + self._status = FakeStatus(f"Name:\tx\nState:\t{state} (state)\nUid:\t{uid}\t{uid}\t{uid}\t{uid}\n") + + def __truediv__(self, part: str) -> FakeStatus: + assert part == "status" + return self._status + + class FakeProc: + def iterdir(self): + yield FakeEntry(41, AGENT_UID, "S") + yield FakeEntry(42, AGENT_UID, "Z") + yield FakeEntry(43, 0, "S") + + monkeypatch.setattr(agent_identity, "Path", lambda p: FakeProc()) + + assert agent_identity._agent_pids() == [41] + + +async def test_harness_spawn_guard_redirects_home_under_isolation(monkeypatch: pytest.MonkeyPatch) -> None: + """The isolation flag must be latched BEFORE the scrub pops it from os.environ.""" + + from coder_eval.agents.antigravity_agent import AntigravityAgent + + monkeypatch.setenv("CODER_EVAL_AGENT_ISOLATION", "1") + monkeypatch.setenv("HOME", "/root") + agent = AntigravityAgent(parse_agent_config(type=AgentKind.ANTIGRAVITY)) + agent._env_path_prepend = [] + + async with agent._harness_spawn_guard(): + assert os.environ["HOME"] == AGENT_HOME + assert "CODER_EVAL_AGENT_ISOLATION" not in os.environ + + assert os.environ["HOME"] == "/root" + assert os.environ["CODER_EVAL_AGENT_ISOLATION"] == "1" + + def test_explicitly_disabled_isolation_stays_disabled(tmp_path: Path) -> None: task = TaskDefinition( task_id="opt-out", From bcb6bb86a3385483f454638a7bd8e1b09f861f0f Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Wed, 12 Aug 2026 16:03:19 +0300 Subject: [PATCH 4/7] fix(isolation): guard raw-path rewrite alias to absolute paths so a bare reference.file cannot corrupt mount_point --- src/coder_eval/isolation/docker_runner.py | 14 +++++++-- tests/test_docker_identity_isolation.py | 37 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index 53e7944b..e85f6989 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -752,8 +752,11 @@ def _prepare_isolated_sources(self) -> None: self._host_to_private_paths[str(task_dir)] = CONTAINER_TASK_DIR # Alias the unresolved textual form too: staged-YAML strings may # carry a symlinked prefix (macOS /tmp -> /private/tmp) that the - # resolved key would never match. - self._host_to_private_paths.setdefault(str(self.rt.task_file.parent), CONTAINER_TASK_DIR) + # resolved key would never match. Only when it is itself an + # absolute path (see _register_private_mount for why). + raw_task_dir = self.rt.task_file.parent + if raw_task_dir.is_absolute(): + self._host_to_private_paths.setdefault(str(raw_task_dir), CONTAINER_TASK_DIR) from coder_eval.models import TemplateDirSource @@ -789,13 +792,18 @@ def _register_private_mount(self, source: Path, target: str) -> None: # The rewrite map keys on BOTH the resolved and the raw textual form: # staged-YAML strings usually match the raw form, while the mount and # dedup use the resolved one (symlinked prefixes differ between them). + # The raw alias is added ONLY when it is itself an absolute path: a + # relative form such as "." (a bare-filename reference.file whose + # parent is the cwd) would otherwise become a substring-replace key in + # _rewrite_task_paths and corrupt every unrelated field (e.g. every + # TemplateDirSource.mount_point, which defaults to "."). raw_key = str(source) source = source.resolve() key = str(source) if key in self._host_to_private_paths: return self._host_to_private_paths[key] = target - if raw_key != key: + if raw_key != key and Path(raw_key).is_absolute(): self._host_to_private_paths.setdefault(raw_key, target) self._private_source_mounts.append((source, target)) diff --git a/tests/test_docker_identity_isolation.py b/tests/test_docker_identity_isolation.py index cbe1aa15..517892b6 100644 --- a/tests/test_docker_identity_isolation.py +++ b/tests/test_docker_identity_isolation.py @@ -319,6 +319,43 @@ def test_skills_repo_path_value_rewritten_when_staged(tmp_path: Path, monkeypatc assert "SKILLS_REPO_PATH" not in argv # name-only form must not double-forward +def test_rewrite_task_paths_preserves_relative_mount_point(tmp_path: Path) -> None: + """A bare-filename reference.file must not turn '.' into a substring-replace key. + + reference.file="RESOLUTION.md" has parent Path("."). Registering its raw + textual form as a rewrite key would rewrite every TemplateDirSource + mount_point (default ".") to the references path, and the container-side + mount_point validator rejects the resulting absolute value. + """ + + from coder_eval.models import ReferenceSource, TemplateDirSource + + tpl = tmp_path / "tpl" + tpl.mkdir() + task = TaskDefinition( + task_id="bare-reference", + description="test", + initial_prompt="work", + agent=ClaudeCodeAgentConfig(type=AgentKind.CLAUDE_CODE), + reference=ReferenceSource(file="RESOLUTION.md"), + sandbox=SandboxConfig( + driver="docker", + template_sources=[TemplateDirSource(type="template_dir", path=str(tpl))], + docker=DockerDriverConfig(agent_isolation=True), + ), + success_criteria=[RunCommandCriterion(description="dynamic", command="python check.py")], + ) + task_dir = tmp_path / "task" + task_dir.mkdir() + runner = DockerRunner(MagicMock(task=task, task_file=task_dir / "task.yaml", run_dir=tmp_path / "run")) + runner._prepare_isolated_sources() + + assert "." not in runner._host_to_private_paths + rewritten = runner._rewrite_task_paths(task.model_dump(mode="json")) + for source in rewritten["sandbox"]["template_sources"]: + assert source["mount_point"] == "." + + def test_claude_mount_target_is_posix_under_isolation(tmp_path: Path) -> None: runner = _make_argv_runner(tmp_path, agent_isolation=True) runner._claude_mount_src = tmp_path / "claude-copy" From 22f2a3ec2662897dd8292bbe23a80b570ce8da0c Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Wed, 12 Aug 2026 18:02:06 +0300 Subject: [PATCH 5/7] fix(isolation): resolve antigravity localharness via SDK and intercept via ANTIGRAVITY_HARNESS_PATH; forward CODEX_API_VERSION --- src/coder_eval/agents/antigravity_agent.py | 93 ++++++++++++++++++++-- src/coder_eval/models/sandbox.py | 5 +- tests/test_antigravity_agent.py | 81 +++++++++++++++++++ tests/test_docker_wildcard_env.py | 7 ++ 4 files changed, 180 insertions(+), 6 deletions(-) diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 38c196e1..195d2794 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -226,6 +226,7 @@ def __init__( # for the harness's run_command tool — applied at spawn (see start()). self._env_path_prepend: list[str] = [] self._drop_shim_dir: Path | None = None + self._drop_shim_wrapper: Path | None = None # _state / _iteration / _iteration_was_incremented / pending_turn lifecycle # bookkeeping lives on the Agent base class (shared defaults + helpers). self._log = PrefixedAdapter(logger, {"prefix": instance_name}) @@ -234,12 +235,74 @@ def _effective_model(self) -> str: """Resolve the model: task ``agent.model`` > ``ANTIGRAVITY_MODEL`` > default.""" return self.config.model or settings.antigravity_model or _DEFAULT_MODEL - def _stage_localharness_drop_shim(self) -> Path: - """Shadow localharness with a wrapper around the shared setpriv policy.""" + @staticmethod + def _resolve_real_localharness() -> str: + """Resolve the REAL bundled localharness binary the way the SDK does. + + The google-antigravity wheel ships the binary at + ``google/antigravity/bin/localharness[.exe]`` and the SDK resolves it via + importlib (``ANTIGRAVITY_HARNESS_PATH`` env, then distribution files, then + package resources, then PATH last) - so a bare ``shutil.which`` misses the + wheel-bundled binary entirely. Prefer the SDK's own resolver so the shim + wraps exactly the binary the SDK would launch; if that private symbol moves + in an SDK bump, replicate its importlib order rather than silently + regressing to a PATH-only lookup. + """ + try: + from google.antigravity.connections.local.local_connection import ( # pyright: ignore[reportMissingImports] + _get_default_binary_path, + ) + except ImportError: + pass + else: + try: + return str(_get_default_binary_path()) + except Exception as e: + raise RuntimeError( + "agent isolation is enabled but the SDK could not resolve the localharness binary" + ) from e + + # Fallback: replicate the SDK resolver's importlib.metadata -> + # importlib.resources -> shutil.which order (see _get_default_binary_path + # in google.antigravity.connections.local.local_connection). + import importlib.metadata + import importlib.resources + import sys + + with contextlib.suppress(importlib.metadata.PackageNotFoundError, ValueError, AttributeError): + dist = importlib.metadata.distribution("google-antigravity") + for f in dist.files or []: + if ( + str(f) + .replace("\\", "/") + .endswith(("google/antigravity/bin/localharness", "google/antigravity/bin/localharness.exe")) + ): + candidate = os.path.abspath(str(f.locate())) + if os.path.exists(candidate): + return candidate + suffix = "bin/localharness.exe" if sys.platform == "win32" else "bin/localharness" + with contextlib.suppress(ImportError, AttributeError, KeyError): + candidate = str(importlib.resources.files("google.antigravity").joinpath(suffix)) + if os.path.exists(candidate): + return candidate + if path := shutil.which("localharness"): + return path + raise RuntimeError( + "agent isolation is enabled but the localharness binary could not be resolved " + + "(checked the SDK resolver, the google-antigravity wheel, and PATH)" + ) - real = shutil.which("localharness") - if real is None: - raise RuntimeError("agent isolation is enabled but localharness is not available on PATH") + def _stage_localharness_drop_shim(self) -> Path: + """Shadow localharness with a wrapper around the shared setpriv policy. + + The shim dir is also prepended to PATH (belt and suspenders), but PATH + alone never intercepts the launch: the SDK checks + ``ANTIGRAVITY_HARNESS_PATH`` FIRST and resolves its wheel-bundled binary + via importlib BEFORE ever consulting PATH. The load-bearing seam is the + env var - ``_harness_spawn_guard`` points it at the staged wrapper + (tracked on ``self._drop_shim_wrapper``) for the spawn window. + """ + real = self._resolve_real_localharness() shim_dir = Path(tempfile.mkdtemp(prefix="antigravity-drop-")) wrapper = shim_dir / "localharness" wrapper.write_text( @@ -248,6 +311,7 @@ def _stage_localharness_drop_shim(self) -> Path: ) wrapper.chmod(0o555) self._drop_shim_dir = shim_dir + self._drop_shim_wrapper = wrapper return shim_dir def _resolve_skills_paths(self, plugin_tools_dir: str | None) -> list[str]: @@ -418,6 +482,12 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: never affects the live harness. The lock is taken even when no prepend dirs were configured: a no-prepend spawn must still wait out any in-flight mutated- PATH window, or its harness would inherit another task's mock dirs. + + Under agent isolation the same transient window also redirects ``HOME`` to + ``AGENT_HOME`` and points ``ANTIGRAVITY_HARNESS_PATH`` at the staged setpriv + wrapper - the env var the SDK's binary resolver consults FIRST, and the only + seam that intercepts the launch (its bundled binary resolves via importlib + before PATH is ever consulted). Both are restored in ``finally``. """ async with _harness_spawn_lock(): # Latch the flag BEFORE the scrub: the pop below removes @@ -432,11 +502,18 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: path_key = next((k for k in os.environ if k.upper() == "PATH"), "PATH") original_path = os.environ.get(path_key) original_home = os.environ.get("HOME") + original_harness_path = os.environ.get("ANTIGRAVITY_HARNESS_PATH") + # The SDK checks ANTIGRAVITY_HARNESS_PATH FIRST and PATH last (the + # wheel-bundled binary resolves via importlib before PATH), so the env + # var is the only seam that reliably launches the setpriv wrapper. + harness_wrapper = self._drop_shim_wrapper if isolation_active else None if self._env_path_prepend: os.environ[path_key] = os.pathsep.join([*self._env_path_prepend, original_path or ""]) self._log.debug("PATH prepend for harness spawn: %s", os.pathsep.join(self._env_path_prepend)) if isolation_active: os.environ["HOME"] = AGENT_HOME + if harness_wrapper is not None: + os.environ["ANTIGRAVITY_HARNESS_PATH"] = str(harness_wrapper) try: yield finally: @@ -451,6 +528,11 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: os.environ.pop("HOME", None) else: os.environ["HOME"] = original_home + if harness_wrapper is not None: + if original_harness_path is None: + os.environ.pop("ANTIGRAVITY_HARNESS_PATH", None) + else: + os.environ["ANTIGRAVITY_HARNESS_PATH"] = original_harness_path async def communicate( self, @@ -626,6 +708,7 @@ async def _teardown(self) -> None: with contextlib.suppress(Exception): await stack.aclose() shim_dir, self._drop_shim_dir = self._drop_shim_dir, None + self._drop_shim_wrapper = None if shim_dir is not None: with contextlib.suppress(Exception): await asyncio.to_thread(shutil.rmtree, shim_dir, ignore_errors=True) diff --git a/src/coder_eval/models/sandbox.py b/src/coder_eval/models/sandbox.py index 611ba666..948f9d71 100644 --- a/src/coder_eval/models/sandbox.py +++ b/src/coder_eval/models/sandbox.py @@ -257,10 +257,13 @@ class DockerDriverConfig(BaseModel): # binary falls back to a ChatGPT login that doesn't exist in the # container and auth fails. CODEX_API_KEY drives login_api_key; # CODEX_BASE_URL routes to a custom endpoint (e.g. gateway); - # CODEX_MODEL selects the model when agent.model is unset. + # CODEX_MODEL selects the model when agent.model is unset; + # CODEX_API_VERSION adds the api-version query the Azure-routed + # endpoint requires - without it routing fails before any tool call. "CODEX_API_KEY", "CODEX_BASE_URL", "CODEX_MODEL", + "CODEX_API_VERSION", # Antigravity agent auth/routing — the google-antigravity local harness # authenticates against the Gemini API with GEMINI_API_KEY; without it # the in-container harness has no credential and fails. ANTIGRAVITY_MODEL diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index 99747950..29210c25 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -691,3 +691,84 @@ def _local_agent_config(**kwargs): assert captured["path"] == f"/sandbox/mocks{os.pathsep}/sandbox/bins{os.pathsep}/parent/bin" # ...and PATH was restored once the spawn completed. assert os.environ["PATH"] == "/parent/bin" + + +# --- agent isolation: localharness drop shim + ANTIGRAVITY_HARNESS_PATH seam ------ +# +# The SDK resolves its harness binary via ANTIGRAVITY_HARNESS_PATH first, then +# importlib (the wheel-bundled binary), and shutil.which only LAST - so the shim +# must resolve the REAL binary through the SDK resolver (a PATH-only lookup misses +# the bundled binary entirely) and the spawn guard must point the env var, not +# just PATH, at the setpriv wrapper. These stub the resolver via sys.modules so +# no google-antigravity install is needed. + + +def test_stage_drop_shim_resolves_via_sdk_resolver_not_path(monkeypatch, tmp_path): + """The shim wraps the resolver-resolved binary even when shutil.which finds nothing.""" + import shlex + import shutil + import sys + from types import ModuleType + + from coder_eval.agents import antigravity_agent + from coder_eval.models import CONTAINER_DROP_SHIM + + monkeypatch.setenv("CODER_EVAL_AGENT_ISOLATION", "1") + fake_real = tmp_path / "real-localharness" + fake_real.write_text("#!/bin/sh\n", encoding="utf-8") + local_connection = ModuleType("google.antigravity.connections.local.local_connection") + local_connection._get_default_binary_path = lambda: str(fake_real) + monkeypatch.setitem(sys.modules, "google.antigravity.connections.local.local_connection", local_connection) + # The wheel-bundled binary is NOT on PATH - which() must not be load-bearing. + monkeypatch.setattr(antigravity_agent.shutil, "which", lambda *_a, **_k: None) + + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + shim_dir = agent._stage_localharness_drop_shim() + try: + wrapper = shim_dir / "localharness" + assert wrapper.is_file() + assert os.access(wrapper, os.X_OK) + if os.name == "posix": + assert wrapper.stat().st_mode & 0o777 == 0o555 + expected = f'#!/usr/bin/env bash\nexec {CONTAINER_DROP_SHIM} {shlex.quote(str(fake_real))} "$@"\n' + assert wrapper.read_text(encoding="utf-8") == expected + assert agent._drop_shim_dir == shim_dir + assert agent._drop_shim_wrapper == wrapper + finally: + wrapper.chmod(0o755) # the 0o555 wrapper is read-only on Windows; unlock for rmtree + shutil.rmtree(shim_dir, ignore_errors=True) + + +async def test_harness_spawn_guard_sets_and_restores_antigravity_harness_path(monkeypatch, tmp_path): + """Under isolation the guard points ANTIGRAVITY_HARNESS_PATH at the staged wrapper. + + The env var is the load-bearing seam: the SDK checks it FIRST and resolves its + bundled binary via importlib before ever consulting PATH, so a PATH-only shadow + never intercepts the launch. Prior value restored (or removed) on exit. + """ + monkeypatch.setenv("CODER_EVAL_AGENT_ISOLATION", "1") + monkeypatch.setenv("ANTIGRAVITY_HARNESS_PATH", "/prior/harness") + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + wrapper = tmp_path / "localharness" + agent._drop_shim_wrapper = wrapper + + async with agent._harness_spawn_guard(): + assert os.environ["ANTIGRAVITY_HARNESS_PATH"] == str(wrapper) + assert os.environ["ANTIGRAVITY_HARNESS_PATH"] == "/prior/harness" + + monkeypatch.delenv("ANTIGRAVITY_HARNESS_PATH") + async with agent._harness_spawn_guard(): + assert os.environ["ANTIGRAVITY_HARNESS_PATH"] == str(wrapper) + assert "ANTIGRAVITY_HARNESS_PATH" not in os.environ + + +async def test_harness_spawn_guard_leaves_harness_path_alone_without_isolation(monkeypatch, tmp_path): + """The non-isolated path keeps relying on the SDK's own default resolution.""" + monkeypatch.delenv("CODER_EVAL_AGENT_ISOLATION", raising=False) + monkeypatch.delenv("ANTIGRAVITY_HARNESS_PATH", raising=False) + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + agent._drop_shim_wrapper = tmp_path / "localharness" # staged, but isolation is off + + async with agent._harness_spawn_guard(): + assert "ANTIGRAVITY_HARNESS_PATH" not in os.environ + assert "ANTIGRAVITY_HARNESS_PATH" not in os.environ diff --git a/tests/test_docker_wildcard_env.py b/tests/test_docker_wildcard_env.py index 0c61775f..15753a6c 100644 --- a/tests/test_docker_wildcard_env.py +++ b/tests/test_docker_wildcard_env.py @@ -44,3 +44,10 @@ def test_env_passthrough_extra_list_persists(self): extras = ["TOKEN_A", "TOKEN_B", "TOKEN_C"] cfg = DockerDriverConfig(env_passthrough_extra=extras) assert cfg.env_passthrough_extra == extras + + def test_default_allowlist_forwards_codex_auth_and_routing(self): + """All CODEX_* routing vars forward by default - the Azure-routed endpoint + needs CODEX_API_VERSION or the in-container codex binary fails routing.""" + cfg = DockerDriverConfig() + for var in ("CODEX_API_KEY", "CODEX_BASE_URL", "CODEX_MODEL", "CODEX_API_VERSION"): + assert var in cfg.env_passthrough From d95451a8f67700582ab61dd36ae7cd07491dc4fe Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Wed, 12 Aug 2026 18:31:15 +0300 Subject: [PATCH 6/7] fix(isolation): give antigravity agent-owned save_dir and app_data_dir so the dropped harness child can write its state --- src/coder_eval/agents/antigravity_agent.py | 31 ++++++++++++++++++++++ tests/test_antigravity_agent.py | 30 +++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 195d2794..975de93f 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -366,6 +366,27 @@ def _resolve_skills_paths(self, plugin_tools_dir: str | None) -> list[str]: self._log.debug("Antigravity skills_paths resolved: %s", roots) return roots + def _stage_isolation_state_dirs(self) -> tuple[str, str]: + """Return agent-writable ``(save_dir, app_data_dir)`` for the harness. + + The SDK otherwise creates its trajectory database under + ``tempfile.mkdtemp`` and its app-data under ``~/.gemini/antigravity``, + both resolved in the root parent process. The localharness child runs + as the dropped agent UID and cannot write into a root-owned directory, + so it fails at start. Place both below the agent home and grant them to + the agent identity so the dropped child owns what it must write. + """ + + from coder_eval.isolation.agent_identity import grant_agent_workspace + + state_root = Path(AGENT_HOME) / ".antigravity-isolation" + save_dir = state_root / "save" + app_data_dir = state_root / "app" + for directory in (save_dir, app_data_dir): + directory.mkdir(parents=True, exist_ok=True) + grant_agent_workspace(state_root) + return str(save_dir), str(app_data_dir) + def _resolve_workspaces(self, skills_paths: list[str]) -> list[str]: """Workspace roots for the harness's ``workspace_only`` file-tool policy. @@ -425,9 +446,19 @@ async def start( # from the environment itself (and raise a clear error if truly unset). api_key = os.getenv("GEMINI_API_KEY") or None skills_paths = self._resolve_skills_paths(plugin_tools_dir) + # Under isolation the harness state directories must be owned by the + # dropped agent UID; otherwise the SDK's root-created defaults deny + # the child every write. None in the non-isolated path so the SDK + # keeps its own defaults. + save_dir: str | None = None + app_data_dir: str | None = None + if agent_isolation_enabled(): + save_dir, app_data_dir = self._stage_isolation_state_dirs() cfg = LocalAgentConfig( model=self._effective_model(), api_key=api_key, + save_dir=save_dir, + app_data_dir=app_data_dir, # File tools are confined to ``workspaces`` by the auto-prepended # workspace_only policy. Scope to the sandbox workdir (the write # target — process-cwd-independent so concurrent host-mode tasks diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index 29210c25..b55637ac 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -772,3 +772,33 @@ async def test_harness_spawn_guard_leaves_harness_path_alone_without_isolation(m async with agent._harness_spawn_guard(): assert "ANTIGRAVITY_HARNESS_PATH" not in os.environ assert "ANTIGRAVITY_HARNESS_PATH" not in os.environ + + +def test_isolation_state_dirs_are_agent_owned_under_agent_home(monkeypatch, tmp_path): + """save_dir / app_data_dir are created below the agent home and granted to the agent UID. + + The SDK otherwise resolves both in the root parent (tempfile.mkdtemp and + ~/.gemini/antigravity), which the dropped localharness child cannot write. + """ + from coder_eval.agents import antigravity_agent + from coder_eval.isolation import agent_identity + + monkeypatch.setenv("CODER_EVAL_AGENT_ISOLATION", "1") + # AGENT_HOME is /home/agent on the real image; redirect it to a writable + # temp root so the test runs on any host, and stub the chown-based grant + # (which requires Linux root) to record its argument. + fake_home = tmp_path / "agent-home" + fake_home.mkdir() + monkeypatch.setattr(antigravity_agent, "AGENT_HOME", str(fake_home)) + granted: list[str] = [] + monkeypatch.setattr(agent_identity, "grant_agent_workspace", lambda p: granted.append(str(p))) + + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + save_dir, app_data_dir = agent._stage_isolation_state_dirs() + + state_root = fake_home / ".antigravity-isolation" + assert save_dir == str(state_root / "save") + assert app_data_dir == str(state_root / "app") + assert (state_root / "save").is_dir() + assert (state_root / "app").is_dir() + assert granted == [str(state_root)] From 9ddb782e60570df9855638db928a9ffc91c89394 Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Thu, 13 Aug 2026 08:30:57 +0300 Subject: [PATCH 7/7] refactor(isolation): remove dead AGENT_USERNAME and collapse duplicated env-scrub, env-restore, home-override, and mount-skip logic --- src/coder_eval/agents/antigravity_agent.py | 38 ++++++++-------------- src/coder_eval/agents/codex_agent.py | 29 +++++++++-------- src/coder_eval/isolation/docker_runner.py | 14 ++++---- src/coder_eval/models/__init__.py | 2 -- src/coder_eval/models/container_paths.py | 1 - src/coder_eval/utils.py | 10 ++++-- 6 files changed, 42 insertions(+), 52 deletions(-) diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 975de93f..0cc6e3f7 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -72,11 +72,7 @@ TurnEndStatus, TurnStartEvent, ) -from coder_eval.utils import ( - AGENT_ENV_SCRUB_PREFIXES, - AGENT_ENV_SCRUB_VARS, - expand_env_vars, -) +from coder_eval.utils import expand_env_vars, is_agent_scrubbed_env logger = logging.getLogger(__name__) @@ -102,6 +98,14 @@ def _harness_spawn_lock() -> asyncio.Lock: return _HARNESS_SPAWN_LOCK +def _restore_env(key: str, original: str | None) -> None: + """Restore an env var mutated for the spawn window (pop if originally unset).""" + if original is None: + os.environ.pop(key, None) + else: + os.environ[key] = original + + # Recommended Gemini coding model when a task pins no ``agent.model`` and neither # ``--model`` nor ``ANTIGRAVITY_MODEL`` is set. Gemini 3.5 Flash is Antigravity 2.0's # default coding model (2026-05) — it outperforms the older Gemini 3.1 Pro on coding / @@ -525,18 +529,11 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: # CODER_EVAL_AGENT_ISOLATION itself, so a later # agent_isolation_enabled() call would always read False. isolation_active = agent_isolation_enabled() - scrubbed = { - name: os.environ.pop(name) - for name in list(os.environ) - if name in AGENT_ENV_SCRUB_VARS or name.startswith(AGENT_ENV_SCRUB_PREFIXES) - } + scrubbed = {name: os.environ.pop(name) for name in list(os.environ) if is_agent_scrubbed_env(name)} path_key = next((k for k in os.environ if k.upper() == "PATH"), "PATH") original_path = os.environ.get(path_key) original_home = os.environ.get("HOME") original_harness_path = os.environ.get("ANTIGRAVITY_HARNESS_PATH") - # The SDK checks ANTIGRAVITY_HARNESS_PATH FIRST and PATH last (the - # wheel-bundled binary resolves via importlib before PATH), so the env - # var is the only seam that reliably launches the setpriv wrapper. harness_wrapper = self._drop_shim_wrapper if isolation_active else None if self._env_path_prepend: os.environ[path_key] = os.pathsep.join([*self._env_path_prepend, original_path or ""]) @@ -550,20 +547,11 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: finally: os.environ.update(scrubbed) if self._env_path_prepend: - if original_path is None: - os.environ.pop(path_key, None) - else: - os.environ[path_key] = original_path + _restore_env(path_key, original_path) if isolation_active: - if original_home is None: - os.environ.pop("HOME", None) - else: - os.environ["HOME"] = original_home + _restore_env("HOME", original_home) if harness_wrapper is not None: - if original_harness_path is None: - os.environ.pop("ANTIGRAVITY_HARNESS_PATH", None) - else: - os.environ["ANTIGRAVITY_HARNESS_PATH"] = original_harness_path + _restore_env("ANTIGRAVITY_HARNESS_PATH", original_harness_path) async def communicate( self, diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index 68a6262e..2af24815 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -1125,15 +1125,22 @@ def _build_codex_env(self) -> dict[str, str] | None: path_key = next((k for k in os.environ if k.upper() == "PATH"), "PATH") env[path_key] = os.pathsep.join([*self._env_path_prepend, os.environ.get(path_key, "")]) self._log.debug(f"PATH prepend: {os.pathsep.join(self._env_path_prepend)}") + # Point login shells at the generated profile dir (see + # _setup_login_shell_home) while pinning codex state (auth, rollout + # sessions) to its real location — _codex_home() reads the same + # resolution for sub-agent rollout recovery, so both sides agree. + # HOME steers bash/sh; ZDOTDIR steers zsh (the macOS default + # shell), which ignores HOME for dotfile selection when it is set. + # Under isolation start() always materializes _login_shell_home; the + # AGENT_HOME fallback keeps the root harness HOME out of the + # app-server env even if that ever changes. if self._login_shell_home is not None: - # Point login shells at the generated profile dir (see - # _setup_login_shell_home) while pinning codex state (auth, rollout - # sessions) to its real location — _codex_home() reads the same - # resolution for sub-agent rollout recovery, so both sides agree. - # HOME steers bash/sh; ZDOTDIR steers zsh (the macOS default - # shell), which ignores HOME for dotfile selection when it is set. - env["HOME"] = str(self._login_shell_home) - env["ZDOTDIR"] = str(self._login_shell_home) + home_override = str(self._login_shell_home) + else: + home_override = AGENT_HOME if agent_isolation_enabled() else None + if home_override is not None: + env["HOME"] = home_override + env["ZDOTDIR"] = home_override # The binary hard-errors on an explicitly set CODEX_HOME that does # not exist (unset, it materializes the ~/.codex default itself) — # hosts that auth via CODEX_API_KEY never ran `codex login`, so @@ -1141,12 +1148,6 @@ def _build_codex_env(self) -> dict[str, str] | None: codex_home = self._codex_home() codex_home.mkdir(parents=True, exist_ok=True) env["CODEX_HOME"] = str(codex_home) - elif agent_isolation_enabled(): - env["HOME"] = AGENT_HOME - env["ZDOTDIR"] = AGENT_HOME - codex_home = self._codex_home() - codex_home.mkdir(parents=True, exist_ok=True) - env["CODEX_HOME"] = str(codex_home) return env if env else None @staticmethod diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index e85f6989..86bfbf5d 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -764,11 +764,9 @@ def _prepare_isolated_sources(self) -> None: for source in self.rt.task.sandbox.template_sources or []: if not isinstance(source, TemplateDirSource): continue - host_path = Path(source.path) - if task_dir is not None and (host_path.resolve() == task_dir or task_dir in host_path.resolve().parents): - continue - self._register_private_mount(host_path, f"/opt/coder-eval/grader/templates/source-{template_index}") - template_index += 1 + target = f"/opt/coder-eval/grader/templates/source-{template_index}" + if self._register_external_private_path(Path(source.path), target): + template_index += 1 reference = self.rt.task.reference if reference is not None: @@ -781,12 +779,14 @@ def _prepare_isolated_sources(self) -> None: Path(reference.file).parent, "/opt/coder-eval/grader/references/file-parent" ) - def _register_external_private_path(self, source: Path, target: str) -> None: + def _register_external_private_path(self, source: Path, target: str) -> bool: + """Register a private mount unless ``source`` is already covered by the task-dir mount.""" task_dir = self.rt.task_file.parent.resolve() if self.rt.task_file else None resolved = source.resolve() if task_dir is not None and (resolved == task_dir or task_dir in resolved.parents): - return + return False self._register_private_mount(source, target) + return True def _register_private_mount(self, source: Path, target: str) -> None: # The rewrite map keys on BOTH the resolved and the raw textual form: diff --git a/src/coder_eval/models/__init__.py b/src/coder_eval/models/__init__.py index 730f7ef2..9c0da6c9 100644 --- a/src/coder_eval/models/__init__.py +++ b/src/coder_eval/models/__init__.py @@ -22,7 +22,6 @@ AGENT_GID, AGENT_HOME, AGENT_UID, - AGENT_USERNAME, CONTAINER_AGENT_WORK_DIR, CONTAINER_CLAUDE_SHIM, CONTAINER_DROP_SHIM, @@ -276,7 +275,6 @@ "AGENT_GID", "AGENT_HOME", "AGENT_UID", - "AGENT_USERNAME", "CONTAINER_AGENT_WORK_DIR", "CONTAINER_CLAUDE_SHIM", "CONTAINER_DROP_SHIM", diff --git a/src/coder_eval/models/container_paths.py b/src/coder_eval/models/container_paths.py index 35877a98..a83eb6de 100644 --- a/src/coder_eval/models/container_paths.py +++ b/src/coder_eval/models/container_paths.py @@ -27,7 +27,6 @@ AGENT_UID = 2000 AGENT_GID = 2000 -AGENT_USERNAME = "agent" AGENT_HOME = "/home/agent" CONTAINER_DROP_SHIM = "/usr/local/bin/coder_eval_drop_privilege.sh" diff --git a/src/coder_eval/utils.py b/src/coder_eval/utils.py index 95f2ef93..33ad8ab0 100644 --- a/src/coder_eval/utils.py +++ b/src/coder_eval/utils.py @@ -102,6 +102,12 @@ def process_plugins( AGENT_ENV_SCRUB_PREFIXES: tuple[str, ...] = ("CODER_EVAL_",) +def is_agent_scrubbed_env(name: str) -> bool: + """Whether an env var is harness-only and must be hidden from agent subprocesses.""" + + return name in AGENT_ENV_SCRUB_VARS or name.startswith(AGENT_ENV_SCRUB_PREFIXES) + + def scrub_agent_env_overrides() -> dict[str, str]: """Mask harness-only variables in SDK subprocess environments. @@ -111,9 +117,7 @@ def scrub_agent_env_overrides() -> dict[str, str]: during its serialized spawn window. """ - return { - name: "" for name in os.environ if name in AGENT_ENV_SCRUB_VARS or name.startswith(AGENT_ENV_SCRUB_PREFIXES) - } + return {name: "" for name in os.environ if is_agent_scrubbed_env(name)} SKIP = object() # Sentinel marking values that serialize_value should drop from the result.