-
Notifications
You must be signed in to change notification settings - Fork 117
[Feature][config] per-tool auto-approve settings + nonInteractiveMode early-exit #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,16 +44,21 @@ | |
| "args": [ | ||
| "@modelcontextprotocol/server-filesystem", | ||
| "/path/to/allowed/directory" | ||
| ] | ||
| ], | ||
| "alwaysAllow": ["list_directory", "file_info"] | ||
| }, | ||
| { | ||
| "name": "github", | ||
| "command": "npx", | ||
| "args": ["@modelcontextprotocol/server-github"], | ||
| "env": { | ||
| "GITHUB_TOKEN": "your-github-token" | ||
| } | ||
| }, | ||
| "alwaysAllow": ["list_repositories"] | ||
| } | ||
| ] | ||
| ], | ||
| "nanocoderTools": { | ||
| "alwaysAllow": ["execute_bash"] | ||
| } | ||
|
Comment on lines
+59
to
+62
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| } | ||
namar0x0309 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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 | ||||||||||||
will-lamerton marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
| validationFailed = true; | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
@@ -329,15 +328,8 @@ export const processAssistantResponse = async ( | |||||||||||
| args: unknown, | ||||||||||||
| ) => boolean | Promise<boolean> | ||||||||||||
| )(parsedArgs); | ||||||||||||
| } catch (error) { | ||||||||||||
| const logger = getLogger(); | ||||||||||||
| logger.debug( | ||||||||||||
| 'needsApproval evaluation failed, requiring approval', | ||||||||||||
| { | ||||||||||||
| toolName: toolCall.function.name, | ||||||||||||
| error, | ||||||||||||
| }, | ||||||||||||
| ); | ||||||||||||
| } catch { | ||||||||||||
will-lamerton marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
| // 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) { | ||||||||||||
|
||||||||||||
| if (shouldExecuteDirectly) { | |
| if (shouldExecuteDirectly) { | |
| toolsToExecuteDirectly.push(toolCall); | |
| } else { | |
| toolsNeedingConfirmation.push(toolCall); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
execute_bashin thealwaysAllowconfiguration is a high-risk security practice. While this is just an example file, consider:write_fileorstring_replace)This would better demonstrate secure configuration practices and prevent users from blindly copying a potentially dangerous setting.