[WEB-8074] fix: scope IssueListEndpoint to guest created_by#9374
[WEB-8074] fix: scope IssueListEndpoint to guest created_by#9374mguptahub wants to merge 1 commit into
Conversation
IssueListEndpoint.get (/workspaces/<slug>/projects/<project_id>/issues/list/) returned any issue whose id was passed in ?issues=, without the guest created_by restriction its sibling IssueViewSet.list enforces. A project GUEST (role=5) on a project with guest_view_all_features=False could read issues they did not author by supplying their ids (GHSA-32c7-84jc-4w67). Replicate the guest scope: when the requester is an active role=5 ProjectMember and not project.guest_view_all_features, filter the queryset to created_by=request.user. Applied to the base queryset so it flows through filtering, annotation and grouping. Contract regression tests cover the restricted guest (own-only), a full member (sees all), and a guest with guest_view_all_features enabled (sees all); fail-before verified. Co-authored-by: Plane AI <noreply@plane.so>
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
📝 WalkthroughWalkthroughAdds a guest-visibility restriction to IssueListEndpoint.get that limits issue-list results to issues created by the requester when the requester is a role-5 project member and the project's guest_view_all_features flag is disabled. Adds a corresponding contract test module. ChangesGuest issue list scoping
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Guest
participant IssueListEndpoint
participant Project
participant IssueQueryset
Guest->>IssueListEndpoint: GET issue list (issue ids)
IssueListEndpoint->>Project: fetch project
IssueListEndpoint->>IssueListEndpoint: check role==5 and guest_view_all_features
alt guest restricted
IssueListEndpoint->>IssueQueryset: filter created_by=request.user
end
IssueListEndpoint-->>Guest: return filtered issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/plane/app/views/issue/base.py (1)
96-109: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOptimize: avoid unconditional
Project.objects.get()for non-guest requests.The
Project.objects.get()on line 98 executes on every request to this endpoint, even for non-guest users who don't need the guest restriction. Similarly, theProjectMember.objects.filter(...).exists()check always runs. Sinceprojectis only used forguest_view_all_features, you can leverage Python's short-circuitandto skip the Project query entirely for non-guests by moving the ProjectMember check first:♻️ Proposed refactor
- # Restrict guests without full feature access to issues they created, - # mirroring IssueViewSet.list. - project = Project.objects.get(pk=project_id, workspace__slug=slug) - if ( - ProjectMember.objects.filter( - workspace__slug=slug, - project_id=project_id, - member=request.user, - role=5, - is_active=True, - ).exists() - and not project.guest_view_all_features - ): - queryset = queryset.filter(created_by=request.user) + # Restrict guests without full feature access to issues they created, + # mirroring IssueViewSet.list. + if ( + ProjectMember.objects.filter( + workspace__slug=slug, + project_id=project_id, + member=request.user, + role=5, + is_active=True, + ).exists() + and not Project.objects.filter( + pk=project_id, workspace__slug=slug + ).values_list("guest_view_all_features", flat=True).first() + ): + queryset = queryset.filter(created_by=request.user)This reduces from 2 queries to 1 for non-guest users (the majority), since
andshort-circuits when theProjectMembercheck returnsFalse. For guests, it remains 2 queries but avoids constructing a fullProjectmodel instance.🤖 Prompt for 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. In `@apps/api/plane/app/views/issue/base.py` around lines 96 - 109, In the issue filtering logic in the base issue view, the Project lookup runs unconditionally even when the requester is not a guest, causing avoidable database work. Reorder the guest restriction check so the ProjectMember existence test is evaluated first and use short-circuiting to skip the Project.objects.get call unless the user is actually an active guest member; keep the created_by filtering behavior the same and preserve the existing guest_view_all_features gate in the same branch.
🤖 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.
Nitpick comments:
In `@apps/api/plane/app/views/issue/base.py`:
- Around line 96-109: In the issue filtering logic in the base issue view, the
Project lookup runs unconditionally even when the requester is not a guest,
causing avoidable database work. Reorder the guest restriction check so the
ProjectMember existence test is evaluated first and use short-circuiting to skip
the Project.objects.get call unless the user is actually an active guest member;
keep the created_by filtering behavior the same and preserve the existing
guest_view_all_features gate in the same branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 510f19ca-beb9-421a-b03d-1034124ef10b
📒 Files selected for processing (2)
apps/api/plane/app/views/issue/base.pyapps/api/plane/tests/contract/app/test_issue_list_guest_scope_app.py
There was a problem hiding this comment.
Pull request overview
Fixes an authorization gap in the IssueListEndpoint.get API (/workspaces/<slug>/projects/<project_id>/issues/list/) where restricted project guests could fetch arbitrary issues by ID, by applying the same guest created_by scoping used elsewhere when guest_view_all_features=False.
Changes:
- Apply guest scoping to the base IssueListEndpoint queryset so restricted guests only see issues they created.
- Add contract regression tests covering restricted guests, full members, and guests with
guest_view_all_features=True.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/api/plane/app/views/issue/base.py | Adds guest created_by scoping for IssueListEndpoint so restricted guests can’t fetch non-authored issues by ID. |
| apps/api/plane/tests/contract/app/test_issue_list_guest_scope_app.py | Adds contract tests to prevent regressions of the guest scoping vulnerability (GHSA-32c7-84jc-4w67). |
| # Restrict guests without full feature access to issues they created, | ||
| # mirroring IssueViewSet.list. | ||
| project = Project.objects.get(pk=project_id, workspace__slug=slug) | ||
| if ( | ||
| ProjectMember.objects.filter( | ||
| workspace__slug=slug, | ||
| project_id=project_id, | ||
| member=request.user, | ||
| role=5, | ||
| is_active=True, | ||
| ).exists() | ||
| and not project.guest_view_all_features | ||
| ): | ||
| queryset = queryset.filter(created_by=request.user) |
Summary
IssueListEndpoint.get(/workspaces/<slug>/projects/<project_id>/issues/list/) returned any issue whose id was supplied in?issues=, without applying the guestcreated_byrestriction that its siblingIssueViewSet.listenforces. A project GUEST (role=5) on a project withguest_view_all_features=Falsecould therefore read issues they did not author by passing their ids.Advisory: GHSA-32c7-84jc-4w67 · MEDIUM.
Fix
Replicate the guest scope from
IssueViewSet.list: when the requester is an active role=5ProjectMemberandnot project.guest_view_all_features, filter the queryset tocreated_by=request.user. Applied to the base queryset so it flows through filtering, annotation, and grouping.(
Project.objects.get()is safe here —@allow_permission(level=PROJECT)already guarantees the requester is a project member, same as the sibling.)Tests
plane/tests/contract/app/test_issue_list_guest_scope_app.py:guest_view_all_features=Truegets every requested issue.Fail-before verified: with the fix stashed, the restricted guest reads the foreign issue; the two positive controls pass regardless.
Gates: ruff ✅ ·
manage.py check✅.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests