From 45dd4cb271877ba963f809739ee9a34c7a55898c Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Wed, 16 Sep 2026 16:53:13 +0200 Subject: [PATCH] fix: refresh grant options only after the ACL apply validates its plan Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb --- .../windmill-api-workspaces/src/datatable_acl.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 84bc949dd5..396a456e4e 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -1523,13 +1523,6 @@ async fn apply_datatable_acl( // have exhausted. let governing = authorize_acl_change(&db, &authed, &w_id, &datatable_name).await?; let (mut client, mut notices, dbname) = connect_as_admin_unchecked(&db, &governing).await?; - // Postgres only lets a role pass on a privilege it holds with grant option, and an instance - // database provisioned before data table roles holds none. Best-effort: a grant this fails to - // enable is refused below rather than skipped. - if let Err(e) = windmill_common::ensure_instance_db_grant_options_unchecked(&db, &dbname).await - { - tracing::warn!("Could not refresh grant options on '{dbname}': {e}"); - } // Held until the change is committed: a role renamed or dropped meanwhile would change what // the plan names, and a settings save could move the entry onto another database. Taken in the @@ -1555,6 +1548,15 @@ async fn apply_datatable_acl( )); } + // A role passes on only privileges it holds with grant option, which a database provisioned + // before data table roles lacks; a grant this fails to enable is refused below. After the plan + // check, since the planner reads these grants and would refuse what it just accepted; it + // connects as the server's own Postgres user, so it takes nothing from the pool. + if let Err(e) = windmill_common::ensure_instance_db_grant_options_unchecked(&db, &dbname).await + { + tracing::warn!("Could not refresh grant options on '{dbname}': {e}"); + } + // One transaction: a half-applied ownership transfer leaves one schema's objects owned by two // different roles. let pg_tx = client.transaction().await.map_err(|e| {