fix(files): download a markdown file as a zip only when it really has assets

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

@@ -11,23 +11,29 @@ const {
   mockGetFileMetadataById,
   mockVerifyFileAccess,
   mockDownloadFile,
-  mockExtractEmbeddedImageIds,
+  mockExtractEmbeddedFileRefs,
 } = vi.hoisted(() => ({
   mockCheckAuth: vi.fn(),
   mockGetFileMetadataById: vi.fn(),
   mockVerifyFileAccess: vi.fn(),
   mockDownloadFile: vi.fn(),
-  mockExtractEmbeddedImageIds: vi.fn(),
+  mockExtractEmbeddedFileRefs: vi.fn(),
 }))
 
+/** `embedded-image-refs.test.ts` covers the grammar itself. */
+function embeds(...ids: string[]) {
+  mockExtractEmbeddedFileRefs.mockReturnValue({ keys: [], ids })
+}
+
 vi.mock('@/lib/auth/hybrid', () => ({ checkSessionOrInternalAuth: mockCheckAuth }))
 vi.mock('@/lib/uploads/server/metadata', () => ({
   getFileMetadataById: mockGetFileMetadataById,
 }))
 vi.mock('@/app/api/files/authorization', () => ({ verifyFileAccess: mockVerifyFileAccess }))
 vi.mock('@/lib/uploads/core/storage-service', () => ({ downloadFile: mockDownloadFile }))
-vi.mock('@/lib/copilot/tools/server/files/embedded-image-refs', () => ({
-  extractEmbeddedImageIds: mockExtractEmbeddedImageIds,
+vi.mock('@/lib/uploads/server/embedded-image-refs', () => ({
+  extractEmbeddedFileRefs: mockExtractEmbeddedFileRefs,
+  storedFileId: (spelledId: string) => decodeURIComponent(spelledId),
 }))
 vi.mock('@sim/audit', () => ({
   recordAudit: vi.fn(),
@@ -58,43 +64,35 @@ function assetRecord(id: string, size: number) {
   }
 }
 
-describe('markdown export bundling', () => {
-  beforeEach(() => {
-    vi.clearAllMocks()
-    mockCheckAuth.mockResolvedValue({ success: true, userId: 'user-1' })
-    mockVerifyFileAccess.mockResolvedValue(true)
-    mockGetFileMetadataById.mockImplementation(async (id: string) =>
-      id === DOC_ID
-        ? {
-            id: DOC_ID,
-            key: 'workspace/ws-1/doc.md',
-            originalName: 'doc.md',
-            contentType: 'text/markdown',
-            context: 'workspace',
-            size: 1024,
-            workspaceId: 'ws-1',
-          }
-        : assetRecord(id, 1 * MB)
-    )
-    mockDownloadFile.mockResolvedValue(Buffer.from('# Doc\n'))
-    mockExtractEmbeddedImageIds.mockReturnValue([])
-  })
+const DOC_RECORD = {
+  id: DOC_ID,
+  key: 'workspace/ws-1/doc.md',
+  originalName: 'doc.md',
+  contentType: 'text/markdown',
+  context: 'workspace',
+  size: 1024,
+  workspaceId: 'ws-1',
+}
+
+function assetsResolveTo(assetFor: (id: string) => unknown) {
+  mockGetFileMetadataById.mockImplementation(async (id: string) =>
+    id === DOC_ID ? DOC_RECORD : assetFor(id)
+  )
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  mockCheckAuth.mockResolvedValue({ success: true, userId: 'user-1' })
+  mockVerifyFileAccess.mockResolvedValue(true)
+  assetsResolveTo((id) => assetRecord(id, 1 * MB))
+  mockDownloadFile.mockResolvedValue(Buffer.from('# Doc\n'))
+  embeds()
+})
 
+describe('markdown export bundling', () => {
   it('rejects on declared asset bytes before downloading any of them', async () => {
-    mockExtractEmbeddedImageIds.mockReturnValue(['a', 'b', 'c'])
-    mockGetFileMetadataById.mockImplementation(async (id: string) =>
-      id === DOC_ID
-        ? {
-            id: DOC_ID,
-            key: 'workspace/ws-1/doc.md',
-            originalName: 'doc.md',
-            contentType: 'text/markdown',
-            context: 'workspace',
-            size: 1024,
-            workspaceId: 'ws-1',
-          }
-        : assetRecord(id, 100 * MB)
-    )
+    embeds('a', 'b', 'c')
+    assetsResolveTo((id) => assetRecord(id, 100 * MB))
 
     const response = await GET(request(), context)
 
@@ -106,7 +104,7 @@ describe('markdown export bundling', () => {
 
   it('counts the document body against the export limit, not just its assets', async () => {
     // Assets alone sit under the cap; the body is what carries the bundle over it.
-    mockExtractEmbeddedImageIds.mockReturnValue(['a'])
+    embeds('a')
     mockDownloadFile.mockResolvedValue(Buffer.alloc(250 * MB))
 
     const response = await GET(request(), context)
@@ -116,7 +114,7 @@ describe('markdown export bundling', () => {
   })
 
   it('caps the document body read rather than loading it unbounded', async () => {
-    mockExtractEmbeddedImageIds.mockReturnValue([])
+    embeds()
 
     await GET(request(), context)
 
@@ -125,7 +123,7 @@ describe('markdown export bundling', () => {
   })
 
   it('reports an oversized body as a size rejection, not a server error', async () => {
-    mockExtractEmbeddedImageIds.mockReturnValue([])
+    embeds()
     mockDownloadFile.mockRejectedValue(
       new PayloadSizeLimitError({ label: 'storage file download', maxBytes: 1 })
     )
@@ -138,7 +136,7 @@ describe('markdown export bundling', () => {
   })
 
   it('caps each asset download rather than trusting its declared size', async () => {
-    mockExtractEmbeddedImageIds.mockReturnValue(['a'])
+    embeds('a')
 
     await GET(request(), context)
 
@@ -149,7 +147,7 @@ describe('markdown export bundling', () => {
   })
 
   it('drops an unreadable asset instead of failing the whole export', async () => {
-    mockExtractEmbeddedImageIds.mockReturnValue(['good', 'bad'])
+    embeds('good', 'bad')
     mockDownloadFile.mockImplementation(async ({ key }: { key: string }) => {
       if (key.endsWith('doc.md')) return Buffer.from('# Doc\n![x](/api/files/view/good)\n')
       if (key.endsWith('bad')) throw new Error('storage down')
@@ -164,8 +162,31 @@ describe('markdown export bundling', () => {
     expect(zip.file('assets/bad.png')).toBeNull()
   })
 
+  /**
+   * The two id representations have to stay distinct: metadata resolves by the stored id, while the
+   * rewrite finds the embed by the spelling the document used. Collapsing them either drops the
+   * asset or bundles it behind a link still pointing at the API.
+   */
+  it('resolves and rewrites an embed whose id is percent-encoded in the document', async () => {
+    embeds('wf%5Fa')
+    assetsResolveTo((id) => (id === 'wf_a' ? assetRecord(id, 1 * MB) : null))
+    mockDownloadFile.mockImplementation(async ({ key }: { key: string }) =>
+      key.endsWith('doc.md')
+        ? Buffer.from('# Doc\n![x](/api/files/view/wf%5Fa)\n')
+        : Buffer.from('png-bytes')
+    )
+
+    const response = await GET(request(), context)
+
+    const zip = await JSZip.loadAsync(Buffer.from(await response.arrayBuffer()))
+    expect(zip.file('assets/wf_a.png')).not.toBeNull()
+    const md = await zip.file('doc.md')?.async('string')
+    expect(md).toContain('./assets/wf_a.png')
+    expect(md).not.toContain('/api/files/view/')
+  })
+
   it('skips an asset the caller cannot read', async () => {
-    mockExtractEmbeddedImageIds.mockReturnValue(['secret'])
+    embeds('secret')
     mockVerifyFileAccess.mockImplementation(async (key: string) => !key.endsWith('secret'))
 
     const response = await GET(request(), context)
@@ -177,3 +198,47 @@ describe('markdown export bundling', () => {
     )
   })
 })
+
+describe('markdown export format', () => {
+  async function expectPlainMarkdown(response: Response) {
+    expect(response.status).toBe(200)
+    expect(response.headers.get('Content-Type')).toBe('text/markdown; charset=utf-8')
+    expect(response.headers.get('Content-Disposition')).toContain('doc.md')
+    expect(await response.text()).toBe('# Doc\n')
+  }
+
+  it('returns the document itself when it embeds nothing', async () => {
+    await expectPlainMarkdown(await GET(request(), context))
+  })
+
+  /**
+   * The reported bug: a document that references files which no longer resolve downloaded as a zip
+   * whose `assets/` folder was empty. The format follows what was bundled, not what was referenced.
+   */
+  it('returns the document itself when no embed resolves to a file', async () => {
+    embeds('gone', 'also-gone')
+    assetsResolveTo(() => null)
+
+    await expectPlainMarkdown(await GET(request(), context))
+  })
+
+  it('returns the document itself when every embed fails to download', async () => {
+    embeds('a')
+    mockDownloadFile.mockImplementation(async ({ key }: { key: string }) => {
+      if (key.endsWith('doc.md')) return Buffer.from('# Doc\n')
+      throw new Error('storage down')
+    })
+
+    await expectPlainMarkdown(await GET(request(), context))
+  })
+
+  it('bundles a zip once at least one embed resolves', async () => {
+    embeds('a')
+
+    const response = await GET(request(), context)
+
+    expect(response.headers.get('Content-Type')).toBe('application/zip')
+    const zip = await JSZip.loadAsync(Buffer.from(await response.arrayBuffer()))
+    expect(zip.file('assets/a.png')).not.toBeNull()
+  })
+})

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

@@ -8,14 +8,14 @@ import { NextResponse } from 'next/server'
 import { fileExportContract } from '@/lib/api/contracts/storage-transfer'
 import { parseRequest } from '@/lib/api/server'
 import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
-import { extractEmbeddedImageIds } from '@/lib/copilot/tools/server/files/embedded-image-refs'
 import { MATERIALIZE_CONCURRENCY, mapWithConcurrency } from '@/lib/core/utils/concurrency'
 import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 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 { getFileMetadataById } from '@/lib/uploads/server/metadata'
 import { formatFileSize } from '@/lib/uploads/utils/file-utils'
 import { verifyFileAccess } from '@/app/api/files/authorization'
@@ -149,30 +149,18 @@ export const GET = withRouteHandler(
     }
     let mdContent = mdBuffer.toString('utf-8')
 
-    const imageIds = extractEmbeddedImageIds(mdContent)
+    // Ids only: a serve-URL embed names a storage key, which the bundler has no id to rewrite the
+    // markdown against, so those images stay pointed at their original URL.
+    const { ids: imageIds } = extractEmbeddedFileRefs(mdContent)
 
     logger.info('Exporting markdown', { id, imageCount: imageIds.length })
 
-    if (imageIds.length === 0) {
-      const mdName = safeFilename(record.originalName)
-      const mdBytes = Buffer.from(mdContent, 'utf-8')
-      auditExport('markdown', 0)
-      return new NextResponse(new Uint8Array(mdBytes), {
-        status: 200,
-        headers: {
-          'Content-Type': 'text/markdown; charset=utf-8',
-          'Content-Disposition': `attachment; ${encodeFilenameForHeader(mdName)}`,
-          'Content-Length': String(mdBytes.length),
-        },
-      })
-    }
-
     // Metadata first: declared sizes bound the download before a byte is read, and the
     // authorization check costs nothing to run here.
     const assetTargets = (
       await mapWithConcurrency(imageIds, MATERIALIZE_CONCURRENCY, async (imageId) => {
         try {
-          const imgRecord = await getFileMetadataById(imageId)
+          const imgRecord = await getFileMetadataById(storedFileId(imageId))
           if (!imgRecord) return null
           if (!(await verifyFileAccess(imgRecord.key, userId))) return null
           return { imageId, record: imgRecord }
@@ -234,6 +222,21 @@ export const GET = withRouteHandler(
       assetMap.set(imageId, { filename, buffer })
     }
 
+    // Format follows what was bundled, not what was referenced: an embed can point at a file that is
+    // missing, unreadable, or oversized, and an empty `assets/` zip is a worse answer than the
+    // document itself. `mdContent` is still unrewritten here, so `mdBuffer` holds exactly its bytes.
+    if (assetMap.size === 0) {
+      auditExport('markdown', 0)
+      return new NextResponse(new Uint8Array(mdBuffer), {
+        status: 200,
+        headers: {
+          'Content-Type': 'text/markdown; charset=utf-8',
+          'Content-Disposition': `attachment; ${encodeFilenameForHeader(safeFilename(record.originalName))}`,
+          'Content-Length': String(mdBuffer.length),
+        },
+      })
+    }
+
     for (const [imageId, asset] of assetMap) {
       const escapedId = imageId.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
       const replacement = `./assets/${asset.filename}`

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

@@ -4,16 +4,13 @@ import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
 import { getPublicInlineFileContract } from '@/lib/api/contracts/public-shares'
 import { parseRequest } from '@/lib/api/server'
-import {
-  extractEmbeddedImageIds,
-  extractEmbeddedImageKeys,
-} from '@/lib/copilot/tools/server/files/embedded-image-refs'
 import { validateDeploymentAuth } from '@/lib/core/security/deployment-auth'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { enforcePublicFileRateLimit } from '@/lib/public-shares/rate-limit'
 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 { serveInlineImage } from '@/app/api/files/serve-inline-image'
 import { createErrorResponse, FileNotFoundError } from '@/app/api/files/utils'
@@ -29,8 +26,9 @@ const logger = createLogger('PublicInlineFileAPI')
  * instead of broken icons. The share grants the document bytes; this route extends that grant to the
  * document's referenced images only, behind three gates that together hold the security boundary:
  *
- * 1. Referenced-by-doc — the requested key/id must appear in the shared document's current bytes. The
- *    token is a capability for the document and its embeds, never an arbitrary workspace file.
+ * 1. Referenced-by-doc — the requested key/id must be embedded as an image by the shared document's
+ *    current bytes. The token is a capability for the document and its embeds, never an arbitrary
+ *    workspace file, and never one the document merely links to or mentions in prose.
  * 2. Same-workspace — the referenced file must be a `workspace` file in the document's own workspace
  *    ({@link resolveWorkspaceInlineImage}). This blocks any cross-workspace reference (which an author
  *    can write but must never resolve) from loading.
@@ -74,9 +72,8 @@ 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 referenced = ref.fileId
-        ? extractEmbeddedImageIds(docText).includes(ref.fileId)
-        : extractEmbeddedImageKeys(docText).includes(ref.key as string)
+      const { keys, ids } = extractEmbeddedFileRefs(docText)
+      const referenced = ref.fileId ? ids.includes(ref.fileId) : keys.includes(ref.key as string)
       if (!referenced) {
         throw new FileNotFoundError('Not found')
       }

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

@@ -11,7 +11,6 @@ import {
 import { createMarkdownEditorExtensions } from './editor-extensions'
 import {
   extractImageFiles,
-  extractImgSrcs,
   findHostedImageAttrs,
   hasHostedImageHtml,
   htmlReferencesSrc,
@@ -151,19 +150,6 @@ describe('hasHostedImageHtml', () => {
   })
 })
 
-describe('extractImgSrcs', () => {
-  it('extracts every img src in document order, including duplicates', () => {
-    expect(
-      extractImgSrcs('<img src="/a.png"><p>text</p><img src="/b.png"><img src="/a.png">')
-    ).toEqual(['/a.png', '/b.png', '/a.png'])
-  })
-
-  it('returns an empty array for html with no img', () => {
-    expect(extractImgSrcs('<p>hello</p>')).toEqual([])
-    expect(extractImgSrcs('')).toEqual([])
-  })
-})
-
 describe('shouldSkipFileUpload (shared by paste and drop)', () => {
   const isHosted = (src: string) => src.startsWith('/api/files/view/')
   const hostedHtml = '<img src="/api/files/view/wf_abc">'