[INFRA-774] fix(security): scope WorkSpaceMemberSerializer to block cross-workspace member reassignment via PATCH by mguptahub · Pull Request #9705 · makeplane/plane

mguptahub · 2026-08-28T05:48:57Z

Summary

WorkSpaceMemberSerializer declared fields = "__all__" with no read_only_fields, so DRF auto-generated a writable workspace FK. WorkSpaceMemberViewSet.partial_update passes raw request.data straight into the serializer with no scrubbing.
A workspace ADMIN could PATCH any other active member's WorkspaceMember row with a workspace field pointing at a foreign workspace's UUID — moving that row (with whatever role was also in the body, e.g. 20/admin) into the foreign workspace. No invitation flow, no consent from the target workspace's owner, no audit trail.
Fix: add workspace/member (plus the usual created_by/updated_by/created_at/updated_at) to read_only_fields on WorkSpaceMemberSerializer, matching the pattern already used on WorkSpaceSerializer right above it in the same file. Applied the same fix to the sibling WorkspaceMemberMeSerializer/WorkspaceMemberAdminSerializer for defense-in-depth — same model, same footgun shape, even though today they're only ever instantiated read-only.
Incidental: one pre-existing line in the same file (validate_name's raise statement) got reformatted by ruff format — unrelated to this fix, kept since it matches the project's canonical style.

Test plan

New regression tests in apps/api/plane/tests/contract/app/test_workspace_member_cross_tenant_reassignment.py: admin cannot move another member's row into a foreign workspace via PATCH (with a positive control that the legitimate role-only PATCH still works)
Verified fail-before/pass-after via git stash on the fix — the cross-workspace-move test fails against the pre-fix code (row actually moves), passes after
Related workspace serializer/view test suites still green (45 passed)
ruff check / ruff format --check clean

Summary by CodeRabbit

Bug Fixes
- Prevented workspace membership records from being reassigned to a different workspace through updates.
- Protected workspace, member, audit, activity, and deletion details from modification during membership updates.
- Legitimate role updates continue to work as expected.
- Prevented membership status or deletion metadata from bypassing safety checks.
Tests
- Added regression coverage for cross-workspace reassignment and unauthorized field updates.
- Verified that valid role changes remain successful while protected changes are rejected.

…er on WorkSpaceMemberSerializer WorkSpaceMemberSerializer declared fields = "__all__" with no read_only_fields, so DRF auto-generated a writable workspace FK. WorkSpaceMemberViewSet.partial_update passes raw request.data straight into the serializer with no scrubbing, so a workspace ADMIN could PATCH any other active member's row with a workspace field pointing at a foreign workspace's UUID — moving that row (with whatever role was also in the body) into the foreign workspace with no invitation, no consent from its owner, and no audit trail. Add workspace/member (plus the usual created_by/updated_by/created_at/ updated_at) to read_only_fields on WorkSpaceMemberSerializer and its siblings WorkspaceMemberMeSerializer/WorkspaceMemberAdminSerializer — same model, same footgun shape, even though the latter two are only ever instantiated read-only today. Co-authored-by: Plane AI <noreply@plane.so>

coderabbitai · 2026-08-28T05:50:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c583a06-7f77-4094-961a-e2255cd62a59

📥 Commits

Reviewing files that changed from the base of the PR and between 926e84c and 07739f2.

📒 Files selected for processing (2)

apps/api/plane/app/serializers/workspace.py
apps/api/plane/tests/contract/app/test_workspace_member_cross_tenant_reassignment.py

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Walkthrough

Workspace-member serializers now share read-only field configuration for workspace, member, audit fields, is_active, and deleted_at. Contract tests verify that protected PATCH fields remain unchanged while valid role updates continue to succeed.

Changes

Workspace member protection

Layer / File(s)	Summary
Restrict workspace-member serializer fields `apps/api/plane/app/serializers/workspace.py`	The shared read-only field constant now covers protected membership and state fields. All three workspace-member serializers use the constant. The name validation message is reformatted without behavioral change.
Test protected workspace-member PATCH behavior `apps/api/plane/tests/contract/app/test_workspace_member_cross_tenant_reassignment.py`	Contract tests verify that foreign workspace values, `is_active`, and `deleted_at` do not change membership state while role-only updates still apply.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 07739

The change narrows workspace-member updates to prevent cross-workspace reassignment while preserving legitimate role changes, with regression tests covering the protected behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: dheeru0198

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the security fix that blocks cross-workspace member reassignment through PATCH.
Description check	✅ Passed	The description clearly explains the vulnerability, fix, scope, regression tests, and verification results. It does not use all template headings, but it provides the required core information and rem…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description clearly explains the vulnerability, fix, scope, regression tests, and verification results. It does not use all template headings, but it provides the required core information and remains complete and on-topic.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch infra-774/workspace-member-scope-to-caller

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

makeplane · 2026-08-28T05:50:36Z

Linked to Plane Work Item(s)

(INFRA-774) fix(security): scope WorkSpaceMemberSerializer to block cross-workspace member reassignment via PATCH

This comment was auto-generated by Plane

Address /code-review finding on PR #9705: the identical 7-entry read_only_fields list was duplicated verbatim across all three WorkspaceMember-backed serializers. Extracted to WORKSPACE_MEMBER_READ_ONLY_FIELDS so a future field addition/removal can't silently drift between them and reopen the same write-scoping gap. Co-authored-by: Plane AI <noreply@plane.so>

mguptahub · 2026-08-28T05:53:18Z

Addressed the /code-review finding in 926e84c: extracted the identical 7-entry read_only_fields list (duplicated verbatim across all three WorkspaceMember-backed serializers) into a shared WORKSPACE_MEMBER_READ_ONLY_FIELDS constant, so a future field addition/removal can't silently drift between them and reopen the same gap.

No correctness issues were found — the review traced every call site of all three serializers and confirmed partial_update is the only write path, and verified the id/member overlap between explicit field declarations and read_only_fields is a DRF no-op, not a crash.

Related workspace serializer/view suites still green (45 passed) after the refactor.

Copilot

Pull request overview

This PR hardens the Django REST Framework workspace member PATCH endpoint to prevent cross-workspace reassignment of WorkspaceMember rows (a cross-tenant privilege escalation vector), and adds a regression test to ensure the issue does not recur.

Changes:

Marked workspace and member (and standard audit fields) as read-only on WorkSpaceMemberSerializer and related serializers to prevent cross-workspace row moves via PATCH.
Added a contract regression test covering the attempted cross-workspace move plus a positive control ensuring role-only PATCH still works.
Applied incidental ruff format reflow to a pre-existing validation error raise.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
apps/api/plane/app/serializers/workspace.py	Makes key `WorkspaceMember` FK fields read-only to prevent cross-tenant member reassignment via PATCH.
apps/api/plane/tests/contract/app/test_workspace_member_cross_tenant_reassignment.py	Adds regression coverage for the cross-workspace PATCH exploit and validates legitimate role updates still succeed.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…rializers Address Copilot review findings on PR #9705: - WorkSpaceMemberSerializer (and its siblings) also exposed is_active and deleted_at as writable via fields = "__all__". Every legitimate place that flips these fields (WorkSpaceMemberViewSet.destroy/.leave, invite acceptance) does so via direct model-field assignment, never through this serializer — so this was a side channel letting an admin PATCH around destroy()'s own safety checks (self-removal, role-outranking, last-project-admin orphaning) and its ProjectMember deactivation cascade. deleted_at is worse: the default manager filters on it, so setting it directly silently vanishes the row from every normal queryset with a forgeable timestamp and no audit trail. - Added both fields to WORKSPACE_MEMBER_READ_ONLY_FIELDS. Two new regression tests, fail-before verified (both fail against the pre-this-commit code: is_active flips, and the deleted_at row genuinely vanishes from WorkspaceMember.objects). - Fixed a docstring inaccuracy: partial_update lives on WorkSpaceMemberViewSet, not on the serializer. Co-authored-by: Plane AI <noreply@plane.so>

mguptahub · 2026-08-28T06:14:25Z

Addressed both Copilot findings in 07739f2:

Fixed the docstring inaccuracy (partial_update lives on WorkSpaceMemberViewSet, not the serializer).
Confirmed the is_active/deleted_at mass-assignment gap was real and fixed it: both fields added to WORKSPACE_MEMBER_READ_ONLY_FIELDS across all three WorkspaceMember-backed serializers. Two new regression tests, fail-before verified — the deleted_at one is worth flagging: setting it via PATCH actually made the row disappear from WorkspaceMember.objects entirely (the default manager filters on it), a stealthier bypass than is_active since there's no error, just silent vanishing with a forgeable timestamp.

Full related suite still green (47 passed), ruff clean.

mguptahub requested a review from dheeru0198 as a code owner August 28, 2026 05:48

Copilot AI lite review requested due to automatic review settings August 28, 2026 05:48

mguptahub requested a review from pablohashescobar as a code owner August 28, 2026 05:48

Copilot started reviewing on behalf of mguptahub August 28, 2026 05:49 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread apps/api/plane/tests/contract/app/test_workspace_member_cross_tenant_reassignment.py Outdated

Comment thread apps/api/plane/app/serializers/workspace.py Outdated

Navigation Menu

Uh oh!

[INFRA-774] fix(security): scope WorkSpaceMemberSerializer to block cross-workspace member reassignment via PATCH - #9705

[INFRA-774] fix(security): scope WorkSpaceMemberSerializer to block cross-workspace member reassignment via PATCH#9705
mguptahub wants to merge 3 commits into
previewfrom
infra-774/workspace-member-scope-to-caller

mguptahub commented Aug 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 28, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

makeplane Bot commented Aug 28, 2026

Uh oh!

mguptahub commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

mguptahub commented Aug 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mguptahub commented Aug 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

makeplane Bot commented Aug 28, 2026

Uh oh!

mguptahub commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

mguptahub commented Aug 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mguptahub commented Aug 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 28, 2026 •

edited

Loading