Skip to content

Commit 4edc169

Browse files
waleedlatif1waleedlatif1
authored
fix(files): preserve nested folder structure when archiving workspace files (#5985)
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.
1 parent 334c7c8 commit 4edc169

5 files changed

Lines changed: 280 additions & 74 deletions

File tree

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

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
downloadServableFileFromStorage,
4242
} from '@/lib/uploads/utils/file-utils.server'
4343
import { docNotReadyResponse } from '@/lib/uploads/utils/servable-file-response'
44+
import { buildZipEntryPaths } from '@/lib/uploads/zip-entry-path'
4445
import { performMoveWorkspaceFileItems } from '@/lib/workspace-files/orchestration'
4546
import {
4647
assertActiveWorkspaceAccess,
@@ -175,37 +176,19 @@ const stripExtension = (name: string): string => {
175176
/**
176177
* Reduce an arbitrary name to a safe, flat file name: takes the final path
177178
* segment, drops directory and traversal components, and falls back when the
178-
* result would be empty or a dot segment. Used for zip entry names and the
179-
* compress archive name so untrusted input cannot introduce nested or
180-
* zip-slip-style paths.
179+
* result would be empty or a dot segment. Used for the compress archive name so
180+
* untrusted input cannot introduce nested or zip-slip-style paths.
181181
*/
182182
const toFlatFileName = (name: string, fallback: string): string => {
183183
const leaf = name.replace(/\\/g, '/').split('/').pop()?.trim()
184184
if (!leaf || leaf === '.' || leaf === '..') return fallback
185185
return leaf
186186
}
187187

188-
/**
189-
* Return a zip entry name unique within `usedNames`, appending a numeric suffix
190-
* before the extension on collision (e.g., "data.csv" -> "data (1).csv").
191-
*/
192-
const uniqueZipEntryName = (name: string, usedNames: Set<string>): string => {
193-
if (!usedNames.has(name)) {
194-
usedNames.add(name)
195-
return name
196-
}
197-
198-
const dot = name.lastIndexOf('.')
199-
const base = dot > 0 ? name.slice(0, dot) : name
200-
const ext = dot > 0 ? name.slice(dot) : ''
201-
let counter = 1
202-
let candidate = `${base} (${counter})${ext}`
203-
while (usedNames.has(candidate)) {
204-
counter += 1
205-
candidate = `${base} (${counter})${ext}`
206-
}
207-
usedNames.add(candidate)
208-
return candidate
188+
/** A file bound for a compress archive, paired with the workspace folder it lives in. */
189+
interface ArchiveEntry {
190+
file: UserFile
191+
folderPath: string | null
209192
}
210193

211194
const isLikelyTextBuffer = (buffer: Buffer): boolean => isUtf8(buffer) && !buffer.includes(0)
@@ -678,17 +661,27 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
678661
)
679662
}
680663

681-
const userFiles: UserFile[] = workspaceFiles
682-
.map((file) => workspaceFileToUserFile(file))
683-
.filter((file): file is NonNullable<ReturnType<typeof workspaceFileToUserFile>> =>
684-
Boolean(file)
685-
)
686-
.concat(selectedInputFiles)
664+
const workspaceEntries: ArchiveEntry[] = workspaceFiles.flatMap((file) => {
665+
const userFile = workspaceFileToUserFile(file)
666+
return userFile ? [{ file: userFile, folderPath: file?.folderPath ?? null }] : []
667+
})
668+
669+
// Picker/upload values carry no workspace folder, so they archive at the root.
670+
const archiveEntries = workspaceEntries.concat(
671+
selectedInputFiles.map((file) => ({ file, folderPath: null }))
672+
)
673+
const userFiles: UserFile[] = archiveEntries.map((entry) => entry.file)
674+
675+
// Mirror the workspace folder layout, dropping the ancestor chain the whole
676+
// selection shares so archiving one folder does not nest it under its parents.
677+
const entryPaths = buildZipEntryPaths(
678+
archiveEntries.map((entry) => ({ name: entry.file.name, folderPath: entry.folderPath })),
679+
{ rebaseOnCommonFolder: true }
680+
)
687681

688682
const zip = new JSZip()
689-
const usedNames = new Set<string>()
690683
let totalBytes = 0
691-
for (const userFile of userFiles) {
684+
for (const [index, userFile] of userFiles.entries()) {
692685
const denied = await assertToolFileAccess(userFile.key, userId, requestId, logger)
693686
if (denied) return denied
694687

@@ -707,7 +700,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
707700
{ status: 413 }
708701
)
709702
}
710-
zip.file(uniqueZipEntryName(toFlatFileName(userFile.name, 'file'), usedNames), buffer)
703+
zip.file(entryPaths[index], buffer)
711704
}
712705

