diff --git a/AGENTS.md b/AGENTS.md index a315f1a67c..beab1c5cba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,9 @@ Open-source platform for internal tools, workflows, API integrations, background reaches the DB only through the API, so `Connection::Http` paths are never taken by a plain `cargo run`; a normal build cannot start one at all. - **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow +- **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 - **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-4515bdeded3b40abbdcc554c9562cce3225d2b66dc9fe23b1fa59cfaa120cd30.json b/backend/.sqlx/query-4515bdeded3b40abbdcc554c9562cce3225d2b66dc9fe23b1fa59cfaa120cd30.json new file mode 100644 index 0000000000..b456310822 --- /dev/null +++ b/backend/.sqlx/query-4515bdeded3b40abbdcc554c9562cce3225d2b66dc9fe23b1fa59cfaa120cd30.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings\n SET operator_settings = COALESCE(operator_settings, '{}'::jsonb) || $1\n WHERE workspace_id = $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Jsonb", + "Text" + ] + }, + "nullable": [] + }, + "hash": "4515bdeded3b40abbdcc554c9562cce3225d2b66dc9fe23b1fa59cfaa120cd30" +} diff --git a/backend/.sqlx/query-457575e0858db5747964d4b2a69d3506774e50f871963da9697c7035def2aedd.json b/backend/.sqlx/query-457575e0858db5747964d4b2a69d3506774e50f871963da9697c7035def2aedd.json new file mode 100644 index 0000000000..dfff54ec4b --- /dev/null +++ b/backend/.sqlx/query-457575e0858db5747964d4b2a69d3506774e50f871963da9697c7035def2aedd.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT 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": "schedules!", + "type_info": "Bool" + }, + { + "ordinal": 1, + "name": "triggers!", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + null, + null + ] + }, + "hash": "457575e0858db5747964d4b2a69d3506774e50f871963da9697c7035def2aedd" +} diff --git a/backend/migrations/20260918141240_notify_operator_settings_change.down.sql b/backend/migrations/20260918141240_notify_operator_settings_change.down.sql new file mode 100644 index 0000000000..5f554f6bd0 --- /dev/null +++ b/backend/migrations/20260918141240_notify_operator_settings_change.down.sql @@ -0,0 +1,2 @@ +DROP TRIGGER IF EXISTS operator_settings_change_trigger ON workspace_settings; +DROP FUNCTION IF EXISTS notify_operator_settings_change(); diff --git a/backend/migrations/20260918141240_notify_operator_settings_change.up.sql b/backend/migrations/20260918141240_notify_operator_settings_change.up.sql new file mode 100644 index 0000000000..1899b65898 --- /dev/null +++ b/backend/migrations/20260918141240_notify_operator_settings_change.up.sql @@ -0,0 +1,18 @@ +-- The write rights in `operator_settings` are authorization decisions read through a per-process +-- cache. Without this, withdrawing one on a single API replica leaves every other replica +-- authorizing writes until its own entry expires. +CREATE OR REPLACE FUNCTION notify_operator_settings_change() +RETURNS TRIGGER AS $$ +BEGIN + INSERT INTO notify_event (channel, payload) + VALUES ('notify_operator_settings_change', NEW.workspace_id); + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS operator_settings_change_trigger ON workspace_settings; +CREATE TRIGGER operator_settings_change_trigger +AFTER UPDATE OF operator_settings ON workspace_settings +FOR EACH ROW +WHEN (OLD.operator_settings IS DISTINCT FROM NEW.operator_settings) +EXECUTE FUNCTION notify_operator_settings_change(); diff --git a/backend/src/main.rs b/backend/src/main.rs index 9e7a93cfcd..f4b19c1ee9 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -46,26 +46,24 @@ use windmill_common::{ CUSTOM_TAGS_SETTING, DEFAULT_TAGS_PER_WORKSPACE_SETTING, DEFAULT_TAGS_WORKSPACES_SETTING, DISABLE_PASSWORD_LOGIN_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS, EXPOSE_DEBUG_METRICS_SETTING, EXPOSE_METRICS_SETTING, EXTRA_PIP_INDEX_URL_SETTING, - FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING, - HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, - HUB_API_SECRET_SETTING, HUB_BASE_URL_SETTING, INDEXER_SETTING, - INSTANCE_EVENTS_WEBHOOK_SETTING, INSTANCE_PYTHON_VERSION_SETTING, + FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, + HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, HUB_API_SECRET_SETTING, HUB_BASE_URL_SETTING, + INDEXER_SETTING, INSTANCE_EVENTS_WEBHOOK_SETTING, INSTANCE_PYTHON_VERSION_SETTING, JOB_DEFAULT_TIMEOUT_SECS_SETTING, JOB_ISOLATION_SETTING, JWT_SECRET_SETTING, KEEP_JOB_DIR_SETTING, LICENSE_KEY_SETTING, MAVEN_REPOS_SETTING, MAVEN_SETTINGS_XML_SETTING, MCP_DISABLE_TOKEN_QUERY_PARAM_SETTING, MONITOR_LOGS_ON_OBJECT_STORE_SETTING, - NO_DEFAULT_MAVEN_SETTING, - NPM_CONFIG_REGISTRY_SETTING, NSJAIL_TMPFS_SIZE_MB_SETTING, NSJAIL_TMP_BACKING_SETTING, - NUGET_CONFIG_SETTING, OAUTH_SETTING, OTEL_SETTING, OTEL_TRACES_RETENTION_SECS_SETTING, - OTEL_TRACING_PROXY_SETTING, PIP_INDEX_URL_SETTING, POWERSHELL_REPO_PAT_SETTING, - POWERSHELL_REPO_URL_SETTING, PREVIEW_TAGS_OVERRIDE_SETTING, REQUEST_SIZE_LIMIT_SETTING, - REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING, RESTART_COORDINATION_SETTING, - RETENTION_PERIOD_SECS_OVERRIDES_SETTING, RETENTION_PERIOD_SECS_SETTING, RUBY_REPOS_SETTING, - SAML_METADATA_SETTING, SANDBOX_IMAGE_CACHE_MAX_MB_SETTING, - SANDBOX_IMAGE_DEFAULT_REGISTRY_SETTING, SANDBOX_IMAGE_MAX_SIZE_MB_SETTING, - SANDBOX_IMAGE_PULL_POLICY_SETTING, SANDBOX_REGISTRY_AUTH_SETTING, SCIM_TOKEN_SETTING, - SERVICE_LOG_RETENTION_SECS_SETTING, SMTP_SETTING, STORE_AUDIT_LOGS_S3_SETTING, - TEAMS_SETTING, TIMEOUT_WAIT_RESULT_SETTING, UV_EXCLUDE_NEWER_SETTING, - UV_INDEX_STRATEGY_SETTING, UV_PYTHON_INSTALL_MIRROR_SETTING, + NO_DEFAULT_MAVEN_SETTING, NPM_CONFIG_REGISTRY_SETTING, NSJAIL_TMPFS_SIZE_MB_SETTING, + NSJAIL_TMP_BACKING_SETTING, NUGET_CONFIG_SETTING, OAUTH_SETTING, OTEL_SETTING, + OTEL_TRACES_RETENTION_SECS_SETTING, OTEL_TRACING_PROXY_SETTING, PIP_INDEX_URL_SETTING, + POWERSHELL_REPO_PAT_SETTING, POWERSHELL_REPO_URL_SETTING, PREVIEW_TAGS_OVERRIDE_SETTING, + REQUEST_SIZE_LIMIT_SETTING, REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING, + RESTART_COORDINATION_SETTING, RETENTION_PERIOD_SECS_OVERRIDES_SETTING, + RETENTION_PERIOD_SECS_SETTING, RUBY_REPOS_SETTING, SAML_METADATA_SETTING, + SANDBOX_IMAGE_CACHE_MAX_MB_SETTING, SANDBOX_IMAGE_DEFAULT_REGISTRY_SETTING, + SANDBOX_IMAGE_MAX_SIZE_MB_SETTING, SANDBOX_IMAGE_PULL_POLICY_SETTING, + SANDBOX_REGISTRY_AUTH_SETTING, SCIM_TOKEN_SETTING, SERVICE_LOG_RETENTION_SECS_SETTING, + SMTP_SETTING, STORE_AUDIT_LOGS_S3_SETTING, TEAMS_SETTING, TIMEOUT_WAIT_RESULT_SETTING, + UV_EXCLUDE_NEWER_SETTING, UV_INDEX_STRATEGY_SETTING, UV_PYTHON_INSTALL_MIRROR_SETTING, WORKSPACE_FAIRNESS_DURATION_SECS_SETTING, WORKSPACE_FAIRNESS_ENABLED_SETTING, WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING, WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING, WORKSPACE_MAX_QUEUED_JOBS_SETTING, WORKSPACE_REGISTRIES_SETTING, @@ -128,22 +126,21 @@ use windmill_worker::{ use crate::monitor::{ initial_load, load_concurrency_key_max_queued, load_disable_password_login, load_fork_workspace_tag_append_fork_suffix, load_keep_job_dir, - load_mcp_disable_token_query_param, load_metrics_debug_enabled, - load_preview_tags_override, load_require_preexisting_user, load_retention_period_overrides, - load_tag_per_workspace_enabled, load_tag_per_workspace_workspaces, - load_workspace_fairness_duration_secs, load_workspace_fairness_enabled, - load_workspace_fairness_max_percent, load_workspace_fairness_min_total, - load_workspace_max_queued_jobs, monitor_db, reload_app_workspaced_route_setting, - reload_audit_log_retention_days_setting, reload_base_url_setting, - reload_bun_install_min_release_age_setting, reload_bunfig_install_scopes_setting, - reload_critical_alert_mute_ui_setting, reload_critical_alert_mute_zombie_job_restart_setting, + load_mcp_disable_token_query_param, load_metrics_debug_enabled, load_preview_tags_override, + load_require_preexisting_user, load_retention_period_overrides, load_tag_per_workspace_enabled, + load_tag_per_workspace_workspaces, load_workspace_fairness_duration_secs, + load_workspace_fairness_enabled, load_workspace_fairness_max_percent, + load_workspace_fairness_min_total, load_workspace_max_queued_jobs, monitor_db, + reload_app_workspaced_route_setting, reload_audit_log_retention_days_setting, + reload_base_url_setting, reload_bun_install_min_release_age_setting, + reload_bunfig_install_scopes_setting, reload_critical_alert_mute_ui_setting, + reload_critical_alert_mute_zombie_job_restart_setting, reload_critical_alerts_on_token_expiry_setting, reload_critical_error_channels_setting, reload_extra_pip_index_url_setting, reload_http_route_default_allowed_origins_setting, - reload_http_route_workspaced_route_setting, - reload_hub_api_secret_setting, reload_hub_base_url_setting, - reload_instance_events_webhook_setting, reload_job_default_timeout_setting, - reload_job_isolation_setting, reload_jwt_secret_setting, reload_license_key, - reload_npm_config_registry_setting, reload_nsjail_tmp_backing_setting, + reload_http_route_workspaced_route_setting, reload_hub_api_secret_setting, + reload_hub_base_url_setting, reload_instance_events_webhook_setting, + reload_job_default_timeout_setting, reload_job_isolation_setting, reload_jwt_secret_setting, + reload_license_key, reload_npm_config_registry_setting, reload_nsjail_tmp_backing_setting, reload_nsjail_tmpfs_size_setting, reload_otel_traces_retention_secs_setting, reload_otel_tracing_proxy_setting, reload_pip_index_url_setting, reload_retention_period_setting, reload_sandbox_image_cache_max_setting, @@ -422,7 +419,6 @@ struct HubResourceTypeRaw { pub display_name: Option>, } - /// Processed resource type with parsed schema #[derive(serde::Deserialize, serde::Serialize, Clone)] pub struct HubResourceType { @@ -1835,6 +1831,13 @@ async fn process_notify_event( ); windmill_common::variables::CUSTOM_ENVS_CACHE.remove(payload); } + "notify_operator_settings_change" => { + tracing::info!( + "Operator settings change detected, invalidating operator rights cache: {}", + payload + ); + windmill_common::workspaces::invalidate_operator_rights_cache(payload); + } "notify_asset_producer_change" => { tracing::debug!( "Asset producer change for workspace {}, invalidating producer-writes cache", diff --git a/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs b/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs new file mode 100644 index 0000000000..f9769a468a --- /dev/null +++ b/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs @@ -0,0 +1,98 @@ +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() +} + +fn new_schedule(path: &str) -> serde_json::Value { + json!({ + "path": path, + "schedule": "0 0 * * * *", + "timezone": "UTC", + "script_path": "u/operator/some_script", + "is_flow": false, + "enabled": false, + }) +} + +/// Schedules and triggers are rights operators hold until an admin withdraws them, so the stored +/// setting has to survive a payload that never mentions it: this endpoint takes whole-object +/// bodies from git-sync files written before the keys existed, and either default would let such a +/// file silently flip the right on every pull. +#[sqlx::test(migrations = "../migrations", fixtures("base", "permissions_test"))] +async fn test_operator_manage_rights(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 admin = reqwest::Client::new(); + let c = operator_client(); + + let set_settings = async |body: serde_json::Value| -> anyhow::Result { + let resp = admin + .post(format!("{api}/workspaces/operator_settings")) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&body) + .send() + .await?; + let status = resp.status().as_u16(); + invalidate_operator_rights_cache(WS); + Ok(status) + }; + + // Never configured: the right is held. + let resp = c + .post(format!("{api}/schedules/create")) + .json(&new_schedule("u/operator/sched_default")) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + + // An admin withdraws it. + assert_eq!(set_settings(json!({"manage_schedules": false})).await?, 200); + + let resp = c + .post(format!("{api}/schedules/create")) + .json(&new_schedule("u/operator/sched_withdrawn")) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + + // A payload omitting the key must not restore it. This is what an older git-sync settings file + // looks like, and what a serde or SQL default of either polarity would get wrong. + assert_eq!(set_settings(json!({"runs": true})).await?, 200); + + let resp = c + .post(format!("{api}/schedules/create")) + .json(&new_schedule("u/operator/sched_still_withdrawn")) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + + // And the other direction: omitting the key must not withdraw a stored grant, which is what a + // plain `bool` field would do by serializing its own default over it. + assert_eq!(set_settings(json!({"manage_schedules": true})).await?, 200); + assert_eq!(set_settings(json!({"runs": true})).await?, 200); + + let resp = c + .post(format!("{api}/schedules/create")) + .json(&new_schedule("u/operator/sched_restored")) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + + Ok(()) +} diff --git a/backend/windmill-api-schedule/src/lib.rs b/backend/windmill-api-schedule/src/lib.rs index dfd4a6e3ba..a9993e6964 100644 --- a/backend/windmill-api-schedule/src/lib.rs +++ b/backend/windmill-api-schedule/src/lib.rs @@ -38,6 +38,7 @@ use windmill_common::{ escape_ilike_pattern, not_found_if_none, paginate, Pagination, ScheduleType, StripPath, }, worker::to_raw_value, + workspaces::{check_operator_can_manage, ManageKind}, }; use windmill_git_sync::{handle_deployment_metadata, DeployedObject}; use windmill_queue::schedule::push_scheduled_job; @@ -271,6 +272,14 @@ async fn create_schedule( Json(ns): Json, ) -> Result { check_scopes(&authed, || format!("schedules:write:{}", ns.path))?; + check_operator_can_manage( + &db, + &w_id, + authed.is_operator, + ManageKind::Schedules, + "create schedules", + ) + .await?; reject_reserved_schedule_path(&ns.path)?; let authed = maybe_refresh_folders(&ns.path, &w_id, authed, &db).await; @@ -542,6 +551,14 @@ async fn edit_schedule( ) -> Result { let path = path.to_path(); check_scopes(&authed, || format!("schedules:write:{}", path))?; + check_operator_can_manage( + &db, + &w_id, + authed.is_operator, + ManageKind::Schedules, + "edit schedules", + ) + .await?; reject_reserved_schedule_path(path)?; let authed = maybe_refresh_folders(&path, &w_id, authed, &db).await; @@ -1124,9 +1141,20 @@ pub async fn set_enabled( Path((w_id, path)): Path<(String, StripPath)>, Json(payload): Json, ) -> Result { - let mut tx = user_db.begin(&authed).await?; let path = path.to_path(); check_scopes(&authed, || format!("schedules:write:{}", path))?; + // Before the tx: this reads the root pool, and a second pooled connection taken while the RLS + // tx is held self-deadlocks on a single-connection pool. + check_operator_can_manage( + &db, + &w_id, + authed.is_operator, + ManageKind::Schedules, + "enable or disable schedules", + ) + .await?; + + let mut tx = user_db.begin(&authed).await?; reject_reserved_schedule_path(path)?; // Block enabling a schedule in a fork when an ancestor has the same path @@ -1308,6 +1336,14 @@ async fn delete_schedule( ) -> Result { let path = path.to_path(); check_scopes(&authed, || format!("schedules:write:{}", path))?; + check_operator_can_manage( + &db, + &w_id, + authed.is_operator, + ManageKind::Schedules, + "delete schedules", + ) + .await?; reject_reserved_schedule_path(path)?; let mut tx = user_db.begin(&authed).await?; diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 879babff4f..78d01228f1 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -10488,6 +10488,14 @@ 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. + #[serde(default, skip_serializing_if = "Option::is_none")] + manage_schedules: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + manage_triggers: Option, } async fn update_operator_settings( @@ -10502,8 +10510,12 @@ async fn update_operator_settings( let settings_json = serde_json::json!(settings); + // Merge rather than overwrite so an omitted key keeps its stored value. The visibility flags + // are plain bools and always serialize, so this is a no-op for them. sqlx::query!( - "UPDATE workspace_settings SET operator_settings = $1 WHERE workspace_id = $2", + "UPDATE workspace_settings + SET operator_settings = COALESCE(operator_settings, '{}'::jsonb) || $1 + WHERE workspace_id = $2", settings_json, &w_id ) @@ -10512,6 +10524,8 @@ async fn update_operator_settings( tx.commit().await?; + windmill_common::workspaces::invalidate_operator_rights_cache(&w_id); + // Trigger git sync for operator settings changes handle_deployment_metadata( &authed.email, diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index d37c647db2..ea6f3c66de 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -36584,6 +36584,12 @@ components: workers: type: boolean description: Whether operators can view workers page + manage_schedules: + type: boolean + description: Whether operators can create, edit and delete schedules. Granted unless withdrawn; omitting the field leaves the stored value unchanged. + manage_triggers: + type: boolean + description: Whether operators can create, edit and delete triggers. Granted unless withdrawn; omitting the field leaves the stored value unchanged. WorkspaceComparison: type: object diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 711b4dac29..6ffe209f6c 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1186,6 +1186,112 @@ pub fn invalidate_protection_rules_cache(workspace_id: &str) { PROTECTION_RULES_CACHE.remove(workspace_id); } +// Operator rights cache + +lazy_static::lazy_static! { + static ref OPERATOR_RIGHTS_CACHE: Cache = Cache::new(1000); +} + +/// Writes an operator may perform unless the workspace withdraws them. Unlike the visibility +/// flags beside them in `operator_settings`, these are enforced; unlike a right a workspace +/// grants, they are held by default and cost no seat. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct OperatorManageRights { + pub schedules: bool, + pub triggers: bool, +} + +/// Granted unless explicitly withdrawn. Every path that materializes these rights without a +/// stored setting - a workspace with no `workspace_settings` row included - has to land here, or +/// an upgrade silently revokes what operators could do the day before. +impl Default for OperatorManageRights { + fn default() -> Self { + Self { schedules: true, triggers: true } + } +} + +/// Which withdrawable capability a gate needs. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum ManageKind { + Schedules, + Triggers, +} + +impl OperatorManageRights { + pub fn has(&self, kind: ManageKind) -> bool { + match kind { + ManageKind::Schedules => self.schedules, + ManageKind::Triggers => self.triggers, + } + } +} + +/// What operators of this workspace may still write. Per workspace, not per user. +/// +/// Read on every gated write, so it is cached with a 60s TTL. This gates writes, so a withdrawal +/// cannot wait out that TTL on the rest of the fleet: an `AFTER UPDATE OF operator_settings` +/// trigger publishes `notify_operator_settings_change` and every server drops its entry through +/// `process_notify_event`. [`invalidate_operator_rights_cache`] is the local half of that, and +/// what a test flipping the setting directly has to call itself. +/// +/// 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. +pub async fn operator_manage_rights(db: &DB, workspace_id: &str) -> Result { + let now = chrono::Utc::now().timestamp(); + + if let Some((rights, expiry)) = OPERATOR_RIGHTS_CACHE.get(workspace_id) { + if expiry > now { + return Ok(rights); + } + } + + // Coalesced to true, matching `OperatorManageRights::default`: an absent key means the + // workspace never configured the right, not that it withdrew it. + let row = sqlx::query!( + "SELECT 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 + ) + .fetch_optional(db) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to fetch operator settings for {workspace_id}: {e:#}" + )) + })?; + + let rights = row + .map(|r| OperatorManageRights { schedules: r.schedules, triggers: r.triggers }) + .unwrap_or_default(); + + OPERATOR_RIGHTS_CACHE.insert(workspace_id.to_string(), (rights, now + 60)); + + Ok(rights) +} + +/// Invalidate the operator rights cache for a workspace +pub fn invalidate_operator_rights_cache(workspace_id: &str) { + OPERATOR_RIGHTS_CACHE.remove(workspace_id); +} + +/// Gate for a write operators may perform unless the workspace withdrew it. `action` completes +/// "Operators cannot {action} in this workspace". +pub async fn check_operator_can_manage( + db: &DB, + workspace_id: &str, + is_operator: bool, + kind: ManageKind, + action: &str, +) -> Result<()> { + if is_operator && !operator_manage_rights(db, workspace_id).await?.has(kind) { + return Err(Error::NotAuthorized(format!( + "Operators cannot {action} in this workspace" + ))); + } + Ok(()) +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum RuleCheckResult { Allowed, diff --git a/backend/windmill-trigger/src/handler.rs b/backend/windmill-trigger/src/handler.rs index 66d6770354..41301ce5a8 100644 --- a/backend/windmill-trigger/src/handler.rs +++ b/backend/windmill-trigger/src/handler.rs @@ -24,6 +24,7 @@ use windmill_common::{ }, utils::{paginate, Pagination, StripPath}, worker::CLOUD_HOSTED, + workspaces::{check_operator_can_manage, ManageKind}, DB, }; use windmill_git_sync::DeployedObject; @@ -548,6 +549,15 @@ async fn create_trigger( ) })?; + check_operator_can_manage( + &db, + &workspace_id, + authed.is_operator, + ManageKind::Triggers, + "create triggers", + ) + .await?; + if *CLOUD_HOSTED && !T::IS_ALLOWED_ON_CLOUD { return Err(Error::BadRequest(format!( "{} triggers are not supported on multi-tenant cloud, use dedicated cloud or self-host", @@ -818,6 +828,15 @@ async fn update_trigger( ) })?; + check_operator_can_manage( + &db, + &workspace_id, + authed.is_operator, + ManageKind::Triggers, + "edit triggers", + ) + .await?; + edit_trigger.error_handling.validate()?; handler @@ -972,6 +991,14 @@ async fn delete_trigger( check_scopes(&authed, || { format!("{}:write:{}", T::scope_domain_name(), &path) })?; + check_operator_can_manage( + &db, + &workspace_id, + authed.is_operator, + ManageKind::Triggers, + "delete triggers", + ) + .await?; let mut tx = user_db.begin(&authed).await?; @@ -1113,6 +1140,14 @@ async fn set_trigger_mode( ) -> Result { let path = path.to_path(); check_scopes(&authed, || format!("{}:write", T::scope_domain_name()))?; + check_operator_can_manage( + &db, + &workspace_id, + authed.is_operator, + ManageKind::Triggers, + "enable or disable triggers", + ) + .await?; let mut tx = user_db.begin(&authed).await?; diff --git a/docs/operator-write-rights.md b/docs/operator-write-rights.md new file mode 100644 index 0000000000..4895bc2b7b --- /dev/null +++ b/docs/operator-write-rights.md @@ -0,0 +1,40 @@ +# Operator write rights + +Most of `workspace_settings.operator_settings` is visibility: flags that hide pages from operators +so the UI stays uncluttered. They are not enforced, and were never meant to be. + +`manage_schedules` and `manage_triggers` are different. They are **enforced** on the write paths, +because hiding the schedules page never stopped an operator creating a schedule through the API, +the CLI or MCP. An admin who wants operators to see what is scheduled without letting them change +it could not express that with a visibility flag alone. + +Read them with `windmill_common::workspaces::operator_manage_rights` (60s cache) and gate a write +with `check_operator_can_manage`, naming a `ManageKind`. 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. + +The cache is per process, so withdrawing a right has to reach every replica: an `AFTER UPDATE OF +operator_settings` trigger writes a `notify_operator_settings_change` row and `process_notify_event` +drops the entry. Keep both ends if you touch either, or a workspace that withdrew a right keeps +authorizing writes on every other replica until its own entry expires. + +## Granted unless withdrawn + +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 both were wrong in the obvious first draft: + +- The read coalesces to **true** (`operator_manage_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. +- The update endpoint **merges** into the stored jsonb and takes these two as `Option`, so an + omitted key keeps its stored value. It has to: `operator_settings` is git-synced as a whole + object (`cli/src/core/settings.ts` posts the file's contents verbatim), so a settings file + written before these keys existed reaches the endpoint on every pull. A serde or SQL default of + either polarity turns that pull into a silent withdrawal or a silent restoration. + +The visibility flags are plain `bool` and always serialize, so the merge is a no-op for them and +their behaviour is unchanged. + +Do not model a right that an admin *grants* on these. Granted-by-default and granted-on-request +are opposite polarities, and a gate written for one is wrong for the other. diff --git a/frontend/src/lib/components/settings/WorkspaceOperatorSettings.svelte b/frontend/src/lib/components/settings/WorkspaceOperatorSettings.svelte index ba8f42cba7..e00d1a648c 100644 --- a/frontend/src/lib/components/settings/WorkspaceOperatorSettings.svelte +++ b/frontend/src/lib/components/settings/WorkspaceOperatorSettings.svelte @@ -6,6 +6,7 @@ import Section from '$lib/components/Section.svelte' import Head from '$lib/components/table/Head.svelte' import Cell from '$lib/components/table/Cell.svelte' + import Toggle from '$lib/components/Toggle.svelte' import { WorkspaceService } from '$lib/gen' import { workspaceStore } from '$lib/stores' import { sendUserToast } from '$lib/toast' @@ -25,18 +26,33 @@ workers: true }) - let originalSettings = $state({ ...untrack(() => operatorWorkspaceSettings) }) + // Kept out of `operatorWorkspaceSettings` so the visibility table's "Enable all" never flips a + // write right, and so these rows stay out of that table. Withdrawable rather than granted: + // operators hold them until an admin turns them off. + let manageSchedules = $state(true) + let manageTriggers = $state(true) + + let originalSettings = $state({ + ...untrack(() => operatorWorkspaceSettings), + manage_schedules: true, + manage_triggers: true + }) let isChanged = $state(false) let currentWorkspace: string | null = $state(null) + const settingsPayload = $derived({ + ...operatorWorkspaceSettings, + manage_schedules: manageSchedules, + manage_triggers: manageTriggers + }) + async function saveSettings() { - console.log('Saving operator settings:', operatorWorkspaceSettings) try { await WorkspaceService.updateOperatorSettings({ workspace: $workspaceStore!, - requestBody: operatorWorkspaceSettings + requestBody: settingsPayload }) - originalSettings = { ...operatorWorkspaceSettings } + originalSettings = { ...settingsPayload } isChanged = false sendUserToast('Operator settings saved successfully!', false) } catch (error) { @@ -66,18 +82,26 @@ workspace: $workspaceStore }) if (settings.operator_settings !== null) { - operatorWorkspaceSettings = { + const { + manage_schedules: remoteSchedules, + manage_triggers: remoteTriggers, + ...remoteVisibility + } = settings.operator_settings ?? {} + operatorWorkspaceSettings = { ...operatorWorkspaceSettings, ...remoteVisibility } + manageSchedules = remoteSchedules ?? true + manageTriggers = remoteTriggers ?? true + originalSettings = { ...operatorWorkspaceSettings, - ...(settings.operator_settings ?? {}) + manage_schedules: manageSchedules, + manage_triggers: manageTriggers } - originalSettings = { ...operatorWorkspaceSettings } } })() } }) $effect(() => { - isChanged = JSON.stringify(operatorWorkspaceSettings) !== JSON.stringify(originalSettings) + isChanged = JSON.stringify(settingsPayload) !== JSON.stringify(originalSettings) }) const allDisabled = $derived( @@ -105,6 +129,24 @@ {/snippet} +
+ Schedules and triggers + + Operators can create, edit and delete schedules and triggers wherever their folder permissions + let them write. Turn these off to withdraw that. + + + +
+