diff --git a/plugins/platforms/teams/adapter.py b/plugins/platforms/teams/adapter.py index 10a108803f..ce4db45b76 100644 --- a/plugins/platforms/teams/adapter.py +++ b/plugins/platforms/teams/adapter.py @@ -346,14 +346,15 @@ class TeamsAdapter(BasePlatformAdapter): def __init__(self, config: PlatformConfig): super().__init__(config, Platform("teams")) - extra = config.extra or {} + # Kept on the instance: ``platforms.teams.extra.*`` keys are read after construction too. + self._extra: Dict[str, Any] = config.extra or {} self._client_id, self._client_secret, self._tenant_id = _credentials(config) # (token, expiry monotonic ts) for connector attachment auth; refreshed under # _bf_token_lock so concurrent attachments can't stampede the STS. self._bf_token_cache: Optional[tuple] = None self._bf_token_lock: Optional[asyncio.Lock] = None - self._port = coerce_port(extra.get("port") or _get_scoped_secret("TEAMS_PORT", str(_DEFAULT_PORT)), _DEFAULT_PORT) - _raw_host = extra.get("host") or _get_scoped_secret("TEAMS_HOST", "") or _DEFAULT_HOST # falsy → dual-stack None + self._port = coerce_port(self._extra.get("port") or _get_scoped_secret("TEAMS_PORT", str(_DEFAULT_PORT)), _DEFAULT_PORT) + _raw_host = self._extra.get("host") or _get_scoped_secret("TEAMS_HOST", "") or _DEFAULT_HOST # falsy → dual-stack None self._host: Optional[str] = str(_raw_host) if _raw_host else None self._app: Optional["App"] = None self._runner: Optional["web.AppRunner"] = None @@ -363,7 +364,6 @@ class TeamsAdapter(BasePlatformAdapter): self._require_mention: bool = self._parse_require_mention(config) # Outbound activity ids (bounded) so require_mention can exempt replies to our own messages. self._sent_ids: deque = deque(maxlen=500) - self._sent_id_set: set = set() @staticmethod def _parse_require_mention(config) -> bool: @@ -371,15 +371,10 @@ class TeamsAdapter(BasePlatformAdapter): default as TELEGRAM_REQUIRE_MENTION). Without RSC Teams only delivers mention activities to a group bot anyway, so the gate changes nothing until the app gains ChannelMessage.Read.Group / ChatMessage.Read.Chat and starts receiving every conversation message.""" - configured = _extra_or_secret( - config.extra, - "require_mention", - "TEAMS_REQUIRE_MENTION", - False, - ) + configured = _extra_or_secret(config.extra, "require_mention", "TEAMS_REQUIRE_MENTION", False) if isinstance(configured, bool): return configured - return str(configured).lower() not in {"false", "0", "no", "off"} + return str(configured).strip().lower() not in {"false", "0", "no", "off"} async def connect(self, *, is_reconnect: bool = False) -> bool: # Reconnect paths reach here without create_adapter()'s installer — re-run to bind SDK globals. @@ -488,8 +483,12 @@ class TeamsAdapter(BasePlatformAdapter): async def _on_message(self, ctx: ActivityContext[MessageActivity]) -> None: activity = ctx.activity - bot_id = self._app.id if self._app else None - if bot_id and getattr(activity.from_, "id", None) == bot_id: + # Teams writes the bot's conversation identity as ``28:`` (activity.recipient) while + # App.id is the bare app id — accept both when deciding "is this us". + recipient_id = getattr(getattr(activity, "recipient", None), "id", None) + bot_ids = {i for i in (self._app.id if self._app else None, recipient_id) if isinstance(i, str) and i} + bot_ids |= {f"28:{i}" for i in tuple(bot_ids) if not i.startswith("28:")} + if getattr(activity.from_, "id", None) in bot_ids: return msg_id = getattr(activity, "id", None) if msg_id and self._dedup.is_duplicate(msg_id): @@ -499,17 +498,12 @@ class TeamsAdapter(BasePlatformAdapter): if conv_id: # cache the conversation reference for proactive sends (approval cards, etc.) self._conv_refs[conv_id] = ctx.conversation_ref text = activity.text if hasattr(activity, "text") and activity.text else "" - mentioned_bot = self._activity_mentions_bot(activity, bot_id, text) - non_personal = getattr(conv, "conversation_type", None) != "personal" - if self._require_mention and non_personal: - # RSC-delivered history: every conversation message arrives. Keep the ones that - # @mention the bot or reply to one of its own messages, drop the rest before - # attachment downloads make a gated message cost anything. - reply_to_bot = getattr(activity, "reply_to_id", None) in self._sent_id_set - if not mentioned_bot and not reply_to_bot: - logger.debug( - "[teams] Dropping non-personal message without a bot mention (chat=%s, msg=%s)", - conv_id, msg_id) + if self._require_mention and getattr(conv, "conversation_type", None) != "personal": + # RSC-delivered history: every channel/groupChat message arrives. Keep the ones that + # @mention the bot or reply to one of its own messages; drop the rest BEFORE the + # attachment loop so a gated post never downloads anything onto the host. + if not self._activity_mentions_bot(activity, bot_ids, text) and getattr(activity, "reply_to_id", None) not in self._sent_ids: + logger.debug("[teams] Dropping non-personal message without a bot mention (chat=%s, msg=%s)", conv_id, msg_id) return if "" in text: # strip the BotName tags Teams prepends for @mentions text = re.sub(r"[^<]*\s*", "", text).strip() @@ -530,19 +524,15 @@ class TeamsAdapter(BasePlatformAdapter): text=text, source=source, message_type=msg_type, message_id=msg_id, media_urls=[path for path, _, _ in media], media_types=[mt for _, mt, _ in media])) - def _activity_mentions_bot( - self, activity: Any, bot_id: Optional[str], text: str - ) -> bool: - """True when the activity carries a mention entity pointing at the bot, or — for payloads - where the entity list is absent — an ```` tag in the text (Teams' rendered mention form).""" - bot_id = bot_id or self._client_id - for entity in getattr(activity, "entities", None) or []: - if getattr(entity, "type", None) != "mention": - continue - mentioned = getattr(entity, "mentioned", None) - if mentioned and str(getattr(mentioned, "id", "")) == str(bot_id): - return True - return "" in text + @staticmethod + def _activity_mentions_bot(activity: Any, bot_ids: set, text: str) -> bool: + """True when a ``mention`` entity points at the bot (``mentioned.id`` is ``28:`` on the + wire; ``bot_ids`` carries both spellings). A payload with no mention entities at all falls back + to the rendered ```` tag; one that mentions only other people does not.""" + mentions = [e for e in getattr(activity, "entities", None) or [] if getattr(e, "type", None) == "mention"] + if not mentions: + return "" in text + return any(str(getattr(getattr(e, "mentioned", None), "id", "")) in bot_ids for e in mentions) async def _cache_attachment(self, att: Any) -> Optional[tuple]: """Download + cache one inbound attachment → ``(path, media_type, kind)`` or ``None``.""" @@ -618,14 +608,10 @@ class TeamsAdapter(BasePlatformAdapter): return result def _remember_sent(self, result: Any) -> None: - """Track an outbound activity id (bounded) for the require_mention reply exemption.""" + """Track an outbound activity id (bounded deque) for the require_mention reply exemption.""" sent_id = getattr(result, "id", None) - if not sent_id: - return - if len(self._sent_ids) == self._sent_ids.maxlen: - self._sent_id_set.discard(self._sent_ids[0]) - self._sent_ids.append(sent_id) - self._sent_id_set.add(sent_id) + if isinstance(sent_id, str) and sent_id: + self._sent_ids.append(sent_id) @staticmethod def _invoke_message(text: str) -> "InvokeResponse[AdaptiveCardActionMessageResponse]": diff --git a/plugins/platforms/teams/plugin.yaml b/plugins/platforms/teams/plugin.yaml index 6d6e30171f..b0d6394791 100644 --- a/plugins/platforms/teams/plugin.yaml +++ b/plugins/platforms/teams/plugin.yaml @@ -42,6 +42,10 @@ optional_env: description: "Allow any Teams user to trigger the bot (dev only)" prompt: "Allow all users? (true/false)" password: false + - name: TEAMS_REQUIRE_MENTION + description: "Only answer channel/group-chat messages that @mention the bot or reply to it (default false; needed once the app has RSC message-read consent)" + prompt: "Require @mention in channels and group chats? (true/false)" + password: false - name: TEAMS_HOME_CHANNEL description: "Default chat/channel ID for cron / notification delivery" prompt: "Home channel (or empty)" diff --git a/tests/gateway/test_teams.py b/tests/gateway/test_teams.py index b73e188e47..c309731a02 100644 --- a/tests/gateway/test_teams.py +++ b/tests/gateway/test_teams.py @@ -1116,135 +1116,79 @@ class TestTeamsMediaAttachments: # --------------------------------------------------------------------------- class TestTeamsRequireMention: - """With resource-specific consent the adapter receives every conversation - message, not just mentions — ``require_mention`` must gate non-personal - chats while personal chats and replies-to-the-bot stay ungated.""" + """With resource-specific consent Teams delivers every channel/groupChat message, not just + mentions. ``require_mention`` must drop unaddressed non-personal posts BEFORE the attachment + loop, keep @mentions (wire id ``28:``) / replies to the bot / personal chats, and be + read env-over-YAML like every other adapter.""" - def _make_adapter(self, require_mention=None, **extra): - if require_mention is not None: - extra["require_mention"] = require_mention + APP_ID = "bot-id" + + def _make_adapter(self, monkeypatch=None, **extra): adapter = TeamsAdapter(_make_config( - client_id="bot-id", client_secret="secret", tenant_id="tenant", **extra, - )) + client_id=self.APP_ID, client_secret="secret", tenant_id="tenant", **extra)) adapter._app = MagicMock() - adapter._app.id = "bot-id" + adapter._app.id = self.APP_ID adapter.handle_message = AsyncMock() + adapter._fetch_attachment_bytes = AsyncMock(return_value=b"\x89PNG" + b"\0" * 32) return adapter - def _make_activity( - self, - *, - text="Hello", - conversation_type="channel", - entities=None, - reply_to_id=None, - activity_id="activity-rm-001", - ): + def _activity(self, conversation_type, *, text="hello", mentioned_id=None, reply_to_id=None): activity = MagicMock() activity.text = text - activity.id = activity_id - activity.from_ = MagicMock() - activity.from_.id = "user-123" - activity.from_.aad_object_id = "aad-456" - activity.from_.name = "Test User" - activity.conversation = MagicMock() - activity.conversation.id = "19:channel@thread.v2" - activity.conversation.conversation_type = conversation_type - activity.conversation.name = "Channel" - activity.conversation.tenant_id = "tenant-789" - activity.attachments = [] - activity.entities = entities or [] + activity.id = f"act-{conversation_type}-{mentioned_id}-{reply_to_id}" + activity.from_ = MagicMock(aad_object_id="aad-456", name="Test User") + activity.from_.id = "29:user-123" + activity.recipient = MagicMock() + activity.recipient.id = f"28:{self.APP_ID}" + activity.conversation = MagicMock(conversation_type=conversation_type, tenant_id="t") + activity.conversation.id = "19:conv@thread.v2" + activity.conversation.name = "Conv" + att = MagicMock(content_type="image/png") + att.name = "a.png" + att.content_url = "https://smba.trafficmanager.net/emea/v3/attachments/1/views/original" + activity.attachments = [att] activity.reply_to_id = reply_to_id + activity.entities = [] + if mentioned_id: + entity = MagicMock(type="mention") + entity.mentioned = MagicMock() + entity.mentioned.id = mentioned_id + activity.entities = [entity] return activity - def _mention_entity(self, mentioned_id="bot-id"): - entity = MagicMock() - entity.type = "mention" - entity.mentioned = MagicMock() - entity.mentioned.id = mentioned_id - return entity - - def _make_ctx(self, activity): + @pytest.mark.anyio + @pytest.mark.parametrize("conversation_type, kwargs, dispatched", [ + ("channel", {}, False), + ("groupChat", {}, False), + ("channel", {"text": "Alice hi", "mentioned_id": "29:alice"}, False), # someone else + ("channel", {"text": "Hermes hi", "mentioned_id": "28:bot-id"}, True), # wire form of the bot id + ("groupChat", {"text": "Hermes hi", "mentioned_id": "bot-id"}, True), + ("channel", {"reply_to_id": "bot-msg-1"}, True), + ("personal", {}, True), + ]) + async def test_gate_drops_unaddressed_non_personal_before_attachment_download( + self, conversation_type, kwargs, dispatched, + ): + adapter = self._make_adapter(require_mention=True) + adapter._sent_ids.append("bot-msg-1") ctx = MagicMock() - ctx.activity = activity - return ctx + ctx.activity = self._activity(conversation_type, **kwargs) + await adapter._on_message(ctx) + assert adapter.handle_message.await_count == (1 if dispatched else 0) + assert adapter._fetch_attachment_bytes.await_count == (1 if dispatched else 0) - @pytest.mark.asyncio - async def test_channel_message_without_mention_is_dropped_when_enabled(self): - adapter = self._make_adapter(require_mention=True) - await adapter._on_message(self._make_ctx(self._make_activity())) - adapter.handle_message.assert_not_awaited() - - @pytest.mark.asyncio - async def test_channel_message_with_mention_entity_passes_when_enabled(self): - adapter = self._make_adapter(require_mention=True) - activity = self._make_activity( - text="Hermes run the report", entities=[self._mention_entity()] - ) - await adapter._on_message(self._make_ctx(activity)) - adapter.handle_message.assert_awaited_once() - - @pytest.mark.asyncio - async def test_group_message_with_at_tag_only_passes_when_enabled(self): - # Payloads without an entity list still carry the rendered mention form. - adapter = self._make_adapter(require_mention=True) - activity = self._make_activity( - text="Hermes status?", conversation_type="groupChat") - await adapter._on_message(self._make_ctx(activity)) - adapter.handle_message.assert_awaited_once() - - @pytest.mark.asyncio - async def test_mention_of_another_user_is_dropped_when_enabled(self): - adapter = self._make_adapter(require_mention=True) - activity = self._make_activity( - entities=[self._mention_entity(mentioned_id="other-user")] - ) - await adapter._on_message(self._make_ctx(activity)) - adapter.handle_message.assert_not_awaited() - - @pytest.mark.asyncio - async def test_reply_to_bot_message_passes_when_enabled(self): - adapter = self._make_adapter(require_mention=True) - adapter._remember_sent(MagicMock(id="bot-msg-7")) - activity = self._make_activity(reply_to_id="bot-msg-7") - await adapter._on_message(self._make_ctx(activity)) - adapter.handle_message.assert_awaited_once() - - @pytest.mark.asyncio - async def test_reply_to_foreign_message_is_dropped_when_enabled(self): - adapter = self._make_adapter(require_mention=True) - adapter._remember_sent(MagicMock(id="bot-msg-7")) - activity = self._make_activity(reply_to_id="someone-else-msg") - await adapter._on_message(self._make_ctx(activity)) - adapter.handle_message.assert_not_awaited() - - @pytest.mark.asyncio - async def test_personal_chat_stays_ungated_when_enabled(self): - adapter = self._make_adapter(require_mention=True) - activity = self._make_activity(conversation_type="personal") - await adapter._on_message(self._make_ctx(activity)) - adapter.handle_message.assert_awaited_once() - - @pytest.mark.asyncio - async def test_gate_inactive_by_default(self): - # Opt-in default (same as TELEGRAM_REQUIRE_MENTION): without RSC Teams only - # delivers mention activities, so an ungated adapter keeps today's behaviour. - adapter = self._make_adapter() - await adapter._on_message(self._make_ctx(self._make_activity())) - adapter.handle_message.assert_awaited_once() - - @pytest.mark.asyncio - async def test_env_override_enables_gate(self, monkeypatch): - monkeypatch.setenv("TEAMS_REQUIRE_MENTION", "true") - adapter = self._make_adapter() - assert adapter._require_mention is True - await adapter._on_message(self._make_ctx(self._make_activity())) - adapter.handle_message.assert_not_awaited() - - def test_sent_id_tracking_is_bounded(self): - adapter = self._make_adapter() - for i in range(600): - adapter._remember_sent(MagicMock(id=f"sent-{i}")) - assert len(adapter._sent_ids) == 500 - assert "sent-0" not in adapter._sent_id_set - assert "sent-599" in adapter._sent_id_set + @pytest.mark.parametrize("yaml_value, env_value, expected", [ + (None, None, False), # opt-in: absent key leaves every conversation ungated + (True, None, True), + ("false", None, False), + (True, "false", False), # explicit env beats YAML, like MATRIX_/MATTERMOST_REQUIRE_MENTION + (False, "true", True), + ]) + def test_require_mention_read_env_over_yaml(self, monkeypatch, yaml_value, env_value, expected): + monkeypatch.delenv("TEAMS_REQUIRE_MENTION", raising=False) + if env_value is not None: + monkeypatch.setenv("TEAMS_REQUIRE_MENTION", env_value) + extra = {} if yaml_value is None else {"require_mention": yaml_value} + adapter = self._make_adapter(**extra) + assert adapter._require_mention is expected + assert adapter._extra.get("require_mention") == yaml_value # extras stay readable on the instance diff --git a/website/docs/user-guide/messaging/teams.md b/website/docs/user-guide/messaging/teams.md index d64efd8a03..45af6d85d1 100644 --- a/website/docs/user-guide/messaging/teams.md +++ b/website/docs/user-guide/messaging/teams.md @@ -22,6 +22,8 @@ Need meeting summaries from Microsoft Graph events rather than normal bot conver Teams delivers @mentions as regular messages with `BotName` tags, which Hermes strips automatically before processing. +Without resource-specific consent (RSC) Teams only delivers messages that @mention the bot, so no filtering is needed. Once the app manifest grants `ChannelMessage.Read.Group` or `ChatMessage.Read.Chat`, Teams delivers **every** message in the conversation — set `require_mention: true` (or `TEAMS_REQUIRE_MENTION=true`) so the bot only answers channel/group-chat messages that @mention it or reply to one of its own messages. Personal chats are never gated, and a gated message is dropped before its attachments are downloaded. + --- For source or local installs, include the Teams extra so the bundled adapter can @@ -168,6 +170,7 @@ Open the printed link in your browser — it opens directly in the Teams client. | `TEAMS_HOME_CHANNEL` | Conversation ID for cron/proactive message delivery | | `TEAMS_HOME_CHANNEL_NAME` | Display name for the home channel | | `TEAMS_PORT` | Webhook port (default: `3978`) | +| `TEAMS_REQUIRE_MENTION` | Set `true` to answer only @mentions / replies to the bot in channels and group chats (default: `false`; for apps with RSC message-read consent) | ### config.yaml @@ -182,6 +185,7 @@ platforms: client_secret: "your-secret" tenant_id: "your-tenant-id" port: 3978 + require_mention: false # true once the app has RSC message-read consent ``` ---