diff --git a/CHANGELOG.md b/CHANGELOG.md index 87a23dd6..7c79c800 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +## [v0.51.590] — 2026-06-22 — Release UW (close cross-profile credential leak on /api/providers + /api/models) + +### Fixed + +- **A named profile can no longer inherit the server's process-level provider credentials.** On a multi-profile instance, reads behind `/api/providers`, `/api/provider/quota`, and the synchronous `/api/models` rebuild could fall through to the server process environment — so a named profile with no key of its own would surface (and use, for quota probes) the default profile's API key. Credential reads now route through a thread-local profile channel that refuses the process-env fallback under a profile-scoped read: provider/model env vars, the agent auth-registry credentials (incl. OAuth tokens like `ANTHROPIC_TOKEN`/`CLAUDE_CODE_OAUTH_TOKEN`), the generic `CUSTOM_API_KEY`, the full AWS/Bedrock credential chain, and the Azure identity + managed-identity families are all scrubbed from the quota subprocess and the detached-worker model-rebuild env, and `${VAR}` config-template expansion no longer reconstructs a process credential under a scoped read. Region/base-URL config values are preserved. Thanks @rodboev. (#4544, fixes #3961) + ## [v0.51.589] — 2026-06-22 — Release UV (guard state.db replay after compressed anchors) ### Fixed diff --git a/api/config.py b/api/config.py index 9c769b59..fc42f035 100644 --- a/api/config.py +++ b/api/config.py @@ -25,6 +25,7 @@ import urllib.error import urllib.request import uuid from pathlib import Path +from typing import Any from urllib.parse import parse_qs, urlparse # ── Basic layout ────────────────────────────────────────────────────────────── @@ -251,12 +252,52 @@ if _AGENT_DIR is not None: else: _HERMES_FOUND = False +# ── Thread-local env context ───────────────────────────────────────────────── +# Defined BEFORE the config-file section because _expand_env_vars() (below) calls +# _thread_local_env_value() and the import-time reload_config() runs during module +# load — a forward reference here would NameError on any startup config.yaml that +# uses a ${VAR} reference. Depends only on os + threading (both imported above). +_thread_ctx = threading.local() + + +def _thread_local_env_value(name: str, default: str = "") -> str: + """Return thread-local profile env first, then process env, for provider reads.""" + env_name = str(name or "").strip() + if not env_name: + return default or "" + + thread_env = getattr(_thread_ctx, "env", {}) + if isinstance(thread_env, dict) and env_name in thread_env: + thread_value = thread_env.get(env_name) + if thread_value is None: + return default or "" + return str(thread_value) + + if bool(getattr(_thread_ctx, "block_process_env_fallback", False)): + return default or "" + + return str(os.getenv(env_name, default or "")) + + # ── Config file (reloadable -- supports profile switching) ────────────────── def _expand_env_vars(obj): - """Recursively expand ${VAR} references in config values using os.environ.""" + """Recursively expand ${VAR} references in config values. + + Uses the thread-local-first profile env lookup (_thread_local_env_value) so a + ${VAR} reference in a profile's config.yaml resolves to that profile's value, + and — critically — does NOT fall back to the server process os.environ when a + profile-scoped readonly/background scope set block_process_env_fallback. The + raw (unexpanded) dict is what gets cached; this expansion re-runs on every + read against the current thread's scope, so a cross-profile credential + (e.g. config api_key: ${ANTHROPIC_TOKEN}) can't be reconstructed from the + server process env for a named profile that has no such value (#3961).""" if isinstance(obj, str): - return re.sub(r"\${([^}]+)}", lambda m: os.environ.get(m.group(1), m.group(0)), obj) + return re.sub( + r"\${([^}]+)}", + lambda m: _thread_local_env_value(m.group(1), m.group(0)), + obj, + ) if isinstance(obj, dict): return {k: _expand_env_vars(v) for k, v in obj.items()} if isinstance(obj, list): @@ -449,7 +490,29 @@ def reload_config() -> None: if config_path.exists(): loaded = _yaml.safe_load(config_path.read_text(encoding="utf-8")) if isinstance(loaded, dict): - _cfg_cache.update(_expand_env_vars(loaded)) + # The process-global _cfg_cache must reflect PROCESS-env + # expansion, never a profile-scoped block_process_env_fallback + # view — otherwise a reload that fires while a readonly/worker + # scope is active (profile alternation resolves _get_config_path + # to the named profile, #798 TLS) would bake under-expanded + # literal ${VAR}s into the shared cache and starve concurrent + # readers of the module-level `cfg` alias. Expansion re-runs + # per-read elsewhere; here we pin the cache to the unscoped view. + _prev_block = getattr(_thread_ctx, "block_process_env_fallback", False) + _prev_env = getattr(_thread_ctx, "env", None) + try: + _thread_ctx.block_process_env_fallback = False + _thread_ctx.env = {} + _cfg_cache.update(_expand_env_vars(loaded)) + finally: + _thread_ctx.block_process_env_fallback = _prev_block + if _prev_env is None: + try: + del _thread_ctx.env + except AttributeError: + pass + else: + _thread_ctx.env = _prev_env try: _cfg_mtime = Path(config_path).stat().st_mtime except OSError: @@ -1330,13 +1393,13 @@ def _legacy_custom_api_key_env_name(provider_id: object) -> str: def _lookup_custom_api_key_env(provider_id: object) -> str | None: """Look up sanitized custom-provider env first, then legacy broken shape.""" env_name = _api_key_env_name(provider_id) - api_key = os.getenv(env_name, "").strip() + api_key = _thread_local_env_value(env_name).strip() if api_key: return api_key legacy_env_name = _legacy_custom_api_key_env_name(provider_id) if legacy_env_name and legacy_env_name != env_name: - legacy_key = os.getenv(legacy_env_name, "").strip() + legacy_key = _thread_local_env_value(legacy_env_name).strip() if legacy_key: if legacy_env_name not in _LEGACY_CUSTOM_API_KEY_ENV_WARNED: _LEGACY_CUSTOM_API_KEY_ENV_WARNED.add(legacy_env_name) @@ -2545,13 +2608,13 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st if raw_api_key is not None: key_text = str(raw_api_key).strip() if key_text.startswith("${") and key_text.endswith("}") and len(key_text) > 3: - api_key = os.getenv(key_text[2:-1], "").strip() or None + api_key = _thread_local_env_value(key_text[2:-1]).strip() or None elif key_text: api_key = key_text if not api_key: key_env = str(raw_key_env or "").strip() if key_env: - api_key = os.getenv(key_env, "").strip() or None + api_key = _thread_local_env_value(key_env).strip() or None if not api_key and provider_hint: api_key = _lookup_custom_api_key_env(provider_hint) return api_key @@ -4310,6 +4373,90 @@ def _credential_pool_profile_tag() -> str: return "" +def _pool_entry_payloads(provider_id: str) -> list[dict[str, Any]]: + """Return explicit credential-pool entry payloads for the active profile. + + Readonly profile scopes must not let ``load_pool()`` seed from process env, + because that can materialize server-default credentials into a named + profile's auth store. In that mode, read raw auth.json payloads only. + """ + _pid = _resolve_provider_alias(provider_id) + if bool(getattr(_thread_ctx, "block_process_env_fallback", False)): + try: + from hermes_cli.auth import read_credential_pool as _read_credential_pool + + raw_entries = _read_credential_pool(_pid) + except ImportError: + return [] + payloads: list[dict[str, Any]] = [] + for entry in raw_entries: + if not isinstance(entry, dict): + continue + if _is_ambient_gh_cli_entry( + str(entry.get("source", "") or ""), + str(entry.get("label", "") or ""), + str(entry.get("key_source", "") or ""), + ): + continue + payloads.append(dict(entry)) + return payloads + + try: + from agent.credential_pool import load_pool as _load_pool + + _ck = (_credential_pool_profile_tag(), _pid) + _cached = _CREDENTIAL_POOL_CACHE.get(_ck) + if _cached is not None: + _cp_ts, _cp_pool = _cached + if (time.time() - _cp_ts) < 86400.0: + _all_entries = _cp_pool.entries() if _cp_pool is not None and hasattr(_cp_pool, "entries") else [] + else: + _cp_pool = _load_pool(_pid) + _CREDENTIAL_POOL_CACHE[_ck] = (time.time(), _cp_pool) + _all_entries = _cp_pool.entries() if _cp_pool is not None and hasattr(_cp_pool, "entries") else [] + else: + _cp_pool = _load_pool(_pid) + _CREDENTIAL_POOL_CACHE[_ck] = (time.time(), _cp_pool) + _all_entries = _cp_pool.entries() if _cp_pool is not None and hasattr(_cp_pool, "entries") else [] + except ImportError: + return [] + + payloads = [] + for entry in _all_entries: + if _is_ambient_gh_cli_entry( + str(getattr(entry, "source", "") or ""), + str(getattr(entry, "label", "") or ""), + str(getattr(entry, "key_source", "") or ""), + ): + continue + if hasattr(entry, "to_dict") and callable(entry.to_dict): + payload = entry.to_dict() + elif isinstance(entry, dict): + payload = dict(entry) + else: + try: + payload = dict(vars(entry)) + except TypeError: + payload = {} + if not isinstance(payload, dict): + payload = {} + payload = dict(payload) + payload.setdefault("source", str(getattr(entry, "source", "") or "")) + payload.setdefault("label", str(getattr(entry, "label", "") or "")) + payload.setdefault("key_source", str(getattr(entry, "key_source", "") or "")) + runtime_api_key = getattr(entry, "runtime_api_key", None) + if runtime_api_key: + payload["runtime_api_key"] = runtime_api_key + base_url = getattr(entry, "base_url", None) + if base_url: + payload["base_url"] = base_url + inference_base_url = getattr(entry, "inference_base_url", None) + if inference_base_url: + payload["inference_base_url"] = inference_base_url + payloads.append(payload) + return payloads + + def _has_explicit_pool_credentials(provider_id: str) -> bool: """Return True when the credential pool has at least one non-ambient entry for *provider_id* (i.e. not a gh-cli / GITHUB_TOKEN auto-detect). @@ -4318,35 +4465,7 @@ def _has_explicit_pool_credentials(provider_id: str) -> bool: detection, model listing, live-model fetch) don't pay the ~10s load_pool cost more than once per TTL window. """ - try: - from agent.credential_pool import load_pool as _load_pool - - _pid = _resolve_provider_alias(provider_id) - _ck = (_credential_pool_profile_tag(), _pid) - _cached = _CREDENTIAL_POOL_CACHE.get(_ck) - if _cached is not None: - _cp_ts, _cp_pool = _cached - if (time.time() - _cp_ts) < 86400.0: - _all_entries = _cp_pool.entries() - else: - _cp_pool = _load_pool(_pid) - _CREDENTIAL_POOL_CACHE[_ck] = (time.time(), _cp_pool) - _all_entries = _cp_pool.entries() - else: - _cp_pool = _load_pool(_pid) - _CREDENTIAL_POOL_CACHE[_ck] = (time.time(), _cp_pool) - _all_entries = _cp_pool.entries() - - return any( - not _is_ambient_gh_cli_entry( - str(getattr(e, "source", "") or ""), - str(getattr(e, "label", "") or ""), - str(getattr(e, "key_source", "") or ""), - ) - for e in _all_entries - ) - except ImportError: - return False + return bool(_pool_entry_payloads(provider_id)) _provider_models_invalidated_ts: dict[str, float] = {} # provider_id -> timestamp of last invalidation # Disk-backed in-memory cache for get_available_models(). @@ -5454,7 +5573,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", ): - val = os.getenv(k) + val = _thread_local_env_value(k) if val: all_env[k] = val if all_env.get("ANTHROPIC_API_KEY"): @@ -5777,7 +5896,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: "API_KEY", ) for key in api_key_vars: - api_key = (all_env.get(key) or os.getenv(key) or "").strip() + api_key = (all_env.get(key) or _thread_local_env_value(key) or "").strip() if api_key: break @@ -5819,7 +5938,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: if not _cp_api_key: _cp_key_env = str(_cp.get("key_env") or "").strip() if _cp_key_env: - _cp_api_key = str(os.getenv(_cp_key_env) or "").strip() + _cp_api_key = _thread_local_env_value(_cp_key_env).strip() # Fallback: check credential pool for both api_key and base_url if (not _cp_api_key or not _cp_base_url) and _slug: try: @@ -6651,7 +6770,9 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: if _LIVE_REBUILD_BUDGET_SECONDS <= 0: try: # Foreground thread already carries the request-profile TLS; - # apply the profile env (no-op for default) for the live probe. + # apply the mirrored profile env (no-op for default) for the + # live probe because provider_model_ids() still has raw + # os.getenv()/HERMES_HOME readers on this synchronous path. _sync_scope = ( _prof_env_request("models rebuild (sync)") if _prof_env_request is not None @@ -7112,7 +7233,9 @@ def _evict_session_agent(session_id: str) -> None: logger.debug("Failed to close _session_db on eviction for %s", session_id, exc_info=True) # ── Thread-local env context ───────────────────────────────────────────────── -_thread_ctx = threading.local() +# (_thread_ctx + _thread_local_env_value are defined near the top of this module, +# above the config-file section, so _expand_env_vars can reference them at the +# import-time reload_config() without a forward-reference NameError.) def _set_thread_env(**kwargs): diff --git a/api/profiles.py b/api/profiles.py index d822a85a..7b8eccdf 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -885,6 +885,181 @@ def filter_runtime_env_for_gateway_parity(env: dict[str, str]) -> dict[str, str] return filtered +# Credential env vars the agent runtime resolves via raw os.getenv() that are +# NOT in hermes_cli.auth.PROVIDER_REGISTRY (so the registry-derived scrub set +# would miss them). Fail-closed list — verified against the installed agent: +# CUSTOM_API_KEY hermes_cli/models.py (generic custom provider key) +# AZURE_ANTHROPIC_KEY hermes_cli/runtime_provider.py (Azure-hosted Anthropic) +# AZURE_FOUNDRY_API_KEY hermes_cli/runtime_provider.py (Azure Foundry key) +# AZURE_* identity family agent/azure_identity_adapter.py (service-principal / +# workload-identity model auth) +# AWS_BEARER_TOKEN_BEDROCK hermes_cli/model_switch.py (Bedrock bearer token) +# AWS_* credential chain agent/bedrock_adapter.py + model_switch._has_aws_creds +# (boto3 access keys, session token, profile, +# container/web-identity credential providers) +# NOTE: region/base-url config vars (AWS_REGION, AWS_DEFAULT_REGION, +# AZURE_FOUNDRY_BASE_URL) are deliberately NOT included — they're configuration, +# not credentials, and the child probe may legitimately need them. +# Stripping these in a profile-scoped read prevents an empty named profile from +# inheriting the server-process credential (#3961 residual cross-profile leak). +_NON_REGISTRY_AGENT_CREDENTIAL_ENV_NAMES: tuple[str, ...] = ( + "CUSTOM_API_KEY", + # Anthropic OAuth/token aliases. These ARE in the agent auth registry, but + # are duplicated here as a fail-closed floor so the scrub still covers them + # when the agent package can't be imported (e.g. a WebUI-only CI/test env + # where hermes_cli.auth is absent) — the registry union is best-effort. + "ANTHROPIC_TOKEN", + "CLAUDE_CODE_OAUTH_TOKEN", + "AZURE_ANTHROPIC_KEY", + "AZURE_FOUNDRY_API_KEY", + "AZURE_CLIENT_ID", + "AZURE_CLIENT_SECRET", + "AZURE_TENANT_ID", + "AZURE_FEDERATED_TOKEN_FILE", + # Azure managed-identity (App Service MSI / IMDS) credential-source vars — + # agent/azure_identity_adapter.py treats these as ManagedIdentityCredential + # sources, so an empty named profile must not inherit the host's managed + # identity. (NOT AZURE_FOUNDRY_BASE_URL — that's config, kept.) + "IDENTITY_ENDPOINT", + "IDENTITY_HEADER", + "MSI_ENDPOINT", + "MSI_SECRET", + "AWS_BEARER_TOKEN_BEDROCK", + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "AWS_SESSION_TOKEN", + "AWS_PROFILE", + "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI", + "AWS_CONTAINER_CREDENTIALS_FULL_URI", + "AWS_WEB_IDENTITY_TOKEN_FILE", +) + + +def _agent_registry_credential_env_names() -> set[str]: + """Credential env-var names the *agent* runtime reads, beyond the WebUI's own + settable-key map. Two sources: + + 1. ``hermes_cli.auth.PROVIDER_REGISTRY[*].api_key_env_vars`` — every provider + the agent CLI knows, incl. OAuth/token-flow providers like Anthropic's + ``ANTHROPIC_TOKEN`` / ``CLAUDE_CODE_OAUTH_TOKEN`` that the WebUI's own + ``_PROVIDER_ENV_VAR`` map omits (they aren't WebUI-settable API keys). + 2. ``_NON_REGISTRY_AGENT_CREDENTIAL_ENV_NAMES`` — a fail-closed fallback for + credential env vars the agent resolves via raw ``os.getenv()`` that are NOT + in the auth registry (the generic ``CUSTOM_API_KEY`` and the AWS/Bedrock + credential family the bedrock adapter relies on). + + A profile scrub built only from the WebUI map would leave all of these in + ``os.environ`` — letting an empty named profile inherit the server-process + credential on the quota subprocess and detached-worker model-rebuild paths + (#3961 residual cross-profile leak).""" + names: set[str] = set(_NON_REGISTRY_AGENT_CREDENTIAL_ENV_NAMES) + try: + from hermes_cli.auth import PROVIDER_REGISTRY + + registry = PROVIDER_REGISTRY + items = registry.items() if hasattr(registry, "items") else enumerate(registry) + for _key, entry in items: + env_vars = getattr(entry, "api_key_env_vars", None) + for env_var in env_vars or (): + if env_var: + names.add(str(env_var)) + except Exception: + logger.debug( + "Failed to load agent registry credential env names for profile scope", + exc_info=True, + ) + return names + + +def _profile_secret_env_names(profile_home_path: Path) -> set[str]: + names: set[str] = set() + try: + from api.providers import _provider_credential_env_vars + + names.update(_provider_credential_env_vars()) + except Exception: + logger.debug( + "Failed to load provider credential env names for profile scope", + exc_info=True, + ) + + # Also scrub credential env vars the agent runtime resolves directly + # (OAuth/token-flow providers absent from the WebUI's settable-key map) so a + # profile-scoped read can't inherit the server process's ANTHROPIC_TOKEN / + # CLAUDE_CODE_OAUTH_TOKEN etc. (#3961 cross-profile residual leak). + names.update(_agent_registry_credential_env_names()) + + config_path = Path(profile_home_path) / "config.yaml" + if not config_path.exists(): + return names + try: + payload = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {} + except Exception: + logger.debug( + "Failed to inspect custom-provider credential env names from %s", + config_path, + exc_info=True, + ) + return names + + custom_providers = payload.get("custom_providers") if isinstance(payload, dict) else None + if not isinstance(custom_providers, list): + return names + for custom_provider in custom_providers: + if not isinstance(custom_provider, dict): + continue + key_env = str(custom_provider.get("key_env") or "").strip() + if key_env: + names.add(key_env) + api_key = str(custom_provider.get("api_key") or "").strip() + match = re.fullmatch(r"\$\{([^}]+)\}", api_key) + if match: + env_name = str(match.group(1) or "").strip() + if env_name: + names.add(env_name) + return names + + +def _apply_profile_env_to_process( + process_env, + safe_runtime_env: dict[str, str], + *, + secret_env_names: set[str], +) -> dict[str, Optional[str]]: + scoped_keys = set(safe_runtime_env) | set(secret_env_names) + previous_env = {key: process_env.get(key) for key in scoped_keys} + for key in secret_env_names: + if key not in safe_runtime_env: + process_env.pop(key, None) + return previous_env + + +_secret_scope_available = None + + +def _resolve_secret_scope_module(): + global _secret_scope_available + import sys as _sys + mod = _sys.modules.get('agent.secret_scope') + if mod is not None: + return mod + if _secret_scope_available is False: + return None + if _secret_scope_available is None: + try: + import importlib.util + _secret_scope_available = importlib.util.find_spec('agent') is not None + except Exception: + _secret_scope_available = False + if _secret_scope_available: + try: + from agent.secret_scope import set_secret_scope, reset_secret_scope # noqa: F401 + return _sys.modules.get('agent.secret_scope') + except ImportError: + _secret_scope_available = False + return None + + @contextmanager def profile_env_for_background_worker( session, @@ -914,6 +1089,7 @@ def profile_env_for_background_worker( profile_home_path = Path(get_hermes_home_for_profile(profile)) runtime_env = get_profile_runtime_env(profile_home_path) safe_runtime_env = filter_runtime_env_for_gateway_parity(runtime_env) + secret_env_names = _profile_secret_env_names(profile_home_path) except Exception: log.debug( "Failed to resolve profile env for %s profile %s; falling back to current env", @@ -936,10 +1112,29 @@ def profile_env_for_background_worker( old_hermes_home = None had_hermes_home = False previous_thread_env = getattr(_thread_ctx, "env", {}).copy() + previous_block_process_env = bool( + getattr(_thread_ctx, "block_process_env_fallback", False) + ) + _scope_token = None + _has_scope = False try: _set_thread_env(**thread_env) + _thread_ctx.block_process_env_fallback = True + _secret_scope_mod = _resolve_secret_scope_module() + _scope_token = None + _has_scope = False + if _secret_scope_mod is not None: + try: + _scope_token = _secret_scope_mod.set_secret_scope(thread_env) + _has_scope = True + except Exception: + pass with _ENV_LOCK: - old_runtime_env = {key: os.environ.get(key) for key in safe_runtime_env} + old_runtime_env = _apply_profile_env_to_process( + os.environ, + safe_runtime_env, + secret_env_names=secret_env_names, + ) had_hermes_home = "HERMES_HOME" in os.environ old_hermes_home = os.environ.get("HERMES_HOME") skill_home_snapshot = snapshot_skill_home_modules() @@ -956,6 +1151,12 @@ def profile_env_for_background_worker( ) yield finally: + if _has_scope and _secret_scope_mod is not None: + try: + _secret_scope_mod.reset_secret_scope(_scope_token) + except Exception: + pass + _thread_ctx.block_process_env_fallback = previous_block_process_env if previous_thread_env: _set_thread_env(**previous_thread_env) else: @@ -975,31 +1176,113 @@ def profile_env_for_background_worker( @contextmanager -def profile_env_for_active_request( +def profile_env_for_active_request_readonly( purpose: str = "provider/model read", logger_override: Optional[logging.Logger] = None, ): - """Apply the active per-request profile's env around a read-only API call (#3957). + """Apply the active per-request profile's env to thread-local state only (#3957). WebUI profile switching is per-client/cookie scoped (issue #798): a browser on a named profile sets a ``hermes_profile`` cookie, which ``server.py`` - turns into a thread-local via ``set_request_profile()``. But the per-client - switch deliberately does NOT reload the profile's ``.env`` into - ``os.environ`` (that would mutate process-global state shared with other - clients). So read-only endpoints that resolve provider credentials through - ``os.environ`` / ``HERMES_HOME`` — ``/api/providers`` (``get_auth_status`` - probes) and ``/api/models`` (``provider_model_ids`` / custom-key lookup) — - resolve against the *default* profile's credentials instead of the active - one. Symptoms: Settings → Providers times out and the model picker shows - only the default profile's models. + turns into a thread-local via ``set_request_profile()``. This wrapper keeps + provider-credential reads isolated to the request profile and does not touch + process-wide environment for read-only endpoints. - This mirrors what streaming already does for the duration of an agent run - (``profile_env_for_background_worker``): it temporarily applies the active - profile's ``.env`` + terminal config for the duration of the wrapped read, - then restores the previous env under ``_ENV_LOCK``. + A thread-local read-only scope is used for ``/api/providers`` and + ``/api/models`` flows that now resolve credentials through thread-local + environment first. It also sets a context-local Hermes-home override so + agent-side auth-store reads stay on the active profile without mutating + process-global ``os.environ``. - No-ops (zero overhead, byte-identical behavior) for the default/root profile - — the overwhelmingly common single-profile deployment is unaffected. + No-ops for the default/root profile, which is the common single-profile + deployment case. + """ + profile = (get_active_profile_name() or "").strip() + if not profile or _is_root_profile(profile): + yield + return + try: + from api.config import _clear_thread_env, _set_thread_env, _thread_ctx + profile_home_path = Path(get_hermes_home_for_profile(profile)) + runtime_env = get_profile_runtime_env(profile_home_path) + safe_runtime_env = filter_runtime_env_for_gateway_parity(runtime_env) + except Exception: + log = logger_override or logger + log.debug( + "Failed to resolve profile env for active request profile %s in %s; " + "falling back to current env", + profile, + purpose, + exc_info=True, + ) + yield + return + try: + from hermes_constants import ( + reset_hermes_home_override, + set_hermes_home_override, + ) + except Exception: + reset_hermes_home_override = None + set_hermes_home_override = None + + thread_env = dict(safe_runtime_env) + thread_env["HERMES_HOME"] = str(profile_home_path) + previous_thread_env = getattr(_thread_ctx, "env", {}).copy() + previous_block_process_env = bool( + getattr(_thread_ctx, "block_process_env_fallback", False) + ) + home_override_token = None + _scope_token = None + _has_scope = False + try: + _set_thread_env(**thread_env) + _thread_ctx.block_process_env_fallback = True + _secret_scope_mod = _resolve_secret_scope_module() + _scope_token = None + _has_scope = False + if _secret_scope_mod is not None: + try: + _scope_token = _secret_scope_mod.set_secret_scope(thread_env) + _has_scope = True + except Exception: + pass + if set_hermes_home_override is not None: + home_override_token = set_hermes_home_override(profile_home_path) + yield + finally: + if _has_scope and _secret_scope_mod is not None: + try: + _secret_scope_mod.reset_secret_scope(_scope_token) + except Exception: + pass + if home_override_token is not None and reset_hermes_home_override is not None: + try: + reset_hermes_home_override(home_override_token) + except Exception: + (logger_override or logger).debug( + "Failed to reset Hermes-home override for active request profile %s in %s", + profile, + purpose, + exc_info=True, + ) + _thread_ctx.block_process_env_fallback = previous_block_process_env + if previous_thread_env: + _set_thread_env(**previous_thread_env) + else: + _clear_thread_env() + + +@contextmanager +def profile_env_for_active_request( + purpose: str = "active request", + logger_override: Optional[logging.Logger] = None, +): + """Apply the active per-request profile through the legacy mirrored path. + + Some request-scoped readers still delegate into Hermes helpers that resolve + credentials directly from process env or ``get_hermes_home()``. Those paths + stay on the mirrored scope until they are fully audited. """ profile = (get_active_profile_name() or "").strip() if not profile or _is_root_profile(profile): diff --git a/api/providers.py b/api/providers.py index bf7fece8..ec4a1b49 100644 --- a/api/providers.py +++ b/api/providers.py @@ -37,9 +37,11 @@ from api.config import ( _custom_provider_slug_from_name, _get_label_for_model, _models_from_live_provider_ids, + _pool_entry_payloads, _read_live_provider_model_ids, _read_visible_codex_cache_model_ids, _save_yaml_config_file, + _thread_local_env_value, get_config, invalidate_models_cache, reload_config, @@ -741,6 +743,18 @@ _PROVIDER_ENV_VAR_ALIASES: dict[str, tuple[str, ...]] = { "opencode-go": ("OPENCODE_API_KEY",), } + +def _provider_credential_env_vars() -> tuple[str, ...]: + names = {name for name in _PROVIDER_ENV_VAR.values() if name} + for aliases in _PROVIDER_ENV_VAR_ALIASES.values(): + for alias in aliases or (): + if alias: + names.add(alias) + return tuple(sorted(names)) + + +_PROVIDER_CREDENTIAL_ENV_VARS = _provider_credential_env_vars() + # Providers that use OAuth or token flows — their credentials are managed # through the Hermes CLI, not via API keys. The WebUI cannot set these. _OAUTH_PROVIDERS = frozenset({ @@ -853,11 +867,7 @@ def _local_pool_snapshot(provider): or None if the provider has no pool or no entries. """ try: - from agent.credential_pool import load_pool - from api.config import _is_ambient_gh_cli_entry - - pool = load_pool(provider) - entries = list(pool.entries()) if pool is not None and hasattr(pool, "entries") else [] + entries = [SimpleNamespace(**payload) for payload in _pool_entry_payloads(provider)] except Exception: return None if not entries: @@ -868,11 +878,6 @@ def _local_pool_snapshot(provider): exhausted_count = 0 dead_count = 0 for index, entry in enumerate(entries, start=1): - source = str(_entry_value(entry, "source") or "") - label_val = str(_entry_value(entry, "label", "source") or "") - key_source = str(_entry_value(entry, "key_source") or "") - if _is_ambient_gh_cli_entry(source, label_val, key_source): - continue label = _safe_entry_label(entry, index) entry_status = str(_entry_value(entry, "last_status") or "").strip().lower() if entry_status == "dead": @@ -1032,10 +1037,10 @@ def _provider_has_shadowed_codex_oauth_value(provider_id: str) -> bool: env_path = _get_hermes_home() / ".env" env_values = _load_env_file(env_path) values.append(env_values.get(env_var)) - values.append(os.getenv(env_var)) + values.append(_thread_local_env_value(env_var)) for alias in _PROVIDER_ENV_VAR_ALIASES.get(provider_id, ()) or (): values.append(env_values.get(alias)) - values.append(os.getenv(alias)) + values.append(_thread_local_env_value(alias)) cfg = get_config() model_cfg = cfg.get("model", {}) @@ -1054,7 +1059,7 @@ def _provider_has_shadowed_codex_oauth_value(provider_id: str) -> bool: if isinstance(cp, dict) and _custom_provider_name_matches(provider_id, cp.get("name")): cp_key = cp.get("api_key") if isinstance(cp_key, str) and cp_key.startswith("${") and cp_key.endswith("}"): - values.append(os.getenv(cp_key[2:-1])) + values.append(_thread_local_env_value(cp_key[2:-1])) else: values.append(cp_key) return any(_looks_like_codex_oauth_token(str(value or "")) for value in values) @@ -1175,7 +1180,7 @@ def _provider_has_key(provider_id: str) -> bool: env_file_value = env_values.get(env_var) if _provider_value_counts_as_api_key(provider_id, env_file_value): return True - env_value = os.getenv(env_var) + env_value = _thread_local_env_value(env_var) if _provider_value_counts_as_api_key(provider_id, env_value): return True # Fall back to legacy env-var aliases (e.g. lmstudio's pre-#1500 @@ -1184,7 +1189,7 @@ def _provider_has_key(provider_id: str) -> bool: for alias in _PROVIDER_ENV_VAR_ALIASES.get(provider_id, ()) or (): if _provider_value_counts_as_api_key(provider_id, env_values.get(alias)): return True - if _provider_value_counts_as_api_key(provider_id, os.getenv(alias)): + if _provider_value_counts_as_api_key(provider_id, _thread_local_env_value(alias)): return True # Check credential pool — covers custom providers registered via # `hermes auth add` which store keys in auth.json (not config.yaml). @@ -1238,14 +1243,14 @@ def _get_provider_api_key(provider_id: str) -> str | None: env_file_value = env_values.get(env_var) if _provider_value_counts_as_api_key(provider_id, env_file_value): return str(env_file_value).strip() or None - env_value = os.getenv(env_var) + env_value = _thread_local_env_value(env_var) if _provider_value_counts_as_api_key(provider_id, env_value): return str(env_value).strip() or None for alias in _PROVIDER_ENV_VAR_ALIASES.get(provider_id, ()) or (): alias_file_value = env_values.get(alias) if _provider_value_counts_as_api_key(provider_id, alias_file_value): return str(alias_file_value).strip() or None - alias_value = os.getenv(alias) + alias_value = _thread_local_env_value(alias) if _provider_value_counts_as_api_key(provider_id, alias_value): return str(alias_value).strip() or None @@ -1273,26 +1278,26 @@ def _get_provider_api_key(provider_id: str) -> str | None: if _custom_provider_name_matches(provider_id, cp.get("name")): cp_key = str(cp.get("api_key") or "").strip() if cp_key.startswith("${") and cp_key.endswith("}"): - return os.getenv(cp_key[2:-1], "").strip() or None + return _thread_local_env_value(cp_key[2:-1]).strip() or None if _provider_value_counts_as_api_key(provider_id, cp_key): return cp_key # Fallback: try credential pool (e.g. bothub key stored via auth.json) - try: - from api.config import _has_explicit_pool_credentials, _resolve_provider_alias - if _has_explicit_pool_credentials(provider_id): - from agent.credential_pool import load_pool - # Must resolve alias here too: _has_explicit_pool_credentials does it - # internally, but load_pool sees the original unresolved provider_id. - _resolved = _resolve_provider_alias(provider_id) - pool = load_pool(_resolved) - if pool: - entry = pool.select() - if entry: - key = getattr(entry, "runtime_api_key", "") or getattr(entry, "access_token", "") - if key: - return key - except ImportError: - pass + for entry in _pool_entry_payloads(provider_id): + status = str(entry.get("last_status") or "").strip().lower() + if status == "dead": + continue + if status == "exhausted": + ns = SimpleNamespace(**entry) + if _entry_is_pool_exhausted(ns): + continue + key = str( + entry.get("runtime_api_key") + or entry.get("agent_key") + or entry.get("access_token") + or "" + ).strip() + if key: + return key return None @@ -1394,6 +1399,24 @@ def _agent_fetch_account_usage(provider: str, *, base_url: str | None = None, ap def _account_usage_subprocess_env(home: Path, provider: str, api_key: str | None) -> dict[str, str]: env = dict(os.environ) + try: + from api.config import _thread_ctx + except Exception: + _thread_ctx = None + if bool(getattr(_thread_ctx, "block_process_env_fallback", False)): + # Rely on the centralized profile scrub set (api.profiles), which unions + # the WebUI provider env vars + the agent auth registry + the non-registry + # agent credential fallback (CUSTOM_API_KEY, AWS/Bedrock family). Falling + # back to the WebUI-only set keeps the probe fail-closed if that import + # fails. (#3961 — don't leave a partial local AWS set here.) + _strip = set(_PROVIDER_CREDENTIAL_ENV_VARS) + try: + from api.profiles import _profile_secret_env_names, get_active_hermes_home + _strip.update(_profile_secret_env_names(get_active_hermes_home())) + except Exception: + _strip.update({"AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY"}) + for env_name in _strip: + env.pop(env_name, None) env["HERMES_HOME"] = str(Path(home)) # Profile .env values should affect only the child quota probe, not the @@ -2382,7 +2405,7 @@ def get_providers() -> dict[str, Any]: env_values = _load_env_file(env_path) if _provider_value_counts_as_api_key(pid, env_values.get(env_var)): key_source = "env_file" - elif _provider_value_counts_as_api_key(pid, os.getenv(env_var)): + elif _provider_value_counts_as_api_key(pid, _thread_local_env_value(env_var)): key_source = "env_var" else: # Canonical name not set; check legacy aliases (e.g. lmstudio's @@ -2395,7 +2418,7 @@ def get_providers() -> dict[str, Any]: key_source = "env_file" aliased = True break - if _provider_value_counts_as_api_key(pid, os.getenv(alias)): + if _provider_value_counts_as_api_key(pid, _thread_local_env_value(alias)): key_source = "env_var" aliased = True break @@ -2592,7 +2615,7 @@ def get_providers() -> dict[str, Any]: # Replace env var reference to check actual value if cp_api_key.startswith("${") and cp_api_key.endswith("}"): env_var = cp_api_key[2:-1] - cp_has_key = bool(os.getenv(env_var, "").strip()) + cp_has_key = bool(_thread_local_env_value(env_var).strip()) # Fallback: check credential pool (key added via hermes auth add) if not cp_has_key: try: diff --git a/api/routes.py b/api/routes.py index cc794da4..ddeeb0de 100644 --- a/api/routes.py +++ b/api/routes.py @@ -9112,8 +9112,8 @@ def handle_get(handler, parsed) -> bool: # profile's (#3957). Without this, get_auth_status() probes on a # non-default profile resolve the wrong/empty creds and can stall past # the 30s frontend timeout. No-op for the default profile. - from api.profiles import profile_env_for_active_request - with profile_env_for_active_request("/api/providers", logger_override=logger): + from api.profiles import profile_env_for_active_request_readonly + with profile_env_for_active_request_readonly("/api/providers", logger_override=logger): return j(handler, get_providers()) # ── Plugins/hooks visibility (read-only, no callback/source internals) ── @@ -9129,8 +9129,8 @@ def handle_get(handler, parsed) -> bool: # read/write runs under the process-default profile, so a multi-profile # client would see (and seed) the default profile's pool instead of its # own (#4247/#4067 profile-isolation class). - from api.profiles import profile_env_for_active_request - with profile_env_for_active_request("/api/provider/quota", logger_override=logger): + from api.profiles import profile_env_for_active_request_readonly + with profile_env_for_active_request_readonly("/api/provider/quota", logger_override=logger): return j(handler, get_provider_quota(provider_id, refresh=refresh)) if parsed.path == "/api/provider/cost-history": diff --git a/tests/test_issue3957_profile_providers_models.py b/tests/test_issue3957_profile_providers_models.py index 2fb66a66..d77a76c3 100644 --- a/tests/test_issue3957_profile_providers_models.py +++ b/tests/test_issue3957_profile_providers_models.py @@ -19,11 +19,13 @@ The fix: - ``api.config._get_models_cache_path()`` returns a profile-keyed path (``models_cache..json`` for named profiles; unchanged ``models_cache.json`` for the default/root profile). - - ``api.profiles.profile_env_for_active_request()`` applies the active + - ``api.profiles.profile_env_for_active_request_readonly()`` applies the active per-request profile's env around the read; no-op for the default profile. """ import os +import sys +import types from pathlib import Path import api.config as config @@ -113,14 +115,14 @@ def test_active_request_env_noop_for_default_profile(monkeypatch): monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "default") monkeypatch.setattr(profiles, "_is_root_profile", lambda n: n in ("", "default")) monkeypatch.delenv("ISSUE_3957_PROBE", raising=False) - with profiles.profile_env_for_active_request("test"): + with profiles.profile_env_for_active_request_readonly("test"): # No env mutation, no HERMES_HOME change for the default profile. assert os.environ.get("ISSUE_3957_PROBE") is None assert os.environ.get("ISSUE_3957_PROBE") is None def test_active_request_env_applies_named_profile_env(monkeypatch, tmp_path): - """A named profile's .env is applied inside the block and restored after.""" + """A named profile's .env is bound to thread-local state, process env untouched.""" base = tmp_path / ".hermes" (base / "profiles" / "work").mkdir(parents=True) (base / "profiles" / "work" / ".env").write_text( @@ -130,14 +132,17 @@ def test_active_request_env_applies_named_profile_env(monkeypatch, tmp_path): monkeypatch.delenv("ISSUE_3957_PROBE", raising=False) # Simulate the per-request cookie context (issue #798). + monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env") profiles.set_request_profile("work") try: assert profiles.get_active_profile_name() == "work" - assert os.environ.get("ISSUE_3957_PROBE") is None - with profiles.profile_env_for_active_request("test"): - assert os.environ.get("ISSUE_3957_PROBE") == "from-work-profile" + assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env" + with profiles.profile_env_for_active_request_readonly("test"): + assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-work-profile" + assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env" # Restored after the block exits. - assert os.environ.get("ISSUE_3957_PROBE") is None + assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env" + assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env" finally: profiles.clear_request_profile() @@ -151,41 +156,267 @@ def test_active_request_env_restores_on_exception(monkeypatch, tmp_path): ) monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) monkeypatch.delenv("ISSUE_3957_PROBE", raising=False) + monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env") profiles.set_request_profile("work") try: with_raised = False try: - with profiles.profile_env_for_active_request("test"): - assert os.environ.get("ISSUE_3957_PROBE") == "from-work-profile" + with profiles.profile_env_for_active_request_readonly("test"): + assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-work-profile" + assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env" raise ValueError("boom") except ValueError: with_raised = True assert with_raised - assert os.environ.get("ISSUE_3957_PROBE") is None + assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env" + assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env" finally: profiles.clear_request_profile() +def test_active_request_scope_prefers_profile_key_over_process_env_for_custom_provider( + monkeypatch, + tmp_path, +): + """Profile-scope thread env resolves custom-provider env vars before process env.""" + base = tmp_path / ".hermes" + (base / "profiles" / "work").mkdir(parents=True) + (base / "profiles" / "work" / ".env").write_text( + "ISSUE_3957_CUSTOM_KEY=from-work-profile\n", encoding="utf-8" + ) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ISSUE_3957_CUSTOM_KEY", "from-process-env") + monkeypatch.setattr( + config, + "get_config", + lambda: { + "custom_providers": [ + {"name": "Team", "api_key": "${ISSUE_3957_CUSTOM_KEY}"} + ] + }, + ) + + profiles.set_request_profile("work") + try: + assert config.resolve_custom_provider_connection("custom:team") == ( + "from-process-env", + None, + ) + with profiles.profile_env_for_active_request_readonly("test"): + assert config.resolve_custom_provider_connection("custom:team") == ( + "from-work-profile", + None, + ) + assert os.environ.get("ISSUE_3957_CUSTOM_KEY") == "from-process-env" + assert config._thread_local_env_value("ISSUE_3957_CUSTOM_KEY") == ( + "from-process-env" + ) + finally: + profiles.clear_request_profile() + + +def test_active_request_scope_sets_context_local_hermes_home(monkeypatch, tmp_path): + """Request scope keeps agent-side Hermes-home readers on the active profile.""" + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + current_home = {"value": None} + + fake_constants = types.SimpleNamespace() + + def _set_override(path): + previous = current_home["value"] + current_home["value"] = Path(path) + return previous + + def _reset_override(token): + current_home["value"] = token + + fake_constants.set_hermes_home_override = _set_override + fake_constants.reset_hermes_home_override = _reset_override + fake_constants.get_hermes_home = lambda: current_home["value"] + monkeypatch.setitem(sys.modules, "hermes_constants", fake_constants) + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("test"): + assert fake_constants.get_hermes_home() == work_home + finally: + profiles.clear_request_profile() + + +def test_active_request_scope_restores_state_when_home_reset_fails(monkeypatch, tmp_path): + """Readonly scope still clears thread-local state if Hermes-home reset raises.""" + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + (work_home / ".env").write_text( + "ISSUE_3957_PROBE=from-work-profile\n", encoding="utf-8" + ) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env") + current_home = {"value": None} + + fake_constants = types.SimpleNamespace() + + def _set_override(path): + previous = current_home["value"] + current_home["value"] = Path(path) + return previous + + def _reset_override(token): + current_home["value"] = token + raise RuntimeError("reset failed") + + fake_constants.set_hermes_home_override = _set_override + fake_constants.reset_hermes_home_override = _reset_override + fake_constants.get_hermes_home = lambda: current_home["value"] + monkeypatch.setitem(sys.modules, "hermes_constants", fake_constants) + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("test"): + assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-work-profile" + assert fake_constants.get_hermes_home() == work_home + assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env" + assert getattr(config._thread_ctx, "block_process_env_fallback", False) is False + finally: + profiles.clear_request_profile() + + +def test_active_request_legacy_scope_still_mirrors_process_env(monkeypatch, tmp_path): + """Live-model request scope still mirrors env for agent-side readers.""" + base = tmp_path / ".hermes" + (base / "profiles" / "work").mkdir(parents=True) + (base / "profiles" / "work" / ".env").write_text( + "ISSUE_3957_PROBE=from-work-profile\n", encoding="utf-8" + ) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request("test"): + assert os.environ.get("ISSUE_3957_PROBE") == "from-work-profile" + assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env" + finally: + profiles.clear_request_profile() + + +def test_active_request_readonly_scope_blocks_process_env_fallback(monkeypatch, tmp_path): + """Named profiles without a key should not inherit the process-default key.""" + from api.providers import _provider_has_key + + base = tmp_path / ".hermes" + (base / "profiles" / "work").mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("OPENAI_API_KEY", "from-process-env") + + profiles.set_request_profile("work") + try: + assert _provider_has_key("openai") is True + with profiles.profile_env_for_active_request_readonly("test"): + assert _provider_has_key("openai") is False + finally: + profiles.clear_request_profile() + + +def test_active_request_readonly_scope_blocks_pool_env_seed(monkeypatch, tmp_path): + """Readonly profile reads must not let load_pool seed process-default keys.""" + from api.providers import _get_provider_api_key, _provider_has_key + + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("OPENROUTER_API_KEY", "from-process-env") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("test"): + assert _provider_has_key("openrouter") is False + assert _get_provider_api_key("openrouter") is None + finally: + profiles.clear_request_profile() + + assert (work_home / "auth.json").exists() is False + + def test_providers_and_models_routes_wrap_in_profile_env(): """The two read routes are profile-scoped for non-default profiles (#3957). Structural guard: a future refactor that drops the wiring would silently reintroduce the bug, so pin it at the source level. - - /api/providers wraps the synchronous read in profile_env_for_active_request. - - /api/models relies on get_available_models() scoping its detached - rebuild worker via profile_scope_for_detached_worker (the request-thread - wrapper cannot reach the worker thread — Codex CORE finding). + - /api/providers and /api/provider/quota wrap the synchronous read in + profile_env_for_active_request_readonly. + - /api/models/live stays on the mirrored profile_env_for_active_request + path because provider_model_ids() still delegates into agent helpers + that read process env / HERMES_HOME directly. + - /api/models relies on get_available_models() using the mirrored request + scope for the budget<=0 sync rebuild plus profile_scope_for_detached_worker + for the detached rebuild worker (the request-thread wrapper cannot reach + the worker thread — Codex CORE finding). """ routes_src = Path(profiles.__file__).resolve().parent.joinpath("routes.py").read_text( encoding="utf-8" ) - assert "profile_env_for_active_request" in routes_src + assert 'with profile_env_for_active_request("/api/models/live"' in routes_src + assert "profile_env_for_active_request_readonly" in routes_src config_src = Path(config.__file__).resolve().read_text(encoding="utf-8") + assert "profile_env_for_active_request as _prof_env_request" in config_src assert "profile_scope_for_detached_worker" in config_src assert "_get_models_cache_path" in config_src +def test_models_sync_rebuild_uses_legacy_mirrored_env(monkeypatch, tmp_path): + """The budget<=0 sync rebuild still mirrors profile env into os.environ.""" + base = tmp_path / ".hermes" + (base / "profiles" / "work").mkdir(parents=True) + (base / "profiles" / "work" / ".env").write_text( + "ISSUE_3957_PROBE=from-work-profile\n", encoding="utf-8" + ) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env") + monkeypatch.setattr(config, "_LIVE_REBUILD_BUDGET_SECONDS", 0) + monkeypatch.setattr(config, "_available_models_cache", None) + monkeypatch.setattr(config, "_available_models_cache_ts", 0.0) + monkeypatch.setattr(config, "_available_models_cache_source_fingerprint", None) + monkeypatch.setattr(config, "_cache_build_in_progress", False) + monkeypatch.setattr(config, "_load_models_cache_from_disk", lambda: None) + monkeypatch.setattr(config, "_save_models_cache_to_disk", lambda result: None) + monkeypatch.setattr(config, "_models_cache_source_fingerprint", lambda: "issue-3957") + seen = {} + + def _capture_rebuild(_builder): + seen["process_env"] = os.environ.get("ISSUE_3957_PROBE") + seen["thread_env"] = config._thread_local_env_value("ISSUE_3957_PROBE") + return {"active_provider": None, "default_model": "", "groups": []} + + monkeypatch.setattr(config, "_invoke_models_rebuild", _capture_rebuild) + + profiles.set_request_profile("work") + try: + result = config.get_available_models() + finally: + profiles.clear_request_profile() + + assert seen["process_env"] == "from-work-profile" + assert seen["thread_env"] == "from-work-profile" + assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env" + assert result["groups"] == [] + + +def test_thread_local_env_value_none_default_returns_empty_string(monkeypatch): + """A None default never escapes the string-return contract.""" + monkeypatch.setattr(config._thread_ctx, "env", {"ISSUE_3957_NONE": None}, raising=False) + monkeypatch.setattr( + config._thread_ctx, "block_process_env_fallback", False, raising=False + ) + assert config._thread_local_env_value("ISSUE_3957_NONE", None) == "" + + # ───────────────────────────────────────────────────────────────────────────── # Facet A (worker thread) — the detached models-rebuild worker is profile-scoped # (Codex CORE finding: the request-thread wrapper cannot reach the worker thread) @@ -249,3 +480,423 @@ def test_detached_worker_scope_binds_profile_on_new_thread(monkeypatch, tmp_path assert out["after_name"] == "models_cache.json" # restored assert out["after_env"] is None + +def test_detached_worker_prefers_profile_key_for_custom_provider(monkeypatch, tmp_path): + """Detached worker scope resolves custom-provider env from thread profile, not process env.""" + import threading + + base = tmp_path / ".hermes" + (base / "profiles" / "work").mkdir(parents=True) + (base / "profiles" / "work" / ".env").write_text( + "ISSUE_3957_CUSTOM_KEY=from-worker-profile\n", encoding="utf-8" + ) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ISSUE_3957_CUSTOM_KEY", "from-process-env") + monkeypatch.setattr( + config, + "get_config", + lambda: { + "custom_providers": [ + {"name": "team", "api_key": "${ISSUE_3957_CUSTOM_KEY}"} + ] + }, + ) + + out = {} + + def worker(): + with profiles.profile_scope_for_detached_worker("work", "test-worker"): + out["value"] = config.resolve_custom_provider_connection("custom:team")[0] + + t = threading.Thread(target=worker) + t.start() + t.join() + assert out["value"] == "from-worker-profile" + + +def test_detached_worker_scope_blocks_pool_env_seed(monkeypatch, tmp_path): + """Detached worker scope must not let load_pool seed process-default keys.""" + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("OPENROUTER_API_KEY", "from-process-env") + + with profiles.profile_scope_for_detached_worker("work", "test-worker"): + assert os.environ.get("OPENROUTER_API_KEY") is None + assert config._has_explicit_pool_credentials("openrouter") is False + assert getattr(config._thread_ctx, "block_process_env_fallback", False) is True + + assert os.environ.get("OPENROUTER_API_KEY") == "from-process-env" + assert getattr(config._thread_ctx, "block_process_env_fallback", False) is False + assert (work_home / "auth.json").exists() is False + + +def test_detached_worker_scope_scrubs_absent_custom_provider_key_env(monkeypatch, tmp_path): + """Detached worker scope clears missing custom-provider key_env fallbacks too.""" + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + (work_home / "config.yaml").write_text( + "custom_providers:\n" + " - name: Team\n" + " base_url: https://example.invalid/v1\n" + " key_env: ISSUE_3957_CUSTOM_KEY\n", + encoding="utf-8", + ) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ISSUE_3957_CUSTOM_KEY", "from-process-env") + + with profiles.profile_scope_for_detached_worker("work", "test-worker"): + assert os.environ.get("ISSUE_3957_CUSTOM_KEY") is None + assert config._thread_local_env_value("ISSUE_3957_CUSTOM_KEY") == "" + + assert os.environ.get("ISSUE_3957_CUSTOM_KEY") == "from-process-env" + + +def test_account_usage_subprocess_env_blocks_process_default_key(monkeypatch, tmp_path): + """Readonly quota probes must not inherit process-default provider keys.""" + from api.providers import _account_usage_subprocess_env + + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("OPENAI_API_KEY", "from-process-env") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("quota probe"): + env = _account_usage_subprocess_env(work_home, "openai", None) + finally: + profiles.clear_request_profile() + + assert env["HERMES_HOME"] == str(work_home) + assert "OPENAI_API_KEY" not in env + + +def test_active_request_scope_installs_secret_scope(monkeypatch, tmp_path): + """Inside readonly scope, agent.secret_scope sees profile env, not process env.""" + import types + + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("OPENROUTER_API_KEY", "process-default-key") + + # Inject fake agent.secret_scope that records calls + call_log = {} + + def fake_set_secret_scope(scope_dict): + call_log["set_scope"] = dict(scope_dict) + return "fake_token" + + def fake_reset_secret_scope(token): + call_log["reset_called"] = True + + fake_secret_scope = types.ModuleType("agent.secret_scope") + fake_secret_scope.set_secret_scope = fake_set_secret_scope + fake_secret_scope.reset_secret_scope = fake_reset_secret_scope + prev_agent = sys.modules.get("agent") + prev_ss = sys.modules.get("agent.secret_scope") + sys.modules["agent.secret_scope"] = fake_secret_scope + sys.modules["agent"] = types.ModuleType("agent") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("test"): + pass + finally: + profiles.clear_request_profile() + if prev_ss is not None: + sys.modules["agent.secret_scope"] = prev_ss + else: + sys.modules.pop("agent.secret_scope", None) + if prev_agent is not None: + sys.modules["agent"] = prev_agent + else: + sys.modules.pop("agent", None) + profiles._secret_scope_available = None + + # Verify the scope was set with profile env only + assert "set_scope" in call_log + assert "OPENROUTER_API_KEY" not in call_log["set_scope"] + assert "HERMES_HOME" in call_log["set_scope"] + # Verify reset was called + assert call_log.get("reset_called") is True + + +def test_detached_worker_scope_installs_secret_scope(monkeypatch, tmp_path): + """Inside detached worker scope, agent.secret_scope sees profile env, not process env.""" + import threading + import types + + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("OPENROUTER_API_KEY", "process-default-key") + + # Inject fake agent.secret_scope that records calls + call_log = {} + + def fake_set_secret_scope(scope_dict): + call_log["set_scope"] = dict(scope_dict) + return "fake_token" + + def fake_reset_secret_scope(token): + call_log["reset_called"] = True + + fake_secret_scope = types.ModuleType("agent.secret_scope") + fake_secret_scope.set_secret_scope = fake_set_secret_scope + fake_secret_scope.reset_secret_scope = fake_reset_secret_scope + prev_agent = sys.modules.get("agent") + prev_ss = sys.modules.get("agent.secret_scope") + sys.modules["agent.secret_scope"] = fake_secret_scope + sys.modules["agent"] = types.ModuleType("agent") + + result = {"scope_was_set": False} + + def worker_body(): + result["scope_was_set"] = "set_scope" in call_log + + # Capture the profile on the main thread + profiles.set_request_profile("work") + captured_profile = profiles.get_active_profile_name() + try: + with profiles.profile_scope_for_detached_worker(captured_profile): + thread = threading.Thread(target=worker_body) + thread.start() + thread.join() + finally: + profiles.clear_request_profile() + if prev_ss is not None: + sys.modules["agent.secret_scope"] = prev_ss + else: + sys.modules.pop("agent.secret_scope", None) + if prev_agent is not None: + sys.modules["agent"] = prev_agent + else: + sys.modules.pop("agent", None) + profiles._secret_scope_available = None + + # Verify the scope was set with profile env only + assert "set_scope" in call_log + assert "OPENROUTER_API_KEY" not in call_log["set_scope"] + assert "HERMES_HOME" in call_log["set_scope"] + # Verify reset was called + assert call_log.get("reset_called") is True + + +def test_account_usage_subprocess_env_strips_bedrock_keys(monkeypatch, tmp_path): + """Quota probes must not inherit AWS/Bedrock keys when block_process_env_fallback is set.""" + from api.providers import _account_usage_subprocess_env + + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("AWS_ACCESS_KEY_ID", "aws-key-id") + monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "aws-secret") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("quota probe"): + env = _account_usage_subprocess_env(work_home, "bedrock", None) + finally: + profiles.clear_request_profile() + + assert env["HERMES_HOME"] == str(work_home) + assert "AWS_ACCESS_KEY_ID" not in env + assert "AWS_SECRET_ACCESS_KEY" not in env + + +def test_account_usage_subprocess_env_strips_custom_key_env(monkeypatch, tmp_path): + """Quota probes must strip custom provider key_env when block_process_env_fallback is set.""" + from api.providers import _account_usage_subprocess_env + + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + + # Create a config.yaml with a custom provider that has key_env + config_yaml = work_home / "config.yaml" + config_yaml.write_text( + """ +custom_providers: + - key_env: MY_CUSTOM_API_KEY +""" + ) + + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("MY_CUSTOM_API_KEY", "custom-secret") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("quota probe"): + env = _account_usage_subprocess_env(work_home, "openai", None) + finally: + profiles.clear_request_profile() + + assert env["HERMES_HOME"] == str(work_home) + assert "MY_CUSTOM_API_KEY" not in env + + +def test_account_usage_subprocess_env_strips_anthropic_token_aliases(monkeypatch, tmp_path): + """Quota probes must not inherit the process-default Anthropic OAuth/token env + vars (ANTHROPIC_TOKEN / CLAUDE_CODE_OAUTH_TOKEN) for an empty named profile. + + These are agent-runtime credential env vars absent from the WebUI's settable + _PROVIDER_ENV_VAR map, so the strip set must derive them from the agent + registry — otherwise the anthropic quota subprocess resolves them via + resolve_anthropic_token() and leaks the server-process credential (#3961).""" + from api.providers import _account_usage_subprocess_env + + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ANTHROPIC_TOKEN", "process-default-anthropic-token") + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "process-default-oauth-token") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("quota probe"): + env = _account_usage_subprocess_env(work_home, "anthropic", None) + finally: + profiles.clear_request_profile() + + assert env["HERMES_HOME"] == str(work_home) + assert "ANTHROPIC_TOKEN" not in env + assert "CLAUDE_CODE_OAUTH_TOKEN" not in env + + +def test_detached_worker_scope_scrubs_anthropic_token_aliases(monkeypatch, tmp_path): + """Detached/sync model-rebuild scope must scrub the process-default Anthropic + OAuth/token env vars too — verified agent model code can resolve Anthropic + models through raw os.getenv() of these names, so an empty named profile + must not see the server-process token (#3961 detached-worker leak).""" + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ANTHROPIC_TOKEN", "process-default-anthropic-token") + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "process-default-oauth-token") + + with profiles.profile_scope_for_detached_worker("work", "test-worker"): + assert os.environ.get("ANTHROPIC_TOKEN") is None + assert os.environ.get("CLAUDE_CODE_OAUTH_TOKEN") is None + assert config._thread_local_env_value("ANTHROPIC_TOKEN") == "" + assert config._thread_local_env_value("CLAUDE_CODE_OAUTH_TOKEN") == "" + + assert os.environ.get("ANTHROPIC_TOKEN") == "process-default-anthropic-token" + assert os.environ.get("CLAUDE_CODE_OAUTH_TOKEN") == "process-default-oauth-token" + + +def test_account_usage_subprocess_env_strips_non_registry_agent_creds(monkeypatch, tmp_path): + """Quota probes must not inherit process-default credential env vars the agent + resolves via raw os.getenv() but that are NOT in the auth registry — the + generic CUSTOM_API_KEY and the AWS/Bedrock credential family. Otherwise a + custom/AWS-backed provider quota probe leaks the server-process credential + to an empty named profile (#3961 residual leak class).""" + from api.providers import _account_usage_subprocess_env + + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("CUSTOM_API_KEY", "process-default-custom-key") + monkeypatch.setenv("AWS_PROFILE", "process-default-aws-profile") + monkeypatch.setenv("AWS_BEARER_TOKEN_BEDROCK", "process-default-bedrock-token") + monkeypatch.setenv("AWS_CONTAINER_CREDENTIALS_FULL_URI", "http://169.254.170.2/creds") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request_readonly("quota probe"): + env = _account_usage_subprocess_env(work_home, "openai", None) + finally: + profiles.clear_request_profile() + + assert env["HERMES_HOME"] == str(work_home) + assert "CUSTOM_API_KEY" not in env + assert "AWS_PROFILE" not in env + assert "AWS_BEARER_TOKEN_BEDROCK" not in env + assert "AWS_CONTAINER_CREDENTIALS_FULL_URI" not in env + + +def test_detached_worker_scope_scrubs_non_registry_agent_creds(monkeypatch, tmp_path): + """Detached/sync model-rebuild scope must scrub the non-registry agent + credential env vars (CUSTOM_API_KEY, AWS/Bedrock family) too — the agent's + custom-provider and bedrock-adapter paths resolve them via raw os.getenv(), + so an empty named profile must not see the server-process value (#3961).""" + base = tmp_path / ".hermes" + work_home = base / "profiles" / "work" + work_home.mkdir(parents=True) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("CUSTOM_API_KEY", "process-default-custom-key") + monkeypatch.setenv("AWS_PROFILE", "process-default-aws-profile") + monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "process-default-aws-secret") + monkeypatch.setenv("AZURE_CLIENT_SECRET", "process-default-azure-secret") + monkeypatch.setenv("AZURE_FOUNDRY_API_KEY", "process-default-foundry-key") + monkeypatch.setenv("IDENTITY_ENDPOINT", "http://169.254.169.254/msi") + monkeypatch.setenv("MSI_ENDPOINT", "http://169.254.169.254/msi") + + with profiles.profile_scope_for_detached_worker("work", "test-worker"): + assert os.environ.get("CUSTOM_API_KEY") is None + assert os.environ.get("AWS_PROFILE") is None + assert os.environ.get("AWS_SECRET_ACCESS_KEY") is None + assert os.environ.get("AZURE_CLIENT_SECRET") is None + assert os.environ.get("AZURE_FOUNDRY_API_KEY") is None + assert os.environ.get("IDENTITY_ENDPOINT") is None + assert os.environ.get("MSI_ENDPOINT") is None + assert config._thread_local_env_value("CUSTOM_API_KEY") == "" + + assert os.environ.get("CUSTOM_API_KEY") == "process-default-custom-key" + assert os.environ.get("AWS_PROFILE") == "process-default-aws-profile" + assert os.environ.get("AWS_SECRET_ACCESS_KEY") == "process-default-aws-secret" + assert os.environ.get("AZURE_CLIENT_SECRET") == "process-default-azure-secret" + assert os.environ.get("AZURE_FOUNDRY_API_KEY") == "process-default-foundry-key" + assert os.environ.get("IDENTITY_ENDPOINT") == "http://169.254.169.254/msi" + assert os.environ.get("MSI_ENDPOINT") == "http://169.254.169.254/msi" + + +def test_expand_env_vars_does_not_leak_process_env_under_block_scope(monkeypatch): + """Config ${VAR} expansion must not reconstruct a server-process credential + for a profile-scoped readonly/background read (#3961 config-template vector). + + A profile config.yaml of e.g. `api_key: ${ANTHROPIC_TOKEN}` previously + expanded via raw os.environ in _expand_env_vars, so _get_provider_api_key + could rebuild the process token and pass it through even when the scrub + stripped the child env. The expansion now routes through the thread-local + accessor and refuses the process-env fallback when block_process_env_fallback + is set.""" + monkeypatch.setenv("ANTHROPIC_TOKEN", "process-default-anthropic-token") + + # No active scope: normal behavior — expands from the process env. + assert config._expand_env_vars({"api_key": "${ANTHROPIC_TOKEN}"}) == { + "api_key": "process-default-anthropic-token" + } + + # Profile-scoped readonly/background scope with no profile value for the var: + # must NOT fall back to the process env (leaves the reference unexpanded). + prev_block = getattr(config._thread_ctx, "block_process_env_fallback", False) + prev_env = getattr(config._thread_ctx, "env", None) + config._thread_ctx.block_process_env_fallback = True + config._thread_ctx.env = {} + try: + assert config._expand_env_vars({"api_key": "${ANTHROPIC_TOKEN}"}) == { + "api_key": "${ANTHROPIC_TOKEN}" + } + # A value present in the profile's thread-local env IS used (own value). + config._thread_ctx.env = {"ANTHROPIC_TOKEN": "profile-own-token"} + assert config._expand_env_vars({"api_key": "${ANTHROPIC_TOKEN}"}) == { + "api_key": "profile-own-token" + } + finally: + config._thread_ctx.block_process_env_fallback = prev_block + if prev_env is None: + config._thread_ctx.env = {} + else: + config._thread_ctx.env = prev_env +