fix: sanitize qqofficial command tags#8756
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a sanitization method, _sanitize_command_interaction_tags, to extract text from QQ Official command interaction tags within incoming messages, along with corresponding unit tests. The review feedback highlights potential AttributeError crashes if message.content is None (such as in attachment-only messages) when calling .strip() or .replace(). It is recommended to add defensive checks to handle None content gracefully and to include a test case covering this scenario.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| abm.message_str = ( | ||
| QQOfficialPlatformAdapter._sanitize_command_interaction_tags( | ||
| QQOfficialPlatformAdapter._parse_face_message( | ||
| message.content.strip() | ||
| ) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
If message.content is None (which can happen if the message contains only attachments/images and no text), calling message.content.strip() will raise an AttributeError and crash the message handler. We should add a defensive check to handle None content gracefully.
raw_content = message.content.strip() if message.content else ""
abm.message_str = (
QQOfficialPlatformAdapter._sanitize_command_interaction_tags(
QQOfficialPlatformAdapter._parse_face_message(raw_content)
)
)| plain_content = ( | ||
| QQOfficialPlatformAdapter._sanitize_command_interaction_tags( | ||
| QQOfficialPlatformAdapter._parse_face_message( | ||
| message.content.replace( | ||
| "<@!" + str(abm.self_id) + ">", | ||
| "", | ||
| ).strip() | ||
| ) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Similarly, if message.content is None, calling message.content.replace(...) will raise an AttributeError. We should safely default to an empty string if message.content is None.
| plain_content = ( | |
| QQOfficialPlatformAdapter._sanitize_command_interaction_tags( | |
| QQOfficialPlatformAdapter._parse_face_message( | |
| message.content.replace( | |
| "<@!" + str(abm.self_id) + ">", | |
| "", | |
| ).strip() | |
| ) | |
| ) | |
| ) | |
| raw_content = message.content or "" | |
| plain_content = ( | |
| QQOfficialPlatformAdapter._sanitize_command_interaction_tags( | |
| QQOfficialPlatformAdapter._parse_face_message( | |
| raw_content.replace( | |
| "<@!" + str(abm.self_id) + ">", | |
| "", | |
| ).strip() | |
| ) | |
| ) | |
| ) |
| @pytest.mark.asyncio | ||
| async def test_parse_from_qqofficial_sanitizes_message_str_and_plain_component( | ||
| monkeypatch, | ||
| ) -> None: | ||
| monkeypatch.setattr( | ||
| qqofficial_platform_adapter.botpy.message, | ||
| "GroupMessage", | ||
| FakeGroupMessage, | ||
| ) | ||
| raw_message = FakeGroupMessage( | ||
| 'hello <qqbot-cmd-input text="/quick-map" show="quick map" />' | ||
| ) | ||
|
|
||
| message = await QQOfficialPlatformAdapter._parse_from_qqofficial( | ||
| raw_message, | ||
| MessageType.GROUP_MESSAGE, | ||
| ) | ||
|
|
||
| assert message.message_str == "hello /quick-map" | ||
| assert _plain_texts(message.message) == ["hello /quick-map"] |
There was a problem hiding this comment.
It would be beneficial to add a test case to verify that the parser handles None content gracefully without crashing, especially since we added defensive checks for it.
@pytest.mark.asyncio
async def test_parse_from_qqofficial_sanitizes_message_str_and_plain_component(
monkeypatch,
) -> None:
monkeypatch.setattr(
qqofficial_platform_adapter.botpy.message,
"GroupMessage",
FakeGroupMessage,
)
raw_message = FakeGroupMessage(
'hello <qqbot-cmd-input text="/quick-map" show="quick map" />'
)
message = await QQOfficialPlatformAdapter._parse_from_qqofficial(
raw_message,
MessageType.GROUP_MESSAGE,
)
assert message.message_str == "hello /quick-map"
assert _plain_texts(message.message) == ["hello /quick-map"]
@pytest.mark.asyncio
async def test_parse_from_qqofficial_handles_none_content(
monkeypatch,
) -> None:
monkeypatch.setattr(
qqofficial_platform_adapter.botpy.message,
"GroupMessage",
FakeGroupMessage,
)
raw_message = FakeGroupMessage(None)
message = await QQOfficialPlatformAdapter._parse_from_qqofficial(
raw_message,
MessageType.GROUP_MESSAGE,
)
assert message.message_str == ""
assert _plain_texts(message.message) == [""]References
- New functionality, such as handling attachments, should be accompanied by corresponding unit tests.
dosubot
Bot
added
size:M
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
test_parse_from_qqofficial_handles_none_guild_contentyou passMessageType.GROUP_MESSAGEwhile monkeypatchingbotpy.message.MessagetoFakeGuildMessage, which may mean this test never actually exercises the guild-specific code path; consider using the appropriate message type to align the test with its intent. - The logic that strips
message.content, sanitizes command interaction tags, and parses face messages is duplicated between the group and guild branches in_parse_from_qqofficial; consider extracting this into a small helper to keep the normalization path consistent and reduce maintenance overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `test_parse_from_qqofficial_handles_none_guild_content` you pass `MessageType.GROUP_MESSAGE` while monkeypatching `botpy.message.Message` to `FakeGuildMessage`, which may mean this test never actually exercises the guild-specific code path; consider using the appropriate message type to align the test with its intent.
- The logic that strips `message.content`, sanitizes command interaction tags, and parses face messages is duplicated between the group and guild branches in `_parse_from_qqofficial`; consider extracting this into a small helper to keep the normalization path consistent and reduce maintenance overhead.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
问题2 跨平台显示异常：如果通过 AstrBot 将消息路由、转发或同步到其他非 QQ 平台（如微信、Telegram、KOOK 或 WebUI），这些标签会作为纯文本原样显示，造成严重的阅读干扰和排版混乱。这个并没有解决哎 |
|
@Zzzzzzouhang 感谢反馈。关于「问题2 跨平台显示异常」，想先同步一下当前实现的覆盖范围，再确认是否还有遗漏的场景。 最新的提交（ abm.message_str = ..._normalize_message_content(message.content)
msg.append(Plain(abm.message_str)) # 跨平台转发用的就是这个 Plain所以 如果你在实测中仍看到标签泄漏，能否补充一下具体路径？尤其是：转发是通过哪个插件 / 机制做的？因为有些同步插件会直接读取 另外 issue 里提到的「框架层全局清洗配置」（可自定义需剥离的标签正则列表）是个更通用的特性，影响面比这个 qqofficial 适配器修复大不少。按贡献规范，这类新特性更适合单独开一个 PR 并先和维护者对齐设计，避免把这个聚焦的 bugfix 撑得过大。你觉得呢？ |
2 participants
Summary
textattribute when presentFixes #8755
Root Cause
QQ Official can include platform-specific command interaction tags in incoming
message content. AstrBot parsed that content directly into
message_strand thePlaincomponent, so the raw tag could reach LLM context or cross-platformdisplay.
Tests
UV_PROJECT_ENVIRONMENT=/tmp/astrbot-venv-8755 uv run pytest -q tests/test_qqofficial_platform_adapter.pyUV_PROJECT_ENVIRONMENT=/tmp/astrbot-venv-8755 uv run ruff format --check .UV_PROJECT_ENVIRONMENT=/tmp/astrbot-venv-8755 uv run ruff check .UV_PROJECT_ENVIRONMENT=/tmp/astrbot-venv-8755 uv run pytest -q tests/test_neo_skill_sync.py tests/test_neo_skill_tools.py tests/test_computer_skill_sync.py tests/test_skill_manager_sandbox_cache.py tests/test_dashboard.py::test_neo_skills_routesNote: the exact
make pr-test-neotarget was not available in this WindowsPowerShell environment, so I ran its lint and Neo pytest portions under WSL with
a Linux
uvenvironment.Summary by Sourcery
Sanitize QQ Official incoming message content by stripping or replacing platform-specific command interaction tags while preserving user-visible text, and add tests to cover the sanitizer and QQ Official message parsing.
Bug Fixes:
Tests: