From 417d1a403ea6af3bb8f4a4e48ab0dfed251af236 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:37:37 +0800 Subject: [PATCH] fix(markdown): allow attachment download file-card hrefs Co-authored-by: multica-agent --- packages/ui/markdown/file-cards.ts | 18 +++- packages/views/editor/attachment.test.tsx | 25 +++++ packages/views/editor/file-card-href.test.ts | 102 +++++++++++------- .../views/editor/readonly-content.test.tsx | 25 +++++ .../issues/components/comment-card.test.tsx | 24 +++++ 5 files changed, 155 insertions(+), 39 deletions(-) diff --git a/packages/ui/markdown/file-cards.ts b/packages/ui/markdown/file-cards.ts index ed4863389..35debcc81 100644 --- a/packages/ui/markdown/file-cards.ts +++ b/packages/ui/markdown/file-cards.ts @@ -15,22 +15,36 @@ const IMAGE_EXTS = /\.(png|jpe?g|gif|webp|svg|ico|bmp|tiff?)$/i +// Keep in sync with UUID_RE in packages/core/types/attachment-url.ts. +const ATTACHMENT_UUID_SOURCE = + '[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 ATTACHMENT_DOWNLOAD_URL_SOURCE = `/api/attachments/${ATTACHMENT_UUID_SOURCE}/download` +const ATTACHMENT_DOWNLOAD_URL_RE = new RegExp( + `^${ATTACHMENT_DOWNLOAD_URL_SOURCE}$`, +) + /** * URL alternation accepted inside `!file[name](url)` markdown. * * Restricted to: * - `/uploads/...` site-relative paths (LocalStorage backend with no LOCAL_UPLOAD_BASE_URL) + * - `/api/attachments//download` site-relative attachment downloads * - `http(s)://...` absolute URLs (S3 / CloudFront / hosted) * * Anything else — `javascript:`, `data:`, protocol-relative `//host/x`, other * APIs `/api/…`, path-traversal `/../…` — is rejected so a stored file-card * cannot be turned into an out-of-band navigation. */ -export const FILE_CARD_URL_PATTERN = /\/uploads\/[^)]*|https?:\/\/[^)]+/ +export const FILE_CARD_URL_PATTERN = new RegExp( + `/uploads/[^)]*|https?:\\/\\/[^)]+|${ATTACHMENT_DOWNLOAD_URL_SOURCE}`, +) /** Prefix test applied by renderers to validate `data-href` before opening it. */ export function isAllowedFileCardHref(href: string): boolean { - return /^(https?:\/\/|\/uploads\/)/i.test(href) + return ( + /^(https?:\/\/|\/uploads\/)/i.test(href) || + ATTACHMENT_DOWNLOAD_URL_RE.test(href) + ) } /** New syntax: !file[name](url) — unambiguous, no hostname matching needed. */ diff --git a/packages/views/editor/attachment.test.tsx b/packages/views/editor/attachment.test.tsx index 5601775ac..cfc1fe95d 100644 --- a/packages/views/editor/attachment.test.tsx +++ b/packages/views/editor/attachment.test.tsx @@ -564,6 +564,31 @@ describe("Attachment — file-card dispatch", () => { expect(document.querySelector("img")).toBeNull(); }); + it("url-only stable attachment download file-card resolves to record and downloads by id", () => { + const id = "11111111-2222-3333-4444-555555555555"; + const href = `/api/attachments/${id}/download`; + resolverState.attachments = [ + makeRecord({ + id, + filename: "manual.pdf", + content_type: "application/pdf", + url: "/uploads/manual.pdf", + markdown_url: href, + download_url: href, + }), + ]; + + renderWithQuery( + , + ); + + expect(screen.getByText("manual.pdf")).toBeTruthy(); + fireEvent.mouseDown(screen.getByTitle("Download")); + expect(downloadMock).toHaveBeenCalledWith(id); + }); + it("uploading file-card surfaces the uploading template, no Preview/Download", () => { renderWithQuery( { - it.each([ - ["/uploads/ok", true], - ["/uploads/workspaces/abc/file.png", true], - ["https://cdn.example.com/x", true], - ["http://localhost:8080/uploads/x.png", true], - ["HTTPS://CDN.EXAMPLE.COM/x", true], - ])("accepts %s", (href, expected) => { - expect(isAllowedFileCardHref(href)).toBe(expected); + it.each(allowedClickHrefs)("accepts %s", (href) => { + expect(isAllowedFileCardHref(href)).toBe(true); }); - it.each([ - ["javascript:alert(1)", false], - ["JavaScript:alert(1)", false], - ["data:text/html,xss", false], - ["//evil.com/x", false], - ["/../api/x", false], - ["/api/x", false], - ["/api/internal/x", false], - ["", false], - ["ftp://example.com/x", false], - ["uploads/x", false], - ])("rejects %s", (href, expected) => { - expect(isAllowedFileCardHref(href)).toBe(expected); + it.each(rejectedFileCardHrefs)("rejects %s", (href) => { + expect(isAllowedFileCardHref(href)).toBe(false); }); }); @@ -39,25 +62,20 @@ describe("FILE_CARD_URL_PATTERN", () => { `^!file\\[([^\\]]*)\\]\\((${FILE_CARD_URL_PATTERN.source})\\)$`, ); - it.each([ - "!file[doc.md](/uploads/x.md)", - "!file[name](/uploads/workspaces/abc/019e.md)", - "!file[doc.md](https://cdn.example.com/x.md)", - "!file[doc.md](http://localhost:8080/uploads/x.md)", - ])("parses %s", (input) => { - expect(parser.test(input)).toBe(true); + it.each(parsedAllowedFileCardHrefs)("parses %s", (href) => { + expect(parser.test(`!file[doc.md](${href})`)).toBe(true); + }); + + it.each(rejectedFileCardHrefs)("does not parse %s", (href) => { + expect(parser.test(`!file[doc.md](${href})`)).toBe(false); }); it.each([ - "!file[evil.txt](javascript:alert(1))", - "!file[evil.txt](data:text/html,xss)", - "!file[evil.txt](//evil.com/x)", - "!file[evil.txt](/../api/x)", - "!file[evil.txt](/api/x)", - "!file[doc.md](uploads/x.md)", - "!file[doc.md](ftp://example.com/x)", - ])("does not parse %s", (input) => { - expect(parser.test(input)).toBe(false); + ...parsedAllowedFileCardHrefs.map((href) => [href, true] as const), + ...rejectedFileCardHrefs.map((href) => [href, false] as const), + ])("matches the click gate for %s", (href, expected) => { + expect(parser.test(`!file[doc.md](${href})`)).toBe(expected); + expect(isAllowedFileCardHref(href)).toBe(expected); }); }); @@ -71,6 +89,16 @@ describe("preprocessFileCards (integration)", () => { expect(out).toContain('data-filename="doc.md"'); }); + it("converts !file[…](attachment download URL) into a file-card div", () => { + const out = preprocessFileCards( + `!file[doc.md](${ATTACHMENT_DOWNLOAD})`, + cdn, + ); + expect(out).toContain('data-type="fileCard"'); + expect(out).toContain(`data-href="${ATTACHMENT_DOWNLOAD}"`); + expect(out).toContain('data-filename="doc.md"'); + }); + it("leaves a protocol-relative href untouched (not parsed as file-card)", () => { const out = preprocessFileCards("!file[evil.txt](//evil.com/x)", cdn); expect(out).not.toContain('data-type="fileCard"'); diff --git a/packages/views/editor/readonly-content.test.tsx b/packages/views/editor/readonly-content.test.tsx index 5cc649d5e..0a00706de 100644 --- a/packages/views/editor/readonly-content.test.tsx +++ b/packages/views/editor/readonly-content.test.tsx @@ -470,6 +470,31 @@ describe("ReadonlyContent file-card → AttachmentBlock HTML routing", () => { //

