From 69036c173a6b309c5dc243b7bdc4fbb5cf281651 Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Mon, 3 Aug 2026 13:24:21 +0530 Subject: [PATCH 1/3] [SECUR-243] fix(security): scope issue comment + relation create to the URL project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProjectEntityPermission and allow_permission validate only that the caller is an active member of the URL's project_id. Neither validates that the sibling issue_id path parameter belongs to that project or workspace, and two write handlers used issue_id unscoped: - IssueCommentViewSet.create resolved the issue with Issue.objects.get(pk=issue_id). Any member of any one project could comment on an issue in another project or workspace, and the 201 response serialized that issue's full issue_detail — name, description_json, description_html, priority, dates, sequence_id — back to the caller, making this a cross-tenant read as well as a write. - IssueRelationViewSet.create workspace-scoped the request-body issues list but used the URL issue_id unscoped as the other side of the relation, so the earlier scoping fix covered one side of the relationship and missed the other. Both lookups are now bound to workspace__slug + project_id and 404 otherwise, matching the pattern already used by sub_issue.py get() and PagesDescriptionViewSet. The third vector in the advisory (SubIssuesEndpoint.post) is already addressed by open PR #9466 and is deliberately not touched here. Adds contract regression coverage for both endpoints: cross-workspace and same-workspace-other-project both 404 with nothing written and no detail leaked, plus positive controls for the in-project paths. Advisory: GHSA-hvx3-58mp-5fpx Co-authored-by: Plane AI --- apps/api/plane/app/views/issue/comment.py | 9 +- apps/api/plane/app/views/issue/relation.py | 9 + .../app/test_issue_subresource_scope_app.py | 247 ++++++++++++++++++ 3 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py diff --git a/apps/api/plane/app/views/issue/comment.py b/apps/api/plane/app/views/issue/comment.py index 34fe0f9e4b9..1e0928a4a96 100644 --- a/apps/api/plane/app/views/issue/comment.py +++ b/apps/api/plane/app/views/issue/comment.py @@ -63,7 +63,14 @@ def get_queryset(self): @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) def create(self, request, slug, project_id, issue_id): project = Project.objects.get(pk=project_id) - issue = Issue.objects.get(pk=issue_id) + # SECURITY: bind the issue to the URL workspace + project. `allow_permission` only + # checks that the caller is a member of `project_id`, never that `issue_id` lives in + # it, so a bare pk lookup let any project member comment on an issue in another + # project/workspace — and read that issue's full `issue_detail` back out of the 201 + # response (GHSA-hvx3-58mp-5fpx). + issue = Issue.objects.filter(pk=issue_id, workspace__slug=slug, project_id=project_id).first() + if issue is None: + return Response({"error": "Issue not found"}, status=status.HTTP_404_NOT_FOUND) if ( ProjectMember.objects.filter( workspace__slug=slug, diff --git a/apps/api/plane/app/views/issue/relation.py b/apps/api/plane/app/views/issue/relation.py index 5fe44e55f2d..2f9439d495e 100644 --- a/apps/api/plane/app/views/issue/relation.py +++ b/apps/api/plane/app/views/issue/relation.py @@ -217,6 +217,15 @@ def create(self, request, slug, project_id, issue_id): issues = request.data.get("issues", []) project = Project.objects.get(pk=project_id) + # SECURITY: bind the URL issue to the workspace + project before using it as one + # side of the relation. The body `issues` are scoped below, but `issue_id` came + # straight from the URL and ProjectEntityPermission only checks the caller's + # membership of `project_id`, not that `issue_id` belongs to it — so the earlier + # scoping fix covered one side of the relationship and missed the other + # (GHSA-hvx3-58mp-5fpx). + if not Issue.issue_objects.filter(pk=issue_id, workspace__slug=slug, project_id=project_id).exists(): + return Response({"error": "Issue not found"}, status=status.HTTP_404_NOT_FOUND) + # Scope to workspace to prevent cross-tenant IDOR # Relations can cross projects so only workspace scope is enforced issues = list( diff --git a/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py b/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py new file mode 100644 index 00000000000..01a1a228866 --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py @@ -0,0 +1,247 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +"""Contract tests for issue sub-resource scoping on comment + relation create. + +Regression coverage for GHSA-hvx3-58mp-5fpx (SECUR-243). + +``ProjectEntityPermission`` / ``allow_permission`` validate only that the caller is +an active member of the URL's ``project_id``. Neither validates that the sibling +``issue_id`` path parameter belongs to that project or workspace, and two write +handlers then used ``issue_id`` unscoped: + +* ``IssueCommentViewSet.create`` — ``Issue.objects.get(pk=issue_id)``. The 201 + response serializes the foreign issue's full ``issue_detail`` (name, description, + priority, dates, ``sequence_id``) back to the caller, so this is a cross-tenant + *read* as well as a write. +* ``IssueRelationViewSet.create`` — the request-body ``issues`` were workspace-scoped + but the URL ``issue_id`` used as the other side of the relation was not. + +Both fixes bind the lookup to ``workspace__slug`` + ``project_id`` and 404 otherwise. +""" + +from unittest import mock +from uuid import uuid4 + +import pytest +from rest_framework import status +from rest_framework.test import APIClient + +from plane.db.models import ( + Issue, + IssueComment, + IssueRelation, + Project, + ProjectMember, + User, + Workspace, + WorkspaceMember, +) + + +def comments_url(slug, project_id, issue_id): + return f"/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/comments/" + + +def relation_url(slug, project_id, issue_id): + return f"/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/issue-relation/" + + +@pytest.fixture(autouse=True) +def _no_activity(db): + """Stub the deferred activity tasks (broker) and base_host (needs APP_BASE_URL / + WEB_URL, unset in the test env) so the success paths run cleanly.""" + with ( + mock.patch("plane.app.views.issue.comment.issue_activity"), + mock.patch("plane.app.views.issue.comment.model_activity"), + mock.patch("plane.app.views.issue.comment.base_host", return_value="http://testserver"), + mock.patch("plane.app.views.issue.relation.issue_activity"), + mock.patch("plane.app.views.issue.relation.base_host", return_value="http://testserver"), + ): + yield + + +def _make_user(prefix): + unique_id = uuid4().hex[:8] + user = User.objects.create( + email=f"{prefix}-{unique_id}@plane.so", + username=f"{prefix}_{unique_id}", + first_name=prefix.title(), + last_name="User", + ) + user.set_password("test-password") + user.save() + return user + + +def _make_project(name, identifier, workspace, owner, members=()): + project = Project.objects.create(name=name, identifier=identifier, workspace=workspace, created_by=owner) + for member in members: + ProjectMember.objects.create(project=project, member=member, workspace=workspace, role=20) + return project + + +@pytest.fixture +def attacker(db): + """A user with a workspace and project of their very own.""" + return _make_user("attacker") + + +@pytest.fixture +def attacker_workspace(db, attacker): + workspace = Workspace.objects.create( + name="Attacker Workspace", owner=attacker, slug=f"attacker-ws-{uuid4().hex[:8]}" + ) + WorkspaceMember.objects.create(workspace=workspace, member=attacker, role=20) + return workspace + + +@pytest.fixture +def attacker_project(db, attacker_workspace, attacker): + return _make_project("Attacker Project", "ATK", attacker_workspace, attacker, [attacker]) + + +@pytest.fixture +def attacker_issue(db, attacker_workspace, attacker_project): + return Issue.objects.create(name="Attacker's own issue", project=attacker_project, workspace=attacker_workspace) + + +@pytest.fixture +def victim_workspace(db): + """A completely separate tenant. The attacker is not a member.""" + owner = _make_user("victim") + workspace = Workspace.objects.create(name="Victim Workspace", owner=owner, slug=f"victim-ws-{uuid4().hex[:8]}") + WorkspaceMember.objects.create(workspace=workspace, member=owner, role=20) + workspace.owner_user = owner + return workspace + + +@pytest.fixture +def victim_issue(db, victim_workspace): + project = _make_project( + "Victim Project", + "VIC", + victim_workspace, + victim_workspace.owner_user, + [victim_workspace.owner_user], + ) + return Issue.objects.create( + name="Confidential victim issue", + description_html="

Secret roadmap detail

", + priority="urgent", + project=project, + workspace=victim_workspace, + ) + + +@pytest.fixture +def sibling_project_issue(db, attacker_workspace, attacker): + """An issue in a *different project of the attacker's own workspace* that the + attacker is not a member of — the intra-workspace case.""" + other_owner = _make_user("colleague") + WorkspaceMember.objects.create(workspace=attacker_workspace, member=other_owner, role=20) + project = _make_project("Sibling Project", "SIB", attacker_workspace, other_owner, [other_owner]) + return Issue.objects.create(name="Sibling project issue", project=project, workspace=attacker_workspace) + + +@pytest.fixture +def attacker_client(attacker): + client = APIClient() + client.force_authenticate(user=attacker) + return client + + +@pytest.mark.contract +class TestIssueCommentCreateScope: + """``issue_id`` must belong to the URL project, or the request 404s.""" + + @pytest.mark.django_db + def test_rejects_cross_workspace_issue(self, attacker_client, attacker_workspace, attacker_project, victim_issue): + url = comments_url(attacker_workspace.slug, attacker_project.id, victim_issue.id) + response = attacker_client.post(url, {"comment_html": "

hi

"}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + # No comment may be planted on the foreign issue... + assert not IssueComment.objects.filter(issue_id=victim_issue.id).exists() + # ...and none of its private detail may leak back in the response. + assert "Confidential victim issue" not in str(getattr(response, "data", "")) + assert "Secret roadmap detail" not in str(getattr(response, "data", "")) + + @pytest.mark.django_db + def test_rejects_same_workspace_other_project_issue( + self, attacker_client, attacker_workspace, attacker_project, sibling_project_issue + ): + url = comments_url(attacker_workspace.slug, attacker_project.id, sibling_project_issue.id) + response = attacker_client.post(url, {"comment_html": "

