mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-08 03:01:47 +00:00
fix(credential_pool): resolve key mix-up when custom providers share base_url
When multiple custom_providers share the same base_url but have different API keys, get_custom_provider_pool_key() always returned the first match, causing wrong-key unauthorized errors. Add provider_name parameter to prefer exact name matches over base_url-only matching, with fallback for backward compatibility. Fixes #19083
This commit is contained in:
parent
3c8154e62c
commit
e38ea38079
3 changed files with 59 additions and 3 deletions
|
|
@ -305,14 +305,29 @@ def _iter_custom_providers(config: Optional[dict] = None):
|
|||
yield _normalize_custom_pool_name(name), entry
|
||||
|
||||
|
||||
def get_custom_provider_pool_key(base_url: str) -> Optional[str]:
|
||||
def get_custom_provider_pool_key(base_url: 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.
|
||||
|
||||
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.
|
||||
"""
|
||||
if not base_url:
|
||||
return None
|
||||
normalized_url = base_url.strip().rstrip("/")
|
||||
|
||||
# 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)
|
||||
for norm_name, entry in _iter_custom_providers():
|
||||
if norm_name == normalized_name:
|
||||
return f"{CUSTOM_POOL_PREFIX}{norm_name}"
|
||||
|
||||
# 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:
|
||||
|
|
|
|||
|
|
@ -319,9 +319,10 @@ def _try_resolve_from_custom_pool(
|
|||
base_url: str,
|
||||
provider_label: str,
|
||||
api_mode_override: Optional[str] = None,
|
||||
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)
|
||||
pool_key = get_custom_provider_pool_key(base_url, provider_name=provider_name)
|
||||
if not pool_key:
|
||||
return None
|
||||
try:
|
||||
|
|
@ -521,7 +522,7 @@ 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"))
|
||||
pool_result = _try_resolve_from_custom_pool(base_url, "custom", custom_provider.get("api_mode"), provider_name=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.
|
||||
|
|
@ -640,8 +641,11 @@ def _resolve_openrouter_runtime(
|
|||
|
||||
# For custom endpoints, check if a credential pool exists
|
||||
if effective_provider == "custom" and base_url:
|
||||
# Pass requested_provider so pool lookup prefers name match over base_url,
|
||||
# fixing credential mix-ups when multiple custom providers share a base_url.
|
||||
pool_result = _try_resolve_from_custom_pool(
|
||||
base_url, effective_provider, _parse_api_mode(model_cfg.get("api_mode")),
|
||||
provider_name=requested_provider if requested_norm != "custom" else None,
|
||||
)
|
||||
if pool_result:
|
||||
return pool_result
|
||||
|
|
|
|||
|
|
@ -924,6 +924,43 @@ def test_get_custom_provider_pool_key(tmp_path, monkeypatch):
|
|||
assert get_custom_provider_pool_key("") is None
|
||||
|
||||
|
||||
def test_get_custom_provider_pool_key_prefers_name_over_base_url(tmp_path, monkeypatch):
|
||||
"""When two custom providers share the same base_url, provider_name resolves to the correct one."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||
(tmp_path / "hermes").mkdir(parents=True, exist_ok=True)
|
||||
import yaml
|
||||
config_path = tmp_path / "hermes" / "config.yaml"
|
||||
config_path.write_text(yaml.dump({
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "provider-a",
|
||||
"base_url": "http://gateway:8080/v1",
|
||||
"api_key": "sk-aaa",
|
||||
},
|
||||
{
|
||||
"name": "provider-b",
|
||||
"base_url": "http://gateway:8080/v1",
|
||||
"api_key": "sk-bbb",
|
||||
},
|
||||
]
|
||||
}))
|
||||
|
||||
from agent.credential_pool import get_custom_provider_pool_key
|
||||
|
||||
# Without provider_name, first match wins (backward compatible)
|
||||
assert get_custom_provider_pool_key("http://gateway:8080/v1") == "custom:provider-a"
|
||||
|
||||
# With provider_name, exact name match wins regardless of order
|
||||
assert get_custom_provider_pool_key("http://gateway:8080/v1", provider_name="provider-b") == "custom:provider-b"
|
||||
assert get_custom_provider_pool_key("http://gateway:8080/v1", provider_name="provider-a") == "custom:provider-a"
|
||||
|
||||
# Name match with non-matching base_url still works via fallback
|
||||
assert get_custom_provider_pool_key("http://gateway:8080/v1", provider_name="nonexistent") == "custom:provider-a"
|
||||
|
||||
# Empty provider_name is same as None (backward compatible)
|
||||
assert get_custom_provider_pool_key("http://gateway:8080/v1", provider_name="") == "custom:provider-a"
|
||||
|
||||
|
||||
def test_list_custom_pool_providers(tmp_path, monkeypatch):
|
||||
"""list_custom_pool_providers returns custom: pool keys from auth.json."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue