fix: gate a clone's schema on reaching the source, and answer a finished clone asked again

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

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)

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?);

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<DB>,
@@ -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,

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.

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::<Vec<_>>()
             .join(", ");
         Ok(format!(

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;