From d87c12b7ec8cb5a4537974792d5a6f25878600ef Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Mon, 29 Jun 2026 14:43:52 -0700 Subject: [PATCH] fix(mcp): advertise UI capability at host level and align app-resource handling Per the MCP Apps spec (SEP-1865) the io.modelcontextprotocol/ui capability is negotiated once per client-server session and declares the host's rendering ability, not a per-user preference; per-tenant apps policy belongs in the host's downstream layer. Advertise the capability from the global apps setting at every connection-building path instead of the per-request resolved value, so a shared serverName-keyed connection no longer has its capability fixed by whichever scope opened it first. Per-tenant apps-enabled stays enforced downstream in callTool resource attachment and the app endpoints. Classify only text/html;profile=mcp-app resources as app-backed so the bridge metadata matches the client isMcpAppResource check; a plain text/html ui:// resource renders as a static srcDoc on both sides instead of carrying dead tool-result metadata. Redact the tool-result _meta (resultMeta) from ui_resources during share serialization so model-hidden metadata cannot ride into a public shared transcript. Resolve app follow-up requests through the originating tool call's connection path rather than forcing a user-scoped connection, which the app-level guard rejects for shared servers. --- packages/api/src/mcp/ConnectionsRepository.ts | 4 +- packages/api/src/mcp/MCPManager.ts | 38 ++++++------------ packages/api/src/mcp/UserConnectionManager.ts | 4 +- .../api/src/mcp/__tests__/MCPManager.test.ts | 10 ++--- .../api/src/mcp/__tests__/parsers.test.ts | 33 ++++++++++++++- packages/api/src/mcp/parsers.ts | 23 +++++++---- .../data-schemas/src/methods/share.test.ts | 40 +++++++++++++++++++ packages/data-schemas/src/methods/share.ts | 21 +++++++++- 8 files changed, 127 insertions(+), 46 deletions(-) diff --git a/packages/api/src/mcp/ConnectionsRepository.ts b/packages/api/src/mcp/ConnectionsRepository.ts index 76a315501f..5296f4fc82 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, appsEnabled } = + const { allowedDomains, allowedAddresses, useSSRFProtection } = await registry.resolveAllowlists({ userId: this.ownerId }); const connection = await MCPConnectionFactory.create( { @@ -88,7 +88,7 @@ export class ConnectionsRepository { useSSRFProtection, allowedDomains, allowedAddresses, - enableApps: appsEnabled, + enableApps: registry.getAppsEnabled(), }, this.oauthOpts, ); diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index f259f9af90..5041d7cea3 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -195,7 +195,7 @@ export class MCPManager extends UserConnectionManager { } const registry = MCPServersRegistry.getInstance(); - const { allowedDomains, allowedAddresses, useSSRFProtection, appsEnabled } = + const { allowedDomains, allowedAddresses, useSSRFProtection } = await registry.resolveAllowlists({ userId: user?.id, role: user?.role }); await this.assertResolvedRuntimeConfigAllowed({ config: serverConfig, @@ -217,7 +217,7 @@ export class MCPManager extends UserConnectionManager { useSSRFProtection, allowedDomains, allowedAddresses, - enableApps: appsEnabled, + enableApps: registry.getAppsEnabled(), }; const finalizeDiscoveryResult = async ( @@ -656,7 +656,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, appsEnabled } = + const { allowedDomains, allowedAddresses, useSSRFProtection } = await registry.resolveAllowlists({ userId, role: user?.role }); cleanupRequestOAuthHandler = MCPConnectionFactory.attachRequestOAuthHandler( { @@ -667,7 +667,7 @@ Please follow these instructions when using tools from the respective MCP server useSSRFProtection, allowedDomains, allowedAddresses, - enableApps: appsEnabled, + enableApps: registry.getAppsEnabled(), }, { useOAuth: true, @@ -837,28 +837,14 @@ Please follow these instructions when using tools from the respective MCP server } } - // A config-tier override for this name resolves above via configServers, but the shared - // app-connection pool resolves configs without it and would return the base server's - // connection. Route overrides through a request-scoped connection so iframe reads/calls reach - // the overridden server rather than the cached base one. - const hasConfigOverride = !!user && configServers?.[serverName] != null; - const connection = hasConfigOverride - ? await this.getUserConnection({ - serverName, - user, - serverConfig: rawConfig ?? undefined, - customUserVars, - flowManager, - tokenMethods, - }) - : await this.getConnection({ - serverName, - user, - serverConfig: rawConfig ?? undefined, - customUserVars, - flowManager, - tokenMethods, - }); + const connection = await this.getConnection({ + serverName, + user, + serverConfig: rawConfig ?? undefined, + customUserVars, + flowManager, + tokenMethods, + }); // Refresh headers when the config can be fully resolved: env-var-only configs always, and // customUserVar configs only when the route supplied those vars. Without them, re-processing diff --git a/packages/api/src/mcp/UserConnectionManager.ts b/packages/api/src/mcp/UserConnectionManager.ts index e4a8ec0173..785dfb6755 100644 --- a/packages/api/src/mcp/UserConnectionManager.ts +++ b/packages/api/src/mcp/UserConnectionManager.ts @@ -453,7 +453,7 @@ export abstract class UserConnectionManager { graphTokenResolver, }); const registry = MCPServersRegistry.getInstance(); - const { allowedDomains, allowedAddresses, useSSRFProtection, appsEnabled } = + const { allowedDomains, allowedAddresses, useSSRFProtection } = await registry.resolveAllowlists({ userId: user?.id, role: user?.role }); await this.assertResolvedRuntimeConfigAllowed({ config: runtimeConfig, @@ -472,7 +472,7 @@ export abstract class UserConnectionManager { useSSRFProtection, allowedDomains, allowedAddresses, - enableApps: appsEnabled, + enableApps: registry.getAppsEnabled(), ephemeralConnection, }; diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index 1a6e045554..83a2f102b8 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -1279,7 +1279,7 @@ describe('MCPManager', () => { }); }); - it('routes config-override app requests through a request-scoped connection honoring the override', async () => { + it('forwards configServers, flowManager, and tokenMethods to getConnection', async () => { (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({ source: 'yaml', type: 'sse', @@ -1295,8 +1295,8 @@ describe('MCPManager', () => { } as unknown as MCPConnection; const manager = await MCPManager.createInstance(newMCPServersConfig()); - const getUserConnectionSpy = jest - .spyOn(manager, 'getUserConnection') + const getConnectionSpy = jest + .spyOn(manager, 'getConnection') .mockResolvedValue(mockConnection); const flowManager = {} as never; @@ -1319,8 +1319,8 @@ describe('MCPManager', () => { 'user-123', configServers, ); - expect(getUserConnectionSpy).toHaveBeenCalledWith( - expect.objectContaining({ serverName: 'cfg-server', flowManager, tokenMethods }), + expect(getConnectionSpy).toHaveBeenCalledWith( + expect.objectContaining({ flowManager, tokenMethods }), ); }); diff --git a/packages/api/src/mcp/__tests__/parsers.test.ts b/packages/api/src/mcp/__tests__/parsers.test.ts index 93e788e14e..8339dd4572 100644 --- a/packages/api/src/mcp/__tests__/parsers.test.ts +++ b/packages/api/src/mcp/__tests__/parsers.test.ts @@ -301,12 +301,16 @@ describe('formatToolContent', () => { expect(artifacts).toBeUndefined(); }); - it('attaches the tool result to embedded ui:// resources for the app bridge', () => { + it('attaches the tool result to embedded mcp-app resources for the app bridge', () => { const result: t.MCPToolCallResponse = { content: [ { type: 'resource', - resource: { uri: 'ui://app', mimeType: 'text/html', text: '

hi

' }, + resource: { + uri: 'ui://app', + mimeType: 'text/html;profile=mcp-app', + text: '

hi

', + }, }, ], structuredContent: { count: 3 }, @@ -328,6 +332,31 @@ describe('formatToolContent', () => { expect(uiResourceArtifact?.content).toEqual(result.content); }); + it('renders a plain text/html ui:// resource statically without app-bridge metadata', () => { + const result: t.MCPToolCallResponse = { + content: [ + { + type: 'resource', + resource: { uri: 'ui://static', mimeType: 'text/html', text: '

hi

' }, + }, + ], + structuredContent: { count: 3 }, + }; + + const [content, artifacts] = formatToolContent(result, 'openai', { + serverName: 'srv', + toolName: 'do_thing', + }); + + const uiResourceArtifact = artifacts?.ui_resources?.data?.[0]; + expect(content).toContain('UI Resource Marker:'); + expect(uiResourceArtifact).toMatchObject({ uri: 'ui://static' }); + expect(uiResourceArtifact?.serverName).toBeUndefined(); + expect(uiResourceArtifact?.toolName).toBeUndefined(); + expect(uiResourceArtifact?.structuredContent).toBeUndefined(); + expect(uiResourceArtifact?.resultMeta).toBeUndefined(); + }); + it('suppresses embedded ui:// resources when apps are disabled for the scope', () => { const result: t.MCPToolCallResponse = { content: [ diff --git a/packages/api/src/mcp/parsers.ts b/packages/api/src/mcp/parsers.ts index 850b5eeaae..637f19592e 100644 --- a/packages/api/src/mcp/parsers.ts +++ b/packages/api/src/mcp/parsers.ts @@ -264,18 +264,27 @@ export function formatToolContent( const itemUi = (item.resource._meta as { ui?: Record } | undefined)?.ui as | { csp?: UIResource['csp']; permissions?: UIResource['permissions'] } | undefined; + // Only the text/html;profile=mcp-app profile runs the App Bridge on the client + // (isMcpAppResource); a plain text/html ui:// resource renders as a static srcDoc, so the + // tool result/context fields would be dead, misleading metadata on it. Classify the same + // way on both sides and attach the bridge payload only for the app profile. + const isAppBacked = (item.resource.mimeType ?? '').includes('profile=mcp-app'); const uiResource: UIResource = { ...item.resource, resourceId, - serverName: metadata?.serverName, - toolName: metadata?.toolName, - structuredContent: result?.structuredContent, - content: result?.content, - isError: result?.isError, - resultMeta: (result as { _meta?: Record })?._meta, csp: itemUi?.csp ?? metadata?.csp, permissions: itemUi?.permissions ?? metadata?.permissions, - toolArgs: metadata?.toolArgs, + ...(isAppBacked + ? { + serverName: metadata?.serverName, + toolName: metadata?.toolName, + structuredContent: result?.structuredContent, + content: result?.content, + isError: result?.isError, + resultMeta: (result as { _meta?: Record })?._meta, + toolArgs: metadata?.toolArgs, + } + : {}), }; uiResources.push(uiResource); resourceText.push(`UI Resource ID: ${resourceId}`); diff --git a/packages/data-schemas/src/methods/share.test.ts b/packages/data-schemas/src/methods/share.test.ts index 5f092fc26a..1b0ad7b831 100644 --- a/packages/data-schemas/src/methods/share.test.ts +++ b/packages/data-schemas/src/methods/share.test.ts @@ -543,6 +543,46 @@ describe('Share Methods', () => { expect(attachment).not.toHaveProperty('storageKey'); expect(attachment).not.toHaveProperty('metadata'); }); + + test('strips resultMeta from shared ui_resources while keeping render fields', async () => { + const userId = new mongoose.Types.ObjectId().toString(); + const conversationId = `conv_${nanoid()}`; + const shareId = `share_${nanoid()}`; + + const message = await Message.create({ + messageId: `msg_${nanoid()}`, + conversationId, + user: userId, + text: 'has app', + isCreatedByUser: false, + attachments: [ + { + type: 'ui_resources', + ui_resources: [ + { + resourceId: 'res1', + uri: 'ui://app', + mimeType: 'text/html;profile=mcp-app', + text: '

hi

', + serverName: 'srv', + toolName: 'do_thing', + resultMeta: { secret: 'hidden-from-model' }, + }, + ], + }, + ], + }); + + await SharedLink.create({ shareId, conversationId, user: userId, messages: [message._id] }); + + const result = await shareMethods.getSharedMessages(shareId); + const attachment = result?.messages[0]?.attachments?.[0] as unknown as { + ui_resources?: Array>; + }; + const resource = attachment?.ui_resources?.[0]; + expect(resource).toMatchObject({ uri: 'ui://app', resourceId: 'res1', text: '

hi

' }); + expect(resource).not.toHaveProperty('resultMeta'); + }); }); describe('getSharedLinks', () => { diff --git a/packages/data-schemas/src/methods/share.ts b/packages/data-schemas/src/methods/share.ts index f76cc0e29f..a0dee47296 100644 --- a/packages/data-schemas/src/methods/share.ts +++ b/packages/data-schemas/src/methods/share.ts @@ -67,6 +67,19 @@ const SENSITIVE_SHARED_FILE_FIELDS = new Set([ 'metadata', ]); +/** + * The MCP tool result `_meta` is carried on a UI resource as `resultMeta` for the App Bridge to + * hydrate from, but it is intentionally kept out of the model-visible result and must not become + * part of a public shared transcript. MCP apps never render in a shared view anyway, so drop it. + */ +function sanitizeSharedUIResource(resource: unknown): unknown { + if (!resource || typeof resource !== 'object' || Array.isArray(resource)) { + return resource; + } + const { resultMeta: _resultMeta, ...rest } = resource as Record; + return rest; +} + /** * Strip storage/identity-internal fields from a file or attachment while keeping * render-relevant data (including tool-call payloads keyed by tool name). @@ -78,9 +91,13 @@ function sanitizeSharedFile(value: unknown): t.SharedFile | null { const result: t.SharedFile = {}; for (const [key, fieldValue] of Object.entries(value as Record)) { - if (!SENSITIVE_SHARED_FILE_FIELDS.has(key)) { - result[key] = fieldValue; + if (SENSITIVE_SHARED_FILE_FIELDS.has(key)) { + continue; } + result[key] = + key === 'ui_resources' && Array.isArray(fieldValue) + ? fieldValue.map(sanitizeSharedUIResource) + : fieldValue; } return Object.keys(result).length > 0 ? result : null;