mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
fix(key_management): keep read-only keys read-only in non-admin preset transitions
A non-admin could widen a read-only (info_routes) key to llm_api or full access through the preset carve-out. Read-only keys now stay read-only unless a proxy admin widens them; the other preset transitions, including the LIT-4891 llm_api to full access switch, still work. Also converts the transition tests to assert on a returned outcome so the no-403 cases carry real assertions.
This commit is contained in:
parent
4bfc6766e7
commit
81c48f810e
2 changed files with 81 additions and 31 deletions
|
|
@ -736,6 +736,9 @@ def _check_allowed_routes_caller_permission(
|
|||
)
|
||||
|
||||
|
||||
_READ_ONLY_ALLOWED_ROUTES_PRESET: Final = frozenset(("info_routes",))
|
||||
|
||||
|
||||
def _is_safe_preset_route_transition(
|
||||
incoming_allowed_routes: Sequence[str] | None,
|
||||
existing_allowed_routes: Sequence[str] | None,
|
||||
|
|
@ -743,13 +746,16 @@ def _is_safe_preset_route_transition(
|
|||
"""
|
||||
True when every route on BOTH sides is a safe `key_type` preset bucket
|
||||
(empty = full access, which non-admins already get from a default
|
||||
`/key/generate`). Requiring the existing side too keeps an owner from
|
||||
clearing an admin-set custom route restriction (LIT-4139).
|
||||
`/key/generate`), with one carve-out: a read-only (`info_routes`) key
|
||||
stays read-only, so widening it needs an admin. Requiring the existing
|
||||
side to be a safe preset keeps an owner from clearing an admin-set
|
||||
custom route restriction (LIT-4139).
|
||||
"""
|
||||
return all(
|
||||
route in _NON_ADMIN_SAFE_ALLOWED_ROUTES_PRESETS
|
||||
for route in (*(incoming_allowed_routes or ()), *(existing_allowed_routes or ()))
|
||||
)
|
||||
incoming: Final = frozenset(incoming_allowed_routes or ())
|
||||
existing: Final = frozenset(existing_allowed_routes or ())
|
||||
if not (incoming | existing) <= _NON_ADMIN_SAFE_ALLOWED_ROUTES_PRESETS:
|
||||
return False
|
||||
return existing != _READ_ONLY_ALLOWED_ROUTES_PRESET or incoming == existing
|
||||
|
||||
|
||||
def _enforce_allowed_routes_update_permission(
|
||||
|
|
|
|||
|
|
@ -11054,56 +11054,100 @@ class TestLIT4891SafePresetKeyTypeTransition:
|
|||
)
|
||||
|
||||
async def _run_update(self, data, existing_key_row):
|
||||
await _validate_update_key_data(
|
||||
data=data,
|
||||
existing_key_row=existing_key_row,
|
||||
user_api_key_dict=self._make_auth(),
|
||||
llm_router=None,
|
||||
premium_user=False,
|
||||
prisma_client=AsyncMock(),
|
||||
user_api_key_cache=MagicMock(),
|
||||
)
|
||||
try:
|
||||
await _validate_update_key_data(
|
||||
data=data,
|
||||
existing_key_row=existing_key_row,
|
||||
user_api_key_dict=self._make_auth(),
|
||||
llm_router=None,
|
||||
premium_user=False,
|
||||
prisma_client=AsyncMock(),
|
||||
user_api_key_cache=MagicMock(),
|
||||
)
|
||||
except HTTPException as exc:
|
||||
return exc
|
||||
return None
|
||||
|
||||
def _assert_routes_403(self, exc):
|
||||
assert exc is not None
|
||||
assert exc.status_code == 403
|
||||
assert "Only proxy admins can set" in str(exc.detail)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_owner_can_clear_safe_preset_to_full_access(self):
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=[]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["llm_api_routes"]),
|
||||
assert (
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=[]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["llm_api_routes"]),
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_owner_can_switch_full_access_to_safe_preset(self):
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=["llm_api_routes"]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=[]),
|
||||
assert (
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=["llm_api_routes"]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=[]),
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_owner_can_switch_between_safe_presets(self):
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=["info_routes"]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["llm_api_routes"]),
|
||||
async def test_non_admin_owner_can_narrow_to_read_only_preset(self):
|
||||
assert (
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=["info_routes"]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["llm_api_routes"]),
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_can_resend_read_only_preset_unchanged(self):
|
||||
assert (
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=["info_routes"]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["info_routes"]),
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_cannot_widen_read_only_key_to_full_access(self):
|
||||
self._assert_routes_403(
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=[]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["info_routes"]),
|
||||
)
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_cannot_widen_read_only_key_to_llm_api(self):
|
||||
self._assert_routes_403(
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=["llm_api_routes"]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["info_routes"]),
|
||||
)
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_cannot_clear_custom_route_restriction(self):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
self._assert_routes_403(
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=[]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["/chat/completions"]),
|
||||
)
|
||||
assert exc_info.value.status_code == 403
|
||||
assert "Only proxy admins can set" in str(exc_info.value.detail)
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_cannot_set_non_preset_routes(self):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
self._assert_routes_403(
|
||||
await self._run_update(
|
||||
data=UpdateKeyRequest(key="sk-test", allowed_routes=["management_routes"]),
|
||||
existing_key_row=self._make_existing_key(allowed_routes=["llm_api_routes"]),
|
||||
)
|
||||
assert exc_info.value.status_code == 403
|
||||
assert "Only proxy admins can set" in str(exc_info.value.detail)
|
||||
)
|
||||
|
||||
|
||||
class TestKeyOwnerPrivilegeEscalation:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue