diff --git a/openhands-sdk/openhands/sdk/__init__.py b/openhands-sdk/openhands/sdk/__init__.py index d7946ab4ae..e93ec4a0b6 100644 --- a/openhands-sdk/openhands/sdk/__init__.py +++ b/openhands-sdk/openhands/sdk/__init__.py @@ -49,9 +49,14 @@ ) from openhands.sdk.plugin import Plugin from openhands.sdk.settings import ( + ACP_CODEX_SUBSCRIPTION_AUTH_SECRET, + ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET, ACP_PROVIDERS, + ACP_SUBSCRIPTION_AUTH_SECRETS, ACPAgentSettings, + ACPFileSecretSpec, ACPProviderInfo, + ACPSubscriptionAuthSecretInfo, AgentSettings, AgentSettingsBase, AgentSettingsConfig, @@ -191,9 +196,14 @@ def __getattr__(name: str) -> Any: "CondenserSettings", "ConversationSettings", "VerificationSettings", + "ACP_CODEX_SUBSCRIPTION_AUTH_SECRET", + "ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET", + "ACPFileSecretSpec", "ACP_PROVIDERS", "ACPAgentSettings", "ACPProviderInfo", + "ACPSubscriptionAuthSecretInfo", + "ACP_SUBSCRIPTION_AUTH_SECRETS", "AgentSettings", "AgentSettingsBase", "AgentSettingsConfig", diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index 744c44fb34..ea554fd2ee 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/openhands-sdk/openhands/sdk/agent/acp_agent.py @@ -19,10 +19,12 @@ import asyncio import json import os +import re +import tempfile import threading import time import uuid -from collections.abc import Generator +from collections.abc import Generator, Mapping from pathlib import Path from typing import TYPE_CHECKING, Any, Literal @@ -59,6 +61,8 @@ from openhands.sdk.observability.laminar import maybe_init_laminar, observe from openhands.sdk.secret import SecretSource from openhands.sdk.settings.acp_providers import ( + ACP_PROVIDERS, + ACPFileSecretSpec, build_session_model_meta, detect_acp_provider_by_agent_name, ) @@ -122,6 +126,60 @@ "CLAUDE_CONFIG_DIR": frozenset({"ANTHROPIC_API_KEY", "ANTHROPIC_BASE_URL"}), } + +def _safe_file_secret_dir_name(index: int, secret_name: str) -> str: + """Return a stable tempdir child name for one materialised secret.""" + safe_name = re.sub(r"[^A-Za-z0-9_.-]+", "_", secret_name).strip("._") + return f"{index:02d}-{safe_name or 'secret'}" + + +def _render_file_secret_template( + template: str, + *, + root: Path, + directory: Path, + file: Path, +) -> str: + """Expand supported placeholders in an ACP file-secret env var or arg.""" + return ( + template.replace("{root}", str(root)) + .replace("{dir}", str(directory)) + .replace("{file}", str(file)) + ) + + +def _write_secret_file(path: Path, value: str) -> None: + """Create or replace a secret file without a world-readable intermediate.""" + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC + fd = os.open(path, flags, 0o600) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + fd = -1 + f.write(value) + finally: + if fd >= 0: + os.close(fd) + path.chmod(0o600) + + +def _default_acp_file_secret_specs() -> list[ACPFileSecretSpec]: + """Return built-in ACP file-secret mappings for direct ``ACPAgent`` use.""" + return [ + spec for provider in ACP_PROVIDERS.values() for spec in provider.file_secrets + ] + + +def _matching_file_secret_names( + secrets: Mapping[str, Any] | None, + specs: list[ACPFileSecretSpec], +) -> set[str]: + """Return AgentContext secret names consumed by ACP file-secret specs.""" + if not secrets or not specs: + return set() + configured_names = {spec.secret_name for spec in specs} + return set(secrets) & configured_names + + # Limit for asyncio.StreamReader buffers used by the ACP subprocess pipes. # The default (64 KiB) is too small for session_update notifications that # carry large tool-call outputs (e.g. file contents, test results). When @@ -176,6 +234,14 @@ def _make_dummy_llm() -> LLM: _CHATGPT_AUTH_PATH = Path(".codex") / "auth.json" +def _has_chatgpt_auth_file(env: dict[str, str]) -> bool: + """Return whether Codex ChatGPT auth is available on disk.""" + codex_home = env.get("CODEX_HOME") + if codex_home: + return (Path(codex_home) / "auth.json").is_file() + return (Path.home() / _CHATGPT_AUTH_PATH).is_file() + + def _select_auth_method( auth_methods: list[Any], env: dict[str, str], @@ -191,9 +257,8 @@ def _select_auth_method( """ method_ids = {m.id for m in auth_methods} # Prefer ChatGPT subscription login when the auth file is present. - if "chatgpt" in method_ids: - if (Path.home() / _CHATGPT_AUTH_PATH).is_file(): - return "chatgpt" + if "chatgpt" in method_ids and _has_chatgpt_auth_file(env): + return "chatgpt" # Fall back to explicit API key env vars. for method_id, env_var in _AUTH_METHOD_ENV_MAP.items(): if method_id in method_ids and env_var in env: @@ -690,6 +755,13 @@ class ACPAgent(AgentBase): default_factory=dict, description="Additional environment variables for the ACP server process", ) + acp_file_secrets: list[ACPFileSecretSpec] = Field( + default_factory=_default_acp_file_secret_specs, + description=( + "AgentContext secret values to materialise as credential files for " + "the ACP server process." + ), + ) @field_serializer("acp_env", when_used="always") def _serialize_acp_env(self, value: dict[str, str], info): @@ -759,6 +831,13 @@ def model_post_init(self, __context: object) -> None: _suffix_install_state: str = PrivateAttr(default="unused") _installed_suffix: str | None = PrivateAttr(default=None) + # Per-agent temp directory holding any file-content secrets materialised + # from ``AgentContext.secrets`` via ``acp_file_secrets``. Created lazily + # on the first secret that needs it; cleaned up in :meth:`close`. + _file_secrets_tempdir: Any = PrivateAttr( + default=None + ) # tempfile.TemporaryDirectory | None + # -- Helpers ----------------------------------------------------------- def _record_usage( @@ -972,10 +1051,141 @@ def _render_suffix(self, state: ConversationState) -> str | None: if not self.agent_context: return None secret_infos = state.secret_registry.get_secret_infos() - return self.agent_context.to_acp_prompt_context( + prompt_context = self.agent_context + file_secret_names = _matching_file_secret_names( + self.agent_context.secrets, + self.acp_file_secrets, + ) + if file_secret_names: + prompt_context = self.agent_context.model_copy( + update={ + "secrets": { + name: secret + for name, secret in (self.agent_context.secrets or {}).items() + if name not in file_secret_names + } + } + ) + secret_infos = [ + info + for info in secret_infos + if info.get("name") not in file_secret_names + ] + return prompt_context.to_acp_prompt_context( additional_secret_infos=secret_infos ) + def _materialise_file_secrets( + self, + env: dict[str, str], + ) -> tuple[dict[str, Any], list[str]]: + """Materialise configured file-content secrets onto the filesystem. + + Walks ``self.agent_context.secrets`` for names matched by + ``self.acp_file_secrets``. Each matching secret has its value written + into ``self._file_secrets_tempdir``. The generated file path can then + be exposed to the subprocess through templated env vars or appended + command args from the matching :class:`ACPFileSecretSpec`. + + File-secrets are dropped from the returned dict so the regular env-var + injection loop does not also export their potentially large payloads. + + Returns ``(remaining_secrets, extra_args)``. ``remaining_secrets`` is + the subset of ``agent_context.secrets`` that should still flow through + the standard env-var injection path; ``extra_args`` should be appended + to the ACP subprocess command. + """ + if not self.agent_context or not self.agent_context.secrets: + return {}, [] + if not self.acp_file_secrets: + return dict(self.agent_context.secrets), [] + + remaining: dict[str, Any] = {} + specs_by_name: dict[str, list[ACPFileSecretSpec]] = {} + for spec in self.acp_file_secrets: + specs_by_name.setdefault(spec.secret_name, []).append(spec) + + pending: list[tuple[str, ACPFileSecretSpec, str]] = [] + for name, secret in self.agent_context.secrets.items(): + specs = specs_by_name.get(name) + if not specs: + remaining[name] = secret + continue + value = ( + secret.get_value() if isinstance(secret, SecretSource) else str(secret) + ) + if not value: + logger.warning( + "Configured ACP file-secret %r has an empty value; " + "skipping materialisation", + name, + ) + continue + for spec in specs: + pending.append((name, spec, value)) + + if not pending: + return remaining, [] + + if self._file_secrets_tempdir is None: + self._file_secrets_tempdir = tempfile.TemporaryDirectory( + prefix="acp-file-secrets-" + ) + base = Path(self._file_secrets_tempdir.name) + extra_args: list[str] = [] + + for index, (name, spec, value) in enumerate(pending): + # Each file-secret gets its own subdirectory so env-var-to-dir + # handlers (e.g. CODEX_HOME) don't inadvertently expose other + # file-secrets' files as siblings. + target_dir = base / _safe_file_secret_dir_name(index, name) + target_dir.mkdir(mode=0o700, parents=True, exist_ok=True) + target_dir.chmod(0o700) + target_path = target_dir / spec.relative_path + target_path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + for parent in (target_path.parent, *target_path.parent.parents): + if parent == target_dir.parent: + break + parent.chmod(0o700) + _write_secret_file(target_path, value) + + for env_name, template in spec.env.items(): + if env_name in self.acp_env: + logger.info( + "Preserving configured ACP env var %r over file-secret %r", + env_name, + name, + ) + continue + if env_name in env and not spec.overwrite_env: + raise ValueError( + f"ACP file-secret {name!r} would overwrite existing " + f"environment variable {env_name!r}" + ) + env[env_name] = _render_file_secret_template( + template, + root=base, + directory=target_dir, + file=target_path, + ) + + extra_args.extend( + _render_file_secret_template( + arg, + root=base, + directory=target_dir, + file=target_path, + ) + for arg in spec.args + ) + logger.info( + "Materialised ACP file-secret %r → %s", + name, + target_path, + ) + + return remaining, extra_args + def _start_acp_server(self, state: ConversationState) -> None: """Start the ACP subprocess and initialize the session.""" client = _OpenHandsACPBridge() @@ -985,20 +1195,26 @@ def _start_acp_server(self, state: ConversationState) -> None: env = default_environment() env.update(os.environ) env.update(self.acp_env) - # Inject secrets from agent_context. acp_env entries take precedence - # (already set above), so we only fill keys not already present. + # Materialise configured file-secrets (e.g. Codex auth.json, Gemini + # GAC JSON, or custom ACP server credentials) to local files and set + # their controlling env vars on ``env``. Returned dict is + # ``agent_context.secrets`` minus the file-secrets, which we now feed + # through the regular env-var injection loop below. + regular_secrets, file_secret_args = self._materialise_file_secrets(env) + # Inject the remaining (plain) secrets from agent_context. acp_env + # entries take precedence (already set above), but agent_context + # secrets intentionally override inherited os.environ values. # SecretSource.get_value() is synchronous; calling it here is safe # because _start_acp_server is a regular (non-async) method. - if self.agent_context and self.agent_context.secrets: - for name, secret in self.agent_context.secrets.items(): - if name not in env: - value = ( - secret.get_value() - if isinstance(secret, SecretSource) - else str(secret) - ) - if value: - env[name] = value + for name, secret in regular_secrets.items(): + if name not in self.acp_env: + value = ( + secret.get_value() + if isinstance(secret, SecretSource) + else str(secret) + ) + if value: + env[name] = value # Strip CLAUDECODE so nested Claude Code instances don't refuse to start env.pop("CLAUDECODE", None) @@ -1011,7 +1227,7 @@ def _start_acp_server(self, state: ConversationState) -> None: env.pop(conflict, None) command = self.acp_command[0] - args = list(self.acp_command[1:]) + list(self.acp_args) + args = list(self.acp_command[1:]) + list(self.acp_args) + file_secret_args working_dir = str(state.workspace.working_dir) @@ -1603,6 +1819,16 @@ def _cleanup(self) -> None: logger.debug("Error closing executor: %s", e) self._executor = None + # Remove any file-secrets we materialised for this agent. Done after + # subprocess + executor teardown so we don't yank credential files + # out from under a still-running child. + if self._file_secrets_tempdir is not None: + try: + self._file_secrets_tempdir.cleanup() + except Exception as e: + logger.debug("Error cleaning up ACP file-secrets tempdir: %s", e) + self._file_secrets_tempdir = None + def __del__(self) -> None: try: self.close() diff --git a/openhands-sdk/openhands/sdk/settings/__init__.py b/openhands-sdk/openhands/sdk/settings/__init__.py index 1f6b33db66..3e30caeaaa 100644 --- a/openhands-sdk/openhands/sdk/settings/__init__.py +++ b/openhands-sdk/openhands/sdk/settings/__init__.py @@ -3,8 +3,13 @@ from typing import TYPE_CHECKING, Any from .acp_providers import ( + ACP_CODEX_SUBSCRIPTION_AUTH_SECRET, + ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET, ACP_PROVIDERS, + ACP_SUBSCRIPTION_AUTH_SECRETS, + ACPFileSecretSpec, ACPProviderInfo, + ACPSubscriptionAuthSecretInfo, build_session_model_meta, detect_acp_provider_by_agent_name, get_acp_provider, @@ -75,8 +80,13 @@ } __all__ = [ + "ACP_CODEX_SUBSCRIPTION_AUTH_SECRET", + "ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET", + "ACPFileSecretSpec", "ACP_PROVIDERS", "ACPProviderInfo", + "ACPSubscriptionAuthSecretInfo", + "ACP_SUBSCRIPTION_AUTH_SECRETS", "build_session_model_meta", "AGENT_SETTINGS_SCHEMA_VERSION", "CONVERSATION_SETTINGS_SCHEMA_VERSION", diff --git a/openhands-sdk/openhands/sdk/settings/acp_providers.py b/openhands-sdk/openhands/sdk/settings/acp_providers.py index 44c5bc0f3a..85f539b968 100644 --- a/openhands-sdk/openhands/sdk/settings/acp_providers.py +++ b/openhands-sdk/openhands/sdk/settings/acp_providers.py @@ -13,6 +13,10 @@ used by ``ACPAgent`` to auto-detect mode / protocol - ``supports_set_session_model`` whether to use the ``set_session_model`` protocol call (vs ``_meta``) for model selection +- ``file_secrets`` provider-default credential files to materialise + from ``AgentContext.secrets`` +- ``subscription_auth_secret`` UX metadata that explains which custom secret + enables subscription-style auth for the provider Callers outside the SDK (e.g. ``openhands-agent-server``, the ``OpenHands`` frontend) can import :data:`ACP_PROVIDERS` and :func:`get_acp_provider` instead @@ -23,9 +27,114 @@ from collections.abc import Mapping from dataclasses import dataclass, field +from pathlib import Path from types import MappingProxyType from typing import Any +from pydantic import BaseModel, ConfigDict, Field, field_validator + + +class ACPFileSecretSpec(BaseModel): + """Declarative mapping from an ``AgentContext`` secret to a credential file. + + The SDK writes ``secret_name`` into ``relative_path`` under a private + per-agent temp directory, then expands ``{file}``, ``{dir}``, and ``{root}`` + placeholders in ``env`` and ``args`` so the ACP subprocess can find it. + """ + + model_config = ConfigDict(frozen=True) + + secret_name: str = Field( + ..., + min_length=1, + description="Name of the AgentContext secret whose value should be written.", + ) + relative_path: str = Field( + ..., + min_length=1, + description=( + "Relative path for the generated credential file inside an " + "SDK-owned temp directory." + ), + ) + env: dict[str, str] = Field( + default_factory=dict, + description=( + "Environment variables to set on the ACP subprocess. Values may use " + "{file}, {dir}, and {root} placeholders." + ), + ) + args: list[str] = Field( + default_factory=list, + description=( + "Extra command arguments to append when this secret is materialised. " + "Values may use {file}, {dir}, and {root} placeholders." + ), + ) + overwrite_env: bool = Field( + default=True, + description=( + "Whether generated env vars may replace values already present in " + "the ACP subprocess environment." + ), + ) + + @field_validator("relative_path") + @classmethod + def _validate_relative_path(cls, value: str) -> str: + path = Path(value) + if path.is_absolute() or ".." in path.parts or path == Path("."): + raise ValueError( + "relative_path must be a non-empty relative path under the " + "SDK-created temp directory" + ) + return value + + +class ACPSubscriptionAuthSecretInfo(BaseModel): + """UX-facing instructions for configuring subscription-style ACP auth. + + Clients can render this metadata to tell users which custom secret to + create, what value it should contain, and how the SDK will use it. + """ + + model_config = ConfigDict(frozen=True) + + secret_name: str = Field( + ..., + min_length=1, + description="Name of the custom secret users should create.", + ) + label: str = Field( + ..., + min_length=1, + description="Human-readable label for the secret.", + ) + description: str = Field( + ..., + min_length=1, + description="Short explanation suitable for settings UIs.", + ) + value_description: str = Field( + ..., + min_length=1, + description="Description of the expected secret value format.", + ) + setup_steps: tuple[str, ...] = Field( + default_factory=tuple, + description="Ordered setup instructions for users.", + ) + extract_commands: tuple[str, ...] = Field( + default_factory=tuple, + description="Optional local commands that print or locate the secret value.", + ) + file_secret: ACPFileSecretSpec | None = Field( + default=None, + description=( + "File-secret mapping the SDK uses when this custom secret is present." + ), + ) + @dataclass(frozen=True) class ACPProviderInfo: @@ -91,6 +200,31 @@ class ACPProviderInfo: - ``None`` — codex-acp, gemini-cli """ + file_secrets: tuple[ACPFileSecretSpec, ...] = field( + default_factory=tuple, + compare=False, + ) + """Credential file mappings enabled by default for this built-in provider.""" + + subscription_auth_secret: ACPSubscriptionAuthSecretInfo | None = field( + default=None, + compare=False, + ) + """UX-facing custom-secret instructions for subscription-style auth.""" + + +_CODEX_AUTH_JSON_FILE_SECRET = ACPFileSecretSpec( + secret_name="CODEX_AUTH_JSON", + relative_path="auth.json", + env={"CODEX_HOME": "{dir}"}, +) + +_GEMINI_APPLICATION_CREDENTIALS_FILE_SECRET = ACPFileSecretSpec( + secret_name="GOOGLE_APPLICATION_CREDENTIALS_JSON", + relative_path="gcloud-credentials.json", + env={"GOOGLE_APPLICATION_CREDENTIALS": "{file}"}, +) + ACP_PROVIDERS: Mapping[str, ACPProviderInfo] = MappingProxyType( { @@ -115,6 +249,25 @@ class ACPProviderInfo: agent_name_patterns=("codex-acp",), supports_set_session_model=True, session_meta_key=None, + file_secrets=(_CODEX_AUTH_JSON_FILE_SECRET,), + subscription_auth_secret=ACPSubscriptionAuthSecretInfo( + secret_name="CODEX_AUTH_JSON", + label="Codex ChatGPT auth.json", + description=( + "Create this custom secret to let codex-acp use a ChatGPT " + "Plus/Pro/Team subscription login instead of an API key." + ), + value_description=( + "The complete JSON contents of the Codex CLI auth file." + ), + setup_steps=( + "Run the Codex CLI login flow on a trusted machine.", + "Create a custom secret named CODEX_AUTH_JSON.", + "Paste the complete contents of ~/.codex/auth.json as the value.", + ), + extract_commands=("cat ~/.codex/auth.json",), + file_secret=_CODEX_AUTH_JSON_FILE_SECRET, + ), ), "gemini-cli": ACPProviderInfo( key="gemini-cli", @@ -126,12 +279,55 @@ class ACPProviderInfo: agent_name_patterns=("gemini-cli",), supports_set_session_model=True, session_meta_key=None, + file_secrets=(_GEMINI_APPLICATION_CREDENTIALS_FILE_SECRET,), + subscription_auth_secret=ACPSubscriptionAuthSecretInfo( + secret_name="GOOGLE_APPLICATION_CREDENTIALS_JSON", + label="Gemini CLI Google credentials JSON", + description=( + "Create this custom secret to let gemini-cli authenticate " + "non-interactively with Google credentials that the SDK " + "writes to a file before starting ACP." + ), + value_description=( + "A Google Application Default Credentials or service-account " + "JSON document." + ), + setup_steps=( + "Create or locate a Google credentials JSON file usable by " + "Gemini CLI.", + "Create a custom secret named GOOGLE_APPLICATION_CREDENTIALS_JSON.", + "Paste the complete JSON file contents as the value.", + "Set any required Gemini CLI env vars such as " + "GOOGLE_GENAI_USE_VERTEXAI, GOOGLE_CLOUD_PROJECT, and " + "GOOGLE_CLOUD_LOCATION in ACP environment variables.", + ), + extract_commands=("cat /path/to/google-credentials.json",), + file_secret=_GEMINI_APPLICATION_CREDENTIALS_FILE_SECRET, + ), ), } ) """Read-only registry of built-in ACP providers keyed by ``acp_server`` value.""" +ACP_SUBSCRIPTION_AUTH_SECRETS: Mapping[str, ACPSubscriptionAuthSecretInfo] = ( + MappingProxyType( + { + key: info.subscription_auth_secret + for key, info in ACP_PROVIDERS.items() + if info.subscription_auth_secret is not None + } + ) +) +"""Read-only subscription-auth secret instructions keyed by ACP provider.""" + +ACP_CODEX_SUBSCRIPTION_AUTH_SECRET = ACP_SUBSCRIPTION_AUTH_SECRETS["codex"] +"""Custom-secret instructions for Codex ChatGPT subscription auth.""" + +ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET = ACP_SUBSCRIPTION_AUTH_SECRETS["gemini-cli"] +"""Custom-secret instructions for Gemini CLI Google credential auth.""" + + def get_acp_provider(key: str) -> ACPProviderInfo | None: """Return the :class:`ACPProviderInfo` for ``key``, or ``None`` if unknown.""" return ACP_PROVIDERS.get(key) diff --git a/openhands-sdk/openhands/sdk/settings/model.py b/openhands-sdk/openhands/sdk/settings/model.py index 6fff5b7f45..19d0bf6f0b 100644 --- a/openhands-sdk/openhands/sdk/settings/model.py +++ b/openhands-sdk/openhands/sdk/settings/model.py @@ -48,7 +48,7 @@ from openhands.sdk.utils.redact import sanitize_dict from openhands.sdk.workspace import LocalWorkspace -from .acp_providers import ACPProviderInfo, get_acp_provider +from .acp_providers import ACPFileSecretSpec, ACPProviderInfo, get_acp_provider from .metadata import ( SETTINGS_METADATA_KEY, SETTINGS_SECTION_METADATA_KEY, @@ -311,7 +311,7 @@ def _default_llm_settings() -> LLM: _RequestT = TypeVar("_RequestT") -AGENT_SETTINGS_SCHEMA_VERSION = 3 +AGENT_SETTINGS_SCHEMA_VERSION = 4 CONVERSATION_SETTINGS_SCHEMA_VERSION = 1 @@ -458,6 +458,18 @@ def _migrate_agent_settings_v2_to_v3(payload: dict[str, Any]) -> dict[str, Any]: return migrated +def _migrate_agent_settings_v3_to_v4(payload: dict[str, Any]) -> dict[str, Any]: + """Bump for persisted ACP file-secret settings. + + The field itself validates without structural migration, but the schema + version must advance so older SDKs reject payloads that may contain + ``acp_file_secrets`` instead of silently dropping them. + """ + migrated = dict(payload) + migrated["schema_version"] = 4 + return migrated + + def _migrate_conversation_settings_v0_to_v1( payload: dict[str, Any], ) -> dict[str, Any]: @@ -470,6 +482,7 @@ def _migrate_conversation_settings_v0_to_v1( 0: _migrate_agent_settings_v0_to_v1, 1: _migrate_agent_settings_v1_to_v2, 2: _migrate_agent_settings_v2_to_v3, + 3: _migrate_agent_settings_v3_to_v4, } _CONVERSATION_SETTINGS_MIGRATIONS: dict[int, PersistedSettingsMigrator] = { 0: _migrate_conversation_settings_v0_to_v1, @@ -1054,6 +1067,25 @@ def _serialize_acp_env(self, value: dict[str, str], info): """Mask ``acp_env`` values via :func:`serialize_secret`.""" return {k: serialize_secret(SecretStr(v), info) for k, v in value.items()} + acp_file_secrets: list[ACPFileSecretSpec] = Field( + default_factory=list, + description=( + "Custom AgentContext secrets to write as files before launching " + "the ACP subprocess. Built-in providers may add their own defaults." + ), + json_schema_extra={ + SETTINGS_METADATA_KEY: SettingsFieldMetadata( + label="ACP file secrets", + prominence=SettingProminence.MINOR, + ).model_dump(), + SETTINGS_SECTION_METADATA_KEY: SettingsSectionMetadata( + key="acp", + label="ACP (Agent Client Protocol)", + variant="acp", + ).model_dump(), + }, + ) + acp_model: str | None = Field( default=None, description=( @@ -1127,7 +1159,8 @@ def _serialize_acp_env(self, value: dict[str, str], info): default=None, description=( "Prompt-only context for the ACP server. Secrets are injected into " - "the subprocess environment by ACPAgent." + "the subprocess environment or configured credential files by " + "ACPAgent." ), ) @@ -1200,6 +1233,20 @@ def resolve_acp_env(self) -> dict[str, str]: **dict(self.acp_env), } + def resolve_acp_file_secrets(self) -> list[ACPFileSecretSpec]: + """Return file-secret mappings for the effective ACP server. + + Built-in providers contribute their credential-file conventions from + the provider registry. User-supplied entries are appended so custom ACP + servers can define their own file handling, and known providers can add + extra credential files without changing SDK code. + """ + specs: list[ACPFileSecretSpec] = [] + if self.provider_info is not None: + specs.extend(self.provider_info.file_secrets) + specs.extend(self.acp_file_secrets) + return specs + def resolve_acp_command(self) -> list[str]: """Return the effective subprocess command for this settings block. @@ -1236,6 +1283,7 @@ def create_agent(self) -> ACPAgent: acp_command=self.resolve_acp_command(), acp_args=list(self.acp_args), acp_env=self.resolve_acp_env(), + acp_file_secrets=self.resolve_acp_file_secrets(), acp_model=self.acp_model, acp_session_mode=self.acp_session_mode, acp_prompt_timeout=self.acp_prompt_timeout, diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py index 4d19773e85..813ae586bd 100644 --- a/tests/sdk/agent/test_acp_agent.py +++ b/tests/sdk/agent/test_acp_agent.py @@ -5,6 +5,7 @@ import asyncio import json import uuid +from pathlib import Path from typing import Any from unittest.mock import AsyncMock, MagicMock, patch @@ -34,6 +35,7 @@ SystemPromptEvent, ) from openhands.sdk.llm import ImageContent, Message, TextContent +from openhands.sdk.settings import ACP_PROVIDERS, ACPFileSecretSpec from openhands.sdk.skills import KeywordTrigger, Skill from openhands.sdk.tool.builtins.finish import FinishAction from openhands.sdk.utils.pydantic_secrets import REDACTED_SECRET_VALUE @@ -49,6 +51,14 @@ def _make_agent(**kwargs) -> ACPAgent: return ACPAgent(acp_command=["echo", "test"], **kwargs) +def _file_secret_specs_for(*provider_keys: str) -> list[ACPFileSecretSpec]: + return [ + spec + for provider_key in provider_keys + for spec in ACP_PROVIDERS[provider_key].file_secrets + ] + + def _make_state(tmp_path) -> ConversationState: agent = _make_agent() workspace = LocalWorkspace(working_dir=str(tmp_path)) @@ -361,6 +371,29 @@ def test_agent_context_to_acp_prompt_context_includes_secrets(self): assert "GitHub authentication token" in prompt assert "$MY_API_KEY" in prompt + def test_render_suffix_omits_file_secret_payload_env_vars(self, tmp_path): + """File-backed secrets should not be advertised as raw env vars.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + acp_file_secrets=_file_secret_specs_for("codex"), + agent_context=AgentContext( + secrets={ + "CODEX_AUTH_JSON": StaticSecret(value=SecretStr("{}")), + "GITHUB_TOKEN": StaticSecret(value=SecretStr("ghp_secret")), + }, + current_datetime=None, + ), + ) + + prompt = agent._render_suffix(_make_state(tmp_path)) + + assert prompt is not None + assert "$CODEX_AUTH_JSON" not in prompt + assert "$GITHUB_TOKEN" in prompt + def test_agent_context_to_acp_prompt_context_includes_legacy_repo_skills(self): context = AgentContext( skills=[ @@ -2736,6 +2769,31 @@ def test_chatgpt_auth_file(self, tmp_path): with patch("openhands.sdk.agent.acp_agent.Path.home", return_value=tmp_path): assert _select_auth_method(methods, {}) == "chatgpt" + def test_chatgpt_auth_file_from_codex_home_env(self, tmp_path): + methods = [self._make_auth_method("chatgpt")] + codex_home = tmp_path / "codex-home" + codex_home.mkdir() + (codex_home / "auth.json").write_text("{}", encoding="utf-8") + + assert _select_auth_method(methods, {"CODEX_HOME": str(codex_home)}) == ( + "chatgpt" + ) + + def test_codex_home_without_auth_file_disables_home_fallback(self, tmp_path): + methods = [ + self._make_auth_method("chatgpt"), + self._make_auth_method("openai-api-key"), + ] + codex_home = tmp_path / "codex-home" + codex_home.mkdir() + home_auth_dir = tmp_path / ".codex" + home_auth_dir.mkdir() + (home_auth_dir / "auth.json").write_text("{}", encoding="utf-8") + + env = {"CODEX_HOME": str(codex_home), "OPENAI_API_KEY": "sk-test"} + with patch("openhands.sdk.agent.acp_agent.Path.home", return_value=tmp_path): + assert _select_auth_method(methods, env) == "openai-api-key" + def test_empty_auth_methods(self): assert _select_auth_method([], {}) is None @@ -3573,7 +3631,12 @@ def _make_conn(): return conn @staticmethod - def _run_start_capturing_env(agent, tmp_path) -> dict: + def _run_start_capturing_env( + agent, + tmp_path, + capture_args: list[str] | None = None, + extra_os_env: dict[str, str] | None = None, + ) -> dict: """Run _start_acp_server and return the env dict passed to the subprocess.""" from contextlib import ExitStack @@ -3587,6 +3650,8 @@ def _run_start_capturing_env(agent, tmp_path) -> dict: mock_process.stdout = MagicMock() async def _fake_create_subprocess_exec(*_args, env=None, **_kwargs): + if capture_args is not None: + capture_args[:] = [str(arg) for arg in _args] captured.update(env or {}) return mock_process @@ -3621,6 +3686,8 @@ async def _fake_filter(_src, _dst): return_value=MagicMock(), ) ) + if extra_os_env is not None: + stack.enter_context(patch.dict("os.environ", extra_os_env, clear=False)) agent._start_acp_server(state) return captured @@ -3644,6 +3711,27 @@ def test_static_secret_injected_into_subprocess_env(self, tmp_path): env = self._run_start_capturing_env(agent, tmp_path) assert env.get("GITHUB_TOKEN") == "ghp_test123" + def test_agent_context_secret_overrides_inherited_env(self, tmp_path): + """A runtime secret wins over the same key inherited from os.environ.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + agent_context=AgentContext( + secrets={ + "GITHUB_TOKEN": StaticSecret(value=SecretStr("ghp_agent_secret")) + } + ) + ) + env = self._run_start_capturing_env( + agent, + tmp_path, + extra_os_env={"GITHUB_TOKEN": "ghp_inherited"}, + ) + + assert env.get("GITHUB_TOKEN") == "ghp_agent_secret" + def test_acp_env_takes_precedence_over_agent_context_secret(self, tmp_path): """An explicit acp_env entry wins over the same key in agent_context.secrets.""" from pydantic import SecretStr @@ -3686,6 +3774,297 @@ def test_empty_string_secret_not_injected(self, tmp_path): assert "EMPTY_SECRET" not in env +class TestACPFileSecretMaterialisation: + """Configured secret names get written to disk before the subprocess spawns. + + Some ACP servers authenticate via a JSON credential file rather than an + env var. When ``agent_context.secrets`` contains one of the configured + names in ``acp_file_secrets``, the SDK must write the payload to a + per-agent tempdir, set the corresponding env var on the subprocess, and + drop the secret from the regular env-var injection path so its + (potentially large) payload is not leaked into ``os.environ``. + """ + + @staticmethod + def _run_start_capturing_env( + agent, + tmp_path, + capture_args: list[str] | None = None, + extra_os_env: dict[str, str] | None = None, + ) -> dict: + return TestACPSecretsEnvInjection._run_start_capturing_env( + agent, + tmp_path, + capture_args=capture_args, + extra_os_env=extra_os_env, + ) + + def test_codex_auth_json_materialised_with_codex_home(self, tmp_path): + """CODEX_AUTH_JSON → auth.json on disk + CODEX_HOME=.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + payload = '{"auth_mode":"chatgpt","tokens":{"id_token":"tok"}}' + agent = _make_agent( + acp_file_secrets=_file_secret_specs_for("codex"), + agent_context=AgentContext( + secrets={"CODEX_AUTH_JSON": StaticSecret(value=SecretStr(payload))} + ), + ) + env = self._run_start_capturing_env(agent, tmp_path) + + codex_home = env.get("CODEX_HOME") + assert codex_home, "CODEX_HOME should be set after materialisation" + auth_path = Path(codex_home) / "auth.json" + assert auth_path.is_file() + assert auth_path.read_text() == payload + # Payload must NOT also be exported as a giant env var + assert "CODEX_AUTH_JSON" not in env + + def test_google_credentials_materialised_with_path_env(self, tmp_path): + """GOOGLE_APPLICATION_CREDENTIALS_JSON → file + GAC=.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + payload = '{"type":"service_account","project_id":"p","client_email":"x@y"}' + agent = _make_agent( + acp_file_secrets=_file_secret_specs_for("gemini-cli"), + agent_context=AgentContext( + secrets={ + "GOOGLE_APPLICATION_CREDENTIALS_JSON": StaticSecret( + value=SecretStr(payload) + ) + } + ), + ) + env = self._run_start_capturing_env(agent, tmp_path) + + gac = env.get("GOOGLE_APPLICATION_CREDENTIALS") + assert gac, "GOOGLE_APPLICATION_CREDENTIALS should be set" + gac_path = Path(gac) + assert gac_path.is_file() + assert gac_path.read_text() == payload + assert "GOOGLE_APPLICATION_CREDENTIALS_JSON" not in env + + def test_custom_file_secret_materialises_env_and_args(self, tmp_path): + """Custom ACP servers can declare their own file-secret convention.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + payload = '{"token":"custom"}' + launch_args: list[str] = [] + agent = ACPAgent( + acp_command=["custom-acp"], + acp_args=["--existing"], + acp_file_secrets=[ + ACPFileSecretSpec( + secret_name="CUSTOM_AUTH_JSON", + relative_path="nested/config.json", + env={ + "CUSTOM_AUTH_FILE": "{file}", + "CUSTOM_AUTH_DIR": "{dir}", + "CUSTOM_AUTH_ROOT": "{root}", + }, + args=["--config", "{file}"], + ) + ], + agent_context=AgentContext( + secrets={"CUSTOM_AUTH_JSON": StaticSecret(value=SecretStr(payload))} + ), + ) + env = self._run_start_capturing_env( + agent, + tmp_path, + capture_args=launch_args, + ) + + auth_file = Path(env["CUSTOM_AUTH_FILE"]) + auth_dir = Path(env["CUSTOM_AUTH_DIR"]) + auth_root = Path(env["CUSTOM_AUTH_ROOT"]) + assert auth_file.is_file() + assert auth_file.read_text() == payload + assert auth_file == auth_dir / "nested" / "config.json" + assert auth_dir.parent == auth_root + assert "CUSTOM_AUTH_JSON" not in env + assert launch_args == [ + "custom-acp", + "--existing", + "--config", + str(auth_file), + ] + + def test_secret_with_no_file_secret_spec_flows_through_env(self, tmp_path): + """There is no global Codex/Gemini magic for custom ACP commands.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + acp_file_secrets=[], + agent_context=AgentContext( + secrets={"CODEX_AUTH_JSON": StaticSecret(value=SecretStr("{}"))} + ), + ) + env = self._run_start_capturing_env(agent, tmp_path) + + assert "CODEX_HOME" not in env + assert env["CODEX_AUTH_JSON"] == "{}" + + def test_file_secret_env_collision_can_raise(self, tmp_path): + """Specs can opt out of replacing an inherited env var.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + acp_file_secrets=[ + ACPFileSecretSpec( + secret_name="CUSTOM_AUTH_JSON", + relative_path="config.json", + env={"CUSTOM_AUTH_FILE": "{file}"}, + overwrite_env=False, + ) + ], + agent_context=AgentContext( + secrets={"CUSTOM_AUTH_JSON": StaticSecret(value=SecretStr("{}"))} + ), + ) + + with pytest.raises(ValueError, match="would overwrite"): + self._run_start_capturing_env( + agent, + tmp_path, + extra_os_env={"CUSTOM_AUTH_FILE": "/already/set.json"}, + ) + + def test_acp_env_takes_precedence_over_file_secret_env(self, tmp_path): + """Explicit acp_env entries must not be overwritten by file-secrets.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + explicit_path = "/mounted/gcloud-credentials.json" + agent = _make_agent( + acp_env={"GOOGLE_APPLICATION_CREDENTIALS": explicit_path}, + acp_file_secrets=_file_secret_specs_for("gemini-cli"), + agent_context=AgentContext( + secrets={ + "GOOGLE_APPLICATION_CREDENTIALS_JSON": StaticSecret( + value=SecretStr('{"type":"service_account"}') + ) + } + ), + ) + + env = self._run_start_capturing_env(agent, tmp_path) + + assert env["GOOGLE_APPLICATION_CREDENTIALS"] == explicit_path + assert "GOOGLE_APPLICATION_CREDENTIALS_JSON" not in env + + def test_materialised_file_is_readable_only_by_owner(self, tmp_path): + """File-secret payloads on disk must be 0o600.""" + import stat as stat_mod + + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + acp_file_secrets=_file_secret_specs_for("codex"), + agent_context=AgentContext( + secrets={"CODEX_AUTH_JSON": StaticSecret(value=SecretStr("{}"))} + ), + ) + env = self._run_start_capturing_env(agent, tmp_path) + auth_path = Path(env["CODEX_HOME"]) / "auth.json" + mode = stat_mod.S_IMODE(auth_path.stat().st_mode) + assert mode == 0o600 + + def test_plain_secrets_still_exported_as_env_vars(self, tmp_path): + """Unknown secret names continue to flow through the env-var path.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + acp_file_secrets=_file_secret_specs_for("codex"), + agent_context=AgentContext( + secrets={ + "CODEX_AUTH_JSON": StaticSecret(value=SecretStr("{}")), + "GITHUB_TOKEN": StaticSecret(value=SecretStr("ghp_xyz")), + } + ), + ) + env = self._run_start_capturing_env(agent, tmp_path) + # File-secret was materialised + assert env["CODEX_HOME"] + # Plain secret was injected as env var + assert env.get("GITHUB_TOKEN") == "ghp_xyz" + + def test_empty_file_secret_value_is_skipped(self, tmp_path): + """An empty payload must not produce a file or set CODEX_HOME.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + acp_file_secrets=_file_secret_specs_for("codex"), + agent_context=AgentContext( + secrets={"CODEX_AUTH_JSON": StaticSecret(value=SecretStr(""))} + ), + ) + env = self._run_start_capturing_env(agent, tmp_path) + assert "CODEX_HOME" not in env + assert agent._file_secrets_tempdir is None + + def test_multiple_file_secrets_share_one_tempdir(self, tmp_path): + """Codex + Gemini in the same agent both materialise under one base.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + acp_file_secrets=_file_secret_specs_for("codex", "gemini-cli"), + agent_context=AgentContext( + secrets={ + "CODEX_AUTH_JSON": StaticSecret(value=SecretStr("{}")), + "GOOGLE_APPLICATION_CREDENTIALS_JSON": StaticSecret( + value=SecretStr('{"type":"service_account"}') + ), + } + ), + ) + env = self._run_start_capturing_env(agent, tmp_path) + codex_dir = Path(env["CODEX_HOME"]).resolve() + gac_file = Path(env["GOOGLE_APPLICATION_CREDENTIALS"]).resolve() + # Both live under the same per-agent tempdir, but in distinct subdirs + # so handlers that expose their directory don't reveal each other. + assert codex_dir.parent == gac_file.parent.parent + assert codex_dir != gac_file.parent + + def test_close_removes_materialised_tempdir(self, tmp_path): + """``ACPAgent.close()`` must clean up the per-agent tempdir.""" + from pydantic import SecretStr + + from openhands.sdk.secret import StaticSecret + + agent = _make_agent( + acp_file_secrets=_file_secret_specs_for("codex"), + agent_context=AgentContext( + secrets={"CODEX_AUTH_JSON": StaticSecret(value=SecretStr("{}"))} + ), + ) + env = self._run_start_capturing_env(agent, tmp_path) + codex_home = Path(env["CODEX_HOME"]) + assert codex_home.is_dir() + agent.close() + assert not codex_home.exists() + assert agent._file_secrets_tempdir is None + + class TestACPEnvConflictSuppression: """CLAUDE_CONFIG_DIR OAuth auth must not coexist with API-key env vars. diff --git a/tests/sdk/settings/test_acp_providers.py b/tests/sdk/settings/test_acp_providers.py index 1aee88951b..849acaf679 100644 --- a/tests/sdk/settings/test_acp_providers.py +++ b/tests/sdk/settings/test_acp_providers.py @@ -7,7 +7,10 @@ import pytest from openhands.sdk.settings.acp_providers import ( + ACP_CODEX_SUBSCRIPTION_AUTH_SECRET, + ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET, ACP_PROVIDERS, + ACP_SUBSCRIPTION_AUTH_SECRETS, ACPProviderInfo, build_session_model_meta, detect_acp_provider_by_agent_name, @@ -153,6 +156,32 @@ def test_detect_returns_matching_provider_for_all_registered_patterns(self): f"pattern {pattern!r} matched {detected.key!r}, expected {key!r}" ) + def test_subscription_auth_secret_registry_exports_codex_and_gemini(self): + assert set(ACP_SUBSCRIPTION_AUTH_SECRETS) == {"codex", "gemini-cli"} + assert ( + ACP_CODEX_SUBSCRIPTION_AUTH_SECRET is ACP_SUBSCRIPTION_AUTH_SECRETS["codex"] + ) + assert ( + ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET + is ACP_SUBSCRIPTION_AUTH_SECRETS["gemini-cli"] + ) + + def test_codex_subscription_auth_secret_metadata(self): + auth = ACP_PROVIDERS["codex"].subscription_auth_secret + assert auth is not None + assert auth.secret_name == "CODEX_AUTH_JSON" + assert "ChatGPT" in auth.description + assert auth.file_secret is not None + assert auth.file_secret.env == {"CODEX_HOME": "{dir}"} + + def test_gemini_subscription_auth_secret_metadata(self): + auth = ACP_PROVIDERS["gemini-cli"].subscription_auth_secret + assert auth is not None + assert auth.secret_name == "GOOGLE_APPLICATION_CREDENTIALS_JSON" + assert "Google" in auth.description + assert auth.file_secret is not None + assert auth.file_secret.env == {"GOOGLE_APPLICATION_CREDENTIALS": "{file}"} + class TestBuildSessionModelMeta: def test_empty_when_no_model(self): diff --git a/tests/sdk/test_settings.py b/tests/sdk/test_settings.py index 3609d945c8..7843d70e31 100644 --- a/tests/sdk/test_settings.py +++ b/tests/sdk/test_settings.py @@ -7,8 +7,11 @@ from openhands.agent_server.models import StartConversationRequest from openhands.sdk import ( + ACP_CODEX_SUBSCRIPTION_AUTH_SECRET, + ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET, LLM, ACPAgentSettings, + ACPFileSecretSpec, Agent, AgentContext, AgentSettings, @@ -137,6 +140,7 @@ def test_acp_agent_settings_export_schema_has_acp_section() -> None: "acp_command", "acp_args", "acp_env", + "acp_file_secrets", "acp_model", "acp_session_mode", "acp_prompt_timeout", @@ -148,6 +152,14 @@ def test_acp_agent_settings_export_schema_has_acp_section() -> None: assert acp_fields["acp_command"].prominence is SettingProminence.MINOR +def test_acp_subscription_auth_secret_infos_are_exported_package_level() -> None: + assert ACP_CODEX_SUBSCRIPTION_AUTH_SECRET.secret_name == "CODEX_AUTH_JSON" + assert ( + ACP_GEMINI_CLI_SUBSCRIPTION_AUTH_SECRET.secret_name + == "GOOGLE_APPLICATION_CREDENTIALS_JSON" + ) + + def test_conversation_settings_export_schema_groups_sections() -> None: schema = ConversationSettings.export_schema() @@ -352,7 +364,7 @@ def test_validate_agent_settings_migrates_v0_llm_payload() -> None: settings = validate_agent_settings({"llm": {"model": "test-model"}}) assert isinstance(settings, OpenHandsAgentSettings) - assert settings.schema_version == 3 + assert settings.schema_version == AGENT_SETTINGS_SCHEMA_VERSION assert settings.agent_kind == "openhands" assert settings.llm.model == "test-model" @@ -368,8 +380,8 @@ def test_validate_agent_settings_dispatches_current_acp_payload() -> None: ) assert isinstance(settings, ACPAgentSettings) - # v1 → v2 → v3 keeps ACP payloads intact while bumping schema_version. - assert settings.schema_version == 3 + # v1 → v2 → v3 → v4 keeps ACP payloads intact while bumping schema_version. + assert settings.schema_version == AGENT_SETTINGS_SCHEMA_VERSION assert settings.acp_command == ["npx", "-y", "claude-agent-acp"] @@ -385,7 +397,7 @@ def test_validate_agent_settings_canonicalizes_legacy_llm_kind() -> None: ) assert isinstance(settings, OpenHandsAgentSettings) - assert settings.schema_version == 3 + assert settings.schema_version == AGENT_SETTINGS_SCHEMA_VERSION assert settings.agent_kind == "openhands" assert settings.llm.model == "legacy-model" @@ -404,16 +416,38 @@ def test_validate_agent_settings_drops_legacy_verification_fields() -> None: ) assert isinstance(settings, OpenHandsAgentSettings) - assert settings.schema_version == 3 + assert settings.schema_version == AGENT_SETTINGS_SCHEMA_VERSION verification = settings.verification.model_dump(mode="json") assert verification["critic_enabled"] is True assert "confirmation_mode" not in verification assert "security_analyzer" not in verification +def test_validate_agent_settings_migrates_v3_to_v4_for_acp_file_secrets() -> None: + settings = validate_agent_settings( + { + "schema_version": 3, + "agent_kind": "acp", + "acp_server": "custom", + "acp_command": ["custom-acp"], + "acp_file_secrets": [ + { + "secret_name": "CUSTOM_AUTH_JSON", + "relative_path": "auth.json", + "env": {"CUSTOM_AUTH_FILE": "{file}"}, + } + ], + } + ) + + assert isinstance(settings, ACPAgentSettings) + assert settings.schema_version == AGENT_SETTINGS_SCHEMA_VERSION + assert settings.acp_file_secrets[0].secret_name == "CUSTOM_AUTH_JSON" + + def test_validate_agent_settings_rejects_newer_schema_version() -> None: - with pytest.raises(ValueError, match="newer than supported version 3"): - validate_agent_settings({"schema_version": 4, "llm": {"model": "m"}}) + with pytest.raises(ValueError, match="newer than supported version 4"): + validate_agent_settings({"schema_version": 5, "llm": {"model": "m"}}) def test_conversation_settings_from_persisted_migrates_v0_payload() -> None: @@ -634,6 +668,35 @@ def test_acp_resolve_provider_env_custom_server_empty() -> None: assert settings.resolve_provider_env() == {} +def test_acp_resolve_file_secrets_from_known_provider_defaults() -> None: + codex = ACPAgentSettings(acp_server="codex") + codex_specs = codex.resolve_acp_file_secrets() + assert [spec.secret_name for spec in codex_specs] == ["CODEX_AUTH_JSON"] + assert codex_specs[0].env == {"CODEX_HOME": "{dir}"} + + gemini = ACPAgentSettings(acp_server="gemini-cli") + gemini_specs = gemini.resolve_acp_file_secrets() + assert [spec.secret_name for spec in gemini_specs] == [ + "GOOGLE_APPLICATION_CREDENTIALS_JSON" + ] + assert gemini_specs[0].env == {"GOOGLE_APPLICATION_CREDENTIALS": "{file}"} + + +def test_acp_resolve_file_secrets_custom_server_uses_custom_specs_only() -> None: + spec = ACPFileSecretSpec( + secret_name="CUSTOM_AUTH_JSON", + relative_path="auth.json", + env={"CUSTOM_AUTH_FILE": "{file}"}, + ) + settings = ACPAgentSettings( + acp_server="custom", + acp_command=["custom-acp"], + acp_file_secrets=[spec], + ) + + assert settings.resolve_acp_file_secrets() == [spec] + + def test_acp_resolve_acp_env_explicit_entries_override_provider_env() -> None: settings = ACPAgentSettings( acp_server="claude-code", @@ -655,9 +718,27 @@ def test_acp_create_agent_passes_resolved_env_and_agent_context() -> None: agent = settings.create_agent() assert agent.acp_env == {"OPENAI_API_KEY": "sk-openai"} + assert [spec.secret_name for spec in agent.acp_file_secrets] == ["CODEX_AUTH_JSON"] assert agent.agent_context == context +def test_acp_create_agent_passes_custom_file_secrets() -> None: + spec = ACPFileSecretSpec( + secret_name="CUSTOM_AUTH_JSON", + relative_path="auth.json", + env={"CUSTOM_AUTH_FILE": "{file}"}, + ) + settings = ACPAgentSettings( + acp_server="custom", + acp_command=["custom-acp"], + acp_file_secrets=[spec], + ) + + agent = settings.create_agent() + + assert agent.acp_file_secrets == [spec] + + # --------------------------------------------------------------------------- # Legacy ``AgentSettings`` compatibility # ---------------------------------------------------------------------------