Skip to content

Commit 69029ee

Browse files
committed
fix(mcp): isolate rejected files before publishing results
1 parent 027adbc commit 69029ee

6 files changed

Lines changed: 184 additions & 76 deletions

File tree

‎apps/sim/lib/internal/mcp/presentation.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,12 @@ export async function presentMcpToolResult(
5454
const registry = context.resolvedSecretTraceRegistry?.forkForPropagatedEntries()
5555
const value = {
5656
arguments: presentation.arguments,
57-
result: projectMcpEncodedContents(result, registry),
58-
resources: projectMcpEncodedContents({ contents: presentation.resources ?? [] }, registry)
59-
.contents,
57+
result: projectMcpEncodedContents(result, registry, 'omit'),
58+
resources: projectMcpEncodedContents(
59+
{ contents: presentation.resources ?? [] },
60+
registry,
61+
'omit'
62+
).contents,
6063
tool: {
6164
name: tool.name,
6265
title: tool.title || tool.name,

‎apps/sim/lib/mcp/application/presentation.integration.ts‎

Lines changed: 112 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@ let appCalls = 0
6969
let appAvailable = true
7070
let listingOnlyPolicy = false
7171
let providerTitle = 'Quarterly report'
72+
let providerMime = 'text/plain'
7273
let linkedReportText = 'Remote resource bytes'
7374
let linkedResourceMissing = false
75+
let linkedResourceProtected = false
7476
let resourceReads = 0
7577
let echoAppUri = false
7678
let connectDomain = 'https://allowed.test'
@@ -180,9 +182,9 @@ const provider = createServer(async (request, response) => {
180182
name: providerTitle,
181183
title: providerTitle,
182184
uri: sourceUri,
183-
mimeType: 'text/plain',
185+
mimeType: providerMime,
184186
},
185-
...(linkedResourceMissing
187+
...(linkedResourceMissing || linkedResourceProtected
186188
? [
187189
{
188190
type: 'resource' as const,
@@ -232,7 +234,29 @@ const provider = createServer(async (request, response) => {
232234
],
233235
}
234236
if (params.arguments?.malformed)
235-
return { content: [{ type: 'image', mimeType: 'image/png', data: 'YR==' }] }
237+
return {
238+
content: [
239+
...(params.arguments.malformed === 'base64'
240+
? [{ type: 'image' as const, mimeType: 'image/png', data: 'YR==' }]
241+
: [
242+
{
243+
type: 'resource' as const,
244+
resource: {
245+
uri: 'file:///rejected.bin',
246+
mimeType:
247+
params.arguments.malformed === 'protected'
248+
? 'application/octet-stream'
249+
: 'text/plain; charset=unsupported',
250+
blob: Buffer.from(credentialCanary).toString('base64'),
251+
},
252+
},
253+
]),
254+
{
255+
type: 'resource',
256+
resource: { uri: sourceUri, mimeType: 'text/plain', text: 'Valid attachment' },
257+
},
258+
],
259+
}
236260
return {
237261
content: [
238262
{ type: 'text' as const, text: 'Report ready' },
@@ -268,13 +292,23 @@ const provider = createServer(async (request, response) => {
268292
}))
269293
protocol.setRequestHandler(ReadResourceRequestSchema, async ({ params }) => {
270294
resourceReads++
295+
if (linkedResourceProtected && params.uri === sourceUri)
296+
return {
297+
contents: [
298+
{
299+
uri: sourceUri,
300+
mimeType: 'application/octet-stream',
301+
blob: Buffer.from(credentialCanary).toString('base64'),
302+
},
303+
],
304+
}
271305
if (linkedResourceMissing && params.uri === sourceUri)
272306
throw new Error('Synthetic missing linked resource')
273307
return {
274308
contents: [
275309
{
276310
uri: params.uri,
277-
mimeType: `${params.uri === appUri ? 'text/html;profile=mcp-app' : 'text/plain'}${encodedCredential ? `; charset=${encodedCharset}` : params.uri === appUri ? '; charset=utf-8' : ''}`,
311+
mimeType: `${params.uri === appUri ? 'text/html;profile=mcp-app' : providerMime}${encodedCredential ? `; charset=${encodedCharset}` : params.uri === appUri ? '; charset=utf-8' : ''}`,
278312
...(encodedCredential
279313
? {
280314
blob: encodeFixtureText(
@@ -432,6 +466,7 @@ beforeAll(async () => {
432466
afterEach(() => {
433467
linkedReportText = 'Remote resource bytes'
434468
linkedResourceMissing = false
469+
linkedResourceProtected = false
435470
echoAppUri = false
436471
})
437472

@@ -627,34 +662,38 @@ describe('native MCP results over real transport, storage and Postgres', () => {
627662
}
628663
)
629664

630-
it('preserves valid attachments and the App when a linked snapshot fails', async () => {
631-
linkedResourceMissing = true
632-
const { result, receipt } = await executeReport({ linked: true })
633-
expect(receipt.hasApp).toBe(true)
634-
expect(receipt.items.map((item) => item.index)).toEqual([1, 2])
635-
const asset = await readMcpResultAsset.execute({
636-
principal: session,
637-
input: { chatId, id: receipt.id, index: 1 },
638-
})
639-
expect(asset.buffer.toString()).toContain('Encoded report: ')
640-
expect(asset.buffer.toString()).not.toContain(credentialCanary)
641-
const later = await readMcpResultAsset.execute({
642-
principal: session,
643-
input: { chatId, id: receipt.id, index: 2 },
644-
})
645-
expect(later.buffer.toString()).toBe('Remote resource bytes')
646-
await expect(
647-
readMcpResultAsset.execute({
665+
it.each([false, true])(
666+
'preserves valid attachments when a link fails (protected=%s)',
667+
async (protectedBytes) => {
668+
linkedResourceMissing = !protectedBytes
669+
linkedResourceProtected = protectedBytes
670+
const { result, receipt } = await executeReport({ linked: true })
671+
expect(receipt.hasApp).toBe(true)
672+
expect(receipt.items.map((item) => item.index)).toEqual([1, 2])
673+
const asset = await readMcpResultAsset.execute({
648674
principal: session,
649-
input: { chatId, id: receipt.id, index: 0 },
675+
input: { chatId, id: receipt.id, index: 1 },
650676
})
651-
).rejects.toThrow('MCP file not found')
652-
expect(JSON.stringify(result.output)).not.toContain('Only the app should receive this')
653-
expect(JSON.stringify(result.output)).not.toContain(
654-
Buffer.from(`Encoded report: ${credentialCanary}:end`).toString('base64')
655-
)
656-
expect(JSON.stringify(result.output)).toContain('could not be displayed')
657-
})
677+
expect(asset.buffer.toString()).toContain('Encoded report: ')
678+
expect(asset.buffer.toString()).not.toContain(credentialCanary)
679+
const later = await readMcpResultAsset.execute({
680+
principal: session,
681+
input: { chatId, id: receipt.id, index: 2 },
682+
})
683+
expect(later.buffer.toString()).toBe('Remote resource bytes')
684+
await expect(
685+
readMcpResultAsset.execute({
686+
principal: session,
687+
input: { chatId, id: receipt.id, index: 0 },
688+
})
689+
).rejects.toThrow('MCP file not found')
690+
expect(JSON.stringify(result.output)).not.toContain('Only the app should receive this')
691+
expect(JSON.stringify(result.output)).not.toContain(
692+
Buffer.from(`Encoded report: ${credentialCanary}:end`).toString('base64')
693+
)
694+
expect(JSON.stringify(result.output)).toContain('could not be displayed')
695+
}
696+
)
658697

659698
it('does not download linked resources returned by a live App call', async () => {
660699
const { receipt } = await executeReport()
@@ -726,25 +765,44 @@ describe('native MCP results over real transport, storage and Postgres', () => {
726765
}
727766
})
728767

729-
it('rejects malformed provider bytes and aborts App calls before provider mutation', async () => {
730-
const { receipt } = await executeReport({ malformed: true })
731-
await expect(
732-
readMcpResultAsset.execute({
768+
it.each(['base64', 'protected', 'charset'])(
769+
'withholds a rejected %s file while retaining attachments and the App',
770+
async (problem) => {
771+
const { receipt } = await executeReport({ malformed: problem })
772+
expect(receipt.items.map((item) => item.index)).toEqual([1])
773+
const asset = await readMcpResultAsset.execute({
733774
principal: session,
734-
input: { chatId, id: receipt.id, index: 0 },
775+
input: { chatId, id: receipt.id, index: 1 },
735776
})
736-
).rejects.toThrow('Invalid MCP file encoding')
737-
const before = appCalls
738-
const controller = new AbortController()
739-
controller.abort()
740-
await expect(
741-
callMcpAppTool.execute({
777+
expect(asset.buffer.toString()).toBe('Valid attachment')
778+
const frame = await readMcpAppFrame.execute({
779+
principal: session,
780+
input: { chatId, id: receipt.id },
781+
})
782+
expect(frame.buffer.length).toBeGreaterThan(0)
783+
const saved = await readMcpResult.execute({
742784
principal: session,
743-
input: { chatId, id: receipt.id, name: 'change_report', signal: controller.signal },
785+
input: { chatId, id: receipt.id },
744786
})
745-
).rejects.toThrow()
746-
expect(appCalls).toBe(before)
747-
})
787+
expect(JSON.stringify(saved)).not.toContain(Buffer.from(credentialCanary).toString('base64'))
788+
await expect(
789+
readMcpResultAsset.execute({
790+
principal: session,
791+
input: { chatId, id: receipt.id, index: 0 },
792+
})
793+
).rejects.toThrow('MCP file not found')
794+
const before = appCalls
795+
const controller = new AbortController()
796+
controller.abort()
797+
await expect(
798+
callMcpAppTool.execute({
799+
principal: session,
800+
input: { chatId, id: receipt.id, name: 'change_report', signal: controller.signal },
801+
})
802+
).rejects.toThrow()
803+
expect(appCalls).toBe(before)
804+
}
805+
)
748806

749807
it('uses authenticated listing metadata when the App read omits its policy', async () => {
750808
const { receipt } = await executeReport()
@@ -760,16 +818,24 @@ describe('native MCP results over real transport, storage and Postgres', () => {
760818
listingOnlyPolicy = false
761819
}
762820
})
763-
it('opens results with long provider tool and resource titles', async () => {
821+
it.each([
822+
['text/plain', 'text/plain'],
823+
[`text/plain; description="${'long parameter '.repeat(20)}"`, 'text/plain'],
824+
[`application/${'x'.repeat(150)}`, 'application/octet-stream'],
825+
])('opens long provider metadata with MIME %s', async (mimeType, expectedMime) => {
764826
providerTitle = 'Long report title '.repeat(20)
827+
providerMime = mimeType
765828
try {
766829
const { receipt } = await executeReport({ linked: true })
767830
const asset = await readMcpResultAsset.execute({
768831
principal: session,
769832
input: { chatId, id: receipt.id, index: 0 },
770833
})
771834
expect(asset.buffer.toString()).toBe('Remote resource bytes')
835+
expect(asset.contentType).toBe(expectedMime)
836+
expect(receipt.items[0].mimeType).toBe(expectedMime)
772837
} finally {
838+
providerMime = 'text/plain'
773839
providerTitle = 'Quarterly report'
774840
}
775841
})

‎apps/sim/lib/mcp/encoded-content.ts‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@ type McpContentResult = Pick<CallToolResult, 'content'> | Pick<ReadResourceResul
1111
/** Checks decoded provider bytes before an encoded resource crosses into a saved or live App. */
1212
export function projectMcpEncodedContents<T extends McpContentResult>(
1313
value: T,
14-
registry: ResolvedSecretTraceRegistry | undefined
14+
registry: ResolvedSecretTraceRegistry | undefined,
15+
rejectedFiles: 'error' | 'omit' = 'error'
1516
): T {
1617
if (!isJsonWithinByteLimit(value, MCP_PRESENTATION_MAX_BYTES))
1718
throw new OrchestrationError('payload_too_large', 'MCP result exceeds 12 MiB')
18-
const project = (encoded: string, mimeType?: string) => {
19-
const bytes = Buffer.from(encoded, 'base64')
19+
const projectFile = (encoded: string, mimeType?: string) => {
20+
const bytes = decodeMcpBase64(encoded)
2021
let mime: MIMEType | undefined
2122
try {
2223
mime = mimeType ? new MIMEType(mimeType) : undefined
@@ -60,24 +61,43 @@ export function projectMcpEncodedContents<T extends McpContentResult>(
6061
mime?.params.delete('charset')
6162
return { encoded: Buffer.from(projection.value).toString('base64'), mimeType: mime?.toString() }
6263
}
64+
const project = (encoded: string, mimeType?: string) => {
65+
try {
66+
return projectFile(encoded, mimeType)
67+
} catch (error) {
68+
if (
69+
rejectedFiles === 'omit' &&
70+
error instanceof OrchestrationError &&
71+
(error.code === 'validation' || error.code === 'forbidden')
72+
)
73+
return undefined
74+
throw error
75+
}
76+
}
6377
if ('contents' in value)
6478
return {
6579
...value,
66-
contents: value.contents.map((resource) => {
67-
if (!('blob' in resource)) return resource
80+
contents: value.contents.flatMap<ReadResourceResult['contents'][number]>((resource) => {
81+
if (!('blob' in resource)) return [resource]
6882
const projected = project(resource.blob, resource.mimeType)
69-
return { ...resource, blob: projected.encoded, mimeType: projected.mimeType }
83+
return projected
84+
? [{ ...resource, blob: projected.encoded, mimeType: projected.mimeType }]
85+
: []
7086
}),
7187
}
7288
return {
7389
...value,
7490
content: value.content.map((item) => {
7591
if (item.type === 'image' || item.type === 'audio') {
7692
const projected = project(item.data, item.mimeType)
93+
if (!projected)
94+
return { type: 'text' as const, text: 'MCP file output could not be displayed.' }
7795
return { ...item, data: projected.encoded, mimeType: projected.mimeType ?? item.mimeType }
7896
}
7997
if (item.type === 'resource' && 'blob' in item.resource) {
8098
const projected = project(item.resource.blob, item.resource.mimeType)
99+
if (!projected)
100+
return { type: 'text' as const, text: 'MCP file output could not be displayed.' }
81101
return {
82102
...item,
83103
resource: { ...item.resource, blob: projected.encoded, mimeType: projected.mimeType },
@@ -87,3 +107,13 @@ export function projectMcpEncodedContents<T extends McpContentResult>(
87107
}),
88108
}
89109
}
110+
111+
/** Decodes canonical bounded MCP file bytes for publication and historical asset reads. */
112+
export function decodeMcpBase64(value: string): Buffer {
113+
if (value.length > MCP_PRESENTATION_MAX_BYTES || value.length % 4 !== 0)
114+
throw new OrchestrationError('validation', 'Invalid MCP file encoding')
115+
const buffer = Buffer.from(value, 'base64')
116+
if (buffer.toString('base64') !== value)
117+
throw new OrchestrationError('validation', 'Invalid MCP file encoding')
118+
return buffer
119+
}

‎apps/sim/lib/mcp/presentation-storage.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createHash } from 'node:crypto'
2+
import { MIMEType } from 'node:util'
23
import {
34
CallToolResultSchema,
45
type ReadResourceResult,
@@ -72,10 +73,15 @@ export async function storeMcpPresentation(input: {
7273
? input.resources?.find((resource) => resource.uri === item.uri)
7374
: undefined
7475
if (item.type === 'resource_link' && !snapshot) return []
75-
const mimeType =
76+
const declaredMimeType =
7677
item.type === 'image' || item.type === 'audio'
7778
? item.mimeType
7879
: snapshot?.mimeType || resource?.mimeType || 'application/octet-stream'
80+
let mimeType = 'application/octet-stream'
81+
try {
82+
const essence = new MIMEType(declaredMimeType).essence
83+
if (essence.length <= 128) mimeType = essence
84+
} catch {}
7985
const identity = resource
8086
? digest(`${input.workspaceId}:${input.connectionId}:${resource.uri}`)
8187
: `${id}:${index}`

0 commit comments

Comments
 (0)