From ea8a66719f4140db653f57425f790ba52fa2d678 Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Wed, 22 Jul 2026 17:55:26 +0530 Subject: [PATCH] [WEB-8332] fix: validate role before guest-cascade, make it atomic (Copilot #9460) partial_update cascaded the guest project-role downgrade BEFORE the serializer was validated/saved, using a manual int() cast that 500s on non-integer input. Reorder: validate the serializer first, then cascade using serializer.validated_data["role"] inside a transaction.atomic() with save(), so a bad role returns 400 (not 500) and project roles can't be downgraded when the member update doesn't persist. Adds a non-integer-role 400 test. Co-authored-by: Plane AI --- apps/api/plane/app/views/workspace/member.py | 23 +++++++++++++------ ...st_workspace_member_mass_assignment_app.py | 19 +++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/apps/api/plane/app/views/workspace/member.py b/apps/api/plane/app/views/workspace/member.py index 46eedb3638..0b7a94c3ae 100644 --- a/apps/api/plane/app/views/workspace/member.py +++ b/apps/api/plane/app/views/workspace/member.py @@ -3,6 +3,7 @@ # See the LICENSE file for details. # Django imports +from django.db import transaction from django.db.models import Count, Q, OuterRef, Subquery, IntegerField from django.utils import timezone from django.db.models.functions import Coalesce @@ -84,10 +85,6 @@ class WorkSpaceMemberViewSet(BaseViewSet): status=status.HTTP_400_BAD_REQUEST, ) - # If a user is moved to a guest role he can't have any other role in projects - if "role" in request.data and int(request.data.get("role")) == 5: - ProjectMember.objects.filter(workspace__slug=slug, member_id=workspace_member.member_id).update(role=5) - # SECURITY: The only field this endpoint is allowed to mutate is ``role``. # ``WorkSpaceMemberSerializer`` is declared with ``fields = "__all__"`` and # ``DynamicBaseSerializer`` ignores the ``fields=`` kwarg for writes, so @@ -101,10 +98,22 @@ class WorkSpaceMemberViewSet(BaseViewSet): serializer = WorkSpaceMemberSerializer(workspace_member, data=allowed_data, partial=True) - if serializer.is_valid(): + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + # Validate the role (via the serializer) BEFORE cascading, and do the + # cascade + save atomically. Otherwise a bad role (e.g. non-integer) would + # 500 on the manual int() cast, and the guest project-role downgrade could + # persist even if the member update never succeeds. + with transaction.atomic(): + # If a user is moved to a guest role they can't hold any other project role. + if serializer.validated_data.get("role") == 5: + ProjectMember.objects.filter( + workspace__slug=slug, member_id=workspace_member.member_id + ).update(role=5) serializer.save() - return Response(serializer.data, status=status.HTTP_200_OK) - return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + return Response(serializer.data, status=status.HTTP_200_OK) @allow_permission(allowed_roles=[ROLE.ADMIN], level="WORKSPACE") def destroy(self, request, slug, pk): diff --git a/apps/api/plane/tests/contract/app/test_workspace_member_mass_assignment_app.py b/apps/api/plane/tests/contract/app/test_workspace_member_mass_assignment_app.py index 7b7b0acfcb..3a60ccf40b 100644 --- a/apps/api/plane/tests/contract/app/test_workspace_member_mass_assignment_app.py +++ b/apps/api/plane/tests/contract/app/test_workspace_member_mass_assignment_app.py @@ -158,3 +158,22 @@ class TestWorkspaceMemberMassAssignment: assert response.status_code == status.HTTP_400_BAD_REQUEST own_member.refresh_from_db() assert own_member.role == 20 + + def test_non_integer_role_is_400_not_500(self, attacker_workspace): + """A non-integer role must be a validation error (400), not a 500 — and + the guest project-role cascade must not run when the update is invalid.""" + attacker_ws, attacker = attacker_workspace + target = _make_user("bad-role-target@plane.so") + target_member = _add_member(attacker_ws, target, role=15) + + client = APIClient() + client.force_authenticate(user=attacker) + response = client.patch( + _member_detail_url(attacker_ws.slug, target_member.id), + {"role": "not-a-number"}, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + target_member.refresh_from_db() + assert target_member.role == 15