🛠️ fix: fail closed on OBO MCP user identity mismatch

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.
This commit is contained in:
J.C. Bartle 2026-06-28 15:42:06 -04:00
parent d3a6f78ff8
commit 1786da6c00
2 changed files with 161 additions and 1 deletions

View file

@ -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<typeof createAbortHandler>} */
let abortHandler = null;
/** @type {AbortSignal} */

View file

@ -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)', () => {