fix(env): stage a loaded artifact without passing its names to the shell as code

src/agent_env/env/envs/mcp_server.py

@@ -5,6 +5,7 @@
 import asyncio
 import logging
 import os
+import shlex
 import uuid
 from importlib.metadata import version as pkg_version
 from typing import TYPE_CHECKING, Any, Callable, ClassVar, Optional
@@ -24,6 +25,7 @@
 from agent_env.env.gateway import GatewayMode
 from agent_env.providers.sandbox_providers.sandbox_provider import SANDBOX_MODE_CONTAINER
 from agent_env.attribution import Attribution
+from agent_env.utils.paths import validate_file_name
 
 if TYPE_CHECKING:
     from agent_env.artifact import CliArtifact, EnvironmentArtifact, EnvironmentUniverseArtifact
@@ -231,6 +233,7 @@ async def _staged_artifact_size(self, file_artifact) -> int | None:
         return metadata.size if metadata is not None else None
 
     async def _copy_artifact_into_container(self, file_artifact) -> str:
+        validate_file_name(file_artifact.filename)
         container_path = f"/data/{file_artifact.filename}"
         if self._sandbox.mode == SANDBOX_MODE_CONTAINER:
             await self._sandbox.write_file_from_object(file_artifact.object_url, container_path)
@@ -241,12 +244,11 @@ async def _copy_artifact_into_container(self, file_artifact) -> str:
             from agent_env.providers.env_providers.constants import GATEWAY_APP_DIR
             stage_dir = f"{GATEWAY_APP_DIR}/_artifact_staging"
             vm_temp_path = f"{stage_dir}/{self.environment_name}-{uuid.uuid4().hex[:8]}-{file_artifact.filename}"
-            await self._sandbox.exec_script(f"mkdir -p {stage_dir}")
+            await self._sandbox.exec_script(f"mkdir -p {shlex.quote(stage_dir)}")
             await self._sandbox.load_object_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.docker_cp(vm_temp_path, f"{container_id}:{container_path}")
-            await self._sandbox.exec_script(f"rm -f {vm_temp_path}")
+            await self._sandbox.exec_script(f"docker exec {shlex.quote(container_id)} mkdir -p /data")
+            await self._sandbox.docker_cp(vm_temp_path, f"{container_id}:{container_path}", remove_source=True)
         return container_path
 
     async def _load_environment_artifact_legacy(self, file_artifact) -> None:

src/agent_env/env/envs/website.py

@@ -5,6 +5,7 @@
 import asyncio
 import logging
 import os
+import shlex
 import uuid
 from importlib.metadata import version as pkg_version
 from typing import TYPE_CHECKING, Any, Callable, ClassVar, Optional
@@ -22,6 +23,7 @@
 )
 from agent_env.env.gateway import GatewayMode
 from agent_env.attribution import Attribution
+from agent_env.utils.paths import validate_file_name
 if TYPE_CHECKING:
     from agent_env.bundle.authoring import AuthoringContext
     from agent_env.entity.export import ExportContext
