fix(dashboard): allow plugin page iframe popups#8457
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Relaxing the iframe sandbox with
allow-popupsand especiallyallow-popups-to-escape-sandboxhas security implications; consider whether popups truly need to escape the sandbox or ifallow-popupsalone is sufficient, and document/limit this behavior to trusted plugin origins if possible. - Instead of hardcoding the sandbox string, consider moving it to a computed property or constant (e.g.,
pluginPageSandboxAttrs) so it’s easier to audit and adjust permissions in one place if sandbox flags need to change again.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Relaxing the iframe sandbox with `allow-popups` and especially `allow-popups-to-escape-sandbox` has security implications; consider whether popups truly need to escape the sandbox or if `allow-popups` alone is sufficient, and document/limit this behavior to trusted plugin origins if possible.
- Instead of hardcoding the sandbox string, consider moving it to a computed property or constant (e.g., `pluginPageSandboxAttrs`) so it’s easier to audit and adjust permissions in one place if sandbox flags need to change again.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/PluginPagePage.vue" line_range="499" />
<code_context>
class="plugin-page-frame"
referrerpolicy="no-referrer"
- sandbox="allow-scripts allow-forms allow-downloads"
+ sandbox="allow-scripts allow-forms allow-downloads allow-popups allow-popups-to-escape-sandbox"
@load="handleIframeLoad"
></iframe>
</code_context>
<issue_to_address>
**🚨 issue (security):** Reconsider `allow-popups-to-escape-sandbox` because it significantly relaxes isolation and can lead to navigation of the top window.
This flag lets popup windows break out of the iframe sandbox and potentially navigate the opener/top window, which increases the impact of any compromised or malicious plugin. If plugins only need to open external links, consider using just `allow-popups` and routing external navigation through a controlled in-app handler instead of enabling `allow-popups-to-escape-sandbox`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| class="plugin-page-frame" | ||
| referrerpolicy="no-referrer" | ||
| sandbox="allow-scripts allow-forms allow-downloads" | ||
| sandbox="allow-scripts allow-forms allow-downloads allow-popups allow-popups-to-escape-sandbox" |
There was a problem hiding this comment.
🚨 issue (security): Reconsider allow-popups-to-escape-sandbox because it significantly relaxes isolation and can lead to navigation of the top window.
This flag lets popup windows break out of the iframe sandbox and potentially navigate the opener/top window, which increases the impact of any compromised or malicious plugin. If plugins only need to open external links, consider using just allow-popups and routing external navigation through a controlled in-app handler instead of enabling allow-popups-to-escape-sandbox.
There was a problem hiding this comment.
Code Review
This pull request updates the iframe sandbox configuration in PluginPagePage.vue to include allow-popups and allow-popups-to-escape-sandbox. The reviewer raised a critical security concern, pointing out that allow-popups-to-escape-sandbox can lead to sandbox escape and credential theft (such as JWT tokens) because the iframe shares the same origin. The reviewer recommended implementing a Bridge proxy mechanism to safely handle external link opening instead of bypassing the sandbox.
| class="plugin-page-frame" | ||
| referrerpolicy="no-referrer" | ||
| sandbox="allow-scripts allow-forms allow-downloads" | ||
| sandbox="allow-scripts allow-forms allow-downloads allow-popups allow-popups-to-escape-sandbox" |
There was a problem hiding this comment.
⚠️ 安全风险提示:allow-popups-to-escape-sandbox 导致沙箱逃逸\n\n由于插件页面的 iframe 与 Dashboard 处于同源(均在 /api/plugin/page/content/... 下),如果启用了 allow-popups-to-escape-sandbox,会引入严重的沙箱逃逸风险:\n\n1. 逃逸原理:iframe 内的插件页面可以通过 window.open 打开一个同源的页面(例如 Dashboard 的其他路由,或其自身的页面)。\n2. 权限提升:由于新打开的窗口/标签页脱离了沙箱限制,它将以完整的同源权限运行,从而可以直接访问 Dashboard 的 localStorage(窃取 JWT Token)、cookie 并调用后台敏感 API。\n3. 危害:这使得原本通过 sandbox(未设置 allow-same-origin)建立的隔离屏障完全失效。\n\n---\n\n### 💡 推荐的黄金方案:通过 Bridge 代理打开外链\n\n为了在保持严格沙箱(不启用 allow-popups-to-escape-sandbox)的同时,允许插件页面正常打开外部链接,推荐使用 Bridge 代理机制:\n\n#### 1. 在 PluginPagePage.vue 中支持 window:open 动作\n在 handleBridgeRequest(约第 210 行起)中增加对 window:open 的处理,由父窗口(Dashboard)来安全地执行 window.open:\n```javascript\nif (action === "window:open") {\n const { url } = message;\n if (typeof url === "string" && /^https?:\/\//i.test(url)) {
window.open(url, \"_blank\", \"noopener,noreferrer\");\n sendBridgeResponse(requestId, true, {});\n } else {
throw new Error(\"Invalid or unsafe URL\");\n }\n return;\n}\n```\n\n#### 2. 在 `astrbot/dashboard/plugin_page_bridge.js` 中自动拦截与代理\n在 Bridge SDK 中,可以重写 `window.open` 并全局拦截 `target=\"_blank\"` 的链接点击,将其无缝转发给父窗口:\n```javascript\n// 1. 导出代理方法\nwindow.AstrBotPluginPage = {\n // ... 其他方法\n openExternal(url) {\n return makeRequest(\"window:open\", { url });\n }\n};\n\n// 2. 自动代理 window.open\nconst originalOpen = window.open;\nwindow.open = function(url, target, features) {\n if (target === \"_blank\" || !target) {\n window.AstrBotPluginPage.openExternal(url);\n return null;\n }\n return originalOpen.apply(this, arguments);\n};\n\n// 3. 自动拦截 a[target=\"_blank\"] 点击\ndocument.addEventListener(\"click\", (event) => {\n const anchor = event.target.closest(\"a\");\n if (anchor && anchor.target === \"_blank\") {\n const href = anchor.href;\n if (href && /^https?:\\/\\//i.test(href)) {\n event.preventDefault();\n window.AstrBotPluginPage.openExternal(href);\n }\n }\n});\n```\n\n通过这种方式,插件开发者无需修改任何代码(标准的 `window.open` 和 `<a>` 标签会被自动代理),同时 Dashboard 能够保持极高的安全性。
sandbox="allow-scripts allow-forms allow-downloads"
|
关于两个模型的审核结论, sandbox="allow-popups-to-escape-sandbox" 高危问题,实际是误判。该策略只影响浏览器端行为,不会直接影响 AstrBot 后端、服务器、项目文件或运行时环境。主要影响面是用户本地浏览器,用户可能被插件页面打开到外部站点,外部站点按普通页面运行,不再被 iframe sandbox 限制。主要价值在于例如OAuth/外部页面完整流程等,让插件能够实现更有创造力的内容,而主要风险就是打开的外部网页链接本身可能执行的内容。 |
修复插件自定义 Page 无法弹出新窗口或新标签页的问题。
插件自定义 Page 通过 iframe 加载时,当前 sandbox 权限未允许弹窗,导致页面内使用
window.open()或target="_blank"打开新窗口/新标签页时会被浏览器拦截,对插件自定义page的功能丰富度不友好。Modifications / 改动点
修改
dashboard/src/views/PluginPagePage.vue为插件自定义 Page iframe 的
sandbox属性增加allow-popups和allow-popups-to-escape-sandbox保留原有 sandbox 权限:
allow-scripts、allow-forms、allow-downloadsThis is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 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
New Features: