diff --git a/apps/docs/content/docs/cli/files.mdx b/apps/docs/content/docs/cli/files.mdx index 597b8fbb2d8..a6fc46dd930 100644 --- a/apps/docs/content/docs/cli/files.mdx +++ b/apps/docs/content/docs/cli/files.mdx @@ -102,7 +102,7 @@ sim files describe [options] | Argument | Required | Description | | --- | --- | --- | -| `fileId` | Yes | File identifier. | +| `fileId` | Yes | File identifier, or the file’s VFS path: `files//`. | @@ -303,7 +303,7 @@ sim files rename [options] | Argument | Required | Description | | --- | --- | --- | -| `fileId` | Yes | File identifier. | +| `fileId` | Yes | File identifier, or the file’s VFS path: `files//`. | @@ -613,7 +613,7 @@ sim files delete [options] | Argument | Required | Description | | --- | --- | --- | -| `fileId` | Yes | File identifier. | +| `fileId` | Yes | File identifier, or the file’s VFS path: `files//`. | diff --git a/apps/docs/content/docs/cli/reference.mdx b/apps/docs/content/docs/cli/reference.mdx index df99c0861bf..9648f138e13 100644 --- a/apps/docs/content/docs/cli/reference.mdx +++ b/apps/docs/content/docs/cli/reference.mdx @@ -1171,7 +1171,7 @@ sim files delete [options] | Argument | Required | Description | | --- | --- | --- | -| `fileId` | Yes | File identifier. | +| `fileId` | Yes | File identifier, or the file’s VFS path: `files//`. | @@ -1416,7 +1416,7 @@ sim files describe [options] | Argument | Required | Description | | --- | --- | --- | -| `fileId` | Yes | File identifier. | +| `fileId` | Yes | File identifier, or the file’s VFS path: `files//`. | @@ -1581,7 +1581,7 @@ sim files rename [options] | Argument | Required | Description | | --- | --- | --- | -| `fileId` | Yes | File identifier. | +| `fileId` | Yes | File identifier, or the file’s VFS path: `files//`. | diff --git a/apps/docs/openapi-v2-files-audit.json b/apps/docs/openapi-v2-files-audit.json index ec710bef7bc..85d2765c10a 100644 --- a/apps/docs/openapi-v2-files-audit.json +++ b/apps/docs/openapi-v2-files-audit.json @@ -2151,13 +2151,12 @@ "name": "fileId", "in": "path", "required": true, - "description": "File identifier.", + "description": "File identifier, or the file’s VFS path: `files//`.", "schema": { "type": "string", "minLength": 1, - "maxLength": 128, - "pattern": "^[A-Za-z0-9_-]+$", - "description": "File identifier." + "maxLength": 4096, + "description": "File identifier, or the file’s VFS path: `files//`." } }, { @@ -2238,13 +2237,12 @@ "name": "fileId", "in": "path", "required": true, - "description": "File identifier.", + "description": "File identifier, or the file’s VFS path: `files//`.", "schema": { "type": "string", "minLength": 1, - "maxLength": 128, - "pattern": "^[A-Za-z0-9_-]+$", - "description": "File identifier." + "maxLength": 4096, + "description": "File identifier, or the file’s VFS path: `files//`." } } ], @@ -2489,13 +2487,12 @@ "name": "fileId", "in": "path", "required": true, - "description": "File identifier.", + "description": "File identifier, or the file’s VFS path: `files//`.", "schema": { "type": "string", "minLength": 1, - "maxLength": 128, - "pattern": "^[A-Za-z0-9_-]+$", - "description": "File identifier." + "maxLength": 4096, + "description": "File identifier, or the file’s VFS path: `files//`." } }, { diff --git a/apps/sim/lib/api/contracts/v2/files.ts b/apps/sim/lib/api/contracts/v2/files.ts index 113bad59775..326864468d3 100644 --- a/apps/sim/lib/api/contracts/v2/files.ts +++ b/apps/sim/lib/api/contracts/v2/files.ts @@ -45,6 +45,7 @@ import { import { MAX_FOLDER_PATH_BYTES, MAX_FOLDER_PATH_SEGMENTS } from '@/lib/folders/paths' 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 { FILE_SEARCH_DEFAULT_MAX_RESULTS, @@ -302,6 +303,17 @@ export const v2FileReferenceParamsSchema = z.object({ export type V2FileReferenceParams = z.output +/** Also takes the `files/…` path `glob` and listings print, so no listing round-trip finds the id. */ +const v2FileIdOrPathParamsSchema = z.object({ + fileId: requiredFieldSchema('File reference is required') + .max(MAX_FOLDER_PATH_BYTES, 'File reference is too long') + .refine( + (value) => isWorkspaceFileVfsPath(value) || workspaceFileIdSchema.safeParse(value).success, + 'Invalid file id or path: pass a file id or its `files//` VFS path' + ) + .describe('File identifier, or the file’s VFS path: `files//`.'), +}) + export const v2CreateFileBodySchema = z .object({ workspaceId: workspaceIdSchema.describe('Workspace in which to create the file.'), @@ -943,7 +955,7 @@ export const v2ReadFileTextContract = defineRouteContract({ export const v2GetFileContract = defineRouteContract({ method: 'GET', path: '/api/v2/files/[fileId]/metadata', - params: v2FileParamsSchema, + params: v2FileIdOrPathParamsSchema, query: v2GetFileMetadataQuerySchema, response: { mode: 'json', @@ -955,7 +967,7 @@ export const v2RenameFileContract = defineRouteContract({ method: 'PATCH', path: '/api/v2/files/[fileId]', query: noInputSchema, - params: v2FileParamsSchema, + params: v2FileIdOrPathParamsSchema, body: v2RenameFileBodySchema, response: { mode: 'json', @@ -966,7 +978,7 @@ export const v2RenameFileContract = defineRouteContract({ export const v2DeleteFileContract = defineRouteContract({ method: 'DELETE', path: '/api/v2/files/[fileId]', - params: v2FileParamsSchema, + params: v2FileIdOrPathParamsSchema, query: v2FileWorkspaceQuerySchema, response: { mode: 'json', diff --git a/apps/sim/lib/uploads/contexts/workspace/file-manager.ts b/apps/sim/lib/uploads/contexts/workspace/file-manager.ts index c0b6c9078f4..c905e6d3a6e 100644 --- a/apps/sim/lib/uploads/contexts/workspace/file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/file-manager.ts @@ -2342,6 +2342,40 @@ async function getWorkspaceFileByExactReference( return folderId ? getWorkspaceFileByName(workspaceId, segments.at(-1) ?? '', { folderId }) : null } +/** + * The live workspace file whose canonical `files/…` path — as listings print it — is `vfsPath`, + * or null. Unlike {@link findWorkspaceFileRecord} there is no virtual `/content` or `/meta.json` + * suffix and no leaf-name fallback, so a mutation addressed by path lands only on the file it + * names. Two names that display alike are ambiguous and refused. Throws `VfsPathError` if + * malformed. + */ +export async function getWorkspaceFileByVfsPath( + workspaceId: string, + vfsPath: string +): Promise { + const segments = decodeVfsReferenceSegments(vfsPath.slice('files/'.length)) + const leaf = segments.at(-1) + if (leaf === undefined) return null + const pathKey = segments.map(normalizeVfsSegment).join('/') + const candidates = await listWorkspaceFileCandidates(workspaceId, { + ids: [], + leafNames: [normalizeDisplaySegment(leaf)], + }) + const matches = candidates.filter( + (file) => + canonicalWorkspaceFilePath({ folderPath: file.folderPath, name: file.name }).slice( + 'files/'.length + ) === pathKey + ) + if (matches.length > 1) { + throw new OrchestrationError( + 'validation', + `"${vfsPath}" names more than one file whose names display alike; pass the file id instead` + ) + } + return matches[0] ?? null +} + /** * Resolve a workspace file record from either its id or a VFS/name reference. * diff --git a/apps/sim/lib/vfs/path.ts b/apps/sim/lib/vfs/path.ts index 0e3b0bd30e5..a77037db694 100644 --- a/apps/sim/lib/vfs/path.ts +++ b/apps/sim/lib/vfs/path.ts @@ -10,6 +10,11 @@ export class VfsPathError extends Error { } } +/** A `files/…` workspace-file VFS path. File ids are drawn from `[A-Za-z0-9_-]`, so no id is one. */ +export function isWorkspaceFileVfsPath(reference: string): boolean { + return reference.startsWith('files/') +} + /** * The name a VFS path segment displays: NFC-composed, trimmed, control characters removed, * whitespace runs collapsed. Its SQL twin is `displaySegmentKey` in `@sim/db/schema`, which an diff --git a/apps/sim/lib/workspace-files/__integration__/file-path-references.integration.ts b/apps/sim/lib/workspace-files/__integration__/file-path-references.integration.ts new file mode 100644 index 00000000000..8497d72627f --- /dev/null +++ b/apps/sim/lib/workspace-files/__integration__/file-path-references.integration.ts @@ -0,0 +1,233 @@ +/** + * 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. + */ +import { db } from '@sim/db' +import { permissions, user, workspace, workspaceFiles } from '@sim/db/schema' +import { assertDisposableTestDatabaseUrl } from '@sim/db/testing/test-infrastructure' +import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' +import { generateId } from '@sim/utils/id' +import { eq, inArray, sql } from 'drizzle-orm' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/file-manager' +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' + +const alice = generateId() +const bob = generateId() +const carol = generateId() +const workspaceId = generateId() +const otherWorkspaceId = generateId() +const folders = { docs: generateId(), media: generateId(), foreign: generateId() } +const session = (userId: string) => ({ kind: 'session' as const, userId, sessionId: generateId() }) + +async function insertFile(input: { + name: string + folderId?: string | null + workspaceId?: string + context?: string + deleted?: boolean +}): Promise { + const id = `wf_${generateId()}` + const owner = input.workspaceId ?? workspaceId + await db.execute(sql` + INSERT INTO ${workspaceFiles} (id, key, user_id, workspace_id, folder_id, context, original_name, + content_type, size_bytes, uploaded_at, updated_at, deleted_at) + VALUES (${id}, ${`workspace/${owner}/${generateId()}-${input.name}`}, ${alice}, ${owner}, + ${input.folderId ?? null}, ${input.context ?? 'workspace'}, ${input.name}, 'text/csv', 10, + now(), now(), ${input.deleted ? sql`now()` : sql`NULL`})`) + return id +} + +async function row(id: string) { + const [file] = await db + .select({ name: workspaceFiles.originalName, deletedAt: workspaceFiles.deletedAt }) + .from(workspaceFiles) + .where(eq(workspaceFiles.id, id)) + return file +} + +const describeFile = (fileId: string, userId = alice, assertedWorkspaceId = workspaceId) => + readWorkspaceFileMetadataWithVersion.execute({ + principal: session(userId), + input: { fileId, assertedWorkspaceId }, + }) + +const deleteFile = (fileId: string, userId = alice) => + deleteWorkspaceFileOperation.execute({ + principal: session(userId), + input: { fileId, assertedWorkspaceId: workspaceId }, + }) + +describe('single-file operations addressed by VFS path', () => { + const ids = { + rootReport: '', + docsReport: '', + mediaReport: '', + foreignReport: '', + chatUpload: '', + archived: '', + docsContent: '', + docsOnly: '', + spaced: '', + alikeSpaced: '', + alikeSingle: '', + } + + beforeAll(async () => { + assertDisposableTestDatabaseUrl(process.env.DATABASE_URL ?? '') + await db.insert(user).values( + [alice, bob, carol].map((id) => ({ + id, + name: `Fixture ${id}`, + email: `${id}@fixture.test`, + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + })) + ) + await insertWorkspaceFixture(db, [ + { id: workspaceId, name: 'Path fixture', ownerId: alice, billedAccountUserId: alice }, + { id: otherWorkspaceId, name: 'Other fixture', ownerId: alice, billedAccountUserId: alice }, + ]) + await db.insert(permissions).values([ + { + id: generateId(), + userId: alice, + entityType: 'workspace', + entityId: workspaceId, + permissionType: 'admin', + }, + { + id: generateId(), + userId: bob, + entityType: 'workspace', + entityId: workspaceId, + permissionType: 'read', + }, + { + id: generateId(), + userId: alice, + entityType: 'workspace', + entityId: otherWorkspaceId, + permissionType: 'admin', + }, + ]) + await db.execute(sql` + INSERT INTO folder (id, name, user_id, workspace_id, resource_type, parent_id) VALUES + (${folders.docs}, 'Docs', ${alice}, ${workspaceId}, 'file', NULL), + (${folders.media}, 'Media', ${alice}, ${workspaceId}, 'file', NULL), + (${folders.foreign}, 'Docs', ${alice}, ${otherWorkspaceId}, 'file', NULL)`) + ids.rootReport = await insertFile({ name: 'report.csv' }) + ids.docsReport = await insertFile({ name: 'report.csv', folderId: folders.docs }) + ids.mediaReport = await insertFile({ name: 'report.csv', folderId: folders.media }) + ids.foreignReport = await insertFile({ + name: 'report.csv', + folderId: folders.foreign, + workspaceId: otherWorkspaceId, + }) + ids.chatUpload = await insertFile({ name: 'notes.txt', context: 'mothership' }) + ids.archived = await insertFile({ name: 'old.csv', deleted: true }) + ids.docsContent = await insertFile({ name: 'content', folderId: folders.docs }) + ids.docsOnly = await insertFile({ name: 'only-here.csv', folderId: folders.docs }) + ids.spaced = await insertFile({ name: 'my notes.csv' }) + ids.alikeSpaced = await insertFile({ name: 'a b.csv', folderId: folders.media }) + ids.alikeSingle = await insertFile({ name: 'a b.csv', folderId: folders.media }) + }) + + afterAll(async () => { + await deleteWorkspaceFixture(db, inArray(workspace.id, [workspaceId, otherWorkspaceId])) + await db.delete(user).where(inArray(user.id, [alice, bob, carol])) + }) + + it('describes the one file at the exact path, not a same-named file elsewhere', async () => { + expect((await describeFile('files/Docs/report.csv')).file.id).toBe(ids.docsReport) + expect((await describeFile('files/report.csv')).file.id).toBe(ids.rootReport) + await expect(describeFile('files/Archive/report.csv')).rejects.toMatchObject({ + code: 'not_found', + }) + await expect(describeFile('files/Docs')).rejects.toMatchObject({ code: 'not_found' }) + await expect(describeFile('files/only-here.csv')).rejects.toMatchObject({ + code: 'not_found', + }) + }) + + it('treats a trailing `content` or `meta.json` as a name, never as the parent file', async () => { + expect((await describeFile('files/Docs/content')).file.id).toBe(ids.docsContent) + await expect(deleteFile('files/report.csv/content')).rejects.toMatchObject({ + code: 'not_found', + }) + await expect(deleteFile('files/report.csv/meta.json')).rejects.toMatchObject({ + code: 'not_found', + }) + expect((await row(ids.rootReport))?.deletedAt).toBeNull() + }) + + it('resolves the path a listing prints for a name it displays normalized', async () => { + const listed = workspaceFileVfsPath((await describeFile(ids.spaced)).file) + expect((await describeFile(listed)).file.id).toBe(ids.spaced) + }) + + it('refuses a path two display-alike names share instead of picking one', async () => { + const listed = workspaceFileVfsPath((await describeFile(ids.alikeSingle)).file) + await expect(deleteFile(listed)).rejects.toMatchObject({ code: 'validation' }) + for (const id of [ids.alikeSpaced, ids.alikeSingle]) { + expect((await row(id))?.deletedAt).toBeNull() + } + }) + + it('renames and deletes only the file the path names', async () => { + const renamed = await renameWorkspaceFile.execute({ + principal: session(alice), + input: { fileId: 'files/Docs/report.csv', assertedWorkspaceId: workspaceId, name: 'q3.csv' }, + }) + expect(renamed.file.id).toBe(ids.docsReport) + expect((await row(ids.docsReport))?.name).toBe('q3.csv') + + expect(await deleteFile('files/Media/report.csv')).toMatchObject({ + id: ids.mediaReport, + deleted: true, + }) + expect((await row(ids.mediaReport))?.deletedAt).not.toBeNull() + + for (const untouched of [ids.rootReport, ids.foreignReport]) { + expect(await row(untouched)).toMatchObject({ name: 'report.csv', deletedAt: null }) + } + }) + + it('resolves a path only inside the asserted workspace', async () => { + await expect( + describeFile('files/Docs/report.csv', alice, otherWorkspaceId) + ).resolves.toMatchObject({ file: { id: ids.foreignReport } }) + await expect( + readWorkspaceFileMetadataWithVersion.execute({ + principal: session(alice), + input: { fileId: 'files/report.csv' }, + }) + ).rejects.toMatchObject({ code: 'not_found' }) + }) + + it('never reaches a chat upload or an archived file through a path', async () => { + await expect(deleteFile('files/notes.txt')).rejects.toMatchObject({ code: 'not_found' }) + expect((await row(ids.chatUpload))?.deletedAt).toBeNull() + await expect(describeFile('files/old.csv')).rejects.toMatchObject({ code: 'not_found' }) + }) + + it('keeps authorization: a read member cannot delete and a non-member cannot read', async () => { + await expect(deleteFile('files/report.csv', bob)).rejects.toMatchObject({ code: 'forbidden' }) + await expect(describeFile('files/report.csv', carol)).rejects.toMatchObject({ + code: 'forbidden', + }) + expect((await row(ids.rootReport))?.deletedAt).toBeNull() + }) + + it('rejects a dot-segment path instead of walking out of its folder', async () => { + await expect(deleteFile('files/Docs/../report.csv')).rejects.toMatchObject({ + code: 'validation', + }) + expect((await row(ids.rootReport))?.deletedAt).toBeNull() + }) +}) diff --git a/apps/sim/lib/workspace-files/application/file-context.ts b/apps/sim/lib/workspace-files/application/file-context.ts index a9e46b5a492..67d82c8ed23 100644 --- a/apps/sim/lib/workspace-files/application/file-context.ts +++ b/apps/sim/lib/workspace-files/application/file-context.ts @@ -3,14 +3,17 @@ import { OrchestrationError } from '@/lib/core/orchestration/types' import type { DbOrTx } from '@/lib/db/types' import { type ActiveWorkspaceFileContext, + getWorkspaceFileByVfsPath, loadActiveWorkspaceFileContext, loadWorkspaceFileLifecycleContext, type WorkspaceFileLifecycleContext, } from '@/lib/uploads/contexts/workspace/file-manager' +import { isWorkspaceFileVfsPath, VfsPathError } from '@/lib/vfs/path' import { ownedFileKind } from '@/lib/workspace-files/owned-files' import { matchesFileOwner } from '@/lib/workspace-files/ownership' export interface WorkspaceFileContextInput { + /** A file id, or — with `assertedWorkspaceId` — the file's exact `files/…` VFS path. */ fileId: string assertedWorkspaceId?: string includeDeleted?: boolean @@ -74,11 +77,30 @@ function requireWorkspaceFileContext( return canonical } +/** A path becomes an id first, so the canonical load and every authorization check run unchanged. */ +async function resolveFileId(input: WorkspaceFileContextInput): Promise { + if (!isWorkspaceFileVfsPath(input.fileId)) return input.fileId + if (input.assertedWorkspaceId === undefined) { + throw new OrchestrationError('not_found', 'File not found') + } + const file = await getWorkspaceFileByVfsPath(input.assertedWorkspaceId, input.fileId).catch( + (error: unknown) => { + if (!(error instanceof VfsPathError)) throw error + throw new OrchestrationError( + 'validation', + `"${input.fileId}" is not a valid workspace file path: ${error.message}` + ) + } + ) + if (!file) throw new OrchestrationError('not_found', 'File not found') + return file.id +} + export async function resolveActiveWorkspaceFileContext( input: WorkspaceFileContextInput ): Promise { const canonical = requireWorkspaceFileContext( - await loadActiveWorkspaceFileContext(input.fileId, { + await loadActiveWorkspaceFileContext(await resolveFileId(input), { includeDeleted: input.includeDeleted, includeChatUploads: input.includeChatUploads, includeOwnedFiles: Boolean(input.ownedFilePrincipal), diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 73865f365d0..73a894aa959 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -17680,7 +17680,7 @@ export const V2_OPERATIONS = { method: 'DELETE', path: '/api/v2/files/[fileId]', pathParams: ['fileId'] as const, - pathParamDocs: { fileId: 'File identifier.' }, + pathParamDocs: { fileId: 'File identifier, or the file’s VFS path: `files//`.' }, responseMode: 'json', summary: 'Delete file', query: { @@ -18673,7 +18673,7 @@ export const V2_OPERATIONS = { method: 'GET', path: '/api/v2/files/[fileId]/metadata', pathParams: ['fileId'] as const, - pathParamDocs: { fileId: 'File identifier.' }, + pathParamDocs: { fileId: 'File identifier, or the file’s VFS path: `files//`.' }, responseMode: 'json', summary: 'Get file metadata', query: { @@ -22959,7 +22959,7 @@ export const V2_OPERATIONS = { method: 'PATCH', path: '/api/v2/files/[fileId]', pathParams: ['fileId'] as const, - pathParamDocs: { fileId: 'File identifier.' }, + pathParamDocs: { fileId: 'File identifier, or the file’s VFS path: `files//`.' }, responseMode: 'json', summary: 'Rename file', body: {