@@ -187,17 +189,18 @@ async def load_file_artifact_universe(self, file_artifact_universe: "Any", desti
     async def _copy_artifact_into_container(self, file_artifact) -> str:
         from agent_env.providers.env_providers.constants import AGENT_ENV_WEBSITE_BACKEND_SUFFIX, DOCKER_COMPOSE_PATH
         filename = file_artifact.filename
+        validate_file_name(filename)
         backend_service = f"{self.environment_name}-{AGENT_ENV_WEBSITE_BACKEND_SUFFIX}"
         # Random suffix so concurrent loads on a shared sandbox (parallel /load-universe,
         # MCP+website sharing a service_name, repeated loads) don't race on a shared path.
         vm_path = f"/tmp/_artifact_{self.environment_name}-{uuid.uuid4().hex[:8]}-{filename}"
         container_path = f"/tmp/data/{filename}"
-        compose = f"docker compose -f {DOCKER_COMPOSE_PATH}"
+        compose = f"docker compose -f {shlex.quote(DOCKER_COMPOSE_PATH)}"
 
         await self._sandbox.load_object_file(file_artifact.object_url, vm_path)
-        await self._sandbox.exec_script(f"{compose} exec -T {backend_service} mkdir -p /tmp/data")
-        await self._sandbox.exec_script(f"{compose} cp {vm_path} {backend_service}:{container_path}")
-        await self._sandbox.exec_script(f"rm -f {vm_path}")
+        await self._sandbox.exec_script(f"{compose} exec -T {shlex.quote(backend_service)} mkdir -p /tmp/data")
+        await self._sandbox.exec_script(f"{compose} cp {shlex.quote(vm_path)} {shlex.quote(f'{backend_service}:{container_path}')}")
+        await self._sandbox.exec_script(f"rm -f {shlex.quote(vm_path)}")
         return container_path
 
     async def _load_environment_artifact_legacy(self, file_artifact) -> None:

src/agent_env/utils/paths.py

@@ -25,6 +25,12 @@ def validate_relative_filename(filename: str) -> None:
         raise ValueError(f"Filename {filename!r} must not contain '..' segments")
 
 
+def validate_file_name(filename: str) -> None:
+    """Reject anything but one plain file name: an env stages a loaded artifact at host and container paths built from it."""
+    if filename in ("", ".", "..") or "/" in filename or "\\" in filename or any(ord(c) < 32 or ord(c) == 127 for c in filename):
+        raise ValueError(f"Artifact filename {filename!r} must be a single file name, with no '/', '\\' or control characters")
+
+
 def os_reason(e: OSError) -> str:
     """What an OSError says went wrong, as a problem message names it."""
     if e.errno == errno.ELOOP:

tst/integration/env/gateway/gateway_test.py

@@ -2076,7 +2076,7 @@ async def recording_send(self, request, *args, **kwargs):
 
 @pytest.mark.asyncio
 @pytest.mark.integration
-async def test_gateway_with_agentenv_environment(agentenv_items_env):
+async def test_gateway_with_agentenv_environment(agentenv_items_env, monkeypatch, tmp_path):
     """v1 data-plane (/v1/data:*) served by AgentEnvEnvironment + decorators.
 
     Counterpart to the legacy /api/reset coverage in test_gateway_with_single_mcp_server."""
@@ -2191,6 +2191,11 @@ async def assert_tool_sees_items(session, tools_result, tool_names):
         await env.load_environment_artifact(csv_artifact)
         assert (await protocol_v1.get_data(base)).parts[0].data == {"items": ["file:text/csv:a,b\n1,2"]}
 
+        # A file name with shell syntax in it is staged and loaded as it is, and runs nothing on the sandbox host.
+        await _load_shell_named_artifact(env, "items", monkeypatch, tmp_path)
+        assert (await protocol_v1.get_data(base)).parts[0].data == {"items": ["shell-named"]}
+        assert not (tmp_path / "pwned").exists()
+
         # Environment Card extension end-to-end (urn:agentenv:set-errors/v1): a server-local
         # error-injection capability declared with @extension. Invoking it makes the named tool
         # start raising at the given rate; calls succeed before and produce errors after.
@@ -2359,7 +2364,7 @@ async def test_verify_universe_roundtrip_export_all_v1_and_fallback(multi_env_it
 
 @pytest.mark.asyncio
 @pytest.mark.integration
-async def test_gateway_with_website_env_v1(agentenv_website_env):
+async def test_gateway_with_website_env_v1(agentenv_website_env, monkeypatch, tmp_path):
     """v1 data-plane branch of WebsiteEnv.load_environment_artifact: supports_v1 -> reset + add inline,
     served by a Starlette AgentEnvStarletteApplication backend behind the gateway."""
     env = Env.get(agentenv_website_env.id, version=agentenv_website_env.version)
@@ -2403,10 +2408,32 @@ async def test_gateway_with_website_env_v1(agentenv_website_env):
         )
         await env.load_environment_artifact(csv_artifact)
         assert (await protocol_v1.get_data(base)).parts[0].data == {"items": ["file:text/csv:a,b\n1,2"]}
+
+        await _load_shell_named_artifact(env, "webitems", monkeypatch, tmp_path)
+        assert (await protocol_v1.get_data(base)).parts[0].data == {"items": ["shell-named"]}
+        assert not (tmp_path / "pwned").exists()
     finally:
         await env.close()
 
 
+async def _load_shell_named_artifact(env, environment_name: str, monkeypatch, tmp_path) -> None:
+    """Load a seed whose file name runs ``touch pwned`` if the shell parses it, from ``tmp_path`` so a run leaves it there."""
+    artifact = EnvironmentArtifact.put(
+        id=f"test-{environment_name}-shell-named-service-data",
+        environment_name=environment_name,
+        file_artifact=FileArtifact.put_bytes(
+            id=f"test-{environment_name}-shell-named-data",
+            description=f"{environment_name} seed with shell syntax in its file name",
+            filename="$(touch pwned) `touch pwned`.json",
+            content=json.dumps({"items": ["shell-named"]}).encode(),
+            content_type="application/json",
+        ),
+    )
+    with monkeypatch.context() as m:
+        m.chdir(tmp_path)
+        await env.load_environment_artifact(artifact)
+
+
 @pytest.mark.asyncio
 @pytest.mark.integration
 async def test_gateway_triggers_extension(sandbox_provider, mcp_server_envs, email_service_artifact):

tst/unit/env/envs/test_artifact_staging.py

@@ -0,0 +1,111 @@
+"""An env stages a loaded artifact's file through its sandbox's shell: a file or env name with shell syntax in it is
+staged and copied as it is and runs nothing, and a file name that isn't one plain name is refused before anything runs.
+A real LocalSandbox runs the scripts; docker is a stub that logs its arguments."""
+
+from __future__ import annotations
+
+import os
+import shlex
+from pathlib import Path
+from types import SimpleNamespace
+from unittest.mock import AsyncMock
+
+import pytest
+
+from agent_env.env.envs.mcp_server import MCPServerEnv
+from agent_env.env.envs.website import WebsiteEnv
+from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox
+
+_SHELL_NAMES = [
+    ("email", "x;touch pwned;#.json"),
+    ("email", "$(touch pwned).json"),
+    ("email", "`touch pwned`.json"),
+    ("email", "x' ; touch pwned ; '.json"),
+    ("email", "seed data.json"),
+    ("e;touch pwned;#", "seed.json"),
+]
+_NOT_ONE_NAME = ["../seed.json", "a/b.json", "a\\b.json", "..", ".", "", "seed\n.json", "seed\x00.json"]
+
+
+@pytest.fixture
+def host(tmp_path, monkeypatch):
+    log = tmp_path / "docker.log"
+    stub = tmp_path / "bin" / "docker"
+    stub.parent.mkdir()
+    stub.write_text(f'#!/bin/sh\nfor a in "$@"; do printf "%s\\n" "$a"; done >> {shlex.quote(str(log))}\necho --- >> {shlex.quote(str(log))}\n')
+    stub.chmod(0o755)
+    monkeypatch.setenv("PATH", f"{stub.parent}{os.pathsep}{os.environ['PATH']}")
+    monkeypatch.chdir(tmp_path)
+    sandbox = LocalSandbox(work_dir=tmp_path / "app")
+    sandbox.work_dir.mkdir()
+
+    async def load_object_file(object_url, path):
+        Path(sandbox._rewrite_app_arg(path)).write_bytes(b"{}")
+
+    sandbox.load_object_file = load_object_file
+
+    def docker_calls() -> list[list[str]]:
+        return [call.strip("\n").split("\n") for call in log.read_text().split("---\n")[:-1]] if log.exists() else []
+
+    return SimpleNamespace(sandbox=sandbox, root=tmp_path, docker_calls=docker_calls)
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(("environment_name", "filename"), _SHELL_NAMES)
+async def test_an_mcp_server_stages_a_shell_looking_name_as_it_is(host, environment_name, filename):
+    env = _mcp(host, environment_name)
+
+    assert await env._copy_artifact_into_container(_artifact(filename)) == f"/data/{filename}"
+
+    exec_mkdir, cp = host.docker_calls()
+    assert exec_mkdir == ["exec", "cid", "mkdir", "-p", "/data"]
+    assert cp[0] == "cp" and cp[2] == f"cid:/data/{filename}"
+    staged = Path(cp[1])
+    assert staged.parent == host.sandbox.work_dir / "_artifact_staging" and staged.name.endswith(filename)
+    assert not (host.root / "pwned").exists()
+    assert not staged.exists()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(("environment_name", "filename"), _SHELL_NAMES)
+async def test_a_website_stages_a_shell_looking_name_as_it_is(host, environment_name, filename):
+    env = _website(host, environment_name)
+
+    assert await env._copy_artifact_into_container(_artifact(filename)) == f"/tmp/data/{filename}"
+
+    exec_mkdir, cp = host.docker_calls()
+    service = f"{environment_name}-website-backend"
+    compose = ["compose", "-f", str(host.sandbox.work_dir / "docker-compose.yml")]
+    assert exec_mkdir == [*compose, "exec", "-T", service, "mkdir", "-p", "/tmp/data"]
+    assert cp[:4] == [*compose, "cp"] and cp[5] == f"{service}:/tmp/data/{filename}"
+    assert not (host.root / "pwned").exists()
+    assert cp[4].endswith(filename) and not Path(cp[4]).exists()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("make_env", [lambda h: _mcp(h, "email"), lambda h: _website(h, "email")], ids=["mcp", "website"])
+@pytest.mark.parametrize("filename", _NOT_ONE_NAME)
+async def test_a_name_that_is_not_one_plain_file_name_is_refused_before_anything_runs(host, make_env, filename):
+    with pytest.raises(ValueError, match="must be a single file name"):
+        await make_env(host)._copy_artifact_into_container(_artifact(filename))
+
+    assert host.docker_calls() == [] and list(host.sandbox.work_dir.iterdir()) == []
+
+
+def _mcp(host, environment_name: str) -> MCPServerEnv:
+    env = MCPServerEnv(id="mcp-email", version=1, docker_image_artifact=None, environment_name=environment_name)
+    env._sandbox = host.sandbox
+    env._env_provider = SimpleNamespace(_get_container_id=AsyncMock(return_value="cid"))
+    return env
+
+
+def _website(host, environment_name: str) -> WebsiteEnv:
+    image = SimpleNamespace(id="img", version=1, type="docker_image", image_name="img:1")
+    env = WebsiteEnv(id="w", version=1, backend_docker_image_artifact=image, frontend_docker_image_artifact=image,
+                     environment_name=environment_name)
+    env._sandbox = host.sandbox
+    return env
+
+
+def _artifact(filename: str) -> SimpleNamespace:
+    return SimpleNamespace(filename=filename, object_url="file:///store/seed")