From e6ace77c6884526f074a78c2912bd7ef6c43d7c7 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 5 Oct 2026 22:35:42 -0700 Subject: [PATCH 1/6] Load artifacts into a local agent's container load_artifact into an agent on the local provider failed: its scripts start with `sudo docker ...`, which ran this host's sudo, and the /app rewrite also changed the container side of `docker cp f c:/app/x`, so the files landed outside the container. LocalSandbox now drops `sudo` where a bash -c script runs it as a command, and leaves /app alone after ":" (the container side of a copy or a -v mount). collect_artifacts, verify_sandbox and the validator each had their own agent-container lookup. They now share find_agent_container. The validator's copy lacked the guard against borrowing another run's container, so its changelog marker check could read the wrong agent. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/a2a_agent/validator.py | 19 +-------- .../sandbox_providers/local_sandbox.py | 11 +++-- .../task_step/task_steps/collect_artifacts.py | 38 ++---------------- .../task_steps/sandbox_utils/sandbox_utils.py | 25 ++++++++++++ .../task_steps/verifiers/verify_sandbox.py | 27 +------------ .../task_step/test_verify_sandbox_local.py | 23 +++++++++++ .../test_validator_changelog_objects.py | 39 ++++++++++++++++++ .../sandbox_providers/local_sandbox_test.py | 32 +++++++++++++++ tst/unit/task_step/test_collect_artifacts.py | 7 ++-- .../test_sandbox_utils_agent_container.py | 40 +++++++++++++++++++ 10 files changed, 178 insertions(+), 83 deletions(-) create mode 100644 tst/unit/task_step/test_sandbox_utils_agent_container.py diff --git a/src/agent_env/a2a_agent/validator.py b/src/agent_env/a2a_agent/validator.py index 58b344c..05f07ec 100644 --- a/src/agent_env/a2a_agent/validator.py +++ b/src/agent_env/a2a_agent/validator.py @@ -54,6 +54,7 @@ VIDEO_PROBE_MP4_B64, VIDEO_PROBE_PROMPT, ) +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container if TYPE_CHECKING: from agent_env.a2a_agent.a2a_agent import A2AAgent @@ -686,7 +687,7 @@ def record(*, supported, advertised, save_ok, apply_ok, roundtrip_ok, note=""): if apply_agent.sandbox_type else get_agent_sandbox_provider()) sandbox = await provider.get_sandbox(apply_agent.sandbox_id) if sandbox.mode == SANDBOX_MODE_VM: - container = await A2AAgentValidator._discover_agent_container(sandbox) + container = await find_agent_container(sandbox) args = ("sudo", "docker", "exec", container, "cat", marker_path) else: args = ("cat", marker_path) @@ -706,22 +707,6 @@ def record(*, supported, advertised, save_ok, apply_ok, roundtrip_ok, note=""): record(supported=roundtrip_ok, advertised=advertised, save_ok=save_ok, apply_ok=True, roundtrip_ok=roundtrip_ok) - @staticmethod - async def _discover_agent_container(sandbox) -> str: - """Find the agent container on a VM sandbox (mirrors collect_artifacts / - verify_sandbox): prefer 'agent-api', else the first 'a2a-agent-*'.""" - exit_code, stdout, stderr = await sandbox.exec_with_output( - "sudo", "docker", "ps", "--format", "{{.Names}}") - if exit_code != 0: - raise RuntimeError(f"docker ps failed: {stderr[:200]}") - running = [n.strip() for n in stdout.splitlines() if n.strip()] - if sandbox.container_name in running: - return sandbox.container_name - fallback = [n for n in running if n.startswith("a2a-agent-")] - if not fallback: - raise RuntimeError(f"no agent container found; running: {running}") - return fallback[0] - @staticmethod def _upload_install_test_image_fixture(agent: "A2AAgent"): """Upload a minimal Dockerfile as a FileArtifactUniverse so the install diff --git a/src/agent_env/providers/sandbox_providers/local_sandbox.py b/src/agent_env/providers/sandbox_providers/local_sandbox.py index 51c589e..f6cf06d 100644 --- a/src/agent_env/providers/sandbox_providers/local_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/local_sandbox.py @@ -42,7 +42,10 @@ logger = logging.getLogger(__name__) -_APP_PATH_PATTERN = re.compile(r"(? Any: """Execute a command locally via subprocess. Strips 'sudo' and points /app at the local work directory, both as a path argument and - inside the script of a top-level ``bash -c``, unless the command runs in a container. - Returns an object with .stdout, .stderr streams and .wait() method, matching the + inside the script of a top-level ``bash -c``; /app is left alone when the command runs in a + container. Returns an object with .stdout, .stderr streams and .wait() method, matching the interface expected by exec_with_output(). """ cmd = [c for c in command if c != "sudo"] + if cmd[:2] == ["bash", "-c"] and len(cmd) > 2: + cmd[2] = _SCRIPT_SUDO.sub(r"\1\2", cmd[2]) if not _runs_in_container(cmd): is_script = cmd[:2] == ["bash", "-c"] cmd = [ diff --git a/src/agent_env/task_step/task_steps/collect_artifacts.py b/src/agent_env/task_step/task_steps/collect_artifacts.py index e8fbd3f..15f4e25 100644 --- a/src/agent_env/task_step/task_steps/collect_artifacts.py +++ b/src/agent_env/task_step/task_steps/collect_artifacts.py @@ -84,6 +84,7 @@ from agent_env.task_step.context import TaskStepContext from agent_env.entity_refs import EntityRef from agent_env.task_step.task_step import TaskStep, TaskStepDependency +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container from agent_env.task_step.thread_work import finish_on_thread logger = logging.getLogger(__name__) @@ -443,39 +444,6 @@ async def _resolve_live_sandbox(self, provider, sandbox_id: str): f"a re-run-from-step needs the original live sandbox; re-run the full task." ) from err - async def _discover_container(self, sandbox) -> str: - """Find the agent container running on the VM. - - The A2A agent deploy hardcodes the container name to 'agent-api' - (see agent_env/a2a_agent/a2a_agent.py). We also accept any - 'a2a-agent-*' container as a fallback in case the naming scheme - evolves. - """ - exit_code, stdout, stderr = await sandbox.exec_with_output( - "sudo", "docker", "ps", - "--format", "{{.Names}}", - ) - if exit_code != 0: - raise RuntimeError(f"Failed to list containers: {stderr[:300]}") - running = [n.strip() for n in stdout.splitlines() if n.strip()] - # Prefer this sandbox's own container name, fall back to the a2a-agent-* prefix. - if sandbox.container_name in running: - return sandbox.container_name - if getattr(sandbox, "owns_container", False): - # Its Docker host is shared, so any other agent container there is another run's. - raise RuntimeError( - f"Agent container {sandbox.container_name!r} is not running. Running containers: {running}." - ) - fallback = [n for n in running if n.startswith("a2a-agent-")] - if fallback: - if len(fallback) > 1: - logger.warning(f"Multiple a2a-agent-* containers found; using first: {fallback}") - return fallback[0] - raise RuntimeError( - f"No agent container found on the VM (looked for 'agent-api' or 'a2a-agent-*'). " - f"Running containers: {running}. Has deploy_agent been run in this task?" - ) - async def _get_file_size(self, sandbox, container: Optional[str], source_path: str) -> int: """Get file size on the agent's filesystem. Returns -1 only if the file is genuinely absent; a `stat` that fails for any other reason RAISES so a failed size check is a @@ -809,7 +777,7 @@ async def _collect_via_agent_container(self, context, store, artifact_id, versio sandbox = await self._resolve_live_sandbox(provider, agent.sandbox_id) logger.info(f"Connected to sandbox {agent.sandbox_id} (mode={sandbox.mode})") - container = await self._discover_container(sandbox) if sandbox.mode == SANDBOX_MODE_VM else None + container = await find_agent_container(sandbox) if sandbox.mode == SANDBOX_MODE_VM else None if container: logger.info(f"Using agent container: {container}") @@ -847,7 +815,7 @@ async def _collect_via_vm_host(self, context, store, artifact_id, version): async def _collect_via_sandbox_container(self, context, store, artifact_id, version): """Collect from a plain container started by ``run_docker_container``. - The agent path finds its container by discovery, and ``_discover_container`` only accepts + The agent path finds its container by discovery, and ``find_agent_container`` only accepts ``agent-api`` / ``a2a-agent-*`` names — so a task that deploys a sandbox and runs an ordinary image had no way to get its files out. Here the (sandbox, container) pair is named explicitly and resolved exactly as ``load_artifact`` resolves it. diff --git a/src/agent_env/task_step/task_steps/sandbox_utils/sandbox_utils.py b/src/agent_env/task_step/task_steps/sandbox_utils/sandbox_utils.py index 975f849..10149ee 100644 --- a/src/agent_env/task_step/task_steps/sandbox_utils/sandbox_utils.py +++ b/src/agent_env/task_step/task_steps/sandbox_utils/sandbox_utils.py @@ -77,3 +77,28 @@ async def fetch_container_logs(agent, tail: int = 500) -> Optional[str]: sandbox_id, e, exc_info=True, ) return None + + +async def find_agent_container(sandbox) -> str: + """The agent container running on a VM sandbox: the sandbox's own ``container_name``, else the + first ``a2a-agent-*``. A sandbox that owns its container never takes another: its Docker host + is shared, so any other agent container there is another run's.""" + exit_code, stdout, stderr = await sandbox.exec_with_output("sudo", "docker", "ps", "--format", "{{.Names}}") + if exit_code != 0: + raise RuntimeError(f"Failed to list containers: {stderr[:300]}") + running = [n.strip() for n in stdout.splitlines() if n.strip()] + if sandbox.container_name in running: + return sandbox.container_name + if getattr(sandbox, "owns_container", False): + raise RuntimeError( + f"Agent container {sandbox.container_name!r} is not running. Running containers: {running}." + ) + fallback = [n for n in running if n.startswith("a2a-agent-")] + if fallback: + if len(fallback) > 1: + logger.warning(f"Multiple a2a-agent-* containers found; using first: {fallback}") + return fallback[0] + raise RuntimeError( + f"No agent container found on the VM (looked for {sandbox.container_name!r} or 'a2a-agent-*'). " + f"Running containers: {running}. Has deploy_agent been run in this task?" + ) diff --git a/src/agent_env/task_step/task_steps/verifiers/verify_sandbox.py b/src/agent_env/task_step/task_steps/verifiers/verify_sandbox.py index 7bb7c3a..aeaa7d3 100644 --- a/src/agent_env/task_step/task_steps/verifiers/verify_sandbox.py +++ b/src/agent_env/task_step/task_steps/verifiers/verify_sandbox.py @@ -36,6 +36,7 @@ ) from agent_env.task_step.context import TaskStepContext from agent_env.task_step.task_step import TaskStep, TaskStepDependency +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container from agent_env.task_step.task_steps.verifiers.scoring import ScoreAggregator, aggregate_score logger = logging.getLogger(__name__) @@ -120,7 +121,7 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: sandbox = await (self._agent_sandbox(context) if on_agent else self._deployed_sandbox(context)) logger.info(f"Connected to sandbox {sandbox.sandbox_id} (mode={sandbox.mode})") container = ( - await self._discover_container(sandbox) + await find_agent_container(sandbox) if on_agent and sandbox.mode == SANDBOX_MODE_VM else None ) @@ -287,27 +288,3 @@ async def _eval_shell( exit_code, _, stderr = await self._exec(sandbox, args) return _outcome(exit_code == 0, f"exit={exit_code}; stderr={stderr[:200]}") - async def _discover_container(self, sandbox) -> str: - """Find the agent container on a VM sandbox. Mirrors collect_artifacts._discover_container.""" - exit_code, stdout, stderr = await sandbox.exec_with_output( - "sudo", "docker", "ps", "--format", "{{.Names}}", - ) - if exit_code != 0: - raise RuntimeError(f"Failed to list containers: {stderr[:300]}") - running = [n.strip() for n in stdout.splitlines() if n.strip()] - if sandbox.container_name in running: - return sandbox.container_name - if getattr(sandbox, "owns_container", False): - # Its Docker host is shared, so any other agent container there is another run's. - raise RuntimeError( - f"Agent container {sandbox.container_name!r} is not running. Running containers: {running}." - ) - fallback = [n for n in running if n.startswith("a2a-agent-")] - if fallback: - if len(fallback) > 1: - logger.warning(f"Multiple a2a-agent-* containers found; using first: {fallback}") - return fallback[0] - raise RuntimeError( - f"No agent container found on the VM (looked for 'agent-api' or 'a2a-agent-*'). " - f"Running containers: {running}." - ) diff --git a/tst/integration/task_step/test_verify_sandbox_local.py b/tst/integration/task_step/test_verify_sandbox_local.py index 264d27b..cf4e7df 100644 --- a/tst/integration/task_step/test_verify_sandbox_local.py +++ b/tst/integration/task_step/test_verify_sandbox_local.py @@ -13,6 +13,8 @@ import pytest_asyncio from agent_env.artifact import FileArtifact, FileArtifactUniverse +from agent_env.a2a_agent.a2a_agent import DeployedA2AAgent +from agent_env.a2a_agent.store import get_a2a_agent_instance_store from agent_env.artifact.store import reset_artifact_store from agent_env.config import configure, get_config, reset_config from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox, LocalSandboxProvider @@ -168,6 +170,27 @@ async def test_collect_artifacts_reads_a_local_agents_file_from_its_container(lo assert get_config().get_object_store().get(url) == f"{token}\n".encode() +@pytest.mark.asyncio +async def test_load_artifact_puts_a_universe_in_a_local_agents_container(local_agent, tmp_path): + sandbox, context = local_agent + instance = get_a2a_agent_instance_store().create_instance(DeployedA2AAgent( + agent_id="agent", agent_version=1, a2a_url=sandbox.tunnel_urls[80], sandbox_id=sandbox.sandbox_id, + agent_card={}, sandbox_type="local", + ), 600) + context.deployed_agents[0].instance_id = instance.instance_id + universe = _greeting_universe(tmp_path / "greeting", uuid.uuid4().hex[:8]) + step = LoadArtifactTaskStep( + id="load", version=None, artifact_id=universe.id, agent_name="agent", destination_path="/app/greeting", + ) + + await step.execute(context) + + shown = subprocess.run(["docker", "exec", sandbox.container_name, "cat", "/app/greeting/hello.txt"], + capture_output=True, text=True) + assert (shown.returncode, shown.stdout) == (0, "hello, world\n") + assert list(sandbox.work_dir.iterdir()) == [sandbox.work_dir / ".agent-container-mode"] + + def _write_in_container(sandbox, path, text): """Write ``text`` at ``path`` inside the sandbox's container, as the agent would.""" script = f"mkdir -p $(dirname {path}) && echo {text} > {path}" diff --git a/tst/unit/a2a_agent/test_validator_changelog_objects.py b/tst/unit/a2a_agent/test_validator_changelog_objects.py index bb715ea..e348e61 100644 --- a/tst/unit/a2a_agent/test_validator_changelog_objects.py +++ b/tst/unit/a2a_agent/test_validator_changelog_objects.py @@ -149,3 +149,42 @@ async def test_validator_records_a_capture_it_cannot_send_instead_of_raising(sto assert not requests assert verification["apply"] is False assert verification["note"].startswith("apply request failed: ") + + +@pytest.mark.asyncio +async def test_the_marker_is_never_read_from_another_runs_container(store, requests, monkeypatch): + """A reattached local agent owns its container; with that container gone, the check fails rather than + reading the marker out of another run's agent on the same Docker host.""" + calls: list[tuple] = [] + + class _ReattachedLocalAgent: + mode = "vm" + container_name = "agent-local-apply1" + owns_container = True + + async def exec_with_output(self, *args): + calls.append(args) + if args[:3] == ("sudo", "docker", "ps"): + return 0, "a2a-agent-other\n", "" + return 0, "marker-token", "" + + class _Provider: + async def get_sandbox(self, sandbox_id): + return _ReattachedLocalAgent() + + async def close(self): + return None + + monkeypatch.setattr(provider_mod, "build_sandbox_provider", lambda spec: _Provider()) + for name in ("000000.tar", "000001.tar"): + store.put(f"changelog/run-1/{name}", b"increment") + context = _context( + {"object_url": store.object_url("changelog/run-1"), "transfer_mode": "objects"}, + ["increments"], + ) + + verification = await _validate(context) + + assert verification["roundtrip"] is False + assert "'agent-local-apply1' is not running" in verification["note"] + assert calls == [("sudo", "docker", "ps", "--format", "{{.Names}}")] diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_test.py index 534827c..d76d398 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_test.py @@ -683,9 +683,41 @@ async def test_exec_leaves_a_docker_exec_script_naming_the_containers_app_alone( await sandbox.exec("sudo", "bash", "-c", script) + assert spawned == [("bash", "-c", script.removeprefix("sudo "))] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("script, ran", [ + ("sudo docker exec -u 0 c mkdir -p /files", "docker exec -u 0 c mkdir -p /files"), + ("sudo docker cp /tmp/s/. c:/files && rm -f /tmp/s", "docker cp /tmp/s/. c:/files && rm -f /tmp/s"), + ("set -e\nsudo docker ps", "set -e\ndocker ps"), + ("a; sudo b | sudo tee f && v=$(sudo cat g)", "a; b | tee f && v=$(cat g)"), +]) +async def test_exec_drops_sudo_a_script_runs_as_a_command_on_this_host(spawned, script, ran): + await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("sudo", "bash", "-c", script) + + assert spawned == [("bash", "-c", ran)] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("script", ["docker exec c sudo ls", "echo pseudo sudoers"]) +async def test_exec_keeps_sudo_that_is_not_a_command_on_this_host(spawned, script): + await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("bash", "-c", script) + assert spawned == [("bash", "-c", script)] +@pytest.mark.asyncio +@pytest.mark.parametrize("script, ran", [ + ("docker cp /tmp/x/. agent-local-1:/app/files", "docker cp /tmp/x/. agent-local-1:/app/files"), + ("docker run -v /app/h:/app/c img", "docker run -v /tmp/agent-env-work/h:/app/c img"), +]) +async def test_exec_leaves_the_container_side_of_a_copy_or_mount_alone(spawned, script, ran): + await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("bash", "-c", script) + + assert spawned == [("bash", "-c", ran)] + + @pytest.mark.parametrize("script", ["ls ~/app/x", "cat ${HOME}/app/x", "cat $(pwd)/app/x", "cat $APP/app/x"]) def test_rewrite_app_script_keeps_app_after_an_expansion(script): assert LocalSandbox(work_dir=Path("/tmp/agent-env-work"))._rewrite_app_script(script) == script diff --git a/tst/unit/task_step/test_collect_artifacts.py b/tst/unit/task_step/test_collect_artifacts.py index f667679..ff8861f 100644 --- a/tst/unit/task_step/test_collect_artifacts.py +++ b/tst/unit/task_step/test_collect_artifacts.py @@ -21,6 +21,7 @@ from agent_env.task_step.context import DeployedAgent, PromptResponse, TaskStepContext from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox, LocalSandboxProvider from agent_env.task_step.task_steps.collect_artifacts import CollectArtifactsTaskStep, _exec_args, _is_url_entry +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container from agent_env.env.env import DeployedEnv, DeployedGatewayEnv, EnvCapabilityUnsupported from agent_env.env.gateway.constants import EXT_STEP_URI, GATEWAY_EXTENSIONS, WELL_KNOWN_PATH from tst.unit.event_loop_probe import on_event_loop @@ -547,7 +548,7 @@ def test_a_reattached_local_agent_is_read_inside_its_own_container(self, tmp_pat sandbox, calls = _reattached_local_agent(tmp_path, monkeypatch, running="a2a-agent-other\nagent-local-agent1\n") step = CollectArtifactsTaskStep(id="collect", version=None, agent_name="solver") - container = _run(step._discover_container(sandbox)) + container = _run(find_agent_container(sandbox)) _run(step._list_base_directory(sandbox, container)) assert calls == [ @@ -561,7 +562,7 @@ def test_a_reattached_local_agent_never_borrows_another_runs_container(self, tmp sandbox, calls = _reattached_local_agent(tmp_path, monkeypatch, running="a2a-agent-other\n") with pytest.raises(RuntimeError, match="'agent-local-agent1' is not running"): - _run(CollectArtifactsTaskStep(id="collect", version=None)._discover_container(sandbox)) + _run(find_agent_container(sandbox)) assert calls == [("sudo", "docker", "ps", "--format", "{{.Names}}")] @@ -716,7 +717,7 @@ class TestSandboxContainerPath: """`container_name` + `sandbox_name`: collecting from a plain run_docker_container container. Before this path existed, a task built from deploy_sandbox -> run_docker_container had no way - to get files out: `_discover_container` only matches `agent-api` / `a2a-agent-*`, so the agent + to get files out: `find_agent_container` only matches `agent-api` / `a2a-agent-*`, so the agent path found nothing and the CUA path refuses a non-CUA env. """ diff --git a/tst/unit/task_step/test_sandbox_utils_agent_container.py b/tst/unit/task_step/test_sandbox_utils_agent_container.py new file mode 100644 index 0000000..db42e62 --- /dev/null +++ b/tst/unit/task_step/test_sandbox_utils_agent_container.py @@ -0,0 +1,40 @@ +"""``find_agent_container``: which container on a VM sandbox is the agent's.""" + +from __future__ import annotations + +import asyncio +from types import SimpleNamespace + +import pytest + +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container + + +def _sandbox(running: str, *, owns_container: bool = False, exit_code: int = 0): + async def exec_with_output(*args): + assert args == ("sudo", "docker", "ps", "--format", "{{.Names}}") + return exit_code, running, "boom" + + return SimpleNamespace(container_name="agent-local-1", owns_container=owns_container, exec_with_output=exec_with_output) + + +def test_the_sandboxs_own_container_wins(): + assert asyncio.run(find_agent_container(_sandbox("a2a-agent-x\nagent-local-1\n"))) == "agent-local-1" + + +def test_a_sandbox_that_owns_its_container_never_takes_another(): + with pytest.raises(RuntimeError, match="'agent-local-1' is not running"): + asyncio.run(find_agent_container(_sandbox("a2a-agent-x\n", owns_container=True))) + + +def test_a_vm_the_run_does_not_own_falls_back_to_an_a2a_agent_container(): + assert asyncio.run(find_agent_container(_sandbox("db\na2a-agent-x\na2a-agent-y\n"))) == "a2a-agent-x" + + +@pytest.mark.parametrize("running, exit_code, message", [ + ("db\n", 0, "No agent container found"), + ("", 1, "Failed to list containers: boom"), +]) +def test_no_agent_container_is_an_error(running, exit_code, message): + with pytest.raises(RuntimeError, match=message): + asyncio.run(find_agent_container(_sandbox(running, exit_code=exit_code))) From e8252fd570a0abd18d480cad1ebeba02fbfe8bd2 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 5 Oct 2026 22:47:00 -0700 Subject: [PATCH 2/6] Keep sudo meant for a container, and host paths after a colon Dropping sudo from a script also reached a command quoted for a container (`docker exec c bash -c 'make; sudo make install'`). Only sudo outside quotes, where this host's shell runs it, is dropped now. Skipping /app after every ":" also skipped host paths such as `PATH=$PATH:/app/bin`. Only the container side of a `docker cp` and of a `-v`/`--volume` mount is skipped now. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/local_sandbox.py | 15 +++++++++------ .../sandbox_providers/local_sandbox_test.py | 19 ++++++++++++++++++- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/local_sandbox.py b/src/agent_env/providers/sandbox_providers/local_sandbox.py index f6cf06d..fd50761 100644 --- a/src/agent_env/providers/sandbox_providers/local_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/local_sandbox.py @@ -42,10 +42,12 @@ logger = logging.getLogger(__name__) -# Not after ":": that is the container side of `docker cp f c:/app/x` or `-v h:/app/x`. -_APP_PATH_PATTERN = re.compile(r"(? str: return arg def _rewrite_app_script(self, script: str) -> str: - return _APP_PATH_PATTERN.sub(lambda _: str(self._work_dir), script) + in_container = {m.end() for m in _CONTAINER_APP.finditer(script)} + return _APP_PATH_PATTERN.sub(lambda m: m[0] if m.start() in in_container else str(self._work_dir), script) async def terminate(self) -> None: """Tear down whatever this sandbox is running. @@ -224,7 +227,7 @@ async def exec(self, *command: str) -> Any: """ cmd = [c for c in command if c != "sudo"] if cmd[:2] == ["bash", "-c"] and len(cmd) > 2: - cmd[2] = _SCRIPT_SUDO.sub(r"\1\2", cmd[2]) + cmd[2] = _HOST_SUDO.sub(lambda m: m[0] if m[1] is None else m[1] + m[2], cmd[2]) if not _runs_in_container(cmd): is_script = cmd[:2] == ["bash", "-c"] cmd = [ diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_test.py index d76d398..a0e1ebf 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_test.py @@ -700,7 +700,13 @@ async def test_exec_drops_sudo_a_script_runs_as_a_command_on_this_host(spawned, @pytest.mark.asyncio -@pytest.mark.parametrize("script", ["docker exec c sudo ls", "echo pseudo sudoers"]) +@pytest.mark.parametrize("script", [ + "docker exec c sudo ls", + "echo pseudo sudoers", + "docker exec c bash -c 'make; sudo make install'", + "docker exec c bash -c 'it'\"'\"'s; sudo ls'", + 'echo "a; sudo b"', +]) async def test_exec_keeps_sudo_that_is_not_a_command_on_this_host(spawned, script): await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("bash", "-c", script) @@ -710,7 +716,10 @@ async def test_exec_keeps_sudo_that_is_not_a_command_on_this_host(spawned, scrip @pytest.mark.asyncio @pytest.mark.parametrize("script, ran", [ ("docker cp /tmp/x/. agent-local-1:/app/files", "docker cp /tmp/x/. agent-local-1:/app/files"), + ("docker cp agent-local-1:/app/out.txt /app/out.txt", "docker cp agent-local-1:/app/out.txt /tmp/agent-env-work/out.txt"), + ("docker cp /tmp/x c:'/app/a b'", "docker cp /tmp/x c:'/app/a b'"), ("docker run -v /app/h:/app/c img", "docker run -v /tmp/agent-env-work/h:/app/c img"), + ("docker run --volume=data:/app img", "docker run --volume=data:/app img"), ]) async def test_exec_leaves_the_container_side_of_a_copy_or_mount_alone(spawned, script, ran): await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("bash", "-c", script) @@ -718,6 +727,14 @@ async def test_exec_leaves_the_container_side_of_a_copy_or_mount_alone(spawned, assert spawned == [("bash", "-c", ran)] +@pytest.mark.parametrize("script, ran", [ + ("PATH=$PATH:/app/bin tool", "PATH=$PATH:/tmp/agent-env-work/bin tool"), + ("docker cp /tmp/x c:/files && PATH=$PATH:/app/bin t", "docker cp /tmp/x c:/files && PATH=$PATH:/tmp/agent-env-work/bin t"), +]) +def test_rewrite_app_script_rewrites_a_host_path_after_a_colon(script, ran): + assert LocalSandbox(work_dir=Path("/tmp/agent-env-work"))._rewrite_app_script(script) == ran + + @pytest.mark.parametrize("script", ["ls ~/app/x", "cat ${HOME}/app/x", "cat $(pwd)/app/x", "cat $APP/app/x"]) def test_rewrite_app_script_keeps_app_after_an_expansion(script): assert LocalSandbox(work_dir=Path("/tmp/agent-env-work"))._rewrite_app_script(script) == script From 34f702d4cf32ea8bb5919c4fb08b6f5ccda1fe94 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 5 Oct 2026 23:28:14 -0700 Subject: [PATCH 3/6] Stop writing sudo inside scripts, instead of stripping it Eight scripts in load_artifact and the unit-tests verifier began with `sudo docker`. Each already runs under exec_script's `sudo bash -c`, so the inner sudo gave no extra privilege where the provider keeps the outer one (E2B, the Scale sandboxes). Where the provider drops it, the inner one broke the script: the host's sudo asks for a password locally, and the Modal VM image has no sudo. The other docker calls in scripts, such as run_docker_container's, already had none. With the inner sudo gone, LocalSandbox no longer rewrites sudo in script text. New local integration tests load an EnvironmentArtifact into a container and run the unit-tests verifier in one. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/local_sandbox.py | 9 +-- .../task_step/task_steps/load_artifact.py | 10 +-- .../run_container_unit_tests_verifier.py | 6 +- .../task_step/test_verify_sandbox_local.py | 63 ++++++++++++++++++- .../sandbox_providers/local_sandbox_test.py | 27 -------- 5 files changed, 72 insertions(+), 43 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/local_sandbox.py b/src/agent_env/providers/sandbox_providers/local_sandbox.py index fd50761..2a5905f 100644 --- a/src/agent_env/providers/sandbox_providers/local_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/local_sandbox.py @@ -45,9 +45,6 @@ _APP_PATH_PATTERN = re.compile(r"(? Any: """Execute a command locally via subprocess. Strips 'sudo' and points /app at the local work directory, both as a path argument and - inside the script of a top-level ``bash -c``; /app is left alone when the command runs in a - container. Returns an object with .stdout, .stderr streams and .wait() method, matching the + inside the script of a top-level ``bash -c``, unless the command runs in a container. + Returns an object with .stdout, .stderr streams and .wait() method, matching the interface expected by exec_with_output(). """ cmd = [c for c in command if c != "sudo"] - if cmd[:2] == ["bash", "-c"] and len(cmd) > 2: - cmd[2] = _HOST_SUDO.sub(lambda m: m[0] if m[1] is None else m[1] + m[2], cmd[2]) if not _runs_in_container(cmd): is_script = cmd[:2] == ["bash", "-c"] cmd = [ diff --git a/src/agent_env/task_step/task_steps/load_artifact.py b/src/agent_env/task_step/task_steps/load_artifact.py index c4d1ac6..12b70eb 100644 --- a/src/agent_env/task_step/task_steps/load_artifact.py +++ b/src/agent_env/task_step/task_steps/load_artifact.py @@ -662,10 +662,10 @@ async def _stage_environment_payload_into_container( ) else: await sandbox.exec_script( - f"sudo docker exec -u 0 {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" + f"docker exec -u 0 {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" ) await sandbox.exec_script( - f"sudo docker cp {shlex.quote(vm_stage)}/. " + f"docker cp {shlex.quote(vm_stage)}/. " f"{shlex.quote(container_name)}:{shlex.quote(destination)}" ) finally: @@ -747,7 +747,7 @@ async def _load_universe_into_container(sandbox, container_name: str, universe, return [] await sandbox.exec_script( - f"sudo docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" + f"docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" ) loaded: list[str] = [] total = len(file_artifacts) @@ -764,10 +764,10 @@ async def _load_universe_into_container(sandbox, container_name: str, universe, await sandbox.load_s3_file(fa.object_url, vm_temp) if parent and parent != destination: await sandbox.exec_script( - f"sudo docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(parent)}" + f"docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(parent)}" ) await sandbox.exec_script( - f"sudo docker cp {shlex.quote(vm_temp)} " + f"docker cp {shlex.quote(vm_temp)} " f"{shlex.quote(container_name)}:{shlex.quote(dest_path)} && " f"rm -f {shlex.quote(vm_temp)}" ) diff --git a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py index f3764a0..01cc5c1 100644 --- a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py +++ b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py @@ -262,7 +262,7 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: f"{setup_cmd.splitlines()[0][:140]}" ) await sandbox.exec_script( - f"sudo docker exec -u {shlex.quote(self.user)} {env_flags} " + f"docker exec -u {shlex.quote(self.user)} {env_flags} " f"{shlex.quote(self.container_name)} bash -c {shlex.quote(setup_cmd)}" ) @@ -272,7 +272,7 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: f"in container '{self.container_name}': {command[:160]}" ) wrapped = ( - f"sudo docker exec -u {shlex.quote(self.user)} {env_flags} " + f"docker exec -u {shlex.quote(self.user)} {env_flags} " f"{shlex.quote(self.container_name)} " f"timeout --kill-after=10 {self.timeout_sec} bash -c {shlex.quote(command)}" ) @@ -429,7 +429,7 @@ async def _extract_file(self, sandbox, path_in_container: str) -> str: """`docker cp` a file out of the container, read it from the VM, return text.""" vm_temp = f"/tmp/_verifier_out_{uuid.uuid4().hex[:8]}" await sandbox.exec_script( - f"sudo docker cp {shlex.quote(self.container_name)}:{shlex.quote(path_in_container)} " + f"docker cp {shlex.quote(self.container_name)}:{shlex.quote(path_in_container)} " f"{shlex.quote(vm_temp)}" ) try: diff --git a/tst/integration/task_step/test_verify_sandbox_local.py b/tst/integration/task_step/test_verify_sandbox_local.py index cf4e7df..5131b26 100644 --- a/tst/integration/task_step/test_verify_sandbox_local.py +++ b/tst/integration/task_step/test_verify_sandbox_local.py @@ -5,24 +5,31 @@ an agent's, so they need Docker too, but no model. """ +import io +import json import shutil import subprocess import uuid +import zipfile import pytest import pytest_asyncio from agent_env.artifact import FileArtifact, FileArtifactUniverse +from agent_env.artifact.artifacts.environment import EnvironmentArtifact from agent_env.a2a_agent.a2a_agent import DeployedA2AAgent from agent_env.a2a_agent.store import get_a2a_agent_instance_store from agent_env.artifact.store import reset_artifact_store from agent_env.config import configure, get_config, reset_config from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox, LocalSandboxProvider from agent_env.task import Task -from agent_env.task_step.context import DeployedAgent, TaskStepContext +from agent_env.task_step.context import DeployedAgent, DeployedSandbox, TaskStepContext from agent_env.task_step.task_steps.collect_artifacts import CollectArtifactsTaskStep from agent_env.task_step.task_steps.deploy_sandbox import DeploySandboxTaskStep from agent_env.task_step.task_steps.load_artifact import LoadArtifactTaskStep +from agent_env.task_step.task_steps.verifiers.run_container_unit_tests_verifier import ( + RunContainerUnitTestsVerifierTaskStep, +) from agent_env.task_step.task_steps.verifiers.verify_sandbox import VerifySandboxTaskStep from tst.util.capabilities import missing_capability_reason @@ -191,6 +198,60 @@ async def test_load_artifact_puts_a_universe_in_a_local_agents_container(local_a assert list(sandbox.work_dir.iterdir()) == [sandbox.work_dir / ".agent-container-mode"] +@pytest.mark.asyncio +async def test_load_artifact_stages_an_environment_payload_in_a_local_container(local_agent, tmp_path): + sandbox, context = local_agent + _as_deployed_container(sandbox, context) + payload = io.BytesIO() + with zipfile.ZipFile(payload, "w") as zf: + zf.writestr("root/docs/readme.md", "root doc\n") + zf.writestr("data.json", json.dumps({"files": [{"path": "notes/a.txt", "content": "from data.json\n"}]})) + (tmp_path / "payload.zip").write_bytes(payload.getvalue()) + suffix = uuid.uuid4().hex[:8] + artifact = EnvironmentArtifact.put( + id=f"payload-{suffix}", environment_name="filesystem", + file_artifact=FileArtifact.put(id=f"payload-{suffix}-zip", description="payload", file_path=str(tmp_path / "payload.zip")), + ) + step = LoadArtifactTaskStep( + id="load", version=None, artifact_id=artifact.id, sandbox_name="box", container_name=sandbox.container_name, + destination_path="/app/my files", + ) + + await step.execute(context) + + for path, text in (("/app/my files/docs/readme.md", "root doc\n"), ("/app/my files/notes/a.txt", "from data.json\n")): + shown = subprocess.run(["docker", "exec", sandbox.container_name, "cat", path], capture_output=True, text=True) + assert (shown.returncode, shown.stdout) == (0, text) + + +@pytest.mark.asyncio +async def test_the_unit_tests_verifier_runs_in_a_local_container(local_agent): + sandbox, context = local_agent + _as_deployed_container(sandbox, context) + _write_in_container(sandbox, "/app/data/hello.txt", "hello") + step = RunContainerUnitTestsVerifierTaskStep( + id="verify", version=None, sandbox_name="box", container_name=sandbox.container_name, + setup_commands=["mkdir -p /logs && touch /logs/setup-ran"], + command="test -f /logs/setup-ran && grep -q hello /app/data/hello.txt && echo '{\"reward\": 1}' > /logs/reward.json", + result_paths=["/logs/reward.json"], + ) + + ctx = await step.execute(context) + + entry = ctx.metadata["verifications"]["verify"] + assert (entry["exit_code"], entry["extracted_files"]) == (0, {"/logs/reward.json": {"reward": 1}}) + + +def _as_deployed_container(sandbox, context): + """Record the agent's container as a ``run_docker_container`` container on the deployed sandbox ``box``.""" + context.deployed_sandboxes.append( + DeployedSandbox(sandbox_name="box", sandbox_id=sandbox.sandbox_id, sandbox_mode="vm", sandbox_type="local"), + ) + context.metadata.setdefault("deployed_docker_containers", []).append( + {"container_name": sandbox.container_name, "sandbox_name": "box", "sandbox_id": sandbox.sandbox_id}, + ) + + def _write_in_container(sandbox, path, text): """Write ``text`` at ``path`` inside the sandbox's container, as the agent would.""" script = f"mkdir -p $(dirname {path}) && echo {text} > {path}" diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_test.py index a0e1ebf..8ecb4fc 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_test.py @@ -683,33 +683,6 @@ async def test_exec_leaves_a_docker_exec_script_naming_the_containers_app_alone( await sandbox.exec("sudo", "bash", "-c", script) - assert spawned == [("bash", "-c", script.removeprefix("sudo "))] - - -@pytest.mark.asyncio -@pytest.mark.parametrize("script, ran", [ - ("sudo docker exec -u 0 c mkdir -p /files", "docker exec -u 0 c mkdir -p /files"), - ("sudo docker cp /tmp/s/. c:/files && rm -f /tmp/s", "docker cp /tmp/s/. c:/files && rm -f /tmp/s"), - ("set -e\nsudo docker ps", "set -e\ndocker ps"), - ("a; sudo b | sudo tee f && v=$(sudo cat g)", "a; b | tee f && v=$(cat g)"), -]) -async def test_exec_drops_sudo_a_script_runs_as_a_command_on_this_host(spawned, script, ran): - await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("sudo", "bash", "-c", script) - - assert spawned == [("bash", "-c", ran)] - - -@pytest.mark.asyncio -@pytest.mark.parametrize("script", [ - "docker exec c sudo ls", - "echo pseudo sudoers", - "docker exec c bash -c 'make; sudo make install'", - "docker exec c bash -c 'it'\"'\"'s; sudo ls'", - 'echo "a; sudo b"', -]) -async def test_exec_keeps_sudo_that_is_not_a_command_on_this_host(spawned, script): - await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("bash", "-c", script) - assert spawned == [("bash", "-c", script)] From a501be2f189a4979380a044a2233d0f1fa08a09c Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 5 Oct 2026 23:43:05 -0700 Subject: [PATCH 4/6] Pin a fully quoted docker cp container argument keeping /app Co-Authored-By: Claude Opus 5.5 (1M context) --- tst/unit/providers/sandbox_providers/local_sandbox_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_test.py index 8ecb4fc..7a2768e 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_test.py @@ -691,6 +691,8 @@ async def test_exec_leaves_a_docker_exec_script_naming_the_containers_app_alone( ("docker cp /tmp/x/. agent-local-1:/app/files", "docker cp /tmp/x/. agent-local-1:/app/files"), ("docker cp agent-local-1:/app/out.txt /app/out.txt", "docker cp agent-local-1:/app/out.txt /tmp/agent-env-work/out.txt"), ("docker cp /tmp/x c:'/app/a b'", "docker cp /tmp/x c:'/app/a b'"), + ("docker cp /tmp/x 'agent-local-1:/app/files'", "docker cp /tmp/x 'agent-local-1:/app/files'"), + ('docker cp /tmp/x "agent-local-1:/app/files"', 'docker cp /tmp/x "agent-local-1:/app/files"'), ("docker run -v /app/h:/app/c img", "docker run -v /tmp/agent-env-work/h:/app/c img"), ("docker run --volume=data:/app img", "docker run --volume=data:/app img"), ]) From 2d6b92a0de4f7576dd63238c833c23d47a562240 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 6 Oct 2026 06:17:20 -0700 Subject: [PATCH 5/6] Leave a docker cp script alone, as a docker exec script is Finding the container side of a copy or mount in script text kept missing forms. A script that starts with `docker cp` is now treated like one that starts with `docker exec`: the /app it names is the container's, and the host side of these copies is a VM temp path. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/local_sandbox.py | 9 +++---- .../sandbox_providers/local_sandbox_test.py | 24 +++++-------------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/local_sandbox.py b/src/agent_env/providers/sandbox_providers/local_sandbox.py index 2a5905f..3d4a050 100644 --- a/src/agent_env/providers/sandbox_providers/local_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/local_sandbox.py @@ -43,13 +43,11 @@ logger = logging.getLogger(__name__) _APP_PATH_PATTERN = re.compile(r"(? bool: - """A ``docker exec``, as arguments or as a ``bash -c`` script: its /app is the container's.""" + """A ``docker exec``, as arguments or as a ``bash -c`` script, or a ``docker cp`` script: its /app is the container's.""" return cmd[:2] == ["docker", "exec"] or ( cmd[:2] == ["bash", "-c"] and len(cmd) > 2 and bool(_IN_CONTAINER_SCRIPT.match(cmd[2])) ) @@ -188,8 +186,7 @@ def _rewrite_app_arg(self, arg: str) -> str: return arg def _rewrite_app_script(self, script: str) -> str: - in_container = {m.end() for m in _CONTAINER_APP.finditer(script)} - return _APP_PATH_PATTERN.sub(lambda m: m[0] if m.start() in in_container else str(self._work_dir), script) + return _APP_PATH_PATTERN.sub(lambda _: str(self._work_dir), script) async def terminate(self) -> None: """Tear down whatever this sandbox is running. diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_test.py index 7a2768e..0f88596 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_test.py @@ -687,27 +687,15 @@ async def test_exec_leaves_a_docker_exec_script_naming_the_containers_app_alone( @pytest.mark.asyncio -@pytest.mark.parametrize("script, ran", [ - ("docker cp /tmp/x/. agent-local-1:/app/files", "docker cp /tmp/x/. agent-local-1:/app/files"), - ("docker cp agent-local-1:/app/out.txt /app/out.txt", "docker cp agent-local-1:/app/out.txt /tmp/agent-env-work/out.txt"), - ("docker cp /tmp/x c:'/app/a b'", "docker cp /tmp/x c:'/app/a b'"), - ("docker cp /tmp/x 'agent-local-1:/app/files'", "docker cp /tmp/x 'agent-local-1:/app/files'"), - ('docker cp /tmp/x "agent-local-1:/app/files"', 'docker cp /tmp/x "agent-local-1:/app/files"'), - ("docker run -v /app/h:/app/c img", "docker run -v /tmp/agent-env-work/h:/app/c img"), - ("docker run --volume=data:/app img", "docker run --volume=data:/app img"), +@pytest.mark.parametrize("script", [ + "docker cp /tmp/x/. agent-local-1:/app/files", + "docker cp /tmp/x 'agent-local-1:/app/a b' && rm -f /tmp/x", + "docker cp agent-local-1:/app/out.txt /tmp/out.txt", ]) -async def test_exec_leaves_the_container_side_of_a_copy_or_mount_alone(spawned, script, ran): +async def test_exec_leaves_a_docker_cp_script_naming_the_containers_app_alone(spawned, script): await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("bash", "-c", script) - assert spawned == [("bash", "-c", ran)] - - -@pytest.mark.parametrize("script, ran", [ - ("PATH=$PATH:/app/bin tool", "PATH=$PATH:/tmp/agent-env-work/bin tool"), - ("docker cp /tmp/x c:/files && PATH=$PATH:/app/bin t", "docker cp /tmp/x c:/files && PATH=$PATH:/tmp/agent-env-work/bin t"), -]) -def test_rewrite_app_script_rewrites_a_host_path_after_a_colon(script, ran): - assert LocalSandbox(work_dir=Path("/tmp/agent-env-work"))._rewrite_app_script(script) == ran + assert spawned == [("bash", "-c", script)] @pytest.mark.parametrize("script", ["ls ~/app/x", "cat ${HOME}/app/x", "cat $(pwd)/app/x", "cat $APP/app/x"]) From ab5167368e8e9b0e3d48038e786e9dba634cf450 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 6 Oct 2026 06:37:39 -0700 Subject: [PATCH 6/6] Pass docker cp paths as arguments, not script text Leaving a whole `docker cp` script alone broke the MCP artifact load on the local provider: it stages under the host's /app, which must still point at the work dir. The five copies now go through VmSandbox.docker_cp, which hands both paths to a fixed script as arguments. The local sandbox already maps an argument that starts with /app and leaves `container:/app/...` alone, so it maps the host side and never the container's, without reading script text. LocalSandbox is back to main's. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/env/envs/mcp_server.py | 2 +- .../sandbox_providers/local_sandbox.py | 4 +- .../providers/sandbox_providers/sandbox.py | 13 +++++- .../task_step/task_steps/load_artifact.py | 11 +---- .../run_container_unit_tests_verifier.py | 5 +-- .../sandbox_providers/local_sandbox_test.py | 13 +++--- .../sandbox_provider_test.py | 5 +++ .../sandbox_providers/vm_sandbox_test.py | 42 +++++++++++++++++-- tst/unit/task_step/test_load_artifact_file.py | 3 ++ tst/util/exec_scripts.py | 9 ++++ 10 files changed, 80 insertions(+), 27 deletions(-) create mode 100644 tst/util/exec_scripts.py diff --git a/src/agent_env/env/envs/mcp_server.py b/src/agent_env/env/envs/mcp_server.py index c7db144..c80a7aa 100644 --- a/src/agent_env/env/envs/mcp_server.py +++ b/src/agent_env/env/envs/mcp_server.py @@ -222,7 +222,7 @@ async def _copy_artifact_into_container(self, file_artifact) -> str: await self._sandbox.load_s3_file(file_artifact.object_url, vm_temp_path) container_id = await self._env_provider._get_container_id(self._sandbox, self.environment_name) await self._sandbox.exec_script(f"docker exec {container_id} mkdir -p /data") - await self._sandbox.exec_script(f"docker cp {vm_temp_path} {container_id}:{container_path}") + await self._sandbox.docker_cp(vm_temp_path, f"{container_id}:{container_path}") await self._sandbox.exec_script(f"rm -f {vm_temp_path}") return container_path diff --git a/src/agent_env/providers/sandbox_providers/local_sandbox.py b/src/agent_env/providers/sandbox_providers/local_sandbox.py index 3d4a050..51c589e 100644 --- a/src/agent_env/providers/sandbox_providers/local_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/local_sandbox.py @@ -43,11 +43,11 @@ logger = logging.getLogger(__name__) _APP_PATH_PATTERN = re.compile(r"(? bool: - """A ``docker exec``, as arguments or as a ``bash -c`` script, or a ``docker cp`` script: its /app is the container's.""" + """A ``docker exec``, as arguments or as a ``bash -c`` script: its /app is the container's.""" return cmd[:2] == ["docker", "exec"] or ( cmd[:2] == ["bash", "-c"] and len(cmd) > 2 and bool(_IN_CONTAINER_SCRIPT.match(cmd[2])) ) diff --git a/src/agent_env/providers/sandbox_providers/sandbox.py b/src/agent_env/providers/sandbox_providers/sandbox.py index fd2e939..dd259aa 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox.py +++ b/src/agent_env/providers/sandbox_providers/sandbox.py @@ -326,11 +326,22 @@ async def _remove_vm_temp_file(self, *vm_paths: str) -> None: except Exception as e: logger.warning(f"Best-effort cleanup of {', '.join(vm_paths)} failed (ignored): {e}") + async def docker_cp(self, source: str, destination: str, *, remove_source: bool = False) -> None: + """``docker cp source destination``, one side ``container:path``. The paths go as arguments, not + script text, so a sandbox that maps its paths (the local one maps /app) maps only the host side. + ``remove_source`` deletes the copied host file in the same exec.""" + script = 'docker cp "$1" "$2"' + (' && rm -f "$1"' if remove_source else "") + exit_code, stdout, stderr = await self.exec_with_output("sudo", "bash", "-c", script, "docker-cp", source, destination) + if exit_code != 0: + raise RuntimeError( + f"docker cp {source} {destination} failed (exit {exit_code}):\nstdout: {stdout[-1500:]}\nstderr: {stderr[-1500:]}" + ) + async def _copy_into_container(self, vm_path: str, destination_path: str) -> None: parent = os.path.dirname(destination_path) if parent: await self.exec_script(f"docker exec -u 0 {shlex.quote(self.container_name)} mkdir -p {shlex.quote(parent)}") - await self.exec_script(f"docker cp {shlex.quote(vm_path)} {self.container_name}:{shlex.quote(destination_path)}") + await self.docker_cp(vm_path, f"{self.container_name}:{destination_path}") @staticmethod def _staging_path(kind: str, destination_path: str) -> str: diff --git a/src/agent_env/task_step/task_steps/load_artifact.py b/src/agent_env/task_step/task_steps/load_artifact.py index 12b70eb..cb3cfc1 100644 --- a/src/agent_env/task_step/task_steps/load_artifact.py +++ b/src/agent_env/task_step/task_steps/load_artifact.py @@ -664,10 +664,7 @@ async def _stage_environment_payload_into_container( await sandbox.exec_script( f"docker exec -u 0 {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" ) - await sandbox.exec_script( - f"docker cp {shlex.quote(vm_stage)}/. " - f"{shlex.quote(container_name)}:{shlex.quote(destination)}" - ) + await sandbox.docker_cp(f"{vm_stage}/.", f"{container_name}:{destination}") finally: try: await sandbox.exec_script(f"rm -rf {shlex.quote(vm_payload)} {shlex.quote(vm_stage)}") @@ -766,10 +763,6 @@ async def _load_universe_into_container(sandbox, container_name: str, universe, await sandbox.exec_script( f"docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(parent)}" ) - await sandbox.exec_script( - f"docker cp {shlex.quote(vm_temp)} " - f"{shlex.quote(container_name)}:{shlex.quote(dest_path)} && " - f"rm -f {shlex.quote(vm_temp)}" - ) + await sandbox.docker_cp(vm_temp, f"{container_name}:{dest_path}", remove_source=True) loaded.append(filename) return loaded diff --git a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py index 01cc5c1..16f3eba 100644 --- a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py +++ b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py @@ -428,10 +428,7 @@ def _upload_text_artifact(text: str, artifact_id: str, description: str, s3_url: async def _extract_file(self, sandbox, path_in_container: str) -> str: """`docker cp` a file out of the container, read it from the VM, return text.""" vm_temp = f"/tmp/_verifier_out_{uuid.uuid4().hex[:8]}" - await sandbox.exec_script( - f"docker cp {shlex.quote(self.container_name)}:{shlex.quote(path_in_container)} " - f"{shlex.quote(vm_temp)}" - ) + await sandbox.docker_cp(f"{self.container_name}:{path_in_container}", vm_temp) try: exit_code, stdout, stderr = await sandbox.exec_with_output("sudo", "cat", vm_temp) if exit_code != 0: diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_test.py index 0f88596..d6e22fa 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_test.py @@ -687,15 +687,14 @@ async def test_exec_leaves_a_docker_exec_script_naming_the_containers_app_alone( @pytest.mark.asyncio -@pytest.mark.parametrize("script", [ - "docker cp /tmp/x/. agent-local-1:/app/files", - "docker cp /tmp/x 'agent-local-1:/app/a b' && rm -f /tmp/x", - "docker cp agent-local-1:/app/out.txt /tmp/out.txt", +@pytest.mark.parametrize("source, destination, ran", [ + ("/app/_artifact_staging/x", "c:/app/x", ("/tmp/agent-env-work/_artifact_staging/x", "c:/app/x")), + ("c:/app/out.txt", "/app/out.txt", ("c:/app/out.txt", "/tmp/agent-env-work/out.txt")), ]) -async def test_exec_leaves_a_docker_cp_script_naming_the_containers_app_alone(spawned, script): - await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).exec("bash", "-c", script) +async def test_docker_cp_points_only_the_host_side_at_the_work_dir(spawned, source, destination, ran): + await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).docker_cp(source, destination) - assert spawned == [("bash", "-c", script)] + assert spawned == [("bash", "-c", 'docker cp "$1" "$2"', "docker-cp", *ran)] @pytest.mark.parametrize("script", ["ls ~/app/x", "cat ${HOME}/app/x", "cat $(pwd)/app/x", "cat $APP/app/x"]) diff --git a/tst/unit/providers/sandbox_providers/sandbox_provider_test.py b/tst/unit/providers/sandbox_providers/sandbox_provider_test.py index a8b2013..baea342 100644 --- a/tst/unit/providers/sandbox_providers/sandbox_provider_test.py +++ b/tst/unit/providers/sandbox_providers/sandbox_provider_test.py @@ -15,6 +15,7 @@ from agent_env.providers.sandbox_providers.sandbox import Sandbox, VmSandbox from agent_env.providers.sandbox_providers.sandbox_provider import _BUILTIN_SANDBOX_PROVIDERS, SandboxProvider, build_sandbox_provider from agent_env.store import ImageStore, RegistryAuth +from tst.util.exec_scripts import script_run class _FakeSandbox(Sandbox): @@ -313,6 +314,10 @@ async def exec_script(self, script: str, *, max_retries: int = 0) -> str: self.scripts.append(script) return "" + async def exec_with_output(self, *args): + self.scripts.append(script_run(args)) + return 0, "", "" + @pytest.mark.asyncio async def test_load_s3_file_curl_retries_dns_failures(): diff --git a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py index 775125b..75ae6d8 100644 --- a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py @@ -10,6 +10,7 @@ from agent_env.providers.sandbox_providers.sandbox import VmSandbox from agent_env.store import set_object_store from agent_env.config import reset_config +from tst.util.exec_scripts import script_run class _SigningStore: @@ -49,7 +50,7 @@ async def exec_with_output(self, *args): if args[:2] == ("sudo", "docker") and "images" in args: return 0, self._images_stdout, "" if args[:2] == ("sudo", "bash"): - self.scripts.append(args[-1]) + self.scripts.append(script_run(args)) return 0, "", "" async def write_file_from_text(self, content, destination_path): # pragma: no cover @@ -109,7 +110,7 @@ async def exec(self, *command): # pragma: no cover async def exec_with_output(self, *args): if args[:3] == ("sudo", "bash", "-c"): - self.scripts.append(args[-1]) + self.scripts.append(script_run(args)) return 0, "", "" @@ -181,7 +182,7 @@ async def exec(self, *command): # pragma: no cover async def exec_with_output(self, *args): if args[:3] == ("sudo", "bash", "-c"): - script = args[-1] + script = script_run(args) self.scripts.append(script) if script.startswith(self._fail_on): return -1, "", "" @@ -386,3 +387,38 @@ async def test_write_host_file_writes_on_the_host_in_bounded_chunks(): b64_path = "/tmp/agentenv_run_code/input.json.b64" assert _b64_from_chunk_scripts(sandbox.scripts, b64_path) == base64.b64encode(data).decode() assert sandbox.scripts[-1] == f"base64 -d {b64_path} > /tmp/agentenv_run_code/input.json && rm -f {b64_path}" + + +class _ArgsRecorder(VmSandbox): + def __init__(self, exit_code: int = 0): + self.calls: list[tuple[str, ...]] = [] + self._exit_code = exit_code + + async def terminate(self) -> None: # pragma: no cover + pass + + async def exec(self, *command): # pragma: no cover + return None + + async def exec_with_output(self, *args): + self.calls.append(args) + return self._exit_code, "", "no such container" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("remove_source, script", [ + (False, 'docker cp "$1" "$2"'), + (True, 'docker cp "$1" "$2" && rm -f "$1"'), +]) +async def test_docker_cp_passes_its_paths_as_arguments(remove_source, script): + sandbox = _ArgsRecorder() + + await sandbox.docker_cp("/tmp/a b", "c:/app/x", remove_source=remove_source) + + assert sandbox.calls == [("sudo", "bash", "-c", script, "docker-cp", "/tmp/a b", "c:/app/x")] + + +@pytest.mark.asyncio +async def test_a_failed_docker_cp_raises_with_its_stderr(): + with pytest.raises(RuntimeError, match="(?s)exit 1.*no such container"): + await _ArgsRecorder(exit_code=1).docker_cp("/tmp/a", "c:/x") diff --git a/tst/unit/task_step/test_load_artifact_file.py b/tst/unit/task_step/test_load_artifact_file.py index 32cac10..83fe8a6 100644 --- a/tst/unit/task_step/test_load_artifact_file.py +++ b/tst/unit/task_step/test_load_artifact_file.py @@ -36,6 +36,9 @@ async def exec_script(self, script, **kw): async def exec(self, *command): self.commands.append(" ".join(command)) + async def docker_cp(self, source, destination, *, remove_source=False): + self.commands.append(f"docker cp {source} {destination}") + async def load_s3_file(self, s3_url, destination_path): self.pulls.append((s3_url, destination_path)) diff --git a/tst/util/exec_scripts.py b/tst/util/exec_scripts.py new file mode 100644 index 0000000..bb54f28 --- /dev/null +++ b/tst/util/exec_scripts.py @@ -0,0 +1,9 @@ +"""What a recording fake sandbox saw a ``sudo bash -c`` exec run.""" + + +def script_run(args: tuple[str, ...]) -> str: + """The script of ``("sudo", "bash", "-c", script, name, *params)``, each ``"$n"`` replaced by its param.""" + script = args[3] + for n, param in enumerate(args[5:], 1): + script = script.replace(f'"${n}"', param) + return script