diff --git a/CHANGELOG.md b/CHANGELOG.md index 07dee9f0..56b49a2b 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/api/config.py b/api/config.py index 7402009f..e4a0b719 100644 --- a/api/config.py +++ b/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 - # only, so without this, tests (and live provider key edits) see a - # stale CredentialPool from a prior auth_store payload — the test_ - # credential_pool_providers suite was hitting this directly. + # Clear the credential pool cache too (all profiles). Without this, + # tests (and live provider key edits) see a stale CredentialPool from a + # prior auth_store payload — the test_credential_pool_providers suite was + # 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) - _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) 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). - _CREDENTIAL_POOL_CACHE.pop(provider_id, None) - _CREDENTIAL_POOL_CACHE.pop(_resolve_provider_alias(provider_id), None) + # may be called with either, and both paths cache under their own key), + # scoped to the active profile's cache key. + _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 - _cached = _CREDENTIAL_POOL_CACHE.get(_pid) + # Check credential pool cache first (profile-scoped key + # 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: diff --git a/api/routes.py b/api/routes.py index 4b6fdb4b..2b96bb54 100644 --- a/api/routes.py +++ b/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: diff --git a/tests/test_credential_pool_profile_scoping.py b/tests/test_credential_pool_profile_scoping.py new file mode 100644 index 00000000..8fdc8891 --- /dev/null +++ b/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()