fix(files): preserve editor content and isolate shared image reads

Author: waleedlatif1

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

@@ -1,10 +1,13 @@
 /**
  * @vitest-environment node
  */
+
+import { recordAudit } from '@sim/audit'
 import { createMockRequest } from '@sim/testing'
 import JSZip from 'jszip'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
+import { getServeStoragePrefix } from '@/lib/uploads/config'
 
 const {
   mockCheckAuth,
@@ -92,6 +95,62 @@ beforeEach(() => {
 })
 
 describe('markdown export bundling', () => {
+  it('rejects an unauthenticated request before reading file metadata', async () => {
+    mockCheckAuth.mockResolvedValue({ success: false })
+    const response = await GET(request(), context)
+
+    expect(response.status).toBe(401)
+    expect(await response.json()).toEqual({ error: 'Unauthorized' })
+    expect(mockGetFileMetadataById).not.toHaveBeenCalled()
+    expect(recordAudit).not.toHaveBeenCalled()
+  })
+
+  it('preserves legacy missing-file and access-denied responses', async () => {
+    mockGetFileMetadataById.mockResolvedValueOnce(null)
+    const missing = await GET(request(), context)
+    expect(missing.status).toBe(404)
+    expect(await missing.json()).toEqual({ error: 'Not found' })
+
+    mockVerifyFileAccess.mockResolvedValue(false)
+    const forbidden = await GET(request(), context)
+    expect(forbidden.status).toBe(403)
+    expect(await forbidden.json()).toEqual({ error: 'Forbidden' })
+    expect(mockDownloadFile).not.toHaveBeenCalled()
+    expect(recordAudit).not.toHaveBeenCalled()
+  })
+
+  it('keeps non-Markdown downloads as authorized serve redirects', async () => {
+    mockGetFileMetadataById.mockResolvedValue({
+      ...DOC_RECORD,
+      originalName: 'report.pdf',
+      contentType: 'application/pdf',
+      context: 'chat',
+    })
+    const response = await GET(request(), context)
+
+    expect(response.status).toBe(302)
+    expect(response.headers.get('location')).toContain(
+      `/api/files/serve/${getServeStoragePrefix()}/${encodeURIComponent(DOC_RECORD.key)}`
+    )
+    expect(mockDownloadFile).not.toHaveBeenCalled()
+    expect(recordAudit).toHaveBeenCalledWith(
+      expect.objectContaining({
+        metadata: expect.objectContaining({ format: 'file', assetCount: 0 }),
+      })
+    )
+  })
+
+  it('preserves exact plain Markdown bytes and download headers', async () => {
+    const content = '\uFEFF---\r\ntitle: "Résumé"\r\n---\r\n\r\n# 你好 😀\r\n'
+    mockDownloadFile.mockResolvedValue(Buffer.from(content))
+    const response = await GET(request(), context)
+
+    expect(Buffer.from(await response.arrayBuffer())).toEqual(Buffer.from(content))
+    expect(response.headers.get('content-type')).toBe('text/markdown; charset=utf-8')
+    expect(response.headers.get('content-length')).toBe(String(Buffer.byteLength(content)))
+    expect(response.headers.get('content-disposition')).toContain('doc.md')
+  })
+
   it('rejects on declared asset bytes before downloading any of them', async () => {
     embeds('a', 'b', 'c')
     assetsResolveTo((id) => assetRecord(id, 100 * MB))
@@ -148,6 +207,23 @@ describe('markdown export bundling', () => {
     expect(assetCall?.[0].maxBytes).toBe(25 * MB)
   })
 
+  it('rejects actual aggregate bytes that exceed underreported asset metadata', async () => {
+    const ids = Array.from({ length: 30 }, (_, index) => `image-${index}`)
+    embeds(...ids)
+    assetsResolveTo((id) => assetRecord(id, 1))
+    const asset = Buffer.alloc(25 * MB)
+    mockDownloadFile.mockImplementation(async ({ key }: { key: string }) =>
+      key === DOC_RECORD.key ? Buffer.from('# Doc\n') : asset
+    )
+
+    const response = await GET(request(), context)
+
+    expect(response.status).toBe(400)
+    expect((await response.json()).error).toContain('exceeds')
+    expect(recordAudit).not.toHaveBeenCalled()
+    expect(mockDownloadFile.mock.calls.length).toBeLessThan(ids.length + 1)
+  })
+
   it('drops an unreadable asset instead of failing the whole export', async () => {
     embeds('good', 'bad')
     mockDownloadFile.mockImplementation(async ({ key }: { key: string }) => {

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

@@ -1,8 +1,6 @@
-import path from 'node:path'
 import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
 import { createLogger } from '@sim/logger'
 import { toError } from '@sim/utils/errors'
-import JSZip from 'jszip'
 import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
 import { fileExportContract } from '@/lib/api/contracts/storage-transfer'
@@ -16,6 +14,13 @@ 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 {
+  createMarkdownExport,
+  MAX_EXPORT_TOTAL_BYTES,
+  type MarkdownExportAsset,
+  type MarkdownExportResult,
+  MarkdownExportSizeError,
+} from '@/lib/uploads/server/markdown-export'
 import { getFileMetadataById } from '@/lib/uploads/server/metadata'
 import { getWorkspaceFileSize } from '@/lib/uploads/shared/types'
 import { storedFileId } from '@/lib/uploads/utils/embedded-image-ref'
@@ -25,18 +30,6 @@ import { encodeFilenameForHeader } from '@/app/api/files/utils'
 
 const logger = createLogger('FilesExportAPI')
 
-/**
- * Byte ceilings for a bundled export. The bytes behind an embed list are whatever the
- * author put there, so without these the export would materialize unbounded assets in
- * one request. They match the bulk-download route, so the two export surfaces reject at
- * the same size.
- *
- * There is deliberately no count cap here: `extractEmbeddedFileRefs` already stops at
- * `MAX_EMBEDDED_IMAGES`, so the list this route receives is bounded before it arrives.
- */
-const MAX_EXPORT_ASSET_BYTES = 25 * 1024 * 1024
-const MAX_EXPORT_TOTAL_BYTES = 250 * 1024 * 1024
-
 const MARKDOWN_MIME_TYPES = new Set(['text/markdown', 'text/x-markdown'])
 const MARKDOWN_EXTENSIONS = new Set(['md', 'markdown'])
 
@@ -46,22 +39,6 @@ function isMarkdown(originalName: string, contentType: string): boolean {
   return MARKDOWN_EXTENSIONS.has(ext)
 }
 
-function safeFilename(name: string): string {
-  return path
-    .basename(name)
-    .replace(/["\\]/g, '_')
-    .replace(/[\r\n\t]/g, '')
-}
-
-function deduplicatedFilename(preferred: string, existing: Set<string>, imageId: string): string {
-  if (!existing.has(preferred)) return preferred
-  const ext = path.extname(preferred)
-  const base = path.basename(preferred, ext)
-  const short = `${base}_${imageId.slice(0, 8)}${ext}`
-  if (!existing.has(short)) return short
-  return `${base}_${imageId}${ext}`
-}
-
 export const GET = withRouteHandler(
   async (request: NextRequest, context: { params: Promise<{ id: string }> }) => {
     const parsed = await parseRequest(fileExportContract, request, context)
@@ -152,7 +129,7 @@ export const GET = withRouteHandler(
         { status: 400 }
       )
     }
-    let mdContent = mdBuffer.toString('utf-8')
+    const mdContent = mdBuffer.toString('utf-8')
 
     // 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.
@@ -174,106 +151,42 @@ export const GET = withRouteHandler(
           ) {
             return null
           }
-          return { imageId, record: imgRecord, size: getWorkspaceFileSize(imgRecord) }
-        } catch (error) {
-          logger.warn('Failed to resolve asset for export', {
+          return {
             imageId,
-            error: toError(error).message,
-          })
-          return null
-        }
-      })
-    ).filter((target): target is NonNullable<typeof target> => target !== null)
-
-    // The body counts against the same budget as its assets — the zip holds both, so a
-    // limit that measured only the attachments would not describe the archive produced.
-    const bundleBytes = mdBuffer.length + assetTargets.reduce((sum, target) => sum + target.size, 0)
-    if (bundleBytes > MAX_EXPORT_TOTAL_BYTES) {
-      return NextResponse.json(
-        {
-          error: `This document and its embedded files total ${formatFileSize(bundleBytes)}, which exceeds the ${formatFileSize(MAX_EXPORT_TOTAL_BYTES)} export limit.`,
-        },
-        { status: 400 }
-      )
-    }
-
-    const fetched = await mapWithConcurrency(
-      assetTargets,
-      MATERIALIZE_CONCURRENCY,
-      async ({ imageId, record: imgRecord }) => {
-        try {
-          const buffer = await downloadFile({
             key: imgRecord.key,
             context: imgRecord.context as StorageContext,
-            maxBytes: MAX_EXPORT_ASSET_BYTES,
-          })
-          return { imageId, originalName: imgRecord.originalName, buffer }
+            originalName: imgRecord.originalName,
+            size: getWorkspaceFileSize(imgRecord),
+          } satisfies MarkdownExportAsset
         } catch (error) {
-          // A single unreadable or oversized asset drops out of the bundle rather than
-          // failing the whole export; the markdown keeps its original link.
-          logger.warn('Failed to fetch asset for export', {
+          logger.warn('Failed to resolve asset for export', {
             imageId,
             error: toError(error).message,
           })
           return null
         }
-      }
-    )
-
-    const assetMap = new Map<string, { filename: string; buffer: Buffer }>()
-    const usedFilenames = new Set<string>()
-
-    for (const result of fetched) {
-      if (!result) continue
-      const { imageId, originalName, buffer } = result
-      const preferred = safeFilename(originalName)
-      const filename = deduplicatedFilename(preferred, usedFilenames, imageId)
-      usedFilenames.add(filename)
-      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}`
-      // Rewrite both embed spellings the extractor resolves to this id — the view URL and the in-app
-      // `/workspace/<ws>/files/<id>` path — so a bundled asset never leaves a broken link in the export.
-      mdContent = mdContent
-        .replace(new RegExp(`/api/files/view/${escapedId}`, 'g'), () => replacement)
-        .replace(new RegExp(`/workspace/[A-Za-z0-9-]+/files/${escapedId}`, 'g'), () => replacement)
-    }
+    ).filter((target): target is NonNullable<typeof target> => target !== null)
 
-    const zip = new JSZip()
-    zip.file(safeFilename(record.originalName), mdContent)
-    const assetsFolder = zip.folder('assets')!
-    for (const { filename, buffer } of assetMap.values()) {
-      assetsFolder.file(filename, buffer)
+    let exported: MarkdownExportResult
+    try {
+      exported = await createMarkdownExport({
+        content: mdBuffer,
+        fileName: record.originalName,
+        assets: assetTargets,
+      })
+    } catch (error) {
+      if (!(error instanceof MarkdownExportSizeError)) throw error
+      return NextResponse.json({ error: error.message }, { status: 400 })
     }
 
-    const zipBuffer = await zip.generateAsync({ type: 'nodebuffer', compression: 'DEFLATE' })
-    const zipName = safeFilename(`${record.originalName.replace(/\.[^.]+$/, '')}.zip`)
-
-    auditExport('zip', assetMap.size)
-    return new NextResponse(new Uint8Array(zipBuffer), {
+    auditExport(exported.format, exported.assetCount)
+    return new NextResponse(new Uint8Array(exported.buffer), {
       status: 200,
       headers: {
-        'Content-Type': 'application/zip',
-        'Content-Disposition': `attachment; ${encodeFilenameForHeader(zipName)}`,
-        'Content-Length': String(zipBuffer.length),
+        'Content-Type': exported.contentType,
+        'Content-Disposition': `attachment; ${encodeFilenameForHeader(exported.fileName)}`,
+        'Content-Length': String(exported.buffer.length),
       },
     })
   }