diff --git a/backend/migrations/20260914133544_datatable_clone.up.sql b/backend/migrations/20260914133544_datatable_clone.up.sql index 56cfcc7eb6..11273ed5d7 100644 --- a/backend/migrations/20260914133544_datatable_clone.up.sql +++ b/backend/migrations/20260914133544_datatable_clone.up.sql @@ -10,6 +10,9 @@ CREATE TABLE datatable_clone ( source_workspace_id VARCHAR(50) NOT NULL, source_datatable VARCHAR(255) NOT NULL, created_by VARCHAR(255) NOT NULL, + -- `schema_only` or `schema_and_data`. NULL for a database created empty, to be filled by a + -- separate import. + fork_behavior VARCHAR(20), created_at TIMESTAMPTZ NOT NULL DEFAULT now(), -- The fork that took it. NULL while nothing has. claimed_by_workspace_id VARCHAR(50) diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index bc176ae264..1b73c95d6a 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -640,7 +640,10 @@ async fn a_data_table_under_roles_is_cloned_only_with_its_grants( let test_db: String = sqlx::query_scalar("SELECT current_database()::text") .fetch_one(&db) .await?; - let target = format!("wm_fork_{}", test_db.trim_start_matches('_').to_lowercase()); + let target = format!( + "wm_fork_{}", + test_db[test_db.len().saturating_sub(24)..].to_lowercase() + ); // Rows are a workspace admin's to copy, as they were before roles. let resp = authed( @@ -656,6 +659,21 @@ async fn a_data_table_under_roles_is_cloned_only_with_its_grants( assert_eq!(resp.status(), 403, "{}", resp.text().await?); assert!(!database_exists(&db, &target).await?); + // Even the schema alone lists every table, which a member covered by no role cannot read in + // the parent. + let resp = authed( + client().post(format!("{parent}/clone_pg_database")), + "SECRET_TOKEN_3", + ) + .json( + &json!({"source": "datatable://main", "target_dbname": target, + "fork_behavior": "schema_only"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + assert!(!database_exists(&db, &target).await?); + // The copy an admin asks for goes ahead in an edition that replays grants, and is refused // before any database exists in one that does not. The fixture's database does not exist, so // the dump fails either way — and leaves nothing behind. @@ -672,7 +690,7 @@ async fn a_data_table_under_roles_is_cloned_only_with_its_grants( assert_ne!(resp.status(), 200); let body = resp.text().await?; #[cfg(all(feature = "private", feature = "enterprise"))] - assert!(!body.contains("Enterprise Edition"), "{body}"); + assert!(body.contains("pg_dump"), "{body}"); #[cfg(not(all(feature = "private", feature = "enterprise")))] assert!(body.contains("Enterprise Edition"), "{body}"); assert!(!database_exists(&db, &target).await?); diff --git a/backend/windmill-api-workspaces/src/datatable_clone.rs b/backend/windmill-api-workspaces/src/datatable_clone.rs index 00e96038be..ee58126954 100644 --- a/backend/windmill-api-workspaces/src/datatable_clone.rs +++ b/backend/windmill-api-workspaces/src/datatable_clone.rs @@ -35,6 +35,7 @@ use windmill_common::workspaces::{ use windmill_common::{PgDatabase, DB}; use crate::datatable_acl::connect_with_notices; +use crate::datatable_permissions::ensure_reaches_datatable; use crate::workspaces::{ create_database_on_server, ensure_datatable_is_clonable, pg_dump_database, pg_import_dump, record_datatable_clone, DumpFile, PgDumpOptions, @@ -51,10 +52,10 @@ pub struct ClonePgDatabaseRequest { /// Copy data table `source` of this workspace into a new database `target_dbname`. /// -/// Who may copy is what it was before data table roles: anyone for the schema, an admin of this -/// workspace for the rows. A copy of a data table under roles is safe to hand to a fork because -/// the fork takes it governed by the source's roles, and the replay gives those roles exactly the -/// privileges they hold on the source. +/// Who may copy is what it was before data table roles — anyone for the schema, an admin of this +/// workspace for the rows — narrowed under roles to whoever may connect as one of them. A copy of +/// a data table under roles is safe to hand to a fork because the fork takes it governed by the +/// source's roles, and the replay gives those roles exactly the privileges they hold on the source. pub(crate) async fn clone_pg_database( authed: ApiAuthed, Extension(db): Extension, @@ -100,15 +101,45 @@ pub(crate) async fn clone_pg_database( .to_string(), )); } + // Even a copy of the schema alone shows every table of the data table, which under roles only + // those who may connect as one of them can read — through the catalogs, whatever the role. + ensure_reaches_datatable(&db, &w_id, &name, &authed).await?; let governing = ensure_datatable_is_clonable(&db, &w_id, &name).await?; if governing.datatable.permissions.is_some() { crate::datatable_replay_oss::ensure_replay()?; } + let behavior = if schema_only { + "schema_only" + } else { + "schema_and_data" + }; + // A copy runs to completion even when the request carrying it times out, so asking again is how + // a caller learns it finished: the same copy, recorded and not yet taken, is answered as done + // rather than refused for a name already in use. + let finished = sqlx::query_scalar::<_, bool>( + "SELECT EXISTS (SELECT 1 FROM datatable_clone + WHERE dbname = $1 AND source_workspace_id = $2 AND source_datatable = $3 + AND created_by = $4 AND fork_behavior = $5 AND claimed_by_workspace_id IS NULL)", + ) + .bind(&req.target_dbname) + .bind(&w_id) + .bind(&name) + .bind(&authed.email) + .bind(behavior) + .fetch_one(&db) + .await?; + if finished { + return Ok(format!( + "Data table '{name}' is already cloned into '{}'", + req.target_dbname + )); + } + // Detached from the request: a client that goes away mid-copy must still leave either a // finished copy or no database at all, and dropping the handler's future would skip the // cleanup. - let clone = CloneJob { db, authed, w_id, name, target: req.target_dbname, schema_only }; + let clone = CloneJob { db, authed, w_id, name, target: req.target_dbname, behavior }; tokio::spawn(clone.run(governing)) .await .map_err(|e| Error::internal_err(format!("The clone stopped unexpectedly: {e}")))? @@ -120,7 +151,8 @@ struct CloneJob { w_id: String, name: String, target: String, - schema_only: bool, + /// `schema_only` or `schema_and_data`. + behavior: &'static str, } impl CloneJob { @@ -137,7 +169,7 @@ impl CloneJob { let dump = pg_dump_database( &source_pg, PgDumpOptions { - schema_only: self.schema_only, + schema_only: self.behavior == "schema_only", no_owner: true, no_acl: is_instance, ..Default::default() @@ -209,6 +241,7 @@ impl CloneJob { .await?; // Everything so far was decided before the locks. + ensure_reaches_datatable(&self.db, &self.w_id, &self.name, &self.authed).await?; let now = ensure_datatable_is_clonable(&self.db, &self.w_id, &self.name).await?; let same_database = match (&now.datatable.database, &governing.datatable.database) { (Some(now), Some(then)) => { @@ -236,13 +269,16 @@ impl CloneJob { replay = Some((source, target, notices, catalog_roles)); } - record_datatable_clone(&mut *tx, &self.target, &self.w_id, &self.name, &self.authed) - .await?; - let behavior = if self.schema_only { - "schema_only" - } else { - "schema_and_data" - }; + let behavior = self.behavior; + record_datatable_clone( + &mut *tx, + &self.target, + &self.w_id, + &self.name, + &self.authed, + Some(behavior), + ) + .await?; audit_log( &mut *tx, &self.authed, diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index f07cf9ba55..34920d1b6d 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3302,6 +3302,7 @@ async fn create_pg_database( &w_id, &name, &authed, + None, ) .await?; } @@ -3367,18 +3368,22 @@ pub(crate) async fn record_datatable_clone( w_id: &str, datatable: &str, authed: &ApiAuthed, + fork_behavior: Option<&str>, ) -> Result<()> { sqlx::query( - "INSERT INTO datatable_clone (dbname, source_workspace_id, source_datatable, created_by) - VALUES ($1, $2, $3, $4) + "INSERT INTO datatable_clone + (dbname, source_workspace_id, source_datatable, created_by, fork_behavior) + VALUES ($1, $2, $3, $4, $5) ON CONFLICT (dbname) DO UPDATE SET source_workspace_id = EXCLUDED.source_workspace_id, source_datatable = EXCLUDED.source_datatable, created_by = EXCLUDED.created_by, - created_at = now(), claimed_by_workspace_id = NULL", + fork_behavior = EXCLUDED.fork_behavior, created_at = now(), + claimed_by_workspace_id = NULL", ) .bind(dbname) .bind(w_id) .bind(datatable) .bind(&authed.email) + .bind(fork_behavior) .execute(conn) .await?; Ok(()) @@ -7898,8 +7903,8 @@ async fn snapshot_datatable_schema( /// a fork admin could then edit to widen their own access to it. A pointer has nothing local to /// edit: the parent's entry stays the only place the decision lives. /// -/// The cloned data tables are skipped: they own a fresh database of their own, and they keep the -/// copied `permissions` as their starting point, which they then govern. +/// The cloned data tables are skipped: they own a fresh database of their own, and +/// `apply_forked_datatable` has already replaced their copied `permissions` with `governed_by`. async fn point_kept_datatables_at_parent( tx: &mut Transaction<'_, Postgres>, parent_w_id: &str, @@ -8043,6 +8048,10 @@ async fn apply_forked_datatable( authed, ) .await?; + // The fork keeps a snapshot of the source's schema, which under roles is only for those who + // may connect as one of them — as the copy itself was. + crate::datatable_permissions::ensure_reaches_datatable(db, parent_w_id, &fdt.name, authed) + .await?; // The copy holds the rows of what governs the source, so that entry keeps deciding who reaches // them. Settled from the source as it resolves now: the settings clone may have handed the fork // a pointer, or a clone of its own. diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 1d023566e4..04ab1bdb77 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -1010,17 +1010,19 @@ pub(crate) async fn delete_workspace( // 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!" + // workspace's, or be a clone taking its roles from one, and deleting the workspace it names + // leaves it 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_as::<_, (String, String)>( + r#"SELECT ws.workspace_id, dt.key FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt WHERE dt.value->'reference'->>'workspace_id' = $1 + OR dt.value->'governed_by'->>'workspace_id' = $1 ORDER BY ws.workspace_id, dt.key"#, - &w_id, ) + .bind(&w_id) .fetch_all(&db) .await .unwrap_or_default(); @@ -1348,7 +1350,7 @@ pub(crate) async fn delete_workspace( } else { let stranded = stranded_pointers .iter() - .map(|r| format!("{}/{}", r.workspace_id, r.datatable)) + .map(|(workspace_id, datatable)| format!("{workspace_id}/{datatable}")) .collect::>() .join(", "); Ok(format!( diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 4f69ad84a4..9b38d1054a 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1521,10 +1521,18 @@ pub async fn resolve_governing_datatable( // 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." - )) + if clone.is_some() { + Error::NotFound(format!( + "Data table '{name}' of workspace '{workspace_id}', which this clone \ + takes its roles from, no longer exists, so nobody is let into the copy." + )) + } else { + 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;