From 1a024f30a32c7684dd611cedf3becfc220888a7a Mon Sep 17 00:00:00 2001 From: Coco Sheng Date: Tue, 19 May 2026 10:44:09 -0400 Subject: [PATCH] fix: allow configured MCP servers in non-interactive mode (#27215) --- packages/core/src/policy/config.test.ts | 140 ++++++++++++++++++++++++ packages/core/src/policy/config.ts | 32 ++++++ packages/core/src/policy/types.ts | 1 + 3 files changed, 173 insertions(+) diff --git a/packages/core/src/policy/config.test.ts b/packages/core/src/policy/config.test.ts index 0d23eaaeed..01ddbc030c 100644 --- a/packages/core/src/policy/config.test.ts +++ b/packages/core/src/policy/config.test.ts @@ -24,6 +24,7 @@ import { import { Storage } from '../config/storage.js'; import * as tomlLoader from './toml-loader.js'; import { coreEvents } from '../utils/events.js'; +import { MCPServerConfig } from '../config/config.js'; vi.unmock('../config/storage.js'); @@ -279,6 +280,145 @@ describe('createPolicyEngineConfig', () => { expect(untrustedRule).toBeUndefined(); }); + it('should NOT automatically allow configured MCP servers in non-interactive mode by default', async () => { + const config = await createPolicyEngineConfig( + { + mcpServers: { + 'server-1': new MCPServerConfig('node', []), + }, + }, + ApprovalMode.DEFAULT, + MOCK_DEFAULT_DIR, + false, // non-interactive + ); + + const rule = config.rules?.find( + (r) => r.mcpName === 'server-1' && r.decision === PolicyDecision.ALLOW, + ); + expect(rule).toBeUndefined(); + }); + + it('should automatically allow configured MCP servers in non-interactive mode if opted-in', async () => { + const config = await createPolicyEngineConfig( + { + mcp: { autoAllowInHeadless: true }, + mcpServers: { + 'server-1': new MCPServerConfig('node', []), + 'server-2': new MCPServerConfig('python', []), + }, + }, + ApprovalMode.DEFAULT, + MOCK_DEFAULT_DIR, + false, // non-interactive + ); + + const rule1 = config.rules?.find( + (r) => r.mcpName === 'server-1' && r.decision === PolicyDecision.ALLOW, + ); + const rule2 = config.rules?.find( + (r) => r.mcpName === 'server-2' && r.decision === PolicyDecision.ALLOW, + ); + + expect(rule1).toBeDefined(); + expect(rule1?.source).toBe('Settings (Headless MCP Auto-Allow)'); + expect(rule2).toBeDefined(); + expect(rule2?.source).toBe('Settings (Headless MCP Auto-Allow)'); + }); + + it('should NOT automatically allow configured MCP servers in interactive mode even if opted-in', async () => { + const config = await createPolicyEngineConfig( + { + mcp: { autoAllowInHeadless: true }, + mcpServers: { + 'server-1': new MCPServerConfig('node', []), + }, + }, + ApprovalMode.DEFAULT, + MOCK_DEFAULT_DIR, + true, // interactive + ); + + const rule = config.rules?.find( + (r) => r.mcpName === 'server-1' && r.decision === PolicyDecision.ALLOW, + ); + expect(rule).toBeUndefined(); + }); + + it('should NOT duplicate allow rules if an MCP server is already explicitly allowed, wildcard allowed, or trusted', async () => { + const config = await createPolicyEngineConfig( + { + mcp: { + autoAllowInHeadless: true, + allowed: ['server-1', '*'], + }, + mcpServers: { + 'server-1': new MCPServerConfig('node', []), + 'server-2': new MCPServerConfig('node', []), + 'server-3': { trust: true }, + 'server-4': new MCPServerConfig('node', []), + }, + }, + ApprovalMode.DEFAULT, + MOCK_DEFAULT_DIR, + false, // non-interactive + ); + + // server-1: already in mcp.allowed + const rules1 = config.rules?.filter( + (r) => r.mcpName === 'server-1' && r.decision === PolicyDecision.ALLOW, + ); + expect(rules1).toHaveLength(1); + expect(rules1?.[0].source).toBe('Settings (MCP Allowed)'); + + // server-2: covered by '*' in mcp.allowed + // Note: the logic adds a rule for '*' which will match server-2 at runtime, + // but the loop in headless auto-allow should skip adding a specific rule for server-2. + const rules2 = config.rules?.filter( + (r) => r.mcpName === 'server-2' && r.decision === PolicyDecision.ALLOW, + ); + expect(rules2).toHaveLength(0); + + // server-3: already trusted + const rules3 = config.rules?.filter( + (r) => r.mcpName === 'server-3' && r.decision === PolicyDecision.ALLOW, + ); + expect(rules3).toHaveLength(1); + expect(rules3?.[0].source).toBe('Settings (MCP Trusted)'); + + // server-4: NOT explicitly allowed or trusted, but SHOULD NOT be added because '*' exists in mcp.allowed + const rules4 = config.rules?.filter( + (r) => r.mcpName === 'server-4' && r.decision === PolicyDecision.ALLOW, + ); + expect(rules4).toHaveLength(0); + + // Verify the wildcard rule exists + const wildcardRule = config.rules?.find( + (r) => r.mcpName === '*' && r.decision === PolicyDecision.ALLOW, + ); + expect(wildcardRule).toBeDefined(); + expect(wildcardRule?.toolName).toBe('mcp_*'); + }); + + it('should use correct tool name pattern for wildcard server in headless auto-allow', async () => { + const config = await createPolicyEngineConfig( + { + mcp: { autoAllowInHeadless: true }, + mcpServers: { + '*': new MCPServerConfig('node', []), + }, + }, + ApprovalMode.DEFAULT, + MOCK_DEFAULT_DIR, + false, // non-interactive + ); + + const rule = config.rules?.find( + (r) => r.mcpName === '*' && r.decision === PolicyDecision.ALLOW, + ); + expect(rule).toBeDefined(); + expect(rule?.toolName).toBe('mcp_*'); + }); + it('should handle multiple MCP server configurations together', async () => { const config = await createPolicyEngineConfig( { diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 6d978479cb..64af14f1cd 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -600,6 +600,38 @@ export async function createPolicyEngineConfig( } } + // In non-interactive mode, automatically allow all configured MCP servers if opted-in. + // This ensures that tools provided by these servers are available without + // requiring explicit entries in settings.mcp.allowed. + if ( + !interactive && + settings.mcp?.autoAllowInHeadless && + settings.mcpServers + ) { + for (const serverName of Object.keys(settings.mcpServers)) { + // Avoid duplicates if already explicitly allowed, allowed via wildcard, or trusted. + if ( + settings.mcp?.allowed?.includes(serverName) || + settings.mcp?.allowed?.includes('*') || + settings.mcpServers[serverName].trust + ) { + continue; + } + + rules.push({ + toolName: + serverName === '*' + ? `${MCP_TOOL_PREFIX}*` + : `${MCP_TOOL_PREFIX}${serverName}_*`, + mcpName: serverName, + decision: PolicyDecision.ALLOW, + priority: ALLOWED_MCP_SERVER_PRIORITY, + source: 'Settings (Headless MCP Auto-Allow)', + modes: nonPlanModes, + }); + } + } + return { rules, checkers, diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 672e3f8416..5af3754903 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -333,6 +333,7 @@ export interface PolicySettings { mcp?: { excluded?: string[]; allowed?: string[]; + autoAllowInHeadless?: boolean; }; tools?: { core?: string[];