From e636d8f6519e13d1592ad1a48dd9ee9b2ae08fa2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 12:21:07 -0700 Subject: [PATCH] fix(mcp): apply the workspace personal API key setting to MCP serve auth --- .../api/mcp/serve/[serverId]/route.test.ts | 81 +++++++++++++++++++ .../sim/app/api/mcp/serve/[serverId]/route.ts | 19 ++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts index e0598d0ccbe..2b46e536aa0 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts @@ -270,6 +270,7 @@ describe('MCP Serve Route', () => { workspaceId: 'ws-1', isPublic: false, createdBy: 'owner-1', + workspaceAllowsPersonalApiKeys: true, }, ]) .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) @@ -315,6 +316,85 @@ describe('MCP Serve Route', () => { }) }) + it('rejects a personal api key when the workspace disallows personal api keys', async () => { + dbChainMockFns.limit.mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Private Server', + workspaceId: 'ws-1', + isPublic: false, + createdBy: 'owner-1', + workspaceAllowsPersonalApiKeys: false, + }, + ]) + hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'api_key', + apiKeyType: 'personal', + }) + mockGetUserEntityPermissions.mockResolvedValueOnce('write') + + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + headers: { 'X-API-Key': 'pk_test_123' }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'tool_a', arguments: { q: 'test' } }, + }), + }) + const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) }) + const body = await response.json() + + expect(response.status).toBe(403) + expect(body.error).toBe('Personal API keys are not allowed for this workspace') + expect(fetchMock).not.toHaveBeenCalled() + expect(mockGenerateInternalToken).not.toHaveBeenCalled() + }) + + it('allows a workspace api key when the workspace disallows personal api keys', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Private Server', + workspaceId: 'ws-1', + isPublic: false, + createdBy: 'owner-1', + workspaceAllowsPersonalApiKeys: false, + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }]) + hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'api_key', + apiKeyType: 'workspace', + workspaceId: 'ws-1', + }) + mockGetUserEntityPermissions.mockResolvedValueOnce('write') + mockGenerateInternalToken.mockResolvedValueOnce('internal-token-user-1') + fetchMock.mockResolvedValueOnce(createWorkflowExecutionResponse({ output: { ok: true } })) + + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + headers: { 'X-API-Key': 'wsk_test_123' }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'tool_a', arguments: { q: 'test' } }, + }), + }) + const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) }) + + expect(response.status).toBe(200) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + it('forwards nested MCP arguments without changing falsy or null values', async () => { dbChainMockFns.limit .mockResolvedValueOnce([ @@ -1150,6 +1230,7 @@ describe('MCP Serve Route', () => { workspaceId: 'ws-1', isPublic: false, createdBy: 'owner-1', + workspaceAllowsPersonalApiKeys: true, }, ]) .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.ts index c5a8146d200..9db86217f01 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.ts @@ -363,6 +363,7 @@ async function getServer(serverId: string) { workspaceId: workflowMcpServer.workspaceId, isPublic: workflowMcpServer.isPublic, createdBy: workflowMcpServer.createdBy, + workspaceAllowsPersonalApiKeys: workspace.allowPersonalApiKeys, }) .from(workflowMcpServer) .innerJoin(workspace, eq(workflowMcpServer.workspaceId, workspace.id)) @@ -424,11 +425,25 @@ async function authorizeMcpServeRequest( return { response: NextResponse.json({ error: 'Forbidden' }, { status: 403 }) } } + /** + * Not redundant with the same check in `/api/workflows/{id}/execute`: tool + * calls bridge to that route with an internal JWT, so its API-key branch + * never sees this request. + */ + const isPersonalApiKey = auth.authType === AuthType.API_KEY && auth.apiKeyType === 'personal' + if (isPersonalApiKey && !server.workspaceAllowsPersonalApiKeys) { + return { + response: NextResponse.json( + { error: 'Personal API keys are not allowed for this workspace' }, + { status: 403 } + ), + } + } + return { executeAuthContext: { userId: auth.userId, - useAuthenticatedUserAsActor: - auth.authType === AuthType.API_KEY && auth.apiKeyType === 'personal', + useAuthenticatedUserAsActor: isPersonalApiKey, }, } }