[Fix] Key Alias Re-validation on Update Blocks Legacy Aliases

[yuneng-jiang]

Relevant issues

User report: keys created with @ in key_alias (e.g. user@domain.com) before stricter validation rules cannot be edited in the Admin UI, even when the alias itself isn't being changed. This is to fix a breaking change.

Summary

Failure Path (Before Fix)

When updating or regenerating a virtual key, the backend re-validates the existing key_alias against current format rules — even when the alias hasn't changed. Keys created before @ was added to the allowed regex (or before validation existed) become uneditable because the unchanged alias fails the newer validation.

Fix

Skip _validate_key_alias_format() on the update and regenerate paths when the submitted key_alias matches the existing value in the database. Validation still runs when the alias is actually being changed.

Testing

• 4 new unit tests in TestKeyAliasSkipValidationOnUnchanged:
  • Unchanged legacy alias skips validation (even under stricter regex)
  • Changed alias to invalid value still rejected
  • Changed alias to valid value passes
  • None alias always skips validation
• All 7 existing + new alias validation tests pass

Type

🐛 Bug Fix
✅ Test

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
litellm	Ready	Preview, Comment	Mar 17, 2026 1:10am

[greptile-apps]

Greptile Summary

This PR fixes a breaking change where keys created before enable_key_alias_format_validation existed (or before a specific character was added to the allowed regex) could not be edited in the Admin UI because the backend re-validated an unchanged key_alias against current format rules. The fix correctly guards _validate_key_alias_format() with an equality check against the existing DB value in both update_key_fn and _execute_virtual_key_regeneration, applying the skip only when the submitted alias is identical to the stored alias.

Key observations:

• The production-code change is minimal, symmetric (both update paths fixed), and logically sound. Edge cases (None alias, clearing an alias, changing to invalid/valid alias) all behave correctly.
• _validate_key_alias_format() already handles None by returning early, so the new guard does not introduce any new null-reference risk.
• The four new unit tests verify the skip behaviour but test the if/else conditional inline instead of calling update_key_fn or _execute_virtual_key_regeneration directly. This means a future refactor that accidentally removes the guard in the actual functions would not be caught by these tests.
• mock_prisma is accepted as a fixture parameter in three of the four new tests but is never referenced in their bodies.
• Three of the four new tests use @pytest.mark.asyncio despite containing no await calls, making them effectively synchronous tests with an unnecessary decorator.

Confidence Score: 4/5

• Safe to merge — the production-code change is small, correct, and does not introduce regressions; test quality is the main concern.
• The two-line guard added in each endpoint function is logically correct and handles all identified edge cases. The fix is gated behind the existing enable_key_alias_format_validation flag (default False), limiting blast radius. The only weakness is that the new tests exercise the conditional logic inline rather than calling the real endpoint functions, which leaves a thin regression-detection gap.
• The test file deserves another look: the four new tests provide weaker regression protection than they appear to, and the unused mock_prisma fixture references should be cleaned up.

Important Files Changed

Filename	Overview
litellm/proxy/management_endpoints/key_management_endpoints.py	Skips _validate_key_alias_format() on both the update and regenerate paths when the submitted key_alias equals the existing value in the DB — correctly fixing the legacy-alias edit-block bug with no logic regressions identified.
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py	Adds 4 new tests verifying skip-validation logic, but they replicate the if/else conditional inline rather than calling the actual endpoint functions, leaving a test coverage gap; mock_prisma is declared but never used in 3 of the 4 tests, and 3 tests use @pytest.mark.asyncio without any awaits.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["update_key_fn / _execute_virtual_key_regeneration\ncalled with update payload"] --> B["prepare_key_update_data()\nnon_default_values = payload fields (exclude_unset=True)"]
    B --> C{"new_key_alias\n== existing_key_row.key_alias?"}
    C -- "Yes (unchanged alias)" --> D["⏭ Skip _validate_key_alias_format()\nLegacy alias preserved"]
    C -- "No (alias changed or absent)" --> E["_validate_key_alias_format(new_key_alias)"]
    E -- "None → early return\nValid format → pass" --> F["_enforce_unique_key_alias()"]
    E -- "Invalid format &\nvalidation enabled" --> G["❌ ProxyException 400"]
    D --> F
    F --> H["Update DB record"]

Loading

_{Last reviewed commit: 4a92db8}

[greptile-apps]

mock_prisma fixture parameter is unused in tests 1–3

The mock_prisma fixture is requested as a parameter in test_update_key_unchanged_legacy_alias_passes, test_update_key_changed_alias_still_validated, and test_update_key_changed_to_valid_alias_passes, but its value is never referenced inside any of those test bodies. Since none of the tests actually call update_key_fn or interact with the DB, the fixture is dead weight here. If the intent is to guard against real DB calls, the fixture should be used; if there is no such guard needed, the parameter can be removed.

[greptile-apps]

@pytest.mark.asyncio on effectively synchronous tests

test_update_key_changed_alias_still_validated, test_update_key_changed_to_valid_alias_passes, and test_update_key_alias_none_skips_validation are all decorated with @pytest.mark.asyncio but contain no await expressions — they are effectively synchronous. Depending on the version of pytest-asyncio in use, this can produce warnings or unexpected behavior in strict mode. These three tests should either be plain def functions (removing async def and the decorator) or actually exercise async code paths.

[greptile-apps]

None from missing key vs. explicit None are indistinguishable here

non_default_values.get("key_alias", None) returns None both when the client omits key_alias from the payload (field not present after exclude_unset=True) AND when the client explicitly sends "key_alias": null to clear the alias.

If the existing key has a non-None alias (e.g. "user@domain.com") and the client sends a payload without key_alias at all, this evaluates to None != "user@domain.com" → True, and _validate_key_alias_format(None) is called. _validate_key_alias_format handles None safely (returns early), so there is no runtime error. However, the semantics are slightly misleading: the condition fires as though the alias "changed to None" even when it wasn't touched.

This is a pre-existing ambiguity and not a regression introduced by this PR, but it is worth documenting with a comment so future maintainers understand the intent.

[greptile-apps]

Tests replicate logic inline instead of calling the actual endpoints

The new tests in TestKeyAliasSkipValidationOnUnchanged manually copy-paste the if new_alias != existing_alias: _validate_key_alias_format(new_alias) conditional inline rather than calling update_key_fn or _execute_virtual_key_regeneration directly. This means these tests will not catch a regression if someone later removes or misplaces the skip logic inside the actual endpoint functions.

For example, if update_key_fn is refactored and the if new_key_alias != existing_key_row.key_alias: guard is accidentally dropped, all four tests here would still pass while the production bug reappears.

For stronger regression protection, consider calling the actual functions with a mocked prisma_client (as the existing TestLIT1884KeyUpdateValidation class does) rather than re-implementing the conditional inline.