diff --git a/src/opencode_a2a/config.py b/src/opencode_a2a/config.py index ad72f65..c46effd 100644 --- a/src/opencode_a2a/config.py +++ b/src/opencode_a2a/config.py @@ -3,10 +3,11 @@ import json from typing import Annotated, Any, Literal, cast -from pydantic import Field, field_validator +from pydantic import BeforeValidator, Field, model_validator from pydantic_settings import BaseSettings, NoDecode, SettingsConfigDict from opencode_a2a import __version__ +from opencode_a2a.sandbox_policy import SandboxPolicy SandboxMode = Literal[ "unknown", @@ -34,7 +35,6 @@ "custom", ] OutsideWorkspaceAccess = Literal["unknown", "allowed", "disallowed", "custom"] -DeclaredStringList = Annotated[tuple[str, ...], NoDecode] def _parse_declared_list(value: Any) -> tuple[str, ...]: @@ -59,6 +59,9 @@ def _parse_declared_list(value: Any) -> tuple[str, ...]: raise TypeError("Expected a comma-separated string, JSON array, or sequence.") +DeclaredStringList = Annotated[tuple[str, ...], NoDecode, BeforeValidator(_parse_declared_list)] + + class Settings(BaseSettings): model_config = SettingsConfigDict( env_prefix="", @@ -173,18 +176,12 @@ class Settings(BaseSettings): alias="A2A_CLIENT_SUPPORTED_TRANSPORTS", ) - @field_validator( - "a2a_sandbox_writable_roots", - "a2a_network_allowed_domains", - "a2a_client_supported_transports", - mode="before", - ) - @classmethod - def _normalize_declared_lists(cls, value: Any) -> tuple[str, ...]: - return _parse_declared_list(value) + @model_validator(mode="after") + def _validate_sandbox_policy(self) -> Settings: + SandboxPolicy.from_settings(self).validate_configuration() + return self @classmethod def from_env(cls) -> Settings: - # BaseSettings constructor loads values from env and applies validation. settings_cls: type[BaseSettings] = cls return cast(Settings, settings_cls()) diff --git a/src/opencode_a2a/contracts/extensions.py b/src/opencode_a2a/contracts/extensions.py index d65e4d2..dc26419 100644 --- a/src/opencode_a2a/contracts/extensions.py +++ b/src/opencode_a2a/contracts/extensions.py @@ -302,10 +302,6 @@ class DeploymentConditionalMethod: toggle: str reason_when_disabled: str = "disabled_by_configuration" - @property - def availability(self) -> str: - return "enabled" if self.enabled else "disabled" - def control_method_flag(self) -> dict[str, Any]: return { "enabled_by_default": False, @@ -315,7 +311,7 @@ def control_method_flag(self) -> dict[str, Any]: def method_retention(self) -> dict[str, Any]: return { "surface": "extension", - "availability": self.availability, + "availability": "enabled" if self.enabled else "disabled", "retention": "deployment-conditional", "extension_uri": self.extension_uri, "toggle": self.toggle, @@ -411,7 +407,7 @@ def build_capability_snapshot(*, runtime_profile: RuntimeProfile) -> JsonRpcCapa conditional_methods={ SESSION_CONTROL_METHODS["shell"]: DeploymentConditionalMethod( method=SESSION_CONTROL_METHODS["shell"], - enabled=runtime_profile.session_shell_enabled, + enabled=runtime_profile.session_shell.enabled, extension_uri=SESSION_QUERY_EXTENSION_URI, toggle=SESSION_SHELL_TOGGLE, ) diff --git a/src/opencode_a2a/execution/executor.py b/src/opencode_a2a/execution/executor.py index 8e516ed..6e0ee3e 100644 --- a/src/opencode_a2a/execution/executor.py +++ b/src/opencode_a2a/execution/executor.py @@ -34,8 +34,8 @@ map_a2a_parts_to_opencode_parts, summarize_a2a_parts, ) +from ..sandbox_policy import SandboxPolicy from .event_helpers import _enqueue_artifact_update -from .policy import PolicyEnforcer from .request_context import ( _build_history, _extract_opencode_directory, @@ -207,7 +207,7 @@ async def run(self) -> None: ) if self._pending_preferred_claim: - await self._executor._finalize_preferred_session_binding( + await self._executor._session_manager.finalize_preferred_session_binding( identity=self._prepared.identity, context_id=self._context_id, session_id=self._session_id, @@ -296,14 +296,16 @@ async def _bind_session(self) -> None: ( self._session_id, self._pending_preferred_claim, - ) = await self._executor._get_or_create_session( + ) = await self._executor._session_manager.get_or_create_session( self._prepared.identity, self._context_id, self._prepared.session_title or self._prepared.user_text, preferred_session_id=self._prepared.bound_session_id, directory=self._prepared.directory, ) - self._session_lock = await self._executor._get_session_lock(self._session_id) + self._session_lock = await self._executor._session_manager.get_session_lock( + self._session_id + ) await self._session_lock.acquire() async with self._executor._lock: self._executor._running_session_ids[self._execution_key] = self._session_id @@ -500,7 +502,7 @@ async def _handle_non_streaming_response( async def _cleanup(self) -> None: if self._pending_preferred_claim and self._session_id: with suppress(Exception): - await self._executor._release_preferred_session_claim( + await self._executor._session_manager.release_preferred_session_claim( identity=self._prepared.identity, session_id=self._session_id, ) @@ -534,7 +536,10 @@ def __init__( self._streaming_enabled = streaming_enabled self._cancel_abort_timeout_seconds = max(0.0, float(cancel_abort_timeout_seconds)) self._a2a_client_manager = a2a_client_manager - self._policy = PolicyEnforcer(client=client) + self._sandbox_policy = SandboxPolicy.from_settings( + client.settings, + workspace_root=client.directory, + ) self._session_manager = SessionManager( client=client, session_cache_ttl_seconds=session_cache_ttl_seconds, @@ -560,23 +565,6 @@ def _emit_metric( ) -> None: _emit_metric(name, value, **labels) - def _resolve_and_validate_directory(self, requested: str | None) -> str | None: - return self._policy.resolve_directory(requested) - - def resolve_directory_for_control(self, requested: str | None) -> str | None: - return self._policy.resolve_directory_for_control(requested) - - async def claim_session_for_control(self, *, identity: str, session_id: str) -> bool: - return await self._claim_preferred_session(identity=identity, session_id=session_id) - - async def finalize_session_for_control(self, *, identity: str, session_id: str) -> None: - """Finalize control-session ownership after upstream call succeeds.""" - await self._finalize_session_claim(identity=identity, session_id=session_id) - - async def release_session_for_control(self, *, identity: str, session_id: str) -> None: - """Release pending control-session ownership on failure.""" - await self._release_preferred_session_claim(identity=identity, session_id=session_id) - async def _maybe_handle_tools( self, raw_response: dict[str, Any] ) -> list[dict[str, Any]] | None: @@ -777,7 +765,10 @@ async def execute(self, context: RequestContext, event_queue: EventQueue) -> Non requested_dir = _extract_opencode_directory(context) try: - directory = self._resolve_and_validate_directory(requested_dir) + directory = self._sandbox_policy.resolve_directory( + requested_dir, + default_directory=self._client.directory, + ) except ValueError as e: logger.warning("Directory validation failed: %s", e) await self._emit_error( @@ -952,57 +943,6 @@ async def cancel(self, context: RequestContext, event_queue: EventQueue) -> None abort_outcome=abort_outcome, ) - async def _get_or_create_session( - self, - identity: str, - context_id: str, - title: str, - *, - preferred_session_id: str | None = None, - directory: str | None = None, - ) -> tuple[str, bool]: - return await self._session_manager.get_or_create_session( - identity, - context_id, - title, - preferred_session_id=preferred_session_id, - directory=directory, - ) - - async def _finalize_preferred_session_binding( - self, - *, - identity: str, - context_id: str, - session_id: str, - ) -> None: - await self._session_manager.finalize_preferred_session_binding( - identity=identity, - context_id=context_id, - session_id=session_id, - ) - - async def _claim_preferred_session(self, *, identity: str, session_id: str) -> bool: - return await self._session_manager.claim_preferred_session( - identity=identity, - session_id=session_id, - ) - - async def _finalize_session_claim(self, *, identity: str, session_id: str) -> None: - await self._session_manager.finalize_session_claim( - identity=identity, - session_id=session_id, - ) - - async def _release_preferred_session_claim(self, *, identity: str, session_id: str) -> None: - await self._session_manager.release_preferred_session_claim( - identity=identity, - session_id=session_id, - ) - - async def _get_session_lock(self, session_id: str) -> asyncio.Lock: - return await self._session_manager.get_session_lock(session_id) - async def _emit_error( self, event_queue: EventQueue, diff --git a/src/opencode_a2a/execution/policy.py b/src/opencode_a2a/execution/policy.py deleted file mode 100644 index 7c16236..0000000 --- a/src/opencode_a2a/execution/policy.py +++ /dev/null @@ -1,44 +0,0 @@ -from __future__ import annotations - -import os -from pathlib import Path - - -class PolicyEnforcer: - def __init__(self, *, client) -> None: - self._client = client - - def resolve_directory(self, requested: str | None) -> str | None: - base_dir_str = self._client.directory or os.getcwd() - base_path = Path(base_dir_str).resolve() - - if requested is not None and not isinstance(requested, str): - raise ValueError("Directory must be a string path") - - requested = requested.strip() if requested else requested - if not requested: - return str(base_path) - - def _resolve_requested(path: str) -> Path: - candidate = Path(path) - if not candidate.is_absolute(): - candidate = base_path / candidate - return candidate.resolve() - - if not self._client.settings.a2a_allow_directory_override: - requested_path = _resolve_requested(requested) - if requested_path == base_path: - return str(base_path) - raise ValueError("Directory override is disabled by service configuration") - - requested_path = _resolve_requested(requested) - try: - requested_path.relative_to(base_path) - except ValueError as err: - raise ValueError( - f"Directory {requested} is outside the allowed workspace {base_path}" - ) from err - return str(requested_path) - - def resolve_directory_for_control(self, requested: str | None) -> str | None: - return self.resolve_directory(requested) diff --git a/src/opencode_a2a/profile/runtime.py b/src/opencode_a2a/profile/runtime.py index 2c420ab..86de2f8 100644 --- a/src/opencode_a2a/profile/runtime.py +++ b/src/opencode_a2a/profile/runtime.py @@ -4,6 +4,7 @@ from typing import Any from ..config import Settings +from ..sandbox_policy import SandboxPolicy PROFILE_ID = "opencode-a2a-single-tenant-coding-v1" DEPLOYMENT_ID = "single_tenant_shared_workspace" @@ -165,10 +166,6 @@ class RuntimeProfile: service_features: ServiceFeaturesProfile runtime_context: RuntimeContext - @property - def session_shell_enabled(self) -> bool: - return self.session_shell.enabled - def runtime_features_dict(self) -> dict[str, Any]: return { "directory_binding": self.directory_binding.as_dict(), @@ -206,6 +203,10 @@ def health_payload( def build_runtime_profile(settings: Settings) -> RuntimeProfile: + sandbox_policy = SandboxPolicy.from_settings(settings) + shell_enabled = sandbox_policy.is_session_shell_enabled( + enabled_by_config=settings.a2a_enable_session_shell, + ) directory_scope = ( "workspace_root_or_descendant" if settings.a2a_allow_directory_override @@ -219,8 +220,8 @@ def build_runtime_profile(settings: Settings) -> RuntimeProfile: scope=directory_scope, ), session_shell=SessionShellProfile( - enabled=settings.a2a_enable_session_shell, - availability="enabled" if settings.a2a_enable_session_shell else "disabled", + enabled=shell_enabled, + availability="enabled" if shell_enabled else "disabled", ), execution_environment=ExecutionEnvironmentProfile( sandbox=SandboxProfile( diff --git a/src/opencode_a2a/sandbox_policy.py b/src/opencode_a2a/sandbox_policy.py new file mode 100644 index 0000000..2a56e8d --- /dev/null +++ b/src/opencode_a2a/sandbox_policy.py @@ -0,0 +1,111 @@ +from __future__ import annotations + +import os +from dataclasses import dataclass +from pathlib import Path +from typing import Any + + +@dataclass(frozen=True) +class SandboxPolicy: + workspace_root: Path + allow_directory_override: bool + sandbox_mode: str + filesystem_scope: str + writable_roots: tuple[Path, ...] + write_access_scope: str + + @classmethod + def from_settings( + cls, + settings: Any, + *, + workspace_root: str | None = None, + ) -> SandboxPolicy: + base_path = Path(workspace_root or settings.opencode_workspace_root or os.getcwd()) + writable_roots = tuple( + Path(root).resolve() + for root in settings.a2a_sandbox_writable_roots + if isinstance(root, str) and root.strip() + ) + return cls( + workspace_root=base_path.resolve(), + allow_directory_override=settings.a2a_allow_directory_override, + sandbox_mode=settings.a2a_sandbox_mode, + filesystem_scope=settings.a2a_sandbox_filesystem_scope, + writable_roots=writable_roots, + write_access_scope=settings.a2a_write_access_scope, + ) + + def resolve_directory( + self, + requested: str | None, + *, + default_directory: str | None = None, + ) -> str | None: + base_path = Path(default_directory).resolve() if default_directory else self.workspace_root + + if requested is not None and not isinstance(requested, str): + raise ValueError("Directory must be a string path") + + requested = requested.strip() if requested else requested + if not requested: + return str(base_path) + + requested_path = Path(requested) + if not requested_path.is_absolute(): + requested_path = base_path / requested_path + requested_path = requested_path.resolve() + + if not self.allow_directory_override: + if requested_path == base_path: + return str(base_path) + raise ValueError("Directory override is disabled by service configuration") + + try: + requested_path.relative_to(base_path) + except ValueError as err: + raise ValueError( + f"Directory {requested} is outside the allowed workspace {base_path}" + ) from err + return str(requested_path) + + def is_session_shell_enabled( + self, + *, + enabled_by_config: bool, + ) -> bool: + if not enabled_by_config: + return False + if self.sandbox_mode == "read-only": + return False + if self.write_access_scope == "none": + return False + return True + + def validate_configuration(self) -> None: + if self.write_access_scope == "none" and self.writable_roots: + raise ValueError( + "Declared writable roots are incompatible with A2A_WRITE_ACCESS_SCOPE=none" + ) + if self.write_access_scope == "workspace_only" or self.filesystem_scope == "workspace_only": + outside_workspace = [ + str(root) + for root in self.writable_roots + if not _is_within_workspace(root, workspace_root=self.workspace_root) + ] + if outside_workspace: + joined = ", ".join(outside_workspace) + raise ValueError( + "Declared writable roots must stay within the workspace root when " + "the configured scope is workspace_only: " + f"{joined}" + ) + + +def _is_within_workspace(path: Path, *, workspace_root: Path) -> bool: + try: + path.relative_to(workspace_root) + except ValueError: + return False + return True diff --git a/src/opencode_a2a/server/application.py b/src/opencode_a2a/server/application.py index 69af4a5..c229187 100644 --- a/src/opencode_a2a/server/application.py +++ b/src/opencode_a2a/server/application.py @@ -6,6 +6,7 @@ import secrets from contextlib import asynccontextmanager from contextvars import ContextVar, Token +from functools import partial from typing import TYPE_CHECKING import uvicorn @@ -361,14 +362,6 @@ def __init__(self, settings: Settings) -> None: self.clients: dict[str, _ClientCacheEntry] = {} self._lock = asyncio.Lock() - @property - def cache_ttl_seconds(self) -> float: - return self._cache_ttl_seconds - - @property - def cache_maxsize(self) -> int: - return self._cache_maxsize - @asynccontextmanager async def borrow_client(self, agent_url: str): url = agent_url.rstrip("/") @@ -384,8 +377,8 @@ async def borrow_client(self, agent_url: str): async with self._lock: now = self._now() entry = self.clients.get(url) - if entry is not None and self._entry_expired(entry, now=now): - if self._entry_in_use(entry): + if entry is not None and entry.expires_at is not None and entry.expires_at <= now: + if entry.borrow_count > 0 or entry.client.is_busy(): entry.pending_eviction = True else: self.clients.pop(url, None) @@ -396,12 +389,16 @@ async def borrow_client(self, agent_url: str): entry = _ClientCacheEntry( client=A2AClient(url, settings=self.client_settings), last_used=now, - expires_at=self._expires_at_for(now), + expires_at=None + if self._cache_ttl_seconds <= 0 + else now + self._cache_ttl_seconds, ) self.clients[url] = entry else: entry.last_used = now - entry.expires_at = self._expires_at_for(now) + entry.expires_at = ( + None if self._cache_ttl_seconds <= 0 else now + self._cache_ttl_seconds + ) entry.pending_eviction = False entry.borrow_count += 1 to_close.extend(self._evict_locked(now=now, protected_keys={url})) @@ -417,7 +414,9 @@ async def borrow_client(self, agent_url: str): if current.borrow_count > 0: current.borrow_count -= 1 current.last_used = now - current.expires_at = self._expires_at_for(now) + current.expires_at = ( + None if self._cache_ttl_seconds <= 0 else now + self._cache_ttl_seconds + ) to_close = self._evict_locked(now=now) await self._close_clients(to_close) @@ -428,11 +427,6 @@ async def close_all(self) -> None: for client in clients: await client.close() - def _expires_at_for(self, now: float) -> float | None: - if self._cache_ttl_seconds <= 0: - return None - return now + self._cache_ttl_seconds - def _evict_locked( self, *, @@ -443,10 +437,10 @@ def _evict_locked( to_close: list[A2AClient] = [] for key, entry in list(self.clients.items()): - expired = self._entry_expired(entry, now=now) + expired = entry.expires_at is not None and entry.expires_at <= now if not expired and not entry.pending_eviction: continue - if key in protected or self._entry_in_use(entry): + if key in protected or entry.borrow_count > 0 or entry.client.is_busy(): entry.pending_eviction = True continue self.clients.pop(key, None) @@ -463,7 +457,7 @@ def _evict_locked( break if key in protected: continue - if self._entry_in_use(entry): + if entry.borrow_count > 0 or entry.client.is_busy(): entry.pending_eviction = True continue self.clients.pop(key, None) @@ -471,12 +465,6 @@ def _evict_locked( return to_close - def _entry_expired(self, entry: _ClientCacheEntry, *, now: float) -> bool: - return entry.expires_at is not None and entry.expires_at <= now - - def _entry_in_use(self, entry: _ClientCacheEntry) -> bool: - return entry.borrow_count > 0 or entry.client.is_busy() - async def _close_clients(self, clients: list[A2AClient]) -> None: for client in clients: await client.close() @@ -535,10 +523,21 @@ def create_app(settings: Settings) -> FastAPI: upstream_client=upstream_client, protocol_version=settings.a2a_protocol_version, supported_methods=capability_snapshot.supported_jsonrpc_methods(), - directory_resolver=executor.resolve_directory_for_control, - session_claim=executor.claim_session_for_control, - session_claim_finalize=executor.finalize_session_for_control, - session_claim_release=executor.release_session_for_control, + directory_resolver=( + partial( + executor._sandbox_policy.resolve_directory, + default_directory=upstream_client.directory, + ) + if hasattr(executor, "_sandbox_policy") + else None + ), + session_claim=getattr(executor._session_manager, "claim_preferred_session", None), + session_claim_finalize=getattr(executor._session_manager, "finalize_session_claim", None), + session_claim_release=getattr( + executor._session_manager, + "release_preferred_session_claim", + None, + ), methods=jsonrpc_methods, ) rest_adapter = RESTAdapter( diff --git a/tests/config/test_settings.py b/tests/config/test_settings.py index fc98f68..9e1d432 100644 --- a/tests/config/test_settings.py +++ b/tests/config/test_settings.py @@ -11,7 +11,7 @@ def test_settings_missing_required(): with mock.patch.dict(os.environ, {}, clear=True): with pytest.raises(ValidationError) as excinfo: - Settings.from_env() + Settings() # Should mention missing required fields errors = excinfo.value.errors() field_names = [e["loc"][0] for e in errors] @@ -39,7 +39,7 @@ def test_settings_valid(): "A2A_WRITE_ACCESS_OUTSIDE_WORKSPACE": "allowed", } with mock.patch.dict(os.environ, env, clear=True): - settings = Settings.from_env() + settings = Settings() assert settings.a2a_bearer_token == "test-token" assert settings.opencode_timeout == 300.0 assert settings.opencode_workspace_root == "/srv/workspaces/alpha" @@ -66,7 +66,7 @@ def test_settings_ignore_legacy_opencode_directory_env() -> None: "OPENCODE_DIRECTORY": "/legacy/workspace", } with mock.patch.dict(os.environ, env, clear=True): - settings = Settings.from_env() + settings = Settings() assert settings.opencode_workspace_root is None @@ -78,7 +78,37 @@ def test_settings_reject_negative_max_request_body_bytes(): } with mock.patch.dict(os.environ, env, clear=True): with pytest.raises(ValidationError) as excinfo: - Settings.from_env() + Settings() field_names = [e["loc"][0] for e in excinfo.value.errors()] assert "A2A_MAX_REQUEST_BODY_BYTES" in field_names + + +def test_settings_reject_declared_writable_roots_outside_workspace_for_workspace_only_scope(): + env = { + "A2A_BEARER_TOKEN": "test-token", + "OPENCODE_WORKSPACE_ROOT": "/srv/workspaces/alpha", + "A2A_SANDBOX_WRITABLE_ROOTS": "/srv/workspaces/alpha,/tmp/opencode", + "A2A_WRITE_ACCESS_SCOPE": "workspace_only", + } + with mock.patch.dict(os.environ, env, clear=True): + with pytest.raises(ValidationError) as excinfo: + Settings() + + assert "Declared writable roots must stay within the workspace root" in str(excinfo.value) + + +def test_settings_reject_declared_writable_roots_when_write_scope_is_none(): + env = { + "A2A_BEARER_TOKEN": "test-token", + "OPENCODE_WORKSPACE_ROOT": "/srv/workspaces/alpha", + "A2A_SANDBOX_WRITABLE_ROOTS": "/srv/workspaces/alpha/tmp", + "A2A_WRITE_ACCESS_SCOPE": "none", + } + with mock.patch.dict(os.environ, env, clear=True): + with pytest.raises(ValidationError) as excinfo: + Settings() + + assert "Declared writable roots are incompatible with A2A_WRITE_ACCESS_SCOPE=none" in str( + excinfo.value + ) diff --git a/tests/execution/test_directory_validation.py b/tests/execution/test_directory_validation.py index 93e8a51..1aa0343 100644 --- a/tests/execution/test_directory_validation.py +++ b/tests/execution/test_directory_validation.py @@ -24,49 +24,61 @@ def mock_client(): def test_resolve_and_validate_directory_valid(mock_client): executor = OpencodeAgentExecutor(mock_client, streaming_enabled=False) + resolve_directory = lambda requested: executor._sandbox_policy.resolve_directory( # noqa: E731, SLF001 + requested, + default_directory=mock_client.directory, + ) # Setup mock workspace base_dir = Path("/tmp/workspace").resolve() # Valid subpath requested = "/tmp/workspace/project1" - resolved = executor._resolve_and_validate_directory(requested) + resolved = resolve_directory(requested) assert resolved == str(Path(requested).resolve()) # Valid base path - resolved = executor._resolve_and_validate_directory("/tmp/workspace") + resolved = resolve_directory("/tmp/workspace") assert resolved == str(base_dir) # Relative path should be resolved against workspace root, not process cwd. - resolved = executor._resolve_and_validate_directory("project2/sub") + resolved = resolve_directory("project2/sub") assert resolved == str((base_dir / "project2/sub").resolve()) def test_resolve_and_validate_directory_traversal(mock_client): executor = OpencodeAgentExecutor(mock_client, streaming_enabled=False) + resolve_directory = lambda requested: executor._sandbox_policy.resolve_directory( # noqa: E731, SLF001 + requested, + default_directory=mock_client.directory, + ) # Attempt traversal with pytest.raises(ValueError, match="outside the allowed workspace"): - executor._resolve_and_validate_directory("/tmp/workspace/../secret") + resolve_directory("/tmp/workspace/../secret") with pytest.raises(ValueError, match="outside the allowed workspace"): - executor._resolve_and_validate_directory("/etc/passwd") + resolve_directory("/etc/passwd") with pytest.raises(ValueError, match="outside the allowed workspace"): - executor._resolve_and_validate_directory("../secret") + resolve_directory("../secret") def test_resolve_and_validate_directory_override_disabled(mock_client): # Disable override mock_client._settings.a2a_allow_directory_override = False executor = OpencodeAgentExecutor(mock_client, streaming_enabled=False) + resolve_directory = lambda requested: executor._sandbox_policy.resolve_directory( # noqa: E731, SLF001 + requested, + default_directory=mock_client.directory, + ) # Deny different path with pytest.raises(ValueError, match="override is disabled"): - executor._resolve_and_validate_directory("/tmp/workspace/other") + resolve_directory("/tmp/workspace/other") # Allow same path (resolved) - resolved = executor._resolve_and_validate_directory("/tmp/workspace/./") + resolved = resolve_directory("/tmp/workspace/./") assert resolved == str(Path("/tmp/workspace").resolve()) diff --git a/tests/execution/test_session_ownership.py b/tests/execution/test_session_ownership.py index 6ab75c4..363fa23 100644 --- a/tests/execution/test_session_ownership.py +++ b/tests/execution/test_session_ownership.py @@ -302,7 +302,7 @@ async def test_pending_preferred_session_claim_blocks_other_identity(): AsyncMock(spec=OpencodeUpstreamClient), streaming_enabled=False ) - session_id, pending = await executor._get_or_create_session( + session_id, pending = await executor._session_manager.get_or_create_session( "user-1", "context-A", "hello", @@ -312,11 +312,14 @@ async def test_pending_preferred_session_claim_blocks_other_identity(): assert pending is True with pytest.raises(PermissionError, match="not owned by you"): - await executor._get_or_create_session( + await executor._session_manager.get_or_create_session( "user-2", "context-B", "hello", preferred_session_id="session-X", ) - await executor._release_preferred_session_claim(identity="user-1", session_id="session-X") + await executor._session_manager.release_preferred_session_claim( + identity="user-1", + session_id="session-X", + ) diff --git a/tests/jsonrpc/test_jsonrpc_unsupported_method.py b/tests/jsonrpc/test_jsonrpc_unsupported_method.py index 8303a74..0641e4f 100644 --- a/tests/jsonrpc/test_jsonrpc_unsupported_method.py +++ b/tests/jsonrpc/test_jsonrpc_unsupported_method.py @@ -83,3 +83,38 @@ async def test_disabled_shell_reports_current_supported_methods() -> None: assert error["data"]["type"] == "METHOD_NOT_SUPPORTED" assert error["data"]["method"] == "opencode.sessions.shell" assert "opencode.sessions.shell" not in error["data"]["supported_methods"] + + +@pytest.mark.asyncio +async def test_policy_disabled_shell_reports_current_supported_methods() -> None: + settings = make_settings( + a2a_bearer_token="test-token", + a2a_enable_session_shell=True, + a2a_sandbox_mode="read-only", + a2a_write_access_scope="workspace_only", + ) + app = create_app(settings) + transport = httpx.ASGITransport(app=app) + + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.post( + "/", + headers={"Authorization": "Bearer test-token"}, + json={ + "jsonrpc": "2.0", + "id": 125, + "method": "opencode.sessions.shell", + "params": { + "session_id": "s-1", + "request": {"agent": "code-reviewer", "command": "pwd"}, + }, + }, + ) + + assert response.status_code == 200 + body = response.json() + error = body["error"] + assert error["code"] == -32601 + assert error["data"]["type"] == "METHOD_NOT_SUPPORTED" + assert error["data"]["method"] == "opencode.sessions.shell" + assert "opencode.sessions.shell" not in error["data"]["supported_methods"] diff --git a/tests/jsonrpc/test_opencode_session_extension_prompt_async.py b/tests/jsonrpc/test_opencode_session_extension_prompt_async.py index ad85b50..4bb9796 100644 --- a/tests/jsonrpc/test_opencode_session_extension_prompt_async.py +++ b/tests/jsonrpc/test_opencode_session_extension_prompt_async.py @@ -443,7 +443,7 @@ async def session_prompt_async(self, session_id: str, request: dict, *, director @pytest.mark.asyncio async def test_session_prompt_async_release_failure_does_not_override_response(monkeypatch, caplog): import opencode_a2a.server.application as app_module - from opencode_a2a.execution.executor import OpencodeAgentExecutor + from opencode_a2a.execution.session_manager import SessionManager class NetworkErrorPromptAsyncClient(DummyOpencodeUpstreamClient): async def session_prompt_async(self, session_id: str, request: dict, *, directory=None): @@ -451,15 +451,13 @@ async def session_prompt_async(self, session_id: str, request: dict, *, director req = httpx.Request("POST", "http://opencode/session/s-1/prompt_async") raise httpx.ConnectError("network down", request=req) - async def _release_raises( - self: OpencodeAgentExecutor, *, identity: str, session_id: str - ) -> None: + async def _release_raises(self: SessionManager, *, identity: str, session_id: str) -> None: del identity, session_id raise RuntimeError("release failed") caplog.set_level(logging.ERROR) monkeypatch.setattr(app_module, "OpencodeUpstreamClient", NetworkErrorPromptAsyncClient) - monkeypatch.setattr(OpencodeAgentExecutor, "release_session_for_control", _release_raises) + monkeypatch.setattr(SessionManager, "release_preferred_session_claim", _release_raises) app = app_module.create_app( make_settings(a2a_bearer_token="t-1", a2a_log_payloads=False, **_BASE_SETTINGS) ) diff --git a/tests/profile/test_profile_runtime.py b/tests/profile/test_profile_runtime.py index 6708e8a..775f176 100644 --- a/tests/profile/test_profile_runtime.py +++ b/tests/profile/test_profile_runtime.py @@ -115,3 +115,20 @@ def test_profile_runtime_uses_conservative_execution_environment_defaults() -> N "outside_workspace": "unknown", }, } + + +def test_profile_runtime_disables_shell_when_policy_is_read_only() -> None: + settings = make_settings( + a2a_bearer_token="test-token", + a2a_enable_session_shell=True, + a2a_sandbox_mode="read-only", + a2a_write_access_scope="workspace_only", + ) + + profile = build_runtime_profile(settings) + + assert profile.runtime_features_dict()["session_shell"] == { + "enabled": False, + "availability": "disabled", + "toggle": "A2A_ENABLE_SESSION_SHELL", + } diff --git a/tests/server/test_agent_card.py b/tests/server/test_agent_card.py index 76e2c8b..d540ff6 100644 --- a/tests/server/test_agent_card.py +++ b/tests/server/test_agent_card.py @@ -390,3 +390,22 @@ def test_agent_card_skills_hide_shell_when_disabled_by_default() -> None: assert all("opencode.sessions.shell" not in example for example in session_skill.examples) assert "provider-private" in provider_skill.tags assert any("opencode.providers.list" in example for example in provider_skill.examples) + + +def test_agent_card_hides_shell_when_policy_disables_it() -> None: + card = build_agent_card( + make_settings( + a2a_bearer_token="test-token", + a2a_enable_session_shell=True, + a2a_sandbox_mode="read-only", + a2a_write_access_scope="workspace_only", + ) + ) + ext_by_uri = {ext.uri: ext for ext in card.capabilities.extensions or []} + + session_query = ext_by_uri[SESSION_QUERY_EXTENSION_URI] + compatibility = ext_by_uri[COMPATIBILITY_PROFILE_EXTENSION_URI] + + assert "shell" not in session_query.params["methods"] + assert "opencode.sessions.shell" not in session_query.params["method_contracts"] + assert compatibility.params["runtime_features"]["session_shell"]["availability"] == "disabled" diff --git a/tests/server/test_transport_contract.py b/tests/server/test_transport_contract.py index e15612d..065bbe2 100644 --- a/tests/server/test_transport_contract.py +++ b/tests/server/test_transport_contract.py @@ -1,5 +1,6 @@ import logging -from unittest.mock import MagicMock +import types +from unittest.mock import AsyncMock, MagicMock import httpx import pytest @@ -498,18 +499,12 @@ async def execute(self, _context, _event_queue) -> None: # noqa: ANN001 async def cancel(self, _context, _event_queue) -> None: # noqa: ANN001 raise NotImplementedError - def resolve_directory_for_control(self, requested: str | None) -> str | None: - return requested - - async def claim_session_for_control(self, *, identity: str, session_id: str) -> bool: - del identity, session_id - return False - - async def finalize_session_for_control(self, *, identity: str, session_id: str) -> None: - del identity, session_id - - async def release_session_for_control(self, *, identity: str, session_id: str) -> None: - del identity, session_id + _sandbox_policy = types.SimpleNamespace(resolve_directory=lambda requested, **_: requested) + _session_manager = types.SimpleNamespace( + claim_preferred_session=AsyncMock(return_value=False), + finalize_session_claim=AsyncMock(), + release_preferred_session_claim=AsyncMock(), + ) monkeypatch.setattr(app_module, "OpencodeUpstreamClient", DummyChatOpencodeUpstreamClient) monkeypatch.setattr(app_module, "OpencodeAgentExecutor", _CapturingExecutor) @@ -552,8 +547,8 @@ def test_create_app_propagates_outbound_client_settings(monkeypatch) -> None: assert settings.use_client_preference is True assert settings.bearer_token == "peer-token" assert settings.supported_transports == ("HTTP+JSON", "JSONRPC") - assert client_manager.cache_ttl_seconds == 321.0 - assert client_manager.cache_maxsize == 12 + assert client_manager._cache_ttl_seconds == 321.0 # noqa: SLF001 + assert client_manager._cache_maxsize == 12 # noqa: SLF001 def test_create_app_requires_control_guard_hooks(monkeypatch) -> None: @@ -577,7 +572,13 @@ def __init__( session_cache_maxsize, a2a_client_manager, ) - self.claim_session_for_control = None + self._session_manager = types.SimpleNamespace( + finalize_session_claim=AsyncMock(), + release_preferred_session_claim=AsyncMock(), + ) + self._sandbox_policy = types.SimpleNamespace( + resolve_directory=lambda requested, **_: requested + ) async def execute(self, _context, _event_queue) -> None: # noqa: ANN001 raise NotImplementedError @@ -585,15 +586,6 @@ async def execute(self, _context, _event_queue) -> None: # noqa: ANN001 async def cancel(self, _context, _event_queue) -> None: # noqa: ANN001 raise NotImplementedError - def resolve_directory_for_control(self, requested: str | None) -> str | None: - return requested - - async def finalize_session_for_control(self, *, identity: str, session_id: str) -> None: - del identity, session_id - - async def release_session_for_control(self, *, identity: str, session_id: str) -> None: - del identity, session_id - monkeypatch.setattr(app_module, "OpencodeUpstreamClient", DummyChatOpencodeUpstreamClient) monkeypatch.setattr(app_module, "OpencodeAgentExecutor", _BrokenExecutor)