Skip to content

fix: prevent privilege escalation in project member role updates #8833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sriramveeraghanta merged 1 commit into from
Apr 6, 2026
+29 −14
Merged

fix: prevent privilege escalation in project member role updates #8833

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 29 additions & 14 deletions apps/api/plane/app/views/project/member.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,21 +226,36 @@ def partial_update(self, request, slug, project_id, pk):
is_active=True,
)

if workspace_role in [5] and int(request.data.get("role", project_member.role)) in [15, 20]:
return Response(
{"error": "You cannot add a user with role higher than the workspace role"},
status=status.HTTP_400_BAD_REQUEST,
)
if "role" in request.data:
# Only Admins can modify roles
if requested_project_member.role < ROLE.ADMIN.value and not is_workspace_admin:
return Response(
{"error": "You do not have permission to update roles"},
status=status.HTTP_403_FORBIDDEN,
)
Comment on lines +229 to +235
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces security-critical authorization logic for role updates, but there doesn’t appear to be contract/unit coverage for the project member role-update endpoint. Given existing contract tests for other project/workspace endpoints under apps/api/plane/tests/contract/, add automated tests for the scenarios in the PR test plan (Guest/Member forbidden; Admin demotion only; workspace-admin bypass; non-role patch still allowed).

Copilot uses AI. Check for mistakes.

if (
"role" in request.data
and int(request.data.get("role", project_member.role)) > requested_project_member.role
and not is_workspace_admin
):
return Response(
{"error": "You cannot update a role that is higher than your own role"},
status=status.HTTP_400_BAD_REQUEST,
)
# Cannot modify a member whose role is equal to or higher than your own
if project_member.role >= requested_project_member.role and not is_workspace_admin:
Comment on lines +229 to +238
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_workspace_admin is computed from the target member’s WorkspaceMember.role (the user being edited), but it’s used here as the bypass condition for the requester (e.g., ... and not is_workspace_admin). This lets a non-admin requester bypass all role checks when editing a workspace admin target, and it also fails to grant the intended bypass to actual workspace-admin requesters. Consider fetching the requester’s workspace role separately (e.g., requester_workspace_role) and using that for bypass decisions; keep the target’s workspace role only for validating what roles can be assigned to that target.

Copilot uses AI. Check for mistakes.
return Response(
{"error": "You cannot update the role of a member with a role equal to or higher than your own"},
status=status.HTTP_403_FORBIDDEN,
)

new_role = int(request.data.get("role"))
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unhandled ValueError on non-numeric role input

If request.data.get("role") contains a non-integer value (e.g., "abc" or null), int() raises ValueError, resulting in a 500 error instead of a proper 400 validation response.

🛡️ Proposed fix 
+            try:
+                new_role = int(request.data.get("role"))
+            except (ValueError, TypeError):
+                return Response(
+                    {"error": "Invalid role value"},
+                    status=status.HTTP_400_BAD_REQUEST,
+                )
-            new_role = int(request.data.get("role"))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
new_role = int(request.data.get("role"))
try:
new_role = int(request.data.get("role"))
except (ValueError, TypeError):
return Response(
{"error": "Invalid role value"},
status=status.HTTP_400_BAD_REQUEST,
)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/api/plane/app/views/project/member.py` at line 244, The code currently
does new_role = int(request.data.get("role")) which will raise
ValueError/TypeError for non-numeric or missing values; wrap this conversion in
validation: check that request.data.get("role") is not None, attempt int(...)
inside a try/except catching ValueError and TypeError, and on error return a
proper 400 validation response (or raise
rest_framework.exceptions.ValidationError) with a clear message instead of
letting a 500 propagate; reference the new_role variable and the
request.data.get("role") conversion when applying the change.


# Cannot assign a role equal to or higher than your own
if new_role >= requested_project_member.role and not is_workspace_admin:
return Response(
Comment on lines +244 to +248
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new_role = int(request.data.get("role")) can raise ValueError/TypeError (e.g., role is "", null, or a non-numeric string), resulting in a 500 before serializer validation runs. Handle coercion defensively (catch conversion errors and return a 400 with a validation-style error), or rely on the serializer’s validated value instead of casting directly from request.data.

Copilot uses AI. Check for mistakes.
{"error": "You cannot assign a role equal to or higher than your own"},
status=status.HTTP_403_FORBIDDEN,
)

# Cannot assign a role higher than the target's workspace role
if workspace_role in [5] and new_role in [15, 20]:
return Response(
{"error": "You cannot add a user with role higher than the workspace role"},
status=status.HTTP_400_BAD_REQUEST,
)

Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workspace-role constraint enforced during create() (workspace admins cannot be added with a lower project role) isn’t enforced here: a workspace admin target could be updated to Member/Guest, which appears to violate the invariant enforced at creation time. If that invariant is required, add the symmetric lower-bound check for workspace_role == ROLE.ADMIN.value during role updates as well (or document why updates are allowed to diverge).

Suggested change
# Enforce invariant: workspace admins must not have a lower project role
if is_workspace_admin and new_role < ROLE.ADMIN.value:
return Response(
{"error": "You cannot set a workspace admin's project role lower than admin"},
status=status.HTTP_400_BAD_REQUEST,
)

Copilot uses AI. Check for mistakes.
serializer = ProjectMemberSerializer(project_member, data=request.data, partial=True)

Expand Down
Loading