Preserve GRANTED BY semantics for distributed object privileges
- Dominant language
- C
- Stars
- 12.8k
- Forks
- 794
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 31
Description
## Problem
Citus's manually reconstructed distributed-table `GRANT`/`REVOKE` commands omit `GrantStmt.grantor` (`GRANTED BY`). Coordinator and shard ACL grantors can therefore diverge, and a matching `REVOKE ... GRANTED BY` can remove the coordinator ACL while leaving shard ACLs intact.
This is a pre-existing distributed authorization correctness bug, not a PostgreSQL 19 parser gap. PostgreSQL 19 widens where `GRANTED BY` is accepted, but the distributed table deparser already loses the field.
## Verified reproduction
- `GRANT ... GRANTED BY gap_grantor` records `gap_grantor` on the coordinator but the current executing user on both shards.
- After correcting shard grantors manually, `REVOKE ... GRANTED BY gap_grantor` removes the coordinator ACL but leaves shard ACLs.
- Generic object and role deparsers preserve the grantor; the distributed-table path in `grant.c` is the affected special case.
## Acceptance criteria
- Distributed-table GRANT/REVOKE preserves explicit `GRANTED BY` on coordinator and all shards.
- Authorization and role-membership checks match PostgreSQL behavior.
- Quoted role names and keyword/current-user forms are handled safely.
- Multi-node tests assert identical ACL grantor and revoke results across coordinator/workers.
- Existing GRANT/REVOKE forms without `GRANTED BY` remain unchanged on supported PostgreSQL versions.
## Additional verified defect: shared-object REVOKE clause ordering
`AppendGrantSharedSuffix()` emits `RESTRICT`/`CASCADE` before `GRANTED BY`, but PostgreSQL grammar requires:
```sql
REVOKE ... FROM ... [GRANTED BY ...] [CASCADE|RESTRICT]
```
Generated worker SQL therefore fails at `GRANTED BY` for schema, database, sequence, function/procedure/routine/aggregate, FDW, and foreign-server privileges. GRANT propagation succeeds; REVOKE fails atomically and leaves coordinator/workers unchanged.
This malformed ordering exists on PG16–19. PG19 makes the table-path omission more severe by allowing an inherited eligible grantor and scoping REVOKE to that exact grantor.
## Minimum implementation plan
1. In `AppendGrantSharedSuffix()`, emit `GRANTED BY` before `RESTRICT`/`CASCADE`.
2. In the manually reconstructed table path:
- GRANT: emit grantor after `WITH GRANT OPTION`.
- REVOKE: emit grantor after grantees and before `CASCADE`/`RESTRICT`.
3. Reuse the existing role-spec serializer; no version gate or broad refactor is needed.
4. Add PG19 table coverage for quoted inherited grantors, grant options, physical-shard grantor equality, and complete REVOKE.
5. Add compact cross-version PG16–19 coverage for all shared-object callers and suffix combinations.
## Runtime evidence
- Distributed table GRANT records the requested grantor on the coordinator but the executing user on physical shards; matching REVOKE leaves shard ACLs behind.
- Shared-object REVOKE produces invalid worker SQL such as `... RESTRICT GRANTED BY ...`.
- Implicit/explicit RESTRICT, CASCADE, WITH GRANT OPTION, and GRANT OPTION FOR were reproduced.
- Role-membership GRANT/REVOKE already emits the correct order and is unaffected.
## Precise PG19 table reproduction
A table owned by the explicit grantor is not a discriminating test because PostgreSQL workers independently choose that same role as the best grantor. The verified divergent case requires the named inherited grantor to differ from the best grantor:
- Table owned by a third role.
- `role_a` and `role_b` both hold grant option.
- Login `grant_actor` inherits both.
- `GRANT ... GRANTED BY role_b` records `role_b` on the coordinator but unfixed shards independently choose `role_a`.
- Selective grants/revokes by role_a/role_b produce real coordinator/shard privilege divergence (`has_table_privilege` can be true on coordinator and false on shards, or the reverse).
The table fix should serialize the explicit grantor in the grammar-correct slot: after `WITH GRANT OPTION` for GRANT, and after grantees but before `CASCADE|RESTRICT` for REVOKE. It compiles cross-version but needs PG19 regression coverage because inherited alternative grantors become expressible there.
Implementation remains split into two independently reviewable changes:
1. Cross-version shared-object REVOKE suffix ordering targeting `main`.
2. PG19 table grantor serialization and regression targeting `pg19-support`.
## Accepted shared-object ordering implementation
Local commit `d866381b1` targets `main` and changes 3 files (+48/-1): one product-line ordering fix in `citus_grantutils.c` plus schema propagation regression coverage.
It emits `GRANTED BY` before `CASCADE|RESTRICT`, matching PostgreSQL GRANT/REVOKE grammar. Runtime negative control produced worker syntax errors before the fix and clean coordinator/worker ACL parity after it.
Independent release review found no significant issues across GRANT/REVOKE, grant options, behavior clauses, supported shared-object callers, role quoting, and PG16–19 compatibility.
The table grantor-serialization implementation remains a separate PG19-support change and is not part of this commit.
## Accepted table-grantor implementation
Local PG19-support commit `7bff7063a308cdc722a91a8e50092fcf1e7e3bc1` changes `commands/grant.c` plus PG19 regression coverage (3 files, +305/-4). It serializes explicit table grantors in grammar-correct GRANT/REVOKE slots.
The discriminating test uses a third-party owner and two inherited eligible grantors so the explicit grantor differs from PostgreSQL's independently selected worker grantor. Stock negative control flips four physical-shard assertions: grantor identity, selective-revoke privilege parity, quoted grantor/grant option, and revoke-grant-option identity. Coordinator rows remain unchanged, proving the worker propagation defect.
Independent release review found no significant issues. This remains separate from main PR #8766, which fixes shared-object REVOKE suffix ordering and intentionally only tracks this combined issue.
Implementation PRs:
- #8766 — shared-object ordering on `main` (Tracks #8759; must not close it alone)
- table-grantor PR — pending publication against `pg19-support`
Implementation PR: #8767 (table grantor propagation on pg19-support).
Contributor guide
Research direction
The issue describes fixes in `citus_grantutils.c` for shared-object suffix ordering and `commands/grant.c` for distributed-table grantor serialization, with regression coverage for PostgreSQL 16–19. Read the relevant grant propagation code and existing regression tests; however, implementation PRs #8766 and #8767 already address the work, so this is not an available first issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, postgresql
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 1/100