From 6e4fd821121d9a2365209eab57674e4260a51284 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:53:03 -0700 Subject: [PATCH 01/10] feat(files): support zip extraction --- .../[id]/files/[fileId]/route.test.ts | 67 +++- .../workspaces/[id]/files/[fileId]/route.ts | 18 ++ .../workspace/[workspaceId]/files/files.tsx | 49 ++- .../queries/workspace-file-folders.test.ts | 55 ++++ .../hooks/queries/workspace-file-folders.ts | 20 ++ apps/sim/lib/api/contracts/workspace-files.ts | 18 ++ apps/sim/lib/uploads/archive.test.ts | 223 ++++++++++--- apps/sim/lib/uploads/archive.ts | 108 +++++-- .../workspace-file-folder-manager.test.ts | 124 +++++++- .../workspace-file-folder-manager.ts | 108 ++++++- .../workspace/workspace-file-manager.ts | 60 +++- .../workspace-file-storage-accounting.test.ts | 65 ++++ .../uploads/upload-session/service.test.ts | 29 +- .../sim/lib/uploads/upload-session/service.ts | 10 +- .../api/internal-error-policies.test.ts | 22 ++ .../api/internal-error-policies.ts | 17 +- .../application/create-workspace-file.ts | 6 +- .../extract-workspace-file.test.ts | 301 ++++++++++++++++++ .../application/extract-workspace-file.ts | 201 ++++++++++++ .../workspace-files/application/operations.ts | 6 + 20 files changed, 1404 insertions(+), 103 deletions(-) create mode 100644 apps/sim/hooks/queries/workspace-file-folders.test.ts create mode 100644 apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts create mode 100644 apps/sim/lib/workspace-files/application/extract-workspace-file.ts diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts index a0b4d86aba6..ea575747450 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts @@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ getSession: vi.fn(), + extract: vi.fn(), rename: vi.fn(), deleteItems: vi.fn(), getUserEntityPermissions: vi.fn(), @@ -14,6 +15,13 @@ const mocks = vi.hoisted(() => ({ vi.mock('@/lib/auth', () => ({ getSession: mocks.getSession })) +vi.mock('@/lib/workspace-files/application/extract-workspace-file', () => ({ + extractWorkspaceFile: { + operation: { id: 'files.extract_archive', minimumRole: 'write', workspaceApiKey: 'deny' }, + execute: mocks.extract, + }, +})) + vi.mock('@/lib/workspace-files/application/rename-workspace-file', () => ({ renameWorkspaceFile: { operation: { id: 'files.rename', minimumRole: 'write', workspaceApiKey: 'allow' }, @@ -38,7 +46,8 @@ import { WorkspaceApiKeyScopeAuthorizationError, } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' -import { PATCH } from '@/app/api/workspaces/[id]/files/[fileId]/route' +import { ArchiveError } from '@/lib/uploads/archive' +import { PATCH, POST } from '@/app/api/workspaces/[id]/files/[fileId]/route' const WORKSPACE_ID = 'workspace-1' const FILE_ID = 'wf_1' @@ -55,6 +64,15 @@ function callRename(body: unknown) { ) } +function callExtract() { + return POST( + new NextRequest(`http://localhost:3000/api/workspaces/${WORKSPACE_ID}/files/${FILE_ID}`, { + method: 'POST', + }), + context + ) +} + function fileRecord() { return { id: FILE_ID, @@ -79,6 +97,7 @@ describe('PATCH /api/workspaces/[id]/files/[fileId]', () => { session: { id: 'session-1' }, }) mocks.rename.mockResolvedValue({ file: fileRecord() }) + mocks.extract.mockResolvedValue({ folderName: 'bundle', extractedCount: 2, skippedCount: 0 }) }) it('authenticates before parsing the request', async () => { @@ -170,3 +189,49 @@ describe('PATCH /api/workspaces/[id]/files/[fileId]', () => { }) }) }) + +describe('POST /api/workspaces/[id]/files/[fileId]', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.getSession.mockResolvedValue({ + user: { id: 'user-1' }, + session: { id: 'session-1' }, + }) + mocks.extract.mockResolvedValue({ folderName: 'bundle', extractedCount: 2, skippedCount: 0 }) + }) + + it('passes a session principal and canonical assertion to the extraction use case', async () => { + const response = await callExtract() + + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ + success: true, + folderName: 'bundle', + extractedCount: 2, + skippedCount: 0, + }) + expect(mocks.extract).toHaveBeenCalledWith({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { fileId: FILE_ID, assertedWorkspaceId: WORKSPACE_ID }, + request: expect.anything(), + }) + }) + + it('authenticates before invoking extraction', async () => { + mocks.getSession.mockResolvedValue(null) + + const response = await callExtract() + + expect(response.status).toBe(401) + expect(mocks.extract).not.toHaveBeenCalled() + }) + + it('returns a caller-safe error for an invalid zip', async () => { + mocks.extract.mockRejectedValue(new ArchiveError('invalid', 'Not a valid .zip archive.')) + + const response = await callExtract() + + expect(response.status).toBe(400) + expect(await response.json()).toEqual({ error: 'Not a valid .zip archive.' }) + }) +}) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts index a93c34f07f3..e0e7cae5646 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts @@ -1,5 +1,6 @@ import { deleteWorkspaceFileContract, + extractWorkspaceFileContract, renameWorkspaceFileContract, } from '@/lib/api/contracts/workspace-files' import { @@ -14,10 +15,27 @@ import { internalFilePresenters, } from '@/lib/workspace-files/api' import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file' +import { extractWorkspaceFile } from '@/lib/workspace-files/application/extract-workspace-file' import { fileOperations } from '@/lib/workspace-files/application/operations' import { renameWorkspaceFile } from '@/lib/workspace-files/application/rename-workspace-file' export const dynamic = 'force-dynamic' +export const maxDuration = 300 + +/** + * POST /api/workspaces/[id]/files/[fileId] + * Unzip an archive file into a new folder beside it (requires write permission) + */ +export const POST = defineInternalJsonRoute({ + contract: extractWorkspaceFileContract, + auth: internalSessionAuth, + operation: fileOperations.extractArchive, + rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal file behavior' }), + errorPolicy: internalFileErrorPolicies.extractArchive, + mapInput: ({ params }) => ({ fileId: params.fileId, assertedWorkspaceId: params.id }), + useCase: extractWorkspaceFile, + present: (result) => ({ success: true, ...result }), +}) /** * PATCH /api/workspaces/[id]/files/[fileId] diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index b29e169914b..ff70a3b7dd2 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -33,12 +33,14 @@ import { formatFileSize, getFileExtension, getMimeTypeFromExtension, + isArchiveFileName, isAudioFileType, isVideoFileType, resolveEffectiveMimeType, } from '@/lib/uploads/utils/file-utils' import { isSupportedExtension, + SUPPORTED_ARCHIVE_EXTENSIONS, SUPPORTED_AUDIO_EXTENSIONS, SUPPORTED_CODE_EXTENSIONS, SUPPORTED_DOCUMENT_EXTENSIONS, @@ -119,6 +121,7 @@ import { useWorkspaceMembersQuery, type WorkspaceMember } from '@/hooks/queries/ import { useBulkArchiveWorkspaceFileItems, useCreateWorkspaceFileFolder, + useExtractWorkspaceFile, useMoveWorkspaceFileItems, useUpdateWorkspaceFileFolder, useWorkspaceFileFolders, @@ -174,6 +177,7 @@ const SUPPORTED_EXTENSIONS = [ ...SUPPORTED_AUDIO_EXTENSIONS, ...SUPPORTED_VIDEO_EXTENSIONS, ...SUPPORTED_IMAGE_EXTENSIONS, + ...SUPPORTED_ARCHIVE_EXTENSIONS, ] as const const ACCEPT_ATTR = SUPPORTED_EXTENSIONS.map((ext) => `.${ext}`).join(',') @@ -189,6 +193,7 @@ const COLUMNS: ResourceColumn[] = [ const MIME_TYPE_LABELS: Record = { 'application/pdf': 'PDF', + 'application/zip': 'ZIP', 'application/msword': 'Word', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document': 'Word', 'application/vnd.ms-excel': 'Excel', @@ -277,6 +282,7 @@ export function Files() { const deleteFile = useDeleteWorkspaceFile() const renameFile = useRenameWorkspaceFile() const createFolder = useCreateWorkspaceFileFolder() + const extractFile = useExtractWorkspaceFile() const updateFolder = useUpdateWorkspaceFileFolder() const moveItems = useMoveWorkspaceFileItems() const bulkArchiveItems = useBulkArchiveWorkspaceFileItems() @@ -386,6 +392,8 @@ export function Files() { }) const [showUnsavedChangesAlert, setShowUnsavedChangesAlert] = useState(false) const [showDeleteConfirm, setShowDeleteConfirm] = useState(false) + const [extractTargetId, setExtractTargetId] = useState(null) + const extractTarget = extractTargetId ? (fileById.get(extractTargetId) ?? null) : null const contextMenuItemRef = useRef(null) const [deleteTarget, setDeleteTarget] = useState<{ fileIds: string[] @@ -1430,6 +1438,11 @@ export function Files() { void setFilesParams({ folderId: parsed.id, new: null }) return } + const file = fileByIdRef.current.get(parsed.id) + if (file && isArchiveFileName(file.name)) { + if (!extractFile.isPending) setExtractTargetId(file.id) + return + } router.push( currentFolderId ? `/workspace/${workspaceId}/files/${parsed.id}?folderId=${currentFolderId}` @@ -1437,9 +1450,24 @@ export function Files() { ) } }, - [router, workspaceId, currentFolderId, setFilesParams] + [router, workspaceId, currentFolderId, setFilesParams, extractFile.isPending] ) + const handleExtract = async () => { + if (!extractTarget || !canEdit) return + try { + await extractFile.mutateAsync({ + workspaceId, + fileId: extractTarget.id, + fileName: extractTarget.name, + }) + } catch (error) { + logger.error('Failed to unzip archive:', error) + } finally { + setExtractTargetId(null) + } + } + const handleUploadClick = useCallback(() => { if (!canEdit || uploading) return fileInputRef.current?.click() @@ -1944,6 +1972,25 @@ export function Files() { isPending={deleteFile.isPending || bulkArchiveItems.isPending} /> + !open && setExtractTargetId(null)} + title='Unzip archive?' + text={[ + 'This will unzip ', + { text: extractTarget?.name ?? 'this archive', bold: true }, + ' into a new folder beside it.', + ]} + confirm={{ + label: 'Unzip', + onClick: () => void handleExtract(), + variant: 'primary', + pending: extractFile.isPending, + pendingLabel: 'Unzipping...', + disabled: !canEdit, + }} + /> + {shareModal} ({ + queryClient: { + invalidateQueries: vi.fn(), + }, +})) + +vi.mock('@sim/emcn', () => ({ + toast: { error: vi.fn(), success: vi.fn() }, +})) + +vi.mock('@tanstack/react-query', () => ({ + keepPreviousData: {}, + useMutation: vi.fn((options) => options), + useQuery: vi.fn(), + useQueryClient: vi.fn(() => queryClient), +})) + +vi.mock('@/lib/api/client/request', () => ({ requestJson: vi.fn() })) + +const variables = { workspaceId: 'workspace-1', fileId: 'file-1', fileName: 'archive.zip' } + +describe('useExtractWorkspaceFile reconciliation', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('invalidates file browsers after success', () => { + const mutation = useExtractWorkspaceFile() + + mutation.onSuccess( + { success: true, folderName: 'archive', extractedCount: 2, skippedCount: 0 }, + variables + ) + mutation.onSettled(undefined, undefined, variables) + + expect(queryClient.invalidateQueries).toHaveBeenCalledTimes(3) + }) + + it('invalidates file browsers after an API error response', () => { + const mutation = useExtractWorkspaceFile() + const error = new ApiClientError({ status: 409, message: 'Folder exists', body: {} }) + + mutation.onError(error, variables) + mutation.onSettled(undefined, error, variables) + + expect(queryClient.invalidateQueries).toHaveBeenCalledTimes(3) + }) +}) diff --git a/apps/sim/hooks/queries/workspace-file-folders.ts b/apps/sim/hooks/queries/workspace-file-folders.ts index f9338436288..bdddc5e6856 100644 --- a/apps/sim/hooks/queries/workspace-file-folders.ts +++ b/apps/sim/hooks/queries/workspace-file-folders.ts @@ -11,6 +11,7 @@ import { updateWorkspaceFileFolderContract, type WorkspaceFileFolderApi, } from '@/lib/api/contracts/workspace-file-folders' +import { extractWorkspaceFileContract } from '@/lib/api/contracts/workspace-files' import { buildWorkspaceFileFolderDisplayPath, parseWorkspaceFileFolderDisplayPath, @@ -87,6 +88,25 @@ export function useCreateWorkspaceFileFolder() { }) } +export function useExtractWorkspaceFile() { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: async (variables: { workspaceId: string; fileId: string; fileName: string }) => + requestJson(extractWorkspaceFileContract, { + params: { id: variables.workspaceId, fileId: variables.fileId }, + }), + onSuccess: (data, variables) => { + toast.success(`Unzipped "${variables.fileName}" into "${data.folderName}"`) + }, + onError: (error) => { + toast.error(toError(error).message) + }, + onSettled: (_data, _error, variables) => { + invalidateWorkspaceFileBrowsers(queryClient, variables.workspaceId) + }, + }) +} + export function useUpdateWorkspaceFileFolder() { const queryClient = useQueryClient() return useMutation({ diff --git a/apps/sim/lib/api/contracts/workspace-files.ts b/apps/sim/lib/api/contracts/workspace-files.ts index 3540f4b4db0..0c7f683b3d2 100644 --- a/apps/sim/lib/api/contracts/workspace-files.ts +++ b/apps/sim/lib/api/contracts/workspace-files.ts @@ -145,6 +145,14 @@ const listWorkspaceFilesResponseSchema = workspaceFileSuccessSchema.extend({ export type ListWorkspaceFilesResponse = z.output +export const extractWorkspaceFileResponseSchema = workspaceFileSuccessSchema.extend({ + folderName: z.string(), + extractedCount: z.number().int().nonnegative(), + skippedCount: z.number().int().nonnegative(), +}) + +export type ExtractWorkspaceFileResponse = z.output + export const listWorkspaceFilesContract = defineRouteContract({ method: 'GET', path: '/api/workspaces/[id]/files', @@ -184,6 +192,16 @@ export const renameWorkspaceFileContract = defineRouteContract({ error: renameWorkspaceFileErrorSchema, }) +export const extractWorkspaceFileContract = defineRouteContract({ + method: 'POST', + path: '/api/workspaces/[id]/files/[fileId]', + params: workspaceFileParamsSchema, + response: { + mode: 'json', + schema: extractWorkspaceFileResponseSchema, + }, +}) + export const updateWorkspaceFileDimensionsContract = defineRouteContract({ method: 'PATCH', path: '/api/workspaces/[id]/files/[fileId]/dimensions', diff --git a/apps/sim/lib/uploads/archive.test.ts b/apps/sim/lib/uploads/archive.test.ts index 91fe18f2de0..3e49a34687c 100644 --- a/apps/sim/lib/uploads/archive.test.ts +++ b/apps/sim/lib/uploads/archive.test.ts @@ -16,35 +16,42 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' * - `exactName: true` throws `FileConflictError` on a duplicate leaf name, while * `exactName: false` auto-suffixes, mirroring `uploadWorkspaceFile`. */ -const { store, mockUpload, mockDelete, mockEnsureFolder, mockDeleteFolder } = vi.hoisted(() => ({ - store: { - folderIdByPath: new Map(), - fileKeys: new Set(), - /** Paths passed to the folder-delete operation, in call order. */ - deletedFolderPaths: [] as string[], - sequence: 0, - }, - mockUpload: vi.fn(), - mockDelete: vi.fn(), - mockEnsureFolder: vi.fn(), - mockDeleteFolder: vi.fn(), -})) +const { store, mockUpload, mockPurge, mockEnsureFolder, mockArchiveFolderIfEmpty } = vi.hoisted( + () => ({ + store: { + folderIdByPath: new Map(), + fileKeys: new Set(), + blockedFolderIds: new Set(), + /** Paths passed to the folder-delete operation, in call order. */ + deletedFolderPaths: [] as string[], + sequence: 0, + }, + mockUpload: vi.fn(), + mockPurge: vi.fn(), + mockEnsureFolder: vi.fn(), + mockArchiveFolderIfEmpty: vi.fn(), + }) +) vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({ ensureWorkspaceFileFolderPathOperation: { execute: mockEnsureFolder }, - deleteWorkspaceFileFolderOperation: { execute: mockDeleteFolder }, })) vi.mock('@/lib/workspace-files/application/create-workspace-file', () => ({ createWorkspaceFileFromBuffer: { execute: mockUpload, }, })) -vi.mock('@/lib/workspace-files/application/delete-workspace-file', () => ({ - deleteWorkspaceFileOperation: { - execute: mockDelete, - }, +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + purgeCreatedWorkspaceFile: mockPurge, +})) +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-folder-manager', () => ({ + archiveWorkspaceFileFolderIfEmpty: mockArchiveFolderIfEmpty, })) -import { buildFolderPath } from '@/lib/folders/paths' +import { + buildFolderPath, + MAX_FOLDER_PATH_BYTES, + MAX_FOLDER_PATH_SEGMENTS, +} from '@/lib/folders/paths' import { decompressArchiveBufferToWorkspaceFiles, MAX_ARCHIVE_CENTRAL_DIR_EXTRA_BYTES, @@ -137,6 +144,7 @@ beforeEach(() => { vi.clearAllMocks() store.folderIdByPath.clear() store.fileKeys.clear() + store.blockedFolderIds.clear() store.deletedFolderPaths.length = 0 store.sequence = 0 @@ -161,24 +169,19 @@ beforeEach(() => { return { folderId, createdFolderIds } }) - mockDeleteFolder.mockImplementation( - async ({ input }: { input: { folderId?: string; recursive?: boolean } }) => { - const path = folderPathById(input.folderId) - // Mirrors `deleteWorkspaceFileFolderOperation`, which raises `not_found` when - // nothing was archived — deleting a parent before its children would make the - // child's own delete hit this. - if (!path) throw new Error('Folder not found') - store.deletedFolderPaths.push(path) - for (const [candidate] of store.folderIdByPath) { - if (candidate === path || candidate.startsWith(`${path}/`)) { - store.folderIdByPath.delete(candidate) - } - } - return { deletedItems: { files: 0, folders: 1 } } - } - ) + mockArchiveFolderIfEmpty.mockImplementation(async ({ folderId }: { folderId: string }) => { + const path = folderPathById(folderId) + if (!path) throw new Error('Folder not found') + const hasChild = [...store.folderIdByPath.keys()].some((candidate) => + candidate.startsWith(`${path}/`) + ) + if (store.blockedFolderIds.has(folderId) || hasChild) throw new Error('Folder is not empty') + store.deletedFolderPaths.push(path) + store.folderIdByPath.delete(path) + return true + }) - mockDelete.mockResolvedValue(undefined) + mockPurge.mockResolvedValue(true) mockUpload.mockImplementation( async ({ input, @@ -319,7 +322,7 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { 'other.txt', 'report (1).txt', ]) - expect(mockDelete).not.toHaveBeenCalled() + expect(mockPurge).not.toHaveBeenCalled() }) it('marks extracted files unknown when an archive has secret provenance', async () => { @@ -338,7 +341,10 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect(mockUpload).toHaveBeenCalledTimes(2) for (const call of mockUpload.mock.calls) { expect(call[0].input).toEqual( - expect.objectContaining({ secretProvenance: { status: 'unknown' } }) + expect.objectContaining({ + secretProvenance: { status: 'unknown' }, + notifyWorkspaceChange: false, + }) ) } }) @@ -459,12 +465,29 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { // (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' }) + const uploadedAt = new Date('2026-08-17T12:00:00.000Z') mockUpload .mockResolvedValueOnce({ - file: { id: 'f_a', name: 'a.txt', url: '/a', key: 'k/a', size: 5 }, + file: { + id: 'f_a', + name: 'a.txt', + url: '/a', + key: 'k/a', + size: 5, + folderId: 'folder_archive', + updatedAt: uploadedAt, + }, }) .mockResolvedValueOnce({ - file: { id: 'f_b', name: 'b.txt', url: '/b', key: 'k/b', size: 6 }, + file: { + id: 'f_b', + name: 'b.txt', + url: '/b', + key: 'k/b', + size: 6, + folderId: 'folder_archive', + updatedAt: uploadedAt, + }, }) .mockRejectedValueOnce(new Error('storage quota exceeded')) @@ -475,13 +498,23 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { }) ).rejects.toThrow('storage quota exceeded') - expect(mockDelete).toHaveBeenCalledTimes(2) - expect(mockDelete).toHaveBeenCalledWith( - expect.objectContaining({ input: { fileId: 'f_a', assertedWorkspaceId: 'ws' } }) - ) - expect(mockDelete).toHaveBeenCalledWith( - expect.objectContaining({ input: { fileId: 'f_b', assertedWorkspaceId: 'ws' } }) - ) + expect(mockPurge).toHaveBeenCalledTimes(2) + expect(mockPurge).toHaveBeenCalledWith({ + workspaceId: 'ws', + fileId: 'f_a', + key: 'k/a', + expectedName: 'a.txt', + expectedFolderId: 'folder_archive', + expectedUpdatedAt: uploadedAt, + }) + expect(mockPurge).toHaveBeenCalledWith({ + workspaceId: 'ws', + fileId: 'f_b', + key: 'k/b', + expectedName: 'b.txt', + expectedFolderId: 'folder_archive', + expectedUpdatedAt: uploadedAt, + }) }) it('rolls back the folders it created when an upload fails mid-extraction', async () => { @@ -504,10 +537,8 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { ).rejects.toThrow('storage quota exceeded') expect([...store.folderIdByPath.keys()]).toEqual([]) - expect(mockDeleteFolder).toHaveBeenCalledWith( - expect.objectContaining({ - input: expect.objectContaining({ workspaceId: 'ws', recursive: true }), - }) + expect(mockArchiveFolderIfEmpty).toHaveBeenCalledWith( + expect.objectContaining({ workspaceId: 'ws' }) ) }) @@ -533,7 +564,7 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect([...store.folderIdByPath.keys()].sort()).toEqual(['/bundle', '/bundle/keep']) expect(store.deletedFolderPaths).toEqual(['/bundle/fresh']) - const deletedIds = mockDeleteFolder.mock.calls.map(([args]) => args.input.folderId) + const deletedIds = mockArchiveFolderIfEmpty.mock.calls.map(([args]) => args.folderId) for (const preexistingId of preexistingIds) { expect(deletedIds).not.toContain(preexistingId) } @@ -562,6 +593,27 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect([...store.folderIdByPath.keys()]).toEqual([]) }) + it('preserves created folders that gain collaborator content before rollback', async () => { + const buffer = await buildZip({ 'nested/one.txt': 'first', 'nested/two.txt': 'second' }) + mockUpload + .mockImplementationOnce(async ({ input }) => { + store.blockedFolderIds.add(input.folderId) + return { file: { id: 'f_one', name: 'one.txt', url: '/one', key: 'k/one', size: 5 } } + }) + .mockRejectedValueOnce(new Error('storage quota exceeded')) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + principal: TEST_PRINCIPAL, + rootFolderSegments: ['bundle'], + }) + ).rejects.toThrow('storage quota exceeded') + + expect([...store.folderIdByPath.keys()]).toEqual(['/bundle', '/bundle/nested']) + expect(store.deletedFolderPaths).toEqual([]) + }) + 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 @@ -584,6 +636,73 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect(result.skipped).toBe(501) }) + it('rejects a materialized tree above the bulk-operation limit before creating its root', async () => { + const buffer = await buildZip({ 'nested/file.txt': 'x' }) + const prepareRootFolder = vi.fn(async () => ['bundle']) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + principal: TEST_PRINCIPAL, + prepareRootFolder, + materializedRootFolderCount: 1, + maxMaterializedItems: 2, + }) + ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'too_many_entries' }) + + expect(prepareRootFolder).not.toHaveBeenCalled() + expect(mockEnsureFolder).not.toHaveBeenCalled() + expect(mockUpload).not.toHaveBeenCalled() + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + principal: TEST_PRINCIPAL, + prepareRootFolder, + materializedRootFolderCount: 1, + maxMaterializedItems: 3, + }) + ).resolves.toMatchObject({ extracted: [expect.objectContaining({ name: 'file.txt' })] }) + expect(prepareRootFolder).toHaveBeenCalledOnce() + }) + + it.each([ + { + label: 'too many folder segments', + entryName: `${Array.from({ length: MAX_FOLDER_PATH_SEGMENTS + 1 }, () => 'x').join('/')}/file.txt`, + expectedMessage: `Folder paths cannot exceed ${MAX_FOLDER_PATH_SEGMENTS} segments`, + }, + { + label: 'too many encoded folder-path bytes', + entryName: `${'x'.repeat(MAX_FOLDER_PATH_BYTES)}/file.txt`, + expectedMessage: `Folder paths cannot exceed ${MAX_FOLDER_PATH_BYTES} bytes`, + }, + ])( + 'rejects $label before enumerating or creating folders', + async ({ entryName, expectedMessage }) => { + const buffer = await buildZip({ [entryName]: 'x' }) + const prepareRootFolder = vi.fn(async () => ['bundle']) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + principal: TEST_PRINCIPAL, + prepareRootFolder, + materializedRootFolderCount: 1, + maxMaterializedItems: 5000, + }) + ).rejects.toMatchObject({ + name: 'ArchiveError', + reason: 'invalid', + message: expect.stringContaining(expectedMessage), + }) + + expect(prepareRootFolder).not.toHaveBeenCalled() + expect(mockEnsureFolder).not.toHaveBeenCalled() + 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'), { diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts index 561d48330f0..7f5014ef538 100644 --- a/apps/sim/lib/uploads/archive.ts +++ b/apps/sim/lib/uploads/archive.ts @@ -1,16 +1,19 @@ import { Buffer } from 'buffer' import type { Readable } from 'stream' import type { Principal } from '@sim/auth/principal' +import { createLogger } from '@sim/logger' import JSZip from 'jszip' import { readZipCentralDirectoryStats } from '@/lib/file-parsers/zip-guard' +import { buildFolderPath, FolderPathError } from '@/lib/folders/paths' +import { archiveWorkspaceFileFolderIfEmpty } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' +import { + purgeCreatedWorkspaceFile, + type WorkspaceFileRecord, +} from '@/lib/uploads/contexts/workspace/workspace-file-manager' import type { WorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' import { createWorkspaceFileFromBuffer } from '@/lib/workspace-files/application/create-workspace-file' -import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file' -import { - deleteWorkspaceFileFolderOperation, - ensureWorkspaceFileFolderPathOperation, -} from '@/lib/workspace-files/application/workspace-file-folders' +import { ensureWorkspaceFileFolderPathOperation } from '@/lib/workspace-files/application/workspace-file-folders' import type { UserFile } from '@/executor/types' /** @@ -31,6 +34,8 @@ import type { UserFile } from '@/executor/types' * entry in both passes. */ +const logger = createLogger('ArchiveExtraction') + /** Input archive download/size cap. */ export const MAX_ARCHIVE_BYTES = 100 * 1024 * 1024 /** Maximum number of entries extracted from a single archive. */ @@ -255,6 +260,14 @@ function throwInflateCapError(reason: 'entry' | 'total', entryName: string): nev * 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). * + * When `prepareRootFolder` is provided it takes precedence over + * `rootFolderSegments`: it runs once, only after the caps have been proven and + * only when at least one safe entry exists, and extraction lands under the + * segments it returns. `materializedRootFolderCount` must equal the number of + * folders that callback will create, so the `maxMaterializedItems` pre-check + * (files + implied folders + root folders) counts them and rejects an + * over-limit archive before the callback materializes anything. + * * 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. Decompression is not byte-preserving, @@ -267,6 +280,9 @@ export async function decompressArchiveBufferToWorkspaceFiles( workspaceId: string principal: Principal rootFolderSegments?: string[] + prepareRootFolder?: () => Promise + materializedRootFolderCount?: number + maxMaterializedItems?: number skipNoiseEntries?: boolean secretProvenance?: WorkspaceFileSecretProvenance } @@ -275,6 +291,9 @@ export async function decompressArchiveBufferToWorkspaceFiles( workspaceId, principal, rootFolderSegments = [], + prepareRootFolder, + materializedRootFolderCount = 0, + maxMaterializedItems, skipNoiseEntries = false, secretProvenance = { status: 'unknown' }, } = opts @@ -323,6 +342,38 @@ export async function decompressArchiveBufferToWorkspaceFiles( ) } + for (const { entry, segments } of safeEntries) { + try { + buildFolderPath(segments.slice(0, -1)) + } catch (error) { + if (!(error instanceof FolderPathError)) throw error + throw new ArchiveError( + 'invalid', + `Archive contains an invalid folder path: ${error.message}`, + entry.name + ) + } + } + + if (maxMaterializedItems !== undefined) { + const folderPaths = new Set() + for (const { segments } of safeEntries) { + for (let depth = 1; depth < segments.length; depth += 1) { + folderPaths.add(segments.slice(0, depth).join('\0')) + } + } + const materializedItems = + safeEntries.length + + folderPaths.size + + (safeEntries.length > 0 ? materializedRootFolderCount : 0) + if (materializedItems > maxMaterializedItems) { + throw new ArchiveError( + 'too_many_entries', + `Archive would create ${materializedItems} files and folders; the maximum is ${maxMaterializedItems}.` + ) + } + } + // Cheap declared-size fast-reject for honestly-declared archives. let declaredTotal = 0 for (const { entry } of safeEntries) { @@ -346,6 +397,9 @@ export async function decompressArchiveBufferToWorkspaceFiles( validatedTotal += result.size } + const resolvedRootFolderSegments = + safeEntries.length > 0 && prepareRootFolder ? await prepareRootFolder() : rootFolderSegments + // 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 *and* @@ -356,6 +410,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( const folderIdCache = new Map() /** Only folders this call inserted, in creation order — never a reused one. */ const createdFolderIds: string[] = [] + const createdFiles: WorkspaceFileRecord[] = [] const extracted: UserFile[] = [] let totalBytes = 0 try { @@ -366,7 +421,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( const entryBuffer = result.buffer as Buffer const leafName = segments[segments.length - 1] - const folderSegments = [...rootFolderSegments, ...segments.slice(0, -1)] + const folderSegments = [...resolvedRootFolderSegments, ...segments.slice(0, -1)] const folderKey = folderSegments.join('/') let folderId = folderIdCache.get(folderKey) if (folderId === undefined) { @@ -396,9 +451,11 @@ export async function decompressArchiveBufferToWorkspaceFiles( // roll back an otherwise valid extraction. exactName: false, secretProvenance: extractedSecretProvenance, + notifyWorkspaceChange: false, }, }) ).file + createdFiles.push(uploaded) extracted.push({ id: uploaded.id, name: uploaded.name, @@ -410,28 +467,41 @@ export async function decompressArchiveBufferToWorkspaceFiles( }) } } catch (error) { - for (const file of extracted) { + for (const file of createdFiles) { try { - await deleteWorkspaceFileOperation.execute({ - principal, - input: { fileId: file.id, assertedWorkspaceId: workspaceId }, + await purgeCreatedWorkspaceFile({ + workspaceId, + fileId: file.id, + key: file.key, + expectedName: file.name, + expectedFolderId: file.folderId ?? null, + expectedUpdatedAt: file.updatedAt, + }) + } catch (cleanupError) { + // Best-effort cleanup never masks the extraction failure. + logger.error('Failed to purge extracted file during rollback', { + workspaceId, + fileId: file.id, + key: file.key, + cleanupError, }) - } catch { - // Best-effort: a file whose cleanup fails is still soft-deletable by hand; - // the original error is what the caller needs to see. } } // Deepest-first (creation order records parents before children), so a parent is // never removed out from under a child that is still being cleaned up. for (let index = createdFolderIds.length - 1; index >= 0; index--) { try { - await deleteWorkspaceFileFolderOperation.execute({ - principal, - input: { workspaceId, folderId: createdFolderIds[index], recursive: true }, + await archiveWorkspaceFileFolderIfEmpty({ + workspaceId, + folderId: createdFolderIds[index], + }) + } catch (cleanupError) { + // Best-effort cleanup never masks the extraction failure. + logger.warn('Failed to archive created folder during rollback', { + workspaceId, + folderId: createdFolderIds[index], + cleanupError, }) - } catch { - // Best-effort: a folder whose cleanup fails is still deletable by hand; - // the original error is what the caller needs to see. } } throw error diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts index 578a9bb2d71..a4de1ab884f 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts @@ -2,11 +2,28 @@ * @vitest-environment node */ -import { describe, expect, it } from 'vitest' +import { dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockAcquireFolderMutationLock, mockDeduplicateFolderName } = vi.hoisted(() => ({ + mockAcquireFolderMutationLock: vi.fn(), + mockDeduplicateFolderName: vi.fn(), +})) + +vi.mock('@/lib/folders/locks', () => ({ + acquireFolderMutationLock: mockAcquireFolderMutationLock, +})) + +vi.mock('@/lib/folders/naming', () => ({ + deduplicateFolderName: mockDeduplicateFolderName, +})) + import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import { MAX_FOLDER_PATH_SEGMENTS } from '@/lib/folders/paths' import { + archiveWorkspaceFileFolderIfEmpty, buildWorkspaceFileFolderPathMap, + createWorkspaceFileFolder, ensureWorkspaceFileFolderPath, normalizeWorkspaceFileItemName, WorkspaceFileFolderConflictError, @@ -14,6 +31,52 @@ import { WorkspaceFileMoveConflictError, } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' +describe('createWorkspaceFileFolder', () => { + beforeEach(() => { + resetDbChainMock() + mockAcquireFolderMutationLock.mockReset() + mockDeduplicateFolderName.mockReset() + }) + + it('uses the shared numeric suffix allocator when exact naming is disabled', async () => { + const now = new Date('2026-08-17T12:00:00.000Z') + const inserted = { + id: 'folder-archive-3', + resourceType: 'file', + workspaceId: 'workspace-1', + userId: 'user-1', + name: 'Archive (3)', + parentId: null, + sortOrder: 0, + deletedAt: null, + createdAt: now, + updatedAt: now, + } + mockDeduplicateFolderName.mockResolvedValueOnce('Archive (3)') + dbChainMockFns.returning.mockResolvedValueOnce([inserted]) + + await expect( + createWorkspaceFileFolder({ + workspaceId: 'workspace-1', + userId: 'user-1', + name: 'Archive', + exactName: false, + }) + ).resolves.toMatchObject({ name: 'Archive (3)' }) + + expect(mockDeduplicateFolderName).toHaveBeenCalledWith( + expect.anything(), + 'workspace-1', + null, + 'Archive', + 'file' + ) + expect(dbChainMockFns.values).toHaveBeenCalledWith( + expect.objectContaining({ name: 'Archive (3)' }) + ) + }) +}) + describe('workspace file folder paths', () => { it('builds nested paths from parent relationships', () => { const paths = buildWorkspaceFileFolderPathMap([ @@ -98,3 +161,62 @@ describe('workspace file folder failure classification', () => { expect(asOrchestrationError(wrapped)?.code).toBe('conflict') }) }) + +describe('archiveWorkspaceFileFolderIfEmpty', () => { + beforeEach(() => { + resetDbChainMock() + mockAcquireFolderMutationLock.mockReset() + }) + + it('archives an empty folder under the folder mutation lock', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([{ id: 'folder-1' }]) + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'folder-1' }]) + + await expect( + archiveWorkspaceFileFolderIfEmpty({ workspaceId: 'workspace-1', folderId: 'folder-1' }) + ).resolves.toBe(true) + + expect(mockAcquireFolderMutationLock).toHaveBeenCalledWith( + expect.anything(), + 'workspace-1', + 'file' + ) + }) + + it('returns false without archiving when the folder is missing or already archived', async () => { + dbChainMockFns.limit.mockResolvedValueOnce([]) + + await expect( + archiveWorkspaceFileFolderIfEmpty({ workspaceId: 'workspace-1', folderId: 'folder-1' }) + ).resolves.toBe(false) + + expect(dbChainMockFns.returning).not.toHaveBeenCalled() + }) + + it('refuses to archive a folder that still holds an active file', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([{ id: 'folder-1' }]) + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ id: 'file-1' }]) + + await expect( + archiveWorkspaceFileFolderIfEmpty({ workspaceId: 'workspace-1', folderId: 'folder-1' }) + ).rejects.toMatchObject({ code: 'conflict', message: 'Folder is not empty' }) + + expect(dbChainMockFns.returning).not.toHaveBeenCalled() + }) + + it('refuses to archive a folder with an active child folder', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([{ id: 'folder-1' }]) + .mockResolvedValueOnce([{ id: 'child-1' }]) + .mockResolvedValueOnce([]) + + await expect( + archiveWorkspaceFileFolderIfEmpty({ workspaceId: 'workspace-1', folderId: 'folder-1' }) + ).rejects.toMatchObject({ code: 'conflict' }) + }) +}) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts index 34d8be56d6b..583751820fd 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts @@ -493,8 +493,9 @@ export async function createWorkspaceFileFolder(params: { name: string parentId?: string | null sortOrder?: number + exactName?: boolean }): Promise { - const name = normalizeWorkspaceFileItemName(params.name, 'Folder') + const requestedName = normalizeWorkspaceFileItemName(params.name, 'Folder') const folder = await db.transaction(async (tx) => { await acquireWorkspaceFileFolderMutationLock(tx, params.workspaceId) @@ -519,22 +520,35 @@ export async function createWorkspaceFileFolder(params: { } } - const existingFolders = await tx - .select({ id: folderTable.id }) - .from(folderTable) - .where( - and( - eq(folderTable.workspaceId, params.workspaceId), - isFileFolder, - eq(folderTable.name, name), - folderParentCondition(parentId), - isNull(folderTable.deletedAt) + const name = + params.exactName === false + ? await deduplicateFolderName( + tx, + params.workspaceId, + parentId, + requestedName, + FILE_FOLDER_RESOURCE_TYPE + ) + : requestedName + + if (params.exactName !== false) { + const existingFolders = await tx + .select({ id: folderTable.id }) + .from(folderTable) + .where( + and( + eq(folderTable.workspaceId, params.workspaceId), + isFileFolder, + eq(folderTable.name, name), + folderParentCondition(parentId), + isNull(folderTable.deletedAt) + ) ) - ) - .limit(1) + .limit(1) - if (existingFolders.length > 0) { - throw new WorkspaceFileFolderConflictError(name) + if (existingFolders.length > 0) { + throw new WorkspaceFileFolderConflictError(name) + } } const [sortOrderResult] = await tx @@ -1570,3 +1584,67 @@ export async function deleteWorkspaceFileFolderByPath(params: { return { folders: archivedFolders.length, files: archivedFiles.length } }) } + +/** Archives an exact folder only while it has no active files or child folders. */ +export async function archiveWorkspaceFileFolderIfEmpty(params: { + workspaceId: string + folderId: string +}): Promise { + return db.transaction(async (tx) => { + await acquireWorkspaceFileFolderMutationLock(tx, params.workspaceId) + + const [folder] = await tx + .select({ id: folderTable.id }) + .from(folderTable) + .where( + and( + eq(folderTable.id, params.folderId), + eq(folderTable.workspaceId, params.workspaceId), + isFileFolder, + isNull(folderTable.deletedAt) + ) + ) + .limit(1) + if (!folder) return false + + const [childFolder] = await tx + .select({ id: folderTable.id }) + .from(folderTable) + .where( + and( + eq(folderTable.parentId, params.folderId), + eq(folderTable.workspaceId, params.workspaceId), + isFileFolder, + isNull(folderTable.deletedAt) + ) + ) + .limit(1) + const [file] = await tx + .select({ id: workspaceFiles.id }) + .from(workspaceFiles) + .where( + and( + eq(workspaceFiles.folderId, params.folderId), + eq(workspaceFiles.workspaceId, params.workspaceId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) + ) + .limit(1) + if (childFolder || file) throw new OrchestrationError('conflict', 'Folder is not empty') + + const [archived] = await tx + .update(folderTable) + .set({ deletedAt: new Date(), updatedAt: new Date() }) + .where( + and( + eq(folderTable.id, params.folderId), + eq(folderTable.workspaceId, params.workspaceId), + isFileFolder, + isNull(folderTable.deletedAt) + ) + ) + .returning({ id: folderTable.id }) + return Boolean(archived) + }) +} 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 daac8d062af..484523923b9 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -390,6 +390,7 @@ export async function uploadWorkspaceFile( folderPath?: string exactName?: boolean secretProvenance?: WorkspaceFileSecretProvenance + notifyWorkspaceChange?: boolean } ): Promise { logger.info(`Uploading workspace file: ${fileName} for workspace ${workspaceId}`) @@ -512,9 +513,9 @@ export async function uploadWorkspaceFile( `Successfully uploaded workspace file: ${uniqueName} with key: ${uploadResult.key}` ) - // Fan out the live-tree signal for this server-buffered path. Upload-session - // finalization sends its own notification after registering metadata. - await notifyWorkspaceFilesChanged(workspaceId) + if (options?.notifyWorkspaceChange !== false) { + await notifyWorkspaceFilesChanged(workspaceId) + } return mapUploadedWorkspaceFileRecord(finalized.inserted, workspaceId, folderPath) } catch (error) { @@ -2048,6 +2049,59 @@ export async function deleteWorkspaceFile(workspaceId: string, fileId: string): } } +/** + * Permanently removes a file created by an in-flight archive extraction only while + * its name, folder, and update timestamp still match the creation result. This is + * rollback-only: ordinary user deletion remains recoverable through {@link deleteWorkspaceFile}. + */ +export async function purgeCreatedWorkspaceFile(params: { + workspaceId: string + fileId: string + key: string + expectedName: string + expectedFolderId: string | null + expectedUpdatedAt: Date +}): Promise { + const storageBillingContext = await resolveStorageBillingContext(params.workspaceId) + const expectedFolder = + params.expectedFolderId === null + ? isNull(workspaceFiles.folderId) + : eq(workspaceFiles.folderId, params.expectedFolderId) + const purgedKey = await db.transaction(async (tx) => { + const [deleted] = await tx + .delete(workspaceFiles) + .where( + and( + eq(workspaceFiles.id, params.fileId), + eq(workspaceFiles.workspaceId, params.workspaceId), + eq(workspaceFiles.key, params.key), + eq(workspaceFiles.originalName, params.expectedName), + expectedFolder, + eq(workspaceFiles.updatedAt, params.expectedUpdatedAt), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) + ) + .returning({ + key: workspaceFiles.key, + size: workspaceFiles.size, + sizeBytes: workspaceFiles.sizeBytes, + }) + if (!deleted) return null + + await decrementStorageUsageForBillingContextInTx( + tx, + storageBillingContext, + workspaceFileSize(deleted) + ) + return deleted.key + }) + + if (!purgedKey) return false + await deleteFile({ key: purgedKey, context: 'workspace' }) + return true +} + /** * Restore a soft-deleted workspace file. */ diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts index c1ea0f4fb93..e50c9fcdb68 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts @@ -1,8 +1,10 @@ /** * @vitest-environment node */ +import { workspaceFiles } from '@sim/db/schema' import { dbChainMockFns, resetDbChainMock } from '@sim/testing' import { describeError } from '@sim/utils/errors' +import { eq } from 'drizzle-orm' import { beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -103,6 +105,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({ import { ContentVersionConflictError, deleteWorkspaceFile, + purgeCreatedWorkspaceFile, registerUploadedWorkspaceFile, restoreWorkspaceFile, updateWorkspaceFileContent, @@ -262,6 +265,53 @@ describe('workspace file metadata and storage accounting', () => { ) }) + it('purges exact archive-created metadata, bytes, and accounting during rollback', async () => { + const extractedRow = { ...FILE_ROW, folderId: 'folder-archive' } + dbChainMockFns.returning.mockResolvedValueOnce([extractedRow]) + + await expect( + purgeCreatedWorkspaceFile({ + workspaceId: FILE_ROW.workspaceId, + fileId: FILE_ROW.id, + key: FILE_ROW.key, + expectedName: FILE_ROW.originalName, + expectedFolderId: extractedRow.folderId, + expectedUpdatedAt: FILE_ROW.updatedAt, + }) + ).resolves.toBe(true) + + expect(eq).toHaveBeenCalledWith(workspaceFiles.originalName, FILE_ROW.originalName) + expect(eq).toHaveBeenCalledWith(workspaceFiles.folderId, extractedRow.folderId) + expect(eq).toHaveBeenCalledWith(workspaceFiles.updatedAt, FILE_ROW.updatedAt) + expect(mockDecrementStorageUsageForBillingContextInTx).toHaveBeenCalledWith( + expect.any(Object), + STORAGE_CONTEXT, + FILE_ROW.size + ) + expect(mockDeleteFile).toHaveBeenCalledWith({ key: FILE_ROW.key, context: 'workspace' }) + expect(mockDecrementStorageUsageForBillingContextInTx.mock.invocationCallOrder[0]).toBeLessThan( + mockDeleteFile.mock.invocationCallOrder[0] + ) + }) + + it('leaves an extracted file untouched when its creation identity no longer matches', async () => { + dbChainMockFns.returning.mockResolvedValueOnce([]) + + await expect( + purgeCreatedWorkspaceFile({ + workspaceId: FILE_ROW.workspaceId, + fileId: FILE_ROW.id, + key: FILE_ROW.key, + expectedName: FILE_ROW.originalName, + expectedFolderId: 'folder-archive', + expectedUpdatedAt: FILE_ROW.updatedAt, + }) + ).resolves.toBe(false) + + expect(mockDecrementStorageUsageForBillingContextInTx).not.toHaveBeenCalled() + expect(mockDeleteFile).not.toHaveBeenCalled() + }) + it('preserves the driver cause so the SQLSTATE survives the upload wrapper', async () => { const driver = Object.assign( new Error('cannot execute SELECT FOR UPDATE in a read-only transaction'), @@ -302,6 +352,21 @@ describe('workspace file metadata and storage accounting', () => { expect(mockReplaceWorkspaceFileSecretProvenanceInTx).not.toHaveBeenCalled() }) + it('allows extraction to batch the workspace notification', async () => { + dbChainMockFns.returning.mockResolvedValueOnce([FILE_ROW]) + + await uploadWorkspaceFile( + FILE_ROW.workspaceId, + FILE_ROW.userId, + Buffer.from('hello'), + FILE_ROW.originalName, + FILE_ROW.contentType, + { notifyWorkspaceChange: false } + ) + + expect(mockNotifyWorkspaceFilesChanged).not.toHaveBeenCalled() + }) + it('persists explicitly supplied workspace upload provenance', async () => { dbChainMockFns.returning.mockResolvedValueOnce([FILE_ROW]) diff --git a/apps/sim/lib/uploads/upload-session/service.test.ts b/apps/sim/lib/uploads/upload-session/service.test.ts index fdbc1a72eb3..0bebbeb2d80 100644 --- a/apps/sim/lib/uploads/upload-session/service.test.ts +++ b/apps/sim/lib/uploads/upload-session/service.test.ts @@ -17,6 +17,7 @@ const { mockInitiateMultipart, mockListMultipartParts, mockResolveBillingContext, + mockUploadStorageProvider, } = vi.hoisted(() => ({ mockAbortProviderUpload: vi.fn(), mockCheckStorageQuota: vi.fn(), @@ -27,6 +28,7 @@ const { mockInitiateMultipart: vi.fn(), mockListMultipartParts: vi.fn(), mockResolveBillingContext: vi.fn(), + mockUploadStorageProvider: vi.fn(() => 's3' as const), })) vi.mock('@/lib/billing/storage', () => ({ @@ -63,7 +65,7 @@ vi.mock('@/lib/uploads/upload-session/provider', () => ({ headProviderObject: mockHeadObject, initiateMultipartProviderUpload: mockInitiateMultipart, listMultipartProviderParts: mockListMultipartParts, - uploadStorageProvider: vi.fn(() => 's3'), + uploadStorageProvider: mockUploadStorageProvider, })) import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -109,6 +111,7 @@ describe('upload sessions', () => { resetDbChainMock() mockResolveBillingContext.mockResolvedValue({ workspaceId: WORKSPACE_ID }) mockCheckStorageQuota.mockResolvedValue({ allowed: true }) + mockUploadStorageProvider.mockReturnValue('s3') mockCreatePutTransfer.mockResolvedValue({ method: 'put', url: 'https://storage.example/upload', @@ -624,6 +627,30 @@ describe('upload sessions', () => { expect(mockCreatePutTransfer).not.toHaveBeenCalled() }) + it('uses proxy-safe multipart parts for large local uploads', async () => { + const fileSize = UPLOAD_SESSION_PART_SIZE + 1 + mockUploadStorageProvider.mockReturnValue('local') + mockInitiateMultipart.mockResolvedValueOnce({ provider: 'local', providerUploadId: null }) + dbChainMockFns.returning.mockResolvedValueOnce([ + uploadRow({ + fileSize, + method: 'multipart', + storageProvider: 'local', + partSize: UPLOAD_SESSION_PART_SIZE, + partCount: 2, + }), + ]) + + const created = await createWorkspaceUpload(fileSize) + + expect(created.transfer).toEqual({ + method: 'multipart', + partSize: UPLOAD_SESSION_PART_SIZE, + partCount: 2, + }) + expect(mockCreatePutTransfer).not.toHaveBeenCalled() + }) + it('preserves multipart request bounds before provider signing', async () => { const multipart = sessionRecord({ method: 'multipart', diff --git a/apps/sim/lib/uploads/upload-session/service.ts b/apps/sim/lib/uploads/upload-session/service.ts index 98f972f4250..5ce9847db66 100644 --- a/apps/sim/lib/uploads/upload-session/service.ts +++ b/apps/sim/lib/uploads/upload-session/service.ts @@ -207,11 +207,6 @@ export async function createUploadSession( }) } const { storageContext, finalKey } = resolveUploadStorage(params, id) - const method: UploadTransferMethod = - params.fileSize <= UPLOAD_SESSION_PUT_MAX_BYTES ? 'put' : 'multipart' - const partSize = method === 'multipart' ? UPLOAD_SESSION_PART_SIZE : null - const partCount = - method === 'multipart' ? Math.ceil(params.fileSize / UPLOAD_SESSION_PART_SIZE) : null if (requiresStorageQuota(params.purpose)) { if (!workspaceId) throw new Error(`${params.purpose} upload is missing workspaceId`) @@ -223,6 +218,11 @@ export async function createUploadSession( } const provider = uploadStorageProvider() + const putMaxBytes = provider === 'local' ? UPLOAD_SESSION_PART_SIZE : UPLOAD_SESSION_PUT_MAX_BYTES + const method: UploadTransferMethod = params.fileSize <= putMaxBytes ? 'put' : 'multipart' + const partSize = method === 'multipart' ? UPLOAD_SESSION_PART_SIZE : null + const partCount = + method === 'multipart' ? Math.ceil(params.fileSize / UPLOAD_SESSION_PART_SIZE) : null if (provider === 'local') await maybeCleanupLocalUploadArtifacts() const objectMetadata = uploadSessionObjectMetadata({ id, diff --git a/apps/sim/lib/workspace-files/api/internal-error-policies.test.ts b/apps/sim/lib/workspace-files/api/internal-error-policies.test.ts index e8794b28bb5..383aac48911 100644 --- a/apps/sim/lib/workspace-files/api/internal-error-policies.test.ts +++ b/apps/sim/lib/workspace-files/api/internal-error-policies.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it } from 'vitest' import { StorageLimitExceededError } from '@/lib/billing/storage' import { OrchestrationError } from '@/lib/core/orchestration/types' +import { ArchiveError } from '@/lib/uploads/archive' import { internalFileErrorPolicies } from '@/lib/workspace-files/api/internal-error-policies' import { CompiledCheckTooLargeError, @@ -45,4 +46,25 @@ describe('internal file error policies', () => { headers: undefined, }) }) + + it('maps archive extraction failures onto caller-safe statuses', () => { + expect( + internalFileErrorPolicies.extractArchive.project( + new ArchiveError('invalid', 'Not a valid .zip archive.') + ) + ).toEqual({ + status: 400, + body: { error: 'Not a valid .zip archive.' }, + headers: undefined, + }) + expect( + internalFileErrorPolicies.extractArchive.project( + new ArchiveError('too_many_entries', 'Archive has 1001 files; the maximum is 1000.') + ) + ).toEqual({ + status: 413, + body: { error: 'Archive has 1001 files; the maximum is 1000.' }, + headers: undefined, + }) + }) }) diff --git a/apps/sim/lib/workspace-files/api/internal-error-policies.ts b/apps/sim/lib/workspace-files/api/internal-error-policies.ts index a54b65bc492..40332c243dc 100644 --- a/apps/sim/lib/workspace-files/api/internal-error-policies.ts +++ b/apps/sim/lib/workspace-files/api/internal-error-policies.ts @@ -8,6 +8,7 @@ import { } from '@/lib/api/server/routes' import { StorageLimitExceededError } from '@/lib/billing/storage' import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' +import { ArchiveError } from '@/lib/uploads/archive' import { CompiledCheckTooLargeError, CompiledCheckUnsupportedError, @@ -81,6 +82,16 @@ const inline: InternalErrorPolicy = { const FILE_NOT_FOUND_MESSAGE = 'File not found' +const concealResourceAuthorization = createInternalResourceConcealmentPolicy({ + base: internalOrchestrationErrorPolicy, + notFoundMessage: FILE_NOT_FOUND_MESSAGE, +}) + +const extractArchive = extendInternalErrorPolicy(concealResourceAuthorization, (error) => { + if (!(error instanceof ArchiveError)) return null + return internalErrorResponse(error.reason === 'invalid' ? 400 : 413, { error: error.message }) +}) + export const internalFileErrorPolicies = { default: internalOrchestrationErrorPolicy, content, @@ -88,10 +99,7 @@ export const internalFileErrorPolicies = { * Single-file internal routes reach the same use cases as the concealing v2 * file routes, so they withhold the same cross-tenant existence signal. */ - concealResourceAuthorization: createInternalResourceConcealmentPolicy({ - base: internalOrchestrationErrorPolicy, - notFoundMessage: FILE_NOT_FOUND_MESSAGE, - }), + concealResourceAuthorization, concealContentAuthorization: createInternalResourceConcealmentPolicy({ base: content, notFoundMessage: FILE_NOT_FOUND_MESSAGE, @@ -100,5 +108,6 @@ export const internalFileErrorPolicies = { compiledCheck, downloadUrl, downloadArchive, + extractArchive, inline, } as const diff --git a/apps/sim/lib/workspace-files/application/create-workspace-file.ts b/apps/sim/lib/workspace-files/application/create-workspace-file.ts index edcbc830a91..65d2e41a19e 100644 --- a/apps/sim/lib/workspace-files/application/create-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/create-workspace-file.ts @@ -36,6 +36,7 @@ export interface CreateWorkspaceFileResult { export interface CreateWorkspaceFileBufferInput extends Omit { content: Buffer + notifyWorkspaceChange?: boolean } async function resolveCreateWorkspaceFileContext(workspaceId: string) { @@ -51,7 +52,9 @@ async function createAuthorizedWorkspaceFile({ workspace, }: { principal: Principal - input: Omit + input: Omit & { + notifyWorkspaceChange?: boolean + } content: Buffer workspace: Awaited> }): Promise { @@ -71,6 +74,7 @@ async function createAuthorizedWorkspaceFile({ folderPath: input.folderPath, exactName: input.exactName, secretProvenance: input.secretProvenance ?? EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE, + notifyWorkspaceChange: input.notifyWorkspaceChange, } ) } catch (error) { diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts new file mode 100644 index 00000000000..aba7e7f53a1 --- /dev/null +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts @@ -0,0 +1,301 @@ +/** + * @vitest-environment node + */ +import { Buffer } from 'buffer' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { OrchestrationError } from '@/lib/core/orchestration/types' + +const mocks = vi.hoisted(() => ({ + archiveFolderIfEmpty: vi.fn(), + atomicallyClaim: vi.fn(), + createFolder: vi.fn(), + decompress: vi.fn(), + fetchBuffer: vi.fn(), + getFile: vi.fn(), + getSecretProvenance: vi.fn(), + loadContext: vi.fn(), + notify: vi.fn(), + releaseLease: vi.fn(), + resolvePermission: vi.fn(), +})) + +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: () => true, + resolveEffectiveWorkspacePermission: mocks.resolvePermission, +})) + +vi.mock('@/lib/realtime/notify', () => ({ notifyWorkspaceFilesChanged: mocks.notify })) + +vi.mock('@/lib/core/idempotency/service', () => ({ + IdempotencyService: class MockIdempotencyService { + atomicallyClaim(...args: unknown[]) { + return mocks.atomicallyClaim(...args) + } + + release(...args: unknown[]) { + return mocks.releaseLease(...args) + } + }, +})) + +vi.mock('@/lib/uploads/archive', () => ({ + decompressArchiveBufferToWorkspaceFiles: mocks.decompress, + MAX_ARCHIVE_BYTES: 100 * 1024 * 1024, +})) + +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-folder-manager', () => ({ + archiveWorkspaceFileFolderIfEmpty: mocks.archiveFolderIfEmpty, + createWorkspaceFileFolder: mocks.createFolder, +})) + +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + fetchWorkspaceFileBuffer: mocks.fetchBuffer, + getWorkspaceFile: mocks.getFile, + loadActiveWorkspaceFileContext: mocks.loadContext, +})) + +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({ + getBoundWorkspaceFileSecretProvenance: mocks.getSecretProvenance, +})) + +import { extractWorkspaceFile } from '@/lib/workspace-files/application/extract-workspace-file' + +const principal = { kind: 'session', userId: 'user-1', sessionId: 'session-1' } as const +const context = { + fileId: 'file-1', + workspaceId: 'workspace-1', + workspaceOrganizationId: null, + allowPersonalApiKeys: true, + billedAccountUserId: 'billing-owner', +} +const file = { + id: 'file-1', + workspaceId: 'workspace-1', + name: 'bundle.zip', + key: 'workspace/workspace-1/bundle.zip', + size: 256, + folderPath: 'Projects/Imports', + storageContext: 'workspace' as const, + folderId: 'folder-imports', +} + +describe('extractWorkspaceFile', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.loadContext.mockResolvedValue(context) + mocks.resolvePermission.mockResolvedValue('write') + mocks.getFile.mockResolvedValue(file) + mocks.createFolder.mockResolvedValue({ + id: 'folder-bundle', + name: 'bundle', + path: 'Projects/Imports/bundle', + }) + mocks.archiveFolderIfEmpty.mockResolvedValue(true) + mocks.atomicallyClaim.mockResolvedValue({ + claimed: true, + normalizedKey: 'workspace-file:extract:workspace-1:file-1', + storageMethod: 'database', + claimToken: 'claim-1', + }) + mocks.fetchBuffer.mockResolvedValue(Buffer.from('zip')) + mocks.getSecretProvenance.mockResolvedValue({ status: 'exact', entries: [] }) + mocks.decompress.mockImplementation(async (_content, options) => { + await options.prepareRootFolder() + return { + extracted: [{ id: 'extracted-1' }, { id: 'extracted-2' }], + skipped: 1, + skippedUnsafePaths: [], + } + }) + mocks.notify.mockResolvedValue(undefined) + mocks.releaseLease.mockResolvedValue(undefined) + }) + + it('extracts into a same-name folder beside the archive', async () => { + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).resolves.toEqual({ folderName: 'bundle', extractedCount: 2, skippedCount: 1 }) + + expect(mocks.createFolder).toHaveBeenCalledWith({ + workspaceId: 'workspace-1', + userId: 'user-1', + name: 'bundle', + parentId: 'folder-imports', + exactName: false, + }) + expect(mocks.fetchBuffer).toHaveBeenCalledWith(file, { maxBytes: 100 * 1024 * 1024 }) + expect(mocks.decompress).toHaveBeenCalledWith(Buffer.from('zip'), { + workspaceId: 'workspace-1', + principal, + prepareRootFolder: expect.any(Function), + materializedRootFolderCount: 1, + maxMaterializedItems: 5000, + skipNoiseEntries: true, + secretProvenance: { status: 'exact', entries: [] }, + }) + expect(mocks.atomicallyClaim).toHaveBeenCalledWith('extract', 'workspace-1:file-1') + expect(mocks.releaseLease).toHaveBeenCalledWith( + 'workspace-file:extract:workspace-1:file-1', + 'database', + 'claim-1' + ) + expect(mocks.notify).toHaveBeenCalledWith('workspace-1') + }) + + it('rejects non-zip files before reading storage', async () => { + mocks.getFile.mockResolvedValue({ ...file, name: 'bundle.txt' }) + + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).rejects.toMatchObject({ code: 'validation', message: 'Only .zip files can be unzipped' }) + + expect(mocks.fetchBuffer).not.toHaveBeenCalled() + expect(mocks.decompress).not.toHaveBeenCalled() + }) + + it('rejects a second extraction while the same archive is already being extracted', async () => { + mocks.atomicallyClaim + .mockResolvedValueOnce({ + claimed: true, + normalizedKey: 'workspace-file:extract:workspace-1:file-1', + storageMethod: 'database', + claimToken: 'claim-1', + }) + .mockResolvedValueOnce({ + claimed: false, + normalizedKey: 'workspace-file:extract:workspace-1:file-1', + storageMethod: 'database', + existingResult: { status: 'in-progress' }, + }) + let releaseFetch: ((value: Buffer) => void) | undefined + mocks.fetchBuffer.mockImplementationOnce( + () => + new Promise((resolve) => { + releaseFetch = resolve + }) + ) + + const firstExtraction = extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + await vi.waitFor(() => expect(mocks.fetchBuffer).toHaveBeenCalledOnce()) + + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).rejects.toMatchObject({ + code: 'conflict', + message: 'This archive is already being unzipped', + }) + + releaseFetch?.(Buffer.from('zip')) + await expect(firstExtraction).resolves.toMatchObject({ extractedCount: 2 }) + expect(mocks.atomicallyClaim).toHaveBeenCalledTimes(2) + expect(mocks.releaseLease).toHaveBeenCalledOnce() + }) + + it('rejects extraction when another server owns the archive lease', async () => { + mocks.atomicallyClaim.mockResolvedValueOnce({ + claimed: false, + normalizedKey: 'workspace-file:extract:workspace-1:file-1', + storageMethod: 'database', + existingResult: { status: 'in-progress' }, + }) + + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).rejects.toMatchObject({ + code: 'conflict', + message: 'This archive is already being unzipped', + }) + + expect(mocks.getFile).not.toHaveBeenCalled() + expect(mocks.releaseLease).not.toHaveBeenCalled() + }) + + it('uses a suffixed destination instead of merging into a stranded folder', async () => { + mocks.createFolder.mockResolvedValueOnce({ + id: 'folder-bundle-3', + name: 'bundle (3)', + path: 'Projects/Imports/bundle (3)', + }) + + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).resolves.toEqual({ folderName: 'bundle (3)', extractedCount: 2, skippedCount: 1 }) + + expect(mocks.createFolder).toHaveBeenCalledWith( + expect.objectContaining({ name: 'bundle', exactName: false }) + ) + }) + + it('only removes the destination folder when it is still empty after extraction fails', async () => { + mocks.decompress.mockImplementationOnce(async (_content, options) => { + await options.prepareRootFolder() + throw new Error('invalid archive') + }) + + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).rejects.toThrow('invalid archive') + + expect(mocks.archiveFolderIfEmpty).toHaveBeenCalledWith({ + workspaceId: 'workspace-1', + folderId: 'folder-bundle', + }) + expect(mocks.notify).toHaveBeenCalledOnce() + expect(mocks.notify).toHaveBeenCalledWith('workspace-1') + }) + + it('leaves a destination folder that gained collaborators content during rollback', async () => { + mocks.decompress.mockImplementationOnce(async (_content, options) => { + await options.prepareRootFolder() + throw new Error('storage quota exceeded') + }) + mocks.archiveFolderIfEmpty.mockRejectedValueOnce( + new OrchestrationError('conflict', 'Folder is not empty') + ) + + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).rejects.toThrow('storage quota exceeded') + + expect(mocks.archiveFolderIfEmpty).toHaveBeenCalledWith( + expect.objectContaining({ folderId: 'folder-bundle' }) + ) + expect(mocks.notify).toHaveBeenCalledOnce() + }) + + it('rejects non-session principals before loading the file', async () => { + await expect( + extractWorkspaceFile.execute({ + principal: { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-1' }, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).rejects.toMatchObject({ code: 'forbidden' }) + + expect(mocks.loadContext).not.toHaveBeenCalled() + expect(mocks.fetchBuffer).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts new file mode 100644 index 00000000000..f7c02bcc654 --- /dev/null +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts @@ -0,0 +1,201 @@ +import { AuditAction, AuditResourceType } from '@sim/audit' +import { resolvePrincipalAttribution } from '@sim/auth/principal' +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' +import type { AuthorizedWorkspaceUseCaseContext } from '@/lib/core/application' +import { IdempotencyService } from '@/lib/core/idempotency/service' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { notifyWorkspaceFilesChanged } from '@/lib/realtime/notify' +import { decompressArchiveBufferToWorkspaceFiles, MAX_ARCHIVE_BYTES } from '@/lib/uploads/archive' +import { + archiveWorkspaceFileFolderIfEmpty, + createWorkspaceFileFolder, +} from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' +import { + type ActiveWorkspaceFileContext, + fetchWorkspaceFileBuffer, + getWorkspaceFile, +} from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import { getBoundWorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { isArchiveFileName } from '@/lib/uploads/utils/file-utils' +import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/application/authorized-workspace-file-use-case' +import { fileOperations } from '@/lib/workspace-files/application/operations' +import { resolveActiveWorkspaceFileContext } from '@/lib/workspace-files/application/workspace-file-context' +import { parseWorkspaceFileFolderDisplayPath } from '@/lib/workspace-files/folder-display-path' +import { MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS } from '@/lib/workspace-files/limits' + +const logger = createLogger('ExtractWorkspaceFile') +const EXTRACTION_LEASE_TTL_SECONDS = 6 * 60 +const extractionLeases = new IdempotencyService({ + namespace: 'workspace-file', + ttlSeconds: EXTRACTION_LEASE_TTL_SECONDS, + inProgressTtlSeconds: EXTRACTION_LEASE_TTL_SECONDS, + retryFailures: true, + storeResultBody: false, + forceStorage: 'database', +}) + +export interface ExtractWorkspaceFileInput { + fileId: string + assertedWorkspaceId?: string +} + +export interface ExtractWorkspaceFileResult { + folderName: string + extractedCount: number + skippedCount: number +} + +type ExtractWorkspaceFileUseCaseContext = AuthorizedWorkspaceUseCaseContext< + typeof fileOperations.extractArchive, + ExtractWorkspaceFileInput, + ActiveWorkspaceFileContext +> + +function archiveFolderName(fileName: string): string { + const stripped = fileName + .replace(/\.zip$/i, '') + .normalize('NFC') + .replace(/[\x00-\x1f\x7f]/g, '') + .replace(/[/\\]/g, '-') + .trim() + return stripped && stripped !== '.' && stripped !== '..' ? stripped : 'archive' +} + +async function withExtractionLease( + workspaceId: string, + fileId: string, + extract: () => Promise +): Promise { + const claim = await extractionLeases.atomicallyClaim('extract', `${workspaceId}:${fileId}`) + if (!claim.claimed) { + throw new OrchestrationError('conflict', 'This archive is already being unzipped') + } + if (!claim.claimToken) throw new Error('Archive extraction lease is missing its fencing token') + + try { + return await extract() + } finally { + await extractionLeases + .release(claim.normalizedKey, claim.storageMethod, claim.claimToken) + .catch((error) => { + logger.warn('Failed to release archive extraction lease; TTL will expire it', { + workspaceId, + fileId, + error: getErrorMessage(error), + }) + }) + } +} + +async function executeExtractWorkspaceFile( + useCaseContext: ExtractWorkspaceFileUseCaseContext +): Promise { + const { principal, context } = useCaseContext + return withExtractionLease(context.workspaceId, context.fileId, () => + extractWorkspaceFileContents(useCaseContext) + ) +} + +async function extractWorkspaceFileContents({ + principal, + context, +}: ExtractWorkspaceFileUseCaseContext): Promise { + const file = await getWorkspaceFile(context.workspaceId, context.fileId, { throwOnError: true }) + if (!file) throw new OrchestrationError('not_found', 'File not found') + if (!isArchiveFileName(file.name)) { + throw new OrchestrationError('validation', 'Only .zip files can be unzipped') + } + if (file.size > MAX_ARCHIVE_BYTES) { + throw new OrchestrationError( + 'payload_too_large', + `Archive exceeds the ${MAX_ARCHIVE_BYTES / 1024 / 1024} MB unzip limit` + ) + } + + const folderName = archiveFolderName(file.name) + const [content, secretProvenance] = await Promise.all([ + fetchWorkspaceFileBuffer(file, { maxBytes: MAX_ARCHIVE_BYTES }), + getBoundWorkspaceFileSecretProvenance(context.workspaceId, { + fileId: file.id, + key: file.key, + context: file.storageContext ?? 'workspace', + }), + ]) + let rootFolder: Awaited> | undefined + + try { + const result = await decompressArchiveBufferToWorkspaceFiles(content, { + workspaceId: context.workspaceId, + principal, + prepareRootFolder: async () => { + const attribution = resolvePrincipalAttribution(principal, { + workspaceBillingOwnerUserId: context.billedAccountUserId, + }) + rootFolder = await createWorkspaceFileFolder({ + workspaceId: context.workspaceId, + userId: attribution.attributedUserId, + name: folderName, + parentId: file.folderId, + exactName: false, + }) + return parseWorkspaceFileFolderDisplayPath(rootFolder.path) + }, + materializedRootFolderCount: 1, + maxMaterializedItems: MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS, + skipNoiseEntries: true, + secretProvenance, + }) + if (result.extracted.length === 0) { + throw new OrchestrationError('validation', `No files could be unzipped from "${file.name}"`) + } + if (result.skippedUnsafePaths.length > 0) { + logger.warn('Skipped unsafe archive entries', { + workspaceId: context.workspaceId, + fileId: file.id, + entryNames: result.skippedUnsafePaths, + }) + } + + return { + folderName: rootFolder?.name ?? folderName, + extractedCount: result.extracted.length, + skippedCount: result.skipped, + } + } catch (error) { + if (rootFolder) { + try { + await archiveWorkspaceFileFolderIfEmpty({ + workspaceId: context.workspaceId, + folderId: rootFolder.id, + }) + } catch (cleanupError) { + logger.warn('Left non-empty archive destination folder after extraction error', { + workspaceId: context.workspaceId, + folderId: rootFolder.id, + cleanupError, + }) + } + await notifyWorkspaceFilesChanged(context.workspaceId) + } + throw error + } +} + +export const extractWorkspaceFile = defineAuthorizedWorkspaceFileUseCase({ + operation: fileOperations.extractArchive, + resolveContext: ({ input }) => resolveActiveWorkspaceFileContext(input), + execute: executeExtractWorkspaceFile, + projectAudit: ({ context, result }) => ({ + action: AuditAction.FILE_UPDATED, + resourceType: AuditResourceType.FILE, + resourceId: context.fileId, + description: `Unzipped workspace file ${context.fileId}`, + metadata: { + destinationFolder: result.folderName, + extractedCount: result.extractedCount, + skippedCount: result.skippedCount, + }, + }), + afterSuccess: ({ context }) => notifyWorkspaceFilesChanged(context.workspaceId), +}) diff --git a/apps/sim/lib/workspace-files/application/operations.ts b/apps/sim/lib/workspace-files/application/operations.ts index a4e0c89839c..3c0890ce8f9 100644 --- a/apps/sim/lib/workspace-files/application/operations.ts +++ b/apps/sim/lib/workspace-files/application/operations.ts @@ -59,6 +59,12 @@ export const fileOperations = { workspaceApiKey: 'allow', ...ALL_COPILOT_PRINCIPAL_POLICY, }), + extractArchive: defineWorkspaceOperation({ + id: 'files.extract_archive', + minimumRole: 'write', + workspaceApiKey: 'deny', + principalKinds: ['session'], + }), updateContent: defineWorkspaceOperation({ id: 'files.update_content', minimumRole: 'write', From d54609735754632e6d357395fdbab125d31c4a33 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:04:15 -0700 Subject: [PATCH 02/10] fix(files): harden zip extraction safety --- apps/sim/lib/uploads/archive.test.ts | 27 ++++++++++ apps/sim/lib/uploads/archive.ts | 43 ++++++++++------ .../workspace-file-folder-manager.test.ts | 6 +++ .../workspace-file-folder-manager.ts | 4 ++ .../workspace/workspace-file-manager.ts | 50 +++++++++++++------ .../workspace-file-storage-accounting.test.ts | 30 +++++++++-- .../extract-workspace-file.test.ts | 23 ++++++++- .../application/extract-workspace-file.ts | 8 ++- 8 files changed, 155 insertions(+), 36 deletions(-) diff --git a/apps/sim/lib/uploads/archive.test.ts b/apps/sim/lib/uploads/archive.test.ts index 3e49a34687c..796ad9f43b9 100644 --- a/apps/sim/lib/uploads/archive.test.ts +++ b/apps/sim/lib/uploads/archive.test.ts @@ -703,6 +703,33 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { } ) + it('includes the destination prefix when validating archive folder paths', async () => { + const buffer = await buildZip({ 'nested/file.txt': 'x' }) + const prepareRootFolder = vi.fn(async () => ['unused']) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + principal: TEST_PRINCIPAL, + rootFolderSegments: Array.from( + { length: MAX_FOLDER_PATH_SEGMENTS }, + (_, index) => `existing-${index}` + ), + prepareRootFolder, + }) + ).rejects.toMatchObject({ + name: 'ArchiveError', + reason: 'invalid', + message: expect.stringContaining( + `Folder paths cannot exceed ${MAX_FOLDER_PATH_SEGMENTS} segments` + ), + }) + + expect(prepareRootFolder).not.toHaveBeenCalled() + expect(mockEnsureFolder).not.toHaveBeenCalled() + 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'), { diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts index 7f5014ef538..dda5c8f5d88 100644 --- a/apps/sim/lib/uploads/archive.ts +++ b/apps/sim/lib/uploads/archive.ts @@ -263,10 +263,13 @@ function throwInflateCapError(reason: 'entry' | 'total', entryName: string): nev * When `prepareRootFolder` is provided it takes precedence over * `rootFolderSegments`: it runs once, only after the caps have been proven and * only when at least one safe entry exists, and extraction lands under the - * segments it returns. `materializedRootFolderCount` must equal the number of - * folders that callback will create, so the `maxMaterializedItems` pre-check - * (files + implied folders + root folders) counts them and rejects an - * over-limit archive before the callback materializes anything. + * segments it returns. Before inserting a deduplicated root, the callback must + * pass its final segments to the supplied validator so the complete destination + * path is rejected before any folder mutation. `materializedRootFolderCount` + * must equal the number of folders that callback will create, so the + * `maxMaterializedItems` pre-check (files + implied folders + root folders) + * counts them and rejects an over-limit archive before the callback materializes + * anything. * * Filesystem-noise entries (`__MACOSX/`, `.DS_Store`, `Thumbs.db`) are extracted * verbatim unless `skipNoiseEntries` is set — the HTTP decompress route preserves @@ -280,7 +283,9 @@ export async function decompressArchiveBufferToWorkspaceFiles( workspaceId: string principal: Principal rootFolderSegments?: string[] - prepareRootFolder?: () => Promise + prepareRootFolder?: ( + validateRootFolderSegments: (rootFolderSegments: string[]) => void + ) => Promise materializedRootFolderCount?: number maxMaterializedItems?: number skipNoiseEntries?: boolean @@ -342,18 +347,21 @@ export async function decompressArchiveBufferToWorkspaceFiles( ) } - for (const { entry, segments } of safeEntries) { - try { - buildFolderPath(segments.slice(0, -1)) - } catch (error) { - if (!(error instanceof FolderPathError)) throw error - throw new ArchiveError( - 'invalid', - `Archive contains an invalid folder path: ${error.message}`, - entry.name - ) + const validateRootFolderSegments = (candidateRootFolderSegments: string[]): void => { + for (const { entry, segments } of safeEntries) { + try { + buildFolderPath([...candidateRootFolderSegments, ...segments.slice(0, -1)]) + } catch (error) { + if (!(error instanceof FolderPathError)) throw error + throw new ArchiveError( + 'invalid', + `Archive contains an invalid folder path: ${error.message}`, + entry.name + ) + } } } + validateRootFolderSegments(rootFolderSegments) if (maxMaterializedItems !== undefined) { const folderPaths = new Set() @@ -398,7 +406,10 @@ export async function decompressArchiveBufferToWorkspaceFiles( } const resolvedRootFolderSegments = - safeEntries.length > 0 && prepareRootFolder ? await prepareRootFolder() : rootFolderSegments + safeEntries.length > 0 && prepareRootFolder + ? await prepareRootFolder(validateRootFolderSegments) + : rootFolderSegments + validateRootFolderSegments(resolvedRootFolderSegments) // 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 diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts index a4de1ab884f..8b6963f7382 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts @@ -52,6 +52,7 @@ describe('createWorkspaceFileFolder', () => { createdAt: now, updatedAt: now, } + const validateResolvedName = vi.fn() mockDeduplicateFolderName.mockResolvedValueOnce('Archive (3)') dbChainMockFns.returning.mockResolvedValueOnce([inserted]) @@ -61,6 +62,7 @@ describe('createWorkspaceFileFolder', () => { userId: 'user-1', name: 'Archive', exactName: false, + validateResolvedName, }) ).resolves.toMatchObject({ name: 'Archive (3)' }) @@ -74,6 +76,10 @@ describe('createWorkspaceFileFolder', () => { expect(dbChainMockFns.values).toHaveBeenCalledWith( expect.objectContaining({ name: 'Archive (3)' }) ) + expect(validateResolvedName).toHaveBeenCalledWith('Archive (3)') + expect(validateResolvedName.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.values.mock.invocationCallOrder[0] + ) }) }) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts index 583751820fd..d4004d3050f 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts @@ -494,6 +494,8 @@ export async function createWorkspaceFileFolder(params: { parentId?: string | null sortOrder?: number exactName?: boolean + /** Validates the exact post-deduplication name before the folder row is inserted. */ + validateResolvedName?: (name: string) => void }): Promise { const requestedName = normalizeWorkspaceFileItemName(params.name, 'Folder') @@ -531,6 +533,8 @@ export async function createWorkspaceFileFolder(params: { ) : requestedName + params.validateResolvedName?.(name) + if (params.exactName !== false) { const existingFolders = await tx .select({ id: folderTable.id }) 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 484523923b9..368826e69a0 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -2051,8 +2051,10 @@ export async function deleteWorkspaceFile(workspaceId: string, fileId: string): /** * Permanently removes a file created by an in-flight archive extraction only while - * its name, folder, and update timestamp still match the creation result. This is - * rollback-only: ordinary user deletion remains recoverable through {@link deleteWorkspaceFile}. + * its name, folder, and update timestamp still match the creation result. The exact + * row stays locked while storage is deleted, so a storage failure leaves metadata + * and accounting intact for retry. This is rollback-only: ordinary user deletion + * remains recoverable through {@link deleteWorkspaceFile}. */ export async function purgeCreatedWorkspaceFile(params: { workspaceId: string @@ -2067,7 +2069,33 @@ export async function purgeCreatedWorkspaceFile(params: { params.expectedFolderId === null ? isNull(workspaceFiles.folderId) : eq(workspaceFiles.folderId, params.expectedFolderId) - const purgedKey = await db.transaction(async (tx) => { + return db.transaction(async (tx) => { + const [lockedFile] = await tx + .select({ + id: workspaceFiles.id, + key: workspaceFiles.key, + size: workspaceFiles.size, + sizeBytes: workspaceFiles.sizeBytes, + }) + .from(workspaceFiles) + .where( + and( + eq(workspaceFiles.id, params.fileId), + eq(workspaceFiles.workspaceId, params.workspaceId), + eq(workspaceFiles.key, params.key), + eq(workspaceFiles.originalName, params.expectedName), + expectedFolder, + eq(workspaceFiles.updatedAt, params.expectedUpdatedAt), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) + ) + .for('update') + .limit(1) + if (!lockedFile) return false + + await deleteFile({ key: lockedFile.key, context: 'workspace' }) + const [deleted] = await tx .delete(workspaceFiles) .where( @@ -2082,24 +2110,16 @@ export async function purgeCreatedWorkspaceFile(params: { isNull(workspaceFiles.deletedAt) ) ) - .returning({ - key: workspaceFiles.key, - size: workspaceFiles.size, - sizeBytes: workspaceFiles.sizeBytes, - }) - if (!deleted) return null + .returning({ id: workspaceFiles.id }) + if (!deleted) throw new Error('Locked archive-created file could not be deleted') await decrementStorageUsageForBillingContextInTx( tx, storageBillingContext, - workspaceFileSize(deleted) + workspaceFileSize(lockedFile) ) - return deleted.key + return true }) - - if (!purgedKey) return false - await deleteFile({ key: purgedKey, context: 'workspace' }) - return true } /** diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts index e50c9fcdb68..a584fa08173 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts @@ -267,6 +267,7 @@ describe('workspace file metadata and storage accounting', () => { it('purges exact archive-created metadata, bytes, and accounting during rollback', async () => { const extractedRow = { ...FILE_ROW, folderId: 'folder-archive' } + dbChainMockFns.limit.mockResolvedValueOnce([extractedRow]) dbChainMockFns.returning.mockResolvedValueOnce([extractedRow]) await expect( @@ -289,13 +290,16 @@ describe('workspace file metadata and storage accounting', () => { FILE_ROW.size ) expect(mockDeleteFile).toHaveBeenCalledWith({ key: FILE_ROW.key, context: 'workspace' }) - expect(mockDecrementStorageUsageForBillingContextInTx.mock.invocationCallOrder[0]).toBeLessThan( - mockDeleteFile.mock.invocationCallOrder[0] + expect(mockDeleteFile.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.delete.mock.invocationCallOrder[0] + ) + expect(dbChainMockFns.delete.mock.invocationCallOrder[0]).toBeLessThan( + mockDecrementStorageUsageForBillingContextInTx.mock.invocationCallOrder[0] ) }) it('leaves an extracted file untouched when its creation identity no longer matches', async () => { - dbChainMockFns.returning.mockResolvedValueOnce([]) + dbChainMockFns.limit.mockResolvedValueOnce([]) await expect( purgeCreatedWorkspaceFile({ @@ -312,6 +316,26 @@ describe('workspace file metadata and storage accounting', () => { expect(mockDeleteFile).not.toHaveBeenCalled() }) + it('keeps archive-created metadata and accounting when storage deletion fails', async () => { + const extractedRow = { ...FILE_ROW, folderId: 'folder-archive' } + dbChainMockFns.limit.mockResolvedValueOnce([extractedRow]) + mockDeleteFile.mockRejectedValueOnce(new Error('storage unavailable')) + + await expect( + purgeCreatedWorkspaceFile({ + workspaceId: FILE_ROW.workspaceId, + fileId: FILE_ROW.id, + key: FILE_ROW.key, + expectedName: FILE_ROW.originalName, + expectedFolderId: extractedRow.folderId, + expectedUpdatedAt: FILE_ROW.updatedAt, + }) + ).rejects.toThrow('storage unavailable') + + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + expect(mockDecrementStorageUsageForBillingContextInTx).not.toHaveBeenCalled() + }) + it('preserves the driver cause so the SQLSTATE survives the upload wrapper', async () => { const driver = Object.assign( new Error('cannot execute SELECT FOR UPDATE in a read-only transaction'), diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts index aba7e7f53a1..3038c789be6 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts @@ -100,7 +100,7 @@ describe('extractWorkspaceFile', () => { mocks.fetchBuffer.mockResolvedValue(Buffer.from('zip')) mocks.getSecretProvenance.mockResolvedValue({ status: 'exact', entries: [] }) mocks.decompress.mockImplementation(async (_content, options) => { - await options.prepareRootFolder() + await options.prepareRootFolder(vi.fn()) return { extracted: [{ id: 'extracted-1' }, { id: 'extracted-2' }], skipped: 1, @@ -112,6 +112,24 @@ describe('extractWorkspaceFile', () => { }) it('extracts into a same-name folder beside the archive', async () => { + const validateRootFolderSegments = vi.fn() + mocks.decompress.mockImplementationOnce(async (_content, options) => { + await options.prepareRootFolder(validateRootFolderSegments) + return { + extracted: [{ id: 'extracted-1' }, { id: 'extracted-2' }], + skipped: 1, + skippedUnsafePaths: [], + } + }) + mocks.createFolder.mockImplementationOnce(async (options) => { + options.validateResolvedName('bundle') + return { + id: 'folder-bundle', + name: 'bundle', + path: 'Projects/Imports/bundle', + } + }) + await expect( extractWorkspaceFile.execute({ principal, @@ -125,11 +143,14 @@ describe('extractWorkspaceFile', () => { name: 'bundle', parentId: 'folder-imports', exactName: false, + validateResolvedName: expect.any(Function), }) + expect(validateRootFolderSegments).toHaveBeenCalledWith(['Projects', 'Imports', 'bundle']) expect(mocks.fetchBuffer).toHaveBeenCalledWith(file, { maxBytes: 100 * 1024 * 1024 }) expect(mocks.decompress).toHaveBeenCalledWith(Buffer.from('zip'), { workspaceId: 'workspace-1', principal, + rootFolderSegments: ['Projects', 'Imports', 'bundle'], prepareRootFolder: expect.any(Function), materializedRootFolderCount: 1, maxMaterializedItems: 5000, diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts index f7c02bcc654..616bf2c3a9f 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts @@ -114,6 +114,9 @@ async function extractWorkspaceFileContents({ } const folderName = archiveFolderName(file.name) + const parentFolderSegments = file.folderPath + ? parseWorkspaceFileFolderDisplayPath(file.folderPath) + : [] const [content, secretProvenance] = await Promise.all([ fetchWorkspaceFileBuffer(file, { maxBytes: MAX_ARCHIVE_BYTES }), getBoundWorkspaceFileSecretProvenance(context.workspaceId, { @@ -128,7 +131,8 @@ async function extractWorkspaceFileContents({ const result = await decompressArchiveBufferToWorkspaceFiles(content, { workspaceId: context.workspaceId, principal, - prepareRootFolder: async () => { + rootFolderSegments: [...parentFolderSegments, folderName], + prepareRootFolder: async (validateRootFolderSegments) => { const attribution = resolvePrincipalAttribution(principal, { workspaceBillingOwnerUserId: context.billedAccountUserId, }) @@ -138,6 +142,8 @@ async function extractWorkspaceFileContents({ name: folderName, parentId: file.folderId, exactName: false, + validateResolvedName: (resolvedName) => + validateRootFolderSegments([...parentFolderSegments, resolvedName]), }) return parseWorkspaceFileFolderDisplayPath(rootFolder.path) }, From c779a15eebc916adb5503d4b0bef5c76706484e4 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:13:47 -0700 Subject: [PATCH 03/10] fix(files): batch extraction notifications --- apps/sim/lib/uploads/archive.test.ts | 14 ++++++++++---- apps/sim/lib/uploads/archive.ts | 7 +++++++ .../application/extract-workspace-file.test.ts | 1 + .../application/extract-workspace-file.ts | 1 + 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/uploads/archive.test.ts b/apps/sim/lib/uploads/archive.test.ts index 796ad9f43b9..43bcba8aa04 100644 --- a/apps/sim/lib/uploads/archive.test.ts +++ b/apps/sim/lib/uploads/archive.test.ts @@ -16,8 +16,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' * - `exactName: true` throws `FileConflictError` on a duplicate leaf name, while * `exactName: false` auto-suffixes, mirroring `uploadWorkspaceFile`. */ -const { store, mockUpload, mockPurge, mockEnsureFolder, mockArchiveFolderIfEmpty } = vi.hoisted( - () => ({ +const { store, mockUpload, mockPurge, mockEnsureFolder, mockArchiveFolderIfEmpty, mockNotify } = + vi.hoisted(() => ({ store: { folderIdByPath: new Map(), fileKeys: new Set(), @@ -30,8 +30,9 @@ const { store, mockUpload, mockPurge, mockEnsureFolder, mockArchiveFolderIfEmpty mockPurge: vi.fn(), mockEnsureFolder: vi.fn(), mockArchiveFolderIfEmpty: vi.fn(), - }) -) + mockNotify: vi.fn(), + })) +vi.mock('@/lib/realtime/notify', () => ({ notifyWorkspaceFilesChanged: mockNotify })) vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({ ensureWorkspaceFileFolderPathOperation: { execute: mockEnsureFolder }, })) @@ -182,6 +183,7 @@ beforeEach(() => { }) mockPurge.mockResolvedValue(true) + mockNotify.mockResolvedValue(undefined) mockUpload.mockImplementation( async ({ input, @@ -226,6 +228,8 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect(result.extracted).toHaveLength(2) expect(result.skippedUnsafePaths).toEqual([]) expect(mockUpload).toHaveBeenCalledTimes(2) + expect(mockNotify).toHaveBeenCalledOnce() + expect(mockNotify).toHaveBeenCalledWith('ws') const leafNames = mockUpload.mock.calls.map(([args]) => args.input.name).sort() expect(leafNames).toEqual(['report.txt', 'sheet.csv']) // Entries are rooted under the archive's folder; nested paths are preserved. @@ -499,6 +503,8 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { ).rejects.toThrow('storage quota exceeded') expect(mockPurge).toHaveBeenCalledTimes(2) + expect(mockNotify).toHaveBeenCalledOnce() + expect(mockNotify).toHaveBeenCalledWith('ws') expect(mockPurge).toHaveBeenCalledWith({ workspaceId: 'ws', fileId: 'f_a', diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts index dda5c8f5d88..17e7a1d516c 100644 --- a/apps/sim/lib/uploads/archive.ts +++ b/apps/sim/lib/uploads/archive.ts @@ -5,6 +5,7 @@ import { createLogger } from '@sim/logger' import JSZip from 'jszip' import { readZipCentralDirectoryStats } from '@/lib/file-parsers/zip-guard' import { buildFolderPath, FolderPathError } from '@/lib/folders/paths' +import { notifyWorkspaceFilesChanged } from '@/lib/realtime/notify' import { archiveWorkspaceFileFolderIfEmpty } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { purgeCreatedWorkspaceFile, @@ -290,6 +291,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( maxMaterializedItems?: number skipNoiseEntries?: boolean secretProvenance?: WorkspaceFileSecretProvenance + notifyWorkspaceChange?: boolean } ): Promise { const { @@ -301,6 +303,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( maxMaterializedItems, skipNoiseEntries = false, secretProvenance = { status: 'unknown' }, + notifyWorkspaceChange = true, } = opts const extractedSecretProvenance: WorkspaceFileSecretProvenance = secretProvenance.status === 'exact' && secretProvenance.entries.length === 0 @@ -515,8 +518,12 @@ export async function decompressArchiveBufferToWorkspaceFiles( }) } } + if (notifyWorkspaceChange) await notifyWorkspaceFilesChanged(workspaceId) throw error } + if (notifyWorkspaceChange && extracted.length > 0) { + await notifyWorkspaceFilesChanged(workspaceId) + } return { extracted, skipped, skippedUnsafePaths } } diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts index 3038c789be6..13fc836d283 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts @@ -156,6 +156,7 @@ describe('extractWorkspaceFile', () => { maxMaterializedItems: 5000, skipNoiseEntries: true, secretProvenance: { status: 'exact', entries: [] }, + notifyWorkspaceChange: false, }) expect(mocks.atomicallyClaim).toHaveBeenCalledWith('extract', 'workspace-1:file-1') expect(mocks.releaseLease).toHaveBeenCalledWith( diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts index 616bf2c3a9f..89f4be66e18 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts @@ -151,6 +151,7 @@ async function extractWorkspaceFileContents({ maxMaterializedItems: MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS, skipNoiseEntries: true, secretProvenance, + notifyWorkspaceChange: false, }) if (result.extracted.length === 0) { throw new OrchestrationError('validation', `No files could be unzipped from "${file.name}"`) From f139e280206b63a23fc691ee4a885a698dc33955 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:22:48 -0700 Subject: [PATCH 04/10] fix(files): defer rollback storage cleanup --- .../app/api/webhooks/outbox/process/route.ts | 2 + .../workspace/workspace-file-manager.ts | 41 +++++++--- .../workspace-file-storage-accounting.test.ts | 65 +++++++++++++--- ...kspace-file-storage-cleanup-outbox.test.ts | 74 +++++++++++++++++++ .../workspace-file-storage-cleanup-outbox.ts | 54 ++++++++++++++ 5 files changed, 217 insertions(+), 19 deletions(-) create mode 100644 apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.test.ts create mode 100644 apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.ts diff --git a/apps/sim/app/api/webhooks/outbox/process/route.ts b/apps/sim/app/api/webhooks/outbox/process/route.ts index a9edf8bc231..75d41fa3bee 100644 --- a/apps/sim/app/api/webhooks/outbox/process/route.ts +++ b/apps/sim/app/api/webhooks/outbox/process/route.ts @@ -10,6 +10,7 @@ import { processOutboxEvents } from '@/lib/core/outbox/service' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { knowledgeDocumentProcessingOutboxHandlers } from '@/lib/knowledge/documents/processing-outbox-handler' +import { workspaceFileStorageCleanupOutboxHandlers } from '@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox' import { workflowDeploymentOutboxHandlers } from '@/lib/workflows/deployment-outbox' import { invitationMigrationOutboxHandlers } from '@/lib/workspaces/admin-move' import { reapStaleBackgroundWork } from '@/ee/workspace-forking/lib/background-work/store' @@ -25,6 +26,7 @@ const handlers = { ...enterpriseIssuanceOutboxHandlers, ...invitationMigrationOutboxHandlers, ...knowledgeDocumentProcessingOutboxHandlers, + ...workspaceFileStorageCleanupOutboxHandlers, ...workflowDeploymentOutboxHandlers, } as const 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 368826e69a0..2f974f1be66 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -57,6 +57,10 @@ import { type WorkspaceFileSecretProvenance, type WorkspaceFileSecretProvenancePolicy, } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { + enqueueWorkspaceFileStorageCleanup, + processWorkspaceFileStorageCleanupNow, +} from '@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox' import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key' import { deleteFile, @@ -2051,10 +2055,10 @@ export async function deleteWorkspaceFile(workspaceId: string, fileId: string): /** * Permanently removes a file created by an in-flight archive extraction only while - * its name, folder, and update timestamp still match the creation result. The exact - * row stays locked while storage is deleted, so a storage failure leaves metadata - * and accounting intact for retry. This is rollback-only: ordinary user deletion - * remains recoverable through {@link deleteWorkspaceFile}. + * its name, folder, and update timestamp still match the creation result. The matching + * metadata deletion, accounting update, and durable storage-cleanup event commit together. This + * is rollback-only: ordinary user deletion remains recoverable through + * {@link deleteWorkspaceFile}. */ export async function purgeCreatedWorkspaceFile(params: { workspaceId: string @@ -2069,7 +2073,7 @@ export async function purgeCreatedWorkspaceFile(params: { params.expectedFolderId === null ? isNull(workspaceFiles.folderId) : eq(workspaceFiles.folderId, params.expectedFolderId) - return db.transaction(async (tx) => { + const cleanupEventId = await db.transaction(async (tx) => { const [lockedFile] = await tx .select({ id: workspaceFiles.id, @@ -2092,9 +2096,7 @@ export async function purgeCreatedWorkspaceFile(params: { ) .for('update') .limit(1) - if (!lockedFile) return false - - await deleteFile({ key: lockedFile.key, context: 'workspace' }) + if (!lockedFile) return null const [deleted] = await tx .delete(workspaceFiles) @@ -2118,8 +2120,29 @@ export async function purgeCreatedWorkspaceFile(params: { storageBillingContext, workspaceFileSize(lockedFile) ) - return true + return enqueueWorkspaceFileStorageCleanup(tx, { key: lockedFile.key }) }) + if (!cleanupEventId) return false + + try { + const result = await processWorkspaceFileStorageCleanupNow(cleanupEventId) + if (result !== 'completed') { + logger.warn('Archive rollback storage cleanup deferred to outbox retry', { + workspaceId: params.workspaceId, + fileId: params.fileId, + cleanupEventId, + result, + }) + } + } catch (error) { + logger.warn('Archive rollback storage cleanup deferred after inline processing error', { + workspaceId: params.workspaceId, + fileId: params.fileId, + cleanupEventId, + error: getErrorMessage(error), + }) + } + return true } /** diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts index a584fa08173..f0988f8abe4 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts @@ -10,6 +10,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockDecrementStorageUsageForBillingContextInTx, mockDeleteFile, + mockEnqueueWorkspaceFileStorageCleanup, mockGetWorkspaceWithOwner, mockHasCloudStorage, mockHeadObject, @@ -21,6 +22,7 @@ const { mockMaybeNotifyStorageLimitForBillingContext, mockMergeEditIntoLiveFileDoc, mockNotifyWorkspaceFilesChanged, + mockProcessWorkspaceFileStorageCleanupNow, mockResolveStorageBillingContext, mockResolveFolderPathFromIndex, mockResolveWorkspaceFileFolderTarget, @@ -29,6 +31,7 @@ const { } = vi.hoisted(() => ({ mockDecrementStorageUsageForBillingContextInTx: vi.fn(), mockDeleteFile: vi.fn(), + mockEnqueueWorkspaceFileStorageCleanup: vi.fn(), mockGetWorkspaceWithOwner: vi.fn(), mockHasCloudStorage: vi.fn(), mockHeadObject: vi.fn(), @@ -40,6 +43,7 @@ const { mockMaybeNotifyStorageLimitForBillingContext: vi.fn(), mockMergeEditIntoLiveFileDoc: vi.fn(), mockNotifyWorkspaceFilesChanged: vi.fn(), + mockProcessWorkspaceFileStorageCleanupNow: vi.fn(), mockResolveStorageBillingContext: vi.fn(), mockResolveFolderPathFromIndex: vi.fn(), mockResolveWorkspaceFileFolderTarget: vi.fn(), @@ -78,6 +82,11 @@ vi.mock('@/lib/uploads/core/storage-service', () => ({ uploadFile: mockUploadFile, })) +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox', () => ({ + enqueueWorkspaceFileStorageCleanup: mockEnqueueWorkspaceFileStorageCleanup, + processWorkspaceFileStorageCleanupNow: mockProcessWorkspaceFileStorageCleanupNow, +})) + vi.mock('@/lib/uploads/contexts/workspace/workspace-file-folder-manager', () => ({ assertWorkspaceFileFolderTarget: mockAssertWorkspaceFileFolderTarget, buildWorkspaceFileFolderPathMap: vi.fn(() => new Map()), @@ -155,8 +164,10 @@ describe('workspace file metadata and storage accounting', () => { mockDecrementStorageUsageForBillingContextInTx.mockResolvedValue(undefined) mockMaybeNotifyStorageLimitForBillingContext.mockResolvedValue(undefined) mockDeleteFile.mockResolvedValue(undefined) + mockEnqueueWorkspaceFileStorageCleanup.mockResolvedValue('cleanup-event-1') mockMergeEditIntoLiveFileDoc.mockResolvedValue(undefined) mockNotifyWorkspaceFilesChanged.mockResolvedValue(undefined) + mockProcessWorkspaceFileStorageCleanupNow.mockResolvedValue('completed') mockReplaceWorkspaceFileSecretProvenanceInTx.mockResolvedValue(undefined) }) @@ -265,7 +276,7 @@ describe('workspace file metadata and storage accounting', () => { ) }) - it('purges exact archive-created metadata, bytes, and accounting during rollback', async () => { + it('atomically purges exact archive-created metadata and accounting before storage cleanup', async () => { const extractedRow = { ...FILE_ROW, folderId: 'folder-archive' } dbChainMockFns.limit.mockResolvedValueOnce([extractedRow]) dbChainMockFns.returning.mockResolvedValueOnce([extractedRow]) @@ -289,13 +300,17 @@ describe('workspace file metadata and storage accounting', () => { STORAGE_CONTEXT, FILE_ROW.size ) - expect(mockDeleteFile).toHaveBeenCalledWith({ key: FILE_ROW.key, context: 'workspace' }) - expect(mockDeleteFile.mock.invocationCallOrder[0]).toBeLessThan( - dbChainMockFns.delete.mock.invocationCallOrder[0] - ) + expect(mockEnqueueWorkspaceFileStorageCleanup).toHaveBeenCalledWith(expect.any(Object), { + key: FILE_ROW.key, + }) expect(dbChainMockFns.delete.mock.invocationCallOrder[0]).toBeLessThan( mockDecrementStorageUsageForBillingContextInTx.mock.invocationCallOrder[0] ) + expect(mockDecrementStorageUsageForBillingContextInTx.mock.invocationCallOrder[0]).toBeLessThan( + mockEnqueueWorkspaceFileStorageCleanup.mock.invocationCallOrder[0] + ) + expect(mockProcessWorkspaceFileStorageCleanupNow).toHaveBeenCalledWith('cleanup-event-1') + expect(mockDeleteFile).not.toHaveBeenCalled() }) it('leaves an extracted file untouched when its creation identity no longer matches', async () => { @@ -313,13 +328,18 @@ describe('workspace file metadata and storage accounting', () => { ).resolves.toBe(false) expect(mockDecrementStorageUsageForBillingContextInTx).not.toHaveBeenCalled() + expect(mockEnqueueWorkspaceFileStorageCleanup).not.toHaveBeenCalled() + expect(mockProcessWorkspaceFileStorageCleanupNow).not.toHaveBeenCalled() expect(mockDeleteFile).not.toHaveBeenCalled() }) - it('keeps archive-created metadata and accounting when storage deletion fails', async () => { + it('does not touch storage when the metadata and accounting transaction fails', async () => { const extractedRow = { ...FILE_ROW, folderId: 'folder-archive' } dbChainMockFns.limit.mockResolvedValueOnce([extractedRow]) - mockDeleteFile.mockRejectedValueOnce(new Error('storage unavailable')) + dbChainMockFns.returning.mockResolvedValueOnce([extractedRow]) + mockDecrementStorageUsageForBillingContextInTx.mockRejectedValueOnce( + new Error('accounting unavailable') + ) await expect( purgeCreatedWorkspaceFile({ @@ -330,10 +350,35 @@ describe('workspace file metadata and storage accounting', () => { expectedFolderId: extractedRow.folderId, expectedUpdatedAt: FILE_ROW.updatedAt, }) - ).rejects.toThrow('storage unavailable') + ).rejects.toThrow('accounting unavailable') - expect(dbChainMockFns.delete).not.toHaveBeenCalled() - expect(mockDecrementStorageUsageForBillingContextInTx).not.toHaveBeenCalled() + expect(mockEnqueueWorkspaceFileStorageCleanup).not.toHaveBeenCalled() + expect(mockProcessWorkspaceFileStorageCleanupNow).not.toHaveBeenCalled() + expect(mockDeleteFile).not.toHaveBeenCalled() + }) + + it('keeps deferred cleanup durable when immediate processing fails', async () => { + const extractedRow = { ...FILE_ROW, folderId: 'folder-archive' } + dbChainMockFns.limit.mockResolvedValueOnce([extractedRow]) + dbChainMockFns.returning.mockResolvedValueOnce([extractedRow]) + mockProcessWorkspaceFileStorageCleanupNow.mockRejectedValueOnce( + new Error('outbox processor unavailable') + ) + + await expect( + purgeCreatedWorkspaceFile({ + workspaceId: FILE_ROW.workspaceId, + fileId: FILE_ROW.id, + key: FILE_ROW.key, + expectedName: FILE_ROW.originalName, + expectedFolderId: extractedRow.folderId, + expectedUpdatedAt: FILE_ROW.updatedAt, + }) + ).resolves.toBe(true) + + expect(mockEnqueueWorkspaceFileStorageCleanup).toHaveBeenCalledOnce() + expect(mockProcessWorkspaceFileStorageCleanupNow).toHaveBeenCalledWith('cleanup-event-1') + expect(mockDeleteFile).not.toHaveBeenCalled() }) it('preserves the driver cause so the SQLSTATE survives the upload wrapper', async () => { diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.test.ts new file mode 100644 index 00000000000..c47a8abd4d9 --- /dev/null +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.test.ts @@ -0,0 +1,74 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockDeleteFile } = vi.hoisted(() => ({ + mockDeleteFile: vi.fn(), +})) + +vi.mock('@/lib/uploads/core/storage-service', () => ({ + deleteFile: mockDeleteFile, +})) + +import type { OutboxEventContext } from '@/lib/core/outbox/service' +import { + WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT, + workspaceFileStorageCleanupOutboxHandlers, +} from '@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox' + +function context(): OutboxEventContext { + return { + eventId: 'cleanup-event-1', + eventType: WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT, + attempts: 0, + maxAttempts: 10, + signal: new AbortController().signal, + checkpointPayload: vi.fn(), + } +} + +function handler() { + const registered = + workspaceFileStorageCleanupOutboxHandlers[WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT] + if (!registered) throw new Error('Workspace file storage cleanup handler is not registered') + return registered +} + +describe('workspace file storage cleanup outbox', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDeleteFile.mockResolvedValue(undefined) + }) + + it('deletes the deferred workspace object', async () => { + await handler()({ key: 'workspace/ws/file.txt' }, context()) + + expect(mockDeleteFile).toHaveBeenCalledWith({ + key: 'workspace/ws/file.txt', + context: 'workspace', + }) + }) + + it('treats an already-missing local object as completed', async () => { + mockDeleteFile.mockRejectedValueOnce(Object.assign(new Error('missing'), { code: 'ENOENT' })) + + await expect(handler()({ key: 'workspace/ws/file.txt' }, context())).resolves.toBeUndefined() + }) + + it('rejects malformed payloads without touching storage', async () => { + await expect(handler()({ key: '' }, context())).rejects.toThrow( + 'Workspace file storage cleanup outbox payload is missing key' + ) + + expect(mockDeleteFile).not.toHaveBeenCalled() + }) + + it('propagates storage failures for retry', async () => { + mockDeleteFile.mockRejectedValueOnce(new Error('storage unavailable')) + + await expect(handler()({ key: 'workspace/ws/file.txt' }, context())).rejects.toThrow( + 'storage unavailable' + ) + }) +}) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.ts new file mode 100644 index 00000000000..80dbcff4226 --- /dev/null +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.ts @@ -0,0 +1,54 @@ +import type { db } from '@sim/db' +import { describeError } from '@sim/utils/errors' +import { + enqueueOutboxEvent, + type OutboxHandler, + type OutboxHandlerRegistry, + processOutboxEventById, +} from '@/lib/core/outbox/service' +import { deleteFile } from '@/lib/uploads/core/storage-service' + +export const WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT = 'workspace-file.storage.cleanup' + +interface WorkspaceFileStorageCleanupPayload { + key: string +} + +function parsePayload(payload: unknown): WorkspaceFileStorageCleanupPayload { + if (!payload || typeof payload !== 'object' || Array.isArray(payload)) { + throw new Error('Workspace file storage cleanup outbox payload must be an object') + } + const key = (payload as Record).key + if (typeof key !== 'string' || key.trim().length === 0) { + throw new Error('Workspace file storage cleanup outbox payload is missing key') + } + return { key } +} + +const cleanupWorkspaceFileStorage: OutboxHandler = async (rawPayload, context) => { + const payload = parsePayload(rawPayload) + context.signal.throwIfAborted() + try { + await deleteFile({ key: payload.key, context: 'workspace' }) + } catch (error) { + if (describeError(error).code === 'ENOENT') return + throw error + } +} + +export const workspaceFileStorageCleanupOutboxHandlers = { + [WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT]: cleanupWorkspaceFileStorage, +} satisfies OutboxHandlerRegistry + +/** Enqueues storage deletion in the transaction that removes the corresponding metadata. */ +export function enqueueWorkspaceFileStorageCleanup( + executor: Pick, + payload: WorkspaceFileStorageCleanupPayload +): Promise { + return enqueueOutboxEvent(executor, WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT, payload) +} + +/** Attempts a newly committed cleanup immediately; the outbox worker retries incomplete work. */ +export function processWorkspaceFileStorageCleanupNow(eventId: string) { + return processOutboxEventById(eventId, workspaceFileStorageCleanupOutboxHandlers) +} From 0d255a200d74ce1cda16293f689f0af26c7a0800 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:37:17 -0700 Subject: [PATCH 05/10] fix(files): restore reliable drag uploads --- .../workspace/[workspaceId]/files/files.tsx | 9 +++++-- .../uploads/upload-session/service.test.ts | 10 ++++++-- .../sim/lib/uploads/upload-session/service.ts | 24 ++++++++++++------- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index ff70a3b7dd2..7f1fed9cc0b 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -1923,9 +1923,14 @@ export function Files() { } /> {isDraggingOver ? ( -
+
-

Drop to upload

+
+

Drop to upload

+

+ Release files here to add them to this workspace +

+
) : null} diff --git a/apps/sim/lib/uploads/upload-session/service.test.ts b/apps/sim/lib/uploads/upload-session/service.test.ts index 0bebbeb2d80..d7746abdfaf 100644 --- a/apps/sim/lib/uploads/upload-session/service.test.ts +++ b/apps/sim/lib/uploads/upload-session/service.test.ts @@ -741,8 +741,8 @@ describe('upload sessions', () => { partCount: 2, }) const parts = [ - { partNumber: 1, etag: 'etag-1', size: UPLOAD_SESSION_PART_SIZE }, { partNumber: 2, etag: 'etag-2', size: 3 }, + { partNumber: 1, etag: 'etag-1', size: UPLOAD_SESSION_PART_SIZE }, ] mockListMultipartParts.mockResolvedValue(parts) mockHeadObject @@ -760,7 +760,13 @@ describe('upload sessions', () => { expect.objectContaining({ key: FINAL_KEY, providerUploadId: 'provider-upload-1' }) ) expect(mockCompleteMultipart).toHaveBeenCalledWith( - expect.objectContaining({ key: FINAL_KEY, parts }) + expect.objectContaining({ + key: FINAL_KEY, + parts: [ + { partNumber: 1, etag: 'etag-1', size: UPLOAD_SESSION_PART_SIZE }, + { partNumber: 2, etag: 'etag-2', size: 3 }, + ], + }) ) expect(finalize).toHaveBeenCalledOnce() }) diff --git a/apps/sim/lib/uploads/upload-session/service.ts b/apps/sim/lib/uploads/upload-session/service.ts index 5ce9847db66..71e3b55c67d 100644 --- a/apps/sim/lib/uploads/upload-session/service.ts +++ b/apps/sim/lib/uploads/upload-session/service.ts @@ -627,14 +627,16 @@ export async function completeUploadSession(params: { if (claimed.method === 'put') { throw new UploadSessionError('conflict', 'Uploaded object not found') } - const parts = await listMultipartProviderParts({ - provider: claimed.storageProvider, - providerUploadId: claimed.providerUploadId, - uploadId: claimed.id, - key: claimed.finalKey, - context: claimed.storageContext, - }) - validateProviderParts(claimed, parts) + const parts = validateProviderParts( + claimed, + await listMultipartProviderParts({ + provider: claimed.storageProvider, + providerUploadId: claimed.providerUploadId, + uploadId: claimed.id, + key: claimed.finalKey, + context: claimed.storageContext, + }) + ) try { await completeMultipartProviderUpload({ provider: claimed.storageProvider, @@ -1036,7 +1038,10 @@ async function claimSession( return sessionFromRow(row, '') } -function validateProviderParts(session: UploadSessionRecord, parts: CompletedUploadPart[]): void { +function validateProviderParts( + session: UploadSessionRecord, + parts: CompletedUploadPart[] +): CompletedUploadPart[] { if (!session.partCount) throw new Error('Multipart upload is missing partCount') if (parts.length !== session.partCount) { throw new UploadSessionError( @@ -1067,6 +1072,7 @@ function validateProviderParts(session: UploadSessionRecord, parts: CompletedUpl ) } } + return sorted } function assertObjectIdentity( From 16a774a003c4bd1fc27666050c5d6ae1cea9f10d Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:16:44 -0700 Subject: [PATCH 06/10] fix(files): use explicit archive extraction route --- .../[id]/files/[fileId]/extract/route.test.ts | 82 +++++++++++++++++++ .../[id]/files/[fileId]/extract/route.ts | 27 ++++++ .../[id]/files/[fileId]/route.test.ts | 67 +-------------- .../workspaces/[id]/files/[fileId]/route.ts | 18 ---- apps/sim/lib/api/contracts/workspace-files.ts | 2 +- 5 files changed, 111 insertions(+), 85 deletions(-) create mode 100644 apps/sim/app/api/workspaces/[id]/files/[fileId]/extract/route.test.ts create mode 100644 apps/sim/app/api/workspaces/[id]/files/[fileId]/extract/route.ts diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/extract/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/extract/route.test.ts new file mode 100644 index 00000000000..ac4073d41e5 --- /dev/null +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/extract/route.test.ts @@ -0,0 +1,82 @@ +/** + * @vitest-environment node + */ +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + getSession: vi.fn(), + extract: vi.fn(), +})) + +vi.mock('@/lib/auth', () => ({ getSession: mocks.getSession })) + +vi.mock('@/lib/workspace-files/application/extract-workspace-file', () => ({ + extractWorkspaceFile: { + operation: { id: 'files.extract_archive', minimumRole: 'write', workspaceApiKey: 'deny' }, + execute: mocks.extract, + }, +})) + +import { ArchiveError } from '@/lib/uploads/archive' +import { POST } from '@/app/api/workspaces/[id]/files/[fileId]/extract/route' + +const WORKSPACE_ID = 'workspace-1' +const FILE_ID = 'wf_1' +const context = { params: Promise.resolve({ id: WORKSPACE_ID, fileId: FILE_ID }) } + +function callExtract() { + return POST( + new NextRequest( + `http://localhost:3000/api/workspaces/${WORKSPACE_ID}/files/${FILE_ID}/extract`, + { method: 'POST' } + ), + context + ) +} + +describe('POST /api/workspaces/[id]/files/[fileId]/extract', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.getSession.mockResolvedValue({ + user: { id: 'user-1' }, + session: { id: 'session-1' }, + }) + mocks.extract.mockResolvedValue({ folderName: 'bundle', extractedCount: 2, skippedCount: 0 }) + }) + + it('passes a session principal and canonical assertion to the extraction use case', async () => { + const response = await callExtract() + + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ + success: true, + folderName: 'bundle', + extractedCount: 2, + skippedCount: 0, + }) + expect(mocks.extract).toHaveBeenCalledWith({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { fileId: FILE_ID, assertedWorkspaceId: WORKSPACE_ID }, + request: expect.anything(), + }) + }) + + it('authenticates before invoking extraction', async () => { + mocks.getSession.mockResolvedValue(null) + + const response = await callExtract() + + expect(response.status).toBe(401) + expect(mocks.extract).not.toHaveBeenCalled() + }) + + it('returns a caller-safe error for an invalid zip', async () => { + mocks.extract.mockRejectedValue(new ArchiveError('invalid', 'Not a valid .zip archive.')) + + const response = await callExtract() + + expect(response.status).toBe(400) + expect(await response.json()).toEqual({ error: 'Not a valid .zip archive.' }) + }) +}) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/extract/route.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/extract/route.ts new file mode 100644 index 00000000000..f7740fe4b63 --- /dev/null +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/extract/route.ts @@ -0,0 +1,27 @@ +import { extractWorkspaceFileContract } from '@/lib/api/contracts/workspace-files' +import { + defineInternalJsonRoute, + internalRateLimits, + internalSessionAuth, +} from '@/lib/api/server/routes' +import { internalFileErrorPolicies } from '@/lib/workspace-files/api' +import { extractWorkspaceFile } from '@/lib/workspace-files/application/extract-workspace-file' +import { fileOperations } from '@/lib/workspace-files/application/operations' + +export const dynamic = 'force-dynamic' +export const maxDuration = 300 + +/** + * POST /api/workspaces/[id]/files/[fileId]/extract + * Unzip an archive file into a new folder beside it (requires write permission) + */ +export const POST = defineInternalJsonRoute({ + contract: extractWorkspaceFileContract, + auth: internalSessionAuth, + operation: fileOperations.extractArchive, + rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal file behavior' }), + errorPolicy: internalFileErrorPolicies.extractArchive, + mapInput: ({ params }) => ({ fileId: params.fileId, assertedWorkspaceId: params.id }), + useCase: extractWorkspaceFile, + present: (result) => ({ success: true, ...result }), +}) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts index ea575747450..a0b4d86aba6 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts @@ -6,7 +6,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ getSession: vi.fn(), - extract: vi.fn(), rename: vi.fn(), deleteItems: vi.fn(), getUserEntityPermissions: vi.fn(), @@ -15,13 +14,6 @@ const mocks = vi.hoisted(() => ({ vi.mock('@/lib/auth', () => ({ getSession: mocks.getSession })) -vi.mock('@/lib/workspace-files/application/extract-workspace-file', () => ({ - extractWorkspaceFile: { - operation: { id: 'files.extract_archive', minimumRole: 'write', workspaceApiKey: 'deny' }, - execute: mocks.extract, - }, -})) - vi.mock('@/lib/workspace-files/application/rename-workspace-file', () => ({ renameWorkspaceFile: { operation: { id: 'files.rename', minimumRole: 'write', workspaceApiKey: 'allow' }, @@ -46,8 +38,7 @@ import { WorkspaceApiKeyScopeAuthorizationError, } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' -import { ArchiveError } from '@/lib/uploads/archive' -import { PATCH, POST } from '@/app/api/workspaces/[id]/files/[fileId]/route' +import { PATCH } from '@/app/api/workspaces/[id]/files/[fileId]/route' const WORKSPACE_ID = 'workspace-1' const FILE_ID = 'wf_1' @@ -64,15 +55,6 @@ function callRename(body: unknown) { ) } -function callExtract() { - return POST( - new NextRequest(`http://localhost:3000/api/workspaces/${WORKSPACE_ID}/files/${FILE_ID}`, { - method: 'POST', - }), - context - ) -} - function fileRecord() { return { id: FILE_ID, @@ -97,7 +79,6 @@ describe('PATCH /api/workspaces/[id]/files/[fileId]', () => { session: { id: 'session-1' }, }) mocks.rename.mockResolvedValue({ file: fileRecord() }) - mocks.extract.mockResolvedValue({ folderName: 'bundle', extractedCount: 2, skippedCount: 0 }) }) it('authenticates before parsing the request', async () => { @@ -189,49 +170,3 @@ describe('PATCH /api/workspaces/[id]/files/[fileId]', () => { }) }) }) - -describe('POST /api/workspaces/[id]/files/[fileId]', () => { - beforeEach(() => { - vi.clearAllMocks() - mocks.getSession.mockResolvedValue({ - user: { id: 'user-1' }, - session: { id: 'session-1' }, - }) - mocks.extract.mockResolvedValue({ folderName: 'bundle', extractedCount: 2, skippedCount: 0 }) - }) - - it('passes a session principal and canonical assertion to the extraction use case', async () => { - const response = await callExtract() - - expect(response.status).toBe(200) - expect(await response.json()).toEqual({ - success: true, - folderName: 'bundle', - extractedCount: 2, - skippedCount: 0, - }) - expect(mocks.extract).toHaveBeenCalledWith({ - principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, - input: { fileId: FILE_ID, assertedWorkspaceId: WORKSPACE_ID }, - request: expect.anything(), - }) - }) - - it('authenticates before invoking extraction', async () => { - mocks.getSession.mockResolvedValue(null) - - const response = await callExtract() - - expect(response.status).toBe(401) - expect(mocks.extract).not.toHaveBeenCalled() - }) - - it('returns a caller-safe error for an invalid zip', async () => { - mocks.extract.mockRejectedValue(new ArchiveError('invalid', 'Not a valid .zip archive.')) - - const response = await callExtract() - - expect(response.status).toBe(400) - expect(await response.json()).toEqual({ error: 'Not a valid .zip archive.' }) - }) -}) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts index e0e7cae5646..a93c34f07f3 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts @@ -1,6 +1,5 @@ import { deleteWorkspaceFileContract, - extractWorkspaceFileContract, renameWorkspaceFileContract, } from '@/lib/api/contracts/workspace-files' import { @@ -15,27 +14,10 @@ import { internalFilePresenters, } from '@/lib/workspace-files/api' import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file' -import { extractWorkspaceFile } from '@/lib/workspace-files/application/extract-workspace-file' import { fileOperations } from '@/lib/workspace-files/application/operations' import { renameWorkspaceFile } from '@/lib/workspace-files/application/rename-workspace-file' export const dynamic = 'force-dynamic' -export const maxDuration = 300 - -/** - * POST /api/workspaces/[id]/files/[fileId] - * Unzip an archive file into a new folder beside it (requires write permission) - */ -export const POST = defineInternalJsonRoute({ - contract: extractWorkspaceFileContract, - auth: internalSessionAuth, - operation: fileOperations.extractArchive, - rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal file behavior' }), - errorPolicy: internalFileErrorPolicies.extractArchive, - mapInput: ({ params }) => ({ fileId: params.fileId, assertedWorkspaceId: params.id }), - useCase: extractWorkspaceFile, - present: (result) => ({ success: true, ...result }), -}) /** * PATCH /api/workspaces/[id]/files/[fileId] diff --git a/apps/sim/lib/api/contracts/workspace-files.ts b/apps/sim/lib/api/contracts/workspace-files.ts index 0c7f683b3d2..bf275a55fb7 100644 --- a/apps/sim/lib/api/contracts/workspace-files.ts +++ b/apps/sim/lib/api/contracts/workspace-files.ts @@ -194,7 +194,7 @@ export const renameWorkspaceFileContract = defineRouteContract({ export const extractWorkspaceFileContract = defineRouteContract({ method: 'POST', - path: '/api/workspaces/[id]/files/[fileId]', + path: '/api/workspaces/[id]/files/[fileId]/extract', params: workspaceFileParamsSchema, response: { mode: 'json', From 2d9ed105d3b70d59b5ac695fd1b3f337202e31ec Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:25:11 -0700 Subject: [PATCH 07/10] fix(ci): account for archive extraction route --- scripts/check-api-validation-contracts.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/check-api-validation-contracts.ts b/scripts/check-api-validation-contracts.ts index 1e26e115df9..ee60c365129 100644 --- a/scripts/check-api-validation-contracts.ts +++ b/scripts/check-api-validation-contracts.ts @@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries') const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors') const BASELINE = { - totalRoutes: 1119, - zodRoutes: 1119, + totalRoutes: 1120, + zodRoutes: 1120, nonZodRoutes: 0, } as const From 07661d5c352af545f5c28cd4359cb730886ca47e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 16:44:56 -0700 Subject: [PATCH 08/10] refactor(files): bound every archive extraction and trim the extractor's option surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `maxMaterializedItems` was opt-in, so only the new unzip route bounded its output tree — the copilot `materialize_file` and `POST /api/tools/file/manage` extract paths had no cap on folder creation at all. An archive within MAX_ARCHIVE_ENTRIES can still imply far more folders than files, so the cap now defaults to MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS and applies to all three callers. `materializedRootFolderCount` was a hand-maintained number that had to agree with what an opaque callback would create, and the callee could not check it; drift surfaced only as an over-limit archive slipping past the cap. It is now derived from whether `prepareRootFolder` ran, so the contract is just "the callback creates exactly one folder". Also single-sources the ArchiveError -> HTTP status map (it was copied into both the internal error policy and the tools route), drops IdempotencyService config that only `executeWithIdempotency` reads (the extraction lease uses atomicallyClaim/release, so no result is ever stored), hoists the duplicated predicates in purgeCreatedWorkspaceFile and archiveWorkspaceFileFolderIfEmpty so a lock and its write cannot diverge, and names UPLOAD_SESSION_LOCAL_PUT_MAX_BYTES rather than overloading the multipart part size as the local single-PUT ceiling. Adds coverage for the two guards nothing exercised: the re-validation of the segments `prepareRootFolder` actually returned, and the default cap applying with no caller opt-in. UI: the drop overlay used --surface-4 unconditionally, which renders grey over the light-mode canvas; matches the canonical overlay's --white/dark:--surface-4 and swaps arbitrary px type sizes for named tokens. --- apps/sim/app/api/tools/file/manage/route.ts | 3 +- .../workspace/[workspaceId]/files/files.tsx | 10 +-- apps/sim/lib/uploads/archive.test.ts | 56 ++++++++++++++- apps/sim/lib/uploads/archive.ts | 71 ++++++++++++------- .../workspace-file-folder-manager.ts | 46 +++++------- .../workspace/workspace-file-manager.ts | 37 ++++------ .../lib/uploads/upload-session/provider.ts | 3 +- .../sim/lib/uploads/upload-session/service.ts | 31 ++++---- .../api/internal-error-policies.ts | 4 +- .../application/create-workspace-file.ts | 4 +- .../extract-workspace-file.test.ts | 2 - .../application/extract-workspace-file.ts | 12 ++-- 12 files changed, 165 insertions(+), 114 deletions(-) diff --git a/apps/sim/app/api/tools/file/manage/route.ts b/apps/sim/app/api/tools/file/manage/route.ts index 48936493e09..fdf8d5df047 100644 --- a/apps/sim/app/api/tools/file/manage/route.ts +++ b/apps/sim/app/api/tools/file/manage/route.ts @@ -34,6 +34,7 @@ import { type DecompressResult, decompressArchiveBufferToWorkspaceFiles, MAX_ARCHIVE_BYTES, + statusForArchiveError, } from '@/lib/uploads/archive' import type { getWorkspaceFile } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { @@ -1183,7 +1184,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { 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 status = statusForArchiveError(archiveError) return NextResponse.json( { success: false, error: `"${archive.name}": ${archiveError.message}` }, { status } diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 7f1fed9cc0b..f2fce25c67a 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -1440,7 +1440,7 @@ export function Files() { } const file = fileByIdRef.current.get(parsed.id) if (file && isArchiveFileName(file.name)) { - if (!extractFile.isPending) setExtractTargetId(file.id) + setExtractTargetId(file.id) return } router.push( @@ -1450,7 +1450,7 @@ export function Files() { ) } }, - [router, workspaceId, currentFolderId, setFilesParams, extractFile.isPending] + [router, workspaceId, currentFolderId, setFilesParams] ) const handleExtract = async () => { @@ -1923,11 +1923,11 @@ export function Files() { } /> {isDraggingOver ? ( -
+
-

Drop to upload

-

+

Drop to upload

+

Release files here to add them to this workspace

diff --git a/apps/sim/lib/uploads/archive.test.ts b/apps/sim/lib/uploads/archive.test.ts index 43bcba8aa04..d7b4f6f0189 100644 --- a/apps/sim/lib/uploads/archive.test.ts +++ b/apps/sim/lib/uploads/archive.test.ts @@ -59,6 +59,7 @@ import { MAX_ARCHIVE_CENTRAL_DIR_RECORDS, MAX_ARCHIVE_ENTRY_BYTES, } from '@/lib/uploads/archive' +import { MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS } from '@/lib/workspace-files/limits' const TEST_PRINCIPAL = { kind: 'session', @@ -651,7 +652,6 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { workspaceId: 'ws', principal: TEST_PRINCIPAL, prepareRootFolder, - materializedRootFolderCount: 1, maxMaterializedItems: 2, }) ).rejects.toMatchObject({ name: 'ArchiveError', reason: 'too_many_entries' }) @@ -665,7 +665,6 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { workspaceId: 'ws', principal: TEST_PRINCIPAL, prepareRootFolder, - materializedRootFolderCount: 1, maxMaterializedItems: 3, }) ).resolves.toMatchObject({ extracted: [expect.objectContaining({ name: 'file.txt' })] }) @@ -694,7 +693,6 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { workspaceId: 'ws', principal: TEST_PRINCIPAL, prepareRootFolder, - materializedRootFolderCount: 1, maxMaterializedItems: 5000, }) ).rejects.toMatchObject({ @@ -736,6 +734,58 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect(mockUpload).not.toHaveBeenCalled() }) + it('applies the workspace bulk limit by default, so every caller is bounded', async () => { + // 1000 files, each under six folders unique to it: 7000 materialized items from an entry + // count that is itself within MAX_ARCHIVE_ENTRIES. No caller opts in to this cap. + const entries: Record = {} + for (let index = 0; index < 1000; index += 1) { + entries[`a${index}/b/c/d/e/f/file.txt`] = 'x' + } + const buffer = await buildZip(entries) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + principal: TEST_PRINCIPAL, + }) + ).rejects.toMatchObject({ + name: 'ArchiveError', + reason: 'too_many_entries', + message: expect.stringContaining(`the maximum is ${MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS}`), + }) + + expect(mockEnsureFolder).not.toHaveBeenCalled() + expect(mockUpload).not.toHaveBeenCalled() + }) + + it('re-validates the segments prepareRootFolder actually returned, before any upload', async () => { + const buffer = await buildZip({ 'nested/file.txt': 'x' }) + // A callback that validates one path and returns a different, invalid one. The callee + // cannot trust the callback to have checked what it returns, so it re-checks itself. + const prepareRootFolder = vi.fn(async (validate: (segments: string[]) => void) => { + validate(['bundle']) + return Array.from({ length: MAX_FOLDER_PATH_SEGMENTS + 1 }, (_, index) => `deep-${index}`) + }) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + principal: TEST_PRINCIPAL, + prepareRootFolder, + }) + ).rejects.toMatchObject({ + name: 'ArchiveError', + reason: 'invalid', + message: expect.stringContaining( + `Folder paths cannot exceed ${MAX_FOLDER_PATH_SEGMENTS} segments` + ), + }) + + expect(prepareRootFolder).toHaveBeenCalledOnce() + expect(mockEnsureFolder).not.toHaveBeenCalled() + 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'), { diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts index 17e7a1d516c..b5cd3bf88e5 100644 --- a/apps/sim/lib/uploads/archive.ts +++ b/apps/sim/lib/uploads/archive.ts @@ -15,6 +15,7 @@ import type { WorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/works import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' import { createWorkspaceFileFromBuffer } from '@/lib/workspace-files/application/create-workspace-file' import { ensureWorkspaceFileFolderPathOperation } from '@/lib/workspace-files/application/workspace-file-folders' +import { MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS } from '@/lib/workspace-files/limits' import type { UserFile } from '@/executor/types' /** @@ -87,6 +88,15 @@ export class ArchiveError extends Error { } } +/** + * The caller-facing HTTP status for an {@link ArchiveError}. A malformed archive is the + * caller's request being wrong (400); every other reason is a cap the payload exceeded (413). + * Single-sourced beside the reason union so a new variant is classified in exactly one place. + */ +export function statusForArchiveError(error: ArchiveError): number { + return error.reason === 'invalid' ? 400 : 413 +} + const MB = 1024 * 1024 /** @@ -264,13 +274,14 @@ function throwInflateCapError(reason: 'entry' | 'total', entryName: string): nev * When `prepareRootFolder` is provided it takes precedence over * `rootFolderSegments`: it runs once, only after the caps have been proven and * only when at least one safe entry exists, and extraction lands under the - * segments it returns. Before inserting a deduplicated root, the callback must - * pass its final segments to the supplied validator so the complete destination - * path is rejected before any folder mutation. `materializedRootFolderCount` - * must equal the number of folders that callback will create, so the - * `maxMaterializedItems` pre-check (files + implied folders + root folders) - * counts them and rejects an over-limit archive before the callback materializes - * anything. + * segments it returns. It must create exactly one folder, and must pass its + * final segments to the supplied validator before inserting it so the complete + * destination path is rejected before any folder mutation. That one folder is + * counted by the `maxMaterializedItems` pre-check (files + implied folders + + * root folder), which rejects an over-limit archive before the callback + * materializes anything. That cap always applies — it defaults to + * {@link MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS} — because the file count alone + * does not bound folder creation. * * Filesystem-noise entries (`__MACOSX/`, `.DS_Store`, `Thumbs.db`) are extracted * verbatim unless `skipNoiseEntries` is set — the HTTP decompress route preserves @@ -287,7 +298,6 @@ export async function decompressArchiveBufferToWorkspaceFiles( prepareRootFolder?: ( validateRootFolderSegments: (rootFolderSegments: string[]) => void ) => Promise - materializedRootFolderCount?: number maxMaterializedItems?: number skipNoiseEntries?: boolean secretProvenance?: WorkspaceFileSecretProvenance @@ -299,8 +309,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( principal, rootFolderSegments = [], prepareRootFolder, - materializedRootFolderCount = 0, - maxMaterializedItems, + maxMaterializedItems = MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS, skipNoiseEntries = false, secretProvenance = { status: 'unknown' }, notifyWorkspaceChange = true, @@ -366,24 +375,29 @@ export async function decompressArchiveBufferToWorkspaceFiles( } validateRootFolderSegments(rootFolderSegments) - if (maxMaterializedItems !== undefined) { - const folderPaths = new Set() - for (const { segments } of safeEntries) { - for (let depth = 1; depth < segments.length; depth += 1) { - folderPaths.add(segments.slice(0, depth).join('\0')) - } - } - const materializedItems = - safeEntries.length + - folderPaths.size + - (safeEntries.length > 0 ? materializedRootFolderCount : 0) - if (materializedItems > maxMaterializedItems) { - throw new ArchiveError( - 'too_many_entries', - `Archive would create ${materializedItems} files and folders; the maximum is ${maxMaterializedItems}.` - ) + const impliedFolderPaths = new Set() + for (const { segments } of safeEntries) { + let prefix = '' + for (let depth = 0; depth < segments.length - 1; depth += 1) { + prefix += `\0${segments[depth]}` + impliedFolderPaths.add(prefix) } } + /** + * Bounds the whole output tree, not just the file count: 1000 entries nested 64 deep imply + * far more folders than files, and nothing else caps folder creation. `prepareRootFolder` + * contributes exactly one more folder when it runs. + */ + const materializedItems = + safeEntries.length + + impliedFolderPaths.size + + (safeEntries.length > 0 && prepareRootFolder ? 1 : 0) + if (materializedItems > maxMaterializedItems) { + throw new ArchiveError( + 'too_many_entries', + `Archive would create ${materializedItems} files and folders; the maximum is ${maxMaterializedItems}.` + ) + } // Cheap declared-size fast-reject for honestly-declared archives. let declaredTotal = 0 @@ -412,7 +426,10 @@ export async function decompressArchiveBufferToWorkspaceFiles( safeEntries.length > 0 && prepareRootFolder ? await prepareRootFolder(validateRootFolderSegments) : rootFolderSegments - validateRootFolderSegments(resolvedRootFolderSegments) + // Re-check what the callback actually returned; identical segments were proven above. + if (resolvedRootFolderSegments !== rootFolderSegments) { + validateRootFolderSegments(resolvedRootFolderSegments) + } // 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 diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts index d4004d3050f..4ec715d2474 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts @@ -522,20 +522,20 @@ export async function createWorkspaceFileFolder(params: { } } - const name = - params.exactName === false - ? await deduplicateFolderName( - tx, - params.workspaceId, - parentId, - requestedName, - FILE_FOLDER_RESOURCE_TYPE - ) - : requestedName + const deduplicate = params.exactName === false + const name = deduplicate + ? await deduplicateFolderName( + tx, + params.workspaceId, + parentId, + requestedName, + FILE_FOLDER_RESOURCE_TYPE + ) + : requestedName params.validateResolvedName?.(name) - if (params.exactName !== false) { + if (!deduplicate) { const existingFolders = await tx .select({ id: folderTable.id }) .from(folderTable) @@ -1594,20 +1594,19 @@ export async function archiveWorkspaceFileFolderIfEmpty(params: { workspaceId: string folderId: string }): Promise { + const isTargetFolder = and( + eq(folderTable.id, params.folderId), + eq(folderTable.workspaceId, params.workspaceId), + isFileFolder, + isNull(folderTable.deletedAt) + ) return db.transaction(async (tx) => { await acquireWorkspaceFileFolderMutationLock(tx, params.workspaceId) const [folder] = await tx .select({ id: folderTable.id }) .from(folderTable) - .where( - and( - eq(folderTable.id, params.folderId), - eq(folderTable.workspaceId, params.workspaceId), - isFileFolder, - isNull(folderTable.deletedAt) - ) - ) + .where(isTargetFolder) .limit(1) if (!folder) return false @@ -1640,14 +1639,7 @@ export async function archiveWorkspaceFileFolderIfEmpty(params: { const [archived] = await tx .update(folderTable) .set({ deletedAt: new Date(), updatedAt: new Date() }) - .where( - and( - eq(folderTable.id, params.folderId), - eq(folderTable.workspaceId, params.workspaceId), - isFileFolder, - isNull(folderTable.deletedAt) - ) - ) + .where(isTargetFolder) .returning({ id: folderTable.id }) return Boolean(archived) }) 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 2f974f1be66..9531643db52 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -2073,6 +2073,17 @@ export async function purgeCreatedWorkspaceFile(params: { params.expectedFolderId === null ? isNull(workspaceFiles.folderId) : eq(workspaceFiles.folderId, params.expectedFolderId) + /** The full creation identity. Shared so the lock and the delete can never diverge. */ + const matchesCreatedFile = and( + eq(workspaceFiles.id, params.fileId), + eq(workspaceFiles.workspaceId, params.workspaceId), + eq(workspaceFiles.key, params.key), + eq(workspaceFiles.originalName, params.expectedName), + expectedFolder, + eq(workspaceFiles.updatedAt, params.expectedUpdatedAt), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) const cleanupEventId = await db.transaction(async (tx) => { const [lockedFile] = await tx .select({ @@ -2082,36 +2093,14 @@ export async function purgeCreatedWorkspaceFile(params: { sizeBytes: workspaceFiles.sizeBytes, }) .from(workspaceFiles) - .where( - and( - eq(workspaceFiles.id, params.fileId), - eq(workspaceFiles.workspaceId, params.workspaceId), - eq(workspaceFiles.key, params.key), - eq(workspaceFiles.originalName, params.expectedName), - expectedFolder, - eq(workspaceFiles.updatedAt, params.expectedUpdatedAt), - eq(workspaceFiles.context, 'workspace'), - isNull(workspaceFiles.deletedAt) - ) - ) + .where(matchesCreatedFile) .for('update') .limit(1) if (!lockedFile) return null const [deleted] = await tx .delete(workspaceFiles) - .where( - and( - eq(workspaceFiles.id, params.fileId), - eq(workspaceFiles.workspaceId, params.workspaceId), - eq(workspaceFiles.key, params.key), - eq(workspaceFiles.originalName, params.expectedName), - expectedFolder, - eq(workspaceFiles.updatedAt, params.expectedUpdatedAt), - eq(workspaceFiles.context, 'workspace'), - isNull(workspaceFiles.deletedAt) - ) - ) + .where(matchesCreatedFile) .returning({ id: workspaceFiles.id }) if (!deleted) throw new Error('Locked archive-created file could not be deleted') diff --git a/apps/sim/lib/uploads/upload-session/provider.ts b/apps/sim/lib/uploads/upload-session/provider.ts index 279da0eac2d..4386be70afa 100644 --- a/apps/sim/lib/uploads/upload-session/provider.ts +++ b/apps/sim/lib/uploads/upload-session/provider.ts @@ -52,7 +52,8 @@ export type { UploadStorageProvider } from '@/lib/uploads/upload-session/types' * Multipart parts are re-signed on demand by the per-surface `.../parts` * endpoints, so a long-lived multipart session always outlives its part URLs * and recovers by asking for new ones. A whole-object PUT has no such endpoint - * and needs none: it is size-capped by `UPLOAD_SESSION_PUT_MAX_BYTES`, is + * and needs none: it is size-capped by the provider's single-PUT ceiling + * (`UPLOAD_SESSION_PUT_MAX_BYTES`, or `UPLOAD_SESSION_LOCAL_PUT_MAX_BYTES` for `local`), is * issued and used within one client call, and is not resumable — an expired PUT * URL and an interrupted PUT have the identical recovery of starting a new * session. Nothing durable is written in either case, because the transfer is diff --git a/apps/sim/lib/uploads/upload-session/service.ts b/apps/sim/lib/uploads/upload-session/service.ts index 71e3b55c67d..785084835d6 100644 --- a/apps/sim/lib/uploads/upload-session/service.ts +++ b/apps/sim/lib/uploads/upload-session/service.ts @@ -45,6 +45,14 @@ import type { export const UPLOAD_SESSION_PUT_MAX_BYTES = 50 * 1024 * 1024 export const UPLOAD_SESSION_PART_SIZE = 8 * 1024 * 1024 +/** + * Single-PUT ceiling for the `local` provider. A local PUT is proxied through an app route + * rather than sent to object storage, so it must stay under the route's body limit; anything + * larger goes multipart, whose parts are already sized to fit. Kept equal to + * {@link UPLOAD_SESSION_PART_SIZE} but named separately so tuning part size for cloud + * throughput cannot silently move the local proxy threshold. + */ +export const UPLOAD_SESSION_LOCAL_PUT_MAX_BYTES = UPLOAD_SESSION_PART_SIZE export const UPLOAD_SESSION_MAX_PART_URLS = 100 export const UPLOAD_SESSION_TTL_MS = 24 * 60 * 60 * 1000 export const UPLOAD_SESSION_ASSET_MAX_BYTES = 5 * 1024 * 1024 @@ -218,7 +226,8 @@ export async function createUploadSession( } const provider = uploadStorageProvider() - const putMaxBytes = provider === 'local' ? UPLOAD_SESSION_PART_SIZE : UPLOAD_SESSION_PUT_MAX_BYTES + const putMaxBytes = + provider === 'local' ? UPLOAD_SESSION_LOCAL_PUT_MAX_BYTES : UPLOAD_SESSION_PUT_MAX_BYTES const method: UploadTransferMethod = params.fileSize <= putMaxBytes ? 'put' : 'multipart' const partSize = method === 'multipart' ? UPLOAD_SESSION_PART_SIZE : null const partCount = @@ -627,16 +636,14 @@ export async function completeUploadSession(params: { if (claimed.method === 'put') { throw new UploadSessionError('conflict', 'Uploaded object not found') } - const parts = validateProviderParts( - claimed, - await listMultipartProviderParts({ - provider: claimed.storageProvider, - providerUploadId: claimed.providerUploadId, - uploadId: claimed.id, - key: claimed.finalKey, - context: claimed.storageContext, - }) - ) + const providerParts = await listMultipartProviderParts({ + provider: claimed.storageProvider, + providerUploadId: claimed.providerUploadId, + uploadId: claimed.id, + key: claimed.finalKey, + context: claimed.storageContext, + }) + const parts = validatedSortedProviderParts(claimed, providerParts) try { await completeMultipartProviderUpload({ provider: claimed.storageProvider, @@ -1038,7 +1045,7 @@ async function claimSession( return sessionFromRow(row, '') } -function validateProviderParts( +function validatedSortedProviderParts( session: UploadSessionRecord, parts: CompletedUploadPart[] ): CompletedUploadPart[] { diff --git a/apps/sim/lib/workspace-files/api/internal-error-policies.ts b/apps/sim/lib/workspace-files/api/internal-error-policies.ts index 40332c243dc..2e899c8e6b2 100644 --- a/apps/sim/lib/workspace-files/api/internal-error-policies.ts +++ b/apps/sim/lib/workspace-files/api/internal-error-policies.ts @@ -8,7 +8,7 @@ import { } from '@/lib/api/server/routes' import { StorageLimitExceededError } from '@/lib/billing/storage' import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' -import { ArchiveError } from '@/lib/uploads/archive' +import { ArchiveError, statusForArchiveError } from '@/lib/uploads/archive' import { CompiledCheckTooLargeError, CompiledCheckUnsupportedError, @@ -89,7 +89,7 @@ const concealResourceAuthorization = createInternalResourceConcealmentPolicy({ const extractArchive = extendInternalErrorPolicy(concealResourceAuthorization, (error) => { if (!(error instanceof ArchiveError)) return null - return internalErrorResponse(error.reason === 'invalid' ? 400 : 413, { error: error.message }) + return internalErrorResponse(statusForArchiveError(error), { error: error.message }) }) export const internalFileErrorPolicies = { diff --git a/apps/sim/lib/workspace-files/application/create-workspace-file.ts b/apps/sim/lib/workspace-files/application/create-workspace-file.ts index 65d2e41a19e..67f7dfea494 100644 --- a/apps/sim/lib/workspace-files/application/create-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/create-workspace-file.ts @@ -52,9 +52,7 @@ async function createAuthorizedWorkspaceFile({ workspace, }: { principal: Principal - input: Omit & { - notifyWorkspaceChange?: boolean - } + input: Omit content: Buffer workspace: Awaited> }): Promise { diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts index 13fc836d283..1c7b15dc8a4 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts @@ -152,8 +152,6 @@ describe('extractWorkspaceFile', () => { principal, rootFolderSegments: ['Projects', 'Imports', 'bundle'], prepareRootFolder: expect.any(Function), - materializedRootFolderCount: 1, - maxMaterializedItems: 5000, skipNoiseEntries: true, secretProvenance: { status: 'exact', entries: [] }, notifyWorkspaceChange: false, diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts index 89f4be66e18..0141054fc22 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts @@ -22,16 +22,16 @@ import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/appl import { fileOperations } from '@/lib/workspace-files/application/operations' import { resolveActiveWorkspaceFileContext } from '@/lib/workspace-files/application/workspace-file-context' import { parseWorkspaceFileFolderDisplayPath } from '@/lib/workspace-files/folder-display-path' -import { MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS } from '@/lib/workspace-files/limits' const logger = createLogger('ExtractWorkspaceFile') const EXTRACTION_LEASE_TTL_SECONDS = 6 * 60 +/** + * Used only through `atomicallyClaim`/`release`, never `executeWithIdempotency`, so no + * result is ever stored — this is a lease, not a memoized operation. + */ const extractionLeases = new IdempotencyService({ namespace: 'workspace-file', ttlSeconds: EXTRACTION_LEASE_TTL_SECONDS, - inProgressTtlSeconds: EXTRACTION_LEASE_TTL_SECONDS, - retryFailures: true, - storeResultBody: false, forceStorage: 'database', }) @@ -91,7 +91,7 @@ async function withExtractionLease( async function executeExtractWorkspaceFile( useCaseContext: ExtractWorkspaceFileUseCaseContext ): Promise { - const { principal, context } = useCaseContext + const { context } = useCaseContext return withExtractionLease(context.workspaceId, context.fileId, () => extractWorkspaceFileContents(useCaseContext) ) @@ -147,8 +147,6 @@ async function extractWorkspaceFileContents({ }) return parseWorkspaceFileFolderDisplayPath(rootFolder.path) }, - materializedRootFolderCount: 1, - maxMaterializedItems: MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS, skipNoiseEntries: true, secretProvenance, notifyWorkspaceChange: false, From ef57474505f5d187b2930ad2463f5ec69376dd75 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 16:49:08 -0700 Subject: [PATCH 09/10] fix(files): bound the extraction write loop so it cannot outlive its lease MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor Bugbot flagged two related holes, both rooted in the write loop being unbounded: 1. `maxDuration` is a Next.js route-segment config that serverless platforms enforce and self-hosted deployments do not. A slow extraction (up to 1000 sequential uploads) could therefore outrun the six-minute lease, and `IdempotencyService` reclaims an expired in-progress claim — so a second unzip of the same archive could start beside the first. 2. Nothing rolls back a process killed mid-pass-2, so a timeout stranded the destination folder and every file written so far. `decompressArchiveBufferToWorkspaceFiles` now takes an `AbortSignal` and checks it between entries in both passes, and the extraction use case supplies a 180s deadline. The abort unwinds through the existing all-or-nothing rollback, so the work stops on our terms with the tree cleaned up, well inside both the route's 300s budget and the 360s lease. That closes (1) outright — the holder can no longer outlive its lease on any platform — and converts (2) from a stranded partial tree into a clean rollback for the slow case that actually triggers it. A SIGKILL still cannot be caught; that needs a durable job and is out of scope here. The overrun surfaces as a caller-fixable 413 naming the archive rather than an opaque 500 from the raw DOMException. --- apps/sim/lib/uploads/archive.test.ts | 24 +++++++++++++++++ apps/sim/lib/uploads/archive.ts | 9 +++++++ .../extract-workspace-file.test.ts | 23 ++++++++++++++++ .../application/extract-workspace-file.ts | 26 +++++++++++++++++++ 4 files changed, 82 insertions(+) diff --git a/apps/sim/lib/uploads/archive.test.ts b/apps/sim/lib/uploads/archive.test.ts index d7b4f6f0189..d3b807e0c77 100644 --- a/apps/sim/lib/uploads/archive.test.ts +++ b/apps/sim/lib/uploads/archive.test.ts @@ -734,6 +734,30 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect(mockUpload).not.toHaveBeenCalled() }) + it('aborts mid-extraction on the caller signal and rolls the partial tree back', async () => { + const buffer = await buildZip({ 'a.txt': 'x', 'b.txt': 'y', 'c.txt': 'z' }) + const controller = new AbortController() + const commit = mockUpload.getMockImplementation()! + mockUpload.mockImplementation(async (args: any) => { + const uploaded = await commit(args) + // Abort once the first file is committed, so rollback has something to undo. + controller.abort() + return uploaded + }) + + await expect( + decompressArchiveBufferToWorkspaceFiles(buffer, { + workspaceId: 'ws', + principal: TEST_PRINCIPAL, + signal: controller.signal, + }) + ).rejects.toMatchObject({ name: 'AbortError' }) + + expect(mockUpload).toHaveBeenCalledOnce() + expect(mockPurge).toHaveBeenCalledOnce() + expect(mockPurge).toHaveBeenCalledWith(expect.objectContaining({ expectedName: 'a.txt' })) + }) + it('applies the workspace bulk limit by default, so every caller is bounded', async () => { // 1000 files, each under six folders unique to it: 7000 materialized items from an entry // count that is itself within MAX_ARCHIVE_ENTRIES. No caller opts in to this cap. diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts index b5cd3bf88e5..2a6b4ff1385 100644 --- a/apps/sim/lib/uploads/archive.ts +++ b/apps/sim/lib/uploads/archive.ts @@ -271,6 +271,11 @@ function throwInflateCapError(reason: 'entry' | 'total', entryName: string): nev * 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). * + * `signal` aborts between entries in both passes. Callers that hold a lease or run + * under a request deadline must pass one: the write loop is otherwise unbounded, and a + * process killed mid-pass-2 strands a partial tree that no `catch` can roll back. + * Aborting instead unwinds through the same all-or-nothing rollback as any other failure. + * * When `prepareRootFolder` is provided it takes precedence over * `rootFolderSegments`: it runs once, only after the caps have been proven and * only when at least one safe entry exists, and extraction lands under the @@ -298,6 +303,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( prepareRootFolder?: ( validateRootFolderSegments: (rootFolderSegments: string[]) => void ) => Promise + signal?: AbortSignal maxMaterializedItems?: number skipNoiseEntries?: boolean secretProvenance?: WorkspaceFileSecretProvenance @@ -309,6 +315,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( principal, rootFolderSegments = [], prepareRootFolder, + signal, maxMaterializedItems = MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS, skipNoiseEntries = false, secretProvenance = { status: 'unknown' }, @@ -413,6 +420,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( // persisting anything, so a lying header aborts before any upload happens. let validatedTotal = 0 for (const { entry } of safeEntries) { + signal?.throwIfAborted() const result = await inflateEntryWithinCaps( entry, MAX_ARCHIVE_TOTAL_BYTES - validatedTotal, @@ -446,6 +454,7 @@ export async function decompressArchiveBufferToWorkspaceFiles( let totalBytes = 0 try { for (const { entry, segments } of safeEntries) { + signal?.throwIfAborted() const result = await inflateEntryWithinCaps(entry, MAX_ARCHIVE_TOTAL_BYTES - totalBytes, true) if (!result.ok) throwInflateCapError(result.reason, entry.name) totalBytes += result.size diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts index 1c7b15dc8a4..e4d5a43a0b7 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts @@ -152,6 +152,7 @@ describe('extractWorkspaceFile', () => { principal, rootFolderSegments: ['Projects', 'Imports', 'bundle'], prepareRootFolder: expect.any(Function), + signal: expect.any(AbortSignal), skipNoiseEntries: true, secretProvenance: { status: 'exact', entries: [] }, notifyWorkspaceChange: false, @@ -285,6 +286,28 @@ describe('extractWorkspaceFile', () => { expect(mocks.notify).toHaveBeenCalledWith('workspace-1') }) + it('reports a budget overrun as a caller-fixable error, not the raw abort', async () => { + mocks.decompress.mockImplementationOnce(async (_content, options) => { + await options.prepareRootFolder() + // Stand in for the deadline firing mid-write: the extractor aborts, rolls back, and + // rethrows the DOMException, which must not reach the caller as an opaque 500. + Object.defineProperty(options.signal, 'aborted', { value: true }) + throw Object.assign(new Error('The operation was aborted'), { name: 'TimeoutError' }) + }) + + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).rejects.toMatchObject({ + code: 'payload_too_large', + message: expect.stringContaining('took too long and was rolled back'), + }) + + expect(mocks.archiveFolderIfEmpty).toHaveBeenCalledOnce() + }) + it('leaves a destination folder that gained collaborators content during rollback', async () => { mocks.decompress.mockImplementationOnce(async (_content, options) => { await options.prepareRootFolder() diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts index 0141054fc22..cec0ae22e9c 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts @@ -24,6 +24,19 @@ import { resolveActiveWorkspaceFileContext } from '@/lib/workspace-files/applica import { parseWorkspaceFileFolderDisplayPath } from '@/lib/workspace-files/folder-display-path' const logger = createLogger('ExtractWorkspaceFile') +/** + * Wall-clock budget for the extraction itself, deliberately shorter than the route's + * `maxDuration` so the work stops on our terms — with the all-or-nothing rollback still + * able to run — rather than the platform killing the process mid-write and stranding a + * partial tree. Self-hosted deployments do not enforce `maxDuration` at all, so this is + * the only thing bounding the write loop there. + */ +const EXTRACTION_BUDGET_MS = 180 * 1000 +/** + * Must exceed {@link EXTRACTION_BUDGET_MS} plus the worst-case rollback, because + * `IdempotencyService` reclaims an expired in-progress claim: a holder that outlives its + * own lease would let a second unzip of the same archive start beside it. + */ const EXTRACTION_LEASE_TTL_SECONDS = 6 * 60 /** * Used only through `atomicallyClaim`/`release`, never `executeWithIdempotency`, so no @@ -113,6 +126,7 @@ async function extractWorkspaceFileContents({ ) } + const deadline = AbortSignal.timeout(EXTRACTION_BUDGET_MS) const folderName = archiveFolderName(file.name) const parentFolderSegments = file.folderPath ? parseWorkspaceFileFolderDisplayPath(file.folderPath) @@ -147,6 +161,7 @@ async function extractWorkspaceFileContents({ }) return parseWorkspaceFileFolderDisplayPath(rootFolder.path) }, + signal: deadline, skipNoiseEntries: true, secretProvenance, notifyWorkspaceChange: false, @@ -183,6 +198,17 @@ async function extractWorkspaceFileContents({ } await notifyWorkspaceFilesChanged(context.workspaceId) } + if (deadline.aborted && !(error instanceof OrchestrationError)) { + logger.warn('Archive extraction exceeded its budget and was rolled back', { + workspaceId: context.workspaceId, + fileId: file.id, + budgetMs: EXTRACTION_BUDGET_MS, + }) + throw new OrchestrationError( + 'payload_too_large', + `Unzipping "${file.name}" took too long and was rolled back. Try a smaller archive.` + ) + } throw error } } From 52b5c2f94050e4ffc772dba46a042ed30cca00f7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 16:55:59 -0700 Subject: [PATCH 10/10] fix(files): only remap the deadline abort itself, and stop overclaiming rollback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups on the budget deadline, both reported by Cursor Bugbot: `deadline.aborted` stays true for the rest of the request once the timer fires, so it cannot decide whether *this* error was the abort. An `ArchiveError` or storage failure thrown mid-entry after the timer fired was being relabelled as a timeout and returned as a 413, hiding the real cause. The catch now matches the thrown value against `deadline.reason` — `throwIfAborted()` throws exactly that object, so the check is identity-exact and cannot capture an unrelated failure. The message also claimed a rollback that has not necessarily happened: the budget covers the archive download too, so it can fire before the first write, when there is nothing to roll back. It now says the unzip was cancelled and claims nothing about what was written. Including the download in the budget is deliberate — the lease it has to fit inside starts earlier still — so the TSDoc says that rather than "the extraction itself". --- .../extract-workspace-file.test.ts | 38 ++++++++++++++++--- .../application/extract-workspace-file.ts | 18 +++++---- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts index e4d5a43a0b7..8fecb8e22f1 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts @@ -286,13 +286,18 @@ describe('extractWorkspaceFile', () => { expect(mocks.notify).toHaveBeenCalledWith('workspace-1') }) + /** Fires the deadline the way `AbortSignal.timeout` does: `reason` is what gets thrown. */ + function expireDeadline(signal: AbortSignal): unknown { + const reason = new DOMException('The operation was aborted due to timeout', 'TimeoutError') + Object.defineProperty(signal, 'aborted', { value: true }) + Object.defineProperty(signal, 'reason', { value: reason }) + return reason + } + it('reports a budget overrun as a caller-fixable error, not the raw abort', async () => { mocks.decompress.mockImplementationOnce(async (_content, options) => { await options.prepareRootFolder() - // Stand in for the deadline firing mid-write: the extractor aborts, rolls back, and - // rethrows the DOMException, which must not reach the caller as an opaque 500. - Object.defineProperty(options.signal, 'aborted', { value: true }) - throw Object.assign(new Error('The operation was aborted'), { name: 'TimeoutError' }) + throw expireDeadline(options.signal) }) await expect( @@ -302,12 +307,35 @@ describe('extractWorkspaceFile', () => { }) ).rejects.toMatchObject({ code: 'payload_too_large', - message: expect.stringContaining('took too long and was rolled back'), + message: expect.stringContaining('took too long and was cancelled'), }) expect(mocks.archiveFolderIfEmpty).toHaveBeenCalledOnce() }) + it('keeps the real cause when a failure races the deadline', async () => { + // The signal stays aborted for the rest of the request, so an unrelated mid-entry + // failure after the timer fires must not be relabelled as a timeout. + mocks.decompress.mockImplementationOnce(async (_content, options) => { + await options.prepareRootFolder() + expireDeadline(options.signal) + throw Object.assign(new Error('Archive entry "a.txt" could not be decompressed'), { + name: 'ArchiveError', + reason: 'invalid', + }) + }) + + await expect( + extractWorkspaceFile.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' }, + }) + ).rejects.toMatchObject({ + name: 'ArchiveError', + message: expect.stringContaining('could not be decompressed'), + }) + }) + it('leaves a destination folder that gained collaborators content during rollback', async () => { mocks.decompress.mockImplementationOnce(async (_content, options) => { await options.prepareRootFolder() diff --git a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts index cec0ae22e9c..f0ceac11066 100644 --- a/apps/sim/lib/workspace-files/application/extract-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/extract-workspace-file.ts @@ -25,11 +25,12 @@ import { parseWorkspaceFileFolderDisplayPath } from '@/lib/workspace-files/folde const logger = createLogger('ExtractWorkspaceFile') /** - * Wall-clock budget for the extraction itself, deliberately shorter than the route's - * `maxDuration` so the work stops on our terms — with the all-or-nothing rollback still - * able to run — rather than the platform killing the process mid-write and stranding a - * partial tree. Self-hosted deployments do not enforce `maxDuration` at all, so this is - * the only thing bounding the write loop there. + * Wall-clock budget for the whole operation — archive download included, because the lease + * this must fit inside starts earlier still. Deliberately shorter than the route's + * `maxDuration` so the work stops on our terms, with the all-or-nothing rollback still able + * to run, rather than the platform killing the process mid-write and stranding a partial + * tree. Self-hosted deployments do not enforce `maxDuration` at all, so this is the only + * thing bounding the write loop there. */ const EXTRACTION_BUDGET_MS = 180 * 1000 /** @@ -198,15 +199,16 @@ async function extractWorkspaceFileContents({ } await notifyWorkspaceFilesChanged(context.workspaceId) } - if (deadline.aborted && !(error instanceof OrchestrationError)) { - logger.warn('Archive extraction exceeded its budget and was rolled back', { + if (deadline.aborted && error === deadline.reason) { + logger.warn('Archive extraction exceeded its budget', { workspaceId: context.workspaceId, fileId: file.id, budgetMs: EXTRACTION_BUDGET_MS, + wroteAnything: Boolean(rootFolder), }) throw new OrchestrationError( 'payload_too_large', - `Unzipping "${file.name}" took too long and was rolled back. Try a smaller archive.` + `Unzipping "${file.name}" took too long and was cancelled. Try a smaller archive.` ) } throw error