From 0efddb94cf4489315c820f85d99ecef5bd68c987 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Tue, 8 Sep 2026 17:40:19 +0200 Subject: [PATCH] fix(datatables): validate a rename against the save it describes, and re-check under the locks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three from the round, all about deciding on state that could already have moved. A permission save resolved the data table and checked it was instance-backed before taking any lock, then wrote under one. A config save committing in between could move the table onto a PostgreSQL resource — recreating exactly what the transition guard refuses — or rename it, in which case the write targeted a key that no longer existed and reported success having changed nothing. It now re-resolves and re-checks on the locked state. Rename validation checked that the source existed before and the target existed after, which still accepts `main -> decoy` against a save that keeps both: every fork of `main` then follows onto a different data table, silently, because it keeps resolving. The rule is now the actual old-to-new key transition — a source may only survive if another rename took its name, and a target may only pre-exist if another rename freed it. That also stops two sources sharing one target, and it admits a swap, which the previous guard refused: `datatables` is keyed by name, so a swap cannot be done one save at a time, and refusing it was a regression against main. The pointer cascade now runs in two passes through a temporary name, the way the migration cascade one layer down already handles the same shape, so `A -> B` with `B -> C` moves each pointer once from what it named before the save. The tenant mutators say what they are for: they write an access decision for any workspace named, with an arbitrary mutation, and exist for the transaction that frees or renames a principal. Editing a decision on purpose belongs in the permissions endpoint. Carried in the same change: the stranded-fork list is a field rather than a phrase to grep out of a success string; the pointer cascade matches with `EXISTS` instead of a `LIKE` over the whole document, so a workspace whose pointers name something else is not rewritten to a byte-identical value under an exclusive lock; and `InstanceDatatableRole` drops the serde derives left over from the JSON document, one of which would emit `pwd`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR --- ...73c242f9ba758db66e9a5e16f30e3e1494201.json | 16 ++ ...ed1ce17184271668622c53e94de7224da38ca.json | 14 -- .../tests/datatable_roles.rs | 79 ++++++++ .../src/datatable_permissions.rs | 17 ++ .../windmill-api-workspaces/src/workspaces.rs | 180 +++++++++++------- backend/windmill-api/openapi.yaml | 17 +- .../windmill-common/src/datatable_roles.rs | 9 +- backend/windmill-common/src/workspaces.rs | 17 +- .../DataTableSettings.svelte | 27 ++- 9 files changed, 274 insertions(+), 102 deletions(-) create mode 100644 backend/.sqlx/query-06ce02cd7ce2f5a57355153edb573c242f9ba758db66e9a5e16f30e3e1494201.json delete mode 100644 backend/.sqlx/query-3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca.json diff --git a/backend/.sqlx/query-06ce02cd7ce2f5a57355153edb573c242f9ba758db66e9a5e16f30e3e1494201.json b/backend/.sqlx/query-06ce02cd7ce2f5a57355153edb573c242f9ba758db66e9a5e16f30e3e1494201.json new file mode 100644 index 0000000000..b543f00b44 --- /dev/null +++ b/backend/.sqlx/query-06ce02cd7ce2f5a57355153edb573c242f9ba758db66e9a5e16f30e3e1494201.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings ws\n SET datatable = (\n SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(\n dt.key,\n CASE WHEN dt.value->'reference'->>'workspace_id' = $1\n AND dt.value->'reference'->>'datatable' = $2\n THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text))\n ELSE dt.value END\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE EXISTS (\n SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d\n WHERE d.value->'reference'->>'workspace_id' = $1\n AND d.value->'reference'->>'datatable' = $2\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "06ce02cd7ce2f5a57355153edb573c242f9ba758db66e9a5e16f30e3e1494201" +} diff --git a/backend/.sqlx/query-3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca.json b/backend/.sqlx/query-3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca.json deleted file mode 100644 index 764ddd9808..0000000000 --- a/backend/.sqlx/query-3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE global_settings SET value = jsonb_set(COALESCE(value, '{}'::jsonb), '{roles}', $1)\n WHERE name = 'custom_instance_pg_databases'", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Jsonb" - ] - }, - "nullable": [] - }, - "hash": "3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca" -} diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index dff2b59cb3..feb9aa1b3e 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -433,3 +433,82 @@ async fn renaming_a_governing_data_table_carries_its_forks(db: Pool) - assert_eq!(resp.status(), 200, "{}", resp.text().await?); Ok(()) } + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_rename_has_to_match_the_save_it_claims_to_describe( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let url = format!("http://localhost:{port}/api/w/test-workspace/workspaces/edit_datatable_config"); + + let instance = |path: &str| { + json!({"database": {"resource_type": "instance", "resource_path": path}}) + }; + + // Fork pointers are rewritten from the rename list, so a rename nobody performed moves every + // fork of one data table onto another. `main` survives this save, so it was not renamed. + let resp = authed(client().post(&url), "SECRET_TOKEN") + .json(&json!({ + "settings": {"datatables": {"main": instance("dt_main"), "decoy": instance("dt_two")}}, + "renames": [{"from": "main", "to": "decoy"}], + "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 400, "a forged rename was accepted"); + + let entry: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->'main'->'reference' FROM workspace_settings + WHERE workspace_id = $1", + ) + .bind("wm-fork-dt") + .fetch_one(&db) + .await?; + assert_eq!(entry.unwrap()["datatable"], "main", "the fork was repointed anyway"); + + // A swap is two renames whose sources and targets cross. It cannot be done one at a time — + // `datatables` is keyed by name — so refusing it would be a regression, and applying the two + // in order without a temporary name would carry `main`'s pointers back to `main`. + let resp = authed(client().post(&url), "SECRET_TOKEN") + .json(&json!({ + "settings": {"datatables": {"main": instance("dt_two"), "other": instance("dt_main")}}, + "renames": [{"from": "main", "to": "other"}, {"from": "other", "to": "main"}], + "deleted_datatables": [] + })) + .send() + .await?; + // `other` does not exist yet, so this particular pair is still refused — the swap shape is + // covered by the pair below, which starts from two real data tables. + assert_eq!(resp.status(), 400, "{}", resp.text().await?); + + sqlx::query( + r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, '{datatables,other}', + '{"database": {"resource_type": "instance", "resource_path": "dt_two"}}'::jsonb) + WHERE workspace_id = 'test-workspace'"#, + ) + .execute(&db) + .await?; + + let resp = authed(client().post(&url), "SECRET_TOKEN") + .json(&json!({ + "settings": {"datatables": {"main": instance("dt_two"), "other": instance("dt_main")}}, + "renames": [{"from": "main", "to": "other"}, {"from": "other", "to": "main"}], + "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 200, "a swap was refused: {}", resp.text().await?); + + // The fork named `main`, which is now called `other`. + let entry: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->'main'->'reference' FROM workspace_settings + WHERE workspace_id = $1", + ) + .bind("wm-fork-dt") + .fetch_one(&db) + .await?; + assert_eq!(entry.unwrap()["datatable"], "other", "the swap did not carry the pointer"); + Ok(()) +} diff --git a/backend/windmill-api-workspaces/src/datatable_permissions.rs b/backend/windmill-api-workspaces/src/datatable_permissions.rs index 41ffbb6d35..861df58f53 100644 --- a/backend/windmill-api-workspaces/src/datatable_permissions.rs +++ b/backend/windmill-api-workspaces/src/datatable_permissions.rs @@ -287,6 +287,23 @@ async fn set_datatable_permissions( .fetch_optional(&mut *tx) .await?; + // Everything above was decided on a read taken before the locks. A settings save committing in + // between could have moved this data table onto a PostgreSQL resource — recreating the exact + // state the transition guard refuses — or renamed it, in which case the write below would + // target a key that no longer exists and report success having changed nothing. Re-resolve and + // re-check on the locked state; the earlier pass stays because it is what refuses without + // taking locks at all. + let governing = resolve_governing_datatable(&db, &w_id, &datatable_name).await?; + ensure_governs_datatable(&db, &authed, &w_id, &governing).await?; + if req.permissioned && !governing.is_instance() { + return Err(Error::BadRequest(format!( + "Data table '{}' is backed by a Postgres resource. Data table roles are logins on the \ + Windmill instance's own Postgres, so only a data table on the instance database can \ + use them.", + governing.name + ))); + } + let permissions = if req.permissioned { let catalog = windmill_common::datatable_roles::read_role_catalog_tx(&mut tx).await?; let mut roles: BTreeMap = BTreeMap::new(); diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index bd88c5a055..302c8666af 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3583,13 +3583,28 @@ async fn edit_ducklake_config( Ok(format!("Edit ducklake config for workspace {}", &w_id)) } +/// What a save left behind. `stranded_references` names the data tables in other workspaces that +/// were governed by one this save deleted — a field rather than a sentence in a success string, +/// so the UI decides whether to warn on the data rather than on the server's prose. +#[derive(Serialize)] +pub struct EditDataTableConfigResult { + #[serde(skip_serializing_if = "Vec::is_empty")] + stranded_references: Vec, +} + +#[derive(Serialize)] +pub struct StrandedReference { + workspace_id: String, + datatable: String, +} + async fn edit_datatable_config( authed: ApiAuthed, Extension(db): Extension, Path(w_id): Path, ApiAuthed { is_admin, username, .. }: ApiAuthed, Json(mut new_config): Json, -) -> Result { +) -> JsonResult { require_admin(is_admin, &username)?; let is_superadmin = require_super_admin(&db, &authed).await.is_ok(); @@ -3626,40 +3641,55 @@ async fn edit_datatable_config( for r in &new_config.renames { crate::datatable_migrations::validate_datatable_path_segment(&r.from)?; crate::datatable_migrations::validate_new_datatable_name(&r.to)?; - // A rename is a claim about what this save is doing, and other workspaces' pointers are - // rewritten from it. Unchecked, a caller could submit an unchanged configuration with - // `main -> missing` and repoint every fork of `main` at a name nothing has. - if !old_datatables.contains_key(&r.from) { - return Err(Error::BadRequest(format!( - "Cannot rename data table '{}': this workspace has no such data table", - r.from - ))); - } - if !new_config.settings.datatables.contains_key(&r.to) { - return Err(Error::BadRequest(format!( - "Cannot rename data table '{}' to '{}': the save does not contain '{}'", - r.from, r.to, r.to - ))); - } } - // `A -> B` and `B -> C` applied one after another would move what pointed at `A` all the way - // to `C`. Each pointer moves once, from what it named before this save. - if new_config.renames.len() > 1 { - let mut seen = std::collections::HashSet::new(); + // A rename is a claim about what this save is doing, and other workspaces' pointers are + // rewritten from it — so the claim has to match the configuration it describes, or a caller + // can move every fork of one data table onto another by asserting a rename that did not + // happen. The shape below is what "these old keys became those new keys" actually means. + { + let old_keys = &old_datatables; + let new_keys = &new_config.settings.datatables; + let froms: std::collections::HashSet<&str> = + new_config.renames.iter().map(|r| r.from.as_str()).collect(); + let tos: std::collections::HashSet<&str> = + new_config.renames.iter().map(|r| r.to.as_str()).collect(); + if froms.len() != new_config.renames.len() { + return Err(Error::BadRequest( + "A data table is renamed twice in one save".to_string(), + )); + } + if tos.len() != new_config.renames.len() { + return Err(Error::BadRequest( + "Two data tables are renamed to the same name in one save".to_string(), + )); + } for r in &new_config.renames { - if !seen.insert(r.from.as_str()) { + if !old_keys.contains_key(&r.from) { return Err(Error::BadRequest(format!( - "Data table '{}' is renamed twice in one save", + "Cannot rename data table '{}': this workspace has no such data table", r.from ))); } - } - for r in &new_config.renames { - if seen.contains(r.to.as_str()) && r.to != r.from { + if !new_keys.contains_key(&r.to) { return Err(Error::BadRequest(format!( - "Data table '{}' is both renamed and the target of another rename in one \ - save; do them one at a time", - r.to + "Cannot rename data table '{}' to '{}': the save does not contain '{}'", + r.from, r.to, r.to + ))); + } + // The source has to be gone, or gone-and-reoccupied by another rename — which is what + // a swap is. Without this, `main -> decoy` passes against a save that keeps both, and + // every fork of `main` silently follows onto a different data table. + if new_keys.contains_key(&r.from) && !tos.contains(r.from.as_str()) { + return Err(Error::BadRequest(format!( + "Data table '{}' is renamed to '{}' but the save still contains '{}'", + r.from, r.to, r.from + ))); + } + // And the target has to be free, or freed by another rename. + if old_keys.contains_key(&r.to) && !froms.contains(r.to.as_str()) { + return Err(Error::BadRequest(format!( + "Cannot rename data table '{}' to '{}': '{}' already exists", + r.from, r.to, r.to ))); } } @@ -3789,34 +3819,21 @@ async fn edit_datatable_config( .await?; // A fork points at a data table by name, so a rename here has to follow or every fork's entry - // resolves to nothing. Inside the transaction: the rename and the pointers that name it are one - // change, and half of it is a fork whose jobs stop. - for r in &new_config.renames { - sqlx::query!( - r#"UPDATE workspace_settings ws - SET datatable = ( - SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg( - dt.key, - CASE WHEN dt.value->'reference'->>'workspace_id' = $1 - AND dt.value->'reference'->>'datatable' = $2 - THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text)) - ELSE dt.value END - )) - FROM jsonb_each(ws.datatable->'datatables') dt - ) - WHERE jsonb_typeof(ws.datatable->'datatables') = 'object' - AND ws.datatable::text LIKE '%"reference"%'"#, - &w_id, - &r.from, - &r.to, - ) - .execute(&mut *tx) - .await?; + // resolves to nothing. In two passes through a temporary name, like the migration cascade one + // layer down: applied in order, `sa -> sb` then `sb -> sa` would move what pointed at `sa` all + // the way back to `sa`, and `A -> B`, `B -> C` would carry `A`'s pointers to `C`. Each pointer + // moves once, from what it named before this save. Inside the transaction: the rename and the + // pointers that name it are one change, and half of it is a fork whose jobs stop. + for (i, r) in new_config.renames.iter().enumerate() { + repoint_datatable_references(&mut tx, &w_id, &r.from, &format!("__wm_rename_tmp/{i}")).await?; + } + for (i, r) in new_config.renames.iter().enumerate() { + repoint_datatable_references(&mut tx, &w_id, &format!("__wm_rename_tmp/{i}"), &r.to).await?; } // A deletion cannot be followed the same way — there is nothing to point at any more. Read who // is left stranded so the caller is told, the way deleting a workspace does. - let mut stranded: Vec = Vec::new(); + let mut stranded: Vec = Vec::new(); for name in &new_config.deleted_datatables { let rows = sqlx::query!( r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!" @@ -3829,10 +3846,10 @@ async fn edit_datatable_config( ) .fetch_all(&mut *tx) .await?; - stranded.extend( - rows.into_iter() - .map(|r| format!("{}/{}", r.workspace_id, r.datatable)), - ); + stranded.extend(rows.into_iter().map(|r| StrandedReference { + workspace_id: r.workspace_id, + datatable: r.datatable, + })); } tx.commit().await?; @@ -3849,19 +3866,7 @@ async fn edit_datatable_config( ) .await?; - if stranded.is_empty() { - Ok(format!("Edit datatable config for workspace {}", &w_id)) - } else { - Ok(format!( - concat!( - "Edit datatable config for workspace {}. These data tables were governed by one ", - "you deleted and no longer resolve: {}. Their databases still exist; a superadmin ", - "can point them at another workspace's data table." - ), - &w_id, - stranded.join(", ") - )) - } + Ok(Json(EditDataTableConfigResult { stranded_references: stranded })) } #[derive(Deserialize)] @@ -7826,6 +7831,43 @@ async fn point_kept_datatables_at_parent( Ok(()) } +/// Move every pointer in any workspace that names `(w_id, from)` to `(w_id, to)`. +/// +/// `EXISTS` rather than a `LIKE` over the whole document: the update rewrites the row, so matching +/// every workspace that holds any pointer would rewrite rows to a byte-identical value and hold an +/// exclusive lock on them until commit. +async fn repoint_datatable_references( + tx: &mut Transaction<'_, Postgres>, + w_id: &str, + from: &str, + to: &str, +) -> Result<()> { + sqlx::query!( + r#"UPDATE workspace_settings ws + SET datatable = ( + SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg( + dt.key, + CASE WHEN dt.value->'reference'->>'workspace_id' = $1 + AND dt.value->'reference'->>'datatable' = $2 + THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text)) + ELSE dt.value END + )) + FROM jsonb_each(ws.datatable->'datatables') dt + ) + WHERE EXISTS ( + SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d + WHERE d.value->'reference'->>'workspace_id' = $1 + AND d.value->'reference'->>'datatable' = $2 + )"#, + w_id, + from, + to, + ) + .execute(&mut **tx) + .await?; + Ok(()) +} + async fn apply_forked_datatable( db: &DB, tx: &mut Transaction<'_, Postgres>, diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index a87e9c09be..c36236294c 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -5505,7 +5505,22 @@ paths: description: status content: application/json: - schema: {} + schema: + type: object + properties: + stranded_references: + description: >- + Data tables in other workspaces that were governed by one this save deleted + and no longer resolve. + type: array + items: + type: object + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string /w/{workspace}/workspaces/run_datatable_migrations/{datatable_name}: post: diff --git a/backend/windmill-common/src/datatable_roles.rs b/backend/windmill-common/src/datatable_roles.rs index c2f49cd9b5..ab5e519c3a 100644 --- a/backend/windmill-common/src/datatable_roles.rs +++ b/backend/windmill-common/src/datatable_roles.rs @@ -17,8 +17,6 @@ use std::collections::BTreeMap; -use serde::{Deserialize, Serialize}; - use crate::{ error::{Error, Result}, DB, @@ -35,11 +33,13 @@ pub const CUSTOM_INSTANCE_USER: &str = "custom_instance_user"; /// One catalog entry, as stored in `datatable_role`. The password is per role and instance-wide; /// it belongs to the instance, not to any workspace's settings. -#[derive(Deserialize, Serialize, Clone)] +/// No `Serialize`/`Deserialize`: the catalog is rows now, and a derived `Serialize` would emit +/// `pwd` — the same way out for a credential that the hand-written `Debug` below closes on the log +/// side. +#[derive(Clone)] pub struct InstanceDatatableRole { /// The Postgres role name, verbatim. pub name: String, - #[serde(default = "crate::more_serde::default_true")] pub enabled: bool, /// Absent only for a role whose provisioning did not finish; resolving as it then errors /// rather than falling back to admin. @@ -49,7 +49,6 @@ pub struct InstanceDatatableRole { /// backend, while this one is minted here and never entered by anyone, so there is nothing for /// a ref to point at. Encrypting generated secrets at rest is a separate change that would /// take the replication password with it. - #[serde(default, skip_serializing_if = "Option::is_none")] pub pwd: Option, } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index ec3f834537..20fe602a41 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1927,6 +1927,12 @@ pub async fn ensure_datatable_admin_access( /// transaction. `change` reports whether it touched anything; the row is only written when /// something did. /// +/// Authorization: writes an access decision for any workspace named, with an arbitrary mutation, +/// and checks nothing. It exists for the cascades below — the transaction that frees or renames a +/// principal — so callers MUST be the operation that made the principal change, and MUST run in +/// its transaction. Anything editing a decision on purpose belongs in the permissions endpoint, +/// which is gated on the workspace that governs the data table. +/// /// The tenant lists name principals of this workspace, so anything that frees or renames one has /// to come through here in the same transaction that frees it — otherwise a `u/alice` reused by a /// later account silently inherits her access. @@ -1985,7 +1991,8 @@ where } /// Drop a freed principal (`u/alice`, `g/analysts`, `f/finance`) from every tenant list of one -/// workspace. +/// workspace. Same contract as [`update_datatable_permissions_in_workspace`]: for the transaction +/// that frees the principal, not for editing a decision. pub async fn remove_datatable_tenant_in_workspace( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, w_id: &str, @@ -2003,7 +2010,8 @@ pub async fn remove_datatable_tenant_in_workspace( .await } -/// Follow a renamed principal through every tenant list of one workspace. +/// Follow a renamed principal through every tenant list of one workspace. Same contract as +/// [`update_datatable_permissions_in_workspace`]. pub async fn rename_datatable_tenant_in_workspace( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, w_id: &str, @@ -2034,7 +2042,10 @@ pub async fn rename_datatable_tenant_in_workspace( } /// Strip a deleted instance role from every workspace that had tenanted it, so nothing is left -/// naming a role that no longer exists. A data table whose default role was the deleted one falls +/// naming a role that no longer exists. +/// +/// Authorization: reaches every workspace on the instance. Callers MUST be the superadmin path +/// that just dropped the role from the cluster — it exists to follow that, not to edit tenants. A data table whose default role was the deleted one falls /// back to `admin` — the one role that is always present. pub async fn forget_datatable_role_everywhere(db: &DB, role_id: &str) -> Result<()> { let workspaces = sqlx::query_scalar!( diff --git a/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte b/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte index 19e7499044..9c2d64998b 100644 --- a/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte +++ b/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte @@ -228,16 +228,23 @@ requestBody: { settings, renames, deleted_datatables } }) dataTableSettings = clone(tempSettings) - // The server says here when a delete left another workspace's data table pointing at - // nothing. Swallowing it is what made that failure silent for the person who caused it. - const stranded = typeof result === 'string' && result.includes('no longer resolve') - sendUserToast( - stranded ? result : 'Data table settings saved successfully', - stranded ? 'warning' : 'success', - [], - undefined, - stranded ? 20000 : 5000 - ) + // A delete can leave another workspace's data table governed by nothing. Swallowing + // that is what made it silent for the person who caused it. + const stranded = result?.stranded_references ?? [] + if (stranded.length > 0) { + sendUserToast( + `These data tables were governed by one you deleted and no longer resolve: ${stranded + .map((s) => `${s.workspace_id}/${s.datatable}`) + .join(', ')}. Their databases still exist; a superadmin can point them at another ` + + `workspace's data table.`, + 'warning', + [], + undefined, + 20000 + ) + } else { + sendUserToast('Data table settings saved successfully') + } } catch (e) { sendUserToast(e, true) console.error('Error saving data table settings', e)