UN-2868 [FIX] Make settings read-only and block deletion on resources shared with a user

[kirtimanmishrazipstack]

What

Shared access now means different things depending on the resource:

• Prompt Studio and Agentic Prompt Studio are shared for collaboration — prompts, Settings and LLM profiles are all editable by a shared user.
• Everything else — workflows, ETL/Task pipelines, API deployments, adapters, connectors, Look-Ups — is shared for use: open, run, pass access on, but do not reconfigure.
• Renaming and deleting stay with the owner everywhere. Sharing onward stays open; only the owner revokes access or shares org-wide.
• Locked controls are greyed out with a reason on hover, not hidden.
• Two unrelated fixes: a shared user saw a project's internal ID instead of its name, and the workflow rename dialog opened empty.

Why

Reported by a customer — a shared user could repoint another team's workflow at a different output folder. And where the server did refuse a write, the screen said nothing, so people filled in a form and lost the work.

The first attempt applied read-and-run-only everywhere. That was wrong for Prompt Studio: prompts, preamble, grammar and the LLM profile all go together on every run, so granting one and withholding the rest describes a job nobody does. Hence the split by what the resource is for.

How

Resource	A shared user can	Owner / co-owner only
Prompt Studio	Add, edit, reorder, delete prompts. Change every Settings tab. Create, edit, delete LLM profiles and set the default. Toggle SinglePass. Run and view output.	Rename, delete
Agentic Prompt Studio	Change project settings (LLM, agent LLM, lightweight LLM, text-extractor). Work on documents and prompts. Run.	Rename, delete
Lookup Studio	View and use it inside a shared Prompt Studio project. Now reachable via group sharing, which was missing.	Rename, delete
Workflows	Open, run, view runs.	Connector config, DB/tool/HITL settings, rename, delete
ETL / Task pipelines	Open, view runs and logs.	Enable toggle, cron, connector config, rename, delete
API deployments	Open, view and copy existing keys.	Enable toggle, add a key, a key's active toggle, rename, delete
Adapters	Use it in an LLM profile or workflow.	Edit, delete
Connectors	Use it in a workflow.	Edit, delete

Can this PR break any existing features. If yes, please list possible items. If no, please explain why.

• Shared users lose configuration access outside Prompt Studio — connector and tool settings, HITL, status toggles, key creation. These were writes to someone else's resource; this is the ticket's product decision.
• Owners see one change — the connector window's Save now also saves HITL rules and the separate "Save Rules" button is gone (cloud PR). Nothing writes until Save.
• Prompt Studio is unchanged for shared users — only rename and delete moved.
• Reading, running, sharing onward, service accounts and API keys are all untouched.

Database Migrations

• None.

Env Config

• None.

Relevant Docs

• None.

Related Issues or PRs

• Cloud PR UN-1751 [FIX] Resolved SharePoint ETL sync failures causing incomplete data extraction #1751 — must merge after this one. It imports WorkflowOwnerMutationMixin, canEditResource and ReadOnlyNotice from here.

Dependencies Versions

• None.

Notes on Testing

Surface	Shared user	Owner / admin
Prompt Studio — add, edit, reorder, delete a prompt	available	unchanged
Prompt Studio — Settings modal, all tabs	available	unchanged
Prompt Studio — create, edit, delete an LLM profile; set default	available	unchanged
Prompt Studio — rename	blocked (400)	saves
Prompt Studio — delete	greyed out	unchanged
Prompt Studio — profile create by a non-shared org member	blocked (403)	—
Workflow connector config — read / change	200 / blocked (403)	200 / 200
Workflow connector config, not shared at all	blocked (404)	—
Workflow tool settings	read-only notice, no Save	unchanged
Rename from a resource list / builder header	greyed out	opens pre-filled, saves
Adapter, connector settings	greyed out	unchanged
ETL pipeline / API deployment status toggle	greyed out	unchanged
Manage Keys — New Key, per-key active toggle	greyed out	unchanged
Manage Keys — view and copy a key	available	unchanged
Delete / Share, on any resource	greyed out / available	unchanged
PS project name shown in a shared workflow	name, not ID	unchanged

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

[greptile-apps]

via Greptile

Confidence Score: 5/5

The PR appears safe to merge.

Summary

• Adds workflow-parent mutation checks, anti-reparenting validation, and access-scoped endpoint handling.
• Keeps shared Prompt Studio prompts, settings, and profiles collaborative while restricting rename and deletion.
• Disables owner-only workflow, pipeline, deployment, connector, and key-management controls for shared users with read-only feedback.
• Corrects shared workflow project-name display and workflow rename initialization.
• Makes combined connector and HITL saves stop after endpoint failures and report a single outcome.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Share[Shared resource access] --> Kind{Resource type}
  Kind -->|Prompt Studio| Collaborate[Collaborative settings and content edits]
  Kind -->|Workflow or operational resource| UseOnly[Read, run, and use]
  Collaborate --> OwnerOps[Rename and delete remain owner-only]
  UseOnly --> Locked[Configuration and lifecycle controls are read-only]
  Locked --> Backend[Backend mutation checks]
  Locked --> Frontend[Disabled controls with explanation]

