[SECUR-236] fix: grouped-paginator cursor bound + Project mass-assignment

apps/api/plane/app/serializers/cycle.py

@@ -40,7 +40,19 @@ def validate(self, data):
     class Meta:
         model = Cycle
         fields = "__all__"
-        read_only_fields = ["workspace", "project", "owned_by", "archived_at"]
+        # created_by/updated_by are audit fields set server-side by BaseModel.save()
+        # from the request user; with fields="__all__" they are otherwise
+        # client-writable, letting a caller forge attribution on the cycle. save()
+        # never re-stamps them on update, so a supplied value would survive — mark
+        # them read-only so the client value is ignored.
+        read_only_fields = [
+            "workspace",
+            "project",
+            "owned_by",
+            "archived_at",
+            "created_by",
+            "updated_by",
+        ]
 
 
 class CycleSerializer(BaseSerializer):

apps/api/plane/app/serializers/project.py

@@ -34,7 +34,13 @@ class ProjectSerializer(BaseSerializer):
     class Meta:
         model = Project
         fields = "__all__"
-        read_only_fields = ["workspace", "deleted_at"]
+        # created_by/updated_by are audit fields set server-side by BaseModel.save()
+        # from the request user; with fields="__all__" they are otherwise client-writable,
+        # letting a caller forge project ownership/attribution. save() stamps created_by
+        # unconditionally on create, but on update it only stamps updated_by and never
+        # touches created_by — so a client-supplied created_by on a PATCH would survive
+        # untouched. Mark them read-only so the client value is ignored.
+        read_only_fields = ["workspace", "deleted_at", "created_by", "updated_by"]
 
     def validate_name(self, name):
         project_id = self.instance.id if self.instance else None

apps/api/plane/app/serializers/view.py

@@ -59,13 +59,20 @@ class IssueViewSerializer(DynamicBaseSerializer):
     class Meta:
         model = IssueView
         fields = "__all__"
+        # created_by/updated_by are audit fields set server-side by BaseModel.save()
+        # from the request user; with fields="__all__" they are otherwise
+        # client-writable, letting a caller forge attribution on the view. save()
+        # never re-stamps them on update, so a supplied value would survive — mark
+        # them read-only so the client value is ignored.
         read_only_fields = [
             "workspace",
             "project",
             "query",
             "owned_by",
             "access",
             "is_locked",
+            "created_by",
+            "updated_by",
         ]
 
     def create(self, validated_data):

apps/api/plane/tests/unit/serializers/test_mass_assignment.py

