From e13b5e71ef7003389b0a01da1ff76e48f7fa9e89 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:18:59 +0530 Subject: [PATCH] fix(gateway): bind the secondary callback scope without on-loop secret hydration The adapter auth check is synchronous and runs on the adapter's event loop: once per inline-button tap, and once per keystroke for Telegram inline queries. Entering `_profile_runtime_scope` with its default `hydrate_secrets=True` there calls `hydrate_profile_secret_sources`, which takes the process-global secret-source lock and may resolve external secret backends. That lock contention is the heartbeat-starvation class #99519 moved off the loop. Startup and the per-profile message path already hydrate this profile's sources off-loop, so the callback now binds the scope with `hydrate_secrets=False` (the reconnect-retry precedent in the same module): `build_profile_secret_scope` still re-reads the profile's `.env` per call, so allowlist edits keep reaching the next tap. The regression test also seeds the default profile's allowlist (file and live os.environ) with a different user and asserts the secondary's bot refuses them; on main that user was admitted on the secondary's buttons. --- gateway/run_adapters.py | 16 +++++----- .../test_multiplex_interactive_auth.py | 29 ++++++++++++------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/gateway/run_adapters.py b/gateway/run_adapters.py index 350d0420f4..acab8f0318 100644 --- a/gateway/run_adapters.py +++ b/gateway/run_adapters.py @@ -1796,10 +1796,8 @@ class GatewayAdapterLifecycleMixin: """ from gateway.run import get_hermes_home transport_home = Path(get_hermes_home()) if self._multiplex_on() and profile_name is None else None - # Resolve the owning profile's home once; the scope itself is entered per call so a callback - # reads the same per-turn allowlist freshness (and external-secret hydration) as the message - # path — ``_make_profile_message_handler`` re-reads the profile's ``.env`` per message, and a - # snapshot built here would keep a tap denied after an operator edits that ``.env``. + # Resolved once; the scope is entered per call so an ``.env`` allowlist edit reaches the next + # tap, matching the message path's per-message re-read. profile_home = self._routed_profile_home(profile_name) if profile_name else None def check( @@ -1821,12 +1819,12 @@ class GatewayAdapterLifecycleMixin: if adapter is not None: source._transport_adapter_ref = _weakref.ref(adapter) if transport_home is None: - # Per call, like every sibling secondary handler: same ``.env`` freshness and hydration - # as the message path. ``_scope_or_null`` keeps the fail-closed behavior for an - # unresolvable profile home (bind nothing; an unscoped read raises instead of - # borrowing another profile's env). + # Sync, on the adapter's event loop (per tap, per inline-query keystroke): never + # hydrate external secret sources here — that takes the process-global source lock + # (#99519). Startup and the message path hydrate off-loop; this reads their cache. from gateway.run import _profile_runtime_scope - with self._scope_or_null(_profile_runtime_scope, profile_home): + with self._scope_or_null( + functools.partial(_profile_runtime_scope, hydrate_secrets=False), profile_home): return self._is_user_authorized(source) # Canonicalize FIRST (callback sources never went through ``build_source``): the routed # profile's pairing store is consulted, allowlists read under the transport home. diff --git a/tests/gateway/test_multiplex_interactive_auth.py b/tests/gateway/test_multiplex_interactive_auth.py index 86727ce7a9..a7c15407a3 100644 --- a/tests/gateway/test_multiplex_interactive_auth.py +++ b/tests/gateway/test_multiplex_interactive_auth.py @@ -20,8 +20,8 @@ def mux_home(tmp_path, monkeypatch): home = tmp_path / "hh" (home / "profiles" / "secondary").mkdir(parents=True) - (home / ".env").write_text("") - (home / "profiles" / "secondary" / ".env").write_text("") + (home / ".env").write_text("", encoding="utf-8") + (home / "profiles" / "secondary" / ".env").write_text("", encoding="utf-8") monkeypatch.setenv("HERMES_HOME", str(home)) for key in ( "TELEGRAM_ALLOWED_USERS", @@ -76,7 +76,7 @@ def test_routed_primary_callback_uses_routed_pairing_store_and_transport_allowli runner = _runner(mux_home) store = runner.pairing_stores["secondary"] store._save_json(store._approved_path("telegram"), {"777": {}}) - (mux_home / ".env").write_text("TELEGRAM_ALLOWED_USERS=999\n") + (mux_home / ".env").write_text("TELEGRAM_ALLOWED_USERS=999\n", encoding="utf-8") tg = _telegram(runner) # Paired only in the routed profile → allowed in the routed chat only. @@ -93,7 +93,7 @@ def test_bot_sender_reaches_allow_bots_policy_through_callback(mux_home): from gateway.run import _profile_runtime_scope runner = _runner(mux_home) - (mux_home / ".env").write_text("TELEGRAM_ALLOWED_USERS=999\nTELEGRAM_ALLOW_BOTS=all\n") + (mux_home / ".env").write_text("TELEGRAM_ALLOWED_USERS=999\nTELEGRAM_ALLOW_BOTS=all\n", encoding="utf-8") tg = _telegram(runner) def msg(uid, is_bot): @@ -110,13 +110,21 @@ def test_bot_sender_reaches_allow_bots_policy_through_callback(mux_home): assert tg._is_user_authorized_from_message(msg(4343, False)) is False -def test_secondary_owned_callback_reads_own_profile_allowlist(mux_home): +def test_secondary_owned_callback_reads_own_profile_allowlist(mux_home, monkeypatch): """#120639: a secondary profile's own bot authorizes inline-button callers against the OWNING profile's allowlist — the same ``.env`` scope the cold-path message handler reads — even though the callback fires outside - any profile runtime scope, straight off the adapter's event loop.""" + any profile runtime scope, straight off the adapter's event loop. The + default profile's allowlist never leaks in, and the sync check never + hydrates external secret sources on that loop (#99519 class).""" + import hermes_cli.env_loader as env_loader + + hydrated = [] + monkeypatch.setattr(env_loader, "hydrate_profile_secret_sources", hydrated.append) runner = _runner(mux_home) - (mux_home / "profiles" / "secondary" / ".env").write_text("TELEGRAM_ALLOWED_USERS=555\n") + (mux_home / ".env").write_text("TELEGRAM_ALLOWED_USERS=999\n", encoding="utf-8") + monkeypatch.setenv("TELEGRAM_ALLOWED_USERS", "999") # the default profile's live os.environ + (mux_home / "profiles" / "secondary" / ".env").write_text("TELEGRAM_ALLOWED_USERS=555\n", encoding="utf-8") tg = _telegram(runner) tg._hermes_profile_name = "secondary" runner._profile_adapters = {"secondary": {Platform.TELEGRAM: tg}} @@ -127,6 +135,7 @@ def test_secondary_owned_callback_reads_own_profile_allowlist(mux_home): # No ambient profile scope: the adapter event loop invokes the callback bare. assert tg._is_callback_user_authorized("555", chat_id="111", chat_type="private") is True assert tg._is_callback_user_authorized("999", chat_id="111", chat_type="private") is False + assert hydrated == [] def test_secondary_callback_allowlist_follows_env_edits(mux_home): @@ -137,7 +146,7 @@ def test_secondary_callback_allowlist_follows_env_edits(mux_home): ``.env`` per message), no allow/deny split until the adapter reconnects.""" runner = _runner(mux_home) env_path = mux_home / "profiles" / "secondary" / ".env" - env_path.write_text("TELEGRAM_ALLOWED_USERS=555\n") + env_path.write_text("TELEGRAM_ALLOWED_USERS=555\n", encoding="utf-8") tg = _telegram(runner) tg._hermes_profile_name = "secondary" runner._profile_adapters = {"secondary": {Platform.TELEGRAM: tg}} @@ -146,7 +155,7 @@ def test_secondary_callback_allowlist_follows_env_edits(mux_home): ) assert tg._is_callback_user_authorized("777", chat_id="111", chat_type="private") is False - env_path.write_text("TELEGRAM_ALLOWED_USERS=555,777\n") # operator adds a user at runtime + env_path.write_text("TELEGRAM_ALLOWED_USERS=555,777\n", encoding="utf-8") # operator adds a user at runtime assert tg._is_callback_user_authorized("777", chat_id="111", chat_type="private") is True @@ -161,7 +170,7 @@ def test_slack_interactive_auth_prefers_wired_profile_check(mux_home, monkeypatc runner = _runner(mux_home) runner.adapters = {} sec_home = mux_home / "profiles" / "secondary" - (sec_home / ".env").write_text("SLACK_ALLOWED_USERS=U_SEC\n") + (sec_home / ".env").write_text("SLACK_ALLOWED_USERS=U_SEC\n", encoding="utf-8") monkeypatch.setenv("SLACK_ALLOW_ALL_USERS", "true") def slack(with_check):