diff --git a/api/app/clients/tools/util/handleTools.js b/api/app/clients/tools/util/handleTools.js index 89a79f3cbd..4577d8a9ea 100644 --- a/api/app/clients/tools/util/handleTools.js +++ b/api/app/clients/tools/util/handleTools.js @@ -35,7 +35,12 @@ const { createGeminiImageTool, createOpenAIImageTools, } = require('../'); -const { createMCPTool, createMCPTools, resolveConfigServers } = require('~/server/services/MCP'); +const { + createMCPTool, + createMCPTools, + resolveConfigServers, + userCanUseMCPServers, +} = require('~/server/services/MCP'); const { createFileSearchTool, primeFiles: primeSearchFiles } = require('./fileSearch'); const { primeFiles: primeCodeFiles } = require('~/server/services/Files/Code/process'); const { getUserPluginAuthValue } = require('~/server/services/PluginService'); @@ -227,6 +232,9 @@ const loadTools = async ({ }; const requestedTools = {}; + const hasMCPTools = tools.some((toolName) => toolName && mcpToolPattern.test(toolName)); + const canUseMCP = hasMCPTools ? await userCanUseMCPServers(options.req?.user) : true; + let loggedMCPDenied = false; if (functions === true) { toolConstructors.dalle = DALLE3; @@ -266,7 +274,7 @@ const loadTools = async ({ /** Resolve config-source servers for the current user/tenant context */ let configServers; - if (tools.some((tool) => tool && mcpToolPattern.test(tool))) { + if (hasMCPTools && canUseMCP) { configServers = await resolveConfigServers(options.req); } @@ -345,6 +353,16 @@ const loadTools = async ({ }; continue; } else if (tool && mcpToolPattern.test(tool)) { + if (!canUseMCP) { + if (!loggedMCPDenied) { + logger.warn( + `[handleTools] User ${options.req?.user?.id} lacks MCP server use permission`, + ); + loggedMCPDenied = true; + } + continue; + } + const [toolName, serverName] = tool.split(Constants.mcp_delimiter); if (toolName === Constants.mcp_server) { /** Placeholder used for UI purposes */ diff --git a/api/server/controllers/agents/filterAuthorizedTools.spec.js b/api/server/controllers/agents/filterAuthorizedTools.spec.js index e6b41aef16..4bdb870524 100644 --- a/api/server/controllers/agents/filterAuthorizedTools.spec.js +++ b/api/server/controllers/agents/filterAuthorizedTools.spec.js @@ -1,12 +1,13 @@ const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); -const { Constants } = require('librechat-data-provider'); +const { Constants, actionDelimiter } = require('librechat-data-provider'); const { agentSchema } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); const d = Constants.mcp_delimiter; const mockGetAllServerConfigs = jest.fn(); +const mockUserCanUseMCPServers = jest.fn(); jest.mock('~/server/services/Config', () => ({ getCachedTools: jest.fn().mockResolvedValue({ @@ -24,6 +25,7 @@ jest.mock('~/config', () => ({ jest.mock('~/server/services/MCP', () => ({ resolveConfigServers: jest.fn().mockResolvedValue({}), + userCanUseMCPServers: (...args) => mockUserCanUseMCPServers(...args), })); jest.mock('~/server/services/Files/strategies', () => ({ @@ -106,6 +108,7 @@ describe('MCP Tool Authorization', () => { authorizedServer: { type: 'sse', url: 'https://authorized.example.com' }, anotherServer: { type: 'sse', url: 'https://another.example.com' }, }); + mockUserCanUseMCPServers.mockResolvedValue(true); mockReq = { user: { @@ -127,11 +130,13 @@ describe('MCP Tool Authorization', () => { describe('filterAuthorizedTools', () => { const availableTools = { web_search: true, custom_tool: true }; const userId = 'test-user-123'; + const testUser = { id: userId, role: 'USER' }; test('should keep authorized MCP tools and strip unauthorized ones', async () => { const result = await filterAuthorizedTools({ tools: [`toolA${d}authorizedServer`, `toolB${d}forbiddenServer`, 'web_search'], userId, + user: testUser, availableTools, }); @@ -140,6 +145,39 @@ describe('MCP Tool Authorization', () => { expect(result).not.toContain(`toolB${d}forbiddenServer`); }); + test('should strip MCP tools when user lacks MCP server use permission', async () => { + mockUserCanUseMCPServers.mockResolvedValue(false); + + const result = await filterAuthorizedTools({ + tools: [ + `toolA${d}authorizedServer`, + `${Constants.mcp_all}${d}authorizedServer`, + 'web_search', + ], + userId, + user: testUser, + availableTools, + }); + + expect(result).toEqual(['web_search']); + expect(mockUserCanUseMCPServers).toHaveBeenCalledWith({ id: userId, role: 'USER' }); + expect(mockGetAllServerConfigs).not.toHaveBeenCalled(); + }); + + test('should strip MCP tools when user context is missing', async () => { + mockUserCanUseMCPServers.mockResolvedValueOnce(false); + + const result = await filterAuthorizedTools({ + tools: [`toolA${d}authorizedServer`, 'web_search'], + userId, + availableTools, + }); + + expect(result).toEqual(['web_search']); + expect(mockUserCanUseMCPServers).toHaveBeenCalledWith(undefined); + expect(mockGetAllServerConfigs).not.toHaveBeenCalled(); + }); + test('should keep system tools without querying MCP registry', async () => { const result = await filterAuthorizedTools({ tools: ['execute_code', 'file_search', 'web_search'], @@ -170,6 +208,7 @@ describe('MCP Tool Authorization', () => { const result = await filterAuthorizedTools({ tools: [`toolA${d}someServer`, 'web_search'], userId, + user: testUser, availableTools, }); @@ -188,6 +227,7 @@ describe('MCP Tool Authorization', () => { `steal${d}nonexistent`, ], userId, + user: testUser, availableTools, }); @@ -224,6 +264,7 @@ describe('MCP Tool Authorization', () => { await filterAuthorizedTools({ tools: [`tool${d}authorizedServer`], userId: 'specific-user-id', + user: { id: 'specific-user-id', role: 'USER' }, availableTools, }); @@ -241,6 +282,7 @@ describe('MCP Tool Authorization', () => { const result = await filterAuthorizedTools({ tools: [`tool${d}config-override-server`, `tool${d}unauthorizedServer`], userId, + user: testUser, availableTools, configServers, }); @@ -254,6 +296,7 @@ describe('MCP Tool Authorization', () => { await filterAuthorizedTools({ tools: [`tool1${d}authorizedServer`, `tool2${d}anotherServer`, `tool3${d}unknownServer`], userId, + user: testUser, availableTools, }); @@ -270,6 +313,7 @@ describe('MCP Tool Authorization', () => { const result = await filterAuthorizedTools({ tools: [...existingTools, `newTool${d}unknownServer`, 'web_search'], userId, + user: testUser, availableTools, existingTools, }); @@ -288,6 +332,7 @@ describe('MCP Tool Authorization', () => { const result = await filterAuthorizedTools({ tools: [`toolA${d}serverA`, 'web_search'], userId, + user: testUser, availableTools, }); @@ -303,6 +348,7 @@ describe('MCP Tool Authorization', () => { const result = await filterAuthorizedTools({ tools: [malformedTool, `legit${d}serverA`, 'web_search'], userId, + user: testUser, availableTools, existingTools: [malformedTool, `legit${d}serverA`], }); @@ -312,6 +358,43 @@ describe('MCP Tool Authorization', () => { expect(result).not.toContain(malformedTool); }); + test('should gate app-level MCP tools present in the global tool cache', async () => { + const appMcpTool = `appTool${d}authorizedServer`; + const forbiddenAppMcpTool = `appTool${d}forbiddenServer`; + const cacheWithMCPTools = { + ...availableTools, + [appMcpTool]: true, + [forbiddenAppMcpTool]: true, + }; + + const result = await filterAuthorizedTools({ + tools: [appMcpTool, forbiddenAppMcpTool, 'web_search'], + userId, + user: testUser, + availableTools: cacheWithMCPTools, + }); + + expect(result).toContain(appMcpTool); + expect(result).toContain('web_search'); + expect(result).not.toContain(forbiddenAppMcpTool); + }); + + test('should strip app-level MCP tools from the cache when user lacks MCP server use permission', async () => { + mockUserCanUseMCPServers.mockResolvedValue(false); + const appMcpTool = `appTool${d}authorizedServer`; + const cacheWithMCPTools = { ...availableTools, [appMcpTool]: true }; + + const result = await filterAuthorizedTools({ + tools: [appMcpTool, 'web_search'], + userId, + user: testUser, + availableTools: cacheWithMCPTools, + }); + + expect(result).toEqual(['web_search']); + expect(mockGetAllServerConfigs).not.toHaveBeenCalled(); + }); + test('should reject malformed MCP tool keys with multiple delimiters', async () => { const result = await filterAuthorizedTools({ tools: [ @@ -321,6 +404,7 @@ describe('MCP Tool Authorization', () => { 'web_search', ], userId, + user: testUser, availableTools, }); @@ -348,6 +432,27 @@ describe('MCP Tool Authorization', () => { expect(agent.tools).not.toContain(`attack${d}forbiddenServer`); }); + test('should strip all MCP tools on create when user lacks MCP server use permission', async () => { + mockUserCanUseMCPServers.mockResolvedValue(false); + mockReq.body = { + provider: 'openai', + model: 'gpt-4', + name: 'MCP Denied Test Agent', + tools: [ + 'web_search', + `validTool${d}authorizedServer`, + `${Constants.mcp_all}${d}authorizedServer`, + ], + }; + + await createAgentHandler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(201); + const agent = mockRes.json.mock.calls[0][0]; + expect(agent.tools).toEqual(['web_search']); + expect(agent.mcpServerNames).toEqual([]); + }); + test('should not 500 when MCP registry is uninitialized', async () => { getMCPServersRegistry.mockImplementation(() => { throw new Error('MCPServersRegistry has not been initialized.'); @@ -446,6 +551,129 @@ describe('MCP Tool Authorization', () => { expect(updatedAgent.tools).not.toContain(`attack${d}forbiddenServer`); }); + test('should strip all MCP tools, including retained ones, when user lacks MCP server use permission', async () => { + mockUserCanUseMCPServers.mockResolvedValue(false); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + tools: ['web_search', `existingTool${d}authorizedServer`, `newTool${d}anotherServer`], + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + const updatedAgent = mockRes.json.mock.calls[0][0]; + // Permission revoked: update must not preserve stale MCP bindings, matching + // the create/duplicate/revert paths. + expect(updatedAgent.tools).toEqual(['web_search']); + expect(mockGetAllServerConfigs).not.toHaveBeenCalled(); + }); + + test('should strip retained MCP tools on an unrelated owner edit after permission revocation', async () => { + mockUserCanUseMCPServers.mockResolvedValue(false); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + name: 'Renamed After Revocation', + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + const updatedAgent = mockRes.json.mock.calls[0][0]; + expect(updatedAgent.tools).toEqual(['web_search']); + expect(updatedAgent.name).toBe('Renamed After Revocation'); + }); + + test('should not strip shared agent MCP tools on unrelated editor changes after revocation', async () => { + mockUserCanUseMCPServers.mockResolvedValue(false); + mockReq.user.id = new mongoose.Types.ObjectId().toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + name: 'Shared Rename After Revocation', + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + const updatedAgent = mockRes.json.mock.calls[0][0]; + const agentInDb = await Agent.findOne({ id: existingAgentId }); + expect(updatedAgent.tools).toContain(`existingTool${d}authorizedServer`); + expect(updatedAgent.name).toBe('Shared Rename After Revocation'); + expect(agentInDb.tools).toContain(`existingTool${d}authorizedServer`); + expect(agentInDb.mcpServerNames).toEqual(['authorizedServer']); + }); + + test('should not strip shared agent MCP tools on frontend-style full tools save after revocation', async () => { + mockUserCanUseMCPServers.mockResolvedValue(false); + mockReq.user.id = new mongoose.Types.ObjectId().toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + name: 'Shared Full Save After Revocation', + tools: ['web_search', `existingTool${d}authorizedServer`], + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + const updatedAgent = mockRes.json.mock.calls[0][0]; + const agentInDb = await Agent.findOne({ id: existingAgentId }); + expect(updatedAgent.tools).toContain(`existingTool${d}authorizedServer`); + expect(updatedAgent.name).toBe('Shared Full Save After Revocation'); + expect(agentInDb.tools).toContain(`existingTool${d}authorizedServer`); + expect(agentInDb.mcpServerNames).toEqual(['authorizedServer']); + expect(mockGetAllServerConfigs).not.toHaveBeenCalled(); + }); + + test('should reject new shared-agent MCP tools after revocation while retaining existing MCP tools', async () => { + mockUserCanUseMCPServers.mockResolvedValue(false); + mockReq.user.id = new mongoose.Types.ObjectId().toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + tools: ['web_search', `existingTool${d}authorizedServer`, `newTool${d}anotherServer`], + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + const updatedAgent = mockRes.json.mock.calls[0][0]; + const agentInDb = await Agent.findOne({ id: existingAgentId }); + expect(updatedAgent.tools).toContain(`existingTool${d}authorizedServer`); + expect(updatedAgent.tools).not.toContain(`newTool${d}anotherServer`); + expect(agentInDb.tools).toContain(`existingTool${d}authorizedServer`); + expect(agentInDb.tools).not.toContain(`newTool${d}anotherServer`); + expect(agentInDb.mcpServerNames).toEqual(['authorizedServer']); + expect(mockGetAllServerConfigs).not.toHaveBeenCalled(); + }); + + test('should not strip action tools whose operationId contains the MCP delimiter on revocation', async () => { + // `sync_mcp_state_action_...` contains the `_mcp_` substring but is a + // genuine OpenAPI action tool (isActionTool === true). Losing + // MCP_SERVERS.USE must not drop it — action use is unrelated to MCP. + const actionTool = `sync_mcp_state${actionDelimiter}api---example---com`; + await Agent.updateOne( + { id: existingAgentId }, + { $set: { tools: ['web_search', actionTool] } }, + ); + + mockUserCanUseMCPServers.mockResolvedValue(false); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + name: 'Edited Without MCP Permission', + tools: ['web_search', actionTool], + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + const updatedAgent = mockRes.json.mock.calls[0][0]; + const agentInDb = await Agent.findOne({ id: existingAgentId }); + expect(updatedAgent.tools).toContain(actionTool); + expect(updatedAgent.tools).toContain('web_search'); + expect(agentInDb.mcpServerNames).toEqual([]); + }); + test('should allow adding authorized MCP tools', async () => { mockReq.user.id = existingAgentAuthorId.toString(); mockReq.params.id = existingAgentId; diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index 95bf5c6246..79ebbbd2ae 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -24,6 +24,7 @@ const { AccessRoleIds, PrincipalType, EToolResources, + isActionTool, PermissionBits, actionDelimiter, AgentCapabilities, @@ -42,7 +43,7 @@ const { resizeAvatar } = require('~/server/services/Files/images/avatar'); const { getFileStrategy } = require('~/server/utils/getFileStrategy'); const { filterFile } = require('~/server/services/Files/process'); const { getCachedTools } = require('~/server/services/Config'); -const { resolveConfigServers } = require('~/server/services/MCP'); +const { resolveConfigServers, userCanUseMCPServers } = require('~/server/services/MCP'); const { getMCPServersRegistry } = require('~/config'); const { getLogStores } = require('~/cache'); const db = require('~/models'); @@ -190,6 +191,7 @@ const isSubagentsCapabilityEnabled = (req) => { * @param {string[]} params.tools - Raw tool strings from the request * @param {string} params.userId - Requesting user ID for MCP server access check * @param {string} [params.role] - Requesting user's role for ACL principal resolution + * @param {object} [params.user] - Requesting user for MCP server use permission checks * @param {Record} params.availableTools - Global non-MCP tool cache * @param {string[]} [params.existingTools] - Tools already persisted on the agent document * @param {Record} [params.configServers] - Config-source MCP servers resolved from appConfig overrides @@ -199,6 +201,7 @@ const filterAuthorizedTools = async ({ tools, userId, role, + user, availableTools, existingTools, configServers, @@ -207,14 +210,26 @@ const filterAuthorizedTools = async ({ let mcpServerConfigs; let registryUnavailable = false; const existingToolSet = existingTools?.length ? new Set(existingTools) : null; + const hasMCPTools = tools.some((tool) => tool?.includes(Constants.mcp_delimiter)); + const canUseMCP = hasMCPTools ? await userCanUseMCPServers(user) : true; + let loggedMCPDenied = false; for (const tool of tools) { - if (availableTools[tool] || systemTools[tool]) { - filteredTools.push(tool); + const isActionToolName = typeof tool === 'string' && isActionTool(tool); + const isMCPTool = tool?.includes(Constants.mcp_delimiter) && !isActionToolName; + + if (!isMCPTool) { + if (availableTools[tool] || systemTools[tool] || isActionToolName) { + filteredTools.push(tool); + } continue; } - if (!tool?.includes(Constants.mcp_delimiter)) { + if (!canUseMCP) { + if (!loggedMCPDenied) { + logger.warn(`[filterAuthorizedTools] User ${userId} lacks MCP server use permission`); + loggedMCPDenied = true; + } continue; } @@ -392,6 +407,7 @@ const createAgentHandler = async (req, res) => { tools, userId, role: req.user.role, + user: req.user, availableTools, configServers, }); @@ -611,27 +627,53 @@ const updateAgentHandler = async (req, res) => { }); } - if (updateData.tools) { - const existingToolSet = new Set(existingAgent.tools ?? []); - const newMCPTools = updateData.tools.filter( - (t) => !existingToolSet.has(t) && t?.includes(Constants.mcp_delimiter), - ); + const isMCPTool = (t) => + typeof t === 'string' && t.includes(Constants.mcp_delimiter) && !isActionTool(t); + const hasToolUpdate = updateData.tools !== undefined; + const editingOwnAgent = existingAgent.author?.toString() === req.user.id; + const existingTools = existingAgent.tools ?? []; + const effectiveTools = (hasToolUpdate ? updateData.tools : existingAgent.tools) ?? []; + const requestedMCPTools = effectiveTools.filter(isMCPTool); + const existingMCPTools = existingTools.filter(isMCPTool); - if (newMCPTools.length > 0) { - const [availableTools, configServers] = await Promise.all([ - getCachedTools().then((t) => t ?? {}), - resolveConfigServers(req), - ]); - const approvedNew = await filterAuthorizedTools({ - tools: newMCPTools, - userId: req.user.id, - role: req.user.role, - availableTools, - configServers, - }); - const rejectedSet = new Set(newMCPTools.filter((t) => !approvedNew.includes(t))); - if (rejectedSet.size > 0) { - updateData.tools = updateData.tools.filter((t) => !rejectedSet.has(t)); + if (requestedMCPTools.length > 0 || (hasToolUpdate && existingMCPTools.length > 0)) { + if (!(await userCanUseMCPServers(req.user))) { + if (editingOwnAgent) { + updateData.tools = effectiveTools.filter((t) => !isMCPTool(t)); + } else if (hasToolUpdate) { + const existingMCPToolSet = new Set(existingMCPTools); + const nextTools = updateData.tools.filter( + (t) => !isMCPTool(t) || existingMCPToolSet.has(t), + ); + const nextToolSet = new Set(nextTools); + for (const existingMCPTool of existingMCPTools) { + if (!nextToolSet.has(existingMCPTool)) { + nextTools.push(existingMCPTool); + } + } + updateData.tools = nextTools; + } + } else if (hasToolUpdate) { + const existingToolSet = new Set(existingTools); + const newMCPTools = requestedMCPTools.filter((t) => !existingToolSet.has(t)); + + if (newMCPTools.length > 0) { + const [availableTools, configServers] = await Promise.all([ + getCachedTools().then((t) => t ?? {}), + resolveConfigServers(req), + ]); + const approvedNew = await filterAuthorizedTools({ + tools: newMCPTools, + userId: req.user.id, + role: req.user.role, + user: req.user, + availableTools, + configServers, + }); + const rejectedSet = new Set(newMCPTools.filter((t) => !approvedNew.includes(t))); + if (rejectedSet.size > 0) { + updateData.tools = updateData.tools.filter((t) => !rejectedSet.has(t)); + } } } } @@ -788,6 +830,7 @@ const duplicateAgentHandler = async (req, res) => { tools: newAgentData.tools, userId, role: req.user.role, + user: req.user, availableTools, existingTools: newAgentData.tools, configServers, @@ -1163,6 +1206,7 @@ const revertAgentVersionHandler = async (req, res) => { tools: updatedAgent.tools, userId: req.user.id, role: req.user.role, + user: req.user, availableTools, existingTools: updatedAgent.tools, configServers, diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index 3a3e6ac796..b86d339dac 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -25,6 +25,7 @@ const mockRegistryInstance = { getAllowedDomains: jest.fn().mockReturnValue(null), getAllowedAddresses: jest.fn().mockReturnValue(null), }; +let mockMCPUseAllowed = true; jest.mock('@librechat/api', () => { const actual = jest.requireActual('@librechat/api'); @@ -46,7 +47,15 @@ jest.mock('@librechat/api', () => { deleteUserTokens: jest.fn(), }, getUserMCPAuthMap: jest.fn(), - generateCheckAccess: jest.fn(() => (req, res, next) => next()), + generateCheckAccess: jest.fn(({ permissionType, permissions }) => (req, res, next) => { + const { PermissionTypes, Permissions } = require('librechat-data-provider'); + const isMCPUseCheck = + permissionType === PermissionTypes.MCP_SERVERS && permissions.includes(Permissions.USE); + if (isMCPUseCheck && !mockMCPUseAllowed) { + return res.status(403).json({ message: 'Forbidden: Insufficient permissions' }); + } + return next(); + }), MCPServersRegistry: { getInstance: () => mockRegistryInstance, }, @@ -175,6 +184,7 @@ describe('MCP Routes', () => { jest.clearAllMocks(); mockResolveAllMcpConfigs.mockResolvedValue({}); mockResolveMcpConfigNames.mockResolvedValue([]); + mockMCPUseAllowed = true; }); describe('GET /:serverName/oauth/initiate', () => { @@ -1957,6 +1967,16 @@ describe('MCP Routes', () => { }); describe('GET /tools', () => { + it('should deny MCP tools when user lacks MCP server use permission', async () => { + mockMCPUseAllowed = false; + + const response = await request(app).get('/api/mcp/tools'); + + expect(response.status).toBe(403); + expect(response.body).toEqual({ message: 'Forbidden: Insufficient permissions' }); + expect(mockResolveAllMcpConfigs).not.toHaveBeenCalled(); + }); + it('should continue returning MCP tools when one server cache lookup fails', async () => { const { Constants } = require('librechat-data-provider'); const { logger } = require('@librechat/data-schemas'); diff --git a/api/server/routes/mcp.js b/api/server/routes/mcp.js index 219f6455dc..09d6b7d49a 100644 --- a/api/server/routes/mcp.js +++ b/api/server/routes/mcp.js @@ -68,7 +68,7 @@ const checkMCPCreate = generateCheckAccess({ * Get all MCP tools available to the user * Returns only MCP tools, completely decoupled from regular LibreChat tools */ -router.get('/tools', requireJwtAuth, async (req, res) => { +router.get('/tools', requireJwtAuth, checkMCPUsePermissions, async (req, res) => { return getMCPTools(req, res); }); diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index 85c29ddfdc..2d81d66070 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -7,6 +7,7 @@ const { Constants: AgentConstants, } = require('@librechat/agents'); const { + checkAccess, sendEvent, MCPOAuthHandler, isMCPDomainAllowed, @@ -16,14 +17,22 @@ const { resolveJsonSchemaRefs, buildOAuthToolCallName, } = require('@librechat/api'); -const { Time, CacheKeys, Constants, isAssistantsEndpoint } = require('librechat-data-provider'); +const { + Time, + CacheKeys, + Constants, + Permissions, + PermissionTypes, + isAssistantsEndpoint, +} = require('librechat-data-provider'); const { getOAuthReconnectionManager, getMCPServersRegistry, getFlowStateManager, getMCPManager, } = require('~/config'); -const { findToken, createToken, updateToken, deleteTokens } = require('~/models'); +const db = require('~/models'); +const { findToken, createToken, updateToken, deleteTokens } = db; const { getGraphApiToken } = require('./GraphTokenService'); const { reinitMCPServer } = require('./Tools/mcp'); const { getAppConfig } = require('./Config'); @@ -36,6 +45,24 @@ const RECONNECT_THROTTLE_MS = 10_000; const missingToolCache = new Map(); const MISSING_TOOL_TTL_MS = 10_000; +async function userCanUseMCPServers(user) { + if (!user?.id || !user?.role) { + return false; + } + + try { + return await checkAccess({ + user, + permissionType: PermissionTypes.MCP_SERVERS, + permissions: [Permissions.USE], + getRoleByName: db.getRoleByName, + }); + } catch (error) { + logger.error(`[MCP][User: ${user.id}] Failed MCP permission check`, error); + return false; + } +} + function evictStale(map, ttl) { if (map.size <= MAX_CACHE_SIZE) { return; @@ -586,6 +613,7 @@ async function createMCPTool({ return createToolInstance({ res, + user, provider, toolName, serverName, @@ -597,6 +625,7 @@ async function createMCPTool({ function createToolInstance({ res, + user: capturedUser = null, toolName, serverName, serverConfig: capturedServerConfig, @@ -624,18 +653,24 @@ function createToolInstance({ /** @type {(toolArguments: Object | string, config?: GraphRunnableConfig) => Promise} */ const _call = async (toolArguments, config) => { - const userId = config?.configurable?.user?.id || config?.configurable?.user_id; + const permissionUser = config?.configurable?.user ?? capturedUser; + const userId = + config?.configurable?.user?.id || config?.configurable?.user_id || capturedUser?.id; /** @type {ReturnType} */ let abortHandler = null; /** @type {AbortSignal} */ let derivedSignal = null; try { + const provider = (config?.metadata?.provider || capturedProvider)?.toLowerCase(); + const canUseMCP = await userCanUseMCPServers(permissionUser); + if (!canUseMCP) { + throw new Error('Forbidden: Insufficient MCP server permissions'); + } const flowsCache = getLogStores(CacheKeys.FLOWS); const flowManager = getFlowStateManager(flowsCache); derivedSignal = config?.signal ? AbortSignal.any([config.signal]) : undefined; const mcpManager = getMCPManager(userId); - const provider = (config?.metadata?.provider || capturedProvider)?.toLowerCase(); const { args: _args, stepId, ...toolCall } = config.toolCall ?? {}; const flowId = `${serverName}:oauth_login:${config.metadata.thread_id}:${config.metadata.run_id}`; @@ -888,6 +923,7 @@ async function getServerConnectionStatus( module.exports = { createMCPTool, createMCPTools, + userCanUseMCPServers, getMCPSetupData, resolveConfigServers, resolveMcpConfigNames, diff --git a/api/server/services/MCP.spec.js b/api/server/services/MCP.spec.js index 8b9f055ff8..31d8055239 100644 --- a/api/server/services/MCP.spec.js +++ b/api/server/services/MCP.spec.js @@ -38,7 +38,7 @@ jest.mock('@librechat/api', () => { const { logger } = require('@librechat/data-schemas'); const { MCPOAuthHandler } = require('@librechat/api'); -const { CacheKeys, Constants } = require('librechat-data-provider'); +const { CacheKeys, Constants, Permissions, PermissionTypes } = require('librechat-data-provider'); const D = Constants.mcp_delimiter; const { createMCPTool, @@ -71,6 +71,8 @@ jest.mock('~/models', () => ({ findToken: jest.fn(), createToken: jest.fn(), updateToken: jest.fn(), + deleteTokens: jest.fn(), + getRoleByName: jest.fn(), })); jest.mock('./Tools/mcp', () => ({ @@ -660,12 +662,14 @@ describe('tests for the new helper functions used by the MCP connection status e describe('User parameter passing tests', () => { let mockReinitMCPServer; + let mockGetMCPManager; let mockGetFlowStateManager; let mockGetLogStores; beforeEach(() => { jest.clearAllMocks(); mockReinitMCPServer = require('./Tools/mcp').reinitMCPServer; + mockGetMCPManager = require('~/config').getMCPManager; mockGetFlowStateManager = require('~/config').getFlowStateManager; mockGetLogStores = require('~/cache').getLogStores; @@ -813,6 +817,53 @@ describe('User parameter passing tests', () => { // Verify reinitMCPServer was NOT called since tool was in cache expect(mockReinitMCPServer).not.toHaveBeenCalled(); }); + + it('should reject tool execution when user lacks MCP server use permission', async () => { + const mockUser = { id: 'mcp-denied-user', role: 'USER' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + const { getRoleByName } = require('~/models'); + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: false, + }, + }, + }); + + const mcpTool = await createMCPTool({ + res: mockRes, + user: mockUser, + toolKey: `test-tool${D}test-server`, + provider: 'openai', + userMCPAuthMap: {}, + availableTools: { + [`test-tool${D}test-server`]: { + function: { + description: 'Cached tool', + parameters: { type: 'object', properties: {} }, + }, + }, + }, + }); + + await expect( + mcpTool.invoke( + {}, + { + configurable: { + user: mockUser, + }, + metadata: { + provider: 'openai', + }, + toolCall: {}, + }, + ), + ).rejects.toThrow( + '[MCP][test-server][test-tool] tool call failed: Forbidden: Insufficient MCP server permissions', + ); + expect(mockGetMCPManager).not.toHaveBeenCalled(); + }); }); describe('reinitMCPServer (via reconnectServer)', () => { diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 83e47e445b..261ac42547 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -63,7 +63,7 @@ const { primeFiles: primeCodeFiles } = require('~/server/services/Files/Code/pro const { manifestToolMap, toolkits } = require('~/app/clients/tools/manifest'); const { createOnSearchResults } = require('~/server/services/Tools/search'); const { reinitMCPServer } = require('~/server/services/Tools/mcp'); -const { resolveConfigServers } = require('~/server/services/MCP'); +const { resolveConfigServers, userCanUseMCPServers } = require('~/server/services/MCP'); const { recordUsage } = require('~/server/services/Threads'); const { loadTools } = require('~/app/clients/tools/util'); const { redactMessage } = require('~/config/parsers'); @@ -551,6 +551,8 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to const codeExecutionEnabled = agent.tools?.includes(Tools.execute_code) === true && enabledCapabilities.has(AgentCapabilities.execute_code); + const hasMCPTools = agent.tools?.some((tool) => tool?.includes(Constants.mcp_delimiter)); + const canUseMCP = hasMCPTools ? await userCanUseMCPServers(req.user) : true; const filteredTools = agent.tools?.filter((tool) => { if (tool === Tools.file_search) { @@ -565,6 +567,9 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to if (isActionTool(tool)) { return actionsEnabled; } + if (tool?.includes(Constants.mcp_delimiter)) { + return areToolsEnabled && canUseMCP; + } if (!areToolsEnabled) { return false; } @@ -577,9 +582,9 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to /** @type {Record>} */ let userMCPAuthMap; - if (agent.tools?.some((t) => t.includes(Constants.mcp_delimiter))) { + if (filteredTools?.some((t) => t.includes(Constants.mcp_delimiter))) { userMCPAuthMap = await getUserMCPAuthMap({ - tools: agent.tools, + tools: filteredTools, userId: req.user.id, findPluginAuthsByKeys, }); @@ -983,6 +988,8 @@ async function loadAgentTools({ }; const areToolsEnabled = checkCapability(AgentCapabilities.tools); const actionsEnabled = checkCapability(AgentCapabilities.actions); + const hasMCPTools = agent.tools?.some((tool) => tool?.includes(Constants.mcp_delimiter)); + const canUseMCP = hasMCPTools ? await userCanUseMCPServers(req.user) : true; let includesWebSearch = false; const _agentTools = agent.tools?.filter((tool) => { @@ -995,6 +1002,8 @@ async function loadAgentTools({ return includesWebSearch; } else if (isActionTool(tool)) { return actionsEnabled; + } else if (tool?.includes(Constants.mcp_delimiter)) { + return areToolsEnabled && canUseMCP; } else if (!areToolsEnabled) { return false; } @@ -1012,9 +1021,9 @@ async function loadAgentTools({ /** @type {Record>} */ let userMCPAuthMap; - if (agent.tools?.some((t) => t.includes(Constants.mcp_delimiter))) { + if (_agentTools?.some((t) => t.includes(Constants.mcp_delimiter))) { userMCPAuthMap = await getUserMCPAuthMap({ - tools: agent.tools, + tools: _agentTools, userId: req.user.id, findPluginAuthsByKeys, }); diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index ab95b389ca..451120eaa3 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -77,6 +77,7 @@ jest.mock('~/config', () => ({ })); jest.mock('~/server/services/MCP', () => ({ resolveConfigServers: (...args) => mockResolveConfigServers(...args), + userCanUseMCPServers: jest.fn().mockResolvedValue(true), })); jest.mock('~/cache', () => ({ getLogStores: jest.fn(() => ({})), @@ -255,6 +256,28 @@ describe('ToolService - Action Capability Gating', () => { expect(callArgs.tools).toContain(regularTool); }); + it('should filter MCP tool definitions when user lacks MCP server use permission', async () => { + const { userCanUseMCPServers } = require('~/server/services/MCP'); + userCanUseMCPServers.mockResolvedValueOnce(false); + + const mcpTool = `search${Constants.mcp_delimiter}myserver`; + const capabilities = [AgentCapabilities.tools]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + await loadAgentTools({ + req, + res: {}, + agent: { id: 'agent_123', tools: [regularTool, mcpTool] }, + definitionsOnly: true, + }); + + expect(mockLoadToolDefinitions).toHaveBeenCalledTimes(1); + const [callArgs] = mockLoadToolDefinitions.mock.calls[0]; + expect(callArgs.tools).toContain(regularTool); + expect(callArgs.tools).not.toContain(mcpTool); + }); + it('should return actionsEnabled in the result', async () => { const capabilities = [AgentCapabilities.tools]; const req = createMockReq(capabilities); diff --git a/packages/api/src/agents/openai/service.spec.ts b/packages/api/src/agents/openai/service.spec.ts new file mode 100644 index 0000000000..542ea54b6a --- /dev/null +++ b/packages/api/src/agents/openai/service.spec.ts @@ -0,0 +1,107 @@ +import { createAgentChatCompletion } from './service'; +import type { ChatCompletionDependencies } from './service'; + +jest.mock('@librechat/data-schemas', () => ({ + logger: { + debug: jest.fn(), + error: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + }, +})); + +type CreateRunArgs = { user?: Record }; +type ProcessStreamConfig = { configurable?: Record }; + +function createMockReq(user?: Record) { + return { + body: { + model: 'agent_test', + messages: [{ role: 'user', content: 'hi' }], + stream: false, + }, + user, + on: jest.fn(), + } as unknown as Parameters[0]; +} + +function createMockRes() { + const res: Record = { + setHeader: jest.fn(), + flushHeaders: jest.fn(), + write: jest.fn(), + end: jest.fn(), + headersSent: false, + }; + res.status = jest.fn(() => res); + res.json = jest.fn(() => res); + return res as unknown as Parameters[1]; +} + +describe('createAgentChatCompletion - MCP permission user propagation', () => { + let createRun: jest.Mock; + let processStream: jest.Mock; + let deps: ChatCompletionDependencies; + + beforeEach(() => { + processStream = jest.fn().mockResolvedValue(undefined); + createRun = jest.fn().mockResolvedValue({ processStream }); + + deps = { + getAgent: jest.fn().mockResolvedValue({ + id: 'agent_test', + provider: 'openai', + model: 'gpt-4o-mini', + tools: [], + }), + initializeAgent: jest.fn().mockResolvedValue({ + id: 'agent_test', + provider: 'openai', + model: 'gpt-4o-mini', + tools: [], + attachments: [], + toolContextMap: {}, + maxContextTokens: 1000, + model_parameters: {}, + }), + createRun: createRun as unknown as ChatCompletionDependencies['createRun'], + }; + }); + + it('forwards the role-bearing safe user to createRun and configurable.user', async () => { + const req = createMockReq({ + id: 'user-123', + role: 'ADMIN', + email: 'admin@example.com', + password: 'secret', + }); + + await createAgentChatCompletion(req, createMockRes(), deps); + + expect(createRun).toHaveBeenCalledTimes(1); + const runArgs = createRun.mock.calls[0][0] as CreateRunArgs; + expect(runArgs.user).toMatchObject({ id: 'user-123', role: 'ADMIN' }); + // createSafeUser must strip sensitive fields. + expect(runArgs.user).not.toHaveProperty('password'); + + expect(processStream).toHaveBeenCalledTimes(1); + const streamConfig = processStream.mock.calls[0][1] as ProcessStreamConfig; + expect(streamConfig.configurable?.user).toMatchObject({ id: 'user-123', role: 'ADMIN' }); + expect(streamConfig.configurable?.user_id).toBe('user-123'); + }); + + it('falls back to a bare id when no authenticated user is attached', async () => { + const req = createMockReq(undefined); + + await createAgentChatCompletion(req, createMockRes(), deps); + + expect(createRun).toHaveBeenCalledTimes(1); + const runArgs = createRun.mock.calls[0][0] as CreateRunArgs; + expect(runArgs.user).toEqual({ id: 'api-user' }); + + const streamConfig = processStream.mock.calls[0][1] as ProcessStreamConfig; + // No role present → the runtime MCP check fails closed. + expect(streamConfig.configurable?.user).toEqual({ id: 'api-user' }); + expect(streamConfig.configurable?.user).not.toHaveProperty('role'); + }); +}); diff --git a/packages/api/src/agents/openai/service.ts b/packages/api/src/agents/openai/service.ts index 0d637f7a50..dd8688d051 100644 --- a/packages/api/src/agents/openai/service.ts +++ b/packages/api/src/agents/openai/service.ts @@ -39,6 +39,7 @@ import { createChunk, writeSSE, } from './handlers'; +import { createSafeUser } from '~/utils'; import type { ToolExecuteOptions } from '../handlers'; /** @@ -500,7 +501,17 @@ export async function createAgentChatCompletion( // Create and run the agent if (deps.createRun) { - const userId = (req as unknown as { user?: { id?: string } }).user?.id ?? 'api-user'; + const reqUser = (req as unknown as { user?: Parameters[0] }).user; + const userId = reqUser?.id ?? 'api-user'; + /** + * Propagate the full safe user (id + role), matching the in-repo agent + * controllers (responses.js / openai.js). The runtime MCP permission + * check reads `configurable.user`; passing only `user_id` would make + * every MCP tool call fail closed for an authenticated caller. When the + * host app didn't attach a user, this falls back to a bare id, which + * correctly leaves MCP gated. + */ + const safeUser: Record = { ...createSafeUser(reqUser), id: userId }; const run = await deps.createRun({ agents: [initializedAgent], @@ -512,7 +523,7 @@ export async function createAgentChatCompletion( messageId: requestId, conversationId, }, - user: { id: userId }, + user: safeUser, }); if (run) { @@ -523,6 +534,7 @@ export async function createAgentChatCompletion( configurable: { thread_id: conversationId, user_id: userId, + user: safeUser, }, signal: abortController.signal, streamMode: 'values', diff --git a/packages/api/src/mcp/__tests__/parsers.test.ts b/packages/api/src/mcp/__tests__/parsers.test.ts index afc3fd9de3..3d1c71edbd 100644 --- a/packages/api/src/mcp/__tests__/parsers.test.ts +++ b/packages/api/src/mcp/__tests__/parsers.test.ts @@ -29,6 +29,18 @@ describe('formatToolContent', () => { expect(content).toBe('(No response)'); expect(artifacts).toBeUndefined(); }); + + it('should preserve the image payload in the string for unrecognized providers', () => { + const result: t.MCPToolCallResponse = { + content: [{ type: 'image', data: 'iVBORw0KGgoAAAA...', mimeType: 'image/png' }], + }; + + const [content, artifacts] = formatToolContent(result, 'unknown' as t.Provider); + + expect(artifacts).toBeUndefined(); + expect(content).toContain('iVBORw0KGgoAAAA...'); + expect(content).toContain('image/png'); + }); }); describe('recognized providers', () => { @@ -91,6 +103,16 @@ describe('formatToolContent', () => { }); describe('image handling', () => { + const originalMaxImageBytes = process.env.MCP_IMAGE_DATA_MAX_BYTES; + + afterEach(() => { + if (originalMaxImageBytes === undefined) { + delete process.env.MCP_IMAGE_DATA_MAX_BYTES; + return; + } + process.env.MCP_IMAGE_DATA_MAX_BYTES = originalMaxImageBytes; + }); + it('should handle images with http URLs', () => { const result: t.MCPToolCallResponse = { content: [{ type: 'image', data: 'https://example.com/image.png', mimeType: 'image/png' }], @@ -147,6 +169,83 @@ describe('formatToolContent', () => { expect(artifacts).toBeDefined(); expect(artifacts?.content).toHaveLength(2); }); + + it('should reject oversized base64 image data before creating artifacts', () => { + process.env.MCP_IMAGE_DATA_MAX_BYTES = '3'; + const result: t.MCPToolCallResponse = { + content: [{ type: 'image', data: 'QUJDRA==', mimeType: 'image/png' }], + }; + + expect(() => formatToolContent(result, 'openai')).toThrow( + 'MCP image result exceeds maximum size of 3 bytes', + ); + }); + + it('should allow base64 image data when decoded size is within the cap', () => { + process.env.MCP_IMAGE_DATA_MAX_BYTES = '4'; + const result: t.MCPToolCallResponse = { + content: [{ type: 'image', data: 'QUJDRA==', mimeType: 'image/png' }], + }; + + const [content, artifacts] = formatToolContent(result, 'openai'); + + expect(content).toBe(''); + expect(artifacts?.content?.[0]).toEqual({ + type: 'image_url', + image_url: { url: 'data:image/png;base64,QUJDRA==' }, + }); + }); + + it('should reject oversized image data for unrecognized providers before stringifying', () => { + process.env.MCP_IMAGE_DATA_MAX_BYTES = '3'; + const result: t.MCPToolCallResponse = { + content: [{ type: 'image', data: 'QUJDRA==', mimeType: 'image/png' }], + }; + + expect(() => formatToolContent(result, 'unknown' as t.Provider)).toThrow( + 'MCP image result exceeds maximum size of 3 bytes', + ); + }); + + it('should not apply the image data cap to remote image URLs', () => { + process.env.MCP_IMAGE_DATA_MAX_BYTES = '3'; + const result: t.MCPToolCallResponse = { + content: [{ type: 'image', data: 'https://example.com/large.png', mimeType: 'image/png' }], + }; + + const [content, artifacts] = formatToolContent(result, 'openai'); + + expect(content).toBe(''); + expect(artifacts?.content?.[0]).toEqual({ + type: 'image_url', + image_url: { url: 'https://example.com/large.png' }, + }); + }); + + it('should enforce the image cap on base64 data that merely starts with "http"', () => { + process.env.MCP_IMAGE_DATA_MAX_BYTES = '3'; + const result: t.MCPToolCallResponse = { + content: [{ type: 'image', data: 'httpAAAAAAAA', mimeType: 'image/png' }], + }; + + expect(() => formatToolContent(result, 'openai')).toThrow( + 'MCP image result exceeds maximum size of 3 bytes', + ); + }); + + it('should treat base64 starting with "http" as inline data, not a remote URL', () => { + const result: t.MCPToolCallResponse = { + content: [{ type: 'image', data: 'httpAAAA', mimeType: 'image/png' }], + }; + + const [content, artifacts] = formatToolContent(result, 'openai'); + + expect(content).toBe(''); + expect(artifacts?.content?.[0]).toEqual({ + type: 'image_url', + image_url: { url: 'data:image/png;base64,httpAAAA' }, + }); + }); }); describe('resource handling', () => { diff --git a/packages/api/src/mcp/parsers.ts b/packages/api/src/mcp/parsers.ts index c9b824e782..04065f62ad 100644 --- a/packages/api/src/mcp/parsers.ts +++ b/packages/api/src/mcp/parsers.ts @@ -3,10 +3,57 @@ import { Tools } from 'librechat-data-provider'; import type { UIResource } from 'librechat-data-provider'; import type * as t from './types'; +export const DEFAULT_MCP_IMAGE_DATA_MAX_BYTES = 10 * 1024 * 1024; + function generateResourceId(text: string): string { return crypto.createHash('sha256').update(text).digest('hex').substring(0, 10); } +function getMCPImageDataMaxBytes(): number { + const raw = process.env.MCP_IMAGE_DATA_MAX_BYTES; + if (!raw) { + return DEFAULT_MCP_IMAGE_DATA_MAX_BYTES; + } + + const parsed = Number(raw); + return Number.isSafeInteger(parsed) && parsed > 0 ? parsed : DEFAULT_MCP_IMAGE_DATA_MAX_BYTES; +} + +function getBase64Padding(data: string): number { + if (data.endsWith('==')) { + return 2; + } + if (data.endsWith('=')) { + return 1; + } + return 0; +} + +function estimateBase64ImageBytes(data: string): number { + const padding = getBase64Padding(data); + return Math.max(0, Math.floor((data.length * 3) / 4) - padding); +} + +function isRemoteImageUrl(data: string): boolean { + return data.startsWith('http://') || data.startsWith('https://'); +} + +function assertImageDataWithinLimit(item: t.ImageContent): void { + if (isRemoteImageUrl(item.data)) { + return; + } + + const maxBytes = getMCPImageDataMaxBytes(); + const estimatedBytes = estimateBase64ImageBytes(item.data); + if (estimatedBytes <= maxBytes) { + return; + } + + throw new Error( + `MCP image result exceeds maximum size of ${maxBytes} bytes: ${estimatedBytes} bytes`, + ); +} + const RECOGNIZED_PROVIDERS = new Set([ 'google', 'anthropic', @@ -38,7 +85,7 @@ const imageFormatters: Record = { default: (item) => ({ type: 'image_url', image_url: { - url: item.data.startsWith('http') ? item.data : `data:${item.mimeType};base64,${item.data}`, + url: isRemoteImageUrl(item.data) ? item.data : `data:${item.mimeType};base64,${item.data}`, }, }), }; @@ -71,6 +118,9 @@ function parseAsString(result: t.MCPToolCallResponse): string { } return resourceText.join('\n'); } + if (isImageContent(item)) { + assertImageDataWithinLimit(item); + } return JSON.stringify(item, null, 2); }) .filter(Boolean) @@ -120,6 +170,7 @@ export function formatToolContent( if (!isImageContent(item)) { return; } + assertImageDataWithinLimit(item); const formatter = imageFormatters.default as t.ImageFormatter; const formattedImage = formatter(item); diff --git a/packages/data-schemas/src/methods/agent.spec.ts b/packages/data-schemas/src/methods/agent.spec.ts index 2ae40cf89b..eb5a15fedc 100644 --- a/packages/data-schemas/src/methods/agent.spec.ts +++ b/packages/data-schemas/src/methods/agent.spec.ts @@ -8,6 +8,8 @@ import { PrincipalModel, PermissionBits, EToolResources, + Constants, + actionDelimiter, } from 'librechat-data-provider'; import type { UpdateWithAggregationPipeline, @@ -520,6 +522,42 @@ describe('Agent Methods', () => { expect(retrievedAgent!.description).toBe('Test description'); }); + test('should derive mcpServerNames only from MCP tools on create', async () => { + const { agentId, authorId } = createTestIds(); + const actionTool = `sync${Constants.mcp_delimiter}state${actionDelimiter}api---example---com`; + const mcpTool = `search${Constants.mcp_delimiter}authorizedServer`; + + const newAgent = await createAgent({ + id: agentId, + name: 'MCP Names Agent', + provider: 'test', + model: 'test-model', + author: authorId, + tools: [actionTool, mcpTool], + }); + + expect(newAgent.mcpServerNames).toEqual(['authorizedServer']); + }); + + test('should derive mcpServerNames only from MCP tools on update', async () => { + const { agentId, authorId } = createTestIds(); + const actionTool = `sync${Constants.mcp_delimiter}state${actionDelimiter}api---example---com`; + const mcpTool = `search${Constants.mcp_delimiter}authorizedServer`; + + await createAgent({ + id: agentId, + name: 'MCP Names Agent', + provider: 'test', + model: 'test-model', + author: authorId, + tools: [], + }); + + const updatedAgent = await updateAgent({ id: agentId }, { tools: [actionTool, mcpTool] }); + + expect(updatedAgent!.mcpServerNames).toEqual(['authorizedServer']); + }); + test('should delete an agent', async () => { const agentId = `agent_${uuidv4()}`; const authorId = new mongoose.Types.ObjectId(); diff --git a/packages/data-schemas/src/methods/agent.ts b/packages/data-schemas/src/methods/agent.ts index 6f11fa720b..8a593b4498 100644 --- a/packages/data-schemas/src/methods/agent.ts +++ b/packages/data-schemas/src/methods/agent.ts @@ -1,5 +1,11 @@ import crypto from 'node:crypto'; -import { Constants, EToolResources, ResourceType, actionDelimiter } from 'librechat-data-provider'; +import { + Constants, + EToolResources, + ResourceType, + actionDelimiter, + isActionTool, +} from 'librechat-data-provider'; import type { AgentToolResources } from 'librechat-data-provider'; import type { FilterQuery, Model, Types } from 'mongoose'; import type { IAgent, IAclEntry } from '~/types'; @@ -46,7 +52,7 @@ function extractMCPServerNames(tools: string[] | undefined | null): string[] { } const serverNames = new Set(); for (const tool of tools) { - if (!tool || !tool.includes(mcp_delimiter)) { + if (!tool || !tool.includes(mcp_delimiter) || isActionTool(tool)) { continue; } const parts = tool.split(mcp_delimiter);