diff --git a/gateway/config.py b/gateway/config.py index 4021beede5..9cf4ec12f6 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -900,6 +900,12 @@ def load_gateway_config() -> GatewayConfig: if "dm_mention_threads" in matrix_cfg and not os.getenv("MATRIX_DM_MENTION_THREADS"): os.environ["MATRIX_DM_MENTION_THREADS"] = str(matrix_cfg["dm_mention_threads"]).lower() + # Feishu settings → env vars (env vars take precedence) + feishu_cfg = yaml_cfg.get("feishu", {}) + if isinstance(feishu_cfg, dict): + if "allow_bots" in feishu_cfg and not os.getenv("FEISHU_ALLOW_BOTS"): + os.environ["FEISHU_ALLOW_BOTS"] = str(feishu_cfg["allow_bots"]).lower() + except Exception as e: logger.warning( "Failed to process config.yaml — falling back to .env / gateway.json values. " diff --git a/gateway/platforms/feishu.py b/gateway/platforms/feishu.py index 7d25a227fc..8bc2ae816e 100644 --- a/gateway/platforms/feishu.py +++ b/gateway/platforms/feishu.py @@ -64,7 +64,7 @@ from dataclasses import dataclass, field from datetime import datetime from pathlib import Path from types import SimpleNamespace -from typing import Any, Dict, List, Optional, Sequence +from typing import Any, Dict, List, Literal, Optional, Sequence from urllib.error import HTTPError, URLError from urllib.parse import urlencode from urllib.request import Request, urlopen @@ -388,6 +388,8 @@ class FeishuAdapterSettings: admins: frozenset[str] = frozenset() default_group_policy: str = "" group_rules: Dict[str, FeishuGroupRule] = field(default_factory=dict) + allow_bots: str = "none" # "none" | "mentions" | "all" + require_mention: bool = True @dataclass @@ -397,6 +399,7 @@ class FeishuGroupRule: policy: str # "open" | "allowlist" | "blacklist" | "admin_only" | "disabled" allowlist: set[str] = field(default_factory=set) blacklist: set[str] = field(default_factory=set) + require_mention: Optional[bool] = None # None = inherit global @dataclass @@ -406,6 +409,40 @@ class FeishuBatchState: counts: Dict[str, int] = field(default_factory=dict) +# --------------------------------------------------------------------------- +# Admission: policy types +# --------------------------------------------------------------------------- + + +RejectReason = Literal[ + "self_echo", + "self_ids_unknown", + "bots_disabled", + "bot_not_mentioned", + "group_policy_rejected", +] + + +def _is_bot_sender(sender: Any) -> bool: + # receive_v1 docs say {user, bot}; accept "app" defensively. + return getattr(sender, "sender_type", "") in ("bot", "app") + + +def _sender_identity(sender: Any) -> frozenset: + # Take any non-empty id variant — tenant sender_id_type decides which are populated. + sid = getattr(sender, "sender_id", None) + if sid is None: + return frozenset() + return frozenset( + v for v in ( + getattr(sid, "open_id", None), + getattr(sid, "user_id", None), + getattr(sid, "union_id", None), + ) + if v + ) + + # --------------------------------------------------------------------------- # Markdown rendering helpers # --------------------------------------------------------------------------- @@ -1378,10 +1415,16 @@ class FeishuAdapter(BasePlatformAdapter): for chat_id, rule_cfg in raw_group_rules.items(): if not isinstance(rule_cfg, dict): continue + # Only override when the key is explicitly set — missing vs false + # must not collapse. + per_chat_require_mention: Optional[bool] = None + if "require_mention" in rule_cfg: + per_chat_require_mention = _to_boolean(rule_cfg.get("require_mention")) group_rules[str(chat_id)] = FeishuGroupRule( policy=str(rule_cfg.get("policy", "open")).strip().lower(), allowlist=set(str(u).strip() for u in rule_cfg.get("allowlist", []) if str(u).strip()), blacklist=set(str(u).strip() for u in rule_cfg.get("blacklist", []) if str(u).strip()), + require_mention=per_chat_require_mention, ) # Bot-level admins @@ -1391,6 +1434,16 @@ class FeishuAdapter(BasePlatformAdapter): # Default group policy (for groups not in group_rules) default_group_policy = str(extra.get("default_group_policy", "")).strip().lower() + # Env-only so adapter and gateway auth bypass share one source; yaml + # feishu.allow_bots is bridged to this env var at config load. + allow_bots = os.getenv("FEISHU_ALLOW_BOTS", "none").strip().lower() + if allow_bots not in ("none", "mentions", "all"): + logger.warning( + "[Feishu] Unknown allow_bots=%r, falling back to 'none'. Valid: none, mentions, all.", + allow_bots, + ) + allow_bots = "none" + return FeishuAdapterSettings( app_id=str(extra.get("app_id") or os.getenv("FEISHU_APP_ID", "")).strip(), app_secret=str(extra.get("app_secret") or os.getenv("FEISHU_APP_SECRET", "")).strip(), @@ -1447,6 +1500,10 @@ class FeishuAdapter(BasePlatformAdapter): admins=admins, default_group_policy=default_group_policy, group_rules=group_rules, + allow_bots=allow_bots, + require_mention=_to_boolean( + extra.get("require_mention", os.getenv("FEISHU_REQUIRE_MENTION", "true")) + ), ) def _apply_settings(self, settings: FeishuAdapterSettings) -> None: @@ -1477,6 +1534,8 @@ class FeishuAdapter(BasePlatformAdapter): self._ws_reconnect_interval = settings.ws_reconnect_interval self._ws_ping_interval = settings.ws_ping_interval self._ws_ping_timeout = settings.ws_ping_timeout + self._allow_bots = settings.allow_bots + self._require_mention = settings.require_mention def _build_event_handler(self) -> Any: if EventDispatcherHandler is None: @@ -2190,30 +2249,28 @@ class FeishuAdapter(BasePlatformAdapter): event = getattr(data, "event", None) message = getattr(event, "message", None) sender = getattr(event, "sender", None) - sender_id = getattr(sender, "sender_id", None) - if not message or not sender_id: - logger.debug("[Feishu] Dropping malformed inbound event: missing message or sender_id") + if not message or not sender or not getattr(sender, "sender_id", None): + logger.debug("[Feishu] Dropping malformed inbound event: missing message/sender") return message_id = getattr(message, "message_id", None) if not message_id or self._is_duplicate(message_id): logger.debug("[Feishu] Dropping duplicate/missing message_id: %s", message_id) return - if self._is_self_sent_bot_message(event): - logger.debug("[Feishu] Dropping self-sent bot event: %s", message_id) + + reason = self._admit(sender, message) + if reason is not None: + logger.debug("[Feishu] dropping inbound event: %s", reason) return chat_type = getattr(message, "chat_type", "p2p") - chat_id = getattr(message, "chat_id", "") or "" - if chat_type != "p2p" and not self._should_accept_group_message(message, sender_id, chat_id): - logger.debug("[Feishu] Dropping group message that failed mention/policy gate: %s", message_id) - return await self._process_inbound_message( data=data, message=message, - sender_id=sender_id, + sender_id=getattr(sender, "sender_id", None), chat_type=chat_type, message_id=message_id, + is_bot=_is_bot_sender(sender), ) def _on_message_read_event(self, data: P2ImMessageMessageReadV1) -> None: @@ -2390,10 +2447,11 @@ class FeishuAdapter(BasePlatformAdapter): msg = items[0] if items else None if not msg: return + # GET im/v1/messages returns sender.id=app_id for bot messages — + # peer bots and us share sender_type="app" but differ on app_id. sender = getattr(msg, "sender", None) - sender_type = str(getattr(sender, "sender_type", "") or "").lower() - if sender_type != "app": - return # only route reactions on our own bot messages + if str(getattr(sender, "id", "") or "") != self._app_id: + return # only route reactions on this bot's own messages chat_id = str(getattr(msg, "chat_id", "") or "") chat_type_raw = str(getattr(msg, "chat_type", "p2p") or "p2p") if not chat_id: @@ -2680,6 +2738,7 @@ class FeishuAdapter(BasePlatformAdapter): sender_id: Any, chat_type: str, message_id: str, + is_bot: bool = False, ) -> None: text, inbound_type, media_urls, media_types, mentions = await self._extract_message_content(message) @@ -2705,19 +2764,27 @@ class FeishuAdapter(BasePlatformAdapter): ) reply_to_text = await self._fetch_message_text(reply_to_message_id) if reply_to_message_id else None + sender_primary = ( + getattr(sender_id, "open_id", None) + or getattr(sender_id, "user_id", None) + or getattr(sender_id, "union_id", None) + or "" + ) logger.info( - "[Feishu] Inbound %s message received: id=%s type=%s chat_id=%s text=%r media=%d", + "[Feishu] Inbound %s message received: id=%s type=%s chat_id=%s sender=%s:%s text=%r media=%d", "dm" if chat_type == "p2p" else "group", message_id, inbound_type.value, getattr(message, "chat_id", "") or "", + "bot" if is_bot else "user", + sender_primary, text[:120], len(media_urls), ) chat_id = getattr(message, "chat_id", "") or "" chat_info = await self.get_chat_info(chat_id) - sender_profile = await self._resolve_sender_profile(sender_id) + sender_profile = await self._resolve_sender_profile(sender_id, is_bot=is_bot) source = self.build_source( chat_id=chat_id, chat_name=chat_info.get("name") or chat_id or "Feishu Chat", @@ -2726,6 +2793,7 @@ class FeishuAdapter(BasePlatformAdapter): user_name=sender_profile["user_name"], thread_id=getattr(message, "thread_id", None) or None, user_id_alt=sender_profile["user_id_alt"], + is_bot=is_bot, ) normalized = MessageEvent( text=text, @@ -3448,7 +3516,12 @@ class FeishuAdapter(BasePlatformAdapter): return "dm" return "group" - async def _resolve_sender_profile(self, sender_id: Any) -> Dict[str, Optional[str]]: + async def _resolve_sender_profile( + self, + sender_id: Any, + *, + is_bot: bool = False, + ) -> Dict[str, Optional[str]]: """Map Feishu's three-tier user IDs onto Hermes' SessionSource fields. Preference order for the primary ``user_id`` field: @@ -3465,7 +3538,11 @@ class FeishuAdapter(BasePlatformAdapter): union_id = getattr(sender_id, "union_id", None) or None # Prefer tenant-scoped user_id; fall back to app-scoped open_id. primary_id = user_id or open_id - display_name = await self._resolve_sender_name_from_api(primary_id or union_id) + # bot/v3/bots/basic_batch only accepts open_id. + name_lookup_id = open_id if is_bot else (primary_id or union_id) + display_name = await self._resolve_sender_name_from_api( + name_lookup_id, is_bot=is_bot, + ) return { "user_id": primary_id, "user_name": display_name, @@ -3485,11 +3562,14 @@ class FeishuAdapter(BasePlatformAdapter): self._sender_name_cache.pop(sender_id, None) return None - async def _resolve_sender_name_from_api(self, sender_id: Optional[str]) -> Optional[str]: - """Fetch the sender's display name from the Feishu contact API with a 10-minute cache. - - ID-type detection mirrors openclaw: ou_ → open_id, on_ → union_id, else user_id. - Failures are silently suppressed; the message pipeline must not block on name resolution. + async def _resolve_sender_name_from_api( + self, + sender_id: Optional[str], + *, + is_bot: bool = False, + ) -> Optional[str]: + """Bots divert to bot/basic_batch — contact API doesn't return bot names. + Failures are silent so the pipeline never blocks on name resolution. """ if not sender_id or not self._client: return None @@ -3499,7 +3579,16 @@ class FeishuAdapter(BasePlatformAdapter): now = time.time() cached_name = self._get_cached_sender_name(trimmed) if cached_name is not None: - return cached_name + return cached_name or None # "" cached means "known nameless" + if is_bot: + names = await self._fetch_bot_names([trimmed]) + if names is None: + return None + expire_at = now + _FEISHU_SENDER_NAME_TTL_SECONDS + for oid, name in names.items(): + self._sender_name_cache[oid] = (name, expire_at) + hit = self._sender_name_cache.get(trimmed) + return (hit[0] or None) if hit else None try: from lark_oapi.api.contact.v3 import GetUserRequest # lazy import if trimmed.startswith("ou_"): @@ -3528,6 +3617,35 @@ class FeishuAdapter(BasePlatformAdapter): logger.debug("[Feishu] Failed to resolve sender name for %s", sender_id, exc_info=True) return None + async def _fetch_bot_names(self, bot_ids: List[str]) -> Optional[Dict[str, str]]: + if not self._client or not bot_ids: + return None + try: + req = ( + BaseRequest.builder() + .http_method(HttpMethod.GET) + .uri("/open-apis/bot/v3/bots/basic_batch") + .queries([("bot_ids", oid) for oid in bot_ids]) + .token_types({AccessTokenType.TENANT}) + .build() + ) + resp = await asyncio.to_thread(self._client.request, req) + content = getattr(getattr(resp, "raw", None), "content", None) + if not content: + return None + payload = json.loads(content) + if payload.get("code") != 0: + return None + bots = (payload.get("data") or {}).get("bots") or {} + return { + oid: str(info.get("name") or "").strip() + for oid, info in bots.items() + if oid + } + except Exception: + logger.debug("[Feishu] Failed to fetch bot names for %s", bot_ids, exc_info=True) + return None + async def _fetch_message_text(self, message_id: str) -> Optional[str]: if not self._client or not message_id: return None @@ -3591,10 +3709,60 @@ class FeishuAdapter(BasePlatformAdapter): logger.exception("[Feishu] Background inbound processing failed") # ========================================================================= - # Group policy and mention gating + # Inbound admission # ========================================================================= - def _allow_group_message(self, sender_id: Any, chat_id: str = "") -> bool: + def _admit(self, sender: Any, message: Any) -> Optional[RejectReason]: + sender_ids = _sender_identity(sender) + self_ids = frozenset(v for v in (self._bot_open_id, self._bot_user_id) if v) + is_bot = _is_bot_sender(sender) + is_group = getattr(message, "chat_type", "p2p") != "p2p" + chat_id = getattr(message, "chat_id", "") or "" + require_mention = is_group and self._require_mention_for(chat_id) + + # Defensive only — Feishu doesn't echo our outbound back as inbound, + # and open_id is always populated on both sides. + if self_ids and sender_ids & self_ids: + return "self_echo" + + if is_bot: + mode = self._allow_bots + if mode != "mentions" and mode != "all": + return "bots_disabled" + # Defensive: pre-hydration or malformed payloads. + if not self_ids or not sender_ids: + return "self_ids_unknown" + # Step 4 covers mention enforcement for groups when require_mention + # is on; check here only on paths step 4 won't reach. + if mode == "mentions" and not require_mention and not self._mentions_self(message): + return "bot_not_mentioned" + + if not is_group: + return None + + if not self._allow_group_message( + getattr(sender, "sender_id", None), chat_id, is_bot=is_bot, + ): + return "group_policy_rejected" + if require_mention and not self._mentions_self(message): + return "group_policy_rejected" + return None + + 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: + return rule.require_mention + return self._require_mention + + # --- Group policy --------------------------------------------------------- + + def _allow_group_message( + self, + sender_id: Any, + chat_id: str = "", + *, + is_bot: bool = False, + ) -> bool: """Per-group policy gate for non-DM traffic.""" sender_open_id = getattr(sender_id, "open_id", None) sender_user_id = getattr(sender_id, "user_id", None) @@ -3613,12 +3781,17 @@ class FeishuAdapter(BasePlatformAdapter): allowlist = self._allowed_group_users blacklist = set() + # Channel locks apply to everyone; allowlist/blacklist only gate humans + # (bots were already cleared upstream by FEISHU_ALLOW_BOTS). if policy == "disabled": return False if policy == "open": return True if policy == "admin_only": return False + if is_bot: + return True + if policy == "allowlist": return bool(sender_ids and (sender_ids & allowlist)) if policy == "blacklist": @@ -3626,17 +3799,16 @@ class FeishuAdapter(BasePlatformAdapter): return bool(sender_ids and (sender_ids & self._allowed_group_users)) - def _should_accept_group_message(self, message: Any, sender_id: Any, chat_id: str = "") -> bool: - """Require an explicit @mention before group messages enter the agent.""" - if not self._allow_group_message(sender_id, chat_id): - return False - # @_all is Feishu's @everyone placeholder — always route to the bot. + # --- Mention detection ---------------------------------------------------- + + def _mentions_self(self, message: Any) -> bool: + # @_all is Feishu's @everyone placeholder. raw_content = getattr(message, "content", "") or "" if "@_all" in raw_content: return True mentions = getattr(message, "mentions", None) or [] - if mentions: - return self._message_mentions_bot(mentions) + if mentions and self._message_mentions_bot(mentions): + return True normalized = normalize_feishu_message( message_type=getattr(message, "message_type", "") or "", raw_content=raw_content, @@ -3645,23 +3817,6 @@ class FeishuAdapter(BasePlatformAdapter): ) return self._post_mentions_bot(normalized.mentions) - def _is_self_sent_bot_message(self, event: Any) -> bool: - """Return True only for Feishu events emitted by this Hermes bot.""" - sender = getattr(event, "sender", None) - sender_type = str(getattr(sender, "sender_type", "") or "").strip().lower() - if sender_type not in {"bot", "app"}: - return False - - sender_id = getattr(sender, "sender_id", None) - sender_open_id = str(getattr(sender_id, "open_id", "") or "").strip() - sender_user_id = str(getattr(sender_id, "user_id", "") or "").strip() - - if self._bot_open_id and sender_open_id == self._bot_open_id: - return True - if self._bot_user_id and sender_user_id == self._bot_user_id: - return True - return False - def _message_mentions_bot(self, mentions: List[Any]) -> bool: # IDs trump names: when both sides have open_id (or both user_id), # match requires equal IDs. Name fallback only when either side diff --git a/gateway/run.py b/gateway/run.py index 78bd7139be..c7ed455001 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -3958,6 +3958,11 @@ class GatewayRunner: Platform.QQBOT: "QQ_ALLOW_ALL_USERS", Platform.YUANBAO: "YUANBAO_ALLOW_ALL_USERS", } + # Bots admitted by {PLATFORM}_ALLOW_BOTS bypass the human allowlist (#4466). + platform_allow_bots_map = { + Platform.DISCORD: "DISCORD_ALLOW_BOTS", + Platform.FEISHU: "FEISHU_ALLOW_BOTS", + } # Plugin platforms: check the registry for auth env var names if source.platform not in platform_env_map: @@ -3977,14 +3982,9 @@ class GatewayRunner: if platform_allow_all_var and os.getenv(platform_allow_all_var, "").lower() in ("true", "1", "yes"): return True - # Discord bot senders that passed the DISCORD_ALLOW_BOTS platform - # filter are already authorized at the platform level — skip the - # user allowlist. Without this, bot messages allowed by - # DISCORD_ALLOW_BOTS=mentions/all would be rejected here with - # "Unauthorized user" (fixes #4466). - if source.platform == Platform.DISCORD and getattr(source, "is_bot", False): - allow_bots = os.getenv("DISCORD_ALLOW_BOTS", "none").lower().strip() - if allow_bots in ("mentions", "all"): + if getattr(source, "is_bot", False): + allow_bots_var = platform_allow_bots_map.get(source.platform) + if allow_bots_var and os.getenv(allow_bots_var, "none").lower().strip() in ("mentions", "all"): return True # Discord role-based access (DISCORD_ALLOWED_ROLES): the adapter's diff --git a/tests/gateway/feishu_helpers.py b/tests/gateway/feishu_helpers.py new file mode 100644 index 0000000000..753a61a70a --- /dev/null +++ b/tests/gateway/feishu_helpers.py @@ -0,0 +1,65 @@ +"""Shared fixtures for Feishu adapter tests (admission, group policy, dispatch).""" + +from __future__ import annotations + +import threading +from types import SimpleNamespace +from typing import Any, Optional + + +def make_sender(sender_type: str = "user", open_id: str = "ou_human", + user_id: Optional[str] = None, union_id: Optional[str] = None) -> Any: + return SimpleNamespace( + sender_type=sender_type, + sender_id=SimpleNamespace(open_id=open_id, user_id=user_id, union_id=union_id), + ) + + +def make_message(message_id: str = "om_xxx", chat_type: str = "p2p", + chat_id: str = "oc_1", mentions: Optional[list] = None) -> Any: + return SimpleNamespace( + message_id=message_id, + chat_type=chat_type, + chat_id=chat_id, + mentions=mentions, + content="", + message_type="text", + ) + + +def make_adapter_skeleton( + *, + bot_open_id: str = "ou_me", + bot_user_id: str = "", + allow_bots: str = "none", + require_mention: bool = True, + group_policy: str = "allowlist", +) -> Any: + from gateway.platforms.feishu import FeishuAdapter + + adapter = object.__new__(FeishuAdapter) + adapter._bot_open_id = bot_open_id + adapter._bot_user_id = bot_user_id + adapter._bot_name = "" + adapter._app_id = "" + adapter._admins = set() + adapter._group_rules = {} + adapter._group_policy = group_policy + adapter._default_group_policy = group_policy + adapter._allowed_group_users = frozenset() + adapter._allow_bots = allow_bots + adapter._require_mention = require_mention + return adapter + + +def install_dedup_state(adapter: Any, seen: Optional[dict] = None) -> None: + adapter._seen_message_ids = dict(seen) if seen else {} + adapter._seen_message_order = list((seen or {}).keys()) + adapter._dedup_cache_size = 100 + adapter._dedup_lock = threading.Lock() + adapter._dedup_state_path = None + adapter._persist_seen_message_ids = lambda: None + + +def stub_mention(adapter: Any, mentions_self: bool) -> None: + adapter._mentions_self = lambda _message: mentions_self diff --git a/tests/gateway/test_config.py b/tests/gateway/test_config.py index f68ac72ed2..669545c8f4 100644 --- a/tests/gateway/test_config.py +++ b/tests/gateway/test_config.py @@ -360,6 +360,38 @@ class TestLoadGatewayConfig: "C01ABC": "Code review mode", } + def test_bridges_feishu_allow_bots_from_config_yaml_to_env(self, tmp_path, monkeypatch): + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + config_path = hermes_home / "config.yaml" + config_path.write_text( + "feishu:\n allow_bots: mentions\n", + encoding="utf-8", + ) + + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.delenv("FEISHU_ALLOW_BOTS", raising=False) + + load_gateway_config() + + assert os.environ.get("FEISHU_ALLOW_BOTS") == "mentions" + + def test_feishu_allow_bots_env_takes_precedence_over_config_yaml(self, tmp_path, monkeypatch): + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + config_path = hermes_home / "config.yaml" + config_path.write_text( + "feishu:\n allow_bots: all\n", + encoding="utf-8", + ) + + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("FEISHU_ALLOW_BOTS", "none") + + load_gateway_config() + + assert os.environ.get("FEISHU_ALLOW_BOTS") == "none" + def test_invalid_quick_commands_in_config_yaml_are_ignored(self, tmp_path, monkeypatch): hermes_home = tmp_path / ".hermes" hermes_home.mkdir() diff --git a/tests/gateway/test_feishu.py b/tests/gateway/test_feishu.py index f21b7dcef8..ea5a805729 100644 --- a/tests/gateway/test_feishu.py +++ b/tests/gateway/test_feishu.py @@ -8,6 +8,7 @@ import time import unittest from pathlib import Path from types import SimpleNamespace +from typing import Dict from unittest.mock import AsyncMock, Mock, patch from gateway.platforms.base import ProcessingOutcome @@ -557,6 +558,16 @@ class TestAdapterModule(unittest.TestCase): self.assertEqual(fake_client._ping_interval, 4) +def _admits_group(adapter, message, sender_id, chat_id=""): + """Group-path shim: run a message through ``_admit`` and return a bool.""" + sender = SimpleNamespace(sender_type="user", sender_id=sender_id) + if not hasattr(message, "chat_type"): + message.chat_type = "group" + if chat_id: + message.chat_id = chat_id + return adapter._admit(sender, message) is None + + class TestAdapterBehavior(unittest.TestCase): @patch.dict(os.environ, {}, clear=True) def test_build_event_handler_registers_reaction_and_card_processors(self): @@ -689,6 +700,67 @@ class TestAdapterBehavior(unittest.TestCase): adapter._on_reaction_event("im.message.reaction.created_v1", data) run_threadsafe.assert_called_once() + def _build_reaction_adapter(self, *, msg_sender_id: str): + """Build a FeishuAdapter wired up to return a single GET-message result.""" + from gateway.config import PlatformConfig + from gateway.platforms.feishu import FeishuAdapter + + adapter = FeishuAdapter(PlatformConfig()) + adapter._app_id = "cli_self_app" + adapter._bot_open_id = "ou_self_bot" + adapter._bot_user_id = "u_self_bot" + + msg = SimpleNamespace( + sender=SimpleNamespace(sender_type="app", id=msg_sender_id, id_type="app_id"), + chat_id="oc_chat", + chat_type="group", + ) + response = SimpleNamespace(success=lambda: True, data=SimpleNamespace(items=[msg])) + adapter._client = SimpleNamespace( + im=SimpleNamespace( + v1=SimpleNamespace(message=SimpleNamespace(get=Mock(return_value=response))) + ) + ) + adapter._build_get_message_request = Mock(return_value=object()) + adapter._handle_message_with_guards = AsyncMock() + adapter._resolve_sender_profile = AsyncMock( + return_value={"user_id": "u_human", "user_name": "Human", "user_id_alt": None} + ) + adapter.get_chat_info = AsyncMock(return_value={"name": "Test Chat"}) + return adapter + + @patch.dict(os.environ, {}, clear=True) + def test_reaction_on_peer_bot_message_is_not_routed(self): + # GET im/v1/messages sender for bot messages carries id=app_id; a peer + # bot's message has a different app_id than ours, so it must be dropped. + adapter = self._build_reaction_adapter(msg_sender_id="cli_peer_app") + + event = SimpleNamespace( + message_id="om_peer_msg", + user_id=SimpleNamespace(open_id="ou_human", user_id=None, union_id=None), + reaction_type=SimpleNamespace(emoji_type="THUMBSUP"), + ) + data = SimpleNamespace(event=event) + asyncio.run( + adapter._handle_reaction_event("im.message.reaction.created_v1", data) + ) + adapter._handle_message_with_guards.assert_not_awaited() + + @patch.dict(os.environ, {}, clear=True) + def test_reaction_on_our_own_bot_message_is_routed(self): + adapter = self._build_reaction_adapter(msg_sender_id="cli_self_app") + + event = SimpleNamespace( + message_id="om_self_msg", + user_id=SimpleNamespace(open_id="ou_human", user_id=None, union_id=None), + reaction_type=SimpleNamespace(emoji_type="THUMBSUP"), + ) + data = SimpleNamespace(event=event) + asyncio.run( + adapter._handle_reaction_event("im.message.reaction.created_v1", data) + ) + adapter._handle_message_with_guards.assert_awaited_once() + @patch.dict(os.environ, {"FEISHU_GROUP_POLICY": "open"}, clear=True) def test_group_message_requires_mentions_even_when_policy_open(self): from gateway.config import PlatformConfig @@ -697,10 +769,10 @@ class TestAdapterBehavior(unittest.TestCase): adapter = FeishuAdapter(PlatformConfig()) message = SimpleNamespace(mentions=[]) sender_id = SimpleNamespace(open_id="ou_any", user_id=None) - self.assertFalse(adapter._should_accept_group_message(message, sender_id, "")) + self.assertFalse(_admits_group(adapter, message, sender_id, "")) message_with_mention = SimpleNamespace(mentions=[SimpleNamespace(key="@_user_1")]) - self.assertFalse(adapter._should_accept_group_message(message_with_mention, sender_id, "")) + self.assertFalse(_admits_group(adapter, message_with_mention, sender_id, "")) @patch.dict(os.environ, {"FEISHU_GROUP_POLICY": "open"}, clear=True) def test_group_message_with_other_user_mention_is_rejected_when_bot_identity_unknown(self): @@ -714,59 +786,10 @@ class TestAdapterBehavior(unittest.TestCase): id=SimpleNamespace(open_id="ou_other", user_id="u_other"), ) - self.assertFalse(adapter._should_accept_group_message(SimpleNamespace(mentions=[other_mention]), sender_id, "")) - - @patch.dict( - os.environ, - { - "FEISHU_BOT_OPEN_ID": "ou_hermes", - "FEISHU_BOT_USER_ID": "u_hermes", - }, - clear=True, - ) - def test_other_bot_sender_is_not_treated_as_self_sent_message(self): - from gateway.config import PlatformConfig - from gateway.platforms.feishu import FeishuAdapter - - adapter = FeishuAdapter(PlatformConfig()) - event = SimpleNamespace( - sender=SimpleNamespace( - sender_type="bot", - sender_id=SimpleNamespace(open_id="ou_other_bot", user_id="u_other_bot"), - ) + self.assertFalse( + _admits_group(adapter, SimpleNamespace(mentions=[other_mention]), sender_id, "") ) - self.assertFalse(adapter._is_self_sent_bot_message(event)) - - @patch.dict( - os.environ, - { - "FEISHU_BOT_OPEN_ID": "ou_hermes", - "FEISHU_BOT_USER_ID": "u_hermes", - }, - clear=True, - ) - def test_self_bot_sender_is_treated_as_self_sent_message(self): - from gateway.config import PlatformConfig - from gateway.platforms.feishu import FeishuAdapter - - adapter = FeishuAdapter(PlatformConfig()) - by_open_id = SimpleNamespace( - sender=SimpleNamespace( - sender_type="bot", - sender_id=SimpleNamespace(open_id="ou_hermes", user_id="u_other"), - ) - ) - by_user_id = SimpleNamespace( - sender=SimpleNamespace( - sender_type="app", - sender_id=SimpleNamespace(open_id="ou_other", user_id="u_hermes"), - ) - ) - - self.assertTrue(adapter._is_self_sent_bot_message(by_open_id)) - self.assertTrue(adapter._is_self_sent_bot_message(by_user_id)) - @patch.dict( os.environ, { @@ -792,14 +815,14 @@ class TestAdapterBehavior(unittest.TestCase): ) self.assertTrue( - adapter._should_accept_group_message( + _admits_group(adapter, mentioned, SimpleNamespace(open_id="ou_allowed", user_id=None), "", ) ) self.assertFalse( - adapter._should_accept_group_message( + _admits_group(adapter, mentioned, SimpleNamespace(open_id="ou_blocked", user_id=None), "", @@ -828,14 +851,14 @@ class TestAdapterBehavior(unittest.TestCase): ) self.assertTrue( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_alice", user_id=None), "oc_chat_a", ) ) self.assertFalse( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_charlie", user_id=None), "oc_chat_a", @@ -864,14 +887,14 @@ class TestAdapterBehavior(unittest.TestCase): ) self.assertTrue( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_alice", user_id=None), "oc_chat_b", ) ) self.assertFalse( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_blocked", user_id=None), "oc_chat_b", @@ -900,14 +923,14 @@ class TestAdapterBehavior(unittest.TestCase): ) self.assertTrue( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_admin", user_id=None), "oc_chat_c", ) ) self.assertFalse( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_regular", user_id=None), "oc_chat_c", @@ -936,14 +959,14 @@ class TestAdapterBehavior(unittest.TestCase): ) self.assertTrue( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_admin", user_id=None), "oc_chat_d", ) ) self.assertFalse( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_regular", user_id=None), "oc_chat_d", @@ -973,7 +996,7 @@ class TestAdapterBehavior(unittest.TestCase): ) self.assertTrue( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_admin", user_id=None), "oc_chat_e", @@ -997,7 +1020,7 @@ class TestAdapterBehavior(unittest.TestCase): ) self.assertTrue( - adapter._should_accept_group_message( + _admits_group(adapter, message, SimpleNamespace(open_id="ou_anyone", user_id=None), "oc_chat_unknown", @@ -1022,8 +1045,12 @@ class TestAdapterBehavior(unittest.TestCase): id=SimpleNamespace(open_id="ou_other", user_id="u_other"), ) - self.assertTrue(adapter._should_accept_group_message(SimpleNamespace(mentions=[bot_mention]), sender_id, "")) - self.assertFalse(adapter._should_accept_group_message(SimpleNamespace(mentions=[other_mention]), sender_id, "")) + self.assertTrue( + _admits_group(adapter, SimpleNamespace(mentions=[bot_mention]), sender_id, "") + ) + self.assertFalse( + _admits_group(adapter, SimpleNamespace(mentions=[other_mention]), sender_id, "") + ) @patch.dict(os.environ, {"FEISHU_GROUP_POLICY": "open"}, clear=True) def test_group_message_matches_bot_name_when_only_name_available(self): @@ -1048,8 +1075,12 @@ class TestAdapterBehavior(unittest.TestCase): id=SimpleNamespace(open_id=None, user_id=None), ) - self.assertTrue(adapter._should_accept_group_message(SimpleNamespace(mentions=[name_only_mention]), sender_id, "")) - self.assertFalse(adapter._should_accept_group_message(SimpleNamespace(mentions=[different_mention]), sender_id, "")) + self.assertTrue( + _admits_group(adapter, SimpleNamespace(mentions=[name_only_mention]), sender_id, "") + ) + self.assertFalse( + _admits_group(adapter, SimpleNamespace(mentions=[different_mention]), sender_id, "") + ) # Case 2: bot's open_id IS known — a same-name human with different # open_id must NOT admit (IDs override names). @@ -1066,8 +1097,17 @@ class TestAdapterBehavior(unittest.TestCase): id=SimpleNamespace(open_id="ou_bot", user_id=None), ) - self.assertFalse(adapter2._should_accept_group_message(SimpleNamespace(mentions=[same_name_other_id_mention]), sender_id, "")) - self.assertTrue(adapter2._should_accept_group_message(SimpleNamespace(mentions=[bot_mention]), sender_id, "")) + self.assertFalse( + _admits_group( + adapter2, + SimpleNamespace(mentions=[same_name_other_id_mention]), + sender_id, + "", + ) + ) + self.assertTrue( + _admits_group(adapter2, SimpleNamespace(mentions=[bot_mention]), sender_id, "") + ) @patch.dict(os.environ, {}, clear=True) def test_extract_post_message_as_text(self): @@ -1411,6 +1451,7 @@ class TestAdapterBehavior(unittest.TestCase): data=SimpleNamespace(event=SimpleNamespace(message=message)), message=message, sender_id=SimpleNamespace(open_id="ou_user", user_id=None, union_id=None), + is_bot=False, chat_type="p2p", message_id="om_command", ) @@ -1522,13 +1563,14 @@ class TestAdapterBehavior(unittest.TestCase): user_id="u_user", union_id="on_union", ) - data = SimpleNamespace(event=SimpleNamespace(message=message, sender=SimpleNamespace(sender_id=sender_id))) + sender = SimpleNamespace(sender_type="user", sender_id=sender_id) + data = SimpleNamespace(event=SimpleNamespace(message=message, sender=sender)) asyncio.run( adapter._process_inbound_message( data=data, message=message, - sender_id=sender_id, + sender_id=sender.sender_id, chat_type="p2p", message_id="om_text", ) @@ -1761,13 +1803,14 @@ class TestAdapterBehavior(unittest.TestCase): message_id="om_group_text", ) sender_id = SimpleNamespace(open_id="ou_user", user_id=None, union_id=None) + sender = SimpleNamespace(sender_type="user", sender_id=sender_id) data = SimpleNamespace(event=SimpleNamespace(message=message)) asyncio.run( adapter._process_inbound_message( data=data, message=message, - sender_id=sender_id, + sender_id=sender.sender_id, chat_type="group", message_id="om_group_text", ) @@ -1805,6 +1848,7 @@ class TestAdapterBehavior(unittest.TestCase): data=SimpleNamespace(event=SimpleNamespace(message=message)), message=message, sender_id=SimpleNamespace(open_id="ou_user", user_id=None, union_id=None), + is_bot=False, chat_type="p2p", message_id="om_reply", ) @@ -2667,11 +2711,12 @@ class TestAdapterBehavior(unittest.TestCase): @unittest.skipUnless(_HAS_LARK_OAPI, "lark-oapi not installed") class TestHydrateBotIdentity(unittest.TestCase): - """Hydration of bot identity via /open-apis/bot/v3/info and application info. + """Hydration of bot identity via ``/open-apis/bot/v3/info``. - Covers the manual-setup path where FEISHU_BOT_OPEN_ID / FEISHU_BOT_USER_ID - are not configured. Hydration must populate _bot_open_id so that - _is_self_sent_bot_message() can filter the adapter's own outbound echoes. + Covers the manual-setup path where ``FEISHU_BOT_OPEN_ID`` / + ``FEISHU_BOT_NAME`` are not configured — hydration populates them so + self-echo protection and group @mention gating both have something to + match against. """ def _make_adapter(self): @@ -2700,11 +2745,6 @@ class TestHydrateBotIdentity(unittest.TestCase): self.assertEqual(adapter._bot_open_id, "ou_hermes_hydrated") self.assertEqual(adapter._bot_name, "Hermes Bot") - # Application-info fallback must NOT run when bot_name is already set. - self.assertFalse( - adapter._client.application.v6.application.get.called - if hasattr(adapter._client, "application") else False - ) @patch.dict( os.environ, @@ -2721,7 +2761,6 @@ class TestHydrateBotIdentity(unittest.TestCase): asyncio.run(adapter._hydrate_bot_identity()) - # Neither probe should run — both fields are already populated. adapter._client.request.assert_not_called() self.assertEqual(adapter._bot_open_id, "ou_env") self.assertEqual(adapter._bot_name, "Env Hermes") @@ -2766,33 +2805,6 @@ class TestHydrateBotIdentity(unittest.TestCase): self.assertEqual(adapter._bot_open_id, "") self.assertEqual(adapter._bot_name, "Fallback Bot") - @patch.dict(os.environ, {}, clear=True) - def test_hydrated_open_id_enables_self_send_filter(self): - """E2E: after hydration, _is_self_sent_bot_message() rejects adapter's own id.""" - adapter = self._make_adapter() - adapter._client = Mock() - payload = json.dumps( - {"code": 0, "bot": {"bot_name": "Hermes", "open_id": "ou_hermes"}} - ).encode("utf-8") - adapter._client.request = Mock(return_value=SimpleNamespace(raw=SimpleNamespace(content=payload))) - - asyncio.run(adapter._hydrate_bot_identity()) - - self_event = SimpleNamespace( - sender=SimpleNamespace( - sender_type="bot", - sender_id=SimpleNamespace(open_id="ou_hermes", user_id=""), - ) - ) - peer_event = SimpleNamespace( - sender=SimpleNamespace( - sender_type="bot", - sender_id=SimpleNamespace(open_id="ou_peer_bot", user_id=""), - ) - ) - self.assertTrue(adapter._is_self_sent_bot_message(self_event)) - self.assertFalse(adapter._is_self_sent_bot_message(peer_event)) - @unittest.skipUnless(_HAS_LARK_OAPI, "lark-oapi not installed") class TestPendingInboundQueue(unittest.TestCase): @@ -3137,7 +3149,7 @@ class TestGroupMentionAtAll(unittest.TestCase): mentions=[], ) sender_id = SimpleNamespace(open_id="ou_any", user_id=None) - self.assertTrue(adapter._should_accept_group_message(message, sender_id, "")) + self.assertTrue(_admits_group(adapter, message, sender_id, "")) @patch.dict(os.environ, {"FEISHU_GROUP_POLICY": "allowlist", "FEISHU_ALLOWED_USERS": "ou_allowed"}, clear=True) def test_at_all_still_requires_policy_gate(self): @@ -3149,15 +3161,15 @@ class TestGroupMentionAtAll(unittest.TestCase): message = SimpleNamespace(content='{"text":"@_all attention"}', mentions=[]) # Non-allowlisted user — should be blocked even with @_all. blocked_sender = SimpleNamespace(open_id="ou_blocked", user_id=None) - self.assertFalse(adapter._should_accept_group_message(message, blocked_sender, "")) + self.assertFalse(_admits_group(adapter, message, blocked_sender, "")) # Allowlisted user — should pass. allowed_sender = SimpleNamespace(open_id="ou_allowed", user_id=None) - self.assertTrue(adapter._should_accept_group_message(message, allowed_sender, "")) + self.assertTrue(_admits_group(adapter, message, allowed_sender, "")) @unittest.skipUnless(_HAS_LARK_OAPI, "lark-oapi not installed") class TestSenderNameResolution(unittest.TestCase): - """Tests for _resolve_sender_name_from_api.""" + """Tests for _resolve_sender_name_from_api (contact API + cache).""" @patch.dict(os.environ, {}, clear=True) def test_returns_none_when_client_is_none(self): @@ -3261,6 +3273,137 @@ class TestSenderNameResolution(unittest.TestCase): self.assertIsNone(result) +@unittest.skipUnless(_HAS_LARK_OAPI, "lark-oapi not installed") +class TestBotNameResolution(unittest.TestCase): + """Tests for the bot branch of _resolve_sender_name_from_api (basic_batch API + shared cache).""" + + @staticmethod + def _batch_payload(bots: Dict[str, str]): + import json as _json + body = { + oid: {"bot_id": oid, "name": name, "i18n_names": {"en_us": name}} + for oid, name in bots.items() + } + return _json.dumps({"code": 0, "msg": "", "data": {"bots": body, "failed_bots": {}}}).encode() + + def _build_adapter_with_bots(self, bots: Dict[str, str]): + from gateway.config import PlatformConfig + from gateway.platforms.feishu import FeishuAdapter + + adapter = FeishuAdapter(PlatformConfig()) + calls = [] + + def _fake_request(request): + calls.append(request) + return SimpleNamespace(raw=SimpleNamespace(content=self._batch_payload(bots))) + + adapter._client = SimpleNamespace(request=_fake_request) + return adapter, calls + + @patch.dict(os.environ, {}, clear=True) + def test_returns_cached_bot_name_without_api_call(self): + from gateway.config import PlatformConfig + from gateway.platforms.feishu import FeishuAdapter + + adapter = FeishuAdapter(PlatformConfig()) + adapter._sender_name_cache["ou_peer"] = ("Peer Bot", time.time() + 600) + adapter._client = SimpleNamespace( + request=lambda _r: (_ for _ in ()).throw(RuntimeError("should not fetch")) + ) + result = asyncio.run(adapter._resolve_sender_name_from_api("ou_peer", is_bot=True)) + self.assertEqual(result, "Peer Bot") + + @patch.dict(os.environ, {}, clear=True) + def test_fetches_and_caches_bot_name(self): + adapter, calls = self._build_adapter_with_bots({"ou_peer": "Peer Bot"}) + + async def _direct(func, *args, **kwargs): + return func(*args, **kwargs) + + with patch("gateway.platforms.feishu.asyncio.to_thread", side_effect=_direct): + result = asyncio.run(adapter._resolve_sender_name_from_api("ou_peer", is_bot=True)) + + self.assertEqual(result, "Peer Bot") + self.assertEqual(adapter._sender_name_cache["ou_peer"][0], "Peer Bot") + self.assertEqual(len(calls), 1) + self.assertIn("/open-apis/bot/v3/bots/basic_batch", calls[0].uri) + # Feishu expects repeated ?bot_ids= params, not comma-joined. + self.assertEqual(calls[0].queries, [("bot_ids", "ou_peer")]) + + @patch.dict(os.environ, {}, clear=True) + def test_api_failure_returns_none_and_does_not_poison_cache(self): + from gateway.config import PlatformConfig + from gateway.platforms.feishu import FeishuAdapter + + adapter = FeishuAdapter(PlatformConfig()) + + def _broken_request(_req): + raise RuntimeError("API down") + + adapter._client = SimpleNamespace(request=_broken_request) + + async def _direct(func, *args, **kwargs): + return func(*args, **kwargs) + + with patch("gateway.platforms.feishu.asyncio.to_thread", side_effect=_direct): + result = asyncio.run(adapter._resolve_sender_name_from_api("ou_peer", is_bot=True)) + + self.assertIsNone(result) + self.assertNotIn("ou_peer", adapter._sender_name_cache) + + @patch.dict(os.environ, {}, clear=True) + def test_bot_absent_from_response_is_not_cached(self): + """Bot not in ``data.bots`` (e.g. landed in ``failed_bots``) → no + cache entry, next lookup re-fetches.""" + adapter, _ = self._build_adapter_with_bots({"ou_other": "Other Bot"}) + + async def _direct(func, *args, **kwargs): + return func(*args, **kwargs) + + with patch("gateway.platforms.feishu.asyncio.to_thread", side_effect=_direct): + result = asyncio.run(adapter._resolve_sender_name_from_api("ou_ghost", is_bot=True)) + + self.assertIsNone(result) + self.assertNotIn("ou_ghost", adapter._sender_name_cache) + + @patch.dict(os.environ, {}, clear=True) + def test_empty_name_in_response_is_negative_cached(self): + """API returns name="" → cache "" so repeat lookups short-circuit.""" + adapter, calls = self._build_adapter_with_bots({"ou_nameless": ""}) + + async def _direct(func, *args, **kwargs): + return func(*args, **kwargs) + + with patch("gateway.platforms.feishu.asyncio.to_thread", side_effect=_direct): + first = asyncio.run(adapter._resolve_sender_name_from_api("ou_nameless", is_bot=True)) + second = asyncio.run(adapter._resolve_sender_name_from_api("ou_nameless", is_bot=True)) + + self.assertIsNone(first) + self.assertIsNone(second) + self.assertEqual(adapter._sender_name_cache["ou_nameless"][0], "") + self.assertEqual(len(calls), 1) + + @patch.dict(os.environ, {}, clear=True) + def test_non_zero_code_returns_none(self): + from gateway.config import PlatformConfig + from gateway.platforms.feishu import FeishuAdapter + + adapter = FeishuAdapter(PlatformConfig()) + error_payload = b'{"code":99991663,"msg":"permission denied"}' + adapter._client = SimpleNamespace( + request=lambda _r: SimpleNamespace(raw=SimpleNamespace(content=error_payload)) + ) + + async def _direct(func, *args, **kwargs): + return func(*args, **kwargs) + + with patch("gateway.platforms.feishu.asyncio.to_thread", side_effect=_direct): + result = asyncio.run(adapter._resolve_sender_name_from_api("ou_peer", is_bot=True)) + + self.assertIsNone(result) + self.assertNotIn("ou_peer", adapter._sender_name_cache) + + @unittest.skipUnless(_HAS_LARK_OAPI, "lark-oapi not installed") class TestProcessingReactions(unittest.TestCase): """Typing on start → removed on SUCCESS, swapped for CrossMark on FAILURE, diff --git a/tests/gateway/test_feishu_bot_admission.py b/tests/gateway/test_feishu_bot_admission.py new file mode 100644 index 0000000000..83b7023843 --- /dev/null +++ b/tests/gateway/test_feishu_bot_admission.py @@ -0,0 +1,745 @@ +"""Adapter-layer tests for Feishu bot-sender admission (``FeishuAdapter._admit``).""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import Any + +import pytest + +from tests.gateway.feishu_helpers import ( + install_dedup_state, + make_adapter_skeleton, + make_message, + make_sender, + stub_mention, +) + + +# --- FeishuAdapterSettings wiring ------------------------------------------ + + +@pytest.mark.parametrize( + "env_value, expected", + [ + ("none", "none"), + ("mentions", "mentions"), + ("all", "all"), + (" Mentions ", "mentions"), + ], +) +def test_feishu_load_settings_populates_allow_bots(monkeypatch, env_value, expected): + from gateway.platforms.feishu import FeishuAdapter + + monkeypatch.setenv("FEISHU_APP_ID", "cli_test") + monkeypatch.setenv("FEISHU_APP_SECRET", "secret_test") + monkeypatch.setenv("FEISHU_ALLOW_BOTS", env_value) + + settings = FeishuAdapter._load_settings(extra={}) + assert settings.allow_bots == expected + + +def test_feishu_load_settings_allow_bots_defaults_to_none(monkeypatch): + from gateway.platforms.feishu import FeishuAdapter + + monkeypatch.setenv("FEISHU_APP_ID", "cli_test") + monkeypatch.setenv("FEISHU_APP_SECRET", "secret_test") + monkeypatch.delenv("FEISHU_ALLOW_BOTS", raising=False) + + settings = FeishuAdapter._load_settings(extra={}) + assert settings.allow_bots == "none" + + +def test_feishu_load_settings_ignores_extra_allow_bots(monkeypatch): + # extra is ignored — env is single source of truth (yaml is bridged to env). + from gateway.platforms.feishu import FeishuAdapter + + monkeypatch.setenv("FEISHU_APP_ID", "cli_test") + monkeypatch.setenv("FEISHU_APP_SECRET", "secret_test") + monkeypatch.delenv("FEISHU_ALLOW_BOTS", raising=False) + + settings = FeishuAdapter._load_settings(extra={"allow_bots": "all"}) + assert settings.allow_bots == "none" + + +def test_feishu_load_settings_falls_back_to_env_when_extra_missing(monkeypatch): + from gateway.platforms.feishu import FeishuAdapter + + monkeypatch.setenv("FEISHU_APP_ID", "cli_test") + monkeypatch.setenv("FEISHU_APP_SECRET", "secret_test") + monkeypatch.setenv("FEISHU_ALLOW_BOTS", "mentions") + + settings = FeishuAdapter._load_settings(extra={}) + assert settings.allow_bots == "mentions" + + +def test_feishu_load_settings_warns_on_unknown_allow_bots(monkeypatch, caplog): + import logging + + from gateway.platforms.feishu import FeishuAdapter + + monkeypatch.setenv("FEISHU_APP_ID", "cli_test") + monkeypatch.setenv("FEISHU_APP_SECRET", "secret_test") + monkeypatch.setenv("FEISHU_ALLOW_BOTS", "menton") # typo + + with caplog.at_level(logging.WARNING, logger="gateway.platforms.feishu"): + settings = FeishuAdapter._load_settings(extra={}) + + assert settings.allow_bots == "none" + assert any("allow_bots" in r.message and "menton" in r.message for r in caplog.records) + + +@pytest.mark.parametrize( + "env_value, extra, expected", + [ + (None, {}, True), + ("false", {}, False), + ("true", {}, True), + ("true", {"require_mention": False}, False), + ], +) +def test_feishu_load_settings_require_mention(monkeypatch, env_value, extra, expected): + from gateway.platforms.feishu import FeishuAdapter + + monkeypatch.setenv("FEISHU_APP_ID", "cli_test") + monkeypatch.setenv("FEISHU_APP_SECRET", "secret_test") + if env_value is None: + monkeypatch.delenv("FEISHU_REQUIRE_MENTION", raising=False) + else: + monkeypatch.setenv("FEISHU_REQUIRE_MENTION", env_value) + + settings = FeishuAdapter._load_settings(extra=extra) + assert settings.require_mention is expected + + +def test_feishu_load_settings_parses_per_group_require_mention(monkeypatch): + from gateway.platforms.feishu import FeishuAdapter + + monkeypatch.setenv("FEISHU_APP_ID", "cli_test") + monkeypatch.setenv("FEISHU_APP_SECRET", "secret_test") + + settings = FeishuAdapter._load_settings(extra={ + "group_rules": { + "oc_free": {"policy": "open", "require_mention": False}, + "oc_strict": {"policy": "open", "require_mention": True}, + "oc_inherit": {"policy": "open"}, + }, + }) + assert settings.group_rules["oc_free"].require_mention is False + assert settings.group_rules["oc_strict"].require_mention is True + assert settings.group_rules["oc_inherit"].require_mention is None + + +# --- Module-level helpers -------------------------------------------------- + + +def test_sender_identity_collects_every_non_empty_id_variant(): + from gateway.platforms.feishu import _sender_identity + + sender = SimpleNamespace( + sender_id=SimpleNamespace(open_id="ou_x", user_id="", union_id="un_x"), + ) + assert _sender_identity(sender) == frozenset({"ou_x", "un_x"}) + + +def test_sender_identity_handles_missing_sender_id(): + from gateway.platforms.feishu import _sender_identity + + assert _sender_identity(SimpleNamespace()) == frozenset() + + +@pytest.mark.parametrize("sender_type", ["bot", "app"]) +def test_is_bot_sender_treats_bot_and_app_as_bot_origin(sender_type): + from gateway.platforms.feishu import _is_bot_sender + + assert _is_bot_sender(SimpleNamespace(sender_type=sender_type)) is True + + +@pytest.mark.parametrize("sender_type", ["user", "", None]) +def test_is_bot_sender_rejects_non_bot_origin(sender_type): + from gateway.platforms.feishu import _is_bot_sender + + assert _is_bot_sender(SimpleNamespace(sender_type=sender_type)) is False + + +# --- _admit pipeline matrix ------------------------------------------------ +# +# Covers the four-step admission pipeline (self_echo → bot_policy → +# DM bypass → group_policy + mention) as a single result-only matrix. +# Each row pins one decision in the pipeline; tests asserting call-count +# semantics live below in their own functions. + + +def _admit_case( + *, + adapter: dict | None = None, + sender: dict | None = None, + message: dict | None = None, + mentions_self: bool | None = None, + expected: str | None = None, +): + return { + "adapter": adapter or {}, + "sender": sender or {}, + "message": message or {}, + "mentions_self": mentions_self, + "expected": expected, + } + + +_ADMIT_CASES = [ + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_me", "allow_bots": "all"}, + sender={"sender_type": "bot", "open_id": "ou_me"}, + expected="self_echo", + ), + id="self_echo:open_id_under_all_mode", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "", "bot_user_id": "u_me", "allow_bots": "all"}, + sender={"sender_type": "bot", "open_id": None, "user_id": "u_me"}, + expected="self_echo", + ), + id="self_echo:user_id_only", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_me", "allow_bots": "all"}, + sender={"sender_type": "bot", "open_id": "ou_me", "user_id": "u_me", "union_id": "un_me"}, + expected="self_echo", + ), + id="self_echo:mixed_ids", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "bot_user_id": "u_self", "allow_bots": "all"}, + sender={"sender_type": "bot", "open_id": None, "user_id": "u_self"}, + expected="self_echo", + ), + id="self_echo:user_id_when_bot_user_id_set", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "allow_bots": "none"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + expected="bots_disabled", + ), + id="bots_disabled:mode_none", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "allow_bots": ""}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + expected="bots_disabled", + ), + id="bots_disabled:mode_empty", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "allow_bots": "loose"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + expected="bots_disabled", + ), + id="bots_disabled:mode_unknown_value", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "", "allow_bots": "none"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + expected="bots_disabled", + ), + id="bots_disabled:wins_over_self_ids_unknown", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "", "allow_bots": "all"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + expected="self_ids_unknown", + ), + id="self_ids_unknown:bot_sender_no_self_ids", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "", "allow_bots": "all"}, + sender={"sender_type": "app", "open_id": "ou_peer"}, + expected="self_ids_unknown", + ), + id="self_ids_unknown:app_sender_no_self_ids", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "allow_bots": "all"}, + sender={"sender_type": "app", "open_id": None}, + expected="self_ids_unknown", + ), + id="self_ids_unknown:no_sender_ids", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "allow_bots": "mentions"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + mentions_self=False, + expected="bot_not_mentioned", + ), + id="mentions_mode:not_mentioned_dm", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "allow_bots": "mentions"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + mentions_self=True, + expected=None, + ), + id="mentions_mode:mentioned_dm", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "allow_bots": "all"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + mentions_self=False, + expected=None, + ), + id="all_mode:not_mentioned_dm", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "ou_self", "allow_bots": "all"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + mentions_self=True, + expected=None, + ), + id="all_mode:mentioned_dm", + ), + pytest.param( + _admit_case( + adapter={"bot_open_id": "", "allow_bots": "none"}, + sender={"sender_type": "user", "open_id": "ou_human"}, + expected=None, + ), + id="human:dm_admitted_regardless_of_allow_bots", + ), + pytest.param( + _admit_case( + adapter={"allow_bots": "all"}, + sender={"sender_type": "user", "open_id": "ou_human"}, + message={"message_id": "om_ok", "chat_type": "p2p"}, + expected=None, + ), + id="human:p2p_admitted", + ), + pytest.param( + _admit_case( + adapter={ + "bot_open_id": "ou_self", + "require_mention": False, + "group_policy": "open", + }, + sender={"sender_type": "user", "open_id": "ou_human"}, + message={"chat_type": "group"}, + mentions_self=False, + expected=None, + ), + id="require_mention_false:group_human_no_mention_admitted", + ), + pytest.param( + _admit_case( + adapter={ + "bot_open_id": "ou_self", + "allow_bots": "all", + "require_mention": False, + "group_policy": "open", + }, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + message={"chat_type": "group"}, + mentions_self=False, + expected=None, + ), + id="require_mention_false:group_bot_all_mode_admitted", + ), + pytest.param( + _admit_case( + adapter={ + "bot_open_id": "ou_self", + "allow_bots": "mentions", + "require_mention": False, + "group_policy": "open", + }, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + message={"chat_type": "group"}, + mentions_self=False, + expected="bot_not_mentioned", + ), + id="require_mention_false:group_bot_mentions_mode_still_gated", + ), +] + + +@pytest.mark.parametrize("case", _ADMIT_CASES) +def test_admit_pipeline(case): + adapter = make_adapter_skeleton(**case["adapter"]) + if case["mentions_self"] is not None: + stub_mention(adapter, case["mentions_self"]) + sender = make_sender(**case["sender"]) + message = make_message(**case["message"]) + assert adapter._admit(sender, message) == case["expected"] + + +# --- Mention call-count semantics ------------------------------------------ + + +def test_admit_skips_mention_check_under_all_mode(): + # Tripwire: under allow_bots=all the mention path must not be probed. + adapter = make_adapter_skeleton(bot_open_id="ou_self", allow_bots="all") + calls = 0 + + def _tripwire(_message): + nonlocal calls + calls += 1 + return False + + adapter._mentions_self = _tripwire + + sender = make_sender(sender_type="bot", open_id="ou_peer") + assert adapter._admit(sender, make_message()) is None + assert calls == 0 + + +def test_admit_group_mention_checked_once_per_call(): + # Stage 2 (mentions mode) and stage 4 (group require_mention) must not + # double-evaluate _mentions_self for the same admit call. + adapter = make_adapter_skeleton( + bot_open_id="ou_self", allow_bots="mentions", require_mention=True, + group_policy="open", + ) + calls = 0 + + def _counting(_message): + nonlocal calls + calls += 1 + return True + + adapter._mentions_self = _counting + + sender = make_sender(sender_type="bot", open_id="ou_peer") + assert adapter._admit(sender, make_message(chat_type="group")) is None + assert calls == 1 + + +# --- Per-group require_mention override ------------------------------------ + + +def test_admit_per_group_require_mention_overrides_global(): + from gateway.platforms.feishu import FeishuGroupRule + + adapter = make_adapter_skeleton( + bot_open_id="ou_self", require_mention=True, group_policy="open", + ) + adapter._group_rules = { + "oc_free": FeishuGroupRule(policy="open", require_mention=False), + } + stub_mention(adapter, False) + + sender = make_sender(sender_type="user", open_id="ou_human") + assert adapter._admit(sender, make_message(chat_id="oc_free", chat_type="group")) is None + assert ( + adapter._admit(sender, make_message(chat_id="oc_other", chat_type="group")) + == "group_policy_rejected" + ) + + +# --- Hydration ------------------------------------------------------------- + + +def test_hydrate_bot_identity_populates_self_ids_from_bot_v3_info(monkeypatch): + import asyncio + + from gateway.platforms.feishu import FeishuAdapter + + adapter = object.__new__(FeishuAdapter) + adapter._bot_open_id = "" + adapter._bot_user_id = "" + adapter._bot_name = "" + adapter._allow_bots = "all" + + captured = {} + + def _fake_request(request): + captured["uri"] = getattr(request, "uri", None) + captured["http_method"] = getattr(request, "http_method", None) + return SimpleNamespace(raw=SimpleNamespace( + content=b'{"code":0,"bot":{"app_name":"Hermes","open_id":"ou_hydrated"}}' + )) + + adapter._client = SimpleNamespace(request=_fake_request) + + asyncio.run(adapter._hydrate_bot_identity()) + + assert captured["uri"] == "/open-apis/bot/v3/info" + assert str(captured["http_method"]).endswith("GET") + assert adapter._bot_open_id == "ou_hydrated" + assert adapter._bot_name == "Hermes" + # /bot/v3/info doesn't surface user_id, so _bot_user_id stays empty. + assert adapter._bot_user_id == "" + + +def test_resolve_sender_profile_uses_open_id_for_bot_name_lookup(): + import asyncio + + from gateway.platforms.feishu import FeishuAdapter + + adapter = object.__new__(FeishuAdapter) + adapter._client = object() + adapter._sender_name_cache = {} + seen_ids = [] + + async def _fake_fetch_bot_names(bot_ids): + seen_ids.extend(bot_ids) + return {"ou_peer": "Peer Bot"} + + adapter._fetch_bot_names = _fake_fetch_bot_names + + profile = asyncio.run( + adapter._resolve_sender_profile( + SimpleNamespace(open_id="ou_peer", user_id="u_peer", union_id="on_peer"), + is_bot=True, + ) + ) + + assert seen_ids == ["ou_peer"] + assert profile["user_id"] == "u_peer" + assert profile["user_name"] == "Peer Bot" + + +# --- _allow_group_message matrix ------------------------------------------- +# +# Bot-bypass semantics: admitted bots skip allowlist/blacklist (parallel +# human-scope filters), but channel-level locks (disabled, admin_only) and +# admin short-circuits still apply. + + +def _group_case( + *, + adapter: dict | None = None, + admins: set | None = None, + group_rules: dict | None = None, + sender: dict | None = None, + chat_id: str = "oc_1", + is_bot: bool = False, + expected: bool = False, +): + return { + "adapter": adapter or {}, + "admins": admins or set(), + "group_rules": group_rules or {}, + "sender": sender or {}, + "chat_id": chat_id, + "is_bot": is_bot, + "expected": expected, + } + + +def _group_rule(policy: str, **kwargs): + from gateway.platforms.feishu import FeishuGroupRule + return FeishuGroupRule(policy=policy, **kwargs) + + +_GROUP_CASES = [ + pytest.param( + _group_case( + sender={"sender_type": "bot", "open_id": "ou_peer"}, + is_bot=True, + expected=True, + ), + id="bot:bypasses_default_allowlist", + ), + pytest.param( + _group_case( + sender={"sender_type": "user", "open_id": "ou_stranger"}, + is_bot=False, + expected=False, + ), + id="human:gated_by_default_allowlist", + ), + pytest.param( + _group_case( + admins={"ou_peer"}, + sender={"sender_type": "bot", "open_id": "ou_peer"}, + is_bot=True, + expected=True, + ), + id="bot:admin_short_circuit", + ), + pytest.param( + _group_case( + admins={"u_admin"}, + sender={"sender_type": "user", "open_id": None, "user_id": "u_admin"}, + is_bot=False, + expected=True, + ), + id="human:admin_via_user_id", + ), + pytest.param( + _group_case( + sender={"sender_type": "bot", "open_id": "ou_peer"}, + is_bot=True, + expected=True, + ), + id="bot:allowlist_skipped", + ), + pytest.param( + _group_case( + sender={"sender_type": "app", "open_id": "ou_peer"}, + is_bot=True, + expected=True, + ), + id="app:allowlist_skipped", + ), +] + + +# Channel-lock cases need group_rules construction; keep them in a separate +# parametrize so we can use _group_rule() (FeishuGroupRule import). +_GROUP_RULE_CASES = [ + pytest.param( + "disabled", "bot", False, + id="bot:disabled_policy_blocks_even_with_bypass", + ), + pytest.param( + "disabled", "app", False, + id="app:disabled_policy_blocks_even_with_bypass", + ), + pytest.param( + "admin_only", "bot", False, + id="bot:admin_only_policy_blocks_non_admin", + ), + pytest.param( + "admin_only", "app", False, + id="app:admin_only_policy_blocks_non_admin", + ), +] + + +@pytest.mark.parametrize("case", _GROUP_CASES) +def test_allow_group_message_matrix(case): + adapter = make_adapter_skeleton(**case["adapter"]) + adapter._admins = case["admins"] + adapter._group_rules = case["group_rules"] + sender = make_sender(**case["sender"]) + assert adapter._allow_group_message( + sender_id=sender.sender_id, + chat_id=case["chat_id"], + is_bot=case["is_bot"], + ) is case["expected"] + + +@pytest.mark.parametrize("policy, sender_type, expected", _GROUP_RULE_CASES) +def test_allow_group_message_channel_locks_apply_to_bots(policy, sender_type, expected): + adapter = make_adapter_skeleton() + adapter._group_rules = {"oc_locked": _group_rule(policy)} + sender = make_sender(sender_type=sender_type, open_id="ou_peer") + assert adapter._allow_group_message( + sender_id=sender.sender_id, + chat_id="oc_locked", + is_bot=True, + ) is expected + + +@pytest.mark.parametrize("sender_type", ["bot", "app"]) +def test_allow_group_message_blacklist_is_human_scope_only(sender_type): + # blacklist is parallel to allowlist (human-scope); admitted bots bypass + # it. To block a specific bot, gate upstream via FEISHU_ALLOW_BOTS. + adapter = make_adapter_skeleton() + adapter._group_rules = { + "oc_1": _group_rule("blacklist", blacklist={"ou_peer"}) + } + sender = make_sender(sender_type=sender_type, open_id="ou_peer") + assert adapter._allow_group_message( + sender_id=sender.sender_id, + chat_id="oc_1", + is_bot=True, + ) is True + + +# --- Realistic payload smoke ----------------------------------------------- + + +def test_admit_accepts_realistic_bot_at_bot_group_event(): + # Locks in the real im.message.receive_v1 payload shape under mode=mentions. + adapter = make_adapter_skeleton(bot_open_id="ou_self", allow_bots="mentions") + + mention = SimpleNamespace( + key="@_user_1", + id=SimpleNamespace(union_id="on_mentionUnion", user_id="", open_id="ou_self"), + name="Hermes", + mentioned_type="bot", + tenant_key="tenant_ab", + ) + message = SimpleNamespace( + message_id="om_realistic_bot_at_bot", + chat_id="oc_real", + chat_type="group", + message_type="text", + content='{"text":"@_user_1 hello"}', + mentions=[mention], + ) + sender = SimpleNamespace( + sender_type="bot", + sender_id=SimpleNamespace(union_id="on_peerUnion", user_id="u_peer", open_id="ou_peer_bot"), + tenant_key="tenant_ab", + ) + + assert adapter._admit(sender, message) is None + + +# --- Event-dispatch plumbing ----------------------------------------------- + + +def test_handle_message_event_data_drops_bot_sender_by_default(): + import asyncio + + adapter = make_adapter_skeleton() + install_dedup_state(adapter) + processed = [] + + async def _fake_process_inbound_message(**kwargs): + processed.append(kwargs) + + adapter._process_inbound_message = _fake_process_inbound_message + + data = SimpleNamespace( + event=SimpleNamespace( + sender=make_sender(sender_type="bot", open_id="ou_peer"), + message=make_message(message_id="om_bot_default", chat_type="p2p"), + ) + ) + + asyncio.run(adapter._handle_message_event_data(data)) + assert processed == [] + + +def test_handle_message_event_data_forwards_sender_when_admitted(): + import asyncio + + adapter = make_adapter_skeleton(allow_bots="all") + install_dedup_state(adapter) + captured = {} + + async def _fake_process_inbound_message(**kwargs): + captured.update(kwargs) + + adapter._process_inbound_message = _fake_process_inbound_message + + sender = make_sender(sender_type="bot", open_id="ou_peer") + data = SimpleNamespace( + event=SimpleNamespace( + sender=sender, + message=make_message(message_id="om_bot_ok", chat_type="p2p"), + ) + ) + + asyncio.run(adapter._handle_message_event_data(data)) + assert captured.get("sender_id") is sender.sender_id + assert captured.get("is_bot") is True + assert captured.get("message_id") == "om_bot_ok" diff --git a/tests/gateway/test_feishu_bot_auth_bypass.py b/tests/gateway/test_feishu_bot_auth_bypass.py new file mode 100644 index 0000000000..4dd83a1bd3 --- /dev/null +++ b/tests/gateway/test_feishu_bot_auth_bypass.py @@ -0,0 +1,113 @@ +"""Regression guard for Feishu bot-sender authorization bypass. + +Mirrors tests/gateway/test_discord_bot_auth_bypass.py for Platform.FEISHU. +Without the bypass in gateway/run.py, Feishu bot senders admitted by the +adapter would be rejected at _is_user_authorized with "Unauthorized user" +— same class of bug as Discord #4466. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest + +from gateway.session import Platform, SessionSource + + +@pytest.fixture(autouse=True) +def _isolate_feishu_env(monkeypatch): + for var in ( + "FEISHU_ALLOW_BOTS", + "FEISHU_ALLOWED_USERS", + "FEISHU_ALLOW_ALL_USERS", + "GATEWAY_ALLOW_ALL_USERS", + "GATEWAY_ALLOWED_USERS", + ): + monkeypatch.delenv(var, raising=False) + + +def _make_bare_runner(): + from gateway.run import GatewayRunner + + runner = object.__new__(GatewayRunner) + runner.pairing_store = SimpleNamespace(is_approved=lambda *_a, **_kw: False) + return runner + + +def _make_feishu_bot_source(open_id: str = "ou_peer"): + return SessionSource( + platform=Platform.FEISHU, + chat_id="oc_1", + chat_type="group", + user_id=open_id, + user_name="PeerBot", + is_bot=True, + ) + + +def _make_feishu_human_source(open_id: str = "ou_human"): + return SessionSource( + platform=Platform.FEISHU, + chat_id="oc_1", + chat_type="group", + user_id=open_id, + user_name="Human", + is_bot=False, + ) + + +def test_feishu_bot_authorized_when_allow_bots_mentions(monkeypatch): + runner = _make_bare_runner() + monkeypatch.setenv("FEISHU_ALLOW_BOTS", "mentions") + monkeypatch.setenv("FEISHU_ALLOWED_USERS", "ou_human") + + assert runner._is_user_authorized(_make_feishu_bot_source("ou_peer")) is True + + +def test_feishu_bot_authorized_when_allow_bots_all(monkeypatch): + runner = _make_bare_runner() + monkeypatch.setenv("FEISHU_ALLOW_BOTS", "all") + monkeypatch.setenv("FEISHU_ALLOWED_USERS", "ou_human") + + assert runner._is_user_authorized(_make_feishu_bot_source()) is True + + +def test_feishu_bot_NOT_authorized_when_allow_bots_none(monkeypatch): + runner = _make_bare_runner() + monkeypatch.setenv("FEISHU_ALLOW_BOTS", "none") + monkeypatch.setenv("FEISHU_ALLOWED_USERS", "ou_human") + + assert runner._is_user_authorized(_make_feishu_bot_source("ou_peer")) is False + + +def test_feishu_bot_NOT_authorized_when_allow_bots_unset(monkeypatch): + runner = _make_bare_runner() + monkeypatch.setenv("FEISHU_ALLOWED_USERS", "ou_human") + + assert runner._is_user_authorized(_make_feishu_bot_source("ou_peer")) is False + + +def test_feishu_human_still_checked_against_allowlist_when_bot_policy_set(monkeypatch): + """FEISHU_ALLOW_BOTS=all must NOT open the gate for humans.""" + runner = _make_bare_runner() + monkeypatch.setenv("FEISHU_ALLOW_BOTS", "all") + monkeypatch.setenv("FEISHU_ALLOWED_USERS", "ou_human") + + assert runner._is_user_authorized(_make_feishu_human_source("ou_stranger")) is False + assert runner._is_user_authorized(_make_feishu_human_source("ou_human")) is True + + +def test_feishu_bot_bypass_does_not_leak_to_other_platforms(monkeypatch): + """FEISHU_ALLOW_BOTS=all must not authorize Telegram/Discord bot sources.""" + runner = _make_bare_runner() + monkeypatch.setenv("FEISHU_ALLOW_BOTS", "all") + + telegram_bot = SessionSource( + platform=Platform.TELEGRAM, + chat_id="123", + chat_type="channel", + user_id="999", + is_bot=True, + ) + assert runner._is_user_authorized(telegram_bot) is False diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index 235d84654f..afe2c40d2a 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -300,6 +300,8 @@ For cloud sandbox backends, persistence is filesystem-oriented. `TERMINAL_LIFETI | `FEISHU_ENCRYPT_KEY` | Optional encryption key for webhook mode | | `FEISHU_VERIFICATION_TOKEN` | Optional verification token for webhook mode | | `FEISHU_ALLOWED_USERS` | Comma-separated Feishu user IDs allowed to message the bot | +| `FEISHU_ALLOW_BOTS` | `none` (default) / `mentions` / `all` — accept inbound messages from other bots. See [bot-to-bot messaging](../user-guide/messaging/feishu.md#bot-to-bot-messaging) | +| `FEISHU_REQUIRE_MENTION` | `true` (default) / `false` — whether group messages must @mention the bot. Override per-chat via `group_rules..require_mention`. | | `FEISHU_HOME_CHANNEL` | Feishu chat ID for cron delivery and notifications | | `WECOM_BOT_ID` | WeCom AI Bot ID from admin console | | `WECOM_SECRET` | WeCom AI Bot secret | diff --git a/website/docs/user-guide/messaging/feishu.md b/website/docs/user-guide/messaging/feishu.md index d2b52dff4b..879964c80f 100644 --- a/website/docs/user-guide/messaging/feishu.md +++ b/website/docs/user-guide/messaging/feishu.md @@ -201,19 +201,45 @@ FEISHU_GROUP_POLICY=allowlist # default | `allowlist` | Hermes only responds to @mentions from users listed in `FEISHU_ALLOWED_USERS`. | | `disabled` | Hermes ignores all group messages entirely. | -In all modes, the bot must be explicitly @mentioned (or @all) in the group before the message is processed. Direct messages bypass this gate. +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. -### Bot Identity for @Mention Gating - -For precise @mention detection in groups, the adapter needs to know the bot's identity. It can be provided explicitly: +Set `FEISHU_REQUIRE_MENTION=false` to let Hermes read all group traffic without requiring an @mention: ```bash -FEISHU_BOT_OPEN_ID=ou_xxx -FEISHU_BOT_USER_ID=xxx -FEISHU_BOT_NAME=MyBot +FEISHU_REQUIRE_MENTION=false ``` -If none of these are set, the adapter will attempt to auto-discover the bot name via the Application Info API on startup. For this to work, grant the `admin:app.info:readonly` or `application:application:self_manage` permission scope. +For per-chat control, set `require_mention` on a `group_rules` entry — see [Per-Group Access Control](#per-group-access-control) below. + +### Bot Identity + +Hermes auto-detects the bot's `open_id` and display name on startup. You only need to set these manually when auto-detection cannot reach the Feishu API, or when your app uses tenant-scoped user IDs: + +```bash +FEISHU_BOT_OPEN_ID=ou_xxx # only when auto-detection fails +FEISHU_BOT_USER_ID=xxx # required if your app uses sender_id_type=user_id +FEISHU_BOT_NAME=MyBot # only when auto-detection fails +``` + +## Bot-to-Bot Messaging + +By default Hermes ignores messages sent by other bots. Enable bot-to-bot messaging when you want Hermes to participate in A2A orchestration or receive notifications from other bots in the same group. + +```bash +FEISHU_ALLOW_BOTS=mentions # default: none +``` + +| Value | Behavior | +|-------|----------| +| `none` | Ignore all messages from other bots (default). | +| `mentions` | Accept only when the peer bot @mentions Hermes. | +| `all` | Accept every peer bot message. | + +Also configurable as `feishu.allow_bots` in `config.yaml` (env wins when both are set). + +Peer bots do not need to be added to `FEISHU_ALLOWED_USERS` — that allowlist applies to human senders only. + +Grant the `application:bot.basic_info:read` scope to display peer bot names; without it, peer bots still route correctly but appear as their `open_id`. ## Interactive Card Actions @@ -426,6 +452,9 @@ platforms: policy: "blacklist" blacklist: - "ou_blocked_user" + "oc_free_chat": + policy: "open" + require_mention: false # overrides FEISHU_REQUIRE_MENTION for this chat ``` | Policy | Description | @@ -436,6 +465,8 @@ platforms: | `admin_only` | Only users in the global `admins` list can use the bot in this group | | `disabled` | Bot ignores all messages in this group | +Set `require_mention: false` on a `group_rules` entry to skip the @-mention requirement for that specific chat. When omitted, the chat inherits the global `FEISHU_REQUIRE_MENTION` value. + Groups not listed in `group_rules` fall back to `default_group_policy` (defaults to the value of `FEISHU_GROUP_POLICY`). ## Deduplication @@ -455,6 +486,8 @@ Inbound messages are deduplicated using message IDs with a 24-hour TTL. The dedu | `FEISHU_DOMAIN` | — | `feishu` | `feishu` (China) or `lark` (international) | | `FEISHU_CONNECTION_MODE` | — | `websocket` | `websocket` or `webhook` | | `FEISHU_ALLOWED_USERS` | — | _(empty)_ | Comma-separated open_id list for user allowlist | +| `FEISHU_ALLOW_BOTS` | — | `none` | Accept messages from other bots: `none`, `mentions`, or `all` | +| `FEISHU_REQUIRE_MENTION` | — | `true` | Whether group messages must @mention the bot | | `FEISHU_HOME_CHANNEL` | — | — | Chat ID for cron/notification output | | `FEISHU_ENCRYPT_KEY` | — | _(empty)_ | Encrypt key for webhook signature verification | | `FEISHU_VERIFICATION_TOKEN` | — | _(empty)_ | Verification token for webhook payload auth | @@ -487,7 +520,9 @@ WebSocket and per-group ACL settings are configured via `config.yaml` under `pla | `Webhook rejected: invalid signature` | Ensure `FEISHU_ENCRYPT_KEY` matches the encrypt key in your Feishu app config | | Post messages show as plain text | The Feishu API rejected the post payload; this is normal fallback behavior. Check logs for details. | | Images/files not received by bot | Grant `im:message` and `im:resource` permission scopes to your Feishu app | -| Bot identity not auto-detected | Grant `admin:app.info:readonly` scope, or set `FEISHU_BOT_OPEN_ID` / `FEISHU_BOT_NAME` manually | +| Bot identity not auto-detected | Usually a transient network issue reaching Feishu's bot info endpoint. Set `FEISHU_BOT_OPEN_ID` and `FEISHU_BOT_NAME` manually as a workaround. | +| Peer bot messages still ignored after enabling `FEISHU_ALLOW_BOTS` | Hermes can't identify itself yet — set `FEISHU_BOT_OPEN_ID` (and `FEISHU_BOT_USER_ID` if your app uses `sender_id_type=user_id`). | +| Peer bots show as `ou_xxxxxx` instead of by name | Grant the `application:bot.basic_info:read` scope. | | Error 200340 when clicking approval buttons | Enable **Interactive Card** capability and configure **Card Request URL** in the Feishu Developer Console. See [Required Feishu App Configuration](#required-feishu-app-configuration) above. | | `Webhook rate limit exceeded` | More than 120 requests/minute from the same IP. This is usually a misconfiguration or loop. |