fix(files): resolve an embed by its stored id wherever one is read from a document

The export bundler decoded an embed's spelling before looking it up, but the
file-agent's embeddability warning did not, so a percent-encoded embed the
export resolves and bundles could still be reported as one that will not
survive an export. Both now share one helper.

Request-supplied ids are untouched: their route contracts already constrain
them to the plain id charset, so there is no spelling to decode.

Author: waleedlatif1

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

@@ -33,6 +33,7 @@ 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(),

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

@@ -15,7 +15,7 @@ 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 } from '@/lib/uploads/server/embedded-image-refs'
+import { extractEmbeddedFileRefs, storedFileId } from '@/lib/uploads/server/embedded-image-refs'
 import { getFileMetadataById } from '@/lib/uploads/server/metadata'
 import { formatFileSize } from '@/lib/uploads/utils/file-utils'
 import { verifyFileAccess } from '@/app/api/files/authorization'
@@ -51,19 +51,6 @@ function safeFilename(name: string): string {
     .replace(/[\r\n\t]/g, '')
 }
 
-/**
- * The stored id behind an embed's spelling. Ids arrive spelled as the document writes them, because
- * the rewrite below locates each embed by searching for that spelling; metadata lookup needs the
- * decoded id instead, so the two representations are kept distinct rather than reconciled.
- */
-function storedId(spelledId: string): string {
-  try {
-    return decodeURIComponent(spelledId)
-  } catch {
-    return spelledId
-  }
-}
-
 function deduplicatedFilename(preferred: string, existing: Set<string>, imageId: string): string {
   if (!existing.has(preferred)) return preferred
   const ext = path.extname(preferred)
@@ -173,7 +160,7 @@ export const GET = withRouteHandler(
     const assetTargets = (
       await mapWithConcurrency(imageIds, MATERIALIZE_CONCURRENCY, async (imageId) => {
         try {
-          const imgRecord = await getFileMetadataById(storedId(imageId))
+          const imgRecord = await getFileMetadataById(storedFileId(imageId))
           if (!imgRecord) return null
           if (!(await verifyFileAccess(imgRecord.key, userId))) return null
           return { imageId, record: imgRecord }

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

@@ -44,4 +44,19 @@ describe('findUnembeddableImageRefs', () => {
     expect(await findUnembeddableImageRefs(content, WORKSPACE_ID)).toEqual([])
     expect(mockGetFileMetadataById).not.toHaveBeenCalled()
   })
+
+  /**
+   * The export bundler resolves an embed by its stored id, so reporting the same embed as one that
+   * will not survive an export would contradict what the export actually does with it.
+   */
+  it('resolves a percent-encoded embed by its stored id, like the export does', async () => {
+    mockGetFileMetadataById.mockImplementation(async (id: string) =>
+      id === 'wf_abc' ? { context: 'workspace', workspaceId: WORKSPACE_ID } : null
+    )
+
+    expect(await findUnembeddableImageRefs('![a](/api/files/view/wf%5Fabc)', WORKSPACE_ID)).toEqual(
+      []
+    )
+    expect(mockGetFileMetadataById).toHaveBeenCalledWith('wf_abc')
+  })
 })

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

@@ -1,4 +1,4 @@
-import { extractEmbeddedFileRefs } from '@/lib/uploads/server/embedded-image-refs'
+import { extractEmbeddedFileRefs, storedFileId } from '@/lib/uploads/server/embedded-image-refs'
 import { getFileMetadataById } from '@/lib/uploads/server/metadata'
 
 /**
@@ -9,6 +9,9 @@ import { getFileMetadataById } from '@/lib/uploads/server/metadata'
  * flagged by id alone, without disclosing the referenced file's real context or owning workspace, so
  * the result can't be used to probe files outside this workspace. Best-effort and never throws, so a
  * content write is never blocked by this validation.
+ *
+ * Resolved through {@link storedFileId}, like the export bundler: an embed the export would resolve
+ * and bundle must not be reported here as one that will not survive it.
  */
 export async function findUnembeddableImageRefs(
   content: string,
@@ -19,7 +22,7 @@ export async function findUnembeddableImageRefs(
   const checked = await Promise.all(
     ids.map(async (id): Promise<string | null> => {
       try {
-        const record = await getFileMetadataById(id)
+        const record = await getFileMetadataById(storedFileId(id))
         const embeddable = record?.context === 'workspace' && record.workspaceId === workspaceId
         return embeddable ? null : id
       } catch {

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

@@ -10,6 +10,24 @@ 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