Loading

_{Reviews (15) · Last reviewed commit: "UN-2868 [FIX] Drop the focusable tooltip..."}

[kirtimanmishrazipstack]

@greptile-apps Both findings fixed in 06198cc, plus three more found while tracing the second one.

Partial save / false close — valid, and the sharper of the two. handleValidateAndSubmit caught its own error and returned, so a failed endpoint write fell straight through to the HITL write and handleSaveAndClose dismissed the modal. Both writes now report success and handleSave bails on the first failure:

if (!(await handleValidateAndSubmit(formDataConfig))) {
  return false;
}

Create guard fails open — fixed, replied inline.

Found alongside these:

• The HITL write now only runs when its form is actually dirty. Chaining it unconditionally meant every connector save also wrote a rule row.
• A view-only user could be asked whether to save unsaved changes on close, which they cannot have made. hasUnsavedChanges() returns false when the workflow is not editable.
• ReadOnlyNotice used defaultProps, which React 19 ignores on function components, so the default message never applied. Now a default parameter.

The companion cloud PR (#1751) also fixes the dirty flag that made this reachable: the HITL form was dirty from the moment it opened, because the baseline was either null or the server's raw rule_json compared against a builder-normalised round-trip. Sync destination and TTL are folded into that flag too, since they lost their own Save button.

Re-verified the permission matrix against real shared-workflow data after the change — owner and admin-not-owner write, shared viewer 403, non-shared 404, reads unaffected on both database and API destinations.

[greptile-apps]

Thanks for the follow-up. The partial-save/false-close path and the create permission guard are addressed, along with the HITL dirty-write, view-only close prompt, and React 19 ReadOnlyNotice default issues. The companion cloud PR covers the remaining dirty-state normalization and destination/TTL tracking, and the permission matrix was re-verified across both destination types. No further concerns from me.

[kirtimanmishrazipstack]

@greptile-apps review

Both findings from the previous pass are addressed in 06198cc, which landed after that review was generated.

[kirtimanmishrazipstack]

@greptile-apps please review again.

[kirtimanmishrazipstack]

@greptile-apps Requesting a re-review. One clarification first, on the part of this PR most likely to read as an over-block.

Blocking the workflow destination settings for shared users is intentional, and it deliberately includes HITL.

The destination connector modal carries three things behind a single Save: the connector configuration (DB table, credentials), the HITL review rules and percentage, and the sync destination plus TTL. All three configure the same workflow endpoint — HITL is not a separate review surface bolted onto a settings screen, it is part of how that workflow writes its output.

So the whole modal is owner-only on a shared workflow, and the three viewsets are gated identically:

• WorkflowEndpointViewSet (OSS) uses WorkflowOwnerMutationMixin
• RuleEngineViewSet and SettingsViewSet (cloud manual_review_v2/views.py:2831,2980) use the same mixin

Gating the connector half and leaving the HITL half open would have been the inconsistency.

What a shared user keeps: opening the modal and reading every value, and running the workflow. What they lose: repointing someone else's pipeline at a different table, and changing what fraction of its output goes to human review.

This is intentionally different from Prompt Studio in the same PR. A Prompt Studio project is shared for collaboration, so prompts, settings and LLM profiles stay editable. A workflow is shared for use. The full per-resource split is the table in the How section of the description. On every resource, renaming, deleting and deciding who else gets access stay with the owner.

Both directions of this were flagged earlier in the review and then withdrawn once the intent was clear, so flagging it again either way is expected to be a false positive.

[chandrasekharan-zipstack]

Standardized PR Review — UN-2868 (OSS half)

Verdict: BLOCK · Mode: INITIAL · rubric: unstract plugin v0.18.1 (16 lenses)

Summary — Critical: 2 · High: 11 · Medium: 12 · Low: 5 · Lenses run: 16/16

Posting Critical and High only, as requested. Medium/Low findings are held back and can be posted on request.

This PR is a real improvement on the status quo. It is blocked because the new gate ships alongside a docstring asserting an invariant the code does not hold — which is the specific failure mode that makes a permission change dangerous, since the next reader stops re-checking.

Reviewed as if this PR merges first; the cloud companion (Zipstack/unstract-cloud#1751) imports WorkflowOwnerMutationMixin and canEditResource from here. Cross-repo claims were verified against both PR trees, not against main.

---

Unanchored findings

These sit outside this PR's diff, so GitHub has no line to attach them to. They are included because the PR's stated contract ("backend enforces; frontend gates") depends on them.

[Critical] [Lens 4] — tool_instance create is ungated; the new Agency lock is frontend-only.
backend/tool_instance_v2/views.py:113-118 gates only update/partial_update/destroy. ToolInstanceSerializer.create (backend/tool_instance_v2/serializers.py:257-260) resolves Workflow.objects.get(pk=workflow_id) with no user scoping, and the same path flips workflow.is_active = True on that foreign workflow. Any authenticated org member can POST a tool onto a workflow they do not own. This PR disables the corresponding UI control at Agency.jsx:1166/:1176, so the lock is presentation-only here.
Pre-existing, not introduced by this PR. Fix: inherit WorkflowOwnerMutationMixin on ToolInstanceViewSet (deleting its duplicate get_permissions) and scope the workflow lookup through for_user.

[High] [Lens 4] — File-history deletion is still open to read-only shared users.
backend/workflow_manager/workflow_v2/file_history_views.py:26 uses IsWorkflowOwnerOrShared, which admits _is_resource_viewer and shared_to_org. Both destroy (:85) and clear (:99) are destructive. Separately, clear_file_marker is not in the IsOwner() action list (workflow_manager/workflow_v2/views.py:91-98), so it falls through to IsOwnerOrSharedUserOrSharedToOrg — and it is exposed as a GET (views.py:355), making it reachable by prefetch or a pasted URL with no CSRF in the way. The PR title says "block deletion on resources shared with a user"; these are the most destructive per-workflow deletion paths and they are not blocked.
Fix: gate both on is_workflow_mutator (already exported at permissions/permission.py:121), and change clear_file_marker to POST.

[High] [Lens 1] — The workflow builder's connector-type dropdown was left ungated and now fails.
frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx:238-251 is disabled={!allowChangeEndpoint} only — never canEdit. It fires handleEndpointUpdate straight at the endpoint this PR newly gates, so a shared user picks a connector type and gets a bare "Failed to update" toast. The ConfigureConnectorModal behind it was gated; its launcher was not. useWorkflowCanEdit is imported in four files but not this one.

[High] [Lens 13] — Zero test files across 24 changed files, on a diff that is almost entirely authorization logic.
ParentToolAccess widens ProfileManager writes to shared viewers, WorkflowOwnerMutationMixin adds a new collection-level create gate, and rename enforcement moved from a permission class into serializer field validation. None of it is exercised. backend/CLAUDE.md states tests are required after implementation, and the harness already exists at backend/permissions/tests/test_owner_management.py. Highest-value cases: shared viewer denied on endpoint update/destroy; shared viewer allowed on ProfileManager create/update/destroy; shared viewer denied on a tool_name PATCH but allowed on any other settings field.

---

Generated by the standardized 16-lens review. Verified findings only — 11 agent claims were dropped as false after checking both PR trees.

[chandrasekharan-zipstack]

[Critical] [Lens 4 — Security] — workflow_endpoint_list has no permission class and never calls get_object(); any org member reads any workflow's connector credentials.

This new docstring states "Shared users may read it", which reads as a class-wide invariant. It holds for update/partial_update/destroy and not for the list action.

workflow_endpoint_list (views.py:88-101, routed at workflow_manager/urls.py:14) calls WorkflowEndpointUtils.get_endpoints_for_workflow(pk) directly, bypassing both get_queryset() — the only access filter on this viewset — and object permissions. WorkflowHelper.get_workflow_by_id is a bare Workflow.objects.get(pk=id) with no scoping, and DEFAULT_PERMISSION_CLASSES is [] (backend/backend/settings/base.py:640), so get_permissions returning list(super().get_permissions()) for this action yields nothing.

The response serializes through ConnectorInstanceSerializer, whose to_representation pops only CONNECTOR_AUTH and blanks connector_metadata only for UnstractCloudStorage (connector_v2/serializers.py:162-176). S3/GCS/DB connector_metadata is returned decrypted to a user with no relationship to the workflow.

Failure mode: GET /api/v1/unstract/<org>/workflow/<any-uuid>/endpoint/ — horizontal privilege escalation inside an org. No error is raised or logged, because the authorization that should have refused is simply never evaluated.

Pre-existing, but this PR is what adds the docstring asserting the opposite.

Fix:

workflow = get_object_or_404(Workflow.objects.for_user(request.user), pk=pk)
endpoints = WorkflowEndpointUtils.get_endpoints_for_workflow(workflow.id)

Confidence: High (verified against the PR tree).

[kirtimanmishrazipstack]

@chandrasekharan-zipstack Fixed in 44a06b9, with a follow-up in 28cfde6 — and one half deliberately left open for your call.

workflow_endpoint_list now resolves through Workflow.objects.for_user() before serialising, so an unrelated org member gets a 404 instead of the workflow's connector credentials.

Chasing it turned up two more paths to the same credentials:

• A second route to them. WorkflowEndpointSerializer.connector_instance_id is a writable PrimaryKeyRelatedField whose queryset was ConnectorInstance.objects.all() — org-scoped, not user-scoped, unlike every sibling. Any member could create their own workflow (becoming its owner, so IsParentWorkflowOwner passes), PATCH their endpoint onto a colleague's connector, and read the decrypted metadata out of the PATCH response. Scoped with for_user in 28cfde6.
• WorkflowEndpointAPIView logged dict(request.headers) at INFO — including the internal service bearer key that authenticates it, on the view that returns decrypted connector metadata for any workflow in any organization. No redaction filter exists anywhere. Dropped the headers entry and lowered the line to debug.

Still open — your decision. Scoping bounds who reaches the workflow, not what the response contains. A user the workflow is shared with (directly, by group, or org-wide) still receives the owner's decrypted S3/GCS/Postgres credentials, because the nested ConnectorInstanceSerializer decrypts everything except UnstractCloudStorage. Redacting it changes what the read-only connector view shows shared users, so I didn't decide that alone. I did narrow the two comments that implied the exposure was closed.

[chandrasekharan-zipstack]

[High] [Lens 4 — Security] — The gate validates the current parent, never the target parent, and workflow is writable.

IsParentWorkflowOwner.has_object_permission resolves ownership from obj.workflow — the parent the row has now. WorkflowEndpointSerializer is fields = "__all__" with only workflow_name marked read-only, so workflow itself is a writable FK.

Failure mode: the owner of workflow A sends PATCH /workflow/endpoint/<A-endpoint>/ with {"workflow": "<B-id>"}. The object check passes (they do own A), then the save reassigns the row to B, owned by a different user in the same org. B's owner now executes against A's connector instance, bucket, or table.

That is the customer-reported bug — a shared user repointing another team's workflow at a different output folder — still reachable after the fix that targets it. The same shape applies to the cloud companion: RuleEngineSerializer.read_only_fields is ["id", "created_by", "modified_by"] and SettingsSerializer is fields = "__all__", so both HITL viewsets inherit it.

Fix: mark workflow read-only on WorkflowEndpointSerializer (and on the two cloud HITL serializers), or validate in the mixin that validated_data.get("workflow") equals instance.workflow on update.

Two notes in this class's favour, verified while checking: perform_create correctly fails closed on a missing workflow, and the MRO order is right at every current call site. The base-order fragility (a bare mixin overriding get_permissions/perform_create, which DRF also defines) is a trap for the next viewset that copies it, not a live bug — worth an __init_subclass__ assertion given this is now a cross-repo contract.

Confidence: High on the field config (verified); the reassignment PATCH was not executed against a live DB.

[kirtimanmishrazipstack]

@chandrasekharan-zipstack Half of this held and half didn't — and the half that held is now closed at eight sites, not one.

The OSS half is disconfirmed. WorkflowEndpointSerializer.workflow is not writable: WorkflowEndpoint.workflow is editable=False (models.py:34), which DRF honours by forcing read_only=True. I checked by building the serializer rather than reading Meta — the only writable fields are configuration, connection_type and connector_instance_id. So the reparenting PATCH you describe can't land there. You'd flagged your own uncertainty on exactly this ("the reassignment PATCH was not executed against a live DB"); the field-config half is the part that was off.

The cloud half was live and is fixed (1d9c4f6a). Both RuleEngineSerializer.workflow and SettingsSerializer.workflow built read_only=False. One correction worth recording: only SettingsSerializer was actually exploitable — RuleEngineSerializer has a custom update() that assigns only percentage/rule_string/rule_json/rule_logic and never touches workflow. The guard there is defence-in-depth, not a closed hole, and I've said so in the commit so nobody removes it later believing it was load-bearing.

The class is wider than either of us listed. Sweeping by gate rather than by field found eight sites: RuleEngine, HITLSettings, ToolInstance, ProfileManager, APIKey (api and pipeline), ToolStudioPrompt, and WorkflowEndpoint — the last safe only by that editable=False. All closed across 1d9c4f6a / 28cfde6 / ac31510 / c67c55c.

Two of those are worth your attention:

• APIKeySerializer.api/.pipeline were writable on PATCH. create spends ~60 lines refusing a path-vs-body mismatch; update had nothing, so a live key could be retargeted onto a deployment the caller doesn't own.
• The eighth site was an alias of a field I'd already guarded in the same file. ToolInstance.workflow's attname is workflow_id, and the serializer declares a writable workflow_id alias. validate_workflow fires only on the key workflow; a body of {"workflow_id": ...} skipped it entirely and DRF's setattr(instance, "workflow_id", uuid) wrote the FK column directly.

That last one is the argument for your __init_subclass__ suggestion, or better: hoisting the check into WorkflowOwnerMutationMixin so it covers attname aliases. Enumerating this class by field name provably cannot close it. I did not do that refactor — it touches eight viewsets across both repos and I didn't want it unreviewed at the end of an automated run. Recommended in the commit message.

[chandrasekharan-zipstack]

[High] [Lens 1 — Spec & intent] — Every "hover for the reason" tooltip added on a disabled control never renders.

The PR's stated UX is "locked controls greyed out with a reason on hover, not hidden." The shim Tooltip renders <TooltipTrigger asChild> (Radix) and depends on pointer/focus events reaching the child. The shim Button carries disabled:pointer-events-none — asserted by this repo's own test at frontend/src/components/ui/cascade-and-affordances.test.jsx:194-199 — and a native disabled button or switch swallows mouseenter. The tooltip content never mounts.

Affected, all added by this PR:

• ToolNavBar.jsx:85-101 (this hunk)
• CardFieldComponents.jsx:73-81 and :113-121
• ManageKeys.jsx:264-272 and :309-317
• ApiDeploymentCardConfig.jsx:99-113
• PipelineCardConfig.jsx:327-341

User impact: 6 of the 8 new lock affordances communicate nothing — a dead grey control with no explanation, which is the "the screen said nothing" outcome this PR exists to eliminate. ResourceTable.jsx:302-315 works only because it uses aria-disabled on a raw <button> rather than disabled, so the same feature behaves two different ways.

The codebase already documents this exact trap and its fix at frontend/src/components/logging/log-modal/LogModal.jsx:262-267:

"span wrapper — antd disabled Buttons swallow pointer events so a direct Tooltip child never receives mouseenter and the disabled-state tooltip never shows."

Fix: wrap each disabled child in a <span> inside the Tooltip, or fix it once in the shim — when title is set and the child has disabled, render the trigger wrapped.

Confidence: High (mechanism confirmed by the repo's own test and precedent comment; UI not run).

[kirtimanmishrazipstack]

@chandrasekharan-zipstack Fixed in d592324, then corrected in 81f2cea — your diagnosis was right and my first fix was incomplete.

Fixed once in the shim rather than at six call sites: Tooltip now wraps a disabled child so pointer events land on the wrapper.

The correction matters. A span alone does not restore hover — the browser suppresses mouse events on a disabled control and doesn't bubble them, so the span only becomes the hit target once the child stops hit-testing. ui/button.tsx sets disabled:pointer-events-none and worked; switch, checkbox, input, label, radio-group, select and textarea set only cursor-not-allowed and did not. That includes the live case this PR is about — the owner-only Switch in ManageKeys.jsx:264. So my first pass silently helped only the Buttons. The wrapper now neutralises the child itself, and carries tabIndex so the tooltip is reachable by keyboard at all, which a disabled child never is.

One correction to the finding: ResourceTable's own action buttons use aria-disabled, not disabled, and aria-disabled elements receive pointer events normally — so those two tooltips already worked. The beneficiaries are the Button/Switch call sites.

[chandrasekharan-zipstack]

[High] [Lens 13 — Testing / Lens 4 — Security] — This swap leaves a test asserting the inverse of production, still passing.

Switching to ParentToolAccess widens ProfileManager create/destroy/partial_update/update from owner-only to any VIEWER-role member (prompt_studio/permission.py:24, via _can_access_tool). That is the intended collaborative rule for Prompt Studio — no objection to the behaviour.

The problem is what it leaves behind. After this diff IsParentToolOwner has zero production callers (grep -rn IsParentToolOwner backend --include='*.py' returns only its definition, one test, and two docstrings), but backend/permissions/tests/test_owner_management.py:326-369 still constructs IsParentToolOwner() directly and asserts assertFalse(self._perm(self.viewer, child)) at :359-362.

Failure mode: CI reports green on ProfileManager mutation authz while the shipped endpoint returns 200 for exactly that user. The live gate, ParentToolAccess, is untested in both directions — so any future re-tightening or further loosening is invisible to the suite. A dead authz class that still has a passing test suite is also a trap for the next person who imports it by name; prompt_studio/permission.py:86 still points readers at it as the pattern to mirror.

Fix: retarget IsParentToolOwnerTests at ParentToolAccess (flip viewer to assertTrue, keep outsider assertFalse, add a service-account case) and delete IsParentToolOwner.

Confidence: High.

[kirtimanmishrazipstack]

@chandrasekharan-zipstack Not fixed — blocked by a project rule, and I'd like your explicit go.

The finding is correct: after this diff IsParentToolOwner has zero production callers, while test_owner_management.py:326-369 still constructs it directly and asserts assertFalse(self._perm(self.viewer, child)). CI is green on ProfileManager mutation authz while the shipped endpoint returns 200 for exactly that user, and the live gate ParentToolAccess is untested in both directions.

It can't be half-done: deleting the dead class breaks the test that still imports it, so the fix necessarily edits a test. CLAUDE.md says "Never write, modify, or suggest tests unless the user explicitly uses the word 'test.'" — so I stopped rather than work around it.

Say the word and it's one pass: retarget IsParentToolOwnerTests at ParentToolAccess (viewer flips to assertTrue, outsider stays assertFalse, add a service-account case) and delete the class.

Same blocker applies to test coverage for the four behaviour changes in this batch — the refusal paths (404 on a non-member endpoint list, the reparent refusals, 403-not-400 on rename, the delete-button state) have no assertions anywhere.

[chandrasekharan-zipstack]

[High] [Lens 7 — API contract / Lens 4 — Security] — An authorization decision expressed as field validation: wrong status, wrong shape, and bypassable.

raise ValidationError("Only the owner can rename this project.") renders as 400 {"tool_name": ["Only the owner can rename this project."]}. The sibling decision for the same resource — delete — goes through IsOwner (views.py:160) and returns 403. Same actor, same resource, same predicate, two status codes and two response shapes.

Three consequences:

1. Clients cannot distinguish "that name is invalid" from "you are not allowed." Any form-error handler paints an authz refusal under the name input as though retyping would fix it.
2. The rule is invisible to permission introspection — view.get_permissions() reports rename as ungated, so the browsable API, schema generation, and any permission-walking test all read it as open.
3. The guard is conditional on payload shape. DRF runs a field validator only when the field is present and only through this serializer. Any sibling/import serializer that writes tool_name, a future .save(tool_name=…) helper, or a refactor folding this into validate_name_field silently drops the check.

Also worth deciding deliberately: this serializer is fields = "__all__", and the gate covers exactly one field. description, icon and author remain writable by any shared user, which the comment at views.py:155-156 does not acknowledge.

Fix: raise rest_framework.exceptions.PermissionDenied(...) — same placement, correct status, and it reads as authorization to anyone scanning the file. The cloud companion has the identical issue at agentic_studio_v1/serializers.py:188.

Confidence: High.

[kirtimanmishrazipstack]

@chandrasekharan-zipstack Fixed in 44a06b9, both repos.

PermissionDenied instead of ValidationError, so the rename refusal returns 403 with the same shape as its sibling destroy. Verified against the installed DRF 3.17.1 and drf-standardized-errors 0.15.0 rather than from memory: to_internal_value catches only ValidationError/DjangoValidationError, so PermissionDenied propagates out of is_valid() and renders 403 — and middleware/exception.py's only rewrite is gated on 404, so it passes through unmodified.

Your point 1 was the concrete one: AddCustomToolFormModal.jsx:132 does getBackendErrorDetail("tool_name", backendErrors), so the authz refusal was being painted under the name input as though retyping would fix it. That's gone. Checked the blast radius too — the axios interceptor logs out on 401 only, useExceptionHandler has a client_error branch that surfaces errors[0].detail, and the rename submit passes no setBackendErrors, so nothing depended on the old attr: "tool_name" binding.

Cloud twin at agentic_studio_v1/serializers.py fixed the same way in 1d9c4f6a.

Your closing note — that the gate covers one field while description, icon and author stay writable by any shared user — is untouched and still stands as a deliberate decision to make.

[chandrasekharan-zipstack]

[High] [Lens 3 — Correctness] — canEditResource fails open, and cannot distinguish "still loading" from "this serializer does not emit the field".

is_owner === undefined → true. The comment justifies this as the in-flight-request window, but the predicate has no way to tell that apart from a payload that simply lacks the key.

Two concrete exposures:

• APIDeploymentSerializer — the non-list class returned for retrieve/create/update (api_v2/api_deployment_views.py:305-308) — does not carry is_owner. Any caller that seeds a store or table row from a non-list response unlocks every Edit/Delete/toggle on it.
• Store defaults are details: {} (store/workflow-store.js:12, store/custom-tool-store.js:17), which also takes the fail-open branch.

Worse, the two ends of this contract fail in opposite directions. Every producer emits is_owner: false when self.context.get("request") is missing (prompt_studio_core_v2/serializers.py:123, workflow_v2/serializers.py:81), so a serializer constructed without context silently demotes the resource's owner to read-only — and there are already two such call sites in tree (workflow_v2/views.py:347, endpoint_v2/views.py:101). One optional boolean is carrying three states (absent / false-because-no-context / false-because-genuinely-not-owner) with two collapsed and the third inverted at the boundary.

Failure mode: a serializer regression that drops is_owner produces no error, no warning, and no visible symptom — the UI just stops enforcing, silently restoring the pre-PR behaviour this change exists to prevent.

Fix: take an explicit loading argument rather than overloading undefined, fail closed on a fetched resource with no is_owner, and warn once so the gap is visible. On the producer side, either omit the key entirely when there is no request (so absent uniformly means unknown) or assert — a serializer emitting an authorization flag with no requester is always a bug.

Confidence: High that the branch exists; Medium that a live call site reaches it today.

[kirtimanmishrazipstack]

@chandrasekharan-zipstack Partly fixed in 28cfde6. The concrete exposure is closed; the contract itself is not, and that's deliberate.

Closed: APIDeploymentSerializer — the class returned for retrieve/create/update — now carries is_owner, so a consumer seeding a store from a non-list response no longer takes the fail-open branch.

Not changed: is_owner === undefined → true itself. Your fix (an explicit loading argument, fail closed on a fetched resource, warn once) is the right shape, but it inverts the default for eight call sites and both store defaults — details: {} in workflow-store.js and custom-tool-store.js both currently take the fail-open branch. Flipping that at the end of an automated run, with no test coverage on any of those call sites, is how an owner ends up locked out of their own resource. It wants doing deliberately with the tests to back it.

Your observation that the two ends fail in opposite directions is the part I'd most want preserved in whatever we do: producers emit is_owner: false when self.context.get("request") is missing, so a serializer built without context demotes the resource's own owner — and there are live no-context call sites. One optional boolean carrying three states, two collapsed and the third inverted at the boundary.

[chandrasekharan-zipstack]

[High] [Lens 3 — Correctness] — Frictionless adapters become undeletable, and the tooltip gives a reason that is false for them.

The new gate is canEditResource(item, sessionDetails) — i.e. is_owner || isAdmin. But the backend deliberately lets any org member delete a frictionless adapter: AdapterInstanceViewSet.get_permissions routes destroy to IsFrictionLessAdapterDelete, which returns True unconditionally when obj.is_friction_less (adapter_processor_v2/views.py:171-172, permissions/permission.py:304-312).

Those adapters are visible to every member (adapter_processor_v2/models.py:54-59, Q(is_friction_less=True)) and their owner is masked as "Unstract" in the payload (adapter_processor_v2/serializers.py:194-196), so is_owner is False for essentially everyone. AdapterListSerializer does not expose is_friction_less at all (serializers.py:165-176), so the frontend cannot special-case these rows even if it wanted to.

User impact: the only UI route to remove platform-provisioned onboarding adapters disappears for every non-admin, and the tooltip asserts an ownership rule that does not govern the row.

Fix: add is_friction_less to AdapterListSerializer.Meta.fields and gate delete on canEdit || item?.is_friction_less for adapters. Keep Edit disabled — IsFrictionLessAdapter correctly refuses update/retrieve on those rows for everyone.

Confidence: High.

[kirtimanmishrazipstack]

@chandrasekharan-zipstack Fixed in 44a06b9, exactly as you specified.

is_friction_less added to AdapterListSerializer.Meta.fields, and the table's delete gate is now locked && !item?.is_friction_less. Edit stays locked — IsFrictionLessAdapter correctly refuses update/retrieve on those rows for everyone.

Checked the leak question before adding the field: AdapterListSerializer carries no adapter_metadata, and its to_representation already masks created_by_email/owner_emails to "Unstract" for frictionless rows, so the flag was already inferable from the response.

Also traced the four cases so the carve-out can't accidentally unlock anything else: is_owner absent → unchanged; is_owner:false on a non-adapter row (is_friction_less undefined) → still locked; ordinary adapter → still locked; frictionless → unlocked. Workflows, ListOfTools and ConnectorsPage rows are unaffected.

One thing I'd flag back: a later review pass argued this puts an adapter-specific field inside a table that also backs Workflows, Prompt Studio and Tool Settings, and that a canDelete predicate passed by the adapters page would keep the knowledge where it belongs. I left it as you wrote it — worth a view if you disagree.

[chandrasekharan-zipstack]

[High] [Lens 16 — Doc accuracy] — This docstring states an owner-only rule the code does not enforce, on the sharing surface.

The comment asserts that "who it is shared with" stays with the owner. It does not. The share action is not in the IsOwner() list in prompt_studio_core_v2/views.py:159 (which gates only destroy, add_co_owner, remove_co_owner), so it falls through to IsOwnerOrSharedUserOrSharedToOrg, and ShareAuthorizationService explicitly permits non-owners to add — "direct shared users and group members can add (with scope limits) but not remove" (tenant_account_v2/sharing_helpers.py:381-389).

This PR ships both statements of the rule. The frontend comments added in the same diff say the opposite and are correct: CardFieldComponents.jsx:82-83 ("Sharing stays open to shared users: they may pass access on to a group they belong to, or to a user in the same organisation") and ResourceTable.jsx:291.

A maintainer auditing the share surface from this docstring will believe an owner-only gate exists where none does — the over-grant direction.

Fix: reword to match ShareAuthorizationService — only the project's existence (destroy) and co-ownership are owner-only; shared users may add viewers/groups but not remove them.

Two related accuracy issues in this file, same lens:

• :19-20 says the rename is "gated on the project viewset". It is not — this PR deliberately put it in the serializer (prompt_studio_core_v2/serializers.py:136-138 says so explicitly). Someone verifying "is rename protected?" by reading the viewset finds nothing.
• :62 says # Imported here: the models pull in this module at import time. No model imports prompt_studio.permission — only three view modules and one test do. The stated reason for the non-obvious deferred import is false, so the next engineer either hoists it on a bad premise or preserves a workaround nobody can justify.

Confidence: High.

[kirtimanmishrazipstack]

@chandrasekharan-zipstack All three fixed in 0c9fb44.

• The sharing claim is reworded to match ShareAuthorizationService: renaming, deleting and removing access stay with the owner; sharing onward does not.
• The rename line now points at CustomToolSerializer.validate_tool_name rather than claiming the viewset gates it.
• The deferred-import comment is deleted rather than reworded. I first replaced it with "importing these at module scope creates a cycle", then tested that claim before committing — there is no cycle. prompt_studio_core_v2.models imports cleanly without prompt_studio.permission, and the reverse order works too. Rather than ship a second unverified reason in place of the first, I removed the justification and left the local import alone.

Worth knowing: a later review pass argued that comment was load-bearing and should be restored. I kept it deleted, because the assertion it made is measurably false and I'd rather have no reason than a wrong one. If you know the real reason the import is deferred, that's worth a line.

[chandrasekharan-zipstack]

[High] [Lens 4 — Security] — Org-wide-shared projects get a split rule this comment does not describe, and the frontend cannot represent it.

_can_access_tool (prompt_studio/permission.py:14-29) admits owner / direct viewer / group member / org admin — but not shared_to_org. IsOwnerOrSharedUserOrSharedToOrg, used here for update/partial_update and for profile reads at prompt_profile_manager_v2/views.py:34, does admit it.

Failure mode: a user whose only access is an org-wide share can PATCH the project's settings and read its profiles, but gets 403 on every prompt edit (PromptAcesssToUser, permission.py:41-44) and every profile write. Half the collaborative surface works and half refuses, for the same user on the same project.

The frontend cannot express that state: usePromptStudioCanEdit collapses to a single is_owner boolean, so this user is shown one consistent affordance that is wrong for half the actions.

The class docstring at prompt_studio/permission.py:17-18 states the opposite of the code — "a shared user edits the project's prompts and settings, the same as its owner."

Fix: decide whether shared_to_org is inside or outside the collaborative set and apply it to both sides. Then pin it: (direct viewer, group, org-wide, unrelated member) × (settings PATCH, prompt PATCH, profile POST) is nine cheap API assertions that would have caught this without hand testing.

Confidence: High.

[kirtimanmishrazipstack]

@chandrasekharan-zipstack Fixed in d592324 — decided in, not out.

_can_access_tool now admits shared_to_org, so both sides of the collaborative surface agree. An org-wide-shared user gets the same answer from _can_access_tool and IsOwnerOrSharedUserOrSharedToOrg instead of 200 on settings and 403 on every prompt edit.

Reasoning for that direction rather than the other: the PR's stated rule is that Prompt Studio is shared for collaboration, and IsOwnerOrSharedUserOrSharedToOrg — already used for settings PATCH and profile reads on the same project — admits it. Narrowing that side instead would have taken settings access away from a group who have it today.

Verified both directions: an org-shared user with no membership, no group and no admin role is now admitted; an unrelated user on a non-shared tool is still refused.

The class docstring that stated the opposite is corrected in 0c9fb44. Your nine-assertion suggestion — (direct viewer, group, org-wide, unrelated) × (settings PATCH, prompt PATCH, profile POST) — is exactly the right pin and is blocked only by the CLAUDE.md test rule; see my reply on the IsParentToolOwner thread.

[github-actions]

Frontend Lint Report (Biome)

✅ All checks passed! No linting or formatting issues found.

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Unstract test results

Per-group results

Status	Group	Tier	Passed	Failed	Errors	Skipped	Duration (s)
✅	e2e-api-deployment	e2e	3	0	0	0	18.7
✅	e2e-coowners	e2e	1	0	0	0	1.3
✅	e2e-etl	e2e	1	0	0	0	10.4
✅	e2e-login	e2e	2	0	0	0	1.2
✅	e2e-prompt-studio	e2e	1	0	0	0	9.6
✅	e2e-smoke	e2e	2	0	0	0	1.1
✅	e2e-workflow	e2e	1	0	0	0	16.3
❌	frontend	unit	0	1	0	0	0.0
✅	integration-backend	integration	535	0	0	26	55.1
✅	integration-connectors	integration	1	0	0	7	7.9
✅	integration-workers	integration	157	0	0	1	49.9
❌	ui	e2e	0	1	0	0	0.0
✅	unit-backend	unit	1276	0	0	1	45.7
✅	unit-connectors	unit	63	0	0	0	9.9
✅	unit-core	unit	115	0	0	0	2.1
✅	unit-platform-service	unit	15	0	0	0	2.7
✅	unit-rig	unit	120	0	0	0	4.7
✅	unit-runner	unit	5	0	0	0	3.0
✅	unit-sdk1	unit	563	0	0	0	30.0
✅	unit-workers	unit	1425	0	0	1	155.7
	TOTAL		4286	2	0	36	425.4

Critical paths

⚠️ Critical paths not yet covered

• workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)

✅ Covered critical paths

• auth-login — covered by e2e-login
• adapter-register-llm — covered by integration-backend
• workflow-author — covered by integration-backend
• co-owner-manage — covered by integration-backend, e2e-coowners
• workflow-create-execute — covered by e2e-workflow
• api-deployment-provision — covered by integration-backend
• api-deployment-auth — covered by integration-backend
• api-deployment-run — covered by e2e-api-deployment
• mcp-server-auth — covered by integration-backend
• mcp-platform-auth — covered by integration-backend
• platform-key-whoami — covered by integration-backend
• prompt-studio-author — covered by integration-backend
• prompt-studio-fetch-response — covered by e2e-prompt-studio
• connector-register-test — covered by integration-backend
• pipeline-etl-execute — covered by e2e-etl
• usage-aggregate-read — covered by integration-backend
• usage-token-tracking — covered by e2e-api-deployment
• callback-result-delivery — covered by e2e-api-deployment