From 90540dccdd92e45556b03c64f4bdc67685c878a8 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Fri, 23 Oct 2026 05:33:33 -0700 Subject: [PATCH] fix: check fork rights first and gate edit in fork on the forked workspace's role - create_workspace_fork runs check_can_fork before the stale fork-state purges. - The fork form's default base falls back to a base the role may fork. - Script and flow rows show Edit in fork when the role hides the direct editor. - Run details read the run workspace's capabilities for Edit in fork. - Document that a dev workspace with copy_members keeps the creator's parent role. Co-Authored-By: Claude Opus 5.5 (1M context) --- backend/windmill-api-workspaces/src/workspaces.rs | 3 ++- docs/operator-write-rights.md | 4 +++- frontend/src/lib/components/common/table/FlowRow.svelte | 2 +- frontend/src/lib/components/common/table/ScriptRow.svelte | 2 +- frontend/src/lib/components/runs/RunDetail.svelte | 2 +- .../workspaceSettings/CreateWorkspaceInner.svelte | 7 ++++--- frontend/src/lib/utils/editInFork.ts | 7 ++++--- 7 files changed, 16 insertions(+), 11 deletions(-) diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index c33140da1e..05c49e559a 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -9697,6 +9697,8 @@ async fn create_workspace_fork( Query(q): Query, Json(nw): Json, ) -> Result { + // Ahead of everything below: the stale-state purges must not run for a caller refused here. + check_can_fork(&db, &authed, &parent_workspace_id).await?; enforce_fork_depth(&db, &parent_workspace_id, 0).await?; #[cfg(feature = "cloud")] if *CLOUD_HOSTED { @@ -9790,7 +9792,6 @@ async fn create_workspace_fork( #[cfg(not(feature = "enterprise"))] _check_nb_of_workspaces(&db).await?; - check_can_fork(&db, &authed, &parent_workspace_id).await?; enforce_isolated_forks(&db, &authed, &parent_workspace_id, &nw).await?; if nw.is_dev_workspace { diff --git a/docs/operator-write-rights.md b/docs/operator-write-rights.md index c6e8bf78b5..d856ea46a4 100644 --- a/docs/operator-write-rights.md +++ b/docs/operator-write-rights.md @@ -133,7 +133,9 @@ restricts everyone it does not exempt. `canCreateFork` in `frontend/src/lib/util mirrors it. A member who forks is a developer in the fork whatever their role: the creator row copies only -`is_admin`, so `workspace_role` stays NULL and `operator` keeps its false default. That is the +`is_admin`, so `workspace_role` stays NULL and `operator` keeps its false default. The exception +is a dev workspace created with `copy_members`: the copy brings the creator's parent row, role +included, and the creator row is then a no-op, so they keep their parent role there. That is the point of granting `fork`, a copy to build in, and it means granting it to a restricted role grants that role arbitrary code execution. Deploying scripts, flows and apps back runs with the user's role in the parent, where its write checks refuse what the role cannot author, so an admin or a diff --git a/frontend/src/lib/components/common/table/FlowRow.svelte b/frontend/src/lib/components/common/table/FlowRow.svelte index 870b59c80f..91a6a62e81 100644 --- a/frontend/src/lib/components/common/table/FlowRow.svelte +++ b/frontend/src/lib/components/common/table/FlowRow.svelte @@ -189,7 +189,7 @@ {/if} - {#if !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!showEditButton || !flow.canWrite)} + {#if !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (hideForOperator || !showEditButton || !flow.canWrite)} {/if} diff --git a/frontend/src/lib/components/common/table/ScriptRow.svelte b/frontend/src/lib/components/common/table/ScriptRow.svelte index 1ee5b6dec2..63697f2a04 100644 --- a/frontend/src/lib/components/common/table/ScriptRow.svelte +++ b/frontend/src/lib/components/common/table/ScriptRow.svelte @@ -254,7 +254,7 @@ {/if} {/if} {/if} - {#if !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!showEditButton || !script.canWrite)} + {#if !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces) && (!$roleCapabilities.code || !showEditButton || !script.canWrite)} {/if} diff --git a/frontend/src/lib/components/runs/RunDetail.svelte b/frontend/src/lib/components/runs/RunDetail.svelte index d4eb476189..423d994d07 100644 --- a/frontend/src/lib/components/runs/RunDetail.svelte +++ b/frontend/src/lib/components/runs/RunDetail.svelte @@ -773,7 +773,7 @@ (job?.job_kind === 'script' || job?.job_kind === 'flow') && (!showEditButton || !editRights.capabilities.code) && !isCloudHosted() && - editInForkAllowed(workspaceId, $userWorkspaces) + editInForkAllowed(workspaceId, $userWorkspaces, editRights.capabilities) ) /** Which actions the row has: they come and go as the job runs and completes, often while the * row is folded into the menu, where the bar cannot see them change (`actionsVersion`). */ diff --git a/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte b/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte index a45453700f..120eb4adb4 100644 --- a/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte +++ b/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte @@ -153,9 +153,10 @@ subtitle: w.is_dev_workspace ? 'dev workspace' : w.id === familyRoot?.id ? undefined : 'fork' })) ) - let defaultBaseWorkspaceId = $derived( - findDefaultForkBase($workspaceStore, forkableWorkspaces)?.id - ) + let defaultBaseWorkspaceId = $derived.by(() => { + const id = findDefaultForkBase($workspaceStore, forkableWorkspaces)?.id + return baseCandidates.some((w) => w.id === id) ? id : baseCandidates[0]?.id + }) // Seed the base once the family is known; keep an explicit user choice as long as it stays valid. $effect(() => { if (!isFork) return diff --git a/frontend/src/lib/utils/editInFork.ts b/frontend/src/lib/utils/editInFork.ts index 30e59d57e8..0239cae82c 100644 --- a/frontend/src/lib/utils/editInFork.ts +++ b/frontend/src/lib/utils/editInFork.ts @@ -24,13 +24,14 @@ export type ItemType = 'script' | 'flow' | 'app' | 'raw_app' * A role without `code` gets it only when it may fork and there is no canonical dev: a new fork * makes the member a developer, but in an existing dev workspace they keep their role and have * nothing to edit. User identity is read non-reactively (it's stable within a session); reactivity - * comes from the workspace args. + * comes from the workspace args. Pass `caps` when `currentWorkspaceId` is not the navigation + * workspace: `$roleCapabilities` describes only that one. */ export function editInForkAllowed( currentWorkspaceId: string | undefined, - allWorkspaces: UserWorkspace[] + allWorkspaces: UserWorkspace[], + caps: RoleCapabilities = get(roleCapabilities) ): boolean { - const caps = get(roleCapabilities) const user = get(userStore) const dev = findCanonicalDevWorkspace(currentWorkspaceId, allWorkspaces) if (!caps.code) return canCreateFork(user, caps) && !dev