Skip to content

🔒 使用 DOMPurify 清理公告通知 HTML 内容#1274

Merged
CodFrm merged 2 commits intomainfrom
fix/1273
Mar 3, 2026
Merged

🔒 使用 DOMPurify 清理公告通知 HTML 内容#1274
CodFrm merged 2 commits intomainfrom
fix/1273

Conversation

@CodFrm
Copy link
Member

@CodFrm CodFrm commented Mar 3, 2026

概述 Descriptions

close #1273

变更内容 Changes

  • 使用 DOMPurify 对服务端下发的公告 HTML 进行白名单过滤,防止潜在的 UI 注入风险
  • 新增 src/pkg/utils/sanitize.ts 封装清理逻辑
  • 允许的标签:b, i, a, br, p, strong, em, span
  • 允许的 CSS 属性:color, font-size, font-weight, font-style
  • 其它标签、属性和危险的 CSS 属性(如 positionbackground)会被自动过滤

  • Sanitize server-provided notice HTML with DOMPurify whitelist filtering to prevent potential UI injection risks
  • Add src/pkg/utils/sanitize.ts to encapsulate sanitization logic
  • Allowed tags: b, i, a, br, p, strong, em, span
  • Allowed CSS properties: color, font-size, font-weight, font-style
  • All other tags, attributes, and dangerous CSS properties (e.g. position, background) are automatically stripped

截图 Screenshots

N/A

