diff --git a/src/agents/extensions/sandbox/blaxel/sandbox.py b/src/agents/extensions/sandbox/blaxel/sandbox.py index aee3211fbd..ed8ce2ea1b 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", + ) 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 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..3e3002a202 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", + ) 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 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..67dfb86330 100644 --- a/src/agents/extensions/sandbox/daytona/sandbox.py +++ b/src/agents/extensions/sandbox/daytona/sandbox.py @@ -1259,6 +1259,12 @@ async def create( ) -> SandboxSession: if manifest is None: manifest = Manifest(root=DEFAULT_DAYTONA_WORKSPACE_ROOT) + manifest._reject_process_environment_values( + backend_id="daytona", + supported_alternative=( + "use DockerSandboxClient for protected process environment transport" + ), + ) self._validate_manifest_for_create(manifest) timeouts_in = options.timeouts @@ -1332,6 +1338,12 @@ async def resume( ) -> SandboxSession: if not isinstance(state, DaytonaSandboxSessionState): raise TypeError("DaytonaSandboxClient.resume expects a DaytonaSandboxSessionState") + state.manifest._reject_process_environment_values( + backend_id="daytona", + supported_alternative=( + "use DockerSandboxClient for protected process environment transport" + ), + ) state.assert_path_grants_rebound() daytona_sandbox = None diff --git a/src/agents/extensions/sandbox/e2b/sandbox.py b/src/agents/extensions/sandbox/e2b/sandbox.py index 389b665c44..8ca5703378 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", + ) 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 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 848b8e12a3..5c2366b40f 100644 --- a/src/agents/extensions/sandbox/modal/sandbox.py +++ b/src/agents/extensions/sandbox/modal/sandbox.py @@ -2110,6 +2110,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", + ) self._validate_manifest_for_create(manifest) app_name = options.app_name if not app_name: @@ -2289,6 +2293,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 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..05f7836a3e 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", + ) _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 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..a3f14b5f69 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", + ) 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 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..d82bd1cced 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 @@ -1154,9 +1305,25 @@ def _call_has_configured_mount_authority( return True sandbox_session_state_descriptor = sandbox_session_metadata[0]["state"] values = (*args, *kwargs.values()) + has_configured_process_environment_client = False 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"): + has_configured_process_environment_client = 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) + or ( + has_configured_process_environment_client + and value._has_process_environment_values() + ) + ): + return True decorated_owner_type = _decorated_owner_type(function) pending = [(value, False) for value in values] @@ -1175,13 +1342,23 @@ 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) + or ( + has_configured_process_environment_client + and value._has_process_environment_values() + ) + ): 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 +1418,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..78c27ae231 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,74 @@ 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 is granted by trusted sandbox client runtime configuration, + not by serialized manifest data. + """ + + 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 sandbox client with " + "trusted process environment bindings" + ) + + +def _validate_process_environment_name(name: str) -> None: + if not isinstance(name, str): + raise TypeError("Process environment variable names must be strings.") + 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.""" + + if isinstance(allowed_process_environment_keys, str): + raise TypeError( + "allowed_process_environment_keys must be an iterable of names, not a string." + ) + 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 +318,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 +426,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 by trusted sandbox client " + "runtime configuration, not in manifest input." + ) return value @field_validator("entries", mode="before") @@ -293,6 +474,117 @@ 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, + ) + environment_values: dict[str, str | EnvValue | EnvEntry] = {} + for key, value in self.environment.value.items(): + if isinstance(value, ProcessEnvValue): + environment_values[key] = value.model_copy() + elif isinstance(value, EnvEntry) and isinstance(value.value, ProcessEnvValue): + environment_values[key] = value.model_copy( + update={"value": value.value.model_copy()} + ) + else: + environment_values[key] = value + trusted = self.model_copy( + update={ + "environment": self.environment.model_copy(update={"value": environment_values}) + } + ) + trusted._process_environment_access = requested_bindings & declared_bindings + return trusted + + async def resolve_environment(self) -> dict[str, str]: + """Resolve the environment on a runtime-only copy bound by a trusted sandbox client.""" + + 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..8d9022425e 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 @@ -55,6 +54,8 @@ def __init__( self._client = client self._owns_session = owns_session self._cleanup_lock = asyncio.Lock() + self._cleanup_requests = 0 + self._cleanup_started = False self._cleaned = False self._started = False @@ -67,54 +68,62 @@ def state(self) -> SandboxSessionState: return self._session.state async def ensure_started(self) -> None: - if self._started and await self._session.running(): - return - if not self._owns_session and await self._session.running(): + async with self._cleanup_lock: + if self._cleanup_started or self._cleanup_requests > 0: + raise RuntimeError("Sandbox session resource cleanup has already started") + if self._started and await self._session.running(): + return + if not self._owns_session and await self._session.running(): + self._started = True + return + await self._session.start() self._started = True - return - await self._session.start() - self._started = True @redact_mount_error_data async def cleanup(self) -> None: if not self._owns_session: return - async with self._cleanup_lock: - if self._cleaned: - return - self._cleaned = True + self._cleanup_requests += 1 + try: + async with self._cleanup_lock: + self._cleanup_started = True + if self._cleaned: + return - cleanup_error: BaseException | None = None - try: - await self._session.run_pre_stop_hooks() - except BaseException as exc: # pragma: no cover - cleanup_error = exc - if cleanup_error is None and not self._session._pre_stop_hooks_failed: + cleanup_error: BaseException | None = None try: - await self._session.stop() + await self._session.run_pre_stop_hooks() except BaseException as exc: # pragma: no cover - if cleanup_error is None: - cleanup_error = exc - try: - await self._session.shutdown() - except BaseException as exc: # pragma: no cover - if cleanup_error is None: cleanup_error = exc - finally: + if cleanup_error is None and not self._session._pre_stop_hooks_failed: + try: + await self._session.stop() + except BaseException as exc: # pragma: no cover + if cleanup_error is None: + cleanup_error = exc try: - if self._client is not None and isinstance(self._session, SandboxSession): - await self._client.delete(self._session) + await self._session.shutdown() except BaseException as exc: # pragma: no cover if cleanup_error is None: cleanup_error = exc finally: try: - await self._session._aclose_dependencies() + if self._client is not None and isinstance(self._session, SandboxSession): + await self._client.delete(self._session) except BaseException as exc: # pragma: no cover if cleanup_error is None: cleanup_error = exc - if cleanup_error is not None: - raise cleanup_error + finally: + try: + await self._session._aclose_dependencies() + except BaseException as exc: # pragma: no cover + if cleanup_error is None: + cleanup_error = exc + if cleanup_error is not None: + raise cleanup_error + self._cleaned = True + finally: + self._cleanup_requests -= 1 @dataclass @@ -280,7 +289,8 @@ async def cleanup(self) -> dict[str, object] | None: if cleanup_error is None: resume_state = self.serialize_resume_state() finally: - self._resources_by_agent.clear() + if cleanup_error is None: + self._resources_by_agent.clear() self._current_agent_id = None self._release_agents() if cleanup_error is not None: @@ -596,10 +606,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 +628,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( @@ -837,10 +852,26 @@ def _process_resumed_state_manifest( resume_manifest, run_as_user=cls._agent_run_as_user(agent), ) + persisted_process_environment_bindings = ( + resume_manifest._declared_process_environment_bindings() + ) + processed_process_environment_bindings = ( + frozenset() + if processed_manifest is None + else processed_manifest._declared_process_environment_bindings() + ) + if not persisted_process_environment_bindings.issubset( + processed_process_environment_bindings + ): + raise ValueError( + "Resumed sandbox sessions cannot remove or change ProcessEnvValue bindings; " + "create a fresh sandbox session instead" + ) if processed_manifest is None: return session_state processed_state = session_state.model_copy(update={"manifest": processed_manifest}) processed_state = processed_state.rebind_persisted_path_grants(processed_manifest) + processed_state._resume_persisted_manifest = resume_manifest if not processed_state.mount_authority_redacted: return processed_state processed_trusted_manifest = cls._process_manifest( @@ -848,10 +879,12 @@ def _process_resumed_state_manifest( trusted_manifest, run_as_user=cls._agent_run_as_user(agent), ) - return processed_state.rebind_persisted_mount_authority( + rebound_state = processed_state.rebind_persisted_mount_authority( processed_trusted_manifest, provider_backend_id=provider_backend_id, ) + rebound_state._resume_persisted_manifest = resume_manifest + return rebound_state @staticmethod def _agent_run_as_user(agent: SandboxAgent[Any]) -> User | None: diff --git a/src/agents/sandbox/sandboxes/docker.py b/src/agents/sandbox/sandboxes/docker.py index fd9ebbe556..8f34ebdd23 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 @@ -50,9 +50,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 @@ -68,7 +71,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 ( @@ -273,6 +276,7 @@ def close(self) -> None: pass +@_register_sdk_process_environment_session_type class DockerSandboxSession(BaseSandboxSession): _docker_client: DockerSDKClient _container: Container @@ -282,6 +286,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( @@ -304,6 +309,16 @@ 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_session_id: uuid.UUID | None = 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( @@ -320,6 +335,105 @@ 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 _capture_process_environment_previous_volume_names( + self, + container: Container, + ) -> None: + previous_session_id = self._process_environment_resume_previous_session_id + if previous_session_id is None: + return + captured_names = _docker_owned_volume_names_for_container( + container, + session_id=previous_session_id, + ) + self._process_environment_resume_previous_volume_names = tuple( + dict.fromkeys( + ( + *self._process_environment_resume_previous_volume_names, + *captured_names, + ) + ) + ) + def supports_pty(self) -> bool: return True @@ -633,6 +747,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, @@ -913,27 +1028,63 @@ async def write( await self._rm_best_effort(staging_path) async def running(self) -> bool: + container = self._container + if container is None: + deferred_container_id = self._process_environment_resume_previous_container_id + if deferred_container_id is None: + return False + loader = self._process_environment_resume_previous_container_loader + try: + container = loader(deferred_container_id) + except docker.errors.NotFound: + return False + if container is None: + return False # docker-py caches container attributes; refresh to avoid stale status, # especially right after start/stop. try: - self._container.reload() + container.reload() except docker.errors.APIError: # Best-effort: if we can't reload, fall back to last known status. pass - return cast(str, self._container.status) == "running" + return cast(str, 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: @@ -954,6 +1105,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, @@ -1088,6 +1240,7 @@ async def pty_exec_start( original_token_count=original_token_count, ) + @redact_mount_error_data async def pty_write_stdin( self, *, @@ -1505,8 +1658,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() @@ -1524,7 +1683,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 @@ -1560,6 +1719,7 @@ async def create( self._cleanup_failed_create_resources( container=container, volume_names=volume_names, + surface_cleanup_failure=manifest._has_process_environment_values(), ) raise @@ -1568,58 +1728,110 @@ 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, - ) cleanup_error: BaseException | None = None try: await inner.shutdown() 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 + if container_id == retained_container_id: + inner._capture_process_environment_previous_volume_names(container) + 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 + 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, + ) + 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 @@ -1628,8 +1840,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 @@ -1641,10 +1857,151 @@ 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.manifest._validate_process_environment_access() 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_session_id = original_session_id + previous_manifest = state.resume_persisted_manifest or state.manifest + inner._process_environment_resume_previous_volume_names = tuple( + _docker_volume_names_for_manifest( + previous_manifest, + session_id=original_session_id, + ) + ) + previous_container_reusable = True + + async def start_replacement() -> None: + nonlocal previous_container_reusable + 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" + ) + process_environment = state.manifest._snapshot_process_environment_values() + if previous_container_reusable: + try: + previous_container = self.get_container(original_container_id) + except docker.errors.NotFound: + previous_container = None + else: + previous_container = None + if previous_container is not None: + inner._capture_process_environment_previous_volume_names(previous_container) + previous_state = state.model_copy(update={"manifest": previous_manifest}) + previous_session = DockerSandboxSession( + container=previous_container, + docker_client=self.docker_client, + state=previous_state, + ) + previous_session.set_dependencies(inner._dependencies) + previous_was_running = await previous_session.running() + try: + if not previous_was_running: + previous_container.start() + await previous_session._persist_snapshot() + finally: + if not previous_was_running: + try: + previous_container.stop() + except BaseException: + previous_container_reusable = False + previous_container_retired = False + try: + previous_container.remove(force=True) + previous_container_retired = True + except BaseException: + kill = getattr(previous_container, "kill", None) + if not callable(kill): + raise _process_environment_error( + "Docker could not make the previous protected " + "container inactive after workspace persistence" + ) from None + try: + kill() + except BaseException: + raise _process_environment_error( + "Docker could not make the previous protected " + "container inactive after workspace persistence" + ) from None + if previous_container_retired: + inner._process_environment_resume_previous_container_id = None + raise + 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" + ) + resolved_environment = { + **await state.manifest._resolve_environment_without_process_values(), + **process_environment, + } + 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, + network_mode=state.network_mode, + 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: @@ -1715,18 +2072,19 @@ async def _create_container( exposed_ports: tuple[int, ...] = (), network_mode: Literal["none"] | None = None, 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, @@ -1958,6 +2316,25 @@ def _docker_volume_names_for_manifest( ] +def _docker_owned_volume_names_for_container( + container: Container, + *, + session_id: uuid.UUID, +) -> list[str]: + container.reload() + raw_mounts = container.attrs.get("Mounts") + mounts = raw_mounts if isinstance(raw_mounts, list) else [] + prefix = f"sandbox_{session_id.hex}_" + return [ + name + for mount in mounts + if isinstance(mount, dict) + and mount.get("Type") == "volume" + and isinstance((name := mount.get("Name")), str) + and name.startswith(prefix) + ] + + def _docker_volume_mounts_for_manifest(manifest: Manifest) -> list[tuple[Mount, Path]]: mounts: list[tuple[Mount, Path]] = [] root = posix_path_as_path(coerce_posix_path(manifest.root)) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index 4d8595b15e..28df2304c6 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", + ) _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 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..3c882012bc 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,31 @@ 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 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 +325,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 +589,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 +1373,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..a46e408a25 100644 --- a/src/agents/sandbox/session/sandbox_session_state.py +++ b/src/agents/sandbox/session/sandbox_session_state.py @@ -42,6 +42,7 @@ class SandboxSessionState(BaseModel): _path_grants_require_rebind: tuple[str, ...] = PrivateAttr(default=()) _mount_authority_redacted: bool = PrivateAttr(default=False) _mount_authority_rebound: bool = PrivateAttr(default=False) + _resume_persisted_manifest: Manifest | None = PrivateAttr(default=None) @property def path_grants_require_rebind(self) -> tuple[str, ...]: @@ -57,6 +58,12 @@ def mount_authority_rebound(self) -> bool: return self._mount_authority_rebound + @property + def resume_persisted_manifest(self) -> Manifest | None: + """Return the pre-capability manifest for runtime-only resume cleanup.""" + + return self._resume_persisted_manifest + def _sanitize_persisted_provider_identity( self, data: dict[str, Any], diff --git a/tests/extensions/sandbox/test_daytona.py b/tests/extensions/sandbox/test_daytona.py index 7f2df5bb4f..2046007913 100644 --- a/tests/extensions/sandbox/test_daytona.py +++ b/tests/extensions/sandbox/test_daytona.py @@ -37,7 +37,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, @@ -529,6 +529,41 @@ async def unmount_path(self, session: object, path: Path) -> None: class TestDaytonaSandbox: + @pytest.mark.asyncio + async def test_create_rejects_process_environment_before_provider_create( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + manifest = Manifest(environment=Environment(value={"TOKEN": ProcessEnvValue()})) + + async with daytona_module.DaytonaSandboxClient() as client: + with pytest.raises(ValueError, match="daytona does not support ProcessEnvValue"): + await client.create( + manifest=manifest, + options=daytona_module.DaytonaSandboxClientOptions(), + ) + + assert _FakeAsyncDaytona.create_calls == [] + + @pytest.mark.asyncio + async def test_resume_rejects_process_environment_before_provider_lookup( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + daytona_module = _load_daytona_module(monkeypatch) + state = daytona_module.DaytonaSandboxSessionState( + manifest=Manifest(environment=Environment(value={"TOKEN": ProcessEnvValue()})), + snapshot=NoopSnapshot(id="snapshot"), + sandbox_id="existing", + ) + + async with daytona_module.DaytonaSandboxClient() as client: + with pytest.raises(ValueError, match="daytona does not support ProcessEnvValue"): + await client.resume(state) + + assert _FakeAsyncDaytona.get_calls == [] + @pytest.mark.asyncio async def test_create_uses_daytona_safe_default_workspace_root( self, diff --git a/tests/sandbox/test_compatibility_guards.py b/tests/sandbox/test_compatibility_guards.py index a358f76ea7..2e359e09e9 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 @@ -901,6 +901,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 5c535e57bf..10de3bc5cb 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,940 @@ 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", + network_mode="none", + ) + 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" + } + assert kwargs["network_mode"] == "none" + 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" + assert await resumed.running() is True + assert reconnect_calls == ["existing-container"] + reconnect_calls.clear() + 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_persists_stopped_workspace_before_replacement( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + state = DockerSandboxSessionState( + manifest=Manifest(environment=Environment(value={name: ProcessEnvValue()})), + 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="stopped", container_id="existing-container") + replacement = _ResumeContainer(status="created", container_id="replacement") + events: list[str] = [] + + def start_existing() -> None: + events.append("start") + existing.status = "running" + + def stop_existing() -> None: + events.append("stop") + existing.status = "stopped" + + existing.start = start_existing + existing.stop = stop_existing + existing.remove = lambda **_kwargs: None + replacement.start = lambda: None + + async def persist_snapshot(_session: BaseSandboxSession) -> None: + events.append("persist") + + async def create_container(*_args: object, **_kwargs: object) -> _ResumeContainer: + events.append("create") + return replacement + + async def start_without_workspace_setup(_session: BaseSandboxSession) -> None: + return None + + monkeypatch.setattr(client, "get_container", lambda _container_id: existing) + monkeypatch.setattr(client, "_create_container", create_container) + monkeypatch.setattr(DockerSandboxSession, "_persist_snapshot", persist_snapshot) + monkeypatch.setattr(BaseSandboxSession, "start", start_without_workspace_setup) + + resumed = await client.resume(state) + await resumed.start() + + assert events[:4] == ["start", "persist", "stop", "create"] + + +@pytest.mark.asyncio +async def test_docker_resume_restores_stopped_container_after_persist_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + state = DockerSandboxSessionState( + manifest=Manifest(environment=Environment(value={name: ProcessEnvValue()})), + 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="stopped", container_id="existing-container") + events: list[str] = [] + + def start_existing() -> None: + events.append("start") + existing.status = "running" + + def stop_existing() -> None: + events.append("stop") + existing.status = "stopped" + + async def fail_persist(_session: BaseSandboxSession) -> None: + events.append("persist") + raise RuntimeError("persist failed") + + async def unexpected_create(*_args: object, **_kwargs: object) -> _ResumeContainer: + raise AssertionError("replacement must not be created after persist failure") + + existing.start = start_existing + existing.stop = stop_existing + monkeypatch.setattr(client, "get_container", lambda _container_id: existing) + monkeypatch.setattr(client, "_create_container", unexpected_create) + monkeypatch.setattr(DockerSandboxSession, "_persist_snapshot", fail_persist) + + resumed = await client.resume(state) + with pytest.raises(RuntimeError, match="protected process environment"): + await resumed.start() + + assert events == ["start", "persist", "stop"] + assert existing.status == "stopped" + + +@pytest.mark.asyncio +async def test_docker_resume_retires_stopped_container_when_restoring_stop_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + state = DockerSandboxSessionState( + manifest=Manifest(environment=Environment(value={name: ProcessEnvValue()})), + 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="stopped", container_id="existing-container") + remove_calls: list[dict[str, object]] = [] + + def start_existing() -> None: + existing.status = "running" + + def fail_stop_existing() -> None: + raise RuntimeError("stop failed") + + def remove_existing(**kwargs: object) -> None: + remove_calls.append(kwargs) + + async def persist_snapshot(_session: BaseSandboxSession) -> None: + return None + + async def unexpected_create(*_args: object, **_kwargs: object) -> _ResumeContainer: + raise AssertionError("replacement must not be created after stop failure") + + existing.start = start_existing + existing.stop = fail_stop_existing + existing.remove = remove_existing + monkeypatch.setattr(client, "get_container", lambda _container_id: existing) + monkeypatch.setattr(client, "_create_container", unexpected_create) + monkeypatch.setattr(DockerSandboxSession, "_persist_snapshot", persist_snapshot) + + resumed = await client.resume(state) + with pytest.raises(RuntimeError, match="protected process environment"): + await resumed.start() + + assert remove_calls == [{"force": True}] + assert resumed._inner._process_environment_resume_previous_container_id is None # noqa: SLF001 + + +@pytest.mark.asyncio +async def test_docker_resume_kills_stopped_container_when_retirement_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + state = DockerSandboxSessionState( + manifest=Manifest(environment=Environment(value={name: ProcessEnvValue()})), + 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="stopped", container_id="existing-container") + replacement = _ResumeContainer(status="created", container_id="replacement") + kill_calls = 0 + start_calls = 0 + + def start_existing() -> None: + nonlocal start_calls + start_calls += 1 + existing.status = "running" + + def fail_stop_existing() -> None: + raise RuntimeError("stop failed") + + def fail_remove_existing(**_kwargs: object) -> None: + raise RuntimeError("remove failed") + + def kill_existing() -> None: + nonlocal kill_calls + kill_calls += 1 + existing.status = "stopped" + + async def persist_snapshot(_session: BaseSandboxSession) -> None: + return None + + async def create_container(*_args: object, **_kwargs: object) -> _ResumeContainer: + return replacement + + async def start_without_workspace_setup(_session: BaseSandboxSession) -> None: + return None + + existing.start = start_existing + existing.stop = fail_stop_existing + existing.remove = fail_remove_existing + existing.kill = kill_existing + monkeypatch.setattr(client, "get_container", lambda _container_id: existing) + monkeypatch.setattr(client, "_create_container", create_container) + monkeypatch.setattr(DockerSandboxSession, "_persist_snapshot", persist_snapshot) + 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 kill_calls == 1 + assert existing.status == "stopped" + assert ( + resumed._inner._process_environment_resume_previous_container_id # noqa: SLF001 + == "existing-container" + ) + + with pytest.raises(RuntimeError, match="protected process environment"): + await resumed.start() + + assert start_calls == 1 + assert resumed.state.container_id == "replacement" + + +@pytest.mark.asyncio +async def test_docker_resume_restores_inactivity_when_stopped_container_start_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + state = DockerSandboxSessionState( + manifest=Manifest(environment=Environment(value={name: ProcessEnvValue()})), + 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="stopped", container_id="existing-container") + events: list[str] = [] + + def fail_start_existing() -> None: + events.append("start") + existing.status = "running" + raise RuntimeError("start failed") + + def stop_existing() -> None: + events.append("stop") + existing.status = "stopped" + + async def unexpected_persist(_session: BaseSandboxSession) -> None: + raise AssertionError("workspace persistence must not run after start failure") + + existing.start = fail_start_existing + existing.stop = stop_existing + monkeypatch.setattr(client, "get_container", lambda _container_id: existing) + monkeypatch.setattr(DockerSandboxSession, "_persist_snapshot", unexpected_persist) + + resumed = await client.resume(state) + with pytest.raises(RuntimeError, match="protected process environment"): + await resumed.start() + + assert events == ["start", "stop"] + assert existing.status == "stopped" + + +@pytest.mark.asyncio +async def test_docker_resume_checks_snapshot_before_custom_environment_resolution( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class _NonRestorableSnapshot(_RestorableSnapshot): + type: Literal["test-non-restorable-docker"] = "test-non-restorable-docker" + + async def restorable(self, *, dependencies: Dependencies | None = None) -> bool: + _ = dependencies + return False + + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + manifest = Manifest(environment=Environment(value={name: ProcessEnvValue(), "CUSTOM": "value"})) + state = DockerSandboxSessionState( + manifest=manifest, + snapshot=_NonRestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="missing-container", + ) + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + custom_resolution_calls = 0 + + async def unexpected_custom_resolution() -> dict[str, str]: + nonlocal custom_resolution_calls + custom_resolution_calls += 1 + raise AssertionError("custom environment resolution must not run") + + monkeypatch.setattr( + client, + "get_container", + lambda _container_id: (_ for _ in ()).throw(docker.errors.NotFound("missing")), + ) + resumed = await client.resume(state) + monkeypatch.setattr( + resumed.state.manifest, + "_resolve_environment_without_process_values", + unexpected_custom_resolution, + ) + + with pytest.raises(ValueError, match="restorable snapshot"): + await resumed.start() + + assert custom_resolution_calls == 0 + + +@pytest.mark.asyncio +async def test_docker_deferred_resume_running_handles_missing_container( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + state = DockerSandboxSessionState( + manifest=Manifest(environment=Environment(value={name: ProcessEnvValue()})), + snapshot=_RestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="missing-container", + ) + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + monkeypatch.setattr(client, "get_container", lambda _container_id: None) + + resumed = await client.resume(state) + + assert await resumed.running() is False + + +@pytest.mark.asyncio +async def test_docker_resume_redacts_failure_before_process_environment_rebind( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + state = DockerSandboxSessionState( + manifest=Manifest(environment=Environment(value={name: ProcessEnvValue()})), + snapshot=_RestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="container", + ) + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + + def fail_rebind(_manifest: Manifest) -> Manifest: + raise RuntimeError("provider-secret") + + monkeypatch.setattr(client, "_bind_process_environment_manifest", fail_rebind) + + with pytest.raises(RuntimeError, match="protected process environment") as exc_info: + await client.resume(state) + + assert "provider-secret" not in str(exc_info.value) + + +@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") + original_session_id = uuid.UUID("12345678-1234-5678-1234-567812345678") + previous_volume_name = docker_sandbox._docker_volume_name( # noqa: SLF001 + session_id=original_session_id, + mount_path=Path("/workspace/removed"), + ) + manifest = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + previous_manifest = Manifest( + entries={ + "removed": S3Mount( + bucket="bucket", + mount_strategy=DockerVolumeMountStrategy(driver="rclone"), + ) + }, + environment=manifest.environment, + ) + state = DockerSandboxSessionState( + manifest=manifest, + snapshot=_RestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="existing-container", + session_id=original_session_id, + ) + state._resume_persisted_manifest = previous_manifest # noqa: SLF001 + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(_ResumeContainer(status="running")), + allowed_process_environment_keys={name}, + ) + existing = _ResumeContainer( + status="running", + container_id="existing-container", + mounts=[ + {"Type": "volume", "Name": previous_volume_name}, + {"Type": "volume", "Name": "foreign-volume"}, + ], + ) + 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: + assert _session.state.manifest is previous_manifest + 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 + assert resumed._inner._process_environment_resume_previous_volume_names == ( # noqa: SLF001 + previous_volume_name, + ) + + +@pytest.mark.asyncio +async def test_docker_resume_retains_preprocessed_volume_names_without_old_container( + monkeypatch: pytest.MonkeyPatch, +) -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + monkeypatch.setenv(name, "current-value") + original_session_id = uuid.UUID("12345678-1234-5678-1234-567812345678") + persisted_manifest = Manifest( + entries={ + "removed": S3Mount( + bucket="bucket", + mount_strategy=DockerVolumeMountStrategy(driver="rclone"), + ) + }, + environment=Environment(value={name: ProcessEnvValue()}), + ) + expected_volume_names = tuple( + docker_sandbox._docker_volume_names_for_manifest( # noqa: SLF001 + persisted_manifest, + session_id=original_session_id, + ) + ) + state = DockerSandboxSessionState( + manifest=Manifest(environment=Environment(value={name: ProcessEnvValue()})), + snapshot=_RestorableSnapshot(id="snapshot"), + image=DEFAULT_PYTHON_SANDBOX_IMAGE, + container_id="missing-container", + session_id=original_session_id, + ) + state._resume_persisted_manifest = persisted_manifest # noqa: SLF001 + client = DockerSandboxClient( + docker_client=_ResumeDockerClient(docker.errors.NotFound("missing")), + allowed_process_environment_keys={name}, + ) + + resumed = await client.resume(state) + + assert ( + resumed._inner._process_environment_resume_previous_volume_names # noqa: SLF001 + == expected_volume_names + ) + + +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..6396dcb2d1 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,410 @@ 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_client_runtime_binding( + 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="sandbox client with trusted"): + 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_client_config_rejects_bare_string_allowlist() -> None: + with pytest.raises(TypeError, match="iterable of names, not a string"): + _normalize_process_environment_bindings(allowed_process_environment_keys="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_access_snapshots_process_declarations() -> None: + source = ProcessEnvValue(name="PROD_TOKEN") + entry = EnvEntry(value=ProcessEnvValue(name="ENTRY_TOKEN")) + manifest = Manifest( + environment=Environment( + value={ + "TOKEN": source, + "ENTRY": entry, + } + ) + ) + + trusted = manifest._with_process_environment_access( + ("TOKEN", "PROD_TOKEN"), + ("ENTRY", "ENTRY_TOKEN"), + ) + manifest.environment.value.clear() + source.name = "MUTATED_TOKEN" + entry.value.name = "MUTATED_ENTRY_TOKEN" + + assert trusted._declared_process_environment_bindings() == frozenset( + {("TOKEN", "PROD_TOKEN"), ("ENTRY", "ENTRY_TOKEN")} + ) + assert trusted._has_process_environment_access() is True + + +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 sandbox client runtime configuration"): + 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..b9dbfec2c5 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,91 @@ 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 + + +@pytest.mark.asyncio +async def test_configured_process_environment_client_does_not_redact_plain_manifest() -> None: + class _ConfiguredClient: + _process_environment_bindings = frozenset({("TOKEN", "TOKEN")}) + + @redact_mount_error_data + async def fail(*, client: object, manifest: Manifest) -> None: + _ = (client, manifest) + raise RuntimeError("plain-manifest-error") + + with pytest.raises(RuntimeError, match="plain-manifest-error"): + await fail(client=_ConfiguredClient(), manifest=Manifest()) + + +def test_mixed_authority_process_environment_validation_preserves_safe_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) + + @redact_mount_error_data_sync + def validate(*, manifest: Manifest) -> None: + manifest._validate_process_environment_access() # noqa: SLF001 + + with pytest.raises(ValueError, match=f"variable {name!r} is not set") as exc_info: + validate(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 + + @pytest.mark.parametrize( "mount", [ diff --git a/tests/sandbox/test_runtime.py b/tests/sandbox/test_runtime.py index ef37d6fb01..276aa4d75e 100644 --- a/tests/sandbox/test_runtime.py +++ b/tests/sandbox/test_runtime.py @@ -77,6 +77,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, @@ -248,6 +249,84 @@ async def stop(self) -> None: raise RuntimeError("stop failed") +class _FailOnceStopSession(_FakeSession): + async def stop(self) -> None: + await super().stop() + if self.stop_calls == 1: + raise RuntimeError("stop failed") + + +@pytest.mark.asyncio +async def test_owned_session_cleanup_waits_for_inflight_start() -> None: + start_gate = asyncio.Event() + session = _FakeSession(Manifest(), start_gate=start_gate) + resources = _SandboxSessionResources(session=session, client=None, owns_session=True) + + start_task = asyncio.create_task(resources.ensure_started()) + await asyncio.sleep(0) + assert session.start_calls == 1 + + cleanup_task = asyncio.create_task(resources.cleanup()) + await asyncio.sleep(0) + assert session.stop_calls == 0 + assert session.shutdown_calls == 0 + + start_gate.set() + await start_task + await cleanup_task + + assert session.stop_calls == 1 + assert session.shutdown_calls == 1 + with pytest.raises(RuntimeError, match="cleanup has already started"): + await resources.ensure_started() + + +@pytest.mark.asyncio +async def test_cancelled_cleanup_waiter_does_not_poison_inflight_start() -> None: + start_gate = asyncio.Event() + session = _FakeSession(Manifest(), start_gate=start_gate) + resources = _SandboxSessionResources(session=session, client=None, owns_session=True) + + start_task = asyncio.create_task(resources.ensure_started()) + await asyncio.sleep(0) + cleanup_task = asyncio.create_task(resources.cleanup()) + await asyncio.sleep(0) + + cleanup_task.cancel() + with pytest.raises(asyncio.CancelledError): + await cleanup_task + + start_gate.set() + await start_task + await resources.ensure_started() + + assert session.start_calls == 1 + await resources.cleanup() + + +@pytest.mark.asyncio +async def test_owned_session_cleanup_retries_after_failure() -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + session = _FailOnceStopSession( + Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + ) + resources = _SandboxSessionResources(session=session, client=None, owns_session=True) + + with pytest.raises(RuntimeError, match="protected process environment"): + await resources.cleanup() + assert session.shutdown_calls == 1 + assert session.close_dependency_calls == 1 + with pytest.raises(RuntimeError, match="cleanup has already started"): + await resources.ensure_started() + await resources.cleanup() + + assert session.stop_calls == 2 + assert session.shutdown_calls == 2 + assert session.close_dependency_calls == 2 + + def _external_mount_manifest(secret_access_key: str) -> Manifest: return Manifest( entries={ @@ -540,7 +619,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 @@ -1226,6 +1305,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)], @@ -3902,6 +4001,145 @@ 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, + ) + + class _RemoveProcessEnvironmentCapability(Capability): + type: str = "remove-process-environment" + + def process_manifest(self, manifest: Manifest) -> Manifest: + return manifest.model_copy(update={"environment": Environment(value={})}) + + with pytest.raises(ValueError, match="cannot remove or change ProcessEnvValue bindings"): + SandboxRuntimeSessionManager._process_resumed_state_manifest( + agent=agent, + capabilities=[_RemoveProcessEnvironmentCapability()], + session_state=session_state, + trusted_manifest=current_trusted, + provider_backend_id="test", + ) + + +def test_resume_does_not_rebind_changed_process_environment_reference() -> None: + name = "SANDBOX_TEST_PROCESS_ENV_VALUE" + source_name = "SANDBOX_TEST_PROCESS_ENV_SOURCE" + current_trusted = Manifest( + environment=Environment(value={name: ProcessEnvValue()}) + )._with_process_environment_access(name) + session_state = TestSessionState( + manifest=current_trusted, + snapshot=NoopSnapshot(id="snapshot"), + ) + agent = SandboxAgent( + name="worker", + model=ScriptedModel(), + instructions="Worker.", + default_manifest=current_trusted, + ) + + class _ChangeProcessEnvironmentCapability(Capability): + type: str = "change-process-environment" + + def process_manifest(self, manifest: Manifest) -> Manifest: + return manifest.model_copy( + update={"environment": Environment(value={name: ProcessEnvValue(name=source_name)})} + ) + + with pytest.raises(ValueError, match="cannot remove or change ProcessEnvValue bindings"): + SandboxRuntimeSessionManager._process_resumed_state_manifest( + agent=agent, + capabilities=[_ChangeProcessEnvironmentCapability()], + session_state=session_state, + trusted_manifest=current_trusted, + provider_backend_id="test", + ) + + +def test_resume_preserves_preprocessed_manifest_for_provider_cleanup() -> None: + persisted_manifest = Manifest(entries={"old.txt": File(content=b"old")}) + session_state = TestSessionState( + manifest=persisted_manifest, + snapshot=NoopSnapshot(id="snapshot"), + ) + agent = SandboxAgent( + name="worker", + model=ScriptedModel(), + instructions="Worker.", + default_manifest=persisted_manifest, + ) + + class _RemoveEntryCapability(Capability): + type: str = "remove-entry" + + def process_manifest(self, manifest: Manifest) -> Manifest: + return manifest.model_copy(update={"entries": {}}) + + processed = SandboxRuntimeSessionManager._process_resumed_state_manifest( + agent=agent, + capabilities=[_RemoveEntryCapability()], + session_state=session_state, + trusted_manifest=persisted_manifest, + provider_backend_id="docker", + ) + + assert processed.resume_persisted_manifest is persisted_manifest + + @pytest.mark.asyncio async def test_session_manager_rebinds_redacted_external_mount_authority() -> None: trusted_manifest = Manifest( @@ -4264,6 +4502,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..ee3c01a837 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,37 @@ 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_resume_reports_unsupported_process_environment_after_roundtrip() -> None: + state = UnixLocalSandboxSessionState( + manifest=Manifest(environment=Environment(value={"TOKEN": ProcessEnvValue()})), + snapshot=NoopSnapshot(id="noop"), + ) + restored = UnixLocalSandboxSessionState.model_validate_json(state.model_dump_json()) + + with pytest.raises(ValueError, match="unix_local does not support ProcessEnvValue"): + await UnixLocalSandboxClient().resume(restored) + + @pytest.mark.asyncio async def test_unix_local_rejects_host_path_before_creating_workspace( tmp_path: Path,