From 0c97a883af75ace61844710dc58c6f1732d0faed Mon Sep 17 00:00:00 2001 From: Drexuxux Date: Wed, 5 Aug 2026 11:45:40 +0300 Subject: [PATCH] fix(model-switch): read picker key_env through the per-profile secret scope 854007d1c routed the remaining main-agent fallback key reads through agent.secret_scope so the multiplexed gateway's per-profile scope applies. list_authenticated_providers - which gateway/slash_commands.py calls directly for /model - still resolved custom-endpoint and fallback-entry credentials with raw os.environ.get(key_env), so under multiplex_profiles one profile's picker reads whatever key the process environment happens to hold, i.e. another profile's. no multiplexing : profileA-key (unchanged) scope installed : profileB-key (was profileA-key) Route both reads through a _scoped_key_env() helper over secret_scope.get_secret(). get_secret is identical to os.getenv when multiplexing is off, so single-profile deployments are byte-for-byte unchanged; a fail-closed UnscopedSecretError is treated as "no credential visible for this profile", which is how the picker already handles a missing key. Scope: only the two key_env credential reads. The other environment reads in that function are provider-presence probes (AWS creds, LM_BASE_URL), a separate concern. --- hermes_cli/model_switch.py | 30 +++++++++++-- .../test_model_picker_secret_scope.py | 44 +++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 tests/hermes_cli/test_model_picker_secret_scope.py diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 5bf6f5fe765a6..9252dbddb2421 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -2024,6 +2024,30 @@ def prewarm_picker_cache_async() -> Optional["_threading.Thread"]: return t +def _scoped_key_env(name: str) -> str: + """Read a provider key env var through the per-profile secret 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, in a multiplexer. That is + the class swept in 854007d1c for the fallback/aux key reads; the picker's + ``key_env`` reads were not covered. + + 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. + """ + if not name: + return "" + try: + from agent.secret_scope import get_secret + + return (get_secret(name, "") or "").strip() + except Exception: + return "" + + def list_authenticated_providers( current_provider: str = "", current_base_url: str = "", @@ -2791,7 +2815,7 @@ def list_authenticated_providers( api_key = str(ep_cfg.get("api_key", "") or "").strip() if not api_key: key_env = str(ep_cfg.get("key_env", "") or "").strip() - api_key = os.environ.get(key_env, "").strip() if key_env else "" + api_key = _scoped_key_env(key_env) discover = ep_cfg.get("discover_models", True) if isinstance(discover, str): discover = discover.lower() not in {"false", "no", "0"} @@ -2965,9 +2989,7 @@ def list_authenticated_providers( continue inline_api_key = (entry.get("api_key") or "").strip() key_env = (entry.get("key_env") or "").strip() - api_key = inline_api_key or ( - os.environ.get(key_env, "").strip() if key_env else "" - ) + api_key = inline_api_key or _scoped_key_env(key_env) api_mode = str( entry.get("api_mode") or entry.get("transport") diff --git a/tests/hermes_cli/test_model_picker_secret_scope.py b/tests/hermes_cli/test_model_picker_secret_scope.py new file mode 100644 index 0000000000000..21e11e77d645c --- /dev/null +++ b/tests/hermes_cli/test_model_picker_secret_scope.py @@ -0,0 +1,44 @@ +"""The /model picker must read provider keys through the per-profile scope. + +854007d1c ("route remaining main-agent fallback key reads through +secret_scope") swept the fallback/auxiliary key reads. ``key_env`` lookups in +``list_authenticated_providers`` — which the gateway's ``/model`` handler calls +directly — were not covered, so under ``multiplex_profiles`` one profile's +picker resolved another profile's key from the process environment. +""" + +import os + +from agent import secret_scope +from hermes_cli.model_switch import _scoped_key_env + + +class TestPickerKeyEnvScope: + def test_unscoped_read_matches_the_process_environment(self, monkeypatch): + """Single-profile deployments must behave exactly as before.""" + monkeypatch.setenv("ACME_KEY", "from-environment") + + assert _scoped_key_env("ACME_KEY") == "from-environment" + + def test_installed_scope_wins_over_the_process_environment(self, monkeypatch): + """The multiplexed gateway installs a scope per turn; the picker must + read that profile's credential, not whatever the process inherited.""" + monkeypatch.setenv("ACME_KEY", "other-profile-key") + token = secret_scope.set_secret_scope({"ACME_KEY": "this-profile-key"}) + try: + assert _scoped_key_env("ACME_KEY") == "this-profile-key" + finally: + secret_scope.reset_secret_scope(token) + + assert _scoped_key_env("ACME_KEY") == "other-profile-key" + + def test_absent_key_and_empty_name_resolve_empty(self, monkeypatch): + monkeypatch.delenv("ACME_KEY", raising=False) + + assert _scoped_key_env("ACME_KEY") == "" + assert _scoped_key_env("") == "" + + def test_value_is_stripped(self, monkeypatch): + monkeypatch.setenv("ACME_KEY", " padded ") + + assert _scoped_key_env("ACME_KEY") == "padded"