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 index 8497d72627f..d5516584d40 100644 --- a/apps/sim/lib/workspace-files/__integration__/file-path-references.integration.ts +++ b/apps/sim/lib/workspace-files/__integration__/file-path-references.integration.ts @@ -2,7 +2,7 @@ * 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. + * unchanged. A caller who cannot reach the workspace learns nothing about the paths in it. */ import { db } from '@sim/db' import { permissions, user, workspace, workspaceFiles } from '@sim/db/schema' @@ -62,6 +62,25 @@ const deleteFile = (fileId: string, userId = alice) => input: { fileId, assertedWorkspaceId: workspaceId }, }) +/** The `{ code, message }` each path is refused with, or null where it resolved. */ +const refusalsFor = ( + principal: Parameters[0]['principal'], + paths: string[] +) => + Promise.all( + paths.map((fileId) => + readWorkspaceFileMetadataWithVersion + .execute({ principal, input: { fileId, assertedWorkspaceId: workspaceId } }) + .then( + () => null, + (error: { code?: string; message?: string }) => ({ + code: error.code, + message: error.message, + }) + ) + ) + ) + describe('single-file operations addressed by VFS path', () => { const ids = { rootReport: '', @@ -77,6 +96,8 @@ describe('single-file operations addressed by VFS path', () => { alikeSingle: '', } + const ambiguousPath = async () => workspaceFileVfsPath((await describeFile(ids.alikeSingle)).file) + beforeAll(async () => { assertDisposableTestDatabaseUrl(process.env.DATABASE_URL ?? '') await db.insert(user).values( @@ -224,6 +245,48 @@ describe('single-file operations addressed by VFS path', () => { expect((await row(ids.rootReport))?.deletedAt).toBeNull() }) + it('answers a non-member the same for an ambiguous, a missing and an existing path', async () => { + const outcomes = await refusalsFor(session(carol), [ + await ambiguousPath(), + 'files/missing.csv', + 'files/report.csv', + ]) + expect(outcomes[0]).toMatchObject({ code: 'forbidden' }) + expect(outcomes[1]).toEqual(outcomes[0]) + expect(outcomes[2]).toEqual(outcomes[0]) + await expect(describeFile('files/Docs/../report.csv', carol)).rejects.toMatchObject({ + code: 'validation', + }) + }) + + it('answers a delegation scoped to one file the same for every other path', async () => { + const delegated = { + kind: 'delegated' as const, + serviceId: 'executor' as const, + subjectUserId: alice, + workspaceId, + delegationId: generateId(), + audience: 'sim:workspace-files', + issuedAt: new Date(Date.now() - 1_000), + expiresAt: new Date(Date.now() + 60_000), + resourceScope: { fileId: ids.rootReport, executionId: generateId() }, + } + await expect( + readWorkspaceFileMetadataWithVersion.execute({ + principal: delegated, + input: { fileId: 'files/report.csv', assertedWorkspaceId: workspaceId }, + }) + ).resolves.toMatchObject({ file: { id: ids.rootReport } }) + const outcomes = await refusalsFor(delegated, [ + await ambiguousPath(), + 'files/missing.csv', + 'files/Docs/content', + ]) + expect(outcomes[0]).toMatchObject({ code: 'forbidden' }) + expect(outcomes[1]).toEqual(outcomes[0]) + expect(outcomes[2]).toEqual(outcomes[0]) + }) + 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', diff --git a/apps/sim/lib/workspace-files/application/authorized-workspace-file-use-case.ts b/apps/sim/lib/workspace-files/application/authorized-workspace-file-use-case.ts index 46ecccbc243..7fe3214d77d 100644 --- a/apps/sim/lib/workspace-files/application/authorized-workspace-file-use-case.ts +++ b/apps/sim/lib/workspace-files/application/authorized-workspace-file-use-case.ts @@ -1,12 +1,17 @@ +import type { Principal } from '@sim/auth/principal' import { type AuthorizedWorkspaceUseCaseDefinition, + authorizeWorkspaceOperation, defineAuthorizedWorkspaceUseCase, type WorkspaceOperation, } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace' import { type WorkspaceFileAuthorizationContext, workspaceFileDelegationPolicy, } from '@/lib/workspace-files/application/authorization' +import { UnresolvedWorkspaceFileVfsPathError } from '@/lib/workspace-files/application/unresolved-vfs-path' type AuthorizedWorkspaceFileUseCaseDefinition< O extends WorkspaceOperation, @@ -15,6 +20,24 @@ type AuthorizedWorkspaceFileUseCaseDefinition< R, > = Omit, 'authorizationOptions'> +const authorizationOptions = { delegation: workspaceFileDelegationPolicy } + +/** + * Lets a path's unresolved answer out only to a caller the operation admits to the asserted + * workspace; anyone else gets the refusal a resolved file would have given them. The context names + * no file, so a delegation scoped to one file is refused here exactly as it is for any other file. + */ +async function admitUnresolvedPath( + principal: Principal, + operation: WorkspaceOperation, + unresolved: UnresolvedWorkspaceFileVfsPathError +): Promise { + const workspace = await loadActiveWorkspaceContext(unresolved.workspaceId) + if (!workspace) throw new OrchestrationError('not_found', 'File not found') + await authorizeWorkspaceOperation(principal, operation, workspace, authorizationOptions) + throw unresolved +} + export function defineAuthorizedWorkspaceFileUseCase< const O extends WorkspaceOperation, I, @@ -23,6 +46,14 @@ export function defineAuthorizedWorkspaceFileUseCase< >(definition: AuthorizedWorkspaceFileUseCaseDefinition) { return defineAuthorizedWorkspaceUseCase({ ...definition, - authorizationOptions: { delegation: workspaceFileDelegationPolicy }, + async resolveContext(args) { + try { + return await definition.resolveContext(args) + } catch (error) { + if (!(error instanceof UnresolvedWorkspaceFileVfsPathError)) throw error + return admitUnresolvedPath(args.principal, definition.operation, error) + } + }, + authorizationOptions, }) } diff --git a/apps/sim/lib/workspace-files/application/file-context.ts b/apps/sim/lib/workspace-files/application/file-context.ts index 67d82c8ed23..a05fb082c7a 100644 --- a/apps/sim/lib/workspace-files/application/file-context.ts +++ b/apps/sim/lib/workspace-files/application/file-context.ts @@ -9,6 +9,7 @@ import { type WorkspaceFileLifecycleContext, } from '@/lib/uploads/contexts/workspace/file-manager' import { isWorkspaceFileVfsPath, VfsPathError } from '@/lib/vfs/path' +import { UnresolvedWorkspaceFileVfsPathError } from '@/lib/workspace-files/application/unresolved-vfs-path' import { ownedFileKind } from '@/lib/workspace-files/owned-files' import { matchesFileOwner } from '@/lib/workspace-files/ownership' @@ -77,36 +78,54 @@ function requireWorkspaceFileContext( return canonical } +/** + * A malformed path is decided before any read and says nothing about the workspace, so it stays a + * plain validation error. The lookup's own refusal (two names that display alike) does not. + */ +function rethrowVfsLookupError(vfsPath: string, workspaceId: string, error: unknown): never { + if (error instanceof VfsPathError) { + throw new OrchestrationError( + 'validation', + `"${vfsPath}" is not a valid workspace file path: ${error.message}` + ) + } + if (error instanceof OrchestrationError && error.code === 'validation') { + throw new UnresolvedWorkspaceFileVfsPathError(workspaceId, 'validation', error.message) + } + throw error +} + /** 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) { + const workspaceId = input.assertedWorkspaceId + if (workspaceId === 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}` - ) - } + const file = await getWorkspaceFileByVfsPath(workspaceId, input.fileId).catch((error: unknown) => + rethrowVfsLookupError(input.fileId, workspaceId, error) ) - if (!file) throw new OrchestrationError('not_found', 'File not found') + if (!file) + throw new UnresolvedWorkspaceFileVfsPathError(workspaceId, 'not_found', 'File not found') return file.id } export async function resolveActiveWorkspaceFileContext( input: WorkspaceFileContextInput ): Promise { - const canonical = requireWorkspaceFileContext( - await loadActiveWorkspaceFileContext(await resolveFileId(input), { - includeDeleted: input.includeDeleted, - includeChatUploads: input.includeChatUploads, - includeOwnedFiles: Boolean(input.ownedFilePrincipal), - }), - input.assertedWorkspaceId - ) + const loaded = await loadActiveWorkspaceFileContext(await resolveFileId(input), { + includeDeleted: input.includeDeleted, + includeChatUploads: input.includeChatUploads, + includeOwnedFiles: Boolean(input.ownedFilePrincipal), + }) + if (!loaded && isWorkspaceFileVfsPath(input.fileId) && input.assertedWorkspaceId) { + throw new UnresolvedWorkspaceFileVfsPathError( + input.assertedWorkspaceId, + 'not_found', + 'File not found' + ) + } + const canonical = requireWorkspaceFileContext(loaded, input.assertedWorkspaceId) if (input.ownedFilePrincipal) await assertOwnedFileAccess(input.ownedFilePrincipal, canonical) return canonical } diff --git a/apps/sim/lib/workspace-files/application/unresolved-vfs-path.ts b/apps/sim/lib/workspace-files/application/unresolved-vfs-path.ts new file mode 100644 index 00000000000..23e07df4b49 --- /dev/null +++ b/apps/sim/lib/workspace-files/application/unresolved-vfs-path.ts @@ -0,0 +1,17 @@ +import { OrchestrationError } from '@/lib/core/orchestration/types' + +/** + * A `files/…` path that names no single live file in the asserted workspace. Whether it is missing + * or names two files whose names display alike describes that workspace's contents, so + * `defineAuthorizedWorkspaceFileUseCase` admits the caller to the workspace before letting it out. + */ +export class UnresolvedWorkspaceFileVfsPathError extends OrchestrationError { + constructor( + readonly workspaceId: string, + code: 'not_found' | 'validation', + message: string + ) { + super(code, message) + this.name = 'UnresolvedWorkspaceFileVfsPathError' + } +}