Skip to content

fix(key_management): allow /key/update to keep or shrink MCP server grants the key already holds - #38463

Merged
yassin-berriai merged 2 commits into
litellm_internal_stagingfrom
litellm_fix_key_update_mcp_grandfather
Aug 27, 2026
Merged

yassin-berriai merged 2 commits into
litellm_internal_stagingfrom
litellm_fix_key_update_mcp_grandfather

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Team keys holding MCP servers outside the team allowlist 403 on every /key/update
  • Unrelated edits (budget, models) and partial grant removals were blocked

How it solves it:

  • /key/update now treats the key's already-stored MCP grants as valid
  • Keeping or shrinking existing grants passes; adding new out-of-team servers still 403s
  • Grants are not carried over when the update moves the key to another team

User Flow

Before: a proxy admin cannot edit a team key whose MCP servers are no longer in the team allowlist

  1. They send POST https://litellm-domain/key/update with the key and its current object_permission.mcp_servers list unchanged
  2. The response is 403 "Key requests MCP servers not allowed by team ... Team allows: []" even though they added nothing
  3. Removing just one of the servers from the list also comes back 403
  4. The only way to save any edit is to strip every out-of-allowlist server from the key first

After: the same admin can keep or shrink those grants, and only genuinely new servers are checked

  1. They send the same POST https://litellm-domain/key/update with the unchanged mcp_servers list and get 200 with the updated key object
  2. Sending the list with one server removed also returns 200
  3. Adding a server that is neither held by the key nor allowed by the team still returns 403 naming only that server
  4. A key holding out-of-allowlist servers still cannot gain any additional server another team member could not grant

Relevant issues

Linear ticket

Resolves LIT-6062

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

Setup shared by both runs: local proxy on http://localhost:4000 backed by Postgres, two DB-registered MCP servers 8931af5f-8628-4058-a6e3-2fa6df234b90 (A) and 1bc10590-6d87-4375-8dd4-ef92e8cd8a05 (B), a team c92919d0-6551-4525-af2f-6124493751d2 whose allowlist was emptied after the key was created, and a team key repro-key-6062 whose object_permission still holds A and B. $K is that key, $SA/$SB are the server ids

Before (3b50819)

Re-send the key's own unchanged grants

  1. curl -X POST http://localhost:4000/key/update -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d "{\"key\":\"$K\",\"object_permission\":{\"mcp_servers\":[\"$SA\",\"$SB\"]}}"
  2. HTTP 403: {"error":{"message":"{'error': \"Key requests MCP servers not allowed by team 'c92919d0-6551-4525-af2f-6124493751d2': ['1bc10590-6d87-4375-8dd4-ef92e8cd8a05', '8931af5f-8628-4058-a6e3-2fa6df234b90']. Team allows: []. Global (allow_all_keys) servers: [].\"}","type":"auth_error","code":"403"}}

Shrink to one already-held server

  1. Same call with \"mcp_servers\":[\"$SA\"]
  2. HTTP 403 with the same "not allowed by team" error

Control: unrelated edit with no object_permission

  1. Same call with {"key":"$K","max_budget":25} and no object_permission
  2. HTTP 200 with the updated key object, confirming the proxy and key are otherwise healthy

After (ede05eb)

Re-send the key's own unchanged grants

  1. curl -X POST http://localhost:4000/key/update -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d "{\"key\":\"$K\",\"object_permission\":{\"mcp_servers\":[\"$SA\",\"$SB\"]}}"
  2. HTTP 200, response includes "key_alias": "repro-key-6062" and the full key object

Adding a genuinely new out-of-team server still fails

  1. Registered a third server C (0d53fcae-a01e-4b07-81bf-f8b1b6f21675), not held by the key and not in the team allowlist, then same call with \"mcp_servers\":[\"$SA\",\"$SB\",\"$SC\"]
  2. HTTP 403 naming only C: Key requests MCP servers not allowed by team 'c92919d0-6551-4525-af2f-6124493751d2': ['0d53fcae-a01e-4b07-81bf-f8b1b6f21675']. Team allows: []. Global (allow_all_keys) servers: [].

Shrink to one already-held server

  1. Same call with \"mcp_servers\":[\"$SA\"]
  2. HTTP 200, key object returned with the shrunk grant saved

Type

🐛 Bug Fix

Caveats (if any)

Medium

  • Only the same-team update path grandfathers; moving the key to another team revalidates strictly
  • Sibling stale-grant behavior for access groups, toolsets, search tools, and vector stores is unchanged and out of scope here

Low

  • /key/regenerate keeps its existing strict validation; a follow-up could reuse the same mechanism
  • The key lookup on /key/update now includes the object_permission relation instead of issuing a second query during validation

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Link to Devin session: https://app.devin.ai/sessions/b8fa5b91b8fc4c6a93aef58d1b46fb99
Open in Devin Desktop: https://app.devin.ai/desktop/session/b8fa5b91b8fc4c6a93aef58d1b46fb99?variant=devin
Requested by: @yassin-berriai

…rants the key already holds

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

No action taken on #38463 — it currently has zero labels, so the required enterprise gate isn't met. No GitHub or Linear changes made.

@codecov

codecov Bot commented Aug 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR allows same-team key updates to retain or reduce existing MCP server grants without permitting new grants outside the team allowlist.

  • Loads the key’s object-permission relation with the existing key query.
  • Reuses that relation to identify grandfathered MCP server grants.
  • Preserves strict validation when moving a key to another team.
  • Adds coverage for unchanged, reduced, new, tool-scoped, and sentinel grants.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported redundant object-permission lookup has been removed by reusing the relation included in the existing-key query.

Important Files Changed

Filename Overview
litellm/proxy/management_endpoints/key_management_endpoints.py Includes the object-permission relation in the existing-key lookup and passes it to validation only when the team remains unchanged.
litellm/proxy/management_helpers/object_permission_utils.py Resolves grandfathered MCP server identifiers from the supplied relation without repeating the object-permission database lookup.
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py Verifies relation loading and same-team propagation into MCP validation.
tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py Covers retention, shrinking, rejection of new grants, strict validation without an existing permission, tool permissions, and sentinels.

Reviews (2): Last reviewed commit: "fix(key_management): reuse key row's inc..." | Re-trigger Greptile

Comment thread litellm/proxy/management_helpers/object_permission_utils.py Outdated
…ad of a second lookup

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review: the key update now reuses the included object_permission relation instead of a second database lookup

@codspeed

codspeed Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_fix_key_update_mcp_grandfather (ede05eb) with litellm_internal_staging (3b50819)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (1666949) during the generation of this report, so 3b50819 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants