Skip to content

Commit 492fbd0

Browse files
committed
fix: #2962 normalize sandbox paths and add Windows CI
1 parent da82b2c commit 492fbd0

9 files changed

Lines changed: 213 additions & 43 deletions

File tree

.github/workflows/tests.yml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,33 @@ jobs:
104104
if: steps.changes.outputs.run != 'true'
105105
run: echo "Skipping tests for non-code changes."
106106

107+
tests-windows:
108+
runs-on: windows-latest
109+
env:
110+
OPENAI_API_KEY: fake-for-tests
111+
steps:
112+
- name: Checkout repository
113+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
114+
- name: Detect code changes
115+
id: changes
116+
shell: bash
117+
run: ./.github/scripts/detect-changes.sh code "${{ github.event.pull_request.base.sha || github.event.before }}" "${{ github.sha }}"
118+
- name: Setup uv
119+
if: steps.changes.outputs.run == 'true'
120+
uses: astral-sh/setup-uv@cec208311dfd045dd5311c1add060b2062131d57
121+
with:
122+
enable-cache: true
123+
python-version: "3.14"
124+
- name: Install dependencies
125+
if: steps.changes.outputs.run == 'true'
126+
run: uv sync --all-extras --all-packages --group dev
127+
- name: Run tests
128+
if: steps.changes.outputs.run == 'true'
129+
run: uv run pytest
130+
- name: Skip tests
131+
if: steps.changes.outputs.run != 'true'
132+
run: echo "Skipping tests for non-code changes."
133+
107134
build-docs:
108135
runs-on: ubuntu-latest
109136
env:

src/agents/extensions/sandbox/modal/sandbox.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,8 @@ async def _ensure_backend_started(self) -> None:
418418

419419
async def _prepare_backend_workspace(self) -> None:
420420
# Ensure workspace root exists before the base workspace flow needs it.
421-
await self.exec("mkdir", "-p", "--", str(Path(self.state.manifest.root)), shell=False)
421+
root = self._workspace_path_policy().sandbox_root().as_posix()
422+
await self.exec("mkdir", "-p", "--", root, shell=False)
422423

423424
async def _after_start(self) -> None:
424425
self._running = True

src/agents/sandbox/session/base_sandbox_session.py

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import shutil
77
import tempfile
88
from collections.abc import Awaitable, Callable, Mapping, Sequence
9-
from pathlib import Path
9+
from pathlib import Path, PurePath, PurePosixPath
1010
from typing import Literal, TypeVar, cast
1111

