fix(datatables): cover routines in ownership, grants and default privileges

Functions live in pg_proc, so a schema changing hands left them behind and a grant on ALL FUNCTIONS disappeared from the drawer on the next read, with no way to revoke it. Read them alongside pg_class, and name them by their identity arguments, which is what tells two of one name apart. ALTER DEFAULT PRIVILEGES binds only the roles it names, so a role created after a 'created later' grant produced tables no one else could read. Replay the rules already in force for each new role.
2026-09-05 16:03:47 +00:00 · 2026-09-01 04:09:19 +02:00
parent 7619a7a51e
commit 2bc39481da
5 changed files with 293 additions and 52 deletions
@@ -58,8 +58,13 @@ pub enum AclTarget {
    /// The data table's own database — where the privilege to create schemas
    /// lives.
    Database,
-    Schema { schema: String },
-    Table { schema: String, table: String },
+    Schema {
+        schema: String,
+    },
+    Table {
+        schema: String,
+        table: String,
+    },
 }

 impl AclTarget {
@@ -110,9 +115,9 @@ impl TryFrom<AclTargetQuery> for AclTarget {
            ("database", _, _) => Ok(AclTarget::Database),
            ("schema", Some(schema), _) => Ok(AclTarget::Schema { schema }),
            ("table", Some(schema), Some(table)) => Ok(AclTarget::Table { schema, table }),
-            ("schema" | "table", None, _) => Err(Error::BadRequest(
-                "This target needs a schema".to_string(),
-            )),
+            ("schema" | "table", None, _) => {
+                Err(Error::BadRequest("This target needs a schema".to_string()))
+            }
            ("table", _, None) => Err(Error::BadRequest(
                "A table target needs a table".to_string(),
            )),
@@ -237,6 +242,10 @@ pub struct AclObject {
    pub name: String,
    /// `TABLE`, `SEQUENCE`, ... — what the object is, since the keyword differs.
    pub kind: String,
+    /// A routine is identified by its argument types, not by its name: two
+    /// `f` in one schema are two objects. Absent for everything else.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub args: Option<String>,
 }

 /// A grant as the database has it, in Windmill's vocabulary where it can be.
@@ -461,10 +470,9 @@ fn plan_statements(
            statements.push(format!("ALTER {} OWNER TO {}", target.object(dbname), role));
            for object in existing_objects {
                statements.push(format!(
-                    "ALTER {} {}.{} OWNER TO {}",
+                    "ALTER {} {} OWNER TO {}",
                    object.keyword,
-                    schema,
-                    quote_ident(&object.name),
+                    object_ref(&schema, &object.name, object.args.as_deref()),
                    role
                ));
            }
@@ -552,11 +560,10 @@ fn plan_statements(
                (_, None) if !objects.is_empty() => {
                    for object in objects {
                        statements.push(format!(
-                            "REVOKE {} ON {} {}.{} FROM {}",
+                            "REVOKE {} ON {} {} FROM {}",
                            privileges,
                            object_keyword(&object.kind)?,
-                            schema,
-                            quote_ident(&object.name),
+                            object_ref(&schema, &object.name, object.args.as_deref()),
                            role
                        ));
                    }
@@ -609,6 +616,21 @@ struct OwnedObject {
    name: String,
    /// The keyword `ALTER ... OWNER TO` takes for this kind of object.
    keyword: &'static str,
+    /// Identity arguments of a routine, which is what tells two of the same
+    /// name apart. `None` for a relation.
+    args: Option<String>,
+}
+
+/// `"schema"."name"` — with `(args)` for a routine, which is not identified
+/// without them. `quoted_schema` comes quoted already, being the same for a
+/// whole plan.
+fn object_ref(quoted_schema: &str, name: &str, args: Option<&str>) -> String {
+    format!(
+        "{}.{}{}",
+        quoted_schema,
+        quote_ident(name),
+        args.map(|a| format!("({a})")).unwrap_or_default()
+    )
 }

 fn keyword_of_relkind(relkind: i8) -> Option<&'static str> {
@@ -643,13 +665,40 @@ async fn read_owned_objects(
                pg_error_message(&e)
            ))
        })?;
-    Ok(rows
+    let mut objects: Vec<OwnedObject> = rows
        .into_iter()
        .filter_map(|row| {
-            keyword_of_relkind(row.get::<_, i8>(1))
-                .map(|keyword| OwnedObject { name: row.get(0), keyword })
+            keyword_of_relkind(row.get::<_, i8>(1)).map(|keyword| OwnedObject {
+                name: row.get(0),
+                keyword,
+                args: None,
+            })
        })
-        .collect())
+        .collect();
+    // Routines live in `pg_proc`, not `pg_class`, and would keep the previous
+    // owner while the schema they are in changes hands. `ALTER ROUTINE` covers
+    // functions, procedures and aggregates alike.
+    let routines = client
+        .query(
+            "SELECT p.proname, pg_get_function_identity_arguments(p.oid)
+             FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace
+             WHERE n.nspname = $1
+             ORDER BY p.proname",
+            &[&schema],
+        )
+        .await
+        .map_err(|e| {
+            Error::internal_err(format!(
+                "Failed to list the routines of schema '{schema}': {}",
+                pg_error_message(&e)
+            ))
+        })?;
+    objects.extend(routines.into_iter().map(|row| OwnedObject {
+        name: row.get(0),
+        keyword: "ROUTINE",
+        args: Some(row.get(1)),
+    }));
+    Ok(objects)
 }

 async fn get_datatable_acl(
@@ -660,8 +709,8 @@ async fn get_datatable_acl(
 ) -> JsonResult<DatatableAclInfo> {
    let role = query.role.clone();
    let target: AclTarget = query.try_into()?;
-    let (client, conn) = connect_as_caller(&db, &authed, &w_id, &datatable_name, role.as_deref())
-        .await?;
+    let (client, conn) =
+        connect_as_caller(&db, &authed, &w_id, &datatable_name, role.as_deref()).await?;
    let roles = role_map(
        &db,
        &w_id,
@@ -716,8 +765,8 @@ async fn get_datatable_acl(
                ))
            })?,
    };
