Skip to content

Commit cf7ffe1

Browse files
icecrasher321icecrasher321
committed
fix(executor): authorize files with workflow principals
1 parent 016683a commit cf7ffe1

13 files changed

Lines changed: 554 additions & 103 deletions

‎apps/sim/lib/execution/payloads/materialization.server.test.ts‎

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { mockDownloadServableFileFromStorage, mockVerifyFileAccess } = vi.hoisted(() => ({
7-
mockDownloadServableFileFromStorage: vi.fn(),
8-
mockVerifyFileAccess: vi.fn(),
9-
}))
6+
const { mockDownloadServableFileFromStorage, mockResolveWorkspaceFile, mockVerifyFileAccess } =
7+
vi.hoisted(() => ({
8+
mockDownloadServableFileFromStorage: vi.fn(),
9+
mockResolveWorkspaceFile: vi.fn(),
10+
mockVerifyFileAccess: vi.fn(),
11+
}))
1012

1113
vi.mock('@/lib/uploads/utils/file-utils.server', () => ({
1214
downloadServableFileFromStorage: mockDownloadServableFileFromStorage,
@@ -16,6 +18,10 @@ vi.mock('@/app/api/files/authorization', () => ({
1618
verifyFileAccess: mockVerifyFileAccess,
1719
}))
1820

21+
vi.mock('@/lib/workspace-files/application/resolve-workspace-file-reference', () => ({
22+
resolveWorkspaceFileReference: mockResolveWorkspaceFile,
23+
}))
24+
1925
import { readUserFileContent } from '@/lib/execution/payloads/materialization.server'
2026
import type { UserFile } from '@/executor/types'
2127

@@ -36,6 +42,7 @@ describe('readUserFileContent', () => {
3642
vi.clearAllMocks()
3743
generatedPdf.size = PDF_SOURCE.length
3844
mockVerifyFileAccess.mockResolvedValue(true)
45+
mockResolveWorkspaceFile.mockResolvedValue({ id: 'file-1' })
3946
mockDownloadServableFileFromStorage.mockResolvedValue({
4047
buffer: PDF_BYTES,
4148
contentType: 'application/pdf',
@@ -53,4 +60,77 @@ describe('readUserFileContent', () => {
5360
expect(content).not.toBe(PDF_SOURCE.toString('base64'))
5461
expect(generatedPdf.size).toBe(PDF_BYTES.length)
5562
})
63+
64+
it('authorizes execution-scoped files without inventing a human subject', async () => {
65+
const executionFile: UserFile = {
66+
id: 'file-2',
67+
name: 'result.txt',
68+
url: '',
69+
size: 6,
70+
type: 'text/plain',
71+
key: 'execution/workspace-1/workflow-1/execution-1/result.txt',
72+
context: 'execution',
73+
}
74+
mockDownloadServableFileFromStorage.mockResolvedValueOnce({ buffer: Buffer.from('result') })
75+
76+
await expect(
77+
readUserFileContent(executionFile, {
78+
workspaceId: 'workspace-1',
79+
workflowId: 'workflow-1',
80+
executionId: 'execution-1',
81+
encoding: 'text',
82+
})
83+
).resolves.toBe('result')
84+
85+
expect(mockVerifyFileAccess).not.toHaveBeenCalled()
86+
})
87+
88+
it('authorizes workspace files with the preserved actorless deployment principal', async () => {
89+
const principal = {
90+
kind: 'delegated' as const,
91+
serviceId: 'executor' as const,
92+
workspaceId: 'workspace-1',
93+
delegationId: 'function-1',
94+
audience: 'sim:function-executions',
95+
issuedAt: new Date(Date.now() - 1_000),
96+
expiresAt: new Date(Date.now() + 60_000),
97+
delegationContext: {
98+
kind: 'workflow_execution' as const,
99+
workflowId: 'workflow-1',
100+
executionId: 'execution-1',
101+
principal: {
102+
kind: 'system' as const,
103+
serviceId: 'schedule' as const,
104+
workspaceId: 'workspace-1',
105+
workflowId: 'workflow-1',
106+
},
107+
currentWorkflow: {
108+
workflowId: 'workflow-1',
109+
mode: 'deployment' as const,
110+
deploymentVersionId: 'deployment-1',
111+
},
112+
},
113+
}
114+
115+
await readUserFileContent(generatedPdf, {
116+
principal,
117+
workspaceId: 'workspace-1',
118+
workflowId: 'workflow-1',
119+
executionId: 'execution-1',
120+
requestId: 'request-1',
121+
encoding: 'base64',
122+
})
123+
124+
expect(mockVerifyFileAccess).not.toHaveBeenCalled()
125+
expect(mockResolveWorkspaceFile).toHaveBeenCalledWith(
126+
expect.objectContaining({
127+
workspaceId: 'workspace-1',
128+
reference: generatedPdf.key,
129+
principal: expect.objectContaining({
130+
audience: 'sim:workspace-files',
131+
delegationContext: principal.delegationContext,
132+
}),
133+
})
134+
)
135+
})
56136
})

‎apps/sim/lib/execution/payloads/materialization.server.ts‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import type { Principal } from '@sim/auth/principal'
12
import { createLogger, type Logger } from '@sim/logger'
23
import { toError } from '@sim/utils/errors'
4+
import { OrchestrationError } from '@/lib/core/orchestration/types'
35
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
46
import { isUserFileWithMetadata } from '@/lib/core/utils/user-file'
57
import {
@@ -23,11 +25,15 @@ import {
2325
isGeneratedDocumentSourceType,
2426
} from '@/lib/uploads/utils/file-utils'
2527
import { downloadServableFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
28+
import { rebindWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/application/delegated-principal'
29+
import { fileOperations } from '@/lib/workspace-files/application/operations'
30+
import { resolveWorkspaceFileReference } from '@/lib/workspace-files/application/resolve-workspace-file-reference'
2631
import type { UserFile } from '@/executor/types'
2732

2833
const logger = createLogger('ExecutionPayloadMaterialization')
2934

3035
export interface ExecutionMaterializationContext {
36+
principal?: Principal
3137
workflowId?: string
3238
workspaceId?: string
3339
executionId?: string
@@ -244,7 +250,7 @@ function assertExecutionFileScope(key: string, options: ExecutionMaterialization
244250
}
245251
}
246252

247-
function getVerifiedStorageContext(file: UserFile): StorageContext {
253+
function getVerifiedStorageContext(file: Pick<UserFile, 'key' | 'context'>): StorageContext {
248254
if (!file.key) {
249255
throw new Error('File content requires a storage key.')
250256
}
@@ -258,13 +264,43 @@ function getVerifiedStorageContext(file: UserFile): StorageContext {
258264
}
259265

260266
export async function assertUserFileContentAccess(
261-
file: UserFile,
267+
file: Pick<UserFile, 'key' | 'context'>,
262268
options: ExecutionMaterializationContext
263269
): Promise<void> {
264270
const context = getVerifiedStorageContext(file)
265271

266272
if (context === 'execution') {
267273
assertExecutionFileScope(file.key, options)
274+
return
275+
}
276+
277+
if (context === 'workspace' && options.principal && options.workspaceId) {
278+
const principal =
279+
options.principal.kind === 'delegated'
280+
? rebindWorkspaceFileDelegatedPrincipal({
281+
principal: options.principal,
282+
workspaceId: options.workspaceId,
283+
delegationId: `execution-file-read:${options.requestId ?? 'unknown'}`,
284+
...(options.principal.resourceScope?.fileId
285+
? { fileId: options.principal.resourceScope.fileId }
286+
: {}),
287+
...(options.principal.resourceScope?.chatId
288+
? { chatId: options.principal.resourceScope.chatId }
289+
: {}),
290+
...(options.executionId ? { executionId: options.executionId } : {}),
291+
})
292+
: options.principal
293+
try {
294+
await resolveWorkspaceFileReference({
295+
principal,
296+
operation: fileOperations.readContent,
297+
workspaceId: options.workspaceId,
298+
reference: file.key,
299+
})
300+
return
301+
} catch (error) {
302+
if (!(error instanceof OrchestrationError && error.code === 'not_found')) throw error
303+
}
268304
}
269305

270306
if (!options.userId) {

‎apps/sim/lib/function-execution/application/execute-function.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,45 @@ describe('executeFunction', () => {
6464
allowPersonalApiKeys: true,
6565
})
6666
mocks.executeRequest.mockResolvedValue(Response.json({ success: true }))
67+
mocks.resolvePermission.mockResolvedValue('write')
68+
})
69+
70+
it('uses only the real workflow subject for legacy file contexts', async () => {
71+
const humanPrincipal: WorkflowExecutionDelegatedPrincipal = {
72+
...principal,
73+
subjectUserId: 'invoking-user',
74+
delegationContext: {
75+
...principal.delegationContext!,
76+
principal: {
77+
kind: 'session',
78+
userId: 'invoking-user',
79+
sessionId: 'session-1',
80+
},
81+
},
82+
}
83+
84+
await executeFunction.execute({
85+
principal: humanPrincipal,
86+
input: {
87+
workspaceId: 'workspace-1',
88+
body: {
89+
code: 'return 1',
90+
workspaceId: 'workspace-1',
91+
executionId: 'execution-1',
92+
},
93+
headers: new Headers(),
94+
},
95+
})
96+
97+
expect(mocks.executeRequest).toHaveBeenCalledWith(
98+
expect.anything(),
99+
expect.anything(),
100+
expect.objectContaining({
101+
attributedUserId: 'invoking-user',
102+
fileAccessUserId: 'invoking-user',
103+
principal: humanPrincipal,
104+
})
105+
)
67106
})
68107

69108
it('keeps an actorless deployed principal authoritative and attributes legacy work afterward', async () => {

‎apps/sim/lib/function-execution/application/execute-function.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { resolvePrincipalAttribution } from '@sim/auth/principal'
1+
import { resolvePrincipalAttribution, resolvePrincipalSubject } from '@sim/auth/principal'
22
import { type FunctionExecuteBody, functionExecuteBodySchema } from '@/lib/api/contracts'
33
import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
44
import { OrchestrationError } from '@/lib/core/orchestration/types'
@@ -47,6 +47,7 @@ export const executeFunction = defineAuthorizedWorkspaceUseCase({
4747
const { attributedUserId } = resolvePrincipalAttribution(principal, {
4848
workspaceBillingOwnerUserId: context.billedAccountUserId,
4949
})
50+
const subject = resolvePrincipalSubject(principal)
5051
return executeFunctionRequest(
5152
{
5253
headers: input.headers,
@@ -56,6 +57,7 @@ export const executeFunction = defineAuthorizedWorkspaceUseCase({
5657
{
5758
attributedUserId,
5859
principal,
60+
...(subject?.kind === 'sim_user' ? { fileAccessUserId: subject.userId } : {}),
5961
...(input.sandboxProfile ? { sandboxProfile: input.sandboxProfile } : {}),
6062
}
6163
)

‎apps/sim/lib/function-execution/execute-request.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,8 @@ interface FunctionRouteExecutionContext {
974974
largeValueKeys?: string[]
975975
fileKeys?: string[]
976976
allowLargeValueWorkflowScope?: boolean
977-
userId?: string
977+
attributedUserId: string
978+
fileAccessUserId?: string
978979
requestId: string
979980
resolvedSecretNames: Set<string>
980981
includePrivateResolvedSecretNames: boolean
@@ -1075,6 +1076,7 @@ function createFunctionRuntimeBrokers(
10751076
const largeValueKeys = context.largeValueKeys
10761077
const fileKeys = context.fileKeys
10771078
const base = {
1079+
principal: context.principal,
10781080
requestId: context.requestId,
10791081
workflowId: context.workflowId,
10801082
workspaceId: context.workspaceId,
@@ -1083,7 +1085,7 @@ function createFunctionRuntimeBrokers(
10831085
largeValueKeys,
10841086
fileKeys,
10851087
allowLargeValueWorkflowScope: context.allowLargeValueWorkflowScope,
1086-
userId: context.userId,
1088+
userId: context.fileAccessUserId,
10871089
logger,
10881090
}
10891091

@@ -1155,7 +1157,7 @@ async function compactFunctionRouteBody<T>(
11551157
workflowId: context.workflowId,
11561158
workspaceId: context.workspaceId,
11571159
executionId: context.executionId,
1158-
userId: context.userId,
1160+
userId: context.attributedUserId,
11591161
preserveRoot: true,
11601162
requireDurable: Boolean(context.workspaceId && context.workflowId && context.executionId),
11611163
})
@@ -1824,6 +1826,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
18241826

18251827
export interface TrustedFunctionExecutionAuth {
18261828
attributedUserId: string
1829+
fileAccessUserId?: string
18271830
principal: DelegatedPrincipal
18281831
sandboxProfile?: 'mothership'
18291832
}
@@ -2004,7 +2007,8 @@ export async function executeFunctionRequest(
20042007
largeValueKeys,
20052008
fileKeys,
20062009
allowLargeValueWorkflowScope,
2007-
userId: auth.attributedUserId,
2010+
attributedUserId: auth.attributedUserId,
2011+
fileAccessUserId: auth.fileAccessUserId,
20082012
requestId,
20092013
resolvedSecretNames: new Set<string>(),
20102014
includePrivateResolvedSecretNames,

‎apps/sim/lib/internal/file/execute-tool.test.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ describe('executeFileTool', () => {
9595
expect.objectContaining(input),
9696
expect.objectContaining({
9797
workspaceId: 'workspace-1',
98-
userId: 'user-1',
98+
attributedUserId: 'user-1',
99+
fileAccessUserId: 'user-1',
99100
requestId: 'request-1',
100101
})
101102
)
@@ -114,7 +115,8 @@ describe('executeFileTool', () => {
114115
workspaceId: 'workspace-1',
115116
workflowId: 'workflow-1',
116117
executionId: 'execution-1',
117-
userId: 'user-1',
118+
attributedUserId: 'user-1',
119+
fileAccessUserId: 'user-1',
118120
})
119121
)
120122
expect(mocks.executeManage).not.toHaveBeenCalled()
@@ -156,7 +158,10 @@ describe('executeFileTool', () => {
156158

157159
expect(mocks.executeManage).toHaveBeenCalledWith(
158160
expect.objectContaining(MANAGE_INPUTS.file_get),
159-
expect.objectContaining({ userId: 'invoking-user' })
161+
expect.objectContaining({
162+
attributedUserId: 'invoking-user',
163+
fileAccessUserId: 'invoking-user',
164+
})
160165
)
161166
})
162167

@@ -210,7 +215,8 @@ describe('executeFileTool', () => {
210215
expect.objectContaining(MANAGE_INPUTS.file_decompress),
211216
expect.objectContaining({
212217
principal,
213-
userId: 'workspace-owner',
218+
attributedUserId: 'workspace-owner',
219+
fileAccessUserId: undefined,
214220
workspaceId: 'workspace-1',
215221
})
216222
)

0 commit comments

Comments
 (0)