From 0b5ea60bfd2a01545f1a05a9df6de150761672a8 Mon Sep 17 00:00:00 2001 From: yucheng Date: Sat, 3 Oct 2026 02:10:22 +0000 Subject: [PATCH] fix(proxy): keep project object permission validation on the stored payload Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../management_endpoints/project_endpoints.py | 13 +++---- .../management/test_project_lifecycle.py | 27 ------------- .../test_project_endpoints_prisma.py | 38 +++++++++++++++---- 3 files changed, 35 insertions(+), 43 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/management_endpoints/project_endpoints.py b/enterprise/litellm_enterprise/proxy/management_endpoints/project_endpoints.py index 5a754141063..ccdf5057b1c 100644 --- a/enterprise/litellm_enterprise/proxy/management_endpoints/project_endpoints.py +++ b/enterprise/litellm_enterprise/proxy/management_endpoints/project_endpoints.py @@ -790,16 +790,8 @@ async def update_project( data, existing_project, _router_access_group_names(llm_router) ) - object_permission_data: Final = ( - data.object_permission.model_dump(exclude_none=True) if data.object_permission is not None else None - ) - object_permission_payload: Final = ( - _OBJECT_PERMISSION_PAYLOAD.validate_python(object_permission_data) if object_permission_data else None - ) - # Prepare update data update_data = _jsonified(prisma_client, data.model_dump(exclude_none=True, exclude={"project_id"})) - update_data.pop("object_permission", None) update_data["updated_by"] = user_api_key_dict.user_id or litellm_proxy_admin_name # Handle budget updates @@ -809,6 +801,11 @@ async def update_project( **({"max_budget": None} if "max_budget" in data.model_fields_set and data.max_budget is None else {}), } + object_permission_data: Final = update_data.pop("object_permission", None) + object_permission_payload: Final = ( + _OBJECT_PERMISSION_PAYLOAD.validate_python(object_permission_data) if object_permission_data else None + ) + if data.team_id is not None: current_project_record: Final = await _writer_project_table(prisma_client).find_unique( where={"project_id": data.project_id} diff --git a/tests/integration/management/test_project_lifecycle.py b/tests/integration/management/test_project_lifecycle.py index 25b8da7875d..bf5dac0da89 100644 --- a/tests/integration/management/test_project_lifecycle.py +++ b/tests/integration/management/test_project_lifecycle.py @@ -65,17 +65,6 @@ def _clear_key_object_permission(key: str, permission_id: str) -> None: ) -def _clear_project_object_permission(project_id: str, permission_id: str) -> None: - write_rows( - 'UPDATE "LiteLLM_ProjectTable" SET object_permission_id = NULL WHERE project_id = %s', - (project_id,), - ) - write_rows( - 'DELETE FROM "LiteLLM_ObjectPermissionTable" WHERE object_permission_id = %s', - (permission_id,), - ) - - def _deleted_key_rows(key: str) -> list[dict[str, JsonValue]]: return read_rows( 'SELECT token FROM "LiteLLM_DeletedVerificationToken" WHERE token = %s', @@ -660,22 +649,8 @@ def test_project_update_rejects_moving_project_with_attached_key(ownership_gatew budget: Final = scenario.budget(max_budget=3) attached_project: Final = scenario.project(team_a, budget_id=budget, models=[model]) key: Final = scenario.key(team_id=team_a, project_id=attached_project, models=[model]) - permission_seed: Final = ownership_gateway.request( - "POST", - "/project/update", - {"project_id": attached_project, "object_permission": {"vector_stores": ["existing-store"]}}, - ) - assert permission_seed.status_code == 200, permission_seed.text - project_permission: Final = read_rows( - 'SELECT object_permission_id FROM "LiteLLM_ProjectTable" WHERE project_id = %s', - (attached_project,), - ) - assert len(project_permission) == 1 - permission_id: Final = string_value(project_permission[0]["object_permission_id"]) - scenario.cleanups.callback(_clear_project_object_permission, attached_project, permission_id) project_before: Final = _project_rows(attached_project) key_before: Final = _key_rows(key) - permission_before: Final = _object_permission_rows(permission_id) budget_before: Final = _budget_rows(budget) moved_with_key: Final = ownership_gateway.request( "POST", @@ -684,13 +659,11 @@ def test_project_update_rejects_moving_project_with_attached_key(ownership_gatew "project_id": attached_project, "team_id": team_b, "max_budget": 11, - "object_permission": {"vector_stores": ["replacement-store"]}, }, ) assert moved_with_key.status_code == 400, moved_with_key.text assert _project_rows(attached_project) == project_before assert _key_rows(key) == key_before - assert _object_permission_rows(permission_id) == permission_before assert _budget_rows(budget) == budget_before diff --git a/tests/unit/enterprise/proxy/management_endpoints/test_project_endpoints_prisma.py b/tests/unit/enterprise/proxy/management_endpoints/test_project_endpoints_prisma.py index cd185179ee9..ad2ce582c1f 100644 --- a/tests/unit/enterprise/proxy/management_endpoints/test_project_endpoints_prisma.py +++ b/tests/unit/enterprise/proxy/management_endpoints/test_project_endpoints_prisma.py @@ -1269,6 +1269,36 @@ def _written_project_data(mock_prisma: mock.MagicMock) -> dict: return mock_prisma.db.litellm_projecttable.update.await_args.kwargs["data"] +@pytest.mark.asyncio +async def test_update_project_object_permission_validation_precedes_budget_write( + monkeypatch: pytest.MonkeyPatch, +) -> None: + project_id: Final = "project-object-permission-validation" + mock_prisma: Final = _project_update_mocks(monkeypatch, {}) + mock_prisma.db.litellm_projecttable.find_unique.return_value.budget_id = "budget-project" + budget_table: Final = mock.MagicMock() + budget_table.update = mock.AsyncMock() + mock_prisma.db.litellm_budgettable = budget_table + + def jsonify_object_permission_as_string(payload: dict[str, object]) -> dict[str, object]: + return {**payload, "object_permission": '{"vector_stores": ["replacement-store"]}'} + + mock_prisma.jsonify_object = jsonify_object_permission_as_string + + with pytest.raises(ProxyException) as error: + await _run_project_update( + project_id, + max_budget=50, + object_permission={"vector_stores": ["replacement-store"]}, + ) + + assert error.value.code == "500" + assert "Input should be a valid dictionary" in error.value.message + assert "input_type=str" in error.value.message + budget_table.update.assert_not_awaited() + mock_prisma.db.litellm_projecttable.update.assert_not_awaited() + + @pytest.mark.asyncio async def test_update_project_rejects_move_when_attached_teamless_key_exists( monkeypatch: pytest.MonkeyPatch, @@ -1278,14 +1308,9 @@ async def test_update_project_rejects_move_when_attached_teamless_key_exists( mock_prisma: Final = _project_update_mocks(monkeypatch, {}) mock_prisma.db.litellm_projecttable.find_unique.return_value.team_id = "team-a" mock_prisma.db.litellm_projecttable.find_unique.return_value.budget_id = "budget-project" - mock_prisma.db.litellm_projecttable.find_unique.return_value.object_permission_id = "permission-project" budget_table: Final = mock.MagicMock() budget_table.update = mock.AsyncMock() - permission_table: Final = mock.MagicMock() - permission_table.update = mock.AsyncMock() - permission_table.create = mock.AsyncMock() mock_prisma.db.litellm_budgettable = budget_table - mock_prisma.db.litellm_objectpermissiontable = permission_table mock_prisma.db.litellm_teamtable.find_unique = mock.AsyncMock( return_value=LiteLLM_TeamTable(team_id=destination_team_id) ) @@ -1306,7 +1331,6 @@ async def test_update_project_rejects_move_when_attached_teamless_key_exists( project_id, team_id=destination_team_id, max_budget=50, - object_permission={"vector_stores": ["replacement-store"]}, ) expected_detail: Final = { @@ -1320,8 +1344,6 @@ async def test_update_project_rejects_move_when_attached_teamless_key_exists( mock_prisma.writer_db.litellm_verificationtoken.count.assert_awaited_once() mock_prisma.db.litellm_verificationtoken.count.assert_not_awaited() budget_table.update.assert_not_awaited() - permission_table.update.assert_not_awaited() - permission_table.create.assert_not_awaited() mock_prisma.db.litellm_projecttable.update.assert_not_awaited()