fix(auth): look up keyed custom providers by durable pool slug
hermes auth add stores providers.<key> credentials under the config slug, but runtime only tried custom:<display-name> and then sent the no-key-required placeholder. Try the slug first, keep the legacy namespace as fallback, and thread provider_key/key_env through named custom resolution.
This commit is contained in:
@@ -7025,6 +7025,40 @@ def resolve_provider_client(
|
||||
custom_key = build_command_token_provider(
|
||||
custom_key_cmd, custom_entry.get("name") or provider
|
||||
) or custom_key
|
||||
if not custom_key:
|
||||
try:
|
||||
from agent.credential_pool import (
|
||||
custom_provider_pool_key_candidates,
|
||||
load_pool,
|
||||
)
|
||||
|
||||
pool_name = (
|
||||
custom_entry.get("provider_key")
|
||||
or custom_entry.get("name")
|
||||
or provider
|
||||
)
|
||||
for pool_key in custom_provider_pool_key_candidates(
|
||||
custom_base, pool_name
|
||||
):
|
||||
try:
|
||||
pool = load_pool(pool_key)
|
||||
except Exception:
|
||||
continue
|
||||
if not pool.has_credentials():
|
||||
continue
|
||||
pool_entry = pool.select()
|
||||
if pool_entry is None:
|
||||
continue
|
||||
pool_api_key = (
|
||||
getattr(pool_entry, "runtime_api_key", None)
|
||||
or getattr(pool_entry, "access_token", "")
|
||||
or ""
|
||||
)
|
||||
if str(pool_api_key).strip():
|
||||
custom_key = str(pool_api_key).strip()
|
||||
break
|
||||
except Exception:
|
||||
pass
|
||||
custom_key = custom_key or "no-key-required"
|
||||
if custom_key == "no-key-required":
|
||||
logger.warning(
|
||||
|
||||
@@ -489,34 +489,83 @@ def _iter_custom_providers(config: Optional[dict] = None):
|
||||
yield _normalize_custom_pool_name(name), entry
|
||||
|
||||
|
||||
def get_custom_provider_pool_key(base_url: Optional[str], provider_name: Optional[str] = None) -> Optional[str]:
|
||||
"""Look up the custom_providers list in config.yaml and return 'custom:<name>' for a matching base_url.
|
||||
def _custom_entry_name_aliases(norm_name: str, entry: Dict[str, Any]) -> set:
|
||||
aliases = {norm_name}
|
||||
provider_key = _normalize_custom_pool_name(str(entry.get("provider_key") or ""))
|
||||
if provider_key:
|
||||
aliases.add(provider_key)
|
||||
return aliases
|
||||
|
||||
When provider_name is given, prefer matching by name first (solving the case where
|
||||
multiple custom providers share the same base_url but have different API keys).
|
||||
Falls back to base_url matching when no name match is found.
|
||||
|
||||
Returns None if no match is found.
|
||||
def _requested_custom_name_aliases(provider_name: str) -> set:
|
||||
normalized = _normalize_custom_pool_name(provider_name)
|
||||
aliases = {normalized} if normalized else set()
|
||||
if normalized.startswith(CUSTOM_POOL_PREFIX):
|
||||
suffix = _normalize_custom_pool_name(normalized[len(CUSTOM_POOL_PREFIX):])
|
||||
if suffix:
|
||||
aliases.add(suffix)
|
||||
return aliases
|
||||
|
||||
|
||||
def _pool_keys_for_custom_entry(norm_name: str, entry: Dict[str, Any]) -> List[str]:
|
||||
"""Durable ``providers.<key>`` slug first, then legacy ``custom:<name>``."""
|
||||
keys: List[str] = []
|
||||
seen = set()
|
||||
|
||||
def _add(key: str) -> None:
|
||||
normalized = str(key or "").strip().lower()
|
||||
if normalized and normalized not in seen:
|
||||
seen.add(normalized)
|
||||
keys.append(normalized)
|
||||
|
||||
provider_key = _normalize_custom_pool_name(str(entry.get("provider_key") or ""))
|
||||
if provider_key:
|
||||
_add(provider_key)
|
||||
if norm_name:
|
||||
_add(f"{CUSTOM_POOL_PREFIX}{norm_name}")
|
||||
return keys
|
||||
|
||||
|
||||
def custom_provider_pool_key_candidates(
|
||||
base_url: Optional[str],
|
||||
provider_name: Optional[str] = None,
|
||||
) -> List[str]:
|
||||
"""Return pool keys to try for a custom endpoint.
|
||||
|
||||
``hermes auth add <key>`` stores new-style ``providers.<key>`` credentials
|
||||
under the durable config slug (``b-ai``). Older rows and legacy
|
||||
``custom_providers:`` entries still live under ``custom:<display-name>``.
|
||||
Try the slug first, then the legacy namespace, so a populated pool is not
|
||||
skipped in favour of the ``no-key-required`` placeholder.
|
||||
"""
|
||||
if not base_url:
|
||||
return None
|
||||
return []
|
||||
normalized_url = base_url.strip().rstrip("/")
|
||||
requested_aliases = (
|
||||
_requested_custom_name_aliases(provider_name) if provider_name else set()
|
||||
)
|
||||
|
||||
# When a provider name is given, try to match by name first.
|
||||
# This fixes the P1 bug where two custom providers sharing the same
|
||||
# base_url always resolve to the first one's credentials.
|
||||
if provider_name:
|
||||
normalized_name = _normalize_custom_pool_name(provider_name)
|
||||
if requested_aliases:
|
||||
for norm_name, entry in _iter_custom_providers():
|
||||
if norm_name == normalized_name:
|
||||
return f"{CUSTOM_POOL_PREFIX}{norm_name}"
|
||||
if requested_aliases & _custom_entry_name_aliases(norm_name, entry):
|
||||
return _pool_keys_for_custom_entry(norm_name, entry)
|
||||
|
||||
# Fall back to base_url matching (original behavior)
|
||||
for norm_name, entry in _iter_custom_providers():
|
||||
entry_url = str(entry.get("base_url") or "").strip().rstrip("/")
|
||||
if entry_url and entry_url == normalized_url:
|
||||
return f"{CUSTOM_POOL_PREFIX}{norm_name}"
|
||||
return None
|
||||
return _pool_keys_for_custom_entry(norm_name, entry)
|
||||
return []
|
||||
|
||||
|
||||
def get_custom_provider_pool_key(base_url: Optional[str], provider_name: Optional[str] = None) -> Optional[str]:
|
||||
"""Look up the matching custom provider and return its preferred pool key.
|
||||
|
||||
Prefers the durable ``providers.<key>`` slug when present, otherwise
|
||||
``custom:<normalized-name>``. When provider_name is given, match by name
|
||||
first so two custom providers sharing a base_url keep separate keys.
|
||||
"""
|
||||
candidates = custom_provider_pool_key_candidates(base_url, provider_name)
|
||||
return candidates[0] if candidates else None
|
||||
|
||||
|
||||
def list_custom_pool_providers() -> List[str]:
|
||||
@@ -557,6 +606,36 @@ def get_pool_strategy(provider: str) -> str:
|
||||
return STRATEGY_FILL_FIRST
|
||||
|
||||
|
||||
def _keyed_custom_pool_matches(
|
||||
pool_provider: str,
|
||||
provider_norm: str,
|
||||
base_url: Optional[str],
|
||||
) -> bool:
|
||||
"""Match a durable ``providers.<key>`` pool against runtime identities."""
|
||||
runtime_url = str(base_url or "").strip().rstrip("/")
|
||||
if not runtime_url:
|
||||
return False
|
||||
try:
|
||||
for normalized_name, entry in _iter_custom_providers():
|
||||
provider_key = _normalize_custom_pool_name(
|
||||
str(entry.get("provider_key") or "")
|
||||
)
|
||||
if provider_key != pool_provider:
|
||||
continue
|
||||
aliases = _custom_entry_name_aliases(normalized_name, entry)
|
||||
aliases.add(f"{CUSTOM_POOL_PREFIX}{normalized_name}")
|
||||
if provider_key:
|
||||
aliases.add(f"{CUSTOM_POOL_PREFIX}{provider_key}")
|
||||
configured_url = str(entry.get("base_url") or "").strip().rstrip("/")
|
||||
if provider_norm == "custom":
|
||||
return runtime_url == configured_url
|
||||
runtime_aliases = _requested_custom_name_aliases(provider_norm)
|
||||
return bool(runtime_aliases & aliases) and runtime_url == configured_url
|
||||
except Exception:
|
||||
return False
|
||||
return False
|
||||
|
||||
|
||||
def credential_pool_matches_provider(
|
||||
pool_or_provider: Any,
|
||||
provider: Optional[str],
|
||||
@@ -567,10 +646,12 @@ def credential_pool_matches_provider(
|
||||
|
||||
Named custom endpoints may use three identities: the live agent can retain
|
||||
the configured name/provider key, newer runtime paths normalize it to
|
||||
``custom``, and the pool is keyed ``custom:<name>``. Accept those aliases
|
||||
only when the runtime endpoint belongs to the same configured custom
|
||||
provider. Empty identities fail closed. Legacy pool adapters without a
|
||||
``provider`` attribute remain compatible; production pools are scoped.
|
||||
``custom``, and the pool may be keyed either as the durable
|
||||
``providers.<key>`` slug or as legacy ``custom:<name>``. Accept those
|
||||
aliases only when the runtime endpoint belongs to the same configured
|
||||
custom provider. Empty identities fail closed. Legacy pool adapters
|
||||
without a ``provider`` attribute remain compatible; production pools
|
||||
are scoped.
|
||||
"""
|
||||
raw_pool_provider = getattr(pool_or_provider, "provider", None)
|
||||
if raw_pool_provider is None:
|
||||
@@ -586,13 +667,18 @@ def credential_pool_matches_provider(
|
||||
if not pool_provider or not provider_norm:
|
||||
return False
|
||||
if not pool_provider.startswith(CUSTOM_POOL_PREFIX):
|
||||
return pool_provider == provider_norm
|
||||
if pool_provider == provider_norm:
|
||||
return True
|
||||
return _keyed_custom_pool_matches(pool_provider, provider_norm, base_url)
|
||||
if provider_norm == "custom":
|
||||
try:
|
||||
matched_pool = get_custom_provider_pool_key(base_url or "")
|
||||
if str(matched_pool or "").strip().lower() == pool_provider:
|
||||
return True
|
||||
candidates = custom_provider_pool_key_candidates(base_url or "")
|
||||
except Exception:
|
||||
return False
|
||||
return str(matched_pool or "").strip().lower() == pool_provider
|
||||
return pool_provider in {str(key).strip().lower() for key in candidates}
|
||||
|
||||
runtime_url = str(base_url or "").strip().rstrip("/")
|
||||
if not runtime_url:
|
||||
@@ -625,9 +711,10 @@ def credential_pool_matches_provider(
|
||||
def resolve_runtime_pool_key(provider: Optional[str], base_url: Optional[str]) -> str:
|
||||
"""Resolve the credential-pool key for a runtime provider identity.
|
||||
|
||||
Named custom runtimes retain their configured alias while their pool is
|
||||
stored under ``custom:<name>``. Return that scoped key only when the
|
||||
canonical provider/endpoint boundary accepts it; otherwise preserve the
|
||||
Named custom runtimes retain their configured alias while their pool may
|
||||
be stored under the durable ``providers.<key>`` slug or legacy
|
||||
``custom:<name>``. Return that scoped key only when the canonical
|
||||
provider/endpoint boundary accepts it; otherwise preserve the
|
||||
normalized runtime identity so callers fail closed.
|
||||
"""
|
||||
provider_norm = str(provider or "").strip().lower()
|
||||
@@ -644,18 +731,19 @@ def resolve_runtime_pool_key(provider: Optional[str], base_url: Optional[str]) -
|
||||
):
|
||||
return str(candidate).strip().lower()
|
||||
else:
|
||||
# Named and exact custom runtimes are keyed by provider identity,
|
||||
# while auth storage remains keyed by display name. Search the
|
||||
# configured candidates by identity before considering endpoint;
|
||||
# this prevents a sibling sharing the URL from lending its pool.
|
||||
for normalized_name, _entry in _iter_custom_providers():
|
||||
candidate = f"{CUSTOM_POOL_PREFIX}{normalized_name}"
|
||||
if credential_pool_matches_provider(
|
||||
candidate,
|
||||
provider_norm,
|
||||
base_url=base_url,
|
||||
):
|
||||
return candidate
|
||||
# Named and exact custom runtimes are keyed by provider identity.
|
||||
# Auth storage prefers the durable providers.<key> slug, with
|
||||
# legacy custom:<display-name> as fallback. Search configured
|
||||
# candidates by identity before considering endpoint so a sibling
|
||||
# sharing the URL cannot lend its pool.
|
||||
for normalized_name, entry in _iter_custom_providers():
|
||||
for candidate in _pool_keys_for_custom_entry(normalized_name, entry):
|
||||
if credential_pool_matches_provider(
|
||||
candidate,
|
||||
provider_norm,
|
||||
base_url=base_url,
|
||||
):
|
||||
return candidate
|
||||
except Exception:
|
||||
pass
|
||||
return provider_norm
|
||||
|
||||
@@ -15,6 +15,7 @@ from agent.credential_pool import (
|
||||
CredentialPool,
|
||||
PooledCredential,
|
||||
credential_pool_matches_provider,
|
||||
custom_provider_pool_key_candidates,
|
||||
get_custom_provider_pool_key,
|
||||
load_pool,
|
||||
)
|
||||
@@ -670,42 +671,59 @@ def _try_resolve_from_custom_pool(
|
||||
provider_name: Optional[str] = None,
|
||||
) -> Optional[Dict[str, Any]]:
|
||||
"""Check if a credential pool exists for a custom endpoint and return a runtime dict if so."""
|
||||
pool_key = get_custom_provider_pool_key(base_url, provider_name=provider_name)
|
||||
if not pool_key:
|
||||
return None
|
||||
candidates: list[str] = []
|
||||
seen = set()
|
||||
|
||||
def _add(key: Optional[str]) -> None:
|
||||
normalized = str(key or "").strip().lower()
|
||||
if normalized and normalized not in seen:
|
||||
seen.add(normalized)
|
||||
candidates.append(normalized)
|
||||
|
||||
try:
|
||||
pool = load_pool(pool_key)
|
||||
if not pool.has_credentials():
|
||||
return None
|
||||
entry = pool.select()
|
||||
if entry is None:
|
||||
return None
|
||||
pool_api_key = getattr(entry, "runtime_api_key", None) or getattr(entry, "access_token", "")
|
||||
if not pool_api_key:
|
||||
return None
|
||||
if not has_usable_secret(pool_api_key) and _loopback_hostname(base_url_hostname(base_url)):
|
||||
# Legacy configs commonly used short/placeholder keys ('123',
|
||||
# 'm', ...) for local no-auth services like Ollama -- fine for
|
||||
# the endpoint itself, but has_usable_secret's 4-char floor
|
||||
# (added after these configs were written) now rejects them
|
||||
# here with no migration path. Every OTHER resolution path in
|
||||
# this file already substitutes "no-key-required" for a
|
||||
# loopback endpoint with no usable secret (the config-based
|
||||
# custom_providers fallback a few hundred lines below, and the
|
||||
# "actual" provider's local-offline exemption further down) --
|
||||
# this pool path was the one gap (issue #86864).
|
||||
pool_api_key = "no-key-required"
|
||||
return {
|
||||
"provider": provider_label,
|
||||
"api_mode": api_mode_override or _detect_api_mode_for_url(base_url) or "chat_completions",
|
||||
"base_url": base_url,
|
||||
"api_key": pool_api_key,
|
||||
"source": f"pool:{pool_key}",
|
||||
"credential_pool": pool,
|
||||
}
|
||||
for key in custom_provider_pool_key_candidates(base_url, provider_name):
|
||||
_add(key)
|
||||
except Exception:
|
||||
pass
|
||||
_add(get_custom_provider_pool_key(base_url, provider_name=provider_name))
|
||||
if not candidates:
|
||||
return None
|
||||
|
||||
for pool_key in candidates:
|
||||
try:
|
||||
pool = load_pool(pool_key)
|
||||
if not pool.has_credentials():
|
||||
continue
|
||||
entry = pool.select()
|
||||
if entry is None:
|
||||
continue
|
||||
pool_api_key = getattr(entry, "runtime_api_key", None) or getattr(entry, "access_token", "")
|
||||
if not pool_api_key:
|
||||
continue
|
||||
if not has_usable_secret(pool_api_key) and _loopback_hostname(base_url_hostname(base_url)):
|
||||
# Legacy configs commonly used short/placeholder keys ('123',
|
||||
# 'm', ...) for local no-auth services like Ollama -- fine for
|
||||
# the endpoint itself, but has_usable_secret's 4-char floor
|
||||
# (added after these configs were written) now rejects them
|
||||
# here with no migration path. Every OTHER resolution path in
|
||||
# this file already substitutes "no-key-required" for a
|
||||
# loopback endpoint with no usable secret (the config-based
|
||||
# custom_providers fallback a few hundred lines below, and the
|
||||
# "actual" provider's local-offline exemption further down) --
|
||||
# this pool path was the one gap (issue #86864).
|
||||
pool_api_key = "no-key-required"
|
||||
return {
|
||||
"provider": provider_label,
|
||||
"api_mode": api_mode_override or _detect_api_mode_for_url(base_url) or "chat_completions",
|
||||
"base_url": base_url,
|
||||
"api_key": pool_api_key,
|
||||
"source": f"pool:{pool_key}",
|
||||
"credential_pool": pool,
|
||||
}
|
||||
except Exception:
|
||||
continue
|
||||
return None
|
||||
|
||||
|
||||
def _filter_capabilities(value: Any) -> Dict[str, bool]:
|
||||
"""Return the string-keyed boolean capabilities accepted at runtime."""
|
||||
@@ -834,6 +852,11 @@ def _get_named_custom_provider(requested_provider: str) -> Optional[Dict[str, An
|
||||
"api_key": resolved_api_key,
|
||||
"model": entry.get("default_model", ""),
|
||||
}
|
||||
provider_key = str(ep_name or "").strip()
|
||||
if provider_key:
|
||||
result["provider_key"] = provider_key
|
||||
if key_env:
|
||||
result["key_env"] = key_env
|
||||
extra_body = entry.get("extra_body")
|
||||
if isinstance(extra_body, dict):
|
||||
result["extra_body"] = dict(extra_body)
|
||||
@@ -1267,7 +1290,12 @@ def _resolve_named_custom_runtime(
|
||||
return None
|
||||
|
||||
# Check if a credential pool exists for this custom endpoint
|
||||
pool_result = _try_resolve_from_custom_pool(base_url, "custom", custom_provider.get("api_mode"), provider_name=custom_provider.get("name"))
|
||||
pool_result = _try_resolve_from_custom_pool(
|
||||
base_url,
|
||||
"custom",
|
||||
custom_provider.get("api_mode"),
|
||||
provider_name=custom_provider.get("provider_key") or custom_provider.get("name"),
|
||||
)
|
||||
if pool_result:
|
||||
# Propagate the model name even when using pooled credentials —
|
||||
# the pool doesn't know about the custom_providers model field.
|
||||
|
||||
@@ -89,14 +89,14 @@ def test_runtime_pool_key_resolves_all_custom_runtime_identities():
|
||||
)
|
||||
]
|
||||
with patch("agent.credential_pool._iter_custom_providers", return_value=configured):
|
||||
assert resolve_runtime_pool_key("custom", endpoint) == "custom:sibling-display"
|
||||
assert resolve_runtime_pool_key("custom", endpoint) == "sibling-provider"
|
||||
assert (
|
||||
resolve_runtime_pool_key("gemini-no-filter", endpoint)
|
||||
== "custom:gemini-display"
|
||||
== "gemini-no-filter"
|
||||
)
|
||||
assert (
|
||||
resolve_runtime_pool_key("custom:gemini-no-filter", endpoint)
|
||||
== "custom:gemini-display"
|
||||
== "gemini-no-filter"
|
||||
)
|
||||
assert (
|
||||
resolve_runtime_pool_key(
|
||||
@@ -127,11 +127,11 @@ def test_runtime_pool_key_resolves_modern_provider_in_mixed_config():
|
||||
with patch("agent.credential_pool._load_config_safe", return_value=config):
|
||||
assert (
|
||||
resolve_runtime_pool_key("gemini-no-filter", endpoint)
|
||||
== "custom:gemini-display"
|
||||
== "gemini-no-filter"
|
||||
)
|
||||
assert (
|
||||
resolve_runtime_pool_key("custom:gemini-no-filter", endpoint)
|
||||
== "custom:gemini-display"
|
||||
== "gemini-no-filter"
|
||||
)
|
||||
assert (
|
||||
resolve_runtime_pool_key(
|
||||
|
||||
@@ -517,7 +517,7 @@ class TestRestorePrimaryRuntime:
|
||||
|
||||
assert result is True
|
||||
assert agent._credential_pool is primary_pool
|
||||
load_pool.assert_called_once_with("custom:gemini-display")
|
||||
load_pool.assert_called_once_with("gemini-no-filter")
|
||||
agent._swap_credential.assert_called_once_with(primary_pool.select.return_value)
|
||||
|
||||
def test_restore_named_custom_pool_wrong_endpoint_fails_closed(self):
|
||||
|
||||
Reference in New Issue
Block a user