row. HtmlAttachmentPreview replaces it entirely. expect(queryByText("report.html")).toBeNull(); }); + + it("renders a stable attachment download URL as file-card chrome", () => { + const id = "11111111-2222-3333-4444-555555555555"; + const href = `/api/attachments/${id}/download`; + const attachment = { + id, + url: "/uploads/report.pdf", + filename: "report.pdf", + content_type: "application/pdf", + size_bytes: 1024, + markdown_url: href, + download_url: href, + } as any; + + const { container, getByText } = renderWithQuery( + , + ); + + expect(getByText("report.pdf")).toBeTruthy(); + expect(container.querySelector("iframe")).toBeNull(); + expect(container.querySelector("img")).toBeNull(); + }); }); describe("ReadonlyContent slash command rendering", () => { diff --git a/packages/views/issues/components/comment-card.test.tsx b/packages/views/issues/components/comment-card.test.tsx index a31a9327d..5df8d8a77 100644 --- a/packages/views/issues/components/comment-card.test.tsx +++ b/packages/views/issues/components/comment-card.test.tsx @@ -86,3 +86,27 @@ describe("AttachmentList — standalone HTML attachment routes through Attachmen expect(screen.queryByText("report.html")).toBeNull(); }); }); + +describe("AttachmentList — inline attachment filtering", () => { + it("does not render a bottom attachment row when the body already has the stable file-card URL", () => { + const id = "11111111-2222-3333-4444-555555555555"; + const href = `/api/attachments/${id}/download`; + const attachment = { + id, + url: "/uploads/report.pdf", + filename: "report.pdf", + content_type: "application/pdf", + size_bytes: 1024, + } as any; + + const { container } = renderWithQuery( + , + ); + + expect(screen.queryByText("report.pdf")).toBeNull(); + expect(container.firstChild).toBeNull(); + }); +});