diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 31af977c4..0d141d0fc 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -1655,6 +1655,29 @@ class BasePlatformAdapter(ABC): """ return len + @property + def enforces_own_access_policy(self) -> bool: + """Whether this adapter gates inbound access before dispatch. + + Some adapters (WeCom, Weixin, Yuanbao, QQBot) implement a documented + config-driven access surface — ``dm_policy`` / ``group_policy`` / + ``allow_from`` / ``group_allow_from`` in ``PlatformConfig.extra`` — and + enforce it at intake: a message is dropped inside the adapter and never + reaches the gateway unless it already passed that policy. + + The gateway's env-based allowlist check runs *after* the adapter, so for + these platforms a message arriving at ``_is_user_authorized`` has, by + definition, already been authorized by the adapter. Without this flag the + gateway would then deny it again (no env allowlist → default deny), + silently breaking ``dm_policy: open`` and config-only allowlists. + + Adapters that own their access policy override this to return ``True``. + The gateway treats that as "already authorized at intake" and skips the + env-allowlist default-deny. Adapters that delegate access control to the + gateway leave it ``False`` (the default). + """ + return False + def supports_draft_streaming( self, chat_type: Optional[str] = None, diff --git a/gateway/platforms/qqbot/adapter.py b/gateway/platforms/qqbot/adapter.py index eecf4febf..5b4a396ed 100644 --- a/gateway/platforms/qqbot/adapter.py +++ b/gateway/platforms/qqbot/adapter.py @@ -269,6 +269,11 @@ class QQAdapter(BasePlatformAdapter): def name(self) -> str: return "QQBot" + @property + def enforces_own_access_policy(self) -> bool: + """QQBot gates DM/group access at intake via dm_policy/group_policy.""" + return True + # ------------------------------------------------------------------ # Connection lifecycle # ------------------------------------------------------------------ diff --git a/gateway/platforms/wecom.py b/gateway/platforms/wecom.py index 1569d5faf..c11756430 100644 --- a/gateway/platforms/wecom.py +++ b/gateway/platforms/wecom.py @@ -847,6 +847,11 @@ class WeComAdapter(BasePlatformAdapter): # Policy helpers # ------------------------------------------------------------------ + @property + def enforces_own_access_policy(self) -> bool: + """WeCom gates DM/group access at intake via dm_policy/group_policy.""" + return True + def _is_dm_allowed(self, sender_id: str) -> bool: if self._dm_policy == "disabled": return False diff --git a/gateway/platforms/weixin.py b/gateway/platforms/weixin.py index 26a8efd5b..025bf052c 100644 --- a/gateway/platforms/weixin.py +++ b/gateway/platforms/weixin.py @@ -1397,6 +1397,11 @@ class WeixinAdapter(BasePlatformAdapter): logger.info("[%s] inbound from=%s type=%s media=%d", self.name, _safe_id(sender_id), source.chat_type, len(media_paths)) await self.handle_message(event) + @property + def enforces_own_access_policy(self) -> bool: + """Weixin gates DM/group access at intake via dm_policy/group_policy.""" + return True + def _is_dm_allowed(self, sender_id: str) -> bool: if self._dm_policy == "disabled": return False diff --git a/gateway/platforms/yuanbao.py b/gateway/platforms/yuanbao.py index f6781fe3a..6dc54dbcd 100644 --- a/gateway/platforms/yuanbao.py +++ b/gateway/platforms/yuanbao.py @@ -4691,6 +4691,11 @@ class YuanbaoAdapter(BasePlatformAdapter): # Abstract method implementations # ------------------------------------------------------------------ + @property + def enforces_own_access_policy(self) -> bool: + """Yuanbao gates DM/group access at intake via dm_policy/group_policy.""" + return True + async def connect(self) -> bool: """Connect to Yuanbao WS gateway and authenticate. diff --git a/gateway/run.py b/gateway/run.py index e5d9095d2..584e72a5e 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -6542,6 +6542,25 @@ class GatewayRunner: return YuanbaoAdapter(config) return None + + def _adapter_enforces_own_access_policy(self, platform: Optional[Platform]) -> bool: + """Whether the adapter for *platform* gates access at intake itself. + + Mirrors ``BasePlatformAdapter.enforces_own_access_policy``. Adapters + such as WeCom, Weixin, Yuanbao, and QQBot evaluate their documented + ``dm_policy`` / ``group_policy`` / ``allow_from`` config before a + message is dispatched to the gateway, so a message that reaches + ``_is_user_authorized`` has already been authorized by the adapter. + Defaults to ``False`` when the adapter is unknown or doesn't expose + the flag. + """ + if not platform: + return False + adapter = self.adapters.get(platform) + if adapter is None: + return False + return bool(getattr(adapter, "enforces_own_access_policy", False)) + def _is_user_authorized(self, source: SessionSource) -> bool: """ Check if a user is authorized to use the bot. @@ -6681,6 +6700,15 @@ class GatewayRunner: global_allowlist = os.getenv("GATEWAY_ALLOWED_USERS", "").strip() if not platform_allowlist and not group_user_allowlist and not group_chat_allowlist and not global_allowlist: + # No env allowlists configured. Adapters that own their own + # config-driven access policy (dm_policy / group_policy / + # allow_from / group_allow_from) already gated this message at + # intake — it would not have reached the gateway otherwise — so + # honor that decision instead of falling through to the + # env-only default-deny below, which would silently break + # `dm_policy: open` and config-only allowlists. (#34515) + if self._adapter_enforces_own_access_policy(source.platform): + return True # No allowlists configured -- check global allow-all flag return os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"} @@ -6788,6 +6816,20 @@ class GatewayRunner: if config.unauthorized_dm_behavior != "pair": # non-default → explicit override return config.unauthorized_dm_behavior + # Config-driven dm_policy (WeCom / Weixin / Yuanbao / QQBot). An + # allowlist or disabled DM policy means the operator restricted access, + # so unauthorized DMs should be dropped silently rather than answered + # with a pairing code. An explicit pairing policy opts back into codes. + if platform and config and hasattr(config, "platforms"): + platform_cfg = config.platforms.get(platform) + extra = getattr(platform_cfg, "extra", None) if platform_cfg else None + if isinstance(extra, dict): + dm_policy = str(extra.get("dm_policy") or "").strip().lower() + if dm_policy == "pairing": + return "pair" + if dm_policy in {"allowlist", "disabled"}: + return "ignore" + # No explicit override. Fall back to allowlist-aware default: # if any allowlist is configured for this platform, silently drop # unauthorized messages instead of sending pairing codes. diff --git a/scripts/release.py b/scripts/release.py index b4d5c3239..6c255108c 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -59,6 +59,7 @@ AUTHOR_MAP = { "wangpuv@hotmail.com": "wangpuv", "202622897+ticketclosed-wontfix@users.noreply.github.com": "ticketclosed-wontfix", "wuxuebin1993@gmail.com": "victorGPT", + "frowte3k@gmail.com": "Frowtek", "211828103+julio-cloudvisor@users.noreply.github.com": "julio-cloudvisor", "17778+kweiner@users.noreply.github.com": "kweiner", "223516181+faisfamilytravel@users.noreply.github.com": "faisfamilytravel", diff --git a/tests/gateway/test_config_driven_access_policy.py b/tests/gateway/test_config_driven_access_policy.py new file mode 100644 index 000000000..8659fb884 --- /dev/null +++ b/tests/gateway/test_config_driven_access_policy.py @@ -0,0 +1,234 @@ +"""Tests for config-driven platform access policies at the gateway layer. + +Background (#34515): WeCom, Weixin, Yuanbao, and QQBot expose a documented +config-driven access surface (``dm_policy`` / ``group_policy`` / ``allow_from`` +/ ``group_allow_from`` in ``PlatformConfig.extra``) and enforce it at intake — +a message is dropped inside the adapter and never reaches the gateway unless it +already passed that policy. + +The gateway's env-based allowlist check (``_is_user_authorized``) runs *after* +the adapter. Before the fix it fell through to an env-only default-deny when no +``PLATFORM_ALLOWED_USERS`` env var was set, silently rejecting ``dm_policy: +open`` and config-only allowlists even though the adapter had already +authorized the sender. + +The fix is a single drift-proof contract: adapters that own their access policy +declare ``enforces_own_access_policy`` (a ``BasePlatformAdapter`` property, +default ``False``). The gateway trusts that flag and skips the env-only +default-deny for those platforms, rather than re-implementing each adapter's +policy logic a second time. +""" + +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.config import GatewayConfig, Platform, PlatformConfig +from gateway.session import SessionSource + + +# Platforms whose adapters own their access policy at intake. +_OWN_POLICY_PLATFORMS = [ + Platform.WECOM, + Platform.WEIXIN, + Platform.YUANBAO, + Platform.QQBOT, +] + + +def _clear_auth_env(monkeypatch) -> None: + for key in ( + "WECOM_ALLOWED_USERS", + "WEIXIN_ALLOWED_USERS", + "YUANBAO_ALLOWED_USERS", + "QQ_ALLOWED_USERS", + "QQ_GROUP_ALLOWED_USERS", + "TELEGRAM_ALLOWED_USERS", + "GATEWAY_ALLOWED_USERS", + "GATEWAY_ALLOW_ALL_USERS", + "WECOM_ALLOW_ALL_USERS", + "WEIXIN_ALLOW_ALL_USERS", + "YUANBAO_ALLOW_ALL_USERS", + "QQ_ALLOW_ALL_USERS", + ): + monkeypatch.delenv(key, raising=False) + + +def _make_runner(platform: Platform, config: GatewayConfig, *, enforces: bool): + """Build a bare GatewayRunner with one adapter for *platform*. + + ``enforces`` controls whether the adapter declares + ``enforces_own_access_policy`` — i.e. whether it owns its access gate. + """ + from gateway.run import GatewayRunner + + runner = object.__new__(GatewayRunner) + runner.config = config + adapter = SimpleNamespace(send=AsyncMock(), enforces_own_access_policy=enforces) + runner.adapters = {platform: adapter} + runner.pairing_store = MagicMock() + runner.pairing_store.is_approved.return_value = False + runner.pairing_store._is_rate_limited.return_value = False + return runner, adapter + + +def _source(platform: Platform, *, chat_type: str = "dm") -> SessionSource: + return SessionSource( + platform=platform, + user_id="some-user", + chat_id="some-chat", + user_name="tester", + chat_type=chat_type, + ) + + +# --------------------------------------------------------------------------- +# Layer 1: the base-class contract and per-adapter overrides +# --------------------------------------------------------------------------- + + +def test_base_adapter_defaults_to_not_owning_access_policy(): + """Adapters that don't override the property delegate to the gateway.""" + from gateway.platforms.base import BasePlatformAdapter + + # The default lives on the base property descriptor. + assert BasePlatformAdapter.enforces_own_access_policy.fget(object()) is False + + +@pytest.mark.parametrize( + "module_path, class_name", + [ + ("gateway.platforms.wecom", "WeComAdapter"), + ("gateway.platforms.weixin", "WeixinAdapter"), + ("gateway.platforms.yuanbao", "YuanbaoAdapter"), + ("gateway.platforms.qqbot.adapter", "QQAdapter"), + ], +) +def test_own_policy_adapters_declare_the_flag(module_path, class_name): + """The four config-policy adapters override the flag to True.""" + import importlib + + module = importlib.import_module(module_path) + adapter_cls = getattr(module, class_name) + # Property is overridden on the subclass and returns True regardless of + # instance state (it reflects a static capability, not runtime config). + value = adapter_cls.enforces_own_access_policy.fget(object.__new__(adapter_cls)) + assert value is True + + +# --------------------------------------------------------------------------- +# Layer 2: gateway trusts the adapter-enforced flag +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("platform", _OWN_POLICY_PLATFORMS) +def test_own_policy_platform_authorized_without_env_allowlist(monkeypatch, platform): + """A message reaching the gateway from an own-policy adapter is trusted. + + With no env allowlist set, the gateway must NOT default-deny — the adapter + already authorized the sender at intake (e.g. ``dm_policy: open``). + """ + _clear_auth_env(monkeypatch) + config = GatewayConfig( + platforms={platform: PlatformConfig(enabled=True, extra={"dm_policy": "open"})} + ) + runner, _adapter = _make_runner(platform, config, enforces=True) + + assert runner._is_user_authorized(_source(platform)) is True + + +@pytest.mark.parametrize("platform", _OWN_POLICY_PLATFORMS) +def test_own_policy_platform_authorized_for_group_chat(monkeypatch, platform): + """Group traffic from an own-policy adapter is trusted the same way.""" + _clear_auth_env(monkeypatch) + config = GatewayConfig( + platforms={platform: PlatformConfig(enabled=True, extra={"group_policy": "open"})} + ) + runner, _adapter = _make_runner(platform, config, enforces=True) + + assert runner._is_user_authorized(_source(platform, chat_type="group")) is True + + +def test_non_owning_platform_still_default_denies(monkeypatch): + """Adapters that don't own their policy keep the env-only default-deny.""" + _clear_auth_env(monkeypatch) + config = GatewayConfig( + platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="t")} + ) + runner, _adapter = _make_runner(Platform.TELEGRAM, config, enforces=False) + + assert runner._is_user_authorized(_source(Platform.TELEGRAM)) is False + + +def test_env_allowlist_still_takes_precedence_for_own_policy_platform(monkeypatch): + """When an env allowlist IS set, it governs — adapter trust is a fallback. + + The adapter-trust branch only fires when no env allowlist exists, so an + operator who sets ``WECOM_ALLOWED_USERS`` still gets env-based gating and + a non-listed user is denied. + """ + _clear_auth_env(monkeypatch) + monkeypatch.setenv("WECOM_ALLOWED_USERS", "allowed-user") + config = GatewayConfig( + platforms={Platform.WECOM: PlatformConfig(enabled=True, extra={"dm_policy": "open"})} + ) + runner, _adapter = _make_runner(Platform.WECOM, config, enforces=True) + + listed = SessionSource( + platform=Platform.WECOM, user_id="allowed-user", chat_id="c", + user_name="t", chat_type="dm", + ) + stranger = SessionSource( + platform=Platform.WECOM, user_id="stranger", chat_id="c", + user_name="t", chat_type="dm", + ) + assert runner._is_user_authorized(listed) is True + assert runner._is_user_authorized(stranger) is False + + +def test_unknown_adapter_does_not_crash_trust_check(monkeypatch): + """No adapter registered for the platform → safe default-deny.""" + _clear_auth_env(monkeypatch) + config = GatewayConfig(platforms={Platform.WECOM: PlatformConfig(enabled=True)}) + runner, _adapter = _make_runner(Platform.WECOM, config, enforces=True) + runner.adapters = {} # nothing registered + + assert runner._adapter_enforces_own_access_policy(Platform.WECOM) is False + assert runner._is_user_authorized(_source(Platform.WECOM)) is False + + +# --------------------------------------------------------------------------- +# Layer 3: unauthorized-DM behavior reads config dm_policy +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "dm_policy, expected", + [ + ("allowlist", "ignore"), + ("disabled", "ignore"), + ("pairing", "pair"), + ], +) +def test_unauthorized_dm_behavior_follows_config_dm_policy(monkeypatch, dm_policy, expected): + """A restrictive dm_policy drops unauthorized DMs; pairing opts back in.""" + _clear_auth_env(monkeypatch) + config = GatewayConfig( + platforms={Platform.WECOM: PlatformConfig(enabled=True, extra={"dm_policy": dm_policy})} + ) + runner, _adapter = _make_runner(Platform.WECOM, config, enforces=True) + + assert runner._get_unauthorized_dm_behavior(Platform.WECOM) == expected + + +def test_unauthorized_dm_behavior_open_policy_keeps_default(monkeypatch): + """``dm_policy: open`` is not restrictive → falls through to the default.""" + _clear_auth_env(monkeypatch) + config = GatewayConfig( + platforms={Platform.WECOM: PlatformConfig(enabled=True, extra={"dm_policy": "open"})} + ) + runner, _adapter = _make_runner(Platform.WECOM, config, enforces=True) + + # No allowlist + no restrictive policy → open-gateway pairing default. + assert runner._get_unauthorized_dm_behavior(Platform.WECOM) == "pair"