From 321090efcc5493760ca3219868674d72bc6a283f Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 20 Jun 2026 02:33:54 -0400 Subject: [PATCH] fix(#3961): preserve mirrored live-model profile reads --- api/config.py | 2 +- api/profiles.py | 23 ++++++++++- api/routes.py | 8 ++-- ...test_issue3957_profile_providers_models.py | 40 +++++++++++++++---- 4 files changed, 59 insertions(+), 14 deletions(-) diff --git a/api/config.py b/api/config.py index c1f995f5..63435e3c 100644 --- a/api/config.py +++ b/api/config.py @@ -6415,7 +6415,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: try: from api.profiles import ( get_active_profile_name as _gapn, - profile_env_for_active_request as _prof_env_request, + profile_env_for_active_request_readonly as _prof_env_request, profile_scope_for_detached_worker as _prof_scope_worker, ) _active_profile_name = (_gapn() or "").strip() diff --git a/api/profiles.py b/api/profiles.py index 9d231d02..4b8950a3 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -901,7 +901,7 @@ def profile_env_for_background_worker( @contextmanager -def profile_env_for_active_request( +def profile_env_for_active_request_readonly( purpose: str = "provider/model read", logger_override: Optional[logging.Logger] = None, ): @@ -964,6 +964,27 @@ def profile_env_for_active_request( _clear_thread_env() +@contextmanager +def profile_env_for_active_request( + purpose: str = "active request", + logger_override: Optional[logging.Logger] = None, +): + """Apply the active per-request profile through the legacy mirrored path. + + Some request-scoped readers still delegate into Hermes helpers that resolve + credentials directly from process env or ``get_hermes_home()``. Those paths + stay on the mirrored scope until they are fully audited. + """ + 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 + ): + yield + + @contextmanager def profile_scope_for_detached_worker( profile_name, diff --git a/api/routes.py b/api/routes.py index a637ad56..a5a31bd1 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7773,8 +7773,8 @@ def handle_get(handler, parsed) -> bool: # profile's (#3957). Without this, get_auth_status() probes on a # non-default profile resolve the wrong/empty creds and can stall past # the 30s frontend timeout. No-op for the default profile. - from api.profiles import profile_env_for_active_request - with profile_env_for_active_request("/api/providers", logger_override=logger): + from api.profiles import profile_env_for_active_request_readonly + with profile_env_for_active_request_readonly("/api/providers", logger_override=logger): return j(handler, get_providers()) # ── Plugins/hooks visibility (read-only, no callback/source internals) ── @@ -7790,8 +7790,8 @@ def handle_get(handler, parsed) -> bool: # read/write runs under the process-default profile, so a multi-profile # client would see (and seed) the default profile's pool instead of its # own (#4247/#4067 profile-isolation class). - from api.profiles import profile_env_for_active_request - with profile_env_for_active_request("/api/provider/quota", logger_override=logger): + from api.profiles import profile_env_for_active_request_readonly + with profile_env_for_active_request_readonly("/api/provider/quota", logger_override=logger): return j(handler, get_provider_quota(provider_id, refresh=refresh)) if parsed.path == "/api/provider/cost-history": diff --git a/tests/test_issue3957_profile_providers_models.py b/tests/test_issue3957_profile_providers_models.py index 3c7e3e6b..02875d8f 100644 --- a/tests/test_issue3957_profile_providers_models.py +++ b/tests/test_issue3957_profile_providers_models.py @@ -19,7 +19,7 @@ The fix: - ``api.config._get_models_cache_path()`` returns a profile-keyed path (``models_cache..json`` for named profiles; unchanged ``models_cache.json`` for the default/root profile). - - ``api.profiles.profile_env_for_active_request()`` applies the active + - ``api.profiles.profile_env_for_active_request_readonly()`` applies the active per-request profile's env around the read; no-op for the default profile. """ @@ -113,7 +113,7 @@ def test_active_request_env_noop_for_default_profile(monkeypatch): monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "default") monkeypatch.setattr(profiles, "_is_root_profile", lambda n: n in ("", "default")) monkeypatch.delenv("ISSUE_3957_PROBE", raising=False) - with profiles.profile_env_for_active_request("test"): + with profiles.profile_env_for_active_request_readonly("test"): # No env mutation, no HERMES_HOME change for the default profile. assert os.environ.get("ISSUE_3957_PROBE") is None assert os.environ.get("ISSUE_3957_PROBE") is None @@ -135,7 +135,7 @@ def test_active_request_env_applies_named_profile_env(monkeypatch, tmp_path): try: assert profiles.get_active_profile_name() == "work" assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env" - with profiles.profile_env_for_active_request("test"): + with profiles.profile_env_for_active_request_readonly("test"): 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. @@ -160,7 +160,7 @@ def test_active_request_env_restores_on_exception(monkeypatch, tmp_path): try: with_raised = False try: - with profiles.profile_env_for_active_request("test"): + with profiles.profile_env_for_active_request_readonly("test"): 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") @@ -201,7 +201,7 @@ def test_active_request_scope_prefers_profile_key_over_process_env_for_custom_pr "from-process-env", None, ) - with profiles.profile_env_for_active_request("test"): + with profiles.profile_env_for_active_request_readonly("test"): assert config.resolve_custom_provider_connection("custom:team") == ( "from-work-profile", None, @@ -225,18 +225,41 @@ def test_active_request_scope_sets_context_local_hermes_home(monkeypatch, tmp_pa profiles.set_request_profile("work") try: - with profiles.profile_env_for_active_request("test"): + with profiles.profile_env_for_active_request_readonly("test"): assert get_hermes_home() == work_home finally: profiles.clear_request_profile() +def test_active_request_legacy_scope_still_mirrors_process_env(monkeypatch, tmp_path): + """Live-model request scope still mirrors env for agent-side readers.""" + base = tmp_path / ".hermes" + (base / "profiles" / "work").mkdir(parents=True) + (base / "profiles" / "work" / ".env").write_text( + "ISSUE_3957_PROBE=from-work-profile\n", encoding="utf-8" + ) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env") + + profiles.set_request_profile("work") + try: + with profiles.profile_env_for_active_request("test"): + assert os.environ.get("ISSUE_3957_PROBE") == "from-work-profile" + assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env" + finally: + profiles.clear_request_profile() + + def test_providers_and_models_routes_wrap_in_profile_env(): """The two read routes are profile-scoped for non-default profiles (#3957). Structural guard: a future refactor that drops the wiring would silently reintroduce the bug, so pin it at the source level. - - /api/providers wraps the synchronous read in profile_env_for_active_request. + - /api/providers and /api/provider/quota wrap the synchronous read in + profile_env_for_active_request_readonly. + - /api/models/live stays on the mirrored profile_env_for_active_request + path because provider_model_ids() still delegates into agent helpers + that read process env / HERMES_HOME directly. - /api/models relies on get_available_models() scoping its detached rebuild worker via profile_scope_for_detached_worker (the request-thread wrapper cannot reach the worker thread — Codex CORE finding). @@ -244,7 +267,8 @@ def test_providers_and_models_routes_wrap_in_profile_env(): routes_src = Path(profiles.__file__).resolve().parent.joinpath("routes.py").read_text( encoding="utf-8" ) - assert "profile_env_for_active_request" in routes_src + assert 'with profile_env_for_active_request("/api/models/live"' in routes_src + assert "profile_env_for_active_request_readonly" in routes_src config_src = Path(config.__file__).resolve().read_text(encoding="utf-8") assert "profile_scope_for_detached_worker" in config_src assert "_get_models_cache_path" in config_src