From 7c16d482fd1d75116481ac32b39c801881916f5e Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Thu, 17 Sep 2026 16:34:14 +0200 Subject: [PATCH] fix(datatables): refuse fork cleanup of an external database another workspace uses Co-Authored-By: Claude Opus 5 (1M context) --- backend/ee-repo-ref.txt | 2 +- backend/windmill-api-settings/src/lib.rs | 2 +- .../windmill-api-workspaces/src/workspaces_extra.rs | 5 ++++- backend/windmill-common/src/external_instance_pg.rs | 13 +++++++++---- .../windmill-common/src/external_instance_pg_oss.rs | 2 +- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 7532fcaed6..6f1b5b251b 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -f920b69b68d0ce2e907774c75573ffd4d9f916ca \ No newline at end of file +3bb6bfdadf005e6729dd17e5dcace9ac24bc244f \ No newline at end of file diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 3bf04d1ef7..5fee9ee99a 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -1887,7 +1887,7 @@ async fn drop_external_instance_pg_database( require_super_admin(&db, &authed).await?; // A data table naming a dropped database fails on every job, far from the drop that caused it. windmill_common::external_instance_pg::drop_external_instance_database_unchecked( - &db, &dbname, true, + &db, &dbname, None, ) .await?; windmill_audit::audit_oss::audit_log( diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 975c178982..abb2e60739 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -1421,8 +1421,11 @@ pub async fn drop_forked_datatable_databases( let dropped = if database.resource_type == windmill_common::workspaces::DataTableCatalogResourceType::ExternalInstance { + // Its own entry still names the copy; another workspace's never should. windmill_common::external_instance_pg::drop_external_instance_database_unchecked( - &db, db_to_drop, false, + &db, + db_to_drop, + Some(&w_id), ) .await } else { diff --git a/backend/windmill-common/src/external_instance_pg.rs b/backend/windmill-common/src/external_instance_pg.rs index 957d908ffc..38773fc5d7 100644 --- a/backend/windmill-common/src/external_instance_pg.rs +++ b/backend/windmill-common/src/external_instance_pg.rs @@ -215,19 +215,20 @@ pub async fn create_external_instance_database_unchecked( } /// Drop `dbname` from the external cluster: only a database Windmill registered creating, and still -/// carries the mark it set there. With `refuse_if_used`, refuse while a data table names it. +/// carries the mark it set there. Refused while a data table names it, except one in +/// `usage_allowed_in`: the fork whose own copy is being cleaned up. /// /// Authorization: checks nothing. Callers MUST be superadmin, or be deleting the fork that owns /// this `wm_fork_` database. pub async fn drop_external_instance_database_unchecked( db: &DB, dbname: &str, - refuse_if_used: bool, + usage_allowed_in: Option<&str>, ) -> Result<()> { crate::external_instance_pg_oss::drop_external_instance_database_unchecked( db, dbname, - refuse_if_used, + usage_allowed_in, ) .await } @@ -273,7 +274,8 @@ pub async fn ensure_external_instance_database_registered( /// check taken outside it could pass while a database create still reads the old cluster, which /// would then register a database there after the setting names another one. /// -/// Authorization: checks nothing. Callers MUST be superadmin. +/// Authorization: checks nothing. Callers MUST be superadmin, or the declarative instance config +/// sync, which applies what the operator deployed. pub async fn write_external_instance_pg_setting( db: &DB, value: Option<&serde_json::Value>, @@ -316,6 +318,9 @@ pub async fn write_external_instance_pg_setting( /// [`write_external_instance_pg_setting`] for a settings diff: writes the key if the diff touches /// it, and takes it out of the diff so the generic apply does not write it again. +/// +/// Authorization: checks nothing. Callers MUST be superadmin, or the declarative instance config +/// sync, which applies what the operator deployed. pub async fn write_external_instance_pg_from_diff( db: &DB, diff: &mut crate::instance_config::SettingsDiff, diff --git a/backend/windmill-common/src/external_instance_pg_oss.rs b/backend/windmill-common/src/external_instance_pg_oss.rs index 7f6168878e..a04a8c9cd6 100644 --- a/backend/windmill-common/src/external_instance_pg_oss.rs +++ b/backend/windmill-common/src/external_instance_pg_oss.rs @@ -73,7 +73,7 @@ mod ce { pub(crate) async fn drop_external_instance_database_unchecked( _db: &DB, _dbname: &str, - _refuse_if_used: bool, + _usage_allowed_in: Option<&str>, ) -> Result<()> { Err(unavailable()) }