Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/docs/content/docs/cli/files.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ sim files batch-delete [file-ids...] [options]

| Argument | Required | Description |
| --- | --- | --- |
| `file-ids...` | No | File identifiers to delete. Same as --file-ids. |
| `file-ids...` | No | Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths. Same as --file-ids. |

</CommandTable>

Expand All @@ -653,7 +653,7 @@ sim files batch-delete [file-ids...] [options]

| Option | Required | Description |
| --- | --- | --- |
| `--file-ids <value...>` | No | File identifiers to delete. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). |
| `--file-ids <value...>` | No | Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). |
| `-y, --yes` | Yes | Confirm this operation. |

</CommandTable>
Expand Down
4 changes: 2 additions & 2 deletions apps/docs/content/docs/cli/reference.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -981,7 +981,7 @@ sim files batch-delete [file-ids...] [options]

| Argument | Required | Description |
| --- | --- | --- |
| `file-ids...` | No | File identifiers to delete. Same as --file-ids. |
| `file-ids...` | No | Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths. Same as --file-ids. |

</CommandTable>

Expand All @@ -991,7 +991,7 @@ sim files batch-delete [file-ids...] [options]

| Option | Required | Description |
| --- | --- | --- |
| `--file-ids <value...>` | No | File identifiers to delete. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). |
| `--file-ids <value...>` | No | Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). |
| `-y, --yes` | Yes | Confirm this operation. |

</CommandTable>
Expand Down
2 changes: 1 addition & 1 deletion apps/docs/openapi-v2-files-audit.json
Original file line number Diff line number Diff line change
Expand Up @@ -7369,7 +7369,7 @@
"type": "string",
"minLength": 1
},
"description": "File identifiers to delete."
"description": "Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths."
}
},
"required": ["workspaceId", "fileIds"],
Expand Down
6 changes: 5 additions & 1 deletion apps/sim/app/api/v2/files/bulk-delete/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ export const POST = defineV2JsonRoute({
operation: fileOperations.delete,
rateLimit: v2RateLimits.publicApi,
errorPolicy: v2FileErrorPolicies.default,
mapInput: ({ body }) => ({ workspaceId: body.workspaceId, fileIds: body.fileIds }),
mapInput: ({ body }) => ({
workspaceId: body.workspaceId,
fileIds: body.fileIds,
acceptVfsPath: true,
}),
useCase: archiveWorkspaceFileItemsOperation,
present: ({ deletedItems }) => ({ data: { deletedItems: { files: deletedItems.files } } }),
})
9 changes: 7 additions & 2 deletions apps/sim/lib/api/contracts/v2/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ import { MAX_FOLDER_PATH_BYTES, MAX_FOLDER_PATH_SEGMENTS } from '@/lib/folders/p
import { MAX_WORKSPACE_FILE_SIZE } from '@/lib/uploads/shared/types'
import { MAX_TEXT_EXTRACTION_BYTES } from '@/lib/uploads/utils/file'
import { isWorkspaceFileVfsPath } from '@/lib/vfs/path'
import { MAX_ZIP_DOWNLOAD_FILES } from '@/lib/workspace-files/limits'
import {
MAX_WORKSPACE_FILE_BULK_REQUEST_PATHS,
MAX_ZIP_DOWNLOAD_FILES,
} from '@/lib/workspace-files/limits'
import {
FILE_SEARCH_DEFAULT_MAX_RESULTS,
FILE_SEARCH_MAX_QUERY_LENGTH,
Expand Down Expand Up @@ -527,7 +530,9 @@ export type V2MoveFileItemsResult = z.output<typeof v2MoveFileItemsResultSchema>
export const v2BulkDeleteFilesBodySchema = z
.object({
workspaceId: workspaceIdSchema.describe('Workspace containing the files.'),
fileIds: fileIdsSchema.describe('File identifiers to delete.'),
fileIds: fileIdsSchema.describe(
`Files to delete, each by identifier or by the VFS path of a live file: \`files/<folder>/<name>\`. Pass the id when two files in a folder have names that display alike. At most ${MAX_WORKSPACE_FILE_BULK_REQUEST_PATHS} entries may be paths.`
),
})
.strict()

Expand Down
37 changes: 37 additions & 0 deletions apps/sim/lib/workflows/editing/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,43 @@ describe('validateInputsForBlock', () => {
).toBe(false)
})

