Skip to content

[SECUR-247] fix(security): scope ProjectMemberPermission POST to the URL project - #9596

Open
mguptahub wants to merge 2 commits into
previewfrom
secur-247/project-member-permission-post-scope
Open

[SECUR-247] fix(security): scope ProjectMemberPermission POST to the URL project#9596
mguptahub wants to merge 2 commits into
previewfrom
secur-247/project-member-permission-post-scope

Conversation

@mguptahub

@mguptahub mguptahub commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

ProjectMemberPermission.has_permission scopes its SAFE_METHODS branch and its trailing (write) branch to project_id=view.project_id. The POST branch checked workspace membership only. Any workspace MEMBER could therefore create sub-resources in a project they do not belong to.

Verified against preview @ 1c8a60f858. Closes 2 HIGH advisories (SECUR-247 carries the mapping).

Impact

The reported vector is deploy-board creation — the publish action:

  1. Attacker is a role-15 workspace MEMBER with no ProjectMember row for a network=0 (Secret) project
  2. POST .../project-deploy-boards/ returns 200 and the public anchor
  3. Space's AllowAny endpoints serve that anchor to unauthenticated callers — work-item list and detail, including description_html

A second report notes project_id from the URL was never checked against slug, so a user who owns any workspace on the instance could aim their own slug at another tenant's project id.

The change

# before — workspace only
if request.method == "POST":
    return WorkspaceMember.objects.filter(
        workspace__slug=view.workspace_slug, member=request.user,
        role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value], is_active=True).exists()

# after — matches the branches either side
    return ProjectMember.objects.filter(
        workspace__slug=view.workspace_slug, member=request.user,
        role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value],
        project_id=view.project_id, is_active=True).exists()

Plus DeployBoardViewSet.create now validates project_id against slug before get_or_create.

Blast radius — please read

ProjectMemberPermission is shared. I audited every consumer:

Consumer POST via this class? Effect
DeployBoardViewSet yes fixed — the reported vector
LabelListCreateAPIEndpoint.post yes also fixed — a workspace member could create labels in a project they weren't in
LabelDetailAPIEndpoint inherits same
ProjectMemberListCreateAPIEndpoint no — overrides get_permissions() to ProjectAdminPermission for non-GET unaffected
ProjectMemberLiteAPIEndpoint GET only unaffected

So the diff closes label creation too. That is the same root cause, not scope creep — but it is more than the two advisories name, and worth knowing when reviewing.

ProjectBasePermission is deliberately untouched. It has a near-identical POST branch at project.py:25, but there the workspace-only check is correct — it genuinely guards project creation. Its misuse by the archive endpoint is tracked separately (SECUR-249). ProjectMemberPermission is not used for project creation anywhere; the stale comment claiming otherwise is likely how this survived review, and is now replaced.

Update after review

Two Copilot findings, both valid and both fixed in f494bfd76d:

A duplicate permission class. plane/utils/permissions/project.py holds a second ProjectMemberPermission which — comments aside — was byte-identical, including the unscoped POST branch. It is imported (api/views/member.py:23), but its POST branch is currently unreachable: ProjectMemberListCreateAPIEndpoint.get_permissions() routes non-GET to ProjectAdminPermission, and the other consumer is GET-only. So it was a latent hazard rather than a second live vector. Scoped anyway — two same-named classes that have already drifted make reintroduction easy, and this repo has previously had to patch the same duplication in the page permission classes. Both copies now note they must not drift.

Worth flagging that my original blast-radius audit grepped for the class name and never checked import sources, which is how I missed it.

404 wording. The new guard said "Project not found"; the module already uses "Project does not exist" (base.py:230). Aligned.

Consolidating the two permission modules is the real fix but is wider than a security patch should carry — worth its own ticket.

I did not add workspace_id to get_or_create's lookup keys — that changes matching semantics and risks creating duplicate boards against existing rows where workspace is null. The validation is a separate guard.

Tests

Extended test_deploy_board_project_scope_app.py, which already covered the SAFE_METHODS sibling of this same class. Four new tests:

  • non-project-member publish → 403
  • denied publish leaks no anchor and creates no DeployBoard row (a 403 that still created the board would leave the project published)
  • positive control — project member can still publish
  • own slug + foreign project id → rejected, no board created

Fail-before verified: 3 failed / 3 passed unpatched → 6 passed patched. The positive control and both pre-existing tests pass in both runs, confirming they are independent of the change.

6 passed in 4.59s

Summary by CodeRabbit

  • Bug Fixes
    • Restricted publishing permissions to members of the selected project.
    • Prevented publishing content from projects in another workspace.
    • Invalid or unauthorized publishing requests now return an error without creating a deploy board.
    • Prevented unauthorized users from accessing project publishing data.

The SAFE_METHODS branch and the trailing branch both bind project_id; only
POST did not, checking workspace membership alone. Any workspace member could
therefore create sub-resources in a project they do not belong to.

The reported impact is deploy-board creation: publishing a Secret project
returns the public anchor, which Space then serves to anonymous callers —
work item list and detail including description_html. LabelListCreateAPIEndpoint
runs through the same branch and is closed by the same change.

ProjectMemberListCreateAPIEndpoint is unaffected: it overrides get_permissions()
to use ProjectAdminPermission for non-GET.

ProjectBasePermission has a similar-looking POST branch and is deliberately left
alone — there the workspace-only check is correct, since it guards project
creation itself.

Also validates project_id against the URL slug in DeployBoardViewSet.create;
get_or_create lookup keys are unchanged to avoid matching differently against
existing rows.

