diff --git a/apps/sim/lib/auth/principal.test.ts b/apps/sim/lib/auth/principal.test.ts index 1121a6d9fc5..e938f062a58 100644 --- a/apps/sim/lib/auth/principal.test.ts +++ b/apps/sim/lib/auth/principal.test.ts @@ -8,6 +8,7 @@ import { resolvePrincipalAttribution, resolvePrincipalAuditAttribution, resolvePrincipalSubject, + resolvePrincipalSubjectUserId, serializePrincipal, toPrincipalActor, } from '@sim/auth/principal' @@ -43,6 +44,70 @@ describe('principal subject users', () => { ).toBe('delegated-user') }) + it('resolves the same subject without demanding one', () => { + expect( + resolvePrincipalSubjectUserId({ + kind: 'session', + userId: 'session-user', + sessionId: 'session-1', + }) + ).toBe('session-user') + expect( + resolvePrincipalSubjectUserId({ + kind: 'delegated', + serviceId: 'executor', + subjectUserId: 'delegated-user', + 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'), + }) + ).toBe('delegated-user') + }) + + it('answers undefined for an actorless caller rather than throwing', () => { + // The distinction the two helpers exist to make visible: a schedule, a webhook + // with no external subject, and a workspace key are all authorized callers that + // simply have no person. Attribution-only reads take this branch. + expect( + resolvePrincipalSubjectUserId({ + kind: 'system', + serviceId: 'schedule', + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + }) + ).toBeUndefined() + expect( + resolvePrincipalSubjectUserId({ + kind: 'delegated', + serviceId: 'executor', + 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', + workflowId: 'workflow-1', + principal: { + kind: 'system', + serviceId: 'schedule', + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + }, + }, + }) + ).toBeUndefined() + expect( + resolvePrincipalSubjectUserId({ + kind: 'workspace_api_key', + keyId: 'key-1', + workspaceId: 'workspace-1', + }) + ).toBeUndefined() + }) + it('fails fast instead of fabricating a workspace-key subject', () => { expect(() => requirePrincipalSubjectUserId({ diff --git a/apps/sim/lib/credential-groups/application/create-invite-link.test.ts b/apps/sim/lib/credential-groups/application/create-invite-link.test.ts index 80625359fed..388ee4e8f5a 100644 --- a/apps/sim/lib/credential-groups/application/create-invite-link.test.ts +++ b/apps/sim/lib/credential-groups/application/create-invite-link.test.ts @@ -104,6 +104,48 @@ describe('createCredentialGroupInviteLink', () => { expect(mocks.resolveGroup).not.toHaveBeenCalled() }) + it('issues an unattributed link for an actorless run', async () => { + // A schedule (or a webhook with no external subject) reaches this with a real + // admin-scoped delegation and no person on it. The delegation is the authority; + // the issuer is only recorded, and `created_by` is nullable — so this issues the + // link with no issuer rather than refusing, which is what it did when the + // subject was demanded here. + const { subjectUserId: _subject, ...base } = executorPrincipal() + // What actually authorizes an actorless caller: the delegation is running a + // deployment. No user is consulted anywhere in that decision. + const actorless = { + ...base, + delegationContext: { + kind: 'workflow_execution' as const, + workflowId: 'workflow-1', + principal: { + kind: 'system' as const, + serviceId: 'schedule' as const, + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + }, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment' as const, + deploymentVersionId: 'version-1', + }, + }, + } + + const result = await createCredentialGroupInviteLink.execute({ + principal: actorless, + input: { credentialGroupId: 'group-1', email: 'person@example.com' }, + }) + + expect(result.invitationLink).toBe('https://sim.ai/credential-groups/enroll/token-1') + expect(mocks.createInvitationLink).toHaveBeenCalledWith( + 'workspace-1', + 'group-1', + undefined, + 'person@example.com' + ) + }) + it('rejects delegation scoped to another Credential Group', async () => { await expect( createCredentialGroupInviteLink.execute({ diff --git a/apps/sim/lib/credential-groups/application/create-invite-link.ts b/apps/sim/lib/credential-groups/application/create-invite-link.ts index 90046594d62..9290ba46e59 100644 --- a/apps/sim/lib/credential-groups/application/create-invite-link.ts +++ b/apps/sim/lib/credential-groups/application/create-invite-link.ts @@ -1,5 +1,5 @@ import { AuditAction, AuditResourceType } from '@sim/audit' -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' +import { resolvePrincipalSubjectUserId } from '@sim/auth/principal' import { isValidEmailSyntax, normalizeEmail } from '@sim/utils/string' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -39,7 +39,9 @@ export const createCredentialGroupInviteLink = defineAuthorizedWorkspaceUseCase( return await createCredentialGroupInvitationLink( context.workspaceId, context.credentialGroupId, - requirePrincipalSubjectUserId(principal), + // Attribution, not authority: the delegation's admin-scoped Credential Group + // grant is what permits this. An actorless run records no issuer. + resolvePrincipalSubjectUserId(principal), email ) } catch (error) { diff --git a/apps/sim/lib/credential-groups/application/send-invite.ts b/apps/sim/lib/credential-groups/application/send-invite.ts index 071d0136204..31ed72e9330 100644 --- a/apps/sim/lib/credential-groups/application/send-invite.ts +++ b/apps/sim/lib/credential-groups/application/send-invite.ts @@ -1,5 +1,4 @@ import { AuditAction, AuditResourceType } from '@sim/audit' -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { isValidEmailSyntax, normalizeEmail } from '@sim/utils/string' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -41,7 +40,7 @@ export const sendCredentialGroupInvite = defineAuthorizedWorkspaceUseCase({ } await requireCredentialGroupsAvailable(context.workspaceId) - const userId = requirePrincipalSubjectUserId(principal) + const userId = requireCredentialGroupWorkflowSubject(principal) const inviter = await loadCredentialGroupInviterIdentity(userId) const inviterName = inviter?.name?.trim() || inviter?.email if (!inviterName) { diff --git a/apps/sim/lib/credential-groups/enrollments.ts b/apps/sim/lib/credential-groups/enrollments.ts index 093ddaad70a..91e661c7238 100644 --- a/apps/sim/lib/credential-groups/enrollments.ts +++ b/apps/sim/lib/credential-groups/enrollments.ts @@ -330,7 +330,14 @@ async function getInvitationContext( async function issueInvitation( context: InvitationContext, - userId: string, + /** + * Who to record as the issuer, when there is someone. Attribution only — the + * authority to invite comes from the delegation, so an actorless run (a schedule, + * or a webhook with no external subject) issues an unattributed invitation rather + * than none. `created_by` is nullable and `on delete set null`, so a row with no + * issuer is a shape the schema already carries. + */ + userId: string | undefined, email: string, options: SendInvitationOptions ): Promise { @@ -387,7 +394,7 @@ async function issueInvitation( completedAt: preservesProgress ? current.completedAt : null, revokedAt: null, lastDeliveryError: null, - createdBy: userId, + createdBy: userId ?? null, updatedAt: now, } const [next] = current @@ -664,7 +671,8 @@ export async function inviteCredentialGroupEnrollment( export async function createCredentialGroupInvitationLink( workspaceId: string, groupId: string, - userId: string, + /** See {@link issueInvitation}: the issuer is attribution, never the authority. */ + userId: string | undefined, email: string ): Promise { const context = await getInvitationContext(workspaceId, groupId) 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 7902af3d970..6fe719b99a8 100644 --- a/apps/sim/lib/credentials/application/authorized-credential-use-case.ts +++ b/apps/sim/lib/credentials/application/authorized-credential-use-case.ts @@ -88,6 +88,7 @@ export function defineAuthorizedCredentialUseCase< async authorizeResource({ principal, 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) ) if ( diff --git a/apps/sim/lib/credentials/application/connection-target.ts b/apps/sim/lib/credentials/application/connection-target.ts index a7cb618ceab..38dfcd482b7 100644 --- a/apps/sim/lib/credentials/application/connection-target.ts +++ b/apps/sim/lib/credentials/application/connection-target.ts @@ -46,6 +46,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 targetCredentialId = credentialId const credential = await getWorkspaceCredential({ diff --git a/apps/sim/lib/custom-tools/application/use-cases.ts b/apps/sim/lib/custom-tools/application/use-cases.ts index 177b6e379b1..a0a20abaebe 100644 --- a/apps/sim/lib/custom-tools/application/use-cases.ts +++ b/apps/sim/lib/custom-tools/application/use-cases.ts @@ -70,6 +70,7 @@ async function resolveAvailableToolContext(args: { 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), workspaceId: workspace.workspaceId, }) @@ -129,6 +130,7 @@ export const listAvailableCustomToolsUseCase = defineAuthorizedWorkspaceUseCase( authorizationOptions, async execute({ principal, 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), workspaceId: context.workspaceId, }) @@ -353,6 +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), title, schema: input.schema ?? context.tool.schema, @@ -422,6 +425,7 @@ export const deleteAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase 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), }) if (!deleted) throw new OrchestrationError('not_found', 'Custom tool not found') diff --git a/apps/sim/lib/internal/deployments/execute-tool.ts b/apps/sim/lib/internal/deployments/execute-tool.ts index 13f70b35f68..7bfe4d4bf42 100644 --- a/apps/sim/lib/internal/deployments/execute-tool.ts +++ b/apps/sim/lib/internal/deployments/execute-tool.ts @@ -3,7 +3,6 @@ import { isPlainRecord } from '@sim/utils/object' import type { ZodError, ZodType } from 'zod' import { getValidationErrorMessage } from '@/lib/api/server' import { concealCrossTenantResourceError } from '@/lib/api/server/routes' -import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import { deploymentsDeployBodySchema, @@ -20,6 +19,11 @@ import { executeDeploymentsUndeploy, } from '@/lib/internal/deployments/operations' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' +import { + classifyInternalToolIdentityFault, + internalToolIdentityFaultMessage, + internalToolIdentityFaultStatus, +} from '@/lib/internal/tool-operations/identity-faults' import type { InternalToolOperationCall, InternalToolOperationHandler, @@ -52,11 +56,12 @@ function parseInput(schema: ZodType, request: InternalToolOperationCall) { } function errorResponse(request: InternalToolOperationCall, error: unknown): Response { - if ( - error instanceof InvalidInternalDelegationBindingError || - (error instanceof Error && error.message === 'Authentication required') - ) { - return Response.json({ success: false, error: 'Authentication required' }, { status: 401 }) + const identityFault = classifyInternalToolIdentityFault(error) + if (identityFault) { + return Response.json( + { success: false, error: internalToolIdentityFaultMessage(identityFault) }, + { status: internalToolIdentityFaultStatus(identityFault) } + ) } const classified = asOrchestrationError( diff --git a/apps/sim/lib/internal/file/execute-tool.ts b/apps/sim/lib/internal/file/execute-tool.ts index b709572e237..1d70d1f5a0f 100644 --- a/apps/sim/lib/internal/file/execute-tool.ts +++ b/apps/sim/lib/internal/file/execute-tool.ts @@ -1,16 +1,16 @@ -import { - PrincipalSubjectUserRequiredError, - resolvePrincipalAttribution, - resolvePrincipalSubject, -} from '@sim/auth/principal' +import { resolvePrincipalAttribution, resolvePrincipalSubject } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { fileParseContract } from '@/lib/api/contracts/storage-transfer' import { fileManageContract } from '@/lib/api/contracts/tools/file' -import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' import { executeFileManageOperation } from '@/lib/internal/file/operations' import { executeFileParserOperation } from '@/lib/internal/file/parser' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' +import { + classifyInternalToolIdentityFault, + internalToolIdentityFaultMessage, + internalToolIdentityFaultStatus, +} from '@/lib/internal/tool-operations/identity-faults' import { parseInternalToolInput } from '@/lib/internal/tool-operations/parse-input' import type { InternalToolOperationHandler } from '@/lib/internal/tool-operations/types' import { WORKSPACE_FILES_DELEGATION_AUDIENCE } from '@/lib/workspace-files/application/authorization' @@ -104,12 +104,12 @@ export const executeFileTool: InternalToolOperationHandler = async (request) => return response } catch (error) { request.signal?.throwIfAborted() - if ( - error instanceof InvalidInternalDelegationBindingError || - error instanceof PrincipalSubjectUserRequiredError || - (error instanceof Error && error.message === 'Authentication required') - ) { - return Response.json({ success: false, error: 'Authentication required' }, { status: 401 }) + const identityFault = classifyInternalToolIdentityFault(error) + if (identityFault) { + return Response.json( + { success: false, error: internalToolIdentityFaultMessage(identityFault) }, + { status: internalToolIdentityFaultStatus(identityFault) } + ) } const message = getErrorMessage(error, 'Unknown error') logger.error('File operation dispatch failed', { diff --git a/apps/sim/lib/internal/knowledge/execute-tool.ts b/apps/sim/lib/internal/knowledge/execute-tool.ts index a9fc25818ae..16f5535f5fc 100644 --- a/apps/sim/lib/internal/knowledge/execute-tool.ts +++ b/apps/sim/lib/internal/knowledge/execute-tool.ts @@ -18,7 +18,6 @@ import { upsertKnowledgeDocumentContract, } from '@/lib/api/contracts/knowledge' import type { JsonErrorResponseDescriptor } from '@/lib/api/server/routes/types' -import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' import { createChunkOperation, createDocumentsOperation, @@ -37,6 +36,11 @@ import { upsertDocumentOperation, } from '@/lib/internal/knowledge/operations' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' +import { + classifyInternalToolIdentityFault, + internalToolIdentityFaultMessage, + internalToolIdentityFaultStatus, +} from '@/lib/internal/tool-operations/identity-faults' import { parseInternalContractInput, parseInternalOperationInput, @@ -125,11 +129,12 @@ export const executeKnowledgeTool: InternalToolOperationHandler = async (request audience: KNOWLEDGE_DELEGATION_AUDIENCE, }) } catch (error) { - if ( - error instanceof InvalidInternalDelegationBindingError || - (error instanceof Error && error.message === 'Authentication required') - ) { - return Response.json({ error: 'Authentication required' }, { status: 401 }) + const identityFault = classifyInternalToolIdentityFault(error) + if (identityFault) { + return Response.json( + { error: internalToolIdentityFaultMessage(identityFault) }, + { status: internalToolIdentityFaultStatus(identityFault) } + ) } throw error } diff --git a/apps/sim/lib/internal/logs/execute-tool.test.ts b/apps/sim/lib/internal/logs/execute-tool.test.ts index e4daa1dc157..842d133d9b4 100644 --- a/apps/sim/lib/internal/logs/execute-tool.test.ts +++ b/apps/sim/lib/internal/logs/execute-tool.test.ts @@ -2,7 +2,10 @@ * @vitest-environment node */ -import type { WorkflowExecutionDelegatedPrincipal } from '@sim/auth/principal' +import { + PrincipalSubjectUserRequiredError, + type WorkflowExecutionDelegatedPrincipal, +} from '@sim/auth/principal' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { ExecutionContext } from '@/executor/types' @@ -25,6 +28,7 @@ vi.mock('@/lib/internal/logs/operations', () => ({ })) import { executeLogsTool } from '@/lib/internal/logs/execute-tool' +import { ExecutorDelegationOriginRequiredError } from '@/lib/internal/tool-operations/identity-faults' const PRINCIPAL: WorkflowExecutionDelegatedPrincipal = { kind: 'delegated', @@ -169,4 +173,41 @@ describe('executeLogsTool', () => { expect(invalidResponse.status).toBe(500) expect(await invalidResponse.json()).toEqual({ error: 'Failed to fetch log' }) }) + + it('answers a missing execution context as unauthenticated, not as a broken tool', async () => { + // A caller with no executor delegation origin never established an identity. + // The error was untyped, so it fell past the classifier into a generic 500. + mocks.createPrincipal.mockRejectedValueOnce(new ExecutorDelegationOriginRequiredError()) + + const response = await executeLogsTool({ + toolId: 'logs_query', + input: {}, + headers: new Headers(), + context: CONTEXT, + requestId: 'request-1', + }) + + expect(response.status).toBe(401) + expect(await response.json()).toEqual({ error: 'Authentication required' }) + }) + + it('names the missing identity instead of answering an opaque 500', async () => { + // The regression this guards: an operation that still demands a person answered + // every scheduled run with `Failed to fetch log`, which says nothing about why. + mocks.getRun.mockRejectedValueOnce(new PrincipalSubjectUserRequiredError('delegated')) + + const response = await executeLogsTool({ + toolId: 'logs_get_run_details', + input: { executionId: 'execution-1' }, + headers: new Headers(), + context: CONTEXT, + requestId: 'request-1', + }) + + expect(response.status).toBe(403) + expect(await response.json()).toEqual({ + error: + 'This tool requires a user identity, and this run has none — scheduled and webhook triggers run without a user', + }) + }) }) diff --git a/apps/sim/lib/internal/logs/execute-tool.ts b/apps/sim/lib/internal/logs/execute-tool.ts index 59870040852..58ba3b5ef73 100644 --- a/apps/sim/lib/internal/logs/execute-tool.ts +++ b/apps/sim/lib/internal/logs/execute-tool.ts @@ -12,7 +12,6 @@ import { logIdParamsSchema, } from '@/lib/api/contracts/logs' import { serializeZodIssues } from '@/lib/api/server/validation' -import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import { executeLogsGet, @@ -22,6 +21,11 @@ import { type LogsToolOperationContext, } from '@/lib/internal/logs/operations' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' +import { + classifyInternalToolIdentityFault, + internalToolIdentityFaultMessage, + internalToolIdentityFaultStatus, +} from '@/lib/internal/tool-operations/identity-faults' import type { InternalToolOperationHandler } from '@/lib/internal/tool-operations/types' import { LOGS_DELEGATION_AUDIENCE } from '@/lib/logs/application/authorization' @@ -137,11 +141,12 @@ export const executeLogsTool: InternalToolOperationHandler = async (request) => return Response.json(dispatched.contract.response.schema.parse(dispatched.body)) } catch (error) { request.signal?.throwIfAborted() - if ( - error instanceof InvalidInternalDelegationBindingError || - (error instanceof Error && error.message === 'Authentication required') - ) { - return Response.json({ error: 'Authentication required' }, { status: 401 }) + const identityFault = classifyInternalToolIdentityFault(error) + if (identityFault) { + return Response.json( + { error: internalToolIdentityFaultMessage(identityFault) }, + { status: internalToolIdentityFaultStatus(identityFault) } + ) } return errorResponse(request.toolId, error) } diff --git a/apps/sim/lib/internal/mcp/discover-tools.ts b/apps/sim/lib/internal/mcp/discover-tools.ts index 78050f11e9c..fcf1f709a46 100644 --- a/apps/sim/lib/internal/mcp/discover-tools.ts +++ b/apps/sim/lib/internal/mcp/discover-tools.ts @@ -25,7 +25,8 @@ export async function discoverMcpServerToolsAsExecutor({ signal?.throwIfAborted() const result = await discoverMcpServerToolsUseCase.execute({ principal, - input: { workspaceId, serverId }, + // See `executionActorUserId`: preserves the pre-in-process behavior for unattended runs. + input: { workspaceId, serverId, executionActorUserId: context.userId }, }) 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 33ce7d4413f..e271ed3657a 100644 --- a/apps/sim/lib/internal/mcp/execute-tool.ts +++ b/apps/sim/lib/internal/mcp/execute-tool.ts @@ -3,7 +3,6 @@ import { resolvePrincipalSubject } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { isPlainRecord } from '@sim/utils/object' -import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' import { capExecutionTimeoutMs, getAsyncExecutionTimeoutForBillingAttribution, @@ -11,6 +10,11 @@ import { } from '@/lib/core/execution-limits' import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' +import { + classifyInternalToolIdentityFault, + internalToolIdentityFaultMessage, + internalToolIdentityFaultStatus, +} from '@/lib/internal/tool-operations/identity-faults' import type { InternalToolOperationHandler } from '@/lib/internal/tool-operations/types' import { MCP_SERVER_DELEGATION_AUDIENCE } from '@/lib/mcp/application/authorization' import { executeMcpToolUseCase, McpToolsNotAllowedError } from '@/lib/mcp/application/execute-tool' @@ -145,6 +149,9 @@ 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, @@ -168,11 +175,12 @@ export const executeMcpTool: InternalToolOperationHandler = async (request) => { ) } catch (error) { request.signal?.throwIfAborted() - if ( - error instanceof InvalidInternalDelegationBindingError || - (error instanceof Error && error.message === 'Authentication required') - ) { - return Response.json({ success: false, error: 'Authentication required' }, { status: 401 }) + const identityFault = classifyInternalToolIdentityFault(error) + if (identityFault) { + return Response.json( + { success: false, error: internalToolIdentityFaultMessage(identityFault) }, + { status: internalToolIdentityFaultStatus(identityFault) } + ) } if (error instanceof McpToolsNotAllowedError) { return createResponse( diff --git a/apps/sim/lib/internal/memory/execute-tool.ts b/apps/sim/lib/internal/memory/execute-tool.ts index 19374b80108..b3c0fe5dd91 100644 --- a/apps/sim/lib/internal/memory/execute-tool.ts +++ b/apps/sim/lib/internal/memory/execute-tool.ts @@ -13,7 +13,6 @@ import { memoryPostBodySchema, } from '@/lib/api/contracts/memory' import { serializeZodIssues } from '@/lib/api/server/validation' -import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import { executeMemoryAdd, @@ -25,6 +24,11 @@ import { } from '@/lib/internal/memory/operations' import { createMemoryToolResponse, MemoryProvenanceError } from '@/lib/internal/memory/provenance' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' +import { + classifyInternalToolIdentityFault, + internalToolIdentityFaultMessage, + internalToolIdentityFaultStatus, +} from '@/lib/internal/tool-operations/identity-faults' import type { InternalToolOperationHandler } from '@/lib/internal/tool-operations/types' import { MEMORY_DELEGATION_AUDIENCE } from '@/lib/memory/application/authorization' @@ -142,13 +146,11 @@ export const executeMemoryTool: InternalToolOperationHandler = async (request) = ) } catch (error) { request.signal?.throwIfAborted() - if ( - error instanceof InvalidInternalDelegationBindingError || - (error instanceof Error && error.message === 'Authentication required') - ) { + const identityFault = classifyInternalToolIdentityFault(error) + if (identityFault) { return Response.json( - { success: false, error: { message: 'Authentication required' } }, - { status: 401 } + { success: false, error: { message: internalToolIdentityFaultMessage(identityFault) } }, + { status: internalToolIdentityFaultStatus(identityFault) } ) } return failureResponse(request.toolId, error) diff --git a/apps/sim/lib/internal/principals/executor.ts b/apps/sim/lib/internal/principals/executor.ts index 9295ef88fb5..51c5069d736 100644 --- a/apps/sim/lib/internal/principals/executor.ts +++ b/apps/sim/lib/internal/principals/executor.ts @@ -1,6 +1,7 @@ import { type DelegatedPrincipal, resolvePrincipalSubject } from '@sim/auth/principal' import { generateId } from '@sim/utils/id' import { bindInternalExecutorDelegation } from '@/lib/auth/internal-delegation' +import { ExecutorDelegationOriginRequiredError } from '@/lib/internal/tool-operations/identity-faults' import type { InternalToolOperationContext } from '@/lib/internal/tool-operations/types' import type { ExecutorDelegationOrigin } from '@/executor/types' @@ -70,6 +71,6 @@ export async function createExecutorPrincipalFromExecutionContext({ expiresAt, }: CreateExecutorPrincipalFromExecutionContextInput) { const origin = context.executorDelegationOrigin - if (!origin) throw new Error('Executor delegation origin is required') + if (!origin) throw new ExecutorDelegationOriginRequiredError() return bindExecutorPrincipal(origin, audience, resourceScope, expiresAt) } diff --git a/apps/sim/lib/internal/table/execute-tool.ts b/apps/sim/lib/internal/table/execute-tool.ts index 299d94f4a75..d0776a23fde 100644 --- a/apps/sim/lib/internal/table/execute-tool.ts +++ b/apps/sim/lib/internal/table/execute-tool.ts @@ -16,7 +16,6 @@ import { upsertTableRowContract, } from '@/lib/api/contracts/tables' import { type InternalErrorPolicy, internalOrchestrationErrorPolicy } from '@/lib/api/server/routes' -import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' import { executeTableCreate, @@ -35,6 +34,11 @@ import { type TableToolOperationResult, } from '@/lib/internal/table/operations' import { createTableToolResponse } from '@/lib/internal/table/provenance' +import { + classifyInternalToolIdentityFault, + internalToolIdentityFaultMessage, + internalToolIdentityFaultStatus, +} from '@/lib/internal/tool-operations/identity-faults' import { parseInternalContractInput } from '@/lib/internal/tool-operations/parse-contract-input' import type { InternalToolOperationHandler } from '@/lib/internal/tool-operations/types' import { internalTableErrorPolicies } from '@/lib/table/api/route-policies' @@ -292,11 +296,12 @@ export const executeTableTool: InternalToolOperationHandler = async (request) => return createTableToolResponse(validatedBody, result.result.provenance) } catch (error) { request.signal?.throwIfAborted() - if ( - error instanceof InvalidInternalDelegationBindingError || - (error instanceof Error && error.message === 'Authentication required') - ) { - return Response.json({ error: 'Authentication required' }, { status: 401 }) + const identityFault = classifyInternalToolIdentityFault(error) + if (identityFault) { + return Response.json( + { error: internalToolIdentityFaultMessage(identityFault) }, + { status: internalToolIdentityFaultStatus(identityFault) } + ) } return errorResponse( request.toolId, diff --git a/apps/sim/lib/internal/tool-operations/identity-faults.ts b/apps/sim/lib/internal/tool-operations/identity-faults.ts new file mode 100644 index 00000000000..80a03624a7b --- /dev/null +++ b/apps/sim/lib/internal/tool-operations/identity-faults.ts @@ -0,0 +1,70 @@ +import { PrincipalSubjectUserRequiredError } from '@sim/auth/principal' +import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' + +/** + * Identity failures that every in-process tool handler answers the same way. + * + * `unauthenticated` is a caller that never established a runtime identity — a + * missing or unbindable executor delegation. + * + * `subject_user_required` is a caller that IS authenticated but has no human + * subject to act as, which is the normal shape of an actorless run: a schedule, + * or a webhook carrying no external subject. It is distinct from + * `unauthenticated` because retrying, re-authenticating, or fixing credentials + * cannot resolve it — the operation is simply not available to that trigger, and + * an operator reading the log needs to be told so rather than shown a generic + * failure. The Logs detail tools returned an opaque 500 for exactly this for one + * evening, which is why the classification lives here rather than per handler. + */ +export type InternalToolIdentityFault = 'unauthenticated' | 'subject_user_required' + +/** + * An in-process tool ran without the execution context that proves who called it. + * + * Typed rather than a bare `Error` so this boundary can answer it as the + * unauthenticated failure it is; left untyped it fell past every classifier into a + * generic 500, which reads as "the tool broke" rather than "this caller never + * established an identity". + * + * It lives here rather than beside its thrower because the classifier must not + * import the executor-principal module: nearly every handler test mocks that + * module, and an `instanceof` against a mock that omits the export throws. + */ +export class ExecutorDelegationOriginRequiredError extends Error { + constructor() { + super('Executor delegation origin is required') + this.name = 'ExecutorDelegationOriginRequiredError' + } +} + +const FAULTS: Record = { + unauthenticated: { status: 401, message: 'Authentication required' }, + subject_user_required: { + status: 403, + message: + 'This tool requires a user identity, and this run has none — scheduled and webhook triggers run without a user', + }, +} + +/** Classifies an identity fault, or returns `undefined` for any other failure. */ +export function classifyInternalToolIdentityFault( + error: unknown +): InternalToolIdentityFault | undefined { + if (error instanceof PrincipalSubjectUserRequiredError) return 'subject_user_required' + if ( + error instanceof InvalidInternalDelegationBindingError || + error instanceof ExecutorDelegationOriginRequiredError || + (error instanceof Error && error.message === 'Authentication required') + ) { + return 'unauthenticated' + } + return undefined +} + +export function internalToolIdentityFaultStatus(fault: InternalToolIdentityFault): number { + return FAULTS[fault].status +} + +export function internalToolIdentityFaultMessage(fault: InternalToolIdentityFault): string { + return FAULTS[fault].message +} diff --git a/apps/sim/lib/internal/windchill/execute-tool.ts b/apps/sim/lib/internal/windchill/execute-tool.ts index 689e140e6ad..35a0141ed5f 100644 --- a/apps/sim/lib/internal/windchill/execute-tool.ts +++ b/apps/sim/lib/internal/windchill/execute-tool.ts @@ -7,9 +7,13 @@ import type { import { windchillOperationBodySchema } from '@/lib/api/contracts/tools/windchill' import { getValidationErrorMessage } from '@/lib/api/server' import { DEFAULT_MAX_JSON_BODY_BYTES } from '@/lib/api/server/validation' -import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation' import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' +import { + classifyInternalToolIdentityFault, + internalToolIdentityFaultMessage, + internalToolIdentityFaultStatus, +} from '@/lib/internal/tool-operations/identity-faults' import type { InternalToolOperationHandler } from '@/lib/internal/tool-operations/types' import { WindchillProviderError } from '@/lib/internal/windchill/client' import { WindchillOperationError } from '@/lib/internal/windchill/errors' @@ -90,11 +94,12 @@ export const executeWindchillTool: InternalToolOperationHandler = async (request return Response.json({ success: true, output } satisfies WindchillOperationResponse) } catch (error) { signal?.throwIfAborted() - if ( - error instanceof InvalidInternalDelegationBindingError || - (error instanceof Error && error.message === 'Authentication required') - ) { - return failureResponse('Authentication required', 401) + const identityFault = classifyInternalToolIdentityFault(error) + if (identityFault) { + return failureResponse( + internalToolIdentityFaultMessage(identityFault), + internalToolIdentityFaultStatus(identityFault) + ) } logger.error(`[${requestId}] Windchill operation failed`, { operation: toolId, diff --git a/apps/sim/lib/internal/windchill/operations.ts b/apps/sim/lib/internal/windchill/operations.ts index 5bd72c664d1..f8bc8c30e2f 100644 --- a/apps/sim/lib/internal/windchill/operations.ts +++ b/apps/sim/lib/internal/windchill/operations.ts @@ -482,6 +482,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) ) signal?.throwIfAborted() @@ -491,6 +492,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), }) signal?.throwIfAborted() @@ -575,6 +577,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), requestId, signal diff --git a/apps/sim/lib/knowledge/api/internal-route.ts b/apps/sim/lib/knowledge/api/internal-route.ts index 68fa01cd54f..5608a476923 100644 --- a/apps/sim/lib/knowledge/api/internal-route.ts +++ b/apps/sim/lib/knowledge/api/internal-route.ts @@ -32,6 +32,7 @@ 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) } 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 bf1e3659c38..6deb81fe26c 100644 --- a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts +++ b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts @@ -126,6 +126,7 @@ export function defineAuthorizedKnowledgeUseCase< 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 ) { 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 e7ca3425751..ccf4af4a8f0 100644 --- a/apps/sim/lib/knowledge/application/billing.ts +++ b/apps/sim/lib/knowledge/application/billing.ts @@ -20,6 +20,7 @@ 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) return resolvePrincipalAttribution(principal, { workspaceBillingOwnerUserId: context.billedAccountUserId, diff --git a/apps/sim/lib/logs/application/read-execution-snapshot.ts b/apps/sim/lib/logs/application/read-execution-snapshot.ts index e5b645710c0..c44f803852a 100644 --- a/apps/sim/lib/logs/application/read-execution-snapshot.ts +++ b/apps/sim/lib/logs/application/read-execution-snapshot.ts @@ -1,4 +1,4 @@ -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' +import { resolvePrincipalSubjectUserId } from '@sim/auth/principal' import { db } from '@sim/db' import { jobExecutionLogs, workflowExecutionLogs, workflowExecutionSnapshots } from '@sim/db/schema' import { eq, inArray } from 'drizzle-orm' @@ -168,8 +168,10 @@ const authorizedReadExecutionSnapshotUseCase = defineAuthorizedWorkspaceUseCase( )) as WorkflowExecutionLog['executionData'] const traceSpans = (executionData?.traceSpans as TraceSpan[]) || [] if (traceSpans.length > 0) { + // Attribution, not authorization: the publisher's policy is the only gate, + // and an actorless run has no user to name. await hydrateChildTraces(traceSpans, { - viewerUserId: requirePrincipalSubjectUserId(principal), + viewerUserId: resolvePrincipalSubjectUserId(principal), }) } diff --git a/apps/sim/lib/logs/application/read-log-detail.test.ts b/apps/sim/lib/logs/application/read-log-detail.test.ts new file mode 100644 index 00000000000..ee291213706 --- /dev/null +++ b/apps/sim/lib/logs/application/read-log-detail.test.ts @@ -0,0 +1,126 @@ +/** + * @vitest-environment node + */ + +import type { Principal } from '@sim/auth/principal' +import { workflowExecutionLogs } from '@sim/db/schema' +import { queueTableRows, resetDbChainMock } from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + readLogDetail: vi.fn(), + resolveWorkspace: vi.fn(), + resolvePermission: vi.fn(), +})) + +vi.mock('@/lib/logs/fetch-log-detail', () => ({ + readLogDetail: mocks.readLogDetail, +})) + +vi.mock('@/lib/workspaces/application/workspace-context', () => ({ + resolveActiveWorkspaceApplicationContext: mocks.resolveWorkspace, +})) + +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: (held: string | null, required: string) => + held === 'admin' || held === required || (held === 'write' && required === 'read'), + resolveEffectiveWorkspacePermission: mocks.resolvePermission, +})) + +import { readLogDetailUseCase } from '@/lib/logs/application/read-log-detail' + +const WORKSPACE_ID = 'workspace-1' +const EXECUTION_ID = 'execution-1' + +/** + * What a scheduled run actually holds: a delegation whose workflow principal is the + * actorless `system:schedule`, so `subjectUserId` is absent. Its workspace reach comes + * from running a deployment, which is the branch `workspace-authorization.ts` admits + * without a subject — so this exercises the real authorization path, not a stub. + */ +const SCHEDULED_PRINCIPAL: Principal = { + kind: 'delegated', + serviceId: 'executor', + workspaceId: WORKSPACE_ID, + delegationId: 'delegation-1', + audience: 'sim:logs', + issuedAt: new Date(Date.now() - 1_000), + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + delegationContext: { + kind: 'workflow_execution', + workflowId: 'workflow-1', + principal: { + kind: 'system', + serviceId: 'schedule', + workspaceId: WORKSPACE_ID, + workflowId: 'workflow-1', + }, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'version-1', + }, + }, +} + +const HUMAN_PRINCIPAL: Principal = { + ...SCHEDULED_PRINCIPAL, + subjectUserId: 'user-1', + delegationContext: { + kind: 'workflow_execution', + workflowId: 'workflow-1', + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'version-1', + }, + }, +} + +function queueLogRow(): void { + queueTableRows(workflowExecutionLogs, [{ workspaceId: WORKSPACE_ID, executionId: EXECUTION_ID }]) +} + +describe('readLogDetailUseCase', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mocks.resolveWorkspace.mockResolvedValue({ + workspaceId: WORKSPACE_ID, + workspaceOrganizationId: null, + allowPersonalApiKeys: true, + }) + mocks.readLogDetail.mockResolvedValue({ id: 'log-1', executionId: EXECUTION_ID }) + mocks.resolvePermission.mockResolvedValue('admin') + }) + + afterAll(resetDbChainMock) + + it('reads a run for an actorless schedule, passing no viewer', async () => { + queueLogRow() + + const result = await readLogDetailUseCase.execute({ + principal: SCHEDULED_PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, lookupColumn: 'executionId', lookupValue: EXECUTION_ID }, + }) + + expect(result.detail).toMatchObject({ id: 'log-1' }) + expect(mocks.readLogDetail).toHaveBeenCalledWith( + expect.objectContaining({ workspaceId: WORKSPACE_ID, viewerUserId: undefined }) + ) + }) + + it('still names the human behind a run that has one', async () => { + queueLogRow() + + await readLogDetailUseCase.execute({ + principal: HUMAN_PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, lookupColumn: 'executionId', lookupValue: EXECUTION_ID }, + }) + + expect(mocks.readLogDetail).toHaveBeenCalledWith( + expect.objectContaining({ viewerUserId: 'user-1' }) + ) + }) +}) diff --git a/apps/sim/lib/logs/application/read-log-detail.ts b/apps/sim/lib/logs/application/read-log-detail.ts index 3af63a0dc53..326a3047ee1 100644 --- a/apps/sim/lib/logs/application/read-log-detail.ts +++ b/apps/sim/lib/logs/application/read-log-detail.ts @@ -1,4 +1,4 @@ -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' +import { resolvePrincipalSubjectUserId } from '@sim/auth/principal' import { db } from '@sim/db' import { jobExecutionLogs, workflowExecutionLogs } from '@sim/db/schema' import { eq } from 'drizzle-orm' @@ -77,8 +77,10 @@ const authorizedReadLogDetailUseCase = defineAuthorizedWorkspaceUseCase({ authorizationOptions: logDelegationAuthorization(), async execute({ principal, input, context }) { input.signal?.throwIfAborted() + // Attribution, not authorization: an actorless run (a schedule, or a webhook + // with no external subject) reads its own workspace's logs with no user to name. const detail = await readLogDetail({ - viewerUserId: requirePrincipalSubjectUserId(principal), + viewerUserId: resolvePrincipalSubjectUserId(principal), workspaceId: context.workspaceId, lookupColumn: input.lookupColumn, lookupValue: input.lookupValue, diff --git a/apps/sim/lib/logs/execution/hydrate-child-traces.test.ts b/apps/sim/lib/logs/execution/hydrate-child-traces.test.ts index 627e36de1b9..0af90346a48 100644 --- a/apps/sim/lib/logs/execution/hydrate-child-traces.test.ts +++ b/apps/sim/lib/logs/execution/hydrate-child-traces.test.ts @@ -134,6 +134,35 @@ describe('hydrateChildTraces', () => { expect(mockCheckWorkspaceAccess).not.toHaveBeenCalled() }) + it('joins the same run for an actorless read, which carries no reader at all', async () => { + // A scheduled run inspecting its own child has no user on its principal. The + // viewer is attribution, never a gate, so its absence must change nothing about + // what is joined — and must not throw, which is how this broke in production. + mockSelect.mockResolvedValue([ + { + executionId: 'child-exec-1', + workspaceId: 'ws-source', + workflowId: 'wf-source', + stateSnapshotId: 'snap-1', + executionData: {}, + }, + ]) + const spans = [boundarySpan('child-exec-1')] + + const result = await hydrateChildTraces(spans, {}) + + expect(result.hydrated).toBe(1) + expect(spans[0].childTraceAccess).toBe('granted') + expect(spans[0].children?.[0].name).toBe('Agent 1') + expect(spans[0].childWorkflowSnapshotId).toBe('snap-1') + // Pinned rather than left implicit: materialization is told there is no owner, + // instead of being handed a stand-in the run never authorized. + expect(mockMaterialize).toHaveBeenCalledWith( + {}, + expect.objectContaining({ workspaceId: 'ws-source', userId: undefined }) + ) + }) + it('refuses a handle whose block is not opted in, however it got there', async () => { // The decisive case: handles persisted before this policy existed meant "a child // ran, authorize the reader", not "the publisher consented". Reading presence as diff --git a/apps/sim/lib/logs/execution/hydrate-child-traces.ts b/apps/sim/lib/logs/execution/hydrate-child-traces.ts index ae815f88f70..cb4281eb92e 100644 --- a/apps/sim/lib/logs/execution/hydrate-child-traces.ts +++ b/apps/sim/lib/logs/execution/hydrate-child-traces.ts @@ -37,11 +37,13 @@ export interface ChildTraceDropCounts { export interface HydrateChildTracesOptions { /** - * The user reading the log. NOT an authorization input — the only policy is the - * publisher's, and it is the same answer for every reader. Carried so large-value - * materialization and secret projection have an owner to attribute their reads to. + * The user reading the log, when there is one. NOT an authorization input — the + * only policy is the publisher's, and it is the same answer for every reader. + * Carried so large-value materialization and secret projection have an owner to + * attribute their reads to; an actorless run has none, and needs none, because + * the display path only ever reads. */ - viewerUserId: string + viewerUserId?: string maxDepth?: number maxRows?: number } diff --git a/apps/sim/lib/logs/fetch-log-detail.test.ts b/apps/sim/lib/logs/fetch-log-detail.test.ts index 259eb9b8fa2..326032257bc 100644 --- a/apps/sim/lib/logs/fetch-log-detail.test.ts +++ b/apps/sim/lib/logs/fetch-log-detail.test.ts @@ -8,12 +8,17 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ materializeExecutionData: vi.fn(), + hydrateChildTraces: vi.fn(), })) vi.mock('@/lib/logs/execution/trace-store', () => ({ materializeExecutionDataForDisplay: mocks.materializeExecutionData, })) +vi.mock('@/lib/logs/execution/hydrate-child-traces', () => ({ + hydrateChildTraces: mocks.hydrateChildTraces, +})) + vi.mock('@/lib/logs/execution-origin', () => ({ workflowExecutionOriginSql: () => ({ as: () => ({}) }), })) @@ -25,6 +30,7 @@ describe('readLogDetail', () => { vi.clearAllMocks() resetDbChainMock() mocks.materializeExecutionData.mockResolvedValue({}) + mocks.hydrateChildTraces.mockResolvedValue({ hydrated: 0, dropped: {} }) }) afterAll(resetDbChainMock) @@ -80,4 +86,72 @@ describe('readLogDetail', () => { expect(joinedTables).not.toContain(workflowExecutionSnapshots) expect(joinedTables).not.toContain(user) }) + + it('reads a log for an actorless run, which has no viewer to attribute to', async () => { + // A scheduled run inspecting its own execution has no user on its principal. + // Attribution is the only thing the viewer feeds on this path, so its absence + // must return the same detail rather than throwing, which is how the Logs tools + // started answering every scheduled run with an opaque 500. + queueTableRows(workflowExecutionLogs, [ + { + id: 'log-1', + workflowId: 'workflow-1', + executionId: 'execution-1', + deploymentVersionId: null, + deploymentVersion: null, + deploymentVersionName: null, + level: 'info', + status: 'completed', + trigger: 'manual', + startedAt: new Date('2026-01-01T00:00:00.000Z'), + endedAt: new Date('2026-01-01T00:00:01.000Z'), + totalDurationMs: 1000, + executionData: {}, + costTotal: null, + files: null, + createdAt: new Date('2026-01-01T00:00:00.000Z'), + workflowName: 'Workflow', + workflowDescription: null, + workflowFolderId: null, + workflowUserId: 'user-1', + workflowWorkspaceId: 'workspace-1', + workflowCreatedAt: new Date('2026-01-01T00:00:00.000Z'), + workflowUpdatedAt: new Date('2026-01-01T00:00:00.000Z'), + pausedStatus: null, + pausedTotalPauseCount: 0, + pausedResumedCount: 0, + executionOrigin: null, + }, + ]) + queueTableRows(usageLog, []) + mocks.materializeExecutionData.mockResolvedValue({ + traceSpans: [ + { + id: 'span-1', + name: 'Agent 1', + type: 'agent', + duration: 5, + startTime: '2026-01-01T00:00:00.000Z', + endTime: '2026-01-01T00:00:00.005Z', + }, + ], + }) + + const result = await readLogDetail({ + workspaceId: 'workspace-1', + lookupColumn: 'id', + lookupValue: 'log-1', + }) + + expect(result).toMatchObject({ id: 'log-1', executionId: 'execution-1' }) + // Pinned explicitly: both consumers are told there is no owner, rather than + // being handed a stand-in the run never authorized. + expect(mocks.materializeExecutionData).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ workspaceId: 'workspace-1', userId: undefined }) + ) + expect(mocks.hydrateChildTraces).toHaveBeenCalledWith(expect.any(Array), { + viewerUserId: undefined, + }) + }) }) diff --git a/apps/sim/lib/logs/fetch-log-detail.ts b/apps/sim/lib/logs/fetch-log-detail.ts index ef5423b94a9..41a89227a12 100644 --- a/apps/sim/lib/logs/fetch-log-detail.ts +++ b/apps/sim/lib/logs/fetch-log-detail.ts @@ -29,7 +29,13 @@ export function jobCostTotal(raw: unknown): { total: number } | null { } interface FetchLogDetailArgs { - viewerUserId: string + /** + * The user reading the log, when there is one. Attribution only — workspace + * authorization already happened upstream, and the display path never writes. + * An actorless run (a schedule, or a webhook with no external subject) has no + * user to name and passes none. + */ + viewerUserId?: string workspaceId: string lookupColumn: LookupColumn lookupValue: string diff --git a/apps/sim/lib/mcp/application/authorization.ts b/apps/sim/lib/mcp/application/authorization.ts index 3e854da883b..7bd10b74492 100644 --- a/apps/sim/lib/mcp/application/authorization.ts +++ b/apps/sim/lib/mcp/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 MCP_SERVER_DELEGATION_AUDIENCE = 'sim:mcp-servers' @@ -10,3 +12,44 @@ export const mcpServerDelegationPolicy = { workspaceOrganizationId: string | null allowPersonalApiKeys: boolean }> + +/** + * The user whose MCP server credentials an operation presents. + * + * An MCP call connects to a third-party server with one person's stored + * 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 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 + * credentials of its own, and those runs have always connected as the actor. + * 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. + */ +export function requireMcpCredentialUserId( + principal: Principal, + executionActorUserId?: string +): string { + const userId = resolvePrincipalSubjectUserId(principal) ?? executionActorUserId + if (!userId) { + throw new OrchestrationError( + 'forbidden', + 'MCP servers are reached with a user\u2019s own credentials, and this run has none' + ) + } + return userId +} diff --git a/apps/sim/lib/mcp/application/execute-tool.test.ts b/apps/sim/lib/mcp/application/execute-tool.test.ts index c775680d98d..b0711c22960 100644 --- a/apps/sim/lib/mcp/application/execute-tool.test.ts +++ b/apps/sim/lib/mcp/application/execute-tool.test.ts @@ -1,10 +1,7 @@ /** * @vitest-environment node */ -import { - PrincipalSubjectUserRequiredError, - type WorkflowExecutionDelegatedPrincipal, -} from '@sim/auth/principal' +import type { WorkflowExecutionDelegatedPrincipal } from '@sim/auth/principal' import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ @@ -171,7 +168,89 @@ describe('executeMcpToolUseCase', () => { expect(mocks.executeTool).not.toHaveBeenCalled() }) - it('does not invent a user for actorless system execution', async () => { + it('keeps an unattended run connecting as its execution actor', async () => { + // Pre-in-process behavior: the executor minted an internal token from + // 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, + input: { + workspaceId: WORKSPACE.workspaceId, + serverId: SERVER.id, + toolName: 'lookup', + arguments: { count: '2', enabled: 'true', tags: 'a,b' }, + executionActorUserId: 'execution-actor', + }, + }) + + expect(mocks.assertPermissionsAllowed).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'execution-actor' }) + ) + 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. + await executeMcpToolUseCase.execute({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE.workspaceId, + serverId: SERVER.id, + toolName: 'lookup', + arguments: { count: '2', enabled: 'true', tags: 'a,b' }, + executionActorUserId: 'someone-else', + }, + }) + + expect(mocks.assertPermissionsAllowed).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'user-1' }) + ) + }) + + it('keeps an external-subject webhook connecting as the execution actor', async () => { + // A webhook's external_user subject is a real identity but never a Sim user, so + // it has no Sim credentials of its own and these runs have always connected as + // 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, + delegationContext: { + ...ACTORLESS_PRINCIPAL.delegationContext, + principal: { + kind: 'system' as const, + serviceId: 'webhook' as const, + workspaceId: WORKSPACE.workspaceId, + workflowId: 'workflow-1', + webhookId: 'webhook-1', + provider: 'slack', + subject: { + kind: 'external_user' as const, + provider: 'slack', + tenantId: 'T1', + subjectId: 'U1', + }, + }, + }, + } + + await executeMcpToolUseCase.execute({ + principal: externalSubjectPrincipal, + input: { + workspaceId: WORKSPACE.workspaceId, + serverId: SERVER.id, + toolName: 'lookup', + arguments: { count: '2', enabled: 'true', tags: 'a,b' }, + executionActorUserId: 'execution-actor', + }, + }) + + expect(mocks.assertPermissionsAllowed).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'execution-actor' }) + ) + }) + + it('refuses when the run names no user and carries no actor either', async () => { await expect( executeMcpToolUseCase.execute({ principal: ACTORLESS_PRINCIPAL, @@ -181,7 +260,10 @@ describe('executeMcpToolUseCase', () => { toolName: 'lookup', }, }) - ).rejects.toEqual(new PrincipalSubjectUserRequiredError('delegated')) + ).rejects.toMatchObject({ + code: 'forbidden', + message: 'MCP servers are reached with a user\u2019s own credentials, and this run has none', + }) expect(mocks.assertPermissionsAllowed).not.toHaveBeenCalled() expect(mocks.discoverServerTools).not.toHaveBeenCalled() diff --git a/apps/sim/lib/mcp/application/execute-tool.ts b/apps/sim/lib/mcp/application/execute-tool.ts index be542e71f0d..8f71be4582d 100644 --- a/apps/sim/lib/mcp/application/execute-tool.ts +++ b/apps/sim/lib/mcp/application/execute-tool.ts @@ -1,10 +1,12 @@ -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { SIM_VIA_HEADER, serializeCallChain } from '@/lib/execution/call-chain' -import { mcpServerDelegationPolicy } from '@/lib/mcp/application/authorization' +import { + mcpServerDelegationPolicy, + requireMcpCredentialUserId, +} from '@/lib/mcp/application/authorization' import { resolveMcpServerContext } from '@/lib/mcp/application/context' import { mcpServerOperations } from '@/lib/mcp/application/operations' import { mcpService } from '@/lib/mcp/service' @@ -24,6 +26,11 @@ 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[] @@ -132,7 +139,7 @@ export const executeMcpToolUseCase = defineAuthorizedWorkspaceUseCase({ authorizationOptions: { delegation: mcpServerDelegationPolicy }, async execute({ principal, input, context }): Promise { input.signal?.throwIfAborted() - const userId = requirePrincipalSubjectUserId(principal) + const userId = requireMcpCredentialUserId(principal, input.executionActorUserId) 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 e35a74c6606..18085334775 100644 --- a/apps/sim/lib/mcp/application/use-cases.ts +++ b/apps/sim/lib/mcp/application/use-cases.ts @@ -1,11 +1,14 @@ import { AuditAction, AuditResourceType } from '@sim/audit' -import { requirePrincipalSubjectUserId, resolvePrincipalAttribution } from '@sim/auth/principal' +import { resolvePrincipalAttribution } from '@sim/auth/principal' import { getPostgresErrorCode } from '@sim/utils/errors' import type { CursorKey, ListSortOrder } from '@/lib/api/list-query' import { defineAuthorizedWorkspaceUseCase, ForbiddenOperationError } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { sanitizeUrlForLog } from '@/lib/core/utils/logging' -import { mcpServerDelegationPolicy } from '@/lib/mcp/application/authorization' +import { + mcpServerDelegationPolicy, + requireMcpCredentialUserId, +} from '@/lib/mcp/application/authorization' import { type McpServerContext, type McpWorkspaceContext, @@ -88,6 +91,11 @@ 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 } @@ -98,7 +106,7 @@ export const discoverMcpToolsUseCase = defineAuthorizedWorkspaceUseCase({ authorizationOptions, async execute({ principal, input, context }) { const tools = await mcpService.discoverTools( - requirePrincipalSubjectUserId(principal), + requireMcpCredentialUserId(principal, input.executionActorUserId), context.workspaceId, /** * A public `refresh` skips the positive cache but keeps the failure @@ -114,6 +122,11 @@ 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 } @@ -151,7 +164,7 @@ export const discoverMcpServerToolsUseCase = defineAuthorizedWorkspaceUseCase({ } const tools = await mcpService.discoverServerTools( - requirePrincipalSubjectUserId(principal), + requireMcpCredentialUserId(principal, input.executionActorUserId), context.server.id, context.workspaceId, /** diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index 7746b60db91..4ca671cdc4b 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -2,8 +2,8 @@ import { isDeepStrictEqual } from 'node:util' import { AuditAction, AuditResourceType } from '@sim/audit' import { type Principal, - requirePrincipalSubjectUserId, resolvePrincipalAttribution, + resolvePrincipalSubjectUserId, } from '@sim/auth/principal' import { db } from '@sim/db' import { getRequestContext } from '@sim/logger' @@ -496,7 +496,10 @@ export const queryTableRows = defineAuthorizedTableUseCase({ const orgId = await getWorkspaceOrganizationId(context.workspaceId) if ( !(await isFeatureEnabled('tables-v2-api', { - userId: requirePrincipalSubjectUserId(principal), + // An actorless run has no user to match a per-user rule against, and a + // missing one resolves the admin clause to `false` without a query — so + // the gate only ever narrows here, never widens. + userId: resolvePrincipalSubjectUserId(principal), orgId, })) ) { diff --git a/apps/sim/lib/workflows/application/deployments.ts b/apps/sim/lib/workflows/application/deployments.ts index 067f847d0bf..dfc5eadc1d3 100644 --- a/apps/sim/lib/workflows/application/deployments.ts +++ b/apps/sim/lib/workflows/application/deployments.ts @@ -243,6 +243,7 @@ export const revertWorkflowVersion = defineAuthorizedWorkflowUseCase({ operation: workflowOperations.revertVersion, resolveContext: resolveWorkflowContext, 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) await requireMutableWorkflow(context.workflowId) const result = await performRevertToVersion({ 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 e28789a8846..247af5d1198 100644 --- a/apps/sim/lib/workspace-files/application/share-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/share-workspace-file.ts @@ -72,6 +72,7 @@ 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 existingShare = await getShareForResource('file', context.fileId) diff --git a/package.json b/package.json index 35e5619bd11..115b8854347 100644 --- a/package.json +++ b/package.json @@ -14,12 +14,13 @@ "dev:sockets": "cd apps/realtime && bun run dev", "dev:full": "bunx concurrently -n \"App,Realtime\" -c \"cyan,magenta\" \"cd apps/sim && bun run dev\" \"cd apps/realtime && bun run dev\"", "dev:full:capped": "bunx concurrently -n \"App,Realtime\" -c \"cyan,magenta\" \"cd apps/sim && bun run dev:capped\" \"cd apps/realtime && bun run dev\"", - "test": "bun run test:setup && bun run test:npm-package-versions && bun run test:icon-path-precision && bun run test:tool-registry-boundary && bun run test:tool-request-boundary && bun run test:migrations-safety && bun run test:generators && turbo run test", + "test": "bun run test:setup && bun run test:npm-package-versions && bun run test:icon-path-precision && bun run test:tool-registry-boundary && bun run test:tool-request-boundary && bun run test:actorless-executor-operations && bun run test:migrations-safety && bun run test:generators && turbo run test", "test:setup": "bun run --cwd packages/sim-setup test", "test:npm-package-versions": "bunx vitest run scripts/bump-npm-package-versions.test.ts", "test:icon-path-precision": "bunx vitest run scripts/check-icon-path-precision.test.ts", "test:tool-registry-boundary": "bunx vitest run scripts/check-tool-registry-boundary.test.ts", "test:tool-request-boundary": "bunx vitest run scripts/check-tool-request-boundary.test.ts", + "test:actorless-executor-operations": "bunx vitest run scripts/check-actorless-executor-operations.test.ts", "test:migrations-safety": "bunx vitest run scripts/check-migrations-safety.test.ts", "test:generators": "bunx vitest run scripts/generate-v2-cli-api.test.ts scripts/generate-cli-docs.test.ts scripts/generate-docs.test.ts", "format": "turbo run format", @@ -44,6 +45,7 @@ "check:api-validation:strict": "bun run scripts/check-api-validation-contracts.ts --check --enforce-boundary-baseline", "check:realtime-prune": "bun run scripts/check-realtime-prune-graph.ts", "check:tool-request-boundary": "bun run scripts/check-tool-request-boundary.ts", + "check:actorless-executor-operations": "bun run scripts/check-actorless-executor-operations.ts", "check:tool-registry-boundary": "bun run scripts/check-tool-registry-boundary.ts --check", "check:trigger-block-cycle": "bun run scripts/check-trigger-block-cycle.ts", "check:import-specifiers": "bun run scripts/check-import-specifiers.ts", diff --git a/packages/auth/src/principal.ts b/packages/auth/src/principal.ts index 0b435434183..3336e5bb624 100644 --- a/packages/auth/src/principal.ts +++ b/packages/auth/src/principal.ts @@ -115,10 +115,25 @@ export class PrincipalSubjectUserRequiredError extends Error { } } +/** + * The Sim user a principal represents, or `undefined` when it represents none. + * + * Actorless callers are ordinary, not exceptional: a scheduled or webhook run, a + * workspace API key, and a Credential Group enrollment all act with real authority + * and no human behind them. Use this wherever the user is attribution — a name to + * record a read or write under — and {@link requirePrincipalSubjectUserId} only + * where the operation's meaning genuinely collapses without one, so that choice is + * visible at the call site instead of hidden in a ternary. + */ +export function resolvePrincipalSubjectUserId(principal: Principal): string | undefined { + const subject = resolvePrincipalSubject(principal) + return subject?.kind === 'sim_user' ? subject.userId : undefined +} + /** Resolves the real human subject represented by a principal or fails fast. */ export function requirePrincipalSubjectUserId(principal: Principal): string { - const subject = resolvePrincipalSubject(principal) - if (subject?.kind === 'sim_user') return subject.userId + const userId = resolvePrincipalSubjectUserId(principal) + if (userId !== undefined) return userId throw new PrincipalSubjectUserRequiredError(principal.kind) } diff --git a/scripts/check-actorless-executor-operations.test.ts b/scripts/check-actorless-executor-operations.test.ts new file mode 100644 index 00000000000..778efbf612b --- /dev/null +++ b/scripts/check-actorless-executor-operations.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from 'vitest' +import { + auditSubjectRequirements, + parseOperationPolicies, + referencedOperations, +} from './check-actorless-executor-operations' + +describe('operation policy parsing', () => { + it('reads an inline delegatedServices list', () => { + const policies = parseOperationPolicies(` + export const logOperations = { + list: defineWorkspaceOperation({ + id: 'logs.list', + delegatedServices: ['copilot', 'executor'], + }), + readStats: defineWorkspaceOperation({ + id: 'logs.read_stats', + delegatedServices: ['copilot'], + }), + } as const + `) + + expect(policies.get('logOperations.list')).toBe(true) + expect(policies.get('logOperations.readStats')).toBe(false) + }) + + it('resolves a policy spread into the definition', () => { + const policies = parseOperationPolicies(` + const READER_POLICY = { + principalKinds: ['session', 'delegated'], + delegatedServices: ['copilot', 'executor'], + } as const + export const logOperations = { + readDetail: defineWorkspaceOperation({ id: 'logs.read_detail', ...READER_POLICY }), + } as const + `) + + expect(policies.get('logOperations.readDetail')).toBe(true) + }) + + it('resolves an operation declared through a same-file factory', () => { + const policies = parseOperationPolicies(` + const TOOL_POLICY = { delegatedServices: ['copilot', 'executor'] } as const + const UI_POLICY = { delegatedServices: ['copilot'] } as const + function toolReadOperation(id: Id) { + return defineWorkspaceOperation({ id, minimumRole: 'read', ...TOOL_POLICY }) + } + function readOperation(id: Id) { + return defineWorkspaceOperation({ id, minimumRole: 'read', ...UI_POLICY }) + } + export const tableOperations = { + queryRows: toolReadOperation('tables.rows.query'), + listTables: readOperation('tables.list'), + } as const + `) + + expect(policies.get('tableOperations.queryRows')).toBe(true) + expect(policies.get('tableOperations.listTables')).toBe(false) + }) + + it('treats an operation with no delegated services as executor-free', () => { + const policies = parseOperationPolicies(` + export const workspaceOperations = { + read: defineWorkspaceOperation({ id: 'workspaces.read', minimumRole: 'read' }), + } as const + `) + + expect(policies.get('workspaceOperations.read')).toBe(false) + }) +}) + +describe('operation references', () => { + it('collects only declared operations', () => { + const referenced = referencedOperations( + ` + const useCase = defineAuthorizedWorkspaceUseCase({ + operation: logOperations.readDetail, + execute: () => input.signal?.throwIfAborted(), + }) + `, + new Set(['logOperations.readDetail', 'logOperations.list']) + ) + + expect(referenced).toEqual(['logOperations.readDetail']) + }) +}) + +describe('subject requirement audit', () => { + const call = ' const userId = requirePrincipalSubjectUserId(principal)' + + it('flags an unannotated call', () => { + const findings = auditSubjectRequirements(`function run() {\n${call}\n}`, ['ops.thing']) + + expect(findings).toEqual([ + { file: '', line: 2, reason: 'unannotated', operations: ['ops.thing'] }, + ]) + }) + + it('accepts an annotated call', () => { + const source = `function run() {\n // actorless-unsupported: skills belong to a person\n${call}\n}` + + expect(auditSubjectRequirements(source, [])).toEqual([]) + }) + + it('tolerates context comments above the annotation', () => { + const source = [ + 'function run() {', + ' // The library is per-user.', + ' // actorless-unsupported: skills belong to a person', + call, + '}', + ].join('\n') + + expect(auditSubjectRequirements(source, [])).toEqual([]) + }) + + it('rejects an annotation with no reason', () => { + const source = `function run() {\n // actorless-unsupported:\n${call}\n}` + + expect(auditSubjectRequirements(source, [])).toEqual([ + { file: '', line: 3, reason: 'empty-reason', operations: [] }, + ]) + }) + + it('ignores an annotation separated from the call by code', () => { + const source = [ + 'function run() {', + ' // actorless-unsupported: not attached to the call below', + ' const workspaceId = context.workspaceId', + call, + '}', + ].join('\n') + + expect(auditSubjectRequirements(source, [])).toEqual([ + { file: '', line: 4, reason: 'unannotated', operations: [] }, + ]) + }) +}) diff --git a/scripts/check-actorless-executor-operations.ts b/scripts/check-actorless-executor-operations.ts new file mode 100644 index 00000000000..91a49b46f8d --- /dev/null +++ b/scripts/check-actorless-executor-operations.ts @@ -0,0 +1,275 @@ +#!/usr/bin/env bun +/** + * Fails when an operation an actorless run can reach demands a human subject. + * + * A workflow executes under a `Principal`, and several triggers have no person + * behind them: a schedule, the public API, a webhook carrying no external subject. + * Those runs still hold real authority — `workspace-authorization.ts` admits an + * actorless executor delegation whose current workflow is a deployment — so they + * are authorized callers with no `subjectUserId`. Any use case they can reach that + * calls `requirePrincipalSubjectUserId` therefore throws, and because + * `PrincipalSubjectUserRequiredError` is not an `OrchestrationError` it surfaces as + * an opaque 500 rather than anything a workflow author can act on. + * + * That is not hypothetical: the Logs detail tools broke for every scheduled run + * this way, and the failure reached production because nothing connected "this + * operation admits `delegatedServices: ['executor']`" to "this use case requires a + * person". This audit connects them. + * + * A subject is genuinely required often enough that the rule is an annotation, not + * a ban. Write `// actorless-unsupported: ` above the call to declare that + * the operation has no meaning without a person — the annotation turns a silent + * 500 into a documented gap that a reviewer can weigh. + */ +import { readdirSync, readFileSync, statSync } from 'node:fs' +import { dirname, join, relative, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' + +const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url)) +const ROOT = resolve(SCRIPT_DIR, '..') +const SCAN_ROOTS = ['apps/sim/lib', 'apps/sim/app'] +const REQUIRE_CALL = 'requirePrincipalSubjectUserId(' +const ANNOTATION = 'actorless-unsupported:' +const MAX_ANNOTATION_LOOKBACK = 3 + +/** + * `apps/sim/lib/internal/**` is the in-process tool surface: every handler under it + * mints an executor delegation, so its modules are executor-reachable whether or + * not they bind a named operation. + */ +const EXECUTOR_SURFACE_PREFIX = 'apps/sim/lib/internal/' + +export interface ActorlessFinding { + file: string + line: number + reason: 'unannotated' | 'empty-reason' + operations: string[] +} + +/** How the file was shown to be reachable by an actorless run. */ +type Reachability = 'internal-surface' | 'declared-operation' | 'unproven' + +/** Text of the balanced `(...)` or `{...}` group that starts at `openIndex`. */ +function balancedGroup(source: string, openIndex: number): string { + const open = source[openIndex] + const close = open === '(' ? ')' : '}' + let depth = 0 + for (let index = openIndex; index < source.length; index++) { + const char = source[index] + if (char === open) depth++ + else if (char === close) { + depth-- + if (depth === 0) return source.slice(openIndex, index + 1) + } + } + return source.slice(openIndex) +} + +function admitsExecutor(text: string): boolean { + const match = /delegatedServices\s*:\s*\[([^\]]*)\]/.exec(text) + return match ? /['"]executor['"]/.test(match[1]) : false +} + +/** + * Maps every `.` declared in an operations module to whether its + * policy admits an executor delegation, resolving same-file policy constants that + * are spread into the definition (e.g. `...LOG_READER_PRINCIPAL_POLICY`). + */ +export function parseOperationPolicies(source: string): Map { + const policies = new Map() + + const spreadable = new Map() + const constPattern = /(?:^|\n)\s*(?:export\s+)?const\s+([A-Za-z0-9_$]+)\s*=\s*\{/g + for (let match = constPattern.exec(source); match; match = constPattern.exec(source)) { + const braceIndex = source.indexOf('{', match.index + match[0].length - 1) + spreadable.set(match[1], admitsExecutor(balancedGroup(source, braceIndex))) + } + + /** Whether a `defineWorkspaceOperation({...})` call admits an executor delegation. */ + const definitionAdmitsExecutor = (definition: string): boolean => { + if (admitsExecutor(definition)) return true + return [...definition.matchAll(/\.\.\.([A-Za-z0-9_$]+)/g)].some( + (spread) => spreadable.get(spread[1]) === true + ) + } + + // Several domains declare their operations through same-file factories + // (`toolReadOperation('tables.rows.query')`) rather than inline, so the policy has + // to be resolved through the factory or those operations read as executor-free. + const factories = new Map() + const factoryPattern = /(?:^|\n)\s*(?:export\s+)?function\s+([A-Za-z0-9_$]+)\s*[<(]/g + for (let match = factoryPattern.exec(source); match; match = factoryPattern.exec(source)) { + const bodyIndex = source.indexOf('{', match.index + match[0].length - 1) + if (bodyIndex === -1) continue + const body = balancedGroup(source, bodyIndex) + const defineIndex = body.indexOf('defineWorkspaceOperation') + if (defineIndex === -1) continue + const parenIndex = body.indexOf('(', defineIndex) + factories.set(match[1], definitionAdmitsExecutor(balancedGroup(body, parenIndex))) + } + + const namespacePattern = /(?:^|\n)\s*export\s+const\s+([A-Za-z0-9_$]+)\s*=\s*\{/g + for (let match = namespacePattern.exec(source); match; match = namespacePattern.exec(source)) { + const namespace = match[1] + const braceIndex = source.indexOf('{', match.index + match[0].length - 1) + const body = balancedGroup(source, braceIndex) + + const entryPattern = /([A-Za-z0-9_$]+)\s*:\s*([A-Za-z0-9_$]+)\s*\(/g + for (let entry = entryPattern.exec(body); entry; entry = entryPattern.exec(body)) { + const [, key, callee] = entry + if (callee === 'defineWorkspaceOperation') { + const parenIndex = body.indexOf('(', entry.index + entry[0].length - 1) + policies.set( + `${namespace}.${key}`, + definitionAdmitsExecutor(balancedGroup(body, parenIndex)) + ) + } else if (factories.has(callee)) { + policies.set(`${namespace}.${key}`, factories.get(callee) === true) + } + } + } + + return policies +} + +/** The declared operations a module references, whether to define or to bind them. */ +export function referencedOperations(source: string, known: Set): string[] { + const referenced = new Set() + for (const match of source.matchAll(/([A-Za-z0-9_$]+)\.([A-Za-z0-9_$]+)/g)) { + const id = `${match[1]}.${match[2]}` + if (known.has(id)) referenced.add(id) + } + return [...referenced].sort() +} + +/** + * Flags `requirePrincipalSubjectUserId` calls that are not declared actorless-unsupported. + * Mirrors the placement rule the boundary annotations use: the annotation must sit in one + * of the preceding comment lines, so extra context above it is fine. + */ +export function auditSubjectRequirements(source: string, operations: string[]): ActorlessFinding[] { + const findings: ActorlessFinding[] = [] + const lines = source.split('\n') + + for (const [index, line] of lines.entries()) { + if (!line.includes(REQUIRE_CALL)) continue + + let annotation: string | undefined + for (let back = index - 1; back >= 0 && back >= index - MAX_ANNOTATION_LOOKBACK; back--) { + const candidate = lines[back].trim() + if (candidate === '') continue + if (!candidate.startsWith('//') && !candidate.startsWith('*')) break + const found = candidate.indexOf(ANNOTATION) + if (found !== -1) { + annotation = candidate.slice(found + ANNOTATION.length).trim() + break + } + } + + if (annotation === undefined) { + findings.push({ file: '', line: index + 1, reason: 'unannotated', operations }) + } else if (annotation === '') { + findings.push({ file: '', line: index + 1, reason: 'empty-reason', operations }) + } + } + + return findings +} + +function walk(directory: string, into: string[]): string[] { + for (const entry of readdirSync(directory)) { + if (entry === 'node_modules' || entry === '.next') continue + const full = join(directory, entry) + if (statSync(full).isDirectory()) walk(full, into) + else if (full.endsWith('.ts') && !full.endsWith('.test.ts')) into.push(full) + } + return into +} + +function main(): void { + const sourceFiles = SCAN_ROOTS.flatMap((root) => walk(join(ROOT, root), [])) + + const policies = new Map() + for (const file of sourceFiles) { + if (!file.endsWith('/application/operations.ts')) continue + for (const [id, executor] of parseOperationPolicies(readFileSync(file, 'utf8'))) { + policies.set(id, executor) + } + } + const known = new Set(policies.keys()) + const executorAdmitting = new Set([...policies].filter(([, yes]) => yes).map(([id]) => id)) + + // Domains with at least one executor-admitting operation. A shared use-case + // factory in such a domain names no operation of its own — it takes one as an + // argument — so it cannot be proven executor-free and fails closed here. + const executorDomains = new Set() + for (const file of sourceFiles) { + if (!file.endsWith('/application/operations.ts')) continue + const parsed = parseOperationPolicies(readFileSync(file, 'utf8')) + if ([...parsed.values()].some(Boolean)) { + executorDomains.add(relative(ROOT, dirname(dirname(file)))) + } + } + + const findings: ActorlessFinding[] = [] + const reachabilityByFinding = new Map() + let auditedFiles = 0 + + for (const file of sourceFiles) { + const source = readFileSync(file, 'utf8') + if (!source.includes(REQUIRE_CALL)) continue + + const relativePath = relative(ROOT, file) + const operations = referencedOperations(source, known) + const reachability: Reachability | undefined = relativePath.startsWith(EXECUTOR_SURFACE_PREFIX) + ? 'internal-surface' + : operations.some((id) => policies.get(id) === true) + ? 'declared-operation' + : operations.length === 0 && + [...executorDomains].some((domain) => relativePath.startsWith(`${domain}/`)) + ? 'unproven' + : undefined + if (!reachability) continue + + auditedFiles++ + for (const audited of auditSubjectRequirements(source, operations)) { + const finding = { ...audited, file: relativePath } + findings.push(finding) + reachabilityByFinding.set(finding, reachability) + } + } + + if (findings.length > 0) { + console.error( + 'Operations an actorless run can reach must not silently require a human subject:' + ) + for (const finding of findings) { + const reachability = reachabilityByFinding.get(finding) + const via = + reachability === 'internal-surface' + ? 'in-process tool surface' + : reachability === 'unproven' + ? 'shared use case in a domain with executor-admitting operations' + : `reachable via ${finding.operations.filter((id) => executorAdmitting.has(id)).join(', ')}` + const problem = + finding.reason === 'empty-reason' + ? `${ANNOTATION} needs a reason` + : `unannotated requirePrincipalSubjectUserId (${via})` + console.error(` ${finding.file}:${finding.line} ${problem}`) + } + console.error( + `\nA scheduled, public-API, or subject-less webhook run reaches these with no user, and` + + `\n\`requirePrincipalSubjectUserId\` throws a 500 there rather than anything actionable.` + + `\nEither resolve the user optionally (\`resolvePrincipalSubjectUserId\`) when it is only` + + `\nattribution, or declare the gap with \`// ${ANNOTATION} \` above the call.` + ) + process.exit(1) + } + + console.log( + `✓ no undeclared human-subject requirements on actorless-reachable operations ` + + `(${executorAdmitting.size} executor-admitting operations, ${auditedFiles} files audited)` + ) +} + +if (import.meta.main) main()