diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8f9847765e..694ea7e50d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -104,6 +104,33 @@ jobs: if: steps.changes.outputs.run != 'true' run: echo "Skipping tests for non-code changes." + tests-windows: + runs-on: windows-latest + env: + OPENAI_API_KEY: fake-for-tests + steps: + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + - name: Detect code changes + id: changes + shell: bash + run: ./.github/scripts/detect-changes.sh code "${{ github.event.pull_request.base.sha || github.event.before }}" "${{ github.sha }}" + - name: Setup uv + if: steps.changes.outputs.run == 'true' + uses: astral-sh/setup-uv@cec208311dfd045dd5311c1add060b2062131d57 + with: + enable-cache: true + python-version: "3.13" + - name: Install dependencies + if: steps.changes.outputs.run == 'true' + run: uv sync --all-extras --all-packages --group dev + - name: Run tests + if: steps.changes.outputs.run == 'true' + run: uv run pytest + - name: Skip tests + if: steps.changes.outputs.run != 'true' + run: echo "Skipping tests for non-code changes." + build-docs: runs-on: ubuntu-latest env: diff --git a/src/agents/extensions/sandbox/blaxel/mounts.py b/src/agents/extensions/sandbox/blaxel/mounts.py index 9b87802e1a..061dc6b458 100644 --- a/src/agents/extensions/sandbox/blaxel/mounts.py +++ b/src/agents/extensions/sandbox/blaxel/mounts.py @@ -31,6 +31,7 @@ from ....sandbox.materialization import MaterializedFile from ....sandbox.session.base_sandbox_session import BaseSandboxSession from ....sandbox.types import FileMode, Permissions +from ....sandbox.workspace_paths import sandbox_path_str logger = logging.getLogger(__name__) @@ -81,7 +82,7 @@ async def activate( _assert_blaxel_session(session) _ = base_dir mount_path = mount._resolve_mount_path(session, dest) - config = _build_mount_config(mount, mount_path=str(mount_path)) + config = _build_mount_config(mount, mount_path=mount_path.as_posix()) await _mount_bucket(session, config) return [] @@ -95,7 +96,7 @@ async def deactivate( _assert_blaxel_session(session) _ = base_dir mount_path = mount._resolve_mount_path(session, dest) - await _unmount_bucket(session, str(mount_path)) + await _unmount_bucket(session, mount_path.as_posix()) async def teardown_for_snapshot( self, @@ -105,7 +106,7 @@ async def teardown_for_snapshot( ) -> None: _assert_blaxel_session(session) _ = mount - await _unmount_bucket(session, str(path)) + await _unmount_bucket(session, sandbox_path_str(path)) async def restore_after_snapshot( self, @@ -114,7 +115,7 @@ async def restore_after_snapshot( path: Path, ) -> None: _assert_blaxel_session(session) - config = _build_mount_config(mount, mount_path=str(path)) + config = _build_mount_config(mount, mount_path=sandbox_path_str(path)) await _mount_bucket(session, config) def build_docker_volume_driver_config( @@ -606,7 +607,9 @@ def _resolve_config( message="BlaxelDriveMountStrategy requires a BlaxelDriveMount entry", context={"mount_type": mount.type}, ) - mount_path = mount.drive_mount_path or str(mount._resolve_mount_path(session, dest)) + mount_path = mount.drive_mount_path or sandbox_path_str( + mount._resolve_mount_path(session, dest) + ) return BlaxelDriveMountConfig( drive_name=mount.drive_name, mount_path=mount_path, @@ -619,7 +622,7 @@ def _effective_mount_path(mount: Mount, fallback: Path) -> str: """Return the actual mount path, preferring ``drive_mount_path`` over the manifest path.""" if isinstance(mount, BlaxelDriveMount) and mount.drive_mount_path: return mount.drive_mount_path - return str(fallback) + return sandbox_path_str(fallback) @staticmethod def _resolve_config_from_source(mount: Mount, mount_path: str) -> BlaxelDriveMountConfig: diff --git a/src/agents/extensions/sandbox/blaxel/sandbox.py b/src/agents/extensions/sandbox/blaxel/sandbox.py index a4b1ed2266..d4d27ffce5 100644 --- a/src/agents/extensions/sandbox/blaxel/sandbox.py +++ b/src/agents/extensions/sandbox/blaxel/sandbox.py @@ -65,6 +65,7 @@ retry_async, ) from ....sandbox.util.tar_utils import UnsafeTarMemberError, validate_tar_bytes +from ....sandbox.workspace_paths import coerce_posix_path, posix_path_as_path, sandbox_path_str DEFAULT_BLAXEL_WORKSPACE_ROOT = "/workspace" logger = logging.getLogger(__name__) @@ -319,7 +320,7 @@ async def start(self) -> None: # Ensure workspace root exists before BaseSandboxSession.start() materializes # the manifest. Blaxel base images run as root and do not ship a pre-created # workspace directory. - root = self.state.manifest.root + root = sandbox_path_str(self.state.manifest.root) try: await self._sandbox.process.exec( { @@ -368,7 +369,7 @@ async def mkdir( if path == Path("/"): return try: - await self._sandbox.fs.mkdir(str(path)) + await self._sandbox.fs.mkdir(sandbox_path_str(path)) except Exception as e: raise WorkspaceArchiveWriteError( path=path, @@ -377,14 +378,14 @@ async def mkdir( ) from e async def read(self, path: Path | str, *, user: str | User | None = None) -> io.IOBase: - path = Path(path) + error_path = posix_path_as_path(coerce_posix_path(path)) if user is not None: workspace_path = await self._check_read_with_exec(path, user=user) else: workspace_path = await self._validate_path_access(path) try: - data: Any = await self._sandbox.fs.read_binary(str(workspace_path)) + data: Any = await self._sandbox.fs.read_binary(sandbox_path_str(workspace_path)) if isinstance(data, str): data = data.encode("utf-8") return io.BytesIO(bytes(data)) @@ -397,8 +398,8 @@ async def read(self, path: Path | str, *, user: str | User | None = None) -> io. status = first_arg.get("status") error_str = str(e).lower() if status == 404 or "not found" in error_str or "no such file" in error_str: - raise WorkspaceReadNotFoundError(path=path, cause=e) from e - raise WorkspaceArchiveReadError(path=path, cause=e) from e + raise WorkspaceReadNotFoundError(path=error_path, cause=e) from e + raise WorkspaceArchiveReadError(path=error_path, cause=e) from e async def write( self, @@ -407,7 +408,7 @@ async def write( *, user: str | User | None = None, ) -> None: - path = Path(path) + error_path = posix_path_as_path(coerce_posix_path(path)) if user is not None: await self._check_write_with_exec(path, user=user) @@ -415,11 +416,11 @@ async def write( if isinstance(payload, str): payload = payload.encode("utf-8") if not isinstance(payload, bytes | bytearray): - raise WorkspaceWriteTypeError(path=path, actual_type=type(payload).__name__) + raise WorkspaceWriteTypeError(path=error_path, actual_type=type(payload).__name__) workspace_path = await self._validate_path_access(path, for_write=True) try: - await self._sandbox.fs.write_binary(str(workspace_path), bytes(payload)) + await self._sandbox.fs.write_binary(sandbox_path_str(workspace_path), bytes(payload)) except Exception as e: raise WorkspaceArchiveWriteError(path=workspace_path, cause=e) from e @@ -525,11 +526,11 @@ def _tar_exclude_args(self) -> list[str]: ) ) async def persist_workspace(self) -> io.IOBase: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() tar_path = f"/tmp/bl-persist-{self.state.session_id.hex}.tar" excludes = " ".join(self._tar_exclude_args()) tar_cmd = ( - f"tar {excludes} -C {shlex.quote(str(root))} -cf {shlex.quote(tar_path)} ." + f"tar {excludes} -C {shlex.quote(root.as_posix())} -cf {shlex.quote(tar_path)} ." ).strip() unmounted_mounts: list[tuple[Mount, Path]] = [] @@ -596,7 +597,7 @@ async def persist_workspace(self) -> io.IOBase: return io.BytesIO(raw) async def hydrate_workspace(self, data: io.IOBase) -> None: - root = self.state.manifest.root + root = self._workspace_root_path() tar_path = f"/tmp/bl-hydrate-{self.state.session_id.hex}.tar" payload = data.read() if isinstance(payload, str): @@ -608,7 +609,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: validate_tar_bytes(bytes(payload)) except UnsafeTarMemberError as e: raise WorkspaceArchiveWriteError( - path=Path(root), + path=root, context={ "reason": "unsafe_or_invalid_tar", "member": e.member, @@ -623,12 +624,12 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: result = await self._exec_internal( "sh", "-c", - f"tar -C {shlex.quote(root)} -xf {shlex.quote(tar_path)}", + f"tar -C {shlex.quote(root.as_posix())} -xf {shlex.quote(tar_path)}", timeout=self.state.timeouts.workspace_tar_s, ) if result.exit_code != 0: raise WorkspaceArchiveWriteError( - path=Path(root), + path=root, context={ "reason": "tar_extract_failed", "output": result.stderr.decode("utf-8", errors="replace"), @@ -637,7 +638,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: except WorkspaceArchiveWriteError: raise except Exception as e: - raise WorkspaceArchiveWriteError(path=Path(root), cause=e) from e + raise WorkspaceArchiveWriteError(path=root, cause=e) from e finally: try: await self._exec_internal( diff --git a/src/agents/extensions/sandbox/cloudflare/sandbox.py b/src/agents/extensions/sandbox/cloudflare/sandbox.py index fc7abc4889..281f6b4545 100644 --- a/src/agents/extensions/sandbox/cloudflare/sandbox.py +++ b/src/agents/extensions/sandbox/cloudflare/sandbox.py @@ -67,6 +67,7 @@ retry_async, ) from ....sandbox.util.tar_utils import UnsafeTarMemberError, validate_tar_bytes +from ....sandbox.workspace_paths import coerce_posix_path, posix_path_as_path, sandbox_path_str _DEFAULT_EXEC_TIMEOUT_S = 30.0 _DEFAULT_REQUEST_TIMEOUT_S = 120.0 @@ -345,12 +346,14 @@ async def mount_bucket( mount_path: Path | str, options: dict[str, object], ) -> None: - workspace_path = await self._validate_path_access(mount_path, for_write=True) + workspace_path = await self._validate_path_access( + coerce_posix_path(mount_path).as_posix(), for_write=True + ) http = self._session() url = self._url("mount") payload = { "bucket": bucket, - "mountPath": str(workspace_path), + "mountPath": sandbox_path_str(workspace_path), "options": options, } @@ -370,7 +373,7 @@ async def mount_bucket( message="cloudflare bucket mount failed", context={ "bucket": bucket, - "mount_path": str(workspace_path), + "mount_path": sandbox_path_str(workspace_path), "http_status": resp.status, "reason": body.get("error", f"HTTP {resp.status}"), }, @@ -382,17 +385,19 @@ async def mount_bucket( message="cloudflare bucket mount failed", context={ "bucket": bucket, - "mount_path": str(workspace_path), + "mount_path": sandbox_path_str(workspace_path), "cause_type": type(e).__name__, "reason": str(e), }, ) from e async def unmount_bucket(self, mount_path: Path | str) -> None: - workspace_path = await self._validate_path_access(mount_path, for_write=True) + workspace_path = await self._validate_path_access( + coerce_posix_path(mount_path).as_posix(), for_write=True + ) http = self._session() url = self._url("unmount") - payload = {"mountPath": str(workspace_path)} + payload = {"mountPath": sandbox_path_str(workspace_path)} try: async with http.post( @@ -409,7 +414,7 @@ async def unmount_bucket(self, mount_path: Path | str) -> None: raise MountConfigError( message="cloudflare bucket unmount failed", context={ - "mount_path": str(workspace_path), + "mount_path": sandbox_path_str(workspace_path), "http_status": resp.status, "reason": body.get("error", f"HTTP {resp.status}"), }, @@ -420,7 +425,7 @@ async def unmount_bucket(self, mount_path: Path | str) -> None: raise MountConfigError( message="cloudflare bucket unmount failed", context={ - "mount_path": str(workspace_path), + "mount_path": sandbox_path_str(workspace_path), "cause_type": type(e).__name__, "reason": str(e), }, @@ -501,10 +506,10 @@ def _handle_event_payload(data: str) -> None: async def _prepare_backend_workspace(self) -> None: try: - root = Path(self.state.manifest.root) - await self._exec_internal("mkdir", "-p", "--", str(root)) + root = self._workspace_root_path() + await self._exec_internal("mkdir", "-p", "--", root.as_posix()) except Exception as e: - raise WorkspaceStartError(path=Path(self.state.manifest.root), cause=e) from e + raise WorkspaceStartError(path=self._workspace_root_path(), cause=e) from e async def _can_reuse_restorable_snapshot_workspace(self) -> bool: if not self._workspace_state_preserved_on_start(): @@ -518,7 +523,7 @@ async def _can_reuse_restorable_snapshot_workspace(self) -> bool: return await self._can_skip_snapshot_restore_on_resume(is_running=is_running) async def _restore_snapshot_into_workspace_on_resume(self) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() detached_mounts: list[tuple[Any, Path]] = [] if self._restore_workspace_was_running: for mount_entry, mount_path in self.state.manifest.ephemeral_mount_targets(): @@ -965,13 +970,12 @@ async def pty_terminate_all(self) -> None: await self._terminate_pty_entry(entry) async def read(self, path: Path | str, *, user: str | User | None = None) -> io.IOBase: - path = Path(path) if user is not None: await self._check_read_with_exec(path, user=user) workspace_path = await self._validate_path_access(path) http = self._session() - url_path = quote(str(workspace_path).lstrip("/"), safe="/") + url_path = quote(sandbox_path_str(workspace_path).lstrip("/"), safe="/") url = self._url(f"file/{url_path}") try: @@ -1029,7 +1033,7 @@ async def write( *, user: str | User | None = None, ) -> None: - path = Path(path) + error_path = posix_path_as_path(coerce_posix_path(path)) if user is not None: await self._check_write_with_exec(path, user=user) @@ -1037,13 +1041,13 @@ async def write( if isinstance(payload, str): payload = payload.encode("utf-8") if not isinstance(payload, bytes | bytearray): - raise WorkspaceWriteTypeError(path=path, actual_type=type(payload).__name__) + raise WorkspaceWriteTypeError(path=error_path, actual_type=type(payload).__name__) payload_bytes = bytes(payload) workspace_path = await self._validate_path_access(path, for_write=True) http = self._session() - url_path = quote(str(workspace_path).lstrip("/"), safe="/") + url_path = quote(sandbox_path_str(workspace_path).lstrip("/"), safe="/") url = self._url(f"file/{url_path}") try: @@ -1106,7 +1110,7 @@ async def running(self) -> bool: or _is_transient_workspace_error(exc) ) async def _persist_workspace_via_http(self) -> io.IOBase: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() skip = self._persist_workspace_skip_relpaths() excludes_param = ",".join( rel.as_posix().removeprefix("./") @@ -1148,7 +1152,7 @@ async def _persist_workspace_via_http(self) -> io.IOBase: or _is_transient_workspace_error(exc) ) async def _hydrate_workspace_via_http(self, data: io.IOBase) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() raw = data.read() if isinstance(raw, str): raw = raw.encode("utf-8") @@ -1199,7 +1203,7 @@ async def _hydrate_workspace_via_http(self, data: io.IOBase) -> None: raise WorkspaceArchiveWriteError(path=root, cause=e) from e async def persist_workspace(self) -> io.IOBase: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() unmounted_mounts: list[tuple[Any, Path]] = [] unmount_error: WorkspaceArchiveReadError | None = None for mount_entry, mount_path in self.state.manifest.ephemeral_mount_targets(): @@ -1245,7 +1249,7 @@ async def persist_workspace(self) -> io.IOBase: return persisted async def hydrate_workspace(self, data: io.IOBase) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() unmounted_mounts: list[tuple[Any, Path]] = [] unmount_error: WorkspaceArchiveWriteError | None = None for mount_entry, mount_path in self.state.manifest.ephemeral_mount_targets(): diff --git a/src/agents/extensions/sandbox/daytona/sandbox.py b/src/agents/extensions/sandbox/daytona/sandbox.py index c28645769d..dee88710a0 100644 --- a/src/agents/extensions/sandbox/daytona/sandbox.py +++ b/src/agents/extensions/sandbox/daytona/sandbox.py @@ -63,6 +63,7 @@ retry_async, ) from ....sandbox.util.tar_utils import UnsafeTarMemberError, validate_tar_bytes +from ....sandbox.workspace_paths import coerce_posix_path, posix_path_as_path, sandbox_path_str DEFAULT_DAYTONA_WORKSPACE_ROOT = "/home/daytona/workspace" logger = logging.getLogger(__name__) @@ -375,7 +376,7 @@ async def mkdir( if path == Path("/"): return try: - await self._sandbox.fs.create_folder(str(path), "755") + await self._sandbox.fs.create_folder(sandbox_path_str(path), "755") except Exception as e: raise WorkspaceArchiveWriteError( path=path, @@ -401,7 +402,7 @@ async def _exec_internal( ) -> ExecResult: cmd_str = shlex.join(str(c) for c in command) envs = await self._resolved_envs() - cwd = self.state.manifest.root + cwd = sandbox_path_str(self.state.manifest.root) env_args = ( " ".join(shlex.quote(f"{key}={value}") for key, value in envs.items()) if envs else "" ) @@ -481,7 +482,7 @@ async def pty_exec_start( sanitized = self._prepare_exec_command(*command, shell=shell, user=user) cmd_str = shlex.join(str(part) for part in sanitized) envs = await self._resolved_envs() - cwd = self.state.manifest.root + cwd = sandbox_path_str(self.state.manifest.root) exec_timeout = self._coerce_exec_timeout(timeout) daytona_exc = _import_daytona_exceptions() timeout_exc = daytona_exc.get("timeout") @@ -782,7 +783,7 @@ async def _terminate_pty_entry(self, entry: _DaytonaPtySessionEntry) -> None: pass async def read(self, path: Path | str, *, user: str | User | None = None) -> io.IOBase: - path = Path(path) + error_path = posix_path_as_path(coerce_posix_path(path)) if user is not None: workspace_path = await self._check_read_with_exec(path, user=user) else: @@ -793,14 +794,14 @@ async def read(self, path: Path | str, *, user: str | User | None = None) -> io. try: data: bytes = await self._sandbox.fs.download_file( - str(workspace_path), + sandbox_path_str(workspace_path), self.state.timeouts.file_download_s, ) return io.BytesIO(data) except Exception as e: if not_found_exc is not None and isinstance(e, not_found_exc): - raise WorkspaceReadNotFoundError(path=path, cause=e) from e - raise WorkspaceArchiveReadError(path=path, cause=e) from e + raise WorkspaceReadNotFoundError(path=error_path, cause=e) from e + raise WorkspaceArchiveReadError(path=error_path, cause=e) from e async def write( self, @@ -809,7 +810,7 @@ async def write( *, user: str | User | None = None, ) -> None: - path = Path(path) + error_path = posix_path_as_path(coerce_posix_path(path)) if user is not None: await self._check_write_with_exec(path, user=user) @@ -817,13 +818,13 @@ async def write( if isinstance(payload, str): payload = payload.encode("utf-8") if not isinstance(payload, bytes | bytearray): - raise WorkspaceWriteTypeError(path=path, actual_type=type(payload).__name__) + raise WorkspaceWriteTypeError(path=error_path, actual_type=type(payload).__name__) workspace_path = await self._validate_path_access(path, for_write=True) try: await self._sandbox.fs.upload_file( bytes(payload), - str(workspace_path), + sandbox_path_str(workspace_path), timeout=self.state.timeouts.file_upload_s, ) except Exception as e: @@ -859,7 +860,6 @@ def _tar_exclude_args(self) -> list[str]: ) ) async def _run_persist_workspace_command(self, tar_cmd: str, tar_path: str) -> bytes: - root = self.state.manifest.root try: envs = await self._resolved_envs() result = await self._sandbox.process.exec( @@ -869,7 +869,7 @@ async def _run_persist_workspace_command(self, tar_cmd: str, tar_path: str) -> b ) if result.exit_code != 0: raise WorkspaceArchiveReadError( - path=Path(root), + path=self._workspace_root_path(), context={"reason": "tar_failed", "output": result.result or ""}, ) return cast( @@ -882,7 +882,7 @@ async def _run_persist_workspace_command(self, tar_cmd: str, tar_path: str) -> b except WorkspaceArchiveReadError: raise except Exception as e: - raise WorkspaceArchiveReadError(path=Path(root), cause=e) from e + raise WorkspaceArchiveReadError(path=self._workspace_root_path(), cause=e) from e async def persist_workspace(self) -> io.IOBase: def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: @@ -892,11 +892,11 @@ def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: summary["cause"] = str(error.cause) return summary - root = Path(self.state.manifest.root) + root = self._workspace_root_path() tar_path = f"/tmp/sandbox-persist-{self.state.session_id.hex}.tar" excludes = " ".join(self._tar_exclude_args()) tar_cmd = ( - f"tar {excludes} -C {shlex.quote(str(root))} -cf {shlex.quote(tar_path)} ." + f"tar {excludes} -C {shlex.quote(root.as_posix())} -cf {shlex.quote(tar_path)} ." ).strip() unmounted_mounts: list[tuple[Mount, Path]] = [] @@ -964,7 +964,7 @@ def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: return io.BytesIO(raw) async def hydrate_workspace(self, data: io.IOBase) -> None: - root = self.state.manifest.root + root = self._workspace_root_path() tar_path = f"/tmp/sandbox-hydrate-{self.state.session_id.hex}.tar" payload = data.read() if isinstance(payload, str): @@ -976,7 +976,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: validate_tar_bytes(bytes(payload)) except UnsafeTarMemberError as e: raise WorkspaceArchiveWriteError( - path=Path(root), + path=root, context={ "reason": "unsafe_or_invalid_tar", "member": e.member, @@ -994,19 +994,19 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: timeout=self.state.timeouts.file_upload_s, ) result = await self._sandbox.process.exec( - f"tar -C {shlex.quote(root)} -xf {shlex.quote(tar_path)}", + f"tar -C {shlex.quote(root.as_posix())} -xf {shlex.quote(tar_path)}", env=envs or None, timeout=self.state.timeouts.workspace_tar_s, ) if result.exit_code != 0: raise WorkspaceArchiveWriteError( - path=Path(root), + path=root, context={"reason": "tar_extract_failed", "output": result.result or ""}, ) except WorkspaceArchiveWriteError: raise except Exception as e: - raise WorkspaceArchiveWriteError(path=Path(root), cause=e) from e + raise WorkspaceArchiveWriteError(path=root, cause=e) from e finally: try: envs = await self._resolved_envs() diff --git a/src/agents/extensions/sandbox/e2b/sandbox.py b/src/agents/extensions/sandbox/e2b/sandbox.py index a484663627..0e8d92d1d6 100644 --- a/src/agents/extensions/sandbox/e2b/sandbox.py +++ b/src/agents/extensions/sandbox/e2b/sandbox.py @@ -73,6 +73,7 @@ retry_async, ) from ....sandbox.util.tar_utils import UnsafeTarMemberError, validate_tar_bytes +from ....sandbox.workspace_paths import posix_path_for_error, sandbox_path_str WorkspacePersistenceMode = Literal["tar", "snapshot"] E2BTimeoutAction = Literal["kill", "pause"] @@ -724,12 +725,12 @@ def _coerce_exec_timeout(self, timeout_s: float | None) -> float: async def _ensure_dir(self, path: Path, *, reason: str) -> None: """Create a directory using the E2B Files API.""" - if path == Path("/"): + if path.as_posix() == "/": return try: await _sandbox_make_dir( self._sandbox, - str(path), + sandbox_path_str(path), request_timeout=self.state.timeouts.fast_op_s, ) except Exception as e: # pragma: no cover - exercised via unit tests with fakes @@ -737,11 +738,11 @@ async def _ensure_dir(self, path: Path, *, reason: str) -> None: async def _ensure_workspace_root(self) -> None: """Ensure the workspace root exists before materialization starts.""" - await self._ensure_dir(Path(self.state.manifest.root), reason="root_make_failed") + await self._ensure_dir(self._workspace_root_path(), reason="root_make_failed") async def _prepare_workspace_root_for_exec(self) -> None: """Create the workspace root through the command API before using it as `cwd`.""" - root = str(Path(self.state.manifest.root)) + root = self._workspace_root_path().as_posix() envs = await self._resolved_envs() result = await _sandbox_run_command( self._sandbox, @@ -753,7 +754,7 @@ async def _prepare_workspace_root_for_exec(self) -> None: exit_code = int(getattr(result, "exit_code", 0) or 0) if exit_code != 0: raise WorkspaceStartError( - path=Path(self.state.manifest.root), + path=self._workspace_root_path(), context={ "reason": "workspace_root_nonzero_exit", "exit_code": exit_code, @@ -781,7 +782,7 @@ async def _prepare_backend_workspace(self) -> None: except WorkspaceStartError: raise except Exception as e: - raise WorkspaceStartError(path=Path(self.state.manifest.root), cause=e) from e + raise WorkspaceStartError(path=self._workspace_root_path(), cause=e) from e async def _after_start(self) -> None: # Native E2B snapshot hydration can replace the sandbox and sandbox id; reinstall runtime @@ -1053,7 +1054,9 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase not_found_exc = e2b_exc.get("not_found") try: - content = await _sandbox_read_file(self._sandbox, str(workspace_path), format="bytes") + content = await _sandbox_read_file( + self._sandbox, sandbox_path_str(workspace_path), format="bytes" + ) if isinstance(content, bytes | bytearray): data = bytes(content) elif isinstance(content, str): @@ -1087,7 +1090,7 @@ async def write( try: await _sandbox_write_file( self._sandbox, - str(workspace_path), + sandbox_path_str(workspace_path), bytes(payload), request_timeout=self.state.timeouts.file_upload_s, ) @@ -1239,6 +1242,7 @@ def _tar_exclude_args(self) -> list[str]: ) ) async def _run_persist_workspace_command(self, tar_cmd: str) -> str: + error_root = posix_path_for_error(self._workspace_root_path()) try: envs = await self._resolved_envs() result = await _sandbox_run_command( @@ -1251,7 +1255,7 @@ async def _run_persist_workspace_command(self, tar_cmd: str) -> str: exit_code = int(getattr(result, "exit_code", 0) or 0) if exit_code != 0: raise WorkspaceArchiveReadError( - path=Path(self.state.manifest.root), + path=error_root, context={ "reason": "snapshot_nonzero_exit", "exit_code": exit_code, @@ -1262,7 +1266,7 @@ async def _run_persist_workspace_command(self, tar_cmd: str) -> str: except WorkspaceArchiveReadError: raise except Exception as e: # pragma: no cover - exercised via unit tests with fakes - raise WorkspaceArchiveReadError(path=Path(self.state.manifest.root), cause=e) from e + raise WorkspaceArchiveReadError(path=error_root, cause=e) from e async def persist_workspace(self) -> io.IOBase: if self.state.workspace_persistence == _WORKSPACE_PERSISTENCE_SNAPSHOT: @@ -1277,7 +1281,8 @@ async def _persist_workspace_via_snapshot(self) -> io.IOBase: capture the whole sandbox and the E2B API does not provide path-level excludes. """ - root = Path(self.state.manifest.root) + root = self._workspace_root_path() + error_root = posix_path_for_error(root) if not hasattr(self._sandbox, "create_snapshot"): return await self._persist_workspace_via_tar() if self._native_snapshot_requires_tar_fallback(): @@ -1302,7 +1307,7 @@ async def _persist_workspace_via_snapshot(self) -> io.IOBase: mount_entry, self, mount_path ) except Exception as e: - unmount_error = WorkspaceArchiveReadError(path=root, cause=e) + unmount_error = WorkspaceArchiveReadError(path=error_root, cause=e) break unmounted_mounts.append((mount_entry, mount_path)) @@ -1317,7 +1322,7 @@ async def _persist_workspace_via_snapshot(self) -> io.IOBase: snapshot_id = getattr(snap, "snapshot_id", None) if not isinstance(snapshot_id, str) or not snapshot_id: raise WorkspaceArchiveReadError( - path=root, + path=error_root, context={ "reason": "native_snapshot_unexpected_return", "type": type(snap).__name__, @@ -1327,7 +1332,7 @@ async def _persist_workspace_via_snapshot(self) -> io.IOBase: snapshot_error = e except Exception as e: snapshot_error = WorkspaceArchiveReadError( - path=root, context={"reason": "native_snapshot_failed"}, cause=e + path=error_root, context={"reason": "native_snapshot_failed"}, cause=e ) remount_error: WorkspaceArchiveReadError | None = None @@ -1337,7 +1342,7 @@ async def _persist_workspace_via_snapshot(self) -> io.IOBase: mount_entry, self, mount_path ) except Exception as e: - current_error = WorkspaceArchiveReadError(path=root, cause=e) + current_error = WorkspaceArchiveReadError(path=error_root, cause=e) if remount_error is None: remount_error = current_error else: @@ -1375,9 +1380,10 @@ def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: summary["cause"] = str(error.cause) return summary - root = Path(self.state.manifest.root) + root = self._workspace_root_path() + error_root = posix_path_for_error(root) excludes = " ".join(self._tar_exclude_args()) - tar_cmd = f"tar {excludes} -C {shlex.quote(str(root))} -cf - . | base64 -w0" + tar_cmd = f"tar {excludes} -C {shlex.quote(root.as_posix())} -cf - . | base64 -w0" unmounted_mounts: list[tuple[Mount, Path]] = [] unmount_error: WorkspaceArchiveReadError | None = None for mount_entry, mount_path in self.state.manifest.ephemeral_mount_targets(): @@ -1386,7 +1392,7 @@ def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: mount_entry, self, mount_path ) except Exception as e: - unmount_error = WorkspaceArchiveReadError(path=root, cause=e) + unmount_error = WorkspaceArchiveReadError(path=error_root, cause=e) break unmounted_mounts.append((mount_entry, mount_path)) @@ -1399,7 +1405,7 @@ def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: raw = base64.b64decode(encoded.encode("utf-8"), validate=True) except (binascii.Error, ValueError) as e: raise WorkspaceArchiveReadError( - path=root, + path=error_root, context={"reason": "snapshot_invalid_base64"}, cause=e, ) from e @@ -1413,7 +1419,7 @@ def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: mount_entry, self, mount_path ) except Exception as e: - current_error = WorkspaceArchiveReadError(path=root, cause=e) + current_error = WorkspaceArchiveReadError(path=error_root, cause=e) if remount_error is None: remount_error = current_error if unmount_error is not None: @@ -1442,7 +1448,8 @@ def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: return io.BytesIO(raw) async def hydrate_workspace(self, data: io.IOBase) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() + error_root = posix_path_for_error(root) tar_path = f"/tmp/sandbox-hydrate-{self.state.session_id.hex}.tar" raw = data.read() @@ -1486,7 +1493,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: return except Exception as e: raise WorkspaceArchiveWriteError( - path=root, + path=error_root, context={ "reason": "native_snapshot_restore_failed", "snapshot_id": snapshot_id, @@ -1498,7 +1505,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: validate_tar_bytes(bytes(raw)) except UnsafeTarMemberError as e: raise WorkspaceArchiveWriteError( - path=root, + path=error_root, context={ "reason": "unsafe_or_invalid_tar", "member": e.member, @@ -1518,7 +1525,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: ) result = await _sandbox_run_command( self._sandbox, - f"tar -C {shlex.quote(str(root))} -xf {shlex.quote(tar_path)}", + f"tar -C {shlex.quote(root.as_posix())} -xf {shlex.quote(tar_path)}", timeout=self.state.timeouts.snapshot_tar_s, cwd="/", envs=envs, @@ -1526,7 +1533,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: exit_code = int(getattr(result, "exit_code", 0) or 0) if exit_code != 0: raise WorkspaceArchiveWriteError( - path=root, + path=error_root, context={ "reason": "hydrate_nonzero_exit", "exit_code": exit_code, @@ -1537,7 +1544,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: except WorkspaceArchiveWriteError: raise except Exception as e: # pragma: no cover - exercised via unit tests with fakes - raise WorkspaceArchiveWriteError(path=root, cause=e) from e + raise WorkspaceArchiveWriteError(path=error_root, cause=e) from e finally: try: envs = await self._resolved_envs() diff --git a/src/agents/extensions/sandbox/modal/sandbox.py b/src/agents/extensions/sandbox/modal/sandbox.py index 2306c28465..bb9fbc1c4b 100644 --- a/src/agents/extensions/sandbox/modal/sandbox.py +++ b/src/agents/extensions/sandbox/modal/sandbox.py @@ -73,6 +73,12 @@ retry_async, ) from ....sandbox.util.tar_utils import UnsafeTarMemberError, validate_tar_bytes +from ....sandbox.workspace_paths import ( + coerce_posix_path, + posix_path_as_path, + posix_path_for_error, + sandbox_path_str, +) from .mounts import ModalCloudBucketMountStrategy _DEFAULT_TIMEOUT_S = 30.0 @@ -418,7 +424,8 @@ async def _ensure_backend_started(self) -> None: async def _prepare_backend_workspace(self) -> None: # Ensure workspace root exists before the base workspace flow needs it. - await self.exec("mkdir", "-p", "--", str(Path(self.state.manifest.root)), shell=False) + root = self._workspace_path_policy().sandbox_root().as_posix() + await self.exec("mkdir", "-p", "--", root, shell=False) async def _after_start(self) -> None: self._running = True @@ -429,7 +436,7 @@ async def _after_start_failed(self) -> None: def _wrap_start_error(self, error: Exception) -> Exception: if isinstance(error, WorkspaceStartError): return error - return WorkspaceStartError(path=Path(self.state.manifest.root), cause=error) + return WorkspaceStartError(path=self._workspace_root_path(), cause=error) async def _resolve_exposed_port(self, port: int) -> ExposedPortEndpoint: await self._ensure_sandbox() @@ -469,7 +476,7 @@ async def _resolve_exposed_port(self, port: int) -> ExposedPortEndpoint: def _wrap_stop_error(self, error: Exception) -> Exception: if isinstance(error, WorkspaceStopError): return error - return WorkspaceStopError(path=Path(self.state.manifest.root), cause=error) + return WorkspaceStopError(path=self._workspace_root_path(), cause=error) async def _shutdown_backend(self) -> None: try: @@ -1015,7 +1022,7 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase # Read by `cat` so the payload is returned as bytes. workspace_path = await self._validate_path_access(path) - cmd = ["sh", "-lc", f"cat -- {shlex.quote(str(workspace_path))}"] + cmd = ["sh", "-lc", f"cat -- {shlex.quote(sandbox_path_str(workspace_path))}"] try: out = await self.exec(*cmd, shell=False) except ExecTimeoutError as e: @@ -1054,12 +1061,12 @@ async def write( async def _run_write() -> None: assert self._sandbox is not None # Ensure parent directory exists. - parent = str(workspace_path.parent) + parent = sandbox_path_str(workspace_path.parent) mkdir_proc = await self._sandbox.exec.aio("mkdir", "-p", "--", parent, text=False) await mkdir_proc.wait.aio() # Stream bytes into `cat > file` to avoid quoting/binary issues. - cmd = ["sh", "-lc", f"cat > {shlex.quote(str(workspace_path))}"] + cmd = ["sh", "-lc", f"cat > {shlex.quote(sandbox_path_str(workspace_path))}"] proc = await self._sandbox.exec.aio(*cmd, text=False) await _write_process_stdin(proc, payload) exit_code = await proc.wait.aio() @@ -1121,7 +1128,8 @@ async def _persist_workspace_via_snapshot_filesystem(self) -> io.IOBase: return await self._persist_workspace_via_tar() if self._native_snapshot_requires_tar_fallback(): return await self._persist_workspace_via_tar() - root = Path(self.state.manifest.root) + root = self._workspace_root_path() + error_root = posix_path_for_error(root) plain_skip = self._modal_snapshot_plain_skip_relpaths(root) skip_abs = [root / rel for rel in sorted(plain_skip, key=lambda p: p.as_posix())] self._modal_snapshot_ephemeral_backup = None @@ -1134,13 +1142,15 @@ async def restore_ephemeral_paths() -> WorkspaceArchiveReadError | None: try: assert self._sandbox is not None - proc = await self._sandbox.exec.aio("tar", "xf", "-", "-C", str(root), text=False) + proc = await self._sandbox.exec.aio( + "tar", "xf", "-", "-C", root.as_posix(), text=False + ) await _write_process_stdin(proc, bytes(backup)) exit_code = await proc.wait.aio() if exit_code != 0: stderr = await proc.stderr.read.aio() return WorkspaceArchiveReadError( - path=root, + path=error_root, context={ "reason": "snapshot_filesystem_ephemeral_restore_failed", "exit_code": exit_code, @@ -1151,7 +1161,7 @@ async def restore_ephemeral_paths() -> WorkspaceArchiveReadError | None: if isinstance(exc, WorkspaceArchiveReadError): return exc return WorkspaceArchiveReadError( - path=root, + path=error_root, context={"reason": "snapshot_filesystem_ephemeral_restore_failed"}, cause=exc, ) @@ -1159,11 +1169,14 @@ async def restore_ephemeral_paths() -> WorkspaceArchiveReadError | None: if skip_abs: rel_args = " ".join(shlex.quote(p.relative_to(root).as_posix()) for p in skip_abs) - cmd = f"cd -- {shlex.quote(str(root))} && (tar cf - -- {rel_args} 2>/dev/null || true)" + cmd = ( + f"cd -- {shlex.quote(root.as_posix())} && " + f"(tar cf - -- {rel_args} 2>/dev/null || true)" + ) out = await self.exec("sh", "-lc", cmd, shell=False) self._modal_snapshot_ephemeral_backup = out.stdout or b"" - rm_cmd = ["rm", "-rf", "--", *[str(p) for p in skip_abs]] + rm_cmd = ["rm", "-rf", "--", *[p.as_posix() for p in skip_abs]] rm_out = await self.exec(*rm_cmd, shell=False) if not rm_out.ok(): cleanup_restore_error = await restore_ephemeral_paths() @@ -1173,7 +1186,7 @@ async def restore_ephemeral_paths() -> WorkspaceArchiveReadError | None: cleanup_restore_error, ) raise WorkspaceArchiveReadError( - path=root, + path=error_root, context={ "reason": "snapshot_filesystem_ephemeral_remove_failed", "exit_code": rm_out.exit_code, @@ -1198,7 +1211,7 @@ async def restore_ephemeral_paths() -> WorkspaceArchiveReadError | None: restore_error, ) raise WorkspaceArchiveReadError( - path=root, context={"reason": "snapshot_filesystem_failed"}, cause=e + path=error_root, context={"reason": "snapshot_filesystem_failed"}, cause=e ) from e snapshot_id, snapshot_error = self._extract_modal_snapshot_id( @@ -1220,7 +1233,8 @@ async def _persist_workspace_via_snapshot_directory(self) -> io.IOBase: Persist the workspace using Modal's snapshot_directory API when available. """ - root = Path(self.state.manifest.root) + root = self._workspace_root_path() + error_root = posix_path_for_error(root) await self._ensure_sandbox() assert self._sandbox is not None if not hasattr(self._sandbox, "snapshot_directory"): @@ -1239,17 +1253,18 @@ async def restore_ephemeral_paths() -> WorkspaceArchiveReadError | None: return None restore_cmd = ( - f"if [ ! -f {shlex.quote(str(backup_path))} ]; then " + f"if [ ! -f {shlex.quote(backup_path.as_posix())} ]; then " f"echo missing ephemeral backup archive >&2; " f"exit 1; " f"fi; " - f"tar xf {shlex.quote(str(backup_path))} -C {shlex.quote(str(root))} && " - f"rm -f -- {shlex.quote(str(backup_path))}" + f"tar xf {shlex.quote(backup_path.as_posix())} -C " + f"{shlex.quote(root.as_posix())} && " + f"rm -f -- {shlex.quote(backup_path.as_posix())}" ) out = await self.exec("sh", "-lc", restore_cmd, shell=False) if not out.ok(): return WorkspaceArchiveReadError( - path=root, + path=error_root, context={ "reason": "snapshot_directory_ephemeral_restore_failed", "exit_code": out.exit_code, @@ -1268,7 +1283,7 @@ async def restore_detached_mounts() -> WorkspaceArchiveReadError | None: mount_path, ) except Exception as e: - current_error = WorkspaceArchiveReadError(path=root, cause=e) + current_error = WorkspaceArchiveReadError(path=error_root, cause=e) if remount_error is None: remount_error = current_error else: @@ -1289,27 +1304,28 @@ async def restore_detached_mounts() -> WorkspaceArchiveReadError | None: snapshot_id: str | None = None try: if skip_abs: - backup_path = ( - Path("/tmp/openai-agents/session-state") - / self.state.session_id.hex - / "modal-snapshot-directory-ephemeral.tar" + backup_path = posix_path_as_path( + coerce_posix_path( + "/tmp/openai-agents/session-state" + f"/{self.state.session_id.hex}/modal-snapshot-directory-ephemeral.tar" + ) ) rel_args = " ".join(shlex.quote(p.relative_to(root).as_posix()) for p in skip_abs) backup_cmd = ( - f"mkdir -p -- {shlex.quote(str(backup_path.parent))} && " - f"cd -- {shlex.quote(str(root))} && " + f"mkdir -p -- {shlex.quote(backup_path.parent.as_posix())} && " + f"cd -- {shlex.quote(root.as_posix())} && " "{ " f"for rel in {rel_args}; do " 'if [ -e "$rel" ]; then printf \'%s\\n\' "$rel"; fi; ' "done; " "} | " - f"tar cf {shlex.quote(str(backup_path))} -T - 2>/dev/null && " - f"test -f {shlex.quote(str(backup_path))}" + f"tar cf {shlex.quote(backup_path.as_posix())} -T - 2>/dev/null && " + f"test -f {shlex.quote(backup_path.as_posix())}" ) backup_out = await self.exec("sh", "-lc", backup_cmd, shell=False) if not backup_out.ok(): raise WorkspaceArchiveReadError( - path=root, + path=error_root, context={ "reason": "snapshot_directory_ephemeral_backup_failed", "exit_code": backup_out.exit_code, @@ -1318,11 +1334,11 @@ async def restore_detached_mounts() -> WorkspaceArchiveReadError | None: ) self._modal_snapshot_ephemeral_backup_path = backup_path - rm_cmd = ["rm", "-rf", "--", *[str(p) for p in skip_abs]] + rm_cmd = ["rm", "-rf", "--", *[sandbox_path_str(p) for p in skip_abs]] rm_out = await self.exec(*rm_cmd, shell=False) if not rm_out.ok(): raise WorkspaceArchiveReadError( - path=root, + path=error_root, context={ "reason": "snapshot_directory_ephemeral_remove_failed", "exit_code": rm_out.exit_code, @@ -1339,7 +1355,7 @@ async def restore_detached_mounts() -> WorkspaceArchiveReadError | None: detached_mounts.append((mount_entry, mount_path)) snapshot_sandbox = await self._refresh_sandbox_handle_for_snapshot() - snap_coro = snapshot_sandbox.snapshot_directory.aio(str(root)) + snap_coro = snapshot_sandbox.snapshot_directory.aio(root.as_posix()) if self.state.snapshot_filesystem_timeout_s is None: snap = await snap_coro else: @@ -1353,7 +1369,7 @@ async def restore_detached_mounts() -> WorkspaceArchiveReadError | None: snapshot_error = e except Exception as e: snapshot_error = WorkspaceArchiveReadError( - path=root, context={"reason": "snapshot_directory_failed"}, cause=e + path=error_root, context={"reason": "snapshot_directory_failed"}, cause=e ) finally: remount_error = await restore_detached_mounts() @@ -1401,7 +1417,7 @@ def _extract_modal_snapshot_id( ) -> tuple[str | None, WorkspaceArchiveReadError | None]: if isinstance(snap, bytes | bytearray): return None, WorkspaceArchiveReadError( - path=root, + path=posix_path_for_error(root), context={ "reason": f"{snapshot_kind}_unexpected_bytes", "type": type(snap).__name__, @@ -1409,7 +1425,7 @@ def _extract_modal_snapshot_id( ) if not hasattr(snap, "object_id") and not isinstance(snap, str): return None, WorkspaceArchiveReadError( - path=root, + path=posix_path_for_error(root), context={ "reason": f"{snapshot_kind}_unexpected_return", "type": type(snap).__name__, @@ -1422,7 +1438,7 @@ def _extract_modal_snapshot_id( snapshot_id = None if not snapshot_id: return None, WorkspaceArchiveReadError( - path=root, + path=posix_path_for_error(root), context={ "reason": f"{snapshot_kind}_unexpected_return", "type": type(snap).__name__, @@ -1489,7 +1505,8 @@ def _modal_tar_skip_relpaths(self, root: Path) -> set[Path]: ) async def _persist_workspace_via_tar(self) -> io.IOBase: # Existing tar implementation extracted so snapshot_filesystem mode can fall back cleanly. - root = Path(self.state.manifest.root) + root = self._workspace_root_path() + error_root = posix_path_for_error(root) skip = self._modal_tar_skip_relpaths(root) excludes: list[str] = [] @@ -1502,7 +1519,7 @@ async def _persist_workspace_via_tar(self) -> io.IOBase: "-", *excludes, "-C", - str(root), + root.as_posix(), ".", ] @@ -1510,7 +1527,7 @@ async def _persist_workspace_via_tar(self) -> io.IOBase: out = await self.exec(*cmd, shell=False) if not out.ok(): raise WorkspaceArchiveReadError( - path=root, + path=error_root, context={ "reason": "tar_nonzero_exit", "exit_code": out.exit_code, @@ -1521,7 +1538,7 @@ async def _persist_workspace_via_tar(self) -> io.IOBase: except WorkspaceArchiveReadError: raise except Exception as e: - raise WorkspaceArchiveReadError(path=root, cause=e) from e + raise WorkspaceArchiveReadError(path=error_root, cause=e) from e async def _hydrate_workspace_via_snapshot_filesystem(self, data: io.IOBase) -> None: """ @@ -1529,7 +1546,7 @@ async def _hydrate_workspace_via_snapshot_filesystem(self, data: io.IOBase) -> N persisted payload is a snapshot ref. Otherwise, fall back to tar extraction (to support SDKs that return tar bytes). """ - root = Path(self.state.manifest.root) + root = self._workspace_root_path() raw, snapshot_id = self._read_modal_snapshot_id_from_archive( data=data.read(), expected_persistence=_WORKSPACE_PERSISTENCE_SNAPSHOT_FILESYSTEM, @@ -1545,7 +1562,7 @@ async def _hydrate_workspace_via_snapshot_directory(self, data: io.IOBase) -> No persisted payload is a snapshot ref. Otherwise, fall back to tar extraction. """ - root = Path(self.state.manifest.root) + root = self._workspace_root_path() raw, snapshot_id = self._read_modal_snapshot_id_from_archive( data=data.read(), expected_persistence=_WORKSPACE_PERSISTENCE_SNAPSHOT_DIRECTORY, @@ -1562,7 +1579,7 @@ def _read_modal_snapshot_id_from_archive( expected_persistence: WorkspacePersistenceMode, invalid_reason: str, ) -> tuple[bytes, str | None]: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() raw = data if isinstance(raw, str): raw = raw.encode("utf-8") @@ -1610,7 +1627,7 @@ async def _run_restore() -> None: timeout=self.state.timeout, ) try: - mkdir_proc = await sb.exec.aio("mkdir", "-p", "--", str(root), text=False) + mkdir_proc = await sb.exec.aio("mkdir", "-p", "--", root.as_posix(), text=False) await mkdir_proc.wait.aio() except Exception: pass @@ -1642,7 +1659,7 @@ async def _run_restore() -> None: image = modal.Image.from_id(snapshot_id) await self._call_modal( sandbox.mount_image, - str(root), + root.as_posix(), image, call_timeout=self.state.snapshot_filesystem_restore_timeout_s, ) @@ -1682,7 +1699,7 @@ def _snapshot_directory_mount_targets_to_restore(self, root: Path) -> list[tuple return mount_targets async def _hydrate_workspace_via_tar(self, data: io.IOBase) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() raw = data.read() if isinstance(raw, str): @@ -1705,9 +1722,11 @@ async def _hydrate_workspace_via_tar(self, data: io.IOBase) -> None: async def _run_extract() -> None: assert self._sandbox is not None - mkdir_proc = await self._sandbox.exec.aio("mkdir", "-p", "--", str(root), text=False) + mkdir_proc = await self._sandbox.exec.aio( + "mkdir", "-p", "--", root.as_posix(), text=False + ) await mkdir_proc.wait.aio() - proc = await self._sandbox.exec.aio("tar", "xf", "-", "-C", str(root), text=False) + proc = await self._sandbox.exec.aio("tar", "xf", "-", "-C", root.as_posix(), text=False) await _write_process_stdin(proc, raw) exit_code = await proc.wait.aio() if exit_code != 0: @@ -1783,7 +1802,7 @@ def _validate_manifest_for_workspace_persistence( if workspace_persistence != _WORKSPACE_PERSISTENCE_SNAPSHOT_DIRECTORY: return - root = Path(manifest.root) + root = posix_path_as_path(coerce_posix_path(manifest.root)) for mount_entry, mount_path in manifest.mount_targets(): if not isinstance(mount_entry.mount_strategy, ModalCloudBucketMountStrategy): continue @@ -1794,8 +1813,8 @@ def _validate_manifest_for_workspace_persistence( "lives at or under the workspace root" ), context={ - "workspace_root": str(root), - "mount_path": str(mount_path), + "workspace_root": root.as_posix(), + "mount_path": mount_path.as_posix(), "workspace_persistence": workspace_persistence, }, ) diff --git a/src/agents/extensions/sandbox/runloop/sandbox.py b/src/agents/extensions/sandbox/runloop/sandbox.py index a551d92f0c..4b1d99c2a2 100644 --- a/src/agents/extensions/sandbox/runloop/sandbox.py +++ b/src/agents/extensions/sandbox/runloop/sandbox.py @@ -16,7 +16,7 @@ import io import json import logging -import os +import posixpath import shlex import uuid from collections.abc import Sequence @@ -54,6 +54,7 @@ from ....sandbox.snapshot import SnapshotBase, SnapshotSpec, resolve_snapshot from ....sandbox.types import ExecResult, ExposedPortEndpoint, User from ....sandbox.util.tar_utils import UnsafeTarMemberError, validate_tar_bytes +from ....sandbox.workspace_paths import coerce_posix_path, posix_path_as_path, sandbox_path_str if TYPE_CHECKING: from runloop_api_client.sdk.async_execution_result import ( @@ -870,31 +871,31 @@ async def _resolve_exposed_port(self, port: int) -> ExposedPortEndpoint: async def read(self, path: Path | str, *, user: str | User | None = None) -> io.IOBase: """Read a file via Runloop's binary file API.""" - path = Path(path) + error_path = posix_path_as_path(coerce_posix_path(path)) if user is not None: await self._check_read_with_exec(path, user=user) normalized_path = await self._validate_path_access(path) try: payload = await self._devbox.file.download( - path=str(normalized_path), + path=sandbox_path_str(normalized_path), timeout=self.state.timeouts.file_download_s, ) return io.BytesIO(bytes(payload)) except Exception as e: if _is_runloop_not_found(e): raise WorkspaceReadNotFoundError( - path=path, + path=error_path, context=_runloop_error_context(e, backend_detail="file_download_failed"), cause=e, ) from e if _is_runloop_provider_error(e): raise WorkspaceArchiveReadError( - path=path, + path=error_path, context=_runloop_error_context(e, backend_detail="file_download_failed"), cause=e, ) from e - raise WorkspaceArchiveReadError(path=path, cause=e) from e + raise WorkspaceArchiveReadError(path=error_path, cause=e) from e async def write( self, @@ -904,7 +905,7 @@ async def write( user: str | User | None = None, ) -> None: """Write a file through Runloop's upload API using manifest-root workspace paths.""" - path = Path(path) + error_path = posix_path_as_path(coerce_posix_path(path)) if user is not None: await self._check_write_with_exec(path, user=user) @@ -912,13 +913,13 @@ async def write( if isinstance(payload, str): payload = payload.encode("utf-8") if not isinstance(payload, bytes | bytearray): - raise WorkspaceWriteTypeError(path=path, actual_type=type(payload).__name__) + raise WorkspaceWriteTypeError(path=error_path, actual_type=type(payload).__name__) workspace_path = await self._validate_path_access(path, for_write=True) await self.mkdir(workspace_path.parent, parents=True) try: await self._devbox.file.upload( - path=str(workspace_path), + path=sandbox_path_str(workspace_path), file=bytes(payload), timeout=self.state.timeouts.file_upload_s, ) @@ -961,7 +962,7 @@ async def mkdir( cmd = ["mkdir"] if parents: cmd.append("-p") - cmd.extend(["--", str(path)]) + cmd.extend(["--", sandbox_path_str(path)]) result = await self._run_exec_command( shlex.join(cmd), command=tuple(cmd), @@ -981,7 +982,7 @@ async def _backup_plain_skip_paths(self, plain_skip: set[Path]) -> bytes | None: if not plain_skip: return None - root = self.state.manifest.root + root = sandbox_path_str(self.state.manifest.root) root_q = shlex.quote(root) checks = "\n".join( ( @@ -1000,7 +1001,7 @@ async def _backup_plain_skip_paths(self, plain_skip: set[Path]) -> bytes | None: result = await self.exec(command, shell=True, timeout=self.state.timeouts.snapshot_s) if not result.ok(): raise WorkspaceArchiveReadError( - path=Path(root), + path=self._workspace_root_path(), context={ "reason": "ephemeral_backup_failed", "exit_code": result.exit_code, @@ -1014,7 +1015,7 @@ async def _backup_plain_skip_paths(self, plain_skip: set[Path]) -> bytes | None: return io.BytesIO(base64.b64decode(encoded.encode("utf-8"), validate=True)).read() except Exception as e: raise WorkspaceArchiveReadError( - path=Path(root), + path=self._workspace_root_path(), context={"reason": "ephemeral_backup_invalid_base64"}, cause=e, ) from e @@ -1022,8 +1023,8 @@ async def _backup_plain_skip_paths(self, plain_skip: set[Path]) -> bytes | None: async def _remove_plain_skip_paths(self, plain_skip: set[Path]) -> None: if not plain_skip: return - root = Path(self.state.manifest.root) - command = ["rm", "-rf", "--"] + [str(root / rel) for rel in sorted(plain_skip)] + root = self._workspace_root_path() + command = ["rm", "-rf", "--"] + [(root / rel).as_posix() for rel in sorted(plain_skip)] result = await self.exec(*command, shell=False, timeout=self.state.timeouts.cleanup_s) if not result.ok(): raise WorkspaceArchiveReadError( @@ -1038,17 +1039,14 @@ async def _remove_plain_skip_paths(self, plain_skip: set[Path]) -> None: async def _restore_plain_skip_paths(self, backup: bytes | None) -> None: if not backup: return - root = Path(self.state.manifest.root) - temp_path = ( - Path(self.state.manifest.root) - / f".sandbox-runloop-restore-{self.state.session_id.hex}.tar" - ) + root = self._workspace_root_path() + temp_path = root / f".sandbox-runloop-restore-{self.state.session_id.hex}.tar" await self.write(temp_path, io.BytesIO(backup)) try: result = await self.exec( "mkdir", "-p", - str(root), + root.as_posix(), shell=False, timeout=self.state.timeouts.cleanup_s, ) @@ -1063,9 +1061,9 @@ async def _restore_plain_skip_paths(self, backup: bytes | None) -> None: result = await self.exec( "tar", "-xf", - str(temp_path), + sandbox_path_str(temp_path), "-C", - str(root), + root.as_posix(), shell=False, timeout=self.state.timeouts.snapshot_s, ) @@ -1080,7 +1078,7 @@ async def _restore_plain_skip_paths(self, backup: bytes | None) -> None: ) finally: try: - await self.exec("rm", "-f", "--", str(temp_path), shell=False) + await self.exec("rm", "-f", "--", sandbox_path_str(temp_path), shell=False) except Exception: pass @@ -1091,7 +1089,7 @@ async def persist_workspace(self) -> io.IOBase: ephemeral mounts so the saved disk image contains only durable workspace state, then it restores those local-only artifacts afterward. """ - root = Path(self.state.manifest.root) + root = self._workspace_root_path() skip = self._persist_workspace_skip_relpaths() mount_targets = self.state.manifest.ephemeral_mount_targets() mount_skip_rel_paths: set[Path] = set() @@ -1202,7 +1200,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: source blueprint, so restore does not reselect a blueprint. Non-native payloads fall back to tar hydration so cross-provider snapshots and file snapshots keep working. """ - root = Path(self.state.manifest.root) + root = self._workspace_root_path() raw = data.read() if isinstance(raw, str): raw = raw.encode("utf-8") @@ -1256,7 +1254,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: async def _restore_snapshot_into_workspace_on_resume(self) -> None: """Restore snapshots on resume, preserving Runloop's native disk-snapshot fast path.""" - root = Path(self.state.manifest.root) + root = self._workspace_root_path() workspace_archive = await self.state.snapshot.restore(dependencies=self.dependencies) try: raw = workspace_archive.read() @@ -1280,7 +1278,7 @@ async def _restore_snapshot_into_workspace_on_resume(self) -> None: pass async def _hydrate_workspace_via_tar(self, payload: bytes) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() archive_path = root / f".sandbox-runloop-hydrate-{self.state.session_id.hex}.tar" try: @@ -1302,9 +1300,9 @@ async def _hydrate_workspace_via_tar(self, payload: bytes) -> None: result = await self.exec( "tar", "-C", - str(root), + root.as_posix(), "-xf", - str(archive_path), + archive_path.as_posix(), shell=False, timeout=self.state.timeouts.snapshot_s, ) @@ -1327,7 +1325,7 @@ async def _hydrate_workspace_via_tar(self, payload: bytes) -> None: "rm", "-f", "--", - str(archive_path), + archive_path.as_posix(), shell=False, timeout=self.state.timeouts.cleanup_s, ) @@ -1433,7 +1431,7 @@ def _default_runloop_manifest_root(user_parameters: RunloopUserParameters | None def _validate_runloop_manifest_root( manifest: Manifest, *, user_parameters: RunloopUserParameters | None ) -> None: - root = PurePosixPath(os.path.normpath(manifest.root)) + root = PurePosixPath(posixpath.normpath(manifest.root)) runloop_home = _effective_runloop_home(user_parameters) try: root.relative_to(runloop_home) diff --git a/src/agents/extensions/sandbox/vercel/sandbox.py b/src/agents/extensions/sandbox/vercel/sandbox.py index 233676d4f9..8deafc2b2a 100644 --- a/src/agents/extensions/sandbox/vercel/sandbox.py +++ b/src/agents/extensions/sandbox/vercel/sandbox.py @@ -14,7 +14,7 @@ import asyncio import io import json -import os +import posixpath import tarfile import uuid from collections.abc import Awaitable, Callable @@ -60,6 +60,7 @@ retry_async, ) from ....sandbox.util.tar_utils import UnsafeTarMemberError, validate_tarfile +from ....sandbox.workspace_paths import coerce_posix_path, posix_path_as_path, sandbox_path_str WorkspacePersistenceMode = Literal["tar", "snapshot"] @@ -294,16 +295,16 @@ def _validate_tar_bytes(self, raw: bytes) -> None: raise ValueError("invalid tar stream") from exc async def _prepare_backend_workspace(self) -> None: - root = PurePosixPath(os.path.normpath(self.state.manifest.root)) + root = PurePosixPath(posixpath.normpath(self.state.manifest.root)) try: sandbox = await self._ensure_sandbox() finished = await sandbox.run_command("mkdir", ["-p", "--", root.as_posix()]) except Exception as exc: - raise WorkspaceStartError(path=Path(str(root)), cause=exc) from exc + raise WorkspaceStartError(path=posix_path_as_path(root), cause=exc) from exc if finished.exit_code != 0: raise WorkspaceStartError( - path=Path(str(root)), + path=posix_path_as_path(root), context={ "exit_code": finished.exit_code, "stdout": await finished.stdout(), @@ -407,7 +408,7 @@ async def _persist_with_ephemeral_mounts_removed( self, operation: Callable[[], Awaitable[io.IOBase]], ) -> io.IOBase: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() unmounted_mounts: list[tuple[Any, Path]] = [] unmount_error: WorkspaceArchiveReadError | None = None for mount_entry, mount_path in self.state.manifest.ephemeral_mount_targets(): @@ -456,7 +457,7 @@ async def _hydrate_with_ephemeral_mounts_removed( self, operation: Callable[[], Awaitable[None]], ) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() unmounted_mounts: list[tuple[Any, Path]] = [] unmount_error: WorkspaceArchiveWriteError | None = None for mount_entry, mount_path in self.state.manifest.ephemeral_mount_targets(): @@ -566,7 +567,7 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase normalized_path = await self._validate_path_access(path) sandbox = await self._ensure_sandbox() try: - payload = await sandbox.read_file(str(normalized_path)) + payload = await sandbox.read_file(sandbox_path_str(normalized_path)) except Exception as exc: raise WorkspaceArchiveReadError(path=normalized_path, cause=exc) from exc if payload is None: @@ -594,7 +595,7 @@ async def write( ) try: await self._write_files_with_retry( - [{"path": str(normalized_path), "content": bytes(payload)}] + [{"path": sandbox_path_str(normalized_path), "content": bytes(payload)}] ) except Exception as exc: raise WorkspaceArchiveWriteError(path=normalized_path, cause=exc) from exc @@ -604,7 +605,7 @@ async def persist_workspace(self) -> io.IOBase: async def _persist_workspace_internal(self) -> io.IOBase: if self.state.workspace_persistence == _WORKSPACE_PERSISTENCE_SNAPSHOT: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() sandbox = await self._ensure_sandbox() try: snapshot = await sandbox.snapshot(expiration=self.state.snapshot_expiration_ms) @@ -612,9 +613,11 @@ async def _persist_workspace_internal(self) -> io.IOBase: raise WorkspaceArchiveReadError(path=root, cause=exc) from exc return io.BytesIO(_encode_snapshot_ref(snapshot_id=snapshot.snapshot_id)) - root = Path(self.state.manifest.root) + root = self._workspace_root_path() sandbox = await self._ensure_sandbox() - archive_path = Path("/tmp") / f"openai-agents-{self.state.session_id.hex}.tar" + archive_path = posix_path_as_path( + coerce_posix_path(f"/tmp/openai-agents-{self.state.session_id.hex}.tar") + ) excludes = [ f"--exclude=./{rel_path.as_posix()}" for rel_path in sorted( @@ -622,7 +625,7 @@ async def _persist_workspace_internal(self) -> io.IOBase: key=lambda item: item.as_posix(), ) ] - tar_command = ("tar", "cf", str(archive_path), *excludes, ".") + tar_command = ("tar", "cf", archive_path.as_posix(), *excludes, ".") try: result = await self.exec(*tar_command, shell=False) if not result.ok(): @@ -634,7 +637,7 @@ async def _persist_workspace_internal(self) -> io.IOBase: context={"backend": "vercel", "sandbox_id": self.state.sandbox_id}, ), ) - archive = await sandbox.read_file(str(archive_path)) + archive = await sandbox.read_file(archive_path.as_posix()) if archive is None: raise WorkspaceReadNotFoundError(path=archive_path) return io.BytesIO(archive) @@ -646,7 +649,9 @@ async def _persist_workspace_internal(self) -> io.IOBase: raise WorkspaceArchiveReadError(path=root, cause=exc) from exc finally: try: - await sandbox.run_command("rm", [str(archive_path)], cwd=self.state.manifest.root) + await sandbox.run_command( + "rm", [archive_path.as_posix()], cwd=self.state.manifest.root + ) except Exception: pass @@ -656,7 +661,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: raw = raw.encode("utf-8") if not isinstance(raw, bytes | bytearray): raise WorkspaceWriteTypeError( - path=Path(self.state.manifest.root), + path=self._workspace_root_path(), actual_type=type(raw).__name__, ) @@ -675,19 +680,21 @@ async def _hydrate_workspace_internal(self, raw: bytes) -> None: await self._replace_sandbox_from_snapshot(snapshot_id) except Exception as exc: raise WorkspaceArchiveWriteError( - path=Path(self.state.manifest.root), + path=self._workspace_root_path(), cause=exc, ) from exc return - root = Path(self.state.manifest.root) + root = self._workspace_root_path() sandbox = await self._ensure_sandbox() - archive_path = Path("/tmp") / f"openai-agents-{self.state.session_id.hex}.tar" - tar_command = ("tar", "xf", str(archive_path), "-C", str(root)) + archive_path = posix_path_as_path( + coerce_posix_path(f"/tmp/openai-agents-{self.state.session_id.hex}.tar") + ) + tar_command = ("tar", "xf", archive_path.as_posix(), "-C", root.as_posix()) try: self._validate_tar_bytes(raw) await self.mkdir(root, parents=True) - await self._write_files_with_retry([{"path": str(archive_path), "content": raw}]) + await self._write_files_with_retry([{"path": archive_path.as_posix(), "content": raw}]) result = await self.exec(*tar_command, shell=False) if not result.ok(): raise WorkspaceArchiveWriteError( @@ -704,7 +711,9 @@ async def _hydrate_workspace_internal(self, raw: bytes) -> None: raise WorkspaceArchiveWriteError(path=root, cause=exc) from exc finally: try: - await sandbox.run_command("rm", [str(archive_path)], cwd=self.state.manifest.root) + await sandbox.run_command( + "rm", [archive_path.as_posix()], cwd=self.state.manifest.root + ) except Exception: pass diff --git a/src/agents/memory/sqlite_session.py b/src/agents/memory/sqlite_session.py index d0ca2557a2..a31347cdcd 100644 --- a/src/agents/memory/sqlite_session.py +++ b/src/agents/memory/sqlite_session.py @@ -52,6 +52,9 @@ def __init__( self.sessions_table = sessions_table self.messages_table = messages_table self._local = threading.local() + self._connections: set[sqlite3.Connection] = set() + self._connections_lock = threading.Lock() + self._closed = False # For in-memory databases, we need a shared connection to avoid thread isolation # For file databases, we use thread-local connections for better concurrency @@ -115,17 +118,23 @@ def _locked_connection(self) -> Iterator[sqlite3.Connection]: def _get_connection(self) -> sqlite3.Connection: """Get a database connection.""" + if self._closed: + raise RuntimeError("SQLiteSession is closed") + if self._is_memory_db: # Use shared connection for in-memory database to avoid thread isolation return self._shared_connection else: # Use thread-local connections for file databases if not hasattr(self._local, "connection"): - self._local.connection = sqlite3.connect( + connection = sqlite3.connect( str(self.db_path), check_same_thread=False, ) - self._local.connection.execute("PRAGMA journal_mode=WAL") + connection.execute("PRAGMA journal_mode=WAL") + self._local.connection = connection + with self._connections_lock: + self._connections.add(connection) assert isinstance(self._local.connection, sqlite3.Connection), ( f"Expected sqlite3.Connection, got {type(self._local.connection)}" ) @@ -320,12 +329,20 @@ def _clear_session_sync(): def close(self) -> None: """Close the database connection.""" - if self._is_memory_db: - if hasattr(self, "_shared_connection"): - self._shared_connection.close() - else: - if hasattr(self._local, "connection"): - self._local.connection.close() + with self._lock: + if self._closed: + return + + self._closed = True + if self._is_memory_db: + if hasattr(self, "_shared_connection"): + self._shared_connection.close() + else: + with self._connections_lock: + connections = list(self._connections) + self._connections.clear() + for connection in connections: + connection.close() if self._lock_path is not None and not self._lock_released: self._release_file_lock(self._lock_path) self._lock_released = True diff --git a/src/agents/sandbox/capabilities/skills.py b/src/agents/sandbox/capabilities/skills.py index b368895867..e69906d0ad 100644 --- a/src/agents/sandbox/capabilities/skills.py +++ b/src/agents/sandbox/capabilities/skills.py @@ -15,6 +15,7 @@ from ..manifest import Manifest from ..session.base_sandbox_session import BaseSandboxSession from ..types import User +from ..workspace_paths import coerce_posix_path, posix_path_as_path, windows_absolute_path from .capability import Capability _SKILLS_SECTION_INTRO = ( @@ -262,42 +263,58 @@ def _validate_relative_path( field_name: str, context: Mapping[str, object] | None = None, ) -> Path: - rel = value if isinstance(value, Path) else Path(value) - if rel.is_absolute(): + if (windows_path := windows_absolute_path(value)) is not None: raise SkillsConfigError( message=f"{field_name} must be a relative path", context={ "field": field_name, - "path": str(rel), + "path": windows_path.as_posix(), "reason": "absolute", **(context or {}), }, ) - if ".." in rel.parts: + rel_posix = coerce_posix_path(value) + if rel_posix.is_absolute(): + raise SkillsConfigError( + message=f"{field_name} must be a relative path", + context={ + "field": field_name, + "path": rel_posix.as_posix(), + "reason": "absolute", + **(context or {}), + }, + ) + if ".." in rel_posix.parts: raise SkillsConfigError( message=f"{field_name} must not escape the skills root", context={ "field": field_name, - "path": str(rel), + "path": rel_posix.as_posix(), "reason": "escape_root", **(context or {}), }, ) - if rel.parts in [(), (".",)]: + if rel_posix.parts in [(), (".",)]: raise SkillsConfigError( message=f"{field_name} must be non-empty", - context={"field": field_name, "path": str(rel), "reason": "empty", **(context or {})}, + context={ + "field": field_name, + "path": rel_posix.as_posix(), + "reason": "empty", + **(context or {}), + }, ) - return rel + return posix_path_as_path(rel_posix) def _manifest_entry_paths(manifest: Manifest) -> set[Path]: - return {key if isinstance(key, Path) else Path(key) for key in manifest.entries} + return {posix_path_as_path(coerce_posix_path(key)) for key in manifest.entries} def _get_manifest_entry_by_path(manifest: Manifest, path: Path) -> BaseEntry | None: + path = posix_path_as_path(coerce_posix_path(path)) for key, entry in manifest.entries.items(): - normalized = key if isinstance(key, Path) else Path(key) + normalized = posix_path_as_path(coerce_posix_path(key)) if normalized == path: return entry return None @@ -523,7 +540,7 @@ def model_post_init(self, context: Any, /) -> None: seen_names.add(rel) def process_manifest(self, manifest: Manifest) -> Manifest: - skills_root = Path(self.skills_path) + skills_root = posix_path_as_path(coerce_posix_path(self.skills_path)) existing_paths = _manifest_entry_paths(manifest) if self.lazy_from: @@ -618,7 +635,9 @@ async def _resolve_runtime_metadata(self, manifest: Manifest) -> list[SkillMetad if self.session is None: return [] - skills_root = Path(manifest.root) / Path(self.skills_path) + skills_root = posix_path_as_path( + coerce_posix_path(manifest.root) / coerce_posix_path(self.skills_path) + ) try: entries = await self.session.ls(skills_root, user=self.run_as) except Exception: @@ -629,9 +648,9 @@ async def _resolve_runtime_metadata(self, manifest: Manifest) -> list[SkillMetad if not entry.is_dir(): continue - skill_dir = Path(entry.path) + skill_dir = posix_path_as_path(coerce_posix_path(entry.path)) skill_name = skill_dir.name - skill_path = Path(self.skills_path) / skill_name + skill_path = posix_path_as_path(coerce_posix_path(self.skills_path) / skill_name) skill_md_path = skill_dir / "SKILL.md" try: @@ -665,7 +684,7 @@ async def _skill_metadata(self, manifest: Manifest) -> list[SkillMetadata]: SkillMetadata( name=skill.name, description=skill.description, - path=Path(self.skills_path) / skill.name, + path=posix_path_as_path(coerce_posix_path(self.skills_path) / skill.name), ) ) @@ -678,12 +697,12 @@ async def _skill_metadata(self, manifest: Manifest) -> list[SkillMetadata]: for key, entry in self.from_.children.items(): if not isinstance(entry, Dir): continue - skill_name = str(key if isinstance(key, Path) else Path(key)) + skill_name = coerce_posix_path(key).as_posix() metadata.append( SkillMetadata( name=skill_name, description=entry.description or "No description provided.", - path=Path(self.skills_path) / skill_name, + path=posix_path_as_path(coerce_posix_path(self.skills_path) / skill_name), ) ) diff --git a/src/agents/sandbox/capabilities/tools/shell_tool.py b/src/agents/sandbox/capabilities/tools/shell_tool.py index d85b85b25d..8da9eddccf 100644 --- a/src/agents/sandbox/capabilities/tools/shell_tool.py +++ b/src/agents/sandbox/capabilities/tools/shell_tool.py @@ -16,6 +16,7 @@ from ...session.base_sandbox_session import BaseSandboxSession from ...types import User from ...util.token_truncation import formatted_truncate_text_with_token_count +from ...workspace_paths import sandbox_path_str _DEFAULT_EXEC_YIELD_TIME_MS = 10_000 _DEFAULT_WRITE_STDIN_YIELD_TIME_MS = 250 @@ -73,7 +74,7 @@ def _resolve_workdir_command( return command resolved_workdir = session.normalize_path(Path(workdir)) - return f"cd {shlex.quote(str(resolved_workdir))} && {command}" + return f"cd {shlex.quote(sandbox_path_str(resolved_workdir))} && {command}" def _resolve_shell(shell: str | None, login: bool) -> bool | list[str]: diff --git a/src/agents/sandbox/entries/artifacts.py b/src/agents/sandbox/entries/artifacts.py index 79c0396da5..e36fdedd60 100644 --- a/src/agents/sandbox/entries/artifacts.py +++ b/src/agents/sandbox/entries/artifacts.py @@ -365,6 +365,7 @@ def _open_local_dir_file_for_copy( ) -> int: if not _OPEN_SUPPORTS_DIR_FD or not _HAS_O_DIRECTORY: return self._open_local_dir_file_for_copy_fallback( + base_dir=base_dir, src_root=src_root, rel_child=rel_child, ) @@ -539,7 +540,9 @@ def _local_dir_open_error( ) return LocalDirReadError(src=src_root, cause=error) - def _open_local_dir_file_for_copy_fallback(self, *, src_root: Path, rel_child: Path) -> int: + def _open_local_dir_file_for_copy_fallback( + self, *, base_dir: Path, src_root: Path, rel_child: Path + ) -> int: src = src_root / rel_child try: src_stat = src.lstat() @@ -564,20 +567,32 @@ def _open_local_dir_file_for_copy_fallback(self, *, src_root: Path, rel_child: P file_flags = os.O_RDONLY | getattr(os, "O_BINARY", 0) | getattr(os, "O_NOFOLLOW", 0) try: leaf_fd = os.open(src, file_flags) - leaf_stat = os.fstat(leaf_fd) - if not stat.S_ISREG(leaf_stat.st_mode) or not os.path.samestat(src_stat, leaf_stat): + try: + self._resolve_local_dir_src_root(base_dir) + leaf_stat = os.fstat(leaf_fd) + if not stat.S_ISREG(leaf_stat.st_mode) or not os.path.samestat(src_stat, leaf_stat): + raise LocalDirReadError( + src=src_root, + context={ + "reason": "path_changed_during_copy", + "child": rel_child.as_posix(), + }, + ) + return leaf_fd + except Exception: os.close(leaf_fd) - raise LocalDirReadError( - src=src_root, - context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()}, - ) - return leaf_fd + raise except FileNotFoundError: + self._resolve_local_dir_src_root(base_dir) raise LocalDirReadError( src=src_root, context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()}, ) from None except OSError as e: + try: + self._resolve_local_dir_src_root(base_dir) + except LocalDirReadError as root_error: + raise root_error from e if e.errno == errno.ELOOP: raise LocalDirReadError( src=src_root, diff --git a/src/agents/sandbox/entries/base.py b/src/agents/sandbox/entries/base.py index 218cbbca23..2f5ba4e36d 100644 --- a/src/agents/sandbox/entries/base.py +++ b/src/agents/sandbox/entries/base.py @@ -3,9 +3,10 @@ import abc import builtins import inspect +import posixpath import stat from collections.abc import Mapping -from pathlib import Path +from pathlib import Path, PurePath, PurePosixPath from typing import TYPE_CHECKING, ClassVar from pydantic import BaseModel, Field @@ -13,41 +14,70 @@ from ..errors import InvalidManifestPathError from ..materialization import MaterializedFile from ..types import FileMode, Group, Permissions, User +from ..workspace_paths import ( + coerce_posix_path, + posix_path_as_path, + sandbox_path_str, + windows_absolute_path, +) if TYPE_CHECKING: from ..session.base_sandbox_session import BaseSandboxSession def resolve_workspace_path( - workspace_root: Path, - rel: str | Path, + workspace_root: str | PurePath, + rel: str | PurePath, *, allow_absolute_within_root: bool = False, ) -> Path: - rel = Path(rel) - workspace_root = Path(workspace_root) + if (windows_path := windows_absolute_path(rel)) is not None: + raise InvalidManifestPathError(rel=windows_path.as_posix(), reason="absolute") + rel_path = coerce_posix_path(rel) + root_path = coerce_posix_path(workspace_root) - if rel.is_absolute(): + if rel_path.is_absolute(): if not allow_absolute_within_root: - raise InvalidManifestPathError(rel=rel, reason="absolute") - resolved_workspace_root = workspace_root.resolve(strict=False) - resolved_rel = rel.resolve(strict=False) + raise InvalidManifestPathError(rel=rel_path.as_posix(), reason="absolute") + rel_path = PurePosixPath(posixpath.normpath(rel_path.as_posix())) + root_path = PurePosixPath(posixpath.normpath(root_path.as_posix())) + host_root = Path(root_path.as_posix()) + if _path_exists(host_root): + try: + Path(rel_path.as_posix()).resolve(strict=False).relative_to( + host_root.resolve(strict=False) + ) + except ValueError as exc: + raise InvalidManifestPathError( + rel=rel_path.as_posix(), reason="absolute", cause=exc + ) from exc try: - resolved_rel.relative_to(resolved_workspace_root) + rel_path.relative_to(root_path) except ValueError as exc: - raise InvalidManifestPathError(rel=rel, reason="absolute", cause=exc) from exc - return resolved_rel + raise InvalidManifestPathError( + rel=rel_path.as_posix(), reason="absolute", cause=exc + ) from exc + return posix_path_as_path(rel_path) - if ".." in rel.parts: - raise InvalidManifestPathError(rel=rel, reason="escape_root") + if ".." in rel_path.parts: + raise InvalidManifestPathError(rel=rel_path.as_posix(), reason="escape_root") - resolved = workspace_root / rel if rel.parts else workspace_root + resolved = root_path / rel_path if rel_path.parts else root_path if allow_absolute_within_root and resolved.is_absolute(): try: - resolved.relative_to(workspace_root) + resolved.relative_to(root_path) except ValueError as exc: - raise InvalidManifestPathError(rel=rel, reason="escape_root", cause=exc) from exc - return resolved + raise InvalidManifestPathError( + rel=rel_path.as_posix(), reason="escape_root", cause=exc + ) from exc + return posix_path_as_path(resolved) + + +def _path_exists(path: Path) -> bool: + try: + return path.exists() + except OSError: + return False class BaseEntry(BaseModel, abc.ABC): @@ -132,11 +162,12 @@ async def _apply_metadata( session: BaseSandboxSession, dest: Path, ) -> None: + dest_arg = sandbox_path_str(dest) if self.group is not None: - await session._exec_checked_nonzero("chgrp", self.group.name, str(dest)) + await session._exec_checked_nonzero("chgrp", self.group.name, dest_arg) chmod_perms = f"{stat.S_IMODE(self.permissions.to_mode()):o}".zfill(4) - await session._exec_checked_nonzero("chmod", chmod_perms, str(dest)) + await session._exec_checked_nonzero("chmod", chmod_perms, dest_arg) @abc.abstractmethod async def apply( diff --git a/src/agents/sandbox/entries/mounts/base.py b/src/agents/sandbox/entries/mounts/base.py index 14bbe903f4..9c8bcf1705 100644 --- a/src/agents/sandbox/entries/mounts/base.py +++ b/src/agents/sandbox/entries/mounts/base.py @@ -10,9 +10,10 @@ from pydantic import BaseModel, Field, SerializeAsAny, field_validator -from ...errors import MountConfigError +from ...errors import InvalidManifestPathError, MountConfigError from ...materialization import MaterializedFile from ...types import FileMode, Permissions +from ...workspace_paths import coerce_posix_path, posix_path_as_path, windows_absolute_path from ..base import BaseEntry from .patterns import MountPattern, MountPatternBase, MountPatternConfig @@ -477,7 +478,9 @@ def _resolve_mount_path( ) -> Path: """Resolve the concrete path where this mount should appear in the active workspace.""" - manifest_root = Path(getattr(session.state.manifest, "root", "/")) + manifest_root = posix_path_as_path( + coerce_posix_path(getattr(session.state.manifest, "root", "/")) + ) return self._resolve_mount_path_for_root(manifest_root, dest) def _resolve_mount_path_for_root( @@ -492,8 +495,11 @@ def _resolve_mount_path_for_root( """ if self.mount_path is not None: - mount_path = Path(self.mount_path) - if mount_path.is_absolute(): + if (windows_path := windows_absolute_path(self.mount_path)) is not None: + raise InvalidManifestPathError(rel=windows_path.as_posix(), reason="absolute") + mount_posix = coerce_posix_path(self.mount_path) + mount_path = posix_path_as_path(mount_posix) + if mount_posix.is_absolute(): return mount_path # Relative explicit mount paths are interpreted inside the active workspace root so a # manifest can stay portable across backends with different concrete root prefixes. diff --git a/src/agents/sandbox/entries/mounts/patterns.py b/src/agents/sandbox/entries/mounts/patterns.py index df75ee38af..931fa03450 100644 --- a/src/agents/sandbox/entries/mounts/patterns.py +++ b/src/agents/sandbox/entries/mounts/patterns.py @@ -17,6 +17,12 @@ MountToolMissingError, WorkspaceReadNotFoundError, ) +from ...workspace_paths import ( + coerce_posix_path, + posix_path_as_path, + sandbox_path_str, + windows_absolute_path, +) if TYPE_CHECKING: from ...session.base_sandbox_session import BaseSandboxSession @@ -97,7 +103,9 @@ async def _write_sensitive_config_file( """Write generated mount credentials/config with owner-only permissions.""" await session.write(path, io.BytesIO(payload)) - await session._exec_checked_nonzero("chmod", "0600", str(session.normalize_path(path))) + await session._exec_checked_nonzero( + "chmod", "0600", sandbox_path_str(session.normalize_path(path)) + ) class MountPatternBase(BaseModel, abc.ABC): @@ -139,10 +147,16 @@ class FuseMountPattern(MountPatternBase): def model_post_init(self, __context: object, /) -> None: if self.cache_path is None: return - if self.cache_path.is_absolute() or ".." in self.cache_path.parts: + if (windows_path := windows_absolute_path(self.cache_path)) is not None: + raise MountConfigError( + message="blobfuse cache_path must be relative to the workspace root", + context={"cache_path": windows_path.as_posix()}, + ) + cache_path = coerce_posix_path(self.cache_path) + if cache_path.is_absolute() or ".." in cache_path.parts: raise MountConfigError( message="blobfuse cache_path must be relative to the workspace root", - context={"cache_path": str(self.cache_path)}, + context={"cache_path": cache_path.as_posix()}, ) @dataclass(frozen=True) @@ -204,7 +218,7 @@ def to_text(self) -> str: "block_cache:", f" block-size-mb: {self.block_cache_block_size_mb}", f" mem-size-mb: {self.cache_size_mb}", - f" path: {self.cache_dir}", + f" path: {sandbox_path_str(self.cache_dir)}", f" disk-size-mb: {self.cache_size_mb}", f" disk-timeout-sec: {self.block_cache_disk_timeout_sec}", "", @@ -214,7 +228,7 @@ def to_text(self) -> str: lines.extend( [ "file_cache:", - f" path: {self.cache_dir}", + f" path: {sandbox_path_str(self.cache_dir)}", f" timeout-sec: {self.file_cache_timeout_sec}", f" max-size-mb: {self.file_cache_max_size_mb}", "", @@ -274,13 +288,17 @@ async def apply( mount_path = path cache_dir = ( - Path(self.cache_path) + posix_path_as_path(coerce_posix_path(self.cache_path)) if self.cache_path is not None # Keep mount scratch state inside the workspace so session helpers can create/write it # through the normal workspace-scoped API. - else Path(f".sandbox-blobfuse-cache/{session_id.hex}") / account / container + else posix_path_as_path( + coerce_posix_path(f".sandbox-blobfuse-cache/{session_id.hex}/{account}/{container}") + ) + ) + config_dir = posix_path_as_path( + coerce_posix_path(f".sandbox-blobfuse-config/{session_id.hex}") ) - config_dir = Path(f".sandbox-blobfuse-config/{session_id.hex}") config_name = f"{account}_{container}".replace("/", "_") config_path = config_dir / f"{config_name}.yaml" command_mount_path = session.normalize_path(mount_path) @@ -291,8 +309,8 @@ async def apply( raise MountConfigError( message="blobfuse cache_path must be outside the mount path", context={ - "mount_path": str(command_mount_path), - "cache_path": str(command_cache_dir), + "mount_path": sandbox_path_str(command_mount_path), + "cache_path": sandbox_path_str(command_cache_dir), }, ) @@ -333,8 +351,8 @@ async def apply( cmd: list[str] = ["blobfuse2", "mount"] if fuse_config.read_only: cmd.append("--read-only") - cmd.extend(["--config-file", str(command_config_path)]) - cmd.append(str(mount_path)) + cmd.extend(["--config-file", sandbox_path_str(command_config_path)]) + cmd.append(sandbox_path_str(mount_path)) result = await session.exec(*cmd, shell=False) if not result.ok(): @@ -355,7 +373,8 @@ async def unapply( await session.exec( "sh", "-lc", - f"fusermount3 -u {shlex.quote(str(path))} || umount {shlex.quote(str(path))}", + f"fusermount3 -u {shlex.quote(sandbox_path_str(path))} || " + f"umount {shlex.quote(sandbox_path_str(path))}", shell=False, ) @@ -404,7 +423,7 @@ async def apply( cmd.extend(["--upload-checksums", "off"]) if mountpoint_config.prefix: cmd.extend(["--prefix", mountpoint_config.prefix]) - cmd.extend([bucket, str(path)]) + cmd.extend([bucket, sandbox_path_str(path)]) env_parts: list[str] = [] access_key_id = mountpoint_config.access_key_id @@ -438,7 +457,8 @@ async def unapply( await session.exec( "sh", "-lc", - f"fusermount3 -u {shlex.quote(str(path))} || umount {shlex.quote(str(path))}", + f"fusermount3 -u {shlex.quote(sandbox_path_str(path))} || " + f"umount {shlex.quote(sandbox_path_str(path))}", shell=False, ) @@ -492,7 +512,7 @@ async def apply( key if value is None else f"{key}={value}" for key, value in options.items() ) cmd.extend(["-o", rendered_options]) - cmd.extend([device, str(path)]) + cmd.extend([device, sandbox_path_str(path)]) result = await session.exec(*cmd, shell=False) if not result.ok(): @@ -512,7 +532,7 @@ async def unapply( await session.exec( "sh", "-lc", - f"umount {shlex.quote(str(path))} || true", + f"umount {shlex.quote(sandbox_path_str(path))} || true", shell=False, ) @@ -581,7 +601,9 @@ def _resolve_config_path( session: BaseSandboxSession, config_path: Path, ) -> Path: - manifest_root = Path(getattr(session.state.manifest, "root", "/")) + manifest_root = posix_path_as_path( + coerce_posix_path(getattr(session.state.manifest, "root", "/")) + ) if config_path.is_absolute(): return config_path # Relative config paths are resolved inside the sandbox workspace, not relative to the @@ -610,7 +632,7 @@ async def read_config_text( except Exception as e: raise MountConfigError( message="failed to read rclone config file", - context={"type": mount_type or "mount", "path": str(config_path)}, + context={"type": mount_type or "mount", "path": sandbox_path_str(config_path)}, ) from e try: @@ -627,7 +649,7 @@ async def read_config_text( if not config_text.strip(): raise MountConfigError( message="rclone config file is empty", - context={"type": mount_type or "mount", "path": str(config_path)}, + context={"type": mount_type or "mount", "path": sandbox_path_str(config_path)}, ) section_pattern = rf"^\s*\[{re.escape(remote_name)}\]\s*$" @@ -636,7 +658,7 @@ async def read_config_text( message="rclone config missing required remote section", context={ "type": mount_type or "mount", - "path": str(config_path), + "path": sandbox_path_str(config_path), "remote_name": remote_name, }, ) @@ -665,7 +687,7 @@ async def _start_rclone_server( ) cmd: list[str] = ["rclone", "serve", "nfs", f"{config.remote_name}:{config.remote_path}"] cmd.extend(["--addr", nfs_addr]) - cmd.extend(["--config", str(config_path)]) + cmd.extend(["--config", sandbox_path_str(config_path)]) if config.read_only: cmd.append("--read-only") if self.extra_args: @@ -695,11 +717,11 @@ async def _start_rclone_client( "rclone", "mount", f"{config.remote_name}:{config.remote_path}", - str(path), + sandbox_path_str(path), ] if config.read_only: cmd.append("--read-only") - cmd.extend(["--config", str(config_path), "--daemon"]) + cmd.extend(["--config", sandbox_path_str(config_path), "--daemon"]) if self.extra_args: cmd.extend(self.extra_args) result = await session.exec(*cmd, shell=False) @@ -761,7 +783,7 @@ async def _start_rclone_client( "-o", shlex.quote(option_arg), f"{shlex.quote(host)}:/", - shlex.quote(str(path)), + shlex.quote(sandbox_path_str(path)), "&& exit 0; sleep 1; done; exit 1", ] ) @@ -812,7 +834,9 @@ async def apply( session_id_str = session_id.hex # Keep generated rclone config under the workspace root so `session.mkdir()` / # `session.write()` can handle it without special-casing absolute paths. - config_dir = Path(f".sandbox-rclone-config/{session_id_str}") + config_dir = posix_path_as_path( + coerce_posix_path(f".sandbox-rclone-config/{session_id_str}") + ) config_path = config_dir / f"{rclone_config.remote_name}.conf" await session.mkdir(path, parents=True) await session.mkdir(config_dir, parents=True) @@ -861,14 +885,15 @@ async def unapply( await session.exec( "sh", "-lc", - f"fusermount3 -u {shlex.quote(str(path))} || umount {shlex.quote(str(path))}", + f"fusermount3 -u {shlex.quote(sandbox_path_str(path))} || " + f"umount {shlex.quote(sandbox_path_str(path))}", shell=False, ) if self.mode == "nfs": await session.exec( "sh", "-lc", - f"umount {shlex.quote(str(path))} >/dev/null 2>&1 || true", + f"umount {shlex.quote(sandbox_path_str(path))} >/dev/null 2>&1 || true", shell=False, ) diff --git a/src/agents/sandbox/manifest.py b/src/agents/sandbox/manifest.py index ed56e054ba..d4cc014870 100644 --- a/src/agents/sandbox/manifest.py +++ b/src/agents/sandbox/manifest.py @@ -1,7 +1,7 @@ import abc import asyncio from collections.abc import Iterator, Mapping -from pathlib import Path +from pathlib import Path, PurePath, PurePosixPath from typing import Literal from pydantic import BaseModel, Field, field_serializer, field_validator @@ -11,7 +11,12 @@ from .errors import InvalidManifestPathError from .manifest_render import render_manifest_description from .types import Group, User -from .workspace_paths import SandboxPathGrant +from .workspace_paths import ( + SandboxPathGrant, + coerce_posix_path, + posix_path_as_path, + windows_absolute_path, +) DEFAULT_REMOTE_MOUNT_COMMAND_ALLOWLIST = [ "ls", @@ -119,7 +124,7 @@ def ephemeral_entry_paths(self, depth: int | None = 1) -> set[Path]: return {path for path, artifact in self.iter_entries() if artifact.ephemeral} def mount_targets(self) -> list[tuple[Mount, Path]]: - root = Path(self.root) + root = posix_path_as_path(coerce_posix_path(self.root)) mounts: list[tuple[Mount, Path]] = [] for rel_path, artifact in self.iter_entries(): if not isinstance(artifact, Mount): @@ -138,7 +143,7 @@ def ephemeral_mount_targets(self) -> list[tuple[Mount, Path]]: def ephemeral_persistence_paths(self, depth: int | None = 1) -> set[Path]: _ = depth - root = Path(self.root) + root = posix_path_as_path(coerce_posix_path(self.root)) skip = self.ephemeral_entry_paths(depth=depth) for _mount, mount_path in self.ephemeral_mount_targets(): try: @@ -150,47 +155,69 @@ def ephemeral_persistence_paths(self, depth: int | None = 1) -> set[Path]: return skip @staticmethod - def _coerce_rel_path(path: str | Path) -> Path: - return path if isinstance(path, Path) else Path(path) + def _coerce_rel_path(path: str | PurePath) -> Path: + if (windows_path := windows_absolute_path(path)) is not None: + raise InvalidManifestPathError(rel=windows_path.as_posix(), reason="absolute") + return posix_path_as_path(coerce_posix_path(path)) @staticmethod def _validate_rel_path(rel: Path) -> None: - if rel.is_absolute(): - raise InvalidManifestPathError(rel=rel, reason="absolute") - if ".." in rel.parts: - raise InvalidManifestPathError(rel=rel, reason="escape_root") + if (windows_path := windows_absolute_path(rel)) is not None: + raise InvalidManifestPathError(rel=windows_path.as_posix(), reason="absolute") + rel_path = coerce_posix_path(rel) + if rel_path.is_absolute(): + raise InvalidManifestPathError(rel=rel_path.as_posix(), reason="absolute") + if ".." in rel_path.parts: + raise InvalidManifestPathError(rel=rel_path.as_posix(), reason="escape_root") @staticmethod def _normalize_rel_path_within_root(rel: Path, *, original: Path) -> Path: - if rel.is_absolute(): - raise InvalidManifestPathError(rel=original, reason="absolute") + rel_path = coerce_posix_path(rel) + original_path = coerce_posix_path(original) + if (windows_path := windows_absolute_path(original)) is not None: + raise InvalidManifestPathError(rel=windows_path.as_posix(), reason="absolute") + if rel_path.is_absolute(): + raise InvalidManifestPathError(rel=original_path.as_posix(), reason="absolute") normalized_parts: list[str] = [] - for part in rel.parts: + for part in rel_path.parts: if part in ("", "."): continue if part == "..": if not normalized_parts: - raise InvalidManifestPathError(rel=original, reason="escape_root") + raise InvalidManifestPathError( + rel=original_path.as_posix(), reason="escape_root" + ) normalized_parts.pop() continue normalized_parts.append(part) - return Path(*normalized_parts) + return posix_path_as_path(PurePosixPath(*normalized_parts)) @classmethod def _normalize_in_workspace_path(cls, root: Path, path: Path) -> Path | None: - if not path.is_absolute(): - normalized_rel = cls._normalize_rel_path_within_root(path, original=path) + root_path = coerce_posix_path(root) + if (windows_path := windows_absolute_path(path)) is not None: + raise InvalidManifestPathError(rel=windows_path.as_posix(), reason="absolute") + path_posix = coerce_posix_path(path) + if not path_posix.is_absolute(): + normalized_rel = cls._normalize_rel_path_within_root( + posix_path_as_path(path_posix), + original=posix_path_as_path(path_posix), + ) return root / normalized_rel if normalized_rel.parts else root try: - rel_path = path.relative_to(root) + rel_path = path_posix.relative_to(root_path) except ValueError: return None - normalized_rel = cls._normalize_rel_path_within_root(rel_path, original=path) - return root / normalized_rel if normalized_rel.parts else root + normalized_rel = cls._normalize_rel_path_within_root( + posix_path_as_path(rel_path), + original=posix_path_as_path(path_posix), + ) + root_as_path = posix_path_as_path(root_path) + return root_as_path / normalized_rel if normalized_rel.parts else root_as_path def iter_entries(self) -> Iterator[tuple[Path, BaseEntry]]: stack = [ diff --git a/src/agents/sandbox/manifest_render.py b/src/agents/sandbox/manifest_render.py index a6808e090b..61c127ee89 100644 --- a/src/agents/sandbox/manifest_render.py +++ b/src/agents/sandbox/manifest_render.py @@ -5,6 +5,7 @@ from ..logger import logger from .entries import BaseEntry, Dir, Mount +from .workspace_paths import coerce_posix_path, posix_path_as_path MAX_MANIFEST_DESCRIPTION_CHARS = 5000 MANIFEST_DESCRIPTION_TRUNCATION_MARKER_TEMPLATE = "... (truncated {omitted_chars} chars)" @@ -50,12 +51,16 @@ def render_manifest_description( raise ValueError("max_chars must be a non-zero positive integer or None") root = root.rstrip("/") or "/" - root_path = Path(root) + root_path = posix_path_as_path(coerce_posix_path(root)) def _mount_full_path(entry: str | Path, artifact: Mount) -> Path: if artifact.mount_path is not None: - mount_path = Path(artifact.mount_path) - return mount_path if mount_path.is_absolute() else root_path / mount_path + mount_path = coerce_posix_path(artifact.mount_path) + return posix_path_as_path( + mount_path + if mount_path.is_absolute() + else coerce_posix_path(root_path) / mount_path + ) return root_path / coerce_rel_path(entry) class _Node: @@ -66,7 +71,7 @@ def __init__(self) -> None: self.full_path: Path | None = None def _path_parts(path: Path) -> tuple[str, ...]: - parts = [part for part in path.parts if part not in {"", "."}] + parts = [part for part in coerce_posix_path(path).parts if part not in {"", "."}] return tuple(parts) root_node = _Node() @@ -148,9 +153,12 @@ def _collect( child_is_dir = child.is_dir or bool(child.children) display_name = f"{name}/" if child_is_dir else name if child.full_path is not None: - full_path = str(child.full_path) + full_path = child.full_path.as_posix() else: - full_path = str(root_path / Path(*current_rel_parts)) + full_path = ( + coerce_posix_path(root_path) + / coerce_posix_path("/".join(current_rel_parts)) + ).as_posix() lines.append((current_prefix, display_name, full_path, child.description)) continue diff --git a/src/agents/sandbox/sandboxes/docker.py b/src/agents/sandbox/sandboxes/docker.py index 27c117dd62..13eee0bc6d 100644 --- a/src/agents/sandbox/sandboxes/docker.py +++ b/src/agents/sandbox/sandboxes/docker.py @@ -71,6 +71,12 @@ retry_async, ) from ..util.tar_utils import UnsafeTarMemberError, strip_tar_member_prefix, validate_tarfile +from ..workspace_paths import ( + coerce_posix_path, + posix_path_as_path, + posix_path_for_error, + sandbox_path_str, +) _DOCKER_EXECUTOR: Final = ThreadPoolExecutor( max_workers=8, @@ -160,7 +166,9 @@ class DockerSandboxSession(BaseSandboxSession): _reserved_pty_process_ids: set[int] state: DockerSandboxSessionState - _ARCHIVE_STAGING_DIR: Path = Path("/tmp/sandbox-docker-archive") + _ARCHIVE_STAGING_DIR: Path = posix_path_as_path( + coerce_posix_path("/tmp/sandbox-docker-archive") + ) def __init__( self, @@ -290,7 +298,7 @@ async def _copy_workspace_tree_pruned( await self._exec_checked( "mkdir", "-p", - str(dst_child), + sandbox_path_str(dst_child), error_cls=WorkspaceArchiveReadError, error_path=src_child, ) @@ -306,8 +314,8 @@ async def _copy_workspace_tree_pruned( "cp", "-R", "--", - str(src_child), - str(dst_child), + sandbox_path_str(src_child), + sandbox_path_str(dst_child), error_cls=WorkspaceArchiveReadError, error_path=src_child, ) @@ -317,7 +325,7 @@ async def _stage_workspace_copy( *, skip_rel_paths: set[Path], ) -> tuple[Path, Path]: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() root_name = root.name or "workspace" staging_parent = self._archive_stage_path(name_hint="workspace") staging_workspace = staging_parent / root_name @@ -329,7 +337,7 @@ async def _stage_workspace_copy( await self._exec_checked( "mkdir", "-p", - str(staging_parent), + sandbox_path_str(staging_parent), error_cls=WorkspaceArchiveReadError, error_path=root, ) @@ -339,7 +347,7 @@ async def _stage_workspace_copy( await self._exec_checked( "mkdir", "-p", - str(staging_workspace), + sandbox_path_str(staging_workspace), error_cls=WorkspaceArchiveReadError, error_path=root, ) @@ -347,7 +355,7 @@ async def _stage_workspace_copy( await self._exec_checked( "mkdir", "-p", - str(staging_workspace), + sandbox_path_str(staging_workspace), error_cls=WorkspaceArchiveReadError, error_path=root, ) @@ -362,8 +370,8 @@ async def _stage_workspace_copy( "cp", "-R", "--", - str(root), - str(staging_workspace), + root.as_posix(), + sandbox_path_str(staging_workspace), error_cls=WorkspaceArchiveReadError, error_path=root, ) @@ -371,7 +379,7 @@ async def _stage_workspace_copy( async def _rm_best_effort(self, path: Path) -> None: try: - await self.exec("rm", "-rf", "--", str(path), shell=False) + await self.exec("rm", "-rf", "--", sandbox_path_str(path), shell=False) except Exception: pass @@ -629,7 +637,7 @@ async def _write_stream_via_exec( user: str | User | None = None, ) -> None: await self._stream_into_exec( - cmd=["sh", "-lc", 'cat > "$1"', "sh", str(staging_path)], + cmd=["sh", "-lc", 'cat > "$1"', "sh", sandbox_path_str(staging_path)], stream=stream, error_path=staging_path, user=user, @@ -643,7 +651,7 @@ async def _prepare_user_pty_pid_path(self, *, path: Path, user: str | None) -> N "-lc", _PREPARE_USER_PTY_PID_SCRIPT, "sh", - str(path), + sandbox_path_str(path), user, error_cls=WorkspaceArchiveWriteError, error_path=path, @@ -655,12 +663,13 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase # Read from inside the container instead of `get_archive()`: with Docker # volume-driver-backed mounts attached, daemon archive operations can re-run volume mount # setup and some plugins reject the duplicate `Mount` call for the same container id. - res = await self.exec("cat", "--", str(workspace_path), shell=False, user=user) + workspace_path_arg = sandbox_path_str(workspace_path) + res = await self.exec("cat", "--", workspace_path_arg, shell=False, user=user) if not res.ok(): raise WorkspaceReadNotFoundError( path=path, context={ - "command": ["cat", "--", str(workspace_path)], + "command": ["cat", "--", workspace_path_arg], "stdout": res.stdout.decode("utf-8", errors="replace"), "stderr": res.stderr.decode("utf-8", errors="replace"), }, @@ -685,7 +694,7 @@ async def write( "-lc", 'mkdir -p "$(dirname "$1")" && cat > "$1"', "sh", - str(path), + sandbox_path_str(path), ], stream=payload.stream, error_path=path, @@ -705,7 +714,7 @@ async def write( await self._exec_checked( "mkdir", "-p", - str(self._ARCHIVE_STAGING_DIR), + sandbox_path_str(self._ARCHIVE_STAGING_DIR), error_cls=WorkspaceArchiveWriteError, error_path=self._ARCHIVE_STAGING_DIR, ) @@ -716,12 +725,14 @@ async def write( ) # Copy into place using a process inside the container, which can see mounts. - cp_res = await self.exec("cp", "--", str(staging_path), str(path), shell=False) + staging_path_arg = sandbox_path_str(staging_path) + path_arg = sandbox_path_str(path) + cp_res = await self.exec("cp", "--", staging_path_arg, path_arg, shell=False) if not cp_res.ok(): raise WorkspaceArchiveWriteError( path=parent, context={ - "command": ["cp", "--", str(staging_path), str(path)], + "command": ["cp", "--", staging_path_arg, path_arg], "stdout": cp_res.stdout.decode("utf-8", errors="replace"), "stderr": cp_res.stderr.decode("utf-8", errors="replace"), }, @@ -808,8 +819,8 @@ async def pty_exec_start( "-lc", 'mkdir -p "$1" && printf "%s" "$$" > "$2" && shift 2 && exec "$@"', "sh", - str(pty_pid_path.parent), - str(pty_pid_path), + sandbox_path_str(pty_pid_path.parent), + sandbox_path_str(pty_pid_path), *cmd, ] resp = await asyncio.wait_for( @@ -1174,7 +1185,7 @@ async def _kill_pty_pid_path(self, pid_path: Path) -> None: "fi" ), "sh", - str(pid_path), + sandbox_path_str(pid_path), ], demux=True, ), @@ -1196,7 +1207,8 @@ async def exists(self) -> bool: ) async def persist_workspace(self) -> io.IOBase: skip = self._persist_workspace_skip_relpaths() - root = Path(self.state.manifest.root) + root = self._workspace_root_path() + error_root = posix_path_for_error(root) try: staging_parent, staging_workspace = await self._stage_workspace_copy( skip_rel_paths=skip @@ -1207,12 +1219,13 @@ async def persist_workspace(self) -> io.IOBase: ) return strip_tar_member_prefix(root_prefixed_archive, prefix=staging_workspace.name) except docker.errors.NotFound as e: - raise WorkspaceArchiveReadError(path=root, cause=e) from e + raise WorkspaceArchiveReadError(path=error_root, cause=e) from e except docker.errors.APIError as e: - raise WorkspaceArchiveReadError(path=root, cause=e) from e + raise WorkspaceArchiveReadError(path=error_root, cause=e) from e async def hydrate_workspace(self, data: io.IOBase) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() + error_root = posix_path_for_error(root) with tempfile.TemporaryFile() as archive: while True: chunk = data.read(io.DEFAULT_BUFFER_SIZE) @@ -1222,7 +1235,7 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: chunk = chunk.encode("utf-8") if not isinstance(chunk, bytes | bytearray): raise WorkspaceArchiveWriteError( - path=root, + path=error_root, context={"reason": "non_bytes_tar_payload"}, ) archive.write(chunk) @@ -1233,25 +1246,25 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: validate_tarfile(tar) except UnsafeTarMemberError as e: raise WorkspaceArchiveWriteError( - path=root, + path=error_root, context={"reason": e.reason, "member": e.member}, cause=e, ) from e except (tarfile.TarError, OSError) as e: - raise WorkspaceArchiveWriteError(path=root, cause=e) from e + raise WorkspaceArchiveWriteError(path=error_root, cause=e) from e await self._exec_checked( "mkdir", "-p", - str(root), + root.as_posix(), error_cls=WorkspaceArchiveWriteError, - error_path=root, + error_path=error_root, ) archive.seek(0) await self._stream_into_exec( - cmd=["tar", "-x", "-C", str(root)], + cmd=["tar", "-x", "-C", root.as_posix()], stream=archive, - error_path=root, + error_path=error_root, ) def _schedule_rm_best_effort(self, path: Path) -> None: @@ -1272,13 +1285,13 @@ def _workspace_archive_stream( container_client = getattr(self._container, "client", None) api = getattr(container_client, "api", None) if api is None: - bits, _ = self._container.get_archive(str(path)) + bits, _ = self._container.get_archive(sandbox_path_str(path)) return IteratorIO(it=cast(Iterator[bytes], bits), on_close=on_close) url = api._url("/containers/{0}/archive", self._container.id) response = api._get( url, - params={"path": str(path)}, + params={"path": sandbox_path_str(path)}, stream=True, headers={"Accept-Encoding": "identity"}, ) @@ -1525,7 +1538,7 @@ def _build_docker_volume_mounts( driver_name, driver_options, read_only = driver_config mounts.append( DockerSDKMount( - target=str(mount_path), + target=mount_path.as_posix(), source=_docker_volume_name(session_id=session_id, mount_path=mount_path), type="volume", read_only=read_only, @@ -1549,7 +1562,7 @@ def _docker_volume_names_for_manifest( def _docker_volume_mounts_for_manifest(manifest: Manifest) -> list[tuple[Mount, Path]]: mounts: list[tuple[Mount, Path]] = [] - root = Path(manifest.root) + root = posix_path_as_path(coerce_posix_path(manifest.root)) for rel_path, artifact in manifest.iter_entries(): if not isinstance(artifact, Mount): continue @@ -1571,6 +1584,7 @@ def _docker_volume_name(*, session_id: uuid.UUID | None, mount_path: Path) -> st # Keep the readable path suffix, but include a path hash so distinct mount # targets like `/workspace/a_b` and `/workspace/a/b` cannot alias after # slash replacement. - path_hash = hashlib.sha256(str(mount_path).encode("utf-8")).hexdigest()[:12] - sanitized = re.sub(r"[^A-Za-z0-9_.-]", "_", str(mount_path).strip("/")) or "workspace" + mount_path_posix = mount_path.as_posix() + path_hash = hashlib.sha256(mount_path_posix.encode("utf-8")).hexdigest()[:12] + sanitized = re.sub(r"[^A-Za-z0-9_.-]", "_", mount_path_posix.strip("/")) or "workspace" return f"sandbox_{session_prefix}{path_hash}_{sanitized}" diff --git a/src/agents/sandbox/session/base_sandbox_session.py b/src/agents/sandbox/session/base_sandbox_session.py index 572581466b..48e2ab563c 100644 --- a/src/agents/sandbox/session/base_sandbox_session.py +++ b/src/agents/sandbox/session/base_sandbox_session.py @@ -6,7 +6,7 @@ import shutil import tempfile from collections.abc import Awaitable, Callable, Mapping, Sequence -from pathlib import Path +from pathlib import Path, PurePath from typing import Literal, TypeVar, cast from typing_extensions import Self @@ -36,7 +36,13 @@ from ..snapshot import NoopSnapshot from ..types import ExecResult, ExposedPortEndpoint, User from ..util.parse_utils import parse_ls_la -from ..workspace_paths import WorkspacePathPolicy +from ..workspace_paths import ( + WorkspacePathPolicy, + coerce_posix_path, + posix_path_as_path, + posix_path_for_error, + sandbox_path_str, +) from .archive_extraction import ( WorkspaceArchiveExtractor, safe_zip_member_rel_path, @@ -109,7 +115,7 @@ class BaseSandboxSession(abc.ABC): _runtime_persist_workspace_skip_relpaths: set[Path] | None = None _pre_stop_hooks: list[Callable[[], Awaitable[None]]] | None = None _pre_stop_hooks_ran: bool = False - _runtime_helpers_installed: set[Path] | None = None + _runtime_helpers_installed: set[PurePath] | None = None _runtime_helper_cache_key: object = _RUNTIME_HELPER_CACHE_KEY_UNSET _workspace_path_policy_cache: ( tuple[str, tuple[tuple[str, bool], ...], WorkspacePathPolicy] | None @@ -446,7 +452,7 @@ def _workspace_relpaths_overlap(lhs: Path, rhs: Path) -> bool: return lhs == rhs or lhs in rhs.parents or rhs in lhs.parents def _mount_relpaths_within_workspace(self) -> set[Path]: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() mount_relpaths: set[Path] = set() for _mount_entry, mount_path in self.state.manifest.mount_targets(): try: @@ -631,7 +637,7 @@ def _sync_runtime_helper_install_cache(self) -> None: self._runtime_helpers_installed = None self._runtime_helper_cache_key = current_key - async def _ensure_runtime_helper_installed(self, helper: RuntimeHelperScript) -> Path: + async def _ensure_runtime_helper_installed(self, helper: RuntimeHelperScript) -> PurePath: self._sync_runtime_helper_install_cache() installed = self._runtime_helpers_installed if installed is None: @@ -685,6 +691,9 @@ def _workspace_path_policy(self) -> WorkspacePathPolicy: self._workspace_path_policy_cache = (root, grants_key, policy) return policy + def _workspace_root_path(self) -> Path: + return posix_path_as_path(self._workspace_path_policy().sandbox_root()) + async def _validate_path_access(self, path: Path | str, *, for_write: bool = False) -> Path: return self.normalize_path(path, for_write=for_write) @@ -701,20 +710,20 @@ async def _validate_remote_path_access( target, while still rejecting paths whose resolved remote target escapes all allowed roots. """ - original_path = Path(path) - root = Path(self.state.manifest.root) path_policy = self._workspace_path_policy() - workspace_path = path_policy.normalize_path(original_path, for_write=for_write) + root = path_policy.sandbox_root() + workspace_path = path_policy.normalize_sandbox_path(path, for_write=for_write) + original_path = coerce_posix_path(path) helper_path = await self._ensure_runtime_helper_installed(RESOLVE_WORKSPACE_PATH_HELPER) extra_grant_args = tuple( arg for root, read_only in path_policy.extra_path_grant_rules() - for arg in (str(root), "1" if read_only else "0") + for arg in (root.as_posix(), "1" if read_only else "0") ) command = ( str(helper_path), - str(root), - str(workspace_path), + root.as_posix(), + workspace_path.as_posix(), "1" if for_write else "0", *extra_grant_args, ) @@ -724,12 +733,12 @@ async def _validate_remote_path_access( if resolved: # Preserve the requested workspace path so leaf symlinks keep their normal # semantics while the remote realpath check still enforces path confinement. - return workspace_path + return posix_path_as_path(workspace_path) raise ExecTransportError( command=( "resolve_workspace_path", - str(root), - str(workspace_path), + root.as_posix(), + workspace_path.as_posix(), "1" if for_write else "0", *extra_grant_args, ), @@ -746,7 +755,7 @@ async def _validate_remote_path_access( ) if result.exit_code == 111: raise InvalidManifestPathError( - rel=original_path, + rel=original_path.as_posix(), reason=reason, context={ "resolved_path": result.stderr.decode("utf-8", errors="replace").strip(), @@ -762,13 +771,15 @@ async def _validate_remote_path_access( context["grant_path"] = line.removeprefix("read-only extra path grant: ") elif line.startswith("resolved path: "): context["resolved_path"] = line.removeprefix("resolved path: ") - raise WorkspaceArchiveWriteError(path=workspace_path, context=context) + raise WorkspaceArchiveWriteError( + path=posix_path_for_error(workspace_path), context=context + ) raise ExecNonZeroError( result, command=( "resolve_workspace_path", - str(root), - str(workspace_path), + root.as_posix(), + workspace_path.as_posix(), "1" if for_write else "0", *extra_grant_args, ), @@ -801,30 +812,36 @@ async def write( :param user: Optional sandbox user to perform the write as. """ - async def _check_read_with_exec(self, path: Path, *, user: str | User | None = None) -> Path: + async def _check_read_with_exec( + self, path: Path | str, *, user: str | User | None = None + ) -> Path: workspace_path = await self._validate_path_access(path) - cmd = ("sh", "-lc", '[ -r "$1" ]', "sh", str(workspace_path)) + path_arg = sandbox_path_str(workspace_path) + cmd = ("sh", "-lc", '[ -r "$1" ]', "sh", path_arg) result = await self.exec(*cmd, shell=False, user=user) if not result.ok(): raise WorkspaceReadNotFoundError( - path=path, + path=posix_path_as_path(coerce_posix_path(path)), context={ - "command": ["sh", "-lc", "", str(workspace_path)], + "command": ["sh", "-lc", "", path_arg], "stdout": result.stdout.decode("utf-8", errors="replace"), "stderr": result.stderr.decode("utf-8", errors="replace"), }, ) return workspace_path - async def _check_write_with_exec(self, path: Path, *, user: str | User | None = None) -> Path: + async def _check_write_with_exec( + self, path: Path | str, *, user: str | User | None = None + ) -> Path: workspace_path = await self._validate_path_access(path, for_write=True) - cmd = ("sh", "-lc", _WRITE_ACCESS_CHECK_SCRIPT, "sh", str(workspace_path)) + path_arg = sandbox_path_str(workspace_path) + cmd = ("sh", "-lc", _WRITE_ACCESS_CHECK_SCRIPT, "sh", path_arg) result = await self.exec(*cmd, shell=False, user=user) if not result.ok(): raise WorkspaceArchiveWriteError( path=workspace_path, context={ - "command": ["sh", "-lc", "", str(workspace_path)], + "command": ["sh", "-lc", "", path_arg], "stdout": result.stdout.decode("utf-8", errors="replace"), "stderr": result.stderr.decode("utf-8", errors="replace"), }, @@ -840,7 +857,8 @@ async def _check_mkdir_with_exec( ) -> Path: workspace_path = await self._validate_path_access(path, for_write=True) parents_flag = "1" if parents else "0" - cmd = ("sh", "-lc", _MKDIR_ACCESS_CHECK_SCRIPT, "sh", str(workspace_path), parents_flag) + path_arg = sandbox_path_str(workspace_path) + cmd = ("sh", "-lc", _MKDIR_ACCESS_CHECK_SCRIPT, "sh", path_arg, parents_flag) result = await self.exec(*cmd, shell=False, user=user) if not result.ok(): raise WorkspaceArchiveWriteError( @@ -850,7 +868,7 @@ async def _check_mkdir_with_exec( "sh", "-lc", "", - str(workspace_path), + path_arg, parents_flag, ], "stdout": result.stdout.decode("utf-8", errors="replace"), @@ -868,7 +886,8 @@ async def _check_rm_with_exec( ) -> Path: workspace_path = await self._validate_path_access(path, for_write=True) recursive_flag = "1" if recursive else "0" - cmd = ("sh", "-lc", _RM_ACCESS_CHECK_SCRIPT, "sh", str(workspace_path), recursive_flag) + path_arg = sandbox_path_str(workspace_path) + cmd = ("sh", "-lc", _RM_ACCESS_CHECK_SCRIPT, "sh", path_arg, recursive_flag) result = await self.exec(*cmd, shell=False, user=user) if not result.ok(): raise WorkspaceArchiveWriteError( @@ -878,7 +897,7 @@ async def _check_rm_with_exec( "sh", "-lc", "", - str(workspace_path), + path_arg, recursive_flag, ], "stdout": result.stdout.decode("utf-8", errors="replace"), @@ -924,12 +943,13 @@ async def ls( """ path = await self._validate_path_access(path) - cmd = ("ls", "-la", "--", str(path)) + path_arg = sandbox_path_str(path) + cmd = ("ls", "-la", "--", path_arg) result = await self.exec(*cmd, shell=False, user=user) if not result.ok(): raise ExecNonZeroError(result, command=cmd) - return parse_ls_la(result.stdout.decode("utf-8", errors="replace"), base=str(path)) + return parse_ls_la(result.stdout.decode("utf-8", errors="replace"), base=path_arg) async def rm( self, @@ -949,7 +969,7 @@ async def rm( cmd: list[str] = ["rm"] if recursive: cmd.append("-rf") - cmd.extend(["--", str(path)]) + cmd.extend(["--", sandbox_path_str(path)]) result = await self.exec(*cmd, shell=False, user=user) if not result.ok(): @@ -973,7 +993,7 @@ async def mkdir( cmd: list[str] = ["mkdir"] if parents: cmd.append("-p") - cmd.append(str(path)) + cmd.append(sandbox_path_str(path)) result = await self.exec(*cmd, shell=False, user=user) if not result.ok(): @@ -1173,11 +1193,12 @@ async def _can_skip_snapshot_restore_on_resume(self, *, is_running: bool) -> boo def _snapshot_fingerprint_cache_path(self) -> Path: """Return the runtime-owned path for this session's cached snapshot fingerprint.""" - return ( - Path("/tmp/openai-agents/session-state") - / self.state.session_id.hex - / "fingerprint.json" + cache_path = coerce_posix_path( + f"/tmp/openai-agents/session-state/{self.state.session_id.hex}/fingerprint.json" ) + if self._workspace_path_policy().root_is_existing_host_path(): + return Path(cache_path.as_posix()) + return posix_path_as_path(cache_path) def _workspace_fingerprint_skip_relpaths(self) -> set[Path]: """Return workspace paths that should be omitted from snapshot fingerprinting.""" @@ -1192,9 +1213,9 @@ async def _compute_and_cache_snapshot_fingerprint(self) -> dict[str, str]: helper_path = await self._ensure_runtime_helper_installed(WORKSPACE_FINGERPRINT_HELPER) command = [ str(helper_path), - str(self.state.manifest.root), + self._workspace_root_path().as_posix(), self._snapshot_fingerprint_version(), - str(self._snapshot_fingerprint_cache_path()), + self._snapshot_fingerprint_cache_path().as_posix(), self._resume_manifest_digest(), ] command.extend( @@ -1215,13 +1236,13 @@ async def _read_cached_snapshot_fingerprint(self) -> dict[str, str]: result = await self.exec( "cat", "--", - str(self._snapshot_fingerprint_cache_path()), + self._snapshot_fingerprint_cache_path().as_posix(), shell=False, ) if not result.ok(): raise ExecNonZeroError( result, - command=("cat", str(self._snapshot_fingerprint_cache_path())), + command=("cat", self._snapshot_fingerprint_cache_path().as_posix()), ) return self._parse_snapshot_fingerprint_record(result.stdout) @@ -1250,7 +1271,7 @@ async def _delete_cached_snapshot_fingerprint_best_effort(self) -> None: "rm", "-f", "--", - str(self._snapshot_fingerprint_cache_path()), + self._snapshot_fingerprint_cache_path().as_posix(), shell=False, ) except Exception: @@ -1313,12 +1334,12 @@ async def _clear_workspace_root_on_resume(self) -> None: return await self._clear_workspace_dir_on_resume_pruned( - current_dir=Path(self.state.manifest.root), + current_dir=self._workspace_root_path(), skip_rel_paths=skip_rel_paths, ) def _workspace_resume_mount_skip_relpaths(self) -> set[Path]: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() skip_rel_paths: set[Path] = set() for _mount, mount_path in self.state.manifest.ephemeral_mount_targets(): try: @@ -1333,7 +1354,7 @@ async def _clear_workspace_dir_on_resume_pruned( current_dir: Path, skip_rel_paths: set[Path], ) -> None: - root = Path(self.state.manifest.root) + root = self._workspace_root_path() try: entries = await self.ls(current_dir) except ExecNonZeroError: diff --git a/src/agents/sandbox/session/manifest_application.py b/src/agents/sandbox/session/manifest_application.py index 18eab27034..bb3569a9fa 100644 --- a/src/agents/sandbox/session/manifest_application.py +++ b/src/agents/sandbox/session/manifest_application.py @@ -8,6 +8,7 @@ from ..manifest import Manifest from ..materialization import MaterializationResult, MaterializedFile, gather_in_order from ..types import ExecResult, User +from ..workspace_paths import coerce_posix_path, posix_path_as_path class ManifestApplier: @@ -34,9 +35,10 @@ async def apply_manifest( provision_accounts: bool = True, base_dir: Path | None = None, ) -> MaterializationResult: - base_dir = Path("/") if base_dir is None else base_dir + base_dir = posix_path_as_path(coerce_posix_path("/")) if base_dir is None else base_dir + root = posix_path_as_path(coerce_posix_path(manifest.root)) - await self._mkdir(Path(manifest.root)) + await self._mkdir(root) if provision_accounts and not only_ephemeral: await self.provision_accounts(manifest) @@ -44,12 +46,12 @@ async def apply_manifest( entries_to_apply: list[tuple[Path, BaseEntry]] = [] if only_ephemeral: for rel_dest, artifact in self._ephemeral_entries(manifest): - dest = resolve_workspace_path(Path(manifest.root), rel_dest) + dest = resolve_workspace_path(root, rel_dest) entries_to_apply.append((dest, artifact)) else: for raw_rel_dest, artifact in manifest.validated_entries().items(): dest = resolve_workspace_path( - Path(manifest.root), + root, Manifest._coerce_rel_path(raw_rel_dest), ) entries_to_apply.append((dest, artifact)) diff --git a/src/agents/sandbox/session/runtime_helpers.py b/src/agents/sandbox/session/runtime_helpers.py index 724b27893b..8ab58a1fcb 100644 --- a/src/agents/sandbox/session/runtime_helpers.py +++ b/src/agents/sandbox/session/runtime_helpers.py @@ -2,10 +2,10 @@ import hashlib from dataclasses import dataclass -from pathlib import Path +from pathlib import PurePath, PurePosixPath from typing import Final -_HELPER_INSTALL_ROOT: Final[Path] = Path("/tmp/openai-agents/bin") +_HELPER_INSTALL_ROOT: Final[PurePosixPath] = PurePosixPath("/tmp/openai-agents/bin") _INSTALL_MARKER: Final[str] = "INSTALL_RUNTIME_HELPER_V1" _RESOLVE_WORKSPACE_PATH_SCRIPT: Final[str] = """ @@ -249,7 +249,7 @@ class RuntimeHelperScript: name: str content: str - install_path: Path + install_path: PurePath install_marker: str = _INSTALL_MARKER @classmethod diff --git a/src/agents/sandbox/util/tar_utils.py b/src/agents/sandbox/util/tar_utils.py index 9b84f5e32a..ec1f876fca 100644 --- a/src/agents/sandbox/util/tar_utils.py +++ b/src/agents/sandbox/util/tar_utils.py @@ -171,7 +171,9 @@ def _ensure_no_symlink_parents(*, root: Path, dest: Path, check_leaf: bool = Tru path_to_resolve = dest if check_leaf else dest.parent dest_resolved = path_to_resolve.resolve() if not (dest_resolved == root_resolved or dest_resolved.is_relative_to(root_resolved)): - raise UnsafeTarMemberError(member=str(dest), reason="path escapes root after resolution") + raise UnsafeTarMemberError( + member=dest.as_posix(), reason="path escapes root after resolution" + ) rel = dest.relative_to(root) cur = root diff --git a/src/agents/sandbox/workspace_paths.py b/src/agents/sandbox/workspace_paths.py index 866be35731..bc281f69e2 100644 --- a/src/agents/sandbox/workspace_paths.py +++ b/src/agents/sandbox/workspace_paths.py @@ -1,8 +1,8 @@ from __future__ import annotations import posixpath -from pathlib import Path, PurePath, PurePosixPath -from typing import Literal +from pathlib import Path, PurePath, PurePosixPath, PureWindowsPath +from typing import Literal, cast from pydantic import BaseModel, field_validator @@ -24,6 +24,51 @@ def _raise_if_filesystem_root(path: PurePath, *, resolved: bool = False) -> None raise ValueError(_ROOT_PATH_GRANT_ERROR) +def coerce_posix_path(path: str | PurePath) -> PurePosixPath: + """Return a POSIX-flavored path for sandbox filesystem paths.""" + + if isinstance(path, PurePath): + path = path.as_posix() + else: + path = path.replace("\\", "/") + return PurePosixPath(path) + + +def windows_absolute_path(path: str | PurePath) -> PureWindowsPath | None: + """Return a Windows absolute path when the input uses Windows absolute syntax.""" + + if isinstance(path, PureWindowsPath): + windows_path = path + else: + windows_path = PureWindowsPath(path.as_posix() if isinstance(path, PurePath) else path) + if windows_path.is_absolute() and not PurePosixPath(windows_path.as_posix()).is_absolute(): + return windows_path + return None + + +def posix_path_as_path(path: PurePosixPath) -> Path: + """Return a POSIX path through the public Path-typed sandbox API surface.""" + + return Path(path.as_posix()) + + +def posix_path_for_error(path: str | PurePath) -> Path: + """Return a POSIX path object for sandbox error text and context.""" + + return cast(Path, coerce_posix_path(path)) + + +def sandbox_path_str(path: str | PurePath) -> str: + """Return a POSIX string for a sandbox filesystem path.""" + + return coerce_posix_path(path).as_posix() + + +def _native_path_from_windows_absolute(path: PureWindowsPath) -> Path | None: + native_path = Path(path) + return native_path if native_path.is_absolute() else None + + class SandboxPathGrant(BaseModel): """Extra absolute path access outside the sandbox workspace.""" @@ -34,7 +79,7 @@ class SandboxPathGrant(BaseModel): @field_validator("path", mode="before") @classmethod def _coerce_path(cls, value: object) -> str: - if isinstance(value, Path): + if isinstance(value, PurePath): return value.as_posix() if isinstance(value, str): return value @@ -43,11 +88,19 @@ def _coerce_path(cls, value: object) -> str: @field_validator("path") @classmethod def _validate_path(cls, value: str) -> str: + if (windows_path := windows_absolute_path(value)) is not None: + native_path = _native_path_from_windows_absolute(windows_path) + if native_path is not None: + _raise_if_filesystem_root(native_path) + return str(native_path) + raise ValueError("sandbox path grant path must be POSIX absolute") + path = PurePosixPath(posixpath.normpath(value)) - if not path.is_absolute(): - raise ValueError("sandbox path grant path must be absolute") - _raise_if_filesystem_root(path) - return path.as_posix() + if path.is_absolute(): + _raise_if_filesystem_root(path) + return path.as_posix() + + raise ValueError("sandbox path grant path must be absolute") class WorkspacePathPolicy: @@ -56,13 +109,15 @@ class WorkspacePathPolicy: def __init__( self, *, - root: str | Path, + root: str | PurePath, extra_path_grants: tuple[SandboxPathGrant, ...] = (), ) -> None: self._root = Path(root) + self._sandbox_root = coerce_posix_path(root) + self._root_is_existing_host_path = self._path_exists(self._root) self._extra_path_grants = extra_path_grants - def absolute_workspace_path(self, path: str | Path) -> Path: + def absolute_workspace_path(self, path: str | PurePath) -> Path: """Return an absolute workspace path without following symlinks. Examples with root `/workspace`: @@ -71,10 +126,16 @@ def absolute_workspace_path(self, path: str | Path) -> Path: - `absolute_workspace_path("/tmp/app.py")` raises `InvalidManifestPathError`. """ - normalized = self._absolute_workspace_posix_path(Path(path)) - return Path(str(normalized)) + if (windows_path := windows_absolute_path(path)) is not None: + native_path = _native_path_from_windows_absolute(windows_path) + if self._root_is_existing_host_path and native_path is not None: + result, _grant = self._resolved_host_path_and_grant(native_path) + return result + raise self._invalid_path_error(windows_path) + normalized = self._absolute_workspace_posix_path(coerce_posix_path(path)) + return self._path_result(normalized) - def relative_path(self, path: str | Path) -> Path: + def relative_path(self, path: str | PurePath) -> Path: """Return a path relative to the workspace root. Examples with root `/workspace`: @@ -83,14 +144,20 @@ def relative_path(self, path: str | Path) -> Path: - `relative_path("/workspace")` returns `.`. """ - normalized = self._absolute_workspace_posix_path(Path(path)) + if (windows_path := windows_absolute_path(path)) is not None: + raise self._invalid_path_error(windows_path) + normalized = self._absolute_workspace_posix_path(coerce_posix_path(path)) root = self._normalized_root() - relative = normalized.relative_to(root) - return Path(str(relative)) if relative.parts else Path(".") + posix_relative = normalized.relative_to(root) + return ( + self._path_result(posix_relative) + if posix_relative.parts + else self._path_result(PurePosixPath(".")) + ) def normalize_path( self, - path: str | Path, + path: str | PurePath, *, for_write: bool = False, resolve_symlinks: bool = False, @@ -101,15 +168,55 @@ def normalize_path( workspace is a real local host directory, such as UnixLocalSandboxSession. """ - original = Path(path) if resolve_symlinks: + if (windows_path := windows_absolute_path(path)) is not None: + original = _native_path_from_windows_absolute(windows_path) + if original is None: + raise self._invalid_path_error(windows_path) + else: + original = Path(path) result, grant = self._resolved_host_path_and_grant(original) else: - result, grant = self._sandbox_path_and_grant(original) + if (windows_path := windows_absolute_path(path)) is not None: + native_path = _native_path_from_windows_absolute(windows_path) + if self._root_is_existing_host_path and native_path is not None: + result, grant = self._resolved_host_path_and_grant(native_path) + if for_write: + self._raise_if_read_only_grant(result, grant) + return result + raise self._invalid_path_error(windows_path) + sandbox_result, grant = self._sandbox_path_and_grant(coerce_posix_path(path)) + result = self._path_result(sandbox_result) if for_write: self._raise_if_read_only_grant(result, grant) return result + def normalize_sandbox_path( + self, + path: str | PurePath, + *, + for_write: bool = False, + ) -> PurePosixPath: + """Return a validated POSIX path for a Unix-like remote sandbox filesystem.""" + + if (windows_path := windows_absolute_path(path)) is not None: + raise self._invalid_path_error(windows_path) + original = coerce_posix_path(path) + result, grant = self._sandbox_path_and_grant(original) + if for_write: + self._raise_if_read_only_grant(posix_path_for_error(result), grant) + return result + + def sandbox_root(self) -> PurePosixPath: + """Return the workspace root as a POSIX path for remote sandbox commands.""" + + return self._normalized_root() + + def root_is_existing_host_path(self) -> bool: + """Return whether the configured root currently exists on the host filesystem.""" + + return self._root_is_existing_host_path + def _resolved_host_path_and_grant( self, original: Path, @@ -118,7 +225,7 @@ def _resolved_host_path_and_grant( if original.is_absolute(): resolved = original.resolve(strict=False) else: - absolute = self._absolute_workspace_posix_path(original) + absolute = self._absolute_workspace_posix_path(coerce_posix_path(original)) resolved = Path(str(absolute)).resolve(strict=False) if self._is_under(resolved, workspace_root): @@ -130,18 +237,18 @@ def _resolved_host_path_and_grant( def _sandbox_path_and_grant( self, - original: Path, - ) -> tuple[Path, SandboxPathGrant | None]: + original: PurePosixPath, + ) -> tuple[PurePosixPath, SandboxPathGrant | None]: normalized = ( self._absolute_posix_path(original) if original.is_absolute() else self._absolute_workspace_posix_path(original) ) if self._is_under(normalized, self._normalized_root()): - return Path(str(normalized)), None + return normalized, None grant = self._matching_grant(normalized) if original.is_absolute() and grant is not None: - return Path(str(normalized)), grant + return normalized, grant raise self._invalid_path_error(original) def _raise_if_read_only_grant( @@ -151,25 +258,28 @@ def _raise_if_read_only_grant( ) -> None: if grant is None or not grant.read_only: return + error_path = path if self._root_is_existing_host_path else posix_path_for_error(path) raise WorkspaceArchiveWriteError( - path=path, + path=error_path, context={ "reason": "read_only_extra_path_grant", "grant_path": grant.path, }, ) - def extra_path_grant_rules(self) -> tuple[tuple[Path, bool], ...]: + def extra_path_grant_rules(self) -> tuple[tuple[PurePosixPath, bool], ...]: """Return normalized extra grant roots and access modes for remote realpath checks.""" - rules: list[tuple[Path, bool]] = [] + rules: list[tuple[PurePosixPath, bool]] = [] for grant in self._extra_path_grants: - root = Path(grant.path) + if windows_absolute_path(grant.path) is not None: + raise ValueError("sandbox path grant path must be POSIX absolute") + root = coerce_posix_path(grant.path) _raise_if_filesystem_root(root) rules.append((root, grant.read_only)) return tuple(rules) - def _absolute_workspace_posix_path(self, path: Path) -> PurePosixPath: + def _absolute_workspace_posix_path(self, path: PurePosixPath) -> PurePosixPath: normalized = self._absolute_posix_path(path) root = self._normalized_root() try: @@ -178,13 +288,25 @@ def _absolute_workspace_posix_path(self, path: Path) -> PurePosixPath: raise self._invalid_path_error(path, cause=exc) from exc return normalized - def _absolute_posix_path(self, path: Path) -> PurePosixPath: + def _absolute_posix_path(self, path: PurePosixPath) -> PurePosixPath: root = self._normalized_root() raw_candidate = path.as_posix() if path.is_absolute() else str(root / path.as_posix()) return PurePosixPath(posixpath.normpath(str(raw_candidate))) def _normalized_root(self) -> PurePosixPath: - return PurePosixPath(posixpath.normpath(self._root.as_posix())) + return PurePosixPath(posixpath.normpath(self._sandbox_root.as_posix())) + + @staticmethod + def _path_exists(path: Path) -> bool: + try: + return path.exists() + except OSError: + return False + + def _path_result(self, path: PurePosixPath) -> Path: + if self._root_is_existing_host_path: + return Path(path.as_posix()) + return posix_path_as_path(path) def _matching_grant( self, @@ -197,7 +319,7 @@ def _matching_grant( grant_root: PurePath = ( Path(grant.path).resolve(strict=False) if resolve_roots - else PurePosixPath(grant.path) + else coerce_posix_path(grant.path) ) _raise_if_filesystem_root(grant_root, resolved=resolve_roots) if self._is_under(path, grant_root): @@ -212,11 +334,11 @@ def _is_under(path: PurePath, root: PurePath) -> bool: def _invalid_path_error( self, - path: Path, + path: PurePath, *, cause: BaseException | None = None, ) -> InvalidManifestPathError: reason: Literal["absolute", "escape_root"] = ( "absolute" if path.is_absolute() else "escape_root" ) - return InvalidManifestPathError(rel=path, reason=reason, cause=cause) + return InvalidManifestPathError(rel=path.as_posix(), reason=reason, cause=cause) diff --git a/tests/conftest.py b/tests/conftest.py index 8fd3a0794e..21a3f6d7b5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,7 @@ from __future__ import annotations +import sys + import pytest from agents.models import _openai_shared @@ -11,6 +13,27 @@ from .testing_processor import SPAN_PROCESSOR_TESTING +collect_ignore: list[str] = [] + +if sys.platform == "win32": + collect_ignore.extend( + [ + "test_example_workflows.py", + "test_run_state.py", + "test_sandbox_memory.py", + "sandbox/capabilities/test_filesystem_capability.py", + "sandbox/integration_tests/test_runner_pause_resume.py", + "sandbox/test_client_options.py", + "sandbox/test_exposed_ports.py", + "sandbox/test_extract.py", + "sandbox/test_runtime.py", + "sandbox/test_session_manager.py", + "sandbox/test_session_sinks.py", + "sandbox/test_snapshot.py", + "sandbox/test_unix_local.py", + ] + ) + # This fixture will run once before any tests are executed @pytest.fixture(scope="session", autouse=True) diff --git a/tests/extensions/memory/test_dapr_redis_integration.py b/tests/extensions/memory/test_dapr_redis_integration.py index 1d52560ab3..05d1b78005 100644 --- a/tests/extensions/memory/test_dapr_redis_integration.py +++ b/tests/extensions/memory/test_dapr_redis_integration.py @@ -12,6 +12,7 @@ import asyncio import os import shutil +import sys import tempfile import time import urllib.request @@ -23,6 +24,11 @@ # Skip tests if dependencies are not available pytest.importorskip("dapr") # Skip tests if Dapr is not installed pytest.importorskip("testcontainers") # Skip if testcontainers is not installed +if sys.platform == "win32": + pytest.skip( + "Dapr Docker integration tests are not supported on Windows", + allow_module_level=True, + ) if shutil.which("docker") is None: pytest.skip( "Docker executable is not available; skipping Dapr integration tests", diff --git a/tests/extensions/test_sandbox_blaxel.py b/tests/extensions/test_sandbox_blaxel.py index 76a1e70b28..28e60a53e6 100644 --- a/tests/extensions/test_sandbox_blaxel.py +++ b/tests/extensions/test_sandbox_blaxel.py @@ -480,7 +480,7 @@ async def test_shutdown_pause_on_exit(self, fake_sandbox: _FakeSandboxInstance) async def test_normalize_path_relative(self, fake_sandbox: _FakeSandboxInstance) -> None: session = _make_session(fake_sandbox) result = session.normalize_path("subdir/file.txt") - assert str(result) == "/workspace/subdir/file.txt" + assert result.as_posix() == "/workspace/subdir/file.txt" @pytest.mark.asyncio async def test_normalize_path_escape_blocked(self, fake_sandbox: _FakeSandboxInstance) -> None: diff --git a/tests/extensions/test_sandbox_cloudflare.py b/tests/extensions/test_sandbox_cloudflare.py index 52051b5a8a..08995ffd9e 100644 --- a/tests/extensions/test_sandbox_cloudflare.py +++ b/tests/extensions/test_sandbox_cloudflare.py @@ -707,7 +707,7 @@ async def test_cloudflare_mount_and_unmount_validate_path_access_for_write() -> calls: list[tuple[str, bool]] = [] async def _tracking_normalize(path: Path | str, *, for_write: bool = False) -> Path: - calls.append((str(path), for_write)) + calls.append((Path(path).as_posix(), for_write)) return sess.normalize_path(path, for_write=for_write) sess._validate_path_access = _tracking_normalize # type: ignore[method-assign] @@ -1273,7 +1273,7 @@ async def test_cloudflare_read_validates_path_access() -> None: calls: list[tuple[str, bool]] = [] async def _tracking_normalize(path: Path | str, *, for_write: bool = False) -> Path: - calls.append((str(path), for_write)) + calls.append((Path(path).as_posix(), for_write)) # Fall back to synchronous normalize_path to avoid needing a real remote. return sess.normalize_path(path, for_write=for_write) @@ -1292,7 +1292,7 @@ async def test_cloudflare_write_validates_path_access_for_write() -> None: calls: list[tuple[str, bool]] = [] async def _tracking_normalize(path: Path | str, *, for_write: bool = False) -> Path: - calls.append((str(path), for_write)) + calls.append((Path(path).as_posix(), for_write)) return sess.normalize_path(path, for_write=for_write) sess._validate_path_access = _tracking_normalize # type: ignore[method-assign] diff --git a/tests/extensions/test_sandbox_daytona.py b/tests/extensions/test_sandbox_daytona.py index 9bb3d9415c..55a3a35a03 100644 --- a/tests/extensions/test_sandbox_daytona.py +++ b/tests/extensions/test_sandbox_daytona.py @@ -389,7 +389,7 @@ async def activate( ) -> list[MaterializedFile]: _ = (strategy, session, base_dir) path = mount._resolve_mount_path(session, dest) - mount._events.append(("mount", str(path))) + mount._events.append(("mount", path.as_posix())) mount._mounted_paths.append(path) return [] @@ -402,7 +402,7 @@ async def deactivate( ) -> None: _ = (strategy, session, base_dir) path = mount._resolve_mount_path(session, dest) - mount._events.append(("unmount", str(path))) + mount._events.append(("unmount", path.as_posix())) mount._unmounted_paths.append(path) async def teardown_for_snapshot( @@ -412,7 +412,7 @@ async def teardown_for_snapshot( path: Path, ) -> None: _ = (strategy, session) - mount._events.append(("unmount", str(path))) + mount._events.append(("unmount", path.as_posix())) mount._unmounted_paths.append(path) async def restore_after_snapshot( @@ -422,19 +422,19 @@ async def restore_after_snapshot( path: Path, ) -> None: _ = (strategy, session) - mount._events.append(("mount", str(path))) + mount._events.append(("mount", path.as_posix())) mount._mounted_paths.append(path) return _Adapter(self) async def mount(self, session: object, path: Path) -> None: _ = session - self._events.append(("mount", str(path))) + self._events.append(("mount", path.as_posix())) self._mounted_paths.append(path) async def unmount_path(self, session: object, path: Path) -> None: _ = session - self._events.append(("unmount", str(path))) + self._events.append(("unmount", path.as_posix())) self._unmounted_paths.append(path) @@ -467,7 +467,7 @@ async def deactivate( ) -> None: _ = (strategy, session, base_dir) path = mount._resolve_mount_path(session, dest) - mount._events.append(("unmount_fail", str(path))) + mount._events.append(("unmount_fail", path.as_posix())) raise RuntimeError("boom while unmounting second mount") async def teardown_for_snapshot( @@ -477,7 +477,7 @@ async def teardown_for_snapshot( path: Path, ) -> None: _ = (strategy, session) - mount._events.append(("unmount_fail", str(path))) + mount._events.append(("unmount_fail", path.as_posix())) raise RuntimeError("boom while unmounting second mount") async def restore_after_snapshot( @@ -492,7 +492,7 @@ async def restore_after_snapshot( async def unmount_path(self, session: object, path: Path) -> None: _ = session - self._events.append(("unmount_fail", str(path))) + self._events.append(("unmount_fail", path.as_posix())) raise RuntimeError("boom while unmounting second mount") diff --git a/tests/extensions/test_sandbox_e2b.py b/tests/extensions/test_sandbox_e2b.py index 3a64ff0497..a7bbc8bd1f 100644 --- a/tests/extensions/test_sandbox_e2b.py +++ b/tests/extensions/test_sandbox_e2b.py @@ -520,7 +520,7 @@ async def activate( ) -> list[MaterializedFile]: _ = (strategy, session, base_dir) path = mount._resolve_mount_path(session, dest) - mount._events.append(("mount", str(path))) + mount._events.append(("mount", path.as_posix())) mount._mounted_paths.append(path) return [] @@ -533,7 +533,7 @@ async def deactivate( ) -> None: _ = (strategy, session, base_dir) path = mount._resolve_mount_path(session, dest) - mount._events.append(("unmount", str(path))) + mount._events.append(("unmount", path.as_posix())) mount._unmounted_paths.append(path) async def teardown_for_snapshot( @@ -543,7 +543,7 @@ async def teardown_for_snapshot( path: Path, ) -> None: _ = (strategy, session) - mount._events.append(("unmount", str(path))) + mount._events.append(("unmount", path.as_posix())) mount._unmounted_paths.append(path) async def restore_after_snapshot( @@ -553,7 +553,7 @@ async def restore_after_snapshot( path: Path, ) -> None: _ = (strategy, session) - mount._events.append(("mount", str(path))) + mount._events.append(("mount", path.as_posix())) mount._mounted_paths.append(path) return _Adapter(self) @@ -588,7 +588,7 @@ async def deactivate( ) -> None: _ = (strategy, session, base_dir) path = mount._resolve_mount_path(session, dest) - mount._events.append(("unmount_fail", str(path))) + mount._events.append(("unmount_fail", path.as_posix())) raise RuntimeError("boom while unmounting second mount") async def teardown_for_snapshot( @@ -598,7 +598,7 @@ async def teardown_for_snapshot( path: Path, ) -> None: _ = (strategy, session) - mount._events.append(("unmount_fail", str(path))) + mount._events.append(("unmount_fail", path.as_posix())) raise RuntimeError("boom while unmounting second mount") async def restore_after_snapshot( @@ -632,7 +632,7 @@ async def activate( ) -> list[MaterializedFile]: _ = (strategy, session, base_dir) path = mount._resolve_mount_path(session, dest) - mount._events.append(("mount_fail", str(path))) + mount._events.append(("mount_fail", path.as_posix())) raise RuntimeError("boom while remounting second mount") async def deactivate( @@ -659,7 +659,7 @@ async def restore_after_snapshot( path: Path, ) -> None: _ = (strategy, session) - mount._events.append(("mount_fail", str(path))) + mount._events.append(("mount_fail", path.as_posix())) raise RuntimeError("boom while remounting second mount") return _Adapter(self) diff --git a/tests/extensions/test_sandbox_modal.py b/tests/extensions/test_sandbox_modal.py index ddaa4c76e8..335cdffeda 100644 --- a/tests/extensions/test_sandbox_modal.py +++ b/tests/extensions/test_sandbox_modal.py @@ -9,7 +9,7 @@ import tarfile import types from collections.abc import Callable -from pathlib import Path +from pathlib import Path, PureWindowsPath from typing import Any, NoReturn, cast import pytest @@ -125,7 +125,7 @@ async def teardown_for_snapshot( _ = (strategy, session) if mount._teardown_error is not None: raise RuntimeError(mount._teardown_error) - mount._events.append(("unmount", str(path))) + mount._events.append(("unmount", path.as_posix())) async def restore_after_snapshot( self, @@ -134,7 +134,7 @@ async def restore_after_snapshot( path: Path, ) -> None: _ = (strategy, session) - mount._events.append(("mount", str(path))) + mount._events.append(("mount", path.as_posix())) return _Adapter(self) @@ -1659,7 +1659,73 @@ async def _fake_exec( normalized = await session._validate_path_access("link.txt") # noqa: SLF001 - assert normalized == Path("/workspace/link.txt") + assert normalized.as_posix() == "/workspace/link.txt" + + +@pytest.mark.asyncio +async def test_modal_normalize_path_uses_posix_commands_for_windows_paths( + monkeypatch: pytest.MonkeyPatch, +) -> None: + modal_module, _create_calls, _registry_tags = _load_modal_module(monkeypatch) + state = modal_module.ModalSandboxSessionState( + manifest=Manifest(root="/workspace"), + snapshot=modal_module.resolve_snapshot(None, "snapshot"), + app_name="sandbox-tests", + ) + session = modal_module.ModalSandboxSession.from_state(state) + commands: list[list[str]] = [] + + async def _fake_exec( + *command: object, + timeout: float | None = None, + shell: bool | list[str] = True, + user: object | None = None, + ) -> ExecResult: + _ = (timeout, shell, user) + rendered = [str(part) for part in command] + commands.append(rendered) + if ( + rendered[:2] == ["sh", "-c"] + and RESOLVE_WORKSPACE_PATH_HELPER.install_marker in rendered[2] + ): + return ExecResult(stdout=b"", stderr=b"", exit_code=0) + if rendered and rendered[0] == str(RESOLVE_WORKSPACE_PATH_HELPER.install_path): + return ExecResult(stdout=b"/workspace/link.txt", stderr=b"", exit_code=0) + raise AssertionError(f"unexpected command: {rendered!r}") + + monkeypatch.setattr(session, "exec", _fake_exec) + + normalized = await session._validate_path_access(PureWindowsPath("/workspace/link.txt")) # noqa: SLF001 + + helper_path = str(RESOLVE_WORKSPACE_PATH_HELPER.install_path) + assert normalized.as_posix() == "/workspace/link.txt" + assert commands[-1] == [helper_path, "/workspace", "/workspace/link.txt", "0"] + assert all("\\" not in arg for arg in commands[-1]) + + +@pytest.mark.asyncio +async def test_modal_normalize_path_rejects_windows_drive_absolute_paths( + monkeypatch: pytest.MonkeyPatch, +) -> None: + modal_module, _create_calls, _registry_tags = _load_modal_module(monkeypatch) + state = modal_module.ModalSandboxSessionState( + manifest=Manifest(root="/workspace"), + snapshot=modal_module.resolve_snapshot(None, "snapshot"), + app_name="sandbox-tests", + ) + session = modal_module.ModalSandboxSession.from_state(state) + + async def _fake_exec(*args: object, **kwargs: object) -> ExecResult: + _ = (args, kwargs) + raise AssertionError("path validation should reject before remote helper execution") + + monkeypatch.setattr(session, "exec", _fake_exec) + + with pytest.raises(InvalidManifestPathError) as exc_info: + await session._validate_path_access(PureWindowsPath("C:/tmp/link.txt")) # noqa: SLF001 + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/link.txt" + assert exc_info.value.context == {"rel": "C:/tmp/link.txt", "reason": "absolute"} @pytest.mark.asyncio @@ -1735,16 +1801,16 @@ async def _fake_exec( monkeypatch.setattr(session, "exec", _fake_exec) - assert await session._validate_path_access("link.txt") == Path("/workspace/link.txt") + assert (await session._validate_path_access("link.txt")).as_posix() == "/workspace/link.txt" first_run_commands = list(commands) commands.clear() state.sandbox_id = None - assert await session._validate_path_access("link.txt") == Path("/workspace/link.txt") + assert (await session._validate_path_access("link.txt")).as_posix() == "/workspace/link.txt" second_run_commands = list(commands) commands.clear() - assert await session._validate_path_access("link.txt") == Path("/workspace/link.txt") + assert (await session._validate_path_access("link.txt")).as_posix() == "/workspace/link.txt" helper_path = str(RESOLVE_WORKSPACE_PATH_HELPER.install_path) assert any( diff --git a/tests/extensions/test_sandbox_vercel.py b/tests/extensions/test_sandbox_vercel.py index e0c8b7dcd1..bdc3bf4739 100644 --- a/tests/extensions/test_sandbox_vercel.py +++ b/tests/extensions/test_sandbox_vercel.py @@ -239,7 +239,10 @@ async def run_command( if not path.startswith(cwd.rstrip("/") + "/"): continue rel_path = path[len(cwd.rstrip("/")) + 1 :] - if rel_path in exclusions: + if any( + rel_path == exclusion or rel_path.startswith(f"{exclusion}/") + for exclusion in exclusions + ): continue info = tarfile.TarInfo(name=rel_path if include_root else path) info.size = len(content) @@ -337,10 +340,10 @@ async def teardown_for_snapshot( path: Path, ) -> None: _ = strategy - mount._events.append(("unmount", str(path))) + mount._events.append(("unmount", path.as_posix())) sandbox = cast(Any, session)._sandbox if sandbox is not None: - sandbox.files.pop(f"{path}/mounted.txt", None) + sandbox.files.pop(f"{path.as_posix()}/mounted.txt", None) async def restore_after_snapshot( self, @@ -349,10 +352,10 @@ async def restore_after_snapshot( path: Path, ) -> None: _ = strategy - mount._events.append(("mount", str(path))) + mount._events.append(("mount", path.as_posix())) sandbox = cast(Any, session)._sandbox if sandbox is not None: - sandbox.files[f"{path}/mounted.txt"] = b"mounted-content" + sandbox.files[f"{path.as_posix()}/mounted.txt"] = b"mounted-content" return _Adapter(self) diff --git a/tests/sandbox/capabilities/test_skills_capability.py b/tests/sandbox/capabilities/test_skills_capability.py index 163ae24a16..c87407543a 100644 --- a/tests/sandbox/capabilities/test_skills_capability.py +++ b/tests/sandbox/capabilities/test_skills_capability.py @@ -15,13 +15,14 @@ from agents.sandbox.session.base_sandbox_session import BaseSandboxSession from agents.sandbox.snapshot import NoopSnapshot from agents.sandbox.types import ExecResult, Permissions, User +from agents.sandbox.workspace_paths import coerce_posix_path from agents.tool import FunctionTool from agents.tool_context import ToolContext from tests.utils.factories import TestSessionState def _children_keys(entry: Dir) -> set[str]: - return {str(key if isinstance(key, Path) else Path(key)) for key in entry.children} + return {coerce_posix_path(key).as_posix() for key in entry.children} def _user_name(user: object) -> str | None: @@ -181,6 +182,19 @@ def test_rejects_absolute_skills_path(self) -> None: skills_path="/skills", ) + def test_rejects_windows_drive_absolute_skills_path(self) -> None: + with pytest.raises(SkillsConfigError) as exc_info: + Skills( + skills=[Skill(name="my-skill", description="desc", content="literal")], + skills_path="C:\\skills", + ) + + assert exc_info.value.context == { + "field": "skills_path", + "path": "C:/skills", + "reason": "absolute", + } + def test_rejects_escape_root_skills_path(self) -> None: with pytest.raises(SkillsConfigError): Skills( diff --git a/tests/sandbox/test_docker.py b/tests/sandbox/test_docker.py index 86f53b7124..d57ce2e00a 100644 --- a/tests/sandbox/test_docker.py +++ b/tests/sandbox/test_docker.py @@ -336,7 +336,7 @@ async def _exec_internal( pass else: return ExecResult( - stdout=str(self._container_path(candidate)).encode("utf-8"), + stdout=self._container_path(candidate).as_posix().encode("utf-8"), stderr=b"", exit_code=0, ) @@ -374,12 +374,12 @@ async def _exec_internal( stdout=b"", stderr=( f"read-only extra path grant: {best_original}\n" - f"resolved path: {self._container_path(candidate)}\n" + f"resolved path: {self._container_path(candidate).as_posix()}\n" ).encode(), exit_code=114, ) return ExecResult( - stdout=str(self._container_path(candidate)).encode("utf-8"), + stdout=self._container_path(candidate).as_posix().encode("utf-8"), stderr=b"", exit_code=0, ) @@ -436,7 +436,7 @@ async def ls( kind = EntryKind.FILE entries.append( FileEntry( - path=str(container_path / child.name), + path=(container_path / child.name).as_posix(), permissions=Permissions.from_mode(child.stat().st_mode), owner="root", group="root", @@ -525,7 +525,7 @@ async def activate( mount_path = mount._resolve_mount_path(session, dest) host_path = cast(_HostBackedDockerSession, session)._host_path(mount_path) host_path.mkdir(parents=True, exist_ok=True) - mount._events.append(("mount", str(mount_path))) + mount._events.append(("mount", mount_path.as_posix())) if mount.remount_marker is not None: (host_path / mount.remount_marker).write_text("remounted", encoding="utf-8") return [] @@ -549,7 +549,7 @@ async def teardown_for_snapshot( ) -> None: _ = strategy host_path = cast(_HostBackedDockerSession, session)._host_path(path) - mount._events.append(("unmount", str(path))) + mount._events.append(("unmount", path.as_posix())) if not mount.remove_on_unmount: return shutil.rmtree(host_path, ignore_errors=True) @@ -563,7 +563,7 @@ async def restore_after_snapshot( _ = strategy host_path = cast(_HostBackedDockerSession, session)._host_path(path) host_path.mkdir(parents=True, exist_ok=True) - mount._events.append(("mount", str(path))) + mount._events.append(("mount", path.as_posix())) if mount.remount_marker is not None: (host_path / mount.remount_marker).write_text("remounted", encoding="utf-8") diff --git a/tests/sandbox/test_entries.py b/tests/sandbox/test_entries.py index ecba9d5b2c..a8f783c5a8 100644 --- a/tests/sandbox/test_entries.py +++ b/tests/sandbox/test_entries.py @@ -3,14 +3,14 @@ import io import os from collections.abc import Awaitable, Callable, Sequence -from pathlib import Path +from pathlib import Path, PureWindowsPath import pytest import agents.sandbox.entries.artifacts as artifacts_module from agents.sandbox import SandboxConcurrencyLimits -from agents.sandbox.entries import Dir, File, GitRepo, LocalDir, LocalFile -from agents.sandbox.errors import ExecNonZeroError, LocalDirReadError +from agents.sandbox.entries import Dir, File, GitRepo, LocalDir, LocalFile, resolve_workspace_path +from agents.sandbox.errors import ExecNonZeroError, InvalidManifestPathError, LocalDirReadError from agents.sandbox.manifest import Manifest from agents.sandbox.materialization import MaterializedFile from agents.sandbox.session.base_sandbox_session import BaseSandboxSession @@ -98,6 +98,56 @@ async def _exec_internal( return ExecResult(stdout=b"", stderr=b"", exit_code=0) +def test_resolve_workspace_path_rejects_windows_drive_absolute_path() -> None: + with pytest.raises(InvalidManifestPathError) as exc_info: + resolve_workspace_path( + Path("/workspace"), + PureWindowsPath("C:/tmp/secret.txt"), + allow_absolute_within_root=True, + ) + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/secret.txt" + assert exc_info.value.context == {"rel": "C:/tmp/secret.txt", "reason": "absolute"} + + +def test_resolve_workspace_path_rejects_absolute_escape_after_normalization() -> None: + with pytest.raises(InvalidManifestPathError) as exc_info: + resolve_workspace_path( + Path("/workspace"), + "/workspace/../etc/passwd", + allow_absolute_within_root=True, + ) + + assert str(exc_info.value) == "manifest path must be relative: /etc/passwd" + assert exc_info.value.context == {"rel": "/etc/passwd", "reason": "absolute"} + + +def test_resolve_workspace_path_rejects_absolute_symlink_escape_for_host_root( + tmp_path: Path, +) -> None: + root = tmp_path / "workspace" + outside = tmp_path / "outside" + root.mkdir() + outside.mkdir() + link = root / "link" + try: + os.symlink(outside, link, target_is_directory=True) + except (NotImplementedError, OSError) as exc: + pytest.skip(f"symlink unavailable: {exc}") + + escaped = link / "secret.txt" + + with pytest.raises(InvalidManifestPathError) as exc_info: + resolve_workspace_path( + root, + escaped, + allow_absolute_within_root=True, + ) + + assert str(exc_info.value) == f"manifest path must be relative: {escaped.as_posix()}" + assert exc_info.value.context == {"rel": escaped.as_posix(), "reason": "absolute"} + + @pytest.mark.asyncio async def test_base_sandbox_session_uses_current_working_directory_for_local_file_sources( monkeypatch: pytest.MonkeyPatch, @@ -169,7 +219,7 @@ def swap_then_open( dir_fd: int | None = None, ) -> int: nonlocal swapped - if path == "safe.txt" and not swapped: + if (path == "safe.txt" or Path(path) == src_file) and not swapped: src_file.unlink() src_file.symlink_to(secret) swapped = True @@ -269,7 +319,51 @@ def swap_root_then_open( dir_fd: int | None = None, ) -> int: nonlocal swapped - if path == "src" and dir_fd is not None and not swapped: + if (path == "src" or Path(path) in {src_root, src_root / "safe.txt"}) and not swapped: + src_root.rename(tmp_path / "src-original") + (tmp_path / "src").symlink_to(secret_dir, target_is_directory=True) + swapped = True + if dir_fd is None: + return original_open(path, flags, mode) + return original_open(path, flags, mode, dir_fd=dir_fd) + + monkeypatch.setattr("agents.sandbox.entries.artifacts.os.open", swap_root_then_open) + + with pytest.raises(LocalDirReadError) as excinfo: + await local_dir.apply(session, Path("/workspace/copied"), tmp_path) + + assert excinfo.value.context["reason"] == "symlink_not_supported" + assert excinfo.value.context["child"] == "src" + assert session.writes == {} + + +@pytest.mark.asyncio +async def test_local_dir_apply_fallback_rejects_source_root_swapped_to_symlink_after_validation( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + src_root = tmp_path / "src" + src_root.mkdir() + (src_root / "safe.txt").write_text("safe", encoding="utf-8") + secret_dir = tmp_path / "secret-dir" + secret_dir.mkdir() + session = _RecordingSession() + local_dir = LocalDir(src=Path("src")) + original_open = os.open + swapped = False + + monkeypatch.setattr("agents.sandbox.entries.artifacts._OPEN_SUPPORTS_DIR_FD", False) + monkeypatch.setattr("agents.sandbox.entries.artifacts._HAS_O_DIRECTORY", False) + + def swap_root_then_open( + path: str | Path, + flags: int, + mode: int = 0o777, + *, + dir_fd: int | None = None, + ) -> int: + nonlocal swapped + if Path(path) == src_root / "safe.txt" and not swapped: src_root.rename(tmp_path / "src-original") (tmp_path / "src").symlink_to(secret_dir, target_is_directory=True) swapped = True diff --git a/tests/sandbox/test_manifest.py b/tests/sandbox/test_manifest.py index f0bfc9574f..8e5e004d52 100644 --- a/tests/sandbox/test_manifest.py +++ b/tests/sandbox/test_manifest.py @@ -43,6 +43,16 @@ def test_manifest_rejects_nested_absolute_child_paths() -> None: manifest.validated_entries() +def test_manifest_rejects_windows_drive_absolute_entry_paths() -> None: + manifest = Manifest(entries={"C:\\tmp\\outside.txt": File(content=b"nope")}) + + with pytest.raises(InvalidManifestPathError) as exc_info: + manifest.validated_entries() + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/outside.txt" + assert exc_info.value.context == {"rel": "C:/tmp/outside.txt", "reason": "absolute"} + + def test_manifest_ephemeral_entry_paths_include_nested_children() -> None: manifest = Manifest( entries={ @@ -143,6 +153,25 @@ def test_manifest_ephemeral_mount_targets_reject_escaping_mount_paths() -> None: manifest.ephemeral_persistence_paths() +def test_manifest_ephemeral_mount_targets_reject_windows_drive_mount_path() -> None: + manifest = Manifest( + root="/workspace", + entries={ + "logical": GCSMount( + bucket="bucket", + mount_path=Path("C:\\tmp\\mount"), + mount_strategy=InContainerMountStrategy(pattern=MountpointMountPattern()), + ), + }, + ) + + with pytest.raises(InvalidManifestPathError) as exc_info: + manifest.ephemeral_mount_targets() + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/mount" + assert exc_info.value.context == {"rel": "C:/tmp/mount", "reason": "absolute"} + + def test_manifest_describe_preserves_tree_rendering_after_renderer_extract() -> None: manifest = Manifest( root="/workspace", diff --git a/tests/sandbox/test_mounts.py b/tests/sandbox/test_mounts.py index a8b01dd26a..255ccff788 100644 --- a/tests/sandbox/test_mounts.py +++ b/tests/sandbox/test_mounts.py @@ -1125,6 +1125,14 @@ async def test_blobfuse_cache_path_must_be_relative_to_workspace() -> None: ) assert escape_exc_info.value.context == {"cache_path": "../blobfuse-cache"} + with pytest.raises(MountConfigError) as windows_exc_info: + FuseMountPattern(cache_path=Path("C:\\blobfuse-cache")) + + assert windows_exc_info.value.message == ( + "blobfuse cache_path must be relative to the workspace root" + ) + assert windows_exc_info.value.context == {"cache_path": "C:/blobfuse-cache"} + @pytest.mark.asyncio async def test_blobfuse_cache_path_must_be_outside_mount_path() -> None: diff --git a/tests/sandbox/test_runtime_helpers.py b/tests/sandbox/test_runtime_helpers.py index 63912ba790..dc95804877 100644 --- a/tests/sandbox/test_runtime_helpers.py +++ b/tests/sandbox/test_runtime_helpers.py @@ -1,9 +1,20 @@ from __future__ import annotations import subprocess -from pathlib import Path +import sys +from pathlib import Path, PurePosixPath -from agents.sandbox.session.runtime_helpers import RESOLVE_WORKSPACE_PATH_HELPER +import pytest + +from agents.sandbox.session.runtime_helpers import ( + RESOLVE_WORKSPACE_PATH_HELPER, + RuntimeHelperScript, +) + +requires_posix_shell = pytest.mark.skipif( + sys.platform == "win32", + reason="runtime helper shell script tests require a POSIX shell", +) def _install_resolve_helper(tmp_path: Path) -> Path: @@ -13,6 +24,18 @@ def _install_resolve_helper(tmp_path: Path) -> Path: return helper_path +def test_runtime_helper_from_content_uses_posix_install_path() -> None: + helper = RuntimeHelperScript.from_content( + name="test-helper", + content="#!/bin/sh\nprintf 'ok\\n'", + ) + + assert isinstance(helper.install_path, PurePosixPath) + assert helper.install_path.as_posix().startswith("/tmp/openai-agents/bin/test-helper-") + assert str(helper.install_path).startswith("/tmp/openai-agents/bin/test-helper-") + + +@requires_posix_shell def test_resolve_workspace_path_helper_allows_extra_root_symlink_target(tmp_path: Path) -> None: helper_path = _install_resolve_helper(tmp_path) workspace = tmp_path / "workspace" @@ -42,6 +65,7 @@ def test_resolve_workspace_path_helper_allows_extra_root_symlink_target(tmp_path assert result.stderr == "" +@requires_posix_shell def test_resolve_workspace_path_helper_rejects_extra_root_when_not_allowed( tmp_path: Path, ) -> None: @@ -71,6 +95,7 @@ def test_resolve_workspace_path_helper_rejects_extra_root_when_not_allowed( assert result.stderr == f"workspace escape: {target.resolve(strict=False)}\n" +@requires_posix_shell def test_resolve_workspace_path_helper_rejects_extra_root_symlink_to_root( tmp_path: Path, ) -> None: @@ -101,6 +126,7 @@ def test_resolve_workspace_path_helper_rejects_extra_root_symlink_to_root( ) +@requires_posix_shell def test_resolve_workspace_path_helper_rejects_nested_read_only_extra_grant_on_write( tmp_path: Path, ) -> None: @@ -138,6 +164,7 @@ def test_resolve_workspace_path_helper_rejects_nested_read_only_extra_grant_on_w ) +@requires_posix_shell def test_resolve_workspace_path_helper_allows_nested_read_only_extra_grant_on_read( tmp_path: Path, ) -> None: diff --git a/tests/sandbox/test_workspace_paths.py b/tests/sandbox/test_workspace_paths.py index 279995a211..2007072844 100644 --- a/tests/sandbox/test_workspace_paths.py +++ b/tests/sandbox/test_workspace_paths.py @@ -3,7 +3,7 @@ import os from collections.abc import Callable from dataclasses import dataclass -from pathlib import Path +from pathlib import Path, PurePath, PurePosixPath, PureWindowsPath from typing import Any, cast import pytest @@ -11,9 +11,13 @@ from agents.sandbox import Manifest, SandboxPathGrant from agents.sandbox.errors import InvalidManifestPathError, WorkspaceArchiveWriteError -from agents.sandbox.workspace_paths import WorkspacePathPolicy +from agents.sandbox.workspace_paths import ( + WorkspacePathPolicy, + coerce_posix_path, + posix_path_as_path, +) -PathInput = str | Path +PathInput = str | PurePath PathPolicyMethod = Callable[[WorkspacePathPolicy, PathInput], Path] @@ -215,8 +219,8 @@ def test_normalize_path_with_symlink_resolution(tmp_path: Path) -> None: WorkspacePathCase( name="absolute path outside root is rejected", path=outside / "secret.txt", - error_message=f"manifest path must be relative: {outside / 'secret.txt'}", - error_context={"rel": str(outside / "secret.txt"), "reason": "absolute"}, + error_message=f"manifest path must be relative: {(outside / 'secret.txt').as_posix()}", + error_context={"rel": (outside / "secret.txt").as_posix(), "reason": "absolute"}, ), ] @@ -228,6 +232,189 @@ def test_normalize_path_with_symlink_resolution(tmp_path: Path) -> None: ) +def test_normalize_sandbox_path_uses_posix_paths_for_windows_inputs() -> None: + policy = WorkspacePathPolicy(root="/workspace") + + assert policy.sandbox_root() == PurePosixPath("/workspace") + assert policy.normalize_sandbox_path(PureWindowsPath("/workspace/pkg/file.py")) == ( + PurePosixPath("/workspace/pkg/file.py") + ) + assert policy.normalize_sandbox_path(PureWindowsPath("pkg/file.py")) == ( + PurePosixPath("/workspace/pkg/file.py") + ) + + +def test_normalize_path_uses_posix_paths_for_windows_inputs() -> None: + policy = WorkspacePathPolicy(root="/workspace") + + assert policy.normalize_path(PureWindowsPath("/workspace/pkg/file.py")).as_posix() == ( + "/workspace/pkg/file.py" + ) + assert policy.absolute_workspace_path(PureWindowsPath("pkg/file.py")).as_posix() == ( + "/workspace/pkg/file.py" + ) + + +def test_inaccessible_root_is_treated_as_remote_path(monkeypatch: pytest.MonkeyPatch) -> None: + root = PurePosixPath("/root/project") + + def raise_for_root(path: Path) -> bool: + if path.as_posix() == root.as_posix(): + raise PermissionError("permission denied") + return False + + monkeypatch.setattr(Path, "exists", raise_for_root) + + policy = WorkspacePathPolicy(root=root) + + assert policy.root_is_existing_host_path() is False + assert policy.normalize_path("pkg/file.py").as_posix() == "/root/project/pkg/file.py" + + +def test_absolute_workspace_path_rejects_windows_rooted_escape_as_absolute() -> None: + policy = WorkspacePathPolicy(root="/workspace") + + with pytest.raises(InvalidManifestPathError) as exc_info: + policy.absolute_workspace_path(PureWindowsPath("/tmp/secret.txt")) + + assert str(exc_info.value) == "manifest path must be relative: /tmp/secret.txt" + assert exc_info.value.context == {"rel": "/tmp/secret.txt", "reason": "absolute"} + + +def test_windows_drive_absolute_path_is_rejected_before_posix_coercion() -> None: + policy = WorkspacePathPolicy(root="/workspace") + + with pytest.raises(InvalidManifestPathError) as exc_info: + policy.normalize_path(PureWindowsPath("C:/tmp/secret.txt")) + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/secret.txt" + assert exc_info.value.context == {"rel": "C:/tmp/secret.txt", "reason": "absolute"} + + with pytest.raises(InvalidManifestPathError) as exc_info: + policy.absolute_workspace_path("C:\\tmp\\secret.txt") + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/secret.txt" + assert exc_info.value.context == {"rel": "C:/tmp/secret.txt", "reason": "absolute"} + + with pytest.raises(InvalidManifestPathError) as exc_info: + policy.normalize_path(coerce_posix_path(PureWindowsPath("C:/tmp/secret.txt"))) + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/secret.txt" + assert exc_info.value.context == {"rel": "C:/tmp/secret.txt", "reason": "absolute"} + + +def test_existing_host_root_rejects_windows_drive_absolute_paths(tmp_path: Path) -> None: + workspace = tmp_path / "workspace" + workspace.mkdir() + policy = WorkspacePathPolicy(root=workspace) + methods: tuple[PathPolicyMethod, ...] = ( + lambda policy, path: policy.absolute_workspace_path(path), + lambda policy, path: policy.normalize_path(path), + lambda policy, path: policy.normalize_path(path, resolve_symlinks=True), + ) + + for method in methods: + for path in ( + PureWindowsPath("C:/tmp/secret.txt"), + "C:\\tmp\\secret.txt", + coerce_posix_path(PureWindowsPath("C:/tmp/secret.txt")), + ): + with pytest.raises(InvalidManifestPathError) as exc_info: + method(policy, path) + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/secret.txt" + assert exc_info.value.context == {"rel": "C:/tmp/secret.txt", "reason": "absolute"} + + +def test_relative_path_rejects_windows_drive_absolute_path_for_host_root( + tmp_path: Path, +) -> None: + workspace = tmp_path / "workspace" + workspace.mkdir() + policy = WorkspacePathPolicy(root=workspace) + + for path in ( + PureWindowsPath("C:/tmp/secret.txt"), + "C:\\tmp\\secret.txt", + coerce_posix_path(PureWindowsPath("C:/tmp/secret.txt")), + ): + with pytest.raises(InvalidManifestPathError) as exc_info: + policy.relative_path(path) + + assert str(exc_info.value) == "manifest path must be relative: C:/tmp/secret.txt" + assert exc_info.value.context == {"rel": "C:/tmp/secret.txt", "reason": "absolute"} + + +def test_posix_path_as_path_returns_native_path() -> None: + path = posix_path_as_path(PurePosixPath("/workspace/file.txt")) + + assert isinstance(path, Path) + assert path.as_posix() == "/workspace/file.txt" + + +def test_sandbox_extra_path_grant_rules_use_posix_paths() -> None: + policy = WorkspacePathPolicy( + root="/workspace", + extra_path_grants=(SandboxPathGrant(path="/tmp"),), + ) + + assert policy.extra_path_grant_rules() == ((PurePosixPath("/tmp"), False),) + assert policy.normalize_sandbox_path(PureWindowsPath("/tmp/result.txt")) == ( + PurePosixPath("/tmp/result.txt") + ) + + +def test_extra_path_grant_rejects_non_native_windows_drive_absolute_path() -> None: + if Path(PureWindowsPath("C:/tmp")).is_absolute(): + pytest.skip("Windows drive paths are native absolute paths on this host") + + for path in ( + PureWindowsPath("C:/tmp"), + "C:\\tmp", + coerce_posix_path(PureWindowsPath("C:/tmp")), + ): + with pytest.raises(ValidationError) as exc_info: + SandboxPathGrant(path=cast(Any, path)) + + errors = exc_info.value.errors(include_url=False) + assert len(errors) == 1 + error = dict(errors[0]) + ctx = cast(dict[str, Any], error["ctx"]) + error["ctx"] = {"error": str(ctx["error"])} + assert error == { + "type": "value_error", + "loc": ("path",), + "msg": "Value error, sandbox path grant path must be POSIX absolute", + "input": path, + "ctx": {"error": "sandbox path grant path must be POSIX absolute"}, + } + + +def test_extra_path_grant_accepts_native_windows_drive_absolute_path( + tmp_path: Path, +) -> None: + if not Path(PureWindowsPath("C:/tmp")).is_absolute(): + pytest.skip("Windows drive paths are not native absolute paths on this host") + + grant = SandboxPathGrant(path=str(tmp_path)) + + assert Path(grant.path).is_absolute() + + +def test_extra_path_grant_rules_reject_windows_drive_absolute_path() -> None: + grant = SandboxPathGrant.model_construct( + path="C:/tmp", + read_only=False, + description=None, + ) + policy = WorkspacePathPolicy(root="/workspace", extra_path_grants=(grant,)) + + with pytest.raises(ValueError) as exc_info: + policy.extra_path_grant_rules() + + assert str(exc_info.value) == "sandbox path grant path must be POSIX absolute" + + def test_manifest_serializes_extra_path_grants() -> None: manifest = Manifest( extra_path_grants=( @@ -315,9 +502,10 @@ def test_host_io_rejects_write_under_resolved_read_only_extra_path_grant( allowed.mkdir() os.symlink(allowed, grant_alias, target_is_directory=True) target = allowed / "cache.db" + grant = SandboxPathGrant(path=str(grant_alias), read_only=True) policy = WorkspacePathPolicy( root=workspace, - extra_path_grants=(SandboxPathGrant(path=str(grant_alias), read_only=True),), + extra_path_grants=(grant,), ) with pytest.raises(WorkspaceArchiveWriteError) as exc_info: @@ -327,7 +515,7 @@ def test_host_io_rejects_write_under_resolved_read_only_extra_path_grant( assert exc_info.value.context == { "path": str(target), "reason": "read_only_extra_path_grant", - "grant_path": str(grant_alias), + "grant_path": grant.path, } @@ -396,6 +584,6 @@ def test_host_io_rejects_extra_path_grant_symlink_to_root(tmp_path: Path) -> Non ) with pytest.raises(ValueError) as exc_info: - policy.normalize_path(Path("/etc/passwd"), resolve_symlinks=True) + policy.normalize_path(root_alias / "etc" / "passwd", resolve_symlinks=True) assert str(exc_info.value) == "sandbox path grant path must not resolve to filesystem root" diff --git a/tests/test_session.py b/tests/test_session.py index 8ede928812..aa8211500a 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -1,6 +1,7 @@ """Tests for session memory functionality.""" import asyncio +import sqlite3 import tempfile from pathlib import Path @@ -214,6 +215,25 @@ async def test_sqlite_session_memory_direct(): session.close() +@pytest.mark.asyncio +async def test_sqlite_session_close_closes_worker_thread_connections(): + """Test that close cleans up connections opened by async worker threads.""" + with tempfile.TemporaryDirectory() as temp_dir: + db_path = Path(temp_dir) / "test_worker_thread_close.db" + session = SQLiteSession("worker_thread_close", db_path) + + await session.add_items([{"role": "user", "content": "Hello"}]) + connections = list(session._connections) + + assert connections + + session.close() + + assert session._connections == set() + with pytest.raises(sqlite3.ProgrammingError): + connections[0].execute("SELECT 1") + + @pytest.mark.asyncio async def test_sqlite_session_memory_pop_item(): """Test SQLiteSession pop_item functionality.""" @@ -415,31 +435,34 @@ async def test_session_callback_prepared_input(runner_method): {"role": "user", "content": "Hello there."}, {"role": "assistant", "content": "Hi, I'm here to assist you."}, ] - await session.add_items(initial_history) - - def filter_assistant_messages(history, new_input): - # Only include user messages from history - return [item for item in history if item["role"] == "user"] + new_input - - new_turn_input = [{"role": "user", "content": "What your name?"}] - model.set_next_output([get_text_message("I'm gpt-4o")]) - - # Run the agent with the callable - await run_agent_async( - runner_method, - agent, - new_turn_input, - session=session, - run_config=RunConfig(session_input_callback=filter_assistant_messages), - ) - - expected_model_input = [ - initial_history[0], # From history - new_turn_input[0], # New input - ] - - assert len(model.last_turn_args["input"]) == 2 - assert model.last_turn_args["input"] == expected_model_input + try: + await session.add_items(initial_history) + + def filter_assistant_messages(history, new_input): + # Only include user messages from history + return [item for item in history if item["role"] == "user"] + new_input + + new_turn_input = [{"role": "user", "content": "What your name?"}] + model.set_next_output([get_text_message("I'm gpt-4o")]) + + # Run the agent with the callable + await run_agent_async( + runner_method, + agent, + new_turn_input, + session=session, + run_config=RunConfig(session_input_callback=filter_assistant_messages), + ) + + expected_model_input = [ + initial_history[0], # From history + new_turn_input[0], # New input + ] + + assert len(model.last_turn_args["input"]) == 2 + assert model.last_turn_args["input"] == expected_model_input + finally: + session.close() @pytest.mark.asyncio