[INFRA-499] fix(security): require workspace membership to read a global view

[mguptahub]

Second layer of the stack, on top of #9652. Same bug family, one layer over: #9652 closed routed verbs that fall through to a DRF generic mixin. This closes an action that is defined on the viewset but carries no authorization check — which no fall-through scan can see, because a method we wrote with no check on it looks identical to a correctly guarded one.

The fix — WorkspaceViewViewSet.retrieve

It was the only action on the class with no check at all, and get_queryset() supplied none either:

.filter(workspace__slug=self.kwargs.get("slug"))
.filter(project__isnull=True)
.filter(Q(owned_by=self.request.user) | Q(access=1))

That last clause reads as a visibility predicate and is vacuous. access sits in IssueViewSerializer.read_only_fields, so the API never sets it, and the model defaults it to 1 (Public) — so every row matches. A filter that matches everything looks like scoping and provides none.

Result: any authenticated account holding a view id could read any global view in any workspace, including one it had no membership in. Now requires workspace membership, matching the role set on list().

Verified by reverting the fix: the test fails with 200 and the full view payload.

Also here — regression coverage for a guard that had none

IssueDetailIdentifierEndpoint was reported to me as missing the guest restriction. It is not. The membership check at the top of get() is followed, after the issue is fetched, by an explicit role-5 / guest_view_all_features / created_by check returning 403. I wrote a fix for it, discovered during fail-before verification that the endpoint was already guarded, and reverted the change — this PR contains no code change for it.

What it did have was no test. That endpoint resolves work items by human-readable sequence number, so it's an enumeration surface: losing the guard lets a guest walk PROJ-1..PROJ-N and read every work item's full detail including description_html, plus the project and issue UUIDs every other endpoint keys on. Those five tests pin it.

They are verified non-vacuous: neutering the existing check makes test_guest_cannot_read_foreign_work_item fail with 200 and the foreign work item's full payload. Worth stating explicitly, because my first attempt at that verification patched a different guard with the same shape earlier in the same file and the tests stayed green — a vacuous pass I'd otherwise have shipped.

Verification

• 8 contract tests, each property with a positive control (own item still readable, full member unaffected, guest_view_all_features=True still permissive, workspace member and workspace guest both still read the view).
• Fail-before verified per property, targeting each guard individually by line rather than by pattern.
• Full suite green against a real database: 527 passed, contract + unit.
• ruff check and ruff format clean on both changed files.

Note for reviewers

The role set on retrieve deliberately matches list() — admin, member, guest. The defect is that non-members could read at all, not which member roles may. Tightening beyond that is a product decision, not a security one, so it is out of scope here.

The broader audit this came from is still open: enumerating every action across app/, api/ and space/ that has neither a decorator nor an inline check. #9652's count of 27 is a floor, not a ceiling, precisely because of the class this PR is in.

Refs INFRA-499.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a833e26-06e0-441d-a3a2-763f82eea80b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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)

• (INFRA-499) fix(security): audit defined-but-undecorated viewset actions — the class no fall-through scan can see

This comment was auto-generated by Plane

[Copilot]

Pull request overview

This PR closes an authorization gap in the app API by ensuring that reading a global workspace view (WorkspaceViewViewSet.retrieve) requires workspace membership, aligning the access boundary with the existing list() behavior. It also adds contract/regression coverage for authorization logic that is implemented via inline checks rather than @allow_permission decorators.

Changes:

• Require workspace membership on WorkspaceViewViewSet.retrieve by adding an @allow_permission(..., level="WORKSPACE") guard.
• Add contract tests covering (1) the workspace-view retrieve membership requirement and (2) regression protection for IssueDetailIdentifierEndpoint guest scoping.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
apps/api/plane/app/views/view/base.py	Adds workspace-membership authorization to WorkspaceViewViewSet.retrieve to prevent cross-workspace reads of global views.
apps/api/plane/tests/contract/app/test_undecorated_action_authz_app.py	Introduces contract tests for the fixed endpoint and regression tests for the identifier-based issue detail endpoint.

Suppressed comments (1)

apps/api/plane/tests/contract/app/test_undecorated_action_authz_app.py:264

• The file ends mid-test: test_workspace_guest_can_read_a_global_view is missing its final assertions/closing lines, leaving a dangling assert block and causing a syntax error.

        assert response.status_code == status.HTTP_200_OK, (
            f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
        )

---

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

[mguptahub]

Addressed review feedback: `WorkspaceViewViewSet.retrieve()` required workspace membership but did not reapply the guest-ownership restriction that `list()` already enforces. Since `get_queryset()`'s `Q(owned_by=request.user) | Q(access=1)` clause is effectively always true (`access` is read-only on the serializer and defaults to Public on the model), a GUEST-role member who owned no views could still fetch any other member's global view directly by id.

Fix mirrors the existing pattern in the project-scoped `IssueViewViewSet.retrieve()` in the same file, which already reapplies this same guest-ownership check inline. After the workspace-membership check and the "view not found" check, `retrieve()` now also checks: if the caller is a workspace GUEST and does not own the view, return 404 — consistent with the "missing view" branch already on this endpoint, rather than introducing a new 403 shape.

Also flipped the PR's own contract test, `test_workspace_guest_can_read_a_global_view` (renamed to `test_workspace_guest_cannot_read_a_global_view_they_do_not_own`), which asserted 200 for a guest reading a view they don't own — that was the gap the test itself was demonstrating. It now asserts 404. Added a new positive control, `test_workspace_guest_can_read_a_global_view_they_own`, confirming a guest reading their own view still succeeds.

All 5 tests in `TestWorkspaceViewRetrieveRequiresMembership` pass, plus the full `-k view` suite (35 tests) with no regressions.

[github-actions]

React Doctor skipped this pull request — it changed no React files.

_{Reviewed by React Doctor for commit 7a5dd57.}