Skip to content

Commit a4fe85c

Browse files
authored
fix(sandbox): reject host sources in dictionary sandbox manifests (#5100)
fix: reject host sources in dictionary sandbox manifests
1 parent 9415f7e commit a4fe85c

4 files changed

Lines changed: 111 additions & 4 deletions

File tree

‎src/agents/run_config.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,11 @@ class SandboxRunConfig:
232232
"""Explicit sandbox session state to resume from when not using `RunState` payloads."""
233233

234234
manifest: Manifest | None = None
235-
"""Optional sandbox manifest override for fresh session creation."""
235+
"""Optional sandbox manifest override for fresh session creation.
236+
237+
Dictionary inputs cannot authorize local host sources or extra path grants. Configure
238+
these on a trusted `Manifest` instance after validating the host paths in application code.
239+
"""
236240

237241
snapshot: SnapshotSpec | SnapshotBase | None = None
238242
"""Optional sandbox snapshot used for fresh session creation."""

‎src/agents/sandbox/manifest.py‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from .._config_coercion import coerce_pydantic_config
2121
from ..util._asyncio_tasks import gather_with_cancel
2222
from ._mount_security import redact_mount_validation_error_data_sync
23-
from .entries import BaseEntry, Dir, Mount, resolve_workspace_path
23+
from .entries import BaseEntry, Dir, LocalDir, LocalFile, Mount, resolve_workspace_path
2424
from .errors import InvalidManifestPathError
2525
from .manifest_render import render_manifest_description
2626
from .types import Group, User
@@ -603,4 +603,14 @@ def _coerce_manifest(value: Manifest | dict[str, Any], *, parameter_name: str) -
603603
f"{parameter_name}.extra_path_grants must be configured on a trusted "
604604
"Manifest instance, not in a dictionary"
605605
)
606-
return coerce_pydantic_config(value, Manifest, parameter_name=parameter_name)
606+
manifest = coerce_pydantic_config(value, Manifest, parameter_name=parameter_name)
607+
if isinstance(value, dict):
608+
for _, entry in manifest.iter_entries():
609+
if isinstance(entry, LocalFile) or (
610+
isinstance(entry, LocalDir) and entry.src is not None
611+
):
612+
raise TypeError(
613+
f"{parameter_name} local host sources must be configured on a trusted "
614+
"Manifest instance, not in a dictionary"
615+
)
616+
return manifest

‎src/agents/sandbox/sandbox_agent.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ class SandboxAgent(Agent[TContext]):
3535
"""
3636

3737
default_manifest: Manifest | None = None
38-
"""Default sandbox manifest for new sessions created by `Runner` sandbox execution."""
38+
"""Default sandbox manifest for new sessions created by `Runner` sandbox execution.
39+
40+
Dictionary inputs cannot authorize local host sources or extra path grants. Configure
41+
these on a trusted `Manifest` instance after validating the host paths in application code.
42+
"""
3943

4044
base_instructions: (
4145
str
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
from collections.abc import Callable
2+
from pathlib import Path
3+
from typing import Any
4+
5+
import pytest
6+
7+
from agents import RunConfig
8+
from agents.run_config import SandboxRunConfig
9+
from agents.sandbox import SandboxAgent
10+
from agents.sandbox.entries import Dir, File, LocalDir, LocalFile
11+
from agents.sandbox.manifest import Manifest
12+
13+
14+
@pytest.fixture(params=["sandbox_config", "run_config", "agent"])
15+
def configure_manifest(request: pytest.FixtureRequest) -> Callable[[Any], Manifest]:
16+
def configure(value: Any) -> Manifest:
17+
if request.param == "sandbox_config":
18+
manifest = SandboxRunConfig(manifest=value).manifest
19+
elif request.param == "run_config":
20+
config = RunConfig(sandbox={"manifest": value})
21+
assert config.sandbox is not None
22+
manifest = config.sandbox.manifest
23+
else:
24+
manifest = SandboxAgent(name="test", default_manifest=value).default_manifest
25+
assert isinstance(manifest, Manifest)
26+
return manifest
27+
28+
return configure
29+
30+
31+
@pytest.mark.parametrize("entry_type", ["local_file", "local_dir"])
32+
@pytest.mark.parametrize("nested", [False, True])
33+
def test_dictionary_manifest_rejects_host_sources(
34+
configure_manifest: Callable[[Any], Manifest], entry_type: str, nested: bool
35+
) -> None:
36+
entry: dict[str, Any] = {"type": entry_type, "src": ".env"}
37+
if nested:
38+
entry = {"type": "dir", "children": {"copied": entry}}
39+
40+
with pytest.raises(TypeError, match="must be configured on a trusted Manifest"):
41+
configure_manifest({"entries": {"output": entry}})
42+
43+
44+
def test_dictionary_manifest_rejects_typed_host_source_entries(
45+
configure_manifest: Callable[[Any], Manifest],
46+
) -> None:
47+
with pytest.raises(TypeError, match="must be configured on a trusted Manifest"):
48+
configure_manifest({"entries": {"output": Dir(children={"copied": LocalFile(src=".env")})}})
49+
50+
51+
def test_configuration_preserves_trusted_manifest_host_sources(
52+
configure_manifest: Callable[[Any], Manifest],
53+
) -> None:
54+
manifest = Manifest(
55+
entries={
56+
"file": LocalFile(src="input.txt"),
57+
"nested": Dir(children={"directory": LocalDir(src="data")}),
58+
}
59+
)
60+
61+
assert configure_manifest(manifest) is manifest
62+
file = manifest.entries["file"]
63+
assert isinstance(file, LocalFile)
64+
assert file.src == Path("input.txt")
65+
66+
67+
def test_dictionary_manifest_preserves_entries_without_host_sources(
68+
configure_manifest: Callable[[Any], Manifest],
69+
) -> None:
70+
manifest = configure_manifest(
71+
{
72+
"entries": {
73+
"nested": {
74+
"type": "dir",
75+
"children": {"hello.txt": {"type": "file", "content": "hello"}},
76+
},
77+
"empty": {"type": "local_dir"},
78+
}
79+
}
80+
)
81+
82+
directory = manifest.entries["nested"]
83+
assert isinstance(directory, Dir)
84+
file = directory.children["hello.txt"]
85+
assert isinstance(file, File)
86+
assert file.content == b"hello"
87+
empty = manifest.entries["empty"]
88+
assert isinstance(empty, LocalDir)
89+
assert empty.src is None

0 commit comments

Comments
 (0)