From 7befc1d2dd76f436758aeacfb625557ecefe798c Mon Sep 17 00:00:00 2001 From: aniruddhaadak80 Date: Mon, 24 Aug 2026 12:03:16 +0530 Subject: [PATCH] fix(gateway): route platform authorization reads through the profile secret scope Under gateway.multiplex_profiles, secondary profiles are constructed inside _profile_runtime_scope and their .env lives in the profile's secret scope - gateway/run.py explicitly does NOT mutate os.environ with it. Four adapters still read their AUTHORIZATION config via raw os.getenv, so every secondary profile either (a) silently missed its own env-only allowlists/policies (fail-closed: all DMs dropped at intake) or (b) inherited the default profile's GATEWAY_ALLOW_ALL_USERS=true / allowlists from the shared process env (fail-open admissions): - weixin.py: WEIXIN_DM_POLICY / WEIXIN_ALLOWED_USERS / WEIXIN_GROUP_ALLOWED_USERS / WEIXIN_ALLOW_ALL_USERS + GATEWAY_ALLOW_ALL_USERS in _open_dm_opted_in - yuanbao.py: YUANBAO_DM_POLICY / DM_ALLOW_FROM / GROUP_POLICY / GROUP_ALLOW_FROM / ALLOW_ALL_USERS (new _yb_secret helper; AccessPolicy hard-gates intake) - signal.py: SIGNAL_GROUP_ALLOWED_USERS / SIGNAL_ALLOWED_USERS (new _sig_secret helper; empty scoped group list previously meant "drop all groups" silently) - wecom/adapter.py: WECOM_DM_POLICY / WECOM_ALLOWED_USERS / WECOM_GROUP_POLICY / WECOM_ALLOW_ALL_USERS + GATEWAY_ALLOW_ALL_USERS - while credentials one line above already used _get_scoped_secret - gateway/run.py::_own_policy_open_startup_violation: the open-policy startup guard validated GATEWAY_ALLOW_ALL_USERS via raw os.getenv even though its sibling dm/group reads already used the scoped _getenv All reads now go through the canonical fail-closed scoped shape QQ's _resolve_qq_secret already used (scope hit wins; unscoped single-profile callers keep legacy os.environ behavior). Regression suite drives the real scope contextvar across all four helpers plus the admission gates and the startup guard, asserting both directions: profile values are visible under multiplex, default-profile values never leak. Fixes #93522 --- gateway/platforms/signal.py | 27 ++- gateway/platforms/weixin.py | 6 +- gateway/platforms/yuanbao.py | 38 +++- plugins/platforms/wecom/adapter.py | 6 +- tests/gateway/test_platform_authz_scope.py | 212 +++++++++++++++++++++ 5 files changed, 275 insertions(+), 14 deletions(-) create mode 100644 tests/gateway/test_platform_authz_scope.py diff --git a/gateway/platforms/signal.py b/gateway/platforms/signal.py index 2282193cc9..e3a0177e71 100644 --- a/gateway/platforms/signal.py +++ b/gateway/platforms/signal.py @@ -250,6 +250,26 @@ def validate_signal_config(config: PlatformConfig) -> bool: # Signal Adapter # --------------------------------------------------------------------------- +def _sig_secret(name: str, default: str = "") -> str: + """Resolve a per-profile ``SIGNAL_*`` setting honoring the active secret + scope (#93522). + + Under ``gateway.multiplex_profiles`` every secondary profile is + constructed inside ``_profile_runtime_scope`` (``gateway/run.py``) and + its ``.env`` lives in that scope — raw ``os.getenv`` misses it and + leaks the default profile's values instead. The primary/active profile + is constructed without a scope and legitimately owns ``os.environ`` + (same canonical shape as QQ's ``_resolve_qq_secret``). + """ + from agent.secret_scope import UnscopedSecretError, get_secret + + try: + val = get_secret(name, default) + except UnscopedSecretError: + val = os.getenv(name) + return val if val is not None else default + + class SignalAdapter(BasePlatformAdapter): """Signal messenger adapter using signal-cli HTTP daemon.""" @@ -268,7 +288,10 @@ class SignalAdapter(BasePlatformAdapter): self.ignore_stories = extra.get("ignore_stories", True) # Parse allowlists — group policy is derived from presence of group allowlist - group_allowed_str = os.getenv("SIGNAL_GROUP_ALLOWED_USERS", "") + # Scoped reads (#93522): allowlists are per-profile authorization + # config; raw os.getenv misses secondary profiles' .env values and + # leaks the default profile's list into them. + group_allowed_str = _sig_secret("SIGNAL_GROUP_ALLOWED_USERS", "") self.group_allow_from = set(_parse_comma_list(group_allowed_str)) # Mention filter — only respond in groups when the bot account is @mentioned. @@ -285,7 +308,7 @@ class SignalAdapter(BasePlatformAdapter): # every inbound DM from any contact gets a 👀 reaction). # "*" means all users allowed (open mode); empty means no restriction # recorded at adapter level (run.py still enforces auth separately). - dm_allowed_str = os.getenv("SIGNAL_ALLOWED_USERS", "*") + dm_allowed_str = _sig_secret("SIGNAL_ALLOWED_USERS", "*") self.dm_allow_from = set(_parse_comma_list(dm_allowed_str)) # HTTP client diff --git a/gateway/platforms/weixin.py b/gateway/platforms/weixin.py index 9828716e6c..5cefbffeb2 100644 --- a/gateway/platforms/weixin.py +++ b/gateway/platforms/weixin.py @@ -1537,9 +1537,11 @@ class WeixinAdapter(BasePlatformAdapter): await self.handle_message(event) def _open_dm_opted_in(self) -> bool: - if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}: + # Scoped reads (#93522): the default profile's allow-all flag must + # not leak into a multiplexed secondary profile's admission gate. + if (_wx_secret("GATEWAY_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}: return True - return os.getenv("WEIXIN_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"} + return (_wx_secret("WEIXIN_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"} def _is_dm_allowed(self, sender_id: str) -> bool: if self._dm_policy == "disabled": diff --git a/gateway/platforms/yuanbao.py b/gateway/platforms/yuanbao.py index b07b609fb7..7dc3f41b84 100644 --- a/gateway/platforms/yuanbao.py +++ b/gateway/platforms/yuanbao.py @@ -97,7 +97,6 @@ from gateway.platforms.yuanbao_proto import ( next_seq_no, ) from gateway.session import build_session_key -from gateway.authz_mixin import _platform_gate_env logger = logging.getLogger(__name__) @@ -1262,6 +1261,27 @@ class ChatRoutingMiddleware(InboundMiddleware): await next_fn() +def _yb_secret(name: str, default: Optional[str] = None) -> Optional[str]: + """Resolve a per-profile ``YUANBAO_*`` / gateway setting honoring the + active secret scope (#93522). + + Under ``gateway.multiplex_profiles`` every secondary profile is + constructed inside ``_profile_runtime_scope`` (``gateway/run.py``) and + its ``.env`` lives in that scope — raw ``os.getenv`` misses it and + leaks the default profile's values instead. The primary/active profile + is constructed without a scope and legitimately owns ``os.environ``, + so fall back to it there (same canonical shape as QQ's + ``_resolve_qq_secret``). + """ + from agent.secret_scope import UnscopedSecretError, get_secret + + try: + val = get_secret(name, default) + except UnscopedSecretError: + val = os.getenv(name) + return val if val is not None else default + + class AccessPolicy: """Platform-level DM / Group access control policy. @@ -1283,9 +1303,9 @@ class AccessPolicy: self._group_allow_from = group_allow_from def _open_dm_opted_in(self) -> bool: - if _platform_gate_env("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}: + if (_yb_secret("GATEWAY_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}: return True - return _platform_gate_env("YUANBAO_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"} + return (_yb_secret("YUANBAO_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"} def is_dm_allowed(self, sender_id: str) -> bool: """Strict DM authorization — pairing does not imply access.""" @@ -4943,27 +4963,29 @@ class YuanbaoAdapter(BasePlatformAdapter): # Reply-to dedup: inbound_msg_id -> expire_ts # ------------------------------------------------------------------ - # Access control policy (DM / Group) + # Access control policy (DM / Group) — scoped reads (#93522) # ------------------------------------------------------------------ dm_policy: str = ( _extra.get("dm_policy") - or _platform_gate_env("YUANBAO_DM_POLICY", "pairing") + or _yb_secret("YUANBAO_DM_POLICY") + or "pairing" ).strip().lower() _dm_allow_from_raw: str = ( _extra.get("dm_allow_from") - or _platform_gate_env("YUANBAO_DM_ALLOW_FROM", "") + or _yb_secret("YUANBAO_DM_ALLOW_FROM", "") ) dm_allow_from: list[str] = [x.strip() for x in _dm_allow_from_raw.split(",") if x.strip()] group_policy: str = ( _extra.get("group_policy") - or _platform_gate_env("YUANBAO_GROUP_POLICY", "pairing") + or _yb_secret("YUANBAO_GROUP_POLICY") + or "pairing" ).strip().lower() _group_allow_from_raw: str = ( _extra.get("group_allow_from") - or _platform_gate_env("YUANBAO_GROUP_ALLOW_FROM", "") + or _yb_secret("YUANBAO_GROUP_ALLOW_FROM", "") ) group_allow_from: list[str] = [x.strip() for x in _group_allow_from_raw.split(",") if x.strip()] diff --git a/plugins/platforms/wecom/adapter.py b/plugins/platforms/wecom/adapter.py index d05e7f0119..162d4a35bc 100644 --- a/plugins/platforms/wecom/adapter.py +++ b/plugins/platforms/wecom/adapter.py @@ -893,9 +893,11 @@ class WeComAdapter(BasePlatformAdapter): return True def _open_dm_opted_in(self) -> bool: - if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}: + # Scoped reads (#93522): the default profile's allow-all flag must + # not leak into a multiplexed secondary profile's admission gate. + if (_get_scoped_secret("GATEWAY_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"}: return True - return os.getenv("WECOM_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"} + return (_get_scoped_secret("WECOM_ALLOW_ALL_USERS", "") or "").lower() in {"true", "1", "yes"} def _is_dm_allowed(self, sender_id: str) -> bool: if self._dm_policy == "disabled": diff --git a/tests/gateway/test_platform_authz_scope.py b/tests/gateway/test_platform_authz_scope.py new file mode 100644 index 0000000000..73d54d4499 --- /dev/null +++ b/tests/gateway/test_platform_authz_scope.py @@ -0,0 +1,212 @@ +"""Platform authorization reads must honor the per-profile secret scope (#93522). + +Under ``gateway.multiplex_profiles`` every secondary profile is constructed +inside ``_profile_runtime_scope`` and its ``.env`` lives in that scope — +``gateway/run.py`` explicitly does NOT mutate ``os.environ`` with it. Raw +``os.getenv`` authorization reads therefore (a) silently miss the secondary +profile's own allowlists (fail-closed no-replies) and (b) leak the default +profile's ``GATEWAY_ALLOW_ALL_USERS`` / allowlists into secondary profiles +(fail-open admissions). + +Covers the scoped env helpers of weixin / yuanbao / signal / wecom plus the +``_open_dm_opted_in`` gates built on them. Canonical shape reference: +QQ's ``_resolve_qq_secret``. +""" + +import contextlib +import os + +import pytest + +from agent.secret_scope import reset_secret_scope, set_secret_scope + + +@contextlib.contextmanager +def _scope(secrets): + token = set_secret_scope(secrets) + try: + yield + finally: + reset_secret_scope(token) + + +@pytest.fixture +def profile_scope(): + """Install a secondary-profile secret scope; os.environ stays 'default'.""" + with _scope( + { + "WEIXIN_DM_POLICY": "allowlist", + "WEIXIN_ALLOWED_USERS": "wx-alice", + "WEIXIN_ALLOW_ALL_USERS": "", + "YUANBAO_DM_POLICY": "open", + "SIGNAL_ALLOWED_USERS": "sig-bob", + "WECOM_DM_POLICY": "allowlist", + "WECOM_ALLOWED_USERS": "wecom-carol", + } + ): + yield + + +# ───────────────────────────────────────────────────────────────────── +# Scoped helpers: read the scope, never leak os.environ past it +# ───────────────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "module_path,helper_name,var", + [ + ("gateway.platforms.weixin", "_wx_secret", "WEIXIN_ALLOWED_USERS"), + ("gateway.platforms.yuanbao", "_yb_secret", "YUANBAO_DM_POLICY"), + ("gateway.platforms.signal", "_sig_secret", "SIGNAL_ALLOWED_USERS"), + ("plugins.platforms.wecom.adapter", "_get_scoped_secret", "WECOM_ALLOWED_USERS"), + ], +) +def test_helper_reads_profile_scope_first(module_path, helper_name, var, monkeypatch): + """Scope value wins over anything in os.environ.""" + monkeypatch.setenv(var, "FROM-DEFAULT-ENV") + module = __import__(module_path, fromlist=[helper_name]) + helper = getattr(module, helper_name) + + with _scope({var: "from-profile-scope"}): + assert helper(var, "") == "from-profile-scope" + + +@pytest.fixture +def multiplex_on(monkeypatch): + """Simulate a multiplexed deployment: scoped misses fail closed. + + With multiplexing off, a scope miss deliberately falls through to + os.environ (the cron-overlay contract) — the cross-profile leak only + exists while the multiplexer is active, which is exactly the #93522 + scenario. + """ + monkeypatch.setattr("agent.secret_scope._MULTIPLEX_ACTIVE", True) + + +@pytest.mark.parametrize( + "module_path,helper_name", + [ + ("gateway.platforms.weixin", "_wx_secret"), + ("gateway.platforms.yuanbao", "_yb_secret"), + ("gateway.platforms.signal", "_sig_secret"), + ("plugins.platforms.wecom.adapter", "_get_scoped_secret"), + ], +) +def test_helper_does_not_leak_default_env_into_scoped_miss( + module_path, helper_name, multiplex_on, monkeypatch +): + """Scoped miss must return the default, NOT fall through to os.environ. + + This is the fail-closed half of #93522: the default profile's + ``*_ALLOW_ALL_USERS=true`` in process env must not answer a secondary + profile's gate. + """ + monkeypatch.setenv("WEIXIN_ALLOW_ALL_USERS", "true") + monkeypatch.setenv("YUANBAO_ALLOW_ALL_USERS", "true") + monkeypatch.setenv("SIGNAL_ALLOWED_USERS", "*") + monkeypatch.setenv("WECOM_ALLOW_ALL_USERS", "true") + + module = __import__(module_path, fromlist=[helper_name]) + helper = getattr(module, helper_name) + + with _scope({}): # scope installed, keys absent + assert helper("WEIXIN_ALLOW_ALL_USERS", "") == "" + assert helper("YUANBAO_ALLOW_ALL_USERS", "") == "" + assert helper("WECOM_ALLOW_ALL_USERS", "") == "" + # signal's "*" default means open at adapter level — the scoped + # read must still not see the unscoped value: + assert helper("SIGNAL_ALLOWED_USERS", "restricted-default") == "restricted-default" + + +@pytest.mark.parametrize( + "module_path,helper_name,var", + [ + ("gateway.platforms.weixin", "_wx_secret", "WEIXIN_ALLOWED_USERS"), + ("gateway.platforms.yuanbao", "_yb_secret", "YUANBAO_DM_POLICY"), + ("gateway.platforms.signal", "_sig_secret", "SIGNAL_ALLOWED_USERS"), + ("plugins.platforms.wecom.adapter", "_get_scoped_secret", "WECOM_ALLOWED_USERS"), + ], +) +def test_helper_falls_back_to_environ_without_scope(module_path, helper_name, var, monkeypatch): + """Single-profile (no scope installed): os.environ remains authoritative.""" + monkeypatch.setenv(var, "legacy-env-value") + module = __import__(module_path, fromlist=[helper_name]) + helper = getattr(module, helper_name) + assert helper(var, "") == "legacy-env-value" + + +# ───────────────────────────────────────────────────────────────────── +# Admission gates built on the helpers +# ───────────────────────────────────────────────────────────────────── + + +def test_weixin_open_gate_ignores_default_env_under_scope(profile_scope, multiplex_on, monkeypatch): + """Default profile sets GATEWAY_ALLOW_ALL_USERS=true; secondary scope does + NOT carry it — the gate must stay closed.""" + monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true") + + from gateway.platforms.weixin import WeixinAdapter + + adapter = WeixinAdapter.__new__(WeixinAdapter) + assert adapter._open_dm_opted_in() is False + + +def test_weixin_open_gate_honors_scope_opt_in(): + from gateway.platforms.weixin import WeixinAdapter + + with _scope({"WEIXIN_ALLOW_ALL_USERS": "yes"}): + adapter = WeixinAdapter.__new__(WeixinAdapter) + assert adapter._open_dm_opted_in() is True + + +def test_yuanbao_access_policy_gate_ignores_default_env(profile_scope, multiplex_on, monkeypatch): + monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true") + + from gateway.platforms.yuanbao import AccessPolicy + + policy = AccessPolicy( + dm_policy="open", dm_allow_from=[], group_policy="pairing", group_allow_from=[] + ) + assert policy._open_dm_opted_in() is False + + +def test_wecom_open_gate_ignores_default_env_under_scope(profile_scope, multiplex_on, monkeypatch): + monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true") + + from plugins.platforms.wecom.adapter import WeComAdapter + + adapter = WeComAdapter.__new__(WeComAdapter) + assert adapter._open_dm_opted_in() is False + + +def test_startup_guard_uses_scoped_gateway_flag(multiplex_on): + """_own_policy_open_startup_violation must consult the scope for + GATEWAY_ALLOW_ALL_USERS, not raw os.environ (#93522).""" + from gateway.platforms.base import Platform + from gateway.run import _own_policy_open_startup_violation + + class _PlatformConfig: + enabled = True + extra = {"dm_policy": "open"} + + class _Config: + platforms = {Platform.YUANBAO: _PlatformConfig()} + + cfg = _Config() + with _scope({"GATEWAY_ALLOW_ALL_USERS": "true"}): + # Scope says opted-in even though os.environ doesn't -> no violation. + assert _own_policy_open_startup_violation(cfg) is None + + with _scope({}): + # Scope lacks the opt-in while os.environ has it -> violation (the + # default profile's flag must not satisfy this profile's check). + monkey_env_backup = os.environ.get("GATEWAY_ALLOW_ALL_USERS") + os.environ["GATEWAY_ALLOW_ALL_USERS"] = "true" + try: + reason = _own_policy_open_startup_violation(cfg) + assert reason is not None and "yuanbao" in reason.lower() + finally: + if monkey_env_backup is None: + os.environ.pop("GATEWAY_ALLOW_ALL_USERS", None) + else: + os.environ["GATEWAY_ALLOW_ALL_USERS"] = monkey_env_backup