From 57701b2239d51995e2a84f311aa64e489bb340b2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 24 Aug 2026 09:44:14 -0700 Subject: [PATCH] fix(files): normalize encoded embedded ids --- .../app/api/files/export/[id]/route.test.ts | 1 - apps/sim/app/api/files/export/[id]/route.ts | 3 ++- .../files/public/[token]/inline/route.test.ts | 9 +++++++++ .../api/files/public/[token]/inline/route.ts | 5 ++++- .../rich-markdown-editor/image.test.ts | 12 ++++++++++++ apps/sim/hooks/use-file-content-source.tsx | 3 ++- .../tools/server/files/embedded-image-refs.ts | 3 ++- .../lib/uploads/server/embedded-image-refs.ts | 18 ------------------ .../uploads/utils/embedded-image-ref.test.ts | 17 ++++++++++++++++- .../lib/uploads/utils/embedded-image-ref.ts | 13 +++++++++++++ 10 files changed, 60 insertions(+), 24 deletions(-) diff --git a/apps/sim/app/api/files/export/[id]/route.test.ts b/apps/sim/app/api/files/export/[id]/route.test.ts index 8b6e7b53d24..1d83cdd02f9 100644 --- a/apps/sim/app/api/files/export/[id]/route.test.ts +++ b/apps/sim/app/api/files/export/[id]/route.test.ts @@ -33,7 +33,6 @@ vi.mock('@/app/api/files/authorization', () => ({ verifyFileAccess: mockVerifyFi vi.mock('@/lib/uploads/core/storage-service', () => ({ downloadFile: mockDownloadFile })) vi.mock('@/lib/uploads/server/embedded-image-refs', () => ({ extractEmbeddedFileRefs: mockExtractEmbeddedFileRefs, - storedFileId: (spelledId: string) => decodeURIComponent(spelledId), })) vi.mock('@sim/audit', () => ({ recordAudit: vi.fn(), diff --git a/apps/sim/app/api/files/export/[id]/route.ts b/apps/sim/app/api/files/export/[id]/route.ts index ceb2d6d118e..4f67b7b8353 100644 --- a/apps/sim/app/api/files/export/[id]/route.ts +++ b/apps/sim/app/api/files/export/[id]/route.ts @@ -15,8 +15,9 @@ import { captureServerEvent } from '@/lib/posthog/server' import type { StorageContext } from '@/lib/uploads/config' import { getServeStoragePrefix } from '@/lib/uploads/config' import { downloadFile } from '@/lib/uploads/core/storage-service' -import { extractEmbeddedFileRefs, storedFileId } from '@/lib/uploads/server/embedded-image-refs' +import { extractEmbeddedFileRefs } from '@/lib/uploads/server/embedded-image-refs' import { getFileMetadataById } from '@/lib/uploads/server/metadata' +import { storedFileId } from '@/lib/uploads/utils/embedded-image-ref' import { formatFileSize } from '@/lib/uploads/utils/file-utils' import { verifyFileAccess } from '@/app/api/files/authorization' import { encodeFilenameForHeader } from '@/app/api/files/utils' diff --git a/apps/sim/app/api/files/public/[token]/inline/route.test.ts b/apps/sim/app/api/files/public/[token]/inline/route.test.ts index 3f2b654bda0..5d3e7871d06 100644 --- a/apps/sim/app/api/files/public/[token]/inline/route.test.ts +++ b/apps/sim/app/api/files/public/[token]/inline/route.test.ts @@ -67,6 +67,15 @@ describe('GET /api/files/public/[token]/inline', () => { expect(res.headers.get('content-type')).toBe('image/png') }) + it('serves an image whose id is percent-encoded in the document', async () => { + mockDownloadFile.mockImplementation(downloadByKey('![a](/api/files/view/wf%5Fabc)')) + + const res = await GET(req('fileId=wf%5Fabc'), params) + + expect(res.status).toBe(200) + expect(mockResolveImage).toHaveBeenCalledWith('ws-1', { fileId: 'wf_abc' }) + }) + it('serves a key-referenced image', async () => { mockDownloadFile.mockImplementation( downloadByKey(`![a](/api/files/serve/${encodeURIComponent(IMG_KEY)}?context=workspace)`) diff --git a/apps/sim/app/api/files/public/[token]/inline/route.ts b/apps/sim/app/api/files/public/[token]/inline/route.ts index 85405a74712..80926733a67 100644 --- a/apps/sim/app/api/files/public/[token]/inline/route.ts +++ b/apps/sim/app/api/files/public/[token]/inline/route.ts @@ -12,6 +12,7 @@ import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager' import { downloadFile } from '@/lib/uploads/core/storage-service' import { extractEmbeddedFileRefs } from '@/lib/uploads/server/embedded-image-refs' import { resolveWorkspaceInlineImage } from '@/lib/uploads/server/inline-image' +import { storedFileId } from '@/lib/uploads/utils/embedded-image-ref' import { serveInlineImage } from '@/app/api/files/serve-inline-image' import { createErrorResponse, FileNotFoundError } from '@/app/api/files/utils' @@ -73,7 +74,9 @@ export const GET = withRouteHandler( // Referenced-by-doc gate: the share grants exactly the images the document embeds. const docText = (await downloadFile({ key: doc.key, context: 'workspace' })).toString('utf-8') const { keys, ids } = extractEmbeddedFileRefs(docText) - const referenced = ref.fileId ? ids.includes(ref.fileId) : keys.includes(ref.key as string) + const referenced = ref.fileId + ? ids.some((id) => storedFileId(id) === ref.fileId) + : keys.includes(ref.key as string) if (!referenced) { throw new FileNotFoundError('Not found') } diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.test.ts index a2879b6da6f..78c1662dc38 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.test.ts @@ -28,6 +28,18 @@ describe('content-source resolveImageSrc', () => { ) }) + it('normalizes a percent-encoded id before building an inline request', () => { + const workspace = createWorkspaceFileContentSource('ws-1') + const publicShare = createPublicFileContentSource('tok_1', '/api/files/public/tok_1/content') + + expect(workspace.resolveImageSrc('/api/files/view/wf%5Fabc')).toBe( + '/api/workspaces/ws-1/files/inline?fileId=wf_abc' + ) + expect(publicShare.resolveImageSrc('/api/files/view/wf%5Fabc')).toBe( + '/api/files/public/tok_1/inline?fileId=wf_abc' + ) + }) + it('passes external/data srcs through unchanged in both sources', () => { const ws = createWorkspaceFileContentSource('ws-1') const pub = createPublicFileContentSource('tok_1', '/c') diff --git a/apps/sim/hooks/use-file-content-source.tsx b/apps/sim/hooks/use-file-content-source.tsx index bcdd1d3f493..9917fbe1257 100644 --- a/apps/sim/hooks/use-file-content-source.tsx +++ b/apps/sim/hooks/use-file-content-source.tsx @@ -4,6 +4,7 @@ import { createContext, useContext } from 'react' import { type EmbeddedFileRef, extractEmbeddedFileRef, + storedFileId, } from '@/lib/uploads/utils/embedded-image-ref' export interface FileContentUrlOptions { @@ -24,7 +25,7 @@ export interface FileContentUrlOptions { function inlineRefQuery(ref: NonNullable): string { return 'key' in ref ? `key=${encodeURIComponent(ref.key)}` - : `fileId=${encodeURIComponent(ref.fileId)}` + : `fileId=${encodeURIComponent(storedFileId(ref.fileId))}` } export interface ImageDimensions { diff --git a/apps/sim/lib/copilot/tools/server/files/embedded-image-refs.ts b/apps/sim/lib/copilot/tools/server/files/embedded-image-refs.ts index 569ef320f00..e1f01ae3034 100644 --- a/apps/sim/lib/copilot/tools/server/files/embedded-image-refs.ts +++ b/apps/sim/lib/copilot/tools/server/files/embedded-image-refs.ts @@ -1,5 +1,6 @@ -import { extractEmbeddedFileRefs, storedFileId } from '@/lib/uploads/server/embedded-image-refs' +import { extractEmbeddedFileRefs } from '@/lib/uploads/server/embedded-image-refs' import { getFileMetadataById } from '@/lib/uploads/server/metadata' +import { storedFileId } from '@/lib/uploads/utils/embedded-image-ref' /** * Returns the ids of image embeds in `content` that will not render or survive a workspace export. diff --git a/apps/sim/lib/uploads/server/embedded-image-refs.ts b/apps/sim/lib/uploads/server/embedded-image-refs.ts index fb3574e20df..4ba624b093f 100644 --- a/apps/sim/lib/uploads/server/embedded-image-refs.ts +++ b/apps/sim/lib/uploads/server/embedded-image-refs.ts @@ -10,24 +10,6 @@ import { extractEmbeddedFileRef, extractImgSrcs } from '@/lib/uploads/utils/embe /** Hard cap on embedded images resolved from one document — bounds export bundles and the share cascade. */ export const MAX_EMBEDDED_IMAGES = 50 -/** - * The stored id behind the spelling a document used. {@link extractEmbeddedFileRefs} returns ids - * exactly as the document writes them, because that is what the export bundler searches for when it - * rewrites an embed — but storage is keyed by the decoded id. Every consumer that resolves one of - * these ids goes through here, so a document's spelling and the stored id stay distinct without - * either side re-deriving the other. - * - * Only for ids read out of document text. Ids arriving as request input are already constrained to - * the plain id charset by their route contract, so they need no decoding. - */ -export function storedFileId(spelledId: string): string { - try { - return decodeURIComponent(spelledId) - } catch { - return spelledId - } -} - /** * A parser of this module's own, not the `marked` singleton: the public share's referenced-by-doc * gate authorizes against what this returns, and a global `marked.use()` elsewhere in the process diff --git a/apps/sim/lib/uploads/utils/embedded-image-ref.test.ts b/apps/sim/lib/uploads/utils/embedded-image-ref.test.ts index d74297187c6..1bde01a81dc 100644 --- a/apps/sim/lib/uploads/utils/embedded-image-ref.test.ts +++ b/apps/sim/lib/uploads/utils/embedded-image-ref.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { extractEmbeddedFileRef, extractImgSrcs } from '@/lib/uploads/utils/embedded-image-ref' +import { + extractEmbeddedFileRef, + extractImgSrcs, + storedFileId, +} from '@/lib/uploads/utils/embedded-image-ref' const KEY = 'workspace/W1/1700000000000-deadbeefdeadbeef-photo.png' const ENCODED = encodeURIComponent(KEY) @@ -37,6 +41,17 @@ describe('extractEmbeddedFileRef', () => { }) }) +describe('storedFileId', () => { + it('decodes a document-spelled id exactly once', () => { + expect(storedFileId('wf%5Fabc')).toBe('wf_abc') + expect(storedFileId('wf%255Fabc')).toBe('wf%5Fabc') + }) + + it('leaves malformed encodings unchanged', () => { + expect(storedFileId('wf%5')).toBe('wf%5') + }) +}) + describe('extractImgSrcs', () => { it('reads double-quoted, single-quoted, and unquoted srcs in document order', () => { expect( diff --git a/apps/sim/lib/uploads/utils/embedded-image-ref.ts b/apps/sim/lib/uploads/utils/embedded-image-ref.ts index 27ffa1c1dd4..f49a97b1269 100644 --- a/apps/sim/lib/uploads/utils/embedded-image-ref.ts +++ b/apps/sim/lib/uploads/utils/embedded-image-ref.ts @@ -12,6 +12,19 @@ /** A reference parsed from an embed `src`: a workspace storage key, a workspace file id, or neither. */ export type EmbeddedFileRef = { key: string } | { fileId: string } | null +/** + * The stored id behind the spelling a document used. Embedded refs retain ids exactly as written so + * export rewriting can find the original URL, while storage and inline routes use the decoded id. + * Decode exactly once so malformed or double-encoded spellings continue to fail closed. + */ +export function storedFileId(spelledId: string): string { + try { + return decodeURIComponent(spelledId) + } catch { + return spelledId + } +} + /** * Parse a single embed `src` into the workspace file it references, normalizing the spellings the * editor and file agent produce: `/api/files/serve/` (incl. `s3/`/`blob/`/`gcs/` prefixes), `/api/files/view/`,