Skip to content

Commit e13bd2f

Browse files
committed
address comments
1 parent 83a0ba8 commit e13bd2f

9 files changed

Lines changed: 217 additions & 105 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ const logger = createLogger('WorkspaceFilesDownloadAPI')
1717
function safeZipPath(path: string): string {
1818
return path
1919
.split('/')
20-
.map((segment) => segment.trim().replace(/[<>:"\\|?*\x00-\x1f]/g, '_'))
20+
.map((segment) => {
21+
const cleaned = segment.trim().replace(/[<>:"\\|?*\x00-\x1f]/g, '_')
22+
return cleaned === '.' || cleaned === '..' ? '_' : cleaned
23+
})
2124
.filter(Boolean)
2225
.join('/')
2326
}

apps/sim/app/workspace/[workspaceId]/files/files.tsx

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ export function Files() {
689689
)
690690

691691
async function uploadFiles(filesToUpload: File[], targetFolderId = currentFolderId) {
692-
if (!workspaceId || filesToUpload.length === 0) return
692+
if (!workspaceId || filesToUpload.length === 0 || !canEdit) return
693693

694694
const oversized: string[] = []
695695
const sizeFiltered = filesToUpload.filter((f) => {
@@ -904,14 +904,19 @@ export function Files() {
904904
}, [selectedFileIds, selectedFolderIds, currentFolderId])
905905

906906
const handleMoveSelected = useCallback(async () => {
907-
await moveItems.mutateAsync({
908-
workspaceId,
909-
fileIds: selectedFileIds,
910-
folderIds: selectedFolderIds,
911-
targetFolderId: moveTargetFolderId,
912-
})
913-
setSelectedRowIds(new Set())
914-
setShowMoveModal(false)
907+
try {
908+
await moveItems.mutateAsync({
909+
workspaceId,
910+
fileIds: selectedFileIds,
911+
folderIds: selectedFolderIds,
912+
targetFolderId: moveTargetFolderId,
913+
})
914+
setSelectedRowIds(new Set())
915+
setShowMoveModal(false)
916+
} catch (error) {
917+
logger.error('Failed to move selected items:', error)
918+
toast.error(error instanceof Error ? error.message : 'Failed to move selected items')
919+
}
915920
}, [workspaceId, selectedFileIds, selectedFolderIds, moveTargetFolderId, moveItems])
916921

917922
const fileDetailBreadcrumbs = useMemo(
@@ -1075,10 +1080,21 @@ export function Files() {
10751080

10761081
const handleContextMenuDownload = useCallback(() => {
10771082
const item = contextMenuItemRef.current
1078-
if (!item || item.kind !== 'file') return
1083+
if (!item) return
1084+
const rowId = item.kind === 'file' ? fileRowId(item.file.id) : folderRowId(item.folder.id)
1085+
if (selectedRowIds.has(rowId) && selectedRowIds.size > 1) {
1086+
handleBulkDownload()
1087+
closeContextMenu()
1088+
return
1089+
}
1090+
if (item.kind === 'folder') {
1091+
window.location.href = `/api/workspaces/${workspaceId}/files/download?folderIds=${encodeURIComponent(item.folder.id)}`
1092+
closeContextMenu()
1093+
return
1094+
}
10791095
handleDownload(item.file)
10801096
closeContextMenu()
1081-
}, [handleDownload, closeContextMenu])
1097+
}, [selectedRowIds, handleBulkDownload, closeContextMenu, workspaceId, handleDownload])
10821098

10831099
const handleContextMenuRename = useCallback(() => {
10841100
const item = contextMenuItemRef.current
@@ -1091,14 +1107,20 @@ export function Files() {
10911107
const handleContextMenuDelete = useCallback(() => {
10921108
const item = contextMenuItemRef.current
10931109
if (!item) return
1110+
const rowId = item.kind === 'file' ? fileRowId(item.file.id) : folderRowId(item.folder.id)
1111+
if (selectedRowIds.has(rowId) && selectedRowIds.size > 1) {
1112+
handleBulkDelete()
1113+
closeContextMenu()
1114+
return
1115+
}
10941116
setDeleteTarget(
10951117
item.kind === 'file'
10961118
? { fileIds: [item.file.id], folderIds: [], name: item.file.name }
10971119
: { fileIds: [], folderIds: [item.folder.id], name: item.folder.name, isFolder: true }
10981120
)
10991121
setShowDeleteConfirm(true)
11001122
closeContextMenu()
1101-
}, [closeContextMenu])
1123+
}, [selectedRowIds, handleBulkDelete, closeContextMenu])
11021124

11031125
const handleContentContextMenu = useCallback(
11041126
(e: React.MouseEvent) => {
@@ -1115,9 +1137,10 @@ export function Files() {
11151137
)
11161138

11171139
const handleListUploadFile = useCallback(() => {
1140+
if (!canEdit || uploading) return
11181141
fileInputRef.current?.click()
11191142
closeListContextMenu()
1120-
}, [closeListContextMenu])
1143+
}, [canEdit, uploading, closeListContextMenu])
11211144

11221145
const prevFileIdRef = useRef(fileIdFromRoute)
11231146
if (fileIdFromRoute !== prevFileIdRef.current) {
@@ -1279,8 +1302,9 @@ export function Files() {
12791302
)
12801303

12811304
const handleUploadClick = useCallback(() => {
1305+
if (!canEdit || uploading) return
12821306
fileInputRef.current?.click()
1283-
}, [])
1307+
}, [canEdit, uploading])
12841308

12851309
const searchConfig: SearchConfig = {
12861310
value: inputValue,
@@ -1310,6 +1334,7 @@ export function Files() {
13101334
label: uploadButtonLabel,
13111335
icon: Upload,
13121336
onClick: handleUploadClick,
1337+
disabled: uploading || !canEdit,
13131338
},
13141339
{
13151340
label: 'New folder',
@@ -1372,14 +1397,20 @@ export function Files() {
13721397
() => [
13731398
{ value: '__root__', label: 'Files' },
13741399
...folders
1375-
.filter((folder) => !selectedFolderIds.includes(folder.id))
1400+
.filter((folder) => {
1401+
if (selectedFolderIds.includes(folder.id)) return false
1402+
return selectedFolderIds.every(
1403+
(selectedFolderId) =>
1404+
!descendantFolderIdsByFolderId.get(selectedFolderId)?.has(folder.id)
1405+
)
1406+
})
13761407
.map((folder) => ({
13771408
value: folder.id,
13781409
label: folder.path,
13791410
icon: Folder,
13801411
})),
13811412
],
1382-
[folders, selectedFolderIds]
1413+
[folders, selectedFolderIds, descendantFolderIdsByFolderId]
13831414
)
13841415

13851416
const sortConfig: SortConfig = useMemo(
@@ -1734,7 +1765,7 @@ export function Files() {
17341765
type='file'
17351766
className='hidden'
17361767
onChange={handleFileChange}
1737-
disabled={uploading}
1768+
disabled={uploading || !canEdit}
17381769
accept={ACCEPT_ATTR}
17391770
multiple
17401771
/>

apps/sim/lib/api/contracts/workspace-file-folders.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,22 @@ const workspaceFileFoldersSuccessSchema = z.object({
3434
success: z.boolean(),
3535
})
3636

37+
const workspaceFileFolderNameSchema = z
38+
.string({ error: 'Name is required' })
39+
.trim()
40+
.min(1, 'Name is required')
41+
.refine(
42+
(name) => name !== '.' && name !== '..' && !name.includes('/') && !name.includes('\\'),
43+
'Name cannot contain path separators or dot segments'
44+
)
45+
3746
export const createWorkspaceFileFolderBodySchema = z.object({
38-
name: z.string({ error: 'Name is required' }).trim().min(1, 'Name is required'),
47+
name: workspaceFileFolderNameSchema,
3948
parentId: z.string().nullable().optional(),
4049
})
4150

4251
export const updateWorkspaceFileFolderBodySchema = z.object({
43-
name: z.string().trim().min(1, 'Name is required').optional(),
52+
name: workspaceFileFolderNameSchema.optional(),
4453
parentId: z.string().nullable().optional(),
4554
sortOrder: z.number().int().optional(),
4655
})

apps/sim/lib/api/contracts/workspace-files.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,17 @@ export const listWorkspaceFilesQuerySchema = z.object({
1515
scope: workspaceFileScopeSchema.default('active'),
1616
})
1717

18+
const workspaceFileNameSchema = z
19+
.string({ error: 'Name is required' })
20+
.trim()
21+
.min(1, 'Name is required')
22+
.refine(
23+
(name) => name !== '.' && name !== '..' && !name.includes('/') && !name.includes('\\'),
24+
'Name cannot contain path separators or dot segments'
25+
)
26+
1827
export const renameWorkspaceFileBodySchema = z.object({
19-
name: z
20-
.string({ error: 'Name is required' })
21-
.refine((name) => name.trim().length > 0, { message: 'Name is required' }),
28+
name: workspaceFileNameSchema,
2229
})
2330

2431
export const updateWorkspaceFileContentBodySchema = z.object({
@@ -166,7 +173,7 @@ export const workspaceFileCompiledCheckContract = defineRouteContract({
166173
})
167174

168175
export const workspacePresignedUploadBodySchema = z.object({
169-
fileName: z.string().min(1, 'fileName is required'),
176+
fileName: workspaceFileNameSchema,
170177
contentType: z.string().min(1, 'contentType is required'),
171178
fileSize: z.number().nonnegative('fileSize must be a non-negative number'),
172179
folderId: z.string().nullable().optional(),
@@ -203,7 +210,7 @@ export const workspacePresignedUploadContract = defineRouteContract({
203210

204211
export const registerWorkspaceFileBodySchema = z.object({
205212
key: z.string().min(1, 'key is required'),
206-
name: z.string().min(1, 'name is required'),
213+
name: workspaceFileNameSchema,
207214
contentType: z.string().min(1, 'contentType is required'),
208215
folderId: z.string().nullable().optional(),
209216
})

apps/sim/lib/copilot/tools/server/files/workspace-file.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,13 @@ export function inferContentType(fileName: string, explicitType?: string): strin
9797
export function validateFlatWorkspaceFileName(fileName: string): string | null {
9898
const trimmed = fileName.trim()
9999
if (!trimmed) return 'File name cannot be empty'
100-
if (trimmed.split('/').some((segment) => !segment.trim()))
100+
const segments = trimmed.split('/').map((segment) => segment.trim())
101+
if (segments.some((segment) => !segment)) {
101102
return 'File path cannot contain empty segments'
103+
}
104+
if (segments.some((segment) => segment === '.' || segment === '..' || segment.includes('\\'))) {
105+
return 'File path cannot contain dot segments or backslashes'
106+
}
102107
return null
103108
}
104109

apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
*/
44

55
import { describe, expect, it } from 'vitest'
6-
import { buildWorkspaceFileFolderPathMap } from './workspace-file-folder-manager'
6+
import {
7+
buildWorkspaceFileFolderPathMap,
8+
normalizeWorkspaceFileItemName,
9+
} from './workspace-file-folder-manager'
710

811
describe('workspace file folder paths', () => {
912
it('builds nested paths from parent relationships', () => {
@@ -17,4 +20,14 @@ describe('workspace file folder paths', () => {
1720
expect(paths.get('quarterly')).toBe('Reports/Quarterly')
1821
expect(paths.get('archive')).toBe('Archive')
1922
})
23+
24+
it('rejects names that would create ambiguous paths', () => {
25+
expect(normalizeWorkspaceFileItemName('Reports', 'Folder')).toBe('Reports')
26+
expect(() => normalizeWorkspaceFileItemName('A/B', 'Folder')).toThrow(
27+
'Folder name cannot contain path separators or dot segments'
28+
)
29+
expect(() => normalizeWorkspaceFileItemName('..', 'File')).toThrow(
30+
'File name cannot contain path separators or dot segments'
31+
)
32+
})
2033
})

0 commit comments

Comments
 (0)