[WEB-7892] fix(security): scope attachment PATCH/DELETE/GET by issue_id, drop created_by overwrite (GHSA-5mxw-g5mw-3v3w)

[mguptahub]

Summary

• Adds issue_id=issue_id to FileAsset.objects.get() in all three V2 issue attachment handlers that were missing it: patch, delete, and get (single asset path)
• Removes issue_attachment.created_by = request.user from patch — created_by is set at creation time and must not be overwritten by the upload-confirm call

Security context

Advisory: GHSA-5mxw-g5mw-3v3w | Severity: High | CWE-639 (Authorization Bypass Through User-Controlled Key)

The PATCH, DELETE, and GET (single) handlers looked up FileAsset by (pk, workspace, project_id) only. The issue_id URL parameter was silently ignored. Any project member could supply their own issue_id in the URL while using a victim's attachment UUID as pk — the server found and operated on the attachment regardless. In PATCH, created_by = request.user transferred ownership to the attacker, locking the original uploader out of their own file's delete right.

The V1 delete handler already scoped by issue_id correctly — this PR brings the V2 handlers in line with that pattern.

Files changed

• apps/api/plane/app/views/issue/attachment.py

Test plan

• PATCH with mismatched issue_id (attacker's issue, victim's asset UUID) — verify 404
• DELETE with mismatched issue_id — verify 404
• GET single with mismatched issue_id — verify 404
• Normal upload flow: POST → PATCH with correct issue_id — verify 204, created_by unchanged
• Original uploader can still DELETE after PATCH confirm

Summary by CodeRabbit

• Bug Fixes
  • Attachment actions now apply only to files belonging to the selected issue, reducing the risk of viewing, updating, or deleting the wrong attachment.
  • Upload completion updates no longer change who originally created an attachment, preserving the correct owner information.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d54dc26-b217-4261-ac2d-08cc03c807b5

📥 Commits

Reviewing files that changed from the base of the PR and between 1e8f363 and 7f38f37.

📒 Files selected for processing (1)

• apps/api/plane/app/views/issue/attachment.py

---

📝 Walkthrough

Walkthrough

Issue attachment delete, redirect, and upload-state update paths now scope FileAsset lookups by issue_id. The patch path also keeps the original created_by value when marking an attachment uploaded.

Changes

Issue attachment asset scoping

Layer / File(s)	Summary
Scoped attachment lookup and upload update apps/api/plane/app/views/issue/attachment.py	delete, get, and patch now fetch FileAsset records with issue_id included, and patch preserves created_by while setting is_uploaded.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

Hop hop, the bunny did see,
One issue, one asset, all tidy as can be.
No creator hops away in the night,
Uploaded and scoped, snug and right.
Thump! The burrow hums with glee.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the PR’s main security fix and the affected attachment handlers.
Description check	✅ Passed	The description covers summary, security context, files changed, and test plan, but omits the type-of-change checkboxes, screenshots, and references.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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-7892/attachment-issue-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

• X
• Mastodon
• Reddit
• LinkedIn

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

[makeplane]

Linked to Plane Work Item(s)

• (WEB-7892) fix(security): scope issue attachment PATCH/DELETE/GET by issue_id to prevent ownership hijacking (GHSA-5mxw-g5mw-3v3w)

This comment was auto-generated by Plane