From 8ebc3d420fde1dda022ea787c7602c629bc7dd4b Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 15 Sep 2026 12:11:37 -0700 Subject: [PATCH] fix(feishu): warn once when the empty-allowlist default drops group messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under multiplex a secondary profile reads FEISHU_GROUP_POLICY from its own secret scope only (deliberate 0.21.3 isolation), so a profile whose .env carries no FEISHU_* policy keys falls back to `allowlist` with an empty FEISHU_ALLOWED_USERS and every human group message is rejected while DMs keep working. That deny was logged only at DEBUG, making it look like the events never arrived (#111420). Keep the scoped read as is — no environ fallthrough. Instead, the first group drop caused by the untouched allowlist default logs once at WARNING naming the chat and the keys to set (FEISHU_GROUP_POLICY / FEISHU_ALLOWED_USERS in the profile's own .env, or group_rules in its config.yaml). Operator-configured denies (populated allowlist, per-chat rule, non-allowlist policy) and later drops stay at DEBUG. The predicate lives in the topical sibling feishu_admission_diagnostics.py. Docs: the Group Message Policy section now states the per-profile read and where to put the keys under a multiplexed gateway. Co-authored-by: bear0328 Co-authored-by: NanPan <111261006+poijygfdyy@users.noreply.github.com> --- plugins/platforms/feishu/adapter.py | 22 ++++++ .../feishu/feishu_admission_diagnostics.py | 36 ++++++++++ .../test_feishu_group_policy_surfacing.py | 67 +++++++++++++++++++ website/docs/user-guide/messaging/feishu.md | 2 + 4 files changed, 127 insertions(+) create mode 100644 plugins/platforms/feishu/feishu_admission_diagnostics.py create mode 100644 tests/gateway/test_feishu_group_policy_surfacing.py diff --git a/plugins/platforms/feishu/adapter.py b/plugins/platforms/feishu/adapter.py index c27eadb143..e1106b2cbc 100644 --- a/plugins/platforms/feishu/adapter.py +++ b/plugins/platforms/feishu/adapter.py @@ -1998,6 +1998,8 @@ class FeishuAdapter(BasePlatformAdapter): reason = self._admit(sender, message) if reason is not None: logger.debug("[Feishu] dropping inbound event: %s", reason) + if reason == "group_policy_rejected": + self._warn_once_empty_allowlist_deny(getattr(message, "chat_id", "") or "") return await self._process_inbound_message( data=data, message=message, sender_id=getattr(sender, "sender_id", None), @@ -3282,6 +3284,26 @@ class FeishuAdapter(BasePlatformAdapter): return "group_policy_rejected" return None + # Class default so the first drop after construction is the one that warns. + _warned_empty_allowlist_deny = False + + def _warn_once_empty_allowlist_deny(self, chat_id: str) -> None: + """One WARNING when group traffic dies on the untouched allowlist default (#111420). + + The policy read is scoped per profile on purpose — never inherit another profile's + FEISHU_GROUP_POLICY here; only make the resulting deny visible above DEBUG. + """ + if self._warned_empty_allowlist_deny: + return + from plugins.platforms.feishu.feishu_admission_diagnostics import empty_allowlist_drop_warning + text = empty_allowlist_drop_warning( + chat_id=chat_id, group_rules=self._group_rules, + default_group_policy=self._default_group_policy, allowed_group_users=self._allowed_group_users, + ) + if text: + self._warned_empty_allowlist_deny = True + logger.warning(text) + def _require_mention_for(self, chat_id: str) -> bool: rule = self._group_rules.get(chat_id) if chat_id else None if rule and rule.require_mention is not None: diff --git a/plugins/platforms/feishu/feishu_admission_diagnostics.py b/plugins/platforms/feishu/feishu_admission_diagnostics.py new file mode 100644 index 0000000000..e221dc0b67 --- /dev/null +++ b/plugins/platforms/feishu/feishu_admission_diagnostics.py @@ -0,0 +1,36 @@ +"""Operator-facing diagnostics for Feishu inbound admission. + +Admission rejections are logged at DEBUG (busy groups with ``require_mention`` drop most +traffic by design). The one rejection that is almost never intended — the code-default +``allowlist`` policy with an empty ``FEISHU_ALLOWED_USERS`` — gets one WARNING that names +the keys to set, because under multiplex a secondary profile reads only its own ``.env`` +and a launch-profile ``FEISHU_GROUP_POLICY=open`` is (deliberately) not inherited. +""" + +from __future__ import annotations + +from typing import Any, Collection, Mapping, Optional + + +def empty_allowlist_drop_warning( + *, + chat_id: str, + group_rules: Mapping[str, Any], + default_group_policy: str, + allowed_group_users: Collection[str], +) -> Optional[str]: + """Message for a group drop caused by the empty-allowlist default, else None. + + A per-chat ``group_rules`` entry, a non-``allowlist`` policy or a populated allowlist + means the operator configured the deny; only the untouched default is surfaced. + """ + if chat_id in group_rules or default_group_policy != "allowlist" or allowed_group_users: + return None + return ( + f"[Feishu] Dropped a group message in chat {chat_id}: group policy is 'allowlist' and " + "FEISHU_ALLOWED_USERS is empty for this profile, so every human group message is rejected. " + "Set FEISHU_GROUP_POLICY=open or FEISHU_ALLOWED_USERS= in this profile's .env " + "(under multiplex each profile reads only its own .env; the default profile's value is not " + "inherited), or platforms.feishu.extra.group_rules in its config.yaml. " + "Further drops are logged at DEBUG." + ) diff --git a/tests/gateway/test_feishu_group_policy_surfacing.py b/tests/gateway/test_feishu_group_policy_surfacing.py new file mode 100644 index 0000000000..0509c5b745 --- /dev/null +++ b/tests/gateway/test_feishu_group_policy_surfacing.py @@ -0,0 +1,67 @@ +"""Operator-facing surfacing of the empty-allowlist default deny in Feishu group admission. + +Secret scoping is deliberate: a secondary multiplex profile never inherits the launch +profile's FEISHU_GROUP_POLICY. What must not happen is the resulting deny being visible only +at DEBUG (#111420) — the first such drop is a WARNING naming the keys to set. +""" + +from __future__ import annotations + +import asyncio +import logging +from types import SimpleNamespace + +from tests.gateway.feishu_helpers import ( + install_dedup_state, + make_adapter_skeleton, + make_message, + make_sender, +) + +_LOGGER = "plugins.platforms.feishu.adapter" + + +def _group_event(message_id: str, chat_id: str = "oc_chat") -> SimpleNamespace: + return SimpleNamespace( + event=SimpleNamespace( + sender=make_sender(open_id="ou_human"), + message=make_message(message_id=message_id, chat_type="group", chat_id=chat_id), + ) + ) + + +def _drops(adapter, caplog, *events) -> list[logging.LogRecord]: + with caplog.at_level(logging.DEBUG, logger=_LOGGER): + for ev in events: + asyncio.run(adapter._handle_message_event_data(ev)) + return [r for r in caplog.records if "group" in r.getMessage().lower() and "dropp" in r.getMessage().lower()] + + +def test_empty_allowlist_default_deny_warns_once_with_config_keys(caplog): + adapter = make_adapter_skeleton(group_policy="allowlist") + install_dedup_state(adapter) + + records = _drops(adapter, caplog, _group_event("om_1"), _group_event("om_2"), _group_event("om_3")) + + warnings = [r for r in records if r.levelno == logging.WARNING] + assert len(warnings) == 1, [r.getMessage() for r in records] + text = warnings[0].getMessage() + assert "FEISHU_ALLOWED_USERS" in text and "FEISHU_GROUP_POLICY" in text and "oc_chat" in text + # Later drops stay at DEBUG — the warning is a one-shot diagnostic, not per-message noise. + assert sum(r.levelno == logging.DEBUG for r in records) == 3 + + +def test_configured_group_deny_stays_at_debug(caplog): + """An operator-chosen deny (populated allowlist, or a per-chat rule) is not the misconfiguration.""" + adapter = make_adapter_skeleton(group_policy="allowlist") + install_dedup_state(adapter) + adapter._allowed_group_users = frozenset({"ou_someone_else"}) + + ruled = make_adapter_skeleton(group_policy="allowlist") + install_dedup_state(ruled) + from plugins.platforms.feishu.adapter import FeishuGroupRule + ruled._group_rules = {"oc_chat": FeishuGroupRule(policy="disabled", allowlist=set(), blacklist=set(), require_mention=None)} + + records = _drops(adapter, caplog, _group_event("om_a")) + _drops(ruled, caplog, _group_event("om_b")) + + assert records and all(r.levelno == logging.DEBUG for r in records), [r.getMessage() for r in records] diff --git a/website/docs/user-guide/messaging/feishu.md b/website/docs/user-guide/messaging/feishu.md index 7e7bbbde49..a483395f63 100644 --- a/website/docs/user-guide/messaging/feishu.md +++ b/website/docs/user-guide/messaging/feishu.md @@ -238,6 +238,8 @@ FEISHU_GROUP_POLICY=allowlist # default In all modes, the bot must be explicitly @mentioned (or @all) in the group before the message is processed. Direct messages always bypass this gate. +With the default `allowlist` policy and an empty `FEISHU_ALLOWED_USERS`, every human group message is rejected while DMs keep working. The first such drop is logged once at `WARNING` with the keys to set; later drops are `DEBUG`. Under a [multiplexed gateway](../multi-profile-gateways.md), each profile reads only its **own** `.env` — a `FEISHU_GROUP_POLICY=open` in the default profile's `.env` does not apply to a secondary profile's bot. Put `FEISHU_GROUP_POLICY` / `FEISHU_ALLOWED_USERS` in `profiles//.env`, or use `group_rules` in that profile's `config.yaml`. + Set `FEISHU_REQUIRE_MENTION=false` to let Hermes read all group traffic without requiring an @mention: ```bash