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.
This commit is contained in:
parent
bf7c716648
commit
0c97a883af
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
Loading…
Reference in New Issue