Skip to content

Commit 5798075

Browse files
fix(executor): preserve actors for actorless tool calls
1 parent 2bda859 commit 5798075

38 files changed

Lines changed: 473 additions & 161 deletions

apps/sim/executor/handlers/credential-group/credential-group-handler.test.ts

Lines changed: 86 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ import type { ExecutionContext } from '@/executor/types'
88
import type { SerializedBlock } from '@/serializer/types'
99

1010
const mocks = vi.hoisted(() => ({
11-
authenticate: vi.fn(),
12-
buildHeaders: vi.fn(),
11+
createPrincipal: vi.fn(),
1312
createInviteLink: vi.fn(),
1413
enforceInviteRateLimit: vi.fn(),
1514
listCredentials: vi.fn(),
@@ -22,10 +21,6 @@ vi.mock('@/lib/credential-groups/application/create-invite-link', () => ({
2221
createCredentialGroupInviteLink: { execute: mocks.createInviteLink },
2322
}))
2423

25-
vi.mock('@/lib/credential-groups/application/delegation', () => ({
26-
authenticateCredentialGroupDelegation: mocks.authenticate,
27-
}))
28-
2924
vi.mock('@/lib/credential-groups/application/list-credentials', () => ({
3025
listCredentialGroupCredentials: { execute: mocks.listCredentials },
3126
}))
@@ -53,8 +48,8 @@ vi.mock('@/lib/credential-groups/rate-limit', () => ({
5348
enforceCredentialGroupInvitationExecutionRateLimit: mocks.enforceInviteRateLimit,
5449
}))
5550

56-
vi.mock('@/executor/utils/http', () => ({
57-
buildExecutorDelegationHeaders: mocks.buildHeaders,
51+
vi.mock('@/lib/internal/principals/executor', () => ({
52+
createExecutorPrincipalFromExecutionContext: mocks.createPrincipal,
5853
}))
5954

6055
import { CredentialGroupBlockHandler } from '@/executor/handlers/credential-group/credential-group-handler'
@@ -92,8 +87,7 @@ const block = { metadata: { id: BlockType.CREDENTIAL_GROUP } } as SerializedBloc
9287
describe('CredentialGroupBlockHandler', () => {
9388
beforeEach(() => {
9489
vi.clearAllMocks()
95-
mocks.buildHeaders.mockResolvedValue({ Authorization: 'Bearer executor-token' })
96-
mocks.authenticate.mockResolvedValue(principal)
90+
mocks.createPrincipal.mockResolvedValue(principal)
9791
})
9892

9993
it('recognizes only Credential Group blocks', () => {
@@ -122,7 +116,11 @@ describe('CredentialGroupBlockHandler', () => {
122116
cursor: ' credential-1 ',
123117
})
124118

125-
expect(mocks.authenticate).toHaveBeenCalledWith('Bearer executor-token', 'group-1')
119+
expect(mocks.createPrincipal).toHaveBeenCalledWith({
120+
context,
121+
audience: 'sim:credential-groups',
122+
resourceScope: { credentialGroupId: 'group-1' },
123+
})
126124
expect(mocks.listCredentials).toHaveBeenCalledWith({
127125
principal,
128126
input: {
@@ -136,6 +134,73 @@ describe('CredentialGroupBlockHandler', () => {
136134
expect(result).toEqual({ credentials: [], count: 0, hasMore: false, nextCursor: null })
137135
})
138136

137+
it('lists credentials for an actorless workflow execution', async () => {
138+
const executionPrincipal = {
139+
kind: 'system' as const,
140+
serviceId: 'schedule' as const,
141+
workspaceId: 'workspace-1',
142+
workflowId: 'workflow-1',
143+
}
144+
const actorlessPrincipal: WorkflowExecutionDelegatedPrincipal = {
145+
kind: 'delegated',
146+
serviceId: 'executor',
147+
workspaceId: 'workspace-1',
148+
delegationId: 'delegation-actorless',
149+
audience: 'sim:credential-groups',
150+
issuedAt: new Date(Date.now() - 1_000),
151+
expiresAt: new Date(Date.now() + 60_000),
152+
resourceScope: { credentialGroupId: 'group-1' },
153+
delegationContext: {
154+
kind: 'workflow_execution',
155+
workflowId: 'workflow-1',
156+
principal: executionPrincipal,
157+
currentWorkflow: {
158+
workflowId: 'workflow-1',
159+
mode: 'deployment',
160+
deploymentVersionId: 'deployment-version-1',
161+
},
162+
},
163+
}
164+
const actorlessContext = {
165+
...context,
166+
userId: undefined,
167+
principal: executionPrincipal,
168+
executorDelegationOrigin: {
169+
workflowId: 'workflow-1',
170+
principal: executionPrincipal,
171+
currentWorkflow: actorlessPrincipal.delegationContext.currentWorkflow,
172+
},
173+
} as ExecutionContext
174+
mocks.createPrincipal.mockResolvedValueOnce(actorlessPrincipal)
175+
mocks.listCredentials.mockResolvedValue({
176+
credentials: [],
177+
count: 0,
178+
hasMore: false,
179+
nextCursor: null,
180+
})
181+
182+
await new CredentialGroupBlockHandler().execute(actorlessContext, block, {
183+
operation: 'list_credentials',
184+
credentialGroupId: 'group-1',
185+
})
186+
187+
expect(mocks.createPrincipal).toHaveBeenCalledWith({
188+
context: actorlessContext,
189+
audience: 'sim:credential-groups',
190+
resourceScope: { credentialGroupId: 'group-1' },
191+
})
192+
expect(mocks.listCredentials).toHaveBeenCalledWith({
193+
principal: actorlessPrincipal,
194+
input: {
195+
credentialGroupId: 'group-1',
196+
limit: 100,
197+
cursor: undefined,
198+
email: undefined,
199+
credentialProviderIds: undefined,
200+
},
201+
})
202+
})
203+
139204
it('lists groups under workspace-scoped delegation', async () => {
140205
mocks.listGroups.mockResolvedValue({
141206
credentialGroups: [],
@@ -149,7 +214,10 @@ describe('CredentialGroupBlockHandler', () => {
149214
limit: 10,
150215
})
151216

152-
expect(mocks.authenticate).toHaveBeenCalledWith('Bearer executor-token', undefined)
217+
expect(mocks.createPrincipal).toHaveBeenCalledWith({
218+
context,
219+
audience: 'sim:credential-groups',
220+
})
153221
expect(mocks.listGroups).toHaveBeenCalledWith({
154222
principal,
155223
input: { workspaceId: 'workspace-1', limit: 10, cursor: undefined },
@@ -201,7 +269,11 @@ describe('CredentialGroupBlockHandler', () => {
201269
email: ' person@example.com ',
202270
})
203271

204-
expect(mocks.authenticate).toHaveBeenCalledWith('Bearer executor-token', 'group-1')
272+
expect(mocks.createPrincipal).toHaveBeenCalledWith({
273+
context,
274+
audience: 'sim:credential-groups',
275+
resourceScope: { credentialGroupId: 'group-1' },
276+
})
205277
expect(mocks.enforceInviteRateLimit).toHaveBeenCalledWith('workspace-1')
206278
expect(mocks.enforceInviteRateLimit.mock.invocationCallOrder[0]).toBeLessThan(
207279
mocks.createInviteLink.mock.invocationCallOrder[0]!
@@ -236,7 +308,6 @@ describe('CredentialGroupBlockHandler', () => {
236308
await expect(
237309
new CredentialGroupBlockHandler().execute(context, block, { operation: 'unknown' })
238310
).rejects.toThrow('Unsupported Credential Group operation: unknown')
239-
expect(mocks.buildHeaders).not.toHaveBeenCalled()
240-
expect(mocks.authenticate).not.toHaveBeenCalled()
311+
expect(mocks.createPrincipal).not.toHaveBeenCalled()
241312
})
242313
})

apps/sim/executor/handlers/credential-group/credential-group-handler.ts

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createLogger } from '@sim/logger'
2+
import { CREDENTIAL_GROUP_DELEGATION_AUDIENCE } from '@/lib/credential-groups/application/authorization'
23
import { createCredentialGroupInviteLink } from '@/lib/credential-groups/application/create-invite-link'
3-
import { authenticateCredentialGroupDelegation } from '@/lib/credential-groups/application/delegation'
44
import { listCredentialGroupCredentials } from '@/lib/credential-groups/application/list-credentials'
55
import { listCredentialGroupsForWorkflow } from '@/lib/credential-groups/application/list-groups'
66
import {
@@ -11,10 +11,10 @@ import { sendCredentialGroupInvite } from '@/lib/credential-groups/application/s
1111
import { MAX_CREDENTIAL_GROUP_CREDENTIAL_PAGE_SIZE } from '@/lib/credential-groups/credentials'
1212
import type { CredentialGroupEnrollmentStatus } from '@/lib/credential-groups/enrollments'
1313
import { enforceCredentialGroupInvitationExecutionRateLimit } from '@/lib/credential-groups/rate-limit'
14+
import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor'
1415
import type { BlockOutput } from '@/blocks/types'
1516
import { BlockType } from '@/executor/constants'
16-
import type { BlockHandler, ExecutionContext, ExecutorDelegationOrigin } from '@/executor/types'
17-
import { buildExecutorDelegationHeaders } from '@/executor/utils/http'
17+
import type { BlockHandler, ExecutionContext } from '@/executor/types'
1818
import type { SerializedBlock } from '@/serializer/types'
1919

2020
const logger = createLogger('CredentialGroupBlockHandler')
@@ -84,13 +84,6 @@ function requireString(value: unknown, label: string): string {
8484
return parsed
8585
}
8686

87-
function delegationOrigin(ctx: ExecutionContext): ExecutorDelegationOrigin {
88-
if (!ctx.executorDelegationOrigin) {
89-
throw new Error('Credential Group operations require an authenticated workflow execution')
90-
}
91-
return ctx.executorDelegationOrigin
92-
}
93-
9487
export class CredentialGroupBlockHandler implements BlockHandler {
9588
canHandle(block: SerializedBlock): boolean {
9689
return block.metadata?.id === BlockType.CREDENTIAL_GROUP
@@ -103,14 +96,18 @@ export class CredentialGroupBlockHandler implements BlockHandler {
10396
): Promise<BlockOutput> {
10497
if (!ctx.workspaceId) throw new Error('workspaceId is required for Credential Group operations')
10598
const operation = parseOperation(inputs.operation)
99+
if (!ctx.executorDelegationOrigin) {
100+
throw new Error('Credential Group operations require an authenticated workflow execution')
101+
}
106102
const credentialGroupId =
107103
operation === 'list_groups'
108104
? undefined
109105
: requireString(inputs.credentialGroupId, 'Credential Group')
110-
const headers = await buildExecutorDelegationHeaders(delegationOrigin(ctx))
111-
const authorization = headers.Authorization
112-
if (!authorization) throw new Error('Executor delegation authorization is missing')
113-
const principal = await authenticateCredentialGroupDelegation(authorization, credentialGroupId)
106+
const principal = await createExecutorPrincipalFromExecutionContext({
107+
context: ctx,
108+
audience: CREDENTIAL_GROUP_DELEGATION_AUDIENCE,
109+
...(credentialGroupId ? { resourceScope: { credentialGroupId } } : {}),
110+
})
114111

115112
switch (operation) {
116113
case 'list_credentials': {

apps/sim/lib/copilot/tools/handlers/deployment/manage.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ export async function executeLoadDeployment(
395395
workflowId,
396396
assertedWorkspaceId: context.workspaceId,
397397
version: target.version,
398+
executionActorUserId: context.userId,
398399
})
399400

400401
const label = target.version === 'active' ? 'the live deployment' : `version ${target.version}`

apps/sim/lib/copilot/tools/handlers/oauth.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ describe('executeOAuthGetAuthLink', () => {
5353
workspaceId: 'workspace-1',
5454
providerName: 'gmail',
5555
credentialId: undefined,
56+
executionActorUserId: 'user-1',
5657
})
5758
const url = new URL((result.output as { oauth_url: string }).oauth_url)
5859
expect(url.pathname).toBe('/api/auth/oauth2/authorize')

apps/sim/lib/copilot/tools/handlers/oauth.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export async function executeOAuthGetAuthLink(
3939
workspaceId,
4040
providerName,
4141
credentialId,
42+
executionActorUserId: context.userId,
4243
})
4344
const callbackURL = context.workflowId
4445
? `${baseUrl}/workspace/${workspaceId}/w/${context.workflowId}`

apps/sim/lib/core/orchestration/types.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,11 @@ export function asOrchestrationError(error: unknown): OrchestrationError | null
110110
}
111111

112112
/**
113-
* The slice of an HTTP request the audit log reads for client IP and user-agent
114-
* capture. Optional on every orchestration function so the non-HTTP callers —
115-
* copilot tools, background jobs — can omit what they do not have.
113+
* Transport metadata available to an application operation. HTTP callers carry
114+
* headers for audit capture; executor adapters may also preserve the legacy
115+
* execution actor used by pre-application-boundary internal routes.
116116
*/
117117
export interface OrchestrationRequestContext {
118118
headers: { get(name: string): string | null }
119+
executionActorUserId?: string
119120
}

apps/sim/lib/credential-groups/application/delegation.ts

Lines changed: 0 additions & 41 deletions
This file was deleted.

apps/sim/lib/credentials/application/authorization.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import type { Principal } from '@sim/auth/principal'
1+
import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal'
22
import type { WorkspaceDelegationPolicy } from '@/lib/core/application'
3+
import { OrchestrationError } from '@/lib/core/orchestration/types'
34
import type { ManagedOAuthCredentialApplicationContext } from '@/lib/credentials/managed-oauth'
45

56
export const CREDENTIAL_DELEGATION_AUDIENCE = 'sim:credentials'
@@ -21,3 +22,24 @@ export const managedOAuthCredentialDelegationPolicy = {
2122
context: ManagedOAuthCredentialApplicationContext
2223
) => principal.resourceScope?.credentialId === context.credentialId,
2324
} satisfies WorkspaceDelegationPolicy<ManagedOAuthCredentialApplicationContext>
25+
26+
/**
27+
* Resolves the user whose credential grants an operation evaluates.
28+
*
29+
* `executionActorUserId` is the user the legacy internal route authenticated as.
30+
* Workspace authorization remains principal-based, and a principal subject
31+
* always takes precedence over this compatibility value.
32+
*/
33+
export function requireCredentialExecutionUserId(
34+
principal: Principal,
35+
executionActorUserId?: string
36+
): string {
37+
const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId
38+
if (!userId) {
39+
throw new OrchestrationError(
40+
'forbidden',
41+
'Credential access requires a user subject or execution actor'
42+
)
43+
}
44+
return userId
45+
}

apps/sim/lib/credentials/application/authorized-credential-use-case.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import type { Principal } from '@sim/auth/principal'
2-
import { requirePrincipalSubjectUserId } from '@sim/auth/principal'
32
import {
43
type AuthorizedWorkspaceUseCaseDefinition,
54
defineAuthorizedWorkspaceUseCase,
@@ -9,7 +8,10 @@ import {
98
import { OrchestrationError } from '@/lib/core/orchestration/types'
109
import type { CredentialActorContext } from '@/lib/credentials/access'
1110
import { getCredentialActorContext } from '@/lib/credentials/access'
12-
import { credentialDelegationPolicy } from '@/lib/credentials/application/authorization'
11+
import {
12+
credentialDelegationPolicy,
13+
requireCredentialExecutionUserId,
14+
} from '@/lib/credentials/application/authorization'
1315
import type { CredentialOperation } from '@/lib/credentials/application/operations'
1416
import type { CredentialRow } from '@/lib/credentials/queries'
1517

@@ -74,7 +76,9 @@ type AuthorizedCredentialUseCaseDefinition<
7476
> = Omit<
7577
AuthorizedWorkspaceUseCaseDefinition<O, I, C, R>,
7678
'authorizationOptions' | 'authorizeResource'
77-
>
79+
> & {
80+
resolveExecutionActorUserId?: (input: I) => string | undefined
81+
}
7882

7983
export function defineAuthorizedCredentialUseCase<
8084
const O extends CredentialOperation,
@@ -85,11 +89,10 @@ export function defineAuthorizedCredentialUseCase<
8589
return defineAuthorizedWorkspaceUseCase({
8690
...definition,
8791
authorizationOptions: { delegation: credentialDelegationPolicy },
88-
async authorizeResource({ principal, context }) {
92+
async authorizeResource({ principal, input, context }) {
8993
const actor = await getCredentialActorContext(
9094
context.credential.id,
91-
// actorless-unsupported: credential access is decided per person; an actorless run has no credential grants
92-
requirePrincipalSubjectUserId(principal)
95+
requireCredentialExecutionUserId(principal, definition.resolveExecutionActorUserId?.(input))
9396
)
9497
if (
9598
!actor.credential ||

0 commit comments

Comments
 (0)