Skip to content

fix: allow safe key type updates for non-admins - #35132

Open
patrickswedish wants to merge 9 commits into
BerriAI:litellm_internal_stagingfrom
patrickswedish:fix_key_type_updates
Open

fix: allow safe key type updates for non-admins#35132
patrickswedish wants to merge 9 commits into
BerriAI:litellm_internal_stagingfrom
patrickswedish:fix_key_type_updates

Conversation

@patrickswedish

@patrickswedish patrickswedish commented Jul 29, 2026

Copy link
Copy Markdown

TLDR

Problem this solves:

  • Non-admin Full access key updates return 403
  • UI sends an explicit empty allowed-routes list

How it solves it:

  • Accept empty routes only with safe key presets
  • Keep management and arbitrary routes admin-only

Relevant issues

Fixes #34975

Linear ticket

Pre-Submission checklist

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • 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)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

At commit 0c7825e, the Proxy Auth & Key Management CI suite passed and Codecov confirmed that all modified, coverable lines are covered. The complete repository check set is green except the unrelated UI unit-test job, which GitHub cancelled rather than failed.

Focused local result: 16 passed, 373 deselected.

Type

🐛 Bug Fix

Changes

  • Add key_type to key update requests
  • Apply safe key presets before persistence
  • Preserve admin enforcement for management and arbitrary routes
  • Add focused safe-preset and rejection regression coverage

QA runbook

  • TestAllowedRoutesCallerPermission::test_non_admin_update_key_accepts_empty_routes_with_safe_key_type proves safe presets work for a key owner

    • Start a proxy with an admin key and a non-admin user
    • Create a personal key owned by that user
    • Call /key/update as that user with allowed_routes: [] and each of default, llm_api, and read_only
    • Expect success and the preset's derived routes to persist
    • Sanity check: direct arbitrary routes still return 403
  • TestAllowedRoutesCallerPermission::test_non_admin_update_key_rejects_empty_routes_with_management_key_type proves management access stays restricted

    • Call /key/update as the key owner with allowed_routes: [] and key_type: management
    • Expect 403 mentioning allowed_routes
    • Sanity check: an admin can still apply management routes

Final Attestation

  • The tests check the intended safe presets, restricted management access, and arbitrary-route rejection paths.

@CLAassistant

CLAassistant commented Jul 29, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed-hq

codspeed-hq Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing patrickswedish:fix_key_type_updates (e1fa451) with litellm_internal_staging (4eecf7a)

Open in CodSpeed

@patrickswedish
patrickswedish marked this pull request as ready for review July 30, 2026 19:04
@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR allows non-admin key owners to apply safe key-type presets during key updates while preserving admin-only enforcement for management and arbitrary routes

  • Adds key_type to the key-update request and generated dashboard schema
  • Derives and persists preset routes during key updates
  • Separately validates caller-supplied routes and preset-derived routes
  • Adds regression coverage for safe presets and management-route rejection

Confidence Score: 5/5

The PR appears safe to merge because preset-derived routes are validated against the non-admin allowlist before the same deterministic mapping is persisted

Explicit arbitrary routes remain subject to the existing admin check, safe presets resolve to approved route groups, the management preset remains rejected for non-admin callers, and updates omitting key_type retain their prior partial-update behavior

Important Files Changed

Filename Overview
litellm/proxy/_types.py Adds an optional key-type preset to partial key-update requests without changing requests that omit the field
litellm/proxy/management_endpoints/key_management_endpoints.py Applies key-type presets consistently during validation and persistence while limiting non-admin callers to safe route groups
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py Covers successful non-admin safe-preset updates and rejection of the management preset
ui/litellm-dashboard/src/lib/http/schema.d.ts Regenerates the dashboard API contract to expose the new optional update field

Reviews (1): Last reviewed commit: "fix: align key update schema and lint" | Re-trigger Greptile

Comment thread litellm/proxy/management_endpoints/key_management_endpoints.py Outdated
@veria-ai

veria-ai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 2 · PR risk: 0/10

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0c7825e606

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread litellm/proxy/management_endpoints/key_management_endpoints.py
Comment thread litellm/proxy/management_endpoints/key_management_endpoints.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 089e75a0f4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/litellm-dashboard/src/components/templates/key_edit_view.tsx Outdated
Comment thread litellm/proxy/management_endpoints/key_management_endpoints.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58411baf36

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread litellm/proxy/management_endpoints/key_management_endpoints.py Outdated
Comment thread litellm/proxy/management_endpoints/key_management_endpoints.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b4e0b0353

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread litellm/proxy/management_endpoints/key_management_endpoints.py
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.

[Bug]: Non-admin users can no longer change key type of existing keys on v1.92.0

2 participants