fix(mcp): carry apps flag through the request resolver and canonicalize resource-read auth

resolveMCPAllowlists now returns appsEnabled from the merged tenant-scoped config, so a
tenant/role/user override of mcpSettings.apps reaches the registry's per-request resolution and
callTool attaches no UI resource for users whose tenant disabled apps.

Authorize app-driven resource reads in the canonical (fully percent-decoded) space the server
resolves and reject any relative path segment, so a percent-encoded traversal such as %2e%2e%2f can
no longer match an advertised template. Exact resources/list matches are unaffected.

Trim narrating comments across the MCP Apps changes so the code is self-documenting.
This commit is contained in:
Dustin Healy 2026-06-29 00:52:58 -07:00
parent f101d73f72
commit 87341c67c0
11 changed files with 103 additions and 85 deletions

View file

@ -24,15 +24,13 @@ const { getLogStores } = require('~/cache');
const MCP_INVALID_REQUEST = -32600;
/**
* Resolves the request-scoped config, the user's custom variables, and the OAuth flow/token
* context for a server so app follow-up requests can connect to config-sourced servers and
* re-resolve credentialed or OAuth connections even when the original tool-call connection is gone.
* Resolves the request-scoped config and auth context so app follow-up requests can reconnect to
* config-sourced servers even when the original tool-call connection is gone.
*/
const resolveAppContext = async (req, serverName) => {
const userId = req.user?.id;
// Fail closed on config resolution: an app request targets one server by name, so a transient
// failure must reject rather than fall back to the base config for that name and proxy to the
// wrong server. Auth map resolution may still degrade, since a missing var fails closed downstream.
// Fail closed on config resolution: a transient failure must reject rather than fall back to the
// base config and proxy to the wrong server. (Auth map resolution fails closed downstream.)
const [configServers, userMCPAuthMap] = await Promise.all([
resolveConfigServers(req, { throwOnError: true }),
Promise.resolve()
@ -63,8 +61,7 @@ const readMCPResource = async (req, res) => {
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
// backend failure, so return 400 and skip the error-level log, mirroring appToolCall.
// A denied read is an expected client error, so return 400 and skip the error-level log.
if (error && typeof error === 'object' && error.code === MCP_INVALID_REQUEST) {
return res.status(400).json({ error: error.message });
}

View file

@ -15,6 +15,7 @@ async function resolveMCPAllowlists(ctx) {
return {
allowedDomains: appConfig?.mcpSettings?.allowedDomains,
allowedAddresses: appConfig?.mcpSettings?.allowedAddresses,
appsEnabled: appConfig?.mcpSettings?.apps,
};
}

View file

@ -6,7 +6,6 @@ interface MessagesViewContextValue {
conversation: ReturnType<typeof useChatContext>['conversation'];
conversationId: string | null | undefined;
/** True when the view cannot mutate server state (shared/search); MCP App bridges render display-only. */
readOnly: boolean;
/** Submission and control states */
@ -117,11 +116,7 @@ export function useMessagesViewContext() {
return context;
}
/**
* True when MCP App bridges should be display-only: the shared view, the /search route, or any
* mount outside an interactive MessagesViewProvider. Defaults to read-only when no provider is
* present so a new render context never accidentally enables live, auth-bearing app actions.
*/
/** Defaults to read-only when no provider is present, so live auth-bearing app actions stay off. */
export function useIsMessagesViewReadOnly(): boolean {
return useContext(MessagesViewContext)?.readOnly ?? true;
}

View file

@ -77,9 +77,7 @@ const MCPAppView = React.memo(function MCPAppView({
}
const isAppBacked = isMcpAppResource(app);
// Read-only views (shared/search) don't fetch app HTML from the viewer's MCP server, so a
// server-fetched (resourceUri-only) app cannot render; show a placeholder instead of a failing
// iframe. Inline apps (with persisted HTML) still render.
// Read-only views don't fetch app HTML, so a resourceUri-only app shows a placeholder.
if (isAppBacked && !app.text && readOnly) {
return (
<div className="my-2 flex items-center gap-2 rounded-lg border border-border-light bg-surface-secondary px-4 py-3 text-sm text-text-secondary">

View file

@ -21,8 +21,7 @@ export function ShareMessagesProvider({ messages, children }: ShareMessagesProvi
() => ({
conversation: null,
conversationId: undefined,
// Share view is read-only: MCP App bridges must render display-only and never proxy
// auth-bearing tool calls or resource reads against the viewer's MCP servers.
// Read-only so app bridges never proxy auth-bearing calls against the viewer's MCP servers.
readOnly: true,
// These are required by the context but not used in share view
ask: () => {},

View file

@ -35,9 +35,8 @@ export function useAppBridge(
) {
const user = useRecoilValue(store.user);
const { ask } = useOptionalMessagesOperations();
// Shared transcripts and /search render read-only: the embedded app must not proxy tool calls or
// resource reads against the viewer's MCP servers with the viewer's auth. Such views render the
// app display-only (initial tool input/result still shown), with no host-bound action handlers.
// Read-only views (shared transcripts, /search) must not let the embedded app proxy tool calls
// or resource reads against the viewer's MCP servers with the viewer's auth.
const readOnly = useIsMessagesViewReadOnly();
const queryClient = useQueryClient();
const bridgeRef = useRef<AppBridge | null>(null);
@ -79,8 +78,8 @@ export function useAppBridge(
const theme = document.documentElement.classList.contains('dark') ? 'dark' : 'light';
const { locale, timeZone } = Intl.DateTimeFormat().resolvedOptions();
// Display-only views advertise no host-bound action capabilities, so a well-behaved app
// disables those affordances instead of issuing calls the host will ignore.
// Display-only views advertise no host-bound action capabilities so a well-behaved app
// disables those affordances rather than issuing calls the host ignores.
const interactive = !readOnlyRef.current;
bridge = new AppBridge(
@ -118,7 +117,7 @@ export function useAppBridge(
};
// Host-bound actions (tool calls, resource reads/lists, model messages) run with the viewer's
// auth, so they are only wired in interactive views — never for shared transcripts or /search.
// auth, so they are only wired in interactive views, never in shared transcripts or /search.
if (interactive) {
bridge.oncalltool = async (params) =>
callMCPAppTool(
@ -153,8 +152,8 @@ export function useAppBridge(
return;
}
sandboxReadyHandled = true;
// Read-only views (shared/search) must not resolve app HTML from the viewer's MCP server,
// so only inline (persisted) HTML renders here; resourceUri-only apps stay display-only.
// Read-only views must not resolve app HTML from the viewer's MCP server, so only inline
// (persisted) HTML renders here.
if (!resource.text && readOnlyRef.current) {
logger.debug(
'[MCP App] Read-only view: skipping server HTML fetch for resourceUri-only app',

View file

@ -79,9 +79,8 @@ export class MCPManager extends UserConnectionManager {
*/
private readonly toolCacheConnStamp = new Map<string, string>();
/**
* Per-connection snapshot of the resource URIs and URI templates a server advertises, used to
* authorize app-driven `resources/read` so an embedded app can only proxy resources the server
* publicly exposes not arbitrary `file://`/`secret://` URIs it happens to be reachable for.
* Snapshot of the resources a server advertises, used to authorize app-driven `resources/read`
* so an embedded app can only proxy publicly exposed resources, not arbitrary reachable URIs.
*/
private readonly advertisedResourceCache = new Map<
string,
@ -89,7 +88,6 @@ export class MCPManager extends UserConnectionManager {
>();
private readonly advertisedResourceConnStamp = new Map<string, string>();
/** Bounds the resources/list + templates/list pagination loops when snapshotting advertised resources. */
private static readonly RESOURCE_LIST_MAX_PAGES = 20;
/** Creates and initializes the singleton MCPManager instance */
@ -404,18 +402,16 @@ Please follow these instructions when using tools from the respective MCP server
}
/**
* App-level connections can be transparently recreated when a server config changes
* (ConnectionsRepository.get), so cached tool metadata is only valid while it was built
* from the current connection instance.
* App-level connections can be recreated when a server config changes, so cached tool metadata
* is only valid while it was built from the current connection instance.
*/
private connStamp(connection: MCPConnection): string {
return `${connection.createdAt}:${connection.toolListVersion}`;
}
/**
* Freshness stamp for the advertised-resource cache: keyed on the connection instance (createdAt)
* and the resources/list_changed counter, so removed/added server resources re-authorize without
* waiting for a reconnect.
* Freshness stamp keyed on the connection instance and the resources/list_changed counter, so
* removed or added server resources re-authorize without waiting for a reconnect.
*/
private resourceConnStamp(connection: MCPConnection): string {
return `${connection.createdAt}:${connection.resourceListVersion}`;
@ -453,8 +449,8 @@ Please follow these instructions when using tools from the respective MCP server
if (isToolVisibilityModelOnly(tool)) {
modelOnly.add(tool.name);
}
// A malformed `_meta.ui.resourceUri` on one tool must not abort discovery for the whole
// server, so isolate the parse: a bad declaration only disables that tool's UI metadata.
// A malformed `_meta.ui.resourceUri` on one tool only disables that tool's UI metadata,
// never aborting discovery for the whole server.
try {
const uri = getToolUiResourceUri(tool);
if (uri) {
@ -731,10 +727,9 @@ Please follow these instructions when using tools from the respective MCP server
requiresEphemeralUserConnection(rawConfig),
);
if (resourceMeta) {
// 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.
// Honor the per-request apps setting so a tenant that disabled apps gets no UI resource
// (it would otherwise render as a broken iframe once the gated app endpoints reject the
// follow-up calls). Resolved lazily so non-app tools skip the per-request lookup.
const { appsEnabled } = await registry.resolveAllowlists({ userId, role: user?.role });
if (!appsEnabled) {
resourceMeta = undefined;
@ -745,7 +740,7 @@ Please follow these instructions when using tools from the respective MCP server
}
}
} catch {
// Non-critical -- tools render without the app UI
/* empty */
}
}
@ -785,10 +780,6 @@ Please follow these instructions when using tools from the respective MCP server
}
}
/**
* Reads a UI resource from an MCP server.
* Used by MCP Apps iframes to fetch additional resources via the host.
*/
/**
* Resolves the same registry-backed config the original tool call used and hands it to
* getConnection so config-source servers resolve, then refreshes headers for non-DB-sourced
@ -950,14 +941,15 @@ Please follow these instructions when using tools from the respective MCP server
`${logPrefix} Resource "${uri}" is not permitted.`,
);
}
// Exact advertised URIs are trusted as-is. A template match must additionally not resolve to a
// path-traversal URI, so a parameterized template can never authorize an unrelated resource.
if (advertised.uris.has(uri)) {
return;
}
// Match templates in canonical (fully percent-decoded) space, never raw bytes, so an encoded
// traversal like `%2e%2e%2f` cannot slip past a template guard.
const canonicalUri = MCPManager.canonicalizeUri(uri);
if (
!uri.split('/').includes('..') &&
advertised.templates.some((pattern) => pattern.test(uri))
canonicalUri != null &&
advertised.templates.some((pattern) => pattern.test(canonicalUri))
) {
return;
}
@ -1007,7 +999,16 @@ Please follow these instructions when using tools from the respective MCP server
{ timeout: connection.timeout },
);
for (const template of result.resourceTemplates) {
const pattern = MCPManager.uriTemplateToRegExp(template.uriTemplate);
// Compile templates in the same decoded space the requested URI is canonicalized into,
// so matching is encoding-agnostic; fall back to the raw template if it is not valid
// percent-encoding.
let templateStr = template.uriTemplate;
try {
templateStr = decodeURIComponent(templateStr);
} catch {
/* keep raw template */
}
const pattern = MCPManager.uriTemplateToRegExp(templateStr);
if (pattern) {
templates.push(pattern);
}
@ -1030,6 +1031,31 @@ Please follow these instructions when using tools from the respective MCP server
return entry;
}
/**
* Fully percent-decodes a URI to the canonical form a server resolves. Returns null when it
* cannot be decoded or contains a relative (`.`/`..`) segment, so neither encoded traversal nor
* relative segments can satisfy a template guard.
*/
private static canonicalizeUri(uri: string): string | null {
let current = uri;
for (let depth = 0; depth < 5; depth++) {
let decoded: string;
try {
decoded = decodeURIComponent(current);
} catch {
return null;
}
if (decoded === current) {
break;
}
current = decoded;
}
if (current.split(/[/\\]/).some((segment) => segment === '.' || segment === '..')) {
return null;
}
return current;
}
/**
* Converts an RFC 6570 resource URI template into an anchored matcher. Simple expansions match a
* single path segment; reserved/operator expansions (`{+x}`, `{#x}`, `{/x}`, ...) may span `/`.

View file

@ -1455,6 +1455,32 @@ describe('MCPManager', () => {
const methods = request.mock.calls.map((c) => (c[0] as { method: string }).method);
expect(methods).not.toContain('resources/read');
});
it('rejects a percent-encoded path traversal even when a broad template would match', async () => {
const request = jest.fn().mockImplementation((req: { method: string }) => {
if (req.method === 'resources/list') {
return Promise.resolve({ resources: [] });
}
if (req.method === 'resources/templates/list') {
return Promise.resolve({ resourceTemplates: [{ uriTemplate: 'file://docs{+path}' }] });
}
return Promise.resolve({ contents: [] });
});
const manager = await MCPManager.createInstance(newMCPServersConfig());
jest.spyOn(manager, 'getConnection').mockResolvedValue(buildConnection(request));
await expect(
manager.readResource({
userId: 'user-123',
serverName: 'srv',
uri: 'file://docs/%2e%2e%2fsecret',
user: mockUser as IUser,
}),
).rejects.toThrow(/not advertised/);
const methods = request.mock.calls.map((c) => (c[0] as { method: string }).method);
expect(methods).not.toContain('resources/read');
});
});
describe('getConnection', () => {

View file

@ -1,8 +1,6 @@
/**
* MCP Apps tool-metadata helpers, mirrored from the spec's reference
* implementation in `@modelcontextprotocol/ext-apps`. They are reimplemented
* here so `@librechat/api` (emitted as CommonJS) never statically imports the
* ESM-only ext-apps package; the client bundle keeps importing ext-apps directly.
* Reimplemented here so `@librechat/api` (emitted as CommonJS) never statically
* imports the ESM-only `@modelcontextprotocol/ext-apps` package.
*/
import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';
@ -22,17 +20,10 @@ interface McpUiToolMeta {
visibility?: McpUiToolVisibility[];
}
/** Deprecated flat metadata key for a tool's UI resource URI. */
export const RESOURCE_URI_META_KEY = 'ui/resourceUri';
/** MIME type identifying HTML content as an MCP App UI resource. */
export const RESOURCE_MIME_TYPE = 'text/html;profile=mcp-app';
/**
* Extract a tool's UI resource URI. Prefers the nested `_meta.ui.resourceUri`
* format and falls back to the deprecated flat `_meta["ui/resourceUri"]`.
* Throws if a URI is present but does not use the `ui://` scheme.
*/
export function getToolUiResourceUri(tool: ToolWithMeta): string | undefined {
const uiMeta = tool._meta?.ui as McpUiToolMeta | undefined;
let uri: unknown = uiMeta?.resourceUri;
@ -49,23 +40,17 @@ export function getToolUiResourceUri(tool: ToolWithMeta): string | undefined {
return undefined;
}
/** True when a tool is exposed to the model only (never callable from an app). */
export function isToolVisibilityModelOnly(tool: ToolWithMeta): boolean {
const visibility = (tool._meta?.ui as McpUiToolMeta | undefined)?.visibility;
return Array.isArray(visibility) && visibility.length === 1 && visibility[0] === 'model';
}
/** True when a tool is exposed to the app only (hidden from the model). */
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.
*/
/** Declared here rather than importing MCPManager to avoid a circular import. */
export interface MCPAppsProxyManager {
readResource(args: {
userId: string;
@ -110,7 +95,6 @@ export interface MCPAppsProxyManager {
}): Promise<unknown>;
}
/** Request-scoped context shared by every MCP App proxy service. */
export interface MCPAppRequestContext {
userId: string;
serverName: string;
@ -121,7 +105,6 @@ export interface MCPAppRequestContext {
tokenMethods?: TokenMethods;
}
/** Reads an MCP App resource after validating the server name and uri. */
export async function readAppResource(
manager: MCPAppsProxyManager,
ctx: MCPAppRequestContext,
@ -145,7 +128,6 @@ export async function readAppResource(
});
}
/** Lists MCP App resources after validating the server name and optional cursor. */
export async function listAppResources(
manager: MCPAppsProxyManager,
ctx: MCPAppRequestContext,
@ -169,7 +151,6 @@ export async function listAppResources(
});
}
/** Lists MCP App resource templates after validating the server name and optional cursor. */
export async function listAppResourceTemplates(
manager: MCPAppsProxyManager,
ctx: MCPAppRequestContext,
@ -193,7 +174,6 @@ export async function listAppResourceTemplates(
});
}
/** Proxies an MCP App tool call after validating the server name, tool name, and arguments. */
export async function callAppTool(
manager: MCPAppsProxyManager,
ctx: MCPAppRequestContext,

View file

@ -1147,9 +1147,8 @@ export class MCPConnection extends EventEmitter {
public toolListVersion = 0;
/**
* Bumped on every resources/list_changed notification. Consumers that cache the set of resources
* a server advertises fold this into their freshness check so cached authorization data is
* refreshed when the server adds or removes resources on a live connection.
* Bumped on every resources/list_changed notification so consumers caching a server's advertised
* resources refresh their authorization data when resources are added or removed.
*/
public resourceListVersion = 0;
@ -1267,9 +1266,8 @@ export class MCPConnection extends EventEmitter {
if (params.oauthTokens) {
this.oauthTokens = params.oauthTokens;
}
// Advertise MCP Apps support so servers using the ext-apps graceful-degradation path
// (getUiCapability) expose app-enhanced tools. The capability rides on the `extensions`
// field keyed by ext-apps EXTENSION_ID. Suppressed when MCP Apps are disabled by config.
// Advertise the UI capability so servers expose app-enhanced tools; suppressed when MCP Apps
// are disabled by config.
const appsEnabled = params.enableApps !== false;
const capabilities: ClientCapabilities = appsEnabled
? { extensions: { 'io.modelcontextprotocol/ui': { mimeTypes: [RESOURCE_MIME_TYPE] } } }

View file

@ -251,9 +251,8 @@ export class MCPServersRegistry {
}> {
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.
// Apps are tenant/principal-scoped, so honor a per-request override of `mcpSettings.apps`,
// falling back to the YAML base when the resolver omits it, is absent, or fails.
let appsEnabled = this.getAppsEnabled();
if (this.allowlistResolver) {
try {