mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-02 01:45:52 +02:00
fix(mobile): align !file card preprocess with web parser + CLI escaped labels (MUL-4287)
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/ <UUID>/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 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
55
apps/mobile/lib/markdown/preprocess.test.ts
Normal file
55
apps/mobile/lib/markdown/preprocess.test.ts
Normal file
@@ -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 = ``;
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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/<UUID>/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");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user