From 60edca1938b8a5edea668d02f67caf082a1283aa Mon Sep 17 00:00:00 2001 From: J Date: Sat, 13 Jun 2026 15:36:22 +0800 Subject: [PATCH] fix: re-sign inline attachment media Co-authored-by: multica-agent --- packages/views/editor/attachment.test.tsx | 67 +++++++++++++++-- packages/views/editor/attachment.tsx | 88 +++++++++++++++-------- server/internal/handler/config.go | 2 +- server/internal/handler/config_test.go | 27 +++++++ 4 files changed, 147 insertions(+), 37 deletions(-) diff --git a/packages/views/editor/attachment.test.tsx b/packages/views/editor/attachment.test.tsx index 7620293b7..ac019eca8 100644 --- a/packages/views/editor/attachment.test.tsx +++ b/packages/views/editor/attachment.test.tsx @@ -1,17 +1,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { render, screen, fireEvent } from "@testing-library/react"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; import type { ReactElement, ReactNode } from "react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { Attachment as AttachmentRecord } from "@multica/core/types"; const { getAttachmentTextContentMock, + getAttachmentMock, getBaseUrlMock, downloadMock, openExternalMock, openByUrlMock, } = vi.hoisted(() => ({ getAttachmentTextContentMock: vi.fn(), + getAttachmentMock: vi.fn(), // Default: empty base URL so existing tests render site-relative URLs // through the proxy (i.e. exactly the way the web app behaves). The // absolutize-specific suite below overrides this to simulate Desktop / @@ -25,6 +27,7 @@ const { vi.mock("@multica/core/api", () => ({ api: { getAttachmentTextContent: getAttachmentTextContentMock, + getAttachment: getAttachmentMock, getBaseUrl: getBaseUrlMock, }, PreviewTooLargeError: class extends Error {}, @@ -159,6 +162,9 @@ beforeEach(() => { // the web app's same-origin proxy. Tests that simulate Desktop / mobile // webview override per-case via getBaseUrlMock.mockReturnValue(...). getBaseUrlMock.mockReturnValue(""); + getAttachmentMock.mockImplementation(async (id: string) => + makeRecord({ id }), + ); }); afterEach(() => { @@ -181,6 +187,7 @@ describe("Attachment — image dispatch", () => { expect(screen.getByTitle("View")).toBeTruthy(); expect(screen.getByTitle("Download")).toBeTruthy(); expect(screen.getByTitle("Copy link")).toBeTruthy(); + expect(getAttachmentMock).not.toHaveBeenCalled(); // Trash only shows in editable mode. expect(screen.queryByTitle("Delete")).toBeNull(); }); @@ -226,7 +233,7 @@ describe("Attachment — image dispatch", () => { expect(downloadMock).toHaveBeenCalledWith("att-1"); }); - it("renders the configured CDN URL when description markdown stores the stable API URL", () => { + it("does not choose raw CDN when the durable markdown URL is an attachment API endpoint", () => { configStore.setState({ cdnDomain: "cdn.example.test" }); const id = "11111111-2222-3333-4444-555555555555"; const markdownUrl = `https://multica-api.copilothub.ai/api/attachments/${id}/download`; @@ -254,22 +261,26 @@ describe("Attachment — image dispatch", () => { ); const img = document.querySelector("img"); - expect(img?.getAttribute("src")).toBe( - "https://cdn.example.test/uploads/ws/shot.png", - ); + expect(img?.getAttribute("src")).toBe(markdownUrl); + expect(img?.getAttribute("src")).not.toBe(att.url); }); - it("opens preview with the same resolved media URL when a reopened draft record has no download_url", () => { + it("re-signs a reopened draft record that has no download_url instead of rendering raw CDN", async () => { configStore.setState({ cdnDomain: "cdn.example.test" }); const id = "11111111-2222-3333-4444-555555555555"; const markdownUrl = `https://multica-api.copilothub.ai/api/attachments/${id}/download`; const mediaUrl = "https://cdn.example.test/uploads/ws/shot.png"; + const signedUrl = `${mediaUrl}?Policy=fresh&Signature=fresh&Key-Pair-Id=kp`; const att = makeRecord({ id, url: mediaUrl, markdown_url: markdownUrl, download_url: "", }); + getAttachmentMock.mockResolvedValueOnce({ + ...att, + download_url: signedUrl, + }); resolverState.attachments = [att]; renderWithQuery( @@ -283,12 +294,54 @@ describe("Attachment — image dispatch", () => { />, ); + const img = document.querySelector("img"); + expect(img?.getAttribute("src")).toBe(markdownUrl); + expect(img?.getAttribute("src")).not.toBe(mediaUrl); + + await waitFor(() => { + expect(document.querySelector("img")?.getAttribute("src")).toBe(signedUrl); + }); + expect(getAttachmentMock).toHaveBeenCalledWith(id); + }); + + it("opens preview with the same fresh signed URL after inline re-sign", async () => { + configStore.setState({ cdnDomain: "cdn.example.test" }); + const id = "11111111-2222-3333-4444-555555555555"; + const markdownUrl = `https://multica-api.copilothub.ai/api/attachments/${id}/download`; + const mediaUrl = "https://cdn.example.test/uploads/ws/shot.png"; + const signedUrl = `${mediaUrl}?Policy=fresh&Signature=fresh&Key-Pair-Id=kp`; + const att = makeRecord({ + id, + url: mediaUrl, + markdown_url: markdownUrl, + download_url: "", + }); + getAttachmentMock.mockResolvedValueOnce({ + ...att, + download_url: signedUrl, + }); + resolverState.attachments = [att]; + + renderWithQuery( + , + ); + + await waitFor(() => { + expect(document.querySelector("img")?.getAttribute("src")).toBe(signedUrl); + }); fireEvent.click(screen.getByTitle("View")); const imageSrcs = [...document.querySelectorAll("img")].map((img) => img.getAttribute("src"), ); - expect(imageSrcs).toEqual([mediaUrl, mediaUrl]); + expect(imageSrcs).toEqual([signedUrl, signedUrl]); expect(imageSrcs).not.toContain(""); }); diff --git a/packages/views/editor/attachment.tsx b/packages/views/editor/attachment.tsx index 5109ff420..920e0ae9e 100644 --- a/packages/views/editor/attachment.tsx +++ b/packages/views/editor/attachment.tsx @@ -35,6 +35,8 @@ import { copyText } from "@multica/ui/lib/clipboard"; import { api } from "@multica/core/api"; import { useConfigStore } from "@multica/core/config"; import type { Attachment as AttachmentRecord } from "@multica/core/types"; +import { attachmentIdFromDownloadURL } from "@multica/core/types/attachment-url"; +import { useQuery } from "@tanstack/react-query"; import { useT } from "../i18n"; import { useAttachmentDownloadResolver } from "./attachment-download-context"; import { useAttachmentPreview } from "./attachment-preview-modal"; @@ -243,18 +245,27 @@ function pickInlineMediaURL( cdnDomain: string, ): string { const dl = record.download_url ?? ""; - if ( - /^https?:\/\//i.test(dl) && - /[?&](Signature|X-Amz-Signature|Key-Pair-Id|Expires|X-Amz-Expires)=/i.test(dl) - ) { + if (hasSignedHTTPURL(dl)) { return dl; } - if (storageURLMatchesCdnDomain(record.url, cdnDomain)) return record.url; + if ( + storageURLMatchesCdnDomain(record.url, cdnDomain) && + !attachmentIdFromDownloadURL(record.markdown_url) + ) { + return record.url; + } if (record.markdown_url) return record.markdown_url; if (record.url) return record.url; return fallback; } +function hasSignedHTTPURL(rawURL: string): boolean { + return ( + /^https?:\/\//i.test(rawURL) && + /[?&](Signature|X-Amz-Signature|Key-Pair-Id|Expires|X-Amz-Expires)=/i.test(rawURL) + ); +} + function storageURLMatchesCdnDomain(rawURL: string, cdnDomain: string): boolean { const expected = normalizeHost(cdnDomain); if (!rawURL || !expected) return false; @@ -302,51 +313,70 @@ export function Attachment({ const preview = useAttachmentPreview(); const state = normalize(attachment, resolveAttachment, cdnDomain); + const shouldRefreshInlineMedia = Boolean( + state.record?.id && + !hasSignedHTTPURL(state.record.download_url ?? "") && + attachmentIdFromDownloadURL(state.record.markdown_url || state.url), + ); + const { data: refreshedRecord } = useQuery({ + queryKey: ["attachments", state.record?.id, "inline-media"], + queryFn: () => api.getAttachment(state.record!.id), + enabled: shouldRefreshInlineMedia, + staleTime: 25 * 60 * 1000, + gcTime: 30 * 60 * 1000, + }); + const renderState = refreshedRecord + ? normalize( + { kind: "record", attachment: refreshedRecord }, + resolveAttachment, + cdnDomain, + ) + : state; const forceKind = attachment.kind === "url" ? attachment.forceKind : undefined; const kind = forceKind ?? - (state.filename || state.contentType - ? getPreviewKind(state.contentType, state.filename) + (renderState.filename || renderState.contentType + ? getPreviewKind(renderState.contentType, renderState.filename) : null); const openPreview = () => { - if (state.record) { + if (renderState.record) { preview.tryOpen({ kind: "full", attachment: { - ...state.record, - download_url: state.url || state.record.download_url, + ...renderState.record, + download_url: renderState.url || renderState.record.download_url, }, }); return; } - if (state.url) { + if (renderState.url) { preview.tryOpen({ kind: "url", - url: state.url, - filename: state.filename, + url: renderState.url, + filename: renderState.filename, }); } }; const handleDownload = () => { - if (state.attachmentId) { - download(state.attachmentId); + if (renderState.attachmentId) { + download(renderState.attachmentId); return; } - if (state.url) openByUrl(state.url); + if (renderState.url) openByUrl(renderState.url); }; if (kind === "image") { return ( <>