mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(config): invalidate load_config cache when referenced ${VAR} env values change
The load_config() cache is keyed on config file mtime/size only, so a
load_config() that runs before load_hermes_dotenv() populates the process
environment caches the unexpanded ${VAR} literal and serves it for the
life of the process — auxiliary.<task>.api_key/base_url env refs reach the
provider client verbatim (auth failure / silent fallback), while
providers.* appear to work because provider credential resolution re-reads
the environment at call time.
Record a snapshot of every ${VAR} name referenced in the raw config
(user + managed) with its os.environ value at expansion time, and treat
the cache as stale when any of those values change. Covers both the late
.env load and in-process key rotation; an unchanged environment still
takes the cache-hit path.
Fixes #58514
This commit is contained in:
parent
c9a150d640
commit
e3203e4d80
2 changed files with 99 additions and 6 deletions
|
|
@ -224,9 +224,11 @@ _LAST_EXPANDED_CONFIG_BY_PATH: Dict[str, Any] = {}
|
|||
# produces a fresh inode, so stat() sees a new mtime_ns and the next
|
||||
# load repopulates automatically — no explicit invalidation hook.
|
||||
# Cached tuple is (user_mtime_ns, user_size, managed_mtime_ns, managed_size,
|
||||
# merged_value) — the managed-file signature is folded in so editing the
|
||||
# managed-scope config.yaml invalidates the cache (see managed_scope).
|
||||
_LOAD_CONFIG_CACHE: Dict[str, Tuple[int, int, int, int, Dict[str, Any]]] = {}
|
||||
# merged_value, env_ref_snapshot) — the managed-file signature is folded in so
|
||||
# editing the managed-scope config.yaml invalidates the cache (see
|
||||
# managed_scope), and the env snapshot invalidates it when a referenced ${VAR}
|
||||
# changes value (late .env load, in-process rotation — #58514).
|
||||
_LOAD_CONFIG_CACHE: Dict[str, Tuple[int, int, int, int, Dict[str, Any], Dict[str, Optional[str]]]] = {}
|
||||
# (path, mtime_ns, size) -> cached raw yaml dict. Same pattern as
|
||||
# _LOAD_CONFIG_CACHE but for read_raw_config() — used when callers want
|
||||
# the user's on-disk values without defaults merged in.
|
||||
|
|
@ -6147,6 +6149,31 @@ def _expand_env_vars(obj):
|
|||
return obj
|
||||
|
||||
|
||||
def _env_ref_snapshot(obj, snapshot=None):
|
||||
"""Map every ``${VAR}`` name referenced in config values to its current
|
||||
``os.environ`` value (``None`` when unset).
|
||||
|
||||
Stored alongside cached ``load_config()`` results so a cache hit can
|
||||
detect that the cached expansion was made against a *different*
|
||||
environment — e.g. a ``load_config()`` that ran before
|
||||
``load_hermes_dotenv()`` populated the process env, or an env var
|
||||
rotated in-process after the first load. File mtime/size alone cannot
|
||||
see either case (#58514).
|
||||
"""
|
||||
if snapshot is None:
|
||||
snapshot = {}
|
||||
if isinstance(obj, str):
|
||||
for name in re.findall(r"\${([^}]+)}", obj):
|
||||
snapshot[name] = os.environ.get(name)
|
||||
elif isinstance(obj, dict):
|
||||
for value in obj.values():
|
||||
_env_ref_snapshot(value, snapshot)
|
||||
elif isinstance(obj, list):
|
||||
for item in obj:
|
||||
_env_ref_snapshot(item, snapshot)
|
||||
return snapshot
|
||||
|
||||
|
||||
def _items_by_unique_name(items):
|
||||
"""Return a name-indexed dict only when all items have unique string names."""
|
||||
if not isinstance(items, list):
|
||||
|
|
@ -6746,7 +6773,14 @@ def _load_config_impl(*, want_deepcopy: bool) -> Dict[str, Any]:
|
|||
|
||||
cached = _LOAD_CONFIG_CACHE.get(path_key)
|
||||
if cached is not None and cache_sig is not None and cached[:4] == cache_sig:
|
||||
return copy.deepcopy(cached[4]) if want_deepcopy else cached[4]
|
||||
# File signatures match, but the cached expansion is only valid if
|
||||
# every ${VAR} it was expanded against still has the same value.
|
||||
# Without this, a load_config() that ran before load_hermes_dotenv()
|
||||
# pins unexpanded literals (e.g. auxiliary.<task>.api_key) for the
|
||||
# life of the process (#58514).
|
||||
env_snapshot = cached[5] if len(cached) > 5 else {}
|
||||
if all(os.environ.get(k) == v for k, v in env_snapshot.items()):
|
||||
return copy.deepcopy(cached[4]) if want_deepcopy else cached[4]
|
||||
|
||||
config = copy.deepcopy(DEFAULT_CONFIG)
|
||||
|
||||
|
|
@ -6783,9 +6817,15 @@ def _load_config_impl(*, want_deepcopy: bool) -> Dict[str, Any]:
|
|||
# (deepcopy=True) callers can mutate freely without affecting the
|
||||
# cached value, and ``load_config_readonly()`` (deepcopy=False)
|
||||
# callers all see the same stable cached object. The cached tuple is
|
||||
# (user_mtime, user_size, managed_mtime, managed_size, value).
|
||||
# (user_mtime, user_size, managed_mtime, managed_size, value,
|
||||
# env_ref_snapshot). The snapshot records the environment values
|
||||
# this expansion was made against so later loads can detect env
|
||||
# drift (late .env load, in-process rotation) — see cache hit above.
|
||||
cached_copy = copy.deepcopy(expanded)
|
||||
_LOAD_CONFIG_CACHE[path_key] = (*cache_sig, cached_copy)
|
||||
env_snapshot = _env_ref_snapshot(normalized)
|
||||
if managed_config:
|
||||
_env_ref_snapshot(managed_config, env_snapshot)
|
||||
_LOAD_CONFIG_CACHE[path_key] = (*cache_sig, cached_copy, env_snapshot)
|
||||
# On the readonly path return the same cached object subsequent
|
||||
# calls will see — keeps "two readonly calls return the same
|
||||
# object" invariant that callers may rely on for identity checks.
|
||||
|
|
|
|||
|
|
@ -94,6 +94,59 @@ class TestLoadConfigExpansion:
|
|||
assert config["model"]["api_key"] == "${NOT_SET_XYZ_123}"
|
||||
|
||||
|
||||
class TestLoadConfigCacheEnvStaleness:
|
||||
"""The load_config() cache must not pin expansions made against a stale
|
||||
environment (#58514): a load before load_hermes_dotenv() runs, or an env
|
||||
var rotated in-process, must not keep serving the old expansion."""
|
||||
|
||||
def test_env_var_appearing_after_first_load_invalidates_cache(self, tmp_path, monkeypatch):
|
||||
config_yaml = "auxiliary:\n vision:\n api_key: ${LATE_DOTENV_KEY_58514}\n"
|
||||
config_file = tmp_path / "config.yaml"
|
||||
config_file.write_text(config_yaml)
|
||||
|
||||
monkeypatch.delenv("LATE_DOTENV_KEY_58514", raising=False)
|
||||
monkeypatch.setitem(load_config.__globals__, "get_config_path", lambda: config_file)
|
||||
|
||||
# First load happens before the var exists (pre-dotenv): literal kept.
|
||||
assert load_config()["auxiliary"]["vision"]["api_key"] == "${LATE_DOTENV_KEY_58514}"
|
||||
|
||||
# .env load brings the var in — same file mtime/size, env changed.
|
||||
monkeypatch.setenv("LATE_DOTENV_KEY_58514", "nvapi-real")
|
||||
assert load_config()["auxiliary"]["vision"]["api_key"] == "nvapi-real"
|
||||
|
||||
def test_env_var_rotation_invalidates_cache(self, tmp_path, monkeypatch):
|
||||
config_yaml = "providers:\n mistral:\n api_key: ${ROTATED_KEY_58514}\n"
|
||||
config_file = tmp_path / "config.yaml"
|
||||
config_file.write_text(config_yaml)
|
||||
|
||||
monkeypatch.setenv("ROTATED_KEY_58514", "key-v1")
|
||||
monkeypatch.setitem(load_config.__globals__, "get_config_path", lambda: config_file)
|
||||
|
||||
assert load_config()["providers"]["mistral"]["api_key"] == "key-v1"
|
||||
|
||||
monkeypatch.setenv("ROTATED_KEY_58514", "key-v2")
|
||||
assert load_config()["providers"]["mistral"]["api_key"] == "key-v2"
|
||||
|
||||
def test_unchanged_env_still_serves_cache(self, tmp_path, monkeypatch):
|
||||
config_yaml = "providers:\n mistral:\n api_key: ${STABLE_KEY_58514}\n"
|
||||
config_file = tmp_path / "config.yaml"
|
||||
config_file.write_text(config_yaml)
|
||||
|
||||
monkeypatch.setenv("STABLE_KEY_58514", "key-stable")
|
||||
monkeypatch.setitem(load_config.__globals__, "get_config_path", lambda: config_file)
|
||||
|
||||
load_config()
|
||||
# load_config_readonly() returns the cached object itself, so object
|
||||
# identity across calls proves the cache-hit path was taken (a rebuild
|
||||
# would produce a fresh dict).
|
||||
readonly = load_config.__globals__["load_config_readonly"]
|
||||
first = readonly()
|
||||
second = readonly()
|
||||
|
||||
assert first is second
|
||||
assert first["providers"]["mistral"]["api_key"] == "key-stable"
|
||||
|
||||
|
||||
class TestLoadCliConfigExpansion:
|
||||
"""Verify that load_cli_config() also expands ${VAR} references."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue