Skip to content

Commit 68ee1e5

Browse files
committed
fix
1 parent 57756db commit 68ee1e5

4 files changed

Lines changed: 109 additions & 10 deletions

File tree

src/agents/sandbox/entries/base.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ def resolve_workspace_path(
4141
raise InvalidManifestPathError(rel=rel_path.as_posix(), reason="absolute")
4242
rel_path = PurePosixPath(posixpath.normpath(rel_path.as_posix()))
4343
root_path = PurePosixPath(posixpath.normpath(root_path.as_posix()))
44+
host_root = Path(root_path.as_posix())
45+
if _path_exists(host_root):
46+
try:
47+
Path(rel_path.as_posix()).resolve(strict=False).relative_to(
48+
host_root.resolve(strict=False)
49+
)
50+
except ValueError as exc:
51+
raise InvalidManifestPathError(
52+
rel=rel_path.as_posix(), reason="absolute", cause=exc
53+
) from exc
4454
try:
4555
rel_path.relative_to(root_path)
4656
except ValueError as exc:
@@ -63,6 +73,13 @@ def resolve_workspace_path(
6373
return posix_path_as_path(resolved)
6474

6575

76+
def _path_exists(path: Path) -> bool:
77+
try:
78+
return path.exists()
79+
except OSError:
80+
return False
81+
82+
6683
class BaseEntry(BaseModel, abc.ABC):
6784
type: str
6885
_subclass_registry: ClassVar[dict[str, builtins.type[BaseEntry]]] = {}

src/agents/sandbox/workspace_paths.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ def sandbox_path_str(path: str | PurePath) -> str:
6464
return coerce_posix_path(path).as_posix()
6565

6666

67+
def _native_path_from_windows_absolute(path: PureWindowsPath) -> Path | None:
68+
native_path = Path(path)
69+
return native_path if native_path.is_absolute() else None
70+
71+
6772
class SandboxPathGrant(BaseModel):
6873
"""Extra absolute path access outside the sandbox workspace."""
6974

@@ -83,16 +88,14 @@ def _coerce_path(cls, value: object) -> str:
8388
@field_validator("path")
8489
@classmethod
8590
def _validate_path(cls, value: str) -> str:
91+
if windows_absolute_path(value) is not None:
92+
raise ValueError("sandbox path grant path must be POSIX absolute")
93+
8694
path = PurePosixPath(posixpath.normpath(value))
8795
if path.is_absolute():
8896
_raise_if_filesystem_root(path)
8997
return path.as_posix()
9098

91-
windows_path = PureWindowsPath(value)
92-
if windows_path.is_absolute():
93-
_raise_if_filesystem_root(windows_path)
94-
return windows_path.as_posix()
95-
9699
raise ValueError("sandbox path grant path must be absolute")
97100

98101

@@ -120,8 +123,9 @@ def absolute_workspace_path(self, path: str | PurePath) -> Path:
120123
"""
121124

122125
if (windows_path := windows_absolute_path(path)) is not None:
123-
if self._root_is_existing_host_path:
124-
result, _grant = self._resolved_host_path_and_grant(Path(windows_path))
126+
native_path = _native_path_from_windows_absolute(windows_path)
127+
if self._root_is_existing_host_path and native_path is not None:
128+
result, _grant = self._resolved_host_path_and_grant(native_path)
125129
return result
126130
raise self._invalid_path_error(windows_path)
127131
normalized = self._absolute_workspace_posix_path(coerce_posix_path(path))
@@ -161,12 +165,18 @@ def normalize_path(
161165
"""
162166

163167
if resolve_symlinks:
164-
original = Path(path)
168+
if (windows_path := windows_absolute_path(path)) is not None:
169+
original = _native_path_from_windows_absolute(windows_path)
170+
if original is None:
171+
raise self._invalid_path_error(windows_path)
172+
else:
173+
original = Path(path)
165174
result, grant = self._resolved_host_path_and_grant(original)
166175
else:
167176
if (windows_path := windows_absolute_path(path)) is not None:
168-
if self._root_is_existing_host_path:
169-
result, grant = self._resolved_host_path_and_grant(Path(windows_path))
177+
native_path = _native_path_from_windows_absolute(windows_path)
178+
if self._root_is_existing_host_path and native_path is not None:
179+
result, grant = self._resolved_host_path_and_grant(native_path)
170180
if for_write:
171181
self._raise_if_read_only_grant(result, grant)
172182
return result

tests/sandbox/test_entries.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,32 @@ def test_resolve_workspace_path_rejects_absolute_escape_after_normalization() ->
122122
assert exc_info.value.context == {"rel": "/etc/passwd", "reason": "absolute"}
123123

124124

125+
def test_resolve_workspace_path_rejects_absolute_symlink_escape_for_host_root(
126+
tmp_path: Path,
127+
) -> None:
128+
root = tmp_path / "workspace"
129+
outside = tmp_path / "outside"
130+
root.mkdir()
131+
outside.mkdir()
132+
link = root / "link"
133+
try:
134+
os.symlink(outside, link, target_is_directory=True)
135+
except (NotImplementedError, OSError) as exc:
136+
pytest.skip(f"symlink unavailable: {exc}")
137+
138+
escaped = link / "secret.txt"
139+
140+
with pytest.raises(InvalidManifestPathError) as exc_info:
141+
resolve_workspace_path(
142+
root,
143+
escaped,
144+
allow_absolute_within_root=True,
145+
)
146+
147+
assert str(exc_info.value) == f"manifest path must be relative: {escaped.as_posix()}"
148+
assert exc_info.value.context == {"rel": escaped.as_posix(), "reason": "absolute"}
149+
150+
125151
@pytest.mark.asyncio
126152
async def test_base_sandbox_session_uses_current_working_directory_for_local_file_sources(
127153
monkeypatch: pytest.MonkeyPatch,

tests/sandbox/test_workspace_paths.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,29 @@ def test_windows_drive_absolute_path_is_rejected_before_posix_coercion() -> None
303303
assert exc_info.value.context == {"rel": "C:/tmp/secret.txt", "reason": "absolute"}
304304

305305

306+
def test_existing_host_root_rejects_windows_drive_absolute_paths(tmp_path: Path) -> None:
307+
workspace = tmp_path / "workspace"
308+
workspace.mkdir()
309+
policy = WorkspacePathPolicy(root=workspace)
310+
methods: tuple[PathPolicyMethod, ...] = (
311+
lambda policy, path: policy.absolute_workspace_path(path),
312+
lambda policy, path: policy.normalize_path(path),
313+
lambda policy, path: policy.normalize_path(path, resolve_symlinks=True),
314+
)
315+
316+
for method in methods:
317+
for path in (
318+
PureWindowsPath("C:/tmp/secret.txt"),
319+
"C:\\tmp\\secret.txt",
320+
coerce_posix_path(PureWindowsPath("C:/tmp/secret.txt")),
321+
):
322+
with pytest.raises(InvalidManifestPathError) as exc_info:
323+
method(policy, path)
324+
325+
assert str(exc_info.value) == "manifest path must be relative: C:/tmp/secret.txt"
326+
assert exc_info.value.context == {"rel": "C:/tmp/secret.txt", "reason": "absolute"}
327+
328+
306329
def test_relative_path_rejects_windows_drive_absolute_path_for_host_root(
307330
tmp_path: Path,
308331
) -> None:
@@ -341,6 +364,29 @@ def test_sandbox_extra_path_grant_rules_use_posix_paths() -> None:
341364
)
342365

343366

367+
def test_extra_path_grant_rejects_windows_drive_absolute_path() -> None:
368+
for path in (
369+
PureWindowsPath("C:/tmp"),
370+
"C:\\tmp",
371+
coerce_posix_path(PureWindowsPath("C:/tmp")),
372+
):
373+
with pytest.raises(ValidationError) as exc_info:
374+
SandboxPathGrant(path=cast(Any, path))
375+
376+
errors = exc_info.value.errors(include_url=False)
377+
assert len(errors) == 1
378+
error = dict(errors[0])
379+
ctx = cast(dict[str, Any], error["ctx"])
380+
error["ctx"] = {"error": str(ctx["error"])}
381+
assert error == {
382+
"type": "value_error",
383+
"loc": ("path",),
384+
"msg": "Value error, sandbox path grant path must be POSIX absolute",
385+
"input": path,
386+
"ctx": {"error": "sandbox path grant path must be POSIX absolute"},
387+
}
388+
389+
344390
def test_manifest_serializes_extra_path_grants() -> None:
345391
manifest = Manifest(
346392
extra_path_grants=(

0 commit comments

Comments
 (0)