diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index a3679b84bd6..6f44418df49 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -2241,6 +2241,12 @@ async def _validate_mcp_servers_for_key_update( return normalized_object_permission +def _budget_limit_state( + budget_limits: list[BudgetLimitEntry] | None, +) -> tuple[tuple[str, float], ...]: + return tuple((window.budget_duration, window.max_budget) for window in budget_limits or []) + + async def _validate_update_key_data( data: UpdateKeyRequest, existing_key_row: Any, @@ -2313,20 +2319,21 @@ async def _validate_update_key_data( # - Anyone else (non-PROXY_ADMIN, not the owner, not a team member # on a team key): must pass _check_key_admin_access (PROXY_ADMIN # / key-owner / team-admin / org-admin of the key). - # - max_budget / spend / budget_limits: always require the admin - # check, even for the key owner or a team member (matches the - # existing admin-only budget semantics). budget_limits uses - # model_fields_set because an explicit null/[] clears the field - # and must gate the same as setting or changing it. # - spend gates on presence alone (not a value diff): the DB spend # lags the live cross-pod counter, so letting an "unchanged" spend # through the non-admin path would let a key owner / team member # overwrite the live counter below real usage and silently weaken # enforcement. + _existing_budget_limits = [ + BudgetLimitEntry.model_validate(window) for window in (getattr(existing_key_row, "budget_limits", None) or []) + ] + _budget_limits_changed = "budget_limits" in data.model_fields_set and _budget_limit_state( + data.budget_limits + ) != _budget_limit_state(_existing_budget_limits) _is_budget_change = ( (data.max_budget is not None and data.max_budget != existing_key_row.max_budget) or data.spend is not None - or "budget_limits" in data.model_fields_set + or _budget_limits_changed ) _existing_metadata = getattr(existing_key_row, "metadata", None) diff --git a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py index c9abdf09a5d..9dcd0f7f8df 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py @@ -10458,7 +10458,7 @@ class TestKeyOwnerPrivilegeEscalation: Policy: - created_by == caller → can edit any non-budget field without admin - created_by != caller (assigned user) → must pass admin check for any edit - - budget changes (max_budget/spend) → always require admin + - budget changes → always require admin - PROXY_ADMIN → unrestricted """ @@ -10472,6 +10472,7 @@ class TestKeyOwnerPrivilegeEscalation: row.team_id = None row.max_budget = None row.spend = 0.0 + row.budget_limits = None row.organization_id = None row.project_id = None return row @@ -10631,6 +10632,13 @@ class TestKeyOwnerPrivilegeEscalation: """Clearing budget_limits is a budget change and requires admin.""" data = UpdateKeyRequest(key="sk-test", budget_limits=cleared_value) existing = self._make_existing_key(created_by="creator-123") + existing.budget_limits = [ + { + "budget_duration": "30d", + "max_budget": 100.0, + "reset_at": "2026-08-01T00:00:00Z", + } + ] auth = self._make_auth(user_id="creator-123") mock_check = AsyncMock( @@ -10652,6 +10660,59 @@ class TestKeyOwnerPrivilegeEscalation: ) mock_check.assert_called_once() + @pytest.mark.asyncio + @pytest.mark.parametrize("unchanged_value", [[], None]) + async def test_creator_can_send_unchanged_empty_budget_limits( + self, unchanged_value + ): + data = UpdateKeyRequest(key="sk-test", budget_limits=unchanged_value) + existing = self._make_existing_key(created_by="creator-123") + auth = self._make_auth(user_id="creator-123") + + mock_check = AsyncMock() + with patch( + "litellm.proxy.management_endpoints.key_management_endpoints._check_key_admin_access", + mock_check, + ): + await _validate_update_key_data( + data=data, + existing_key_row=existing, + user_api_key_dict=auth, + llm_router=None, + premium_user=False, + prisma_client=AsyncMock(), + user_api_key_cache=MagicMock(), + ) + mock_check.assert_not_called() + + @pytest.mark.asyncio + async def test_creator_can_send_unchanged_existing_budget_limits(self): + unchanged_window = { + "budget_duration": "30d", + "max_budget": 100.0, + "reset_at": "2026-08-01T00:00:00Z", + } + data = UpdateKeyRequest(key="sk-test", budget_limits=[unchanged_window]) + existing = self._make_existing_key(created_by="creator-123") + existing.budget_limits = [unchanged_window] + auth = self._make_auth(user_id="creator-123") + + mock_check = AsyncMock() + with patch( + "litellm.proxy.management_endpoints.key_management_endpoints._check_key_admin_access", + mock_check, + ): + await _validate_update_key_data( + data=data, + existing_key_row=existing, + user_api_key_dict=auth, + llm_router=None, + premium_user=False, + prisma_client=AsyncMock(), + user_api_key_cache=MagicMock(), + ) + mock_check.assert_not_called() + @pytest.mark.asyncio async def test_admin_can_clear_budget_limits(self): data = UpdateKeyRequest(key="sk-test", budget_limits=[]) diff --git a/ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx b/ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx index f8b8e5b6de3..f230cf16b6d 100644 --- a/ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx @@ -662,7 +662,31 @@ describe("KeyEditView", () => { }); }); - it("should resend existing budget windows on submit when they are left untouched", async () => { + it("should omit budget_limits when no budget windows exist", async () => { + const onSubmitMock = vi.fn().mockResolvedValue(undefined); + renderWithProviders( + {}} + onSubmit={onSubmitMock} + accessToken={"test-token"} + userID={"test-user"} + userRole={"user"} + premiumUser={false} + />, + ); + + const submitButton = await screen.findByRole("button", { name: /save changes/i }); + await userEvent.click(submitButton); + + await waitFor(() => { + expect(onSubmitMock).toHaveBeenCalled(); + const callArgs = onSubmitMock.mock.calls[0][0]; + expect(callArgs.budget_limits).toBeUndefined(); + }); + }); + + it("should omit existing budget windows when they are left untouched", async () => { const onSubmitMock = vi.fn().mockResolvedValue(undefined); const keyDataWithWindow = { ...MOCK_KEY_DATA, @@ -686,7 +710,7 @@ describe("KeyEditView", () => { await waitFor(() => { expect(onSubmitMock).toHaveBeenCalled(); const callArgs = onSubmitMock.mock.calls[0][0]; - expect(callArgs.budget_limits).toEqual([{ budget_duration: "30d", max_budget: 100 }]); + expect(callArgs.budget_limits).toBeUndefined(); }); }); diff --git a/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx b/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx index dce7b9bd949..e0ba0c612db 100644 --- a/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx @@ -87,6 +87,13 @@ const getKeyTypeFromRoutes = (allowedRoutes: string[] | null | undefined): strin return "default"; }; +const areBudgetWindowsEqual = (first: readonly BudgetWindowEntry[], second: readonly BudgetWindowEntry[]): boolean => + first.length === second.length && + first.every( + (window, index) => + window.budget_duration === second[index].budget_duration && window.max_budget === second[index].max_budget, + ); + export function KeyEditView({ keyData, onCancel, @@ -306,18 +313,16 @@ export function KeyEditView({ values.duration = null; } - // Reconcile multi-window budget limits from the editor state, dropping - // incomplete entries (no max_budget). Sending [] tells the backend to clear - // all stored windows, so only send it when the user removed every window; - // when entries remain but are still incomplete, omit the field so the saved - // windows are left untouched (JSON.stringify drops the undefined key). const validWindows = budgetLimits.filter( (w) => w.budget_duration && w.max_budget !== null && w.max_budget !== undefined, ); - if (validWindows.length > 0) { - values.budget_limits = validWindows; - } else if (budgetLimits.length === 0) { - values.budget_limits = []; + const originalWindows = Array.isArray(keyData.budget_limits) ? keyData.budget_limits : []; + if (!areBudgetWindowsEqual(budgetLimits, originalWindows)) { + if (validWindows.length > 0) { + values.budget_limits = validWindows; + } else if (budgetLimits.length === 0) { + values.budget_limits = []; + } } // Always send the current per-tag limit map so removing every row