From 2718d6846c07ca343cfac4502771edd4195411e8 Mon Sep 17 00:00:00 2001 From: Diego Imbert <70353967+diegoimbert@users.noreply.github.com> Date: Mon, 21 Sep 2026 09:06:42 +0200 Subject: [PATCH 01/13] feat(datatables): clone a data table under roles with its owners and grants (#11120) Claude-Session: https://claude.ai/code/session_01UbrtwiYNfayrmqouBJHwGV Co-authored-by: Claude Opus 5 (1M context) --- ...522398d2a3ae99d0568b785e63749d6f3225a.json | 15 + ...df6bc09f19cf53b141fac5aefa981463d39fa.json | 29 + ...052b42aaf5c87e5be6333b2df67a81990c8a6.json | 16 + backend/ee-repo-ref.txt | 2 +- .../tests/datatable_roles.rs | 229 ++++++- .../src/datatable_acl.rs | 96 ++- .../src/datatable_clone.rs | 560 ++++++++++++++++ .../src/datatable_replay_oss.rs | 48 ++ backend/windmill-api-workspaces/src/lib.rs | 4 + .../windmill-api-workspaces/src/workspaces.rs | 619 ++++++++++++++---- .../src/workspaces_extra.rs | 30 +- backend/windmill-api/openapi.yaml | 35 +- backend/windmill-common/src/lib.rs | 136 +++- backend/windmill-common/src/workspaces.rs | 142 +++- cli/src/commands/workspace/fork.ts | 24 +- .../lib/components/InstanceSettings.svelte | 14 +- .../datatableAcl/PgAclEditor.svelte | 7 +- .../CreateWorkspaceInner.svelte | 22 +- .../DataTablePermissionsButton.svelte | 6 + .../ForkDatatableSection.svelte | 152 +---- 20 files changed, 1815 insertions(+), 371 deletions(-) create mode 100644 backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json create mode 100644 backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json create mode 100644 backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json create mode 100644 backend/windmill-api-workspaces/src/datatable_clone.rs create mode 100644 backend/windmill-api-workspaces/src/datatable_replay_oss.rs diff --git a/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json b/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json new file mode 100644 index 0000000000..2ff2722f77 --- /dev/null +++ b/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json @@ -0,0 +1,15 @@ +{ + "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 (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $2\n THEN jsonb_set(r.v, '{governed_by,workspace_id}', to_jsonb($1::text))\n ELSE r.v END\n FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $2\n THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text))\n ELSE dt.value END AS v) r)\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'\n AND (ws.datatable::text LIKE '%\"reference\"%'\n OR ws.datatable::text LIKE '%\"governed_by\"%')", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a" +} diff --git a/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json b/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json new file mode 100644 index 0000000000..af8d0eb1ab --- /dev/null +++ b/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n FROM workspace_settings ws\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE EXISTS (\n SELECT 1 FROM (VALUES ('reference'), ('governed_by')) k(link)\n WHERE dt.value->k.link->>'workspace_id' = $1\n AND dt.value->k.link->>'datatable' = $2\n )", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "workspace_id!", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "datatable!", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false, + null + ] + }, + "hash": "e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa" +} diff --git a/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json b/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json new file mode 100644 index 0000000000..1be51feb85 --- /dev/null +++ b/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.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 (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $1\n AND r.v->'governed_by'->>'datatable' = $2\n THEN jsonb_set(r.v, '{governed_by,datatable}', to_jsonb($3::text))\n ELSE r.v END\n FROM (SELECT 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 AS v) r)\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 LATERAL (VALUES ('reference'), ('governed_by')) k(link)\n WHERE d.value->k.link->>'workspace_id' = $1\n AND d.value->k.link->>'datatable' = $2\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6" +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index d5f3b9c086..d039bd8b4a 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -875dbc26f75743115a244fe22085290fc14635ff +3145e422d61d580f0a82804f075285c112879da0 diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index d2f6ca325e..d1b4501735 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -627,21 +627,234 @@ async fn a_rename_has_to_match_the_save_it_claims_to_describe( Ok(()) } +async fn database_exists(db: &Pool, name: &str) -> anyhow::Result { + Ok( + sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM pg_database WHERE datname = $1)") + .bind(name) + .fetch_one(db) + .await?, + ) +} + +async fn workspace_exists(db: &Pool, id: &str) -> anyhow::Result { + Ok( + sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM workspace WHERE id = $1)") + .bind(id) + .fetch_one(db) + .await?, + ) +} + #[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] -async fn a_data_table_under_roles_is_not_copied_into_a_fork( +async fn a_data_table_under_roles_is_cloned_only_with_its_grants( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + // The database a fork's copy of `main` goes into is named after the fork. Every request below + // is refused before it is created, so the cluster-wide name never collides with a sibling run. + let target = "wm_fork_copy__main"; + let fork = |token: &str, forked: Value| { + authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/create_fork" + )), + token, + ) + .json(&json!({"id": "wm-fork-copy", "name": "copy", "forked_datatables": [forked]})) + }; + + // Rows are a workspace admin's to copy, as they were before roles. + let resp = fork( + "SECRET_TOKEN_2", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_and_data"}), + ) + .send() + .await?; + 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 = fork( + "SECRET_TOKEN_3", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + assert!(!database_exists(&db, target).await?); + + // Refused in the first phase too, before a git branch is created for a fork that cannot be. + let resp = authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/create_workspace_fork_branch" + )), + "SECRET_TOKEN_3", + ) + .json( + &json!({"id": "wm-fork-copy", "name": "copy", "forked_datatables": [ + {"name": "main", "new_dbname": target, "fork_behavior": "schema_only"} + ]}), + ) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + + // A database the request did not create — whatever the data table, under roles or not — may be + // a copy left behind by a deleted fork, which the entry would reach without its governance. + let resp = fork( + "SECRET_TOKEN", + json!({"name": "other", "new_dbname": "wm_fork_copy__other"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 400); + assert!( + resp.text().await?.contains("fork_behavior"), + "a database the fork did not create was taken" + ); + assert!(!workspace_exists(&db, "wm-fork-copy").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 neither a database nor a fork behind. + let resp = fork( + "SECRET_TOKEN", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}), + ) + .send() + .await?; + assert_ne!(resp.status(), 200); + let body = resp.text().await?; + #[cfg(all(feature = "private", feature = "enterprise"))] + 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?); + assert!(!workspace_exists(&db, "wm-fork-copy").await?); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_clone_takes_its_roles_from_the_data_table_it_was_cloned_from( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + sqlx::query( + r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, '{datatables,copy}', '{ + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"}, + "governed_by": {"workspace_id": "test-workspace", "datatable": "main"}, + "forked_from": {} + }'::jsonb) WHERE workspace_id = 'wm-fork-dt'"#, + ) + .execute(&db) + .await?; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let fork = format!("http://localhost:{port}/api/w/wm-fork-dt/workspaces"); + + // `test-user-2` administers the fork, and is only a tenant of `analytics` in the parent. + let resp = authed( + client().get(format!("{fork}/datatable_usable_roles/copy")), + "SECRET_TOKEN_2", + ) + .send() + .await?; + let body: Value = resp.json().await?; + assert_eq!(body["roles"], json!(["analytics"]), "{body}"); + + let resp = authed( + client().get(format!("{fork}/datatable_permissions/copy")), + "SECRET_TOKEN_2", + ) + .send() + .await?; + let body: Value = resp.json().await?; + assert_eq!(body["editable"], false, "{body}"); + assert_eq!(body["clone_of"]["workspace_id"], "test-workspace", "{body}"); + + // Nobody changes a clone's roles, a superadmin included: they are the source's. + for token in ["SECRET_TOKEN_2", "SECRET_TOKEN"] { + let resp = authed( + client().post(format!("{fork}/datatable_permissions/copy")), + token, + ) + .json(&json!({"permissioned": false})) + .send() + .await?; + assert_eq!(resp.status(), 400, "{token} changed a clone's roles"); + } + + // A settings save cannot clear the link. + let resp = authed( + client().post(format!("{fork}/edit_datatable_config")), + "SECRET_TOKEN_2", + ) + .json(&json!({ + "settings": {"datatables": {"main": {}, "copy": { + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"} + }}}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + let governed_by: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->'copy'->'governed_by' FROM workspace_settings + WHERE workspace_id = 'wm-fork-dt'", + ) + .fetch_one(&db) + .await?; + assert_eq!(governed_by.unwrap()["datatable"], "main"); + + // Nor move it, a superadmin included: its grants were replayed into that database alone. + let resp = authed( + client().post(format!("{fork}/edit_datatable_config")), + "SECRET_TOKEN", + ) + .json(&json!({ + "settings": {"datatables": {"main": {}, "copy": { + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__elsewhere"} + }}}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 400, "{}", resp.text().await?); + + // Nor reach its database through a second entry without roles, which would connect as `admin`. + let resp = authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/edit_datatable_config" + )), + "SECRET_TOKEN", + ) + .json(&json!({ + "settings": {"datatables": { + "main": {"database": {"resource_type": "instance", "resource_path": "dt_main"}}, + "alias": {"database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"}} + }}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!(status, 400, "{body}"); + assert!(body.contains("without carrying those roles"), "{body}"); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_data_table_under_roles_is_not_copied_without_its_grants( db: Pool, ) -> anyhow::Result<()> { initialize_tracing().await; - // `pg_dump` carries no roles and the restore drops ACLs, so a copy would arrive with the - // parent's tenants and none of the grants behind them: every role but admin denied by - // Postgres in a data table that reads as configured. Refuse the copy rather than ship that, - // and refuse it before any data moves. let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); - // Both halves of the clone: the database the copy would land in, then the copy itself. The - // first has to refuse too, or a permissioned fork leaves an empty registered database that - // no data table entry names and nothing collects. let resp = authed( client().post(format!( "http://localhost:{port}/api/w/test-workspace/workspaces/create_pg_database" diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 6bd14316fc..04dc3d4e80 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -245,6 +245,8 @@ pub struct DatatableAclInfo { /// Whether this caller may plan and apply changes: they administer the data table, on an /// edition that has the planner. pub editable: bool, + /// Whether the data table is a clone, whose grants stay as they were copied from its source. + pub clone: bool, /// Whether the server is Postgres 17 or later, which added the `MAINTAIN` table privilege. pub supports_maintain: bool, /// The database the target lives in, which no target carries itself. @@ -335,13 +337,40 @@ async fn connect_as_admin_unchecked( pg.user = Some(CUSTOM_INSTANCE_USER.to_string()); pg.password = Some(windmill_common::utils::get_custom_pg_instance_password(db).await?); let dbname = pg.dbname.clone(); - let (client, mut connection) = pg.connect(Some(db)).await?; + let (client, notices) = connect_with_notices(db, &pg).await?; + Ok((client, notices, dbname)) +} + +/// A connection to `pg`, and the notices Postgres sends on it — which is where a grant or revoke +/// that changed nothing is reported ([`execute_acl_statements`]). +pub(crate) async fn connect_with_notices( + db: &DB, + pg: &PgDatabase, +) -> Result<(tokio_postgres::Client, mpsc::UnboundedReceiver)> { + let (client, connection) = pg.connect(Some(db)).await?; + Ok(( + client, + drive_with_notices(connection, windmill_common::TokioPgConnection::poll_message), + )) +} + +type PollMessage = + fn( + &mut C, + &mut std::task::Context<'_>, + ) -> std::task::Poll>>; + +/// Drive `connection` in the background with `poll`, forwarding its notices. +pub(crate) fn drive_with_notices( + mut connection: C, + poll: PollMessage, +) -> mpsc::UnboundedReceiver { // Unbounded: the driver must never wait on the receiver, which only drains once the statement // the driver is carrying has completed. let (notices_tx, notices) = mpsc::unbounded_channel(); tokio::spawn(async move { loop { - match std::future::poll_fn(|cx| connection.poll_message(cx)).await { + match std::future::poll_fn(|cx| poll(&mut connection, cx)).await { Some(Ok(AsyncMessage::Notice(notice))) => { let _ = notices_tx.send(notice); } @@ -354,7 +383,41 @@ async fn connect_as_admin_unchecked( } } }); - Ok((client, notices, dbname)) + notices +} + +/// Run `statements` in order on `tx`, failing on the first that errors or that Postgres only warns +/// about. A privilege the connection cannot pass on is a warning to Postgres (`01007` / `01006`), +/// which then carries on having changed nothing; returning drops the transaction, rolling back +/// everything before it. +/// +/// Authorization: none. Runs `statements` as the connection `tx` is on; callers MUST have authorized +/// changing that database's access, and built the statements themselves. +pub(crate) async fn execute_acl_statements( + tx: &tokio_postgres::Transaction<'_>, + notices: &mut mpsc::UnboundedReceiver, + statements: &[String], +) -> Result<()> { + while notices.try_recv().is_ok() {} + for statement in statements { + tx.batch_execute(statement).await.map_err(|e| { + Error::ExecutionErr(format!( + "Failed to run `{statement}`: {}", + pg_error_message(&e) + )) + })?; + while let Ok(notice) = notices.try_recv() { + if *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_GRANTED + || *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_REVOKED + { + return Err(Error::ExecutionErr(format!( + "`{statement}` did not take effect ({}), so nothing was applied", + notice.message() + ))); + } + } + } + Ok(()) } /// An object whose ownership follows the schema's. @@ -399,10 +462,12 @@ macro_rules! schema_owned_objects { AND x.refobjsubid <> 0)))" }; } +#[allow(unused_imports)] +pub(crate) use schema_owned_objects; /// The keyword `ALTER ... OWNER TO` takes for a kind of object, as `pg_identify_object` names the /// kind. A kind missing here is refused rather than skipped, which would leave it behind. -fn owned_keyword(kind: &str) -> Option<&'static str> { +pub(crate) fn owned_keyword(kind: &str) -> Option<&'static str> { Some(match kind { "table" => "TABLE", "view" => "VIEW", @@ -1054,6 +1119,7 @@ async fn get_datatable_acl( owner: role_name_of(&owner), roles, editable, + clone: governing.governor.is_some(), supports_maintain, dbname, grants, @@ -1625,27 +1691,7 @@ async fn apply_datatable_acl( pg_error_message(&e) )) })?; - for statement in &plan.statements { - pg_tx.batch_execute(statement).await.map_err(|e| { - Error::ExecutionErr(format!( - "Failed to run `{statement}`: {}", - pg_error_message(&e) - )) - })?; - // A privilege the connection cannot pass on is only a warning to Postgres, which then - // carries on having changed nothing. Returning drops the transaction, rolling back - // everything before it. - while let Ok(notice) = notices.try_recv() { - if *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_GRANTED - || *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_REVOKED - { - return Err(Error::ExecutionErr(format!( - "`{statement}` did not take effect ({}), so nothing was applied", - notice.message() - ))); - } - } - } + execute_acl_statements(&pg_tx, &mut notices, &plan.statements).await?; // A schema's objects were listed before the transaction opened; one committed since would stay // with its old owner. One created while this transaction is still open can still slip past, as diff --git a/backend/windmill-api-workspaces/src/datatable_clone.rs b/backend/windmill-api-workspaces/src/datatable_clone.rs new file mode 100644 index 0000000000..be728782af --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_clone.rs @@ -0,0 +1,560 @@ +/* + * Author: Ruben Fiszel + * Copyright: Windmill Labs, Inc 2022 + * This file and its contents are licensed under the AGPLv3 License. + * Please see the included NOTICE for copyright information and + * LICENSE-AGPL for a copy of the license. + */ + +//! Copying a data table's database for the fork being created. +//! +//! The fork request makes its copies before it writes the fork: each is created, filled and — for a +//! data table under roles — given the source's owners and grants. What each copy was made from +//! stays in the request ([`MadeCopy`]), so the fork's transaction checks it against the source as it +//! is then, and a fork that fails drops the instance copies it made ([`drop_copies_after`]) and names +//! the others, which live on servers of the workspace's own. + +use std::collections::BTreeSet; + +use windmill_api_auth::ApiAuthed; +use windmill_common::datatable_roles::{lock_role_catalog, read_role_catalog_tx}; +use windmill_common::error::{pg_error_message, Error, Result}; +use windmill_common::utils::require_admin; +use windmill_common::worker::CLOUD_HOSTED; +use windmill_common::workspaces::{ + get_datatable_resource_from_db_unchecked, DataTableDatabase, DataTableForkBehavior, + GoverningDatatable, +}; +use windmill_common::{PgDatabase, DB}; + +use crate::datatable_acl::connect_with_notices; +use crate::datatable_permissions::ensure_reaches_governing_datatable; +use crate::workspaces::{ + create_database_on_server, ensure_datatable_is_clonable, pg_dump_database, pg_import_dump, + DumpFile, PgDumpOptions, +}; + +/// A database this request created and filled for one data table of the fork. +pub(crate) struct MadeCopy { + /// The data table's name, in the parent and in the fork. + pub(crate) name: String, + pub(crate) dbname: String, + pub(crate) behavior: DataTableForkBehavior, + /// The database the source resolved to when it was copied. + pub(crate) source_database: DataTableDatabase, + /// Whether the source's owners and grants were replayed into the copy: it was under roles. + pub(crate) replayed: bool, + /// For a resource-backed copy, the resource and variables its connection was resolved from. + pub(crate) connection: Option, +} + +/// What a resource-backed data table's connection is resolved from: its resource, and every +/// resource and variable that one references, as stored — and, for a secret kept in an external +/// backend, the value that backend holds. The connection is resolved from the snapshot itself +/// ([`ConnectionSnapshot::resolve`]), so it is exactly the one these rows describe. +#[derive(PartialEq)] +pub(crate) struct ConnectionSnapshot { + root: String, + resources: std::collections::BTreeMap>, + /// Stored value and whether it is secret. + variables: std::collections::BTreeMap>, + external_secrets: std::collections::BTreeMap, +} + +/// Read the [`ConnectionSnapshot`] of resource `resource_path` in workspace `w_id`. +/// +/// Authorization: none, and it holds secret values. Callers MUST have authorized using that +/// resource, and never disclose the snapshot or what it resolves to. +pub(crate) async fn connection_snapshot( + db: &DB, + conn: &mut sqlx::PgConnection, + w_id: &str, + resource_path: &str, +) -> Result { + let root = resource_path.trim_start_matches("$res:").to_string(); + let mut snapshot = ConnectionSnapshot { + root: root.clone(), + resources: Default::default(), + variables: Default::default(), + external_secrets: Default::default(), + }; + let mut pending = vec![root]; + while let Some(path) = pending.pop() { + if snapshot.resources.contains_key(&path) { + continue; + } + let value: Option = sqlx::query_scalar( + "SELECT value FROM resource WHERE workspace_id = $1 AND path = $2", + ) + .bind(w_id) + .bind(&path) + .fetch_optional(&mut *conn) + .await? + .flatten(); + let mut strings = vec![]; + collect_strings(value.as_ref(), &mut strings); + for reference in strings { + if let Some(var) = reference.strip_prefix("$var:") { + if snapshot.variables.contains_key(var) { + continue; + } + let row: Option<(String, bool)> = sqlx::query_as( + "SELECT value, is_secret FROM variable WHERE workspace_id = $1 AND path = $2", + ) + .bind(w_id) + .bind(var) + .fetch_optional(&mut *conn) + .await?; + if let Some((stored, true)) = &row { + if windmill_common::secret_backend::is_external_stored_value(stored) { + let secret = windmill_common::secret_backend::get_secret_value( + db, w_id, var, stored, + ) + .await?; + snapshot.external_secrets.insert(var.to_string(), secret); + } + } + snapshot.variables.insert(var.to_string(), row); + } else if let Some(res) = reference.strip_prefix("$res:") { + pending.push(res.to_string()); + } + } + snapshot.resources.insert(path, value); + } + Ok(snapshot) +} + +impl ConnectionSnapshot { + /// The connection these rows resolve to, as the data table's own resolution would: references + /// substituted, secrets decrypted. + pub(crate) async fn resolve(&self, db: &DB, w_id: &str) -> Result { + let root = self.resource(&self.root)?; + self.substitute(db, w_id, root, 0).await + } + + /// Whether the resource itself holds the connection's fields, which the fork repoints by setting + /// its `dbname`, rather than being a reference to another resource. + pub(crate) fn root_holds_connection(&self) -> bool { + self.resource(&self.root).is_ok_and(|v| v.is_object()) + } + + fn resource(&self, path: &str) -> Result<&serde_json::Value> { + self.resources + .get(path) + .and_then(|v| v.as_ref()) + .ok_or_else(|| Error::NotFound(format!("resource {path} does not exist"))) + } + + fn substitute<'a>( + &'a self, + db: &'a DB, + w_id: &'a str, + value: &'a serde_json::Value, + depth: usize, + ) -> std::pin::Pin> + Send + 'a>> + { + Box::pin(async move { + if depth > 32 { + return Err(Error::BadRequest("resource references nest too deeply".to_string())); + } + Ok(match value { + serde_json::Value::Object(map) => { + let mut out = serde_json::Map::new(); + for (key, val) in map { + out.insert(key.clone(), self.substitute(db, w_id, val, depth).await?); + } + serde_json::Value::Object(out) + } + serde_json::Value::Array(items) => { + let mut out = vec![]; + for val in items { + out.push(self.substitute(db, w_id, val, depth).await?); + } + serde_json::Value::Array(out) + } + serde_json::Value::String(s) if s.starts_with("$res:") => { + let path = &s[5..]; + self.substitute(db, w_id, self.resource(path)?, depth + 1).await? + } + serde_json::Value::String(s) if s.starts_with("$var:") => { + let path = &s[5..]; + let (stored, secret) = self + .variables + .get(path) + .and_then(|v| v.as_ref()) + .ok_or_else(|| Error::NotFound(format!("variable {path} does not exist")))?; + serde_json::Value::String(match (secret, self.external_secrets.get(path)) { + (false, _) => stored.clone(), + (true, Some(external)) => external.clone(), + (true, None) => windmill_common::variables::decrypt( + &windmill_common::variables::build_crypt(db, w_id).await?, + stored.clone(), + ) + .map_err(|e| { + Error::internal_err(format!("Error decrypting variable {s}: {e}")) + })?, + }) + } + other => other.clone(), + }) + }) + } +} + +fn collect_strings<'a>(value: Option<&'a serde_json::Value>, out: &mut Vec<&'a str>) { + match value { + Some(serde_json::Value::String(s)) => out.push(s), + Some(serde_json::Value::Array(items)) => { + items.iter().for_each(|v| collect_strings(Some(v), out)) + } + Some(serde_json::Value::Object(map)) => { + map.values().for_each(|v| collect_strings(Some(v), out)) + } + _ => {} + } +} + +/// What a data table of the fork should be copied as. +pub(crate) struct CopyRequest<'a> { + pub(crate) name: &'a str, + pub(crate) dbname: &'a str, + pub(crate) behavior: DataTableForkBehavior, +} + +/// Check that `authed` may copy each data table of `parent_w_id` as asked, before anything is +/// created. +/// +/// Who may copy is what it was before data table roles — anyone for the schema, an admin of the +/// workspace for the rows — narrowed under roles to whoever may connect as one of them: even the +/// schema alone lists every table, which under roles only role holders can read in the parent. +pub(crate) async fn authorize_copies( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + requests: &[CopyRequest<'_>], +) -> Result<()> { + for request in requests { + match request.behavior { + DataTableForkBehavior::KeepOriginal => { + return Err(Error::BadRequest(format!( + "Data table '{}' is kept, which copies nothing", + request.name + ))) + } + DataTableForkBehavior::SchemaOnly => {} + DataTableForkBehavior::SchemaAndData => { + require_admin(authed.is_admin, &authed.username)?; + if *CLOUD_HOSTED { + return Err(Error::BadRequest( + "Cloning schema and data is not available on cloud".to_string(), + )); + } + } + } + windmill_common::validate_dbname(request.dbname)?; + if !request.dbname.starts_with("wm_fork_") { + return Err(Error::BadRequest(format!( + "Forked datatable database name '{}' must start with 'wm_fork_'", + request.dbname + ))); + } + let governing = ensure_datatable_is_clonable(db, parent_w_id, request.name).await?; + ensure_reaches_governing_datatable(db, parent_w_id, request.name, &governing, authed) + .await?; + if governing.datatable.permissions.is_some() { + crate::datatable_replay_oss::ensure_replay()?; + } + } + Ok(()) +} + +/// Make every copy in `requests`, in order, once [`authorize_copies`] allows them all. When one +/// fails, the copies already made are dropped before the error is returned. +pub(crate) async fn make_copies( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + requests: &[CopyRequest<'_>], +) -> Result> { + authorize_copies(db, authed, parent_w_id, requests).await?; + let mut copies = Vec::with_capacity(requests.len()); + for request in requests { + match make_copy(db, authed, parent_w_id, request).await { + Ok(copy) => copies.push(copy), + Err(e) => return Err(drop_copies_after(db, copies, e).await), + } + } + Ok(copies) +} + +/// Drop every copy, returning `error` — with what could not be dropped appended, since nothing +/// else will name those databases again. +pub(crate) async fn drop_copies_after(db: &DB, copies: Vec, error: Error) -> Error { + let mut stranded = Vec::new(); + for copy in copies { + if let Err(e) = drop_copy(db, ©.source_database, ©.dbname).await { + tracing::error!("Could not drop '{}' after a failed fork: {e}", copy.dbname); + stranded.push(format!("'{}' ({e})", copy.dbname)); + } + } + if stranded.is_empty() { + error + } else { + Error::ExecutionErr(format!( + "{error}. These databases created for the fork could not be dropped: {}", + stranded.join(", ") + )) + } +} + +async fn drop_copy(db: &DB, source_database: &DataTableDatabase, dbname: &str) -> Result<()> { + if source_database.resource_type + == windmill_common::workspaces::DataTableCatalogResourceType::Instance + { + let users = windmill_common::drop_unused_instance_database(db, dbname).await?; + if users.is_empty() { + Ok(()) + } else { + Err(Error::BadRequest(format!( + "kept, since workspaces {} now use it", + users.join(", ") + ))) + } + } else { + // On a server of the workspace's own, where a resource edited meanwhile can already name it + // and nothing locks such an edit: dropping it could take someone's data. + Err(Error::BadRequest( + "kept on its PostgreSQL server, where it may already be in use; drop it there once it \ + is not, before forking the same data table under this id again" + .to_string(), + )) + } +} + +async fn make_copy( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + request: &CopyRequest<'_>, +) -> Result { + let governing = ensure_datatable_is_clonable(db, parent_w_id, request.name).await?; + let source_database = governing.datatable.database.clone().ok_or_else(|| { + Error::internal_err(format!( + "Data table '{}' resolves to an entry that owns no database", + request.name + )) + })?; + let is_instance = governing.is_instance(); + // Resolved from the snapshot, not read again: the connection the copy is made on is then + // exactly what these rows describe, which the fork's own clone of them is checked against. + let (server, connection): (PgDatabase, Option) = if is_instance { + let server = serde_json::from_value( + get_datatable_resource_from_db_unchecked(db, parent_w_id, request.name).await?, + ) + .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {e}")))?; + (server, None) + } else { + let snapshot = connection_snapshot( + db, + &mut *db.acquire().await?, + &governing.workspace_id, + &source_database.resource_path, + ) + .await?; + if !snapshot.root_holds_connection() { + return Err(Error::BadRequest(format!( + "Data table '{}' uses resource '{}', which only refers to another resource: the \ + fork's copy of it could not be pointed at the copied database. Point the data \ + table at the resource holding the connection, then fork again.", + request.name, source_database.resource_path + ))); + } + let server = serde_json::from_value(snapshot.resolve(db, &governing.workspace_id).await?) + .map_err(|e| { + Error::internal_err(format!("Failed to parse database credentials: {e}")) + })?; + (server, Some(snapshot)) + }; + + // Dumped before the database exists, so a source that cannot be read leaves nothing behind. + // Ownership never carries over: the restore runs as the target's connection user. Grants do, + // except on the instance, where the replay below is what reproduces them. + let dump = pg_dump_database( + &server, + PgDumpOptions { + schema_only: request.behavior == DataTableForkBehavior::SchemaOnly, + no_owner: true, + no_acl: is_instance, + ..Default::default() + }, + ) + .await?; + + if is_instance { + windmill_common::create_custom_instance_database(db, request.dbname, "datatable").await?; + } else { + create_database_on_server(db, &server, request.dbname).await?; + } + + let target = PgDatabase { dbname: request.dbname.to_string(), ..server.clone() }; + let filled = fill( + db, + authed, + parent_w_id, + request.name, + &governing, + &server, + &target, + &dump, + ) + .await; + match filled { + Ok(replayed) => Ok(MadeCopy { + name: request.name.to_string(), + dbname: request.dbname.to_string(), + behavior: request.behavior, + source_database, + replayed, + connection, + }), + Err(e) => { + if let Err(drop_err) = drop_copy(db, &source_database, request.dbname).await { + tracing::error!( + "Could not drop '{}' after a failed copy of data table '{}': {drop_err}", + request.dbname, + request.name + ); + return Err(Error::ExecutionErr(format!( + "{e}. The database '{}' created for the copy could not be dropped: {drop_err}", + request.dbname + ))); + } + Err(e) + } + } +} + +/// Restore the dump into the new database and, for a data table under roles, replay the source's +/// owners and grants into it. Returns whether it replayed. +#[allow(clippy::too_many_arguments)] +async fn fill( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + name: &str, + governing: &GoverningDatatable, + source: &PgDatabase, + target: &PgDatabase, + dump: &DumpFile, +) -> Result { + pg_import_dump(target, dump).await?; + + // Held until the replay commits: a role renamed or dropped meanwhile would change what the + // replay names, and a settings save could move the source onto another database or put it under + // roles. Taken in the same order as the permissions save and the ACL apply. + // On a connection of its own: the checks below take theirs from the pool, and a lock holder + // drawn from the pool too could leave a small one with nothing to give them. + let database_url = windmill_common::get_database_url().await?; + let mut lock_holder = + ::connect_with(&database_url.connect_options().await?) + .await + .map_err(|e| Error::internal_err(format!("Failed to connect to the database: {e}")))?; + let mut tx = sqlx::Connection::begin(&mut lock_holder).await?; + lock_role_catalog(&mut tx).await?; + lock_settings_rows(&mut tx, parent_w_id, name).await?; + + // Everything so far was decided before the locks. + let now = ensure_datatable_is_clonable(db, parent_w_id, name).await?; + ensure_reaches_governing_datatable(db, parent_w_id, name, &now, authed).await?; + if !same_database(&now.datatable.database, &governing.datatable.database) { + return Err(Error::BadRequest(format!( + "Data table '{name}' moved to another database while it was being copied; fork again" + ))); + } + + let replayed = now.datatable.permissions.is_some(); + if replayed { + crate::datatable_replay_oss::ensure_replay()?; + let catalog = read_role_catalog_tx(&mut tx).await?; + // The replay leaves `CONNECT` to the catalog, and creating the database only tried to set + // it: a copy `PUBLIC` could still connect to would admit logins the source turns away. + windmill_common::datatable_roles::converge_connect_grants_with( + db, + &target.dbname, + &catalog, + ) + .await?; + let catalog_roles: BTreeSet = catalog.values().map(|r| r.name.clone()).collect(); + let (source, _source_notices) = connect_with_notices(db, source).await?; + let (mut target, mut notices) = connect_with_notices(db, target).await?; + // One transaction on the copy: a replay that stops halfway leaves objects owned by one role + // and granted as another. + let pg_tx = target.transaction().await.map_err(|e| { + Error::internal_err(format!( + "Failed to open a transaction on the copy: {}", + pg_error_message(&e) + )) + })?; + crate::datatable_replay_oss::replay_owners_and_grants( + &source, + &pg_tx, + &mut notices, + &catalog_roles, + ) + .await?; + pg_tx.commit().await.map_err(|e| { + Error::internal_err(format!( + "Failed to commit the replayed grants: {}", + pg_error_message(&e) + )) + })?; + } + tx.commit().await?; + Ok(replayed) +} + +/// Lock every settings row the resolution of data table `name` of `start_w_id` passes through, in +/// the order it passes them — each pointer, the entry that owns the database, and each workspace its +/// roles come from — and return them. Deleting or repointing any of them would leave a copy made +/// for the resolution answering for another one. +pub(crate) async fn lock_settings_rows( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + start_w_id: &str, + name: &str, +) -> Result> { + let mut path: Vec<(String, String)> = vec![]; + let (mut w_id, mut datatable) = (start_w_id.to_string(), name.to_string()); + loop { + if path.contains(&(w_id.clone(), datatable.clone())) || path.len() > 32 { + return Err(Error::BadRequest(format!( + "Data table '{name}' of workspace '{start_w_id}' resolves through a cycle" + ))); + } + let entry: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->$2 FROM workspace_settings + WHERE workspace_id = $1 FOR UPDATE", + ) + .bind(&w_id) + .bind(&datatable) + .fetch_optional(&mut **tx) + .await? + .flatten(); + path.push((w_id.clone(), datatable.clone())); + let entry: Option = + entry.and_then(|e| serde_json::from_value(e).ok()); + match entry.and_then(|e| e.reference.or(e.governed_by)) { + Some(next) => (w_id, datatable) = (next.workspace_id, next.datatable), + None => break, + } + } + Ok(path.into_iter().map(|(w_id, _)| w_id).collect()) +} + +pub(crate) fn same_database(a: &Option, b: &Option) -> bool { + match (a, b) { + (Some(a), Some(b)) => { + a.resource_type == b.resource_type && a.resource_path == b.resource_path + } + _ => false, + } +} diff --git a/backend/windmill-api-workspaces/src/datatable_replay_oss.rs b/backend/windmill-api-workspaces/src/datatable_replay_oss.rs new file mode 100644 index 0000000000..edcf8e402b --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_replay_oss.rs @@ -0,0 +1,48 @@ +/* + * Author: Ruben Fiszel + * Copyright: Windmill Labs, Inc 2022 + * This file and its contents are licensed under the AGPLv3 License. + * Please see the included NOTICE for copyright information and + * LICENSE-AGPL for a copy of the license. + */ + +//! Where the replay of a data table's owners and grants into its copy comes from: the enterprise +//! one, or a refusal. Without it a data table under roles is not copied at all: its rows would +//! arrive owned by the admin connection with no grant for any role. + +#[cfg(all(feature = "private", feature = "enterprise"))] +pub(crate) use crate::datatable_replay_ee::replay_owners_and_grants; + +#[cfg(all(feature = "private", feature = "enterprise"))] +pub(crate) fn ensure_replay() -> windmill_common::error::Result<()> { + Ok(()) +} + +#[cfg(not(all(feature = "private", feature = "enterprise")))] +use { + std::collections::BTreeSet, + tokio::sync::mpsc, + tokio_postgres::error::DbError, + windmill_common::error::{Error, Result}, +}; + +/// Checked before anything is created. +#[cfg(not(all(feature = "private", feature = "enterprise")))] +pub(crate) fn ensure_replay() -> Result<()> { + Err(Error::BadRequest( + "Cloning a data table under roles is a Windmill Enterprise Edition feature: the copy \ + needs the source's owners and grants replayed. Fork it keeping the original database \ + instead." + .to_string(), + )) +} + +#[cfg(not(all(feature = "private", feature = "enterprise")))] +pub(crate) async fn replay_owners_and_grants( + _source: &tokio_postgres::Client, + _target: &tokio_postgres::Transaction<'_>, + _notices: &mut mpsc::UnboundedReceiver, + _catalog_roles: &BTreeSet, +) -> Result<()> { + ensure_replay() +} diff --git a/backend/windmill-api-workspaces/src/lib.rs b/backend/windmill-api-workspaces/src/lib.rs index 5dfdece644..e7da2b8496 100644 --- a/backend/windmill-api-workspaces/src/lib.rs +++ b/backend/windmill-api-workspaces/src/lib.rs @@ -3,9 +3,11 @@ pub mod ai_session_backups; pub mod data_metrics; pub mod datatable_acl; pub mod datatable_acl_oss; +pub mod datatable_clone; pub mod datatable_migrations; pub mod datatable_permissions; pub mod datatable_permissions_oss; +pub mod datatable_replay_oss; pub mod deployment_requests; pub mod workspaces; pub mod workspaces_extra; @@ -16,6 +18,8 @@ pub mod workspaces_ee; #[cfg(all(feature = "private", feature = "enterprise"))] pub mod datatable_acl_ee; +#[cfg(all(feature = "private", feature = "enterprise"))] +pub mod datatable_replay_ee; #[cfg(all(feature = "private", feature = "enterprise"))] pub mod datatable_permissions_ee; diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 76105dbc24..4822c2f3b9 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -501,8 +501,8 @@ struct CreateWorkspaceFork { id: String, name: String, color: Option, - /// Datatable names that were forked. For each, the backend will update the - /// forked workspace's datatable config to point to the new database. + /// Data tables the fork gets a copy of rather than a pointer at the parent's. For each, the + /// fork's entry is pointed at the new database. #[serde(default)] forked_datatables: Vec, /// Lakes the user explicitly chose to SHARE with the parent (the fork then reads and @@ -534,6 +534,11 @@ struct CreateWorkspaceFork { struct ForkedDatatableInfo { name: String, new_dbname: String, + /// What this request copies into `new_dbname`, which it creates. Optional only so that the + /// entry older CLIs send — naming a database they created and filled themselves — is refused + /// with a message rather than a parse error. + #[serde(default)] + fork_behavior: Option, } #[derive(Deserialize)] @@ -2233,8 +2238,8 @@ async fn list_datatables( name, resource_type: database.resource_type.as_ref().to_string(), resource_path: database.resource_path.clone(), - governing_workspace_id: (governing.workspace_id != w_id) - .then(|| governing.workspace_id.clone()), + governing_workspace_id: (governing.governing_workspace_id() != w_id) + .then(|| governing.governing_workspace_id().to_string()), permissioned: governing.datatable.permissions.is_some(), }); } @@ -2916,6 +2921,18 @@ fn truncate_column_default(default: String) -> String { mod tests { use super::*; + #[test] + fn a_dev_workspace_copies_into_the_fork_database_namespace() { + assert_eq!( + forked_datatable_dbname("wm-fork-my-fork", "main"), + "wm_fork_my_fork__main" + ); + assert_eq!( + forked_datatable_dbname("my-dev", "main"), + "wm_fork_my_dev__main" + ); + } + /// The header of a pg_dump, followed by an object whose body also holds a `SET`. const DUMP: &str = "--\n\ -- PostgreSQL database dump\n\ @@ -3384,7 +3401,10 @@ async fn server_setting_names(pg_db: &PgDatabase) -> Result> { /// so a dump that breaks partway through imports partially and reads as a success. /// ON_ERROR_STOP surfaces the failure and --single-transaction makes the restore /// all-or-nothing, leaving the target as it was and the import retryable. -async fn pg_import_dump(target_db: &PgDatabase, dump_file: &DumpFile) -> Result<()> { +/// +/// Authorization: none. Writes into `target_db` with the credentials it carries; callers MUST have +/// authorized writing to that database. +pub(crate) async fn pg_import_dump(target_db: &PgDatabase, dump_file: &DumpFile) -> Result<()> { let supported_settings = server_setting_names(target_db).await?; comment_out_unsupported_settings(dump_file, &supported_settings).await?; @@ -3433,7 +3453,7 @@ async fn create_pg_database( // exists rather than leaving an empty registered `wm_fork_…` behind. if let Some(reference) = req.source.strip_prefix("datatable://") { let (name, _) = parse_datatable_ref_for(&db, &w_id, reference).await?; - ensure_datatable_is_clonable(&db, &w_id, &name).await?; + ensure_copied_without_roles(&ensure_datatable_is_clonable(&db, &w_id, &name).await?)?; } // Non-superadmin: restrict dbname to wm_fork_ prefix @@ -3452,50 +3472,62 @@ async fn create_pg_database( } else { let source_pg = resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.source).await?; - let (client, connection) = source_pg.connect(Some(&db)).await?; - let join_handle = tokio::spawn(async move { connection.await }); - - let row = client - .query_one( - "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", - &[&req.target_dbname], - ) - .await - .map_err(|e| { - Error::internal_err(format!( - "Failed to check database existence: {}", - pg_error_message(&e) - )) - })?; - let db_exists: bool = row.get(0); - - if db_exists { - drop(client); - let _ = windmill_common::shutdown_pg_connection(join_handle).await; - return Err(Error::BadRequest(format!( - "Database '{}' already exists on the resource server", - req.target_dbname - ))); - } - - client - .execute(&format!("CREATE DATABASE \"{}\"", &req.target_dbname), &[]) - .await - .map_err(|e| { - Error::internal_err(format!( - "Failed to create database '{}': {}", - req.target_dbname, - pg_error_message(&e) - )) - })?; - - drop(client); - windmill_common::shutdown_pg_connection(join_handle).await?; + create_database_on_server(&db, &source_pg, &req.target_dbname).await?; } Ok(format!("Created database '{}'", req.target_dbname)) } +/// `CREATE DATABASE` on the server `server` connects to, refusing a name already taken there. +/// +/// Authorization: none. Callers MUST have authorized creating a database on that server — resolved +/// from a data table or resource the caller may administer. +pub(crate) async fn create_database_on_server( + db: &DB, + server: &PgDatabase, + dbname: &str, +) -> Result<()> { + windmill_common::validate_dbname(dbname)?; + let (client, connection) = server.connect(Some(db)).await?; + let join_handle = tokio::spawn(async move { connection.await }); + + let row = client + .query_one( + "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", + &[&dbname], + ) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to check database existence: {}", + pg_error_message(&e) + )) + })?; + let db_exists: bool = row.get(0); + + if db_exists { + drop(client); + let _ = windmill_common::shutdown_pg_connection(join_handle).await; + return Err(Error::BadRequest(format!( + "Database '{dbname}' already exists on the resource server" + ))); + } + + client + .execute(&format!("CREATE DATABASE \"{dbname}\""), &[]) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to create database '{dbname}': {}", + pg_error_message(&e) + )) + })?; + + drop(client); + windmill_common::shutdown_pg_connection(join_handle).await?; + Ok(()) +} + #[derive(Deserialize)] struct ImportPgDatabaseRequest { source: String, @@ -3505,48 +3537,22 @@ struct ImportPgDatabaseRequest { fork_behavior: DataTableForkBehavior, } -/// Refuse to copy a data table that is under roles. +/// Whether data table `name` of `w_id` has a shape a copy can be made of, returning what governs +/// it. Checked before anything is created — by the fork request for the copies it makes, and by +/// `create_pg_database` — and again when the fork's entry is written. /// -/// `pg_dump` carries no roles and the import runs with `--no-privileges`, so a clone arrives with -/// its objects owned by the admin connection and no `GRANT` for any role. The settings copy brings -/// `permissions` across, so the fork's tenants pass Windmill's check, connect as the role they were -/// given, and are then denied by Postgres on everything — a data table that looks configured and -/// answers nothing. +/// It is not the check for a data table under roles: a copy of one is only correct with its +/// owners and grants replayed, which the fork request does and the older endpoints refuse +/// ([`ensure_copied_without_roles`]). /// -/// It fails closed rather than open, so this is a usability cliff rather than a hole, and the fix -/// is to replay the source's owners and ACLs into the clone. That is a change of its own; until it -/// exists, refusing is the honest answer. Dropping `permissions` from the clone instead would be -/// the unsafe half: the copy holds the parent's rows, so an unpermissioned clone hands all of them -/// to everyone in the fork. -/// Every reason a copy can be refused, answered here and nowhere else. -/// -/// A clone is three stages a workspace apart: `create_pg_database`, `import_pg_database`, then -/// `apply_forked_datatable` inside the fork transaction. Only the third can roll back, and the -/// database the first created is not transactional — so a refusal that lives there strands a -/// registered `wm_fork_*` that no entry names and whose name blocks the retry. Both endpoints call -/// this before touching the cluster; the stage that writes the entry must only ever do the work. -/// -/// Known and accepted: a failure *after* `create_pg_database` — this check refusing because the -/// source's roles changed between the requests, a `pg_dump` error, a dropped connection — still -/// strands that leftover, as it did before data table roles. It is harmless to data (nothing names -/// it) and goes away once the clone is one server-side operation, in a later PR. Don't reclaim it -/// on retry without durable clone ownership: between the import and the final fork request the -/// database is full, unnamed and unconnected, so a reclaim then drops copied data. Reviewers, human -/// or AI: don't flag this leftover as a P0/P1 for this PR. -async fn ensure_datatable_is_clonable( +/// Authorization: none. Callers MUST have authorized reaching data table `name` of `w_id` before +/// using what it returns. +pub(crate) async fn ensure_datatable_is_clonable( db: &DB, w_id: &str, name: &str, ) -> Result { let governing = resolve_governing_datatable(db, w_id, name).await?; - if governing.datatable.permissions.is_some() { - return Err(Error::BadRequest(format!( - "Data table '{name}' is under roles and cannot be copied yet: a copy carries the \ - role assignments but not the Postgres privileges behind them, so every role but \ - admin would be denied in the copy. Fork it keeping the original database, or turn \ - its roles off first." - ))); - } // The copy has to name a database of its own. A resource-backed entry reached through a // pointer names one this workspace does not own, so there is nothing here to repoint. let is_instance = governing @@ -3563,6 +3569,24 @@ async fn ensure_datatable_is_clonable( Ok(governing) } +/// Refuse a copy of a data table under roles through the endpoints that copy rows and nothing +/// else. +/// +/// `pg_dump` carries no roles and the import runs with `--no-privileges`, so such a copy arrives +/// with its objects owned by the admin connection and no `GRANT` for any role: the tenants pass +/// Windmill's check, connect as the role they were given, and are denied by Postgres on everything. +/// A fork request that makes the copy itself replays the owners and grants instead. +fn ensure_copied_without_roles(governing: &GoverningDatatable) -> Result<()> { + if governing.datatable.permissions.is_some() { + return Err(Error::BadRequest(format!( + "Data table '{}' is under roles, so a copy has to carry its owners and grants: \ + clone it through the fork wizard or `wmill workspace fork`, which do.", + governing.name + ))); + } + Ok(()) +} + /// Import (pg_dump/pg_import) from source to target async fn import_pg_database( authed: ApiAuthed, @@ -3577,7 +3601,7 @@ async fn import_pg_database( if let Some(reference) = req.source.strip_prefix("datatable://") { let (name, _) = parse_datatable_ref_for(&db, &w_id, reference).await?; - ensure_datatable_is_clonable(&db, &w_id, &name).await?; + ensure_copied_without_roles(&ensure_datatable_is_clonable(&db, &w_id, &name).await?)?; } if req.fork_behavior == DataTableForkBehavior::SchemaAndData { @@ -3698,6 +3722,15 @@ async fn edit_ducklake_config( } let mut tx = db.begin().await?; + windmill_common::lock_instance_databases( + &mut tx, + new_config.settings.ducklakes.values().filter_map(|lake| { + (lake.catalog.resource_type + == windmill_common::workspaces::DucklakeCatalogResourceType::Instance) + .then_some(lake.catalog.resource_path.as_str()) + }), + ) + .await?; let args_for_audit = format!("{:?}", new_config.settings); audit_log( @@ -3799,6 +3832,16 @@ async fn edit_datatable_config( let is_superadmin = require_super_admin(&db, &authed).await.is_ok(); let mut tx = db.begin().await?; + windmill_common::lock_instance_databases( + &mut tx, + new_config.settings.datatables.values().filter_map(|dt| { + dt.database + .as_ref() + .filter(|d| d.resource_type == DataTableCatalogResourceType::Instance) + .map(|d| d.resource_path.as_str()) + }), + ) + .await?; // Read under the row lock this transaction will write with. `permissions`, `reference` and // `forked_from` are carried across from what this read returns, so a permissions save @@ -3924,12 +3967,28 @@ async fn edit_datatable_config( // hand the fork the database outright. `forked_from` is the clone stamp the fork flow // writes: whether an entry has one is carried the same way, since it is what marks the // database droppable, but the schema baseline inside it is the diff view's to advance. + // `governed_by` is a clone's `reference` for its roles, and clearing it would hand the + // fork the copied rows the same way. dt.permissions = old.and_then(|old| old.permissions.clone()); dt.reference = old.and_then(|old| old.reference.clone()); + dt.governed_by = old.and_then(|old| old.governed_by.clone()); dt.forked_from = match old.and_then(|old| old.forked_from.as_ref()) { Some(stored) => Some(dt.forked_from.take().unwrap_or_else(|| stored.clone())), None => None, }; + // A clone's roles and grants were replayed into the database it was copied into, and hold + // for that database alone: whoever saves, it stays where it is. + if dt.governed_by.is_some() + && !crate::datatable_clone::same_database( + &dt.database, + &old.and_then(|old| old.database.clone()), + ) + { + return Err(Error::BadRequest(format!( + "Data table '{name}' is a clone taking its roles from the data table it was copied \ + from, and its grants hold for its own database only: it cannot be moved." + ))); + } // Carrying the block onto a resource-backed entry would produce a data table the chokepoint // refuses on every job — a save that succeeds and breaks everything afterwards. Refuse it // instead: turning roles off first is one step, and it keeps discarding an access decision @@ -3968,9 +4027,18 @@ async fn edit_datatable_config( // workspace does not own. Pointing an entry at another workspace's data table is not checked // here because it cannot be requested at all: `reference` is overwritten from the stored entry // above, for every caller. + // + // Compared against the entry the carried fields came from, not the one stored under the same + // name: otherwise swapping two names keeps each database in place while moving a clone's + // `governed_by` off the copy it governs. if !is_superadmin { for (name, dt) in new_config.settings.datatables.iter() { - let old_dt = old_datatables.get(name); + let old_dt = old_datatables.get( + rename_src + .get(name.as_str()) + .copied() + .unwrap_or(name.as_str()), + ); if dt .database .as_ref() @@ -4010,7 +4078,8 @@ async fn edit_datatable_config( .settings .datatables .iter() - .filter(|(_, dt)| dt.permissions.is_none()) + // A clone carries its roles through `governed_by` rather than `permissions`. + .filter(|(_, dt)| dt.permissions.is_none() && dt.governed_by.is_none()) .filter_map(|(name, dt)| { let db = dt .database @@ -4043,7 +4112,7 @@ async fn edit_datatable_config( sqlx::query_scalar( "SELECT DISTINCT dt.value->'database'->>'resource_path' FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt - WHERE ws.workspace_id <> $1 AND dt.value ? 'permissions' + WHERE ws.workspace_id <> $1 AND (dt.value ? 'permissions' OR dt.value ? 'governed_by') AND dt.value->'database'->>'resource_type' = 'instance'", ) .bind(&w_id) @@ -4052,7 +4121,7 @@ async fn edit_datatable_config( }; for (name, dbname) in newly_pointed { let governed_here = old_datatables.values().any(|old| { - old.permissions.is_some() + (old.permissions.is_some() || old.governed_by.is_some()) && old.database.as_ref().is_some_and(|d| { d.resource_type == DataTableCatalogResourceType::Instance && d.resource_path == dbname @@ -4112,8 +4181,11 @@ async fn edit_datatable_config( r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!" FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt - WHERE dt.value->'reference'->>'workspace_id' = $1 - AND dt.value->'reference'->>'datatable' = $2"#, + WHERE EXISTS ( + SELECT 1 FROM (VALUES ('reference'), ('governed_by')) k(link) + WHERE dt.value->k.link->>'workspace_id' = $1 + AND dt.value->k.link->>'datatable' = $2 + )"#, &w_id, name, ) @@ -8241,6 +8313,7 @@ async fn create_workspace_fork_branch( // dangling branch on the synced repos. check_fork_w_id_conflict(&db, &nw.id).await?; purge_stale_fork_diff_state(&db, &nw.id).await?; + validate_forked_datatables(&db, &authed, &w_id, &nw).await?; Ok(Json( handle_fork_branch_creation(&authed.email, &authed.username, &db, &w_id, &nw.id).await?, @@ -8280,8 +8353,9 @@ 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 dropped their copied `permissions` — for a clone of a data +/// table under roles, in favor of `governed_by`. async fn point_kept_datatables_at_parent( tx: &mut Transaction<'_, Postgres>, parent_w_id: &str, @@ -8336,6 +8410,7 @@ async fn point_kept_datatables_at_parent( workspace_id: parent_w_id.to_string(), datatable: name.clone(), }), + governed_by: None, forked_from: None, migrations_enabled: dt.migrations_enabled, permissions: None, @@ -8356,7 +8431,8 @@ async fn point_kept_datatables_at_parent( Ok(()) } -/// Move every pointer in any workspace that names `(w_id, from)` to `(w_id, to)`. +/// Move every pointer, and every clone's `governed_by`, 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 @@ -8372,17 +8448,22 @@ async fn repoint_datatable_references( 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 + (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $1 + AND r.v->'governed_by'->>'datatable' = $2 + THEN jsonb_set(r.v, '{governed_by,datatable}', to_jsonb($3::text)) + ELSE r.v END + FROM (SELECT 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 AS v) r) )) 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 + SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d, + LATERAL (VALUES ('reference'), ('governed_by')) k(link) + WHERE d.value->k.link->>'workspace_id' = $1 + AND d.value->k.link->>'datatable' = $2 )"#, w_id, from, @@ -8400,17 +8481,8 @@ async fn apply_forked_datatable( parent_w_id: &str, forked_w_id: &str, fdt: &ForkedDatatableInfo, + copy: &crate::datatable_clone::MadeCopy, ) -> Result<()> { - // Cloning reads the parent's whole schema as admin and hands the copy to the fork, so it is - // for the workspace that governs the data table — a fork can use one, never duplicate it. - windmill_common::workspaces::ensure_datatable_admin_access( - db, - parent_w_id, - &fdt.name, - &DatatableAccess::Authed(authed.to_authed_ref()), - ) - .await?; - let governing = ensure_datatable_is_clonable(db, parent_w_id, &fdt.name).await?; windmill_common::validate_dbname(&fdt.new_dbname)?; if !fdt.new_dbname.starts_with("wm_fork_") { return Err(Error::BadRequest(format!( @@ -8418,6 +8490,52 @@ async fn apply_forked_datatable( fdt.new_dbname ))); } + // Held until the fork commits, as the permissions save holds them: the source moved onto + // another database, or put under roles or taken off them, since its copy was made would link + // the copy to roles its grants were not replayed for. + let locked = crate::datatable_clone::lock_settings_rows(tx, parent_w_id, &fdt.name).await?; + let governing = ensure_datatable_is_clonable(db, parent_w_id, &fdt.name).await?; + let under_roles = governing.datatable.permissions.is_some(); + let unchanged = crate::datatable_clone::same_database( + &governing.datatable.database, + &Some(copy.source_database.clone()), + ) && copy.replayed == under_roles + && crate::datatable_clone::lock_settings_rows(tx, parent_w_id, &fdt.name).await? == locked; + if !unchanged { + return Err(Error::BadRequest(format!( + "Data table '{}' changed while it was being copied for this fork — its database, or \ + whether it is under roles. Create the fork again.", + fdt.name + ))); + } + // 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_governing_datatable( + db, + parent_w_id, + &fdt.name, + &governing, + authed, + ) + .await?; + // Under roles, the copy holds rows the source's roles decide who reaches, so that entry keeps + // deciding. Settled from the source as it resolves now: the settings clone may have handed the + // fork a pointer, or a clone of its own. A copy of a data table without roles is the fork's, + // as it was before roles existed: everyone reached all of it already. + let governed_by = governing + .datatable + .permissions + .is_some() + .then(|| { + serde_json::to_value(governing.governor.clone().unwrap_or_else(|| { + windmill_common::workspaces::DataTableReference { + workspace_id: governing.workspace_id.clone(), + datatable: governing.name.clone(), + } + })) + }) + .transpose() + .map_err(|e| Error::internal_err(format!("serializing a clone's governor: {e}")))?; // Snapshot the schema from the source (parent) datatable let schema = snapshot_datatable_schema(db, parent_w_id, &fdt.name).await?; @@ -8463,33 +8581,42 @@ async fn apply_forked_datatable( "resource_type": "instance", "resource_path": &fdt.new_dbname, }); - sqlx::query!( + sqlx::query( r#"UPDATE workspace_settings SET datatable = jsonb_set( - jsonb_set( - datatable #- ARRAY['datatables', $2, 'reference'], - ARRAY['datatables', $2, 'database'], $3::jsonb), - ARRAY['datatables', $2, 'forked_from'], $4::jsonb - ) + datatable #- ARRAY['datatables', $2, 'reference'], + ARRAY['datatables', $2, 'database'], $3::jsonb) WHERE workspace_id = $1"#, - forked_w_id, - &fdt.name, - new_database, - forked_from, ) + .bind(forked_w_id) + .bind(&fdt.name) + .bind(new_database) .execute(&mut **tx) .await?; } else { - // Resource: update the resource's dbname and mark as ws_specific + // Resource: point it at the copy and mark it ws_specific. What the settings clone carried is + // the source's resource and variables as they are now, which an edit made while the copy + // was being made can have changed, and changed back: the clone has to be what the copy's + // connection was resolved from. let resource_path = &database.resource_path; - sqlx::query!( + let cloned = + crate::datatable_clone::connection_snapshot(db, &mut **tx, forked_w_id, resource_path) + .await?; + if copy.connection.as_ref() != Some(&cloned) { + return Err(Error::BadRequest(format!( + "The resource of data table '{}' changed while it was being copied for this \ + fork. Create the fork again.", + fdt.name + ))); + } + sqlx::query( r#"UPDATE resource SET value = jsonb_set(value, '{dbname}', to_jsonb($3::text)) WHERE workspace_id = $1 AND path = $2"#, - forked_w_id, - resource_path, - &fdt.new_dbname, ) + .bind(forked_w_id) + .bind(resource_path) + .bind(&fdt.new_dbname) .execute(&mut **tx) .await?; @@ -8500,20 +8627,31 @@ async fn apply_forked_datatable( ) .execute(&mut **tx) .await?; - - // Set forked_from on the datatable config - sqlx::query!( - r#"UPDATE workspace_settings - SET datatable = jsonb_set(datatable, ARRAY['datatables', $2, 'forked_from'], $3::jsonb) - WHERE workspace_id = $1"#, - forked_w_id, - &fdt.name, - forked_from, - ) - .execute(&mut **tx) - .await?; } + // The settings clone copied the source's `permissions` along, and a clone of a clone its + // `governed_by`: a clone keeps no `permissions` of its own, and a link only under roles. + sqlx::query( + r#"UPDATE workspace_settings + SET datatable = jsonb_set( + CASE WHEN $3::jsonb IS NULL + THEN datatable #- ARRAY['datatables', $2, 'permissions'] + #- ARRAY['datatables', $2, 'governed_by'] + ELSE jsonb_set( + datatable #- ARRAY['datatables', $2, 'permissions'], + ARRAY['datatables', $2, 'governed_by'], $3::jsonb) + END, + ARRAY['datatables', $2, 'forked_from'], $4::jsonb + ) + WHERE workspace_id = $1"#, + ) + .bind(forked_w_id) + .bind(&fdt.name) + .bind(governed_by) + .bind(forked_from) + .execute(&mut **tx) + .await?; + Ok(()) } @@ -8848,7 +8986,169 @@ async fn create_workspace_fork( ensure_no_existing_dev_workspace(&db, &parent_workspace_id).await?; } + // Refused here, before any database exists; `make_copies` checks again under the locks. + validate_forked_datatables(&db, &authed, &parent_workspace_id, &nw).await?; + + // Detached from the request: a client that goes away while the copies are made must still end + // with the fork created or no copy left behind, and dropping the handler's future would skip + // that cleanup. + tokio::spawn(async move { + let fork_id = nw.id.clone(); + let copies = crate::datatable_clone::make_copies( + &db, + &authed, + &parent_workspace_id, + ©_requests(&nw.forked_datatables), + ) + .await?; + let replayed: Vec = copies + .iter() + .filter(|c| c.replayed) + .map(|c| c.behavior) + .collect(); + match write_workspace_fork( + db.clone(), + authed, + parent_workspace_id, + nw, + dev_workspace_label, + &copies, + ) + .await + { + Ok(message) => { + for behavior in replayed { + windmill_common::feature_usage::log_feature_usage( + "datatable", + "clone_replayed", + match behavior { + DataTableForkBehavior::SchemaOnly => "schema_only", + _ => "schema_and_data", + }, + ); + } + Ok(message) + } + // A commit whose acknowledgement was lost can still have committed, and a concurrent + // request for the same id can have: once the write has settled, the cleanup keeps + // whichever copies a committed fork names, and drops the rest. + Err(e) => match wait_for_fork_write(&db, &fork_id).await { + Ok(()) => Err(crate::datatable_clone::drop_copies_after(&db, copies, e).await), + Err(settle) => { + tracing::error!( + "Could not tell whether fork '{fork_id}' was created, so its copies were \ + kept: {settle}" + ); + Err(e) + } + }, + } + }) + .await + .map_err(|e| Error::internal_err(format!("Creating the fork stopped unexpectedly: {e}")))? +} + +/// The database a data table of the fork or dev workspace `fork_id` is copied into, as the wizard +/// and the CLI derive it. A dev workspace's id has no `wm-fork-` prefix, and its copies are dropped +/// by the same `wm_fork_` rule as a fork's. Dev workspace `x` and fork `wm-fork-x` share a name, as +/// their branches do: the second to copy a data table of that name is refused at `CREATE`. +fn forked_datatable_dbname(fork_id: &str, datatable: &str) -> String { + let suffix = fork_id + .strip_prefix(windmill_common::workspaces::WM_FORK_PREFIX) + .unwrap_or(fork_id); + format!("wm_fork_{}__{datatable}", suffix.replace('-', "_")) +} + +/// Wait until no transaction writing fork `fork_id` is still running: the lock +/// `write_workspace_fork` holds until its transaction ends is taken and released. +async fn wait_for_fork_write(db: &DB, fork_id: &str) -> Result<()> { + let mut tx = db.begin().await?; + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('fork:' || $1))") + .bind(fork_id) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok(()) +} + +/// Everything about a fork's data tables that can be refused before a git branch or a database is +/// created, and that the fork request re-checks. +/// +/// Every data table is copied by the fork request, into a database it creates: an entry naming a +/// database that already exists could reach a copy made for another fork — one left behind by a +/// deleted fork included — without that copy's governance. +async fn validate_forked_datatables( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + nw: &CreateWorkspaceFork, +) -> Result<()> { + let mut names = HashSet::new(); + for fdt in &nw.forked_datatables { + if fdt.fork_behavior.is_none() { + return Err(Error::BadRequest(format!( + "Data table '{}' names no `fork_behavior`: the fork request makes the copy of each \ + data table itself. Update the Windmill CLI.", + fdt.name + ))); + } + let expected = forked_datatable_dbname(&nw.id, &fdt.name); + if fdt.new_dbname != expected { + return Err(Error::BadRequest(format!( + "Data table '{}' of fork '{}' is copied into database '{expected}', not '{}'", + fdt.name, nw.id, fdt.new_dbname + ))); + } + if !names.insert(fdt.name.as_str()) { + return Err(Error::BadRequest(format!( + "Data table '{}' is named more than once in this fork", + fdt.name + ))); + } + } + crate::datatable_clone::authorize_copies( + db, + authed, + parent_w_id, + ©_requests(&nw.forked_datatables), + ) + .await +} + +/// The copies a fork request asks this request to make. [`validate_forked_datatables`] refuses an +/// entry without a `fork_behavior`. +fn copy_requests( + forked_datatables: &[ForkedDatatableInfo], +) -> Vec> { + forked_datatables + .iter() + .filter_map(|fdt| { + fdt.fork_behavior + .map(|behavior| crate::datatable_clone::CopyRequest { + name: &fdt.name, + dbname: &fdt.new_dbname, + behavior, + }) + }) + .collect() +} + +/// Write the fork, with its data tables pointed at `copies`. +async fn write_workspace_fork( + db: DB, + authed: ApiAuthed, + parent_workspace_id: String, + nw: CreateWorkspaceFork, + dev_workspace_label: Option, + copies: &[crate::datatable_clone::MadeCopy], +) -> Result { let mut tx: Transaction<'_, Postgres> = db.begin().await?; + // Held until this transaction ends: after an error, the copies' cleanup waits on it, so it reads + // whether the fork exists only once a commit still being resolved has settled. + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('fork:' || $1))") + .bind(&nw.id) + .execute(&mut *tx) + .await?; if nw.is_dev_workspace { // The checks above ran outside a transaction, so the parent's eligibility and the chain's @@ -8974,10 +9274,41 @@ async fn create_workspace_fork( repoint_unresolvable_cloned_identities(&mut tx, &forked_id, &authed).await?; + // Held until the fork commits, as settings saves naming these databases hold it: an entry + // saved elsewhere while a copy was being made would reach it without the governance written + // below, and the alias check on that save saw no governed entry yet. + let instance_copies: Vec<&str> = copies + .iter() + .filter(|c| c.source_database.resource_type == DataTableCatalogResourceType::Instance) + .map(|c| c.dbname.as_str()) + .collect(); + windmill_common::lock_instance_databases(&mut tx, instance_copies.iter().copied()).await?; + for dbname in &instance_copies { + let users = windmill_common::instance_database_users(&mut *tx, dbname).await?; + if !users.is_empty() { + return Err(Error::BadRequest(format!( + "Database '{dbname}' copied for this fork is already used by workspaces {}; \ + fork again", + users.join(", ") + ))); + } + } + // 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) - .await?; + let copy = copies.iter().find(|c| c.name == fdt.name).ok_or_else(|| { + Error::internal_err(format!("No copy was made of data table '{}'", fdt.name)) + })?; + apply_forked_datatable( + &db, + &mut tx, + &authed, + &parent_workspace_id, + &forked_id, + fdt, + copy, + ) + .await?; } point_kept_datatables_at_parent( @@ -9036,6 +9367,20 @@ async fn create_workspace_fork( .await?; } + let copied = copies + .iter() + .map(|c| { + format!( + "{} ({})", + c.name, + match c.behavior { + DataTableForkBehavior::SchemaOnly => "schema_only", + _ => "schema_and_data", + } + ) + }) + .collect::>() + .join(", "); audit_log( &mut *tx, &authed, @@ -9043,7 +9388,7 @@ async fn create_workspace_fork( ActionKind::Create, &forked_id, Some(nw.name.as_str()), - None, + (!copied.is_empty()).then(|| [("copied_datatables", copied.as_str())].into()), ) .await?; tx.commit().await?; diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 62ab812a8f..82423480fa 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -502,21 +502,25 @@ pub(crate) async fn change_workspace_id( // A fork's data table entry names the workspace that governs it by id, so the rename has to // follow there too — anywhere, not just in the reparented children: a detached workspace can // point at this one without being its fork. Left behind, the pointer resolves to the archived - // shell and every job through it stops. + // shell and every job through it stops. A clone's `governed_by` names it the same way. info!("Re-pointing data table references to the new workspace id"); 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' = $2 - THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text)) - ELSE dt.value END + (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $2 + THEN jsonb_set(r.v, '{governed_by,workspace_id}', to_jsonb($1::text)) + ELSE r.v END + FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $2 + THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text)) + ELSE dt.value END AS v) r) )) FROM jsonb_each(ws.datatable->'datatables') dt ) WHERE jsonb_typeof(ws.datatable->'datatables') = 'object' - AND ws.datatable::text LIKE '%"reference"%'"#, + AND (ws.datatable::text LIKE '%"reference"%' + OR ws.datatable::text LIKE '%"governed_by"%')"#, &rw.new_id, &old_id, ) @@ -1003,17 +1007,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(); @@ -1341,7 +1347,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-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 4ddfbffa28..9c4212c234 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -33872,6 +33872,15 @@ components: $ref: "#/components/schemas/DatatableRoleTenants" governing_workspace_id: type: string + clone_of: + type: object + description: for a clone, the data table whose roles it takes + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string editable: type: boolean available_roles: @@ -34110,7 +34119,7 @@ components: DatatableAclInfo: type: object - required: [owner, roles, editable, supports_maintain, dbname, grants, children] + required: [owner, roles, editable, clone, supports_maintain, dbname, grants, children] properties: owner: type: string @@ -34122,6 +34131,9 @@ components: editable: type: boolean description: whether the caller may plan and apply changes + clone: + type: boolean + description: whether this is a clone, whose grants stay as they were copied supports_maintain: type: boolean description: whether the server is Postgres 17+, which added the MAINTAIN table privilege @@ -35274,6 +35286,16 @@ components: new_dbname: type: string description: "New database name for the fork" + fork_behavior: + type: string + enum: + - schema_only + - schema_and_data + description: >- + What the fork request copies into `new_dbname`, which it creates — with the + owners and grants of a data table under roles. This server refuses an entry + without it; servers predating it expect `new_dbname` created and filled + beforehand. shared_ducklakes: type: array items: @@ -36237,6 +36259,17 @@ components: type: string datatable: type: string + governed_by: + description: >- + On a clone, the data table it was copied from, whose roles it takes. Server-owned + like `reference`. + type: object + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string migrations_enabled: type: boolean description: Whether the SQL migrations feature is opted in for this data table diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index a67bb6bfa1..5feaba7800 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -1474,8 +1474,98 @@ pub fn validate_dbname(dbname: &str) -> error::Result<()> { Ok(()) } +/// Lock the instance databases among `names` until `tx` ends, in a stable order. Taken by every +/// settings save naming an instance database, and by [`drop_unused_instance_database`]: a save +/// cannot start using a database between that drop's check that nothing does and the drop. +pub async fn lock_instance_databases<'a>( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + names: impl IntoIterator, +) -> error::Result<()> { + let names: std::collections::BTreeSet<&str> = names.into_iter().collect(); + for name in names { + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('instance_database:' || $1))") + .bind(name) + .execute(&mut **tx) + .await?; + } + Ok(()) +} + +/// Drop instance database `dbname`, which a request just created, unless a workspace names it as a +/// data table or a ducklake catalog — the workspaces returned, with nothing dropped. A database is +/// registered on the instance as soon as it is created, so a superadmin can point a workspace at it +/// before the request that created it gives up on it. +/// +/// Authorization: none. Callers MUST pass only a database the same request created and has not +/// handed to anything yet. +/// +/// The lock, the check and the drop share one connection: a second one taken from the pool while +/// the first is held could wait forever on a small pool. +pub async fn drop_unused_instance_database(db: &DB, dbname: &str) -> error::Result> { + let mut conn = db.acquire().await?; + let key = format!("instance_database:{dbname}"); + sqlx::query("SELECT pg_advisory_lock(hashtext($1))") + .bind(&key) + .execute(&mut *conn) + .await?; + let dropped = drop_if_unused_on(&mut conn, dbname).await; + let unlocked = sqlx::query("SELECT pg_advisory_unlock(hashtext($1))") + .bind(&key) + .execute(&mut *conn) + .await; + if unlocked.is_err() { + // Closing the connection is what releases a session lock it could not release itself. + drop(conn.detach()); + } + dropped +} + +async fn drop_if_unused_on( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result> { + let users = instance_database_users(conn, dbname).await?; + if users.is_empty() { + drop_custom_instance_database_on(conn, dbname).await?; + } + Ok(users) +} + +/// The workspaces naming instance database `dbname` as a data table or a ducklake catalog. Taken +/// under [`lock_instance_databases`] for `dbname`, the answer holds until that lock is released. +/// +/// Authorization: none, and it reads every workspace's settings. Callers MUST pass only a database +/// their own request created, and may name the workspaces returned only to a caller allowed to +/// create or drop instance databases. +pub async fn instance_database_users( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result> { + Ok(sqlx::query_scalar( + "SELECT DISTINCT ws.workspace_id FROM workspace_settings ws + WHERE EXISTS (SELECT 1 FROM jsonb_each(CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object' + THEN ws.datatable->'datatables' ELSE '{}'::jsonb END) dt + WHERE dt.value->'database'->>'resource_type' = 'instance' + AND dt.value->'database'->>'resource_path' = $1) + OR EXISTS (SELECT 1 FROM jsonb_each(CASE WHEN jsonb_typeof(ws.ducklake->'ducklakes') = 'object' + THEN ws.ducklake->'ducklakes' ELSE '{}'::jsonb END) dl + WHERE dl.value->'catalog'->>'resource_type' = 'instance' + AND dl.value->'catalog'->>'resource_path' = $1)", + ) + .bind(dbname) + .fetch_all(conn) + .await?) +} + /// Drop a custom instance database: validate, terminate connections, DROP DATABASE, remove from global_settings. pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Result<()> { + drop_custom_instance_database_on(&mut *db.acquire().await?, dbname).await +} + +async fn drop_custom_instance_database_on( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result<()> { let dbname = dbname.trim(); validate_dbname(dbname)?; @@ -1490,7 +1580,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", dbname ) - .fetch_one(db) + .fetch_one(&mut *conn) .await? .unwrap_or(false); @@ -1501,7 +1591,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '{}' AND pid <> pg_backend_pid()", dbname.replace('\'', "''") )) - .execute(db) + .execute(&mut *conn) .await { tracing::warn!("Failed to terminate connections to '{}': {}", dbname, e); @@ -1510,7 +1600,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu // Drop the database // SAFETY: `dbname` has been validated via validate_dbname() before reaching this point. sqlx::query(&format!("DROP DATABASE IF EXISTS \"{}\"", dbname)) - .execute(db) + .execute(&mut *conn) .await .map_err(|e| { error::Error::internal_err(format!("Failed to drop database '{}': {}", dbname, e)) @@ -1526,7 +1616,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu r#"UPDATE global_settings SET value = value #- ARRAY['databases', $1] WHERE name = 'custom_instance_pg_databases'"#, dbname ) - .execute(db) + .execute(&mut *conn) .await?; Ok(()) @@ -1600,6 +1690,35 @@ pub async fn create_custom_instance_database( error::Error::internal_err(format!("Failed to create database '{}': {}", dbname, e)) })?; + // Nothing names a database that failed past this point, and its name blocks the retry: drop it + // rather than leave it behind. + if let Err(e) = finish_custom_instance_database(db, dbname, tag).await { + match drop_unused_instance_database(db, dbname).await { + Ok(users) if !users.is_empty() => tracing::warn!( + "Kept '{dbname}' after failing to set it up: workspaces {} use it", + users.join(", ") + ), + Ok(_) => {} + Err(drop_err) => { + tracing::error!("Could not drop '{dbname}' after failing to set it up: {drop_err}") + } + } + return Err(e); + } + + // A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT + // would otherwise let every role in regardless of what this instance defines. Best-effort: a + // failure here leaves the database usable as `admin`, and the next role change repairs it. + if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await { + tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); + } + + tracing::info!("Created custom instance database '{}'", dbname); + Ok(()) +} + +/// Grant `custom_instance_user` its privileges on a database just created, and register it. +async fn finish_custom_instance_database(db: &DB, dbname: &str, tag: &str) -> error::Result<()> { // Grant permissions to custom_instance_user let wmill_pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; let new_pg_creds = PgDatabase { dbname: dbname.to_string(), ..wmill_pg_creds }; @@ -1634,15 +1753,6 @@ pub async fn create_custom_instance_database( ) .execute(db) .await?; - - // A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT - // would otherwise let every role in regardless of what this instance defines. Best-effort: a - // failure here leaves the database usable as `admin`, and the next role change repairs it. - if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await { - tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); - } - - tracing::info!("Created custom instance database '{}'", dbname); Ok(()) } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 199b2f0906..6f82e3a261 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1304,6 +1304,12 @@ pub struct DataTable { /// nothing local for a fork admin to widen. #[serde(default, skip_serializing_if = "Option::is_none")] pub reference: Option, + /// Set on a *clone* — a terminal entry whose database was copied from the entry this names. The + /// copy holds that entry's rows, so who may connect as which role stays that entry's decision: + /// the clone carries no `permissions` of its own and is governed like a pointer, while + /// connecting to its own database. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub governed_by: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub forked_from: Option, /// Whether the SQL-migrations feature is opted in for this data table. @@ -1362,9 +1368,16 @@ pub const DATATABLE_TENANT_WILDCARD: &str = "*"; /// enough to survive a fork of a fork. const DATATABLE_REFERENCE_MAX_DEPTH: usize = 20; -/// Exactly one of `database` and `reference` must be set. Called wherever an entry is persisted, -/// so nothing downstream has to handle an entry that is both or neither. +/// Exactly one of `database` and `reference` must be set, and a clone's `governed_by` leaves no +/// `permissions` beside it. Called wherever an entry is persisted, so nothing downstream has to +/// handle an entry that is both or neither, or a clone with a decision of its own. pub fn validate_datatable_shape(name: &str, dt: &DataTable) -> Result<()> { + if dt.governed_by.is_some() && (dt.database.is_none() || dt.permissions.is_some()) { + return Err(Error::BadRequest(format!( + "Data table '{name}' is a clone, which owns a database and takes its roles from the \ + data table it was cloned from" + ))); + } match (&dt.database, &dt.reference) { (Some(_), None) | (None, Some(_)) => Ok(()), (Some(_), Some(_)) => Err(Error::BadRequest(format!( @@ -1452,23 +1465,28 @@ pub async fn read_datatable_entry(db: &DB, w_id: &str, name: &str) -> Result(datatable.clone())?) } -/// The terminal entry a reference chain lands on: the workspace that governs the data table, the -/// entry name there, and the entry itself. A terminal entry resolves to itself. +/// The terminal entry a reference chain lands on, and what governs it: the workspace and name of +/// the entry that owns the database, the entry itself, and — for a clone — the entry its +/// `governed_by` chain lands on. A terminal entry that is not a clone resolves to itself and +/// governs itself. /// -/// Every decision downstream — which database to connect to, whose `permissions` apply, whose -/// members tenants are evaluated against, who may administer it — is taken on this, never on the -/// entry the caller named. +/// Every decision downstream is taken on this, never on the entry the caller named. Which database +/// to connect to comes from `workspace_id` / `name` / `datatable.database`; whose `permissions` +/// apply (already in `datatable.permissions`), whose members tenants are evaluated against and who +/// may administer it come from [`GoverningDatatable::governing_workspace_id`]. /// /// Authorization: resolving deliberately crosses into the governing workspace, so it answers for a /// workspace the caller may not belong to and checks nothing itself. It is the input to the /// checks, not one of them: callers MUST pass what it returns to /// [`can_use_datatable_role_in_governing_workspace`] or [`ensure_datatable_admin_access`] before -/// acting on it, and MUST NOT return its `permissions` or `workspace_id` to a caller from +/// acting on it, and MUST NOT return its `permissions` or workspace ids to a caller from /// elsewhere without gating on the answer. pub struct GoverningDatatable { pub workspace_id: String, pub name: String, pub datatable: DataTable, + /// For a clone, the entry whose `permissions` govern it. `None` when the entry governs itself. + pub governor: Option, } impl GoverningDatatable { @@ -1480,6 +1498,13 @@ impl GoverningDatatable { .as_ref() .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance) } + + /// The workspace whose admins administer the data table and whose members its tenants are. + pub fn governing_workspace_id(&self) -> &str { + self.governor + .as_ref() + .map_or(&self.workspace_id, |g| &g.workspace_id) + } } pub async fn resolve_governing_datatable( @@ -1490,6 +1515,9 @@ pub async fn resolve_governing_datatable( let mut workspace_id = w_id.to_string(); let mut name = name.to_string(); let mut hops = 0; + // The clone a `governed_by` chain started from: it keeps its database, and takes the + // `permissions` of wherever the chain lands. + let mut clone: Option<(String, String, DataTable)> = None; for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH { let datatable = read_datatable_entry(db, &workspace_id, &name) .await @@ -1500,21 +1528,48 @@ 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; validate_datatable_shape(&name, &datatable)?; - match &datatable.reference { - None => return Ok(GoverningDatatable { workspace_id, name, datatable }), - Some(reference) => { - workspace_id = reference.workspace_id.clone(); - name = reference.datatable.clone(); + let next = match (&datatable.reference, &datatable.governed_by) { + (Some(reference), _) => reference.clone(), + (None, Some(governed_by)) => { + let governed_by = governed_by.clone(); + if clone.is_none() { + clone = Some((workspace_id.clone(), name.clone(), datatable)); + } + governed_by } - } + (None, None) => { + return Ok(match clone { + None => GoverningDatatable { workspace_id, name, datatable, governor: None }, + Some((clone_w_id, clone_name, mut clone_datatable)) => { + clone_datatable.permissions = datatable.permissions; + GoverningDatatable { + workspace_id: clone_w_id, + name: clone_name, + datatable: clone_datatable, + governor: Some(DataTableReference { workspace_id, datatable: name }), + } + } + }); + } + }; + workspace_id = next.workspace_id; + name = next.datatable; } Err(Error::BadRequest(format!( "Data table '{name}' points at another data table through more than \ @@ -1554,17 +1609,20 @@ pub async fn resolve_workspace_governing_datatables( let mut entries = Entries::new(); let listed = load(db, &[w_id.to_string()], &mut entries).await?; - // (index into `listed`, workspace, entry name) still to be followed. - let mut cursors: Vec<(usize, String, String)> = listed + // (index into `listed`, workspace, entry name, the clone the chain started from) still to be + // followed. A clone keeps its own database and takes the `permissions` of wherever its + // `governed_by` chain lands, as the single resolution does. + type Clone = Option<(String, String, DataTable)>; + let mut cursors: Vec<(usize, String, String, Clone)> = listed .iter() .enumerate() - .map(|(i, name)| (i, w_id.to_string(), name.clone())) + .map(|(i, name)| (i, w_id.to_string(), name.clone(), None)) .collect(); let mut resolved: Vec<(usize, GoverningDatatable)> = vec![]; for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH { let mut next = vec![]; - for (i, ws, name) in cursors.drain(..) { + for (i, ws, name, clone) in cursors.drain(..) { let Some(value) = entries .get(&ws) .and_then(|m| m.get(&name)) @@ -1578,14 +1636,41 @@ pub async fn resolve_workspace_governing_datatables( if validate_datatable_shape(&name, &datatable).is_err() { continue; } - match &datatable.reference { - None => { - resolved.push((i, GoverningDatatable { workspace_id: ws, name, datatable })) - } - Some(reference) => next.push(( + match (&datatable.reference, &datatable.governed_by) { + (Some(reference), _) => next.push(( i, reference.workspace_id.clone(), reference.datatable.clone(), + clone, + )), + (None, Some(governed_by)) => { + let (governor_ws, governor_name) = + (governed_by.workspace_id.clone(), governed_by.datatable.clone()); + let clone = clone.or(Some((ws, name, datatable))); + next.push((i, governor_ws, governor_name, clone)); + } + (None, None) => resolved.push(( + i, + match clone { + None => GoverningDatatable { + workspace_id: ws, + name, + datatable, + governor: None, + }, + Some((clone_ws, clone_name, mut clone_datatable)) => { + clone_datatable.permissions = datatable.permissions; + GoverningDatatable { + workspace_id: clone_ws, + name: clone_name, + datatable: clone_datatable, + governor: Some(DataTableReference { + workspace_id: ws, + datatable: name, + }), + } + } + }, )), } } @@ -1594,7 +1679,7 @@ pub async fn resolve_workspace_governing_datatables( } let to_load: Vec = next .iter() - .map(|(_, ws, _)| ws.clone()) + .map(|(_, ws, _, _)| ws.clone()) .filter(|ws| !entries.contains_key(ws)) .collect::>() .into_iter() @@ -3356,6 +3441,7 @@ mod tests { resource_path: "dt_main".to_string(), }), reference: None, + governed_by: None, forked_from: None, migrations_enabled: None, permissions: None, diff --git a/cli/src/commands/workspace/fork.ts b/cli/src/commands/workspace/fork.ts index 219252bb7a..0aca0984ff 100644 --- a/cli/src/commands/workspace/fork.ts +++ b/cli/src/commands/workspace/fork.ts @@ -239,6 +239,7 @@ async function createWorkspaceFork( interface ForkedDatatableInfo { name: string; new_dbname: string; + fork_behavior?: "schema_only" | "schema_and_data"; } const forkedDatatables: ForkedDatatableInfo[] = []; @@ -285,6 +286,23 @@ async function createWorkspaceFork( const newDbName = `${trueWorkspaceId.replace(/-/g, "_")}__${dt.name}`; + const forkBehavior = dtBehavior as "schema_only" | "schema_and_data"; + // A server that reports `permissioned` copies each data table in the fork request itself, + // and refuses a database copied beforehand. An older one only takes a database copied here. + if (typeof dt.permissioned === "boolean") { + log.info( + colors.blue( + ` Datatable "${dt.name}" will be cloned (${forkBehavior === "schema_only" ? "schema" : "schema + data"}) into "${newDbName}" when the fork is created.` + ) + ); + forkedDatatables.push({ + name: dt.name, + new_dbname: newDbName, + fork_behavior: forkBehavior, + }); + continue; + } + try { log.info( colors.blue(` Creating database "${newDbName}" for datatable "${dt.name}"...`) @@ -300,7 +318,7 @@ async function createWorkspaceFork( log.info( colors.blue( - ` Importing ${dtBehavior === "schema_only" ? "schema" : "schema + data"}...` + ` Importing ${forkBehavior === "schema_only" ? "schema" : "schema + data"}...` ) ); @@ -310,7 +328,7 @@ async function createWorkspaceFork( source: `datatable://${dt.name}`, target: `datatable://${dt.name}`, target_dbname_override: newDbName, - fork_behavior: dtBehavior as "schema_only" | "schema_and_data", + fork_behavior: forkBehavior, }, }); @@ -336,6 +354,8 @@ async function createWorkspaceFork( id: trueWorkspaceId, name: opts.createWorkspaceName ?? workspaceName ?? trueWorkspaceId, color: forkColor, + // So a clone the fork would refuse is refused before any branch is created. + forked_datatables: forkedDatatables, }, }); if (gitSyncJobIds && gitSyncJobIds.length > 0) { diff --git a/frontend/src/lib/components/InstanceSettings.svelte b/frontend/src/lib/components/InstanceSettings.svelte index abe37c3537..d87de125a9 100644 --- a/frontend/src/lib/components/InstanceSettings.svelte +++ b/frontend/src/lib/components/InstanceSettings.svelte @@ -1094,9 +1094,10 @@ the home page’s create menu and hub-project picker are opened and from which entry point, the name of any public hub project imported from the home page and how far that import got, whether a pre-approved trial offer was opened, whether data tables are put - under roles and whether callers name a role or take the default, and which kinds of - access change (grant, revoke, ownership, default privileges) are applied to data - tables, last 30 days)
  • feature adoption (counts of which flow, script, trigger, worker and data table @@ -1163,9 +1164,10 @@ the home page’s create menu and hub-project picker are opened and from which entry point, the name of any public hub project imported from the home page and how far that import got, whether a pre-approved trial offer was opened, whether data tables are put - under roles and whether callers name a role or take the default, and which kinds of - access change (grant, revoke, ownership, default privileges) are applied to data - tables, last 30 days)
  • feature adoption (counts of which flow, script, trigger, worker and data table diff --git a/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte b/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte index 801ec13ffb..a3a765014a 100644 --- a/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte +++ b/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte @@ -114,7 +114,12 @@ Loading… {:else}
    - {#if !info.editable} + {#if info.clone} + + Read only: this data table is a clone, and its owners and grants stay as they were copied + from the data table it was cloned from. + + {:else if !info.editable} Read only: access is changed by the admins of the workspace that governs this data table, on Windmill Enterprise Edition. diff --git a/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte b/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte index 2b0962f32e..d0f86b36ad 100644 --- a/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte +++ b/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte @@ -380,7 +380,7 @@ failedSyncJobs = [] forkCreationError = '' - // Clone datatables BEFORE creating the workspace fork + // Each data table copy is confirmed first; the fork request makes the copies if (forkDatatableSection) { const queue = forkDatatableSection.buildCloneQueue(prefixed_id) if (queue.length > 0) { @@ -396,6 +396,15 @@ } async function completeFork(prefixed_id: string): Promise { + // The fork request makes these copies, and drops them if the fork is not created + const forkedDatatables = forkDatatableSection + ? forkDatatableSection.getConfirmedCloneJobs().map((job) => ({ + name: job.name, + new_dbname: job._newDbName, + fork_behavior: job.behavior + })) + : [] + let gitSyncJobIds: string[] try { gitSyncJobIds = await WorkspaceService.createWorkspaceForkGitBranch({ @@ -404,6 +413,9 @@ id: prefixed_id, name, color: colorEnabled && workspaceColor ? workspaceColor : undefined, + // Sent in this first phase too, so a clone the fork would refuse is refused before + // any branch is created. + forked_datatables: forkedDatatables, is_dev_workspace: createAsDevWorkspace, dev_workspace_label: createAsDevWorkspace ? devWorkspaceLabel : undefined, // Send the lock intent in this first phase too so the backend can reject a non-admin's @@ -457,14 +469,6 @@ return } - // Build forked_datatables info from completed clone jobs - const forkedDatatables = forkDatatableSection - ? forkDatatableSection.getCompletedCloneJobs().map((job) => ({ - name: job.name, - new_dbname: job._newDbName - })) - : [] - try { await WorkspaceService.createWorkspaceFork({ workspace: baseWorkspaceId!, diff --git a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte index 8a75027c68..1a08de1351 100644 --- a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte +++ b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte @@ -258,6 +258,12 @@ data table backed by that database can use one. This one is backed by a PostgreSQL resource — grant access on that server directly. + {:else if info?.clone_of} + + This data table is a clone of {info.clone_of.datatable} + in workspace {info.clone_of.workspace_id}, so it takes + its roles from there. You are evaluated as a member of that workspace. + {:else if governing} This data table points at the one in workspace {governing} - export type ForkStep = { - label: string - status: 'pending' | 'running' | 'done' | 'error' - error?: string - } - export type DatatableCloneJob = { name: string resourceType: string behavior: 'schema_only' | 'schema_and_data' - steps: ForkStep[] _newDbName: string - _isInstance: boolean - _sourceWorkspace: string - _targetWorkspace: string - _resourcePath: string } @@ -27,7 +16,6 @@ import Label from '../Label.svelte' import Alert from '../common/alert/Alert.svelte' import ConfirmationModal from '../common/confirmationModal/ConfirmationModal.svelte' - import { Check, X, Loader2 } from 'lucide-svelte' interface Props { // Workspace whose datatables are cloned into the fork (the fork's base). Falls back to the @@ -52,111 +40,46 @@ let cloneModalOpen = $state(false) let currentCloneJob: DatatableCloneJob | undefined = $state(undefined) let cloneQueue: DatatableCloneJob[] = $state([]) - let cloneRunning = $state(false) export function hasDatatables(): boolean { return (allDatatables.current?.length ?? 0) > 0 } export function buildCloneQueue(targetWorkspaceId: string): DatatableCloneJob[] { + // A fork attempt starts here: what an earlier attempt confirmed is not this one's to send. + confirmedJobs = [] return (allDatatables.current ?? []) .filter((dt) => { const behavior = datatableBehaviors[dt.name] ?? 'keep_original' return behavior !== 'keep_original' }) - .map((dt) => { - const behavior = datatableBehaviors[dt.name] as 'schema_only' | 'schema_and_data' - const isInstance = dt.resource_type === 'instance' - const newDbName = `${targetWorkspaceId.replace(/-/g, '_')}__${dt.name}` - - const steps: ForkStep[] = [ - { - label: `CREATE DATABASE "${newDbName}"`, - status: 'pending' - }, - { - label: `pg_dump → pg_import (${behavior === 'schema_only' ? 'schema only' : 'schema + data'})`, - status: 'pending' - } - ] - - return { - name: dt.name, - resourceType: dt.resource_type, - behavior, - steps, - _newDbName: newDbName, - _isInstance: isInstance, - _sourceWorkspace: effectiveSource!, - _targetWorkspace: targetWorkspaceId, - _resourcePath: dt.resource_path - } - }) + .map((dt) => ({ + name: dt.name, + resourceType: dt.resource_type, + behavior: datatableBehaviors[dt.name] as 'schema_only' | 'schema_and_data', + // A dev workspace's id has no `wm-fork-` prefix; its copies are named like a fork's + _newDbName: `wm_fork_${targetWorkspaceId.replace(/^wm-fork-/, '').replace(/-/g, '_')}__${dt.name}` + })) } - let completedJobs: DatatableCloneJob[] = $state([]) + let confirmedJobs: DatatableCloneJob[] = $state([]) + // Each clone is only confirmed here: the fork request makes the copies, and drops them if the + // fork is not created. export function startCloning(queue: DatatableCloneJob[]) { - completedJobs = [] + confirmedJobs = [] cloneQueue = queue currentCloneJob = cloneQueue[0] cloneModalOpen = true } - export function getCompletedCloneJobs(): DatatableCloneJob[] { - return completedJobs - } - - async function executeCloneJob(job: DatatableCloneJob) { - cloneRunning = true - let stepIdx = 0 - - // Step 1: Create the database - job.steps[stepIdx].status = 'running' - try { - await WorkspaceService.createPgDatabase({ - workspace: job._sourceWorkspace, - requestBody: { - source: `datatable://${job.name}`, - target_dbname: job._newDbName - } - }) - job.steps[stepIdx].status = 'done' - } catch (e: any) { - job.steps[stepIdx].status = 'error' - job.steps[stepIdx].error = e?.body ?? e?.message ?? String(e) - cloneRunning = false - return - } - stepIdx++ - - // Step 2: Import data - job.steps[stepIdx].status = 'running' - try { - await WorkspaceService.importPgDatabase({ - workspace: job._sourceWorkspace, - requestBody: { - source: `datatable://${job.name}`, - target: `datatable://${job.name}`, - target_dbname_override: job._newDbName, - fork_behavior: job.behavior - } - }) - job.steps[stepIdx].status = 'done' - } catch (e: any) { - job.steps[stepIdx].status = 'error' - job.steps[stepIdx].error = e?.body ?? e?.message ?? String(e) - cloneRunning = false - return - } - stepIdx++ - - cloneRunning = false + export function getConfirmedCloneJobs(): DatatableCloneJob[] { + return confirmedJobs } function advanceCloneQueue() { if (currentCloneJob) { - completedJobs.push(currentCloneJob) + confirmedJobs.push(currentCloneJob) } const idx = cloneQueue.indexOf(currentCloneJob!) if (idx < cloneQueue.length - 1) { @@ -213,13 +136,9 @@ {#if cloneModalOpen && currentCloneJob} { - await executeCloneJob(currentCloneJob!) - advanceCloneQueue() - }} + onConfirmed={advanceCloneQueue} onCanceled={() => { cloneModalOpen = false currentCloneJob = undefined @@ -241,39 +160,16 @@ {#if currentCloneJob.resourceType === 'instance'}

    - This will run CREATE DATABASE {currentCloneJob.steps[0]?.label.match(/"([^"]+)"/)?.[1] ?? ''} on the Windmill PostgreSQL instance. + Creating the fork will run CREATE DATABASE {currentCloneJob._newDbName} on the Windmill + PostgreSQL instance. A data table under roles keeps its owners and grants in the copy, and its + roles stay decided where they are decided today.

    {:else}

    - This will run CREATE DATABASE on the resource's PostgreSQL server. + Creating the fork will run CREATE DATABASE {currentCloneJob._newDbName} on the resource's + PostgreSQL server.

    {/if} - -
    - {#each currentCloneJob.steps as step} -
    - {#if step.status === 'done'} - - {:else if step.status === 'running'} - - {:else if step.status === 'error'} - - {:else} -
    - {/if} - - {step.label} - -
    - {#if step.error} -

    {step.error}

    - {/if} - {/each} -
    +

    If the fork cannot be created, the copy is dropped.

    {/if} From d2480b4bbdba7aa5e78a0980919ad492534f54c4 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Mon, 21 Sep 2026 09:12:08 +0200 Subject: [PATCH 02/13] fix: open the raw app data table drawer when the workspace has none Selecting the first data table of an empty list passed undefined to the name check, which threw instead of opening the drawer on no data table. Co-Authored-By: Claude Opus 5 (1M context) --- .../lib/components/raw_apps/RawAppDataTableDrawer.svelte | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte b/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte index 8b9c31f452..0122c032b9 100644 --- a/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte @@ -210,11 +210,17 @@ ) } - function selectDatatable(datatable: string, role?: string) { + // `undefined` where the workspace has no data table at all: the drawer opens on none, and + // its tree says so, rather than on a name that does not exist. + function selectDatatable(datatable: string | undefined, role?: string) { // A row clicked under another data table has just set the selection it should open on. openSchemaKey = selectedSchemaKey openTableKey = selectedTableKey selectedDatatable = datatable + if (datatable === undefined) { + selectedRole = undefined + return + } // A data table opens as the role its picked tables were browsed as, else the one the app // already uses it through. connectAs( From eff0adc17409b48ddf11d927d0a0563b4f7bc5cb Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Mon, 21 Sep 2026 09:45:15 +0200 Subject: [PATCH 03/13] fix(datatables): offer cloning a data table under roles where its grants can be replayed The server clones such a data table and replays the source's owners and grants, which only the Enterprise Edition does, so the fork wizard hid both clone options everywhere instead of on a build that cannot replay them. Co-Authored-By: Claude Opus 5 (1M context) --- .../workspaceSettings/ForkDatatableSection.svelte | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte b/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte index 10f4d668b8..9627e4a026 100644 --- a/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte +++ b/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte @@ -9,7 +9,7 @@