Skip to content

Commit 1fb8cb3

Browse files
committed
fix(mcp): preserve snapshots and protect managed app results
1 parent 16ba3d8 commit 1fb8cb3

23 files changed

Lines changed: 855 additions & 249 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-category.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ vi.mock('@/lib/uploads/utils/validation', () => ({
77

88
vi.mock('@/lib/uploads/utils/file-utils', () => fileUtilsMock)
99

10-
import { resolveFileCategory } from './file-category'
10+
import { resolveFileCategory } from '@/lib/uploads/utils/file-category'
1111

1212
describe('resolveFileCategory — MIME priority', () => {
1313
it('text/plain MIME + .pdf extension → text-editable (MIME wins)', () => {

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { Music } from '@sim/emcn/icons'
55
import dynamic from 'next/dynamic'
66
import type { FileDownloadSource } from '@/lib/uploads/client/download'
77
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
8+
import { resolveFileCategory } from '@/lib/uploads/utils/file-category'
89
import { resolveMediaMimeType } from '@/lib/uploads/utils/file-utils'
910
import {
1011
useWorkspaceFileBinary,
@@ -18,7 +19,6 @@ import {
1819
} from '@/hooks/use-file-content-source'
1920
import { CsvTablePreview } from './csv-table-preview'
2021
import { DocxPreview } from './docx-preview'
21-
import { resolveFileCategory } from './file-category'
2222
import { ImagePreview } from './image-preview'
2323
import type { PdfDocumentSource } from './pdf-viewer'
2424
import { PptxPreview } from './pptx-preview'

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export { resolveFileCategory } from './file-category'
1+
export { resolveFileCategory } from '@/lib/uploads/utils/file-category'
22
export type { PreviewMode } from './file-viewer'
33
export {
44
FileViewer,

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/mcp-result/mcp-result.tsx‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { useRef, useState } from 'react'
44
import { Chip, ChipLink } from '@sim/emcn'
55
import { type McpPresentationReceipt, mcpPresentationAssetUrl } from '@/lib/mcp/presentation'
6+
import { resolveFileCategory } from '@/lib/uploads/utils/file-category'
67
import { useChatSurface } from '@/app/workspace/[workspaceId]/home/components/chat-surface-context'
78
import { McpApp } from '@/app/workspace/[workspaceId]/home/components/message-content/components/mcp-result/mcp-app'
89
import { useOptionalMothershipResources } from '@/app/workspace/[workspaceId]/home/components/mothership-resources-context'
@@ -50,18 +51,20 @@ export function McpResult({ receipt }: McpResultProps) {
5051
key={`${item.identity}:${item.index}`}
5152
className='flex min-w-0 flex-col items-start gap-2'
5253
>
53-
{item.kind === 'image' && (
54-
<img
55-
src={url}
56-
alt={item.title}
57-
loading='lazy'
58-
className='max-h-[400px] max-w-full rounded-lg object-contain'
59-
/>
60-
)}
61-
{item.kind === 'audio' && (
62-
// biome-ignore lint/a11y/useMediaCaption: MCP audio results do not include a caption track.
63-
<audio src={url} controls preload='none' aria-label={item.title} />
64-
)}
54+
{item.kind === 'image' &&
55+
resolveFileCategory(item.mimeType, '') === 'image-previewable' && (
56+
<img
57+
src={url}
58+
alt={item.title}
59+
loading='lazy'
60+
className='max-h-[400px] max-w-full rounded-lg object-contain'
61+
/>
62+
)}
63+
{item.kind === 'audio' &&
64+
resolveFileCategory(item.mimeType, '') === 'audio-previewable' && (
65+
// biome-ignore lint/a11y/useMediaCaption: MCP audio results do not include a caption track.
66+
<audio src={url} controls preload='none' aria-label={item.title} />
67+
)}
6568
{resources ? (
6669
<Chip
6770
variant='border'

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/mcp-resource-content.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { getErrorMessage } from '@sim/utils/errors'
55
import { mcpPresentationAssetUrl } from '@/lib/mcp/presentation'
66
import type { MothershipResource } from '@/lib/mothership/resources/types'
77
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
8-
import { resolveFileCategory } from '@/app/workspace/[workspaceId]/files/components/file-viewer/file-category'
8+
import { resolveFileCategory } from '@/lib/uploads/utils/file-category'
99
import { FileViewer } from '@/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer'
1010
import { useMcpPresentationMetadata } from '@/hooks/queries/mcp-presentations'
1111
import type { FileContentSource } from '@/hooks/use-file-content-source'
@@ -41,7 +41,7 @@ function McpArtifactPreview({ chatId, presentationId, index }: McpArtifactPrevie
4141
['application/json', 'application/xml'].includes(item.mimeType)
4242
const previewable =
4343
plainText ||
44-
item.kind === 'image' ||
44+
(item.kind === 'image' && resolveFileCategory(item.mimeType, '') === 'image-previewable') ||
4545
(item.kind === 'audio' && resolveFileCategory(item.mimeType, '') === 'audio-previewable') ||
4646
item.mimeType === 'application/pdf'
4747
if (!previewable)

‎apps/sim/lib/credential-groups/application/authorization.ts‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,7 @@ import {
1919
isOrganizationCredentialType,
2020
type OrganizationCredentialType,
2121
} from '@/lib/credential-groups/credential-types'
22-
import type {
23-
CredentialGroupCredentialListContext,
24-
ManagedCredentialGroupBinding,
25-
} from '@/lib/credential-groups/credentials'
22+
import type { CredentialGroupCredentialListContext } from '@/lib/credential-groups/credentials'
2623
import {
2724
isManagedCredentialGroupBindingLive,
2825
loadCredentialGroupEnrollmentAccessForSubject,
@@ -108,26 +105,21 @@ export function requireCredentialGroupWorkflowActor(principal: Principal): Princ
108105
}
109106

110107
/**
111-
* Authorizes a person using their own Credential Group credential from Chat.
112-
* The copilot delegation names the signed-in user and no workflow, so only the
108+
* Authorizes a person using their own Credential Group credential from Chat or an App.
109+
* The session or copilot delegation names the signed-in user and no workflow, so only the
113110
* actor statement is evaluated: the credential must be the one collected under
114111
* that user's own live enrollment. Nothing the model passes can widen this;
115112
* the acting user is the delegation's subject, not a tool argument.
116113
*/
117114
async function requireCredentialGroupActorCredentialAccess(
118-
principal: Extract<Principal, { kind: 'delegated' }>,
115+
principal: Principal,
119116
context: CredentialGroupAuthorizationContext & { credentialGroupEnrollmentId: string },
120-
binding: ManagedCredentialGroupBinding | null,
121117
resourcePolicy: ResourcePolicyBindingFor<'credential_group'>
122118
): Promise<void> {
123119
const subject = resolvePrincipalSubject(principal)
124120
if (subject?.kind !== 'sim_user' || !subject.userId) {
125121
throw new OrchestrationError('forbidden', 'Credential Group actor access required')
126122
}
127-
/** Chat mints OAuth credentials only; a credential with no OAuth binding is not its to use. */
128-
if (!binding) {
129-
throw new OrchestrationError('forbidden', 'Credential Group credential access denied')
130-
}
131123
if (context.organizationId) {
132124
const actorAccess = await loadCredentialGroupEnrollmentAccessForSubject(
133125
context.credentialGroupId,
@@ -174,7 +166,11 @@ export async function requireCredentialGroupCredentialAccess(
174166
},
175167
resourcePolicy: ResourcePolicyBindingFor<'credential_group'>
176168
): Promise<void> {
177-
if (principal.kind === 'delegated' && principal.serviceId === 'copilot') {
169+
const managedMcp = context.credentialType.startsWith('mcp:')
170+
const actorAccess =
171+
(principal.kind === 'session' && managedMcp) ||
172+
(principal.kind === 'delegated' && principal.serviceId === 'copilot')
173+
if (actorAccess) {
178174
const subject = resolvePrincipalSubject(principal)
179175
if (subject?.kind !== 'sim_user' || !subject.userId) {
180176
throw new OrchestrationError('forbidden', 'Credential Group actor access required')
@@ -192,6 +188,9 @@ export async function requireCredentialGroupCredentialAccess(
192188
if (binding && !isManagedCredentialGroupBindingLive(binding)) {
193189
throw new OrchestrationError('forbidden', 'Credential Group credential access denied')
194190
}
191+
if (actorAccess && !managedMcp && !binding) {
192+
throw new OrchestrationError('forbidden', 'Credential Group credential access denied')
193+
}
195194
if (context.organizationId) {
196195
if (!isOrganizationCredentialType(context.credentialType))
197196
throw new Error('Organization credential access requires a canonical credential type')
@@ -200,8 +199,8 @@ export async function requireCredentialGroupCredentialAccess(
200199
context.credentialType
201200
)
202201
}
203-
if (principal.kind === 'delegated' && principal.serviceId === 'copilot') {
204-
return requireCredentialGroupActorCredentialAccess(principal, context, binding, resourcePolicy)
202+
if (actorAccess) {
203+
return requireCredentialGroupActorCredentialAccess(principal, context, resourcePolicy)
205204
}
206205
if (!context.organizationId) {
207206
throw new OrchestrationError(

‎apps/sim/lib/credentials/application/discover-managed-mcp-tools.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { AuditAction, AuditResourceType } from '@sim/audit'
2-
import { resolvePrincipalSubject } from '@sim/auth/principal'
2+
import { resolvePrincipalSubject, resolvePrincipalSubjectUserId } from '@sim/auth/principal'
33
import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
44
import { OrchestrationError } from '@/lib/core/orchestration/types'
55
import { requireCredentialGroupCredentialAccess } from '@/lib/credential-groups/application/authorization'
@@ -16,12 +16,14 @@ import { snapshotMcpTool } from '@/lib/mcp/presentation-metadata'
1616
import { mcpService } from '@/lib/mcp/service'
1717
import { compileMcpToolSchema } from '@/lib/mcp/tool-schema'
1818
import { assertWorkspaceCapability } from '@/lib/permission-groups/capability-assertions'
19+
import type { ResolvedSecretTraceProvenanceV1 } from '@/executor/utils/resolved-secret-trace-registry'
1920

2021
export interface DiscoverManagedMcpToolsInput {
2122
workspaceId: string
2223
credentialId: string
2324
assertedServerId?: string
2425
signal?: AbortSignal
26+
onResolvedSecretTraceProvenance?: (provenance: ResolvedSecretTraceProvenanceV1) => void
2527
}
2628

2729
export const discoverManagedMcpToolsUseCase = defineAuthorizedWorkspaceUseCase({
@@ -50,6 +52,7 @@ export const discoverManagedMcpToolsUseCase = defineAuthorizedWorkspaceUseCase({
5052
},
5153
async execute({ principal, input, context }) {
5254
input.signal?.throwIfAborted()
55+
const userId = resolvePrincipalSubjectUserId(principal)
5356
const runtime = await loadManagedMcpRuntimeCredential(context.credentialId, context.workspaceId)
5457
if (
5558
runtime.mcpServerId !== context.mcpServerId ||
@@ -70,7 +73,11 @@ export const discoverManagedMcpToolsUseCase = defineAuthorizedWorkspaceUseCase({
7073
loadProvider: () => loadManagedMcpAuthProvider(runtime.credentialId, runtime.workspaceId),
7174
},
7275
input.signal,
73-
{ requireComplete: true }
76+
{
77+
requireComplete: true,
78+
provenanceScope: userId ? { userId, workspaceId: context.workspaceId } : undefined,
79+
onResolvedSecretTraceProvenance: input.onResolvedSecretTraceProvenance,
80+
}
7481
)
7582
await saveManagedMcpToolSnapshot(
7683
runtime.credentialId,

‎apps/sim/lib/internal/mcp/execute-tool.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,10 @@ export const executeMcpTool: InternalToolOperationHandler = async (request) => {
206206
getRemainingExecutionMs(request.signal)
207207
)
208208
const commonInput = {
209+
onResolvedSecretTraceProvenance: provenance
210+
? (value: Parameters<ResolvedSecretTraceProvenanceAccumulator['record']>[0]) =>
211+
provenance?.record(value)
212+
: undefined,
209213
workspaceId: request.context.workspaceId,
210214
toolName,
211215
arguments: args,
@@ -222,9 +226,6 @@ export const executeMcpTool: InternalToolOperationHandler = async (request) => {
222226
const input: ExecuteMcpToolInput = {
223227
...commonInput,
224228
serverId: target.serverId,
225-
onResolvedSecretTraceProvenance: provenance
226-
? (value) => provenance?.record(value)
227-
: undefined,
228229
}
229230
result = await executeMcpToolUseCase.execute({ principal, input })
230231
} else {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js'
1+
import { CallToolResultSchema, ReadResourceResultSchema } from '@modelcontextprotocol/sdk/types.js'
22
import { createLogger } from '@sim/logger'
33
import { isPlainRecord } from '@sim/utils/object'
44
import {
@@ -51,6 +51,8 @@ export async function presentMcpToolResult(
5151
const value = {
5252
arguments: presentation.arguments,
5353
result: projectMcpEncodedContents(result, registry),
54+
resources: projectMcpEncodedContents({ contents: presentation.resources ?? [] }, registry)
55+
.contents,
5456
title: tool.title || tool.name,
5557
}
5658
const projection = projectResolvedSecretModelJsonContent(
@@ -72,6 +74,7 @@ export async function presentMcpToolResult(
7274
tool: { ...tool, title: projection.value.title },
7375
arguments: projection.value.arguments,
7476
result: CallToolResultSchema.parse(projection.value.result),
77+
resources: ReadResourceResultSchema.shape.contents.parse(projection.value.resources),
7578
signal,
7679
secretProvenance: bindDurableSecretProvenanceToValue(
7780
durableSecretProvenanceFromRegistry(

‎apps/sim/lib/mcp/application/execute-managed-tool.ts‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { AuditAction, AuditResourceType } from '@sim/audit'
2-
import { resolvePrincipalSubject } from '@sim/auth/principal'
2+
import { resolvePrincipalSubject, resolvePrincipalSubjectUserId } from '@sim/auth/principal'
33
import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
44
import { OrchestrationError } from '@/lib/core/orchestration/types'
55
import { requireCredentialGroupCredentialAccess } from '@/lib/credential-groups/application/authorization'
@@ -22,6 +22,7 @@ import {
2222
loadMcpOperationAccess,
2323
requireMcpOperationAccess,
2424
} from '@/lib/mcp/application/operation-access'
25+
import { createMcpToolPresentation } from '@/lib/mcp/application/presentation'
2526
import {
2627
isMcpToolVisible,
2728
matchesMcpAppOrigin,
@@ -30,6 +31,7 @@ import {
3031
import { mcpService } from '@/lib/mcp/service'
3132
import type { McpTool, McpToolCall, McpToolSchema } from '@/lib/mcp/types'
3233
import { assertWorkspaceCapability } from '@/lib/permission-groups/capability-assertions'
34+
import type { ResolvedSecretTraceProvenanceV1 } from '@/executor/utils/resolved-secret-trace-registry'
3335

3436
export interface ExecuteManagedMcpToolInput {
3537
workspaceId: string
@@ -42,6 +44,7 @@ export interface ExecuteManagedMcpToolInput {
4244
signal?: AbortSignal
4345
includePresentation?: boolean
4446
appOrigin?: { toolName: string; resourceUri: string }
47+
onResolvedSecretTraceProvenance?: (provenance: ResolvedSecretTraceProvenanceV1) => void
4548
}
4649

4750
function requireToolSchema(value: unknown): McpToolSchema {
@@ -78,6 +81,7 @@ export const executeManagedMcpToolUseCase = defineAuthorizedWorkspaceUseCase({
7881
},
7982
async execute({ principal, input, context }): Promise<ExecuteMcpToolResult> {
8083
input.signal?.throwIfAborted()
84+
const userId = resolvePrincipalSubjectUserId(principal)
8185
const runtime = await loadManagedMcpRuntimeCredential(context.credentialId, context.workspaceId)
8286
if (
8387
runtime.mcpServerId !== context.mcpServerId ||
@@ -103,7 +107,11 @@ export const executeManagedMcpToolUseCase = defineAuthorizedWorkspaceUseCase({
103107
loadProvider: () => loadManagedMcpAuthProvider(runtime.credentialId, runtime.workspaceId),
104108
},
105109
input.signal,
106-
{ requireComplete: true }
110+
{
111+
requireComplete: true,
112+
provenanceScope: userId ? { userId, workspaceId: context.workspaceId } : undefined,
113+
onResolvedSecretTraceProvenance: input.onResolvedSecretTraceProvenance,
114+
}
107115
)
108116
await saveManagedMcpToolSnapshot(
109117
runtime.credentialId,
@@ -160,6 +168,9 @@ export const executeManagedMcpToolUseCase = defineAuthorizedWorkspaceUseCase({
160168
credentialOperations.useManagedMcp.resourcePolicy
161169
)
162170
const providerResult = await mcpService.executeManagedMcpTool({
171+
userId,
172+
workspaceId: context.workspaceId,
173+
onResolvedSecretTraceProvenance: input.onResolvedSecretTraceProvenance,
163174
connectionId: runtime.credentialId,
164175
serverId: runtime.mcpServerId,
165176
scope: runtime.scope,
@@ -171,8 +182,26 @@ export const executeManagedMcpToolUseCase = defineAuthorizedWorkspaceUseCase({
171182
})
172183
input.signal?.throwIfAborted()
173184
const result = transformToolResult(providerResult)
174-
if (input.includePresentation)
175-
result.presentation = { tool, result: providerResult, arguments: args }
185+
if (input.includePresentation && userId)
186+
result.presentation = await createMcpToolPresentation(
187+
{ tool, result: providerResult, arguments: args },
188+
(uri, signal) =>
189+
mcpService.readResource({
190+
serverId: current.mcpServerId,
191+
workspaceId: context.workspaceId,
192+
userId,
193+
uri,
194+
signal,
195+
onResolvedSecretTraceProvenance: input.onResolvedSecretTraceProvenance,
196+
managed: {
197+
connectionId: context.credentialId,
198+
scope: current.scope,
199+
loadAuthProvider: () =>
200+
loadManagedMcpAuthProvider(context.credentialId, context.workspaceId),
201+
},
202+
}),
203+
input.signal
204+
)
176205
return result
177206
},
178207
projectAudit: ({ input, context }) => ({

0 commit comments

Comments
 (0)