feat: 为插件添加统一webhook#5758
Conversation
dosubot
Bot
added
the
size:M
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! 此拉取请求旨在通过引入统一的 Webhook 注册和分发机制，增强插件系统的功能。此前，插件无法像平台适配器那样通过统一入口接收外部回调，限制了其与外部服务的集成能力。本次变更通过在核心上下文中建立 Webhook 注册表、为插件提供便捷的注册方法以及在 Dashboard 中实现专用的路由和分发逻辑，解决了这一问题。这使得插件能够无缝地接收和处理来自外部系统的回调，从而扩展了其潜在的应用场景和集成能力。 Highlights
Changelog
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并留下了一些整体性的反馈：
- 统一 webhook 注册表目前作为类级可变字典（
registered_unified_webhooks）存储在Context上，这可能会在多个Context实例之间泄露状态（例如在测试或多进程/多线程环境中）；建议将其移动为在__init__中初始化的实例属性，以获得更好的隔离性。 - 在
srv_plug_webhook_route中，你将request.method直接与已存储的methods列表进行比较，而该列表已被规范化为大写；为了避免任何细微的大小写问题并让逻辑更加自洽，建议改为使用if request.method.upper() not in methods:。 - 鉴权白名单中添加的是
"/api/plug/webhook"，而实际路由是"/api/plug/webhook/<webhook_uuid>"；请仔细检查匹配逻辑（例如是全等匹配还是前缀匹配）是否确实把所有 webhook 回调都视作在白名单中，并根据/api/platform/webhook的处理方式调整字符串或条件以保持一致。
给 AI Agent 的提示词
请根据这次代码评审中的评论进行修改：
## 整体性评论
- 统一 webhook 注册表目前作为类级可变字典（`registered_unified_webhooks`）存储在 `Context` 上，这可能会在多个 `Context` 实例之间泄露状态（例如在测试或多进程/多核环境中）；建议将其移动为在 `__init__` 中初始化的实例属性，以获得更好的隔离性。
- 在 `srv_plug_webhook_route` 中，你将 `request.method` 直接与已存储的 `methods` 列表进行比较，而该列表已被规范化为大写；为了避免任何细微的大小写问题并让逻辑更加自洽，建议改为使用 `if request.method.upper() not in methods:`。
- 鉴权白名单中添加的是 `"/api/plug/webhook"`，而实际路由是 `"/api/plug/webhook/<webhook_uuid>"`；请仔细检查匹配逻辑（例如是全等匹配还是前缀匹配）是否确实把所有 webhook 回调都视作在白名单中，并根据 `/api/platform/webhook` 的处理方式调整字符串或条件以保持一致。
## 单独评论
### 评论 1
<location path="astrbot/core/star/context.py" line_range="60" />
<code_context>
"""暴露给插件的接口上下文。"""
registered_web_apis: list = []
+ registered_unified_webhooks: dict[
+ str, tuple[Callable[..., Awaitable[Any]], list[str], str]
+ ] = {}
</code_context>
<issue_to_address>
**issue (complexity):** 考虑用一个带类型的小容器替换现在的位置参数元组，并将 webhook 注册表改为实例属性而不是类级状态，以简化理解和使用。
你可以通过两点来降低这里的心智负担：(1) 用一个带类型的小容器替换位置参数元组；(2) 避免将注册表定义为类级可变状态。
### 1. 用小型 dataclass / TypedDict 替换元组
现在调用方必须记住 `(handler, methods, desc)` 的顺序。用一个小 dataclass 可以让结构自解释：
```python
from dataclasses import dataclass
from typing import Callable, Awaitable, Any
@dataclass
class UnifiedWebhook:
handler: Callable[..., Awaitable[Any]]
methods: list[str]
desc: str = ""
```
然后修改注册表类型：
```python
class Context:
# before:
# registered_unified_webhooks: dict[
# str, tuple[Callable[..., Awaitable[Any]], list[str], str]
# ] = {}
registered_unified_webhooks: dict[str, UnifiedWebhook]
```
再更新 setter：
```python
def register_unified_webhook(
self,
webhook_uuid: str,
view_handler: Callable[..., Awaitable[Any]],
methods: list[str] | None = None,
desc: str = "",
) -> None:
normalized_methods = [method.upper() for method in (methods or ["GET", "POST"])]
self.registered_unified_webhooks[webhook_uuid] = UnifiedWebhook(
handler=view_handler,
methods=normalized_methods,
desc=desc,
)
```
这样可以保持行为完全一致，但在调用点和阅读注册表时结构会更加清晰。
### 2. 将注册表移动为实例属性
当前的类属性让该注册表对所有 `Context` 实例来说实际上是全局共享的。除非你明确需要跨实例共享，否则可以通过按实例初始化来降低生命周期复杂度：
```python
class Context:
def __init__(
self,
event_queue: Queue,
config: AstrBotConfig,
db: BaseDatabase,
# ...
):
self.registered_unified_webhooks: dict[str, UnifiedWebhook] = {}
# existing init logic...
```
这样可以把 webhook 回调限定在各自的 `Context` 中，使测试中的状态更容易推理，并避免不同测试/不同实例之间的意外交叉污染，同时保留当前所有功能。
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The unified webhook registry is stored on
Contextas a class-level mutable dict (registered_unified_webhooks), which can leak state across multipleContextinstances (e.g., in tests or multi-core setups); consider moving this to an instance attribute initialized in__init__for better isolation. - In
srv_plug_webhook_route, you comparerequest.methoddirectly against the storedmethodslist, which is normalized to upper case; to avoid any subtle case issues and keep the logic self-contained, consider doingif request.method.upper() not in methods:. - The auth whitelist adds
"/api/plug/webhook"while the actual route is"/api/plug/webhook/<webhook_uuid>"; double-check that the matching logic (e.g., equality vs prefix) actually treats all webhook callbacks as whitelisted, and adjust the string or condition to be consistent with how/api/platform/webhookis handled.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The unified webhook registry is stored on `Context` as a class-level mutable dict (`registered_unified_webhooks`), which can leak state across multiple `Context` instances (e.g., in tests or multi-core setups); consider moving this to an instance attribute initialized in `__init__` for better isolation.
- In `srv_plug_webhook_route`, you compare `request.method` directly against the stored `methods` list, which is normalized to upper case; to avoid any subtle case issues and keep the logic self-contained, consider doing `if request.method.upper() not in methods:`.
- The auth whitelist adds `"/api/plug/webhook"` while the actual route is `"/api/plug/webhook/<webhook_uuid>"`; double-check that the matching logic (e.g., equality vs prefix) actually treats all webhook callbacks as whitelisted, and adjust the string or condition to be consistent with how `/api/platform/webhook` is handled.
## Individual Comments
### Comment 1
<location path="astrbot/core/star/context.py" line_range="60" />
<code_context>
"""暴露给插件的接口上下文。"""
registered_web_apis: list = []
+ registered_unified_webhooks: dict[
+ str, tuple[Callable[..., Awaitable[Any]], list[str], str]
+ ] = {}
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the positional tuple with a small typed container and making the webhook registry an instance attribute instead of class-level state to simplify understanding and usage.
You can reduce the mental overhead here by (1) replacing the positional tuple with a small typed container and (2) avoiding class-level mutable state for the registry.
### 1. Replace the tuple with a small dataclass / TypedDict
Right now callers have to remember `(handler, methods, desc)` ordering. A small dataclass makes this self-documenting:
```python
from dataclasses import dataclass
from typing import Callable, Awaitable, Any
@dataclass
class UnifiedWebhook:
handler: Callable[..., Awaitable[Any]]
methods: list[str]
desc: str = ""
```
Then change the registry type:
```python
class Context:
# before:
# registered_unified_webhooks: dict[
# str, tuple[Callable[..., Awaitable[Any]], list[str], str]
# ] = {}
registered_unified_webhooks: dict[str, UnifiedWebhook]
```
And update the setter:
```python
def register_unified_webhook(
self,
webhook_uuid: str,
view_handler: Callable[..., Awaitable[Any]],
methods: list[str] | None = None,
desc: str = "",
) -> None:
normalized_methods = [method.upper() for method in (methods or ["GET", "POST"])]
self.registered_unified_webhooks[webhook_uuid] = UnifiedWebhook(
handler=view_handler,
methods=normalized_methods,
desc=desc,
)
```
This keeps behavior identical but makes the structure obvious at call sites and when reading the registry.
### 2. Move registry to an instance attribute
The current class attribute makes the registry effectively global for all `Context` instances. Unless you explicitly rely on cross-instance sharing, you can reduce lifecycle complexity by initializing it per instance:
```python
class Context:
def __init__(
self,
event_queue: Queue,
config: AstrBotConfig,
db: BaseDatabase,
# ...
):
self.registered_unified_webhooks: dict[str, UnifiedWebhook] = {}
# existing init logic...
```
This isolates webhook callbacks to each `Context`, makes the state easier to reason about in tests, and avoids accidental cross-test / cross-instance leakage, while preserving all current functionality.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dosubot
Bot
added
the
feature:plugin
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
![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 PR introduces a unified webhook capability for the plugin system, with a clear design and implementation. However, the implementation contains a significant authentication bypass vulnerability due to overly broad prefix matching in the authentication middleware. Additionally, the webhook registry lacks a cleanup mechanism, potentially leading to stale webhooks, and there's a functional bug that could cause most webhook handlers to crash due to unexpected arguments. A minor issue was also identified in dashboard/server.py, where the new webhook route only accepts GET and POST methods, limiting plugin flexibility. These issues should be addressed before merging.
| registered_unified_webhooks: dict[ | ||
| str, tuple[Callable[..., Awaitable[Any]], list[str], str] | ||
| ] = {} |
There was a problem hiding this comment.
The registered_unified_webhooks dictionary is defined as a class attribute in the Context class. It is never cleared when plugins are reloaded or uninstalled. This means that webhooks registered by a plugin will remain active even after the plugin has been disabled or removed. An attacker who knows the webhook_uuid can still trigger the old plugin's code, which may lead to unexpected behavior or security risks. This registry should be cleared or updated during the plugin unbinding process in PluginManager.
| allowed_endpoints = [ | ||
| "/api/auth/login", | ||
| "/api/file", | ||
| "/api/plug/webhook", |
There was a problem hiding this comment.
The auth_middleware uses request.path.startswith(prefix) to bypass authentication for certain endpoints. By adding "/api/plug/webhook" to the allowed_endpoints list, any plugin-registered route that starts with webhook (e.g., /api/plug/webhook_info, /api/plug/webhook_settings) will also bypass authentication. This is because all plugin routes are mounted under /api/plug/. This leads to an unintended authentication bypass for potentially sensitive plugin APIs.
| if request.method not in methods: | ||
| return jsonify(Response().error("请求方法不被允许").__dict__), 405 | ||
|
|
||
| return await view_handler(*args, **kwargs) |
There was a problem hiding this comment.
In srv_plug_webhook_route, the view_handler is called with *args and **kwargs. Since webhook_uuid is a variable part of the URL rule, it is passed as a keyword argument in kwargs. If the view_handler registered by the plugin does not accept this argument (as shown in the example in the PR description), the request will fail with a TypeError. Consider removing webhook_uuid from kwargs before calling the handler if it's not intended for the plugin's consumption.
| self.app.add_url_rule( | ||
| "/api/plug/webhook/<webhook_uuid>", | ||
| view_func=self.srv_plug_webhook_route, | ||
| methods=["GET", "POST"], |
There was a problem hiding this comment.
当前 add_url_rule 中硬编码了 methods=["GET", "POST"]，这会导致插件无法注册和使用 GET 和 POST 之外的其他 HTTP 方法（如 PUT, DELETE 等）。即使插件在 register_unified_webhook 中指定了其他方法，请求也会在到达 srv_plug_webhook_route 视图函数之前被 Quart 框架以 405 Method Not Allowed 拒绝。
为了让插件能够灵活使用各种 HTTP 方法，建议在这里允许更多常见的方法。具体的方法校验已在 srv_plug_webhook_route 函数中正确处理。
另外，/api/plug/<path:subpath> 路由似乎也存在类似限制，可以考虑一并修改。
| methods=["GET", "POST"], | |
| methods=["GET", "POST", "PUT", "DELETE", "PATCH", "HEAD", "OPTIONS"], |
dosubot
Bot
added
size:XS
1 participant
Modifications / 改动点
本次改动的动机是为插件体系补齐“统一 webhook 注册与回调分发”能力，解决插件无法像平台适配器一样通过统一入口接收外部回调的问题；同时提供最小示例插件，便于快速验证链路是否可用。
在 astrbot/core/star/context.py 中新增插件统一 webhook 注册表与注册方法：
在 astrbot/core/star/base.py 中为 Star 新增便捷方法：
在 astrbot/dashboard/server.py 中新增插件统一 webhook 路由与分发：
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Usage / 使用方法
在插件
initialize方法注册：Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
通过仪表盘（dashboard）API，为插件添加统一的 webhook 注册和分发支持。
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
Add unified webhook registration and dispatch support for plugins via the dashboard API.
New Features:
Enhancements: