CERBERUS/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2026-08-04 06:52:47 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

fix(mcp): resolve apps per request, tighten resource templates, extract app controller

Browse source 
Resolve mcpSettings.apps per request through the tenant-scoped allowlist resolver (inheriting the
YAML base when omitted) and consult it in callTool: when a tenant/role/user has apps disabled, the
tool result is returned with no UI resource attached, so those users no longer get a broken iframe
that the gated app endpoints reject. The OAuth-path connection advertises the resolved value.

Constrain query and query-continuation URI-template operators to their declared variable names
instead of the whole query string, so a template like file://items{?id} no longer authorizes
unrelated query parameters such as ?admin=true. The path-traversal guard still applies.

Move the MCP Apps per-endpoint validation and orchestration into packages/api as TypeScript
service functions (readAppResource, listAppResources, listAppResourceTemplates, callAppTool)
exported from @librechat/api, delegating through a structural manager interface to avoid a circular
import. The /api controllers become thin adapters; resolveAppContext, the sandbox file serve, and
the requireMCPAppsEnabled middleware stay in /api as request-bound glue.
This commit is contained in:
Dustin Healy 2026-06-28 23:52:37 -07:00
parent e459984f21
commit f101d73f72
9 changed files with 255 additions and 99 deletions
Download patch file Download diff file Expand all files Collapse all files

114
api/server/controllers/mcpApps.js
View file

@ -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);

1
packages/api/src/index.ts
View file

@ -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';

43
packages/api/src/mcp/MCPManager.ts
View file

@ -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 += '[^/?#]+';

1
packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts
View file

@ -35,6 +35,7 @@ const mockRegistryInstance = {
allowedDomains: mockGetAllowedDomains(),
allowedAddresses: mockGetAllowedAddresses(),
useSSRFProtection: mockShouldEnableSSRFProtection(),
appsEnabled: true,
})),
};

1
packages/api/src/mcp/__tests__/MCPManager.test.ts
View file

@ -59,6 +59,7 @@ const mockRegistryInstance = {
allowedDomains: mockGetAllowedDomains(),
allowedAddresses: mockGetAllowedAddresses(),
useSSRFProtection: mockShouldEnableSSRFProtection(),
appsEnabled: true,
})),
};

4
packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts
View file

@ -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,
}),
});

168
packages/api/src/mcp/apps.ts
View file

@ -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<string, unknown> | 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<string, t.ParsedServerConfig>;
customUserVars?: Record<string, string>;
flowManager?: FlowStateManager<MCPOAuthTokens | null>;
tokenMethods?: TokenMethods;
}): Promise<unknown>;
listResources(args: {
userId: string;
serverName: string;
user?: IUser;
cursor?: string;
configServers?: Record<string, t.ParsedServerConfig>;
customUserVars?: Record<string, string>;
flowManager?: FlowStateManager<MCPOAuthTokens | null>;
tokenMethods?: TokenMethods;
}): Promise<unknown>;
listResourceTemplates(args: {
userId: string;
serverName: string;
user?: IUser;
cursor?: string;
configServers?: Record<string, t.ParsedServerConfig>;
customUserVars?: Record<string, string>;
flowManager?: FlowStateManager<MCPOAuthTokens | null>;
tokenMethods?: TokenMethods;
}): Promise<unknown>;
appToolCall(args: {
userId: string;
serverName: string;
toolName: string;
toolArguments: Record<string, unknown>;
user?: IUser;
configServers?: Record<string, t.ParsedServerConfig>;
customUserVars?: Record<string, string>;
flowManager?: FlowStateManager<MCPOAuthTokens | null>;
tokenMethods?: TokenMethods;
}): Promise<unknown>;
}
/** Request-scoped context shared by every MCP App proxy service. */
export interface MCPAppRequestContext {
userId: string;
serverName: string;
user?: IUser;
configServers?: Record<string, t.ParsedServerConfig>;
customUserVars?: Record<string, string>;
flowManager?: FlowStateManager<MCPOAuthTokens | null>;
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<unknown> {
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<unknown> {
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<unknown> {
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<unknown> {
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<string, unknown>) || {},
user: ctx.user,
configServers: ctx.configServers,
customUserVars: ctx.customUserVars,
flowManager: ctx.flowManager,
tokenMethods: ctx.tokenMethods,
});
}

18
packages/api/src/mcp/registry/MCPServersRegistry.ts
View file

@ -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,
};
}

4
packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts
View file

@ -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,
});
});