From c5711155f410ec6dacc539ac5e9a48185ef2386c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 11:59:43 -0700 Subject: [PATCH 01/10] improvement(copilot): bound image decode work on the VFS file read path Apply explicit size and pixel limits when preparing workspace images for vision, and carry the specific rejection reason through to the read placeholder instead of a generic message. - cap the source image by byte size, enforced by the download itself - give sharp an explicit pixel budget instead of disabling its own - refuse oversized dimensions before the resize ladder runs - retry a failed resize at a smaller dimension rather than re-running the identical decode at each quality step - pass maxBytes on the text and document read paths too --- apps/sim/lib/copilot/vfs/file-reader.test.ts | 80 +++++++++-- apps/sim/lib/copilot/vfs/file-reader.ts | 136 +++++++++++++++---- apps/sim/lib/uploads/server/heic.ts | 9 +- 3 files changed, 176 insertions(+), 49 deletions(-) diff --git a/apps/sim/lib/copilot/vfs/file-reader.test.ts b/apps/sim/lib/copilot/vfs/file-reader.test.ts index 5f070a01c06..68f320c8469 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.test.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.test.ts @@ -3,7 +3,8 @@ */ import { randomFillSync } from 'node:crypto' -import { describe, expect, it, vi } from 'vitest' +import { crc32 } from 'node:zlib' +import { beforeEach, describe, expect, it, vi } from 'vitest' const { fetchWorkspaceFileBuffer } = vi.hoisted(() => ({ fetchWorkspaceFileBuffer: vi.fn(), @@ -16,6 +17,7 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ import { readFileRecord } from '@/lib/copilot/vfs/file-reader' const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 +const MAX_IMAGE_SOURCE_BYTES = 25 * 1024 * 1024 async function makeNoisePng(width: number, height: number): Promise { const sharp = (await import('sharp')).default @@ -26,9 +28,71 @@ async function makeNoisePng(width: number, height: number): Promise { .toBuffer() } +/** + * A decompression bomb: a few hundred bytes on the wire declaring a raster far too + * large to decode. Built by rewriting the IHDR dimensions of a real PNG rather than + * by rendering one, because rendering the raster is the very cost under test. + */ +async function makeBombPng(width: number, height: number): Promise { + const sharp = (await import('sharp')).default + const png = await sharp({ create: { width: 1, height: 1, channels: 3, background: '#fff' } }) + .png() + .toBuffer() + png.writeUInt32BE(width, 16) + png.writeUInt32BE(height, 20) + // IHDR's CRC covers the chunk type and data — bytes 12..29 of a PNG. + png.writeUInt32BE(crc32(png.subarray(12, 29)), 29) + return png +} + +function imageRecord(name: string, size: number, type = 'image/png') { + return { + id: 'wf_img', + workspaceId: 'ws_1', + name, + key: `uploads/${name}`, + path: `/api/files/serve/uploads%2F${name}?context=mothership`, + size, + type, + uploadedBy: 'user_1', + uploadedAt: new Date(), + deletedAt: null, + storageContext: 'mothership' as const, + } +} + const SHARP_TEST_TIMEOUT_MS = 30_000 describe('readFileRecord', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it( + 'rejects a decompression bomb without decoding its raster', + async () => { + // 9e8 pixels — ~3.6GB once decoded as RGBA. + const bomb = await makeBombPng(30_000, 30_000) + expect(bomb.length).toBeLessThan(MAX_IMAGE_READ_BYTES) + + fetchWorkspaceFileBuffer.mockResolvedValue(bomb) + + const result = await readFileRecord(imageRecord('bomb.png', bomb.length)) + + expect(result?.attachment).toBeUndefined() + expect(result?.content).toContain('It is too large to decode safely.') + }, + SHARP_TEST_TIMEOUT_MS + ) + + it('rejects an oversized image on its stored size before fetching it', async () => { + const result = await readFileRecord(imageRecord('huge.png', MAX_IMAGE_SOURCE_BYTES + 1)) + + expect(fetchWorkspaceFileBuffer).not.toHaveBeenCalled() + expect(result?.attachment).toBeUndefined() + expect(result?.content).toContain('Image too large to read inline') + }) + it( 'downscales oversized images into attachments that fit the read limit', async () => { @@ -37,19 +101,7 @@ describe('readFileRecord', () => { fetchWorkspaceFileBuffer.mockResolvedValue(largePng) - const result = await readFileRecord({ - id: 'wf_large', - workspaceId: 'ws_1', - name: 'chesspng.png', - key: 'uploads/chesspng.png', - path: '/api/files/serve/uploads%2Fchesspng.png?context=mothership', - size: largePng.length, - type: 'image/png', - uploadedBy: 'user_1', - uploadedAt: new Date(), - deletedAt: null, - storageContext: 'mothership', - }) + const result = await readFileRecord(imageRecord('chesspng.png', largePng.length)) expect(result?.attachment?.type).toBe('image') expect(result?.content).toContain('resized for vision') diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index e23c647b77c..3cda1cb8e0e 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -16,6 +16,7 @@ import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/works import { fetchWorkspaceFileBuffer } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { isHeifContainer, transcodeHeicToJpeg } from '@/lib/uploads/server/heic' import { + formatFileSize, isImageFileType, MODEL_SUPPORTED_IMAGE_MIME_TYPES, resolveEffectiveMimeType, @@ -39,6 +40,15 @@ const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 // 5 MB // produce huge extracted text; reject up front to avoid wasting a // download + parse only to blow past the tool-result budget. const MAX_PARSEABLE_READ_BYTES = 5 * 1024 * 1024 // 5 MB +/** Source-image byte ceiling. Sits above the 20MB HEIC transcode ceiling, so a HEIF is bounded by the tighter of the two. */ +const MAX_IMAGE_SOURCE_BYTES = 25 * 1024 * 1024 +/** + * Pixel ceiling on the decoded raster. libvips materialises the whole raster, and + * an allocation this large OOM-kills the process rather than throwing, so it has to + * be refused up front. 100MP caps the decode near 400MB while clearing every real + * camera — a 48MP iPhone still is 8064x6048. + */ +const MAX_IMAGE_INPUT_PIXELS = 100_000_000 const MAX_IMAGE_DIMENSION = 1568 const IMAGE_RESIZE_DIMENSIONS = [1568, 1280, 1024, 768] const IMAGE_QUALITY_STEPS = [85, 70, 55, 40] @@ -85,6 +95,20 @@ interface PreparedVisionImage { resized: boolean } +/** + * Shown to the model verbatim in the read placeholder, so each value names the one + * thing that actually failed rather than a disjunction of everything that might have. + */ +const VisionImageRejection = { + Undecodable: 'It could not be decoded.', + TooManyPixels: 'It is too large to decode safely.', + TooLargeAfterResize: 'It still exceeded the 5MB vision limit after resizing.', +} as const + +type VisionImageResult = + | { ok: true; image: PreparedVisionImage } + | { ok: false; reason: (typeof VisionImageRejection)[keyof typeof VisionImageRejection] } + /** * Prepare an image for vision models: detect media type, optionally * resize/compress with sharp, and return the prepared buffer. @@ -98,7 +122,7 @@ interface PreparedVisionImage { async function prepareImageForVision( sourceBuffer: Buffer, claimedType: string -): Promise { +): Promise { return getVfsTracer().startActiveSpan( TraceSpan.CopilotVfsPrepareImage, { @@ -107,7 +131,7 @@ async function prepareImageForVision( [TraceAttr.CopilotVfsInputMediaTypeClaimed]: claimedType, }, }, - async (span) => { + async (span): Promise => { try { const detectedType = detectImageMime(sourceBuffer, claimedType) span.setAttribute(TraceAttr.CopilotVfsInputMediaTypeDetected, detectedType) @@ -128,11 +152,17 @@ async function prepareImageForVision( TraceAttr.CopilotVfsOutcome, fitsWithoutSharp ? 'passthrough_no_sharp' : 'rejected_no_sharp' ) - return fitsWithoutSharp - ? { buffer: sourceBuffer, mediaType: detectedType, resized: false } - : null + if (!fitsWithoutSharp) return { ok: false, reason: VisionImageRejection.Undecodable } + return { + ok: true, + image: { buffer: sourceBuffer, mediaType: detectedType, resized: false }, + } } + // Left unguarded deliberately: metadata() only parses the header, so it + // allocates nothing proportional to the declared dimensions, and enabling the + // guard here would route an oversized image into the passthrough branch below + // — which hands the bytes to the model instead of refusing them. const readMetadata = (candidate: Buffer) => sharpModule(candidate, { limitInputPixels: false }) .metadata() @@ -171,7 +201,8 @@ async function prepareImageForVision( TraceAttr.CopilotVfsOutcome, passthroughViable ? 'passthrough_no_metadata' : 'rejected_no_metadata' ) - return passthroughViable ? { buffer, mediaType, resized: false } : null + if (!passthroughViable) return { ok: false, reason: VisionImageRejection.Undecodable } + return { ok: true, image: { buffer, mediaType, resized: false } } } const width = metadata.width ?? 0 @@ -181,6 +212,20 @@ async function prepareImageForVision( [TraceAttr.CopilotVfsInputHeight]: height, }) + const pixels = width * height + if (pixels > MAX_IMAGE_INPUT_PIXELS) { + logger.warn('Rejected image above the decode pixel budget', { + mediaType, + width, + height, + pixels, + budget: MAX_IMAGE_INPUT_PIXELS, + bytes: buffer.length, + }) + span.setAttribute(TraceAttr.CopilotVfsOutcome, 'rejected_pixel_budget') + return { ok: false, reason: VisionImageRejection.TooManyPixels } + } + // A format the model cannot decode has to be re-encoded even when it is // already small enough — the ladder below emits JPEG or WebP, both of // which it accepts. @@ -196,7 +241,7 @@ async function prepareImageForVision( [TraceAttr.CopilotVfsOutputBytes]: buffer.length, [TraceAttr.CopilotVfsOutputMediaType]: mediaType, }) - return { buffer, mediaType, resized: false } + return { ok: true, image: { buffer, mediaType, resized: false } } } const hasAlpha = Boolean( @@ -208,16 +253,19 @@ async function prepareImageForVision( span.setAttribute(TraceAttr.CopilotVfsHasAlpha, hasAlpha) let attempts = 0 + let decodeFailed = false for (const dimension of IMAGE_RESIZE_DIMENSIONS) { for (const quality of IMAGE_QUALITY_STEPS) { attempts += 1 try { - const pipeline = sharpModule(buffer, { limitInputPixels: false }).rotate().resize({ - width: dimension, - height: dimension, - fit: 'inside', - withoutEnlargement: true, - }) + const pipeline = sharpModule(buffer, { limitInputPixels: MAX_IMAGE_INPUT_PIXELS }) + .rotate() + .resize({ + width: dimension, + height: dimension, + fit: 'inside', + withoutEnlargement: true, + }) const transformed = hasAlpha ? { @@ -262,12 +310,20 @@ async function prepareImageForVision( [TraceAttr.CopilotVfsOutcome]: CopilotVfsOutcome.Resized, }) return { - buffer: transformed.buffer, - mediaType: transformed.mediaType, - resized: true, + ok: true, + image: { + buffer: transformed.buffer, + mediaType: transformed.mediaType, + resized: true, + }, } } } catch (err) { + // Move to the next dimension rather than the next quality: the quality + // rungs re-decode the identical source and only change the encoder, so + // repeating a failed decode there is pure waste. A smaller dimension is + // worth trying — libvips shrinks JPEG on load, so it decodes less. + decodeFailed = true logger.warn('Failed image resize attempt for VFS read', { mediaType, dimension, @@ -279,6 +335,7 @@ async function prepareImageForVision( [TraceAttr.CopilotVfsResizeQuality]: quality, [TraceAttr.ErrorMessage]: toError(err).message.slice(0, 500), }) + break } } } @@ -288,7 +345,12 @@ async function prepareImageForVision( [TraceAttr.CopilotVfsResizeAttempts]: attempts, [TraceAttr.CopilotVfsOutcome]: CopilotVfsOutcome.RejectedTooLargeAfterResize, }) - return null + return { + ok: false, + reason: decodeFailed + ? VisionImageRejection.Undecodable + : VisionImageRejection.TooLargeAfterResize, + } } catch (err) { recordSpanError(span, err) throw err @@ -342,33 +404,45 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise MAX_IMAGE_SOURCE_BYTES) { + span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) + return { + content: `[Image too large to read inline: ${record.name} (${record.size} bytes, limit ${MAX_IMAGE_SOURCE_BYTES})]`, + totalLines: 1, + } + } + const originalBuffer = await fetchWorkspaceFileBuffer(record, { + maxBytes: MAX_IMAGE_SOURCE_BYTES, + }) const prepared = await prepareImageForVision(originalBuffer, record.type) - if (!prepared) { + if (!prepared.ok) { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) return { - content: `[Image unavailable: ${record.name} (${(record.size / 1024 / 1024).toFixed(1)}MB). It could not be decoded, or still exceeded the 5MB vision limit after resizing.]`, + content: `[Image unavailable: ${record.name} (${formatFileSize(record.size)}). ${prepared.reason}]`, totalLines: 1, } } - const sizeKb = (prepared.buffer.length / 1024).toFixed(1) - const resizeNote = prepared.resized ? ', resized for vision' : '' + const { buffer, mediaType, resized } = prepared.image + const sizeKb = (buffer.length / 1024).toFixed(1) + const resizeNote = resized ? ', resized for vision' : '' span.setAttributes({ [TraceAttr.CopilotVfsReadOutcome]: CopilotVfsReadOutcome.ImagePrepared, - [TraceAttr.CopilotVfsReadOutputBytes]: prepared.buffer.length, - [TraceAttr.CopilotVfsReadOutputMediaType]: prepared.mediaType, - [TraceAttr.CopilotVfsReadImageResized]: prepared.resized, + [TraceAttr.CopilotVfsReadOutputBytes]: buffer.length, + [TraceAttr.CopilotVfsReadOutputMediaType]: mediaType, + [TraceAttr.CopilotVfsReadImageResized]: resized, }) return { - content: `Image: ${record.name} (${sizeKb}KB, ${prepared.mediaType}${resizeNote})`, + content: `Image: ${record.name} (${sizeKb}KB, ${mediaType}${resizeNote})`, totalLines: 1, attachment: { type: 'image', name: record.name, source: { type: 'base64' as const, - media_type: prepared.mediaType, - data: prepared.buffer.toString('base64'), + media_type: mediaType, + data: buffer.toString('base64'), }, }, } @@ -384,7 +458,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise Date: Sat, 8 Aug 2026 12:08:23 -0700 Subject: [PATCH 02/10] fix(copilot): keep the too-large placeholder when the download cap trips The recorded size is client-declared, so the download's maxBytes is the check that actually holds. Breaching it threw past the placeholder and surfaced as a failed read on all three capped paths. Rethrow PayloadSizeLimitError unwrapped from fetchWorkspaceFileBuffer so callers can tell a size breach from a transport error, and answer with the same too-large placeholder the recorded-size check already returns. --- apps/sim/lib/copilot/vfs/file-reader.test.ts | 14 ++++ apps/sim/lib/copilot/vfs/file-reader.ts | 81 ++++++++++++++----- .../workspace/workspace-file-manager.ts | 5 ++ 3 files changed, 79 insertions(+), 21 deletions(-) diff --git a/apps/sim/lib/copilot/vfs/file-reader.test.ts b/apps/sim/lib/copilot/vfs/file-reader.test.ts index 68f320c8469..0f64b198935 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.test.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.test.ts @@ -15,6 +15,7 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ })) import { readFileRecord } from '@/lib/copilot/vfs/file-reader' +import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 const MAX_IMAGE_SOURCE_BYTES = 25 * 1024 * 1024 @@ -85,6 +86,19 @@ describe('readFileRecord', () => { SHARP_TEST_TIMEOUT_MS ) + it('reports the too-large placeholder when a understated record.size hides an oversized object', async () => { + // `record.size` is client-declared, so the download cap is the check that holds — + // and breaching it must still read as "too large", not as a failed read. + fetchWorkspaceFileBuffer.mockRejectedValue( + new PayloadSizeLimitError({ label: 'workspace file', maxBytes: MAX_IMAGE_SOURCE_BYTES }) + ) + + const result = await readFileRecord(imageRecord('understated.png', 1024)) + + expect(result?.attachment).toBeUndefined() + expect(result?.content).toContain('Image too large to read inline') + }) + it('rejects an oversized image on its stored size before fetching it', async () => { const result = await readFileRecord(imageRecord('huge.png', MAX_IMAGE_SOURCE_BYTES + 1)) diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index 3cda1cb8e0e..293bb84a6c8 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -12,6 +12,7 @@ import { TraceEvent } from '@/lib/copilot/generated/trace-events-v1' import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1' import { recordFileRead } from '@/lib/copilot/request/metrics' import { markSpanForError } from '@/lib/copilot/request/otel' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { fetchWorkspaceFileBuffer } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { isHeifContainer, transcodeHeicToJpeg } from '@/lib/uploads/server/heic' @@ -79,6 +80,29 @@ function getExtension(filename: string): string { return dot >= 0 ? filename.slice(dot + 1).toLowerCase() : '' } +/** + * Download a record under an authoritative byte cap, returning null when the stored + * object breaches it. `record.size` is client-declared, so a caller's own size check + * can pass while the real bytes do not — this is the check that actually holds, and + * null lets the caller answer with its too-large placeholder rather than a read failure. + */ +async function fetchWithinLimit( + record: WorkspaceFileRecord, + maxBytes: number +): Promise { + try { + return await fetchWorkspaceFileBuffer(record, { maxBytes }) + } catch (err) { + if (!isPayloadSizeLimitError(err)) throw err + logger.warn('Workspace file exceeded its read cap', { + fileName: record.name, + recordedSize: record.size, + maxBytes, + }) + return null + } +} + function detectImageMime(buf: Buffer, claimed: string): string { if (buf.length < 12) return claimed if (buf[0] === 0xff && buf[1] === 0xd8 && buf[2] === 0xff) return 'image/jpeg' @@ -404,18 +428,22 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise MAX_IMAGE_SOURCE_BYTES) { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) - return { - content: `[Image too large to read inline: ${record.name} (${record.size} bytes, limit ${MAX_IMAGE_SOURCE_BYTES})]`, - totalLines: 1, - } + return imageTooLarge + } + const originalBuffer = await fetchWithinLimit(record, MAX_IMAGE_SOURCE_BYTES) + if (!originalBuffer) { + span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) + return imageTooLarge } - const originalBuffer = await fetchWorkspaceFileBuffer(record, { - maxBytes: MAX_IMAGE_SOURCE_BYTES, - }) const prepared = await prepareImageForVision(originalBuffer, record.type) if (!prepared.ok) { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) @@ -450,15 +478,20 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise MAX_TEXT_READ_BYTES) { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.TextTooLarge) - return { - content: `[File too large to display inline: ${record.name} (${record.size} bytes, limit ${MAX_TEXT_READ_BYTES})]`, - totalLines: 1, - } + return textTooLarge } - const buffer = await fetchWorkspaceFileBuffer(record, { maxBytes: MAX_TEXT_READ_BYTES }) + const buffer = await fetchWithinLimit(record, MAX_TEXT_READ_BYTES) + if (!buffer) { + span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.TextTooLarge) + return textTooLarge + } const content = buffer.toString('utf-8') const lines = content.split('\n').length span.setAttributes({ @@ -472,19 +505,25 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise MAX_PARSEABLE_READ_BYTES) { span.setAttribute( TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.DocumentTooLarge ) - return { - content: `[Document too large to parse inline: ${record.name} (${record.size} bytes, limit ${MAX_PARSEABLE_READ_BYTES})]`, - totalLines: 1, - } + return documentTooLarge + } + const buffer = await fetchWithinLimit(record, MAX_PARSEABLE_READ_BYTES) + if (!buffer) { + span.setAttribute( + TraceAttr.CopilotVfsReadOutcome, + CopilotVfsReadOutcome.DocumentTooLarge + ) + return documentTooLarge } - const buffer = await fetchWorkspaceFileBuffer(record, { - maxBytes: MAX_PARSEABLE_READ_BYTES, - }) try { const { parseBuffer } = await import('@/lib/file-parsers') const result = await parseBuffer(buffer, ext) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts index 4bae36791dc..dbe1a921bbc 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -26,6 +26,7 @@ import { normalizeVfsSegment } from '@/lib/copilot/vfs/normalize-segment' import { canonicalWorkspaceFilePath, decodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils' import { generateRequestId } from '@/lib/core/utils/request' import { generateRestoreName } from '@/lib/core/utils/restore-name' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import type { DbOrTx } from '@/lib/db/types' import { mergeEditIntoLiveFileDoc, notifyWorkspaceFilesChanged } from '@/lib/realtime/notify' import { getServePathPrefix } from '@/lib/uploads' @@ -1251,6 +1252,10 @@ export async function fetchWorkspaceFileBuffer( return buffer } catch (error) { logger.error(`Failed to download workspace file ${fileRecord.name}:`, error) + // Rethrow a `maxBytes` breach unwrapped: callers distinguish "too large" from a + // transport failure to answer with their own placeholder, and re-wrapping it in a + // plain Error would erase the only thing that tells the two apart. + if (isPayloadSizeLimitError(error)) throw error throw new Error(`Failed to download file: ${getErrorMessage(error, 'Unknown error')}`) } } From f4cb8589fd450c7761c18e2bd774b0cd56b7a05e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 12:16:16 -0700 Subject: [PATCH 03/10] fix(copilot): report the accurate reason and byte size on image rejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - track whether any resize rung produced an encode, rather than whether any threw: a rung that throws followed by rungs that encode but never fit was reported as undecodable, and the span outcome disagreed with the placeholder - format the rejected size with includeBytes, since formatFileSize collapses anything under 1KB to '0 Bytes' — which is every decompression bomb --- apps/sim/lib/copilot/vfs/file-reader.test.ts | 3 +++ apps/sim/lib/copilot/vfs/file-reader.ts | 19 ++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/copilot/vfs/file-reader.test.ts b/apps/sim/lib/copilot/vfs/file-reader.test.ts index 0f64b198935..90f4e42d52b 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.test.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.test.ts @@ -82,6 +82,9 @@ describe('readFileRecord', () => { expect(result?.attachment).toBeUndefined() expect(result?.content).toContain('It is too large to decode safely.') + // The byte count must survive formatting — a sub-1KB bomb formatted without + // `includeBytes` collapses to "0 Bytes" next to the real reason. + expect(result?.content).toContain(`(${bomb.length} Bytes)`) }, SHARP_TEST_TIMEOUT_MS ) diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index 293bb84a6c8..daf238593b0 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -277,7 +277,10 @@ async function prepareImageForVision( span.setAttribute(TraceAttr.CopilotVfsHasAlpha, hasAlpha) let attempts = 0 - let decodeFailed = false + // Whether any rung got as far as producing an encoded buffer. That, not + // "did anything throw", is what separates "cannot be decoded at all" from + // "decodes fine, just never small enough". + let encodedAny = false for (const dimension of IMAGE_RESIZE_DIMENSIONS) { for (const quality of IMAGE_QUALITY_STEPS) { attempts += 1 @@ -305,6 +308,7 @@ async function prepareImageForVision( mediaType: 'image/jpeg', } + encodedAny = true span.addEvent(TraceEvent.CopilotVfsResizeAttempt, { [TraceAttr.CopilotVfsResizeDimension]: dimension, [TraceAttr.CopilotVfsResizeQuality]: quality, @@ -347,7 +351,6 @@ async function prepareImageForVision( // rungs re-decode the identical source and only change the encoder, so // repeating a failed decode there is pure waste. A smaller dimension is // worth trying — libvips shrinks JPEG on load, so it decodes less. - decodeFailed = true logger.warn('Failed image resize attempt for VFS read', { mediaType, dimension, @@ -367,13 +370,15 @@ async function prepareImageForVision( span.setAttributes({ [TraceAttr.CopilotVfsResized]: false, [TraceAttr.CopilotVfsResizeAttempts]: attempts, - [TraceAttr.CopilotVfsOutcome]: CopilotVfsOutcome.RejectedTooLargeAfterResize, + [TraceAttr.CopilotVfsOutcome]: encodedAny + ? CopilotVfsOutcome.RejectedTooLargeAfterResize + : 'rejected_resize_failed', }) return { ok: false, - reason: decodeFailed - ? VisionImageRejection.Undecodable - : VisionImageRejection.TooLargeAfterResize, + reason: encodedAny + ? VisionImageRejection.TooLargeAfterResize + : VisionImageRejection.Undecodable, } } catch (err) { recordSpanError(span, err) @@ -448,7 +453,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise Date: Sat, 8 Aug 2026 12:28:03 -0700 Subject: [PATCH 04/10] fix(copilot): match the oversized-image placeholder in the read-size gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isOversizedReadPlaceholder tested for '[Image too large:', a prefix no code emitted — the image path's only placeholder was '[Image unavailable:'. The new source-cap placeholder is the first real one, so point the gate at it and fix the synthetic string in the test to match what the reader actually returns. --- apps/sim/lib/copilot/tools/handlers/vfs.test.ts | 2 +- apps/sim/lib/copilot/tools/handlers/vfs.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts index 5edb62a789c..9015a35584c 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts @@ -182,7 +182,7 @@ describe('vfs handlers oversize policy', () => { it('fails oversized image placeholder when image exceeds size limit', async () => { const vfs = makeVfs() vfs.readFileContent.mockResolvedValue({ - content: '[Image too large: huge.png (10.0MB, limit 5MB)]', + content: '[Image too large to read inline: huge.png (26214401 bytes, limit 26214400)]', totalLines: 1, }) getOrMaterializeVFS.mockResolvedValue(vfs) diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.ts b/apps/sim/lib/copilot/tools/handlers/vfs.ts index b70cd2d9a23..3402f3325d7 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.ts @@ -79,7 +79,7 @@ function serializedResultSize(value: unknown): number { function isOversizedReadPlaceholder(content: string): boolean { return ( content.startsWith('[File too large to display inline:') || - content.startsWith('[Image too large:') || + content.startsWith('[Image too large to read inline:') || content.startsWith('[Compiled artifact too large:') ) } From 7888d3d0455b66e2bed9d0fec02b9a1637e95c21 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 13:50:30 -0700 Subject: [PATCH 05/10] improvement(copilot): align the VFS read caps and share the read placeholders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up review of the bounded image read. Three consistency defects and one shared-definition gap, none of which changed what the limits protect against. - derive the image source cap from the FormData upload ceiling instead of a number of its own. That cap exists for the same failure mode — a route holding an entire file in worker memory — so anything uploadable stays readable, and the tighter value was refusing images that read fine before for no gain - report a HEIF past the WebAssembly transcoder's own ceiling as a size refusal; it was falling through and telling the model the file could not be decoded - classify an oversized document like an oversized file or image. One of the three size refusals was reported to the model as a successful one-line read - report the observed size, not the recorded one, when the download cap trips — the recorded size is the figure that cap exists to distrust - move the read placeholders into one module that builds and matches them. Producers and matchers sat in four files and had already drifted twice - stop emitting trace outcomes absent from the generated contract, and derive the "vision limit" wording from the constant instead of restating it --- .../lib/copilot/tools/handlers/vfs.test.ts | 18 ++ apps/sim/lib/copilot/tools/handlers/vfs.ts | 9 +- apps/sim/lib/copilot/vfs/file-reader.test.ts | 35 +++- apps/sim/lib/copilot/vfs/file-reader.ts | 182 ++++++++++-------- apps/sim/lib/copilot/vfs/operations.test.ts | 26 ++- apps/sim/lib/copilot/vfs/operations.ts | 13 +- apps/sim/lib/copilot/vfs/read-placeholders.ts | 76 ++++++++ apps/sim/lib/copilot/vfs/workspace-vfs.ts | 7 +- apps/sim/lib/uploads/server/heic.ts | 2 +- 9 files changed, 259 insertions(+), 109 deletions(-) create mode 100644 apps/sim/lib/copilot/vfs/read-placeholders.ts diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts index 9015a35584c..fd229f30830 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts @@ -196,6 +196,24 @@ describe('vfs handlers oversize policy', () => { expect(result.error).toContain('too large') }) + it('returns an undecodable image placeholder as content, not as a size failure', async () => { + const vfs = makeVfs() + // Not a size problem — the bytes were read fine and the reason is already in the + // message, so the model should see it rather than a "too large, use grep" error. + vfs.readFileContent.mockResolvedValue({ + content: '[Image unavailable: bomb.png (90 Bytes). It is too large to decode safely.]', + totalLines: 1, + }) + getOrMaterializeVFS.mockResolvedValue(vfs) + + const result = await executeVfsRead( + { path: 'files/bomb.png/content' }, + { userId: 'user-1', workflowId: 'wf-1', workspaceId: 'ws-1' } + ) + + expect(result.success).toBe(true) + }) + it('reads canonical file leaf metadata without fetching dynamic content', async () => { const vfs = makeVfs() vfs.read.mockReturnValue({ diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.ts b/apps/sim/lib/copilot/tools/handlers/vfs.ts index 3402f3325d7..67e04629667 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.ts @@ -8,6 +8,7 @@ import { getOrMaterializeVFS } from '@/lib/copilot/vfs' import type { GrepCountEntry, GrepMatch } from '@/lib/copilot/vfs/operations' import { WorkspaceFileGrepError } from '@/lib/copilot/vfs/operations' import { encodeVfsSegment } from '@/lib/copilot/vfs/path-utils' +import { isOversizedReadPlaceholder } from '@/lib/copilot/vfs/read-placeholders' import { importWorkspaceFileSecretProvenanceForModelView, type WorkspaceFileSecretProvenanceIdentity, @@ -76,14 +77,6 @@ function serializedResultSize(value: unknown): number { } } -function isOversizedReadPlaceholder(content: string): boolean { - return ( - content.startsWith('[File too large to display inline:') || - content.startsWith('[Image too large to read inline:') || - content.startsWith('[Compiled artifact too large:') - ) -} - function hasModelAttachment(result: unknown): boolean { if (!result || typeof result !== 'object') { return false diff --git a/apps/sim/lib/copilot/vfs/file-reader.test.ts b/apps/sim/lib/copilot/vfs/file-reader.test.ts index 90f4e42d52b..4b52dacb0b0 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.test.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.test.ts @@ -14,11 +14,13 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ fetchWorkspaceFileBuffer, })) -import { readFileRecord } from '@/lib/copilot/vfs/file-reader' +import { + MAX_IMAGE_READ_BYTES, + MAX_IMAGE_SOURCE_BYTES, + readFileRecord, +} from '@/lib/copilot/vfs/file-reader' import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' - -const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 -const MAX_IMAGE_SOURCE_BYTES = 25 * 1024 * 1024 +import { MAX_TRANSCODE_INPUT_BYTES } from '@/lib/uploads/server/heic' async function makeNoisePng(width: number, height: number): Promise { const sharp = (await import('sharp')).default @@ -89,17 +91,34 @@ describe('readFileRecord', () => { SHARP_TEST_TIMEOUT_MS ) - it('reports the too-large placeholder when a understated record.size hides an oversized object', async () => { - // `record.size` is client-declared, so the download cap is the check that holds — - // and breaching it must still read as "too large", not as a failed read. + it('reports the too-large placeholder when an understated record.size hides an oversized object', async () => { fetchWorkspaceFileBuffer.mockRejectedValue( - new PayloadSizeLimitError({ label: 'workspace file', maxBytes: MAX_IMAGE_SOURCE_BYTES }) + new PayloadSizeLimitError({ + label: 'workspace file', + maxBytes: MAX_IMAGE_SOURCE_BYTES, + observedBytes: MAX_IMAGE_SOURCE_BYTES + 5_000, + }) ) const result = await readFileRecord(imageRecord('understated.png', 1024)) expect(result?.attachment).toBeUndefined() expect(result?.content).toContain('Image too large to read inline') + // The observed size, not the understated 1024 the cap exists to distrust. + expect(result?.content).toContain(`${MAX_IMAGE_SOURCE_BYTES + 5_000} bytes`) + }) + + it('reports an oversized HEIF as a size refusal, not as a corrupt file', async () => { + // `ftyp`+`heic` brand, past the WebAssembly transcoder's own tighter ceiling. + const heif = Buffer.alloc(MAX_TRANSCODE_INPUT_BYTES + 1) + heif.write('ftypheic', 4, 'ascii') + fetchWorkspaceFileBuffer.mockResolvedValue(heif) + + const result = await readFileRecord(imageRecord('photo.heic', heif.length, 'image/heic')) + + expect(result?.attachment).toBeUndefined() + expect(result?.content).toContain('It is too large to decode safely.') + expect(result?.content).not.toContain('It could not be decoded.') }) it('rejects an oversized image on its stored size before fetching it', async () => { diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index daf238593b0..83267d0ac75 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -12,10 +12,16 @@ import { TraceEvent } from '@/lib/copilot/generated/trace-events-v1' import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1' import { recordFileRead } from '@/lib/copilot/request/metrics' import { markSpanForError } from '@/lib/copilot/request/otel' +import { readPlaceholder } from '@/lib/copilot/vfs/read-placeholders' import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { fetchWorkspaceFileBuffer } from '@/lib/uploads/contexts/workspace/workspace-file-manager' -import { isHeifContainer, transcodeHeicToJpeg } from '@/lib/uploads/server/heic' +import { + isHeifContainer, + MAX_TRANSCODE_INPUT_BYTES, + transcodeHeicToJpeg, +} from '@/lib/uploads/server/heic' +import { MAX_WORKSPACE_FORMDATA_FILE_SIZE } from '@/lib/uploads/shared/types' import { formatFileSize, isImageFileType, @@ -36,13 +42,25 @@ const logger = createLogger('FileReader') /** Inline text-read cap — exported so callers can align their own byte-sniff budgets with what read() can actually display. */ export const MAX_TEXT_READ_BYTES = 5 * 1024 * 1024 // 5 MB -const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 // 5 MB +/** Vision-attachment cap: what the prepared image must fit into after resizing. */ +export const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 // 5 MB // Parseable-document byte cap. Large office/PDF files can still // produce huge extracted text; reject up front to avoid wasting a // download + parse only to blow past the tool-result budget. const MAX_PARSEABLE_READ_BYTES = 5 * 1024 * 1024 // 5 MB -/** Source-image byte ceiling. Sits above the 20MB HEIC transcode ceiling, so a HEIF is bounded by the tighter of the two. */ -const MAX_IMAGE_SOURCE_BYTES = 25 * 1024 * 1024 +/** + * Source-image byte ceiling, checked before the download and enforced by it. A + * workspace file may be up to {@link MAX_WORKSPACE_FILE_SIZE}, and buffering one of + * those whole would exhaust memory on its own — the pixel budget below bounds the + * decode, not the transfer. + * + * Deliberately the FormData upload ceiling rather than a number of its own: that cap + * exists for exactly this failure mode (a route holding an entire file in worker + * memory), so anything a user could upload through it stays readable here. Picking + * something tighter would refuse images that read fine today for no security gain — + * a decompression bomb is small, and it is the pixel budget that stops it. + */ +export const MAX_IMAGE_SOURCE_BYTES = MAX_WORKSPACE_FORMDATA_FILE_SIZE /** * Pixel ceiling on the decoded raster. libvips materialises the whole raster, and * an allocation this large OOM-kills the process rather than throwing, so it has to @@ -81,25 +99,31 @@ function getExtension(filename: string): string { } /** - * Download a record under an authoritative byte cap, returning null when the stored - * object breaches it. `record.size` is client-declared, so a caller's own size check - * can pass while the real bytes do not — this is the check that actually holds, and - * null lets the caller answer with its too-large placeholder rather than a read failure. + * Download a record under an authoritative byte cap. `record.size` is client-declared, + * so a caller's own size check can pass while the real bytes do not — this is the + * check that actually holds. + * + * On a breach it reports the observed size when the error carries one, because the + * recorded size is exactly the figure this cap exists to distrust: quoting it back + * would print a tiny number beside a much larger limit. */ +type CappedFetch = { buffer: Buffer } | { tooLarge: true; observedBytes?: number } + async function fetchWithinLimit( record: WorkspaceFileRecord, maxBytes: number -): Promise { +): Promise { try { - return await fetchWorkspaceFileBuffer(record, { maxBytes }) + return { buffer: await fetchWorkspaceFileBuffer(record, { maxBytes }) } } catch (err) { if (!isPayloadSizeLimitError(err)) throw err logger.warn('Workspace file exceeded its read cap', { fileName: record.name, recordedSize: record.size, + observedBytes: err.observedBytes, maxBytes, }) - return null + return { tooLarge: true, observedBytes: err.observedBytes } } } @@ -119,19 +143,14 @@ interface PreparedVisionImage { resized: boolean } -/** - * Shown to the model verbatim in the read placeholder, so each value names the one - * thing that actually failed rather than a disjunction of everything that might have. - */ +/** Shown to the model verbatim, so each value names the one thing that failed. */ const VisionImageRejection = { Undecodable: 'It could not be decoded.', - TooManyPixels: 'It is too large to decode safely.', - TooLargeAfterResize: 'It still exceeded the 5MB vision limit after resizing.', -} as const + TooLargeToDecode: 'It is too large to decode safely.', + TooLargeAfterResize: `It still exceeded the ${formatFileSize(MAX_IMAGE_READ_BYTES)} vision limit after resizing.`, +} -type VisionImageResult = - | { ok: true; image: PreparedVisionImage } - | { ok: false; reason: (typeof VisionImageRejection)[keyof typeof VisionImageRejection] } +type VisionImageResult = { ok: true; image: PreparedVisionImage } | { ok: false; reason: string } /** * Prepare an image for vision models: detect media type, optionally @@ -174,7 +193,9 @@ async function prepareImageForVision( sourceBuffer.length <= MAX_IMAGE_READ_BYTES span.setAttribute( TraceAttr.CopilotVfsOutcome, - fitsWithoutSharp ? 'passthrough_no_sharp' : 'rejected_no_sharp' + fitsWithoutSharp + ? CopilotVfsOutcome.PassthroughNoSharp + : CopilotVfsOutcome.RejectedNoSharp ) if (!fitsWithoutSharp) return { ok: false, reason: VisionImageRejection.Undecodable } return { @@ -207,6 +228,16 @@ async function prepareImageForVision( let metadata = await readMetadata(sourceBuffer) if (!metadata && isHeifContainer(sourceBuffer)) { + // The WebAssembly fallback is single-threaded and holds its own ceiling, + // well under the source cap above. Say so rather than letting the transcode + // decline and report the file as corrupt. + if (sourceBuffer.length > MAX_TRANSCODE_INPUT_BYTES) { + logger.warn('Rejected HEIF above the transcode ceiling', { + bytes: sourceBuffer.length, + ceiling: MAX_TRANSCODE_INPUT_BYTES, + }) + return { ok: false, reason: VisionImageRejection.TooLargeToDecode } + } const transcoded = await transcodeHeicToJpeg(sourceBuffer) if (transcoded) { buffer = transcoded @@ -223,7 +254,9 @@ async function prepareImageForVision( MODEL_SUPPORTED_IMAGE_MIME_TYPES.has(mediaType) && buffer.length <= MAX_IMAGE_READ_BYTES span.setAttribute( TraceAttr.CopilotVfsOutcome, - passthroughViable ? 'passthrough_no_metadata' : 'rejected_no_metadata' + passthroughViable + ? CopilotVfsOutcome.PassthroughNoMetadata + : CopilotVfsOutcome.RejectedNoMetadata ) if (!passthroughViable) return { ok: false, reason: VisionImageRejection.Undecodable } return { ok: true, image: { buffer, mediaType, resized: false } } @@ -246,8 +279,11 @@ async function prepareImageForVision( budget: MAX_IMAGE_INPUT_PIXELS, bytes: buffer.length, }) - span.setAttribute(TraceAttr.CopilotVfsOutcome, 'rejected_pixel_budget') - return { ok: false, reason: VisionImageRejection.TooManyPixels } + // No `CopilotVfsOutcome` member covers a pre-decode refusal, and that + // vocabulary is generated from a contract this repo does not own — emitting + // an unlisted value would just be dropped downstream. The dimensions are on + // the span above and the reason is in the warning. + return { ok: false, reason: VisionImageRejection.TooLargeToDecode } } // A format the model cannot decode has to be re-encoded even when it is @@ -277,9 +313,7 @@ async function prepareImageForVision( span.setAttribute(TraceAttr.CopilotVfsHasAlpha, hasAlpha) let attempts = 0 - // Whether any rung got as far as producing an encoded buffer. That, not - // "did anything throw", is what separates "cannot be decoded at all" from - // "decodes fine, just never small enough". + // Separates "cannot be decoded at all" from "decodes fine, never small enough". let encodedAny = false for (const dimension of IMAGE_RESIZE_DIMENSIONS) { for (const quality of IMAGE_QUALITY_STEPS) { @@ -347,10 +381,9 @@ async function prepareImageForVision( } } } catch (err) { - // Move to the next dimension rather than the next quality: the quality - // rungs re-decode the identical source and only change the encoder, so - // repeating a failed decode there is pure waste. A smaller dimension is - // worth trying — libvips shrinks JPEG on load, so it decodes less. + // Next dimension, not next quality: the quality rungs re-decode the + // identical source, so repeating a failed decode there is pure waste. + // A smaller dimension is worth trying — libvips shrinks JPEG on load. logger.warn('Failed image resize attempt for VFS read', { mediaType, dimension, @@ -370,9 +403,7 @@ async function prepareImageForVision( span.setAttributes({ [TraceAttr.CopilotVfsResized]: false, [TraceAttr.CopilotVfsResizeAttempts]: attempts, - [TraceAttr.CopilotVfsOutcome]: encodedAny - ? CopilotVfsOutcome.RejectedTooLargeAfterResize - : 'rejected_resize_failed', + [TraceAttr.CopilotVfsOutcome]: CopilotVfsOutcome.RejectedTooLargeAfterResize, }) return { ok: false, @@ -433,27 +464,24 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise MAX_IMAGE_SOURCE_BYTES) { + const imageTooLarge = (bytes: number) => { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) - return imageTooLarge - } - const originalBuffer = await fetchWithinLimit(record, MAX_IMAGE_SOURCE_BYTES) - if (!originalBuffer) { - span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) - return imageTooLarge + return { + content: readPlaceholder.imageTooLarge(record.name, bytes, MAX_IMAGE_SOURCE_BYTES), + totalLines: 1, + } } - const prepared = await prepareImageForVision(originalBuffer, record.type) + // The recorded size only skips a doomed download; the cap on the download + // itself is what bounds the bytes actually read. + if (record.size > MAX_IMAGE_SOURCE_BYTES) return imageTooLarge(record.size) + const fetched = await fetchWithinLimit(record, MAX_IMAGE_SOURCE_BYTES) + if ('tooLarge' in fetched) return imageTooLarge(fetched.observedBytes ?? record.size) + + const prepared = await prepareImageForVision(fetched.buffer, record.type) if (!prepared.ok) { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) return { - content: `[Image unavailable: ${record.name} (${formatFileSize(record.size, { includeBytes: true })}). ${prepared.reason}]`, + content: readPlaceholder.imageUnavailable(record.name, record.size, prepared.reason), totalLines: 1, } } @@ -483,20 +511,18 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise MAX_TEXT_READ_BYTES) { + const textTooLarge = (bytes: number) => { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.TextTooLarge) - return textTooLarge + return { + content: readPlaceholder.fileTooLarge(record.name, bytes, MAX_TEXT_READ_BYTES), + totalLines: 1, + } } + if (record.size > MAX_TEXT_READ_BYTES) return textTooLarge(record.size) - const buffer = await fetchWithinLimit(record, MAX_TEXT_READ_BYTES) - if (!buffer) { - span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.TextTooLarge) - return textTooLarge - } + const fetched = await fetchWithinLimit(record, MAX_TEXT_READ_BYTES) + if ('tooLarge' in fetched) return textTooLarge(fetched.observedBytes ?? record.size) + const buffer = fetched.buffer const content = buffer.toString('utf-8') const lines = content.split('\n').length span.setAttributes({ @@ -510,28 +536,28 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise MAX_PARSEABLE_READ_BYTES) { + const documentTooLarge = (bytes: number) => { span.setAttribute( TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.DocumentTooLarge ) - return documentTooLarge + return { + content: readPlaceholder.documentTooLarge( + record.name, + bytes, + MAX_PARSEABLE_READ_BYTES + ), + totalLines: 1, + } } - const buffer = await fetchWithinLimit(record, MAX_PARSEABLE_READ_BYTES) - if (!buffer) { - span.setAttribute( - TraceAttr.CopilotVfsReadOutcome, - CopilotVfsReadOutcome.DocumentTooLarge - ) - return documentTooLarge + if (record.size > MAX_PARSEABLE_READ_BYTES) return documentTooLarge(record.size) + const fetched = await fetchWithinLimit(record, MAX_PARSEABLE_READ_BYTES) + if ('tooLarge' in fetched) { + return documentTooLarge(fetched.observedBytes ?? record.size) } try { const { parseBuffer } = await import('@/lib/file-parsers') - const result = await parseBuffer(buffer, ext) + const result = await parseBuffer(fetched.buffer, ext) const content = result.content || '' const lines = content.split('\n').length span.setAttributes({ @@ -551,7 +577,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise { return new Map(entries) @@ -194,3 +194,27 @@ describe('grep regex safety', () => { expect(grep(files, 'alpha')).toHaveLength(1) }) }) + +describe('grepReadResult placeholders', () => { + const grepPlaceholder = (content: string) => + grepReadResult('files/x.png/content', { content, totalLines: 1 }, 'x', 'files/x.png/content') + + /** + * Every `readFileRecord` placeholder carries no searchable text, so grep must + * report the placeholder rather than matching against its own prose. + */ + it.each([ + '[Image unavailable: bomb.png (90 Bytes). It is too large to decode safely.]', + '[Image too large to read inline: huge.png (26214401 bytes, limit 26214400)]', + '[File too large to display inline: big.txt (99 bytes, limit 5)]', + '[Document too large to parse inline: big.pdf (99 bytes, limit 5)]', + '[Binary file: app.bin (application/octet-stream, 10 bytes). Cannot display as text.]', + ])('reports %s instead of grepping it', (content) => { + expect(() => grepPlaceholder(content)).toThrow(WorkspaceFileGrepError) + expect(() => grepPlaceholder(content)).toThrow(content) + }) + + it('still greps ordinary single-line content', () => { + expect(grepPlaceholder('x marks the spot')).toHaveLength(1) + }) +}) diff --git a/apps/sim/lib/copilot/vfs/operations.ts b/apps/sim/lib/copilot/vfs/operations.ts index c3ee423cd7d..f73dfd58f6b 100644 --- a/apps/sim/lib/copilot/vfs/operations.ts +++ b/apps/sim/lib/copilot/vfs/operations.ts @@ -1,6 +1,7 @@ import { createLogger } from '@sim/logger' import { truncate } from '@sim/utils/string' import micromatch from 'micromatch' +import { isNonGreppablePlaceholder } from '@/lib/copilot/vfs/read-placeholders' import { compileLinearRegex, isPlainText, @@ -64,18 +65,6 @@ export class WorkspaceFileGrepError extends Error { } } -/** - * True when file content is one of `readFileRecord`'s non-text placeholders - * (binary, unparseable, or over the inline read cap) — these carry no searchable - * content, so grepping them should report the placeholder instead. - */ -function isNonGreppablePlaceholder(content: string, totalLines: number): boolean { - if (totalLines !== 1) return false - return /^\[(File too large|Image too large|Document too large|Could not parse|Binary file|Compiled artifact too large)/.test( - content.trim() - ) -} - /** * Run a single-file content grep over an already-resolved file read result, * shared by workspace-file grep (`WorkspaceVFS.grepFile`) and chat-upload grep. diff --git a/apps/sim/lib/copilot/vfs/read-placeholders.ts b/apps/sim/lib/copilot/vfs/read-placeholders.ts new file mode 100644 index 00000000000..b2bf65f540b --- /dev/null +++ b/apps/sim/lib/copilot/vfs/read-placeholders.ts @@ -0,0 +1,76 @@ +/** + * The bracketed placeholders a VFS read returns in place of file content, and the + * predicates that classify them. Producers and matchers live in different modules; + * hand-written copies of the same prefix are how an oversized image once slipped past + * the read-size gate, which tested for a prefix no producer emitted. + * + * Keep free of heavy imports — the tool handlers pull it in without wanting the VFS. + */ + +import { formatFileSize } from '@/lib/uploads/utils/file-utils' + +const PREFIX = { + fileTooLarge: '[File too large to display inline:', + imageTooLarge: '[Image too large to read inline:', + imageUnavailable: '[Image unavailable:', + documentTooLarge: '[Document too large to parse inline:', + compiledArtifactTooLarge: '[Compiled artifact too large:', + couldNotParse: '[Could not parse', + binaryFile: '[Binary file:', +} as const + +export const readPlaceholder = { + fileTooLarge: (name: string, bytes: number, limit: number) => + `${PREFIX.fileTooLarge} ${name} (${bytes} bytes, limit ${limit})]`, + imageTooLarge: (name: string, bytes: number, limit: number) => + `${PREFIX.imageTooLarge} ${name} (${bytes} bytes, limit ${limit})]`, + // Formats here rather than at the call site: without `includeBytes` every + // sub-1KB file — which is every decompression bomb — prints as "0 Bytes". + imageUnavailable: (name: string, bytes: number, reason: string) => + `${PREFIX.imageUnavailable} ${name} (${formatFileSize(bytes, { includeBytes: true })}). ${reason}]`, + documentTooLarge: (name: string, bytes: number, limit: number) => + `${PREFIX.documentTooLarge} ${name} (${bytes} bytes, limit ${limit})]`, + compiledArtifactTooLarge: (name: string, bytes: number, limit: number) => + `${PREFIX.compiledArtifactTooLarge} ${name} (${bytes} bytes, limit ${limit})]`, + couldNotParse: (name: string, type: string, bytes: number) => + `${PREFIX.couldNotParse} ${name} (${type}, ${bytes} bytes)]`, + binaryFile: (name: string, type: string, bytes: number) => + `${PREFIX.binaryFile} ${name} (${type}, ${bytes} bytes). Cannot display as text.]`, +} as const + +/** + * Placeholders standing in for content that exists but exceeded a read cap; the read + * handler turns these into a tool error. + * + * Every size refusal belongs here — a document that breaches its cap is the same + * kind of answer as a file or an image that does, and reporting one of the three as + * a successful read was an inconsistency, not a distinction. + * + * Deliberately narrower than {@link isNonGreppablePlaceholder}: a parse failure or + * a binary file is not a size problem, and `[Image unavailable:` covers undecodable + * images as well as oversized ones, so neither belongs on the size path. + */ +const OVERSIZED_PREFIXES = [ + PREFIX.fileTooLarge, + PREFIX.imageTooLarge, + PREFIX.documentTooLarge, + PREFIX.compiledArtifactTooLarge, +] as const + +/** Every placeholder — none of them carry text worth searching. */ +const NON_GREPPABLE_PREFIXES = Object.values(PREFIX) + +export function isOversizedReadPlaceholder(content: string): boolean { + return OVERSIZED_PREFIXES.some((prefix) => content.startsWith(prefix)) +} + +/** + * True when a read result is a placeholder rather than file content. Only ever a + * single line, which is what keeps a real file that merely opens with `[Binary file:` + * greppable. + */ +export function isNonGreppablePlaceholder(content: string, totalLines: number): boolean { + if (totalLines !== 1) return false + const trimmed = content.trim() + return NON_GREPPABLE_PREFIXES.some((prefix) => trimmed.startsWith(prefix)) +} diff --git a/apps/sim/lib/copilot/vfs/workspace-vfs.ts b/apps/sim/lib/copilot/vfs/workspace-vfs.ts index 251e4cf0a4a..71b44baf539 100644 --- a/apps/sim/lib/copilot/vfs/workspace-vfs.ts +++ b/apps/sim/lib/copilot/vfs/workspace-vfs.ts @@ -62,6 +62,7 @@ import { canonicalWorkspaceFilePath, encodeVfsPathSegments, } from '@/lib/copilot/vfs/path-utils' +import { readPlaceholder } from '@/lib/copilot/vfs/read-placeholders' import type { DeploymentData, KbTagDefinitionSummary, @@ -1237,7 +1238,11 @@ export class WorkspaceVFS { } if (compiled.length > MAX_COMPILED_ATTACHMENT_BYTES) { return bindWorkspaceFileResult(record, { - content: `[Compiled artifact too large: ${record.name} (${compiled.length} bytes, limit ${MAX_COMPILED_ATTACHMENT_BYTES})]`, + content: readPlaceholder.compiledArtifactTooLarge( + record.name, + compiled.length, + MAX_COMPILED_ATTACHMENT_BYTES + ), totalLines: 1, }) } diff --git a/apps/sim/lib/uploads/server/heic.ts b/apps/sim/lib/uploads/server/heic.ts index 668aa69fd57..b0fbf8c1c68 100644 --- a/apps/sim/lib/uploads/server/heic.ts +++ b/apps/sim/lib/uploads/server/heic.ts @@ -27,7 +27,7 @@ const HEIF_BRANDS = new Set([...HEVC_HEIF_BRANDS, 'mif1', 'msf1', 'avif', 'avis' * This bounds file size, not pixel count. A small file declaring enormous * dimensions is rejected during parse by libheif's own security limits. */ -const MAX_TRANSCODE_INPUT_BYTES = 20 * 1024 * 1024 +export const MAX_TRANSCODE_INPUT_BYTES = 20 * 1024 * 1024 /** A real `ftyp` box holds a handful of brands; anything larger is malformed or hostile. */ const MAX_FTYP_BOX_BYTES = 512 From 582a3c47fe06310451ac3141b541286e92242485 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 14:05:23 -0700 Subject: [PATCH 06/10] fix(copilot): report the fetched size on an unavailable image The bytes are in hand by the time preparation fails, so quoting record.size back recreated the contradictory placeholder the download-cap path already fixed: a client-declared figure printed beside the real reason. Same defect, one line below the one it was fixed in. --- apps/sim/lib/copilot/vfs/file-reader.test.ts | 6 ++++-- apps/sim/lib/copilot/vfs/file-reader.ts | 8 +++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/copilot/vfs/file-reader.test.ts b/apps/sim/lib/copilot/vfs/file-reader.test.ts index 4b52dacb0b0..5808a8c51f9 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.test.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.test.ts @@ -80,11 +80,13 @@ describe('readFileRecord', () => { fetchWorkspaceFileBuffer.mockResolvedValue(bomb) - const result = await readFileRecord(imageRecord('bomb.png', bomb.length)) + // Recorded size deliberately disagrees with the real bytes: it is client-declared, + // so the placeholder must report what was actually fetched. + const result = await readFileRecord(imageRecord('bomb.png', 999_999)) expect(result?.attachment).toBeUndefined() expect(result?.content).toContain('It is too large to decode safely.') - // The byte count must survive formatting — a sub-1KB bomb formatted without + // The byte count must survive formatting too — a sub-1KB bomb formatted without // `includeBytes` collapses to "0 Bytes" next to the real reason. expect(result?.content).toContain(`(${bomb.length} Bytes)`) }, diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index 83267d0ac75..fd856a27ff5 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -481,7 +481,13 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise Date: Sat, 8 Aug 2026 14:30:50 -0700 Subject: [PATCH 07/10] test(copilot): pin the read caps, and correct what the limits actually claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit follow-up. No behaviour change; the code was right and the comments explaining it were not. Measured the bomb through this exact pipeline rather than reasoning about it (100MP/256MP/576MP/1024MP): libvips decodes sequentially, so peak RSS stays flat in the tens of MB no matter what the header declares. What scales is CPU, roughly linearly — ~240ms at 100MP, ~1.35s at 1024MP, once per resize rung. The pixel budget is a CPU bound, not the memory bound the comment described, and the "~400MB raster" arithmetic was wrong by an order of magnitude. - say that, with the measurements, so the next reader tunes against the real cost - stop claiming the source byte cap covers everything a user can upload: presigned and multipart accept gigabytes, so an image above it is stored fine and simply cannot be read inline. Deliberate trade, now stated as one - correct the resize-ladder comment, which asserted the failure is always a decode when the try also wraps the encoder - correct two claims in read-placeholders: the handler does pull the VFS, and the oversized set excludes an image size refusal it said it included Tests: cover the document and compiled-artifact size refusals (the document one was the newest behaviour change and had no coverage at all), the text and document download caps, and that the cap is handed to the download rather than merely producing the right message. Placeholder cases are built from the producers instead of hand-copied — a literal only proves the matcher agrees with the test, which is the drift this module exists to prevent. --- .../lib/copilot/tools/handlers/vfs.test.ts | 34 +++++++++----- apps/sim/lib/copilot/vfs/file-reader.test.ts | 35 ++++++++++++++ apps/sim/lib/copilot/vfs/file-reader.ts | 46 ++++++++++++------- apps/sim/lib/copilot/vfs/operations.test.ts | 46 ++++++++++++++----- apps/sim/lib/copilot/vfs/read-placeholders.ts | 17 +++---- 5 files changed, 128 insertions(+), 50 deletions(-) diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts index fd229f30830..bdfed8af510 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts @@ -51,6 +51,7 @@ vi.mock('./upload-file-reader', () => ({ })) import { WorkspaceFileGrepError } from '@/lib/copilot/vfs/operations' +import { readPlaceholder } from '@/lib/copilot/vfs/read-placeholders' import { executeVfsGlob, executeVfsGrep, executeVfsRead } from './vfs' const OVERSIZED_INLINE_CONTENT = 'x'.repeat(TOOL_RESULT_MAX_INLINE_CHARS + 1) @@ -179,31 +180,41 @@ describe('vfs handlers oversize policy', () => { expect((result.output as { attachment?: { type: string } })?.attachment?.type).toBe('file') }) - it('fails oversized image placeholder when image exceeds size limit', async () => { + /** + * Every size refusal is a failed read, whichever path produced it. Built from the + * producers so a prefix leaving `OVERSIZED_PREFIXES` fails here rather than + * silently downgrading a refusal to a one-line "successful" read. + */ + it.each([ + ['image', readPlaceholder.imageTooLarge('huge.png', 99, 5)], + ['file', readPlaceholder.fileTooLarge('huge.txt', 99, 5)], + ['document', readPlaceholder.documentTooLarge('huge.pdf', 99, 5)], + ['compiled artifact', readPlaceholder.compiledArtifactTooLarge('app.js', 99, 5)], + ])('fails the read when a %s exceeds its size limit', async (_kind, content) => { const vfs = makeVfs() - vfs.readFileContent.mockResolvedValue({ - content: '[Image too large to read inline: huge.png (26214401 bytes, limit 26214400)]', - totalLines: 1, - }) + vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 }) getOrMaterializeVFS.mockResolvedValue(vfs) const result = await executeVfsRead( - { path: 'files/huge.png/content' }, + { path: 'files/huge/content' }, { userId: 'user-1', workflowId: 'wf-1', workspaceId: 'ws-1' } ) expect(result.success).toBe(false) - expect(result.error).toContain('too large') + // The placeholder verbatim, not the generic "grep this instead" fallback. + expect(result.error).toBe(content) }) it('returns an undecodable image placeholder as content, not as a size failure', async () => { const vfs = makeVfs() // Not a size problem — the bytes were read fine and the reason is already in the // message, so the model should see it rather than a "too large, use grep" error. - vfs.readFileContent.mockResolvedValue({ - content: '[Image unavailable: bomb.png (90 Bytes). It is too large to decode safely.]', - totalLines: 1, - }) + const content = readPlaceholder.imageUnavailable( + 'bomb.png', + 90, + 'It is too large to decode safely.' + ) + vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 }) getOrMaterializeVFS.mockResolvedValue(vfs) const result = await executeVfsRead( @@ -212,6 +223,7 @@ describe('vfs handlers oversize policy', () => { ) expect(result.success).toBe(true) + expect((result.output as { content?: string })?.content).toBe(content) }) it('reads canonical file leaf metadata without fetching dynamic content', async () => { diff --git a/apps/sim/lib/copilot/vfs/file-reader.test.ts b/apps/sim/lib/copilot/vfs/file-reader.test.ts index 5808a8c51f9..32fe8d11716 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.test.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.test.ts @@ -17,6 +17,8 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ import { MAX_IMAGE_READ_BYTES, MAX_IMAGE_SOURCE_BYTES, + MAX_PARSEABLE_READ_BYTES, + MAX_TEXT_READ_BYTES, readFileRecord, } from '@/lib/copilot/vfs/file-reader' import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' @@ -108,8 +110,41 @@ describe('readFileRecord', () => { expect(result?.content).toContain('Image too large to read inline') // The observed size, not the understated 1024 the cap exists to distrust. expect(result?.content).toContain(`${MAX_IMAGE_SOURCE_BYTES + 5_000} bytes`) + // And the cap was actually handed to the download — the placeholder alone would + // still appear if the argument were dropped, since the mock rejects regardless. + expect(fetchWorkspaceFileBuffer).toHaveBeenCalledWith(expect.anything(), { + maxBytes: MAX_IMAGE_SOURCE_BYTES, + }) }) + it.each([ + ['text', 'notes.txt', 'text/plain', MAX_TEXT_READ_BYTES, 'File too large to display inline'], + [ + 'document', + 'report.pdf', + 'application/pdf', + MAX_PARSEABLE_READ_BYTES, + 'Document too large to parse inline', + ], + ])( + 'caps the %s download and reports the observed size when it breaches', + async (_kind, name, type, cap, expected) => { + fetchWorkspaceFileBuffer.mockRejectedValue( + new PayloadSizeLimitError({ + label: 'workspace file', + maxBytes: cap, + observedBytes: cap + 7_000, + }) + ) + + const result = await readFileRecord(imageRecord(name, 1024, type)) + + expect(result?.content).toContain(expected) + expect(result?.content).toContain(`${cap + 7_000} bytes`) + expect(fetchWorkspaceFileBuffer).toHaveBeenCalledWith(expect.anything(), { maxBytes: cap }) + } + ) + it('reports an oversized HEIF as a size refusal, not as a corrupt file', async () => { // `ftyp`+`heic` brand, past the WebAssembly transcoder's own tighter ceiling. const heif = Buffer.alloc(MAX_TRANSCODE_INPUT_BYTES + 1) diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index fd856a27ff5..951786bb7f9 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -47,25 +47,33 @@ export const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 // 5 MB // Parseable-document byte cap. Large office/PDF files can still // produce huge extracted text; reject up front to avoid wasting a // download + parse only to blow past the tool-result budget. -const MAX_PARSEABLE_READ_BYTES = 5 * 1024 * 1024 // 5 MB +export const MAX_PARSEABLE_READ_BYTES = 5 * 1024 * 1024 // 5 MB /** - * Source-image byte ceiling, checked before the download and enforced by it. A - * workspace file may be up to {@link MAX_WORKSPACE_FILE_SIZE}, and buffering one of - * those whole would exhaust memory on its own — the pixel budget below bounds the - * decode, not the transfer. + * Source-image byte ceiling, checked before the download and enforced by it. This + * route holds the whole file in worker memory, so it reuses the ceiling the FormData + * upload route set for that same failure mode rather than inventing a number. * - * Deliberately the FormData upload ceiling rather than a number of its own: that cap - * exists for exactly this failure mode (a route holding an entire file in worker - * memory), so anything a user could upload through it stays readable here. Picking - * something tighter would refuse images that read fine today for no security gain — - * a decompression bomb is small, and it is the pixel budget that stops it. + * It does NOT cover everything a user can store: presigned and multipart uploads + * accept up to `MAX_WORKSPACE_FILE_SIZE` (gigabytes), so an image above this ceiling + * is stored fine and simply cannot be read inline. That is a deliberate trade — the + * alternative is buffering a multi-gigabyte file to answer one read — and it is a + * memory budget, not part of the decompression-bomb defence, which is the pixel + * budget below. A bomb is small; no byte cap would catch it. */ export const MAX_IMAGE_SOURCE_BYTES = MAX_WORKSPACE_FORMDATA_FILE_SIZE /** - * Pixel ceiling on the decoded raster. libvips materialises the whole raster, and - * an allocation this large OOM-kills the process rather than throwing, so it has to - * be refused up front. 100MP caps the decode near 400MB while clearing every real - * camera — a 48MP iPhone still is 8064x6048. + * Pixel ceiling on the decoded image, and the actual decompression-bomb defence: a + * few hundred KB of PNG can declare an arbitrarily large raster. + * + * The cost it bounds is CPU, not memory. libvips decodes this pipeline sequentially, + * so peak RSS stays flat (tens of MB) no matter what the header declares — measured + * on this exact pipeline, 100MP..1024MP all sat under ~120MB. What scales is time, + * roughly linearly: ~240ms at 100MP, ~1.35s at 1024MP, once per resize rung. So the + * budget caps what one read can burn, and the `break` below caps how many rungs a + * failing image gets. + * + * 100MP clears every single-shot camera (a 48MP iPhone still is 8064x6048) but will + * refuse a stitched gigapixel panorama, which is the known cost of the ceiling. */ const MAX_IMAGE_INPUT_PIXELS = 100_000_000 const MAX_IMAGE_DIMENSION = 1568 @@ -381,9 +389,13 @@ async function prepareImageForVision( } } } catch (err) { - // Next dimension, not next quality: the quality rungs re-decode the - // identical source, so repeating a failed decode there is pure waste. - // A smaller dimension is worth trying — libvips shrinks JPEG on load. + // Next dimension, not next quality: every quality rung re-decodes the + // same source and only varies the encoder, so a failure here almost + // always repeats. Dropping a dimension is the one thing that can change + // the outcome (JPEG shrinks on load), and it bounds a bomb at 4 decodes + // instead of 16. A genuinely encoder-only failure would lose its lower + // quality rungs at that dimension — no such failure mode is known, and + // 4 attempts is the deliberate ceiling. logger.warn('Failed image resize attempt for VFS read', { mediaType, dimension, diff --git a/apps/sim/lib/copilot/vfs/operations.test.ts b/apps/sim/lib/copilot/vfs/operations.test.ts index d26a6188ff1..7bfeecf7812 100644 --- a/apps/sim/lib/copilot/vfs/operations.test.ts +++ b/apps/sim/lib/copilot/vfs/operations.test.ts @@ -3,6 +3,7 @@ */ import { describe, expect, it } from 'vitest' import { glob, grep, grepReadResult, WorkspaceFileGrepError } from '@/lib/copilot/vfs/operations' +import { readPlaceholder } from '@/lib/copilot/vfs/read-placeholders' function vfsFromEntries(entries: [string, string][]): Map { return new Map(entries) @@ -200,21 +201,42 @@ describe('grepReadResult placeholders', () => { grepReadResult('files/x.png/content', { content, totalLines: 1 }, 'x', 'files/x.png/content') /** - * Every `readFileRecord` placeholder carries no searchable text, so grep must - * report the placeholder rather than matching against its own prose. + * Built from the producers rather than hand-copied: a literal here would only + * prove the matcher agrees with this file, which is exactly the drift that let a + * gate test for a prefix no producer emitted. Covers every builder, so dropping + * one from the shared table fails here. */ - it.each([ - '[Image unavailable: bomb.png (90 Bytes). It is too large to decode safely.]', - '[Image too large to read inline: huge.png (26214401 bytes, limit 26214400)]', - '[File too large to display inline: big.txt (99 bytes, limit 5)]', - '[Document too large to parse inline: big.pdf (99 bytes, limit 5)]', - '[Binary file: app.bin (application/octet-stream, 10 bytes). Cannot display as text.]', - ])('reports %s instead of grepping it', (content) => { - expect(() => grepPlaceholder(content)).toThrow(WorkspaceFileGrepError) - expect(() => grepPlaceholder(content)).toThrow(content) - }) + const everyPlaceholder = Object.entries({ + fileTooLarge: readPlaceholder.fileTooLarge('big.txt', 99, 5), + imageTooLarge: readPlaceholder.imageTooLarge('huge.png', 99, 5), + imageUnavailable: readPlaceholder.imageUnavailable('bomb.png', 90, 'It could not be decoded.'), + documentTooLarge: readPlaceholder.documentTooLarge('big.pdf', 99, 5), + compiledArtifactTooLarge: readPlaceholder.compiledArtifactTooLarge('app.js', 99, 5), + couldNotParse: readPlaceholder.couldNotParse('x.pdf', 'application/pdf', 10), + binaryFile: readPlaceholder.binaryFile('app.bin', 'application/octet-stream', 10), + }) + + it.each(everyPlaceholder)( + 'reports the %s placeholder instead of grepping it', + (_name, content) => { + expect(() => grepPlaceholder(content)).toThrow(WorkspaceFileGrepError) + expect(() => grepPlaceholder(content)).toThrow(content) + } + ) it('still greps ordinary single-line content', () => { expect(grepPlaceholder('x marks the spot')).toHaveLength(1) }) + + it('greps a real multi-line file that merely opens like a placeholder', () => { + // The single-line guard is what keeps this file searchable rather than swallowed. + const content = `${readPlaceholder.binaryFile('app.bin', 'text/plain', 10)}\nx marks the spot` + const matches = grepReadResult( + 'files/notes.txt/content', + { content, totalLines: 2 }, + 'x', + 'files/notes.txt/content' + ) + expect(matches.length).toBeGreaterThan(0) + }) }) diff --git a/apps/sim/lib/copilot/vfs/read-placeholders.ts b/apps/sim/lib/copilot/vfs/read-placeholders.ts index b2bf65f540b..41d0c0a87c0 100644 --- a/apps/sim/lib/copilot/vfs/read-placeholders.ts +++ b/apps/sim/lib/copilot/vfs/read-placeholders.ts @@ -3,8 +3,6 @@ * predicates that classify them. Producers and matchers live in different modules; * hand-written copies of the same prefix are how an oversized image once slipped past * the read-size gate, which tested for a prefix no producer emitted. - * - * Keep free of heavy imports — the tool handlers pull it in without wanting the VFS. */ import { formatFileSize } from '@/lib/uploads/utils/file-utils' @@ -39,16 +37,15 @@ export const readPlaceholder = { } as const /** - * Placeholders standing in for content that exists but exceeded a read cap; the read - * handler turns these into a tool error. + * Placeholders meaning "the file is there, but reading it was refused on size"; the + * read handler turns these into a tool error rather than a one-line success. * - * Every size refusal belongs here — a document that breaches its cap is the same - * kind of answer as a file or an image that does, and reporting one of the three as - * a successful read was an inconsistency, not a distinction. + * File, image, document and compiled artifact all belong here — reporting one of + * the four as a successful read was an inconsistency, not a distinction. * - * Deliberately narrower than {@link isNonGreppablePlaceholder}: a parse failure or - * a binary file is not a size problem, and `[Image unavailable:` covers undecodable - * images as well as oversized ones, so neither belongs on the size path. + * `[Image unavailable:` is excluded even though one of its reasons is a size, because + * its other reasons are not: it also covers an undecodable or unsupported image, and + * those are answers rather than refusals. Callers get it as content. */ const OVERSIZED_PREFIXES = [ PREFIX.fileTooLarge, From 3dd662336e60267d718019a9cf6503dc9b3c50b0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 14:42:26 -0700 Subject: [PATCH 08/10] fix(copilot): match a size refusal by its whole shape, not its prefix isOversizedReadPlaceholder tested startsWith, so a real one-line file opening with one of those prefixes was returned to the model as a tool error instead of its contents. Every builder emits the same '(N bytes, limit M)]' tail, so requiring the full shape costs nothing and makes an accidental collision implausible. The pattern is built from the same prefix table, so it cannot drift from the producers. --- apps/sim/lib/copilot/tools/handlers/vfs.test.ts | 17 +++++++++++++++++ apps/sim/lib/copilot/vfs/read-placeholders.ts | 17 ++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts index bdfed8af510..05225b1e3c1 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts @@ -205,6 +205,23 @@ describe('vfs handlers oversize policy', () => { expect(result.error).toBe(content) }) + it('returns a real file that merely opens like a size refusal', async () => { + // Prefix-only matching would turn this user's file into a tool error. The + // refusal is recognised by its whole shape, which real prose does not have. + const vfs = makeVfs() + const content = '[Document too large to parse inline: is the message we emit here]' + vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 }) + getOrMaterializeVFS.mockResolvedValue(vfs) + + const result = await executeVfsRead( + { path: 'files/notes.md/content' }, + { userId: 'user-1', workflowId: 'wf-1', workspaceId: 'ws-1' } + ) + + expect(result.success).toBe(true) + expect((result.output as { content?: string })?.content).toBe(content) + }) + it('returns an undecodable image placeholder as content, not as a size failure', async () => { const vfs = makeVfs() // Not a size problem — the bytes were read fine and the reason is already in the diff --git a/apps/sim/lib/copilot/vfs/read-placeholders.ts b/apps/sim/lib/copilot/vfs/read-placeholders.ts index 41d0c0a87c0..3708d9a9834 100644 --- a/apps/sim/lib/copilot/vfs/read-placeholders.ts +++ b/apps/sim/lib/copilot/vfs/read-placeholders.ts @@ -57,8 +57,23 @@ const OVERSIZED_PREFIXES = [ /** Every placeholder — none of them carry text worth searching. */ const NON_GREPPABLE_PREFIXES = Object.values(PREFIX) +function escapeRegex(literal: string): string { + return literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + +/** + * Matches a size refusal in full rather than by prefix. Every builder above emits the + * same `… (N bytes, limit M)]` tail, so requiring it costs nothing and stops a real + * one-line file that merely opens with this text from being turned into a tool error + * instead of being returned. Built from {@link OVERSIZED_PREFIXES} so it cannot drift + * from the producers. One unnested `.+` against a fixed suffix — no backtracking. + */ +const OVERSIZED_PATTERN = new RegExp( + `^(?:${OVERSIZED_PREFIXES.map(escapeRegex).join('|')}) .+ \\(\\d+ bytes, limit \\d+\\)\\]$` +) + export function isOversizedReadPlaceholder(content: string): boolean { - return OVERSIZED_PREFIXES.some((prefix) => content.startsWith(prefix)) + return OVERSIZED_PATTERN.test(content) } /** From 0e2b55a2b8ae40e2eb321017e8b25fc18c928816 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 14:55:54 -0700 Subject: [PATCH 09/10] fix(copilot): let a read placeholder carry its own kind instead of sniffing its text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three rounds of review found the same defect in three places: a gate matching a prefix no producer emitted so it never fired, a real file misread as a refusal, a filename newline hiding a refusal. Each fix tightened the text matching and the next round found another way the text was the wrong thing to trust. So stop trusting it. `readPlaceholder.*` now returns the whole read result — content, totalLines, and a `placeholder` kind — and the two classifiers read the kind. A producer cannot emit a placeholder without tagging it, and no amount of user content can imitate one, so both failure directions are gone by construction rather than by a better pattern. `oversized` (the four size refusals) is reported as a tool error; `unreadable` (undecodable, binary, unparseable) is returned as content. The type flows through FileReadResult and ReadResult, so the compiler checks the tag survives to the gates rather than leaving a silent drop to be discovered later. Also removes the totalLines-of-1 heuristic, which was only ever a proxy for this. --- .../lib/copilot/tools/handlers/vfs.test.ts | 45 ++++-- apps/sim/lib/copilot/tools/handlers/vfs.ts | 10 +- apps/sim/lib/copilot/vfs/file-reader.ts | 50 ++----- apps/sim/lib/copilot/vfs/operations.test.ts | 39 +++-- apps/sim/lib/copilot/vfs/operations.ts | 15 +- apps/sim/lib/copilot/vfs/read-placeholders.ts | 135 +++++++++--------- apps/sim/lib/copilot/vfs/workspace-vfs.ts | 10 +- 7 files changed, 152 insertions(+), 152 deletions(-) diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts index 05225b1e3c1..1d80ac9d08b 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts @@ -119,10 +119,9 @@ describe('vfs handlers oversize policy', () => { it('fails file-backed oversized read placeholders with original message', async () => { const vfs = makeVfs() - vfs.readFileContent.mockResolvedValue({ - content: '[File too large to display inline: big.txt (6000000 bytes, limit 5242880)]', - totalLines: 1, - }) + vfs.readFileContent.mockResolvedValue( + readPlaceholder.fileTooLarge('big.txt', 6_000_000, 5_242_880) + ) getOrMaterializeVFS.mockResolvedValue(vfs) const result = await executeVfsRead( @@ -182,7 +181,7 @@ describe('vfs handlers oversize policy', () => { /** * Every size refusal is a failed read, whichever path produced it. Built from the - * producers so a prefix leaving `OVERSIZED_PREFIXES` fails here rather than + * producers so one that stops tagging itself `oversized` fails here rather than * silently downgrading a refusal to a one-line "successful" read. */ it.each([ @@ -190,9 +189,9 @@ describe('vfs handlers oversize policy', () => { ['file', readPlaceholder.fileTooLarge('huge.txt', 99, 5)], ['document', readPlaceholder.documentTooLarge('huge.pdf', 99, 5)], ['compiled artifact', readPlaceholder.compiledArtifactTooLarge('app.js', 99, 5)], - ])('fails the read when a %s exceeds its size limit', async (_kind, content) => { + ])('fails the read when a %s exceeds its size limit', async (_kind, placeholder) => { const vfs = makeVfs() - vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 }) + vfs.readFileContent.mockResolvedValue(placeholder) getOrMaterializeVFS.mockResolvedValue(vfs) const result = await executeVfsRead( @@ -202,14 +201,31 @@ describe('vfs handlers oversize policy', () => { expect(result.success).toBe(false) // The placeholder verbatim, not the generic "grep this instead" fallback. - expect(result.error).toBe(content) + expect(result.error).toBe(placeholder.content) + }) + + it('still fails the read when the stored name contains a newline', async () => { + // Nothing about the message text decides this, so a name that would break a + // text-shape match cannot hide a refusal. + const vfs = makeVfs() + const placeholder = readPlaceholder.fileTooLarge('we\nird.txt', 99, 5) + vfs.readFileContent.mockResolvedValue(placeholder) + getOrMaterializeVFS.mockResolvedValue(vfs) + + const result = await executeVfsRead( + { path: 'files/weird/content' }, + { userId: 'user-1', workflowId: 'wf-1', workspaceId: 'ws-1' } + ) + + expect(result.success).toBe(false) + expect(result.error).toBe(placeholder.content) }) - it('returns a real file that merely opens like a size refusal', async () => { - // Prefix-only matching would turn this user's file into a tool error. The - // refusal is recognised by its whole shape, which real prose does not have. + it('returns a real file whose content is exactly a size-refusal message', async () => { + // Untagged, so it is content. Recognising refusals by their text would turn this + // user's file into a tool error instead of returning it. const vfs = makeVfs() - const content = '[Document too large to parse inline: is the message we emit here]' + const { content } = readPlaceholder.documentTooLarge('huge.pdf', 99, 5) vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 }) getOrMaterializeVFS.mockResolvedValue(vfs) @@ -226,12 +242,13 @@ describe('vfs handlers oversize policy', () => { const vfs = makeVfs() // Not a size problem — the bytes were read fine and the reason is already in the // message, so the model should see it rather than a "too large, use grep" error. - const content = readPlaceholder.imageUnavailable( + const placeholder = readPlaceholder.imageUnavailable( 'bomb.png', 90, 'It is too large to decode safely.' ) - vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 }) + const content = placeholder.content + vfs.readFileContent.mockResolvedValue(placeholder) getOrMaterializeVFS.mockResolvedValue(vfs) const result = await executeVfsRead( diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.ts b/apps/sim/lib/copilot/tools/handlers/vfs.ts index 67e04629667..6fc89b09016 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.ts @@ -330,7 +330,7 @@ export async function executeVfsRead( const isAttachment = hasModelAttachment(uploadResult) if ( !isAttachment && - (isOversizedReadPlaceholder(uploadResult.content) || + (isOversizedReadPlaceholder(uploadResult) || serializedResultSize(uploadResult) > TOOL_RESULT_MAX_INLINE_CHARS) ) { logger.warn('Upload read result too large', { @@ -341,7 +341,7 @@ export async function executeVfsRead( }) return { success: false, - error: isOversizedReadPlaceholder(uploadResult.content) + error: isOversizedReadPlaceholder(uploadResult) ? uploadResult.content : // Same as the workspace-file branch below: this size gate runs on // the whole upload before any window, so "retry with offset/limit" @@ -400,7 +400,7 @@ export async function executeVfsRead( const isAttachment = hasModelAttachment(fileContent) if ( !isAttachment && - (isOversizedReadPlaceholder(fileContent.content) || + (isOversizedReadPlaceholder(fileContent) || serializedResultSize(fileContent) > TOOL_RESULT_MAX_INLINE_CHARS) ) { logger.warn('File read result too large', { @@ -411,7 +411,7 @@ export async function executeVfsRead( }) return { success: false, - error: isOversizedReadPlaceholder(fileContent.content) + error: isOversizedReadPlaceholder(fileContent) ? fileContent.content : 'Read result too large to return inline. Use grep with a more specific pattern or narrower path to locate the relevant section, then retry read with offset/limit. Avoid catch-all greps or full-file reads because they waste context window.', } @@ -459,7 +459,7 @@ export async function executeVfsRead( } if ( !hasModelAttachment(result) && - (isOversizedReadPlaceholder(result.content) || + (isOversizedReadPlaceholder(result) || serializedResultSize(result) > TOOL_RESULT_MAX_INLINE_CHARS) ) { return { diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index 951786bb7f9..59f82036097 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -12,7 +12,7 @@ import { TraceEvent } from '@/lib/copilot/generated/trace-events-v1' import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1' import { recordFileRead } from '@/lib/copilot/request/metrics' import { markSpanForError } from '@/lib/copilot/request/otel' -import { readPlaceholder } from '@/lib/copilot/vfs/read-placeholders' +import { type PlaceholderKind, readPlaceholder } from '@/lib/copilot/vfs/read-placeholders' import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { fetchWorkspaceFileBuffer } from '@/lib/uploads/contexts/workspace/workspace-file-manager' @@ -436,6 +436,8 @@ async function prepareImageForVision( export interface FileReadResult { content: string totalLines: number + /** Set when `content` stands in for the file rather than being it — see `readPlaceholder`. */ + placeholder?: PlaceholderKind attachment?: { type: string name?: string @@ -478,10 +480,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge) - return { - content: readPlaceholder.imageTooLarge(record.name, bytes, MAX_IMAGE_SOURCE_BYTES), - totalLines: 1, - } + return readPlaceholder.imageTooLarge(record.name, bytes, MAX_IMAGE_SOURCE_BYTES) } // The recorded size only skips a doomed download; the cap on the download // itself is what bounds the bytes actually read. @@ -492,16 +491,13 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise { span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.TextTooLarge) - return { - content: readPlaceholder.fileTooLarge(record.name, bytes, MAX_TEXT_READ_BYTES), - totalLines: 1, - } + return readPlaceholder.fileTooLarge(record.name, bytes, MAX_TEXT_READ_BYTES) } if (record.size > MAX_TEXT_READ_BYTES) return textTooLarge(record.size) @@ -559,14 +552,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise MAX_PARSEABLE_READ_BYTES) return documentTooLarge(record.size) const fetched = await fetchWithinLimit(record, MAX_PARSEABLE_READ_BYTES) @@ -594,10 +580,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise { }) describe('grepReadResult placeholders', () => { - const grepPlaceholder = (content: string) => - grepReadResult('files/x.png/content', { content, totalLines: 1 }, 'x', 'files/x.png/content') + const grepResult = (result: { + content: string + totalLines: number + placeholder?: 'oversized' | 'unreadable' + }) => grepReadResult('files/x.png/content', result, 'x', 'files/x.png/content') /** - * Built from the producers rather than hand-copied: a literal here would only - * prove the matcher agrees with this file, which is exactly the drift that let a - * gate test for a prefix no producer emitted. Covers every builder, so dropping - * one from the shared table fails here. + * Built from the producers rather than hand-assembled: covers every builder, so + * one that stops tagging itself fails here. */ const everyPlaceholder = Object.entries({ fileTooLarge: readPlaceholder.fileTooLarge('big.txt', 99, 5), @@ -218,25 +219,19 @@ describe('grepReadResult placeholders', () => { it.each(everyPlaceholder)( 'reports the %s placeholder instead of grepping it', - (_name, content) => { - expect(() => grepPlaceholder(content)).toThrow(WorkspaceFileGrepError) - expect(() => grepPlaceholder(content)).toThrow(content) + (_name, result) => { + expect(() => grepResult(result)).toThrow(WorkspaceFileGrepError) + expect(() => grepResult(result)).toThrow(result.content) } ) it('still greps ordinary single-line content', () => { - expect(grepPlaceholder('x marks the spot')).toHaveLength(1) - }) - - it('greps a real multi-line file that merely opens like a placeholder', () => { - // The single-line guard is what keeps this file searchable rather than swallowed. - const content = `${readPlaceholder.binaryFile('app.bin', 'text/plain', 10)}\nx marks the spot` - const matches = grepReadResult( - 'files/notes.txt/content', - { content, totalLines: 2 }, - 'x', - 'files/notes.txt/content' - ) - expect(matches.length).toBeGreaterThan(0) + expect(grepResult({ content: 'x marks the spot', totalLines: 1 })).toHaveLength(1) + }) + + it('greps a real file whose content is exactly a placeholder message', () => { + // Untagged, so it is content — text alone never makes something a placeholder. + const { content } = readPlaceholder.binaryFile('app.bin', 'text/plain', 10) + expect(grepResult({ content, totalLines: 1 })).toHaveLength(1) }) }) diff --git a/apps/sim/lib/copilot/vfs/operations.ts b/apps/sim/lib/copilot/vfs/operations.ts index f73dfd58f6b..ac6d87c78a6 100644 --- a/apps/sim/lib/copilot/vfs/operations.ts +++ b/apps/sim/lib/copilot/vfs/operations.ts @@ -1,7 +1,10 @@ import { createLogger } from '@sim/logger' import { truncate } from '@sim/utils/string' import micromatch from 'micromatch' -import { isNonGreppablePlaceholder } from '@/lib/copilot/vfs/read-placeholders' +import { + isNonGreppablePlaceholder, + type PlaceholderKind, +} from '@/lib/copilot/vfs/read-placeholders' import { compileLinearRegex, isPlainText, @@ -75,7 +78,12 @@ export class WorkspaceFileGrepError extends Error { */ export function grepReadResult( path: string, - result: { content: string; totalLines: number; attachment?: unknown }, + result: { + content: string + totalLines: number + placeholder?: PlaceholderKind + attachment?: unknown + }, pattern: string, readHint: string, options?: GrepOptions @@ -85,7 +93,7 @@ export function grepReadResult( `Cannot grep "${path}" — it has no searchable text (image/binary). Use read("${readHint}") to view it.` ) } - if (isNonGreppablePlaceholder(result.content, result.totalLines)) { + if (isNonGreppablePlaceholder(result)) { throw new WorkspaceFileGrepError(result.content) } return grep(new Map([[path, result.content]]), pattern, undefined, options) @@ -94,6 +102,7 @@ export function grepReadResult( export interface ReadResult { content: string totalLines: number + placeholder?: PlaceholderKind } /** diff --git a/apps/sim/lib/copilot/vfs/read-placeholders.ts b/apps/sim/lib/copilot/vfs/read-placeholders.ts index 3708d9a9834..634b5f3fe0a 100644 --- a/apps/sim/lib/copilot/vfs/read-placeholders.ts +++ b/apps/sim/lib/copilot/vfs/read-placeholders.ts @@ -1,88 +1,87 @@ /** - * The bracketed placeholders a VFS read returns in place of file content, and the - * predicates that classify them. Producers and matchers live in different modules; - * hand-written copies of the same prefix are how an oversized image once slipped past - * the read-size gate, which tested for a prefix no producer emitted. + * The stand-ins a VFS read returns in place of file content. + * + * A placeholder carries its own classification rather than being recognised by its + * text later. Sniffing the string was wrong twice over: gates drifted from the + * wording they were matching (one tested a prefix no producer emitted, so it never + * fired), and a real file whose content merely opened like a placeholder was + * misclassified as one. Neither is possible when the producer states what it built. */ import { formatFileSize } from '@/lib/uploads/utils/file-utils' -const PREFIX = { - fileTooLarge: '[File too large to display inline:', - imageTooLarge: '[Image too large to read inline:', - imageUnavailable: '[Image unavailable:', - documentTooLarge: '[Document too large to parse inline:', - compiledArtifactTooLarge: '[Compiled artifact too large:', - couldNotParse: '[Could not parse', - binaryFile: '[Binary file:', -} as const +/** + * `oversized` — the file is intact but reading it was refused on size, so the read + * handler reports a tool error rather than a one-line "success". + * + * `unreadable` — the read produced an answer, just not text: undecodable, binary, or + * unparseable. Callers get it as content. Both are equally unsearchable. + */ +export type PlaceholderKind = 'oversized' | 'unreadable' + +export interface ReadPlaceholder { + content: string + totalLines: 1 + placeholder: PlaceholderKind +} + +function placeholder(kind: PlaceholderKind, content: string): ReadPlaceholder { + return { content, totalLines: 1, placeholder: kind } +} export const readPlaceholder = { fileTooLarge: (name: string, bytes: number, limit: number) => - `${PREFIX.fileTooLarge} ${name} (${bytes} bytes, limit ${limit})]`, + placeholder( + 'oversized', + `[File too large to display inline: ${name} (${bytes} bytes, limit ${limit})]` + ), imageTooLarge: (name: string, bytes: number, limit: number) => - `${PREFIX.imageTooLarge} ${name} (${bytes} bytes, limit ${limit})]`, - // Formats here rather than at the call site: without `includeBytes` every - // sub-1KB file — which is every decompression bomb — prints as "0 Bytes". - imageUnavailable: (name: string, bytes: number, reason: string) => - `${PREFIX.imageUnavailable} ${name} (${formatFileSize(bytes, { includeBytes: true })}). ${reason}]`, + placeholder( + 'oversized', + `[Image too large to read inline: ${name} (${bytes} bytes, limit ${limit})]` + ), documentTooLarge: (name: string, bytes: number, limit: number) => - `${PREFIX.documentTooLarge} ${name} (${bytes} bytes, limit ${limit})]`, + placeholder( + 'oversized', + `[Document too large to parse inline: ${name} (${bytes} bytes, limit ${limit})]` + ), compiledArtifactTooLarge: (name: string, bytes: number, limit: number) => - `${PREFIX.compiledArtifactTooLarge} ${name} (${bytes} bytes, limit ${limit})]`, + placeholder( + 'oversized', + `[Compiled artifact too large: ${name} (${bytes} bytes, limit ${limit})]` + ), + /** + * Not `oversized` even when the reason is a size: it also covers an undecodable or + * unsupported image, which is an answer rather than a refusal. + * + * Formats here rather than at the call site — without `includeBytes` every sub-1KB + * file, which is every decompression bomb, prints as "0 Bytes". + */ + imageUnavailable: (name: string, bytes: number, reason: string) => + placeholder( + 'unreadable', + `[Image unavailable: ${name} (${formatFileSize(bytes, { includeBytes: true })}). ${reason}]` + ), couldNotParse: (name: string, type: string, bytes: number) => - `${PREFIX.couldNotParse} ${name} (${type}, ${bytes} bytes)]`, + placeholder('unreadable', `[Could not parse ${name} (${type}, ${bytes} bytes)]`), binaryFile: (name: string, type: string, bytes: number) => - `${PREFIX.binaryFile} ${name} (${type}, ${bytes} bytes). Cannot display as text.]`, + placeholder( + 'unreadable', + `[Binary file: ${name} (${type}, ${bytes} bytes). Cannot display as text.]` + ), } as const -/** - * Placeholders meaning "the file is there, but reading it was refused on size"; the - * read handler turns these into a tool error rather than a one-line success. - * - * File, image, document and compiled artifact all belong here — reporting one of - * the four as a successful read was an inconsistency, not a distinction. - * - * `[Image unavailable:` is excluded even though one of its reasons is a size, because - * its other reasons are not: it also covers an undecodable or unsupported image, and - * those are answers rather than refusals. Callers get it as content. - */ -const OVERSIZED_PREFIXES = [ - PREFIX.fileTooLarge, - PREFIX.imageTooLarge, - PREFIX.documentTooLarge, - PREFIX.compiledArtifactTooLarge, -] as const - -/** Every placeholder — none of them carry text worth searching. */ -const NON_GREPPABLE_PREFIXES = Object.values(PREFIX) - -function escapeRegex(literal: string): string { - return literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +/** A read result, which may or may not have been produced by this module. */ +interface MaybePlaceholder { + placeholder?: PlaceholderKind } -/** - * Matches a size refusal in full rather than by prefix. Every builder above emits the - * same `… (N bytes, limit M)]` tail, so requiring it costs nothing and stops a real - * one-line file that merely opens with this text from being turned into a tool error - * instead of being returned. Built from {@link OVERSIZED_PREFIXES} so it cannot drift - * from the producers. One unnested `.+` against a fixed suffix — no backtracking. - */ -const OVERSIZED_PATTERN = new RegExp( - `^(?:${OVERSIZED_PREFIXES.map(escapeRegex).join('|')}) .+ \\(\\d+ bytes, limit \\d+\\)\\]$` -) - -export function isOversizedReadPlaceholder(content: string): boolean { - return OVERSIZED_PATTERN.test(content) +/** A size refusal, which the read handler surfaces as a tool error. */ +export function isOversizedReadPlaceholder(result: MaybePlaceholder): boolean { + return result.placeholder === 'oversized' } -/** - * True when a read result is a placeholder rather than file content. Only ever a - * single line, which is what keeps a real file that merely opens with `[Binary file:` - * greppable. - */ -export function isNonGreppablePlaceholder(content: string, totalLines: number): boolean { - if (totalLines !== 1) return false - const trimmed = content.trim() - return NON_GREPPABLE_PREFIXES.some((prefix) => trimmed.startsWith(prefix)) +/** Any placeholder — none of them carry text worth searching. */ +export function isNonGreppablePlaceholder(result: MaybePlaceholder): boolean { + return result.placeholder !== undefined } diff --git a/apps/sim/lib/copilot/vfs/workspace-vfs.ts b/apps/sim/lib/copilot/vfs/workspace-vfs.ts index 71b44baf539..d11769bc963 100644 --- a/apps/sim/lib/copilot/vfs/workspace-vfs.ts +++ b/apps/sim/lib/copilot/vfs/workspace-vfs.ts @@ -1237,14 +1237,14 @@ export class WorkspaceVFS { ) } if (compiled.length > MAX_COMPILED_ATTACHMENT_BYTES) { - return bindWorkspaceFileResult(record, { - content: readPlaceholder.compiledArtifactTooLarge( + return bindWorkspaceFileResult( + record, + readPlaceholder.compiledArtifactTooLarge( record.name, compiled.length, MAX_COMPILED_ATTACHMENT_BYTES - ), - totalLines: 1, - }) + ) + ) } return bindWorkspaceFileResult( record, From 50766d1409e421349b92cffe491d031e9cac19e3 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 15:29:59 -0700 Subject: [PATCH 10/10] fix(copilot): use sharp's own pixel default rather than a rounder, lower one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The vulnerability was disabling sharp's pixel guard with limitInputPixels: false, so restoring the library's default is the fix; 100MP was a tidier number we picked, and tidy landed mid-market — a Fuji GFX 100 frame is 11648x8736, or 101.7MP, and was refused by a ceiling that claimed to clear every camera. 268402689 still removes the unbounded declaration the bug allowed and caps a rung near 400ms (measured; the curve is sublinear), while refusing nothing a camera produces. Tests now pin both ends: a 900MP bomb is refused, and 48MP / 61MP / 102MP frames are not. --- apps/sim/lib/copilot/vfs/file-reader.test.ts | 15 ++++++++++++++ apps/sim/lib/copilot/vfs/file-reader.ts | 21 +++++++++++--------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/apps/sim/lib/copilot/vfs/file-reader.test.ts b/apps/sim/lib/copilot/vfs/file-reader.test.ts index 32fe8d11716..531240786b6 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.test.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.test.ts @@ -95,6 +95,21 @@ describe('readFileRecord', () => { SHARP_TEST_TIMEOUT_MS ) + it.each([ + ['48MP iPhone', 8064, 6048], + ['61MP full-frame', 9504, 6336], + ['102MP medium format', 11648, 8736], + ])('does not refuse a %s frame on pixel count', async (_camera, width, height) => { + // Guards the ceiling from being tightened below real hardware. These reach the + // resize ladder and fail there on the stub's truncated pixel data — what matters + // is that they are not turned away by the pixel budget first. + fetchWorkspaceFileBuffer.mockResolvedValue(await makeBombPng(width, height)) + + const result = await readFileRecord(imageRecord('photo.png', 4_000_000)) + + expect(result?.content).not.toContain('It is too large to decode safely.') + }) + it('reports the too-large placeholder when an understated record.size hides an oversized object', async () => { fetchWorkspaceFileBuffer.mockRejectedValue( new PayloadSizeLimitError({ diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index 59f82036097..ac08c7d3e9c 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -65,17 +65,20 @@ export const MAX_IMAGE_SOURCE_BYTES = MAX_WORKSPACE_FORMDATA_FILE_SIZE * Pixel ceiling on the decoded image, and the actual decompression-bomb defence: a * few hundred KB of PNG can declare an arbitrarily large raster. * - * The cost it bounds is CPU, not memory. libvips decodes this pipeline sequentially, - * so peak RSS stays flat (tens of MB) no matter what the header declares — measured - * on this exact pipeline, 100MP..1024MP all sat under ~120MB. What scales is time, - * roughly linearly: ~240ms at 100MP, ~1.35s at 1024MP, once per resize rung. So the - * budget caps what one read can burn, and the `break` below caps how many rungs a - * failing image gets. + * This is sharp's own default rather than a number of our own — the vulnerability + * was disabling it with `limitInputPixels: false`, so restoring it is the fix, and + * any tighter value would be us inventing a ceiling the library did not ask for. A + * round 100MP looked tidy but lands mid-market: a Fuji GFX 100 frame is 11648x8736, + * or 101.7MP, and would have been refused. * - * 100MP clears every single-shot camera (a 48MP iPhone still is 8064x6048) but will - * refuse a stitched gigapixel panorama, which is the known cost of the ceiling. + * The cost it bounds is CPU, not memory. libvips decodes this pipeline sequentially, + * so peak RSS stays flat (tens of MB) whatever the header declares — measured here, + * 100MP..1024MP all sat under ~120MB. Time scales sublinearly: ~240ms at 100MP, + * ~400ms at 256MP, ~1.35s at 1024MP, once per resize rung. So the budget caps the + * worst case at roughly 400ms a rung, the `break` below caps a failing image at four + * rungs, and an unbounded declaration — which is what `false` allowed — is gone. */ -const MAX_IMAGE_INPUT_PIXELS = 100_000_000 +const MAX_IMAGE_INPUT_PIXELS = 268_402_689 const MAX_IMAGE_DIMENSION = 1568 const IMAGE_RESIZE_DIMENSIONS = [1568, 1280, 1024, 768] const IMAGE_QUALITY_STEPS = [85, 70, 55, 40]