Skip to content

Commit 749e745

Browse files
icecrasher321icecrasher321
committed
fix(auth): bound derived workflow delegations
1 parent cf7ffe1 commit 749e745

10 files changed

Lines changed: 88 additions & 15 deletions

File tree

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ function request(
6868
userId: 'user-1',
6969
workspaceId: 'workspace-1',
7070
billingAttribution: BILLING_ATTRIBUTION,
71+
executorDelegationOrigin: {
72+
subjectUserId: 'user-1',
73+
workflowId: 'workflow-1',
74+
executionId: 'execution-1',
75+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
76+
currentWorkflow: { workflowId: 'workflow-1', mode: 'draft' },
77+
},
7178
},
7279
requestId: 'request-1',
7380
...overrides,
@@ -223,7 +230,6 @@ describe('executeFileTool', () => {
223230
})
224231

225232
it('rejects missing trusted identity during principal construction', async () => {
226-
mocks.createPrincipal.mockRejectedValueOnce(new Error('Authentication required'))
227233
const response = await executeFileTool(
228234
request('file_get', MANAGE_INPUTS.file_get, {
229235
context: {
@@ -236,7 +242,7 @@ describe('executeFileTool', () => {
236242
)
237243

238244
expect(response.status).toBe(401)
239-
expect(mocks.createPrincipal).toHaveBeenCalledOnce()
245+
expect(mocks.createPrincipal).not.toHaveBeenCalled()
240246
expect(mocks.executeManage).not.toHaveBeenCalled()
241247
})
242248

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export const executeFileTool: InternalToolOperationHandler = async (request) =>
4242
}
4343

4444
const workspaceId = request.context.workspaceId
45-
if (!workspaceId) {
45+
if (!workspaceId || !request.context.executorDelegationOrigin) {
4646
return Response.json({ success: false, error: 'Authentication required' }, { status: 401 })
4747
}
4848

‎apps/sim/lib/internal/function/execute.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ describe('executeFunctionTool', () => {
2626
})
2727

2828
it('binds executor calls from the canonical origin instead of the compatibility user ID', async () => {
29+
const startedAt = Date.now()
2930
const origin = {
3031
workflowId: 'workflow-1',
3132
executionId: 'execution-1',
@@ -62,7 +63,12 @@ describe('executeFunctionTool', () => {
6263
const headers = new Headers()
6364

6465
await executeFunctionTool({
65-
body: { code: 'return 1', userId: 'forged-user', workspaceId: 'forged-workspace' },
66+
body: {
67+
code: 'return 1',
68+
timeout: 60_000,
69+
userId: 'forged-user',
70+
workspaceId: 'forged-workspace',
71+
},
6672
headers,
6773
context,
6874
requestId: 'request-1',
@@ -71,8 +77,12 @@ describe('executeFunctionTool', () => {
7177
expect(mocks.createPrincipal).toHaveBeenCalledWith({
7278
context,
7379
audience: FUNCTION_EXECUTION_DELEGATION_AUDIENCE,
80+
expiresAt: expect.any(Date),
7481
resourceScope: { executionId: 'execution-1' },
7582
})
83+
const delegatedExpiry = mocks.createPrincipal.mock.calls[0]?.[0].expiresAt as Date
84+
expect(delegatedExpiry.getTime()).toBeGreaterThanOrEqual(startedAt + 60_000)
85+
expect(delegatedExpiry.getTime()).toBeLessThanOrEqual(Date.now() + 60_000)
7686
expect(mocks.execute).toHaveBeenCalledWith({
7787
principal,
7888
input: expect.objectContaining({

‎apps/sim/lib/internal/function/execute.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export async function executeFunctionTool(input: ExecuteFunctionToolInput): Prom
6060
principal = await createExecutorPrincipalFromExecutionContext({
6161
context,
6262
audience: FUNCTION_EXECUTION_DELEGATION_AUDIENCE,
63+
expiresAt,
6364
...(context.executionId ? { resourceScope: { executionId: context.executionId } } : {}),
6465
})
6566
}

‎apps/sim/lib/internal/principals/executor.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,27 @@ describe('createExecutorPrincipalFromExecutionContext', () => {
7070
)
7171
})
7272

73+
it('uses an explicit trusted execution deadline as the delegation expiry', async () => {
74+
const expiresAt = new Date('2026-01-01T01:00:00.000Z')
75+
76+
await createExecutorPrincipalFromExecutionContext({
77+
context: executionContext({
78+
executorDelegationOrigin: {
79+
subjectUserId: 'user-origin',
80+
workflowId: 'workflow-origin',
81+
executionId: 'execution-origin',
82+
},
83+
}),
84+
audience: 'sim:function-executions',
85+
expiresAt,
86+
})
87+
88+
expect(mockBindInternalExecutorDelegation).toHaveBeenCalledWith(
89+
expect.objectContaining({ expiresAt }),
90+
{ audience: 'sim:function-executions' }
91+
)
92+
})
93+
7394
it.each([
7495
{
7596
name: 'schedule',

‎apps/sim/lib/internal/principals/executor.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ export function resolveExecutorOriginSubject(origin: ExecutorDelegationOrigin):
3131
async function bindExecutorPrincipal(
3232
origin: ExecutorDelegationOrigin,
3333
audience: string,
34-
resourceScope?: DelegatedPrincipal['resourceScope']
34+
resourceScope?: DelegatedPrincipal['resourceScope'],
35+
expiresAt?: Date
3536
) {
3637
if (!origin.workflowId.trim()) throw new Error('Authentication required')
3738
const subjectUserId = resolveExecutorOriginSubject(origin)
@@ -46,7 +47,7 @@ async function bindExecutorPrincipal(
4647
...(origin.currentWorkflow ? { currentWorkflow: origin.currentWorkflow } : {}),
4748
delegationId: generateId(),
4849
issuedAt,
49-
expiresAt: new Date(issuedAt.getTime() + EXECUTOR_DELEGATION_TTL_MS),
50+
expiresAt: expiresAt ?? new Date(issuedAt.getTime() + EXECUTOR_DELEGATION_TTL_MS),
5051
},
5152
{
5253
audience,
@@ -59,14 +60,16 @@ export interface CreateExecutorPrincipalFromExecutionContextInput {
5960
context: InternalToolOperationContext
6061
audience: string
6162
resourceScope?: DelegatedPrincipal['resourceScope']
63+
expiresAt?: Date
6264
}
6365

6466
export async function createExecutorPrincipalFromExecutionContext({
6567
context,
6668
audience,
6769
resourceScope,
70+
expiresAt,
6871
}: CreateExecutorPrincipalFromExecutionContextInput) {
6972
const origin = context.executorDelegationOrigin
7073
if (!origin) throw new Error('Executor delegation origin is required')
71-
return bindExecutorPrincipal(origin, audience, resourceScope)
74+
return bindExecutorPrincipal(origin, audience, resourceScope, expiresAt)
7275
}

‎apps/sim/lib/workspace-files/application/delegated-principal.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { rebindWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/app
77

88
describe('rebindWorkspaceFileDelegatedPrincipal', () => {
99
it('preserves actorless workflow identity and deployment authority', () => {
10+
const expiresAt = new Date(Date.now() + 60_000)
1011
const delegationContext = {
1112
kind: 'workflow_execution' as const,
1213
workflowId: 'workflow-1',
@@ -32,7 +33,7 @@ describe('rebindWorkspaceFileDelegatedPrincipal', () => {
3233
delegationId: 'function-1',
3334
audience: 'sim:function-executions',
3435
issuedAt: new Date(Date.now() - 1_000),
35-
expiresAt: new Date(Date.now() + 60_000),
36+
expiresAt,
3637
delegationContext,
3738
},
3839
workspaceId: 'workspace-1',
@@ -48,6 +49,7 @@ describe('rebindWorkspaceFileDelegatedPrincipal', () => {
4849
resourceScope: { executionId: 'execution-1' },
4950
delegationContext,
5051
})
52+
expect(rebound.expiresAt).toEqual(expiresAt)
5153
expect(rebound).not.toHaveProperty('subjectUserId')
5254
})
5355

‎apps/sim/lib/workspace-files/application/delegated-principal.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,12 @@ export function rebindWorkspaceFileDelegatedPrincipal(
6161
delegationId: input.delegationId,
6262
audience: WORKSPACE_FILES_DELEGATION_AUDIENCE,
6363
issuedAt,
64-
expiresAt: new Date(issuedAt.getTime() + WORKSPACE_FILE_DELEGATION_TTL_MS),
64+
expiresAt: new Date(
65+
Math.min(
66+
input.principal.expiresAt.getTime(),
67+
issuedAt.getTime() + WORKSPACE_FILE_DELEGATION_TTL_MS
68+
)
69+
),
6570
resourceScope: {
6671
...(input.fileId ? { fileId: input.fileId } : {}),
6772
...(input.chatId ? { chatId: input.chatId } : {}),

‎apps/sim/tools/index.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -863,14 +863,14 @@ describe('executeTool Function', () => {
863863
workflowId: 'workflow-1',
864864
executionId: 'execution-1',
865865
workspaceId: 'workspace-456',
866-
userId: undefined,
867866
largeValueExecutionIds: ['execution-1'],
868867
largeValueKeys: ['lv_ABCDEFGHIJKL'],
869868
fileKeys: ['file-1'],
870869
allowLargeValueWorkflowScope: true,
871870
},
872871
},
873872
})
873+
expect(mockExecuteFunction.mock.calls[0]?.[0].input.body.userId).toBeUndefined()
874874
expect(mockGenerateInternalToken).not.toHaveBeenCalled()
875875
expect(fetchSpy).not.toHaveBeenCalled()
876876
})
@@ -948,7 +948,6 @@ describe('executeTool Function', () => {
948948
__blockRef_0: { field: 'resolved-output' },
949949
__blockRef_1: largeValueRef,
950950
},
951-
userId: undefined,
952951
workspaceId: 'workspace-456',
953952
workflowId: 'workflow-1',
954953
executionId: 'execution-1',
@@ -960,6 +959,7 @@ describe('executeTool Function', () => {
960959
}),
961960
})
962961
)
962+
expect(mockExecuteFunction.mock.calls[0]?.[0].input.body.userId).toBeUndefined()
963963
expect(mockGenerateInternalToken).not.toHaveBeenCalled()
964964
expect(fetchSpy).not.toHaveBeenCalled()
965965
})
@@ -1042,7 +1042,6 @@ describe('executeTool Function', () => {
10421042
workflowId: 'workflow-1',
10431043
executionId: 'execution-1',
10441044
workspaceId: 'workspace-456',
1045-
userId: undefined,
10461045
largeValueExecutionIds: ['execution-1'],
10471046
largeValueKeys: ['lv_ABCDEFGHIJKL'],
10481047
fileKeys: ['file-1'],
@@ -1051,6 +1050,7 @@ describe('executeTool Function', () => {
10511050
},
10521051
},
10531052
})
1053+
expect(mockExecuteFunction.mock.calls[0]?.[0].input.body.userId).toBeUndefined()
10541054
expect(mockGenerateInternalToken).not.toHaveBeenCalled()
10551055
expect(fetchSpy).not.toHaveBeenCalled()
10561056
})

‎apps/sim/tools/params.test.ts‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
2+
import type { ExecutorDelegationOrigin } from '@/executor/types'
23
import { mergeToolParameters } from '@/tools/merge-params'
34
import * as toolMetadata from '@/tools/metadata'
45
import {
@@ -634,6 +635,13 @@ describe('Tool Parameters Utils', () => {
634635

635636
describe('createLLMToolSchema - child workflow input enrichment', () => {
636637
const mockReadWorkflowInputFields = vi.fn()
638+
const executorDelegationOrigin: ExecutorDelegationOrigin = {
639+
subjectUserId: 'user-1',
640+
workflowId: 'parent-workflow',
641+
executionId: 'execution-1',
642+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
643+
currentWorkflow: { workflowId: 'parent-workflow', mode: 'draft' },
644+
}
637645

638646
beforeEach(() => {
639647
mockReadWorkflowInputFields.mockReset()
@@ -652,6 +660,7 @@ describe('Tool Parameters Utils', () => {
652660
workflowId: 'parent-workflow',
653661
executionId: 'execution-1',
654662
workspaceId: 'workspace-1',
663+
executorDelegationOrigin,
655664
},
656665
mockReadWorkflowInputFields
657666
)
@@ -661,6 +670,7 @@ describe('Tool Parameters Utils', () => {
661670
workflowId: 'parent-workflow',
662671
executionId: 'execution-1',
663672
workspaceId: 'workspace-1',
673+
executorDelegationOrigin,
664674
})
665675
expect(schema.properties.inputMapping.properties).toEqual({
666676
email: { type: 'string', description: 'Recipient address' },
@@ -673,18 +683,24 @@ describe('Tool Parameters Utils', () => {
673683
await createLLMToolSchema(
674684
mockWorkflowExecutorConfig,
675685
{ workflowId: 'parent-workflow' },
676-
{ userId: 'user-1', workflowId: 'parent-workflow', executionId: 'execution-1' },
686+
{
687+
userId: 'user-1',
688+
workflowId: 'parent-workflow',
689+
executionId: 'execution-1',
690+
executorDelegationOrigin,
691+
},
677692
mockReadWorkflowInputFields
678693
)
679694

680695
expect(mockReadWorkflowInputFields).toHaveBeenCalledWith('parent-workflow', {
681696
userId: 'user-1',
682697
workflowId: 'parent-workflow',
683698
executionId: 'execution-1',
699+
executorDelegationOrigin,
684700
})
685701
})
686702

687-
it('leaves inputMapping untyped and issues no request without an execution subject', async () => {
703+
it('leaves inputMapping untyped and issues no request without trusted execution authority', async () => {
688704
const { schema } = await createLLMToolSchema(
689705
mockWorkflowExecutorConfig,
690706
{ workflowId: 'child-workflow' },
@@ -702,10 +718,19 @@ describe('Tool Parameters Utils', () => {
702718
const { schema } = await createLLMToolSchema(
703719
mockWorkflowExecutorConfig,
704720
{ workflowId: 'child-workflow' },
705-
{ userId: 'user-1', workflowId: 'parent-workflow' },
721+
{
722+
userId: 'user-1',
723+
workflowId: 'parent-workflow',
724+
executorDelegationOrigin,
725+
},
706726
mockReadWorkflowInputFields
707727
)
708728

729+
expect(mockReadWorkflowInputFields).toHaveBeenCalledWith('child-workflow', {
730+
userId: 'user-1',
731+
workflowId: 'parent-workflow',
732+
executorDelegationOrigin,
733+
})
709734
expect(schema.properties.inputMapping.properties).toBeUndefined()
710735
})
711736
})

0 commit comments

Comments
 (0)