fix(api): use requester's workspace role in project member role updates (GHSA-x63v-p7wc-47x4)#9014
Conversation
…tes (GHSA-x63v-p7wc-47x4) is_workspace_admin in ProjectMemberViewSet.partial_update was derived from the target member's workspace role, not the requester's. When the target happened to be a workspace admin, all three project-role guards (L231/238/247) were bypassed regardless of who was making the request, allowing a non-admin requester to re-role a workspace admin's project membership. Compute is_workspace_admin from the requester instead and keep the target's workspace role under a distinct name for the existing new-role-vs-workspace-role cap.
sriramveeraghanta
requested review from
dheeru0198 and
pablohashescobar
as code owners
📝 WalkthroughWalkthroughThe ChangesAuthorization Logic Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes an authorization bug in the project member role update endpoint where is_workspace_admin was incorrectly derived from the target member’s workspace role rather than the requester’s, allowing project-role guard checks to be bypassed when the target happened to be a workspace admin (GHSA-x63v-p7wc-47x4).
Changes:
- Compute
is_workspace_adminbased on the requester’sWorkspaceMember.roleinstead of the target’s. - Rename the target role lookup to
target_workspace_roleand continue using it to cap disallowed project role assignments for workspace guests.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/plane/app/views/project/member.py (1)
258-262: ⚡ Quick winAdd symmetric workspace-role constraint to partial_update method.
The constraint at lines 258-262 correctly prevents workspace guests (role=5) from being promoted to higher project roles. However, the
createmethod (lines 73-77) prevents workspace admins (role=20) from assigning lower project roles to members, whilepartial_updatelacks a symmetric constraint preventing workspace admins from being demoted.Consider adding:
if target_workspace_role in [20] and new_role in [5, 15]: return Response( {"error": "You cannot assign a role lower than the workspace role"}, status=status.HTTP_400_BAD_REQUEST, )This ensures workspace admins cannot have their project role demoted in the same way guests cannot be improperly promoted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/plane/app/views/project/member.py` around lines 258 - 262, The partial_update method currently blocks promoting workspace guests (target_workspace_role == 5) to higher project roles but lacks the symmetric check present in create; add a check in partial_update that if target_workspace_role == 20 and new_role in [5, 15] it returns Response({"error": "You cannot assign a role lower than the workspace role"}, status=status.HTTP_400_BAD_REQUEST) so workspace admins cannot be demoted to lower project roles; use the same Response and status.HTTP_400_BAD_REQUEST pattern as the existing constraint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/api/plane/app/views/project/member.py`:
- Around line 258-262: The partial_update method currently blocks promoting
workspace guests (target_workspace_role == 5) to higher project roles but lacks
the symmetric check present in create; add a check in partial_update that if
target_workspace_role == 20 and new_role in [5, 15] it returns
Response({"error": "You cannot assign a role lower than the workspace role"},
status=status.HTTP_400_BAD_REQUEST) so workspace admins cannot be demoted to
lower project roles; use the same Response and status.HTTP_400_BAD_REQUEST
pattern as the existing constraint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf28a5f8-c900-4b1f-af82-7a3d0cb9862b
📒 Files selected for processing (1)
apps/api/plane/app/views/project/member.py
sriramveeraghanta
deleted the
fix/project-member-role-update-workspace-admin-subject
branch
Summary
ProjectMemberViewSet.partial_updatederivedis_workspace_adminfrom the target member's workspace role, not the requester's. When the target happened to be a workspace admin, all three project-role guards (lines 231 / 238 / 247) were bypassed regardless of who made the request — letting a non-admin requester re-role a workspace admin's project membership.is_workspace_adminfrom the requester (request.user) and renames the target lookup totarget_workspace_role, which is still used to capnew_roleagainst the target's workspace role.Trace check
{"role": 15}(advisory PoC){"role": 20}Test plan
PATCH /workspaces/{slug}/projects/{id}/members/{pk}/against a target whose workspace role is Admin — confirm 403.Summary by CodeRabbit