UN-2868 [FIX] Make settings read-only and block deletion on resources shared with a user by kirtimanmishrazipstack · Pull Request #2273 · Zipstack/unstract

kirtimanmishrazipstack · 2026-09-02T08:14:37Z

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.

Three holes found in review and closed here: anyone in the org could attach a tool to a workflow they did not own (which also switched that workflow on), the endpoint that clears execution markers answered a plain link click, and the workflow builder's connector-type dropdown was the one control on that card still offered to shared users.

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

Notes on the mechanism:

canEditResource (frontend) and _can_access_tool (backend) are the single definitions of the rule, so screens cannot drift apart.
DRF never runs object permissions on create, so collection-level actions need their own guard — WorkflowOwnerMutationMixin, ParentToolAccess, and the tool-instance check added here. Without it those routes were open to the whole organisation.
Renaming is checked per field, not per action: a project's name and its settings share one PATCH.
clear_file_marker moves from GET to POST. It has no frontend caller.
IsParentToolOwner is deleted — zero callers after the switch to ParentToolAccess.

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 (403)	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
Add a tool to a workflow	blocked (403)	200

Automated tests

New: backend/permissions/tests/test_shared_user_gates.py. IsParentToolOwnerTests is retargeted at the live gate as ParentToolAccessTests, with the viewer case flipped from denied to allowed.

Test	Asserts
`test_shared_viewer_cannot_change_connector_config`	403, config unchanged
`test_shared_viewer_cannot_delete_the_endpoint`	403, row survives
`test_shared_viewer_can_still_read_it`	200 — refusing a write must not hide the resource
`test_owner_and_co_owner_can_change_it`	200 for both
`test_a_user_with_no_access_gets_404_not_403`	404, so a refusal does not confirm the resource exists
`test_shared_viewer_cannot_add_a_tool`	403 on tool-instance create
`test_a_user_with_no_access_gets_404`	404 on tool-instance create
`test_shared_user_cannot_rename_the_project`	403, name unchanged
`test_shared_user_can_change_a_settings_field`	200 — same endpoint as the rename
`test_owner_can_rename`	200
`test_resending_the_same_name_is_not_a_rename`	200 — echoing the current name is not a rename
`test_parent_tool_viewer_allowed_outsider_denied`	the collaboration rule on profiles
`test_create_resolves_the_parent_from_the_payload`	owner and viewer allowed, outsider refused

Run: tox -e groups -- integration-backend. All are django.test.TestCase, so the backend conftest auto-marks them integration and they join that group with no manifest edit — verified: 24 collected under -m integration across both repos, 24 passed.

Every gate above was broken in turn to confirm the matching test fails. That caught one test of my own that passed for the wrong reason; it was deleted rather than kept.

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

…and org admins Workflow sub-resources were never gated when the sharing model landed. tool_instance_v2 was fixed; endpoint_v2 was missed, so any user a workflow was shared with could change its destination folder, database table or connector. Adds a reusable WorkflowOwnerMutationMixin next to is_workflow_mutator and applies it to WorkflowEndpointViewSet. Sharing -- direct, group or org-wide -- now grants read only; owners, co-owners, org admins and service accounts may still write. The UI now says so up front instead of failing on save: a shared user sees a read-only notice and greyed controls in the connector modal, tool settings, and the Prompt Studio project selector, with no Save button to press. The connector modal's Save now also flushes the HITL plugin's rules. It previously lit up for rule changes it could not save, then closed as if it had saved them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

…tead of its ID The workflow builder resolved the project name from exportedTools, which holds only the viewer's own exported projects. On a shared workflow the lookup missed and fell through to the raw function name, so shared users saw an ID where the owner saw a name. The tool instance already carries the display name (ToolInstanceSerializer sets it from the tool's properties), so fall back to that before falling back to the ID. The ID remains the last resort for a tool the registry can no longer resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

greptile-apps · 2026-09-02T09:44:40Z

via Greptile

Confidence Score: 5/5

The PR appears safe to merge.

Summary

