[WEB-8382][WEB-8383] fix(security): external-API issue comment + attachment authz (GHSA-h4p4, GHSA-xvc5)

[mguptahub]

Summary

Two authorization gaps in the external API (apps/api/plane/api/views/issue.py), both confirmed vulnerable against origin/preview @ a8e53b6ac7:

GHSA-h4p4-mwfg-qh82 — comment tampering (WEB-8382)

IssueCommentDetailAPIEndpoint is guarded by ProjectLitePermission (any active project member, incl. Guest) and edited/deleted comments by id with no author/admin check. A Guest could modify or delete any other user's comments — including an admin's. The web app restricts this to the comment's creator or a project admin; the external API dropped both checks.

GHSA-xvc5-m5jf-gvpj — cross-tenant attachment metadata disclosure (WEB-8383)

IssueAttachmentListCreateAPIEndpoint.get sets no permission_classes (inherits bare IsAuthenticated) and does no membership check, while its post calls user_has_issue_permission. Because API tokens authenticate globally (not workspace-scoped), any token holder could list any issue's attachment metadata (filenames, sizes, S3 keys, uploader) across tenants.

Fix

• Comment patch/delete: require created_by == request.user or project admin (ProjectMember role=ADMIN), else 403.
• Attachment get: load the issue scoped to the URL workspace/project and enforce user_has_issue_permission([ADMIN, MEMBER, GUEST], allow_creator=True), mirroring post.

Tests

tests/contract/api/test_external_api_issue_authz.py — 6 tests: guest cannot patch/delete a comment (403, unchanged), author can patch own + admin can delete (positive); outsider (token, no membership) cannot list attachments (403), project member can (positive). Fail-before verified; ruff clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Restricted issue comment edits and deletions to the comment author or an active project administrator.
  • Prevented unauthorized users from viewing issue attachment metadata across projects.
  • Added clear permission-denied responses when access is not allowed.
  • Updated API documentation to describe authorization errors for these actions.

• Tests

  • Added coverage for comment and attachment authorization scenarios, including cross-project access prevention and valid member access.

[Copilot]

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

[makeplane]

Linked to Plane Work Item(s)

• (WEB-8382) (security) CE — External API: any member can edit/delete others' comments (GHSA-h4p4-mwfg-qh82)
• (WEB-8383) (security) CE — External API: cross-tenant attachment metadata disclosure (GHSA-xvc5-m5jf-gvpj)

This comment was auto-generated by Plane

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cd370460-acc2-4632-b556-3fed311c9276

📥 Commits

Reviewing files that changed from the base of the PR and between 02f2d44 and b9a01fd.

📒 Files selected for processing (1)

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

🚧 Files skipped from review as they are similar to previous changes (1)

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

---

📝 Walkthrough

Walkthrough

The issue API now restricts comment updates and deletions to authors or active project admins. It also checks project roles or issue ownership before listing attachment metadata. Contract tests cover allowed and denied API-token requests.

Changes

Issue API authorization

Layer / File(s)	Summary
Comment authorization enforcement and coverage apps/api/plane/api/views/issue.py, apps/api/plane/tests/contract/api/test_external_api_issue_authz.py	Comment patch and delete handlers reject unauthorized users with 403. Contract tests cover guest, author, and project-admin behavior.
Attachment listing authorization and coverage apps/api/plane/api/views/issue.py, apps/api/plane/tests/contract/api/test_external_api_issue_authz.py	Attachment listing checks project roles or issue ownership and returns 403 to unauthorized users. Contract tests verify member access and attachment metadata.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• makeplane/plane#9367: Strengthens authorization checks and adds forbidden-access regression tests.
• makeplane/plane#9372: Adds project-membership authorization for issue-related assets and contract tests.
• makeplane/plane#9403: Modifies the issue comment patch and delete endpoints.

Suggested reviewers: dheeru0198

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the security fixes for external API issue comment and attachment authorization.
Description check	✅ Passed	The description explains both vulnerabilities, fixes, affected endpoints, tests, and references, but it does not use all template headings or select a change type.
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-8375/external-api-issue-authz

---

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.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

apps/api/plane/tests/contract/api/test_external_api_issue_authz.py (1)

125-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Complete the allowed comment-operation matrix.

These tests cover author PATCH and admin DELETE, but not author DELETE or admin PATCH. Add both cases so either endpoint cannot regress for one permitted actor unnoticed.

🤖 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/tests/contract/api/test_external_api_issue_authz.py` around
lines 125 - 142, Add tests alongside test_author_can_patch_own_comment and
test_admin_can_delete_comment covering author DELETE and admin PATCH. Use
_token_client(author) and _token_client(create_user) respectively, assert HTTP
204 for author deletion and HTTP 200 for admin editing, and follow the existing
COMMENT_URL construction and response-detail assertions.

🤖 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.

Inline comments:
In `@apps/api/plane/api/views/issue.py`:
- Around line 1612-1615: Add the missing HTTP 403 response documentation to the
explicit OpenAPI response maps for update_work_item_comment,
delete_work_item_comment, and list_work_item_attachments in
apps/api/plane/api/views/issue.py at lines 1612-1615, 1690-1693, and 2038-2041,
respectively, matching the existing response schema and style.

---

Nitpick comments:
In `@apps/api/plane/tests/contract/api/test_external_api_issue_authz.py`:
- Around line 125-142: Add tests alongside test_author_can_patch_own_comment and
test_admin_can_delete_comment covering author DELETE and admin PATCH. Use
_token_client(author) and _token_client(create_user) respectively, assert HTTP
204 for author deletion and HTTP 200 for admin editing, and follow the existing
COMMENT_URL construction and response-detail assertions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5618f88c-6bde-4cca-8c52-5cf195a86b42

📥 Commits

Reviewing files that changed from the base of the PR and between a8e53b6 and 02f2d44.

📒 Files selected for processing (2)

• apps/api/plane/api/views/issue.py
• apps/api/plane/tests/contract/api/test_external_api_issue_authz.py

[Copilot]

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

[github-actions]

React Doctor found no issues. 🎉

⚠️ Warning: .github/workflows/react-doctor.yml is configured incorrectly. See below to fix.

React Doctor compares against preview to report only the issues this pull request introduces. This run couldn't complete that comparison (usually a shallow CI checkout with no merge base), so it listed every issue in the changed files, including ones that already existed on preview.

Add fetch-depth: 0 to the actions/checkout step in .github/workflows/react-doctor.yml so the checkout includes the history React Doctor needs:

 jobs:
   react-doctor:
     steps:
       - uses: actions/checkout@v5
+        with:
+          fetch-depth: 0

       - uses: millionco/react-doctor@v2

To silence this warning, set silence-missing-baseline-warning: true on the React Doctor action.

_{Reviewed by React Doctor for commit b9a01fd.}