Skip to content

Commit fbeea53

Browse files
authored
fix(files): normalize encoded embedded ids (#7035)
1 parent efe8a14 commit fbeea53

10 files changed

Lines changed: 60 additions & 24 deletions

File tree

apps/sim/app/api/files/export/[id]/route.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ vi.mock('@/app/api/files/authorization', () => ({ verifyFileAccess: mockVerifyFi
3333
vi.mock('@/lib/uploads/core/storage-service', () => ({ downloadFile: mockDownloadFile }))
3434
vi.mock('@/lib/uploads/server/embedded-image-refs', () => ({
3535
extractEmbeddedFileRefs: mockExtractEmbeddedFileRefs,
36-
storedFileId: (spelledId: string) => decodeURIComponent(spelledId),
3736
}))
3837
vi.mock('@sim/audit', () => ({
3938
recordAudit: vi.fn(),

apps/sim/app/api/files/export/[id]/route.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ import { captureServerEvent } from '@/lib/posthog/server'
1515
import type { StorageContext } from '@/lib/uploads/config'
1616
import { getServeStoragePrefix } from '@/lib/uploads/config'
1717
import { downloadFile } from '@/lib/uploads/core/storage-service'
18-
import { extractEmbeddedFileRefs, storedFileId } from '@/lib/uploads/server/embedded-image-refs'
18+
import { extractEmbeddedFileRefs } from '@/lib/uploads/server/embedded-image-refs'
1919
import { getFileMetadataById } from '@/lib/uploads/server/metadata'
20+
import { storedFileId } from '@/lib/uploads/utils/embedded-image-ref'
2021
import { formatFileSize } from '@/lib/uploads/utils/file-utils'
2122
import { verifyFileAccess } from '@/app/api/files/authorization'
2223
import { encodeFilenameForHeader } from '@/app/api/files/utils'

apps/sim/app/api/files/public/[token]/inline/route.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,15 @@ describe('GET /api/files/public/[token]/inline', () => {
6767
expect(res.headers.get('content-type')).toBe('image/png')
6868
})
6969

70+
it('serves an image whose id is percent-encoded in the document', async () => {
71+
mockDownloadFile.mockImplementation(downloadByKey('![a](/api/files/view/wf%5Fabc)'))
72+
73+
const res = await GET(req('fileId=wf%5Fabc'), params)
74+
75+
expect(res.status).toBe(200)
76+
expect(mockResolveImage).toHaveBeenCalledWith('ws-1', { fileId: 'wf_abc' })
77+
})
78+
7079
it('serves a key-referenced image', async () => {
7180
mockDownloadFile.mockImplementation(
7281
downloadByKey(`![a](/api/files/serve/${encodeURIComponent(IMG_KEY)}?context=workspace)`)

apps/sim/app/api/files/public/[token]/inline/route.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager'
1212
import { downloadFile } from '@/lib/uploads/core/storage-service'
1313
import { extractEmbeddedFileRefs } from '@/lib/uploads/server/embedded-image-refs'
1414
import { resolveWorkspaceInlineImage } from '@/lib/uploads/server/inline-image'
15+
import { storedFileId } from '@/lib/uploads/utils/embedded-image-ref'
1516
import { serveInlineImage } from '@/app/api/files/serve-inline-image'
1617
import { createErrorResponse, FileNotFoundError } from '@/app/api/files/utils'
1718

@@ -73,7 +74,9 @@ export const GET = withRouteHandler(
7374
// Referenced-by-doc gate: the share grants exactly the images the document embeds.
7475
const docText = (await downloadFile({ key: doc.key, context: 'workspace' })).toString('utf-8')
7576
const { keys, ids } = extractEmbeddedFileRefs(docText)
76-
const referenced = ref.fileId ? ids.includes(ref.fileId) : keys.includes(ref.key as string)
77+
const referenced = ref.fileId
78+
? ids.some((id) => storedFileId(id) === ref.fileId)
79+
: keys.includes(ref.key as string)
7780
if (!referenced) {
7881
throw new FileNotFoundError('Not found')
7982
}

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,18 @@ describe('content-source resolveImageSrc', () => {
2828
)
2929
})
3030

31+
it('normalizes a percent-encoded id before building an inline request', () => {
32+
const workspace = createWorkspaceFileContentSource('ws-1')
33+
const publicShare = createPublicFileContentSource('tok_1', '/api/files/public/tok_1/content')
34+
35+
expect(workspace.resolveImageSrc('/api/files/view/wf%5Fabc')).toBe(
36+
'/api/workspaces/ws-1/files/inline?fileId=wf_abc'
37+
)
38+
expect(publicShare.resolveImageSrc('/api/files/view/wf%5Fabc')).toBe(
39+
'/api/files/public/tok_1/inline?fileId=wf_abc'
40+
)
41+
})
42+
3143
it('passes external/data srcs through unchanged in both sources', () => {
3244
const ws = createWorkspaceFileContentSource('ws-1')
3345
const pub = createPublicFileContentSource('tok_1', '/c')

apps/sim/hooks/use-file-content-source.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createContext, useContext } from 'react'
44
import {
55
type EmbeddedFileRef,
66
extractEmbeddedFileRef,
7+
storedFileId,
78
} from '@/lib/uploads/utils/embedded-image-ref'
89

910
export interface FileContentUrlOptions {
@@ -24,7 +25,7 @@ export interface FileContentUrlOptions {
2425
function inlineRefQuery(ref: NonNullable<EmbeddedFileRef>): string {
2526
return 'key' in ref
2627
? `key=${encodeURIComponent(ref.key)}`
27-
: `fileId=${encodeURIComponent(ref.fileId)}`
28+
: `fileId=${encodeURIComponent(storedFileId(ref.fileId))}`
2829
}
2930

3031
export interface ImageDimensions {

apps/sim/lib/copilot/tools/server/files/embedded-image-refs.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { extractEmbeddedFileRefs, storedFileId } from '@/lib/uploads/server/embedded-image-refs'
1+
import { extractEmbeddedFileRefs } from '@/lib/uploads/server/embedded-image-refs'
22
import { getFileMetadataById } from '@/lib/uploads/server/metadata'
3+
import { storedFileId } from '@/lib/uploads/utils/embedded-image-ref'
34

45
/**
56
* Returns the ids of image embeds in `content` that will not render or survive a workspace export.

apps/sim/lib/uploads/server/embedded-image-refs.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,6 @@ import { extractEmbeddedFileRef, extractImgSrcs } from '@/lib/uploads/utils/embe
1010
/** Hard cap on embedded images resolved from one document — bounds export bundles and the share cascade. */
1111
export const MAX_EMBEDDED_IMAGES = 50
1212

13-
/**
14-
* The stored id behind the spelling a document used. {@link extractEmbeddedFileRefs} returns ids
15-
* exactly as the document writes them, because that is what the export bundler searches for when it
16-
* rewrites an embed — but storage is keyed by the decoded id. Every consumer that resolves one of
17-
* these ids goes through here, so a document's spelling and the stored id stay distinct without
18-
* either side re-deriving the other.
19-
*
20-
* Only for ids read out of document text. Ids arriving as request input are already constrained to
21-
* the plain id charset by their route contract, so they need no decoding.
22-
*/
23-
export function storedFileId(spelledId: string): string {
24-
try {
25-
return decodeURIComponent(spelledId)
26-
} catch {
27-
return spelledId
28-
}
29-
}
30-
3113
/**
3214
* A parser of this module's own, not the `marked` singleton: the public share's referenced-by-doc
3315
* gate authorizes against what this returns, and a global `marked.use()` elsewhere in the process

apps/sim/lib/uploads/utils/embedded-image-ref.test.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, expect, it } from 'vitest'
2-
import { extractEmbeddedFileRef, extractImgSrcs } from '@/lib/uploads/utils/embedded-image-ref'
2+
import {
3+
extractEmbeddedFileRef,
4+
extractImgSrcs,
5+
storedFileId,
6+
} from '@/lib/uploads/utils/embedded-image-ref'
37

48
const KEY = 'workspace/W1/1700000000000-deadbeefdeadbeef-photo.png'
59
const ENCODED = encodeURIComponent(KEY)
@@ -37,6 +41,17 @@ describe('extractEmbeddedFileRef', () => {
3741
})
3842
})
3943

44+
describe('storedFileId', () => {
45+
it('decodes a document-spelled id exactly once', () => {
46+
expect(storedFileId('wf%5Fabc')).toBe('wf_abc')
47+
expect(storedFileId('wf%255Fabc')).toBe('wf%5Fabc')
48+
})
49+
50+
it('leaves malformed encodings unchanged', () => {
51+
expect(storedFileId('wf%5')).toBe('wf%5')
52+
})
53+
})
54+
4055
describe('extractImgSrcs', () => {
4156
it('reads double-quoted, single-quoted, and unquoted srcs in document order', () => {
4257
expect(

apps/sim/lib/uploads/utils/embedded-image-ref.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,19 @@
1212
/** A reference parsed from an embed `src`: a workspace storage key, a workspace file id, or neither. */
1313
export type EmbeddedFileRef = { key: string } | { fileId: string } | null
1414

15+
/**
16+
* The stored id behind the spelling a document used. Embedded refs retain ids exactly as written so
17+
* export rewriting can find the original URL, while storage and inline routes use the decoded id.
18+
* Decode exactly once so malformed or double-encoded spellings continue to fail closed.
19+
*/
20+
export function storedFileId(spelledId: string): string {
21+
try {
22+
return decodeURIComponent(spelledId)
23+
} catch {
24+
return spelledId
25+
}
26+
}
27+
1528
/**
1629
* Parse a single embed `src` into the workspace file it references, normalizing the spellings the
1730
* editor and file agent produce: `/api/files/serve/<key>` (incl. `s3/`/`blob/`/`gcs/` prefixes), `/api/files/view/<id>`,

0 commit comments

Comments
 (0)