[WEB-7776] fix(security): scope FileAsset queries to prevent cross-project IDOR (Cluster F)#9288
[WEB-7776] fix(security): scope FileAsset queries to prevent cross-project IDOR (Cluster F)#9288mguptahub wants to merge 3 commits into
Conversation
…oject 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 <noreply@plane.so>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAsset endpoints now require authenticated workspace membership, workspace-logo uploads are limited to admins, and asset lookups in the workspace and space views are scoped more tightly by workspace and project. ChangesAsset Authorization Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
| if not WorkspaceMember.objects.filter(workspace_id=workspace_id, member=request.user, is_active=True).exists(): | ||
| return Response( | ||
| {"error": "Requested resource could not be found.", "status": False}, | ||
| status=status.HTTP_404_NOT_FOUND, | ||
| ) |
There was a problem hiding this comment.
the below changes can be removed after adding permisison class to the view
There was a problem hiding this comment.
Done — added WorkspaceMemberPermission to plane/app/permissions/workspace.py. It resolves the workspace via workspace_id (UUID) or slug kwarg, covering both URL patterns on this endpoint. FileAssetEndpoint and FileAssetViewSet now declare permission_classes = [IsAuthenticated, WorkspaceMemberPermission] and all inline membership checks have been removed.
There was a problem hiding this comment.
Removed — all inline WorkspaceMember checks are gone from get, post, delete, and restore. The permission class handles enforcement before the method body runs.
…mberPermission 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 <noreply@plane.so>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/plane/app/views/asset/base.py`:
- Line 20: The missing-workspace 404 path in FileAssetEndpoint.post() is
unreachable because WorkspaceMemberPermission rejects unknown slugs before the
view logic runs. Update the permission flow used by BaseView/FileAssetEndpoint
so workspace existence is checked there, or remove/adjust the
Workspace.objects.filter(slug=slug).first() 404 branch and its expectation to
match the actual authorization behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c30b2a9-b03a-4974-95b2-28e10003facc
📒 Files selected for processing (3)
apps/api/plane/app/permissions/__init__.pyapps/api/plane/app/permissions/workspace.pyapps/api/plane/app/views/asset/base.py
✅ Files skipped from review due to trivial changes (1)
- apps/api/plane/app/permissions/init.py
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 <noreply@plane.so>
Summary
Multiple asset endpoints were missing project-level scoping on
FileAssetqueryset filters, allowing authenticated users to access, mark-uploaded, or restore assets belonging to other projects or workspaces they are not members of.Fixes applied
ProjectBulkAssetEndpoint.postapp/views/asset/v2.pyproject_id=project_idto asset filterEntityAssetEndpoint.getspace/views/asset.pyproject_id=deploy_board.project_idEntityAssetEndpoint.patchspace/views/asset.pyproject_id=deploy_board.project_idAssetRestoreEndpoint.postspace/views/asset.pyproject_id=deploy_board.project_idFileAssetEndpointV1 (get/post/delete)app/views/asset/base.pyWorkspaceMembermembership checkFileAssetViewSet.restoreV1app/views/asset/base.pyWorkspaceMembermembership checkWorkspaceFileAssetEndpoint.postapp/views/asset/v2.pyWORKSPACE_LOGOupload onROLE.ADMINDuplicateAssetEndpoint.postapp/views/asset/v2.pyAdvisories addressed
GHSA-r2hw (critical), GHSA-jh4v (high), GHSA-8688 (high), GHSA-3hrj, GHSA-3892, GHSA-3ggg, GHSA-gcpp, GHSA-p57q, GHSA-c68q, GHSA-8chr, GHSA-58qm, GHSA-wrrw, GHSA-j4mj, GHSA-85h2, GHSA-29q3, GHSA-mwh2, GHSA-xrpv and related duplicates.
Test plan
POST /api/v1/workspaces/{slug}/projects/{projectB_id}/bulk-asset-save/with the asset ID — should return 404PATCH /spaces/{anchor}/assets/{asset_from_other_project}/— should return 404POST /spaces/{anchor}/assets/{id}/restore/with asset from a different project — should return 404GET /api/workspaces/{ws_id}/{asset_key}from a user not in the workspace — should return 404POST /api/v1/workspaces/{slug}/file-assets/withentity_type=WORKSPACE_LOGO— should return 403DuplicateAssetEndpoint: attempt to duplicate an asset from workspace B while calling endpoint in workspace A — should return 404Co-authored-by: Plane AI noreply@plane.so
Summary by CodeRabbit
Release Notes