Skip to content

Commit 06d7aaa

Browse files
committed
fix(zip-uploads): detect prior nested-only extractions in the re-extract guard
The already-extracted check only looked for files directly inside the archive root folder, so a zip whose entries are all nested (src/index.ts) left only subfolders there and a second extract slipped past the guard, duplicating the tree with ' (1)' suffixes. Extraction roots its whole tree at that folder, so a prior run always leaves a direct file OR a direct subfolder — check both.
1 parent 422d4ac commit 06d7aaa

2 files changed

Lines changed: 49 additions & 16 deletions

File tree

apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,25 @@ describe('executeMaterializeFile - extract operation', () => {
411411
expect(mockDecompress).not.toHaveBeenCalled()
412412
})
413413

414+
it('detects a prior nested-only extraction via subfolders, not just direct files', async () => {
415+
// A zip containing only nested entries (src/index.ts) leaves NO direct files
416+
// under the archive root — only subfolders. The guard must still refuse.
417+
mockFindUpload.mockResolvedValue(zipRow())
418+
mockFindFolder.mockResolvedValue('folder-existing')
419+
dbChainMockFns.limit.mockResolvedValueOnce([]) // no direct files
420+
dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'subfolder-1' }]) // but a subfolder tree
421+
422+
const result = await executeMaterializeFile(
423+
{ fileNames: ['bundle.zip'], operation: 'extract' },
424+
context
425+
)
426+
427+
expect(result.success).toBe(false)
428+
const output = result.output as { failed: Array<{ fileName: string; error: string }> }
429+
expect(output.failed[0].error).toContain('already extracted')
430+
expect(mockDecompress).not.toHaveBeenCalled()
431+
})
432+
414433
it('dedupes repeated fileNames so one call cannot double-extract', async () => {
415434
mockFindUpload.mockResolvedValue(zipRow())
416435
mockFetchBuffer.mockResolvedValue(Buffer.from('zip-bytes'))

apps/sim/lib/copilot/tools/handlers/materialize-file.ts

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
22
import { db } from '@sim/db'
3-
import { workflow, workspaceFiles } from '@sim/db/schema'
3+
import { workflow, workspaceFileFolder, workspaceFiles } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { getErrorMessage, toError } from '@sim/utils/errors'
66
import { generateId } from '@sim/utils/id'
@@ -312,7 +312,6 @@ function archiveFolderBaseName(displayName: string): string {
312312
const stripped = displayName
313313
.replace(/\.zip$/i, '')
314314
.normalize('NFC')
315-
// biome-ignore lint/suspicious/noControlCharactersInRegex: intentionally stripping control chars
316315
.replace(/[\x00-\x1f\x7f]/g, '')
317316
.replace(/[/\\]/g, '-')
318317
.trim()
@@ -379,25 +378,40 @@ async function executeExtract(
379378
const folderPath = `files/${encodeVfsPathSegments([baseName])}`
380379

381380
// Re-running extract must not silently duplicate the tree with " (1)"-suffixed
382-
// copies: when the destination folder already holds files, report it as
383-
// already extracted instead of extracting beside the previous run.
381+
// copies: when the destination folder already holds content, report it as
382+
// already extracted instead of extracting beside the previous run. Direct
383+
// files AND direct subfolders both count — extraction roots its whole tree
384+
// here, so a prior run of a nested-only zip (e.g. src/index.ts) leaves a
385+
// subfolder even when no file sits at the top level.
384386
const existingFolderId = await findWorkspaceFileFolderIdByPath(workspaceId, [baseName])
385387
if (existingFolderId) {
386-
const [existingFile] = await db
387-
.select({ id: workspaceFiles.id })
388-
.from(workspaceFiles)
389-
.where(
390-
and(
391-
eq(workspaceFiles.folderId, existingFolderId),
392-
eq(workspaceFiles.context, 'workspace'),
393-
isNull(workspaceFiles.deletedAt)
388+
const [[existingFile], [existingSubfolder]] = await Promise.all([
389+
db
390+
.select({ id: workspaceFiles.id })
391+
.from(workspaceFiles)
392+
.where(
393+
and(
394+
eq(workspaceFiles.folderId, existingFolderId),
395+
eq(workspaceFiles.context, 'workspace'),
396+
isNull(workspaceFiles.deletedAt)
397+
)
394398
)
395-
)
396-
.limit(1)
397-
if (existingFile) {
399+
.limit(1),
400+
db
401+
.select({ id: workspaceFileFolder.id })
402+
.from(workspaceFileFolder)
403+
.where(
404+
and(
405+
eq(workspaceFileFolder.parentId, existingFolderId),
406+
isNull(workspaceFileFolder.deletedAt)
407+
)
408+
)
409+
.limit(1),
410+
])
411+
if (existingFile || existingSubfolder) {
398412
return {
399413
success: false,
400-
error: `"${fileName}" appears to be already extracted — ${folderPath}/ exists and contains files. List them with glob("${folderPath}/**"). To re-extract, delete that folder first.`,
414+
error: `"${fileName}" appears to be already extracted — ${folderPath}/ exists and contains content. List it with glob("${folderPath}/**"). To re-extract, delete that folder first.`,
401415
}
402416
}
403417
}

0 commit comments

Comments
 (0)