mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-04 02:31:27 +00:00
fix(proxy): apply access denied message to JWT paths, sanitize denial log, await dashboard saves
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
168b5bc4fb
commit
d4d8cc9092
8 changed files with 176 additions and 49 deletions
|
|
@ -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,
|
||||
),
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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)."""
|
||||
|
|
|
|||
|
|
@ -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(<GeneralSettings accessToken="token" userRole="Admin" userID="user" />);
|
||||
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -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<string> = new Set(["Select", "String"]);
|
||||
|
||||
const SettingValueEditor: React.FC<{
|
||||
setting: generalSettingsItem;
|
||||
|
|
@ -213,7 +213,7 @@ const GeneralSettings: React.FC<GeneralSettingsPageProps> = ({ accessToken, user
|
|||
setGeneralSettings(updatedSettings);
|
||||
};
|
||||
|
||||
const handleUpdateField = (fieldName: string) => {
|
||||
const handleUpdateField = async (fieldName: string) => {
|
||||
if (!accessToken) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -222,37 +222,33 @@ const GeneralSettings: React.FC<GeneralSettingsPageProps> = ({ 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
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue