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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user