Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions apps/api/plane/tests/unit/utils/test_order_queryset.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Copyright (c) 2023-present Plane Software, Inc. and contributors
# SPDX-License-Identifier: AGPL-3.0-only
# See the LICENSE file for details.

import pytest

from plane.utils.order_queryset import ISSUE_GROUP_BY_ALLOWLIST, sanitize_order_by


@pytest.mark.unit
class TestIssueGroupByAllowlist:
"""Regression tests for GHSA-wwgj-929g-42cm.

ISSUE_GROUP_BY_ALLOWLIST must contain exactly the field names that
`issue_group_values()` (plane/utils/grouper.py and
plane/space/utils/grouper.py) knows how to resolve safely. Anything
outside this set must never reach the ORM as a raw field name via
GroupedOffsetPaginator/SubGroupedOffsetPaginator.
"""

def test_allowlist_matches_known_safe_group_fields(self):
expected = {
"state_id",
"state__group",
"priority",
"labels__id",
"assignees__id",
"issue_module__module_id",
"cycle_id",
"project_id",
"created_by",
"target_date",
"start_date",
}
assert ISSUE_GROUP_BY_ALLOWLIST == frozenset(expected)

def test_allowlist_rejects_injection_style_fields(self):
# These are exactly the kind of values GHSA-wwgj-929g-42cm's PoC used
# to trigger a 500 / force a blind relational-traversal oracle.
dangerous_values = [
"not_a_field",
"created_by__password",
"workspace__secret_key",
"assignees__password",
"id",
"",
None,
]
for value in dangerous_values:
assert value not in ISSUE_GROUP_BY_ALLOWLIST


@pytest.mark.unit
class TestSanitizeOrderByStillWorks:
"""Sanity check that the pre-existing order_by sanitizer (GHSA-2r95 /
GHSA-w45q) is untouched by this change."""

def test_valid_field_passes_through(self):
allowed = frozenset({"created_at", "priority"})
assert sanitize_order_by("priority", allowed) == "priority"
assert sanitize_order_by("-created_at", allowed) == "-created_at"

def test_invalid_field_falls_back_to_default(self):
allowed = frozenset({"created_at"})
assert sanitize_order_by("not_a_field", allowed, default="-created_at") == "-created_at"
123 changes: 123 additions & 0 deletions apps/api/plane/tests/unit/utils/test_paginator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
# Copyright (c) 2023-present Plane Software, Inc. and contributors
# SPDX-License-Identifier: AGPL-3.0-only
# See the LICENSE file for details.

import pytest
from django.test import RequestFactory
from rest_framework.exceptions import ParseError
from rest_framework.request import Request

from plane.utils.paginator import BasePaginator, Cursor, CursorResult


class _StubGroupedPaginator:
"""Stand-in for GroupedOffsetPaginator/SubGroupedOffsetPaginator that
records its constructor kwargs without touching the DB/ORM. The security
property under test lives entirely in BasePaginator.paginate() — by the
time a real paginator class would run a query, the field name has
already been validated (or rejected)."""

def __init__(self, **kwargs):
self.kwargs = kwargs

def get_result(self, limit, cursor):
return CursorResult(
results=[],
next=Cursor(limit, 1, False, False),
prev=Cursor(limit, -1, True, False),
hits=0,
max_hits=0,
)

def process_results(self, results):
return results


def _make_request(**params):
django_request = RequestFactory().get("/fake-url/", data=params)
return Request(django_request)


@pytest.mark.unit
class TestPaginateGroupByValidation:
"""Regression tests for GHSA-wwgj-929g-42cm.

BasePaginator.paginate() is the single chokepoint all group_by/sub_group_by
call sites (public Spaces endpoint + 5 authenticated issue/cycle/module/
workspace endpoints) funnel through. It must reject any field name outside
ISSUE_GROUP_BY_ALLOWLIST with a 400 (ParseError), before the value ever
reaches a real paginator's F()/.values()/.order_by()/Window partition_by.
"""

def test_invalid_group_by_raises_parse_error(self):
request = _make_request(group_by="created_by__password")
with pytest.raises(ParseError):
BasePaginator().paginate(
request=request,
queryset=None,
paginator_cls=_StubGroupedPaginator,
group_by_field_name="created_by__password",
group_by_fields=[],
count_filter=None,
)

