diff --git a/src/agents/extensions/sandbox/modal/sandbox.py b/src/agents/extensions/sandbox/modal/sandbox.py index 804a745aee..da2bd2939c 100644 --- a/src/agents/extensions/sandbox/modal/sandbox.py +++ b/src/agents/extensions/sandbox/modal/sandbox.py @@ -43,7 +43,6 @@ SandboxError, WorkspaceArchiveReadError, WorkspaceArchiveWriteError, - WorkspaceReadNotFoundError, WorkspaceStartError, WorkspaceStopError, WorkspaceWriteTypeError, @@ -1175,8 +1174,11 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase raise WorkspaceArchiveReadError(path=workspace_path, cause=e) from e if not out.ok(): - raise WorkspaceReadNotFoundError( - path=path, context={"stderr": out.stderr.decode("utf-8", "replace")} + await self._raise_read_error_from_exec( + path=posix_path_as_path(coerce_posix_path(path)), + workspace_path=workspace_path, + command=cmd, + result=out, ) return io.BytesIO(out.stdout) diff --git a/src/agents/sandbox/capabilities/skills.py b/src/agents/sandbox/capabilities/skills.py index 5d58c6703f..01b5b203ad 100644 --- a/src/agents/sandbox/capabilities/skills.py +++ b/src/agents/sandbox/capabilities/skills.py @@ -12,9 +12,10 @@ from ...tool import FunctionTool, Tool from ..entries import BaseEntry, Dir, File, LocalDir, LocalFile -from ..errors import LocalDirReadError, SkillsConfigError +from ..errors import LocalDirReadError, SkillsConfigError, WorkspaceReadNotFoundError from ..manifest import Manifest from ..session.base_sandbox_session import BaseSandboxSession +from ..session.sandbox_session import _read_with_expected_span_errors from ..types import User from ..workspace_paths import ( SandboxPathGrant, @@ -236,8 +237,13 @@ async def load_skill( skill_dest = workspace_root / metadata.path skill_md_path = skill_dest / "SKILL.md" try: - handle = await session.read(skill_md_path, user=user) - except Exception: + handle = await _read_with_expected_span_errors( + session, + skill_md_path, + user=user, + expected_span_errors=(FileNotFoundError, WorkspaceReadNotFoundError), + ) + except (FileNotFoundError, WorkspaceReadNotFoundError): handle = None if handle is not None: handle.close() diff --git a/src/agents/sandbox/sandboxes/docker.py b/src/agents/sandbox/sandboxes/docker.py index 04d1fe500b..4aec8bb6f8 100644 --- a/src/agents/sandbox/sandboxes/docker.py +++ b/src/agents/sandbox/sandboxes/docker.py @@ -42,7 +42,6 @@ ExposedPortUnavailableError, WorkspaceArchiveReadError, WorkspaceArchiveWriteError, - WorkspaceReadNotFoundError, ) from ..manifest import Manifest from ..session import SandboxSession, SandboxSessionState @@ -788,13 +787,12 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase 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( + await self._raise_read_error_from_exec( path=path, - context={ - "command": ["cat", "--", workspace_path_arg], - "stdout": res.stdout.decode("utf-8", errors="replace"), - "stderr": res.stderr.decode("utf-8", errors="replace"), - }, + workspace_path=workspace_path, + command=("cat", "--", workspace_path_arg), + result=res, + user=user, ) return io.BytesIO(res.stdout) diff --git a/src/agents/sandbox/session/base_sandbox_session.py b/src/agents/sandbox/session/base_sandbox_session.py index e51f573285..4f172d3951 100644 --- a/src/agents/sandbox/session/base_sandbox_session.py +++ b/src/agents/sandbox/session/base_sandbox_session.py @@ -3,7 +3,7 @@ import shlex from collections.abc import Awaitable, Callable, Mapping, Sequence from pathlib import Path, PurePath -from typing import Literal, TypeVar +from typing import Literal, NoReturn, TypeVar from typing_extensions import Self @@ -23,6 +23,7 @@ InvalidManifestPathError, MountConfigError, PtySessionNotFoundError, + WorkspaceArchiveReadError, WorkspaceArchiveWriteError, WorkspaceReadNotFoundError, ) @@ -46,10 +47,106 @@ RuntimeHelperScript, ) from .sandbox_session_state import SandboxSessionState +from .utils import _safe_decode _PtyEntryT = TypeVar("_PtyEntryT") _RUNTIME_HELPER_CACHE_KEY_UNSET = object() _WORKSPACE_ROOT_PROBE_TIMEOUT_S = 10.0 +_READ_PATH_PROBE_TIMEOUT_S = 10.0 +_READ_PATH_PROBE_SCRIPT = """ +# READ_PATH_PROBE_V3 +LC_ALL=C +export LC_ALL +path=$1 +resolved_path= +symlink_depth=0 + +resolve_probe_path() { + if [ "$symlink_depth" -gt 40 ] || [ "${#1}" -gt 4095 ]; then + return 2 + fi + if [ "$1" = "/" ]; then + resolved_path=/ + return 0 + fi + + parent=${1%/*} + if [ -z "$parent" ] || [ "$parent" = "$1" ]; then + parent=/ + fi + resolve_probe_path "$parent" || return 2 + resolved_parent=$resolved_path + base=${1##*/} + if [ "${#base}" -gt 255 ]; then + return 2 + fi + if [ "$resolved_parent" = "/" ]; then + candidate=/$base + else + candidate=$resolved_parent/$base + fi + + if [ -L "$candidate" ]; then + target_with_marker=$(readlink -n "$candidate" && printf .) || return 2 + target=${target_with_marker%.} + symlink_depth=$((symlink_depth + 1)) + if [ "$symlink_depth" -gt 40 ]; then + return 2 + fi + case "$target" in + /*) + resolve_probe_path "$target" + ;; + *) + resolve_probe_path "$resolved_parent/$target" + ;; + esac + return $? + fi + + resolved_path=$candidate +} + +resolve_probe_path "$path" || exit 2 +path=$resolved_path +candidate=$path +child= + +while :; do + if [ -e "$candidate" ]; then + if [ "$candidate" = "$path" ]; then + exit 0 + fi + if [ ! -d "$candidate" ] || [ ! -x "$candidate" ]; then + exit 2 + fi + lookup_result=$( + find "$child" -prune -print 2>&1 >/dev/null + lookup_status=$? + printf '.%s' "$lookup_status" + ) + lookup_status=${lookup_result##*.} + lookup_error=${lookup_result%.*} + if [ "$lookup_status" -eq 1 ]; then + lookup_error=$(printf %s "$lookup_error") + case "$lookup_error" in + *": No such file or directory") + exit 1 + ;; + esac + fi + exit 2 + fi + if [ "$candidate" = "/" ]; then + exit 2 + fi + child=$candidate + candidate=${candidate%/*} + if [ -z "$candidate" ]; then + candidate=/ + fi +done +""".strip() _WRITE_ACCESS_CHECK_SCRIPT = ( 'target="$1"\n' 'if [ -e "$target" ]; then\n' @@ -787,16 +884,54 @@ async def _check_read_with_exec( cmd = ("sh", "-lc", '[ -r "$1" ]', "sh", path_arg) result = await self.exec(*cmd, shell=False, user=user) if not result.ok(): - raise WorkspaceReadNotFoundError( + await self._raise_read_error_from_exec( path=posix_path_as_path(coerce_posix_path(path)), - context={ - "command": ["sh", "-lc", "", path_arg], - "stdout": result.stdout.decode("utf-8", errors="replace"), - "stderr": result.stderr.decode("utf-8", errors="replace"), - }, + workspace_path=workspace_path, + command=("sh", "-lc", "", path_arg), + result=result, + user=user, ) return workspace_path + async def _raise_read_error_from_exec( + self, + *, + path: Path, + workspace_path: Path, + command: Sequence[str | Path], + result: ExecResult, + user: str | User | None = None, + ) -> NoReturn: + context: dict[str, object] = { + "command": [str(part) for part in command], + "stdout_bytes": len(result.stdout), + "stderr": _safe_decode(result.stderr, max_chars=4096), + } + if result.exit_code != 1: + raise WorkspaceArchiveReadError(path=path, context=context) + + workspace_path_arg = sandbox_path_str(workspace_path) + try: + probe_result = await self.exec( + "sh", + "-c", + _READ_PATH_PROBE_SCRIPT, + "sh", + workspace_path_arg, + timeout=_READ_PATH_PROBE_TIMEOUT_S, + shell=False, + user=user, + ) + except Exception as e: + raise WorkspaceArchiveReadError(path=path, context=context, cause=e) from e + + context["existence_probe_exit_code"] = probe_result.exit_code + context["existence_probe_stdout_bytes"] = len(probe_result.stdout) + context["existence_probe_stderr"] = _safe_decode(probe_result.stderr, max_chars=4096) + if probe_result.exit_code == 1: + raise WorkspaceReadNotFoundError(path=path, context=context) + raise WorkspaceArchiveReadError(path=path, context=context) + async def _check_write_with_exec( self, path: Path | str, *, user: str | User | None = None ) -> Path: diff --git a/src/agents/sandbox/session/sandbox_session.py b/src/agents/sandbox/session/sandbox_session.py index 1d41bc02fb..66f51c2e24 100644 --- a/src/agents/sandbox/session/sandbox_session.py +++ b/src/agents/sandbox/session/sandbox_session.py @@ -378,6 +378,7 @@ async def _annotate( finish_data: Callable[[T], dict[str, object]] | None = None, ok: Callable[[T], bool] | None = None, outputs: Callable[[T], tuple[bytes | None, bytes | None]] | None = None, + expected_span_errors: tuple[type[BaseException], ...] = (), ) -> T: span_cm = ( custom_span( @@ -403,31 +404,64 @@ async def _annotate( value = await run() except Exception as e: duration_ms = (time.monotonic() - t0) * 1000.0 + expected_span_error = isinstance(e, expected_span_errors) + try: + await self._emit_finish_event( + op=op, + span_id=span_id, + parent_span_id=parent_span_id, + trace_id=trace_id, + duration_ms=duration_ms, + ok=False, + exc=e, + data=start_data, + stdout=None, + stderr=None, + ) + except BaseException as sink_error: + self._apply_trace_finish_data( + span=trace_span, + op=op, + ok=False, + data=start_data, + exc=sink_error, + ) + raise self._apply_trace_finish_data( span=trace_span, op=op, - ok=False, + ok=expected_span_error, data=start_data, - exc=e, + exc=None if expected_span_error else e, ) + raise + + data_finish = finish_data(value) if finish_data is not None else start_data + ok_value = ok(value) if ok is not None else True + stdout, stderr = outputs(value) if outputs is not None else (None, None) + duration_ms = (time.monotonic() - t0) * 1000.0 + try: await self._emit_finish_event( op=op, span_id=span_id, parent_span_id=parent_span_id, trace_id=trace_id, duration_ms=duration_ms, + ok=ok_value, + exc=None, + data=data_finish, + stdout=stdout, + stderr=stderr, + ) + except BaseException as sink_error: + self._apply_trace_finish_data( + span=trace_span, + op=op, ok=False, - exc=e, - data=start_data, - stdout=None, - stderr=None, + data=data_finish, + exc=sink_error, ) raise - - data_finish = finish_data(value) if finish_data is not None else start_data - ok_value = ok(value) if ok is not None else True - stdout, stderr = outputs(value) if outputs is not None else (None, None) - duration_ms = (time.monotonic() - t0) * 1000.0 self._apply_trace_finish_data( span=trace_span, op=op, @@ -435,18 +469,6 @@ async def _annotate( data=data_finish, exc=None, ) - await self._emit_finish_event( - op=op, - span_id=span_id, - parent_span_id=parent_span_id, - trace_id=trace_id, - duration_ms=duration_ms, - ok=ok_value, - exc=None, - data=data_finish, - stdout=stdout, - stderr=stderr, - ) return value async def _emit_finish_event( @@ -599,9 +621,22 @@ async def mkdir( ) -> None: await self._inner.mkdir(path, parents=parents, user=user) - @instrumented_op("read", data=_read_start_data) + async def _read( + self, + path: Path, + *, + user: str | User | None = None, + expected_span_errors: tuple[type[BaseException], ...] = (), + ) -> io.IOBase: + return await self._annotate( + op="read", + start_data=_read_start_data(self, path, user=user), + run=lambda: self._inner.read(path, user=user), + expected_span_errors=expected_span_errors, + ) + async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase: - return await self._inner.read(path, user=user) + return await self._read(path, user=user) @instrumented_op("write", data=_write_start_data) async def write( @@ -644,3 +679,19 @@ async def persist_workspace(self) -> io.IOBase: ) async def hydrate_workspace(self, data: io.IOBase) -> None: await self._inner.hydrate_workspace(data) + + +async def _read_with_expected_span_errors( + session: BaseSandboxSession, + path: Path, + *, + user: str | User | None = None, + expected_span_errors: tuple[type[BaseException], ...], +) -> io.IOBase: + if isinstance(session, SandboxSession): + return await session._read( + path, + user=user, + expected_span_errors=expected_span_errors, + ) + return await session.read(path, user=user) diff --git a/tests/extensions/sandbox/test_modal.py b/tests/extensions/sandbox/test_modal.py index bde186d9aa..335139bd54 100644 --- a/tests/extensions/sandbox/test_modal.py +++ b/tests/extensions/sandbox/test_modal.py @@ -31,6 +31,7 @@ InvalidManifestPathError, MountConfigError, WorkspaceArchiveReadError, + WorkspaceReadNotFoundError, ) from agents.sandbox.files import EntryKind from agents.sandbox.manifest import Environment @@ -545,6 +546,57 @@ def test_modal_deserialize_session_state_defaults_missing_idle_timeout( assert restored.idle_timeout is None +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("probe_exit_code", "expected_error"), + [ + (0, WorkspaceArchiveReadError), + (1, WorkspaceReadNotFoundError), + ], +) +async def test_modal_read_classifies_nonzero_cat_with_path_probe( + monkeypatch: pytest.MonkeyPatch, + probe_exit_code: int, + expected_error: type[Exception], +) -> 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) + results = [ + ExecResult(stdout=b"", stderr=b"cat failed", exit_code=1), + ExecResult(stdout=b"", stderr=b"", exit_code=probe_exit_code), + ] + commands: list[tuple[str, ...]] = [] + + async def validate_path(path: Path, *, for_write: bool = False) -> Path: + _ = (path, for_write) + return Path("/workspace/target.txt") + + async def fake_exec( + *command: str | Path, + timeout: float | None = None, + shell: bool | list[str] = True, + user: object | None = None, + ) -> ExecResult: + _ = (timeout, shell, user) + commands.append(tuple(str(part) for part in command)) + return results.pop(0) + + monkeypatch.setattr(session, "_validate_path_access", validate_path) + monkeypatch.setattr(session, "exec", fake_exec) + + with pytest.raises(expected_error): + await session.read(Path("target.txt")) + + assert len(commands) == 2 + assert commands[0][0:2] == ("sh", "-lc") + assert "READ_PATH_PROBE_V3" in commands[1][2] + + @pytest.mark.asyncio async def test_modal_sandbox_create_passes_modal_cloud_bucket_mounts( monkeypatch: pytest.MonkeyPatch, diff --git a/tests/sandbox/capabilities/test_skills_capability.py b/tests/sandbox/capabilities/test_skills_capability.py index 2603077777..0d3ec69085 100644 --- a/tests/sandbox/capabilities/test_skills_capability.py +++ b/tests/sandbox/capabilities/test_skills_capability.py @@ -10,14 +10,21 @@ from agents.sandbox import Manifest, SandboxPathGrant from agents.sandbox.capabilities import LocalDirLazySkillSource, Skill, Skills from agents.sandbox.entries import Dir, File, LocalDir -from agents.sandbox.errors import SkillsConfigError +from agents.sandbox.errors import ( + SkillsConfigError, + WorkspaceArchiveReadError, + WorkspaceReadNotFoundError, +) from agents.sandbox.files import EntryKind, FileEntry from agents.sandbox.session.base_sandbox_session import BaseSandboxSession +from agents.sandbox.session.sandbox_session import SandboxSession 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 agents.tracing import trace +from tests.testing_processor import fetch_ordered_spans from tests.utils.factories import TestSessionState @@ -127,6 +134,23 @@ async def ls( return entries +class _WorkspaceNotFoundSkillsSession(_SkillsSession): + async def read(self, path: Path, *, user: object = None) -> io.BytesIO: + try: + return await super().read(path, user=user) + except FileNotFoundError as exc: + raise WorkspaceReadNotFoundError(path=path, cause=exc) from exc + + +class _ArchiveReadErrorSkillsSession(_SkillsSession): + async def read(self, path: Path, *, user: object = None) -> io.BytesIO: + self.read_users.append(_user_name(user)) + raise WorkspaceArchiveReadError( + path=path, + cause=PermissionError("simulated permission failure"), + ) + + class TestSkillValidation: def test_rejects_directory_content_artifact(self) -> None: with pytest.raises(SkillsConfigError): @@ -610,6 +634,72 @@ async def test_load_skill_materializes_with_bound_run_as_user(self, tmp_path: Pa assert session.mkdir_users assert set(session.mkdir_users) == {"sandbox-user"} + @pytest.mark.asyncio + @pytest.mark.parametrize( + "session_type", + [_SkillsSession, _WorkspaceNotFoundSkillsSession], + ) + async def test_first_load_omits_only_expected_probe_span_error( + self, + tmp_path: Path, + session_type: type[_SkillsSession], + ) -> None: + workspace_root = tmp_path / "workspace" + workspace_root.mkdir() + src_root = tmp_path / "skills" + skill_dir = src_root / "dynamic-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("# dynamic skill\n", encoding="utf-8") + capability = Skills(lazy_from=LocalDirLazySkillSource(source=LocalDir(src=src_root))) + inner = session_type(_source_granted_manifest(workspace_root, source=src_root)) + + async with SandboxSession(inner) as session: + capability.bind(session) + with trace("lazy_skill_expected_probe_test"): + output = await capability.load_skill("dynamic-skill") + + assert output == { + "status": "loaded", + "skill_name": "dynamic-skill", + "path": ".agents/dynamic-skill", + } + read_spans = [ + span + for span in fetch_ordered_spans() + if span.span_data.export().get("name") == "sandbox.read" + ] + assert len(read_spans) == 1 + assert read_spans[0].error is None + + @pytest.mark.asyncio + async def test_load_skill_propagates_non_not_found_read_error(self, tmp_path: Path) -> None: + workspace_root = tmp_path / "workspace" + workspace_root.mkdir() + src_root = tmp_path / "skills" + skill_dir = src_root / "dynamic-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("# dynamic skill\n", encoding="utf-8") + capability = Skills(lazy_from=LocalDirLazySkillSource(source=LocalDir(src=src_root))) + inner = _ArchiveReadErrorSkillsSession( + _source_granted_manifest(workspace_root, source=src_root) + ) + + async with SandboxSession(inner) as session: + capability.bind(session) + with trace("lazy_skill_unexpected_probe_error_test"): + with pytest.raises(WorkspaceArchiveReadError): + await capability.load_skill("dynamic-skill") + + assert inner.write_users == [] + assert inner.mkdir_users == [] + read_spans = [ + span + for span in fetch_ordered_spans() + if span.span_data.export().get("name") == "sandbox.read" + ] + assert len(read_spans) == 1 + assert read_spans[0].error is not None + @pytest.mark.asyncio async def test_load_skill_rejects_missing_lazy_source_directory(self, tmp_path: Path) -> None: workspace_root = tmp_path / "workspace" diff --git a/tests/sandbox/test_docker.py b/tests/sandbox/test_docker.py index a5798774ea..142044ddf2 100644 --- a/tests/sandbox/test_docker.py +++ b/tests/sandbox/test_docker.py @@ -46,7 +46,9 @@ InvalidManifestPathError, MountConfigError, PtySessionNotFoundError, + WorkspaceArchiveReadError, WorkspaceArchiveWriteError, + WorkspaceReadNotFoundError, ) from agents.sandbox.files import EntryKind, FileEntry from agents.sandbox.manifest import Manifest @@ -298,6 +300,7 @@ def __init__( manifest: Manifest, event_log: list[tuple[str, str]] | None = None, archive_error: Exception | None = None, + read_probe_exit_code: int | None = None, ) -> None: container = _FakeDockerContainer(host_root, archive_error=archive_error) state = DockerSandboxSessionState( @@ -314,6 +317,19 @@ def __init__( self._host_root = host_root self._fake_container = container self._event_log = event_log if event_log is not None else [] + self._read_probe_exit_code = read_probe_exit_code + self._read_probe_users: list[str | None] = [] + + async def _exec_internal_for_user( + self, + *command: str | Path, + timeout: float | None = None, + user: str | None = None, + ) -> ExecResult: + cmd = [str(part) for part in command] + if cmd[:2] == ["sh", "-c"] and "READ_PATH_PROBE_V3" in cmd[2]: + self._read_probe_users.append(user) + return await self._exec_internal(*command, timeout=timeout) async def _exec_internal( self, @@ -327,6 +343,15 @@ async def _exec_internal( return ExecResult(stdout=b"", stderr=b"", exit_code=0) if cmd == ["test", "-x", helper_path]: return ExecResult(stdout=b"", stderr=b"", exit_code=0) + if cmd[:2] == ["sh", "-c"] and "READ_PATH_PROBE_V3" in cmd[2]: + if self._read_probe_exit_code is not None: + return ExecResult( + stdout=b"", + stderr=b"", + exit_code=self._read_probe_exit_code, + ) + exists = self._host_path(cmd[4]).exists() + return ExecResult(stdout=b"", stderr=b"", exit_code=0 if exists else 1) if cmd and cmd[0] == helper_path: for_write = cmd[3] candidate = self._host_path(cmd[2]).resolve(strict=False) @@ -1303,6 +1328,62 @@ async def test_docker_read_returns_file_bytes_without_archive_api(tmp_path: Path assert session._fake_container.archive_calls == [] +@pytest.mark.asyncio +async def test_docker_read_missing_path_raises_not_found_error(tmp_path: Path) -> None: + host_root = tmp_path / "container" + (host_root / "workspace").mkdir(parents=True) + session = _HostBackedDockerSession( + host_root=host_root, + manifest=Manifest(root="/workspace"), + ) + + with pytest.raises(WorkspaceReadNotFoundError): + await session.read(Path("missing.txt")) + + +@pytest.mark.asyncio +async def test_docker_read_existing_unreadable_path_raises_archive_error(tmp_path: Path) -> None: + host_root = tmp_path / "container" + unreadable_path = host_root / "workspace" / "directory" + unreadable_path.mkdir(parents=True) + session = _HostBackedDockerSession( + host_root=host_root, + manifest=Manifest(root="/workspace"), + ) + + with pytest.raises(WorkspaceArchiveReadError): + await session.read(Path("directory")) + + +@pytest.mark.asyncio +async def test_docker_read_indeterminate_probe_raises_archive_error(tmp_path: Path) -> None: + host_root = tmp_path / "container" + (host_root / "workspace").mkdir(parents=True) + session = _HostBackedDockerSession( + host_root=host_root, + manifest=Manifest(root="/workspace"), + read_probe_exit_code=2, + ) + + with pytest.raises(WorkspaceArchiveReadError): + await session.read(Path("inaccessible/missing.txt")) + + +@pytest.mark.asyncio +async def test_docker_read_probe_uses_requested_user(tmp_path: Path) -> None: + host_root = tmp_path / "container" + (host_root / "workspace").mkdir(parents=True) + session = _HostBackedDockerSession( + host_root=host_root, + manifest=Manifest(root="/workspace"), + ) + + with pytest.raises(WorkspaceReadNotFoundError): + await session.read(Path("missing.txt"), user="sandbox-user") + + assert session._read_probe_users == ["sandbox-user"] + + @pytest.mark.asyncio async def test_docker_normalize_path_preserves_safe_leaf_symlink_path(tmp_path: Path) -> None: host_root = tmp_path / "container" diff --git a/tests/sandbox/test_session_sinks.py b/tests/sandbox/test_session_sinks.py index 09efbd83ca..6b2f41505d 100644 --- a/tests/sandbox/test_session_sinks.py +++ b/tests/sandbox/test_session_sinks.py @@ -32,6 +32,7 @@ WorkspaceJsonlSink, ) from agents.sandbox.session.base_sandbox_session import BaseSandboxSession +from agents.sandbox.session.sandbox_session import _read_with_expected_span_errors from agents.sandbox.snapshot import LocalSnapshot from agents.tracing import custom_span, trace from tests.testing_processor import fetch_normalized_spans, fetch_ordered_spans @@ -455,6 +456,129 @@ async def test_sandbox_session_error_events_and_traces_include_retryability( assert error_payload["error_retryable"] is False +@pytest.mark.asyncio +async def test_expected_read_span_error_is_call_scoped_and_preserves_audit_failures( + tmp_path: Path, +) -> None: + events: list[SandboxSessionEvent] = [] + instrumentation = Instrumentation( + sinks=[CallbackSink(lambda e, _sess: events.append(e), mode="sync")] + ) + inner = _build_unix_local_session(tmp_path) + expected_path = Path("expected-missing.txt") + ordinary_path = Path("ordinary-missing.txt") + + with trace("sandbox_expected_read_error_test"): + async with SandboxSession(inner, instrumentation=instrumentation) as session: + results = await asyncio.gather( + _read_with_expected_span_errors( + session, + expected_path, + expected_span_errors=(WorkspaceReadNotFoundError,), + ), + session.read(ordinary_path), + return_exceptions=True, + ) + + assert all(isinstance(result, WorkspaceReadNotFoundError) for result in results) + + read_starts = [ + event + for event in events + if isinstance(event, SandboxSessionStartEvent) and event.op == "read" + ] + path_by_span_id = {event.span_id: event.data["path"] for event in read_starts} + read_spans = [ + span + for span in fetch_ordered_spans() + if span.span_data.export().get("name") == "sandbox.read" + ] + error_by_path = {path_by_span_id[span.span_id]: span.error for span in read_spans} + assert error_by_path[str(expected_path)] is None + assert error_by_path[str(ordinary_path)] is not None + + read_finishes = [ + event + for event in events + if isinstance(event, SandboxSessionFinishEvent) and event.op == "read" + ] + assert len(read_finishes) == 2 + for event in read_finishes: + assert event.ok is False + assert event.error_type == "WorkspaceReadNotFoundError" + assert event.error_code == "workspace_read_not_found" + assert event.error_retryable is False + + +@pytest.mark.asyncio +async def test_expected_read_span_records_finish_sink_failure(tmp_path: Path) -> None: + def fail_read_finish(event: SandboxSessionEvent, _session: BaseSandboxSession) -> None: + if isinstance(event, SandboxSessionFinishEvent) and event.op == "read": + raise ValueError("simulated sink failure") + + instrumentation = Instrumentation( + sinks=[CallbackSink(fail_read_finish, mode="sync", on_error="raise")] + ) + inner = _build_unix_local_session(tmp_path) + + with trace("sandbox_expected_read_sink_failure_test"): + async with SandboxSession(inner, instrumentation=instrumentation) as session: + with pytest.raises(RuntimeError, match="sandbox event sink failed"): + await _read_with_expected_span_errors( + session, + Path("expected-missing.txt"), + expected_span_errors=(WorkspaceReadNotFoundError,), + ) + + read_span = next( + span + for span in fetch_ordered_spans() + if span.span_data.export().get("name") == "sandbox.read" + ) + assert read_span.error is not None + assert read_span.error["message"] == "RuntimeError" + assert read_span.span_data.data["error_type"] == "RuntimeError" + + +@pytest.mark.asyncio +async def test_exec_span_records_cancellation_during_finish_sink_delivery(tmp_path: Path) -> None: + finish_delivery_started = asyncio.Event() + completed_exit_codes: list[int] = [] + + async def block_exec_finish(event: SandboxSessionEvent, _session: BaseSandboxSession) -> None: + if isinstance(event, SandboxSessionFinishEvent) and event.op == "exec": + exit_code = event.data["exit_code"] + assert isinstance(exit_code, int) + completed_exit_codes.append(exit_code) + finish_delivery_started.set() + await asyncio.Event().wait() + + instrumentation = Instrumentation( + sinks=[CallbackSink(block_exec_finish, mode="sync", on_error="raise")] + ) + inner = _build_unix_local_session(tmp_path) + + with trace("sandbox_exec_finish_cancellation_test"): + async with SandboxSession(inner, instrumentation=instrumentation) as session: + exec_task = asyncio.create_task(session.exec("exit 7")) + await finish_delivery_started.wait() + exec_task.cancel() + with pytest.raises(asyncio.CancelledError): + await exec_task + + exec_span = next( + span + for span in fetch_ordered_spans() + if span.span_data.export().get("name") == "sandbox.exec" + ) + assert exec_span.error is not None + assert exec_span.error["message"] == "CancelledError" + assert exec_span.span_data.data["error_type"] == "CancelledError" + assert completed_exit_codes + assert exec_span.span_data.data["exit_code"] == completed_exit_codes[0] + assert exec_span.span_data.data["process.exit.code"] == completed_exit_codes[0] + + @pytest.mark.asyncio async def test_sandbox_session_ops_nest_under_sdk_trace_and_events_carry_trace_ids( tmp_path: Path, diff --git a/tests/sandbox/test_session_utils.py b/tests/sandbox/test_session_utils.py index bf18af4f5d..ca2326da54 100644 --- a/tests/sandbox/test_session_utils.py +++ b/tests/sandbox/test_session_utils.py @@ -1,18 +1,29 @@ from __future__ import annotations import io +import os import shlex +import subprocess +import sys import uuid from pathlib import Path import pytest from agents.sandbox.entries import GCSMount, InContainerMountStrategy, MountpointMountPattern -from agents.sandbox.errors import MountConfigError +from agents.sandbox.errors import ( + MountConfigError, + WorkspaceArchiveReadError, + WorkspaceReadNotFoundError, +) from agents.sandbox.files import EntryKind, FileEntry from agents.sandbox.manifest import Manifest from agents.sandbox.session import SandboxSessionStartEvent -from agents.sandbox.session.base_sandbox_session import BaseSandboxSession +from agents.sandbox.session.base_sandbox_session import ( + _READ_PATH_PROBE_SCRIPT, + _READ_PATH_PROBE_TIMEOUT_S, + BaseSandboxSession, +) from agents.sandbox.session.events import SandboxSessionFinishEvent, validate_sandbox_session_event from agents.sandbox.session.utils import ( _best_effort_stream_len, @@ -71,6 +82,23 @@ def __init__(self, manifest: Manifest) -> None: ) +class _QueuedExecSession(_CaptureExecSession): + def __init__(self, results: list[ExecResult]) -> None: + super().__init__() + self._results = list(results) + self.commands: list[tuple[str, ...]] = [] + self.timeouts: list[float | None] = [] + + async def _exec_internal( + self, + *command: str | Path, + timeout: float | None = None, + ) -> ExecResult: + self.commands.append(tuple(str(part) for part in command)) + self.timeouts.append(timeout) + return self._results.pop(0) + + def test_safe_decode_truncates_and_appends_ellipsis() -> None: assert _safe_decode(b"abcdef", max_chars=3) == "abc…" @@ -225,6 +253,142 @@ async def test_check_rm_with_exec_runs_parent_write_probe_as_user() -> None: assert session.last_command[-2:] == ("/workspace/stale.txt", "0") +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("probe_exit_code", "expected_error"), + [ + (0, WorkspaceArchiveReadError), + (1, WorkspaceReadNotFoundError), + (2, WorkspaceArchiveReadError), + ], +) +async def test_check_read_with_exec_classifies_failure_as_requested_user( + probe_exit_code: int, + expected_error: type[Exception], +) -> None: + session = _QueuedExecSession( + [ + ExecResult(stdout=b"", stderr=b"not readable", exit_code=1), + ExecResult(stdout=b"", stderr=b"", exit_code=probe_exit_code), + ] + ) + + with pytest.raises(expected_error): + await session._check_read_with_exec( + Path("target.txt"), + user=User(name="sandbox-user"), + ) + + assert len(session.commands) == 2 + assert all(command[:4] == ("sudo", "-u", "sandbox-user", "--") for command in session.commands) + assert "READ_PATH_PROBE_V3" in session.commands[1][6] + assert session.timeouts == [None, _READ_PATH_PROBE_TIMEOUT_S] + + +@pytest.mark.asyncio +async def test_check_read_with_exec_treats_nonstandard_check_exit_as_archive_error() -> None: + session = _QueuedExecSession([ExecResult(stdout=b"", stderr=b"check failed", exit_code=127)]) + + with pytest.raises(WorkspaceArchiveReadError): + await session._check_read_with_exec(Path("target.txt")) + + assert len(session.commands) == 1 + + +@pytest.mark.asyncio +async def test_read_error_context_does_not_retain_partial_stdout() -> None: + partial_stdout = b"sensitive partial contents" * 1024 + session = _QueuedExecSession( + [ + ExecResult(stdout=partial_stdout, stderr=b"read failed", exit_code=1), + ExecResult(stdout=b"", stderr=b"", exit_code=2), + ] + ) + + with pytest.raises(WorkspaceArchiveReadError) as exc_info: + await session._check_read_with_exec(Path("target.txt")) + + assert "stdout" not in exc_info.value.context + assert exc_info.value.context["stdout_bytes"] == len(partial_stdout) + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX shell behavior is Unix-specific") +def test_read_path_probe_resolves_symlinks_before_classifying_missing(tmp_path: Path) -> None: + workspace = tmp_path / "workspace" + workspace.mkdir() + dangling = workspace / "dangling" + dangling.symlink_to("missing") + non_directory = workspace / "not-a-directory" + non_directory.write_text("content", encoding="utf-8") + invalid_target = workspace / "invalid-target" + invalid_target.symlink_to("not-a-directory/child") + dangling_parent = workspace / "dangling-parent" + dangling_parent.symlink_to("missing-directory") + invalid_parent = workspace / "invalid-parent" + invalid_parent.symlink_to("not-a-directory") + loop = workspace / "loop" + loop.symlink_to("loop") + newline_target = workspace / "newline-target\n" + newline_target.write_text("content", encoding="utf-8") + newline_link = workspace / "newline-link" + newline_link.symlink_to(newline_target.name) + (workspace / "a").write_text("sibling", encoding="utf-8") + current = workspace + symlink_parts: list[str] = [] + for index in range(41): + real = current / f"real-{index}" + real.mkdir() + link_name = f"link-{index}" + (current / link_name).symlink_to(real.name, target_is_directory=True) + symlink_parts.append(link_name) + current = real + + def probe(path: Path, *, env: dict[str, str] | None = None) -> int: + result = subprocess.run( + ["sh", "-c", _READ_PATH_PROBE_SCRIPT, "sh", str(path)], + check=False, + capture_output=True, + env=env, + timeout=5, + ) + return result.returncode + + assert probe(dangling) == 1 + assert probe(invalid_target) == 2 + assert probe(dangling_parent / "child") == 1 + assert probe(invalid_parent / "child") == 2 + assert probe(loop) == 2 + assert probe(newline_link) == 0 + assert probe(workspace / "[a]") == 1 + assert probe(workspace / "?") == 1 + assert probe(workspace / "*") == 1 + assert probe(workspace.joinpath(*symlink_parts, "missing")) == 2 + assert probe(workspace / ("x" * 256)) == 2 + + fake_bin = tmp_path / "fake-bin" + fake_bin.mkdir() + fake_find = fake_bin / "find" + find_args_log = tmp_path / "find-args.log" + fake_find.write_text( + '#!/bin/sh\nprintf "%s\\n" "$@" > "$FIND_ARGS_LOG"\n' + 'printf "find: %s: Input/output error\\n" "$1" >&2\nexit 1\n', + encoding="utf-8", + ) + fake_find.chmod(0o755) + env = dict(os.environ) + env["PATH"] = f"{fake_bin}{os.pathsep}{env['PATH']}" + env["FIND_ARGS_LOG"] = str(find_args_log) + missing_path = workspace / "missing" + assert probe(missing_path, env=env) == 2 + assert find_args_log.read_text(encoding="utf-8").splitlines() == [ + str(missing_path), + "-prune", + "-print", + ] + fake_find.write_text("#!/bin/sh\nprintf match\n", encoding="utf-8") + assert probe(missing_path, env=env) == 2 + + @pytest.mark.parametrize( ("skip_path", "mount_path"), [