From 2fc210f5745e5075f5542caac9ecf8f2235ed629 Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Thu, 27 Aug 2026 10:33:41 +0530 Subject: [PATCH] [WEB-8353] fix(security): extend guest issue visibility guard to comments + reactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The attachment and activity endpoints already route restricted guests through issue_hidden_from_guest, but this PR's own docstring names comments as in scope too — that part was never wired up. A restricted guest blocked from GET .../history/ on a hidden issue could still call GET .../comments/ (list and retrieve) and read all comment content on that same issue, and could create/destroy CommentReaction rows keyed by comment_id with no issue-visibility check at all. Apply the same guard to IssueCommentViewSet.list/.retrieve. For CommentReactionViewSet.create/.destroy, which only receive comment_id (not issue_id) from the URL, resolve the parent issue first and then apply the identical check. Co-authored-by: Plane AI --- apps/api/plane/app/views/issue/comment.py | 50 ++++++- .../test_guest_issue_subresource_scope_app.py | 133 ++++++++++++++++++ 2 files changed, 182 insertions(+), 1 deletion(-) diff --git a/apps/api/plane/app/views/issue/comment.py b/apps/api/plane/app/views/issue/comment.py index 34fe0f9e4b..4cbf83c657 100644 --- a/apps/api/plane/app/views/issue/comment.py +++ b/apps/api/plane/app/views/issue/comment.py @@ -18,7 +18,7 @@ from rest_framework import status # Module imports from .. import BaseViewSet from plane.app.serializers import IssueCommentSerializer, CommentReactionSerializer -from plane.app.permissions import allow_permission, ROLE +from plane.app.permissions import allow_permission, issue_hidden_from_guest, ROLE from plane.db.models import IssueComment, ProjectMember, CommentReaction, Project, Issue from plane.bgtasks.issue_activities_task import issue_activity from plane.utils.host import base_host @@ -60,6 +60,28 @@ class IssueCommentViewSet(BaseViewSet): .distinct() ) + @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) + def list(self, request, slug, project_id, issue_id): + # A restricted guest may only see the comments of issues they created, + # mirroring the issue-detail visibility rule. + if issue_hidden_from_guest(request, slug, project_id, issue_id): + return Response( + {"error": "You are not allowed to view this issue"}, + status=status.HTTP_403_FORBIDDEN, + ) + return super().list(request, slug, project_id, issue_id) + + @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) + def retrieve(self, request, slug, project_id, issue_id, pk): + # A restricted guest may only see the comments of issues they created, + # mirroring the issue-detail visibility rule. + if issue_hidden_from_guest(request, slug, project_id, issue_id): + return Response( + {"error": "You are not allowed to view this issue"}, + status=status.HTTP_403_FORBIDDEN, + ) + return super().retrieve(request, slug, project_id, issue_id, pk) + @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) def create(self, request, slug, project_id, issue_id): project = Project.objects.get(pk=project_id) @@ -182,6 +204,19 @@ class CommentReactionViewSet(BaseViewSet): @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) def create(self, request, slug, project_id, comment_id): + # A restricted guest may only react to comments on issues they created, + # mirroring the issue-detail visibility rule. Comment reactions are + # keyed by comment_id, so resolve the parent issue first. + issue_id = ( + IssueComment.objects.filter(pk=comment_id, workspace__slug=slug, project_id=project_id) + .values_list("issue_id", flat=True) + .first() + ) + if issue_id and issue_hidden_from_guest(request, slug, project_id, issue_id): + return Response( + {"error": "You are not allowed to view this issue"}, + status=status.HTTP_403_FORBIDDEN, + ) try: serializer = CommentReactionSerializer(data=request.data) if serializer.is_valid(): @@ -211,6 +246,19 @@ class CommentReactionViewSet(BaseViewSet): @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) def destroy(self, request, slug, project_id, comment_id, reaction_code): + # A restricted guest may only react to comments on issues they created, + # mirroring the issue-detail visibility rule. Comment reactions are + # keyed by comment_id, so resolve the parent issue first. + issue_id = ( + IssueComment.objects.filter(pk=comment_id, workspace__slug=slug, project_id=project_id) + .values_list("issue_id", flat=True) + .first() + ) + if issue_id and issue_hidden_from_guest(request, slug, project_id, issue_id): + return Response( + {"error": "You are not allowed to view this issue"}, + status=status.HTTP_403_FORBIDDEN, + ) comment_reaction = CommentReaction.objects.get( workspace__slug=slug, project_id=project_id, diff --git a/apps/api/plane/tests/contract/app/test_guest_issue_subresource_scope_app.py b/apps/api/plane/tests/contract/app/test_guest_issue_subresource_scope_app.py index f5ca59bdb2..a5ed0c210c 100644 --- a/apps/api/plane/tests/contract/app/test_guest_issue_subresource_scope_app.py +++ b/apps/api/plane/tests/contract/app/test_guest_issue_subresource_scope_app.py @@ -12,6 +12,8 @@ not replicate that restriction: * ``IssueAttachmentEndpoint.get`` (v1 ``.../issue-attachments/``) * ``IssueAttachmentV2Endpoint.get`` (v2 ``/assets/v2/.../attachments/``) * ``IssueActivityEndpoint.get`` (``.../history/``) +* ``IssueCommentViewSet.list``/``.retrieve`` (``.../comments/``) +* ``CommentReactionViewSet.create``/``.destroy`` (``.../comments//reactions/``) Each is decorated ``@allow_permission([ADMIN, MEMBER, GUEST])`` and queried by ``issue_id`` only, so a restricted guest who is a legitimate project member could @@ -33,6 +35,7 @@ from rest_framework.test import APIClient from plane.db.models import ( FileAsset, Issue, + IssueComment, Project, ProjectMember, User, @@ -42,6 +45,10 @@ from plane.db.models import ( V1_ATTACH_URL = "/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/issue-attachments/" V2_ATTACH_URL = "/api/assets/v2/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/attachments/" HISTORY_URL = "/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/history/" +COMMENT_LIST_URL = "/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/comments/" +COMMENT_DETAIL_URL = "/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/comments/{pk}/" +REACTION_URL = "/api/workspaces/{slug}/projects/{project_id}/comments/{comment_id}/reactions/" +REACTION_DETAIL_URL = "/api/workspaces/{slug}/projects/{project_id}/comments/{comment_id}/reactions/{reaction_code}/" def _make_issue(name, project, workspace, author): @@ -52,6 +59,12 @@ def _make_issue(name, project, workspace, author): return issue +def _make_comment(issue, project, workspace, actor): + return IssueComment.objects.create( + issue=issue, project=project, actor=actor, comment_html="

secret comment

" + ) + + def _make_attachment(issue, project, workspace): return FileAsset.objects.create( attributes={"name": "secret.pdf", "type": "application/pdf", "size": 100}, @@ -114,6 +127,18 @@ def foreign_issue(db, workspace, project, create_user): return issue +@pytest.fixture +def own_issue_comment(db, workspace, project, own_issue, guest): + """A comment on the guest's own issue.""" + return _make_comment(own_issue, project, workspace, guest) + + +@pytest.fixture +def foreign_issue_comment(db, workspace, project, foreign_issue, create_user): + """A comment on an issue authored by someone other than the guest.""" + return _make_comment(foreign_issue, project, workspace, create_user) + + @pytest.mark.contract @pytest.mark.django_db class TestGuestIssueSubresourceScope: @@ -167,6 +192,56 @@ class TestGuestIssueSubresourceScope: f"Got {response.status_code}: {getattr(response, 'data', None)!r}" ) + def test_guest_blocked_comments_list(self, guest_client, workspace, project, foreign_issue, foreign_issue_comment): + response = guest_client.get( + COMMENT_LIST_URL.format(slug=workspace.slug, project_id=project.id, issue_id=foreign_issue.id) + ) + assert response.status_code == status.HTTP_403_FORBIDDEN, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + + def test_guest_blocked_comment_retrieve( + self, guest_client, workspace, project, foreign_issue, foreign_issue_comment + ): + response = guest_client.get( + COMMENT_DETAIL_URL.format( + slug=workspace.slug, + project_id=project.id, + issue_id=foreign_issue.id, + pk=foreign_issue_comment.id, + ) + ) + assert response.status_code == status.HTTP_403_FORBIDDEN, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + + def test_guest_blocked_comment_reaction_create( + self, guest_client, workspace, project, foreign_issue, foreign_issue_comment + ): + response = guest_client.post( + REACTION_URL.format(slug=workspace.slug, project_id=project.id, comment_id=foreign_issue_comment.id), + {"reaction": "like"}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + + def test_guest_blocked_comment_reaction_destroy( + self, guest_client, workspace, project, foreign_issue, foreign_issue_comment + ): + response = guest_client.delete( + REACTION_DETAIL_URL.format( + slug=workspace.slug, + project_id=project.id, + comment_id=foreign_issue_comment.id, + reaction_code="like", + ) + ) + assert response.status_code == status.HTTP_403_FORBIDDEN, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + # ---- the guest still sees their OWN issue's sub-resources ---------------- def test_guest_allowed_own_issue_attachments(self, guest_client, workspace, project, own_issue): @@ -178,8 +253,66 @@ class TestGuestIssueSubresourceScope: ) assert len(response.data) == 1, f"Guest should see their own attachment: {response.data!r}" + def test_guest_allowed_own_issue_comments_list( + self, guest_client, workspace, project, own_issue, own_issue_comment + ): + response = guest_client.get( + COMMENT_LIST_URL.format(slug=workspace.slug, project_id=project.id, issue_id=own_issue.id) + ) + assert response.status_code == status.HTTP_200_OK, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert len(response.data) == 1, f"Guest should see their own comment: {response.data!r}" + + def test_guest_allowed_own_issue_comment_retrieve( + self, guest_client, workspace, project, own_issue, own_issue_comment + ): + response = guest_client.get( + COMMENT_DETAIL_URL.format( + slug=workspace.slug, project_id=project.id, issue_id=own_issue.id, pk=own_issue_comment.id + ) + ) + assert response.status_code == status.HTTP_200_OK, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + + def test_guest_allowed_own_issue_comment_reaction_create( + self, guest_client, workspace, project, own_issue, own_issue_comment + ): + response = guest_client.post( + REACTION_URL.format(slug=workspace.slug, project_id=project.id, comment_id=own_issue_comment.id), + {"reaction": "like"}, + format="json", + ) + assert response.status_code == status.HTTP_201_CREATED, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + # ---- positive controls: members and unrestricted guests unaffected ------- + def test_member_reads_foreign_issue_comments( + self, session_client, workspace, project, foreign_issue, foreign_issue_comment + ): + """A full project member still reads any issue's comments.""" + response = session_client.get( + COMMENT_LIST_URL.format(slug=workspace.slug, project_id=project.id, issue_id=foreign_issue.id) + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data) == 1 + + def test_unrestricted_guest_reads_foreign_issue_comments( + self, guest_client, workspace, project, foreign_issue, foreign_issue_comment + ): + """When guest_view_all_features is enabled, the guest sees all comments.""" + project.guest_view_all_features = True + project.save(update_fields=["guest_view_all_features"]) + + response = guest_client.get( + COMMENT_LIST_URL.format(slug=workspace.slug, project_id=project.id, issue_id=foreign_issue.id) + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data) == 1 + def test_member_reads_foreign_issue_attachments(self, session_client, workspace, project, foreign_issue): """A full project member still reads any issue's attachments.""" response = session_client.get(