-
Notifications
You must be signed in to change notification settings - Fork 116
[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?
Conversation
|
just saw it's a draft, sowwee :D |
still helpful! Thank You |
0da7dec to
c68e51f
Compare
|
I reimplement/rebased given the many changes since I last started this draft pr. |
Looks great! I have some time Boxing Day morning tomorrow to sort out outstanding pull requests so will take a look then 😄 Merry Christmas 🎄🎄 |
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.
Pull request overview
This PR adds configuration-based auto-approval for MCP server tools and nanocoder tools, along with improved non-interactive mode behavior. Users can configure which tools should automatically execute without requiring user confirmation through alwaysAllow configuration arrays.
Key changes:
- Added
alwaysAllowconfiguration field to MCP server configurations andnanocoderToolsconfiguration - Modified tool approval logic in nanocoder tools (
write_file,string_replace,execute_bash) and MCP client to checkalwaysAllowlists - Introduced non-interactive mode early-exit behavior when non-approved tools are encountered (though implementation has critical bugs)
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| source/wizard/validation.ts | Added validation for MCP server alwaysAllow field to ensure it's an array of strings |
| source/wizard/validation.spec.ts | Added tests for alwaysAllow validation (non-array and non-string items) |
| source/wizard/validation-array.spec.ts | Added test data with alwaysAllow examples |
| source/wizard/templates/mcp-templates.ts | Added alwaysAllow field to MCP server config interface |
| source/types/mcp.ts | Added alwaysAllow field with documentation to MCPServer type |
| source/types/config.ts | Added alwaysAllow and nanocoderTools.alwaysAllow fields to AppConfig |
| source/tools/write-file.tsx | Modified needsApproval to check nanocoder tools config |
| source/tools/string-replace.tsx | Modified needsApproval to check nanocoder tools config |
| source/tools/execute-bash.tsx | Modified needsApproval from boolean to function that checks config |
| source/mcp/mcp-client.ts | Added isToolAutoApproved method and integrated it into tool approval logic |
| source/mcp/mcp-client.spec.ts | Added tests for auto-approval behavior |
| source/hooks/chat-handler/conversation/conversation-loop.tsx | Added non-interactive mode allow-list logic (contains critical bugs) |
| source/config/nanocoder-tools-config.ts | New helper function to check if nanocoder tools are auto-approved |
| source/config/index.ts | Added loading of alwaysAllow and nanocoderTools configuration fields |
| source/commands/mcp.tsx | Added display of auto-approved tools in MCP command output |
| docs/mcp-configuration.md | Added documentation for alwaysAllow field |
| agents.config.example.json | Added example configurations with alwaysAllow fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hey @namar0x0309 - we have done an awful lot of work on the main branch, I think we're ready to proceed with plans we spoke about before the big winter clean up including this :D |
… tools Add support for configuring automatic approval of specific MCP tools through the `alwaysAllow` property in server configurations. This allows certain trusted tools to execute without requiring user confirmation prompts. - Update configuration schema to include `alwaysAllow` arrays - Modify MCP client to respect auto-approval settings - Add documentation for the new configuration option - Include UI indication of auto-approved tools - Add test coverage for approval behavior
c68e51f to
fb17e1c
Compare
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.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ] | ||
| ], | ||
| "nanocoderTools": { | ||
| "alwaysAllow": ["execute_bash"] |
Copilot
AI
Jan 8, 2026
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_bash in the alwaysAllow configuration is a high-risk security practice. While this is just an example file, consider:
- Using a less risky tool as the example (like
write_fileorstring_replace) - Adding a comment warning about the security implications of auto-allowing bash execution
- Documenting that users should carefully consider which tools to auto-approve
This would better demonstrate secure configuration practices and prevent users from blindly copying a potentially dangerous setting.
| "alwaysAllow": ["execute_bash"] | |
| "_comment": "Example configuration: auto-approving powerful tools (especially shell execution like `execute_bash`) can be dangerous. Carefully review which tools, if any, you add to alwaysAllow in your real configuration.", | |
| "alwaysAllow": ["write_file"] |
| 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`, | ||
| ); | ||
| } | ||
| } | ||
| } |
Copilot
AI
Jan 8, 2026
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.
The validation only checks alwaysAllow for MCP servers but does not validate the top-level alwaysAllow or nanocoderTools.alwaysAllow configurations. Consider adding validation to ensure:
- Both fields are arrays of strings (if present)
- The tool names in these arrays correspond to actual available tools
- Users receive helpful error messages if they configure non-existent tool names
This would improve the user experience and catch configuration errors early.
| 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 | ||
| }, |
Copilot
AI
Jan 8, 2026
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.
The changes add isNanocoderToolAlwaysAllowed checks to bypass approval, but there are no tests verifying this new functionality. Consider adding tests to source/tools/needs-approval.spec.ts or a dedicated test file that verify:
- Tools in
nanocoderTools.alwaysAllowskip approval even in normal mode - Tools not in the list still require approval as expected
- Invalid configuration (non-array, non-strings) is handled gracefully
This would ensure the security-critical auto-approval feature works correctly across different scenarios.
| ], | ||
| "nanocoderTools": { | ||
| "alwaysAllow": ["execute_bash"] | ||
| } |
Copilot
AI
Jan 8, 2026
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.
The new top-level alwaysAllow field lacks documentation. This configuration is used specifically for non-interactive mode (--run flag) tool execution and serves a different purpose than nanocoderTools.alwaysAllow. Consider:
- Adding a comment in the example configuration explaining what this field does
- Creating or updating documentation to explain when to use top-level
alwaysAllowvsnanocoderTools.alwaysAllow - Clarifying that this is specifically for non-interactive mode tool approval
Without clear documentation, users may be confused about:
- Why there are two similar
alwaysAllowconfigurations - When to use each one
- How they interact with each other
will-lamerton
left a comment
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.
Hey @namar0x0309 - looking great from my point of view! Couple points:
- Are you able to address the co-pilot review points and ensure all test files related to changes are updated?
- Are you able to document functionality for users in the README?
This is a great addition :D
Thanks! Tackling this today! |
Description
This PR adds configuration-based auto-approval for tools and improves non-interactive mode behavior. Users can now specify which MCP server tools and nanocoder tools should automatically run without requiring confirmation, streamlining trusted workflows while maintaining security.
Key Features:
Per-tool auto-approval via alwaysAllow configuration for both MCP servers and nanocoder tools
Non-interactive mode early-exit when tools requiring approval are encountered
Graceful error handling with clear messaging when approval would be required
Implementation Details
• Added alwaysAllow field to MCP server configuration schema (array of tool names)
• Added alwaysAllow field to nanocoderTools configuration (array of nanocoder tool names)
• Modified tool approval logic in conversation loop to check alwaysAllow lists before requiring confirmation
• Implemented non-interactive mode detection via --run flag
• Added early-exit behavior in non-interactive mode when non-approved tools are encountered
• Updated configuration validation to ensure alwaysAllow is an array of strings
• Configuration examples added to agents.config.example.json
Security Considerations
• Opt-in by design: No tools are auto-approved by default
• Users must explicitly list tool names in alwaysAllow arrays
• High-risk operations can be excluded from auto-approval lists
• Existing approval mechanisms remain unchanged for non-configured tools
• Clear documentation on security implications in MCP configuration guide
• Bash tool respects alwaysAllow configuration but users can choose to exclude it
Breaking Changes
• None - this is an additive feature that maintains backward compatibility
• All new configuration fields (alwaysAllow) are optional
• Existing configurations continue to work without modifications
Testing
• Added unit tests for alwaysAllow configuration validation
• Tests verify array validation (must be array of strings)
• Tests cover non-interactive mode early-exit scenarios
• Verified backward compatibility with existing configurations
• Manual testing with filesystem and GitHub MCP servers
• Tested both approved and non-approved tool scenarios
Documentation Updates
• Updated mcp-configuration.md with alwaysAllow field documentation
• Added configuration examples for common use cases
• Updated agents.config.example.json with example alwaysAllow configurations for MCP servers
• Added nanocoderTools.alwaysAllow example configuration
• Configuration validation updated to handle new optional fields
This feature significantly improves the developer experience for trusted and automated workflows (CI/CD, local development with trusted tools) while maintaining nanocoder's security-first approach.
Type of Change
Testing
Automated Tests
.spec.ts/tsxfilespnpm test:allcompletes successfully)Manual Testing
Checklist