Skip to content

Commit 12be2c5

Browse files
committed
fix(files): every refusal a path lookup reads from the workspace waits for workspace authorization
1 parent 1fcf99d commit 12be2c5

3 files changed

Lines changed: 50 additions & 11 deletions

File tree

‎apps/sim/lib/workspace-files/__integration__/file-path-references.integration.ts‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/
1111
import { generateId } from '@sim/utils/id'
1212
import { eq, inArray, sql } from 'drizzle-orm'
1313
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
14+
import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
1415
import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/file-manager'
1516
import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file'
1617
import { readWorkspaceFileMetadataWithVersion } from '@/lib/workspace-files/application/read-workspace-file-metadata'
@@ -21,6 +22,7 @@ const bob = generateId()
2122
const carol = generateId()
2223
const workspaceId = generateId()
2324
const otherWorkspaceId = generateId()
25+
const overLimitWorkspaceId = generateId()
2426
const folders = { docs: generateId(), media: generateId(), foreign: generateId() }
2527
const session = (userId: string) => ({ kind: 'session' as const, userId, sessionId: generateId() })
2628

@@ -163,7 +165,10 @@ describe('single-file operations addressed by VFS path', () => {
163165
})
164166

165167
afterAll(async () => {
166-
await deleteWorkspaceFixture(db, inArray(workspace.id, [workspaceId, otherWorkspaceId]))
168+
await deleteWorkspaceFixture(
169+
db,
170+
inArray(workspace.id, [workspaceId, otherWorkspaceId, overLimitWorkspaceId])
171+
)
167172
await db.delete(user).where(inArray(user.id, [alice, bob, carol]))
168173
})
169174

@@ -321,6 +326,37 @@ describe('single-file operations addressed by VFS path', () => {
321326
}
322327
})
323328

329+
it('answers a non-member the same for a workspace whose folder tree is too large to read', async () => {
330+
await insertWorkspaceFixture(db, [
331+
{ id: overLimitWorkspaceId, name: 'Over limit', ownerId: alice, billedAccountUserId: alice },
332+
])
333+
await db.insert(permissions).values({
334+
id: generateId(),
335+
userId: alice,
336+
entityType: 'workspace',
337+
entityId: overLimitWorkspaceId,
338+
permissionType: 'admin',
339+
})
340+
await db.execute(sql`
341+
INSERT INTO folder (id, name, user_id, workspace_id, resource_type, parent_id)
342+
SELECT ${generateId()} || '-' || g, 'Folder ' || g, ${alice}, ${overLimitWorkspaceId}, 'file', NULL
343+
FROM generate_series(1, ${MAX_FOLDERS_PER_WORKSPACE + 1}) AS g`)
344+
const answer = (userId: string, fileId: string) =>
345+
describeFile(fileId, userId, overLimitWorkspaceId).then(
346+
() => null,
347+
(error: { code?: string; message?: string }) => ({
348+
code: error.code,
349+
message: error.message,
350+
})
351+
)
352+
const nested = await answer(carol, 'files/Folder 1/report.csv')
353+
expect(nested).toMatchObject({ code: 'forbidden' })
354+
expect(await answer(carol, 'files/missing.csv')).toEqual(nested)
355+
expect(await answer(alice, 'files/Folder 1/report.csv')).toMatchObject({
356+
code: 'payload_too_large',
357+
})
358+
})
359+
324360
it('resolves no path for a caller that did not opt in', async () => {
325361
await expect(
326362
deleteWorkspaceFileOperation.execute({

‎apps/sim/lib/workspace-files/application/file-context.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ function namesVfsPath(input: WorkspaceFileTargetInput): boolean {
9393

9494
/**
9595
* A path becomes an id first, so the canonical load and every authorization check run unchanged.
96-
* A malformed path is refused before any read and says nothing about the workspace. A path that
97-
* names no single live file does, so it leaves as an {@link UnresolvedWorkspaceFileVfsPathError},
98-
* which only a caller the operation admits to the workspace gets to see.
96+
* The resolver's `validation` is decided from the reference alone, before any read, and says
97+
* nothing about the workspace. Every other refusal it raises was read from the workspace, as is a
98+
* path that names no single live file, so each leaves as an
99+
* {@link UnresolvedWorkspaceFileVfsPathError} that only a caller the operation admits sees.
99100
*/
100101
async function resolveFileId(input: WorkspaceFileContextInput): Promise<string> {
101102
if (!namesVfsPath(input)) return input.fileId
@@ -107,14 +108,15 @@ async function resolveFileId(input: WorkspaceFileContextInput): Promise<string>
107108
{ entityType: 'workspace', entityId: workspaceId },
108109
input.fileId
109110
).catch((error: unknown) => {
110-
if (error instanceof OrchestrationError && error.code === 'conflict') {
111+
if (!(error instanceof OrchestrationError) || error.code === 'validation') throw error
112+
if (error.code === 'conflict') {
111113
throw new UnresolvedWorkspaceFileVfsPathError(
112114
workspaceId,
113115
'validation',
114116
`"${input.fileId}" names more than one file whose names display alike; pass the file id instead`
115117
)
116118
}
117-
throw error
119+
throw new UnresolvedWorkspaceFileVfsPathError(workspaceId, error.code, error.message)
118120
})
119121
if (!file) {
120122
throw new UnresolvedWorkspaceFileVfsPathError(workspaceId, 'not_found', 'File not found')

‎apps/sim/lib/workspace-files/application/unresolved-vfs-path.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
1-
import { OrchestrationError } from '@/lib/core/orchestration/types'
1+
import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
22

33
/**
4-
* A `files/…` path that names no single live file in the asserted workspace. Whether it is missing
5-
* or names two files whose names display alike describes that workspace's contents, so
6-
* `defineAuthorizedWorkspaceFileUseCase` admits the caller to the workspace before letting it out.
4+
* A `files/…` path that names no single live file in the asserted workspace, or a refusal its
5+
* lookup read from the workspace (two names that display alike, a folder tree too large to read).
6+
* Each describes that workspace's contents, so `defineAuthorizedWorkspaceFileUseCase` admits the
7+
* caller to the workspace before letting it out.
78
*/
89
export class UnresolvedWorkspaceFileVfsPathError extends OrchestrationError {
910
constructor(
1011
readonly workspaceId: string,
11-
code: 'not_found' | 'validation',
12+
code: OrchestrationErrorCode,
1213
message: string
1314
) {
1415
super(code, message)

0 commit comments

Comments
 (0)