mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
fix(config): stop _normalize_custom_provider_entry mutating the caller's dict
The normalizer writes alias keys into the entry it is given (entry['key_env'] = entry['api_key_env'] and entry[snake] = entry[camel]) while building its normalized copy. Two of its three callers — get_compatible_custom_providers and providers_dict_to_custom_providers — pass live sub-dicts straight from load_config_readonly()'s shared cache (only _custom_provider_entry_to_provider_config defends with dict(entry)). A config written with the documented camelCase / api_key_env aliases therefore gets its cached copy polluted with injected duplicate keys, violating the cache's explicit no-mutation contract; every later load_config() deepcopy inherits the duplicates, and any save_config(load_config()) flow (setup wizard, dashboard writes, model persist) writes them back to config.yaml. The aux-client TLS resolution runs this on every auxiliary client build, so the mutation also happens unlocked on worker threads against a shared object. Shallow-copy the entry up front; the function's return value is a separately-built dict, so behavior is otherwise unchanged.
This commit is contained in:
parent
25927884e0
commit
bfe4cbdc2a
2 changed files with 62 additions and 0 deletions
|
|
@ -1280,6 +1280,15 @@ def _normalize_custom_provider_entry(
|
|||
if not isinstance(entry, dict):
|
||||
return None
|
||||
|
||||
# Shallow-copy before the alias normalization below writes into the
|
||||
# entry: callers (get_compatible_custom_providers,
|
||||
# providers_dict_to_custom_providers) pass live sub-dicts from
|
||||
# load_config_readonly()'s shared cache, and mutating those both
|
||||
# violates the cache's no-mutation contract and leaks duplicated
|
||||
# alias keys back into config.yaml through any later
|
||||
# save_config(load_config()) round-trip.
|
||||
entry = dict(entry)
|
||||
|
||||
# Accept camelCase aliases commonly used in hand-written configs.
|
||||
_CAMEL_ALIASES: Dict[str, str] = {
|
||||
"apiKey": "api_key",
|
||||
|
|
|
|||
|
|
@ -284,3 +284,56 @@ class TestNormalizeCustomProviderEntry:
|
|||
}
|
||||
result = _normalize_custom_provider_entry(entry, provider_key="bad")
|
||||
assert result is None
|
||||
|
||||
|
||||
class TestNormalizeDoesNotMutateInput:
|
||||
"""The normalizer must not write alias keys into the caller's dict.
|
||||
|
||||
get_compatible_custom_providers and providers_dict_to_custom_providers
|
||||
pass live sub-dicts from load_config_readonly()'s shared cache; an
|
||||
in-place write violates the cache's no-mutation contract and leaks
|
||||
duplicated alias keys into config.yaml via save_config(load_config()).
|
||||
"""
|
||||
|
||||
def test_camel_case_entry_is_not_mutated(self):
|
||||
entry = {
|
||||
"name": "x",
|
||||
"baseUrl": "https://api.example.com/v1",
|
||||
"apiKeyEnv": "MY_KEY",
|
||||
}
|
||||
snapshot = dict(entry)
|
||||
result = _normalize_custom_provider_entry(entry, provider_key="x")
|
||||
assert result is not None
|
||||
assert result["base_url"] == "https://api.example.com/v1"
|
||||
assert entry == snapshot
|
||||
|
||||
def test_api_key_env_alias_entry_is_not_mutated(self):
|
||||
entry = {
|
||||
"name": "x",
|
||||
"base_url": "https://api.example.com/v1",
|
||||
"api_key_env": "MY_KEY",
|
||||
}
|
||||
snapshot = dict(entry)
|
||||
result = _normalize_custom_provider_entry(entry, provider_key="x")
|
||||
assert result is not None
|
||||
assert result["key_env"] == "MY_KEY"
|
||||
assert entry == snapshot
|
||||
|
||||
def test_get_compatible_custom_providers_does_not_mutate_config(self):
|
||||
from hermes_cli.config import get_compatible_custom_providers
|
||||
|
||||
config = {
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "ollama",
|
||||
"baseUrl": "https://ollama.example.com/v1",
|
||||
"apiKeyEnv": "OLLAMA_KEY",
|
||||
}
|
||||
]
|
||||
}
|
||||
import copy
|
||||
|
||||
snapshot = copy.deepcopy(config)
|
||||
providers = get_compatible_custom_providers(config)
|
||||
assert providers, "entry should normalize successfully"
|
||||
assert config == snapshot
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue