From 000c8f67887047ce7bebe60e2088051f060ae7a0 Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan Date: Tue, 30 Jun 2026 19:11:15 -0700 Subject: [PATCH 1/4] zip checkpoint --- apps/sim/app/api/tools/file/manage/route.ts | 248 ++----------- apps/sim/lib/copilot/chat/payload.ts | 29 +- .../lib/copilot/generated/tool-catalog-v1.ts | 6 +- .../lib/copilot/generated/tool-schemas-v1.ts | 4 +- .../tools/handlers/materialize-file.test.ts | 96 ++++- .../tools/handlers/materialize-file.ts | 134 ++++++- .../tools/handlers/upload-file-reader.test.ts | 33 ++ .../tools/handlers/upload-file-reader.ts | 13 +- apps/sim/lib/uploads/archive.test.ts | 225 ++++++++++++ apps/sim/lib/uploads/archive.ts | 336 ++++++++++++++++++ .../workspace/workspace-file-manager.ts | 6 +- apps/sim/lib/uploads/utils/file-utils.ts | 26 +- apps/sim/lib/uploads/utils/validation.ts | 23 +- 13 files changed, 944 insertions(+), 235 deletions(-) create mode 100644 apps/sim/lib/uploads/archive.test.ts create mode 100644 apps/sim/lib/uploads/archive.ts diff --git a/apps/sim/app/api/tools/file/manage/route.ts b/apps/sim/app/api/tools/file/manage/route.ts index 4a2b9e260b1..79149fbe133 100644 --- a/apps/sim/app/api/tools/file/manage/route.ts +++ b/apps/sim/app/api/tools/file/manage/route.ts @@ -1,5 +1,4 @@ import { Buffer, isUtf8 } from 'buffer' -import type { Readable } from 'stream' import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' @@ -21,6 +20,14 @@ import { ShareValidationError, upsertFileShare, } from '@/lib/public-shares/share-manager' +import { + ArchiveError, + decompressArchiveBufferToWorkspaceFiles, + MAX_ARCHIVE_BYTES, + MAX_ARCHIVE_ENTRIES, + MAX_ARCHIVE_ENTRY_BYTES, + MAX_ARCHIVE_TOTAL_BYTES, +} from '@/lib/uploads/archive' import { ensureWorkspaceFileFolderPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { fetchWorkspaceFileBuffer, @@ -203,102 +210,6 @@ const uniqueZipEntryName = (name: string, usedNames: Set): string => { return candidate } -/** Input archive download cap for the decompress operation. */ -const MAX_DECOMPRESS_ARCHIVE_BYTES = 100 * 1024 * 1024 -/** Maximum number of entries extracted from a single archive. */ -const MAX_DECOMPRESS_ENTRIES = 1000 -/** Maximum uncompressed size for any single archive entry. */ -const MAX_DECOMPRESS_ENTRY_BYTES = 100 * 1024 * 1024 -/** Maximum total uncompressed size across all entries, to bound zip-bomb expansion. */ -const MAX_DECOMPRESS_TOTAL_BYTES = 200 * 1024 * 1024 - -const S_IFMT = 0o170000 -const S_IFLNK = 0o120000 - -/** - * Read a zip entry's declared uncompressed size without materializing it. This - * value comes straight from the (attacker-controlled) ZIP metadata, so it is only - * usable as a cheap fast-reject for honestly-declared archives — never as the - * authoritative cap. {@link inflateEntryWithinCaps} enforces the real limit on the - * inflated byte stream. - */ -const readEntryUncompressedSize = (entry: JSZip.JSZipObject): number | undefined => { - const data = (entry as JSZip.JSZipObject & { _data?: { uncompressedSize?: number } })._data - const size = data?.uncompressedSize - return typeof size === 'number' && Number.isFinite(size) ? size : undefined -} - -type InflateResult = { ok: true; buffer: Buffer } | { ok: false; reason: 'entry' | 'total' } - -/** - * Inflate a single zip entry through a streaming counting sink, tearing the - * stream down the moment cumulative output would exceed the per-entry cap or the - * remaining total budget. The declared uncompressed size in the ZIP header is - * attacker-controlled and is NOT trusted here: a forged-small or absent size - * cannot cause the full (potentially gigabyte-scale) entry to be materialized in - * memory, because enforcement happens on the actual inflated bytes as they - * arrive. Peak memory is bounded by the cap plus one DEFLATE chunk. - */ -const inflateEntryWithinCaps = ( - entry: JSZip.JSZipObject, - remainingTotalBudget: number -): Promise => - new Promise((resolve, reject) => { - const chunks: Buffer[] = [] - let size = 0 - let settled = false - const stream = entry.nodeStream() as Readable - - const settle = (result: InflateResult) => { - if (settled) return - settled = true - stream.destroy() - resolve(result) - } - - stream.on('data', (chunk: Buffer) => { - size += chunk.length - if (size > MAX_DECOMPRESS_ENTRY_BYTES) { - settle({ ok: false, reason: 'entry' }) - return - } - if (size > remainingTotalBudget) { - settle({ ok: false, reason: 'total' }) - return - } - chunks.push(chunk) - }) - stream.on('end', () => settle({ ok: true, buffer: Buffer.concat(chunks, size) })) - stream.on('error', (error) => { - if (settled) return - settled = true - stream.destroy() - reject(error) - }) - }) - -/** True when a zip entry's unix mode marks it as a symlink (never extracted). */ -const isSymlinkEntry = (entry: JSZip.JSZipObject): boolean => { - const mode = (entry as JSZip.JSZipObject & { unixPermissions?: number | null }).unixPermissions - return typeof mode === 'number' && (mode & S_IFMT) === S_IFLNK -} - -/** - * Normalize a zip entry path into safe workspace folder segments, guarding against - * zip-slip. Returns null for traversal (`..`), so the entry is skipped rather than - * written outside its intended location. - */ -const sanitizeArchiveEntryPath = (rawPath: string): string[] | null => { - const segments = rawPath - .replace(/\\/g, '/') - .split('/') - .map((segment) => segment.trim()) - .filter((segment) => segment.length > 0 && segment !== '.') - - if (segments.length === 0 || segments.includes('..')) return null - return segments -} - const isLikelyTextBuffer = (buffer: Buffer): boolean => isUtf8(buffer) && !buffer.includes(0) /** @@ -896,135 +807,52 @@ export const POST = withRouteHandler(async (request: NextRequest) => { if (denied) return denied const archiveBuffer = await downloadFileFromStorage(archive, requestId, logger, { - maxBytes: MAX_DECOMPRESS_ARCHIVE_BYTES, + maxBytes: MAX_ARCHIVE_BYTES, }) - let zip: JSZip + let result: Awaited> try { - zip = await JSZip.loadAsync(archiveBuffer) - } catch { - return NextResponse.json( - { success: false, error: `"${archive.name}" is not a valid .zip archive` }, - { status: 400 } - ) - } - - const entries = Object.values(zip.files).filter( - (entry) => !entry.dir && !isSymlinkEntry(entry) - ) - if (entries.length > MAX_DECOMPRESS_ENTRIES) { - return NextResponse.json( - { - success: false, - error: `Archive has too many entries to extract. Maximum is ${MAX_DECOMPRESS_ENTRIES}.`, - }, - { status: 413 } - ) - } - - const entryTooLargeResponse = (name: string) => - NextResponse.json( - { - success: false, - error: `Archive entry "${name}" is too large to extract. Maximum is ${ - MAX_DECOMPRESS_ENTRY_BYTES / (1024 * 1024) - } MB per file.`, - }, - { status: 413 } - ) - const totalTooLargeResponse = () => - NextResponse.json( - { - success: false, - error: `Archive expands to more than the ${ - MAX_DECOMPRESS_TOTAL_BYTES / (1024 * 1024) - } MB extraction limit.`, - }, - { status: 413 } - ) - - // Resolve which entries are safe to extract first, so unsafe entries - // (skipped below) never count toward the size caps. - const safeEntries: Array<{ entry: JSZip.JSZipObject; segments: string[] }> = [] - let skippedCount = 0 - for (const entry of entries) { - const segments = sanitizeArchiveEntryPath(entry.name) - if (!segments) { - skippedCount += 1 - logger.warn('Skipping unsafe archive entry', { name: entry.name }) - continue - } - safeEntries.push({ entry, segments }) - } - - let declaredTotal = 0 - for (const { entry } of safeEntries) { - const declaredSize = readEntryUncompressedSize(entry) - if (declaredSize === undefined) continue - if (declaredSize > MAX_DECOMPRESS_ENTRY_BYTES) return entryTooLargeResponse(entry.name) - declaredTotal += declaredSize - if (declaredTotal > MAX_DECOMPRESS_TOTAL_BYTES) return totalTooLargeResponse() - } - - const pending: Array<{ segments: string[]; buffer: Buffer }> = [] - let totalBytes = 0 - for (const { entry, segments } of safeEntries) { - const result = await inflateEntryWithinCaps( - entry, - MAX_DECOMPRESS_TOTAL_BYTES - totalBytes - ) - if (!result.ok) { - return result.reason === 'entry' - ? entryTooLargeResponse(entry.name) - : totalTooLargeResponse() + result = await decompressArchiveBufferToWorkspaceFiles(archiveBuffer, { + workspaceId, + userId, + }) + } catch (archiveError) { + if (archiveError instanceof ArchiveError) { + const status = archiveError.reason === 'invalid' ? 400 : 413 + const message = + archiveError.reason === 'invalid' + ? `"${archive.name}" is not a valid .zip archive` + : archiveError.reason === 'too_many_entries' + ? `Archive has too many entries to extract. Maximum is ${MAX_ARCHIVE_ENTRIES}.` + : archiveError.reason === 'entry_too_large' + ? `Archive entry "${archiveError.entryName}" is too large to extract. Maximum is ${ + MAX_ARCHIVE_ENTRY_BYTES / (1024 * 1024) + } MB per file.` + : `Archive expands to more than the ${ + MAX_ARCHIVE_TOTAL_BYTES / (1024 * 1024) + } MB extraction limit.` + return NextResponse.json({ success: false, error: message }, { status }) } - totalBytes += result.buffer.length - pending.push({ segments, buffer: result.buffer }) + throw archiveError } - if (pending.length === 0) { + if (result.extracted.length === 0) { return NextResponse.json( - { - success: false, - error: `No files could be extracted from "${archive.name}".`, - }, + { success: false, error: `No files could be extracted from "${archive.name}".` }, { status: 422 } ) } - const folderIdCache = new Map() - const extractedFiles: UserFile[] = [] - for (const { segments, buffer } of pending) { - const leafName = segments[segments.length - 1] - const folderSegments = segments.slice(0, -1) - const folderKey = folderSegments.join('/') - let folderId = folderIdCache.get(folderKey) - if (folderId === undefined) { - folderId = await ensureWorkspaceFileFolderPath({ - workspaceId, - userId, - pathSegments: folderSegments, - }) - folderIdCache.set(folderKey, folderId) - } - - const mimeType = getMimeTypeFromExtension(getFileExtension(leafName)) - const uploaded = await uploadWorkspaceFile( - workspaceId, - userId, - buffer, - leafName, - mimeType, - { folderId } - ) - extractedFiles.push({ ...uploaded, url: ensureAbsoluteUrl(uploaded.url) }) - } + const extractedFiles = result.extracted.map((file) => ({ + ...file, + url: ensureAbsoluteUrl(file.url), + })) logger.info('Archive decompressed', { fileId: archive.id, name: archive.name, extractedCount: extractedFiles.length, - skippedCount, + skippedCount: result.skipped, }) return NextResponse.json({ diff --git a/apps/sim/lib/copilot/chat/payload.ts b/apps/sim/lib/copilot/chat/payload.ts index 5afe545ab87..1ca42814edf 100644 --- a/apps/sim/lib/copilot/chat/payload.ts +++ b/apps/sim/lib/copilot/chat/payload.ts @@ -16,6 +16,7 @@ import { encodeVfsSegment } from '@/lib/copilot/vfs/path-utils' import type { BlockVisibilityState } from '@/lib/core/config/block-visibility' import { isE2BDocEnabled, isHosted } from '@/lib/core/config/env-flags' import { trackChatUpload } from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import { buildArchiveExtractGuidance, isArchiveFileName } from '@/lib/uploads/utils/file-utils' import { stripVersionSuffix } from '@/tools/utils' const logger = createLogger('CopilotChatPayload') @@ -343,15 +344,25 @@ export async function buildCopilotRequestPayload( } catch { encodedUploadName = displayName } - const lines = [ - `File "${displayName}" (${mediaType}, ${f.size} bytes) uploaded.`, - `Read with: read("uploads/${encodedUploadName}")`, - `To save permanently: materialize_file(fileName: "${displayName}")`, - ] - if (displayName.endsWith('.json')) { - lines.push( - `To import as a workflow: materialize_file(fileName: "${displayName}", operation: "import")` - ) + let lines: string[] + if (isArchiveFileName(displayName)) { + // A .zip is stored in uploads/ but its contents aren't readable until + // the agent extracts it once into workspace files/ (explicit step). + lines = [ + `Archive "${displayName}" (${mediaType}, ${f.size} bytes) uploaded.`, + buildArchiveExtractGuidance(displayName), + ] + } else { + lines = [ + `File "${displayName}" (${mediaType}, ${f.size} bytes) uploaded.`, + `Read with: read("uploads/${encodedUploadName}")`, + `To save permanently: materialize_file(fileName: "${displayName}")`, + ] + if (displayName.endsWith('.json')) { + lines.push( + `To import as a workflow: materialize_file(fileName: "${displayName}", operation: "import")` + ) + } } uploadContexts.push({ type: 'uploaded_file', diff --git a/apps/sim/lib/copilot/generated/tool-catalog-v1.ts b/apps/sim/lib/copilot/generated/tool-catalog-v1.ts index 6379a9ff6ff..77fa9e44091 100644 --- a/apps/sim/lib/copilot/generated/tool-catalog-v1.ts +++ b/apps/sim/lib/copilot/generated/tool-catalog-v1.ts @@ -2843,8 +2843,8 @@ export const MaterializeFile: ToolCatalogEntry = { operation: { type: 'string', description: - 'What to do with the file. "save" promotes it to a permanent files/ path. "import" imports a workflow JSON as a workspace workflow. Defaults to "save".', - enum: ['save', 'import'], + 'What to do with the file. "save" promotes it to a permanent files/ path. "import" imports a workflow JSON as a workspace workflow. "extract" decompresses a .zip upload into files//. Defaults to "save".', + enum: ['save', 'import', 'extract'], default: 'save', }, }, @@ -4720,6 +4720,7 @@ export const ManageSkillOperationValues = [ export const MaterializeFileOperation = { save: 'save', import: 'import', + extract: 'extract', } as const export type MaterializeFileOperation = @@ -4728,6 +4729,7 @@ export type MaterializeFileOperation = export const MaterializeFileOperationValues = [ MaterializeFileOperation.save, MaterializeFileOperation.import, + MaterializeFileOperation.extract, ] as const export const QueryUserTableOperation = { diff --git a/apps/sim/lib/copilot/generated/tool-schemas-v1.ts b/apps/sim/lib/copilot/generated/tool-schemas-v1.ts index 95caf84ac66..00646f08622 100644 --- a/apps/sim/lib/copilot/generated/tool-schemas-v1.ts +++ b/apps/sim/lib/copilot/generated/tool-schemas-v1.ts @@ -2656,8 +2656,8 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { operation: { type: 'string', description: - 'What to do with the file. "save" promotes it to a permanent files/ path. "import" imports a workflow JSON as a workspace workflow. Defaults to "save".', - enum: ['save', 'import'], + 'What to do with the file. "save" promotes it to a permanent files/ path. "import" imports a workflow JSON as a workspace workflow. "extract" decompresses a .zip upload into files//. Defaults to "save".', + enum: ['save', 'import', 'extract'], default: 'save', }, }, diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts index 6671ddc33ec..87ff107a6f3 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts @@ -6,6 +6,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockCheckStorageQuotaForBillingContext, + mockDecompress, + mockFetchBuffer, mockFindUpload, mockHasCloudStorage, mockHeadObject, @@ -14,6 +16,8 @@ const { mockResolveStorageBillingContext, } = vi.hoisted(() => ({ mockCheckStorageQuotaForBillingContext: vi.fn(), + mockDecompress: vi.fn(), + mockFetchBuffer: vi.fn(), mockFindUpload: vi.fn(), mockHasCloudStorage: vi.fn(), mockHeadObject: vi.fn(), @@ -33,7 +37,25 @@ vi.mock('@/lib/uploads', () => ({ })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ - fetchWorkspaceFileBuffer: vi.fn(), + fetchWorkspaceFileBuffer: mockFetchBuffer, +})) + +vi.mock('@/lib/uploads/archive', () => ({ + decompressArchiveBufferToWorkspaceFiles: mockDecompress, + ArchiveError: class ArchiveError extends Error { + reason: string + entryName?: string + constructor(reason: string, message: string, entryName?: string) { + super(message) + this.name = 'ArchiveError' + this.reason = reason + this.entryName = entryName + } + }, + MAX_ARCHIVE_BYTES: 100 * 1024 * 1024, + MAX_ARCHIVE_ENTRIES: 1000, + MAX_ARCHIVE_ENTRY_BYTES: 100 * 1024 * 1024, + MAX_ARCHIVE_TOTAL_BYTES: 200 * 1024 * 1024, })) vi.mock('@/lib/uploads/core/storage-service', () => ({ @@ -293,3 +315,75 @@ describe('executeMaterializeFile - save storage transition', () => { expect(mockMaybeNotifyStorageLimitForBillingContext).not.toHaveBeenCalled() }) }) + +describe('executeMaterializeFile - extract operation', () => { + beforeEach(() => vi.clearAllMocks()) + + function zipRow(overrides: Record = {}) { + return { + id: 'wf_zip', + key: 'mothership/abc/bundle.zip', + userId: 'user-1', + workspaceId: 'ws-1', + context: 'mothership', + chatId: 'chat-1', + originalName: 'bundle.zip', + displayName: 'bundle.zip', + contentType: 'application/zip', + size: 2048, + deletedAt: null, + uploadedAt: new Date(), + updatedAt: new Date(), + ...overrides, + } + } + + it('dispatches to the archive extractor and returns the unpacked files', async () => { + mockFindUpload.mockResolvedValue(zipRow()) + mockFetchBuffer.mockResolvedValue(Buffer.from('zip-bytes')) + mockDecompress.mockResolvedValue({ + extracted: [ + { id: 'f1', name: 'a.txt', url: '/x', size: 1, type: 'text/plain', key: 'k1' }, + { id: 'f2', name: 'b.txt', url: '/y', size: 2, type: 'text/plain', key: 'k2' }, + ], + skipped: 0, + rootFolderPath: 'files/bundle', + }) + + const result = await executeMaterializeFile( + { fileNames: ['bundle.zip'], operation: 'extract' }, + context + ) + + expect(result.success).toBe(true) + expect(mockDecompress).toHaveBeenCalledTimes(1) + expect(mockDecompress).toHaveBeenCalledWith( + expect.any(Buffer), + expect.objectContaining({ + workspaceId: 'ws-1', + userId: 'user-1', + rootFolderSegments: ['bundle'], + skipNoiseEntries: true, + }) + ) + expect(result.output).toMatchObject({ succeeded: ['bundle.zip'], failed: [] }) + expect(result.resources).toEqual([ + { type: 'file', id: 'f1', title: 'a.txt' }, + { type: 'file', id: 'f2', title: 'b.txt' }, + ]) + }) + + it('refuses to extract an upload that belongs to a different workspace', async () => { + mockFindUpload.mockResolvedValue(zipRow({ workspaceId: 'other-ws' })) + + const result = await executeMaterializeFile( + { fileNames: ['bundle.zip'], operation: 'extract' }, + context + ) + + expect(result.success).toBe(false) + const output = result.output as { failed: Array<{ fileName: string; error: string }> } + expect(output.failed[0].error).toContain('does not belong to this workspace') + expect(mockDecompress).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts index 407e41b58ab..098231a7d58 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts @@ -15,8 +15,17 @@ import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/typ import { findMothershipUploadRowByChatAndName } from '@/lib/copilot/tools/handlers/upload-file-reader' import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils' import { getServePathPrefix } from '@/lib/uploads' +import { + ArchiveError, + decompressArchiveBufferToWorkspaceFiles, + MAX_ARCHIVE_BYTES, + MAX_ARCHIVE_ENTRIES, + MAX_ARCHIVE_ENTRY_BYTES, + MAX_ARCHIVE_TOTAL_BYTES, +} from '@/lib/uploads/archive' import { fetchWorkspaceFileBuffer } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { hasCloudStorage, headObject } from '@/lib/uploads/core/storage-service' +import { isArchiveFileName } from '@/lib/uploads/utils/file-utils' import { parseWorkflowJson } from '@/lib/workflows/operations/import-export' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' import { deduplicateWorkflowName } from '@/lib/workflows/utils' @@ -254,6 +263,120 @@ async function executeImport( } } +/** Map an {@link ArchiveError} reason to a clear, user-facing extract message. */ +function archiveErrorMessage(err: ArchiveError, fileName: string): string { + switch (err.reason) { + case 'invalid': + return `"${fileName}" is not a valid .zip archive.` + case 'too_many_entries': + return `"${fileName}" has too many entries to extract. Maximum is ${MAX_ARCHIVE_ENTRIES}.` + case 'entry_too_large': + return `Archive entry "${err.entryName}" is too large to extract. Maximum is ${ + MAX_ARCHIVE_ENTRY_BYTES / (1024 * 1024) + } MB per file.` + case 'total_too_large': + return `"${fileName}" expands to more than the ${ + MAX_ARCHIVE_TOTAL_BYTES / (1024 * 1024) + } MB extraction limit.` + } +} + +/** + * Decompress an uploaded `.zip` into the workspace `files//` folder tree + * (reusing the shared, capped, zip-slip/bomb-safe extractor). The raw archive + * stays in uploads/; the extracted files persist in the workspace so the agent + * can read them with the normal files/ tooling. This is the explicit "extract + * before reading a zip" step. + */ +async function executeExtract( + fileName: string, + chatId: string, + workspaceId: string, + userId: string +): Promise { + const row = await findMothershipUploadRowByChatAndName(chatId, fileName) + if (!row) { + return { + success: false, + error: `Upload not found: "${fileName}". Use glob("uploads/*") to list available uploads.`, + } + } + + // Defense in depth: the resolver is chat-scoped, but never extract an upload + // that belongs to a different workspace than the one driving this request. + if (row.workspaceId !== workspaceId) { + return { + success: false, + error: `Upload "${fileName}" does not belong to this workspace.`, + } + } + + const displayName = row.displayName ?? row.originalName + if (!isArchiveFileName(displayName)) { + return { + success: false, + error: `"${fileName}" is not a .zip archive — only .zip uploads can be extracted. Read it directly with read("uploads/${fileName}").`, + } + } + + const record = toFileRecord(row) + if (record.size > MAX_ARCHIVE_BYTES) { + return { + success: false, + error: `Archive too large to extract: "${fileName}" (${Math.round( + record.size / 1024 / 1024 + )}MB, limit ${MAX_ARCHIVE_BYTES / 1024 / 1024}MB).`, + } + } + + const baseName = displayName.replace(/\.zip$/i, '').trim() || 'archive' + + let result: Awaited> + try { + const buffer = await fetchWorkspaceFileBuffer(record, { maxBytes: MAX_ARCHIVE_BYTES }) + result = await decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId, + userId, + rootFolderSegments: [baseName], + // The agent-facing extract drops macOS/Windows filesystem cruft so the + // unpacked files/ tree only contains meaningful entries. + skipNoiseEntries: true, + }) + } catch (err) { + if (err instanceof ArchiveError) { + return { success: false, error: archiveErrorMessage(err, fileName) } + } + throw err + } + + if (result.extracted.length === 0) { + return { success: false, error: `No files could be extracted from "${fileName}".` } + } + + // Use the canonical VFS path the extractor actually wrote to, so the glob/read + // hint resolves rather than echoing a hand-encoded (possibly mismatched) name. + const folderPath = result.rootFolderPath + const count = result.extracted.length + + logger.info('Extracted archive into workspace files', { + fileName, + chatId, + folder: baseName, + extractedCount: count, + skipped: result.skipped, + }) + + return { + success: true, + output: { + message: `Extracted ${count} file${count === 1 ? '' : 's'} from "${fileName}" into ${folderPath}/. They now persist in the workspace — list them with glob("${folderPath}/**") and read one with read("${folderPath}//content").`, + fileCount: count, + path: folderPath, + }, + resources: result.extracted.map((f) => ({ type: 'file' as const, id: f.id, title: f.name })), + } +} + export async function executeMaterializeFile( params: Record, context: ExecutionContext @@ -275,12 +398,13 @@ export async function executeMaterializeFile( } const operation = (params.operation as string | undefined) || 'save' - // Only save/import are implemented. Reject anything else with guidance instead of - // silently falling back to save (table/knowledge_base are handled by their subagents). - if (operation !== 'save' && operation !== 'import') { + // save (promote upload → workspace file), import (JSON → workflow), and extract + // (decompress a .zip upload → workspace files/) are implemented. Reject anything + // else with guidance instead of silently falling back to save. + if (operation !== 'save' && operation !== 'import' && operation !== 'extract') { return { success: false, - error: `Unsupported materialize_file operation "${operation}". Use "save" or "import". For CSV/TSV/JSON → use the table subagent; for documents → use the knowledge subagent.`, + error: `Unsupported materialize_file operation "${operation}". Use "save", "import", or "extract". For CSV/TSV/JSON → use the table subagent; for documents → use the knowledge subagent.`, } } const succeeded: string[] = [] @@ -292,6 +416,8 @@ export async function executeMaterializeFile( let result: ToolCallResult if (operation === 'import') { result = await executeImport(fileName, context.chatId, context.workspaceId, context.userId) + } else if (operation === 'extract') { + result = await executeExtract(fileName, context.chatId, context.workspaceId, context.userId) } else { result = await executeSave(fileName, context.chatId, context.workspaceId) } diff --git a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts index 065b287b99e..c5cc86548f5 100644 --- a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts @@ -15,8 +15,10 @@ vi.mock('@/lib/copilot/vfs/file-reader', () => ({ readFileRecord: mockReadFileRecord, })) +import { WorkspaceFileGrepError } from '@/lib/copilot/vfs/operations' import { findMothershipUploadRowByChatAndName, + grepChatUpload, listChatUploads, readChatUpload, } from './upload-file-reader' @@ -167,6 +169,17 @@ describe('readChatUpload', () => { ) }) + it('returns extract-first guidance for a .zip upload instead of reading bytes', async () => { + const row = makeRow({ id: 'wf_z', displayName: 'bundle.zip', contentType: 'application/zip' }) + mockOrderByThenLimit([row]) + + const result = await readChatUpload('bundle.zip', CHAT_ID) + + expect(result?.content).toContain('materialize_file') + expect(result?.content).toContain('extract') + expect(mockReadFileRecord).not.toHaveBeenCalled() + }) + it('returns null when no row matches', async () => { mockOrderByThenLimit([]) dbChainMockFns.orderBy.mockResolvedValueOnce([] as never) @@ -177,3 +190,23 @@ describe('readChatUpload', () => { expect(mockReadFileRecord).not.toHaveBeenCalled() }) }) + +describe('grepChatUpload', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockReadFileRecord.mockReset() + }) + + it('throws WorkspaceFileGrepError with extract-first guidance for a .zip upload', async () => { + const row = makeRow({ id: 'wf_z', displayName: 'bundle.zip', contentType: 'application/zip' }) + mockOrderByThenLimit([row]) + + const error = await grepChatUpload('bundle.zip', CHAT_ID, 'foo').catch((e) => e) + + expect(error).toBeInstanceOf(WorkspaceFileGrepError) + expect(error.message).toContain('materialize_file') + expect(error.message).toContain('extract') + expect(mockReadFileRecord).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts index 0e914229c8a..ea3de99a491 100644 --- a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts +++ b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts @@ -14,6 +14,7 @@ import { import { decodeVfsSegment, encodeVfsSegment } from '@/lib/copilot/vfs/path-utils' import { getServePathPrefix } from '@/lib/uploads' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import { buildArchiveExtractGuidance, isArchiveFileName } from '@/lib/uploads/utils/file-utils' const logger = createLogger('UploadFileReader') @@ -142,7 +143,8 @@ export async function listChatUploads(chatId: string): Promise ({ + mockEnsureFolder: vi.fn(), + mockUpload: vi.fn(), +})) +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-folder-manager', () => ({ + ensureWorkspaceFileFolderPath: mockEnsureFolder, +})) +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + uploadWorkspaceFile: mockUpload, +})) + +import { + decompressArchiveBufferToWorkspaceFiles, + MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES, + MAX_ARCHIVE_CENTRAL_DIR_RECORDS, + MAX_ARCHIVE_ENTRY_BYTES, +} from '@/lib/uploads/archive' + +async function buildZip( + files: Record, + opts?: { symlinks?: string[] } +): Promise { + const zip = new JSZip() + for (const [name, content] of Object.entries(files)) { + const isLink = opts?.symlinks?.includes(name) + zip.file(name, content, isLink ? { unixPermissions: 0o120777 } : undefined) + } + // platform: 'UNIX' so unixPermissions (incl. the symlink mode) round-trip, + // mirroring how macOS/Linux `zip` authors archives. + return Buffer.from(await zip.generateAsync({ type: 'uint8array', platform: 'UNIX' })) +} + +beforeEach(() => { + vi.clearAllMocks() + mockEnsureFolder.mockResolvedValue('folder_1') + mockUpload.mockImplementation(async (_ws: string, _uid: string, buf: Buffer, name: string) => ({ + id: `f_${name}`, + name, + url: `/api/files/serve/${name}`, + key: `workspace/ws/${name}`, + size: buf.length, + type: 'text/plain', + })) +}) + +describe('decompressArchiveBufferToWorkspaceFiles', () => { + it('extracts entries as workspace files under the root folder', async () => { + const buffer = await buildZip({ 'report.txt': 'hi', 'data/sheet.csv': 'a,b' }) + + const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + userId: 'u', + rootFolderSegments: ['bundle'], + }) + + expect(result.extracted).toHaveLength(2) + // The canonical, per-segment-encoded VFS path the files were written under. + expect(result.rootFolderPath).toBe('files/bundle') + expect(mockUpload).toHaveBeenCalledTimes(2) + const leafNames = mockUpload.mock.calls.map((c) => c[3]).sort() + expect(leafNames).toEqual(['report.txt', 'sheet.csv']) + // Entries are rooted under the archive's folder; nested paths are preserved. + expect(mockEnsureFolder).toHaveBeenCalledWith( + expect.objectContaining({ pathSegments: ['bundle'] }) + ) + expect(mockEnsureFolder).toHaveBeenCalledWith( + expect.objectContaining({ pathSegments: ['bundle', 'data'] }) + ) + }) + + it('returns the encoded root folder path for names that need encoding', async () => { + const buffer = await buildZip({ 'a.txt': 'x' }) + + const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + userId: 'u', + rootFolderSegments: ['My Archive'], + }) + + expect(result.rootFolderPath).toBe('files/My%20Archive') + }) + + it('rejects an archive with more central-directory records than the cap, before parsing', async () => { + // One 32-byte central-directory header per record, laid out non-contiguously + // (a full fixed-size header apart) so the signature scan strides one header + // at a time and reads each record's real extra-field-length field — here a + // hard 0 — instead of misreading a neighboring signature's bytes. That keeps + // summed extra bytes at exactly 0, so this archive trips the RECORD cap and + // never the extra-bytes cap, isolating record-count regression coverage. + // JSZip would build one entry per signature it finds (ignoring the EOCD + // count), so the pre-scan must reject this before loadAsync ever runs. + const BLOCK_SIZE = 32 + const records = MAX_ARCHIVE_CENTRAL_DIR_RECORDS + 1 + const buffer = Buffer.alloc(records * BLOCK_SIZE) + for (let r = 0; r < records; r++) { + const base = r * BLOCK_SIZE + // bytes 0-3: PK\x01\x02 central-directory file header signature + buffer[base] = 0x50 + buffer[base + 1] = 0x4b + buffer[base + 2] = 0x01 + buffer[base + 3] = 0x02 + // bytes 30-31: extra field length = 0 (little-endian) → adds zero toward + // MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES, so only the record cap can trip. + buffer.writeUInt16LE(0, base + 30) + } + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { workspaceId: 'ws', userId: 'u' }) + ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'too_many_entries' }) + expect(mockUpload).not.toHaveBeenCalled() + }) + + it('rejects an archive whose central-directory extra fields exceed the byte cap, before parsing', async () => { + // A handful of central-directory headers — far below the record cap — each + // declaring the maximum 0xFFFF extra-field length, so their summed extra + // bytes cross MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES. JSZip would allocate one + // retained object per declared extra field during loadAsync, so the pre-scan + // must reject on summed extra bytes, not just on record count. + const BLOCK_SIZE = 32 + const EXTRA_PER_RECORD = 0xffff + // +5 records of headroom past the cap; still << MAX_ARCHIVE_CENTRAL_DIR_RECORDS + // so THIS cap (extra bytes), not the record count, is what triggers rejection. + const records = Math.ceil(MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES / EXTRA_PER_RECORD) + 5 + expect(records).toBeLessThan(MAX_ARCHIVE_CENTRAL_DIR_RECORDS) + const buffer = Buffer.alloc(records * BLOCK_SIZE) + for (let r = 0; r < records; r++) { + const base = r * BLOCK_SIZE + // bytes 0-3: PK\x01\x02 central-directory file header signature + buffer[base] = 0x50 + buffer[base + 1] = 0x4b + buffer[base + 2] = 0x01 + buffer[base + 3] = 0x02 + // bytes 30-31: extra field length = 0xFFFF (little-endian), the CD max + buffer.writeUInt16LE(EXTRA_PER_RECORD, base + 30) + } + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { workspaceId: 'ws', userId: 'u' }) + ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'too_many_entries' }) + expect(mockUpload).not.toHaveBeenCalled() + }) + + it('throws ArchiveError invalid for a non-zip buffer (no files written)', async () => { + await expect( + decompressArchiveBufferToWorkspaceFiles(Buffer.from('not a zip at all'), { + workspaceId: 'ws', + userId: 'u', + }) + ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'invalid' }) + expect(mockUpload).not.toHaveBeenCalled() + }) + + it('excludes symlinks (uncounted) and skips zip-slip traversal (counted in skipped)', async () => { + const buffer = await buildZip( + { + 'safe.txt': 'ok', + '..\\evil.txt': 'evil', + 'link.txt': '/etc/passwd', + }, + { symlinks: ['link.txt'] } + ) + + const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + userId: 'u', + }) + + // Only the traversal entry counts toward `skipped`; the symlink is filtered + // out before the skip tally (it never becomes a candidate entry). + expect(result.extracted).toHaveLength(1) + expect(result.skipped).toBe(1) + expect(mockUpload).toHaveBeenCalledTimes(1) + expect(mockUpload.mock.calls[0][3]).toBe('safe.txt') + }) + + it('extracts macOS/Windows filesystem-noise entries by default (skipNoiseEntries unset)', async () => { + const buffer = await buildZip({ '__MACOSX/a.txt': 'x', '.DS_Store': 'y' }) + + const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + userId: 'u', + }) + + // Parity with the HTTP decompress route, which extracts these verbatim. + expect(result.extracted).toHaveLength(2) + expect(result.skipped).toBe(0) + expect(mockUpload).toHaveBeenCalledTimes(2) + }) + + it('drops filesystem-noise entries when skipNoiseEntries is set', async () => { + const buffer = await buildZip({ '__MACOSX/a.txt': 'x', '.DS_Store': 'y' }) + + const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + userId: 'u', + skipNoiseEntries: true, + }) + + expect(result.extracted).toEqual([]) + expect(result.skipped).toBe(2) + expect(mockUpload).not.toHaveBeenCalled() + }) + + it('rejects an entry whose declared size exceeds the per-entry cap', async () => { + const zip = new JSZip() + // Highly compressible zeros keep the archive tiny on disk while the declared + // uncompressed size blows past the per-entry cap. + zip.file('big.bin', Buffer.alloc(MAX_ARCHIVE_ENTRY_BYTES + 1024)) + const buffer = Buffer.from( + await zip.generateAsync({ type: 'uint8array', compression: 'DEFLATE' }) + ) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { workspaceId: 'ws', userId: 'u' }) + ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'entry_too_large' }) + expect(mockUpload).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts new file mode 100644 index 00000000000..9cab0a509db --- /dev/null +++ b/apps/sim/lib/uploads/archive.ts @@ -0,0 +1,336 @@ +import { Buffer } from 'buffer' +import type { Readable } from 'stream' +import JSZip from 'jszip' +import { encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils' +import { ensureWorkspaceFileFolderPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' +import { uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' +import type { UserFile } from '@/executor/types' + +/** + * Shared, zip-bomb / zip-slip safe archive primitives plus the one-shot + * "decompress into workspace files/" extractor. + * + * The declared sizes in a ZIP header are attacker-controlled, so the real caps + * are always enforced on the inflated byte stream — never on metadata. The + * parser's object graph is additionally bounded BEFORE parsing via + * {@link centralDirExceedsCaps}, which counts central-directory signatures + * (JSZip builds one entry per signature it finds, NOT per the EOCD's declared + * count) and sums their declared extra-field bytes (JSZip retains one object per + * central-directory extra field) so a crafted archive — whether packed with + * millions of records or a handful of records stuffed with tiny extra fields — + * cannot balloon the parser's heap. Extraction is sequential (one entry inflated + * and uploaded at a time) so peak memory is ~one entry. + */ + +/** Input archive download/size cap. */ +export const MAX_ARCHIVE_BYTES = 100 * 1024 * 1024 +/** Maximum number of entries extracted from a single archive. */ +export const MAX_ARCHIVE_ENTRIES = 1000 +/** Maximum uncompressed size for any single archive entry. */ +export const MAX_ARCHIVE_ENTRY_BYTES = 100 * 1024 * 1024 +/** Maximum total uncompressed size across all entries, to bound zip-bomb expansion. */ +export const MAX_ARCHIVE_TOTAL_BYTES = 200 * 1024 * 1024 + +const S_IFMT = 0o170000 +const S_IFLNK = 0o120000 + +const CD_SIGNATURE = Buffer.from([0x50, 0x4b, 0x01, 0x02]) // PK\x01\x02 central-directory file header +/** Memory bound for the parse-time object graph (distinct from the file-extraction cap). */ +export const MAX_ARCHIVE_CENTRAL_DIR_RECORDS = 10_000 +/** + * Cap on the summed declared extra-field bytes across all central-directory + * records. JSZip allocates one object per CD extra field (>= 4 bytes each), so + * bounding the summed extra-field bytes bounds the parse-time object graph even + * when the record count is tiny (one record may declare up to 65535 extra bytes + * ≈ 16k tiny fields). Legit archives use only tens of bytes/entry, so 4MB is + * generous headroom. + */ +export const MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES = 4 * 1024 * 1024 + +/** Reason a {@link ArchiveError} was raised, for mapping to a caller response. */ +type ArchiveErrorReason = 'invalid' | 'too_many_entries' | 'entry_too_large' | 'total_too_large' + +/** Raised for malformed archives and cap violations so callers can surface a clear message. */ +export class ArchiveError extends Error { + readonly reason: ArchiveErrorReason + readonly entryName?: string + + constructor(reason: ArchiveErrorReason, message: string, entryName?: string) { + super(message) + this.name = 'ArchiveError' + this.reason = reason + this.entryName = entryName + } +} + +/** + * True when the archive's central directory would push JSZip's parse-time object + * graph past a safe bound. A single pass over every `PK\x01\x02` signature bounds + * TWO independent memory vectors: + * - RECORD COUNT: JSZip builds one ZipEntry per CD signature it finds (it does + * NOT trust the EOCD count), so counting signatures bounds the entry graph. + * - EXTRA-FIELD BYTES: `readExtraFields` retains one `{id,length,value}` object + * per extra field, and every extra field is >= 4 bytes, so the summed declared + * extra-field length is an upper bound on those objects. Record count alone + * does NOT bound this — one record may declare up to 65535 extra bytes — so + * the extra-field cap is enforced separately. + * + * This is a conservative UPPER bound on JSZip's real allocation: JSZip only builds + * entries for the contiguous run of signatures starting at the EOCD's + * centralDirOffset, so counting ALL signatures and summing ALL their declared + * extra lengths is always >= what JSZip allocates. That makes it immune to a + * lied/absent EOCD count and to ZIP64 (the CD file-header signature is unchanged + * there). Over-counting only ever rejects, which is safe. + */ +function centralDirExceedsCaps(buffer: Buffer): boolean { + let records = 0 + let extraBytes = 0 + for (let i = buffer.indexOf(CD_SIGNATURE); i !== -1; i = buffer.indexOf(CD_SIGNATURE, i + 4)) { + records += 1 + // The CD header's uint16 LE "extra field length" lives at offset +30; only + // read it when the fixed-size header's extra-length field is fully present. + if (i + 32 <= buffer.length) { + extraBytes += buffer.readUInt16LE(i + 30) + } + if ( + records > MAX_ARCHIVE_CENTRAL_DIR_RECORDS || + extraBytes > MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES + ) { + return true + } + } + return false +} + +/** + * Read a zip entry's declared uncompressed size without materializing it. Comes + * straight from (attacker-controlled) ZIP metadata, so it is only a cheap + * fast-reject for honestly-declared archives — never the authoritative cap. + */ +const readEntryUncompressedSize = (entry: JSZip.JSZipObject): number | undefined => { + const data = (entry as JSZip.JSZipObject & { _data?: { uncompressedSize?: number } })._data + const size = data?.uncompressedSize + return typeof size === 'number' && Number.isFinite(size) ? size : undefined +} + +type InflateResult = { ok: true; buffer: Buffer } | { ok: false; reason: 'entry' | 'total' } + +/** + * Inflate a single zip entry through a streaming counting sink, tearing the + * stream down the moment cumulative output would exceed the per-entry cap or the + * remaining total budget. The declared uncompressed size is NOT trusted: + * enforcement happens on the actual inflated bytes as they arrive, so peak memory + * is bounded by the cap plus one DEFLATE chunk. + */ +const inflateEntryWithinCaps = ( + entry: JSZip.JSZipObject, + remainingTotalBudget: number +): Promise => + new Promise((resolve, reject) => { + const chunks: Buffer[] = [] + let size = 0 + let settled = false + const stream = entry.nodeStream() as Readable + + const settle = (result: InflateResult) => { + if (settled) return + settled = true + stream.destroy() + resolve(result) + } + + stream.on('data', (chunk: Buffer) => { + size += chunk.length + if (size > MAX_ARCHIVE_ENTRY_BYTES) { + settle({ ok: false, reason: 'entry' }) + return + } + if (size > remainingTotalBudget) { + settle({ ok: false, reason: 'total' }) + return + } + chunks.push(chunk) + }) + stream.on('end', () => settle({ ok: true, buffer: Buffer.concat(chunks, size) })) + stream.on('error', (error) => { + if (settled) return + settled = true + stream.destroy() + reject(error) + }) + }) + +/** True when a zip entry's unix mode marks it as a symlink (never extracted). */ +const isSymlinkEntry = (entry: JSZip.JSZipObject): boolean => { + const mode = (entry as JSZip.JSZipObject & { unixPermissions?: number | null }).unixPermissions + return typeof mode === 'number' && (mode & S_IFMT) === S_IFLNK +} + +/** + * Normalize a zip entry path into safe segments, guarding against zip-slip. + * Returns null for traversal (`..`) and empty paths so the entry is skipped. + */ +const sanitizeArchiveEntryPath = (rawPath: string): string[] | null => { + const segments = rawPath + .replace(/\\/g, '/') + .split('/') + .map((segment) => segment.trim()) + .filter((segment) => segment.length > 0 && segment !== '.') + + if (segments.length === 0 || segments.includes('..')) return null + return segments +} + +/** Filesystem cruft that should never surface as a readable archive entry. */ +const isArchiveNoiseEntry = (segments: string[]): boolean => { + if (segments[0] === '__MACOSX') return true + const leaf = segments[segments.length - 1] + return leaf === '.DS_Store' || leaf === 'Thumbs.db' +} + +interface DecompressResult { + /** Workspace files created under the target folder, in extraction order. */ + extracted: UserFile[] + /** Count of entries skipped as unsafe (zip-slip) or filesystem noise. */ + skipped: number + /** + * Canonical, per-segment-encoded VFS path of the root folder the archive was + * extracted into (e.g. `files/My%20Archive`), or `files` for the workspace + * root. Matches what the workspace VFS serves, so the glob/read hint built + * from it resolves to the files that were just written. + */ + rootFolderPath: string +} + +/** + * Decompress an archive buffer into workspace files under `rootFolderSegments` + * (default: the workspace root). Reuses the same caps and zip-slip / zip-bomb / + * symlink guards everywhere. Throws {@link ArchiveError} for an invalid archive + * or a cap violation; returns `{ extracted: [] }` when every entry was skipped. + * + * Memory is bounded: the central-directory record count and summed extra-field + * bytes are gated pre-parse, and entries are inflated and uploaded one at a time, + * so peak working set is ~one entry (≤ the per-entry cap) regardless of how many + * files the archive holds. + * + * Filesystem-noise entries (`__MACOSX/`, `.DS_Store`, `Thumbs.db`) are extracted + * verbatim unless `skipNoiseEntries` is set — the HTTP decompress route preserves + * them; the agent-facing extract path drops them. + */ +export async function decompressArchiveBufferToWorkspaceFiles( + buffer: Buffer, + opts: { + workspaceId: string + userId: string + rootFolderSegments?: string[] + skipNoiseEntries?: boolean + } +): Promise { + const { workspaceId, userId, rootFolderSegments = [], skipNoiseEntries = false } = opts + + if (centralDirExceedsCaps(buffer)) { + throw new ArchiveError( + 'too_many_entries', + `Archive has too many entries. Maximum is ${MAX_ARCHIVE_ENTRIES}.` + ) + } + + let zip: JSZip + try { + zip = await JSZip.loadAsync(buffer) + } catch { + throw new ArchiveError('invalid', 'Not a valid .zip archive') + } + + const realEntries = Object.values(zip.files).filter( + (entry) => !entry.dir && !isSymlinkEntry(entry) + ) + if (realEntries.length > MAX_ARCHIVE_ENTRIES) { + throw new ArchiveError( + 'too_many_entries', + `Archive has too many entries. Maximum is ${MAX_ARCHIVE_ENTRIES}.` + ) + } + + // Resolve safe entries first so unsafe ones never count toward the size caps. + const safeEntries: Array<{ entry: JSZip.JSZipObject; segments: string[] }> = [] + let skipped = 0 + for (const entry of realEntries) { + const segments = sanitizeArchiveEntryPath(entry.name) + if (!segments || (skipNoiseEntries && isArchiveNoiseEntry(segments))) { + skipped += 1 + continue + } + safeEntries.push({ entry, segments }) + } + + // Cheap declared-size fast-reject for honestly-declared archives. + let declaredTotal = 0 + for (const { entry } of safeEntries) { + const declaredSize = readEntryUncompressedSize(entry) + if (declaredSize === undefined) continue + if (declaredSize > MAX_ARCHIVE_ENTRY_BYTES) { + throw new ArchiveError( + 'entry_too_large', + `Archive entry "${entry.name}" is too large to extract.`, + entry.name + ) + } + declaredTotal += declaredSize + if (declaredTotal > MAX_ARCHIVE_TOTAL_BYTES) { + throw new ArchiveError('total_too_large', 'Archive expands beyond the extraction limit.') + } + } + + const folderIdCache = new Map() + const extracted: UserFile[] = [] + let totalBytes = 0 + for (const { entry, segments } of safeEntries) { + const result = await inflateEntryWithinCaps(entry, MAX_ARCHIVE_TOTAL_BYTES - totalBytes) + if (!result.ok) { + throw new ArchiveError( + result.reason === 'entry' ? 'entry_too_large' : 'total_too_large', + result.reason === 'entry' + ? `Archive entry "${entry.name}" is too large to extract.` + : 'Archive expands beyond the extraction limit.', + entry.name + ) + } + totalBytes += result.buffer.length + + const leafName = segments[segments.length - 1] + const folderSegments = [...rootFolderSegments, ...segments.slice(0, -1)] + const folderKey = folderSegments.join('/') + let folderId = folderIdCache.get(folderKey) + if (folderId === undefined) { + folderId = await ensureWorkspaceFileFolderPath({ + workspaceId, + userId, + pathSegments: folderSegments, + }) + folderIdCache.set(folderKey, folderId) + } + + const mimeType = getMimeTypeFromExtension(getFileExtension(leafName)) + const uploaded = await uploadWorkspaceFile( + workspaceId, + userId, + result.buffer, + leafName, + mimeType, + { + folderId, + } + ) + extracted.push(uploaded) + } + + // Encode the root segments the same way the workspace VFS serves folder names + // (per-segment `encodeVfsSegment`), so the advertised path matches the files + // that were just written rather than the raw, unencoded display name. + const rootFolderPath = + rootFolderSegments.length > 0 ? `files/${encodeVfsPathSegments(rootFolderSegments)}` : 'files' + + return { extracted, skipped, rootFolderPath } +} 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 18d26c5f627..6967a7d313f 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -970,13 +970,17 @@ export async function getWorkspaceFile( /** * Download workspace file content */ -export async function fetchWorkspaceFileBuffer(fileRecord: WorkspaceFileRecord): Promise { +export async function fetchWorkspaceFileBuffer( + fileRecord: WorkspaceFileRecord, + options: { maxBytes?: number } = {} +): Promise { logger.info(`Downloading workspace file: ${fileRecord.name}`) try { const buffer = await downloadFile({ key: fileRecord.key, context: fileRecord.storageContext ?? 'workspace', + maxBytes: options.maxBytes, }) logger.info( `Successfully downloaded workspace file: ${fileRecord.name} (${buffer.length} bytes)` diff --git a/apps/sim/lib/uploads/utils/file-utils.ts b/apps/sim/lib/uploads/utils/file-utils.ts index 2e0f0d57cb2..0b415171e3f 100644 --- a/apps/sim/lib/uploads/utils/file-utils.ts +++ b/apps/sim/lib/uploads/utils/file-utils.ts @@ -1,7 +1,11 @@ import type { Logger } from '@sim/logger' import { omit } from '@sim/utils/object' import type { StorageContext } from '@/lib/uploads' -import { ACCEPTED_FILE_TYPES, SUPPORTED_DOCUMENT_EXTENSIONS } from '@/lib/uploads/utils/validation' +import { + ACCEPTED_FILE_TYPES, + SUPPORTED_ARCHIVE_EXTENSIONS, + SUPPORTED_DOCUMENT_EXTENSIONS, +} from '@/lib/uploads/utils/validation' import { isUuid } from '@/executor/constants' import type { UserFile } from '@/executor/types' @@ -206,6 +210,26 @@ export function getFileExtension(filename: string): string { return lastDot !== -1 ? filename.slice(lastDot + 1).toLowerCase() : '' } +const ARCHIVE_EXTENSIONS = new Set(SUPPORTED_ARCHIVE_EXTENSIONS) + +/** + * True when a file name is a supported archive (zip). Detection is by extension + * so it is robust to the varied/empty MIME types browsers assign to archives. + */ +export function isArchiveFileName(filename: string): boolean { + return ARCHIVE_EXTENSIONS.has(getFileExtension(filename)) +} + +/** + * Single source of truth for the "extract a .zip first" guidance shown wherever + * the agent tries to read/grep a raw archive (upload reader, chat payload). A + * `.zip`'s contents aren't readable until it is decompressed into workspace + * `files/`, so this points at the explicit one-time extract step. + */ +export function buildArchiveExtractGuidance(name: string): string { + return `"${name}" is a .zip archive — its contents can't be read directly. Extract it once with materialize_file(fileName: "${name}", operation: "extract"), then read the unpacked files under files/ (e.g. glob("files//**") then read("files///content")).` +} + const EXTENSION_TO_MIME: Record = { // Images jpg: 'image/jpeg', diff --git a/apps/sim/lib/uploads/utils/validation.ts b/apps/sim/lib/uploads/utils/validation.ts index b4e27684f63..a41fc8cb152 100644 --- a/apps/sim/lib/uploads/utils/validation.ts +++ b/apps/sim/lib/uploads/utils/validation.ts @@ -95,6 +95,13 @@ export const SUPPORTED_AUDIO_EXTENSIONS = [ export const SUPPORTED_VIDEO_EXTENSIONS = ['mp4', 'mov', 'avi', 'mkv', 'webm'] as const +/** + * Archive formats accepted as chat attachments. A `.zip` is stored once in + * uploads/; the agent must extract it (materialize_file operation "extract") to + * decompress it into workspace files/ before reading its contents. + */ +export const SUPPORTED_ARCHIVE_EXTENSIONS = ['zip'] as const + export const SUPPORTED_IMAGE_EXTENSIONS = [ 'png', 'jpg', @@ -207,10 +214,18 @@ const SUPPORTED_IMAGE_MIME_TYPES = [ 'image/vnd.microsoft.icon', ] +const SUPPORTED_ARCHIVE_MIME_TYPES = [ + 'application/zip', + 'application/x-zip-compressed', + 'application/x-zip', +] + export const CHAT_ACCEPT_ATTRIBUTE = [ ACCEPT_ATTRIBUTE, ...SUPPORTED_IMAGE_MIME_TYPES, ...SUPPORTED_IMAGE_EXTENSIONS.map((ext) => `.${ext}`), + ...SUPPORTED_ARCHIVE_MIME_TYPES, + ...SUPPORTED_ARCHIVE_EXTENSIONS.map((ext) => `.${ext}`), ].join(',') export interface FileValidationError { @@ -226,14 +241,16 @@ export const SUPPORTED_ATTACHMENT_EXTENSIONS = Array.from( ...SUPPORTED_IMAGE_EXTENSIONS, ...SUPPORTED_AUDIO_EXTENSIONS, ...SUPPORTED_VIDEO_EXTENSIONS, + ...SUPPORTED_ARCHIVE_EXTENSIONS, ]) ) as readonly string[] /** * Validate that a file's extension is allowed as a chat/mothership attachment. * - * Permits documents, code, images, audio, and video — anything users would - * reasonably attach to a chat message. Rejects executables and unknown types. + * Permits documents, code, images, audio, video, and zip archives — anything + * users would reasonably attach to a chat message. Rejects executables and + * unknown types. */ export function validateAttachmentFileType(fileName: string): FileValidationError | null { const raw = extractExtension(fileName) @@ -242,7 +259,7 @@ export function validateAttachmentFileType(fileName: string): FileValidationErro if (!SUPPORTED_ATTACHMENT_EXTENSIONS.includes(extension)) { return { code: 'UNSUPPORTED_FILE_TYPE', - message: `Unsupported file type${extension ? `: ${extension}` : ` for "${fileName}"`}. Supported types include documents, code, images, audio, and video.`, + message: `Unsupported file type${extension ? `: ${extension}` : ` for "${fileName}"`}. Supported types include documents, code, images, audio, video, and zip archives.`, supportedTypes: [...SUPPORTED_ATTACHMENT_EXTENSIONS], } } From d0eb9e5446deaf91a56955a868f987b07ad9df97 Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan Date: Thu, 9 Jul 2026 21:00:14 -0700 Subject: [PATCH 2/4] fix(zip-uploads): harden extract path from review findings - Replace the whole-buffer central-directory signature scan with an EOCD-anchored walk (shared with zip-guard) so zips containing STORED nested archives are no longer falsely rejected, and report the accurate cap (new 'central_dir_too_large' reason) instead of 'Maximum is 1000' - Make extraction all-or-nothing: a discarding validation pass inflates every entry against the caps before anything is uploaded, so lying headers or corrupt entries can't leave partial trees (corrupt DEFLATE streams now surface as ArchiveError instead of raw zlib errors) - Single-source ArchiveError messages; callers surface err.message and map only status/reason (removes the three divergent message maps) - Guard save/import against archives (saving stranded the contents), extend the workspace ownership check to all three operations, dedupe fileNames, and refuse re-extracting into a non-empty folder instead of duplicating the tree with ' (1)' copies - Harden the extraction folder name (dot segments, control chars, separators) and compute the destination before extracting - Sniff small '.zip'-named uploads so mislabeled text files stay readable instead of dead-ending between read and extract - Scope zip acceptance to the mothership flow only (attachment list, accept attribute, upload/presigned gates) so execution/workspace/chat surfaces keep rejecting zips up front - Don't count skipped noise entries toward the 1000-file cap; restore per-entry zip-slip forensics via skippedUnsafePaths logging - Teach materialize_file(fileNames: [...]) in the extract guidance to match the declared schema --- .../sim/app/api/files/presigned/route.test.ts | 4 +- apps/sim/app/api/files/presigned/route.ts | 2 +- apps/sim/app/api/files/upload/route.ts | 9 +- apps/sim/app/api/tools/file/manage/route.ts | 33 ++- .../home/components/user-input/user-input.tsx | 4 +- .../tools/handlers/materialize-file.test.ts | 105 +++++++- .../tools/handlers/materialize-file.ts | 145 ++++++++--- .../tools/handlers/upload-file-reader.test.ts | 40 ++- .../tools/handlers/upload-file-reader.ts | 34 ++- apps/sim/lib/file-parsers/zip-guard.ts | 60 ++++- apps/sim/lib/uploads/archive.test.ts | 177 ++++++++----- apps/sim/lib/uploads/archive.ts | 244 ++++++++++-------- apps/sim/lib/uploads/utils/file-utils.ts | 2 +- apps/sim/lib/uploads/utils/validation.ts | 34 ++- 14 files changed, 654 insertions(+), 239 deletions(-) diff --git a/apps/sim/app/api/files/presigned/route.test.ts b/apps/sim/app/api/files/presigned/route.test.ts index 55f37d718ac..c3e756620ca 100644 --- a/apps/sim/app/api/files/presigned/route.test.ts +++ b/apps/sim/app/api/files/presigned/route.test.ts @@ -588,7 +588,9 @@ describe('/api/files/presigned', () => { const response = await POST(request) expect(response.status).toBe(200) - expect(mockValidateAttachmentFileType).toHaveBeenCalledWith('screenshot.png') + expect(mockValidateAttachmentFileType).toHaveBeenCalledWith('screenshot.png', { + allowArchives: true, + }) expect(mockValidateFileType).not.toHaveBeenCalled() }) diff --git a/apps/sim/app/api/files/presigned/route.ts b/apps/sim/app/api/files/presigned/route.ts index 5fd15fbfe5c..0104f78fcc5 100644 --- a/apps/sim/app/api/files/presigned/route.ts +++ b/apps/sim/app/api/files/presigned/route.ts @@ -142,7 +142,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } - const fileValidationError = validateAttachmentFileType(fileName) + const fileValidationError = validateAttachmentFileType(fileName, { allowArchives: true }) if (fileValidationError) { throw new ValidationError(fileValidationError.message) } diff --git a/apps/sim/app/api/files/upload/route.ts b/apps/sim/app/api/files/upload/route.ts index ab934a64960..1c013d19c98 100644 --- a/apps/sim/app/api/files/upload/route.ts +++ b/apps/sim/app/api/files/upload/route.ts @@ -23,7 +23,7 @@ import type { StorageContext } from '@/lib/uploads/config' import { generateKnowledgeBaseFileKey } from '@/lib/uploads/contexts/knowledge-base/knowledge-base-file-manager' import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { MAX_WORKSPACE_FORMDATA_FILE_SIZE } from '@/lib/uploads/shared/types' -import { isImageFileType, resolveFileType } from '@/lib/uploads/utils/file-utils' +import { isArchiveFileName, isImageFileType, resolveFileType } from '@/lib/uploads/utils/file-utils' import { SUPPORTED_ATTACHMENT_EXTENSIONS, SUPPORTED_IMAGE_EXTENSIONS, @@ -34,9 +34,12 @@ import { createErrorResponse, InvalidRequestError } from '@/app/api/files/utils' const ALLOWED_EXTENSIONS = new Set(SUPPORTED_ATTACHMENT_EXTENSIONS) -function validateFileExtension(filename: string): boolean { +function validateFileExtension(filename: string, context: StorageContext): boolean { const extension = filename.split('.').pop()?.toLowerCase() if (!extension) return false + // Archives are only extractable in the mothership copilot flow; every other + // context keeps rejecting them up front instead of failing downstream. + if (context === 'mothership' && isArchiveFileName(filename)) return true return ALLOWED_EXTENSIONS.has(extension) } @@ -150,7 +153,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { for (const file of files) { const originalName = file.name || 'untitled.md' - if (!validateFileExtension(originalName)) { + if (!validateFileExtension(originalName, context)) { const extension = originalName.split('.').pop()?.toLowerCase() || 'unknown' throw new InvalidRequestError( `File type '${extension}' is not allowed. Allowed types: ${Array.from(ALLOWED_EXTENSIONS).join(', ')}` diff --git a/apps/sim/app/api/tools/file/manage/route.ts b/apps/sim/app/api/tools/file/manage/route.ts index 79149fbe133..6b3632c19f7 100644 --- a/apps/sim/app/api/tools/file/manage/route.ts +++ b/apps/sim/app/api/tools/file/manage/route.ts @@ -22,11 +22,9 @@ import { } from '@/lib/public-shares/share-manager' import { ArchiveError, + type DecompressResult, decompressArchiveBufferToWorkspaceFiles, MAX_ARCHIVE_BYTES, - MAX_ARCHIVE_ENTRIES, - MAX_ARCHIVE_ENTRY_BYTES, - MAX_ARCHIVE_TOTAL_BYTES, } from '@/lib/uploads/archive' import { ensureWorkspaceFileFolderPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { @@ -810,7 +808,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { maxBytes: MAX_ARCHIVE_BYTES, }) - let result: Awaited> + let result: DecompressResult try { result = await decompressArchiveBufferToWorkspaceFiles(archiveBuffer, { workspaceId, @@ -818,20 +816,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => { }) } catch (archiveError) { if (archiveError instanceof ArchiveError) { + // The error message is single-sourced in ArchiveError (caps included); + // only the HTTP status is mapped here. const status = archiveError.reason === 'invalid' ? 400 : 413 - const message = - archiveError.reason === 'invalid' - ? `"${archive.name}" is not a valid .zip archive` - : archiveError.reason === 'too_many_entries' - ? `Archive has too many entries to extract. Maximum is ${MAX_ARCHIVE_ENTRIES}.` - : archiveError.reason === 'entry_too_large' - ? `Archive entry "${archiveError.entryName}" is too large to extract. Maximum is ${ - MAX_ARCHIVE_ENTRY_BYTES / (1024 * 1024) - } MB per file.` - : `Archive expands to more than the ${ - MAX_ARCHIVE_TOTAL_BYTES / (1024 * 1024) - } MB extraction limit.` - return NextResponse.json({ success: false, error: message }, { status }) + return NextResponse.json( + { success: false, error: `"${archive.name}": ${archiveError.message}` }, + { status } + ) } throw archiveError } @@ -848,6 +839,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => { url: ensureAbsoluteUrl(file.url), })) + if (result.skippedUnsafePaths.length > 0) { + logger.warn('Skipped unsafe archive entries', { + fileId: archive.id, + name: archive.name, + entryNames: result.skippedUnsafePaths, + }) + } + logger.info('Archive decompressed', { fileId: archive.id, name: archive.name, diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx index 188ad323b3c..55d73124605 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx @@ -15,7 +15,7 @@ import { createLogger } from '@sim/logger' import { useParams } from 'next/navigation' import { getMothershipAttachmentPreviewUrl } from '@/lib/copilot/chat/attachment-preview' import { SIM_RESOURCE_DRAG_TYPE, SIM_RESOURCES_DRAG_TYPE } from '@/lib/copilot/resource-types' -import { CHAT_ACCEPT_ATTRIBUTE } from '@/lib/uploads/utils/validation' +import { MOTHERSHIP_ACCEPT_ATTRIBUTE } from '@/lib/uploads/utils/validation' import { useChatSurface } from '@/app/workspace/[workspaceId]/home/components/chat-surface-context' import { AnimatedPlaceholderEffect, @@ -597,7 +597,7 @@ const UserInputImpl = forwardRef(function UserI type='file' onChange={handleFileChange} className='hidden' - accept={CHAT_ACCEPT_ATTRIBUTE} + accept={MOTHERSHIP_ACCEPT_ATTRIBUTE} multiple /> diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts index 87ff107a6f3..075961a304f 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts @@ -8,6 +8,7 @@ const { mockCheckStorageQuotaForBillingContext, mockDecompress, mockFetchBuffer, + mockFindFolder, mockFindUpload, mockHasCloudStorage, mockHeadObject, @@ -18,6 +19,7 @@ const { mockCheckStorageQuotaForBillingContext: vi.fn(), mockDecompress: vi.fn(), mockFetchBuffer: vi.fn(), + mockFindFolder: vi.fn(), mockFindUpload: vi.fn(), mockHasCloudStorage: vi.fn(), mockHeadObject: vi.fn(), @@ -40,6 +42,10 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ fetchWorkspaceFileBuffer: mockFetchBuffer, })) +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-folder-manager', () => ({ + findWorkspaceFileFolderIdByPath: mockFindFolder, +})) + vi.mock('@/lib/uploads/archive', () => ({ decompressArchiveBufferToWorkspaceFiles: mockDecompress, ArchiveError: class ArchiveError extends Error { @@ -53,9 +59,6 @@ vi.mock('@/lib/uploads/archive', () => ({ } }, MAX_ARCHIVE_BYTES: 100 * 1024 * 1024, - MAX_ARCHIVE_ENTRIES: 1000, - MAX_ARCHIVE_ENTRY_BYTES: 100 * 1024 * 1024, - MAX_ARCHIVE_TOTAL_BYTES: 200 * 1024 * 1024, })) vi.mock('@/lib/uploads/core/storage-service', () => ({ @@ -72,6 +75,8 @@ vi.mock('@/lib/billing/storage', () => ({ vi.mock('@/lib/copilot/vfs/path-utils', () => ({ canonicalWorkspaceFilePath: vi.fn(() => 'files/report.txt'), + encodeVfsPathSegments: (segments: string[]) => + segments.map((s) => encodeURIComponent(s)).join('/'), })) vi.mock('@/lib/workflows/operations/import-export', () => ({ parseWorkflowJson: vi.fn() })) @@ -317,7 +322,10 @@ describe('executeMaterializeFile - save storage transition', () => { }) describe('executeMaterializeFile - extract operation', () => { - beforeEach(() => vi.clearAllMocks()) + beforeEach(() => { + vi.clearAllMocks() + mockFindFolder.mockResolvedValue(null) + }) function zipRow(overrides: Record = {}) { return { @@ -347,7 +355,7 @@ describe('executeMaterializeFile - extract operation', () => { { id: 'f2', name: 'b.txt', url: '/y', size: 2, type: 'text/plain', key: 'k2' }, ], skipped: 0, - rootFolderPath: 'files/bundle', + skippedUnsafePaths: [], }) const result = await executeMaterializeFile( @@ -386,4 +394,91 @@ describe('executeMaterializeFile - extract operation', () => { expect(output.failed[0].error).toContain('does not belong to this workspace') expect(mockDecompress).not.toHaveBeenCalled() }) + + it('reports an already-extracted archive instead of duplicating the tree', async () => { + mockFindUpload.mockResolvedValue(zipRow()) + mockFindFolder.mockResolvedValue('folder-existing') + dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'f-old' }]) + + const result = await executeMaterializeFile( + { fileNames: ['bundle.zip'], operation: 'extract' }, + context + ) + + expect(result.success).toBe(false) + const output = result.output as { failed: Array<{ fileName: string; error: string }> } + expect(output.failed[0].error).toContain('already extracted') + expect(mockDecompress).not.toHaveBeenCalled() + }) + + it('dedupes repeated fileNames so one call cannot double-extract', async () => { + mockFindUpload.mockResolvedValue(zipRow()) + mockFetchBuffer.mockResolvedValue(Buffer.from('zip-bytes')) + mockDecompress.mockResolvedValue({ + extracted: [{ id: 'f1', name: 'a.txt', url: '/x', size: 1, type: 'text/plain', key: 'k1' }], + skipped: 0, + skippedUnsafePaths: [], + }) + + const result = await executeMaterializeFile( + { fileNames: ['bundle.zip', 'bundle.zip'], operation: 'extract' }, + context + ) + + expect(result.success).toBe(true) + expect(mockDecompress).toHaveBeenCalledTimes(1) + }) + + it('folds degenerate archive names into the "archive" fallback folder', async () => { + mockFindUpload.mockResolvedValue(zipRow({ displayName: '..zip', originalName: '..zip' })) + mockFetchBuffer.mockResolvedValue(Buffer.from('zip-bytes')) + mockDecompress.mockResolvedValue({ + extracted: [{ id: 'f1', name: 'a.txt', url: '/x', size: 1, type: 'text/plain', key: 'k1' }], + skipped: 0, + skippedUnsafePaths: [], + }) + + const result = await executeMaterializeFile( + { fileNames: ['..zip'], operation: 'extract' }, + context + ) + + expect(result.success).toBe(true) + expect(mockDecompress).toHaveBeenCalledWith( + expect.any(Buffer), + expect.objectContaining({ rootFolderSegments: ['archive'] }) + ) + }) +}) + +describe('executeMaterializeFile - save operation on archives', () => { + beforeEach(() => { + vi.clearAllMocks() + mockFindFolder.mockResolvedValue(null) + }) + + it('refuses to save a .zip upload and points at extract instead', async () => { + mockFindUpload.mockResolvedValue({ + id: 'wf_zip', + key: 'mothership/abc/bundle.zip', + userId: 'user-1', + workspaceId: 'ws-1', + context: 'mothership', + chatId: 'chat-1', + originalName: 'bundle.zip', + displayName: 'bundle.zip', + contentType: 'application/zip', + size: 2048, + deletedAt: null, + uploadedAt: new Date(), + updatedAt: new Date(), + }) + + const result = await executeMaterializeFile({ fileNames: ['bundle.zip'] }, context) + + expect(result.success).toBe(false) + const output = result.output as { failed: Array<{ fileName: string; error: string }> } + expect(output.failed[0].error).toContain('operation: "extract"') + expect(mockDecompress).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts index 098231a7d58..b26e87580c1 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts @@ -13,16 +13,15 @@ import { } from '@/lib/billing/storage' import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types' import { findMothershipUploadRowByChatAndName } from '@/lib/copilot/tools/handlers/upload-file-reader' -import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils' +import { canonicalWorkspaceFilePath, encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils' import { getServePathPrefix } from '@/lib/uploads' import { ArchiveError, + type DecompressResult, decompressArchiveBufferToWorkspaceFiles, MAX_ARCHIVE_BYTES, - MAX_ARCHIVE_ENTRIES, - MAX_ARCHIVE_ENTRY_BYTES, - MAX_ARCHIVE_TOTAL_BYTES, } from '@/lib/uploads/archive' +import { findWorkspaceFileFolderIdByPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { fetchWorkspaceFileBuffer } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { hasCloudStorage, headObject } from '@/lib/uploads/core/storage-service' import { isArchiveFileName } from '@/lib/uploads/utils/file-utils' @@ -51,6 +50,20 @@ function toFileRecord(row: typeof workspaceFiles.$inferSelect) { } } +/** + * Cross-workspace ownership guard shared by every operation. The resolver is + * chat-scoped and current write paths always stamp matching workspaceIds, so + * this is defense in depth — but it must hold uniformly: without it, save would + * flip a foreign-workspace row into this workspace and import would read its + * bytes, the exact leak extract blocks. + */ +function uploadBelongsToWorkspace( + row: { workspaceId: string | null }, + workspaceId: string +): boolean { + return row.workspaceId === workspaceId +} + async function executeSave( fileName: string, chatId: string, @@ -63,10 +76,18 @@ async function executeSave( error: `Upload not found: "${fileName}". Use glob("uploads/*") to list available uploads.`, } } - if (row.workspaceId !== workspaceId) { + if (!uploadBelongsToWorkspace(row, workspaceId)) { return { success: false, error: `Upload not found: "${fileName}".` } } + const displayName = row.displayName ?? row.originalName + if (isArchiveFileName(displayName)) { + return { + success: false, + error: `"${fileName}" is a .zip archive — save it by extracting instead: materialize_file(fileNames: ["${fileName}"], operation: "extract") unpacks it into files/ where the contents stay readable. The raw .zip remains in uploads/ for this chat.`, + } + } + const head = await headObject(row.key, 'mothership') if (!head && hasCloudStorage()) { return { success: false, error: `Upload object not found: "${fileName}".` } @@ -167,6 +188,18 @@ async function executeImport( error: `Upload not found: "${fileName}". Use glob("uploads/*") to list available uploads.`, } } + if (!uploadBelongsToWorkspace(row, workspaceId)) { + return { + success: false, + error: `Upload "${fileName}" does not belong to this workspace.`, + } + } + if (isArchiveFileName(row.displayName ?? row.originalName)) { + return { + success: false, + error: `"${fileName}" is a .zip archive, not a workflow JSON. Extract it first: materialize_file(fileNames: ["${fileName}"], operation: "extract").`, + } + } const buffer = await fetchWorkspaceFileBuffer(toFileRecord(row)) const content = buffer.toString('utf-8') @@ -263,22 +296,23 @@ async function executeImport( } } -/** Map an {@link ArchiveError} reason to a clear, user-facing extract message. */ -function archiveErrorMessage(err: ArchiveError, fileName: string): string { - switch (err.reason) { - case 'invalid': - return `"${fileName}" is not a valid .zip archive.` - case 'too_many_entries': - return `"${fileName}" has too many entries to extract. Maximum is ${MAX_ARCHIVE_ENTRIES}.` - case 'entry_too_large': - return `Archive entry "${err.entryName}" is too large to extract. Maximum is ${ - MAX_ARCHIVE_ENTRY_BYTES / (1024 * 1024) - } MB per file.` - case 'total_too_large': - return `"${fileName}" expands to more than the ${ - MAX_ARCHIVE_TOTAL_BYTES / (1024 * 1024) - } MB extraction limit.` - } +/** + * Fold a zip display name into a safe extraction folder name. Mirrors the VFS + * segment normalization (NFC, control-char strip) and rejects the degenerate + * names the folder layer throws plain Errors for (dot segments, separators, + * empty), so a hostile upload name like `..zip` or `\x01.zip` lands in the + * `archive` fallback instead of surfacing a raw internal error — and so the + * VFS-encoded destination path can be computed before anything is extracted. + */ +function archiveFolderBaseName(displayName: string): string { + const stripped = displayName + .replace(/\.zip$/i, '') + .normalize('NFC') + // biome-ignore lint/suspicious/noControlCharactersInRegex: intentionally stripping control chars + .replace(/[\x00-\x1f\x7f]/g, '') + .replace(/[/\\]/g, '-') + .trim() + return !stripped || stripped === '.' || stripped === '..' ? 'archive' : stripped } /** @@ -302,9 +336,7 @@ async function executeExtract( } } - // Defense in depth: the resolver is chat-scoped, but never extract an upload - // that belongs to a different workspace than the one driving this request. - if (row.workspaceId !== workspaceId) { + if (!uploadBelongsToWorkspace(row, workspaceId)) { return { success: false, error: `Upload "${fileName}" does not belong to this workspace.`, @@ -329,9 +361,36 @@ async function executeExtract( } } - const baseName = displayName.replace(/\.zip$/i, '').trim() || 'archive' + // Resolve the destination up front (the encoded path is a pure function of the + // hardened base name), so nothing can throw after files have been written. + const baseName = archiveFolderBaseName(displayName) + const folderPath = `files/${encodeVfsPathSegments([baseName])}` + + // Re-running extract must not silently duplicate the tree with " (1)"-suffixed + // copies: when the destination folder already holds files, report it as + // already extracted instead of extracting beside the previous run. + const existingFolderId = await findWorkspaceFileFolderIdByPath(workspaceId, [baseName]) + if (existingFolderId) { + const [existingFile] = await db + .select({ id: workspaceFiles.id }) + .from(workspaceFiles) + .where( + and( + eq(workspaceFiles.folderId, existingFolderId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) + ) + .limit(1) + if (existingFile) { + return { + success: false, + error: `"${fileName}" appears to be already extracted — ${folderPath}/ exists and contains files. List them with glob("${folderPath}/**"). To re-extract, delete that folder first.`, + } + } + } - let result: Awaited> + let result: DecompressResult try { const buffer = await fetchWorkspaceFileBuffer(record, { maxBytes: MAX_ARCHIVE_BYTES }) result = await decompressArchiveBufferToWorkspaceFiles(buffer, { @@ -344,7 +403,17 @@ async function executeExtract( }) } catch (err) { if (err instanceof ArchiveError) { - return { success: false, error: archiveErrorMessage(err, fileName) } + // Reads sniff small uploads' magic bytes, so a mislabeled ".zip" that + // fails to parse here is genuinely readable via read() — say so instead + // of bouncing the model between extract and read forever. + const mislabeledHint = + err.reason === 'invalid' + ? ` If the file is not actually a zip archive, read it directly with read("uploads/${fileName}").` + : '' + return { + success: false, + error: `Cannot extract "${fileName}": ${err.message}${mislabeledHint}`, + } } throw err } @@ -353,11 +422,16 @@ async function executeExtract( return { success: false, error: `No files could be extracted from "${fileName}".` } } - // Use the canonical VFS path the extractor actually wrote to, so the glob/read - // hint resolves rather than echoing a hand-encoded (possibly mismatched) name. - const folderPath = result.rootFolderPath const count = result.extracted.length + if (result.skippedUnsafePaths.length > 0) { + logger.warn('Skipped unsafe archive entries during extract', { + fileName, + chatId, + entryNames: result.skippedUnsafePaths, + }) + } + logger.info('Extracted archive into workspace files', { fileName, chatId, @@ -381,9 +455,14 @@ export async function executeMaterializeFile( params: Record, context: ExecutionContext ): Promise { - const fileNames: string[] = - (params.fileNames as string[] | undefined) ?? - ([params.fileName as string | undefined].filter(Boolean) as string[]) + // Dedupe: a repeated name in one call would re-run the operation against the + // same upload (for extract, duplicating the unpacked tree with " (1)" copies). + const fileNames: string[] = Array.from( + new Set( + (params.fileNames as string[] | undefined) ?? + ([params.fileName as string | undefined].filter(Boolean) as string[]) + ) + ) if (fileNames.length === 0) { return { success: false, error: "Missing required parameter 'fileNames'" } diff --git a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts index c5cc86548f5..fc2521f9dc6 100644 --- a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts @@ -7,14 +7,22 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('@sim/db', () => dbChainMock) -const { mockReadFileRecord } = vi.hoisted(() => ({ +const { mockReadFileRecord, mockFetchBuffer } = vi.hoisted(() => ({ mockReadFileRecord: vi.fn(), + mockFetchBuffer: vi.fn(), })) vi.mock('@/lib/copilot/vfs/file-reader', () => ({ readFileRecord: mockReadFileRecord, })) +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + fetchWorkspaceFileBuffer: mockFetchBuffer, +})) + +/** A buffer beginning with the ZIP local-file-header magic (PK\x03\x04). */ +const ZIP_SHAPED = Buffer.from([0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x00, 0x00]) + import { WorkspaceFileGrepError } from '@/lib/copilot/vfs/operations' import { findMothershipUploadRowByChatAndName, @@ -172,6 +180,7 @@ describe('readChatUpload', () => { it('returns extract-first guidance for a .zip upload instead of reading bytes', async () => { const row = makeRow({ id: 'wf_z', displayName: 'bundle.zip', contentType: 'application/zip' }) mockOrderByThenLimit([row]) + mockFetchBuffer.mockResolvedValueOnce(ZIP_SHAPED) const result = await readChatUpload('bundle.zip', CHAT_ID) @@ -180,6 +189,34 @@ describe('readChatUpload', () => { expect(mockReadFileRecord).not.toHaveBeenCalled() }) + it('returns extract-first guidance for a large .zip without downloading it', async () => { + const row = makeRow({ + id: 'wf_z', + displayName: 'huge.zip', + contentType: 'application/zip', + size: 50 * 1024 * 1024, + }) + mockOrderByThenLimit([row]) + + const result = await readChatUpload('huge.zip', CHAT_ID) + + expect(result?.content).toContain('materialize_file') + expect(mockFetchBuffer).not.toHaveBeenCalled() + expect(mockReadFileRecord).not.toHaveBeenCalled() + }) + + it('reads a small mislabeled ".zip" (non-zip bytes) normally instead of dead-ending it', async () => { + const row = makeRow({ id: 'wf_m', displayName: 'data.zip', contentType: 'text/csv' }) + mockOrderByThenLimit([row]) + mockFetchBuffer.mockResolvedValueOnce(Buffer.from('a,b,c\n1,2,3\n')) + mockReadFileRecord.mockResolvedValueOnce({ content: 'a,b,c\n1,2,3', totalLines: 2 }) + + const result = await readChatUpload('data.zip', CHAT_ID) + + expect(result).toEqual({ content: 'a,b,c\n1,2,3', totalLines: 2 }) + expect(mockReadFileRecord).toHaveBeenCalledTimes(1) + }) + it('returns null when no row matches', async () => { mockOrderByThenLimit([]) dbChainMockFns.orderBy.mockResolvedValueOnce([] as never) @@ -201,6 +238,7 @@ describe('grepChatUpload', () => { it('throws WorkspaceFileGrepError with extract-first guidance for a .zip upload', async () => { const row = makeRow({ id: 'wf_z', displayName: 'bundle.zip', contentType: 'application/zip' }) mockOrderByThenLimit([row]) + mockFetchBuffer.mockResolvedValueOnce(ZIP_SHAPED) const error = await grepChatUpload('bundle.zip', CHAT_ID, 'foo').catch((e) => e) diff --git a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts index ea3de99a491..25d5eed78c3 100644 --- a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts +++ b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts @@ -12,12 +12,40 @@ import { WorkspaceFileGrepError, } from '@/lib/copilot/vfs/operations' import { decodeVfsSegment, encodeVfsSegment } from '@/lib/copilot/vfs/path-utils' +import { isZipShaped } from '@/lib/file-parsers/zip-guard' import { getServePathPrefix } from '@/lib/uploads' -import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import { + fetchWorkspaceFileBuffer, + type WorkspaceFileRecord, +} from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { buildArchiveExtractGuidance, isArchiveFileName } from '@/lib/uploads/utils/file-utils' const logger = createLogger('UploadFileReader') +/** + * Sniff budget for uploads whose NAME says archive: below this size the actual + * bytes decide (a mislabeled text file named `data.zip` stays readable instead + * of being trapped between read-says-extract and extract-says-invalid); above + * it the extension is trusted so a real 100MB zip is never downloaded just to + * refuse it. + */ +const ARCHIVE_SNIFF_MAX_BYTES = 1024 * 1024 + +/** + * True when the upload should get extract-first guidance: named like an archive + * and — for small files — actually shaped like one. + */ +async function isActualArchiveUpload(record: WorkspaceFileRecord): Promise { + if (!isArchiveFileName(record.name)) return false + if (record.size > ARCHIVE_SNIFF_MAX_BYTES) return true + try { + const buffer = await fetchWorkspaceFileBuffer(record, { maxBytes: ARCHIVE_SNIFF_MAX_BYTES }) + return isZipShaped(buffer) + } catch { + return true + } +} + /** * Canonical comparison key for an upload's VFS name. Accepts both the raw display * name and a percent-encoded segment (decode first — a no-op for raw names — @@ -154,7 +182,7 @@ export async function readChatUpload( const row = await findMothershipUploadRowByChatAndName(chatId, filename) if (!row) return null const record = toWorkspaceFileRecord(row) - if (isArchiveFileName(record.name)) { + if (await isActualArchiveUpload(record)) { return { content: `[${buildArchiveExtractGuidance(record.name)}]`, totalLines: 1 } } return readFileRecord(record) @@ -189,7 +217,7 @@ export async function grepChatUpload( ) } const record = toWorkspaceFileRecord(row) - if (isArchiveFileName(record.name)) { + if (await isActualArchiveUpload(record)) { throw new WorkspaceFileGrepError(buildArchiveExtractGuidance(record.name)) } const result = await readFileRecord(record) diff --git a/apps/sim/lib/file-parsers/zip-guard.ts b/apps/sim/lib/file-parsers/zip-guard.ts index 99aca4f7542..07642b3740c 100644 --- a/apps/sim/lib/file-parsers/zip-guard.ts +++ b/apps/sim/lib/file-parsers/zip-guard.ts @@ -66,7 +66,7 @@ export class ZipBombError extends Error { * closed: a ZIP-shaped buffer the guard cannot parse must be rejected rather * than handed to a decompression library. */ -function isZipShaped(buffer: Buffer): boolean { +export function isZipShaped(buffer: Buffer): boolean { if (buffer.length < 4) { return false } @@ -217,6 +217,64 @@ function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): n return total } +/** Parse-time shape of a ZIP central directory, read without decompressing anything. */ +export interface ZipCentralDirectoryStats { + /** Records in the contiguous central-directory run — what a per-signature parser allocates. */ + entryCount: number + /** Summed declared extra-field bytes across those records. */ + totalExtraFieldBytes: number +} + +/** + * Walk the real central directory (EOCD-anchored, decoy-resistant, ZIP64-aware — + * the same anchoring as {@link assertOoxmlArchiveWithinLimits}) and report its + * record count and summed extra-field bytes, so callers can bound a parser's + * object graph before handing it the buffer. The walk covers the CONTIGUOUS run + * of records at the central-directory offset rather than trusting the EOCD's + * declared count, because that run is what JSZip actually allocates one entry + * per — a lied count can neither hide records nor inflate the tally. Unlike a + * raw whole-buffer signature scan, STORED entry payloads (e.g. a nested `.zip` + * archived without recompression) are never miscounted as records. Returns + * `null` when the buffer is not a parseable ZIP, so callers can fail closed. + */ +export function readZipCentralDirectoryStats(buffer: Buffer): ZipCentralDirectoryStats | null { + if (buffer.length < EOCD_MIN_SIZE) { + return null + } + + const eocdOffset = findEocdOffset(buffer) + if (eocdOffset < 0) { + return null + } + + const location = locateCentralDirectory(buffer, eocdOffset) + if (!location) { + return null + } + + let entryCount = 0 + let totalExtraFieldBytes = 0 + let cursor = location.offset + while ( + cursor + CENTRAL_DIRECTORY_HEADER_MIN_SIZE <= buffer.length && + buffer.readUInt32LE(cursor) === CENTRAL_DIRECTORY_HEADER_SIGNATURE + ) { + const fileNameLength = buffer.readUInt16LE(cursor + 28) + const extraFieldLength = buffer.readUInt16LE(cursor + 30) + const commentLength = buffer.readUInt16LE(cursor + 32) + + entryCount += 1 + totalExtraFieldBytes += extraFieldLength + cursor += CENTRAL_DIRECTORY_HEADER_MIN_SIZE + fileNameLength + extraFieldLength + commentLength + } + + if (entryCount < location.entryCount) { + return null + } + + return { entryCount, totalExtraFieldBytes } +} + /** * Reject an OOXML archive whose declared expanded size or compression ratio * exceeds safe bounds, before any decompression library materializes it. diff --git a/apps/sim/lib/uploads/archive.test.ts b/apps/sim/lib/uploads/archive.test.ts index 345059f204e..1be145263bf 100644 --- a/apps/sim/lib/uploads/archive.test.ts +++ b/apps/sim/lib/uploads/archive.test.ts @@ -24,7 +24,7 @@ import { } from '@/lib/uploads/archive' async function buildZip( - files: Record, + files: Record, opts?: { symlinks?: string[] } ): Promise { const zip = new JSZip() @@ -37,6 +37,36 @@ async function buildZip( return Buffer.from(await zip.generateAsync({ type: 'uint8array', platform: 'UNIX' })) } +const CD_HEADER_SIZE = 46 +const EOCD_SIZE = 22 + +/** + * Hand-craft a structurally valid ZIP central directory + EOCD: `records` + * zero-name CD headers each declaring `extraPerRecord` extra-field bytes + * (with the bytes actually present, so the walk advances correctly), and an + * EOCD at the tail pointing at offset 0. There are no local file entries — + * the pre-parse guard must reject before JSZip ever needs them. + */ +function craftCentralDirectory(records: number, extraPerRecord: number): Buffer { + const recordSize = CD_HEADER_SIZE + extraPerRecord + const buffer = Buffer.alloc(records * recordSize + EOCD_SIZE) + for (let r = 0; r < records; r++) { + const base = r * recordSize + buffer.writeUInt32LE(0x02014b50, base) // central-directory file header signature + buffer.writeUInt16LE(0, base + 28) // file name length + buffer.writeUInt16LE(extraPerRecord, base + 30) // extra field length + buffer.writeUInt16LE(0, base + 32) // comment length + } + const eocd = records * recordSize + buffer.writeUInt32LE(0x06054b50, eocd) // EOCD signature + buffer.writeUInt16LE(Math.min(records, 0xffff), eocd + 8) // entries on this disk + buffer.writeUInt16LE(Math.min(records, 0xffff), eocd + 10) // total entries + buffer.writeUInt32LE(records * recordSize, eocd + 12) // central directory size + buffer.writeUInt32LE(0, eocd + 16) // central directory offset + buffer.writeUInt16LE(0, eocd + 20) // comment length + return buffer +} + beforeEach(() => { vi.clearAllMocks() mockEnsureFolder.mockResolvedValue('folder_1') @@ -61,8 +91,7 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { }) expect(result.extracted).toHaveLength(2) - // The canonical, per-segment-encoded VFS path the files were written under. - expect(result.rootFolderPath).toBe('files/bundle') + expect(result.skippedUnsafePaths).toEqual([]) expect(mockUpload).toHaveBeenCalledTimes(2) const leafNames = mockUpload.mock.calls.map((c) => c[3]).sort() expect(leafNames).toEqual(['report.txt', 'sheet.csv']) @@ -75,78 +104,108 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { ) }) - it('returns the encoded root folder path for names that need encoding', async () => { - const buffer = await buildZip({ 'a.txt': 'x' }) - - const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { - workspaceId: 'ws', - userId: 'u', - rootFolderSegments: ['My Archive'], - }) - - expect(result.rootFolderPath).toBe('files/My%20Archive') - }) - it('rejects an archive with more central-directory records than the cap, before parsing', async () => { - // One 32-byte central-directory header per record, laid out non-contiguously - // (a full fixed-size header apart) so the signature scan strides one header - // at a time and reads each record's real extra-field-length field — here a - // hard 0 — instead of misreading a neighboring signature's bytes. That keeps - // summed extra bytes at exactly 0, so this archive trips the RECORD cap and - // never the extra-bytes cap, isolating record-count regression coverage. - // JSZip would build one entry per signature it finds (ignoring the EOCD - // count), so the pre-scan must reject this before loadAsync ever runs. - const BLOCK_SIZE = 32 - const records = MAX_ARCHIVE_CENTRAL_DIR_RECORDS + 1 - const buffer = Buffer.alloc(records * BLOCK_SIZE) - for (let r = 0; r < records; r++) { - const base = r * BLOCK_SIZE - // bytes 0-3: PK\x01\x02 central-directory file header signature - buffer[base] = 0x50 - buffer[base + 1] = 0x4b - buffer[base + 2] = 0x01 - buffer[base + 3] = 0x02 - // bytes 30-31: extra field length = 0 (little-endian) → adds zero toward - // MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES, so only the record cap can trip. - buffer.writeUInt16LE(0, base + 30) - } + // A structurally valid central directory (EOCD-anchored) with one record more + // than the parse-graph cap. JSZip would build one entry per record in the + // contiguous run, so the pre-parse guard must reject before loadAsync runs — + // and with the accurate central-directory message, not the file-count one. + const buffer = craftCentralDirectory(MAX_ARCHIVE_CENTRAL_DIR_RECORDS + 1, 0) await expect( decompressArchiveBufferToWorkspaceFiles(buffer, { workspaceId: 'ws', userId: 'u' }) - ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'too_many_entries' }) + ).rejects.toMatchObject({ + name: 'ArchiveError', + reason: 'central_dir_too_large', + message: expect.stringContaining(String(MAX_ARCHIVE_CENTRAL_DIR_RECORDS)), + }) expect(mockUpload).not.toHaveBeenCalled() }) it('rejects an archive whose central-directory extra fields exceed the byte cap, before parsing', async () => { - // A handful of central-directory headers — far below the record cap — each - // declaring the maximum 0xFFFF extra-field length, so their summed extra - // bytes cross MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES. JSZip would allocate one - // retained object per declared extra field during loadAsync, so the pre-scan - // must reject on summed extra bytes, not just on record count. - const BLOCK_SIZE = 32 + // A handful of records — far below the record cap — each declaring (and + // carrying) the maximum 0xFFFF extra-field bytes, so their sum crosses + // MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES. JSZip retains one object per declared + // extra field during loadAsync, so the guard must reject on summed extra + // bytes, not just on record count. const EXTRA_PER_RECORD = 0xffff - // +5 records of headroom past the cap; still << MAX_ARCHIVE_CENTRAL_DIR_RECORDS - // so THIS cap (extra bytes), not the record count, is what triggers rejection. const records = Math.ceil(MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES / EXTRA_PER_RECORD) + 5 expect(records).toBeLessThan(MAX_ARCHIVE_CENTRAL_DIR_RECORDS) - const buffer = Buffer.alloc(records * BLOCK_SIZE) - for (let r = 0; r < records; r++) { - const base = r * BLOCK_SIZE - // bytes 0-3: PK\x01\x02 central-directory file header signature - buffer[base] = 0x50 - buffer[base + 1] = 0x4b - buffer[base + 2] = 0x01 - buffer[base + 3] = 0x02 - // bytes 30-31: extra field length = 0xFFFF (little-endian), the CD max - buffer.writeUInt16LE(EXTRA_PER_RECORD, base + 30) + const buffer = craftCentralDirectory(records, EXTRA_PER_RECORD) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { workspaceId: 'ws', userId: 'u' }) + ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'central_dir_too_large' }) + expect(mockUpload).not.toHaveBeenCalled() + }) + + it('extracts a zip whose STORED entry contains foreign central-directory signatures', async () => { + // Regression: a whole-buffer signature scan would count the PK\x01\x02 + // signatures inside this stored payload (a nested archive's central + // directory travels verbatim inside STORED entries) and falsely reject the + // archive. The EOCD-anchored walk must ignore entry payloads entirely. + const signatures = Buffer.alloc((MAX_ARCHIVE_CENTRAL_DIR_RECORDS + 1) * 4) + for (let r = 0; r <= MAX_ARCHIVE_CENTRAL_DIR_RECORDS; r++) { + signatures.writeUInt32LE(0x02014b50, r * 4) } + const zip = new JSZip() + zip.file('inner.zip', signatures, { compression: 'STORE' }) + const buffer = Buffer.from(await zip.generateAsync({ type: 'uint8array' })) + + const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + userId: 'u', + }) + + expect(result.extracted).toHaveLength(1) + expect(mockUpload).toHaveBeenCalledTimes(1) + }) + + it('uploads nothing when an entry is corrupted mid-archive (all-or-nothing)', async () => { + // Corrupt one entry's DEFLATE bytes AFTER the central directory is built, so + // loadAsync parses fine and only streaming inflation fails. The validation + // pass must catch it before ANY entry is uploaded, and the raw zlib error + // must surface as the module's ArchiveError, not leak through. + const zip = new JSZip() + zip.file('fine.txt', 'this entry is intact') + // Incompressible payload so the DEFLATE stream is large enough to stomp + // without touching the following records. + zip.file('bad.bin', Buffer.from(Array.from({ length: 20000 }, (_, i) => (i * 137) % 251))) + const buffer = Buffer.from( + await zip.generateAsync({ type: 'uint8array', compression: 'DEFLATE' }) + ) + const nameOffset = buffer.indexOf(Buffer.from('bad.bin')) + // Local file header: name follows the 30-byte fixed header; data follows the + // name (+ extra field, empty here). Stomp a chunk of the DEFLATE stream. + buffer.fill(0xff, nameOffset + 'bad.bin'.length, nameOffset + 'bad.bin'.length + 256) await expect( decompressArchiveBufferToWorkspaceFiles(buffer, { workspaceId: 'ws', userId: 'u' }) - ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'too_many_entries' }) + ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'invalid' }) expect(mockUpload).not.toHaveBeenCalled() }) + it('does not count noise entries toward the extraction cap when they are being skipped', async () => { + // macOS Finder zips carry a __MACOSX/._* shadow per file, doubling the raw + // entry count. 501 files + 501 shadows = 1002 raw entries — over the + // 1000-file cap — but with skipNoiseEntries set only the 501 real files are + // extracted, so the archive must be accepted. + const files: Record = {} + for (let i = 0; i < 501; i++) { + files[`f${i}.txt`] = 'x' + files[`__MACOSX/._f${i}.txt`] = 'shadow' + } + const buffer = await buildZip(files) + + const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + userId: 'u', + skipNoiseEntries: true, + }) + + expect(result.extracted).toHaveLength(501) + expect(result.skipped).toBe(501) + }) + it('throws ArchiveError invalid for a non-zip buffer (no files written)', async () => { await expect( decompressArchiveBufferToWorkspaceFiles(Buffer.from('not a zip at all'), { @@ -173,9 +232,11 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { }) // Only the traversal entry counts toward `skipped`; the symlink is filtered - // out before the skip tally (it never becomes a candidate entry). + // out before the skip tally (it never becomes a candidate entry). The + // traversal entry's raw name is preserved for the callers' forensic logs. expect(result.extracted).toHaveLength(1) expect(result.skipped).toBe(1) + expect(result.skippedUnsafePaths).toEqual(['..\\evil.txt']) expect(mockUpload).toHaveBeenCalledTimes(1) expect(mockUpload.mock.calls[0][3]).toBe('safe.txt') }) diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts index 9cab0a509db..2046c84ac25 100644 --- a/apps/sim/lib/uploads/archive.ts +++ b/apps/sim/lib/uploads/archive.ts @@ -1,7 +1,7 @@ import { Buffer } from 'buffer' import type { Readable } from 'stream' import JSZip from 'jszip' -import { encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils' +import { readZipCentralDirectoryStats } from '@/lib/file-parsers/zip-guard' import { ensureWorkspaceFileFolderPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' @@ -14,13 +14,15 @@ import type { UserFile } from '@/executor/types' * The declared sizes in a ZIP header are attacker-controlled, so the real caps * are always enforced on the inflated byte stream — never on metadata. The * parser's object graph is additionally bounded BEFORE parsing via - * {@link centralDirExceedsCaps}, which counts central-directory signatures - * (JSZip builds one entry per signature it finds, NOT per the EOCD's declared - * count) and sums their declared extra-field bytes (JSZip retains one object per - * central-directory extra field) so a crafted archive — whether packed with - * millions of records or a handful of records stuffed with tiny extra fields — - * cannot balloon the parser's heap. Extraction is sequential (one entry inflated - * and uploaded at a time) so peak memory is ~one entry. + * {@link readZipCentralDirectoryStats} (EOCD-anchored central-directory walk), + * bounding both the record count (JSZip builds one entry per record in the + * contiguous CD run) and the summed declared extra-field bytes (JSZip retains + * one object per CD extra field), so a crafted archive cannot balloon the + * parser's heap. Extraction is all-or-nothing: every entry is first inflated + * through a counting sink (discarded, nothing persisted) to prove the archive + * fits the caps, and only then inflated again and uploaded — so a cap violation + * or lying header can never leave a partial tree behind. Peak memory stays ~one + * entry in both passes. */ /** Input archive download/size cap. */ @@ -35,7 +37,6 @@ export const MAX_ARCHIVE_TOTAL_BYTES = 200 * 1024 * 1024 const S_IFMT = 0o170000 const S_IFLNK = 0o120000 -const CD_SIGNATURE = Buffer.from([0x50, 0x4b, 0x01, 0x02]) // PK\x01\x02 central-directory file header /** Memory bound for the parse-time object graph (distinct from the file-extraction cap). */ export const MAX_ARCHIVE_CENTRAL_DIR_RECORDS = 10_000 /** @@ -48,10 +49,20 @@ export const MAX_ARCHIVE_CENTRAL_DIR_RECORDS = 10_000 */ export const MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES = 4 * 1024 * 1024 -/** Reason a {@link ArchiveError} was raised, for mapping to a caller response. */ -type ArchiveErrorReason = 'invalid' | 'too_many_entries' | 'entry_too_large' | 'total_too_large' +/** Reason a {@link ArchiveError} was raised, for mapping to a caller response/status. */ +export type ArchiveErrorReason = + | 'invalid' + | 'too_many_entries' + | 'central_dir_too_large' + | 'entry_too_large' + | 'total_too_large' -/** Raised for malformed archives and cap violations so callers can surface a clear message. */ +/** + * Raised for malformed archives and cap violations. `message` is the single + * user-facing source of truth (caps included) — callers surface it verbatim and + * use `reason` only for response mapping (e.g. HTTP status), never to rebuild + * the text. + */ export class ArchiveError extends Error { readonly reason: ArchiveErrorReason readonly entryName?: string @@ -64,43 +75,35 @@ export class ArchiveError extends Error { } } +const MB = 1024 * 1024 + /** - * True when the archive's central directory would push JSZip's parse-time object - * graph past a safe bound. A single pass over every `PK\x01\x02` signature bounds - * TWO independent memory vectors: - * - RECORD COUNT: JSZip builds one ZipEntry per CD signature it finds (it does - * NOT trust the EOCD count), so counting signatures bounds the entry graph. - * - EXTRA-FIELD BYTES: `readExtraFields` retains one `{id,length,value}` object - * per extra field, and every extra field is >= 4 bytes, so the summed declared - * extra-field length is an upper bound on those objects. Record count alone - * does NOT bound this — one record may declare up to 65535 extra bytes — so - * the extra-field cap is enforced separately. - * - * This is a conservative UPPER bound on JSZip's real allocation: JSZip only builds - * entries for the contiguous run of signatures starting at the EOCD's - * centralDirOffset, so counting ALL signatures and summing ALL their declared - * extra lengths is always >= what JSZip allocates. That makes it immune to a - * lied/absent EOCD count and to ZIP64 (the CD file-header signature is unchanged - * there). Over-counting only ever rejects, which is safe. + * Bound JSZip's parse-time object graph before handing it the buffer, using the + * real (EOCD-anchored) central directory: record count bounds the entry graph, + * and summed declared extra-field bytes bound the per-field objects. Throws with + * the accurate cap in the message — these caps are parse-graph bounds, distinct + * from {@link MAX_ARCHIVE_ENTRIES} which limits extracted files after parsing. */ -function centralDirExceedsCaps(buffer: Buffer): boolean { - let records = 0 - let extraBytes = 0 - for (let i = buffer.indexOf(CD_SIGNATURE); i !== -1; i = buffer.indexOf(CD_SIGNATURE, i + 4)) { - records += 1 - // The CD header's uint16 LE "extra field length" lives at offset +30; only - // read it when the fixed-size header's extra-length field is fully present. - if (i + 32 <= buffer.length) { - extraBytes += buffer.readUInt16LE(i + 30) - } - if ( - records > MAX_ARCHIVE_CENTRAL_DIR_RECORDS || - extraBytes > MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES - ) { - return true - } +function assertCentralDirWithinCaps(buffer: Buffer): void { + const stats = readZipCentralDirectoryStats(buffer) + if (!stats) { + throw new ArchiveError( + 'invalid', + 'Not a valid .zip archive — its central directory could not be parsed.' + ) + } + if (stats.entryCount > MAX_ARCHIVE_CENTRAL_DIR_RECORDS) { + throw new ArchiveError( + 'central_dir_too_large', + `Archive central directory has ${stats.entryCount} records; the maximum that can be parsed safely is ${MAX_ARCHIVE_CENTRAL_DIR_RECORDS}.` + ) + } + if (stats.totalExtraFieldBytes > MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES) { + throw new ArchiveError( + 'central_dir_too_large', + `Archive central-directory metadata is too large to parse safely (over ${MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES / MB} MB of extra fields).` + ) } - return false } /** @@ -114,18 +117,22 @@ const readEntryUncompressedSize = (entry: JSZip.JSZipObject): number | undefined return typeof size === 'number' && Number.isFinite(size) ? size : undefined } -type InflateResult = { ok: true; buffer: Buffer } | { ok: false; reason: 'entry' | 'total' } +type InflateResult = + | { ok: true; size: number; buffer: Buffer | null } + | { ok: false; reason: 'entry' | 'total' } /** * Inflate a single zip entry through a streaming counting sink, tearing the * stream down the moment cumulative output would exceed the per-entry cap or the * remaining total budget. The declared uncompressed size is NOT trusted: * enforcement happens on the actual inflated bytes as they arrive, so peak memory - * is bounded by the cap plus one DEFLATE chunk. + * is bounded by the cap plus one DEFLATE chunk. With `retain: false` the bytes + * are counted and discarded (the validation pass), so peak memory is ~one chunk. */ const inflateEntryWithinCaps = ( entry: JSZip.JSZipObject, - remainingTotalBudget: number + remainingTotalBudget: number, + retain: boolean ): Promise => new Promise((resolve, reject) => { const chunks: Buffer[] = [] @@ -150,14 +157,25 @@ const inflateEntryWithinCaps = ( settle({ ok: false, reason: 'total' }) return } - chunks.push(chunk) + if (retain) chunks.push(chunk) }) - stream.on('end', () => settle({ ok: true, buffer: Buffer.concat(chunks, size) })) - stream.on('error', (error) => { + stream.on('end', () => + settle({ ok: true, size, buffer: retain ? Buffer.concat(chunks, size) : null }) + ) + stream.on('error', () => { if (settled) return settled = true stream.destroy() - reject(error) + // A stream error here means the entry's compressed data is corrupt or + // truncated (it passed the central-directory parse) — surface it under the + // module's ArchiveError contract, not as a raw zlib error. + reject( + new ArchiveError( + 'invalid', + `Archive entry "${entry.name}" could not be decompressed — the archive may be corrupted or truncated.`, + entry.name + ) + ) }) }) @@ -189,18 +207,33 @@ const isArchiveNoiseEntry = (segments: string[]): boolean => { return leaf === '.DS_Store' || leaf === 'Thumbs.db' } -interface DecompressResult { +export interface DecompressResult { /** Workspace files created under the target folder, in extraction order. */ extracted: UserFile[] /** Count of entries skipped as unsafe (zip-slip) or filesystem noise. */ skipped: number /** - * Canonical, per-segment-encoded VFS path of the root folder the archive was - * extracted into (e.g. `files/My%20Archive`), or `files` for the workspace - * root. Matches what the workspace VFS serves, so the glob/read hint built - * from it resolves to the files that were just written. + * Raw entry paths rejected by the zip-slip sanitizer (traversal or empty), + * so callers can log the attempted names for forensics. Noise entries + * (`__MACOSX/`, `.DS_Store`) are counted in `skipped` but never listed here. */ - rootFolderPath: string + skippedUnsafePaths: string[] +} + +/** Throw the (single-sourced) ArchiveError for a streaming cap violation. */ +function throwInflateCapError(reason: 'entry' | 'total', entryName: string): never { + if (reason === 'entry') { + throw new ArchiveError( + 'entry_too_large', + `Archive entry "${entryName}" is too large to extract. Maximum is ${MAX_ARCHIVE_ENTRY_BYTES / MB} MB per file.`, + entryName + ) + } + throw new ArchiveError( + 'total_too_large', + `Archive expands to more than the ${MAX_ARCHIVE_TOTAL_BYTES / MB} MB extraction limit.`, + entryName + ) } /** @@ -209,10 +242,12 @@ interface DecompressResult { * symlink guards everywhere. Throws {@link ArchiveError} for an invalid archive * or a cap violation; returns `{ extracted: [] }` when every entry was skipped. * - * Memory is bounded: the central-directory record count and summed extra-field - * bytes are gated pre-parse, and entries are inflated and uploaded one at a time, - * so peak working set is ~one entry (≤ the per-entry cap) regardless of how many - * files the archive holds. + * All-or-nothing: pass 1 inflates every entry through a discarding counting sink + * to enforce the per-entry and total caps on REAL inflated bytes (declared sizes + * can lie), and nothing is uploaded until the whole archive has passed — so a + * mid-archive violation never leaves a partial tree in the workspace. Pass 2 + * re-inflates and uploads one entry at a time. Peak memory stays ~one entry in + * both passes; the cost is inflating twice (CPU only, bounded by the caps). * * Filesystem-noise entries (`__MACOSX/`, `.DS_Store`, `Thumbs.db`) are extracted * verbatim unless `skipNoiseEntries` is set — the HTTP decompress route preserves @@ -229,75 +264,78 @@ export async function decompressArchiveBufferToWorkspaceFiles( ): Promise { const { workspaceId, userId, rootFolderSegments = [], skipNoiseEntries = false } = opts - if (centralDirExceedsCaps(buffer)) { - throw new ArchiveError( - 'too_many_entries', - `Archive has too many entries. Maximum is ${MAX_ARCHIVE_ENTRIES}.` - ) - } + assertCentralDirWithinCaps(buffer) let zip: JSZip try { zip = await JSZip.loadAsync(buffer) } catch { - throw new ArchiveError('invalid', 'Not a valid .zip archive') + throw new ArchiveError('invalid', 'Not a valid .zip archive.') } const realEntries = Object.values(zip.files).filter( (entry) => !entry.dir && !isSymlinkEntry(entry) ) - if (realEntries.length > MAX_ARCHIVE_ENTRIES) { - throw new ArchiveError( - 'too_many_entries', - `Archive has too many entries. Maximum is ${MAX_ARCHIVE_ENTRIES}.` - ) - } // Resolve safe entries first so unsafe ones never count toward the size caps. const safeEntries: Array<{ entry: JSZip.JSZipObject; segments: string[] }> = [] + const skippedUnsafePaths: string[] = [] let skipped = 0 for (const entry of realEntries) { const segments = sanitizeArchiveEntryPath(entry.name) - if (!segments || (skipNoiseEntries && isArchiveNoiseEntry(segments))) { + if (!segments) { + skippedUnsafePaths.push(entry.name) + skipped += 1 + continue + } + if (skipNoiseEntries && isArchiveNoiseEntry(segments)) { skipped += 1 continue } safeEntries.push({ entry, segments }) } + // The entry cap applies to what will actually be extracted — a macOS zip whose + // __MACOSX/ shadows are about to be dropped must not be rejected for them. + if (safeEntries.length > MAX_ARCHIVE_ENTRIES) { + throw new ArchiveError( + 'too_many_entries', + `Archive has ${safeEntries.length} files; the maximum is ${MAX_ARCHIVE_ENTRIES}.` + ) + } + // Cheap declared-size fast-reject for honestly-declared archives. let declaredTotal = 0 for (const { entry } of safeEntries) { const declaredSize = readEntryUncompressedSize(entry) if (declaredSize === undefined) continue - if (declaredSize > MAX_ARCHIVE_ENTRY_BYTES) { - throw new ArchiveError( - 'entry_too_large', - `Archive entry "${entry.name}" is too large to extract.`, - entry.name - ) - } + if (declaredSize > MAX_ARCHIVE_ENTRY_BYTES) throwInflateCapError('entry', entry.name) declaredTotal += declaredSize - if (declaredTotal > MAX_ARCHIVE_TOTAL_BYTES) { - throw new ArchiveError('total_too_large', 'Archive expands beyond the extraction limit.') - } + if (declaredTotal > MAX_ARCHIVE_TOTAL_BYTES) throwInflateCapError('total', entry.name) + } + + // Pass 1 — validate: inflate every entry against the caps without retaining or + // persisting anything, so a lying header aborts before any upload happens. + let validatedTotal = 0 + for (const { entry } of safeEntries) { + const result = await inflateEntryWithinCaps( + entry, + MAX_ARCHIVE_TOTAL_BYTES - validatedTotal, + false + ) + if (!result.ok) throwInflateCapError(result.reason, entry.name) + validatedTotal += result.size } + // Pass 2 — extract: the archive is proven within caps; inflate again and upload. const folderIdCache = new Map() const extracted: UserFile[] = [] let totalBytes = 0 for (const { entry, segments } of safeEntries) { - const result = await inflateEntryWithinCaps(entry, MAX_ARCHIVE_TOTAL_BYTES - totalBytes) - if (!result.ok) { - throw new ArchiveError( - result.reason === 'entry' ? 'entry_too_large' : 'total_too_large', - result.reason === 'entry' - ? `Archive entry "${entry.name}" is too large to extract.` - : 'Archive expands beyond the extraction limit.', - entry.name - ) - } - totalBytes += result.buffer.length + const result = await inflateEntryWithinCaps(entry, MAX_ARCHIVE_TOTAL_BYTES - totalBytes, true) + if (!result.ok) throwInflateCapError(result.reason, entry.name) + totalBytes += result.size + const entryBuffer = result.buffer as Buffer const leafName = segments[segments.length - 1] const folderSegments = [...rootFolderSegments, ...segments.slice(0, -1)] @@ -316,7 +354,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( const uploaded = await uploadWorkspaceFile( workspaceId, userId, - result.buffer, + entryBuffer, leafName, mimeType, { @@ -326,11 +364,5 @@ export async function decompressArchiveBufferToWorkspaceFiles( extracted.push(uploaded) } - // Encode the root segments the same way the workspace VFS serves folder names - // (per-segment `encodeVfsSegment`), so the advertised path matches the files - // that were just written rather than the raw, unencoded display name. - const rootFolderPath = - rootFolderSegments.length > 0 ? `files/${encodeVfsPathSegments(rootFolderSegments)}` : 'files' - - return { extracted, skipped, rootFolderPath } + return { extracted, skipped, skippedUnsafePaths } } diff --git a/apps/sim/lib/uploads/utils/file-utils.ts b/apps/sim/lib/uploads/utils/file-utils.ts index 0b415171e3f..7837617a0cc 100644 --- a/apps/sim/lib/uploads/utils/file-utils.ts +++ b/apps/sim/lib/uploads/utils/file-utils.ts @@ -227,7 +227,7 @@ export function isArchiveFileName(filename: string): boolean { * `files/`, so this points at the explicit one-time extract step. */ export function buildArchiveExtractGuidance(name: string): string { - return `"${name}" is a .zip archive — its contents can't be read directly. Extract it once with materialize_file(fileName: "${name}", operation: "extract"), then read the unpacked files under files/ (e.g. glob("files//**") then read("files///content")).` + return `"${name}" is a .zip archive — its contents can't be read directly. Extract it once with materialize_file(fileNames: ["${name}"], operation: "extract"), then read the unpacked files under files/ (e.g. glob("files//**") then read("files///content")).` } const EXTENSION_TO_MIME: Record = { diff --git a/apps/sim/lib/uploads/utils/validation.ts b/apps/sim/lib/uploads/utils/validation.ts index a41fc8cb152..b400fa621a1 100644 --- a/apps/sim/lib/uploads/utils/validation.ts +++ b/apps/sim/lib/uploads/utils/validation.ts @@ -224,6 +224,16 @@ export const CHAT_ACCEPT_ATTRIBUTE = [ ACCEPT_ATTRIBUTE, ...SUPPORTED_IMAGE_MIME_TYPES, ...SUPPORTED_IMAGE_EXTENSIONS.map((ext) => `.${ext}`), +].join(',') + +/** + * Accept attribute for the mothership copilot input only. Archives are scoped + * here — NOT in {@link CHAT_ACCEPT_ATTRIBUTE} — because only the copilot flow + * has zip handling (materialize_file "extract"); a zip picked in a workflow or + * deployed chat would flow into execution, where no parser exists. + */ +export const MOTHERSHIP_ACCEPT_ATTRIBUTE = [ + CHAT_ACCEPT_ATTRIBUTE, ...SUPPORTED_ARCHIVE_MIME_TYPES, ...SUPPORTED_ARCHIVE_EXTENSIONS.map((ext) => `.${ext}`), ].join(',') @@ -241,25 +251,35 @@ export const SUPPORTED_ATTACHMENT_EXTENSIONS = Array.from( ...SUPPORTED_IMAGE_EXTENSIONS, ...SUPPORTED_AUDIO_EXTENSIONS, ...SUPPORTED_VIDEO_EXTENSIONS, - ...SUPPORTED_ARCHIVE_EXTENSIONS, ]) ) as readonly string[] /** * Validate that a file's extension is allowed as a chat/mothership attachment. * - * Permits documents, code, images, audio, video, and zip archives — anything - * users would reasonably attach to a chat message. Rejects executables and - * unknown types. + * Permits documents, code, images, audio, and video — anything users would + * reasonably attach to a chat message. Rejects executables and unknown types. + * Archives (`.zip`) are accepted only with `allowArchives` — the mothership + * copilot flow, which alone can extract them; every other upload surface keeps + * rejecting them up front rather than failing mid-run downstream. */ -export function validateAttachmentFileType(fileName: string): FileValidationError | null { +export function validateAttachmentFileType( + fileName: string, + options?: { allowArchives?: boolean } +): FileValidationError | null { const raw = extractExtension(fileName) const extension = isAlphanumericExtension(raw) ? raw : '' - if (!SUPPORTED_ATTACHMENT_EXTENSIONS.includes(extension)) { + const allowed = + SUPPORTED_ATTACHMENT_EXTENSIONS.includes(extension) || + (options?.allowArchives === true && + (SUPPORTED_ARCHIVE_EXTENSIONS as readonly string[]).includes(extension)) + + if (!allowed) { + const archiveNote = options?.allowArchives ? ', and zip archives' : '' return { code: 'UNSUPPORTED_FILE_TYPE', - message: `Unsupported file type${extension ? `: ${extension}` : ` for "${fileName}"`}. Supported types include documents, code, images, audio, video, and zip archives.`, + message: `Unsupported file type${extension ? `: ${extension}` : ` for "${fileName}"`}. Supported types include documents, code, images, audio, video${archiveNote}.`, supportedTypes: [...SUPPORTED_ATTACHMENT_EXTENSIONS], } } From 422d4ac264f21bbe44ee62e9ffb8ad408025b406 Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan Date: Fri, 10 Jul 2026 10:59:39 -0700 Subject: [PATCH 3/4] fix(zip-uploads): address review follow-ups on the extract path - Roll back already-uploaded files when an upload fails mid-extraction (storage/DB error, quota crossed), so callers and retries never observe a partial tree - Fold reserved system folder names (.changelogs, .plans) into the 'archive' fallback so extraction can't write into alias-backing namespaces or bypass the already-extracted lookup that hides them - Align the archive byte-sniff budget with the read path's inline text cap (5MB), so any mislabeled '.zip' small enough to be read inline is sniffed and read instead of dead-ending --- .../tools/handlers/materialize-file.test.ts | 26 +++++++ .../tools/handlers/materialize-file.ts | 14 +++- .../tools/handlers/upload-file-reader.test.ts | 1 + .../tools/handlers/upload-file-reader.ts | 12 ++- apps/sim/lib/copilot/vfs/file-reader.ts | 3 +- apps/sim/lib/uploads/archive.test.ts | 24 +++++- apps/sim/lib/uploads/archive.ts | 74 ++++++++++++------- 7 files changed, 120 insertions(+), 34 deletions(-) diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts index 075961a304f..09d72423d26 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts @@ -429,6 +429,32 @@ describe('executeMaterializeFile - extract operation', () => { expect(mockDecompress).toHaveBeenCalledTimes(1) }) + it('folds reserved system folder names into the "archive" fallback folder', async () => { + // '.changelogs' / '.plans' back workflow changelog/plan aliases; extraction + // must never write into them (and the already-extracted lookup hides them, + // so a second extract would silently duplicate). + mockFindUpload.mockResolvedValue( + zipRow({ displayName: '.changelogs.zip', originalName: '.changelogs.zip' }) + ) + mockFetchBuffer.mockResolvedValue(Buffer.from('zip-bytes')) + mockDecompress.mockResolvedValue({ + extracted: [{ id: 'f1', name: 'a.txt', url: '/x', size: 1, type: 'text/plain', key: 'k1' }], + skipped: 0, + skippedUnsafePaths: [], + }) + + const result = await executeMaterializeFile( + { fileNames: ['.changelogs.zip'], operation: 'extract' }, + context + ) + + expect(result.success).toBe(true) + expect(mockDecompress).toHaveBeenCalledWith( + expect.any(Buffer), + expect.objectContaining({ rootFolderSegments: ['archive'] }) + ) + }) + it('folds degenerate archive names into the "archive" fallback folder', async () => { mockFindUpload.mockResolvedValue(zipRow({ displayName: '..zip', originalName: '..zip' })) mockFetchBuffer.mockResolvedValue(Buffer.from('zip-bytes')) diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts index b26e87580c1..a6a5ff29a11 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts @@ -14,6 +14,7 @@ import { import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types' import { findMothershipUploadRowByChatAndName } from '@/lib/copilot/tools/handlers/upload-file-reader' import { canonicalWorkspaceFilePath, encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils' +import { isReservedWorkflowAliasBackingDisplayPath } from '@/lib/copilot/vfs/workflow-aliases' import { getServePathPrefix } from '@/lib/uploads' import { ArchiveError, @@ -303,6 +304,9 @@ async function executeImport( * empty), so a hostile upload name like `..zip` or `\x01.zip` lands in the * `archive` fallback instead of surfacing a raw internal error — and so the * VFS-encoded destination path can be computed before anything is extracted. + * Reserved system backing folders (`.changelogs`, `.plans`) also fall back: + * extraction must never write into — or hide behind — those namespaces (the + * already-extracted lookup skips them, so they'd also duplicate silently). */ function archiveFolderBaseName(displayName: string): string { const stripped = displayName @@ -312,7 +316,15 @@ function archiveFolderBaseName(displayName: string): string { .replace(/[\x00-\x1f\x7f]/g, '') .replace(/[/\\]/g, '-') .trim() - return !stripped || stripped === '.' || stripped === '..' ? 'archive' : stripped + if ( + !stripped || + stripped === '.' || + stripped === '..' || + isReservedWorkflowAliasBackingDisplayPath(stripped) + ) { + return 'archive' + } + return stripped } /** diff --git a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts index fc2521f9dc6..2f78577deef 100644 --- a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.test.ts @@ -14,6 +14,7 @@ const { mockReadFileRecord, mockFetchBuffer } = vi.hoisted(() => ({ vi.mock('@/lib/copilot/vfs/file-reader', () => ({ readFileRecord: mockReadFileRecord, + MAX_TEXT_READ_BYTES: 5 * 1024 * 1024, })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ diff --git a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts index 25d5eed78c3..171a1589a25 100644 --- a/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts +++ b/apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts @@ -3,7 +3,11 @@ import { workspaceFiles } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { and, asc, desc, eq, isNull, or } from 'drizzle-orm' -import { type FileReadResult, readFileRecord } from '@/lib/copilot/vfs/file-reader' +import { + type FileReadResult, + MAX_TEXT_READ_BYTES, + readFileRecord, +} from '@/lib/copilot/vfs/file-reader' import { type GrepCountEntry, type GrepMatch, @@ -27,9 +31,11 @@ const logger = createLogger('UploadFileReader') * bytes decide (a mislabeled text file named `data.zip` stays readable instead * of being trapped between read-says-extract and extract-says-invalid); above * it the extension is trusted so a real 100MB zip is never downloaded just to - * refuse it. + * refuse it. Aligned with the read path's inline text cap — any mislabeled + * file too big to sniff would be rejected by read() as too large anyway, so + * nothing readable is ever dead-ended. */ -const ARCHIVE_SNIFF_MAX_BYTES = 1024 * 1024 +const ARCHIVE_SNIFF_MAX_BYTES = MAX_TEXT_READ_BYTES /** * True when the upload should get extract-first guidance: named like an archive diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index 26388d5621a..b8b4ba4768d 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -26,7 +26,8 @@ function recordSpanError(span: Span, err: unknown) { const logger = createLogger('FileReader') -const MAX_TEXT_READ_BYTES = 5 * 1024 * 1024 // 5 MB +/** 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 // Parseable-document byte cap. Large office/PDF files can still // produce huge extracted text; reject up front to avoid wasting a diff --git a/apps/sim/lib/uploads/archive.test.ts b/apps/sim/lib/uploads/archive.test.ts index 1be145263bf..29ee2154c1a 100644 --- a/apps/sim/lib/uploads/archive.test.ts +++ b/apps/sim/lib/uploads/archive.test.ts @@ -5,15 +5,17 @@ import { Buffer } from 'buffer' import JSZip from 'jszip' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockEnsureFolder, mockUpload } = vi.hoisted(() => ({ +const { mockEnsureFolder, mockUpload, mockDelete } = vi.hoisted(() => ({ mockEnsureFolder: vi.fn(), mockUpload: vi.fn(), + mockDelete: vi.fn(), })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-folder-manager', () => ({ ensureWorkspaceFileFolderPath: mockEnsureFolder, })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ uploadWorkspaceFile: mockUpload, + deleteWorkspaceFile: mockDelete, })) import { @@ -70,6 +72,7 @@ function craftCentralDirectory(records: number, extraPerRecord: number): Buffer beforeEach(() => { vi.clearAllMocks() mockEnsureFolder.mockResolvedValue('folder_1') + mockDelete.mockResolvedValue(undefined) mockUpload.mockImplementation(async (_ws: string, _uid: string, buf: Buffer, name: string) => ({ id: `f_${name}`, name, @@ -184,6 +187,25 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect(mockUpload).not.toHaveBeenCalled() }) + it('rolls back already-uploaded files when an upload fails mid-extraction', async () => { + // Pass 1 validates caps, but an upload itself can still fail mid-loop + // (storage/DB error, quota crossed). Every file written before the failure + // must be deleted so callers and retries never observe a partial tree. + const buffer = await buildZip({ 'a.txt': 'first', 'b.txt': 'second', 'c.txt': 'third' }) + mockUpload + .mockResolvedValueOnce({ id: 'f_a', name: 'a.txt', url: '/a', key: 'k/a', size: 5 }) + .mockResolvedValueOnce({ id: 'f_b', name: 'b.txt', url: '/b', key: 'k/b', size: 6 }) + .mockRejectedValueOnce(new Error('storage quota exceeded')) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { workspaceId: 'ws', userId: 'u' }) + ).rejects.toThrow('storage quota exceeded') + + expect(mockDelete).toHaveBeenCalledTimes(2) + expect(mockDelete).toHaveBeenCalledWith('ws', 'f_a') + expect(mockDelete).toHaveBeenCalledWith('ws', 'f_b') + }) + it('does not count noise entries toward the extraction cap when they are being skipped', async () => { // macOS Finder zips carry a __MACOSX/._* shadow per file, doubling the raw // entry count. 501 files + 501 shadows = 1002 raw entries — over the diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts index 2046c84ac25..d509df7ada8 100644 --- a/apps/sim/lib/uploads/archive.ts +++ b/apps/sim/lib/uploads/archive.ts @@ -3,7 +3,10 @@ import type { Readable } from 'stream' import JSZip from 'jszip' import { readZipCentralDirectoryStats } from '@/lib/file-parsers/zip-guard' import { ensureWorkspaceFileFolderPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' -import { uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import { + deleteWorkspaceFile, + uploadWorkspaceFile, +} from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' import type { UserFile } from '@/executor/types' @@ -328,40 +331,55 @@ export async function decompressArchiveBufferToWorkspaceFiles( } // Pass 2 — extract: the archive is proven within caps; inflate again and upload. + // Uploads themselves can still fail mid-loop (storage/DB errors, quota crossed + // by another writer), so a failure rolls back every file written so far — + // callers and their retries must never observe a partial tree. const folderIdCache = new Map() const extracted: UserFile[] = [] let totalBytes = 0 - for (const { entry, segments } of safeEntries) { - const result = await inflateEntryWithinCaps(entry, MAX_ARCHIVE_TOTAL_BYTES - totalBytes, true) - if (!result.ok) throwInflateCapError(result.reason, entry.name) - totalBytes += result.size - const entryBuffer = result.buffer as Buffer - - const leafName = segments[segments.length - 1] - const folderSegments = [...rootFolderSegments, ...segments.slice(0, -1)] - const folderKey = folderSegments.join('/') - let folderId = folderIdCache.get(folderKey) - if (folderId === undefined) { - folderId = await ensureWorkspaceFileFolderPath({ + try { + for (const { entry, segments } of safeEntries) { + const result = await inflateEntryWithinCaps(entry, MAX_ARCHIVE_TOTAL_BYTES - totalBytes, true) + if (!result.ok) throwInflateCapError(result.reason, entry.name) + totalBytes += result.size + const entryBuffer = result.buffer as Buffer + + const leafName = segments[segments.length - 1] + const folderSegments = [...rootFolderSegments, ...segments.slice(0, -1)] + const folderKey = folderSegments.join('/') + let folderId = folderIdCache.get(folderKey) + if (folderId === undefined) { + folderId = await ensureWorkspaceFileFolderPath({ + workspaceId, + userId, + pathSegments: folderSegments, + }) + folderIdCache.set(folderKey, folderId) + } + + const mimeType = getMimeTypeFromExtension(getFileExtension(leafName)) + const uploaded = await uploadWorkspaceFile( workspaceId, userId, - pathSegments: folderSegments, - }) - folderIdCache.set(folderKey, folderId) + entryBuffer, + leafName, + mimeType, + { + folderId, + } + ) + extracted.push(uploaded) } - - const mimeType = getMimeTypeFromExtension(getFileExtension(leafName)) - const uploaded = await uploadWorkspaceFile( - workspaceId, - userId, - entryBuffer, - leafName, - mimeType, - { - folderId, + } catch (error) { + for (const file of extracted) { + try { + await deleteWorkspaceFile(workspaceId, file.id) + } catch { + // Best-effort: a file whose cleanup fails is still soft-deletable by hand; + // the original error is what the caller needs to see. } - ) - extracted.push(uploaded) + } + throw error } return { extracted, skipped, skippedUnsafePaths } From 06d7aaaac2e6bf93152ee868b368858f7d0a54f9 Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan Date: Fri, 10 Jul 2026 11:22:17 -0700 Subject: [PATCH 4/4] fix(zip-uploads): detect prior nested-only extractions in the re-extract guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The already-extracted check only looked for files directly inside the archive root folder, so a zip whose entries are all nested (src/index.ts) left only subfolders there and a second extract slipped past the guard, duplicating the tree with ' (1)' suffixes. Extraction roots its whole tree at that folder, so a prior run always leaves a direct file OR a direct subfolder — check both. --- .../tools/handlers/materialize-file.test.ts | 19 ++++++++ .../tools/handlers/materialize-file.ts | 46 ++++++++++++------- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts index 09d72423d26..d81b81bc17c 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts @@ -411,6 +411,25 @@ describe('executeMaterializeFile - extract operation', () => { expect(mockDecompress).not.toHaveBeenCalled() }) + it('detects a prior nested-only extraction via subfolders, not just direct files', async () => { + // A zip containing only nested entries (src/index.ts) leaves NO direct files + // under the archive root — only subfolders. The guard must still refuse. + mockFindUpload.mockResolvedValue(zipRow()) + mockFindFolder.mockResolvedValue('folder-existing') + dbChainMockFns.limit.mockResolvedValueOnce([]) // no direct files + dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'subfolder-1' }]) // but a subfolder tree + + const result = await executeMaterializeFile( + { fileNames: ['bundle.zip'], operation: 'extract' }, + context + ) + + expect(result.success).toBe(false) + const output = result.output as { failed: Array<{ fileName: string; error: string }> } + expect(output.failed[0].error).toContain('already extracted') + expect(mockDecompress).not.toHaveBeenCalled() + }) + it('dedupes repeated fileNames so one call cannot double-extract', async () => { mockFindUpload.mockResolvedValue(zipRow()) mockFetchBuffer.mockResolvedValue(Buffer.from('zip-bytes')) diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts index a6a5ff29a11..7c47c0105ac 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts @@ -1,6 +1,6 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { db } from '@sim/db' -import { workflow, workspaceFiles } from '@sim/db/schema' +import { workflow, workspaceFileFolder, workspaceFiles } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' @@ -312,7 +312,6 @@ function archiveFolderBaseName(displayName: string): string { const stripped = displayName .replace(/\.zip$/i, '') .normalize('NFC') - // biome-ignore lint/suspicious/noControlCharactersInRegex: intentionally stripping control chars .replace(/[\x00-\x1f\x7f]/g, '') .replace(/[/\\]/g, '-') .trim() @@ -379,25 +378,40 @@ async function executeExtract( const folderPath = `files/${encodeVfsPathSegments([baseName])}` // Re-running extract must not silently duplicate the tree with " (1)"-suffixed - // copies: when the destination folder already holds files, report it as - // already extracted instead of extracting beside the previous run. + // copies: when the destination folder already holds content, report it as + // already extracted instead of extracting beside the previous run. Direct + // files AND direct subfolders both count — extraction roots its whole tree + // here, so a prior run of a nested-only zip (e.g. src/index.ts) leaves a + // subfolder even when no file sits at the top level. const existingFolderId = await findWorkspaceFileFolderIdByPath(workspaceId, [baseName]) if (existingFolderId) { - const [existingFile] = await db - .select({ id: workspaceFiles.id }) - .from(workspaceFiles) - .where( - and( - eq(workspaceFiles.folderId, existingFolderId), - eq(workspaceFiles.context, 'workspace'), - isNull(workspaceFiles.deletedAt) + const [[existingFile], [existingSubfolder]] = await Promise.all([ + db + .select({ id: workspaceFiles.id }) + .from(workspaceFiles) + .where( + and( + eq(workspaceFiles.folderId, existingFolderId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) ) - ) - .limit(1) - if (existingFile) { + .limit(1), + db + .select({ id: workspaceFileFolder.id }) + .from(workspaceFileFolder) + .where( + and( + eq(workspaceFileFolder.parentId, existingFolderId), + isNull(workspaceFileFolder.deletedAt) + ) + ) + .limit(1), + ]) + if (existingFile || existingSubfolder) { return { success: false, - error: `"${fileName}" appears to be already extracted — ${folderPath}/ exists and contains files. List them with glob("${folderPath}/**"). To re-extract, delete that folder first.`, + error: `"${fileName}" appears to be already extracted — ${folderPath}/ exists and contains content. List it with glob("${folderPath}/**"). To re-extract, delete that folder first.`, } } }