fix(datatables): fail loudly where a role or a pointer can be left half-recorded

Three ways the feature could end up in a state nobody could see or undo.

Creating a role writes the cluster first and the catalog second, but the catalog write was an
`UPDATE` that matched nothing when the instance Postgres settings row was absent — leaving a
live login with a password nobody recorded: invisible to the catalog, un-recreatable because
the name is taken, and un-deletable because there is no entry to delete. It now errors, so
the operation is retryable once the row is restored.

Deleting a workspace only nulls the fork lineage; the data table entries pointing at it are
left resolving to nothing. Sweeping them is not an option — turning a pointer back into a copy
would hand each fork the database outright — so the delete now names the data tables it
stranded, and resolving one says which workspace is missing rather than reporting a data table
this workspace never had.

`InstanceDatatableRole` derived `Debug` while holding a Postgres password; it is now
hand-written so `{:?}` on the catalog cannot put a live credential in a log line.

Adds the two branches the reviews found unpinned: a caller who is not a member of the
governing workspace at all, and `NoIdentity` — the compatibility path for an agent worker that
predates this and sends no job id.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR

backend/.sqlx/query-c5451ea9d9fa5146af242d1ee8c19ebd65b80e7ed9f29b9fb2e03767c2aa94ba.json

@@ -0,0 +1,28 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n           FROM workspace_settings ws\n           CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n           WHERE dt.value->'reference'->>'workspace_id' = $1\n           ORDER BY ws.workspace_id, dt.key",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "workspace_id!",
+        "type_info": "Varchar"
+      },
+      {
+        "ordinal": 1,
+        "name": "datatable!",
+        "type_info": "Text"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text"
+      ]
+    },
+    "nullable": [
+      false,
+      null
+    ]
+  },
+  "hash": "c5451ea9d9fa5146af242d1ee8c19ebd65b80e7ed9f29b9fb2e03767c2aa94ba"
+}

backend/windmill-api-integration-tests/tests/datatable_roles.rs

@@ -260,3 +260,72 @@ async fn a_fork_renaming_its_own_entry_leaves_the_governing_bookkeeping_alone(
     assert_eq!(left, 1, "the fork's delete reached the parent's migrations");
     Ok(())
 }
+
+#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
+async fn a_caller_who_is_not_a_member_of_the_governing_workspace_reaches_nothing(
+    db: Pool<Postgres>,
+) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    // A fork member who was never added to the parent. Their fork membership says nothing there,
+    // and the email lookup that would evaluate them as a member of it finds no row.
+    sqlx::query(
+        "INSERT INTO usr (workspace_id, email, username, is_admin, role)
+         VALUES ('wm-fork-dt', 'test3@windmill.dev', 'test-user-3', false, 'User')",
+    )
+    .execute(&db)
+    .await?;
+
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+    let resp = authed(
+        client().get(format!(
+            "http://localhost:{port}/api/w/wm-fork-dt/workspaces/datatable_usable_roles/main"
+        )),
+        "SECRET_TOKEN_3",
+    )
+    .send()
+    .await?;
+    assert_eq!(resp.status(), 200);
+    let body: Value = resp.json().await?;
+    assert_eq!(body["roles"], json!([]), "{body}");
+    Ok(())
+}
+
+#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
+async fn a_caller_with_no_identity_reaches_a_permissioned_data_table_not_at_all(
+    db: Pool<Postgres>,
+) -> anyhow::Result<()> {
+    use windmill_common::workspaces::{get_datatable_resource_from_db, DatatableAccess};
+
+    initialize_tracing().await;
+    // The compatibility story for an agent worker that predates data table roles and sends no job
+    // id: it keeps resolving an unpermissioned data table, and is refused on a permissioned one
+    // rather than handed an unattributed admin connection.
+    let refused = get_datatable_resource_from_db(
+        &db,
+        "test-workspace",
+        "main",
+        None,
+        DatatableAccess::NoIdentity,
+    )
+    .await;
+    assert!(refused.is_err(), "an unidentified caller was let in");
+
+    sqlx::query(
+        "UPDATE workspace_settings
+         SET datatable = datatable #- '{datatables,main,permissions}'
+         WHERE workspace_id = 'test-workspace'",
+    )
+    .execute(&db)
+    .await?;
+    let resolved = get_datatable_resource_from_db(
+        &db,
+        "test-workspace",
+        "main",
+        None,
+        DatatableAccess::NoIdentity,
+    )
+    .await?;
+    assert_eq!(resolved["dbname"], "dt_main", "{resolved}");
+    Ok(())
+}

backend/windmill-api-settings/src/lib.rs