hi

"}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert not IssueComment.objects.filter(issue_id=sibling_project_issue.id).exists() + + @pytest.mark.django_db + def test_allows_own_project_issue(self, attacker_client, attacker_workspace, attacker_project, attacker_issue): + """Positive control: commenting on an issue in the URL project still works.""" + url = comments_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"comment_html": "

hi

"}, format="json") + + assert response.status_code == status.HTTP_201_CREATED, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert IssueComment.objects.filter(issue_id=attacker_issue.id).exists() + + +@pytest.mark.contract +class TestIssueRelationCreateScope: + """The URL side of the relation must belong to the URL project too.""" + + @pytest.mark.django_db + def test_rejects_cross_workspace_url_issue( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue, victim_issue + ): + url = relation_url(attacker_workspace.slug, attacker_project.id, victim_issue.id) + response = attacker_client.post( + url, {"relation_type": "blocking", "issues": [str(attacker_issue.id)]}, format="json" + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert not IssueRelation.objects.filter(related_issue_id=victim_issue.id).exists() + assert not IssueRelation.objects.filter(issue_id=victim_issue.id).exists() + + @pytest.mark.django_db + def test_rejects_same_workspace_other_project_url_issue( + self, + attacker_client, + attacker_workspace, + attacker_project, + attacker_issue, + sibling_project_issue, + ): + url = relation_url(attacker_workspace.slug, attacker_project.id, sibling_project_issue.id) + response = attacker_client.post( + url, {"relation_type": "blocking", "issues": [str(attacker_issue.id)]}, format="json" + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert not IssueRelation.objects.filter(related_issue_id=sibling_project_issue.id).exists() + + @pytest.mark.django_db + def test_allows_relation_within_project( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue + ): + """Positive control: relating two issues inside the URL project still works.""" + other = Issue.objects.create(name="Second own issue", project=attacker_project, workspace=attacker_workspace) + url = relation_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"relation_type": "blocking", "issues": [str(other.id)]}, format="json") + + assert response.status_code == status.HTTP_201_CREATED, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert IssueRelation.objects.filter(issue_id=other.id, related_issue_id=attacker_issue.id).exists() From 96257d141eebdaa7b26eacc328465b53d34f9ca0 Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Mon, 3 Aug 2026 15:13:18 +0530 Subject: [PATCH 2/3] [SECUR-243] fix(security): scope issue relation removal to the URL project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up from adversarial review of the create-path fix: remove_relation had the same gap on the delete side. It filtered IssueRelation on workspace__slug only and never checked that the URL issue_id belongs to the URL project, so a member of one project could delete relations between two issues of a sibling project in the same workspace — verified 204 with deleted_at set on the sibling project's row. Applies the same participant binding as create(): the URL issue must live in the URL project. The IssueRelation row itself stays workspace-scoped, because relations legitimately span projects and either participant's project may remove them. Also returns 404 instead of a 500 when no relation matches — the handler did .first() and then called .delete() on None (AttributeError -> generic 500). Adds coverage for the sibling-project delete, the missing-relation 404, and a positive control, plus tests pinning the IssueManager boundary: comment.py deliberately uses Issue.objects (intake/triage, archived and draft issues must stay commentable) while relation.py uses Issue.issue_objects (matching the body `issues` filter and SubIssuesEndpoint). That axis was previously untested, so "tidying" the two to match would have silently broken commenting on intake items. Co-authored-by: Plane AI --- apps/api/plane/app/views/issue/relation.py | 12 ++ .../app/test_issue_subresource_scope_app.py | 141 +++++++++++++++++- 2 files changed, 150 insertions(+), 3 deletions(-) diff --git a/apps/api/plane/app/views/issue/relation.py b/apps/api/plane/app/views/issue/relation.py index 2f9439d495e..d790c90b300 100644 --- a/apps/api/plane/app/views/issue/relation.py +++ b/apps/api/plane/app/views/issue/relation.py @@ -280,12 +280,24 @@ def create(self, request, slug, project_id, issue_id): def remove_relation(self, request, slug, project_id, issue_id): related_issue = request.data.get("related_issue", None) + # SECURITY: same binding as create() — the URL issue must belong to the URL + # project before it can be used to select a relation for deletion. Otherwise a + # member of one project could delete relations between issues of a sibling + # project in the same workspace (GHSA-hvx3-58mp-5fpx). The IssueRelation row + # itself stays workspace-scoped only: relations legitimately span projects, and + # either participant's project may remove them. + if not Issue.issue_objects.filter(pk=issue_id, workspace__slug=slug, project_id=project_id).exists(): + return Response({"error": "Issue not found"}, status=status.HTTP_404_NOT_FOUND) + issue_relations = IssueRelation.objects.filter( workspace__slug=slug, ).filter( Q(issue_id=related_issue, related_issue_id=issue_id) | Q(issue_id=issue_id, related_issue_id=related_issue) ) issue_relations = issue_relations.first() + if issue_relations is None: + # Previously fell through to `None.delete()` -> AttributeError -> 500. + return Response({"error": "Issue relation not found"}, status=status.HTTP_404_NOT_FOUND) current_instance = json.dumps(IssueRelationSerializer(issue_relations).data, cls=DjangoJSONEncoder) issue_relations.delete() issue_activity.delay( diff --git a/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py b/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py index 01a1a228866..c532caee334 100644 --- a/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py +++ b/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py @@ -2,13 +2,13 @@ # SPDX-License-Identifier: AGPL-3.0-only # See the LICENSE file for details. -"""Contract tests for issue sub-resource scoping on comment + relation create. +"""Contract tests for issue sub-resource scoping on comment + relation endpoints. Regression coverage for GHSA-hvx3-58mp-5fpx (SECUR-243). ``ProjectEntityPermission`` / ``allow_permission`` validate only that the caller is an active member of the URL's ``project_id``. Neither validates that the sibling -``issue_id`` path parameter belongs to that project or workspace, and two write +``issue_id`` path parameter belongs to that project or workspace, and three write handlers then used ``issue_id`` unscoped: * ``IssueCommentViewSet.create`` — ``Issue.objects.get(pk=issue_id)``. The 201 @@ -17,14 +17,18 @@ *read* as well as a write. * ``IssueRelationViewSet.create`` — the request-body ``issues`` were workspace-scoped but the URL ``issue_id`` used as the other side of the relation was not. +* ``IssueRelationViewSet.remove_relation`` — same gap on the delete path, found while + reviewing the two above: workspace-scoped only, so a member of one project could + delete relations belonging to a sibling project in the same workspace. -Both fixes bind the lookup to ``workspace__slug`` + ``project_id`` and 404 otherwise. +All three bind the lookup to ``workspace__slug`` + ``project_id`` and 404 otherwise. """ from unittest import mock from uuid import uuid4 import pytest +from django.utils import timezone from rest_framework import status from rest_framework.test import APIClient @@ -48,6 +52,10 @@ def relation_url(slug, project_id, issue_id): return f"/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/issue-relation/" +def remove_relation_url(slug, project_id, issue_id): + return f"/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/remove-relation/" + + @pytest.fixture(autouse=True) def _no_activity(db): """Stub the deferred activity tasks (broker) and base_host (needs APP_BASE_URL / @@ -245,3 +253,130 @@ def test_allows_relation_within_project( f"Got {response.status_code}: {getattr(response, 'data', None)!r}" ) assert IssueRelation.objects.filter(issue_id=other.id, related_issue_id=attacker_issue.id).exists() + + +@pytest.mark.contract +class TestIssueRelationRemoveScope: + """``remove_relation`` needs the same participant binding as ``create``. + + The relation row itself stays workspace-scoped — relations legitimately span + projects and either participant may remove them — but the URL ``issue_id`` must + belong to the URL project. + """ + + @pytest.mark.django_db + def test_rejects_same_workspace_other_project_url_issue( + self, attacker_client, attacker_workspace, attacker_project, sibling_project_issue + ): + """A relation wholly inside a sibling project must not be deletable.""" + sibling_project = sibling_project_issue.project + other_sibling_issue = Issue.objects.create( + name="Second sibling issue", project=sibling_project, workspace=attacker_workspace + ) + relation = IssueRelation.objects.create( + issue=other_sibling_issue, + related_issue=sibling_project_issue, + relation_type="blocked_by", + project=sibling_project, + workspace=attacker_workspace, + ) + + url = remove_relation_url(attacker_workspace.slug, attacker_project.id, sibling_project_issue.id) + response = attacker_client.post(url, {"related_issue": str(other_sibling_issue.id)}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + relation.refresh_from_db() + assert relation.deleted_at is None, "Deleted a sibling project's relation" + + @pytest.mark.django_db + def test_missing_relation_404s_rather_than_500( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue + ): + """No matching relation used to hit ``None.delete()`` -> AttributeError -> 500.""" + url = remove_relation_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"related_issue": str(uuid4())}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + + @pytest.mark.django_db + def test_allows_removal_within_project(self, attacker_client, attacker_workspace, attacker_project, attacker_issue): + """Positive control: removing a relation inside the URL project still works.""" + other = Issue.objects.create(name="Second own issue", project=attacker_project, workspace=attacker_workspace) + relation = IssueRelation.objects.create( + issue=other, + related_issue=attacker_issue, + relation_type="blocked_by", + project=attacker_project, + workspace=attacker_workspace, + ) + + url = remove_relation_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"related_issue": str(other.id)}, format="json") + + assert response.status_code == status.HTTP_204_NO_CONTENT, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + relation.refresh_from_db() + assert relation.deleted_at is not None + + +@pytest.mark.contract +class TestIssueManagerBoundary: + """Pin the one axis on which the two fixes deliberately differ. + + ``comment.py`` scopes with ``Issue.objects`` and ``relation.py`` with + ``Issue.issue_objects``. ``IssueManager`` additionally excludes triage-state, + archived and draft issues, so the choice is behavioural, not cosmetic: intake + (triage) and archived issues must stay commentable, while the relation endpoints + follow the ``issue_objects`` convention already used for the body ``issues`` list + and by ``SubIssuesEndpoint``. Without these tests, "tidying" the two to match + would silently break commenting on intake items. + """ + + @pytest.mark.django_db + def test_comment_allowed_on_archived_issue( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue + ): + attacker_issue.archived_at = timezone.now().date() + attacker_issue.save(update_fields=["archived_at"]) + + url = comments_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"comment_html": "

hi

"}, format="json") + + assert response.status_code == status.HTTP_201_CREATED, ( + f"Archived issue must stay commentable, got {response.status_code}" + ) + + @pytest.mark.django_db + def test_comment_allowed_on_draft_issue( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue + ): + attacker_issue.is_draft = True + attacker_issue.save(update_fields=["is_draft"]) + + url = comments_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"comment_html": "

hi

"}, format="json") + + assert response.status_code == status.HTTP_201_CREATED, ( + f"Draft issue must stay commentable, got {response.status_code}" + ) + + @pytest.mark.django_db + def test_relation_create_excludes_archived_url_issue( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue + ): + """Documents the narrowing: ``issue_objects`` excludes archived issues.""" + other = Issue.objects.create(name="Second own issue", project=attacker_project, workspace=attacker_workspace) + attacker_issue.archived_at = timezone.now().date() + attacker_issue.save(update_fields=["archived_at"]) + + url = relation_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"relation_type": "blocking", "issues": [str(other.id)]}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) From 9fd1d537e25b97b8f9bede34e908c4156ae6683e Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Mon, 3 Aug 2026 16:35:56 +0530 Subject: [PATCH 3/3] [SECUR-243] fix(security): scope issue relation list to the URL project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot review on #9531. IssueRelationViewSet.list had the same unscoped issue_id as the write handlers: it filtered relations on workspace__slug only, so a member of project A could list the relations of an issue in project B of the same workspace and receive that issue's name, priority, sequence_id, assignee_ids and label_ids. Verified 200 with the foreign issue's name in the body before the fix. Extracts the binding into a module-level issue_in_project() helper (per CodeRabbit) now that list, create and remove_relation all need it — one place to get right, and the next handler added to this file has an obvious thing to call. Adds contract coverage for the list path (cross-project 404 with no name leak, plus an in-project positive control), extends the IssueManager boundary tests to remove_relation and list so all handlers are pinned to the same manager, and pins that a non-UUID related_issue is a 400 rather than a 500 — Copilot flagged that as a 500 risk, but Django raises ValidationError on UUID coercion and BaseViewSet.handle_exception already converts it. The test locks that in. Co-authored-by: Plane AI --- apps/api/plane/app/views/issue/relation.py | 34 +++-- .../app/test_issue_subresource_scope_app.py | 122 +++++++++++++++++- 2 files changed, 144 insertions(+), 12 deletions(-) diff --git a/apps/api/plane/app/views/issue/relation.py b/apps/api/plane/app/views/issue/relation.py index d790c90b300..b738c41f05a 100644 --- a/apps/api/plane/app/views/issue/relation.py +++ b/apps/api/plane/app/views/issue/relation.py @@ -34,12 +34,30 @@ from plane.utils.host import base_host +def issue_in_project(issue_id, slug, project_id): + """Whether ``issue_id`` belongs to the URL workspace + project. + + ``ProjectEntityPermission`` only checks that the caller is a member of the URL's + ``project_id`` — it never binds the sibling ``issue_id`` path parameter to that + project. Every handler taking both must therefore check this itself, or it is + reachable cross-project/cross-tenant (GHSA-hvx3-58mp-5fpx). + """ + return Issue.issue_objects.filter(pk=issue_id, workspace__slug=slug, project_id=project_id).exists() + + class IssueRelationViewSet(BaseViewSet): serializer_class = IssueRelationSerializer model = IssueRelation permission_classes = [ProjectEntityPermission] def list(self, request, slug, project_id, issue_id): + # SECURITY: the read path needs the same binding as the writes below. Without + # it a member of project A could list the relations of an issue in project B of + # the same workspace and receive that issue's name, priority, assignees and + # labels in the response. + if not issue_in_project(issue_id, slug, project_id): + return Response({"error": "Issue not found"}, status=status.HTTP_404_NOT_FOUND) + issue_relations = ( IssueRelation.objects.filter(Q(issue_id=issue_id) | Q(related_issue=issue_id)) .filter(workspace__slug=self.kwargs.get("slug")) @@ -219,11 +237,9 @@ def create(self, request, slug, project_id, issue_id): # SECURITY: bind the URL issue to the workspace + project before using it as one # side of the relation. The body `issues` are scoped below, but `issue_id` came - # straight from the URL and ProjectEntityPermission only checks the caller's - # membership of `project_id`, not that `issue_id` belongs to it — so the earlier - # scoping fix covered one side of the relationship and missed the other - # (GHSA-hvx3-58mp-5fpx). - if not Issue.issue_objects.filter(pk=issue_id, workspace__slug=slug, project_id=project_id).exists(): + # straight from the URL — so the earlier scoping fix covered one side of the + # relationship and missed the other. + if not issue_in_project(issue_id, slug, project_id): return Response({"error": "Issue not found"}, status=status.HTTP_404_NOT_FOUND) # Scope to workspace to prevent cross-tenant IDOR @@ -283,10 +299,10 @@ def remove_relation(self, request, slug, project_id, issue_id): # SECURITY: same binding as create() — the URL issue must belong to the URL # project before it can be used to select a relation for deletion. Otherwise a # member of one project could delete relations between issues of a sibling - # project in the same workspace (GHSA-hvx3-58mp-5fpx). The IssueRelation row - # itself stays workspace-scoped only: relations legitimately span projects, and - # either participant's project may remove them. - if not Issue.issue_objects.filter(pk=issue_id, workspace__slug=slug, project_id=project_id).exists(): + # project in the same workspace. The IssueRelation row itself stays + # workspace-scoped only: relations legitimately span projects, and either + # participant's project may remove them. + if not issue_in_project(issue_id, slug, project_id): return Response({"error": "Issue not found"}, status=status.HTTP_404_NOT_FOUND) issue_relations = IssueRelation.objects.filter( diff --git a/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py b/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py index c532caee334..ecc38d1574c 100644 --- a/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py +++ b/apps/api/plane/tests/contract/app/test_issue_subresource_scope_app.py @@ -8,8 +8,8 @@ ``ProjectEntityPermission`` / ``allow_permission`` validate only that the caller is an active member of the URL's ``project_id``. Neither validates that the sibling -``issue_id`` path parameter belongs to that project or workspace, and three write -handlers then used ``issue_id`` unscoped: +``issue_id`` path parameter belongs to that project or workspace, and four handlers +then used ``issue_id`` unscoped: * ``IssueCommentViewSet.create`` — ``Issue.objects.get(pk=issue_id)``. The 201 response serializes the foreign issue's full ``issue_detail`` (name, description, @@ -20,8 +20,13 @@ * ``IssueRelationViewSet.remove_relation`` — same gap on the delete path, found while reviewing the two above: workspace-scoped only, so a member of one project could delete relations belonging to a sibling project in the same workspace. +* ``IssueRelationViewSet.list`` — same gap on the read path, raised by Copilot on + PR #9531: a member of project A could list the relations of an issue in project B of + the same workspace and receive its ``name``, ``priority``, ``assignee_ids`` and + ``label_ids``. -All three bind the lookup to ``workspace__slug`` + ``project_id`` and 404 otherwise. +All four bind the lookup to ``workspace__slug`` + ``project_id`` (via +``relation.issue_in_project``) and 404 otherwise. """ from unittest import mock @@ -324,6 +329,61 @@ def test_allows_removal_within_project(self, attacker_client, attacker_workspace assert relation.deleted_at is not None +@pytest.mark.contract +class TestIssueRelationListScope: + """The read path needs the same binding as the writes. + + Raised by Copilot on PR #9531: ``list`` filtered relations on ``workspace__slug`` + only, so a member of project A could read the relations of an issue in project B of + the same workspace — returning that issue's ``name``, ``priority``, ``assignee_ids`` + and ``label_ids``. + """ + + @pytest.mark.django_db + def test_rejects_same_workspace_other_project_url_issue( + self, attacker_client, attacker_workspace, attacker_project, sibling_project_issue + ): + sibling_project = sibling_project_issue.project + related = Issue.objects.create( + name="Sibling relation target", project=sibling_project, workspace=attacker_workspace + ) + IssueRelation.objects.create( + issue=related, + related_issue=sibling_project_issue, + relation_type="blocked_by", + project=sibling_project, + workspace=attacker_workspace, + ) + + url = relation_url(attacker_workspace.slug, attacker_project.id, sibling_project_issue.id) + response = attacker_client.get(url) + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {str(getattr(response, 'data', None))[:300]}" + ) + assert "Sibling relation target" not in str(getattr(response, "data", "")) + + @pytest.mark.django_db + def test_allows_listing_within_project(self, attacker_client, attacker_workspace, attacker_project, attacker_issue): + """Positive control: listing an in-project issue's relations still works.""" + other = Issue.objects.create(name="Second own issue", project=attacker_project, workspace=attacker_workspace) + IssueRelation.objects.create( + issue=other, + related_issue=attacker_issue, + relation_type="blocked_by", + project=attacker_project, + workspace=attacker_workspace, + ) + + url = relation_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.get(url) + + assert response.status_code == status.HTTP_200_OK, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert "Second own issue" in str(response.data), "In-project relation went missing" + + @pytest.mark.contract class TestIssueManagerBoundary: """Pin the one axis on which the two fixes deliberately differ. @@ -380,3 +440,59 @@ def test_relation_create_excludes_archived_url_issue( assert response.status_code == status.HTTP_404_NOT_FOUND, ( f"Got {response.status_code}: {getattr(response, 'data', None)!r}" ) + + @pytest.mark.django_db + def test_relation_remove_excludes_archived_url_issue( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue + ): + """Same narrowing on the delete path (raised by Copilot on PR #9531).""" + other = Issue.objects.create(name="Second own issue", project=attacker_project, workspace=attacker_workspace) + IssueRelation.objects.create( + issue=other, + related_issue=attacker_issue, + relation_type="blocked_by", + project=attacker_project, + workspace=attacker_workspace, + ) + attacker_issue.archived_at = timezone.now().date() + attacker_issue.save(update_fields=["archived_at"]) + + url = remove_relation_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"related_issue": str(other.id)}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + + @pytest.mark.django_db + def test_relation_list_excludes_archived_url_issue( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue + ): + """And on the read path, so all three handlers are pinned to the same manager.""" + attacker_issue.archived_at = timezone.now().date() + attacker_issue.save(update_fields=["archived_at"]) + + url = relation_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.get(url) + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + + @pytest.mark.django_db + def test_non_uuid_related_issue_is_400_not_500( + self, attacker_client, attacker_workspace, attacker_project, attacker_issue + ): + """``related_issue`` goes straight into an ORM filter. + + Copilot flagged this as a 500 risk on PR #9531. It is not: Django raises + ``ValidationError`` on the UUID coercion and ``BaseViewSet.handle_exception`` + converts that to a 400. Pinned so a future change to that handler can't + silently turn malformed input into a 500. + """ + url = remove_relation_url(attacker_workspace.slug, attacker_project.id, attacker_issue.id) + response = attacker_client.post(url, {"related_issue": "not-a-uuid"}, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + )