Skip to content

Commit ad3ce46

Browse files
committed
fix(files): admit a caller only when a files/ path fails to resolve, and refuse file-scoped delegates there
Path resolution now throws a typed UnresolvedWorkspaceFileVfsPathError for a missing or ambiguous path; defineAuthorizedWorkspaceFileUseCase authorizes the asserted workspace with the operation's own options before letting it out. A resolved path keeps the single authorization the id path runs, and a delegation scoped to one file gets the same refusal for every other path.
1 parent aa08d3b commit ad3ce46

4 files changed

Lines changed: 135 additions & 70 deletions

File tree

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

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,25 @@ const deleteFile = (fileId: string, userId = alice) =>
6262
input: { fileId, assertedWorkspaceId: workspaceId },
6363
})
6464

65+
/** The `{ code, message }` each path is refused with, or null where it resolved. */
66+
const refusalsFor = (
67+
principal: Parameters<typeof readWorkspaceFileMetadataWithVersion.execute>[0]['principal'],
68+
paths: string[]
69+
) =>
70+
Promise.all(
71+
paths.map((fileId) =>
72+
readWorkspaceFileMetadataWithVersion
73+
.execute({ principal, input: { fileId, assertedWorkspaceId: workspaceId } })
74+
.then(
75+
() => null,
76+
(error: { code?: string; message?: string }) => ({
77+
code: error.code,
78+
message: error.message,
79+
})
80+
)
81+
)
82+
)
83+
6584
describe('single-file operations addressed by VFS path', () => {
6685
const ids = {
6786
rootReport: '',
@@ -77,6 +96,8 @@ describe('single-file operations addressed by VFS path', () => {
7796
alikeSingle: '',
7897
}
7998

99+
const ambiguousPath = async () => workspaceFileVfsPath((await describeFile(ids.alikeSingle)).file)
100+
80101
beforeAll(async () => {
81102
assertDisposableTestDatabaseUrl(process.env.DATABASE_URL ?? '')
82103
await db.insert(user).values(
@@ -225,18 +246,11 @@ describe('single-file operations addressed by VFS path', () => {
225246
})
226247

227248
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-
)
249+
const outcomes = await refusalsFor(session(carol), [
250+
await ambiguousPath(),
251+
'files/missing.csv',
252+
'files/report.csv',
253+
])
240254
expect(outcomes[0]).toMatchObject({ code: 'forbidden' })
241255
expect(outcomes[1]).toEqual(outcomes[0])
242256
expect(outcomes[2]).toEqual(outcomes[0])
@@ -245,6 +259,34 @@ describe('single-file operations addressed by VFS path', () => {
245259
})
246260
})
247261

262+
it('answers a delegation scoped to one file the same for every other path', async () => {
263+
const delegated = {
264+
kind: 'delegated' as const,
265+
serviceId: 'executor' as const,
266+
subjectUserId: alice,
267+
workspaceId,
268+
delegationId: generateId(),
269+
audience: 'sim:workspace-files',
270+
issuedAt: new Date(Date.now() - 1_000),
271+
expiresAt: new Date(Date.now() + 60_000),
272+
resourceScope: { fileId: ids.rootReport, executionId: generateId() },
273+
}
274+
await expect(
275+
readWorkspaceFileMetadataWithVersion.execute({
276+
principal: delegated,
277+
input: { fileId: 'files/report.csv', assertedWorkspaceId: workspaceId },
278+
})
279+
).resolves.toMatchObject({ file: { id: ids.rootReport } })
280+
const outcomes = await refusalsFor(delegated, [
281+
await ambiguousPath(),
282+
'files/missing.csv',
283+
'files/Docs/content',
284+
])
285+
expect(outcomes[0]).toMatchObject({ code: 'forbidden' })
286+
expect(outcomes[1]).toEqual(outcomes[0])
287+
expect(outcomes[2]).toEqual(outcomes[0])
288+
})
289+
248290
it('rejects a dot-segment path instead of walking out of its folder', async () => {
249291
await expect(deleteFile('files/Docs/../report.csv')).rejects.toMatchObject({
250292
code: 'validation',
Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
1+
import type { Principal } from '@sim/auth/principal'
12
import {
23
type AuthorizedWorkspaceUseCaseDefinition,
4+
authorizeWorkspaceOperation,
35
defineAuthorizedWorkspaceUseCase,
46
type WorkspaceOperation,
57
} from '@/lib/core/application'
8+
import { OrchestrationError } from '@/lib/core/orchestration/types'
9+
import { loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace'
610
import {
711
type WorkspaceFileAuthorizationContext,
812
workspaceFileDelegationPolicy,
913
} from '@/lib/workspace-files/application/authorization'
10-
import { authorizeWorkspaceFileVfsPath } from '@/lib/workspace-files/application/file-context'
14+
import { UnresolvedWorkspaceFileVfsPathError } from '@/lib/workspace-files/application/unresolved-vfs-path'
1115

1216
type AuthorizedWorkspaceFileUseCaseDefinition<
1317
O extends WorkspaceOperation,
@@ -16,6 +20,24 @@ type AuthorizedWorkspaceFileUseCaseDefinition<
1620
R,
1721
> = Omit<AuthorizedWorkspaceUseCaseDefinition<O, I, C, R>, 'authorizationOptions'>
1822

23+
const authorizationOptions = { delegation: workspaceFileDelegationPolicy }
24+
25+
/**
26+
* Lets a path's unresolved answer out only to a caller the operation admits to the asserted
27+
* workspace; anyone else gets the refusal a resolved file would have given them. The context names
28+
* no file, so a delegation scoped to one file is refused here exactly as it is for any other file.
29+
*/
30+
async function admitUnresolvedPath(
31+
principal: Principal,
32+
operation: WorkspaceOperation,
33+
unresolved: UnresolvedWorkspaceFileVfsPathError
34+
): Promise<never> {
35+
const workspace = await loadActiveWorkspaceContext(unresolved.workspaceId)
36+
if (!workspace) throw new OrchestrationError('not_found', 'File not found')
37+
await authorizeWorkspaceOperation(principal, operation, workspace, authorizationOptions)
38+
throw unresolved
39+
}
40+
1941
export function defineAuthorizedWorkspaceFileUseCase<
2042
const O extends WorkspaceOperation,
2143
I,
@@ -25,9 +47,13 @@ export function defineAuthorizedWorkspaceFileUseCase<
2547
return defineAuthorizedWorkspaceUseCase({
2648
...definition,
2749
async resolveContext(args) {
28-
await authorizeWorkspaceFileVfsPath(args.principal, definition.operation, args.input)
29-
return definition.resolveContext(args)
50+
try {
51+
return await definition.resolveContext(args)
52+
} catch (error) {
53+
if (!(error instanceof UnresolvedWorkspaceFileVfsPathError)) throw error
54+
return admitUnresolvedPath(args.principal, definition.operation, error)
55+
}
3056
},
31-
authorizationOptions: { delegation: workspaceFileDelegationPolicy },
57+
authorizationOptions,
3258
})
3359
}

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

Lines changed: 34 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,15 @@
11
import type { Principal } from '@sim/auth/principal'
2-
import { isRecordLike } from '@sim/utils/object'
3-
import { authorizeWorkspaceOperation, type WorkspaceOperation } from '@/lib/core/application'
42
import { OrchestrationError } from '@/lib/core/orchestration/types'
53
import type { DbOrTx } from '@/lib/db/types'
64
import {
75
type ActiveWorkspaceFileContext,
86
getWorkspaceFileByVfsPath,
9-
loadActiveWorkspaceContext,
107
loadActiveWorkspaceFileContext,
118
loadWorkspaceFileLifecycleContext,
129
type WorkspaceFileLifecycleContext,
1310
} from '@/lib/uploads/contexts/workspace/file-manager'
14-
import { decodeVfsReferenceSegments, isWorkspaceFileVfsPath, VfsPathError } from '@/lib/vfs/path'
15-
import { WORKSPACE_FILES_DELEGATION_AUDIENCE } from '@/lib/workspace-files/application/authorization'
11+
import { isWorkspaceFileVfsPath, VfsPathError } from '@/lib/vfs/path'
12+
import { UnresolvedWorkspaceFileVfsPathError } from '@/lib/workspace-files/application/unresolved-vfs-path'
1613
import { ownedFileKind } from '@/lib/workspace-files/owned-files'
1714
import { matchesFileOwner } from '@/lib/workspace-files/ownership'
1815

@@ -81,71 +78,54 @@ function requireWorkspaceFileContext<C extends ActiveWorkspaceFileContext>(
8178
return canonical
8279
}
8380

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-
9281
/**
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.
82+
* A malformed path is decided before any read and says nothing about the workspace, so it stays a
83+
* plain validation error. The lookup's own refusal (two names that display alike) does not.
9884
*/
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)
85+
function rethrowVfsLookupError(vfsPath: string, workspaceId: string, error: unknown): never {
86+
if (error instanceof VfsPathError) {
87+
throw new OrchestrationError(
88+
'validation',
89+
`"${vfsPath}" is not a valid workspace file path: ${error.message}`
90+
)
11291
}
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-
})
92+
if (error instanceof OrchestrationError && error.code === 'validation') {
93+
throw new UnresolvedWorkspaceFileVfsPathError(workspaceId, 'validation', error.message)
94+
}
95+
throw error
11896
}
11997

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-
*/
98+
/** A path becomes an id first, so the canonical load and every authorization check run unchanged. */
12499
async function resolveFileId(input: WorkspaceFileContextInput): Promise<string> {
125100
if (!isWorkspaceFileVfsPath(input.fileId)) return input.fileId
126-
if (input.assertedWorkspaceId === undefined) {
101+
const workspaceId = input.assertedWorkspaceId
102+
if (workspaceId === undefined) {
127103
throw new OrchestrationError('not_found', 'File not found')
128104
}
129-
const file = await getWorkspaceFileByVfsPath(input.assertedWorkspaceId, input.fileId).catch(
130-
(error: unknown) => {
131-
throw invalidVfsPathError(input.fileId, error)
132-
}
105+
const file = await getWorkspaceFileByVfsPath(workspaceId, input.fileId).catch((error: unknown) =>
106+
rethrowVfsLookupError(input.fileId, workspaceId, error)
133107
)
134-
if (!file) throw new OrchestrationError('not_found', 'File not found')
108+
if (!file)
109+
throw new UnresolvedWorkspaceFileVfsPathError(workspaceId, 'not_found', 'File not found')
135110
return file.id
136111
}
137112

138113
export async function resolveActiveWorkspaceFileContext(
139114
input: WorkspaceFileContextInput
140115
): Promise<ActiveWorkspaceFileContext> {
141-
const canonical = requireWorkspaceFileContext(
142-
await loadActiveWorkspaceFileContext(await resolveFileId(input), {
143-
includeDeleted: input.includeDeleted,
144-
includeChatUploads: input.includeChatUploads,
145-
includeOwnedFiles: Boolean(input.ownedFilePrincipal),
146-
}),
147-
input.assertedWorkspaceId
148-
)
116+
const loaded = await loadActiveWorkspaceFileContext(await resolveFileId(input), {
117+
includeDeleted: input.includeDeleted,
118+
includeChatUploads: input.includeChatUploads,
119+
includeOwnedFiles: Boolean(input.ownedFilePrincipal),
120+
})
121+
if (!loaded && isWorkspaceFileVfsPath(input.fileId) && input.assertedWorkspaceId) {
122+
throw new UnresolvedWorkspaceFileVfsPathError(
123+
input.assertedWorkspaceId,
124+
'not_found',
125+
'File not found'
126+
)
127+
}
128+
const canonical = requireWorkspaceFileContext(loaded, input.assertedWorkspaceId)
149129
if (input.ownedFilePrincipal) await assertOwnedFileAccess(input.ownedFilePrincipal, canonical)
150130
return canonical
151131
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { OrchestrationError } from '@/lib/core/orchestration/types'
2+
3+
/**
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.
7+
*/
8+
export class UnresolvedWorkspaceFileVfsPathError extends OrchestrationError {
9+
constructor(
10+
readonly workspaceId: string,
11+
code: 'not_found' | 'validation',
12+
message: string
13+
) {
14+
super(code, message)
15+
this.name = 'UnresolvedWorkspaceFileVfsPathError'
16+
}
17+
}

0 commit comments

Comments
 (0)