From 4cba79b7f00c689ac6d5dd52bf7f22b0248232aa Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Thu, 17 Sep 2026 18:41:11 +0200 Subject: [PATCH 1/5] fix(datatables): authenticate instance database setup before writing its status, and keep a fork reservation across it Co-Authored-By: Claude Opus 5 (1M context) --- backend/windmill-api-settings/src/lib.rs | 25 ++++++++++++++++++++++-- backend/windmill-api/openapi.yaml | 3 +++ backend/windmill-common/src/lib.rs | 4 ++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 5d01baafbd..096bf4e9b0 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -1647,6 +1647,8 @@ struct CustomInstanceDb { tag: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] used_by_workspaces: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + workspace_id: Option, } #[derive(Deserialize, Debug, Serialize, Default)] @@ -1754,12 +1756,31 @@ async fn setup_custom_instance_pg_database( Path(dbname): Path, Json(body): Json, ) -> JsonResult { + // Before anything is recorded: the status written below replaces the registry entry, and with it + // the workspace a fork copy is reserved for. + require_super_admin(&db, &authed).await?; + // A re-run keeps the fork reservation: without it, the workspace the copy was made for could no + // longer import into it or finish its fork. + let workspace_id = sqlx::query_scalar::<_, Option>( + "SELECT value->'databases'->$1->>'workspace_id' FROM global_settings + WHERE name = 'custom_instance_pg_databases'", + ) + .bind(&dbname) + .fetch_optional(&db) + .await? + .flatten(); let mut logs = CustomInstanceDbLogs::default(); let result = setup_custom_instance_pg_database_inner(authed, &db, &dbname, &mut logs).await; let success = result.is_ok(); let error = result.err().map(|e| e.to_string()); - let status = - CustomInstanceDb { logs, success, error, tag: body.tag, used_by_workspaces: vec![] }; + let status = CustomInstanceDb { + logs, + success, + error, + tag: body.tag, + used_by_workspaces: vec![], + workspace_id, + }; let status_json = serde_json::to_value(&status).map_err(to_anyhow)?; // Save that the database was setup successfully sqlx::query!( diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index a3292ecfbc..dde6d53d97 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -33954,6 +33954,9 @@ components: 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. + 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. NewSqsTrigger: type: object diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index ab3a1dc46b..db52194730 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -1653,6 +1653,10 @@ pub async fn create_custom_instance_database( /// `dbname`, unless `w_id` created it for that ([`create_custom_instance_database`]) and nothing uses /// it yet. The `wm_fork_` prefix is no authorization: every instance database answers to the same /// `custom_instance_user`, so a name is all it takes to reach another workspace's copy. +/// +/// Authorization: reads the global registry and every workspace's settings, and names other +/// workspaces in its refusal. Callers MUST have authorized `w_id` for the caller first — a member +/// of it forking or importing there — and MUST NOT call it on a workspace the caller is not in. pub async fn ensure_fork_database_available_to( db: &DB, dbname: &str, From addd77eaf4d1d7f264e6d40c05cbe9914a34515c Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Thu, 17 Sep 2026 18:43:49 +0200 Subject: [PATCH 2/5] fix(datatables): create external databases only on a cluster setup succeeded on, and document the registry reader Co-Authored-By: Claude Opus 5 (1M context) From 57211d6c95e8082f6668d83166f35a14cb8c7a17 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Fri, 18 Sep 2026 00:55:35 +0200 Subject: [PATCH 3/5] fix(datatables): keep only the most recently used DuckDB root certificate files Co-Authored-By: Claude Opus 5 (1M context) --- .../windmill-worker/src/duckdb_executor.rs | 53 +++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/backend/windmill-worker/src/duckdb_executor.rs b/backend/windmill-worker/src/duckdb_executor.rs index 1e42a6f3e2..9a9b1da028 100644 --- a/backend/windmill-worker/src/duckdb_executor.rs +++ b/backend/windmill-worker/src/duckdb_executor.rs @@ -2265,20 +2265,55 @@ fn pg_attach_verification(res: &PgDatabase) -> Result = entries + .filter_map(|e| e.ok()) + .map(|e| e.path()) + .filter(|p| p.extension().is_some_and(|x| x == "pem") && p != keep) + .filter_map(|p| Some((std::fs::metadata(&p).ok()?.modified().ok()?, p))) + .collect(); + if files.len() < PG_ROOTS_KEPT { + return; + } + files.sort(); + for (_, p) in &files[..=files.len() - PG_ROOTS_KEPT] { + let _ = std::fs::remove_file(p); + } +} + fn pg_attach_uri(res: &PgDatabase) -> Result { let uri = res.to_uri(); let Some((mode, roots)) = pg_attach_verification(res)? else { @@ -2881,6 +2916,16 @@ mod tests { assert!(uri.contains("?sslmode=verify-full&sslrootcert="), "{uri}"); let root = urlencoding::decode(uri.split("sslrootcert=").nth(1).unwrap()).unwrap(); let roots = std::fs::read_to_string(root.as_ref()).unwrap(); + for i in 0..(PG_ROOTS_KEPT + 5) { + let mut other = pg("verify-full", Some(false)); + other.root_certificate_pem = Some(format!("-----BEGIN CERTIFICATE-----{i}")); + pg_attach_uri(&other).unwrap(); + } + let kept = std::fs::read_dir(std::env::temp_dir().join("windmill-pg-roots")) + .unwrap() + .filter(|e| e.as_ref().unwrap().path().extension().is_some_and(|x| x == "pem")) + .count(); + assert!(kept <= PG_ROOTS_KEPT, "{kept} root files kept"); assert!(roots.contains("-----BEGIN CERTIFICATE-----test")); let external = serde_json::to_value(pg("verify-full", Some(false))).unwrap(); let attach = &pg_secret_attach_statements(external, "dt").unwrap()[3]; From e550374f06c0694f8dfdd03effededf5b0aba8fe Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Fri, 18 Sep 2026 00:58:19 +0200 Subject: [PATCH 4/5] fix(datatables): migrate fork reservations on workspace rename, and lock the parent's data tables for the whole fork Co-Authored-By: Claude Opus 5 (1M context) --- backend/windmill-api-workspaces/src/workspaces.rs | 4 +++- .../src/workspaces_extra.rs | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 2d840d9e51..411bc8538c 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -8039,7 +8039,6 @@ async fn point_kept_datatables_at_parent( forked_w_id: &str, cloned: &[ForkedDatatableInfo], ) -> Result<()> { - windmill_common::workspaces::lock_fork_datatables(tx, parent_w_id).await?; let settings: Option = sqlx::query_scalar!( "SELECT datatable FROM workspace_settings WHERE workspace_id = $1", forked_w_id @@ -8607,6 +8606,9 @@ async fn create_workspace_fork( } let mut tx: Transaction<'_, Postgres> = db.begin().await?; + // Before the settings clone reads the parent's data tables: a pointer this fork ends up with + // must not be written after cleanup of the parent decided that nothing points at its copies. + windmill_common::workspaces::lock_fork_datatables(&mut tx, &parent_workspace_id).await?; if nw.is_dev_workspace { // The checks above ran outside a transaction, so the parent's eligibility and the chain's diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 480f815ad8..0d64eb4ac1 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -110,6 +110,21 @@ pub(crate) async fn change_workspace_id( .execute(&mut *tx) .await?; + // A fork copy reserved for the old id would otherwise be unreachable: its creator cannot + // import into it or finish its fork under the new id, and nothing else would ever drop it. + sqlx::query( + r#"UPDATE global_settings SET value = jsonb_set(value, '{databases}', ( + SELECT COALESCE(jsonb_object_agg(k, CASE WHEN v->>'workspace_id' = $1 + THEN jsonb_set(v, '{workspace_id}', to_jsonb($2::text)) ELSE v END), '{}'::jsonb) + FROM jsonb_each(COALESCE(value->'databases', '{}'::jsonb)) AS e(k, v) + )) + WHERE name = 'custom_instance_pg_databases'"#, + ) + .bind(&old_id) + .bind(&rw.new_id) + .execute(&mut *tx) + .await?; + // Duplicate workspace settings (keep copy in old workspace for reference) info!("Duplicating workspace_settings table"); sqlx::query!( From 8c554ef1337aa6cd99ab869443dfd30becc65a36 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Fri, 18 Sep 2026 01:00:06 +0200 Subject: [PATCH 5/5] fix(datatables): take the fork data table lock once, before the external cluster's Co-Authored-By: Claude Opus 5 (1M context) --- backend/windmill-api-workspaces/src/workspaces.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index fcc0d685ca..4c3d695c8f 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -8755,6 +8755,8 @@ async fn create_workspace_fork( let mut tx: Transaction<'_, Postgres> = db.begin().await?; // Before the settings clone reads the parent's data tables: a pointer this fork ends up with // must not be written after cleanup of the parent decided that nothing points at its copies. + // Also before the external cluster's lifecycle lock, which finalizing an external copy takes: + // fork cleanup takes the two in this order. windmill_common::workspaces::lock_fork_datatables(&mut tx, &parent_workspace_id).await?; if nw.is_dev_workspace { @@ -8868,9 +8870,6 @@ async fn create_workspace_fork( // re-enables in the fork, with parent-conflict warnings on enable. clone_triggers_and_schedules(&mut tx, &parent_workspace_id, &forked_id).await?; - // Before the external cluster's lifecycle lock, which finalizing an external copy takes: fork - // cleanup takes the two in this order. - windmill_common::workspaces::lock_fork_datatables(&mut tx, &parent_workspace_id).await?; // Update forked datatable settings to point to new databases for fdt in &nw.forked_datatables { apply_forked_datatable(&db, &mut tx, &authed, &parent_workspace_id, &forked_id, fdt)