From cd3efb86ed796b2097f51081b2d53514504fd6c1 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Tue, 13 Oct 2026 08:12:18 -0700 Subject: [PATCH] feat: make an operator who forks a developer in the fork AllowOperatorForking now grants a fork to build in: the creator row copies only is_admin, so the operator is a developer there. Deploying back still runs with their parent role, where operator write checks refuse it. On self-hosted EE this makes them a full seat, which the ruleset toggle says. "Edit in fork" is shown to an operator when they may fork and the workspace has no canonical dev workspace. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...13902ed94816142faf15446b4b9f19c3097d7.json | 16 ++++++++++++ ...0b95beed35ba95dbab948bb9bd1287aae28cd.json | 16 ------------ .../tests/operator_fork.rs | 8 +++--- .../windmill-api-workspaces/src/workspaces.rs | 8 +++--- docs/operator-write-rights.md | 9 ++++--- .../lib/components/common/table/AppRow.svelte | 2 +- .../components/common/table/FlowRow.svelte | 8 ++---- .../components/common/table/ScriptRow.svelte | 11 +++----- .../lib/components/details/FlowDetail.svelte | 7 +---- .../components/details/ScriptDetail.svelte | 7 +---- .../src/lib/components/runs/RunDetail.svelte | 26 ++++++++----------- .../CreateWorkspaceInner.svelte | 2 +- .../workspaceSettings/RulesetEditor.svelte | 3 ++- frontend/src/lib/utils/editInFork.ts | 10 ++++--- 14 files changed, 60 insertions(+), 73 deletions(-) create mode 100644 backend/.sqlx/query-42a0ba479ff164cc190c350927e13902ed94816142faf15446b4b9f19c3097d7.json delete mode 100644 backend/.sqlx/query-be88c05af4d8b86325cebad62370b95beed35ba95dbab948bb9bd1287aae28cd.json diff --git a/backend/.sqlx/query-42a0ba479ff164cc190c350927e13902ed94816142faf15446b4b9f19c3097d7.json b/backend/.sqlx/query-42a0ba479ff164cc190c350927e13902ed94816142faf15446b4b9f19c3097d7.json new file mode 100644 index 0000000000..76eab1d117 --- /dev/null +++ b/backend/.sqlx/query-42a0ba479ff164cc190c350927e13902ed94816142faf15446b4b9f19c3097d7.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO usr\n (workspace_id, email, username, is_admin)\n SELECT $1, email, username, is_admin FROM usr\n WHERE workspace_id = $3 AND email = $2\n ON CONFLICT DO NOTHING\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "42a0ba479ff164cc190c350927e13902ed94816142faf15446b4b9f19c3097d7" +} diff --git a/backend/.sqlx/query-be88c05af4d8b86325cebad62370b95beed35ba95dbab948bb9bd1287aae28cd.json b/backend/.sqlx/query-be88c05af4d8b86325cebad62370b95beed35ba95dbab948bb9bd1287aae28cd.json deleted file mode 100644 index 83d781d094..0000000000 --- a/backend/.sqlx/query-be88c05af4d8b86325cebad62370b95beed35ba95dbab948bb9bd1287aae28cd.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO usr\n (workspace_id, email, username, is_admin, operator, role)\n SELECT $1, email, username, is_admin, operator, role FROM usr\n WHERE workspace_id = $3 AND email = $2\n ON CONFLICT DO NOTHING\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Text", - "Text" - ] - }, - "nullable": [] - }, - "hash": "be88c05af4d8b86325cebad62370b95beed35ba95dbab948bb9bd1287aae28cd" -} diff --git a/backend/windmill-api-integration-tests/tests/operator_fork.rs b/backend/windmill-api-integration-tests/tests/operator_fork.rs index 6d6bc3f052..5fd0177579 100644 --- a/backend/windmill-api-integration-tests/tests/operator_fork.rs +++ b/backend/windmill-api-integration-tests/tests/operator_fork.rs @@ -5,10 +5,10 @@ use sqlx::{Pool, Postgres}; use windmill_test_utils::*; -/// Operators cannot fork until a ruleset carries `AllowOperatorForking`, and the fork keeps them an -/// operator rather than handing them the developer role the bare creator row would default to. +/// Operators cannot fork until a ruleset carries `AllowOperatorForking`, and the fork they then +/// create makes them a developer, not an operator. #[sqlx::test(migrations = "../migrations", fixtures("base"))] -async fn test_operator_fork_needs_ruleset_and_stays_operator( +async fn test_operator_fork_needs_ruleset_and_develops_in_fork( db: Pool, ) -> anyhow::Result<()> { initialize_tracing().await; @@ -57,7 +57,7 @@ async fn test_operator_fork_needs_ruleset_and_stays_operator( ) .fetch_one(&db) .await?; - assert!(operator && !is_admin); + assert!(!operator && !is_admin); Ok(()) } diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 44a7c975b4..7b0aafb01a 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -9798,11 +9798,13 @@ async fn write_workspace_fork( } // Ensure the creator is a member of the fork even without copy_members (or if they aren't a parent - // member). No-op when copy_members already brought their full row. + // member). No-op when copy_members already brought their full row. `operator` is left at its + // false default on purpose: an operator allowed by `AllowOperatorForking` develops in the fork, + // while deploying back still runs with their parent role, where operator write checks refuse it. sqlx::query!( "INSERT INTO usr - (workspace_id, email, username, is_admin, operator, role) - SELECT $1, email, username, is_admin, operator, role FROM usr + (workspace_id, email, username, is_admin) + SELECT $1, email, username, is_admin FROM usr WHERE workspace_id = $3 AND email = $2 ON CONFLICT DO NOTHING ", diff --git a/docs/operator-write-rights.md b/docs/operator-write-rights.md index 5ac8017869..8b0bc37447 100644 --- a/docs/operator-write-rights.md +++ b/docs/operator-write-rights.md @@ -103,6 +103,9 @@ Operators cannot fork unless some protection ruleset of the workspace carries rather than restricts, so it never goes through `check_user_against_rule` and its ruleset's bypass lists play no part. `canCreateFork` in `frontend/src/lib/utils/editInFork.ts` mirrors it. -The fork copies the creator's `operator` flag and `role`, so an operator who forks is an operator in -the fork. Without that, the bare creator row defaults to developer and forking becomes a way to gain -write access. +An operator who forks is a developer in the fork: the creator row copies only `is_admin`, and +`operator` keeps its false default. That is the point of the rule, a copy to build in. It gives no +write access to the parent: deploying a fork's changes back runs with the user's role in the +parent, where the operator write checks refuse it, so an admin or a `wm_deployers` member deploys +them instead. The developer row does cost a seat: the self-hosted seat count takes a user who is a +developer in any workspace, forks included, as a full seat. diff --git a/frontend/src/lib/components/common/table/AppRow.svelte b/frontend/src/lib/components/common/table/AppRow.svelte index a318c1f412..d8d08d1a3a 100644 --- a/frontend/src/lib/components/common/table/AppRow.svelte +++ b/frontend/src/lib/components/common/table/AppRow.svelte @@ -174,7 +174,7 @@ {/if} - {#if !$userStore?.operator && !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!showEditButton || !app.canWrite)} + {#if !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!showEditButton || !app.canWrite)} {/if} diff --git a/frontend/src/lib/components/common/table/FlowRow.svelte b/frontend/src/lib/components/common/table/FlowRow.svelte index 65690121ab..7560658326 100644 --- a/frontend/src/lib/components/common/table/FlowRow.svelte +++ b/frontend/src/lib/components/common/table/FlowRow.svelte @@ -207,8 +207,7 @@ {/if} - - {#if !$userStore?.operator && !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!showEditButton || !flow.canWrite)} + {#if !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!showEditButton || !flow.canWrite)} {/if} @@ -278,10 +277,7 @@ // No `href`: the handler resolves the destination asynchronously, and a melt // menu item's anchor navigates before a delegated onclick can preventDefault it. action: (e) => onEditInForkClick(e, 'flow', path), - hide: - $userStore?.operator || - isCloudHosted() || - !editInForkAllowed($workspaceStore, $userWorkspaces) + hide: isCloudHosted() || !editInForkAllowed($workspaceStore, $userWorkspaces) }, { displayName: 'Audit logs', diff --git a/frontend/src/lib/components/common/table/ScriptRow.svelte b/frontend/src/lib/components/common/table/ScriptRow.svelte index 7ba327f343..8f76d45126 100644 --- a/frontend/src/lib/components/common/table/ScriptRow.svelte +++ b/frontend/src/lib/components/common/table/ScriptRow.svelte @@ -267,9 +267,9 @@ {/if} {/if} - {#if !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!showEditButton || !script.canWrite)} - - {/if} + {/if} + {#if !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!showEditButton || !script.canWrite)} + {/if} onEditInForkClick(e, 'script', script.path), - hide: - $userStore?.operator || - isCloudHosted() || - !editInForkAllowed($workspaceStore, $userWorkspaces) + hide: isCloudHosted() || !editInForkAllowed($workspaceStore, $userWorkspaces) }, { displayName: 'Move/Rename', diff --git a/frontend/src/lib/components/details/FlowDetail.svelte b/frontend/src/lib/components/details/FlowDetail.svelte index 30de5dc7cb..3fbeb227bb 100644 --- a/frontend/src/lib/components/details/FlowDetail.svelte +++ b/frontend/src/lib/components/details/FlowDetail.svelte @@ -381,12 +381,7 @@ }) } - if ( - flow && - !actingUser?.operator && - !isCloudHosted() && - editInForkAllowed(workspace, $userWorkspaces) - ) { + if (flow && !isCloudHosted() && editInForkAllowed(workspace, $userWorkspaces)) { buttons.push({ label: editInForkLabel(workspace, $userWorkspaces), description: editInForkDescription('flow', workspace, $userWorkspaces), diff --git a/frontend/src/lib/components/details/ScriptDetail.svelte b/frontend/src/lib/components/details/ScriptDetail.svelte index 6712092c39..13a5e31050 100644 --- a/frontend/src/lib/components/details/ScriptDetail.svelte +++ b/frontend/src/lib/components/details/ScriptDetail.svelte @@ -544,12 +544,7 @@ }) } - if ( - script && - !actingUser?.operator && - !isCloudHosted() && - editInForkAllowed(workspace, $userWorkspaces) - ) { + if (script && !isCloudHosted() && editInForkAllowed(workspace, $userWorkspaces)) { buttons.push({ label: editInForkLabel(workspace, $userWorkspaces), description: editInForkDescription('script', workspace, $userWorkspaces), diff --git a/frontend/src/lib/components/runs/RunDetail.svelte b/frontend/src/lib/components/runs/RunDetail.svelte index 1241bc9c60..9eef7f2ff9 100644 --- a/frontend/src/lib/components/runs/RunDetail.svelte +++ b/frontend/src/lib/components/runs/RunDetail.svelte @@ -1148,21 +1148,17 @@ /> {/if} {/if} - {#if !showEditButton && !isCloudHosted() && editInForkAllowed(workspaceId, $userWorkspaces)} - - {/if} + {/if} + {#if !showEditButton && !isCloudHosted() && editInForkAllowed(workspaceId, $userWorkspaces)} + {/if} {/if} {#if job?.job_kind === 'script' || job?.job_kind === 'script_hub' || job?.job_kind === 'flow'} diff --git a/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte b/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte index 88aa3ca532..53c690a019 100644 --- a/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte +++ b/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte @@ -128,7 +128,7 @@ // Base candidates are the current workspace's family: its root first, then every fork/dev under it. // Bases the user is an operator in stay listed: the server refuses one whose rulesets lack - // `AllowOperatorForking`, and the fork keeps the operator role. + // `AllowOperatorForking`, and deploying back from the fork runs with their role in the parent. let familyRoot = $derived(findWorkspaceRoot($workspaceStore, forkableWorkspaces)) let baseCandidates = $derived( familyRoot ? [familyRoot, ...findWorkspaceDescendants(familyRoot.id, forkableWorkspaces)] : [] diff --git a/frontend/src/lib/components/workspaceSettings/RulesetEditor.svelte b/frontend/src/lib/components/workspaceSettings/RulesetEditor.svelte index a6a1758a37..505771a21e 100644 --- a/frontend/src/lib/components/workspaceSettings/RulesetEditor.svelte +++ b/frontend/src/lib/components/workspaceSettings/RulesetEditor.svelte @@ -378,7 +378,8 @@ />
Operators cannot fork a workspace unless a ruleset turns this on. It applies to every - operator, whatever the bypass list says, and the operator stays an operator in the fork. + operator, whatever the bypass list says. An operator who forks is a developer in the fork, + and counts as a developer seat, but still cannot deploy changes back to this workspace.
diff --git a/frontend/src/lib/utils/editInFork.ts b/frontend/src/lib/utils/editInFork.ts index 2dad3b90a8..db38537115 100644 --- a/frontend/src/lib/utils/editInFork.ts +++ b/frontend/src/lib/utils/editInFork.ts @@ -19,16 +19,18 @@ export type ItemType = 'script' | 'flow' | 'app' | 'raw_app' * Whether to show the "edit in fork / dev workspace" affordance. Allowed when the user may create a * fork, OR when the current workspace has a canonical dev to route to — routing into an existing dev * workspace creates no fork, so it survives a locked prod that has `DisableWorkspaceForking` set. - * Never for an operator, who has nothing to edit at the other end either way. User identity is read - * non-reactively (it's stable within a session); reactivity comes from the workspace args. + * An operator gets it only when they may fork and there is no canonical dev: a new fork makes them a + * developer, but in an existing dev workspace they keep whatever role they have there. User identity + * is read non-reactively (it's stable within a session); reactivity comes from the workspace args. */ export function editInForkAllowed( currentWorkspaceId: string | undefined, allWorkspaces: UserWorkspace[] ): boolean { const user = get(userStore) - if (user?.operator) return false - return canCreateFork(user) || !!findCanonicalDevWorkspace(currentWorkspaceId, allWorkspaces) + const dev = findCanonicalDevWorkspace(currentWorkspaceId, allWorkspaces) + if (user?.operator) return canCreateFork(user) && !dev + return canCreateFork(user) || !!dev } /** Label for the affordance: "Edit in " when routed to a canonical dev, else "Edit in fork". */