-    let owner_row =
-        owner_row.ok_or_else(|| Error::NotFound(format!("{} not found", target.label(&conn.dbname))))?;
+    let owner_row = owner_row
+        .ok_or_else(|| Error::NotFound(format!("{} not found", target.label(&conn.dbname))))?;
    let owner: String = owner_row.get(0);
    // Membership in the owning role is what Postgres asks for before an ALTER
    // ... OWNER or a GRANT on something you do not own; `admin` holds every role
@@ -789,7 +838,7 @@ async fn read_grants(
        AclTarget::Database => client
            .query(
                "SELECT CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END,
-                        a.privilege_type, NULL::text, NULL::text, NULL::text
+                        a.privilege_type, NULL::text, NULL::text, NULL::text, NULL::text
                 FROM pg_database d, aclexplode(d.datacl) a
                 WHERE d.datname = current_database()",
                &[],
@@ -800,7 +849,7 @@ async fn read_grants(
            let mut out = client
                .query(
                    "SELECT CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END,
-                            a.privilege_type, NULL::text, NULL::text, NULL::text
+                            a.privilege_type, NULL::text, NULL::text, NULL::text, NULL::text
                     FROM pg_namespace n, aclexplode(n.nspacl) a
                     WHERE n.nspname = $1",
                    &[schema],
@@ -812,7 +861,8 @@ async fn read_grants(
                    .query(
                        "SELECT CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END,
                                a.privilege_type, c.relname, NULL::text,
-                                CASE c.relkind WHEN 'S' THEN 'SEQUENCE' ELSE 'TABLE' END
+                                CASE c.relkind WHEN 'S' THEN 'SEQUENCE' ELSE 'TABLE' END,
+                                NULL::text
                         FROM pg_class c
                         JOIN pg_namespace n ON n.oid = c.relnamespace,
                              aclexplode(c.relacl) a
@@ -822,6 +872,24 @@ async fn read_grants(
                    .await
                    .map_err(grant_read_error)?,
            );
+            out.extend(
+                client
+                    .query(
+                        // Routines carry their own acl in `pg_proc`; without this
+                        // a grant made here would vanish on the next read and
+                        // could never be revoked back.
+                        "SELECT CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END,
+                                a.privilege_type, p.proname, NULL::text, 'FUNCTION',
+                                pg_get_function_identity_arguments(p.oid)
+                         FROM pg_proc p
+                         JOIN pg_namespace n ON n.oid = p.pronamespace,
+                              aclexplode(p.proacl) a
+                         WHERE n.nspname = $1",
+                        &[schema],
+                    )
+                    .await
+                    .map_err(grant_read_error)?,
+            );
            out.extend(
                client
                    .query(
@@ -829,7 +897,7 @@ async fn read_grants(
                                a.privilege_type, NULL::text,
                                CASE d.defaclobjtype
                                    WHEN 'r' THEN 'TABLES' WHEN 'S' THEN 'SEQUENCES'
-                                    WHEN 'f' THEN 'FUNCTIONS' ELSE 'TYPES' END, NULL::text
+                                    WHEN 'f' THEN 'FUNCTIONS' ELSE 'TYPES' END, NULL::text, NULL::text
                         FROM pg_default_acl d
                         JOIN pg_namespace n ON n.oid = d.defaclnamespace,
                              aclexplode(d.defaclacl) a
@@ -844,7 +912,7 @@ async fn read_grants(
        AclTarget::Table { schema, table } => client
            .query(
                "SELECT CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END,
-                        a.privilege_type, NULL::text, NULL::text, NULL::text
+                        a.privilege_type, NULL::text, NULL::text, NULL::text, NULL::text
                 FROM pg_class c
                 JOIN pg_namespace n ON n.oid = c.relnamespace,
                      aclexplode(c.relacl) a
@@ -857,18 +925,31 @@ async fn read_grants(

    // One row per privilege — and, for default privileges, one per creating
    // role. Fold them back into one entry per grantee and object.
-    let mut folded: BTreeMap<(String, Option<(String, String)>, Option<String>), Vec<String>> =
-        BTreeMap::new();
+    let mut folded: BTreeMap<
+        (
+            String,
+            Option<(String, String, Option<String>)>,
+            Option<String>,
+        ),
+        Vec<String>,
+    > = BTreeMap::new();
    for row in rows.drain(..) {
        let grantee: String = row.get(0);
        let privilege: String = row.get(1);
        let object: Option<String> = row.get(2);
        let future: Option<String> = row.get(3);
        let object_kind: Option<String> = row.get(4);
+        let object_args: Option<String> = row.get(5);
        folded
            .entry((
                windmill_role_of(roles, &grantee),
-                object.map(|name| (name, object_kind.unwrap_or_else(|| "TABLE".to_string()))),
+                object.map(|name| {
+                    (
+                        name,
+                        object_kind.unwrap_or_else(|| "TABLE".to_string()),
+                        object_args,
+                    )
+                }),
                future,
            ))
            .or_default()
@@ -882,7 +963,7 @@ async fn read_grants(
            AclGrant {
                grantee,
                privileges,
-                object: object.map(|(name, kind)| AclObject { name, kind }),
+                object: object.map(|(name, kind, args)| AclObject { name, kind, args }),
                future,
            }
        })
@@ -967,7 +1048,8 @@ async fn apply_datatable_acl(
 ) -> Result<String> {
    // 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;
+    crate::datatable_permissions::ensure_instance_db_can_delegate(&db, &w_id, &datatable_name)
+        .await;

    let (mut client, plan, dbname) =
        build_acl_plan(&db, &authed, &w_id, &datatable_name, &req).await?;
@@ -1020,8 +1102,13 @@ mod tests {
    #[test]
    fn set_owner_covers_the_schema_and_what_is_in_it() {
        let objects = vec![
-            OwnedObject { name: "orders".to_string(), keyword: "TABLE" },
-            OwnedObject { name: "orders_id_seq".to_string(), keyword: "SEQUENCE" },
+            OwnedObject { name: "orders".to_string(), keyword: "TABLE", args: None },
+            OwnedObject { name: "orders_id_seq".to_string(), keyword: "SEQUENCE", args: None },
+            OwnedObject {
+                name: "total".to_string(),
+                keyword: "ROUTINE",
+                args: Some("integer, text".to_string()),
+            },
        ];
        let plan = plan_statements(
            &schema(),
@@ -1033,11 +1120,14 @@ mod tests {
        )
        .unwrap();
        assert_eq!(
-            plan.statements[..3],
+            plan.statements[..4],
            [
                r#"ALTER SCHEMA "analytics" OWNER TO "wm_analyst_1""#.to_string(),
                r#"ALTER TABLE "analytics"."orders" OWNER TO "wm_analyst_1""#.to_string(),
                r#"ALTER SEQUENCE "analytics"."orders_id_seq" OWNER TO "wm_analyst_1""#.to_string(),
+                // A routine is only named by its arguments.
+                r#"ALTER ROUTINE "analytics"."total"(integer, text) OWNER TO "wm_analyst_1""#
+                    .to_string(),
            ]
        );
        // What the other roles create later stays within the owner's reach.
@@ -1151,6 +1241,7 @@ mod tests {
                objects: vec![AclObject {
                    name: "orders".to_string(),
                    kind: "TABLE".to_string(),
+                    args: None,
                }],
            },
            "dt_probe",
@@ -1244,8 +1335,8 @@ mod tests {
                privileges: vec!["SELECT".to_string()],
                scope: GrantScope::Target,
                objects: vec![
-                    AclObject { name: "a".to_string(), kind: "TABLE".to_string() },
-                    AclObject { name: "b".to_string(), kind: "TABLE".to_string() },
+                    AclObject { name: "a".to_string(), kind: "TABLE".to_string(), args: None },
+                    AclObject { name: "b".to_string(), kind: "TABLE".to_string(), args: None },
                ],
            },
            "dt_probe",
@@ -27,10 +27,10 @@ use std::collections::{BTreeMap, HashSet};
 use windmill_api_auth::ApiAuthed;
 use windmill_audit::audit_oss::audit_log;
 use windmill_audit::ActionKind;
+use windmill_common::ensure_instance_db_grant_options;
 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, DataTableCatalogResourceType, DataTablePermissions, DataTableRole,
@@ -182,6 +182,7 @@ fn plan_role_changes(
    req: &SetDatatablePermissions,
    existing_pg_roles: &HashSet<String>,
    public_schema_is_open: bool,
+    default_acl_rules: &[DefaultAclRule],
 ) -> Result<RolePlan> {
    // A disabled data table has no Postgres roles, whatever its config says, so
    // re-enabling always plans every role as a creation.
@@ -390,9 +391,14 @@ fn plan_role_changes(
                            "Role '{from}' was expected to exist in the database as '{old_pg}' but does not; it will be created as '{pg_rolename}'."
                        ));
                        creates_sql.push(create_role_statement(&pg_rolename, &password));
-                        creates_sql.push(grant_role_to_admin_statement(&pg_rolename, admin_pg_role));
+                        creates_sql
+                            .push(grant_role_to_admin_statement(&pg_rolename, admin_pg_role));
                        creates_sql.push(revoke_public_create_statement(&pg_rolename));
                        creates_sql.push(grant_connect_statement(&pg_rolename, dbname));
+                        creates_sql.extend(replay_default_acl_statements(
+                            &pg_rolename,
+                            default_acl_rules,
+                        ));
                    }
                }
                roles.insert(
@@ -429,6 +435,10 @@ fn plan_role_changes(
                creates_sql.push(grant_role_to_admin_statement(&pg_rolename, admin_pg_role));
                creates_sql.push(revoke_public_create_statement(&pg_rolename));
                creates_sql.push(grant_connect_statement(&pg_rolename, dbname));
+                creates_sql.extend(replay_default_acl_statements(
+                    &pg_rolename,
+                    default_acl_rules,
+                ));
                roles.insert(
                    name.clone(),
                    DataTableRole {
@@ -591,11 +601,7 @@ fn grant_connect_statement(pg_rolename: &str, dbname: &str) -> PlannedStatement
    ))
 }

-pub(crate) async fn read_datatable(
-    db: &DB,
-    w_id: &str,
-    datatable_name: &str,
-) -> Result<DataTable> {
+pub(crate) async fn read_datatable(db: &DB, w_id: &str, datatable_name: &str) -> Result<DataTable> {
    let value = sqlx::query_scalar!(
        "SELECT ws.datatable->'datatables'->$2 FROM workspace_settings ws WHERE ws.workspace_id = $1",
        w_id,
@@ -628,9 +634,7 @@ pub(crate) async fn ensure_instance_db_can_delegate(db: &DB, w_id: &str, datatab
    if datatable.database.resource_type != DataTableCatalogResourceType::Instance {
        return;
    }
-    if let Err(e) =
-        ensure_instance_db_grant_options(db, &datatable.database.resource_path).await
-    {
+    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,
@@ -651,6 +655,98 @@ pub(crate) struct AdminConnection {
    /// Whether `PUBLIC` holds CREATE on schema `public`, i.e. every role in this
    /// database — including the ones created here — can make objects in it.
    pub(crate) public_schema_is_open: bool,
+    /// The default-privilege rules already in force for this data table's roles.
+    pub(crate) default_acl_rules: Vec<DefaultAclRule>,
+}
+
+/// One `ALTER DEFAULT PRIVILEGES` rule as the catalog has it.
+///
+/// Postgres records such a rule per creating role, so a role added later is not
+/// covered by any of them: a grant on "future tables" would quietly stop
+/// applying to whatever that new role creates. Replaying the existing rules for
+/// each new role is what keeps the policy whole.
+#[derive(Debug, Clone, PartialEq)]
+pub(crate) struct DefaultAclRule {
+    /// `None` for a rule that is not scoped to a schema.
+    pub(crate) schema: Option<String>,
+    /// `TABLES`, `SEQUENCES`, `FUNCTIONS` or `TYPES`.
+    pub(crate) objects: String,
+    /// Postgres role the privileges go to, or `PUBLIC`.
+    pub(crate) grantee: String,
+    pub(crate) privileges: Vec<String>,
+}
+
+async fn read_default_acl_rules(client: &tokio_postgres::Client) -> Result<Vec<DefaultAclRule>> {
+    // Only the rules of this feature's own roles — and of the data table's
+    // connection — are replayed; the rest of the cluster's policy is not ours to
+    // copy onto a new role.
+    let rows = client
+        .query(
+            "SELECT n.nspname,
+                    CASE d.defaclobjtype
+                        WHEN 'r' THEN 'TABLES' WHEN 'S' THEN 'SEQUENCES'
+                        WHEN 'f' THEN 'FUNCTIONS' ELSE 'TYPES' END,
+                    CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END,
+                    a.privilege_type
+             FROM pg_default_acl d
+             LEFT JOIN pg_namespace n ON n.oid = d.defaclnamespace,
+                  aclexplode(d.defaclacl) a
+             WHERE pg_get_userbyid(d.defaclrole) LIKE 'wm\\_%'
+                OR pg_get_userbyid(d.defaclrole) = current_user",
+            &[],
+        )
+        .await
+        .map_err(|e| {
+            Error::internal_err(format!(
+                "Failed to read the default privileges: {}",
+                pg_error_message(&e)
+            ))
+        })?;
+    let mut folded: BTreeMap<(Option<String>, String, String), Vec<String>> = BTreeMap::new();
+    for row in rows {
+        folded
+            .entry((row.get(0), row.get(1), row.get(2)))
+            .or_default()
+            .push(row.get(3));
+    }
+    Ok(folded
+        .into_iter()
+        .map(|((schema, objects, grantee), mut privileges)| {
+            privileges.sort();
+            privileges.dedup();
+            DefaultAclRule { schema, objects, grantee, privileges }
+        })
+        .collect())
+}
+
+/// Put a role that did not exist when a default-privilege rule was written under
+/// that same rule, for what it creates from here on.
+fn replay_default_acl_statements(
+    pg_rolename: &str,
+    rules: &[DefaultAclRule],
+) -> Vec<PlannedStatement> {
+    rules
+        .iter()
+        .filter(|rule| rule.grantee != pg_rolename && !rule.privileges.is_empty())
+        .map(|rule| {
+            let grantee = if rule.grantee == "PUBLIC" {
+                "PUBLIC".to_string()
+            } else {
+                quote_ident(&rule.grantee)
+            };
+            PlannedStatement::plain(format!(
+                "ALTER DEFAULT PRIVILEGES FOR ROLE {}{} GRANT {} ON {} TO {};",
+                quote_ident(pg_rolename),
+                rule.schema
+                    .as_ref()
+                    .map(|s| format!(" IN SCHEMA {}", quote_ident(s)))
+                    .unwrap_or_default(),
+                rule.privileges.join(", "),
+                rule.objects,
+                grantee
+            ))
+        })
+        .collect()
 }

 pub(crate) async fn connect_as_admin(
@@ -717,9 +813,17 @@ pub(crate) async fn connect_as_admin(
        })?
        .get(0);

+    let default_acl_rules = read_default_acl_rules(&client).await?;
+
    Ok((
        client,
-        AdminConnection { dbname, admin_pg_role, existing_pg_roles, public_schema_is_open },
+        AdminConnection {
+            dbname,
+            admin_pg_role,
+            existing_pg_roles,
+            public_schema_is_open,
+            default_acl_rules,
+        },
    ))
 }

@@ -740,6 +844,7 @@ async fn build_plan(
        req,
        &conn.existing_pg_roles,
        conn.public_schema_is_open,
+        &conn.default_acl_rules,
    )?;
    Ok((client, plan))
 }
@@ -1021,6 +1126,16 @@ mod tests {
        req: &SetDatatablePermissions,
        existing: &[&str],
        public_schema_is_open: bool,
+    ) -> Result<RolePlan> {
+        plan_with_default_acls(old, req, existing, public_schema_is_open, &[])
+    }
+
+    fn plan_with_default_acls(
+        old: Option<&DataTablePermissions>,
+        req: &SetDatatablePermissions,
+        existing: &[&str],
+        public_schema_is_open: bool,
+        default_acl_rules: &[DefaultAclRule],
    ) -> Result<RolePlan> {
        plan_role_changes(
            W_ID,
@@ -1031,6 +1146,7 @@ mod tests {
            req,
            &existing.iter().map(|r| r.to_string()).collect(),
            public_schema_is_open,
+            default_acl_rules,
        )
    }

@@ -1073,6 +1189,29 @@ mod tests {
            .is_none());
    }

+    /// A default-privilege rule binds only the roles it was written for, so a
+    /// role added later would create tables no one else can read.
+    #[test]
+    fn a_new_role_inherits_the_default_privileges_already_in_force() {
+        let req = SetDatatablePermissions {
+            enabled: true,
+            roles: vec![role("admin", &[]), role("writer", &[])],
+            default_role: None,
+            renames: vec![],
+        };
+        let rules = vec![DefaultAclRule {
+            schema: Some("analytics".to_string()),
+            objects: "TABLES".to_string(),
+            grantee: "wm_analyst".to_string(),
+            privileges: vec!["SELECT".to_string()],
+        }];
+        let plan = plan_with_default_acls(None, &req, &[], false, &rules).unwrap();
+        let pg_role = datatable_pg_role_name(W_ID, DT, "writer");
+        assert!(sql(&plan).contains(&format!(
+            "ALTER DEFAULT PRIVILEGES FOR ROLE \"{pg_role}\" IN SCHEMA \"analytics\" GRANT SELECT ON TABLES TO \"wm_analyst\";"
+        ).as_str()));
+    }
+
    /// A role-scoped REVOKE cannot take back what PUBLIC holds, so the plan has
    /// to say so instead of pretending the statement closed it.
    #[test]
@@ -1239,7 +1378,11 @@ mod tests {
        let old_pg = datatable_pg_role_name(W_ID, DT, "analyst");
        let req = SetDatatablePermissions {
            enabled: true,
-            roles: vec![role("admin", &[]), role("reader", &[]), role("analyst", &[])],
+            roles: vec![
+                role("admin", &[]),
+                role("reader", &[]),
+                role("analyst", &[]),
+            ],
            default_role: None,
            renames: vec![DatatableRoleRename {
                from: "analyst".to_string(),
@@ -34197,6 +34197,9 @@ components:
        kind:
          type: string
          description: TABLE, SEQUENCE, ... — the keyword a REVOKE on it takes
+        args:
+          type: string
+          description: identity arguments of a routine, which is what tells two of the same name apart

    AclGrant:
      type: object
@@ -183,7 +183,7 @@
 					</Head>
 					<tbody class="divide-y">
 						{#each grantRows as grant (grant.grantee + grant.objects
-								.map((o) => o.name)
+								.map((o) => `${o.name}(${o.args ?? ''})`)
 								.join() + (grant.future ?? ''))}
 							<Row>
 								<Cell first>{grant.grantee}</Cell>
@@ -192,9 +192,8 @@
 								<Cell last>
 									{#if info.roles.includes(grant.grantee)}
 										<Button
-											size="xs"
-											color="light"
-											variant="border"
+											unifiedSize="xs"
+											variant="default"
 											iconOnly
 											startIcon={{ icon: Trash2 }}
 											title="Revoke"
@@ -133,8 +133,13 @@ export function groupGrants(grants: AclGrant[]): GroupedGrant[] {
 /** 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`
-	if (grant.objects.length === 1)
-		return `${grant.objects[0].kind.toLowerCase()} ${grant.objects[0].name}`
+	if (grant.objects.length === 1) {
+		const object = grant.objects[0]
+		// A routine's arguments are part of what it is, so two of the same name
+		// would otherwise read as one row twice.
+		const args = object.args !== undefined ? `(${object.args})` : ''
+		return `${object.kind.toLowerCase()} ${object.name}${args}`
+	}
 	if (grant.objects.length > 1)
 		return `${grant.objects.length} ${grant.objects[0].kind.toLowerCase()}s`
 	return 'itself'