From 9d10175822317ba1ed790927b0dbbe8379c88754 Mon Sep 17 00:00:00 2001 From: sadlilas <11658960+sadlilas@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:05:42 -0700 Subject: [PATCH 1/9] security: restrict code-introducing settings to trusted scopes only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Amplifier runs inside a cloned repository it merges the folder's .amplifier/settings.yaml and .amplifier/settings.local.yaml into the run config. Certain settings are code-introducing: they install/download modules and bundles from arbitrary git/path URIs, or redirect module names to attacker-controlled sources. A malicious repo could therefore achieve code execution simply by being opened with amplifier. ## Trust rule Four settings scopes exist: global = ~/.amplifier/settings.yaml -> TRUSTED (outside cwd) session = ~/.amplifier/projects/.../settings.yaml -> TRUSTED (outside cwd) project = ./.amplifier/settings.yaml -> UNTRUSTED (inside cwd) local = ./.amplifier/settings.local.yaml -> UNTRUSTED (inside cwd) Code-introducing settings are now only honored from global + session. ## Changes ### Part A — settings.py - Add TRUSTED_CODE_SCOPES constant (global, session). - Extract _merge_paths() helper from get_merged_settings() loop body. - Add _trusted_paths() and get_trusted_settings() (global + session only). - Add trusted_only: bool = False kwarg to get_app_bundles, get_added_bundles, get_module_sources, get_bundle_sources, get_module_overrides, get_source_overrides, get_provider_overrides. - get_config_overrides() always reads the full merge (not code-introducing). ### Part B — call site flips (behavior changes) - runtime/config.py: get_app_bundles, get_source_overrides, get_module_sources, get_bundle_sources, get_added_bundles all call trusted_only=True. provider_sources extracted for Bundle.prepare also calls get_provider_overrides(trusted_only=True). - bundle_loader/resolvers.py (2 sites): get_module_sources(trusted_only=True) with TypeError guard for older protocol implementations. - bundle_loader/discovery.py (_load_user_registry): get_added_bundles(trusted_only=True). - paths.py (CLISettingsProvider): get_module_sources forwards trusted_only, get_module_source() calls it with trusted_only=True. - provider_sources.py: get_module_sources(trusted_only=True). - Management call sites (commands/, user_registry.py) unchanged — they need full visibility for list/update UX. ### Part C — strip source from runtime provider config (defense in depth) Strip the source key from every provider dict before applying provider_overrides at runtime. source is a prepare-time directive and must not ride along into the running session's provider config. ### Part D — one-time stderr notice _warn_dropped_code_config() compares full-merge vs trusted-merge for all code-introducing settings. If any differ, emits a single line to stderr: 'Ignored code-introducing configuration from this folder's .amplifier/ settings for safety: . Move it to your global (~/.amplifier/settings.yaml) settings to apply it.' ### Part E — tests tests/test_folder_config_trust_boundary.py: 21 tests covering all seven requirements from the spec (trusted_settings merge semantics, per-getter trusted_only behavior, config_overrides regression guard, management-path sanity). --- .../lib/bundle_loader/discovery.py | 4 +- .../lib/bundle_loader/resolvers.py | 29 +- amplifier_app_cli/lib/settings.py | 153 ++++-- amplifier_app_cli/paths.py | 16 +- amplifier_app_cli/provider_sources.py | 6 +- amplifier_app_cli/runtime/config.py | 111 +++- tests/test_folder_config_trust_boundary.py | 497 ++++++++++++++++++ 7 files changed, 754 insertions(+), 62 deletions(-) create mode 100644 tests/test_folder_config_trust_boundary.py diff --git a/amplifier_app_cli/lib/bundle_loader/discovery.py b/amplifier_app_cli/lib/bundle_loader/discovery.py index ed3582ff..b2c118ec 100644 --- a/amplifier_app_cli/lib/bundle_loader/discovery.py +++ b/amplifier_app_cli/lib/bundle_loader/discovery.py @@ -165,7 +165,9 @@ def _load_user_registry(self) -> None: from amplifier_app_cli.lib.settings import AppSettings app_settings = AppSettings() - added_bundles = app_settings.get_added_bundles() + # SECURITY: trusted_only=True — bundle URI mappings are code-introducing; + # a cloned repo must not redirect bundle names to attacker-controlled git sources. + added_bundles = app_settings.get_added_bundles(trusted_only=True) for name, uri in added_bundles.items(): self._registry.register({name: uri}) logger.debug(f"Loaded user bundle '{name}' → {uri}") diff --git a/amplifier_app_cli/lib/bundle_loader/resolvers.py b/amplifier_app_cli/lib/bundle_loader/resolvers.py index 0bbd7b26..4e01e9a6 100644 --- a/amplifier_app_cli/lib/bundle_loader/resolvers.py +++ b/amplifier_app_cli/lib/bundle_loader/resolvers.py @@ -184,8 +184,13 @@ def __repr__(self) -> str: class SettingsProviderProtocol(Protocol): """Interface for settings access.""" - def get_module_sources(self) -> dict[str, str]: - """Get module source overrides from settings.""" + def get_module_sources(self, *, trusted_only: bool = False) -> dict[str, str]: + """Get module source overrides from settings. + + Args: + trusted_only: When True, return only sources from trusted scopes + (global + session), excluding project/local scopes. + """ ... @@ -265,9 +270,15 @@ def resolve_with_layer( logger.debug(f"[module:resolve] {module_id} -> workspace") return (workspace_source, "workspace") - # Layer 3: Settings provider (collapsed project + user settings) + # Layer 3: Settings provider (trusted scopes only at run time). + # SECURITY: pass trusted_only=True so folder-local settings cannot redirect + # module resolution to arbitrary sources. Guard with TypeError for protocol + # implementations that pre-date the trusted_only parameter. if self.settings_provider: - sources = self.settings_provider.get_module_sources() + try: + sources = self.settings_provider.get_module_sources(trusted_only=True) + except TypeError: + sources = self.settings_provider.get_module_sources() if module_id in sources: logger.debug(f"[module:resolve] {module_id} -> settings") return (self._parse_source(sources[module_id], module_id), "settings") @@ -401,9 +412,15 @@ def get_module_source(self, module_id: str) -> str | None: Returns: String source URI, or None if not found. """ - # Check settings provider + # Check settings provider (trusted scopes only at run time). + # SECURITY: pass trusted_only=True so folder-local settings cannot redirect + # module resolution to arbitrary sources. Guard with TypeError for protocol + # implementations that pre-date the trusted_only parameter. if self.settings_provider: - sources = self.settings_provider.get_module_sources() + try: + sources = self.settings_provider.get_module_sources(trusted_only=True) + except TypeError: + sources = self.settings_provider.get_module_sources() if module_id in sources: return sources[module_id] diff --git a/amplifier_app_cli/lib/settings.py b/amplifier_app_cli/lib/settings.py index 4a517113..927bdc79 100644 --- a/amplifier_app_cli/lib/settings.py +++ b/amplifier_app_cli/lib/settings.py @@ -17,6 +17,11 @@ # Backward compatibility alias (OLD AppSettings used ScopeType without "session") ScopeType = Literal["local", "project", "global"] +# Scopes that live OUTSIDE the working directory; only these may introduce or point +# at code (module/bundle sources, source overrides, app/added bundle URIs). +# project/local live inside the possibly-cloned working directory and must never do so. +TRUSTED_CODE_SCOPES = ("global", "session") + @dataclass(frozen=True) class NotificationFlags: @@ -95,9 +100,28 @@ def with_session(self, session_id: str, project_slug: str) -> "AppSettings": new_paths = SettingsPaths.with_session(session_id, project_slug) return AppSettings(new_paths) + def _merge_paths(self, paths: list[Path]) -> dict[str, Any]: + """Merge settings from the given ordered list of paths (later paths win).""" + result: dict[str, Any] = {} + for path in paths: + if path.exists(): + try: + with open(path, encoding="utf-8") as f: + content = yaml.safe_load(f) or {} + result = self._deep_merge(result, content) + except Exception: + pass # Skip malformed files + return result + + def _trusted_paths(self) -> list[Path]: + """Return the ordered list of trusted (outside-cwd) settings paths.""" + paths = [self.paths.global_settings] + if self.paths.session_settings: + paths.append(self.paths.session_settings) + return paths + def get_merged_settings(self) -> dict[str, Any]: """Load and merge settings from all scopes.""" - result: dict[str, Any] = {} # Order: global -> project -> local -> session (most specific wins) paths_to_check = [ self.paths.global_settings, @@ -106,16 +130,17 @@ def get_merged_settings(self) -> dict[str, Any]: ] if self.paths.session_settings: paths_to_check.append(self.paths.session_settings) + return self._merge_paths(paths_to_check) - for path in paths_to_check: - if path.exists(): - try: - with open(path, encoding="utf-8") as f: - content = yaml.safe_load(f) or {} - result = self._deep_merge(result, content) - except Exception: - pass # Skip malformed files - return result + def get_trusted_settings(self) -> dict[str, Any]: + """Merge ONLY scopes outside the working directory (global + session). + + Use this instead of get_merged_settings() when reading code-introducing + settings (module/bundle sources, source overrides, app/added bundle URIs). + project/local scopes live inside the possibly-cloned working directory and + must not be allowed to introduce or redirect code. + """ + return self._merge_paths(self._trusted_paths()) # ----- Bundle settings ----- @@ -158,15 +183,21 @@ def clear_active_bundle(self, scope: Scope = "global") -> None: # ----- App bundle settings ----- - def get_app_bundles(self) -> list[str]: + def get_app_bundles(self, *, trusted_only: bool = False) -> list[str]: """Get list of app bundle URIs that are always composed. App bundles are composed onto every session AFTER the primary bundle. They enable team-wide or user-wide behaviors. Reads from bundle.app setting (list of URIs). + + Args: + trusted_only: When True, read only from trusted scopes (global + session). + Use for run/prepare paths to prevent folder-origin code injection. """ - settings = self.get_merged_settings() + settings = ( + self.get_trusted_settings() if trusted_only else self.get_merged_settings() + ) bundle_settings = settings.get("bundle") or {} app_bundles = bundle_settings.get("app", []) return app_bundles if isinstance(app_bundles, list) else [] @@ -213,16 +244,22 @@ def remove_app_bundle(self, uri: str, scope: Scope = "global") -> bool: # User-added bundles via `bundle add` - name → URI mappings # This replaces the separate bundle-registry.yaml file - def get_added_bundles(self) -> dict[str, str]: + def get_added_bundles(self, *, trusted_only: bool = False) -> dict[str, str]: """Get user-added bundle mappings (name → URI). Returns merged bundle.added from all scopes. Also migrates from legacy bundle-registry.yaml if present. + + Args: + trusted_only: When True, read only from trusted scopes (global + session). + Use for run/prepare paths to prevent folder-origin code injection. """ # First, check for legacy bundle-registry.yaml and migrate if needed self._migrate_legacy_registry() - settings = self.get_merged_settings() + settings = ( + self.get_trusted_settings() if trusted_only else self.get_merged_settings() + ) bundle_settings = settings.get("bundle") or {} added = bundle_settings.get("added", {}) return added if isinstance(added, dict) else {} @@ -335,24 +372,39 @@ def clear_provider(self, scope: Scope = "global") -> None: # ----- Provider override settings (config.providers) ----- - def get_provider_overrides(self) -> list[dict[str, Any]]: + def get_provider_overrides( + self, *, trusted_only: bool = False + ) -> list[dict[str, Any]]: """Return merged provider overrides from config.providers across all scopes. Merges by provider identity key (id if present, else module). More-specific scopes override less-specific: global < project < local < session. Providers not present in higher scopes pass through from lower scopes. + + Args: + trusted_only: When True, merge only global + session scopes (skip project + + local). Use for run/prepare paths to prevent folder-origin + provider sources from injecting code. """ from .merge_utils import _provider_key, merge_module_items # noqa: F401 result: list[dict[str, Any]] = [] - # Scope priority order: global (lowest) → project → local → session (highest) - paths_to_check: list[Path | None] = [ - self.paths.global_settings, - self.paths.project_settings, - self.paths.local_settings, - self.paths.session_settings, - ] + # When trusted_only=True, skip project + local (those live inside the cwd). + # Order is always: global (lowest) → [project → local] → session (highest). + if trusted_only: + paths_to_check: list[Path | None] = [ + self.paths.global_settings, + self.paths.session_settings, + ] + else: + # Scope priority order: global (lowest) → project → local → session (highest) + paths_to_check = [ + self.paths.global_settings, + self.paths.project_settings, + self.paths.local_settings, + self.paths.session_settings, + ] for path in paths_to_check: if path is None or not path.exists(): @@ -466,9 +518,16 @@ def get_overrides(self) -> dict[str, Any]: # ----- Source override settings ----- - def get_module_sources(self) -> dict[str, str]: - """Get merged module source overrides from all scopes.""" - settings = self.get_merged_settings() + def get_module_sources(self, *, trusted_only: bool = False) -> dict[str, str]: + """Get merged module source overrides from all scopes. + + Args: + trusted_only: When True, read only from trusted scopes (global + session). + Use for run/prepare paths to prevent folder-origin code injection. + """ + settings = ( + self.get_trusted_settings() if trusted_only else self.get_merged_settings() + ) return settings.get("sources", {}).get("modules", {}) def add_source_override( @@ -498,9 +557,16 @@ def remove_source_override(self, identifier: str, scope: Scope = "global") -> bo return True return False - def get_bundle_sources(self) -> dict[str, str]: - """Get merged bundle source overrides from all scopes.""" - settings = self.get_merged_settings() + def get_bundle_sources(self, *, trusted_only: bool = False) -> dict[str, str]: + """Get merged bundle source overrides from all scopes. + + Args: + trusted_only: When True, read only from trusted scopes (global + session). + Use for run/prepare paths to prevent folder-origin code injection. + """ + settings = ( + self.get_trusted_settings() if trusted_only else self.get_merged_settings() + ) return settings.get("sources", {}).get("bundles", {}) def add_bundle_source_override( @@ -1092,7 +1158,9 @@ def _merge_tool_configs( # ----- Module override settings (overrides section) ----- - def get_module_overrides(self) -> dict[str, dict[str, Any]]: + def get_module_overrides( + self, *, trusted_only: bool = False + ) -> dict[str, dict[str, Any]]: """Return unified module overrides from overrides section. Expected structure: @@ -1101,17 +1169,27 @@ def get_module_overrides(self) -> dict[str, dict[str, Any]]: source: /local/path/to/module config: inherit_context: recent + + Args: + trusted_only: When True, read only from trusted scopes (global + session). + Use for run/prepare paths to prevent folder-origin code injection. """ - settings = self.get_merged_settings() + settings = ( + self.get_trusted_settings() if trusted_only else self.get_merged_settings() + ) overrides = settings.get("overrides", {}) return overrides if isinstance(overrides, dict) else {} - def get_source_overrides(self) -> dict[str, str]: + def get_source_overrides(self, *, trusted_only: bool = False) -> dict[str, str]: """Return source overrides only (module_id -> source_uri). Convenience method for Bundle.prepare(source_resolver=...). + + Args: + trusted_only: When True, read only from trusted scopes (global + session). + Use for run/prepare paths to prevent folder-origin code injection. """ - overrides = self.get_module_overrides() + overrides = self.get_module_overrides(trusted_only=trusted_only) return { module_id: override["source"] for module_id, override in overrides.items() @@ -1119,8 +1197,15 @@ def get_source_overrides(self) -> dict[str, str]: } def get_config_overrides(self) -> dict[str, dict[str, Any]]: - """Return config overrides only (module_id -> config_dict).""" - overrides = self.get_module_overrides() + """Return config overrides only (module_id -> config_dict). + + Always reads from the full merge (all scopes). Config values are + select-by-name, not code-introducing, so folder-local overrides are + permitted here. Do NOT add trusted_only to this method. + """ + overrides = ( + self.get_module_overrides() + ) # Always full-merge — not code-introducing return { module_id: override.get("config", {}) for module_id, override in overrides.items() diff --git a/amplifier_app_cli/paths.py b/amplifier_app_cli/paths.py index 6a5c5f00..1dbdc02b 100644 --- a/amplifier_app_cli/paths.py +++ b/amplifier_app_cli/paths.py @@ -410,7 +410,7 @@ def create_foundation_resolver() -> "FoundationSettingsResolver": class CLISettingsProvider: """CLI implementation of SettingsProviderProtocol.""" - def get_module_sources(self) -> dict[str, str]: + def get_module_sources(self, *, trusted_only: bool = False) -> dict[str, str]: """Get all module sources from CLI settings. Merges sources from multiple locations: @@ -421,9 +421,15 @@ def get_module_sources(self) -> dict[str, str]: Module-specific sources take precedence over explicit overrides to ensure user-added modules are properly resolved. + + Args: + trusted_only: When True, read only from trusted scopes (global + session) + for the explicit sources.modules lookup. Pass-through to + AppSettings.get_module_sources(trusted_only=trusted_only). """ - # Start with explicit source overrides - sources = dict(settings.get_module_sources()) + # Start with explicit source overrides (SECURITY: pass trusted_only through + # so folder-local sources.modules cannot inject arbitrary module sources). + sources = dict(settings.get_module_sources(trusted_only=trusted_only)) # Extract sources from registered modules (modules.providers[], modules.tools[], etc.) merged = settings.get_merged_settings() @@ -450,8 +456,8 @@ def get_module_sources(self) -> dict[str, str]: return sources def get_module_source(self, module_id: str) -> str | None: - """Get module source from CLI settings.""" - return self.get_module_sources().get(module_id) + """Get module source from CLI settings (trusted scopes only at run time).""" + return self.get_module_sources(trusted_only=True).get(module_id) return FoundationSettingsResolver( settings_provider=CLISettingsProvider(), diff --git a/amplifier_app_cli/provider_sources.py b/amplifier_app_cli/provider_sources.py index db44b185..2a01186e 100644 --- a/amplifier_app_cli/provider_sources.py +++ b/amplifier_app_cli/provider_sources.py @@ -102,8 +102,10 @@ def get_effective_provider_sources( sources = dict(DEFAULT_PROVIDER_SOURCES) if config_manager: - # 1. Apply source overrides for known providers - overrides = config_manager.get_module_sources() + # 1. Apply source overrides for known providers. + # SECURITY: trusted_only=True — sources.modules is code-introducing; a cloned + # repo must not redirect module resolution to attacker-controlled git sources. + overrides = config_manager.get_module_sources(trusted_only=True) for module_id in list(sources.keys()): if module_id in overrides: sources[module_id] = overrides[module_id] diff --git a/amplifier_app_cli/runtime/config.py b/amplifier_app_cli/runtime/config.py index 45b7992a..273c83fa 100644 --- a/amplifier_app_cli/runtime/config.py +++ b/amplifier_app_cli/runtime/config.py @@ -6,6 +6,7 @@ import logging import os import re +import sys from typing import TYPE_CHECKING from typing import Any @@ -22,6 +23,61 @@ logger = logging.getLogger(__name__) +def _warn_dropped_code_config( + app_settings: AppSettings, + *, + trusted_app_bundles: list[str], + trusted_module_sources: dict[str, str], + trusted_bundle_sources: dict[str, str], + trusted_source_overrides: dict[str, str], + trusted_added_bundles: dict[str, str], + trusted_provider_sources: dict[str, str], +) -> None: + """Emit a single stderr notice when project/local scopes contain code-introducing config. + + Compares full-merge values against trusted-only values to detect settings that were + silenced. Emits at most one line so the user knows their folder config was ignored + without listing attacker-controlled URIs or values. + """ + dropped: list[str] = [] + + # Full-merge counterparts (includes project + local scopes) + full_app_bundles = app_settings.get_app_bundles() + full_module_sources = app_settings.get_module_sources() + full_bundle_sources = app_settings.get_bundle_sources() + full_source_overrides = app_settings.get_source_overrides() + full_added_bundles = app_settings.get_added_bundles() + + full_provider_overrides = app_settings.get_provider_overrides() + full_provider_sources = { + p["module"]: p["source"] + for p in full_provider_overrides + if isinstance(p, dict) and "module" in p and "source" in p + } + + if set(full_app_bundles) != set(trusted_app_bundles): + dropped.append("app bundles") + if full_module_sources != trusted_module_sources: + dropped.append("module sources") + if full_bundle_sources != trusted_bundle_sources: + dropped.append("bundle sources") + if full_source_overrides != trusted_source_overrides: + dropped.append("source overrides") + if full_added_bundles != trusted_added_bundles: + dropped.append("added bundles") + if full_provider_sources != trusted_provider_sources: + dropped.append("provider sources") + + if dropped: + labels = ", ".join(dropped) + print( + f"Ignored code-introducing configuration from this folder's .amplifier/ settings " + f"for safety: {labels}. " + f"Move it to your global (~/.amplifier/settings.yaml) settings to apply it.", + file=sys.stderr, + ) + + async def resolve_bundle_config( bundle_name: str, app_settings: AppSettings, @@ -92,26 +148,32 @@ def _on_progress(action: str, detail: str) -> None: _build_notification_behaviors(app_settings.get_notification_flags()) ) - # Add app bundles (user-configured bundles that are always composed) - # App bundles are explicit user configuration, composed AFTER notification behaviors - app_bundles = app_settings.get_app_bundles() + # Add app bundles (user-configured bundles that are always composed). + # SECURITY: trusted_only=True — only honor app bundles from global/session + # scopes (outside the working directory). A cloned repo must not be able + # to inject arbitrary bundles by placing them in .amplifier/settings.yaml. + app_bundles = app_settings.get_app_bundles(trusted_only=True) if app_bundles: compose_behaviors = compose_behaviors + app_bundles - # Get source overrides from unified settings - # This enables settings.yaml overrides to take effect at prepare time - source_overrides = app_settings.get_source_overrides() + # Get source overrides from unified settings. + # SECURITY: trusted_only=True — overrides..source is code-introducing. + source_overrides = app_settings.get_source_overrides(trusted_only=True) - # Get module sources from 'amplifier source add' (sources.modules in settings.yaml) - module_sources = app_settings.get_module_sources() + # Get module sources from 'amplifier source add' (sources.modules in settings.yaml). + # SECURITY: trusted_only=True — sources.modules is code-introducing. + module_sources = app_settings.get_module_sources(trusted_only=True) - # CRITICAL: Also extract provider sources from config.providers[] + # CRITICAL: Also extract provider sources from config.providers[]. # Providers are configured via 'amplifier provider use' and stored in config.providers, # not in overrides section. Bundle.prepare() needs these sources to download provider modules. - provider_overrides = app_settings.get_provider_overrides() + # SECURITY: trusted_only=True — provider source URIs are code-introducing. + provider_overrides_for_prepare = app_settings.get_provider_overrides( + trusted_only=True + ) provider_sources = { provider["module"]: provider["source"] - for provider in provider_overrides + for provider in provider_overrides_for_prepare if isinstance(provider, dict) and "module" in provider and "source" in provider @@ -121,8 +183,21 @@ def _on_progress(action: str, detail: str) -> None: # sources.modules (general) < overrides..source (specific) < config.providers[].source (most specific) combined_sources = {**module_sources, **source_overrides, **provider_sources} - # Get bundle source overrides from settings (sources.bundles in settings.yaml) - bundle_sources = app_settings.get_bundle_sources() + # Get bundle source overrides from settings (sources.bundles in settings.yaml). + # SECURITY: trusted_only=True — sources.bundles is code-introducing. + bundle_sources = app_settings.get_bundle_sources(trusted_only=True) + + # Detect code-introducing settings from project/local scopes that were silenced, + # and emit a single warning so the user understands why their folder config was ignored. + _warn_dropped_code_config( + app_settings, + trusted_app_bundles=app_bundles, + trusted_module_sources=module_sources, + trusted_bundle_sources=bundle_sources, + trusted_source_overrides=source_overrides, + trusted_added_bundles=app_settings.get_added_bundles(trusted_only=True), + trusted_provider_sources=provider_sources, + ) # Load and prepare bundle (downloads modules from git sources) # If compose_behaviors is provided, those behaviors are composed onto the bundle @@ -169,8 +244,16 @@ def _on_progress(action: str, detail: str) -> None: base_cfg = item.get("config", {}) or {} item["config"] = deep_merge(base_cfg, override_cfg) - # Apply provider overrides + # Apply provider overrides (full merge — runtime config is policy, not source resolution). + # `source` is a prepare/install-time directive: it was already honored above via + # get_provider_overrides(trusted_only=True) when building provider_sources for + # Bundle.prepare(). Strip it here so a folder-origin provider entry cannot inject + # an arbitrary module source into the running session's provider config. provider_overrides = app_settings.get_provider_overrides() + provider_overrides = [ + {k: v for k, v in p.items() if k != "source"} if isinstance(p, dict) else p + for p in provider_overrides + ] if provider_overrides: if bundle_config.get("providers"): # Bundle has providers - merge overrides with existing diff --git a/tests/test_folder_config_trust_boundary.py b/tests/test_folder_config_trust_boundary.py new file mode 100644 index 00000000..13c78fa3 --- /dev/null +++ b/tests/test_folder_config_trust_boundary.py @@ -0,0 +1,497 @@ +"""Tests for the folder-config trust boundary (security fix). + +Verifies that code-introducing settings (module/bundle sources, source overrides, +app/added bundle URIs, provider sources) are only honored from trusted scopes +(global + session), never from project/local scopes that live inside the +working directory (a possibly-cloned repository). + +Test layout: + 1. get_trusted_settings — merges only global+session; ignores project+local. + 2. get_module_sources(trusted_only=True) — excludes project-scope source, + includes global-scope source; default includes both. + 3. get_added_bundles(trusted_only=True) — excludes project-scope added bundle. + 4. get_provider_overrides(trusted_only=True) — excludes project-scope provider source. + 5. get_source_overrides(trusted_only=True) — excludes project-scope overrides..source. + 6. get_config_overrides() — STILL includes project-scope overrides..config + (regression guard: config values remain folder-honorable). + 7. Management path sanity: get_app_bundles() (default) still returns + project-scope app bundles so list/management UX is unchanged. +""" + +from pathlib import Path + +import yaml + +from amplifier_app_cli.lib.settings import AppSettings, SettingsPaths + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _write_yaml(path: Path, data: dict) -> None: + """Write a YAML file, creating parent directories as needed.""" + path.parent.mkdir(parents=True, exist_ok=True) + with open(path, "w", encoding="utf-8") as f: + yaml.safe_dump(data, f) + + +def _make_settings( + tmp_path: Path, + *, + global_data: dict | None = None, + project_data: dict | None = None, + local_data: dict | None = None, + session_data: dict | None = None, +) -> AppSettings: + """Build an AppSettings pointed at temp-dir files. + + Files are only created when non-None data is provided. + """ + global_file = tmp_path / "home" / ".amplifier" / "settings.yaml" + project_file = tmp_path / "cwd" / ".amplifier" / "settings.yaml" + local_file = tmp_path / "cwd" / ".amplifier" / "settings.local.yaml" + session_file = ( + tmp_path + / "home" + / ".amplifier" + / "projects" + / "slug" + / "sessions" + / "s1" + / "settings.yaml" + ) + + if global_data is not None: + _write_yaml(global_file, global_data) + if project_data is not None: + _write_yaml(project_file, project_data) + if local_data is not None: + _write_yaml(local_file, local_data) + if session_data is not None: + _write_yaml(session_file, session_data) + + paths = SettingsPaths( + global_settings=global_file, + project_settings=project_file, + local_settings=local_file, + session_settings=session_file if session_data is not None else None, + ) + return AppSettings(paths=paths) + + +# --------------------------------------------------------------------------- +# 1. get_trusted_settings — merges only global + session; ignores project/local +# --------------------------------------------------------------------------- + + +class TestGetTrustedSettings: + def test_includes_global_key(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={"key": "from-global"}, + project_data={"other": "from-project"}, + ) + result = settings.get_trusted_settings() + assert result.get("key") == "from-global" + + def test_excludes_project_key(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={"g": 1}, + project_data={"project_only": "should-be-absent"}, + ) + result = settings.get_trusted_settings() + assert "project_only" not in result + + def test_excludes_local_key(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={"g": 1}, + local_data={"local_only": "should-be-absent"}, + ) + result = settings.get_trusted_settings() + assert "local_only" not in result + + def test_includes_session_key(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={"g": 1}, + session_data={"session_key": "from-session"}, + ) + result = settings.get_trusted_settings() + assert result.get("session_key") == "from-session" + + def test_session_wins_over_global_on_conflict(self, tmp_path: Path) -> None: + """Session (more-specific trusted scope) must override global on conflict.""" + settings = _make_settings( + tmp_path, + global_data={"conflict": "global-value"}, + session_data={"conflict": "session-value"}, + ) + result = settings.get_trusted_settings() + assert result["conflict"] == "session-value" + + def test_project_wins_in_full_merge_but_not_trusted(self, tmp_path: Path) -> None: + """Confirms the split: full merge sees project, trusted merge does not.""" + settings = _make_settings( + tmp_path, + global_data={"key": "global"}, + project_data={"key": "project"}, + ) + assert settings.get_merged_settings()["key"] == "project" + assert settings.get_trusted_settings()["key"] == "global" + + +# --------------------------------------------------------------------------- +# 2. get_module_sources — trusted_only filters project scope +# --------------------------------------------------------------------------- + + +class TestGetModuleSources: + def test_trusted_only_excludes_project_scope_source(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={ + "sources": { + "modules": {"tool-bash": "git+https://global.example.com/tool-bash"} + } + }, + project_data={ + "sources": { + "modules": {"tool-evil": "git+https://evil.example.com/pwned"} + } + }, + ) + result = settings.get_module_sources(trusted_only=True) + assert "tool-evil" not in result, ( + "project-scope source must be excluded when trusted_only=True" + ) + assert "tool-bash" in result, "global-scope source must be present" + + def test_default_includes_both_scopes(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={ + "sources": { + "modules": {"tool-bash": "git+https://global.example.com/tool-bash"} + } + }, + project_data={ + "sources": { + "modules": {"tool-extra": "git+https://project.example.com/extra"} + } + }, + ) + result = settings.get_module_sources() + assert "tool-bash" in result + assert "tool-extra" in result + + def test_trusted_only_project_override_of_global_key_is_blocked( + self, tmp_path: Path + ) -> None: + """A project-scope source must not redirect a globally-defined module.""" + settings = _make_settings( + tmp_path, + global_data={ + "sources": { + "modules": { + "provider-anthropic": "git+https://good.example.com/anthropic" + } + } + }, + project_data={ + "sources": { + "modules": { + "provider-anthropic": "git+https://evil.example.com/anthropic" + } + } + }, + ) + trusted = settings.get_module_sources(trusted_only=True) + full = settings.get_module_sources() + assert trusted["provider-anthropic"] == "git+https://good.example.com/anthropic" + assert full["provider-anthropic"] == "git+https://evil.example.com/anthropic" + + def test_session_source_is_trusted(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={}, + session_data={ + "sources": {"modules": {"tool-x": "git+https://session.example.com/x"}} + }, + ) + result = settings.get_module_sources(trusted_only=True) + assert "tool-x" in result + + +# --------------------------------------------------------------------------- +# 3. get_added_bundles — trusted_only filters project scope +# --------------------------------------------------------------------------- + + +class TestGetAddedBundles: + def test_trusted_only_excludes_project_scope_bundle(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={ + "bundle": { + "added": {"my-bundle": "git+https://global.example.com/bundle"} + } + }, + project_data={ + "bundle": { + "added": {"evil-bundle": "git+https://evil.example.com/pwned"} + } + }, + ) + result = settings.get_added_bundles(trusted_only=True) + assert "evil-bundle" not in result + assert "my-bundle" in result + + def test_default_includes_project_scope_bundle(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={ + "bundle": { + "added": {"my-bundle": "git+https://global.example.com/bundle"} + } + }, + project_data={ + "bundle": { + "added": {"extra-bundle": "git+https://project.example.com/bundle"} + } + }, + ) + result = settings.get_added_bundles() + assert "extra-bundle" in result + + +# --------------------------------------------------------------------------- +# 4. get_provider_overrides — trusted_only filters project-scope provider source +# --------------------------------------------------------------------------- + + +class TestGetProviderOverrides: + def test_trusted_only_excludes_project_scope_provider_source( + self, tmp_path: Path + ) -> None: + """A project-scope config.providers[].source must not reach trusted merge.""" + settings = _make_settings( + tmp_path, + global_data={ + "config": { + "providers": [ + { + "module": "provider-anthropic", + "config": {"model": "claude-3-5-sonnet"}, + } + ] + } + }, + project_data={ + "config": { + "providers": [ + { + "module": "provider-anthropic", + "source": "git+https://evil.example.com/malicious-provider", + } + ] + } + }, + ) + trusted = settings.get_provider_overrides(trusted_only=True) + full = settings.get_provider_overrides() + + # trusted must not carry the malicious source + for provider in trusted: + if provider.get("module") == "provider-anthropic": + assert ( + "source" not in provider + or provider["source"] + != "git+https://evil.example.com/malicious-provider" + ), "project-scope provider source must be absent from trusted overrides" + + # full merge includes it + evil_providers = [ + p + for p in full + if p.get("module") == "provider-anthropic" + and p.get("source") == "git+https://evil.example.com/malicious-provider" + ] + assert len(evil_providers) >= 1, ( + "full merge must include the project-scope provider source" + ) + + def test_trusted_only_allows_global_provider(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={ + "config": { + "providers": [ + { + "module": "provider-anthropic", + "source": "git+https://global.example.com/anthropic", + } + ] + } + }, + ) + result = settings.get_provider_overrides(trusted_only=True) + assert any(p.get("module") == "provider-anthropic" for p in result) + + def test_default_includes_project_scope_provider(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={}, + project_data={ + "config": { + "providers": [ + { + "module": "provider-x", + "source": "git+https://project.example.com/x", + } + ] + } + }, + ) + result = settings.get_provider_overrides() + assert any(p.get("module") == "provider-x" for p in result) + + +# --------------------------------------------------------------------------- +# 5. get_source_overrides — trusted_only excludes project-scope overrides..source +# --------------------------------------------------------------------------- + + +class TestGetSourceOverrides: + def test_trusted_only_excludes_project_scope_override_source( + self, tmp_path: Path + ) -> None: + settings = _make_settings( + tmp_path, + global_data={ + "overrides": { + "tool-bash": {"source": "git+https://global.example.com/tool-bash"} + } + }, + project_data={ + "overrides": { + "tool-evil": {"source": "git+https://evil.example.com/pwned"} + } + }, + ) + result = settings.get_source_overrides(trusted_only=True) + assert "tool-evil" not in result + assert "tool-bash" in result + + def test_default_includes_project_scope_override_source( + self, tmp_path: Path + ) -> None: + settings = _make_settings( + tmp_path, + global_data={}, + project_data={ + "overrides": { + "tool-extra": {"source": "git+https://project.example.com/extra"} + } + }, + ) + result = settings.get_source_overrides() + assert "tool-extra" in result + + +# --------------------------------------------------------------------------- +# 6. get_config_overrides — project-scope .config values are STILL honored +# --------------------------------------------------------------------------- + + +class TestGetConfigOverrides: + def test_project_scope_config_values_still_included(self, tmp_path: Path) -> None: + """Regression guard: overrides..config values are not code-introducing + and must continue to work from project/local scopes. + """ + settings = _make_settings( + tmp_path, + global_data={}, + project_data={ + "overrides": { + "hooks-streaming-ui": { + "config": {"ui": {"show_thinking_stream": False}} + } + } + }, + ) + result = settings.get_config_overrides() + assert "hooks-streaming-ui" in result, ( + "project-scope overrides..config must remain visible in get_config_overrides()" + ) + assert result["hooks-streaming-ui"]["ui"]["show_thinking_stream"] is False + + def test_project_scope_source_excluded_config_included_same_module( + self, tmp_path: Path + ) -> None: + """When a project-scope override has both .source and .config, only .source is dropped + (by trusted_only=True on get_source_overrides). The .config half must still be + present in get_config_overrides() which always reads the full merge. + """ + settings = _make_settings( + tmp_path, + global_data={}, + project_data={ + "overrides": { + "tool-bash": { + "source": "git+https://evil.example.com/bash", + "config": {"allowed_commands": ["ls"]}, + } + } + }, + ) + # source must be absent from trusted source overrides + assert "tool-bash" not in settings.get_source_overrides(trusted_only=True) + + # config must still be present + config_overrides = settings.get_config_overrides() + assert "tool-bash" in config_overrides + assert config_overrides["tool-bash"]["allowed_commands"] == ["ls"] + + +# --------------------------------------------------------------------------- +# 7. Management path sanity — get_app_bundles() (default) returns project scope +# --------------------------------------------------------------------------- + + +class TestManagementPathSanity: + def test_get_app_bundles_default_includes_project_scope( + self, tmp_path: Path + ) -> None: + """get_app_bundles() with default trusted_only=False must still expose + project-scope app bundles so that list/management commands keep full visibility. + """ + settings = _make_settings( + tmp_path, + global_data={}, + project_data={ + "bundle": {"app": ["git+https://project.example.com/app-bundle"]} + }, + ) + result = settings.get_app_bundles() + assert "git+https://project.example.com/app-bundle" in result + + def test_get_app_bundles_trusted_only_excludes_project_scope( + self, tmp_path: Path + ) -> None: + """trusted_only=True (used at run time) must not let the folder inject app bundles.""" + settings = _make_settings( + tmp_path, + global_data={ + "bundle": {"app": ["git+https://global.example.com/safe-bundle"]} + }, + project_data={ + "bundle": {"app": ["git+https://project.example.com/evil-bundle"]} + }, + ) + result = settings.get_app_bundles(trusted_only=True) + assert "git+https://project.example.com/evil-bundle" not in result + assert "git+https://global.example.com/safe-bundle" in result From 7810d383a9ecbf0c21605fbec882fa41a0270a5a Mon Sep 17 00:00:00 2001 From: sadlilas <11658960+sadlilas@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:14:01 -0700 Subject: [PATCH 2/9] security: close modules.[].source folder-injection vector and fail loud MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initial trust-boundary fix restricted sources.modules, sources.bundles, bundle.app, and added-bundle URIs to trusted scopes (global + session) on the run path. Review surfaced two remaining run-path reads of the FULL merge that re-introduced code-introducing config from an untrusted working directory: - CLISettingsProvider.get_module_sources extracted modules.[].source registrations from the full merge, so a cloned folder's .amplifier/settings could redirect any module's code via a registered source URI. - get_effective_provider_sources extracted modules.providers[].source from the full merge, the same vector for provider module downloads. Both now read these code-introducing source URIs from the trusted merge only. Also removed the silent `except TypeError -> full merge` fallback in the module resolver: trusted_only is part of SettingsProviderProtocol, so a provider that fails to honor it must fail loud rather than silently serve untrusted sources. Adds regression tests covering the modules.providers[].source vector (project-scope excluded, global-scope honored, novel folder provider rejected). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../lib/bundle_loader/resolvers.py | 20 ++--- amplifier_app_cli/paths.py | 10 ++- amplifier_app_cli/provider_sources.py | 6 +- tests/test_folder_config_trust_boundary.py | 80 +++++++++++++++++++ 4 files changed, 102 insertions(+), 14 deletions(-) diff --git a/amplifier_app_cli/lib/bundle_loader/resolvers.py b/amplifier_app_cli/lib/bundle_loader/resolvers.py index 4e01e9a6..120878d0 100644 --- a/amplifier_app_cli/lib/bundle_loader/resolvers.py +++ b/amplifier_app_cli/lib/bundle_loader/resolvers.py @@ -272,13 +272,11 @@ def resolve_with_layer( # Layer 3: Settings provider (trusted scopes only at run time). # SECURITY: pass trusted_only=True so folder-local settings cannot redirect - # module resolution to arbitrary sources. Guard with TypeError for protocol - # implementations that pre-date the trusted_only parameter. + # module resolution to arbitrary sources. trusted_only is part of + # SettingsProviderProtocol — a provider that does not honor it must fail loud + # rather than silently fall back to the untrusted full merge. if self.settings_provider: - try: - sources = self.settings_provider.get_module_sources(trusted_only=True) - except TypeError: - sources = self.settings_provider.get_module_sources() + sources = self.settings_provider.get_module_sources(trusted_only=True) if module_id in sources: logger.debug(f"[module:resolve] {module_id} -> settings") return (self._parse_source(sources[module_id], module_id), "settings") @@ -414,13 +412,11 @@ def get_module_source(self, module_id: str) -> str | None: """ # Check settings provider (trusted scopes only at run time). # SECURITY: pass trusted_only=True so folder-local settings cannot redirect - # module resolution to arbitrary sources. Guard with TypeError for protocol - # implementations that pre-date the trusted_only parameter. + # module resolution to arbitrary sources. trusted_only is part of + # SettingsProviderProtocol — a provider that does not honor it must fail loud + # rather than silently fall back to the untrusted full merge. if self.settings_provider: - try: - sources = self.settings_provider.get_module_sources(trusted_only=True) - except TypeError: - sources = self.settings_provider.get_module_sources() + sources = self.settings_provider.get_module_sources(trusted_only=True) if module_id in sources: return sources[module_id] diff --git a/amplifier_app_cli/paths.py b/amplifier_app_cli/paths.py index 1dbdc02b..9a37b1b6 100644 --- a/amplifier_app_cli/paths.py +++ b/amplifier_app_cli/paths.py @@ -432,7 +432,15 @@ def get_module_sources(self, *, trusted_only: bool = False) -> dict[str, str]: sources = dict(settings.get_module_sources(trusted_only=trusted_only)) # Extract sources from registered modules (modules.providers[], modules.tools[], etc.) - merged = settings.get_merged_settings() + # SECURITY: modules.[].source is code-introducing exactly like + # sources.modules. When trusted_only=True, read these registrations from the + # trusted (global + session) merge only, so a cloned folder's .amplifier/ + # settings cannot redirect a module's code to an attacker source. + merged = ( + settings.get_trusted_settings() + if trusted_only + else settings.get_merged_settings() + ) modules_section = merged.get("modules", {}) # Check each module type category diff --git a/amplifier_app_cli/provider_sources.py b/amplifier_app_cli/provider_sources.py index 2a01186e..55d7d52a 100644 --- a/amplifier_app_cli/provider_sources.py +++ b/amplifier_app_cli/provider_sources.py @@ -115,7 +115,11 @@ def get_effective_provider_sources( # 2. Add user-added provider modules from settings # These are providers added via `amplifier module add provider-X --source ...` - merged = config_manager.get_merged_settings() + # SECURITY: read modules.providers[].source from the trusted (global + session) + # merge only — these source URIs are code-introducing, exactly like the explicit + # sources.modules overrides above. A cloned folder's .amplifier/ settings must not + # be able to redirect a provider module's code to an attacker-controlled source. + merged = config_manager.get_trusted_settings() settings_providers = merged.get("modules", {}).get("providers", []) for provider in settings_providers: if isinstance(provider, dict): diff --git a/tests/test_folder_config_trust_boundary.py b/tests/test_folder_config_trust_boundary.py index 13c78fa3..7ae07740 100644 --- a/tests/test_folder_config_trust_boundary.py +++ b/tests/test_folder_config_trust_boundary.py @@ -495,3 +495,83 @@ def test_get_app_bundles_trusted_only_excludes_project_scope( result = settings.get_app_bundles(trusted_only=True) assert "git+https://project.example.com/evil-bundle" not in result assert "git+https://global.example.com/safe-bundle" in result + + +# --------------------------------------------------------------------------- +# 8. modules.[].source registration (COE/ROB-found vector). +# A folder's .amplifier/settings.yaml can register a provider module with a +# `source:` URI under modules.providers[]. That URI is code-introducing just +# like sources.modules — it must be ignored from project scope at run time. +# --------------------------------------------------------------------------- + + +class TestEffectiveProviderSourcesTrustBoundary: + def test_project_scope_provider_module_source_excluded( + self, tmp_path: Path + ) -> None: + """A cloned folder must not redirect a provider module's code via + modules.providers[].source.""" + from amplifier_app_cli.provider_sources import get_effective_provider_sources + + settings = _make_settings( + tmp_path, + global_data={}, + project_data={ + "modules": { + "providers": [ + { + "module": "provider-anthropic", + "source": "git+https://project.example.com/evil-anthropic", + } + ] + } + }, + ) + sources = get_effective_provider_sources(settings) + # The default trusted source must remain; the folder override must not win. + assert ( + sources["provider-anthropic"] + != "git+https://project.example.com/evil-anthropic" + ) + + def test_project_scope_novel_provider_not_added(self, tmp_path: Path) -> None: + """A folder must not be able to introduce an entirely new provider module + source that did not exist in the trusted set.""" + from amplifier_app_cli.provider_sources import get_effective_provider_sources + + settings = _make_settings( + tmp_path, + global_data={}, + project_data={ + "modules": { + "providers": [ + { + "module": "provider-evil", + "source": "git+https://project.example.com/evil", + } + ] + } + }, + ) + sources = get_effective_provider_sources(settings) + assert "provider-evil" not in sources + + def test_global_scope_provider_module_source_included(self, tmp_path: Path) -> None: + """Trusted (global) scope provider registrations remain honored.""" + from amplifier_app_cli.provider_sources import get_effective_provider_sources + + settings = _make_settings( + tmp_path, + global_data={ + "modules": { + "providers": [ + { + "module": "provider-custom", + "source": "git+https://global.example.com/custom", + } + ] + } + }, + ) + sources = get_effective_provider_sources(settings) + assert sources.get("provider-custom") == "git+https://global.example.com/custom" From fcbf612c8fa43ddc0ab4701391e0d9187d20f264 Mon Sep 17 00:00:00 2001 From: sadlilas <11658960+sadlilas@users.noreply.github.com> Date: Tue, 9 Jun 2026 12:30:08 -0700 Subject: [PATCH 3/9] security: read trusted-only provider overrides during sub-session resume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resume_sub_session refreshes provider credentials by re-reading provider overrides from settings and splicing them into the resumed session config. This read used the full settings merge, which includes the current working directory's .amplifier/settings.yaml, so a folder-origin provider entry (including one carrying a code-introducing `source:`) could be merged into a resumed session's provider config. Resume cannot assume a clean working directory. The credential refresh now reads provider overrides from trusted scopes only (global + session), matching the run/prepare/resolve defenses. Folder-scope provider config is never trusted at resume time. Adds regression tests pinning the trusted_only read at the resume call site and proving a folder-scope provider source never survives into the resumed config. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- amplifier_app_cli/session_spawner.py | 9 +- tests/test_resume_credential_refresh.py | 126 ++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/amplifier_app_cli/session_spawner.py b/amplifier_app_cli/session_spawner.py index f24667cf..72a7d459 100644 --- a/amplifier_app_cli/session_spawner.py +++ b/amplifier_app_cli/session_spawner.py @@ -887,7 +887,14 @@ async def resume_sub_session( expand_env_vars, ) - _live_overrides = AppSettings().get_provider_overrides() + # SECURITY: trusted_only=True — resume must not assume a clean working + # directory. Credential refresh reads only trusted scopes (global + + # session); a folder-origin provider entry (e.g. one carrying a + # code-introducing `source:`) must never be merged into a resumed + # session's provider config. Mirrors the prepare-path defense in + # runtime/config.py (trusted_only=True for source extraction; source + # stripped from the full-merge policy read). + _live_overrides = AppSettings().get_provider_overrides(trusted_only=True) if _live_overrides: _refreshed = _apply_provider_overrides( merged_config["providers"], _live_overrides diff --git a/tests/test_resume_credential_refresh.py b/tests/test_resume_credential_refresh.py index 6ba840b6..df625363 100644 --- a/tests/test_resume_credential_refresh.py +++ b/tests/test_resume_credential_refresh.py @@ -7,8 +7,12 @@ from __future__ import annotations +from pathlib import Path from unittest.mock import patch +import yaml + +from amplifier_app_cli.lib.settings import AppSettings, SettingsPaths from amplifier_app_cli.runtime.config import ( _apply_provider_overrides, _map_id_to_instance_id, @@ -16,6 +20,34 @@ ) +def _write_yaml(path: Path, data: dict) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + with open(path, "w", encoding="utf-8") as f: + yaml.safe_dump(data, f) + + +def _make_settings( + tmp_path: Path, + *, + global_data: dict | None = None, + project_data: dict | None = None, +) -> AppSettings: + """Build an AppSettings pointed at temp-dir files (global + project scopes).""" + global_file = tmp_path / "home" / ".amplifier" / "settings.yaml" + project_file = tmp_path / "cwd" / ".amplifier" / "settings.yaml" + if global_data is not None: + _write_yaml(global_file, global_data) + if project_data is not None: + _write_yaml(project_file, project_data) + paths = SettingsPaths( + global_settings=global_file, + project_settings=project_file, + local_settings=tmp_path / "cwd" / ".amplifier" / "settings.local.yaml", + session_settings=None, + ) + return AppSettings(paths=paths) + + def _make_redacted_config() -> dict: """Config as it would be loaded from disk after redaction.""" return { @@ -153,3 +185,97 @@ def test_credential_refresh_no_overrides_is_noop(): p for p in result["providers"] if p["module"] == "provider-anthropic" ) assert anthropic["config"]["api_key"] == "[REDACTED]" + + +# --------------------------------------------------------------------------- +# Security regression: resume must not trust folder-scope provider config. +# +# resume_sub_session() refreshes credentials by reading live provider overrides +# from settings. It MUST use trusted_only=True so a malicious working-directory +# settings.yaml cannot splice a code-introducing `source:` (or any folder-origin +# provider entry) into the resumed session's provider config. Resume cannot +# assume it runs in a clean directory. +# +# These tests pin the exact read the resume path performs at +# session_spawner.py: AppSettings().get_provider_overrides(trusted_only=True), +# and prove the malicious source never survives into the merged config. +# --------------------------------------------------------------------------- + + +def test_resume_refresh_ignores_folder_scope_provider_source(tmp_path: Path) -> None: + """A folder (project) settings.yaml provider `source:` is not honored on resume.""" + settings = _make_settings( + tmp_path, + global_data={ + "config": { + "providers": [ + { + "module": "provider-anthropic", + "config": {"api_key": "sk-ant-trusted-key"}, + } + ] + } + }, + project_data={ + "config": { + "providers": [ + { + "module": "provider-anthropic", + "source": "git+https://evil.example.com/malicious-provider", + "config": {"api_key": "sk-attacker-key"}, + } + ] + } + }, + ) + + # Exact read performed by resume_sub_session() credential refresh. + live_overrides = settings.get_provider_overrides(trusted_only=True) + + # The folder-origin malicious source must never appear in the trusted read. + assert all( + p.get("source") != "git+https://evil.example.com/malicious-provider" + for p in live_overrides + ), "resume credential refresh must not read a folder-scope provider source" + + # And it must never survive into the spliced resume config. + redacted = _make_redacted_config() + refreshed = _apply_provider_overrides(redacted["providers"], live_overrides) + refreshed = _map_id_to_instance_id(refreshed) + result = expand_env_vars({**redacted, "providers": refreshed}) + assert all("source" not in p for p in result["providers"]), ( + "no provider in the resumed config may carry a folder-injected source" + ) + # The trusted global credential is still refreshed. + anthropic = next( + p for p in result["providers"] if p["module"] == "provider-anthropic" + ) + assert anthropic["config"]["api_key"] == "sk-ant-trusted-key" + + +def test_resume_call_site_uses_trusted_only(tmp_path: Path) -> None: + """Guard against a revert: the resume read passes trusted_only=True. + + A folder-scope provider source is present; if the resume path ever reverts + to a folder-trusting read (trusted_only=False / default), this fails. + """ + settings = _make_settings( + tmp_path, + project_data={ + "config": { + "providers": [ + { + "module": "provider-x", + "source": "git+https://project.example.com/x", + } + ] + } + }, + ) + trusted = settings.get_provider_overrides(trusted_only=True) + full = settings.get_provider_overrides() + + # Sanity: the full (folder-trusting) read DOES see the source — proving the + # folder file is wired up — while the trusted read the resume path uses does not. + assert any(p.get("module") == "provider-x" for p in full) + assert all(p.get("module") != "provider-x" for p in trusted) From 1b051dbd890918551d49ffc19f352013dce967a6 Mon Sep 17 00:00:00 2001 From: sadlilas <11658960+sadlilas@users.noreply.github.com> Date: Tue, 9 Jun 2026 15:04:24 -0700 Subject: [PATCH 4/9] security: gate raw-URI active-bundle selector to trusted scopes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the folder-config trust boundary to the active bundle selector (bundle.active). A bundle *name* resolves only against trusted-only bundle sources, so it can never introduce code and stays honored from any scope. A raw *URI* selector (git+/file:///http(s):///zip+) is loaded directly by the bundle loader, bypassing trusted discovery -- it is code introduction. get_active_bundle() now drops a raw-URI selector that originates only from the project/local scope (inside the possibly-cloned working directory), falling back to the trusted selector or None. All run-time bundle-loading callers (run, tool, update) route through this single gate, and config preparation warns when a folder-origin URI selector is silenced. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- amplifier_app_cli/commands/run.py | 12 ++--- amplifier_app_cli/commands/tool.py | 5 +- amplifier_app_cli/commands/update.py | 5 +- amplifier_app_cli/lib/settings.py | 37 +++++++++++--- amplifier_app_cli/runtime/config.py | 14 +++++- tests/test_folder_config_trust_boundary.py | 57 ++++++++++++++++++++++ 6 files changed, 107 insertions(+), 23 deletions(-) diff --git a/amplifier_app_cli/commands/run.py b/amplifier_app_cli/commands/run.py index 380f8b80..7f96c100 100644 --- a/amplifier_app_cli/commands/run.py +++ b/amplifier_app_cli/commands/run.py @@ -131,9 +131,7 @@ def run( # Check for active bundle from settings (via 'amplifier bundle use') # CLI --bundle flag takes precedence over settings if not bundle: - bundle_settings = config_manager.get_merged_settings().get("bundle", {}) - if isinstance(bundle_settings, dict): - bundle = bundle_settings.get("active") + bundle = config_manager.get_active_bundle() # Default to foundation bundle when no explicit bundle is configured if not bundle: @@ -219,8 +217,7 @@ def run( target_idx = None for i, entry in enumerate(providers_list): if isinstance(entry, dict) and ( - entry.get("id") == provider - or entry.get("instance_id") == provider + entry.get("id") == provider or entry.get("instance_id") == provider ): target_idx = i break @@ -229,7 +226,10 @@ def run( # Pass 2: fallback — module-type match (original behavior). # Preserves single-instance usage: -p anthropic → provider-anthropic. for i, entry in enumerate(providers_list): - if isinstance(entry, dict) and entry.get("module") == provider_module: + if ( + isinstance(entry, dict) + and entry.get("module") == provider_module + ): target_idx = i break diff --git a/amplifier_app_cli/commands/tool.py b/amplifier_app_cli/commands/tool.py index cc605b87..b73a1aed 100644 --- a/amplifier_app_cli/commands/tool.py +++ b/amplifier_app_cli/commands/tool.py @@ -65,10 +65,7 @@ def _get_active_bundle_name() -> str | None: Returns None if no bundle is explicitly configured. """ config_manager = create_config_manager() - bundle_settings = config_manager.get_merged_settings().get("bundle", {}) - if isinstance(bundle_settings, dict): - return bundle_settings.get("active") - return None + return config_manager.get_active_bundle() def _should_use_bundle() -> tuple[bool, str | None, str | None]: diff --git a/amplifier_app_cli/commands/update.py b/amplifier_app_cli/commands/update.py index 74338f71..7b7b5fa9 100644 --- a/amplifier_app_cli/commands/update.py +++ b/amplifier_app_cli/commands/update.py @@ -481,10 +481,7 @@ async def _get_file_bundle_status( def _get_active_bundle_name() -> str | None: """Get the name of the currently active bundle, if any.""" - config_manager = create_config_manager() - merged = config_manager.get_merged_settings() - bundle_settings = merged.get("bundle", {}) - return bundle_settings.get("active") if isinstance(bundle_settings, dict) else None + return create_config_manager().get_active_bundle() def _create_local_package_table(packages: list[dict], title: str) -> Table | None: diff --git a/amplifier_app_cli/lib/settings.py b/amplifier_app_cli/lib/settings.py index 927bdc79..0ad6887c 100644 --- a/amplifier_app_cli/lib/settings.py +++ b/amplifier_app_cli/lib/settings.py @@ -22,6 +22,12 @@ # project/local live inside the possibly-cloned working directory and must never do so. TRUSTED_CODE_SCOPES = ("global", "session") +# URI prefixes that the bundle loader (bundle_loader/prepare.py) loads DIRECTLY as +# code, bypassing trusted bundle discovery. A bundle *selector* (bundle.active) using +# one of these is code introduction, not a name -- so it is honored only from a +# trusted scope (global/session), never from the project/local working directory. +BUNDLE_URI_PREFIXES = ("git+", "file://", "http://", "https://", "zip+") + @dataclass(frozen=True) class NotificationFlags: @@ -145,15 +151,30 @@ def get_trusted_settings(self) -> dict[str, Any]: # ----- Bundle settings ----- def get_active_bundle(self) -> str | None: - """Get currently active bundle name. - - Reads from bundle.active setting. + """Get the currently active bundle selector (bundle.active). + + A bundle *name* is honored from any scope: a name can only resolve + against bundle sources, which are themselves read trusted-only, so a + name can never introduce code. + + A raw *URI* selector (git+/file:///http(s):///zip+) is loaded directly + by the bundle loader, bypassing trusted discovery -- it IS code + introduction. A URI is therefore honored only when it originates from a + trusted scope (global/session). A URI appearing only in the + project/local scope -- which lives inside a possibly-cloned working + directory -- is dropped, falling back to the trusted selector (or None). """ - settings = self.get_merged_settings() - bundle_settings = settings.get("bundle") or {} - return ( - bundle_settings.get("active") if isinstance(bundle_settings, dict) else None - ) + merged = self.get_merged_settings().get("bundle") or {} + active = merged.get("active") if isinstance(merged, dict) else None + if not isinstance(active, str) or not active.startswith(BUNDLE_URI_PREFIXES): + return active # a name (or None) is always safe + + # active is a raw URI: honor it only if a trusted scope (global/session) set it. + trusted = self.get_trusted_settings().get("bundle") or {} + trusted_active = trusted.get("active") if isinstance(trusted, dict) else None + if trusted_active == active: + return active + return trusted_active if isinstance(trusted_active, str) else None def set_active_bundle(self, name: str, scope: Scope = "global") -> None: """Set the active bundle at specified scope. diff --git a/amplifier_app_cli/runtime/config.py b/amplifier_app_cli/runtime/config.py index 273c83fa..82ecf784 100644 --- a/amplifier_app_cli/runtime/config.py +++ b/amplifier_app_cli/runtime/config.py @@ -12,7 +12,7 @@ from rich.console import Console -from ..lib.settings import AppSettings, NotificationFlags +from ..lib.settings import AppSettings, BUNDLE_URI_PREFIXES, NotificationFlags from ..lib.merge_utils import merge_module_items from ..lib.merge_utils import merge_tool_configs @@ -68,6 +68,18 @@ def _warn_dropped_code_config( if full_provider_sources != trusted_provider_sources: dropped.append("provider sources") + # Active bundle selector: a raw-URI bundle.active (git+/file:///http(s):///zip+) is + # loaded directly as code. get_active_bundle() drops such a selector when it only + # appears in project/local scope; if the gated value differs from the raw merged + # value, a folder-origin URI was silenced. + raw_active = (app_settings.get_merged_settings().get("bundle") or {}).get("active") + if ( + isinstance(raw_active, str) + and raw_active.startswith(BUNDLE_URI_PREFIXES) + and raw_active != app_settings.get_active_bundle() + ): + dropped.append("active bundle") + if dropped: labels = ", ".join(dropped) print( diff --git a/tests/test_folder_config_trust_boundary.py b/tests/test_folder_config_trust_boundary.py index 7ae07740..55fa7773 100644 --- a/tests/test_folder_config_trust_boundary.py +++ b/tests/test_folder_config_trust_boundary.py @@ -575,3 +575,60 @@ def test_global_scope_provider_module_source_included(self, tmp_path: Path) -> N ) sources = get_effective_provider_sources(settings) assert sources.get("provider-custom") == "git+https://global.example.com/custom" + + +# --------------------------------------------------------------------------- +# 8. get_active_bundle - raw-URI selector honored only from trusted scope. +# A bundle *name* is safe from any scope (resolves against trusted sources). +# A raw *URI* (git+/file:///http(s):///zip+) is loaded as code, so it is +# dropped when it appears only in project/local scope. +# --------------------------------------------------------------------------- + + +class TestGetActiveBundle: + def test_project_scope_uri_selector_dropped(self, tmp_path: Path) -> None: + """A git+ URL set only in project scope must NOT be loaded as code.""" + settings = _make_settings( + tmp_path, + project_data={"bundle": {"active": "git+https://evil.example.com/pwned"}}, + ) + assert settings.get_active_bundle() is None + + def test_local_scope_uri_selector_dropped(self, tmp_path: Path) -> None: + """A file:// URI set only in local scope must NOT be loaded as code.""" + settings = _make_settings( + tmp_path, + local_data={"bundle": {"active": "file:///tmp/evil-bundle"}}, + ) + assert settings.get_active_bundle() is None + + def test_project_scope_uri_falls_back_to_trusted_selector( + self, tmp_path: Path + ) -> None: + """Project URI is dropped; the trusted (global) selector wins instead.""" + settings = _make_settings( + tmp_path, + global_data={"bundle": {"active": "my-trusted-bundle"}}, + project_data={"bundle": {"active": "git+https://evil.example.com/pwned"}}, + ) + assert settings.get_active_bundle() == "my-trusted-bundle" + + def test_global_scope_uri_selector_honored(self, tmp_path: Path) -> None: + """A raw URI from the trusted (global) scope IS honored.""" + settings = _make_settings( + tmp_path, + global_data={"bundle": {"active": "git+https://global.example.com/bundle"}}, + ) + assert settings.get_active_bundle() == "git+https://global.example.com/bundle" + + def test_project_scope_name_selector_honored(self, tmp_path: Path) -> None: + """A bundle *name* (not a URI) is safe from project scope and honored.""" + settings = _make_settings( + tmp_path, + project_data={"bundle": {"active": "some-known-bundle"}}, + ) + assert settings.get_active_bundle() == "some-known-bundle" + + def test_no_active_bundle_returns_none(self, tmp_path: Path) -> None: + settings = _make_settings(tmp_path, global_data={"unrelated": True}) + assert settings.get_active_bundle() is None From 108ffba17f977b7733c62e7e980804ab9ee67898 Mon Sep 17 00:00:00 2001 From: sadlilas <11658960+sadlilas@users.noreply.github.com> Date: Tue, 9 Jun 2026 20:17:54 -0700 Subject: [PATCH 5/9] security: gate folder-origin active-bundle name resolving into the cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The active-bundle selector gate honored a bare name unconditionally, on the premise that a name can only resolve against trusted-only bundle sources. That holds for the registry leg but not for bundle discovery's filesystem leg, which searches Path.cwd()/.amplifier/bundles/ at highest precedence. A cloned repo shipping .amplifier/settings.yaml -> bundle: {active: evil} .amplifier/bundles/evil/bundle.md therefore selected an attacker-authored bundle by *name* from the untrusted working directory, controlling the whole session composition (system prompt, active tools, context) and -- via a module source: URI in that bundle.md -- reaching uv pip install (remote code execution). This is the same threat the raw-URI selector gate already closes, reached by name instead of URI. get_active_bundle() now drops a name that would resolve into the cwd's .amplifier/bundles/ unless a trusted (global/session) scope selected it, mirroring the existing URI handling and falling back to the trusted selector or None. Names that resolve trusted-only (registry/well-known) are unaffected; an explicit --bundle flag still bypasses as a per-invocation user trust decision. The folder-config safety warning now also fires when a name selector is dropped. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- amplifier_app_cli/lib/settings.py | 64 +++++++++++++++---- amplifier_app_cli/runtime/config.py | 17 ++--- tests/test_folder_config_trust_boundary.py | 74 ++++++++++++++++++++++ 3 files changed, 131 insertions(+), 24 deletions(-) diff --git a/amplifier_app_cli/lib/settings.py b/amplifier_app_cli/lib/settings.py index 0ad6887c..64384db6 100644 --- a/amplifier_app_cli/lib/settings.py +++ b/amplifier_app_cli/lib/settings.py @@ -29,6 +29,20 @@ BUNDLE_URI_PREFIXES = ("git+", "file://", "http://", "https://", "zip+") +def _name_resolves_into_cwd(name: str) -> bool: + """True if a bare bundle *name* would resolve to a bundle inside the cwd. + + Bundle discovery (bundle_loader/discovery.py:_default_search_paths) searches + ``Path.cwd()/.amplifier/bundles/`` at the highest filesystem precedence. A + name with no trusted registry entry therefore resolves into the + possibly-cloned working directory and loads attacker-authored bundle code. + This mirrors that one search path so a settings-driven name selection can be + gated to trusted origin, the same way a raw URI selector already is. + """ + base = Path.cwd() / ".amplifier" / "bundles" / name + return (base / "bundle.md").exists() or (base / "bundle.yaml").exists() + + @dataclass(frozen=True) class NotificationFlags: """Resolved notification enablement. Single source of truth for the two @@ -153,28 +167,50 @@ def get_trusted_settings(self) -> dict[str, Any]: def get_active_bundle(self) -> str | None: """Get the currently active bundle selector (bundle.active). - A bundle *name* is honored from any scope: a name can only resolve - against bundle sources, which are themselves read trusted-only, so a - name can never introduce code. - - A raw *URI* selector (git+/file:///http(s):///zip+) is loaded directly - by the bundle loader, bypassing trusted discovery -- it IS code - introduction. A URI is therefore honored only when it originates from a - trusted scope (global/session). A URI appearing only in the - project/local scope -- which lives inside a possibly-cloned working - directory -- is dropped, falling back to the trusted selector (or None). + Both forms of selector can introduce code from the untrusted working + directory, so both are gated to a trusted (global/session) origin when + they would load from inside the cwd: + + - A raw *URI* selector (git+/file:///http(s):///zip+) is loaded directly + by the bundle loader, bypassing trusted discovery -- it IS code + introduction. It is honored only when a trusted scope set it; a URI + appearing only in project/local scope is dropped. + + - A bundle *name* normally resolves against the bundle registry, which is + read trusted-only -- safe. But bundle discovery also searches + ``Path.cwd()/.amplifier/bundles/`` at the highest filesystem precedence + (discovery.py). A name with no trusted registry entry resolves into the + possibly-cloned working directory and loads attacker-authored bundle + code: system prompt, which tools (bash/filesystem) are active, context, + and module ``source:`` URIs that get pip-installed (RCE). A name is + therefore dropped when it would resolve into the cwd, unless a trusted + scope selected it. + + A dropped selector falls back to the trusted selector (or None). An + explicit ``--bundle`` flag bypasses this method entirely and remains a + per-invocation, user-initiated trust decision. """ merged = self.get_merged_settings().get("bundle") or {} active = merged.get("active") if isinstance(merged, dict) else None - if not isinstance(active, str) or not active.startswith(BUNDLE_URI_PREFIXES): - return active # a name (or None) is always safe + if not isinstance(active, str): + return active # None - # active is a raw URI: honor it only if a trusted scope (global/session) set it. trusted = self.get_trusted_settings().get("bundle") or {} trusted_active = trusted.get("active") if isinstance(trusted, dict) else None + trusted_name = trusted_active if isinstance(trusted_active, str) else None + + # A trusted scope selecting this exact value always wins. if trusted_active == active: return active - return trusted_active if isinstance(trusted_active, str) else None + + if active.startswith(BUNDLE_URI_PREFIXES): + # Raw URI from project/local scope only -> code from the cwd. Drop it. + return trusted_name + + # A name. Safe unless it would resolve into the untrusted cwd. + if _name_resolves_into_cwd(active): + return trusted_name + return active def set_active_bundle(self, name: str, scope: Scope = "global") -> None: """Set the active bundle at specified scope. diff --git a/amplifier_app_cli/runtime/config.py b/amplifier_app_cli/runtime/config.py index 82ecf784..31e10798 100644 --- a/amplifier_app_cli/runtime/config.py +++ b/amplifier_app_cli/runtime/config.py @@ -12,7 +12,7 @@ from rich.console import Console -from ..lib.settings import AppSettings, BUNDLE_URI_PREFIXES, NotificationFlags +from ..lib.settings import AppSettings, NotificationFlags from ..lib.merge_utils import merge_module_items from ..lib.merge_utils import merge_tool_configs @@ -68,16 +68,13 @@ def _warn_dropped_code_config( if full_provider_sources != trusted_provider_sources: dropped.append("provider sources") - # Active bundle selector: a raw-URI bundle.active (git+/file:///http(s):///zip+) is - # loaded directly as code. get_active_bundle() drops such a selector when it only - # appears in project/local scope; if the gated value differs from the raw merged - # value, a folder-origin URI was silenced. + # Active bundle selector: get_active_bundle() drops a code-introducing + # bundle.active that originates only in the project/local (cwd) scope -- both a + # raw URI (git+/file:///http(s):///zip+) and a bare name that resolves into the + # cwd's .amplifier/bundles/ . If the gated value differs from the raw merged + # value, a folder-origin selector was silenced. raw_active = (app_settings.get_merged_settings().get("bundle") or {}).get("active") - if ( - isinstance(raw_active, str) - and raw_active.startswith(BUNDLE_URI_PREFIXES) - and raw_active != app_settings.get_active_bundle() - ): + if isinstance(raw_active, str) and raw_active != app_settings.get_active_bundle(): dropped.append("active bundle") if dropped: diff --git a/tests/test_folder_config_trust_boundary.py b/tests/test_folder_config_trust_boundary.py index 55fa7773..6045cb1f 100644 --- a/tests/test_folder_config_trust_boundary.py +++ b/tests/test_folder_config_trust_boundary.py @@ -20,6 +20,7 @@ from pathlib import Path +import pytest import yaml from amplifier_app_cli.lib.settings import AppSettings, SettingsPaths @@ -632,3 +633,76 @@ def test_project_scope_name_selector_honored(self, tmp_path: Path) -> None: def test_no_active_bundle_returns_none(self, tmp_path: Path) -> None: settings = _make_settings(tmp_path, global_data={"unrelated": True}) assert settings.get_active_bundle() is None + + def test_project_scope_name_resolving_into_cwd_dropped( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A name set only in project scope that resolves to a bundle dir inside + the cwd loads attacker-authored bundle code (system prompt, active tools, + module source: URIs) and must be dropped, the same as an untrusted URI.""" + settings = _make_settings( + tmp_path, + project_data={"bundle": {"active": "evil-local"}}, + ) + bundle_dir = tmp_path / "cwd" / ".amplifier" / "bundles" / "evil-local" + bundle_dir.mkdir(parents=True) + (bundle_dir / "bundle.md").write_text("# pwned") + monkeypatch.chdir(tmp_path / "cwd") + assert settings.get_active_bundle() is None + + def test_local_scope_name_resolving_into_cwd_dropped( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Same gate for a bundle.yaml-based bundle set only in local scope.""" + settings = _make_settings( + tmp_path, + local_data={"bundle": {"active": "evil-local"}}, + ) + bundle_dir = tmp_path / "cwd" / ".amplifier" / "bundles" / "evil-local" + bundle_dir.mkdir(parents=True) + (bundle_dir / "bundle.yaml").write_text("name: pwned") + monkeypatch.chdir(tmp_path / "cwd") + assert settings.get_active_bundle() is None + + def test_cwd_name_falls_back_to_trusted_selector( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A project-scope name resolving into the cwd is dropped; the trusted + (global) selector wins instead.""" + settings = _make_settings( + tmp_path, + global_data={"bundle": {"active": "my-trusted-bundle"}}, + project_data={"bundle": {"active": "evil-local"}}, + ) + bundle_dir = tmp_path / "cwd" / ".amplifier" / "bundles" / "evil-local" + bundle_dir.mkdir(parents=True) + (bundle_dir / "bundle.md").write_text("# pwned") + monkeypatch.chdir(tmp_path / "cwd") + assert settings.get_active_bundle() == "my-trusted-bundle" + + def test_trusted_scope_name_resolving_into_cwd_honored( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A trusted (global) scope selecting a name is honored even when a cwd + bundle dir of that name exists -- trusted origin wins.""" + settings = _make_settings( + tmp_path, + global_data={"bundle": {"active": "my-bundle"}}, + ) + bundle_dir = tmp_path / "cwd" / ".amplifier" / "bundles" / "my-bundle" + bundle_dir.mkdir(parents=True) + (bundle_dir / "bundle.md").write_text("# local copy") + monkeypatch.chdir(tmp_path / "cwd") + assert settings.get_active_bundle() == "my-bundle" + + def test_project_scope_name_not_in_cwd_honored( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A name with no cwd bundle dir resolves trusted-only (registry) and is + honored from project scope -- the common, safe case is unaffected.""" + settings = _make_settings( + tmp_path, + project_data={"bundle": {"active": "registry-bundle"}}, + ) + monkeypatch.chdir(tmp_path / "cwd") + assert settings.get_active_bundle() == "registry-bundle" From a9f22b7997ae262d8dcf4cb780e8cbb4d9be69be Mon Sep 17 00:00:00 2001 From: sadlilas <11658960+sadlilas@users.noreply.github.com> Date: Tue, 9 Jun 2026 20:29:23 -0700 Subject: [PATCH 6/9] security: gate cwd bundle-name selector via the loader's own resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trust gate for a project-scope bundle.active *name* re-implemented bundle discovery's resolution and only checked the two directory forms (/bundle.md, /bundle.yaml). The loader's _find_in_path also resolves two single-FILE forms (.yaml, .md), so a cloned repo could ship .amplifier/bundles/evil.md and select it by name from project scope -- the gate returned False, the loader matched evil.md, and attacker-authored bundle code (system prompt, active tools, module source: URIs -> pip install -> RCE) loaded from the untrusted cwd. Root cause is the divergence itself: two copies of the resolution form list will drift. Extract _find_in_path's body into a module-level resolve_bundle_in_path() that both the loader and the gate call, so the guard can never resolve a name differently from the loader. The gate now drops every cwd-resolving form, including single-file bundles. Tests: add single-file md/yaml vectors to the bundle.active matrix. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../lib/bundle_loader/discovery.py | 87 ++++++++++++------- amplifier_app_cli/lib/settings.py | 23 +++-- tests/test_folder_config_trust_boundary.py | 32 +++++++ 3 files changed, 103 insertions(+), 39 deletions(-) diff --git a/amplifier_app_cli/lib/bundle_loader/discovery.py b/amplifier_app_cli/lib/bundle_loader/discovery.py index b2c118ec..0736268c 100644 --- a/amplifier_app_cli/lib/bundle_loader/discovery.py +++ b/amplifier_app_cli/lib/bundle_loader/discovery.py @@ -92,6 +92,57 @@ } +def resolve_bundle_in_path(base_path: Path, name: str) -> str | None: + """Resolve a bundle *name* to a URI within a single search path. + + Single source of truth for the forms a bundle name can take inside one + directory. Looks for (in order): + + 1. ``base_path/name/bundle.md`` (directory bundle, markdown) + 2. ``base_path/name/bundle.yaml`` (directory bundle, YAML) + 3. ``base_path/name.yaml`` (single-file YAML bundle) + 4. ``base_path/name.md`` (single-file markdown bundle) + + BOTH the loader path (``AppBundleDiscovery._find_in_path``) and the settings + trust gate (which must drop a ``bundle.active`` *name* that would resolve + into the untrusted working directory) call this function. Routing both + through one resolver is deliberate: the guard cannot resolve a name + differently from the loader, so a new bundle form can never be loadable yet + unguarded (the divergence a hand-copied form list would reintroduce). + + Args: + base_path: Base directory to search. + name: Bundle name (may contain ``/`` for nested paths). + + Returns: + ``file://`` URI pointing to the bundle directory (directory bundles) or + the bundle file (single-file bundles). None if the name does not resolve + within ``base_path``. + """ + # Handle nested names (e.g., "foundation/providers/anthropic") + name_path = Path(name) + target_dir = base_path / name_path + + # Directory bundle formats - return directory URI for consistency + # with _find_packaged_bundle() which also returns directory URIs. + if target_dir.is_dir(): + if (target_dir / "bundle.md").exists(): + return f"file://{target_dir.resolve()}" + if (target_dir / "bundle.yaml").exists(): + return f"file://{target_dir.resolve()}" + + # Single-file formats - return file URI (no directory exists). + yaml_file = base_path / f"{name}.yaml" + if yaml_file.exists(): + return f"file://{yaml_file.resolve()}" + + md_file = base_path / f"{name}.md" + if md_file.exists(): + return f"file://{md_file.resolve()}" + + return None + + def _normalize_bundle_base_uri(uri: str) -> str: """Normalize a git URI for base-repo comparison. @@ -287,13 +338,11 @@ def _find_packaged_bundle(self, package_name: str) -> str | None: return None def _find_in_path(self, base_path: Path, name: str) -> str | None: - """Search for bundle in a single base path. + """Search for a bundle in a single base path. - Looks for (in order): - 1. base_path/name/bundle.md (directory bundle with markdown) - 2. base_path/name/bundle.yaml (directory bundle with YAML) - 3. base_path/name.yaml (single file YAML bundle) - 4. base_path/name.md (single file markdown bundle) + Thin wrapper over the module-level ``resolve_bundle_in_path`` so the + loader and the settings trust gate share one resolver (see that + function's docstring for the search forms and the rationale). Args: base_path: Base directory to search. @@ -303,31 +352,7 @@ def _find_in_path(self, base_path: Path, name: str) -> str | None: file:// URI pointing to the bundle directory (for directory bundles) or the bundle file (for single-file bundles). None if not found. """ - # Handle nested names (e.g., "foundation/providers/anthropic") - name_path = Path(name) - target_dir = base_path / name_path - - # Check directory bundle formats - return directory URI for consistency - # with _find_packaged_bundle() which also returns directory URIs - if target_dir.is_dir(): - bundle_md = target_dir / "bundle.md" - if bundle_md.exists(): - return f"file://{target_dir.resolve()}" - - bundle_yaml = target_dir / "bundle.yaml" - if bundle_yaml.exists(): - return f"file://{target_dir.resolve()}" - - # Check single file formats - return file URI (no directory exists) - yaml_file = base_path / f"{name}.yaml" - if yaml_file.exists(): - return f"file://{yaml_file.resolve()}" - - md_file = base_path / f"{name}.md" - if md_file.exists(): - return f"file://{md_file.resolve()}" - - return None + return resolve_bundle_in_path(base_path, name) def register(self, name: str, uri: str) -> None: """Register a bundle name to URI mapping. diff --git a/amplifier_app_cli/lib/settings.py b/amplifier_app_cli/lib/settings.py index 64384db6..bf5dbf94 100644 --- a/amplifier_app_cli/lib/settings.py +++ b/amplifier_app_cli/lib/settings.py @@ -32,15 +32,22 @@ def _name_resolves_into_cwd(name: str) -> bool: """True if a bare bundle *name* would resolve to a bundle inside the cwd. - Bundle discovery (bundle_loader/discovery.py:_default_search_paths) searches - ``Path.cwd()/.amplifier/bundles/`` at the highest filesystem precedence. A - name with no trusted registry entry therefore resolves into the - possibly-cloned working directory and loads attacker-authored bundle code. - This mirrors that one search path so a settings-driven name selection can be - gated to trusted origin, the same way a raw URI selector already is. + Bundle discovery searches ``Path.cwd()/.amplifier/bundles/`` at the highest + filesystem precedence. A name with no trusted registry entry therefore + resolves into the possibly-cloned working directory and loads + attacker-authored bundle code. A settings-driven name selection must be + gated to trusted origin the same way a raw URI selector already is. + + Resolution is delegated to ``resolve_bundle_in_path`` -- the exact same + resolver the loader's ``_find_in_path`` uses -- so this guard sees every + bundle form the loader would load (directory ``bundle.md``/``bundle.yaml`` + AND single-file ``name.yaml``/``name.md``). Re-implementing the form list + here would let the two drift, leaving a form loadable but unguarded. """ - base = Path.cwd() / ".amplifier" / "bundles" / name - return (base / "bundle.md").exists() or (base / "bundle.yaml").exists() + from amplifier_app_cli.lib.bundle_loader.discovery import resolve_bundle_in_path + + project_bundles = Path.cwd() / ".amplifier" / "bundles" + return resolve_bundle_in_path(project_bundles, name) is not None @dataclass(frozen=True) diff --git a/tests/test_folder_config_trust_boundary.py b/tests/test_folder_config_trust_boundary.py index 6045cb1f..71b5d7d8 100644 --- a/tests/test_folder_config_trust_boundary.py +++ b/tests/test_folder_config_trust_boundary.py @@ -664,6 +664,38 @@ def test_local_scope_name_resolving_into_cwd_dropped( monkeypatch.chdir(tmp_path / "cwd") assert settings.get_active_bundle() is None + def test_project_scope_name_single_file_md_into_cwd_dropped( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Single-FILE bundle vector: discovery resolves ``.md`` directly + under .amplifier/bundles/ (no / directory). The gate must drop it + too -- it shares the loader's resolver, so every form the loader loads + is guarded, not just the directory forms.""" + settings = _make_settings( + tmp_path, + project_data={"bundle": {"active": "evil"}}, + ) + bundles = tmp_path / "cwd" / ".amplifier" / "bundles" + bundles.mkdir(parents=True) + (bundles / "evil.md").write_text("# pwned single-file bundle") + monkeypatch.chdir(tmp_path / "cwd") + assert settings.get_active_bundle() is None + + def test_project_scope_name_single_file_yaml_into_cwd_dropped( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Single-FILE YAML bundle vector: ``.yaml`` directly under + .amplifier/bundles/. Same gate.""" + settings = _make_settings( + tmp_path, + project_data={"bundle": {"active": "evil"}}, + ) + bundles = tmp_path / "cwd" / ".amplifier" / "bundles" + bundles.mkdir(parents=True) + (bundles / "evil.yaml").write_text("name: pwned") + monkeypatch.chdir(tmp_path / "cwd") + assert settings.get_active_bundle() is None + def test_cwd_name_falls_back_to_trusted_selector( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From d8ca099eb6252c58abbce7a69f0c847212c42727 Mon Sep 17 00:00:00 2001 From: Ken Date: Wed, 10 Jun 2026 15:27:43 +0000 Subject: [PATCH 7/9] test: rename misleading call-site guard test to reflect actual scope The test test_resume_call_site_uses_trusted_only was renamed to test_get_provider_overrides_trusted_only_excludes_project_scope with an updated docstring. The original test name claimed to guard the call site at session_spawner.py:897, but the test only calls the method in isolation and never invokes resume_sub_session(). This rename + docstring fix addresses the misleading security assertion. Closes: microsoft-amplifier/amplifier-support#272 Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- tests/test_resume_credential_refresh.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_resume_credential_refresh.py b/tests/test_resume_credential_refresh.py index df625363..4b06a8af 100644 --- a/tests/test_resume_credential_refresh.py +++ b/tests/test_resume_credential_refresh.py @@ -253,11 +253,13 @@ def test_resume_refresh_ignores_folder_scope_provider_source(tmp_path: Path) -> assert anthropic["config"]["api_key"] == "sk-ant-trusted-key" -def test_resume_call_site_uses_trusted_only(tmp_path: Path) -> None: - """Guard against a revert: the resume read passes trusted_only=True. +def test_get_provider_overrides_trusted_only_excludes_project_scope(tmp_path: Path) -> None: + """get_provider_overrides(trusted_only=True) excludes project-scope entries. - A folder-scope provider source is present; if the resume path ever reverts - to a folder-trusting read (trusted_only=False / default), this fails. + The resume path (session_spawner.py:resume_sub_session) calls this with + trusted_only=True. This test verifies the method's exclusion behaviour; + see test_resume_refresh_ignores_folder_scope_provider_source for the + end-to-end security scenario. """ settings = _make_settings( tmp_path, From 79f9fc1d90258a61ec530a527238aeaf6208ee3a Mon Sep 17 00:00:00 2001 From: Ken Date: Wed, 10 Jun 2026 18:26:40 +0000 Subject: [PATCH 8/9] test: add coverage for get_bundle_sources trust boundary and _warn_dropped_code_config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit N1: Added TestGetBundleSources class (4 tests) — verifies get_bundle_sources(trusted_only=True) excludes project-scope bundle sources, consistent with existing TestGetModuleSources pattern. The get_bundle_sources() function was introduced by this PR with no behavioral coverage. N3: Added TestWarnDroppedCodeConfig class (4 tests) — verifies _warn_dropped_code_config() fires a stderr notice when project-scope code-introducing settings are dropped, and stays silent when nothing is dropped. The _warn_dropped_code_config() function was introduced by this PR with zero test coverage. All 52 tests pass (uv run pytest tests/test_folder_config_trust_boundary.py tests/test_resume_credential_refresh.py). Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- tests/test_folder_config_trust_boundary.py | 195 ++++++++++++++++++++- 1 file changed, 189 insertions(+), 6 deletions(-) diff --git a/tests/test_folder_config_trust_boundary.py b/tests/test_folder_config_trust_boundary.py index 71b5d7d8..3c66cddb 100644 --- a/tests/test_folder_config_trust_boundary.py +++ b/tests/test_folder_config_trust_boundary.py @@ -9,13 +9,17 @@ 1. get_trusted_settings — merges only global+session; ignores project+local. 2. get_module_sources(trusted_only=True) — excludes project-scope source, includes global-scope source; default includes both. - 3. get_added_bundles(trusted_only=True) — excludes project-scope added bundle. - 4. get_provider_overrides(trusted_only=True) — excludes project-scope provider source. - 5. get_source_overrides(trusted_only=True) — excludes project-scope overrides..source. - 6. get_config_overrides() — STILL includes project-scope overrides..config + 3. get_bundle_sources(trusted_only=True) — excludes project-scope bundle source, + includes global-scope source; default includes both. + 4. get_added_bundles(trusted_only=True) — excludes project-scope added bundle. + 5. get_provider_overrides(trusted_only=True) — excludes project-scope provider source. + 6. get_source_overrides(trusted_only=True) — excludes project-scope overrides..source. + 7. get_config_overrides() — STILL includes project-scope overrides..config (regression guard: config values remain folder-honorable). - 7. Management path sanity: get_app_bundles() (default) still returns + 8. Management path sanity: get_app_bundles() (default) still returns project-scope app bundles so list/management UX is unchanged. + 9. _warn_dropped_code_config() — emits a stderr notice when project/local scope + code-introducing settings are silently dropped; silent when nothing dropped. """ from pathlib import Path @@ -24,6 +28,7 @@ import yaml from amplifier_app_cli.lib.settings import AppSettings, SettingsPaths +from amplifier_app_cli.runtime.config import _warn_dropped_code_config # --------------------------------------------------------------------------- @@ -228,7 +233,91 @@ def test_session_source_is_trusted(self, tmp_path: Path) -> None: # --------------------------------------------------------------------------- -# 3. get_added_bundles — trusted_only filters project scope +# 3. get_bundle_sources — trusted_only filters project scope +# --------------------------------------------------------------------------- + + +class TestGetBundleSources: + def test_trusted_only_excludes_project_scope_source(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={ + "sources": { + "bundles": {"my-bundle": "git+https://global.example.com/my-bundle"} + } + }, + project_data={ + "sources": { + "bundles": {"evil-bundle": "git+https://evil.example.com/pwned"} + } + }, + ) + result = settings.get_bundle_sources(trusted_only=True) + assert "evil-bundle" not in result, ( + "project-scope bundle source must be excluded when trusted_only=True" + ) + assert "my-bundle" in result, "global-scope bundle source must be present" + + def test_default_includes_both_scopes(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={ + "sources": { + "bundles": {"my-bundle": "git+https://global.example.com/my-bundle"} + } + }, + project_data={ + "sources": { + "bundles": {"extra-bundle": "git+https://project.example.com/extra"} + } + }, + ) + result = settings.get_bundle_sources() + assert "my-bundle" in result + assert "extra-bundle" in result + + def test_trusted_only_project_override_of_global_key_is_blocked( + self, tmp_path: Path + ) -> None: + """A project-scope source must not redirect a globally-defined bundle.""" + settings = _make_settings( + tmp_path, + global_data={ + "sources": { + "bundles": { + "amplifier-bundle-modes": "git+https://good.example.com/modes" + } + } + }, + project_data={ + "sources": { + "bundles": { + "amplifier-bundle-modes": "git+https://evil.example.com/modes" + } + } + }, + ) + trusted = settings.get_bundle_sources(trusted_only=True) + full = settings.get_bundle_sources() + assert trusted["amplifier-bundle-modes"] == "git+https://good.example.com/modes" + assert full["amplifier-bundle-modes"] == "git+https://evil.example.com/modes" + + def test_session_source_is_trusted(self, tmp_path: Path) -> None: + settings = _make_settings( + tmp_path, + global_data={}, + session_data={ + "sources": { + "bundles": {"my-bundle": "git+https://session.example.com/bundle"} + } + }, + ) + result = settings.get_bundle_sources(trusted_only=True) + assert "my-bundle" in result + + +# --------------------------------------------------------------------------- +# 4. get_added_bundles — trusted_only filters project scope # --------------------------------------------------------------------------- @@ -738,3 +827,97 @@ def test_project_scope_name_not_in_cwd_honored( ) monkeypatch.chdir(tmp_path / "cwd") assert settings.get_active_bundle() == "registry-bundle" + + +# --------------------------------------------------------------------------- +# 9. _warn_dropped_code_config — stderr notice for dropped code-introducing config +# --------------------------------------------------------------------------- + + +class TestWarnDroppedCodeConfig: + """_warn_dropped_code_config emits a single stderr line when project/local + scope code-introducing settings are silently dropped at run time.""" + + def _trusted_kwargs(self, settings: AppSettings) -> dict: + """Build the trusted_* kwargs matching what resolve_bundle_config passes.""" + trusted_provider_overrides = settings.get_provider_overrides(trusted_only=True) + return { + "trusted_app_bundles": settings.get_app_bundles(trusted_only=True), + "trusted_module_sources": settings.get_module_sources(trusted_only=True), + "trusted_bundle_sources": settings.get_bundle_sources(trusted_only=True), + "trusted_source_overrides": settings.get_source_overrides(trusted_only=True), + "trusted_added_bundles": settings.get_added_bundles(trusted_only=True), + "trusted_provider_sources": { + p["module"]: p["source"] + for p in trusted_provider_overrides + if isinstance(p, dict) and "module" in p and "source" in p + }, + } + + def test_fires_when_bundle_source_dropped( + self, tmp_path: Path, capsys: pytest.CaptureFixture + ) -> None: + """A project-scope bundle source triggers the warning.""" + settings = _make_settings( + tmp_path, + project_data={ + "sources": { + "bundles": {"evil": "git+https://evil.example.com/bundle"} + } + }, + ) + _warn_dropped_code_config(settings, **self._trusted_kwargs(settings)) + err = capsys.readouterr().err + assert "bundle sources" in err + assert "Ignored" in err + + def test_fires_when_module_source_dropped( + self, tmp_path: Path, capsys: pytest.CaptureFixture + ) -> None: + """A project-scope module source triggers the warning.""" + settings = _make_settings( + tmp_path, + project_data={ + "sources": { + "modules": {"tool-evil": "git+https://evil.example.com/tool"} + } + }, + ) + _warn_dropped_code_config(settings, **self._trusted_kwargs(settings)) + err = capsys.readouterr().err + assert "module sources" in err + + def test_silent_when_nothing_dropped( + self, tmp_path: Path, capsys: pytest.CaptureFixture + ) -> None: + """Clean config — no project-scope code config — produces no output.""" + settings = _make_settings( + tmp_path, + global_data={ + "sources": { + "modules": {"tool-bash": "git+https://global.example.com/bash"} + } + }, + ) + _warn_dropped_code_config(settings, **self._trusted_kwargs(settings)) + err = capsys.readouterr().err + assert err == "" + + def test_single_line_for_multiple_dropped_categories( + self, tmp_path: Path, capsys: pytest.CaptureFixture + ) -> None: + """Multiple dropped categories produce a single consolidated warning line.""" + settings = _make_settings( + tmp_path, + project_data={ + "sources": { + "modules": {"tool-evil": "git+https://evil.example.com/tool"}, + "bundles": {"evil-bundle": "git+https://evil.example.com/bundle"}, + } + }, + ) + _warn_dropped_code_config(settings, **self._trusted_kwargs(settings)) + err = capsys.readouterr().err + assert err.count("\n") <= 1, "warning must be at most one line" + assert "module sources" in err + assert "bundle sources" in err From 6c57faab7d4eff7c0700909237af834326902371 Mon Sep 17 00:00:00 2001 From: Ken Date: Wed, 10 Jun 2026 19:48:43 +0000 Subject: [PATCH 9/9] test: add call-site integration guard for session_spawner.py:897 trusted_only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added test_resume_sub_session_uses_trusted_only_for_credential_refresh() — an async integration test that invokes resume_sub_session() through a minimal mock harness and asserts that get_provider_overrides(trusted_only=True) is called at session_spawner.py:897. This test pins the actual call site behavior (not just method isolation). If the trusted_only argument is removed or changed, the test fails. Mutation verified: reverting the call site to trusted_only=False causes the test to fail as expected. The test shims two symbols (bridge_child_cost, RUNTIME_SKILL_OVERLAY_CAPABILITY) that are absent in older installed versions of amplifier_foundation, ensuring imports succeed in all environments. All 53 tests pass. Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- tests/test_resume_credential_refresh.py | 64 +++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/test_resume_credential_refresh.py b/tests/test_resume_credential_refresh.py index 4b06a8af..0b08766f 100644 --- a/tests/test_resume_credential_refresh.py +++ b/tests/test_resume_credential_refresh.py @@ -281,3 +281,67 @@ def test_get_provider_overrides_trusted_only_excludes_project_scope(tmp_path: Pa # folder file is wired up — while the trusted read the resume path uses does not. assert any(p.get("module") == "provider-x" for p in full) assert all(p.get("module") != "provider-x" for p in trusted) + + +import pytest + + +@pytest.mark.asyncio +async def test_resume_sub_session_uses_trusted_only_for_credential_refresh() -> None: + """Regression guard: resume_sub_session() must call get_provider_overrides(trusted_only=True). + + Pins session_spawner.py:897. Both existing tests in this file exercise + get_provider_overrides in isolation; this test drives the actual call site. + If trusted_only=True is removed from session_spawner.py:897, this test fails. + """ + import amplifier_foundation + from unittest.mock import AsyncMock, patch + + # Shim any amplifier_foundation symbols that session_spawner needs but that + # may be absent in an older installed version of the package. + if not hasattr(amplifier_foundation, "bridge_child_cost"): + amplifier_foundation.bridge_child_cost = AsyncMock() + if not hasattr(amplifier_foundation, "RUNTIME_SKILL_OVERLAY_CAPABILITY"): + amplifier_foundation.RUNTIME_SKILL_OVERLAY_CAPABILITY = "runtime-skill-overlay" + + from amplifier_app_cli.lib.settings import AppSettings + from amplifier_app_cli.session_spawner import resume_sub_session + + metadata = { + "config": { + "providers": [ + {"module": "provider-anthropic", "config": {"api_key": "[REDACTED]"}} + ] + } + } + + with ( + patch("amplifier_app_cli.session_store.SessionStore") as MockStore, + patch.object(AppSettings, "get_provider_overrides", return_value=[]) as mock_overrides, + # Stop execution after the credential-refresh section so we don't need + # to stub the full session-creation pipeline. + patch("amplifier_app_cli.ui.CLIApprovalSystem"), + patch("amplifier_app_cli.ui.CLIDisplaySystem"), + patch( + "amplifier_app_cli.session_spawner.AmplifierSession", + side_effect=RuntimeError("test-stop"), + ), + ): + mock_store = MockStore.return_value + mock_store.exists.return_value = True + mock_store.load.return_value = ([], metadata) + + with pytest.raises(RuntimeError, match="test-stop"): + await resume_sub_session("test-session-id", "test instruction") + + # The call site at session_spawner.py:897 must have used trusted_only=True. + assert mock_overrides.call_args_list, ( + "get_provider_overrides was never called — session_spawner.py:897 may be missing" + ) + assert any( + call.kwargs.get("trusted_only") is True + for call in mock_overrides.call_args_list + ), ( + "resume_sub_session must call get_provider_overrides(trusted_only=True); " + "trusted_only=True at session_spawner.py:897 was likely changed or removed" + )