diff --git a/agents.config.example.json b/agents.config.example.json index 538ead98..449cfe82 100644 --- a/agents.config.example.json +++ b/agents.config.example.json @@ -44,7 +44,8 @@ "args": [ "@modelcontextprotocol/server-filesystem", "/path/to/allowed/directory" - ] + ], + "alwaysAllow": ["list_directory", "file_info"] }, { "name": "github", @@ -52,8 +53,12 @@ "args": ["@modelcontextprotocol/server-github"], "env": { "GITHUB_TOKEN": "your-github-token" - } + }, + "alwaysAllow": ["list_repositories"] } - ] + ], + "nanocoderTools": { + "alwaysAllow": ["execute_bash"] + } } } diff --git a/docs/mcp-configuration.md b/docs/mcp-configuration.md index 45772d0b..2b3518d2 100644 --- a/docs/mcp-configuration.md +++ b/docs/mcp-configuration.md @@ -36,7 +36,8 @@ Nanocoder supports three transport types for MCP servers: "transport": "stdio", "command": "npx", "args": ["-y", "@modelcontextprotocol/server-filesystem", "/path/to/project"], - "env": {} + "env": {}, + "alwaysAllow": ["list_directory", "file_info"] } ``` @@ -112,6 +113,7 @@ Nanocoder supports three transport types for MCP servers: - `name` (required): Display name for the server - `transport` (required): Transport type (`stdio`, `http`, `websocket`) +- `alwaysAllow` (optional): Array of MCP tool names that can run without user confirmation ### stdio Transport Fields diff --git a/source/commands/mcp.tsx b/source/commands/mcp.tsx index 4d02fc17..6dbccc88 100644 --- a/source/commands/mcp.tsx +++ b/source/commands/mcp.tsx @@ -64,6 +64,7 @@ export function MCP({toolManager}: MCPProps) { "transport": "stdio", "command": "node", "args": ["path/to/server.js"], + "alwaysAllow": ["safe_read", "status"], "env": { "API_KEY": "your-key" } @@ -128,7 +129,12 @@ export function MCP({toolManager}: MCPProps) { Tags: {serverInfo.tags.map(tag => `#${tag}`).join(' ')} )} - + {!!serverInfo?.autoApprovedCommands?.length && ( + + Auto-approved tools:{' '} + {serverInfo.autoApprovedCommands.join(', ')} + + )} {serverTools.length > 0 && ( Tools:{' '} diff --git a/source/config/index.ts b/source/config/index.ts index 418a6998..5e8b9a70 100644 --- a/source/config/index.ts +++ b/source/config/index.ts @@ -113,6 +113,8 @@ function loadAppConfig(): AppConfig { return { providers: processedData.nanocoder.providers ?? [], mcpServers: processedData.nanocoder.mcpServers ?? [], + alwaysAllow: processedData.nanocoder.alwaysAllow ?? [], + nanocoderTools: processedData.nanocoder.nanocoderTools ?? {}, }; } } catch (error) { diff --git a/source/config/nanocoder-tools-config.ts b/source/config/nanocoder-tools-config.ts new file mode 100644 index 00000000..7bfcb09d --- /dev/null +++ b/source/config/nanocoder-tools-config.ts @@ -0,0 +1,16 @@ +import {appConfig} from '@/config/index'; + +/** + * Check if a nanocoder tool is configured to always be allowed + * @param toolName - The name of the tool to check + * @returns true if the tool is in the alwaysAllow list, false otherwise + */ +export function isNanocoderToolAlwaysAllowed(toolName: string): boolean { + const alwaysAllowList = appConfig.nanocoderTools?.alwaysAllow; + + if (!Array.isArray(alwaysAllowList)) { + return false; + } + + return alwaysAllowList.includes(toolName); +} diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 7f1e1bb9..a20ff4e5 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -3,10 +3,10 @@ import type {ConversationStateManager} from '@/app/utils/conversation-state'; import AssistantMessage from '@/components/assistant-message'; import {ErrorMessage} from '@/components/message-box'; import UserMessage from '@/components/user-message'; +import {appConfig} from '@/config/index'; import {parseToolCalls} from '@/tool-calling/index'; import type {ToolManager} from '@/tools/tool-manager'; import type {LLMClient, Message, ToolCall, ToolResult} from '@/types/core'; -import {getLogger} from '@/utils/logging'; import {MessageBuilder} from '@/utils/message-builder'; import {parseToolArguments} from '@/utils/tool-args-parser'; import {displayToolResult} from '@/utils/tool-result-display'; @@ -273,6 +273,9 @@ export const processAssistantResponse = async ( const toolsNeedingConfirmation: ToolCall[] = []; const toolsToExecuteDirectly: ToolCall[] = []; + // Tools that are permitted to auto-run in non-interactive mode + const nonInteractiveAllowList = new Set(appConfig.alwaysAllow ?? []); + for (const toolCall of validToolCalls) { // Check if tool has a validator let validationFailed = false; @@ -290,12 +293,8 @@ export const processAssistantResponse = async ( if (!validationResult.valid) { validationFailed = true; } - } catch (error) { - const logger = getLogger(); - logger.debug('Tool validation threw error', { - toolName: toolCall.function.name, - error, - }); + } catch { + // Validation threw an error - treat as validation failure validationFailed = true; } } @@ -329,15 +328,8 @@ export const processAssistantResponse = async ( args: unknown, ) => boolean | Promise )(parsedArgs); - } catch (error) { - const logger = getLogger(); - logger.debug( - 'needsApproval evaluation failed, requiring approval', - { - toolName: toolCall.function.name, - error, - }, - ); + } catch { + // If evaluation fails, require approval for safety toolNeedsApproval = true; } } @@ -347,13 +339,19 @@ export const processAssistantResponse = async ( // Execute directly if: // 1. Validation failed (need to send error back to model) // 2. Tool has needsApproval: false - // 3. In auto-accept mode (except bash which always needs approval) + // 3. Explicitly allowed in non-interactive mode + // 4. In auto-accept mode (except bash which always needs approval) const isBashTool = toolCall.function.name === 'execute_bash'; - if ( + const isNonInteractiveAllowed = + nonInteractiveMode && + nonInteractiveAllowList.has(toolCall.function.name); + const shouldExecuteDirectly = validationFailed || !toolNeedsApproval || - (developmentMode === 'auto-accept' && !isBashTool) - ) { + isNonInteractiveAllowed || + (developmentMode === 'auto-accept' && !isBashTool); + + if (shouldExecuteDirectly) { toolsToExecuteDirectly.push(toolCall); } else { toolsNeedingConfirmation.push(toolCall); diff --git a/source/mcp/mcp-client.spec.ts b/source/mcp/mcp-client.spec.ts index 15236dae..a158ebec 100644 --- a/source/mcp/mcp-client.spec.ts +++ b/source/mcp/mcp-client.spec.ts @@ -1,4 +1,5 @@ import test from 'ava'; +import {setCurrentMode} from '../context/mode-context'; import {MCPClient} from './mcp-client'; // ============================================================================ @@ -139,7 +140,6 @@ test('MCPClient: isServerConnected returns false for non-existent servers', t => t.false(client.isServerConnected('another-server')); t.false(client.isServerConnected('')); }); - // ============================================================================ // Tests for getAllTools // ============================================================================ @@ -874,13 +874,15 @@ test('MCPClient.getToolMapping: returns mapping from connected HTTP server', asy // Verify mapping structure const firstMapping = mapping.entries().next().value; - const [toolName, mappingInfo] = firstMapping; - - t.is(typeof toolName, 'string'); - t.deepEqual(mappingInfo, { - serverName: 'test-deepwiki', - originalName: toolName, - }); + if (firstMapping) { + const [toolName, mappingInfo] = firstMapping; + + t.is(typeof toolName, 'string'); + t.deepEqual(mappingInfo, { + serverName: 'test-deepwiki', + originalName: toolName, + }); + } await client.disconnect(); }); @@ -943,3 +945,67 @@ test('MCPClient.connectToServer: validates websocket URL protocol', async t => { {message: /websocket URL must use ws:\/\/ or wss:\/\/ protocol/i}, ); }); + +test('MCPClient: alwaysAllow disables approval prompts', async t => { + const client = new MCPClient(); + const serverName = 'auto-server'; + + (client as any).serverTools.set(serverName, [ + { + name: 'safe_tool', + description: 'Safe MCP tool', + inputSchema: {type: 'object'}, + serverName, + }, + ]); + + (client as any).serverConfigs.set(serverName, { + name: serverName, + transport: 'stdio', + alwaysAllow: ['safe_tool'], + }); + + setCurrentMode('normal'); + + const registry = client.getNativeToolsRegistry(); + const tool = registry['safe_tool']; + + t.truthy(tool); + const needsApproval = + typeof tool?.needsApproval === 'function' + ? await tool.needsApproval({}, {toolCallId: 'test', messages: []}) + : tool?.needsApproval ?? true; + t.false(needsApproval); +}); + +test('MCPClient: non-whitelisted tools still require approval', async t => { + const client = new MCPClient(); + const serverName = 'restricted-server'; + + (client as any).serverTools.set(serverName, [ + { + name: 'restricted_tool', + description: 'Requires approval', + inputSchema: {type: 'object'}, + serverName, + }, + ]); + + (client as any).serverConfigs.set(serverName, { + name: serverName, + transport: 'stdio', + alwaysAllow: [], + }); + + setCurrentMode('normal'); + + const registry = client.getNativeToolsRegistry(); + const tool = registry['restricted_tool']; + + t.truthy(tool); + const needsApproval = + typeof tool?.needsApproval === 'function' + ? await tool.needsApproval({}, {toolCallId: 'test', messages: []}) + : tool?.needsApproval ?? false; + t.true(needsApproval); +}); diff --git a/source/mcp/mcp-client.ts b/source/mcp/mcp-client.ts index cb7035a3..5b44d983 100644 --- a/source/mcp/mcp-client.ts +++ b/source/mcp/mcp-client.ts @@ -38,6 +38,15 @@ export class MCPClient { private isConnected: boolean = false; private logger = getLogger(); + private isToolAutoApproved(toolName: string, serverName: string): boolean { + const serverConfig = this.serverConfigs.get(serverName); + if (!serverConfig?.alwaysAllow) { + return false; + } + + return serverConfig.alwaysAllow.includes(toolName); + } + constructor() { this.logger.debug('MCP client initialized'); } @@ -306,6 +315,7 @@ export class MCPClient { // dynamicTool is more explicit about unknown types compared to tool() // MCP schemas come from external servers and are not known at compile time const toolName = mcpTool.name; + const isAutoApproved = this.isToolAutoApproved(toolName, serverName); const coreTool = dynamicTool({ description: mcpTool.description ? `[MCP:${serverName}] ${mcpTool.description}` @@ -313,8 +323,12 @@ export class MCPClient { inputSchema: jsonSchema>( (mcpTool.inputSchema as unknown) || {type: 'object'}, ), - // Medium risk: MCP tools require approval except in auto-accept mode + // Medium risk: MCP tools require approval unless explicitly configured in the server's alwaysAllow list or in auto-accept mode needsApproval: () => { + if (isAutoApproved) { + return false; + } + const mode = getCurrentMode(); return mode !== 'auto-accept'; // true in normal/plan, false in auto-accept }, @@ -608,6 +622,7 @@ export class MCPClient { connected: boolean; description?: string; tags?: string[]; + autoApprovedCommands?: string[]; } | undefined { const client = this.clients.get(serverName); @@ -626,6 +641,7 @@ export class MCPClient { connected: true, description: serverConfig.description, tags: serverConfig.tags, + autoApprovedCommands: serverConfig.alwaysAllow, }; } } diff --git a/source/tools/execute-bash.tsx b/source/tools/execute-bash.tsx index 82c515b6..20905608 100644 --- a/source/tools/execute-bash.tsx +++ b/source/tools/execute-bash.tsx @@ -2,6 +2,7 @@ import {Box, Text} from 'ink'; import React from 'react'; import BashProgress from '@/components/bash-progress'; +import {isNanocoderToolAlwaysAllowed} from '@/config/nanocoder-tools-config'; import {TRUNCATION_OUTPUT_LIMIT} from '@/constants'; import {useTheme} from '@/hooks/useTheme'; import {type BashExecutionState, bashExecutor} from '@/services/bash-executor'; @@ -75,8 +76,16 @@ const executeBashCoreTool = tool({ }, required: ['command'], }), - // High risk: bash commands always require approval in all modes - needsApproval: true, + // High risk: bash commands require approval unless explicitly configured in nanocoderTools.alwaysAllow + needsApproval: () => { + // Check if this tool is configured to always be allowed + if (isNanocoderToolAlwaysAllowed('execute_bash')) { + return false; + } + + // Even in auto-accept mode, bash commands should require approval for security + return true; + }, execute: async (args, _options) => { return await executeExecuteBash(args); }, diff --git a/source/tools/string-replace.tsx b/source/tools/string-replace.tsx index 675e84fd..11052e6b 100644 --- a/source/tools/string-replace.tsx +++ b/source/tools/string-replace.tsx @@ -7,6 +7,7 @@ import React from 'react'; import ToolMessage from '@/components/tool-message'; import {getColors} from '@/config/index'; +import {isNanocoderToolAlwaysAllowed} from '@/config/nanocoder-tools-config'; import {getCurrentMode} from '@/context/mode-context'; import type {NanocoderToolExport} from '@/types/core'; import {jsonSchema, tool} from '@/types/core'; @@ -133,8 +134,13 @@ const stringReplaceCoreTool = tool({ }, required: ['path', 'old_str', 'new_str'], }), - // Medium risk: file write operation, requires approval except in auto-accept mode + // Medium risk: file write operation, requires approval except in auto-accept mode or if configured in nanocoderTools.alwaysAllow needsApproval: () => { + // Check if this tool is configured to always be allowed + if (isNanocoderToolAlwaysAllowed('string_replace')) { + return false; + } + const mode = getCurrentMode(); return mode !== 'auto-accept'; // true in normal/plan, false in auto-accept }, diff --git a/source/tools/write-file.tsx b/source/tools/write-file.tsx index 682d1113..cd7e8c14 100644 --- a/source/tools/write-file.tsx +++ b/source/tools/write-file.tsx @@ -6,6 +6,7 @@ import {Box, Text} from 'ink'; import React from 'react'; import ToolMessage from '@/components/tool-message'; +import {isNanocoderToolAlwaysAllowed} from '@/config/nanocoder-tools-config'; import {getCurrentMode} from '@/context/mode-context'; import {ThemeContext} from '@/hooks/useTheme'; import type {NanocoderToolExport} from '@/types/core'; @@ -69,8 +70,13 @@ const writeFileCoreTool = tool({ }, required: ['path', 'content'], }), - // Medium risk: file write operation, requires approval except in auto-accept mode + // Medium risk: file write operation, requires approval except in auto-accept mode or if configured in nanocoderTools.alwaysAllow needsApproval: () => { + // Check if this tool is configured to always be allowed + if (isNanocoderToolAlwaysAllowed('write_file')) { + return false; + } + const mode = getCurrentMode(); return mode !== 'auto-accept'; // true in normal/plan, false in auto-accept }, diff --git a/source/types/config.ts b/source/types/config.ts index e1cd63a5..6322e374 100644 --- a/source/types/config.ts +++ b/source/types/config.ts @@ -90,6 +90,14 @@ export interface AppConfig { languages: string[]; // File extensions this server handles env?: Record; }[]; + + // Tools that can run automatically in non-interactive mode + alwaysAllow?: string[]; + + // Nanocoder-specific tool configurations + nanocoderTools?: { + alwaysAllow?: string[]; + }; } export interface UserPreferences { diff --git a/source/types/mcp.ts b/source/types/mcp.ts index 4cfde466..75439b8a 100644 --- a/source/types/mcp.ts +++ b/source/types/mcp.ts @@ -28,6 +28,8 @@ export interface MCPServer { maxAttempts: number; backoffMs: number; }; + // Tools that can be executed without asking for approval + alwaysAllow?: string[]; // Common fields description?: string; diff --git a/source/wizard/templates/mcp-templates.ts b/source/wizard/templates/mcp-templates.ts index b7f6282a..c9412220 100644 --- a/source/wizard/templates/mcp-templates.ts +++ b/source/wizard/templates/mcp-templates.ts @@ -26,6 +26,7 @@ export interface McpServerConfig { timeout?: number; // Common + alwaysAllow?: string[]; description?: string; tags?: string[]; enabled?: boolean; diff --git a/source/wizard/validation-array.spec.ts b/source/wizard/validation-array.spec.ts index 62e14581..8d4ea5a1 100644 --- a/source/wizard/validation-array.spec.ts +++ b/source/wizard/validation-array.spec.ts @@ -64,6 +64,7 @@ test('buildConfigObject: preserves all server fields in array format', t => { args: ['-y', '@modelcontextprotocol/server-filesystem', '/tmp'], description: 'File system access', tags: ['files', 'local'], + alwaysAllow: ['list_directory'], }, 'remote-server': { name: 'remote-server', @@ -72,6 +73,7 @@ test('buildConfigObject: preserves all server fields in array format', t => { timeout: 30000, description: 'Remote MCP server', tags: ['remote', 'http'], + alwaysAllow: ['health_check'], }, }; @@ -96,6 +98,7 @@ test('buildConfigObject: preserves all server fields in array format', t => { ]); t.is(filesystemServer!.description, 'File system access'); t.deepEqual(filesystemServer!.tags, ['files', 'local']); + t.deepEqual(filesystemServer!.alwaysAllow, ['list_directory']); // Test remote server fields t.truthy(remoteServer); @@ -105,6 +108,7 @@ test('buildConfigObject: preserves all server fields in array format', t => { t.is(remoteServer!.timeout, 30000); t.is(remoteServer!.description, 'Remote MCP server'); t.deepEqual(remoteServer!.tags, ['remote', 'http']); + t.deepEqual(remoteServer!.alwaysAllow, ['health_check']); }); test('buildConfigObject: sets enabled flag for wizard-generated configs', t => { diff --git a/source/wizard/validation.spec.ts b/source/wizard/validation.spec.ts index f08cb7be..49d8a881 100644 --- a/source/wizard/validation.spec.ts +++ b/source/wizard/validation.spec.ts @@ -138,6 +138,56 @@ test('validateConfig: errors when MCP server missing args', t => { t.regex(result.errors[0], /missing args array/); }); +test('validateConfig: errors when alwaysAllow is not an array', t => { + const providers: ProviderConfig[] = [ + { + name: 'ollama', + baseUrl: 'http://localhost:11434', + models: ['llama2'], + }, + ]; + + const mcpServers = { + filesystem: { + name: 'filesystem', + transport: 'stdio', + command: 'npx', + args: ['-y', '@modelcontextprotocol/server-filesystem', '/tmp'], + alwaysAllow: 'not-an-array', + }, + } as unknown as Record; + + const result = validateConfig(providers, mcpServers); + + t.false(result.valid); + t.regex(result.errors[0], /alwaysAllow/); +}); + +test('validateConfig: errors when alwaysAllow contains non-strings', t => { + const providers: ProviderConfig[] = [ + { + name: 'ollama', + baseUrl: 'http://localhost:11434', + models: ['llama2'], + }, + ]; + + const mcpServers = { + filesystem: { + name: 'filesystem', + transport: 'stdio', + command: 'npx', + args: ['-y', '@modelcontextprotocol/server-filesystem', '/tmp'], + alwaysAllow: ['ok', 123], + }, + } as unknown as Record; + + const result = validateConfig(providers, mcpServers); + + t.false(result.valid); + t.regex(result.errors[0], /non-string/); +}); + test('validateConfig: accumulates multiple errors', t => { const providers: ProviderConfig[] = [ { diff --git a/source/wizard/validation.ts b/source/wizard/validation.ts index 0a9b3207..8cb01f7e 100644 --- a/source/wizard/validation.ts +++ b/source/wizard/validation.ts @@ -61,6 +61,23 @@ export function validateConfig( if (!server.args) { errors.push(`MCP server "${name}" missing args array`); } + + if (server.alwaysAllow && !Array.isArray(server.alwaysAllow)) { + errors.push( + `MCP server "${name}" has invalid alwaysAllow (must be an array of strings)`, + ); + } + + if (Array.isArray(server.alwaysAllow)) { + const invalidItems = server.alwaysAllow.filter( + item => typeof item !== 'string', + ); + if (invalidItems.length > 0) { + errors.push( + `MCP server "${name}" has non-string entries in alwaysAllow`, + ); + } + } } return { @@ -160,6 +177,7 @@ interface ConfigObject { maxAttempts: number; backoffMs: number; }; + alwaysAllow?: string[]; description?: string; tags?: string[]; enabled?: boolean; @@ -222,6 +240,7 @@ export function buildConfigObject( headers: server.headers, auth: server.auth, timeout: server.timeout, + alwaysAllow: server.alwaysAllow, description: server.description, tags: server.tags, enabled: true, // Default to enabled for wizard configurations