This PR separates collaborative Prompt Studio access from read-and-run access for other shared resources while retaining owner-only rename, deletion, and configuration boundaries.

Adds centralized frontend and backend ownership/access gates across workflows, deployments, pipelines, adapters, connectors, and Prompt Studio resources.
Makes restricted controls visibly read-only and prevents keyboard or direct API mutation paths from bypassing those restrictions.
Fixes workflow/project labels, rename initialization, endpoint/tool mutation authorization, and marker-clearing request semantics.
Adds integration coverage for shared-user reads, blocked mutations, ownership, parent-resource resolution, and non-disclosing denials.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    U[Authenticated user] --> R{Resource type}
    R -->|Prompt Studio| C{Has collaborative access?}
    C -->|Yes| E[Edit prompts, settings, and profiles]
    C -->|No| D[Deny access]
    R -->|Workflow, pipeline, deployment, adapter, connector| O{Owner, co-owner, admin, or authorized service account?}
    O -->|Yes| M[Allow configuration mutation]
    O -->|No| V[Allow permitted read, run, and sharing operations only]
    M --> X[Persist resource changes]
    V --> L[Render restricted controls read-only]

_{Reviews (29) · Last reviewed commit: "UN-2868 [MISC] Merge main, resolving the..."}

… save Addresses review feedback plus two defects found alongside it. - perform_create skipped authorization entirely when the payload carried no workflow, then saved anyway. It now fails closed: every viewset using this mixin has a required workflow field, so a payload without one cannot be authorised. - handleValidateAndSubmit swallowed its own error, so a failed endpoint write still fell through to the HITL write and "Save and Close" dismissed the modal. Both writes now report success and the modal stays open on failure. - The HITL write only runs when its form is actually dirty; otherwise every connector save would have written a rule row too. - A view-only user can no longer be asked whether to save unsaved changes on close. They cannot have made any. - ReadOnlyNotice used defaultProps, which React 19 ignores on function components, so its default message never applied. Uses a default parameter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

kirtimanmishrazipstack · 2026-09-02T09:55:39Z

@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 · 2026-09-02T09:56:27Z

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 · 2026-09-02T09:56:44Z

@greptile-apps review

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

The read-only styling suppresses pointer events but leaves fields focusable, so a keyboard user could tab into one and press Enter. That fired a request the backend rejects, producing exactly the confusing error this ticket is about. Both RJSF forms now refuse to submit when the workflow is not editable, so the request is never made. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

…he user Sharing grants read only, but every resource list still offered Edit, Share, Delete and the enable/disable toggle to the people it was shared with. The backend refused them; the UI did not say so. All eight shareable resources render through two shared widgets, so the row actions are gated in one place each -- ResourceTable covers workflows, Prompt Studio, connectors, adapters, agentic projects and lookups; CardActionBox covers pipelines and API deployments. The two card kebabs mix read and write actions, so those are filtered per page: Manage Keys, Notifications and Clear File History go, while View Logs, File History, Sync Now, Code Snippets and Download Postman stay. Running and watching a shared pipeline is still allowed -- that is what sharing is for. The rule itself now lives in one helper, canEditResource, which useWorkflowCanEdit also delegates to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

The Prompt Studio editor is served by CustomToolSerializer, which never carried is_owner -- only the list serializer did. Without it the editor cannot tell a shared user from an owner, so its edit controls cannot be gated. canEditResource now treats a payload that has not arrived yet as editable. The backend refuses the write either way, and the alternative flashes a read-only view at the resource's own owner while the request is in flight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

…d delete Sharing onward is allowed for someone a resource was shared with: they may pass access to a group they belong to, or to a user in the same organisation. ShareAuthorizationService enforces both rules per axis, which is why the share endpoint sits at IsOwnerOrSharedUserOrSharedToOrg rather than IsOwner. The previous commit hid the Share button along with Edit and Delete. Only the latter two should go. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

Settings hold the project's LLM profiles and adapter selections -- the credential-bearing part. A shared user can read them but not change them: the panel gets the read-only notice and its controls are inert. Prompts are deliberately untouched. Editing, running and deleting prompts is what a project is shared for; only the settings panel is restricted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

The scope is settings and deletion, nothing else. The pipeline and API deployment cards had also lost their enable/disable toggle, Manage Keys, Notifications and Clear File History for shared users, which goes further than intended. Both card configs are reverted. Only the Edit and Delete controls in the two shared list widgets stay gated; Share, the toggle and every kebab action are available again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

Hiding the two controls left a shared user with no idea they existed or why they were missing. They now stay on screen, greyed out, with a tooltip reading "Only the owner can change this". Same treatment in both list widgets so every resource looks the same. The rename pencil beside a project title follows the same rule: ToolNavBar takes an editTitleDisabled prop, and Prompt Studio passes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

…rules The connector write showed "Configuration saved successfully" before the rule write ran. A rule failure after it left a green toast on screen next to a red one, reading as though everything had landed. The connector success message is now suppressed when a rule write follows, and the rule write reports the outcome for both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TaTtTMaDriVZgw6BR8HZ4G

…ing-improvements

…locks to owner-only actions Four defects found while testing the shared-user gates. Workflow rename modal opened blank for the owner. `Form.Item` controls its child and injects a value from the form store, so the `defaultValue` on the Input never reached the DOM; the OK button also started disabled and only flipped on change, so the owner had to retype both fields to save. Seed the form via `initialValues` and derive the button's initial state from the props. Workflow builder header offered its rename pencil to shared users. `ToolNavBar` already takes `editTitleDisabled` and Prompt Studio and Agentic already pass it; `MenuLayout` was the one screen that did not. Prompt Studio settings were locked wholesale for shared users, which also blocked adding an LLM profile of their own. The backend only refuses editing and deleting an existing profile (`IsParentToolOwner`) -- create, copy, set default and every other tab are open to a shared user. Move the lock onto the two buttons that are actually refused and drop the modal-wide notice. The rows are built in an effect, so `canEdit` joins its deps: it starts true while `details` loads and would otherwise leave the buttons stale-enabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

…to UN-2868-sharing-improvements

…hared users Shared access is meant to grant read and run only, but Prompt Studio still saved prompt and settings writes from a shared user, and three toggles offered actions the backend already refuses. Prompts. `PromptAcesssToUser` admitted viewers and group-shared users on every method, so their edits and deletes went through; shared access is now limited to SAFE_METHODS. `reorder_prompts` is routed without a pk, so DRF ran no object check at all and any org member could reorder any project's prompts -- it now resolves the prompt and checks it. Settings. Every tab in the modal writes to the project row, so it is owner-only again, LLM profiles included. `update`, `partial_update` and `make_profile_default` move to `IsOwner`, which also closes project rename. `IsParentToolOwner` gains a `has_permission`: `create` was already listed as owner-only, but DRF never calls the object check on a create, which is why a shared user could add an LLM profile and then not edit it. Toggles. The ETL pipeline and API deployment enable/disable switches carried no ownership gate even though both endpoints reject the PATCH. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

… key controls Product call on the ticket thread: sharing is not one uniform rule. A Prompt Studio project is shared for collaboration -- a locked one is not useful to a team -- while adapters, connectors, workflows and deployments are shared for use. Across all of them the owner keeps three things: renaming, deleting, and deciding who else gets access. Prompt Studio therefore reopens. Prompts, every Settings tab and LLM profiles (create, edit, delete, and the default) are back to collaborative, in Agentic Prompt Studio too. The credential concern behind locking profiles does not hold: a profile can only point at adapters the requester already has access to, and it never renders a key. Renaming a project moved to a per-field check in the serializer. Settings and the name share one PATCH endpoint, so gating the action would have closed both. Two checks that were missing stay in place rather than reverting with the rest. `reorder_prompts` is routed without a pk, so DRF ran no object check and any org member could reorder any project's prompts. And `ProfileManagerView` now uses `ParentToolAccess`: create is collection-level, so without a `has_permission` it was open to the whole org rather than to collaborators. Manage Keys hid nothing behind ownership, so a shared user could press New Key and the active toggle and get a 4xx -- the toggle surfacing as "Api deployment not found". Both are disabled now; listing and copying keys are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

…reates Review findings D1-D4, D6, SF2 and CR4. - Shared viewers were served the destination connector's decrypted credentials on the endpoint read path. Redacted for anyone who may only read; the previous test asserted a bare 200 and could not see it. - create_prompt and create_profile_manager never reached the object gate, so any org member could write into any project. The parent now comes from the URL, which both runs the gate and stops the payload naming another project. - clear-file-marker moved to POST but its only caller still sent GET, so clearing file history 405'd for everyone, owners included. - LookupDefinition was missing from the shareable-resource registry, so its group shares were never purged and never shown to an admin deleting the group. - Two gates filtered a UUID column on raw request data ahead of any serializer, turning a malformed id into a 500. - Manage Keys gated the switch and New Key but left per-key Edit and Delete live, so a shared user could fill in a form that only 403s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

Review findings D6, O1 and L16-2/4/6. - PromptAcesssToUser enumerated four ways to qualify and the code has five; point at _can_access_tool instead of restating it. - The share endpoint's docstring counted 7 shareable resources; there are eight, so name the rule rather than the count. - The frontend helper and the Prompt Studio hook both said sharing grants read only, which is not true of Prompt Studio or Agentic PS. - A test cited IsParentToolOwner, which no longer exists and whose successor admits collaborators. - The read-only CSS named the HITL query builder, which no longer uses it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

…ot know Cross-document sweep for the read-only rule corrected in 025c12c. Both widgets render Prompt Studio alongside every other resource, and Prompt Studio is shared for collaboration -- so "sharing grants read only" is wrong for one of their own consumers. What they actually gate is edit and delete, which is owner-only everywhere; say that instead. The card grid also carried a second owner rule, falling back to ``created_by`` when ``is_owner`` was absent. That fallback cannot see co-owners, and every list serializer sends the field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

@action

…re wrong An adversarial pass re-found three findings and raised five more. - The credential redaction asked who owned the workflow once per row, which is a membership query each time. Prefetch the rows on the endpoint queryset and let the owner check read them: measured 44 queries for 7 endpoints both with and without the redaction, so it now costs nothing. A test pins it. - The share docstring traded a stale count ("all 7 resources") for a universal that is false: lookups is the one host whose share entry is org-member-gated. Name the mechanism instead of the class. - The read-only CSS comment dropped the HITL query builder as a stale consumer. It is not stale -- the connector modal still wraps the builder in that class. Stop naming a list that goes stale. - Both new comments called a detail action "collection-level". The conclusion held but the reason did not: no custom @action gets get_object() for free, pk in the route or not. - OwnerFieldRow kept a sessionDetails prop nothing read after the owner rule was collapsed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

The docstring called both halves custom ``@action``s. Only the prompt half is: profiles go through a plain DRF ``create`` whose parent is read from the payload in ``ParentToolAccess.has_permission``. Conflating them points the next reader at a decorator that is not there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

@action

… it again Three attempts, three wrong descriptions -- the last one named a route that does not exist: ProfileManagerView registers no POST, and the live add-profile endpoint is PromptStudioCoreView.create_profile_manager, a detail @action like its sibling. The rule is already stated where it is enforced, once per half, in prompt_studio_core_v2/views.py. A third copy on the test class is a surface that has now gone stale every time it was touched, so remove it rather than correct it a fourth time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

…um its test The security pass caught this in the previous round's own work. The view gate was guarded with parse_uuid, but the controller re-reads prompt_id from validated_data and feeds it to objects.get, where a malformed value is a 500. prompt_id was a CharField, so a complete request still reached it -- the guard only covered the gate. The test hid that: it omitted a required field, so it 400'd before the id was ever parsed and would have passed against no fix at all. It now sends a complete payload and asserts the 400, and fails if the field goes back to CharField. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

Opening a workflow reached through a group worked; its File History tab returned 403. `Workflow.objects.for_user` has always included group shares, so the workflow is listed and opens, but `IsWorkflowOwnerOrShared` stopped at OWNER membership, direct VIEWER membership, `shared_to_org` and org admin. The queryset and the gate disagreed, and the gate was the narrower one. Pre-existing rather than introduced here: this branch only added the `IsWorkflowOwnerForFileHistoryWrite` subclass for delete and clear, and left the read path as it is on main. `FileHistoryViewSet` is the only consumer, so that tab was the whole blast radius. Swept both repos for the same shape afterwards. Every other call site of the owner/viewer predicates without a group branch is deliberately owner-only -- `IsOwner`, `is_workflow_mutator`, `IsRegistryToolOwner`, `IsFrictionLessAdapterDelete`, the co-owner serializers -- since a group share grants read and run, never mutation. Every `for_user` either includes groups or delegates to one that does; `AppDeployment` predates the membership model entirely. Tests: a group member can open file history, the owner still can, and a non-shared org member still cannot -- widening to groups must not widen to everyone. Verified by removing the branch again and watching the first fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

kirtimanmishrazipstack · 2026-09-15T19:28:35Z

Note on the red checks — none are from this PR

test (integration), e2e, report fail before a single test runs. The rig cannot
pull the MinIO image while bringing infrastructure up:

MinioContainer("minio/minio:latest").start()
docker.errors.ImageNotFound: pull access denied for minio/minio,
repository does not exist or may require 'docker login'

It dies in tests/rig/runtime.py → runtime.up(), so no test from this branch is
reached. report is red only because it aggregates those two.

Not specific to this branch: the same failure is currently hitting six other
branches (UN-3494, UN-3853, UN-4078, UN-1031, fix/single-pass-profile-export), and
main's last green run was 2026-09-10. It needs a registry/runner fix, not a code change.

test (unit) passes, which is the tier that actually exercises this PR.

Run locally against the full backend, both tiers: 928 unit passed, 471 integration
passed (28 skips are the pre-existing credential-gated external-DB connector tests).

chandrasekharan-zipstack

Standardized PR Review — FOLLOWUP (UN-2868, OSS half) @ `8d966af`

Verdict — BLOCK on one pre-existing Critical (below). If it is scoped out to a tracked ticket with a written waiver here, the verdict becomes REQUEST CHANGES on the coverage High.

Since d6dbc0ed → 8d966aff. Scope change: YES — new permissions/tests/test_shared_user_gates.py, rewritten test_owner_management.py, gate classes reshaped; re-scanned backend gates/serializers, agency/pipeline/custom-tools frontend, tests. Tests executed locally on an isolated DB (59 pass) and mutation-tested per gate.

Prior findings — 9 inline: 8 RESOLVED, #6 (canEditResource) WAIVED by reviewer. Status replies are in each thread. Unanchored: tool_instance create RESOLVED (tool_instance_v2/views.py:139-148, pinned); file-history writes RESOLVED (file_history_views.py:33-36, workflow_v2/views.py:358); DsSettingsCard RESOLVED (DsSettingsCard.jsx:24,234-246); zero tests PARTIALLY RESOLVED (22 tests, 9 of 14 gates pinned — see High below).

New: Critical: 1 · High: 1 · Medium: 5 · Low: 4. Critical + High are inline below; Medium/Low are in the full report and can be posted on request.

Unanchored findings

none — both new Critical/High anchored inline.

Lens checklist — 1 See M · 2 See M · 3 See M · 4 See C1, H1, M · 5 Clean · 6 Clean · 7 Clean · 8 Clean · 9 Clean · 10 Clean · 11 Clean · 12 N/A — no prompt/model changes · 13 See H1 · 14 Clean · 15 See M · 16 See M.

Open — external callers of GET …/clear-file-marker/ (python client, docs) now 405; the new tests are DB-bound (integration tier) and have not run in CI at this head; H3 (lookup-definition dormant grants) waived by maintainer on the cloud PR.

…es with tests Two findings from the standardized review. **Deploying someone else's workflow.** `workflow` is writable on both the API deployment and pipeline serializers and was bound through a merely org-scoped manager, so any organisation member could POST a deployment naming a colleague's private workflow -- then execute it, with its connectors and adapters, at the owner's cost, and mint API keys against it. Reachable on create and on update. Pre-existing. Both serializers now scope the field in `get_fields`, the pattern `WorkflowEndpointSerializer` already uses, and refuse reparenting the way the other guards in this branch do. The scope is `mutable_workflows_for` -- owners, co-owners, org admins -- not the `for_user` floor. A deployment is a persistent execution surface its creator owns and can share onward, so standing one up is an owner act rather than a use of a shared workflow. Co-ownership is the route for anyone else. Switching to the looser floor is a one-line change in that helper if the product call goes the other way. `resources_visible_via_memberships` grows an optional `role`, so the helper reuses its varchar/UUID cast: joining the membership table directly fails with `operator does not exist: uuid = character varying`. **Tests.** Last round's fixes could each be reverted with the suite staying green. Now pinned: endpoint-list scoping and its credential redaction, both tool-instance reparent guards including the `workflow_id` alias, file-history delete and clear, `clear_file_marker` as POST-only, and the new deployment scoping. Every gate was reverted in turn to confirm the matching test fails. Two of those runs found tests that passed for the wrong reason -- the deployment cases were being refused by the endpoint-configuration check, not by the gate -- so they now assert the `does_not_exist` code specifically. `CreateEndpointGrantsCreatorOwnershipTests` built its parent workflow with `created_by` but no OWNER row, which the scoping correctly refuses; its own docstring says `created_by` is audit-only, so the fixture now grants the row the real create path grants. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

- `WorkflowEndpointSerializer.to_representation` had two `return rep` statements. Sonar reads that as always returning the same value; it does not, since `rep` is mutated between them, but a single exit says so plainly and the rule was flagged BLOCKER. - The ETL and API deployment status tooltips were nested ternaries. Both lift to a named `toggleTitle`, which also removes a second `canEditResource` call per row. - `canEditResource` uses an optional chain for the in-flight-payload guard. Behaviour is unchanged throughout; 72 permission tests and the frontend build pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

…needs `test_deploy_mints_key_and_endpoint` built its workflow with neither ``created_by`` nor an OWNER membership, so the deployable-workflow scoping added in ba4f559 correctly refused it: ``created_by`` is audit-only, and ownership comes from the membership row the real create path writes. The fixture now grants that row, matching ``WorkflowViewSet.perform_create``. Same shape as the `CreateEndpointGrantsCreatorOwnershipTests` fixture corrected in the same commit; CI reported this as the only other one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

Findings from a code review of the last four commits. **Co-owners locked out (2 sites).** Scoping the writable `workflow` field to workflows the requester owns broke every edit of a deployment or pipeline whose workflow belongs to someone else: both forms PUT the whole record back, `workflow` included, so a co-owner of the resource 400'd on a field they were not changing. The bound workflow is now always selectable; `validate_workflow` still refuses an actual move, so reparenting stays shut. **The enable toggle.** The rule is that a shared user may start and stop what is shared with them, and the code said owner-only on both halves. Toggling is now an activation-only PATCH -- one field, nothing riding along -- admitted for anyone the resource is shared with. Settings stay with the owner. The API deployment toggle sends that narrow PATCH instead of a full PUT, as the pipeline one already did. **A test that could not fail.** `test_a_get_is_rejected` drove `as_view({"post": ...})` and sent a GET, so the 405 came from its own method map; it passed with the decorator back on `methods=["get"]`. It now asserts the action's mapping and every route that binds it. **The deploy picker** offered workflows the serializer then refused, with a raw `Invalid pk` naming an id the user never typed. It lists only deployable ones now, and the refusal says why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

Greptile P1. The schema form's own submit was wired to the endpoint-only path, so pressing Enter with both a connector edit and a rule edit pending wrote the endpoint, announced "Configuration saved successfully", and dropped the rules. The Save button took the coordinated path; the keyboard did not. Both now go through `handleSave`. It takes RJSF's validated data when the form supplies it and falls back to state for the button, so neither path loses the fresher value. The Save button's `onClick` is wrapped rather than passed by reference: a bare reference would hand the click event to that new parameter, and the event would then be compared against the saved configuration and written as it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

No code change. The last red run predates the fixture and SonarCloud fixes on this branch; this re-triggers the checks against the current head. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

UN-3853 (#2274) extracted the "Owned By" label into the shared ``resolveOwnerDisplay`` helper. This branch had rewritten the same block in the card view to read ``is_owner``, so the two collided in ``CardFieldComponents`` and on the import line in ``ResourceTable``. Took the helper. It reads ``owner_emails``, so it sees co-owners -- the gap this branch's version was written to close -- and it additionally labels platform-key service accounts and resolves "Me" against the owner actually shown rather than the viewer's own membership. ``OwnerFieldRow`` therefore takes ``sessionDetails`` again, and both card configs pass it. ``is_owner`` is untouched elsewhere: it drives editability via ``canEditResource``, which is a separate question from whose name is on the row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

github-actions · 2026-09-17T05:44:42Z

Frontend Lint Report (Biome)

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

sonarqubecloud · 2026-09-17T05:45:42Z

Quality Gate passed

Issues
2 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 · 2026-09-17T05:51:39Z

Unstract test results

Per-group results

Status	Group	Tier	Passed	Failed	Skipped	Duration (s)
✅	`e2e-api-deployment`	e2e	3	0	0	15.2
✅	`e2e-coowners`	e2e	1	0	0	1.6
✅	`e2e-etl`	e2e	1	0	0	16.7
✅	`e2e-login`	e2e	2	0	0	1.3
✅	`e2e-prompt-studio`	e2e	1	0	0	11.2
✅	`e2e-smoke`	e2e	2	0	0	1.2
✅	`e2e-workflow`	e2e	1	0	0	20.3
❌	`frontend`	unit	0	1	0	0.0
✅	`integration-backend`	integration	598	0	26	45.3
✅	`integration-connectors`	integration	1	0	7	11.0
✅	`integration-workers`	integration	157	0	1	43.9
❌	`ui`	e2e	0	1	0	0.0
✅	`unit-backend`	unit	1276	0	1	45.7
✅	`unit-connectors`	unit	63	0	0	10.2
✅	`unit-core`	unit	115	0	0	2.1
✅	`unit-platform-service`	unit	15	0	0	2.7
✅	`unit-rig`	unit	120	0	0	4.9
✅	`unit-runner`	unit	5	0	0	3.0
✅	`unit-sdk1`	unit	563	0	0	29.8
✅	`unit-workers`	unit	1425	0	1	129.2
	TOTAL		4349	2	36	395.0

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

* UN-4128 [FIX] Stop the Pipeline list page 500ing on the workflow-scoping guard PipelineSerializer.get_fields() (UN-2868, #2273) scopes the workflow field to the requester's own workflows, guarding the single-instance case with `self.instance is not None`. On a list request DRF hands the child serializer of a many=True ListSerializer the whole queryset as self.instance, not one row -- `is not None` let that through and `.workflow_id` crashed on a list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018KZLGSa3oWxgVJdFqvRUQX * UN-4128 Add regression test for pipeline list serializer crash Covers the AttributeError DRF's paginated list GET triggers when self.instance is a list instead of a Pipeline or None. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * UN-4128 Cover the public many=True constructor in the list-scoping test Greptile asked for a test that exercises PipelineSerializer(queryset, many=True) directly instead of hand-setting self.instance. DRF's many_init passes the same instance to the child, so both paths already caught the regression, but this removes any doubt. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

kirtimanmishrazipstack and others added 2 commits September 2, 2026 13:41

kirtimanmishrazipstack changed the title ~~UN-2868 [FIX] Prevent shared users from changing a workflow's destination connector and tool settings~~ UN-2868 [FIX] Make shared workflows read-only for shared users and show the project name instead of its ID Sep 2, 2026

kirtimanmishrazipstack marked this pull request as ready for review September 2, 2026 09:41

greptile-apps Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread backend/permissions/permission.py

kirtimanmishrazipstack and others added 5 commits September 2, 2026 15:31

kirtimanmishrazipstack mentioned this pull request Sep 2, 2026

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

Merged

kirtimanmishrazipstack and others added 2 commits September 2, 2026 16:59

kirtimanmishrazipstack changed the title ~~UN-2868 [FIX] Make shared workflows read-only for shared users and show the project name instead of its ID~~ UN-2868 [FIX] Make settings read-only and block deletion on resources shared with a user Sep 3, 2026

greptile-apps Bot reviewed Sep 3, 2026

View reviewed changes

Comment thread frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx Outdated

kirtimanmishrazipstack and others added 6 commits September 3, 2026 11:17

Merge branch 'main' into UN-2868-sharing-improvements

8a22d05

Merge branch 'main' of github.com:Zipstack/unstract into UN-2868-shar…

f078398

…ing-improvements

Merge remote-tracking branch 'origin/UN-2868-sharing-improvements' in…

08d7e4e

…to UN-2868-sharing-improvements

greptile-apps Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread backend/prompt_studio/permission.py Outdated

Comment thread backend/permissions/permission.py Outdated

Comment thread frontend/src/components/pipelines-or-deployments/pipelines/PipelineCardConfig.jsx Outdated

greptile-apps Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread backend/prompt_studio/permission.py

Merge branch 'main' into UN-2868-sharing-improvements

77c055f

kirtimanmishrazipstack and others added 8 commits September 15, 2026 20:16

kirtimanmishrazipstack requested a review from chandrasekharan-zipstack September 15, 2026 19:25

chandrasekharan-zipstack reviewed Sep 16, 2026

View reviewed changes

Comment thread backend/api_v2/serializers.py

Comment thread backend/workflow_manager/endpoint_v2/views.py

kirtimanmishrazipstack and others added 4 commits September 16, 2026 16:20

Merge branch 'main' into UN-2868-sharing-improvements

732e69b

kirtimanmishrazipstack requested a review from chandrasekharan-zipstack September 16, 2026 12:30

greptile-apps Bot reviewed Sep 16, 2026

View reviewed changes

Comment thread frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx Outdated

kirtimanmishrazipstack and others added 4 commits September 16, 2026 18:35

chandrasekharan-zipstack approved these changes Sep 17, 2026

View reviewed changes

kirtimanmishrazipstack merged commit 1f1b28a into main Sep 17, 2026
11 checks passed

kirtimanmishrazipstack deleted the UN-2868-sharing-improvements branch September 17, 2026 06:41

This was referenced Sep 17, 2026

UN-2868 [FIX] Stop the connector modal reporting a HITL save it discarded #2286

Merged

UN-4128 [FIX] Stop the pipeline/ETL list page from 500ing #2291

Merged

Conversation

kirtimanmishrazipstack commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

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

Database Migrations

Env Config

Relevant Docs

Related Issues or PRs

Dependencies Versions

Notes on Testing

Automated tests

Screenshots

Checklist

Uh oh!

greptile-apps Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Uh oh!

Uh oh!

kirtimanmishrazipstack commented Sep 2, 2026

Uh oh!

greptile-apps Bot commented Sep 2, 2026

Uh oh!

kirtimanmishrazipstack commented Sep 2, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kirtimanmishrazipstack commented Sep 15, 2026

Note on the red checks — none are from this PR

Uh oh!

chandrasekharan-zipstack left a comment

Choose a reason for hiding this comment

Standardized PR Review — FOLLOWUP (UN-2868, OSS half) @ 8d966af

Unanchored findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 17, 2026

Frontend Lint Report (Biome)

Uh oh!

sonarqubecloud Bot commented Sep 17, 2026

Quality Gate passed

Uh oh!

github-actions Bot commented Sep 17, 2026

Unstract test results

Per-group results

Critical paths

⚠️ Critical paths not yet covered

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kirtimanmishrazipstack commented Sep 2, 2026 •

edited

Loading

greptile-apps Bot commented Sep 2, 2026 •

edited

Loading

Standardized PR Review — FOLLOWUP (UN-2868, OSS half) @ `8d966af`