Python: [Generated by SRE Agent] Fix MCP allowed_tools empty list handling#6296
Open
chetantoshniwal wants to merge 2 commits into
Open
Python: [Generated by SRE Agent] Fix MCP allowed_tools empty list handling#6296chetantoshniwal wants to merge 2 commits into
chetantoshniwal wants to merge 2 commits into
Conversation
When allowed_tools is set to an empty list [], the falsy check 'if not self.allowed_tools' incorrectly treats it as unconfigured (same as None), causing all tools to be exposed. Change to an explicit 'is None' check so that an empty list correctly results in no tools being allowed. Co-authored-by: Azure SRE Agent <noreply@microsoft.com>
chetantoshniwal
changed the title
github-actions
Bot
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a security/behavioral bug in the Python MCP tool filtering logic where allowed_tools=[] (explicitly “allow none”) was treated the same as allowed_tools=None (“allow all”), unintentionally exposing all MCP tools.
Changes:
- Update MCP tool filtering to treat only
Noneas “unconfigured” (allow all), while[]correctly means “allow none”. - Add a test case ensuring an empty
allowed_toolslist results in zero exposed tools.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_mcp.py | Fixes the allowed_tools check so [] no longer falls into the “allow all tools” path. |
| python/packages/core/tests/core/test_mcp.py | Adds coverage to assert that allowed_tools=[] exposes no tools via functions. |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
giles17
approved these changes
Jun 3, 2026
| def functions(self) -> list[FunctionTool]: | ||
| """Get the list of functions that are allowed.""" | ||
| if not self.allowed_tools: | ||
| if self.allowed_tools is None: |
Member
There was a problem hiding this comment.
the docstring doesn't actually specify either way, what a empty list means, so let's clarify the docstring for the allowed_tools param to ensure this change is clear.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
allowed_toolsis set to an empty list[], the existing falsy check (if not self.allowed_tools) incorrectly treats it as unconfigured (same asNone), causing all MCP tools to be exposed instead of none.Changes
_mcp.py: Changedif not self.allowed_tools:toif self.allowed_tools is None:so that an empty list correctly results in no tools being available.test_mcp.py: Added test case for empty list[]to the existing parametrizedtest_mcp_tool_allowed_toolstest.Behavior
allowed_toolsvalueNone[]["tool_a"]