diff --git a/AGENTS.md b/AGENTS.md index fd4b60a64b..a57215cde5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,6 +33,9 @@ Open-source platform for internal tools, workflows, API integrations, background administers behind `external_instance` data tables and Ducklake catalogs: its invariants (one lifecycle lock, managed-object markers, the setup gate, per-cluster roles, fork copy ownership) and how to run one locally +- **Data table viewer roles**: `docs/datatable-viewer-roles.md`, on what a connection naming no + role holds: the default role as a floor, where the viewer comes from, why an admin's roles are + not the viewer's, the per-role-set logins and the order their rows and logins are written in - **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow - **Operator write rights**: `docs/operator-write-rights.md` — which `operator_settings` flags are enforced rather than cosmetic, and why a right that is granted-unless-withdrawn needs `Option` diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index ca10ade56a..e82e597fd9 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -5ceb7581a092ad9df1872a2743be2dba550e6129 +d5971fb924d4fdb849e73da6657021902acaacbd diff --git a/backend/migrations/20261012180101_datatable_role_set.down.sql b/backend/migrations/20261012180101_datatable_role_set.down.sql new file mode 100644 index 0000000000..15384edae8 --- /dev/null +++ b/backend/migrations/20261012180101_datatable_role_set.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS datatable_role_set; diff --git a/backend/migrations/20261012180101_datatable_role_set.up.sql b/backend/migrations/20261012180101_datatable_role_set.up.sql new file mode 100644 index 0000000000..0093837d3b --- /dev/null +++ b/backend/migrations/20261012180101_datatable_role_set.up.sql @@ -0,0 +1,21 @@ +-- The logins behind a data table connection made as several roles at once: one Postgres login per +-- distinct set of data table roles, a member of each. Which roles a login holds is its +-- memberships on the cluster, and its name is derived from their catalog ids, so a row only has to +-- carry the credential. +-- +-- A row is written after its login exists and removed before the login is dropped, so a row always +-- names a login that can authenticate with `pwd`. +CREATE TABLE datatable_role_set ( + cluster VARCHAR(20) NOT NULL CHECK (cluster IN ('instance', 'external_instance')), + name VARCHAR(63) NOT NULL, + -- Generated by Windmill, never entered by anyone, and never leaves the server. + pwd TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + -- Recorded at most once a day. A login unused for long enough is dropped, and made again if + -- its set of roles is asked for later. + last_used_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (cluster, name) +); + +GRANT ALL ON datatable_role_set TO windmill_user; +GRANT ALL ON datatable_role_set TO windmill_admin; diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 24f9d8221c..23ab2eaed1 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -4449,6 +4449,24 @@ pub async fn monitor_db( } }; + // Drop the data table role set logins nobody has connected with for a month. Every ~40 min, + // spawned for the same reason as the credential maintenance above: it opens a connection per + // managed database for each login it drops. + let datatable_role_set_sweep_f = async { + if server_mode && iteration.is_some() && iteration.as_ref().unwrap().should_run(240) { + if let Some(db) = conn.as_sql() { + let db = db.clone(); + tokio::spawn(async move { + if let Err(e) = + windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await + { + tracing::error!("Error sweeping unused data table role set logins: {e}"); + } + }); + } + } + }; + // Re-encrypt the legacy-format secret variables once new values are written in AES-256-GCM. // Every ~5 min until one pass has gone through every workspace; spawned so a large instance // does not hold up the join below. @@ -4538,6 +4556,7 @@ pub async fn monitor_db( cleanup_scheduled_job_deletions_f, git_auto_pull_f, git_credential_maintenance_f, + datatable_role_set_sweep_f, ai_session_retention_f, legacy_secret_upgrade_f, pipeline_freshness_watchdog_f, diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index 9800d97841..023d9f20ed 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -1847,3 +1847,222 @@ async fn cleanup_waits_out_a_save_racing_it_for_an_instance_database( assert_eq!(users, vec!["test-workspace".to_string()]); Ok(()) } + +#[cfg(all(feature = "private", feature = "enterprise"))] +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_connection_naming_no_role_holds_the_default_one_and_the_viewers( + db: Pool, +) -> anyhow::Result<()> { + use windmill_common::workspaces::{get_datatable_resource_from_db, DatatableAccess}; + + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let suffix: String = uuid::Uuid::new_v4().simple().to_string()[..8].to_string(); + let names = [ + format!("wmtest_floor_{suffix}"), + format!("wmtest_emea_{suffix}"), + format!("wmtest_amer_{suffix}"), + ]; + let roles_url = format!("http://localhost:{port}/api/settings/datatable_roles"); + const MEMBER_LOGINS: &str = "SELECT m.rolname::text FROM pg_auth_members am + JOIN pg_roles r ON r.oid = am.roleid JOIN pg_roles m ON m.oid = am.member + WHERE r.rolname = $1 AND m.rolname LIKE 'wm\\_set\\_%'"; + + // The job is the app publisher's in every case: only the viewer the server recorded differs. + let connect = |role: Option<&'static str>, viewer: Option<&'static str>| { + get_datatable_resource_from_db( + &db, + "test-workspace", + "main", + role, + DatatableAccess::PermissionedAs { + permissioned_as: "u/test-user", + email: "test@windmill.dev", + end_user_email: viewer, + }, + ) + }; + let set_permissions = |default_role: String, roles: Value| { + sqlx::query( + "UPDATE workspace_settings + SET datatable = jsonb_set(datatable, '{datatables,main,permissions}', $1) + WHERE workspace_id = 'test-workspace'", + ) + .bind(json!({ "default_role": default_role, "roles": roles })) + .execute(&db) + }; + + let outcome = async { + let mut ids = vec![]; + for name in &names { + let created: Value = authed(client().post(&roles_url), "SECRET_TOKEN") + .json(&json!({ "name": name })) + .send() + .await? + .error_for_status()? + .json() + .await?; + ids.push(created["id"].as_str().unwrap().to_string()); + } + // `test-user-2` is named by both regional roles, one of them through a group, and by + // `admin`; `test-user-3` by one; the workspace admin `test-user` by neither. + let roles = json!({ + "admin": {"tenants": ["u/test-user-2"]}, + (ids[0].as_str()): {"tenants": ["*"]}, + (ids[1].as_str()): {"tenants": ["u/test-user-2", "u/test-user-3"]}, + (ids[2].as_str()): {"tenants": ["g/analysts"]}, + }); + set_permissions(ids[0].clone(), roles.clone()).await?; + sqlx::query( + "INSERT INTO usr_to_group (workspace_id, usr, group_) + VALUES ('test-workspace', 'test-user-2', 'analysts')", + ) + .execute(&db) + .await?; + let member_of = |login: String| async { + let mut roles: Vec = sqlx::query_scalar( + "SELECT r.rolname::text FROM pg_auth_members am + JOIN pg_roles r ON r.oid = am.roleid JOIN pg_roles m ON m.oid = am.member + WHERE m.rolname = $1", + ) + .bind(login) + .fetch_all(&db) + .await?; + roles.sort(); + Ok::<_, anyhow::Error>(roles) + }; + + let all = connect(None, Some("test2@windmill.dev")).await?; + let login = all["user"].as_str().unwrap().to_string(); + assert!(login.starts_with("wm_set_"), "{login}"); + let mut expected = names.to_vec(); + expected.sort(); + assert_eq!( + member_of(login.clone()).await?, + expected, + "the login holds the default role and the viewer's, and not `admin`" + ); + assert_eq!(connect(None, Some("test2@windmill.dev")).await?, all); + + let two = connect(None, Some("test3@windmill.dev")).await?; + let mut expected = names[..2].to_vec(); + expected.sort(); + assert_eq!( + member_of(two["user"].as_str().unwrap().to_string()).await?, + expected + ); + + // An admin may use every role, the one owning the tables included. As a viewer they add + // what names them, which here is nothing; nor does a job with no viewer recorded. + for viewer in [Some("test@windmill.dev"), None] { + assert_eq!(connect(None, viewer).await?["user"], names[0].as_str()); + } + // A named role is that role alone, whoever is viewing. + let named: &'static str = Box::leak(names[1].clone().into_boxed_str()); + assert_eq!( + connect(Some(named), Some("test2@windmill.dev")).await?["user"], + names[1].as_str() + ); + // A default of `admin` is the connection it always was: the viewer's roles are not added. + set_permissions("admin".to_string(), roles.clone()).await?; + let admin = connect(None, Some("test2@windmill.dev")).await?; + assert_eq!(admin, connect(Some("admin"), None).await?); + assert!(!admin["user"].as_str().unwrap().starts_with("wm_")); + + // With no default role the viewer's roles are all there is, the one open to everyone + // included, and a connection left with none is refused rather than resolved as anything. + set_permissions("$none".to_string(), roles.clone()).await?; + assert_eq!( + connect(None, Some("test3@windmill.dev")).await?, + two, + "no default changes nothing for a viewer the roles name" + ); + assert!(connect(None, None).await.is_err()); + // The same holds for a default the job's own identity is not a tenant of. + let mut closed = roles.clone(); + closed[ids[0].as_str()] = json!({"tenants": []}); + set_permissions(ids[0].clone(), closed).await?; + let as_member = |viewer: Option<&'static str>| { + get_datatable_resource_from_db( + &db, + "test-workspace", + "main", + None, + DatatableAccess::PermissionedAs { + permissioned_as: "u/test-user-3", + email: "test3@windmill.dev", + end_user_email: viewer, + }, + ) + }; + assert_eq!( + as_member(Some("test3@windmill.dev")).await?["user"], + names[1].as_str() + ); + assert!(as_member(None).await.is_err()); + set_permissions(ids[0].clone(), roles).await?; + + // A set login nobody connects with any more is dropped, a recent one is kept, and a + // dropped one is made again for the next viewer who needs it. + let exists = || async { + sqlx::query_scalar::<_, bool>( + "SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = $1)", + ) + .bind(&login) + .fetch_one(&db) + .await + }; + windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await?; + assert!(exists().await?, "a set login in use was swept"); + sqlx::query("UPDATE datatable_role_set SET last_used_at = now() - interval '31 days'") + .execute(&db) + .await?; + windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await?; + assert!(!exists().await?, "an unused set login was kept"); + assert_eq!( + connect(None, Some("test2@windmill.dev")).await?["user"], + login.as_str() + ); + + // A disabled role must not stay reachable through a login that is a member of it. + authed( + client().post(format!("{roles_url}/{}", ids[2])), + "SECRET_TOKEN", + ) + .json(&json!({ "enabled": false })) + .send() + .await? + .error_for_status()?; + let can_login: bool = + sqlx::query_scalar("SELECT rolcanlogin FROM pg_roles WHERE rolname = $1") + .bind(&login) + .fetch_one(&db) + .await?; + assert!(!can_login, "{login} still logs in with a disabled role"); + assert_eq!( + connect(None, Some("test2@windmill.dev")).await?["user"], + two["user"] + ); + Ok::<_, anyhow::Error>(()) + } + .await; + + // Roles are cluster-wide, so they outlive this test's throwaway database. + for name in &names { + let logins: Vec = sqlx::query_scalar(MEMBER_LOGINS) + .bind(name) + .fetch_all(&db) + .await + .unwrap_or_default(); + for login in logins { + let _ = sqlx::query(&format!("DROP ROLE IF EXISTS \"{login}\"")) + .execute(&db) + .await; + } + let _ = sqlx::query(&format!("DROP ROLE IF EXISTS \"{name}\"")) + .execute(&db) + .await; + } + outcome +} diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index ae01930182..433d7c00be 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -5568,6 +5568,9 @@ paths: type: boolean default_role: type: string + description: >- + The id of one of `roles`, `admin` (the default), or `$none` for no default + role: a connection that names no role then holds its viewer's roles only. roles: type: array items: @@ -34782,6 +34785,7 @@ components: type: boolean default_role: type: string + description: A role id, `admin`, or `$none` when the data table has no default role. roles: type: array items: diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 29268d3cb2..2b93c6daeb 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -8982,6 +8982,7 @@ async fn run_preview_script( check_tag_available_for_workspace(&db, &w_id, &tag, &push_args, &authed).await?; let scope_ceiling = windmill_api_auth::caller_scope_ceiling(&db, &authed).await?; + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let (uuid, tx) = push( &db, tx, @@ -9029,7 +9030,7 @@ async fn run_preview_script( None, Some(&authed.clone().into()), false, - None, + end_user_email, authed.trigger_or_fallback(None), None, scope_ceiling.as_deref(), @@ -9068,6 +9069,7 @@ async fn run_inline_preview_script( if let Some(job_id) = job_id { register_potential_assets_on_inline_execution(job_id, &w_id, &preview); } + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let utils = get_worker_internal_server_inline_utils()?; let result = utils.run_inline_preview_script.as_ref()(RunInlinePreviewScriptFnParams { content: preview.content, @@ -9078,6 +9080,7 @@ async fn run_inline_preview_script( created_by: authed.display_username().to_string(), permissioned_as: username_to_permissioned_as(&authed.username), permissioned_as_email: authed.email.clone(), + end_user_email, lang: preview.language, job_dir: "".to_string(), worker_name: "".to_string(), @@ -9217,6 +9220,7 @@ async fn run_inline_script_inner( .to_string(), )); } + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let utils = get_worker_internal_server_inline_utils()?; let authed_owned: windmill_common::db::Authed = authed.clone().into(); let result = utils.run_inline_script.as_ref()(RunInlineScriptFnParams { @@ -9228,6 +9232,7 @@ async fn run_inline_script_inner( created_by: authed.display_username().to_string(), permissioned_as: username_to_permissioned_as(&authed.username), permissioned_as_email: authed.email.clone(), + end_user_email, job_dir: "".to_string(), worker_name: "".to_string(), worker_dir: "".to_string(), @@ -9404,6 +9409,7 @@ async fn run_bundle_preview_script( // tracing::info!("is_tar 1: {is_tar}"); // hmap.insert("") let scope_ceiling = windmill_api_auth::caller_scope_ceiling(&db, &authed).await?; + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let (uuid, ntx) = push( &db, ltx, @@ -9447,7 +9453,7 @@ async fn run_bundle_preview_script( None, Some(&authed.clone().into()), false, - None, + end_user_email, authed.trigger_or_fallback(None), None, scope_ceiling.as_deref(), @@ -10151,6 +10157,7 @@ async fn run_preview_flow_job( .await?; let scope_ceiling = windmill_api_auth::caller_scope_ceiling(&db, &authed).await?; + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let (uuid, mut tx) = push( &db, tx, @@ -10182,7 +10189,7 @@ async fn run_preview_flow_job( None, Some(&authed.clone().into()), false, - None, + end_user_email, authed.trigger_or_fallback(None), None, scope_ceiling.as_deref(), diff --git a/backend/windmill-common/src/datatable_roles.rs b/backend/windmill-common/src/datatable_roles.rs index e7e490a6a4..522327040d 100644 --- a/backend/windmill-common/src/datatable_roles.rs +++ b/backend/windmill-common/src/datatable_roles.rs @@ -72,6 +72,11 @@ impl DatatableRoleCluster { /// created, renamed or dropped. pub const ADMIN_DATATABLE_ROLE: &str = "admin"; +/// Stands in a data table's `default_role` for "none": a connection that names no role then holds +/// its viewer's roles only, and is refused without any. `$` is outside both the role-name charset +/// and the catalog ids, and a server predating it refuses it as a role the data table lacks. +pub const NO_DEFAULT_DATATABLE_ROLE: &str = "$none"; + /// The login the admin connection uses, and the role every created role is granted to — that /// membership is what later lets it `ALTER ... OWNER TO` a role and drop it. pub const CUSTOM_INSTANCE_USER: &str = "custom_instance_user"; @@ -121,8 +126,13 @@ fn is_reserved_role_name(name: &str) -> bool { || lower.starts_with("pg_") || lower.starts_with("windmill_") || lower.starts_with("custom_instance_") + || lower.starts_with(ROLE_SET_LOGIN_PREFIX) } +/// Prefix of the logins Windmill creates for a connection made as several roles at once. Reserved, +/// so no data table role can be created under, renamed to or taken over at such a name. +pub const ROLE_SET_LOGIN_PREFIX: &str = "wm_set_"; + /// The charset is what makes every downstream interpolation safe: the name reaches Postgres as a /// quoted identifier, a `-- role ` annotation, and a `?role=` query parameter. pub fn validate_role_name(name: &str) -> Result<()> { @@ -414,6 +424,13 @@ pub async fn drop_datatable_role( crate::datatable_roles_oss::drop_datatable_role(db, tx, cluster, name).await } +/// Drop the logins made for a connection as several roles at once that nobody has connected with +/// for a month, on every cluster. They are made again on demand, so this only bounds how many +/// exist. Safe to call from any server at any time: it serializes on the role catalog lock. +pub async fn sweep_unused_role_set_logins(db: &DB) -> Result<()> { + crate::datatable_roles_oss::sweep_unused_role_set_logins(db).await +} + #[cfg(test)] mod tests { use super::*; diff --git a/backend/windmill-common/src/datatable_roles_oss.rs b/backend/windmill-common/src/datatable_roles_oss.rs index 33d711e7f9..739f08d60b 100644 --- a/backend/windmill-common/src/datatable_roles_oss.rs +++ b/backend/windmill-common/src/datatable_roles_oss.rs @@ -29,7 +29,8 @@ pub(crate) use crate::datatable_roles_ee::{ ensure_instance_db_grant_options_unchecked, forget_datatable_role_everywhere, insert_role_catalog_entry, read_role_catalog, read_role_catalog_tx, registered_instance_databases, rename_datatable_role, resolve_datatable_role_connection, - role_cluster, set_datatable_role_login, update_role_catalog_entry, + role_cluster, set_datatable_role_login, sweep_unused_role_set_logins, + update_role_catalog_entry, }; #[cfg(not(all(feature = "private", feature = "enterprise")))] @@ -235,6 +236,11 @@ mod ce { } } + /// No roles, so no set login to sweep. + pub(crate) async fn sweep_unused_role_set_logins(_db: &DB) -> Result<()> { + Ok(()) + } + pub(crate) async fn forget_datatable_role_everywhere( _tx: &mut Tx<'_>, _role_id: &str, diff --git a/backend/windmill-common/src/jobs.rs b/backend/windmill-common/src/jobs.rs index 1a314d34c0..75670fe874 100644 --- a/backend/windmill-common/src/jobs.rs +++ b/backend/windmill-common/src/jobs.rs @@ -474,6 +474,8 @@ pub struct RunInlinePreviewScriptFnParams { pub created_by: String, pub permissioned_as: String, pub permissioned_as_email: String, + /// The caller's recorded end user: `job_perms.end_user_email` of the calling job. + pub end_user_email: Option, pub base_internal_url: String, pub worker_name: String, pub conn: crate::worker::Connection, @@ -496,6 +498,8 @@ pub struct RunInlineScriptFnParams { pub created_by: String, pub permissioned_as: String, pub permissioned_as_email: String, + /// The caller's recorded end user: `job_perms.end_user_email` of the calling job. + pub end_user_email: Option, pub base_internal_url: String, pub worker_name: String, pub conn: crate::worker::Connection, diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index a02bd7f44e..3653a224f1 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1564,7 +1564,8 @@ pub struct DataTableReference { /// The access decision for one data table: which role a caller gets, and who may ask for each. #[derive(Deserialize, Serialize, Debug, Clone, Default)] pub struct DataTablePermissions { - /// A role id from the instance catalog, or `admin`. Absent = `admin`. + /// A role id from the instance catalog, `admin`, or + /// [`crate::datatable_roles::NO_DEFAULT_DATATABLE_ROLE`]. Absent = `admin`. #[serde(default, skip_serializing_if = "Option::is_none")] pub default_role: Option, /// Keyed by instance role id, plus the reserved `admin` key. A role absent from this map @@ -2161,9 +2162,13 @@ pub enum DatatableAccess<'a> { PermissionedAs { permissioned_as: &'a str, email: &'a str, + /// The job's viewer: MUST be what the server recorded in `job_perms.end_user_email`, + /// never an argument or anything else the job or its caller can set. + end_user_email: Option<&'a str>, }, - /// A job identified by id; its owner is read from `v2_job`. For agent workers and anything - /// else that authenticates as infrastructure rather than as the job's user. + /// A job identified by id; its owner is read from `v2_job` and its viewer from `job_perms`. + /// For agent workers and anything else that authenticates as infrastructure rather than as + /// the job's user. Job(uuid::Uuid), /// No identity established. Unpermissioned data tables resolve as before; permissioned ones /// are refused, so a caller predating this feature fails closed. @@ -2211,6 +2216,13 @@ pub async fn can_use_datatable_role_in_governing_workspace( /// /// The resolved role **logs in as itself**. Never `SET ROLE`: a script could `RESET ROLE` its way /// back to admin. +/// +/// A default role other than `admin` is a floor rather than the whole connection: a resolution +/// naming no role holds it when the job's own identity may use it, plus the roles whose tenant +/// lists name the job's viewer, as one login that is a member of each. With no role at all it is +/// refused. Being an admin adds none of the viewer's and `admin` is never among them, so a viewer +/// cannot be handed the connection that owns the tables and bypasses row-level security. A named +/// role is that role alone. pub async fn get_datatable_resource_from_db( db: &DB, w_id: &str, diff --git a/backend/windmill-worker/src/common.rs b/backend/windmill-worker/src/common.rs index 6f6c48baec..6088fb4960 100644 --- a/backend/windmill-worker/src/common.rs +++ b/backend/windmill-worker/src/common.rs @@ -1726,6 +1726,23 @@ pub async fn cached_result_path( hasher.update(b"wac_step:"); hasher.update(step_key.as_bytes()); } + // What a run returns can depend on who it ran for (`WM_END_USER_EMAIL`, a data table used + // as the viewer), so a result cached for one viewer is never served to another. Read from + // `job_perms`: a flow job re-fetched between steps does not carry it. + let end_user: Option> = sqlx::query_scalar( + "SELECT end_user_email FROM job_perms WHERE job_id = $1 AND workspace_id = $2", + ) + .bind(job.id) + .bind(&job.workspace_id) + .fetch_optional(db) + .await?; + // Length-prefixed: the arguments follow undelimited, and the end of one viewer's address + // must not read as the start of another run's first argument name. + if let Some(end_user) = end_user.flatten() { + hasher.update(b"end_user:"); + hasher.update((end_user.len() as u64).to_le_bytes()); + hasher.update(end_user.as_bytes()); + } hash_args( db, client, @@ -2475,6 +2492,52 @@ mod tests { } } + // A cache hit returns before the job runs, so the key is all that keeps a result computed + // for one viewer (a data table used as the viewer, `WM_END_USER_EMAIL`) from another. + #[sqlx::test(migrations = "../migrations")] + async fn a_cached_result_is_keyed_by_the_recorded_viewer(pool: sqlx::PgPool) { + let client = test_client(); + + // The last two differ only in where the viewer's address ends and the first argument + // name starts. + let mut paths = vec![]; + for (viewer, arg) in [ + (Some("a@windmill.dev"), "mode"), + (Some("a@windmill.dev"), "mode"), + (Some("b@windmill.dev"), "mode"), + (None, "mode"), + (Some("a@windmill.co"), "mode"), + (Some("a@windmill.com"), "ode"), + ] { + let args = HashMap::from([(arg.to_string(), to_raw_value(&"all"))]); + let job = MiniPulledJob { + id: uuid::Uuid::new_v4(), + args: Some(sqlx::types::Json(args)), + ..test_job() + }; + sqlx::query( + "INSERT INTO job_perms (job_id, email, username, is_admin, is_operator, + workspace_id, groups, folders, end_user_email) + VALUES ($1, 'publisher@windmill.dev', 'publisher', false, false, 'test', '{}', + '{}', $2)", + ) + .bind(job.id) + .bind(viewer) + .execute(&pool) + .await + .unwrap(); + paths.push(cached_result_path(&pool, &client, &job, None).await.unwrap()); + } + assert_eq!( + paths[0], paths[1], + "one viewer shares their own cached result" + ); + assert_ne!(paths[0], paths[2], "two viewers share a cached result"); + assert_ne!(paths[0], paths[3]); + assert_ne!(paths[2], paths[3]); + assert_ne!(paths[4], paths[5], "an address runs into the argument names"); + } + #[tokio::test] async fn test_transform_array_over_1000_passthrough() { let db_url = std::env::var("DATABASE_URL") diff --git a/backend/windmill-worker/src/duckdb_executor.rs b/backend/windmill-worker/src/duckdb_executor.rs index b8c850a2f4..58e47b9332 100644 --- a/backend/windmill-worker/src/duckdb_executor.rs +++ b/backend/windmill-worker/src/duckdb_executor.rs @@ -2758,6 +2758,7 @@ async fn transform_attach_datatable( DatatableAccess::PermissionedAs { permissioned_as: &job.permissioned_as, email: &job.permissioned_as_email, + end_user_email: job.permissioned_as_end_user_email.as_deref(), }, ) .await? diff --git a/backend/windmill-worker/src/pg_executor.rs b/backend/windmill-worker/src/pg_executor.rs index aac7d549db..6309ba8c95 100644 --- a/backend/windmill-worker/src/pg_executor.rs +++ b/backend/windmill-worker/src/pg_executor.rs @@ -1229,6 +1229,7 @@ pub async fn do_postgresql( DatatableAccess::PermissionedAs { permissioned_as: &job.permissioned_as, email: &job.permissioned_as_email, + end_user_email: job.permissioned_as_end_user_email.as_deref(), }, ) .await? diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index 7c3c3971fa..3aa377c904 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -7253,7 +7253,7 @@ pub fn init_worker_internal_server_inline_utils( base_internal_url, killpill_rx: Arc::new(killpill_rx), run_inline_preview_script: Arc::new(|params| { - let job = MiniPulledJob::new_inline( + let mut job = MiniPulledJob::new_inline( params.workspace_id, params.args, params.created_by, @@ -7265,6 +7265,7 @@ pub fn init_worker_internal_server_inline_utils( "inline_preview".to_string(), Some(params.lang), ); + job.permissioned_as_end_user_email = params.end_user_email; Box::pin(async move { let mut mem_peak: i32 = -1; let mut canceled_by: Option = None; @@ -7314,7 +7315,7 @@ pub fn init_worker_internal_server_inline_utils( let content_info = get_script_content_by_hash(&script_hash, ¶ms.workspace_id, ¶ms.conn) .await?; - let job = MiniPulledJob::new_inline( + let mut job = MiniPulledJob::new_inline( params.workspace_id, params.args, params.created_by, @@ -7326,6 +7327,7 @@ pub fn init_worker_internal_server_inline_utils( "inline_run".to_string(), content_info.language, ); + job.permissioned_as_end_user_email = params.end_user_email; let mut mem_peak: i32 = -1; let mut canceled_by: Option = None; let mut column_order: Option> = None; diff --git a/cli/src/commands/app/app.ts b/cli/src/commands/app/app.ts index 470f0f4b11..0060c2c422 100644 --- a/cli/src/commands/app/app.ts +++ b/cli/src/commands/app/app.ts @@ -115,6 +115,14 @@ export function replaceInlineScripts( } return []; } +/** `parent_version` is the editor's stale-draft base: a version id that changes on + * every deploy and means nothing in another workspace, so it is kept out of the + * tracked file and out of what a push deploys. */ +export function stripEditorState(app: any) { + if (app?.value && typeof app.value == "object") { + delete app.value.parent_version; + } +} export function isExecutionModeAnonymous(app: any) { return app?.["policy"]?.["execution_mode"] == "anonymous"; } @@ -294,6 +302,7 @@ export async function pushApp( } const path = localPath + "app.yaml"; const localApp = (await yamlParseFile(path)) as AppFile; + stripEditorState(localApp); const isolation = statedIsolation(localApp, false); replaceInlineScripts(localApp.value, localPath, true); diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index b9e373abad..5ba8888332 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -149,7 +149,11 @@ import { extractCurrentMapping, } from "../../../windmill-utils-internal/src/inline-scripts/extractor.ts"; import { generateFlowLockInternal } from "../flow/flow_metadata.ts"; -import { markAccessFromPolicy, markIsolationFromPolicy } from "../app/app.ts"; +import { + markAccessFromPolicy, + markIsolationFromPolicy, + stripEditorState, +} from "../app/app.ts"; import { APP_BACKEND_FOLDER, generateAppLocksInternal, @@ -1329,6 +1333,7 @@ export function ZipFSElement( log.error(`Failed to parse app.yaml at path: ${p}`); throw error; } + stripEditorState(app); let inlineScripts; try { inlineScripts = extractInlineScriptsForApps( @@ -2749,12 +2754,18 @@ export async function compareDynFSElement( } return o; } else { + let o: any; try { - return yamlParseContent(k, v); + o = yamlParseContent(k, v); } catch (error) { log.error(`Failed to parse YAML content at path: ${k}`); throw error; } + // A file tracked before the field was dropped still carries it. + if (path.basename(k) == "app.yaml") { + stripEditorState(o); + } + return o; } } diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index ab4ef31c3e..d90a401260 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -6557,7 +6557,7 @@ data: - \`/\` — Specific table in public schema - \`/:
\` — Table in specific schema -**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. \`roles\` records the role the app uses each datatable through; the app's code must pass the same role: \`wmill.datatable('main', { role: 'analyst' })\` in TypeScript, \`wmill.datatable('main', role='analyst')\` in Python. A datatable without an entry is used as its default role. +**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. \`roles\` records the role the app uses each datatable through; the app's code must pass the same role: \`wmill.datatable('main', { role: 'analyst' })\` in TypeScript, \`wmill.datatable('main', role='analyst')\` in Python. A datatable without an entry is used as its default role, if it has one, plus the roles of whoever is using the app unless that default is \`admin\`. ## SQL Migrations (sql_to_apply/) @@ -6863,6 +6863,7 @@ Data tables are PostgreSQL databases managed by Windmill. Backend runnables quer 3. **No DDL inside runnables**: runnables only read and write rows (SELECT, INSERT, UPDATE, DELETE) on existing tables. Never CREATE, ALTER or DROP a table from a runnable. 4. **Qualify table names**: an unqualified name means the \`public\` schema, so write every other table as \`schema.table\`, in table creation and queries alike. The app's \`data\` config sets the default datatable and schema its tables go in; use them consistently across runnables. 5. **Pass the role**: when the app's \`data.roles\` gives a data table a role, every \`wmill.datatable\` call on it passes that role (\`wmill.datatable('main', { role: 'analyst' })\` in TypeScript, \`wmill.datatable('main', role='analyst')\` in Python). The role only reaches what it was granted, so a query outside it fails with \`permission denied\`. +6. **Per-viewer rows**: a data table with no entry in \`data.roles\` is called with no role. Unless its default role is \`admin\`, the connection then holds the default role, if it has one, plus every role the person using the app is a tenant of, and is refused when that is none. Row-level security policies written as \`pg_has_role(current_user, '', 'member')\` return that viewer's rows with nothing added to the runnable. A role pins the connection to that one role for every viewer, so where rows should differ per viewer give the data table no \`data.roles\` entry. \`wmill datatable list\` lists the workspace's data tables. Create or change tables with a migration in \`sql_to_apply/\` (see "SQL Migrations" above), then add them to \`data.tables\` in \`raw_app.yaml\`. diff --git a/docs/datatable-viewer-roles.md b/docs/datatable-viewer-roles.md new file mode 100644 index 0000000000..9855e4f97a --- /dev/null +++ b/docs/datatable-viewer-roles.md @@ -0,0 +1,93 @@ +# Data table connections and the viewer's roles + +A data table under roles (Enterprise Edition) resolves a connection that names a role to that +role alone. A connection that names none is resolved from the data table's default role: + +| Default role | A connection naming no role holds | +|---|---| +| `admin` (until changed) | `admin`, as before viewer roles existed | +| another role | that role when the job's own identity may use it, plus **every role the job's viewer is a tenant of** | +| none (`NO_DEFAULT_DATATABLE_ROLE`) | the viewer's roles only | + +A connection left with no role at all is refused. Postgres row-level security can then answer +per viewer from a backend runnable that runs as the app's publisher, with nothing written in the +runnable. + +Policies test membership, which is true for a role's own login too, so one policy serves a named +connection and an unnamed one: + +```sql +CREATE POLICY emea ON orders FOR SELECT + USING (region = 'emea' AND pg_has_role(current_user, 'sales_emea', 'member')); +``` + +A policy names its role as text, so renaming the role breaks it. + +## Code + +| Where | What | +|---|---| +| `windmill-common/src/workspaces.rs` | `DatatableAccess`, `get_datatable_resource_from_db` | +| `windmill-common/src/datatable_roles_ee.rs` | `resolve_datatable_role_connection`, `viewer_role_ids`, `ensure_role_set_login`, `retire_role_set_logins_of`, `sweep_unused_role_set_logins` | +| `windmill-api-jobs/src/execution.rs` | `run_end_user_email`, where a run's viewer is recorded | +| `frontend/.../workspaceSettings/DataTablePermissionsButton.svelte` | the roles drawer, which warns while the default is `admin` | + +## What holds it together + +- **A default of `admin` is not a floor.** `admin` owns the tables and bypasses row-level + security, so nothing could be added to it, and it is the default until someone changes it. + Such a data table resolves exactly as it did before viewer roles existed. Nothing ever sets a + default back to `admin`: removing or deleting the default role leaves none. +- **The default role is tenant-checked against the job's own identity**, the publisher for a + publisher-mode app, and held only when that passes. The viewer's roles are added either way. +- **No role at all is a refusal, never an empty answer.** There is no login that would read + nothing: what a role sees is up to each table's grants and policies. A job without a recorded + viewer (anonymous app viewer, schedule, trigger, a tool job started by an AI agent step), or + whose viewer is not a member of the governing workspace, therefore needs a default it may use + or a named role. +- **A set login owns what it creates.** Postgres gives a new object to `current_user`, and + default privileges go by the creating role, so a table created over a connection that holds + several roles belongs to that set's login: no other role or viewer reaches it, and it passes to + `custom_instance_user` when the login is swept. Unnamed connections are for rows. To create an + object, name a role or run `SET ROLE ` first, which a set login may do for any role it + holds. Migrations are not affected: one that declares no role runs as `admin`. +- **The viewer is `job_perms.end_user_email`, and nothing else.** The server records it when the + run starts: the session behind an app request, the caller of a direct run or preview, and for a + job started with another job's token, that job's viewer. No argument reaches it. +- **A cached result belongs to one viewer.** `cached_result_path` hashes the run's recorded viewer + into the key, since a cache hit returns before any connection is resolved. Runs with no viewer + recorded keep sharing one entry. +- **Only a tenant list naming the viewer counts.** A workspace admin may *use* every role, + `admin` included. The viewer's roles are those whose tenants name them (user, group, folder, + `*`), and never `admin`. +- **One role is that role's login. Several are a login per distinct set.** `wm_set_` is a + member of exactly those roles, with `INHERIT`; the hash is over their catalog ids, so a rename + moves nothing and a change in someone's groups resolves them to another login instead of + editing one. Its privileges are its memberships, so `SET ROLE` only narrows and `RESET ROLE` + comes back to it. The prefix is reserved in `validate_role_name`. +- **Resolving stays cheap where no viewer could add anything.** The viewer is only looked up when + some role other than the default and `admin` has tenants on that data table. +- **A `datatable_role_set` row with a password names a login that can authenticate with it.** The + row is written after the login exists and removed, in its own commit, before the login is + dropped or made `NOLOGIN`. A login without a row is taken back with a new password on next use, + and so is one whose row has an empty password: that is what a sweep leaves when it could + neither drop a login nor let it back in, so the next sweep still finds it. +- **Set logins are bounded by use, not by the number of role combinations.** One exists only for + a set some viewer actually connected with, so there are at most as many as (viewer, data table) + pairs. `last_used_at` is recorded at most once a day, and `sweep_unused_role_set_logins`, run + from the monitor loop, drops those unused for 30 days; a dropped one is made again on demand. + The sweep goes by this instance's rows, never by the cluster's `wm_set_*` roles: a cluster can + serve several Windmill instances. +- **Disabling or dropping a role retires every set login holding it.** `NOLOGIN` on a role does + not reach a login that is a member of it. +- **A worker or server that predates this connects as the default role alone**, so a mixed fleet + shows a viewer fewer rows, never more. It refuses a data table with no default role, whose + marker it reads as a role the data table lacks. + +## Trust + +A job that names no role runs its author's code with the viewer's reach into the data table, the +same trust as an app in viewer execution mode or a script someone else wrote and you run. Whoever +can deploy a publisher-mode app, or a script that others run, can therefore read through the +non-`admin` roles of the people who use it. Naming a role in the code is how an author pins the +connection to one role instead. diff --git a/frontend/src/lib/components/DBManager.svelte b/frontend/src/lib/components/DBManager.svelte index 837569c575..5b34c66bc5 100644 --- a/frontend/src/lib/components/DBManager.svelte +++ b/frontend/src/lib/components/DBManager.svelte @@ -45,7 +45,12 @@ import { favoriteManager } from './sidebar/FavoriteMenu.svelte' import DatatableRoleBadge from './DatatableRoleBadge.svelte' import type { AclTarget, Asset, DataTableTables } from '$lib/gen' - import { ADMIN_DATATABLE_ROLE, datatableNameTakesRole, type DatatableRowAction } from './dbTypes' + import { + ADMIN_DATATABLE_ROLE, + datatableNameTakesRole, + namedDefaultRole, + type DatatableRowAction + } from './dbTypes' import TextInput from './text_input/TextInput.svelte' import Checkbox from './common/checkbox/Checkbox.svelte' import DbSchemaDiagram from './dbdiagram/DbSchemaDiagram.svelte' @@ -417,7 +422,10 @@ if (!entry?.permissioned || !datatableNameTakesRole(datatable)) return undefined const roles = entry.usable_roles if (roles.length === 0) return { kind: 'none' } - const role = (datatable === currentDatatable ? currentRole : undefined) ?? entry.default_role + const role = + (datatable === currentDatatable ? currentRole : undefined) ?? + namedDefaultRole(entry.default_role) ?? + roles[0] if (roles.length === 1 && roles[0] === ADMIN_DATATABLE_ROLE && role === ADMIN_DATATABLE_ROLE) { return undefined } diff --git a/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte b/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte index f07d596baa..9fd083a75d 100644 --- a/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte +++ b/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte @@ -284,13 +284,21 @@ saveDrawerOpen = false } + // `parent_version` is the draft's stale-check base. Deployed, it would put a version + // id that changes on every deploy into git-synced files and version diffs. + function withoutDraftBase(a: T): T { + if (!a) return a + const { parent_version: _pv, ...rest } = a + return rest as T + } + async function createApp(path: string) { policy = await updatePolicy($app, policy) try { await AppService.createApp({ workspace: $workspaceStore!, requestBody: { - value: $app, + value: withoutDraftBase($app), path, summary: $summary, policy, @@ -349,7 +357,7 @@ orderedJsonStringify( replaceFalseWithUndefined({ summary: $summary, - value: $app, + value: withoutDraftBase($app), path: newEditedPath || savedApp.path, policy, custom_path: customPath, @@ -392,6 +400,7 @@ // Strip off extra information deployedValue = replaceFalseWithUndefined({ ...deployedApp, + value: withoutDraftBase(deployedApp.value as App), id: undefined, created_at: undefined, created_by: undefined, @@ -406,7 +415,7 @@ workspace: $workspaceStore!, path: $appPath!, requestBody: { - value: $app!, + value: withoutDraftBase($app!), summary: $summary, policy, path: npath, @@ -753,7 +762,7 @@ deployed: deployedValue ?? savedApp, current: { summary: $summary, - value: $app, + value: withoutDraftBase($app), path: newEditedPath || savedApp.path, policy, custom_path: customPath, @@ -847,7 +856,7 @@ bind:deployedValue currentValue={{ summary: $summary, - value: $app, + value: withoutDraftBase($app), path: newEditedPath || savedApp?.path, policy, custom_path: customPath, @@ -898,7 +907,7 @@ deployed: deployedValue ?? savedApp, current: { summary: $summary, - value: $app, + value: withoutDraftBase($app), path: newEditedPath || savedApp.path, policy, custom_path: customPath, diff --git a/frontend/src/lib/components/dbTypes.ts b/frontend/src/lib/components/dbTypes.ts index e6d3af12e8..2bb2a92e14 100644 --- a/frontend/src/lib/components/dbTypes.ts +++ b/frontend/src/lib/components/dbTypes.ts @@ -32,6 +32,9 @@ export const isDbType = (str?: string): str is DbType => !!str && dbTypes.includ /** The role every data table has: the one it connects as when it is not under roles. */ export const ADMIN_DATATABLE_ROLE = 'admin' +/** What `default_role` holds for a data table with no default role. Mirrors the backend's + * `NO_DEFAULT_DATATABLE_ROLE`. */ +export const NO_DEFAULT_DATATABLE_ROLE = '$none' /** What the server accepts in `-- role ` and `?role=`. */ export function isDatatableRoleName(name: string): boolean { @@ -52,7 +55,13 @@ export function defaultMigrationRole( permissioned: boolean | undefined, defaultRole: string | undefined ): string | undefined { - return permissioned && !datatableNameTakesRole(name) ? defaultRole : undefined + return permissioned && !datatableNameTakesRole(name) ? namedDefaultRole(defaultRole) : undefined +} + +/** A data table's default role, or `undefined` when it has none: the server reports that as + * `NO_DEFAULT_DATATABLE_ROLE`, which is not a role anything can connect as. */ +export function namedDefaultRole(defaultRole: string | undefined): string | undefined { + return defaultRole === NO_DEFAULT_DATATABLE_ROLE ? undefined : defaultRole } /** `datatable://`, with `?role=` when a role is named. Throws rather than build a diff --git a/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte b/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte index af0af7ed5b..0ebb67094b 100644 --- a/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte @@ -14,6 +14,7 @@ ADMIN_DATATABLE_ROLE, datatableNameTakesRole, defaultMigrationRole, + namedDefaultRole, type DbInput } from '../dbTypes' import type { PendingRowAction, SelectedTable } from '../DBManager.svelte' @@ -173,15 +174,15 @@ return rolesOfCurrent?.permissioned ? selectedRole : undefined } const entry = datatableTree.current.find((t) => t.datatable_name === datatable) - return entry?.permissioned ? entry.default_role : undefined + return entry?.permissioned ? namedDefaultRole(entry.default_role) : undefined } function defaultRoleOf(datatable: string): string | undefined { if (datatable === selectedDatatable && rolesOfCurrent?.permissioned) { - return rolesOfCurrent.default_role + return namedDefaultRole(rolesOfCurrent.default_role) } const entry = datatableTree.current.find((t) => t.datatable_name === datatable) - return entry?.permissioned ? entry.default_role : undefined + return entry?.permissioned ? namedDefaultRole(entry.default_role) : undefined } const tableDatatable = (t: SelectedTable) => t.datatable ?? selectedDatatable diff --git a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte index 680421450d..496c967d7f 100644 --- a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte +++ b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte @@ -23,7 +23,7 @@ import { superadmin } from '$lib/stores' import { sendUserToast } from '$lib/toast' import { deepEqual } from 'fast-equals' - import { ADMIN_DATATABLE_ROLE, isDatatableRoleName } from '../dbTypes' + import { ADMIN_DATATABLE_ROLE, NO_DEFAULT_DATATABLE_ROLE, isDatatableRoleName } from '../dbTypes' import PgAclEditor from '../datatableAcl/PgAclEditor.svelte' import ConfirmationModal from '../common/confirmationModal/ConfirmationModal.svelte' import { createAsyncConfirmationModal } from '../common/confirmationModal/asyncConfirmationModal.svelte' @@ -87,6 +87,8 @@ const pgInstanceName = $derived(pgInstanceNameOf(info?.cluster)) const unusedRoles = $derived(availableRoles.filter((r) => !roles.some((row) => row.id === r.id))) const confirmationModal = createAsyncConfirmationModal() + // Every member of the governing workspace, as the tenant lists spell it. + const EVERYONE_TENANT = '*' let aclEditor = $state(undefined) const hasUnsavedChanges = $derived( @@ -111,9 +113,13 @@ (a, b) => Number(b.id === ADMIN_DATATABLE_ROLE) - Number(a.id === ADMIN_DATATABLE_ROLE) ) // A data table never put under roles comes back with none; admin is what turning the toggle - // on starts from. + // on starts from, open to everyone so that turning it on changes nothing by itself. if (!loaded.some((r) => r.id === ADMIN_DATATABLE_ROLE)) { - loaded.unshift({ id: ADMIN_DATATABLE_ROLE, name: ADMIN_DATATABLE_ROLE, tenants: [] }) + loaded.unshift({ + id: ADMIN_DATATABLE_ROLE, + name: ADMIN_DATATABLE_ROLE, + tenants: [EVERYONE_TENANT] + }) } info = res catalog = undefined @@ -137,7 +143,7 @@ FolderService.listFolderNames({ workspace: ws }) ]) tenantItems = [ - { value: '*', label: 'Everyone', group: 'Anyone in the workspace' }, + { value: EVERYONE_TENANT, label: 'Everyone', group: 'Anyone in the workspace' }, ...users.map((u) => ({ value: `u/${u}`, label: u, group: 'Users' })), ...groups.map((g) => ({ value: `g/${g}`, label: g, group: 'Groups' })), ...folders.map((f) => ({ value: `f/${f}`, label: f, group: 'Folders' })) @@ -218,11 +224,28 @@ } } - function removeRole(role: EditedRole) { + async function removeRole(role: EditedRole) { + if (defaultRoleId === role.id) { + const confirmed = await confirmationModal.ask({ + title: 'Remove the default role?', + children: `Jobs that name no role connect as ${role.name ?? role.id}. Without it the data table has no default role: they connect with the roles of the person running them, and are refused when that person has none.`, + confirmationText: 'Remove', + type: 'danger' + }) + if (!confirmed) return + defaultRoleId = NO_DEFAULT_DATATABLE_ROLE + } roles = roles.filter((r) => r.id !== role.id) - if (defaultRoleId === role.id) defaultRoleId = ADMIN_DATATABLE_ROLE } + const defaultRoleName = $derived(roles.find((r) => r.id === defaultRoleId)?.name ?? defaultRoleId) + const defaultsToAdmin = $derived(defaultRoleId === ADMIN_DATATABLE_ROLE) + const hasNoDefault = $derived(defaultRoleId === NO_DEFAULT_DATATABLE_ROLE) + const hasOtherRoles = $derived(roles.some((r) => r.id !== ADMIN_DATATABLE_ROLE)) + const adminOpenToEveryone = $derived( + !!roles.find((r) => r.id === ADMIN_DATATABLE_ROLE)?.tenants.includes(EVERYONE_TENANT) + ) + async function save() { saving = true // Turning roles off clears the whole block, so every role it named is removed. @@ -320,7 +343,7 @@ options={{ right: 'Put this data table under roles', rightTooltip: - 'Off, every job connects as admin — the connection that owns every table. On, every job resolves to a role, and a caller no tenant covers is refused.' + 'Off, every job connects as admin — the connection that owns every table. On, every job resolves to a role, and a caller no tenant covers is refused. It starts with admin open to everyone, so turning it on changes nothing until the roles are edited.' }} /> @@ -391,8 +414,10 @@ Default - The role a job gets when it names none — no `-- role` annotation, no `?role=` in - the reference. Callers still have to be one of its tenants. + The role a job gets when it names none: no `-- role` annotation, no `?role=` in + the reference. There is always one choice: admin until changed, another role, or + none. Unless it is admin, the roles the person running the job is a tenant of + are added to it. @@ -440,6 +465,29 @@ {/each} + + + No default role + + + + Jobs that name no role get only the roles of the person running them. + + + +
+ { + defaultRoleId = NO_DEFAULT_DATATABLE_ROLE + }} + /> +
+ + + {#if editable} @@ -466,6 +514,47 @@ {/if} + + {#if adminOpenToEveryone} + + Every member of the workspace may use admin{defaultsToAdmin + ? ', and a job that names no role connects as it, exactly as with roles off' + : ' by naming it'}. admin owns every table: it reads every row and ignores grants + and row-level security. Once the other roles are set up, remove Everyone from admin's + tenants{defaultsToAdmin ? ' and choose another default, or none' : ''}. + + {/if} + {#if defaultsToAdmin && hasOtherRoles && !adminOpenToEveryone} + + A job that names no role connects as admin when it runs as a workspace admin or as one + of admin's tenants, which covers every viewer of an app an admin published. admin owns + every table: it reads every row and ignores grants and row-level security. Everyone + else's jobs that name no role are refused. Choose another default, or none, to have + the roles above apply to them. + + {:else if !defaultsToAdmin} +
+ {#if hasNoDefault} + + A job that names no role connects with the roles here that the person running it + is a tenant of. It is refused when they have none, and so is a job with nobody + behind it: a schedule, a trigger, an anonymous app viewer. + + {:else} + + A job that names no role connects as + {defaultRoleName} when it may use it, plus every other + role here that the person running it is a tenant of. With neither, it is refused. + + {/if} + A job that names a role connects as that role alone. + + A table created over a connection that holds several roles belongs to that + combination of roles alone. Run SET ROLE to one role before + creating it, or name a role. + +
+ {/if} {/if} diff --git a/frontend/src/lib/utils_draft_deploy.ts b/frontend/src/lib/utils_draft_deploy.ts index df04bb383a..611d5b96e8 100644 --- a/frontend/src/lib/utils_draft_deploy.ts +++ b/frontend/src/lib/utils_draft_deploy.ts @@ -538,8 +538,14 @@ export async function deployDraft( // no draft. `draft_path` and `summary` are draft-only fields mirrored onto // the App value (the editor drops them on deploy), so strip them from the // value and apply them as the deploy path / summary column. + // `parent_version` is the draft's stale-check base, never deployed. const draft = r.draft as Record | undefined - const { draft_path: draftPath, summary: draftSummary, ...appValue } = draft ?? r.value ?? {} + const { + draft_path: draftPath, + summary: draftSummary, + parent_version: _pv, + ...appValue + } = draft ?? r.value ?? {} // Policy isn't carried in the app draft, so it comes from the deployed app // (or a default). custom_path requires admin on update; non-admins send // undefined so the backend preserves the existing route. The draft has no diff --git a/system_prompts/auto-generated/prompts.ts b/system_prompts/auto-generated/prompts.ts index 097a7af895..c3310351b7 100644 --- a/system_prompts/auto-generated/prompts.ts +++ b/system_prompts/auto-generated/prompts.ts @@ -1131,6 +1131,7 @@ Data tables are PostgreSQL databases managed by Windmill. Backend runnables quer 3. **No DDL inside runnables**: runnables only read and write rows (SELECT, INSERT, UPDATE, DELETE) on existing tables. Never CREATE, ALTER or DROP a table from a runnable. 4. **Qualify table names**: an unqualified name means the \`public\` schema, so write every other table as \`schema.table\`, in table creation and queries alike. The app's \`data\` config sets the default datatable and schema its tables go in; use them consistently across runnables. 5. **Pass the role**: when the app's \`data.roles\` gives a data table a role, every \`wmill.datatable\` call on it passes that role (\`wmill.datatable('main', { role: 'analyst' })\` in TypeScript, \`wmill.datatable('main', role='analyst')\` in Python). The role only reaches what it was granted, so a query outside it fails with \`permission denied\`. +6. **Per-viewer rows**: a data table with no entry in \`data.roles\` is called with no role. Unless its default role is \`admin\`, the connection then holds the default role, if it has one, plus every role the person using the app is a tenant of, and is refused when that is none. Row-level security policies written as \`pg_has_role(current_user, '', 'member')\` return that viewer's rows with nothing added to the runnable. A role pins the connection to that one role for every viewer, so where rows should differ per viewer give the data table no \`data.roles\` entry. ### Querying in TypeScript (Bun/Deno) diff --git a/system_prompts/auto-generated/skills/raw-app/SKILL.md b/system_prompts/auto-generated/skills/raw-app/SKILL.md index 7afcbd2969..bd21352995 100644 --- a/system_prompts/auto-generated/skills/raw-app/SKILL.md +++ b/system_prompts/auto-generated/skills/raw-app/SKILL.md @@ -188,7 +188,7 @@ data: - `/
` — Specific table in public schema - `/:
` — Table in specific schema -**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. `roles` records the role the app uses each datatable through; the app's code must pass the same role: `wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python. A datatable without an entry is used as its default role. +**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. `roles` records the role the app uses each datatable through; the app's code must pass the same role: `wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python. A datatable without an entry is used as its default role, if it has one, plus the roles of whoever is using the app unless that default is `admin`. ## SQL Migrations (sql_to_apply/) @@ -494,6 +494,7 @@ Data tables are PostgreSQL databases managed by Windmill. Backend runnables quer 3. **No DDL inside runnables**: runnables only read and write rows (SELECT, INSERT, UPDATE, DELETE) on existing tables. Never CREATE, ALTER or DROP a table from a runnable. 4. **Qualify table names**: an unqualified name means the `public` schema, so write every other table as `schema.table`, in table creation and queries alike. The app's `data` config sets the default datatable and schema its tables go in; use them consistently across runnables. 5. **Pass the role**: when the app's `data.roles` gives a data table a role, every `wmill.datatable` call on it passes that role (`wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python). The role only reaches what it was granted, so a query outside it fails with `permission denied`. +6. **Per-viewer rows**: a data table with no entry in `data.roles` is called with no role. Unless its default role is `admin`, the connection then holds the default role, if it has one, plus every role the person using the app is a tenant of, and is refused when that is none. Row-level security policies written as `pg_has_role(current_user, '', 'member')` return that viewer's rows with nothing added to the runnable. A role pins the connection to that one role for every viewer, so where rows should differ per viewer give the data table no `data.roles` entry. `wmill datatable list` lists the workspace's data tables. Create or change tables with a migration in `sql_to_apply/` (see "SQL Migrations" above), then add them to `data.tables` in `raw_app.yaml`. diff --git a/system_prompts/base/raw-app-cli.md b/system_prompts/base/raw-app-cli.md index f12762ff5f..e22de102f6 100644 --- a/system_prompts/base/raw-app-cli.md +++ b/system_prompts/base/raw-app-cli.md @@ -183,7 +183,7 @@ data: - `/
` — Specific table in public schema - `/:
` — Table in specific schema -**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. `roles` records the role the app uses each datatable through; the app's code must pass the same role: `wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python. A datatable without an entry is used as its default role. +**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. `roles` records the role the app uses each datatable through; the app's code must pass the same role: `wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python. A datatable without an entry is used as its default role, if it has one, plus the roles of whoever is using the app unless that default is `admin`. ## SQL Migrations (sql_to_apply/) diff --git a/system_prompts/base/raw-app.md b/system_prompts/base/raw-app.md index 23b09f1996..0979000b2a 100644 --- a/system_prompts/base/raw-app.md +++ b/system_prompts/base/raw-app.md @@ -264,6 +264,7 @@ Data tables are PostgreSQL databases managed by Windmill. Backend runnables quer 3. **No DDL inside runnables**: runnables only read and write rows (SELECT, INSERT, UPDATE, DELETE) on existing tables. Never CREATE, ALTER or DROP a table from a runnable. 4. **Qualify table names**: an unqualified name means the `public` schema, so write every other table as `schema.table`, in table creation and queries alike. The app's `data` config sets the default datatable and schema its tables go in; use them consistently across runnables. 5. **Pass the role**: when the app's `data.roles` gives a data table a role, every `wmill.datatable` call on it passes that role (`wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python). The role only reaches what it was granted, so a query outside it fails with `permission denied`. +6. **Per-viewer rows**: a data table with no entry in `data.roles` is called with no role. Unless its default role is `admin`, the connection then holds the default role, if it has one, plus every role the person using the app is a tenant of, and is refused when that is none. Row-level security policies written as `pg_has_role(current_user, '', 'member')` return that viewer's rows with nothing added to the runnable. A role pins the connection to that one role for every viewer, so where rows should differ per viewer give the data table no `data.roles` entry. `wmill datatable list` lists the workspace's data tables. Create or change tables with a migration in `sql_to_apply/` (see "SQL Migrations" above), then add them to `data.tables` in `raw_app.yaml`.