diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index 6435ce0d12f..b13214eaf5e 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -4034,11 +4034,11 @@ async def _get_agent_ids_from_access_groups( ) -def _client_facing_model_access_denied_message(internal_message: str, model: str | list[str]) -> str: +def client_facing_model_access_denied_message(internal_message: str, model: str | list[str]) -> str: template: Final = litellm.model_access_denied_message if not template: return internal_message - verbose_proxy_logger.warning(internal_message) + verbose_proxy_logger.warning(internal_message.replace("\r", "").replace("\n", "")) return template.replace(MODEL_ACCESS_DENIED_MESSAGE_MODEL_PLACEHOLDER, str(model)) @@ -4164,7 +4164,7 @@ def _can_object_call_model( return True raise ProxyException( - message=_client_facing_model_access_denied_message( + message=client_facing_model_access_denied_message( internal_message=f"{object_type} not allowed to access model. This {object_type} can only access models={models}. Tried to access {model}", model=model, ), @@ -4793,7 +4793,7 @@ async def can_user_call_model( if SpecialModelNames.no_default_models.value in user_object.models: raise ProxyException( - message=_client_facing_model_access_denied_message( + message=client_facing_model_access_denied_message( internal_message=f"User not allowed to access model. No default model access, only team models allowed. Tried to access {model}", model=model, ), @@ -5398,7 +5398,7 @@ async def _check_team_member_model_access( ) except ProxyException: raise ProxyException( - message=_client_facing_model_access_denied_message( + message=client_facing_model_access_denied_message( internal_message=f"Team member not allowed to access model. User={valid_token.user_id}, Team={team_object.team_id}, Model={model}. Allowed member models = {member_allowed_models}", model=model, ), diff --git a/litellm/proxy/auth/handle_jwt.py b/litellm/proxy/auth/handle_jwt.py index 94ca3047f45..6e5e75a5e20 100644 --- a/litellm/proxy/auth/handle_jwt.py +++ b/litellm/proxy/auth/handle_jwt.py @@ -66,6 +66,7 @@ from litellm.types.agents import AgentResponse from .auth_checks import ( _allowed_routes_check, allowed_routes_check, + client_facing_model_access_denied_message, get_actual_routes, get_end_user_object, get_org_object, @@ -1339,7 +1340,10 @@ class JWTAuthManager: if model not in role_based_models: raise HTTPException( status_code=403, - detail=f"Role={rbac_role} not allowed to call model={model}. Allowed models={role_based_models}", + detail=client_facing_model_access_denied_message( + internal_message=f"Role={rbac_role} not allowed to call model={model}. Allowed models={role_based_models}", + model=model, + ), ) return True @@ -1370,7 +1374,12 @@ class JWTAuthManager: if requested_model not in allowed_models: raise HTTPException( status_code=403, - detail={"error": f"model={requested_model} not allowed. Allowed_models={allowed_models}"}, + detail={ + "error": client_facing_model_access_denied_message( + internal_message=f"model={requested_model} not allowed. Allowed_models={allowed_models}", + model=requested_model, + ) + }, ) return diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index f758628c02a..b0e1cc2e0fe 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -17435,12 +17435,12 @@ GeneralSettingsUILiteLLMValue = float | bool | str | None class GeneralSettingsUILiteLLMFieldSpec(TypedDict): - type: Literal["Float", "Dollar", "Boolean", "Select", "String"] - description: str - options: NotRequired[tuple[str, ...]] - tab: NotRequired[str] # Admin UI sub-tab this field renders under; None groups it with the rest - default: NotRequired[ - float | bool + type: ReadOnly[Literal["Float", "Dollar", "Boolean", "Select", "String"]] + description: ReadOnly[str] + options: ReadOnly[NotRequired[tuple[str, ...]]] + tab: ReadOnly[NotRequired[str]] # Admin UI sub-tab this field renders under; None groups it with the rest + default: ReadOnly[ + NotRequired[float | bool] ] # reset/clear restores this instead of None; fields whose None means fail-open set it diff --git a/tests/test_litellm/proxy/auth/test_auth_checks.py b/tests/test_litellm/proxy/auth/test_auth_checks.py index e99e92fcd02..0052cfb66d3 100644 --- a/tests/test_litellm/proxy/auth/test_auth_checks.py +++ b/tests/test_litellm/proxy/auth/test_auth_checks.py @@ -1683,8 +1683,6 @@ _DENIED_MESSAGE_TEMPLATE: Final = "The model `{model}` is unavailable for this A def test_can_object_call_model_denial_uses_configured_message_and_logs_detail(monkeypatch, caplog): - """LIT-5283: with model_access_denied_message set, the client sees only the template with - {model} filled in, while the allowed models / access groups stay in the proxy log.""" monkeypatch.setattr(litellm, "model_access_denied_message", _DENIED_MESSAGE_TEMPLATE) with caplog.at_level("WARNING", logger="LiteLLM Proxy"): @@ -1706,6 +1704,26 @@ def test_can_object_call_model_denial_uses_configured_message_and_logs_detail(mo assert "anthropic-sonnet-4-5" in caplog.text +def test_can_object_call_model_denial_log_strips_newlines_from_requested_model(monkeypatch, caplog): + monkeypatch.setattr(litellm, "model_access_denied_message", _DENIED_MESSAGE_TEMPLATE) + + with caplog.at_level("WARNING", logger="LiteLLM Proxy"): + with pytest.raises(ProxyException): + _can_object_call_model( + model="gpt-5.6\r\nWARNING forged log line", + llm_router=None, + models=["internal-models"], + object_type="key", + ) + + denial_records = [r for r in caplog.records if "not allowed to access model" in r.getMessage()] + assert len(denial_records) == 1 + assert denial_records[0].getMessage() == ( + "key not allowed to access model. This key can only access models=['internal-models']. " + "Tried to access gpt-5.6WARNING forged log line" + ) + + @pytest.mark.parametrize("unset_value", [None, ""]) def test_can_object_call_model_denial_unchanged_when_message_not_configured(monkeypatch, unset_value): monkeypatch.setattr(litellm, "model_access_denied_message", unset_value) diff --git a/tests/test_litellm/proxy/auth/test_handle_jwt.py b/tests/test_litellm/proxy/auth/test_handle_jwt.py index 814e31535e0..921472f2f43 100644 --- a/tests/test_litellm/proxy/auth/test_handle_jwt.py +++ b/tests/test_litellm/proxy/auth/test_handle_jwt.py @@ -9,6 +9,8 @@ from fastapi import HTTPException import httpx import pytest +import litellm + from litellm.proxy._types import ( DEFAULT_JWKS_STALE_TTL, JWTLiteLLMRoleMap, @@ -21,6 +23,8 @@ from litellm.proxy._types import ( Member, ProxyErrorTypes, ProxyException, + RoleBasedPermissions, + ScopeMapping, ) from litellm.caching.dual_cache import DualCache from litellm.proxy.agent_endpoints.agent_registry import AgentRegistry @@ -6965,3 +6969,56 @@ async def test_auth_builder_denies_jwt_naming_unregistered_agent_before_admin_ch ) assert exc_info.value.status_code == 403 + + +_JWT_DENIED_MESSAGE_TEMPLATE = "The model `{model}` is unavailable for this identity." + + +@pytest.mark.parametrize( + "configured_message, expected_detail", + [ + (None, "Role=internal_user not allowed to call model=gpt-5.6. Allowed models=['gpt-5.6-mini']"), + ("", "Role=internal_user not allowed to call model=gpt-5.6. Allowed models=['gpt-5.6-mini']"), + (_JWT_DENIED_MESSAGE_TEMPLATE, "The model `gpt-5.6` is unavailable for this identity."), + ], +) +def test_can_rbac_role_call_model_denial_honors_configured_message(monkeypatch, configured_message, expected_detail): + monkeypatch.setattr(litellm, "model_access_denied_message", configured_message) + general_settings = { + "role_permissions": [ + RoleBasedPermissions(role=LitellmUserRoles.INTERNAL_USER, models=["gpt-5.6-mini"]), + ] + } + + with pytest.raises(HTTPException) as exc_info: + JWTAuthManager.can_rbac_role_call_model( + rbac_role=LitellmUserRoles.INTERNAL_USER, + general_settings=general_settings, + model="gpt-5.6", + ) + + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == expected_detail + + +@pytest.mark.parametrize( + "configured_message, expected_error", + [ + (None, "model=gpt-5.6 not allowed. Allowed_models=['gpt-5.6-mini']"), + ("", "model=gpt-5.6 not allowed. Allowed_models=['gpt-5.6-mini']"), + (_JWT_DENIED_MESSAGE_TEMPLATE, "The model `gpt-5.6` is unavailable for this identity."), + ], +) +def test_check_scope_based_access_denial_honors_configured_message(monkeypatch, configured_message, expected_error): + monkeypatch.setattr(litellm, "model_access_denied_message", configured_message) + + with pytest.raises(HTTPException) as exc_info: + JWTAuthManager.check_scope_based_access( + scope_mappings=[ScopeMapping(scope="litellm.api.consumer", models=["gpt-5.6-mini"])], + scopes=["litellm.api.consumer"], + request_data={"model": "gpt-5.6"}, + general_settings={}, + ) + + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == {"error": expected_error} diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index 17847575de1..dd05b90bb67 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -10862,24 +10862,15 @@ def test_validate_max_ui_session_budget_empty_restores_default(empty_value): @pytest.mark.asyncio async def test_update_config_field_model_access_denied_message_sets_live_value(monkeypatch): - """LIT-5283: the client-facing model access denial message is editable from the Admin UI - General tab as a String field, applies live via setattr, and persists under litellm_settings.""" - from unittest.mock import MagicMock + from unittest.mock import AsyncMock, MagicMock import litellm.proxy.proxy_server as ps from litellm.proxy._types import ConfigFieldUpdate, LitellmUserRoles, UserAPIKeyAuth from litellm.proxy.proxy_server import update_config_general_settings - saved: dict = {} - - async def fake_get_config(): - return {"litellm_settings": {}} - - async def fake_save_config(new_config=None): - saved.update(new_config or {}) - - monkeypatch.setattr(ps.proxy_config, "get_config", fake_get_config) - monkeypatch.setattr(ps.proxy_config, "save_config", fake_save_config) + save_config = AsyncMock() + monkeypatch.setattr(ps.proxy_config, "get_config", AsyncMock(return_value={"litellm_settings": {}})) + monkeypatch.setattr(ps.proxy_config, "save_config", save_config) monkeypatch.setattr(ps, "prisma_client", MagicMock()) monkeypatch.setattr(litellm, "store_audit_logs", False) monkeypatch.setattr(litellm, "model_access_denied_message", None) @@ -10895,7 +10886,11 @@ async def test_update_config_field_model_access_denied_message_sets_live_value(m ) assert litellm.model_access_denied_message == "Model `{model}` is unavailable for this key." - assert saved["litellm_settings"]["model_access_denied_message"] == "Model `{model}` is unavailable for this key." + save_config.assert_awaited_once() + saved_config = save_config.await_args.kwargs["new_config"] + assert saved_config["litellm_settings"]["model_access_denied_message"] == ( + "Model `{model}` is unavailable for this key." + ) @pytest.mark.parametrize("bad_value", [True, 3, 1.5, ["x"], {"a": "b"}]) @@ -10918,14 +10913,41 @@ def test_validate_model_access_denied_message_empty_restores_detailed_default(em @pytest.mark.parametrize("empty_value", [None, ""]) def test_validate_expose_router_debug_in_errors_empty_restores_true_default(empty_value): - """Clearing the field from the Admin UI must restore the historical default (debug details - exposed), not the generic Boolean fallback of False.""" from litellm.proxy.proxy_server import _validate_general_settings_ui_litellm_value assert _validate_general_settings_ui_litellm_value("expose_router_debug_in_errors", empty_value) is True assert _validate_general_settings_ui_litellm_value("expose_router_debug_in_errors", False) is False +@pytest.mark.parametrize( + "field_name, booted_value, db_value, read_setting", + [ + ( + "model_access_denied_message", + None, + "Model `{model}` is unavailable for this key.", + lambda: litellm.model_access_denied_message, + ), + ("expose_router_debug_in_errors", True, False, lambda: litellm.expose_router_debug_in_errors), + ], +) +def test_model_access_denied_settings_propagate_on_config_reload( + monkeypatch, field_name, booted_value, db_value, read_setting +): + import litellm.proxy.proxy_server as ps + + monkeypatch.setattr(litellm, field_name, booted_value) + assert read_setting() == booted_value + + ps.ProxyConfig()._update_config_fields( + current_config={"litellm_settings": {}}, + param_name="litellm_settings", + db_param_value={field_name: db_value}, + ) + + assert read_setting() == db_value + + def test_general_settings_ui_defaults_unchanged_for_existing_fields(): """The spec-default mechanism added for max_ui_session_budget must not change what clearing the pre-existing fields restores (None for Float/Select, False for Boolean).""" diff --git a/ui/litellm-dashboard/src/app/(dashboard)/router-settings/_components/general_settings.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/router-settings/_components/general_settings.integration.test.tsx index 1e29b181b78..d002d7c116f 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/router-settings/_components/general_settings.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/router-settings/_components/general_settings.integration.test.tsx @@ -130,6 +130,31 @@ describe("GeneralSettings General tab", () => { expect(deleteConfigFieldSetting).toHaveBeenCalledWith("token", "model_access_denied_message"); expect(vi.mocked(updateConfigFieldSetting).mock.calls).toHaveLength(1); }); + + it("keeps the stored value visible when the reset request fails", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue( + SETTINGS_FIXTURE.map((s) => + s.field_name === "model_access_denied_message" + ? { ...s, field_value: "Model `{model}` is unavailable.", stored_in_db: true } + : { ...s }, + ), + ); + vi.mocked(deleteConfigFieldSetting).mockRejectedValueOnce(new Error("proxy unreachable")); + const user = userEvent.setup(); + renderWithProviders(); + + await user.click(screen.getByText("General")); + const row = await settingsRow("model_access_denied_message"); + const input = within(row).getByRole("textbox") as HTMLInputElement; + expect(within(row).getByText("In DB")).toBeInTheDocument(); + + fireEvent.change(input, { target: { value: "" } }); + await user.click(within(row).getByRole("button", { name: /update/i })); + + expect(deleteConfigFieldSetting).toHaveBeenCalledWith("token", "model_access_denied_message"); + expect(within(row).getByText("In DB")).toBeInTheDocument(); + expect(within(row).queryByText("Not Set")).not.toBeInTheDocument(); + }); }); // The five tabs here are proxy-wide settings. Auto-routers moved to Models + Endpoints. diff --git a/ui/litellm-dashboard/src/app/(dashboard)/router-settings/_components/general_settings.tsx b/ui/litellm-dashboard/src/app/(dashboard)/router-settings/_components/general_settings.tsx index 040b1f3c375..38aa85b0f1e 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/router-settings/_components/general_settings.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/router-settings/_components/general_settings.tsx @@ -40,7 +40,7 @@ const NUMERIC_INPUT_WIDTH = "w-36"; const toNumericValue = (raw: string): number | null => (raw === "" ? null : Number(raw)); const toStringValue = (raw: string): string | null => (raw === "" ? null : raw); -const RESETS_WHEN_CLEARED = new Set(["Select", "String"]); +const RESETS_WHEN_CLEARED: ReadonlySet = new Set(["Select", "String"]); const SettingValueEditor: React.FC<{ setting: generalSettingsItem; @@ -213,7 +213,7 @@ const GeneralSettings: React.FC = ({ accessToken, user setGeneralSettings(updatedSettings); }; - const handleUpdateField = (fieldName: string) => { + const handleUpdateField = async (fieldName: string) => { if (!accessToken) { return; } @@ -222,37 +222,33 @@ const GeneralSettings: React.FC = ({ accessToken, user const fieldValue = setting?.field_value; if (fieldValue == null) { - if (setting && RESETS_WHEN_CLEARED.has(setting.field_type)) handleResetField(fieldName); + if (setting && RESETS_WHEN_CLEARED.has(setting.field_type)) await handleResetField(fieldName); return; } try { - updateConfigFieldSetting(accessToken, fieldName, fieldValue); - // update value in state - - const updatedSettings = generalSettings.map((setting) => - setting.field_name === fieldName ? { ...setting, stored_in_db: true } : setting, + await updateConfigFieldSetting(accessToken, fieldName, fieldValue); + setGeneralSettings((current) => + current.map((setting) => (setting.field_name === fieldName ? { ...setting, stored_in_db: true } : setting)), ); - setGeneralSettings(updatedSettings); } catch (error) { // do something } }; - const handleResetField = (fieldName: string) => { + const handleResetField = async (fieldName: string) => { if (!accessToken) { return; } try { - deleteConfigFieldSetting(accessToken, fieldName); - // update value in state - - const updatedSettings = generalSettings.map((setting) => - setting.field_name === fieldName - ? { ...setting, stored_in_db: null, field_value: setting.field_default_value ?? null } - : setting, + await deleteConfigFieldSetting(accessToken, fieldName); + setGeneralSettings((current) => + current.map((setting) => + setting.field_name === fieldName + ? { ...setting, stored_in_db: null, field_value: setting.field_default_value ?? null } + : setting, + ), ); - setGeneralSettings(updatedSettings); } catch (error) { // do something }