From 912db0194bcc8f7094feb22d28a5f05e634dc9d2 Mon Sep 17 00:00:00 2001 From: Huseyin Demir Date: Fri, 19 Jun 2026 07:39:22 +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 pg_init_privs, introduced in PostgreSQL 9.6, records the "initial" privilege state of objects — either set by initdb (privtype 'i') or by CREATE EXTENSION scripts (privtype 'e'). pg_dump reads this catalog to determine which ACL changes the user made on top of the defaults, so it can emit the right GRANT/REVOKE statements. 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 (dumping from one system where a role existed, restoring to another where it does not) can produce the same situation even on modern releases. The old code passed these dangling aclitem entries through unchanged. A dangling grantee caused pg_dump to emit statements such as GRANT EXECUTE ON FUNCTION foo() TO "87868"; where "87868" is a numeric OID — invalid SQL that fails on restore. A dangling grantor caused pg_dump to emit SET SESSION AUTHORIZATION "87868"; before the GRANT, which likewise fails on restore or pg_upgrade. There is a second, more subtle bug: getAggregates() and getFuncs() use pip.initprivs unfiltered in their WHERE clause to decide whether an object needs ACL output at all. When all initprivs entries for an object are dangling and the live proacl is NULL, the comparison p.proacl IS DISTINCT FROM pip.initprivs evaluates to true (NULL IS DISTINCT FROM ), causing the object to be selected for ACL output. After getAdditionalACLs() returns NULL for the now-empty initprivs, dumpACL() computes the delta from acldefault to NULL and emits a spurious REVOKE that breaks restore. Fix by filtering each aclitem whose grantor or non-PUBLIC grantee OID does not appear in pg_authid, and by applying that filter at all four sites where pg_dump queries pg_init_privs: - getAggregates(): WHERE clause comparison - getFuncs(): WHERE clause comparison - getAdditionalACLs(): SELECT expression (object-level initprivs) - PREPQUERY_GETCOLUMNACLS: SELECT expression (column-level initprivs, objsubid != 0) To avoid duplicating the multi-line subquery at every call site, define a SAFE_INITPRIVS(col) macro that expands to the filtering expression via C adjacent string literal concatenation. 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. A similar issue in pg_dumpall was fixed by commit 74b4438a70b. This bug has existed since pg_init_privs was introduced in 9.6. Backpatch to 14 (oldest currently supported branch). Author: Huseyin Demir Discussion: https://postgr.es/m/19483-80de42dc4e62cfd6%40postgresql.org Backpatch-through: 14 --- src/bin/pg_dump/pg_dump.c | 58 ++++++- .../t/008_pg_dump_dangling_initprivs.pl | 161 ++++++++++++++++++ 2 files changed, 211 insertions(+), 8 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 a0f7f8e2168..7190691ca77 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -237,6 +237,44 @@ static int nsequences = 0; fmtQualifiedId((obj)->dobj.namespace->dobj.name, \ (obj)->dobj.name) +/* + * SQL fragment that strips dangling role OIDs from a pg_init_privs aclitem[] + * column. An entry is dangling when its grantor or non-PUBLIC grantee no + * longer exists in pg_authid. Before commit 53428740391 pg_shdepend did not + * record dependencies for pg_init_privs, so DROP ROLE could silently leave + * behind such entries; cross-cluster restores can produce the same situation + * on modern releases. Passing dangling entries through causes pg_dump to + * emit "SET SESSION AUTHORIZATION " or "GRANT ... TO " with a + * numeric OID as role name, which fails on restore. We silently drop any + * entry whose grantor or grantee is missing; if all entries are removed the + * result is NULL. + * + * The argument must be a compile-time string literal naming the aclitem[] + * column (always "pip.initprivs" in practice). C adjacent string literal + * concatenation splices the expansion into the surrounding query string. + */ +#define SAFE_INITPRIVS(col) \ + "NULLIF(\n" \ + " ARRAY(\n" \ + " SELECT elt FROM pg_catalog.unnest(" col ") AS elt\n" \ + " /* keep only valid entries, i.e. those without... */\n" \ + " WHERE NOT EXISTS (\n" \ + " SELECT 1 FROM pg_catalog.aclexplode(ARRAY[elt]) ace\n" \ + " /* ...a non-existing grantor... */\n" \ + " WHERE NOT EXISTS (\n" \ + " SELECT 1 FROM pg_catalog.pg_roles AS r1\n" \ + " WHERE r1.oid = ace.grantor\n" \ + " )\n" \ + " /* ...or a non-existing grantee (0 means PUBLIC) */\n" \ + " OR ace.grantee <> 0\n" \ + " AND NOT EXISTS (\n" \ + " SELECT 1 FROM pg_catalog.pg_roles AS r2\n" \ + " WHERE r2.oid = ace.grantee\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, @@ -6906,7 +6944,8 @@ getAggregates(Archive *fout) "p.pronamespace != " "(SELECT oid FROM pg_namespace " "WHERE nspname = 'pg_catalog') OR " - "p.proacl IS DISTINCT FROM pip.initprivs", + "p.proacl IS DISTINCT FROM " + SAFE_INITPRIVS("pip.initprivs"), agg_check); if (dopt->binary_upgrade) appendPQExpBufferStr(query, @@ -7088,7 +7127,8 @@ getFuncs(Archive *fout) "refclassid = 'pg_extension'::regclass AND " "deptype = 'e')"); appendPQExpBufferStr(query, - "\n OR p.proacl IS DISTINCT FROM pip.initprivs"); + "\n OR p.proacl IS DISTINCT FROM " + SAFE_INITPRIVS("pip.initprivs")); appendPQExpBufferChar(query, ')'); } else @@ -10900,8 +10940,9 @@ getAdditionalACLs(Archive *fout) if (fout->remoteVersion >= 90600) { 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); @@ -17097,10 +17138,11 @@ dumpTable(Archive *fout, const TableInfo *tbinfo) * and risk to avoid hard-wiring that knowledge here. */ appendPQExpBufferStr(query, - "SELECT at.attname, " - "at.attacl, " - "'{}' AS acldefault, " - "pip.privtype, pip.initprivs " + "SELECT at.attname,\n" + "at.attacl,\n" + "'{}' AS acldefault,\n" + "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..a0ba3650989 --- /dev/null +++ b/src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl @@ -0,0 +1,161 @@ +# 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 +# or SET SESSION AUTHORIZATION statements with numeric OIDs as role names. +# +# Four cases are tested: +# 1. Dangling grantee -- the role that received the privilege was dropped. +# 2. Dangling grantor -- the role that granted the privilege was dropped +# (this caused broken SET SESSION AUTHORIZATION output). +# 3. Column-level ACL -- same dangling-grantee scenario but for a column +# initprivs entry (objsubid != 0). +# 4. Spurious REVOKE -- a pg_catalog function whose only initprivs entry is +# all-dangling; without filtering the WHERE clause in +# getFuncs() the object is incorrectly selected for ACL +# output and dumpACL() emits a spurious REVOKE. + +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'); + +# Set up all four dangling cases in one session. We cannot use normal +# DROP ROLE because pg_shdepend would block it, so we delete roles directly +# from pg_authid with allow_system_table_mods. +$node->safe_psql( + 'regress_dangling', + q{ +SET allow_system_table_mods = true; + +-- Case 1: dangling grantee. +-- ghost_grantee received EXECUTE; after deletion its OID becomes dangling. +CREATE ROLE ghost_grantee; + +CREATE FUNCTION public.test_func_grantee() RETURNS int LANGUAGE sql AS 'SELECT 1'; + +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT p.oid, + (SELECT oid FROM pg_catalog.pg_class WHERE relname = 'pg_proc'), + 0, + 'e', + ARRAY[('ghost_grantee=X/' || current_user)::aclitem] +FROM pg_catalog.pg_proc p +WHERE p.proname = 'test_func_grantee' +AND p.pronamespace = 'public'::regnamespace; + +-- Case 2: dangling grantor. +-- ghost_grantor is the grantor; after deletion its OID appears in +-- SET SESSION AUTHORIZATION output, causing restore failures. +CREATE ROLE ghost_grantor; + +CREATE FUNCTION public.test_func_grantor() RETURNS int LANGUAGE sql AS 'SELECT 2'; + +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT p.oid, + (SELECT oid FROM pg_catalog.pg_class WHERE relname = 'pg_proc'), + 0, + 'e', + ARRAY[('=X/' || 'ghost_grantor')::aclitem] +FROM pg_catalog.pg_proc p +WHERE p.proname = 'test_func_grantor' +AND p.pronamespace = 'public'::regnamespace; + +-- Case 3: column-level dangling grantee (objsubid != 0). +-- ghost_col_role received SELECT on a column; after deletion its OID +-- would appear in a GRANT ... ON TABLE t (col) statement. +CREATE ROLE ghost_col_role; + +CREATE TABLE public.test_tbl (col1 int); + +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT t.oid, + (SELECT oid FROM pg_catalog.pg_class WHERE relname = 'pg_class'), + 1, + 'e', + ARRAY[('ghost_col_role=r/' || current_user)::aclitem] +FROM pg_catalog.pg_class t +WHERE t.relname = 'test_tbl' +AND t.relnamespace = 'public'::regnamespace; + +-- Case 4: spurious REVOKE caused by all-dangling initprivs and NULL proacl. +-- ghost_catalog_role is the only grantee; after deletion the unfiltered WHERE +-- clause in getFuncs() would select int4 for ACL output and dumpACL() would +-- emit a spurious "REVOKE EXECUTE FROM PUBLIC" that breaks restore. +CREATE ROLE ghost_catalog_role; + +INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs) +SELECT p.oid, + (SELECT oid FROM pg_catalog.pg_class WHERE relname = 'pg_proc'), + 0, + 'i', + ARRAY[('ghost_catalog_role=X/' || current_user)::aclitem] +FROM pg_catalog.pg_proc p +WHERE p.proname = 'int4' +AND p.pronamespace = (SELECT oid FROM pg_catalog.pg_namespace + WHERE nspname = 'pg_catalog') +LIMIT 1; + +-- Remove all ghost roles, leaving dangling OIDs in pg_init_privs. +DELETE FROM pg_catalog.pg_authid +WHERE rolname IN ('ghost_grantee', 'ghost_grantor', 'ghost_col_role', + 'ghost_catalog_role'); +}); + +my $tempdir = PostgreSQL::Test::Utils::tempdir; +my $dump_file = "$tempdir/dangling.sql"; + +# pg_dump must succeed even though pg_init_privs has dangling role OIDs. +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); + +# The user-defined objects must still appear in the dump. +like($dump, qr/CREATE FUNCTION public\.test_func_grantee/, + 'function with dangling grantee is present in dump'); +like($dump, qr/CREATE FUNCTION public\.test_func_grantor/, + 'function with dangling grantor is present in dump'); +like($dump, qr/CREATE TABLE public\.test_tbl/, + 'table with dangling column initprivs is present in dump'); + +# No statement should reference a bare numeric OID as a role name. +unlike($dump, qr/GRANT\b.*\bTO\s+"[0-9]+"/, + 'no GRANT with numeric OID as role name'); +unlike($dump, qr/SESSION AUTHORIZATION\s+"[0-9]+"/, + 'no SET SESSION AUTHORIZATION with numeric OID as role name'); + +# No GRANT at all for the dangling-grantee function. +unlike($dump, qr/GRANT\b.*\btest_func_grantee/, + 'no GRANT for test_func_grantee when all initprivs entries are dangling'); + +# No GRANT at all for the dangling-grantor function. +unlike($dump, qr/GRANT\b.*\btest_func_grantor/, + 'no GRANT for test_func_grantor when all initprivs entries are dangling'); + +# No column GRANT for the dangling column initprivs. +unlike($dump, qr/GRANT\b.*\btest_tbl\b.*\(col1\)/, + 'no column GRANT for test_tbl when column initprivs entries are dangling'); + +# No spurious REVOKE for the catalog function whose only initprivs entry was +# all-dangling. Without the WHERE clause filter a delta from acldefault to +# NULL would have caused dumpACL() to emit "REVOKE EXECUTE FROM PUBLIC". +unlike($dump, qr/REVOKE\b.*\bint4\s*\(/, + 'no spurious REVOKE for int4 when all initprivs entries are dangling'); + +done_testing(); -- 2.50.1 (Apple Git-155)