Contract tests cover the denied publish, that no anchor leaks and no board is
created on denial, and cross-workspace project ids — plus a positive control
that a project member can still publish. Fail-before verified: 3 failed /
3 passed unpatched, 6 passed patched.

Co-authored-by: Plane AI <noreply@plane.so>
Copilot AI lite review requested due to automatic review settings August 13, 2026 08:15
@coderabbitai

coderabbitai Bot commented Aug 13, 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 Plus

Run ID: f30d6c1e-109c-4f86-9646-23072b322b89

📥 Commits

Reviewing files that changed from the base of the PR and between 0f92491 and f494bfd.

📒 Files selected for processing (2)
  • apps/api/plane/app/views/project/base.py
  • apps/api/plane/utils/permissions/project.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/api/plane/app/views/project/base.py

📝 Walkthrough

Walkthrough

DeployBoard publishing now requires project membership and validates that the project belongs to the requested workspace. Contract tests cover denied access, side effects, successful publishing, and cross-workspace project IDs.

Changes

DeployBoard project scoping

Layer / File(s) Summary
Project membership authorization
apps/api/plane/app/permissions/project.py, apps/api/plane/utils/permissions/project.py
POST requests now require an eligible workspace member who also belongs to the requested project.
Workspace validation and contract coverage
apps/api/plane/app/views/project/base.py, apps/api/plane/tests/contract/app/test_deploy_board_project_scope_app.py
DeployBoard creation returns 404 Project does not exist for projects outside the requested workspace. Contract tests verify authorization, persistence side effects, successful publishing, and cross-workspace isolation.

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

Mergeability Score: ⚪ Minimal · up to f494b

The change restricts project-member POST actions to the project in the URL and validates the project identifier, preventing unauthorized sub-resource creation. No actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: dheeru0198

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly identifies the security fix and the specific project-scoping change to ProjectMemberPermission POST authorization.
Description check ✅ Passed The description clearly explains the issue, impact, implementation, affected consumers, references, and test results, but it omits the template headings and change-type checklist.
✨ 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 secur-247/project-member-permission-post-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.

@makeplane

makeplane Bot commented Aug 13, 2026

Copy link
Copy Markdown

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 privilege-escalation in the API where ProjectMemberPermission.has_permission previously allowed POST based on workspace membership only, enabling workspace members to create project-scoped sub-resources (e.g., deploy boards/labels) in projects they do not belong to. It also adds a defensive validation in deploy-board creation and extends contract tests to cover the reported vectors.

Changes:

  • Scope ProjectMemberPermission’s POST authorization to the URL project_id by checking ProjectMember membership (not just WorkspaceMember).
  • Add a project_idslug validation guard in DeployBoardViewSet.create before get_or_create.
  • Extend deploy-board contract tests to cover negative/positive publish behavior and cross-workspace slug/project_id mismatch.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
apps/api/plane/app/permissions/project.py Updates ProjectMemberPermission POST checks to require project membership for the URL project_id.
apps/api/plane/app/views/project/base.py Adds an explicit Project existence check to prevent cross-workspace slug + foreign project_id publishes.
apps/api/plane/tests/contract/app/test_deploy_board_project_scope_app.py Adds regression tests for deploy-board publish scoping and cross-tenant slug/project_id mismatch behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/api/plane/app/views/project/base.py Outdated
Comment thread apps/api/plane/app/permissions/project.py
…h too

Addresses review on #9596.

plane/utils/permissions/project.py holds a second ProjectMemberPermission that,
comments aside, was byte-identical to the one in plane/app/permissions. Its POST
branch still checked workspace membership alone.

It is imported (api/views/member.py) but its POST branch is currently
unreachable: ProjectMemberListCreateAPIEndpoint.get_permissions() routes non-GET
to ProjectAdminPermission, and the other consumer is GET-only. So this is a
latent hazard rather than a second live vector — but two same-named classes that
have already drifted make reintroduction easy, and this repo has previously had
to patch the same duplication in the page permission classes. Both copies now
carry a comment saying they must not drift.

Also aligns the deploy-board 404 string with the module's existing wording
("Project does not exist", cf. base.py:230) rather than introducing a second
phrasing for clients to handle.

Co-authored-by: Plane AI <noreply@plane.so>

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

apps/api/plane/app/permissions/project.py:74

  • The POST query is now identical to the generic write query immediately below, so keeping this special case leaves two authorization paths that can drift again—the failure mode this patch addresses. Remove the POST branch and let the shared non-safe-method check handle POST.
        if request.method == "POST":
            return ProjectMember.objects.filter(

apps/api/plane/utils/permissions/project.py:74

  • The POST query is now identical to the generic write query immediately below, so keeping this special case leaves two authorization paths that can drift again—the failure mode this patch addresses. Remove the POST branch and let the shared non-safe-method check handle POST.
        if request.method == "POST":
            return ProjectMember.objects.filter(

apps/api/plane/tests/contract/app/test_deploy_board_project_scope_app.py:190

  • This permits a 403, which ProjectMemberPermission returns before DeployBoardViewSet.create runs for this fixture. Therefore the new slug/project guard can be removed entirely while this test still passes. Add focused coverage that bypasses or overrides the permission layer and expects the view's 404, while retaining the no-board assertion.
        assert response.status_code in (
            status.HTTP_403_FORBIDDEN,
            status.HTTP_404_NOT_FOUND,
        ), f"Got {response.status_code}: {getattr(response, 'data', None)!r}"

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