From 1aa7db86f8d823f60b538806150c04dcb1294fd4 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Fri, 28 Aug 2026 13:48:57 -0700 Subject: [PATCH 1/2] fix(executor): preserve actors for actorless tool calls --- .../credential-group-handler.test.ts | 101 +++++++++++++++--- .../credential-group-handler.ts | 25 ++--- .../tools/handlers/deployment/manage.ts | 1 + .../lib/copilot/tools/handlers/oauth.test.ts | 1 + apps/sim/lib/copilot/tools/handlers/oauth.ts | 1 + apps/sim/lib/core/orchestration/types.ts | 7 +- .../application/delegation.ts | 41 ------- .../credentials/application/authorization.ts | 24 ++++- .../authorized-credential-use-case.ts | 15 +-- .../application/connection-target.test.ts | 21 ++++ .../application/connection-target.ts | 10 +- .../application/credential-crud.ts | 3 + .../prepare-credential-connection.ts | 2 + .../application/service-account.ts | 2 + .../custom-tools/application/authorization.ts | 24 +++++ .../application/use-cases.test.ts | 33 +++++- .../lib/custom-tools/application/use-cases.ts | 38 +++---- .../read-available-by-id-or-title.test.ts | 18 ++++ .../read-available-by-id-or-title.ts | 1 + .../lib/internal/file/execute-tool.test.ts | 2 + apps/sim/lib/internal/file/execute-tool.ts | 1 + apps/sim/lib/internal/file/operations.ts | 2 + .../internal/knowledge/execute-tool.test.ts | 1 + .../lib/internal/knowledge/execute-tool.ts | 7 +- apps/sim/lib/internal/knowledge/operations.ts | 36 ++++--- .../internal/windchill/execute-tool.test.ts | 1 + .../lib/internal/windchill/execute-tool.ts | 7 +- .../lib/internal/windchill/operations.test.ts | 58 ++++++++++ apps/sim/lib/internal/windchill/operations.ts | 32 ++++-- .../lib/knowledge/api/internal-route.test.ts | 5 +- apps/sim/lib/knowledge/api/internal-route.ts | 17 +-- .../knowledge/application/authorization.ts | 5 +- .../authorized-knowledge-use-case.ts | 15 ++- apps/sim/lib/knowledge/application/billing.ts | 18 +++- .../application/authorization.test.ts | 13 +++ .../workflows/application/authorization.ts | 18 +++- .../lib/workflows/application/deployments.ts | 12 +-- .../application/share-workspace-file.ts | 16 ++- 38 files changed, 473 insertions(+), 161 deletions(-) delete mode 100644 apps/sim/lib/credential-groups/application/delegation.ts diff --git a/apps/sim/executor/handlers/credential-group/credential-group-handler.test.ts b/apps/sim/executor/handlers/credential-group/credential-group-handler.test.ts index 0b39e8ac1bc..f1bdeaa44d5 100644 --- a/apps/sim/executor/handlers/credential-group/credential-group-handler.test.ts +++ b/apps/sim/executor/handlers/credential-group/credential-group-handler.test.ts @@ -8,8 +8,7 @@ import type { ExecutionContext } from '@/executor/types' import type { SerializedBlock } from '@/serializer/types' const mocks = vi.hoisted(() => ({ - authenticate: vi.fn(), - buildHeaders: vi.fn(), + createPrincipal: vi.fn(), createInviteLink: vi.fn(), enforceInviteRateLimit: vi.fn(), listCredentials: vi.fn(), @@ -22,10 +21,6 @@ vi.mock('@/lib/credential-groups/application/create-invite-link', () => ({ createCredentialGroupInviteLink: { execute: mocks.createInviteLink }, })) -vi.mock('@/lib/credential-groups/application/delegation', () => ({ - authenticateCredentialGroupDelegation: mocks.authenticate, -})) - vi.mock('@/lib/credential-groups/application/list-credentials', () => ({ listCredentialGroupCredentials: { execute: mocks.listCredentials }, })) @@ -53,8 +48,8 @@ vi.mock('@/lib/credential-groups/rate-limit', () => ({ enforceCredentialGroupInvitationExecutionRateLimit: mocks.enforceInviteRateLimit, })) -vi.mock('@/executor/utils/http', () => ({ - buildExecutorDelegationHeaders: mocks.buildHeaders, +vi.mock('@/lib/internal/principals/executor', () => ({ + createExecutorPrincipalFromExecutionContext: mocks.createPrincipal, })) import { CredentialGroupBlockHandler } from '@/executor/handlers/credential-group/credential-group-handler' @@ -92,8 +87,7 @@ const block = { metadata: { id: BlockType.CREDENTIAL_GROUP } } as SerializedBloc describe('CredentialGroupBlockHandler', () => { beforeEach(() => { vi.clearAllMocks() - mocks.buildHeaders.mockResolvedValue({ Authorization: 'Bearer executor-token' }) - mocks.authenticate.mockResolvedValue(principal) + mocks.createPrincipal.mockResolvedValue(principal) }) it('recognizes only Credential Group blocks', () => { @@ -122,7 +116,11 @@ describe('CredentialGroupBlockHandler', () => { cursor: ' credential-1 ', }) - expect(mocks.authenticate).toHaveBeenCalledWith('Bearer executor-token', 'group-1') + expect(mocks.createPrincipal).toHaveBeenCalledWith({ + context, + audience: 'sim:credential-groups', + resourceScope: { credentialGroupId: 'group-1' }, + }) expect(mocks.listCredentials).toHaveBeenCalledWith({ principal, input: { @@ -136,6 +134,73 @@ describe('CredentialGroupBlockHandler', () => { expect(result).toEqual({ credentials: [], count: 0, hasMore: false, nextCursor: null }) }) + it('lists credentials for an actorless workflow execution', async () => { + const executionPrincipal = { + kind: 'system' as const, + serviceId: 'schedule' as const, + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + } + const actorlessPrincipal: WorkflowExecutionDelegatedPrincipal = { + kind: 'delegated', + serviceId: 'executor', + workspaceId: 'workspace-1', + delegationId: 'delegation-actorless', + audience: 'sim:credential-groups', + issuedAt: new Date(Date.now() - 1_000), + expiresAt: new Date(Date.now() + 60_000), + resourceScope: { credentialGroupId: 'group-1' }, + delegationContext: { + kind: 'workflow_execution', + workflowId: 'workflow-1', + principal: executionPrincipal, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'deployment-version-1', + }, + }, + } + const actorlessContext = { + ...context, + userId: undefined, + principal: executionPrincipal, + executorDelegationOrigin: { + workflowId: 'workflow-1', + principal: executionPrincipal, + currentWorkflow: actorlessPrincipal.delegationContext.currentWorkflow, + }, + } as ExecutionContext + mocks.createPrincipal.mockResolvedValueOnce(actorlessPrincipal) + mocks.listCredentials.mockResolvedValue({ + credentials: [], + count: 0, + hasMore: false, + nextCursor: null, + }) + + await new CredentialGroupBlockHandler().execute(actorlessContext, block, { + operation: 'list_credentials', + credentialGroupId: 'group-1', + }) + + expect(mocks.createPrincipal).toHaveBeenCalledWith({ + context: actorlessContext, + audience: 'sim:credential-groups', + resourceScope: { credentialGroupId: 'group-1' }, + }) + expect(mocks.listCredentials).toHaveBeenCalledWith({ + principal: actorlessPrincipal, + input: { + credentialGroupId: 'group-1', + limit: 100, + cursor: undefined, + email: undefined, + credentialProviderIds: undefined, + }, + }) + }) + it('lists groups under workspace-scoped delegation', async () => { mocks.listGroups.mockResolvedValue({ credentialGroups: [], @@ -149,7 +214,10 @@ describe('CredentialGroupBlockHandler', () => { limit: 10, }) - expect(mocks.authenticate).toHaveBeenCalledWith('Bearer executor-token', undefined) + expect(mocks.createPrincipal).toHaveBeenCalledWith({ + context, + audience: 'sim:credential-groups', + }) expect(mocks.listGroups).toHaveBeenCalledWith({ principal, input: { workspaceId: 'workspace-1', limit: 10, cursor: undefined }, @@ -201,7 +269,11 @@ describe('CredentialGroupBlockHandler', () => { email: ' person@example.com ', }) - expect(mocks.authenticate).toHaveBeenCalledWith('Bearer executor-token', 'group-1') + expect(mocks.createPrincipal).toHaveBeenCalledWith({ + context, + audience: 'sim:credential-groups', + resourceScope: { credentialGroupId: 'group-1' }, + }) expect(mocks.enforceInviteRateLimit).toHaveBeenCalledWith('workspace-1') expect(mocks.enforceInviteRateLimit.mock.invocationCallOrder[0]).toBeLessThan( mocks.createInviteLink.mock.invocationCallOrder[0]! @@ -236,7 +308,6 @@ describe('CredentialGroupBlockHandler', () => { await expect( new CredentialGroupBlockHandler().execute(context, block, { operation: 'unknown' }) ).rejects.toThrow('Unsupported Credential Group operation: unknown') - expect(mocks.buildHeaders).not.toHaveBeenCalled() - expect(mocks.authenticate).not.toHaveBeenCalled() + expect(mocks.createPrincipal).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/executor/handlers/credential-group/credential-group-handler.ts b/apps/sim/executor/handlers/credential-group/credential-group-handler.ts index a7710731a7f..0bc0c24169a 100644 --- a/apps/sim/executor/handlers/credential-group/credential-group-handler.ts +++ b/apps/sim/executor/handlers/credential-group/credential-group-handler.ts @@ -1,6 +1,6 @@ import { createLogger } from '@sim/logger' +import { CREDENTIAL_GROUP_DELEGATION_AUDIENCE } from '@/lib/credential-groups/application/authorization' import { createCredentialGroupInviteLink } from '@/lib/credential-groups/application/create-invite-link' -import { authenticateCredentialGroupDelegation } from '@/lib/credential-groups/application/delegation' import { listCredentialGroupCredentials } from '@/lib/credential-groups/application/list-credentials' import { listCredentialGroupsForWorkflow } from '@/lib/credential-groups/application/list-groups' import { @@ -11,10 +11,10 @@ import { sendCredentialGroupInvite } from '@/lib/credential-groups/application/s import { MAX_CREDENTIAL_GROUP_CREDENTIAL_PAGE_SIZE } from '@/lib/credential-groups/credentials' import type { CredentialGroupEnrollmentStatus } from '@/lib/credential-groups/enrollments' import { enforceCredentialGroupInvitationExecutionRateLimit } from '@/lib/credential-groups/rate-limit' +import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' import type { BlockOutput } from '@/blocks/types' import { BlockType } from '@/executor/constants' -import type { BlockHandler, ExecutionContext, ExecutorDelegationOrigin } from '@/executor/types' -import { buildExecutorDelegationHeaders } from '@/executor/utils/http' +import type { BlockHandler, ExecutionContext } from '@/executor/types' import type { SerializedBlock } from '@/serializer/types' const logger = createLogger('CredentialGroupBlockHandler') @@ -84,13 +84,6 @@ function requireString(value: unknown, label: string): string { return parsed } -function delegationOrigin(ctx: ExecutionContext): ExecutorDelegationOrigin { - if (!ctx.executorDelegationOrigin) { - throw new Error('Credential Group operations require an authenticated workflow execution') - } - return ctx.executorDelegationOrigin -} - export class CredentialGroupBlockHandler implements BlockHandler { canHandle(block: SerializedBlock): boolean { return block.metadata?.id === BlockType.CREDENTIAL_GROUP @@ -103,14 +96,18 @@ export class CredentialGroupBlockHandler implements BlockHandler { ): Promise { if (!ctx.workspaceId) throw new Error('workspaceId is required for Credential Group operations') const operation = parseOperation(inputs.operation) + if (!ctx.executorDelegationOrigin) { + throw new Error('Credential Group operations require an authenticated workflow execution') + } const credentialGroupId = operation === 'list_groups' ? undefined : requireString(inputs.credentialGroupId, 'Credential Group') - const headers = await buildExecutorDelegationHeaders(delegationOrigin(ctx)) - const authorization = headers.Authorization - if (!authorization) throw new Error('Executor delegation authorization is missing') - const principal = await authenticateCredentialGroupDelegation(authorization, credentialGroupId) + const principal = await createExecutorPrincipalFromExecutionContext({ + context: ctx, + audience: CREDENTIAL_GROUP_DELEGATION_AUDIENCE, + ...(credentialGroupId ? { resourceScope: { credentialGroupId } } : {}), + }) switch (operation) { case 'list_credentials': { diff --git a/apps/sim/lib/copilot/tools/handlers/deployment/manage.ts b/apps/sim/lib/copilot/tools/handlers/deployment/manage.ts index 156d83d2a8a..cba01b2e018 100644 --- a/apps/sim/lib/copilot/tools/handlers/deployment/manage.ts +++ b/apps/sim/lib/copilot/tools/handlers/deployment/manage.ts @@ -395,6 +395,7 @@ export async function executeLoadDeployment( workflowId, assertedWorkspaceId: context.workspaceId, version: target.version, + executionActorUserId: context.userId, }) const label = target.version === 'active' ? 'the live deployment' : `version ${target.version}` diff --git a/apps/sim/lib/copilot/tools/handlers/oauth.test.ts b/apps/sim/lib/copilot/tools/handlers/oauth.test.ts index f3926a3ed11..df151222e76 100644 --- a/apps/sim/lib/copilot/tools/handlers/oauth.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/oauth.test.ts @@ -53,6 +53,7 @@ describe('executeOAuthGetAuthLink', () => { workspaceId: 'workspace-1', providerName: 'gmail', credentialId: undefined, + executionActorUserId: 'user-1', }) const url = new URL((result.output as { oauth_url: string }).oauth_url) expect(url.pathname).toBe('/api/auth/oauth2/authorize') diff --git a/apps/sim/lib/copilot/tools/handlers/oauth.ts b/apps/sim/lib/copilot/tools/handlers/oauth.ts index eb24cb86ab6..50bd8f774d6 100644 --- a/apps/sim/lib/copilot/tools/handlers/oauth.ts +++ b/apps/sim/lib/copilot/tools/handlers/oauth.ts @@ -39,6 +39,7 @@ export async function executeOAuthGetAuthLink( workspaceId, providerName, credentialId, + executionActorUserId: context.userId, }) const callbackURL = context.workflowId ? `${baseUrl}/workspace/${workspaceId}/w/${context.workflowId}` diff --git a/apps/sim/lib/core/orchestration/types.ts b/apps/sim/lib/core/orchestration/types.ts index f46c0a4ebbd..454b3d3f5f2 100644 --- a/apps/sim/lib/core/orchestration/types.ts +++ b/apps/sim/lib/core/orchestration/types.ts @@ -110,10 +110,11 @@ export function asOrchestrationError(error: unknown): OrchestrationError | null } /** - * The slice of an HTTP request the audit log reads for client IP and user-agent - * capture. Optional on every orchestration function so the non-HTTP callers — - * copilot tools, background jobs — can omit what they do not have. + * Transport metadata available to an application operation. HTTP callers carry + * headers for audit capture; executor adapters may also preserve the legacy + * execution actor used by pre-application-boundary internal routes. */ export interface OrchestrationRequestContext { headers: { get(name: string): string | null } + executionActorUserId?: string } diff --git a/apps/sim/lib/credential-groups/application/delegation.ts b/apps/sim/lib/credential-groups/application/delegation.ts deleted file mode 100644 index 56ad6895c0a..00000000000 --- a/apps/sim/lib/credential-groups/application/delegation.ts +++ /dev/null @@ -1,41 +0,0 @@ -import type { WorkflowExecutionDelegatedPrincipal } from '@sim/auth/principal' -import { - InvalidInternalDelegationTokenError, - verifyInternalDelegationToken, -} from '@/lib/auth/internal' -import { - bindInternalExecutorDelegation, - InvalidInternalDelegationBindingError, -} from '@/lib/auth/internal-delegation' -import { CREDENTIAL_GROUP_DELEGATION_AUDIENCE } from '@/lib/credential-groups/application/authorization' - -export class InvalidCredentialGroupDelegationError extends Error { - constructor() { - super('Credential Group execution requires valid workflow delegation') - this.name = 'InvalidCredentialGroupDelegationError' - } -} - -/** Authenticates and binds executor claims to Credential Group application scope. */ -export async function authenticateCredentialGroupDelegation( - authorization: string, - credentialGroupId?: string -): Promise { - if (!authorization.startsWith('Bearer ')) throw new InvalidCredentialGroupDelegationError() - - try { - const claims = await verifyInternalDelegationToken(authorization.slice('Bearer '.length)) - return await bindInternalExecutorDelegation(claims, { - audience: CREDENTIAL_GROUP_DELEGATION_AUDIENCE, - ...(credentialGroupId ? { resourceScope: { credentialGroupId } } : {}), - }) - } catch (error) { - if ( - error instanceof InvalidInternalDelegationTokenError || - error instanceof InvalidInternalDelegationBindingError - ) { - throw new InvalidCredentialGroupDelegationError() - } - throw error - } -} diff --git a/apps/sim/lib/credentials/application/authorization.ts b/apps/sim/lib/credentials/application/authorization.ts index fdcca435bb6..f32c8bd950b 100644 --- a/apps/sim/lib/credentials/application/authorization.ts +++ b/apps/sim/lib/credentials/application/authorization.ts @@ -1,5 +1,6 @@ -import type { Principal } from '@sim/auth/principal' +import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal' import type { WorkspaceDelegationPolicy } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' import type { ManagedOAuthCredentialApplicationContext } from '@/lib/credentials/managed-oauth' export const CREDENTIAL_DELEGATION_AUDIENCE = 'sim:credentials' @@ -21,3 +22,24 @@ export const managedOAuthCredentialDelegationPolicy = { context: ManagedOAuthCredentialApplicationContext ) => principal.resourceScope?.credentialId === context.credentialId, } satisfies WorkspaceDelegationPolicy + +/** + * Resolves the user whose credential grants an operation evaluates. + * + * `executionActorUserId` is the user the legacy internal route authenticated as. + * Workspace authorization remains principal-based, and a principal subject + * always takes precedence over this compatibility value. + */ +export function requireCredentialExecutionUserId( + principal: Principal, + executionActorUserId?: string +): string { + const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId + if (!userId) { + throw new OrchestrationError( + 'forbidden', + 'Credential access requires a user subject or execution actor' + ) + } + return userId +} diff --git a/apps/sim/lib/credentials/application/authorized-credential-use-case.ts b/apps/sim/lib/credentials/application/authorized-credential-use-case.ts index 6fe719b99a8..06df33e76c2 100644 --- a/apps/sim/lib/credentials/application/authorized-credential-use-case.ts +++ b/apps/sim/lib/credentials/application/authorized-credential-use-case.ts @@ -1,5 +1,4 @@ import type { Principal } from '@sim/auth/principal' -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { type AuthorizedWorkspaceUseCaseDefinition, defineAuthorizedWorkspaceUseCase, @@ -9,7 +8,10 @@ import { import { OrchestrationError } from '@/lib/core/orchestration/types' import type { CredentialActorContext } from '@/lib/credentials/access' import { getCredentialActorContext } from '@/lib/credentials/access' -import { credentialDelegationPolicy } from '@/lib/credentials/application/authorization' +import { + credentialDelegationPolicy, + requireCredentialExecutionUserId, +} from '@/lib/credentials/application/authorization' import type { CredentialOperation } from '@/lib/credentials/application/operations' import type { CredentialRow } from '@/lib/credentials/queries' @@ -74,7 +76,9 @@ type AuthorizedCredentialUseCaseDefinition< > = Omit< AuthorizedWorkspaceUseCaseDefinition, 'authorizationOptions' | 'authorizeResource' -> +> & { + resolveExecutionActorUserId?: (input: I) => string | undefined +} export function defineAuthorizedCredentialUseCase< const O extends CredentialOperation, @@ -85,11 +89,10 @@ export function defineAuthorizedCredentialUseCase< return defineAuthorizedWorkspaceUseCase({ ...definition, authorizationOptions: { delegation: credentialDelegationPolicy }, - async authorizeResource({ principal, context }) { + async authorizeResource({ principal, input, context }) { const actor = await getCredentialActorContext( context.credential.id, - // actorless-unsupported: credential access is decided per person; an actorless run has no credential grants - requirePrincipalSubjectUserId(principal) + requireCredentialExecutionUserId(principal, definition.resolveExecutionActorUserId?.(input)) ) if ( !actor.credential || diff --git a/apps/sim/lib/credentials/application/connection-target.test.ts b/apps/sim/lib/credentials/application/connection-target.test.ts index 143a068961e..e3d533c27ec 100644 --- a/apps/sim/lib/credentials/application/connection-target.test.ts +++ b/apps/sim/lib/credentials/application/connection-target.test.ts @@ -137,6 +137,27 @@ describe('resolveCredentialConnectionTarget', () => { }) }) + it('uses the legacy execution actor for an actorless reconnect', async () => { + const actorlessPrincipal = { + kind: 'delegated' as const, + serviceId: 'executor' as const, + workspaceId: 'workspace-1', + delegationId: 'delegation-1', + audience: 'sim:credentials', + issuedAt: new Date('2026-08-28T00:00:00.000Z'), + expiresAt: new Date('2099-08-28T00:00:00.000Z'), + } + + await resolveCredentialConnectionTarget({ + principal: actorlessPrincipal, + context, + credentialId: 'credential-1', + executionActorUserId: 'execution-actor', + }) + + expect(mocks.getCredentialActorContext).toHaveBeenCalledWith('credential-1', 'execution-actor') + }) + it('rejects providers whose custom flow cannot reconnect', async () => { mocks.listCatalog.mockResolvedValue([{ ...salesforceProvider, supportsReconnect: false }]) diff --git a/apps/sim/lib/credentials/application/connection-target.ts b/apps/sim/lib/credentials/application/connection-target.ts index 38dfcd482b7..95ee9ef579e 100644 --- a/apps/sim/lib/credentials/application/connection-target.ts +++ b/apps/sim/lib/credentials/application/connection-target.ts @@ -1,7 +1,8 @@ -import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal' +import type { Principal } from '@sim/auth/principal' import { ForbiddenOperationError } from '@/lib/core/application/forbidden' import { OrchestrationError } from '@/lib/core/orchestration/types' import { getCredentialActorContext } from '@/lib/credentials/access' +import { requireCredentialExecutionUserId } from '@/lib/credentials/application/authorization' import { listCredentialProviderCatalog, type OAuthCredentialProviderCatalogEntry, @@ -31,8 +32,10 @@ export async function resolveCredentialConnectionTarget(params: { providerId?: string credentialId?: string assertedProviderId?: string + executionActorUserId?: string }): Promise { - const { principal, context, providerId, credentialId, assertedProviderId } = params + const { principal, context, providerId, credentialId, assertedProviderId, executionActorUserId } = + params if (Boolean(providerId) === Boolean(credentialId)) { throw new Error('Credential connection requires exactly one target identifier') } @@ -46,8 +49,7 @@ export async function resolveCredentialConnectionTarget(params: { } if (!credentialId) throw new Error('Credential reconnect target is missing its credential ID') - // actorless-unsupported: reconnecting rebinds a person's own OAuth grant - const userId = requirePrincipalSubjectUserId(principal) + const userId = requireCredentialExecutionUserId(principal, executionActorUserId) const targetCredentialId = credentialId const credential = await getWorkspaceCredential({ workspaceId: context.workspaceId, diff --git a/apps/sim/lib/credentials/application/credential-crud.ts b/apps/sim/lib/credentials/application/credential-crud.ts index c28bb5302a0..56e4f4c06ac 100644 --- a/apps/sim/lib/credentials/application/credential-crud.ts +++ b/apps/sim/lib/credentials/application/credential-crud.ts @@ -244,6 +244,7 @@ export type UpdateWorkspaceCredentialInput = Omit< PerformUpdateCredentialParams, 'userId' | 'actorName' | 'actorEmail' | 'allowedTypes' | 'reason' | 'request' > & { + executionActorUserId?: string /** * Workspace the caller asserts owns the credential; a mismatch is concealed as * a not-found. The internal surface omits it and resolves the credential's own @@ -254,6 +255,8 @@ export type UpdateWorkspaceCredentialInput = Omit< export const updateWorkspaceCredentialUseCase = defineAuthorizedCredentialUseCase({ operation: credentialOperations.update, + resolveExecutionActorUserId: (input: UpdateWorkspaceCredentialInput) => + input.executionActorUserId, resolveContext: ({ input }: { input: UpdateWorkspaceCredentialInput }) => resolveCredentialApplicationContext(input), async execute({ principal, input, context }) { diff --git a/apps/sim/lib/credentials/application/prepare-credential-connection.ts b/apps/sim/lib/credentials/application/prepare-credential-connection.ts index cedd08e89fd..8cf92d98d08 100644 --- a/apps/sim/lib/credentials/application/prepare-credential-connection.ts +++ b/apps/sim/lib/credentials/application/prepare-credential-connection.ts @@ -14,6 +14,7 @@ export interface PrepareCredentialConnectionInput { workspaceId: string providerName: string credentialId?: string + executionActorUserId?: string } export interface PrepareCredentialConnectionResult { @@ -85,6 +86,7 @@ export const prepareCredentialConnection = defineAuthorizedWorkspaceUseCase({ principal, context, credentialId: input.credentialId, + executionActorUserId: input.executionActorUserId, }) if ( !credentialProviderMatchesService(target.providerId, { diff --git a/apps/sim/lib/credentials/application/service-account.ts b/apps/sim/lib/credentials/application/service-account.ts index 6112b39a99d..d5b8ee838ed 100644 --- a/apps/sim/lib/credentials/application/service-account.ts +++ b/apps/sim/lib/credentials/application/service-account.ts @@ -133,6 +133,7 @@ export const createServiceAccountCredentialUseCase = defineAuthorizedWorkspaceUs export interface DeleteCredentialInput { workspaceId?: string credentialId: string + executionActorUserId?: string } export interface DeleteCredentialResult { @@ -142,6 +143,7 @@ export interface DeleteCredentialResult { export const deleteCredentialUseCase = defineAuthorizedCredentialUseCase({ operation: credentialOperations.delete, + resolveExecutionActorUserId: (input: DeleteCredentialInput) => input.executionActorUserId, resolveContext: ({ input }: { input: DeleteCredentialInput }) => resolveCredentialApplicationContext({ credentialId: input.credentialId, diff --git a/apps/sim/lib/custom-tools/application/authorization.ts b/apps/sim/lib/custom-tools/application/authorization.ts index 4dd7b31f2ce..8baac1840c2 100644 --- a/apps/sim/lib/custom-tools/application/authorization.ts +++ b/apps/sim/lib/custom-tools/application/authorization.ts @@ -1,4 +1,6 @@ +import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal' import type { WorkspaceDelegationPolicy } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' export const CUSTOM_TOOL_DELEGATION_AUDIENCE = 'sim:custom-tools' @@ -10,3 +12,25 @@ export const customToolDelegationPolicy = { workspaceOrganizationId: string | null allowPersonalApiKeys: boolean }> + +/** + * Resolves the user whose custom-tool library an operation reads or mutates. + * + * Actorless execution keeps the pre-application-boundary behavior by falling + * back to `ExecutionContext.userId`, the user the legacy internal route ran as. + * A real principal subject always wins, so the fallback is not an impersonation + * input and is never involved in workspace authorization. + */ +export function requireCustomToolUserId( + principal: Principal, + executionActorUserId?: string +): string { + const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId + if (!userId) { + throw new OrchestrationError( + 'forbidden', + 'Custom tools are resolved from a user library, and this run has no execution actor' + ) + } + return userId +} diff --git a/apps/sim/lib/custom-tools/application/use-cases.test.ts b/apps/sim/lib/custom-tools/application/use-cases.test.ts index 98acac58fe4..625cfdd6e79 100644 --- a/apps/sim/lib/custom-tools/application/use-cases.test.ts +++ b/apps/sim/lib/custom-tools/application/use-cases.test.ts @@ -131,7 +131,7 @@ describe('custom tool application use cases', () => { expect(mocks.audit).not.toHaveBeenCalled() }) - it('authorizes an actorless deployment without enabling personal fallback', async () => { + it('preserves the legacy execution actor for an actorless deployment', async () => { const result = await readAvailableCustomToolByIdOrTitleUseCase.execute({ principal: executorPrincipal({ subjectUserId: undefined, @@ -156,6 +156,7 @@ describe('custom tool application use cases', () => { workspaceId: workspace.workspaceId, identifier: tool.id, lookup: 'id_or_title', + executionActorUserId: 'execution-actor', }, }) @@ -163,11 +164,41 @@ describe('custom tool application use cases', () => { expect(mocks.resolvePermission).not.toHaveBeenCalled() expect(mocks.getAvailableTool).toHaveBeenCalledWith({ identifier: tool.id, + userId: 'execution-actor', workspaceId: workspace.workspaceId, lookup: 'id_or_title', }) }) + it('refuses actorless lookup when the legacy execution actor is missing', async () => { + await expect( + readAvailableCustomToolByIdOrTitleUseCase.execute({ + principal: executorPrincipal({ + subjectUserId: undefined, + delegationContext: { + kind: 'workflow_execution', + workflowId: 'workflow-1', + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'version-1', + }, + }, + }), + input: { + workspaceId: workspace.workspaceId, + identifier: tool.id, + lookup: 'id', + }, + }) + ).rejects.toMatchObject({ + code: 'forbidden', + message: + 'Custom tools are resolved from a user library, and this run has no execution actor', + }) + expect(mocks.getAvailableTool).not.toHaveBeenCalled() + }) + it('conceals a workspace assertion outside the delegated workspace before lookup', async () => { mocks.loadContext.mockResolvedValueOnce({ ...workspace, workspaceId: 'workspace-2' }) diff --git a/apps/sim/lib/custom-tools/application/use-cases.ts b/apps/sim/lib/custom-tools/application/use-cases.ts index a0a20abaebe..581684614f3 100644 --- a/apps/sim/lib/custom-tools/application/use-cases.ts +++ b/apps/sim/lib/custom-tools/application/use-cases.ts @@ -1,16 +1,14 @@ import { AuditAction, AuditResourceType } from '@sim/audit' -import { - type Principal, - requirePrincipalSubjectUserId, - resolvePrincipalAttribution, - resolvePrincipalSubject, -} from '@sim/auth/principal' +import { type Principal, resolvePrincipalAttribution } from '@sim/auth/principal' import type { customTools } from '@sim/db/schema' import { getErrorMessage, getPostgresErrorCode } from '@sim/utils/errors' import type { CursorKey, ListSortOrder } from '@/lib/api/list-query' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' -import { customToolDelegationPolicy } from '@/lib/custom-tools/application/authorization' +import { + customToolDelegationPolicy, + requireCustomToolUserId, +} from '@/lib/custom-tools/application/authorization' import { customToolOperations } from '@/lib/custom-tools/application/operations' import { assertStorableCustomToolSchema, @@ -66,12 +64,12 @@ async function resolveAvailableToolContext(args: { principal: Exclude workspaceId: string toolId: string + executionActorUserId?: string }): Promise { const workspace = await resolveWorkspaceContext(args.workspaceId) const tool = await getCustomToolById({ toolId: args.toolId, - // actorless-unsupported: a custom tool is owned by one user; an actorless run has no library to look in - userId: requirePrincipalSubjectUserId(args.principal), + userId: requireCustomToolUserId(args.principal, args.executionActorUserId), workspaceId: workspace.workspaceId, }) if (!tool) throw new OrchestrationError('not_found', 'Custom tool not found') @@ -121,6 +119,7 @@ export const listWorkspaceCustomToolsUseCase = defineAuthorizedWorkspaceUseCase( export interface ListAvailableCustomToolsInput { workspaceId: string + executionActorUserId?: string } export const listAvailableCustomToolsUseCase = defineAuthorizedWorkspaceUseCase({ @@ -128,10 +127,9 @@ export const listAvailableCustomToolsUseCase = defineAuthorizedWorkspaceUseCase( resolveContext: ({ input }: { input: ListAvailableCustomToolsInput }) => resolveWorkspaceContext(input.workspaceId), authorizationOptions, - async execute({ principal, context }) { + async execute({ principal, input, context }) { const tools = await listCustomTools({ - // actorless-unsupported: the listing is the acting user's own tool library, which an actorless run does not have - userId: requirePrincipalSubjectUserId(principal), + userId: requireCustomToolUserId(principal, input.executionActorUserId), workspaceId: context.workspaceId, }) return { tools } @@ -157,6 +155,7 @@ export interface ReadAvailableCustomToolByIdOrTitleInput { workspaceId: string identifier: string lookup: 'id' | 'id_or_title' + executionActorUserId?: string } export const readAvailableCustomToolByIdOrTitleUseCase = defineAuthorizedWorkspaceUseCase({ @@ -165,10 +164,9 @@ export const readAvailableCustomToolByIdOrTitleUseCase = defineAuthorizedWorkspa resolveWorkspaceContext(input.workspaceId), authorizationOptions, async execute({ principal, input, context }) { - const subject = resolvePrincipalSubject(principal) const tool = await getAvailableCustomTool({ identifier: input.identifier, - ...(subject?.kind === 'sim_user' ? { userId: subject.userId } : {}), + userId: requireCustomToolUserId(principal, input.executionActorUserId), workspaceId: context.workspaceId, lookup: input.lookup, }) @@ -262,6 +260,7 @@ interface UpdateCustomToolFields { schema?: unknown code?: string source?: CustomToolWriteSource + executionActorUserId?: string } export interface UpdateWorkspaceCustomToolInput extends UpdateCustomToolFields { @@ -339,6 +338,7 @@ export const updateAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase principal, workspaceId: input.workspaceId, toolId: input.toolId, + executionActorUserId: input.executionActorUserId, }), authorizationOptions, async execute({ principal, input, context }) { @@ -355,8 +355,7 @@ export const updateAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase const tool = await updateCustomTool({ workspaceId: context.workspaceId, toolId: context.tool.id, - // actorless-unsupported: editing a tool is scoped to its owner; an actorless run owns none - userId: requirePrincipalSubjectUserId(principal), + userId: requireCustomToolUserId(principal, input.executionActorUserId), title, schema: input.schema ?? context.tool.schema, code: input.code ?? context.tool.code, @@ -381,6 +380,7 @@ export interface DeleteWorkspaceCustomToolInput { workspaceId: string toolId: string source?: CustomToolWriteSource + executionActorUserId?: string } export const deleteWorkspaceCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ @@ -419,14 +419,14 @@ export const deleteAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase principal, workspaceId: input.workspaceId, toolId: input.toolId, + executionActorUserId: input.executionActorUserId, }), authorizationOptions, - async execute({ principal, context }) { + async execute({ principal, input, context }) { const deleted = await deleteCustomTool({ workspaceId: context.workspaceId, toolId: context.tool.id, - // actorless-unsupported: deleting a tool is scoped to its owner; an actorless run owns none - userId: requirePrincipalSubjectUserId(principal), + userId: requireCustomToolUserId(principal, input.executionActorUserId), }) if (!deleted) throw new OrchestrationError('not_found', 'Custom tool not found') return { tool: context.tool } diff --git a/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.test.ts b/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.test.ts index 8b459221de2..a68e3bfe3fb 100644 --- a/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.test.ts +++ b/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.test.ts @@ -98,10 +98,28 @@ describe('readAvailableCustomToolByIdOrTitleAsExecutor', () => { workspaceId: principal.workspaceId, identifier: tool.id, lookup: 'id', + executionActorUserId: 'user-1', }, }) }) + it('forwards the legacy execution actor for an actorless principal', async () => { + const context = executionContext() + mocks.createPrincipal.mockResolvedValueOnce({ ...principal, subjectUserId: undefined }) + + await readAvailableCustomToolByIdOrTitleAsExecutor({ + context, + identifier: tool.id, + lookup: 'id', + }) + + expect(mocks.readUseCase.execute).toHaveBeenCalledWith( + expect.objectContaining({ + input: expect.objectContaining({ executionActorUserId: 'user-1' }), + }) + ) + }) + it('stops before principal construction when execution is already cancelled', async () => { const controller = new AbortController() controller.abort(new Error('cancelled')) diff --git a/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.ts b/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.ts index 1fce57b1e89..f2b877fef03 100644 --- a/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.ts +++ b/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.ts @@ -34,6 +34,7 @@ export async function readAvailableCustomToolByIdOrTitleAsExecutor({ workspaceId: principal.workspaceId, identifier, lookup, + executionActorUserId: context.userId, }, }) context.abortSignal?.throwIfAborted() diff --git a/apps/sim/lib/internal/file/execute-tool.test.ts b/apps/sim/lib/internal/file/execute-tool.test.ts index 9079b2b8aaa..a018d7b1e70 100644 --- a/apps/sim/lib/internal/file/execute-tool.test.ts +++ b/apps/sim/lib/internal/file/execute-tool.test.ts @@ -103,6 +103,7 @@ describe('executeFileTool', () => { expect.objectContaining({ workspaceId: 'workspace-1', attributedUserId: 'user-1', + executionActorUserId: 'user-1', fileAccessUserId: 'user-1', requestId: 'request-1', }) @@ -223,6 +224,7 @@ describe('executeFileTool', () => { expect.objectContaining({ principal, attributedUserId: 'workspace-owner', + executionActorUserId: 'legacy-actor', fileAccessUserId: undefined, workspaceId: 'workspace-1', }) diff --git a/apps/sim/lib/internal/file/execute-tool.ts b/apps/sim/lib/internal/file/execute-tool.ts index 1d70d1f5a0f..9be0d1b0277 100644 --- a/apps/sim/lib/internal/file/execute-tool.ts +++ b/apps/sim/lib/internal/file/execute-tool.ts @@ -89,6 +89,7 @@ export const executeFileTool: InternalToolOperationHandler = async (request) => principal, workspaceId, attributedUserId, + executionActorUserId: request.context.userId, fileAccessUserId, workflowId: request.context.workflowId, executionId: request.context.executionId, diff --git a/apps/sim/lib/internal/file/operations.ts b/apps/sim/lib/internal/file/operations.ts index 190592076f8..c05f065942b 100644 --- a/apps/sim/lib/internal/file/operations.ts +++ b/apps/sim/lib/internal/file/operations.ts @@ -86,6 +86,7 @@ export interface FileManageOperationContext { principal: Principal workspaceId: string attributedUserId: string + executionActorUserId?: string fileAccessUserId?: string workflowId: string executionId?: string @@ -878,6 +879,7 @@ export async function executeFileManageOperation( authType, password, allowedEmails, + executionActorUserId: context.executionActorUserId, }, }) ).share diff --git a/apps/sim/lib/internal/knowledge/execute-tool.test.ts b/apps/sim/lib/internal/knowledge/execute-tool.test.ts index c8a07941ed5..d6b78bc13a9 100644 --- a/apps/sim/lib/internal/knowledge/execute-tool.test.ts +++ b/apps/sim/lib/internal/knowledge/execute-tool.test.ts @@ -114,6 +114,7 @@ describe('executeKnowledgeTool', () => { expect.objectContaining({ principal, headers: request.headers, + executionActorUserId: 'trusted-user', signal: controller.signal, }) ) diff --git a/apps/sim/lib/internal/knowledge/execute-tool.ts b/apps/sim/lib/internal/knowledge/execute-tool.ts index 16f5535f5fc..a4874d76588 100644 --- a/apps/sim/lib/internal/knowledge/execute-tool.ts +++ b/apps/sim/lib/internal/knowledge/execute-tool.ts @@ -139,7 +139,12 @@ export const executeKnowledgeTool: InternalToolOperationHandler = async (request throw error } signal?.throwIfAborted() - const context = { principal, headers: request.headers, signal } + const context = { + principal, + headers: request.headers, + executionActorUserId: request.context.userId, + signal, + } const input = normalizeKnowledgeInput(request.input) switch (toolId) { diff --git a/apps/sim/lib/internal/knowledge/operations.ts b/apps/sim/lib/internal/knowledge/operations.ts index 10d179bdece..ec839e14384 100644 --- a/apps/sim/lib/internal/knowledge/operations.ts +++ b/apps/sim/lib/internal/knowledge/operations.ts @@ -55,6 +55,7 @@ import { createKnowledgeDocumentSourceValue } from '@/lib/knowledge/secret-prove export interface KnowledgeOperationContext { principal: WorkflowExecutionDelegatedPrincipal headers: Headers + executionActorUserId?: string signal?: AbortSignal } @@ -79,6 +80,13 @@ function billingAttribution(context: KnowledgeOperationContext, workspaceId: str return requireWorkspaceBillingAttributionHeader(context.headers, { workspaceId }) } +function applicationRequest(context: KnowledgeOperationContext) { + return { + headers: context.headers, + ...(context.executionActorUserId ? { executionActorUserId: context.executionActorUserId } : {}), + } +} + function resolveChunkContentProvenance( context: KnowledgeOperationContext, payload: unknown, @@ -124,7 +132,7 @@ export async function listDocumentsOperation( sortOrder: query.sortOrder, tagFilters, }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) const body = { @@ -193,7 +201,7 @@ export async function createDocumentsOperation( const result = await createKnowledgeDocuments.execute({ principal: context.principal, input, - request: { headers: context.headers }, + request: applicationRequest(context), }) internalKnowledgeAnalytics.documentsUploaded({ principal: context.principal, input, result }) throwIfAborted(context) @@ -229,7 +237,7 @@ export async function readDocumentOperation( documentId, assertedWorkspaceId: context.principal.workspaceId, }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) const body = { success: true, data: toInternalKnowledgeDocument(result.document) } @@ -269,7 +277,7 @@ export async function deleteDocumentOperation( const result = await deleteKnowledgeDocument.execute({ principal: context.principal, input, - request: { headers: context.headers }, + request: applicationRequest(context), }) internalKnowledgeAnalytics.documentDeleted({ principal: context.principal, result }) throwIfAborted(context) @@ -323,7 +331,7 @@ export async function upsertDocumentOperation( const result = await upsertKnowledgeDocument.execute({ principal: context.principal, input, - request: { headers: context.headers }, + request: applicationRequest(context), }) internalKnowledgeAnalytics.documentUpserted({ principal: context.principal, input, result }) throwIfAborted(context) @@ -373,7 +381,7 @@ export async function listChunksOperation( assertedWorkspaceId: context.principal.workspaceId, ...query, }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) const body = { @@ -419,7 +427,7 @@ export async function createChunkOperation( resolveContentProvenance: ({ workspaceId }) => resolveChunkContentProvenance(context, bodyInput, workspaceId, true), }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) const body = { success: true, data: toInternalKnowledgeChunk(result.chunk) } @@ -459,7 +467,7 @@ export async function updateChunkOperation( bodyInput.content !== undefined ), }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) const body = { success: true, data: toInternalKnowledgeChunk(result.chunk) } @@ -500,7 +508,7 @@ export async function deleteChunkOperation( chunkId, assertedWorkspaceId: context.principal.workspaceId, }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) return { body: { success: true, data: { message: 'Chunk deleted successfully' } } } @@ -514,7 +522,7 @@ export async function listConnectorsOperation( const result = await listKnowledgeConnectors.execute({ principal: context.principal, input: { knowledgeBaseId, assertedWorkspaceId: context.principal.workspaceId }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) return { @@ -535,7 +543,7 @@ export async function readConnectorOperation( connectorId, assertedWorkspaceId: context.principal.workspaceId, }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) return { body: { success: true, data: toInternalKnowledgeConnectorDetail(result.connector) } } @@ -560,7 +568,7 @@ export async function syncConnectorOperation( const result = await syncKnowledgeConnector.execute({ principal: context.principal, input, - request: { headers: context.headers }, + request: applicationRequest(context), }) internalKnowledgeAnalytics.connectorSynced({ principal: context.principal, input, result }) throwIfAborted(context) @@ -575,7 +583,7 @@ export async function listTagsOperation( const result = await listKnowledgeTags.execute({ principal: context.principal, input: { knowledgeBaseId, assertedWorkspaceId: context.principal.workspaceId }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) return { @@ -619,7 +627,7 @@ export async function searchOperation( return prepared.registry }, }, - request: { headers: context.headers }, + request: applicationRequest(context), }) throwIfAborted(context) const body = { diff --git a/apps/sim/lib/internal/windchill/execute-tool.test.ts b/apps/sim/lib/internal/windchill/execute-tool.test.ts index 2f1e3780de6..ccb1920a144 100644 --- a/apps/sim/lib/internal/windchill/execute-tool.test.ts +++ b/apps/sim/lib/internal/windchill/execute-tool.test.ts @@ -131,6 +131,7 @@ describe('executeWindchillTool', () => { }) expect(mocks.executeWindchillOperation).toHaveBeenCalledWith(operationInput, { principal: PRINCIPAL, + executionActorUserId: 'user-1', requestId: 'request-1', signal: controller.signal, }) diff --git a/apps/sim/lib/internal/windchill/execute-tool.ts b/apps/sim/lib/internal/windchill/execute-tool.ts index 35a0141ed5f..41f096eac32 100644 --- a/apps/sim/lib/internal/windchill/execute-tool.ts +++ b/apps/sim/lib/internal/windchill/execute-tool.ts @@ -89,7 +89,12 @@ export const executeWindchillTool: InternalToolOperationHandler = async (request return failureResponse('Windchill request operation does not match the selected tool', 400) } - const output = await executeWindchillOperation(input, { principal, requestId, signal }) + const output = await executeWindchillOperation(input, { + principal, + executionActorUserId: context.userId, + requestId, + signal, + }) signal?.throwIfAborted() return Response.json({ success: true, output } satisfies WindchillOperationResponse) } catch (error) { diff --git a/apps/sim/lib/internal/windchill/operations.test.ts b/apps/sim/lib/internal/windchill/operations.test.ts index 95b2b1bff95..879fb9f0e44 100644 --- a/apps/sim/lib/internal/windchill/operations.test.ts +++ b/apps/sim/lib/internal/windchill/operations.test.ts @@ -331,6 +331,41 @@ describe('Windchill operations', () => { }) }) + it('uses the legacy execution actor for actorless file access', async () => { + const rawFile = { + key: 'workspace/specification.pdf', + name: 'specification.pdf', + size: 3, + type: 'application/pdf', + } + mocks.processFilesToUserFiles.mockReturnValue([rawFile]) + mocks.downloadServableFileFromStorage.mockResolvedValue({ + buffer: Buffer.from('pdf'), + contentType: 'application/pdf', + }) + + await executeWindchillOperation( + { + ...BASE, + operation: 'windchill_upload_primary_content', + documentOid: DOCUMENT_OID, + primaryFile: rawFile, + }, + { + principal: { ...PRINCIPAL, subjectUserId: undefined }, + executionActorUserId: 'execution-actor', + requestId: 'request-1', + } + ) + + expect(mocks.assertToolFileAccess).toHaveBeenCalledWith( + rawFile.key, + 'execution-actor', + 'request-1', + expect.anything() + ) + }) + it('fails closed before storage or provider work when file access is denied', async () => { const rawFile = { key: 'other/file.pdf', name: 'file.pdf', size: 3, type: 'application/pdf' } mocks.processFilesToUserFiles.mockReturnValue([rawFile]) @@ -413,4 +448,27 @@ describe('Windchill operations', () => { }) expect(result).not.toHaveProperty('content') }) + + it('attributes actorless provider downloads to the legacy execution actor', async () => { + await executeWindchillOperation( + { + ...BASE, + operation: 'windchill_download_primary_content', + documentOid: DOCUMENT_OID, + }, + { + principal: { ...PRINCIPAL, subjectUserId: undefined }, + executionActorUserId: 'execution-actor', + requestId: 'request-1', + } + ) + + expect(mocks.uploadExecutionFile).toHaveBeenCalledWith( + expect.anything(), + Buffer.from('pdf'), + 'specification.pdf', + 'application/pdf', + 'execution-actor' + ) + }) }) diff --git a/apps/sim/lib/internal/windchill/operations.ts b/apps/sim/lib/internal/windchill/operations.ts index f8bc8c30e2f..ffd8d56b385 100644 --- a/apps/sim/lib/internal/windchill/operations.ts +++ b/apps/sim/lib/internal/windchill/operations.ts @@ -1,6 +1,6 @@ import { type BoundWorkflowExecutionDelegatedPrincipal, - requirePrincipalSubjectUserId, + resolvePrincipalSubjectUserId, } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' @@ -440,6 +440,17 @@ async function loadUploadFiles( return files } +function requireWindchillExecutionUserId( + principal: BoundWorkflowExecutionDelegatedPrincipal, + executionActorUserId?: string +): string { + const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId + if (!userId) { + throw new WindchillOperationError('Windchill file operations require an execution actor', 403) + } + return userId +} + function contentDispositionFileName(value: string | null): string | null { if (!value) return null const encoded = value.match(/filename\*=UTF-8''([^;]+)/i)?.[1] @@ -462,16 +473,19 @@ async function storeDownloadedFile({ buffer, fileName, contentType, + executionActorUserId, signal, }: { principal: BoundWorkflowExecutionDelegatedPrincipal buffer: Buffer fileName: string contentType: string + executionActorUserId?: string signal?: AbortSignal }): Promise { signal?.throwIfAborted() const { workflowId, executionId } = principal.delegationContext + const userId = requireWindchillExecutionUserId(principal, executionActorUserId) if (executionId) { const file = await uploadExecutionFile( { @@ -482,8 +496,7 @@ async function storeDownloadedFile({ buffer, fileName, contentType, - // actorless-unsupported: the uploaded file needs an owning user row; attributing it to the workflow's user is a follow-up - requirePrincipalSubjectUserId(principal) + userId ) signal?.throwIfAborted() return file @@ -492,8 +505,7 @@ async function storeDownloadedFile({ buffer, fileName, contentType, - // actorless-unsupported: the uploaded file needs an owning user row; attributing it to the workflow's user is a follow-up - userId: requirePrincipalSubjectUserId(principal), + userId, }) signal?.throwIfAborted() return file @@ -506,6 +518,7 @@ async function executeDownload( | { operation: 'windchill_download_attachment' } >, principal: BoundWorkflowExecutionDelegatedPrincipal, + executionActorUserId?: string, signal?: AbortSignal ): Promise { const documentUrl = windchillDocumentUrl(body.baseUrl, body.documentOid) @@ -537,6 +550,7 @@ async function executeDownload( buffer: downloaded.buffer, fileName, contentType: mimeType, + executionActorUserId, signal, }) return { @@ -549,6 +563,7 @@ async function executeDownload( export interface WindchillOperationContext { principal: BoundWorkflowExecutionDelegatedPrincipal + executionActorUserId?: string requestId: string signal?: AbortSignal } @@ -557,14 +572,14 @@ export async function executeWindchillOperation( body: WindchillOperationBody, context: WindchillOperationContext ): Promise { - const { principal, requestId, signal } = context + const { principal, executionActorUserId, requestId, signal } = context signal?.throwIfAborted() if ( body.operation === 'windchill_download_primary_content' || body.operation === 'windchill_download_attachment' ) { - return executeDownload(body, principal, signal) + return executeDownload(body, principal, executionActorUserId, signal) } if ( @@ -577,8 +592,7 @@ export async function executeWindchillOperation( : body.attachmentFiles const files = await loadUploadFiles( inputs, - // actorless-unsupported: reads the acting user's own files; attributing it to the workflow's user is a follow-up - requirePrincipalSubjectUserId(principal), + requireWindchillExecutionUserId(principal, executionActorUserId), requestId, signal ) diff --git a/apps/sim/lib/knowledge/api/internal-route.test.ts b/apps/sim/lib/knowledge/api/internal-route.test.ts index 3b04901a13c..775c1c15e8d 100644 --- a/apps/sim/lib/knowledge/api/internal-route.test.ts +++ b/apps/sim/lib/knowledge/api/internal-route.test.ts @@ -98,7 +98,7 @@ describe('internal Knowledge execution attribution', () => { resolveInternalKnowledgeBillingAttribution(request(), executor, 'workspace-1') ).resolves.toEqual(BILLING_ATTRIBUTION) expect(internalKnowledgeProvenanceUserId(request().headers, executor, 'workspace-1')).toBe( - 'billing-owner-1' + 'execution-billing-actor-1' ) expect( resolveKnowledgeAttributedUserId(executor, { @@ -106,8 +106,9 @@ describe('internal Knowledge execution attribution', () => { workspaceOrganizationId: null, allowPersonalApiKeys: true, billedAccountUserId: 'billing-owner-1', + executionActorUserId: 'execution-billing-actor-1', }) - ).toBe('billing-owner-1') + ).toBe('execution-billing-actor-1') }) it('rejects a billing snapshot from another workspace', async () => { diff --git a/apps/sim/lib/knowledge/api/internal-route.ts b/apps/sim/lib/knowledge/api/internal-route.ts index 5608a476923..21fe9b0510f 100644 --- a/apps/sim/lib/knowledge/api/internal-route.ts +++ b/apps/sim/lib/knowledge/api/internal-route.ts @@ -1,7 +1,7 @@ import { type Principal, - requirePrincipalSubjectUserId, resolvePrincipalSubject, + resolvePrincipalSubjectUserId, type SessionPrincipal, } from '@sim/auth/principal' import type { NextRequest } from 'next/server' @@ -20,6 +20,7 @@ import { requireWorkspaceBillingAttributionHeader, resolveBillingAttribution, } from '@/lib/billing/core/billing-attribution' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { PlatformEvents } from '@/lib/core/telemetry' import type { CreateKnowledgeBaseInput, @@ -32,8 +33,11 @@ import { captureServerEvent } from '@/lib/posthog/server' import type { UploadSessionRecord } from '@/lib/uploads/upload-session/service' export function internalKnowledgeActorUserId(principal: Principal): string { - // actorless-unsupported: knowledge writes are attributed to a person; the actorless path is a follow-up - return requirePrincipalSubjectUserId(principal) + const userId = resolvePrincipalSubjectUserId(principal) + if (!userId) { + throw new OrchestrationError('forbidden', 'Knowledge operation requires a user subject') + } + return userId } export function internalKnowledgeProvenanceUserId( @@ -44,10 +48,9 @@ export function internalKnowledgeProvenanceUserId( if (principal.kind !== 'delegated') return internalKnowledgeActorUserId(principal) const subject = resolvePrincipalSubject(principal) if (subject?.kind === 'sim_user') return subject.userId - if (!workspaceId) { - throw new Error('Delegated Knowledge provenance requires a workspace scope') - } - return requireWorkspaceBillingAttributionHeader(headers, { workspaceId }).billedAccountUserId + return requireWorkspaceBillingAttributionHeader(headers, { + workspaceId: workspaceId ?? principal.workspaceId, + }).actorUserId } export function internalKnowledgeAuthType(principal: Principal): AuthTypeValue { diff --git a/apps/sim/lib/knowledge/application/authorization.ts b/apps/sim/lib/knowledge/application/authorization.ts index 15f9dc9b762..32c0e6bfb13 100644 --- a/apps/sim/lib/knowledge/application/authorization.ts +++ b/apps/sim/lib/knowledge/application/authorization.ts @@ -16,11 +16,14 @@ interface KnowledgeResourceIdentifiers { export interface KnowledgeAuthorizationContext extends WorkspaceAuthorizationContext, - KnowledgeResourceIdentifiers {} + KnowledgeResourceIdentifiers { + executionActorUserId?: string +} export interface LegacyPersonalKnowledgeAuthorizationContext extends KnowledgeResourceIdentifiers { workspaceId: undefined legacyPersonalOwnerUserId: string + executionActorUserId?: string } export type KnowledgeResourceAuthorizationContext = diff --git a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts index 6deb81fe26c..dc8d1112a56 100644 --- a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts +++ b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts @@ -1,4 +1,3 @@ -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { defineAuthorizedWorkspaceUseCase, type OperationUseCase, @@ -18,6 +17,7 @@ import { knowledgeDelegationPolicy, type LegacyPersonalKnowledgeAuthorizationContext, } from '@/lib/knowledge/application/authorization' +import { resolveKnowledgeAttributedUserId } from '@/lib/knowledge/application/billing' interface AuthorizedKnowledgeUseCaseContext< O extends WorkspaceOperation, @@ -68,6 +68,13 @@ function assertWorkspaceKnowledgeContext( + context: C, + executionActorUserId?: string +): C { + return executionActorUserId ? { ...context, executionActorUserId } : context +} + export function defineAuthorizedKnowledgeUseCase< const O extends WorkspaceOperation, I, @@ -122,12 +129,12 @@ export function defineAuthorizedKnowledgeUseCase< operation: definition.operation, async execute({ principal, input, request }) { requireAllowedWorkspacePrincipal(principal, definition.operation) - const context = await definition.resolveContext({ principal, input }) + const resolvedContext = await definition.resolveContext({ principal, input }) + const context = withExecutionActor(resolvedContext, request?.executionActorUserId) if (isLegacyPersonalKnowledgeContext(context)) { if ( principal.kind === 'workspace_api_key' || - // actorless-unsupported: a legacy personal knowledge base has exactly one owner, so an actorless caller is never it - requirePrincipalSubjectUserId(principal) !== context.legacyPersonalOwnerUserId + resolveKnowledgeAttributedUserId(principal, context) !== context.legacyPersonalOwnerUserId ) { throw new OrchestrationError('not_found', 'Knowledge base not found') } diff --git a/apps/sim/lib/knowledge/application/billing.ts b/apps/sim/lib/knowledge/application/billing.ts index ccf4af4a8f0..efb7001da93 100644 --- a/apps/sim/lib/knowledge/application/billing.ts +++ b/apps/sim/lib/knowledge/application/billing.ts @@ -1,5 +1,8 @@ -import type { Principal } from '@sim/auth/principal' -import { requirePrincipalSubjectUserId, resolvePrincipalAttribution } from '@sim/auth/principal' +import { + type Principal, + resolvePrincipalAttribution, + resolvePrincipalSubjectUserId, +} from '@sim/auth/principal' import { checkActorUsageLimits } from '@/lib/billing/calculations/usage-monitor' import { type BillingAttributionSnapshot, @@ -7,6 +10,7 @@ import { resolveBillingAttribution, resolveSystemBillingAttribution, } from '@/lib/billing/core/billing-attribution' +import { OrchestrationError } from '@/lib/core/orchestration/types' import type { KnowledgeResourceContext } from '@/lib/knowledge/application/contexts' export class KnowledgeUsageLimitExceededError extends Error { @@ -20,8 +24,14 @@ export function resolveKnowledgeAttributedUserId( principal: Principal, context: KnowledgeResourceContext ): string { - // actorless-unsupported: a workspace-less knowledge base bills its owner directly, with no workspace to attribute to - if (context.workspaceId === undefined) return requirePrincipalSubjectUserId(principal) + const executionUserId = resolvePrincipalSubjectUserId(principal) ?? context.executionActorUserId + if (executionUserId) return executionUserId + if (context.workspaceId === undefined) { + throw new OrchestrationError( + 'forbidden', + 'Knowledge operations require a user subject or execution actor' + ) + } return resolvePrincipalAttribution(principal, { workspaceBillingOwnerUserId: context.billedAccountUserId, }).attributedUserId diff --git a/apps/sim/lib/workflows/application/authorization.test.ts b/apps/sim/lib/workflows/application/authorization.test.ts index 2ee430dca05..b038ff39405 100644 --- a/apps/sim/lib/workflows/application/authorization.test.ts +++ b/apps/sim/lib/workflows/application/authorization.test.ts @@ -5,6 +5,7 @@ import type { DelegatedPrincipal } from '@sim/auth/principal' import { describe, expect, it } from 'vitest' import { + requireWorkflowExecutionUserId, WORKFLOW_DELEGATION_AUDIENCE, workflowDelegationPolicy, } from '@/lib/workflows/application/authorization' @@ -74,3 +75,15 @@ describe('workflow delegation policy', () => { expect(workflowOperations.delete.delegatedServices).not.toContain('executor') }) }) + +describe('workflow execution actor', () => { + it('uses the legacy execution actor when the principal is actorless', () => { + const principal = createExecutorPrincipal({ subjectUserId: undefined }) + + expect(requireWorkflowExecutionUserId(principal, 'execution-actor')).toBe('execution-actor') + }) + + it('prefers a real principal subject over the compatibility actor', () => { + expect(requireWorkflowExecutionUserId(createExecutorPrincipal(), 'someone-else')).toBe('user-1') + }) +}) diff --git a/apps/sim/lib/workflows/application/authorization.ts b/apps/sim/lib/workflows/application/authorization.ts index 87e574fb6c9..82b21bd90a5 100644 --- a/apps/sim/lib/workflows/application/authorization.ts +++ b/apps/sim/lib/workflows/application/authorization.ts @@ -1,8 +1,9 @@ -import type { Principal } from '@sim/auth/principal' +import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal' import type { WorkspaceAuthorizationContext, WorkspaceDelegationPolicy, } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' export const WORKFLOW_DELEGATION_AUDIENCE = 'sim:workflows' @@ -33,3 +34,18 @@ export const workflowDelegationPolicy: WorkspaceDelegationPolicy, async execute({ principal, input, context }) { - // actorless-unsupported: reverting a version is an authored edit and records who made it; no tool exposes it to a run - const userId = requirePrincipalSubjectUserId(principal) + const userId = requireWorkflowExecutionUserId(principal, input.executionActorUserId) await requireMutableWorkflow(context.workflowId) const result = await performRevertToVersion({ workflowId: context.workflowId, diff --git a/apps/sim/lib/workspace-files/application/share-workspace-file.ts b/apps/sim/lib/workspace-files/application/share-workspace-file.ts index 247af5d1198..4192f848084 100644 --- a/apps/sim/lib/workspace-files/application/share-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/share-workspace-file.ts @@ -1,5 +1,5 @@ import { AuditAction, AuditResourceType } from '@sim/audit' -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' +import { resolvePrincipalSubjectUserId } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import type { ShareAuthType, ShareRecord } from '@/lib/api/contracts/public-shares' import { ForbiddenOperationError } from '@/lib/core/application/forbidden' @@ -38,6 +38,7 @@ export interface UpdateWorkspaceFileShareInput { allowedEmails?: string[] token?: string noOpIfInactive?: boolean + executionActorUserId?: string } export interface UpdateWorkspaceFileShareResult { @@ -72,8 +73,13 @@ export const updateWorkspaceFileShare = defineAuthorizedWorkspaceFileUseCase({ return { ...canonical, file } }, async execute({ principal, input, context }): Promise { - // actorless-unsupported: a share link records the person who published it - const subjectUserId = requirePrincipalSubjectUserId(principal) + const userId = resolvePrincipalSubjectUserId(principal) ?? input.executionActorUserId + if (!userId) { + throw new OrchestrationError( + 'forbidden', + 'File sharing requires a user subject or execution actor' + ) + } const existingShare = await getShareForResource('file', context.fileId) if (input.noOpIfInactive && !input.isActive && !existingShare?.isActive) { @@ -83,7 +89,7 @@ export const updateWorkspaceFileShare = defineAuthorizedWorkspaceFileUseCase({ if (input.isActive) { const effectiveAuthType = input.authType ?? existingShare?.authType ?? 'public' try { - await validatePublicFileSharing(subjectUserId, context.workspaceId, effectiveAuthType) + await validatePublicFileSharing(userId, context.workspaceId, effectiveAuthType) } catch (error) { if (error instanceof PublicFileSharingNotAllowedError) throw new ForbiddenOperationError('PUBLIC_SHARING_NOT_ALLOWED', error.message) @@ -96,7 +102,7 @@ export const updateWorkspaceFileShare = defineAuthorizedWorkspaceFileUseCase({ share = await upsertFileShare({ workspaceId: context.workspaceId, fileId: context.fileId, - userId: subjectUserId, + userId, isActive: input.isActive, authType: input.authType, password: input.password, From 53c24fc668671085c2b184a99cc2b813aa3281e4 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Fri, 28 Aug 2026 14:12:45 -0700 Subject: [PATCH 2/2] fix(auth): bind legacy execution actors to principals --- apps/sim/lib/auth/internal-delegation.test.ts | 45 +++++++++++++++++++ apps/sim/lib/auth/internal-delegation.ts | 15 +++++++ apps/sim/lib/auth/principal.test.ts | 44 ++++++++++++++++++ .../tools/handlers/deployment/manage.ts | 1 - .../lib/copilot/tools/handlers/oauth.test.ts | 1 - apps/sim/lib/copilot/tools/handlers/oauth.ts | 1 - apps/sim/lib/core/orchestration/types.ts | 7 +-- .../credentials/application/authorization.ts | 15 +++---- .../authorized-credential-use-case.ts | 8 ++-- .../application/connection-target.test.ts | 14 +++++- .../application/connection-target.ts | 6 +-- .../application/credential-crud.ts | 3 -- .../prepare-credential-connection.ts | 2 - .../application/service-account.ts | 2 - .../custom-tools/application/authorization.ts | 17 +++---- .../application/use-cases.test.ts | 5 ++- .../lib/custom-tools/application/use-cases.ts | 17 +++---- .../read-available-by-id-or-title.test.ts | 30 +++++++++---- .../read-available-by-id-or-title.ts | 1 - .../lib/internal/file/execute-tool.test.ts | 6 ++- apps/sim/lib/internal/file/execute-tool.ts | 1 - apps/sim/lib/internal/file/operations.ts | 2 - .../internal/knowledge/execute-tool.test.ts | 1 - .../lib/internal/knowledge/execute-tool.ts | 1 - apps/sim/lib/internal/knowledge/operations.ts | 36 ++++++--------- apps/sim/lib/internal/mcp/discover-tools.ts | 3 +- apps/sim/lib/internal/mcp/execute-tool.ts | 3 -- .../lib/internal/principals/executor.test.ts | 15 ++++++- apps/sim/lib/internal/principals/executor.ts | 6 ++- .../internal/windchill/execute-tool.test.ts | 1 - .../lib/internal/windchill/execute-tool.ts | 1 - .../lib/internal/windchill/operations.test.ts | 36 +++++++++++++-- apps/sim/lib/internal/windchill/operations.ts | 20 +++------ .../lib/knowledge/api/internal-route.test.ts | 5 ++- .../knowledge/application/authorization.ts | 5 +-- .../authorized-knowledge-use-case.ts | 10 +---- apps/sim/lib/knowledge/application/billing.ts | 4 +- apps/sim/lib/mcp/application/authorization.ts | 33 +++++++------- .../lib/mcp/application/execute-tool.test.ts | 34 +++++++++----- apps/sim/lib/mcp/application/execute-tool.ts | 7 +-- apps/sim/lib/mcp/application/use-cases.ts | 14 +----- .../application/authorization.test.ts | 37 +++++++++++++-- .../workflows/application/authorization.ts | 9 ++-- .../lib/workflows/application/deployments.ts | 3 +- .../application/share-workspace-file.ts | 5 +-- packages/auth/src/principal.ts | 26 +++++++++++ 46 files changed, 358 insertions(+), 200 deletions(-) diff --git a/apps/sim/lib/auth/internal-delegation.test.ts b/apps/sim/lib/auth/internal-delegation.test.ts index f7bd611db2a..2450d21bdb6 100644 --- a/apps/sim/lib/auth/internal-delegation.test.ts +++ b/apps/sim/lib/auth/internal-delegation.test.ts @@ -102,6 +102,41 @@ describe('bindInternalExecutorDelegation', () => { }) }) + it('binds the trusted legacy execution actor only for an actorless principal', async () => { + const principal = await bindInternalExecutorDelegation( + { + ...claims, + subjectUserId: undefined, + principal: { + kind: 'system', + serviceId: 'schedule', + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + }, + }, + { + audience: 'sim:workspace-files', + compatibilityActorUserId: 'execution-actor', + } + ) + + expect(principal.subjectUserId).toBeUndefined() + expect(principal.delegationContext.compatibilityActor).toEqual({ + kind: 'legacy_execution_user', + userId: 'execution-actor', + }) + }) + + it('rejects a compatibility actor when the delegation has a user subject', async () => { + await expect( + bindInternalExecutorDelegation(claims, { + audience: 'sim:workspace-files', + compatibilityActorUserId: 'execution-actor', + }) + ).rejects.toThrow('cannot bind a compatibility actor to a user subject') + expect(mockResolveWorkflow).not.toHaveBeenCalled() + }) + it('binds deployed child authority to its exact historical deployment version', async () => { const currentWorkflow = { workflowId: 'child-workflow', @@ -260,6 +295,16 @@ describe('bindInternalExecutorDelegation', () => { expect(mockResolveWorkflow).not.toHaveBeenCalled() }) + it('fails before canonical loading when the compatibility actor is empty', async () => { + await expect( + bindInternalExecutorDelegation(claims, { + audience: 'sim:workspace-files', + compatibilityActorUserId: ' ', + }) + ).rejects.toThrow('Internal delegation execution actor must not be empty') + expect(mockResolveWorkflow).not.toHaveBeenCalled() + }) + it('classifies a missing canonical execution as an invalid delegation binding', async () => { mockResolveRun.mockRejectedValue(new OrchestrationError('not_found', 'Workflow run not found')) diff --git a/apps/sim/lib/auth/internal-delegation.ts b/apps/sim/lib/auth/internal-delegation.ts index 7f03930ae03..502ab3859b8 100644 --- a/apps/sim/lib/auth/internal-delegation.ts +++ b/apps/sim/lib/auth/internal-delegation.ts @@ -15,6 +15,7 @@ import { export interface BindInternalExecutorDelegationOptions { audience: string resourceScope?: DelegatedPrincipal['resourceScope'] + compatibilityActorUserId?: string } export class InvalidInternalDelegationBindingError extends Error { @@ -30,6 +31,12 @@ export async function bindInternalExecutorDelegation( options: BindInternalExecutorDelegationOptions ): Promise { if (!options.audience.trim()) throw new Error('Internal delegation audience must not be empty') + if (options.compatibilityActorUserId !== undefined && !options.compatibilityActorUserId.trim()) { + throw new Error('Internal delegation execution actor must not be empty') + } + if (claims.subjectUserId && options.compatibilityActorUserId) { + throw new Error('Internal delegation cannot bind a compatibility actor to a user subject') + } let context: ActiveWorkflowApplicationContext let rootDeploymentVersionId: string | null | undefined @@ -107,6 +114,14 @@ export async function bindInternalExecutorDelegation( ...(claims.executionId ? { executionId: claims.executionId } : {}), ...(claims.principal ? { principal: claims.principal } : {}), ...(claims.currentWorkflow ? { currentWorkflow: claims.currentWorkflow } : {}), + ...(options.compatibilityActorUserId + ? { + compatibilityActor: { + kind: 'legacy_execution_user', + userId: options.compatibilityActorUserId, + } as const, + } + : {}), }, } } diff --git a/apps/sim/lib/auth/principal.test.ts b/apps/sim/lib/auth/principal.test.ts index e938f062a58..8e5e4c108c9 100644 --- a/apps/sim/lib/auth/principal.test.ts +++ b/apps/sim/lib/auth/principal.test.ts @@ -7,6 +7,7 @@ import { requirePrincipalSubjectUserId, resolvePrincipalAttribution, resolvePrincipalAuditAttribution, + resolvePrincipalExecutionActorUserId, resolvePrincipalSubject, resolvePrincipalSubjectUserId, serializePrincipal, @@ -108,6 +109,49 @@ describe('principal subject users', () => { ).toBeUndefined() }) + it('resolves only a principal-bound compatibility actor for actorless execution', () => { + const principal = { + kind: 'delegated' as const, + serviceId: 'executor' as const, + workspaceId: 'workspace-1', + delegationId: 'delegation-1', + audience: 'sim:test', + issuedAt: new Date('2026-01-01T00:00:00Z'), + expiresAt: new Date('2026-01-01T00:05:00Z'), + delegationContext: { + kind: 'workflow_execution' as const, + workflowId: 'workflow-1', + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment' as const, + deploymentVersionId: 'deployment-1', + }, + compatibilityActor: { + kind: 'legacy_execution_user' as const, + userId: 'execution-actor', + }, + }, + } + + expect(resolvePrincipalSubjectUserId(principal)).toBeUndefined() + expect(resolvePrincipalExecutionActorUserId(principal)).toBe('execution-actor') + expect( + resolvePrincipalExecutionActorUserId({ + ...principal, + subjectUserId: 'authenticated-user', + }) + ).toBe('authenticated-user') + expect( + resolvePrincipalExecutionActorUserId({ + ...principal, + delegationContext: { + ...principal.delegationContext, + currentWorkflow: { workflowId: 'workflow-1', mode: 'draft' }, + }, + }) + ).toBeUndefined() + }) + it('fails fast instead of fabricating a workspace-key subject', () => { expect(() => requirePrincipalSubjectUserId({ diff --git a/apps/sim/lib/copilot/tools/handlers/deployment/manage.ts b/apps/sim/lib/copilot/tools/handlers/deployment/manage.ts index cba01b2e018..156d83d2a8a 100644 --- a/apps/sim/lib/copilot/tools/handlers/deployment/manage.ts +++ b/apps/sim/lib/copilot/tools/handlers/deployment/manage.ts @@ -395,7 +395,6 @@ export async function executeLoadDeployment( workflowId, assertedWorkspaceId: context.workspaceId, version: target.version, - executionActorUserId: context.userId, }) const label = target.version === 'active' ? 'the live deployment' : `version ${target.version}` diff --git a/apps/sim/lib/copilot/tools/handlers/oauth.test.ts b/apps/sim/lib/copilot/tools/handlers/oauth.test.ts index df151222e76..f3926a3ed11 100644 --- a/apps/sim/lib/copilot/tools/handlers/oauth.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/oauth.test.ts @@ -53,7 +53,6 @@ describe('executeOAuthGetAuthLink', () => { workspaceId: 'workspace-1', providerName: 'gmail', credentialId: undefined, - executionActorUserId: 'user-1', }) const url = new URL((result.output as { oauth_url: string }).oauth_url) expect(url.pathname).toBe('/api/auth/oauth2/authorize') diff --git a/apps/sim/lib/copilot/tools/handlers/oauth.ts b/apps/sim/lib/copilot/tools/handlers/oauth.ts index 50bd8f774d6..eb24cb86ab6 100644 --- a/apps/sim/lib/copilot/tools/handlers/oauth.ts +++ b/apps/sim/lib/copilot/tools/handlers/oauth.ts @@ -39,7 +39,6 @@ export async function executeOAuthGetAuthLink( workspaceId, providerName, credentialId, - executionActorUserId: context.userId, }) const callbackURL = context.workflowId ? `${baseUrl}/workspace/${workspaceId}/w/${context.workflowId}` diff --git a/apps/sim/lib/core/orchestration/types.ts b/apps/sim/lib/core/orchestration/types.ts index 454b3d3f5f2..dabdcfb23bc 100644 --- a/apps/sim/lib/core/orchestration/types.ts +++ b/apps/sim/lib/core/orchestration/types.ts @@ -109,12 +109,7 @@ export function asOrchestrationError(error: unknown): OrchestrationError | null return null } -/** - * Transport metadata available to an application operation. HTTP callers carry - * headers for audit capture; executor adapters may also preserve the legacy - * execution actor used by pre-application-boundary internal routes. - */ +/** Transport metadata available to an application operation for audit capture. */ export interface OrchestrationRequestContext { headers: { get(name: string): string | null } - executionActorUserId?: string } diff --git a/apps/sim/lib/credentials/application/authorization.ts b/apps/sim/lib/credentials/application/authorization.ts index f32c8bd950b..ef384c6b346 100644 --- a/apps/sim/lib/credentials/application/authorization.ts +++ b/apps/sim/lib/credentials/application/authorization.ts @@ -1,4 +1,4 @@ -import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal' +import { type Principal, resolvePrincipalExecutionActorUserId } from '@sim/auth/principal' import type { WorkspaceDelegationPolicy } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import type { ManagedOAuthCredentialApplicationContext } from '@/lib/credentials/managed-oauth' @@ -26,15 +26,12 @@ export const managedOAuthCredentialDelegationPolicy = { /** * Resolves the user whose credential grants an operation evaluates. * - * `executionActorUserId` is the user the legacy internal route authenticated as. - * Workspace authorization remains principal-based, and a principal subject - * always takes precedence over this compatibility value. + * Actorless execution uses only the compatibility actor bound into the executor + * principal by the trusted runtime. Workspace authorization remains + * principal-based, and a principal subject always takes precedence. */ -export function requireCredentialExecutionUserId( - principal: Principal, - executionActorUserId?: string -): string { - const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId +export function requireCredentialExecutionUserId(principal: Principal): string { + const userId = resolvePrincipalExecutionActorUserId(principal) if (!userId) { throw new OrchestrationError( 'forbidden', diff --git a/apps/sim/lib/credentials/application/authorized-credential-use-case.ts b/apps/sim/lib/credentials/application/authorized-credential-use-case.ts index 06df33e76c2..3231a94063f 100644 --- a/apps/sim/lib/credentials/application/authorized-credential-use-case.ts +++ b/apps/sim/lib/credentials/application/authorized-credential-use-case.ts @@ -76,9 +76,7 @@ type AuthorizedCredentialUseCaseDefinition< > = Omit< AuthorizedWorkspaceUseCaseDefinition, 'authorizationOptions' | 'authorizeResource' -> & { - resolveExecutionActorUserId?: (input: I) => string | undefined -} +> export function defineAuthorizedCredentialUseCase< const O extends CredentialOperation, @@ -89,10 +87,10 @@ export function defineAuthorizedCredentialUseCase< return defineAuthorizedWorkspaceUseCase({ ...definition, authorizationOptions: { delegation: credentialDelegationPolicy }, - async authorizeResource({ principal, input, context }) { + async authorizeResource({ principal, context }) { const actor = await getCredentialActorContext( context.credential.id, - requireCredentialExecutionUserId(principal, definition.resolveExecutionActorUserId?.(input)) + requireCredentialExecutionUserId(principal) ) if ( !actor.credential || diff --git a/apps/sim/lib/credentials/application/connection-target.test.ts b/apps/sim/lib/credentials/application/connection-target.test.ts index e3d533c27ec..de2a9cd71c9 100644 --- a/apps/sim/lib/credentials/application/connection-target.test.ts +++ b/apps/sim/lib/credentials/application/connection-target.test.ts @@ -146,13 +146,25 @@ describe('resolveCredentialConnectionTarget', () => { audience: 'sim:credentials', issuedAt: new Date('2026-08-28T00:00:00.000Z'), expiresAt: new Date('2099-08-28T00:00:00.000Z'), + delegationContext: { + kind: 'workflow_execution' as const, + workflowId: 'workflow-1', + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment' as const, + deploymentVersionId: 'deployment-1', + }, + compatibilityActor: { + kind: 'legacy_execution_user' as const, + userId: 'execution-actor', + }, + }, } await resolveCredentialConnectionTarget({ principal: actorlessPrincipal, context, credentialId: 'credential-1', - executionActorUserId: 'execution-actor', }) expect(mocks.getCredentialActorContext).toHaveBeenCalledWith('credential-1', 'execution-actor') diff --git a/apps/sim/lib/credentials/application/connection-target.ts b/apps/sim/lib/credentials/application/connection-target.ts index 95ee9ef579e..6c98ac540ce 100644 --- a/apps/sim/lib/credentials/application/connection-target.ts +++ b/apps/sim/lib/credentials/application/connection-target.ts @@ -32,10 +32,8 @@ export async function resolveCredentialConnectionTarget(params: { providerId?: string credentialId?: string assertedProviderId?: string - executionActorUserId?: string }): Promise { - const { principal, context, providerId, credentialId, assertedProviderId, executionActorUserId } = - params + const { principal, context, providerId, credentialId, assertedProviderId } = params if (Boolean(providerId) === Boolean(credentialId)) { throw new Error('Credential connection requires exactly one target identifier') } @@ -49,7 +47,7 @@ export async function resolveCredentialConnectionTarget(params: { } if (!credentialId) throw new Error('Credential reconnect target is missing its credential ID') - const userId = requireCredentialExecutionUserId(principal, executionActorUserId) + const userId = requireCredentialExecutionUserId(principal) const targetCredentialId = credentialId const credential = await getWorkspaceCredential({ workspaceId: context.workspaceId, diff --git a/apps/sim/lib/credentials/application/credential-crud.ts b/apps/sim/lib/credentials/application/credential-crud.ts index 56e4f4c06ac..c28bb5302a0 100644 --- a/apps/sim/lib/credentials/application/credential-crud.ts +++ b/apps/sim/lib/credentials/application/credential-crud.ts @@ -244,7 +244,6 @@ export type UpdateWorkspaceCredentialInput = Omit< PerformUpdateCredentialParams, 'userId' | 'actorName' | 'actorEmail' | 'allowedTypes' | 'reason' | 'request' > & { - executionActorUserId?: string /** * Workspace the caller asserts owns the credential; a mismatch is concealed as * a not-found. The internal surface omits it and resolves the credential's own @@ -255,8 +254,6 @@ export type UpdateWorkspaceCredentialInput = Omit< export const updateWorkspaceCredentialUseCase = defineAuthorizedCredentialUseCase({ operation: credentialOperations.update, - resolveExecutionActorUserId: (input: UpdateWorkspaceCredentialInput) => - input.executionActorUserId, resolveContext: ({ input }: { input: UpdateWorkspaceCredentialInput }) => resolveCredentialApplicationContext(input), async execute({ principal, input, context }) { diff --git a/apps/sim/lib/credentials/application/prepare-credential-connection.ts b/apps/sim/lib/credentials/application/prepare-credential-connection.ts index 8cf92d98d08..cedd08e89fd 100644 --- a/apps/sim/lib/credentials/application/prepare-credential-connection.ts +++ b/apps/sim/lib/credentials/application/prepare-credential-connection.ts @@ -14,7 +14,6 @@ export interface PrepareCredentialConnectionInput { workspaceId: string providerName: string credentialId?: string - executionActorUserId?: string } export interface PrepareCredentialConnectionResult { @@ -86,7 +85,6 @@ export const prepareCredentialConnection = defineAuthorizedWorkspaceUseCase({ principal, context, credentialId: input.credentialId, - executionActorUserId: input.executionActorUserId, }) if ( !credentialProviderMatchesService(target.providerId, { diff --git a/apps/sim/lib/credentials/application/service-account.ts b/apps/sim/lib/credentials/application/service-account.ts index d5b8ee838ed..6112b39a99d 100644 --- a/apps/sim/lib/credentials/application/service-account.ts +++ b/apps/sim/lib/credentials/application/service-account.ts @@ -133,7 +133,6 @@ export const createServiceAccountCredentialUseCase = defineAuthorizedWorkspaceUs export interface DeleteCredentialInput { workspaceId?: string credentialId: string - executionActorUserId?: string } export interface DeleteCredentialResult { @@ -143,7 +142,6 @@ export interface DeleteCredentialResult { export const deleteCredentialUseCase = defineAuthorizedCredentialUseCase({ operation: credentialOperations.delete, - resolveExecutionActorUserId: (input: DeleteCredentialInput) => input.executionActorUserId, resolveContext: ({ input }: { input: DeleteCredentialInput }) => resolveCredentialApplicationContext({ credentialId: input.credentialId, diff --git a/apps/sim/lib/custom-tools/application/authorization.ts b/apps/sim/lib/custom-tools/application/authorization.ts index 8baac1840c2..f97e6fd0168 100644 --- a/apps/sim/lib/custom-tools/application/authorization.ts +++ b/apps/sim/lib/custom-tools/application/authorization.ts @@ -1,4 +1,4 @@ -import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal' +import { type Principal, resolvePrincipalExecutionActorUserId } from '@sim/auth/principal' import type { WorkspaceDelegationPolicy } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -16,16 +16,13 @@ export const customToolDelegationPolicy = { /** * Resolves the user whose custom-tool library an operation reads or mutates. * - * Actorless execution keeps the pre-application-boundary behavior by falling - * back to `ExecutionContext.userId`, the user the legacy internal route ran as. - * A real principal subject always wins, so the fallback is not an impersonation - * input and is never involved in workspace authorization. + * Actorless execution keeps the pre-application-boundary behavior through the + * compatibility actor bound into the executor principal. A real principal + * subject always wins, and this value is never involved in workspace + * authorization. */ -export function requireCustomToolUserId( - principal: Principal, - executionActorUserId?: string -): string { - const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId +export function requireCustomToolUserId(principal: Principal): string { + const userId = resolvePrincipalExecutionActorUserId(principal) if (!userId) { throw new OrchestrationError( 'forbidden', diff --git a/apps/sim/lib/custom-tools/application/use-cases.test.ts b/apps/sim/lib/custom-tools/application/use-cases.test.ts index 625cfdd6e79..8b4bb2fac7e 100644 --- a/apps/sim/lib/custom-tools/application/use-cases.test.ts +++ b/apps/sim/lib/custom-tools/application/use-cases.test.ts @@ -150,13 +150,16 @@ describe('custom tool application use cases', () => { mode: 'deployment', deploymentVersionId: 'version-1', }, + compatibilityActor: { + kind: 'legacy_execution_user', + userId: 'execution-actor', + }, }, }), input: { workspaceId: workspace.workspaceId, identifier: tool.id, lookup: 'id_or_title', - executionActorUserId: 'execution-actor', }, }) diff --git a/apps/sim/lib/custom-tools/application/use-cases.ts b/apps/sim/lib/custom-tools/application/use-cases.ts index 581684614f3..4d9643837ba 100644 --- a/apps/sim/lib/custom-tools/application/use-cases.ts +++ b/apps/sim/lib/custom-tools/application/use-cases.ts @@ -64,12 +64,11 @@ async function resolveAvailableToolContext(args: { principal: Exclude workspaceId: string toolId: string - executionActorUserId?: string }): Promise { const workspace = await resolveWorkspaceContext(args.workspaceId) const tool = await getCustomToolById({ toolId: args.toolId, - userId: requireCustomToolUserId(args.principal, args.executionActorUserId), + userId: requireCustomToolUserId(args.principal), workspaceId: workspace.workspaceId, }) if (!tool) throw new OrchestrationError('not_found', 'Custom tool not found') @@ -119,7 +118,6 @@ export const listWorkspaceCustomToolsUseCase = defineAuthorizedWorkspaceUseCase( export interface ListAvailableCustomToolsInput { workspaceId: string - executionActorUserId?: string } export const listAvailableCustomToolsUseCase = defineAuthorizedWorkspaceUseCase({ @@ -129,7 +127,7 @@ export const listAvailableCustomToolsUseCase = defineAuthorizedWorkspaceUseCase( authorizationOptions, async execute({ principal, input, context }) { const tools = await listCustomTools({ - userId: requireCustomToolUserId(principal, input.executionActorUserId), + userId: requireCustomToolUserId(principal), workspaceId: context.workspaceId, }) return { tools } @@ -155,7 +153,6 @@ export interface ReadAvailableCustomToolByIdOrTitleInput { workspaceId: string identifier: string lookup: 'id' | 'id_or_title' - executionActorUserId?: string } export const readAvailableCustomToolByIdOrTitleUseCase = defineAuthorizedWorkspaceUseCase({ @@ -166,7 +163,7 @@ export const readAvailableCustomToolByIdOrTitleUseCase = defineAuthorizedWorkspa async execute({ principal, input, context }) { const tool = await getAvailableCustomTool({ identifier: input.identifier, - userId: requireCustomToolUserId(principal, input.executionActorUserId), + userId: requireCustomToolUserId(principal), workspaceId: context.workspaceId, lookup: input.lookup, }) @@ -260,7 +257,6 @@ interface UpdateCustomToolFields { schema?: unknown code?: string source?: CustomToolWriteSource - executionActorUserId?: string } export interface UpdateWorkspaceCustomToolInput extends UpdateCustomToolFields { @@ -338,7 +334,6 @@ export const updateAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase principal, workspaceId: input.workspaceId, toolId: input.toolId, - executionActorUserId: input.executionActorUserId, }), authorizationOptions, async execute({ principal, input, context }) { @@ -355,7 +350,7 @@ export const updateAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase const tool = await updateCustomTool({ workspaceId: context.workspaceId, toolId: context.tool.id, - userId: requireCustomToolUserId(principal, input.executionActorUserId), + userId: requireCustomToolUserId(principal), title, schema: input.schema ?? context.tool.schema, code: input.code ?? context.tool.code, @@ -380,7 +375,6 @@ export interface DeleteWorkspaceCustomToolInput { workspaceId: string toolId: string source?: CustomToolWriteSource - executionActorUserId?: string } export const deleteWorkspaceCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ @@ -419,14 +413,13 @@ export const deleteAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase principal, workspaceId: input.workspaceId, toolId: input.toolId, - executionActorUserId: input.executionActorUserId, }), authorizationOptions, async execute({ principal, input, context }) { const deleted = await deleteCustomTool({ workspaceId: context.workspaceId, toolId: context.tool.id, - userId: requireCustomToolUserId(principal, input.executionActorUserId), + userId: requireCustomToolUserId(principal), }) if (!deleted) throw new OrchestrationError('not_found', 'Custom tool not found') return { tool: context.tool } diff --git a/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.test.ts b/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.test.ts index a68e3bfe3fb..dc2f03e8796 100644 --- a/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.test.ts +++ b/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.test.ts @@ -98,14 +98,25 @@ describe('readAvailableCustomToolByIdOrTitleAsExecutor', () => { workspaceId: principal.workspaceId, identifier: tool.id, lookup: 'id', - executionActorUserId: 'user-1', }, }) }) - it('forwards the legacy execution actor for an actorless principal', async () => { + it('forwards the principal-bound legacy execution actor for an actorless principal', async () => { const context = executionContext() - mocks.createPrincipal.mockResolvedValueOnce({ ...principal, subjectUserId: undefined }) + const actorlessPrincipal = { + ...principal, + subjectUserId: undefined, + delegationContext: { + kind: 'workflow_execution' as const, + workflowId: 'workflow-1', + compatibilityActor: { + kind: 'legacy_execution_user' as const, + userId: 'user-1', + }, + }, + } + mocks.createPrincipal.mockResolvedValueOnce(actorlessPrincipal) await readAvailableCustomToolByIdOrTitleAsExecutor({ context, @@ -113,11 +124,14 @@ describe('readAvailableCustomToolByIdOrTitleAsExecutor', () => { lookup: 'id', }) - expect(mocks.readUseCase.execute).toHaveBeenCalledWith( - expect.objectContaining({ - input: expect.objectContaining({ executionActorUserId: 'user-1' }), - }) - ) + expect(mocks.readUseCase.execute).toHaveBeenCalledWith({ + principal: actorlessPrincipal, + input: { + workspaceId: principal.workspaceId, + identifier: tool.id, + lookup: 'id', + }, + }) }) it('stops before principal construction when execution is already cancelled', async () => { diff --git a/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.ts b/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.ts index f2b877fef03..1fce57b1e89 100644 --- a/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.ts +++ b/apps/sim/lib/internal/custom-tools/read-available-by-id-or-title.ts @@ -34,7 +34,6 @@ export async function readAvailableCustomToolByIdOrTitleAsExecutor({ workspaceId: principal.workspaceId, identifier, lookup, - executionActorUserId: context.userId, }, }) context.abortSignal?.throwIfAborted() diff --git a/apps/sim/lib/internal/file/execute-tool.test.ts b/apps/sim/lib/internal/file/execute-tool.test.ts index a018d7b1e70..cadc082eacf 100644 --- a/apps/sim/lib/internal/file/execute-tool.test.ts +++ b/apps/sim/lib/internal/file/execute-tool.test.ts @@ -103,7 +103,6 @@ describe('executeFileTool', () => { expect.objectContaining({ workspaceId: 'workspace-1', attributedUserId: 'user-1', - executionActorUserId: 'user-1', fileAccessUserId: 'user-1', requestId: 'request-1', }) @@ -197,6 +196,10 @@ describe('executeFileTool', () => { mode: 'deployment' as const, deploymentVersionId: 'deployment-1', }, + compatibilityActor: { + kind: 'legacy_execution_user' as const, + userId: 'legacy-actor', + }, }, } mocks.createPrincipal.mockResolvedValueOnce(principal) @@ -224,7 +227,6 @@ describe('executeFileTool', () => { expect.objectContaining({ principal, attributedUserId: 'workspace-owner', - executionActorUserId: 'legacy-actor', fileAccessUserId: undefined, workspaceId: 'workspace-1', }) diff --git a/apps/sim/lib/internal/file/execute-tool.ts b/apps/sim/lib/internal/file/execute-tool.ts index 9be0d1b0277..1d70d1f5a0f 100644 --- a/apps/sim/lib/internal/file/execute-tool.ts +++ b/apps/sim/lib/internal/file/execute-tool.ts @@ -89,7 +89,6 @@ export const executeFileTool: InternalToolOperationHandler = async (request) => principal, workspaceId, attributedUserId, - executionActorUserId: request.context.userId, fileAccessUserId, workflowId: request.context.workflowId, executionId: request.context.executionId, diff --git a/apps/sim/lib/internal/file/operations.ts b/apps/sim/lib/internal/file/operations.ts index c05f065942b..190592076f8 100644 --- a/apps/sim/lib/internal/file/operations.ts +++ b/apps/sim/lib/internal/file/operations.ts @@ -86,7 +86,6 @@ export interface FileManageOperationContext { principal: Principal workspaceId: string attributedUserId: string - executionActorUserId?: string fileAccessUserId?: string workflowId: string executionId?: string @@ -879,7 +878,6 @@ export async function executeFileManageOperation( authType, password, allowedEmails, - executionActorUserId: context.executionActorUserId, }, }) ).share diff --git a/apps/sim/lib/internal/knowledge/execute-tool.test.ts b/apps/sim/lib/internal/knowledge/execute-tool.test.ts index d6b78bc13a9..c8a07941ed5 100644 --- a/apps/sim/lib/internal/knowledge/execute-tool.test.ts +++ b/apps/sim/lib/internal/knowledge/execute-tool.test.ts @@ -114,7 +114,6 @@ describe('executeKnowledgeTool', () => { expect.objectContaining({ principal, headers: request.headers, - executionActorUserId: 'trusted-user', signal: controller.signal, }) ) diff --git a/apps/sim/lib/internal/knowledge/execute-tool.ts b/apps/sim/lib/internal/knowledge/execute-tool.ts index a4874d76588..03b177ab4c9 100644 --- a/apps/sim/lib/internal/knowledge/execute-tool.ts +++ b/apps/sim/lib/internal/knowledge/execute-tool.ts @@ -142,7 +142,6 @@ export const executeKnowledgeTool: InternalToolOperationHandler = async (request const context = { principal, headers: request.headers, - executionActorUserId: request.context.userId, signal, } const input = normalizeKnowledgeInput(request.input) diff --git a/apps/sim/lib/internal/knowledge/operations.ts b/apps/sim/lib/internal/knowledge/operations.ts index ec839e14384..10d179bdece 100644 --- a/apps/sim/lib/internal/knowledge/operations.ts +++ b/apps/sim/lib/internal/knowledge/operations.ts @@ -55,7 +55,6 @@ import { createKnowledgeDocumentSourceValue } from '@/lib/knowledge/secret-prove export interface KnowledgeOperationContext { principal: WorkflowExecutionDelegatedPrincipal headers: Headers - executionActorUserId?: string signal?: AbortSignal } @@ -80,13 +79,6 @@ function billingAttribution(context: KnowledgeOperationContext, workspaceId: str return requireWorkspaceBillingAttributionHeader(context.headers, { workspaceId }) } -function applicationRequest(context: KnowledgeOperationContext) { - return { - headers: context.headers, - ...(context.executionActorUserId ? { executionActorUserId: context.executionActorUserId } : {}), - } -} - function resolveChunkContentProvenance( context: KnowledgeOperationContext, payload: unknown, @@ -132,7 +124,7 @@ export async function listDocumentsOperation( sortOrder: query.sortOrder, tagFilters, }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) const body = { @@ -201,7 +193,7 @@ export async function createDocumentsOperation( const result = await createKnowledgeDocuments.execute({ principal: context.principal, input, - request: applicationRequest(context), + request: { headers: context.headers }, }) internalKnowledgeAnalytics.documentsUploaded({ principal: context.principal, input, result }) throwIfAborted(context) @@ -237,7 +229,7 @@ export async function readDocumentOperation( documentId, assertedWorkspaceId: context.principal.workspaceId, }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) const body = { success: true, data: toInternalKnowledgeDocument(result.document) } @@ -277,7 +269,7 @@ export async function deleteDocumentOperation( const result = await deleteKnowledgeDocument.execute({ principal: context.principal, input, - request: applicationRequest(context), + request: { headers: context.headers }, }) internalKnowledgeAnalytics.documentDeleted({ principal: context.principal, result }) throwIfAborted(context) @@ -331,7 +323,7 @@ export async function upsertDocumentOperation( const result = await upsertKnowledgeDocument.execute({ principal: context.principal, input, - request: applicationRequest(context), + request: { headers: context.headers }, }) internalKnowledgeAnalytics.documentUpserted({ principal: context.principal, input, result }) throwIfAborted(context) @@ -381,7 +373,7 @@ export async function listChunksOperation( assertedWorkspaceId: context.principal.workspaceId, ...query, }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) const body = { @@ -427,7 +419,7 @@ export async function createChunkOperation( resolveContentProvenance: ({ workspaceId }) => resolveChunkContentProvenance(context, bodyInput, workspaceId, true), }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) const body = { success: true, data: toInternalKnowledgeChunk(result.chunk) } @@ -467,7 +459,7 @@ export async function updateChunkOperation( bodyInput.content !== undefined ), }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) const body = { success: true, data: toInternalKnowledgeChunk(result.chunk) } @@ -508,7 +500,7 @@ export async function deleteChunkOperation( chunkId, assertedWorkspaceId: context.principal.workspaceId, }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) return { body: { success: true, data: { message: 'Chunk deleted successfully' } } } @@ -522,7 +514,7 @@ export async function listConnectorsOperation( const result = await listKnowledgeConnectors.execute({ principal: context.principal, input: { knowledgeBaseId, assertedWorkspaceId: context.principal.workspaceId }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) return { @@ -543,7 +535,7 @@ export async function readConnectorOperation( connectorId, assertedWorkspaceId: context.principal.workspaceId, }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) return { body: { success: true, data: toInternalKnowledgeConnectorDetail(result.connector) } } @@ -568,7 +560,7 @@ export async function syncConnectorOperation( const result = await syncKnowledgeConnector.execute({ principal: context.principal, input, - request: applicationRequest(context), + request: { headers: context.headers }, }) internalKnowledgeAnalytics.connectorSynced({ principal: context.principal, input, result }) throwIfAborted(context) @@ -583,7 +575,7 @@ export async function listTagsOperation( const result = await listKnowledgeTags.execute({ principal: context.principal, input: { knowledgeBaseId, assertedWorkspaceId: context.principal.workspaceId }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) return { @@ -627,7 +619,7 @@ export async function searchOperation( return prepared.registry }, }, - request: applicationRequest(context), + request: { headers: context.headers }, }) throwIfAborted(context) const body = { diff --git a/apps/sim/lib/internal/mcp/discover-tools.ts b/apps/sim/lib/internal/mcp/discover-tools.ts index fcf1f709a46..78050f11e9c 100644 --- a/apps/sim/lib/internal/mcp/discover-tools.ts +++ b/apps/sim/lib/internal/mcp/discover-tools.ts @@ -25,8 +25,7 @@ export async function discoverMcpServerToolsAsExecutor({ signal?.throwIfAborted() const result = await discoverMcpServerToolsUseCase.execute({ principal, - // See `executionActorUserId`: preserves the pre-in-process behavior for unattended runs. - input: { workspaceId, serverId, executionActorUserId: context.userId }, + input: { workspaceId, serverId }, }) signal?.throwIfAborted() return result.tools diff --git a/apps/sim/lib/internal/mcp/execute-tool.ts b/apps/sim/lib/internal/mcp/execute-tool.ts index e271ed3657a..3e900d81af2 100644 --- a/apps/sim/lib/internal/mcp/execute-tool.ts +++ b/apps/sim/lib/internal/mcp/execute-tool.ts @@ -149,9 +149,6 @@ export const executeMcpTool: InternalToolOperationHandler = async (request) => { input: { workspaceId: request.context.workspaceId, serverId, - // The run's execution actor, exactly what the pre-in-process path minted its - // internal token from. Keeps unattended MCP workflows working as before. - executionActorUserId: request.context.userId, toolName, arguments: args, callChain: request.context.callChain, diff --git a/apps/sim/lib/internal/principals/executor.test.ts b/apps/sim/lib/internal/principals/executor.test.ts index 064b418b34c..3409852c2e0 100644 --- a/apps/sim/lib/internal/principals/executor.test.ts +++ b/apps/sim/lib/internal/principals/executor.test.ts @@ -43,6 +43,14 @@ describe('createExecutorPrincipalFromExecutionContext', () => { ...(claims.executionId ? { executionId: claims.executionId } : {}), ...(claims.principal ? { principal: claims.principal } : {}), ...(claims.currentWorkflow ? { currentWorkflow: claims.currentWorkflow } : {}), + ...(options.compatibilityActorUserId + ? { + compatibilityActor: { + kind: 'legacy_execution_user' as const, + userId: options.compatibilityActorUserId, + }, + } + : {}), }, })) }) @@ -66,7 +74,10 @@ describe('createExecutorPrincipalFromExecutionContext', () => { workflowId: 'workflow-origin', executionId: 'execution-origin', }), - { audience: 'sim:tables', resourceScope: { tableId: 'table-1' } } + { + audience: 'sim:tables', + resourceScope: { tableId: 'table-1' }, + } ) }) @@ -152,7 +163,7 @@ describe('createExecutorPrincipalFromExecutionContext', () => { principal, currentWorkflow, }), - { audience: 'sim:tables' } + { audience: 'sim:tables', compatibilityActorUserId: 'user-current' } ) expect(mockBindInternalExecutorDelegation.mock.calls[0]?.[0]).not.toHaveProperty( 'subjectUserId' diff --git a/apps/sim/lib/internal/principals/executor.ts b/apps/sim/lib/internal/principals/executor.ts index 51c5069d736..4aeeb57e5a8 100644 --- a/apps/sim/lib/internal/principals/executor.ts +++ b/apps/sim/lib/internal/principals/executor.ts @@ -33,7 +33,8 @@ async function bindExecutorPrincipal( origin: ExecutorDelegationOrigin, audience: string, resourceScope?: DelegatedPrincipal['resourceScope'], - expiresAt?: Date + expiresAt?: Date, + compatibilityActorUserId?: string ) { if (!origin.workflowId.trim()) throw new Error('Authentication required') const subjectUserId = resolveExecutorOriginSubject(origin) @@ -53,6 +54,7 @@ async function bindExecutorPrincipal( { audience, ...(resourceScope ? { resourceScope } : {}), + ...(!subjectUserId && compatibilityActorUserId ? { compatibilityActorUserId } : {}), } ) } @@ -72,5 +74,5 @@ export async function createExecutorPrincipalFromExecutionContext({ }: CreateExecutorPrincipalFromExecutionContextInput) { const origin = context.executorDelegationOrigin if (!origin) throw new ExecutorDelegationOriginRequiredError() - return bindExecutorPrincipal(origin, audience, resourceScope, expiresAt) + return bindExecutorPrincipal(origin, audience, resourceScope, expiresAt, context.userId) } diff --git a/apps/sim/lib/internal/windchill/execute-tool.test.ts b/apps/sim/lib/internal/windchill/execute-tool.test.ts index ccb1920a144..2f1e3780de6 100644 --- a/apps/sim/lib/internal/windchill/execute-tool.test.ts +++ b/apps/sim/lib/internal/windchill/execute-tool.test.ts @@ -131,7 +131,6 @@ describe('executeWindchillTool', () => { }) expect(mocks.executeWindchillOperation).toHaveBeenCalledWith(operationInput, { principal: PRINCIPAL, - executionActorUserId: 'user-1', requestId: 'request-1', signal: controller.signal, }) diff --git a/apps/sim/lib/internal/windchill/execute-tool.ts b/apps/sim/lib/internal/windchill/execute-tool.ts index 41f096eac32..b49cf727672 100644 --- a/apps/sim/lib/internal/windchill/execute-tool.ts +++ b/apps/sim/lib/internal/windchill/execute-tool.ts @@ -91,7 +91,6 @@ export const executeWindchillTool: InternalToolOperationHandler = async (request const output = await executeWindchillOperation(input, { principal, - executionActorUserId: context.userId, requestId, signal, }) diff --git a/apps/sim/lib/internal/windchill/operations.test.ts b/apps/sim/lib/internal/windchill/operations.test.ts index 879fb9f0e44..badf46d0ba1 100644 --- a/apps/sim/lib/internal/windchill/operations.test.ts +++ b/apps/sim/lib/internal/windchill/operations.test.ts @@ -352,8 +352,22 @@ describe('Windchill operations', () => { primaryFile: rawFile, }, { - principal: { ...PRINCIPAL, subjectUserId: undefined }, - executionActorUserId: 'execution-actor', + principal: { + ...PRINCIPAL, + subjectUserId: undefined, + delegationContext: { + ...PRINCIPAL.delegationContext, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'deployment-1', + }, + compatibilityActor: { + kind: 'legacy_execution_user', + userId: 'execution-actor', + }, + }, + }, requestId: 'request-1', } ) @@ -457,8 +471,22 @@ describe('Windchill operations', () => { documentOid: DOCUMENT_OID, }, { - principal: { ...PRINCIPAL, subjectUserId: undefined }, - executionActorUserId: 'execution-actor', + principal: { + ...PRINCIPAL, + subjectUserId: undefined, + delegationContext: { + ...PRINCIPAL.delegationContext, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'deployment-1', + }, + compatibilityActor: { + kind: 'legacy_execution_user', + userId: 'execution-actor', + }, + }, + }, requestId: 'request-1', } ) diff --git a/apps/sim/lib/internal/windchill/operations.ts b/apps/sim/lib/internal/windchill/operations.ts index ffd8d56b385..c9967e51bbb 100644 --- a/apps/sim/lib/internal/windchill/operations.ts +++ b/apps/sim/lib/internal/windchill/operations.ts @@ -1,6 +1,6 @@ import { type BoundWorkflowExecutionDelegatedPrincipal, - resolvePrincipalSubjectUserId, + resolvePrincipalExecutionActorUserId, } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' @@ -441,10 +441,9 @@ async function loadUploadFiles( } function requireWindchillExecutionUserId( - principal: BoundWorkflowExecutionDelegatedPrincipal, - executionActorUserId?: string + principal: BoundWorkflowExecutionDelegatedPrincipal ): string { - const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId + const userId = resolvePrincipalExecutionActorUserId(principal) if (!userId) { throw new WindchillOperationError('Windchill file operations require an execution actor', 403) } @@ -473,19 +472,17 @@ async function storeDownloadedFile({ buffer, fileName, contentType, - executionActorUserId, signal, }: { principal: BoundWorkflowExecutionDelegatedPrincipal buffer: Buffer fileName: string contentType: string - executionActorUserId?: string signal?: AbortSignal }): Promise { signal?.throwIfAborted() const { workflowId, executionId } = principal.delegationContext - const userId = requireWindchillExecutionUserId(principal, executionActorUserId) + const userId = requireWindchillExecutionUserId(principal) if (executionId) { const file = await uploadExecutionFile( { @@ -518,7 +515,6 @@ async function executeDownload( | { operation: 'windchill_download_attachment' } >, principal: BoundWorkflowExecutionDelegatedPrincipal, - executionActorUserId?: string, signal?: AbortSignal ): Promise { const documentUrl = windchillDocumentUrl(body.baseUrl, body.documentOid) @@ -550,7 +546,6 @@ async function executeDownload( buffer: downloaded.buffer, fileName, contentType: mimeType, - executionActorUserId, signal, }) return { @@ -563,7 +558,6 @@ async function executeDownload( export interface WindchillOperationContext { principal: BoundWorkflowExecutionDelegatedPrincipal - executionActorUserId?: string requestId: string signal?: AbortSignal } @@ -572,14 +566,14 @@ export async function executeWindchillOperation( body: WindchillOperationBody, context: WindchillOperationContext ): Promise { - const { principal, executionActorUserId, requestId, signal } = context + const { principal, requestId, signal } = context signal?.throwIfAborted() if ( body.operation === 'windchill_download_primary_content' || body.operation === 'windchill_download_attachment' ) { - return executeDownload(body, principal, executionActorUserId, signal) + return executeDownload(body, principal, signal) } if ( @@ -592,7 +586,7 @@ export async function executeWindchillOperation( : body.attachmentFiles const files = await loadUploadFiles( inputs, - requireWindchillExecutionUserId(principal, executionActorUserId), + requireWindchillExecutionUserId(principal), requestId, signal ) diff --git a/apps/sim/lib/knowledge/api/internal-route.test.ts b/apps/sim/lib/knowledge/api/internal-route.test.ts index 775c1c15e8d..4d69fea1e32 100644 --- a/apps/sim/lib/knowledge/api/internal-route.test.ts +++ b/apps/sim/lib/knowledge/api/internal-route.test.ts @@ -56,6 +56,10 @@ function executorPrincipal( mode: 'deployment', deploymentVersionId: 'deployment-1', }, + compatibilityActor: { + kind: 'legacy_execution_user', + userId: 'execution-billing-actor-1', + }, ...(originalPrincipal ? { principal: originalPrincipal } : {}), }, } @@ -106,7 +110,6 @@ describe('internal Knowledge execution attribution', () => { workspaceOrganizationId: null, allowPersonalApiKeys: true, billedAccountUserId: 'billing-owner-1', - executionActorUserId: 'execution-billing-actor-1', }) ).toBe('execution-billing-actor-1') }) diff --git a/apps/sim/lib/knowledge/application/authorization.ts b/apps/sim/lib/knowledge/application/authorization.ts index 32c0e6bfb13..15f9dc9b762 100644 --- a/apps/sim/lib/knowledge/application/authorization.ts +++ b/apps/sim/lib/knowledge/application/authorization.ts @@ -16,14 +16,11 @@ interface KnowledgeResourceIdentifiers { export interface KnowledgeAuthorizationContext extends WorkspaceAuthorizationContext, - KnowledgeResourceIdentifiers { - executionActorUserId?: string -} + KnowledgeResourceIdentifiers {} export interface LegacyPersonalKnowledgeAuthorizationContext extends KnowledgeResourceIdentifiers { workspaceId: undefined legacyPersonalOwnerUserId: string - executionActorUserId?: string } export type KnowledgeResourceAuthorizationContext = diff --git a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts index dc8d1112a56..33cd7b143d3 100644 --- a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts +++ b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts @@ -68,13 +68,6 @@ function assertWorkspaceKnowledgeContext( - context: C, - executionActorUserId?: string -): C { - return executionActorUserId ? { ...context, executionActorUserId } : context -} - export function defineAuthorizedKnowledgeUseCase< const O extends WorkspaceOperation, I, @@ -129,8 +122,7 @@ export function defineAuthorizedKnowledgeUseCase< operation: definition.operation, async execute({ principal, input, request }) { requireAllowedWorkspacePrincipal(principal, definition.operation) - const resolvedContext = await definition.resolveContext({ principal, input }) - const context = withExecutionActor(resolvedContext, request?.executionActorUserId) + const context = await definition.resolveContext({ principal, input }) if (isLegacyPersonalKnowledgeContext(context)) { if ( principal.kind === 'workspace_api_key' || diff --git a/apps/sim/lib/knowledge/application/billing.ts b/apps/sim/lib/knowledge/application/billing.ts index efb7001da93..2d372a5da1b 100644 --- a/apps/sim/lib/knowledge/application/billing.ts +++ b/apps/sim/lib/knowledge/application/billing.ts @@ -1,7 +1,7 @@ import { type Principal, resolvePrincipalAttribution, - resolvePrincipalSubjectUserId, + resolvePrincipalExecutionActorUserId, } from '@sim/auth/principal' import { checkActorUsageLimits } from '@/lib/billing/calculations/usage-monitor' import { @@ -24,7 +24,7 @@ export function resolveKnowledgeAttributedUserId( principal: Principal, context: KnowledgeResourceContext ): string { - const executionUserId = resolvePrincipalSubjectUserId(principal) ?? context.executionActorUserId + const executionUserId = resolvePrincipalExecutionActorUserId(principal) if (executionUserId) return executionUserId if (context.workspaceId === undefined) { throw new OrchestrationError( diff --git a/apps/sim/lib/mcp/application/authorization.ts b/apps/sim/lib/mcp/application/authorization.ts index 7bd10b74492..38a67e73f09 100644 --- a/apps/sim/lib/mcp/application/authorization.ts +++ b/apps/sim/lib/mcp/application/authorization.ts @@ -1,4 +1,4 @@ -import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal' +import { type Principal, resolvePrincipalExecutionActorUserId } from '@sim/auth/principal' import type { WorkspaceDelegationPolicy } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -20,15 +20,15 @@ export const mcpServerDelegationPolicy = { * credentials and is gated by that person's permission group, so unlike an * attribution-only read it cannot proceed with nobody named. * - * `executionActorUserId` preserves the behavior that existed before the Logs and - * MCP tools moved in-process. That path minted an internal token from - * `ExecutionContext.userId` and the MCP route ran as that user, so an unattended - * run has always reached MCP as the execution actor. For a schedule, webhook, or - * anonymous public-API run that actor is the workspace system actor resolved - * during preprocessing — the billing payer — not the workflow's author. Keeping - * it is what stops every unattended MCP workflow from breaking; changing it is a - * product decision, not a refactor, and a workspace-level MCP identity is the - * real fix. + * The principal-bound compatibility actor preserves the behavior that existed + * before the Logs and MCP tools moved in-process. That path minted an internal + * token from `ExecutionContext.userId` and the MCP route ran as that user, so an + * unattended run has always reached MCP as the execution actor. For a schedule, + * webhook, or anonymous public-API run that actor is the workspace system actor + * resolved during preprocessing — the billing payer — not the workflow's + * author. Keeping it is what stops every unattended MCP workflow from breaking; + * changing it is a product decision, not a refactor, and a workspace-level MCP + * identity is the real fix. * * The fallback deliberately covers a webhook carrying an `external_user` subject * too. That subject is a real identity but never a Sim user, so it has no Sim @@ -36,15 +36,12 @@ export const mcpServerDelegationPolicy = { * Refusing them here would break working workflows in the name of a boundary the * old path never drew. * - * It is NOT an authorization input: workspace reach is decided by the principal - * before this is read, and a principal that names its own subject always wins, so - * a caller cannot use this to nominate someone else's credentials. + * It is not a separate authorization input: the trusted executor binds it into + * the principal before the use case runs, workspace reach is decided before it + * is read, and a principal that names its own subject always wins. */ -export function requireMcpCredentialUserId( - principal: Principal, - executionActorUserId?: string -): string { - const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId +export function requireMcpCredentialUserId(principal: Principal): string { + const userId = resolvePrincipalExecutionActorUserId(principal) if (!userId) { throw new OrchestrationError( 'forbidden', diff --git a/apps/sim/lib/mcp/application/execute-tool.test.ts b/apps/sim/lib/mcp/application/execute-tool.test.ts index b0711c22960..8406c18a812 100644 --- a/apps/sim/lib/mcp/application/execute-tool.test.ts +++ b/apps/sim/lib/mcp/application/execute-tool.test.ts @@ -85,6 +85,16 @@ const ACTORLESS_PRINCIPAL: WorkflowExecutionDelegatedPrincipal = { }, }, } +const COMPATIBILITY_ACTOR_PRINCIPAL: WorkflowExecutionDelegatedPrincipal = { + ...ACTORLESS_PRINCIPAL, + delegationContext: { + ...ACTORLESS_PRINCIPAL.delegationContext, + compatibilityActor: { + kind: 'legacy_execution_user', + userId: 'execution-actor', + }, + }, +} describe('executeMcpToolUseCase', () => { beforeEach(() => { @@ -173,13 +183,12 @@ describe('executeMcpToolUseCase', () => { // ExecutionContext.userId and MCP ran as that user. Preserved deliberately — // see requireMcpCredentialUserId for why that actor is the payer, not the author. await executeMcpToolUseCase.execute({ - principal: ACTORLESS_PRINCIPAL, + principal: COMPATIBILITY_ACTOR_PRINCIPAL, input: { workspaceId: WORKSPACE.workspaceId, serverId: SERVER.id, toolName: 'lookup', arguments: { count: '2', enabled: 'true', tags: 'a,b' }, - executionActorUserId: 'execution-actor', }, }) @@ -189,17 +198,23 @@ describe('executeMcpToolUseCase', () => { expect(mocks.discoverServerTools.mock.calls[0][0]).toBe('execution-actor') }) - it('lets an authenticated subject win over the execution actor', async () => { - // The property that keeps the fallback from becoming an impersonation handle: - // it is consulted only when the principal names nobody. + it('lets an authenticated subject win over a principal-bound compatibility actor', async () => { await executeMcpToolUseCase.execute({ - principal: PRINCIPAL, + principal: { + ...PRINCIPAL, + delegationContext: { + ...PRINCIPAL.delegationContext, + compatibilityActor: { + kind: 'legacy_execution_user', + userId: 'someone-else', + }, + }, + }, input: { workspaceId: WORKSPACE.workspaceId, serverId: SERVER.id, toolName: 'lookup', arguments: { count: '2', enabled: 'true', tags: 'a,b' }, - executionActorUserId: 'someone-else', }, }) @@ -214,9 +229,9 @@ describe('executeMcpToolUseCase', () => { // the actor. Refusing here would break workflows that worked before the tools // moved in-process, so the fallback deliberately covers this case. const externalSubjectPrincipal = { - ...ACTORLESS_PRINCIPAL, + ...COMPATIBILITY_ACTOR_PRINCIPAL, delegationContext: { - ...ACTORLESS_PRINCIPAL.delegationContext, + ...COMPATIBILITY_ACTOR_PRINCIPAL.delegationContext, principal: { kind: 'system' as const, serviceId: 'webhook' as const, @@ -241,7 +256,6 @@ describe('executeMcpToolUseCase', () => { serverId: SERVER.id, toolName: 'lookup', arguments: { count: '2', enabled: 'true', tags: 'a,b' }, - executionActorUserId: 'execution-actor', }, }) diff --git a/apps/sim/lib/mcp/application/execute-tool.ts b/apps/sim/lib/mcp/application/execute-tool.ts index 8f71be4582d..60dba14a253 100644 --- a/apps/sim/lib/mcp/application/execute-tool.ts +++ b/apps/sim/lib/mcp/application/execute-tool.ts @@ -26,11 +26,6 @@ interface SchemaProperty { export interface ExecuteMcpToolInput { workspaceId: string serverId: string - /** - * The run's execution actor. See {@link requireMcpCredentialUserId}: preserves - * the pre-in-process behavior for runs whose principal names no Sim user. - */ - executionActorUserId?: string toolName: string arguments?: Record callChain?: string[] @@ -139,7 +134,7 @@ export const executeMcpToolUseCase = defineAuthorizedWorkspaceUseCase({ authorizationOptions: { delegation: mcpServerDelegationPolicy }, async execute({ principal, input, context }): Promise { input.signal?.throwIfAborted() - const userId = requireMcpCredentialUserId(principal, input.executionActorUserId) + const userId = requireMcpCredentialUserId(principal) await assertPermissionsAllowed({ userId, workspaceId: context.workspaceId, diff --git a/apps/sim/lib/mcp/application/use-cases.ts b/apps/sim/lib/mcp/application/use-cases.ts index 18085334775..60bdb4840a7 100644 --- a/apps/sim/lib/mcp/application/use-cases.ts +++ b/apps/sim/lib/mcp/application/use-cases.ts @@ -91,11 +91,6 @@ export const listMcpServersUseCase = defineAuthorizedWorkspaceUseCase({ export interface DiscoverMcpToolsInput { workspaceId: string - /** - * The run's execution actor. See {@link requireMcpCredentialUserId}: preserves - * the pre-in-process behavior for runs whose principal names no Sim user. - */ - executionActorUserId?: string refresh?: boolean } @@ -106,7 +101,7 @@ export const discoverMcpToolsUseCase = defineAuthorizedWorkspaceUseCase({ authorizationOptions, async execute({ principal, input, context }) { const tools = await mcpService.discoverTools( - requireMcpCredentialUserId(principal, input.executionActorUserId), + requireMcpCredentialUserId(principal), context.workspaceId, /** * A public `refresh` skips the positive cache but keeps the failure @@ -122,11 +117,6 @@ export const discoverMcpToolsUseCase = defineAuthorizedWorkspaceUseCase({ export interface DiscoverMcpServerToolsInput { workspaceId: string serverId: string - /** - * The run's execution actor. See {@link requireMcpCredentialUserId}: preserves - * the pre-in-process behavior for runs whose principal names no Sim user. - */ - executionActorUserId?: string refresh?: boolean } @@ -164,7 +154,7 @@ export const discoverMcpServerToolsUseCase = defineAuthorizedWorkspaceUseCase({ } const tools = await mcpService.discoverServerTools( - requireMcpCredentialUserId(principal, input.executionActorUserId), + requireMcpCredentialUserId(principal), context.server.id, context.workspaceId, /** diff --git a/apps/sim/lib/workflows/application/authorization.test.ts b/apps/sim/lib/workflows/application/authorization.test.ts index b038ff39405..ac5f38dfb24 100644 --- a/apps/sim/lib/workflows/application/authorization.test.ts +++ b/apps/sim/lib/workflows/application/authorization.test.ts @@ -78,12 +78,43 @@ describe('workflow delegation policy', () => { describe('workflow execution actor', () => { it('uses the legacy execution actor when the principal is actorless', () => { - const principal = createExecutorPrincipal({ subjectUserId: undefined }) + const principal = createExecutorPrincipal({ + subjectUserId: undefined, + delegationContext: { + kind: 'workflow_execution', + workflowId: 'parent-workflow', + currentWorkflow: { + workflowId: 'parent-workflow', + mode: 'deployment', + deploymentVersionId: 'deployment-1', + }, + compatibilityActor: { + kind: 'legacy_execution_user', + userId: 'execution-actor', + }, + }, + }) - expect(requireWorkflowExecutionUserId(principal, 'execution-actor')).toBe('execution-actor') + expect(requireWorkflowExecutionUserId(principal)).toBe('execution-actor') }) it('prefers a real principal subject over the compatibility actor', () => { - expect(requireWorkflowExecutionUserId(createExecutorPrincipal(), 'someone-else')).toBe('user-1') + const principal = createExecutorPrincipal({ + delegationContext: { + kind: 'workflow_execution', + workflowId: 'parent-workflow', + currentWorkflow: { + workflowId: 'parent-workflow', + mode: 'deployment', + deploymentVersionId: 'deployment-1', + }, + compatibilityActor: { + kind: 'legacy_execution_user', + userId: 'someone-else', + }, + }, + }) + + expect(requireWorkflowExecutionUserId(principal)).toBe('user-1') }) }) diff --git a/apps/sim/lib/workflows/application/authorization.ts b/apps/sim/lib/workflows/application/authorization.ts index 82b21bd90a5..1448e30a837 100644 --- a/apps/sim/lib/workflows/application/authorization.ts +++ b/apps/sim/lib/workflows/application/authorization.ts @@ -1,4 +1,4 @@ -import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal' +import { type Principal, resolvePrincipalExecutionActorUserId } from '@sim/auth/principal' import type { WorkspaceAuthorizationContext, WorkspaceDelegationPolicy, @@ -36,11 +36,8 @@ export const workflowDelegationPolicy: WorkspaceDelegationPolicy, async execute({ principal, input, context }) { - const userId = requireWorkflowExecutionUserId(principal, input.executionActorUserId) + const userId = requireWorkflowExecutionUserId(principal) await requireMutableWorkflow(context.workflowId) const result = await performRevertToVersion({ workflowId: context.workflowId, diff --git a/apps/sim/lib/workspace-files/application/share-workspace-file.ts b/apps/sim/lib/workspace-files/application/share-workspace-file.ts index 4192f848084..b1584f0f977 100644 --- a/apps/sim/lib/workspace-files/application/share-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/share-workspace-file.ts @@ -1,5 +1,5 @@ import { AuditAction, AuditResourceType } from '@sim/audit' -import { resolvePrincipalSubjectUserId } from '@sim/auth/principal' +import { resolvePrincipalExecutionActorUserId } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import type { ShareAuthType, ShareRecord } from '@/lib/api/contracts/public-shares' import { ForbiddenOperationError } from '@/lib/core/application/forbidden' @@ -38,7 +38,6 @@ export interface UpdateWorkspaceFileShareInput { allowedEmails?: string[] token?: string noOpIfInactive?: boolean - executionActorUserId?: string } export interface UpdateWorkspaceFileShareResult { @@ -73,7 +72,7 @@ export const updateWorkspaceFileShare = defineAuthorizedWorkspaceFileUseCase({ return { ...canonical, file } }, async execute({ principal, input, context }): Promise { - const userId = resolvePrincipalSubjectUserId(principal) ?? input.executionActorUserId + const userId = resolvePrincipalExecutionActorUserId(principal) if (!userId) { throw new OrchestrationError( 'forbidden', diff --git a/packages/auth/src/principal.ts b/packages/auth/src/principal.ts index 3336e5bb624..e47e9dd9262 100644 --- a/packages/auth/src/principal.ts +++ b/packages/auth/src/principal.ts @@ -78,6 +78,16 @@ export interface WorkflowExecutionDelegationContext { executionId?: string principal?: WorkflowExecutionPrincipal currentWorkflow?: WorkflowExecutionAuthority + /** + * The trusted Sim user ID legacy executor routes ran as before principal wiring. + * + * This is compatibility policy, not the authenticated subject: workspace + * authorization and audit identity continue to use the principal itself. + */ + compatibilityActor?: { + kind: 'legacy_execution_user' + userId: string + } } export type WorkflowExecutionAuthority = @@ -137,6 +147,22 @@ export function requirePrincipalSubjectUserId(principal: Principal): string { throw new PrincipalSubjectUserRequiredError(principal.kind) } +/** + * Resolves the principal's Sim user subject or its principal-bound legacy + * execution actor. + * + * Only operations that deliberately preserve pre-principal executor behavior + * should use this helper. It never changes the principal subject, workspace + * authorization, or audit actor. + */ +export function resolvePrincipalExecutionActorUserId(principal: Principal): string | undefined { + const subjectUserId = resolvePrincipalSubjectUserId(principal) + if (subjectUserId) return subjectUserId + if (principal.kind !== 'delegated' || principal.serviceId !== 'executor') return undefined + if (principal.delegationContext?.currentWorkflow?.mode !== 'deployment') return undefined + return principal.delegationContext?.compatibilityActor?.userId +} + export type WorkflowExecutionPrincipal = | SessionPrincipal | PersonalApiKeyPrincipal