From 624a6d8f4b4e480ca9899da4dcfbcc42e2869ddf Mon Sep 17 00:00:00 2001
From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com>
Date: Mon, 29 Jun 2026 11:07:55 -0700
Subject: [PATCH] fix(mcp): gate apps per-request on app connections and
embedded UI resources
Resolve the allowlist-derived appsEnabled value when creating app-level connections in
ConnectionsRepository so a tenant/role/user override that toggles apps is honored instead of the
boot YAML default.
Gate ui:// resources embedded in tool results on the same per-request setting so a disabled scope
renders them as plain resource text rather than a sandboxed app, resolving appsEnabled lazily only
when a result actually carries a renderable UI resource.
Fail closed in canonicalizeUri when a URI does not stabilize within the decode cap so traversal
encoded more deeply than the cap cannot satisfy a template guard a fully-decoding server resolves
as a parent-directory path.
---
packages/api/src/mcp/ConnectionsRepository.ts | 4 +-
packages/api/src/mcp/MCPManager.ts | 46 ++++++++++++-------
.../api/src/mcp/__tests__/MCPManager.test.ts | 30 ++++++++++++
.../api/src/mcp/__tests__/parsers.test.ts | 21 +++++++++
packages/api/src/mcp/parsers.ts | 37 ++++++++++++---
5 files changed, 113 insertions(+), 25 deletions(-)
diff --git a/packages/api/src/mcp/ConnectionsRepository.ts b/packages/api/src/mcp/ConnectionsRepository.ts
index 5296f4fc82..76a315501f 100644
--- a/packages/api/src/mcp/ConnectionsRepository.ts
+++ b/packages/api/src/mcp/ConnectionsRepository.ts
@@ -78,7 +78,7 @@ export class ConnectionsRepository {
}
}
const registry = MCPServersRegistry.getInstance();
- const { allowedDomains, allowedAddresses, useSSRFProtection } =
+ const { allowedDomains, allowedAddresses, useSSRFProtection, appsEnabled } =
await registry.resolveAllowlists({ userId: this.ownerId });
const connection = await MCPConnectionFactory.create(
{
@@ -88,7 +88,7 @@ export class ConnectionsRepository {
useSSRFProtection,
allowedDomains,
allowedAddresses,
- enableApps: registry.getAppsEnabled(),
+ enableApps: appsEnabled,
},
this.oauthOpts,
);
diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts
index e364f126c3..178b161a4f 100644
--- a/packages/api/src/mcp/MCPManager.ts
+++ b/packages/api/src/mcp/MCPManager.ts
@@ -32,6 +32,7 @@ import {
requiresUserScopedConnection,
} from './utils';
import { mcpOptionsContainGraphTokenPlaceholder, preProcessGraphTokens } from '~/utils/graph';
+import { formatToolContent, resultHasRenderableUiResource } from './parsers';
import { getToolUiResourceUri, isToolVisibilityModelOnly } from './apps';
import { MCPServersInitializer } from './registry/MCPServersInitializer';
import { OboTokenResolutionError, resolveOboToken } from '~/mcp/oauth';
@@ -40,7 +41,6 @@ import { MCPServersRegistry } from './registry/MCPServersRegistry';
import { UserConnectionManager } from './UserConnectionManager';
import { ConnectionsRepository } from './ConnectionsRepository';
import { MCPConnectionFactory } from './MCPConnectionFactory';
-import { formatToolContent } from './parsers';
import { MCPConnection } from './connection';
import { processMCPEnv } from '~/utils/env';
@@ -726,24 +726,28 @@ Please follow these instructions when using tools from the respective MCP server
userId,
requiresEphemeralUserConnection(rawConfig),
);
- if (resourceMeta) {
- // 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;
- } else {
- logger.debug(
- `[MCP][${serverName}][${toolName}] Found resourceUri: ${resourceMeta.uri}`,
- );
- }
- }
} catch {
/* empty */
}
}
+ // The apps toggle must gate both the tool-declared app and any ui:// resource embedded in the
+ // result (either renders an iframe that breaks once the gated app endpoints reject follow-up
+ // calls). Resolved lazily, only when a UI resource is in play, so plain tool calls skip the
+ // per-request lookup.
+ let enableApps = true;
+ if (resourceMeta || resultHasRenderableUiResource(result as t.MCPToolCallResponse)) {
+ ({ appsEnabled: enableApps } = await registry.resolveAllowlists({
+ userId,
+ role: user?.role,
+ }));
+ if (!enableApps) {
+ resourceMeta = undefined;
+ } else if (resourceMeta) {
+ logger.debug(`[MCP][${serverName}][${toolName}] Found resourceUri: ${resourceMeta.uri}`);
+ }
+ }
+
return formatToolContent(
result as t.MCPToolCallResponse,
provider,
@@ -755,8 +759,9 @@ Please follow these instructions when using tools from the respective MCP server
csp: resourceMeta?.csp,
permissions: resourceMeta?.permissions,
toolArgs: toolArguments,
+ enableApps,
}
- : undefined,
+ : { enableApps },
);
} catch (error) {
// Log with context and re-throw or handle as needed
@@ -1033,11 +1038,14 @@ Please follow these instructions when using tools from the respective MCP server
/**
* 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.
+ * cannot be decoded, does not stabilize within the decode cap, or contains a relative (`.`/`..`)
+ * segment, so neither deeply encoded traversal nor relative segments can satisfy a template
+ * guard. Failing closed on the cap matters because a server that decodes until stable would
+ * otherwise receive a traversal this guard never saw in decoded form.
*/
private static canonicalizeUri(uri: string): string | null {
let current = uri;
+ let stabilized = false;
for (let depth = 0; depth < 5; depth++) {
let decoded: string;
try {
@@ -1046,10 +1054,14 @@ Please follow these instructions when using tools from the respective MCP server
return null;
}
if (decoded === current) {
+ stabilized = true;
break;
}
current = decoded;
}
+ if (!stabilized) {
+ return null;
+ }
if (current.split(/[/\\]/).some((segment) => segment === '.' || segment === '..')) {
return null;
}
diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts
index 20dc87954f..99453d316b 100644
--- a/packages/api/src/mcp/__tests__/MCPManager.test.ts
+++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts
@@ -1481,6 +1481,36 @@ describe('MCPManager', () => {
const methods = request.mock.calls.map((c) => (c[0] as { method: string }).method);
expect(methods).not.toContain('resources/read');
});
+
+ it('rejects traversal encoded more deeply than the decode cap rather than treating it as canonical', async () => {
+ let traversal = '%2e%2e%2f';
+ for (let i = 0; i < 6; i++) {
+ traversal = traversal.replace(/%/g, '%25');
+ }
+ 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/${traversal}secret`,
+ 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', () => {
diff --git a/packages/api/src/mcp/__tests__/parsers.test.ts b/packages/api/src/mcp/__tests__/parsers.test.ts
index dec533d4a2..93e788e14e 100644
--- a/packages/api/src/mcp/__tests__/parsers.test.ts
+++ b/packages/api/src/mcp/__tests__/parsers.test.ts
@@ -328,6 +328,27 @@ describe('formatToolContent', () => {
expect(uiResourceArtifact?.content).toEqual(result.content);
});
+ it('suppresses embedded ui:// resources when apps are disabled for the scope', () => {
+ const result: t.MCPToolCallResponse = {
+ content: [
+ {
+ type: 'resource',
+ resource: { uri: 'ui://app', mimeType: 'text/html', text: 'hi
' },
+ },
+ ],
+ };
+
+ const [content, artifacts] = formatToolContent(result, 'openai', {
+ serverName: 'srv',
+ toolName: 'do_thing',
+ enableApps: false,
+ });
+
+ expect(artifacts?.ui_resources).toBeUndefined();
+ expect(content).toContain('Resource URI: ui://app');
+ expect(content).not.toContain('UI Resource Marker:');
+ });
+
it('gives embedded ui:// resources distinct ids per tool result payload', () => {
const resourceIdFor = (sc: Record) =>
formatToolContent(
diff --git a/packages/api/src/mcp/parsers.ts b/packages/api/src/mcp/parsers.ts
index cfdc34767d..850b5eeaae 100644
--- a/packages/api/src/mcp/parsers.ts
+++ b/packages/api/src/mcp/parsers.ts
@@ -155,6 +155,32 @@ function parseAsString(result: t.MCPToolCallResponse): string {
return text;
}
+/**
+ * MCP Apps renders only `ui://` resources whose mime type is HTML (mime omitted defaults to HTML),
+ * the single renderable resource type the spec defines. Used both when attaching a result resource
+ * and when deciding whether a result carries an app the apps toggle must gate.
+ */
+export function isRenderableUiResource(item: t.ToolContentPart): boolean {
+ if (item.type !== 'resource') {
+ return false;
+ }
+ const uri = item.resource?.uri;
+ if (typeof uri !== 'string' || !uri.startsWith('ui://')) {
+ return false;
+ }
+ const mimeType =
+ typeof item.resource.mimeType === 'string' ? item.resource.mimeType : 'text/html';
+ return mimeType.includes('html');
+}
+
+export function resultHasRenderableUiResource(result: t.MCPToolCallResponse): boolean {
+ const content = result?.content;
+ if (!Array.isArray(content)) {
+ return false;
+ }
+ return content.some((item) => isRenderableUiResource(item as t.ToolContentPart));
+}
+
/**
* Converts MCPToolCallResponse content into a plain-text string plus optional artifacts
* (images, UI resources). All providers receive string content; images are separated into
@@ -174,6 +200,7 @@ export function formatToolContent(
csp?: UIResource['csp'];
permissions?: UIResource['permissions'];
toolArgs?: Record;
+ enableApps?: boolean;
},
): t.FormattedContentResult {
if (!RECOGNIZED_PROVIDERS.has(provider)) {
@@ -216,12 +243,10 @@ export function formatToolContent(
},
resource: (item) => {
- // MCP Apps defines a single renderable resource type, text/html;profile=mcp-app, and the
- // host renders HTML only. ui:// resources with other mime types (json, remote-dom) have no
- // renderer, so they fall through to plain resource text instead of an unrenderable marker.
- const mimeType =
- typeof item.resource.mimeType === 'string' ? item.resource.mimeType : 'text/html';
- const isUiResource = item.resource.uri.startsWith('ui://') && mimeType.includes('html');
+ // ui:// resources that are not renderable apps (other mime types) or that arrive for a
+ // scope with apps disabled fall through to plain resource text rather than an unrenderable
+ // or admin-suppressed app marker.
+ const isUiResource = metadata?.enableApps !== false && isRenderableUiResource(item);
const resourceText: string[] = [];
if (isUiResource) {