fix(datatables): close the fail-open in role resolution, and gate the ACL endpoints

A role with a stored pg_rolename but no password resolved to the data table's
own connection, which owns everything — so a caller authorized as one role got
the admin one instead. Exports and git-synced settings redact that password, so
a restored config is exactly the shape that produced it. Refuse instead, and
name the fix.

The ACL endpoints took any workspace member: on a data table without roles
every member resolves to that same admin connection, so ownership and grants
there are the workspace admins' to change, as the roles themselves are.

Also: check a migration batch's roles before applying any of it, keep the role
picker for a single non-default role, drop the revoke button from a default
privilege on types (which no scope can express), and say what
get_datatable_resource_as_default_role actually resolves as.

backend/ee-repo-ref.txt

@@ -1 +1 @@
-e8e323558523176e544927b44acf084c27a1b2aa
+ef2f5f487b90ac64d07d39f791592a5445887c47

backend/windmill-api-workspaces/src/datatable_acl.rs

@@ -241,6 +241,20 @@ async fn connect_as_caller(
     datatable_name: &str,
     role: Option<&str>,
 ) -> Result<(tokio_postgres::Client, CallerConnection)> {
+    // A permissioned data table hands out a connection per role, and the tenant
+    // lists are what say who reaches which. Without permissions every member
+    // resolves to the data table's own connection, which owns everything — so
+    // there it is the workspace admins' to change, as the roles themselves are.
+    if !authed.is_admin
+        && !read_datatable(db, w_id, datatable_name)
+            .await?
+            .permissions
+            .is_some_and(|p| p.enabled)
+    {
+        return Err(Error::NotAuthorized(format!(
+            "Only an admin can manage access on data table '{datatable_name}', which has no roles"
+        )));
+    }
     let resource = get_datatable_resource_from_db(
         db,
         w_id,

backend/windmill-api-workspaces/src/datatable_migrations.rs

@@ -445,7 +445,7 @@ async fn run_datatable_migrations(
 
     let applied_versions = read_applied_versions_on_client(&client, &datatable_name).await?;
 
-    let mut applied = Vec::new();
+    let mut to_run = Vec::new();
     for m in migrations {
         if let Some(only) = query.only {
             // Run a single specific migration, skipping every other one.
@@ -459,11 +459,28 @@ async fn run_datatable_migrations(
         if applied_versions.contains(&m.timestamp) {
             continue;
         }
-        // Fail the batch rather than skip: a migration the caller may not run is a
-        // gap in an ordered sequence, and silently leaving it out would apply
-        // later ones on top of a schema that never got this change.
-        ensure_migration_role_allowed(&db, &w_id, &datatable_name, &authed, &m.code_up, m.timestamp, &m.name)
-            .await?;
+        to_run.push(m);
+    }
+
+    // Fail the batch rather than skip: a migration the caller may not run is a
+    // gap in an ordered sequence, and silently leaving it out would apply later
+    // ones on top of a schema that never got this change. Checked for the whole
+    // batch first, so the refusal does not land half way through it.
+    for m in to_run.iter() {
+        ensure_migration_role_allowed(
+            &db,
+            &w_id,
+            &datatable_name,
+            &authed,
+            &m.code_up,
+            m.timestamp,
+            &m.name,
+        )
+        .await?;
+    }
+
+    let mut applied = Vec::new();
+    for m in to_run {
         run_datatable_migration_job(&db, &user_db, &authed, &w_id, &database_arg, &m.code_up)
             .await
             .map_err(|e| {
@@ -860,7 +877,8 @@ async fn enable_datatable_migrations(
     // Opting in to migrations is one of the places an admin passes through, and
     // an instance database provisioned before the grants carried their options
     // cannot hand privileges to the roles permissions create.
-    crate::datatable_permissions::ensure_instance_db_can_delegate(&db, &w_id, &datatable_name).await;
+    crate::datatable_permissions::ensure_instance_db_can_delegate(&db, &w_id, &datatable_name)
+        .await;
 
     audit_log(
         &db,

backend/windmill-api-workspaces/src/datatable_permissions.rs

@@ -198,9 +198,6 @@ pub(crate) async fn ensure_instance_db_can_delegate(db: &DB, w_id: &str, datatab
     }
 }
 
-/// Connect to the data table's own database as `admin` and report the identity
-/// the plan has to be built against: the database name, the role that owns the
-/// existing objects, and the roles that actually exist in the cluster.
 /// What the plan has to be built against, probed from the data table's own
 /// database rather than assumed from its config.
 pub(crate) struct AdminConnection {
@@ -274,6 +271,9 @@ async fn read_default_acl_rules(client: &tokio_postgres::Client) -> Result<Vec<D
         .collect())
 }
 
+/// Connect to the data table's own database as `admin` and report the identity a
+/// plan has to be built against: the database name, the role that owns the
+/// existing objects, and the roles that actually exist in the cluster.
 pub(crate) async fn connect_as_admin(
     db: &DB,
     w_id: &str,

backend/windmill-api-workspaces/src/workspaces.rs

@@ -2472,10 +2472,11 @@ async fn get_datatable_table_schema(
     }))
 }
 
-/// Resolve a data table for an API caller. Schema browsing and the database
-/// manager always connect as `root`, so a permissioned data table is reachable
-/// from the UI only by a tenant of its `root` role (and by admins).
-async fn get_datatable_resource_as_admin(
+/// Resolve a data table for an API caller as the role it defaults to — schema
+/// browsing, the database manager and copy-between-databases name none — so a
+/// permissioned data table is reachable from the UI only by a tenant of that
+/// role (and by workspace admins, who may use any of them).
+async fn get_datatable_resource_as_default_role(
     db: &DB,
     authed: &ApiAuthed,
     w_id: &str,
@@ -2514,7 +2515,7 @@ async fn get_datatable_schema(
     datatable_name: &str,
 ) -> Result<SchemaMap> {
     // Get the datatable resource (connection credentials)
-    let db_resource = get_datatable_resource_as_admin(db, authed, w_id, datatable_name).await?;
+    let db_resource = get_datatable_resource_as_default_role(db, authed, w_id, datatable_name).await?;
 
     // Parse the resource as PgDatabase
     let pg_db: PgDatabase = serde_json::from_value(db_resource)
@@ -2763,7 +2764,7 @@ async fn get_datatable_table_columns(
         )));
     }
 
-    let db_resource = get_datatable_resource_as_admin(db, authed, w_id, datatable_name).await?;
+    let db_resource = get_datatable_resource_as_default_role(db, authed, w_id, datatable_name).await?;
     let pg_db: PgDatabase = serde_json::from_value(db_resource)
         .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?;
     let (client, connection) = pg_db.connect(Some(db)).await?;
@@ -2986,7 +2987,7 @@ pub(crate) async fn resolve_pg_source_checked(
     source: &str,
 ) -> Result<PgDatabase> {
     let db_resource = if let Some(name) = source.strip_prefix("datatable://") {
-        get_datatable_resource_as_admin(db, authed, w_id, name).await?
+        get_datatable_resource_as_default_role(db, authed, w_id, name).await?
     } else if let Some(path) = source.strip_prefix("$res:") {
         let db_with_authed = windmill_common::db::DbWithOptAuthed::from_authed(
             authed,

backend/windmill-common/src/worker.rs

@@ -1084,7 +1084,7 @@ pub struct SqlAnnotations {
 impl SqlAnnotations {
     /// If the script declares `-- role <name>`, returns the data table role the
     /// query runs as. Only meaningful against a permissioned `datatable://`
-    /// database; absent means the `root` role.
+    /// database; absent means the data table's default role.
     ///
     /// Mirrors `BashAnnotations::ssh_target`: only leading comment lines are
     /// scanned, and an exact `-- role <name>` with a valid role name and nothing

backend/windmill-common/src/workspaces.rs

@@ -1426,10 +1426,20 @@ async fn resolve_datatable_role(
         )));
     }
 
-    Ok(role_entry
-        .pg_rolename
-        .clone()
-        .zip(role_entry.pg_password.clone()))
+    // A role named without a stored credential is not a reason to fall back to
+    // the data table's own connection: that one owns everything, so the caller
+    // would silently get more than the role they asked for. Exports and
+    // git-synced settings redact the password, so a restored config lands here.
+    match (
+        role_entry.pg_rolename.clone(),
+        role_entry.pg_password.clone(),
+    ) {
+        (Some(rolename), Some(password)) => Ok(Some((rolename, password))),
+        (Some(_), None) => Err(Error::internal_err(format!(
+            "Role '{role_name}' of data table '{name}' has no stored credential; save its permissions again to reset it"
+        ))),
+        (None, _) => Ok(None),
+    }
 }
 
 async fn get_datatable_resource_inner(

frontend/src/lib/components/DBManager.svelte

@@ -132,6 +132,9 @@
 		onImport
 	}: Props = $props()
 
+	/** Shown on the entries the server refuses to plan without a license. */
+	const EE_PERMISSIONS_TOOLTIP = 'Data table permissions require an Enterprise license'
+
 	const sameTable = (a: SelectedTable, b: SelectedTable) =>
 		a.datatable === b.datatable && a.schema === b.schema && a.table === b.table
 
@@ -237,8 +240,6 @@
 	/** Whether the connected role may change access on an object: Postgres asks
 	 * for membership of its owner, so for the rest the entry is not offered. */
 	function canManage(datatable: string | undefined, schemaKey: string, table?: string): boolean {
-		// The server refuses to plan any of it without a license.
-		if (!$enterpriseLicense) return false
 		// A workspace admin manages the data table itself, so nothing in it is
 		// hidden from them — `public` is owned by neither Windmill nor its roles.
 		if (canManageDatatable) return true
@@ -690,11 +691,13 @@
 											icon: HistoryIcon,
 											action: () => onDatatableAction?.(dt, 'migrations')
 										},
-										...(canManageDatatable && $enterpriseLicense
+										...(canManageDatatable
 											? [
 													{
 														displayName: 'Roles',
 														icon: KeyRoundIcon,
+														disabled: !$enterpriseLicense,
+														tooltip: EE_PERMISSIONS_TOOLTIP,
 														action: () => onDatatableAction?.(dt, 'roles')
 													}
 												]
@@ -753,6 +756,8 @@
 															{
 																displayName: 'Permissions',
 																icon: KeyRoundIcon,
+																disabled: !$enterpriseLicense,
+																tooltip: EE_PERMISSIONS_TOOLTIP,
 																action: () =>
 																	(aclDrawer = {
 																		datatable: root.datatable,
@@ -839,6 +844,8 @@
 																	{
 																		displayName: 'Permissions',
 																		icon: KeyRoundIcon,
+																		disabled: !$enterpriseLicense,
+																		tooltip: EE_PERMISSIONS_TOOLTIP,
 																		action: () =>
 																			(aclDrawer = {
 																				datatable: root.datatable,

frontend/src/lib/components/datatableAcl/PgAclEditor.svelte

@@ -13,7 +13,7 @@
 	import Cell from '../table/Cell.svelte'
 	import { Trash2 } from 'lucide-svelte'
 	import PgGrantBuilder from './PgGrantBuilder.svelte'
-	import { grantScopeLabel, groupGrants, type AclScope } from './aclScopes'
+	import { grantScopeLabel, groupGrants, revokeScopeOf } from './aclScopes'
 
 	let {
 		workspace,
@@ -190,7 +190,8 @@
 								<Cell><span class="font-mono text-2xs">{grant.privileges.join(', ')}</span></Cell>
 								<Cell>{grantScopeLabel(grant)}</Cell>
 								<Cell last>
-									{#if info.roles.includes(grant.grantee)}
+									{@const revokeScope = revokeScopeOf(grant)}
+									{#if info.roles.includes(grant.grantee) && revokeScope}
 										<Button
 											unifiedSize="xs"
 											variant="default"
@@ -204,9 +205,7 @@
 														type: 'revoke',
 														role: grant.grantee,
 														privileges: grant.privileges,
-														scope: grant.future
-															? (`future_${grant.future.toLowerCase()}` as AclScope)
-															: 'target',
+														scope: revokeScope,
 														objects: grant.objects
 													},
 													`Revoked ${grant.privileges.join(', ')} from ${grant.grantee}`

frontend/src/lib/components/datatableAcl/aclScopes.ts

@@ -130,6 +130,17 @@ export function groupGrants(grants: AclGrant[]): GroupedGrant[] {
 	return rows
 }
 
+/** The scope a revoke of this row takes, or `undefined` when the builder cannot
+ * express it — Postgres also records default privileges on types, which nothing
+ * here grants and the API has no scope for. */
+export function revokeScopeOf(grant: GroupedGrant): AclScope | undefined {
+	if (!grant.future) return 'target'
+	const scope = `future_${grant.future.toLowerCase()}`
+	return (['future_tables', 'future_sequences', 'future_functions'] as const).find(
+		(s) => s === scope
+	)
+}
+
 /** How a row reads back: what it covers, in one phrase. */
 export function grantScopeLabel(grant: GroupedGrant): string {
 	if (grant.future) return `${grant.future.toLowerCase()} created later`

frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte

@@ -469,19 +469,17 @@
 								tooltip: 'Save the settings first',
 								action: () => migrationsButtons[dataTable.name]?.open()
 							},
-							// The server refuses to plan a permissions change without a
-							// license, so the entry is not offered either.
-							...($enterpriseLicense
-								? [
-										{
-											displayName: 'Permissions',
-											icon: KeyRound,
-											disabled: !!dirtyMap[dataTable.name],
-											tooltip: 'Save the settings first',
-											action: () => permissionsButtons[dataTable.name]?.openPermissions()
-										}
-									]
-								: []),
+							{
+								displayName: 'Permissions',
+								icon: KeyRound,
+								// The server refuses to plan a permissions change without a
+								// license, so the entry says so rather than failing later.
+								disabled: !!dirtyMap[dataTable.name] || !$enterpriseLicense,
+								tooltip: !$enterpriseLicense
+									? 'Data table permissions require an Enterprise license'
+									: 'Save the settings first',
+								action: () => permissionsButtons[dataTable.name]?.openPermissions()
+							},
 							{
 								displayName: 'Remove',
 								icon: Trash2,

frontend/src/lib/components/workspaceSettings/NewDataTableMigrationModal.svelte

@@ -239,7 +239,9 @@
 				class="grow"
 				inputProps={{ placeholder: 'Migration name (e.g. add_index_to_customers)' }}
 			/>
-			{#if roleItems.length > 1}
+			<!-- One role is still a choice worth showing when it is not the default
+				 the migration would otherwise run as. -->
+			{#if roleItems.length > 1 || (roleItems.length === 1 && roleItems[0].value !== usableRoles.current?.default_role)}
 				<Select
 					transformInputSelectedText={(s) => `Role: ${s}`}
 					items={roleItems}