Skip to content

Commit 2798467

Browse files
authored
feat(zip): add support for zip files (#5788)
* zip checkpoint * 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 * 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 * fix(zip-uploads): detect prior nested-only extractions in the re-extract guard 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.
1 parent 53c2716 commit 2798467

19 files changed

Lines changed: 1494 additions & 251 deletions

File tree

apps/sim/app/api/files/presigned/route.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,9 @@ describe('/api/files/presigned', () => {
588588

589589
const response = await POST(request)
590590
expect(response.status).toBe(200)
591-
expect(mockValidateAttachmentFileType).toHaveBeenCalledWith('screenshot.png')
591+
expect(mockValidateAttachmentFileType).toHaveBeenCalledWith('screenshot.png', {
592+
allowArchives: true,
593+
})
592594
expect(mockValidateFileType).not.toHaveBeenCalled()
593595
})
594596

apps/sim/app/api/files/presigned/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
142142
)
143143
}
144144

145-
const fileValidationError = validateAttachmentFileType(fileName)
145+
const fileValidationError = validateAttachmentFileType(fileName, { allowArchives: true })
146146
if (fileValidationError) {
147147
throw new ValidationError(fileValidationError.message)
148148
}

apps/sim/app/api/files/upload/route.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import type { StorageContext } from '@/lib/uploads/config'
2323
import { generateKnowledgeBaseFileKey } from '@/lib/uploads/contexts/knowledge-base/knowledge-base-file-manager'
2424
import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
2525
import { MAX_WORKSPACE_FORMDATA_FILE_SIZE } from '@/lib/uploads/shared/types'
26-
import { isImageFileType, resolveFileType } from '@/lib/uploads/utils/file-utils'
26+
import { isArchiveFileName, isImageFileType, resolveFileType } from '@/lib/uploads/utils/file-utils'
2727
import {
2828
SUPPORTED_ATTACHMENT_EXTENSIONS,
2929
SUPPORTED_IMAGE_EXTENSIONS,
@@ -34,9 +34,12 @@ import { createErrorResponse, InvalidRequestError } from '@/app/api/files/utils'
3434

3535
const ALLOWED_EXTENSIONS = new Set<string>(SUPPORTED_ATTACHMENT_EXTENSIONS)
3636

37-
function validateFileExtension(filename: string): boolean {
37+
function validateFileExtension(filename: string, context: StorageContext): boolean {
3838
const extension = filename.split('.').pop()?.toLowerCase()
3939
if (!extension) return false
40+
// Archives are only extractable in the mothership copilot flow; every other
41+
// context keeps rejecting them up front instead of failing downstream.
42+
if (context === 'mothership' && isArchiveFileName(filename)) return true
4043
return ALLOWED_EXTENSIONS.has(extension)
4144
}
4245

@@ -150,7 +153,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
150153
for (const file of files) {
151154
const originalName = file.name || 'untitled.md'
152155

153-
if (!validateFileExtension(originalName)) {
156+
if (!validateFileExtension(originalName, context)) {
154157
const extension = originalName.split('.').pop()?.toLowerCase() || 'unknown'
155158
throw new InvalidRequestError(
156159
`File type '${extension}' is not allowed. Allowed types: ${Array.from(ALLOWED_EXTENSIONS).join(', ')}`

apps/sim/app/api/tools/file/manage/route.ts

Lines changed: 35 additions & 208 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { Buffer, isUtf8 } from 'buffer'
2-
import type { Readable } from 'stream'
32
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
43
import { createLogger } from '@sim/logger'
54
import { getErrorMessage } from '@sim/utils/errors'
@@ -21,6 +20,12 @@ import {
2120
ShareValidationError,
2221
upsertFileShare,
2322
} from '@/lib/public-shares/share-manager'
23+
import {
24+
ArchiveError,
25+
type DecompressResult,
26+
decompressArchiveBufferToWorkspaceFiles,
27+
MAX_ARCHIVE_BYTES,
28+
} from '@/lib/uploads/archive'
2429
import { ensureWorkspaceFileFolderPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager'
2530
import {
2631
fetchWorkspaceFileBuffer,
@@ -203,102 +208,6 @@ const uniqueZipEntryName = (name: string, usedNames: Set<string>): string => {
203208
return candidate
204209
}
205210

206-
/** Input archive download cap for the decompress operation. */
207-
const MAX_DECOMPRESS_ARCHIVE_BYTES = 100 * 1024 * 1024
208-
/** Maximum number of entries extracted from a single archive. */
209-
const MAX_DECOMPRESS_ENTRIES = 1000
210-
/** Maximum uncompressed size for any single archive entry. */
211-
const MAX_DECOMPRESS_ENTRY_BYTES = 100 * 1024 * 1024
212-
/** Maximum total uncompressed size across all entries, to bound zip-bomb expansion. */
213-
const MAX_DECOMPRESS_TOTAL_BYTES = 200 * 1024 * 1024
214-
215-
const S_IFMT = 0o170000
216-
const S_IFLNK = 0o120000
217-
218-
/**
219-
* Read a zip entry's declared uncompressed size without materializing it. This
220-
* value comes straight from the (attacker-controlled) ZIP metadata, so it is only
221-
* usable as a cheap fast-reject for honestly-declared archives — never as the
222-
* authoritative cap. {@link inflateEntryWithinCaps} enforces the real limit on the
223-
* inflated byte stream.
224-
*/
225-
const readEntryUncompressedSize = (entry: JSZip.JSZipObject): number | undefined => {
226-
const data = (entry as JSZip.JSZipObject & { _data?: { uncompressedSize?: number } })._data
227-
const size = data?.uncompressedSize
228-
return typeof size === 'number' && Number.isFinite(size) ? size : undefined
229-
}
230-
231-
type InflateResult = { ok: true; buffer: Buffer } | { ok: false; reason: 'entry' | 'total' }
232-
233-
/**
234-
* Inflate a single zip entry through a streaming counting sink, tearing the
235-
* stream down the moment cumulative output would exceed the per-entry cap or the
236-
* remaining total budget. The declared uncompressed size in the ZIP header is
237-
* attacker-controlled and is NOT trusted here: a forged-small or absent size
238-
* cannot cause the full (potentially gigabyte-scale) entry to be materialized in
239-
* memory, because enforcement happens on the actual inflated bytes as they
240-
* arrive. Peak memory is bounded by the cap plus one DEFLATE chunk.
241-
*/
242-
const inflateEntryWithinCaps = (
243-
entry: JSZip.JSZipObject,
244-
remainingTotalBudget: number
245-
): Promise<InflateResult> =>
246-
new Promise((resolve, reject) => {
247-
const chunks: Buffer[] = []
248-
let size = 0
249-
let settled = false
250-
const stream = entry.nodeStream() as Readable
251-
252-
const settle = (result: InflateResult) => {
253-
if (settled) return
254-
settled = true
255-
stream.destroy()
256-
resolve(result)
257-
}
258-
259-
stream.on('data', (chunk: Buffer) => {
260-
size += chunk.length
261-
if (size > MAX_DECOMPRESS_ENTRY_BYTES) {
262-
settle({ ok: false, reason: 'entry' })
263-
return
264-
}
265-
if (size > remainingTotalBudget) {
266-
settle({ ok: false, reason: 'total' })
267-
return
268-
}
269-
chunks.push(chunk)
270-
})
271-
stream.on('end', () => settle({ ok: true, buffer: Buffer.concat(chunks, size) }))
272-
stream.on('error', (error) => {
273-
if (settled) return
274-
settled = true
275-
stream.destroy()
276-
reject(error)
277-
})
278-
})
279-
280-
/** True when a zip entry's unix mode marks it as a symlink (never extracted). */
281-
const isSymlinkEntry = (entry: JSZip.JSZipObject): boolean => {
282-
const mode = (entry as JSZip.JSZipObject & { unixPermissions?: number | null }).unixPermissions
283-
return typeof mode === 'number' && (mode & S_IFMT) === S_IFLNK
284-
}
285-
286-
/**
287-
* Normalize a zip entry path into safe workspace folder segments, guarding against
288-
* zip-slip. Returns null for traversal (`..`), so the entry is skipped rather than
289-
* written outside its intended location.
290-
*/
291-
const sanitizeArchiveEntryPath = (rawPath: string): string[] | null => {
292-
const segments = rawPath
293-
.replace(/\\/g, '/')
294-
.split('/')
295-
.map((segment) => segment.trim())
296-
.filter((segment) => segment.length > 0 && segment !== '.')
297-
298-
if (segments.length === 0 || segments.includes('..')) return null
299-
return segments
300-
}
301-
302211
const isLikelyTextBuffer = (buffer: Buffer): boolean => isUtf8(buffer) && !buffer.includes(0)
303212

304213
/**
@@ -896,135 +805,53 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
896805
if (denied) return denied
897806

898807
const archiveBuffer = await downloadFileFromStorage(archive, requestId, logger, {
899-
maxBytes: MAX_DECOMPRESS_ARCHIVE_BYTES,
808+
maxBytes: MAX_ARCHIVE_BYTES,
900809
})
901810

902-
let zip: JSZip
811+
let result: DecompressResult
903812
try {
904-
zip = await JSZip.loadAsync(archiveBuffer)
905-
} catch {
906-
return NextResponse.json(
907-
{ success: false, error: `"${archive.name}" is not a valid .zip archive` },
908-
{ status: 400 }
909-
)
910-
}
911-
912-
const entries = Object.values(zip.files).filter(
913-
(entry) => !entry.dir && !isSymlinkEntry(entry)
914-
)
915-
if (entries.length > MAX_DECOMPRESS_ENTRIES) {
916-
return NextResponse.json(
917-
{
918-
success: false,
919-
error: `Archive has too many entries to extract. Maximum is ${MAX_DECOMPRESS_ENTRIES}.`,
920-
},
921-
{ status: 413 }
922-
)
923-
}
924-
925-
const entryTooLargeResponse = (name: string) =>
926-
NextResponse.json(
927-
{
928-
success: false,
929-
error: `Archive entry "${name}" is too large to extract. Maximum is ${
930-
MAX_DECOMPRESS_ENTRY_BYTES / (1024 * 1024)
931-
} MB per file.`,
932-
},
933-
{ status: 413 }
934-
)
935-
const totalTooLargeResponse = () =>
936-
NextResponse.json(
937-
{
938-
success: false,
939-
error: `Archive expands to more than the ${
940-
MAX_DECOMPRESS_TOTAL_BYTES / (1024 * 1024)
941-
} MB extraction limit.`,
942-
},
943-
{ status: 413 }
944-
)
945-
946-
// Resolve which entries are safe to extract first, so unsafe entries
947-
// (skipped below) never count toward the size caps.
948-
const safeEntries: Array<{ entry: JSZip.JSZipObject; segments: string[] }> = []
949-
let skippedCount = 0
950-
for (const entry of entries) {
951-
const segments = sanitizeArchiveEntryPath(entry.name)
952-
if (!segments) {
953-
skippedCount += 1
954-
logger.warn('Skipping unsafe archive entry', { name: entry.name })
955-
continue
956-
}
957-
safeEntries.push({ entry, segments })
958-
}
959-
960-
let declaredTotal = 0
961-
for (const { entry } of safeEntries) {
962-
const declaredSize = readEntryUncompressedSize(entry)
963-
if (declaredSize === undefined) continue
964-
if (declaredSize > MAX_DECOMPRESS_ENTRY_BYTES) return entryTooLargeResponse(entry.name)
965-
declaredTotal += declaredSize
966-
if (declaredTotal > MAX_DECOMPRESS_TOTAL_BYTES) return totalTooLargeResponse()
967-
}
968-
969-
const pending: Array<{ segments: string[]; buffer: Buffer }> = []
970-
let totalBytes = 0
971-
for (const { entry, segments } of safeEntries) {
972-
const result = await inflateEntryWithinCaps(
973-
entry,
974-
MAX_DECOMPRESS_TOTAL_BYTES - totalBytes
975-
)
976-
if (!result.ok) {
977-
return result.reason === 'entry'
978-
? entryTooLargeResponse(entry.name)
979-
: totalTooLargeResponse()
813+
result = await decompressArchiveBufferToWorkspaceFiles(archiveBuffer, {
814+
workspaceId,
815+
userId,
816+
})
817+
} catch (archiveError) {
818+
if (archiveError instanceof ArchiveError) {
819+
// The error message is single-sourced in ArchiveError (caps included);
820+
// only the HTTP status is mapped here.
821+
const status = archiveError.reason === 'invalid' ? 400 : 413
822+
return NextResponse.json(
823+
{ success: false, error: `"${archive.name}": ${archiveError.message}` },
824+
{ status }
825+
)
980826
}
981-
totalBytes += result.buffer.length
982-
pending.push({ segments, buffer: result.buffer })
827+
throw archiveError
983828
}
984829

985-
if (pending.length === 0) {
830+
if (result.extracted.length === 0) {
986831
return NextResponse.json(
987-
{
988-
success: false,
989-
error: `No files could be extracted from "${archive.name}".`,
990-
},
832+
{ success: false, error: `No files could be extracted from "${archive.name}".` },
991833
{ status: 422 }
992834
)
993835
}
994836

995-
const folderIdCache = new Map<string, string | null>()
996-
const extractedFiles: UserFile[] = []
997-
for (const { segments, buffer } of pending) {
998-
const leafName = segments[segments.length - 1]
999-
const folderSegments = segments.slice(0, -1)
1000-
const folderKey = folderSegments.join('/')
1001-
let folderId = folderIdCache.get(folderKey)
1002-
if (folderId === undefined) {
1003-
folderId = await ensureWorkspaceFileFolderPath({
1004-
workspaceId,
1005-
userId,
1006-
pathSegments: folderSegments,
1007-
})
1008-
folderIdCache.set(folderKey, folderId)
1009-
}
837+
const extractedFiles = result.extracted.map((file) => ({
838+
...file,
839+
url: ensureAbsoluteUrl(file.url),
840+
}))
1010841

1011-
const mimeType = getMimeTypeFromExtension(getFileExtension(leafName))
1012-
const uploaded = await uploadWorkspaceFile(
1013-
workspaceId,
1014-
userId,
1015-
buffer,
1016-
leafName,
1017-
mimeType,
1018-
{ folderId }
1019-
)
1020-
extractedFiles.push({ ...uploaded, url: ensureAbsoluteUrl(uploaded.url) })
842+
if (result.skippedUnsafePaths.length > 0) {
843+
logger.warn('Skipped unsafe archive entries', {
844+
fileId: archive.id,
845+
name: archive.name,
846+
entryNames: result.skippedUnsafePaths,
847+
})
1021848
}
1022849

1023850
logger.info('Archive decompressed', {
1024851
fileId: archive.id,
1025852
name: archive.name,
1026853
extractedCount: extractedFiles.length,
1027-
skippedCount,
854+
skippedCount: result.skipped,
1028855
})
1029856

1030857
return NextResponse.json({

apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { createLogger } from '@sim/logger'
1515
import { useParams } from 'next/navigation'
1616
import { getMothershipAttachmentPreviewUrl } from '@/lib/copilot/chat/attachment-preview'
1717
import { SIM_RESOURCE_DRAG_TYPE, SIM_RESOURCES_DRAG_TYPE } from '@/lib/copilot/resource-types'
18-
import { CHAT_ACCEPT_ATTRIBUTE } from '@/lib/uploads/utils/validation'
18+
import { MOTHERSHIP_ACCEPT_ATTRIBUTE } from '@/lib/uploads/utils/validation'
1919
import { useChatSurface } from '@/app/workspace/[workspaceId]/home/components/chat-surface-context'
2020
import {
2121
AnimatedPlaceholderEffect,
@@ -597,7 +597,7 @@ const UserInputImpl = forwardRef<UserInputHandle, UserInputProps>(function UserI
597597
type='file'
598598
onChange={handleFileChange}
599599
className='hidden'
600-
accept={CHAT_ACCEPT_ATTRIBUTE}
600+
accept={MOTHERSHIP_ACCEPT_ATTRIBUTE}
601601
multiple
602602
/>
603603

apps/sim/lib/copilot/chat/payload.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { encodeVfsSegment } from '@/lib/copilot/vfs/path-utils'
1616
import type { BlockVisibilityState } from '@/lib/core/config/block-visibility'
1717
import { isE2BDocEnabled, isHosted } from '@/lib/core/config/env-flags'
1818
import { trackChatUpload } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
19+
import { buildArchiveExtractGuidance, isArchiveFileName } from '@/lib/uploads/utils/file-utils'
1920
import { stripVersionSuffix } from '@/tools/utils'
2021

2122
const logger = createLogger('CopilotChatPayload')
@@ -343,15 +344,25 @@ export async function buildCopilotRequestPayload(
343344
} catch {
344345
encodedUploadName = displayName
345346
}
346-
const lines = [
347-
`File "${displayName}" (${mediaType}, ${f.size} bytes) uploaded.`,
348-
`Read with: read("uploads/${encodedUploadName}")`,
349-
`To save permanently: materialize_file(fileName: "${displayName}")`,
350-
]
351-
if (displayName.endsWith('.json')) {
352-
lines.push(
353-
`To import as a workflow: materialize_file(fileName: "${displayName}", operation: "import")`
354-
)
347+
let lines: string[]
348+
if (isArchiveFileName(displayName)) {
349+
// A .zip is stored in uploads/ but its contents aren't readable until
350+
// the agent extracts it once into workspace files/ (explicit step).
351+
lines = [
352+
`Archive "${displayName}" (${mediaType}, ${f.size} bytes) uploaded.`,
353+
buildArchiveExtractGuidance(displayName),
354+
]
355+
} else {
356+
lines = [
357+
`File "${displayName}" (${mediaType}, ${f.size} bytes) uploaded.`,
358+
`Read with: read("uploads/${encodedUploadName}")`,
359+
`To save permanently: materialize_file(fileName: "${displayName}")`,
360+
]
361+
if (displayName.endsWith('.json')) {
362+
lines.push(
363+
`To import as a workflow: materialize_file(fileName: "${displayName}", operation: "import")`
364+
)
365+
}
355366
}
356367
uploadContexts.push({
357368
type: 'uploaded_file',

0 commit comments

Comments
 (0)