Thread

  1. REVOKE's CASCADE protection doesn't work with INHERITed table owners

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-24T21:57:13Z

    Hi all,
    
    I'm pretty sure the following is unintended behavior. It looks
    potentially related to [1] as well.
    
    TL;DR: The protection in recursive_revoke() against broken GRANT
    OPTION chains doesn't seem to work properly when the grantee also
    holds the privileges of the grantor.
    
    = Setup =
    
    With the following (contrived, minimized from the actual) setup:
    
        -- Set up a role hierarchy (as superuser "jacob")
        CREATE ROLE admins;
        CREATE ROLE bob;
        GRANT admins TO bob;
    
        -- Create two tables with different ownership
        CREATE TABLE my_table(i int);
        CREATE TABLE admin_table(i int);
        ALTER TABLE admin_table OWNER TO admins;
    
        -- Create a grant option chain on both tables
        GRANT ALL ON TABLE my_table TO bob WITH GRANT OPTION;
        GRANT ALL ON TABLE admin_table TO bob WITH GRANT OPTION;
    
        SET ROLE bob;
        GRANT ALL ON TABLE my_table TO bob;
        GRANT ALL ON TABLE admin_table TO bob;
        RESET ROLE;
    
    Now we have the following ACLs:
    
        =# SELECT relname, relowner::regrole, relacl FROM pg_class
            WHERE relname LIKE '%_table';
        -[ RECORD 1
    ]-------------------------------------------------------------------
        relname  | my_table
        relowner | jacob
        relacl   | {jacob=arwdDxtm/jacob,bob=a*r*w*d*D*x*t*m*/jacob,bob=arwdDxtm/bob}
        -[ RECORD 2
    ]-------------------------------------------------------------------
        relname  | admin_table
        relowner | admins
        relacl   | {admins=arwdDxtm/admins,bob=a*r*w*d*D*x*t*m*/admins,bob=arwdDxtm/bob}
    
    = Bug =
    
    With that grant option chain, we try to prevent REVOKE [RESTRICT]
    invocations that would cause problems:
    
        =# REVOKE ALL ON TABLE my_table FROM bob;
        ERROR:  dependent privileges exist
        HINT:  Use CASCADE to revoke them too.
    
    But this protection doesn't work for the admin_table...
    
        =# REVOKE ALL ON TABLE admin_table FROM bob;
        REVOKE
    
    ...resulting in an orphaned ACL.
    
        -[ RECORD 2
    ]----------------------------------------------------------------
        relname  | admin_table
        relowner | admins
        relacl   | {admins=arwdDxtm/admins,bob=arwdDxtm/bob}
    
    Dump/restores of this situation result in complaints, since user "bob"
    isn't able to recreate the grant.
    
    I think the issue is in recursive_revoke()'s usage of aclmask(), which
    in turn uses has_privs_of_role(). It doesn't seem like that's what was
    wanted in this particular case... thoughts?
    
    Thanks,
    --Jacob
    
    [1] https://postgr.es/m/CAM6Zo8wD7RtQNhbQHODc9DobiW+GpT=tnqOSMz4+mnzA9m0zMg@mail.gmail.com
    
    
    
    
  2. Re: REVOKE's CASCADE protection doesn't work with INHERITed table owners

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-26T00:12:42Z

    [moving to -hackers]
    
    On Wed, Jun 24, 2026 at 2:57 PM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    > TL;DR: The protection in recursive_revoke() against broken GRANT
    > OPTION chains doesn't seem to work properly when the grantee also
    > holds the privileges of the grantor.
    
    More accurately: "when an intermediate grantor in the chain only
    indirectly holds the ability to grant."
    
    > I think the issue is in recursive_revoke()'s usage of aclmask(), which
    > in turn uses has_privs_of_role(). It doesn't seem like that's what was
    > wanted in this particular case... thoughts?
    
    I propose changing that to aclmask_direct(), as in the attached, and
    backpatching all the way down.
    
    To try to prove to myself that this works, I added tests to pin each
    of the three cases that are treated differently by aclmask_direct():
    1. the grantor has indirect ownership privileges
    2. the grantor has indirect grant options via INHERIT
    3. the grantor has indirect grant options via PUBLIC (which is already
    disallowed in practice)
    
    I also tried to expand the existing comment, both to point out the
    pitfall and to explain why the short-circuit works. But I've rewritten
    it at least a dozen times, so if anyone can tell me whether I've made
    sense and/or used the terminology appropriately, I'd appreciate it.
    
    > I'm pretty sure the following is unintended behavior. It looks
    > potentially related to [1] as well.
    
    (To fix [1] I suspect we need to make a similar tweak to
    check_circularity(), but I haven't looked into that yet.)
    
    Thanks!
    --Jacob
    
    [1] https://postgr.es/m/CAM6Zo8wD7RtQNhbQHODc9DobiW+GpT=tnqOSMz4+mnzA9m0zMg@mail.gmail.com
    
  3. Re: REVOKE's CASCADE protection doesn't work with INHERITed table owners

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-07-06T11:03:03Z

    Hi,
    
    On Fri, 26 Jun 2026 at 05:43, Jacob Champion <
    jacob.champion@enterprisedb.com> wrote:
    
    > [moving to -hackers]
    >
    > On Wed, Jun 24, 2026 at 2:57 PM Jacob Champion
    > <jacob.champion@enterprisedb.com> wrote:
    > > TL;DR: The protection in recursive_revoke() against broken GRANT
    > > OPTION chains doesn't seem to work properly when the grantee also
    > > holds the privileges of the grantor.
    >
    > More accurately: "when an intermediate grantor in the chain only
    > indirectly holds the ability to grant."
    >
    > > I think the issue is in recursive_revoke()'s usage of aclmask(), which
    > > in turn uses has_privs_of_role(). It doesn't seem like that's what was
    > > wanted in this particular case... thoughts?
    >
    > I propose changing that to aclmask_direct(), as in the attached, and
    > backpatching all the way down.
    >
    > To try to prove to myself that this works, I added tests to pin each
    > of the three cases that are treated differently by aclmask_direct():
    > 1. the grantor has indirect ownership privileges
    > 2. the grantor has indirect grant options via INHERIT
    > 3. the grantor has indirect grant options via PUBLIC (which is already
    > disallowed in practice)
    >
    
    Thanks, this looks right to me.  I traced recursive_revoke() and the
    aclmask() -> aclmask_direct() switch makes sense to me, since a
    grant is only ever attributed to a role that holds the option directly
    (or the owner), it seems right that recursive_revoke() should judge
    "still has it" the same way, rather than counting inherited/superuser
    access?  The three new test cases line up with that too.
    
    I also tried to expand the existing comment, both to point out the
    > pitfall and to explain why the short-circuit works. But I've rewritten
    > it at least a dozen times, so if anyone can tell me whether I've made
    > sense and/or used the terminology appropriately, I'd appreciate it.
    
    
    One tiny comment question: the phrase "granted by any role on the chain"
    in the new comment reads a little oddly to me, would something like
    "still holds the option directly via another grantor" be closer to what
    the code checks?  Could just be me misreading it.
    
    Apart from the above, patch LGTM.
    
    > I'm pretty sure the following is unintended behavior. It looks
    > > potentially related to [1] as well.
    >
    > (To fix [1] I suspect we need to make a similar tweak to
    > check_circularity(), but I haven't looked into that yet.)
    >
    > [1]
    > https://postgr.es/m/CAM6Zo8wD7RtQNhbQHODc9DobiW+GpT=tnqOSMz4+mnzA9m0zMg@mail.gmail.com
    
    
    On check_circularity() for [1]: I tried the same aclmask_direct() swap,
    but since it runs on every GRANT ... WITH GRANT OPTION, which
    pg_dump/restore replays, erroring there could make restore/pg_upgrade of
    an existing cluster (one already holding the [1] self-grant) fail. Feels
    like it may need a companion dump/restore change first.
    
    Regards,
    Ayush