fix(#3961): route provider and model credential reads through the thread-local profile channel
This commit is contained in:
parent
9988f0a3f6
commit
846951304d
4 changed files with 184 additions and 43 deletions
|
|
@ -1288,13 +1288,13 @@ def _legacy_custom_api_key_env_name(provider_id: object) -> str:
|
|||
def _lookup_custom_api_key_env(provider_id: object) -> str | None:
|
||||
"""Look up sanitized custom-provider env first, then legacy broken shape."""
|
||||
env_name = _api_key_env_name(provider_id)
|
||||
api_key = os.getenv(env_name, "").strip()
|
||||
api_key = _thread_local_env_value(env_name).strip()
|
||||
if api_key:
|
||||
return api_key
|
||||
|
||||
legacy_env_name = _legacy_custom_api_key_env_name(provider_id)
|
||||
if legacy_env_name and legacy_env_name != env_name:
|
||||
legacy_key = os.getenv(legacy_env_name, "").strip()
|
||||
legacy_key = _thread_local_env_value(legacy_env_name).strip()
|
||||
if legacy_key:
|
||||
if legacy_env_name not in _LEGACY_CUSTOM_API_KEY_ENV_WARNED:
|
||||
_LEGACY_CUSTOM_API_KEY_ENV_WARNED.add(legacy_env_name)
|
||||
|
|
@ -2408,13 +2408,13 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st
|
|||
if raw_api_key is not None:
|
||||
key_text = str(raw_api_key).strip()
|
||||
if key_text.startswith("${") and key_text.endswith("}") and len(key_text) > 3:
|
||||
api_key = os.getenv(key_text[2:-1], "").strip() or None
|
||||
api_key = _thread_local_env_value(key_text[2:-1]).strip() or None
|
||||
elif key_text:
|
||||
api_key = key_text
|
||||
if not api_key:
|
||||
key_env = str(raw_key_env or "").strip()
|
||||
if key_env:
|
||||
api_key = os.getenv(key_env, "").strip() or None
|
||||
api_key = _thread_local_env_value(key_env).strip() or None
|
||||
if not api_key and provider_hint:
|
||||
api_key = _lookup_custom_api_key_env(provider_hint)
|
||||
return api_key
|
||||
|
|
@ -5234,7 +5234,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
|
|||
"AWS_ACCESS_KEY_ID",
|
||||
"AWS_SECRET_ACCESS_KEY",
|
||||
):
|
||||
val = os.getenv(k)
|
||||
val = _thread_local_env_value(k)
|
||||
if val:
|
||||
all_env[k] = val
|
||||
if all_env.get("ANTHROPIC_API_KEY"):
|
||||
|
|
@ -5557,7 +5557,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
|
|||
"API_KEY",
|
||||
)
|
||||
for key in api_key_vars:
|
||||
api_key = (all_env.get(key) or os.getenv(key) or "").strip()
|
||||
api_key = (all_env.get(key) or _thread_local_env_value(key) or "").strip()
|
||||
if api_key:
|
||||
break
|
||||
|
||||
|
|
@ -5599,7 +5599,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
|
|||
if not _cp_api_key:
|
||||
_cp_key_env = str(_cp.get("key_env") or "").strip()
|
||||
if _cp_key_env:
|
||||
_cp_api_key = str(os.getenv(_cp_key_env) or "").strip()
|
||||
_cp_api_key = _thread_local_env_value(_cp_key_env).strip()
|
||||
# Fallback: check credential pool for both api_key and base_url
|
||||
if (not _cp_api_key or not _cp_base_url) and _slug:
|
||||
try:
|
||||
|
|
@ -6893,6 +6893,22 @@ def _evict_session_agent(session_id: str) -> None:
|
|||
_thread_ctx = threading.local()
|
||||
|
||||
|
||||
def _thread_local_env_value(name: str, default: str = "") -> str:
|
||||
"""Return thread-local profile env first, then process env, for provider reads."""
|
||||
env_name = str(name or "").strip()
|
||||
if not env_name:
|
||||
return default or ""
|
||||
|
||||
thread_env = getattr(_thread_ctx, "env", {})
|
||||
if isinstance(thread_env, dict) and env_name in thread_env:
|
||||
thread_value = thread_env.get(env_name)
|
||||
if thread_value is None:
|
||||
return default
|
||||
return str(thread_value)
|
||||
|
||||
return str(os.getenv(env_name, default or ""))
|
||||
|
||||
|
||||
def _set_thread_env(**kwargs):
|
||||
_thread_ctx.env = kwargs
|
||||
|
||||
|
|
|
|||
|
|
@ -905,36 +905,63 @@ def profile_env_for_active_request(
|
|||
purpose: str = "provider/model read",
|
||||
logger_override: Optional[logging.Logger] = None,
|
||||
):
|
||||
"""Apply the active per-request profile's env around a read-only API call (#3957).
|
||||
"""Apply the active per-request profile's env to thread-local state only (#3957).
|
||||
|
||||
WebUI profile switching is per-client/cookie scoped (issue #798): a browser
|
||||
on a named profile sets a ``hermes_profile`` cookie, which ``server.py``
|
||||
turns into a thread-local via ``set_request_profile()``. But the per-client
|
||||
switch deliberately does NOT reload the profile's ``.env`` into
|
||||
``os.environ`` (that would mutate process-global state shared with other
|
||||
clients). So read-only endpoints that resolve provider credentials through
|
||||
``os.environ`` / ``HERMES_HOME`` — ``/api/providers`` (``get_auth_status``
|
||||
probes) and ``/api/models`` (``provider_model_ids`` / custom-key lookup) —
|
||||
resolve against the *default* profile's credentials instead of the active
|
||||
one. Symptoms: Settings → Providers times out and the model picker shows
|
||||
only the default profile's models.
|
||||
turns into a thread-local via ``set_request_profile()``. This wrapper keeps
|
||||
provider-credential reads isolated to the request profile and does not touch
|
||||
process-wide environment for read-only endpoints.
|
||||
|
||||
This mirrors what streaming already does for the duration of an agent run
|
||||
(``profile_env_for_background_worker``): it temporarily applies the active
|
||||
profile's ``.env`` + terminal config for the duration of the wrapped read,
|
||||
then restores the previous env under ``_ENV_LOCK``.
|
||||
A thread-local read-only scope is used for ``/api/providers`` and
|
||||
``/api/models`` flows that now resolve credentials through thread-local
|
||||
environment first. It also sets a context-local Hermes-home override so
|
||||
agent-side auth-store reads stay on the active profile without mutating
|
||||
process-global ``os.environ``.
|
||||
|
||||
No-ops (zero overhead, byte-identical behavior) for the default/root profile
|
||||
— the overwhelmingly common single-profile deployment is unaffected.
|
||||
No-ops for the default/root profile, which is the common single-profile
|
||||
deployment case.
|
||||
"""
|
||||
profile = (get_active_profile_name() or "").strip()
|
||||
if not profile or _is_root_profile(profile):
|
||||
yield
|
||||
return
|
||||
with profile_env_for_background_worker(
|
||||
profile, purpose, logger_override=logger_override
|
||||
):
|
||||
try:
|
||||
from api.config import _clear_thread_env, _set_thread_env, _thread_ctx
|
||||
from hermes_constants import (
|
||||
reset_hermes_home_override,
|
||||
set_hermes_home_override,
|
||||
)
|
||||
profile_home_path = Path(get_hermes_home_for_profile(profile))
|
||||
runtime_env = get_profile_runtime_env(profile_home_path)
|
||||
safe_runtime_env = filter_runtime_env_for_gateway_parity(runtime_env)
|
||||
except Exception:
|
||||
log = logger_override or logger
|
||||
log.debug(
|
||||
"Failed to resolve profile env for active request profile %s in %s; "
|
||||
"falling back to current env",
|
||||
profile,
|
||||
purpose,
|
||||
exc_info=True,
|
||||
)
|
||||
yield
|
||||
return
|
||||
|
||||
thread_env = dict(safe_runtime_env)
|
||||
thread_env["HERMES_HOME"] = str(profile_home_path)
|
||||
previous_thread_env = getattr(_thread_ctx, "env", {}).copy()
|
||||
home_override_token = None
|
||||
try:
|
||||
_set_thread_env(**thread_env)
|
||||
home_override_token = set_hermes_home_override(profile_home_path)
|
||||
yield
|
||||
finally:
|
||||
if home_override_token is not None:
|
||||
reset_hermes_home_override(home_override_token)
|
||||
if previous_thread_env:
|
||||
_set_thread_env(**previous_thread_env)
|
||||
else:
|
||||
_clear_thread_env()
|
||||
|
||||
|
||||
@contextmanager
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ except ImportError: # pragma: no cover - exercised only where fcntl is unavaila
|
|||
from api.config import (
|
||||
_PROVIDER_DISPLAY,
|
||||
_PROVIDER_MODELS,
|
||||
_thread_local_env_value,
|
||||
_custom_provider_slug_from_name,
|
||||
_get_label_for_model,
|
||||
_models_from_live_provider_ids,
|
||||
|
|
@ -1032,10 +1033,10 @@ def _provider_has_shadowed_codex_oauth_value(provider_id: str) -> bool:
|
|||
env_path = _get_hermes_home() / ".env"
|
||||
env_values = _load_env_file(env_path)
|
||||
values.append(env_values.get(env_var))
|
||||
values.append(os.getenv(env_var))
|
||||
values.append(_thread_local_env_value(env_var))
|
||||
for alias in _PROVIDER_ENV_VAR_ALIASES.get(provider_id, ()) or ():
|
||||
values.append(env_values.get(alias))
|
||||
values.append(os.getenv(alias))
|
||||
values.append(_thread_local_env_value(alias))
|
||||
|
||||
cfg = get_config()
|
||||
model_cfg = cfg.get("model", {})
|
||||
|
|
@ -1054,7 +1055,7 @@ def _provider_has_shadowed_codex_oauth_value(provider_id: str) -> bool:
|
|||
if isinstance(cp, dict) and _custom_provider_name_matches(provider_id, cp.get("name")):
|
||||
cp_key = cp.get("api_key")
|
||||
if isinstance(cp_key, str) and cp_key.startswith("${") and cp_key.endswith("}"):
|
||||
values.append(os.getenv(cp_key[2:-1]))
|
||||
values.append(_thread_local_env_value(cp_key[2:-1]))
|
||||
else:
|
||||
values.append(cp_key)
|
||||
return any(_looks_like_codex_oauth_token(str(value or "")) for value in values)
|
||||
|
|
@ -1175,7 +1176,7 @@ def _provider_has_key(provider_id: str) -> bool:
|
|||
env_file_value = env_values.get(env_var)
|
||||
if _provider_value_counts_as_api_key(provider_id, env_file_value):
|
||||
return True
|
||||
env_value = os.getenv(env_var)
|
||||
env_value = _thread_local_env_value(env_var)
|
||||
if _provider_value_counts_as_api_key(provider_id, env_value):
|
||||
return True
|
||||
# Fall back to legacy env-var aliases (e.g. lmstudio's pre-#1500
|
||||
|
|
@ -1184,7 +1185,7 @@ def _provider_has_key(provider_id: str) -> bool:
|
|||
for alias in _PROVIDER_ENV_VAR_ALIASES.get(provider_id, ()) or ():
|
||||
if _provider_value_counts_as_api_key(provider_id, env_values.get(alias)):
|
||||
return True
|
||||
if _provider_value_counts_as_api_key(provider_id, os.getenv(alias)):
|
||||
if _provider_value_counts_as_api_key(provider_id, _thread_local_env_value(alias)):
|
||||
return True
|
||||
# Check credential pool — covers custom providers registered via
|
||||
# `hermes auth add` which store keys in auth.json (not config.yaml).
|
||||
|
|
@ -1238,14 +1239,14 @@ def _get_provider_api_key(provider_id: str) -> str | None:
|
|||
env_file_value = env_values.get(env_var)
|
||||
if _provider_value_counts_as_api_key(provider_id, env_file_value):
|
||||
return str(env_file_value).strip() or None
|
||||
env_value = os.getenv(env_var)
|
||||
env_value = _thread_local_env_value(env_var)
|
||||
if _provider_value_counts_as_api_key(provider_id, env_value):
|
||||
return str(env_value).strip() or None
|
||||
for alias in _PROVIDER_ENV_VAR_ALIASES.get(provider_id, ()) or ():
|
||||
alias_file_value = env_values.get(alias)
|
||||
if _provider_value_counts_as_api_key(provider_id, alias_file_value):
|
||||
return str(alias_file_value).strip() or None
|
||||
alias_value = os.getenv(alias)
|
||||
alias_value = _thread_local_env_value(alias)
|
||||
if _provider_value_counts_as_api_key(provider_id, alias_value):
|
||||
return str(alias_value).strip() or None
|
||||
|
||||
|
|
@ -1273,7 +1274,7 @@ def _get_provider_api_key(provider_id: str) -> str | None:
|
|||
if _custom_provider_name_matches(provider_id, cp.get("name")):
|
||||
cp_key = str(cp.get("api_key") or "").strip()
|
||||
if cp_key.startswith("${") and cp_key.endswith("}"):
|
||||
return os.getenv(cp_key[2:-1], "").strip() or None
|
||||
return _thread_local_env_value(cp_key[2:-1]).strip() or None
|
||||
if _provider_value_counts_as_api_key(provider_id, cp_key):
|
||||
return cp_key
|
||||
# Fallback: try credential pool (e.g. bothub key stored via auth.json)
|
||||
|
|
@ -2382,7 +2383,7 @@ def get_providers() -> dict[str, Any]:
|
|||
env_values = _load_env_file(env_path)
|
||||
if _provider_value_counts_as_api_key(pid, env_values.get(env_var)):
|
||||
key_source = "env_file"
|
||||
elif _provider_value_counts_as_api_key(pid, os.getenv(env_var)):
|
||||
elif _provider_value_counts_as_api_key(pid, _thread_local_env_value(env_var)):
|
||||
key_source = "env_var"
|
||||
else:
|
||||
# Canonical name not set; check legacy aliases (e.g. lmstudio's
|
||||
|
|
@ -2395,7 +2396,7 @@ def get_providers() -> dict[str, Any]:
|
|||
key_source = "env_file"
|
||||
aliased = True
|
||||
break
|
||||
if _provider_value_counts_as_api_key(pid, os.getenv(alias)):
|
||||
if _provider_value_counts_as_api_key(pid, _thread_local_env_value(alias)):
|
||||
key_source = "env_var"
|
||||
aliased = True
|
||||
break
|
||||
|
|
@ -2592,7 +2593,7 @@ def get_providers() -> dict[str, Any]:
|
|||
# Replace env var reference to check actual value
|
||||
if cp_api_key.startswith("${") and cp_api_key.endswith("}"):
|
||||
env_var = cp_api_key[2:-1]
|
||||
cp_has_key = bool(os.getenv(env_var, "").strip())
|
||||
cp_has_key = bool(_thread_local_env_value(env_var).strip())
|
||||
# Fallback: check credential pool (key added via hermes auth add)
|
||||
if not cp_has_key:
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -120,7 +120,7 @@ def test_active_request_env_noop_for_default_profile(monkeypatch):
|
|||
|
||||
|
||||
def test_active_request_env_applies_named_profile_env(monkeypatch, tmp_path):
|
||||
"""A named profile's .env is applied inside the block and restored after."""
|
||||
"""A named profile's .env is bound to thread-local state, process env untouched."""
|
||||
base = tmp_path / ".hermes"
|
||||
(base / "profiles" / "work").mkdir(parents=True)
|
||||
(base / "profiles" / "work" / ".env").write_text(
|
||||
|
|
@ -130,14 +130,17 @@ def test_active_request_env_applies_named_profile_env(monkeypatch, tmp_path):
|
|||
monkeypatch.delenv("ISSUE_3957_PROBE", raising=False)
|
||||
|
||||
# Simulate the per-request cookie context (issue #798).
|
||||
monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env")
|
||||
profiles.set_request_profile("work")
|
||||
try:
|
||||
assert profiles.get_active_profile_name() == "work"
|
||||
assert os.environ.get("ISSUE_3957_PROBE") is None
|
||||
assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env"
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-work-profile"
|
||||
assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-work-profile"
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env"
|
||||
# Restored after the block exits.
|
||||
assert os.environ.get("ISSUE_3957_PROBE") is None
|
||||
assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env"
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env"
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
|
|
@ -151,18 +154,79 @@ def test_active_request_env_restores_on_exception(monkeypatch, tmp_path):
|
|||
)
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base)
|
||||
monkeypatch.delenv("ISSUE_3957_PROBE", raising=False)
|
||||
monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env")
|
||||
|
||||
profiles.set_request_profile("work")
|
||||
try:
|
||||
with_raised = False
|
||||
try:
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-work-profile"
|
||||
assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-work-profile"
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env"
|
||||
raise ValueError("boom")
|
||||
except ValueError:
|
||||
with_raised = True
|
||||
assert with_raised
|
||||
assert os.environ.get("ISSUE_3957_PROBE") is None
|
||||
assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env"
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env"
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
|
||||
def test_active_request_scope_prefers_profile_key_over_process_env_for_custom_provider(
|
||||
monkeypatch,
|
||||
tmp_path,
|
||||
):
|
||||
"""Profile-scope thread env resolves custom-provider env vars before process env."""
|
||||
base = tmp_path / ".hermes"
|
||||
(base / "profiles" / "work").mkdir(parents=True)
|
||||
(base / "profiles" / "work" / ".env").write_text(
|
||||
"ISSUE_3957_CUSTOM_KEY=from-work-profile\n", encoding="utf-8"
|
||||
)
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base)
|
||||
monkeypatch.setenv("ISSUE_3957_CUSTOM_KEY", "from-process-env")
|
||||
monkeypatch.setattr(
|
||||
config,
|
||||
"get_config",
|
||||
lambda: {
|
||||
"custom_providers": [
|
||||
{"name": "Team", "api_key": "${ISSUE_3957_CUSTOM_KEY}"}
|
||||
]
|
||||
},
|
||||
)
|
||||
|
||||
profiles.set_request_profile("work")
|
||||
try:
|
||||
assert config.resolve_custom_provider_connection("custom:team") == (
|
||||
"from-process-env",
|
||||
None,
|
||||
)
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
assert config.resolve_custom_provider_connection("custom:team") == (
|
||||
"from-work-profile",
|
||||
None,
|
||||
)
|
||||
assert os.environ.get("ISSUE_3957_CUSTOM_KEY") == "from-process-env"
|
||||
assert config._thread_local_env_value("ISSUE_3957_CUSTOM_KEY") == (
|
||||
"from-process-env"
|
||||
)
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
|
||||
def test_active_request_scope_sets_context_local_hermes_home(monkeypatch, tmp_path):
|
||||
"""Request scope keeps agent-side Hermes-home readers on the active profile."""
|
||||
from hermes_constants import get_hermes_home
|
||||
|
||||
base = tmp_path / ".hermes"
|
||||
work_home = base / "profiles" / "work"
|
||||
work_home.mkdir(parents=True)
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base)
|
||||
|
||||
profiles.set_request_profile("work")
|
||||
try:
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
assert get_hermes_home() == work_home
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
|
|
@ -249,3 +313,36 @@ def test_detached_worker_scope_binds_profile_on_new_thread(monkeypatch, tmp_path
|
|||
assert out["after_name"] == "models_cache.json" # restored
|
||||
assert out["after_env"] is None
|
||||
|
||||
|
||||
def test_detached_worker_prefers_profile_key_for_custom_provider(monkeypatch, tmp_path):
|
||||
"""Detached worker scope resolves custom-provider env from thread profile, not process env."""
|
||||
import threading
|
||||
|
||||
base = tmp_path / ".hermes"
|
||||
(base / "profiles" / "work").mkdir(parents=True)
|
||||
(base / "profiles" / "work" / ".env").write_text(
|
||||
"ISSUE_3957_CUSTOM_KEY=from-worker-profile\n", encoding="utf-8"
|
||||
)
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base)
|
||||
monkeypatch.setenv("ISSUE_3957_CUSTOM_KEY", "from-process-env")
|
||||
monkeypatch.setattr(
|
||||
config,
|
||||
"get_config",
|
||||
lambda: {
|
||||
"custom_providers": [
|
||||
{"name": "team", "api_key": "${ISSUE_3957_CUSTOM_KEY}"}
|
||||
]
|
||||
},
|
||||
)
|
||||
|
||||
out = {}
|
||||
|
||||
def worker():
|
||||
with profiles.profile_scope_for_detached_worker("work", "test-worker"):
|
||||
out["value"] = config.resolve_custom_provider_connection("custom:team")[0]
|
||||
|
||||
t = threading.Thread(target=worker)
|
||||
t.start()
|
||||
t.join()
|
||||
assert out["value"] == "from-worker-profile"
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue