From cedf4a3d78675283fa93e4e6ea2d6212bf414667 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:50:16 +0530 Subject: [PATCH] fix(secret-scope): compose the managed .env into every profile secret scope Answers the P1 review on #111187: build_profile_secret_scope() held only /.env plus that profile's external-source snapshot, never the administrator-managed .env. The launch process applies that file LAST with override (_apply_managed_env), so a managed key beats the user's own value in os.environ. Under multiplex semantics get_secret() stops falling back to os.environ on a scope miss, so inside a routed cron fire (and equally inside a real multiplex gateway turn, which builds its scope through the same function via gateway/run.py::_load_profile_secret_scope) a managed-only credential resolved as absent and a managed-vs-user collision resolved to the USER value: reversed precedence. Fix at the source: build_profile_secret_scope() overlays load_managed_env() last, after the profile .env and external sources, skipping process-global names exactly as it does for the other two layers. Every multiplex-authoritative scope (gateway turn, routed desktop fire, external worker env build) is built here, so managed authority is composed once instead of restored per consumer. No generic ambient-env fallback is reintroduced: only the managed file's own keys enter the scope, and only with the managed file's values. Regression (parametrized, two invariants): inside a routed fire a managed-only key resolves through get_secret(); a managed-vs-user collision yields the managed value. --- agent/secret_scope.py | 10 +++++ ...est_cron_multiplex_desktop_ticker_scope.py | 39 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/agent/secret_scope.py b/agent/secret_scope.py index 1b35cc03b5..25961eb7d8 100644 --- a/agent/secret_scope.py +++ b/agent/secret_scope.py @@ -248,6 +248,16 @@ def build_profile_secret_scope(hermes_home: Path) -> Dict[str, str]: except Exception: external_secrets = {} secrets.update((k, v) for k, v in external_secrets.items() if not _is_global_env(k)) + # Administrator-managed ``.env`` LAST, with override: the launch process applies it that way + # (``env_loader._apply_managed_env``) so policy beats a user's own value. Under multiplex + # semantics ``get_secret`` never reads ``os.environ`` on a scope miss, so a scope built from + # the profile files alone would drop a managed-only credential and let the user's value win a + # managed-vs-user collision (#111187 review). Every multiplex-authoritative scope — gateway + # turn, routed cron fire, external worker — is built here, so managed authority is composed + # once, not restored by each consumer. + from hermes_cli.managed_scope import load_managed_env # fail-open: {} when no managed scope + + secrets.update((k, v) for k, v in load_managed_env().items() if not _is_global_env(k)) return secrets diff --git a/tests/cron/test_cron_multiplex_desktop_ticker_scope.py b/tests/cron/test_cron_multiplex_desktop_ticker_scope.py index cf21d4cbd6..9be477c8ca 100644 --- a/tests/cron/test_cron_multiplex_desktop_ticker_scope.py +++ b/tests/cron/test_cron_multiplex_desktop_ticker_scope.py @@ -196,3 +196,42 @@ def test_a_routed_profile_fire_runs_under_multiplex_semantics_for_exactly_its_sc assert routed_profile_fire() is False assert dict(os.environ) == environ_before + + +@pytest.mark.parametrize( + ("routed_env_line", "expected"), + [ + ("", "managed-key"), # managed-only credential: resolvable, not absent + ("ORG_API_KEY=user-key\n", "managed-key"), # managed-vs-user collision: policy wins + ], + ids=["managed-only", "managed-beats-user"], +) +def test_routed_fire_scope_carries_managed_env_authority(tmp_path, monkeypatch, routed_env_line, expected): + """Under multiplex semantics get_secret never falls back to os.environ, so the routed fire's + scope itself must carry the administrator-managed .env with the precedence _apply_managed_env + gives it in the launch process: a managed-only key is present and a managed value beats the + routed profile's own (#111187 review).""" + import cron.scheduler as scheduler + from agent import secret_scope + from cron.scheduler_provider import _profile_cron_scope + from hermes_cli import managed_scope + + launch, routed = tmp_path / "launch", tmp_path / "launch" / "profiles" / "ops" + managed = tmp_path / "managed" + for home in (launch, routed, managed): + (home / "cron").mkdir(parents=True) + (routed / ".env").write_text(routed_env_line, encoding="utf-8") + (managed / ".env").write_text("ORG_API_KEY=managed-key\n", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(launch)) + monkeypatch.setenv("HERMES_MANAGED_DIR", str(managed)) + monkeypatch.setenv("ORG_API_KEY", "managed-key") # what _apply_managed_env left in the launch env + managed_scope.invalidate_managed_cache() + secret_scope.set_multiplex_active(False) + + with _profile_cron_scope(routed): + tokens = scheduler._install_fire_secret_scope() + try: + assert secret_scope.is_multiplex_active() is True + assert secret_scope.get_secret("ORG_API_KEY") == expected + finally: + scheduler._reset_fire_secret_scope(tokens)