From 4db486c18a4be34cc04578cc80b1c5c26ffef62f Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:30:33 +0800 Subject: [PATCH] fix(mobile): align !file card preprocess with web parser + CLI escaped labels (MUL-4287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Howard final-review blocker: mobile's `!file[...]` preprocess didn't keep up with the CLI's file-card output, so agent-produced non-image files rendered nowhere on mobile. - `FILE_LINE_RE` used `[^\]]+` for the label, so the CLI's escaped-bracket output `!file[a\]b.pdf](url)` (cmd_attachment.go escapeMarkdownLabel) never matched — the line stayed literal AND `standaloneAttachments` still hid the fallback card (the URL is in `content`), so the file showed nowhere. - Align the matcher with web's `packages/ui/markdown/file-cards.ts`: label allows backslash-escaped metacharacters (ReDoS-safe class), and the URL is restricted to the same allowlist (site-relative /uploads + /api/attachments/ /download, plus absolute http(s)); disallowed schemes stay plain text. - Unescape the label to the real filename, then re-escape only the chars that would break a markdown LINK label (mobile emits `[📎 name](url)`, re-parsed by the renderer — unlike web's HTML data-filename), so a raw `]` never truncates the link text. No dedup change: once the inline `!file` renders, hiding the standalone card is correct. Added focused unit tests covering the escaped-label case, parens/ backslash unescape, the site-relative URL form, and disallowed-scheme rejection. Co-Authored-By: Claude Opus 4.8 Co-authored-by: multica-agent --- apps/mobile/lib/markdown/preprocess.test.ts | 55 +++++++++++++++++++++ apps/mobile/lib/markdown/preprocess.ts | 37 +++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 apps/mobile/lib/markdown/preprocess.test.ts diff --git a/apps/mobile/lib/markdown/preprocess.test.ts b/apps/mobile/lib/markdown/preprocess.test.ts new file mode 100644 index 0000000000..bed209e3a3 --- /dev/null +++ b/apps/mobile/lib/markdown/preprocess.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import { preprocessMobileMarkdown } from "./preprocess"; + +const UUID = "019f49e2-5b07-7970-beef-c0d537fb8c1d"; +const ABS_URL = `https://multica-app.copilothub.ai/api/attachments/${UUID}/download`; +const REL_URL = `/api/attachments/${UUID}/download`; + +describe("preprocessMobileMarkdown — !file file cards", () => { + it("matches the CLI's escaped-bracket label and keeps it markdown-safe", () => { + // CLI emits `a]b.pdf` escaped as `a\]b.pdf` (cmd_attachment.go + // escapeMarkdownLabel). The old regex stopped at the first `]` and left the + // line literal; now it is captured whole and re-emitted as a tappable link + // whose label stays escaped so the `]` doesn't truncate the link text. + const out = preprocessMobileMarkdown(`!file[a\\]b.pdf](${ABS_URL})`); + expect(out).toBe(`[📎 a\\]b.pdf](${ABS_URL})`); + }); + + it("unescapes non-breaking metacharacters (parens) in the displayed name", () => { + // Parens are legal inside a markdown link label, so they are unescaped for + // display and not re-escaped. + const out = preprocessMobileMarkdown(`!file[report\\(1\\).pdf](${ABS_URL})`); + expect(out).toBe(`[📎 report(1).pdf](${ABS_URL})`); + }); + + it("unescapes an escaped backslash in the label", () => { + const out = preprocessMobileMarkdown(`!file[a\\\\b.pdf](${ABS_URL})`); + expect(out).toBe(`[📎 a\\\\b.pdf](${ABS_URL})`); + }); + + it("renders a plain (unescaped) label", () => { + const out = preprocessMobileMarkdown(`!file[notes.txt](${ABS_URL})`); + expect(out).toBe(`[📎 notes.txt](${ABS_URL})`); + }); + + it("accepts the site-relative /api/attachments URL form (web parity)", () => { + const out = preprocessMobileMarkdown(`!file[a.pdf](${REL_URL})`); + expect(out).toBe(`[📎 a.pdf](${REL_URL})`); + }); + + it("leaves a disallowed-scheme URL as plain text (no out-of-band navigation)", () => { + const line = `!file[x.pdf](javascript:alert(1))`; + expect(preprocessMobileMarkdown(line)).toBe(line); + }); + + it("does not touch inline images (![...] is not a file card)", () => { + const line = `![chart.png](${ABS_URL})`; + expect(preprocessMobileMarkdown(line)).toBe(line); + }); + + it("only transforms the standalone file-card line, leaving surrounding text", () => { + const input = `here is the file\n\n!file[a\\]b.pdf](${ABS_URL})\n\nlet me know`; + const output = `here is the file\n\n[📎 a\\]b.pdf](${ABS_URL})\n\nlet me know`; + expect(preprocessMobileMarkdown(input)).toBe(output); + }); +}); diff --git a/apps/mobile/lib/markdown/preprocess.ts b/apps/mobile/lib/markdown/preprocess.ts index 6d14dad9ae..a78a0bc4bd 100644 --- a/apps/mobile/lib/markdown/preprocess.ts +++ b/apps/mobile/lib/markdown/preprocess.ts @@ -23,7 +23,39 @@ */ import { preprocessMentionShortcodes } from "@multica/core/markdown"; -const FILE_LINE_RE = /^!file\[([^\]]+)\]\((https?:\/\/[^)]+)\)$/; +// File-card line matcher, kept in sync with web's parser in +// `packages/ui/markdown/file-cards.ts` (NEW_FILE_CARD_RE + FILE_CARD_URL_PATTERN): +// +// - Label allows backslash-escaped metacharacters (`\[ \] \\ \( \)`) so a +// filename like `a]b.pdf` — which the CLI escapes to `a\]b.pdf` in its +// `!file[...]` output (see cmd_attachment.go escapeMarkdownLabel) — is +// captured whole. Backslash is excluded from the negated class so +// overlapping alternatives can't backtrack (ReDoS, web #4881). +// - URL is restricted to the same allowlist web accepts: site-relative +// `/uploads/...` and `/api/attachments//download`, plus absolute +// `http(s)://`. Anything else (`javascript:`, `data:`, `//host`, other +// `/api/…`) is left as plain text so a stored card can't become an +// out-of-band navigation. +const ATTACHMENT_UUID = + "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}"; +const FILE_CARD_URL = `/uploads/[^)]*|https?://[^)]+|/api/attachments/${ATTACHMENT_UUID}/download`; +const FILE_LINE_RE = new RegExp( + `^!file\\[((?:\\\\.|[^\\]\\\\])*)\\]\\((${FILE_CARD_URL})\\)$`, +); + +// Unescape the file-card label back to the real filename (mirrors web's +// `newMatch[1].replace(/\\([[\]\\()])/g, "$1")`). +function unescapeFileLabel(label: string): string { + return label.replace(/\\([[\]\\()])/g, "$1"); +} + +// Re-escape only the characters that would break a markdown LINK label, so the +// emitted `[📎 name](url)` stays valid markdown. Mobile's target is a link +// (re-parsed by marked / the enriched renderer), unlike web's HTML +// `data-filename` attribute — so a raw `]` must not truncate the link text. +function escapeLinkLabel(name: string): string { + return name.replace(/([\\[\]])/g, "\\$1"); +} function preprocessFileCards(input: string): string { return input @@ -31,7 +63,8 @@ function preprocessFileCards(input: string): string { .map((line) => { const m = line.trim().match(FILE_LINE_RE); if (!m) return line; - return `[📎 ${m[1]}](${m[2]})`; + const label = escapeLinkLabel(unescapeFileLabel(m[1]!)); + return `[📎 ${label}](${m[2]})`; }) .join("\n"); }