From c6ee793bfe3718336c76b59e0f31c5f6b2a1a360 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 29 Sep 2026 16:18:23 -0500 Subject: [PATCH 01/13] Fire the event trigger for DROP SCHEMA and DROP OWNED `DROP SCHEMA ... CASCADE` and `DROP OWNED BY` can drop an extension, but the trigger only fired for `DROP EXTENSION`. The registered SQL never ran and the leftover row in `extension_drop__commands` made every later `DROP EXTENSION` raise `XD001`. The trigger's `WHEN tag` list now includes both tags. The update script recreates the trigger, since `ALTER EVENT TRIGGER` cannot change `WHEN`. Co-Authored-By: Claude Sonnet 5.5 --- HISTORY.asc | 8 +++ README.md | 3 +- sql/extension_drop--0.1.1--stable.sql | 8 +++ sql/extension_drop.sql | 3 +- test/expected/drop_owned.out | 16 ++++++ test/expected/drop_owned_1.out | 16 ++++++ test/expected/drop_schema_cascade.out | 16 ++++++ test/expected/drop_schema_cascade_1.out | 16 ++++++ test/sql/drop_owned.sql | 66 +++++++++++++++++++++++++ test/sql/drop_schema_cascade.sql | 64 ++++++++++++++++++++++++ 10 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 test/expected/drop_owned.out create mode 100644 test/expected/drop_owned_1.out create mode 100644 test/expected/drop_schema_cascade.out create mode 100644 test/expected/drop_schema_cascade_1.out create mode 100644 test/sql/drop_owned.sql create mode 100644 test/sql/drop_schema_cascade.sql diff --git a/HISTORY.asc b/HISTORY.asc index 7e6369e..bf26060 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -6,6 +6,14 @@ extension_drop depends on cat_tools, which already requires PostgreSQL too (a cat_tools function it calls needs PG11+ internally) -- this just makes the declared floor honest. +== Run drop commands when `DROP SCHEMA ... CASCADE` or `DROP OWNED BY` drops an extension +Previously only `DROP EXTENSION` fired the event trigger, so dropping an +extension either of the other two ways skipped its registered SQL and left a +stale registration that made every later `DROP EXTENSION` fail with `XD001`. +The event trigger now also fires for `DROP SCHEMA` and `DROP OWNED`. Any +stale registration already present must be removed with +`extension_drop__repair()`. + == Remove redundant client_min_messages handling from install script `CREATE EXTENSION` already raises `client_min_messages` to `WARNING` for the install script (only-raising, so a stricter caller is respected) and diff --git a/README.md b/README.md index c38c877..b550a53 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ arbitrary SQL to run later. Grant them to specific roles as needed. ## How it works An event trigger (`extension_drop`, on `sql_drop`) fires whenever an -extension is dropped, runs any SQL registered for it via +extension is dropped -- by `DROP EXTENSION`, `DROP SCHEMA ... CASCADE` or +`DROP OWNED BY` -- runs any SQL registered for it via `extension_drop__add`/`extension_drop__update`, and removes the registration afterward. diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index b011c62..a6e4d38 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -25,4 +25,12 @@ BEGIN END $body$; +-- ALTER EVENT TRIGGER can't change WHEN +DROP EVENT TRIGGER extension_drop; +CREATE EVENT TRIGGER extension_drop + ON sql_drop + WHEN tag IN( 'DROP EXTENSION', 'DROP SCHEMA', 'DROP OWNED' ) -- NOTE! This MUST be IN + EXECUTE PROCEDURE extension_drop__event_trigger() +; + -- vim: sw=2 ts=2 expandtab diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 75319be..b3871aa 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -291,7 +291,8 @@ $body$ CREATE EVENT TRIGGER extension_drop ON sql_drop - WHEN tag IN( 'DROP EXTENSION' ) -- NOTE! This MUST be IN + -- DROP SCHEMA CASCADE and DROP OWNED BY also drop extensions. + WHEN tag IN( 'DROP EXTENSION', 'DROP SCHEMA', 'DROP OWNED' ) -- NOTE! This MUST be IN EXECUTE PROCEDURE extension_drop__event_trigger() ; diff --git a/test/expected/drop_owned.out b/test/expected/drop_owned.out new file mode 100644 index 0000000..36d377c --- /dev/null +++ b/test/expected/drop_owned.out @@ -0,0 +1,16 @@ +\set ECHO none +1..13 +ok 1 - Insert into extension_drop_test_table +ok 2 - Create role +ok 3 - Create test extension as role +ok 4 - Verify extension_drop__get() +ok 5 - Drop owned +ok 6 - Test extension does not exist +ok 7 - extension_drop_test_table is empty +ok 8 - No stale registration +ok 9 - Create test extension +ok 10 - Test extension exists +ok 11 - Drop test extension +ok 12 - Test extension does not exist +ok 13 - extension_drop's schema should not be reachable via search_path +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_owned_1.out b/test/expected/drop_owned_1.out new file mode 100644 index 0000000..3801831 --- /dev/null +++ b/test/expected/drop_owned_1.out @@ -0,0 +1,16 @@ +\set ECHO none +1..13 +ok 1 - Insert into extension_drop_test_table +ok 2 - Create role +ok 3 - Create test extension as role +ok 4 - Verify extension_drop__get() +ok 5 - Drop owned +ok 6 - Test extension does not exist +ok 7 - extension_drop_test_table is empty +ok 8 - No stale registration +ok 9 - Create test extension +ok 10 - Test extension exists +ok 11 - Drop test extension +ok 12 - Test extension does not exist +ok 13 # SKIP existing mode: extension_drop's placement is outside this suite's control +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_schema_cascade.out b/test/expected/drop_schema_cascade.out new file mode 100644 index 0000000..e280978 --- /dev/null +++ b/test/expected/drop_schema_cascade.out @@ -0,0 +1,16 @@ +\set ECHO none +1..13 +ok 1 - Insert into extension_drop_test_table +ok 2 - Create schema +ok 3 - Create test extension in schema +ok 4 - Verify extension_drop__get() +ok 5 - Drop schema cascade +ok 6 - Test extension does not exist +ok 7 - extension_drop_test_table is empty +ok 8 - No stale registration +ok 9 - Create test extension +ok 10 - Test extension exists +ok 11 - Drop test extension +ok 12 - Test extension does not exist +ok 13 - extension_drop's schema should not be reachable via search_path +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_schema_cascade_1.out b/test/expected/drop_schema_cascade_1.out new file mode 100644 index 0000000..a634d57 --- /dev/null +++ b/test/expected/drop_schema_cascade_1.out @@ -0,0 +1,16 @@ +\set ECHO none +1..13 +ok 1 - Insert into extension_drop_test_table +ok 2 - Create schema +ok 3 - Create test extension in schema +ok 4 - Verify extension_drop__get() +ok 5 - Drop schema cascade +ok 6 - Test extension does not exist +ok 7 - extension_drop_test_table is empty +ok 8 - No stale registration +ok 9 - Create test extension +ok 10 - Test extension exists +ok 11 - Drop test extension +ok 12 - Test extension does not exist +ok 13 # SKIP existing mode: extension_drop's placement is outside this suite's control +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/drop_owned.sql b/test/sql/drop_owned.sql new file mode 100644 index 0000000..a79979d --- /dev/null +++ b/test/sql/drop_owned.sql @@ -0,0 +1,66 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * DROP OWNED BY can drop an extension; its registered command must run and + * the registration must go away, or the stale row makes every later DROP + * EXTENSION fail. + */ +SELECT plan( + 0 + + + 1 -- Insert into test table + + + 1 -- Create role + + 1 -- Create test extension as role + + 1 -- Verify extension_drop__get() + + 1 -- Drop owned + + 1 -- Verify test extension is gone + + 1 -- Verify test table is empty + + 1 -- Verify no stale registration + + + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) + + + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) +); + +\i test/helpers/test_ext__insert.sql + +SELECT lives_ok( + 'CREATE ROLE extension_drop_test_owner SUPERUSER' + , 'Create role' +); +-- ALTER EXTENSION can't change the owner, so create it as that role +SELECT lives_ok( + $q$DO $$ BEGIN + SET LOCAL ROLE extension_drop_test_owner; + CREATE EXTENSION extension_drop_test; + RESET ROLE; + END $$$q$ + , 'Create test extension as role' +); +SELECT bag_eq( + format($$SELECT * FROM %I.extension_drop__get('extension_drop_test')$$, :'extension_drop_schema') + , $$SELECT 'extension_drop_test'::name, 'DELETE FROM extension_drop_test_table'::text$$ + , 'Verify extension_drop__get()' +); + +SELECT lives_ok( + 'DROP OWNED BY extension_drop_test_owner' + , 'Drop owned' +); +SELECT hasnt_extension('extension_drop_test', 'Test extension does not exist'); +SELECT is_empty( + $$SELECT * FROM $$ || :'TT' + , :'TT' || ' is empty' +); +SELECT is_empty( + format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') + , 'No stale registration' +); + +\i test/helpers/test_ext__create_drop.sql + +\i test/finish.sql + +-- vi: expandtab sw=2 ts=2 diff --git a/test/sql/drop_schema_cascade.sql b/test/sql/drop_schema_cascade.sql new file mode 100644 index 0000000..81acd22 --- /dev/null +++ b/test/sql/drop_schema_cascade.sql @@ -0,0 +1,64 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * DROP SCHEMA ... CASCADE can drop an extension; its registered command must + * run and the registration must go away, or the stale row makes every later + * DROP EXTENSION fail. + */ +SELECT plan( + 0 + + + 1 -- Insert into test table + + + 1 -- Create schema + + 1 -- Create test extension in it + + 1 -- Verify extension_drop__get() + + 1 -- Drop schema cascade + + 1 -- Verify test extension is gone + + 1 -- Verify test table is empty + + 1 -- Verify no stale registration + + + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) + + + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) +); + +-- Hide the version-specific "drop cascades to" NOTICE +SET client_min_messages = WARNING; + +\i test/helpers/test_ext__insert.sql + +SELECT lives_ok( + 'CREATE SCHEMA extension_drop_test_schema' + , 'Create schema' +); +SELECT lives_ok( + 'CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_schema' + , 'Create test extension in schema' +); +SELECT bag_eq( + format($$SELECT * FROM %I.extension_drop__get('extension_drop_test')$$, :'extension_drop_schema') + , $$SELECT 'extension_drop_test'::name, 'DELETE FROM extension_drop_test_table'::text$$ + , 'Verify extension_drop__get()' +); + +SELECT lives_ok( + 'DROP SCHEMA extension_drop_test_schema CASCADE' + , 'Drop schema cascade' +); +SELECT hasnt_extension('extension_drop_test', 'Test extension does not exist'); +SELECT is_empty( + $$SELECT * FROM $$ || :'TT' + , :'TT' || ' is empty' +); +SELECT is_empty( + format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') + , 'No stale registration' +); + +\i test/helpers/test_ext__create_drop.sql + +\i test/finish.sql + +-- vi: expandtab sw=2 ts=2 From 49a778f6af28f2451f2210d3ee22f6f0832245de Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 29 Sep 2026 16:57:33 -0500 Subject: [PATCH 02/13] Return early from the event trigger unless an extension was dropped Now that the trigger fires for every `DROP SCHEMA` and `DROP OWNED`, it must not read `extension_drop__commands` or run the sanity assert for drops that involve no extension: a non-superuser's ordinary drops failed with a permission error, and a stale row broke unrelated drops. The row variable is a `record` so the function compiles without access to the extension's schema. Adds tests for non-superuser and stale-row drops, and factors the shared post-drop assertions into a helper. Co-Authored-By: Claude Sonnet 5.5 --- HISTORY.asc | 12 ++-- README.md | 3 +- sql/extension_drop--0.1.1--stable.sql | 10 +++- sql/extension_drop.sql | 10 +++- test/expected/unrelated_drops.out | 11 ++++ test/expected/unrelated_drops_1.out | 11 ++++ test/helpers/test_ext__verify_dropped.sql | 11 ++++ test/sql/drop_owned.sql | 14 +---- test/sql/drop_schema_cascade.sql | 14 +---- test/sql/unrelated_drops.sql | 73 +++++++++++++++++++++++ 10 files changed, 136 insertions(+), 33 deletions(-) create mode 100644 test/expected/unrelated_drops.out create mode 100644 test/expected/unrelated_drops_1.out create mode 100644 test/helpers/test_ext__verify_dropped.sql create mode 100644 test/sql/unrelated_drops.sql diff --git a/HISTORY.asc b/HISTORY.asc index bf26060..54478b8 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -7,12 +7,12 @@ too (a cat_tools function it calls needs PG11+ internally) -- this just makes the declared floor honest. == Run drop commands when `DROP SCHEMA ... CASCADE` or `DROP OWNED BY` drops an extension -Previously only `DROP EXTENSION` fired the event trigger, so dropping an -extension either of the other two ways skipped its registered SQL and left a -stale registration that made every later `DROP EXTENSION` fail with `XD001`. -The event trigger now also fires for `DROP SCHEMA` and `DROP OWNED`. Any -stale registration already present must be removed with -`extension_drop__repair()`. +The event trigger now fires for `DROP SCHEMA` and `DROP OWNED` as well as +`DROP EXTENSION`, and returns immediately unless an extension is among the +dropped objects, so other drops are unaffected. Previously the other two +commands skipped the registered SQL and left a stale registration that made +every later `DROP EXTENSION` fail with `XD001`; remove any existing stale +registration with `extension_drop__repair()`. == Remove redundant client_min_messages handling from install script `CREATE EXTENSION` already raises `client_min_messages` to `WARNING` for diff --git a/README.md b/README.md index b550a53..08effaa 100644 --- a/README.md +++ b/README.md @@ -45,4 +45,5 @@ An event trigger (`extension_drop`, on `sql_drop`) fires whenever an extension is dropped -- by `DROP EXTENSION`, `DROP SCHEMA ... CASCADE` or `DROP OWNED BY` -- runs any SQL registered for it via `extension_drop__add`/`extension_drop__update`, and removes the registration -afterward. +afterward. Those commands fire the trigger for every drop, but it returns +immediately unless an extension is among the dropped objects. diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index a6e4d38..ffb8e6b 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -2,9 +2,17 @@ CREATE OR REPLACE FUNCTION extension_drop__event_trigger( ) RETURNS event_trigger LANGUAGE plpgsql SET search_path FROM CURRENT AS $body$ DECLARE - r extension_drop__commands; + r record; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; + + -- Most drops involve no extension; don't touch our table (or assert) for them. + IF NOT EXISTS( + SELECT 1 FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE object_type = 'extension' + ) THEN + RETURN; + END IF; + FOR r IN SELECT c.* FROM extension_drop__commands c diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index b3871aa..ae9757d 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -264,9 +264,17 @@ SELECT __extension_drop.create_function( , 'event_trigger LANGUAGE plpgsql' , $body$ DECLARE - r extension_drop__commands; + r record; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; + + -- Most drops involve no extension; don't touch our table (or assert) for them. + IF NOT EXISTS( + SELECT 1 FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE object_type = 'extension' + ) THEN + RETURN; + END IF; + FOR r IN SELECT c.* FROM extension_drop__commands c diff --git a/test/expected/unrelated_drops.out b/test/expected/unrelated_drops.out new file mode 100644 index 0000000..d1d9177 --- /dev/null +++ b/test/expected/unrelated_drops.out @@ -0,0 +1,11 @@ +\set ECHO none +1..8 +ok 1 - Create non-superuser role +ok 2 - Grant CREATE on database +ok 3 - Non-superuser DROP SCHEMA CASCADE +ok 4 - Non-superuser DROP OWNED BY +ok 5 - Insert stale registration +ok 6 - Stale registration is present +ok 7 - Superuser DROP SCHEMA with stale registration +ok 8 - extension_drop's schema should not be reachable via search_path +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/unrelated_drops_1.out b/test/expected/unrelated_drops_1.out new file mode 100644 index 0000000..6860796 --- /dev/null +++ b/test/expected/unrelated_drops_1.out @@ -0,0 +1,11 @@ +\set ECHO none +1..8 +ok 1 - Create non-superuser role +ok 2 - Grant CREATE on database +ok 3 - Non-superuser DROP SCHEMA CASCADE +ok 4 - Non-superuser DROP OWNED BY +ok 5 - Insert stale registration +ok 6 - Stale registration is present +ok 7 - Superuser DROP SCHEMA with stale registration +ok 8 # SKIP existing mode: extension_drop's placement is outside this suite's control +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/helpers/test_ext__verify_dropped.sql b/test/helpers/test_ext__verify_dropped.sql new file mode 100644 index 0000000..a86f134 --- /dev/null +++ b/test/helpers/test_ext__verify_dropped.sql @@ -0,0 +1,11 @@ +SELECT hasnt_extension('extension_drop_test', 'Test extension does not exist'); +SELECT is_empty( + $$SELECT * FROM $$ || :'TT' + , :'TT' || ' is empty' +); +SELECT is_empty( + format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') + , 'No stale registration' +); + +-- vi: expandtab sw=2 ts=2 diff --git a/test/sql/drop_owned.sql b/test/sql/drop_owned.sql index a79979d..dcc439c 100644 --- a/test/sql/drop_owned.sql +++ b/test/sql/drop_owned.sql @@ -15,9 +15,7 @@ SELECT plan( + 1 -- Create test extension as role + 1 -- Verify extension_drop__get() + 1 -- Drop owned - + 1 -- Verify test extension is gone - + 1 -- Verify test table is empty - + 1 -- Verify no stale registration + + 3 -- Verify extension is gone, table is empty, no stale registration (test/helpers/test_ext__verify_dropped.sql) + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) @@ -49,15 +47,7 @@ SELECT lives_ok( 'DROP OWNED BY extension_drop_test_owner' , 'Drop owned' ); -SELECT hasnt_extension('extension_drop_test', 'Test extension does not exist'); -SELECT is_empty( - $$SELECT * FROM $$ || :'TT' - , :'TT' || ' is empty' -); -SELECT is_empty( - format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') - , 'No stale registration' -); +\i test/helpers/test_ext__verify_dropped.sql \i test/helpers/test_ext__create_drop.sql diff --git a/test/sql/drop_schema_cascade.sql b/test/sql/drop_schema_cascade.sql index 81acd22..a6b46c3 100644 --- a/test/sql/drop_schema_cascade.sql +++ b/test/sql/drop_schema_cascade.sql @@ -15,9 +15,7 @@ SELECT plan( + 1 -- Create test extension in it + 1 -- Verify extension_drop__get() + 1 -- Drop schema cascade - + 1 -- Verify test extension is gone - + 1 -- Verify test table is empty - + 1 -- Verify no stale registration + + 3 -- Verify extension is gone, table is empty, no stale registration (test/helpers/test_ext__verify_dropped.sql) + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) @@ -47,15 +45,7 @@ SELECT lives_ok( 'DROP SCHEMA extension_drop_test_schema CASCADE' , 'Drop schema cascade' ); -SELECT hasnt_extension('extension_drop_test', 'Test extension does not exist'); -SELECT is_empty( - $$SELECT * FROM $$ || :'TT' - , :'TT' || ' is empty' -); -SELECT is_empty( - format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') - , 'No stale registration' -); +\i test/helpers/test_ext__verify_dropped.sql \i test/helpers/test_ext__create_drop.sql diff --git a/test/sql/unrelated_drops.sql b/test/sql/unrelated_drops.sql new file mode 100644 index 0000000..1bdc4a5 --- /dev/null +++ b/test/sql/unrelated_drops.sql @@ -0,0 +1,73 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * DROP SCHEMA and DROP OWNED fire extension_drop's event trigger, so drops + * that involve no extension must not depend on the caller's privileges or on + * the state of extension_drop__commands. + */ +SELECT plan( + 0 + + + 1 -- Create non-superuser role + + 1 -- Grant it CREATE on the database + + 1 -- Non-superuser DROP SCHEMA CASCADE + + 1 -- Non-superuser DROP OWNED BY + + + 1 -- Insert stale registration + + 1 -- Stale registration is present + + 1 -- Superuser DROP SCHEMA with stale registration + + + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) +); + +-- Hide version- and database-specific NOTICE/WARNING text from the drops +SET client_min_messages = ERROR; + +SELECT lives_ok( + 'CREATE ROLE extension_drop_test_plain' + , 'Create non-superuser role' +); +SELECT lives_ok( + format('GRANT CREATE ON DATABASE %I TO extension_drop_test_plain', current_database()) + , 'Grant CREATE on database' +); + +SELECT lives_ok( + $q$DO $$ BEGIN + SET LOCAL ROLE extension_drop_test_plain; + CREATE SCHEMA extension_drop_test_plain_schema; + CREATE TABLE extension_drop_test_plain_schema.t(i int); + DROP SCHEMA extension_drop_test_plain_schema CASCADE; + RESET ROLE; + END $$$q$ + , 'Non-superuser DROP SCHEMA CASCADE' +); +SELECT lives_ok( + $q$DO $$ BEGIN + SET LOCAL ROLE extension_drop_test_plain; + CREATE SCHEMA extension_drop_test_plain_schema; + CREATE TABLE extension_drop_test_plain_schema.t(i int); + DROP OWNED BY extension_drop_test_plain; + RESET ROLE; + END $$$q$ + , 'Non-superuser DROP OWNED BY' +); + +SELECT lives_ok( + format($$INSERT INTO %I.extension_drop__commands VALUES('extension_drop_test_stale', 'SELECT 1')$$, :'extension_drop_schema') + , 'Insert stale registration' +); +SELECT results_eq( + format($$SELECT %I.extension_drop__sanity_check()$$, :'extension_drop_schema') + , $$SELECT '{extension_drop_test_stale}'::name[]$$ + , 'Stale registration is present' +); +SELECT lives_ok( + $$CREATE SCHEMA extension_drop_test_su_schema; DROP SCHEMA extension_drop_test_su_schema$$ + , 'Superuser DROP SCHEMA with stale registration' +); + +\i test/finish.sql + +-- vi: expandtab sw=2 ts=2 From e26c90244d8f9396e85c92b1f74ad000205a74df Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 29 Sep 2026 17:06:51 -0500 Subject: [PATCH 03/13] Comment why the trigger's row variable is a record Co-Authored-By: Claude Sonnet 5.5 --- sql/extension_drop--0.1.1--stable.sql | 2 +- sql/extension_drop.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index ffb8e6b..940420e 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -2,7 +2,7 @@ CREATE OR REPLACE FUNCTION extension_drop__event_trigger( ) RETURNS event_trigger LANGUAGE plpgsql SET search_path FROM CURRENT AS $body$ DECLARE - r record; + r record; -- Not extension_drop__commands: the type must not resolve before the early return BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index ae9757d..2e66442 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -264,7 +264,7 @@ SELECT __extension_drop.create_function( , 'event_trigger LANGUAGE plpgsql' , $body$ DECLARE - r record; + r record; -- Not extension_drop__commands: the type must not resolve before the early return BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; From df5aa134399b995c93a9d3f1b7cbcd9b5c169bbf Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 02:13:39 -0500 Subject: [PATCH 04/13] Explain why the trigger's row variable is a record A typed variable is resolved when the function compiles, before the early return, so a role without access to the extension's schema fails unrelated drops. --- sql/extension_drop--0.1.1--stable.sql | 4 +++- sql/extension_drop.sql | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index 940420e..495640a 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -2,7 +2,9 @@ CREATE OR REPLACE FUNCTION extension_drop__event_trigger( ) RETURNS event_trigger LANGUAGE plpgsql SET search_path FROM CURRENT AS $body$ DECLARE - r record; -- Not extension_drop__commands: the type must not resolve before the early return + -- record, not extension_drop__commands: a typed variable is resolved when the function compiles, before + -- the early return, and a role without access to the extension's schema then fails every unrelated DROP. + r record; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 2e66442..5ff0702 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -264,7 +264,9 @@ SELECT __extension_drop.create_function( , 'event_trigger LANGUAGE plpgsql' , $body$ DECLARE - r record; -- Not extension_drop__commands: the type must not resolve before the early return + -- record, not extension_drop__commands: a typed variable is resolved when the function compiles, before + -- the early return, and a role without access to the extension's schema then fails every unrelated DROP. + r record; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; From 0cae837464a61c873d194bfb2dda9100d344fb24 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 02:15:02 -0500 Subject: [PATCH 05/13] Use a block comment for the record-variable note --- sql/extension_drop--0.1.1--stable.sql | 6 ++++-- sql/extension_drop.sql | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index 495640a..22a338c 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -2,8 +2,10 @@ CREATE OR REPLACE FUNCTION extension_drop__event_trigger( ) RETURNS event_trigger LANGUAGE plpgsql SET search_path FROM CURRENT AS $body$ DECLARE - -- record, not extension_drop__commands: a typed variable is resolved when the function compiles, before - -- the early return, and a role without access to the extension's schema then fails every unrelated DROP. + /* + * record, not extension_drop__commands: a typed variable is resolved when the function compiles, before + * the early return, and a role without access to the extension's schema then fails every unrelated DROP. + */ r record; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 5ff0702..673bce3 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -264,8 +264,10 @@ SELECT __extension_drop.create_function( , 'event_trigger LANGUAGE plpgsql' , $body$ DECLARE - -- record, not extension_drop__commands: a typed variable is resolved when the function compiles, before - -- the early return, and a role without access to the extension's schema then fails every unrelated DROP. + /* + * record, not extension_drop__commands: a typed variable is resolved when the function compiles, before + * the early return, and a role without access to the extension's schema then fails every unrelated DROP. + */ r record; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; From 823be14e53bc70dc2e8807148db0355bebcd8268 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 14:48:32 -0500 Subject: [PATCH 06/13] Consolidate test plan() arithmetic; drop the database CREATE grant from unrelated_drops The non-superuser role now owns schemas created via CREATE SCHEMA ... AUTHORIZATION, so it needs no CREATE privilege on the database. --- test/expected/unrelated_drops.out | 15 +++++++-------- test/expected/unrelated_drops_1.out | 15 +++++++-------- test/sql/drop_owned.sql | 12 ++---------- test/sql/drop_schema_cascade.sql | 12 ++---------- test/sql/unrelated_drops.sql | 30 +++++++++++------------------ 5 files changed, 29 insertions(+), 55 deletions(-) diff --git a/test/expected/unrelated_drops.out b/test/expected/unrelated_drops.out index d1d9177..fdf0059 100644 --- a/test/expected/unrelated_drops.out +++ b/test/expected/unrelated_drops.out @@ -1,11 +1,10 @@ \set ECHO none -1..8 +1..7 ok 1 - Create non-superuser role -ok 2 - Grant CREATE on database -ok 3 - Non-superuser DROP SCHEMA CASCADE -ok 4 - Non-superuser DROP OWNED BY -ok 5 - Insert stale registration -ok 6 - Stale registration is present -ok 7 - Superuser DROP SCHEMA with stale registration -ok 8 - extension_drop's schema should not be reachable via search_path +ok 2 - Non-superuser DROP SCHEMA CASCADE +ok 3 - Non-superuser DROP OWNED BY +ok 4 - Insert stale registration +ok 5 - Stale registration is present +ok 6 - Superuser DROP SCHEMA with stale registration +ok 7 - extension_drop's schema should not be reachable via search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/unrelated_drops_1.out b/test/expected/unrelated_drops_1.out index 6860796..19ec9b0 100644 --- a/test/expected/unrelated_drops_1.out +++ b/test/expected/unrelated_drops_1.out @@ -1,11 +1,10 @@ \set ECHO none -1..8 +1..7 ok 1 - Create non-superuser role -ok 2 - Grant CREATE on database -ok 3 - Non-superuser DROP SCHEMA CASCADE -ok 4 - Non-superuser DROP OWNED BY -ok 5 - Insert stale registration -ok 6 - Stale registration is present -ok 7 - Superuser DROP SCHEMA with stale registration -ok 8 # SKIP existing mode: extension_drop's placement is outside this suite's control +ok 2 - Non-superuser DROP SCHEMA CASCADE +ok 3 - Non-superuser DROP OWNED BY +ok 4 - Insert stale registration +ok 5 - Stale registration is present +ok 6 - Superuser DROP SCHEMA with stale registration +ok 7 # SKIP existing mode: extension_drop's placement is outside this suite's control # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/drop_owned.sql b/test/sql/drop_owned.sql index dcc439c..1e2659f 100644 --- a/test/sql/drop_owned.sql +++ b/test/sql/drop_owned.sql @@ -7,18 +7,10 @@ * EXTENSION fail. */ SELECT plan( - 0 - - + 1 -- Insert into test table - - + 1 -- Create role - + 1 -- Create test extension as role - + 1 -- Verify extension_drop__get() - + 1 -- Drop owned + 1 -- Insert into test table + + 4 -- Create role, create extension as it, verify extension_drop__get(), drop owned + 3 -- Verify extension is gone, table is empty, no stale registration (test/helpers/test_ext__verify_dropped.sql) - + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) - + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) ); diff --git a/test/sql/drop_schema_cascade.sql b/test/sql/drop_schema_cascade.sql index a6b46c3..90c6d22 100644 --- a/test/sql/drop_schema_cascade.sql +++ b/test/sql/drop_schema_cascade.sql @@ -7,18 +7,10 @@ * DROP EXTENSION fail. */ SELECT plan( - 0 - - + 1 -- Insert into test table - - + 1 -- Create schema - + 1 -- Create test extension in it - + 1 -- Verify extension_drop__get() - + 1 -- Drop schema cascade + 1 -- Insert into test table + + 4 -- Create schema, create extension in it, verify extension_drop__get(), drop schema cascade + 3 -- Verify extension is gone, table is empty, no stale registration (test/helpers/test_ext__verify_dropped.sql) - + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) - + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) ); diff --git a/test/sql/unrelated_drops.sql b/test/sql/unrelated_drops.sql index 1bdc4a5..ae742e1 100644 --- a/test/sql/unrelated_drops.sql +++ b/test/sql/unrelated_drops.sql @@ -7,17 +7,9 @@ * the state of extension_drop__commands. */ SELECT plan( - 0 - - + 1 -- Create non-superuser role - + 1 -- Grant it CREATE on the database - + 1 -- Non-superuser DROP SCHEMA CASCADE - + 1 -- Non-superuser DROP OWNED BY - - + 1 -- Insert stale registration - + 1 -- Stale registration is present - + 1 -- Superuser DROP SCHEMA with stale registration - + 1 -- Create non-superuser role + + 2 -- Non-superuser DROP SCHEMA CASCADE, DROP OWNED BY + + 3 -- Stale registration: insert, present, superuser DROP SCHEMA + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) ); @@ -28,16 +20,16 @@ SELECT lives_ok( 'CREATE ROLE extension_drop_test_plain' , 'Create non-superuser role' ); -SELECT lives_ok( - format('GRANT CREATE ON DATABASE %I TO extension_drop_test_plain', current_database()) - , 'Grant CREATE on database' -); +/* + * Schemas are created as superuser but owned by the role, so it needs no + * CREATE privilege on the database. + */ SELECT lives_ok( $q$DO $$ BEGIN + CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain + CREATE TABLE t(i int); SET LOCAL ROLE extension_drop_test_plain; - CREATE SCHEMA extension_drop_test_plain_schema; - CREATE TABLE extension_drop_test_plain_schema.t(i int); DROP SCHEMA extension_drop_test_plain_schema CASCADE; RESET ROLE; END $$$q$ @@ -45,9 +37,9 @@ SELECT lives_ok( ); SELECT lives_ok( $q$DO $$ BEGIN + CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain + CREATE TABLE t(i int); SET LOCAL ROLE extension_drop_test_plain; - CREATE SCHEMA extension_drop_test_plain_schema; - CREATE TABLE extension_drop_test_plain_schema.t(i int); DROP OWNED BY extension_drop_test_plain; RESET ROLE; END $$$q$ From 50bf9bee8d4a8cf66835eb1c798c93fedc30ab7b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 15:59:11 -0500 Subject: [PATCH 07/13] Label each test plan() term with its source, one line per helper Group the per-file assertion counts into a single term and label the helper terms with the helper's path. Totals are unchanged. --- test/sql/drop_owned.sql | 8 ++++---- test/sql/drop_schema_cascade.sql | 8 ++++---- test/sql/unrelated_drops.sql | 5 ++--- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/test/sql/drop_owned.sql b/test/sql/drop_owned.sql index 1e2659f..495ee04 100644 --- a/test/sql/drop_owned.sql +++ b/test/sql/drop_owned.sql @@ -7,10 +7,10 @@ * EXTENSION fail. */ SELECT plan( - 1 -- Insert into test table - + 4 -- Create role, create extension as it, verify extension_drop__get(), drop owned - + 3 -- Verify extension is gone, table is empty, no stale registration (test/helpers/test_ext__verify_dropped.sql) - + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) + 1 -- test/helpers/test_ext__insert.sql + + 4 -- DROP OWNED BY drops the extension + + 3 -- test/helpers/test_ext__verify_dropped.sql + + 4 -- test/helpers/test_ext__create_drop.sql + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) ); diff --git a/test/sql/drop_schema_cascade.sql b/test/sql/drop_schema_cascade.sql index 90c6d22..2cec30d 100644 --- a/test/sql/drop_schema_cascade.sql +++ b/test/sql/drop_schema_cascade.sql @@ -7,10 +7,10 @@ * DROP EXTENSION fail. */ SELECT plan( - 1 -- Insert into test table - + 4 -- Create schema, create extension in it, verify extension_drop__get(), drop schema cascade - + 3 -- Verify extension is gone, table is empty, no stale registration (test/helpers/test_ext__verify_dropped.sql) - + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) + 1 -- test/helpers/test_ext__insert.sql + + 4 -- DROP SCHEMA CASCADE drops the extension + + 3 -- test/helpers/test_ext__verify_dropped.sql + + 4 -- test/helpers/test_ext__create_drop.sql + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) ); diff --git a/test/sql/unrelated_drops.sql b/test/sql/unrelated_drops.sql index ae742e1..e6a3515 100644 --- a/test/sql/unrelated_drops.sql +++ b/test/sql/unrelated_drops.sql @@ -7,9 +7,8 @@ * the state of extension_drop__commands. */ SELECT plan( - 1 -- Create non-superuser role - + 2 -- Non-superuser DROP SCHEMA CASCADE, DROP OWNED BY - + 3 -- Stale registration: insert, present, superuser DROP SCHEMA + 3 -- Non-superuser drops + + 3 -- Stale registration + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) ); From 6b689ffe0c71bbeaef160bef8c0ee76ef507cacd Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:03:19 -0500 Subject: [PATCH 08/13] Raise an actionable error when the caller can't use extension_drop__commands A role that drops an extension (via DROP EXTENSION, DROP SCHEMA ... CASCADE or DROP OWNED) without access to extension_drop__commands got a bare "permission denied for table". The trigger now checks USAGE on the table's schema plus SELECT and DELETE on the table, after the early return for drops that involve no extension, and raises insufficient_privilege with a DETAIL naming the table and a HINT naming its owner and the grants that would fix it. The table is located through pg_extension and pg_class rather than by name, since name lookup skips a schema the caller can't use. The function stays SECURITY INVOKER: SECURITY DEFINER would run registered SQL as the owner. --- HISTORY.asc | 6 ++ README.md | 6 ++ sql/extension_drop--0.1.1--stable.sql | 34 +++++++ sql/extension_drop.sql | 34 +++++++ test/expected/drop_denied.out | 23 +++++ test/expected/drop_denied_1.out | 23 +++++ test/sql/drop_denied.sql | 141 ++++++++++++++++++++++++++ 7 files changed, 267 insertions(+) create mode 100644 test/expected/drop_denied.out create mode 100644 test/expected/drop_denied_1.out create mode 100644 test/sql/drop_denied.sql diff --git a/HISTORY.asc b/HISTORY.asc index 54478b8..32318fe 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -14,6 +14,12 @@ commands skipped the registered SQL and left a stale registration that made every later `DROP EXTENSION` fail with `XD001`; remove any existing stale registration with `extension_drop__repair()`. +Dropping an extension, by any of the three commands, now requires access to +`extension_drop__commands` (USAGE on its schema, SELECT and DELETE on it), +whether or not the extension is registered. A role without it gets an +`insufficient_privilege` error naming the table's owner and the grants that +would fix it, instead of `permission denied for table extension_drop__commands`. + == Remove redundant client_min_messages handling from install script `CREATE EXTENSION` already raises `client_min_messages` to `WARNING` for the install script (only-raising, so a stricter caller is respected) and diff --git a/README.md b/README.md index 00f2bf1..bf10269 100644 --- a/README.md +++ b/README.md @@ -47,3 +47,9 @@ extension is dropped -- by `DROP EXTENSION`, `DROP SCHEMA ... CASCADE` or `extension_drop__add`/`extension_drop__update`, and removes the registration afterward. Those commands fire the trigger for every drop, but it returns immediately unless an extension is among the dropped objects. + +Dropping an extension therefore needs access to `extension_drop__commands` +(USAGE on its schema, SELECT and DELETE on the table), whether or not the +extension is registered. A role without it gets an `insufficient_privilege` +error that names the table's owner and the grants that would fix it; a +superuser, or the role that installed `extension_drop`, always has it. diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index 22a338c..1f94533 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -7,6 +7,8 @@ DECLARE * the early return, and a role without access to the extension's schema then fails every unrelated DROP. */ r record; + t record; + dropping text; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; @@ -17,6 +19,38 @@ BEGIN RETURN; END IF; + /* + * Fail with an actionable error instead of the raw "permission denied" from the loop below. The table is + * found by catalog OIDs, not by name: name lookup silently skips a schema the caller can't USE, which + * would turn this into "does not exist". Not SECURITY DEFINER: that would run registered SQL as the owner. + */ + SELECT c.oid AS oid, c.relnamespace, n.nspname, c.relname, pg_catalog.pg_get_userbyid(c.relowner) AS owner + INTO t + FROM pg_catalog.pg_extension e + JOIN pg_catalog.pg_class c + ON c.relnamespace = e.extnamespace AND c.relname = 'extension_drop__commands' + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE e.extname = 'extension_drop'; + IF FOUND AND NOT ( + pg_catalog.has_schema_privilege(t.relnamespace, 'USAGE') + AND pg_catalog.has_table_privilege(t.oid, 'SELECT') + AND pg_catalog.has_table_privilege(t.oid, 'DELETE') + ) THEN + SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name) + INTO dropping + FROM pg_catalog.pg_event_trigger_dropped_objects() + WHERE object_type = 'extension'; + RAISE EXCEPTION 'permission denied to drop extension(s) %', dropping + USING ERRCODE = 'insufficient_privilege' + , DETAIL = pg_catalog.format( + 'extension_drop must read and delete its registration in %s.%s.' + , pg_catalog.quote_ident(t.nspname), pg_catalog.quote_ident(t.relname)) + , HINT = pg_catalog.format( + 'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s.%s to the current role.' + , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname) + , pg_catalog.quote_ident(t.nspname), pg_catalog.quote_ident(t.relname)); + END IF; + FOR r IN SELECT c.* FROM extension_drop__commands c diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 673bce3..3165d9f 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -269,6 +269,8 @@ DECLARE * the early return, and a role without access to the extension's schema then fails every unrelated DROP. */ r record; + t record; + dropping text; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; @@ -279,6 +281,38 @@ BEGIN RETURN; END IF; + /* + * Fail with an actionable error instead of the raw "permission denied" from the loop below. The table is + * found by catalog OIDs, not by name: name lookup silently skips a schema the caller can't USE, which + * would turn this into "does not exist". Not SECURITY DEFINER: that would run registered SQL as the owner. + */ + SELECT c.oid AS oid, c.relnamespace, n.nspname, c.relname, pg_catalog.pg_get_userbyid(c.relowner) AS owner + INTO t + FROM pg_catalog.pg_extension e + JOIN pg_catalog.pg_class c + ON c.relnamespace = e.extnamespace AND c.relname = 'extension_drop__commands' + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE e.extname = 'extension_drop'; + IF FOUND AND NOT ( + pg_catalog.has_schema_privilege(t.relnamespace, 'USAGE') + AND pg_catalog.has_table_privilege(t.oid, 'SELECT') + AND pg_catalog.has_table_privilege(t.oid, 'DELETE') + ) THEN + SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name) + INTO dropping + FROM pg_catalog.pg_event_trigger_dropped_objects() + WHERE object_type = 'extension'; + RAISE EXCEPTION 'permission denied to drop extension(s) %', dropping + USING ERRCODE = 'insufficient_privilege' + , DETAIL = pg_catalog.format( + 'extension_drop must read and delete its registration in %s.%s.' + , pg_catalog.quote_ident(t.nspname), pg_catalog.quote_ident(t.relname)) + , HINT = pg_catalog.format( + 'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s.%s to the current role.' + , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname) + , pg_catalog.quote_ident(t.nspname), pg_catalog.quote_ident(t.relname)); + END IF; + FOR r IN SELECT c.* FROM extension_drop__commands c diff --git a/test/expected/drop_denied.out b/test/expected/drop_denied.out new file mode 100644 index 0000000..d9906d7 --- /dev/null +++ b/test/expected/drop_denied.out @@ -0,0 +1,23 @@ +\set ECHO none +1..20 +ok 1 - Create non-superuser role +ok 2 - Create schema owned by role containing a registered extension +ok 3 - DROP SCHEMA CASCADE without USAGE on extension_drop's schema +ok 4 - Capture DETAIL and HINT +ok 5 - DETAIL +ok 6 - HINT +ok 7 - Grant USAGE on extension_drop's schema +ok 8 - DROP SCHEMA CASCADE with USAGE but no table privileges +ok 9 - Unregister the extension +ok 10 - DROP SCHEMA CASCADE of an unregistered extension +ok 11 - DROP OWNED BY +ok 12 - Superuser drop schema +ok 13 - No stale registration +ok 14 - Create role +ok 15 - Create test extension as role +ok 16 - Demote role +ok 17 - DROP EXTENSION by the extension's non-superuser owner +ok 18 - Superuser drop extension +ok 19 - No stale registration +ok 20 - extension_drop's schema should not be reachable via search_path +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_denied_1.out b/test/expected/drop_denied_1.out new file mode 100644 index 0000000..7617287 --- /dev/null +++ b/test/expected/drop_denied_1.out @@ -0,0 +1,23 @@ +\set ECHO none +1..20 +ok 1 - Create non-superuser role +ok 2 - Create schema owned by role containing a registered extension +ok 3 - DROP SCHEMA CASCADE without USAGE on extension_drop's schema +ok 4 - Capture DETAIL and HINT +ok 5 - DETAIL +ok 6 - HINT +ok 7 - Grant USAGE on extension_drop's schema +ok 8 - DROP SCHEMA CASCADE with USAGE but no table privileges +ok 9 - Unregister the extension +ok 10 - DROP SCHEMA CASCADE of an unregistered extension +ok 11 - DROP OWNED BY +ok 12 - Superuser drop schema +ok 13 - No stale registration +ok 14 - Create role +ok 15 - Create test extension as role +ok 16 - Demote role +ok 17 - DROP EXTENSION by the extension's non-superuser owner +ok 18 - Superuser drop extension +ok 19 - No stale registration +ok 20 # SKIP existing mode: extension_drop's placement is outside this suite's control +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/drop_denied.sql b/test/sql/drop_denied.sql new file mode 100644 index 0000000..39de81f --- /dev/null +++ b/test/sql/drop_denied.sql @@ -0,0 +1,141 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * A role that can't use extension_drop's registration table must get an + * actionable insufficient_privilege error, not a raw "permission denied for + * table", whenever a drop includes an extension -- registered or not -- via + * any of the three commands the trigger fires for. + */ +SELECT plan( + 2 -- Setup: role, schema with extension + + 1 -- DROP SCHEMA CASCADE without USAGE on extension_drop's schema + + 3 -- DETAIL and HINT + + 2 -- USAGE on extension_drop's schema, but no table privileges + + 2 -- Unregistered extension + + 1 -- DROP OWNED BY + + 2 -- Cleanup + + 4 -- DROP EXTENSION by a role that was a superuser when it created it + + 2 -- Cleanup + + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) +); + +-- Hide version-specific NOTICE/WARNING text from the drops +SET client_min_messages = ERROR; + +\set MSG 'permission denied to drop extension(s) extension_drop_test' + +SELECT lives_ok( + 'CREATE ROLE extension_drop_test_plain' + , 'Create non-superuser role' +); +-- Created as superuser, so the role only owns the schema +SELECT lives_ok( + $$CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain; + CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_plain_schema$$ + , 'Create schema owned by role containing a registered extension' +); + +SELECT throws_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + , '42501' + , :'MSG' + , 'DROP SCHEMA CASCADE without USAGE on extension_drop''s schema' +); + +-- The error text reaches the caller as DETAIL and HINT; stash them in GUCs to compare +SELECT lives_ok( + $q$DO $$ + DECLARE d text; h text; + BEGIN + SET LOCAL ROLE extension_drop_test_plain; + DROP SCHEMA extension_drop_test_plain_schema CASCADE; + EXCEPTION WHEN insufficient_privilege THEN + GET STACKED DIAGNOSTICS d = PG_EXCEPTION_DETAIL, h = PG_EXCEPTION_HINT; + PERFORM set_config('extension_drop_test.detail', d, false), set_config('extension_drop_test.hint', h, false); + END $$$q$ + , 'Capture DETAIL and HINT' +); +SELECT is( + current_setting('extension_drop_test.detail') + , format('extension_drop must read and delete its registration in %I.extension_drop__commands.', :'extension_drop_schema') + , 'DETAIL' +); +SELECT is( + current_setting('extension_drop_test.hint') + , format( + 'Run the drop as role %1$I or a superuser, or grant USAGE on schema %2$I and SELECT, DELETE on %2$I.extension_drop__commands to the current role.' + , current_user, :'extension_drop_schema') + , 'HINT' +); + +SELECT lives_ok( + format('GRANT USAGE ON SCHEMA %I TO extension_drop_test_plain', :'extension_drop_schema') + , 'Grant USAGE on extension_drop''s schema' +); +SELECT throws_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + , '42501' + , :'MSG' + , 'DROP SCHEMA CASCADE with USAGE but no table privileges' +); + +SELECT lives_ok( + format($$SELECT %I.extension_drop__remove('extension_drop_test')$$, :'extension_drop_schema') + , 'Unregister the extension' +); +SELECT throws_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + , '42501' + , :'MSG' + , 'DROP SCHEMA CASCADE of an unregistered extension' +); + +SELECT throws_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP OWNED BY extension_drop_test_plain CASCADE; END $$$q$ + , '42501' + , :'MSG' + , 'DROP OWNED BY' +); + +SELECT lives_ok( + 'DROP SCHEMA extension_drop_test_plain_schema CASCADE' + , 'Superuser drop schema' +); +SELECT is_empty( + format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') + , 'No stale registration' +); + +-- ALTER EXTENSION can't change the owner, so create it as a role that is then demoted +SELECT lives_ok( + 'CREATE ROLE extension_drop_test_owner SUPERUSER' + , 'Create role' +); +SELECT lives_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_owner; CREATE EXTENSION extension_drop_test; RESET ROLE; END $$$q$ + , 'Create test extension as role' +); +SELECT lives_ok( + 'ALTER ROLE extension_drop_test_owner NOSUPERUSER' + , 'Demote role' +); +SELECT throws_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_owner; DROP EXTENSION extension_drop_test; END $$$q$ + , '42501' + , :'MSG' + , 'DROP EXTENSION by the extension''s non-superuser owner' +); + +SELECT lives_ok( + 'DROP EXTENSION extension_drop_test' + , 'Superuser drop extension' +); +SELECT is_empty( + format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') + , 'No stale registration' +); + +\i test/finish.sql + +-- vi: expandtab sw=2 ts=2 From 6628ab188b78004b6d5354184fe30437dbb2dc11 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:19:59 -0500 Subject: [PATCH 09/13] Trap the privilege error instead of pre-checking the catalog The event trigger now reads and deletes the dropped extensions' registrations in one statement wrapped in an exception block for insufficient_privilege and undefined_table (a caller without USAGE on the schema gets "does not exist"). Registered SQL runs afterward, outside the block, so a privilege error from it is never mistaken for extension_drop's own. The handler raises the same actionable insufficient_privilege error, with the original message added to the DETAIL; the table owner and name for the HINT are looked up there, through pg_extension and pg_class, since the caller may lack USAGE. drop_denied now also covers SELECT without DELETE, a fully granted role dropping successfully, and a privilege error from registered SQL surfacing unchanged. --- HISTORY.asc | 2 +- README.md | 2 +- sql/extension_drop--0.1.1--stable.sql | 83 ++++++++++++++------------- sql/extension_drop.sql | 83 ++++++++++++++------------- test/expected/drop_denied.out | 37 +++++++----- test/expected/drop_denied_1.out | 37 +++++++----- test/sql/drop_denied.sql | 82 +++++++++++++++++++++++++- 7 files changed, 214 insertions(+), 112 deletions(-) diff --git a/HISTORY.asc b/HISTORY.asc index 32318fe..4a6129e 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -18,7 +18,7 @@ Dropping an extension, by any of the three commands, now requires access to `extension_drop__commands` (USAGE on its schema, SELECT and DELETE on it), whether or not the extension is registered. A role without it gets an `insufficient_privilege` error naming the table's owner and the grants that -would fix it, instead of `permission denied for table extension_drop__commands`. +would fix it, and quoting the underlying error, instead of `permission denied for table extension_drop__commands`. == Remove redundant client_min_messages handling from install script `CREATE EXTENSION` already raises `client_min_messages` to `WARNING` for diff --git a/README.md b/README.md index bf10269..460cba4 100644 --- a/README.md +++ b/README.md @@ -51,5 +51,5 @@ immediately unless an extension is among the dropped objects. Dropping an extension therefore needs access to `extension_drop__commands` (USAGE on its schema, SELECT and DELETE on the table), whether or not the extension is registered. A role without it gets an `insufficient_privilege` -error that names the table's owner and the grants that would fix it; a +error that names the table's owner, the grants that would fix it and the underlying error; a superuser, or the role that installed `extension_drop`, always has it. diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index 1f94533..bda1406 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -6,61 +6,62 @@ DECLARE * record, not extension_drop__commands: a typed variable is resolved when the function compiles, before * the early return, and a role without access to the extension's schema then fails every unrelated DROP. */ - r record; t record; dropping text; + names text[]; + cmds text[]; + i int; + msg text; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; -- Most drops involve no extension; don't touch our table (or assert) for them. - IF NOT EXISTS( - SELECT 1 FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE object_type = 'extension' - ) THEN + SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name) + INTO dropping + FROM pg_catalog.pg_event_trigger_dropped_objects() + WHERE object_type = 'extension'; + IF dropping IS NULL THEN RETURN; END IF; - /* - * Fail with an actionable error instead of the raw "permission denied" from the loop below. The table is - * found by catalog OIDs, not by name: name lookup silently skips a schema the caller can't USE, which - * would turn this into "does not exist". Not SECURITY DEFINER: that would run registered SQL as the owner. - */ - SELECT c.oid AS oid, c.relnamespace, n.nspname, c.relname, pg_catalog.pg_get_userbyid(c.relowner) AS owner - INTO t - FROM pg_catalog.pg_extension e - JOIN pg_catalog.pg_class c - ON c.relnamespace = e.extnamespace AND c.relname = 'extension_drop__commands' - JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace - WHERE e.extname = 'extension_drop'; - IF FOUND AND NOT ( - pg_catalog.has_schema_privilege(t.relnamespace, 'USAGE') - AND pg_catalog.has_table_privilege(t.oid, 'SELECT') - AND pg_catalog.has_table_privilege(t.oid, 'DELETE') - ) THEN - SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name) - INTO dropping - FROM pg_catalog.pg_event_trigger_dropped_objects() - WHERE object_type = 'extension'; + -- One statement touches the table, so a privilege error here is ours; registered SQL runs after the + -- handler so its own errors pass through untouched. + BEGIN + WITH del AS ( + DELETE FROM extension_drop__commands c + USING pg_catalog.pg_event_trigger_dropped_objects() d + WHERE c.extension_name = d.object_name AND d.object_type = 'extension' + RETURNING c.extension_name, c.sql + ) + SELECT pg_catalog.array_agg(extension_name), pg_catalog.array_agg(sql) INTO names, cmds FROM del; + EXCEPTION WHEN insufficient_privilege OR undefined_table THEN + -- undefined_table: name lookup skips a schema the caller lacks USAGE on, so the table "doesn't exist". + GET STACKED DIAGNOSTICS msg = MESSAGE_TEXT; + + -- Look up the table by OID path, not by name, for the same reason. + SELECT n.nspname, pg_catalog.format('%I.%I', n.nspname, c.relname) AS qname, pg_catalog.pg_get_userbyid(c.relowner) AS owner + INTO t + FROM pg_catalog.pg_extension e + JOIN pg_catalog.pg_class c + ON c.relnamespace = e.extnamespace AND c.relname = 'extension_drop__commands' + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE e.extname = 'extension_drop'; + IF NOT FOUND THEN + RAISE; + END IF; + RAISE EXCEPTION 'permission denied to drop extension(s) %', dropping USING ERRCODE = 'insufficient_privilege' , DETAIL = pg_catalog.format( - 'extension_drop must read and delete its registration in %s.%s.' - , pg_catalog.quote_ident(t.nspname), pg_catalog.quote_ident(t.relname)) + 'extension_drop must read and delete its registration in %s: %s', t.qname, msg) , HINT = pg_catalog.format( - 'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s.%s to the current role.' - , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname) - , pg_catalog.quote_ident(t.nspname), pg_catalog.quote_ident(t.relname)); - END IF; + 'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s to the current role.' + , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname), t.qname); + END; - FOR r IN - SELECT c.* - FROM extension_drop__commands c - JOIN pg_event_trigger_dropped_objects() d - ON c.extension_name = d.object_name - AND d.object_type = 'extension' - LOOP - RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', r.extension_name, r.sql; - EXECUTE r.sql; - DELETE FROM extension_drop__commands WHERE extension_name = r.extension_name; + FOR i IN 1 .. COALESCE(pg_catalog.array_length(cmds, 1), 0) LOOP + RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', names[i], cmds[i]; + EXECUTE cmds[i]; END LOOP; /* diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 3165d9f..5060786 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -268,61 +268,62 @@ DECLARE * record, not extension_drop__commands: a typed variable is resolved when the function compiles, before * the early return, and a role without access to the extension's schema then fails every unrelated DROP. */ - r record; t record; dropping text; + names text[]; + cmds text[]; + i int; + msg text; BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; -- Most drops involve no extension; don't touch our table (or assert) for them. - IF NOT EXISTS( - SELECT 1 FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE object_type = 'extension' - ) THEN + SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name) + INTO dropping + FROM pg_catalog.pg_event_trigger_dropped_objects() + WHERE object_type = 'extension'; + IF dropping IS NULL THEN RETURN; END IF; - /* - * Fail with an actionable error instead of the raw "permission denied" from the loop below. The table is - * found by catalog OIDs, not by name: name lookup silently skips a schema the caller can't USE, which - * would turn this into "does not exist". Not SECURITY DEFINER: that would run registered SQL as the owner. - */ - SELECT c.oid AS oid, c.relnamespace, n.nspname, c.relname, pg_catalog.pg_get_userbyid(c.relowner) AS owner - INTO t - FROM pg_catalog.pg_extension e - JOIN pg_catalog.pg_class c - ON c.relnamespace = e.extnamespace AND c.relname = 'extension_drop__commands' - JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace - WHERE e.extname = 'extension_drop'; - IF FOUND AND NOT ( - pg_catalog.has_schema_privilege(t.relnamespace, 'USAGE') - AND pg_catalog.has_table_privilege(t.oid, 'SELECT') - AND pg_catalog.has_table_privilege(t.oid, 'DELETE') - ) THEN - SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name) - INTO dropping - FROM pg_catalog.pg_event_trigger_dropped_objects() - WHERE object_type = 'extension'; + -- One statement touches the table, so a privilege error here is ours; registered SQL runs after the + -- handler so its own errors pass through untouched. + BEGIN + WITH del AS ( + DELETE FROM extension_drop__commands c + USING pg_catalog.pg_event_trigger_dropped_objects() d + WHERE c.extension_name = d.object_name AND d.object_type = 'extension' + RETURNING c.extension_name, c.sql + ) + SELECT pg_catalog.array_agg(extension_name), pg_catalog.array_agg(sql) INTO names, cmds FROM del; + EXCEPTION WHEN insufficient_privilege OR undefined_table THEN + -- undefined_table: name lookup skips a schema the caller lacks USAGE on, so the table "doesn't exist". + GET STACKED DIAGNOSTICS msg = MESSAGE_TEXT; + + -- Look up the table by OID path, not by name, for the same reason. + SELECT n.nspname, pg_catalog.format('%I.%I', n.nspname, c.relname) AS qname, pg_catalog.pg_get_userbyid(c.relowner) AS owner + INTO t + FROM pg_catalog.pg_extension e + JOIN pg_catalog.pg_class c + ON c.relnamespace = e.extnamespace AND c.relname = 'extension_drop__commands' + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE e.extname = 'extension_drop'; + IF NOT FOUND THEN + RAISE; + END IF; + RAISE EXCEPTION 'permission denied to drop extension(s) %', dropping USING ERRCODE = 'insufficient_privilege' , DETAIL = pg_catalog.format( - 'extension_drop must read and delete its registration in %s.%s.' - , pg_catalog.quote_ident(t.nspname), pg_catalog.quote_ident(t.relname)) + 'extension_drop must read and delete its registration in %s: %s', t.qname, msg) , HINT = pg_catalog.format( - 'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s.%s to the current role.' - , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname) - , pg_catalog.quote_ident(t.nspname), pg_catalog.quote_ident(t.relname)); - END IF; + 'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s to the current role.' + , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname), t.qname); + END; - FOR r IN - SELECT c.* - FROM extension_drop__commands c - JOIN pg_event_trigger_dropped_objects() d - ON c.extension_name = d.object_name - AND d.object_type = 'extension' - LOOP - RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', r.extension_name, r.sql; - EXECUTE r.sql; - DELETE FROM extension_drop__commands WHERE extension_name = r.extension_name; + FOR i IN 1 .. COALESCE(pg_catalog.array_length(cmds, 1), 0) LOOP + RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', names[i], cmds[i]; + EXECUTE cmds[i]; END LOOP; /* diff --git a/test/expected/drop_denied.out b/test/expected/drop_denied.out index d9906d7..1fc85be 100644 --- a/test/expected/drop_denied.out +++ b/test/expected/drop_denied.out @@ -1,5 +1,5 @@ \set ECHO none -1..20 +1..31 ok 1 - Create non-superuser role ok 2 - Create schema owned by role containing a registered extension ok 3 - DROP SCHEMA CASCADE without USAGE on extension_drop's schema @@ -8,16 +8,27 @@ ok 5 - DETAIL ok 6 - HINT ok 7 - Grant USAGE on extension_drop's schema ok 8 - DROP SCHEMA CASCADE with USAGE but no table privileges -ok 9 - Unregister the extension -ok 10 - DROP SCHEMA CASCADE of an unregistered extension -ok 11 - DROP OWNED BY -ok 12 - Superuser drop schema -ok 13 - No stale registration -ok 14 - Create role -ok 15 - Create test extension as role -ok 16 - Demote role -ok 17 - DROP EXTENSION by the extension's non-superuser owner -ok 18 - Superuser drop extension -ok 19 - No stale registration -ok 20 - extension_drop's schema should not be reachable via search_path +ok 9 - Capture DETAIL +ok 10 - DETAIL includes the original error +ok 11 - Unregister the extension +ok 12 - DROP SCHEMA CASCADE of an unregistered extension +ok 13 - DROP OWNED BY +ok 14 - Superuser drop schema +ok 15 - No stale registration +ok 16 - Recreate the registered extension +ok 17 - Grant SELECT +ok 18 - DROP SCHEMA CASCADE with SELECT but not DELETE +ok 19 - Grant DELETE, register SQL that needs CREATEROLE +ok 20 - Registered SQL's own privilege error surfaces unchanged +ok 21 - Register harmless SQL +ok 22 - DROP SCHEMA CASCADE with the privileges the HINT names +ok 23 - No stale registration +ok 24 - Registration removed +ok 25 - Create role +ok 26 - Create test extension as role +ok 27 - Demote role +ok 28 - DROP EXTENSION by the extension's non-superuser owner +ok 29 - Superuser drop extension +ok 30 - No stale registration +ok 31 - extension_drop's schema should not be reachable via search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_denied_1.out b/test/expected/drop_denied_1.out index 7617287..4d96c3a 100644 --- a/test/expected/drop_denied_1.out +++ b/test/expected/drop_denied_1.out @@ -1,5 +1,5 @@ \set ECHO none -1..20 +1..31 ok 1 - Create non-superuser role ok 2 - Create schema owned by role containing a registered extension ok 3 - DROP SCHEMA CASCADE without USAGE on extension_drop's schema @@ -8,16 +8,27 @@ ok 5 - DETAIL ok 6 - HINT ok 7 - Grant USAGE on extension_drop's schema ok 8 - DROP SCHEMA CASCADE with USAGE but no table privileges -ok 9 - Unregister the extension -ok 10 - DROP SCHEMA CASCADE of an unregistered extension -ok 11 - DROP OWNED BY -ok 12 - Superuser drop schema -ok 13 - No stale registration -ok 14 - Create role -ok 15 - Create test extension as role -ok 16 - Demote role -ok 17 - DROP EXTENSION by the extension's non-superuser owner -ok 18 - Superuser drop extension -ok 19 - No stale registration -ok 20 # SKIP existing mode: extension_drop's placement is outside this suite's control +ok 9 - Capture DETAIL +ok 10 - DETAIL includes the original error +ok 11 - Unregister the extension +ok 12 - DROP SCHEMA CASCADE of an unregistered extension +ok 13 - DROP OWNED BY +ok 14 - Superuser drop schema +ok 15 - No stale registration +ok 16 - Recreate the registered extension +ok 17 - Grant SELECT +ok 18 - DROP SCHEMA CASCADE with SELECT but not DELETE +ok 19 - Grant DELETE, register SQL that needs CREATEROLE +ok 20 - Registered SQL's own privilege error surfaces unchanged +ok 21 - Register harmless SQL +ok 22 - DROP SCHEMA CASCADE with the privileges the HINT names +ok 23 - No stale registration +ok 24 - Registration removed +ok 25 - Create role +ok 26 - Create test extension as role +ok 27 - Demote role +ok 28 - DROP EXTENSION by the extension's non-superuser owner +ok 29 - Superuser drop extension +ok 30 - No stale registration +ok 31 # SKIP existing mode: extension_drop's placement is outside this suite's control # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/drop_denied.sql b/test/sql/drop_denied.sql index 39de81f..e1ea177 100644 --- a/test/sql/drop_denied.sql +++ b/test/sql/drop_denied.sql @@ -12,9 +12,15 @@ SELECT plan( + 1 -- DROP SCHEMA CASCADE without USAGE on extension_drop's schema + 3 -- DETAIL and HINT + 2 -- USAGE on extension_drop's schema, but no table privileges + + 2 -- DETAIL + 2 -- Unregistered extension + 1 -- DROP OWNED BY + 2 -- Cleanup + + 1 -- Recreate the registered extension + + 2 -- SELECT but not DELETE + + 2 -- SELECT and DELETE, registered SQL fails with its own privilege error + + 3 -- SELECT and DELETE, registered SQL succeeds + + 1 -- Registration removed + 4 -- DROP EXTENSION by a role that was a superuser when it created it + 2 -- Cleanup + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) @@ -56,9 +62,13 @@ SELECT lives_ok( END $$$q$ , 'Capture DETAIL and HINT' ); -SELECT is( +-- Without USAGE the table can't be found by name ("does not exist"); in existing mode the schema is +-- public, which everyone can use, so it is "permission denied" instead. +SELECT matches( current_setting('extension_drop_test.detail') - , format('extension_drop must read and delete its registration in %I.extension_drop__commands.', :'extension_drop_schema') + , format( + '^extension_drop must read and delete its registration in %s\.extension_drop__commands: (relation "extension_drop__commands" does not exist|permission denied for table extension_drop__commands)$' + , regexp_replace(quote_ident(:'extension_drop_schema'), '([^a-zA-Z0-9_])', '\\\1', 'g')) , 'DETAIL' ); SELECT is( @@ -79,6 +89,25 @@ SELECT throws_ok( , :'MSG' , 'DROP SCHEMA CASCADE with USAGE but no table privileges' ); +SELECT lives_ok( + $q$DO $$ + DECLARE d text; + BEGIN + SET LOCAL ROLE extension_drop_test_plain; + DROP SCHEMA extension_drop_test_plain_schema CASCADE; + EXCEPTION WHEN insufficient_privilege THEN + GET STACKED DIAGNOSTICS d = PG_EXCEPTION_DETAIL; + PERFORM set_config('extension_drop_test.detail', d, false); + END $$$q$ + , 'Capture DETAIL' +); +SELECT is( + current_setting('extension_drop_test.detail') + , format( + 'extension_drop must read and delete its registration in %I.extension_drop__commands: permission denied for table extension_drop__commands' + , :'extension_drop_schema') + , 'DETAIL includes the original error' +); SELECT lives_ok( format($$SELECT %I.extension_drop__remove('extension_drop_test')$$, :'extension_drop_schema') @@ -107,6 +136,55 @@ SELECT is_empty( , 'No stale registration' ); +SELECT lives_ok( + $$CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain; + CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_plain_schema$$ + , 'Recreate the registered extension' +); + +-- SELECT alone is not enough: the trigger deletes the registration +SELECT lives_ok( + format('GRANT SELECT ON %I.extension_drop__commands TO extension_drop_test_plain', :'extension_drop_schema') + , 'Grant SELECT' +); +SELECT throws_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + , '42501' + , :'MSG' + , 'DROP SCHEMA CASCADE with SELECT but not DELETE' +); + +-- A privilege error from the registered SQL itself is not extension_drop's to translate +SELECT lives_ok( + format($$GRANT DELETE ON %1$I.extension_drop__commands TO extension_drop_test_plain; + SELECT %1$I.extension_drop__update('extension_drop_test', 'CREATE ROLE extension_drop_test_never')$$ + , :'extension_drop_schema') + , 'Grant DELETE, register SQL that needs CREATEROLE' +); +SELECT throws_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + , '42501' + , 'permission denied to create role' + , 'Registered SQL''s own privilege error surfaces unchanged' +); + +SELECT lives_ok( + format($$SELECT %I.extension_drop__update('extension_drop_test', 'SELECT 1')$$, :'extension_drop_schema') + , 'Register harmless SQL' +); +SELECT lives_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; RESET ROLE; END $$$q$ + , 'DROP SCHEMA CASCADE with the privileges the HINT names' +); +SELECT is_empty( + format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') + , 'No stale registration' +); +SELECT is_empty( + format($$SELECT 1 FROM %I.extension_drop__commands WHERE extension_name = 'extension_drop_test'$$, :'extension_drop_schema') + , 'Registration removed' +); + -- ALTER EXTENSION can't change the owner, so create it as a role that is then demoted SELECT lives_ok( 'CREATE ROLE extension_drop_test_owner SUPERUSER' From 4c5460c01b2d435e183269c6915ddc48c316198c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:25:35 -0500 Subject: [PATCH 10/13] Tighten privilege-error docs; test the HINT's grants and a group role drop_denied now applies the grants the HINT names (directly, and through a group role) and checks the drop succeeds. HISTORY.asc and README.md no longer describe DROP EXTENSION as newly restricted, describe the trigger's current order (registration removed, then registered SQL run), and note that the registered SQL runs as the caller and may need further privileges. --- HISTORY.asc | 13 ++++++++----- README.md | 14 ++++++++------ test/expected/drop_denied.out | 19 +++++++++++-------- test/expected/drop_denied_1.out | 19 +++++++++++-------- test/sql/drop_denied.sql | 24 ++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 27 deletions(-) diff --git a/HISTORY.asc b/HISTORY.asc index 4a6129e..e92162b 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -14,11 +14,14 @@ commands skipped the registered SQL and left a stale registration that made every later `DROP EXTENSION` fail with `XD001`; remove any existing stale registration with `extension_drop__repair()`. -Dropping an extension, by any of the three commands, now requires access to -`extension_drop__commands` (USAGE on its schema, SELECT and DELETE on it), -whether or not the extension is registered. A role without it gets an -`insufficient_privilege` error naming the table's owner and the grants that -would fix it, and quoting the underlying error, instead of `permission denied for table extension_drop__commands`. +`DROP SCHEMA ... CASCADE` and `DROP OWNED BY` that drop an extension now need +the same access to `extension_drop__commands` that `DROP EXTENSION` always +did (USAGE on its schema, SELECT and DELETE on it), whether or not the +extension is registered. A role without it gets an `insufficient_privilege` +error that quotes the underlying error and names the table's owner and the +grants that give that table access, instead of `permission denied for table +extension_drop__commands` or `type "extension_drop__commands" does not exist`. +The registered SQL runs as the caller and may need further privileges. == Remove redundant client_min_messages handling from install script `CREATE EXTENSION` already raises `client_min_messages` to `WARNING` for diff --git a/README.md b/README.md index 460cba4..dd1db5f 100644 --- a/README.md +++ b/README.md @@ -43,13 +43,15 @@ arbitrary SQL to run later. Grant them to specific roles as needed. An event trigger (`extension_drop`, on `sql_drop`) fires whenever an extension is dropped -- by `DROP EXTENSION`, `DROP SCHEMA ... CASCADE` or -`DROP OWNED BY` -- runs any SQL registered for it via -`extension_drop__add`/`extension_drop__update`, and removes the registration -afterward. Those commands fire the trigger for every drop, but it returns -immediately unless an extension is among the dropped objects. +`DROP OWNED BY` -- removes its registration from `extension_drop__commands` +and runs the SQL registered for it via +`extension_drop__add`/`extension_drop__update`. Those commands fire the +trigger for every drop, but it returns immediately unless an extension is +among the dropped objects. Dropping an extension therefore needs access to `extension_drop__commands` (USAGE on its schema, SELECT and DELETE on the table), whether or not the extension is registered. A role without it gets an `insufficient_privilege` -error that names the table's owner, the grants that would fix it and the underlying error; a -superuser, or the role that installed `extension_drop`, always has it. +error that quotes the underlying error and names the table's owner and the +grants that give that table access. The registered SQL runs as the caller and +may need further privileges. diff --git a/test/expected/drop_denied.out b/test/expected/drop_denied.out index 1fc85be..36eb35d 100644 --- a/test/expected/drop_denied.out +++ b/test/expected/drop_denied.out @@ -1,5 +1,5 @@ \set ECHO none -1..31 +1..34 ok 1 - Create non-superuser role ok 2 - Create schema owned by role containing a registered extension ok 3 - DROP SCHEMA CASCADE without USAGE on extension_drop's schema @@ -24,11 +24,14 @@ ok 21 - Register harmless SQL ok 22 - DROP SCHEMA CASCADE with the privileges the HINT names ok 23 - No stale registration ok 24 - Registration removed -ok 25 - Create role -ok 26 - Create test extension as role -ok 27 - Demote role -ok 28 - DROP EXTENSION by the extension's non-superuser owner -ok 29 - Superuser drop extension -ok 30 - No stale registration -ok 31 - extension_drop's schema should not be reachable via search_path +ok 25 - Recreate the registered extension +ok 26 - Move the grants from the role to a group it belongs to +ok 27 - DROP SCHEMA CASCADE with the privileges held through a group role +ok 28 - Create role +ok 29 - Create test extension as role +ok 30 - Demote role +ok 31 - DROP EXTENSION by the extension's non-superuser owner +ok 32 - Superuser drop extension +ok 33 - No stale registration +ok 34 - extension_drop's schema should not be reachable via search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_denied_1.out b/test/expected/drop_denied_1.out index 4d96c3a..3b088bf 100644 --- a/test/expected/drop_denied_1.out +++ b/test/expected/drop_denied_1.out @@ -1,5 +1,5 @@ \set ECHO none -1..31 +1..34 ok 1 - Create non-superuser role ok 2 - Create schema owned by role containing a registered extension ok 3 - DROP SCHEMA CASCADE without USAGE on extension_drop's schema @@ -24,11 +24,14 @@ ok 21 - Register harmless SQL ok 22 - DROP SCHEMA CASCADE with the privileges the HINT names ok 23 - No stale registration ok 24 - Registration removed -ok 25 - Create role -ok 26 - Create test extension as role -ok 27 - Demote role -ok 28 - DROP EXTENSION by the extension's non-superuser owner -ok 29 - Superuser drop extension -ok 30 - No stale registration -ok 31 # SKIP existing mode: extension_drop's placement is outside this suite's control +ok 25 - Recreate the registered extension +ok 26 - Move the grants from the role to a group it belongs to +ok 27 - DROP SCHEMA CASCADE with the privileges held through a group role +ok 28 - Create role +ok 29 - Create test extension as role +ok 30 - Demote role +ok 31 - DROP EXTENSION by the extension's non-superuser owner +ok 32 - Superuser drop extension +ok 33 - No stale registration +ok 34 # SKIP existing mode: extension_drop's placement is outside this suite's control # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/drop_denied.sql b/test/sql/drop_denied.sql index e1ea177..0134af6 100644 --- a/test/sql/drop_denied.sql +++ b/test/sql/drop_denied.sql @@ -21,6 +21,7 @@ SELECT plan( + 2 -- SELECT and DELETE, registered SQL fails with its own privilege error + 3 -- SELECT and DELETE, registered SQL succeeds + 1 -- Registration removed + + 3 -- The same privileges held through a group role + 4 -- DROP EXTENSION by a role that was a superuser when it created it + 2 -- Cleanup + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) @@ -185,6 +186,29 @@ SELECT is_empty( , 'Registration removed' ); +-- Privileges held only through a group role the caller is a member of +SELECT lives_ok( + $$CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain; + CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_plain_schema$$ + , 'Recreate the registered extension' +); +SELECT lives_ok( + format( + $$SELECT %1$I.extension_drop__update('extension_drop_test', 'SELECT 1'); + REVOKE SELECT, DELETE ON %1$I.extension_drop__commands FROM extension_drop_test_plain; + REVOKE USAGE ON SCHEMA %1$I FROM extension_drop_test_plain; + CREATE ROLE extension_drop_test_group; + GRANT USAGE ON SCHEMA %1$I TO extension_drop_test_group; + GRANT SELECT, DELETE ON %1$I.extension_drop__commands TO extension_drop_test_group; + GRANT extension_drop_test_group TO extension_drop_test_plain$$ + , :'extension_drop_schema') + , 'Move the grants from the role to a group it belongs to' +); +SELECT lives_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; RESET ROLE; END $$$q$ + , 'DROP SCHEMA CASCADE with the privileges held through a group role' +); + -- ALTER EXTENSION can't change the owner, so create it as a role that is then demoted SELECT lives_ok( 'CREATE ROLE extension_drop_test_owner SUPERUSER' From fc01223b4ce040fba0a0b53995c993606668c218 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:27:55 -0500 Subject: [PATCH 11/13] Use block comments where the SQL linter requires them --- sql/extension_drop--0.1.1--stable.sql | 6 ++++-- sql/extension_drop.sql | 6 ++++-- test/sql/drop_denied.sql | 6 ++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index bda1406..5d493a2 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -24,8 +24,10 @@ BEGIN RETURN; END IF; - -- One statement touches the table, so a privilege error here is ours; registered SQL runs after the - -- handler so its own errors pass through untouched. + /* + * One statement touches the table, so a privilege error here is ours; registered SQL runs after the + * handler so its own errors pass through untouched. + */ BEGIN WITH del AS ( DELETE FROM extension_drop__commands c diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 5060786..44e84eb 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -286,8 +286,10 @@ BEGIN RETURN; END IF; - -- One statement touches the table, so a privilege error here is ours; registered SQL runs after the - -- handler so its own errors pass through untouched. + /* + * One statement touches the table, so a privilege error here is ours; registered SQL runs after the + * handler so its own errors pass through untouched. + */ BEGIN WITH del AS ( DELETE FROM extension_drop__commands c diff --git a/test/sql/drop_denied.sql b/test/sql/drop_denied.sql index 0134af6..16efbd3 100644 --- a/test/sql/drop_denied.sql +++ b/test/sql/drop_denied.sql @@ -63,8 +63,10 @@ SELECT lives_ok( END $$$q$ , 'Capture DETAIL and HINT' ); --- Without USAGE the table can't be found by name ("does not exist"); in existing mode the schema is --- public, which everyone can use, so it is "permission denied" instead. +/* + * Without USAGE the table can't be found by name ("does not exist"); in existing mode the schema is + * public, which everyone can use, so it is "permission denied" instead. + */ SELECT matches( current_setting('extension_drop_test.detail') , format( From d5906b777128b8a6ca4ae4e12dfdbc933407efe8 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:37:30 -0500 Subject: [PATCH 12/13] Let unregistered extensions drop without access to extension_drop__commands Add a SECURITY DEFINER extension_drop__registered(name[]) that the event trigger uses to find which dropped extensions are registered. Only those go through the trapped DELETE as the caller. --- HISTORY.asc | 20 ++- README.md | 19 ++- sql/extension_drop--0.1.1--stable.sql | 58 ++++++- sql/extension_drop.sql | 72 +++++++- test/build/expected/build.out | 2 + test/expected/drop_denied.out | 74 ++++++--- test/expected/drop_denied_1.out | 74 ++++++--- test/sql/drop_denied.sql | 226 +++++++++++++++++++------- 8 files changed, 406 insertions(+), 139 deletions(-) diff --git a/HISTORY.asc b/HISTORY.asc index e92162b..b408b84 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -14,14 +14,18 @@ commands skipped the registered SQL and left a stale registration that made every later `DROP EXTENSION` fail with `XD001`; remove any existing stale registration with `extension_drop__repair()`. -`DROP SCHEMA ... CASCADE` and `DROP OWNED BY` that drop an extension now need -the same access to `extension_drop__commands` that `DROP EXTENSION` always -did (USAGE on its schema, SELECT and DELETE on it), whether or not the -extension is registered. A role without it gets an `insufficient_privilege` -error that quotes the underlying error and names the table's owner and the -grants that give that table access, instead of `permission denied for table -extension_drop__commands` or `type "extension_drop__commands" does not exist`. -The registered SQL runs as the caller and may need further privileges. +Dropping an unregistered extension (by any of the three commands) needs no +access to `extension_drop__commands`: the trigger asks the new +`SECURITY DEFINER` function `extension_drop__registered()`, executable by +`PUBLIC`. Dropping a registered extension needs USAGE on the extension's +schema and SELECT and DELETE on the table. A role without that access gets +an `insufficient_privilege` error that quotes the underlying error and names +the table's owner and the grants that give that table access, instead of +`permission denied for table extension_drop__commands` or `type +"extension_drop__commands" does not exist`. A caller without USAGE on the +schema gets that error even for an unregistered extension, since it can't +resolve the helper. The registered SQL runs as the caller and may need +further privileges. == Remove redundant client_min_messages handling from install script `CREATE EXTENSION` already raises `client_min_messages` to `WARNING` for diff --git a/README.md b/README.md index dd1db5f..e58776d 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,13 @@ arbitrary SQL to run later. Grant them to specific roles as needed. Look up the row (`extension_name`, `sql`) registered for `extension_name`. Granted to `PUBLIC` by default. +- `extension_drop__registered(extension_names name[]) RETURNS name[]` + Returns those of `extension_names` that have registered drop commands, and + asserts the table is sane when there are none. `SECURITY DEFINER` (with + `search_path` pinned to the extension's schema, then `pg_temp`); used by + the event trigger so dropping an unregistered extension needs no table + access. Granted to `PUBLIC` by default. + - `extension_drop__sanity_check(ignore name DEFAULT NULL) RETURNS name[]` Returns the names of extensions that have registered drop commands but no longer exist. Should always return an empty array; pass an extension name @@ -49,9 +56,15 @@ and runs the SQL registered for it via trigger for every drop, but it returns immediately unless an extension is among the dropped objects. -Dropping an extension therefore needs access to `extension_drop__commands` -(USAGE on its schema, SELECT and DELETE on the table), whether or not the -extension is registered. A role without it gets an `insufficient_privilege` +Dropping an unregistered extension needs no access to `extension_drop__commands`; +the trigger only asks `extension_drop__registered()` (a `SECURITY DEFINER` +function) which of the dropped extensions are registered. Dropping a +registered extension needs access to the table (USAGE on its schema, SELECT +and DELETE on the table). A role without it gets an `insufficient_privilege` error that quotes the underlying error and names the table's owner and the grants that give that table access. The registered SQL runs as the caller and may need further privileges. + +A caller without USAGE on the extension's schema can't resolve +`extension_drop__registered()`, so it gets that error even for an +unregistered extension. diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index 5d493a2..7eaaa23 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -1,13 +1,47 @@ +/* + * The install script's create_function doesn't exist here, so build the helper directly; see + * extension_drop.sql for why it is SECURITY DEFINER with pg_temp last on its search_path. + */ +DO $do$ +DECLARE + sp text; +BEGIN + SELECT pg_catalog.format('%I', n.nspname) INTO sp + FROM pg_catalog.pg_extension e JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace + WHERE e.extname = 'extension_drop'; + EXECUTE pg_catalog.format($f$ +CREATE OR REPLACE FUNCTION extension_drop__registered( + extension_names name[] +) RETURNS name[] LANGUAGE plpgsql STABLE SECURITY DEFINER SET search_path = %s, pg_temp AS +$body$ +DECLARE + ret name[] := array( + SELECT extension_name FROM extension_drop__commands WHERE extension_name = ANY(extension_names) ORDER BY 1 + ); +BEGIN + -- With nothing registered the trigger returns without its own assert, so any stray row is a stale one. + IF ret = '{}'::name[] THEN + PERFORM extension_drop__sanity_assert(); + END IF; + RETURN ret; +END +$body$ +$f$, sp); +END +$do$; +REVOKE ALL ON FUNCTION extension_drop__registered(name[]) FROM public; +GRANT EXECUTE ON FUNCTION extension_drop__registered(name[]) TO public; +COMMENT ON FUNCTION extension_drop__registered(name[]) IS + $$Returns the given extensions that have drop commands, after asserting the table is sane if there are none.$$; + CREATE OR REPLACE FUNCTION extension_drop__event_trigger( ) RETURNS event_trigger LANGUAGE plpgsql SET search_path FROM CURRENT AS $body$ DECLARE - /* - * record, not extension_drop__commands: a typed variable is resolved when the function compiles, before - * the early return, and a role without access to the extension's schema then fails every unrelated DROP. - */ t record; dropping text; + dropped name[]; + registered name[]; names text[]; cmds text[]; i int; @@ -17,13 +51,27 @@ BEGIN -- Most drops involve no extension; don't touch our table (or assert) for them. SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name) - INTO dropping + , pg_catalog.array_agg(object_name::name ORDER BY object_name) + INTO dropping, dropped FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE object_type = 'extension'; IF dropping IS NULL THEN RETURN; END IF; + /* + * Callers without table access can still drop unregistered extensions. Without USAGE on our schema the + * helper isn't found; fall through to the DELETE below for its clearer error. + */ + BEGIN + registered := extension_drop__registered(dropped); + EXCEPTION WHEN undefined_function OR insufficient_privilege THEN + registered := NULL; + END; + IF registered = '{}'::name[] THEN + RETURN; + END IF; + /* * One statement touches the table, so a privilege error here is ours; registered SQL runs after the * handler so its own errors pass through untouched. diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 44e84eb..97a17ac 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -37,10 +37,24 @@ CREATE FUNCTION __extension_drop.create_function( DECLARE c_clean_args text := cat_tools.routine__parse_arg_types_text(args); + -- A definer function must not search pg_temp ahead of its own schema, so pin the path explicitly. + search_path_clause text := CASE + WHEN options ~* 'security\s+definer' THEN COALESCE( + ( + SELECT pg_catalog.format('= %I, pg_temp', n.nspname) + FROM pg_catalog.pg_extension e JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace + WHERE e.extname = 'extension_drop' + ) + -- Not running as an extension script (e.g. the build smoke test) + , pg_catalog.format('= %I, pg_temp', pg_catalog.current_schema()) + ) + ELSE 'FROM CURRENT' + END; + create_template CONSTANT text := $template$ CREATE OR REPLACE FUNCTION %s( %s -) RETURNS %s SET search_path FROM CURRENT AS +) RETURNS %s SET search_path %s AS %L $template$ ; @@ -71,7 +85,8 @@ BEGIN create_template , function_name , args - , options -- TODO: Force search_path if options ~* 'definer' + , options + , search_path_clause , body ) ) ; @@ -255,6 +270,37 @@ $body$ , '' -- Just revoke all access ); +/* + * REGISTERED + */ +/* + * SECURITY DEFINER so the trigger can tell whether a dropped extension is registered without the caller + * needing table access; reads only, never runs registered SQL. The path is pinned with pg_temp last + * (see create_function) so a caller's temp table can't shadow ours. + */ +SELECT __extension_drop.create_function( + 'extension_drop__registered' + , $$ + extension_names name[] +$$ + , 'name[] LANGUAGE plpgsql STABLE SECURITY DEFINER' + , $body$ +DECLARE + ret name[] := array( + SELECT extension_name FROM extension_drop__commands WHERE extension_name = ANY(extension_names) ORDER BY 1 + ); +BEGIN + -- With nothing registered the trigger returns without its own assert, so any stray row is a stale one. + IF ret = '{}'::name[] THEN + PERFORM extension_drop__sanity_assert(); + END IF; + RETURN ret; +END +$body$ + , $$Returns the given extensions that have drop commands, after asserting the table is sane if there are none.$$ + , 'public' +); + /* * TRIGGER FUNCTION */ @@ -264,12 +310,10 @@ SELECT __extension_drop.create_function( , 'event_trigger LANGUAGE plpgsql' , $body$ DECLARE - /* - * record, not extension_drop__commands: a typed variable is resolved when the function compiles, before - * the early return, and a role without access to the extension's schema then fails every unrelated DROP. - */ t record; dropping text; + dropped name[]; + registered name[]; names text[]; cmds text[]; i int; @@ -279,13 +323,27 @@ BEGIN -- Most drops involve no extension; don't touch our table (or assert) for them. SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name) - INTO dropping + , pg_catalog.array_agg(object_name::name ORDER BY object_name) + INTO dropping, dropped FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE object_type = 'extension'; IF dropping IS NULL THEN RETURN; END IF; + /* + * Callers without table access can still drop unregistered extensions. Without USAGE on our schema the + * helper isn't found; fall through to the DELETE below for its clearer error. + */ + BEGIN + registered := extension_drop__registered(dropped); + EXCEPTION WHEN undefined_function OR insufficient_privilege THEN + registered := NULL; + END; + IF registered = '{}'::name[] THEN + RETURN; + END IF; + /* * One statement touches the table, so a privilege error here is ours; registered SQL runs after the * handler so its own errors pass through untouched. diff --git a/test/build/expected/build.out b/test/build/expected/build.out index 44215eb..c66ee3c 100644 --- a/test/build/expected/build.out +++ b/test/build/expected/build.out @@ -19,4 +19,6 @@ INSTALL + + # TRANSACTION INTENTIONALLY LEFT OPEN diff --git a/test/expected/drop_denied.out b/test/expected/drop_denied.out index 36eb35d..8800d78 100644 --- a/test/expected/drop_denied.out +++ b/test/expected/drop_denied.out @@ -1,37 +1,57 @@ \set ECHO none -1..34 +1..54 ok 1 - Create non-superuser role ok 2 - Create schema owned by role containing a registered extension -ok 3 - DROP SCHEMA CASCADE without USAGE on extension_drop's schema +ok 3 - Registered: DROP SCHEMA CASCADE without USAGE on extension_drop's schema ok 4 - Capture DETAIL and HINT ok 5 - DETAIL ok 6 - HINT ok 7 - Grant USAGE on extension_drop's schema -ok 8 - DROP SCHEMA CASCADE with USAGE but no table privileges +ok 8 - Registered: DROP SCHEMA CASCADE with USAGE but no table privileges ok 9 - Capture DETAIL ok 10 - DETAIL includes the original error -ok 11 - Unregister the extension -ok 12 - DROP SCHEMA CASCADE of an unregistered extension -ok 13 - DROP OWNED BY -ok 14 - Superuser drop schema -ok 15 - No stale registration -ok 16 - Recreate the registered extension -ok 17 - Grant SELECT -ok 18 - DROP SCHEMA CASCADE with SELECT but not DELETE -ok 19 - Grant DELETE, register SQL that needs CREATEROLE -ok 20 - Registered SQL's own privilege error surfaces unchanged -ok 21 - Register harmless SQL -ok 22 - DROP SCHEMA CASCADE with the privileges the HINT names -ok 23 - No stale registration -ok 24 - Registration removed -ok 25 - Recreate the registered extension -ok 26 - Move the grants from the role to a group it belongs to -ok 27 - DROP SCHEMA CASCADE with the privileges held through a group role -ok 28 - Create role -ok 29 - Create test extension as role -ok 30 - Demote role -ok 31 - DROP EXTENSION by the extension's non-superuser owner -ok 32 - Superuser drop extension -ok 33 - No stale registration -ok 34 - extension_drop's schema should not be reachable via search_path +ok 11 - Grant SELECT +ok 12 - Registered: DROP SCHEMA CASCADE with SELECT but not DELETE +ok 13 - Grant DELETE, register SQL that needs CREATEROLE +ok 14 - Registered SQL's own privilege error surfaces unchanged +ok 15 - Register harmless SQL +ok 16 - Registered: DROP SCHEMA CASCADE with the privileges the HINT names +ok 17 - No stale registration +ok 18 - Registration removed +ok 19 - Recreate the registered extension +ok 20 - Move the grants from the role to a group it belongs to +ok 21 - Registered: DROP SCHEMA CASCADE with the privileges held through a group role +ok 22 - Remove all of the role's access +ok 23 - Recreate the extension +ok 24 - Unregister the extension +ok 25 - Unregistered: DROP SCHEMA CASCADE without USAGE on extension_drop's schema +ok 26 - Grant USAGE on extension_drop's schema +ok 27 - Unregistered: DROP SCHEMA CASCADE with USAGE but no table privileges +ok 28 - Recreate the extension +ok 29 - Unregister the extension +ok 30 - Unregistered: DROP OWNED BY with USAGE but no table privileges +ok 31 - Recreate the extension +ok 32 - Unregister the extension +ok 33 - Grant SELECT +ok 34 - Unregistered: DROP SCHEMA CASCADE with SELECT but not DELETE +ok 35 - Revoke SELECT +ok 36 - Recreate the registered extension +ok 37 - Registered extension is still found when the caller has a temp table of the same name +ok 38 - Unregister the extension +ok 39 - Superuser drop schema +ok 40 - Recreate the extension +ok 41 - Unregister the extension +ok 42 - Insert a stale registration +ok 43 - Unregistered: DROP SCHEMA CASCADE with a stale registration present +ok 44 - Repair +ok 45 - Superuser drop schema +ok 46 - Create role +ok 47 - Create test extension as role +ok 48 - Demote role +ok 49 - DROP EXTENSION by the extension's non-superuser owner +ok 50 - Unregister the extension +ok 51 - Grant USAGE on extension_drop's schema +ok 52 - Unregistered: DROP EXTENSION by the extension's non-superuser owner with USAGE but no table privileges +ok 53 - No stale registration +ok 54 - extension_drop's schema should not be reachable via search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_denied_1.out b/test/expected/drop_denied_1.out index 3b088bf..a50673a 100644 --- a/test/expected/drop_denied_1.out +++ b/test/expected/drop_denied_1.out @@ -1,37 +1,57 @@ \set ECHO none -1..34 +1..54 ok 1 - Create non-superuser role ok 2 - Create schema owned by role containing a registered extension -ok 3 - DROP SCHEMA CASCADE without USAGE on extension_drop's schema +ok 3 - Registered: DROP SCHEMA CASCADE without USAGE on extension_drop's schema ok 4 - Capture DETAIL and HINT ok 5 - DETAIL ok 6 - HINT ok 7 - Grant USAGE on extension_drop's schema -ok 8 - DROP SCHEMA CASCADE with USAGE but no table privileges +ok 8 - Registered: DROP SCHEMA CASCADE with USAGE but no table privileges ok 9 - Capture DETAIL ok 10 - DETAIL includes the original error -ok 11 - Unregister the extension -ok 12 - DROP SCHEMA CASCADE of an unregistered extension -ok 13 - DROP OWNED BY -ok 14 - Superuser drop schema -ok 15 - No stale registration -ok 16 - Recreate the registered extension -ok 17 - Grant SELECT -ok 18 - DROP SCHEMA CASCADE with SELECT but not DELETE -ok 19 - Grant DELETE, register SQL that needs CREATEROLE -ok 20 - Registered SQL's own privilege error surfaces unchanged -ok 21 - Register harmless SQL -ok 22 - DROP SCHEMA CASCADE with the privileges the HINT names -ok 23 - No stale registration -ok 24 - Registration removed -ok 25 - Recreate the registered extension -ok 26 - Move the grants from the role to a group it belongs to -ok 27 - DROP SCHEMA CASCADE with the privileges held through a group role -ok 28 - Create role -ok 29 - Create test extension as role -ok 30 - Demote role -ok 31 - DROP EXTENSION by the extension's non-superuser owner -ok 32 - Superuser drop extension -ok 33 - No stale registration -ok 34 # SKIP existing mode: extension_drop's placement is outside this suite's control +ok 11 - Grant SELECT +ok 12 - Registered: DROP SCHEMA CASCADE with SELECT but not DELETE +ok 13 - Grant DELETE, register SQL that needs CREATEROLE +ok 14 - Registered SQL's own privilege error surfaces unchanged +ok 15 - Register harmless SQL +ok 16 - Registered: DROP SCHEMA CASCADE with the privileges the HINT names +ok 17 - No stale registration +ok 18 - Registration removed +ok 19 - Recreate the registered extension +ok 20 - Move the grants from the role to a group it belongs to +ok 21 - Registered: DROP SCHEMA CASCADE with the privileges held through a group role +ok 22 - Remove all of the role's access +ok 23 - Recreate the extension +ok 24 - Unregister the extension +ok 25 # SKIP existing mode: the role can use extension_drop's schema +ok 26 - Grant USAGE on extension_drop's schema +ok 27 - Unregistered: DROP SCHEMA CASCADE with USAGE but no table privileges +ok 28 - Recreate the extension +ok 29 - Unregister the extension +ok 30 - Unregistered: DROP OWNED BY with USAGE but no table privileges +ok 31 - Recreate the extension +ok 32 - Unregister the extension +ok 33 - Grant SELECT +ok 34 - Unregistered: DROP SCHEMA CASCADE with SELECT but not DELETE +ok 35 - Revoke SELECT +ok 36 - Recreate the registered extension +ok 37 - Registered extension is still found when the caller has a temp table of the same name +ok 38 - Unregister the extension +ok 39 - Superuser drop schema +ok 40 - Recreate the extension +ok 41 - Unregister the extension +ok 42 - Insert a stale registration +ok 43 - Unregistered: DROP SCHEMA CASCADE with a stale registration present +ok 44 - Repair +ok 45 - Superuser drop schema +ok 46 - Create role +ok 47 - Create test extension as role +ok 48 - Demote role +ok 49 - DROP EXTENSION by the extension's non-superuser owner +ok 50 - Unregister the extension +ok 51 - Grant USAGE on extension_drop's schema +ok 52 - Unregistered: DROP EXTENSION by the extension's non-superuser owner with USAGE but no table privileges +ok 53 - No stale registration +ok 54 # SKIP existing mode: extension_drop's placement is outside this suite's control # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/drop_denied.sql b/test/sql/drop_denied.sql index 16efbd3..0a75ef4 100644 --- a/test/sql/drop_denied.sql +++ b/test/sql/drop_denied.sql @@ -2,10 +2,11 @@ \i test/pgxntool/setup.sql /* - * A role that can't use extension_drop's registration table must get an - * actionable insufficient_privilege error, not a raw "permission denied for - * table", whenever a drop includes an extension -- registered or not -- via - * any of the three commands the trigger fires for. + * Dropping an extension that is registered with extension_drop needs access to + * extension_drop's registration table; without it the caller must get an actionable + * insufficient_privilege error, not a raw "permission denied for table". Dropping an + * unregistered extension needs no grants at all, but a stale registration still fails. + * Covers the three commands the trigger fires for. */ SELECT plan( 2 -- Setup: role, schema with extension @@ -13,17 +14,19 @@ SELECT plan( + 3 -- DETAIL and HINT + 2 -- USAGE on extension_drop's schema, but no table privileges + 2 -- DETAIL - + 2 -- Unregistered extension - + 1 -- DROP OWNED BY - + 2 -- Cleanup - + 1 -- Recreate the registered extension + 2 -- SELECT but not DELETE + 2 -- SELECT and DELETE, registered SQL fails with its own privilege error - + 3 -- SELECT and DELETE, registered SQL succeeds + + 4 -- SELECT and DELETE, registered SQL succeeds + 1 -- Registration removed - + 3 -- The same privileges held through a group role + + 4 -- The same privileges held through a group role + + 3 -- Unregistered, no USAGE: recreate, unregister, DROP SCHEMA CASCADE + + 2 -- Unregistered, USAGE only: DROP SCHEMA CASCADE + + 3 -- Unregistered, USAGE only: DROP OWNED BY + + 5 -- Unregistered, SELECT but not DELETE + + 4 -- Temp table shadowing extension_drop__commands + + 6 -- Stale registration + 4 -- DROP EXTENSION by a role that was a superuser when it created it - + 2 -- Cleanup + + 3 -- DROP EXTENSION of an unregistered extension by that role + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) ); @@ -31,6 +34,10 @@ SELECT plan( SET client_min_messages = ERROR; \set MSG 'permission denied to drop extension(s) extension_drop_test' +\set DROP_SCHEMA 'DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; RESET ROLE; END $$' +\set DROP_OWNED 'DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP OWNED BY extension_drop_test_plain CASCADE; RESET ROLE; END $$' +\set RECREATE 'CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain; CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_plain_schema' +\set UNREGISTER 'SELECT ' :"extension_drop_schema" '.extension_drop__remove(''extension_drop_test'')' SELECT lives_ok( 'CREATE ROLE extension_drop_test_plain' @@ -38,16 +45,15 @@ SELECT lives_ok( ); -- Created as superuser, so the role only owns the schema SELECT lives_ok( - $$CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain; - CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_plain_schema$$ + :'RECREATE' , 'Create schema owned by role containing a registered extension' ); SELECT throws_ok( - $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + :'DROP_SCHEMA' , '42501' , :'MSG' - , 'DROP SCHEMA CASCADE without USAGE on extension_drop''s schema' + , 'Registered: DROP SCHEMA CASCADE without USAGE on extension_drop''s schema' ); -- The error text reaches the caller as DETAIL and HINT; stash them in GUCs to compare @@ -87,10 +93,10 @@ SELECT lives_ok( , 'Grant USAGE on extension_drop''s schema' ); SELECT throws_ok( - $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + :'DROP_SCHEMA' , '42501' , :'MSG' - , 'DROP SCHEMA CASCADE with USAGE but no table privileges' + , 'Registered: DROP SCHEMA CASCADE with USAGE but no table privileges' ); SELECT lives_ok( $q$DO $$ @@ -112,49 +118,16 @@ SELECT is( , 'DETAIL includes the original error' ); -SELECT lives_ok( - format($$SELECT %I.extension_drop__remove('extension_drop_test')$$, :'extension_drop_schema') - , 'Unregister the extension' -); -SELECT throws_ok( - $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ - , '42501' - , :'MSG' - , 'DROP SCHEMA CASCADE of an unregistered extension' -); - -SELECT throws_ok( - $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP OWNED BY extension_drop_test_plain CASCADE; END $$$q$ - , '42501' - , :'MSG' - , 'DROP OWNED BY' -); - -SELECT lives_ok( - 'DROP SCHEMA extension_drop_test_plain_schema CASCADE' - , 'Superuser drop schema' -); -SELECT is_empty( - format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') - , 'No stale registration' -); - -SELECT lives_ok( - $$CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain; - CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_plain_schema$$ - , 'Recreate the registered extension' -); - --- SELECT alone is not enough: the trigger deletes the registration +-- SELECT alone is not enough for a registered extension: the trigger deletes the registration SELECT lives_ok( format('GRANT SELECT ON %I.extension_drop__commands TO extension_drop_test_plain', :'extension_drop_schema') , 'Grant SELECT' ); SELECT throws_ok( - $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + :'DROP_SCHEMA' , '42501' , :'MSG' - , 'DROP SCHEMA CASCADE with SELECT but not DELETE' + , 'Registered: DROP SCHEMA CASCADE with SELECT but not DELETE' ); -- A privilege error from the registered SQL itself is not extension_drop's to translate @@ -165,7 +138,7 @@ SELECT lives_ok( , 'Grant DELETE, register SQL that needs CREATEROLE' ); SELECT throws_ok( - $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; END $$$q$ + :'DROP_SCHEMA' , '42501' , 'permission denied to create role' , 'Registered SQL''s own privilege error surfaces unchanged' @@ -176,8 +149,8 @@ SELECT lives_ok( , 'Register harmless SQL' ); SELECT lives_ok( - $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; RESET ROLE; END $$$q$ - , 'DROP SCHEMA CASCADE with the privileges the HINT names' + :'DROP_SCHEMA' + , 'Registered: DROP SCHEMA CASCADE with the privileges the HINT names' ); SELECT is_empty( format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') @@ -190,8 +163,7 @@ SELECT is_empty( -- Privileges held only through a group role the caller is a member of SELECT lives_ok( - $$CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain; - CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_plain_schema$$ + :'RECREATE' , 'Recreate the registered extension' ); SELECT lives_ok( @@ -207,8 +179,130 @@ SELECT lives_ok( , 'Move the grants from the role to a group it belongs to' ); SELECT lives_ok( - $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; RESET ROLE; END $$$q$ - , 'DROP SCHEMA CASCADE with the privileges held through a group role' + :'DROP_SCHEMA' + , 'Registered: DROP SCHEMA CASCADE with the privileges held through a group role' +); +SELECT lives_ok( + 'REVOKE extension_drop_test_group FROM extension_drop_test_plain' + , 'Remove all of the role''s access' +); + +-- Unregistered extensions need no grants, but still need to be findable: no USAGE means the helper isn't +SELECT lives_ok( + :'RECREATE' + , 'Recreate the extension' +); +SELECT lives_ok( + :'UNREGISTER' + , 'Unregister the extension' +); +-- In existing mode extension_drop is in public, which every role can use, so there is no "no USAGE" case +SELECT has_schema_privilege('extension_drop_test_plain', :'extension_drop_schema', 'USAGE') AS plain_has_usage \gset +\if :plain_has_usage + SELECT skip('existing mode: the role can use extension_drop''s schema', 1); +\else + SELECT throws_ok( + :'DROP_SCHEMA' + , '42501' + , :'MSG' + , 'Unregistered: DROP SCHEMA CASCADE without USAGE on extension_drop''s schema' + ); +\endif + +SELECT lives_ok( + format('GRANT USAGE ON SCHEMA %I TO extension_drop_test_plain', :'extension_drop_schema') + , 'Grant USAGE on extension_drop''s schema' +); +SELECT lives_ok( + :'DROP_SCHEMA' + , 'Unregistered: DROP SCHEMA CASCADE with USAGE but no table privileges' +); + +SELECT lives_ok( + :'RECREATE' + , 'Recreate the extension' +); +SELECT lives_ok( + :'UNREGISTER' + , 'Unregister the extension' +); +SELECT lives_ok( + :'DROP_OWNED' + , 'Unregistered: DROP OWNED BY with USAGE but no table privileges' +); + +SELECT lives_ok( + :'RECREATE' + , 'Recreate the extension' +); +SELECT lives_ok( + :'UNREGISTER' + , 'Unregister the extension' +); +SELECT lives_ok( + format('GRANT SELECT ON %I.extension_drop__commands TO extension_drop_test_plain', :'extension_drop_schema') + , 'Grant SELECT' +); +SELECT lives_ok( + :'DROP_SCHEMA' + , 'Unregistered: DROP SCHEMA CASCADE with SELECT but not DELETE' +); +SELECT lives_ok( + format('REVOKE SELECT ON %I.extension_drop__commands FROM extension_drop_test_plain', :'extension_drop_schema') + , 'Revoke SELECT' +); + +-- A temp table named like ours must not hide the real registration from the helper +SELECT lives_ok( + :'RECREATE' + , 'Recreate the registered extension' +); +SELECT throws_ok( + $q$DO $$ + BEGIN + SET LOCAL ROLE extension_drop_test_plain; + CREATE TEMP TABLE extension_drop__commands(extension_name name, sql text); + DROP SCHEMA extension_drop_test_plain_schema CASCADE; + END $$$q$ + , '42501' + , :'MSG' + , 'Registered extension is still found when the caller has a temp table of the same name' +); +SELECT lives_ok( + :'UNREGISTER' + , 'Unregister the extension' +); +SELECT lives_ok( + 'DROP SCHEMA extension_drop_test_plain_schema CASCADE' + , 'Superuser drop schema' +); + +-- A stale registration must still fail the drop of an unregistered extension +SELECT lives_ok( + :'RECREATE' + , 'Recreate the extension' +); +SELECT lives_ok( + :'UNREGISTER' + , 'Unregister the extension' +); +SELECT lives_ok( + format($$INSERT INTO %I.extension_drop__commands VALUES('extension_drop_test_stale', 'SELECT 1')$$, :'extension_drop_schema') + , 'Insert a stale registration' +); +SELECT throws_ok( + :'DROP_SCHEMA' + , 'XD001' + , 'unexpected drop commands' + , 'Unregistered: DROP SCHEMA CASCADE with a stale registration present' +); +SELECT lives_ok( + format('SELECT %I.extension_drop__repair()', :'extension_drop_schema') + , 'Repair' +); +SELECT lives_ok( + 'DROP SCHEMA extension_drop_test_plain_schema CASCADE' + , 'Superuser drop schema' ); -- ALTER EXTENSION can't change the owner, so create it as a role that is then demoted @@ -232,8 +326,16 @@ SELECT throws_ok( ); SELECT lives_ok( - 'DROP EXTENSION extension_drop_test' - , 'Superuser drop extension' + :'UNREGISTER' + , 'Unregister the extension' +); +SELECT lives_ok( + format('GRANT USAGE ON SCHEMA %I TO extension_drop_test_owner', :'extension_drop_schema') + , 'Grant USAGE on extension_drop''s schema' +); +SELECT lives_ok( + $q$DO $$ BEGIN SET LOCAL ROLE extension_drop_test_owner; DROP EXTENSION extension_drop_test; RESET ROLE; END $$$q$ + , 'Unregistered: DROP EXTENSION by the extension''s non-superuser owner with USAGE but no table privileges' ); SELECT is_empty( format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') From 5c91fa98d5fd30ed8e0172cb590b15659cd24728 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:43:06 -0500 Subject: [PATCH 13/13] Name the table, owner and extensions in the privilege error; test revoked helper EXECUTE The primary message now reads: permission denied for table "s"."extension_drop__commands" (owned by role "r"), which extension_drop needs to drop extension(s) X. DETAIL carries the underlying error. Restate the unregistered-drop requirement as a single rule (USAGE on the extension's schema), and cover the trap's insufficient_privilege branch with a test. --- HISTORY.asc | 24 +++++++------- README.md | 21 +++++------- sql/extension_drop--0.1.1--stable.sql | 8 ++--- sql/extension_drop.sql | 8 ++--- test/expected/drop_denied.out | 46 ++++++++++++++------------ test/expected/drop_denied_1.out | 46 ++++++++++++++------------ test/sql/drop_denied.sql | 47 ++++++++++++++++++++++----- 7 files changed, 120 insertions(+), 80 deletions(-) diff --git a/HISTORY.asc b/HISTORY.asc index b408b84..31138df 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -14,18 +14,18 @@ commands skipped the registered SQL and left a stale registration that made every later `DROP EXTENSION` fail with `XD001`; remove any existing stale registration with `extension_drop__repair()`. -Dropping an unregistered extension (by any of the three commands) needs no -access to `extension_drop__commands`: the trigger asks the new -`SECURITY DEFINER` function `extension_drop__registered()`, executable by -`PUBLIC`. Dropping a registered extension needs USAGE on the extension's -schema and SELECT and DELETE on the table. A role without that access gets -an `insufficient_privilege` error that quotes the underlying error and names -the table's owner and the grants that give that table access, instead of -`permission denied for table extension_drop__commands` or `type -"extension_drop__commands" does not exist`. A caller without USAGE on the -schema gets that error even for an unregistered extension, since it can't -resolve the helper. The registered SQL runs as the caller and may need -further privileges. +Dropping an unregistered extension (by any of the three commands) needs only +USAGE on the extension's schema, to resolve the new `SECURITY DEFINER` +function `extension_drop__registered()` (executable by `PUBLIC`), which the +trigger asks which of the dropped extensions are registered; it needs no +privileges on `extension_drop__commands`. Dropping a registered extension +additionally needs SELECT and DELETE on that table. A role without the access +gets an `insufficient_privilege` error that names the table, its owner and +the extensions being dropped, with the underlying error as DETAIL and a HINT +giving the grants that fix it, instead of `permission denied for table +extension_drop__commands` or `type "extension_drop__commands" does not +exist`. The registered SQL runs as the caller and may need further +privileges. == Remove redundant client_min_messages handling from install script `CREATE EXTENSION` already raises `client_min_messages` to `WARNING` for diff --git a/README.md b/README.md index e58776d..5106949 100644 --- a/README.md +++ b/README.md @@ -56,15 +56,12 @@ and runs the SQL registered for it via trigger for every drop, but it returns immediately unless an extension is among the dropped objects. -Dropping an unregistered extension needs no access to `extension_drop__commands`; -the trigger only asks `extension_drop__registered()` (a `SECURITY DEFINER` -function) which of the dropped extensions are registered. Dropping a -registered extension needs access to the table (USAGE on its schema, SELECT -and DELETE on the table). A role without it gets an `insufficient_privilege` -error that quotes the underlying error and names the table's owner and the -grants that give that table access. The registered SQL runs as the caller and -may need further privileges. - -A caller without USAGE on the extension's schema can't resolve -`extension_drop__registered()`, so it gets that error even for an -unregistered extension. +Dropping an unregistered extension needs only USAGE on the extension's +schema (to resolve `extension_drop__registered()`, a `SECURITY DEFINER` +function executable by `PUBLIC`, which the trigger asks which of the dropped +extensions are registered); it needs no privileges on `extension_drop__commands`. +Dropping a registered extension additionally needs SELECT and DELETE on that +table. A role without the access gets an `insufficient_privilege` error that +names the table, its owner and the extensions being dropped, with the +underlying error as DETAIL and a HINT giving the grants that fix it. The +registered SQL runs as the caller and may need further privileges. diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index 7eaaa23..bd552f0 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -89,7 +89,7 @@ BEGIN GET STACKED DIAGNOSTICS msg = MESSAGE_TEXT; -- Look up the table by OID path, not by name, for the same reason. - SELECT n.nspname, pg_catalog.format('%I.%I', n.nspname, c.relname) AS qname, pg_catalog.pg_get_userbyid(c.relowner) AS owner + SELECT n.nspname, c.relname, pg_catalog.format('%I.%I', n.nspname, c.relname) AS qname, pg_catalog.pg_get_userbyid(c.relowner) AS owner INTO t FROM pg_catalog.pg_extension e JOIN pg_catalog.pg_class c @@ -100,10 +100,10 @@ BEGIN RAISE; END IF; - RAISE EXCEPTION 'permission denied to drop extension(s) %', dropping + RAISE EXCEPTION 'permission denied for table "%"."%" (owned by role "%"), which extension_drop needs to drop extension(s) %' + , t.nspname, t.relname, t.owner, dropping USING ERRCODE = 'insufficient_privilege' - , DETAIL = pg_catalog.format( - 'extension_drop must read and delete its registration in %s: %s', t.qname, msg) + , DETAIL = msg , HINT = pg_catalog.format( 'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s to the current role.' , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname), t.qname); diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 97a17ac..5987f05 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -361,7 +361,7 @@ BEGIN GET STACKED DIAGNOSTICS msg = MESSAGE_TEXT; -- Look up the table by OID path, not by name, for the same reason. - SELECT n.nspname, pg_catalog.format('%I.%I', n.nspname, c.relname) AS qname, pg_catalog.pg_get_userbyid(c.relowner) AS owner + SELECT n.nspname, c.relname, pg_catalog.format('%I.%I', n.nspname, c.relname) AS qname, pg_catalog.pg_get_userbyid(c.relowner) AS owner INTO t FROM pg_catalog.pg_extension e JOIN pg_catalog.pg_class c @@ -372,10 +372,10 @@ BEGIN RAISE; END IF; - RAISE EXCEPTION 'permission denied to drop extension(s) %', dropping + RAISE EXCEPTION 'permission denied for table "%"."%" (owned by role "%"), which extension_drop needs to drop extension(s) %' + , t.nspname, t.relname, t.owner, dropping USING ERRCODE = 'insufficient_privilege' - , DETAIL = pg_catalog.format( - 'extension_drop must read and delete its registration in %s: %s', t.qname, msg) + , DETAIL = msg , HINT = pg_catalog.format( 'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s to the current role.' , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname), t.qname); diff --git a/test/expected/drop_denied.out b/test/expected/drop_denied.out index 8800d78..25f61f4 100644 --- a/test/expected/drop_denied.out +++ b/test/expected/drop_denied.out @@ -1,5 +1,5 @@ \set ECHO none -1..54 +1..60 ok 1 - Create non-superuser role ok 2 - Create schema owned by role containing a registered extension ok 3 - Registered: DROP SCHEMA CASCADE without USAGE on extension_drop's schema @@ -9,7 +9,7 @@ ok 6 - HINT ok 7 - Grant USAGE on extension_drop's schema ok 8 - Registered: DROP SCHEMA CASCADE with USAGE but no table privileges ok 9 - Capture DETAIL -ok 10 - DETAIL includes the original error +ok 10 - DETAIL is the original error ok 11 - Grant SELECT ok 12 - Registered: DROP SCHEMA CASCADE with SELECT but not DELETE ok 13 - Grant DELETE, register SQL that needs CREATEROLE @@ -35,23 +35,29 @@ ok 32 - Unregister the extension ok 33 - Grant SELECT ok 34 - Unregistered: DROP SCHEMA CASCADE with SELECT but not DELETE ok 35 - Revoke SELECT -ok 36 - Recreate the registered extension -ok 37 - Registered extension is still found when the caller has a temp table of the same name -ok 38 - Unregister the extension -ok 39 - Superuser drop schema -ok 40 - Recreate the extension -ok 41 - Unregister the extension -ok 42 - Insert a stale registration -ok 43 - Unregistered: DROP SCHEMA CASCADE with a stale registration present -ok 44 - Repair +ok 36 - Recreate the extension +ok 37 - Unregister the extension +ok 38 - Revoke EXECUTE on the helper +ok 39 - Unregistered: DROP SCHEMA CASCADE without EXECUTE on the helper or table privileges +ok 40 - Restore EXECUTE on the helper +ok 41 - Superuser drop schema +ok 42 - Recreate the registered extension +ok 43 - Registered extension is still found when the caller has a temp table of the same name +ok 44 - Unregister the extension ok 45 - Superuser drop schema -ok 46 - Create role -ok 47 - Create test extension as role -ok 48 - Demote role -ok 49 - DROP EXTENSION by the extension's non-superuser owner -ok 50 - Unregister the extension -ok 51 - Grant USAGE on extension_drop's schema -ok 52 - Unregistered: DROP EXTENSION by the extension's non-superuser owner with USAGE but no table privileges -ok 53 - No stale registration -ok 54 - extension_drop's schema should not be reachable via search_path +ok 46 - Recreate the extension +ok 47 - Unregister the extension +ok 48 - Insert a stale registration +ok 49 - Unregistered: DROP SCHEMA CASCADE with a stale registration present +ok 50 - Repair +ok 51 - Superuser drop schema +ok 52 - Create role +ok 53 - Create test extension as role +ok 54 - Demote role +ok 55 - DROP EXTENSION by the extension's non-superuser owner +ok 56 - Unregister the extension +ok 57 - Grant USAGE on extension_drop's schema +ok 58 - Unregistered: DROP EXTENSION by the extension's non-superuser owner with USAGE but no table privileges +ok 59 - No stale registration +ok 60 - extension_drop's schema should not be reachable via search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_denied_1.out b/test/expected/drop_denied_1.out index a50673a..741f476 100644 --- a/test/expected/drop_denied_1.out +++ b/test/expected/drop_denied_1.out @@ -1,5 +1,5 @@ \set ECHO none -1..54 +1..60 ok 1 - Create non-superuser role ok 2 - Create schema owned by role containing a registered extension ok 3 - Registered: DROP SCHEMA CASCADE without USAGE on extension_drop's schema @@ -9,7 +9,7 @@ ok 6 - HINT ok 7 - Grant USAGE on extension_drop's schema ok 8 - Registered: DROP SCHEMA CASCADE with USAGE but no table privileges ok 9 - Capture DETAIL -ok 10 - DETAIL includes the original error +ok 10 - DETAIL is the original error ok 11 - Grant SELECT ok 12 - Registered: DROP SCHEMA CASCADE with SELECT but not DELETE ok 13 - Grant DELETE, register SQL that needs CREATEROLE @@ -35,23 +35,29 @@ ok 32 - Unregister the extension ok 33 - Grant SELECT ok 34 - Unregistered: DROP SCHEMA CASCADE with SELECT but not DELETE ok 35 - Revoke SELECT -ok 36 - Recreate the registered extension -ok 37 - Registered extension is still found when the caller has a temp table of the same name -ok 38 - Unregister the extension -ok 39 - Superuser drop schema -ok 40 - Recreate the extension -ok 41 - Unregister the extension -ok 42 - Insert a stale registration -ok 43 - Unregistered: DROP SCHEMA CASCADE with a stale registration present -ok 44 - Repair +ok 36 - Recreate the extension +ok 37 - Unregister the extension +ok 38 - Revoke EXECUTE on the helper +ok 39 - Unregistered: DROP SCHEMA CASCADE without EXECUTE on the helper or table privileges +ok 40 - Restore EXECUTE on the helper +ok 41 - Superuser drop schema +ok 42 - Recreate the registered extension +ok 43 - Registered extension is still found when the caller has a temp table of the same name +ok 44 - Unregister the extension ok 45 - Superuser drop schema -ok 46 - Create role -ok 47 - Create test extension as role -ok 48 - Demote role -ok 49 - DROP EXTENSION by the extension's non-superuser owner -ok 50 - Unregister the extension -ok 51 - Grant USAGE on extension_drop's schema -ok 52 - Unregistered: DROP EXTENSION by the extension's non-superuser owner with USAGE but no table privileges -ok 53 - No stale registration -ok 54 # SKIP existing mode: extension_drop's placement is outside this suite's control +ok 46 - Recreate the extension +ok 47 - Unregister the extension +ok 48 - Insert a stale registration +ok 49 - Unregistered: DROP SCHEMA CASCADE with a stale registration present +ok 50 - Repair +ok 51 - Superuser drop schema +ok 52 - Create role +ok 53 - Create test extension as role +ok 54 - Demote role +ok 55 - DROP EXTENSION by the extension's non-superuser owner +ok 56 - Unregister the extension +ok 57 - Grant USAGE on extension_drop's schema +ok 58 - Unregistered: DROP EXTENSION by the extension's non-superuser owner with USAGE but no table privileges +ok 59 - No stale registration +ok 60 # SKIP existing mode: extension_drop's placement is outside this suite's control # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/drop_denied.sql b/test/sql/drop_denied.sql index 0a75ef4..ac3af46 100644 --- a/test/sql/drop_denied.sql +++ b/test/sql/drop_denied.sql @@ -23,6 +23,7 @@ SELECT plan( + 2 -- Unregistered, USAGE only: DROP SCHEMA CASCADE + 3 -- Unregistered, USAGE only: DROP OWNED BY + 5 -- Unregistered, SELECT but not DELETE + + 6 -- Helper not executable: falls through to the caller's DELETE + 4 -- Temp table shadowing extension_drop__commands + 6 -- Stale registration + 4 -- DROP EXTENSION by a role that was a superuser when it created it @@ -33,12 +34,18 @@ SELECT plan( -- Hide version-specific NOTICE/WARNING text from the drops SET client_min_messages = ERROR; -\set MSG 'permission denied to drop extension(s) extension_drop_test' \set DROP_SCHEMA 'DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP SCHEMA extension_drop_test_plain_schema CASCADE; RESET ROLE; END $$' \set DROP_OWNED 'DO $$ BEGIN SET LOCAL ROLE extension_drop_test_plain; DROP OWNED BY extension_drop_test_plain CASCADE; RESET ROLE; END $$' \set RECREATE 'CREATE SCHEMA extension_drop_test_plain_schema AUTHORIZATION extension_drop_test_plain; CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_plain_schema' \set UNREGISTER 'SELECT ' :"extension_drop_schema" '.extension_drop__remove(''extension_drop_test'')' +-- The primary message names the table, its owner and the extensions being dropped +SELECT format( + 'permission denied for table "%s"."extension_drop__commands" (owned by role "%s"), which extension_drop needs to drop extension(s) extension_drop_test' + , :'extension_drop_schema' + , (SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = format('%I.extension_drop__commands', :'extension_drop_schema')::regclass) +) AS "MSG" \gset + SELECT lives_ok( 'CREATE ROLE extension_drop_test_plain' , 'Create non-superuser role' @@ -75,9 +82,7 @@ SELECT lives_ok( */ SELECT matches( current_setting('extension_drop_test.detail') - , format( - '^extension_drop must read and delete its registration in %s\.extension_drop__commands: (relation "extension_drop__commands" does not exist|permission denied for table extension_drop__commands)$' - , regexp_replace(quote_ident(:'extension_drop_schema'), '([^a-zA-Z0-9_])', '\\\1', 'g')) + , '^(relation "extension_drop__commands" does not exist|permission denied for table extension_drop__commands)$' , 'DETAIL' ); SELECT is( @@ -112,10 +117,8 @@ SELECT lives_ok( ); SELECT is( current_setting('extension_drop_test.detail') - , format( - 'extension_drop must read and delete its registration in %I.extension_drop__commands: permission denied for table extension_drop__commands' - , :'extension_drop_schema') - , 'DETAIL includes the original error' + , 'permission denied for table extension_drop__commands' + , 'DETAIL is the original error' ); -- SELECT alone is not enough for a registered extension: the trigger deletes the registration @@ -252,6 +255,34 @@ SELECT lives_ok( , 'Revoke SELECT' ); +-- With EXECUTE on the helper revoked, the trigger falls through to the caller's own DELETE +SELECT lives_ok( + :'RECREATE' + , 'Recreate the extension' +); +SELECT lives_ok( + :'UNREGISTER' + , 'Unregister the extension' +); +SELECT lives_ok( + format('REVOKE EXECUTE ON FUNCTION %I.extension_drop__registered(name[]) FROM PUBLIC', :'extension_drop_schema') + , 'Revoke EXECUTE on the helper' +); +SELECT throws_ok( + :'DROP_SCHEMA' + , '42501' + , :'MSG' + , 'Unregistered: DROP SCHEMA CASCADE without EXECUTE on the helper or table privileges' +); +SELECT lives_ok( + format('GRANT EXECUTE ON FUNCTION %I.extension_drop__registered(name[]) TO PUBLIC', :'extension_drop_schema') + , 'Restore EXECUTE on the helper' +); +SELECT lives_ok( + 'DROP SCHEMA extension_drop_test_plain_schema CASCADE' + , 'Superuser drop schema' +); + -- A temp table named like ours must not hide the real registration from the helper SELECT lives_ok( :'RECREATE'