diff --git a/amplifier_app_cli/commands/provider.py b/amplifier_app_cli/commands/provider.py index c3269d9..6d27725 100644 --- a/amplifier_app_cli/commands/provider.py +++ b/amplifier_app_cli/commands/provider.py @@ -3,7 +3,7 @@ import os import re import time -from typing import Any, cast +from typing import Any, NamedTuple, cast import click from rich.console import Console @@ -162,25 +162,50 @@ def _find_claimed_env_var_owner(settings: AppSettings, env_var: str) -> str: return "an existing instance" -def _prompt_env_var_collision( - settings: AppSettings, +class CredentialDecision(NamedTuple): + """How a provider instance binds to its credential env var. + + ``env_var_overrides``: ``{type_default_env_var: instance_env_var}`` rename + map threaded into ``configure_provider`` (empty ⇒ use the type default). + + ``binding_modes``: ``{resolved_env_var: "shared" | "separate"}`` threaded + into ``configure_provider`` so a *shared* credential is never prompted + for or overwritten, and a *separate* one may be left unset for runtime + injection. Empty ⇒ today's behavior (no per-instance binding decision). + """ + + env_var_overrides: dict[str, str] + binding_modes: dict[str, str] + + +def _count_instances_using_env_var(settings: AppSettings, env_var: str) -> int: + """Number of configured provider instances (across all merged scopes) + whose config references ``${env_var}``. ``>= 2`` means the credential is + genuinely shared, so editing any one of them must preserve the shared + binding rather than overwrite the common secret. + """ + placeholder = f"${{{env_var}}}" + count = 0 + for p in settings.get_provider_overrides(): + config = p.get("config", {}) + if isinstance(config, dict) and placeholder in config.values(): + count += 1 + return count + + +def _prompt_separate_env_var( key_manager: KeyManager, module_id: str, instance_id: str | None, default_name: str, claimed: set[str], ) -> str: - """Interactive resolution for a same-default-name credential collision - (design §5.2 step 4). Returns the chosen, validated, unclaimed env var - name for this instance. Raises (EOFError, KeyboardInterrupt) on cancel. + """Derive, prefill, prompt for and validate a *distinct* credential env + var for a same-type instance that opted for a separate credential + (design §5.2 step 4). Returns the chosen, validated, unclaimed name. + Raises (EOFError, KeyboardInterrupt) on cancel, or ValueError if the id + can't produce a usable, non-colliding suggestion (§5.4.2). """ - owner = _find_claimed_env_var_owner(settings, default_name) - console.print( - f"\n [yellow]{module_id} already uses {default_name} " - f"(instance '{owner}'). This instance needs its own credential " - f"source.[/yellow]" - ) - suggested = _suggest_instance_env_var(module_id, instance_id or module_id, claimed) # Exact-name prefill only (design §5.4.3) -- a single os.environ.get() @@ -225,18 +250,72 @@ def _prompt_env_var_collision( return chosen_name +def _prompt_credential_binding( + settings: AppSettings, + key_manager: KeyManager, + module_id: str, + instance_id: str | None, + default_name: str, + claimed: set[str], +) -> CredentialDecision: + """Interactive fork for a same-type instance whose type-default + credential env var is already claimed: reuse the existing credential + (default) or configure a separate one. + + * Reuse -> bind to ``default_name``; never prompt for or overwrite its + value (``binding_modes = {default_name: "shared"}``). + * Separate -> derive/validate a distinct name and rename to it + (``env_var_overrides = {default_name: chosen}``, + ``binding_modes = {chosen: "separate"}``). + + Raises (EOFError, KeyboardInterrupt) on cancel, or ValueError if the id + can't produce a usable, non-colliding suggestion for the separate path. + """ + owner = _find_claimed_env_var_owner(settings, default_name) + console.print( + f"\n [bold]Another '{module_id}' instance already uses " + f"{default_name} (instance '{owner}').[/bold]" + ) + console.print(" How should this instance authenticate?") + console.print( + f" [1] Reuse {default_name} -- same credential, different " + f"model/settings [dim](default)[/dim]" + ) + console.print( + " [2] Separate credential -- its own env var (different " + "account/org/key)" + ) + choice = Prompt.ask(" Choice", choices=["1", "2"], default="1") + + if choice == "1": + # Reuse: bind to the existing credential; the secret prompt is + # skipped entirely and the stored value is left untouched. + console.print(f" [dim]This instance will reuse {default_name}.[/dim]") + return CredentialDecision({}, {default_name: "shared"}) + + chosen_name = _prompt_separate_env_var( + key_manager, module_id, instance_id, default_name, claimed + ) + return CredentialDecision({default_name: chosen_name}, {chosen_name: "separate"}) + + def _resolve_env_var_overrides( settings: AppSettings, key_manager: KeyManager, module_id: str, instance_id: str | None, -) -> dict[str, str]: - """Resolve the `env_var_overrides` map for a new provider instance - (design §5.2). Empty dict means "use the type default" -- no collision - detected. May raise (EOFError, KeyboardInterrupt) on user cancel, or - ValueError if the instance id cannot produce a usable, non-colliding - suggestion (design §5.4.2) -- caller decides how to react (re-prompt vs. - exit). +) -> CredentialDecision: + """Resolve the credential binding for a NEW provider instance (design + §5.2). Returns an empty decision when no collision is detected -- the + first instance of a type keeps today's no-extra-prompt UX. When the + type-default name is already claimed, forks to reuse-vs-separate via + ``_prompt_credential_binding`` (offer to reuse -- the default -- or + configure a separate credential). + + May raise (EOFError, KeyboardInterrupt) on user cancel, or ValueError if + the instance id cannot produce a usable, non-colliding suggestion for a + separate credential (design §5.4.2) -- caller decides how to react + (re-prompt vs. exit). Collision detection asks "does another *configured instance* already own this name?", so it uses ``_config_claimed_env_vars`` (${VAR} references @@ -252,38 +331,56 @@ def _resolve_env_var_overrides( claimed = _config_claimed_env_vars(settings) default_name = _secret_env_var_for(module_id) if not default_name or default_name not in claimed: - return {} + return CredentialDecision({}, {}) - chosen_name = _prompt_env_var_collision( + return _prompt_credential_binding( settings, key_manager, module_id, instance_id, default_name, claimed ) - return {default_name: chosen_name} def _recover_env_var_override( - module_id: str, existing_config: dict[str, Any] | None -) -> dict[str, str]: - """Recover an instance's existing credential env var from its stored + settings: AppSettings, + module_id: str, + instance_id: str | None, + existing_config: dict[str, Any] | None, +) -> CredentialDecision: + """Recover an instance's existing credential binding from its stored placeholder so re-configuring it doesn't reset to the type default and silently re-collide with another instance (design §5.3 -- the "must not miss" fix). + + If the recovered env var is shared by another configured instance, mark + it ``"shared"`` so editing this instance never prompts for or overwrites + the common secret (the reuse invariant must survive edits, not just the + initial add). """ if not existing_config: - return {} + return CredentialDecision({}, {}) default_name = _secret_env_var_for(module_id) field_id = _secret_field_id_for(module_id) if not default_name or not field_id: - return {} + return CredentialDecision({}, {}) raw_value = existing_config.get(field_id) - if ( + if not ( isinstance(raw_value, str) and raw_value.startswith("${") and raw_value.endswith("}") ): - current_name = raw_value[2:-1] - if current_name and current_name != default_name: - return {default_name: current_name} - return {} + return CredentialDecision({}, {}) + + current_name = raw_value[2:-1] + if not current_name: + return CredentialDecision({}, {}) + + overrides: dict[str, str] = {} + if current_name != default_name: + overrides = {default_name: current_name} + + binding_modes: dict[str, str] = {} + if _count_instances_using_env_var(settings, current_name) >= 2: + binding_modes = {current_name: "shared"} + + return CredentialDecision(overrides, binding_modes) @click.group() @@ -479,7 +576,7 @@ def provider_add(ctx: click.Context, provider_type: str | None, scope: str) -> N # name in one pass. key_manager = KeyManager() try: - env_var_overrides = _resolve_env_var_overrides( + credential = _resolve_env_var_overrides( settings, key_manager, module_id, instance_id ) except ValueError as e: @@ -494,7 +591,8 @@ def provider_add(ctx: click.Context, provider_type: str | None, scope: str) -> N config = configure_provider( module_id, key_manager, - env_var_overrides=env_var_overrides, + env_var_overrides=credential.env_var_overrides, + credential_binding_modes=credential.binding_modes, settings=settings, ) except (click.Abort, click.ClickException): @@ -819,8 +917,11 @@ def provider_edit(name: str, scope: str) -> None: # Bug 3 (design §5.3, "must not miss"): recover the instance's existing # credential env var from its stored placeholder so re-configuring it # doesn't reset to the type default and silently re-collide. - env_var_overrides = _recover_env_var_override( - module_id, existing_config if isinstance(existing_config, dict) else None + credential = _recover_env_var_override( + settings, + module_id, + entry.get("id"), + existing_config if isinstance(existing_config, dict) else None, ) # Run configure_provider with existing config as defaults @@ -829,7 +930,8 @@ def provider_edit(name: str, scope: str) -> None: module_id, key_manager, existing_config=existing_config, - env_var_overrides=env_var_overrides, + env_var_overrides=credential.env_var_overrides, + credential_binding_modes=credential.binding_modes, settings=settings, ) @@ -1251,7 +1353,7 @@ def _manage_add_provider(settings: AppSettings, scope: Scope = "global") -> None # name in one pass. key_manager = KeyManager() try: - env_var_overrides = _resolve_env_var_overrides( + credential = _resolve_env_var_overrides( settings, key_manager, module_id, instance_id ) except ValueError as e: @@ -1266,7 +1368,8 @@ def _manage_add_provider(settings: AppSettings, scope: Scope = "global") -> None config = configure_provider( module_id, key_manager, - env_var_overrides=env_var_overrides, + env_var_overrides=credential.env_var_overrides, + credential_binding_modes=credential.binding_modes, settings=settings, ) except (click.Abort, KeyboardInterrupt, EOFError): @@ -1363,8 +1466,11 @@ def _manage_edit_provider( # Bug 3 (design §5.3, "must not miss"): recover the instance's existing # credential env var from its stored placeholder so re-configuring it # doesn't reset to the type default and silently re-collide. - env_var_overrides = _recover_env_var_override( - module_id, existing_config if isinstance(existing_config, dict) else None + credential = _recover_env_var_override( + settings, + module_id, + entry.get("id"), + existing_config if isinstance(existing_config, dict) else None, ) key_manager = KeyManager() @@ -1373,7 +1479,8 @@ def _manage_edit_provider( module_id, key_manager, existing_config=existing_config, - env_var_overrides=env_var_overrides, + env_var_overrides=credential.env_var_overrides, + credential_binding_modes=credential.binding_modes, settings=settings, ) except (click.Abort, KeyboardInterrupt, EOFError): diff --git a/amplifier_app_cli/provider_config_utils.py b/amplifier_app_cli/provider_config_utils.py index 400703d..6122315 100644 --- a/amplifier_app_cli/provider_config_utils.py +++ b/amplifier_app_cli/provider_config_utils.py @@ -506,6 +506,7 @@ def _prompt_for_field( collected_config: dict[str, Any], existing_config: dict[str, Any] | None = None, env_var_overrides: dict[str, str] | None = None, + credential_binding_modes: dict[str, str] | None = None, ) -> tuple[str, Any]: """Prompt user for a single config field value. @@ -518,6 +519,22 @@ def _prompt_for_field( instance_env_var}``, resolved by the caller (design §5.2/§5.3) when a same-type instance needs a distinct credential name from the provider type's declared default. + credential_binding_modes: Optional map of ``{resolved_env_var: mode}`` + where ``mode`` is ``"shared"`` or ``"separate"``. Resolved by the + caller when adding/editing a same-type instance: + + * ``"shared"`` -- this instance reuses an existing instance's + credential env var. The secret field is NEVER prompted for or + overwritten; only its ``${VAR}`` placeholder is persisted, so + both instances resolve the same secret at runtime. + * ``"separate"`` -- this instance owns a distinct credential env + var. It is prompted as usual, but may be left blank to persist + the placeholder UNSET for runtime injection (shell / CI / DTU + passthrough) instead of hard-failing on a required secret. + + Mechanism only: this function does not decide the mode, it just + honors whatever it is handed. Fields with no matching entry keep + today's behavior exactly (fully backward compatible). Returns: Tuple of (field_id, value) @@ -531,6 +548,7 @@ def _prompt_for_field( if declared_env_var else declared_env_var ) + binding_mode = (credential_binding_modes or {}).get(env_var) if env_var else None default = field.get("default") required = field.get("required", True) @@ -592,6 +610,17 @@ def _prompt_for_field( # No choices defined, fall through to text if field_type == "secret": + # Shared credential binding: this instance reuses another instance's + # credential env var. Never prompt for or overwrite its stored value + # -- just persist the placeholder so both instances resolve the same + # secret at runtime. + if env_var and binding_mode == "shared": + console.print( + f" [dim]Reusing existing credential {env_var}; its stored " + f"value is left untouched.[/dim]" + ) + return field_id, f"${{{env_var}}}" + prompt_suffix = " (press Enter to keep existing)" if existing_value else "" value = Prompt.ask(f"{prompt_text}{prompt_suffix}", password=True, default="") @@ -606,6 +635,17 @@ def _prompt_for_field( if existing_value: console.print("[green]✓ Using existing[/green]") return field_id, f"${{{env_var}}}" if env_var else existing_value + # Separate credential binding left blank: persist the placeholder + # UNSET so the value can be injected from the environment at runtime + # (shell / CI / DTU passthrough) instead of hard-failing. Scoped to an + # explicit per-instance binding -- a first-instance/normal required + # secret still errors, preserving today's setup guardrail. + if env_var and binding_mode == "separate": + console.print( + f" [yellow]No value entered; {env_var} will be resolved from " + f"the environment at runtime.[/yellow]" + ) + return field_id, f"${{{env_var}}}" if required: console.print("[red]Error: Required field[/red]") raise ValueError(f"{field['display_name']} is required") @@ -640,6 +680,7 @@ def configure_provider( existing_config: dict[str, Any] | None = None, non_interactive: bool = False, env_var_overrides: dict[str, str] | None = None, + credential_binding_modes: dict[str, str] | None = None, settings: AppSettings | None = None, ) -> dict[str, Any] | None: """Configure a provider using its self-declared config_fields. @@ -664,6 +705,11 @@ def configure_provider( when a same-type instance needs a distinct credential name. Mechanism only -- this function does not compute collisions, it just uses whatever name it is handed. + credential_binding_modes: Optional map of ``{resolved_env_var: mode}`` + (``"shared"`` or ``"separate"``) threaded into ``_prompt_for_field`` + so a reused credential env var is never re-prompted/overwritten and + a separate one may be left unset for runtime injection. Mechanism + only -- the caller decides the mode. settings: Optional AppSettings, used only to detect a same-type credential collision in ``non_interactive`` mode and fail loudly instead of silently reusing the type default (design §5.4.5). @@ -738,13 +784,19 @@ def configure_provider( if declared else declared ) + mode = ( + (credential_binding_modes or {}).get(env_var) if env_var else None + ) # Fail loud instead of silently reusing the type default when # it's already claimed by another instance (design §5.4.5). + # An explicit per-instance binding (shared reuse or separate) + # is intentional, so it is exempt from this guard. if ( settings is not None and declared and env_var == declared and declared not in (env_var_overrides or {}) + and mode is None and declared in _claimed_env_vars(settings) ): raise ValueError( @@ -754,7 +806,13 @@ def configure_provider( f"env_var_overrides mapping for this instance " f"instead of relying on the type default." ) - if env_var and os.environ.get(env_var): + if mode is not None: + # Explicit per-instance binding: persist the placeholder + # whether or not the value is set in the environment. A + # separate binding may be intentionally unset for runtime + # injection; a shared one reuses an existing instance's key. + collected_config[field_id] = f"${{{env_var}}}" + elif env_var and os.environ.get(env_var): collected_config[field_id] = f"${{{env_var}}}" elif existing_config and field_id in existing_config: collected_config[field_id] = existing_config[field_id] @@ -764,7 +822,12 @@ def configure_provider( # Prompt for the field (pass existing_config for defaults) field_id, value = _prompt_for_field( - field, key_manager, collected_config, existing_config, env_var_overrides + field, + key_manager, + collected_config, + existing_config, + env_var_overrides, + credential_binding_modes, ) if value is not None: collected_config[field_id] = value @@ -832,13 +895,19 @@ def configure_provider( if declared else declared ) + mode = ( + (credential_binding_modes or {}).get(env_var) if env_var else None + ) # Fail loud instead of silently reusing the type default when # it's already claimed by another instance (design §5.4.5). + # An explicit per-instance binding (shared reuse or separate) + # is intentional, so it is exempt from this guard. if ( settings is not None and declared and env_var == declared and declared not in (env_var_overrides or {}) + and mode is None and declared in _claimed_env_vars(settings) ): raise ValueError( @@ -848,7 +917,13 @@ def configure_provider( f"env_var_overrides mapping for this instance " f"instead of relying on the type default." ) - if env_var and os.environ.get(env_var): + if mode is not None: + # Explicit per-instance binding: persist the placeholder + # whether or not the value is set in the environment. A + # separate binding may be intentionally unset for runtime + # injection; a shared one reuses an existing instance's key. + collected_config[field_id] = f"${{{env_var}}}" + elif env_var and os.environ.get(env_var): collected_config[field_id] = f"${{{env_var}}}" elif existing_config and field_id in existing_config: collected_config[field_id] = existing_config[field_id] @@ -858,7 +933,12 @@ def configure_provider( # Prompt for the field (pass existing_config for defaults) field_id, value = _prompt_for_field( - field, key_manager, collected_config, existing_config, env_var_overrides + field, + key_manager, + collected_config, + existing_config, + env_var_overrides, + credential_binding_modes, ) if value is not None: collected_config[field_id] = value diff --git a/amplifier_app_cli/runtime/config.py b/amplifier_app_cli/runtime/config.py index 032a9e4..8535884 100644 --- a/amplifier_app_cli/runtime/config.py +++ b/amplifier_app_cli/runtime/config.py @@ -16,6 +16,7 @@ from ..lib.merge_utils import merge_module_items from ..lib.merge_utils import merge_tool_configs from ..lib.merge_utils import _normalize_module_entry +from ..provider_loader import get_provider_info if TYPE_CHECKING: @@ -305,6 +306,15 @@ def _on_progress(action: str, detail: str) -> None: if console: console.print(f"[dim]Bundle '{bundle_name}' prepared successfully[/dim]") + # Fail loud (before mount) on an unresolved *required* credential + # placeholder, instead of letting env-var expansion silently turn it + # into "" and letting the provider module fall back to its own ambient + # credential -- see _validate_provider_credentials for why this matters + # for the reuse-or-separate multi-instance flow. + raw_providers = bundle_config.get("providers") + if isinstance(raw_providers, list): + _validate_provider_credentials(raw_providers) + # Expand environment variables # IMPORTANT: Must expand BEFORE syncing to mount_plan, so ${ANTHROPIC_API_KEY} etc. become actual values bundle_config = expand_env_vars(bundle_config) @@ -842,6 +852,79 @@ def _merge_module_lists( ENV_PATTERN = re.compile(r"\$\{([^}:]+)(?::([^}]*))?}") +def _validate_provider_credentials(providers: list[Any]) -> None: + """Fail loudly, before session mount, when a provider instance's + configured credential placeholder resolves to nothing. + + Why this exists: ``expand_env_vars`` (below) treats an unset ``${VAR}`` + as an empty string. Several provider modules treat an empty/absent + ``api_key`` config value as "not configured" and fall back to their own + canonical ambient env var (e.g. ``OPENAI_API_KEY``). For a *separate* + per-instance credential binding left unset for runtime injection (see + the reuse-or-separate wizard flow in ``commands/provider.py`` / + ``provider_config_utils.py``, design doc §5.2), that fallback silently + routes the instance through a *different* account's key -- exactly the + wrong-account failure the reuse-or-separate flow exists to prevent. + Raising here turns that into a clear, actionable error at session start + instead of a silent cross-account credential mixup. + + Only enforced for fields the provider declares as + ``field_type == "secret"`` AND ``required`` (default True): optional / + keyless secrets (e.g. a local Chat Completions server with no API key) + are intentionally left alone, and a placeholder with an inline + ``${VAR:-default}`` default is also left alone (the default already + covers "unset"). + + App-CLI policy only -- no core, provider-contract, or settings-schema + changes. When provider metadata can't be loaded (custom/removed + provider, import error, etc.), validation is skipped for that entry -- + consistent with how the rest of this module already treats a missing + ``get_provider_info()`` result. + """ + for entry in providers: + if not isinstance(entry, dict): + continue + module_id = entry.get("module") + config = entry.get("config") + if not isinstance(module_id, str) or not isinstance(config, dict): + continue + + info = get_provider_info(module_id) + if not info: + continue + + for field in info.get("config_fields") or []: + if not isinstance(field, dict) or field.get("field_type") != "secret": + continue + if not field.get("required", True): + continue + + field_id = field.get("id") + if not field_id: + continue + raw_value = config.get(field_id) + if not isinstance(raw_value, str): + continue + + match = ENV_PATTERN.fullmatch(raw_value) + if not match: + continue + var_name, default = match.group(1), match.group(2) + if default is not None: + # An inline default already covers "unset" -- not our concern. + continue + if os.environ.get(var_name): + continue + + label = entry.get("id") or module_id + raise ValueError( + f"Credential environment variable {var_name} for provider " + f"'{label}' is not set. Set it before starting the " + f"session; Amplifier will not fall back to another " + f"provider's credential." + ) + + def expand_env_vars(config: dict[str, Any]) -> dict[str, Any]: """Expand ${VAR} references within configuration values.""" diff --git a/tests/test_provider_instance_credentials.py b/tests/test_provider_instance_credentials.py index ff02ef5..0d7ec9a 100644 --- a/tests/test_provider_instance_credentials.py +++ b/tests/test_provider_instance_credentials.py @@ -725,6 +725,408 @@ def _fake_configure_provider(module_id, key_manager, **kwargs): assert by_id["anthropic-opus"]["config"]["api_key"] == "${ANTHROPIC_API_KEY}" +# ============================================================ +# Reuse-vs-separate credential binding for same-type instances +# (offer to reuse an existing credential env var, or configure a +# separate one). Covers the shared, separate, unset, and +# edit-preservation paths. +# ============================================================ + + +class TestReuseVsSeparateCredentialBinding: + def test_reuse_choice_shares_env_var_and_never_prompts_for_secret( + self, tmp_path, monkeypatch + ): + """Choosing "reuse" (default) binds the new instance to the existing + credential env var, adds NO rename, and marks it "shared" so the + secret is never prompted for or overwritten.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + from amplifier_app_cli.commands.provider import _resolve_env_var_overrides + + settings = _make_settings(tmp_path) + _seed_provider( + settings, + "provider-anthropic", + {"api_key": "${ANTHROPIC_API_KEY}"}, + provider_id="anthropic-opus", + scope="global", + ) + mock_key_manager = MagicMock() + + with ( + patch( + "amplifier_app_cli.commands.provider._secret_env_var_for", + return_value="ANTHROPIC_API_KEY", + ), + patch( + "amplifier_app_cli.commands.provider.Prompt.ask", + # Only the fork prompt is asked; "1" = reuse. No name prompt. + side_effect=["1"], + ) as mock_ask, + ): + decision = _resolve_env_var_overrides( + settings, mock_key_manager, "provider-anthropic", "anthropic-sonnet" + ) + + assert decision.env_var_overrides == {} + assert decision.binding_modes == {"ANTHROPIC_API_KEY": "shared"} + mock_ask.assert_called_once() + + def test_reuse_is_the_default_choice(self, tmp_path, monkeypatch): + """Pressing Enter (empty input) at the fork prompt must default to + reuse -- design requirement: reuse is the DEFAULT offer.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + from amplifier_app_cli.commands.provider import _prompt_credential_binding + + settings = _make_settings(tmp_path) + _seed_provider( + settings, + "provider-anthropic", + {"api_key": "${ANTHROPIC_API_KEY}"}, + provider_id="anthropic-opus", + scope="global", + ) + mock_key_manager = MagicMock() + + with patch( + "amplifier_app_cli.commands.provider.Prompt.ask" + ) as mock_ask: + mock_ask.return_value = "1" # simulate default accepted + decision = _prompt_credential_binding( + settings, + mock_key_manager, + "provider-anthropic", + "anthropic-sonnet", + "ANTHROPIC_API_KEY", + {"ANTHROPIC_API_KEY"}, + ) + + # The prompt itself must declare "1" (reuse) as its default. + _, kwargs = mock_ask.call_args + assert kwargs.get("default") == "1" + assert decision.binding_modes == {"ANTHROPIC_API_KEY": "shared"} + + def test_separate_choice_derives_distinct_env_var(self, tmp_path, monkeypatch): + """Choosing "separate" derives a distinct env var and marks it + "separate" (secure-storage path unchanged: the secret field is + still prompted normally).""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + from amplifier_app_cli.commands.provider import _resolve_env_var_overrides + + settings = _make_settings(tmp_path) + _seed_provider( + settings, + "provider-anthropic", + {"api_key": "${ANTHROPIC_API_KEY}"}, + provider_id="anthropic-opus", + scope="global", + ) + mock_key_manager = MagicMock() + mock_key_manager.has_key.return_value = False + mock_key_manager.has_stored_key.return_value = False + + with ( + patch( + "amplifier_app_cli.commands.provider._secret_env_var_for", + return_value="ANTHROPIC_API_KEY", + ), + patch( + "amplifier_app_cli.commands.provider.Prompt.ask", + side_effect=["2", "ANTHROPIC_FABLE_API_KEY"], + ), + ): + decision = _resolve_env_var_overrides( + settings, mock_key_manager, "provider-anthropic", "anthropic-fable" + ) + + assert decision.env_var_overrides == { + "ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY" + } + assert decision.binding_modes == {"ANTHROPIC_FABLE_API_KEY": "separate"} + + def test_shared_binding_never_prompts_for_or_overwrites_secret(self): + """`_prompt_for_field` with binding_mode="shared" must return the + placeholder immediately -- no secret Prompt.ask call, no + key_manager.save_key call.""" + from amplifier_app_cli.provider_config_utils import _prompt_for_field + + field = { + "id": "api_key", + "display_name": "API Key", + "field_type": "secret", + "prompt": "Enter your API key", + "env_var": "ANTHROPIC_API_KEY", + "required": True, + } + mock_key_manager = MagicMock() + + with patch( + "amplifier_app_cli.provider_config_utils.Prompt.ask" + ) as mock_ask: + field_id, value = _prompt_for_field( + field, + mock_key_manager, + collected_config={}, + existing_config=None, + credential_binding_modes={"ANTHROPIC_API_KEY": "shared"}, + ) + + assert field_id == "api_key" + assert value == "${ANTHROPIC_API_KEY}" + mock_ask.assert_not_called() + mock_key_manager.save_key.assert_not_called() + + def test_separate_binding_left_blank_persists_unset_placeholder(self): + """A "separate" binding left blank at the prompt must persist the + ${VAR} placeholder UNSET for runtime injection -- NOT raise, and + NOT call key_manager.save_key (no value to save).""" + from amplifier_app_cli.provider_config_utils import _prompt_for_field + + field = { + "id": "api_key", + "display_name": "API Key", + "field_type": "secret", + "prompt": "Enter your API key", + "env_var": "ANTHROPIC_WORK_API_KEY", + "required": True, + } + mock_key_manager = MagicMock() + + with patch( + "amplifier_app_cli.provider_config_utils.Prompt.ask", + return_value="", + ): + field_id, value = _prompt_for_field( + field, + mock_key_manager, + collected_config={}, + existing_config=None, + credential_binding_modes={"ANTHROPIC_WORK_API_KEY": "separate"}, + ) + + assert field_id == "api_key" + assert value == "${ANTHROPIC_WORK_API_KEY}" + mock_key_manager.save_key.assert_not_called() + + def test_separate_binding_can_still_use_secure_storage(self): + """A "separate" binding is NOT forced blank -- entering a value at + the prompt still saves it through the existing secure-storage flow + (KeyManager.save_key), unchanged.""" + from amplifier_app_cli.provider_config_utils import _prompt_for_field + + field = { + "id": "api_key", + "display_name": "API Key", + "field_type": "secret", + "prompt": "Enter your API key", + "env_var": "ANTHROPIC_WORK_API_KEY", + "required": True, + } + mock_key_manager = MagicMock() + + with patch( + "amplifier_app_cli.provider_config_utils.Prompt.ask", + return_value="sk-separate-value", + ): + field_id, value = _prompt_for_field( + field, + mock_key_manager, + collected_config={}, + existing_config=None, + credential_binding_modes={"ANTHROPIC_WORK_API_KEY": "separate"}, + ) + + assert field_id == "api_key" + assert value == "${ANTHROPIC_WORK_API_KEY}" + mock_key_manager.save_key.assert_called_once_with( + "ANTHROPIC_WORK_API_KEY", "sk-separate-value" + ) + + def test_edit_preserves_shared_binding_no_prompt_no_overwrite(self, tmp_path): + """Editing ONE of two instances that share a credential env var must + recover binding_modes={"shared"} so the edit never prompts for or + overwrites the common secret (the reuse invariant must survive + edits, not just the initial add).""" + from amplifier_app_cli.commands.provider import _manage_edit_provider + + settings = _make_settings(tmp_path) + scope_data = { + "config": { + "providers": [ + { + "module": "provider-anthropic", + "id": "anthropic-opus", + "config": { + "default_model": "claude-opus", + "api_key": "${ANTHROPIC_API_KEY}", + "priority": 1, + }, + }, + { + "module": "provider-anthropic", + "id": "anthropic-sonnet", + "config": { + "default_model": "claude-sonnet", + "api_key": "${ANTHROPIC_API_KEY}", + "priority": 2, + }, + }, + ] + } + } + settings._write_scope("global", scope_data) + + providers = settings.get_provider_overrides() + idx = next( + i for i, p in enumerate(providers, 1) if p.get("id") == "anthropic-sonnet" + ) + + captured: dict = {} + + def _fake_configure_provider(module_id, key_manager, **kwargs): + captured["env_var_overrides"] = kwargs.get("env_var_overrides") + captured["credential_binding_modes"] = kwargs.get( + "credential_binding_modes" + ) + return { + "default_model": "claude-sonnet", + "api_key": "${ANTHROPIC_API_KEY}", + } + + with ( + patch( + "amplifier_app_cli.commands.provider.configure_provider", + side_effect=_fake_configure_provider, + ), + patch("amplifier_app_cli.commands.provider.KeyManager"), + patch( + "amplifier_app_cli.commands.provider._secret_env_var_for", + return_value="ANTHROPIC_API_KEY", + ), + patch( + "amplifier_app_cli.commands.provider._secret_field_id_for", + return_value="api_key", + ), + ): + _manage_edit_provider(settings, f"e{idx}", providers, scope="global") + + assert captured["env_var_overrides"] == {} + assert captured["credential_binding_modes"] == {"ANTHROPIC_API_KEY": "shared"} + + # The shared value on both instances is untouched. + raw = settings.get_scope_provider_overrides("global") + by_id = {p["id"]: p for p in raw} + assert by_id["anthropic-sonnet"]["config"]["api_key"] == "${ANTHROPIC_API_KEY}" + assert by_id["anthropic-opus"]["config"]["api_key"] == "${ANTHROPIC_API_KEY}" + + def test_non_interactive_shared_binding_persists_placeholder_when_unset(self): + """Non-interactive parity: a "shared" binding persists the + placeholder even when the shared env var happens to be unset in + THIS process's environment (it reuses the other instance's stored + value at runtime, so it must not be treated as missing).""" + from amplifier_app_cli.provider_config_utils import configure_provider + + mock_key_manager = MagicMock() + with patch( + "amplifier_app_cli.provider_config_utils.get_provider_info", + return_value=_mock_provider_info(), + ): + config = configure_provider( + "provider-anthropic", + mock_key_manager, + non_interactive=True, + credential_binding_modes={"ANTHROPIC_API_KEY": "shared"}, + ) + + assert config is not None + assert config["api_key"] == "${ANTHROPIC_API_KEY}" + + def test_non_interactive_separate_binding_persists_placeholder_when_unset(self): + """Non-interactive parity: a "separate" binding persists the + placeholder unset for runtime injection instead of silently + skipping the field.""" + from amplifier_app_cli.provider_config_utils import configure_provider + + mock_key_manager = MagicMock() + with patch( + "amplifier_app_cli.provider_config_utils.get_provider_info", + return_value=_mock_provider_info(env_var="ANTHROPIC_WORK_API_KEY"), + ): + config = configure_provider( + "provider-anthropic", + mock_key_manager, + non_interactive=True, + credential_binding_modes={"ANTHROPIC_WORK_API_KEY": "separate"}, + ) + + assert config is not None + assert config["api_key"] == "${ANTHROPIC_WORK_API_KEY}" + + def test_non_interactive_explicit_binding_exempt_from_fail_loud(self, tmp_path): + """An explicit binding mode (shared or separate) is intentional, so + it must be exempt from the §5.4.5 non-interactive collision + fail-loud guard even when the type default is already claimed.""" + from amplifier_app_cli.provider_config_utils import configure_provider + + settings = _make_settings(tmp_path) + _seed_provider( + settings, + "provider-anthropic", + {"api_key": "${ANTHROPIC_API_KEY}"}, + provider_id="anthropic-opus", + scope="global", + ) + mock_key_manager = MagicMock() + + with patch( + "amplifier_app_cli.provider_config_utils.get_provider_info", + return_value=_mock_provider_info(), + ): + # No exception, even though ANTHROPIC_API_KEY is already claimed + # by anthropic-opus -- the "shared" mode makes this intentional. + config = configure_provider( + "provider-anthropic", + mock_key_manager, + non_interactive=True, + settings=settings, + credential_binding_modes={"ANTHROPIC_API_KEY": "shared"}, + ) + + assert config is not None + assert config["api_key"] == "${ANTHROPIC_API_KEY}" + + def test_first_instance_blank_required_secret_still_errors(self, monkeypatch): + """Scope guard: WITHOUT an explicit per-instance binding mode, a + blank required secret still errors (today's first-time-setup + guardrail is unchanged).""" + from amplifier_app_cli.provider_config_utils import _prompt_for_field + + # Use a name guaranteed unset in the ambient environment so the test + # exercises the "no existing value" required-field path deterministically. + monkeypatch.delenv("UNSET_FIRST_INSTANCE_API_KEY", raising=False) + field = { + "id": "api_key", + "display_name": "API Key", + "field_type": "secret", + "prompt": "Enter your API key", + "env_var": "UNSET_FIRST_INSTANCE_API_KEY", + "required": True, + } + mock_key_manager = MagicMock() + + with patch( + "amplifier_app_cli.provider_config_utils.Prompt.ask", + return_value="", + ): + with pytest.raises(ValueError): + _prompt_for_field( + field, + mock_key_manager, + collected_config={}, + existing_config=None, + ) + + # ============================================================ # Silent-clobber race regression (§5.4.1 fix): a still-unnormalized # literal secret for an EXISTING instance must not let a brand-new @@ -901,15 +1303,20 @@ def test_prefill_uses_live_env_value_when_set(self, tmp_path, monkeypatch): ), patch( "amplifier_app_cli.commands.provider.Prompt.ask", - return_value="ANTHROPIC_FABLE_API_KEY", + # 1st prompt = reuse-vs-separate fork (choose separate); + # 2nd prompt = the per-instance env var name. + side_effect=["2", "ANTHROPIC_FABLE_API_KEY"], ) as mock_ask, ): - overrides = _resolve_env_var_overrides( + decision = _resolve_env_var_overrides( settings, mock_key_manager, "provider-anthropic", "anthropic-fable" ) - assert overrides == {"ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY"} - # default= kwarg on the prompt call should be the derived suggestion + assert decision.env_var_overrides == { + "ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY" + } + assert decision.binding_modes == {"ANTHROPIC_FABLE_API_KEY": "separate"} + # default= kwarg on the LAST (name) prompt should be the derived suggestion _, kwargs = mock_ask.call_args assert kwargs.get("default") == "ANTHROPIC_FABLE_API_KEY" @@ -938,14 +1345,16 @@ def test_no_prefill_note_when_env_var_not_set(self, tmp_path, monkeypatch): ), patch( "amplifier_app_cli.commands.provider.Prompt.ask", - return_value="ANTHROPIC_FABLE_API_KEY", + side_effect=["2", "ANTHROPIC_FABLE_API_KEY"], ), ): - overrides = _resolve_env_var_overrides( + decision = _resolve_env_var_overrides( settings, mock_key_manager, "provider-anthropic", "anthropic-fable" ) - assert overrides == {"ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY"} + assert decision.env_var_overrides == { + "ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY" + } # ============================================================ @@ -982,7 +1391,7 @@ def test_warns_and_reuses_keys_env_only_leftover(self, tmp_path, monkeypatch): ), patch( "amplifier_app_cli.commands.provider.Prompt.ask", - return_value="ANTHROPIC_FABLE_API_KEY", + side_effect=["2", "ANTHROPIC_FABLE_API_KEY"], ), patch("amplifier_app_cli.commands.provider.console") as mock_console, ): @@ -1035,15 +1444,17 @@ def test_real_key_manager_leftover_is_reused_not_rejected( ), patch( "amplifier_app_cli.commands.provider.Prompt.ask", - return_value="ANTHROPIC_FABLE_API_KEY", + side_effect=["2", "ANTHROPIC_FABLE_API_KEY"], ), patch("amplifier_app_cli.commands.provider.console") as mock_console, ): - overrides = _resolve_env_var_overrides( + decision = _resolve_env_var_overrides( settings, key_manager, "provider-anthropic", "anthropic-fable" ) - assert overrides == {"ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY"} + assert decision.env_var_overrides == { + "ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY" + } printed = " ".join(str(c) for c in mock_console.print.call_args_list) assert "stored credential" in printed.lower() assert "already in use by another" not in printed.lower() @@ -1077,14 +1488,15 @@ def test_first_instance_gets_no_collision_prompt(self, tmp_path, monkeypatch): ), patch("amplifier_app_cli.commands.provider.Prompt.ask") as mock_ask, ): - overrides = _resolve_env_var_overrides( + decision = _resolve_env_var_overrides( settings, MagicMock(), "provider-anthropic", None ) - assert overrides == {}, ( + assert decision.env_var_overrides == {}, ( "A keys.env-only leftover is owned by no instance -- the first " "instance of the type must use the type default (§5.2 step 3)." ) + assert decision.binding_modes == {} mock_ask.assert_not_called() def test_second_instance_still_collides(self, tmp_path, monkeypatch): @@ -1112,15 +1524,18 @@ def test_second_instance_still_collides(self, tmp_path, monkeypatch): ), patch( "amplifier_app_cli.commands.provider.Prompt.ask", - return_value="ANTHROPIC_FABLE_API_KEY", + side_effect=["2", "ANTHROPIC_FABLE_API_KEY"], ) as mock_ask, ): - overrides = _resolve_env_var_overrides( + decision = _resolve_env_var_overrides( settings, mock_key_manager, "provider-anthropic", "anthropic-fable" ) - assert overrides == {"ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY"} - mock_ask.assert_called_once() + assert decision.env_var_overrides == { + "ANTHROPIC_API_KEY": "ANTHROPIC_FABLE_API_KEY" + } + # Collision path entered: fork prompt (choice) + separate-name prompt. + assert mock_ask.call_count == 2 def test_provider_add_cli_writes_entry_despite_stale_key( self, tmp_path, monkeypatch @@ -1369,8 +1784,11 @@ def test_provider_add_cli_exits_on_degenerate_suggestion(self, tmp_path): return_value="ANTHROPIC_API_KEY", ), ): - # instance_id "---" sanitizes to an empty suffix. - result = runner.invoke(provider, ["add", "anthropic"], input="---\n") + # instance_id "---" sanitizes to an empty suffix. Choose the + # SEPARATE credential path ("2") so the degenerate id reaches + # _suggest_instance_env_var and fails loudly (the reuse path + # never derives a name, so it wouldn't exercise this guard). + result = runner.invoke(provider, ["add", "anthropic"], input="---\n2\n") assert result.exit_code != 0 diff --git a/tests/test_runtime_credential_validation.py b/tests/test_runtime_credential_validation.py new file mode 100644 index 0000000..4a69148 --- /dev/null +++ b/tests/test_runtime_credential_validation.py @@ -0,0 +1,276 @@ +"""Tests for the runtime credential-fallback hardening. + +Covers `_validate_provider_credentials()` (amplifier_app_cli.runtime.config), +added alongside the reuse-or-separate multi-instance credential wizard +(docs/designs/provider-instance-credentials.md). Without this guard, an +unresolved *separate* credential placeholder expands to "" at runtime and +several provider modules treat that as "not configured", silently falling +back to their own canonical ambient env var -- routing the "separate" +instance through the WRONG account's key. This must fail loudly instead. +""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from amplifier_app_cli.runtime.config import _validate_provider_credentials + + +def _provider_info(env_var: str = "OPENAI_API_KEY", required: bool = True) -> dict: + return { + "display_name": "OpenAI", + "config_fields": [ + { + "id": "api_key", + "display_name": "API Key", + "field_type": "secret", + "prompt": "Enter your API key", + "env_var": env_var, + "required": required, + } + ], + } + + +class TestValidateProviderCredentials: + def test_unset_separate_binding_fails_loud(self, monkeypatch): + """The core hardening case: a required secret field whose ${VAR} + placeholder resolves to nothing must raise BEFORE session mount, + instead of silently letting expand_env_vars turn it into "" (which + would let the provider module fall back to its own ambient var -- + e.g. a *different* account's OPENAI_API_KEY).""" + monkeypatch.delenv("OPENAI_WORK_API_KEY", raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "sk-shared-account-key") + + providers = [ + { + "module": "provider-openai", + "id": "openai-work", + "config": {"api_key": "${OPENAI_WORK_API_KEY}"}, + } + ] + + with patch( + "amplifier_app_cli.runtime.config.get_provider_info", + return_value=_provider_info(env_var="OPENAI_WORK_API_KEY"), + ), pytest.raises(ValueError, match="OPENAI_WORK_API_KEY"): + _validate_provider_credentials(providers) + + def test_set_credential_does_not_raise(self, monkeypatch): + """A required secret field whose env var IS set must pass silently.""" + monkeypatch.setenv("OPENAI_WORK_API_KEY", "sk-distinct-value") + + providers = [ + { + "module": "provider-openai", + "id": "openai-work", + "config": {"api_key": "${OPENAI_WORK_API_KEY}"}, + } + ] + + with patch( + "amplifier_app_cli.runtime.config.get_provider_info", + return_value=_provider_info(env_var="OPENAI_WORK_API_KEY"), + ): + _validate_provider_credentials(providers) # must not raise + + def test_shared_binding_with_set_var_does_not_raise(self, monkeypatch): + """Two instances sharing the SAME credential env var (the "reuse" + path) must not raise as long as that shared var is set -- this is + the normal, intended shared-credential configuration.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-shared-value") + + providers = [ + { + "module": "provider-anthropic", + "id": "anthropic-opus", + "config": {"api_key": "${ANTHROPIC_API_KEY}"}, + }, + { + "module": "provider-anthropic", + "id": "anthropic-sonnet", + "config": {"api_key": "${ANTHROPIC_API_KEY}"}, + }, + ] + + with patch( + "amplifier_app_cli.runtime.config.get_provider_info", + return_value=_provider_info(env_var="ANTHROPIC_API_KEY"), + ): + _validate_provider_credentials(providers) # must not raise + + def test_optional_keyless_secret_field_unset_does_not_raise(self, monkeypatch): + """A secret field declared `required=False` (e.g. a local/keyless + Chat Completions server) must be left alone even when unset -- + the fail-loud guard is scoped to REQUIRED credential fields only.""" + monkeypatch.delenv("LOCAL_SERVER_API_KEY", raising=False) + + providers = [ + { + "module": "provider-chat-completions", + "id": "local-server", + "config": {"api_key": "${LOCAL_SERVER_API_KEY}"}, + } + ] + + with patch( + "amplifier_app_cli.runtime.config.get_provider_info", + return_value=_provider_info(env_var="LOCAL_SERVER_API_KEY", required=False), + ): + _validate_provider_credentials(providers) # must not raise + + def test_missing_provider_metadata_skips_validation(self, monkeypatch): + """When provider metadata can't be loaded (custom/removed provider, + import error, etc.) validation is skipped for that entry rather + than blocking session start -- consistent with how the rest of + this module treats a missing get_provider_info() result.""" + monkeypatch.delenv("SOME_UNSET_VAR", raising=False) + + providers = [ + { + "module": "provider-custom-thing", + "id": "custom", + "config": {"api_key": "${SOME_UNSET_VAR}"}, + } + ] + + with patch( + "amplifier_app_cli.runtime.config.get_provider_info", + return_value=None, + ): + _validate_provider_credentials(providers) # must not raise + + def test_literal_value_is_not_validated(self, monkeypatch): + """A literal (non-placeholder) config value is untouched by this + guard -- it isn't an unresolved env var reference at all.""" + providers = [ + { + "module": "provider-openai", + "id": "openai-work", + "config": {"api_key": "sk-literal-value-not-a-placeholder"}, + } + ] + + with patch( + "amplifier_app_cli.runtime.config.get_provider_info", + return_value=_provider_info(env_var="OPENAI_WORK_API_KEY"), + ): + _validate_provider_credentials(providers) # must not raise + + def test_inline_default_placeholder_is_not_validated(self, monkeypatch): + """A placeholder carrying an inline default (${VAR:default}) already + has its own "unset" handling via expand_env_vars -- this guard only + concerns itself with bare ${VAR} placeholders.""" + monkeypatch.delenv("OPENAI_WORK_API_KEY", raising=False) + + providers = [ + { + "module": "provider-openai", + "id": "openai-work", + "config": {"api_key": "${OPENAI_WORK_API_KEY:not-needed}"}, + } + ] + + with patch( + "amplifier_app_cli.runtime.config.get_provider_info", + return_value=_provider_info(env_var="OPENAI_WORK_API_KEY"), + ): + _validate_provider_credentials(providers) # must not raise + + def test_multiple_providers_identifies_the_failing_instance(self, monkeypatch): + """With several configured providers, the error must name the + instance and env var actually at fault, not a generic message.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-fine") + monkeypatch.delenv("OPENAI_WORK_API_KEY", raising=False) + + providers = [ + { + "module": "provider-anthropic", + "id": "anthropic-opus", + "config": {"api_key": "${ANTHROPIC_API_KEY}"}, + }, + { + "module": "provider-openai", + "id": "openai-work", + "config": {"api_key": "${OPENAI_WORK_API_KEY}"}, + }, + ] + + def _info(module_id: str): + if module_id == "provider-anthropic": + return _provider_info(env_var="ANTHROPIC_API_KEY") + return _provider_info(env_var="OPENAI_WORK_API_KEY") + + with patch( + "amplifier_app_cli.runtime.config.get_provider_info", + side_effect=_info, + ), pytest.raises(ValueError) as exc_info: + _validate_provider_credentials(providers) + + assert "OPENAI_WORK_API_KEY" in str(exc_info.value) + assert "openai-work" in str(exc_info.value) + + def test_non_dict_or_non_module_entries_are_skipped(self): + """Malformed provider entries must not crash the guard.""" + providers = [ + "not-a-dict", + {"config": {"api_key": "${SOMETHING}"}}, # no "module" + {"module": 123, "config": {"api_key": "${SOMETHING}"}}, # bad module type + {"module": "provider-x", "config": "not-a-dict"}, # bad config type + ] + _validate_provider_credentials(providers) # must not raise + + +# ============================================================ +# Integration: the guard actually runs inside resolve_bundle_config(), +# before expand_env_vars() would otherwise silently launder the unset +# placeholder into "". +# ============================================================ + + +class TestValidateProviderCredentialsIntegration: + @pytest.mark.asyncio + async def test_resolve_bundle_config_raises_for_unset_separate_credential( + self, monkeypatch + ): + from amplifier_app_cli.runtime.config import resolve_bundle_config + + monkeypatch.delenv("OPENAI_WORK_API_KEY", raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "sk-shared-account-key") + + mount_plan = { + "providers": [ + { + "module": "provider-openai", + "id": "openai-work", + "config": {"api_key": "${OPENAI_WORK_API_KEY}"}, + } + ], + } + mock_prepared = MagicMock() + mock_prepared.mount_plan = mount_plan + mock_prepared.bundle.load_agent_metadata = MagicMock() + + settings = MagicMock() + settings.get_config_overrides.return_value = {} + settings.get_provider_overrides.return_value = [] + settings.get_tool_overrides.return_value = [] + settings.get_notification_hook_overrides.return_value = [] + settings.get_routing_config.return_value = None + settings.get_source_overrides.return_value = {} + settings.get_module_sources.return_value = {} + settings.get_bundle_sources.return_value = {} + + with ( + patch( + "amplifier_app_cli.lib.bundle_loader.prepare.load_and_prepare_bundle", + new_callable=AsyncMock, + return_value=mock_prepared, + ), + patch("amplifier_app_cli.paths.get_bundle_search_paths", return_value=[]), + patch("amplifier_app_cli.lib.bundle_loader.AppBundleDiscovery"), + patch( + "amplifier_app_cli.runtime.config.get_provider_info", + return_value=_provider_info(env_var="OPENAI_WORK_API_KEY"), + ),pytest.raises(ValueError, match="OPENAI_WORK_API_KEY") + ): + await resolve_bundle_config(bundle_name="test", app_settings=settings)