diff --git a/src/server/lib/validation/agentSessionConfigSchemas.ts b/src/server/lib/validation/agentSessionConfigSchemas.ts index 7c04ad2f..6ae885c0 100644 --- a/src/server/lib/validation/agentSessionConfigSchemas.ts +++ b/src/server/lib/validation/agentSessionConfigSchemas.ts @@ -18,7 +18,7 @@ const toolRuleSchema = { type: 'object', properties: { toolKey: { type: 'string', minLength: 1, maxLength: 255 }, - mode: { type: 'string', enum: ['allow', 'deny'] }, + mode: { type: 'string', enum: ['allow', 'require_approval', 'deny'] }, }, required: ['toolKey', 'mode'], additionalProperties: false, diff --git a/src/server/lib/validation/agentSessionConfigValidator.ts b/src/server/lib/validation/agentSessionConfigValidator.ts index 67d19b55..7ba4b6fb 100644 --- a/src/server/lib/validation/agentSessionConfigValidator.ts +++ b/src/server/lib/validation/agentSessionConfigValidator.ts @@ -103,7 +103,7 @@ export function validateAgentSessionControlPlaneConfig(config: Partial 255) { throw new AgentSessionConfigValidationError(`toolRules entry "${rule.toolKey}" exceeds maximum toolKey length.`); } - if (rule.mode !== 'allow' && rule.mode !== 'deny') { + if (rule.mode !== 'allow' && rule.mode !== 'require_approval' && rule.mode !== 'deny') { throw new AgentSessionConfigValidationError( `toolRules entry "${rule.toolKey}" has unsupported mode "${rule.mode}".` ); diff --git a/src/server/services/__tests__/agentSessionConfig.test.ts b/src/server/services/__tests__/agentSessionConfig.test.ts index 4e2502bb..404f87b8 100644 --- a/src/server/services/__tests__/agentSessionConfig.test.ts +++ b/src/server/services/__tests__/agentSessionConfig.test.ts @@ -111,6 +111,81 @@ describe('AgentSessionConfigService', () => { }); }); + it('persists require-approval tool overrides in control-plane config', async () => { + const service = makeService(); + + await expect( + service.setGlobalConfig({ + toolRules: [ + { + toolKey: 'mcp__sandbox__workspace_read_file', + mode: 'require_approval', + }, + ], + }) + ).resolves.toEqual({ + toolRules: [ + { + toolKey: 'mcp__sandbox__workspace_read_file', + mode: 'require_approval', + }, + ], + }); + + expect(mockGlobalConfigSetConfig).toHaveBeenCalledWith('agentSessionDefaults', { + controlPlane: { + toolRules: [ + { + toolKey: 'mcp__sandbox__workspace_read_file', + mode: 'require_approval', + }, + ], + }, + }); + }); + + it('treats explicit tool rules as effective overrides in the inventory', async () => { + const service = makeService(); + + jest.spyOn(service, 'getGlobalConfig').mockResolvedValue({ + toolRules: [ + { + toolKey: 'mcp__sandbox__workspace_read_file', + mode: 'allow', + }, + ], + }); + jest.spyOn(service, 'getEffectiveConfig').mockResolvedValue({ + systemPrompt: 'base', + appendSystemPrompt: 'append', + toolRules: [ + { + toolKey: 'mcp__sandbox__workspace_read_file', + mode: 'allow', + }, + ], + }); + jest.spyOn(AgentPolicyService, 'getEffectivePolicy').mockResolvedValue({ + ...DEFAULT_AGENT_APPROVAL_POLICY, + rules: { + ...DEFAULT_AGENT_APPROVAL_POLICY.rules, + read: 'deny', + }, + }); + + const entries = await service.listToolInventory('global'); + const readFileEntry = entries.find((entry) => entry.toolName === 'workspace.read_file'); + + expect(readFileEntry).toEqual( + expect.objectContaining({ + approvalMode: 'deny', + scopeRuleMode: 'allow', + effectiveRuleMode: 'allow', + availability: 'available', + }) + ); + }); + it('updates runtime settings without overwriting control-plane settings', async () => { const service = makeService(); diff --git a/src/server/services/agent/CapabilityService.ts b/src/server/services/agent/CapabilityService.ts index de13ccb6..2ae87d80 100644 --- a/src/server/services/agent/CapabilityService.ts +++ b/src/server/services/agent/CapabilityService.ts @@ -57,9 +57,17 @@ function resolvePrimaryRepo(session: AgentSession): string | undefined { return session.selectedServices?.[0]?.repo || undefined; } -function isToolAllowed(toolRules: AgentSessionToolRule[] | undefined, toolKey: string): boolean { +function resolveToolApprovalMode({ + toolRules, + toolKey, + capabilityMode, +}: { + toolRules: AgentSessionToolRule[] | undefined; + toolKey: string; + capabilityMode: AgentApprovalMode; +}): AgentApprovalMode { const rule = toolRules?.find((item) => item.toolKey === toolKey); - return rule?.mode !== 'deny'; + return rule?.mode || capabilityMode; } function resolveSessionWorkspaceGatewayBaseUrl(session: AgentSession): string | null { @@ -344,9 +352,13 @@ export default class AgentCapabilityService { entry.toolName, entry.annotations || discoveredTool.annotations ); - const mode = AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey); + const mode = resolveToolApprovalMode({ + toolRules, + toolKey: entry.toolKey, + capabilityMode: AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey), + }); - if (mode === 'deny' || !isToolAllowed(toolRules, entry.toolKey)) { + if (mode === 'deny') { continue; } @@ -485,10 +497,14 @@ export default class AgentCapabilityService { } const capabilityKey = AgentPolicyService.capabilityForMcpTool(discoveredTool.name, discoveredTool.annotations); - const mode = AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey); const toolName = buildAgentToolKey(server.slug, discoveredTool.name); + const mode = resolveToolApprovalMode({ + toolRules, + toolKey: toolName, + capabilityMode: AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey), + }); - if (mode === 'deny' || !isToolAllowed(toolRules, toolName)) { + if (mode === 'deny') { continue; } diff --git a/src/server/services/agent/__tests__/CapabilityService.test.ts b/src/server/services/agent/__tests__/CapabilityService.test.ts index bd7e011c..0462aa23 100644 --- a/src/server/services/agent/__tests__/CapabilityService.test.ts +++ b/src/server/services/agent/__tests__/CapabilityService.test.ts @@ -22,6 +22,7 @@ const mockListTools = jest.fn(); const mockCallTool = jest.fn(); const mockClose = jest.fn(); const mockLoggerWarn = jest.fn(); +const mockModeForCapability = jest.fn(() => 'allow'); let currentTransport: Record | null = null; @@ -57,7 +58,7 @@ jest.mock('../PolicyService', () => ({ __esModule: true, default: { capabilityForMcpTool: jest.fn(() => 'external_mcp_read'), - modeForCapability: jest.fn(() => 'allow'), + modeForCapability: (...args: unknown[]) => mockModeForCapability(...args), }, })); @@ -107,6 +108,7 @@ describe('AgentCapabilityService.buildToolSet', () => { beforeEach(() => { jest.clearAllMocks(); + mockModeForCapability.mockReturnValue('allow'); currentTransport = null; mockResolveServersForRepo.mockResolvedValue([stdioServer]); mockConnect.mockImplementation(async (transport) => { @@ -228,4 +230,29 @@ describe('AgentCapabilityService.buildToolSet', () => { expect(tools.mcp__figma__get_design_context).toBeUndefined(); expect(mockLoggerWarn).toHaveBeenCalled(); }); + + it('lets session tool rules override the family approval mode for sandbox tools', async () => { + mockModeForCapability.mockReturnValue('deny'); + + const tools = await AgentCapabilityService.buildToolSet({ + session, + repoFullName: 'example-org/example-repo', + userIdentity, + approvalPolicy: {} as any, + toolRules: [ + { + toolKey: 'mcp__sandbox__workspace_read_file', + mode: 'allow', + }, + ], + workspaceToolDiscoveryTimeoutMs: 4500, + workspaceToolExecutionTimeoutMs: 22000, + }); + + expect(tools.mcp__sandbox__workspace_read_file).toEqual( + expect.objectContaining({ + needsApproval: false, + }) + ); + }); }); diff --git a/src/server/services/agent/__tests__/sandboxToolCatalog.test.ts b/src/server/services/agent/__tests__/sandboxToolCatalog.test.ts index 19da2132..428804b4 100644 --- a/src/server/services/agent/__tests__/sandboxToolCatalog.test.ts +++ b/src/server/services/agent/__tests__/sandboxToolCatalog.test.ts @@ -107,4 +107,26 @@ describe('sandboxToolCatalog', () => { '- do not claim a tool is unavailable unless it is not equipped here or a real tool call fails', ]); }); + + it('keeps explicitly allowed tools in the prompt summary even when the family is denied', () => { + const lines = buildSessionWorkspacePromptLines({ + approvalPolicy: { + ...DEFAULT_AGENT_APPROVAL_POLICY, + rules: { + ...DEFAULT_AGENT_APPROVAL_POLICY.rules, + read: 'deny', + }, + }, + toolRules: [ + { + toolKey: 'mcp__sandbox__workspace_read_file', + mode: 'allow', + }, + ], + includeSkills: false, + }); + + expect(lines.join('\n')).toContain('workspace.read_file'); + expect(lines.join('\n')).not.toContain('workspace.glob'); + }); }); diff --git a/src/server/services/agent/sandboxToolCatalog.ts b/src/server/services/agent/sandboxToolCatalog.ts index 186433b1..057c3b8e 100644 --- a/src/server/services/agent/sandboxToolCatalog.ts +++ b/src/server/services/agent/sandboxToolCatalog.ts @@ -284,13 +284,10 @@ function isSessionWorkspaceToolAllowed( toolRules: AgentSessionToolRule[] = [] ): boolean { const rule = toolRules.find((item) => item.toolKey === entry.toolKey); - if (rule?.mode === 'deny') { - return false; - } - const capabilityKey = AgentPolicyService.capabilityForMcpTool(entry.toolName, entry.annotations); + const mode = rule?.mode || AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey); - return AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey) !== 'deny'; + return mode !== 'deny'; } export function buildSessionWorkspacePromptLines({ diff --git a/src/server/services/agentSessionConfig.ts b/src/server/services/agentSessionConfig.ts index faae2b99..cd0e7cf3 100644 --- a/src/server/services/agentSessionConfig.ts +++ b/src/server/services/agentSessionConfig.ts @@ -166,7 +166,7 @@ function normalizeToolRules(value: unknown): AgentSessionToolRule[] { const toolKey = typeof (entry as { toolKey?: unknown }).toolKey === 'string' ? (entry as { toolKey: string }).toolKey : ''; const mode = (entry as { mode?: unknown }).mode; - if (!toolKey || (mode !== 'allow' && mode !== 'deny')) { + if (!toolKey || (mode !== 'allow' && mode !== 'require_approval' && mode !== 'deny')) { continue; } deduped.set(toolKey, { toolKey, mode }); @@ -502,11 +502,12 @@ export default class AgentSessionConfigService extends BaseService { const approvalMode = AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey); const scopeRuleMode = toRuleSelection(activeScopeConfig.toolRules || [], toolKey); const effectiveRuleMode = toRuleSelection(effectiveConfig.toolRules, toolKey); + const resolvedApprovalMode = effectiveRuleMode === 'inherit' ? approvalMode : effectiveRuleMode; const availability = - approvalMode === 'deny' - ? 'blocked_by_policy' - : effectiveRuleMode === 'deny' - ? 'blocked_by_tool_rule' + resolvedApprovalMode === 'deny' + ? effectiveRuleMode === 'deny' + ? 'blocked_by_tool_rule' + : 'blocked_by_policy' : 'available'; entries.push({ diff --git a/src/server/services/types/agentSessionConfig.ts b/src/server/services/types/agentSessionConfig.ts index 055bd736..e4c1e5cb 100644 --- a/src/server/services/types/agentSessionConfig.ts +++ b/src/server/services/types/agentSessionConfig.ts @@ -16,7 +16,7 @@ import type { AgentApprovalMode, AgentCapabilityKey } from 'server/services/agent/types'; -export type AgentSessionToolRuleMode = 'allow' | 'deny'; +export type AgentSessionToolRuleMode = AgentApprovalMode; export type AgentSessionToolRuleSelection = AgentSessionToolRuleMode | 'inherit'; export interface AgentSessionToolRule { diff --git a/src/shared/openApiSpec.ts b/src/shared/openApiSpec.ts index 17c22330..0bef0da9 100644 --- a/src/shared/openApiSpec.ts +++ b/src/shared/openApiSpec.ts @@ -1506,7 +1506,7 @@ export const openApiSpecificationForV2Api: OAS3Options = { toolKey: { type: 'string' }, mode: { type: 'string', - enum: ['allow', 'deny'], + enum: ['allow', 'require_approval', 'deny'], }, }, required: ['toolKey', 'mode'], @@ -1631,11 +1631,11 @@ export const openApiSpecificationForV2Api: OAS3Options = { approvalMode: { $ref: '#/components/schemas/AgentApprovalMode' }, scopeRuleMode: { type: 'string', - enum: ['inherit', 'allow', 'deny'], + enum: ['inherit', 'allow', 'require_approval', 'deny'], }, effectiveRuleMode: { type: 'string', - enum: ['inherit', 'allow', 'deny'], + enum: ['inherit', 'allow', 'require_approval', 'deny'], }, availability: { type: 'string',