Open
Conversation
dosubot
Bot
added
the
size:L
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 2 个问题，并给出了一些整体性反馈：
- 在
get_plugin_extension_page中，plugin_obj.name被直接插入到注入的<script>块中；建议对其进行转义或通过json.dumps（或等效方法）序列化，以避免在插件名称包含引号或特殊字符时破坏脚本或造成注入风险。 - 当前的 HTML 注入依赖
html_content.replace("</head>", injected_script + "</head>")，如果源文档缺少闭合的</head>，或者使用了不同的大小写/空白格式，就会失效；建议使用更健壮的注入策略，或者在找不到</head>时退而求其次，将脚本追加到</body>之前。 - 在前端，请求
axios.get(/api/plugin/extension_page?name=${pluginName})应该对pluginName做 URL 编码（例如使用encodeURIComponent），以正确处理带有空格或特殊字符的插件名称。
给 AI Agent 的提示词
请根据下面的代码审查意见进行修改：
## 总体意见
- 在 `get_plugin_extension_page` 中，`plugin_obj.name` 被直接插入到注入的 `<script>` 块中；建议对其进行转义或通过 `json.dumps`（或等效方法）序列化，以避免在插件名称包含引号或特殊字符时破坏脚本或造成注入风险。
- 当前的 HTML 注入依赖 `html_content.replace("</head>", injected_script + "</head>")`，如果源文档缺少闭合的 `</head>`，或者使用了不同的大小写/空白格式，就会失效；建议使用更健壮的注入策略，或者在找不到 `</head>` 时退而求其次，将脚本追加到 `</body>` 之前。
- 在前端，请求 `axios.get(`/api/plugin/extension_page?name=${pluginName}`)` 应该对 `pluginName` 做 URL 编码（例如使用 `encodeURIComponent`），以正确处理带有空格或特殊字符的插件名称。
## 逐条评论
### 评论 1
<location> `dashboard/src/views/ExtensionPage.vue:701` </location>
<code_context>
+// 获取扩展页面内容
+async function fetchExtensionPage(pluginName) {
+ try {
+ const res = await axios.get(`/api/plugin/extension_page?name=${pluginName}`);
+ if (res.data.status === "ok") {
+ extensionPageDialog.html = res.data.data.html;
</code_context>
<issue_to_address>
**issue (bug_risk):** 在构建查询字符串时对 `pluginName` 进行 URL 编码。
未编码的值如果包含空格、`?`、`&` 或非 ASCII 字符，可能会导致该请求被破坏或路由错误。请在 URL 中使用 `encodeURIComponent(pluginName)`（例如：``axios.get(`/api/plugin/extension_page?name=${encodeURIComponent(pluginName)}`)``），以确保适用于所有插件名称。
</issue_to_address>
### 评论 2
<location> `dashboard/src/views/ExtensionPage.vue:2712` </location>
<code_context>
+ <iframe
+ :srcdoc="extensionPageDialog.html"
+ style="width: 100%; height: 100%; border: none;"
+ sandbox="allow-scripts allow-forms allow-same-origin"
+ ></iframe>
</code_context>
<issue_to_address>
**🚨 issue (security):** 重新审视 iframe 的 sandbox 设置；`allow-same-origin` 会削弱对不受信任插件内容的隔离。
在同时启用 `allow-scripts` 和 `allow-same-origin` 的情况下，插件的 JS 实际上以和控制面板相同的源运行，可以读写 `localStorage`、cookies 以及 `ASTRBOT_CONFIG` 以外的其他同源数据。如果插件的 HTML 并非完全可信，这会显著扩大其可访问的范围。建议移除 `allow-same-origin`，让其在独立的源中运行，同时仍可通过你注入的脚本读取 `window.ASTRBOT_CONFIG`，或者以其他方式严格限制插件代码的可访问范围。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In
get_plugin_extension_page,plugin_obj.nameis interpolated directly into the injected<script>block; consider escaping or serializing viajson.dumps(or equivalent) to avoid breaking the script or creating an injection vector if plugin names contain quotes or special characters. - The HTML injection relies on
html_content.replace("</head>", injected_script + "</head>"), which will fail if the source lacks a closing</head>or uses different casing/whitespace; consider a more robust injection strategy or falling back to appending the script before</body>when</head>is not found. - On the frontend, the request
axios.get(/api/plugin/extension_page?name=${pluginName})should URL-encodepluginName(e.g.,encodeURIComponent) to handle plugin names with spaces or special characters correctly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `get_plugin_extension_page`, `plugin_obj.name` is interpolated directly into the injected `<script>` block; consider escaping or serializing via `json.dumps` (or equivalent) to avoid breaking the script or creating an injection vector if plugin names contain quotes or special characters.
- The HTML injection relies on `html_content.replace("</head>", injected_script + "</head>")`, which will fail if the source lacks a closing `</head>` or uses different casing/whitespace; consider a more robust injection strategy or falling back to appending the script before `</body>` when `</head>` is not found.
- On the frontend, the request `axios.get(`/api/plugin/extension_page?name=${pluginName}`)` should URL-encode `pluginName` (e.g., `encodeURIComponent`) to handle plugin names with spaces or special characters correctly.
## Individual Comments
### Comment 1
<location> `dashboard/src/views/ExtensionPage.vue:701` </location>
<code_context>
+// 获取扩展页面内容
+async function fetchExtensionPage(pluginName) {
+ try {
+ const res = await axios.get(`/api/plugin/extension_page?name=${pluginName}`);
+ if (res.data.status === "ok") {
+ extensionPageDialog.html = res.data.data.html;
</code_context>
<issue_to_address>
**issue (bug_risk):** URL-encode `pluginName` when building the query string.
Unencoded values containing spaces, `?`, `&`, or non-ASCII characters can break or misroute this request. Use `encodeURIComponent(pluginName)` in the URL (e.g., ``axios.get(`/api/plugin/extension_page?name=${encodeURIComponent(pluginName)}`)``) to ensure it works for all plugin names.
</issue_to_address>
### Comment 2
<location> `dashboard/src/views/ExtensionPage.vue:2712` </location>
<code_context>
+ <iframe
+ :srcdoc="extensionPageDialog.html"
+ style="width: 100%; height: 100%; border: none;"
+ sandbox="allow-scripts allow-forms allow-same-origin"
+ ></iframe>
</code_context>
<issue_to_address>
**🚨 issue (security):** Revisit the iframe sandbox settings; `allow-same-origin` weakens isolation for untrusted plugin content.
With both `allow-scripts` and `allow-same-origin`, plugin JS effectively runs as the dashboard origin and can read/write `localStorage`, cookies, and other origin-scoped data beyond `ASTRBOT_CONFIG`. If plugin HTML isn’t fully trusted, this substantially expands its access. Consider removing `allow-same-origin` so it runs in a unique origin while still reading `window.ASTRBOT_CONFIG` via your injected script, or otherwise tightly limit what plugin code can reach.
</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
Member
|
要如何使用？方便给一个文档吗 |
Author
|
Closed
Author
可以了 |
2 tasks
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并给出了一些整体层面的反馈：
- 在
get_plugin_extension_page中，你只在存在</head>标签时注入window.ASTRBOT_CONFIG；可以考虑在找不到<head>时，退而求其次，将脚本插入到</body>之前或整个文档的最前面，这样即使页面没有<head>标签也能拿到配置。 - 扩展页面的 iframe 使用了
sandbox="allow-scripts allow-forms allow-same-origin"，这会给嵌入页面完整的同源访问权限；你可能需要移除allow-same-origin（只依赖注入的 token）或以其他方式收紧 sandbox，以降低恶意插件页面带来的影响。 ExtensionPage.vue中扩展页面对话框工具栏按钮文案 "关闭" 是硬编码的；建议使用现有的 i18n 系统来处理这段文案，以保持 UI 完整本地化。
给 AI Agent 的提示
请解决本次代码评审中的这些评论：
## 总体评论
- 在 `get_plugin_extension_page` 中，你只在存在 `</head>` 标签时注入 `window.ASTRBOT_CONFIG`；可以考虑在找不到 `<head>` 时，退而求其次，将脚本插入到 `</body>` 之前或整个文档的最前面，这样即使页面没有 `<head>` 标签也能拿到配置。
- 扩展页面的 iframe 使用了 `sandbox="allow-scripts allow-forms allow-same-origin"`，这会给嵌入页面完整的同源访问权限；你可能需要移除 `allow-same-origin`（只依赖注入的 token）或以其他方式收紧 sandbox，以降低恶意插件页面带来的影响。
- `ExtensionPage.vue` 中扩展页面对话框工具栏按钮文案 "关闭" 是硬编码的；建议使用现有的 i18n 系统来处理这段文案，以保持 UI 完整本地化。
## 单独评论
### 评论 1
<location path="astrbot/dashboard/routes/plugin.py" line_range="751" />
<code_context>
+ return Response().error(f"插件 {plugin_name} 未配置扩展页面").__dict__
+
+ try:
+ with open(extension_page_path, encoding="utf-8") as f:
+ html_content = f.read()
+
</code_context>
<issue_to_address>
**suggestion (performance):** 这个异步处理函数中的文件 I/O 仍然是同步执行的，可以放到已有的 `to_thread` 调用中一起处理。
现在存在性检查已经使用了 `asyncio.to_thread`，但 `open(...).read()` 仍然在事件循环线程上运行，对于大文件或较慢的存储，会阻塞其他请求。
你可以通过把读取操作也放进 `to_thread` 来保持非阻塞，例如：
```python
html_content = await asyncio.to_thread(
lambda: Path(extension_page_path).read_text(encoding="utf-8")
)
```
这样可以让处理函数对异步更友好，避免在事件循环中执行文件 I/O。
建议的实现方式：
```python
import os
from pathlib import Path
```
```python
if not await asyncio.to_thread(os.path.exists, extension_page_path):
return Response().error(f"插件 {plugin_name} 未配置扩展页面").__dict__
try:
html_content = await asyncio.to_thread(
lambda: Path(extension_page_path).read_text(encoding="utf-8")
)
```
1. 确认稍后在这个处理函数中确实会使用 `html_content`（例如返回或传入模板/响应）；目前代码片段中只看到读取步骤。
2. 如果该模块已经在别处引入了 `Path`，或者使用了不同的导入风格（例如在共享工具模块中统一 `from pathlib import Path`），请调整导入方式以符合现有约定。
</issue_to_address>
### 评论 2
<location path="astrbot/dashboard/routes/plugin.py" line_range="769" />
<code_context>
+ # 使用正则表达式不区分大小写查找 </head> 标签
+ pattern = re.compile(r"</head\s*>", re.IGNORECASE)
+ html_content = pattern.sub(injected_script + r"</head>", html_content)
</code_context>
<issue_to_address>
**suggestion:** 如果扩展页面缺少 </head> 标签，当前注入逻辑会静默失败；建议增加一个兜底的注入位置。
当前的正则方式仅在存在 `</head>` 时工作。如果缺少该标签，`pattern.sub` 不会产生任何替换，页面也就拿不到 `window.ASTRBOT_CONFIG`，这对插件作者来说可能比较困惑。
可以考虑：如果替换后 `html_content` 没有发生变化，则退而选择在存在 `</body>` 时注入到其前面，或者直接追加到文档末尾。这样可以让注入逻辑适配更多 HTML 变体。
</issue_to_address>
### 评论 3
<location path="astrbot/dashboard/routes/plugin.py" line_range="60" />
<code_context>
"/plugin/reload": ("POST", self.reload_plugins),
"/plugin/readme": ("GET", self.get_plugin_readme),
"/plugin/changelog": ("GET", self.get_plugin_changelog),
+ "/plugin/extension_page": ("GET", self.get_plugin_extension_page),
"/plugin/source/get": ("GET", self.get_custom_source),
"/plugin/source/save": ("POST", self.save_custom_source),
</code_context>
<issue_to_address>
**issue (complexity):** 建议把插件查找、目录解析、异步文件系统检查以及 HTML 注入等共用逻辑抽取到可复用的辅助函数中，以简化新的扩展页面功能及相关处理函数。
你可以通过把这些重复模式集中到一些小的辅助函数中并在各个处理函数间复用，来降低扩展页面相关逻辑的复杂度。
### 1. 统一插件目录解析和插件查找
`get_plugins`、`get_plugin_readme`、`get_plugin_changelog` 和 `get_plugin_extension_page` 都在重复实现“按名称查找插件”和“解析插件目录（保留插件与非保留插件）”。
可以新增辅助函数：
```python
def _find_plugin_by_name(self, name: str):
for plugin in self.plugin_manager.context.get_all_stars():
if plugin.name == name:
return plugin
return None
def _get_plugin_dir(self, plugin_obj) -> str:
base = (
self.plugin_manager.reserved_plugin_path
if plugin_obj.reserved
else self.plugin_manager.plugin_store_path
)
return os.path.join(base, plugin_obj.root_dir_name or "")
```
然后在使用处做简化，例如在 `get_plugin_readme` 中：
```python
async def get_plugin_readme(self):
plugin_name = request.args.get("name")
if not plugin_name:
return Response().error("插件名称不能为空").__dict__
plugin_obj = self._find_plugin_by_name(plugin_name)
if not plugin_obj:
return Response().error(f"插件 {plugin_name} 不存在").__dict__
if not plugin_obj.root_dir_name:
return Response().error(f"插件 {plugin_name} 目录不存在").__dict__
plugin_dir = self._get_plugin_dir(plugin_obj)
if not await asyncio.to_thread(os.path.isdir, plugin_dir):
return Response().error(f"无法找到插件 {plugin_name} 的目录").__dict__
readme_path = os.path.join(plugin_dir, "README.md")
if not await asyncio.to_thread(os.path.isfile, readme_path):
return Response().error(f"插件 {plugin_name} 没有README文件").__dict__
with open(readme_path, encoding="utf-8") as f:
readme_content = f.read()
return Response().ok({"content": readme_content}, "成功获取README内容").__dict__
```
`get_plugin_changelog` 和 `get_plugin_extension_page` 也可以类似简化：
```python
plugin_obj = self._find_plugin_by_name(plugin_name)
# ...
plugin_dir = self._get_plugin_dir(plugin_obj)
```
在 `get_plugins` 中则可以：
```python
plugin_dir = self._get_plugin_dir(plugin)
_t["extension_page"] = await self._aexists(os.path.join(plugin_dir, "web", "index.html"))
```
这样可以保持保留插件/非保留插件处理的一致性，并去除重复的分支逻辑。
### 2. 封装异步文件系统检查
内联的 `await asyncio.to_thread(os.path.isdir, ...)` / `os.path.isfile` / `os.path.exists` 在代码中出现多次，让处理函数显得比较嘈杂。
可以新增几个简单的异步辅助函数：
```python
async def _aisdir(self, path: str) -> bool:
return await asyncio.to_thread(os.path.isdir, path)
async def _aisfile(self, path: str) -> bool:
return await asyncio.to_thread(os.path.isfile, path)
async def _aexists(self, path: str) -> bool:
return await asyncio.to_thread(os.path.exists, path)
```
然后在调用处简化为：
```python
if not await self._aisdir(plugin_dir):
...
if not await self._aisfile(readme_path):
...
if await self._aisfile(changelog_path):
...
if not await self._aexists(extension_page_path):
...
```
在保持异步 offloading 的前提下，这样可以让控制流更易于阅读。
### 3. 抽取 HTML 注入为辅助函数
`get_plugin_extension_page` 中的正则编译和注入字符串可以抽取出来，以更清晰地表达意图：
```python
def _inject_astrbot_config(self, html_content: str, plugin_obj) -> str:
safe_plugin_name = json.dumps(plugin_obj.name)
injected_script = f"""
<script>
window.ASTRBOT_CONFIG = {{
apiUrl: "/api/plug",
pluginName: {safe_plugin_name},
apiToken: localStorage.getItem("token") || ""
}};
</script>
"""
pattern = re.compile(r"</head\s*>", re.IGNORECASE)
return pattern.sub(injected_script + r"</head>", html_content)
```
然后 `get_plugin_extension_page` 可以简化为：
```python
with open(extension_page_path, encoding="utf-8") as f:
html_content = f.read()
html_content = self._inject_astrbot_config(html_content, plugin_obj)
return (
Response()
.ok(
{
"html": html_content,
"title": f"{plugin_obj.display_name or plugin_obj.name} 扩展页面",
},
"成功获取扩展页面",
)
.__dict__
)
```
这些小的辅助函数在保留现有功能（包括保留插件支持和异步文件系统检查）的同时，使每个处理函数的核心逻辑更短、更易维护。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
get_plugin_extension_page, you only injectwindow.ASTRBOT_CONFIGwhen a</head>tag is present; consider falling back to appending the script before</body>or at the top of the document so pages without a<head>still receive the config. - The extension page iframe uses
sandbox="allow-scripts allow-forms allow-same-origin", which gives the embedded page full same-origin access; you may want to dropallow-same-origin(and rely solely on the injected token) or otherwise tighten the sandbox to reduce the impact of a malicious plugin page. - The extension page dialog toolbar button label "关闭" in
ExtensionPage.vueis hardcoded; consider using the existing i18n system for this text to keep the UI fully localized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `get_plugin_extension_page`, you only inject `window.ASTRBOT_CONFIG` when a `</head>` tag is present; consider falling back to appending the script before `</body>` or at the top of the document so pages without a `<head>` still receive the config.
- The extension page iframe uses `sandbox="allow-scripts allow-forms allow-same-origin"`, which gives the embedded page full same-origin access; you may want to drop `allow-same-origin` (and rely solely on the injected token) or otherwise tighten the sandbox to reduce the impact of a malicious plugin page.
- The extension page dialog toolbar button label "关闭" in `ExtensionPage.vue` is hardcoded; consider using the existing i18n system for this text to keep the UI fully localized.
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/plugin.py" line_range="751" />
<code_context>
+ return Response().error(f"插件 {plugin_name} 未配置扩展页面").__dict__
+
+ try:
+ with open(extension_page_path, encoding="utf-8") as f:
+ html_content = f.read()
+
</code_context>
<issue_to_address>
**suggestion (performance):** Direct file I/O in this async handler is still done synchronously and could be moved into the existing to_thread usage.
The existence check now uses `asyncio.to_thread`, but `open(...).read()` still runs on the event loop thread and can stall other requests for large files or slow storage.
You can keep this non-blocking by moving the read into `to_thread` as well, e.g.:
```python
html_content = await asyncio.to_thread(
lambda: Path(extension_page_path).read_text(encoding="utf-8")
)
```
This keeps the handler fully async-friendly and avoids blocking the event loop with file I/O.
Suggested implementation:
```python
import os
from pathlib import Path
```
```python
if not await asyncio.to_thread(os.path.exists, extension_page_path):
return Response().error(f"插件 {plugin_name} 未配置扩展页面").__dict__
try:
html_content = await asyncio.to_thread(
lambda: Path(extension_page_path).read_text(encoding="utf-8")
)
```
1. Ensure that `html_content` is actually used later in this handler (e.g. returned or passed into a template/response); currently only the read step is visible in the snippet.
2. If the module already imports `Path` elsewhere or has a different import style (e.g. `from pathlib import Path` in a shared utilities module), adjust the import block to match existing conventions.
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/routes/plugin.py" line_range="769" />
<code_context>
+ # 使用正则表达式不区分大小写查找 </head> 标签
+ pattern = re.compile(r"</head\s*>", re.IGNORECASE)
+ html_content = pattern.sub(injected_script + r"</head>", html_content)
</code_context>
<issue_to_address>
**suggestion:** If the extension page lacks a </head> tag, the injection silently does nothing; consider a fallback injection point.
The regex approach works only when a `</head>` exists. If it’s missing, `pattern.sub` is a no-op and the page never gets `window.ASTRBOT_CONFIG`, which can be confusing for plugin authors.
Consider: if the substitution doesn’t change `html_content`, fall back to injecting before `</body>` when present, or appending at the end of the document. This would make the injection behave correctly for more HTML variants.
</issue_to_address>
### Comment 3
<location path="astrbot/dashboard/routes/plugin.py" line_range="60" />
<code_context>
"/plugin/reload": ("POST", self.reload_plugins),
"/plugin/readme": ("GET", self.get_plugin_readme),
"/plugin/changelog": ("GET", self.get_plugin_changelog),
+ "/plugin/extension_page": ("GET", self.get_plugin_extension_page),
"/plugin/source/get": ("GET", self.get_custom_source),
"/plugin/source/save": ("POST", self.save_custom_source),
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared plugin lookup, directory resolution, async filesystem checks, and HTML injection into reusable helpers to simplify the new extension-page feature and related handlers.
You can reduce the new extension-page complexity by centralizing the repeated patterns into small helpers and reusing them across handlers.
### 1. Centralize plugin-dir resolution and plugin lookup
`get_plugins`, `get_plugin_readme`, `get_plugin_changelog`, and `get_plugin_extension_page` all reimplement “find plugin by name” and “resolve plugin directory (reserved vs non-reserved)”.
Add helpers:
```python
def _find_plugin_by_name(self, name: str):
for plugin in self.plugin_manager.context.get_all_stars():
if plugin.name == name:
return plugin
return None
def _get_plugin_dir(self, plugin_obj) -> str:
base = (
self.plugin_manager.reserved_plugin_path
if plugin_obj.reserved
else self.plugin_manager.plugin_store_path
)
return os.path.join(base, plugin_obj.root_dir_name or "")
```
Then simplify usages, e.g. in `get_plugin_readme`:
```python
async def get_plugin_readme(self):
plugin_name = request.args.get("name")
if not plugin_name:
return Response().error("插件名称不能为空").__dict__
plugin_obj = self._find_plugin_by_name(plugin_name)
if not plugin_obj:
return Response().error(f"插件 {plugin_name} 不存在").__dict__
if not plugin_obj.root_dir_name:
return Response().error(f"插件 {plugin_name} 目录不存在").__dict__
plugin_dir = self._get_plugin_dir(plugin_obj)
if not await asyncio.to_thread(os.path.isdir, plugin_dir):
return Response().error(f"无法找到插件 {plugin_name} 的目录").__dict__
readme_path = os.path.join(plugin_dir, "README.md")
if not await asyncio.to_thread(os.path.isfile, readme_path):
return Response().error(f"插件 {plugin_name} 没有README文件").__dict__
with open(readme_path, encoding="utf-8") as f:
readme_content = f.read()
return Response().ok({"content": readme_content}, "成功获取README内容").__dict__
```
Similarly for `get_plugin_changelog` and `get_plugin_extension_page`:
```python
plugin_obj = self._find_plugin_by_name(plugin_name)
# ...
plugin_dir = self._get_plugin_dir(plugin_obj)
```
And in `get_plugins`:
```python
plugin_dir = self._get_plugin_dir(plugin)
_t["extension_page"] = await self._aexists(os.path.join(plugin_dir, "web", "index.html"))
```
This keeps reserved/non-reserved handling consistent and removes duplicated branches.
### 2. Wrap async filesystem checks
Inline `await asyncio.to_thread(os.path.isdir, ...)` / `os.path.isfile` / `os.path.exists` appears multiple times and makes the handlers noisier.
Add small async helpers:
```python
async def _aisdir(self, path: str) -> bool:
return await asyncio.to_thread(os.path.isdir, path)
async def _aisfile(self, path: str) -> bool:
return await asyncio.to_thread(os.path.isfile, path)
async def _aexists(self, path: str) -> bool:
return await asyncio.to_thread(os.path.exists, path)
```
Then simplify:
```python
if not await self._aisdir(plugin_dir):
...
if not await self._aisfile(readme_path):
...
if await self._aisfile(changelog_path):
...
if not await self._aexists(extension_page_path):
...
```
This makes the control flow easier to scan while preserving the async offloading.
### 3. Extract HTML injection into a helper
The regex compilation and injection string in `get_plugin_extension_page` can be extracted to clarify intent:
```python
def _inject_astrbot_config(self, html_content: str, plugin_obj) -> str:
safe_plugin_name = json.dumps(plugin_obj.name)
injected_script = f"""
<script>
window.ASTRBOT_CONFIG = {{
apiUrl: "/api/plug",
pluginName: {safe_plugin_name},
apiToken: localStorage.getItem("token") || ""
}};
</script>
"""
pattern = re.compile(r"</head\s*>", re.IGNORECASE)
return pattern.sub(injected_script + r"</head>", html_content)
```
Then `get_plugin_extension_page` becomes:
```python
with open(extension_page_path, encoding="utf-8") as f:
html_content = f.read()
html_content = self._inject_astrbot_config(html_content, plugin_obj)
return (
Response()
.ok(
{
"html": html_content,
"title": f"{plugin_obj.display_name or plugin_obj.name} 扩展页面",
},
"成功获取扩展页面",
)
.__dict__
)
```
These small helpers keep all existing functionality (including reserved plugin support and async FS checks) while making each handler’s core logic shorter and more maintainable.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return Response().error(f"插件 {plugin_name} 未配置扩展页面").__dict__ | ||
|
|
||
| try: | ||
| with open(extension_page_path, encoding="utf-8") as f: |
Contributor
There was a problem hiding this comment.
suggestion (performance): 这个异步处理函数中的文件 I/O 仍然是同步执行的，可以放到已有的 to_thread 调用中一起处理。
现在存在性检查已经使用了 asyncio.to_thread，但 open(...).read() 仍然在事件循环线程上运行，对于大文件或较慢的存储，会阻塞其他请求。
你可以通过把读取操作也放进 to_thread 来保持非阻塞，例如：
html_content = await asyncio.to_thread(
lambda: Path(extension_page_path).read_text(encoding="utf-8")
)这样可以让处理函数对异步更友好，避免在事件循环中执行文件 I/O。
建议的实现方式：
import os
from pathlib import Path if not await asyncio.to_thread(os.path.exists, extension_page_path):
return Response().error(f"插件 {plugin_name} 未配置扩展页面").__dict__
try:
html_content = await asyncio.to_thread(
lambda: Path(extension_page_path).read_text(encoding="utf-8")
)- 确认稍后在这个处理函数中确实会使用
html_content（例如返回或传入模板/响应）；目前代码片段中只看到读取步骤。 - 如果该模块已经在别处引入了
Path，或者使用了不同的导入风格（例如在共享工具模块中统一from pathlib import Path），请调整导入方式以符合现有约定。
Original comment in English
suggestion (performance): Direct file I/O in this async handler is still done synchronously and could be moved into the existing to_thread usage.
The existence check now uses asyncio.to_thread, but open(...).read() still runs on the event loop thread and can stall other requests for large files or slow storage.
You can keep this non-blocking by moving the read into to_thread as well, e.g.:
html_content = await asyncio.to_thread(
lambda: Path(extension_page_path).read_text(encoding="utf-8")
)This keeps the handler fully async-friendly and avoids blocking the event loop with file I/O.
Suggested implementation:
import os
from pathlib import Path if not await asyncio.to_thread(os.path.exists, extension_page_path):
return Response().error(f"插件 {plugin_name} 未配置扩展页面").__dict__
try:
html_content = await asyncio.to_thread(
lambda: Path(extension_page_path).read_text(encoding="utf-8")
)- Ensure that
html_contentis actually used later in this handler (e.g. returned or passed into a template/response); currently only the read step is visible in the snippet. - If the module already imports
Pathelsewhere or has a different import style (e.g.from pathlib import Pathin a shared utilities module), adjust the import block to match existing conventions.
|
|
||
| # 使用正则表达式不区分大小写查找 </head> 标签 | ||
| pattern = re.compile(r"</head\s*>", re.IGNORECASE) | ||
| html_content = pattern.sub(injected_script + r"</head>", html_content) |
Contributor
There was a problem hiding this comment.
suggestion: 如果扩展页面缺少 标签，当前注入逻辑会静默失败；建议增加一个兜底的注入位置。
当前的正则方式仅在存在 </head> 时工作。如果缺少该标签，pattern.sub 不会产生任何替换，页面也就拿不到 window.ASTRBOT_CONFIG，这对插件作者来说可能比较困惑。
可以考虑：如果替换后 html_content 没有发生变化，则退而选择在存在 </body> 时注入到其前面，或者直接追加到文档末尾。这样可以让注入逻辑适配更多 HTML 变体。
Original comment in English
suggestion: If the extension page lacks a tag, the injection silently does nothing; consider a fallback injection point.
The regex approach works only when a </head> exists. If it’s missing, pattern.sub is a no-op and the page never gets window.ASTRBOT_CONFIG, which can be confusing for plugin authors.
Consider: if the substitution doesn’t change html_content, fall back to injecting before </body> when present, or appending at the end of the document. This would make the injection behave correctly for more HTML variants.
| "/plugin/reload": ("POST", self.reload_plugins), | ||
| "/plugin/readme": ("GET", self.get_plugin_readme), | ||
| "/plugin/changelog": ("GET", self.get_plugin_changelog), | ||
| "/plugin/extension_page": ("GET", self.get_plugin_extension_page), |
Contributor
There was a problem hiding this comment.
issue (complexity): 建议把插件查找、目录解析、异步文件系统检查以及 HTML 注入等共用逻辑抽取到可复用的辅助函数中，以简化新的扩展页面功能及相关处理函数。
你可以通过把这些重复模式集中到一些小的辅助函数中并在各个处理函数间复用，来降低扩展页面相关逻辑的复杂度。
1. 统一插件目录解析和插件查找
get_plugins、get_plugin_readme、get_plugin_changelog 和 get_plugin_extension_page 都在重复实现“按名称查找插件”和“解析插件目录（保留插件与非保留插件）”。
可以新增辅助函数：
def _find_plugin_by_name(self, name: str):
for plugin in self.plugin_manager.context.get_all_stars():
if plugin.name == name:
return plugin
return None
def _get_plugin_dir(self, plugin_obj) -> str:
base = (
self.plugin_manager.reserved_plugin_path
if plugin_obj.reserved
else self.plugin_manager.plugin_store_path
)
return os.path.join(base, plugin_obj.root_dir_name or "")然后在使用处做简化，例如在 get_plugin_readme 中：
async def get_plugin_readme(self):
plugin_name = request.args.get("name")
if not plugin_name:
return Response().error("插件名称不能为空").__dict__
plugin_obj = self._find_plugin_by_name(plugin_name)
if not plugin_obj:
return Response().error(f"插件 {plugin_name} 不存在").__dict__
if not plugin_obj.root_dir_name:
return Response().error(f"插件 {plugin_name} 目录不存在").__dict__
plugin_dir = self._get_plugin_dir(plugin_obj)
if not await asyncio.to_thread(os.path.isdir, plugin_dir):
return Response().error(f"无法找到插件 {plugin_name} 的目录").__dict__
readme_path = os.path.join(plugin_dir, "README.md")
if not await asyncio.to_thread(os.path.isfile, readme_path):
return Response().error(f"插件 {plugin_name} 没有README文件").__dict__
with open(readme_path, encoding="utf-8") as f:
readme_content = f.read()
return Response().ok({"content": readme_content}, "成功获取README内容").__dict__get_plugin_changelog 和 get_plugin_extension_page 也可以类似简化：
plugin_obj = self._find_plugin_by_name(plugin_name)
# ...
plugin_dir = self._get_plugin_dir(plugin_obj)在 get_plugins 中则可以：
plugin_dir = self._get_plugin_dir(plugin)
_t["extension_page"] = await self._aexists(os.path.join(plugin_dir, "web", "index.html"))这样可以保持保留插件/非保留插件处理的一致性，并去除重复的分支逻辑。
2. 封装异步文件系统检查
内联的 await asyncio.to_thread(os.path.isdir, ...) / os.path.isfile / os.path.exists 在代码中出现多次，让处理函数显得比较嘈杂。
可以新增几个简单的异步辅助函数：
async def _aisdir(self, path: str) -> bool:
return await asyncio.to_thread(os.path.isdir, path)
async def _aisfile(self, path: str) -> bool:
return await asyncio.to_thread(os.path.isfile, path)
async def _aexists(self, path: str) -> bool:
return await asyncio.to_thread(os.path.exists, path)然后在调用处简化为：
if not await self._aisdir(plugin_dir):
...
if not await self._aisfile(readme_path):
...
if await self._aisfile(changelog_path):
...
if not await self._aexists(extension_page_path):
...在保持异步 offloading 的前提下，这样可以让控制流更易于阅读。
3. 抽取 HTML 注入为辅助函数
get_plugin_extension_page 中的正则编译和注入字符串可以抽取出来，以更清晰地表达意图：
def _inject_astrbot_config(self, html_content: str, plugin_obj) -> str:
safe_plugin_name = json.dumps(plugin_obj.name)
injected_script = f"""
<script>
window.ASTRBOT_CONFIG = {{
apiUrl: "/api/plug",
pluginName: {safe_plugin_name},
apiToken: localStorage.getItem("token") || ""
}};
</script>
"""
pattern = re.compile(r"</head\s*>", re.IGNORECASE)
return pattern.sub(injected_script + r"</head>", html_content)然后 get_plugin_extension_page 可以简化为：
with open(extension_page_path, encoding="utf-8") as f:
html_content = f.read()
html_content = self._inject_astrbot_config(html_content, plugin_obj)
return (
Response()
.ok(
{
"html": html_content,
"title": f"{plugin_obj.display_name or plugin_obj.name} 扩展页面",
},
"成功获取扩展页面",
)
.__dict__
)这些小的辅助函数在保留现有功能（包括保留插件支持和异步文件系统检查）的同时，使每个处理函数的核心逻辑更短、更易维护。
Original comment in English
issue (complexity): Consider extracting shared plugin lookup, directory resolution, async filesystem checks, and HTML injection into reusable helpers to simplify the new extension-page feature and related handlers.
You can reduce the new extension-page complexity by centralizing the repeated patterns into small helpers and reusing them across handlers.
1. Centralize plugin-dir resolution and plugin lookup
get_plugins, get_plugin_readme, get_plugin_changelog, and get_plugin_extension_page all reimplement “find plugin by name” and “resolve plugin directory (reserved vs non-reserved)”.
Add helpers:
def _find_plugin_by_name(self, name: str):
for plugin in self.plugin_manager.context.get_all_stars():
if plugin.name == name:
return plugin
return None
def _get_plugin_dir(self, plugin_obj) -> str:
base = (
self.plugin_manager.reserved_plugin_path
if plugin_obj.reserved
else self.plugin_manager.plugin_store_path
)
return os.path.join(base, plugin_obj.root_dir_name or "")Then simplify usages, e.g. in get_plugin_readme:
async def get_plugin_readme(self):
plugin_name = request.args.get("name")
if not plugin_name:
return Response().error("插件名称不能为空").__dict__
plugin_obj = self._find_plugin_by_name(plugin_name)
if not plugin_obj:
return Response().error(f"插件 {plugin_name} 不存在").__dict__
if not plugin_obj.root_dir_name:
return Response().error(f"插件 {plugin_name} 目录不存在").__dict__
plugin_dir = self._get_plugin_dir(plugin_obj)
if not await asyncio.to_thread(os.path.isdir, plugin_dir):
return Response().error(f"无法找到插件 {plugin_name} 的目录").__dict__
readme_path = os.path.join(plugin_dir, "README.md")
if not await asyncio.to_thread(os.path.isfile, readme_path):
return Response().error(f"插件 {plugin_name} 没有README文件").__dict__
with open(readme_path, encoding="utf-8") as f:
readme_content = f.read()
return Response().ok({"content": readme_content}, "成功获取README内容").__dict__Similarly for get_plugin_changelog and get_plugin_extension_page:
plugin_obj = self._find_plugin_by_name(plugin_name)
# ...
plugin_dir = self._get_plugin_dir(plugin_obj)And in get_plugins:
plugin_dir = self._get_plugin_dir(plugin)
_t["extension_page"] = await self._aexists(os.path.join(plugin_dir, "web", "index.html"))This keeps reserved/non-reserved handling consistent and removes duplicated branches.
2. Wrap async filesystem checks
Inline await asyncio.to_thread(os.path.isdir, ...) / os.path.isfile / os.path.exists appears multiple times and makes the handlers noisier.
Add small async helpers:
async def _aisdir(self, path: str) -> bool:
return await asyncio.to_thread(os.path.isdir, path)
async def _aisfile(self, path: str) -> bool:
return await asyncio.to_thread(os.path.isfile, path)
async def _aexists(self, path: str) -> bool:
return await asyncio.to_thread(os.path.exists, path)Then simplify:
if not await self._aisdir(plugin_dir):
...
if not await self._aisfile(readme_path):
...
if await self._aisfile(changelog_path):
...
if not await self._aexists(extension_page_path):
...This makes the control flow easier to scan while preserving the async offloading.
3. Extract HTML injection into a helper
The regex compilation and injection string in get_plugin_extension_page can be extracted to clarify intent:
def _inject_astrbot_config(self, html_content: str, plugin_obj) -> str:
safe_plugin_name = json.dumps(plugin_obj.name)
injected_script = f"""
<script>
window.ASTRBOT_CONFIG = {{
apiUrl: "/api/plug",
pluginName: {safe_plugin_name},
apiToken: localStorage.getItem("token") || ""
}};
</script>
"""
pattern = re.compile(r"</head\s*>", re.IGNORECASE)
return pattern.sub(injected_script + r"</head>", html_content)Then get_plugin_extension_page becomes:
with open(extension_page_path, encoding="utf-8") as f:
html_content = f.read()
html_content = self._inject_astrbot_config(html_content, plugin_obj)
return (
Response()
.ok(
{
"html": html_content,
"title": f"{plugin_obj.display_name or plugin_obj.name} 扩展页面",
},
"成功获取扩展页面",
)
.__dict__
)These small helpers keep all existing functionality (including reserved plugin support and async FS checks) while making each handler’s core logic shorter and more maintainable.
Soulter
force-pushed
the
master
branch
2 times, most recently
from
faf411f to
0068960
Compare
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
2 participants
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.
Modifications / 改动点
核心文件修改：
后端修改 (
astrbot/dashboard/routes/plugin.py)/plugin/extension_pageAPI 端点，用于获取插件扩展页面的 HTML 内容get_plugins()方法中添加extension_page字段，标识插件是否具有扩展页面<plugin_root>/web/index.htmlwindow.ASTRBOT_CONFIG配置对象（包含 API 地址、插件名称、JWT 令牌）前端组件修改 (
dashboard/src/components/shared/ExtensionCard.vue)view-extension-page事件发射器viewExtensionPage()函数mdi-web图标前端视图修改 (
dashboard/src/views/ExtensionPage.vue)extensionPageDialog响应式状态（包含 show、html、title 字段）openExtensionPage()和fetchExtensionPage()函数sandbox="allow-scripts allow-forms allow-same-origin"安全属性国际化修改
dashboard/src/i18n/locales/zh-CN/features/extension.json：添加"extensionPage": "插件扩展页面"dashboard/src/i18n/locales/en-US/features/extension.json：添加"extensionPage": "Extension Page"实现的功能：
插件可提供自定义 HTML 页面（固定位置：
web/index.html）自动检测并标识具有扩展页面的插件
扩展页面通过 iframe 沙箱环境安全加载
自动注入配置对象
window.ASTRBOT_CONFIG供前端使用使用 JWT 认证机制确保 API 通信安全
完整的向后兼容（无扩展页面的插件不受影响）
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.Summary by Sourcery
添加对插件自定义扩展页面的支持，这些页面可以在后端被发现，并可从仪表盘中以沙盒化全屏对话框的方式打开。
新功能：
web/index.html位置标记那些提供扩展页面的插件。iframe渲染的全屏对话框中打开插件的扩展页面。Original summary in English
Summary by Sourcery
Add support for plugin-defined extension pages that can be discovered on the backend and opened from the dashboard in a sandboxed full-screen dialog.
New Features:
新功能：
Original summary in English
Summary by Sourcery
添加对插件自定义扩展页面的支持，这些页面可以在后端被发现，并可从仪表盘中以沙盒化全屏对话框的方式打开。
新功能：
web/index.html位置标记那些提供扩展页面的插件。iframe渲染的全屏对话框中打开插件的扩展页面。Original summary in English
Summary by Sourcery
Add support for plugin-defined extension pages that can be discovered on the backend and opened from the dashboard in a sandboxed full-screen dialog.
New Features: