check_circularity does not prevent from creating circular grants
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253365psql -h localhost -U postgresBuilt from patchset v4 (message #4), August 23, 2026 at 11:32 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253365_4 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253365_4 && git checkout t253365_4Patchset v4 (message #4) is on t253365_4
HI!
I discovered a sequence of ddl which creates grant configuration,
unrestorable from pg_dump-pg_restore.
Simpliet repro will be:
CREATE ROLE r1 LOGIN;
CREATE ROLE r2 LOGIN;
CREATE ROLE r3;
GRANT r3 TO r2;
GRANT CREATE ON SCHEMA public TO r1;
SET ROLE r1;
CREATE VIEW v AS SELECT;
GRANT SELECT ON v TO r2 WITH GRANT OPTION; -- r2=r*/r1
GRANT SELECT ON v TO r3 WITH GRANT OPTION; -- r3=r*/r
SET ROLE r2;
GRANT SELECT ON v TO r2 WITH GRANT OPTION; -- r2=r*/r2 -- self
grant, already bad
Now, this is still pg_dump-pg_restore-able, but after we REVOKE r3
from r2 it wouldn't.
From my understanding, the reason is check_circularity tries to get
grantor's independently-derived privileges using aclmask function, but
this function also checks for has_privs_of_role in acl array.
This change fixes the problem for this exact case:
```
reshke@yezzey-cbdb-bench:~/pgpure$ cat p.pa
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index e2547d719ed..6f996cf2439 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -1281,7 +1281,7 @@ cc_restart:
aip = ACL_DAT(acl);
for (i = 0; i < num; i++)
{
- if (aip[i].ai_grantee == mod_aip->ai_grantee &&
+ if (has_privs_of_role(mod_aip->ai_grantee, aip[i].ai_grantee) &&
ACLITEM_GET_GOPTIONS(aip[i]) != ACL_NO_RIGHTS)
{
Acl *new_acl;
```I dont know if this is correct for all possible scenarios thought
--
Best regards,
Kirill Reshke
On 10 Aug 2026, at 12:03, Kirill Reshke <reshkekirill@gmail.com> wrote:
I discovered a sequence of ddl which creates grant configuration,
unrestorable from pg_dump-pg_restore.
The diagnosis looks right, but I think the proposed fix is too broad.
Suppose the owner grants an option directly to role A, role B is a member
of A, and A grants the option to B. This is not circular: after revoking
B's membership in A, the grant from A remains valid. Changing the loop to
has_privs_of_role(B, A) would remove A's independently held option and
reject this case.
select_best_grantor() uses aclmask_direct() instead of aclmask(). WDYT about
this route?
Best regards, Andrey Borodin.
Hi,
On Mon, 10 Aug 2026 at 15:13, Andrey Borodin <x4mmm@yandex-team.ru> wrote:
On 10 Aug 2026, at 12:03, Kirill Reshke <reshkekirill@gmail.com> wrote:
I discovered a sequence of ddl which creates grant configuration,
unrestorable from pg_dump-pg_restore.The diagnosis looks right, but I think the proposed fix is too broad.
Suppose the owner grants an option directly to role A, role B is a member
of A, and A grants the option to B. This is not circular: after revoking
B's membership in A, the grant from A remains valid. Changing the loop to
has_privs_of_role(B, A) would remove A's independently held option and
reject this case.select_best_grantor() uses aclmask_direct() instead of aclmask(). WDYT
about
this route?
There was some prior discussion around this area on [0]/messages/by-id/CAJTYsWUvyQchDAA6y2a9YdLcApG=ccArpsbr77FeNZyx40bnmQ@mail.gmail.com.
Not sure if it's the same block, but just adding reference.
Regards,
Ayush
[0]: /messages/by-id/CAJTYsWUvyQchDAA6y2a9YdLcApG=ccArpsbr77FeNZyx40bnmQ@mail.gmail.com
/messages/by-id/CAJTYsWUvyQchDAA6y2a9YdLcApG=ccArpsbr77FeNZyx40bnmQ@mail.gmail.com
On Mon, 10 Aug 2026 at 15:35, Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:
Hi,
On Mon, 10 Aug 2026 at 15:13, Andrey Borodin <x4mmm@yandex-team.ru> wrote:
On 10 Aug 2026, at 12:03, Kirill Reshke <reshkekirill@gmail.com> wrote:
I discovered a sequence of ddl which creates grant configuration,
unrestorable from pg_dump-pg_restore.The diagnosis looks right, but I think the proposed fix is too broad.
Suppose the owner grants an option directly to role A, role B is a member
of A, and A grants the option to B. This is not circular: after revoking
B's membership in A, the grant from A remains valid. Changing the loop to
has_privs_of_role(B, A) would remove A's independently held option and
reject this case.select_best_grantor() uses aclmask_direct() instead of aclmask(). WDYT about
this route?There was some prior discussion around this area on [0].
Not sure if it's the same block, but just adding reference.
Regards,
Ayush[0] /messages/by-id/CAJTYsWUvyQchDAA6y2a9YdLcApG=ccArpsbr77FeNZyx40bnmQ@mail.gmail.com
v2 with aclmask_direct function used in check_circular + regress test for this.
Ayush, yep i am aware of the thread you are pointing to, I had looked
into it before posting, but it looks like this is a slightly different
case.
Anyway, I want to review [0]/messages/by-id/CAJTYsWUvyQchDAA6y2a9YdLcApG=ccArpsbr77FeNZyx40bnmQ@mail.gmail.com -- Best regards, Kirill Reshke soon, It was on my to-do list. Thank you
[0]: /messages/by-id/CAJTYsWUvyQchDAA6y2a9YdLcApG=ccArpsbr77FeNZyx40bnmQ@mail.gmail.com -- Best regards, Kirill Reshke
--
Best regards,
Kirill Reshke