[WEB-8289] fix: scope draft-to-issue conversion to the draft owner - #9443
[WEB-8289] fix: scope draft-to-issue conversion to the draft owner#9443mguptahub wants to merge 3 commits into
Conversation
…HSA-vfqm-7rh7-84hq) WorkspaceDraftIssueViewSet.create_draft_to_issue resolved the draft with get_queryset().filter(pk=draft_id) — scoped only to the workspace, no created_by check — while the decorator admits any workspace ADMIN/MEMBER. Drafts are private to their creator, so any member could pass another user's draft_id to convert that draft into an issue, reassign (steal) its file assets, and delete the victim's draft. Scope the lookup to created_by=request.user (mirroring retrieve / partial_update) and 404 when no owned draft matches. The explicit null guard also removes a latent AttributeError on the following .project_id access when the draft is not found. Adds contract regression tests (fail-before verified): a non-owner member is rejected with 404 and the draft + its assets are left intact, with a positive control confirming the owner can still convert. Co-authored-by: Plane AI <noreply@plane.so>
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughDraft-to-issue conversion now restricts lookup to the draft creator and returns ChangesDraft conversion ownership
Estimated code review effort: 2 (Simple) | ~10 minutes 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes a high-severity authorization bug in the draft-to-issue conversion endpoint by ensuring only the draft owner can convert their draft into an issue, preventing other workspace members from converting/deleting someone else’s private drafts and migrating their file assets.
Changes:
- Scope
create_draft_to_issuedraft lookup tocreated_by=request.userand return 404 when not found/owned. - Add contract tests covering the negative (non-owner) and positive (owner) conversion paths, including file-asset migration/retention assertions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/api/plane/app/views/workspace/draft.py | Restricts draft-to-issue conversion to drafts owned by the requesting user and returns a 404 otherwise. |
| apps/api/plane/tests/contract/app/test_draft_to_issue_owner_scope_app.py | Adds contract regression tests validating owner scoping and ensuring file assets aren’t reassigned/deleted by non-owners. |
Project and FileAsset fixtures passed created_by=create_user to objects.create(), but BaseModel.save() nulls created_by when there is no current request user (tests), so the ownership claim was silently a no-op. Instantiate + save(created_by_id=...) instead — same pattern already used for the draft — so the fixtures are accurate. Co-authored-by: Plane AI <noreply@plane.so>
|
Thanks @copilot — both addressed in You're right: Note this didn't affect the test's correctness (the security assertions key off the draft's |
Explanations kept unchanged; only the IDs are removed. Co-authored-by: Plane AI <noreply@plane.so>
|
React Doctor found 4 issues in 4 files · 1 error & 3 warnings · score 82 / 100 (Needs work) · vs Errors
3 warnings
|
Summary
Fixes WEB-8289 (HIGH).
WorkspaceDraftIssueViewSet.create_draft_to_issueresolved the draft withget_queryset().filter(pk=draft_id)— scoped only to the workspace, with nocreated_bycheck — while the decorator admits any workspace ADMIN/MEMBER.Draft issues are private to their creator (the sibling
list/retrieve/partial_updatemethods all filtercreated_by=request.user). So any workspace member could supply another user'sdraft_idand:FileAsset.objects.filter(draft_issue_id=draft_id).update(issue_id=...)), anddraft_issue.delete()).Fix
Scope the lookup to the owner and 404 otherwise:
This mirrors the exact pattern already used by
retrieve/partial_update. The explicit null guard also removes a latentAttributeErroron the subsequent.project_idaccess when no draft matches.Tests
New contract suite
plane/tests/contract/app/test_draft_to_issue_owner_scope_app.py(fail-before verified — the security test returns 201 without the fix):Verification
ruff check: cleanmanage.py check(test settings): no issuespytest(new suite): 2/2 green; fail-before verifiedNotes
DraftIssuePropertyValueEndpoint(draft custom-field values, same-project) is being tracked as a separate follow-up, not part of this advisory.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests