ENV_VAR placeholder expansion in config and gateway

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #53584 fix(config): accept placeholder base_url in custom provider validation (#14457)

  merged to main

  files: hermes_cli/config.py, tests/hermes_cli/test_provider_config_validation.py

  Show the diff

  • hermes_cli/config.py

    @@ -4269,6 +4269,14 @@ def _normalize_custom_provider_entry(
             raw_url = entry.get(url_key)
             if isinstance(raw_url, str) and raw_url.strip():
                 candidate = raw_url.strip()
    +            # Accept URLs containing unresolved placeholder tokens — both
    +            # ``${ENV_VAR}`` env-refs and bare ``{region}``-style templates —
    +            # without URL validation. They are expanded at runtime, so a
    +            # caller reaching this normalizer with raw (un-expanded) config
    +            # would otherwise see the provider silently dropped (#14457).
    +            if re.search(r"\{[^}]+\}", candidate):
    +                base_url = candidate
    +                break
                 parsed = urlparse(candidate)
                 if parsed.scheme and parsed.netloc:
                     base_url = candidate

  • tests/hermes_cli/test_provider_config_validation.py

    @@ -191,3 +191,49 @@ def test_models_empty_list_omitted(self):
             result = _normalize_custom_provider_entry(entry)
             assert result is not None
             assert "models" not in result
    +
    +    def test_env_var_placeholder_in_base_url_not_rejected(self):
    +        """A base_url that is an un-expanded ${ENV_VAR} placeholder must not be
    +        rejected as an invalid URL — it is expanded at runtime, so a caller
    +        reaching this normalizer with raw config would otherwise see the
    +        provider silently dropped. Regression test for #14457."""
    +        entry = {
    +            "name": "PROVIDER_A",
    +            "base_url": "${PROVIDER_A_BASE_URL}",
    +            "key_env": "PROVIDER_A_API_KEY",
    +        }
    +        result = _normalize_custom_provider_entry(entry, provider_key="PROVIDER_A")
    +        assert result is not None
    +        assert result["base_url"] == "${PROVIDER_A_BASE_URL}"
    +
    +    def test_multiple_env_vars_in_base_url(self):
    +        """base_url with multiple ${VAR} placeholders is accepted verbatim."""
    +        entry = {
    +            "name": "multi-var-provider",
    +            "base_url": "${SCHEME}://${HOST}:${PORT}/v1",
    +        }
    +        result = _normalize_custom_provider_entry(entry)
    +        assert result is not None
    +        assert result["base_url"] == "${SCHEME}://${HOST}:${PORT}/v1"
    +
    +    def test_bare_brace_region_placeholder_accepted(self):
    +        """A bare {region}-style template token (not an env-ref) is also
    +        accepted without validation, supporting region-substitution URLs."""
    +        entry = {
    +            "name": "regional",
    +            "base_url": "https://{region}.api.example.com/v1",
    +        }
    +        result = _normalize_custom_provider_entry(entry, provider_key="regional")
    +        assert result is not None
    +        assert result["base_url"] == "https://{region}.api.example.com/v1"
    +
    +    def test_invalid_url_without_placeholder_still_rejected(self):
    +        """A malformed URL with no scheme/host AND no placeholder token is
    +        still rejected — the placeholder bypass must not weaken validation of
    +        ordinary literal URLs."""
    +        entry = {
    +            "name": "bad",
    +            "base_url": "not-a-url",
    +        }
    +        result = _normalize_custom_provider_entry(entry, provider_key="bad")
    +        assert result is None

• #59139 fix(config): invalidate load_config cache when referenced ${VAR} env values change

  merged to main

  files: hermes_cli/config.py, scripts/release.py, tests/hermes_cli/test_config_env_expansion.py

  Show the diff

  • hermes_cli/config.py

    @@ -224,9 +224,11 @@ def _reject_denylisted_env_var(key: str) -> None:
     # 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.

  • scripts/release.py

    @@ -47,6 +47,7 @@
     AUTHOR_MAP = {
         "[redacted:email]": "devatnull",  # PR #58704 salvage (whatsapp: native Baileys polls, clarify-as-poll, location pins, structured quoted replies, PTT/audio split, bridge_helpers extraction)
         "[redacted:email]": "tjp2021",  # PR #4097 salvage (copilot: per-turn x-initiator header so user prompts bill as premium requests; #3040)
    +    "[redacted:email]": "falkoro",  # PRs #58519/#58520 salvage (config: env-ref-aware load_config cache invalidation; auxiliary: honor auxiliary.<task>.base_url/api_key with explicit provider arg)
         "[redacted:email]": "kevinrajaram",  # PR #3850 salvage (gateway: add POSIX system dirs to PATH so launchctl/systemctl resolve under UV's minimal-PATH bundled Python; #3849)
         "[redacted:email]": "lord-dubious",  # PR #58453 salvage (preserve static custom provider models declared as dict rows)
         "[redacted:email]": "JAlmanzarMint",  # PR #52688 salvage (vision: rasterize SVG / re-encode unsupported raster formats to PNG before embedding), folded into #57890

  • tests/hermes_cli/test_config_env_expansion.py

    @@ -94,6 +94,59 @@ def test_load_config_unresolved_kept_verbatim(self, tmp_path, monkeypatch):
             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: [redacted:secret-assignment]"
    +        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: [redacted:secret-assignment]"
    +        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: [redacted:secret-assignment]"
    +        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."""