Skip to content

Commit f3a00a2

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(files): harden Markdown PDF export
1 parent 1f97719 commit f3a00a2

6 files changed

Lines changed: 257 additions & 27 deletions

File tree

apps/sim/app/api/files/export/[id]/markdown-pdf.test.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ Smart quotes “work”, Greek Ω stays readable, and unsupported emoji 🚀 fal
4040
const exported = true
4141
\`\`\`
4242
43-
![Embedded image](/api/files/view/image-1)
43+
![Embedded image](/workspace/ws-1/files/image-1)
4444
4545
${repeatedParagraphs}`
4646

@@ -57,4 +57,19 @@ ${repeatedParagraphs}`
5757
expect(document.getTitle()).toBe('Export title')
5858
expect(document.getPageCount()).toBeGreaterThan(1)
5959
})
60+
61+
it('falls back instead of decoding an image above the pixel ceiling', async () => {
62+
const oversizedSvg = Buffer.from(
63+
'<svg xmlns="http://www.w3.org/2000/svg" width="20000" height="20000"><rect width="100%" height="100%" fill="red"/></svg>'
64+
)
65+
66+
const buffer = await renderMarkdownPdf({
67+
markdown: '![Too large](/api/files/view/image-1)',
68+
title: 'Bounded image',
69+
images: new Map([['image-1', oversizedSvg]]),
70+
})
71+
72+
expect(buffer.subarray(0, 4).toString()).toBe('%PDF')
73+
expect((await PDFDocument.load(buffer)).getPageCount()).toBe(1)
74+
})
6075
})

apps/sim/app/api/files/export/[id]/markdown-pdf.tsx

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@react-pdf/renderer'
1515
import { marked, type Token, type Tokens } from 'marked'
1616
import sharp from 'sharp'
17+
import { extractEmbeddedFileRef } from '@/lib/uploads/utils/embedded-image-ref'
1718

1819
type PdfImage = { data: Buffer; format: 'png' }
1920

@@ -25,6 +26,18 @@ const FONT_DIR = join(process.cwd(), 'public', 'brand', 'fonts')
2526
const GEIST_REGULAR = join(FONT_DIR, 'Geist-Regular.ttf')
2627
const GEIST_MEDIUM = join(FONT_DIR, 'Geist-Medium.ttf')
2728

29+
/**
30+
* PDF images never render wider than the A4 content box, so retaining camera-resolution
31+
* rasters only increases Sharp and React PDF work. The dimension matches the app's existing
32+
* inline-image preparation ceiling; the aggregate budgets bound work across a document.
33+
*/
34+
const MAX_PDF_IMAGE_DIMENSION = 1568
35+
const MAX_PDF_IMAGE_INPUT_PIXELS = 268_402_689
36+
const MAX_PDF_TOTAL_INPUT_PIXELS = 268_402_689
37+
const MAX_PDF_TOTAL_OUTPUT_PIXELS = 25_000_000
38+
const MAX_PDF_IMAGE_BYTES = 12 * 1024 * 1024
39+
const MAX_PDF_TOTAL_IMAGE_BYTES = 32 * 1024 * 1024
40+
2841
Font.register({
2942
family: 'Geist',
3043
fonts: [
@@ -148,15 +161,8 @@ function safeLink(href: string): string | undefined {
148161
}
149162

150163
function embeddedImageId(href: string): string | undefined {
151-
const match =
152-
href.match(/\/api\/files\/view\/([^/?#]+)/) ??
153-
href.match(/\/workspace\/[^/]+\/files\/([^/?#]+)/)
154-
if (!match?.[1]) return undefined
155-
try {
156-
return decodeURIComponent(match[1])
157-
} catch {
158-
return match[1]
159-
}
164+
const ref = extractEmbeddedFileRef(href)
165+
return ref && 'fileId' in ref ? ref.fileId : undefined
160166
}
161167

162168
function renderInline(tokens: Token[], keyPrefix: string): ReactNode[] {
@@ -423,9 +429,55 @@ async function normalizeImages(
423429
images: ReadonlyMap<string, Buffer>
424430
): Promise<Map<string, PdfImage>> {
425431
const normalized = new Map<string, PdfImage>()
432+
let totalInputPixels = 0
433+
let totalOutputPixels = 0
434+
let totalImageBytes = 0
435+
426436
for (const [id, buffer] of images) {
427437
try {
428-
normalized.set(id, { data: await sharp(buffer).rotate().png().toBuffer(), format: 'png' })
438+
const pipeline = sharp(buffer, { limitInputPixels: MAX_PDF_IMAGE_INPUT_PIXELS })
439+
const metadata = await pipeline.metadata()
440+
if (!metadata.width || !metadata.height) continue
441+
442+
const inputPixels = metadata.width * metadata.height
443+
if (
444+
!Number.isSafeInteger(inputPixels) ||
445+
totalInputPixels + inputPixels > MAX_PDF_TOTAL_INPUT_PIXELS
446+
) {
447+
continue
448+
}
449+
totalInputPixels += inputPixels
450+
451+
const scale = Math.min(
452+
1,
453+
MAX_PDF_IMAGE_DIMENSION / metadata.width,
454+
MAX_PDF_IMAGE_DIMENSION / metadata.height
455+
)
456+
const outputWidth = Math.max(1, Math.round(metadata.width * scale))
457+
const outputHeight = Math.max(1, Math.round(metadata.height * scale))
458+
const outputPixels = outputWidth * outputHeight
459+
if (totalOutputPixels + outputPixels > MAX_PDF_TOTAL_OUTPUT_PIXELS) continue
460+
461+
const data = await pipeline
462+
.rotate()
463+
.resize({
464+
width: MAX_PDF_IMAGE_DIMENSION,
465+
height: MAX_PDF_IMAGE_DIMENSION,
466+
fit: 'inside',
467+
withoutEnlargement: true,
468+
})
469+
.png()
470+
.toBuffer()
471+
if (
472+
data.length > MAX_PDF_IMAGE_BYTES ||
473+
totalImageBytes + data.length > MAX_PDF_TOTAL_IMAGE_BYTES
474+
) {
475+
continue
476+
}
477+
478+
totalOutputPixels += outputPixels
479+
totalImageBytes += data.length
480+
normalized.set(id, { data, format: 'png' })
429481
} catch {
430482
// Keep the PDF usable when an otherwise downloadable attachment is not a renderable image.
431483
}

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

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,19 @@ const {
1313
mockDownloadFile,
1414
mockExtractEmbeddedImageIds,
1515
mockRenderMarkdownPdf,
16+
mockEnforceUserRateLimit,
17+
mockRecordAudit,
18+
mockCaptureServerEvent,
1619
} = vi.hoisted(() => ({
1720
mockCheckAuth: vi.fn(),
1821
mockGetFileMetadataById: vi.fn(),
1922
mockVerifyFileAccess: vi.fn(),
2023
mockDownloadFile: vi.fn(),
2124
mockExtractEmbeddedImageIds: vi.fn(),
2225
mockRenderMarkdownPdf: vi.fn(),
26+
mockEnforceUserRateLimit: vi.fn(),
27+
mockRecordAudit: vi.fn(),
28+
mockCaptureServerEvent: vi.fn(),
2329
}))
2430

2531
vi.mock('@/lib/auth/hybrid', () => ({ checkSessionOrInternalAuth: mockCheckAuth }))
@@ -34,12 +40,15 @@ vi.mock('@/lib/copilot/tools/server/files/embedded-image-refs', () => ({
3440
vi.mock('@/app/api/files/export/[id]/markdown-pdf', () => ({
3541
renderMarkdownPdf: mockRenderMarkdownPdf,
3642
}))
43+
vi.mock('@/lib/core/rate-limiter/route-helpers', () => ({
44+
enforceUserRateLimit: mockEnforceUserRateLimit,
45+
}))
3746
vi.mock('@sim/audit', () => ({
38-
recordAudit: vi.fn(),
47+
recordAudit: mockRecordAudit,
3948
AuditAction: { FILE_DOWNLOADED: 'file.downloaded' },
4049
AuditResourceType: { FILE: 'file' },
4150
}))
42-
vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: vi.fn() }))
51+
vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mockCaptureServerEvent }))
4352

4453
import { GET } from '@/app/api/files/export/[id]/route'
4554

@@ -90,6 +99,7 @@ describe('markdown export bundling', () => {
9099
mockDownloadFile.mockResolvedValue(Buffer.from('# Doc\n'))
91100
mockExtractEmbeddedImageIds.mockReturnValue([])
92101
mockRenderMarkdownPdf.mockResolvedValue(Buffer.from('%PDF-generated'))
102+
mockEnforceUserRateLimit.mockResolvedValue(null)
93103
})
94104

95105
it('returns the stored Markdown unchanged when no format is requested', async () => {
@@ -100,6 +110,7 @@ describe('markdown export bundling', () => {
100110
expect(response.headers.get('Content-Disposition')).toContain('doc.md')
101111
expect(Buffer.from(await response.arrayBuffer()).toString()).toBe('# Doc\n')
102112
expect(mockRenderMarkdownPdf).not.toHaveBeenCalled()
113+
expect(mockEnforceUserRateLimit).not.toHaveBeenCalled()
103114
})
104115

105116
it('renders Markdown as a directly downloadable PDF', async () => {
@@ -115,6 +126,23 @@ describe('markdown export bundling', () => {
115126
images: expect.any(Map),
116127
})
117128
expect(mockRenderMarkdownPdf.mock.calls[0][0].images.size).toBe(0)
129+
expect(mockEnforceUserRateLimit).toHaveBeenCalledWith('markdown-pdf-export', 'user-1', {
130+
maxTokens: 3,
131+
refillRate: 3,
132+
refillIntervalMs: 60_000,
133+
})
134+
})
135+
136+
it('stops a rate-limited PDF export before reading the document', async () => {
137+
mockEnforceUserRateLimit.mockResolvedValue(
138+
new Response(JSON.stringify({ error: 'Rate limit exceeded' }), { status: 429 })
139+
)
140+
141+
const response = await GET(request('pdf'), context)
142+
143+
expect(response.status).toBe(429)
144+
expect(mockDownloadFile).not.toHaveBeenCalled()
145+
expect(mockRenderMarkdownPdf).not.toHaveBeenCalled()
118146
})
119147

120148
it('passes only authorized, readable embedded images to the PDF renderer', async () => {
@@ -134,6 +162,37 @@ describe('markdown export bundling', () => {
134162
expect(images.get('good')).toEqual(Buffer.from('png-bytes'))
135163
})
136164

165+
it('records an image-containing PDF as one downloaded file', async () => {
166+
mockExtractEmbeddedImageIds.mockReturnValue(['image-1'])
167+
168+
await GET(request('pdf'), context)
169+
170+
expect(mockRecordAudit).toHaveBeenCalledWith(
171+
expect.objectContaining({
172+
metadata: expect.objectContaining({ assetCount: 1, format: 'pdf' }),
173+
})
174+
)
175+
expect(mockCaptureServerEvent).toHaveBeenCalledWith(
176+
'user-1',
177+
'file_downloaded',
178+
expect.objectContaining({ file_count: 1, is_bulk: false }),
179+
{ groups: { workspace: 'ws-1' } }
180+
)
181+
})
182+
183+
it('keeps image-containing ZIP telemetry bulk', async () => {
184+
mockExtractEmbeddedImageIds.mockReturnValue(['image-1'])
185+
186+
await GET(request(), context)
187+
188+
expect(mockCaptureServerEvent).toHaveBeenCalledWith(
189+
'user-1',
190+
'file_downloaded',
191+
expect.objectContaining({ file_count: 2, is_bulk: true }),
192+
{ groups: { workspace: 'ws-1' } }
193+
)
194+
})
195+
137196
it('rejects PDF format for a non-Markdown file', async () => {
138197
mockGetFileMetadataById.mockResolvedValue({
139198
id: DOC_ID,
@@ -197,6 +256,25 @@ describe('markdown export bundling', () => {
197256
expect(bodyCall?.[0].maxBytes).toBe(250 * MB)
198257
})
199258

259+
it('uses a smaller document limit for PDF rendering', async () => {
260+
await GET(request('pdf'), context)
261+
262+
const bodyCall = mockDownloadFile.mock.calls.find(([options]) => options.key.endsWith('doc.md'))
263+
expect(bodyCall?.[0].maxBytes).toBe(256 * 1024)
264+
})
265+
266+
it('reports an oversized PDF body with the PDF-specific limit', async () => {
267+
mockDownloadFile.mockRejectedValue(
268+
new PayloadSizeLimitError({ label: 'storage file download', maxBytes: 1 })
269+
)
270+
271+
const response = await GET(request('pdf'), context)
272+
273+
expect(response.status).toBe(400)
274+
expect((await response.json()).error).toContain('256 KB PDF export limit')
275+
expect(mockRenderMarkdownPdf).not.toHaveBeenCalled()
276+
})
277+
200278
it('reports an oversized body as a size rejection, not a server error', async () => {
201279
mockExtractEmbeddedImageIds.mockReturnValue([])
202280
mockDownloadFile.mockRejectedValue(
@@ -221,6 +299,41 @@ describe('markdown export bundling', () => {
221299
expect(assetCall?.[0].maxBytes).toBe(25 * MB)
222300
})
223301

302+
it('uses a smaller per-asset limit for PDF rendering', async () => {
303+
mockExtractEmbeddedImageIds.mockReturnValue(['a'])
304+
305+
await GET(request('pdf'), context)
306+
307+
const assetCall = mockDownloadFile.mock.calls.find(
308+
([options]) => options.key === 'workspace/ws-1/a'
309+
)
310+
expect(assetCall?.[0].maxBytes).toBe(10 * MB)
311+
})
312+
313+
it('rejects PDF source material above its aggregate input limit', async () => {
314+
mockExtractEmbeddedImageIds.mockReturnValue(['a', 'b'])
315+
mockGetFileMetadataById.mockImplementation(async (id: string) =>
316+
id === DOC_ID
317+
? {
318+
id: DOC_ID,
319+
key: 'workspace/ws-1/doc.md',
320+
originalName: 'doc.md',
321+
contentType: 'text/markdown',
322+
context: 'workspace',
323+
size: 1024,
324+
workspaceId: 'ws-1',
325+
}
326+
: assetRecord(id, 30 * MB)
327+
)
328+
329+
const response = await GET(request('pdf'), context)
330+
331+
expect(response.status).toBe(400)
332+
expect((await response.json()).error).toContain('50 MB PDF export limit')
333+
expect(mockDownloadFile).toHaveBeenCalledTimes(1)
334+
expect(mockRenderMarkdownPdf).not.toHaveBeenCalled()
335+
})
336+
224337
it('drops an unreadable asset instead of failing the whole export', async () => {
225338
mockExtractEmbeddedImageIds.mockReturnValue(['good', 'bad'])
226339
mockDownloadFile.mockImplementation(async ({ key }: { key: string }) => {

0 commit comments

Comments
 (0)