From a4de062ac08e7d8b8f2188ea4a50f752055d420c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 27 Jul 2026 14:36:51 -0700 Subject: [PATCH] fix(files): preserve nested folder structure when archiving workspace files The file_compress tool flattened every entry to its leaf name, so asking Sim to package a folder produced a zip with all files at the root and collision suffixes instead of the folder layout. Archive entry paths now mirror the workspace folder structure, with the ancestor chain the whole selection shares dropped so a single folder is not nested under its parents. The bulk download route reuses the same builder instead of its own copy of the path sanitizing and dedup logic. --- apps/sim/app/api/tools/file/manage/route.ts | 59 ++++----- .../workspaces/[id]/files/download/route.ts | 52 ++------ apps/sim/lib/uploads/zip-entry-path.test.ts | 117 +++++++++++++++++ apps/sim/lib/uploads/zip-entry-path.ts | 124 ++++++++++++++++++ apps/sim/tools/file/compress.ts | 2 +- 5 files changed, 280 insertions(+), 74 deletions(-) create mode 100644 apps/sim/lib/uploads/zip-entry-path.test.ts create mode 100644 apps/sim/lib/uploads/zip-entry-path.ts diff --git a/apps/sim/app/api/tools/file/manage/route.ts b/apps/sim/app/api/tools/file/manage/route.ts index 6b3632c19f7..248002656ee 100644 --- a/apps/sim/app/api/tools/file/manage/route.ts +++ b/apps/sim/app/api/tools/file/manage/route.ts @@ -41,6 +41,7 @@ import { downloadServableFileFromStorage, } from '@/lib/uploads/utils/file-utils.server' import { docNotReadyResponse } from '@/lib/uploads/utils/servable-file-response' +import { buildZipEntryPaths } from '@/lib/uploads/zip-entry-path' import { performMoveWorkspaceFileItems } from '@/lib/workspace-files/orchestration' import { assertActiveWorkspaceAccess, @@ -175,9 +176,8 @@ const stripExtension = (name: string): string => { /** * Reduce an arbitrary name to a safe, flat file name: takes the final path * segment, drops directory and traversal components, and falls back when the - * result would be empty or a dot segment. Used for zip entry names and the - * compress archive name so untrusted input cannot introduce nested or - * zip-slip-style paths. + * result would be empty or a dot segment. Used for the compress archive name so + * untrusted input cannot introduce nested or zip-slip-style paths. */ const toFlatFileName = (name: string, fallback: string): string => { const leaf = name.replace(/\\/g, '/').split('/').pop()?.trim() @@ -185,27 +185,10 @@ const toFlatFileName = (name: string, fallback: string): string => { return leaf } -/** - * Return a zip entry name unique within `usedNames`, appending a numeric suffix - * before the extension on collision (e.g., "data.csv" -> "data (1).csv"). - */ -const uniqueZipEntryName = (name: string, usedNames: Set): string => { - if (!usedNames.has(name)) { - usedNames.add(name) - return name - } - - const dot = name.lastIndexOf('.') - const base = dot > 0 ? name.slice(0, dot) : name - const ext = dot > 0 ? name.slice(dot) : '' - let counter = 1 - let candidate = `${base} (${counter})${ext}` - while (usedNames.has(candidate)) { - counter += 1 - candidate = `${base} (${counter})${ext}` - } - usedNames.add(candidate) - return candidate +/** A file bound for a compress archive, paired with the workspace folder it lives in. */ +interface ArchiveEntry { + file: UserFile + folderPath: string | null } const isLikelyTextBuffer = (buffer: Buffer): boolean => isUtf8(buffer) && !buffer.includes(0) @@ -678,17 +661,27 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } - const userFiles: UserFile[] = workspaceFiles - .map((file) => workspaceFileToUserFile(file)) - .filter((file): file is NonNullable> => - Boolean(file) - ) - .concat(selectedInputFiles) + const workspaceEntries: ArchiveEntry[] = workspaceFiles.flatMap((file) => { + const userFile = workspaceFileToUserFile(file) + return userFile ? [{ file: userFile, folderPath: file?.folderPath ?? null }] : [] + }) + + // Picker/upload values carry no workspace folder, so they archive at the root. + const archiveEntries = workspaceEntries.concat( + selectedInputFiles.map((file) => ({ file, folderPath: null })) + ) + const userFiles: UserFile[] = archiveEntries.map((entry) => entry.file) + + // Mirror the workspace folder layout, dropping the ancestor chain the whole + // selection shares so archiving one folder does not nest it under its parents. + const entryPaths = buildZipEntryPaths( + archiveEntries.map((entry) => ({ name: entry.file.name, folderPath: entry.folderPath })), + { rebaseOnCommonFolder: true } + ) const zip = new JSZip() - const usedNames = new Set() let totalBytes = 0 - for (const userFile of userFiles) { + for (const [index, userFile] of userFiles.entries()) { const denied = await assertToolFileAccess(userFile.key, userId, requestId, logger) if (denied) return denied @@ -707,7 +700,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { { status: 413 } ) } - zip.file(uniqueZipEntryName(toFlatFileName(userFile.name, 'file'), usedNames), buffer) + zip.file(entryPaths[index], buffer) } const zipBuffer = await zip.generateAsync({ diff --git a/apps/sim/app/api/workspaces/[id]/files/download/route.ts b/apps/sim/app/api/workspaces/[id]/files/download/route.ts index b6220308e55..577f0e7b2dc 100644 --- a/apps/sim/app/api/workspaces/[id]/files/download/route.ts +++ b/apps/sim/app/api/workspaces/[id]/files/download/route.ts @@ -14,34 +14,13 @@ import { listWorkspaceFiles, } from '@/lib/uploads/contexts/workspace' import { formatFileSize } from '@/lib/uploads/utils/file-utils' +import { buildZipEntryPaths } from '@/lib/uploads/zip-entry-path' import { verifyWorkspaceMembership } from '@/app/api/workflows/utils' const logger = createLogger('WorkspaceFilesDownloadAPI') const MAX_ZIP_DOWNLOAD_FILES = 100 const MAX_ZIP_DOWNLOAD_BYTES = 250 * 1024 * 1024 -function safeZipPath(path: string): string { - return path - .split('/') - .map((segment) => { - const cleaned = segment.trim().replace(/[<>:"\\|?*\x00-\x1f]/g, '_') - return cleaned === '.' || cleaned === '..' ? '_' : cleaned - }) - .filter(Boolean) - .join('/') -} - -function withZipPathSuffix(path: string, suffix: number): string { - const slashIndex = path.lastIndexOf('/') - const directory = slashIndex >= 0 ? `${path.slice(0, slashIndex + 1)}` : '' - const filename = slashIndex >= 0 ? path.slice(slashIndex + 1) : path - const dotIndex = filename.lastIndexOf('.') - - return dotIndex > 0 - ? `${directory}${filename.slice(0, dotIndex)} (${suffix})${filename.slice(dotIndex)}` - : `${directory}${filename} (${suffix})` -} - function collectDescendantFolderIds( selectedFolderIds: string[], folders: Array<{ id: string; parentId: string | null }> @@ -115,25 +94,18 @@ export const GET = withRouteHandler( const buffers = await Promise.all(filesToZip.map((file) => fetchWorkspaceFileBuffer(file))) - // Assemble zip synchronously so path deduplication is deterministic. + // Entry paths stay workspace-root-relative so a mixed selection of folders and + // loose files keeps the layout the user sees in the files list. + const entryPaths = buildZipEntryPaths( + filesToZip.map((file) => ({ + name: file.name, + folderPath: file.folderId ? folderPaths.get(file.folderId) : null, + })) + ) + const zip = new JSZip() - const usedPaths = new Set() - for (let i = 0; i < filesToZip.length; i++) { - const file = filesToZip[i] - const buffer = buffers[i] - const folderPath = file.folderId ? folderPaths.get(file.folderId) : null - const basePath = - safeZipPath(folderPath ? `${folderPath}/${file.name}` : file.name) || - safeZipPath(file.name) || - file.id - let zipPath = basePath - let suffix = 2 - while (usedPaths.has(zipPath)) { - zipPath = withZipPathSuffix(basePath, suffix) - suffix++ - } - usedPaths.add(zipPath) - zip.file(zipPath, buffer) + for (const [index, buffer] of buffers.entries()) { + zip.file(entryPaths[index], buffer) } const zipBuffer = await zip.generateAsync({ type: 'nodebuffer' }) diff --git a/apps/sim/lib/uploads/zip-entry-path.test.ts b/apps/sim/lib/uploads/zip-entry-path.test.ts new file mode 100644 index 00000000000..afd84774217 --- /dev/null +++ b/apps/sim/lib/uploads/zip-entry-path.test.ts @@ -0,0 +1,117 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { buildZipEntryPaths } from '@/lib/uploads/zip-entry-path' + +describe('buildZipEntryPaths', () => { + it('mirrors the workspace folder layout', () => { + expect( + buildZipEntryPaths([ + { name: '00_Executive-Overview.docx', folderPath: 'Motherland-Campaign' }, + { name: 'hero-key-art.png', folderPath: 'Motherland-Campaign/visuals' }, + { name: 'notes.md', folderPath: null }, + ]) + ).toEqual([ + 'Motherland-Campaign/00_Executive-Overview.docx', + 'Motherland-Campaign/visuals/hero-key-art.png', + 'notes.md', + ]) + }) + + it('keeps same-named files in different folders apart', () => { + expect( + buildZipEntryPaths([ + { name: 'README.docx', folderPath: 'Evidence/01-Access-Control' }, + { name: 'README.docx', folderPath: 'Evidence/02-Awareness-Training' }, + ]) + ).toEqual([ + 'Evidence/01-Access-Control/README.docx', + 'Evidence/02-Awareness-Training/README.docx', + ]) + }) + + it('suffixes collisions before the extension without touching the directory', () => { + expect( + buildZipEntryPaths([ + { name: 'report.pdf', folderPath: 'docs' }, + { name: 'report.pdf', folderPath: 'docs' }, + { name: 'report.pdf', folderPath: 'docs' }, + { name: 'notes', folderPath: 'docs' }, + { name: 'notes', folderPath: 'docs' }, + ]) + ).toEqual([ + 'docs/report.pdf', + 'docs/report (1).pdf', + 'docs/report (2).pdf', + 'docs/notes', + 'docs/notes (1)', + ]) + }) + + it('drops the shared ancestor chain when rebasing', () => { + expect( + buildZipEntryPaths( + [ + { name: 'plan.docx', folderPath: 'Clients/Acme/Campaign' }, + { name: 'hero.png', folderPath: 'Clients/Acme/Campaign/visuals' }, + ], + { rebaseOnCommonFolder: true } + ) + ).toEqual(['plan.docx', 'visuals/hero.png']) + }) + + it('compares the shared ancestor segment by segment, not by string prefix', () => { + expect( + buildZipEntryPaths( + [ + { name: 'a.txt', folderPath: 'Reports' }, + { name: 'b.txt', folderPath: 'Reports-2026' }, + ], + { rebaseOnCommonFolder: true } + ) + ).toEqual(['Reports/a.txt', 'Reports-2026/b.txt']) + }) + + it('rebases to the archive root when every file shares one folder', () => { + expect( + buildZipEntryPaths( + [ + { name: 'a.txt', folderPath: 'Clients/Acme' }, + { name: 'b.txt', folderPath: 'Clients/Acme' }, + ], + { rebaseOnCommonFolder: true } + ) + ).toEqual(['a.txt', 'b.txt']) + }) + + it('keeps full paths when a root-level file joins the selection', () => { + expect( + buildZipEntryPaths( + [ + { name: 'a.txt', folderPath: 'Clients/Acme' }, + { name: 'b.txt', folderPath: null }, + ], + { rebaseOnCommonFolder: true } + ) + ).toEqual(['Clients/Acme/a.txt', 'b.txt']) + }) + + it('strips traversal and platform-illegal characters', () => { + expect( + buildZipEntryPaths([ + { name: '../../etc/passwd', folderPath: 'docs' }, + { name: 'C:\\Windows\\system.ini', folderPath: '../secrets' }, + { name: 'quarterly:report?.pdf', folderPath: 'q1/./q2' }, + ]) + ).toEqual(['docs/passwd', '_/secrets/system.ini', 'q1/_/q2/quarterly_report_.pdf']) + }) + + it('falls back to a usable name when nothing survives sanitization', () => { + expect(buildZipEntryPaths([{ name: '..', folderPath: null }])).toEqual(['file']) + }) + + it('returns no paths for an empty selection', () => { + expect(buildZipEntryPaths([], { rebaseOnCommonFolder: true })).toEqual([]) + }) +}) diff --git a/apps/sim/lib/uploads/zip-entry-path.ts b/apps/sim/lib/uploads/zip-entry-path.ts new file mode 100644 index 00000000000..08e64e0cb9e --- /dev/null +++ b/apps/sim/lib/uploads/zip-entry-path.ts @@ -0,0 +1,124 @@ +/** Characters that are illegal in file names on common desktop platforms. */ +const ILLEGAL_ENTRY_CHARS = /[<>:"\\|?*\x00-\x1f]/g + +/** A workspace file to place inside an archive. */ +export interface ZipEntrySource { + /** Display name of the file. Only its final path segment is used. */ + name: string + /** Workspace-root-relative folder path holding the file, or null when it sits at the root. */ + folderPath?: string | null +} + +export interface BuildZipEntryPathsOptions { + /** + * Drop the deepest folder path shared by every source, so an archive built from + * one folder's contents is not buried under that folder's ancestor chain. + */ + rebaseOnCommonFolder?: boolean +} + +/** Split a folder path into non-empty segments. */ +function toSegments(folderPath?: string | null): string[] { + return folderPath ? folderPath.split('/').filter(Boolean) : [] +} + +/** + * Reduce a name to a safe, single path segment: takes the final segment, drops + * directory and traversal components, and falls back when nothing usable remains. + * Keeps untrusted names from introducing nested or zip-slip-style paths. + */ +function toLeafName(name: string): string { + const leaf = name.replace(/\\/g, '/').split('/').pop()?.trim() + if (!leaf || leaf === '.' || leaf === '..') return 'file' + return leaf +} + +/** + * Sanitize a `/`-joined entry path segment by segment: strips characters that are + * illegal on common desktop platforms, neutralizes `.`/`..` traversal segments, and + * drops empty segments. Returns `''` when no usable segment remains. + */ +function safeEntryPath(path: string): string { + return path + .split('/') + .map((segment) => { + const cleaned = segment.trim().replace(ILLEGAL_ENTRY_CHARS, '_') + return cleaned === '.' || cleaned === '..' ? '_' : cleaned + }) + .filter(Boolean) + .join('/') +} + +/** + * Append a numeric suffix to the file name of an entry path, leaving the directory + * portion intact (`docs/report.pdf` -> `docs/report (1).pdf`). + */ +function withEntrySuffix(path: string, suffix: number): string { + const slashIndex = path.lastIndexOf('/') + const directory = path.slice(0, slashIndex + 1) + const fileName = path.slice(slashIndex + 1) + const dotIndex = fileName.lastIndexOf('.') + + return dotIndex > 0 + ? `${directory}${fileName.slice(0, dotIndex)} (${suffix})${fileName.slice(dotIndex)}` + : `${directory}${fileName} (${suffix})` +} + +/** + * Longest folder path shared by every source, compared segment by segment so that + * sibling folders like `Reports` and `Reports-2026` are never treated as nested. + * Returns `''` when the sources have no common folder. + */ +function commonFolderSegments(sources: ZipEntrySource[]): string[] { + let common: string[] | null = null + + for (const source of sources) { + const segments = toSegments(source.folderPath) + if (common === null) { + common = segments + continue + } + let matched = 0 + while ( + matched < common.length && + matched < segments.length && + common[matched] === segments[matched] + ) { + matched++ + } + common = common.slice(0, matched) + if (common.length === 0) break + } + + return common ?? [] +} + +/** + * Build unique, traversal-safe zip entry paths that mirror the workspace folder + * layout of `sources`. Returns one path per source, in the same order. + * + * Collisions get a numeric suffix before the extension, matching the workspace + * file naming convention (`report.pdf` -> `report (1).pdf`). + */ +export function buildZipEntryPaths( + sources: ZipEntrySource[], + options?: BuildZipEntryPathsOptions +): string[] { + const rebaseLength = options?.rebaseOnCommonFolder ? commonFolderSegments(sources).length : 0 + const usedPaths = new Set() + + return sources.map((source) => { + const leafName = toLeafName(source.name) + const folderSegments = toSegments(source.folderPath).slice(rebaseLength) + const basePath = safeEntryPath([...folderSegments, leafName].join('/')) || leafName + + let candidate = basePath + let suffix = 1 + while (usedPaths.has(candidate)) { + candidate = withEntrySuffix(basePath, suffix) + suffix++ + } + usedPaths.add(candidate) + return candidate + }) +} diff --git a/apps/sim/tools/file/compress.ts b/apps/sim/tools/file/compress.ts index 3dc28ef58d1..d2f236dc815 100644 --- a/apps/sim/tools/file/compress.ts +++ b/apps/sim/tools/file/compress.ts @@ -12,7 +12,7 @@ export const fileCompressTool: ToolConfig = { id: 'file_compress', name: 'File Compress', description: - 'Compress one or more workspace files into a single .zip archive stored in the workspace, for bundling files to download, transfer, or store.', + 'Compress one or more workspace files into a single .zip archive stored in the workspace, for bundling files to download, transfer, or store. Preserves the workspace folder structure of the selected files.', version: '1.0.0', params: {