diff --git a/src/agents/extensions/sandbox/blaxel/sandbox.py b/src/agents/extensions/sandbox/blaxel/sandbox.py index aee3211fbd..310d20a3a9 100644 --- a/src/agents/extensions/sandbox/blaxel/sandbox.py +++ b/src/agents/extensions/sandbox/blaxel/sandbox.py @@ -1071,6 +1071,10 @@ async def create( ) -> SandboxSession: if manifest is None: manifest = Manifest(root=DEFAULT_BLAXEL_WORKSPACE_ROOT) + manifest._reject_process_environment_values( + backend_id="blaxel", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient", + ) self._validate_manifest_for_create(manifest) timeouts_in = options.timeouts @@ -1154,6 +1158,10 @@ async def resume( if not isinstance(state, BlaxelSandboxSessionState): raise TypeError("BlaxelSandboxClient.resume expects a BlaxelSandboxSessionState") state.assert_path_grants_rebound() + state.manifest._reject_process_environment_values( + backend_id="blaxel", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient instead", + ) SandboxInstance = _import_blaxel_sdk() blaxel_sandbox = None reconnected = False diff --git a/src/agents/extensions/sandbox/cloudflare/sandbox.py b/src/agents/extensions/sandbox/cloudflare/sandbox.py index 8efad06a2e..94665ad860 100644 --- a/src/agents/extensions/sandbox/cloudflare/sandbox.py +++ b/src/agents/extensions/sandbox/cloudflare/sandbox.py @@ -1584,6 +1584,10 @@ async def create( if manifest is None: manifest = Manifest() + manifest._reject_process_environment_values( + backend_id="cloudflare", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient", + ) self._validate_manifest_for_create(manifest) if manifest.root != "/workspace": raise ConfigurationError( @@ -1636,6 +1640,10 @@ async def resume(self, state: SandboxSessionState) -> SandboxSession: "CloudflareSandboxClient.resume expects a CloudflareSandboxSessionState" ) state.assert_path_grants_rebound() + state.manifest._reject_process_environment_values( + backend_id="cloudflare", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient instead", + ) if state.mount_authority_rebound or _manifest_has_configured_mount_authority( state.manifest ): diff --git a/src/agents/extensions/sandbox/daytona/sandbox.py b/src/agents/extensions/sandbox/daytona/sandbox.py index d62c5021ad..437fa227c9 100644 --- a/src/agents/extensions/sandbox/daytona/sandbox.py +++ b/src/agents/extensions/sandbox/daytona/sandbox.py @@ -19,6 +19,7 @@ import time import uuid from collections import deque +from collections.abc import Awaitable, Callable, Iterable, Mapping from dataclasses import dataclass, field from pathlib import Path from typing import Any, Literal, cast @@ -40,9 +41,12 @@ WorkspaceStartError, WorkspaceWriteTypeError, ) -from ....sandbox.manifest import Manifest +from ....sandbox.manifest import Manifest, _process_environment_error from ....sandbox.session import SandboxSession, SandboxSessionState -from ....sandbox.session.base_sandbox_session import BaseSandboxSession +from ....sandbox.session.base_sandbox_session import ( + BaseSandboxSession, + _register_sdk_process_environment_session_type, +) from ....sandbox.session.dependencies import Dependencies from ....sandbox.session.manager import Instrumentation from ....sandbox.session.pty_output import collect_pty_output @@ -58,7 +62,7 @@ from ....sandbox.session.runtime_helpers import RESOLVE_WORKSPACE_PATH_HELPER, RuntimeHelperScript from ....sandbox.session.sandbox_client import BaseSandboxClient, BaseSandboxClientOptions from ....sandbox.session.tar_workspace import shell_tar_exclude_args -from ....sandbox.snapshot import SnapshotBase, SnapshotSpec, resolve_snapshot +from ....sandbox.snapshot import NoopSnapshot, SnapshotBase, SnapshotSpec, resolve_snapshot from ....sandbox.types import ExecResult, ExposedPortEndpoint, User from ....sandbox.util.retry import ( TRANSIENT_HTTP_STATUS_CODES, @@ -393,6 +397,7 @@ class _DaytonaPtySessionEntry: worker_task: asyncio.Task[None] | None = None +@_register_sdk_process_environment_session_type class DaytonaSandboxSession(BaseSandboxSession): """Daytona-backed sandbox session implementation.""" @@ -401,6 +406,11 @@ class DaytonaSandboxSession(BaseSandboxSession): _pty_lock: asyncio.Lock _pty_sessions: dict[int, _DaytonaPtySessionEntry] _reserved_pty_process_ids: set[int] + _process_environment_resume_previous_sandbox_id: str | None + _process_environment_resume_previous_sandbox_loader: Any + _process_environment_resume_start: Callable[[], Awaitable[None]] | None + _process_environment_failed_candidate_sandbox_id: str | None + _process_environment_failed_candidate_sandbox: Any def __init__(self, *, state: DaytonaSandboxSessionState, sandbox: Any) -> None: self.state = state @@ -408,6 +418,13 @@ def __init__(self, *, state: DaytonaSandboxSessionState, sandbox: Any) -> None: self._pty_lock = asyncio.Lock() self._pty_sessions = {} self._reserved_pty_process_ids = set() + self._process_environment_start_lock = asyncio.Lock() + self._process_environment_resume_started = False + self._process_environment_resume_start = None + self._process_environment_resume_previous_sandbox_id = None + self._process_environment_resume_previous_sandbox_loader = None + self._process_environment_failed_candidate_sandbox_id = None + self._process_environment_failed_candidate_sandbox = None @classmethod def from_state( @@ -422,6 +439,74 @@ def from_state( def sandbox_id(self) -> str: return self.state.sandbox_id + @redact_mount_error_data + async def start(self) -> None: + async with self._process_environment_start_lock: + await self._cleanup_process_environment_failed_candidate() + if self._process_environment_resume_started: + if not await self.running(): + self._set_start_state_preserved(True, system=True) + await super().start() + await self._retire_process_environment_previous_sandbox() + return + deferred_start = getattr(self, "_process_environment_resume_start", None) + if deferred_start is None: + await super().start() + return + await deferred_start() + self._process_environment_resume_start = None + self._process_environment_resume_started = True + + async def _cleanup_process_environment_failed_candidate(self) -> None: + sandbox_id = self._process_environment_failed_candidate_sandbox_id + if sandbox_id is None: + return + sandbox = self._process_environment_failed_candidate_sandbox + if sandbox is None: + loader = self._process_environment_resume_previous_sandbox_loader + try: + sandbox = await loader(sandbox_id) + except Exception as error: + not_found_error_types = _daytona_not_found_error_types() + if not not_found_error_types or not isinstance(error, not_found_error_types): + raise + self._process_environment_failed_candidate_sandbox_id = None + return + try: + await sandbox.delete() + except Exception as error: + not_found_error_types = _daytona_not_found_error_types() + if not not_found_error_types or not isinstance(error, not_found_error_types): + raise + self._process_environment_failed_candidate_sandbox_id = None + self._process_environment_failed_candidate_sandbox = None + + async def _retire_process_environment_previous_sandbox( + self, + *, + previous_sandbox: Any = None, + ) -> None: + previous_sandbox_id = self._process_environment_resume_previous_sandbox_id + if previous_sandbox_id is None: + return + if previous_sandbox is None: + loader = self._process_environment_resume_previous_sandbox_loader + try: + previous_sandbox = await loader(previous_sandbox_id) + except Exception as error: + not_found_error_types = _daytona_not_found_error_types() + if not not_found_error_types or not isinstance(error, not_found_error_types): + raise + self._process_environment_resume_previous_sandbox_id = None + return + try: + await previous_sandbox.delete() + except Exception as error: + not_found_error_types = _daytona_not_found_error_types() + if not not_found_error_types or not isinstance(error, not_found_error_types): + raise + self._process_environment_resume_previous_sandbox_id = None + async def _resolve_exposed_port(self, port: int) -> ExposedPortEndpoint: try: preview = await self._sandbox.create_signed_preview_url( @@ -463,13 +548,44 @@ async def _resolve_exposed_port(self, port: int) -> ExposedPortEndpoint: ) from e async def _shutdown_backend(self) -> None: + cleanup_error: BaseException | None = None try: - if self.state.pause_on_exit: - await self._sandbox.stop() - else: - await self._sandbox.delete() - except Exception: - pass + await self._cleanup_process_environment_failed_candidate() + except BaseException as exc: + cleanup_error = exc + deferred_sandbox_id = self._process_environment_resume_previous_sandbox_id + if self._sandbox is None and deferred_sandbox_id is not None: + try: + loader = self._process_environment_resume_previous_sandbox_loader + self._sandbox = await loader(deferred_sandbox_id) + except Exception as exc: + not_found_error_types = _daytona_not_found_error_types() + if not_found_error_types and isinstance(exc, not_found_error_types): + self._process_environment_resume_previous_sandbox_id = None + elif cleanup_error is None: + cleanup_error = exc + if self._sandbox is not None: + try: + if self.state.pause_on_exit: + await self._sandbox.stop() + else: + await self._sandbox.delete() + except Exception as exc: + not_found_error_types = _daytona_not_found_error_types() + if not_found_error_types and isinstance(exc, not_found_error_types): + pass + elif ( + self.state.manifest._has_process_environment_access() and cleanup_error is None + ): + cleanup_error = exc + if deferred_sandbox_id is not None and self.state.sandbox_id != deferred_sandbox_id: + try: + await self._retire_process_environment_previous_sandbox() + except BaseException as exc: + if cleanup_error is None: + cleanup_error = exc + if cleanup_error is not None: + raise cleanup_error from None async def _validate_path_access(self, path: Path | str, *, for_write: bool = False) -> Path: return await self._validate_remote_path_access(path, for_write=for_write) @@ -545,8 +661,20 @@ async def mkdir( ) from e async def _resolved_envs(self) -> dict[str, str]: - manifest_envs = await self.state.manifest.environment.resolve() - return {**self.state.base_env_vars, **manifest_envs} + return await self._resolved_command_envs() + + async def _resolved_command_envs(self) -> dict[str, str]: + manifest_envs = await self.state.manifest._resolve_environment_without_process_values() + process_destinations = { + destination + for destination, _source in self.state.manifest._declared_process_environment_bindings() + } + base_envs = { + key: value + for key, value in self.state.base_env_vars.items() + if key not in process_destinations + } + return {**base_envs, **manifest_envs} def _coerce_exec_timeout(self, timeout_s: float | None) -> float: if timeout_s is None: @@ -561,7 +689,7 @@ async def _exec_internal( timeout: float | None = None, ) -> ExecResult: cmd_str = shlex.join(str(c) for c in command) - envs = await self._resolved_envs() + envs = await self._resolved_command_envs() 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 "" @@ -627,6 +755,7 @@ def _remaining_timeout() -> float: def supports_pty(self) -> bool: return True + @redact_mount_error_data async def pty_exec_start( self, *command: str | Path, @@ -640,7 +769,7 @@ async def pty_exec_start( PtySize = _import_pty_size() 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() + envs = await self._resolved_envs() if tty else await self._resolved_command_envs() cwd = sandbox_path_str(self.state.manifest.root) exec_timeout = self._coerce_exec_timeout(timeout) timeout_error_types = _daytona_timeout_error_types() @@ -1041,6 +1170,7 @@ async def _run_persist_workspace_command(self, tar_cmd: str, tar_path: str) -> b retryable=retryable, ) from e + @redact_mount_error_data async def persist_workspace(self) -> io.IOBase: def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: summary = {"message": error.message} @@ -1120,6 +1250,7 @@ def _error_context_summary(error: WorkspaceArchiveReadError) -> dict[str, str]: assert raw is not None return io.BytesIO(raw) + @redact_mount_error_data async def hydrate_workspace(self, data: io.IOBase) -> None: root = self._workspace_root_path() tar_path = f"/tmp/sandbox-hydrate-{self.state.session_id.hex}.tar" @@ -1145,9 +1276,9 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: cause=e, ) from e + envs = await self._resolved_envs() try: - await self.mkdir(root, parents=True) - envs = await self._resolved_envs() + await self._sandbox.fs.create_folder(sandbox_path_str(root), "755") await self._sandbox.fs.upload_file( bytes(payload), tar_path, @@ -1169,7 +1300,6 @@ async def hydrate_workspace(self, data: io.IOBase) -> None: raise WorkspaceArchiveWriteError(path=root, cause=e) from e finally: try: - envs = await self._resolved_envs() await self._sandbox.process.exec( f"rm -f -- {shlex.quote(tar_path)}", env=envs or None, @@ -1192,6 +1322,8 @@ def __init__( api_url: str | None = None, instrumentation: Instrumentation | None = None, dependencies: Dependencies | None = None, + allowed_process_environment_keys: Iterable[str] = (), + process_environment_bindings: Mapping[str, str] | None = None, ) -> None: AsyncDaytona, DaytonaConfig, _, _ = _import_daytona_sdk() config = DaytonaConfig(api_key=api_key, api_url=api_url) if (api_key or api_url) else None @@ -1200,6 +1332,10 @@ def __init__( instrumentation if instrumentation is not None else Instrumentation() ) self._dependencies = dependencies + self._configure_process_environment_bindings( + allowed_process_environment_keys=allowed_process_environment_keys, + process_environment_bindings=process_environment_bindings, + ) async def _build_create_params( self, @@ -1208,13 +1344,19 @@ async def _build_create_params( image: str | None, env_vars: dict[str, str] | None, manifest: Manifest, + process_envs: dict[str, str] | None = None, name: str | None = None, resources: DaytonaSandboxResources | None = None, auto_stop_interval: int | None = None, ) -> Any: _, _, CreateSandboxFromSnapshotParams, CreateSandboxFromImageParams = _import_daytona_sdk() base_envs = dict(env_vars or {}) - creation_envs = base_envs or None + resolved_process_envs = ( + process_envs + if process_envs is not None + else await manifest._resolve_process_environment_values() + ) + creation_envs = {**base_envs, **resolved_process_envs} or None if sandbox_snapshot_name: return CreateSandboxFromSnapshotParams( @@ -1259,7 +1401,7 @@ async def create( ) -> SandboxSession: if manifest is None: manifest = Manifest(root=DEFAULT_DAYTONA_WORKSPACE_ROOT) - self._validate_manifest_for_create(manifest) + manifest = self._validate_manifest_for_create(manifest) timeouts_in = options.timeouts if isinstance(timeouts_in, DaytonaSandboxTimeouts): @@ -1281,29 +1423,46 @@ async def create( resources=options.resources, auto_stop_interval=options.auto_stop_interval, ) - daytona_sandbox = await self._daytona.create(params, timeout=options.create_timeout) + daytona_sandbox = None + try: + daytona_sandbox = await self._daytona.create(params, timeout=options.create_timeout) - snapshot_instance = resolve_snapshot(snapshot, str(session_id)) - state = DaytonaSandboxSessionState( - session_id=session_id, - manifest=manifest, - snapshot=snapshot_instance, - sandbox_id=daytona_sandbox.id, - sandbox_snapshot_name=options.sandbox_snapshot_name, - image=options.image, - base_env_vars=dict(options.env_vars or {}), - pause_on_exit=options.pause_on_exit, - create_timeout=options.create_timeout, - start_timeout=options.start_timeout, - name=sandbox_name, - resources=options.resources, - auto_stop_interval=options.auto_stop_interval, - timeouts=timeouts, - exposed_ports=options.exposed_ports, - exposed_port_url_ttl_s=options.exposed_port_url_ttl_s, - ) - inner = DaytonaSandboxSession.from_state(state, sandbox=daytona_sandbox) - return self._wrap_session(inner, instrumentation=self._instrumentation) + snapshot_instance = resolve_snapshot(snapshot, str(session_id)) + state = DaytonaSandboxSessionState( + session_id=session_id, + manifest=manifest, + snapshot=snapshot_instance, + sandbox_id=daytona_sandbox.id, + sandbox_snapshot_name=options.sandbox_snapshot_name, + image=options.image, + base_env_vars=dict(options.env_vars or {}), + pause_on_exit=options.pause_on_exit, + create_timeout=options.create_timeout, + start_timeout=options.start_timeout, + name=sandbox_name, + resources=options.resources, + auto_stop_interval=options.auto_stop_interval, + timeouts=timeouts, + exposed_ports=options.exposed_ports, + exposed_port_url_ttl_s=options.exposed_port_url_ttl_s, + ) + inner = DaytonaSandboxSession.from_state(state, sandbox=daytona_sandbox) + return self._wrap_session(inner, instrumentation=self._instrumentation) + except BaseException: + if daytona_sandbox is not None: + try: + await daytona_sandbox.delete() + except Exception as cleanup_error: + not_found_error_types = _daytona_not_found_error_types() + if manifest._has_process_environment_values() and ( + not not_found_error_types + or not isinstance(cleanup_error, not_found_error_types) + ): + raise _process_environment_error( + "Daytona failed to clean up protected create resources; " + f"sandbox_id={daytona_sandbox.id!r}" + ) from None + raise async def close(self) -> None: """Close the underlying AsyncDaytona HTTP client session.""" @@ -1322,7 +1481,12 @@ async def delete(self, session: SandboxSession) -> SandboxSession: try: await inner.shutdown() except Exception: - pass + if ( + inner._process_environment_resume_previous_sandbox_id is not None + or inner._process_environment_failed_candidate_sandbox_id is not None + or inner.state.manifest._has_process_environment_access() + ): + raise return session @redact_mount_error_data @@ -1332,18 +1496,99 @@ async def resume( ) -> SandboxSession: if not isinstance(state, DaytonaSandboxSessionState): raise TypeError("DaytonaSandboxClient.resume expects a DaytonaSandboxSessionState") + state.manifest = self._bind_process_environment_manifest(state.manifest) state.assert_path_grants_rebound() + if state.manifest._has_process_environment_values(): + previous_sandbox_id = state.sandbox_id + previous_workspace_root_ready = state.workspace_root_ready + inner = DaytonaSandboxSession.from_state(state, sandbox=None) + inner._process_environment_resume_previous_sandbox_id = previous_sandbox_id + inner._process_environment_resume_previous_sandbox_loader = self._daytona.get + + async def start_replacement() -> None: + if isinstance(state.snapshot, NoopSnapshot): + raise _process_environment_error( + "Daytona cannot resume ProcessEnvValue without a live workspace or " + "restorable snapshot; use a restorable snapshot to preserve the " + "workspace" + ) + process_envs = state.manifest._snapshot_process_environment_values() + previous_sandbox = None + daytona_sandbox = None + try: + try: + previous_sandbox = await self._daytona.get(previous_sandbox_id) + except Exception as error: + not_found_error_types = _daytona_not_found_error_types() + if not not_found_error_types or not isinstance( + error, not_found_error_types + ): + raise + previous_sandbox = None + if previous_sandbox is not None: + previous_session = DaytonaSandboxSession.from_state( + state, sandbox=previous_sandbox + ) + previous_session.set_dependencies(inner._dependencies) + if await previous_session.running(): + await previous_session._persist_snapshot() + if not await state.snapshot.restorable(dependencies=inner._dependencies): + raise _process_environment_error( + "Daytona cannot resume ProcessEnvValue without a live workspace or " + "restorable snapshot; use a restorable snapshot to preserve the " + "workspace" + ) + params = await self._build_create_params( + sandbox_snapshot_name=state.sandbox_snapshot_name, + image=state.image, + env_vars=state.base_env_vars, + manifest=state.manifest, + process_envs=process_envs, + name=state.name, + resources=state.resources, + auto_stop_interval=state.auto_stop_interval, + ) + daytona_sandbox = await self._daytona.create( + params, timeout=state.create_timeout + ) + state.sandbox_id = daytona_sandbox.id + state.workspace_root_ready = False + inner._sandbox = daytona_sandbox + inner._set_start_state_preserved(False, system=False) + await BaseSandboxSession.start(inner) + except BaseException: + if daytona_sandbox is not None: + inner._process_environment_failed_candidate_sandbox = daytona_sandbox + inner._process_environment_failed_candidate_sandbox_id = daytona_sandbox.id + try: + await inner._cleanup_process_environment_failed_candidate() + except Exception: + pass + state.sandbox_id = previous_sandbox_id + state.workspace_root_ready = previous_workspace_root_ready + inner._sandbox = previous_sandbox + raise + inner._process_environment_resume_start = None + inner._process_environment_resume_started = True + await inner._retire_process_environment_previous_sandbox( + previous_sandbox=previous_sandbox + ) + + inner._process_environment_resume_start = start_replacement + return self._wrap_session(inner, instrumentation=self._instrumentation) + daytona_sandbox = None reconnected = False - try: - daytona_sandbox = await self._daytona.get(state.sandbox_id) - SandboxState = _import_sandbox_state() - if getattr(daytona_sandbox, "state", None) != SandboxState.STARTED: - await daytona_sandbox.start(timeout=state.start_timeout) - reconnected = True - except Exception as e: - log_tool_action_debug(logger, "Daytona sandbox lookup failed; recreating", e) + if not state.manifest._has_process_environment_values(): + try: + daytona_sandbox = await self._daytona.get(state.sandbox_id) + SandboxState = _import_sandbox_state() + if getattr(daytona_sandbox, "state", None) != SandboxState.STARTED: + await daytona_sandbox.start(timeout=state.start_timeout) + reconnected = True + except Exception as e: + log_tool_action_debug(logger, "Daytona sandbox lookup failed; recreating", e) if not reconnected or daytona_sandbox is None: params = await self._build_create_params( diff --git a/src/agents/extensions/sandbox/e2b/sandbox.py b/src/agents/extensions/sandbox/e2b/sandbox.py index 389b665c44..5ccb2e1445 100644 --- a/src/agents/extensions/sandbox/e2b/sandbox.py +++ b/src/agents/extensions/sandbox/e2b/sandbox.py @@ -1697,6 +1697,10 @@ async def create( if options is None: raise ValueError("E2BSandboxClient.create requires options") manifest = manifest if manifest is not None else Manifest() + manifest._reject_process_environment_values( + backend_id="e2b", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient", + ) self._validate_manifest_for_create(manifest) sandbox_type = _coerce_sandbox_type(options.sandbox_type) @@ -1777,6 +1781,10 @@ async def resume( if not isinstance(state, E2BSandboxSessionState): raise TypeError("E2BSandboxClient.resume expects an E2BSandboxSessionState") state.assert_path_grants_rebound() + state.manifest._reject_process_environment_values( + backend_id="e2b", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient instead", + ) sandbox_type = _coerce_sandbox_type(state.sandbox_type) SandboxClass = _import_sandbox_class(sandbox_type) diff --git a/src/agents/extensions/sandbox/modal/sandbox.py b/src/agents/extensions/sandbox/modal/sandbox.py index 71c7c551b8..dbd2f61303 100644 --- a/src/agents/extensions/sandbox/modal/sandbox.py +++ b/src/agents/extensions/sandbox/modal/sandbox.py @@ -2096,6 +2096,10 @@ async def create( if options is None: raise ValueError("ModalSandboxClient.create requires options with app_name") manifest = manifest if manifest is not None else Manifest() + manifest._reject_process_environment_values( + backend_id="modal", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient", + ) self._validate_manifest_for_create(manifest) app_name = options.app_name if not app_name: @@ -2273,6 +2277,10 @@ async def resume( if not isinstance(state, ModalSandboxSessionState): raise TypeError("ModalSandboxClient.resume expects a ModalSandboxSessionState") state.assert_path_grants_rebound() + state.manifest._reject_process_environment_values( + backend_id="modal", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient instead", + ) if _manifest_has_configured_mount_authority(state.manifest) and not ( state.mount_authority_rebound ): diff --git a/src/agents/extensions/sandbox/runloop/sandbox.py b/src/agents/extensions/sandbox/runloop/sandbox.py index 2662a55db4..2a0f93c967 100644 --- a/src/agents/extensions/sandbox/runloop/sandbox.py +++ b/src/agents/extensions/sandbox/runloop/sandbox.py @@ -1599,6 +1599,10 @@ async def create( if manifest is not None else Manifest(root=_default_runloop_manifest_root(user_parameters)) ) + manifest._reject_process_environment_values( + backend_id="runloop", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient", + ) _validate_runloop_manifest_root(manifest, user_parameters=user_parameters) self._validate_manifest_for_create(manifest) @@ -1697,6 +1701,10 @@ async def resume( if not isinstance(state, RunloopSandboxSessionState): raise TypeError("RunloopSandboxClient.resume expects a RunloopSandboxSessionState") state.assert_path_grants_rebound() + state.manifest._reject_process_environment_values( + backend_id="runloop", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient instead", + ) devbox = None reconnected = False diff --git a/src/agents/extensions/sandbox/vercel/sandbox.py b/src/agents/extensions/sandbox/vercel/sandbox.py index cf2d91afe6..2eb2a5e790 100644 --- a/src/agents/extensions/sandbox/vercel/sandbox.py +++ b/src/agents/extensions/sandbox/vercel/sandbox.py @@ -1564,6 +1564,10 @@ async def create( _resolve_manifest_root(manifest), options.allow_s3_credential_exposure, ) + resolved_manifest._reject_process_environment_values( + backend_id="vercel", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient", + ) try: self._validate_manifest_for_create(resolved_manifest) trusted_s3_mounts = _vercel_s3_mount_map(resolved_manifest) @@ -1623,6 +1627,10 @@ async def resume(self, state: SandboxSessionState) -> SandboxSession: if not isinstance(state, VercelSandboxSessionState): raise TypeError("VercelSandboxClient.resume expects a VercelSandboxSessionState") state.assert_path_grants_rebound() + state.manifest._reject_process_environment_values( + backend_id="vercel", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient instead", + ) if state.s3_mounts_non_resumable or _vercel_s3_mounts(state.manifest): raise MountConfigError( message=( diff --git a/src/agents/sandbox/_mount_security.py b/src/agents/sandbox/_mount_security.py index 95d08055d1..deaedac575 100644 --- a/src/agents/sandbox/_mount_security.py +++ b/src/agents/sandbox/_mount_security.py @@ -9,7 +9,7 @@ from collections.abc import Callable, Collection, Coroutine, Iterable, Mapping from functools import wraps from pathlib import PurePath, PurePosixPath -from typing import TYPE_CHECKING, Any, NoReturn, ParamSpec, TypeVar, cast, get_args +from typing import TYPE_CHECKING, Any, Literal, NoReturn, ParamSpec, TypeVar, cast, get_args from urllib.parse import urlsplit from ..exceptions import ( @@ -411,6 +411,8 @@ class _InContainerMountCredentialCapability: _RCLONE_SAFE_VALUE_ARGS = frozenset({"buffer-size", "gid", "uid"}) _SAFE_MOUNT_VALIDATION_MESSAGE_ATTR = "_agents_safe_mount_validation_message" _SAFE_MOUNT_VALIDATION_MESSAGE_MARKER = object() +_SAFE_PROCESS_ENVIRONMENT_ERROR_ATTR = "_agents_safe_process_environment_error" +_SAFE_PROCESS_ENVIRONMENT_ERROR_MARKER = object() _SANDBOX_ERROR_OPS = frozenset(get_args(OpName)) _STRUCTURED_SANDBOX_ERROR_SAFE_SUBTYPE_STATE: tuple[ tuple[type[SandboxError], tuple[tuple[str, object], ...]], ... @@ -487,7 +489,7 @@ class _InvalidRawMountManifestError(ValueError): def redact_mount_error_data( function: Callable[_P, Coroutine[Any, Any, _T]], ) -> Callable[_P, Coroutine[Any, Any, _T]]: - """Replace failures after clearing async frames that handled mount authority.""" + """Replace failures after clearing async frames that handled protected authority.""" @wraps(function) async def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _T: @@ -496,17 +498,40 @@ async def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _T: kwargs, function=function, ) + call_has_process_environment_access = _call_has_process_environment_access( + args, + kwargs, + function=function, + ) safe_error: BaseException | None = None try: return await function(*args, **kwargs) except BaseException as error: error_is_redacted = _is_error_data_redacted(error) - if call_has_authority or error_is_redacted: + safe_process_environment_error = ( + _replace_safe_process_environment_error(error) + if call_has_process_environment_access + else None + ) + if safe_process_environment_error is not None: + safe_error = safe_process_environment_error + elif call_has_authority: + safe_error = _replace_protected_mount_error(error) + elif call_has_process_environment_access: + safe_error = _replace_protected_process_environment_error(error) + elif error_is_redacted: safe_error = _replace_protected_mount_error(error) else: raise - del args, kwargs, call_has_authority, error_is_redacted + del ( + args, + kwargs, + call_has_authority, + call_has_process_environment_access, + error_is_redacted, + safe_process_environment_error, + ) assert safe_error is not None _raise_data_redacted_error(safe_error) @@ -527,21 +552,44 @@ def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _T: kwargs, function=function, ) + call_has_process_environment_access = _call_has_process_environment_access( + args, + kwargs, + function=function, + ) safe_error: BaseException | None = None try: return function(*args, **kwargs) except BaseException as error: error_is_redacted = _is_error_data_redacted(error) - if preserve_value_error_type and call_has_authority and isinstance(error, ValueError): + safe_process_environment_error = ( + _replace_safe_process_environment_error(error) + if call_has_process_environment_access + else None + ) + if safe_process_environment_error is not None: + safe_error = safe_process_environment_error + elif preserve_value_error_type and call_has_authority and isinstance(error, ValueError): discard_mount_source_exception(error) safe_error = ValueError("sandbox mount validation failed") _mark_error_data_redacted(safe_error) - elif call_has_authority or error_is_redacted: + elif call_has_authority: + safe_error = _replace_protected_mount_error(error) + elif call_has_process_environment_access: + safe_error = _replace_protected_process_environment_error(error) + elif error_is_redacted: safe_error = _replace_protected_mount_error(error) else: raise - del args, kwargs, call_has_authority, error_is_redacted + del ( + args, + kwargs, + call_has_authority, + call_has_process_environment_access, + error_is_redacted, + safe_process_environment_error, + ) assert safe_error is not None _raise_data_redacted_error(safe_error) @@ -588,16 +636,89 @@ def _replace_mount_error( def _replace_protected_mount_error(error: BaseException) -> BaseException: + return _replace_protected_sandbox_error( + error, + message="sandbox operation failed while using a protected mount configuration", + ) + + +def _replace_protected_process_environment_error(error: BaseException) -> BaseException: + return _replace_protected_sandbox_error( + error, + message="sandbox operation failed while using protected process environment values", + ) + + +def _mark_process_environment_error_safe(error: ValueError) -> None: + setattr( + error, + _SAFE_PROCESS_ENVIRONMENT_ERROR_ATTR, + _SAFE_PROCESS_ENVIRONMENT_ERROR_MARKER, + ) + + +def _replace_safe_process_environment_error(error: BaseException) -> ValueError | None: + pending = [error] + seen: set[int] = set() + message: str | None = None + while pending: + current = pending.pop() + if id(current) in seen: + continue + seen.add(id(current)) + if type(current) is ValueError: + state = _base_exception_instance_dict(current) + if ( + state is not None + and _exact_string_state_value(state, _SAFE_PROCESS_ENVIRONMENT_ERROR_ATTR) + is _SAFE_PROCESS_ENVIRONMENT_ERROR_MARKER + ): + try: + args = cast(Any, BaseException.args).__get__(current, ValueError) + except BaseException: + args = None + if type(args) is tuple and len(args) == 1 and type(args[0]) is str: + message = args[0] + break + for descriptor in ( + cast(Any, BaseException.__cause__), + cast(Any, BaseException.__context__), + ): + try: + candidate = descriptor.__get__(current, type(current)) + except BaseException: + continue + if issubclass(type(candidate), BaseException): + pending.append(cast(BaseException, candidate)) + if message is None: + return None + + discard_mount_source_exception(error) + safe_error = ValueError(message) + _mark_process_environment_error_safe(safe_error) + _mark_error_data_redacted(safe_error) + return safe_error + + +def _replace_protected_sandbox_error( + error: BaseException, + *, + message: str, +) -> BaseException: process_control_error = _replace_data_redacted_process_control_error(error) if process_control_error is not None: return process_control_error - structured_error = _replace_structured_sandbox_error(error) + structured_error = _replace_structured_sandbox_error(error, message=message) if structured_error is not None: return structured_error - return _replace_mount_operation_error(error) + return _replace_protected_operation_error(error, message=message) -def _replace_structured_sandbox_error(error: BaseException) -> SandboxError | None: +def _replace_structured_sandbox_error( + error: BaseException, + *, + message: str = "sandbox operation failed while using a protected mount configuration", +) -> SandboxError | None: error_type = type(error) state = _base_exception_instance_dict(error) if state is None: @@ -636,7 +757,6 @@ def _replace_structured_sandbox_error(error: BaseException) -> SandboxError | No discard_mount_source_exception(error) safe_error = cast(SandboxError, BaseException.__new__(error_type)) - message = "sandbox operation failed while using a protected mount configuration" object.__setattr__(safe_error, "message", message) object.__setattr__(safe_error, "error_code", error_code) object.__setattr__(safe_error, "op", cast(OpName, op)) @@ -650,11 +770,9 @@ def _replace_structured_sandbox_error(error: BaseException) -> SandboxError | No return safe_error -def _replace_mount_operation_error(error: BaseException) -> RuntimeError: +def _replace_protected_operation_error(error: BaseException, *, message: str) -> RuntimeError: discard_mount_source_exception(error) - safe_error = RuntimeError( - "sandbox operation failed while using a protected mount configuration" - ) + safe_error = RuntimeError(message) _mark_error_data_redacted(safe_error) return safe_error @@ -1016,6 +1134,10 @@ def _manifest_has_configured_mount_authority(manifest: Manifest) -> bool: return False +def _manifest_has_process_environment_access(manifest: Manifest) -> bool: + return bool(manifest._process_environment_access) + + def _mount_has_or_may_hide_configured_authority(mount: Mount) -> bool: """Classify untrusted mount implementations without reading their configuration.""" @@ -1142,7 +1264,36 @@ def _call_has_configured_mount_authority( *, function: Callable[..., object], ) -> bool: - """Inspect SDK-owned call-boundary state for protected authority.""" + return _call_has_configured_authority( + args, + kwargs, + function=function, + authority_kind="mount", + ) + + +def _call_has_process_environment_access( + args: tuple[object, ...], + kwargs: Mapping[str, object], + *, + function: Callable[..., object], +) -> bool: + return _call_has_configured_authority( + args, + kwargs, + function=function, + authority_kind="process_environment", + ) + + +def _call_has_configured_authority( + args: tuple[object, ...], + kwargs: Mapping[str, object], + *, + function: Callable[..., object], + authority_kind: Literal["mount", "process_environment"], +) -> bool: + """Inspect SDK-owned call-boundary state for one protected authority kind.""" from .manifest import Manifest from .session.base_sandbox_session import BaseSandboxSession @@ -1155,8 +1306,20 @@ def _call_has_configured_mount_authority( sandbox_session_state_descriptor = sandbox_session_metadata[0]["state"] values = (*args, *kwargs.values()) for value in values: - if type(value) is Manifest and _manifest_has_configured_mount_authority(value): - return True + if authority_kind == "process_environment": + try: + if vars(value).get("_process_environment_bindings"): + return True + except TypeError: + pass + if type(value) is Manifest: + if authority_kind == "mount" and _manifest_has_configured_mount_authority(value): + return True + if ( + authority_kind == "process_environment" + and _manifest_has_process_environment_access(value) + ): + return True decorated_owner_type = _decorated_owner_type(function) pending = [(value, False) for value in values] @@ -1175,13 +1338,20 @@ def _call_has_configured_mount_authority( value_metadata = _static_type_metadata(type(value)) value_mro = () if value_metadata is None else value_metadata[1] if type(value) is Manifest: - if _manifest_has_configured_mount_authority(value): + if authority_kind == "mount" and _manifest_has_configured_mount_authority(value): + return True + if ( + authority_kind == "process_environment" + and _manifest_has_process_environment_access(value) + ): return True continue if any(base is Manifest for base in value_mro): return True if any(base is Mount for base in value_mro): - if _mount_has_or_may_hide_configured_authority(cast(Mount, value)): + if authority_kind == "mount" and _mount_has_or_may_hide_configured_authority( + cast(Mount, value) + ): return True continue if type(value) is dict: @@ -1241,16 +1411,19 @@ def _call_has_configured_mount_authority( if found and candidate is not None: pending.append((candidate, False)) - credentials_found, credentials = _exact_string_state_entry( - state, - "_trusted_s3_mount_credentials", - ) - if type(credentials) is dict: - for configured in dict.values(credentials): - if type(configured) is tuple and any(item is not None for item in configured): - return True - elif credentials_found and credentials is not None: - return True + if authority_kind == "mount": + credentials_found, credentials = _exact_string_state_entry( + state, + "_trusted_s3_mount_credentials", + ) + if type(credentials) is dict: + for configured in dict.values(credentials): + if type(configured) is tuple and any( + item is not None for item in configured + ): + return True + elif credentials_found and credentials is not None: + return True except BaseException: return True return False diff --git a/src/agents/sandbox/manifest.py b/src/agents/sandbox/manifest.py index 97a3677442..f64f68e934 100644 --- a/src/agents/sandbox/manifest.py +++ b/src/agents/sandbox/manifest.py @@ -1,6 +1,7 @@ import abc import inspect -from collections.abc import Iterator, Mapping +import os +from collections.abc import Iterable, Iterator, Mapping from dataclasses import dataclass from pathlib import Path, PurePath, PurePosixPath from typing import Any, ClassVar, Literal @@ -19,7 +20,10 @@ from .._config_coercion import coerce_pydantic_config from ..util._asyncio_tasks import gather_with_cancel -from ._mount_security import redact_mount_validation_error_data_sync +from ._mount_security import ( + _mark_process_environment_error_safe, + redact_mount_validation_error_data_sync, +) from .entries import BaseEntry, Dir, Mount, resolve_workspace_path from .errors import InvalidManifestPathError from .manifest_render import render_manifest_description @@ -71,6 +75,19 @@ } ) +_PROCESS_ENVIRONMENT_ACCESS_KEYS = frozenset( + { + "process_environment_access", + "_process_environment_access", + "processEnvironmentAccess", + "_processEnvironmentAccess", + "process_environment_allowed_names", + "_process_environment_allowed_names", + "processEnvironmentAllowedNames", + "_processEnvironmentAllowedNames", + } +) + @dataclass(frozen=True) class _MountCredentialExposurePolicy: @@ -147,6 +164,67 @@ async def resolve(self) -> str: return self.value +class ProcessEnvValue(EnvValue): + """References a value in the SDK process environment. + + The source name defaults to the containing environment mapping key. Process + environment access must be granted on a trusted ``Manifest`` instance. + """ + + type: Literal["process_env"] = "process_env" + name: str | None = None + + @field_validator("name") + @classmethod + def _validate_name(cls, value: str | None) -> str | None: + if value is not None: + _validate_process_environment_name(value) + return value + + async def resolve(self) -> str: + raise _process_environment_error( + "ProcessEnvValue must be resolved through a trusted Manifest with " + "process environment access" + ) + + +def _validate_process_environment_name(name: str) -> None: + if not name: + raise ValueError("Process environment variable names must not be empty.") + if "=" in name or "\x00" in name: + raise ValueError("Process environment variable names must not contain '=' or NUL.") + + +def _normalize_process_environment_bindings( + *, + allowed_process_environment_keys: Iterable[str] = (), + process_environment_bindings: Mapping[str, str] | None = None, +) -> frozenset[tuple[str, str]]: + """Normalize trusted client configuration into exact destination/source bindings.""" + + normalized: dict[str, str] = {} + for key in allowed_process_environment_keys: + _validate_process_environment_name(key) + normalized[key] = key + for destination, source_name in (process_environment_bindings or {}).items(): + _validate_process_environment_name(destination) + _validate_process_environment_name(source_name) + existing_source = normalized.get(destination) + if existing_source is not None and existing_source != source_name: + raise ValueError( + "Process environment client configuration has conflicting bindings for " + f"destination {destination!r}" + ) + normalized[destination] = source_name + return frozenset(normalized.items()) + + +def _process_environment_error(message: str) -> ValueError: + error = ValueError(message) + _mark_process_environment_error_safe(error) + return error + + def _serialize_env_value_with_type(value: EnvValue, serialized: object) -> dict[str, Any]: if EnvValue._subclass_registry.get(value.type) is not type(value): raise PydanticSerializationError( @@ -233,14 +311,98 @@ def normalized(self) -> dict[str, EnvEntry]: return result async def resolve(self) -> dict[str, str]: + return await self._resolve(process_environment_access=frozenset()) + + async def _resolve( + self, + *, + process_environment_access: frozenset[tuple[str, str]], + include_process_values: bool = True, + include_non_process_values: bool = True, + ) -> dict[str, str]: normalized = self.normalized() - keys = normalized.keys() + process_bindings = _validate_process_environment_bindings( + normalized, + process_environment_access=process_environment_access, + require_values_present=include_process_values, + ) + process_values = ( + { + key: _read_process_environment_value(source_name) + for key, source_name in process_bindings.items() + } + if include_process_values + else {} + ) + custom_keys = ( + [ + key + for key, entry in normalized.items() + if not isinstance(entry.value, ProcessEnvValue) + ] + if include_non_process_values + else [] + ) + # `EnvValue` is an extension point, so these are user-supplied coroutines that # can reach a secret store or the network. A bare gather returns on the first # failure and leaves the rest running, which is how a rejected lookup ends up # with sibling fetches still in flight after the manifest has already failed. - values = await gather_with_cancel(*[normalized[key].value.resolve() for key in keys]) - return dict(zip(keys, values, strict=False)) + custom_values: tuple[str, ...] = () + resolved_custom_values: dict[str, str] = {} + try: + custom_values = await gather_with_cancel( + *[normalized[key].value.resolve() for key in custom_keys] + ) + resolved_custom_values = dict(zip(custom_keys, custom_values, strict=False)) + return { + key: ( + process_values[key] if key in process_bindings else resolved_custom_values[key] + ) + for key in normalized + if (include_process_values and key in process_bindings) + or (include_non_process_values and key not in process_bindings) + } + except BaseException: + custom_values = () + resolved_custom_values.clear() + process_values.clear() + raise + + +def _validate_process_environment_bindings( + normalized: Mapping[str, EnvEntry], + *, + process_environment_access: frozenset[tuple[str, str]], + require_values_present: bool = True, +) -> dict[str, str]: + process_bindings: dict[str, str] = {} + for key, entry in normalized.items(): + if not isinstance(entry.value, ProcessEnvValue): + continue + _validate_process_environment_name(key) + source_name = entry.value.name if entry.value.name is not None else key + _validate_process_environment_name(source_name) + if (key, source_name) not in process_environment_access: + raise _process_environment_error( + f"Process environment binding {source_name!r} -> {key!r} is not granted; " + "configure the sandbox client with an allowed process environment binding" + ) + if require_values_present and source_name not in os.environ: + raise _process_environment_error( + f"Process environment variable {source_name!r} is not set" + ) + process_bindings[key] = source_name + return process_bindings + + +def _read_process_environment_value(source_name: str) -> str: + try: + return os.environ[source_name] + except KeyError: + raise _process_environment_error( + f"Process environment variable {source_name!r} is not set" + ) from None class Manifest(BaseModel): @@ -257,17 +419,29 @@ class Manifest(BaseModel): _mount_credential_exposure_policy: _MountCredentialExposurePolicy = PrivateAttr( default_factory=_MountCredentialExposurePolicy ) + _process_environment_access: frozenset[tuple[str, str]] = PrivateAttr(default_factory=frozenset) + + def __getstate__(self) -> dict[Any, Any]: + state = super().__getstate__() + private_state = dict(state.get("__pydantic_private__") or {}) + private_state["_process_environment_access"] = frozenset() + state["__pydantic_private__"] = private_state + return state @model_validator(mode="before") @classmethod def _reject_mount_credential_exposure_policy_input(cls, value: object) -> object: - if isinstance(value, Mapping) and _MOUNT_CREDENTIAL_EXPOSURE_POLICY_KEYS.intersection( - value - ): - raise TypeError( - "In-container mount credential exposure must be configured on a trusted " - "Manifest instance, not in manifest input." - ) + if isinstance(value, Mapping): + if _MOUNT_CREDENTIAL_EXPOSURE_POLICY_KEYS.intersection(value): + raise TypeError( + "In-container mount credential exposure must be configured on a trusted " + "Manifest instance, not in manifest input." + ) + if _PROCESS_ENVIRONMENT_ACCESS_KEYS.intersection(value): + raise TypeError( + "Process environment access must be configured on a trusted Manifest " + "instance, not in manifest input." + ) return value @field_validator("entries", mode="before") @@ -293,6 +467,103 @@ def validated_entries(self) -> dict[str | Path, BaseEntry]: pass return validated + def _with_process_environment_access( + self, + *bindings: frozenset[tuple[str, str]] | str | tuple[str, str], + ) -> "Manifest": + """Attach trusted client-owned process environment bindings at runtime.""" + + declared_bindings = self._declared_process_environment_bindings() + if len(bindings) == 1 and isinstance(bindings[0], frozenset): + requested_bindings = bindings[0] + else: + same_name_keys = [binding for binding in bindings if isinstance(binding, str)] + renamed_bindings = { + binding[0]: binding[1] + for binding in bindings + if isinstance(binding, tuple) and len(binding) == 2 + } + requested_bindings = _normalize_process_environment_bindings( + allowed_process_environment_keys=same_name_keys, + process_environment_bindings=renamed_bindings, + ) + trusted = self.model_copy() + trusted._process_environment_access = requested_bindings & declared_bindings + return trusted + + async def resolve_environment(self) -> dict[str, str]: + """Resolve the sandbox environment using this manifest's runtime-only authority.""" + + return await self.environment._resolve( + process_environment_access=self._process_environment_access + ) + + async def _resolve_environment_without_process_values(self) -> dict[str, str]: + """Resolve non-process values without materializing protected process values.""" + + return await self.environment._resolve( + process_environment_access=self._process_environment_access, + include_process_values=False, + ) + + async def _resolve_process_environment_values(self) -> dict[str, str]: + """Resolve only protected process values for an out-of-band provider channel.""" + + return await self.environment._resolve( + process_environment_access=self._process_environment_access, + include_non_process_values=False, + ) + + def _validate_process_environment_access(self) -> None: + """Validate process environment references without returning their values.""" + + _validate_process_environment_bindings( + self.environment.normalized(), + process_environment_access=self._process_environment_access, + ) + + def _snapshot_process_environment_values(self) -> dict[str, str]: + """Snapshot protected process values before provider or resolver side effects.""" + + normalized = self.environment.normalized() + process_bindings = _validate_process_environment_bindings( + normalized, + process_environment_access=self._process_environment_access, + ) + return { + key: _read_process_environment_value(source_name) + for key, source_name in process_bindings.items() + } + + def _declared_process_environment_bindings(self) -> frozenset[tuple[str, str]]: + return frozenset( + (key, entry.value.name if entry.value.name is not None else key) + for key, entry in self.environment.normalized().items() + if isinstance(entry.value, ProcessEnvValue) + ) + + def _has_process_environment_values(self) -> bool: + return bool(self._declared_process_environment_bindings()) + + def _has_process_environment_access(self) -> bool: + return bool( + self._process_environment_access & self._declared_process_environment_bindings() + ) + + def _reject_process_environment_values( + self, + *, + backend_id: str, + supported_alternative: str, + ) -> None: + if not self._has_process_environment_values(): + return + raise _process_environment_error( + f"{backend_id} does not support ProcessEnvValue because it cannot transport " + "protected values while enforcing the required host-environment isolation and " + f"out-of-band provider boundary; {supported_alternative}" + ) + @redact_mount_validation_error_data_sync def with_in_container_mount_credential_exposure_acknowledged( self, *mount_paths: str | PurePath diff --git a/src/agents/sandbox/runtime_session_manager.py b/src/agents/sandbox/runtime_session_manager.py index bc1a5379e9..505a914a07 100644 --- a/src/agents/sandbox/runtime_session_manager.py +++ b/src/agents/sandbox/runtime_session_manager.py @@ -23,7 +23,6 @@ _replace_protected_mount_error, _validate_manifest_mount_provenance, redact_mount_error_data, - validate_manifest_mount_credential_boundaries, ) from .capabilities import Capability from .entries import BaseEntry, Dir, Mount, resolve_workspace_path @@ -596,10 +595,16 @@ async def _process_live_session_manifest( current_manifest, run_as_user=cls._agent_run_as_user(agent), ) + if current_manifest._has_process_environment_values() or ( + processed_manifest is not None and processed_manifest._has_process_environment_values() + ): + raise ValueError( + "Injected sandbox sessions cannot use ProcessEnvValue bindings; " + "use a client-owned fresh session or resume path instead" + ) if processed_manifest is None or processed_manifest == current_manifest: - validate_manifest_mount_credential_boundaries( - current_manifest, - provider_backend_id=session.state.type, + await session._validate_manifest_before_provider_probe( + manifest=current_manifest, ) running = await session.running() await session._validate_manifest_application( @@ -612,9 +617,8 @@ async def _process_live_session_manifest( current_manifest=current_manifest, processed_manifest=processed_manifest, ) - validate_manifest_mount_credential_boundaries( - processed_manifest, - provider_backend_id=session.state.type, + await session._validate_manifest_before_provider_probe( + manifest=processed_manifest, ) running = await session.running() await session._validate_manifest_application( diff --git a/src/agents/sandbox/sandboxes/docker.py b/src/agents/sandbox/sandboxes/docker.py index 3a8411b6f6..0e8480df74 100644 --- a/src/agents/sandbox/sandboxes/docker.py +++ b/src/agents/sandbox/sandboxes/docker.py @@ -12,7 +12,7 @@ import time import uuid from collections import deque -from collections.abc import Iterable, Iterator +from collections.abc import Awaitable, Callable, Iterable, Iterator, Mapping from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass, field from pathlib import Path @@ -48,9 +48,12 @@ WorkspaceArchiveReadError, WorkspaceArchiveWriteError, ) -from ..manifest import Manifest +from ..manifest import Manifest, _process_environment_error from ..session import SandboxSession, SandboxSessionState -from ..session.base_sandbox_session import BaseSandboxSession +from ..session.base_sandbox_session import ( + BaseSandboxSession, + _register_sdk_process_environment_session_type, +) from ..session.dependencies import Dependencies from ..session.manager import Instrumentation from ..session.pty_output import collect_pty_output @@ -66,7 +69,7 @@ from ..session.runtime_helpers import RESOLVE_WORKSPACE_PATH_HELPER, RuntimeHelperScript from ..session.sandbox_client import BaseSandboxClient, BaseSandboxClientOptions from ..session.workspace_payloads import coerce_write_payload -from ..snapshot import SnapshotBase, SnapshotSpec, resolve_snapshot +from ..snapshot import NoopSnapshot, SnapshotBase, SnapshotSpec, resolve_snapshot from ..types import ExecResult, ExposedPortEndpoint, User from ..util.iterator_io import IteratorIO from ..util.retry import ( @@ -242,6 +245,7 @@ def close(self) -> None: pass +@_register_sdk_process_environment_session_type class DockerSandboxSession(BaseSandboxSession): _docker_client: DockerSDKClient _container: Container @@ -251,6 +255,7 @@ class DockerSandboxSession(BaseSandboxSession): _pty_processes: dict[int, _DockerPtyProcessEntry] _reserved_pty_process_ids: set[int] _cleanup_tasks: set[asyncio.Task[None]] + _process_environment_resume_start: Callable[[], Awaitable[None]] | None state: DockerSandboxSessionState _ARCHIVE_STAGING_DIR: Path = posix_path_as_path( @@ -273,6 +278,15 @@ def __init__( self._pty_processes = {} self._reserved_pty_process_ids = set() self._cleanup_tasks = set() + self._process_environment_start_lock = asyncio.Lock() + self._process_environment_resume_started = False + self._process_environment_resume_start = None + self._process_environment_resume_previous_container_id: str | None = None + self._process_environment_resume_previous_container_loader: Any = None + self._process_environment_resume_previous_volume_names: tuple[str, ...] = () + self._process_environment_failed_candidate_container: Container | None = None + self._process_environment_failed_candidate_container_id: str | None = None + self._process_environment_failed_candidate_volume_names: tuple[str, ...] = () @classmethod def from_state( @@ -289,6 +303,85 @@ def supports_docker_volume_mounts(self) -> bool: return True + @redact_mount_error_data + async def start(self) -> None: + async with self._process_environment_start_lock: + self._cleanup_process_environment_failed_candidate() + if self._process_environment_resume_started: + if not await self.running(): + self._set_start_state_preserved(True) + await super().start() + self._retire_process_environment_previous_resources() + return + deferred_start = getattr(self, "_process_environment_resume_start", None) + if deferred_start is None: + await super().start() + return + await deferred_start() + self._process_environment_resume_start = None + self._process_environment_resume_started = True + + def _cleanup_process_environment_failed_candidate(self) -> None: + container_id = self._process_environment_failed_candidate_container_id + container = self._process_environment_failed_candidate_container + if container is None and container_id is not None: + try: + container = self._docker_client.containers.get(container_id) + except docker.errors.NotFound: + self._process_environment_failed_candidate_container_id = None + if container is not None: + try: + container.remove(force=True) + except docker.errors.NotFound: + self._process_environment_failed_candidate_container_id = None + else: + self._process_environment_failed_candidate_container_id = None + if self._process_environment_failed_candidate_container_id is None: + self._process_environment_failed_candidate_container = None + + remaining_volume_names: list[str] = [] + for volume_name in self._process_environment_failed_candidate_volume_names: + try: + self._docker_client.volumes.get(volume_name).remove() + except docker.errors.NotFound: + continue + except Exception: + remaining_volume_names.append(volume_name) + self._process_environment_failed_candidate_volume_names = tuple(remaining_volume_names) + if ( + self._process_environment_failed_candidate_container is not None + or self._process_environment_failed_candidate_container_id is not None + or remaining_volume_names + ): + raise RuntimeError("Docker failed to clean up a process environment replacement") + + def _retire_process_environment_previous_resources( + self, + *, + previous_container: Container | None = None, + ) -> None: + previous_container_id = self._process_environment_resume_previous_container_id + if previous_container_id is not None: + if previous_container is None: + loader = self._process_environment_resume_previous_container_loader + try: + previous_container = loader(previous_container_id) + except docker.errors.NotFound: + self._process_environment_resume_previous_container_id = None + if previous_container is not None: + try: + previous_container.remove(force=True) + except docker.errors.NotFound: + pass + self._process_environment_resume_previous_container_id = None + + for volume_name in self._process_environment_resume_previous_volume_names: + try: + self._docker_client.volumes.get(volume_name).remove() + except docker.errors.NotFound: + continue + self._process_environment_resume_previous_volume_names = () + def supports_pty(self) -> bool: return True @@ -602,6 +695,7 @@ def _coerce_exec_user(user: str | User | None) -> str | None: return user.name return user + @redact_mount_error_data async def exec( self, *command: str | Path, @@ -892,17 +986,41 @@ async def running(self) -> bool: return cast(str, self._container.status) == "running" async def _shutdown_backend(self) -> None: - # Best-effort: stop the container if it exists. - try: - self._container.reload() - except Exception: - pass + cleanup_error: BaseException | None = None try: - if await self.running(): - self._container.stop() - except Exception: - # If the container is already gone/stopped, ignore. - pass + self._cleanup_process_environment_failed_candidate() + except BaseException as exc: + cleanup_error = exc + deferred_container_id = self._process_environment_resume_previous_container_id + if self._container is None and deferred_container_id is not None: + loader = self._process_environment_resume_previous_container_loader + try: + self._container = loader(deferred_container_id) + except docker.errors.NotFound: + self._process_environment_resume_previous_container_id = None + except BaseException as exc: + if cleanup_error is None: + cleanup_error = exc + # Best-effort: stop the container if it exists. + if self._container is not None: + try: + self._container.reload() + except Exception: + pass + try: + if await self.running(): + self._container.stop() + except Exception: + # If the container is already gone/stopped, ignore. + pass + if deferred_container_id is not None and self.state.container_id != deferred_container_id: + try: + self._retire_process_environment_previous_resources() + except BaseException as exc: + if cleanup_error is None: + cleanup_error = exc + if cleanup_error is not None: + raise cleanup_error from None @staticmethod def _start_exec_socket(*, api: Any, exec_id: str, tty: bool = False) -> _DockerExecSocket: @@ -923,6 +1041,7 @@ def _start_exec_socket(*, api: Any, exec_id: str, tty: bool = False) -> _DockerE raw_sock = getattr(sock, "_sock", sock) return _DockerExecSocket(sock=sock, raw_sock=raw_sock, response=response) + @redact_mount_error_data async def pty_exec_start( self, *command: str | Path, @@ -1057,6 +1176,7 @@ async def pty_exec_start( original_token_count=original_token_count, ) + @redact_mount_error_data async def pty_write_stdin( self, *, @@ -1474,8 +1594,14 @@ def __init__( *, instrumentation: Instrumentation | None = None, dependencies: Dependencies | None = None, + allowed_process_environment_keys: Iterable[str] = (), + process_environment_bindings: Mapping[str, str] | None = None, ) -> None: super().__init__() + self._configure_process_environment_bindings( + allowed_process_environment_keys=allowed_process_environment_keys, + process_environment_bindings=process_environment_bindings, + ) self.docker_client = docker_client self._instrumentation = ( instrumentation if instrumentation is not None else Instrumentation() @@ -1493,7 +1619,7 @@ async def create( image = options.image session_id = uuid.uuid4() manifest = manifest if manifest is not None else Manifest() - self._validate_manifest_for_create(manifest) + manifest = self._validate_manifest_for_create(manifest) _validate_docker_path_grants(manifest) volume_names = _docker_volume_names_for_manifest(manifest, session_id=session_id) container: Container | None = None @@ -1527,6 +1653,7 @@ async def create( self._cleanup_failed_create_resources( container=container, volume_names=volume_names, + surface_cleanup_failure=manifest._has_process_environment_values(), ) raise @@ -1535,28 +1662,46 @@ def _cleanup_failed_create_resources( *, container: Container | None, volume_names: Iterable[str], + surface_cleanup_failure: bool = False, ) -> None: """Best-effort cleanup when Docker resource acquisition does not return a session.""" + cleanup_failed = False + container_id = getattr(container, "id", None) if container is not None: try: container.remove(force=True) - except Exception: + except docker.errors.NotFound: pass + except Exception: + cleanup_failed = True + failed_volume_names: list[str] = [] for volume_name in volume_names: try: self.docker_client.volumes.get(volume_name).remove() + except docker.errors.NotFound: + continue except Exception: - pass + cleanup_failed = True + failed_volume_names.append(volume_name) + if surface_cleanup_failure and cleanup_failed: + raise _process_environment_error( + "Docker failed to clean up protected create resources; " + f"container_id={container_id!r}, volume_names={failed_volume_names!r}" + ) from None @redact_mount_error_data async def delete(self, session: SandboxSession) -> SandboxSession: inner = session._inner if not isinstance(inner, DockerSandboxSession): raise TypeError("DockerSandboxClient.delete expects a DockerSandboxSession") - volume_names = _docker_volume_names_for_manifest( - inner.state.manifest, - session_id=inner.state.session_id, + volume_names = ( + *_docker_volume_names_for_manifest( + inner.state.manifest, + session_id=inner.state.session_id, + ), + *inner._process_environment_resume_previous_volume_names, + *inner._process_environment_failed_candidate_volume_names, ) cleanup_error: BaseException | None = None try: @@ -1564,29 +1709,61 @@ async def delete(self, session: SandboxSession) -> SandboxSession: except BaseException as exc: cleanup_error = exc - try: - container = self.docker_client.containers.get(inner.state.container_id) - except docker.errors.NotFound: - container = None - except BaseException as exc: - container = None - if cleanup_error is None: - cleanup_error = exc - else: + retained_container_id = inner._process_environment_resume_previous_container_id + failed_candidate_container_id = inner._process_environment_failed_candidate_container_id + container_ids = tuple( + dict.fromkeys( + container_id + for container_id in ( + inner.state.container_id, + retained_container_id, + failed_candidate_container_id, + ) + if container_id is not None + ) + ) + for container_id in container_ids: try: - container.remove() + container = self.docker_client.containers.get(container_id) except docker.errors.NotFound: - pass + if container_id == retained_container_id: + inner._process_environment_resume_previous_container_id = None + if container_id == failed_candidate_container_id: + inner._process_environment_failed_candidate_container_id = None + inner._process_environment_failed_candidate_container = None + continue + except BaseException as exc: + if cleanup_error is None: + cleanup_error = exc + continue + try: + if container_id in {retained_container_id, failed_candidate_container_id}: + container.remove(force=True) + if container_id == retained_container_id: + inner._process_environment_resume_previous_container_id = None + if container_id == failed_candidate_container_id: + inner._process_environment_failed_candidate_container_id = None + inner._process_environment_failed_candidate_container = None + else: + container.remove() + except docker.errors.NotFound: + if container_id == retained_container_id: + inner._process_environment_resume_previous_container_id = None + if container_id == failed_candidate_container_id: + inner._process_environment_failed_candidate_container_id = None + inner._process_environment_failed_candidate_container = None except BaseException as exc: if cleanup_error is None: cleanup_error = exc + retained_volumes_removed = True for volume_name in volume_names: try: volume = self.docker_client.volumes.get(volume_name) except docker.errors.NotFound: continue except BaseException as exc: + retained_volumes_removed = False if cleanup_error is None: cleanup_error = exc continue @@ -1595,8 +1772,12 @@ async def delete(self, session: SandboxSession) -> SandboxSession: except docker.errors.NotFound: continue except BaseException as exc: + retained_volumes_removed = False if cleanup_error is None: cleanup_error = exc + if retained_volumes_removed: + inner._process_environment_resume_previous_volume_names = () + inner._process_environment_failed_candidate_volume_names = () if cleanup_error is not None: raise cleanup_error from None return session @@ -1608,10 +1789,106 @@ async def resume( ) -> SandboxSession: if not isinstance(state, DockerSandboxSessionState): raise TypeError("DockerSandboxClient.resume expects a DockerSandboxSessionState") + state.manifest = self._bind_process_environment_manifest(state.manifest) state.assert_path_grants_rebound() _validate_docker_path_grants(state.manifest) + if state.manifest._has_process_environment_values(): + original_container_id = state.container_id + original_session_id = state.session_id + original_workspace_root_ready = state.workspace_root_ready + inner = DockerSandboxSession( + container=cast(Container, None), + docker_client=self.docker_client, + state=state, + ) + inner._process_environment_resume_previous_container_id = original_container_id + inner._process_environment_resume_previous_container_loader = self.get_container + inner._process_environment_resume_previous_volume_names = tuple( + _docker_volume_names_for_manifest( + state.manifest, + session_id=original_session_id, + ) + ) + + async def start_replacement() -> None: + if isinstance(state.snapshot, NoopSnapshot): + raise _process_environment_error( + "Docker cannot resume ProcessEnvValue without a live workspace or " + "restorable snapshot; use a restorable snapshot to preserve the " + "workspace" + ) + resolved_environment = await state.manifest.resolve_environment() + try: + previous_container = self.get_container(original_container_id) + except docker.errors.NotFound: + previous_container = None + if previous_container is not None: + previous_session = DockerSandboxSession( + container=previous_container, + docker_client=self.docker_client, + state=state, + ) + previous_session.set_dependencies(inner._dependencies) + if await previous_session.running(): + await previous_session._persist_snapshot() + if not await state.snapshot.restorable(dependencies=inner._dependencies): + raise _process_environment_error( + "Docker cannot resume ProcessEnvValue without a live workspace or " + "restorable snapshot; use a restorable snapshot to preserve the workspace" + ) + replacement_session_id = uuid.uuid4() + replacement_volume_names = _docker_volume_names_for_manifest( + state.manifest, + session_id=replacement_session_id, + ) + container: Container | None = None + try: + state.session_id = replacement_session_id + container = await self._create_container( + state.image, + manifest=state.manifest, + exposed_ports=state.exposed_ports, + session_id=replacement_session_id, + resolved_environment=resolved_environment, + ) + container_id = container.id + assert container_id is not None + state.container_id = container_id + state.workspace_root_ready = False + inner._container = container + inner._resume_workspace_probe_pending = True + inner._set_start_state_preserved(False) + await BaseSandboxSession.start(inner) + except BaseException: + if container is not None: + inner._process_environment_failed_candidate_container = container + inner._process_environment_failed_candidate_container_id = container.id + inner._process_environment_failed_candidate_volume_names = tuple( + replacement_volume_names + ) + state.container_id = original_container_id + state.session_id = original_session_id + state.workspace_root_ready = original_workspace_root_ready + inner._container = previous_container + try: + inner._cleanup_process_environment_failed_candidate() + except Exception: + pass + raise + inner._process_environment_resume_start = None + inner._process_environment_resume_started = True + inner._retire_process_environment_previous_resources( + previous_container=previous_container + ) + + inner._process_environment_resume_start = start_replacement + return self._wrap_session(inner, instrumentation=self._instrumentation) configured_authority = _manifest_has_configured_mount_authority(state.manifest) - requires_fresh_resource = state.mount_authority_rebound or configured_authority + requires_fresh_resource = ( + state.mount_authority_rebound + or state.manifest._has_process_environment_values() + or configured_authority + ) container = None if requires_fresh_resource else self.get_container(state.container_id) reused_existing_container = container is not None if container is not None: @@ -1676,18 +1953,19 @@ async def _create_container( manifest: Manifest | None = None, exposed_ports: tuple[int, ...] = (), session_id: uuid.UUID | None = None, + resolved_environment: dict[str, str] | None = None, ) -> Container: + environment = resolved_environment if manifest is not None: _validate_docker_path_grants(manifest) + if manifest is not None and environment is None: + environment = await manifest.resolve_environment() # create image if it does not exist if not self.image_exists(image): repo, tag = parse_repository_tag(image) self.docker_client.images.pull(repo, tag=tag or None, all_tags=False) assert self.image_exists(image) - environment: dict[str, str] | None = None - if manifest is not None: - environment = await manifest.environment.resolve() create_kwargs: dict[str, object] = { "entrypoint": ["tail"], "image": image, diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index 4d8595b15e..1ad3c646cf 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/src/agents/sandbox/sandboxes/unix_local.py @@ -1115,6 +1115,10 @@ async def create( ) -> SandboxSession: resolved_options = options if options is not None else UnixLocalSandboxClientOptions() manifest = manifest if manifest is not None else Manifest() + manifest._reject_process_environment_values( + backend_id="unix_local", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient", + ) _assert_unix_local_host_path_grants_unsupported(manifest) self._validate_manifest_for_create(manifest) # For local execution, runner-created sessions should always get an isolated temp root @@ -1175,6 +1179,10 @@ async def resume( if not isinstance(state, UnixLocalSandboxSessionState): raise TypeError("UnixLocalSandboxClient.resume expects a UnixLocalSandboxSessionState") state.assert_path_grants_rebound() + state.manifest._reject_process_environment_values( + backend_id="unix_local", + supported_alternative="use DockerSandboxClient or DaytonaSandboxClient instead", + ) _assert_unix_local_host_path_grants_unsupported(state.manifest) inner = UnixLocalSandboxSession.from_state(state) return self._wrap_session(inner, instrumentation=self._instrumentation) diff --git a/src/agents/sandbox/session/base_sandbox_session.py b/src/agents/sandbox/session/base_sandbox_session.py index d377bea9ef..b8e00ddda3 100644 --- a/src/agents/sandbox/session/base_sandbox_session.py +++ b/src/agents/sandbox/session/base_sandbox_session.py @@ -3,8 +3,9 @@ import io import shlex from collections.abc import Awaitable, Callable, Mapping, Sequence +from functools import wraps from pathlib import Path, PurePath -from typing import Literal, NoReturn, TypeVar +from typing import Any, Literal, NoReturn, TypeVar from typing_extensions import Self @@ -55,6 +56,26 @@ _RUNTIME_HELPER_CACHE_KEY_UNSET = object() _WORKSPACE_ROOT_PROBE_TIMEOUT_S = 10.0 _READ_PATH_PROBE_TIMEOUT_S = 10.0 +_PROCESS_ENVIRONMENT_OPERATION_GUARD = "_process_environment_operation_guard" +_PROCESS_ENVIRONMENT_VALIDATED_OPERATIONS = ( + "apply_manifest", + "exec", + "extract", + "hydrate_workspace", + "ls", + "mkdir", + "persist_workspace", + "provision_manifest_accounts", + "pty_exec_start", + "pty_terminate_all", + "pty_write_stdin", + "read", + "resolve_exposed_port", + "rm", + "running", + "start", + "write", +) _READ_PATH_PROBE_SCRIPT = """ # READ_PATH_PROBE_V3 LC_ALL=C @@ -196,6 +217,52 @@ ) +@redact_mount_error_data +async def _run_redacted_process_environment_operation( + operation: Any, + self: "BaseSandboxSession", + *args: object, + **kwargs: object, +) -> object: + return await operation(self, *args, **kwargs) + + +def _guard_process_environment_operation(operation: Any) -> Any: + @wraps(operation) + async def guarded( + self: "BaseSandboxSession", + *args: object, + **kwargs: object, + ) -> object: + self._validate_process_environment_compatibility() + if self.state.manifest._has_process_environment_values(): + return await _run_redacted_process_environment_operation( + operation, + self, + *args, + **kwargs, + ) + return await operation(self, *args, **kwargs) + + setattr(guarded, _PROCESS_ENVIRONMENT_OPERATION_GUARD, True) + return guarded + + +_SDK_PROCESS_ENVIRONMENT_SESSION_TYPES: set[type["BaseSandboxSession"]] = set() +_SessionClassT = TypeVar("_SessionClassT", bound=type["BaseSandboxSession"]) + + +def _register_sdk_process_environment_session_type( + session_type: _SessionClassT, +) -> _SessionClassT: + _SDK_PROCESS_ENVIRONMENT_SESSION_TYPES.add(session_type) + return session_type + + +def _is_sdk_process_environment_session(session: "BaseSandboxSession") -> bool: + return type(session) in _SDK_PROCESS_ENVIRONMENT_SESSION_TYPES + + class BaseSandboxSession(abc.ABC): state: SandboxSessionState _dependencies: Dependencies | None = None @@ -224,6 +291,32 @@ class BaseSandboxSession(abc.ABC): _max_local_dir_file_concurrency: int | None = DEFAULT_MAX_LOCAL_DIR_FILE_CONCURRENCY _archive_limits: SandboxArchiveLimits | None = None + def __init_subclass__(cls, **kwargs: object) -> None: + super().__init_subclass__(**kwargs) + for name in _PROCESS_ENVIRONMENT_VALIDATED_OPERATIONS: + operation = getattr(cls, name, None) + if operation is None or getattr(operation, _PROCESS_ENVIRONMENT_OPERATION_GUARD, False): + continue + setattr(cls, name, _guard_process_environment_operation(operation)) + + def _validate_process_environment_compatibility( + self, + *, + manifest: Manifest | None = None, + ) -> None: + current_manifest = manifest or self.state.manifest + if not current_manifest._has_process_environment_values(): + return + if _is_sdk_process_environment_session(self): + return + current_manifest._reject_process_environment_values( + backend_id=getattr(self.state, "type", type(self).__name__), + supported_alternative=( + "use DockerSandboxClient or DaytonaSandboxClient for protected " + "process environment transport" + ), + ) + def _runtime_has_protected_mount_authority(self) -> bool: """Return whether SDK-owned runtime state contains live mount authority.""" @@ -233,6 +326,7 @@ def _runtime_has_protected_mount_authority(self) -> bool: async def start(self) -> None: from .._mount_security import validate_manifest_mount_credential_boundaries + self._validate_process_environment_compatibility() validate_manifest_mount_credential_boundaries( self.state.manifest, provider_backend_id=self.state.type, @@ -496,11 +590,15 @@ async def _aclose_impl(self) -> None: cleanup_error = exc try: if cleanup_error is None and not self._pre_stop_hooks_failed: - await self.stop() - await self.shutdown() - except BaseException as exc: - if cleanup_error is None: - cleanup_error = exc + try: + await self.stop() + except BaseException as exc: + cleanup_error = exc + try: + await self.shutdown() + except BaseException as exc: + if cleanup_error is None: + cleanup_error = exc finally: try: await self._aclose_dependencies() @@ -1276,8 +1374,18 @@ async def _validate_manifest_application( session_running: bool | None = None, ) -> None: _ = (only_ephemeral, session_running) + await self._validate_manifest_before_provider_probe( + manifest=manifest, + ) + + async def _validate_manifest_before_provider_probe( + self, + *, + manifest: Manifest | None = None, + ) -> None: from .._mount_security import validate_manifest_mount_credential_boundaries + self._validate_process_environment_compatibility(manifest=manifest) validate_manifest_mount_credential_boundaries( manifest or self.state.manifest, provider_backend_id=self.state.type, diff --git a/src/agents/sandbox/session/sandbox_client.py b/src/agents/sandbox/session/sandbox_client.py index f887cf3827..0f2527d44c 100644 --- a/src/agents/sandbox/session/sandbox_client.py +++ b/src/agents/sandbox/session/sandbox_client.py @@ -1,7 +1,7 @@ from __future__ import annotations import abc -from collections.abc import Mapping +from collections.abc import Iterable, Mapping from typing import Any, ClassVar, Generic, TypeVar, cast from pydantic import BaseModel, ConfigDict, model_serializer @@ -11,7 +11,7 @@ redact_mount_error_data_sync, ) from ..errors import MountConfigError -from ..manifest import Manifest +from ..manifest import Manifest, _normalize_process_environment_bindings from ..snapshot import SnapshotBase, SnapshotSpec from .base_sandbox_session import BaseSandboxSession from .dependencies import Dependencies @@ -110,6 +110,21 @@ class BaseSandboxClient(abc.ABC, Generic[ClientOptionsT]): backend_id: str supports_default_options: bool = False _dependencies: Dependencies | None = None + _process_environment_bindings: frozenset[tuple[str, str]] = frozenset() + + def _configure_process_environment_bindings( + self, + *, + allowed_process_environment_keys: Iterable[str] = (), + process_environment_bindings: Mapping[str, str] | None = None, + ) -> None: + self._process_environment_bindings = _normalize_process_environment_bindings( + allowed_process_environment_keys=allowed_process_environment_keys, + process_environment_bindings=process_environment_bindings, + ) + + def _bind_process_environment_manifest(self, manifest: Manifest) -> Manifest: + return manifest._with_process_environment_access(self._process_environment_bindings) def _resolve_dependencies(self) -> Dependencies | None: if self._dependencies is None: @@ -138,11 +153,13 @@ def _validate_manifest_for_create( ) -> Manifest: from .._mount_security import validate_manifest_mount_credential_boundaries + trusted_manifest = self._bind_process_environment_manifest(manifest) validate_manifest_mount_credential_boundaries( - manifest, + trusted_manifest, provider_backend_id=self.backend_id, ) - return manifest + trusted_manifest._validate_process_environment_access() + return trusted_manifest @abc.abstractmethod async def create( diff --git a/src/agents/sandbox/session/sandbox_session.py b/src/agents/sandbox/session/sandbox_session.py index 923f025857..deab4d3cac 100644 --- a/src/agents/sandbox/session/sandbox_session.py +++ b/src/agents/sandbox/session/sandbox_session.py @@ -264,6 +264,13 @@ def state(self, value: SandboxSessionState) -> None: # pragma: no cover def _runtime_has_protected_mount_authority(self) -> bool: return self._inner._runtime_has_protected_mount_authority() + def _validate_process_environment_compatibility( + self, + *, + manifest: Manifest | None = None, + ) -> None: + self._inner._validate_process_environment_compatibility(manifest=manifest) + @property def dependencies(self) -> Dependencies: return self._inner.dependencies @@ -550,6 +557,13 @@ async def _validate_manifest_application( session_running=session_running, ) + async def _validate_manifest_before_provider_probe( + self, + *, + manifest: Manifest | None = None, + ) -> None: + await self._inner._validate_manifest_before_provider_probe(manifest=manifest) + async def apply_manifest(self, *, only_ephemeral: bool = False) -> MaterializationResult: return await super().apply_manifest(only_ephemeral=only_ephemeral) @@ -583,6 +597,7 @@ async def _resolve_exposed_port(self, port: int) -> ExposedPortEndpoint: _ = port raise NotImplementedError("this should never be invoked") + @redact_mount_error_data async def pty_exec_start( self, *command: str | Path, diff --git a/src/agents/sandbox/session/sandbox_session_state.py b/src/agents/sandbox/session/sandbox_session_state.py index 43dea6cb2e..889ef73779 100644 --- a/src/agents/sandbox/session/sandbox_session_state.py +++ b/src/agents/sandbox/session/sandbox_session_state.py @@ -333,6 +333,7 @@ def assert_path_grants_rebound(self) -> None: self.manifest, provider_backend_id=self.type, ) + self.manifest._validate_process_environment_access() if self.mount_authority_redacted: raise ValueError( diff --git a/tests/extensions/sandbox/test_daytona.py b/tests/extensions/sandbox/test_daytona.py index 7f2df5bb4f..14a1a0f518 100644 --- a/tests/extensions/sandbox/test_daytona.py +++ b/tests/extensions/sandbox/test_daytona.py @@ -6,6 +6,7 @@ import io import shlex import sys +import tarfile import types import uuid from collections import deque @@ -37,7 +38,7 @@ from agents.sandbox.entries.mounts.base import InContainerMountAdapter from agents.sandbox.errors import ExecTimeoutError, ExecTransportError, MountConfigError from agents.sandbox.files import EntryKind -from agents.sandbox.manifest import Environment +from agents.sandbox.manifest import Environment, ProcessEnvValue from agents.sandbox.materialization import MaterializedFile from agents.sandbox.session.base_sandbox_session import ( _MKDIR_ACCESS_CHECK_SCRIPT, @@ -604,7 +605,7 @@ async def test_create_passes_only_option_env_vars_to_daytona( self, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Verify manifest env vars are not passed into Daytona's create-time env shell.""" + """Verify ordinary manifest env vars stay scoped to Daytona commands.""" daytona_module = _load_daytona_module(monkeypatch) @@ -626,6 +627,460 @@ async def test_create_passes_only_option_env_vars_to_daytona( "ONLY_OPTION": "1", } + @pytest.mark.asyncio + async def test_process_environment_uses_creation_env_not_command_text( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "daytona-protected-value" + monkeypatch.setenv(name, secret) + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + ) + + async with daytona_module.DaytonaSandboxClient( + allowed_process_environment_keys={name} + ) as client: + session = await client.create( + snapshot=_RestorableSnapshot(id="snapshot"), + manifest=manifest, + options=daytona_module.DaytonaSandboxClientOptions(), + ) + sandbox = _FakeAsyncDaytona.current_sandbox + assert sandbox is not None + await session.exec("true", shell=False) + + params, _timeout = _FakeAsyncDaytona.create_calls[0] + assert cast(Any, params).env_vars == {name: secret} + _session_id, request, _kwargs = sandbox.process.execute_session_command_calls[0] + assert secret not in cast(str, cast(Any, request).command) + + @pytest.mark.asyncio + async def test_protected_create_surfaces_failed_cleanup_identity( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "daytona-protected-value" + monkeypatch.setenv(name, secret) + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + ) + + async def fail_delete(sandbox: _FakeDaytonaSandbox) -> None: + sandbox.delete_calls += 1 + raise RuntimeError(f"cleanup failed with {secret}") + + def fail_snapshot_resolution(*_args: object, **_kwargs: object) -> object: + raise RuntimeError(f"snapshot failed with {secret}") + + monkeypatch.setattr(_FakeDaytonaSandbox, "delete", fail_delete) + monkeypatch.setattr(daytona_module, "resolve_snapshot", fail_snapshot_resolution) + + async with daytona_module.DaytonaSandboxClient( + allowed_process_environment_keys={name} + ) as client: + with pytest.raises(ValueError, match="sandbox_id='sandbox-123'") as exc_info: + await client.create( + manifest=manifest, + options=daytona_module.DaytonaSandboxClientOptions(), + ) + + assert secret not in str(exc_info.value) + assert exc_info.value.__cause__ is None + assert exc_info.value.__context__ is None + + @pytest.mark.asyncio + async def test_protected_shutdown_surfaces_current_sandbox_delete_failure( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "daytona-protected-value" + monkeypatch.setenv(name, secret) + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + sandbox = _FakeDaytonaSandbox(sandbox_id="protected-sandbox") + + async def fail_delete() -> None: + raise RuntimeError(f"cleanup failed with {secret}") + + monkeypatch.setattr(sandbox, "delete", fail_delete) + session = daytona_module.DaytonaSandboxSession.from_state( + daytona_module.DaytonaSandboxSessionState( + manifest=manifest, + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id=sandbox.id, + ), + sandbox=sandbox, + ) + + with pytest.raises(RuntimeError, match="protected process environment") as exc_info: + await session.shutdown() + + assert secret not in str(exc_info.value) + assert session.state.sandbox_id == "protected-sandbox" + + @pytest.mark.asyncio + async def test_protected_shutdown_accepts_missing_current_sandbox( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "daytona-protected-value") + + class _FakeNotFound(Exception): + pass + + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + sandbox = _FakeDaytonaSandbox(sandbox_id="protected-sandbox") + monkeypatch.setattr( + daytona_module, + "_daytona_not_found_error_types", + lambda: (_FakeNotFound,), + ) + monkeypatch.setattr(sandbox, "delete", AsyncMock(side_effect=_FakeNotFound())) + session = daytona_module.DaytonaSandboxSession.from_state( + daytona_module.DaytonaSandboxSessionState( + manifest=manifest, + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id=sandbox.id, + ), + sandbox=sandbox, + ) + + await session.shutdown() + + @pytest.mark.asyncio + async def test_resume_rebind_recreates_with_current_process_environment( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "old-value") + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + + async def persist_snapshot(_session: BaseSandboxSession) -> None: + return None + + monkeypatch.setattr( + daytona_module.DaytonaSandboxSession, + "_persist_snapshot", + persist_snapshot, + ) + + async def start_without_workspace_setup(_session: BaseSandboxSession) -> None: + return None + + monkeypatch.setattr(BaseSandboxSession, "start", start_without_workspace_setup) + + async with daytona_module.DaytonaSandboxClient( + allowed_process_environment_keys={name} + ) as client: + session = await client.create( + snapshot=_RestorableSnapshot(id="snapshot"), + manifest=manifest, + options=daytona_module.DaytonaSandboxClientOptions(), + ) + state = session.state + previous_sandbox_id = state.sandbox_id + monkeypatch.setenv(name, "current-value") + _FakeAsyncDaytona.create_calls.clear() + _FakeAsyncDaytona.get_calls.clear() + + resumed = await client.resume(state) + assert _FakeAsyncDaytona.get_calls == [] + assert _FakeAsyncDaytona.create_calls == [] + await resumed.start() + + assert _FakeAsyncDaytona.get_calls == [previous_sandbox_id] + assert len(_FakeAsyncDaytona.create_calls) == 1 + params, _timeout = _FakeAsyncDaytona.create_calls[0] + assert cast(Any, params).env_vars == {name: "current-value"} + assert resumed._inner._workspace_state_preserved_on_start() is False # noqa: SLF001 + + @pytest.mark.asyncio + async def test_resume_revalidates_process_environment_before_lookup( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + + async with daytona_module.DaytonaSandboxClient( + allowed_process_environment_keys={name} + ) as client: + session = await client.create( + snapshot=_RestorableSnapshot(id="snapshot"), + manifest=manifest, + options=daytona_module.DaytonaSandboxClientOptions(), + ) + _FakeAsyncDaytona.get_calls.clear() + resumed = await client.resume(session.state) + monkeypatch.delenv(name) + + with pytest.raises(ValueError, match="is not set"): + await resumed.start() + + assert _FakeAsyncDaytona.get_calls == [] + + @pytest.mark.asyncio + async def test_resume_rejects_noop_snapshot_before_lookup( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + state = daytona_module.DaytonaSandboxSessionState( + manifest=manifest, + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id="existing-sandbox", + ) + + async with daytona_module.DaytonaSandboxClient( + allowed_process_environment_keys={name} + ) as client: + resumed = await client.resume(state) + + with pytest.raises(ValueError, match="restorable snapshot"): + await resumed.start() + + assert _FakeAsyncDaytona.get_calls == [] + assert _FakeAsyncDaytona.create_calls == [] + + @pytest.mark.asyncio + async def test_started_replacement_restarts_before_retrying_retirement( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + session = daytona_module.DaytonaSandboxSession.from_state( + daytona_module.DaytonaSandboxSessionState( + manifest=manifest, + snapshot=_RestorableSnapshot(id="snapshot"), + sandbox_id="replacement", + ), + sandbox=_FakeDaytonaSandbox(sandbox_id="replacement"), + ) + session._process_environment_resume_started = True # noqa: SLF001 + session._process_environment_resume_previous_sandbox_id = "previous" # noqa: SLF001 + running = AsyncMock(return_value=False) + retirement = AsyncMock() + base_start = AsyncMock() + monkeypatch.setattr(session, "running", running) + monkeypatch.setattr( + session, + "_retire_process_environment_previous_sandbox", + retirement, + ) + monkeypatch.setattr(BaseSandboxSession, "start", base_start) + + await session.start() + + running.assert_awaited_once() + base_start.assert_awaited_once() + retirement.assert_awaited_once() + assert session._workspace_state_preserved_on_start() is True # noqa: SLF001 + assert session._system_state_preserved_on_start() is True # noqa: SLF001 + + @pytest.mark.asyncio + async def test_failed_replacement_cleanup_retains_candidate_for_retry( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + candidate = _FakeDaytonaSandbox(sandbox_id="candidate") + delete_calls = 0 + + async def delete_candidate() -> None: + nonlocal delete_calls + delete_calls += 1 + if delete_calls == 1: + raise RuntimeError("candidate cleanup failed") + + monkeypatch.setattr(candidate, "delete", delete_candidate) + session = daytona_module.DaytonaSandboxSession.from_state( + daytona_module.DaytonaSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id="existing", + ), + sandbox=_FakeDaytonaSandbox(sandbox_id="existing"), + ) + session._process_environment_failed_candidate_sandbox = candidate # noqa: SLF001 + session._process_environment_failed_candidate_sandbox_id = "candidate" # noqa: SLF001 + + with pytest.raises(RuntimeError, match="candidate cleanup failed"): + await session._cleanup_process_environment_failed_candidate() # noqa: SLF001 + + assert session._process_environment_failed_candidate_sandbox is candidate # noqa: SLF001 + assert session._process_environment_failed_candidate_sandbox_id == "candidate" # noqa: SLF001 + + await session._cleanup_process_environment_failed_candidate() # noqa: SLF001 + + assert delete_calls == 2 + assert session._process_environment_failed_candidate_sandbox is None # noqa: SLF001 + assert session._process_environment_failed_candidate_sandbox_id is None # noqa: SLF001 + + @pytest.mark.asyncio + async def test_failed_replacement_cleanup_clears_missing_candidate( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + + class _FakeNotFound(Exception): + pass + + candidate = _FakeDaytonaSandbox(sandbox_id="candidate") + monkeypatch.setattr( + daytona_module, + "_daytona_not_found_error_types", + lambda: (_FakeNotFound,), + ) + monkeypatch.setattr(candidate, "delete", AsyncMock(side_effect=_FakeNotFound())) + session = daytona_module.DaytonaSandboxSession.from_state( + daytona_module.DaytonaSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id="existing", + ), + sandbox=_FakeDaytonaSandbox(sandbox_id="existing"), + ) + session._process_environment_failed_candidate_sandbox = candidate # noqa: SLF001 + session._process_environment_failed_candidate_sandbox_id = "candidate" # noqa: SLF001 + + await session._cleanup_process_environment_failed_candidate() # noqa: SLF001 + + assert session._process_environment_failed_candidate_sandbox is None # noqa: SLF001 + assert session._process_environment_failed_candidate_sandbox_id is None # noqa: SLF001 + + @pytest.mark.asyncio + async def test_previous_retirement_clears_missing_sandbox( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + + class _FakeNotFound(Exception): + pass + + previous = _FakeDaytonaSandbox(sandbox_id="previous") + monkeypatch.setattr( + daytona_module, + "_daytona_not_found_error_types", + lambda: (_FakeNotFound,), + ) + monkeypatch.setattr(previous, "delete", AsyncMock(side_effect=_FakeNotFound())) + session = daytona_module.DaytonaSandboxSession.from_state( + daytona_module.DaytonaSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id="replacement", + ), + sandbox=_FakeDaytonaSandbox(sandbox_id="replacement"), + ) + session._process_environment_resume_previous_sandbox_id = "previous" # noqa: SLF001 + + await session._retire_process_environment_previous_sandbox( # noqa: SLF001 + previous_sandbox=previous + ) + + assert session._process_environment_resume_previous_sandbox_id is None # noqa: SLF001 + + @pytest.mark.asyncio + async def test_shutdown_clears_missing_deferred_sandbox( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + + class _FakeNotFound(Exception): + pass + + async def load_missing(_sandbox_id: str) -> _FakeDaytonaSandbox: + raise _FakeNotFound() + + monkeypatch.setattr( + daytona_module, + "_daytona_not_found_error_types", + lambda: (_FakeNotFound,), + ) + session = daytona_module.DaytonaSandboxSession.from_state( + daytona_module.DaytonaSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id="previous", + ), + sandbox=_FakeDaytonaSandbox(sandbox_id="previous"), + ) + session._sandbox = None # noqa: SLF001 + session._process_environment_resume_previous_sandbox_id = "previous" # noqa: SLF001 + session._process_environment_resume_previous_sandbox_loader = load_missing # noqa: SLF001 + + await session._shutdown_backend() # noqa: SLF001 + + assert session._process_environment_resume_previous_sandbox_id is None # noqa: SLF001 + + @pytest.mark.asyncio + async def test_shutdown_surfaces_deferred_sandbox_loader_failure( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + + async def fail_load(_sandbox_id: str) -> _FakeDaytonaSandbox: + raise RuntimeError("loader failed") + + session = daytona_module.DaytonaSandboxSession.from_state( + daytona_module.DaytonaSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id="previous", + ), + sandbox=_FakeDaytonaSandbox(sandbox_id="previous"), + ) + session._sandbox = None # noqa: SLF001 + session._process_environment_resume_previous_sandbox_id = "previous" # noqa: SLF001 + session._process_environment_resume_previous_sandbox_loader = fail_load # noqa: SLF001 + + with pytest.raises(RuntimeError, match="loader failed"): + await session._shutdown_backend() # noqa: SLF001 + + assert session._process_environment_resume_previous_sandbox_id == "previous" # noqa: SLF001 + @pytest.mark.asyncio async def test_exec_enforces_subsecond_caller_timeout( self, @@ -1125,6 +1580,39 @@ async def test_persist_workspace_remounts_mounts_after_snapshot( assert mount._unmounted_paths == [mount_path] assert mount._mounted_paths == [mount_path] + @pytest.mark.asyncio + async def test_tar_hydrate_resolves_environment_once_before_workspace_mutation( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + sandbox = _FakeDaytonaSandbox() + state = daytona_module.DaytonaSandboxSessionState( + manifest=Manifest(root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT), + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id=sandbox.id, + ) + session = daytona_module.DaytonaSandboxSession.from_state(state, sandbox=sandbox) + resolve_calls = 0 + mkdir_calls_during_resolution: list[int] = [] + + async def resolve_once() -> dict[str, str]: + nonlocal resolve_calls + resolve_calls += 1 + mkdir_calls_during_resolution.append(len(sandbox.fs.create_folder_calls)) + return {"TOKEN": "snapshot-value"} + + payload = io.BytesIO() + with tarfile.open(fileobj=payload, mode="w"): + pass + payload.seek(0) + monkeypatch.setattr(session, "_resolved_envs", resolve_once) + + await session.hydrate_workspace(payload) + + assert resolve_calls == 1 + assert mkdir_calls_during_resolution == [0] + @pytest.mark.asyncio async def test_persist_workspace_marks_stopped_sandbox_non_retryable( self, @@ -1337,6 +1825,111 @@ async def test_pty_start_write_and_exit(self, monkeypatch: pytest.MonkeyPatch) - assert finished.process_id is None assert finished.exit_code == 0 + @pytest.mark.asyncio + async def test_pty_uses_out_of_band_process_environment( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "daytona-protected-value" + monkeypatch.setenv(name, secret) + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + + async with daytona_module.DaytonaSandboxClient( + allowed_process_environment_keys={name} + ) as client: + session = await client.create( + manifest=manifest, + options=daytona_module.DaytonaSandboxClientOptions(), + ) + sandbox = _FakeAsyncDaytona.current_sandbox + assert sandbox is not None + await session.pty_exec_start("python3", shell=False, tty=True, yield_time_s=0.05) + + params, _timeout = _FakeAsyncDaytona.create_calls[0] + assert cast(Any, params).env_vars == {name: secret} + assert sandbox.process.create_pty_session_calls[0]["envs"] is None + await session.pty_terminate_all() + + @pytest.mark.asyncio + async def test_pty_failure_redacts_process_environment_values( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + sandbox = _FakeDaytonaSandbox() + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "daytona-pty-secret" + monkeypatch.setenv(name, secret) + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + state = daytona_module.DaytonaSandboxSessionState( + manifest=manifest, + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id=sandbox.id, + ) + session = daytona_module.DaytonaSandboxSession.from_state(state, sandbox=sandbox) + provider_error = FileNotFoundError(f"provider failed with {secret}") + sandbox.process.create_pty_session_error = provider_error + + with pytest.raises( + ExecTransportError, + match="protected process environment values", + ) as exc_info: + await session.pty_exec_start("python3", shell=False, tty=True) + + assert secret not in str(exc_info.value) + assert exc_info.value.__cause__ is None + assert exc_info.value.__context__ is None + assert cast(Any, BaseException.args).__get__(provider_error, type(provider_error)) == () + assert provider_error.__traceback__ is None + + @pytest.mark.asyncio + async def test_pty_terminate_failure_redacts_process_environment_values( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + sandbox = _FakeDaytonaSandbox() + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "daytona-pty-terminate-secret" + monkeypatch.setenv(name, secret) + manifest = Manifest( + root=daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + state = daytona_module.DaytonaSandboxSessionState( + manifest=manifest, + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id=sandbox.id, + ) + session = daytona_module.DaytonaSandboxSession.from_state(state, sandbox=sandbox) + provider_error = RuntimeError(f"provider failed with {secret}") + + async def fail_terminate(_entry: object) -> None: + raise provider_error + + monkeypatch.setattr(session, "_terminate_pty_entry", fail_terminate) + session._pty_sessions[1] = cast(Any, object()) # noqa: SLF001 + + with pytest.raises( + RuntimeError, + match="protected process environment values", + ) as exc_info: + await session.pty_terminate_all() + + assert secret not in str(exc_info.value) + assert exc_info.value.__cause__ is None + assert exc_info.value.__context__ is None + assert cast(Any, BaseException.args).__get__(provider_error, type(provider_error)) == () + assert provider_error.__traceback__ is None + @pytest.mark.asyncio async def test_stop_terminates_live_pty_sessions(self, monkeypatch: pytest.MonkeyPatch) -> None: daytona_module = _load_daytona_module(monkeypatch) diff --git a/tests/sandbox/test_compatibility_guards.py b/tests/sandbox/test_compatibility_guards.py index 0e3a2bcea4..a26ac94aa5 100644 --- a/tests/sandbox/test_compatibility_guards.py +++ b/tests/sandbox/test_compatibility_guards.py @@ -40,7 +40,7 @@ RcloneMountPattern, S3FilesMountPattern, ) -from agents.sandbox.manifest import EnvValue, StrEnvValue +from agents.sandbox.manifest import EnvValue, ProcessEnvValue, StrEnvValue from agents.sandbox.session.sandbox_client import BaseSandboxClientOptions from agents.sandbox.session.sandbox_session_state import SandboxSessionState from agents.sandbox.snapshot import LocalSnapshot, NoopSnapshot, RemoteSnapshot, SnapshotBase @@ -894,6 +894,7 @@ def test_core_discriminator_type_strings_are_stable() -> None: InContainerMountStrategy: "in_container", DockerVolumeMountStrategy: "docker_volume", StrEnvValue: "str", + ProcessEnvValue: "process_env", } for cls, expected_type in expected_types.items(): diff --git a/tests/sandbox/test_docker.py b/tests/sandbox/test_docker.py index 366000d113..b06085e857 100644 --- a/tests/sandbox/test_docker.py +++ b/tests/sandbox/test_docker.py @@ -13,7 +13,7 @@ import uuid from collections.abc import Callable, Iterator from pathlib import Path -from typing import cast +from typing import Literal, cast import docker.errors # type: ignore[import-untyped] import pytest @@ -53,7 +53,7 @@ WorkspaceReadNotFoundError, ) from agents.sandbox.files import EntryKind, FileEntry -from agents.sandbox.manifest import Manifest +from agents.sandbox.manifest import Environment, Manifest, ProcessEnvValue from agents.sandbox.materialization import MaterializedFile from agents.sandbox.sandboxes.docker import ( DockerSandboxClient, @@ -62,11 +62,27 @@ DockerSandboxSessionState, ) from agents.sandbox.session.base_sandbox_session import BaseSandboxSession +from agents.sandbox.session.dependencies import Dependencies from agents.sandbox.session.runtime_helpers import RESOLVE_WORKSPACE_PATH_HELPER -from agents.sandbox.snapshot import NoopSnapshot +from agents.sandbox.snapshot import NoopSnapshot, SnapshotBase from agents.sandbox.types import ExecResult, Permissions +class _RestorableSnapshot(SnapshotBase): + type: Literal["test-restorable-docker"] = "test-restorable-docker" + + async def persist(self, data: io.IOBase, *, dependencies: Dependencies | None = None) -> None: + _ = (data, dependencies) + + async def restore(self, *, dependencies: Dependencies | None = None) -> io.IOBase: + _ = dependencies + return io.BytesIO(b"") + + async def restorable(self, *, dependencies: Dependencies | None = None) -> bool: + _ = dependencies + return True + + class _FakeDockerContainer: def __init__(self, host_root: Path, *, archive_error: Exception | None = None) -> None: self._host_root = host_root @@ -1800,6 +1816,498 @@ def _missing_image(_image: str) -> bool: assert docker_client.images.calls == [("localhost:5000/myimg", "latest", False)] +@pytest.mark.asyncio +async def test_docker_resolves_process_environment_before_image_operations( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "available-when-granted") + manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + monkeypatch.delenv(name) + client = DockerSandboxClient(docker_client=cast(object, _FakeDockerClient())) + + def _unexpected_image_lookup(_image: str) -> bool: + raise AssertionError("image lookup must not start before environment resolution") + + monkeypatch.setattr(client, "image_exists", _unexpected_image_lookup) + + with pytest.raises(ValueError, match=f"variable {name!r} is not set"): + await client._create_container(DEFAULT_PYTHON_SANDBOX_IMAGE, manifest=manifest) + + +@pytest.mark.asyncio +async def test_docker_client_binds_renamed_process_environment_reference( + monkeypatch: pytest.MonkeyPatch, +) -> None: + source_name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(source_name, "current-value") + manifest = Manifest(environment=Environment(value={"TOKEN": ProcessEnvValue(name=source_name)})) + client = DockerSandboxClient( + docker_client=cast(object, _FakeDockerClient()), + process_environment_bindings={"TOKEN": source_name}, + ) + + trusted_manifest = client._validate_manifest_for_create(manifest) # noqa: SLF001 + + assert await trusted_manifest.resolve_environment() == {"TOKEN": "current-value"} + with pytest.raises(ValueError, match="configure the sandbox client"): + await manifest.resolve_environment() + + +@pytest.mark.parametrize("destination", ["INVALID=DEST", "INVALID\x00DEST"]) +@pytest.mark.asyncio +async def test_docker_rejects_invalid_process_environment_destination_before_image_operations( + destination: str, + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "secret") + client = DockerSandboxClient(docker_client=cast(object, _FakeDockerClient())) + + def _unexpected_image_lookup(_image: str) -> bool: + raise AssertionError("image lookup must not start before destination validation") + + monkeypatch.setattr(client, "image_exists", _unexpected_image_lookup) + + with pytest.raises(ValueError, match="must not contain '=' or NUL"): + Manifest( + environment=Environment(value={destination: ProcessEnvValue(name=name)}) + )._with_process_environment_access((destination, name)) + + +@pytest.mark.asyncio +async def test_docker_resume_rebind_recreates_with_current_process_environment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest(environment=Environment(value={name: ProcessEnvValue()})) + state = DockerSandboxSessionState( + manifest=manifest, + snapshot=_RestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="existing-container", + ) + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + existing = _ResumeContainer(status="running", container_id="existing-container") + replacement = _ResumeContainer(status="created", container_id="replacement") + existing.remove = lambda **_kwargs: None + replacement.start = lambda: None + + reconnect_calls: list[str] = [] + + def reconnect(container_id: str) -> object: + reconnect_calls.append(container_id) + return existing + + async def create_container(*args: object, **kwargs: object) -> _ResumeContainer: + _ = args + assert await cast(Manifest, kwargs["manifest"]).resolve_environment() == { + name: "current-value" + } + return replacement + + monkeypatch.setattr(client, "get_container", reconnect) + monkeypatch.setattr(client, "_create_container", create_container) + + async def persist_snapshot(_session: BaseSandboxSession) -> None: + return None + + monkeypatch.setattr(DockerSandboxSession, "_persist_snapshot", persist_snapshot) + + async def start_without_workspace_setup(_session: BaseSandboxSession) -> None: + return None + + monkeypatch.setattr(BaseSandboxSession, "start", start_without_workspace_setup) + + resumed = await client.resume(state) + assert reconnect_calls == [] + assert resumed.state.container_id == "existing-container" + await resumed.start() + + assert reconnect_calls == ["existing-container"] + assert resumed.state.container_id == "replacement" + assert resumed._inner._workspace_state_preserved_on_start() is False # noqa: SLF001 + + +@pytest.mark.asyncio +async def test_docker_resume_revalidates_process_environment_before_lookup( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + state = DockerSandboxSessionState( + manifest=manifest, + snapshot=_RestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="existing-container", + ) + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + reconnect_calls: list[str] = [] + + def reconnect(container_id: str) -> object: + reconnect_calls.append(container_id) + return _ResumeContainer(status="running", container_id=container_id) + + monkeypatch.setattr(client, "get_container", reconnect) + resumed = await client.resume(state) + monkeypatch.delenv(name) + + with pytest.raises(ValueError, match="is not set"): + await resumed.start() + + assert reconnect_calls == [] + + +@pytest.mark.asyncio +async def test_docker_resume_rejects_noop_snapshot_before_lookup( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + state = DockerSandboxSessionState( + manifest=manifest, + snapshot=NoopSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="existing-container", + ) + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + reconnect_calls: list[str] = [] + + def reconnect(container_id: str) -> object: + reconnect_calls.append(container_id) + return _ResumeContainer(status="running", container_id=container_id) + + monkeypatch.setattr(client, "get_container", reconnect) + resumed = await client.resume(state) + + with pytest.raises(ValueError, match="restorable snapshot"): + await resumed.start() + + assert reconnect_calls == [] + + +@pytest.mark.asyncio +async def test_docker_retirement_failure_keeps_started_replacement( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + state = DockerSandboxSessionState( + manifest=manifest, + snapshot=_RestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="existing-container", + ) + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + existing = _ResumeContainer(status="running", container_id="existing-container") + replacement = _ResumeContainer(status="created", container_id="replacement") + + def fail_retirement(**_kwargs: object) -> None: + raise RuntimeError("retire") + + existing.remove = fail_retirement + replacement.start = lambda: None + + monkeypatch.setattr(client, "get_container", lambda _container_id: existing) + + async def create_container(*_args: object, **_kwargs: object) -> _ResumeContainer: + return replacement + + monkeypatch.setattr(client, "_create_container", create_container) + + async def persist_snapshot(_session: BaseSandboxSession) -> None: + return None + + monkeypatch.setattr(DockerSandboxSession, "_persist_snapshot", persist_snapshot) + + async def start_without_workspace_setup(_session: BaseSandboxSession) -> None: + return None + + monkeypatch.setattr(BaseSandboxSession, "start", start_without_workspace_setup) + + resumed = await client.resume(state) + with pytest.raises(RuntimeError, match="protected process environment"): + await resumed.start() + + assert resumed.state.container_id == "replacement" + assert resumed._inner._container is replacement # noqa: SLF001 + assert resumed._inner._process_environment_resume_start is None # noqa: SLF001 + + +def test_docker_failed_replacement_cleanup_retains_candidate_for_retry() -> None: + state = DockerSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="existing-container", + ) + current = _ResumeContainer(status="running", container_id="existing-container") + candidate = _ResumeContainer(status="created", container_id="candidate") + remove_calls = 0 + + def remove_candidate(**_kwargs: object) -> None: + nonlocal remove_calls + remove_calls += 1 + if remove_calls == 1: + raise RuntimeError("candidate cleanup failed") + + candidate.remove = remove_candidate + client = DockerSandboxClient(docker_client=_ResumeDockerClient(current)) + session = DockerSandboxSession.from_state( + state, + container=current, + docker_client=client.docker_client, + ) + session._process_environment_failed_candidate_container = candidate # noqa: SLF001 + session._process_environment_failed_candidate_container_id = "candidate" # noqa: SLF001 + + with pytest.raises(RuntimeError, match="candidate cleanup failed"): + session._cleanup_process_environment_failed_candidate() # noqa: SLF001 + + assert session._process_environment_failed_candidate_container is candidate # noqa: SLF001 + assert session._process_environment_failed_candidate_container_id == "candidate" # noqa: SLF001 + + session._cleanup_process_environment_failed_candidate() # noqa: SLF001 + + assert remove_calls == 2 + assert session._process_environment_failed_candidate_container is None # noqa: SLF001 + assert session._process_environment_failed_candidate_container_id is None # noqa: SLF001 + + +def test_docker_failed_replacement_cleanup_clears_missing_candidate() -> None: + state = DockerSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="existing-container", + ) + current = _ResumeContainer(status="running", container_id="existing-container") + candidate = _ResumeContainer(status="created", container_id="candidate") + + def remove_missing_candidate(**_kwargs: object) -> None: + raise docker.errors.NotFound("candidate missing") + + candidate.remove = remove_missing_candidate + client = DockerSandboxClient(docker_client=_ResumeDockerClient(current)) + session = DockerSandboxSession.from_state( + state, + container=current, + docker_client=client.docker_client, + ) + session._process_environment_failed_candidate_container = candidate # noqa: SLF001 + session._process_environment_failed_candidate_container_id = "candidate" # noqa: SLF001 + + session._cleanup_process_environment_failed_candidate() # noqa: SLF001 + + assert session._process_environment_failed_candidate_container is None # noqa: SLF001 + assert session._process_environment_failed_candidate_container_id is None # noqa: SLF001 + + +def test_docker_previous_retirement_clears_missing_container() -> None: + state = DockerSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="replacement", + ) + current = _ResumeContainer(status="running", container_id="replacement") + previous = _ResumeContainer(status="running", container_id="previous") + + def remove_missing_previous(**_kwargs: object) -> None: + raise docker.errors.NotFound("previous missing") + + previous.remove = remove_missing_previous + client = DockerSandboxClient(docker_client=_ResumeDockerClient(current)) + session = DockerSandboxSession.from_state( + state, + container=current, + docker_client=client.docker_client, + ) + session._process_environment_resume_previous_container_id = "previous" # noqa: SLF001 + + session._retire_process_environment_previous_resources( # noqa: SLF001 + previous_container=previous + ) + + assert session._process_environment_resume_previous_container_id is None # noqa: SLF001 + + +@pytest.mark.asyncio +async def test_docker_shutdown_clears_missing_deferred_container() -> None: + state = DockerSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="previous", + ) + current = _ResumeContainer(status="running", container_id="previous") + client = DockerSandboxClient(docker_client=_ResumeDockerClient(current)) + session = DockerSandboxSession.from_state( + state, + container=current, + docker_client=client.docker_client, + ) + session._container = None # noqa: SLF001 + session._process_environment_resume_previous_container_id = "previous" # noqa: SLF001 + + def load_missing(_container_id: str) -> _ResumeContainer: + raise docker.errors.NotFound("previous missing") + + session._process_environment_resume_previous_container_loader = load_missing # noqa: SLF001 + + await session._shutdown_backend() # noqa: SLF001 + + assert session._process_environment_resume_previous_container_id is None # noqa: SLF001 + + +@pytest.mark.asyncio +async def test_docker_shutdown_surfaces_deferred_container_loader_failure() -> None: + state = DockerSandboxSessionState( + manifest=Manifest(), + snapshot=NoopSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="previous", + ) + current = _ResumeContainer(status="running", container_id="previous") + client = DockerSandboxClient(docker_client=_ResumeDockerClient(current)) + session = DockerSandboxSession.from_state( + state, + container=current, + docker_client=client.docker_client, + ) + session._container = None # noqa: SLF001 + session._process_environment_resume_previous_container_id = "previous" # noqa: SLF001 + + def fail_load(_container_id: str) -> _ResumeContainer: + raise RuntimeError("loader failed") + + session._process_environment_resume_previous_container_loader = fail_load # noqa: SLF001 + + with pytest.raises(RuntimeError, match="loader failed"): + await session._shutdown_backend() # noqa: SLF001 + + assert session._process_environment_resume_previous_container_id == "previous" # noqa: SLF001 + + +@pytest.mark.asyncio +async def test_docker_protected_create_surfaces_failed_cleanup_identity( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest(environment=Environment(value={name: ProcessEnvValue()})) + + class _FailingCleanupStartedContainer(_StartedContainer): + id = "protected-container" + + def remove(self, **kwargs: object) -> None: + super().remove(**kwargs) + raise RuntimeError("cleanup failed with current-value") + + container = _FailingCleanupStartedContainer() + docker_client = _DeleteDockerClient(container=container, volumes={}) + client = DockerSandboxClient( + docker_client=cast(object, docker_client), + allowed_process_environment_keys={name}, + ) + + async def create_container(*_args: object, **_kwargs: object) -> _StartedContainer: + return container + + def fail_snapshot_resolution(*_args: object, **_kwargs: object) -> object: + raise RuntimeError("snapshot failed with current-value") + + monkeypatch.setattr(client, "_create_container", create_container) + monkeypatch.setattr(docker_sandbox, "resolve_snapshot", fail_snapshot_resolution) + + with pytest.raises(ValueError, match="container_id='protected-container'") as exc_info: + await client.create( + manifest=manifest, + options=DockerSandboxClientOptions(image=DEFAULT_PYTHON_SANDBOX_IMAGE), + ) + + assert "current-value" not in str(exc_info.value) + assert exc_info.value.__cause__ is None + assert exc_info.value.__context__ is None + + +@pytest.mark.asyncio +async def test_docker_concurrent_resume_start_runs_replacement_once( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + state = DockerSandboxSessionState( + manifest=manifest, + snapshot=_RestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="existing-container", + ) + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + existing = _ResumeContainer(status="running", container_id="existing-container") + replacement = _ResumeContainer(status="created", container_id="replacement") + existing.remove = lambda **_kwargs: None + replacement.start = lambda: None + monkeypatch.setattr(client, "get_container", lambda _container_id: existing) + create_calls = 0 + + async def create_container(*_args: object, **_kwargs: object) -> _ResumeContainer: + nonlocal create_calls + create_calls += 1 + return replacement + + monkeypatch.setattr(client, "_create_container", create_container) + + async def persist_snapshot(_session: BaseSandboxSession) -> None: + return None + + monkeypatch.setattr(DockerSandboxSession, "_persist_snapshot", persist_snapshot) + shared_start_calls = 0 + + async def start_without_workspace_setup(_session: BaseSandboxSession) -> None: + nonlocal shared_start_calls + shared_start_calls += 1 + replacement.status = "running" + + monkeypatch.setattr(BaseSandboxSession, "start", start_without_workspace_setup) + + resumed = await client.resume(state) + await asyncio.gather(resumed.start(), resumed.start()) + + assert create_calls == 1 + assert shared_start_calls == 1 + + @pytest.mark.asyncio async def test_docker_create_container_publishes_exposed_ports( monkeypatch: pytest.MonkeyPatch, diff --git a/tests/sandbox/test_manifest.py b/tests/sandbox/test_manifest.py index 0f5eef6bc1..a206e4b289 100644 --- a/tests/sandbox/test_manifest.py +++ b/tests/sandbox/test_manifest.py @@ -1,6 +1,7 @@ import asyncio import contextlib import json +import pickle from pathlib import Path from typing import ClassVar, Literal @@ -16,7 +17,15 @@ MountpointMountPattern, ) from agents.sandbox.errors import InvalidManifestPathError -from agents.sandbox.manifest import EnvEntry, Environment, EnvValue, Manifest, StrEnvValue +from agents.sandbox.manifest import ( + EnvEntry, + Environment, + EnvValue, + Manifest, + ProcessEnvValue, + StrEnvValue, + _normalize_process_environment_bindings, +) from agents.sandbox.manifest_render import _truncate_manifest_description @@ -41,6 +50,36 @@ def _serialize_reference(self) -> dict[str, str]: return {"key": self.key} +class _NonCopyableClient: + def __deepcopy__(self, _memo: object) -> "_NonCopyableClient": + raise RuntimeError("client must not be copied") + + +class _ClientBackedEnvValue(EnvValue): + type: Literal["test.client_backed"] = "test.client_backed" + client: object + + async def resolve(self) -> str: + return "resolved" + + +@pytest.mark.asyncio +async def test_manifest_pickle_revokes_process_environment_authority( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "pickle-secret") + trusted = Manifest( + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + + restored = pickle.loads(pickle.dumps(trusted)) + + assert restored._process_environment_access == frozenset() + with pytest.raises(ValueError, match=f"binding {name!r} -> {name!r} is not granted"): + await restored.resolve_environment() + + def test_manifest_rejects_nested_child_paths_that_escape_workspace() -> None: manifest = Manifest( entries={ @@ -340,6 +379,379 @@ def test_manifest_round_trips_str_env_value() -> None: } +@pytest.mark.asyncio +async def test_manifest_resolves_same_name_and_renamed_process_environment_values( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("SANDBOX_TEST_SAME_NAME", "same-value") + monkeypatch.setenv("SANDBOX_TEST_SOURCE_NAME", "renamed-value") + manifest = Manifest( + environment=Environment( + value={ + "SANDBOX_TEST_SAME_NAME": ProcessEnvValue(), + "SANDBOX_TEST_DESTINATION": ProcessEnvValue(name="SANDBOX_TEST_SOURCE_NAME"), + "PLAIN": "literal", + } + ) + )._with_process_environment_access( + "SANDBOX_TEST_SAME_NAME", + ("SANDBOX_TEST_DESTINATION", "SANDBOX_TEST_SOURCE_NAME"), + ) + + assert await manifest.resolve_environment() == { + "SANDBOX_TEST_SAME_NAME": "same-value", + "SANDBOX_TEST_DESTINATION": "renamed-value", + "PLAIN": "literal", + } + + monkeypatch.setenv("SANDBOX_TEST_SAME_NAME", "rotated-value") + assert (await manifest.resolve_environment())["SANDBOX_TEST_SAME_NAME"] == "rotated-value" + + +@pytest.mark.asyncio +async def test_process_environment_access_distinguishes_missing_and_empty_values( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + monkeypatch.delenv(name, raising=False) + + with pytest.raises(ValueError, match=f"variable {name!r} is not set"): + await manifest.resolve_environment() + + monkeypatch.setenv(name, "") + + assert await manifest.resolve_environment() == {name: ""} + + +@pytest.mark.asyncio +async def test_non_process_environment_resolution_does_not_read_process_values( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "creation-only-value") + manifest = Manifest( + environment=Environment( + value={ + name: ProcessEnvValue(), + "LITERAL": "literal-value", + } + ) + )._with_process_environment_access(name) + monkeypatch.delenv(name) + + assert await manifest._resolve_environment_without_process_values() == { + "LITERAL": "literal-value" + } + monkeypatch.setenv(name, "current-value") + assert await manifest._resolve_process_environment_values() == {name: "current-value"} + + +@pytest.mark.asyncio +async def test_process_environment_access_is_runtime_only( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "must-not-be-serialized" + monkeypatch.setenv(name, secret) + untrusted = Manifest(environment=Environment(value={"TOKEN": ProcessEnvValue(name=name)})) + manifest = untrusted._with_process_environment_access(("TOKEN", name)) + + payload = manifest.model_dump(mode="json") + serialized = json.dumps(payload) + restored = Manifest.model_validate(payload) + + assert payload["environment"] == {"value": {"TOKEN": {"type": "process_env", "name": name}}} + assert secret not in serialized + assert "process_environment_access" not in serialized + assert type(restored.environment.value["TOKEN"]) is ProcessEnvValue + + with pytest.raises(ValueError, match=f"binding {name!r} -> 'TOKEN' is not granted"): + await untrusted.resolve_environment() + with pytest.raises(ValueError, match=f"binding {name!r} -> 'TOKEN' is not granted"): + await restored.resolve_environment() + + rebound = restored._with_process_environment_access(("TOKEN", name)) + assert await rebound.resolve_environment() == {"TOKEN": secret} + + +@pytest.mark.asyncio +async def test_process_environment_reference_requires_manifest_authority( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "from-process") + value = ProcessEnvValue(name=name) + environment = Environment(value={"TOKEN": value}) + + with pytest.raises(ValueError, match="must be resolved through a trusted Manifest"): + await value.resolve() + with pytest.raises(ValueError, match=f"binding {name!r} -> 'TOKEN' is not granted"): + await environment.resolve() + + +@pytest.mark.asyncio +async def test_process_environment_access_is_bound_to_the_sandbox_name( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "secret") + trusted = Manifest( + environment=Environment(value={"TOKEN": ProcessEnvValue(name=name)}) + )._with_process_environment_access(("TOKEN", name)) + tampered = trusted.model_copy( + update={ + "environment": Environment(value={"EXFIL": ProcessEnvValue(name=name)}), + }, + deep=True, + ) + + with pytest.raises(ValueError, match=f"binding {name!r} -> 'EXFIL' is not granted"): + await tampered.resolve_environment() + + +def test_process_environment_client_config_rejects_conflicting_destinations() -> None: + with pytest.raises(ValueError, match="conflicting bindings"): + _normalize_process_environment_bindings( + allowed_process_environment_keys={"TOKEN"}, + process_environment_bindings={"TOKEN": "PROD_TOKEN"}, + ) + + +def test_process_environment_access_does_not_copy_unrelated_resolvers() -> None: + resolver = _ClientBackedEnvValue(client=_NonCopyableClient()) + manifest = Manifest( + environment=Environment( + value={ + "TOKEN": ProcessEnvValue(), + "CUSTOM": resolver, + } + ) + ) + + trusted = manifest._with_process_environment_access("TOKEN") + + assert trusted is not manifest + assert trusted.environment.value["CUSTOM"] is resolver + + +def test_process_environment_client_rebind_replaces_prior_runtime_authority() -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + privileged = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + + rebound = privileged._with_process_environment_access(frozenset()) + + assert rebound._process_environment_access == frozenset() + + +@pytest.mark.asyncio +async def test_process_environment_access_grants_only_requested_destination_for_shared_source( + monkeypatch: pytest.MonkeyPatch, +) -> None: + source_name = "SANDBOX_TEST_SHARED_PROCESS_ENV_VALUE" + monkeypatch.setenv(source_name, "secret") + manifest = Manifest( + environment=Environment( + value={ + "TOKEN": ProcessEnvValue(name=source_name), + "EXFIL": ProcessEnvValue(name=source_name), + } + ) + ) + + trusted = manifest._with_process_environment_access(("TOKEN", source_name)) + + assert trusted._process_environment_access == frozenset({("TOKEN", source_name)}) + with pytest.raises(ValueError, match=f"binding {source_name!r} -> 'EXFIL' is not granted"): + await trusted.resolve_environment() + + +@pytest.mark.asyncio +async def test_process_environment_bindings_are_validated_before_custom_resolvers_start( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "secret") + resolver_started = False + + class RecordingEnvValue(EnvValue): + type: Literal["test.recording_process_env_sibling"] = "test.recording_process_env_sibling" + + async def resolve(self) -> str: + nonlocal resolver_started + resolver_started = True + return "resolved" + + manifest = Manifest( + environment=Environment( + value={ + "CUSTOM": RecordingEnvValue(), + "TOKEN": ProcessEnvValue(name=name), + } + ) + ) + + with pytest.raises(ValueError, match=f"binding {name!r} -> 'TOKEN' is not granted"): + await manifest.resolve_environment() + + assert resolver_started is False + + +@pytest.mark.asyncio +async def test_process_environment_destination_names_are_validated_before_resolution( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "secret") + resolver_started = False + + class RecordingEnvValue(EnvValue): + type: Literal["test.recording_invalid_destination_sibling"] = ( + "test.recording_invalid_destination_sibling" + ) + + async def resolve(self) -> str: + nonlocal resolver_started + resolver_started = True + return "resolved" + + for destination in ("INVALID=DEST", "INVALID\x00DEST"): + with pytest.raises(ValueError, match="must not contain '=' or NUL"): + Manifest( + environment=Environment( + value={ + destination: ProcessEnvValue(name=name), + "CUSTOM": RecordingEnvValue(), + } + ) + )._with_process_environment_access((destination, name)) + + assert resolver_started is False + + +def _manifest_traceback_locals(error: BaseException) -> str: + frames: list[dict[str, object]] = [] + traceback = error.__traceback__ + while traceback is not None: + frame = traceback.tb_frame + if frame.f_code.co_filename.endswith("/agents/sandbox/manifest.py"): + frames.append(dict(frame.f_locals)) + traceback = traceback.tb_next + return repr(frames) + + +@pytest.mark.asyncio +async def test_custom_resolver_failure_does_not_retain_process_values_in_traceback( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "process-secret-must-not-reach-traceback" + monkeypatch.setenv(name, secret) + + class FailingEnvValue(EnvValue): + type: Literal["test.failing_process_env_sibling"] = "test.failing_process_env_sibling" + + async def resolve(self) -> str: + raise RuntimeError("custom resolver failed") + + manifest = Manifest( + environment=Environment( + value={ + "TOKEN": ProcessEnvValue(name=name), + "CUSTOM": FailingEnvValue(), + } + ) + )._with_process_environment_access(("TOKEN", name)) + + with pytest.raises(RuntimeError, match="custom resolver failed") as exc_info: + await manifest.resolve_environment() + + assert secret not in _manifest_traceback_locals(exc_info.value) + + +@pytest.mark.asyncio +async def test_process_values_are_snapshotted_before_custom_resolvers( + monkeypatch: pytest.MonkeyPatch, +) -> None: + first_name = "SANDBOX_TEST_FIRST_PROCESS_ENV_VALUE" + second_name = "SANDBOX_TEST_SECOND_PROCESS_ENV_VALUE" + secret = "partial-process-secret-must-not-reach-traceback" + monkeypatch.setenv(first_name, secret) + monkeypatch.setenv(second_name, "removed-before-materialization") + + class RemovingEnvValue(EnvValue): + type: Literal["test.removing_process_env_sibling"] = "test.removing_process_env_sibling" + + async def resolve(self) -> str: + monkeypatch.delenv(second_name) + return "custom" + + manifest = Manifest( + environment=Environment( + value={ + "FIRST": ProcessEnvValue(name=first_name), + "SECOND": ProcessEnvValue(name=second_name), + "CUSTOM": RemovingEnvValue(), + } + ) + )._with_process_environment_access(("FIRST", first_name), ("SECOND", second_name)) + + assert await manifest.resolve_environment() == { + "FIRST": secret, + "SECOND": "removed-before-materialization", + "CUSTOM": "custom", + } + + +@pytest.mark.asyncio +async def test_process_snapshot_survives_custom_resolver_mutation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + custom_secret = "custom-secret-must-not-reach-traceback" + monkeypatch.setenv(name, "removed-before-materialization") + + class SecretRemovingEnvValue(EnvValue): + type: Literal["test.secret_removing_process_env_sibling"] = ( + "test.secret_removing_process_env_sibling" + ) + + async def resolve(self) -> str: + monkeypatch.delenv(name) + return custom_secret + + manifest = Manifest( + environment=Environment( + value={ + "TOKEN": ProcessEnvValue(name=name), + "CUSTOM": SecretRemovingEnvValue(), + } + ) + )._with_process_environment_access(("TOKEN", name)) + + assert await manifest.resolve_environment() == { + "TOKEN": "removed-before-materialization", + "CUSTOM": custom_secret, + } + + +@pytest.mark.parametrize( + "payload_key", + [ + "process_environment_access", + "_process_environment_access", + "processEnvironmentAllowedNames", + ], +) +def test_manifest_rejects_serialized_process_environment_authority(payload_key: str) -> None: + with pytest.raises(TypeError, match="trusted Manifest instance"): + Manifest.model_validate({payload_key: ["OPENAI_API_KEY"]}) + + def test_manifest_reads_legacy_discriminator_free_str_env_values() -> None: payload = { "environment": { diff --git a/tests/sandbox/test_mount_security.py b/tests/sandbox/test_mount_security.py index d576b9e467..d9fe4ff1da 100644 --- a/tests/sandbox/test_mount_security.py +++ b/tests/sandbox/test_mount_security.py @@ -68,7 +68,7 @@ PtySessionNotFoundError, SandboxError, ) -from agents.sandbox.manifest import Environment +from agents.sandbox.manifest import Environment, ProcessEnvValue from agents.sandbox.session.base_sandbox_session import BaseSandboxSession from agents.sandbox.session.sandbox_client import BaseSandboxClient from agents.sandbox.session.sandbox_session import SandboxSession @@ -3921,6 +3921,77 @@ async def fail(*, manifest: Manifest) -> None: traceback = traceback.tb_next +@pytest.mark.asyncio +async def test_mixed_authority_preserves_safe_process_environment_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_MISSING_PROCESS_ENV" + mount_secret = "mixed-authority-mount-secret" + monkeypatch.delenv(name, raising=False) + manifest = Manifest( + entries={ + "data": S3Mount( + bucket="bucket", + access_key_id="access-key", + secret_access_key=mount_secret, + mount_strategy=DockerVolumeMountStrategy(driver="rclone"), + ) + }, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + + @redact_mount_error_data + async def resolve(*, manifest: Manifest) -> None: + await manifest.resolve_environment() + + with pytest.raises(ValueError, match=f"variable {name!r} is not set") as exc_info: + await resolve(manifest=manifest) + + assert exc_info.value.__cause__ is None + assert exc_info.value.__context__ is None + traceback = exc_info.value.__traceback__ + while traceback is not None: + frame_path = Path(traceback.tb_frame.f_code.co_filename).as_posix() + if "/src/agents/" in frame_path: + assert mount_secret not in repr(traceback.tb_frame.f_locals) + traceback = traceback.tb_next + + +def test_mixed_authority_resume_validation_preserves_safe_process_environment_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_MISSING_PROCESS_ENV" + mount_secret = "mixed-authority-resume-mount-secret" + monkeypatch.delenv(name, raising=False) + manifest = Manifest( + entries={ + "data": S3Mount( + bucket="bucket", + access_key_id="access-key", + secret_access_key=mount_secret, + mount_strategy=DockerVolumeMountStrategy(driver="rclone"), + ) + }, + environment=Environment(value={name: ProcessEnvValue()}), + )._with_process_environment_access(name) + state = TestSessionState( + manifest=manifest, + snapshot=NoopSnapshot(id="snapshot"), + ).model_copy(update={"type": "docker"}) + + with pytest.raises(ValueError, match=f"variable {name!r} is not set") as exc_info: + state.assert_path_grants_rebound() + + assert exc_info.value.__cause__ is None + assert exc_info.value.__context__ is None + traceback = exc_info.value.__traceback__ + while traceback is not None: + frame_path = Path(traceback.tb_frame.f_code.co_filename).as_posix() + if "/src/agents/" in frame_path: + assert mount_secret not in repr(traceback.tb_frame.f_locals) + traceback = traceback.tb_next + + @pytest.mark.parametrize( "mount", [ diff --git a/tests/sandbox/test_runtime.py b/tests/sandbox/test_runtime.py index fcef40cb0f..e3dc82d08a 100644 --- a/tests/sandbox/test_runtime.py +++ b/tests/sandbox/test_runtime.py @@ -72,6 +72,7 @@ WorkspaceArchiveWriteError, ) from agents.sandbox.files import EntryKind, FileEntry +from agents.sandbox.manifest import Environment, ProcessEnvValue from agents.sandbox.materialization import MaterializationResult, MaterializedFile from agents.sandbox.remote_mount_policy import ( REMOTE_MOUNT_POLICY, @@ -507,7 +508,7 @@ async def test_sandbox_session_aclose_closes_dependencies_when_stop_fails() -> N await session.aclose() assert inner.stop_calls == 1 - assert inner.shutdown_calls == 0 + assert inner.shutdown_calls == 1 assert inner.close_dependency_calls == 1 @@ -1193,6 +1194,26 @@ def test_process_manifest_preserves_mount_acknowledgement_across_replacement() - ) +@pytest.mark.asyncio +async def test_process_manifest_does_not_regrant_process_environment_access_across_replacement( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "from-process") + manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + + processed = SandboxRuntimeSessionManager._process_manifest( + [_ManifestReplacementCapability()], + manifest, + ) + + assert processed is not None + with pytest.raises(ValueError, match="configure the sandbox client"): + await processed.resolve_environment() + + @pytest.mark.parametrize( ("acknowledged_path", "expected_at_replacement_root"), [("/workspace/data", False), ("data", True)], @@ -3869,6 +3890,77 @@ async def test_session_manager_rebinds_persisted_path_grants_from_current_manife assert client.resume_state.path_grants_require_rebind == () +@pytest.mark.asyncio +async def test_session_manager_does_not_rebind_process_environment_access_from_current_manifest( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-worker-value") + trusted_manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + agent = SandboxAgent( + name="worker", + model=ScriptedModel(), + instructions="Worker.", + default_manifest=trusted_manifest, + ) + persisted_manifest = Manifest.model_validate(trusted_manifest.model_dump(mode="json")) + session_state = TestSessionState( + manifest=persisted_manifest, + snapshot=NoopSnapshot(id="resume"), + ) + processed = SandboxRuntimeSessionManager._process_resumed_state_manifest( + agent=agent, + capabilities=[], + session_state=session_state, + trusted_manifest=trusted_manifest, + provider_backend_id="docker", + ) + + assert processed.manifest._has_process_environment_access() is False # noqa: SLF001 + with pytest.raises(ValueError, match="configure the sandbox client"): + await processed.manifest.resolve_environment() + + +@pytest.mark.asyncio +async def test_resume_does_not_rebind_removed_process_environment_reference( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "must-not-be-rebound") + originally_trusted = Manifest( + environment=Environment(value={"TOKEN": ProcessEnvValue(name=name)}) + )._with_process_environment_access(("TOKEN", name)) + current_trusted = originally_trusted.model_copy( + update={"environment": Environment(value={})}, + deep=True, + ) + persisted_manifest = Manifest.model_validate(originally_trusted.model_dump(mode="json")) + session_state = TestSessionState( + manifest=persisted_manifest, + snapshot=NoopSnapshot(id="resume"), + ) + agent = SandboxAgent( + name="worker", + model=ScriptedModel(), + instructions="Worker.", + default_manifest=current_trusted, + ) + + processed = SandboxRuntimeSessionManager._process_resumed_state_manifest( + agent=agent, + capabilities=[], + session_state=session_state, + trusted_manifest=current_trusted, + provider_backend_id="test", + ) + + assert processed.manifest._process_environment_access == frozenset() + with pytest.raises(ValueError, match=f"binding {name!r} -> 'TOKEN' is not granted"): + await processed.manifest.resolve_environment() + + @pytest.mark.asyncio async def test_session_manager_rebinds_redacted_external_mount_authority() -> None: trusted_manifest = Manifest( @@ -4231,6 +4323,31 @@ async def test_session_manager_rejects_unsafe_stopped_injected_session_manifest( assert live_session.state.manifest.entries == {} +@pytest.mark.asyncio +async def test_session_manager_rejects_injected_process_environment_before_probe() -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + live_session = _LiveSessionDeltaRecorder( + Manifest(environment=Environment(value={name: ProcessEnvValue()})) + ) + agent = SandboxAgent(name="worker", model=ScriptedModel(), instructions="Worker.") + manager = SandboxRuntimeSessionManager( + starting_agent=agent, + sandbox_config=SandboxRunConfig(session=live_session), + run_state=None, + ) + + manager.acquire_agent(agent) + with pytest.raises(ValueError, match="client-owned fresh session or resume path"): + await manager.ensure_session( + agent=agent, + capabilities=[], + is_resumed_state=False, + ) + + assert live_session.running_calls == 0 + assert live_session.start_calls == 0 + + @pytest.mark.asyncio @pytest.mark.parametrize("manifest_source", ["run_config", "agent_default"]) async def test_session_manager_redacts_capability_failure_with_external_mount_authority( diff --git a/tests/sandbox/test_session_state_roundtrip.py b/tests/sandbox/test_session_state_roundtrip.py index 670c1b5dd6..3f244945f9 100644 --- a/tests/sandbox/test_session_state_roundtrip.py +++ b/tests/sandbox/test_session_state_roundtrip.py @@ -9,6 +9,7 @@ import io import json +import pickle import uuid from pathlib import Path from typing import ClassVar, Literal, cast @@ -17,7 +18,13 @@ from pydantic import ConfigDict, ValidationError, field_serializer, field_validator from agents.sandbox import Manifest, SandboxPathGrant -from agents.sandbox.manifest import EnvEntry, Environment, EnvValue, StrEnvValue +from agents.sandbox.manifest import ( + EnvEntry, + Environment, + EnvValue, + ProcessEnvValue, + StrEnvValue, +) from agents.sandbox.session import ( BaseSandboxClient, Dependencies, @@ -219,6 +226,53 @@ async def test_parse_restores_manifest_env_value_subclasses(self) -> None: "ENTRY": "resolved-secret-for-entry", } + @pytest.mark.asyncio + async def test_process_environment_authority_is_not_persisted( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + secret = "session-state-secret" + monkeypatch.setenv(name, secret) + manifest = Manifest( + environment=Environment(value={"TOKEN": ProcessEnvValue(name=name)}) + )._with_process_environment_access(("TOKEN", name)) + original = _StubSessionState( + snapshot=NoopSnapshot(id="noop"), + manifest=manifest, + custom_field="custom", + ) + + payload = original.model_dump(mode="json") + serialized = json.dumps(payload) + restored = SandboxSessionState.parse(payload) + + assert secret not in serialized + assert "process_environment_access" not in serialized + with pytest.raises(ValueError, match=f"binding {name!r} -> 'TOKEN' is not granted"): + await restored.manifest.resolve_environment() + + @pytest.mark.asyncio + async def test_process_environment_authority_is_not_pickled( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "pickle-session-state-secret") + original = _StubSessionState( + snapshot=NoopSnapshot(id="noop"), + manifest=Manifest( + environment=Environment(value={"TOKEN": ProcessEnvValue(name=name)}) + )._with_process_environment_access(("TOKEN", name)), + custom_field="custom", + ) + + restored = pickle.loads(pickle.dumps(original)) + + assert restored.manifest._process_environment_access == frozenset() + with pytest.raises(ValueError, match=f"binding {name!r} -> 'TOKEN' is not granted"): + await restored.manifest.resolve_environment() + def test_parse_reads_legacy_discriminator_free_str_env_values(self) -> None: payload = _make_session_state().model_dump(mode="json") payload["manifest"]["environment"] = { diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index 8b097c002c..e928c909dc 100644 --- a/tests/sandbox/test_unix_local.py +++ b/tests/sandbox/test_unix_local.py @@ -10,7 +10,7 @@ from agents.sandbox import SandboxPathGrant from agents.sandbox.errors import PtySessionNotFoundError -from agents.sandbox.manifest import Manifest +from agents.sandbox.manifest import Environment, Manifest, ProcessEnvValue from agents.sandbox.sandboxes.unix_local import ( UnixLocalSandboxClient, UnixLocalSandboxSession, @@ -41,6 +41,25 @@ async def _exec_internal( return ExecResult(stdout=b"", stderr=b"", exit_code=0) +@pytest.mark.asyncio +async def test_unix_local_rejects_process_environment_before_creating_workspace( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def _unexpected_mkdtemp(*args: object, **kwargs: object) -> str: + raise AssertionError(f"unexpected mkdtemp call: {args!r} {kwargs!r}") + + monkeypatch.setattr( + "agents.sandbox.sandboxes.unix_local.tempfile.mkdtemp", + _unexpected_mkdtemp, + ) + manifest = Manifest( + environment=Environment(value={"TOKEN": ProcessEnvValue(name="PROD_KEY")}) + )._with_process_environment_access(("TOKEN", "PROD_KEY")) + + with pytest.raises(ValueError, match="unix_local does not support ProcessEnvValue"): + await UnixLocalSandboxClient().create(manifest=manifest) + + @pytest.mark.asyncio async def test_unix_local_rejects_host_path_before_creating_workspace( tmp_path: Path,