Uh oh!
There was an error while loading. Please reload this page.
feat(mcp): per-server wildcard in api_key MCP tool ACL - #709
Merged
Conversation
`allowed_tools` matched only a bare `"*"` (all tools) or exact `<server>__<tool>` names. Without a live tool list an operator can't know the exact names, so the only usable grant was all-or-nothing. Match entries as single-`*` globs instead, reusing the same `wildcard::wildcard_matches` helper `allowed_models` already uses, so `"<server>__*"` grants every tool on one server (e.g. `"github__*"` permits `github__create_issue` but not `slack__post` or `githubenterprise__create_issue`). A bare `"*"` and exact names keep their meaning; the change is purely additive (no existing grant shifts, since real tool names contain no `*`). Both matchers move together — `ApiKey::can_access_tool` and the proxy-path `ToolAcl::permits` — so their documented "mirror" relationship holds. This makes per-server tool governance reachable ahead of the live tool-list report; the control-plane api_key `allowed_tools` field + dashboard picker (which composes `<server>__*` from the org's registered MCP servers) ship as the paired AISIX-Cloud PR.
📝 WalkthroughWalkthroughTool access authorization changes from exact-match/HashSet-based checks to glob-based wildcard matching using ChangesWildcard Tool ACL
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Update the admin OpenAPI `allowed_tools` description (request + public schemas) to match the code doc and resource schema — it still carried the pre-wildcard wording, so the served contract disagreed with the shipped behavior. Tighten the schema-guard tests to assert the `<server>__*` wording so future drift fails CI. - Pin the leading/middle-`*` breadth with a test: entries are single-`*` globs anywhere (like `allowed_models`), so `"*__readonly"` is a real any-server grant of a same-named tool — documented as intentional, not an accident. - Note on `can_access_tool` that it's test-only; `ToolAcl::permits` is the live enforcement path they mirror.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
An API key's
allowed_tools(per-tool MCP ACL, #668) matched only a bare"*"(every tool) or exact<server>__<tool>names. Since the discovered tool list isn't surfaced anywhere yet, an operator can't know the exact names — so the only usable grant today is all-or-nothing ("*").This adds a per-server wildcard: entries are now matched as single-
*globs (reusing the very samewildcard::wildcard_matcheshelper thatallowed_modelsalready uses), so"<server>__*"grants every tool on one server:"github__*"permitsgithub__create_issue,github__delete_reposlack__post_messageand the prefix-sharinggithubenterprise__create_issueare both denied.A bare
"*"and exact names keep their exact meaning. The change is purely additive — no existing grant shifts, because real composed tool names contain no*.Why this shape
allowed_modelsalready globsopenai/*; tools now globgithub__*through the identical helper, so the two ACLs stay semantically consistent and there's one source of truth.ApiKey::can_access_tool(core) and the proxy-pathToolAcl::permits(aisix-mcp) both delegate towildcard_matches, preserving their documented "mirror" relationship.ToolAcl::from_allowedstill folds a bare"*"intoAllowAll; a<server>__*becomes a scopedAllow.Tests
can_access_tool_enforces_namespaced_allowlistextended:github__*grants github tools, denies a different server and a prefix-sharing server name.tool_acl_from_allowed_semantics: a per-server wildcard is a scopedAllow, notAllowAll.tool_acl_per_server_wildcard_scopes_to_one_server(new, full gateway):alpha__*over two real upstreams exposes only alpha's tools intools/list, calls alpha, and rejects beta (defense-in-depth) — exercisespermitsend-to-end.cargo fmt/clippy -D warnings/aisix-core+aisix-mcpsuites green;dump-schemaregeneratedapi_key.schema.json(description only) and is idempotent.Control-plane pairing
This exposes a richer matching semantics on an existing field; the field itself still needs to become reachable — the CP api_key resource has no
allowed_toolstoday. The paired AISIX-Cloud PR addsallowed_toolsto the api_key model/validation/projection + a dashboard tool-ACL picker that composes<server>__*grants from the org's registered MCP servers (plus a global-*toggle and an exact-name escape hatch). Refs AISIX-Cloud#894 (Phase 2, per-key tool ACL). The live individual-tool list (via a DP→CP report) is a separate fast-follow.Summary by CodeRabbit
New Features
<server>__*.Bug Fixes
Documentation
Independent audit response
A cold audit returned FIX-FIRST (no BLOCK): it traced the matcher character-by-character and confirmed
github__*matches github tools while correctly rejecting the prefix-sharinggithubenterprise__*, no privilege-widening for existing data, and thattools/listfilter +tools/callreject both go through the samepermits. Two MEDIUMs, both fixed ineb8ff95:allowed_toolsdescriptions inopenapi.rs(request + public schemas) still carried the pre-wildcard wording, soGET /admin/openapi.jsondisagreed with the shipped behavior. Updated both to match the code doc andapi_key.schema.json; the schema-guard tests now assert the<server>__*wording so future drift fails CI. (No separate generated artifact exists —openapi.rsis the source of truth; verified the generator propagates the new wording.)*breadth untested. Entries are single-*globs anywhere, mirroringallowed_models— so"*__readonly"is a genuine any-server grant of a same-named tool. Pinned with a test (asserting cross-server match + suffix anchoring) and kept the "single-*globs, mirroringallowed_models" framing that states the general rule. This is intentional and consistent with the model ACL, not an accident.LOWs: added a one-line note that
can_access_toolis test-only (the live path isToolAcl::permits); the literal-*-in-name anddebug_assert!server-name notes are benign/pre-existing (no real grant flips, since composed tool names contain no*).