From 4577dc3f7a6b5a198602b602a45c6b0abdc7204b Mon Sep 17 00:00:00 2001 From: Manish Gupta <59428681+mguptahub@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:26:59 +0530 Subject: [PATCH] [WEB-7776] fix(security): scope FileAsset queries to prevent cross-project IDOR (Cluster F) (#9288) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [WEB-7776] fix(security): scope FileAsset queries to prevent cross-project IDOR (Cluster F) Multiple asset endpoints were missing project-level scoping on FileAsset queryset filters, allowing authenticated users to access, mark-uploaded, or restore assets belonging to other projects/workspaces. - ProjectBulkAssetEndpoint.post: add project_id= scope to asset filter - EntityAssetEndpoint.get/patch: add project_id=deploy_board.project_id - AssetRestoreEndpoint.post: add project_id=deploy_board.project_id - FileAssetEndpoint (V1): add workspace membership check on get/post/delete - FileAssetViewSet.restore (V1): add workspace membership check - WorkspaceFileAssetEndpoint.post: gate WORKSPACE_LOGO on ADMIN role - DuplicateAssetEndpoint.post: restrict source asset to same workspace Fixes GHSA-r2hw, GHSA-jh4v, GHSA-8688, GHSA-3hrj and related advisories. Co-authored-by: Plane AI * refactor(security): replace inline membership checks with WorkspaceMemberPermission class Add WorkspaceMemberPermission to workspace.py — resolves workspace by 'workspace_id' UUID or 'slug' kwarg, covering the mixed URL patterns on FileAssetEndpoint. Apply to FileAssetEndpoint and FileAssetViewSet so membership enforcement lives in the permission layer, not inside each method handler. Co-authored-by: Plane AI * refactor: remove dead 404 guard in FileAssetEndpoint.post() WorkspaceMemberPermission denies requests for non-existent slugs before the view method runs, making the filter().first() + if not workspace branch unreachable. Switch to .get() so any TOCTOU race still surfaces as a 404 via ObjectDoesNotExist. Co-authored-by: Plane AI --------- Co-authored-by: Plane AI --- apps/api/plane/app/permissions/__init__.py | 1 + apps/api/plane/app/permissions/workspace.py | 27 +++++++++++++++++++++ apps/api/plane/app/views/asset/base.py | 10 ++++++-- apps/api/plane/app/views/asset/v2.py | 23 ++++++++++++------ apps/api/plane/space/views/asset.py | 11 +++++---- 5 files changed, 57 insertions(+), 15 deletions(-) diff --git a/apps/api/plane/app/permissions/__init__.py b/apps/api/plane/app/permissions/__init__.py index 22d27694e9..457a13cb7b 100644 --- a/apps/api/plane/app/permissions/__init__.py +++ b/apps/api/plane/app/permissions/__init__.py @@ -9,6 +9,7 @@ from .workspace import ( WorkspaceEntityPermission, WorkspaceViewerPermission, WorkspaceUserPermission, + WorkspaceMemberPermission, ) from .project import ( ProjectBasePermission, diff --git a/apps/api/plane/app/permissions/workspace.py b/apps/api/plane/app/permissions/workspace.py index d979e608f9..4acadb8c1a 100644 --- a/apps/api/plane/app/permissions/workspace.py +++ b/apps/api/plane/app/permissions/workspace.py @@ -108,3 +108,30 @@ class WorkspaceUserPermission(BasePermission): return WorkspaceMember.objects.filter( member=request.user, workspace__slug=view.workspace_slug, is_active=True ).exists() + + +class WorkspaceMemberPermission(BasePermission): + """Allows access only to active workspace members. + + Resolves the workspace via 'slug' or 'workspace_id' in URL kwargs so this + class can be used on endpoints that identify the workspace by either + identifier (e.g. FileAssetEndpoint which mixes both URL patterns). + """ + + def has_permission(self, request, view): + if request.user.is_anonymous: + return False + + workspace_id = view.kwargs.get("workspace_id") + if workspace_id: + return WorkspaceMember.objects.filter( + workspace_id=workspace_id, member=request.user, is_active=True + ).exists() + + slug = view.kwargs.get("slug") + if slug: + return WorkspaceMember.objects.filter( + workspace__slug=slug, member=request.user, is_active=True + ).exists() + + return False diff --git a/apps/api/plane/app/views/asset/base.py b/apps/api/plane/app/views/asset/base.py index 5b55a76a61..af7a547e06 100644 --- a/apps/api/plane/app/views/asset/base.py +++ b/apps/api/plane/app/views/asset/base.py @@ -4,17 +4,20 @@ # Third party imports from rest_framework import status +from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.parsers import MultiPartParser, FormParser, JSONParser # Module imports from ..base import BaseAPIView, BaseViewSet +from plane.app.permissions import WorkspaceMemberPermission from plane.db.models import FileAsset, Workspace from plane.app.serializers import FileAssetSerializer class FileAssetEndpoint(BaseAPIView): parser_classes = (MultiPartParser, FormParser, JSONParser) + permission_classes = [IsAuthenticated, WorkspaceMemberPermission] """ A viewset for viewing and editing task instances. @@ -33,10 +36,11 @@ class FileAssetEndpoint(BaseAPIView): ) def post(self, request, slug): + # WorkspaceMemberPermission already rejects unknown slugs before this runs. + # Use .get() so any TOCTOU race still surfaces as a 404 via ObjectDoesNotExist. + workspace = Workspace.objects.get(slug=slug) serializer = FileAssetSerializer(data=request.data) if serializer.is_valid(): - # Get the workspace - workspace = Workspace.objects.get(slug=slug) serializer.save(workspace_id=workspace.id) return Response(serializer.data, status=status.HTTP_201_CREATED) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -50,6 +54,8 @@ class FileAssetEndpoint(BaseAPIView): class FileAssetViewSet(BaseViewSet): + permission_classes = [IsAuthenticated, WorkspaceMemberPermission] + def restore(self, request, workspace_id, asset_key): asset_key = str(workspace_id) + "/" + asset_key file_asset = FileAsset.objects.get(asset=asset_key) diff --git a/apps/api/plane/app/views/asset/v2.py b/apps/api/plane/app/views/asset/v2.py index b21f70d61f..8441364f58 100644 --- a/apps/api/plane/app/views/asset/v2.py +++ b/apps/api/plane/app/views/asset/v2.py @@ -327,6 +327,17 @@ class WorkspaceFileAssetEndpoint(BaseAPIView): status=status.HTTP_400_BAD_REQUEST, ) + # WORKSPACE_LOGO may only be uploaded by workspace admins + if entity_type == FileAsset.EntityTypeContext.WORKSPACE_LOGO: + workspace_member = WorkspaceMember.objects.filter( + workspace__slug=slug, member=request.user, is_active=True + ).first() + if not workspace_member or workspace_member.role != ROLE.ADMIN.value: + return Response( + {"error": "Only workspace admins can upload a workspace logo."}, + status=status.HTTP_403_FORBIDDEN, + ) + # Check if the file type is allowed allowed_types = [ "image/jpeg", @@ -646,8 +657,8 @@ class ProjectBulkAssetEndpoint(BaseAPIView): if not asset_ids: return Response({"error": "No asset ids provided."}, status=status.HTTP_400_BAD_REQUEST) - # get the asset id - assets = FileAsset.objects.filter(id__in=asset_ids, workspace__slug=slug) + # get the asset id — scope to the project to prevent cross-project IDOR + assets = FileAsset.objects.filter(id__in=asset_ids, workspace__slug=slug, project_id=project_id) # Get the first asset asset = assets.first() @@ -757,15 +768,11 @@ class DuplicateAssetEndpoint(BaseAPIView): return Response({"error": "Project not found"}, status=status.HTTP_404_NOT_FOUND) storage = S3Storage(request=request) - # Scope the source asset lookup to workspaces the caller is a member of - user_workspace_ids = WorkspaceMember.objects.filter( - member=request.user, - is_active=True, - ).values_list("workspace_id", flat=True) + # Restrict the source asset to the same destination workspace to prevent cross-workspace asset copying original_asset = FileAsset.objects.filter( id=asset_id, is_uploaded=True, - workspace_id__in=user_workspace_ids, + workspace=workspace, ).first() if not original_asset: diff --git a/apps/api/plane/space/views/asset.py b/apps/api/plane/space/views/asset.py index bc20724ca8..5220202fef 100644 --- a/apps/api/plane/space/views/asset.py +++ b/apps/api/plane/space/views/asset.py @@ -42,9 +42,10 @@ class EntityAssetEndpoint(BaseAPIView): status=status.HTTP_404_NOT_FOUND, ) - # get the asset id + # get the asset id — scope to project to prevent cross-project IDOR asset = FileAsset.objects.get( workspace_id=deploy_board.workspace_id, + project_id=deploy_board.project_id, pk=pk, entity_type__in=[ FileAsset.EntityTypeContext.ISSUE_DESCRIPTION, @@ -140,8 +141,8 @@ class EntityAssetEndpoint(BaseAPIView): if not deploy_board: return Response({"error": "Project is not published"}, status=status.HTTP_404_NOT_FOUND) - # get the asset id - asset = FileAsset.objects.get(id=pk, workspace=deploy_board.workspace) + # get the asset id — scope to project to prevent cross-project IDOR + asset = FileAsset.objects.get(id=pk, workspace=deploy_board.workspace, project_id=deploy_board.project_id) # get the storage metadata asset.is_uploaded = True # get the storage metadata @@ -180,8 +181,8 @@ class AssetRestoreEndpoint(BaseAPIView): if not deploy_board: return Response({"error": "Project is not published"}, status=status.HTTP_404_NOT_FOUND) - # Get the asset - asset = FileAsset.all_objects.get(id=pk, workspace=deploy_board.workspace) + # Get the asset — scope to project to prevent cross-project IDOR + asset = FileAsset.all_objects.get(id=pk, workspace=deploy_board.workspace, project_id=deploy_board.project_id) asset.is_deleted = False asset.deleted_at = None asset.save(update_fields=["is_deleted", "deleted_at"])