Skip to content

Commit a2eafd9

Browse files
committed
fix(mothership): authorize MCP calls as the chat subject
1 parent a700b9b commit a2eafd9

5 files changed

Lines changed: 167 additions & 11 deletions

File tree

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,62 @@ describe('executeMcpTool', () => {
7373
})
7474
})
7575

76+
it.each(['', 'workflow-1'])(
77+
'uses the authenticated chat subject for a Copilot call with workflowId %j',
78+
async (workflowId) => {
79+
const response = await executeMcpTool({
80+
toolId: 'mcp-server-lookup',
81+
input: { query: 'sim', _context: { userId: 'forged', workspaceId: 'foreign' } },
82+
headers: new Headers(),
83+
context: {
84+
...CONTEXT,
85+
workflowId,
86+
copilotToolExecution: true,
87+
copilotInteractionMode: 'interactive',
88+
chatId: 'chat-1',
89+
toolCallId: 'call-1',
90+
},
91+
requestId: 'request-copilot',
92+
})
93+
expect(response.status).toBe(200)
94+
expect(mocks.createPrincipal).not.toHaveBeenCalled()
95+
expect(mocks.executeUseCase).toHaveBeenCalledWith({
96+
principal: expect.objectContaining({
97+
kind: 'delegated',
98+
serviceId: 'copilot',
99+
subjectUserId: 'user-1',
100+
workspaceId: 'workspace-1',
101+
audience: 'sim:mcp-servers',
102+
delegationId: 'copilot-tool:call-1',
103+
resourceScope: { chatId: 'chat-1' },
104+
}),
105+
input: expect.objectContaining({ arguments: { query: 'sim' } }),
106+
})
107+
}
108+
)
109+
110+
it.each(['userId', 'toolCallId'] as const)(
111+
'rejects incomplete trusted Copilot context without %s',
112+
async (field) => {
113+
const response = await executeMcpTool({
114+
toolId: 'mcp-server-lookup',
115+
input: { _context: { userId: 'forged', toolCallId: 'forged-call' } },
116+
headers: new Headers(),
117+
context: {
118+
...CONTEXT,
119+
workflowId: '',
120+
copilotToolExecution: true,
121+
toolCallId: 'call-1',
122+
[field]: undefined,
123+
},
124+
requestId: 'request-copilot',
125+
})
126+
expect(response.ok).toBe(false)
127+
expect(mocks.executeUseCase).not.toHaveBeenCalled()
128+
expect(mocks.createPrincipal).not.toHaveBeenCalled()
129+
}
130+
)
131+
76132
it('parses direct block arguments and invokes the authorized use case', async () => {
77133
const response = await executeMcpTool({
78134
toolId: 'mcp-server-lookup',

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ import { executeMcpToolUseCase, McpToolsNotAllowedError } from '@/lib/mcp/applic
2121
import { McpOauthRedirectRequired } from '@/lib/mcp/oauth'
2222
import { McpOauthAuthorizationRequiredError } from '@/lib/mcp/types'
2323
import { categorizeError, parseMcpToolId } from '@/lib/mcp/utils'
24+
import {
25+
COPILOT_APPLICATION_DELEGATION_TTL_MS,
26+
createCopilotApplicationPrincipal,
27+
requireTrustedCopilotExecutionContext,
28+
} from '@/lib/mothership/auth/application-delegation'
2429
import {
2530
ResolvedSecretTraceProvenanceAccumulator,
2631
type ResolvedSecretTraceRegistry,
@@ -124,10 +129,17 @@ export const executeMcpTool: InternalToolOperationHandler = async (request) => {
124129

125130
let provenance: ResolvedSecretTraceProvenanceAccumulator | undefined
126131
try {
127-
const principal = await createExecutorPrincipalFromExecutionContext({
128-
context: request.context,
129-
audience: MCP_SERVER_DELEGATION_AUDIENCE,
130-
})
132+
/** Workspace chat tools act as their authenticated subject without requiring a workflow. */
133+
const principal = request.context.copilotToolExecution
134+
? createCopilotApplicationPrincipal(requireTrustedCopilotExecutionContext(request.context), {
135+
audience: MCP_SERVER_DELEGATION_AUDIENCE,
136+
ttlMs: COPILOT_APPLICATION_DELEGATION_TTL_MS,
137+
createDelegationId: (context) => `copilot-tool:${context.toolCallId}`,
138+
})
139+
: await createExecutorPrincipalFromExecutionContext({
140+
context: request.context,
141+
audience: MCP_SERVER_DELEGATION_AUDIENCE,
142+
})
131143
request.signal?.throwIfAborted()
132144
const subject = resolvePrincipalSubject(principal)
133145
provenance =

‎apps/sim/lib/mcp/application/execute-tool.test.ts‎

Lines changed: 92 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import type { WorkflowExecutionDelegatedPrincipal } from '@sim/auth/principal'
4+
import type {
5+
BoundWorkflowExecutionDelegatedPrincipal,
6+
SubjectDelegatedPrincipal,
7+
} from '@sim/auth/principal'
58
import { beforeEach, describe, expect, it, vi } from 'vitest'
69

710
const mocks = vi.hoisted(() => ({
@@ -50,7 +53,7 @@ const SERVER = {
5053
workspaceId: WORKSPACE.workspaceId,
5154
enabled: true,
5255
}
53-
const PRINCIPAL: WorkflowExecutionDelegatedPrincipal = {
56+
const PRINCIPAL: BoundWorkflowExecutionDelegatedPrincipal = {
5457
kind: 'delegated',
5558
serviceId: 'executor',
5659
subjectUserId: 'user-1',
@@ -61,7 +64,7 @@ const PRINCIPAL: WorkflowExecutionDelegatedPrincipal = {
6164
expiresAt: new Date('2099-08-27T00:05:00.000Z'),
6265
delegationContext: { kind: 'workflow_execution', workflowId: 'workflow-1' },
6366
}
64-
const ACTORLESS_PRINCIPAL: WorkflowExecutionDelegatedPrincipal = {
67+
const ACTORLESS_PRINCIPAL: BoundWorkflowExecutionDelegatedPrincipal = {
6568
kind: 'delegated',
6669
serviceId: 'executor',
6770
workspaceId: WORKSPACE.workspaceId,
@@ -85,7 +88,18 @@ const ACTORLESS_PRINCIPAL: WorkflowExecutionDelegatedPrincipal = {
8588
},
8689
},
8790
}
88-
const COMPATIBILITY_ACTOR_PRINCIPAL: WorkflowExecutionDelegatedPrincipal = {
91+
const COPILOT_PRINCIPAL: SubjectDelegatedPrincipal = {
92+
kind: 'delegated',
93+
serviceId: 'copilot',
94+
subjectUserId: 'chat-user',
95+
workspaceId: WORKSPACE.workspaceId,
96+
delegationId: 'copilot-tool:call-1',
97+
audience: 'sim:mcp-servers',
98+
issuedAt: new Date('2026-08-27T00:00:00.000Z'),
99+
expiresAt: new Date('2099-08-27T00:05:00.000Z'),
100+
resourceScope: { chatId: 'chat-1' },
101+
}
102+
const COMPATIBILITY_ACTOR_PRINCIPAL: BoundWorkflowExecutionDelegatedPrincipal = {
89103
...ACTORLESS_PRINCIPAL,
90104
delegationContext: {
91105
...ACTORLESS_PRINCIPAL.delegationContext,
@@ -119,6 +133,80 @@ describe('executeMcpToolUseCase', () => {
119133
mocks.executeTool.mockResolvedValue({ content: [{ type: 'text', text: 'done' }] })
120134
})
121135

136+
it.each(['read', 'write', 'admin'])(
137+
'executes as the current Copilot subject with %s workspace permission',
138+
async (permission) => {
139+
mocks.resolvePermission.mockResolvedValue(permission)
140+
await executeMcpToolUseCase.execute({
141+
principal: COPILOT_PRINCIPAL,
142+
input: {
143+
workspaceId: WORKSPACE.workspaceId,
144+
serverId: SERVER.id,
145+
toolName: 'lookup',
146+
arguments: { count: 1 },
147+
},
148+
})
149+
expect(mocks.resolvePermission).toHaveBeenCalledWith(
150+
'chat-user',
151+
WORKSPACE.workspaceId,
152+
null,
153+
undefined,
154+
{ forUpdate: undefined }
155+
)
156+
expect(mocks.assertPermissionsAllowed).toHaveBeenCalledWith({
157+
userId: 'chat-user',
158+
workspaceId: WORKSPACE.workspaceId,
159+
toolKind: 'mcp',
160+
})
161+
expect(mocks.executeTool).toHaveBeenCalledWith(
162+
'chat-user',
163+
SERVER.id,
164+
{ name: 'lookup', arguments: { count: 1 } },
165+
WORKSPACE.workspaceId,
166+
undefined,
167+
undefined,
168+
{ signal: undefined, timeoutMs: undefined }
169+
)
170+
}
171+
)
172+
173+
it.each([
174+
{ audience: 'sim:other' },
175+
{ workspaceId: 'foreign-workspace' },
176+
{ expiresAt: new Date(0) },
177+
])('rejects invalid Copilot delegation %j before discovery or execution', async (override) => {
178+
await expect(
179+
executeMcpToolUseCase.execute({
180+
principal: { ...COPILOT_PRINCIPAL, ...override },
181+
input: {
182+
workspaceId: WORKSPACE.workspaceId,
183+
serverId: SERVER.id,
184+
toolName: 'lookup',
185+
arguments: { count: 1 },
186+
},
187+
})
188+
).rejects.toMatchObject({ code: 'forbidden' })
189+
expect(mocks.discoverServerTools).not.toHaveBeenCalled()
190+
expect(mocks.executeTool).not.toHaveBeenCalled()
191+
})
192+
193+
it('rechecks the Copilot subject after workspace membership is revoked', async () => {
194+
mocks.resolvePermission.mockResolvedValue(null)
195+
await expect(
196+
executeMcpToolUseCase.execute({
197+
principal: COPILOT_PRINCIPAL,
198+
input: {
199+
workspaceId: WORKSPACE.workspaceId,
200+
serverId: SERVER.id,
201+
toolName: 'lookup',
202+
arguments: { count: 1 },
203+
},
204+
})
205+
).rejects.toMatchObject({ code: 'forbidden' })
206+
expect(mocks.discoverServerTools).not.toHaveBeenCalled()
207+
expect(mocks.executeTool).not.toHaveBeenCalled()
208+
})
209+
122210
it('authorizes, coerces the discovered schema, and preserves execution context', async () => {
123211
const provenance = vi.fn()
124212
const signal = new AbortController().signal

‎apps/sim/lib/mcp/application/operations.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ describe('MCP server operation registry', () => {
1313
})
1414
})
1515

16-
it('admits only the executor delegation for tool execution', () => {
16+
it('admits executor and Copilot delegations for tool execution', () => {
1717
expect(mcpServerOperations.executeTool).toMatchObject({
1818
id: 'mcp_servers.tools.execute',
1919
minimumRole: 'read',
2020
workspaceApiKey: 'deny',
2121
principalKinds: ['delegated'],
22-
delegatedServices: ['executor'],
22+
delegatedServices: ['executor', 'copilot'],
2323
})
2424
})
2525

‎apps/sim/lib/mcp/application/operations.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const DISCOVERY_PRINCIPAL_POLICY = {
1414
} as const
1515
const EXECUTION_PRINCIPAL_POLICY = {
1616
principalKinds: ['delegated'],
17-
delegatedServices: ['executor'],
17+
delegatedServices: ['executor', 'copilot'],
1818
} as const
1919

2020
export const mcpServerOperations = {

0 commit comments

Comments
 (0)