713706
const zipBuffer = await zip.generateAsync({

‎apps/sim/app/api/workspaces/[id]/files/download/route.ts‎

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -14,34 +14,13 @@ import {
1414
listWorkspaceFiles,
1515
} from '@/lib/uploads/contexts/workspace'
1616
import { formatFileSize } from '@/lib/uploads/utils/file-utils'
17+
import { buildZipEntryPaths } from '@/lib/uploads/zip-entry-path'
1718
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'
1819

1920
const logger = createLogger('WorkspaceFilesDownloadAPI')
2021
const MAX_ZIP_DOWNLOAD_FILES = 100
2122
const MAX_ZIP_DOWNLOAD_BYTES = 250 * 1024 * 1024
2223

23-
function safeZipPath(path: string): string {
24-
return path
25-
.split('/')
26-
.map((segment) => {
27-
const cleaned = segment.trim().replace(/[<>:"\\|?*\x00-\x1f]/g, '_')
28-
return cleaned === '.' || cleaned === '..' ? '_' : cleaned
29-
})
30-
.filter(Boolean)
31-
.join('/')
32-
}
33-
34-
function withZipPathSuffix(path: string, suffix: number): string {
35-
const slashIndex = path.lastIndexOf('/')
36-
const directory = slashIndex >= 0 ? `${path.slice(0, slashIndex + 1)}` : ''
37-
const filename = slashIndex >= 0 ? path.slice(slashIndex + 1) : path
38-
const dotIndex = filename.lastIndexOf('.')
39-
40-
return dotIndex > 0
41-
? `${directory}${filename.slice(0, dotIndex)} (${suffix})${filename.slice(dotIndex)}`
42-
: `${directory}${filename} (${suffix})`
43-
}
44-
4524
function collectDescendantFolderIds(
4625
selectedFolderIds: string[],
4726
folders: Array<{ id: string; parentId: string | null }>
@@ -115,25 +94,18 @@ export const GET = withRouteHandler(
11594

11695
const buffers = await Promise.all(filesToZip.map((file) => fetchWorkspaceFileBuffer(file)))
11796

118-
// Assemble zip synchronously so path deduplication is deterministic.
97+
// Entry paths stay workspace-root-relative so a mixed selection of folders and
98+
// loose files keeps the layout the user sees in the files list.
99+
const entryPaths = buildZipEntryPaths(
100+
filesToZip.map((file) => ({
101+
name: file.name,
102+
folderPath: file.folderId ? folderPaths.get(file.folderId) : null,
103+
}))
104+
)
105+
119106
const zip = new JSZip()
120-
const usedPaths = new Set<string>()
121-
for (let i = 0; i < filesToZip.length; i++) {
122-
const file = filesToZip[i]
123-
const buffer = buffers[i]
124-
const folderPath = file.folderId ? folderPaths.get(file.folderId) : null
125-
const basePath =
126-
safeZipPath(folderPath ? `${folderPath}/${file.name}` : file.name) ||
127-
safeZipPath(file.name) ||
128-
file.id
129-
let zipPath = basePath
130-
let suffix = 2
131-
while (usedPaths.has(zipPath)) {
132-
zipPath = withZipPathSuffix(basePath, suffix)
133-
suffix++
134-
}
135-
usedPaths.add(zipPath)
136-
zip.file(zipPath, buffer)
107+
for (const [index, buffer] of buffers.entries()) {
108+
zip.file(entryPaths[index], buffer)
137109
}
138110

139111
const zipBuffer = await zip.generateAsync({ type: 'nodebuffer' })

‎apps/sim/lib/uploads/zip-entry-path.test.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { buildZipEntryPaths } from '@/lib/uploads/zip-entry-path'
6+
7+
describe('buildZipEntryPaths', () => {
8+
it('mirrors the workspace folder layout', () => {
9+
expect(
10+
buildZipEntryPaths([
11+
{ name: '00_Executive-Overview.docx', folderPath: 'Motherland-Campaign' },
12+
{ name: 'hero-key-art.png', folderPath: 'Motherland-Campaign/visuals' },
13+
{ name: 'notes.md', folderPath: null },
14+
])
15+
).toEqual([
16+
'Motherland-Campaign/00_Executive-Overview.docx',
17+
'Motherland-Campaign/visuals/hero-key-art.png',
18+
'notes.md',
19+
])
20+
})
21+
22+
it('keeps same-named files in different folders apart', () => {
23+
expect(
24+
buildZipEntryPaths([
25+
{ name: 'README.docx', folderPath: 'Evidence/01-Access-Control' },
26+
{ name: 'README.docx', folderPath: 'Evidence/02-Awareness-Training' },
27+
])
28+
).toEqual([
29+
'Evidence/01-Access-Control/README.docx',
30+
'Evidence/02-Awareness-Training/README.docx',
31+
])
32+
})
33+
34+
it('suffixes collisions before the extension without touching the directory', () => {
35+
expect(
36+
buildZipEntryPaths([
37+
{ name: 'report.pdf', folderPath: 'docs' },
38+
{ name: 'report.pdf', folderPath: 'docs' },
39+
{ name: 'report.pdf', folderPath: 'docs' },
40+
{ name: 'notes', folderPath: 'docs' },
41+
{ name: 'notes', folderPath: 'docs' },
42+
])
43+
).toEqual([
44+
'docs/report.pdf',
45+
'docs/report (1).pdf',
46+
'docs/report (2).pdf',
47+
'docs/notes',
48+
'docs/notes (1)',
49+
])
50+
})
51+
52+
it('drops the shared ancestor chain when rebasing', () => {
53+
expect(
54+
buildZipEntryPaths(
55+
[
56+
{ name: 'plan.docx', folderPath: 'Clients/Acme/Campaign' },
57+
{ name: 'hero.png', folderPath: 'Clients/Acme/Campaign/visuals' },
58+
],
59+
{ rebaseOnCommonFolder: true }
60+
)
61+
).toEqual(['plan.docx', 'visuals/hero.png'])
62+
})
63+
64+
it('compares the shared ancestor segment by segment, not by string prefix', () => {
65+
expect(
66+
buildZipEntryPaths(
67+
[
68+
{ name: 'a.txt', folderPath: 'Reports' },
69+
{ name: 'b.txt', folderPath: 'Reports-2026' },
70+
],
71+
{ rebaseOnCommonFolder: true }
72+
)
73+
).toEqual(['Reports/a.txt', 'Reports-2026/b.txt'])
74+
})
75+
76+
it('rebases to the archive root when every file shares one folder', () => {
77+
expect(
78+
buildZipEntryPaths(
79+
[
80+
{ name: 'a.txt', folderPath: 'Clients/Acme' },
81+
{ name: 'b.txt', folderPath: 'Clients/Acme' },
82+
],
83+
{ rebaseOnCommonFolder: true }
84+
)
85+
).toEqual(['a.txt', 'b.txt'])
86+
})
87+
88+
it('keeps full paths when a root-level file joins the selection', () => {
89+
expect(
90+
buildZipEntryPaths(
91+
[
92+
{ name: 'a.txt', folderPath: 'Clients/Acme' },
93+
{ name: 'b.txt', folderPath: null },
94+
],
95+
{ rebaseOnCommonFolder: true }
96+
)
97+
).toEqual(['Clients/Acme/a.txt', 'b.txt'])
98+
})
99+
100+
it('strips traversal and platform-illegal characters', () => {
101+
expect(
102+
buildZipEntryPaths([
103+
{ name: '../../etc/passwd', folderPath: 'docs' },
104+
{ name: 'C:\\Windows\\system.ini', folderPath: '../secrets' },
105+
{ name: 'quarterly:report?.pdf', folderPath: 'q1/./q2' },
106+
])
107+
).toEqual(['docs/passwd', '_/secrets/system.ini', 'q1/_/q2/quarterly_report_.pdf'])
108+
})
109+
110+
it('falls back to a usable name when nothing survives sanitization', () => {
111+
expect(buildZipEntryPaths([{ name: '..', folderPath: null }])).toEqual(['file'])
112+
})
113+
114+
it('returns no paths for an empty selection', () => {
115+
expect(buildZipEntryPaths([], { rebaseOnCommonFolder: true })).toEqual([])
116+
})
117+
})

0 commit comments

Comments
 (0)