fix(datatables): check the external cluster setting on its own transaction, and gate the registry probe

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

backend/windmill-api-workspaces/src/workspaces.rs

@@ -4106,6 +4106,14 @@ async fn edit_datatable_config(
         if unchanged {
             continue;
         }
+        // Before the registration check, whose refusal would otherwise tell a workspace admin
+        // which databases exist on the cluster.
+        if !is_superadmin {
+            return Err(Error::BadRequest(
+                "Only superadmins can create or modify data tables with Instance databases"
+                    .to_string(),
+            ));
+        }
         if database.resource_type == DataTableCatalogResourceType::ExternalInstance {
             windmill_common::external_instance_pg::ensure_external_instance_available()?;
             windmill_common::external_instance_pg::ensure_external_instance_database_registered(
@@ -4114,12 +4122,6 @@ async fn edit_datatable_config(
             )
             .await?;
         }
-        if !is_superadmin {
-            return Err(Error::BadRequest(
-                "Only superadmins can create or modify data tables with Instance databases"
-                    .to_string(),
-            ));
-        }
     }
 
     // Worked out from the locked entries rather than taken from `deleted_datatables`: a settings

backend/windmill-api/openapi.yaml

@@ -34089,7 +34089,7 @@ components:
           type: array
           items:
             type: string
-          description: Workspaces that reference this database via a ducklake catalog or datatable database with resource_type 'instance'. Computed at request time, not persisted.
+          description: Workspaces that reference this database through a ducklake catalog or a datatable database of the kind being listed — 'instance' for the instance databases endpoint, 'external_instance' for the external cluster one. Computed at request time, not persisted, and only returned to superadmins.
         workspace_id:
           type: string
           description: The workspace a member created this database for as a fork copy. Only that workspace can import into it or point a fork at it.

backend/windmill-common/src/external_instance_pg.rs

@@ -178,9 +178,9 @@ pub async fn external_instance_database_usages<'c>(
 /// Refuse to unset the cluster while Windmill still has databases on it, or a workspace still
 /// points at one: every data table there would stop resolving. Allowed on every edition, so a
 /// downgraded instance can still clear a setting it no longer uses.
-pub async fn ensure_external_instance_pg_removable(db: &DB) -> Result<()> {
-    let state = read_external_instance_pg_state(db).await?;
-    let usages = external_instance_database_usages(db).await?;
+pub async fn ensure_external_instance_pg_removable(conn: &mut sqlx::PgConnection) -> Result<()> {
+    let state = read_external_instance_pg_state(&mut *conn).await?;
+    let usages = external_instance_database_usages(&mut *conn).await?;
     if state.databases.is_empty() && usages.is_empty() {
         return Ok(());
     }
@@ -270,6 +270,9 @@ pub async fn lock_external_instance_pg_state(
 /// Refuse a data table naming `dbname` unless Windmill created it on the external cluster. Takes
 /// the lock drops take, so none can remove the database before `tx`, which saves the data table,
 /// commits.
+///
+/// Authorization: its refusal says whether Windmill created a database of that name, which is
+/// instance-wide knowledge. Callers MUST have authorized the caller as superadmin first.
 pub async fn ensure_external_instance_database_registered(
     tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
     dbname: &str,
@@ -309,9 +312,12 @@ pub async fn write_external_instance_pg_setting(
     };
     let mut tx = db.begin().await?;
     lock_external_instance_pg_state(&mut tx).await?;
+    // Every check runs on this transaction's own connection: it holds the advisory lock, and
+    // taking a second connection from the pool while other writers queue on that lock is how a
+    // small pool deadlocks.
     match value {
         None => {
-            ensure_external_instance_pg_removable(db).await?;
+            ensure_external_instance_pg_removable(&mut tx).await?;
             sqlx::query("DELETE FROM global_settings WHERE name = $1")
                 .bind(EXTERNAL_INSTANCE_PG_SETTING)
                 .execute(&mut *tx)
@@ -319,7 +325,7 @@ pub async fn write_external_instance_pg_setting(
         }
         Some(value) => {
             crate::external_instance_pg_oss::validate_external_instance_pg_setting(value)?;
-            ensure_external_instance_pg_not_repointed(db, value).await?;
+            ensure_external_instance_pg_not_repointed(&mut tx, value).await?;
             sqlx::query(
                 "INSERT INTO global_settings (name, value) VALUES ($1, $2)
                  ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value, updated_at = now()",
@@ -365,10 +371,10 @@ pub async fn write_external_instance_pg_from_diff(
 /// Data tables name databases, not clusters, so they would silently resolve to whatever the new
 /// cluster holds under the same names. Other fields (admin login, sslmode) may change freely.
 async fn ensure_external_instance_pg_not_repointed(
-    db: &DB,
+    conn: &mut sqlx::PgConnection,
     value: &serde_json::Value,
 ) -> Result<()> {
-    let Some(current) = read_external_instance_pg_config(db).await? else {
+    let Some(current) = read_external_instance_pg_config(&mut *conn).await? else {
         return Ok(());
     };
     let Ok(desired) = serde_json::from_value::<ExternalInstancePg>(value.clone()) else {
@@ -377,8 +383,8 @@ async fn ensure_external_instance_pg_not_repointed(
     if external_instance_pg_address(&current) == external_instance_pg_address(&desired) {
         return Ok(());
     }
-    let state = read_external_instance_pg_state(db).await?;
-    let usages = external_instance_database_usages(db).await?;
+    let state = read_external_instance_pg_state(&mut *conn).await?;
+    let usages = external_instance_database_usages(&mut *conn).await?;
     if state.databases.is_empty() && usages.is_empty() {
         return Ok(());
     }