From 79c985ba73181968a7ded0ad5b438843a42cd0f5 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 28 Aug 2026 16:11:08 +0530 Subject: [PATCH 01/18] UN-3315 [FIX] Honour shared_to_org for Prompt Studio prompt edits Projects shared via "Share with everyone" set shared_to_org on the parent CustomTool. IsOwnerOrSharedUserOrSharedToOrg already honours that flag, so such a project was visible to the whole org -- but PromptAcesssToUser, which guards prompt/note CRUD, never checked it. The result was that only the owner could edit prompts in a project shared with everyone. Adds the shared_to_org check to PromptAcesssToUser so prompt access matches the project access the share already granted. UN-3542 (last org admin demotion) needs no change: the _ensure_not_last_admin_demotion guard already landed on main in 2f996d384 (#2048) and is wired into both add_user_role and remove_user_role. The ticket is stale and should be closed rather than reimplemented. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF --- backend/prompt_studio/permission.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index 43eb9c75da..e09a47b361 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -16,8 +16,9 @@ class PromptAcesssToUser(permissions.BasePermission): A user qualifies when they own the parent ``CustomTool``, are a direct viewer (VIEWER membership, UN-2202), reach the project via group sharing - (``ResourceGroupShare`` on the parent tool), or are an org admin - (org-wide admin override, UN-3479). + (``ResourceGroupShare`` on the parent tool), reach it because the parent + tool is shared with the whole org (``shared_to_org``, UN-3315), or are an + org admin (org-wide admin override, UN-3479). """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: @@ -28,6 +29,12 @@ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bo return True if _is_resource_viewer(request.user, tool): return True + # UN-3315: "Share with everyone" sets shared_to_org on the parent tool. + # IsOwnerOrSharedUserOrSharedToOrg already honours it, so a project + # shared this way was visible but its prompts stayed read-only for + # everyone except the owner. + if getattr(tool, "shared_to_org", False): + return True if has_group_access(request.user, tool): return True return OrganizationMemberService.is_user_organization_admin(request.user) From 943b3a0da17b374a1d693fa19b6f7c578f82f9f4 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 01:30:15 +0530 Subject: [PATCH 02/18] UN-3315 [FIX] Restrict prompt deletion to the parent tool's owner Follow-up to 79c985ba7, which made shared_to_org grant access to a project's prompts. PromptAcesssToUser is the sole permission class on ToolStudioPromptView, which routes delete->destroy (prompt_studio_v2/urls.py:13), so that grant covered deletion too. The parent CustomTool's own destroy is owner-only (IsOwner in CustomToolViewSet.get_permissions), which left any org member able to delete every prompt inside a project they could not themselves delete. Product decision: "share with everyone" means view + edit, not delete. Adds IsPromptParentToolOwner and splits get_permissions so only destroy uses it. Reads and edits keep the widened class, matching CustomToolViewSet, which already routes update/partial_update on the tool itself through IsOwnerOrSharedUserOrSharedToOrg. Kept as a separate class rather than teaching IsParentToolOwner to read both prompt_studio_tool and tool_id: a shared authorization class that accumulates per-caller special cases is how these gates drift apart. Also addresses two review findings on 79c985ba7: - getattr(tool, "shared_to_org", False) -> tool.shared_to_org. tool is always a CustomTool, where the field is a non-nullable BooleanField, so the default was unreachable and would only mask a renamed field by silently denying. Matches IsOwnerOrSharedUserOrSharedToOrg, which reads it directly. - The inline comment claimed prompts "stayed read-only for everyone except the owner". That was imprecise: VIEWER and group-share already granted write. Narrowed to the shared_to_org-only user, who genuinely had no access. Known gap, unchanged by this commit: reorder_prompts is a collection-level POST, so get_object() never runs and neither permission class gates it (see prompt_studio_v2/helper.py:28). tool_instance_v2/views.py:196-205 has the pattern that closes it. Needs its own change. Untested: the backend suite does not run in this checkout -- settings import fails at backend/settings/base.py:63 on CELERY_BROKER_BASE_URL=None, and conftest.py notes backend tests do not run under tox in CI. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/permission.py | 54 +++++++++++++++++-- .../prompt_studio/prompt_studio_v2/views.py | 13 ++++- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index e09a47b361..26e09844bb 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -12,13 +12,23 @@ class PromptAcesssToUser(permissions.BasePermission): - """Is the crud to Prompt/Notes allowed to user. + """Read and edit access to a Prompt/Note, inherited from the parent tool. A user qualifies when they own the parent ``CustomTool``, are a direct viewer (VIEWER membership, UN-2202), reach the project via group sharing (``ResourceGroupShare`` on the parent tool), reach it because the parent tool is shared with the whole org (``shared_to_org``, UN-3315), or are an org admin (org-wide admin override, UN-3479). + + Deliberately broader than the workflow rule stated in + ``permissions.permission.is_workflow_mutator`` ("shared access grants read + only, never mutate"): a Prompt Studio share confers *edit* rights on the + project's prompts, matching ``CustomToolViewSet``, which already routes + ``update``/``partial_update`` on the tool itself through + ``IsOwnerOrSharedUserOrSharedToOrg``. + + **Deletion is not included.** ``destroy`` is gated by + :class:`IsPromptParentToolOwner` instead -- see ``ToolStudioPromptView``. """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: @@ -30,16 +40,50 @@ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bo if _is_resource_viewer(request.user, tool): return True # UN-3315: "Share with everyone" sets shared_to_org on the parent tool. - # IsOwnerOrSharedUserOrSharedToOrg already honours it, so a project - # shared this way was visible but its prompts stayed read-only for - # everyone except the owner. - if getattr(tool, "shared_to_org", False): + # IsOwnerOrSharedUserOrSharedToOrg already honours it, so a user whose + # only access came from that flag (no VIEWER row, no group share, not an + # admin) could not reach the project's prompts at all. + # + # Read directly rather than via getattr: ``tool`` is always a + # ``CustomTool``, where ``shared_to_org`` is a non-nullable + # BooleanField, so a default would only mask a renamed field by + # silently denying access. Matches IsOwnerOrSharedUserOrSharedToOrg. + if tool.shared_to_org: return True if has_group_access(request.user, tool): return True return OrganizationMemberService.is_user_organization_admin(request.user) +class IsPromptParentToolOwner(permissions.BasePermission): + """Deletion gate for Prompt Studio prompts/notes. + + Mirrors ``permissions.permission.IsParentToolOwner``, which does the same + for ``ProfileManager``, but reads the parent through ``ToolStudioPrompt``'s + own FK name (``tool_id``) rather than ``prompt_studio_tool``. Kept as a + separate class rather than teaching the shared one to juggle both attribute + names: a shared authorization class that accumulates per-caller special + cases is how these gates drift apart. + + Exists because the parent ``CustomTool``'s own ``destroy`` is owner-only + (``IsOwner`` in ``CustomToolViewSet.get_permissions``). Without this, + UN-3315's org-wide share would let any org member delete every prompt + inside a project they cannot themselves delete. + + ``tool_id`` is nullable (``SET_NULL``), so an orphaned prompt whose parent + tool was deleted falls back to the org-admin check -- it has no owner to + inherit from. + """ + + def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: + if getattr(request.user, "is_service_account", False): + return True + tool = obj.tool_id + if tool is not None and _is_resource_owner(request.user, tool): + return True + return OrganizationMemberService.is_user_organization_admin(request.user) + + class IsRegistryToolOwner(permissions.BasePermission): """Is unpublishing an exported tool allowed to user. diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index a540480274..3be5e4c6fe 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -1,3 +1,5 @@ +from typing import Any + from django.db.models import QuerySet from rest_framework import viewsets from rest_framework.decorators import action @@ -6,7 +8,7 @@ from rest_framework.versioning import URLPathVersioning from utils.filtering import FilterHelper -from prompt_studio.permission import PromptAcesssToUser +from prompt_studio.permission import IsPromptParentToolOwner, PromptAcesssToUser from prompt_studio.prompt_studio_v2.constants import ToolStudioPromptKeys from prompt_studio.prompt_studio_v2.controller import PromptStudioController from prompt_studio.prompt_studio_v2.models import ToolStudioPrompt @@ -32,7 +34,14 @@ class ToolStudioPromptView(viewsets.ModelViewSet): versioning_class = URLPathVersioning serializer_class = ToolStudioPromptSerializer - permission_classes: list[type[PromptAcesssToUser]] = [PromptAcesssToUser] + def get_permissions(self) -> list[Any]: + # Reads and edits honour project sharing (UN-3315); deletion requires + # ownership of the parent tool, matching CustomToolViewSet, whose own + # `destroy` is IsOwner-gated. A shared project's prompts are editable + # by the org, not deletable by it. + if self.action == "destroy": + return [IsPromptParentToolOwner()] + return [PromptAcesssToUser()] def get_serializer_class(self): if self.action == "list": From 4d8a381b01fb9cbdf6d094aac086b7e449d923cf Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 01:37:58 +0530 Subject: [PATCH 03/18] UN-3315 [DOC] Correct three claims in the prompt permission gate Follow-up to 943b3a0da, whose comments overstated what the destroy split achieves and asserted an invariant the models contradict. 1. "Deletion is not included" / "not deletable by it" was false. The bulk sync_prompts route on PromptStudioCoreView rip-and-replaces every prompt in a project and admits org-shared users -- only destroy and the co-owner actions are IsOwner-gated there, and CustomTool.objects.for_user admits shared_to_org=True so no 404 shields it. The split closes per-prompt DELETE and nothing else. Both comments now say so and point at the surviving route rather than implying it does not exist. 2. "tool is always a CustomTool" contradicted the model (tool_id is a nullable SET_NULL FK) and the IsPromptParentToolOwner docstring 25 lines below, which correctly documents the orphan case. A maintainer trusting it would drop the `tool is not None` guard and turn a clean 403 into an AttributeError 500. Dropped the sentence; the "a default masks a renamed field" rationale is true on its own and is what the direct read actually rests on. 3. The divergence docstring cited only the precedent that supports the widening (CustomToolViewSet) and omitted the nearer sibling that chose the other way -- ProfileManagerView gates every mutation behind IsParentToolOwner. Two sub-resources of one parent answer "does a share grant edit?" differently; the docstring now names that rather than reading as though it were settled. No behaviour change: comments and docstrings only. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/permission.py | 24 ++++++++++++------- .../prompt_studio/prompt_studio_v2/views.py | 9 +++---- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index 26e09844bb..558cf36069 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -25,10 +25,18 @@ class PromptAcesssToUser(permissions.BasePermission): only, never mutate"): a Prompt Studio share confers *edit* rights on the project's prompts, matching ``CustomToolViewSet``, which already routes ``update``/``partial_update`` on the tool itself through - ``IsOwnerOrSharedUserOrSharedToOrg``. - - **Deletion is not included.** ``destroy`` is gated by - :class:`IsPromptParentToolOwner` instead -- see ``ToolStudioPromptView``. + ``IsOwnerOrSharedUserOrSharedToOrg``. Note the nearer sibling chose the + other way: ``ProfileManagerView`` gates every mutation behind + ``IsParentToolOwner``, so a shared user can edit a project's prompts but + not its profiles. + + ``destroy`` on ``ToolStudioPromptView`` is gated by + :class:`IsPromptParentToolOwner` instead, so this class does not confer + per-prompt deletion. It is **not** the only way to delete a project's + prompts: the bulk ``sync_prompts`` route on ``PromptStudioCoreView`` still + admits org-shared users and rip-and-replaces every prompt in the project + (``prompt_studio_core_v2/views.py`` -- only ``destroy`` and the co-owner + actions are ``IsOwner``-gated there). """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: @@ -44,10 +52,10 @@ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bo # only access came from that flag (no VIEWER row, no group share, not an # admin) could not reach the project's prompts at all. # - # Read directly rather than via getattr: ``tool`` is always a - # ``CustomTool``, where ``shared_to_org`` is a non-nullable - # BooleanField, so a default would only mask a renamed field by - # silently denying access. Matches IsOwnerOrSharedUserOrSharedToOrg. + # Read directly rather than via getattr: on a CustomTool the field is + # a non-nullable BooleanField, so a default would only mask a renamed + # field by silently denying access. Matches + # IsOwnerOrSharedUserOrSharedToOrg, which also reads it directly. if tool.shared_to_org: return True if has_group_access(request.user, tool): diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index 3be5e4c6fe..5e16af48de 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -35,10 +35,11 @@ class ToolStudioPromptView(viewsets.ModelViewSet): serializer_class = ToolStudioPromptSerializer def get_permissions(self) -> list[Any]: - # Reads and edits honour project sharing (UN-3315); deletion requires - # ownership of the parent tool, matching CustomToolViewSet, whose own - # `destroy` is IsOwner-gated. A shared project's prompts are editable - # by the org, not deletable by it. + # Reads and edits honour project sharing (UN-3315); deleting a prompt + # requires ownership of the parent tool, matching CustomToolViewSet, + # whose own `destroy` is IsOwner-gated. This closes per-prompt DELETE + # only -- the bulk `sync_prompts` route on PromptStudioCoreView still + # lets an org-shared user wipe every prompt in the project. if self.action == "destroy": return [IsPromptParentToolOwner()] return [PromptAcesssToUser()] From 0a96979c70e983c877acd037480551c7e00a7c5b Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 01:42:11 +0530 Subject: [PATCH 04/18] UN-3315 [DOC] Correct the ProfileManagerView counterexample 4d8a381b0 fixed one over-broad closing claim and introduced another. It said ProfileManagerView "gates every mutation behind IsParentToolOwner, so a shared user can edit a project's prompts but not its profiles". Both halves are false in the direction that stops someone hardening those routes: - IsParentToolOwner implements only has_object_permission. DRF never calls it for create (there is no object yet), and ProfileManagerView.create never calls check_object_permissions or get_object, so that route is ungated. The sibling IsParentDeploymentOwner docstring documents this exact DRF gap and notes its view compensates by handing the parent to check_object_permissions -- ProfileManagerView does not. - create_profile_manager and make_profile_default on PromptStudioCoreView both fall through to IsOwnerOrSharedUserOrSharedToOrg, which admits shared_to_org. Narrowed to the three routes IsParentToolOwner actually gates, and named the gap rather than implying profiles are locked down. No behaviour change: docstring only. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/permission.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index 558cf36069..1d65534d6b 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -25,10 +25,14 @@ class PromptAcesssToUser(permissions.BasePermission): only, never mutate"): a Prompt Studio share confers *edit* rights on the project's prompts, matching ``CustomToolViewSet``, which already routes ``update``/``partial_update`` on the tool itself through - ``IsOwnerOrSharedUserOrSharedToOrg``. Note the nearer sibling chose the - other way: ``ProfileManagerView`` gates every mutation behind - ``IsParentToolOwner``, so a shared user can edit a project's prompts but - not its profiles. + ``IsOwnerOrSharedUserOrSharedToOrg``. Note the nearer sibling leans the + other way: ``ProfileManagerView`` routes ``update``/``partial_update``/ + ``destroy`` through ``IsParentToolOwner``. That is narrower than it looks, + though -- ``IsParentToolOwner`` implements only ``has_object_permission``, + which DRF never calls for the object-less ``create``, and the + ``create_profile_manager`` / ``make_profile_default`` routes on + ``PromptStudioCoreView`` admit org-shared users too. So profiles are not + the owner-only counterexample they first appear to be. ``destroy`` on ``ToolStudioPromptView`` is gated by :class:`IsPromptParentToolOwner` instead, so this class does not confer From 75fd2acd1835d21cd09f2ad06980ffcf1004b6fc Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 02:10:03 +0530 Subject: [PATCH 05/18] UN-3315 [FIX] Close the sync_prompts bulk-delete path sync_prompts is a rip-and-replace that deletes every prompt in a project before importing, and PromptStudioCoreView.get_permissions sent only destroy/add_co_owner/remove_co_owner to IsOwner(). Everything else, this route included, fell through to IsOwnerOrSharedUserOrSharedToOrg, which admits shared_to_org -- and CustomTool.objects.for_user admits those tools too, so no 404 shielded it. Any org member of a shared project could wipe every prompt in it, outputs riding along via CASCADE. Two changes, closing two different things: 1. sync_prompts joins the IsOwner() list. Deleting every prompt is owner-level destruction; UN-3315 settled that a share grants view + edit, not delete. 2. An empty-payload guard in PromptStudioHelper.sync_prompts, ahead of the transaction. This one is a correctness fix as much as a security one: the docstring says "deletes all existing prompts and creates new ones", but the create half is a loop over prompts_data that never runs on an empty list. So {"prompts": []} deleted everything, imported nothing, and returned success -- the code did not do what it documented. Raises ValueError to match the default_profile guard immediately below it. The guard sits before `with transaction.atomic()` deliberately: a rollback is not a refusal, and the point is that the delete never executes. Rejects empty only -- a non-empty list is a legitimate replace and still works. Honest scope: this closes the session-user path via IsOwner(), and the empty-payload wipe on both paths via the guard. A read_write platform API key can still call sync_prompts and replace prompts wholesale with a non-empty list -- service accounts short-circuit ahead of every check, and this route declares no required_method tier the way mcp_server does. Known and accepted; deliberately not addressed here. Also updates the two comments 0a96979c7 left behind, which said the sync_prompts hole was open. It is not, for a session user, as of this commit. Untested: the backend suite does not run in this checkout (settings import fails on CELERY_BROKER_BASE_URL=None). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/permission.py | 13 ++++++++----- .../prompt_studio_core_v2/prompt_studio_helper.py | 12 ++++++++++++ .../prompt_studio/prompt_studio_core_v2/views.py | 11 ++++++++++- backend/prompt_studio/prompt_studio_v2/views.py | 7 ++++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index 1d65534d6b..208b2703c5 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -36,11 +36,14 @@ class PromptAcesssToUser(permissions.BasePermission): ``destroy`` on ``ToolStudioPromptView`` is gated by :class:`IsPromptParentToolOwner` instead, so this class does not confer - per-prompt deletion. It is **not** the only way to delete a project's - prompts: the bulk ``sync_prompts`` route on ``PromptStudioCoreView`` still - admits org-shared users and rip-and-replaces every prompt in the project - (``prompt_studio_core_v2/views.py`` -- only ``destroy`` and the co-owner - actions are ``IsOwner``-gated there). + per-prompt deletion. The bulk ``sync_prompts`` route on + ``PromptStudioCoreView`` is likewise ``IsOwner``-gated, so a shared user + cannot reach either deletion path with their session. + + A ``read_write`` platform API key can still call ``sync_prompts`` and + replace a project's prompts wholesale: service accounts short-circuit + ahead of every check here, and that route declares no DELETE-tier + requirement. Known and accepted (UN-3315). """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: diff --git a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py index ba157801ce..ec0a6ab8ef 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py +++ b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py @@ -3171,6 +3171,18 @@ def sync_prompts(tool: CustomTool, import_data: dict, user) -> dict: prompts_data = import_data.get("prompts", []) tool_settings = import_data.get("tool_settings", {}) + # Refuse an empty payload before anything is deleted. This is a + # rip-and-replace, but the replace half is a loop over prompts_data + # that simply does not run when the list is empty -- so an empty or + # missing "prompts" wiped every prompt (and its outputs, via CASCADE) + # and reported success. Must stay ahead of the transaction: rolling + # back is not the same as never executing the delete. + if not prompts_data: + raise ValueError( + "No prompts found in the sync payload. Syncing an empty " + "prompt list would delete every prompt in the project." + ) + # Get the target tool's default profile default_profile = ProfileManager.objects.filter( prompt_studio_tool=tool, is_default=True diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index b3d42cf5f0..f23a20000e 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -153,7 +153,16 @@ def get_serializer_class(self): return CustomToolSerializer def get_permissions(self) -> list[Any]: - if self.action in ["destroy", "add_co_owner", "remove_co_owner"]: + # sync_prompts is a rip-and-replace: it deletes every prompt in the + # project before importing. That is owner-level destruction, so it + # belongs here rather than falling through to the share-aware class + # (UN-3315: a share grants view + edit, not delete). + if self.action in [ + "destroy", + "add_co_owner", + "remove_co_owner", + "sync_prompts", + ]: return [IsOwner()] return [IsOwnerOrSharedUserOrSharedToOrg()] diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index 5e16af48de..26bf030ea1 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -37,9 +37,10 @@ class ToolStudioPromptView(viewsets.ModelViewSet): def get_permissions(self) -> list[Any]: # Reads and edits honour project sharing (UN-3315); deleting a prompt # requires ownership of the parent tool, matching CustomToolViewSet, - # whose own `destroy` is IsOwner-gated. This closes per-prompt DELETE - # only -- the bulk `sync_prompts` route on PromptStudioCoreView still - # lets an org-shared user wipe every prompt in the project. + # whose own `destroy` is IsOwner-gated. The bulk `sync_prompts` route + # there is IsOwner-gated too, so a shared user reaches neither + # deletion path (a read_write API key still can -- see + # PromptAcesssToUser's docstring). if self.action == "destroy": return [IsPromptParentToolOwner()] return [PromptAcesssToUser()] From 5bc4ee96434be7a6dad47e6e5aa46a1440ac7e59 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 02:20:24 +0530 Subject: [PATCH 06/18] UN-3315 [REVERT] Drop the sync_prompts empty-payload guard 75fd2acd1 added a guard refusing {"prompts": []} on the premise that an empty rip-and-replace deleting everything and importing nothing was the code failing its own contract. That premise was wrong. test_prompt_studio_author.py:216 -- test_sync_prompts_clear_bumps_tool_ modified_at -- calls sync_prompts(tool, {"prompts": []}, user) and asserts prompts_deleted == 1, with a docstring describing a prompts-clearing sync as behaviour that must bump modified_at. Clearing every prompt by syncing an empty list is existing, intentional, test-asserted behaviour, not an accident. The guard broke a published capability and would have failed that test the moment anyone could run the suite. Reverts the guard only. The IsOwner() gating from 75fd2acd1 stands: the exposure was always a permissions question, and clearing a project's prompts is now owner-only like every other deletion path. Docstring updated to record the empty-list clear as supported, so the next reader does not re-derive it as a defect. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/permission.py | 30 +++++++++---------- .../prompt_studio_helper.py | 12 -------- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index 208b2703c5..d0ae1b866e 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -25,25 +25,25 @@ class PromptAcesssToUser(permissions.BasePermission): only, never mutate"): a Prompt Studio share confers *edit* rights on the project's prompts, matching ``CustomToolViewSet``, which already routes ``update``/``partial_update`` on the tool itself through - ``IsOwnerOrSharedUserOrSharedToOrg``. Note the nearer sibling leans the - other way: ``ProfileManagerView`` routes ``update``/``partial_update``/ - ``destroy`` through ``IsParentToolOwner``. That is narrower than it looks, - though -- ``IsParentToolOwner`` implements only ``has_object_permission``, - which DRF never calls for the object-less ``create``, and the - ``create_profile_manager`` / ``make_profile_default`` routes on - ``PromptStudioCoreView`` admit org-shared users too. So profiles are not - the owner-only counterexample they first appear to be. + ``IsOwnerOrSharedUserOrSharedToOrg``. The nearer sibling goes the other + way: ``ProfileManagerView`` routes every mutation through + ``IsParentToolOwner``, so a shared user can edit a project's prompts but + not its profiles. That asymmetry is deliberate but unexplained by anything + in the code -- treat it as a question for product, not a rule to copy. ``destroy`` on ``ToolStudioPromptView`` is gated by :class:`IsPromptParentToolOwner` instead, so this class does not confer per-prompt deletion. The bulk ``sync_prompts`` route on - ``PromptStudioCoreView`` is likewise ``IsOwner``-gated, so a shared user - cannot reach either deletion path with their session. - - A ``read_write`` platform API key can still call ``sync_prompts`` and - replace a project's prompts wholesale: service accounts short-circuit - ahead of every check here, and that route declares no DELETE-tier - requirement. Known and accepted (UN-3315). + ``PromptStudioCoreView`` is likewise ``IsOwner``-gated. + + Two deletion paths remain open to a non-owner, both known and accepted + (UN-3315). A ``read_write`` platform API key reaches ``sync_prompts``: + service accounts short-circuit ahead of every check here, and that route + declares no DELETE-tier requirement. And ``sync_prompts`` with an empty + ``prompts`` list clears a project's prompts by design -- supported + behaviour, asserted by + ``test_sync_prompts_clear_bumps_tool_modified_at`` -- so the owner gate, + not payload validation, is what stands between a share and that wipe. """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: diff --git a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py index ec0a6ab8ef..ba157801ce 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py +++ b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py @@ -3171,18 +3171,6 @@ def sync_prompts(tool: CustomTool, import_data: dict, user) -> dict: prompts_data = import_data.get("prompts", []) tool_settings = import_data.get("tool_settings", {}) - # Refuse an empty payload before anything is deleted. This is a - # rip-and-replace, but the replace half is a loop over prompts_data - # that simply does not run when the list is empty -- so an empty or - # missing "prompts" wiped every prompt (and its outputs, via CASCADE) - # and reported success. Must stay ahead of the transaction: rolling - # back is not the same as never executing the delete. - if not prompts_data: - raise ValueError( - "No prompts found in the sync payload. Syncing an empty " - "prompt list would delete every prompt in the project." - ) - # Get the target tool's default profile default_profile = ProfileManager.objects.filter( prompt_studio_tool=tool, is_default=True From c35fbe787a87b2f60db5fd9b508587a377591c11 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 02:20:37 +0530 Subject: [PATCH 07/18] UN-3315 [FIX] Gate reorder_prompts, which no permission class reached reorder_prompts is declared @action(detail=True) but routed at the collection path (prompt/reorder/, urls.py:22) with a signature taking no pk -- the target comes from prompt_id in the request body. DRF therefore never calls get_object(), so has_object_permission never fires, and neither permission class defines has_permission, so BasePermission's default True applied. The helper then fetched on the raw manager (helper.py:28, no for_user scoping) and mutated every sibling row sharing the derived tool_id. Net: any authenticated user could renumber the prompts of any project in any organization, shared or not. Resolves the prompt and calls check_object_permissions explicitly, the same shape ToolInstanceViewSet.reorder uses for the identical collection-POST problem (tool_instance_v2/views.py:196-205). Two details worth stating: - Gated as an EDIT, not a deletion. reorder_prompts resolves to PromptAcesssToUser, so org-shared members can reorder, per UN-3315's view + edit ruling. Routing it to owner-only would have over-restricted. - Org scoping goes through the parent tool. ToolStudioPrompt is a plain BaseModel with no organization field and no for_user manager, so filtering on CustomTool.objects.for_user(...) is what makes a cross-org prompt_id 404 before the permission check rather than after it. A missing prompt_id now raises a 400 rather than reaching the controller, which previously surfaced it as a serializer error further in. Untested: the backend suite does not run in this checkout. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- .../prompt_studio/prompt_studio_v2/views.py | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index 26bf030ea1..3f3dc8b5dd 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -1,14 +1,17 @@ from typing import Any from django.db.models import QuerySet +from django.shortcuts import get_object_or_404 from rest_framework import viewsets from rest_framework.decorators import action +from rest_framework.exceptions import ValidationError from rest_framework.request import Request from rest_framework.response import Response from rest_framework.versioning import URLPathVersioning from utils.filtering import FilterHelper from prompt_studio.permission import IsPromptParentToolOwner, PromptAcesssToUser +from prompt_studio.prompt_studio_core_v2.models import CustomTool from prompt_studio.prompt_studio_v2.constants import ToolStudioPromptKeys from prompt_studio.prompt_studio_v2.controller import PromptStudioController from prompt_studio.prompt_studio_v2.models import ToolStudioPrompt @@ -38,9 +41,8 @@ def get_permissions(self) -> list[Any]: # Reads and edits honour project sharing (UN-3315); deleting a prompt # requires ownership of the parent tool, matching CustomToolViewSet, # whose own `destroy` is IsOwner-gated. The bulk `sync_prompts` route - # there is IsOwner-gated too, so a shared user reaches neither - # deletion path (a read_write API key still can -- see - # PromptAcesssToUser's docstring). + # there is IsOwner-gated too. A read_write API key still reaches both + # -- see PromptAcesssToUser's docstring. if self.action == "destroy": return [IsPromptParentToolOwner()] return [PromptAcesssToUser()] @@ -65,11 +67,34 @@ def get_queryset(self) -> QuerySet | None: def reorder_prompts(self, request: Request) -> Response: """Reorder the sequence of prompts based on the provided data. + Routed at the collection path (``prompt/reorder/``) with the target + taken from ``prompt_id`` in the body, so DRF never calls + ``get_object()`` and the viewset's permission class never fires. The + check below is therefore made explicitly -- same shape as + ``ToolInstanceViewSet.reorder``, which has the same collection-POST + problem. Reordering is an *edit*, so it honours project sharing + (UN-3315) rather than requiring ownership. + Args: request (Request): The HTTP request containing the reorder data. Returns: Response: The HTTP response indicating the status of the reorder operation. """ + prompt_id = request.data.get(ToolStudioPromptKeys.PROMPT_ID) + if not prompt_id: + raise ValidationError({ToolStudioPromptKeys.PROMPT_ID: "This is required."}) + # ToolStudioPrompt carries no organization of its own -- it is a plain + # BaseModel -- so scope through the parent tool, whose for_user() + # queryset is org-bound. A cross-org or invisible id 404s here rather + # than reaching the permission check. + prompt = get_object_or_404( + ToolStudioPrompt.objects.filter( + tool_id__in=CustomTool.objects.for_user(request.user) + ), + pk=prompt_id, + ) + self.check_object_permissions(request, prompt) + prompt_studio_controller = PromptStudioController() return prompt_studio_controller.reorder_prompts(request, ToolStudioPrompt) From 70ff027a7bfabe77138bff1b72da68c6c22a33ec Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 02:20:51 +0530 Subject: [PATCH 08/18] UN-3315 [FIX] Gate ProfileManager creation on parent-tool ownership ProfileManagerView.get_permissions already listed create under IsParentToolOwner(), but that class defined only has_object_permission, which DRF never calls for create -- there is no object yet. So the docstring's guarantee, "Mutations require ownership of the parent tool", was false for exactly one action: any org member could create a ProfileManager against any tool by naming it in the payload. Adds has_permission, reading the parent from the request payload (prompt_studio_tool; ProfileManagerSerializer uses fields = "__all__", so it is present on create) and applying the same ownership test as has_object_permission. Service-account and org-admin fallbacks preserved in both. has_object_permission is untouched, so update/partial_update/ destroy keep resolving through get_object() as before. Non-create actions return True from has_permission and are still decided by has_object_permission -- the collection gate must not double-gate an action whose object check already covers it. Malformed input denies rather than crashes: request.data may be a QueryDict, a list, or unparsed garbage, and the pk is a UUID, so a non-dict body, an absent prompt_studio_tool, or an unparseable id returns False and lets the serializer raise its own 400. Django's ValidationError is what a bad UUID raises here, hence the import. Deliberately extends the shared class rather than adding a sibling, which is the opposite of the IsPromptParentToolOwner call two commits back. Not a contradiction: there, the shared class would have had to juggle two different parent FK names (prompt_studio_tool vs tool_id) for two consumers. Here there is one consumer -- ProfileManagerView is the only non-docstring reference to IsParentToolOwner in the tree -- and what is being added is a method the class was always missing. Untested: the backend suite does not run in this checkout. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/permissions/permission.py | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/backend/permissions/permission.py b/backend/permissions/permission.py index 33d4dd5079..f89f018a91 100644 --- a/backend/permissions/permission.py +++ b/backend/permissions/permission.py @@ -2,6 +2,7 @@ from typing import Any from adapter_processor_v2.models import AdapterInstance +from django.core.exceptions import ValidationError from rest_framework import permissions from rest_framework.request import Request from rest_framework.views import APIView @@ -157,8 +158,46 @@ class IsParentToolOwner(permissions.BasePermission): (UN-2202). Falls back to the object's own owner when it has no parent tool (``prompt_studio_tool`` is nullable) to preserve legacy behaviour for orphan rows. + + ``has_permission`` exists because DRF never calls ``has_object_permission`` + for ``create`` -- there is no object yet -- so listing ``create`` in a + viewset's ``get_permissions`` bought nothing, and the "mutations require + ownership of the parent tool" guarantee was simply false for creates + (UN-3315). It reads the parent from the request payload instead, applying + the same ownership test. """ + def has_permission(self, request: Request, view: APIView) -> bool: + """Collection-level gate, for ``create`` (no object exists yet).""" + if _is_service_account(request): + return True + if _is_organization_admin(request): + return True + + # Non-create actions resolve their parent through get_object(), which + # runs has_object_permission below; nothing to check here. + if getattr(view, "action", None) != "create": + return True + + # ``request.data`` may be a QueryDict, a list, or unparsed garbage. + # A malformed or absent parent is not this gate's error to report -- + # deny, and let the serializer raise the 400 it would have raised. + if not isinstance(request.data, dict): + return False + tool_id = request.data.get("prompt_studio_tool") + if not tool_id: + return False + + from prompt_studio.prompt_studio_core_v2.models import CustomTool + + try: + tool = CustomTool.objects.get(pk=tool_id) + except (CustomTool.DoesNotExist, ValidationError, ValueError, TypeError): + # Unknown or unparseable pk (the field is a UUID). Deny rather + # than 500; the serializer surfaces the validation error. + return False + return _is_resource_owner(request.user, tool) + def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: if _is_service_account(request): return True From 7e6b1cd2a32ac49f98a38c14c53bba4849793589 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 02:29:40 +0530 Subject: [PATCH 09/18] UN-3315 [DOC] Correct three claims about the profile and API-key surfaces Three prose defects from 5bc4ee964 and c35fbe787, all in the closing direction -- each told a reader a surface was shut when it is not. 1. "ProfileManagerView routes every mutation through IsParentToolOwner, so a shared user can edit a project's prompts but not its profiles." False twice over. ProfileManagerView routes no create at all (its urls.py has a single detail path), and profiles are created via PromptStudioCoreView.create_profile_manager, which falls through to IsOwnerOrSharedUserOrSharedToOrg and admits org-shared users -- as does make_profile_default. 75fd2acd1 said exactly this and was correct; 5bc4ee964 deleted it and replaced it with the false claim. Restored. 2. "A read_write API key still reaches both." Only one. Per the tier table in _is_service_account, read_write covers POST/PUT/PATCH and full_access adds DELETE, so a read_write key is refused on per-prompt destroy (an HTTP DELETE) and reaches only sync_prompts (a POST). 3. "Two deletion paths remain open to a non-owner" then listed one route twice, the second entry concluding it is in fact owner-gated. Now states the one path, with the empty-list clear recorded separately as supported behaviour rather than as a hole. No behaviour change: comments and docstrings only. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/permission.py | 29 ++++++++++--------- .../prompt_studio/prompt_studio_v2/views.py | 4 +-- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index d0ae1b866e..9d0b15e5cc 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -25,25 +25,28 @@ class PromptAcesssToUser(permissions.BasePermission): only, never mutate"): a Prompt Studio share confers *edit* rights on the project's prompts, matching ``CustomToolViewSet``, which already routes ``update``/``partial_update`` on the tool itself through - ``IsOwnerOrSharedUserOrSharedToOrg``. The nearer sibling goes the other - way: ``ProfileManagerView`` routes every mutation through - ``IsParentToolOwner``, so a shared user can edit a project's prompts but - not its profiles. That asymmetry is deliberate but unexplained by anything - in the code -- treat it as a question for product, not a rule to copy. + ``IsOwnerOrSharedUserOrSharedToOrg``. ``ProfileManagerView`` looks like a + counterexample -- it routes ``update``/``partial_update``/``destroy`` + through ``IsParentToolOwner`` -- but it is not: profiles are *created* + through ``PromptStudioCoreView.create_profile_manager``, which admits + org-shared users, as does ``make_profile_default``. That surface is + share-permissive and unaddressed here. ``destroy`` on ``ToolStudioPromptView`` is gated by :class:`IsPromptParentToolOwner` instead, so this class does not confer per-prompt deletion. The bulk ``sync_prompts`` route on ``PromptStudioCoreView`` is likewise ``IsOwner``-gated. - Two deletion paths remain open to a non-owner, both known and accepted - (UN-3315). A ``read_write`` platform API key reaches ``sync_prompts``: - service accounts short-circuit ahead of every check here, and that route - declares no DELETE-tier requirement. And ``sync_prompts`` with an empty - ``prompts`` list clears a project's prompts by design -- supported - behaviour, asserted by - ``test_sync_prompts_clear_bumps_tool_modified_at`` -- so the owner gate, - not payload validation, is what stands between a share and that wipe. + One deletion path remains open to a non-owner, known and accepted + (UN-3315): a ``read_write`` platform API key reaches ``sync_prompts``. + Service accounts short-circuit ahead of every check here, and that route + declares no DELETE-tier requirement -- being a POST, it is not covered by + the DELETE tier that guards per-prompt ``destroy``. + + Separately, and not a hole: ``sync_prompts`` with an empty ``prompts`` + list clears a project's prompts by design -- supported behaviour, asserted + by ``test_sync_prompts_clear_bumps_tool_modified_at``. The owner gate, not + payload validation, is what stands between a share and that wipe. """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index 3f3dc8b5dd..047a2badb7 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -41,8 +41,8 @@ def get_permissions(self) -> list[Any]: # Reads and edits honour project sharing (UN-3315); deleting a prompt # requires ownership of the parent tool, matching CustomToolViewSet, # whose own `destroy` is IsOwner-gated. The bulk `sync_prompts` route - # there is IsOwner-gated too. A read_write API key still reaches both - # -- see PromptAcesssToUser's docstring. + # there is IsOwner-gated too. A read_write API key still reaches + # sync_prompts (a POST) -- see PromptAcesssToUser's docstring. if self.action == "destroy": return [IsPromptParentToolOwner()] return [PromptAcesssToUser()] From 6c7c18638ecf5ba2dcb95137ea9488b8741f70fd Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 02:35:46 +0530 Subject: [PATCH 10/18] UN-3315 [TEST] Pin the prompt authorization split Fourteen tests over the three mechanisms enforcing "a share grants view and edit, not delete". Every one is mutation-checked: the fix was reverted, the test observed to fail, then restored. 1. destroy split reverted to a flat permission_classes -> fails 2. split widened to gate update/partial_update too -> fails (x2) 3. check_object_permissions deleted from reorder_prompts -> fails (x2) 4. "sync_prompts" removed from the IsOwner list -> fails 5. the UN-3315 shared_to_org branch removed -> fails Mutation 2 is the over-restriction guard. UN-3315 grants edit; a split that routed every mutation to the owner-only class would pass the deletion tests while silently removing the capability this work exists to add. Mutation 5 covers the same axis from the other side. Imports the real modules rather than slicing bodies out with tests_common.source_extraction, as the sibling registry suite does. That technique's own docstring records why it cannot serve here: bodies are exec-ed out of context, so unreachable code is indistinguishable from wired code -- and "the hook is never reached" IS the reorder_prompts defect. A source-extracted test of PromptAcesssToUser.has_object_permission would have passed both before and after that fix. The same docstring notes the premise behind extraction no longer holds (Django is importable in this tier), and importing also avoids its other two sharp edges. Correcting the record: four earlier commits in this series say "the backend suite does not run in this checkout". That is true only of the DB-backed tier. The permission tier is deliberately DB-free and runs in about a second; the blocker was that settings vars must be exported into the environment, not merely written to test.env. Those notes were wrong. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- .../prompt_studio_v2/tests/__init__.py | 0 .../tests/test_prompt_permission_guards.py | 292 ++++++++++++++++++ 2 files changed, 292 insertions(+) create mode 100644 backend/prompt_studio/prompt_studio_v2/tests/__init__.py create mode 100644 backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py diff --git a/backend/prompt_studio/prompt_studio_v2/tests/__init__.py b/backend/prompt_studio/prompt_studio_v2/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py b/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py new file mode 100644 index 0000000000..5d76e69517 --- /dev/null +++ b/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py @@ -0,0 +1,292 @@ +"""Regression tests for the UN-3315 prompt authorization split. + +``shared_to_org`` grants org members **view and edit** on a project's prompts, +not delete. Three separate mechanisms enforce that, and each has a distinct +failure mode: + +1. ``ToolStudioPromptView.get_permissions`` routes ``destroy`` to + :class:`IsPromptParentToolOwner` while every other action keeps + :class:`PromptAcesssToUser`. Collapsing it back to a flat + ``permission_classes`` reopens org-wide DELETE; widening it to gate all + mutations silently removes the edit rights the ruling granted. Both + directions are covered -- an over-restriction is as much a defect here as + an under-restriction. + +2. ``PromptStudioCoreView.get_permissions`` routes ``sync_prompts`` to + ``IsOwner``. That route rip-and-replaces every prompt in the project, so a + share must not reach it. + +3. ``reorder_prompts`` is routed at a *collection* path with the target taken + from the request body, so DRF never calls ``get_object()`` and the + class-based hook never fires on its own. Its guard is an explicit + ``check_object_permissions`` call inside the action. + +Mechanism 3 is why these tests import the real modules rather than extracting +method bodies with ``tests_common.source_extraction``, as the sibling +``test_registry_tool_delete_guards.py`` does. That technique's own docstring +records the blind spot: bodies are ``exec``-ed out of context, so *unreachable* +code is indistinguishable from wired code -- and "the hook is never reached" is +precisely the bug mechanism 3 fixes. A source-extracted test of +``PromptAcesssToUser.has_object_permission`` would have passed both before and +after that fix. The same docstring notes the premise behind extraction no +longer holds: Django is importable in this tier. Importing also avoids its two +other sharp edges (a decorator above a definition truncates the extracted +slice; a cosmetic annotation change breaks the marker match). + +No database is touched. ``get_permissions()`` is pure, and the collaborators +that would hit the ORM are patched. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import Any +from unittest.mock import patch + +import pytest +from rest_framework.exceptions import ValidationError + +from prompt_studio.permission import IsPromptParentToolOwner, PromptAcesssToUser +from prompt_studio.prompt_studio_core_v2.views import PromptStudioCoreView +from prompt_studio.prompt_studio_v2.views import ToolStudioPromptView + +PERMISSION_MODULE = "prompt_studio.permission" + + +class _User: + def __init__(self, name: str, is_service_account: bool = False) -> None: + self.name = name + self.is_service_account = is_service_account + + def __repr__(self) -> str: # pragma: no cover - debugging aid + return f"" + + +OWNER = _User("owner") +SHARED_MEMBER = _User("shared-member") + + +def _tool(*, owner: _User, shared_to_org: bool) -> SimpleNamespace: + return SimpleNamespace(owner=owner, shared_to_org=shared_to_org) + + +def _prompt(tool: SimpleNamespace | None) -> SimpleNamespace: + """A ``ToolStudioPrompt`` stand-in. The FK really is named ``tool_id``.""" + return SimpleNamespace(tool_id=tool) + + +def _request(user: _User) -> SimpleNamespace: + return SimpleNamespace(user=user, data={}) + + +def _owns(user: _User, obj: Any) -> bool: + return getattr(obj, "owner", None) is user + + +def _permission_for(view_cls: type, action: str) -> list[Any]: + view = view_cls() + view.action = action + return view.get_permissions() + + +class TestPromptDeletionIsOwnerOnly: + """``destroy`` must not be reachable through an org-wide share.""" + + def test_destroy_resolves_the_parent_owner_permission(self) -> None: + """Collapsing the split back to a flat list reopens org-wide DELETE.""" + permissions = _permission_for(ToolStudioPromptView, "destroy") + + assert any(isinstance(p, IsPromptParentToolOwner) for p in permissions), ( + "destroy must resolve IsPromptParentToolOwner; a flat " + "permission_classes would let any org member delete prompts in a " + "shared project" + ) + + @pytest.mark.parametrize("action", ["update", "partial_update", "retrieve"]) + def test_edits_and_reads_keep_the_share_aware_permission(self, action: str) -> None: + """The over-restriction guard. + + UN-3315 grants org-shared members *edit*. Routing every mutation to the + owner-only class would satisfy the deletion test above while quietly + removing the capability the change exists to provide. + """ + permissions = _permission_for(ToolStudioPromptView, action) + + assert any(isinstance(p, PromptAcesssToUser) for p in permissions), ( + f"{action} must keep PromptAcesssToUser so shared members retain " + "the edit rights UN-3315 granted" + ) + assert not any(isinstance(p, IsPromptParentToolOwner) for p in permissions) + + def test_shared_member_is_denied_deletion(self) -> None: + """The behaviour behind the wiring: a share is not an owner.""" + tool = _tool(owner=OWNER, shared_to_org=True) + + with ( + patch(f"{PERMISSION_MODULE}._is_resource_owner", side_effect=_owns), + patch(f"{PERMISSION_MODULE}.OrganizationMemberService") as service, + ): + service.is_user_organization_admin.return_value = False + allowed = IsPromptParentToolOwner().has_object_permission( + _request(SHARED_MEMBER), None, _prompt(tool) + ) + + assert allowed is False, ( + "shared_to_org must not confer deletion -- this is the widening " + "UN-3315 deliberately excluded" + ) + + def test_owner_may_delete(self) -> None: + tool = _tool(owner=OWNER, shared_to_org=True) + + with ( + patch(f"{PERMISSION_MODULE}._is_resource_owner", side_effect=_owns), + patch(f"{PERMISSION_MODULE}.OrganizationMemberService") as service, + ): + service.is_user_organization_admin.return_value = False + allowed = IsPromptParentToolOwner().has_object_permission( + _request(OWNER), None, _prompt(tool) + ) + + assert allowed is True + + def test_orphaned_prompt_denies_rather_than_raising(self) -> None: + """``tool_id`` is nullable (``SET_NULL``). + + An orphaned prompt has no owner to inherit from, so it must fall + through to the org-admin check rather than raising AttributeError on + ``None`` -- a 403, not a 500. + """ + with ( + patch(f"{PERMISSION_MODULE}._is_resource_owner", side_effect=_owns), + patch(f"{PERMISSION_MODULE}.OrganizationMemberService") as service, + ): + service.is_user_organization_admin.return_value = False + allowed = IsPromptParentToolOwner().has_object_permission( + _request(SHARED_MEMBER), None, _prompt(None) + ) + + assert allowed is False + + def test_shared_member_may_still_edit(self) -> None: + """The paired behaviour: the same member denied delete keeps edit.""" + tool = _tool(owner=OWNER, shared_to_org=True) + + with ( + patch(f"{PERMISSION_MODULE}._is_resource_owner", return_value=False), + patch(f"{PERMISSION_MODULE}._is_resource_viewer", return_value=False), + patch(f"{PERMISSION_MODULE}.has_group_access", return_value=False), + patch(f"{PERMISSION_MODULE}.OrganizationMemberService") as service, + ): + service.is_user_organization_admin.return_value = False + allowed = PromptAcesssToUser().has_object_permission( + _request(SHARED_MEMBER), None, _prompt(tool) + ) + + assert allowed is True, ( + "shared_to_org is the only grant path left standing here; if this " + "fails the UN-3315 branch itself has been removed" + ) + + +class TestSyncPromptsIsOwnerOnly: + """``sync_prompts`` deletes every prompt in the project before importing.""" + + def test_sync_prompts_resolves_the_owner_permission(self) -> None: + from permissions.permission import IsOwner + + permissions = _permission_for(PromptStudioCoreView, "sync_prompts") + + assert any(isinstance(p, IsOwner) for p in permissions), ( + "sync_prompts rip-and-replaces every prompt; dropping it from the " + "IsOwner list lets an org-shared member wipe the project" + ) + + def test_reads_are_not_owner_gated(self) -> None: + """The paired direction -- sharing must still reach ordinary reads.""" + from permissions.permission import IsOwnerOrSharedUserOrSharedToOrg + + permissions = _permission_for(PromptStudioCoreView, "retrieve") + + assert any(isinstance(p, IsOwnerOrSharedUserOrSharedToOrg) for p in permissions) + + +class TestReorderPromptsIsGated: + """``reorder_prompts`` is a collection POST -- ``get_object()`` never runs. + + The defect was never that the permission class was wrong; it was that + nothing invoked it. So these tests pin the *call*, not the verdict. + """ + + def test_reorder_resolves_the_share_aware_permission(self) -> None: + """Reordering is an edit, so a share must reach it.""" + permissions = _permission_for(ToolStudioPromptView, "reorder_prompts") + + assert any(isinstance(p, PromptAcesssToUser) for p in permissions) + assert not any(isinstance(p, IsPromptParentToolOwner) for p in permissions), ( + "reordering is an edit, not a deletion -- owner-only would " + "over-restrict against the UN-3315 ruling" + ) + + def test_action_calls_check_object_permissions(self) -> None: + """The regression that matters. + + Deleting the explicit ``check_object_permissions`` call leaves the + action fully functional and completely ungated, which is exactly the + state this fix found it in. + """ + view = ToolStudioPromptView() + view.action = "reorder_prompts" + view.request = None + prompt = _prompt(_tool(owner=OWNER, shared_to_org=True)) + request = SimpleNamespace(user=SHARED_MEMBER, data={"prompt_id": "p-1"}) + + module = "prompt_studio.prompt_studio_v2.views" + with ( + patch(f"{module}.get_object_or_404", return_value=prompt), + patch(f"{module}.CustomTool"), + patch.object(ToolStudioPromptView, "check_object_permissions") as checked, + patch(f"{module}.PromptStudioController") as controller, + ): + controller.return_value.reorder_prompts.return_value = "ok" + view.reorder_prompts(request) + + checked.assert_called_once() + assert checked.call_args.args[1] is prompt, ( + "the permission check must run against the prompt the action is " + "about to reorder" + ) + + def test_permission_check_precedes_the_mutation(self) -> None: + """Order matters: a denial must stop the reorder, not follow it.""" + view = ToolStudioPromptView() + view.action = "reorder_prompts" + view.request = None + prompt = _prompt(_tool(owner=OWNER, shared_to_org=False)) + request = SimpleNamespace(user=SHARED_MEMBER, data={"prompt_id": "p-1"}) + + from rest_framework.exceptions import PermissionDenied + + module = "prompt_studio.prompt_studio_v2.views" + with ( + patch(f"{module}.get_object_or_404", return_value=prompt), + patch(f"{module}.CustomTool"), + patch.object( + ToolStudioPromptView, + "check_object_permissions", + side_effect=PermissionDenied, + ), + patch(f"{module}.PromptStudioController") as controller, + ): + with pytest.raises(PermissionDenied): + view.reorder_prompts(request) + + controller.return_value.reorder_prompts.assert_not_called() + + def test_missing_prompt_id_is_a_400_not_a_crash(self) -> None: + view = ToolStudioPromptView() + view.action = "reorder_prompts" + view.request = None + + with pytest.raises(ValidationError): + view.reorder_prompts(SimpleNamespace(user=SHARED_MEMBER, data={})) From 54f5fa3de1f8c06aadc5b45164b0c7b58aac3563 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 02:37:05 +0530 Subject: [PATCH 11/18] UN-3315 [REVERT] Drop IsParentToolOwner.has_permission, which was unreachable 70ff027a7 added has_permission to close what looked like an ungated create: ProfileManagerView.get_permissions lists "create" under IsParentToolOwner, and that class defined only has_object_permission, which DRF never calls for create. The second half is true. The first is not. ProfileManagerView exposes no create route. prompt_profile_manager_v2/ urls.py binds exactly one path -- profile-manager// -- to {get: retrieve, put: update, patch: partial_update, delete: destroy}. No collection POST, and no router registration anywhere in the tree; the only other references to the viewset are that import and a docstring. So self.action is never "create", the method always took its non-create early return, and it closed nothing. The "create" entry in that get_permissions list is itself dead, but it predates this PR. The real gap is on PromptStudioCoreView: create_profile_manager (views.py:958) and make_profile_default both fall through get_permissions to IsOwnerOrSharedUserOrSharedToOrg, so an org-shared member can create a profile on someone else's project and change which profile is default. Deliberately not addressed here -- gating them narrows a shipped capability on a viewset outside this PR's scope, which is a product decision. make_profile_default carries a second, separate defect worth its own ticket: at views.py:401-407 it clears is_default across the tool's profiles and then resolves the promoted profile with ProfileManager.objects.get(pk=request.data["default_profile"]) on the raw manager, so the profile being promoted is never checked against the tool it is being made default for. No test accompanied 70ff027a7 and none is removed here. A test would have constructed a view with action="create", passed, and mutation-checked correctly while the production path stayed ungated -- vacuous in exactly the way that hides this class of defect. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/permissions/permission.py | 39 ------------------------------- 1 file changed, 39 deletions(-) diff --git a/backend/permissions/permission.py b/backend/permissions/permission.py index f89f018a91..33d4dd5079 100644 --- a/backend/permissions/permission.py +++ b/backend/permissions/permission.py @@ -2,7 +2,6 @@ from typing import Any from adapter_processor_v2.models import AdapterInstance -from django.core.exceptions import ValidationError from rest_framework import permissions from rest_framework.request import Request from rest_framework.views import APIView @@ -158,46 +157,8 @@ class IsParentToolOwner(permissions.BasePermission): (UN-2202). Falls back to the object's own owner when it has no parent tool (``prompt_studio_tool`` is nullable) to preserve legacy behaviour for orphan rows. - - ``has_permission`` exists because DRF never calls ``has_object_permission`` - for ``create`` -- there is no object yet -- so listing ``create`` in a - viewset's ``get_permissions`` bought nothing, and the "mutations require - ownership of the parent tool" guarantee was simply false for creates - (UN-3315). It reads the parent from the request payload instead, applying - the same ownership test. """ - def has_permission(self, request: Request, view: APIView) -> bool: - """Collection-level gate, for ``create`` (no object exists yet).""" - if _is_service_account(request): - return True - if _is_organization_admin(request): - return True - - # Non-create actions resolve their parent through get_object(), which - # runs has_object_permission below; nothing to check here. - if getattr(view, "action", None) != "create": - return True - - # ``request.data`` may be a QueryDict, a list, or unparsed garbage. - # A malformed or absent parent is not this gate's error to report -- - # deny, and let the serializer raise the 400 it would have raised. - if not isinstance(request.data, dict): - return False - tool_id = request.data.get("prompt_studio_tool") - if not tool_id: - return False - - from prompt_studio.prompt_studio_core_v2.models import CustomTool - - try: - tool = CustomTool.objects.get(pk=tool_id) - except (CustomTool.DoesNotExist, ValidationError, ValueError, TypeError): - # Unknown or unparseable pk (the field is a UUID). Deny rather - # than 500; the serializer surfaces the validation error. - return False - return _is_resource_owner(request.user, tool) - def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: if _is_service_account(request): return True From 7d4f2813d7928eb52e51e6ab84bd127c4585dd45 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 02:48:07 +0530 Subject: [PATCH 12/18] UN-3315 [CLEANUP] Trim duplicated prose and save two queries per check Behaviour-preserving cleanup after the fixes settled. Tests re-run and the mutation checks re-confirmed: removing the shared_to_org branch still fails 1 test, removing check_object_permissions still fails 2. Prose: the PromptAcesssToUser docstring had accumulated a routing table stating, from inside a permission class, which actions two other viewsets gate -- facts already stated at both enforcement sites, so three copies that drift independently. Cut to the one line a reader of this class needs. Same for the ProfileManagerView rebuttal and the empty-prompts note, whose subject is payload validation on a route this class does not guard. The read_write API-key gap stays: it is a load-bearing accepted-risk warning, not restatement. The duplicate of it in views.py becomes a pointer. Queries: hoisted `tool.shared_to_org` above the owner and viewer checks. It is a free attribute read and each branch it now precedes runs an .exists() query, so the org-share path -- the one UN-3315 exists to serve -- saves up to two. Order is not otherwise observable: the method is a plain OR of side-effect-free predicates. Added select_related("tool_id") to the reorder lookup, since the permission class dereferences that FK immediately and the parent is already joined by the filter. Not taken, deliberately: - Threading the fetched prompt through PromptStudioController into reorder_prompts_helper to kill its duplicate SELECT. Real (one wasted round-trip per reorder) but it changes two signatures outside this diff and makes the controller's DoesNotExist branch dead. - Extracting a parent-owner base class over IsParentToolOwner / IsRegistryToolOwner / IsPromptParentToolOwner. The duplication is real and predates this PR; consolidating touches two unchanged auth classes, which is not scope for a permissions fix. - Switching the inline service-account/admin checks to the shared _is_service_account / _is_organization_admin helpers. Right in principle and would pick up the per-request admin cache, but all three classes in this file hand-roll them; changing one creates divergence rather than removing it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/permission.py | 35 +++++++------------ .../tests/test_prompt_permission_guards.py | 10 +----- .../prompt_studio/prompt_studio_v2/views.py | 9 ++--- 3 files changed, 18 insertions(+), 36 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index 9d0b15e5cc..878e0ce03c 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -32,42 +32,31 @@ class PromptAcesssToUser(permissions.BasePermission): org-shared users, as does ``make_profile_default``. That surface is share-permissive and unaddressed here. - ``destroy`` on ``ToolStudioPromptView`` is gated by - :class:`IsPromptParentToolOwner` instead, so this class does not confer - per-prompt deletion. The bulk ``sync_prompts`` route on - ``PromptStudioCoreView`` is likewise ``IsOwner``-gated. + This class does not confer deletion; ``destroy`` is gated by + :class:`IsPromptParentToolOwner`. One deletion path remains open to a non-owner, known and accepted (UN-3315): a ``read_write`` platform API key reaches ``sync_prompts``. - Service accounts short-circuit ahead of every check here, and that route - declares no DELETE-tier requirement -- being a POST, it is not covered by - the DELETE tier that guards per-prompt ``destroy``. - - Separately, and not a hole: ``sync_prompts`` with an empty ``prompts`` - list clears a project's prompts by design -- supported behaviour, asserted - by ``test_sync_prompts_clear_bumps_tool_modified_at``. The owner gate, not - payload validation, is what stands between a share and that wipe. + Service accounts short-circuit ahead of every check here, and being a POST + that route is not covered by the DELETE tier that guards per-prompt + ``destroy``. """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: if getattr(request.user, "is_service_account", False): return True tool = obj.tool_id + # UN-3315: "Share with everyone" sets shared_to_org on the parent tool. + # Checked first among the grant paths because it is a free attribute + # read, while every branch below it runs a query -- and it is the path + # UN-3315 exists to serve. Order is not otherwise observable: these are + # side-effect-free predicates OR'd together. + if tool.shared_to_org: + return True if _is_resource_owner(request.user, tool): return True if _is_resource_viewer(request.user, tool): return True - # UN-3315: "Share with everyone" sets shared_to_org on the parent tool. - # IsOwnerOrSharedUserOrSharedToOrg already honours it, so a user whose - # only access came from that flag (no VIEWER row, no group share, not an - # admin) could not reach the project's prompts at all. - # - # Read directly rather than via getattr: on a CustomTool the field is - # a non-nullable BooleanField, so a default would only mask a renamed - # field by silently denying access. Matches - # IsOwnerOrSharedUserOrSharedToOrg, which also reads it directly. - if tool.shared_to_org: - return True if has_group_access(request.user, tool): return True return OrganizationMemberService.is_user_organization_admin(request.user) diff --git a/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py b/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py index 5d76e69517..6f603b68b9 100644 --- a/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py +++ b/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py @@ -26,12 +26,7 @@ ``test_registry_tool_delete_guards.py`` does. That technique's own docstring records the blind spot: bodies are ``exec``-ed out of context, so *unreachable* code is indistinguishable from wired code -- and "the hook is never reached" is -precisely the bug mechanism 3 fixes. A source-extracted test of -``PromptAcesssToUser.has_object_permission`` would have passed both before and -after that fix. The same docstring notes the premise behind extraction no -longer holds: Django is importable in this tier. Importing also avoids its two -other sharp edges (a decorator above a definition truncates the extracted -slice; a cosmetic annotation change breaks the marker match). +precisely the bug mechanism 3 fixes. No database is touched. ``get_permissions()`` is pure, and the collaborators that would hit the ORM are patched. @@ -237,7 +232,6 @@ def test_action_calls_check_object_permissions(self) -> None: """ view = ToolStudioPromptView() view.action = "reorder_prompts" - view.request = None prompt = _prompt(_tool(owner=OWNER, shared_to_org=True)) request = SimpleNamespace(user=SHARED_MEMBER, data={"prompt_id": "p-1"}) @@ -261,7 +255,6 @@ def test_permission_check_precedes_the_mutation(self) -> None: """Order matters: a denial must stop the reorder, not follow it.""" view = ToolStudioPromptView() view.action = "reorder_prompts" - view.request = None prompt = _prompt(_tool(owner=OWNER, shared_to_org=False)) request = SimpleNamespace(user=SHARED_MEMBER, data={"prompt_id": "p-1"}) @@ -286,7 +279,6 @@ def test_permission_check_precedes_the_mutation(self) -> None: def test_missing_prompt_id_is_a_400_not_a_crash(self) -> None: view = ToolStudioPromptView() view.action = "reorder_prompts" - view.request = None with pytest.raises(ValidationError): view.reorder_prompts(SimpleNamespace(user=SHARED_MEMBER, data={})) diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index 047a2badb7..03995b3f8c 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -41,8 +41,7 @@ def get_permissions(self) -> list[Any]: # Reads and edits honour project sharing (UN-3315); deleting a prompt # requires ownership of the parent tool, matching CustomToolViewSet, # whose own `destroy` is IsOwner-gated. The bulk `sync_prompts` route - # there is IsOwner-gated too. A read_write API key still reaches - # sync_prompts (a POST) -- see PromptAcesssToUser's docstring. + # there is IsOwner-gated too. API-key gap: see PromptAcesssToUser. if self.action == "destroy": return [IsPromptParentToolOwner()] return [PromptAcesssToUser()] @@ -87,11 +86,13 @@ def reorder_prompts(self, request: Request) -> Response: # ToolStudioPrompt carries no organization of its own -- it is a plain # BaseModel -- so scope through the parent tool, whose for_user() # queryset is org-bound. A cross-org or invisible id 404s here rather - # than reaching the permission check. + # than reaching the permission check. select_related because the + # permission class immediately dereferences the tool_id FK, and the + # parent is already joined by the filter above. prompt = get_object_or_404( ToolStudioPrompt.objects.filter( tool_id__in=CustomTool.objects.for_user(request.user) - ), + ).select_related("tool_id"), pk=prompt_id, ) self.check_object_permissions(request, prompt) From c3a0316640a822a61cd99c724cf8d7469c9ebfcd Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 03:01:08 +0530 Subject: [PATCH 13/18] UN-3315 [FIX] Scope make_profile_default's lookup to its own tool make_profile_default cleared is_default across the target tool's profiles and then resolved the promoted profile with ProfileManager.objects.get(pk=request.data["default_profile"]) -- the raw manager, with no check that the profile belongs to that tool. Promoting a profile from another tool therefore succeeded, and because the clear had already run, the target was left with NO default of its own and a foreign profile marked default for it. Resolves the profile first, scoped to prompt_studio_tool, and only then clears. Order matters as much as the scoping: 404-ing after the clear would still leave the tool without a default, so a refused promotion must modify nothing. The test pins the ordering, not just the filter -- moving the clear back ahead of the lookup fails it. Also replaces the bare request.data["default_profile"] KeyError (a 500 when the field is absent) with a 400. No capability change: who may call this route is unchanged. Gating it and create_profile_manager to IsOwner was proposed and reversed by the user; org-shared members keep both, as they ship today. Restores two docstring passages that 7d4f2813d cut as redundant. They are not: that sync_prompts with an empty prompts list clears a project BY DESIGN is the conclusion that cost a wrong guard, a commit and a forward revert, and the next reader will re-propose that guard without it. Now says plainly not to. The note that sync_prompts is IsOwner-gated is likewise true and load-bearing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/permission.py | 10 +++++++++- .../prompt_studio/prompt_studio_core_v2/views.py | 16 +++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index 878e0ce03c..2b87bf7c5f 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -33,13 +33,21 @@ class PromptAcesssToUser(permissions.BasePermission): share-permissive and unaddressed here. This class does not confer deletion; ``destroy`` is gated by - :class:`IsPromptParentToolOwner`. + :class:`IsPromptParentToolOwner`, and the bulk ``sync_prompts`` route on + ``PromptStudioCoreView`` is likewise ``IsOwner``-gated. One deletion path remains open to a non-owner, known and accepted (UN-3315): a ``read_write`` platform API key reaches ``sync_prompts``. Service accounts short-circuit ahead of every check here, and being a POST that route is not covered by the DELETE tier that guards per-prompt ``destroy``. + + Separately, and not a hole: ``sync_prompts`` with an empty ``prompts`` + list clears a project's prompts by design -- supported behaviour, asserted + by ``test_sync_prompts_clear_bumps_tool_modified_at``. Do not "fix" it with + a payload guard; that breaks a published contract and its own test. The + owner gate, not payload validation, is what stands between a share and + that wipe. """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index f23a20000e..30f68c9b12 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -13,6 +13,7 @@ from django.db import IntegrityError from django.db.models import Count, OuterRef, QuerySet, Subquery from django.http import HttpRequest, HttpResponse +from django.shortcuts import get_object_or_404 from django.utils import timezone from file_management.constants import FileInformationKey as FileKey from file_management.exceptions import FileNotFound @@ -24,6 +25,7 @@ from plugins import get_plugin from rest_framework import status, viewsets from rest_framework.decorators import action +from rest_framework.exceptions import ValidationError from rest_framework.request import Request from rest_framework.response import Response from rest_framework.versioning import URLPathVersioning @@ -398,11 +400,23 @@ def make_profile_default(self, request: HttpRequest, pk: Any = None) -> Response self.get_object() ) # Assuming you have a get_object method in your viewset + default_profile = request.data.get("default_profile") + if not default_profile: + raise ValidationError({"default_profile": "This field is required."}) + + # Resolve before clearing, and scope to this tool. Unscoped, a profile + # belonging to another tool could be promoted here -- and because the + # clear above had already run, the tool was left with no default of its + # own and a foreign profile marked default. A mismatch now 404s with + # nothing modified. + profile_manager = get_object_or_404( + ProfileManager, pk=default_profile, prompt_studio_tool=prompt_tool + ) + ProfileManager.objects.filter(prompt_studio_tool=prompt_tool).update( is_default=False ) - profile_manager = ProfileManager.objects.get(pk=request.data["default_profile"]) profile_manager.is_default = True profile_manager.save() From 615b16e34290392f29a48e5172133166b06b9fec Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 03:01:21 +0530 Subject: [PATCH 14/18] UN-3315 [TEST] Pin make_profile_default's tool scoping Three tests, each mutation-checked: A1 drop `prompt_studio_tool=` from the lookup -> fails A2 restore the bare request.data[...] KeyError -> fails A3 move the is_default clear back ahead of the lookup (the original bug's ordering) -> fails A3 is the one worth having. A test that only asserted the filter would pass against a version that 404s after wiping the tool's default -- which is the same broken end state, reached a different way. It asserts the clear did not run when the promotion is refused. Extends the over-restriction tripwire to create_profile_manager and make_profile_default, asserting they still resolve the share-aware class. Gating them was proposed and reversed by the user, so this pins the reversal: current behaviour, not the abandoned change. A future edit that sweeps every action to owner-only now fails here. 19 pass in the file; 129 across the permission tier, unchanged from before these commits apart from the 3 added here plus the 4 extra parametrized cases. The 53 Postgres errors in the wider run are pre-existing and identical at the pre-PR baseline. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- .../tests/test_prompt_permission_guards.py | 85 ++++++++++++++++++- 1 file changed, 82 insertions(+), 3 deletions(-) diff --git a/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py b/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py index 6f603b68b9..4b6948fbb5 100644 --- a/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py +++ b/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py @@ -39,6 +39,7 @@ from unittest.mock import patch import pytest +from django.http import Http404 from rest_framework.exceptions import ValidationError from prompt_studio.permission import IsPromptParentToolOwner, PromptAcesssToUser @@ -197,15 +198,93 @@ def test_sync_prompts_resolves_the_owner_permission(self) -> None: "IsOwner list lets an org-shared member wipe the project" ) - def test_reads_are_not_owner_gated(self) -> None: - """The paired direction -- sharing must still reach ordinary reads.""" + @pytest.mark.parametrize( + "action", ["retrieve", "create_profile_manager", "make_profile_default"] + ) + def test_other_actions_are_not_owner_gated(self, action: str) -> None: + """The over-restriction tripwire, asserting CURRENT behaviour. + + Sharing must still reach ordinary reads, and the two profile routes + remain share-permissive by decision -- an org-shared member may create + a profile and change which one is default. Gating them was proposed + and deliberately reversed, so this pins the reversal: a future edit + cannot quietly sweep every action to owner-only. + """ from permissions.permission import IsOwnerOrSharedUserOrSharedToOrg - permissions = _permission_for(PromptStudioCoreView, "retrieve") + permissions = _permission_for(PromptStudioCoreView, action) assert any(isinstance(p, IsOwnerOrSharedUserOrSharedToOrg) for p in permissions) +class TestMakeProfileDefaultIsScopedToItsTool: + """Promoting a default must not reach across tools. + + ``make_profile_default`` clears ``is_default`` across the target tool's + profiles and then promotes one. Resolved unscoped, a profile belonging to + another tool could be promoted -- and since the clear had already run, the + tool was left with no default of its own and a foreign profile marked + default for it. + """ + + @staticmethod + def _call(view: Any, request: Any) -> Any: + return PromptStudioCoreView.make_profile_default(view, request) + + def test_profile_from_another_tool_is_not_promotable(self) -> None: + """The cross-tool promotion this fix closes.""" + view = SimpleNamespace(get_object=lambda: "tool-A") + request = SimpleNamespace(data={"default_profile": "profile-of-tool-B"}) + module = "prompt_studio.prompt_studio_core_v2.views" + + with ( + patch(f"{module}.get_object_or_404", side_effect=Http404) as lookup, + patch(f"{module}.ProfileManager") as profile_manager, + ): + with pytest.raises(Http404): + self._call(view, request) + + # Scoped to the parent tool -- an unscoped lookup would have found it. + assert lookup.call_args.kwargs["prompt_studio_tool"] == "tool-A" + ( + profile_manager.objects.filter.return_value.update.assert_not_called(), + ( + "nothing may be modified when the promotion is refused -- the " + "is_default clear must not run ahead of a failed lookup" + ), + ) + + def test_same_tool_profile_is_promoted(self) -> None: + """The happy path still works, and still clears the old default.""" + view = SimpleNamespace(get_object=lambda: "tool-A") + request = SimpleNamespace(data={"default_profile": "profile-of-tool-A"}) + promoted = SimpleNamespace(is_default=False, profile_id="profile-of-tool-A") + module = "prompt_studio.prompt_studio_core_v2.views" + + with ( + patch(f"{module}.get_object_or_404", return_value=promoted), + patch(f"{module}.ProfileManager") as profile_manager, + ): + promoted.save = lambda: None + response = self._call(view, request) + + assert promoted.is_default is True + profile_manager.objects.filter.assert_called_once_with( + prompt_studio_tool="tool-A" + ) + profile_manager.objects.filter.return_value.update.assert_called_once_with( + is_default=False + ) + assert response.data == {"default_profile": "profile-of-tool-A"} + + def test_missing_default_profile_is_a_400_not_a_500(self) -> None: + """``request.data["default_profile"]`` was a bare KeyError.""" + view = SimpleNamespace(get_object=lambda: "tool-A") + + with pytest.raises(ValidationError): + self._call(view, SimpleNamespace(data={})) + + class TestReorderPromptsIsGated: """``reorder_prompts`` is a collection POST -- ``get_object()`` never runs. From 636cedfa432b0d2e443c3abd9be2b79585a970f6 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 08:19:56 +0530 Subject: [PATCH 15/18] UN-3315 [FIX] Gate a prompt reparent on the parent it leaves PromptAcesssToUser admits org-shared members to update/partial_update while IsPromptParentToolOwner denies them destroy -- but that gate reads the STORED parent, which DRF loads before the update is applied. tool_id is client-writable (fields="__all__", and the model FK leaves editable=True unlike created_by/modified_by), so a denied user could: PATCH /prompt// {"tool_id": ""} -> 200 DELETE /prompt// -> 200 Two requests, each passing every check, and Alice's prompt is gone. Every test this PR added passes throughout. Moving a prompt out of a project removes it from that project -- a deletion from the losing side -- so it now requires what destroy requires, checked against the EXISTING parent. Checking the NEW parent would pass trivially: the attacker's destination is a tool they already own, and the harm is the prompt leaving the original project regardless of where it lands. Three behaviours preserved deliberately, each with a test: - An unchanged tool_id is a no-op, not a reparent. The Prompt Studio UI PATCHes one field at a time (DocumentParser.jsx builds {[name]: value}), so nothing in-tree is affected either way, but a payload echoing the parent back must not 403. - An owner reparenting between tools they own still works. - null is a reparent, not a no-op. Orphaning the row hides it from everyone, its owner and org admins included, once the org filter's INNER JOIN excludes it. A deliberate tightening, not a bug fix: a request that succeeds today will 403. Verified no legitimate caller does this -- the only two PATCH callers in frontend/src hit tool_instance/ and workflow/endpoint/, and the prompt PATCH never carries tool_id. This NARROWS the bypass; it does not make tool_id unwritable. The complete fix is a read-only field on update, declined on API-contract grounds because DRF silently ignores read-only input -- a legitimate reparent would get 200 and no effect. Preserved at: unstract-pr2259-pending/fix1-tool_id-readonly-DECLINED.patch Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- .../prompt_studio/prompt_studio_v2/views.py | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index 03995b3f8c..1a45ea603b 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -4,7 +4,7 @@ from django.shortcuts import get_object_or_404 from rest_framework import viewsets from rest_framework.decorators import action -from rest_framework.exceptions import ValidationError +from rest_framework.exceptions import PermissionDenied, ValidationError from rest_framework.request import Request from rest_framework.response import Response from rest_framework.versioning import URLPathVersioning @@ -51,6 +51,47 @@ def get_serializer_class(self): return ToolStudioPromptListSerializer return ToolStudioPromptSerializer + def update(self, request: Request, *args: Any, **kwargs: Any) -> Response: + """Gate a reparent on the parent the prompt is being moved OUT of. + + ``PromptAcesssToUser`` admits org-shared members here, but + ``IsPromptParentToolOwner`` denies them ``destroy`` -- and that gate + reads the *stored* parent, which DRF loads before the update is + applied. So a writable ``tool_id`` let a denied user PATCH the prompt + into a tool they own and then delete it legitimately, in two requests + that each passed every check. + + Moving a prompt out of a project removes it from that project, which + is a deletion from the losing side, so it requires what ``destroy`` + requires -- checked against the EXISTING parent, not the new one. A + check against the new parent would pass trivially: the attacker's + destination is a tool they already own, and the harm is the prompt + leaving the original project regardless of where it lands. + + A no-op ``tool_id`` (unchanged) stays allowed, so the UI's field-level + PATCH is unaffected. An owner reparenting between tools they own stays + allowed. ``null`` is treated as a reparent, not a no-op: orphaning the + row hides it from everyone, its owner and org admins included, once + the org filter's INNER JOIN excludes it. + + This narrows the bypass; it does not make ``tool_id`` unwritable. The + complete fix is a read-only field on update, which changes what the + serializer accepts and was declined on API-contract grounds. + """ + instance = self.get_object() + if ToolStudioPromptKeys.TOOL_ID in request.data: + requested = request.data[ToolStudioPromptKeys.TOOL_ID] + current = instance.tool_id_id + if requested is None or str(requested) != str(current): + if not IsPromptParentToolOwner().has_object_permission( + request, self, instance + ): + raise PermissionDenied( + "Moving a prompt out of its project requires ownership " + "of that project." + ) + return super().update(request, *args, **kwargs) + def get_queryset(self) -> QuerySet | None: filter_args = FilterHelper.build_filter_args( self.request, From 4fdc528da5ca15e03cbace30a7ad6c65048889ed Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 08:20:25 +0530 Subject: [PATCH 16/18] UN-3315 [FIX] Scope prompt queries to the user's reachable tools get_queryset returned ToolStudioPrompt.objects.all() when unfiltered. list never calls get_object(), and neither permission class defines has_permission, so BasePermission's default True applied and nothing object-level fired: any org member could enumerate every prompt in the organization via GET /prompt/. OrganizationFilterBackend still blocked cross-org access, so the exposure was intra-org, and the list serializer limits it to prompt keys and sequence numbers -- but CustomTool.for_user deliberately hides those projects, and this route handed back their contents anyway. Both branches were open, not just the fallback: the filtered branch takes tool_id straight from the query string with no ownership check of its own, so scoping only the .all() path would have left the branch the UI actually uses exactly as it was. A test pins each. Scoped through the parent because ToolStudioPrompt has no organization field and no for_user manager -- the same route reorder_prompts already takes. This matches the sibling PromptStudioCoreView.get_queryset, which has always scoped with CustomTool.objects.for_user. Not a contract change: the response shape is identical and only the row set narrows. It is a deliberate tightening rather than a bug fix -- a caller listing prompts of a project they cannot reach stops seeing them. Note this also narrows get_object() for the detail routes, turning a 403 into a 404 for unreachable tools. That is an improvement (less enumeration) and does not weaken the deletion gate: for_user ORs in shared_to_org, so a shared member still reaches the prompt and still gets a real 403 from IsPromptParentToolOwner on destroy -- which the existing tests pin. Also corrects the get_permissions comment, which claimed reads honour project sharing. That was true for retrieve and false for list until this change; it now names both levels that enforce it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- .../prompt_studio/prompt_studio_v2/views.py | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index 1a45ea603b..231f13b961 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -38,10 +38,12 @@ class ToolStudioPromptView(viewsets.ModelViewSet): serializer_class = ToolStudioPromptSerializer def get_permissions(self) -> list[Any]: - # Reads and edits honour project sharing (UN-3315); deleting a prompt - # requires ownership of the parent tool, matching CustomToolViewSet, - # whose own `destroy` is IsOwner-gated. The bulk `sync_prompts` route - # there is IsOwner-gated too. API-key gap: see PromptAcesssToUser. + # Reads and edits honour project sharing (UN-3315), enforced on two + # levels: get_queryset scopes every action to the user's reachable + # tools, and these classes gate the object. Deleting a prompt requires + # ownership of the parent tool, matching CustomToolViewSet, whose own + # `destroy` is IsOwner-gated. The bulk `sync_prompts` route there is + # IsOwner-gated too. API-key gap: see PromptAcesssToUser. if self.action == "destroy": return [IsPromptParentToolOwner()] return [PromptAcesssToUser()] @@ -93,15 +95,26 @@ def update(self, request: Request, *args: Any, **kwargs: Any) -> Response: return super().update(request, *args, **kwargs) def get_queryset(self) -> QuerySet | None: + # Scope to tools the user can reach. `list` never calls get_object(), + # and neither permission class defines has_permission, so nothing + # object-level fires on it -- unscoped, any org member could enumerate + # every prompt in the organization. Both branches need this: the + # filtered one takes tool_id straight from the query string with no + # ownership check of its own, so it was equally open. + # + # ToolStudioPrompt has no organization field and no for_user manager, + # so the scoping goes through the parent, exactly as reorder_prompts + # does. for_user ORs in shared_to_org, so a shared member still + # reaches the prompt and still gets a real 403 (not a 404) from + # IsPromptParentToolOwner on destroy. + visible = ToolStudioPrompt.objects.filter( + tool_id__in=CustomTool.objects.for_user(self.request.user) + ) filter_args = FilterHelper.build_filter_args( self.request, ToolStudioPromptKeys.TOOL_ID, ) - if filter_args: - queryset = ToolStudioPrompt.objects.filter(**filter_args) - else: - queryset = ToolStudioPrompt.objects.all() - return queryset + return visible.filter(**filter_args) if filter_args else visible @action(detail=True, methods=["post"]) def reorder_prompts(self, request: Request) -> Response: From 6d0a5bb3807d8c02a25a560bf7f9d35183c5e002 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 08:20:53 +0530 Subject: [PATCH 17/18] UN-3315 [FIX] Stop prompt/reorder.json returning a 500 format_suffix_patterns generates a `.json` variant of every route in this URLconf, and DRF forwards the captured `format` kwarg to the handler. reorder_prompts took only `request`, so the suffixed route raised TypeError -- a 500 raised during dispatch, before the permission check was reached. Absorbs the kwarg in the signature, which is how the DRF mixins handle the same thing (they take *args, **kwargs). Dropping the route from format_suffix_patterns was the alternative, but that call wraps every pattern in the file, so it would have changed the URLconf for routes this PR has no business touching. Fixes a broken contract rather than changing a working one: the route returns 200 where it previously 500'd, and the unsuffixed route is unaffected. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- backend/prompt_studio/prompt_studio_v2/views.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index 231f13b961..90cbb006e1 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -117,9 +117,14 @@ def get_queryset(self) -> QuerySet | None: return visible.filter(**filter_args) if filter_args else visible @action(detail=True, methods=["post"]) - def reorder_prompts(self, request: Request) -> Response: + def reorder_prompts(self, request: Request, **kwargs: Any) -> Response: """Reorder the sequence of prompts based on the provided data. + ``**kwargs`` absorbs the ``format`` argument that + ``format_suffix_patterns`` passes on the ``prompt/reorder.json`` + variant of this route; the bare signature raised ``TypeError`` (500) + there. The DRF mixins take ``*args, **kwargs`` for the same reason. + Routed at the collection path (``prompt/reorder/``) with the target taken from ``prompt_id`` in the body, so DRF never calls ``get_object()`` and the viewset's permission class never fires. The From 4274af6794edf98ace2127d01ee8227a6d8d918f Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 08:20:53 +0530 Subject: [PATCH 18/18] UN-3315 [TEST] Pin the reparent gate, list scoping and the suffix route Fifteen tests across the three fixes, every one mutation-checked -- the fix reverted, the failure observed, the file verified restored: neuter the reparent gate (never deny) -> 2 failed treat null tool_id as a no-op -> 1 failed unscope get_queryset entirely -> 2 failed scope ONLY the unfiltered branch -> 1 failed remove **kwargs from reorder_prompts -> 1 failed Two are over-restriction guards rather than under-restriction ones, which is the half that is easy to omit: an owner must still be able to reparent, and an unchanged tool_id must stay a no-op so the UI's field-level PATCH keeps working. Both fail if the gate is widened to catch every update. The null case earns its own test. Treating {"tool_id": null} as "unchanged" looks reasonable and orphans the row, which the org filter's INNER JOIN then hides from everyone including org admins -- a delete by another name. The two list tests cover the branches separately on purpose: scoping the .all() fallback alone leaves the filtered branch -- the one the UI uses -- exactly as open as before, and a single test over both would not have caught that. Direct imports rather than tests_common.source_extraction. That technique execs method bodies out of context, so unreachable code is indistinguishable from wired code -- and "the hook never fires" is precisely the class of bug these fixes address. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE --- .../tests/test_prompt_permission_guards.py | 177 +++++++++++++++++- 1 file changed, 176 insertions(+), 1 deletion(-) diff --git a/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py b/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py index 4b6948fbb5..91045b139d 100644 --- a/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py +++ b/backend/prompt_studio/prompt_studio_v2/tests/test_prompt_permission_guards.py @@ -40,7 +40,7 @@ import pytest from django.http import Http404 -from rest_framework.exceptions import ValidationError +from rest_framework.exceptions import PermissionDenied, ValidationError from prompt_studio.permission import IsPromptParentToolOwner, PromptAcesssToUser from prompt_studio.prompt_studio_core_v2.views import PromptStudioCoreView @@ -361,3 +361,178 @@ def test_missing_prompt_id_is_a_400_not_a_crash(self) -> None: with pytest.raises(ValidationError): view.reorder_prompts(SimpleNamespace(user=SHARED_MEMBER, data={})) + + def test_format_suffix_route_does_not_crash(self) -> None: + """``prompt/reorder.json`` passes a ``format`` kwarg. + + ``format_suffix_patterns`` generates the suffixed variant and DRF + forwards the captured kwarg to the handler; the bare signature raised + TypeError -- a 500 before the permission check was ever reached. + """ + view = ToolStudioPromptView() + view.action = "reorder_prompts" + prompt = _prompt(_tool(owner=OWNER, shared_to_org=True)) + request = SimpleNamespace(user=SHARED_MEMBER, data={"prompt_id": "p-1"}) + module = "prompt_studio.prompt_studio_v2.views" + + with ( + patch(f"{module}.get_object_or_404", return_value=prompt), + patch(f"{module}.CustomTool"), + patch.object(ToolStudioPromptView, "check_object_permissions"), + patch(f"{module}.PromptStudioController") as controller, + ): + controller.return_value.reorder_prompts.return_value = "ok" + assert view.reorder_prompts(request, format="json") == "ok" + + +class TestPromptListIsScopedToReachableTools: + """``list`` never calls ``get_object()``, so only the queryset gates it.""" + + @staticmethod + def _queryset_for(request: Any, filter_args: dict[str, Any]) -> Any: + view = ToolStudioPromptView() + view.action = "list" + view.request = request + module = "prompt_studio.prompt_studio_v2.views" + + with ( + patch(f"{module}.ToolStudioPrompt") as prompt_model, + patch(f"{module}.CustomTool") as custom_tool, + patch(f"{module}.FilterHelper") as filter_helper, + ): + filter_helper.build_filter_args.return_value = filter_args + view.get_queryset() + + return prompt_model, custom_tool + + def test_unfiltered_list_is_scoped_to_the_users_tools(self) -> None: + """The enumeration hole: `.all()` exposed every prompt in the org.""" + request = SimpleNamespace(user=SHARED_MEMBER) + + prompt_model, custom_tool = self._queryset_for(request, {}) + + custom_tool.objects.for_user.assert_called_once_with(SHARED_MEMBER) + prompt_model.objects.filter.assert_called_once_with( + tool_id__in=custom_tool.objects.for_user.return_value + ) + ( + prompt_model.objects.all.assert_not_called(), + ( + "an unscoped .all() fallback lets any org member enumerate every " + "prompt in the organization" + ), + ) + + def test_filtered_list_is_scoped_too(self) -> None: + """``tool_id`` comes off the query string with no ownership check. + + Scoping only the fallback would leave the filtered branch -- the one + the UI actually uses -- just as open. + """ + request = SimpleNamespace(user=SHARED_MEMBER) + + prompt_model, custom_tool = self._queryset_for(request, {"tool_id": "other"}) + + prompt_model.objects.filter.assert_called_once_with( + tool_id__in=custom_tool.objects.for_user.return_value + ) + # The caller-supplied filter narrows the scoped set, never replaces it. + prompt_model.objects.filter.return_value.filter.assert_called_once_with( + tool_id="other" + ) + + +class TestReparentIsGatedOnTheOldParent: + """Moving a prompt OUT of a project needs what deleting it needs. + + ``PromptAcesssToUser`` admits org-shared members to update, but + ``IsPromptParentToolOwner`` denies them destroy -- and that gate reads the + STORED parent, loaded before the update applies. A writable ``tool_id`` + therefore let a denied user reparent into a tool they own and then delete + legitimately, two requests each passing every check. + + The check is against the EXISTING parent. Checking the new one passes + trivially: the attacker's destination is a tool they already own, and the + harm is the prompt leaving the original project regardless of where it + lands. + """ + + MODULE = "prompt_studio.prompt_studio_v2.views" + + def _update(self, *, stored: str, payload: dict, owner_allows: bool) -> Any: + view = ToolStudioPromptView() + view.action = "partial_update" + instance = SimpleNamespace(tool_id_id=stored) + request = SimpleNamespace(user=SHARED_MEMBER, data=payload) + + with ( + patch.object(ToolStudioPromptView, "get_object", return_value=instance), + patch(f"{self.MODULE}.IsPromptParentToolOwner") as gate, + patch( + "rest_framework.viewsets.ModelViewSet.update", return_value="updated" + ) as parent_update, + ): + gate.return_value.has_object_permission.return_value = owner_allows + try: + result = view.update(request) + except PermissionDenied: + return "denied", parent_update + return result, parent_update + + def test_shared_member_cannot_reparent_out(self) -> None: + """The bypass: step one of reparent-then-delete is now refused.""" + result, parent_update = self._update( + stored="tool-A", payload={"tool_id": "tool-B"}, owner_allows=False + ) + + assert result == "denied", ( + "moving a prompt out of a project is a deletion from that " + "project's side and must require what destroy requires" + ) + parent_update.assert_not_called(), "the move must not be applied" + + def test_owner_may_reparent(self) -> None: + """The over-restriction guard: a legitimate move still works.""" + result, parent_update = self._update( + stored="tool-A", payload={"tool_id": "tool-B"}, owner_allows=True + ) + + assert result == "updated" + parent_update.assert_called_once() + + def test_same_tool_is_a_no_op_not_a_reparent(self) -> None: + """The UI PATCHes single fields; an unchanged tool_id must pass. + + Denies at the gate to prove the no-op path never consults it. + """ + result, parent_update = self._update( + stored="tool-A", payload={"tool_id": "tool-A"}, owner_allows=False + ) + + assert result == "updated", ( + "an unchanged tool_id is not a reparent -- gating it would break " + "every field-level PATCH that echoes the parent back" + ) + parent_update.assert_called_once() + + def test_ordinary_field_edit_is_untouched(self) -> None: + """A payload with no tool_id never reaches the gate at all.""" + result, parent_update = self._update( + stored="tool-A", payload={"prompt_key": "k"}, owner_allows=False + ) + + assert result == "updated" + parent_update.assert_called_once() + + def test_null_tool_id_is_a_reparent(self) -> None: + """Orphaning is not a no-op. + + ``{"tool_id": null}`` hides the row from everyone once the org + filter's INNER JOIN excludes it -- its owner and org admins included. + """ + result, parent_update = self._update( + stored="tool-A", payload={"tool_id": None}, owner_allows=False + ) + + assert result == "denied", "null must be treated as a move, not a no-op" + parent_update.assert_not_called()