@@ -0,0 +1,109 @@
+# Copyright (c) 2023-present Plane Software, Inc. and contributors
+# SPDX-License-Identifier: AGPL-3.0-only
+# See the LICENSE file for details.
+
+"""Regression tests: created_by/updated_by must not be client-forgeable.
+
+BaseModel.save() stamps created_by/updated_by from the current (crum) request
+user; on update it only ever re-stamps updated_by, never created_by. With
+fields="__all__" and no read_only_fields entry, created_by (and, on update,
+updated_by) are ordinary client-writable serializer fields, so a PATCH payload
+can forge who a view/cycle is attributed to. These tests mirror the same gap
+ProjectSerializer was already fixed for, applied to IssueViewSerializer and
+CycleWriteSerializer.
+"""
+
+import pytest
+from crum import set_current_user
+
+from plane.app.serializers.cycle import CycleWriteSerializer
+from plane.app.serializers.view import IssueViewSerializer
+from plane.db.models import Cycle, IssueView, Project, User
+
+
+@pytest.fixture
+def current_user(create_user):
+    """Simulate an authenticated request by populating crum's thread-local
+    current user for the duration of the test — BaseModel.save() reads this
+    to decide who to stamp as updated_by."""
+    set_current_user(create_user)
+    yield create_user
+    set_current_user(None)
+
+
+@pytest.mark.unit
+class TestIssueViewSerializerMassAssignment:
+    """created_by/updated_by must be read-only on IssueViewSerializer."""
+
+    @pytest.mark.django_db
+    def test_created_by_is_not_forgeable_via_update(self, db, workspace, current_user):
+        project = Project.objects.create(name="Test Project", identifier="TESTV", workspace=workspace)
+        attacker = User.objects.create(email="attacker-view@plane.so", username="attacker_view")
+
+        view = IssueView.objects.create(
+            name="Original View",
+            query={},
+            project=project,
+            workspace=workspace,
+            owned_by=current_user,
+        )
+        # BaseModel.save() stamped created_by from the crum current user on
+        # create; pin it explicitly so the assertion below doesn't depend on
+        # that behaviour.
+        IssueView.objects.filter(pk=view.pk).update(created_by=current_user)
+        view.refresh_from_db()
+
+        serializer = IssueViewSerializer(
+            instance=view,
+            data={"name": "Renamed by attacker", "created_by": attacker.id, "updated_by": attacker.id},
+            partial=True,
+        )
+        assert serializer.is_valid(), serializer.errors
+        assert "created_by" not in serializer.validated_data
+        assert "updated_by" not in serializer.validated_data
+
+        saved = serializer.save()
+        saved.refresh_from_db()
+
+        assert saved.name == "Renamed by attacker"
+        assert saved.created_by_id == current_user.id
+        assert saved.created_by_id != attacker.id
+        # updated_by is legitimately stamped from the request user, but must
+        # not be forced to the attacker-supplied value either.
+        assert saved.updated_by_id != attacker.id
+
+
+@pytest.mark.unit
+class TestCycleWriteSerializerMassAssignment:
+    """created_by/updated_by must be read-only on CycleWriteSerializer."""
+
+    @pytest.mark.django_db
+    def test_created_by_is_not_forgeable_via_update(self, db, workspace, current_user):
+        project = Project.objects.create(name="Test Project", identifier="TESTC", workspace=workspace)
+        attacker = User.objects.create(email="attacker-cycle@plane.so", username="attacker_cycle")
+
+        cycle = Cycle.objects.create(
+            name="Original Cycle",
+            project=project,
+            workspace=workspace,
+            owned_by=current_user,
+        )
+        Cycle.objects.filter(pk=cycle.pk).update(created_by=current_user)
+        cycle.refresh_from_db()
+
+        serializer = CycleWriteSerializer(
+            instance=cycle,
+            data={"name": "Renamed by attacker", "created_by": attacker.id, "updated_by": attacker.id},
+            partial=True,
+        )
+        assert serializer.is_valid(), serializer.errors
+        assert "created_by" not in serializer.validated_data
+        assert "updated_by" not in serializer.validated_data
+
+        saved = serializer.save()
+        saved.refresh_from_db()
+
+        assert saved.name == "Renamed by attacker"
+        assert saved.created_by_id == current_user.id
+        assert saved.created_by_id != attacker.id
+        assert saved.updated_by_id != attacker.id

apps/api/plane/tests/unit/utils/test_paginator.py

@@ -121,3 +121,92 @@ def test_no_group_by_is_unaffected(self):
             paginator_cls=_StubGroupedPaginator,
         )
         assert response.data["grouped_by"] is None