def test_invalid_sub_group_by_raises_parse_error(self):
# A valid group_by paired with an invalid sub_group_by must still be
# rejected — the PoC in the advisory used exactly this combination
# (group_by=state_id&sub_group_by=created_by__password).
request = _make_request(group_by="priority", sub_group_by="workspace__secret_key")
with pytest.raises(ParseError):
BasePaginator().paginate(
request=request,
queryset=None,
paginator_cls=_StubGroupedPaginator,
group_by_field_name="priority",
group_by_fields=[],
sub_group_by_field_name="workspace__secret_key",
sub_group_by_fields=[],
count_filter=None,
)

def test_unrecognised_field_never_reaches_paginator_constructor(self):
# Belt-and-braces: assert the stub paginator is never even
# instantiated for a rejected field name.
request = _make_request(group_by="not_a_field")

class _ExplodingPaginator:
def __init__(self, **kwargs):
raise AssertionError("paginator_cls must not be constructed for an invalid group_by field")

with pytest.raises(ParseError):
BasePaginator().paginate(
request=request,
queryset=None,
paginator_cls=_ExplodingPaginator,
group_by_field_name="not_a_field",
group_by_fields=[],
count_filter=None,
)

def test_valid_group_by_and_sub_group_by_pass_through(self):
request = _make_request(group_by="priority", sub_group_by="state_id")
response = BasePaginator().paginate(
request=request,
queryset=None,
paginator_cls=_StubGroupedPaginator,
group_by_field_name="priority",
group_by_fields=[],
sub_group_by_field_name="state_id",
sub_group_by_fields=[],
count_filter=None,
)
assert response.data["grouped_by"] == "priority"
assert response.data["sub_grouped_by"] == "state_id"

def test_no_group_by_is_unaffected(self):
# Plain (non-grouped) pagination must not be touched by this fix.
request = _make_request()
response = BasePaginator().paginate(
request=request,
queryset=None,
paginator_cls=_StubGroupedPaginator,
)
assert response.data["grouped_by"] is None
21 changes: 21 additions & 0 deletions apps/api/plane/utils/order_queryset.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,27 @@
"updated_at",
})

# ---------------------------------------------------------------------------
# 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).
# ---------------------------------------------------------------------------
ISSUE_GROUP_BY_ALLOWLIST = frozenset({
"state_id",
"state__group",
"priority",
"labels__id",
"assignees__id",
"issue_module__module_id",
"cycle_id",
"project_id",
"created_by",
"target_date",
"start_date",
})


def sanitize_order_by(value, allowed_fields, default="-created_at"):
"""Return a safe ordering string derived from *value*.
Expand Down
12 changes: 12 additions & 0 deletions apps/api/plane/utils/paginator.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from rest_framework.response import Response

# Module imports
from plane.utils.order_queryset import ISSUE_GROUP_BY_ALLOWLIST


class Cursor:
Expand Down Expand Up @@ -681,11 +682,22 @@ def paginate(

if not paginator:
if group_by_field_name:
# Validate against the allowlist before the field name reaches
# F()/.values()/.order_by()/Window partition_by in the grouped
# paginators below — prevents unauthenticated ORM field-name
# injection via user-supplied group_by/sub_group_by query params
# (GHSA-wwgj-929g-42cm).
if group_by_field_name not in ISSUE_GROUP_BY_ALLOWLIST:
raise ParseError(detail=f"Invalid group_by field: {group_by_field_name}")

paginator_kwargs["group_by_field_name"] = group_by_field_name
paginator_kwargs["group_by_fields"] = group_by_fields
paginator_kwargs["count_filter"] = count_filter

if sub_group_by_field_name:
if sub_group_by_field_name not in ISSUE_GROUP_BY_ALLOWLIST:
raise ParseError(detail=f"Invalid sub_group_by field: {sub_group_by_field_name}")

paginator_kwargs["sub_group_by_field_name"] = sub_group_by_field_name
paginator_kwargs["sub_group_by_fields"] = sub_group_by_fields

Expand Down
Loading