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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BayTppRCstWX6qTf3LMco5
This commit is contained in:
Ruben Fiszel
2026-09-04 14:19:19 +02:00
co-authored by Claude Fable 5.1
parent 0241ba52b4
commit cb5d00b83b
5 changed files with 78 additions and 23 deletions
@@ -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"
}
+49
View File
@@ -591,6 +591,55 @@ async fn guests_mode_needs_a_scopable_path(db: Pool<Postgres>) -> 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<Postgres>) -> 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(())
}
+2 -1
View File
@@ -2948,7 +2948,8 @@ lazy_static::lazy_static! {
fn guest_session_scopes(app_path: &str) -> Result<Vec<String>> {
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![
@@ -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
)
+24 -19
View File
@@ -336,7 +336,8 @@ fn deployment_rule_for_mode(mode: ExecutionMode) -> Option<ProtectionRuleKind> {
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<String>>(
"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::<ExecutionMode>(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<String>>(
"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::<ExecutionMode>(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,