diff --git a/AGENTS.md b/AGENTS.md index f4bbfec6ce..54b5953748 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,6 +37,8 @@ Open-source platform for internal tools, workflows, API integrations, background - **Operator write rights**: `docs/operator-write-rights.md` — which `operator_settings` flags are enforced rather than cosmetic, and why a right that is granted-unless-withdrawn needs `Option` fields and a jsonb merge rather than a serde default +- **Operator builder rights**: `docs/operator-builder-rights.md` — the workspace setting that lets + operators compose flows, what the composition check must cover, and why it costs a full seat - **Auth surface**: `docs/auth-surface.md` — credential precedence, session/cache invalidation scope, which token labels email their owner at expiry, how OAuth login matches `login_type`, and that every superadmin route refuses `$WM_TOKEN`. Read before designing anything that creates diff --git a/backend/.sqlx/query-094a07eaa5714ec739786717f110ea539e84c33de23476e042a4a1a7cb529dac.json b/backend/.sqlx/query-094a07eaa5714ec739786717f110ea539e84c33de23476e042a4a1a7cb529dac.json new file mode 100644 index 0000000000..e0174943d6 --- /dev/null +++ b/backend/.sqlx/query-094a07eaa5714ec739786717f110ea539e84c33de23476e042a4a1a7cb529dac.json @@ -0,0 +1,34 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT COALESCE((operator_settings->>'builder_flows')::boolean, false) AS \"flows!\",\n COALESCE((operator_settings->>'manage_schedules')::boolean, true) AS \"schedules!\",\n COALESCE((operator_settings->>'manage_triggers')::boolean, true) AS \"triggers!\"\n FROM workspace_settings WHERE workspace_id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "flows!", + "type_info": "Bool" + }, + { + "ordinal": 1, + "name": "schedules!", + "type_info": "Bool" + }, + { + "ordinal": 2, + "name": "triggers!", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + null, + null, + null + ] + }, + "hash": "094a07eaa5714ec739786717f110ea539e84c33de23476e042a4a1a7cb529dac" +} diff --git a/backend/.sqlx/query-298e0e30afdc973c517a9b4ea99f5dd1616a62c6ecbaaea312c686038d21e6fd.json b/backend/.sqlx/query-298e0e30afdc973c517a9b4ea99f5dd1616a62c6ecbaaea312c686038d21e6fd.json new file mode 100644 index 0000000000..9d396e3a33 --- /dev/null +++ b/backend/.sqlx/query-298e0e30afdc973c517a9b4ea99f5dd1616a62c6ecbaaea312c686038d21e6fd.json @@ -0,0 +1,24 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2 AND hash = $3)", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "exists", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Int8" + ] + }, + "nullable": [ + null + ] + }, + "hash": "298e0e30afdc973c517a9b4ea99f5dd1616a62c6ecbaaea312c686038d21e6fd" +} diff --git a/backend/.sqlx/query-3dccc8e745f4a0973541088f66172e74af6828c1ab52cf7a6fd10b305deade85.json b/backend/.sqlx/query-3dccc8e745f4a0973541088f66172e74af6828c1ab52cf7a6fd10b305deade85.json new file mode 100644 index 0000000000..c6e572863a --- /dev/null +++ b/backend/.sqlx/query-3dccc8e745f4a0973541088f66172e74af6828c1ab52cf7a6fd10b305deade85.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT EXISTS(SELECT 1 FROM flow WHERE workspace_id = $1 AND path = $2)", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "exists", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "3dccc8e745f4a0973541088f66172e74af6828c1ab52cf7a6fd10b305deade85" +} diff --git a/backend/.sqlx/query-4bc47050c74a02ab3169c3165898b2af07e995de71564d867b171f97f719fbde.json b/backend/.sqlx/query-4bc47050c74a02ab3169c3165898b2af07e995de71564d867b171f97f719fbde.json new file mode 100644 index 0000000000..9b2b79808d --- /dev/null +++ b/backend/.sqlx/query-4bc47050c74a02ab3169c3165898b2af07e995de71564d867b171f97f719fbde.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2)", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "exists", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "4bc47050c74a02ab3169c3165898b2af07e995de71564d867b171f97f719fbde" +} diff --git a/backend/.sqlx/query-5312b8db714139a94d7ff1c0794af063c36ac17e9d331cd9980b91b28d713c72.json b/backend/.sqlx/query-5312b8db714139a94d7ff1c0794af063c36ac17e9d331cd9980b91b28d713c72.json deleted file mode 100644 index 35b70a239c..0000000000 --- a/backend/.sqlx/query-5312b8db714139a94d7ff1c0794af063c36ac17e9d331cd9980b91b28d713c72.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT bool_and(operator) FROM (\n SELECT operator FROM usr WHERE email = $1 AND is_service_account IS false\n UNION ALL\n SELECT operator FROM workspace_invite WHERE email = $1\n ) t", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "bool_and", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "5312b8db714139a94d7ff1c0794af063c36ac17e9d331cd9980b91b28d713c72" -} diff --git a/backend/.sqlx/query-4afdc63af51e599b9d6fe6cedcd7980fd761a29df0f2a6820f3c8d7a29c3c20a.json b/backend/.sqlx/query-cbc521b505c3953dc624ae6cefe6999e6d0c732da83ebc74ef7678cacd22ad6b.json similarity index 53% rename from backend/.sqlx/query-4afdc63af51e599b9d6fe6cedcd7980fd761a29df0f2a6820f3c8d7a29c3c20a.json rename to backend/.sqlx/query-cbc521b505c3953dc624ae6cefe6999e6d0c732da83ebc74ef7678cacd22ad6b.json index 02a2cb9973..a2a87b680d 100644 --- a/backend/.sqlx/query-4afdc63af51e599b9d6fe6cedcd7980fd761a29df0f2a6820f3c8d7a29c3c20a.json +++ b/backend/.sqlx/query-cbc521b505c3953dc624ae6cefe6999e6d0c732da83ebc74ef7678cacd22ad6b.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "WITH active_users as (SELECT distinct username as email FROM audit_partitioned WHERE timestamp > NOW() - INTERVAL '1 month' AND (operation = 'users.login' OR operation = 'oauth.login' OR operation = 'users.token.refresh') AND username NOT IN (SELECT email FROM usr WHERE is_service_account)),\n active_authors as (SELECT distinct email FROM usr WHERE usr.operator IS false AND email IN (SELECT email FROM active_users)),\n active_authors_agg as (SELECT array_agg(email) as authors FROM active_authors),\n active_ops_agg as (SELECT array_agg(email) as operators from active_users WHERE email NOT IN (SELECT email FROM active_authors))\n SELECT active_authors_agg.authors, active_ops_agg.operators, array_length(active_authors_agg.authors, 1) as author_count, array_length(active_ops_agg.operators, 1) as operator_count FROM active_authors_agg, active_ops_agg", + "query": "WITH active_users as (SELECT distinct username as email FROM audit_partitioned WHERE timestamp > NOW() - INTERVAL '1 month' AND (operation = 'users.login' OR operation = 'oauth.login' OR operation = 'users.token.refresh') AND username NOT IN (SELECT email FROM usr WHERE is_service_account)),\n active_authors as (SELECT distinct t.email FROM usr t LEFT JOIN workspace_settings ws ON ws.workspace_id = t.workspace_id WHERE NOT (t.operator AND NOT COALESCE((ws.operator_settings->>'builder_flows')::boolean, false)) AND t.email IN (SELECT email FROM active_users)),\n active_authors_agg as (SELECT array_agg(email) as authors FROM active_authors),\n active_ops_agg as (SELECT array_agg(email) as operators from active_users WHERE email NOT IN (SELECT email FROM active_authors))\n SELECT active_authors_agg.authors, active_ops_agg.operators, array_length(active_authors_agg.authors, 1) as author_count, array_length(active_ops_agg.operators, 1) as operator_count FROM active_authors_agg, active_ops_agg", "describe": { "columns": [ { @@ -34,5 +34,5 @@ null ] }, - "hash": "4afdc63af51e599b9d6fe6cedcd7980fd761a29df0f2a6820f3c8d7a29c3c20a" + "hash": "cbc521b505c3953dc624ae6cefe6999e6d0c732da83ebc74ef7678cacd22ad6b" } diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 4cfbcb8447..076b313eb6 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -5a2b6b8527250bd12cf856d8a667a9ef3106ec60 +40ac1c5f8cbce3843b582d9b392d3f3cc7eca3e6 diff --git a/backend/tests/inline_preview_auth.rs b/backend/tests/inline_preview_auth.rs index 8841719962..acf511c26c 100644 --- a/backend/tests/inline_preview_auth.rs +++ b/backend/tests/inline_preview_auth.rs @@ -18,7 +18,7 @@ //! //! This test pins down: //! - an Operator's own token is rejected by the operator guard (the core fix; -//! pre-fix this reached the inline executor instead of returning 401), +//! pre-fix this reached the inline executor instead of returning 403), //! - a regular non-operator passes the guard (the fix must not over-block the //! legitimate inline preview flow): in the test harness the worker inline //! utils are not registered, so a caller past the guard gets the distinct @@ -121,10 +121,10 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result // 1. CORE REGRESSION: an Operator must be rejected by the operator guard. // Pre-fix this fell through to the inline executor (arbitrary code - // execution); post-fix it returns 401 with the operator guard message. + // execution); post-fix it returns 403 with the operator guard message. let (status, body) = post(&url, "OPERATOR_TOKEN", &inline_preview_body()).await; assert_eq!( - status, 401, + status, 403, "Operator must be rejected from inline preview (got {status}): {body}" ); assert!( @@ -139,7 +139,7 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result // the operator guard did not reject it. let (status, body) = post(&url, "SECRET_TOKEN_2", &inline_preview_body()).await; assert_ne!( - status, 401, + status, 403, "non-operator must not be blocked by the operator guard (got {status}): {body}" ); assert!( @@ -155,7 +155,7 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result operator_job_token(uuid::Uuid::parse_str(RUNNING_JOB_ID).unwrap()).await; let (status, body) = post(&url, &running_job_token, &datatable_query_body()).await; assert_ne!( - status, 401, + status, 403, "operator job token of a running job must pass the guard for a datatable query (got {status}): {body}" ); assert!( @@ -188,7 +188,7 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result ] { let (status, body) = post(&url, &running_job_token, &payload).await; assert_eq!( - status, 401, + status, 403, "operator job token must be rejected for a {label} payload (got {status}): {body}" ); assert!( @@ -208,7 +208,7 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result let token = operator_job_token(job_id).await; let (status, body) = post(&url, &token, &datatable_query_body()).await; assert_eq!( - status, 401, + status, 403, "operator job token of a {label} job must be rejected (got {status}): {body}" ); assert!( @@ -229,7 +229,7 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result ); let (status, body) = post(&fallback_url, "OPERATOR_TOKEN", &datatable_query_body()).await; assert_eq!( - status, 401, + status, 403, "Operator must be rejected from the preview fallback (got {status}): {body}" ); assert!( @@ -246,7 +246,7 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result let deferred_url = format!("{fallback_url}?{deferral}"); let (status, body) = post(&deferred_url, &running_job_token, &datatable_query_body()).await; assert_eq!( - status, 401, + status, 403, "operator job token must not schedule a deferred preview with {deferral} (got {status}): {body}" ); assert!( diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index c7df69879e..aa86a7ff07 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -16,10 +16,12 @@ use axum::{ }; use windmill_api_auth::{ auth::{list_tokens_internal, TruncatedTokenWithEmail}, - build_scope_path_predicate, check_scopes, maybe_refresh_folders, require_owner_of_path, - ApiAuthed, + build_scope_path_predicate, check_scopes, get_scope_tags, maybe_refresh_folders, + require_owner_of_path, ApiAuthed, +}; +use windmill_common::workspaces::{ + check_deploy_rules, check_operator_can_build_flows, operator_can_build_flows, RuleCheckResult, }; -use windmill_common::workspaces::{check_deploy_rules, RuleCheckResult}; use windmill_common::{ user_drafts::{overlay_or_draft_only, DraftUserRef, UserDraftItemKind, WithDraftOverlay}, utils::HTTP_CLIENT, @@ -35,7 +37,7 @@ use sqlx::{FromRow, Postgres, Transaction}; use windmill_audit::audit_oss::{audit_log, AuditAuthorable}; use windmill_audit::ActionKind; use windmill_common::assets::{clear_static_asset_usage, AssetUsageKind}; -use windmill_common::flows::FlowModule; +use windmill_common::flows::{FlowModule, FlowValue}; use windmill_common::min_version::{ MIN_VERSION_SUPPORTS_DEBOUNCING, MIN_VERSION_SUPPORTS_DEBOUNCING_V2, MIN_VERSION_SUPPORTS_NODE_DEBOUNCING, @@ -241,7 +243,7 @@ async fn list_flows( // Append the authed user's drafts at paths with no deployed flow; see scripts.rs. if lq.include_draft_only.unwrap_or(false) - && !authed.is_operator + && (!authed.is_operator || operator_can_build_flows(&db, &w_id).await?) && offset == 0 && lq.path_start.is_none() && lq.path_exact.is_none() @@ -570,7 +572,13 @@ async fn list_paths_linking_agent( Ok(Json(flows)) } -async fn validate_flow(new_flow: &NewFlow) -> error::Result<()> { +async fn validate_flow( + new_flow: &NewFlow, + authed: &ApiAuthed, + db: &DB, + user_db: &UserDB, + w_id: &str, +) -> error::Result<()> { #[cfg(not(feature = "enterprise"))] if new_flow.ws_error_handler_muted.is_some_and(|val| val) { return Err(Error::BadRequest( @@ -581,9 +589,139 @@ async fn validate_flow(new_flow: &NewFlow) -> error::Result<()> { guard_flow_from_debounce_data(new_flow).await?; + if authed.is_operator { + validate_operator_flow( + &new_flow.parse_flow_value()?, + &new_flow.tag, + new_flow.schema.as_ref().map(|s| s.0.get()), + authed, + db, + user_db, + w_id, + ) + .await?; + } + return Ok(()); } +/// What an operator with builder rights must pass to store a flow, deployed or as a draft: a +/// developer who loads a builder's draft in the editor runs its code as themselves. +pub async fn validate_operator_flow( + value: &FlowValue, + flow_tag: &Option, + schema: Option<&str>, + authed: &ApiAuthed, + db: &DB, + user_db: &UserDB, + w_id: &str, +) -> error::Result<()> { + // Dynamic dropdown code runs as whoever loads the flow's form: it is code like a step's. + if let Some(schema) = schema { + let schema: serde_json::Value = serde_json::from_str(schema)?; + if schema.get("x-windmill-dyn-select-code").is_some() { + return Err(Error::PermissionDenied( + "This flow has dynamic dropdown code, so only a developer can edit it".to_string(), + )); + } + } + validate_operator_composed_flow(value, flow_tag, authed, db, user_db, w_id).await +} + +/// Runs on every write and every preview of a flow authored by an operator with builder rights. +/// The walk in `check_flow_is_composition_only` only sees the value; what it collects is +/// authorized here against the caller's own permissions. +pub async fn validate_operator_composed_flow( + value: &FlowValue, + flow_tag: &Option, + authed: &ApiAuthed, + db: &DB, + user_db: &UserDB, + w_id: &str, +) -> error::Result<()> { + let mut refs = windmill_common::flows::check_flow_is_composition_only(value)?; + + // A tag is how a step picks the worker group it runs on: unauthorized, a builder could route + // a job onto a privileged one. + refs.tags.extend(flow_tag.clone().filter(|t| !t.is_empty())); + if !refs.tags.is_empty() { + // Job-aware: a WM_TOKEN running as a superadmin must not unlock restricted tags. + let is_super_admin = windmill_api_auth::is_super_admin_authed(db, authed).await?; + for tag in &refs.tags { + windmill_common::jobs::check_tag_available_for_workspace_internal( + db, + w_id, + tag, + None, + std::future::ready(w_id.to_string()), + is_super_admin, + get_scope_tags(authed), + ) + .await?; + } + } + + if refs.runnables.is_empty() && refs.pinned_scripts.is_empty() { + return Ok(()); + } + // A flow can step through the same script thirty times; this runs on every write, preview and + // dependency job. + refs.runnables.sort(); + refs.runnables.dedup(); + refs.pinned_scripts + .sort_by_key(|(path, hash)| (path.clone(), hash.0)); + refs.pinned_scripts + .dedup_by_key(|(path, hash)| (path.clone(), hash.0)); + // The worker resolves a step's path with the root DB handle and runs it as that runnable's + // `on_behalf_of`, so composing an unreadable path would run code the builder cannot see. RLS + // on this transaction is the check. + let mut tx = user_db.clone().begin(authed).await?; + for (is_flow, path) in &refs.runnables { + let readable = if *is_flow { + sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM flow WHERE workspace_id = $1 AND path = $2)", + w_id, + path, + ) + } else { + sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2)", + w_id, + path, + ) + } + .fetch_one(&mut *tx) + .await? + .unwrap_or(false); + if !readable { + return Err(Error::PermissionDenied(format!( + "{} {path} does not exist or is not readable by you", + if *is_flow { "Flow" } else { "Script" } + ))); + } + } + // A pinned step is dispatched by its hash alone, ignoring the path beside it, so a readable + // path paired with another script's hash would still run that other script. + for (path, hash) in &refs.pinned_scripts { + let exists = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2 AND hash = $3)", + w_id, + path, + hash.0, + ) + .fetch_one(&mut *tx) + .await? + .unwrap_or(false); + if !exists { + return Err(Error::PermissionDenied(format!( + "Version {hash} is not a readable version of {path}" + ))); + } + } + tx.commit().await?; + Ok(()) +} + async fn create_flow( authed: ApiAuthed, Extension(db): Extension, @@ -592,11 +730,7 @@ async fn create_flow( Path(w_id): Path, Json(mut nf): Json, ) -> Result<(StatusCode, String)> { - if authed.is_operator { - return Err(Error::NotAuthorized( - "Operators cannot create flows for security reasons".to_string(), - )); - } + check_operator_can_build_flows(&db, &w_id, authed.is_operator, "create flows").await?; check_scopes(&authed, || format!("flows:write:{}", nf.path))?; // A `<= 0` flow timeout is "unset", not a 0-second limit that kills every run instantly. @@ -616,7 +750,7 @@ async fn create_flow( return Err(Error::PermissionDenied(msg)); } - validate_flow(&nf).await?; + validate_flow(&nf, &authed, &db, &user_db, &w_id).await?; if *CLOUD_HOSTED { let nb_flows = sqlx::query_scalar!("SELECT COUNT(*) FROM flow WHERE workspace_id = $1", &w_id) @@ -1187,11 +1321,7 @@ async fn update_flow( Path((w_id, flow_path)): Path<(String, StripPath)>, Json(ef): Json, ) -> Result { - if authed.is_operator { - return Err(Error::NotAuthorized( - "Operators cannot update flows for security reasons".to_string(), - )); - } + check_operator_can_build_flows(&db, &w_id, authed.is_operator, "update flows").await?; let flow_path = flow_path.to_path(); // The URL identifies the flow being updated; the body path is only needed to rename. let mut nf = ef.into_new_flow(flow_path); @@ -1218,7 +1348,7 @@ async fn update_flow( return Err(Error::PermissionDenied(msg)); } - validate_flow(&nf).await?; + validate_flow(&nf, &authed, &db, &user_db, &w_id).await?; let authed = maybe_refresh_folders(&flow_path, &w_id, authed, &db).await; let mut tx = user_db.clone().begin(&authed).await?; @@ -1855,11 +1985,7 @@ async fn archive_flow_by_path( Path((w_id, path)): Path<(String, StripPath)>, Json(archived): Json, ) -> Result { - if authed.is_operator { - return Err(Error::NotAuthorized( - "Operators cannot archive flows for security reasons".to_string(), - )); - } + check_operator_can_build_flows(&db, &w_id, authed.is_operator, "archive flows").await?; let path = path.to_path(); check_scopes(&authed, || format!("flows:write:{}", path))?; if let RuleCheckResult::Blocked(msg) = check_deploy_rules( @@ -2000,11 +2126,7 @@ async fn delete_flow_by_path( Path((w_id, path)): Path<(String, StripPath)>, Query(query): Query, ) -> Result { - if authed.is_operator { - return Err(Error::NotAuthorized( - "Operators cannot delete flows for security reasons".to_string(), - )); - } + check_operator_can_build_flows(&db, &w_id, authed.is_operator, "delete flows").await?; let path = path.to_path(); check_scopes(&authed, || format!("flows:write:{}", path))?; if let RuleCheckResult::Blocked(msg) = check_deploy_rules( diff --git a/backend/windmill-api-integration-tests/tests/operator_builder_flows.rs b/backend/windmill-api-integration-tests/tests/operator_builder_flows.rs new file mode 100644 index 0000000000..7be3920124 --- /dev/null +++ b/backend/windmill-api-integration-tests/tests/operator_builder_flows.rs @@ -0,0 +1,317 @@ +use serde_json::json; +use sqlx::{Pool, Postgres}; +use windmill_common::workspaces::invalidate_operator_rights_cache; +use windmill_test_utils::*; + +const WS: &str = "test-workspace"; + +fn operator_client() -> reqwest::Client { + let mut headers = reqwest::header::HeaderMap::new(); + headers.insert( + reqwest::header::AUTHORIZATION, + reqwest::header::HeaderValue::from_str("Bearer OPERATOR_TOKEN_1").unwrap(), + ); + reqwest::ClientBuilder::new() + .default_headers(headers) + .build() + .unwrap() +} + +async fn set_builder(db: &Pool, flows: bool) -> anyhow::Result<()> { + sqlx::query( + "UPDATE workspace_settings SET operator_settings = $1::text::jsonb WHERE workspace_id = $2", + ) + .bind(format!(r#"{{"builder_flows": {flows}}}"#)) + .bind(WS) + .execute(db) + .await?; + // The right is read through a process-global 60s cache keyed by workspace id. + invalidate_operator_rights_cache(WS); + Ok(()) +} + +async fn add_script(db: &Pool, hash: i64, path: &str, owner: &str) -> anyhow::Result<()> { + sqlx::query( + "INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema, + summary, description, lock, extra_perms) + VALUES ($1, $2, $3, 'x', 'bun', 'script', $4, '{}', '', '', '', '{}')", + ) + .bind(WS) + .bind(hash) + .bind(path) + .bind(owner) + .execute(db) + .await?; + Ok(()) +} + +fn composition_flow_at(path: &str, step_path: &str) -> serde_json::Value { + json!({ + "path": path, + "summary": "", + "description": "", + "schema": {}, + "value": {"modules": [{ + "id": "a", + "value": {"type": "script", "path": step_path, "input_transforms": {}} + }]} + }) +} + +fn inline_code_flow(path: &str) -> serde_json::Value { + json!({ + "path": path, + "summary": "", + "description": "", + "schema": {}, + "value": {"modules": [{ + "id": "a", + "value": { + "type": "rawscript", + "content": "export async function main() { return 1 }", + "language": "bun", + "input_transforms": {} + } + }]} + }) +} + +/// The whole boundary in one pass: the builder right lets an operator compose runnables that are +/// already deployed and nothing more, and the endpoints that author code stay shut whether or not +/// it is granted. +#[sqlx::test(migrations = "../migrations", fixtures("base", "permissions_test"))] +async fn test_operator_builder_flows_boundary(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let api = format!("http://localhost:{port}/api/w/{WS}"); + let c = operator_client(); + + // A composition-only flow references a runnable that exists and the builder can read, so the + // fixture needs one. + add_script(&db, 4241, "u/operator/some_script", "operator").await?; + + set_builder(&db, false).await?; + let resp = c + .post(format!("{api}/flows/create")) + .json(&composition_flow_at( + "u/operator/f1", + "u/operator/some_script", + )) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "an operator without the builder right must not create a flow" + ); + + set_builder(&db, true).await?; + // A payload that omits the key, like a git-sync file that predates it, keeps the right. + let resp = reqwest::Client::new() + .post(format!("{api}/workspaces/operator_settings")) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&json!({"runs": true})) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + + let resp = c + .post(format!("{api}/flows/create")) + .json(&composition_flow_at( + "u/operator/f1", + "u/operator/some_script", + )) + .send() + .await?; + assert!( + resp.status().is_success(), + "a builder must be able to create a composition-only flow: {}", + resp.text().await? + ); + + let mut tagged = composition_flow_at("u/operator/f5", "u/operator/some_script"); + tagged["tag"] = json!("privileged_group"); + let resp = c + .post(format!("{api}/flows/create")) + .json(&tagged) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "a builder must not route a flow onto a worker tag the workspace cannot use" + ); + + let mut with_dyn_code = composition_flow_at("u/operator/f1", "u/operator/some_script"); + with_dyn_code["schema"] = + json!({"x-windmill-dyn-select-code": "x", "x-windmill-dyn-select-lang": "bun"}); + let resp = c + .post(format!("{api}/flows/update/u/operator/f1")) + .json(&with_dyn_code) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "a builder must not save a flow carrying dropdown code" + ); + let resp = c + .post(format!("{api}/jobs/run/dynamic_select")) + .json( + &json!({"entrypoint_function": "f", "runnable_ref": {"source": "inline", "code": "x"}}), + ) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "an operator's inline dropdown refusal must not be a 401" + ); + + // A dependency job rewrites bookkeeping stored under the path it names, so a builder must not + // aim one at a path it cannot write. + let resp = c + .post(format!("{api}/jobs/run/flow_dependencies")) + .json(&json!({"path": "u/alice/private_flow", "flow_value": {"modules": []}})) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "a builder must not run a dependency job on a path it cannot write" + ); + + // A legacy draft has no owner, so it lists as the builder's own, and a script draft is one + // the builder cannot write: that row must read as not writable, not fail the whole list. + sqlx::query( + "INSERT INTO draft (workspace_id, path, typ, value) VALUES ($1, 'u/operator/some_script', 'script', '{}')", + ) + .bind(WS) + .execute(&db) + .await?; + let resp = c.get(format!("{api}/drafts/list")).send().await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + let drafts: serde_json::Value = resp.json().await?; + assert_eq!(drafts[0]["can_write"], false, "{drafts}"); + + let resp = c + .post(format!("{api}/flows/create")) + .json(&inline_code_flow("u/operator/f2")) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "a builder must not deploy a flow carrying inline code" + ); + + // Draft storage strips a NUL, turning these keys into the plain ones the check reads. + let mut nul_value = inline_code_flow("u/operator/f1"); + let v = nul_value.as_object_mut().unwrap().remove("value").unwrap(); + nul_value["value\0"] = v; + let mut nul_dyn = composition_flow_at("u/operator/f1", "u/operator/some_script"); + nul_dyn["schema"] = json!({"x-windmill-dyn-select-code\0": "x"}); + for (draft, expected) in [ + ( + composition_flow_at("u/operator/f1", "u/operator/some_script"), + 200, + ), + (inline_code_flow("u/operator/f1"), 403), + (with_dyn_code.clone(), 403), + (nul_value, 403), + (nul_dyn, 403), + ] { + let resp = c + .post(format!("{api}/drafts/update/flow/u/operator/f1")) + .json(&json!({ "value": draft, "force": true })) + .send() + .await?; + let status = resp.status(); + assert_eq!( + status, + expected, + "flow draft {draft}: {}", + resp.text().await? + ); + } + + // Same for the preview path, which runs a request-supplied flow value rather than a stored one. + let resp = c + .post(format!("{api}/jobs/run/preview_flow")) + .json(&json!({"value": inline_code_flow("u/operator/f2")["value"], "args": {}})) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "a builder must not preview a flow carrying inline code" + ); + + // Authoring code directly stays shut with the right granted. + let resp = c + .post(format!("{api}/scripts/create")) + .json(&json!({ + "path": "u/operator/s1", + "summary": "", + "description": "", + "content": "export async function main() { return 1 }", + "language": "bun", + "is_template": false + })) + .send() + .await?; + assert!( + !resp.status().is_success(), + "a builder must not create a script" + ); + + // `permissions_test` gives the operator fixture no rights on `u/alice/**`. + add_script(&db, 4243, "u/alice/private", "alice").await?; + let resp = c + .post(format!("{api}/flows/create")) + .json(&composition_flow_at("u/operator/f4", "u/alice/private")) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "a builder must not compose a runnable it cannot read" + ); + + add_script(&db, 4242, "u/operator/pinned", "operator").await?; + let pinned = |hash: &str| { + json!({ + "path": "u/operator/f3", "summary": "", "description": "", "schema": {}, + "value": {"modules": [{ + "id": "a", + "value": { + "type": "script", "path": "u/operator/pinned", "hash": hash, + "input_transforms": {} + } + }]} + }) + }; + let resp = c + .post(format!("{api}/flows/create")) + .json(&pinned("0000000000000000")) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "a builder must not pin a hash that is not a version of the step's path" + ); + let resp = c + .post(format!("{api}/flows/create")) + .json(&pinned("0000000000001092")) + .send() + .await?; + assert!( + resp.status().is_success(), + "a builder must be able to pin the real version of a readable script: {}", + resp.text().await? + ); + + Ok(()) +} diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 77c8e1701f..8c891bf0cf 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3471,7 +3471,6 @@ fn apply_pg_tls_env( Ok(None) } - #[cfg(test)] mod pg_tls_env_tests { use super::apply_pg_tls_env; @@ -4242,7 +4241,9 @@ async fn edit_datatable_config( // the other managed cluster, one whose role ids name nothing in that cluster's catalog. // Refuse it instead: turning roles off first is one step, and it keeps discarding an access // decision something somebody chose rather than a side effect of moving a database. - let old_kind = old.and_then(|old| old.database.as_ref()).map(|d| d.resource_type); + let old_kind = old + .and_then(|old| old.database.as_ref()) + .map(|d| d.resource_type); if dt.permissions.is_some() && dt .database @@ -10871,8 +10872,12 @@ async fn invite_user( nu.email = nu.email.to_lowercase(); #[cfg(feature = "enterprise")] - if let Some(msg) = - windmill_common::ee_oss::check_seat_cap_for_new_user(&db, &nu.email, nu.operator).await? + if let Some(msg) = windmill_common::ee_oss::check_seat_cap_for_new_user( + &db, + &nu.email, + windmill_common::workspaces::consumes_operator_seat(&db, &w_id, nu.operator).await?, + ) + .await? { return Err(Error::BadRequest(msg)); } @@ -11025,8 +11030,12 @@ async fn add_user( }; #[cfg(feature = "enterprise")] - if let Some(msg) = - windmill_common::ee_oss::check_seat_cap_for_new_user(&db, &nu.email, nu.operator).await? + if let Some(msg) = windmill_common::ee_oss::check_seat_cap_for_new_user( + &db, + &nu.email, + windmill_common::workspaces::consumes_operator_seat(&db, &w_id, nu.operator).await?, + ) + .await? { return Err(Error::BadRequest(msg)); } @@ -11589,10 +11598,15 @@ struct ChangeOperatorSettings { folders: bool, #[serde(default)] workers: bool, - /// Writes operators may perform unless withdrawn, so `None` (key absent) must mean "leave as - /// stored" rather than a value: the row is merged, not overwritten, and this endpoint takes - /// whole-object payloads from git-sync files that predate the key. Defaulting either way here - /// would make an older file silently withdraw or restore the right on every pull. + /// Write rights, so `None` (key absent) must mean "leave as stored" rather than a value: the + /// row is merged, not overwritten, and this endpoint takes whole-object payloads from git-sync + /// files that predate the key. Defaulting either way here would make an older file silently + /// withdraw or grant the right on every push. + /// + /// `builder_flows` lets every operator of this workspace compose flows out of already-deployed + /// runnables, and makes each of them consume a full author seat instead of half of one. + #[serde(default, skip_serializing_if = "Option::is_none")] + builder_flows: Option, #[serde(default, skip_serializing_if = "Option::is_none")] manage_schedules: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -11607,6 +11621,17 @@ async fn update_operator_settings( ) -> Result { require_admin(authed.is_admin, &authed.username)?; + // Every operator of the workspace turns into a full seat, which an offline license may not + // cover. It is a no-op delta when the right is already on. + #[cfg(feature = "enterprise")] + if settings.builder_flows == Some(true) { + if let Some(msg) = + windmill_common::ee_oss::check_seat_cap_for_operator_builder(&db, &w_id).await? + { + return Err(Error::BadRequest(msg)); + } + } + let mut tx = db.begin().await?; let settings_json = serde_json::json!(settings); diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index f3c4c24232..3cdd07b280 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -37549,6 +37549,9 @@ components: workers: type: boolean description: Whether operators can view workers page + builder_flows: + type: boolean + description: Whether operators can compose flows out of existing runnables (consumes a full seat). Omitting the field leaves the stored value unchanged. manage_schedules: type: boolean description: Whether operators can create, edit and delete schedules. Granted unless withdrawn; omitting the field leaves the stored value unchanged. diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index 8bcbb56384..7b2268e38f 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -14,13 +14,16 @@ use axum::{ Json, Router, }; use serde::{Deserialize, Serialize}; +use windmill_api_flows::flows::validate_operator_flow; use windmill_common::{ db::UserDB, error::{Error, Result}, + flows::FlowValue, user_drafts::{DraftUserRef, UserDraftItemKind, ENCRYPTED_DRAFT_PREFIX}, users::resolve_username_to_email, utils::{check_proper_path, strip_json_nul}, variables::{build_crypt, encrypt}, + workspaces::operator_can_build_flows, }; pub fn workspaced_service() -> Router { @@ -101,10 +104,10 @@ async fn list_drafts( Path(w_id): Path, Query(query): Query, ) -> Result>> { - // Operators have no drafts of their own (they can't write any, see - // `require_can_write_path`), so this list is always empty for them. They - // can still READ some collaborators' drafts via `/drafts/get`. - if authed.is_operator { + // Without builder rights an operator has no drafts of their own (they can't write any, see + // `require_can_write_path`), so this list is always empty for them. They can still READ some + // collaborators' drafts via `/drafts/get`. + if authed.is_operator && !operator_can_build_flows(&db, &w_id).await? { return Ok(Json(vec![])); } let all_users = query.all_users.unwrap_or(false); @@ -154,10 +157,13 @@ async fn list_drafts( { Ok(()) => true, // A stored draft can sit at an unwritable path — unauthorized, - // or malformed (`BadRequest`; the `draft` table has no path - // constraint). Either way it's not writable, and one bad row - // must not 400 the whole listing. - Err(Error::NotAuthorized(_)) | Err(Error::BadRequest(_)) => false, + // refused to an operator (`PermissionDenied`), or malformed + // (`BadRequest`; the `draft` table has no path constraint). + // Either way it's not writable, and one bad row must not fail + // the whole listing. + Err(Error::NotAuthorized(_)) + | Err(Error::PermissionDenied(_)) + | Err(Error::BadRequest(_)) => false, Err(e) => return Err(e), }; out.push(row); @@ -464,6 +470,32 @@ async fn update_draft( } } + if authed.is_operator && kind == UserDraftItemKind::Flow { + if let Some(value) = &req.value { + #[derive(Deserialize)] + struct FlowDraft { + #[serde(default)] + value: FlowValue, + schema: Option>, + tag: Option, + } + // The text stored below, NULs stripped: a stripped NUL can rename a key into one + // this check reads. + let draft: FlowDraft = serde_json::from_str(&strip_json_nul(value.0.get())) + .map_err(|e| Error::BadRequest(format!("Invalid flow draft: {e}")))?; + validate_operator_flow( + &draft.value, + &draft.tag, + draft.schema.as_deref().map(|s| s.get()), + &authed, + &db, + &user_db, + &w_id, + ) + .await?; + } + } + let applied = if let Some(value) = &req.value { // Secret variable values must never sit in `draft.value` in plaintext // (see `encrypt_secret_variable_value`). @@ -1111,13 +1143,14 @@ fn table_for_kind(kind: UserDraftItemKind) -> Option<&'static str> { } /// Resolves to `Ok(())` if `authed` may SAVE a draft at `path`. Operators are -/// rejected outright. Two layers: +/// rejected, except for flow drafts in a workspace that granted them builder +/// rights. Two layers: /// 1. Claim-based namespace rules (admin, own `u/`, member `g/`, writable /// `f/`) — mirror what RLS reads from the same JWT claims, and are the /// ENTIRE check for draft-only paths (no deployed row for RLS to use). /// 2. An RLS write-probe on the deployed row (`SELECT ... FOR UPDATE`) for /// what the path can't answer, above all item-level extra_perms grants. -async fn require_can_write_path( +pub(crate) async fn require_can_write_path( authed: &ApiAuthed, db: &DB, user_db: &UserDB, @@ -1128,14 +1161,19 @@ async fn require_can_write_path( if authed.is_admin { return Ok(()); } - // Operators are read-only and never WRITE drafts. Read access is - // deliberately asymmetric: `require_can_read_path` has no operator block, - // so an operator can still READ a draft they can read via `/drafts/get`, - // mirroring their read access to deployed content. Intended. + // Operators are read-only and never WRITE drafts, except a flow draft where the workspace + // granted the builder right: the kind has to be checked, or the right would open drafts of + // kinds it says nothing about. Read access is deliberately asymmetric: + // `require_can_read_path` has no operator block, so an operator can still READ a draft they + // can read via `/drafts/get`, mirroring their read access to deployed content. Intended. if authed.is_operator { - return Err(Error::NotAuthorized( - "operators cannot save drafts".to_string(), - )); + let granted = + matches!(kind, UserDraftItemKind::Flow) && operator_can_build_flows(db, w_id).await?; + if !granted { + return Err(Error::PermissionDenied( + "operators cannot save drafts".to_string(), + )); + } } // Cheap claim-based namespace checks first: they evaluate the same JWT // claims RLS reads, so the outcome matches the policies while sparing the diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 8b63289175..5115148f83 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -26,6 +26,7 @@ use std::time::Instant; use tokio::io::AsyncReadExt; use tower::ServiceBuilder; use url::Url; +use windmill_api_flows::flows::validate_operator_composed_flow; use windmill_common::assets::AssetUsageAccessType; use windmill_common::auth::TOKEN_PREFIX_LEN; #[cfg(feature = "run_inline")] @@ -53,7 +54,9 @@ use windmill_common::worker::{Connection, CLOUD_HOSTED, WINDMILL_DIR}; use windmill_common::workspace_dependencies::{ RawWorkspaceDependencies, MIN_VERSION_WORKSPACE_DEPENDENCIES, }; -use windmill_common::workspaces::{check_user_against_rule, ProtectionRuleKind, RuleCheckResult}; +use windmill_common::workspaces::{ + check_operator_can_build_flows, check_user_against_rule, ProtectionRuleKind, RuleCheckResult, +}; use windmill_common::DYNAMIC_INPUT_CACHE; #[cfg(all(feature = "enterprise", feature = "instance_smtp"))] use windmill_common::{email_oss::send_email_html, server::load_smtp_config}; @@ -8600,7 +8603,9 @@ fn operator_preview_refusal(job_id: Option) -> error::Error { } else { "Operators cannot run preview jobs for security reasons" }; - error::Error::NotAuthorized(reason.to_string()) + // 403, not 401: the frontend reads a 401 as a dead session and logs the user out, and the + // flow editor builders open reaches this route. + error::Error::PermissionDenied(reason.to_string()) } async fn run_preview_script( @@ -9282,14 +9287,32 @@ pub struct RunFlowDependenciesResponse { async fn push_flow_dependencies_job( authed: &ApiAuthed, db: &DB, + user_db: &UserDB, w_id: &str, req: RunFlowDependenciesRequest, ) -> error::Result { check_scopes(authed, || format!("jobs:run"))?; + check_operator_can_build_flows(db, w_id, authed.is_operator, "run dependencies jobs").await?; + // The dependency job locks whatever inline code this request carries, on a worker. A + // composition-only flow has none, so validating here costs a builder nothing and keeps the + // lock step from becoming the way to run code the write path refuses. if authed.is_operator { - return Err(error::Error::NotAuthorized( - "Operators cannot run dependencies jobs for security reasons".to_string(), - )); + validate_operator_composed_flow(&req.flow_value, &None, authed, db, user_db, w_id).await?; + // The job rewrites bookkeeping stored under `req.path` (dependency map, asset usages, + // lock error) even with `skip_flow_update`, so a builder must be able to write there. + crate::drafts::require_can_write_path( + authed, + db, + user_db, + w_id, + windmill_common::user_drafts::UserDraftItemKind::Flow, + &req.path, + ) + .await + .map_err(|e| match e { + error::Error::NotAuthorized(msg) => error::Error::PermissionDenied(msg), + e => e, + })?; } if req.raw_deps.is_some() { @@ -9355,20 +9378,22 @@ async fn push_flow_dependencies_job( async fn run_flow_dependencies_job( authed: ApiAuthed, Extension(db): Extension, + Extension(user_db): Extension, Path(w_id): Path, Json(req): Json, ) -> error::Result { - let uuid = push_flow_dependencies_job(&authed, &db, &w_id, req).await?; + let uuid = push_flow_dependencies_job(&authed, &db, &user_db, &w_id, req).await?; run_wait_result(&db, uuid, &w_id, None, false, &authed.username).await } async fn run_flow_dependencies_job_async( authed: ApiAuthed, Extension(db): Extension, + Extension(user_db): Extension, Path(w_id): Path, Json(req): Json, ) -> error::Result<(StatusCode, String)> { - let uuid = push_flow_dependencies_job(&authed, &db, &w_id, req).await?; + let uuid = push_flow_dependencies_job(&authed, &db, &user_db, &w_id, req).await?; Ok((StatusCode::CREATED, uuid.to_string())) } @@ -9669,15 +9694,24 @@ async fn run_preview_flow_job( Query(run_query): Query, Json(raw_flow): Json, ) -> error::Result<(StatusCode, String)> { - if authed.is_operator { - return Err(error::Error::NotAuthorized( - "Operators cannot run preview jobs for security reasons".to_string(), - )); - } + check_operator_can_build_flows(&db, &w_id, authed.is_operator, "run preview jobs").await?; // Flow preview runs an arbitrary, request-supplied flow definition; require the broad // jobs:run scope so a narrowly-scoped token cannot escape its scope. See run_preview_script. check_scopes(&authed, || format!("jobs:run"))?; require_path_read_access_for_preview(&authed, &raw_flow.path)?; + // A builder must be able to test what it composes, but the submitted value is not the stored + // one: without this the preview is a way to run inline code the write path refuses. + if authed.is_operator { + validate_operator_composed_flow( + &raw_flow.value, + &raw_flow.tag, + &authed, + &db, + &user_db, + &w_id, + ) + .await?; + } // Restarting copies the source runs' step results into the new run, and the queue resolves // them with the service pool, so every run the request names must be readable as the caller. let mut level = raw_flow.restarted_from.as_ref(); @@ -9811,9 +9845,7 @@ async fn run_dynamic_select( DynamicSelectRunnableRef::Inline { .. } ) && authed.is_operator { - return Err(error::Error::NotAuthorized( - "Operators cannot run preview jobs for security reasons".to_string(), - )); + return Err(operator_preview_refusal(None)); } if !is_valid_entrypoint_name(&request.entrypoint_function) { diff --git a/backend/windmill-api/src/runnables.rs b/backend/windmill-api/src/runnables.rs index 43ef23aff2..2cee0efe33 100644 --- a/backend/windmill-api/src/runnables.rs +++ b/backend/windmill-api/src/runnables.rs @@ -34,6 +34,7 @@ use std::collections::HashMap; use windmill_common::{ db::UserDB, error::{Error, JsonResult}, + workspaces::operator_can_build_flows, }; use windmill_types::scripts::ScriptHash; use windmill_types::user_drafts::DraftUserRef; @@ -397,7 +398,7 @@ fn draft_branch_sql(kind: &str) -> String { async fn list_runnables( authed: ApiAuthed, Extension(user_db): Extension, - Extension(_db): Extension, + Extension(db): Extension, Path(w_id): Path, Query(q): Query, ) -> JsonResult { @@ -571,9 +572,9 @@ async fn list_runnables( // Draft-only rows are the caller's own work in progress: never archived, so they // have no place in the archived view, and carrying no labels of their own they are // out of scope of a label filter (as in the per-kind endpoints). Operators don't - // see other people's drafts and have none of their own to see. + // see other people's drafts and have none of their own, except a builder's flows. let include_drafts = q.include_draft_only.unwrap_or(false) - && !authed.is_operator + && (!authed.is_operator || operator_can_build_flows(&db, &w_id).await?) && !show_archived && q.label.as_ref().filter(|s| !s.is_empty()).is_none(); let draft_extras_for = |kind: &str| -> Vec { @@ -659,7 +660,7 @@ async fn list_runnables( keyset: Option<&str>, limit: Option| -> Option { - if !include_drafts || !kinds.contains(&kind) { + if !include_drafts || !kinds.contains(&kind) || (authed.is_operator && kind != "flow") { return None; } // `fav` is ignored: with no favorite join there is nothing to filter on, and the @@ -1004,9 +1005,17 @@ async fn add_draft_counts( q: &CountRunnablesQuery, counts: &mut HashMap, ) -> Result<(), Error> { - if !q.include_draft_only.unwrap_or(false) || authed.is_operator { + if !q.include_draft_only.unwrap_or(false) { return Ok(()); } + // An operator has no drafts of their own, except a builder's flows. + let kinds: &[&str] = if !authed.is_operator { + kinds + } else if kinds.contains(&"flow") && operator_can_build_flows(db, w_id).await? { + &["flow"] + } else { + return Ok(()); + }; // $1 = workspace, $2 = the caller's email. let mut binds: Vec = vec![]; let branches: Vec = kinds diff --git a/backend/windmill-common/src/ee_oss.rs b/backend/windmill-common/src/ee_oss.rs index 2510df0960..82f99c54fb 100644 --- a/backend/windmill-common/src/ee_oss.rs +++ b/backend/windmill-common/src/ee_oss.rs @@ -71,6 +71,14 @@ pub async fn check_seat_cap_for_reactivation( Ok(None) } +#[cfg(all(feature = "enterprise", not(feature = "private")))] +pub async fn check_seat_cap_for_operator_builder( + _db: &DB, + _w_id: &str, +) -> anyhow::Result> { + Ok(None) +} + #[cfg(all(feature = "enterprise", not(feature = "private")))] pub async fn compute_instance_hash(_db: &DB) -> anyhow::Result> { // Implementation is not open source diff --git a/backend/windmill-common/src/flows.rs b/backend/windmill-common/src/flows.rs index 11ffd5da76..b392dda89a 100644 --- a/backend/windmill-common/src/flows.rs +++ b/backend/windmill-common/src/flows.rs @@ -17,6 +17,7 @@ use crate::{ cache::{self, FlowExtras}, db::DB, error::{to_anyhow, Error}, + scripts::ScriptHash, utils::{http_get_from_hub, StripPath}, worker::{to_raw_value, Connection}, DEFAULT_HUB_BASE_URL, HUB_BASE_URL, PRIVATE_HUB_MIN_VERSION, @@ -246,6 +247,156 @@ pub async fn resolve_modules( Ok(()) } +/// Checks a flow value contains nothing but composition of runnables that already exist, which is +/// all an operator with builder rights may author. Walks the modules, the preprocessor and failure +/// modules, every branch, and the `tools` of an AI agent step. +/// +/// Returns what the caller still has to authorize against its own permissions, which this +/// value-only walk cannot: every runnable the steps reference, the worker tags they pin, and the +/// `(path, hash)` pairs of version-pinned steps. See [`ComposedFlowRefs`] for why each one is not +/// already settled by the walk. +pub fn check_flow_is_composition_only(value: &FlowValue) -> Result { + let mut refs = ComposedFlowRefs::default(); + for module in value + .modules + .iter() + .chain(value.preprocessor_module.as_deref()) + .chain(value.failure_module.as_deref()) + { + check_module_is_composition_only(module, &mut refs)?; + } + Ok(refs) +} + +/// What [`check_flow_is_composition_only`] collects for the caller to authorize. +#[derive(Default)] +pub struct ComposedFlowRefs { + pub tags: Vec, + /// Every workspace runnable a step references, as `(is_flow, path)`. + pub runnables: Vec<(bool, String)>, + /// Version-pinned script steps, as `(path, hash)`. + pub pinned_scripts: Vec<(String, ScriptHash)>, +} + +fn check_module_is_composition_only( + module: &FlowModule, + refs: &mut ComposedFlowRefs, +) -> Result<(), Error> { + let value = module + .get_value() + .map_err(|e| Error::BadRequest(format!("Step {} could not be read: {e}", module.id)))?; + check_module_value_is_composition_only(&value, &module.id, refs) +} + +fn check_module_value_is_composition_only( + value: &FlowModuleValue, + id: &str, + refs: &mut ComposedFlowRefs, +) -> Result<(), Error> { + let refuse = |what: &str| Err(refused(id, what)); + // A node id points at code in a `flow_node` row, which only the dependency job produces by + // hoisting a step's code, so an authored value carrying one names another flow's code and slips + // past this walk. Editor payloads come from the un-hoisted `flow_version.value`, so refusing + // them costs nothing legitimate. + let refuse_node = |node: &Option| match node { + Some(_) => refuse("references code stored outside the flow"), + None => Ok(()), + }; + let mut push_tag = |tag: &Option| { + if let Some(tag) = tag.as_deref().filter(|t| !t.is_empty()) { + refs.tags.push(tag.to_string()); + } + }; + + match value { + FlowModuleValue::RawScript { .. } => return refuse("has inline code"), + FlowModuleValue::FlowScript { .. } => { + return refuse("references code stored outside the flow") + } + FlowModuleValue::Identity => {} + FlowModuleValue::Script { path, hash, tag_override, .. } => { + check_composable_path(path, id)?; + push_tag(tag_override); + refs.runnables.push((false, path.clone())); + if let Some(hash) = hash { + refs.pinned_scripts.push((path.clone(), *hash)); + } + } + FlowModuleValue::Flow { path, .. } => { + check_composable_path(path, id)?; + refs.runnables.push((true, path.clone())); + } + FlowModuleValue::ForloopFlow { modules, modules_node, .. } + | FlowModuleValue::WhileloopFlow { modules, modules_node, .. } => { + refuse_node(modules_node)?; + for module in modules { + check_module_is_composition_only(module, refs)?; + } + } + FlowModuleValue::BranchOne { branches, default, default_node } => { + refuse_node(default_node)?; + for module in default { + check_module_is_composition_only(module, refs)?; + } + check_branches_are_composition_only(branches, id, refs)?; + } + FlowModuleValue::BranchAll { branches, .. } => { + check_branches_are_composition_only(branches, id, refs)? + } + FlowModuleValue::AIAgent { tools, tag, agent, .. } => { + // A linked agent resolves its tools from an `ai_agent` resource at run time, and + // operators may write resources, so those tools are outside this check: the list can + // be swapped for a raw script after the flow is deployed. + if agent.is_some() { + return refuse("links an AI agent resource, whose tools live outside the flow"); + } + push_tag(tag); + for tool in tools { + if let ToolValue::FlowModule(value) = &tool.value { + check_module_value_is_composition_only(value, &tool.id, refs)?; + } + } + } + } + Ok(()) +} + +fn check_branches_are_composition_only( + branches: &[Branch], + id: &str, + refs: &mut ComposedFlowRefs, +) -> Result<(), Error> { + for branch in branches { + if branch.modules_node.is_some() { + return Err(refused( + id, + "has a branch that references code stored outside the flow", + )); + } + for module in &branch.modules { + check_module_is_composition_only(module, refs)?; + } + } + Ok(()) +} + +fn refused(id: &str, what: &str) -> Error { + Error::PermissionDenied(format!( + "Step {id} {what}, so only a developer can edit this flow. Builders can compose scripts \ + and flows that are already deployed." + )) +} + +fn check_composable_path(path: &str, id: &str) -> Result<(), Error> { + if path.starts_with("hub/") { + return Err(Error::PermissionDenied(format!( + "Step {id}: hub runnables are not available to operators with builder rights. Deploy \ + it to the workspace first." + ))); + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -281,4 +432,93 @@ mod tests { let err = extract_hub_flow_id_from_path("hub/flows/0").unwrap_err(); assert!(matches!(err, Error::BadRequest(_))); } + + fn flow(value: serde_json::Value) -> FlowValue { + serde_json::from_value(value).unwrap() + } + + #[test] + fn composition_check_accepts_a_composed_flow_and_collects_its_tags() { + let refs = check_flow_is_composition_only(&flow(serde_json::json!({"modules": [{ + "id": "a", + "value": {"type": "forloopflow", "iterator": {"type": "static", "value": []}, + "parallel": false, "modules": [ + {"id": "b", "value": {"type": "script", "path": "f/x/s", "tag_override": "gpu"}}, + {"id": "c", "value": {"type": "flow", "path": "f/x/f"}}, + {"id": "d", "value": {"type": "aiagent", "input_transforms": {}, "tag": "ai", + "tools": [{"id": "t", "value": {"tool_type": "flowmodule", + "type": "script", "path": "f/x/tool"}}]}} + ]} + }]}))) + .unwrap(); + assert_eq!(refs.tags, vec!["gpu".to_string(), "ai".to_string()]); + } + + /// The walk covers `modules`, so a node reference is a way past it: it names code hoisted + /// into a `flow_node` row, possibly another flow's. + #[test] + fn composition_check_rejects_node_references() { + for value in [ + serde_json::json!({"modules": [{"id": "a", "value": {"type": "forloopflow", + "iterator": {"type": "static", "value": []}, "parallel": false, + "modules": [], "modules_node": 7}}]}), + serde_json::json!({"modules": [{"id": "a", "value": {"type": "branchone", + "branches": [], "default": [], "default_node": 7}}]}), + serde_json::json!({"modules": [{"id": "a", "value": {"type": "branchall", + "branches": [{"expr": "true", "modules": [], "modules_node": 7}]}}]}), + serde_json::json!({"modules": [{"id": "a", "value": {"type": "flowscript", + "id": 7, "language": "bun"}}]}), + ] { + assert!(check_flow_is_composition_only(&flow(value)).is_err()); + } + } + + #[test] + fn composition_check_rejects_code_reachable_through_an_ai_agent() { + for value in [ + serde_json::json!({"modules": [{"id": "a", "value": {"type": "aiagent", + "input_transforms": {}, "tools": [{"id": "t", "value": {"tool_type": "flowmodule", + "type": "rawscript", "content": "x", "language": "bun"}}]}}]}), + serde_json::json!({"modules": [{"id": "a", "value": {"type": "aiagent", + "input_transforms": {}, "tools": [], "agent": "$res:f/x/agent"}}]}), + ] { + assert!(check_flow_is_composition_only(&flow(value)).is_err()); + } + } + + #[test] + fn composition_check_rejects_code_in_every_module_slot() { + let inline = serde_json::json!({"type": "rawscript", "content": "x", "language": "bun"}); + for value in [ + serde_json::json!({"modules": [{"id": "a", "value": inline}]}), + serde_json::json!({"modules": [], "failure_module": {"id": "f", "value": inline}}), + serde_json::json!({"modules": [], "preprocessor_module": {"id": "p", "value": inline}}), + ] { + assert!(check_flow_is_composition_only(&flow(value)).is_err()); + } + } + + /// A step carrying a `hash` dispatches on that hash alone: the caller must verify the pair + /// exists and is readable, so the walk has to surface it rather than pass it through. + #[test] + fn composition_check_reports_version_pinned_steps() { + let refs = check_flow_is_composition_only(&flow(serde_json::json!({"modules": [ + {"id": "a", "value": {"type": "script", "path": "f/x/s", "hash": "000000000000007b"}}, + {"id": "b", "value": {"type": "script", "path": "f/x/t"}} + ]}))) + .unwrap(); + assert_eq!( + refs.pinned_scripts, + vec![("f/x/s".to_string(), ScriptHash(123))] + ); + } + + #[test] + fn composition_check_rejects_hub_runnables() { + for kind in ["script", "flow"] { + let value = serde_json::json!({"modules": [{"id": "a", + "value": {"type": kind, "path": "hub/1234/thing"}}]}); + assert!(check_flow_is_composition_only(&flow(value)).is_err()); + } + } } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 448e13af85..4751cb57be 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1044,7 +1044,8 @@ pub async fn guest_app_admits<'c, E: sqlx::Executor<'c, Database = sqlx::Postgre } /// Billable members of `w_id` and the seats they cost, as `ceil(developers + operators/2)`. Service -/// accounts cannot log in and do not take a seat; a disabled member is not billed either. +/// accounts cannot log in and do not take a seat; a disabled member is not billed either. In a +/// workspace that granted operators builder rights, every operator counts as a developer. /// /// The workspace is invoiced by a job outside this codebase that counts the same rows with its own /// SQL. The two must be changed together: this rule disagreeing with that one is what bills a @@ -1063,10 +1064,15 @@ pub async fn billable_seats(db: &crate::DB, w_id: &str) -> Result .fetch_one(db) .await .map_err(|e| Error::internal_err(format!("counting billable seats of {w_id}: {e:#}")))?; + let (developers, operators) = if operator_can_build_flows(db, w_id).await? { + (row.developers + row.operators, 0) + } else { + (row.developers, row.operators) + }; Ok(BillableSeats { - developers: row.developers, - operators: row.operators, - seats: ((row.developers as f64) + 0.5 * (row.operators as f64)).ceil() as i64, + developers, + operators, + seats: ((developers as f64) + 0.5 * (operators as f64)).ceil() as i64, }) } @@ -1189,7 +1195,17 @@ pub fn invalidate_protection_rules_cache(workspace_id: &str) { // Operator rights cache lazy_static::lazy_static! { - static ref OPERATOR_RIGHTS_CACHE: Cache = Cache::new(1000); + static ref OPERATOR_RIGHTS_CACHE: Cache = Cache::new(1000); +} + +/// Every operator right of a workspace, read and cached together because they share one +/// `operator_settings` column and one invalidation. The two groups have opposite polarity: +/// `builder_flows` is granted on request and costs a seat, the `manage` rights are held by +/// default and cost nothing. +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +pub struct OperatorRights { + pub builder_flows: bool, + pub manage: OperatorManageRights, } /// Writes an operator may perform unless the workspace withdraws them. Unlike the visibility @@ -1256,7 +1272,7 @@ impl OperatorManageRights { /// /// Call it before opening an RLS transaction: it takes a connection from the root pool, and a /// second pooled connection held alongside a transaction self-deadlocks on a one-connection pool. -async fn operator_manage_rights(db: &DB, workspace_id: &str) -> Result { +async fn operator_rights(db: &DB, workspace_id: &str) -> Result { let now = chrono::Utc::now().timestamp(); if let Some((rights, expiry)) = OPERATOR_RIGHTS_CACHE.get(workspace_id) { @@ -1265,10 +1281,12 @@ async fn operator_manage_rights(db: &DB, workspace_id: &str) -> Result>'manage_schedules')::boolean, true) AS \"schedules!\", + "SELECT COALESCE((operator_settings->>'builder_flows')::boolean, false) AS \"flows!\", + COALESCE((operator_settings->>'manage_schedules')::boolean, true) AS \"schedules!\", COALESCE((operator_settings->>'manage_triggers')::boolean, true) AS \"triggers!\" FROM workspace_settings WHERE workspace_id = $1", workspace_id @@ -1282,7 +1300,10 @@ async fn operator_manage_rights(db: &DB, workspace_id: &str) -> Result Result Result { + Ok(operator_rights(db, workspace_id).await?.builder_flows) +} + +/// Gate for a write only a workspace that granted builder rights lets operators perform. `action` +/// completes "Operators cannot {action} for security reasons". +pub async fn check_operator_can_build_flows( + db: &DB, + workspace_id: &str, + is_operator: bool, + action: &str, +) -> Result<()> { + if is_operator && !operator_can_build_flows(db, workspace_id).await? { + return Err(Error::PermissionDenied(format!( + "Operators cannot {action} for security reasons" + ))); + } + Ok(()) +} + +/// Whether a membership consumes an operator (half) seat rather than an author seat. A builder +/// right makes an operator an author of deployable artifacts, so it weighs a full seat. +pub async fn consumes_operator_seat( + db: &DB, + workspace_id: &str, + is_operator: bool, +) -> Result { + Ok(is_operator && !operator_can_build_flows(db, workspace_id).await?) +} + /// Invalidate the operator rights cache for a workspace pub fn invalidate_operator_rights_cache(workspace_id: &str) { OPERATOR_RIGHTS_CACHE.remove(workspace_id); @@ -1306,7 +1359,7 @@ pub async fn check_operator_can_manage( is_operator: bool, kind: ManageKind, ) -> Result<()> { - if is_operator && !operator_manage_rights(db, workspace_id).await?.has(kind) { + if is_operator && !operator_rights(db, workspace_id).await?.manage.has(kind) { // 403, not 401: the caller is authenticated and simply lacks the right. The frontend reads // an uncaught 401 as a dead session and logs the user out, so `NotAuthorized` here would // eject an operator from the app instead of telling them why. diff --git a/backend/windmill-common/tests/billing_workspace.rs b/backend/windmill-common/tests/billing_workspace.rs index 3025061249..04455318a6 100644 --- a/backend/windmill-common/tests/billing_workspace.rs +++ b/backend/windmill-common/tests/billing_workspace.rs @@ -118,6 +118,21 @@ async fn paid_seats_and_fork_count(db: Pool) { (2, 2, 3) ); + sqlx::query( + "INSERT INTO workspace_settings (workspace_id, operator_settings) + VALUES ('seat-root', '{\"builder_flows\": true}')", + ) + .execute(&db) + .await + .unwrap(); + windmill_common::workspaces::invalidate_operator_rights_cache("seat-root"); + let breakdown = billable_seats(&db, "seat-root").await.unwrap(); + assert_eq!( + (breakdown.developers, breakdown.operators, breakdown.seats), + (4, 0, 4), + "builder rights bill every operator as a developer" + ); + insert_ws(&db, "seat-fork1", Some("seat-root"), false).await; insert_ws(&db, "seat-fork2", Some("seat-root"), false).await; // A deleted fork itself is not counted... diff --git a/cli/src/core/settings.ts b/cli/src/core/settings.ts index f065877244..57e89f5ded 100644 --- a/cli/src/core/settings.ts +++ b/cli/src/core/settings.ts @@ -413,7 +413,7 @@ export async function pushWorkspaceSettings( const localOperatorSettings = localSettings.operator_settings && { ...localSettings.operator_settings, }; - for (const key of ["manage_schedules", "manage_triggers"] as const) { + for (const key of ["builder_flows", "manage_schedules", "manage_triggers"] as const) { const remote = settings.operator_settings?.[key]; if (localOperatorSettings && localOperatorSettings[key] === undefined && remote !== undefined) { localOperatorSettings[key] = remote; diff --git a/docs/operator-builder-rights.md b/docs/operator-builder-rights.md new file mode 100644 index 0000000000..4db2a69511 --- /dev/null +++ b/docs/operator-builder-rights.md @@ -0,0 +1,76 @@ +# Operator builder rights + +`operator_settings.builder_flows` lets every operator of a workspace compose flows out of runnables +that already exist. It does not make them authors: the boundary the operator role draws is +**authoring code and running arbitrary code**, and this does not move it. + +It is a write right, unlike the visibility flags beside it, and unlike the withdrawable rights in +`docs/operator-write-rights.md` it is granted on request and costs a seat. Read it with +`windmill_common::workspaces::operator_can_build_flows` (60s cache, shared with the withdrawable +rights) and gate a write with `check_operator_can_build_flows`. + +## What the check has to cover + +`check_flow_is_composition_only` (`windmill-common/src/flows.rs`) walks a `FlowValue` and refuses +anything that carries code. Three of its rules exist because the obvious walk misses them: + +- **`FlowScript` and any populated `modules_node` / `default_node`.** These name code hoisted into + a `flow_node` row. Only the dependency job produces them, so an authored value carrying one + names code stored under some other flow. The walk covers `modules`, so a node reference is a way + past it. +- **An AI agent step's `tools`.** `ToolValue::FlowModule` wraps a whole `FlowModuleValue`, so a + tool can be a raw script. +- **An AI agent step's `agent` link.** A linked agent resolves its tools from an `ai_agent` + resource at run time, and operators may write resources, so the tool list is outside this check + and can be swapped for a raw script after the flow is approved. + +It also returns what a value-only walk cannot authorize, for the caller to check against its own +permissions: + +- **the worker tags the steps pin**, or a builder routes a job onto a privileged worker group; +- **every runnable a step references**. `script_to_payload` resolves a step's path with the root DB + handle (`db_authed = None`) and returns the referenced runnable's `on_behalf_of`, which + `worker_flow` then applies to the step job. So composing a path is enough to run it, and to run + it as whoever it runs as: `validate_operator_composed_flow` re-checks each path under the + caller's RLS. This is the general case; the one below is on top of it, not instead of it. +- **the `(path, hash)` of every version-pinned step**. A step carrying a `hash` is dispatched by + that hash alone, with the path beside it never consulted, so a readable path paired with another + script's hash still runs that other script. + +The value is not the only code a flow carries: the schema's `x-windmill-dyn-select-code` fills its +dynamic dropdowns, run as whoever loads the form, so `validate_operator_flow` refuses it like a +step's code. A builder therefore cannot save a developer's flow that has dropdowns; its editor +still previews them through the deployed flow rather than the inline route operators are refused. +Which developer flows a builder may edit at all is left to permissions: write access to the flow +or its folder. + +Call it on every write **and** every preview: `run_preview_flow_job` and +`push_flow_dependencies_job` both take a request-supplied flow value, so leaving either out makes +it the way to run what the write path refuses. A flow draft goes through `validate_operator_flow` +too: a developer who loads a builder's draft in the editor runs its code as themselves. + +## Billing + +An operator of a builder workspace consumes a full author seat: composing deployable artifacts +makes them an author, and there is no half-author. `consumes_operator_seat` is the seat-role +helper; the EE counting queries share `OPERATOR_SEAT_SQL` so the displayed, enforced and reported +numbers agree. The one exception is `get_user_usage` in `stats_ee.rs`: it is a compile-checked +`query!`, which cannot interpolate the constant, so it spells the predicate out. Change both +together. + +On cloud, `billable_seats` applies the same rule; see its doc for the out-of-repo invoice it must +match. + +Granting the right runs `check_seat_cap_for_operator_builder`, which prices the change by counting +seats twice rather than by counting the workspace's operators: an operator who already authors +elsewhere must not be charged again, so re-saving settings that already have the right on is a +zero delta and never blocks. + +## Accepted risks + +- All-or-nothing per workspace: there is no per-user builder role. +- `operator_settings` is git-synced, so a pull can flip every operator's class in a workspace and + the billed seat count with it. +- A builder writes JavaScript expressions: step inputs, branch predicates, `stop_after_if` and + `suspend`. They run in QuickJS with no filesystem or network access, and forbidding them would + leave nothing to compose with. diff --git a/docs/operator-write-rights.md b/docs/operator-write-rights.md index fb9d4051bb..2c18ecd290 100644 --- a/docs/operator-write-rights.md +++ b/docs/operator-write-rights.md @@ -79,7 +79,7 @@ authorizing writes on every other replica until its own entry expires. These name capabilities operators already hold, so absence has to mean "never configured", not a value. That is easy to get wrong in two places, and the obvious implementation gets both wrong: -- The read coalesces to **true** (`operator_manage_rights`), including for a workspace with no +- The read coalesces to **true** (`operator_rights`), including for a workspace with no `workspace_settings` row, which is what `OperatorManageRights::default` is for. Coalescing to false instead revokes the right on upgrade for every workspace that ever saved operator settings, since those rows carry explicit keys and none of them is this one. diff --git a/frontend/src/lib/components/EditableSchemaForm.svelte b/frontend/src/lib/components/EditableSchemaForm.svelte index 79173424d9..11429cb6fe 100644 --- a/frontend/src/lib/components/EditableSchemaForm.svelte +++ b/frontend/src/lib/components/EditableSchemaForm.svelte @@ -19,6 +19,7 @@ import ToggleButtonGroup from './common/toggleButton-v2/ToggleButtonGroup.svelte' import Label from './Label.svelte' import { sendUserToast } from '$lib/toast' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' import Toggle from './Toggle.svelte' import { DynamicInput, @@ -35,9 +36,14 @@ import Section from '$lib/components/Section.svelte' import Editor from './Editor.svelte' import AddPropertyV2 from './schema/AddPropertyV2.svelte' - import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' + import { + useOperatingUser, + useOperatingWorkspace + } from '$lib/components/operatingWorkspace.svelte' const operatingWorkspace = useOperatingWorkspace() + const operatingUser = useOperatingUser() + const operatorBuilderFlows = useOperatorBuilderFlows() // export let openEditTab: () => void = () => {} const dispatch = createEventDispatcher() @@ -88,6 +94,8 @@ schemaFormClassName?: string onChange?: (args: Record) => void workspace?: string | undefined + /** The deployed flow an operator's dropdown previews read their options from. */ + deployedFlowPath?: string } let { @@ -127,7 +135,8 @@ extraTab, schemaFormClassName = undefined, onChange = undefined, - workspace = undefined + workspace = undefined, + deployedFlowPath = undefined }: Props = $props() let ws = $derived(workspace ?? $operatingWorkspace) @@ -469,11 +478,12 @@ order: e.detail } }} - helperScript={{ - source: 'inline', - code: dynCode!, - lang: dynLang! - }} + helperScript={DynamicInput.flowHelperScript( + dynCode, + dynLang, + deployedFlowPath, + operatingUser.current?.operator + )} prettifyHeader={isAppInput} disabled={!!previewSchema} {diff} @@ -486,7 +496,7 @@ {@render runButton?.()} - {#if dynamicFunctions.length > 0} + {#if dynamicFunctions.length > 0 && !$operatorBuilderFlows}
{/each} - {#if showDynOpt} + {#if showDynOpt && !$operatorBuilderFlows} {#each DYNAMIC_OPTIONS as x} {/each} diff --git a/frontend/src/lib/components/FlowBuilder.svelte b/frontend/src/lib/components/FlowBuilder.svelte index 60577ef124..434ff4417f 100644 --- a/frontend/src/lib/components/FlowBuilder.svelte +++ b/frontend/src/lib/components/FlowBuilder.svelte @@ -106,12 +106,13 @@ import { UserDraft } from '$lib/userDraft.svelte' import { setOpenInSessionHandoff } from './sessions/openInSessionContext' import { getEditorStoragePath, setEditorStoragePath } from './editorStoragePathContext' - import { useTriggerLock } from '$lib/operatorWriteRights' + import { useOperatorBuilderFlows, useTriggerLock } from '$lib/operatorWriteRights' import { useOperatingUser, useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' const triggerLock = useTriggerLock() + const operatorBuilderFlows = useOperatorBuilderFlows() const operatingWorkspace = useOperatingWorkspace() const operatingUser = useOperatingUser() @@ -1519,7 +1520,7 @@ {#key renderCount} - {#if !actingUser?.operator} + {#if !actingUser?.operator || $operatorBuilderFlows} {#if $pathStore} {/if} @@ -1673,7 +1674,9 @@ {forceTestTab} {highlightArg} aiChatOpen={aiChatManager.open} - showFlowAiButton={!disableAi && customUi?.topBar?.aiBuilder != false} + showFlowAiButton={!disableAi && + customUi?.topBar?.aiBuilder != false && + !$operatorBuilderFlows} toggleAiChat={() => aiChatManager.toggleOpen()} {sessionOpen} onOpenPreview={flowPreviewButtons?.openPreview} @@ -1702,7 +1705,9 @@ {/if} {:else} - Flow Builder not available to operators +
+ Flow builder not available to operators +
{/if} {/key} diff --git a/frontend/src/lib/components/FlowPreviewContent.svelte b/frontend/src/lib/components/FlowPreviewContent.svelte index 5489e4abb5..151d35be77 100644 --- a/frontend/src/lib/components/FlowPreviewContent.svelte +++ b/frontend/src/lib/components/FlowPreviewContent.svelte @@ -28,7 +28,7 @@ RefreshCw, X } from 'lucide-svelte' - import { sendUserToast, type StateStore } from '$lib/utils' + import { DynamicInput, sendUserToast, type StateStore } from '$lib/utils' import { dfs } from './flows/dfs' import { sliceModules } from './flows/flowStateUtils.svelte' import InputSelectedBadge from './schema/InputSelectedBadge.svelte' @@ -44,9 +44,13 @@ import FlowRestartButton from './FlowRestartButton.svelte' import { useNestedRestartState } from './useNestedRestartState.svelte' import { buildFlowRecording, downloadRecordingJson } from './recording/runRecording' - import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' + import { + useOperatingUser, + useOperatingWorkspace + } from '$lib/components/operatingWorkspace.svelte' const operatingWorkspace = useOperatingWorkspace() + const operatingUser = useOperatingUser() interface Props { previewMode: 'upTo' | 'whole' @@ -552,14 +556,14 @@ savedArgs = $state.snapshot(previewArgs.val) }} bind:isValid - helperScript={flowStore.val.schema?.['x-windmill-dyn-select-code'] && - flowStore.val.schema?.['x-windmill-dyn-select-lang'] - ? { - source: 'inline', - code: flowStore.val.schema['x-windmill-dyn-select-code'] as string, - lang: flowStore.val.schema['x-windmill-dyn-select-lang'] as ScriptLang - } - : undefined} + helperScript={DynamicInput.flowHelperScript( + flowStore.val.schema?.['x-windmill-dyn-select-code'] as string | undefined, + flowStore.val.schema?.['x-windmill-dyn-select-lang'] as + | ScriptLang + | undefined, + $initialPathStore, + operatingUser.current?.operator + )} /> {/key} diff --git a/frontend/src/lib/components/common/table/FlowRow.svelte b/frontend/src/lib/components/common/table/FlowRow.svelte index f9af803694..e000ee25bc 100644 --- a/frontend/src/lib/components/common/table/FlowRow.svelte +++ b/frontend/src/lib/components/common/table/FlowRow.svelte @@ -9,6 +9,7 @@ import type ShareModal from '$lib/components/ShareModal.svelte' import { FlowService, type Flow } from '$lib/gen' import { userStore, userWorkspaces, workspaceStore } from '$lib/stores' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' import { createEventDispatcher } from 'svelte' import Badge from '../badge/Badge.svelte' @@ -42,6 +43,8 @@ import EditInForkButton from './EditInForkButton.svelte' import { isCloudHosted } from '$lib/cloud' + const operatorBuilderFlows = useOperatorBuilderFlows() + interface Props { flow: Flow & { draft_only?: boolean @@ -217,6 +220,7 @@ let { draft_only, path, archived } = flow let owner = isOwner(path, $userStore, $workspaceStore) const canEdit = flow.canWrite && showEditButton + const hideForOperator = $userStore?.operator && !$operatorBuilderFlows if (draft_only) { return [ ...selectMenuItems(rowSelection), @@ -251,7 +255,7 @@ // list endpoint only surfaces own/legacy draft-only rows), so // discarding it never requires write permission on the path. disabled: !showEditButton, - hide: $userStore?.operator + hide: hideForOperator } ] } @@ -267,7 +271,7 @@ icon: GitFork, href: `${base}/flows/add?template=${path}`, disabled: !showEditButton, - hide: $userStore?.operator + hide: hideForOperator }, { displayName: editInForkLabel($workspaceStore, $userWorkspaces), @@ -293,7 +297,7 @@ moveDrawer.openDrawer(path, flow.summary, 'flow') }, disabled: !owner || archived || !canEdit, - hide: $userStore?.operator + hide: hideForOperator }, { displayName: 'Copy path', @@ -321,7 +325,7 @@ action: () => { flowHistory?.open() }, - hide: $userStore?.operator + hide: hideForOperator }, { displayName: 'Schedule', @@ -330,7 +334,7 @@ scheduleEditor?.openNew(true, path) }, disabled: archived, - hide: $userStore?.operator + hide: hideForOperator }, { displayName: 'Permissions', @@ -338,7 +342,7 @@ action: () => { shareModal.openDrawer && shareModal.openDrawer(path, 'flow') }, - hide: $userStore?.operator + hide: hideForOperator }, { displayName: archived ? 'Unarchive' : 'Archive', @@ -348,7 +352,7 @@ }, type: 'delete', disabled: !owner || !canEdit, - hide: $userStore?.operator + hide: hideForOperator }, { displayName: 'Delete', @@ -365,7 +369,7 @@ }, type: 'delete', disabled: !owner || !canEdit, - hide: $userStore?.operator + hide: hideForOperator } ] }} diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 7ccdb3d412..7bf9685e87 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -168,6 +168,7 @@ import type { ArtifactVersionTarget } from '$lib/components/sessions/previewRout import { appendAttachedFilesRoster } from './files/fileTools' import { ENTER_PLAN_MODE_TOOL, EXIT_PLAN_MODE_TOOL } from './planMode' import { PlanModeController, type PlanModeHost } from './planModeController.svelte' +import { navigationOperatorBuilderFlows } from '$lib/operatorWriteRights' // Compaction of the stored history: once the projected request size // (contextTokens — the provider's report when current, a fresh chars/4 @@ -300,6 +301,8 @@ export function supportsPlanMode(mode: AIMode): boolean { return PLAN_MODES.has(mode) } +const isOperatorBuilderFlows = fromStore(navigationOperatorBuilderFlows) + export function isAIModeVisible(mode: AIMode): boolean { return mode !== AIMode.GLOBAL || isGlobalAiEnabled() } @@ -1435,12 +1438,19 @@ export class AIChatManager implements ChatViewHost { .map((s) => ({ ...s, kind: 'skill' as const })) ]) + // The flow and script builders both write code, which the backend refuses from an operator + // with the builder right: leaving them reachable would only produce work that cannot be + // deployed. allowedModes: Record = $derived({ script: this.flowAiChatHelpers === undefined && this.scriptEditorOptions !== undefined && - !this.disabledModes.script, - flow: this.flowAiChatHelpers !== undefined && !this.disabledModes.flow, + !this.disabledModes.script && + !isOperatorBuilderFlows.current, + flow: + this.flowAiChatHelpers !== undefined && + !this.disabledModes.flow && + !isOperatorBuilderFlows.current, app: this.appAiChatHelpers !== undefined && !this.disabledModes.app, navigator: !this.disabledModes.navigator, ask: !this.disabledModes.ask, diff --git a/frontend/src/lib/components/flows/common/FlowCardHeader.svelte b/frontend/src/lib/components/flows/common/FlowCardHeader.svelte index 6d14463c6b..a2fa8e08ba 100644 --- a/frontend/src/lib/components/flows/common/FlowCardHeader.svelte +++ b/frontend/src/lib/components/flows/common/FlowCardHeader.svelte @@ -20,6 +20,7 @@ import type { FlowBuilderWhitelabelCustomUi } from '$lib/components/custom_ui' import DropdownV2 from '$lib/components/DropdownV2.svelte' import { hubBaseUrlStore } from '$lib/stores' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' import { DEFAULT_HUB_BASE_URL, PRIVATE_HUB_MIN_VERSION } from '$lib/hub' import { getLatestHashForScript } from '$lib/scripts' import { sendUserToast, type Item } from '$lib/utils' @@ -30,6 +31,7 @@ import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' const operatingWorkspace = useOperatingWorkspace() + const operatorBuilderFlows = useOperatorBuilderFlows() interface Props { flowModuleValue?: FlowModuleValue | undefined @@ -102,7 +104,7 @@ const scriptItems: Item[] = $derived.by(() => { if (flowModuleValue?.type !== 'script') return [] const items: Item[] = [] - if (!isHub && customUi?.scriptEdit != false) { + if (!isHub && customUi?.scriptEdit != false && !$operatorBuilderFlows) { items.push({ displayName: "Edit the script's code", icon: Pen, @@ -146,7 +148,7 @@ }) } } - if (customUi?.scriptFork != false) { + if (customUi?.scriptFork != false && !$operatorBuilderFlows) { items.push({ displayName: 'Fork into an inline script', icon: GitFork, diff --git a/frontend/src/lib/components/flows/content/FlowInput.svelte b/frontend/src/lib/components/flows/content/FlowInput.svelte index ca5c6d8073..c6744282e2 100644 --- a/frontend/src/lib/components/flows/content/FlowInput.svelte +++ b/frontend/src/lib/components/flows/content/FlowInput.svelte @@ -820,6 +820,7 @@ workspace={opWs} editTab={chatInputsEditTab ? 'inputEditor' : undefined} showDynOpt + deployedFlowPath={$initialPathStore} bind:dynCode bind:dynLang on:delete={(e) => { @@ -878,6 +879,7 @@ addPropertyV2?.handleDeleteArgument([e.detail]) }} showDynOpt + deployedFlowPath={$initialPathStore} displayWebhookWarning editTab={$flowInputEditorState?.selectedTab} {previewSchema} diff --git a/frontend/src/lib/components/flows/content/FlowInputsQuick.svelte b/frontend/src/lib/components/flows/content/FlowInputsQuick.svelte index ae28a377c4..4e8b97f76f 100644 --- a/frontend/src/lib/components/flows/content/FlowInputsQuick.svelte +++ b/frontend/src/lib/components/flows/content/FlowInputsQuick.svelte @@ -8,6 +8,7 @@ import FlowScriptPickerQuick from '../pickers/FlowScriptPickerQuick.svelte' import { defaultScriptLanguages, processInlineLangs } from '$lib/scripts' import { defaultScripts, enterpriseLicense, hubBaseUrlStore } from '$lib/stores' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' import type { SupportedLanguage } from '$lib/common' import { createEventDispatcher, getContext, untrack } from 'svelte' import type { FlowBuilderWhitelabelCustomUi } from '$lib/components/custom_ui' @@ -34,6 +35,7 @@ } from '$lib/components/operatingWorkspace.svelte' const operatingWorkspace = useOperatingWorkspace() + const operatorBuilderFlows = useOperatorBuilderFlows() const operatingUser = useOperatingUser() const actingUser = $derived(operatingUser.current) @@ -155,7 +157,10 @@ preFilter: 'all' | 'workspace' | 'hub', selectedKind: 'script' | 'flow' | 'approval' | 'trigger' | 'preprocessor' | 'failure' ) { - if (['script', 'trigger', 'failure', 'approval', 'preprocessor'].includes(selectedKind)) { + if ( + !$operatorBuilderFlows && + ['script', 'trigger', 'failure', 'approval', 'preprocessor'].includes(selectedKind) + ) { if (!selected && preFilter == 'all') { inlineScripts = langs.filter((lang) => { return ( @@ -248,6 +253,7 @@ let showAiRows = $derived( !disableAi && !$copilotInfo.workspaceDisabled && + !$operatorBuilderFlows && funcDesc?.length > 0 && kind != 'failure' && kind != 'preprocessor' && @@ -276,9 +282,14 @@ preFilter === 'all' && !selected && customUi?.aiSandbox != false && + !$operatorBuilderFlows && matchesAiSandbox ) + // Hub runnables carry code the workspace never reviewed, and the backend refuses them in a + // flow a builder authors, so the hub browser and its integration filters are not offered. + let showHub = $derived(!$operatorBuilderFlows) + // Every result row lives in one keyboard index space, and hovering a row moves that index, so // mouse and keyboard can never highlight two different rows. Offsets follow the render order. let inlineOffset = $derived(topLevelNodes.length) @@ -331,7 +342,7 @@ {/if} {/if} - {#if preFilter === 'hub' || preFilter === 'all'} + {#if showHub && (preFilter === 'hub' || preFilter === 'all')} {#if preFilter == 'all'}
Integrations
{/if} @@ -536,7 +547,7 @@ }} /> {/if} - {#if selectedKind != 'preprocessor' && selectedKind != 'flow'} + {#if showHub && selectedKind != 'preprocessor' && selectedKind != 'flow'} {#if (!selected || selected?.kind === 'integrations') && (preFilter === 'hub' || preFilter === 'all')} {#if !selected && preFilter !== 'hub'}
Hub
diff --git a/frontend/src/lib/components/flows/content/FlowModuleComponent.svelte b/frontend/src/lib/components/flows/content/FlowModuleComponent.svelte index a256a9998a..577a6275d9 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleComponent.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleComponent.svelte @@ -39,6 +39,7 @@ import type { ButtonProp } from '$lib/components/diffEditorTypes' import { loadSchemaFromModule } from '../flowInfers' import { type Job } from '$lib/gen' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' import { checkIfParentLoop } from '../utils.svelte' import { useWorkspaceScriptSettings } from '../useWorkspaceScriptSettings.svelte' import ScriptSettingsBadges from '$lib/components/ScriptSettingsBadges.svelte' @@ -74,6 +75,7 @@ import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' const operatingWorkspace = useOperatingWorkspace() + const operatorBuilderFlows = useOperatorBuilderFlows() const { selectionManager, @@ -235,8 +237,11 @@ !flowModule.value.path?.startsWith('hub/') && flowModule.value.hash == undefined && customUi?.scriptEdit != false && + !$operatorBuilderFlows && $workspaceScriptSettingsDrawer != undefined ) + // Same rule as the graph node's Run button (FlowModuleSchemaItem). + let builderCannotTestStep = $derived($operatorBuilderFlows && flowModule.value.type === 'script') let workspaceScriptNoEditReason = $derived( flowModule.value.type !== 'script' || canEditWorkspaceScriptSettings ? undefined @@ -288,7 +293,7 @@ } function onKeyDown(event: KeyboardEvent) { - if ((event.ctrlKey || event.metaKey) && event.key == 'Enter') { + if ((event.ctrlKey || event.metaKey) && event.key == 'Enter' && !builderCannotTestStep) { event.preventDefault() selected = 'test' modulePreview?.runTestWithStepArgs() @@ -1121,7 +1126,9 @@ {#if !preprocessorModule} {/if} - + {#if !builderCannotTestStep} + + {/if} {#if canShowChatTab && flowModule.value.type === 'aiagent'} - {:else if visibleSelected === 'test'} + {:else if visibleSelected === 'test' && !builderCannotTestStep} {#if debugMode && isDebuggableScript}
{ - const dynCode = additionalInputsSchema?.['x-windmill-dyn-select-code'] - const dynLang = additionalInputsSchema?.['x-windmill-dyn-select-lang'] - if (dynCode && dynLang) { - return { source: 'inline', code: dynCode, lang: dynLang } - } - return undefined - }) + const dynamicInputHelperScript = $derived( + DynamicInput.flowHelperScript( + additionalInputsSchema?.['x-windmill-dyn-select-code'], + additionalInputsSchema?.['x-windmill-dyn-select-lang'], + path, + operatingUser.current?.operator + ) + ) // The composer's attachments feed this input, and the paperclip is its whole editor. const attachmentsTarget = $derived.by(() => { diff --git a/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte b/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte index e6d7223fcb..06c01dd75c 100644 --- a/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte +++ b/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte @@ -35,6 +35,9 @@ import DiffActionBar from './DiffActionBar.svelte' import { getGraphContext } from '$lib/components/graph/graphContext' import MoveHandleButton from '$lib/components/graph/MoveHandleButton.svelte' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' + + const operatorBuilderFlows = useOperatorBuilderFlows() interface Props { selected?: boolean @@ -128,6 +131,12 @@ let newId: string = $state(untrack(() => id) ?? '') + let mod = $derived( + id && flowStore?.val?.value ? dfsPreviousResults(id, flowStore.val, false)[0] : undefined + ) + // A script step is tested as a script preview, which operators are refused: running the + // deployed runnable is left to "Test flow". + let builderCannotTestStep = $derived($operatorBuilderFlows && mod?.value.type === 'script') let moduleTest: ModuleTest | undefined = $state(undefined) let testIsLoading = $state(false) let hover = $state(false) @@ -234,8 +243,6 @@ {/if} {#if deletable && id && flowStore && outputPickerVisible} - {@const flowStoreVal = flowStore.val} - {@const mod = flowStoreVal?.value ? dfsPreviousResults(id, flowStoreVal, false)[0] : undefined} {#if mod && flowStateStore?.val?.[id]} (hover = true)} onmouseleave={() => (hover = false)} > - {#if !isMultiSelected && (hover || selected || testRunDropdownOpen) && outputPickerVisible} + {#if !isMultiSelected && (hover || selected || testRunDropdownOpen) && outputPickerVisible && !builderCannotTestStep}
{#if !testIsLoading} - {#if savedAgentsLoading} + {#if $operatorBuilderFlows} + + {:else if savedAgentsLoading}
Loading saved agents
diff --git a/frontend/src/lib/components/home/CreateActionsMenu.svelte b/frontend/src/lib/components/home/CreateActionsMenu.svelte index a400a73df0..b35ef95aa5 100644 --- a/frontend/src/lib/components/home/CreateActionsMenu.svelte +++ b/frontend/src/lib/components/home/CreateActionsMenu.svelte @@ -25,11 +25,14 @@ import { importScriptStore } from '$lib/components/scripts/scriptStore.svelte' import { importStore } from '$lib/components/apps/store' import { conditionalMelt, getLocalSetting, storeLocalSetting } from '$lib/utils' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' import { createDropdownMenu, melt } from '@melt-ui/svelte' import YAML from 'yaml' import type { Snippet } from 'svelte' import { logFeatureUsage } from '$lib/utils/featureUsage' + const operatorBuilderFlows = useOperatorBuilderFlows() + interface Props { /** Replaces the default `New` button, e.g. with an inline text link. */ trigger?: Snippet @@ -247,6 +250,14 @@ } } + // A builder composes runnables that already exist, so only flows are offered: everything else + // here writes code, which the backend refuses from an operator. + // Derived, not computed once: switching workspace only sets `workspaceStore`, it does not + // remount this component, so a snapshot would keep the previous workspace's kinds. + const options: Option[] = $derived( + $operatorBuilderFlows ? allOptions.filter((o) => o.key === 'flow') : allOptions + ) + // the doc panel only shows while an option is hovered or focused, so the menu opens compact let activeKey: string | undefined = $state(undefined) // every option's import action, surfaced together under the bottom "Import" submenu. @@ -256,7 +267,7 @@ ...(onImportHubProject ? [{ label: 'Import a hub project', onSelect: onImportHubProject }] : []), - ...allOptions.flatMap((o) => o.extras ?? []) + ...options.flatMap((o) => o.extras ?? []) ]) // melt dropdown menu: arrow-key nav, typeahead, focus management and outside/escape @@ -372,7 +383,7 @@ activeKey = undefined } }) - let active = $derived(allOptions.find((o) => o.key === activeKey)) + let active = $derived(options.find((o) => o.key === activeKey)) let activeAc = $derived(active ? accentClasses[active.accent] : undefined) // shared YAML/JSON import drawer, reused by every "Import …" extra @@ -524,7 +535,7 @@ {/if} {/snippet} - {#each allOptions as option (option.key)} + {#each options as option (option.key)} {@const ac = accentClasses[option.accent]} {@const rowClass = 'w-full flex flex-row items-center gap-2.5 rounded-md px-2 py-1.5 text-left cursor-pointer transition-colors focus:outline-none data-[highlighted]:bg-surface-hover hover:bg-surface-hover'} diff --git a/frontend/src/lib/components/home/ItemsList.svelte b/frontend/src/lib/components/home/ItemsList.svelte index 1d9edc683b..351dd0460a 100644 --- a/frontend/src/lib/components/home/ItemsList.svelte +++ b/frontend/src/lib/components/home/ItemsList.svelte @@ -17,6 +17,7 @@ import { resource } from 'runed' import { getDraftItems } from '$lib/workspaceDrafts.svelte' import { disableHubStore, userStore, workspaceStore } from '$lib/stores' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' import type uFuzzy from '@leeoniya/ufuzzy' import { ArrowDownUp, @@ -60,6 +61,9 @@ import { base } from '$lib/base' import BulkActionsBar from './BulkActionsBar.svelte' import { HomeSelection, setHomeSelection, toBulkItem } from './homeSelection.svelte' + + const operatorBuilderFlows = useOperatorBuilderFlows() + interface Props { subtab?: 'flow' | 'script' | 'app' showEditButtons?: boolean @@ -338,7 +342,9 @@ canWrite: canWrite(it.path, (it.extra_perms ?? {}) as any, $userStore) && (it.type === 'script' || it.workspace_id == $workspaceStore) && - !$userStore?.operator + // The builder right covers flows only; a script or an app is still off limits, so + // the row must not offer edit or delete for those. + (!$userStore?.operator || (it.type === 'flow' && $operatorBuilderFlows)) } // combinedItems reads a script's time from `created_at`; the endpoint's // unified `edited_at` holds exactly that for scripts. @@ -1064,7 +1070,7 @@ * whose direct-deploy protection cleared `showEditButtons` — must not be shown them. * Reading archived items is not a write, so it is not gated on this. */ - let canCreateHere = $derived(!$userStore?.operator && showEditButtons) + let canCreateHere = $derived((!$userStore?.operator || $operatorBuilderFlows) && showEditButtons) // The workspace itself holds nothing — no filter is narrowing the list away. It stays // false until the first load resolves: a skeleton already means "loading", and the @@ -1876,9 +1882,12 @@ the menu itself does no permission check. --> {#if canCreateHere} + script and flow hub pickers observe. Nor for a builder: a hub project brings + scripts and apps along. --> (hubPickerOpen = true)} + onImportHubProject={$disableHubStore || $operatorBuilderFlows + ? undefined + : () => (hubPickerOpen = true)} /> {/if}
diff --git a/frontend/src/lib/components/home/WorkspaceEmptyState.svelte b/frontend/src/lib/components/home/WorkspaceEmptyState.svelte index fc36cc334b..1f7bebb1ce 100644 --- a/frontend/src/lib/components/home/WorkspaceEmptyState.svelte +++ b/frontend/src/lib/components/home/WorkspaceEmptyState.svelte @@ -4,9 +4,12 @@ import Popover from '$lib/components/meltComponents/Popover.svelte' import type { HubProjectPick } from '$lib/hubProject' import { disableHubStore } from '$lib/stores' + import { useOperatorBuilderFlows } from '$lib/operatorWriteRights' import CreateActionsMenu from './CreateActionsMenu.svelte' import HubTemplatePicker from './HubTemplatePicker.svelte' + const operatorBuilderFlows = useOperatorBuilderFlows() + interface Props { /** A project was chosen here. The list owns the import dialog, and opens it on this. */ onPick: (project: HubProjectPick) => void @@ -35,6 +38,9 @@ // `border-light`; only the container outline steps up, so nothing outweighs its frame. const rowOpacities = [1, 0.7, 0.4] + // A builder gets no hub template: a hub project brings scripts and apps along. + const showHub = $derived(!$disableHubStore && !$operatorBuilderFlows) + // The inline "create a new one" link is the anchor for the very same New menu the // toolbar button opens, so the menu pops next to the words that promised it. let newLinkEl: HTMLButtonElement | undefined = $state(undefined) @@ -93,9 +99,9 @@ Your scripts, flows and apps will show up here. {/if} {#if canCreate} - - {#if !$disableHubStore} + {#if showHub}