auth_policy: add tool exclusion support to policy rules#523
Conversation
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| if denied_tool == tool_name: | ||
| return False | ||
|
|
||
| for allowed_tool in (t for t in self.tools if not t.startswith("-")): |
There was a problem hiding this comment.
The the tool exclusion logic looks solid but I want to understand if this matches a specific requirement or if its general infrastructure
There was a problem hiding this comment.
It's something I had in my early notes, and thought it had been implemented, so documented it. And decided to leave it in the documentation when I realized that it actually hadn't been implemented, and do it later.
The reason I decided to make excluding groups disallowed is it seemed to me that ['*', "-@run_script"] was just a confusing way to write ["@fixed"]
But actually, there's a worse problem now ['*', "-@run_script"] is just a confusing way to write ['*']. Which is a trap. We should either make it work or add validation to catch it. Probably just making it work is easiest.
There was a problem hiding this comment.
Should we add validation to reject -@ patterns with an error or actually handle excluding tool groups?
There was a problem hiding this comment.
I went ahead and implemented excluding tool groups to remove a corner case. Then I went ahead and implemented basic validation to avoid people using -get_* or something like that...
[I didn't validate that tool names and group names actually existed - to keep things simple and because that makes it more robust if your config file slightly mismatches your MCP server version ... but perhaps that would be a good idea as well. Not sure.]
owtaylor
force-pushed
the
auth-policy-tool-exclusion
branch
from
3f88869 to
0b6d5aa
Compare
Tools or toolsets prefixed with `-` in a rule's tools list now exclude that tool or toolset from matching. Exclusions take precedence over inclusions regardless of order. This makes the code match what has already been documented, except that the toolset exclusion is added compared to the documentation (which is updated for that.) Entries in the rule tool list are now syntactically validated (but not checked whether tools/toolsets actually exist.) Assisted-By: Claude Opus 4.6 <noreply@anthropic.com>
owtaylor
force-pushed
the
auth-policy-tool-exclusion
branch
from
0b6d5aa to
d449b4e
Compare
panyamkeerthana
left a comment
panyamkeerthana
left a comment
There was a problem hiding this comment.
Perfect! The toolset exclusion support fixes the security trap I was worried about. Love the validation that rejects patterns like get_* - makes it fail-fast instead of silently doing nothing.
The refactored matches() helper is clean and the test coverage is comprehensive. Ready to merge. 🚀
2 participants
Tools prefixed with
-in a rule's tools list now exclude that tool from matching. Exclusions take precedence over inclusions regardless of order. This makes the code match what has already been documented.Assisted-By: Claude Opus 4.6 noreply@anthropic.com