describe('file-upload values', () => {
const multiple = { id: 'attachmentFiles', type: 'file-upload' as const, multiple: true }
const single = { id: 'file', type: 'file-upload' as const }
const stored = {
id: 'wf_a',
name: 'brief.txt',
path: '/api/files/serve/workspace%2Fws%2Fbrief.txt?context=workspace',
key: 'workspace/ws/brief.txt',
size: 12,
type: 'text/plain',
}
const validate = (config: typeof multiple | typeof single, value: unknown) =>
validateValueForSubBlockType(config, value, config.id, 'agent', 'agent-1')

it('stores a multi-file field as the list of file objects the editor writes', () => {
const files = [stored, { ...stored, id: 'wf_b', name: 'notes.txt' }]
expect(validate(multiple, files)).toEqual({ valid: true, value: files })
})

it('clears a multi-file field given an empty list, as removing its last file does', () => {
expect(validate(multiple, [])).toEqual({ valid: true, value: null })
})

it('names the entry of a multi-file list that is not a stored file', () => {
const result = validate(multiple, [stored, { name: 'notes.txt' }])
expect(result.valid).toBe(false)
expect(result.error?.error).toContain('attachmentFiles[1]')
expect(validate(multiple, [stored, 'brief.txt']).valid).toBe(false)
})

it('keeps a single-file field to one file object', () => {
expect(validate(single, stored)).toEqual({ valid: true, value: stored })
expect(validate(single, [stored]).valid).toBe(false)
expect(validate(single, []).valid).toBe(false)
})
})

it('stores input-format rows with legacy or aliased types unchanged', () => {
const rows = [
{ name: 'payload', type: 'json' },
Expand Down
20 changes: 13 additions & 7 deletions apps/sim/lib/workflows/editing/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,11 @@ function validateFallbackModelEntry(item: any, index: number): string | null {
return null
}

/** A file object as the file-upload editor stores it: at least its name and serve path. */
function isStoredFile(value: unknown): boolean {
return isPlainRecord(value) && Boolean(value.name) && Boolean(value.path)
}

/**
* Validates a value against its expected subBlock type
* Returns validation result with the value or an error
Expand Down Expand Up @@ -609,30 +614,31 @@ export function validateValueForSubBlockType(
}

case 'file-upload': {
// File upload should be an object with specific properties or null
if (value === null) return { valid: true, value: null }
if (typeof value !== 'object') {
// Removing the editor's last file stores null, so an empty list clears the field too.
if (subBlockConfig.multiple && Array.isArray(value)) {
if (value.length === 0) return { valid: true, value: null }
const index = value.findIndex((file) => !isStoredFile(file))
if (index === -1) return { valid: true, value }
return {
valid: false,
error: {
blockId,
blockType,
field: fieldName,
value,
error: `Invalid file-upload value for field "${fieldName}" - expected object with name and path properties, or null`,
error: `Invalid file-upload entry ${fieldName}[${index}] - each file must be an object with "name" and "path" properties`,
},
}
}
// Validate file object has required properties
if (value && (!value.name || !value.path)) {
if (!isStoredFile(value)) {
return {
valid: false,
error: {
blockId,
blockType,
field: fieldName,
value,
error: `Invalid file-upload object for field "${fieldName}" - must have "name" and "path" properties`,
error: `Invalid file-upload value for field "${fieldName}" - expected ${subBlockConfig.multiple ? 'a list of objects' : 'one object'} with "name" and "path" properties, or null`,
},
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/**
* Real PostgreSQL behavior of single-file operations addressed by a `files/…` VFS path instead
* of an id: the path names exactly one live file in the asserted workspace, never a same-named
* file in another folder or workspace, a chat upload or an archived file, and authorization is
* unchanged. A caller who cannot reach the workspace learns nothing about the paths in it.
* Real PostgreSQL behavior of file operations addressed by a `files/…` VFS path instead of an id:
* the path names exactly one live file in the asserted workspace, never a same-named file in
* another folder or workspace, a chat upload or an archived file, and authorization is
* unchanged. A caller who cannot reach the workspace learns nothing about the paths in it, and a
* bulk delete with one unresolved path deletes nothing.
*/
import { db } from '@sim/db'
import { permissions, user, workspace, workspaceFiles } from '@sim/db/schema'
Expand All @@ -13,6 +14,7 @@ import { eq, inArray, sql } from 'drizzle-orm'
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/file-manager'
import { archiveWorkspaceFileItemsOperation } from '@/lib/workspace-files/application/archive-workspace-file-items'
import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file'
import { readWorkspaceFileMetadataWithVersion } from '@/lib/workspace-files/application/read-workspace-file-metadata'
import { renameWorkspaceFile } from '@/lib/workspace-files/application/rename-workspace-file'
Expand Down Expand Up @@ -64,6 +66,12 @@ const deleteFile = (fileId: string, userId = alice) =>
input: { fileId, acceptVfsPath: true, assertedWorkspaceId: workspaceId },
})

const bulkDelete = (fileIds: string[], userId = alice, acceptVfsPath = true) =>
archiveWorkspaceFileItemsOperation.execute({
principal: session(userId),
input: { workspaceId, fileIds, acceptVfsPath },
})

/** The `{ code, message }` each path is refused with, or null where it resolved. */
const refusalsFor = (
principal: Parameters<typeof readWorkspaceFileMetadataWithVersion.execute>[0]['principal'],
Expand Down Expand Up @@ -373,4 +381,84 @@ describe('single-file operations addressed by VFS path', () => {
})
expect((await row(ids.rootReport))?.deletedAt).toBeNull()
})

