Re: Mystery with REVOKE PRIVILEGE

[Konstantin Knizhnik]

On 20/01/2026 9:26 PM, Tom Lane wrote:

Konstantin Knizhnik <knizh...@garret.ru> writes:

I found some very confusing behaviour of REVOKE PRIVILEGE.
...
where <ADMIN_ROLE> is any role with admin permissions under which you
logged in.

"admin permissions" is not well-defined terminology in Postgres.
I thought perhaps you meant "superuser", but some experimentation
indicates that that role doesn't need any special permissions, it
only has to be the table owner to produce the strange behavior.

So granted read privilege was not revoked.

This seems like it may be a consequence of this statement in
the REVOKE man page:

     If the role executing REVOKE holds privileges indirectly via more
     than one role membership path, it is unspecified which containing
     role will be used to perform the command. In such cases it is best
     practice to use SET ROLE to become the specific role you want to
     do the REVOKE as. Failure to do so might lead to revoking
     privileges other than the ones you intended, or not revoking
     anything at all.

In this example, "somebody" is a member of the table owner role
as well as having some privilege granted directly, so the ambiguity
does exist.  I didn't dig further than that.  It does seem sad
if we fail to revoke a privilege that is an exact match, though.

                        regards, tom lane

The problem really seems to be in `select_best_grantor` - it choose "wrong" grantor. It seems to be the bug because as a result of this operation no privilege is revoked at all (because then `merge_acl_with_grant` found no match).

I wonder if `select_best_grantor` should always prefer exact match?

I mean something like this (see attached patch).

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 05d48412f82..5014845e89c 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -5481,6 +5481,7 @@ select_best_grantor(Oid roleId, AclMode privileges,
        List       *roles_list;
        int                     nrights;
        ListCell   *l;
+       AclMode         otherprivs;
 
        /*
         * The object owner is always treated as having all grant options, so if
@@ -5504,6 +5505,17 @@ select_best_grantor(Oid roleId, AclMode privileges,
        roles_list = roles_is_member_of(roleId, ROLERECURSE_PRIVS,
                                                                        
InvalidOid, NULL);
 
+       otherprivs = aclmask_direct(acl, roleId, ownerId,
+                                                               
needed_goptions, ACLMASK_ANY);
+
+       if (otherprivs != 0)
+       {
+               /* Found a suitable grantor */
+               *grantorId = roleId;
+               *grantOptions = otherprivs;
+               return;
+       }
+
        /* initialize candidate result as default */
        *grantorId = roleId;
        *grantOptions = ACL_NO_RIGHTS;
@@ -5512,7 +5524,6 @@ select_best_grantor(Oid roleId, AclMode privileges,
        foreach(l, roles_list)
        {
                Oid                     otherrole = lfirst_oid(l);
-               AclMode         otherprivs;
 
                otherprivs = aclmask_direct(acl, otherrole, ownerId,
                                                                        
needed_goptions, ACLMASK_ALL);