Skip to content

Commit 8747bef

Browse files
committed
Fix shared agent tool execution and discovery boundaries
1 parent 4c1737b commit 8747bef

24 files changed

Lines changed: 1040 additions & 40 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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1973,9 +1973,9 @@ export const FileV5Block: BlockConfig<FileParserV3Output> = {
19731973
return {
19741974
fileName: params.fileName,
19751975
folderPath: optionalText(params.writeFolderRef),
1976-
...(omitContent ? {} : { content: params.content }),
1976+
content: omitContent ? undefined : params.content,
19771977
...(fileInput ? { fileInput } : {}),
1978-
contentType: params.contentType,
1978+
contentType: params.contentType ?? undefined,
19791979
overwrite: params.overwrite === true || params.overwrite === 'true',
19801980
workspaceId: params._context?.workspaceId,
19811981
}

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(() => ({
@@ -420,6 +422,69 @@ describe('executeFileTool', () => {
420422
)
421423
})
422424

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