feat(dashboard): add plugin webui support with secure scoped assets#5940
Open
lxfight wants to merge 9 commits intoAstrBotDevs:masterfrom
Open
feat(dashboard): add plugin webui support with secure scoped assets#5940lxfight wants to merge 9 commits intoAstrBotDevs:masterfrom
lxfight wants to merge 9 commits intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
the
size:XXL
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! This pull request introduces a robust framework for integrating Web-based User Interfaces (WebUIs) directly into the AstrBot dashboard for individual plugins. It establishes a clear mechanism for plugins to declare their WebUI components, provides secure infrastructure for serving these assets, and facilitates controlled communication between the plugin's WebUI and the main dashboard. This enhancement significantly expands the interactive capabilities of plugins, allowing for richer and more dynamic user experiences within the AstrBot ecosystem. Highlights
Changelog
Activity
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
|
dosubot
bot
added
area:webui
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 5 个问题
给 AI Agents 的提示词
请根据本次代码评审中的评论进行修改：
## 单条评论
### Comment 1
<location path="dashboard/src/views/PluginWebUIPage.vue" line_range="361-367" />
<code_context>
+ </v-alert>
+ </div>
+
+ <iframe
+ v-else
+ ref="iframeRef"
+ :src="iframeSrc"
+ class="plugin-webui-frame"
+ referrerpolicy="no-referrer"
+ sandbox="allow-scripts allow-forms allow-downloads"
+ @load="handleIframeLoad"
+ ></iframe>
</code_context>
<issue_to_address>
**issue (bug_risk):** 被 sandbox 的 iframe 的来源与 bridge 的严格来源检查不兼容，从而阻止了 postMessage 通信。
由于 iframe 使用了 sandbox 但没有添加 `allow-same-origin`，它的 `window.location.origin` 是 `
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/plugin_webui_bridge.js" line_range="3" />
<code_context>
+(function attachAstrBotPluginWebUIBridge() {
+ const CHANNEL = "astrbot-plugin-webui";
+ const TARGET_ORIGIN = window.location.origin;
+ const pendingRequests = new Map();
+ const sseHandlers = new Map();
</code_context>
<issue_to_address>
**issue (bug_risk):** bridge 中的来源比较方式与未启用 `allow-same-origin` 的 sandbox iframe 不兼容。
`TARGET_ORIGIN` 被设置为 `window.location.origin`，但 bridge 随后会通过 `if (event.origin !== TARGET_ORIGIN) return;` 进行过滤。在一个未启用 `allow-same-origin` 的 sandbox iframe 中，iframe 的来源是一个不透明的值（`"null"`），而父页面仍然保留真实的 dashboard 来源，因此来自父页面的有效消息会被丢弃，导致 `ready`／请求／响应协议无法完成。
当你在 `PluginWebUIPage.vue` 中确定 iframe 的 `sandbox` 策略之后，这里的检查也应该与之保持一致。可以二选一：
- 保持严格的来源检查，并为 iframe 添加 `allow-same-origin`，或
- 放宽检查（例如显式允许 `event.origin === "null"` 并使用 `"*"` 作为 `targetOrigin`），前提是你可以接受由此带来的风险。
</issue_to_address>
### Comment 3
<location path="tests/test_dashboard.py" line_range="193" />
<code_context>
+async def test_auth_login(app: Quart, core_lifecycle_td: AstrBotCoreLifecycle):
</code_context>
<issue_to_address>
**suggestion (testing):** 扩展认证测试以断言 JWT Cookie 的属性，包括登出行为
在 `test_auth_login` 中，请扩展断言以覆盖完整的 Cookie 契约：（1）在 `DASHBOARD_JWT_COOKIE_SECURE=False` 的情况下（`testing=True` 下的默认值），断言 `SameSite=Strict`，并且未设置 `Secure`；（2）在另一个测试中，将 `current_app.config["DASHBOARD_JWT_COOKIE_SECURE"] = True`，并断言 JWT Cookie 被标记为 `Secure`。此外，在 `test_logout_clears_cookie_for_plugin_webui` 中，请断言登出响应包含一个显式使 JWT Cookie 过期的 `Set-Cookie`（空值／`Max-Age=0`），而不仅仅是检查登出后的 401 状态码。
</issue_to_address>
### Comment 4
<location path="astrbot/dashboard/routes/plugin.py" line_range="523" />
<code_context>
+ )
+ return response
+
+ async def _serve_plugin_webui_content(
+ self,
+ plugin_name: str,
</code_context>
<issue_to_address>
**issue (complexity):** 建议抽取共有的路径归一化、token／查询参数准备逻辑，以及按后缀划分的资源处理函数，以简化 `_serve_plugin_webui_content` 并减少重复代码。
你在 `PluginRoute` 中新增了大量功能，尤其是 `_serve_plugin_webui_content` 现在在做多个彼此独立的事情。你可以在完全保留当前行为的前提下，通过一些小而明确的抽取来降低复杂度。
### 1. 消除路径归一化逻辑的重复
`_normalize_plugin_webui_asset_path` 和 `_resolve_referenced_asset_path` 都实现了类似的安全检查。将核心逻辑集中到一个地方，可以降低行为分叉的风险，也能让意图更清晰。
```python
def _normalize_plugin_webui_asset_path(asset_path: str) -> str:
return self._normalize_plugin_webui_path(asset_path)
@staticmethod
def _normalize_plugin_webui_path(raw_path: str, base_dir: str | None = None) -> str:
# Normalize slashes and trim
path = raw_path.replace("\\", "/").strip()
if base_dir:
path = posixpath.join(base_dir, path)
normalized = posixpath.normpath(path)
if normalized in {"", "."}:
raise ValueError("Invalid plugin WebUI asset path")
if normalized.startswith("../") or normalized == ".." or normalized.startswith("/"):
raise ValueError("Invalid plugin WebUI asset path")
return normalized
@staticmethod
def _resolve_referenced_asset_path(base_asset_path: str, referenced_url: str) -> str:
parts = urlsplit(referenced_url)
referenced_path = parts.path.strip()
if not referenced_path:
raise ValueError("Plugin WebUI referenced asset path is empty")
base_dir = posixpath.dirname(base_asset_path) if base_asset_path else ""
return PluginRoute._normalize_plugin_webui_path(referenced_path, base_dir=base_dir)
```
这样可以将现有的验证集中到一个地方，上面两个 helper 只需要作为 `_normalize_plugin_webui_path` 的简单封装即可。
### 2. 抽取 token 和查询参数准备逻辑
当前的资源 token 处理与主资源服务函数耦合在一起。将其提取到单独的 helper 中，可以让 `_serve_plugin_webui_content` 更易阅读和测试。
```python
def _prepare_plugin_webui_query_params(
self,
plugin_name: str,
) -> dict[str, str] | None:
asset_token = request.args.get("asset_token", "").strip()
if not asset_token:
asset_token = self._issue_plugin_webui_asset_token(plugin_name) or ""
return {"asset_token": asset_token} if asset_token else None
```
在 `_serve_plugin_webui_content` 中的使用方式：
```python
async def _serve_plugin_webui_content(self, plugin_name: str, asset_path: str):
# ... plugin 查找、file_path 解析 ...
extra_query_params = self._prepare_plugin_webui_query_params(plugin_name)
served_asset_path = asset_path or plugin.webui.entry_file
suffix = file_path.suffix.lower()
# 剩余的按内容类型处理逻辑保持不变
```
这样可以保持全部现有行为，同时把偏“认证”的逻辑与按文件类型处理的逻辑分离开。
### 3. 用简单的分发器扁平化 `_serve_plugin_webui_content`
`if suffix == ...` 的分支链可以通过一个小型映射变得更加声明式，每个 handler 也会更聚焦。你不需要把代码移到其他文件，只是重组现有逻辑。
```python
async def _serve_plugin_webui_content(self, plugin_name: str, asset_path: str):
plugin = self._get_plugin_metadata_by_name(plugin_name)
if not plugin:
return await self._plugin_webui_error_response(404, "Plugin not found")
if not plugin.activated:
return await self._plugin_webui_error_response(403, "Plugin is disabled")
if not plugin.webui:
return await self._plugin_webui_error_response(404, "Plugin WebUI entry not found")
try:
file_path = await self._resolve_plugin_webui_file(plugin, asset_path)
except (FileNotFoundError, ValueError):
return await self._plugin_webui_error_response(404, "Plugin WebUI asset not found")
extra_query_params = self._prepare_plugin_webui_query_params(plugin_name)
served_asset_path = asset_path or plugin.webui.entry_file
suffix = file_path.suffix.lower()
handlers = {
".html": self._serve_plugin_webui_html_asset,
".css": self._serve_plugin_webui_css_asset,
".js": self._serve_plugin_webui_js_asset,
".mjs": self._serve_plugin_webui_js_asset,
}
handler = handlers.get(suffix)
if handler:
return await handler(file_path, plugin_name, served_asset_path, extra_query_params)
return await self._serve_plugin_webui_static_asset(file_path)
```
然后，每个 handler 都会更短、更专注：
```python
async def _serve_plugin_webui_html_asset(
self,
file_path: Path,
plugin_name: str,
asset_path: str,
extra_query_params: dict[str, str] | None,
):
html_text = await self._read_plugin_webui_text(file_path)
rewritten_html = self._rewrite_plugin_webui_html(
html_text,
plugin_name,
asset_path,
extra_query_params=extra_query_params,
)
response = cast(
QuartResponse,
await make_response(rewritten_html, {"Content-Type": "text/html; charset=utf-8"}),
)
return self._apply_plugin_webui_security_headers(response)
async def _serve_plugin_webui_static_asset(self, file_path: Path):
raw_bytes = await self._read_plugin_webui_binary(file_path)
response = cast(
QuartResponse,
await make_response(
raw_bytes,
{"Content-Type": self._guess_plugin_webui_mime_type(file_path)},
),
)
return self._apply_plugin_webui_security_headers(response)
```
这样可以在功能完全不变的前提下，让 `PluginRoute` 更易阅读，并降低 `_serve_plugin_webui_content` 的理解成本，而无需进行更大规模的架构调整。
</issue_to_address>
### Comment 5
<location path="astrbot/dashboard/server.py" line_range="243" />
<code_context>
return r
+ @staticmethod
+ def _extract_dashboard_jwt(allow_asset_token: bool = False) -> str | None:
+ auth_header = request.headers.get("Authorization", "").strip()
+ if auth_header.startswith("Bearer "):
</code_context>
<issue_to_address>
**issue (complexity):** 建议将 WebUI 特定的鉴权路径和 token 处理逻辑拆分到一个独立的 helper 模块中，这样主服务器的鉴权中间件可以专注于通用的 dashboard 认证。
你可以将 WebUI 相关的专用鉴权逻辑隔离出来，使 `auth_middleware` 和 server 类专注于通用职责，而不会改变现有行为。
### 1. 将 WebUI 相关逻辑移动到一个小型 helper 中
从主 server 类中抽取路径／token 类型／scope 相关逻辑：
```python
# plugin_webui_auth.py
from urllib.parse import unquote
from quart import request
PLUGIN_WEBUI_CONTENT_PREFIX = "/api/plugin/webui/content/"
PLUGIN_WEBUI_BRIDGE_PATH = "/api/plugin/webui/bridge-sdk.js"
PLUGIN_WEBUI_TOKEN_TYPE = "plugin_webui_asset"
class PluginWebUIAuth:
@staticmethod
def is_protected_path(path: str) -> bool:
return path.startswith(PLUGIN_WEBUI_CONTENT_PREFIX) or path.startswith(
PLUGIN_WEBUI_BRIDGE_PATH
)
@staticmethod
def is_asset_token(payload: dict) -> bool:
return payload.get("token_type") == PLUGIN_WEBUI_TOKEN_TYPE
@staticmethod
def extract_asset_token() -> str | None:
query_asset_token = request.args.get("asset_token", "").strip()
return query_asset_token or None
@staticmethod
def extract_plugin_name_from_path(path: str) -> str | None:
if not path.startswith(PLUGIN_WEBUI_CONTENT_PREFIX):
return None
remainder = path[len(PLUGIN_WEBUI_CONTENT_PREFIX):]
plugin_part = remainder.split("/", 1)[0] if remainder else ""
return unquote(plugin_part) if plugin_part else None
@classmethod
def is_scope_valid(cls, payload: dict, path: str) -> bool:
if not cls.is_protected_path(path):
return False
if path.startswith(PLUGIN_WEBUI_BRIDGE_PATH):
return True
token_plugin_name = payload.get("plugin_name")
request_plugin_name = cls.extract_plugin_name_from_path(path)
if not isinstance(token_plugin_name, str) or not token_plugin_name or not request_plugin_name:
return False
return token_plugin_name == request_plugin_name
```
### 2. 让 `_extract_dashboard_jwt` 保持通用
让 `_extract_dashboard_jwt` 只处理“普通” dashboard 认证（请求头／Cookie）：
```python
@staticmethod
def _extract_dashboard_jwt() -> str | None:
auth_header = request.headers.get("Authorization", "").strip()
if auth_header.startswith("Bearer "):
token = auth_header.removeprefix("Bearer ").strip()
if token:
return token
cookie_token = request.cookies.get(DASHBOARD_JWT_COOKIE_NAME, "").strip()
return cookie_token or None
```
### 3. 在中间件中使用该 helper
中间件只在更高层面进行调度，而不再内嵌 WebUI 语义：
```python
from .plugin_webui_auth import PluginWebUIAuth
# inside auth_middleware
is_webui = PluginWebUIAuth.is_protected_path(request.path)
token = self._extract_dashboard_jwt()
if not token and is_webui:
token = PluginWebUIAuth.extract_asset_token()
if not token:
r = jsonify(Response().error("未授权").__dict__)
r.status_code = 401
return r
try:
payload = jwt.decode(token, self._jwt_secret, algorithms=["HS256"])
if PluginWebUIAuth.is_asset_token(payload) and not PluginWebUIAuth.is_scope_valid(
payload, request.path
):
r = jsonify(Response().error("Token 无效").__dict__)
r.status_code = 401
return r
username = payload.get("username")
if not isinstance(username, str) or not username.strip():
raise jwt.InvalidTokenError("missing username in token payload")
g.username = username
except jwt.ExpiredSignatureError:
...
except jwt.InvalidTokenError:
...
```
这样可以保留所有当前行为（请求头／Cookie 中的 JWT、查询参数中的资源 token、类型检查、scope 检查），同时降低主 server 类的概念和结构复杂度，并让 WebUI 功能的边界更清晰。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dashboard/src/views/PluginWebUIPage.vue" line_range="361-367" />
<code_context>
+ </v-alert>
+ </div>
+
+ <iframe
+ v-else
+ ref="iframeRef"
+ :src="iframeSrc"
+ class="plugin-webui-frame"
+ referrerpolicy="no-referrer"
+ sandbox="allow-scripts allow-forms allow-downloads"
+ @load="handleIframeLoad"
+ ></iframe>
</code_context>
<issue_to_address>
**issue (bug_risk):** Sandboxed iframe origin breaks the bridge’s strict origin checks, preventing postMessage communication.
Because the iframe is sandboxed without `allow-same-origin`, its `window.location.origin` is `
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/plugin_webui_bridge.js" line_range="3" />
<code_context>
+(function attachAstrBotPluginWebUIBridge() {
+ const CHANNEL = "astrbot-plugin-webui";
+ const TARGET_ORIGIN = window.location.origin;
+ const pendingRequests = new Map();
+ const sseHandlers = new Map();
</code_context>
<issue_to_address>
**issue (bug_risk):** Origin comparison in the bridge is incompatible with a sandboxed iframe without `allow-same-origin`.
`TARGET_ORIGIN` is set to `window.location.origin`, but the bridge then filters with `if (event.origin !== TARGET_ORIGIN) return;`. In a sandboxed iframe without `allow-same-origin`, the iframe’s origin is opaque (`"null"`) while the parent keeps the real dashboard origin, so valid messages from the parent are discarded and the `ready`/request/response protocol can’t complete.
Once you settle the iframe’s `sandbox` policy in `PluginWebUIPage.vue`, this check should be aligned with it. Either:
- Keep strict origin checks and add `allow-same-origin`, or
- Loosen the check (e.g. explicitly allow `event.origin === "null"` and use `"*"` as `targetOrigin`) if that risk is acceptable.
</issue_to_address>
### Comment 3
<location path="tests/test_dashboard.py" line_range="193" />
<code_context>
+async def test_auth_login(app: Quart, core_lifecycle_td: AstrBotCoreLifecycle):
</code_context>
<issue_to_address>
**suggestion (testing):** Extend auth tests to assert JWT cookie attributes, including logout behaviour
In `test_auth_login`, please extend the assertions to cover the full cookie contract: (1) with `DASHBOARD_JWT_COOKIE_SECURE=False` (default under `testing=True`), assert `SameSite=Strict` and that `Secure` is not set; (2) in a separate test where you set `current_app.config["DASHBOARD_JWT_COOKIE_SECURE"] = True`, assert that the JWT cookie is marked `Secure`. Also, in `test_logout_clears_cookie_for_plugin_webui`, assert that the logout response includes a `Set-Cookie` that explicitly expires the JWT cookie (empty value / `Max-Age=0`), rather than only checking for a 401 after logout.
</issue_to_address>
### Comment 4
<location path="astrbot/dashboard/routes/plugin.py" line_range="523" />
<code_context>
+ )
+ return response
+
+ async def _serve_plugin_webui_content(
+ self,
+ plugin_name: str,
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared path normalization, token/query preparation, and per-suffix asset handlers to simplify `_serve_plugin_webui_content` and reduce duplication.
You’ve added a lot of functionality into `PluginRoute`, and `_serve_plugin_webui_content` in particular is now doing several distinct things. You can keep all behavior while reducing complexity with a couple of small, targeted extractions.
### 1. Deduplicate path normalization logic
`_normalize_plugin_webui_asset_path` and `_resolve_referenced_asset_path` both implement similar safety checks. Centralizing the core logic will reduce chances of divergence and make the intent clearer.
```python
def _normalize_plugin_webui_asset_path(asset_path: str) -> str:
return self._normalize_plugin_webui_path(asset_path)
@staticmethod
def _normalize_plugin_webui_path(raw_path: str, base_dir: str | None = None) -> str:
# Normalize slashes and trim
path = raw_path.replace("\\", "/").strip()
if base_dir:
path = posixpath.join(base_dir, path)
normalized = posixpath.normpath(path)
if normalized in {"", "."}:
raise ValueError("Invalid plugin WebUI asset path")
if normalized.startswith("../") or normalized == ".." or normalized.startswith("/"):
raise ValueError("Invalid plugin WebUI asset path")
return normalized
@staticmethod
def _resolve_referenced_asset_path(base_asset_path: str, referenced_url: str) -> str:
parts = urlsplit(referenced_url)
referenced_path = parts.path.strip()
if not referenced_path:
raise ValueError("Plugin WebUI referenced asset path is empty")
base_dir = posixpath.dirname(base_asset_path) if base_asset_path else ""
return PluginRoute._normalize_plugin_webui_path(referenced_path, base_dir=base_dir)
```
This keeps all current validations but in one place, and both helpers are now thin wrappers around `_normalize_plugin_webui_path`.
### 2. Extract token + query param preparation
The asset token handling is intertwined with the main serving function. Pulling it into a helper makes `_serve_plugin_webui_content` easier to scan and test.
```python
def _prepare_plugin_webui_query_params(
self,
plugin_name: str,
) -> dict[str, str] | None:
asset_token = request.args.get("asset_token", "").strip()
if not asset_token:
asset_token = self._issue_plugin_webui_asset_token(plugin_name) or ""
return {"asset_token": asset_token} if asset_token else None
```
Usage in `_serve_plugin_webui_content`:
```python
async def _serve_plugin_webui_content(self, plugin_name: str, asset_path: str):
# ... plugin lookup, file_path resolution ...
extra_query_params = self._prepare_plugin_webui_query_params(plugin_name)
served_asset_path = asset_path or plugin.webui.entry_file
suffix = file_path.suffix.lower()
# rest of content-type-specific logic unchanged
```
This keeps all behavior intact but separates auth-ish concerns from file-type handling.
### 3. Flatten `_serve_plugin_webui_content` with a simple dispatcher
The `if suffix == ...` chain can be made more declarative with a small mapping, keeping each handler focused. This doesn’t require moving code to another file; it only reshapes what’s already there.
```python
async def _serve_plugin_webui_content(self, plugin_name: str, asset_path: str):
plugin = self._get_plugin_metadata_by_name(plugin_name)
if not plugin:
return await self._plugin_webui_error_response(404, "Plugin not found")
if not plugin.activated:
return await self._plugin_webui_error_response(403, "Plugin is disabled")
if not plugin.webui:
return await self._plugin_webui_error_response(404, "Plugin WebUI entry not found")
try:
file_path = await self._resolve_plugin_webui_file(plugin, asset_path)
except (FileNotFoundError, ValueError):
return await self._plugin_webui_error_response(404, "Plugin WebUI asset not found")
extra_query_params = self._prepare_plugin_webui_query_params(plugin_name)
served_asset_path = asset_path or plugin.webui.entry_file
suffix = file_path.suffix.lower()
handlers = {
".html": self._serve_plugin_webui_html_asset,
".css": self._serve_plugin_webui_css_asset,
".js": self._serve_plugin_webui_js_asset,
".mjs": self._serve_plugin_webui_js_asset,
}
handler = handlers.get(suffix)
if handler:
return await handler(file_path, plugin_name, served_asset_path, extra_query_params)
return await self._serve_plugin_webui_static_asset(file_path)
```
Then each handler is short and focused:
```python
async def _serve_plugin_webui_html_asset(
self,
file_path: Path,
plugin_name: str,
asset_path: str,
extra_query_params: dict[str, str] | None,
):
html_text = await self._read_plugin_webui_text(file_path)
rewritten_html = self._rewrite_plugin_webui_html(
html_text,
plugin_name,
asset_path,
extra_query_params=extra_query_params,
)
response = cast(
QuartResponse,
await make_response(rewritten_html, {"Content-Type": "text/html; charset=utf-8"}),
)
return self._apply_plugin_webui_security_headers(response)
async def _serve_plugin_webui_static_asset(self, file_path: Path):
raw_bytes = await self._read_plugin_webui_binary(file_path)
response = cast(
QuartResponse,
await make_response(
raw_bytes,
{"Content-Type": self._guess_plugin_webui_mime_type(file_path)},
),
)
return self._apply_plugin_webui_security_headers(response)
```
This keeps functionality exactly the same but makes `PluginRoute` easier to navigate and reduces the mental load of `_serve_plugin_webui_content` without forcing a larger architectural change.
</issue_to_address>
### Comment 5
<location path="astrbot/dashboard/server.py" line_range="243" />
<code_context>
return r
+ @staticmethod
+ def _extract_dashboard_jwt(allow_asset_token: bool = False) -> str | None:
+ auth_header = request.headers.get("Authorization", "").strip()
+ if auth_header.startswith("Bearer "):
</code_context>
<issue_to_address>
**issue (complexity):** Consider moving the WebUI-specific auth path and token handling into a dedicated helper module so the main server auth middleware stays focused on generic dashboard authentication.
You can isolate the WebUI-specific auth logic to keep `auth_middleware` and the server class focused on generic concerns, without changing behavior.
### 1. Move WebUI-specific logic into a small helper
Extract the path / token-type / scope logic from the main server class:
```python
# plugin_webui_auth.py
from urllib.parse import unquote
from quart import request
PLUGIN_WEBUI_CONTENT_PREFIX = "/api/plugin/webui/content/"
PLUGIN_WEBUI_BRIDGE_PATH = "/api/plugin/webui/bridge-sdk.js"
PLUGIN_WEBUI_TOKEN_TYPE = "plugin_webui_asset"
class PluginWebUIAuth:
@staticmethod
def is_protected_path(path: str) -> bool:
return path.startswith(PLUGIN_WEBUI_CONTENT_PREFIX) or path.startswith(
PLUGIN_WEBUI_BRIDGE_PATH
)
@staticmethod
def is_asset_token(payload: dict) -> bool:
return payload.get("token_type") == PLUGIN_WEBUI_TOKEN_TYPE
@staticmethod
def extract_asset_token() -> str | None:
query_asset_token = request.args.get("asset_token", "").strip()
return query_asset_token or None
@staticmethod
def extract_plugin_name_from_path(path: str) -> str | None:
if not path.startswith(PLUGIN_WEBUI_CONTENT_PREFIX):
return None
remainder = path[len(PLUGIN_WEBUI_CONTENT_PREFIX):]
plugin_part = remainder.split("/", 1)[0] if remainder else ""
return unquote(plugin_part) if plugin_part else None
@classmethod
def is_scope_valid(cls, payload: dict, path: str) -> bool:
if not cls.is_protected_path(path):
return False
if path.startswith(PLUGIN_WEBUI_BRIDGE_PATH):
return True
token_plugin_name = payload.get("plugin_name")
request_plugin_name = cls.extract_plugin_name_from_path(path)
if not isinstance(token_plugin_name, str) or not token_plugin_name or not request_plugin_name:
return False
return token_plugin_name == request_plugin_name
```
### 2. Keep `_extract_dashboard_jwt` generic
Let `_extract_dashboard_jwt` only care about “normal” dashboard auth (headers/cookie):
```python
@staticmethod
def _extract_dashboard_jwt() -> str | None:
auth_header = request.headers.get("Authorization", "").strip()
if auth_header.startswith("Bearer "):
token = auth_header.removeprefix("Bearer ").strip()
if token:
return token
cookie_token = request.cookies.get(DASHBOARD_JWT_COOKIE_NAME, "").strip()
return cookie_token or None
```
### 3. Use the helper from the middleware
The middleware then orchestrates at a higher level, without embedding WebUI semantics:
```python
from .plugin_webui_auth import PluginWebUIAuth
# inside auth_middleware
is_webui = PluginWebUIAuth.is_protected_path(request.path)
token = self._extract_dashboard_jwt()
if not token and is_webui:
token = PluginWebUIAuth.extract_asset_token()
if not token:
r = jsonify(Response().error("未授权").__dict__)
r.status_code = 401
return r
try:
payload = jwt.decode(token, self._jwt_secret, algorithms=["HS256"])
if PluginWebUIAuth.is_asset_token(payload) and not PluginWebUIAuth.is_scope_valid(
payload, request.path
):
r = jsonify(Response().error("Token 无效").__dict__)
r.status_code = 401
return r
username = payload.get("username")
if not isinstance(username, str) or not username.strip():
raise jwt.InvalidTokenError("missing username in token payload")
g.username = username
except jwt.ExpiredSignatureError:
...
except jwt.InvalidTokenError:
...
```
This keeps all current behavior (header/cookie JWTs, asset query tokens, type checks, scope checks) but reduces conceptual and structural complexity in the main server class and makes the WebUI feature boundaries clearer.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
tests/test_dashboard.py
Outdated
| assert data["status"] == "ok" and "token" in data["data"] | ||
| set_cookie_headers = response.headers.getlist("Set-Cookie") | ||
| assert any(DASHBOARD_JWT_COOKIE_NAME in value for value in set_cookie_headers) | ||
| assert any("HttpOnly" in value for value in set_cookie_headers) |
Contributor
There was a problem hiding this comment.
suggestion (testing): 扩展认证测试以断言 JWT Cookie 的属性，包括登出行为
在 test_auth_login 中，请扩展断言以覆盖完整的 Cookie 契约：（1）在 DASHBOARD_JWT_COOKIE_SECURE=False 的情况下（testing=True 下的默认值），断言 SameSite=Strict，并且未设置 Secure；（2）在另一个测试中，将 current_app.config["DASHBOARD_JWT_COOKIE_SECURE"] = True，并断言 JWT Cookie 被标记为 Secure。此外，在 test_logout_clears_cookie_for_plugin_webui 中，请断言登出响应包含一个显式使 JWT Cookie 过期的 Set-Cookie（空值／Max-Age=0），而不仅仅是检查登出后的 401 状态码。
Original comment in English
suggestion (testing): Extend auth tests to assert JWT cookie attributes, including logout behaviour
In test_auth_login, please extend the assertions to cover the full cookie contract: (1) with DASHBOARD_JWT_COOKIE_SECURE=False (default under testing=True), assert SameSite=Strict and that Secure is not set; (2) in a separate test where you set current_app.config["DASHBOARD_JWT_COOKIE_SECURE"] = True, assert that the JWT cookie is marked Secure. Also, in test_logout_clears_cookie_for_plugin_webui, assert that the logout response includes a Set-Cookie that explicitly expires the JWT cookie (empty value / Max-Age=0), rather than only checking for a 401 after logout.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
本次 PR 为插件引入了 WebUI 支持，这是一个设计精良的重要功能。实现过程对安全性给予了高度关注，包括为资源使用带范围的短期 JWT、安全 Cookie、路径遍历保护以及带严格 CSP 头部的沙箱化 iframe。用于提供和重写资源的后端逻辑很全面，前端桥接 SDK 也为插件开发者提供了清晰的接口。此外，PR 还包含了覆盖认证、路由和安全方面的详尽测试，这一点值得称赞。我主要有两个关于代码可维护性和 JavaScript 资源重写逻辑鲁棒性的改进建议。总的来说，这是一次出色的贡献。
Comment on lines
+399
to
+451
| def _rewrite_plugin_webui_js( | ||
| self, | ||
| js_text: str, | ||
| plugin_name: str, | ||
| js_asset_path: str, | ||
| extra_query_params: dict[str, str] | None = None, | ||
| ) -> str: | ||
| def rewrite_specifier(raw_url: str) -> str: | ||
| if not self._is_rewritable_asset_url(raw_url): | ||
| return raw_url | ||
| parts = urlsplit(raw_url) | ||
| asset_path = self._resolve_referenced_asset_path(js_asset_path, raw_url) | ||
| return self._build_plugin_webui_asset_url( | ||
| plugin_name, | ||
| asset_path, | ||
| original_query=parts.query, | ||
| original_fragment=parts.fragment, | ||
| extra_query_params=extra_query_params, | ||
| ) | ||
|
|
||
| def replace_dynamic(match: re.Match[str]) -> str: | ||
| raw_url = match.group("url") | ||
| try: | ||
| rewritten = rewrite_specifier(raw_url) | ||
| except ValueError: | ||
| return match.group(0) | ||
| return ( | ||
| f"{match.group('prefix')}{match.group('quote')}{rewritten}" | ||
| f"{match.group('quote')}{match.group('suffix')}" | ||
| ) | ||
|
|
||
| def replace_from(match: re.Match[str]) -> str: | ||
| raw_url = match.group("url") | ||
| try: | ||
| rewritten = rewrite_specifier(raw_url) | ||
| except ValueError: | ||
| return match.group(0) | ||
| return f"{match.group('prefix')}{match.group('quote')}{rewritten}{match.group('quote')}" | ||
|
|
||
| rewritten_js = _JS_DYNAMIC_IMPORT_RE.sub(replace_dynamic, js_text) | ||
| rewritten_js = _JS_MODULE_FROM_RE.sub(replace_from, rewritten_js) | ||
|
|
||
| def replace_side_effect(match: re.Match[str]) -> str: | ||
| raw_url = match.group("url") | ||
| if raw_url.startswith(("{", "*")): | ||
| return match.group(0) | ||
| try: | ||
| rewritten = rewrite_specifier(raw_url) | ||
| except ValueError: | ||
| return match.group(0) | ||
| return f"{match.group('prefix')}{match.group('quote')}{rewritten}{match.group('quote')}" | ||
|
|
||
| return _JS_SIDE_EFFECT_IMPORT_RE.sub(replace_side_effect, rewritten_js) |
Contributor
There was a problem hiding this comment.
_rewrite_plugin_webui_js 函数中用于重写 JavaScript 导入语句的逻辑比较脆弱，可能存在 bug。问题在于 _JS_SIDE_EFFECT_IMPORT_RE 的模式过于宽泛，它可能会错误地匹配 import ... from ... 语句，例如 import React from "react"。在这种情况下，url 捕获组会得到 React from "react"，这并不是一个有效的模块路径，会导致 rewrite_specifier 失败或产生错误的结果，从而破坏插件的 WebUI 脚本。replace_side_effect 中的 if raw_url.startswith(("{', '*')): 检查不足以防止这类问题。建议重构此逻辑，使正则表达式之间互斥，或在 replace_side_effect 中添加更严格的检查来跳过非纯副作用导入的语句。
Comment on lines
+242
to
+271
| def _normalize_plugin_webui(raw_webui: object) -> PluginWebUIPage | None: | ||
| if not isinstance(raw_webui, dict): | ||
| return None | ||
|
|
||
| raw_display_name = raw_webui.get("display_name") or raw_webui.get("title") | ||
| display_name = ( | ||
| raw_display_name.strip() | ||
| if isinstance(raw_display_name, str) and raw_display_name.strip() | ||
| else "WebUI" | ||
| ) | ||
|
|
||
| raw_root_dir = raw_webui.get("root_dir") or raw_webui.get("root") | ||
| root_dir = ( | ||
| raw_root_dir.strip() | ||
| if isinstance(raw_root_dir, str) and raw_root_dir.strip() | ||
| else "webui" | ||
| ) | ||
|
|
||
| raw_entry_file = raw_webui.get("entry_file") or raw_webui.get("entry") | ||
| entry_file = ( | ||
| raw_entry_file.strip() | ||
| if isinstance(raw_entry_file, str) and raw_entry_file.strip() | ||
| else "index.html" | ||
| ) | ||
|
|
||
| return PluginWebUIPage( | ||
| display_name=display_name, | ||
| root_dir=root_dir, | ||
| entry_file=entry_file, | ||
| ) |
Contributor
There was a problem hiding this comment.
在 _normalize_plugin_webui 方法中，解析 display_name、root_dir 和 entry_file 的逻辑几乎完全相同，存在代码重复。为了提高代码的可读性和可维护性，建议将这部分重复逻辑提取到一个内部辅助函数中。
def _normalize_plugin_webui(raw_webui: object) -> PluginWebUIPage | None:
if not isinstance(raw_webui, dict):
return None
def _get_str_value(keys: list[str], default: str) -> str:
for key in keys:
value = raw_webui.get(key)
if isinstance(value, str) and (stripped := value.strip()):
return stripped
return default
display_name = _get_str_value(["display_name", "title"], "WebUI")
root_dir = _get_str_value(["root_dir", "root"], "webui")
entry_file = _get_str_value(["entry_file", "entry"], "index.html")
return PluginWebUIPage(
display_name=display_name,
root_dir=root_dir,
entry_file=entry_file,
)
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.
本 PR 为 AstrBot 插件引入完整的 WebUI 支持链路，并修复了评审中指出的关键问题。
主要解决：
Modifications / 改动点
新增插件 WebUI 元数据能力：
PluginWebUIPage，并在插件加载流程中标准化解析 WebUI 字段。后端新增插件 WebUI 内容服务与资产路由，支持 HTML/CSS/JS 资源重写、路径安全校验与安全响应头。
新增短时效
asset_token（JWT）用于插件 WebUI 资源访问，并校验 token 类型与插件作用域。新增 bridge SDK 与 Dashboard 的 PluginWebUI 页面，支持插件 WebUI 与父页面间 API、文件上传下载、SSE 交互。
修复 sandbox iframe 下 bridge 通信失效问题：在保持隔离策略下兼容
nullorigin 并绑定消息来源。优化认证 Cookie 策略：
SameSite=Strict、HttpOnly，并通过DASHBOARD_JWT_COOKIE_SECURE支持环境化控制Secure。根据评审建议完成可维护性重构：
astrbot/dashboard/routes/plugin.py：抽取路径归一化、query/token 准备、按后缀分发处理函数。astrbot/dashboard/server.py：将 WebUI token/path/scope 逻辑拆到独立模块。astrbot/dashboard/plugin_webui_auth.py：新增 WebUI 鉴权 helper。修复 JS 资源重写鲁棒性：避免误处理 bare import，仅重写相对模块 specifier。
补充/加强测试：登录/登出 cookie 契约、WebUI 资源鉴权、作用域校验、bridge/重写回归断言。
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
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.