From d97adf102c212f937b64e0bf63641531a715e72d Mon Sep 17 00:00:00 2001 From: Huseyin Demir Date: Sun, 2 Aug 2026 11:10:23 +0200 Subject: [PATCH] pg_dump: skip pg_init_privs entries for non-existent roles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before commit 53428740391, PostgreSQL did not record role dependencies for pg_init_privs entries in pg_shdepend, meaning DROP ROLE could leave behind ACL entries whose grantee or grantor OID no longer exists in pg_authid. Cross-cluster restores can produce the same situation on modern releases. Dangling entries caused pg_dump to emit invalid SQL such as "GRANT ... TO "87868"" with a numeric OID as role name, which fails on restore or pg_upgrade. Additionally, a column that was never explicitly granted anything (its actual attacl stays NULL) but carries an all-dangling pg_init_privs entry could produce a spurious "REVOKE ... FROM "87868"" naming the dangling OID, because the column-ACL query picks up a column whenever it has any pg_init_privs row, regardless of whether attacl is NULL. (The analogous scenario for a plain function with proacl NULL cannot occur: pg_dump only considers a function for ACL output at all when its actual ACL is non-NULL, independent of pg_init_privs.) Fix by filtering each aclitem whose grantor or non-PUBLIC grantee OID does not appear in pg_roles (used instead of pg_authid to support non-superuser pg_dump). The filtering is applied server-side in the queries that fetch pg_init_privs data (getAdditionalACLs and the column-level ACL prepared statement), where the OID check is authoritative. This avoids modifying the WHERE clauses in getAggregates/getFuncs, preserving pg_dump performance on large schemas. If all entries for an object are dangling the result is NULL and no ACL is emitted, which is correct -- we cannot restore grants involving roles that do not exist. Testing: 008_pg_dump_dangling_initprivs.pl covers dangling grantee and grantor on a function, a column with a real grant mixed with a dangling entry, a never-granted column with an all-dangling entry, a legitimate all-digit role name (must not be mistaken for a dangling OID), the PUBLIC grantee case in both directions (valid and dangling grantor), the same dangling-grantee scenario on an aggregate (getAggregates, a separate query from getFuncs), and a run under --binary-upgrade (which has its own ACL-reconstruction path in dumpACL not exercised by a plain dump). Every case is mutation-tested: each fails when the corresponding piece of the fix is reverted, and only then. Rui Zhao identified that the original test passed even with the fix reverted (every test object kept its default ACL, so pg_dump emitted nothing regardless) and contributed the fix: quote_ident() on the current_user splices, a non-default ACL on each test object, a widened catch-all assertion, and the PUBLIC-grantee coverage. Author: Hüseyin Demir Author: Rui Zhao Discussion: https://postgr.es/m/19483-80de42dc4e62cfd6%40postgresql.org Backpatch-through: 14 --- src/bin/pg_dump/pg_dump.c | 37 ++- .../t/008_pg_dump_dangling_initprivs.pl | 227 ++++++++++++++++++ 2 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 4948e6d80c7..14f41b928dc 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -237,6 +237,35 @@ static int nsequences = 0; fmtQualifiedId((obj)->dobj.namespace->dobj.name, \ (obj)->dobj.name) +/* + * SQL expression that filters dangling role OIDs from a pg_init_privs + * aclitem[] column. An aclitem is dangling when its grantor or non-PUBLIC + * grantee OID no longer exists in pg_roles. We use pg_roles rather than + * pg_authid so that non-superuser pg_dump works. + * + * Applied only in queries that fetch pg_init_privs data (not in WHERE clauses + * of per-object queries) to avoid running this subquery per function/aggregate. + */ +#define SAFE_INITPRIVS(col) \ + "NULLIF(\n" \ + " ARRAY(\n" \ + " SELECT elt FROM pg_catalog.unnest(" col ") AS elt\n" \ + " WHERE NOT EXISTS (\n" \ + " SELECT 1 FROM pg_catalog.aclexplode(ARRAY[elt]) ace\n" \ + " WHERE NOT EXISTS (\n" \ + " SELECT 1 FROM pg_catalog.pg_roles\n" \ + " WHERE oid = ace.grantor\n" \ + " )\n" \ + " OR (ace.grantee <> 0\n" \ + " AND NOT EXISTS (\n" \ + " SELECT 1 FROM pg_catalog.pg_roles\n" \ + " WHERE oid = ace.grantee\n" \ + " ))\n" \ + " )\n" \ + " ),\n" \ + " ARRAY[]::pg_catalog.aclitem[]\n" \ + ")" + static void help(const char *progname); static void setup_connection(Archive *AH, const char *dumpencoding, const char *dumpsnapshot, @@ -10748,8 +10777,9 @@ getAdditionalACLs(Archive *fout) /* Fetch initial-privileges data */ printfPQExpBuffer(query, - "SELECT objoid, classoid, objsubid, privtype, initprivs " - "FROM pg_init_privs"); + "SELECT pip.objoid, pip.classoid, pip.objsubid, pip.privtype,\n" + " " SAFE_INITPRIVS("pip.initprivs") " AS initprivs\n" + "FROM pg_catalog.pg_init_privs pip"); res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); @@ -16892,7 +16922,8 @@ dumpTable(Archive *fout, const TableInfo *tbinfo) "SELECT at.attname, " "at.attacl, " "'{}' AS acldefault, " - "pip.privtype, pip.initprivs " + "pip.privtype,\n" + " " SAFE_INITPRIVS("pip.initprivs") " AS initprivs\n" "FROM pg_catalog.pg_attribute at " "LEFT JOIN pg_catalog.pg_init_privs pip ON " "(at.attrelid = pip.objoid " diff --git a/src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl b/src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl new file mode 100644 index 00000000000..5fcfc29a310 --- /dev/null +++ b/src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl @@ -0,0 +1,227 @@ +# Copyright (c) 2024-2026, PostgreSQL Global Development Group +# +# Tests that pg_dump silently skips pg_init_privs entries that reference +# roles no longer present in pg_authid, rather than emitting invalid GRANT +# statements with numeric OIDs as role names. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; +$node->start; + +$node->safe_psql('postgres', 'CREATE DATABASE regress_dangling'); + +# --- Setup --- +# Simulate dangling pg_init_privs entries by inserting grants for a role +# and then deleting the role directly from pg_authid (bypassing pg_shdepend). +$node->safe_psql( + 'regress_dangling', + q{ +SET allow_system_table_mods = true; + +-- Roles for testing +CREATE ROLE ghost_grantee; +CREATE ROLE ghost_grantor; +CREATE ROLE "007"; +CREATE ROLE regress_col_grantee; +CREATE ROLE ghost_col_revoke; + +-- Case 1: dangling grantee (function) +CREATE FUNCTION public.test_func_grantee() RETURNS int LANGUAGE sql AS 'SELECT 1'; +REVOKE ALL ON FUNCTION public.test_func_grantee() FROM PUBLIC; +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT p.oid, + (SELECT oid FROM pg_class WHERE relname = 'pg_proc'), + 0, 'e', + ARRAY[('ghost_grantee=X/' || quote_ident(current_user))::aclitem] +FROM pg_proc p +WHERE p.proname = 'test_func_grantee' + AND p.pronamespace = 'public'::regnamespace; + +-- Case 2: dangling grantor (function) +CREATE FUNCTION public.test_func_grantor() RETURNS int LANGUAGE sql AS 'SELECT 2'; +REVOKE ALL ON FUNCTION public.test_func_grantor() FROM PUBLIC; +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT p.oid, + (SELECT oid FROM pg_class WHERE relname = 'pg_proc'), + 0, 'e', + ARRAY[(quote_ident(current_user) || '=X/ghost_grantor')::aclitem] +FROM pg_proc p +WHERE p.proname = 'test_func_grantor' + AND p.pronamespace = 'public'::regnamespace; + +-- Case 3: dangling column-level grantee (table) +CREATE TABLE public.test_tbl (id int, secret text); +GRANT SELECT (secret) ON public.test_tbl TO regress_col_grantee; +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT c.oid, + (SELECT oid FROM pg_class WHERE relname = 'pg_class'), + 2, 'e', + ARRAY[('ghost_grantee=r/' || quote_ident(current_user))::aclitem] +FROM pg_class c +WHERE c.relname = 'test_tbl' + AND c.relnamespace = 'public'::regnamespace; + +-- Case 4: a column that was *never* explicitly granted anything (its +-- actual attacl stays NULL) but carries an all-dangling pg_init_privs +-- entry. The column-ACL query picks up a column whenever it has *any* +-- pg_init_privs row, regardless of whether attacl is NULL, so this is a +-- genuinely reachable "spurious REVOKE" path -- unlike a plain function +-- with NULL proacl, which pg_dump never even considers for ACL output +-- (DUMP_COMPONENT_ACL is only set there when the actual ACL is non-NULL). +CREATE TABLE public.test_tbl_nullacl (id int, secret text); +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT c.oid, + (SELECT oid FROM pg_class WHERE relname = 'pg_class'), + 2, 'e', + ARRAY[('ghost_col_revoke=r/' || quote_ident(current_user))::aclitem] +FROM pg_class c +WHERE c.relname = 'test_tbl_nullacl' + AND c.relnamespace = 'public'::regnamespace; + +-- Case 5: valid all-digit role "007" with a grant (must NOT be filtered) +CREATE FUNCTION public.test_func_007() RETURNS int LANGUAGE sql AS 'SELECT 7'; +GRANT EXECUTE ON FUNCTION public.test_func_007() TO "007"; + +-- Case 6: PUBLIC grant whose grantor is dangling (must be filtered) +CREATE FUNCTION public.test_func_public_ghost() RETURNS int LANGUAGE sql AS 'SELECT 6'; +REVOKE ALL ON FUNCTION public.test_func_public_ghost() FROM PUBLIC; +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT p.oid, + (SELECT oid FROM pg_class WHERE relname = 'pg_proc'), + 0, 'e', + ARRAY['=X/ghost_grantor'::aclitem] +FROM pg_proc p +WHERE p.proname = 'test_func_public_ghost' + AND p.pronamespace = 'public'::regnamespace; + +-- Case 7: PUBLIC grant with a valid grantor (must NOT be filtered) +CREATE FUNCTION public.test_func_public_ok() RETURNS int LANGUAGE sql AS 'SELECT 7'; +REVOKE ALL ON FUNCTION public.test_func_public_ok() FROM PUBLIC; +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT p.oid, + (SELECT oid FROM pg_class WHERE relname = 'pg_proc'), + 0, 'e', + ARRAY[('=X/' || quote_ident(current_user))::aclitem] +FROM pg_proc p +WHERE p.proname = 'test_func_public_ok' + AND p.pronamespace = 'public'::regnamespace; + +-- Case 8: dangling grantee on an aggregate. Aggregates are stored in +-- pg_proc like plain functions and share the same ACL-diff machinery, but +-- they are fetched by a separate query (getAggregates(), not getFuncs()), +-- so this exercises a genuinely different code path than case 1. +CREATE AGGREGATE public.test_agg_grantee (int4) (SFUNC = int4pl, STYPE = int4, INITCOND = '0'); +REVOKE ALL ON FUNCTION public.test_agg_grantee(int4) FROM PUBLIC; +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT p.oid, + (SELECT oid FROM pg_class WHERE relname = 'pg_proc'), + 0, 'e', + ARRAY[('ghost_grantee=X/' || quote_ident(current_user))::aclitem] +FROM pg_proc p +WHERE p.proname = 'test_agg_grantee' + AND p.pronamespace = 'public'::regnamespace; + +-- Now delete the ghost roles to create dangling OIDs +DELETE FROM pg_authid WHERE rolname = 'ghost_grantee'; +DELETE FROM pg_authid WHERE rolname = 'ghost_grantor'; +DELETE FROM pg_authid WHERE rolname = 'ghost_col_revoke'; + + +}); + +my $tempdir = PostgreSQL::Test::Utils::tempdir; +my $dump_file = "$tempdir/dangling.sql"; + +# pg_dump must succeed even with dangling pg_init_privs entries. +command_ok( + [ + 'pg_dump', + '--port' => $node->port, + '--schema-only', + '-f' => $dump_file, + 'regress_dangling', + ], + 'pg_dump succeeds with dangling pg_init_privs entries'); + +my $dump = slurp_file($dump_file); + +# --- Case 1: dangling grantee --- +like($dump, qr/CREATE FUNCTION public\.test_func_grantee/, + 'case 1: function is present in dump'); +unlike($dump, qr/GRANT\b.*\btest_func_grantee/, + 'case 1: no GRANT for function with dangling grantee'); + +# --- Case 2: dangling grantor --- +like($dump, qr/CREATE FUNCTION public\.test_func_grantor/, + 'case 2: function is present in dump'); +unlike($dump, qr/GRANT\b.*\btest_func_grantor/, + 'case 2: no GRANT for function with dangling grantor'); + +# --- Case 3: column-level dangling --- +like($dump, qr/CREATE TABLE public\.test_tbl/, + 'case 3: table is present in dump'); +unlike($dump, qr/REVOKE\b.*\btest_tbl\b/, + 'case 3: no column-level REVOKE naming a dangling OID'); + +# --- Case 4: never-granted column with all-dangling initprivs --- +like($dump, qr/CREATE TABLE public\.test_tbl_nullacl/, + 'case 4: table is present in dump'); +unlike($dump, qr/REVOKE\b.*\btest_tbl_nullacl\b/, + 'case 4: no spurious REVOKE for never-granted column with all-dangling initprivs'); + +# --- Case 5: valid all-digit role "007" --- +like($dump, qr/CREATE FUNCTION public\.test_func_007/, + 'case 5: function is present in dump'); +like($dump, qr/GRANT\b.*\btest_func_007\b.*TO\s+"007"/, + 'case 5: GRANT to valid all-digit role "007" is preserved'); + +# --- General: no numeric OID as role name (other than the valid "007") --- +# Match any GRANT/REVOKE naming a role as "digits", except the valid "007". +unlike($dump, qr/^(?:GRANT|REVOKE)\b.*\b(?:TO|FROM)\s+"(?!007")[0-9]+"/m, + 'no GRANT/REVOKE with bare numeric OID as role name (other than valid "007")'); + +# --- Case 6: PUBLIC entry with dangling grantor is filtered --- +unlike($dump, qr/GRANT\b.*\btest_func_public_ghost/, + 'case 6: no GRANT for PUBLIC entry with dangling grantor'); + +# --- Case 7: PUBLIC entry with valid grantor survives the filter --- +like($dump, qr/GRANT\b.*\btest_func_public_ok/, + 'case 7: PUBLIC entry with valid grantor is preserved'); + +# --- Case 8: dangling grantee on an aggregate (getAggregates() path) --- +like($dump, qr/CREATE AGGREGATE public\.test_agg_grantee/, + 'case 8: aggregate is present in dump'); +unlike($dump, qr/GRANT\b.*\btest_agg_grantee/, + 'case 8: no GRANT for aggregate with dangling grantee'); + +# --- Case 9: --binary-upgrade has its own ACL-reconstruction code path in +# dumpACL() (the binary_upgrade_set_record_init_privs() preamble) that a +# plain dump never exercises, so it needs its own check against the same +# dangling data. --- +my $dump_file_bu = "$tempdir/dangling_binary_upgrade.sql"; +command_ok( + [ + 'pg_dump', + '--port' => $node->port, + '--schema-only', + '--binary-upgrade', + '-f' => $dump_file_bu, + 'regress_dangling', + ], + 'pg_dump --binary-upgrade succeeds with dangling pg_init_privs entries'); + +my $dump_bu = slurp_file($dump_file_bu); + +unlike( + $dump_bu, + qr/^(?:GRANT|REVOKE)\b.*\b(?:TO|FROM)\s+"(?!007")[0-9]+"/m, + 'case 9: no GRANT/REVOKE with bare numeric OID under --binary-upgrade'); + +done_testing(); -- 2.50.1 (Apple Git-155)