diff --git a/apps/api/plane/app/permissions/page.py b/apps/api/plane/app/permissions/page.py index 844ff4dafbb..6534a8d71fc 100644 --- a/apps/api/plane/app/permissions/page.py +++ b/apps/api/plane/app/permissions/page.py @@ -39,7 +39,20 @@ def has_permission(self, request, view): return False if page_id: - page = Page.objects.get(id=page_id, workspace__slug=slug) + # Scope the page to the project in the URL. Resolving the page by + # workspace + page_id alone allowed a member of one project to read + # pages belonging to another project in the same workspace + # (GHSA-g49r / GHSA-ghcr). Require an *active* ProjectPage link (both + # conditions on the same relation so they match one row) so a page + # removed from the project (soft-deleted link) is also denied. + page = Page.objects.filter( + id=page_id, + workspace__slug=slug, + project_pages__project_id=project_id, + project_pages__deleted_at__isnull=True, + ).first() + if page is None: + return False # Allow access if the user is the owner of the page if page.owned_by_id == user_id: diff --git a/apps/api/plane/app/views/page/version.py b/apps/api/plane/app/views/page/version.py index e102bf1d0b9..cd8a56e32a2 100644 --- a/apps/api/plane/app/views/page/version.py +++ b/apps/api/plane/app/views/page/version.py @@ -19,13 +19,35 @@ class PageVersionEndpoint(BaseAPIView): def get(self, request, slug, project_id, page_id, pk=None): # Check if pk is provided if pk: - # Return a single page version - page_version = PageVersion.objects.get(workspace__slug=slug, page_id=page_id, pk=pk) + # Return a single page version. Scope to an *active* ProjectPage link + # for the URL project so a page belonging to (or removed from) + # another project cannot be read via this endpoint (GHSA-g49r / + # GHSA-ghcr). The active-link partial-unique constraint keeps the + # join to a single row; distinct() is a defensive guard so the + # page__project_pages join can never make get() raise + # MultipleObjectsReturned (a 500). + page_version = ( + PageVersion.objects.filter( + workspace__slug=slug, + page__project_pages__project_id=project_id, + page__project_pages__deleted_at__isnull=True, + page_id=page_id, + pk=pk, + ) + .distinct() + .get() + ) # Serialize the page version serializer = PageVersionDetailSerializer(page_version) return Response(serializer.data, status=status.HTTP_200_OK) - # Return all page versions - page_versions = PageVersion.objects.filter(workspace__slug=slug, page_id=page_id) + # Return all page versions scoped to an active ProjectPage link for the + # URL project (defense in depth). + page_versions = PageVersion.objects.filter( + workspace__slug=slug, + page__project_pages__project_id=project_id, + page__project_pages__deleted_at__isnull=True, + page_id=page_id, + ) # Serialize the page versions serializer = PageVersionSerializer(page_versions, many=True) return Response(serializer.data, status=status.HTTP_200_OK) diff --git a/apps/api/plane/tests/contract/app/test_page_version_project_scope_app.py b/apps/api/plane/tests/contract/app/test_page_version_project_scope_app.py new file mode 100644 index 00000000000..f9176a3ce95 --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_page_version_project_scope_app.py @@ -0,0 +1,149 @@ +# 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-g49r-p85q-qq2w / GHSA-ghcr-frqr-6pqr. + +ProjectPagePermission verified that the caller was a member of the URL +project_id, but PageVersionEndpoint resolved the page (and its versions) by +workspace + page_id only. A member of one project could therefore read the +page versions of a public page belonging to a *different* project in the same +workspace via that project's URL. +""" + +import uuid + +import pytest +from django.utils import timezone +from rest_framework import status + +from plane.db.models import ( + Page, + PageVersion, + Project, + ProjectMember, + ProjectPage, + User, +) + + +def _page_versions_url(slug, project_id, page_id, pk=None): + base = f"/api/workspaces/{slug}/projects/{project_id}/pages/{page_id}/versions/" + return f"{base}{pk}/" if pk else base + + +def _make_project(workspace, identifier): + return Project.objects.create( + name=f"Project {identifier}", + identifier=identifier, + workspace=workspace, + ) + + +def _make_page(workspace, project, owner, access=Page.PUBLIC_ACCESS): + page = Page.objects.create( + workspace=workspace, + owned_by=owner, + access=access, + name="Secret page", + ) + ProjectPage.objects.create(workspace=workspace, project=project, page=page) + return page + + +def _make_version(workspace, page, owner): + return PageVersion.objects.create( + workspace=workspace, + page=page, + owned_by=owner, + description_html="
secret
", + ) + + +@pytest.mark.contract +class TestPageVersionProjectScope: + """The attacker (create_user / session_client) is an active member of + project_a only. Victim owns a public page in project_b.""" + + def _setup(self, workspace, attacker): + victim = User.objects.create(email="victim@plane.so", username=f"victim_{uuid.uuid4().hex[:8]}") + + project_a = _make_project(workspace, "PRJA") + project_b = _make_project(workspace, "PRJB") + + # Attacker is an active member of project A only. + ProjectMember.objects.create(workspace=workspace, project=project_a, member=attacker, role=20) + + # Public page + version living in project B (attacker is NOT a member). + page_b = _make_page(workspace, project_b, victim) + version_b = _make_version(workspace, page_b, victim) + + return victim, project_a, project_b, page_b, version_b + + @pytest.mark.django_db + def test_cross_project_version_list_denied(self, session_client, workspace, create_user): + """Listing another project's page versions via a project the attacker + belongs to must be denied (was a 200 leak).""" + _, project_a, _, page_b, _ = self._setup(workspace, create_user) + + response = session_client.get(_page_versions_url(workspace.slug, project_a.id, page_b.id)) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + @pytest.mark.django_db + def test_cross_project_version_detail_denied(self, session_client, workspace, create_user): + """Reading a single cross-project page version must be denied.""" + _, project_a, _, page_b, version_b = self._setup(workspace, create_user) + + response = session_client.get( + _page_versions_url(workspace.slug, project_a.id, page_b.id, pk=version_b.id) + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + @pytest.mark.django_db + def test_same_project_public_page_versions_allowed(self, session_client, workspace, create_user): + """A public page that genuinely belongs to the attacker's project is + still readable, and its versions are returned.""" + victim, project_a, _, _, _ = self._setup(workspace, create_user) + + # Public page owned by the victim but linked to project A (attacker is a + # member of A). Exercises the public-page access branch (not owner). + page_a = _make_page(workspace, project_a, victim) + version_a = _make_version(workspace, page_a, victim) + + response = session_client.get(_page_versions_url(workspace.slug, project_a.id, page_a.id)) + + assert response.status_code == status.HTTP_200_OK + returned_ids = {str(item["id"]) for item in response.json()} + assert str(version_a.id) in returned_ids + + @pytest.mark.django_db + def test_revoked_project_link_denied(self, session_client, workspace, create_user): + """A page whose ProjectPage link to the attacker's project was + soft-deleted (page removed from the project) must be denied, even + though the attacker is a member of that project.""" + victim, project_a, _, _, _ = self._setup(workspace, create_user) + + page = Page.objects.create( + workspace=workspace, owned_by=victim, access=Page.PUBLIC_ACCESS, name="Removed page" + ) + # Link exists but is soft-deleted → the page no longer belongs to A. + ProjectPage.objects.create( + workspace=workspace, project=project_a, page=page, deleted_at=timezone.now() + ) + _make_version(workspace, page, victim) + + response = session_client.get(_page_versions_url(workspace.slug, project_a.id, page.id)) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + @pytest.mark.django_db + def test_cross_project_version_list_not_a_member_anywhere(self, session_client, workspace, create_user): + """Sanity: a project the attacker is not a member of is denied outright.""" + _, _, project_b, page_b, _ = self._setup(workspace, create_user) + + response = session_client.get(_page_versions_url(workspace.slug, project_b.id, page_b.id)) + + assert response.status_code == status.HTTP_403_FORBIDDEN diff --git a/apps/api/plane/utils/permissions/page.py b/apps/api/plane/utils/permissions/page.py index 844ff4dafbb..6534a8d71fc 100644 --- a/apps/api/plane/utils/permissions/page.py +++ b/apps/api/plane/utils/permissions/page.py @@ -39,7 +39,20 @@ def has_permission(self, request, view): return False if page_id: - page = Page.objects.get(id=page_id, workspace__slug=slug) + # Scope the page to the project in the URL. Resolving the page by + # workspace + page_id alone allowed a member of one project to read + # pages belonging to another project in the same workspace + # (GHSA-g49r / GHSA-ghcr). Require an *active* ProjectPage link (both + # conditions on the same relation so they match one row) so a page + # removed from the project (soft-deleted link) is also denied. + page = Page.objects.filter( + id=page_id, + workspace__slug=slug, + project_pages__project_id=project_id, + project_pages__deleted_at__isnull=True, + ).first() + if page is None: + return False # Allow access if the user is the owner of the page if page.owned_by_id == user_id: