Skip to content

[WEB-8289] fix: scope draft-to-issue conversion to the draft owner (GHSA-vfqm-7rh7-84hq)#9443

Open
mguptahub wants to merge 2 commits into
previewfrom
web-8289/draft-to-issue-owner-scope
Open

[WEB-8289] fix: scope draft-to-issue conversion to the draft owner (GHSA-vfqm-7rh7-84hq)#9443
mguptahub wants to merge 2 commits into
previewfrom
web-8289/draft-to-issue-owner-scope

Conversation

@mguptahub

@mguptahub mguptahub commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes GHSA-vfqm-7rh7-84hq (HIGH). WorkspaceDraftIssueViewSet.create_draft_to_issue resolved the draft with get_queryset().filter(pk=draft_id) — scoped only to the workspace, with no created_by check — while the decorator admits any workspace ADMIN/MEMBER.

Draft issues are private to their creator (the sibling list / retrieve / partial_update methods all filter created_by=request.user). So any workspace member could supply another user's draft_id and:

  • convert that private draft into a real issue,
  • reassign (steal) the draft's file assets to the new issue (FileAsset.objects.filter(draft_issue_id=draft_id).update(issue_id=...)), and
  • delete the victim's draft (draft_issue.delete()).

Fix

Scope the lookup to the owner and 404 otherwise:

draft_issue = self.get_queryset().filter(pk=draft_id, created_by=request.user).first()
if not draft_issue:
    return Response({"error": "The required object does not exist."}, status=404)

This mirrors the exact pattern already used by retrieve / partial_update. The explicit null guard also removes a latent AttributeError on the subsequent .project_id access 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):

  • A non-owner member converting another user's draft → 404, and the draft + its file asset are left intact (not deleted, not reassigned).
  • Positive control: the draft's owner can still convert their own draft (201, draft consumed, asset migrated to the new issue).

Verification

  • ruff check: clean
  • manage.py check (test settings): no issues
  • pytest (new suite): 2/2 green; fail-before verified

Notes

  • Dedupe: GHSA-mc57-3cp7-wvwx (MED) is the same root cause + code path — should be closed as a duplicate of this on release.
  • EE: EE does not override this endpoint, so this CE fix covers EE deployments too. A distinct, lower-severity owner-scope gap on the EE-only 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

    • Restricted “draft-to-issue” conversion so only the draft’s owner can convert it.
    • Unauthorized attempts now return 404 Not Found and do not alter the draft, its attached files, or create an issue.
    • Draft owners can still convert their drafts successfully, including transferring any attached files to the new issue.
  • Tests

    • Added contract tests covering both unauthorized and authorized draft-to-issue conversion paths.

…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>
Copilot AI review requested due to automatic review settings July 20, 2026 05:21
@makeplane

makeplane Bot commented Jul 20, 2026

Copy link
Copy Markdown

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9ef391db-c96a-4458-8e1d-bd2c1b27bc60

📥 Commits

Reviewing files that changed from the base of the PR and between 656acf7 and 7448c69.

📒 Files selected for processing (1)
  • apps/api/plane/tests/contract/app/test_draft_to_issue_owner_scope_app.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/api/plane/tests/contract/app/test_draft_to_issue_owner_scope_app.py

📝 Walkthrough

Walkthrough

Draft-to-issue conversion now restricts lookup to the draft creator and returns 404 for non-owners. Contract tests cover unauthorized access, successful owner conversion, issue creation, draft deletion, and file asset migration.

Changes

Draft conversion ownership

Layer / File(s) Summary
Enforce draft creator ownership
apps/api/plane/app/views/workspace/draft.py
Draft lookup now filters by both ID and creator, returning the standard 404 response when no scoped draft exists.
Validate conversion ownership behavior
apps/api/plane/tests/contract/app/test_draft_to_issue_owner_scope_app.py
Contract fixtures and tests verify that non-owners cannot convert drafts while owners can convert them with the expected draft, issue, and asset state changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: dheeru0198

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: scoping draft-to-issue conversion to the draft owner.
Description check ✅ Passed The description is detailed and covers the change, tests, verification, and references, though it doesn't follow the template headings exactly.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch web-8289/draft-to-issue-owner-scope

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_issue draft lookup to created_by=request.user and 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.

Comment thread apps/api/plane/tests/contract/app/test_draft_to_issue_owner_scope_app.py Outdated
Comment thread apps/api/plane/tests/contract/app/test_draft_to_issue_owner_scope_app.py Outdated
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>
@mguptahub

Copy link
Copy Markdown
Collaborator Author

Thanks @copilot — both addressed in 7448c69.

You're right: Project.objects.create(created_by=create_user) and FileAsset.objects.create(created_by=create_user) were silent no-ops because BaseModel.save() nulls created_by when there's no current request user under tests. Switched both to instantiate + save(created_by_id=create_user.id) — the same pattern already used for the draft — so the fixtures' ownership is real.

Note this didn't affect the test's correctness (the security assertions key off the draft's created_by, which was already set correctly, and the asset checks use draft_issue_id/issue_id), but the fixtures are now accurate. 2/2 tests still green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants