fix(config): skip URL alias without extra_headers instead of returning early (#74465)
get_custom_provider_extra_headers() was returning the result of normalize_extra_headers() on the first matching base_url, even when that entry had no extra_headers configured. A later providers.<name> entry sharing the same URL but with headers set was therefore ignored. Fix: store the normalized headers and only return when non-empty, otherwise continue searching the remaining entries. Fixes #74465
This commit is contained in:
parent
595be544c0
commit
4be138eb03
|
|
@ -1678,7 +1678,9 @@ def get_custom_provider_extra_headers(
|
|||
entry_url = normalize_route_base_url(entry.get("base_url"))
|
||||
if not entry_url or entry_url != target_url:
|
||||
continue
|
||||
return normalize_extra_headers(entry.get("extra_headers"))
|
||||
headers = normalize_extra_headers(entry.get("extra_headers"))
|
||||
if headers:
|
||||
return headers
|
||||
return {}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,16 @@ from hermes_cli.config import (
|
|||
from hermes_cli import models as models_mod
|
||||
|
||||
|
||||
def test_normalize_extra_headers_stringifies_and_drops_none():
|
||||
assert normalize_extra_headers({"X-Int": 7, "X-Str": "v", "X-None": None}) == {
|
||||
"X-Int": "7",
|
||||
"X-Str": "v",
|
||||
}
|
||||
|
||||
|
||||
def test_normalize_extra_headers_rejects_non_dict_and_empty():
|
||||
for bad in (None, "x", 42, ["a"], {}):
|
||||
assert normalize_extra_headers(bad) == {}
|
||||
|
||||
|
||||
def test_normalize_entry_keeps_extra_headers():
|
||||
|
|
@ -32,14 +42,168 @@ def test_normalize_entry_keeps_extra_headers():
|
|||
}
|
||||
|
||||
|
||||
def test_normalize_entry_drops_invalid_extra_headers():
|
||||
for bad in ("not-a-dict", {}, 42, ["a"]):
|
||||
normalized = _normalize_custom_provider_entry(
|
||||
{
|
||||
"name": "my-proxy",
|
||||
"base_url": "https://llm.internal.example.com/v1",
|
||||
"extra_headers": bad,
|
||||
}
|
||||
)
|
||||
assert normalized is not None
|
||||
assert "extra_headers" not in normalized
|
||||
|
||||
|
||||
def test_normalize_entry_stringifies_values_and_skips_none():
|
||||
normalized = _normalize_custom_provider_entry(
|
||||
{
|
||||
"name": "my-proxy",
|
||||
"base_url": "https://llm.internal.example.com/v1",
|
||||
"extra_headers": {"X-Int": 7, "X-None": None},
|
||||
}
|
||||
)
|
||||
assert normalized is not None
|
||||
assert normalized["extra_headers"] == {"X-Int": "7"}
|
||||
|
||||
|
||||
def test_get_custom_provider_extra_headers_matches_base_url():
|
||||
"""Match by normalized base_url returns the entry's extra_headers."""
|
||||
providers = [
|
||||
{
|
||||
"name": "my-proxy",
|
||||
"base_url": "https://llm.internal.example.com/v1",
|
||||
"extra_headers": {"CF-Access-Client-Id": "xxxx.access"},
|
||||
}
|
||||
]
|
||||
# trailing-slash and case insensitive match, mirroring the TLS helper
|
||||
headers = get_custom_provider_extra_headers(
|
||||
"https://LLM.internal.example.com/v1/",
|
||||
custom_providers=providers,
|
||||
)
|
||||
assert headers == {"CF-Access-Client-Id": "xxxx.access"}
|
||||
|
||||
|
||||
def test_get_custom_provider_extra_headers_no_match_returns_empty():
|
||||
"""No matching base_url yields empty dict."""
|
||||
providers = [
|
||||
{
|
||||
"name": "my-proxy",
|
||||
"base_url": "https://llm.internal.example.com/v1",
|
||||
"extra_headers": {"X-Secret": "s"},
|
||||
}
|
||||
]
|
||||
assert get_custom_provider_extra_headers(
|
||||
"https://other.example.com/v1", custom_providers=providers,
|
||||
) == {}
|
||||
# prefix look-alike host must not match (no substring bypass)
|
||||
assert get_custom_provider_extra_headers(
|
||||
"https://llm.internal.example.com.attacker.test/v1",
|
||||
custom_providers=providers,
|
||||
) == {}
|
||||
|
||||
|
||||
def test_get_custom_provider_extra_headers_preserves_extra_path_segment():
|
||||
"""Extra path segment after normalisation is still a mismatch."""
|
||||
providers = [
|
||||
{
|
||||
"base_url": "https://llm.internal.example.com/v1//",
|
||||
"extra_headers": {"Authorization": "secret"},
|
||||
}
|
||||
]
|
||||
assert get_custom_provider_extra_headers(
|
||||
"https://llm.internal.example.com/v1",
|
||||
custom_providers=providers,
|
||||
) == {}
|
||||
|
||||
|
||||
def test_get_custom_provider_extra_headers_skips_alias_without_headers():
|
||||
"""Bug #74465: an earlier entry matching the same URL but without
|
||||
extra_headers must not shadow a later entry that DOES have headers."""
|
||||
providers = [
|
||||
{
|
||||
"name": "direct",
|
||||
"base_url": "http://127.0.0.1:8787/v1",
|
||||
# no extra_headers
|
||||
},
|
||||
{
|
||||
"name": "aether-router",
|
||||
"base_url": "http://127.0.0.1:8787/v1",
|
||||
"extra_headers": {"X-Aether-Route": "my-route"},
|
||||
},
|
||||
]
|
||||
headers = get_custom_provider_extra_headers(
|
||||
"http://127.0.0.1:8787/v1",
|
||||
custom_providers=providers,
|
||||
)
|
||||
assert headers == {"X-Aether-Route": "my-route"}
|
||||
|
||||
|
||||
def test_get_custom_provider_extra_headers_skips_empty_header_dict_alias():
|
||||
"""An earlier entry with an explicit but empty extra_headers dict must
|
||||
not shadow a later entry that carries real headers."""
|
||||
providers = [
|
||||
{
|
||||
"name": "bare-alias",
|
||||
"base_url": "http://127.0.0.1:8787/v1",
|
||||
"extra_headers": {},
|
||||
},
|
||||
{
|
||||
"name": "proxied",
|
||||
"base_url": "http://127.0.0.1:8787/v1",
|
||||
"extra_headers": {"Authorization": "Bearer tok"},
|
||||
},
|
||||
]
|
||||
headers = get_custom_provider_extra_headers(
|
||||
"http://127.0.0.1:8787/v1",
|
||||
custom_providers=providers,
|
||||
)
|
||||
assert headers == {"Authorization": "Bearer tok"}
|
||||
|
||||
|
||||
def test_apply_extra_headers_merges_onto_existing_defaults():
|
||||
"""apply_custom_provider_extra_headers_to_client_kwargs merges headers,
|
||||
with provider-specific values winning over existing defaults."""
|
||||
client_kwargs = {
|
||||
"api_key": "x",
|
||||
"base_url": "https://llm.internal.example.com/v1",
|
||||
"default_headers": {"User-Agent": "curl/8.7.1", "X-Keep": "1"},
|
||||
}
|
||||
providers = [
|
||||
{
|
||||
"name": "my-proxy",
|
||||
"base_url": "https://llm.internal.example.com/v1",
|
||||
"extra_headers": {"User-Agent": "override", "X-New": "2"},
|
||||
}
|
||||
]
|
||||
apply_custom_provider_extra_headers_to_client_kwargs(
|
||||
client_kwargs,
|
||||
"https://llm.internal.example.com/v1",
|
||||
custom_providers=providers,
|
||||
)
|
||||
assert client_kwargs["default_headers"] == {
|
||||
"User-Agent": "override", # provider-specific value wins
|
||||
"X-Keep": "1", # untouched defaults preserved
|
||||
"X-New": "2",
|
||||
}
|
||||
|
||||
|
||||
def test_apply_extra_headers_noop_without_match():
|
||||
"""No matching base_url -> no default_headers key added."""
|
||||
client_kwargs = {"api_key": "x", "base_url": "https://other.example.com/v1"}
|
||||
providers = [
|
||||
{
|
||||
"name": "my-proxy",
|
||||
"base_url": "https://llm.internal.example.com/v1",
|
||||
"extra_headers": {"X-Secret": "s"},
|
||||
}
|
||||
]
|
||||
apply_custom_provider_extra_headers_to_client_kwargs(
|
||||
client_kwargs,
|
||||
"https://other.example.com/v1",
|
||||
custom_providers=providers,
|
||||
)
|
||||
assert "default_headers" not in client_kwargs
|
||||
|
||||
|
||||
def test_fetch_api_models_sends_extra_headers_to_models_probe(monkeypatch):
|
||||
|
|
|
|||
Loading…
Reference in New Issue