From e6794984d2002a06bfddb3f3cf069ecb85f26484 Mon Sep 17 00:00:00 2001
From: Diego Imbert <diego@windmill.dev>
Date: Thu, 10 Sep 2026 08:16:58 +0200
Subject: [PATCH] fix(datatables): refuse to roll back the catalog while roles
 exist
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The down migration dropped the table and left every role behind: live Postgres
logins whose passwords only that table carried, so after a revert Windmill could
neither use, disable nor delete them, and re-applying could not recreate them
because the names were taken. Cleaning up here is not possible either — dropping
a role means reassigning what it owns in every instance database, and a
migration runs in one — so it now refuses while the catalog is non-empty and
says to delete the roles through instance settings, which does the cluster work.

Also enforces the instance-only invariant the resolved-pointer clone relies on
rather than only asserting it in a comment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb
---
 ...0908142148_datatable_role_catalog.down.sql | 15 +++++++++
 .../windmill-api-workspaces/src/workspaces.rs | 32 +++++++++++++------
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/backend/migrations/20260908142148_datatable_role_catalog.down.sql b/backend/migrations/20260908142148_datatable_role_catalog.down.sql
index 0bbf50bc89..26ad1c62d4 100644
--- a/backend/migrations/20260908142148_datatable_role_catalog.down.sql
+++ b/backend/migrations/20260908142148_datatable_role_catalog.down.sql
@@ -1 +1,16 @@
+-- Refuse while the catalog holds anything. Each row is a live Postgres login with a password
+-- only this table carries, so dropping it would leave credentials on the cluster that Windmill can
+-- no longer disable, delete or even name — and re-applying could not recreate them, because the
+-- role names would already be taken. Cleaning them up here is not an option either: dropping a
+-- role means reassigning what it owns in *every* instance database, and a migration runs in one.
+--
+-- Delete the roles through instance settings first; that path does the cluster work.
+DO $$
+BEGIN
+    IF EXISTS (SELECT 1 FROM datatable_role) THEN
+        RAISE EXCEPTION 'Cannot roll back: % data table role(s) still exist as Postgres logins. Delete them in instance settings first, which drops them from the cluster.',
+            (SELECT count(*) FROM datatable_role);
+    END IF;
+END $$;
+
 DROP TABLE IF EXISTS datatable_role;
diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs
index e6dc50b789..74c6f833be 100644
--- a/backend/windmill-api-workspaces/src/workspaces.rs
+++ b/backend/windmill-api-workspaces/src/workspaces.rs
@@ -7969,16 +7969,30 @@ async fn apply_forked_datatable(
     // (`point_kept_datatables_at_parent`), so the resolved entry is one.
     let database = match dt.database.clone() {
         Some(database) => database,
-        None => resolve_governing_datatable(db, parent_w_id, &fdt.name)
-            .await?
-            .datatable
-            .database
-            .ok_or_else(|| {
-                Error::internal_err(format!(
-                    "Data table '{}' resolves to an entry that owns no database",
+        None => {
+            let resolved = resolve_governing_datatable(db, parent_w_id, &fdt.name)
+                .await?
+                .datatable
+                .database
+                .ok_or_else(|| {
+                    Error::internal_err(format!(
+                        "Data table '{}' resolves to an entry that owns no database",
+                        fdt.name
+                    ))
+                })?;
+            // The resource branch below rewrites a resource this workspace owns; a pointer names
+            // one it does not, so following it there would move the fork onto someone else's
+            // database. Checked rather than assumed: only `point_kept_datatables_at_parent`
+            // writes pointers and only for instance entries, but nothing here enforces that.
+            if resolved.resource_type != DataTableCatalogResourceType::Instance {
+                return Err(Error::BadRequest(format!(
+                    "Data table '{}' points at a resource-backed data table in another \
+                     workspace and cannot be cloned; fork it from the workspace that owns it.",
                     fdt.name
-                ))
-            })?,
+                )));
+            }
+            resolved
+        }
     };
 
     if database.resource_type == DataTableCatalogResourceType::Instance {