diff --git a/api/server/controllers/mcp.js b/api/server/controllers/mcp.js index f5850638b2..85b840891d 100644 --- a/api/server/controllers/mcp.js +++ b/api/server/controllers/mcp.js @@ -5,9 +5,10 @@ * @import { MCPServerRegistry } from '@librechat/api' * @import { MCPServerDocument } from 'librechat-data-provider' */ -const { logger } = require('@librechat/data-schemas'); +const { logger, SystemCapabilities } = require('@librechat/data-schemas'); const { checkAccess, + isUserSourced, MCPErrorCodes, redactServerSecrets, redactAllServerSecrets, @@ -17,9 +18,11 @@ const { const { Constants, Permissions, + ResourceType, + PermissionBits, PermissionTypes, - MCPServerUserInputSchema, MCP_USER_INPUT_FIELDS, + MCPServerUserInputSchema, } = require('librechat-data-provider'); const { resolveConfigServers, @@ -27,6 +30,8 @@ const { resolveAllMcpConfigs, } = require('~/server/services/MCP'); const { cacheMCPServerTools, getMCPServerTools } = require('~/server/services/Config'); +const { getResourcePermissionsMap } = require('~/server/services/PermissionService'); +const { hasCapability } = require('~/server/middleware/roles/capabilities'); const { getMCPManager, getMCPServersRegistry } = require('~/config'); const db = require('~/models'); @@ -196,6 +201,55 @@ const getMCPTools = async (req, res) => { res.status(500).json({ message: error.message }); } }; +/** Mirrors canAccessResource's capability bypass plus per-resource ACL EDIT check. */ +async function computeCanEditByServer(req, serverConfigs) { + const canEditByServer = new Map(); + let bypass = false; + try { + bypass = await hasCapability(req.user, SystemCapabilities.MANAGE_MCP_SERVERS); + } catch (err) { + logger.warn(`[computeCanEditByServer] Capability bypass check failed: ${err.message}`); + } + if (bypass) { + for (const name of Object.keys(serverConfigs)) { + canEditByServer.set(name, true); + } + return canEditByServer; + } + const dbIdsToCheck = []; + const dbIdToServerName = new Map(); + for (const [name, config] of Object.entries(serverConfigs)) { + if (config.dbId) { + dbIdsToCheck.push(config.dbId); + dbIdToServerName.set(String(config.dbId), name); + continue; + } + canEditByServer.set(name, isUserSourced(config)); + } + if (dbIdsToCheck.length > 0) { + try { + const permsMap = await getResourcePermissionsMap({ + userId: req.user.id, + role: req.user.role, + resourceType: ResourceType.MCPSERVER, + resourceIds: dbIdsToCheck, + }); + for (const [dbIdStr, name] of dbIdToServerName) { + const bits = permsMap.get(dbIdStr) ?? 0; + canEditByServer.set(name, (bits & PermissionBits.EDIT) !== 0); + } + } catch (err) { + logger.warn( + `[computeCanEditByServer] ACL lookup failed, defaulting to no edit: ${err.message}`, + ); + for (const name of dbIdToServerName.values()) { + canEditByServer.set(name, false); + } + } + } + return canEditByServer; +} + /** * Get all MCP servers with permissions * @route GET /api/mcp/servers @@ -208,7 +262,8 @@ const getMCPServersList = async (req, res) => { } const serverConfigs = await resolveAllMcpConfigs(userId, req.user); - return res.json(redactAllServerSecrets(serverConfigs)); + const canEditByServer = await computeCanEditByServer(req, serverConfigs); + return res.json(redactAllServerSecrets(serverConfigs, { canEditByServer })); } catch (error) { logger.error('[getMCPServersList]', error); res.status(500).json({ error: error.message }); @@ -306,7 +361,7 @@ const createMCPServerController = async (req, res) => { ); res.status(201).json({ serverName: result.serverName, - ...redactServerSecrets(result.config), + ...redactServerSecrets(result.config, { canEdit: true }), }); } catch (error) { logger.error('[createMCPServer]', error); @@ -339,7 +394,9 @@ const getMCPServerById = async (req, res) => { return res.status(404).json({ message: 'MCP server not found' }); } - res.status(200).json(redactServerSecrets(parsedConfig)); + const canEditMap = await computeCanEditByServer(req, { [serverName]: parsedConfig }); + const canEdit = canEditMap.get(serverName) ?? false; + res.status(200).json(redactServerSecrets(parsedConfig, { canEdit })); } catch (error) { logger.error('[getMCPServerById]', error); res.status(500).json({ message: error.message }); @@ -400,7 +457,7 @@ const updateMCPServerController = async (req, res) => { userId, ); - res.status(200).json(redactServerSecrets(parsedConfig)); + res.status(200).json(redactServerSecrets(parsedConfig, { canEdit: true })); } catch (error) { logger.error('[updateMCPServer]', error); const mcpErrorResponse = handleMCPError(error, res); diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index a1f981843c..823bf783fc 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -2600,11 +2600,13 @@ describe('MCP Routes', () => { type: 'sse', url: 'http://server1.com/sse', title: 'Server 1', + source: 'user', }, 'server-2': { type: 'sse', url: 'http://server2.com/sse', title: 'Server 2', + source: 'user', }, }; @@ -2676,7 +2678,7 @@ describe('MCP Routes', () => { mockRegistryInstance.addServer.mockResolvedValue({ serverName: 'test-sse-server', - config: validConfig, + config: { ...validConfig, source: 'user' }, }); const response = await request(app).post('/api/mcp/servers').send({ config: validConfig }); @@ -3096,6 +3098,7 @@ describe('MCP Routes', () => { type: 'sse', url: 'https://mcp-server.example.com/sse', title: 'Test Server', + source: 'user', }; mockRegistryInstance.getServerConfig.mockResolvedValue(mockConfig); @@ -3164,7 +3167,7 @@ describe('MCP Routes', () => { description: 'Updated description', }; - mockRegistryInstance.updateServer.mockResolvedValue(updatedConfig); + mockRegistryInstance.updateServer.mockResolvedValue({ ...updatedConfig, source: 'user' }); const response = await request(app) .patch('/api/mcp/servers/test-server') diff --git a/packages/api/src/mcp/__tests__/utils.test.ts b/packages/api/src/mcp/__tests__/utils.test.ts index bd6baa2fa0..cbed141d0f 100644 --- a/packages/api/src/mcp/__tests__/utils.test.ts +++ b/packages/api/src/mcp/__tests__/utils.test.ts @@ -236,13 +236,93 @@ describe('redactServerSecrets', () => { expect(redacted.customUserVars).toEqual(config.customUserVars); }); - it('should pass URLs through unchanged', () => { + it('should pass URLs through unchanged when caller has edit authority', () => { const config: ParsedServerConfig = { type: 'sse', - url: 'https://mcp.example.com/sse?param=value', + url: 'https://infra.internal/mcp', + source: 'config', + }; + const redacted = redactServerSecrets(config, { canEdit: true }); + expect(redacted.url).toBe('https://infra.internal/mcp'); + }); + + it('should strip url and oauth flow URLs for non-user-sourced configs without edit authority', () => { + const config: ParsedServerConfig = { + type: 'sse', + url: 'https://infra.internal/mcp', + source: 'config', + oauth: { + client_id: 'cid', + authorization_url: 'https://infra.internal/oauth/authorize', + token_url: 'https://infra.internal/oauth/token', + revocation_endpoint: 'https://infra.internal/oauth/revoke', + scope: 'read', + }, }; const redacted = redactServerSecrets(config); - expect(redacted.url).toBe('https://mcp.example.com/sse?param=value'); + expect(redacted.url).toBeUndefined(); + expect(redacted.oauth?.authorization_url).toBeUndefined(); + expect(redacted.oauth?.token_url).toBeUndefined(); + expect(redacted.oauth?.revocation_endpoint).toBeUndefined(); + expect(redacted.oauth?.client_id).toBe('cid'); + expect(redacted.oauth?.scope).toBe('read'); + }); + + it('should strip url for a VIEW-shared user-sourced config when caller lacks edit', () => { + const config: ParsedServerConfig = { + type: 'sse', + url: 'https://owner-private.example.com/mcp', + source: 'user', + dbId: 'abc123', + oauth: { + client_id: 'cid', + authorization_url: 'https://owner-private.example.com/oauth/authorize', + token_url: 'https://owner-private.example.com/oauth/token', + }, + }; + const redacted = redactServerSecrets(config); + expect(redacted.url).toBeUndefined(); + expect(redacted.oauth?.authorization_url).toBeUndefined(); + expect(redacted.oauth?.token_url).toBeUndefined(); + expect(redacted.oauth?.client_id).toBe('cid'); + }); + + it('should retain non-sensitive oauth fields when stripping oauth flow URLs', () => { + const config: ParsedServerConfig = { + type: 'sse', + url: 'https://infra.internal/mcp', + source: 'config', + oauth: { + client_id: 'cid', + client_secret: 'csecret', + authorization_url: 'https://infra.internal/oauth/authorize', + token_url: 'https://infra.internal/oauth/token', + scope: 'read', + redirect_uri: 'https://app.example.com/callback', + }, + }; + const redacted = redactServerSecrets(config); + expect(redacted.oauth?.client_secret).toBeUndefined(); + expect(redacted.oauth?.authorization_url).toBeUndefined(); + expect(redacted.oauth?.token_url).toBeUndefined(); + expect(redacted.oauth?.client_id).toBe('cid'); + expect(redacted.oauth?.scope).toBe('read'); + expect(redacted.oauth?.redirect_uri).toBe('https://app.example.com/callback'); + }); + + it('should preserve customUserVars metadata for non-user-sourced configs without edit authority', () => { + const config: ParsedServerConfig = { + type: 'sse', + url: 'https://infra.internal/mcp', + source: 'config', + customUserVars: { + API_KEY: { title: 'API Key', description: 'Your key', sensitive: true }, + }, + }; + const redacted = redactServerSecrets(config); + expect(redacted.customUserVars).toEqual({ + API_KEY: { title: 'API Key', description: 'Your key', sensitive: true }, + }); }); it('should only include explicitly allowlisted fields', () => { @@ -295,6 +375,55 @@ describe('redactAllServerSecrets', () => { expect(redacted['server-b'].oauth?.client_id).toBe('cid-b'); expect((redacted['server-c'] as Record).command).toBeUndefined(); }); + + it('should pass canEdit through per-server via canEditByServer map', () => { + const configs: Record = { + 'config-server': { + type: 'sse', + url: 'https://infra.internal/mcp', + source: 'config', + oauth: { + client_id: 'cid', + authorization_url: 'https://infra.internal/oauth/authorize', + }, + }, + 'user-server-owner': { + type: 'sse', + url: 'https://owner.example.com/mcp', + source: 'user', + dbId: 'owner-id', + }, + 'user-server-shared': { + type: 'sse', + url: 'https://shared.example.com/mcp', + source: 'user', + dbId: 'shared-id', + }, + }; + const canEditByServer = new Map([ + ['config-server', false], + ['user-server-owner', true], + ['user-server-shared', false], + ]); + const redacted = redactAllServerSecrets(configs, { canEditByServer }); + expect(redacted['config-server'].url).toBeUndefined(); + expect(redacted['config-server'].oauth?.authorization_url).toBeUndefined(); + expect(redacted['user-server-owner'].url).toBe('https://owner.example.com/mcp'); + expect(redacted['user-server-shared'].url).toBeUndefined(); + }); + + it('should expose URL for non-user-sourced configs when canEditByServer is true', () => { + const configs: Record = { + 'config-server': { + type: 'sse', + url: 'https://infra.internal/mcp', + source: 'config', + }, + }; + const canEditByServer = new Map([['config-server', true]]); + const redacted = redactAllServerSecrets(configs, { canEditByServer }); + expect(redacted['config-server'].url).toBe('https://infra.internal/mcp'); + }); }); describe('isInvalidClientMessage', () => { diff --git a/packages/api/src/mcp/utils.ts b/packages/api/src/mcp/utils.ts index 9b7d25429d..ab79f691d3 100644 --- a/packages/api/src/mcp/utils.ts +++ b/packages/api/src/mcp/utils.ts @@ -247,11 +247,19 @@ export function isUserSourced(config: Pick { +export function redactServerSecrets( + config: ParsedServerConfig, + options?: { canEdit?: boolean }, +): Partial { const safe: Partial = { type: config.type, url: config.url, @@ -291,6 +299,19 @@ export function redactServerSecrets(config: ParsedServerConfig): Partial v !== undefined), ) as Partial; @@ -299,10 +320,12 @@ export function redactServerSecrets(config: ParsedServerConfig): Partial, + options?: { canEditByServer?: ReadonlyMap }, ): Record> { const result: Record> = {}; for (const [key, config] of Object.entries(configs)) { - result[key] = redactServerSecrets(config); + const canEdit = options?.canEditByServer?.get(key) ?? false; + result[key] = redactServerSecrets(config, { canEdit }); } return result; }