mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
feat(mcp): opt-in least-privilege default for team key MCP access (#31380)
* feat(mcp): add require_key_mcp_access_defined to stop keys inheriting team MCP servers By default a virtual key that grants no MCP servers of its own inherits its team's full MCP server list. The new general_settings flag require_key_mcp_access_defined (default false) flips this so the team list acts purely as a ceiling: a key reaches only the servers it grants explicitly (or via an access group), and inherits none. This mirrors the existing require_end_user_mcp_access_defined setting. The default is unchanged, so existing deployments keep today's behavior until they opt in. The no-mcp-servers sentinel and key access-group grants are unaffected. * docs(mcp): note require_key_mcp_access_defined effect in resolver docstring
This commit is contained in:
parent
bdafc9a008
commit
f16af8853b
2 changed files with 111 additions and 2 deletions
|
|
@ -624,7 +624,9 @@ class MCPRequestHandler:
|
|||
|
||||
Permission hierarchy (all rules are intersections):
|
||||
1. Get allowed servers from key permissions
|
||||
2. Get allowed servers from team permissions (key inherits from team, or intersection)
|
||||
2. Get allowed servers from team permissions (key inherits from team, or
|
||||
intersection; or inherits nothing when require_key_mcp_access_defined
|
||||
is enabled, making the team a ceiling rather than a default)
|
||||
3. Get allowed servers from end_user permissions (intersected if set)
|
||||
4. Get allowed servers from agent permissions (intersected if set)
|
||||
5. Get allowed servers from org permissions — org acts as a ceiling: if the org
|
||||
|
|
@ -677,7 +679,16 @@ class MCPRequestHandler:
|
|||
if not team_set:
|
||||
base = key_set # no team restriction
|
||||
elif not key_set:
|
||||
base = team_set # key has no own perms → inherits team
|
||||
# A key that grants no MCP servers of its own inherits the
|
||||
# team's by default. With require_key_mcp_access_defined the
|
||||
# team is a ceiling rather than a default, so the key must
|
||||
# grant servers explicitly (or via an access group) to reach
|
||||
# any — it inherits none.
|
||||
base = (
|
||||
set()
|
||||
if general_settings.get("require_key_mcp_access_defined", False)
|
||||
else team_set
|
||||
)
|
||||
else:
|
||||
base = key_set & team_set # both restrict → intersect
|
||||
|
||||
|
|
|
|||
|
|
@ -170,6 +170,104 @@ class TestMCPRequestHandler:
|
|||
mock_key_servers.assert_called_once_with(user_api_key_auth)
|
||||
mock_team_servers.assert_called_once_with(user_api_key_auth)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"require_key_mcp_access_defined,expected",
|
||||
[
|
||||
# Default (flag off): a key with no MCP scope of its own inherits
|
||||
# the team's servers.
|
||||
(False, ["team_server1", "team_server2"]),
|
||||
# Flag on: the team is a ceiling, not a default — the key inherits
|
||||
# nothing and must grant servers explicitly.
|
||||
(True, []),
|
||||
],
|
||||
)
|
||||
async def test_require_key_mcp_access_defined_gates_team_inheritance(
|
||||
self, require_key_mcp_access_defined, expected
|
||||
):
|
||||
"""The require_key_mcp_access_defined general setting flips an empty key
|
||||
from inheriting its team's MCP servers (default) to inheriting none."""
|
||||
auth = UserAPIKeyAuth(
|
||||
api_key="test-key", user_id="test-user", team_id="test-team"
|
||||
)
|
||||
with (
|
||||
patch.object(
|
||||
MCPRequestHandler,
|
||||
"_get_allowed_mcp_servers_for_key",
|
||||
new_callable=AsyncMock,
|
||||
return_value=[],
|
||||
),
|
||||
patch.object(
|
||||
MCPRequestHandler,
|
||||
"_get_allowed_mcp_servers_for_team",
|
||||
new_callable=AsyncMock,
|
||||
return_value=["team_server1", "team_server2"],
|
||||
),
|
||||
patch.object(
|
||||
MCPRequestHandler,
|
||||
"_get_key_access_group_mcp_server_extras",
|
||||
new_callable=AsyncMock,
|
||||
return_value=[],
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
{"require_key_mcp_access_defined": require_key_mcp_access_defined},
|
||||
),
|
||||
):
|
||||
result = await MCPRequestHandler.get_allowed_mcp_servers(auth)
|
||||
|
||||
assert sorted(result) == sorted(expected)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"key_servers,grants,expected,scenario",
|
||||
[
|
||||
# Explicit key subset is still honored under the flag (intersected
|
||||
# with the team ceiling) — the flag only removes empty-key inheritance.
|
||||
(["team_server1"], [], ["team_server1"], "explicit_subset_survives"),
|
||||
# An access-group grant is the escape hatch: it surfaces even though
|
||||
# the key inherits nothing from the team.
|
||||
([], ["granted_server"], ["granted_server"], "access_group_grant_survives"),
|
||||
],
|
||||
)
|
||||
async def test_require_key_mcp_access_defined_preserves_explicit_grants(
|
||||
self, key_servers, grants, expected, scenario
|
||||
):
|
||||
"""With require_key_mcp_access_defined on, a key still reaches servers it
|
||||
grants explicitly or via an access group — only blanket team inheritance
|
||||
is removed."""
|
||||
auth = UserAPIKeyAuth(
|
||||
api_key="test-key",
|
||||
user_id="test-user",
|
||||
team_id="test-team",
|
||||
access_group_ids=["grp"] if grants else [],
|
||||
)
|
||||
with (
|
||||
patch.object(
|
||||
MCPRequestHandler,
|
||||
"_get_allowed_mcp_servers_for_key",
|
||||
new_callable=AsyncMock,
|
||||
return_value=key_servers,
|
||||
),
|
||||
patch.object(
|
||||
MCPRequestHandler,
|
||||
"_get_allowed_mcp_servers_for_team",
|
||||
new_callable=AsyncMock,
|
||||
return_value=["team_server1", "team_server2"],
|
||||
),
|
||||
patch.object(
|
||||
MCPRequestHandler,
|
||||
"_get_key_access_group_mcp_server_extras",
|
||||
new_callable=AsyncMock,
|
||||
return_value=grants,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
{"require_key_mcp_access_defined": True},
|
||||
),
|
||||
):
|
||||
result = await MCPRequestHandler.get_allowed_mcp_servers(auth)
|
||||
|
||||
assert sorted(result) == sorted(expected)
|
||||
|
||||
@pytest.mark.parametrize("team_servers", [[], ["team_server1", "team_server2"]])
|
||||
async def test_no_mcp_servers_sentinel_returns_empty(self, team_servers):
|
||||
"""A key scoped to the no-mcp-servers sentinel resolves to zero servers,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue