fix(datatables): repair an instance database's grant options where they are used

Handing a privilege to a role means granting it, and a privilege held without
the grant option cannot be passed on — Postgres answers such a statement with
a warning and no effect, so the roles silently receive nothing. Databases
provisioned before those options were part of the instance grants still hold
them plain.

The repair now runs on the two paths that need it, saving permissions and
applying an ACL change, rather than only on opting in to migrations: a data
table can use roles without ever touching a migration.
This commit is contained in:
Diego Imbert
2026-08-31 05:55:50 +02:00
parent 38023de582
commit fc3732fb7f
3 changed files with 47 additions and 23 deletions
@@ -797,6 +797,11 @@ async fn apply_datatable_acl(
Json(req): Json<AclChangeRequest>,
) -> Result<String> {
require_admin(authed.is_admin, &authed.username)?;
// Granting is passing a privilege on, which this connection cannot do for a
// privilege it holds without the grant option.
crate::datatable_permissions::ensure_instance_db_can_delegate(&db, &w_id, &datatable_name).await;
let (mut client, plan, dbname) = build_acl_plan(&db, &w_id, &datatable_name, &req).await?;
// One transaction: a half-applied ownership transfer leaves objects of one
@@ -35,10 +35,7 @@ use windmill_common::runnable_settings::{ConcurrencySettingsWithCustom, Debounci
use windmill_common::scripts::ScriptLang;
use windmill_common::users::username_to_permissioned_as;
use windmill_common::worker::{to_raw_value, SqlAnnotations};
use windmill_common::ensure_instance_db_grant_options;
use windmill_common::workspaces::{
get_datatable_resource_from_db_unchecked, DataTableCatalogResourceType,
};
use windmill_common::workspaces::get_datatable_resource_from_db_unchecked;
use windmill_common::{PgDatabase, DB};
use windmill_git_sync::{
handle_deployment_metadata, handle_deployment_metadata_batch, DeployedObject,
@@ -857,24 +854,10 @@ async fn enable_datatable_migrations(
)));
}
// A data table provisioned before instance grants carried `WITH GRANT
// OPTION` cannot hand its privileges to the roles the permissions feature
// creates. Opting in to migrations is where an admin passes through, so
// repair it here; it is a no-op for one that already has them, and only
// applies to instance databases, where Windmill owns the Postgres user.
if let Ok(datatable) = crate::datatable_permissions::read_datatable(&db, &w_id, &datatable_name).await {
if datatable.database.resource_type == DataTableCatalogResourceType::Instance {
if let Err(e) =
ensure_instance_db_grant_options(&db, &datatable.database.resource_path).await
{
tracing::warn!(
"Could not refresh the grant options of instance database '{}': {}. Continuing.",
datatable.database.resource_path,
e
);
}
}
}
// 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;
audit_log(
&db,
@@ -30,9 +30,11 @@ use windmill_audit::ActionKind;
use windmill_common::error::{pg_error_message, Error, JsonResult, Result};
use windmill_common::query_builders::{render_db_quoted_identifier, DbType};
use windmill_common::utils::{rd_string, require_admin};
use windmill_common::ensure_instance_db_grant_options;
use windmill_common::workspaces::{
can_use_datatable_role, datatable_pg_role_name, get_datatable_resource_from_db_unchecked,
DataTable, DataTablePermissions, DataTableRole, DATATABLE_TENANT_WILDCARD, ADMIN_DATATABLE_ROLE,
DataTable, DataTableCatalogResourceType, DataTablePermissions, DataTableRole,
ADMIN_DATATABLE_ROLE, DATATABLE_TENANT_WILDCARD,
};
use windmill_common::{PgDatabase, DB};
@@ -603,6 +605,36 @@ pub(crate) async fn read_datatable(
.map_err(|e| Error::internal_err(format!("Invalid data table config: {e}")))
}
/// Make sure `custom_instance_user` can pass its privileges on, for a data table
/// on an instance database.
///
/// Handing privileges to the roles this feature creates means granting them, and
/// a privilege held without `WITH GRANT OPTION` cannot be granted on — Postgres
/// answers such a statement with a warning and no effect. Databases provisioned
/// before those options were part of the grants still hold them plain, and only
/// the instance's own Postgres user, which owns them, can add the options.
///
/// Idempotent, and a no-op for a data table on a user-provided resource, where
/// Windmill does not own the Postgres user. Never fatal: the caller's own work
/// is what the admin asked for, and it may well not need any of this.
pub(crate) async fn ensure_instance_db_can_delegate(db: &DB, w_id: &str, datatable_name: &str) {
let Ok(datatable) = read_datatable(db, w_id, datatable_name).await else {
return;
};
if datatable.database.resource_type != DataTableCatalogResourceType::Instance {
return;
}
if let Err(e) =
ensure_instance_db_grant_options(db, &datatable.database.resource_path).await
{
tracing::warn!(
"Could not refresh the grant options of instance database '{}': {}. Continuing.",
datatable.database.resource_path,
e
);
}
}
/// 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.
@@ -878,6 +910,10 @@ async fn set_datatable_permissions(
) -> Result<String> {
require_admin(authed.is_admin, &authed.username)?;
// The roles about to be created are handed privileges by this connection,
// which cannot pass on what it holds without the grant option.
ensure_instance_db_can_delegate(&db, &w_id, &datatable_name).await;
// The plan is rebuilt here rather than trusted from the preview: the client
// never gets to choose what runs against the database.
let (mut client, plan) = build_plan(&db, &w_id, &datatable_name, &req).await?;