使用 DOMPurify 对服务端下发的公告 HTML 进行白名单过滤,防止潜在的
UI 注入风险。只允许基础标签和安全的 CSS 属性(颜色、字体相关)。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 针对弹窗里“更新公告/通知”由服务端下发并以 dangerouslySetInnerHTML 渲染的场景,引入 DOMPurify 白名单清理,降低扩展 UI 被远程 HTML 注入的安全风险(对应 issue #1273)。

Changes:

  • 新增 src/pkg/utils/sanitize.ts:封装基于 DOMPurify 的 HTML 清理与 style 属性白名单过滤
  • Popup 公告渲染前对 checkUpdate.notice 做清理后再注入
  • 增加 dompurify 依赖并更新锁文件

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/pkg/utils/sanitize.ts 新增 DOMPurify 清理逻辑与 CSS 白名单 hook
src/pages/popup/App.tsx 公告内容渲染前调用 sanitizeHTML
package.json 添加 dompurify 运行时依赖
pnpm-lock.yaml 锁定新增依赖及其可选依赖
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Comment on lines +6 to +16
// 过滤不安全的 CSS 属性,只保留白名单中的属性
DOMPurify.addHook("afterSanitizeAttributes", (node) => {
if (node instanceof HTMLElement && node.hasAttribute("style")) {
const { style } = node;
for (let i = style.length - 1; i >= 0; i--) {
if (!ALLOWED_CSS_PROPERTIES.includes(style[i])) {
style.removeProperty(style[i]);
}
}
}
});
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里在模块顶层调用 DOMPurify.addHook 会产生全局副作用:任何后续对 DOMPurify 的使用都会被这个 style 白名单逻辑影响,并且在热重载/多实例场景可能重复注册 hook。更稳妥的做法是创建专用的 DOMPurify 实例(例如基于当前 window 创建实例)并只在该实例上注册 hook,或至少加一次性初始化保护。

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +24
// 对 HTML 进行清理,只保留安全的标签和属性
export function sanitizeHTML(html: string): string {
return DOMPurify.sanitize(html, {
ALLOWED_TAGS: ["b", "i", "a", "br", "p", "strong", "em", "span"],
ALLOWED_ATTR: ["href", "target", "style"],
});
}
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

新增的 sanitizeHTML 属于安全关键逻辑,但目前在 src/pkg/utils/ 下没有对应单测覆盖。建议补充 sanitize.test.ts:至少覆盖(1)移除 <script>/事件处理属性(2)过滤非白名单标签与属性(3)style 仅保留白名单 CSS 属性,且能正确剔除如 position/background/top 等。

Copilot generated this review using guidance from repository custom instructions.
style={{ display: showAlert ? "flex" : "none" }}
type="info"
content={<div dangerouslySetInnerHTML={{ __html: checkUpdate.notice || "" }} />}
content={<div dangerouslySetInnerHTML={{ __html: sanitizeHTML(checkUpdate.notice || "") }} />}
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里在组件每次 render 时都会调用一次 DOMPurify 清理,DOMPurify 相对开销较大,可能导致 popup 频繁重渲染时的性能抖动。建议用 useMemo 基于 checkUpdate.notice 缓存清理后的字符串(或在拿到 notice 时就预先清理并存储)。

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +23
return DOMPurify.sanitize(html, {
ALLOWED_TAGS: ["b", "i", "a", "br", "p", "strong", "em", "span"],
ALLOWED_ATTR: ["href", "target", "style"],
});
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sanitizeHTML 允许 target 但未同时允许/补齐 rel="noopener noreferrer"。当公告里存在 <a target="_blank"> 时可能引入反向 tabnabbing 风险(新页面可通过 window.opener 操作原扩展页面)。建议:要么不允许 target,由外层统一用安全方式打开链接;要么在 DOMPurify hook 中对 A 标签设置 rel=noopener noreferrer(并将 rel 加入允许属性)。

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@cyfung1031 cyfung1031 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodFrm 你直接改 JSON 吧。内容在ScriptCat扩充做Render
资讯下载量也会较小
这些 DOMPurify不太可靠

@CodFrm
Copy link
Member Author

CodFrm commented Mar 3, 2026

@CodFrm 你直接改 JSON 吧。内容在ScriptCat扩充做Render 资讯下载量也会较小 这些 DOMPurify不太可靠

什么意思?改JSON?

@cyfung1031
Copy link
Collaborator

cyfung1031 commented Mar 3, 2026

@CodFrm 你直接改 JSON 吧。内容在ScriptCat扩充做Render 资讯下载量也会较小 这些 DOMPurify不太可靠

什么意思?改JSON?

打錯字。改用JSON

由「接收服务端下发的公告 HTML 」改成 「接收服务端下发的公告 JSON 」
把 JSON物件在本地端做解析,显示到 React 元件

而不是直接下载 HTML内容 直接放在 React

@cyfung1031
Copy link
Collaborator

cyfung1031 commented Mar 3, 2026

@CodFrm 你直接改 JSON 吧。内容在ScriptCat扩充做Render 资讯下载量也会较小 这些 DOMPurify不太可靠

什么意思?改JSON?

打錯字。改用JSON

由「接收服务端下发的公告 HTML 」改成 「接收服务端下发的公告 JSON 」 把 JSON物件在本地端做解析,显示到 React 元件

而不是直接下载 HTML内容 直接放在 React

呀。现在已经是 json
只是 json 里放了 html "notice"

把这个 notice 做成物件吧

例如

{
  "notice": {
    "line1": {
      "message": "第一行文字",
      "size": 12,
      "color": "red"
    },
    "line2": {
      "message": "第二行文字",
      "size": 10,
      "color": "#000"
    }
  }
}

@cyfung1031
Copy link
Collaborator

或者你想简单点
在 server 那边做 html -> json
再在 SC extension 做 rendering

可以用这个

https://github.com/lemonadejs/html-to-json

把 type 不合适的都删掉,只保留 div, span, b, p, br 等

@cyfung1031
Copy link
Collaborator

@CodFrm 你直接改 JSON 吧。内容在ScriptCat扩充做Render 资讯下载量也会较小 这些 DOMPurify不太可靠


我收回前言

这个 DOMPurify 看来也不错
就这样吧

@CodFrm CodFrm merged commit 4617709 into main Mar 3, 2026
1 of 3 checks passed
@CodFrm
Copy link
Member Author

CodFrm commented Mar 3, 2026

你改到sw环境去了,好像出问题了

@cyfung1031
Copy link
Collaborator

你改到sw环境去了,好像出问题了

呀。它可能要用 document 来做处理。。。 sw 不能用sanitizeHTML

cyfung1031 added a commit that referenced this pull request Mar 3, 2026
CodFrm pushed a commit that referenced this pull request Mar 4, 2026
* fix #1274

* Update index.ts

* Array.includes -> Set.has
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] security vulnerability

3 participants