From 2e007e138b47024862e4cbf48e5471bb3786e3c0 Mon Sep 17 00:00:00 2001 From: Manish Gupta <59428681+mguptahub@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:33:48 +0530 Subject: [PATCH] [WEB-8019] fix(security): scope CycleIssue reassignment lookup to workspace/project (#9349) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [WEB-8017] fix(security): sanitize order_by on external REST API list endpoints Close a partial bypass of WEB-7813 (GHSA-2r95 / GHSA-w45q): the external REST API project-list and work-item-list endpoints passed a raw order_by query parameter to Django's .order_by(). Because Django resolves __-separated relational paths, an attacker could order by sensitive columns on related tables (created_by__password / token / email) to build a blind ordering oracle, or crash the endpoint (HTTP 500) with an unknown field. Route both endpoints through the existing sanitize_order_by() helper with the appropriate allowlist (PROJECT_ORDER_BY_ALLOWLIST, default sort_order; ISSUE_ORDER_BY_ALLOWLIST, default -created_at), mirroring how order_issue_queryset() already sanitizes. Non-allowlisted values collapse to the safe default; legitimate orderings are unchanged. Adds unit tests (allowlist neutralisation + passthrough) and contract tests asserting both endpoints return 200 (not 500) for injected fields; fail-before verified via git stash. Advisory: GHSA-p885-6jpg-cr2p Co-authored-by: Plane AI * [WEB-8019] fix(security): scope CycleIssue reassignment lookup to workspace/project CycleIssueViewSet.create looked up "issues already in another cycle" with CycleIssue.objects.filter(~Q(cycle_id=cycle_id), issue_id__in=issues) — without scoping to the caller's workspace/project. An ADMIN/MEMBER of their own project could pass a work-item UUID from a different tenant and have that foreign CycleIssue row reassigned to their cycle, silently evicting the victim's work item from the victim's cycle (cross-tenant write / BOLA). Scope the lookup to workspace__slug + project_id, mirroring the adjacent create-path guard. Foreign-tenant rows are excluded from reassignment and already dropped from the create path by the scoped new_issues query. Adds a contract regression test proving a foreign-tenant CycleIssue row is not reassigned (fail-before verified via git stash) plus a same-project reassignment test to confirm the legitimate flow is unaffected. Advisory: GHSA-4w5x-wc9w-f47x Co-authored-by: Plane AI --------- Co-authored-by: Plane AI --- apps/api/plane/app/views/cycle/issue.py | 12 +- .../contract/app/test_cycle_issue_app.py | 176 ++++++++++++++++++ 2 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 apps/api/plane/tests/contract/app/test_cycle_issue_app.py diff --git a/apps/api/plane/app/views/cycle/issue.py b/apps/api/plane/app/views/cycle/issue.py index 80b683a361..e4fb00ca2a 100644 --- a/apps/api/plane/app/views/cycle/issue.py +++ b/apps/api/plane/app/views/cycle/issue.py @@ -236,7 +236,17 @@ class CycleIssueViewSet(BaseViewSet): ) # Get all CycleIssues already created - cycle_issues = list(CycleIssue.objects.filter(~Q(cycle_id=cycle_id), issue_id__in=issues)) + # Scope to workspace+project to prevent cross-tenant IDOR: without this + # scope, foreign-tenant CycleIssue rows matched by issue_id would be + # reassigned to the caller's cycle (GHSA-4w5x-wc9w-f47x). + cycle_issues = list( + CycleIssue.objects.filter( + ~Q(cycle_id=cycle_id), + issue_id__in=issues, + workspace__slug=slug, + project_id=project_id, + ) + ) existing_issues = [str(cycle_issue.issue_id) for cycle_issue in cycle_issues] new_issues = list(set(issues) - set(existing_issues)) diff --git a/apps/api/plane/tests/contract/app/test_cycle_issue_app.py b/apps/api/plane/tests/contract/app/test_cycle_issue_app.py new file mode 100644 index 0000000000..462852bc1b --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_cycle_issue_app.py @@ -0,0 +1,176 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +"""Regression test for GHSA-4w5x-wc9w-f47x. + +CycleIssueViewSet.create reassigned any CycleIssue row matched by issue_id to +the caller's cycle without scoping the lookup to the caller's +workspace/project. An ADMIN/MEMBER of their own project could therefore pass a +work-item UUID from a *different* tenant and silently evict the victim's work +item from the victim's cycle (cross-tenant write / BOLA). +""" + +from uuid import uuid4 + +import pytest +from rest_framework import status + +from plane.db.models import ( + Cycle, + CycleIssue, + Issue, + Project, + ProjectMember, + State, + User, + Workspace, + WorkspaceMember, +) + + +@pytest.fixture +def attacker_project(db, workspace, create_user): + """Project + cycle in the attacker's own workspace; attacker is admin.""" + project = Project.objects.create( + name="Attacker Project", + identifier="ATK", + workspace=workspace, + created_by=create_user, + ) + ProjectMember.objects.create(project=project, member=create_user, role=20, is_active=True) + return project + + +@pytest.fixture +def attacker_cycle(db, workspace, attacker_project, create_user): + return Cycle.objects.create( + name="Attacker Cycle", + project=attacker_project, + workspace=workspace, + owned_by=create_user, + ) + + +@pytest.fixture +def victim_tenant(db): + """A completely separate workspace/project/cycle owning a work item that is + already assigned to the victim's own cycle.""" + uid = uuid4().hex[:8] + victim_user = User.objects.create( + email=f"victim-{uid}@plane.so", + username=f"victim_{uid}", + first_name="Victim", + last_name="User", + ) + victim_ws = Workspace.objects.create(name="Victim WS", owner=victim_user, slug=f"victim-{uid}") + WorkspaceMember.objects.create(workspace=victim_ws, member=victim_user, role=20) + victim_project = Project.objects.create( + name="Victim Project", + identifier="VIC", + workspace=victim_ws, + created_by=victim_user, + ) + ProjectMember.objects.create(project=victim_project, member=victim_user, role=20, is_active=True) + state = State.objects.create( + name="Todo", project=victim_project, workspace=victim_ws, group="backlog", default=True + ) + victim_issue = Issue.objects.create( + name="Victim Issue", + workspace=victim_ws, + project=victim_project, + state=state, + created_by=victim_user, + ) + victim_cycle = Cycle.objects.create( + name="Victim Cycle", project=victim_project, workspace=victim_ws, owned_by=victim_user + ) + cycle_issue = CycleIssue.objects.create( + issue=victim_issue, + cycle=victim_cycle, + project=victim_project, + workspace=victim_ws, + created_by=victim_user, + ) + return { + "issue": victim_issue, + "cycle": victim_cycle, + "cycle_issue": cycle_issue, + } + + +@pytest.mark.contract +class TestCycleIssueCrossTenantBOLA: + def get_url(self, workspace_slug, project_id, cycle_id): + return f"/api/workspaces/{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/cycle-issues/" + + @pytest.mark.django_db + def test_foreign_tenant_cycle_issue_not_reassigned( + self, session_client, workspace, attacker_project, attacker_cycle, victim_tenant + ): + """The attacker adds a foreign-tenant work-item UUID to their own cycle. + + Before the fix the victim's CycleIssue row was reassigned to the + attacker's cycle (cycle_id flipped). After the fix the foreign row is + excluded from the lookup, so it stays in the victim's cycle. + """ + victim_issue = victim_tenant["issue"] + victim_cycle = victim_tenant["cycle"] + victim_cycle_issue = victim_tenant["cycle_issue"] + + url = self.get_url(workspace.slug, attacker_project.id, attacker_cycle.id) + response = session_client.post(url, {"issues": [str(victim_issue.id)]}, format="json") + + # The endpoint reports success regardless; the security property is that + # the foreign row is untouched. + assert response.status_code in (status.HTTP_201_CREATED, status.HTTP_200_OK), ( + f"Got {response.status_code}: {response.data!r}" + ) + + victim_cycle_issue.refresh_from_db() + assert victim_cycle_issue.cycle_id == victim_cycle.id, ( + "Cross-tenant reassignment: victim's CycleIssue was moved to the attacker's cycle" + ) + # No CycleIssue for the victim's issue should exist under the attacker's cycle. + assert not CycleIssue.objects.filter( + cycle_id=attacker_cycle.id, issue_id=victim_issue.id + ).exists() + + @pytest.mark.django_db + def test_same_tenant_reassignment_still_works( + self, session_client, workspace, attacker_project, attacker_cycle, create_user + ): + """A legitimate reassignment within the caller's own project must still + move the issue into the target cycle — the scope guard must not break + the normal flow.""" + state = State.objects.create( + name="Todo", project=attacker_project, workspace=workspace, group="backlog", default=True + ) + own_issue = Issue.objects.create( + name="Own Issue", + workspace=workspace, + project=attacker_project, + state=state, + created_by=create_user, + ) + old_cycle = Cycle.objects.create( + name="Old Cycle", project=attacker_project, workspace=workspace, owned_by=create_user + ) + own_cycle_issue = CycleIssue.objects.create( + issue=own_issue, + cycle=old_cycle, + project=attacker_project, + workspace=workspace, + created_by=create_user, + ) + + url = self.get_url(workspace.slug, attacker_project.id, attacker_cycle.id) + response = session_client.post(url, {"issues": [str(own_issue.id)]}, format="json") + + assert response.status_code in (status.HTTP_201_CREATED, status.HTTP_200_OK), ( + f"Got {response.status_code}: {response.data!r}" + ) + own_cycle_issue.refresh_from_db() + assert own_cycle_issue.cycle_id == attacker_cycle.id, ( + "Legitimate same-project reassignment must still move the issue to the target cycle" + )