describe('bulk delete', () => {
const bulk = { folder: generateId(), one: '', two: '', three: '', rootOne: '' }

beforeAll(async () => {
await db.execute(sql`
INSERT INTO folder (id, name, user_id, workspace_id, resource_type, parent_id)
VALUES (${bulk.folder}, 'Bulk', ${alice}, ${workspaceId}, 'file', NULL)`)
bulk.one = await insertFile({ name: 'one.csv', folderId: bulk.folder })
bulk.two = await insertFile({ name: 'two.csv', folderId: bulk.folder })
bulk.three = await insertFile({ name: 'three.csv', folderId: bulk.folder })
bulk.rootOne = await insertFile({ name: 'one.csv' })
})

const live = async (ids: string[]) =>
Promise.all(ids.map(async (id) => (await row(id))?.deletedAt === null))

it('refuses the whole request when one path names no single live file', async () => {
await expect(
bulkDelete(['files/Bulk/three.csv', 'files/Bulk/missing.csv'])
).rejects.toMatchObject({
code: 'not_found',
message: 'No live file at "files/Bulk/missing.csv"',
})
await expect(
bulkDelete([bulk.three, 'files/Bulk/one.csv,files/Bulk/two.csv'])
).rejects.toMatchObject({
message: expect.stringContaining('pass each path as its own entry'),
})
await expect(bulkDelete([bulk.three, await ambiguousPath()])).rejects.toMatchObject({
code: 'validation',
})
await expect(bulkDelete([bulk.three, 'files/notes.txt'])).rejects.toMatchObject({
code: 'not_found',
})
expect(await live([bulk.one, bulk.two, bulk.three, ids.chatUpload])).toEqual([
true,
true,
true,
true,
])
})

it('keeps authorization and answers a non-member the same for every path', async () => {
await expect(bulkDelete(['files/Bulk/one.csv'], bob)).rejects.toMatchObject({
code: 'forbidden',
})
const refusal = (fileIds: string[]) =>
bulkDelete(fileIds, carol).then(
() => null,
(error: { code?: string; message?: string }) => ({
code: error.code,
message: error.message,
})
)
const existing = await refusal(['files/Bulk/one.csv', 'files/Bulk/two.csv'])
expect(existing).toMatchObject({ code: 'forbidden' })
expect(await refusal(['files/Bulk/one.csv', 'files/Bulk/missing.csv'])).toEqual(existing)
expect(await live([bulk.one, bulk.two])).toEqual([true, true])
})

it('resolves no path for a caller that did not opt in', async () => {
await expect(bulkDelete(['files/Bulk/one.csv'], alice, false)).rejects.toMatchObject({
code: 'not_found',
})
expect(await live([bulk.one])).toEqual([true])
})

it('deletes the files a mix of paths and ids names, and no same-named file elsewhere', async () => {
expect(
await bulkDelete(['files/Bulk/one.csv', bulk.two, 'files/Bulk/two.csv'])
).toMatchObject({ deletedItems: { files: 2 } })
expect(await live([bulk.one, bulk.two, bulk.three, bulk.rootOne])).toEqual([
false,
false,
true,
true,
])
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import {
import { auditMock, auditMockFns } from '@sim/testing/mocks/audit.mock'
import { realtimeNotifyMock, realtimeNotifyMockFns } from '@sim/testing/mocks/realtime-notify.mock'
import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock'
import {
workspaceFileManagerMock,
workspaceFileManagerMockFns,
} from '@sim/testing/mocks/workspace-file-manager.mock'
import {
workspaceUploadsMock,
workspaceUploadsMockFns,
Expand All @@ -16,6 +20,7 @@ const { events } = vi.hoisted(() => ({
}))

vi.mock('@/lib/uploads/contexts/workspace', () => workspaceUploadsMock)
vi.mock('@/lib/uploads/contexts/workspace/file-manager', () => workspaceFileManagerMock)
vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock)
vi.mock('@sim/audit', () => auditMock)
vi.mock('@/lib/realtime/notify', () => realtimeNotifyMock)
Expand All @@ -31,6 +36,7 @@ const mockResolvePermission = workspaceAuthzMockFns.mockResolveEffectiveWorkspac
const mockAssertItems = workspaceUploadsMockFns.mockAssertWorkspaceFileItemsBelongToWorkspace
const mockArchive = workspaceUploadsMockFns.mockBulkArchiveWorkspaceFileItems
const mockLoadContext = workspaceUploadsMockFns.mockLoadWorkspaceFileOperationContext
const mockResolveReference = workspaceFileManagerMockFns.mockResolveFileReference

describe('archiveWorkspaceFileItemsOperation', () => {
beforeEach(() => {
Expand All @@ -57,6 +63,10 @@ describe('archiveWorkspaceFileItemsOperation', () => {
fileIds: ['file-1'],
folderIds: [],
}))
mockResolveReference.mockImplementation(async (_owner: unknown, reference: string) => {
events.push(`lookup ${reference}`)
return reference === 'files/Docs/a.csv' ? { id: 'file-a' } : null
})
})

it('authorizes a delegated bulk selection without borrowing the first file scope', async () => {
Expand Down Expand Up @@ -151,4 +161,30 @@ describe('archiveWorkspaceFileItemsOperation', () => {
expect(events).toEqual(['resolve', 'authorize'])
expect(mockArchive).not.toHaveBeenCalled()
})

describe('with files/ paths', () => {
const deleteByReference = (fileIds: string[], acceptVfsPath = true) =>
archiveWorkspaceFileItemsOperation.execute({
principal: createSessionPrincipal(),
input: { workspaceId: 'ws-1', fileIds, acceptVfsPath },
})

it('looks up no path for a caller it refuses', async () => {
mockResolvePermission.mockImplementation(async () => {
events.push('authorize')
return 'read'
})
await expect(deleteByReference(['files/Docs/a.csv'])).rejects.toMatchObject({
code: 'forbidden',
})
expect(events).toEqual(['resolve', 'authorize'])
})

it('refuses more paths than one request resolves before looking any up', async () => {
await expect(
deleteByReference(Array.from({ length: 101 }, (_, index) => `files/f-${index}.csv`))
).rejects.toThrow('at most 100 file paths')
expect(mockResolveReference).not.toHaveBeenCalled()
})
})
})
Loading
Loading