From ebd0248be25a1675bce29b580b046454074c344e Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 6 Jul 2026 08:43:51 +0800 Subject: [PATCH 01/15] MUL-4016: fix mention tokenizer stacktrace backtracking (#4889) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * MUL-4016: fix mention tokenizer stacktrace backtracking Co-authored-by: multica-agent * fix(editor): de-ambiguate escaped-label regexes to kill ReDoS (MUL-4016) The mention/slash/file-card label regexes used `(?:\\.|[^\]])` where both alternatives can consume a backslash. On an unterminated match, each `\x` run is enumerated 2^n ways — pasting a Java stacktrace (`\~\[...\]`) or a crafted ~50-char string freezes the main thread for seconds (GitHub #4881). Exclude backslash from the char class (`[^\]\\]`) so a backslash can only be consumed by `\\.`. The alternatives become disjoint and matching is linear; legal escaped-bracket labels like `David\[TF\]` still parse unchanged. Fixed in all four sites that shared the pattern: - mention-extension.ts tokenize() - slash-command-extension.ts start() + tokenize() - file-card.tsx FILE_CARD_MARKDOWN_RE - packages/ui/markdown/file-cards.ts NEW_FILE_CARD_RE (runs on every read-only comment/description render, not just the editor) Adds adversarial regression tests (repeated `\a` + missing closing bracket) that fail in ~10-40s against the old regexes and pass in <1ms after the fix. Builds on #4889's marker-first mention start(). Co-Authored-By: Claude Fable 5 Co-authored-by: multica-agent * fix(editor): escape backslash in mention/slash labels for round-trip (MUL-4016) Follow-up to the de-ambiguation fix, addressing Howard's PR review. The linear tokenizer now treats "\" as an escape lead (\\.), so a label whose serialized form contains a bare "\" adjacent to the closing "]" no longer parses back — the "\]" is consumed as an escaped bracket and swallows the boundary. The old ambiguous regex tolerated this by chance; the de-ambiguation exposes it. mention/slash renderMarkdown escaped only [ and ], not \. Switch both to the shared escapeMarkdownLabel() (escapes [ ] \ ( )) and mirror it on parse with replace(/\\([[\]\\()])/g, "$1"), matching what file-card already does. This also converges the three tokenizers on one escape contract. file-card was already correct and is unchanged. Adds parameterized round-trip tests for labels containing "\" / "\]" / parens (e.g. "A\\", "ends\\", "a\\]b"); these fail on the old serializer and pass now. Co-Authored-By: Claude Fable 5 Co-authored-by: multica-agent --------- Co-authored-by: multica-agent Co-authored-by: Claude Fable 5 --- packages/ui/markdown/file-cards.ts | 9 ++- .../extensions/file-card-markdown.test.ts | 27 ++++++++ .../views/editor/extensions/file-card.tsx | 5 +- .../extensions/mention-extension.test.ts | 59 ++++++++++++++++ .../editor/extensions/mention-extension.ts | 69 +++++++++++++++---- .../slash-command-extension.test.ts | 35 ++++++++++ .../extensions/slash-command-extension.ts | 19 +++-- 7 files changed, 201 insertions(+), 22 deletions(-) diff --git a/packages/ui/markdown/file-cards.ts b/packages/ui/markdown/file-cards.ts index 35debcc8168..3be14a5b1b8 100644 --- a/packages/ui/markdown/file-cards.ts +++ b/packages/ui/markdown/file-cards.ts @@ -47,9 +47,14 @@ export function isAllowedFileCardHref(href: string): boolean { ) } -/** New syntax: !file[name](url) — unambiguous, no hostname matching needed. */ +/** + * New syntax: !file[name](url) — unambiguous, no hostname matching needed. + * Backslash is excluded from the label char class so "\x" runs can only be + * consumed by \\. — overlapping alternatives backtrack in 2^n ways (ReDoS, + * GitHub #4881). This runs on every comment/description render. + */ const NEW_FILE_CARD_RE = new RegExp( - `^!file\\[((?:\\\\.|[^\\]])*)\\]\\((${FILE_CARD_URL_PATTERN.source})\\)$`, + `^!file\\[((?:\\\\.|[^\\]\\\\])*)\\]\\((${FILE_CARD_URL_PATTERN.source})\\)$`, ) /** Legacy syntax: [name](cdnUrl) on its own line — matched by CDN hostname. */ diff --git a/packages/views/editor/extensions/file-card-markdown.test.ts b/packages/views/editor/extensions/file-card-markdown.test.ts index 348b6250be6..17a20c93313 100644 --- a/packages/views/editor/extensions/file-card-markdown.test.ts +++ b/packages/views/editor/extensions/file-card-markdown.test.ts @@ -61,6 +61,20 @@ describe("file-card tokenizer", () => { expect(token).toBeDefined(); expect(token!.attributes.filename).toBe("readme.md"); }); + + it("rejects an unterminated file card with escape-pair runs in linear time", () => { + // Each "\a" pair is ambiguous under (?:\\.|[^\]]) — the pre-fix regex + // enumerates 2^28 backtrack paths (~10s) before failing. The disjoint + // char class must fail fast instead. + const src = `!file[${"\\a".repeat(28)}](/uploads/x`; + + const t0 = performance.now(); + const token = tokenize(src); + const elapsed = performance.now() - t0; + + expect(token).toBeUndefined(); + expect(elapsed).toBeLessThan(100); + }); }); // --------------------------------------------------------------------------- @@ -81,4 +95,17 @@ describe("preprocessFileCards", () => { expect(result).toContain('data-type="fileCard"'); expect(result).toContain('data-filename="readme.md"'); }); + + it("rejects an unterminated file card with escape-pair runs in linear time", () => { + // This path runs on every read-only comment/description render, so a + // backtracking label regex here is reachable without opening the editor. + const input = `!file[${"\\a".repeat(28)}](/uploads/x`; + + const t0 = performance.now(); + const result = preprocessFileCards(input, "cdn.example.com"); + const elapsed = performance.now() - t0; + + expect(result).toBe(input); + expect(elapsed).toBeLessThan(100); + }); }); diff --git a/packages/views/editor/extensions/file-card.tsx b/packages/views/editor/extensions/file-card.tsx index acdf7b398e3..9e554f81b77 100644 --- a/packages/views/editor/extensions/file-card.tsx +++ b/packages/views/editor/extensions/file-card.tsx @@ -21,8 +21,11 @@ import { FILE_CARD_URL_PATTERN } from "@multica/ui/markdown"; import { escapeMarkdownLabel } from "../utils/escape-markdown-label"; import { Attachment } from "../attachment"; +// Backslash is excluded from the label char class so "\x" runs can only be +// consumed by \\. — overlapping alternatives backtrack in 2^n ways (ReDoS, +// GitHub #4881). const FILE_CARD_MARKDOWN_RE = new RegExp( - `^!file\\[((?:\\\\.|[^\\]])*)\\]\\((${FILE_CARD_URL_PATTERN.source})\\)`, + `^!file\\[((?:\\\\.|[^\\]\\\\])*)\\]\\((${FILE_CARD_URL_PATTERN.source})\\)`, ); diff --git a/packages/views/editor/extensions/mention-extension.test.ts b/packages/views/editor/extensions/mention-extension.test.ts index 864655e5a95..478d0942405 100644 --- a/packages/views/editor/extensions/mention-extension.test.ts +++ b/packages/views/editor/extensions/mention-extension.test.ts @@ -14,6 +14,9 @@ const renderMarkdown = BaseMentionExtension.config.renderMarkdown as ( node: { attrs: Record }, ) => string; +const JAVA_STACKTRACE_SOURCE_MARKER = + " at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) \\~\\[spring-webmvc-5.3.39.jar!/:5.3.39\\]\n"; + function tokenize(src: string) { const start = startFn(src); if (start === -1) return undefined; @@ -42,6 +45,23 @@ describe("mention tokenizer", () => { expect(token!.attributes.type).toBe("agent"); }); + it.each(["A\\", "ends\\", "a\\]b", "f(x)", "back\\slash"])( + "round-trips a label containing backslash/parens: %j", + (label) => { + // The linear tokenizer treats "\" as an escape lead, so renderMarkdown + // must escape "\" too — otherwise a trailing "\" swallows the closing "]" + // and the mention fails to parse back (regression guard for the + // de-ambiguation fix). + const md = renderMarkdown({ + attrs: { id: "aaa-bbb", label, type: "member" }, + }); + const token = tokenize(md); + expect(token).toBeDefined(); + expect(token!.attributes.label).toBe(label); + expect(token!.attributes.id).toBe("aaa-bbb"); + }, + ); + it("does not match an ordinary Markdown link before a mention", () => { const src = "Check [docs](https://example.com) - [@User](mention://agent/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa)"; @@ -89,4 +109,43 @@ describe("mention tokenizer", () => { expect(token!.attributes.type).toBe("issue"); expect(token!.attributes.id).toBe("aaa-bbb"); }); + + it("does not start at escaped Java stacktrace source markers before a real mention", () => { + const prefix = JAVA_STACKTRACE_SOURCE_MARKER.repeat(3); + const src = `${prefix}[@Alice](mention://member/aaa-bbb)`; + + const start = startFn(src); + expect(start).toBe(prefix.length); + + const token = tokenizeFn(src.slice(start)); + expect(token).toBeDefined(); + expect(token!.attributes.label).toBe("Alice"); + expect(token!.attributes.type).toBe("member"); + expect(token!.attributes.id).toBe("aaa-bbb"); + }); + + it("keeps escaped Java stacktrace source marker detection fast when no mention exists", () => { + const src = JAVA_STACKTRACE_SOURCE_MARKER.repeat(11); + + const t0 = performance.now(); + const start = startFn(src); + const elapsed = performance.now() - t0; + + expect(start).toBe(-1); + expect(elapsed).toBeLessThan(50); + }); + + it("rejects an unterminated mention with escape-pair runs in linear time", () => { + // Each "\a" pair is ambiguous under (?:\\.|[^\]]) — the pre-fix regex + // enumerates 2^28 backtrack paths (~10s) before failing. The disjoint + // char class must fail fast instead. + const src = `[@${"\\a".repeat(28)}](mention://member/abc`; + + const t0 = performance.now(); + const token = tokenizeFn(src); + const elapsed = performance.now() - t0; + + expect(token).toBeUndefined(); + expect(elapsed).toBeLessThan(100); + }); }); diff --git a/packages/views/editor/extensions/mention-extension.ts b/packages/views/editor/extensions/mention-extension.ts index f285ab03440..40f7519aec1 100644 --- a/packages/views/editor/extensions/mention-extension.ts +++ b/packages/views/editor/extensions/mention-extension.ts @@ -2,6 +2,44 @@ import Mention from "@tiptap/extension-mention"; import { mergeAttributes } from "@tiptap/core"; import { ReactNodeViewRenderer } from "@tiptap/react"; import { MentionView } from "./mention-view"; +import { escapeMarkdownLabel } from "../utils/escape-markdown-label"; + +const MENTION_LINK_MARKER = "](mention://"; + +function isEscaped(text: string, index: number): boolean { + let slashCount = 0; + for (let i = index - 1; i >= 0 && text[i] === "\\"; i--) { + slashCount++; + } + return slashCount % 2 === 1; +} + +function findUnescapedMentionMarker(src: string, from = 0): number { + let marker = src.indexOf(MENTION_LINK_MARKER, from); + + while (marker !== -1) { + if (!isEscaped(src, marker)) return marker; + marker = src.indexOf(MENTION_LINK_MARKER, marker + MENTION_LINK_MARKER.length); + } + + return -1; +} + +function findMentionStart(src: string): number { + let marker = findUnescapedMentionMarker(src); + + while (marker !== -1) { + for (let i = marker - 1; i >= 0; i--) { + const char = src[i]; + if (char === "\n" || char === "\r") break; + if (char === "[" && !isEscaped(src, i)) return i; + } + + marker = findUnescapedMentionMarker(src, marker + MENTION_LINK_MARKER.length); + } + + return -1; +} export const BaseMentionExtension = Mention.extend({ addNodeView() { @@ -39,21 +77,24 @@ export const BaseMentionExtension = Mention.extend({ name: "mention", level: "inline" as const, start(src: string) { - // Accept escaped brackets (\\[ \\]) and non-] chars in the label. - // This prevents matching ordinary Markdown links like [docs](url) - // that appear before a mention on the same line. - return src.search(/\[@?(?:\\.|[^\]])+\]\(mention:\/\//); + // Anchor on Multica's mention href first. Scanning forward from every + // "[" backtracks badly on escaped stacktrace markers like \~\[...\]. + return findMentionStart(src); }, tokenize(src: string) { - // Label accepts escaped chars (\\[ \\]) or any non-] character. - // This prevents the label from crossing a ]( Markdown link boundary - // while still supporting bracket-containing names like "David\[TF\]". + // Label accepts escaped chars (\\[ \\]) or any non-] non-backslash + // character. Excluding backslash from the char class keeps the two + // alternatives disjoint — otherwise "\x" runs backtrack in 2^n ways + // (ReDoS, GitHub #4881) — while still supporting bracket-containing + // names like "David\[TF\]". const match = src.match( - /^\[@?((?:\\.|[^\]])+)\]\(mention:\/\/(\w+)\/([^)]+)\)/, + /^\[@?((?:\\.|[^\]\\])+)\]\(mention:\/\/(\w+)\/([^)]+)\)/, ); if (!match) return undefined; - // Unescape backslash-escaped brackets that renderMarkdown may produce. - const rawLabel = match[1]?.replace(/\\\[/g, "[").replace(/\\\]/g, "]"); + // Reverse escapeMarkdownLabel: unescape \[ \] \\ \( \) that + // renderMarkdown produced. Must mirror the escaped set exactly, or a + // label containing "\" fails to round-trip through the linear tokenizer. + const rawLabel = match[1]?.replace(/\\([[\]\\()])/g, "$1"); return { type: "mention", raw: match[0], @@ -67,9 +108,11 @@ export const BaseMentionExtension = Mention.extend({ renderMarkdown: (node: any) => { const { id, label, type = "member" } = node.attrs || {}; const prefix = type === "issue" || type === "project" ? "" : "@"; - // Escape square brackets in the label so the markdown link syntax - // is not broken when the name contains [ or ] (e.g. "David[TF]"). - const safeLabel = (label ?? id).replace(/\[/g, "\\[").replace(/\]/g, "\\]"); + // Escape [ ] \ ( ) in the label so the markdown link syntax is not broken + // and the label survives the linear tokenizer (which now treats "\" as an + // escape lead, not an ordinary char). Must stay in sync with the unescape + // in tokenize() above. Shared with file-card/slash via escapeMarkdownLabel. + const safeLabel = escapeMarkdownLabel(label ?? id); return `[${prefix}${safeLabel}](mention://${type}/${id})`; }, }); diff --git a/packages/views/editor/extensions/slash-command-extension.test.ts b/packages/views/editor/extensions/slash-command-extension.test.ts index e54660ada5f..a83c5b5d006 100644 --- a/packages/views/editor/extensions/slash-command-extension.test.ts +++ b/packages/views/editor/extensions/slash-command-extension.test.ts @@ -79,6 +79,16 @@ describe("slash command tokenizer", () => { expect(tokenize(md)?.attributes.label).toBe("deploy[prod]"); }); + it.each(["A\\", "ends\\", "a\\]b", "f(x)", "back\\slash"])( + "round-trips a label containing backslash/parens: %j", + (label) => { + // renderMarkdown must escape "\" so a trailing "\" does not swallow the + // closing "]" under the linear tokenizer (regression guard). + const md = renderMarkdown({ attrs: { id: "skill-1", label } }); + expect(tokenize(md)?.attributes.label).toBe(label); + }, + ); + it("does not match ordinary markdown links", () => { expect(tokenize("[docs](https://example.com)")).toBeUndefined(); }); @@ -86,4 +96,29 @@ describe("slash command tokenizer", () => { it("does not match slash action links", () => { expect(tokenize("[/deploy](slash://action/deploy)")).toBeUndefined(); }); + + it("rejects an unterminated slash link with escape-pair runs in linear time", () => { + // Each "\a" pair is ambiguous under (?:\\.|[^\]]) — the pre-fix regex + // enumerates 2^28 backtrack paths (~10s) before failing. The disjoint + // char class must fail fast instead. + const src = `[/${"\\a".repeat(28)}](slash://skill/abc`; + + const t0 = performance.now(); + const token = tokenizeFn(src); + const elapsed = performance.now() - t0; + + expect(token).toBeUndefined(); + expect(elapsed).toBeLessThan(100); + }); + + it("returns -1 fast from start() when escape-pair runs precede no slash link", () => { + const src = `[/${"\\a".repeat(28)}] plain text, no slash link`; + + const t0 = performance.now(); + const start = startFn(src); + const elapsed = performance.now() - t0; + + expect(start).toBe(-1); + expect(elapsed).toBeLessThan(100); + }); }); diff --git a/packages/views/editor/extensions/slash-command-extension.ts b/packages/views/editor/extensions/slash-command-extension.ts index eb6147a6ece..b232d1892af 100644 --- a/packages/views/editor/extensions/slash-command-extension.ts +++ b/packages/views/editor/extensions/slash-command-extension.ts @@ -3,6 +3,7 @@ import { mergeAttributes } from "@tiptap/core"; import { ReactNodeViewRenderer } from "@tiptap/react"; import { SlashCommandView } from "./slash-command-view"; import { formatSlashCommandLabel } from "./slash-command-utils"; +import { escapeMarkdownLabel } from "../utils/escape-markdown-label"; export const SlashCommandExtension = Mention.extend({ name: "slashCommand", @@ -35,14 +36,20 @@ export const SlashCommandExtension = Mention.extend({ name: "slashCommand", level: "inline" as const, start(src: string) { - return src.search(/\[\/(?:\\.|[^\]])+\]\(slash:\/\/skill\//); + // Backslash is excluded from the char class so "\x" runs can only be + // consumed by \\. — overlapping alternatives backtrack in 2^n ways + // (ReDoS, GitHub #4881). + return src.search(/\[\/(?:\\.|[^\]\\])+\]\(slash:\/\/skill\//); }, tokenize(src: string) { const match = src.match( - /^\[\/((?:\\.|[^\]])+)\]\(slash:\/\/skill\/([^)]+)\)/, + /^\[\/((?:\\.|[^\]\\])+)\]\(slash:\/\/skill\/([^)]+)\)/, ); if (!match) return undefined; - const rawLabel = match[1]?.replace(/\\\[/g, "[").replace(/\\\]/g, "]"); + // Reverse escapeMarkdownLabel: unescape \[ \] \\ \( \). Must mirror the + // escaped set exactly, or a label containing "\" fails to round-trip + // through the linear tokenizer. + const rawLabel = match[1]?.replace(/\\([[\]\\()])/g, "$1"); return { type: "slashCommand", raw: match[0], @@ -57,9 +64,9 @@ export const SlashCommandExtension = Mention.extend({ renderMarkdown: (node: any) => { const { id, label } = node.attrs || {}; - const safeLabel = formatSlashCommandLabel(label) - .replace(/\[/g, "\\[") - .replace(/\]/g, "\\]"); + // Escape [ ] \ ( ) so the label survives the linear tokenizer; must stay in + // sync with the unescape in tokenize() above. + const safeLabel = escapeMarkdownLabel(formatSlashCommandLabel(label)); return `[/${safeLabel}](slash://skill/${id})`; }, }); From c84a939c8324f16db554e8eb4bec21be0e138cc9 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 6 Jul 2026 09:43:03 +0800 Subject: [PATCH 02/15] fix(views): enable multi-file selection on all attachment upload buttons (#4962) The FileUploadButton component already fans out per-file onSelect callbacks and every editor surface already handles N concurrent uploads (drag-drop and paste were multi-file all along), but five call sites never passed the `multiple` prop, so the OS file dialog capped picks at one file: chat composer, create-issue modal, quick-create modal, issue description, and feedback modal. Fixes MUL-4074. Co-authored-by: Claude Fable 5 Co-authored-by: multica-agent --- packages/views/chat/components/chat-input.tsx | 1 + packages/views/issues/components/issue-detail.tsx | 1 + packages/views/modals/create-issue.tsx | 1 + packages/views/modals/feedback.tsx | 1 + packages/views/modals/quick-create-issue.tsx | 1 + 5 files changed, 5 insertions(+) diff --git a/packages/views/chat/components/chat-input.tsx b/packages/views/chat/components/chat-input.tsx index 43c9f77863a..332b50e34b1 100644 --- a/packages/views/chat/components/chat-input.tsx +++ b/packages/views/chat/components/chat-input.tsx @@ -398,6 +398,7 @@ export function ChatInput({ {uploadEnabled && ( editorRef.current?.uploadFile(file)} /> )} diff --git a/packages/views/issues/components/issue-detail.tsx b/packages/views/issues/components/issue-detail.tsx index 20bd8403055..880db5af32f 100644 --- a/packages/views/issues/components/issue-detail.tsx +++ b/packages/views/issues/components/issue-detail.tsx @@ -1985,6 +1985,7 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr /> descEditorRef.current?.uploadFile(file)} /> diff --git a/packages/views/modals/create-issue.tsx b/packages/views/modals/create-issue.tsx index a3e20aba8f4..772806f8915 100644 --- a/packages/views/modals/create-issue.tsx +++ b/packages/views/modals/create-issue.tsx @@ -864,6 +864,7 @@ export function ManualCreatePanel({
descEditorRef.current?.uploadFile(file)} />
diff --git a/packages/views/modals/feedback.tsx b/packages/views/modals/feedback.tsx index 028cf3954f8..7850e416790 100644 --- a/packages/views/modals/feedback.tsx +++ b/packages/views/modals/feedback.tsx @@ -164,6 +164,7 @@ export function FeedbackModal({
editorRef.current?.uploadFile(file)} />