From cb5d00b83b5407562d06823030a5b93e05d071e6 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 4 Sep 2026 14:19:19 +0200 Subject: [PATCH 1/2] fix: a workspace rename keeps the guest switch; the rename guard reads the deployed mode under the row lock Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01BayTppRCstWX6qTf3LMco5 --- ...e3620111119abf8dda4bde5b835ff934e4f1.json} | 4 +- backend/tests/app_guest_execution_mode.rs | 49 +++++++++++++++++++ backend/windmill-api-users/src/users.rs | 3 +- .../src/workspaces_extra.rs | 2 +- backend/windmill-api/src/apps.rs | 43 +++++++++------- 5 files changed, 78 insertions(+), 23 deletions(-) rename backend/.sqlx/{query-0c5b02b6b70fb8fd2ab3e6c57897038750a44a67360d342b6ef705ef2e4d3007.json => query-e2eee8de61337b7d093f38e3e393e3620111119abf8dda4bde5b835ff934e4f1.json} (51%) diff --git a/backend/.sqlx/query-0c5b02b6b70fb8fd2ab3e6c57897038750a44a67360d342b6ef705ef2e4d3007.json b/backend/.sqlx/query-e2eee8de61337b7d093f38e3e393e3620111119abf8dda4bde5b835ff934e4f1.json similarity index 51% rename from backend/.sqlx/query-0c5b02b6b70fb8fd2ab3e6c57897038750a44a67360d342b6ef705ef2e4d3007.json rename to backend/.sqlx/query-e2eee8de61337b7d093f38e3e393e3620111119abf8dda4bde5b835ff934e4f1.json index e74bd5de0e..4d49d34d10 100644 --- a/backend/.sqlx/query-0c5b02b6b70fb8fd2ab3e6c57897038750a44a67360d342b6ef705ef2e4d3007.json +++ b/backend/.sqlx/query-e2eee8de61337b7d093f38e3e393e3620111119abf8dda4bde5b835ff934e4f1.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, teams_command_script, teams_team_id, teams_team_name, git_app_installations, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, teams_command_script, teams_team_id, teams_team_name, git_app_installations, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts FROM workspace_settings WHERE workspace_id = $2", + "query": "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, teams_command_script, teams_team_id, teams_team_name, git_app_installations, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, teams_command_script, teams_team_id, teams_team_name, git_app_installations, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled FROM workspace_settings WHERE workspace_id = $2", "describe": { "columns": [], "parameters": { @@ -11,5 +11,5 @@ }, "nullable": [] }, - "hash": "0c5b02b6b70fb8fd2ab3e6c57897038750a44a67360d342b6ef705ef2e4d3007" + "hash": "e2eee8de61337b7d093f38e3e393e3620111119abf8dda4bde5b835ff934e4f1" } diff --git a/backend/tests/app_guest_execution_mode.rs b/backend/tests/app_guest_execution_mode.rs index ec6e307495..0b6cc71bf8 100644 --- a/backend/tests/app_guest_execution_mode.rs +++ b/backend/tests/app_guest_execution_mode.rs @@ -591,6 +591,55 @@ async fn guests_mode_needs_a_scopable_path(db: Pool) -> anyhow::Result resp.text().await? ); + // Set on update: an app that already sits on such a path cannot be switched. + let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) + .json(&json!({ + "path": "u/test-user/x:y", + "summary": "App", + "value": {}, + "policy": { "execution_mode": "publisher", "triggerables_v2": {} } + })) + .send() + .await?; + assert_eq!(resp.status(), 201, "{}", resp.text().await?); + let resp = authed( + client().post(format!("{ws}/apps/update/u/test-user/x:y")), + ADMIN_TOKEN, + ) + .json(&json!({ "policy": { "execution_mode": "guest", "triggerables_v2": {} } })) + .send() + .await?; + assert_eq!(resp.status(), 400, "switched to Guests on a `:` path"); + + Ok(()) +} + +/// Renaming a workspace copies its settings; the guest switch must travel with them, +/// or the rename silently shuts every guest app of the workspace. +#[sqlx::test(fixtures("base"))] +async fn a_workspace_rename_keeps_the_guest_switch(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + enable_guests(port, "test-workspace").await?; + let resp = authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/change_workspace_id" + )), + ADMIN_TOKEN, + ) + .json(&json!({ "new_id": "test-workspace-2", "new_name": "Test workspace 2" })) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + let enabled: bool = sqlx::query_scalar( + "SELECT guest_access_enabled FROM workspace_settings WHERE workspace_id = 'test-workspace-2'", + ) + .fetch_one(&db) + .await?; + assert!(enabled, "the guest switch travels with the workspace"); + Ok(()) } diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index 31f8dea2ee..bda651a8cf 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -2948,7 +2948,8 @@ lazy_static::lazy_static! { fn guest_session_scopes(app_path: &str) -> Result> { if !windmill_common::auth::is_scope_literal_path(app_path) { return Err(Error::BadRequest(format!( - "app path {app_path} cannot be scoped: `:`, `,` and `*` are reserved in scopes" + "app path {app_path} cannot be scoped: `:`, `,` and `*` are reserved in scopes, \ + and a leading `/` never matches a route" ))); } Ok(vec![ diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 1968437e40..172c5781e1 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -113,7 +113,7 @@ pub(crate) async fn change_workspace_id( // Duplicate workspace settings (keep copy in old workspace for reference) info!("Duplicating workspace_settings table"); sqlx::query!( - "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, teams_command_script, teams_team_id, teams_team_name, git_app_installations, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, teams_command_script, teams_team_id, teams_team_name, git_app_installations, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts FROM workspace_settings WHERE workspace_id = $2", + "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, teams_command_script, teams_team_id, teams_team_name, git_app_installations, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, teams_command_script, teams_team_id, teams_team_name, git_app_installations, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled FROM workspace_settings WHERE workspace_id = $2", &rw.new_id, &old_id ) diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index fc5853adbc..ad8799da3b 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -336,7 +336,8 @@ fn deployment_rule_for_mode(mode: ExecutionMode) -> Option { fn refuse_unscopable_guest_app(path: &str, mode: ExecutionMode) -> Result<()> { if matches!(mode, ExecutionMode::Guest) && !windmill_common::auth::is_scope_literal_path(path) { return Err(Error::BadRequest(format!( - "app {path} cannot be set to Guests: `:`, `,` and `*` in a path cannot be scoped" + "app {path} cannot be set to Guests: a path with `:`, `,` or `*`, or a leading `/`, \ + cannot be scoped" ))); } Ok(()) @@ -3355,24 +3356,6 @@ async fn update_app_internal<'a>( // the token's write scope, not just the source path. if let Some(npath) = ns.path.as_deref() { check_scopes(&authed, || format!("apps:write:{}", npath))?; - // The destination is what a guest session would be scoped to; a rename that - // carries no policy keeps the deployed mode. - let mode = match ns.policy.as_ref().and_then(|p| p.stated_execution_mode()) { - Some(mode) => mode, - None => sqlx::query_scalar::<_, Option>( - "SELECT policy->>'execution_mode' FROM app WHERE workspace_id = $1 AND path = $2", - ) - .bind(w_id) - .bind(path) - .fetch_optional(&db) - .await? - .flatten() - .and_then(|m| { - serde_json::from_value::(serde_json::Value::String(m)).ok() - }) - .unwrap_or_default(), - }; - refuse_unscopable_guest_app(npath, mode)?; } if raw_app { @@ -3445,6 +3428,28 @@ async fn update_app_internal<'a>( if npath != path { require_owner_of_path(&authed, path)?; + // The destination is what a guest session would be scoped to. A rename + // that carries no policy keeps the deployed mode, read under the row + // lock so a policy update landing alongside cannot slip a guest app + // onto a path it cannot be scoped to. + let mode = match ns.policy.as_ref().and_then(|p| p.stated_execution_mode()) { + Some(mode) => mode, + None => sqlx::query_scalar::<_, Option>( + "SELECT policy->>'execution_mode' FROM app + WHERE path = $1 AND workspace_id = $2 FOR UPDATE", + ) + .bind(path) + .bind(w_id) + .fetch_optional(&mut *tx) + .await? + .flatten() + .and_then(|m| { + serde_json::from_value::(serde_json::Value::String(m)).ok() + }) + .unwrap_or_default(), + }; + refuse_unscopable_guest_app(npath, mode)?; + let exists = sqlx::query_scalar!( "SELECT EXISTS(SELECT 1 FROM app WHERE path = $1 AND workspace_id = $2)", npath, From 6169076d70f36b509f940aa19412fda3c41ea5d7 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 4 Sep 2026 14:30:30 +0200 Subject: [PATCH 2/2] fix: guest_activity follows a workspace rename and goes with a workspace delete Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01BayTppRCstWX6qTf3LMco5 --- backend/tests/app_guest_execution_mode.rs | 13 +++++++++++++ .../windmill-api-workspaces/src/workspaces_extra.rs | 12 ++++++++++++ 2 files changed, 25 insertions(+) diff --git a/backend/tests/app_guest_execution_mode.rs b/backend/tests/app_guest_execution_mode.rs index 0b6cc71bf8..a47129e600 100644 --- a/backend/tests/app_guest_execution_mode.rs +++ b/backend/tests/app_guest_execution_mode.rs @@ -623,6 +623,12 @@ async fn a_workspace_rename_keeps_the_guest_switch(db: Pool) -> anyhow let port = server.addr.port(); enable_guests(port, "test-workspace").await?; + sqlx::query( + "INSERT INTO guest_activity (email, workspace_id, day) + VALUES ('guest@example.com', 'test-workspace', CURRENT_DATE)", + ) + .execute(&db) + .await?; let resp = authed( client().post(format!( "http://localhost:{port}/api/w/test-workspace/workspaces/change_workspace_id" @@ -639,6 +645,13 @@ async fn a_workspace_rename_keeps_the_guest_switch(db: Pool) -> anyhow .fetch_one(&db) .await?; assert!(enabled, "the guest switch travels with the workspace"); + let moved: bool = sqlx::query_scalar( + "SELECT EXISTS(SELECT 1 FROM guest_activity WHERE workspace_id = 'test-workspace-2') + AND NOT EXISTS(SELECT 1 FROM guest_activity WHERE workspace_id = 'test-workspace')", + ) + .fetch_one(&db) + .await?; + assert!(moved, "the guests seen in the workspace follow its new id"); Ok(()) } diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 172c5781e1..f451138859 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -187,6 +187,13 @@ pub(crate) async fn change_workspace_id( .execute(&mut *tx) .await?; + info!("Updating guest_activity table"); + sqlx::query("UPDATE guest_activity SET workspace_id = $1 WHERE workspace_id = $2") + .bind(&rw.new_id) + .bind(&old_id) + .execute(&mut *tx) + .await?; + info!("Updating workspace_invite table"); sqlx::query!( "UPDATE workspace_invite SET workspace_id = $1 WHERE workspace_id = $2", @@ -1112,6 +1119,11 @@ pub(crate) async fn delete_workspace( .execute(&mut *tx) .await?; + sqlx::query("DELETE FROM guest_activity WHERE workspace_id = $1") + .bind(&w_id) + .execute(&mut *tx) + .await?; + sqlx::query!("DELETE FROM token WHERE workspace_id = $1", &w_id) .execute(&mut *tx) .await?;