Skip to content

Commit b788211

Browse files
committed
Fix shared agent tool execution and discovery boundaries
1 parent a03e580 commit b788211

23 files changed

Lines changed: 1039 additions & 39 deletions

File tree

‎apps/sim/blocks/blocks/file.test.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { describe, expect, it } from 'vitest'
2+
import { fileManageWriteBodySchema } from '@/lib/api/contracts/tools/file'
23
import { FileV4Block, FileV5Block } from '@/blocks/blocks/file'
4+
import { fileWriteTool } from '@/tools/file/write'
35

46
describe('FileV4Block', () => {
57
const buildParams = FileV4Block.tools.config.params
@@ -54,6 +56,72 @@ describe('FileV4Block', () => {
5456
describe('FileV5Block', () => {
5557
const buildParams = FileV5Block.tools.config.params
5658

59+
it.each([
60+
[null, null],
61+
['', null],
62+
[undefined, undefined],
63+
['', 'image/png'],
64+
])(
65+
'writes a produced file with blank content %s and MIME override %s',
66+
(content, contentType) => {
67+
const file = {
68+
id: 'file_execution-output',
69+
name: 'test-card-160x90.png',
70+
url: '/api/files/serve/execution-output.png',
71+
key: 'execution/workspace-1/workflow-1/run-1/output.png',
72+
context: 'execution',
73+
type: 'image/png',
74+
size: 514,
75+
}
76+
const inputs = {
77+
operation: 'file_write',
78+
fileName: 'test-card-160x90.png',
79+
writeFolderRef: '/Media%20Studio%20Tests',
80+
writeFileInput: file,
81+
content,
82+
contentType,
83+
overwrite: false,
84+
_context: { workspaceId: 'workspace-1' },
85+
}
86+
// GenericBlockHandler retains raw inputs when applying a block's transform.
87+
const params = { ...inputs, ...buildParams(inputs) }
88+
const parsed = fileManageWriteBodySchema.parse(fileWriteTool.operation.input(params))
89+
expect(parsed.fileInput).toEqual(file)
90+
expect(parsed.content).toBeUndefined()
91+
expect(parsed.contentType).toBe(contentType ?? undefined)
92+
expect(parsed.folderPath).toBe('/Media%20Studio%20Tests')
93+
}
94+
)
95+
96+
it('preserves empty text writes and rejects a file combined with nonempty text', () => {
97+
const text = { operation: 'file_write', fileName: 'empty.txt', content: '', contentType: null }
98+
const parsed = fileManageWriteBodySchema.parse(
99+
fileWriteTool.operation.input({ ...text, ...buildParams(text) })
100+
)
101+
expect(parsed.content).toBe('')
102+
expect(parsed.fileInput).toBeUndefined()
103+
const both = { ...text, content: 'conflict', writeFileInput: { id: 'file-output' } }
104+
expect(
105+
fileManageWriteBodySchema.safeParse(
106+
fileWriteTool.operation.input({ ...both, ...buildParams(both) })
107+
).success
108+
).toBe(false)
109+
})
110+
111+
it('rejects invalid non-string MIME values at the existing contract', () => {
112+
const inputs = {
113+
operation: 'file_write',
114+
fileName: 'note.txt',
115+
content: 'keep this text',
116+
contentType: 42,
117+
}
118+
expect(
119+
fileManageWriteBodySchema.safeParse(
120+
fileWriteTool.operation.input({ ...inputs, ...buildParams(inputs) })
121+
).success
122+
).toBe(false)
123+
})
124+
57125
it('maps each operation directly to its tool', () => {
58126
expect(FileV5Block.tools.config.tool({ operation: 'file_read' })).toBe('file_read')
59127
expect(FileV5Block.tools.config.tool({ operation: 'file_get_content' })).toBe(

‎apps/sim/blocks/blocks/file.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1971,7 +1971,7 @@ export const FileV5Block: BlockConfig<FileParserV3Output> = {
19711971
folderPath: optionalText(params.writeFolderRef),
19721972
content: omitContent ? undefined : params.content,
19731973
...(fileInput ? { fileInput } : {}),
1974-
contentType: params.contentType,
1974+
contentType: params.contentType ?? undefined,
19751975
overwrite: params.overwrite === true || params.overwrite === 'true',
19761976
workspaceId: params._context?.workspaceId,
19771977
}

‎apps/sim/blocks/blocks/mothership.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ interface MothershipResponse extends ToolResponse {
1919
export const MothershipBlock: BlockConfig<MothershipResponse> = {
2020
type: 'mothership',
2121
name: 'Sim Chat',
22-
description: 'Talk to Sim',
22+
description: 'Run a prompt with integration and MCP tools',
2323
longDescription:
24-
'The Sim Chat block sends a prompt with selected integration tools, files, and skill context for a one-shot response within a workflow.',
24+
'Run a one-shot prompt with available workspace integration operations, selected MCP tools, and skill context. Tool access is limited to those operations.',
2525
bestPractices: `
2626
- Use for tasks that require multi-step reasoning, tool use, or cross-service coordination.
2727
- Choose Astra or Opus and a reasoning effort. Astra supports Fast mode.

‎apps/sim/lib/internal/file/execute-tool.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import type { PersonalApiKeyPrincipal } from '@sim/auth/principal'
45
import { createExecutionContext } from '@sim/testing'
56
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
import { InvalidInternalDelegationBindingError } from '@/lib/auth/internal-delegation'
68
import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution'
79

810
const mocks = vi.hoisted(() => ({
@@ -424,6 +426,69 @@ describe('executeFileTool', () => {
424426
)
425427
})
426428

429+
it.each([undefined, 'user-1'])(
430+
'does not derive authority from body fields or userId (%s)',
431+
async (userId) => {
432+
const response = await executeFileTool(
433+
request(
434+
'file_read',
435+
{
436+
operation: 'read',
437+
fileId: 'file-1',
438+
callerPrincipal: { kind: 'session', userId: 'user-1', sessionId: 'forged' },
439+
},
440+
{
441+
context: { workflowId: '', workspaceId: 'workspace-1', userId },
442+
}
443+
)
444+
)
445+
expect(response.status).toBe(401)
446+
expect(mocks.createPrincipal).not.toHaveBeenCalled()
447+
expect(mocks.executeManage).not.toHaveBeenCalled()
448+
}
449+
)
450+
451+
it('keeps executor delegation authoritative when a direct caller is also present', async () => {
452+
const callerPrincipal: PersonalApiKeyPrincipal = {
453+
kind: 'personal_api_key',
454+
userId: 'user-1',
455+
keyId: 'key-1',
456+
}
457+
const call = request('file_read', MANAGE_INPUTS.file_read)
458+
call.context.callerPrincipal = callerPrincipal
459+
const response = await executeFileTool(call)
460+
expect(response.status).toBe(200)
461+
expect(mocks.createPrincipal).toHaveBeenCalled()
462+
expect(mocks.executeManage.mock.calls[0]?.[1].principal).toMatchObject({
463+
kind: 'delegated',
464+
serviceId: 'executor',
465+
})
466+
expect(mocks.executeManage.mock.calls[0]?.[1].principal).not.toBe(callerPrincipal)
467+
})
468+
469+
it('never falls back to a direct caller after invalid executor delegation', async () => {
470+
const call = request('file_read', MANAGE_INPUTS.file_read)
471+
call.context.callerPrincipal = { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-1' }
472+
mocks.createPrincipal.mockRejectedValueOnce(new InvalidInternalDelegationBindingError())
473+
const response = await executeFileTool(call)
474+
expect(response.status).toBe(401)
475+
expect(mocks.createPrincipal).toHaveBeenCalled()
476+
expect(mocks.executeManage).not.toHaveBeenCalled()
477+
})
478+
479+
it('rejects a direct caller without trusted workspace scope', async () => {
480+
const response = await executeFileTool(
481+
request('file_read', MANAGE_INPUTS.file_read, {
482+
context: {
483+
workflowId: '',
484+
callerPrincipal: { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-1' },
485+
},
486+
})
487+
)
488+
expect(response.status).toBe(401)
489+
expect(mocks.executeManage).not.toHaveBeenCalled()
490+
})
491+
427492
it('rejects missing trusted identity during principal construction', async () => {
428493
const response = await executeFileTool(
429494
request('file_get', MANAGE_INPUTS.file_get, {

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,10 @@ export const executeFileTool: InternalToolOperationHandler = async (request) =>
8989
}
9090

9191
const workspaceId = request.context.workspaceId
92-
if (!workspaceId || !request.context.executorDelegationOrigin) {
92+
if (
93+
!workspaceId ||
94+
(!request.context.executorDelegationOrigin && !request.context.callerPrincipal)
95+
) {
9396
return Response.json({ success: false, error: 'Authentication required' }, { status: 401 })
9497
}
9598

@@ -112,10 +115,13 @@ export const executeFileTool: InternalToolOperationHandler = async (request) =>
112115
isParserTool || isSearchTool ? null : parseInternalToolInput(fileManageContract, request.input)
113116
if (manageInput && !manageInput.success) return manageInput.response
114117
try {
115-
const principal = await createExecutorPrincipalFromExecutionContext({
116-
context: request.context,
117-
audience: WORKSPACE_FILES_DELEGATION_AUDIENCE,
118-
})
118+
const principal =
119+
request.context.callerPrincipal && !request.context.executorDelegationOrigin
120+
? request.context.callerPrincipal
121+
: await createExecutorPrincipalFromExecutionContext({
122+
context: request.context,
123+
audience: WORKSPACE_FILES_DELEGATION_AUDIENCE,
124+
})
119125
if (searchInput) {
120126
request.signal?.throwIfAborted()
121127
const result = await searchWorkspaceFileContent.execute({

0 commit comments

Comments
 (0)