From 1786da6c0015a64e68aabc7d81eb464f16365f1c Mon Sep 17 00:00:00 2001 From: "J.C. Bartle" Date: Sun, 28 Jun 2026 15:42:06 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20fix:=20fail=20closed=20?= =?UTF-8?q?on=20OBO=20MCP=20user=20identity=20mismatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an OBO-specific guard before MCP tool execution that requires the effective invocation user and captured request user to both have ids and to match. This prevents OBO tool calls from falling back to a separate configurable.user_id identity after request-bound OBO context has already been captured. Keep the existing user id fallback behavior for non-OBO MCP calls. Tests cover mismatched OBO users, missing user ids, and the matching-user path ignoring a conflicting configurable.user_id. --- api/server/services/MCP.js | 25 +++++- api/server/services/MCP.spec.js | 137 ++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 1 deletion(-) diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index bf4ff02da7..1e98a6424d 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -377,6 +377,24 @@ function createOAuthCallback({ runStepEmitter, runStepDeltaEmitter }) { }; } +function resolveToolCallUserId({ effectiveUser, capturedUser, invocationUserId, serverConfig }) { + if (serverConfig?.obo == null) { + return effectiveUser?.id || invocationUserId || capturedUser?.id; + } + + const effectiveUserId = effectiveUser?.id; + const capturedUserId = capturedUser?.id; + if (!effectiveUserId || !capturedUserId) { + throw new Error('OBO tool calls require matching captured and effective user ids'); + } + + if (effectiveUserId !== capturedUserId) { + throw new Error('OBO tool call user mismatch'); + } + + return effectiveUserId; +} + /** * @param {Object} params * @param {ServerResponse} params.res - The Express response object for sending events. @@ -793,7 +811,12 @@ function createToolInstance({ const _call = async (toolArguments, config) => { const effectiveUser = config?.configurable?.user ?? capturedUser; const permissionUser = effectiveUser; - const userId = effectiveUser?.id || config?.configurable?.user_id || capturedUser?.id; + const userId = resolveToolCallUserId({ + effectiveUser, + capturedUser, + invocationUserId: config?.configurable?.user_id, + serverConfig: capturedServerConfig, + }); /** @type {ReturnType} */ let abortHandler = null; /** @type {AbortSignal} */ diff --git a/api/server/services/MCP.spec.js b/api/server/services/MCP.spec.js index c434df631d..7835911397 100644 --- a/api/server/services/MCP.spec.js +++ b/api/server/services/MCP.spec.js @@ -1310,6 +1310,143 @@ describe('User parameter passing tests', () => { }), ); }); + + it('should reject OBO tool execution when effective and captured users differ', async () => { + const capturedUser = { id: 'captured-user', email: 'captured@example.com', role: 'USER' }; + const effectiveUser = { id: 'effective-user', email: 'effective@example.com', role: 'USER' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + + const mcpTool = await createMCPTool({ + res: mockRes, + user: capturedUser, + toolKey: `test-tool${D}obo-server`, + provider: 'openai', + userMCPAuthMap: {}, + config: { + url: 'https://obo.example.com', + obo: { scopes: 'api://obo-server/Mcp.Tools.ReadWrite' }, + }, + availableTools: { + [`test-tool${D}obo-server`]: { + function: { + description: 'Cached OBO tool', + parameters: { type: 'object', properties: {} }, + }, + }, + }, + }); + + await expect( + mcpTool.invoke( + {}, + { + configurable: { user: effectiveUser }, + metadata: { provider: 'openai', thread_id: 't1', run_id: 'r1' }, + toolCall: {}, + }, + ), + ).rejects.toThrow('OBO tool call user mismatch'); + + expect(mockGetMCPManager).not.toHaveBeenCalled(); + }); + + it('should reject OBO tool execution when an effective or captured user id is missing', async () => { + const capturedUser = { email: 'captured@example.com', role: 'USER' }; + const effectiveUser = { id: 'effective-user', email: 'effective@example.com', role: 'USER' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + + const mcpTool = await createMCPTool({ + res: mockRes, + user: capturedUser, + toolKey: `test-tool${D}obo-server`, + provider: 'openai', + userMCPAuthMap: {}, + config: { + url: 'https://obo.example.com', + obo: { scopes: 'api://obo-server/Mcp.Tools.ReadWrite' }, + }, + availableTools: { + [`test-tool${D}obo-server`]: { + function: { + description: 'Cached OBO tool', + parameters: { type: 'object', properties: {} }, + }, + }, + }, + }); + + await expect( + mcpTool.invoke( + {}, + { + configurable: { user: effectiveUser }, + metadata: { provider: 'openai', thread_id: 't1', run_id: 'r1' }, + toolCall: {}, + }, + ), + ).rejects.toThrow('OBO tool calls require matching captured and effective user ids'); + + expect(mockGetMCPManager).not.toHaveBeenCalled(); + }); + + it('should execute OBO tools when effective and captured user ids match', async () => { + const capturedUser = { id: 'obo-user', email: 'captured@example.com', role: 'USER' }; + const effectiveUser = { id: 'obo-user', email: 'effective@example.com', role: 'USER' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + const { getRoleByName } = require('~/models'); + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: true, + }, + }, + }); + + const mockCallTool = jest.fn().mockResolvedValue(['ok', null]); + mockGetMCPManager.mockReturnValue({ callTool: mockCallTool }); + + const mcpTool = await createMCPTool({ + res: mockRes, + user: capturedUser, + toolKey: `test-tool${D}obo-server`, + provider: 'openai', + userMCPAuthMap: {}, + upstreamTokenProvider: async () => null, + config: { + url: 'https://obo.example.com', + obo: { scopes: 'api://obo-server/Mcp.Tools.ReadWrite' }, + }, + availableTools: { + [`test-tool${D}obo-server`]: { + function: { + description: 'Cached OBO tool', + parameters: { type: 'object', properties: {} }, + }, + }, + }, + }); + + await expect( + mcpTool.invoke( + {}, + { + configurable: { + user: effectiveUser, + user_id: 'third-user', + }, + metadata: { provider: 'openai', thread_id: 't1', run_id: 'r1' }, + toolCall: {}, + }, + ), + ).resolves.toBe('ok'); + + expect(mockGetMCPManager).toHaveBeenCalledWith('obo-user'); + expect(mockCallTool).toHaveBeenCalledWith( + expect.objectContaining({ + user: effectiveUser, + }), + ); + }); }); describe('reinitMCPServer (via reconnectServer)', () => {