fix(dingtalk): harden group reply sender labels
This commit is contained in:
@@ -24,6 +24,17 @@ from nanobot.security.network import validate_resolved_url, validate_url_target
|
||||
|
||||
DINGTALK_MAX_REMOTE_MEDIA_BYTES = 20 * 1024 * 1024
|
||||
DINGTALK_MAX_REMOTE_MEDIA_REDIRECTS = 3
|
||||
_DINGTALK_MARKDOWN_INLINE_SPECIALS = frozenset(r"\`*_{}[]()<>#+-.!|~")
|
||||
_DINGTALK_SENDER_NAME_MAX_CHARS = 80
|
||||
|
||||
|
||||
def _escape_markdown_sender_name(value: str) -> str:
|
||||
"""Render an untrusted display name as one bounded Markdown-safe line."""
|
||||
normalized = " ".join(value.split())[:_DINGTALK_SENDER_NAME_MAX_CHARS]
|
||||
return "".join(
|
||||
f"\\{char}" if char in _DINGTALK_MARKDOWN_INLINE_SPECIALS else char
|
||||
for char in normalized
|
||||
)
|
||||
|
||||
try:
|
||||
from dingtalk_stream import (
|
||||
@@ -719,8 +730,13 @@ class DingTalkChannel(BaseChannel):
|
||||
# sender so the addressed user can spot the reply. Visual only —
|
||||
# DingTalk's markdown robot messages do not push real @ notifications.
|
||||
sender_name = msg.metadata.get("sender_name") if msg.metadata else None
|
||||
if msg.chat_id.startswith("group:") and sender_name:
|
||||
content = f"# @{sender_name}\n\n{content}"
|
||||
safe_sender_name = (
|
||||
_escape_markdown_sender_name(sender_name)
|
||||
if isinstance(sender_name, str)
|
||||
else ""
|
||||
)
|
||||
if msg.chat_id.startswith("group:") and safe_sender_name:
|
||||
content = f"# @{safe_sender_name}\n\n{content}"
|
||||
if not await self._send_markdown_text(token, msg.chat_id, content):
|
||||
raise RuntimeError("DingTalk text message was not delivered")
|
||||
|
||||
@@ -741,7 +757,7 @@ class DingTalkChannel(BaseChannel):
|
||||
async def _on_message(
|
||||
self,
|
||||
content: str,
|
||||
sender_id: str,
|
||||
sender_id: str | None,
|
||||
sender_name: str,
|
||||
conversation_type: str | None = None,
|
||||
conversation_id: str | None = None,
|
||||
@@ -753,6 +769,9 @@ class DingTalkChannel(BaseChannel):
|
||||
"""
|
||||
try:
|
||||
self.logger.info("inbound: {} from {}", content, sender_name)
|
||||
if not sender_id:
|
||||
self.logger.warning("dropping DingTalk message without a sender ID")
|
||||
return
|
||||
is_group = conversation_type == "2" and conversation_id
|
||||
chat_id = f"group:{conversation_id}" if is_group else sender_id
|
||||
session_key = None
|
||||
@@ -768,7 +787,7 @@ class DingTalkChannel(BaseChannel):
|
||||
await self.send(
|
||||
OutboundMessage(
|
||||
channel=self.name,
|
||||
chat_id=str(chat_id), # str() guards a None sender_id
|
||||
chat_id=chat_id,
|
||||
content="该机器人未开启私聊,请在群聊中与我对话。",
|
||||
)
|
||||
)
|
||||
|
||||
@@ -10,15 +10,15 @@ import pytest
|
||||
|
||||
# Check optional dingtalk dependencies before running tests
|
||||
try:
|
||||
from nanobot.channels import dingtalk
|
||||
DINGTALK_AVAILABLE = getattr(dingtalk, "DINGTALK_AVAILABLE", False)
|
||||
import nanobot.channels.dingtalk.runtime as dingtalk_module
|
||||
|
||||
DINGTALK_AVAILABLE = dingtalk_module.DINGTALK_AVAILABLE
|
||||
except ImportError:
|
||||
DINGTALK_AVAILABLE = False
|
||||
|
||||
if not DINGTALK_AVAILABLE:
|
||||
pytest.skip("DingTalk dependencies not installed (dingtalk-stream)", allow_module_level=True)
|
||||
|
||||
import nanobot.channels.dingtalk.runtime as dingtalk_module
|
||||
from nanobot.bus.events import OutboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.channels.dingtalk.runtime import (
|
||||
@@ -154,6 +154,13 @@ async def test_group_user_isolation_true_separates_sessions() -> None:
|
||||
assert msg1.chat_id == msg2.chat_id == "group:conv123"
|
||||
|
||||
|
||||
def test_disable_private_chat_uses_camel_case_config_key() -> None:
|
||||
config = DingTalkConfig.model_validate({"disablePrivateChat": True})
|
||||
|
||||
assert config.disable_private_chat is True
|
||||
assert config.model_dump(mode="json", by_alias=True)["disablePrivateChat"] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dm_rejected_when_private_chat_disabled(monkeypatch) -> None:
|
||||
"""With disable_private_chat=True, a 1:1 DM is rejected: nothing reaches the
|
||||
@@ -278,6 +285,31 @@ async def test_group_send_prepends_sender_mention(monkeypatch) -> None:
|
||||
assert sent_text == "# @Alice\n\nhello"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_group_send_escapes_untrusted_sender_name(monkeypatch) -> None:
|
||||
"""A sender nickname cannot inject extra Markdown blocks into the reply."""
|
||||
config = DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"])
|
||||
channel = DingTalkChannel(config, MessageBus())
|
||||
channel._http = _FakeHttp()
|
||||
|
||||
async def _fake_token() -> str:
|
||||
return "token"
|
||||
|
||||
monkeypatch.setattr(channel, "_get_access_token", _fake_token)
|
||||
|
||||
await channel.send(
|
||||
OutboundMessage(
|
||||
channel="dingtalk",
|
||||
chat_id="group:conv123",
|
||||
content="hello",
|
||||
metadata={"sender_name": "Alice\n# [click](https://evil) *admin*"},
|
||||
)
|
||||
)
|
||||
|
||||
sent_text = json.loads(channel._http.calls[0]["json"]["msgParam"])["text"]
|
||||
assert sent_text == r"# @Alice \# \[click\]\(https://evil\) \*admin\*" + "\n\nhello"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_private_send_does_not_prepend_mention(monkeypatch) -> None:
|
||||
"""Private replies are sent verbatim, without the sender header."""
|
||||
@@ -303,6 +335,30 @@ async def test_private_send_does_not_prepend_mention(monkeypatch) -> None:
|
||||
assert sent_text == "hello"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_message_without_sender_id_is_dropped() -> None:
|
||||
"""Malformed inbound events must not publish or attempt an invalid reply."""
|
||||
config = DingTalkConfig(
|
||||
client_id="app",
|
||||
client_secret="secret",
|
||||
allow_from=["*"],
|
||||
disable_private_chat=True,
|
||||
)
|
||||
bus = MessageBus()
|
||||
channel = DingTalkChannel(config, bus)
|
||||
channel._http = _FakeHttp()
|
||||
|
||||
await channel._on_message(
|
||||
"hello",
|
||||
sender_id=None,
|
||||
sender_name="Unknown",
|
||||
conversation_type="1",
|
||||
)
|
||||
|
||||
assert bus.inbound.empty()
|
||||
assert channel._http.calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handler_uses_voice_recognition_text_when_text_is_empty(monkeypatch) -> None:
|
||||
bus = MessageBus()
|
||||
|
||||
Reference in New Issue
Block a user