Feat: qqofficial webhook raw event bypass#6691
Open
KBVsent wants to merge 6 commits intoAstrBotDevs:masterfrom
Open
Feat: qqofficial webhook raw event bypass#6691KBVsent wants to merge 6 commits intoAstrBotDevs:masterfrom
KBVsent wants to merge 6 commits intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
the
size:XL
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在增强框架的事件处理能力，允许插件直接访问和处理来自平台（特别是QQ官方平台）的原始事件，这些事件在过去可能被框架忽略。通过引入原始事件钩子和传播控制机制，插件现在可以根据这些底层事件实现更复杂的业务逻辑，例如统计Bot的入群退群情况或处理其他特定平台事件，从而提升了框架的灵活性和可扩展性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
dosubot
bot
added
area:platform
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
QQOfficialWebhook.handle_callback, the raw platform event is emitted before the validation branch runs, which means plugins can observe and react to unauthenticated/invalid webhook payloads; consider movingemit_raw_platform_eventafter successful validation (and any signature checks) so hooks only see trusted events. - The
_astrbot_configfield onPlatformis injected and mutated externally inPlatformManagerand onwebchat_inst; it would be cleaner and less error-prone to pass this via the constructor or a dedicated setter instead of relying on external writes to a protected attribute.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `QQOfficialWebhook.handle_callback`, the raw platform event is emitted before the validation branch runs, which means plugins can observe and react to unauthenticated/invalid webhook payloads; consider moving `emit_raw_platform_event` after successful validation (and any signature checks) so hooks only see trusted events.
- The `_astrbot_config` field on `Platform` is injected and mutated externally in `PlatformManager` and on `webchat_inst`; it would be cleaner and less error-prone to pass this via the constructor or a dedicated setter instead of relying on external writes to a protected attribute.
## Individual Comments
### Comment 1
<location path="astrbot/core/pipeline/context_utils.py" line_range="134-141" />
<code_context>
+ if raw_event_type and raw_event_type != event.event_type:
+ continue
+
+ try:
+ assert inspect.iscoroutinefunction(handler.handler)
+ logger.debug(
+ f"hook({hook_type.name}) -> {star_map[handler.handler_module_path].name} - {handler.handler_name}",
+ )
+ await handler.handler(event)
+ except BaseException:
+ logger.error(traceback.format_exc())
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Catching `BaseException` is very broad; consider narrowing this to `Exception` unless you explicitly need to handle system-exiting errors.
This `except BaseException` will also catch `KeyboardInterrupt`, `SystemExit`, and other system-level exceptions, potentially masking shutdown signals or critical failures from plugin code. Unless you truly need to intercept those, prefer `except Exception:` here so those signals can still propagate.
```suggestion
try:
assert inspect.iscoroutinefunction(handler.handler)
logger.debug(
f"hook({hook_type.name}) -> {star_map[handler.handler_module_path].name} - {handler.handler_name}",
)
await handler.handler(event)
except Exception:
logger.error(traceback.format_exc())
```
</issue_to_address>
### Comment 2
<location path="tests/unit/test_raw_platform_event.py" line_range="245-246" />
<code_context>
+ assert result is False
+
+
+class TestEmitRawPlatformEventPluginSet:
+ """Tests for Platform.emit_raw_platform_event plugin_set filtering."""
+
</code_context>
<issue_to_address>
**suggestion (testing):** Assert that emit_raw_platform_event propagates and returns the underlying hook result, not just adjusts plugins_name.
The current tests cover `plugins_name` behavior well, but they don’t verify that `Platform.emit_raw_platform_event` returns the boolean from `call_raw_platform_event_hook`. Since callers (e.g. the QQ webhook server) rely on this return value to decide whether to short‑circuit dispatch, please add a test that mocks `call_raw_platform_event_hook` to return `True` and asserts `await platform.emit_raw_platform_event(...)` is `True` (and optionally another for `False`). This will confirm the stop‑propagation semantics are preserved end‑to‑end.
```suggestion
class TestEmitRawPlatformEventPluginSet:
"""Tests for Platform.emit_raw_platform_event plugin_set filtering."""
@pytest.mark.asyncio
@patch("astrbot.core.platform.platform.call_raw_platform_event_hook", new_callable=AsyncMock)
async def test_emit_raw_platform_event_propagates_true(self, mock_call_raw_platform_event_hook, platform):
"""emit_raw_platform_event should propagate True from call_raw_platform_event_hook."""
mock_call_raw_platform_event_hook.return_value = True
event = RawPlatformEvent(
platform="qq",
event_type="test_event",
raw_payload={"key": "value"},
)
result = await platform.emit_raw_platform_event(event)
assert result is True
mock_call_raw_platform_event_hook.assert_awaited_once_with(event)
@pytest.mark.asyncio
@patch("astrbot.core.platform.platform.call_raw_platform_event_hook", new_callable=AsyncMock)
async def test_emit_raw_platform_event_propagates_false(self, mock_call_raw_platform_event_hook, platform):
"""emit_raw_platform_event should propagate False from call_raw_platform_event_hook."""
mock_call_raw_platform_event_hook.return_value = False
event = RawPlatformEvent(
platform="qq",
event_type="test_event",
raw_payload={"key": "value"},
)
result = await platform.emit_raw_platform_event(event)
assert result is False
mock_call_raw_platform_event_hook.assert_awaited_once_with(event)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
关联issue: #6304
QQ Official平台支持推送多种事件，包括Bot被拉入群，退群等事件，目前关于这部分事件是直接丢弃的状态，插件也没有办法直接获取到这部分时间，因此本pr提供一个原始事件钩子，允许插件读取原始事件来根据平台特殊事件类型进行业务逻辑操作。
Modifications / 改动点
RawPlatformEvent，提供平台元信息访问、extra 数据存取、stop_event/continue_event传播控制。emit_raw_platform_event(...)；接入全局配置并在未显式传参时应用plugin_set过滤。call_raw_platform_event_hook(...)，支持按platform_name/platform_id/event_type过滤并处理停止传播语义。register_on_raw_platform_event(...)注册器。EventType.OnRawPlatformEvent及对应 registry 类型声明。_astrbot_config，保证 raw-event 路径可读取plugin_set。plugin_set行为。Screenshots or Test Results / 运行截图或测试结果
uv run pytest test_raw_platform_event.py -q结果：
10 passed, 3 warnings in 1.58sChecklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Add framework-level support for emitting and handling raw platform events, and wire QQ Official webhook events into this new hook with optional plugin-based filtering and propagation control.
New Features:
Enhancements:
Tests: