Skip to content

fix(mcp): stop refusing tool results authored by another workspace member #6610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
waleedlatif1 merged 1 commit into from
Aug 12, 2026
+244 −2
Merged

fix(mcp): stop refusing tool results authored by another workspace member #6610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
233 changes: 233 additions & 0 deletions apps/sim/app/api/mcp/serve/[serverId]/route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/
import {
dbChainMockFns,
encryptionMock,
encryptionMockFns,
hybridAuthMockFns,
permissionsMock,
permissionsMockFns,
Expand All @@ -31,6 +33,8 @@ const {
fetchMock: vi.fn(),
}))

vi.mock('@/lib/core/security/encryption', () => encryptionMock)

vi.mock('@/lib/billing/core/billing-attribution', () => ({
BILLING_ATTRIBUTION_HEADER: 'x-sim-billing-attribution',
assertBillingAttributionSnapshot: mockAssertBillingAttributionSnapshot,
Expand Down Expand Up @@ -99,6 +103,9 @@ describe('MCP Serve Route', () => {
)
mockAssertBillingAttributionSnapshot.mockImplementation((value: unknown) => value)
mockSerializeBillingAttributionHeader.mockReturnValue('serialized-attribution')
encryptionMockFns.mockDecryptSecret.mockImplementation(async (encryptedValue: string) => ({
decrypted: `decrypted:${encryptedValue}`,
}))
})

afterEach(() => {
Expand Down Expand Up @@ -1121,6 +1128,232 @@ describe('MCP Serve Route', () => {
expect(JSON.parse(body.result.content[0].text)).toEqual(['a', 'b'])
})

it('serves a public tool whose workflow was authored by someone other than the actor', async () => {
dbChainMockFns.limit
.mockResolvedValueOnce([
{
id: 'server-1',
name: 'Public Server',
workspaceId: 'ws-1',
isPublic: true,
createdBy: 'owner-1',
},
])
.mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }])
.mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }])
mockExecuteWorkflowService.mockResolvedValueOnce({
ok: true,
executionId: 'exec-1',
workflowId: 'wf-1',
status: 'completed',
aborted: null,
output: { ok: true },
error: null,
hasResponseBlock: false,
resolvedSecretTraceProvenance: createResolvedSecretTraceProvenance('author-2'),
})

const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', {
method: 'POST',
body: JSON.stringify({
jsonrpc: '2.0',
id: 1,
method: 'tools/call',
params: { name: 'tool_a', arguments: { q: 'test' } },
}),
})
const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) })
const body = await response.json()

expect(response.status).toBe(200)
expect(JSON.parse(body.result.content[0].text)).toEqual({ ok: true })
})

it('serves a private tool whose workflow was authored by someone other than the caller', async () => {
dbChainMockFns.limit
.mockResolvedValueOnce([
{
id: 'server-1',
name: 'Private Server',
workspaceId: 'ws-1',
isPublic: false,
createdBy: 'owner-1',
},
])
.mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }])
.mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }])
/**
* `vi.clearAllMocks()` does not drain `mockResolvedValueOnce` queues, and a public-server
* test never reaches `checkHybridAuth`, so an earlier queued auth would be consumed here.
*/
hybridAuthMockFns.mockCheckHybridAuth.mockReset()
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
success: true,
userId: 'user-1',
authType: 'api_key',
apiKeyType: 'workspace',
workspaceId: 'ws-1',
})
mockGetUserEntityPermissions.mockResolvedValueOnce('write')
mockExecuteWorkflowService.mockResolvedValueOnce({
ok: true,
executionId: 'exec-1',
workflowId: 'wf-1',
status: 'completed',
aborted: null,
output: { ok: true },
error: null,
hasResponseBlock: false,
resolvedSecretTraceProvenance: createResolvedSecretTraceProvenance('author-2'),
})

const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', {
method: 'POST',
headers: { 'X-API-Key': 'wsk_test_123' },
body: JSON.stringify({
jsonrpc: '2.0',
id: 1,
method: 'tools/call',
params: { name: 'tool_a', arguments: { q: 'test' } },
}),
})
const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) })
const body = await response.json()

expect(response.status).toBe(200)
expect(JSON.parse(body.result.content[0].text)).toEqual({ ok: true })
})

it('anonymizes an author-scoped secret label when the actor is not the author', async () => {
dbChainMockFns.limit
.mockResolvedValueOnce([
{
id: 'server-1',
name: 'Public Server',
workspaceId: 'ws-1',
isPublic: true,
createdBy: 'owner-1',
},
])
.mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }])
.mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }])
mockExecuteWorkflowService.mockResolvedValueOnce({
ok: true,
executionId: 'exec-1',
workflowId: 'wf-1',
status: 'completed',
aborted: null,
output: { leaked: 'decrypted:author-ciphertext' },
error: null,
hasResponseBlock: false,
resolvedSecretTraceProvenance: {
...createResolvedSecretTraceProvenance('author-2'),
entries: [{ name: 'AUTHOR_TOKEN', encryptedValue: 'author-ciphertext' }],
},
})

const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', {
method: 'POST',
body: JSON.stringify({
jsonrpc: '2.0',
id: 1,
method: 'tools/call',
params: { name: 'tool_a', arguments: { q: 'test' } },
}),
})
const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) })
const body = await response.json()

expect(response.status).toBe(200)
expect(JSON.parse(body.result.content[0].text)).toEqual({ leaked: '[REDACTED_SECRET]' })
expect(body.result.content[0].text).not.toContain('AUTHOR_TOKEN')
})

it('keeps the author-scoped secret label when the actor is the author', async () => {
dbChainMockFns.limit
.mockResolvedValueOnce([
{
id: 'server-1',
name: 'Public Server',
workspaceId: 'ws-1',
isPublic: true,
createdBy: 'owner-1',
},
])
.mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }])
.mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }])
mockExecuteWorkflowService.mockResolvedValueOnce({
ok: true,
executionId: 'exec-1',
workflowId: 'wf-1',
status: 'completed',
aborted: null,
output: { leaked: 'decrypted:owner-ciphertext' },
error: null,
hasResponseBlock: false,
resolvedSecretTraceProvenance: {
...createResolvedSecretTraceProvenance('owner-1'),
entries: [{ name: 'OWNER_TOKEN', encryptedValue: 'owner-ciphertext' }],
},
})

const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', {
method: 'POST',
body: JSON.stringify({
jsonrpc: '2.0',
id: 1,
method: 'tools/call',
params: { name: 'tool_a', arguments: { q: 'test' } },
}),
})
const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) })
const body = await response.json()

expect(response.status).toBe(200)
expect(JSON.parse(body.result.content[0].text)).toEqual({ leaked: '{{OWNER_TOKEN}}' })
})

it('refuses a tool result whose provenance was stamped for another workspace', async () => {
dbChainMockFns.limit
.mockResolvedValueOnce([
{
id: 'server-1',
name: 'Public Server',
workspaceId: 'ws-1',
isPublic: true,
createdBy: 'owner-1',
},
])
.mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }])
.mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }])
mockExecuteWorkflowService.mockResolvedValueOnce({
ok: true,
executionId: 'exec-1',
workflowId: 'wf-1',
status: 'completed',
aborted: null,
output: { ok: true },
error: null,
hasResponseBlock: false,
resolvedSecretTraceProvenance: createResolvedSecretTraceProvenance('owner-1', 'ws-other'),
})

const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', {
method: 'POST',
body: JSON.stringify({
jsonrpc: '2.0',
id: 1,
method: 'tools/call',
params: { name: 'tool_a', arguments: { q: 'test' } },
}),
})
const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) })
const body = await response.json()

expect(response.status).toBe(500)
expect(body.error.code).toBe(-32603)
})

it('rejects duplicate tool names instead of choosing an arbitrary workflow', async () => {
dbChainMockFns.limit
.mockResolvedValueOnce([
Expand Down
13 changes: 11 additions & 2 deletions apps/sim/app/api/mcp/serve/[serverId]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,20 @@ async function projectWorkflowToolOutput(
provenance: unknown,
scope: { userId: string; workspaceId: string }
): Promise<unknown> {
/**
* Deliberately compares the tenant only, never the user. The executor stamps provenance with
* the workflow AUTHOR (`personalEnvUserId` falls back to `metadata.workflowUserId` on this
* non-session path) while `scope` here is the ACTING caller, and a caller who did not author
* the workflow is the ordinary team configuration — demanding they match refuses every such
* call after the workflow has already run and been billed. Restoring a user comparison here
* is not hardening: the registry compares both scope fields itself and marks every entry
* imported from another user anonymous, so a non-author already sees the opaque redaction
* placeholder rather than the author's secret names.
*/
if (
!isResolvedSecretTraceProvenanceV1(provenance) ||
!provenance.complete ||
provenance.scope?.userId !== scope.userId ||
provenance.scope.workspaceId !== scope.workspaceId
provenance.scope?.workspaceId !== scope.workspaceId
) {
throw new Error('MCP workflow execution provenance is unavailable')
}
Expand Down
Loading