diff --git a/backend/app/channels/dingtalk.py b/backend/app/channels/dingtalk.py index af8beef19..3e4254d6b 100644 --- a/backend/app/channels/dingtalk.py +++ b/backend/app/channels/dingtalk.py @@ -416,6 +416,19 @@ class DingTalkChannel(Channel): # chat_id uses conversation_id for groups, sender_staff_id for P2P chat_id = conversation_id if conversation_type == _CONVERSATION_TYPE_GROUP else sender_staff_id + # An empty chat_id does not identify a conversation, and ChannelStore keys on it: + # a P2P message with no sender keys to the literal "dingtalk:", so every such + # message from every user shares one thread and one history. Mirrors the guard the + # WeChat channel already applies (wechat.py, `if not chat_id: return`) and the one + # this file's own bind path applies before persisting a connection. + if not chat_id: + logger.warning( + "[DingTalk] ignoring message with no conversation identity: conv_type=%s, msg_id=%s", + conversation_type, + msg_id, + ) + return + inbound = self._make_inbound( chat_id=chat_id, user_id=sender_staff_id, diff --git a/backend/tests/test_dingtalk_channel.py b/backend/tests/test_dingtalk_channel.py index f2a65c625..6138d62dc 100644 --- a/backend/tests/test_dingtalk_channel.py +++ b/backend/tests/test_dingtalk_channel.py @@ -334,6 +334,102 @@ class TestOnChatbotMessage: _run(go()) + @pytest.mark.parametrize("sender_staff_id", ["", None]) + def test_p2p_message_without_sender_is_dropped(self, sender_staff_id): + """A P2P chat_id *is* the sender, so an empty one keys every user to one thread. + + ``ChannelStore._key`` builds ``f"{channel}:{chat_id}"`` for a topic-less conversation, + so publishing this would put every senderless P2P message under the literal + ``"dingtalk:"`` — one shared thread, one shared history, across users. + """ + + async def go(): + bus = MessageBus() + bus.publish_inbound = AsyncMock() + channel = DingTalkChannel(bus, config={}) + channel._client_id = "test_key" + channel._main_loop = asyncio.get_event_loop() + channel._running = True + + msg = _make_chatbot_message( + text="hello", + conversation_type=_CONVERSATION_TYPE_P2P, + sender_staff_id=sender_staff_id, + message_id="msg_no_sender", + ) + + channel._send_running_reply = AsyncMock() + channel._on_chatbot_message(msg) + + await asyncio.sleep(0.1) + + bus.publish_inbound.assert_not_awaited() + + _run(go()) + + def test_group_message_without_conversation_id_is_dropped(self): + """The group route reaches the same degenerate key from the other side.""" + + async def go(): + bus = MessageBus() + bus.publish_inbound = AsyncMock() + channel = DingTalkChannel(bus, config={}) + channel._client_id = "test_key" + channel._main_loop = asyncio.get_event_loop() + channel._running = True + + msg = _make_chatbot_message( + text="hello group", + conversation_type=_CONVERSATION_TYPE_GROUP, + sender_staff_id="user_002", + conversation_id="", + message_id="msg_no_conv", + ) + + channel._send_running_reply = AsyncMock() + channel._on_chatbot_message(msg) + + await asyncio.sleep(0.1) + + bus.publish_inbound.assert_not_awaited() + + _run(go()) + + def test_group_message_with_unknown_sender_is_still_delivered(self): + """Reverse anchor: the guard is on the conversation identity, not on the sender. + + A group message identifies its conversation through ``conversation_id``, so an + unknown sender must not cost the whole message. + """ + + async def go(): + bus = MessageBus() + bus.publish_inbound = AsyncMock() + channel = DingTalkChannel(bus, config={}) + channel._client_id = "test_key" + channel._main_loop = asyncio.get_event_loop() + channel._running = True + + msg = _make_chatbot_message( + text="hello group", + conversation_type=_CONVERSATION_TYPE_GROUP, + sender_staff_id="", + conversation_id="conv_group_003", + message_id="msg_group_003", + ) + + channel._send_running_reply = AsyncMock() + channel._on_chatbot_message(msg) + + await asyncio.sleep(0.1) + + bus.publish_inbound.assert_awaited_once() + inbound = bus.publish_inbound.await_args.args[0] + assert inbound.chat_id == "conv_group_003" + assert inbound.topic_id == "msg_group_003" + + _run(go()) + def test_command_classified_correctly(self): async def go(): bus = MessageBus()