[WEB-8110] fix: sanitize page list order_by against an allowlist (#9387)

* [WEB-8110] fix: sanitize page list order_by against an allowlist (GHSA-2v48)

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 <noreply@plane.so>

* [WEB-8110] fix: fold sanitized order_by into a single order_by() call (review)

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", <sanitized>, "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 <noreply@plane.so>

* chore(security): drop advisory identifiers from code comments

Explanations kept unchanged; only the IDs are removed.

Co-authored-by: Plane AI <noreply@plane.so>

---------

Co-authored-by: Plane AI <noreply@plane.so>

apps/api/plane/app/views/page/base.py

@@ -46,6 +46,7 @@ from plane.db.models import (
     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,9 +101,23 @@ class PageViewSet(BaseViewSet):
             .select_related("workspace")
             .select_related("owned_by")
             .annotate(is_favorite=Exists(subquery))
-            .order_by(self.request.GET.get("order_by", "-created_at"))
             .prefetch_related("labels")
-            .order_by("-is_favorite", "-created_at")
+            # 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. 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",
+            )
             .annotate(
                 project=Exists(
                     ProjectPage.objects.filter(page_id=OuterRef("id"), project_id=self.kwargs.get("project_id"))

apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py

@@ -0,0 +1,93 @@
+# 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 page order_by ORM injection (WEB-8110).
+
+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
+
+    @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"]

apps/api/plane/utils/order_queryset.py

@@ -12,8 +12,7 @@ STATE_ORDER = ["backlog", "unstarted", "started", "completed", "cancelled"]
 # order_by allowlists — one per model/endpoint family
 # All contain bare field names (no leading '-'); the sanitizer strips the
 # prefix before looking up, so descending variants are implicitly covered.
-# Prevents ORM order_by injection via user-supplied query params
-# (GHSA-2r95-c453-vxmr / GHSA-w45q-6m65-9498).
+# Prevents ORM order_by injection via user-supplied query params.
 # ---------------------------------------------------------------------------
 
 ISSUE_ORDER_BY_ALLOWLIST = frozenset({
@@ -76,12 +75,20 @@ NOTIFICATION_ORDER_BY_ALLOWLIST = frozenset({
     "updated_at",
 })
 
+# Page list queryset.
+PAGE_ORDER_BY_ALLOWLIST = frozenset({
+    "created_at",
+    "updated_at",
+    "name",
+    "sort_order",
+})
+
 # ---------------------------------------------------------------------------
 # group_by / sub_group_by allowlist for Issue querysets — used by
 # GroupedOffsetPaginator / SubGroupedOffsetPaginator (plane/utils/paginator.py),
 # which pass the field name straight into F(), .values(), .order_by(), and
 # Window partition_by. Prevents unauthenticated ORM field-name injection via
-# user-supplied query params (GHSA-wwgj-929g-42cm).
+# user-supplied query params.
 # ---------------------------------------------------------------------------
 ISSUE_GROUP_BY_ALLOWLIST = frozenset({
     "state_id",
@@ -118,6 +125,7 @@ MODULE_ORDER_BY_ALLOWLIST = frozenset({
     "sort_order",
 })
 
+
 def sanitize_order_by(value, allowed_fields, default="-created_at"):
     """Return a safe ordering string derived from *value*.