From abdb31cd99b5bc470dec850f7fc60cad5e64cfb2 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sun, 13 Sep 2026 01:45:35 +0800 Subject: [PATCH] fix(cli): resolve .env-only key_env credentials for the /model probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/model` fed `validate_requested_model()` a key resolved through `agent.secret_scope.get_secret`, which (multiplexing off) reads only `os.environ`. Hermes does not export `$HERMES_HOME/.env` into the process environment, so a `custom_providers` entry whose `key_env` lives only in `.env` probed `/v1/models` unauthenticated, got 401 and printed a spurious "could not reach this custom endpoint's model listing" note while chat worked fine. Resolve through `get_env_prefer_dotenv` — the chain `client_lifecycle` uses for the real request — when no profile scope is installed. With a scope installed or multiplexing active the scope stays authoritative: a scoped miss still returns "" and never borrows another profile's `.env`/process value. Slimmed from the contributor's two commits (same mechanism, fewer branches, tests trimmed to two invariants). Fixes #109315 --- hermes_cli/model_switch.py | 23 +++++--- .../test_model_picker_secret_scope.py | 52 +++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 4be86539db..e9030d6f78 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1599,16 +1599,23 @@ def _extra_headers_from_config(entry: Any) -> dict[str, str]: def _scoped_key_env(name: str) -> str: - """Read a provider key env var through the per-profile secret scope. + """Read a provider key env var the way the chat path does, honouring the per-profile scope. - The multiplexed gateway installs a secret scope per turn; a raw ``os.environ`` read hands the - current profile whatever key happens to be in the process environment — another profile's. - Identical to ``os.getenv`` when multiplexing is off. A fail-closed ``UnscopedSecretError`` - (multiplexing on, no scope installed) means "no credential visible for this profile here", - which is exactly how the picker already treats a missing key.""" + With a secret scope installed (multiplexed gateway turn, dashboard/kanban workers) the scope's + verdict is authoritative: a hit is this profile's key, a miss must not borrow another profile's + value from the process env or the default ``.env``. Multiplexing on with no scope fails closed + (``UnscopedSecretError`` -> ""). Otherwise resolve through ``get_env_prefer_dotenv`` — the + chain ``client_lifecycle`` uses for the actual request — so a ``key_env`` that lives only in + ``$HERMES_HOME/.env`` authenticates the ``/model`` verification probe (#109315) and a rotated + ``.env`` beats a stale value inherited from the parent shell.""" + if not name: + return "" try: - from agent.secret_scope import get_secret - return (get_secret(name, "") or "").strip() if name else "" + from agent.secret_scope import current_secret_scope, get_secret, is_multiplex_active + if current_secret_scope() is not None or is_multiplex_active(): + return (get_secret(name, "") or "").strip() + from agent.credential_pool import get_env_prefer_dotenv + return (get_env_prefer_dotenv(name) or "").strip() except Exception: return "" diff --git a/tests/hermes_cli/test_model_picker_secret_scope.py b/tests/hermes_cli/test_model_picker_secret_scope.py index 677188027f..34708b05fe 100644 --- a/tests/hermes_cli/test_model_picker_secret_scope.py +++ b/tests/hermes_cli/test_model_picker_secret_scope.py @@ -97,3 +97,55 @@ class TestSwitchModelKeyEnvScope: finally: secret_scope.reset_secret_scope(token) assert captured["key"] == "this-profile-key" + + +class TestPickerKeyEnvDotenv: + """``key_env`` must resolve through the chat path's chain (``get_env_prefer_dotenv``): a key + that lives only in ``$HERMES_HOME/.env`` authenticates the ``/model`` verification probe, and + a scoped multiplex read never borrows the ``.env``/process value of another profile.""" + + def _dotenv(self, monkeypatch, tmp_path, value): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / ".env").write_text(f"ACME_RELAY_KEY={value}\n", encoding="utf-8") + from hermes_cli.config import invalidate_env_cache + invalidate_env_cache() + + def test_switch_probe_uses_dotenv_key_over_stale_process_env(self, monkeypatch, tmp_path): + self._dotenv(monkeypatch, tmp_path, "fresh-dotenv") + monkeypatch.setenv("ACME_RELAY_KEY", "stale-process") + import hermes_cli.model_switch as ms + import hermes_cli.models_validate as mv + + captured = {} + + def _fake_runtime(requested, explicit_api_key=None, explicit_base_url=None, target_model=None, **kw): + return {"api_key": explicit_api_key or "", "base_url": explicit_base_url, "api_mode": ""} + + def _fake_validate(model, provider, api_key=None, base_url=None, api_mode=None, headers=None, **kw): + captured["api_key"] = api_key + return {"accepted": True, "persist": True, "recognized": True, "message": ""} + + monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _fake_runtime) + monkeypatch.setattr(ms, "resolve_alias", lambda *a, **k: None) + monkeypatch.setattr(mv, "validate_requested_model", _fake_validate) + + ms.switch_model( + "some-model", current_provider="openrouter", current_model="x", explicit_provider="acme", + user_providers={"acme": {"base_url": "https://api.acme.test/v1", "key_env": "ACME_RELAY_KEY"}}, + ) + + assert captured["api_key"] == "fresh-dotenv" + + def test_multiplex_scoped_miss_never_borrows_dotenv_or_process_env(self, monkeypatch, tmp_path): + self._dotenv(monkeypatch, tmp_path, "default-profile-key") + monkeypatch.setenv("ACME_RELAY_KEY", "other-profile-key") + secret_scope.set_multiplex_active(True) + try: + assert _scoped_key_env("ACME_RELAY_KEY") == "" # no scope installed: fail closed + token = secret_scope.set_secret_scope({"OTHER": "x"}) + try: + assert _scoped_key_env("ACME_RELAY_KEY") == "" # scoped miss: no fallthrough + finally: + secret_scope.reset_secret_scope(token) + finally: + secret_scope.set_multiplex_active(False)