From 304279f73e50af66fb55e3cceffa4030167cb5fa Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Thu, 22 Oct 2026 07:36:36 -0700 Subject: [PATCH] fix: keep fork out of is_restricted and read it from the caller's role Co-Authored-By: Claude Opus 5.5 (1M context) --- .../windmill-api-workspaces/src/workspaces.rs | 2 +- backend/windmill-common/src/roles.rs | 29 ++++++++++++++++--- docs/workspace-roles.md | 2 +- .../settings/WorkspaceRoleOverrides.svelte | 2 +- frontend/src/lib/roleCapabilities.ts | 9 +++--- 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 5d26a91352..680bf0846d 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -9013,7 +9013,7 @@ async fn check_can_fork(db: &DB, authed: &ApiAuthed, w_id: &str) -> Result<()> { if *DISABLE_WORKSPACE_FORK { require_super_admin(db, authed).await?; } - if !windmill_common::roles::caller_capabilities(db, w_id, authed) + if !windmill_common::roles::caller_role_capabilities(db, w_id, authed) .await? .fork { diff --git a/backend/windmill-common/src/roles.rs b/backend/windmill-common/src/roles.rs index c2caaf97ea..f0eec62f6a 100644 --- a/backend/windmill-common/src/roles.rs +++ b/backend/windmill-common/src/roles.rs @@ -143,11 +143,13 @@ impl RoleCapabilities { } /// Whether a member of this role is gated at all. Every gate skips an unrestricted caller - /// without reading its role, which is what `ApiAuthed::is_operator` carries. Visibility is - /// left out: it only hides pages, and must not turn a full author into an operator for the - /// raw `is_operator` checks, job permissions and credential caches keyed on it. + /// without reading its role, which is what `ApiAuthed::is_operator` carries. Visibility and + /// `fork` are left out: neither may turn a full author into an operator for the raw + /// `is_operator` checks, job permissions and credential caches keyed on it. A role defined + /// before `fork` existed reads it as unset, so it would otherwise turn restricted on upgrade. + /// `fork` is checked with [`caller_role_capabilities`] instead. pub fn is_restricted(&self) -> bool { - Self { visibility: RoleVisibility::ALL, ..*self } != Self::FULL + Self { visibility: RoleVisibility::ALL, fork: true, ..*self } != Self::FULL } /// Whether the role authors deployable artifacts and so costs a full seat rather than half. @@ -714,6 +716,25 @@ async fn member_role(db: &DB, workspace_id: &str, username: &str) -> Result( + db: &DB, + workspace_id: &str, + authed: &A, +) -> Result { + if authed.is_admin() { + return Ok(RoleCapabilities::FULL); + } + match member_role(db, workspace_id, authed.username()).await? { + Some(role) => role_capabilities_by_name(db, workspace_id, &role).await, + None if authed.is_operator() => Ok(RoleCapabilities::NONE), + None => Ok(RoleCapabilities::FULL), + } +} + /// What the caller may do in `workspace_id`. An admin, and any caller authentication did not mark /// restricted, holds everything; a restricted caller holds its role's capabilities, or none if it /// has no membership (a guest, a removed member's job). `workspace_id` MUST be the route's. diff --git a/docs/workspace-roles.md b/docs/workspace-roles.md index 7ebb8b2f7e..3ec59a2519 100644 --- a/docs/workspace-roles.md +++ b/docs/workspace-roles.md @@ -19,7 +19,7 @@ code is `windmill_common::roles`. | `flows` | `none` / `compose` / `full` | yes | | `apps` | `none` / `compose` / `full` | yes; low-code apps and worker-built apps need `full` | | `manage_schedules`, `manage_triggers` | bool | yes, see `docs/operator-write-rights.md` | -| `fork` | bool | yes: both fork endpoints; the forker is a developer in the fork, see `docs/operator-write-rights.md` | +| `fork` | bool | yes: both fork endpoints, read with `caller_role_capabilities`; the forker is a developer in the fork, see `docs/operator-write-rights.md`. Like visibility, not part of `is_restricted` | | `visibility.*` | bool per page | no, UI only | `compose` is the former operator builder right: the composition-only checks of diff --git a/frontend/src/lib/components/settings/WorkspaceRoleOverrides.svelte b/frontend/src/lib/components/settings/WorkspaceRoleOverrides.svelte index 12e1b3e68e..cf91a4255a 100644 --- a/frontend/src/lib/components/settings/WorkspaceRoleOverrides.svelte +++ b/frontend/src/lib/components/settings/WorkspaceRoleOverrides.svelte @@ -58,7 +58,7 @@ function overriddenKeys(o: RoleOverride | undefined): string[] { const c = o?.capabilities ?? {} - const keys = ['code', 'flows', 'apps', 'manage_schedules', 'manage_triggers'].filter( + const keys = ['code', 'flows', 'apps', 'manage_schedules', 'manage_triggers', 'fork'].filter( (k) => c[k] != null ) for (const { key } of VISIBILITY_KEYS) { diff --git a/frontend/src/lib/roleCapabilities.ts b/frontend/src/lib/roleCapabilities.ts index 6790cd380e..8569a490c9 100644 --- a/frontend/src/lib/roleCapabilities.ts +++ b/frontend/src/lib/roleCapabilities.ts @@ -56,17 +56,16 @@ export function capabilitiesIn( return operator ? legacyOperatorCapabilities(workspace?.operator_settings) : FULL_CAPABILITIES } -/** Whether the role is gated at all, as `RoleCapabilities::is_restricted`: visibility is left out, - * since hiding a page must not cost a full author what restricted roles are refused, such as - * forking. */ +/** Whether the role is gated at all, as `RoleCapabilities::is_restricted`: visibility and `fork` + * are left out, since neither must cost a full author what restricted roles are refused. Gate + * forking on `caps.fork` itself. */ export function isRestricted(caps: RoleCapabilities): boolean { return ( !caps.code || caps.flows !== 'full' || caps.apps !== 'full' || !caps.manage_schedules || - !caps.manage_triggers || - !caps.fork + !caps.manage_triggers ) }