From 96b69d071cb9ac060000ed96ec00ac44aff58f58 Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Thu, 9 Jul 2026 17:50:39 +0530 Subject: [PATCH 1/2] [WEB-8110] fix: sanitize page list order_by against an allowlist (GHSA-2v48) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PageViewSet.get_queryset passed the raw order_by query param into .order_by(). In Django 4.2 .order_by() resolves field names at call time, so an unknown field (e.g. order_by=password) raises FieldError → 500 DoS, and a valid relation path (e.g. order_by=owned_by__password) enables ORM relational traversal (GHSA-2v48-qcjw-74ch). Add PAGE_ORDER_BY_ALLOWLIST to utils/order_queryset.py and wrap the param with the existing sanitize_order_by() before it reaches .order_by(), matching the issue/project/view/notification endpoints. Unknown or malformed values fall back to the safe -created_at default. Covers only the app project-pages residual; the 3 external-REST-API sites in the advisory are handled by PR #9348. EE Wiki counterpart: WEB-8111. Add contract regression tests (fail-before verified). Co-authored-by: Plane AI --- apps/api/plane/app/views/page/base.py | 14 +++- .../app/test_page_order_by_allowlist_app.py | 69 +++++++++++++++++++ apps/api/plane/utils/order_queryset.py | 8 +++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py diff --git a/apps/api/plane/app/views/page/base.py b/apps/api/plane/app/views/page/base.py index ec391afc1aa..8d594fc802f 100644 --- a/apps/api/plane/app/views/page/base.py +++ b/apps/api/plane/app/views/page/base.py @@ -46,6 +46,7 @@ UserRecentVisit, ) from plane.utils.error_codes import ERROR_CODES +from plane.utils.order_queryset import PAGE_ORDER_BY_ALLOWLIST, sanitize_order_by # Local imports from ..base import BaseAPIView, BaseViewSet @@ -100,7 +101,18 @@ def get_queryset(self): .select_related("workspace") .select_related("owned_by") .annotate(is_favorite=Exists(subquery)) - .order_by(self.request.GET.get("order_by", "-created_at")) + # Sanitize the user-supplied order_by against an allowlist before it + # reaches .order_by(): Django resolves the field at call time, so an + # unknown field raises FieldError (500 DoS) and a relation path + # (e.g. owned_by__password) enables ORM relational traversal + # (GHSA-2v48). + .order_by( + sanitize_order_by( + self.request.GET.get("order_by", "-created_at"), + PAGE_ORDER_BY_ALLOWLIST, + default="-created_at", + ) + ) .prefetch_related("labels") .order_by("-is_favorite", "-created_at") .annotate( diff --git a/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py b/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py new file mode 100644 index 00000000000..5c2358adc7b --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py @@ -0,0 +1,69 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +""" +Regression tests for GHSA-2v48-qcjw-74ch (page order_by ORM injection). + +PageViewSet.get_queryset passed the raw order_by query param into .order_by(), +which resolves field names at call time — an unknown field raised FieldError +(500 DoS) and a relation path (e.g. owned_by__password) enabled ORM relational +traversal. The param is now sanitized against PAGE_ORDER_BY_ALLOWLIST. +""" + +import pytest +from rest_framework import status + +from plane.db.models import Page, Project, ProjectMember, ProjectPage + + +@pytest.fixture +def project_with_page(db, workspace, create_user): + project = Project.objects.create(name="P", identifier="PRD", workspace=workspace) + ProjectMember.objects.create( + workspace=workspace, project=project, member=create_user, role=20, is_active=True + ) + page = Page.objects.create(workspace=workspace, owned_by=create_user, access=Page.PUBLIC_ACCESS, name="pg") + ProjectPage.objects.create(workspace=workspace, project=project, page=page) + return project, page + + +def _pages_url(slug, project_id): + return f"/api/workspaces/{slug}/projects/{project_id}/pages/" + + +@pytest.mark.contract +class TestPageOrderByAllowlist: + @pytest.mark.django_db + @pytest.mark.parametrize( + "order_by", + [ + "password", # invalid field → FieldError (500) pre-fix + "bogus__field__x", # invalid relation path → FieldError (500) pre-fix + "owned_by__password", # valid relation path → ORM traversal pre-fix + ], + ) + def test_malicious_order_by_is_rejected(self, session_client, workspace, project_with_page, order_by): + project, _ = project_with_page + + response = session_client.get(_pages_url(workspace.slug, project.id), {"order_by": order_by}) + + # Sanitized to the safe default — no 500, no traversal. + assert response.status_code == status.HTTP_200_OK + + @pytest.mark.django_db + @pytest.mark.parametrize("order_by", ["name", "-name", "created_at", "-created_at", "updated_at", "sort_order"]) + def test_allowlisted_order_by_is_accepted(self, session_client, workspace, project_with_page, order_by): + project, _ = project_with_page + + response = session_client.get(_pages_url(workspace.slug, project.id), {"order_by": order_by}) + + assert response.status_code == status.HTTP_200_OK + + @pytest.mark.django_db + def test_no_order_by_param_defaults_ok(self, session_client, workspace, project_with_page): + project, _ = project_with_page + + response = session_client.get(_pages_url(workspace.slug, project.id)) + + assert response.status_code == status.HTTP_200_OK diff --git a/apps/api/plane/utils/order_queryset.py b/apps/api/plane/utils/order_queryset.py index 1ef2b056894..769cff140a5 100644 --- a/apps/api/plane/utils/order_queryset.py +++ b/apps/api/plane/utils/order_queryset.py @@ -76,6 +76,14 @@ "updated_at", }) +# Page list queryset. +PAGE_ORDER_BY_ALLOWLIST = frozenset({ + "created_at", + "updated_at", + "name", + "sort_order", +}) + def sanitize_order_by(value, allowed_fields, default="-created_at"): """Return a safe ordering string derived from *value*. From 0c8e1152d0b39464e494cfbbc3623677b98337fb Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Fri, 10 Jul 2026 10:15:31 +0530 Subject: [PATCH 2/2] [WEB-8110] fix: fold sanitized order_by into a single order_by() call (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit + Copilot on #9387: the sanitized .order_by(user) was immediately overridden by a later .order_by("-is_favorite", "-created_at"), so the order_by param had no effect on the result (dead code) and cost an extra query-build step. Merge them into one .order_by("-is_favorite", , "id") — matching the EE project-pages viewset — so favourites stay pinned first, the allowlisted user ordering actually applies as the secondary sort, and id is a stable pagination tiebreak. The no-param default is unchanged (-created_at). Add a test asserting order_by=name / -name actually reorders the results. Co-authored-by: Plane AI --- apps/api/plane/app/views/page/base.py | 19 ++++++++------- .../app/test_page_order_by_allowlist_app.py | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/apps/api/plane/app/views/page/base.py b/apps/api/plane/app/views/page/base.py index 8d594fc802f..b33466c6af8 100644 --- a/apps/api/plane/app/views/page/base.py +++ b/apps/api/plane/app/views/page/base.py @@ -101,20 +101,23 @@ def get_queryset(self): .select_related("workspace") .select_related("owned_by") .annotate(is_favorite=Exists(subquery)) - # Sanitize the user-supplied order_by against an allowlist before it - # reaches .order_by(): Django resolves the field at call time, so an - # unknown field raises FieldError (500 DoS) and a relation path - # (e.g. owned_by__password) enables ORM relational traversal - # (GHSA-2v48). + .prefetch_related("labels") + # Sanitize the user-supplied order_by against an allowlist: Django + # resolves the field at call time, so an unknown field raises + # FieldError (500 DoS) and a relation path (e.g. owned_by__password) + # enables ORM relational traversal (GHSA-2v48). Favourites stay + # pinned first; the sanitized user ordering is the secondary sort + # (a single .order_by() so it is not overridden), with id as a + # stable tiebreak for pagination. .order_by( + "-is_favorite", sanitize_order_by( self.request.GET.get("order_by", "-created_at"), PAGE_ORDER_BY_ALLOWLIST, default="-created_at", - ) + ), + "id", ) - .prefetch_related("labels") - .order_by("-is_favorite", "-created_at") .annotate( project=Exists( ProjectPage.objects.filter(page_id=OuterRef("id"), project_id=self.kwargs.get("project_id")) diff --git a/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py b/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py index 5c2358adc7b..16a049be99f 100644 --- a/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py +++ b/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py @@ -67,3 +67,27 @@ def test_no_order_by_param_defaults_ok(self, session_client, workspace, project_ response = session_client.get(_pages_url(workspace.slug, project.id)) assert response.status_code == status.HTTP_200_OK + + @pytest.mark.django_db + def test_allowlisted_order_by_actually_orders_results(self, session_client, workspace, create_user): + """An allowlisted order_by must actually affect the result ordering — + guards against the param being silently overridden by a later + .order_by() call.""" + project = Project.objects.create(name="P2", identifier="ORD", workspace=workspace) + ProjectMember.objects.create( + workspace=workspace, project=project, member=create_user, role=20, is_active=True + ) + # Non-favorite public pages so the favourite-first primary sort is a + # no-op and the secondary (name) ordering is observable. + for name in ("Gamma", "Alpha", "Beta"): + page = Page.objects.create( + workspace=workspace, owned_by=create_user, access=Page.PUBLIC_ACCESS, name=name + ) + ProjectPage.objects.create(workspace=workspace, project=project, page=page) + + asc = session_client.get(_pages_url(workspace.slug, project.id), {"order_by": "name"}) + desc = session_client.get(_pages_url(workspace.slug, project.id), {"order_by": "-name"}) + + assert asc.status_code == status.HTTP_200_OK + assert [p["name"] for p in asc.json()] == ["Alpha", "Beta", "Gamma"] + assert [p["name"] for p in desc.json()] == ["Gamma", "Beta", "Alpha"]