From 9315302516fb1599745fa8503443e71309550ecf Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Mon, 13 Apr 2026 16:08:17 -0500 Subject: [PATCH] fix(ui): replace marked.js with markdown-it to fix ReDoS UI freeze (#46707) thanks @zhangfnf Replace marked.js with markdown-it for the control UI chat markdown renderer to eliminate a ReDoS vulnerability that could freeze the browser tab. - Configure markdown-it with custom renderers matching marked.js output - Add GFM www-autolink with trailing punctuation stripping per spec - Escape raw HTML via html_block/html_inline overrides - Flatten remote images to alt text, preserve base64 data URI images - Add task list support via markdown-it-task-lists plugin - Trim trailing CJK characters from auto-linked URLs (RFC 3986) - Keep marked dependency for agents-panels-status-files.ts usage Co-authored-by: zhangfan49 Co-authored-by: Nova --- CHANGELOG.md | 2 + pnpm-lock.yaml | 14 + ui/package.json | 3 + ui/src/markdown-it-task-lists.d.ts | 10 + ui/src/styles/chat/text.css | 14 + ui/src/ui/markdown.test.ts | 494 +++++++++++++++++++++++++++-- ui/src/ui/markdown.ts | 464 ++++++++++++++++++++------- 7 files changed, 869 insertions(+), 132 deletions(-) create mode 100644 ui/src/markdown-it-task-lists.d.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 143645c2c8a..32543f60d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Docs: https://docs.openclaw.ai ## Unreleased +- fix(ui): replace marked.js with markdown-it to fix ReDoS UI freeze (#46707) thanks @zhangfnf + ### Changes - Telegram/forum topics: surface human topic names in agent context, prompt metadata, and plugin hook metadata by learning names from Telegram forum service messages. (#65973) Thanks @ptahdunbar. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f105f25933c..7dc9d79af81 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1297,10 +1297,19 @@ importers: lit: specifier: ^3.3.2 version: 3.3.2 + markdown-it: + specifier: ^14.1.1 + version: 14.1.1 + markdown-it-task-lists: + specifier: ^2.1.1 + version: 2.1.1 marked: specifier: ^18.0.0 version: 18.0.0 devDependencies: + '@types/markdown-it': + specifier: ^14.1.2 + version: 14.1.2 '@vitest/browser-playwright': specifier: 4.1.4 version: 4.1.4(playwright@1.59.1)(vite@8.0.8(@types/node@25.6.0)(esbuild@0.27.7)(jiti@2.6.1)(tsx@4.21.0)(yaml@2.8.3))(vitest@4.1.4) @@ -6055,6 +6064,9 @@ packages: resolution: {integrity: sha512-hXdUTZYIVOt1Ex//jAQi+wTZZpUpwBj/0QsOzqegb3rGMMeJiSEu5xLHnYfBrRV4RH2+OCSOO95Is/7x1WJ4bw==} engines: {node: '>=10'} + markdown-it-task-lists@2.1.1: + resolution: {integrity: sha512-TxFAc76Jnhb2OUu+n3yz9RMu4CwGfaT788br6HhEDlvWfdeJcLUsxk1Hgw2yJio0OXsxv7pyIPmvECY7bMbluA==} + markdown-it@14.1.1: resolution: {integrity: sha512-BuU2qnTti9YKgK5N+IeMubp14ZUKUUw7yeJbkjtosvHiP0AZ5c8IAgEMk79D0eC8F23r4Ac/q8cAIFdm2FtyoA==} hasBin: true @@ -13273,6 +13285,8 @@ snapshots: dependencies: semver: 7.7.4 + markdown-it-task-lists@2.1.1: {} + markdown-it@14.1.1: dependencies: argparse: 2.0.1 diff --git a/ui/package.json b/ui/package.json index f548ae10d6e..d9ae1d0a1e0 100644 --- a/ui/package.json +++ b/ui/package.json @@ -13,9 +13,12 @@ "@noble/ed25519": "3.0.1", "dompurify": "^3.3.3", "lit": "^3.3.2", + "markdown-it": "^14.1.1", + "markdown-it-task-lists": "^2.1.1", "marked": "^18.0.0" }, "devDependencies": { + "@types/markdown-it": "^14.1.2", "@vitest/browser-playwright": "4.1.4", "jsdom": "^29.0.2", "playwright": "^1.59.1", diff --git a/ui/src/markdown-it-task-lists.d.ts b/ui/src/markdown-it-task-lists.d.ts new file mode 100644 index 00000000000..80f53dddadc --- /dev/null +++ b/ui/src/markdown-it-task-lists.d.ts @@ -0,0 +1,10 @@ +declare module "markdown-it-task-lists" { + import type MarkdownIt from "markdown-it"; + interface TaskListsOptions { + enabled?: boolean; + label?: boolean; + labelAfter?: boolean; + } + const plugin: (md: MarkdownIt, options?: TaskListsOptions) => void; + export default plugin; +} diff --git a/ui/src/styles/chat/text.css b/ui/src/styles/chat/text.css index ca2227658db..b06b9d59de0 100644 --- a/ui/src/styles/chat/text.css +++ b/ui/src/styles/chat/text.css @@ -41,6 +41,20 @@ margin-top: 0.25em; } +/* Hide default marker only for unordered task lists; ordered lists keep numbers */ +.chat-text :where(ul > .task-list-item), +.sidebar-markdown :where(ul > .task-list-item), +.chat-thinking :where(ul > .task-list-item) { + list-style: none; +} + +.chat-text :where(.task-list-item-checkbox), +.sidebar-markdown :where(.task-list-item-checkbox), +.chat-thinking :where(.task-list-item-checkbox) { + margin-right: 0.4em; + vertical-align: middle; +} + .chat-text :where(a) { color: var(--accent); text-decoration: underline; diff --git a/ui/src/ui/markdown.test.ts b/ui/src/ui/markdown.test.ts index e27faf8fbaa..831f7b4f7f8 100644 --- a/ui/src/ui/markdown.test.ts +++ b/ui/src/ui/markdown.test.ts @@ -1,8 +1,8 @@ -import { marked } from "marked"; import { describe, expect, it, vi } from "vitest"; -import { toSanitizedMarkdownHtml } from "./markdown.ts"; +import { md, toSanitizedMarkdownHtml } from "./markdown.ts"; describe("toSanitizedMarkdownHtml", () => { + // ── Original tests from before markdown-it migration ── it("renders basic markdown", () => { const html = toSanitizedMarkdownHtml("Hello **world**"); expect(html).toContain("world"); @@ -146,9 +146,9 @@ describe("toSanitizedMarkdownHtml", () => { expect(second).toBe(first); }); - it("falls back to escaped plain text if marked.parse throws (#36213)", () => { - const parseSpy = vi.spyOn(marked, "parse").mockImplementation(() => { - throw new Error("forced parse failure"); + it("falls back to escaped plain text if md.render throws (#36213)", () => { + const renderSpy = vi.spyOn(md, "render").mockImplementation(() => { + throw new Error("forced render failure"); }); const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); const input = `Fallback **probe** ${Date.now()}`; @@ -158,26 +158,484 @@ describe("toSanitizedMarkdownHtml", () => { expect(html).toContain("Fallback **probe**"); expect(warnSpy).toHaveBeenCalledOnce(); } finally { - parseSpy.mockRestore(); + renderSpy.mockRestore(); warnSpy.mockRestore(); } }); - it("keeps adjacent trailing CJK text outside bare auto-links", () => { - const html = toSanitizedMarkdownHtml("https://example.com重新解读"); - expect(html).toContain('https://example.com重新解读"); + // ── Additional tests for markdown-it migration ── + describe("www autolinks", () => { + it("links www.example.com", () => { + const html = toSanitizedMarkdownHtml("Visit www.example.com today"); + expect(html).toContain('"); + }); + + it("links www.example.com with path, query, and fragment", () => { + const html = toSanitizedMarkdownHtml("See www.example.com/path?a=1#section"); + expect(html).toContain(' { + const html = toSanitizedMarkdownHtml("Visit www.example.com:8080/foo"); + expect(html).toContain(' { + const html = toSanitizedMarkdownHtml("Visit www.localhost:3000/path for dev"); + expect(html).toContain(' { + // markdown-it linkify converts IDN to punycode; marked.js percent-encodes. + // Both are valid; we just verify the link is created. + const html1 = toSanitizedMarkdownHtml("Visit www.münich.de"); + expect(html1).toContain("www.münich.de"); + + const html2 = toSanitizedMarkdownHtml("Visit www.café.example"); + expect(html2).toContain("www.café.example"); + }); + + it("links www.foo_bar.example.com with underscores", () => { + const html = toSanitizedMarkdownHtml("Visit www.foo_bar.example.com"); + expect(html).toContain(' { + const html1 = toSanitizedMarkdownHtml("Check www.example.com/help."); + expect(html1).toContain('href="http://www.example.com/help"'); + expect(html1).not.toContain('href="http://www.example.com/help."'); + + const html2 = toSanitizedMarkdownHtml("See www.example.com!"); + expect(html2).toContain('href="http://www.example.com"'); + expect(html2).not.toContain('href="http://www.example.com!"'); + }); + + it("strips entity-like suffixes per GFM spec", () => { + // &hl; looks like an entity reference, so strip it + const html1 = toSanitizedMarkdownHtml("www.google.com/search?q=commonmark&hl;"); + expect(html1).toContain('href="http://www.google.com/search?q=commonmark"'); + expect(html1).toContain("&hl;"); // Entity shown outside link + + // & is also entity-like + const html2 = toSanitizedMarkdownHtml("www.example.com/path&"); + expect(html2).toContain('href="http://www.example.com/path"'); + }); + + it("handles quotes with balance checking", () => { + // Quoted URL — trailing unbalanced " is stripped + const html1 = toSanitizedMarkdownHtml('"www.example.com"'); + expect(html1).toContain('href="http://www.example.com"'); + expect(html1).not.toContain('href="http://www.example.com%22"'); + + // Balanced quotes inside path — preserved + const html2 = toSanitizedMarkdownHtml('www.example.com/path"with"quotes'); + expect(html2).toContain('www.example.com/path"with"quotes'); + + // Trailing unbalanced " — stripped + const html3 = toSanitizedMarkdownHtml('www.example.com/path"'); + expect(html3).toContain('href="http://www.example.com/path"'); + expect(html3).not.toContain('path%22"'); + }); + + it("does NOT link www. domains starting with non-ASCII", () => { + const html1 = toSanitizedMarkdownHtml("Visit www.ünich.de"); + expect(html1).not.toContain(" { + const html = toSanitizedMarkdownHtml("(see www.example.com/foo(bar))"); + expect(html).toContain('href="http://www.example.com/foo(bar)"'); + }); + + it("stops at < character", () => { + // Stops at < character + const html1 = toSanitizedMarkdownHtml("Visit www.example.com/path pattern — stops before < + const html2 = toSanitizedMarkdownHtml("Visit www.example.com/ here"); + expect(html2).toContain('href="http://www.example.com/"'); + expect(html2).toContain("<token>"); + }); + + it("does NOT link bare domains without www", () => { + const html = toSanitizedMarkdownHtml("Visit google.com today"); + expect(html).not.toContain(" { + const html = toSanitizedMarkdownHtml("Check README.md and config.json"); + expect(html).not.toContain(" { + const html = toSanitizedMarkdownHtml("Check 127.0.0.1:8080"); + expect(html).not.toContain(" { + const html = toSanitizedMarkdownHtml("www.example.com重新解读"); + expect(html).toContain('"); + }); + + it("keeps Japanese text outside www auto-links", () => { + const html = toSanitizedMarkdownHtml("www.example.comテスト"); + expect(html).toContain(' { - const html = toSanitizedMarkdownHtml("https://api.example.com?q=重新&lang=en"); - expect(html).toContain('href="https://api.example.com?q=%E9%87%8D%E6%96%B0&lang=en"'); - expect(html).toContain(">https://api.example.com?q=重新&lang=en"); + describe("explicit protocol links", () => { + it("links https:// URLs", () => { + const html = toSanitizedMarkdownHtml("Visit https://example.com"); + expect(html).toContain(' { + const html = toSanitizedMarkdownHtml("Visit http://github.com/openclaw"); + expect(html).toContain(' { + const html = toSanitizedMarkdownHtml("Email me at test@example.com"); + expect(html).toContain(' { + const html = toSanitizedMarkdownHtml("https://example.com重新解读"); + expect(html).toContain('https://example.com"); + expect(html).toContain("重新解读"); + }); + + it("keeps CJK text outside https:// links with path", () => { + const html = toSanitizedMarkdownHtml("https://example.com/path重新解读"); + expect(html).toContain(' { + // CJK in the middle of a URL path (not trailing) must not be trimmed + const html = toSanitizedMarkdownHtml("https://example.com/你/test"); + expect(html).toContain("你/test"); + expect(html).not.toContain("你/test你"); + }); + + it("preserves percent-encoded CJK inside URLs when no raw CJK present", () => { + // Percent-encoded paths without raw CJK are preserved as-is + const html = toSanitizedMarkdownHtml("https://example.com/path/%E4%BD%A0%E5%A5%BD"); + expect(html).toContain(" { + const html = toSanitizedMarkdownHtml("[OpenClaw中文](https://docs.openclaw.ai)"); + expect(html).toContain('href="https://docs.openclaw.ai"'); + expect(html).toContain("OpenClaw中文"); + }); + + it("preserves mailto: scheme when trimming CJK from email links", () => { + // Email followed by space+CJK — linkify recognizes the email, + // then CJK trim should preserve the mailto: prefix. + const html = toSanitizedMarkdownHtml("Contact test@example.com 中文说明"); + expect(html).toContain('href="mailto:test@example.com"'); + expect(html).toContain("test@example.com"); + }); }); - it("preserves valid mixed-script path segments inside auto-links", () => { - const html = toSanitizedMarkdownHtml("https://example.com/path/重新/file"); - expect(html).toContain('href="https://example.com/path/%E9%87%8D%E6%96%B0/file"'); - expect(html).toContain(">https://example.com/path/重新/file"); + describe("HTML escaping", () => { + it("escapes HTML tags as text", () => { + const html = toSanitizedMarkdownHtml("
**bold**
"); + expect(html).toContain("<div>"); + expect(html).not.toContain("
"); + // Inner markdown should NOT be rendered since it's inside escaped HTML + expect(html).toContain("**bold**"); + }); + + it("strips script tags", () => { + const html = toSanitizedMarkdownHtml(""); + expect(html).not.toContain(" { + const html = toSanitizedMarkdownHtml("Check this out"); + expect(html).toContain("<b>"); + expect(html).not.toContain(""); + }); + }); + + describe("task lists", () => { + it("renders task list checkboxes", () => { + const html = toSanitizedMarkdownHtml("- [ ] Unchecked\n- [x] Checked"); + expect(html).toContain(" { + const html = toSanitizedMarkdownHtml("- [ ] Task with [link](https://example.com)"); + expect(html).toContain(' { + const html = toSanitizedMarkdownHtml("- [ ] "); + expect(html).not.toContain(" { + const html = toSanitizedMarkdownHtml("- [ ]
xy
"); + expect(html).toContain("<details>"); + expect(html).not.toContain("
"); + }); + }); + + describe("images", () => { + it("flattens remote images to alt text", () => { + const html = toSanitizedMarkdownHtml("![Alt text](https://example.com/img.png)"); + expect(html).not.toContain(" { + const html = toSanitizedMarkdownHtml("![**Build log**](https://example.com/img.png)"); + expect(html).toContain("**Build log**"); + }); + + it("preserves code formatting in alt text", () => { + const html = toSanitizedMarkdownHtml("![`error.log`](https://example.com/img.png)"); + expect(html).toContain("`error.log`"); + }); + + it("preserves base64 data URI images (#15437)", () => { + const html = toSanitizedMarkdownHtml("![Chart](data:image/png;base64,iVBORw0KGgo=)"); + expect(html).toContain(" { + const html = toSanitizedMarkdownHtml("![](https://example.com/image.png)"); + expect(html).not.toContain(" { + it("renders fenced code blocks", () => { + const html = toSanitizedMarkdownHtml("```ts\nconsole.log(1)\n```"); + expect(html).toContain("
");
+      expect(html).toContain(" {
+      // markdown-it requires a blank line before indented code
+      const html = toSanitizedMarkdownHtml("text\n\n    indented code");
+      expect(html).toContain("
");
+      expect(html).toContain("");
+    });
+
+    it("includes copy button", () => {
+      const html = toSanitizedMarkdownHtml("```\ncode\n```");
+      expect(html).toContain('class="code-block-copy"');
+      expect(html).toContain("data-code=");
+    });
+
+    it("collapses JSON code blocks", () => {
+      const html = toSanitizedMarkdownHtml('```json\n{"key": "value"}\n```');
+      expect(html).toContain(" {
+    it("renders strikethrough", () => {
+      const html = toSanitizedMarkdownHtml("This is ~~deleted~~ text");
+      expect(html).toContain("deleted");
+    });
+
+    it("renders tables", () => {
+      const md = "| A | B |\n|---|---|\n| 1 | 2 |";
+      const html = toSanitizedMarkdownHtml(md);
+      expect(html).toContain("");
+    });
+
+    it("renders basic markdown", () => {
+      const html = toSanitizedMarkdownHtml("**bold** and *italic*");
+      expect(html).toContain("bold");
+      expect(html).toContain("italic");
+    });
+
+    it("renders headings", () => {
+      const html = toSanitizedMarkdownHtml("# Heading 1\n## Heading 2");
+      expect(html).toContain("
");
+      expect(html).toContain("
");
+    });
+
+    it("renders blockquotes", () => {
+      const html = toSanitizedMarkdownHtml("> quote");
+      expect(html).toContain("
");
+    });
+
+    it("renders lists", () => {
+      const html = toSanitizedMarkdownHtml("- item 1\n- item 2");
+      expect(html).toContain("