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

fix(mcp): harden app CSP, fail closed on auth resolution, and rate-limit resource reads

Browse source 
Render non-app (no profile=mcp-app) ui:// HTML inert: the static srcDoc iframes in ToolCall,
MCPUIResource, and UIResourceCarousel now use sandbox="" so scripts and forms run only through the
CSP-applying sandbox proxy. Make the proxy's meta CSP unbypassable by wrapping any document whose
markup precedes <head>, so nothing untrusted is parsed before the policy takes effect.

Fail closed in resolveAppContext when MCP auth-value resolution throws, logging and rejecting rather
than proceeding with unresolved or stale credentials. Validate each MCP_SANDBOX_FRAME_ANCESTORS
token against a scheme://host[:port] pattern so a stray ";" cannot inject an extra CSP directive.

Rate-limit the app resource endpoints (resources/read, list, templates/list) per user, and correct
AppToolResult.content from an empty-tuple type to unknown[]. Add controller tests for the
frame-ancestors validation and the auth fail-closed path.
This commit is contained in:
Dustin Healy 2026-06-30 17:30:56 -07:00
parent b24eee648e
commit 0f708c2eb8
11 changed files with 175 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -29,13 +29,18 @@ const MCP_INVALID_REQUEST = -32600;
*/
const resolveAppContext = async (req, serverName) => {
const userId = req.user?.id;
// 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.)
// Fail closed on both config and auth resolution: a transient lookup failure must reject rather
// than fall back to the base config (wrong server) or to unresolved/stale credentials. A user
// who genuinely has no vars resolves to undefined without throwing, so that path still proceeds.
const [configServers, userMCPAuthMap] = await Promise.all([
resolveConfigServers(req, { throwOnError: true }),
Promise.resolve()
.then(() => getUserMCPAuthMap({ userId, servers: [serverName], findPluginAuthsByKeys }))
.catch(() => undefined),
getUserMCPAuthMap({ userId, servers: [serverName], findPluginAuthsByKeys }).catch((err) => {
logger.error(
`[resolveAppContext] Failed to resolve MCP auth values for user ${userId}, server ${serverName}; failing closed`,
err,
);
throw err;
}),
]);
const customUserVars = userMCPAuthMap?.[`${Constants.mcp_prefix}${serverName}`];
const flowManager = getFlowStateManager(getLogStores(CacheKeys.FLOWS));
@ -160,11 +165,13 @@ const serveMCPSandbox = async (_req, res) => {
// Default to same-origin framing; when a dedicated sandbox origin is deployed, the operator
// lists the allowed host origin(s) so the host page can frame this sandbox cross-origin.
const allowedParents = (process.env.MCP_SANDBOX_FRAME_ANCESTORS || '').trim();
if (allowedParents) {
const ancestors = allowedParents
.split(/[\s,]+/)
.filter(Boolean)
.join(' ');
// Only accept scheme://host[:port] tokens. A raw value is interpolated into the CSP header, so
// an unvalidated token containing ";" would inject an unrelated directive.
const ancestors = allowedParents
.split(/[\s,]+/)
.filter((token) => /^https?:\/\/[a-zA-Z0-9][a-zA-Z0-9.-]*(?::\d{1,5})?$/.test(token))
.join(' ');
if (ancestors) {
res.setHeader('Content-Security-Policy', `frame-ancestors 'self' ${ancestors}`);
res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin');
} else {

98
api/server/controllers/mcpApps.test.js Normal file
View file

@ -0,0 +1,98 @@
jest.mock('@librechat/data-schemas', () => ({
logger: { error: jest.fn(), warn: jest.fn(), debug: jest.fn(), info: jest.fn() },
}));
jest.mock('@librechat/api', () => ({
getUserMCPAuthMap: jest.fn(),
readAppResource: jest.fn(),
listAppResources: jest.fn(),
listAppResourceTemplates: jest.fn(),
callAppTool: jest.fn(),
}));
jest.mock('~/config', () => ({
getMCPManager: jest.fn(),
getFlowStateManager: jest.fn(),
}));
jest.mock('~/server/services/Config', () => ({ getAppConfig: jest.fn() }));
jest.mock('~/server/services/MCP', () => ({ resolveConfigServers: jest.fn() }));
jest.mock('~/models', () => ({
findPluginAuthsByKeys: jest.fn(),
findToken: jest.fn(),
createToken: jest.fn(),
updateToken: jest.fn(),
deleteTokens: jest.fn(),
}));
jest.mock('~/cache', () => ({ getLogStores: jest.fn() }));
const { getUserMCPAuthMap, readAppResource } = require('@librechat/api');
const { resolveConfigServers } = require('~/server/services/MCP');
const { serveMCPSandbox, readMCPResource } = require('./mcpApps');
const makeRes = () => {
const headers = {};
return {
headers,
headersSent: false,
setHeader: jest.fn((k, v) => {
headers[k] = v;
}),
sendFile: jest.fn(),
status: jest.fn().mockReturnThis(),
json: jest.fn().mockReturnThis(),
};
};
describe('serveMCPSandbox frame-ancestors', () => {
const original = process.env.MCP_SANDBOX_FRAME_ANCESTORS;
afterEach(() => {
if (original === undefined) {
delete process.env.MCP_SANDBOX_FRAME_ANCESTORS;
} else {
process.env.MCP_SANDBOX_FRAME_ANCESTORS = original;
}
});
it('allows a valid host origin and marks the resource cross-origin', async () => {
process.env.MCP_SANDBOX_FRAME_ANCESTORS = 'https://host.example.com';
const res = makeRes();
await serveMCPSandbox({}, res);
expect(res.headers['Content-Security-Policy']).toBe(
"frame-ancestors 'self' https://host.example.com",
);
expect(res.headers['Cross-Origin-Resource-Policy']).toBe('cross-origin');
});
it('drops a token that tries to inject an extra directive', async () => {
process.env.MCP_SANDBOX_FRAME_ANCESTORS = 'https://ok.com; script-src *';
const res = makeRes();
await serveMCPSandbox({}, res);
const csp = res.headers['Content-Security-Policy'];
expect(csp).not.toContain('script-src');
// The ";"-bearing token is rejected wholesale, leaving no valid ancestors -> same-origin default.
expect(csp).toBe("frame-ancestors 'self'");
expect(res.headers['X-Frame-Options']).toBe('SAMEORIGIN');
});
it('defaults to same-origin when no ancestors are configured', async () => {
delete process.env.MCP_SANDBOX_FRAME_ANCESTORS;
const res = makeRes();
await serveMCPSandbox({}, res);
expect(res.headers['Content-Security-Policy']).toBe("frame-ancestors 'self'");
expect(res.headers['Cross-Origin-Resource-Policy']).toBe('same-origin');
});
});
describe('resolveAppContext fail-closed', () => {
beforeEach(() => jest.clearAllMocks());
it('rejects the request and does not proxy when auth-value resolution fails', async () => {
resolveConfigServers.mockResolvedValue({});
getUserMCPAuthMap.mockRejectedValue(new Error('db down'));
const req = { user: { id: 'user-1' }, body: { serverName: 'srv', uri: 'ui://x' } };
const res = makeRes();
await readMCPResource(req, res);
expect(readAppResource).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(500);
});
});

33
api/server/middleware/limiters/mcpAppResourceLimiter.js Normal file
View file

@ -0,0 +1,33 @@
const rateLimit = require('express-rate-limit');
const { limiterCache } = require('@librechat/api');
const { ViolationTypes } = require('librechat-data-provider');
const logViolation = require('~/cache/logViolation');
const { TOOL_CALL_VIOLATION_SCORE: score } = process.env;
const handler = async (req, res) => {
const type = ViolationTypes.TOOL_CALL_LIMIT;
const errorMessage = {
type,
max: 120,
limiter: 'user',
windowInMinutes: 1,
};
await logViolation(req, res, type, errorMessage, score);
res.status(429).json({ message: 'Too many app resource requests. Try again later' });
};
const limiterOptions = {
windowMs: 60 * 1000,
max: 120,
handler,
keyGenerator: function (req) {
return req.user?.id;
},
store: limiterCache('mcp_app_resource_limiter'),
};
const mcpAppResourceLimiter = rateLimit(limiterOptions);
module.exports = mcpAppResourceLimiter;

4
api/server/routes/mcp.js
View file

@ -40,6 +40,7 @@ const {
requireMCPAppsEnabled,
} = require('~/server/controllers/mcpApps');
const mcpAppToolCallLimiter = require('~/server/middleware/limiters/mcpAppToolCallLimiter');
const mcpAppResourceLimiter = require('~/server/middleware/limiters/mcpAppResourceLimiter');
const {
getOAuthReconnectionManager,
getMCPServersRegistry,
@ -998,6 +999,7 @@ router.post(
requireJwtAuth,
checkMCPUsePermissions,
requireMCPAppsEnabled,
mcpAppResourceLimiter,
readMCPResource,
);
@ -1010,6 +1012,7 @@ router.post(
requireJwtAuth,
checkMCPUsePermissions,
requireMCPAppsEnabled,
mcpAppResourceLimiter,
listMCPResources,
);
@ -1022,6 +1025,7 @@ router.post(
requireJwtAuth,
checkMCPUsePermissions,
requireMCPAppsEnabled,
mcpAppResourceLimiter,
listMCPResourceTemplates,
);

18
client/public/mcp-sandbox.html
View file

@ -170,11 +170,19 @@
});
function injectIntoHead(html, injection) {
if (/<head[^>]*>/i.test(html)) {
return html.replace(/<head([^>]*)>/i, '<head$1>' + injection);
}
if (/<html[^>]*>/i.test(html)) {
return html.replace(/<html([^>]*)>/i, '<html$1><head>' + injection + '</head>');
// A meta CSP only governs what is parsed after it, so it must come before any untrusted
// markup. If nothing but a doctype/<html> tag precedes <head>, insert the policy as the
// first head child and keep the document's structure. Otherwise (markup before <head>, or
// no <head>) wrap the document so no untrusted bytes are parsed ahead of the policy.
const headMatch = html.match(/<head[^>]*>/i);
if (headMatch) {
const beforeHead = html
.slice(0, headMatch.index)
.replace(/<!doctype[^>]*>/i, '')
.replace(/<html[^>]*>/i, '');
if (!/\S/.test(beforeHead)) {
return html.replace(/<head([^>]*)>/i, '<head$1>' + injection);
}
}
return (
'<!DOCTYPE html><html><head>' + injection + '</head><body>' + html + '</body></html>'

2
client/src/components/Chat/Messages/Content/ToolCall.tsx
View file

@ -90,7 +90,7 @@ const MCPAppView = React.memo(function MCPAppView({
<div className="my-2">
<iframe
srcDoc={app.text}
sandbox="allow-scripts allow-forms"
sandbox=""
style={{ width: '100%', minHeight: '200px', border: 'none' }}
title={app.uri}
/>

2
client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx
View file

@ -101,7 +101,7 @@ function MCPAppCard({
return (
<iframe
srcDoc={resource.text}
sandbox="allow-scripts allow-forms"
sandbox=""
style={{ width: '100%', height: '100%', border: 'none' }}
title={resource.uri}
/>

3
client/src/components/Chat/Messages/Content/__tests__/UIResourceCarousel.test.tsx
View file

@ -88,7 +88,8 @@ describe('UIResourceCarousel', () => {
render(<UIResourceCarousel uiResources={[inlineResource]} />);
const iframe = document.querySelector('iframe');
expect(iframe).toBeInTheDocument();
expect(iframe?.getAttribute('sandbox')).toBe('allow-scripts allow-forms');
// Non-app inline HTML renders inert (no allow-scripts); scripts run only via the sandbox proxy.
expect(iframe?.getAttribute('sandbox')).toBe('');
});
it('inline iframe does not have allow-same-origin', () => {

2
client/src/components/MCPUIResource/MCPUIResource.tsx
View file

@ -121,7 +121,7 @@ export function MCPUIResource(props: MCPUIResourceProps) {
<span className="mx-1 inline-block w-full align-middle">
<iframe
srcDoc={uiResource.text}
sandbox="allow-scripts allow-forms"
sandbox=""
style={{ width: '100%', minHeight: '200px', border: 'none' }}
title={uiResource.uri}
/>

5
client/src/components/MCPUIResource/__tests__/MCPUIResource.test.tsx
View file

@ -96,8 +96,9 @@ describe('MCPUIResource', () => {
const iframe = document.querySelector('iframe');
expect(iframe).toBeInTheDocument();
expect(iframe?.getAttribute('sandbox')).toBe('allow-scripts allow-forms');
expect(iframe?.getAttribute('sandbox')).not.toContain('allow-same-origin');
// Non-app (no profile=mcp-app) inline HTML renders inert: scripts/forms run only through the
// sandbox-proxy app path, so this static iframe must not grant allow-scripts.
expect(iframe?.getAttribute('sandbox')).toBe('');
});
it('renders nothing for resources that are not renderable', () => {

4
client/src/utils/mcpApps.ts
View file

@ -2,7 +2,7 @@ import { request, apiBaseUrl } from 'librechat-data-provider';
import type { UIResource } from 'librechat-data-provider';
export type AppToolResult = {
content: [];
content: unknown[];
structuredContent?: Record<string, unknown>;
isError?: boolean;
_meta?: Record<string, unknown>;
@ -28,7 +28,7 @@ export function isMcpAppResource(resource: UIResource): boolean {
*/
export function buildAppToolResult(resource: UIResource): AppToolResult | undefined {
const sc = resource.structuredContent as Record<string, unknown> | undefined | null;
const content = (resource.content as [] | undefined) ?? [];
const content = (resource.content as unknown[] | undefined) ?? [];
const meta = resource.resultMeta as Record<string, unknown> | undefined;
const hasStructured = !!sc && typeof sc === 'object' && !Array.isArray(sc);
const isAppBacked = !!(resource.toolName && resource.serverName);