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..a47129e600 100644 --- a/backend/tests/app_guest_execution_mode.rs +++ b/backend/tests/app_guest_execution_mode.rs @@ -591,6 +591,68 @@ 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?; + 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" + )), + 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"); + 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-auth/src/scopes.rs b/backend/windmill-api-auth/src/scopes.rs index 5bd7b4d275..7409237296 100644 --- a/backend/windmill-api-auth/src/scopes.rs +++ b/backend/windmill-api-auth/src/scopes.rs @@ -805,7 +805,8 @@ pub fn guest_session_scopes(app_path: &str) -> windmill_common::error::Result 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(()) @@ -3359,24 +3360,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 { @@ -3449,6 +3432,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,