Skip to content

Commit 2c4e3f8

Browse files
committed
fix(files): authorize the workspace before resolving a files/ path
1 parent 3e23d39 commit 2c4e3f8

3 files changed

Lines changed: 73 additions & 8 deletions

File tree

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Real PostgreSQL behavior of single-file operations addressed by a `files/…` VFS path instead
33
* of an id: the path names exactly one live file in the asserted workspace, never a same-named
44
* file in another folder or workspace, a chat upload or an archived file, and authorization is
5-
* unchanged.
5+
* unchanged. A caller who cannot reach the workspace learns nothing about the paths in it.
66
*/
77
import { db } from '@sim/db'
88
import { permissions, user, workspace, workspaceFiles } from '@sim/db/schema'
@@ -224,6 +224,27 @@ describe('single-file operations addressed by VFS path', () => {
224224
expect((await row(ids.rootReport))?.deletedAt).toBeNull()
225225
})
226226

227+
it('answers a non-member the same for an ambiguous, a missing and an existing path', async () => {
228+
const ambiguous = workspaceFileVfsPath((await describeFile(ids.alikeSingle)).file)
229+
const outcomes = await Promise.all(
230+
[ambiguous, 'files/missing.csv', 'files/report.csv'].map((path) =>
231+
describeFile(path, carol).then(
232+
() => null,
233+
(error: { code?: string; message?: string }) => ({
234+
code: error.code,
235+
message: error.message,
236+
})
237+
)
238+
)
239+
)
240+
expect(outcomes[0]).toMatchObject({ code: 'forbidden' })
241+
expect(outcomes[1]).toEqual(outcomes[0])
242+
expect(outcomes[2]).toEqual(outcomes[0])
243+
await expect(describeFile('files/Docs/../report.csv', carol)).rejects.toMatchObject({
244+
code: 'validation',
245+
})
246+
})
247+
227248
it('rejects a dot-segment path instead of walking out of its folder', async () => {
228249
await expect(deleteFile('files/Docs/../report.csv')).rejects.toMatchObject({
229250
code: 'validation',

‎apps/sim/lib/workspace-files/application/authorized-workspace-file-use-case.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type WorkspaceFileAuthorizationContext,
88
workspaceFileDelegationPolicy,
99
} from '@/lib/workspace-files/application/authorization'
10+
import { authorizeWorkspaceFileVfsPath } from '@/lib/workspace-files/application/file-context'
1011

1112
type AuthorizedWorkspaceFileUseCaseDefinition<
1213
O extends WorkspaceOperation,
@@ -23,6 +24,10 @@ export function defineAuthorizedWorkspaceFileUseCase<
2324
>(definition: AuthorizedWorkspaceFileUseCaseDefinition<O, I, C, R>) {
2425
return defineAuthorizedWorkspaceUseCase({
2526
...definition,
27+
async resolveContext(args) {
28+
await authorizeWorkspaceFileVfsPath(args.principal, definition.operation, args.input)
29+
return definition.resolveContext(args)
30+
},
2631
authorizationOptions: { delegation: workspaceFileDelegationPolicy },
2732
})
2833
}

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

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import type { Principal } from '@sim/auth/principal'
2+
import { isRecordLike } from '@sim/utils/object'
3+
import { authorizeWorkspaceOperation, type WorkspaceOperation } from '@/lib/core/application'
24
import { OrchestrationError } from '@/lib/core/orchestration/types'
35
import type { DbOrTx } from '@/lib/db/types'
46
import {
57
type ActiveWorkspaceFileContext,
68
getWorkspaceFileByVfsPath,
9+
loadActiveWorkspaceContext,
710
loadActiveWorkspaceFileContext,
811
loadWorkspaceFileLifecycleContext,
912
type WorkspaceFileLifecycleContext,
1013
} from '@/lib/uploads/contexts/workspace/file-manager'
11-
import { isWorkspaceFileVfsPath, VfsPathError } from '@/lib/vfs/path'
14+
import { decodeVfsReferenceSegments, isWorkspaceFileVfsPath, VfsPathError } from '@/lib/vfs/path'
15+
import { WORKSPACE_FILES_DELEGATION_AUDIENCE } from '@/lib/workspace-files/application/authorization'
1216
import { ownedFileKind } from '@/lib/workspace-files/owned-files'
1317
import { matchesFileOwner } from '@/lib/workspace-files/ownership'
1418

@@ -77,19 +81,54 @@ function requireWorkspaceFileContext<C extends ActiveWorkspaceFileContext>(
7781
return canonical
7882
}
7983

80-
/** A path becomes an id first, so the canonical load and every authorization check run unchanged. */
84+
function invalidVfsPathError(vfsPath: string, error: unknown): unknown {
85+
if (!(error instanceof VfsPathError)) return error
86+
return new OrchestrationError(
87+
'validation',
88+
`"${vfsPath}" is not a valid workspace file path: ${error.message}`
89+
)
90+
}
91+
92+
/**
93+
* Admits the principal into the asserted workspace before a `files/…` path is looked up by name
94+
* there, so nothing the lookup answers (a missing file, two names that display alike) reaches a
95+
* caller with no access to that workspace. A malformed path is refused first: deciding that reads
96+
* nothing and says nothing about the workspace. Any other input passes untouched. A delegation
97+
* scoped to one file is checked once the path names a file, by the use case's own authorization.
98+
*/
99+
export async function authorizeWorkspaceFileVfsPath(
100+
principal: Principal,
101+
operation: WorkspaceOperation,
102+
input: unknown
103+
): Promise<void> {
104+
if (!isRecordLike(input)) return
105+
const { fileId: vfsPath, assertedWorkspaceId } = input
106+
if (typeof vfsPath !== 'string' || !isWorkspaceFileVfsPath(vfsPath)) return
107+
if (typeof assertedWorkspaceId !== 'string') return
108+
try {
109+
decodeVfsReferenceSegments(vfsPath.slice('files/'.length))
110+
} catch (error) {
111+
throw invalidVfsPathError(vfsPath, error)
112+
}
113+
const workspace = await loadActiveWorkspaceContext(assertedWorkspaceId)
114+
if (!workspace) throw new OrchestrationError('not_found', 'File not found')
115+
await authorizeWorkspaceOperation(principal, operation, workspace, {
116+
delegation: { audience: WORKSPACE_FILES_DELEGATION_AUDIENCE, isWithinScope: () => true },
117+
})
118+
}
119+
120+
/**
121+
* A path becomes an id first, so the canonical load and every authorization check run unchanged.
122+
* Use cases reach this only after {@link authorizeWorkspaceFileVfsPath} admitted the caller.
123+
*/
81124
async function resolveFileId(input: WorkspaceFileContextInput): Promise<string> {
82125
if (!isWorkspaceFileVfsPath(input.fileId)) return input.fileId
83126
if (input.assertedWorkspaceId === undefined) {
84127
throw new OrchestrationError('not_found', 'File not found')
85128
}
86129
const file = await getWorkspaceFileByVfsPath(input.assertedWorkspaceId, input.fileId).catch(
87130
(error: unknown) => {
88-
if (!(error instanceof VfsPathError)) throw error
89-
throw new OrchestrationError(
90-
'validation',
91-
`"${input.fileId}" is not a valid workspace file path: ${error.message}`
92-
)
131+
throw invalidVfsPathError(input.fileId, error)
93132
}
94133
)
95134
if (!file) throw new OrchestrationError('not_found', 'File not found')

0 commit comments

Comments
 (0)