+
+
+class _ExplodingPaginator:
+    """Fails if constructed — proves the cursor guard rejects BEFORE any paginator runs."""
+
+    def __init__(self, **kwargs):
+        raise AssertionError("paginator_cls must not be constructed for an invalid cursor")
+
+
+@pytest.mark.unit
+class TestCursorBounds:
+    """paginate() must reject an out-of-bounds client cursor before it drives slicing.
+
+    The grouped paginators use cursor.value as the per-group page size
+    (stop = offset + (cursor.value or limit) + 1). A negative value slices the queryset
+    with a negative stop -> ValueError('Negative indexing is not supported') -> HTTP 500;
+    a huge value fetches far more than max_per_page rows per group (cap bypass / DoS).
+    cursor.offset must be non-negative."""
+
+    @pytest.mark.parametrize("cursor", ["-1:0:0", "1000000:0:0", "20:-1:0"])
+    def test_out_of_bounds_cursor_rejected_before_paginator(self, cursor):
+        request = _make_request(cursor=cursor)
+        with pytest.raises(ParseError):
+            BasePaginator().paginate(
+                request=request,
+                queryset=None,
+                paginator_cls=_ExplodingPaginator,
+                default_per_page=20,
+                max_per_page=1000,
+            )
+
+    def test_valid_cursor_passes_the_guard(self):
+        request = _make_request(cursor="20:0:0")
+        response = BasePaginator().paginate(
+            request=request,
+            queryset=None,
+            paginator_cls=_StubGroupedPaginator,
+            default_per_page=20,
+            max_per_page=1000,
+        )
+        assert response.data["results"] == []
+
+    def test_cursor_value_at_max_is_allowed(self):
+        request = _make_request(cursor="1000:0:0")
+        response = BasePaginator().paginate(
+            request=request,
+            queryset=None,
+            paginator_cls=_StubGroupedPaginator,
+            default_per_page=20,
+            max_per_page=1000,
+        )
+        assert response.data["results"] == []
+
+
+@pytest.mark.unit
+class TestGetPerPageNonPositiveRejected:
+    """A non-positive per_page (negative or zero) must be rejected, not just a
+    too-large one.
+
+    OffsetPaginator.get_result() slices the queryset with
+    queryset[offset : offset + limit]; get_per_page() previously only checked
+    per_page against max_per_page (an upper bound), so a negative per_page sailed
+    through untouched, reached the slice as a negative stop, and raised Django's
+    unhandled "Negative indexing is not supported" -> HTTP 500. This is the same
+    crash class TestCursorBounds closes for the cursor-driven grouped paginators,
+    reachable here on every non-grouped paginated endpoint via a plain query
+    param.
+
+    A per_page of exactly 0 has the same failure mode: it reaches
+    OffsetPaginator.get_result() with limit=0, where math.ceil(count / limit)
+    raises an unhandled ZeroDivisionError -> HTTP 500."""
+
+    @pytest.mark.parametrize("per_page", [-1, -50, -1000, 0])
+    def test_non_positive_per_page_rejected_by_get_per_page(self, per_page):
+        request = _make_request(per_page=str(per_page))
+        with pytest.raises(ParseError):
+            BasePaginator().get_per_page(request, default_per_page=20, max_per_page=1000)
+
+    @pytest.mark.parametrize("per_page", ["-1", "0"])
+    def test_non_positive_per_page_rejected_before_paginator_runs(self, per_page):
+        request = _make_request(per_page=per_page)
+        with pytest.raises(ParseError):
+            BasePaginator().paginate(
+                request=request,
+                queryset=None,
+                paginator_cls=_ExplodingPaginator,
+                default_per_page=20,
+                max_per_page=1000,
+            )

apps/api/plane/utils/paginator.py

@@ -646,12 +646,26 @@ def get_per_page(self, request, default_per_page=1000, max_per_page=1000):
         except ValueError:
             raise ParseError(detail="Invalid per_page parameter.")
 
-        max_per_page = max(max_per_page, default_per_page)
+        max_per_page = self._effective_max_per_page(max_per_page, default_per_page)
+        # A non-positive per_page reaches OffsetPaginator.get_result() unmodified.
+        # A negative value produces a negative slice bound
+        # (queryset[offset : offset + per_page]), which raises Django's unhandled
+        # "Negative indexing is not supported" (HTTP 500) — the same crash class
+        # the cursor-bound check below closes, just reachable on every paginated
+        # endpoint via a plain query param. A zero value reaches
+        # math.ceil(count / limit) with limit=0 and raises an unhandled
+        # ZeroDivisionError (HTTP 500) instead.
+        if per_page <= 0:
+            raise ParseError(detail="Invalid per_page value. Must be greater than zero.")
         if per_page > max_per_page:
             raise ParseError(detail=f"Invalid per_page value. Cannot exceed {max_per_page}.")
 
         return per_page
 
+    @staticmethod
+    def _effective_max_per_page(max_per_page, default_per_page):
+        return max(max_per_page, default_per_page)
+
     def paginate(
         self,
         request,
@@ -680,6 +694,17 @@ def paginate(
         except ValueError:
             raise ParseError(detail="Invalid cursor parameter.")
 
+        # Bound the client-supplied cursor before it drives any slicing. The grouped
+        # paginators use cursor.value as the per-group page size
+        # (stop = offset + (cursor.value or limit) + 1). Left unbounded, a negative value
+        # slices the queryset with a negative stop -> "Negative indexing is not supported"
+        # (HTTP 500), and a huge value fetches far more than max_per_page rows per group
+        # (max_per_page cap bypass / resource-exhaustion DoS). cursor.offset is the page
+        # index and must be non-negative.
+        effective_max_per_page = self._effective_max_per_page(max_per_page, default_per_page)
+        if not (0 <= input_cursor.value <= effective_max_per_page) or input_cursor.offset < 0:
+            raise ParseError(detail="Invalid cursor parameter.")
+
         if not paginator:
             if group_by_field_name:
                 # Validate against the allowlist before the field name reaches