From 6e79d9636f2d4d3d906501b04780c763b0d0bdc1 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Tue, 1 Sep 2026 23:15:43 +0200 Subject: [PATCH] fix(datatables): keep the database's own grants to admin out of reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit admin is the login the data table reaches Postgres through, so revoking on the database itself takes away what every role here connects with — and what the role that would grant it back connects with. Refuse it server-side, and stop the drawer from offering a row it cannot act on. --- .../src/datatable_acl.rs | 28 +++++++++++++++++-- .../datatableAcl/PgAclEditor.svelte | 7 +++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 62ef3b65df..7467c06aac 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -980,6 +980,18 @@ async fn build_acl_plan( AclChange::Grant { role, .. } | AclChange::Revoke { role, .. } => role, }; let pg_role = pg_role_of(&roles, role_name)?; + // `admin` is the login the data table itself reaches Postgres through, so a + // revoke on the database would take away what every role here connects with + // — including the one that would have to grant it back. + if matches!(req.change, AclChange::Revoke { .. }) + && matches!(req.target, AclTarget::Database) + && role_name == ADMIN_DATATABLE_ROLE + { + return Err(Error::BadRequest(format!( + "'{ADMIN_DATATABLE_ROLE}' is how this data table reaches its database; \ + revoking on the database itself would lock every role out of it" + ))); + } // The roles a plan may also write about: what a schema's new owner is kept // in reach of, and whose future objects a `created later` grant covers. Both // are `ALTER DEFAULT PRIVILEGES FOR ROLE `, which speaks for that role @@ -1125,10 +1137,17 @@ mod tests { grant(GrantScope::FutureSequences), grant(GrantScope::FutureFunctions), ] { - assert_eq!(reached_object_classes(&schema(), &change), None, "{change:?}"); + assert_eq!( + reached_object_classes(&schema(), &change), + None, + "{change:?}" + ); } assert_eq!( - reached_object_classes(&table(), &AclChange::SetOwner { role: "analyst".to_string() }), + reached_object_classes( + &table(), + &AclChange::SetOwner { role: "analyst".to_string() } + ), None ); @@ -1149,6 +1168,9 @@ mod tests { reached_object_classes(&schema(), &revoke(GrantScope::AllTables)), Some((["r", "p", "v", "m", "f"].as_slice(), false)) ); - assert_eq!(reached_object_classes(&schema(), &revoke(GrantScope::Target)), None); + assert_eq!( + reached_object_classes(&schema(), &revoke(GrantScope::Target)), + None + ); } } diff --git a/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte b/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte index febecce58b..7ff3fd41f0 100644 --- a/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte +++ b/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte @@ -14,6 +14,7 @@ import { Trash2 } from 'lucide-svelte' import PgGrantBuilder from './PgGrantBuilder.svelte' import { grantScopeLabel, groupGrants, revokeScopeOf } from './aclScopes' + import { ADMIN_DATATABLE_ROLE } from '../dbTypes' let { workspace, @@ -192,8 +193,10 @@ {@const revokeScope = revokeScopeOf(grant)} - {#if info.roles.includes(grant.grantee) && revokeScope && info.can_manage} + on one the caller does not own has nothing to offer. And the + database's own grants to `admin` are what every role here + connects with, so they are not the drawer's to take away. --> + {#if info.roles.includes(grant.grantee) && revokeScope && info.can_manage && !(target.kind === 'database' && grant.grantee === ADMIN_DATATABLE_ROLE)}