1212
from typing_extensions import Self
@@ -109,7 +109,7 @@ class BaseSandboxSession(abc.ABC):
109109
_runtime_persist_workspace_skip_relpaths: set[Path] | None = None
110110
_pre_stop_hooks: list[Callable[[], Awaitable[None]]] | None = None
111111
_pre_stop_hooks_ran: bool = False
112-
_runtime_helpers_installed: set[Path] | None = None
112+
_runtime_helpers_installed: set[PurePath] | None = None
113113
_runtime_helper_cache_key: object = _RUNTIME_HELPER_CACHE_KEY_UNSET
114114
_workspace_path_policy_cache: (
115115
tuple[str, tuple[tuple[str, bool], ...], WorkspacePathPolicy] | None
@@ -631,7 +631,7 @@ def _sync_runtime_helper_install_cache(self) -> None:
631631
self._runtime_helpers_installed = None
632632
self._runtime_helper_cache_key = current_key
633633

634-
async def _ensure_runtime_helper_installed(self, helper: RuntimeHelperScript) -> Path:
634+
async def _ensure_runtime_helper_installed(self, helper: RuntimeHelperScript) -> PurePath:
635635
self._sync_runtime_helper_install_cache()
636636
installed = self._runtime_helpers_installed
637637
if installed is None:
@@ -701,20 +701,20 @@ async def _validate_remote_path_access(
701701
target, while still rejecting paths whose resolved remote target escapes all allowed roots.
702702
"""
703703

704-
original_path = Path(path)
705-
root = Path(self.state.manifest.root)
704+
original_path = PurePosixPath(path.as_posix() if isinstance(path, PurePath) else path)
706705
path_policy = self._workspace_path_policy()
707-
workspace_path = path_policy.normalize_path(original_path, for_write=for_write)
706+
root = path_policy.sandbox_root()
707+
workspace_path = path_policy.normalize_sandbox_path(original_path, for_write=for_write)
708708
helper_path = await self._ensure_runtime_helper_installed(RESOLVE_WORKSPACE_PATH_HELPER)
709709
extra_grant_args = tuple(
710710
arg
711711
for root, read_only in path_policy.extra_path_grant_rules()
712-
for arg in (str(root), "1" if read_only else "0")
712+
for arg in (root.as_posix(), "1" if read_only else "0")
713713
)
714714
command = (
715715
str(helper_path),
716-
str(root),
717-
str(workspace_path),
716+
root.as_posix(),
717+
workspace_path.as_posix(),
718718
"1" if for_write else "0",
719719
*extra_grant_args,
720720
)
@@ -724,12 +724,12 @@ async def _validate_remote_path_access(
724724
if resolved:
725725
# Preserve the requested workspace path so leaf symlinks keep their normal
726726
# semantics while the remote realpath check still enforces path confinement.
727-
return workspace_path
727+
return cast(Path, workspace_path)
728728
raise ExecTransportError(
729729
command=(
730730
"resolve_workspace_path",
731-
str(root),
732-
str(workspace_path),
731+
root.as_posix(),
732+
workspace_path.as_posix(),
733733
"1" if for_write else "0",
734734
*extra_grant_args,
735735
),
@@ -746,7 +746,7 @@ async def _validate_remote_path_access(
746746
)
747747
if result.exit_code == 111:
748748
raise InvalidManifestPathError(
749-
rel=original_path,
749+
rel=original_path.as_posix(),
750750
reason=reason,
751751
context={
752752
"resolved_path": result.stderr.decode("utf-8", errors="replace").strip(),
@@ -762,13 +762,13 @@ async def _validate_remote_path_access(
762762
context["grant_path"] = line.removeprefix("read-only extra path grant: ")
763763
elif line.startswith("resolved path: "):
764764
context["resolved_path"] = line.removeprefix("resolved path: ")
765-
raise WorkspaceArchiveWriteError(path=workspace_path, context=context)
765+
raise WorkspaceArchiveWriteError(path=Path(workspace_path.as_posix()), context=context)
766766
raise ExecNonZeroError(
767767
result,
768768
command=(
769769
"resolve_workspace_path",
770-
str(root),
771-
str(workspace_path),
770+
root.as_posix(),
771+
workspace_path.as_posix(),
772772
"1" if for_write else "0",
773773
*extra_grant_args,
774774
),

src/agents/sandbox/session/runtime_helpers.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
import hashlib
44
from dataclasses import dataclass
5-
from pathlib import Path
5+
from pathlib import PurePath, PurePosixPath
66
from typing import Final
77

8-
_HELPER_INSTALL_ROOT: Final[Path] = Path("/tmp/openai-agents/bin")
8+
_HELPER_INSTALL_ROOT: Final[PurePosixPath] = PurePosixPath("/tmp/openai-agents/bin")
99
_INSTALL_MARKER: Final[str] = "INSTALL_RUNTIME_HELPER_V1"
1010

1111
_RESOLVE_WORKSPACE_PATH_SCRIPT: Final[str] = """
@@ -249,7 +249,7 @@
249249
class RuntimeHelperScript:
250250
name: str
251251
content: str
252-
install_path: Path
252+
install_path: PurePath
253253
install_marker: str = _INSTALL_MARKER
254254

255255
@classmethod

src/agents/sandbox/workspace_paths.py

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class SandboxPathGrant(BaseModel):
3434
@field_validator("path", mode="before")
3535
@classmethod
3636
def _coerce_path(cls, value: object) -> str:
37-
if isinstance(value, Path):
37+
if isinstance(value, PurePath):
3838
return value.as_posix()
3939
if isinstance(value, str):
4040
return value
@@ -56,10 +56,11 @@ class WorkspacePathPolicy:
5656
def __init__(
5757
self,
5858
*,
59-
root: str | Path,
59+
root: str | PurePath,
6060
extra_path_grants: tuple[SandboxPathGrant, ...] = (),
6161
) -> None:
6262
self._root = Path(root)
63+
self._sandbox_root = self._coerce_posix_path(root)
6364
self._extra_path_grants = extra_path_grants
6465

6566
def absolute_workspace_path(self, path: str | Path) -> Path:
@@ -105,11 +106,31 @@ def normalize_path(
105106
if resolve_symlinks:
106107
result, grant = self._resolved_host_path_and_grant(original)
107108
else:
108-
result, grant = self._sandbox_path_and_grant(original)
109+
sandbox_result, grant = self._sandbox_path_and_grant(self._coerce_posix_path(original))
110+
result = Path(sandbox_result.as_posix())
109111
if for_write:
110112
self._raise_if_read_only_grant(result, grant)
111113
return result
112114

115+
def normalize_sandbox_path(
116+
self,
117+
path: str | PurePath,
118+
*,
119+
for_write: bool = False,
120+
) -> PurePosixPath:
121+
"""Return a validated POSIX path for a Unix-like remote sandbox filesystem."""
122+
123+
original = self._coerce_posix_path(path)
124+
result, grant = self._sandbox_path_and_grant(original)
125+
if for_write:
126+
self._raise_if_read_only_grant(Path(result.as_posix()), grant)
127+
return result
128+
129+
def sandbox_root(self) -> PurePosixPath:
130+
"""Return the workspace root as a POSIX path for remote sandbox commands."""
131+
132+
return self._normalized_root()
133+
113134
def _resolved_host_path_and_grant(
114135
self,
115136
original: Path,
@@ -130,18 +151,18 @@ def _resolved_host_path_and_grant(
130151

131152
def _sandbox_path_and_grant(
132153
self,
133-
original: Path,
134-
) -> tuple[Path, SandboxPathGrant | None]:
154+
original: PurePath,
155+
) -> tuple[PurePosixPath, SandboxPathGrant | None]:
135156
normalized = (
136157
self._absolute_posix_path(original)
137158
if original.is_absolute()
138159
else self._absolute_workspace_posix_path(original)
139160
)
140161
if self._is_under(normalized, self._normalized_root()):
141-
return Path(str(normalized)), None
162+
return normalized, None
142163
grant = self._matching_grant(normalized)
143164
if original.is_absolute() and grant is not None:
144-
return Path(str(normalized)), grant
165+
return normalized, grant
145166
raise self._invalid_path_error(original)
146167

147168
def _raise_if_read_only_grant(
@@ -159,17 +180,17 @@ def _raise_if_read_only_grant(
159180
},
160181
)
161182

162-
def extra_path_grant_rules(self) -> tuple[tuple[Path, bool], ...]:
183+
def extra_path_grant_rules(self) -> tuple[tuple[PurePosixPath, bool], ...]:
163184
"""Return normalized extra grant roots and access modes for remote realpath checks."""
164185

165-
rules: list[tuple[Path, bool]] = []
186+
rules: list[tuple[PurePosixPath, bool]] = []
166187
for grant in self._extra_path_grants:
167-
root = Path(grant.path)
188+
root = self._coerce_posix_path(grant.path)
168189
_raise_if_filesystem_root(root)
169190
rules.append((root, grant.read_only))
170191
return tuple(rules)
171192

172-
def _absolute_workspace_posix_path(self, path: Path) -> PurePosixPath:
193+
def _absolute_workspace_posix_path(self, path: PurePath) -> PurePosixPath:
173194
normalized = self._absolute_posix_path(path)
174195
root = self._normalized_root()
175196
try:
@@ -178,13 +199,13 @@ def _absolute_workspace_posix_path(self, path: Path) -> PurePosixPath:
178199
raise self._invalid_path_error(path, cause=exc) from exc
179200
return normalized
180201

181-
def _absolute_posix_path(self, path: Path) -> PurePosixPath:
202+
def _absolute_posix_path(self, path: PurePath) -> PurePosixPath:
182203
root = self._normalized_root()
183204
raw_candidate = path.as_posix() if path.is_absolute() else str(root / path.as_posix())
184205
return PurePosixPath(posixpath.normpath(str(raw_candidate)))
185206

186207
def _normalized_root(self) -> PurePosixPath:
187-
return PurePosixPath(posixpath.normpath(self._root.as_posix()))
208+
return PurePosixPath(posixpath.normpath(self._sandbox_root.as_posix()))
188209

189210
def _matching_grant(
190211
self,
@@ -212,11 +233,17 @@ def _is_under(path: PurePath, root: PurePath) -> bool:
212233

213234
def _invalid_path_error(
214235
self,
215-
path: Path,
236+
path: PurePath,
216237
*,
217238
cause: BaseException | None = None,
218239
) -> InvalidManifestPathError:
219240
reason: Literal["absolute", "escape_root"] = (
220241
"absolute" if path.is_absolute() else "escape_root"
221242
)
222-
return InvalidManifestPathError(rel=path, reason=reason, cause=cause)
243+
return InvalidManifestPathError(rel=path.as_posix(), reason=reason, cause=cause)
244+
245+
@staticmethod
246+
def _coerce_posix_path(path: str | PurePath) -> PurePosixPath:
247+
if isinstance(path, PurePath):
248+
path = path.as_posix()
249+
return PurePosixPath(posixpath.normpath(path))

tests/conftest.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from __future__ import annotations
22

3+
import sys
4+
35
import pytest
46

57
from agents.models import _openai_shared
@@ -11,6 +13,27 @@
1113

1214
from .testing_processor import SPAN_PROCESSOR_TESTING
1315

16+
collect_ignore: list[str] = []
17+
18+
if sys.platform == "win32":
19+
collect_ignore.extend(
20+
[
21+
"test_example_workflows.py",
22+
"test_run_state.py",
23+
"test_sandbox_memory.py",
24+
"sandbox/capabilities/test_filesystem_capability.py",
25+
"sandbox/integration_tests/test_runner_pause_resume.py",
26+
"sandbox/test_client_options.py",
27+
"sandbox/test_exposed_ports.py",
28+
"sandbox/test_extract.py",
29+
"sandbox/test_runtime.py",
30+
"sandbox/test_session_manager.py",
31+
"sandbox/test_session_sinks.py",
32+
"sandbox/test_snapshot.py",
33+
"sandbox/test_unix_local.py",
34+
]
35+
)
36+
1437

1538
# This fixture will run once before any tests are executed
1639
@pytest.fixture(scope="session", autouse=True)

0 commit comments

Comments
 (0)