diff --git a/apps/api/src/handlers/mcp/__tests__/custom-mcp.test.ts b/apps/api/src/handlers/mcp/__tests__/custom-mcp.test.ts index b3be60e353..f4ce190a37 100644 --- a/apps/api/src/handlers/mcp/__tests__/custom-mcp.test.ts +++ b/apps/api/src/handlers/mcp/__tests__/custom-mcp.test.ts @@ -23,7 +23,13 @@ const { mockFindCustomServer: vi.fn(), mockFindConnection: vi.fn(), mockGetValidAccessToken: vi.fn(), - mockResolveApprovalBlocks: vi.fn(async () => new Map()), + mockResolveApprovalBlocks: vi.fn( + async (): Promise<{ + blocks: Map; + defaultBlock?: 'needs_approval'; + shadowDefaultTools: boolean; + }> => ({ blocks: new Map(), shadowDefaultTools: false }), + ), mockClaimTaskToolCall: vi.fn(async () => false), })); @@ -463,7 +469,10 @@ describe('createCustomMcpProxy', () => { describe('tool approval policies', () => { afterEach(() => { - mockResolveApprovalBlocks.mockImplementation(async () => new Map()); + mockResolveApprovalBlocks.mockImplementation(async () => ({ + blocks: new Map(), + shadowDefaultTools: false, + })); mockClaimTaskToolCall.mockReset().mockResolvedValue(false); }); @@ -498,9 +507,10 @@ describe('createCustomMcpProxy', () => { mockFindCustomServer.mockResolvedValue( buildServerRow({ url: upstreamUrl() }), ); - mockResolveApprovalBlocks.mockResolvedValue( - new Map([['dangerous_tool', 'needs_approval']]), - ); + mockResolveApprovalBlocks.mockResolvedValue({ + blocks: new Map([['dangerous_tool', 'needs_approval']]), + shadowDefaultTools: false, + }); const response = await postMcp(createApp(), { jsonrpc: '2.0', @@ -519,9 +529,10 @@ describe('createCustomMcpProxy', () => { mockFindCustomServer.mockResolvedValue( buildServerRow({ url: upstreamUrl() }), ); - mockResolveApprovalBlocks.mockResolvedValue( - new Map([['dangerous_tool', 'needs_approval']]), - ); + mockResolveApprovalBlocks.mockResolvedValue({ + blocks: new Map([['dangerous_tool', 'needs_approval']]), + shadowDefaultTools: false, + }); mockClaimTaskToolCall.mockResolvedValue(true); const response = await postMcp(createApp(), { @@ -546,9 +557,10 @@ describe('createCustomMcpProxy', () => { mockFindCustomServer.mockResolvedValue( buildServerRow({ url: upstreamUrl() }), ); - mockResolveApprovalBlocks.mockResolvedValue( - new Map([['dangerous_tool', 'needs_approval']]), - ); + mockResolveApprovalBlocks.mockResolvedValue({ + blocks: new Map([['dangerous_tool', 'needs_approval']]), + shadowDefaultTools: false, + }); const list = await postMcp(createApp(), { jsonrpc: '2.0', @@ -576,13 +588,43 @@ describe('createCustomMcpProxy', () => { expect(mockClaimTaskToolCall).not.toHaveBeenCalled(); }); - it('hides blocked tools from tools/list', async () => { + it('gates every default tool of a task while Auto mode is on', async () => { mockFindCustomServer.mockResolvedValue( buildServerRow({ url: upstreamUrl() }), ); - mockResolveApprovalBlocks.mockResolvedValue( - new Map([['dangerous_tool', 'reject']]), + mockResolveApprovalBlocks.mockResolvedValue({ + blocks: new Map([['safe_tool', 'allow']]), + defaultBlock: 'needs_approval', + shadowDefaultTools: false, + }); + mockClaimTaskToolCall.mockResolvedValue(false); + + const gated = await postMcp(createApp(), { + jsonrpc: '2.0', + id: 5, + method: 'tools/call', + params: { name: 'dangerous_tool', arguments: {} }, + }); + expect(gated.status).toBe(403); + + // A tool someone chose to always allow needs no approval. + const allowed = await postMcp(createApp(), { + jsonrpc: '2.0', + id: 6, + method: 'tools/call', + params: { name: 'safe_tool', arguments: {} }, + }); + expect(allowed.status).toBe(200); + }); + + it('hides blocked tools from tools/list', async () => { + mockFindCustomServer.mockResolvedValue( + buildServerRow({ url: upstreamUrl() }), ); + mockResolveApprovalBlocks.mockResolvedValue({ + blocks: new Map([['dangerous_tool', 'reject']]), + shadowDefaultTools: false, + }); const response = await postMcp(createApp(), { jsonrpc: '2.0', diff --git a/apps/api/src/handlers/mcp/__tests__/integration-mcp.test.ts b/apps/api/src/handlers/mcp/__tests__/integration-mcp.test.ts index 062eb3eb8d..51152a68f9 100644 --- a/apps/api/src/handlers/mcp/__tests__/integration-mcp.test.ts +++ b/apps/api/src/handlers/mcp/__tests__/integration-mcp.test.ts @@ -13,7 +13,10 @@ const { mockGetTaskHumanOwnerUserIds, mockResolveApprovalBlocks, } = vi.hoisted(() => ({ - mockResolveApprovalBlocks: vi.fn(async () => new Map()), + mockResolveApprovalBlocks: vi.fn(async () => ({ + blocks: new Map(), + shadowDefaultTools: false, + })), mockFindTaskRun: vi.fn(), mockFindConnection: vi.fn(), mockFindEnablement: vi.fn(), @@ -25,6 +28,11 @@ const { vi.mock('../tool-approval-enforcement', () => ({ describeProxyToolApprovalBlock: () => '', resolveProxyToolApprovalBlocks: mockResolveApprovalBlocks, + resolveProxyToolApprovalBlock: ( + approvals: { blocks: Map; defaultBlock?: string }, + toolName: string, + ) => approvals.blocks.get(toolName) ?? approvals.defaultBlock, + shadowProxyToolCall: () => undefined, })); vi.mock('@roomote/db/server', () => ({ diff --git a/apps/api/src/handlers/mcp/__tests__/linear.test.ts b/apps/api/src/handlers/mcp/__tests__/linear.test.ts index 711f8bd456..db3282f143 100644 --- a/apps/api/src/handlers/mcp/__tests__/linear.test.ts +++ b/apps/api/src/handlers/mcp/__tests__/linear.test.ts @@ -4,12 +4,20 @@ import type { RunTokenContext } from '@roomote/types'; import type { Variables } from '../../../types'; const { mockResolveApprovalBlocks } = vi.hoisted(() => ({ - mockResolveApprovalBlocks: vi.fn(async () => new Map()), + mockResolveApprovalBlocks: vi.fn(async () => ({ + blocks: new Map(), + shadowDefaultTools: false, + })), })); vi.mock('../tool-approval-enforcement', () => ({ describeProxyToolApprovalBlock: () => 'blocked by policy', resolveProxyToolApprovalBlocks: mockResolveApprovalBlocks, + resolveProxyToolApprovalBlock: ( + approvals: { blocks: Map; defaultBlock?: string }, + toolName: string, + ) => approvals.blocks.get(toolName) ?? approvals.defaultBlock, + shadowProxyToolCall: () => undefined, })); vi.mock('@roomote/db/server', () => ({ @@ -73,9 +81,10 @@ describe('createLinearMcp tool approval policies', () => { it('refuses a policy-blocked tool under the linear id without contacting Linear', async () => { const fetchMock = vi.fn(); vi.stubGlobal('fetch', fetchMock); - mockResolveApprovalBlocks.mockResolvedValue( - new Map([['save_issue', 'needs_approval']]), - ); + mockResolveApprovalBlocks.mockResolvedValue({ + blocks: new Map([['save_issue', 'needs_approval']]), + shadowDefaultTools: false, + }); const response = await post({ jsonrpc: '2.0', diff --git a/apps/api/src/handlers/mcp/__tests__/native-tool-approvals.test.ts b/apps/api/src/handlers/mcp/__tests__/native-tool-approvals.test.ts index 3894bbd9a8..2bd7d1bfc6 100644 --- a/apps/api/src/handlers/mcp/__tests__/native-tool-approvals.test.ts +++ b/apps/api/src/handlers/mcp/__tests__/native-tool-approvals.test.ts @@ -1,13 +1,28 @@ -const { mockResolveBlocks, mockClaim } = vi.hoisted(() => ({ - mockResolveBlocks: vi.fn(async () => new Map()), +type Approvals = { + blocks: Map; + defaultBlock?: 'needs_approval'; + shadowDefaultTools: boolean; +}; + +const { mockResolveBlocks, mockClaim, mockShadow } = vi.hoisted(() => ({ + mockResolveBlocks: vi.fn( + async (): Promise => ({ + blocks: new Map(), + shadowDefaultTools: false, + }), + ), mockClaim: vi.fn(async () => false), + mockShadow: vi.fn(), })); vi.mock('../tool-approval-enforcement', () => ({ claimProxyTaskToolCall: mockClaim, describeProxyToolApprovalBlock: (toolName: string, block: string) => `${toolName}:${block}`, + resolveProxyToolApprovalBlock: (approvals: Approvals, toolName: string) => + approvals.blocks.get(toolName) ?? approvals.defaultBlock, resolveProxyToolApprovalBlocks: mockResolveBlocks, + shadowProxyToolCall: mockShadow, })); vi.mock('../proxy-utils', () => ({ @@ -39,17 +54,21 @@ import { resolveNativeToolApprovalGuard } from '../native-tool-approvals'; describe('resolveNativeToolApprovalGuard', () => { beforeEach(() => { vi.clearAllMocks(); - mockResolveBlocks.mockResolvedValue(new Map()); + mockResolveBlocks.mockResolvedValue({ + blocks: new Map(), + shadowDefaultTools: false, + }); mockClaim.mockResolvedValue(false); }); it('hides disabled tools from native tools/list responses', async () => { - mockResolveBlocks.mockResolvedValue( - new Map([ + mockResolveBlocks.mockResolvedValue({ + blocks: new Map([ ['disabled_tool', 'reject'], ['ask_tool', 'needs_approval'], ]), - ); + shadowDefaultTools: false, + }); const guard = await resolveNativeToolApprovalGuard({ auth: { userId: 'user-1', tokenType: 'auth' }, integrationId: 'notion', @@ -79,12 +98,13 @@ describe('resolveNativeToolApprovalGuard', () => { }); it('refuses disabled calls and only releases ask calls after approval', async () => { - mockResolveBlocks.mockResolvedValue( - new Map([ + mockResolveBlocks.mockResolvedValue({ + blocks: new Map([ ['disabled_tool', 'reject'], ['ask_tool', 'needs_approval'], ]), - ); + shadowDefaultTools: false, + }); const guard = await resolveNativeToolApprovalGuard({ auth: { userId: null, tokenType: 'run', runId: 42 }, integrationId: 'notion', @@ -114,4 +134,47 @@ describe('resolveNativeToolApprovalGuard', () => { }), ).resolves.toBeNull(); }); + + it('holds default tools for a claim while Auto mode is on and offers every call for shadow assessment', async () => { + mockResolveBlocks.mockResolvedValue({ + blocks: new Map(), + defaultBlock: 'needs_approval', + shadowDefaultTools: true, + }); + const guard = await resolveNativeToolApprovalGuard({ + auth: { userId: null, tokenType: 'run', runId: 42 }, + integrationId: 'notion', + }); + + const held = await guard.checkCall({ + id: 3, + method: 'tools/call', + params: { name: 'any_tool', arguments: { q: 1 } }, + }); + expect(held?.status).toBe(403); + expect(mockShadow).toHaveBeenCalledWith( + expect.objectContaining({ defaultBlock: 'needs_approval' }), + expect.objectContaining({ + integrationId: 'notion', + toolName: 'any_tool', + args: { q: 1 }, + taskId: 'task-1', + }), + ); + expect(mockClaim).toHaveBeenCalledWith({ + taskId: 'task-1', + integrationId: 'notion', + toolName: 'any_tool', + args: { q: 1 }, + }); + + mockClaim.mockResolvedValue(true); + await expect( + guard.checkCall({ + id: 3, + method: 'tools/call', + params: { name: 'any_tool', arguments: { q: 1 } }, + }), + ).resolves.toBeNull(); + }); }); diff --git a/apps/api/src/handlers/mcp/__tests__/tool-approval-enforcement.test.ts b/apps/api/src/handlers/mcp/__tests__/tool-approval-enforcement.test.ts index d5838dd2ba..7ab8903d20 100644 --- a/apps/api/src/handlers/mcp/__tests__/tool-approval-enforcement.test.ts +++ b/apps/api/src/handlers/mcp/__tests__/tool-approval-enforcement.test.ts @@ -5,6 +5,8 @@ const { mockSessionForTask, mockOverrides, mockClaim, + mockAutoState, + mockShadow, } = vi.hoisted(() => ({ mockExperiment: vi.fn(async () => true), mockDeployment: vi.fn(async () => [] as unknown[]), @@ -12,6 +14,8 @@ const { mockSessionForTask: vi.fn(async () => null as { id: string } | null), mockOverrides: vi.fn(async () => [] as unknown[]), mockClaim: vi.fn(async () => true), + mockAutoState: vi.fn(async () => ({ mode: 'off' }) as unknown), + mockShadow: vi.fn(), })); vi.mock('@roomote/db/server', () => ({ @@ -24,18 +28,30 @@ vi.mock('@roomote/db/server', () => ({ claimTaskIntegrationToolCall: mockClaim, fingerprintIntegrationToolCall: (input: unknown) => JSON.stringify(input), })); +vi.mock( + '@roomote/cloud-agents/server/integration-tool-auto-evaluation', + () => ({ + resolveIntegrationToolAutoState: mockAutoState, + recordIntegrationToolShadowEvaluationInBackground: mockShadow, + }), +); import { claimProxyTaskToolCall, + resolveProxyToolApprovalBlock, resolveProxyToolApprovalBlocks, + shadowProxyToolCall, } from '../tool-approval-enforcement'; const policy = ( integrationId: string, toolName: string, - mode: 'auto' | 'ask' | 'reject', + mode: 'always_allow' | 'ask' | 'reject', ) => ({ integrationId, toolName, mode }); +const blocksOf = (approvals: { blocks: Map }) => + Object.fromEntries(approvals.blocks); + describe('resolveProxyToolApprovalBlocks', () => { beforeEach(() => { vi.clearAllMocks(); @@ -48,6 +64,7 @@ describe('resolveProxyToolApprovalBlocks', () => { mockUser.mockResolvedValue([]); mockSessionForTask.mockResolvedValue(null); mockOverrides.mockResolvedValue([]); + mockAutoState.mockResolvedValue({ mode: 'off' }); }); it('blocks reject for every caller and ask only for task runs', async () => { @@ -56,10 +73,12 @@ describe('resolveProxyToolApprovalBlocks', () => { tokenType: 'run', resolveActingUserId: async () => 'user-1', }); - expect(Object.fromEntries(task)).toEqual({ + expect(blocksOf(task)).toEqual({ delete_issue: 'reject', save_issue: 'needs_approval', }); + expect(task.defaultBlock).toBeUndefined(); + expect(task.shadowDefaultTools).toBe(false); // A Session already decided its native ask before the call got here. const session = await resolveProxyToolApprovalBlocks({ @@ -67,7 +86,7 @@ describe('resolveProxyToolApprovalBlocks', () => { tokenType: 'auth', resolveActingUserId: async () => 'user-1', }); - expect(Object.fromEntries(session)).toEqual({ delete_issue: 'reject' }); + expect(blocksOf(session)).toEqual({ delete_issue: 'reject' }); }); it("applies the stricter of the deployment and the acting user's policy", async () => { @@ -81,8 +100,7 @@ describe('resolveProxyToolApprovalBlocks', () => { tokenType: 'run', resolveActingUserId: async () => 'user-1', }); - expect(mockUser).toHaveBeenCalledWith('user-1'); - expect(Object.fromEntries(blocks)).toEqual({ + expect(blocksOf(blocks)).toEqual({ delete_issue: 'reject', save_issue: 'reject', list_issues: 'needs_approval', @@ -90,53 +108,53 @@ describe('resolveProxyToolApprovalBlocks', () => { }); it('governs a custom server by its own layer only, since names can coincide', async () => { - mockDeployment.mockResolvedValue([ - policy('tools', 'shared_only', 'reject'), - ]); - mockUser.mockResolvedValue([policy('tools', 'personal_only', 'reject')]); - const resolve = (policyScope?: 'deployment' | 'personal') => - resolveProxyToolApprovalBlocks({ - integrationId: 'tools', - policyScope, - tokenType: 'run', - resolveActingUserId: async () => 'user-1', - }).then((blocks) => [...blocks.keys()].sort()); - - expect(await resolve('deployment')).toEqual(['shared_only']); - expect(await resolve('personal')).toEqual(['personal_only']); - // Built-in integrations take both layers. - expect(await resolve()).toEqual(['personal_only', 'shared_only']); + mockUser.mockResolvedValue([policy('linear', 'list_issues', 'reject')]); + const shared = await resolveProxyToolApprovalBlocks({ + integrationId: 'linear', + policyScope: 'deployment', + tokenType: 'run', + resolveActingUserId: async () => 'user-1', + }); + expect(blocksOf(shared)).toEqual({ + delete_issue: 'reject', + save_issue: 'needs_approval', + }); + expect(mockUser).not.toHaveBeenCalled(); + + const personal = await resolveProxyToolApprovalBlocks({ + integrationId: 'linear', + policyScope: 'personal', + tokenType: 'run', + resolveActingUserId: async () => 'user-1', + }); + expect(blocksOf(personal)).toEqual({ list_issues: 'reject' }); }); it('reads no personal policies for a run without a human actor', async () => { - await resolveProxyToolApprovalBlocks({ + const blocks = await resolveProxyToolApprovalBlocks({ integrationId: 'linear', tokenType: 'run', resolveActingUserId: async () => null, }); expect(mockUser).not.toHaveBeenCalled(); + expect(blocksOf(blocks)).toEqual({ + delete_issue: 'reject', + save_issue: 'needs_approval', + }); }); it('blocks nothing and reads nothing while the experiment is off', async () => { mockExperiment.mockResolvedValue(false); + const resolveActingUserId = vi.fn(async () => 'user-1'); const blocks = await resolveProxyToolApprovalBlocks({ integrationId: 'linear', tokenType: 'run', - resolveActingUserId: async () => 'user-1', + resolveActingUserId, }); - expect(blocks.size).toBe(0); + expect(blocks.blocks.size).toBe(0); + expect(resolveActingUserId).not.toHaveBeenCalled(); expect(mockDeployment).not.toHaveBeenCalled(); - expect(mockUser).not.toHaveBeenCalled(); - }); - - it('holds an auto tool for a task run exactly like an ask tool', async () => { - mockDeployment.mockResolvedValue([policy('linear', 'save_issue', 'auto')]); - const task = await resolveProxyToolApprovalBlocks({ - integrationId: 'linear', - tokenType: 'run', - resolveActingUserId: async () => 'user-1', - }); - expect(Object.fromEntries(task)).toEqual({ save_issue: 'needs_approval' }); + expect(mockAutoState).not.toHaveBeenCalled(); }); it("applies the task's session overrides to a task run only", async () => { @@ -156,7 +174,7 @@ describe('resolveProxyToolApprovalBlocks', () => { resolveActingUserId: async () => 'user-1', resolveTaskId: async () => 'task-1', }); - expect(Object.fromEntries(task)).toEqual({ + expect(blocksOf(task)).toEqual({ delete_issue: 'reject', list_issues: 'needs_approval', }); @@ -169,9 +187,77 @@ describe('resolveProxyToolApprovalBlocks', () => { resolveActingUserId: async () => 'user-1', resolveTaskId: async () => 'task-1', }); - expect(Object.fromEntries(session)).toEqual({ delete_issue: 'reject' }); + expect(blocksOf(session)).toEqual({ delete_issue: 'reject' }); expect(mockOverrides).not.toHaveBeenCalled(); }); + + it("gates every default tool of a task while Auto is on, except a person's choices", async () => { + mockAutoState.mockResolvedValue({ mode: 'on' }); + mockDeployment.mockResolvedValue([ + policy('linear', 'get_issue', 'always_allow'), + policy('linear', 'delete_issue', 'reject'), + ]); + mockSessionForTask.mockResolvedValue({ id: 'session-1' }); + mockOverrides.mockResolvedValue([ + { integrationId: 'linear', toolName: 'list_issues', mode: 'allow' }, + ]); + const task = await resolveProxyToolApprovalBlocks({ + integrationId: 'linear', + tokenType: 'run', + resolveActingUserId: async () => 'user-1', + resolveTaskId: async () => 'task-1', + }); + expect(task.defaultBlock).toBe('needs_approval'); + expect(blocksOf(task)).toEqual({ + get_issue: 'allow', + delete_issue: 'reject', + list_issues: 'allow', + }); + expect(resolveProxyToolApprovalBlock(task, 'save_issue')).toBe( + 'needs_approval', + ); + expect(resolveProxyToolApprovalBlock(task, 'get_issue')).toBe('allow'); + + // A Session decided its own native asks; nothing extra at the proxy. + const session = await resolveProxyToolApprovalBlocks({ + integrationId: 'linear', + tokenType: 'auth', + resolveActingUserId: async () => 'user-1', + }); + expect(session.defaultBlock).toBeUndefined(); + }); + + it('shadow-assesses default tool calls only while shadowing', async () => { + mockAutoState.mockResolvedValue({ mode: 'shadow' }); + const approvals = await resolveProxyToolApprovalBlocks({ + integrationId: 'linear', + tokenType: 'auth', + resolveActingUserId: async () => 'user-1', + }); + expect(approvals.shadowDefaultTools).toBe(true); + const call = { + integrationId: 'linear', + args: {}, + userId: 'user-1', + taskId: null, + }; + shadowProxyToolCall(approvals, { ...call, toolName: 'list_issues' }); + expect(mockShadow).toHaveBeenCalledWith( + expect.objectContaining({ toolName: 'list_issues' }), + ); + // A tool with a stored choice is not Auto's to assess. + shadowProxyToolCall(approvals, { ...call, toolName: 'delete_issue' }); + expect(mockShadow).toHaveBeenCalledTimes(1); + + mockAutoState.mockResolvedValue({ mode: 'off' }); + const off = await resolveProxyToolApprovalBlocks({ + integrationId: 'linear', + tokenType: 'auth', + resolveActingUserId: async () => 'user-1', + }); + shadowProxyToolCall(off, { ...call, toolName: 'list_issues' }); + expect(mockShadow).toHaveBeenCalledTimes(1); + }); }); describe('claimProxyTaskToolCall', () => { diff --git a/apps/api/src/handlers/mcp/native-tool-approvals.ts b/apps/api/src/handlers/mcp/native-tool-approvals.ts index 0d12c9a7e0..3866c8bbda 100644 --- a/apps/api/src/handlers/mcp/native-tool-approvals.ts +++ b/apps/api/src/handlers/mcp/native-tool-approvals.ts @@ -1,8 +1,10 @@ import { claimProxyTaskToolCall, describeProxyToolApprovalBlock, + resolveProxyToolApprovalBlock, resolveProxyToolApprovalBlocks, - type ProxyToolApprovalBlock, + shadowProxyToolCall, + type ProxyToolApprovals, } from './tool-approval-enforcement'; import { getJsonRpcMethod, @@ -43,25 +45,31 @@ export async function readNativeMcpRequestBody( * Native in-process MCP handlers do not pass through createMcpProxy, so they * need the same policy guard explicitly. Reject hides a tool from tools/list; * ask still appears for the model and is held until the Session owner decides. + * A tool nobody has made a choice about follows Auto mode exactly as it does + * at the proxy: shadow-assessed, or held for a claim while Auto is on. */ export async function resolveNativeToolApprovalGuard(input: { auth: McpAuthContext; integrationId: string; }): Promise { - const blocks = await resolveProxyToolApprovalBlocks({ + const approvals = await resolveProxyToolApprovalBlocks({ integrationId: input.integrationId, tokenType: input.auth.tokenType, resolveActingUserId: () => resolveTaskOrSessionUserIdOrNull(input.auth), resolveTaskId: () => resolveRunTokenTaskId(input.auth), }); - return blocks.size === 0 ? NOOP_GUARD : new NativeGuard(input, blocks); + const inert = + approvals.blocks.size === 0 && + !approvals.defaultBlock && + !approvals.shadowDefaultTools; + return inert ? NOOP_GUARD : new NativeGuard(input, approvals); } class NativeGuard implements NativeToolApprovalGuard { constructor( private readonly input: { auth: McpAuthContext; integrationId: string }, - private readonly blocks: ReadonlyMap, + private readonly approvals: ProxyToolApprovals, ) {} async checkCall(body: unknown): Promise { @@ -76,17 +84,27 @@ class NativeGuard implements NativeToolApprovalGuard { const toolName = getToolCallName(body); if (!toolName) return null; - const block = this.blocks.get(toolName); - if (!block) return null; + const args = (body as { params?: { arguments?: unknown } }).params + ?.arguments; + const taskId = await resolveRunTokenTaskId(this.input.auth); + shadowProxyToolCall(this.approvals, { + integrationId: this.input.integrationId, + toolName, + args, + userId: this.input.auth.userId ?? null, + taskId, + }); + + const block = resolveProxyToolApprovalBlock(this.approvals, toolName); + if (!block || block === 'allow') return null; if (block === 'needs_approval') { try { const approved = await claimProxyTaskToolCall({ - taskId: await resolveRunTokenTaskId(this.input.auth), + taskId, integrationId: this.input.integrationId, toolName, - args: (body as { params?: { arguments?: unknown } }).params - ?.arguments, + args, }); if (approved) return null; } catch { @@ -135,7 +153,7 @@ class NativeGuard implements NativeToolApprovalGuard { typeof tool === 'object' && 'name' in tool && typeof tool.name === 'string' && - this.blocks.get(tool.name) === 'reject' + this.approvals.blocks.get(tool.name) === 'reject' ), ); diff --git a/apps/api/src/handlers/mcp/proxy-utils.ts b/apps/api/src/handlers/mcp/proxy-utils.ts index 03c3cb2eda..24b216caef 100644 --- a/apps/api/src/handlers/mcp/proxy-utils.ts +++ b/apps/api/src/handlers/mcp/proxy-utils.ts @@ -26,8 +26,10 @@ import { import { claimProxyTaskToolCall, describeProxyToolApprovalBlock, + resolveProxyToolApprovalBlock, resolveProxyToolApprovalBlocks, - type ProxyToolApprovalBlock, + shadowProxyToolCall, + type ProxyToolApprovals, } from './tool-approval-enforcement'; type JsonRpcRequestId = string | number | null; @@ -1032,10 +1034,13 @@ export function createMcpProxy(config: McpProxyConfig) { // A rejected tool is hidden and refused exactly like a disabled one; only // the refusal message differs. A tool that needs approval stays listed, // and each call to it must claim the Session owner's approval below. - let toolApprovalBlocks = new Map(); + let toolApprovals: ProxyToolApprovals = { + blocks: new Map(), + shadowDefaultTools: false, + }; if (credentials.toolApprovalIntegrationId) { try { - toolApprovalBlocks = await resolveProxyToolApprovalBlocks({ + toolApprovals = await resolveProxyToolApprovalBlocks({ integrationId: credentials.toolApprovalIntegrationId, policyScope: credentials.toolApprovalPolicyScope, tokenType: auth.tokenType, @@ -1058,7 +1063,7 @@ export function createMcpProxy(config: McpProxyConfig) { `Failed to resolve ${name} tool approval policies`, ); } - const rejectedToolNames = [...toolApprovalBlocks] + const rejectedToolNames = [...toolApprovals.blocks] .filter(([, block]) => block === 'reject') .map(([toolName]) => toolName); if (rejectedToolNames.length > 0) { @@ -1103,7 +1108,8 @@ export function createMcpProxy(config: McpProxyConfig) { const hasToolRestrictions = Boolean( effectiveAllowedToolNames || credentials.disabledToolNames?.length || - toolApprovalBlocks.size, + toolApprovals.blocks.size || + toolApprovals.defaultBlock, ); if ( @@ -1149,11 +1155,11 @@ export function createMcpProxy(config: McpProxyConfig) { }, ), ); - const approvalBlock = toolApprovalBlocks.get(toolName); + const approvalBlock = toolApprovals.blocks.get(toolName); return jsonRpcErrorResponse( 403, -32000, - approvalBlock + approvalBlock && approvalBlock !== 'allow' ? describeProxyToolApprovalBlock(toolName, approvalBlock) : `${name} MCP tool "${toolName}" is not allowed on this endpoint`, getJsonRpcRequestId(parsedBody), @@ -1163,10 +1169,23 @@ export function createMcpProxy(config: McpProxyConfig) { const gatedToolName = method === 'POST' ? getToolCallName(parsedBody) : null; + const callArguments = ( + parsedBody as { params?: { arguments?: unknown } } | undefined + )?.params?.arguments; + if (gatedToolName && credentials.toolApprovalIntegrationId) { + shadowProxyToolCall(toolApprovals, { + integrationId: credentials.toolApprovalIntegrationId, + toolName: gatedToolName, + args: callArguments, + userId: auth.userId ?? null, + taskId: await resolveRunTokenTaskId(auth), + }); + } if ( gatedToolName && credentials.toolApprovalIntegrationId && - toolApprovalBlocks.get(gatedToolName) === 'needs_approval' + resolveProxyToolApprovalBlock(toolApprovals, gatedToolName) === + 'needs_approval' ) { let approved = false; try { @@ -1174,8 +1193,7 @@ export function createMcpProxy(config: McpProxyConfig) { taskId: await resolveRunTokenTaskId(auth), integrationId: credentials.toolApprovalIntegrationId, toolName: gatedToolName, - args: (parsedBody as { params?: { arguments?: unknown } }).params - ?.arguments, + args: callArguments, }); } catch (error) { // Fail closed: an unreadable approval is not an approval. diff --git a/apps/api/src/handlers/mcp/tool-approval-enforcement.ts b/apps/api/src/handlers/mcp/tool-approval-enforcement.ts index b1fe5eb96e..f3147fd73f 100644 --- a/apps/api/src/handlers/mcp/tool-approval-enforcement.ts +++ b/apps/api/src/handlers/mcp/tool-approval-enforcement.ts @@ -9,13 +9,30 @@ import { listIntegrationToolUserPolicies, } from '@roomote/db/server'; import { - integrationToolModeAsks, + integrationToolModeIsAutoAssessed, resolveEffectiveIntegrationToolMode, resolveGoverningIntegrationToolPolicies, type IntegrationToolPolicyMode, } from '@roomote/types'; +import { + recordIntegrationToolShadowEvaluationInBackground, + resolveIntegrationToolAutoState, +} from '@roomote/cloud-agents/server/integration-tool-auto-evaluation'; + +export type ProxyToolApprovalBlock = 'reject' | 'needs_approval' | 'allow'; -export type ProxyToolApprovalBlock = 'reject' | 'needs_approval'; +export type ProxyToolApprovals = { + /** Per-tool blocks from stored choices and session overrides. */ + blocks: Map; + /** + * What a tool with no entry in `blocks` gets. `needs_approval` while Auto + * mode is on and the caller is a task: every default tool call must then + * claim an approval, whether a person's or the model's. + */ + defaultBlock?: 'needs_approval'; + /** Whether a call to a default tool should be shadow-assessed. */ + shadowDefaultTools: boolean; +}; /** * Experiment-gated (`integrationToolApprovals`) enforcement of per-tool @@ -50,10 +67,16 @@ export async function resolveProxyToolApprovalBlocks(input: { resolveActingUserId: () => Promise; /** The run token's task, for its Session's overrides. */ resolveTaskId?: () => Promise; -}): Promise> { +}): Promise { const blocks = new Map(); + const result: ProxyToolApprovals = { blocks, shadowDefaultTools: false }; if (!(await isDeploymentExperimentEnabled('integrationToolApprovals'))) { - return blocks; + return result; + } + const autoState = await resolveIntegrationToolAutoState(); + result.shadowDefaultTools = autoState.mode === 'shadow'; + if (autoState.mode === 'on' && input.tokenType === 'run') { + result.defaultBlock = 'needs_approval'; } const actingUserId = input.policyScope === 'deployment' @@ -99,11 +122,45 @@ export async function resolveProxyToolApprovalBlocks(input: { }); if (mode === 'reject') { blocks.set(toolName, 'reject'); - } else if (integrationToolModeAsks(mode) && input.tokenType === 'run') { + } else if (mode === 'ask' && input.tokenType === 'run') { blocks.set(toolName, 'needs_approval'); + } else if ( + result.defaultBlock && + !integrationToolModeIsAutoAssessed({ + policyMode: policyModes.get(toolName), + sessionOverrideMode: overrideModes.get(toolName), + }) + ) { + // A person's choice to run this tool: no approval to claim. + blocks.set(toolName, 'allow'); } } - return blocks; + return result; +} + +/** How the proxy treats one named tool call. */ +export function resolveProxyToolApprovalBlock( + approvals: ProxyToolApprovals, + toolName: string, +): ProxyToolApprovalBlock | 'allow' | undefined { + return approvals.blocks.get(toolName) ?? approvals.defaultBlock; +} + +/** Shadow-assess a call to a default tool; never awaited. */ +export function shadowProxyToolCall( + approvals: ProxyToolApprovals, + input: { + integrationId: string; + toolName: string; + args: unknown; + userId: string | null; + taskId: string | null; + }, +): void { + if (!approvals.shadowDefaultTools || approvals.blocks.has(input.toolName)) { + return; + } + recordIntegrationToolShadowEvaluationInBackground(input); } /** @@ -133,5 +190,7 @@ export function describeProxyToolApprovalBlock( ): string { return block === 'reject' ? `Tool "${toolName}" is disabled by a tool approval policy.` - : `Tool "${toolName}" needs approval before it runs, and this call has not been approved.`; + : block === 'needs_approval' + ? `Tool "${toolName}" needs approval before it runs, and this call has not been approved.` + : `Tool "${toolName}" is allowed.`; } diff --git a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx index 1dd855f11f..57805a6c4d 100644 --- a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx +++ b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx @@ -140,4 +140,27 @@ describe('PendingIntegrationToolApprovals', () => { expect(screen.getByText('No additional details.')).toBeInTheDocument(); expect(screen.queryByText('No arguments')).not.toBeInTheDocument(); }); + + it('tells the person what Auto made of the call, when it looked', () => { + render( + + + , + ); + expect(screen.getByTestId('auto-evaluation')).toHaveTextContent( + 'Auto flagged this call as risky and asked you.', + ); + }); }); diff --git a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx index b8e6b247d5..ccb8de1190 100644 --- a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx +++ b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx @@ -14,6 +14,7 @@ import { import { MCP_INTEGRATIONS, type IntegrationToolApprovalMetadata, + type IntegrationToolAutoEvaluation, } from '@roomote/types'; const integrationNames = new Map( @@ -64,6 +65,25 @@ function approvalPrompt(item: IntegrationToolApprovalMetadata): string { return `Let ${name} use this tool?`; } +/** + * One line on what Auto made of the call, so the person deciding knows + * why they are being asked. Auto never rejects, so this only ever explains + * why it did not run the call on its own. + */ +function describeAutoEvaluation( + evaluation: IntegrationToolAutoEvaluation, +): string { + if (evaluation.unavailable === 'no_model') { + return 'Auto couldn’t check this call because no decision model is available.'; + } + if (evaluation.unavailable === 'error') { + return 'Auto couldn’t check this call.'; + } + return evaluation.recommendation === 'approve' + ? 'Auto would have run this call.' + : 'Auto flagged this call as risky and asked you.'; +} + function summarizeArgs(argsSummary: unknown): string | null { if ( argsSummary === null || @@ -141,6 +161,14 @@ export function PendingIntegrationToolApprovals({

Roomote is waiting for your approval to continue.

+ {item.autoEvaluation ? ( +

+ {describeAutoEvaluation(item.autoEvaluation)} +

+ ) : null} diff --git a/apps/web/src/components/settings/IntegrationToolApprovalControls.tsx b/apps/web/src/components/settings/IntegrationToolApprovalControls.tsx index 24558997d0..4d9661a480 100644 --- a/apps/web/src/components/settings/IntegrationToolApprovalControls.tsx +++ b/apps/web/src/components/settings/IntegrationToolApprovalControls.tsx @@ -24,20 +24,21 @@ import { type LucideIcon, } from '@/components/system'; -const APPROVAL_MODES: { +/** + * The stored choices a tool can be given. Auto, the default, is no choice at + * all: a row shows it as nothing pressed, and pressing the selected choice + * again returns to it. The group row also offers Auto, so a whole group can + * be returned to it in one step. + */ +type ApprovalModeOption = { mode: IntegrationToolPolicyMode; label: string; tooltip: string; icon: LucideIcon; -}[] = [ - { - mode: 'auto', - label: 'Auto', - tooltip: 'Defer to the configured judgement model', - icon: Scale, - }, +}; +const STORED_MODES: ApprovalModeOption[] = [ { - mode: 'allow', + mode: 'always_allow', label: 'Always allow', tooltip: 'Always allow', icon: CheckCheck, @@ -50,6 +51,10 @@ const APPROVAL_MODES: { }, { mode: 'reject', label: 'Disable', tooltip: 'Disable', icon: Ban }, ]; +const BULK_MODES: ApprovalModeOption[] = [ + { mode: 'allow', label: 'Auto', tooltip: 'Auto', icon: Scale }, + ...STORED_MODES, +]; /** * Experiment-gated (`integrationToolApprovals`) per-tool approval mode: one @@ -61,11 +66,13 @@ function IntegrationToolApprovalModeControl({ toolName, value, disabled, + options = STORED_MODES, onChange, }: { toolName: string; value?: IntegrationToolPolicyMode; disabled?: boolean; + options?: ApprovalModeOption[]; onChange: (mode: IntegrationToolPolicyMode) => void; }) { return ( @@ -74,7 +81,7 @@ function IntegrationToolApprovalModeControl({ aria-label={`Approval mode for ${toolName}`} className="flex shrink-0 items-center" > - {APPROVAL_MODES.map(({ mode, label, tooltip, icon: Icon }) => { + {options.map(({ mode, label, tooltip, icon: Icon }) => { const checked = mode === value; return ( @@ -86,7 +93,9 @@ function IntegrationToolApprovalModeControl({ aria-label={label} disabled={disabled} onPressedChange={() => { + // Pressing the selected choice again returns to Auto. if (!checked) onChange(mode); + else if (mode !== 'allow') onChange('allow'); }} >