fix review: profile-scope _CREDENTIAL_POOL_CACHE + getattr-safe runtime_api_key (#4247)

Codex gate caught a cross-profile leak: _CREDENTIAL_POOL_CACHE was keyed
by provider id alone, so a custom provider configured under profile A made
profile B report has_key=True from a stale cache hit (then 401 at request).
Key the cache by (active-profile auth-store path, provider id) at all
get/set/evict sites via _credential_pool_profile_tag(). Also make the two
new bare _entry.runtime_api_key reads getattr-safe (OAuth-only entries lack
the attr), matching the guard the author already applied at providers.py.
Adds a cross-profile isolation regression test.

Co-authored-by: akrhin <akrhin@users.noreply.github.com>

CHANGELOG.md

@@ -7,7 +7,7 @@
 
 ### Fixed
 
-- **Custom and aliased providers with credential-pool keys now show as configured (#4247).** Providers whose API keys live in the credential pool (added via `hermes auth add`) rather than env vars were reported unconfigured in the WebUI, so their models didn't appear. Provider detection now also consults the credential pool via `_has_explicit_pool_credentials()`, which filters the ambient `gh auth token` entry (so Copilot no longer falsely shows configured on any machine with `gh` installed) and is cache-routed to avoid the cold per-provider pool-load latency. Provider aliases (`glm`→`zai`, `github`→`copilot`, etc.) are resolved before loading the pool at every retrieval site, so detection and key-retrieval always read the same pool. Thanks @akrhin.
+- **Custom and aliased providers with credential-pool keys now show as configured (#4247).** Providers whose API keys live in the credential pool (added via `hermes auth add`) rather than env vars were reported unconfigured in the WebUI, so their models didn't appear. Provider detection now also consults the credential pool via `_has_explicit_pool_credentials()`, which filters the ambient `gh auth token` entry (so Copilot no longer falsely shows configured on any machine with `gh` installed) and is cache-routed to avoid the cold per-provider pool-load latency. Provider aliases (`glm`→`zai`, `github`→`copilot`, etc.) are resolved before loading the pool at every retrieval site, so detection and key-retrieval always read the same pool. The credential-pool cache is also now scoped to the active profile, so a custom provider configured in one profile no longer shows as configured in another profile that lacks the key. Thanks @akrhin.
 
 ## [v0.51.460] — 2026-06-16 — Release PU (custom voice providers recognized)
 

api/config.py

@@ -4036,7 +4036,24 @@ def _static_models_catalog_without_live_probes() -> dict:
 # session is expensive (~10s for zai due to endpoint probing).  The credential pool
 # only changes when the user adds/removes credentials, which is rare; a 24h TTL
 # is plenty safe and ensures get_available_models() cold paths are fast.
-_CREDENTIAL_POOL_CACHE: dict[str, tuple[float, "CredentialPool"]] = {}  # noqa: F821  forward-ref string annotation, resolved at runtime  # pid -> (ts, pool)
+_CREDENTIAL_POOL_CACHE: dict[tuple[str, str], tuple[float, "CredentialPool"]] = {}  # noqa: F821  forward-ref string annotation, resolved at runtime  # (profile_tag, pid) -> (ts, pool)
+
+
+def _credential_pool_profile_tag() -> str:
+    """Active-profile identity for the credential-pool cache key.
+
+    The credential pool is per-Hermes-profile (it lives in that profile's
+    auth.json). Keying the process-global cache by provider id ALONE lets a
+    pool loaded under profile A satisfy a lookup under profile B in the same
+    server process — so a custom provider configured only in A would falsely
+    report configured in B (and then 401 at request time). Scoping every
+    cache key by the active profile's auth-store path keeps pools from
+    crossing profile boundaries.
+    """
+    try:
+        return str(_get_auth_store_path())
+    except Exception:
+        return ""
 
 
 def _has_explicit_pool_credentials(provider_id: str) -> bool:
@@ -4051,18 +4068,19 @@ def _has_explicit_pool_credentials(provider_id: str) -> bool:
         from agent.credential_pool import load_pool as _load_pool
 
         _pid = _resolve_provider_alias(provider_id)
-        _cached = _CREDENTIAL_POOL_CACHE.get(_pid)
+        _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[_pid] = (time.time(), _cp_pool)
+                _CREDENTIAL_POOL_CACHE[_ck] = (time.time(), _cp_pool)
                 _all_entries = _cp_pool.entries()
         else:
             _cp_pool = _load_pool(_pid)
-            _CREDENTIAL_POOL_CACHE[_pid] = (time.time(), _cp_pool)
+            _CREDENTIAL_POOL_CACHE[_ck] = (time.time(), _cp_pool)
             _all_entries = _cp_pool.entries()
 
         return any(
@@ -4563,10 +4581,10 @@ def invalidate_models_cache():
         _available_models_cache_source_fingerprint = None
         _cache_build_in_progress = False
         _cache_build_cv.notify_all()
-        # Clear the credential pool cache too. The cache key is provider_id
+        # Clear the credential pool cache too (all profiles). Without this,
-        # only, so without this, tests (and live provider key edits) see a
+        # tests (and live provider key edits) see a stale CredentialPool from a
-        # stale CredentialPool from a prior auth_store payload — the test_
+        # prior auth_store payload — the test_credential_pool_providers suite was
-        # credential_pool_providers suite was hitting this directly.
+        # hitting this directly. A full reset is intentionally profile-wide.
         _CREDENTIAL_POOL_CACHE.clear()
     # Also delete the disk cache so the next cold build starts fresh.
     # Disk delete is outside the lock — file I/O shouldn't block other readers.
@@ -4587,8 +4605,9 @@ def invalidate_credential_pool_cache(provider_id: str):
     """
     global _CREDENTIAL_POOL_CACHE
     with _available_models_cache_lock:
-        _CREDENTIAL_POOL_CACHE.pop(provider_id, None)
+        _cp_tag = _credential_pool_profile_tag()
-        _CREDENTIAL_POOL_CACHE.pop(_resolve_provider_alias(provider_id), None)
+        _CREDENTIAL_POOL_CACHE.pop((_cp_tag, provider_id), None)
+        _CREDENTIAL_POOL_CACHE.pop((_cp_tag, _resolve_provider_alias(provider_id)), None)
     try:
         # api.providers imports from api.config; keep this lazy to avoid
         # import-cycle/module-initialization issues.
@@ -4620,9 +4639,11 @@ def invalidate_provider_models_cache(provider_id: str):
         _provider_models_invalidated_ts[provider_id] = time.time()
         # Also evict the credential pool so the next cold path re-loads it.
         # Must evict both the original key and its canonical form (load_pool
-        # may be called with either, and both paths cache under their own key).
+        # may be called with either, and both paths cache under their own key),
-        _CREDENTIAL_POOL_CACHE.pop(provider_id, None)
+        # scoped to the active profile's cache key.
-        _CREDENTIAL_POOL_CACHE.pop(_resolve_provider_alias(provider_id), None)
+        _cp_tag = _credential_pool_profile_tag()
+        _CREDENTIAL_POOL_CACHE.pop((_cp_tag, provider_id), None)
+        _CREDENTIAL_POOL_CACHE.pop((_cp_tag, _resolve_provider_alias(provider_id)), None)
     _delete_models_cache_on_disk()
 
 
@@ -4982,8 +5003,10 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
                     for _pid in list(_pool.keys()):
                         try:
                             _canonical_pid = _resolve_provider_alias(str(_pid))
-                            # Check credential pool cache first
+                            # Check credential pool cache first (profile-scoped key
-                            _cached = _CREDENTIAL_POOL_CACHE.get(_pid)
+                            # so a pool loaded under another profile can't leak in).
+                            _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:
@@ -4991,12 +5014,12 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
                                 else:
                                     _lp_t0 = time.monotonic()
                                     _cp_pool = _load_pool(_pid)
-                                    _CREDENTIAL_POOL_CACHE[_pid] = (time.time(), _cp_pool)
+                                    _CREDENTIAL_POOL_CACHE[_ck] = (time.time(), _cp_pool)
                                     _all_entries = _cp_pool.entries()
                             else:
                                 _lp_t0 = time.monotonic()
                                 _cp_pool = _load_pool(_pid)
-                                _CREDENTIAL_POOL_CACHE[_pid] = (time.time(), _cp_pool)
+                                _CREDENTIAL_POOL_CACHE[_ck] = (time.time(), _cp_pool)
                                 _all_entries = _cp_pool.entries()
                             _explicit = [
                                 e for e in _all_entries
@@ -5481,7 +5504,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
                                 _entry = _pool.select()
                                 if _entry:
                                     if not _cp_api_key:
-                                        _cp_api_key = _entry.runtime_api_key
+                                        _cp_api_key = getattr(_entry, "runtime_api_key", "") or ""
                                     if not _cp_base_url:
                                         _cp_base_url = str(getattr(_entry, "base_url", "") or "").strip()
                     except ImportError:

api/routes.py

@@ -12441,7 +12441,7 @@ def _handle_live_models(handler, parsed):
                                 _lm_entry = _lm_pool.select()
                                 if _lm_entry:
                                     if not _api_key:
-                                        _api_key = _lm_entry.runtime_api_key
+                                        _api_key = getattr(_lm_entry, "runtime_api_key", "") or ""
                                     if not _base_url:
                                         _base_url = str(getattr(_lm_entry, "base_url", "") or "").strip()
                     except ImportError:

tests/test_credential_pool_profile_scoping.py

@@ -0,0 +1,83 @@
+"""Regression test: the credential-pool detection cache must be profile-scoped.
+
+Guards the cross-profile leak Codex caught on #4247 — _CREDENTIAL_POOL_CACHE
+was keyed by provider id alone, so a custom provider configured under profile A
+made profile B (no pool entry) report has_key=True from a stale cache hit.
+The cache key now includes the active profile's auth-store path.
+
+Note: `_has_explicit_pool_credentials` does `from agent.credential_pool import
+load_pool` at call time. The bundled `agent` package is importable locally but
+NOT in CI's environment, so the test injects a stub `agent.credential_pool`
+module into sys.modules rather than importing/monkeypatching the real one.
+"""
+import sys
+import types
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+import api.config as config  # noqa: E402
+
+
+class _FakeEntry:
+    def __init__(self, source="config_yaml", label="custom:bothub", key_source="config_yaml"):
+        self.source = source
+        self.label = label
+        self.key_source = key_source
+
+
+class _FakePool:
+    def __init__(self, entries):
+        self._entries = entries
+
+    def entries(self):
+        return list(self._entries)
+
+
+def test_has_explicit_pool_credentials_is_profile_scoped(monkeypatch):
+    """A pool loaded under profile A must not satisfy a lookup under profile B."""
+    config._CREDENTIAL_POOL_CACHE.clear()
+
+    # Profile A's auth store has a custom:bothub pool entry; profile B has none.
+    profile_pools = {
+        "/tmp/profileA/auth.json": _FakePool([_FakeEntry()]),
+        "/tmp/profileB/auth.json": _FakePool([]),
+    }
+    active = {"path": "/tmp/profileA/auth.json"}
+
+    monkeypatch.setattr(config, "_get_auth_store_path", lambda: active["path"])
+    monkeypatch.setattr(config, "_resolve_provider_alias", lambda p: p)
+
+    # Inject a stub `agent.credential_pool` so the in-function import resolves
+    # here regardless of whether the real bundled agent is installed (it is not
+    # on CI). Build a stub `agent` parent package too if needed.
+    def _fake_load_pool(provider):
+        return profile_pools[active["path"]]
+
+    fake_cp = types.ModuleType("agent.credential_pool")
+    fake_cp.load_pool = _fake_load_pool
+    fake_agent = sys.modules.get("agent")
+    created_agent = False
+    if fake_agent is None:
+        fake_agent = types.ModuleType("agent")
+        fake_agent.__path__ = []  # mark as package
+        monkeypatch.setitem(sys.modules, "agent", fake_agent)
+        created_agent = True
+    monkeypatch.setitem(sys.modules, "agent.credential_pool", fake_cp)
+    # If a real `agent` package exists, temporarily point its attribute too.
+    if not created_agent:
+        monkeypatch.setattr(fake_agent, "credential_pool", fake_cp, raising=False)
+
+    # Under profile A: custom:bothub IS configured (real entry, not ambient gh_cli).
+    active["path"] = "/tmp/profileA/auth.json"
+    assert config._has_explicit_pool_credentials("custom:bothub") is True
+
+    # Switch to profile B (no pool entry). It must NOT inherit A's cached pool.
+    active["path"] = "/tmp/profileB/auth.json"
+    assert config._has_explicit_pool_credentials("custom:bothub") is False
+
+    # Back to A — still configured (its own cache key survives).
+    active["path"] = "/tmp/profileA/auth.json"
+    assert config._has_explicit_pool_credentials("custom:bothub") is True
+
+    config._CREDENTIAL_POOL_CACHE.clear()