From 2d09d33997318e2c42bdbf207bd70bd9df6cfe2c Mon Sep 17 00:00:00 2001
From: Diego Imbert <diego@windmill.dev>
Date: Fri, 4 Sep 2026 11:47:52 +0200
Subject: [PATCH] fix(datatables): the three transactions that still locked
 before the settings row
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Offboarding reassigns a departing user's scripts, flows, apps and resources —
rows a rename writes while holding the settings row — so it takes that row
first, and offboarding from several workspaces walks them in `workspace_id`
order. Removing a member locked the `usr` row before the removal reached the
settings row, and a rename renamed the `password` row before its first
workspace: both now come after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5arH3G2Sa1Qqm32veJQ1n
---
 backend/windmill-api-users/src/users.rs |  5 +++++
 backend/windmill-api/src/offboarding.rs | 17 +++++++++--------
 backend/windmill-api/src/users.rs       | 18 ++++++++++--------
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs
index 1b82897622..1084d274e2 100644
--- a/backend/windmill-api-users/src/users.rs
+++ b/backend/windmill-api-users/src/users.rs
@@ -2585,6 +2585,11 @@ async fn delete_workspace_user(
 ) -> Result<String> {
     let mut tx = db.begin().await?;
 
+    // Before the `usr` row below — see `lock_workspace_settings_unchecked`. The
+    // removal itself takes this row too; re-acquiring it inside a transaction
+    // costs nothing.
+    windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, &w_id).await?;
+
     // Locked so that the authorization below and the delete it guards see the same row.
     let target = sqlx::query!(
         "SELECT email, is_admin FROM usr where username = $1 AND workspace_id = $2 FOR UPDATE",
diff --git a/backend/windmill-api/src/offboarding.rs b/backend/windmill-api/src/offboarding.rs
index 737db81b3e..c645ba60cc 100644
--- a/backend/windmill-api/src/offboarding.rs
+++ b/backend/windmill-api/src/offboarding.rs
@@ -488,8 +488,10 @@ pub(crate) async fn global_offboard_preview(
 ) -> JsonResult<GlobalOffboardPreview> {
     require_super_admin(&db, &authed).await?;
 
+    // Ordered, so offboarding from several workspaces takes their settings rows
+    // in the same sequence as every other multi-workspace path.
     let workspaces = sqlx::query!(
-        "SELECT workspace_id, username FROM usr WHERE email = $1",
+        "SELECT workspace_id, username FROM usr WHERE email = $1 ORDER BY workspace_id",
         &email
     )
     .fetch_all(&db)
@@ -830,6 +832,12 @@ async fn offboard_user_from_workspace<'c>(
     reassign_to: &str,
     new_permissioned_as: &str,
 ) -> Result<OffboardSummary> {
+    // Before this transaction locks anything else — see
+    // `lock_workspace_settings_unchecked`. Everything below reassigns rows a
+    // rename or a deletion writes while holding this row.
+    let datatable_settings =
+        windmill_common::workspaces::lock_workspace_settings_unchecked(tx, w_id).await?;
+
     let new_prefix = reassign_to.to_string();
     let departing = windmill_common::users::username_to_permissioned_as(username);
 
@@ -989,13 +997,6 @@ async fn offboard_user_from_workspace<'c>(
     // A data table names its database by resource path, so one just moved has to
     // move in the config too: left behind it stops resolving, and the path it
     // named is free for a resource pointing somewhere else entirely.
-    let datatable_settings = sqlx::query_scalar!(
-        "SELECT datatable FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE",
-        w_id
-    )
-    .fetch_optional(&mut **tx)
-    .await?
-    .flatten();
     if let Some(mut settings) = datatable_settings {
         if windmill_common::workspaces::move_datatable_resource_paths(
             &mut settings,
diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs
index aeb1423ce5..a52b9fb50b 100644
--- a/backend/windmill-api/src/users.rs
+++ b/backend/windmill-api/src/users.rs
@@ -208,14 +208,6 @@ async fn rename_user(
         )));
     }
 
-    sqlx::query!(
-        "UPDATE password SET username = $1 WHERE email = $2",
-        ru.new_username,
-        user_email
-    )
-    .execute(&mut *tx)
-    .await?;
-
     // Ordered, so a rename and a deletion that touch the same workspaces take
     // their settings rows in the same sequence rather than head-on.
     let workspace_usernames = sqlx::query!(
@@ -240,6 +232,16 @@ async fn rename_user(
         .await?;
     }
 
+    // After the settings rows above: the deletion paths take `password` while
+    // holding one, so taking it first here would be the other order.
+    sqlx::query!(
+        "UPDATE password SET username = $1 WHERE email = $2",
+        ru.new_username,
+        user_email
+    )
+    .execute(&mut *tx)
+    .await?;
+
     audit_log(
         &mut *tx,
         &authed,