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 68e4976a7ff..f1d65d24bbd 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts @@ -5,6 +5,8 @@ */ import { dbChainMockFns, + encryptionMock, + encryptionMockFns, hybridAuthMockFns, permissionsMock, permissionsMockFns, @@ -31,6 +33,8 @@ const { fetchMock: vi.fn(), })) +vi.mock('@/lib/core/security/encryption', () => encryptionMock) + vi.mock('@/lib/billing/core/billing-attribution', () => ({ BILLING_ATTRIBUTION_HEADER: 'x-sim-billing-attribution', assertBillingAttributionSnapshot: mockAssertBillingAttributionSnapshot, @@ -99,6 +103,9 @@ describe('MCP Serve Route', () => { ) mockAssertBillingAttributionSnapshot.mockImplementation((value: unknown) => value) mockSerializeBillingAttributionHeader.mockReturnValue('serialized-attribution') + encryptionMockFns.mockDecryptSecret.mockImplementation(async (encryptedValue: string) => ({ + decrypted: `decrypted:${encryptedValue}`, + })) }) afterEach(() => { @@ -1121,6 +1128,232 @@ describe('MCP Serve Route', () => { expect(JSON.parse(body.result.content[0].text)).toEqual(['a', 'b']) }) + it('serves a public tool whose workflow was authored by someone other than the actor', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Public Server', + workspaceId: 'ws-1', + isPublic: true, + createdBy: 'owner-1', + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }]) + mockExecuteWorkflowService.mockResolvedValueOnce({ + ok: true, + executionId: 'exec-1', + workflowId: 'wf-1', + status: 'completed', + aborted: null, + output: { ok: true }, + error: null, + hasResponseBlock: false, + resolvedSecretTraceProvenance: createResolvedSecretTraceProvenance('author-2'), + }) + + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + 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(200) + expect(JSON.parse(body.result.content[0].text)).toEqual({ ok: true }) + }) + + it('serves a private tool whose workflow was authored by someone other than the caller', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Private Server', + workspaceId: 'ws-1', + isPublic: false, + createdBy: 'owner-1', + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }]) + /** + * `vi.clearAllMocks()` does not drain `mockResolvedValueOnce` queues, and a public-server + * test never reaches `checkHybridAuth`, so an earlier queued auth would be consumed here. + */ + hybridAuthMockFns.mockCheckHybridAuth.mockReset() + hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'api_key', + apiKeyType: 'workspace', + workspaceId: 'ws-1', + }) + mockGetUserEntityPermissions.mockResolvedValueOnce('write') + mockExecuteWorkflowService.mockResolvedValueOnce({ + ok: true, + executionId: 'exec-1', + workflowId: 'wf-1', + status: 'completed', + aborted: null, + output: { ok: true }, + error: null, + hasResponseBlock: false, + resolvedSecretTraceProvenance: createResolvedSecretTraceProvenance('author-2'), + }) + + 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' }) }) + const body = await response.json() + + expect(response.status).toBe(200) + expect(JSON.parse(body.result.content[0].text)).toEqual({ ok: true }) + }) + + it('anonymizes an author-scoped secret label when the actor is not the author', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Public Server', + workspaceId: 'ws-1', + isPublic: true, + createdBy: 'owner-1', + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }]) + mockExecuteWorkflowService.mockResolvedValueOnce({ + ok: true, + executionId: 'exec-1', + workflowId: 'wf-1', + status: 'completed', + aborted: null, + output: { leaked: 'decrypted:author-ciphertext' }, + error: null, + hasResponseBlock: false, + resolvedSecretTraceProvenance: { + ...createResolvedSecretTraceProvenance('author-2'), + entries: [{ name: 'AUTHOR_TOKEN', encryptedValue: 'author-ciphertext' }], + }, + }) + + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + 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(200) + expect(JSON.parse(body.result.content[0].text)).toEqual({ leaked: '[REDACTED_SECRET]' }) + expect(body.result.content[0].text).not.toContain('AUTHOR_TOKEN') + }) + + it('keeps the author-scoped secret label when the actor is the author', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Public Server', + workspaceId: 'ws-1', + isPublic: true, + createdBy: 'owner-1', + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }]) + mockExecuteWorkflowService.mockResolvedValueOnce({ + ok: true, + executionId: 'exec-1', + workflowId: 'wf-1', + status: 'completed', + aborted: null, + output: { leaked: 'decrypted:owner-ciphertext' }, + error: null, + hasResponseBlock: false, + resolvedSecretTraceProvenance: { + ...createResolvedSecretTraceProvenance('owner-1'), + entries: [{ name: 'OWNER_TOKEN', encryptedValue: 'owner-ciphertext' }], + }, + }) + + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + 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(200) + expect(JSON.parse(body.result.content[0].text)).toEqual({ leaked: '{{OWNER_TOKEN}}' }) + }) + + it('refuses a tool result whose provenance was stamped for another workspace', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Public Server', + workspaceId: 'ws-1', + isPublic: true, + createdBy: 'owner-1', + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }]) + mockExecuteWorkflowService.mockResolvedValueOnce({ + ok: true, + executionId: 'exec-1', + workflowId: 'wf-1', + status: 'completed', + aborted: null, + output: { ok: true }, + error: null, + hasResponseBlock: false, + resolvedSecretTraceProvenance: createResolvedSecretTraceProvenance('owner-1', 'ws-other'), + }) + + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + 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(500) + expect(body.error.code).toBe(-32603) + }) + it('rejects duplicate tool names instead of choosing an arbitrary workflow', async () => { dbChainMockFns.limit .mockResolvedValueOnce([ diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.ts index b7fd9db0c55..80c16fdda81 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.ts @@ -205,11 +205,20 @@ async function projectWorkflowToolOutput( provenance: unknown, scope: { userId: string; workspaceId: string } ): Promise { + /** + * Deliberately compares the tenant only, never the user. The executor stamps provenance with + * the workflow AUTHOR (`personalEnvUserId` falls back to `metadata.workflowUserId` on this + * non-session path) while `scope` here is the ACTING caller, and a caller who did not author + * the workflow is the ordinary team configuration — demanding they match refuses every such + * call after the workflow has already run and been billed. Restoring a user comparison here + * is not hardening: the registry compares both scope fields itself and marks every entry + * imported from another user anonymous, so a non-author already sees the opaque redaction + * placeholder rather than the author's secret names. + */ if ( !isResolvedSecretTraceProvenanceV1(provenance) || !provenance.complete || - provenance.scope?.userId !== scope.userId || - provenance.scope.workspaceId !== scope.workspaceId + provenance.scope?.workspaceId !== scope.workspaceId ) { throw new Error('MCP workflow execution provenance is unavailable') }