[WEB-8289] fix: scope draft-to-issue conversion to the draft owner (GHSA-vfqm-7rh7-84hq)#9443
[WEB-8289] fix: scope draft-to-issue conversion to the draft owner (GHSA-vfqm-7rh7-84hq)#9443mguptahub wants to merge 2 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 |
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 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 |
Summary
Fixes GHSA-vfqm-7rh7-84hq (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