diff --git a/api/server/controllers/mcpApps.js b/api/server/controllers/mcpApps.js index 1f992754f3..38bafd914c 100644 --- a/api/server/controllers/mcpApps.js +++ b/api/server/controllers/mcpApps.js @@ -1,7 +1,13 @@ const path = require('path'); const { logger } = require('@librechat/data-schemas'); const { CacheKeys, Constants } = require('librechat-data-provider'); -const { getUserMCPAuthMap } = require('@librechat/api'); +const { + getUserMCPAuthMap, + readAppResource, + listAppResources, + listAppResourceTemplates, + callAppTool, +} = require('@librechat/api'); const { getMCPManager, getFlowStateManager } = require('~/config'); const { getAppConfig } = require('~/server/services/Config'); const { resolveConfigServers } = require('~/server/services/MCP'); @@ -48,31 +54,13 @@ const readMCPResource = async (req, res) => { } const { serverName, uri } = req.body; - if (!serverName || !uri) { - return res.status(400).json({ error: 'serverName and uri are required' }); - } - // The serverResources capability lets an app read any resource the connected MCP server - // exposes (ui:// templates plus supporting data such as file:// or custom schemes), so the - // proxy only requires a non-empty string and leaves resource authorization to the server. - if (typeof uri !== 'string' || uri.length === 0) { - return res.status(400).json({ error: 'uri must be a non-empty string' }); - } - - const mcpManager = getMCPManager(); - const { configServers, customUserVars, flowManager, tokenMethods } = await resolveAppContext( - req, - serverName, - ); - const result = await mcpManager.readResource({ + const ctx = { userId, serverName, - uri, user: req.user, - configServers, - customUserVars, - flowManager, - tokenMethods, - }); + ...(await resolveAppContext(req, serverName)), + }; + const result = await readAppResource(getMCPManager(), ctx, uri); return res.json(result); } catch (error) { // A denied read (non-advertised / non-ui:// resource) is an expected client error, not a @@ -94,30 +82,18 @@ const listMCPResources = async (req, res) => { } const { serverName, cursor } = req.body; - if (!serverName) { - return res.status(400).json({ error: 'serverName is required' }); - } - if (cursor !== undefined && typeof cursor !== 'string') { - return res.status(400).json({ error: 'cursor must be a string' }); - } - - const mcpManager = getMCPManager(); - const { configServers, customUserVars, flowManager, tokenMethods } = await resolveAppContext( - req, - serverName, - ); - const result = await mcpManager.listResources({ + const ctx = { userId, serverName, user: req.user, - cursor, - configServers, - customUserVars, - flowManager, - tokenMethods, - }); + ...(await resolveAppContext(req, serverName)), + }; + const result = await listAppResources(getMCPManager(), ctx, cursor); return res.json(result); } catch (error) { + if (error && typeof error === 'object' && error.code === MCP_INVALID_REQUEST) { + return res.status(400).json({ error: error.message }); + } logger.error('[listMCPResources] Error:', error); return res.status(500).json({ error: 'Failed to list resources' }); } @@ -132,30 +108,18 @@ const listMCPResourceTemplates = async (req, res) => { } const { serverName, cursor } = req.body; - if (!serverName) { - return res.status(400).json({ error: 'serverName is required' }); - } - if (cursor !== undefined && typeof cursor !== 'string') { - return res.status(400).json({ error: 'cursor must be a string' }); - } - - const mcpManager = getMCPManager(); - const { configServers, customUserVars, flowManager, tokenMethods } = await resolveAppContext( - req, - serverName, - ); - const result = await mcpManager.listResourceTemplates({ + const ctx = { userId, serverName, user: req.user, - cursor, - configServers, - customUserVars, - flowManager, - tokenMethods, - }); + ...(await resolveAppContext(req, serverName)), + }; + const result = await listAppResourceTemplates(getMCPManager(), ctx, cursor); return res.json(result); } catch (error) { + if (error && typeof error === 'object' && error.code === MCP_INVALID_REQUEST) { + return res.status(400).json({ error: error.message }); + } logger.error('[listMCPResourceTemplates] Error:', error); return res.status(500).json({ error: 'Failed to list resource templates' }); } @@ -170,33 +134,13 @@ const appToolCall = async (req, res) => { } const { serverName, toolName, arguments: toolArgs } = req.body; - if (!serverName || !toolName) { - return res.status(400).json({ error: 'serverName and toolName are required' }); - } - if ( - toolArgs !== undefined && - toolArgs !== null && - (typeof toolArgs !== 'object' || Array.isArray(toolArgs)) - ) { - return res.status(400).json({ error: 'arguments must be an object' }); - } - - const mcpManager = getMCPManager(); - const { configServers, customUserVars, flowManager, tokenMethods } = await resolveAppContext( - req, - serverName, - ); - const result = await mcpManager.appToolCall({ + const ctx = { userId, serverName, - toolName, - toolArguments: toolArgs || {}, user: req.user, - configServers, - customUserVars, - flowManager, - tokenMethods, - }); + ...(await resolveAppContext(req, serverName)), + }; + const result = await callAppTool(getMCPManager(), ctx, toolName, toolArgs); return res.json(result); } catch (error) { logger.error('[appToolCall] Error:', error); diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index ead04e0b87..b8822aeee1 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -19,6 +19,7 @@ export * from './mcp/zod'; export * from './mcp/errors'; export * from './mcp/cache'; export * from './mcp/tools'; +export * from './mcp/apps'; export * from './mcp/request'; /* Utilities */ export * from './mcp/utils'; diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index c2a3eae11e..8af04c7413 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -660,7 +660,7 @@ Please follow these instructions when using tools from the respective MCP server resolvedHeaders['Authorization'] = `Bearer ${oboTokens.access_token}`; } if (userId && user && oauthStart && flowManager && isOAuthServer(currentOptions)) { - const { allowedDomains, allowedAddresses, useSSRFProtection } = + const { allowedDomains, allowedAddresses, useSSRFProtection, appsEnabled } = await registry.resolveAllowlists({ userId, role: user?.role }); cleanupRequestOAuthHandler = MCPConnectionFactory.attachRequestOAuthHandler( { @@ -671,7 +671,7 @@ Please follow these instructions when using tools from the respective MCP server useSSRFProtection, allowedDomains, allowedAddresses, - enableApps: registry.getAppsEnabled(), + enableApps: appsEnabled, }, { useOAuth: true, @@ -731,9 +731,18 @@ Please follow these instructions when using tools from the respective MCP server requiresEphemeralUserConnection(rawConfig), ); if (resourceMeta) { - logger.debug( - `[MCP][${serverName}][${toolName}] Found resourceUri: ${resourceMeta.uri}`, - ); + // App-backed tool: honor the per-request `mcpSettings.apps` setting so a tenant that + // disabled apps gets no UI resource attached (it would otherwise render as a broken + // iframe once the gated app endpoints reject the follow-up calls). Resolved lazily here + // so ordinary, non-app tools skip the per-request lookup. + const { appsEnabled } = await registry.resolveAllowlists({ userId, role: user?.role }); + if (!appsEnabled) { + resourceMeta = undefined; + } else { + logger.debug( + `[MCP][${serverName}][${toolName}] Found resourceUri: ${resourceMeta.uri}`, + ); + } } } catch { // Non-critical -- tools render without the app UI @@ -1042,8 +1051,20 @@ Please follow these instructions when using tools from the respective MCP server } // Each RFC 6570 operator expands to a bounded shape. Never emit an unrestricted `.+`: // because this regex is the allow-list for app-driven resources/read, a query/fragment - // template must not authorize unrelated path-traversal URIs. - switch (template[i + 1] ?? '') { + // template must not authorize unrelated reads or path traversal. + const expr = template.slice(i + 1, end); + const op = expr[0] ?? ''; + // Variable names declared in this expansion (operator + `:prefix`/`*explode` modifiers + // stripped), used to constrain query expansions to their declared keys rather than an + // open query string. + const keys = expr + .replace(/^[+#./;?&]/, '') + .split(',') + .map((name) => name.split(/[:*]/)[0].trim()) + .filter(Boolean) + .map((name) => name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) + .join('|'); + switch (op) { case '+': // reserved expansion: may legitimately include "/" pattern += '[^?#]+'; break; @@ -1059,11 +1080,11 @@ Please follow these instructions when using tools from the respective MCP server case ';': // path-style params pattern += '(?:;[^/?#]+)+'; break; - case '?': // query (must start with a literal "?") - pattern += '\\?[^#]*'; + case '?': // query: only the declared parameter names, in any order + pattern += keys ? `\\?(?:${keys})=[^#&]*(?:&(?:${keys})=[^#&]*)*` : '\\?[^#]*'; break; - case '&': // query continuation - pattern += '&[^#]*'; + case '&': // query continuation: only the declared parameter names + pattern += keys ? `(?:&(?:${keys})=[^#&]*)+` : '&[^#]*'; break; default: // simple expansion: a single value, no reserved chars pattern += '[^/?#]+'; diff --git a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts index 37c5b11237..ae3b534f7a 100644 --- a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts +++ b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts @@ -35,6 +35,7 @@ const mockRegistryInstance = { allowedDomains: mockGetAllowedDomains(), allowedAddresses: mockGetAllowedAddresses(), useSSRFProtection: mockShouldEnableSSRFProtection(), + appsEnabled: true, })), }; diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index 75d502c611..e416fa1224 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -59,6 +59,7 @@ const mockRegistryInstance = { allowedDomains: mockGetAllowedDomains(), allowedAddresses: mockGetAllowedAddresses(), useSSRFProtection: mockShouldEnableSSRFProtection(), + appsEnabled: true, })), }; diff --git a/packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts b/packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts index 661af087cc..95edb55407 100644 --- a/packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts +++ b/packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts @@ -98,6 +98,7 @@ describe('MCP OAuth Race Condition Fixes', () => { allowedDomains: null, allowedAddresses: null, useSSRFProtection: false, + appsEnabled: true, }), }); @@ -176,6 +177,7 @@ describe('MCP OAuth Race Condition Fixes', () => { allowedDomains: null, allowedAddresses: null, useSSRFProtection: false, + appsEnabled: true, }), }); @@ -263,6 +265,7 @@ describe('MCP OAuth Race Condition Fixes', () => { allowedDomains: null, allowedAddresses: null, useSSRFProtection: false, + appsEnabled: true, }), }); @@ -369,6 +372,7 @@ describe('MCP OAuth Race Condition Fixes', () => { allowedDomains: null, allowedAddresses: null, useSSRFProtection: false, + appsEnabled: true, }), }); diff --git a/packages/api/src/mcp/apps.ts b/packages/api/src/mcp/apps.ts index a8fd83cd12..0a61a82154 100644 --- a/packages/api/src/mcp/apps.ts +++ b/packages/api/src/mcp/apps.ts @@ -5,6 +5,12 @@ * ESM-only ext-apps package; the client bundle keeps importing ext-apps directly. */ +import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js'; +import type { TokenMethods, IUser } from '@librechat/data-schemas'; +import type { FlowStateManager } from '~/flow/manager'; +import type { MCPOAuthTokens } from './oauth'; +import type * as t from './types'; + interface ToolWithMeta { _meta?: Record | null; } @@ -54,3 +60,165 @@ export function isToolVisibilityAppOnly(tool: ToolWithMeta): boolean { const visibility = (tool._meta?.ui as McpUiToolMeta | undefined)?.visibility; return Array.isArray(visibility) && visibility.length === 1 && visibility[0] === 'app'; } + +/** + * Structural manager interface backing the MCP App proxy services. Declared here + * rather than importing MCPManager so this module stays free of a circular import + * (MCPManager imports the helpers above). The argument shapes mirror MCPManager. + */ +export interface MCPAppsProxyManager { + readResource(args: { + userId: string; + serverName: string; + uri: string; + user?: IUser; + configServers?: Record; + customUserVars?: Record; + flowManager?: FlowStateManager; + tokenMethods?: TokenMethods; + }): Promise; + listResources(args: { + userId: string; + serverName: string; + user?: IUser; + cursor?: string; + configServers?: Record; + customUserVars?: Record; + flowManager?: FlowStateManager; + tokenMethods?: TokenMethods; + }): Promise; + listResourceTemplates(args: { + userId: string; + serverName: string; + user?: IUser; + cursor?: string; + configServers?: Record; + customUserVars?: Record; + flowManager?: FlowStateManager; + tokenMethods?: TokenMethods; + }): Promise; + appToolCall(args: { + userId: string; + serverName: string; + toolName: string; + toolArguments: Record; + user?: IUser; + configServers?: Record; + customUserVars?: Record; + flowManager?: FlowStateManager; + tokenMethods?: TokenMethods; + }): Promise; +} + +/** Request-scoped context shared by every MCP App proxy service. */ +export interface MCPAppRequestContext { + userId: string; + serverName: string; + user?: IUser; + configServers?: Record; + customUserVars?: Record; + flowManager?: FlowStateManager; + tokenMethods?: TokenMethods; +} + +/** Reads an MCP App resource after validating the server name and uri. */ +export async function readAppResource( + manager: MCPAppsProxyManager, + ctx: MCPAppRequestContext, + uri: unknown, +): Promise { + if (!ctx.serverName) { + throw new McpError(ErrorCode.InvalidRequest, 'serverName and uri are required'); + } + if (typeof uri !== 'string' || uri.length === 0) { + throw new McpError(ErrorCode.InvalidRequest, 'uri must be a non-empty string'); + } + return manager.readResource({ + userId: ctx.userId, + serverName: ctx.serverName, + uri, + user: ctx.user, + configServers: ctx.configServers, + customUserVars: ctx.customUserVars, + flowManager: ctx.flowManager, + tokenMethods: ctx.tokenMethods, + }); +} + +/** Lists MCP App resources after validating the server name and optional cursor. */ +export async function listAppResources( + manager: MCPAppsProxyManager, + ctx: MCPAppRequestContext, + cursor: unknown, +): Promise { + if (!ctx.serverName) { + throw new McpError(ErrorCode.InvalidRequest, 'serverName is required'); + } + if (cursor !== undefined && typeof cursor !== 'string') { + throw new McpError(ErrorCode.InvalidRequest, 'cursor must be a string'); + } + return manager.listResources({ + userId: ctx.userId, + serverName: ctx.serverName, + user: ctx.user, + cursor, + configServers: ctx.configServers, + customUserVars: ctx.customUserVars, + flowManager: ctx.flowManager, + tokenMethods: ctx.tokenMethods, + }); +} + +/** Lists MCP App resource templates after validating the server name and optional cursor. */ +export async function listAppResourceTemplates( + manager: MCPAppsProxyManager, + ctx: MCPAppRequestContext, + cursor: unknown, +): Promise { + if (!ctx.serverName) { + throw new McpError(ErrorCode.InvalidRequest, 'serverName is required'); + } + if (cursor !== undefined && typeof cursor !== 'string') { + throw new McpError(ErrorCode.InvalidRequest, 'cursor must be a string'); + } + return manager.listResourceTemplates({ + userId: ctx.userId, + serverName: ctx.serverName, + user: ctx.user, + cursor, + configServers: ctx.configServers, + customUserVars: ctx.customUserVars, + flowManager: ctx.flowManager, + tokenMethods: ctx.tokenMethods, + }); +} + +/** Proxies an MCP App tool call after validating the server name, tool name, and arguments. */ +export async function callAppTool( + manager: MCPAppsProxyManager, + ctx: MCPAppRequestContext, + toolName: unknown, + toolArguments: unknown, +): Promise { + if (!ctx.serverName || !toolName) { + throw new McpError(ErrorCode.InvalidRequest, 'serverName and toolName are required'); + } + if ( + toolArguments !== undefined && + toolArguments !== null && + (typeof toolArguments !== 'object' || Array.isArray(toolArguments)) + ) { + throw new McpError(ErrorCode.InvalidRequest, 'arguments must be an object'); + } + return manager.appToolCall({ + userId: ctx.userId, + serverName: ctx.serverName, + toolName: toolName as string, + toolArguments: (toolArguments as Record) || {}, + user: ctx.user, + configServers: ctx.configServers, + customUserVars: ctx.customUserVars, + flowManager: ctx.flowManager, + tokenMethods: ctx.tokenMethods, + }); +} diff --git a/packages/api/src/mcp/registry/MCPServersRegistry.ts b/packages/api/src/mcp/registry/MCPServersRegistry.ts index 9f26871354..4762464872 100644 --- a/packages/api/src/mcp/registry/MCPServersRegistry.ts +++ b/packages/api/src/mcp/registry/MCPServersRegistry.ts @@ -94,9 +94,12 @@ export interface MCPAllowlistContext { * dependency. Reads the ALS tenant context internally; pass the acting user to also pick up * user/role-scoped overrides. */ -export type MCPAllowlistResolver = ( - ctx?: MCPAllowlistContext, -) => Promise<{ allowedDomains?: string[] | null; allowedAddresses?: string[] | null }>; +export type MCPAllowlistResolver = (ctx?: MCPAllowlistContext) => Promise<{ + allowedDomains?: string[] | null; + allowedAddresses?: string[] | null; + /** Per-request `mcpSettings.apps`. Omit to inherit the YAML base; `false` disables apps. */ + appsEnabled?: boolean; +}>; /** Effective allowlists resolved for a request. */ interface ResolvedMCPAllowlists { @@ -244,14 +247,22 @@ export class MCPServersRegistry { allowedDomains?: string[] | null; allowedAddresses?: string[] | null; useSSRFProtection: boolean; + appsEnabled: boolean; }> { let allowedDomains = this.allowedDomains; let allowedAddresses = this.allowedAddresses; + // MCP Apps, like the allowlists, are tenant/principal-scoped: resolve the per-request value so a + // tenant/role/user override of `mcpSettings.apps` is honored. Inherit the YAML base when the + // resolver omits it; fall back to the base entirely if the resolver is absent or fails. + let appsEnabled = this.getAppsEnabled(); if (this.allowlistResolver) { try { const resolved = await this.allowlistResolver(ctx); allowedDomains = resolved.allowedDomains; allowedAddresses = resolved.allowedAddresses; + if (resolved.appsEnabled !== undefined) { + appsEnabled = resolved.appsEnabled !== false; + } } catch (error) { logger.warn( '[MCPServersRegistry] Allowlist resolver failed; falling back to YAML base allowlists', @@ -263,6 +274,7 @@ export class MCPServersRegistry { allowedDomains, allowedAddresses, useSSRFProtection: !Array.isArray(allowedDomains) || allowedDomains.length === 0, + appsEnabled, }; } diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts index 8ce0dedab6..1c704141a9 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts @@ -241,6 +241,7 @@ describe('MCPServersRegistry', () => { allowedDomains: ['yaml.com'], allowedAddresses: ['10.0.0.0/8'], useSSRFProtection: false, + appsEnabled: true, }); }); @@ -250,6 +251,7 @@ describe('MCPServersRegistry', () => { allowedDomains: undefined, allowedAddresses: undefined, useSSRFProtection: true, + appsEnabled: true, }); }); @@ -267,6 +269,7 @@ describe('MCPServersRegistry', () => { allowedDomains: ['admin-added.com'], allowedAddresses: ['172.16.0.0/12'], useSSRFProtection: false, + appsEnabled: true, }); }); @@ -278,6 +281,7 @@ describe('MCPServersRegistry', () => { allowedDomains: ['yaml.com'], allowedAddresses: null, useSSRFProtection: false, + appsEnabled: true, }); });