@@ -2635,18 +2635,31 @@ fn datatable_role_infos(
 
 /// Persist the catalog next to the instance Postgres password, in the same `global_settings` row
 /// the instance database registry lives in.
+///
+/// Errors when it matches nothing. The cluster is written first, so a silent no-op here would
+/// leave a live Postgres login with a password nobody recorded: invisible to the catalog,
+/// un-recreatable (the name is taken) and un-deletable (there is no entry to delete). The row is
+/// normally planted by the boot converge, but that swallows its own failures, so this is a check
+/// rather than an assumption.
 async fn write_role_catalog(
     db: &DB,
     catalog: &windmill_common::datatable_roles::DatatableRoleCatalog,
 ) -> error::Result<()> {
     let value = serde_json::to_value(catalog).map_err(to_anyhow)?;
-    sqlx::query!(
+    let written = sqlx::query!(
         "UPDATE global_settings SET value = jsonb_set(COALESCE(value, '{}'::jsonb), '{roles}', $1)
          WHERE name = 'custom_instance_pg_databases'",
         value
     )
     .execute(db)
-    .await?;
+    .await?
+    .rows_affected();
+    if written == 0 {
+        return Err(error::Error::internal_err(
+            "The instance Postgres settings row is missing, so the data table role catalog could              not be recorded. Refresh the custom instance user password in instance settings to              recreate it, then try again."
+                .to_string(),
+        ));
+    }
     Ok(())
 }
 

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

@@ -995,6 +995,22 @@ pub(crate) async fn delete_workspace(
     // but the destructive cleanup itself runs only after the commit below: a delete that
     // fails mid-way must never leave a live workspace with its fork data destroyed and no
     // registry row to retry from. Read-only: nothing is dropped here.
+    // Read before the delete: another workspace's data table entry can point at one of this
+    // workspace's, and deleting the workspace it names leaves that pointer resolving to nothing.
+    // Nothing sweeps them — turning them back into copies would hand each fork the database
+    // outright — so the deleter is told which data tables they just stranded.
+    let stranded_pointers = sqlx::query!(
+        r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!"
+           FROM workspace_settings ws
+           CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt
+           WHERE dt.value->'reference'->>'workspace_id' = $1
+           ORDER BY ws.workspace_id, dt.key"#,
+        &w_id,
+    )
+    .fetch_all(&db)
+    .await
+    .unwrap_or_default();
+
     let fork_ducklake_cleanups = prepare_fork_ducklake_cleanups(&db, &w_id, None)
         .await
         .unwrap_or_else(|e| {
@@ -1313,7 +1329,19 @@ pub(crate) async fn delete_workspace(
         tracing::warn!("failed to broadcast fork lineage change: {e:#}");
     }
 
-    Ok(format!("Deleted workspace {}", &w_id))
+    if stranded_pointers.is_empty() {
+        Ok(format!("Deleted workspace {}", &w_id))
+    } else {
+        let stranded = stranded_pointers
+            .iter()
+            .map(|r| format!("{}/{}", r.workspace_id, r.datatable))
+            .collect::<Vec<_>>()
+            .join(", ");
+        Ok(format!(
+            "Deleted workspace {}. These data tables were governed by it and no longer resolve:              {}. Their databases still exist; a superadmin can point them at another workspace's              data table.",
+            &w_id, stranded
+        ))
+    }
 }
 
 #[derive(Deserialize)]

backend/windmill-common/src/datatable_roles.rs

@@ -36,7 +36,7 @@ pub const CUSTOM_INSTANCE_USER: &str = "custom_instance_user";
 /// One catalog entry. The password is per role and instance-wide; it lives here rather than in any
 /// workspace's settings, next to the `custom_instance_user` password in the same
 /// `custom_instance_pg_databases` row.
-#[derive(Deserialize, Serialize, Clone, Debug)]
+#[derive(Deserialize, Serialize, Clone)]
 #[cfg_attr(feature = "instance_config_schema", derive(schemars::JsonSchema))]
 pub struct InstanceDatatableRole {
     /// The Postgres role name, verbatim.
@@ -49,6 +49,18 @@ pub struct InstanceDatatableRole {
     pub pwd: Option<String>,
 }
 
+/// Hand-written so `{:?}` on a catalog cannot put a live Postgres password in a log line or an
+/// audit record. Everything else about the entry is safe to print.
+impl std::fmt::Debug for InstanceDatatableRole {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("InstanceDatatableRole")
+            .field("name", &self.name)
+            .field("enabled", &self.enabled)
+            .field("pwd", &self.pwd.as_ref().map(|_| "<redacted>"))
+            .finish()
+    }
+}
+
 pub type DatatableRoleCatalog = BTreeMap<String, InstanceDatatableRole>;
 
 /// Names Postgres or Windmill already owns. `admin` is excluded because it never reaches the

backend/windmill-common/src/workspaces.rs

@@ -1477,8 +1477,24 @@ pub async fn resolve_governing_datatable(
 ) -> Result<GoverningDatatable> {
     let mut workspace_id = w_id.to_string();
     let mut name = name.to_string();
+    let mut hops = 0;
     for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH {
-        let datatable = read_datatable_entry(db, &workspace_id, &name).await?;
+        let datatable = read_datatable_entry(db, &workspace_id, &name)
+            .await
+            .map_err(|e| {
+                if hops == 0 {
+                    e
+                } else {
+                    // A pointer outlives the workspace it names: deleting one only nulls the fork
+                    // lineage, it does not sweep the entries that pointed at it. Say which one is
+                    // gone rather than reporting a data table this workspace never had.
+                    Error::NotFound(format!(
+                        "Data table '{name}' of workspace '{workspace_id}' governs this one and no \
+                         longer exists. A superadmin can point this data table somewhere else."
+                    ))
+                }
+            })?;
+        hops += 1;
         validate_datatable_shape(&name, &datatable)?;
         match &datatable.reference {
             None => return Ok(GoverningDatatable { workspace_id, name, datatable }),