diff --git a/ansible/files/postgresql_config/supautils.conf.j2 b/ansible/files/postgresql_config/supautils.conf.j2 index 16ce476d6..63c10761b 100644 --- a/ansible/files/postgresql_config/supautils.conf.j2 +++ b/ansible/files/postgresql_config/supautils.conf.j2 @@ -1,13 +1,13 @@ -supautils.extensions_parameter_overrides = '{"pg_cron":{"schema":"pg_catalog"}}' +supautils.extensions_parameter_overrides = '{"pg_cron":{"schema":"pg_catalog"},"amcheck":{"schema":"pg_catalog"}}' supautils.policy_grants = '{"postgres":["auth.audit_log_entries","auth.flow_state","auth.identities","auth.instances","auth.mfa_amr_claims","auth.mfa_challenges","auth.mfa_factors","auth.oauth_clients","auth.one_time_tokens","auth.refresh_tokens","auth.saml_providers","auth.saml_relay_states","auth.sessions","auth.sso_domains","auth.sso_providers","auth.users","realtime.messages","realtime.subscription","storage.buckets","storage.buckets_analytics","storage.objects","storage.prefixes","storage.s3_multipart_uploads","storage.s3_multipart_uploads_parts"]}' supautils.drop_trigger_grants = '{"postgres":["auth.audit_log_entries","auth.flow_state","auth.identities","auth.instances","auth.mfa_amr_claims","auth.mfa_challenges","auth.mfa_factors","auth.oauth_clients","auth.one_time_tokens","auth.refresh_tokens","auth.saml_providers","auth.saml_relay_states","auth.sessions","auth.sso_domains","auth.sso_providers","auth.users","realtime.messages","realtime.subscription","storage.buckets","storage.buckets_analytics","storage.objects","storage.prefixes","storage.s3_multipart_uploads","storage.s3_multipart_uploads_parts"]}' # full list: address_standardizer, address_standardizer_data_us, adminpack, amcheck, autoinc, bloom, btree_gin, btree_gist, citext, cube, dblink, dict_int, dict_xsyn, earthdistance, file_fdw, fuzzystrmatch, hstore, http, hypopg, index_advisor, insert_username, intagg, intarray, isn, lo, ltree, moddatetime, old_snapshot, orioledb, pageinspect, pg_buffercache, pg_cron, pg_freespacemap, pg_graphql, pg_hashids, pg_jsonschema, pg_net, pg_prewarm, pg_repack, pg_stat_monitor, pg_stat_statements, pg_surgery, pg_tle, pg_trgm, pg_visibility, pg_walinspect, pgaudit, pgcrypto, pgjwt, pgmq, pgroonga, pgroonga_database, pgrouting, pgrowlocks, pgsodium, pgstattuple, pgtap, plcoffee, pljava, plls, plpgsql, plpgsql_check, plv8, postgis, postgis_raster, postgis_sfcgal, postgis_tiger_geocoder, postgis_topology, postgres_fdw, refint, rum, seg, sslinfo, supabase_vault, supautils, tablefunc, tcn, timescaledb, tsm_system_rows, tsm_system_time, unaccent, uuid-ossp, vector, wrappers, xml2 -# omitted because may be unsafe: adminpack, amcheck, file_fdw, lo, old_snapshot, pageinspect, pg_freespacemap, pg_surgery, pg_visibility +# omitted because may be unsafe: adminpack, file_fdw, lo, old_snapshot, pageinspect, pg_freespacemap, pg_surgery, pg_visibility # omitted because deprecated: intagg, xml2 # omitted because doesn't require superuser: pgmq # omitted because protected: plpgsql # NOTE: keep nix/tests/prime-superuser.sql in sync with the "may be unsafe" + "deprecated" lists above. -supautils.privileged_extensions = 'address_standardizer, address_standardizer_data_us, autoinc, bloom, btree_gin, btree_gist, citext, cube, dblink, dict_int, dict_xsyn, earthdistance, fuzzystrmatch, hstore, http, hypopg, index_advisor, insert_username, intarray, isn, ltree, moddatetime, orioledb, pg_buffercache, pg_cron, pg_graphql, pg_hashids, pg_jsonschema, pg_net, pg_prewarm, pg_repack, pg_stat_monitor, pg_stat_statements, pg_tle, pg_trgm, pg_walinspect, pgaudit, pgcrypto, pgjwt, pgroonga, pgroonga_database, pgrouting, pgrowlocks, pgsodium, pgstattuple, pgtap, plcoffee, pljava, plls, plpgsql_check, plv8, postgis, postgis_raster, postgis_sfcgal, postgis_tiger_geocoder, postgis_topology, postgres_fdw, refint, rum, seg, sslinfo, supabase_vault, supautils, tablefunc, tcn, timescaledb, tsm_system_rows, tsm_system_time, unaccent, uuid-ossp, vector, wrappers' +supautils.privileged_extensions = 'address_standardizer, address_standardizer_data_us, amcheck, autoinc, bloom, btree_gin, btree_gist, citext, cube, dblink, dict_int, dict_xsyn, earthdistance, fuzzystrmatch, hstore, http, hypopg, index_advisor, insert_username, intarray, isn, ltree, moddatetime, orioledb, pg_buffercache, pg_cron, pg_graphql, pg_hashids, pg_jsonschema, pg_net, pg_prewarm, pg_repack, pg_stat_monitor, pg_stat_statements, pg_tle, pg_trgm, pg_walinspect, pgaudit, pgcrypto, pgjwt, pgroonga, pgroonga_database, pgrouting, pgrowlocks, pgsodium, pgstattuple, pgtap, plcoffee, pljava, plls, plpgsql_check, plv8, postgis, postgis_raster, postgis_sfcgal, postgis_tiger_geocoder, postgis_topology, postgres_fdw, refint, rum, seg, sslinfo, supabase_vault, supautils, tablefunc, tcn, timescaledb, tsm_system_rows, tsm_system_time, unaccent, uuid-ossp, vector, wrappers' supautils.extension_custom_scripts_path = '/etc/postgresql-custom/extension-custom-scripts' supautils.restrict_extension_versions = 'warn' supautils.privileged_extensions_superuser = 'supabase_admin' diff --git a/ansible/files/postgresql_extension_custom_scripts/amcheck/after-create.sql b/ansible/files/postgresql_extension_custom_scripts/amcheck/after-create.sql new file mode 100644 index 000000000..9190471e9 --- /dev/null +++ b/ansible/files/postgresql_extension_custom_scripts/amcheck/after-create.sql @@ -0,0 +1,19 @@ +do $$ +declare + saved_search_path text := (select current_setting('search_path')); + r record; +begin + perform set_config('search_path', '', true); + + for r in + select p.oid::regprocedure as sig + from pg_depend d + join pg_proc p on p.oid = d.objid and d.classid = 'pg_proc'::regclass + join pg_extension e on e.oid = d.refobjid + where e.extname = 'amcheck' and d.deptype = 'e' + loop + execute format('grant execute on function %s to postgres', r.sig); + end loop; + + perform set_config('search_path', saved_search_path, true); +end $$; diff --git a/ansible/vars.yml b/ansible/vars.yml index 930129c95..77b3d8cf0 100644 --- a/ansible/vars.yml +++ b/ansible/vars.yml @@ -11,9 +11,9 @@ postgres_major: # This is the source of truth for Postgres versions used in the Dockerfiles, and # is used to derive image tags and base images in the release matrix. postgres_release: - postgresorioledb-17: "17.9.0.018-orioledb" - postgres17: "17.6.1.165" - postgres15: "15.14.1.165" + postgresorioledb-17: "17.9.0.018-orioledb-amcheck-1" + postgres17: "17.6.1.165-amcheck-1" + postgres15: "15.14.1.165-amcheck-1" # Docker release matrix — base images built first, layered images built on top. # tag and base_tag are derived at build time from postgres_release via release_key. # tag_suffix is appended to the release version to form the final image tag. diff --git a/nix/checks.nix b/nix/checks.nix index ab7a98825..56d55003e 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -255,6 +255,7 @@ "pg_net_worker_privileges" # needs the authenticated/postgres roles from the full migrations, not present in the CLI prime file "pg_cron_trigger_privileges" # needs pg_cron + the postgres role and cron-schema grants from the full migrations, not in the CLI prime file "supautils_restrict_versions" # needs the postgres role + primed hstore from the full migrations/prime, not present in the CLI variant + "amcheck" # needs the postgres/anon/authenticated/service_role roles and the default privileges from the full migrations, plus amcheck primed by prime.sql # Version-specific extension tests "z_17_ext_interface" "z_17_pg_stat_monitor" diff --git a/nix/ext/tests/amcheck-upgrade.nix b/nix/ext/tests/amcheck-upgrade.nix new file mode 100644 index 000000000..f91da378f --- /dev/null +++ b/nix/ext/tests/amcheck-upgrade.nix @@ -0,0 +1,191 @@ +# amcheck across a 15 to 17 pg_upgrade (PSQL-1327). +# +# amcheck is pinned to the pg_catalog schema by +# supautils.extensions_parameter_overrides, which is what keeps EXECUTE away from +# anon, authenticated and service_role. nix/tests/sql/amcheck.sql asserts that on +# a fresh install. +# +# This test covers what pg_regress cannot reach: whether the schema and the +# grants survive a major version upgrade. Two things could break it. Extension +# member function ACLs ride on pg_init_privs through pg_upgrade's dump and +# restore. And 15 ships amcheck 1.3 while 17 ships 1.4, so the upgrade also runs +# an ALTER EXTENSION UPDATE that creates new functions, each picking up whatever +# default privileges apply at that moment. +# +# Upgraded projects are the population the ticket is about (customers hitting +# corrupt indexes after a 15 to 17 upgrade), so a silent re-grant here would undo +# the fix for exactly the people it was written for. +# +# 15 to 17 only. There is no upgrade path into orioledb, since its specialisation +# wipes the data directory, so an orioledb leg would assert nothing here. +{ self, pkgs }: +let + testLib = import ./lib.nix { inherit self pkgs; }; +in +pkgs.testers.runNixOSTest { + name = "amcheck-upgrade"; + nodes.server = + { ... }: + { + imports = [ + (testLib.makeSupabaseTestConfig { + majorVersion = "15"; + }) + ]; + + specialisation.postgresql17.configuration = testLib.makeUpgradeSpecialisation { + fromMajorVersion = "15"; + toMajorVersion = "17"; + }; + }; + testScript = + { nodes, ... }: + let + pg17-configuration = "${nodes.server.system.build.toplevel}/specialisation/postgresql17"; + in + '' + pg17_configuration = "${pg17-configuration}" + + # postgres holds EXECUTE on amcheck (installed into pg_catalog by supautils); + # the three PostgREST roles hold nothing. Aggregated over every function + # the extension owns, so it holds as amcheck grows from 6 on 15 to 8 on 17. + EXPECTED_ACL = "anon,f,f\nauthenticated,f,f\npostgres,t,t\nservice_role,f,f" + + ACL_QUERY = ( + "select r.rolname, " + "bool_and(has_function_privilege(r.rolname, p.oid, 'execute')), " + "bool_or(has_function_privilege(r.rolname, p.oid, 'execute')) " + "from pg_proc p " + "join pg_depend d on d.objid = p.oid and d.deptype = 'e' " + "join pg_extension e on e.oid = d.refobjid and e.extname = 'amcheck' " + "cross join (values ('postgres'), ('anon'), ('authenticated'), ('service_role')) " + "as r(rolname) group by r.rolname order by r.rolname" + ) + + IDENTITY_QUERY = ( + "select extversion, extnamespace::regnamespace " + "from pg_extension where extname = 'amcheck'" + ) + + def sql(query, role=None): + # Connect as supabase_admin (the superuser this harness authenticates + # as) and set the target role via the startup packet, so each statement + # gets its own session instead of one implicit multi-statement -c. + prefix = "" + if role is not None: + prefix = "PGOPTIONS=\"-c role=" + role + "\" " + return server.succeed( + prefix + + "psql -U supabase_admin -d postgres -t -A -F, -v ON_ERROR_STOP=1 -c \"" + + query + + "\"" + ).strip() + + def sql_fails(query, role=None): + prefix = "" + if role is not None: + prefix = "PGOPTIONS=\"-c role=" + role + "\" " + return server.fail( + prefix + + "psql -U supabase_admin -d postgres -t -A -v ON_ERROR_STOP=1 -c \"" + + query + + "\" 2>&1" + ) + + start_all() + + # Wait for full Supabase initialization (postgres + init-scripts + migrations), + # since the roles and default privileges under test come from the migrations. + server.wait_for_unit("supabase-db-init.service") + + with subtest("Preconditions"): + assert sql("select rolsuper from pg_roles where rolname = 'postgres'") == "f", ( + "postgres is a superuser; every grant assertion below would be vacuous" + ) + # Fail loudly if role switching is broken, rather than silently + # running the whole suite as supabase_admin. + whoami = sql("select current_user", role="postgres") + assert whoami == "postgres", f"expected to be acting as postgres, got: {whoami}" + + with subtest("A non-superuser installs amcheck on PG 15, pinned to pg_catalog"): + sql("create extension amcheck", role="postgres") + before = sql(IDENTITY_QUERY) + assert before.endswith(",pg_catalog"), ( + f"expected amcheck in the pg_catalog schema, got: {before}" + ) + print(f"PG 15 amcheck: {before}") + + with subtest("PG 15 grants reach postgres only"): + acl_before = sql(ACL_QUERY) + assert acl_before == EXPECTED_ACL, f"unexpected PG 15 grants:\n{acl_before}" + + with subtest("switch to postgresql 17"): + server.execute(f"{pg17_configuration}/bin/switch-to-configuration test >&2") + server.wait_for_unit("postgresql.service") + + with subtest("pg_upgrade preserves amcheck's version, schema and grants"): + after = sql(IDENTITY_QUERY) + assert after == before, f"amcheck changed across pg_upgrade: {before} -> {after}" + acl_after = sql(ACL_QUERY) + assert acl_after == EXPECTED_ACL, f"grants changed across pg_upgrade:\n{acl_after}" + + with subtest("The 1.3 -> 1.4 extension update preserves the grants"): + # The platform replays pg_upgrade's generated update_extensions.sql + # (admin_api_scripts/pg_upgrade_scripts/complete.sh). This harness runs + # raw pg_upgrade, so replay that file if present, else ALTER explicitly. + # Either route creates the new 1.4 functions, which is what we inspect. + has_script = server.succeed( + "test -f /var/lib/postgresql/update_extensions.sql && echo yes || echo no" + ).strip() + if has_script == "yes": + server.succeed( + "psql -U supabase_admin -d postgres -v ON_ERROR_STOP=1 " + "-f /var/lib/postgresql/update_extensions.sql" + ) + else: + sql("alter extension amcheck update") + + default_version = sql( + "select default_version from pg_available_extensions where name = 'amcheck'" + ) + updated = sql(IDENTITY_QUERY) + installed_version = updated.split(",")[0] + assert installed_version == default_version, ( + f"expected amcheck at PG 17's default {default_version}, got {installed_version}" + ) + # If 15 and 17 ever ship the same amcheck version there are no new + # functions to acquire grants, and this subtest silently tests nothing. + assert installed_version != before.split(",")[0], ( + f"amcheck was already at {installed_version} on PG 15; this subtest no " + "longer exercises a version bump and needs rewriting" + ) + assert updated.endswith(",pg_catalog"), ( + f"amcheck left the pg_catalog schema during the update: {updated}" + ) + + acl_updated = sql(ACL_QUERY) + assert acl_updated == EXPECTED_ACL, ( + f"grants changed when amcheck was updated to {installed_version}:\n{acl_updated}" + ) + + with subtest("The support workflow still works after the upgrade"): + sql( + "create table amcheck_heap(i int primary key) using heap; " + "insert into amcheck_heap select generate_series(1, 100)", + role="postgres", + ) + sql("select pg_catalog.bt_index_check('amcheck_heap_pkey'::regclass)", role="postgres") + # amcheck has no per relation gate, which is the point: a customer can + # check an auth index they do not own after an upgrade corrupts it. + sql("select pg_catalog.bt_index_check('auth.users_pkey'::regclass)", role="postgres") + + with subtest("The API roles are still locked out after the upgrade"): + for role in ["anon", "authenticated", "service_role"]: + err = sql_fails( + "select pg_catalog.bt_index_check('auth.users_pkey'::regclass)", role=role + ) + assert "permission denied for function bt_index_check" in err, ( + f"expected {role} to be denied, got: {err}" + ) + ''; +} diff --git a/nix/tests/expected/amcheck.out b/nix/tests/expected/amcheck.out new file mode 100644 index 000000000..3f74e0204 --- /dev/null +++ b/nix/tests/expected/amcheck.out @@ -0,0 +1,132 @@ +-- amcheck for customers (PSQL-1327). +-- +-- amcheck is in supautils.privileged_extensions, so the non-superuser postgres +-- role can install it and call bt_index_check() instead of running a full +-- REINDEX DATABASE. +-- +-- supautils.extensions_parameter_overrides pins it to pg_catalog, the same as pg_cron. +-- +-- Why not public: supautils creates privileged extensions as supabase_admin, and +-- ALTER DEFAULT PRIVILEGES (init-scripts/00000000000000-initial-schema.sql) grants +-- EXECUTE on anything supabase_admin creates in public to anon, authenticated and +-- service_role, the PostgREST API roles. amcheck does no permission check of its own +-- (upstream verify_nbtree.c: "Intentionally not checking permissions"), so in public +-- it would be callable over the REST API against any index, including auth's. +-- pg_catalog has no such default-privilege grant, so the API roles never get access. +-- +-- This suite asserts the placement (pg_catalog), that postgres can use it, and that +-- the API roles cannot. +-- the pin lives in the platform config +show supautils.extensions_parameter_overrides; + supautils.extensions_parameter_overrides +----------------------------------------------------------------------- + {"pg_cron":{"schema":"pg_catalog"},"amcheck":{"schema":"pg_catalog"}} +(1 row) + +-- precondition: postgres must not be a superuser, or nothing below is meaningful +select rolsuper from pg_roles where rolname = 'postgres'; + rolsuper +---------- + f +(1 row) + +-- prime.sql already created amcheck; drop it so the creates below are observable +drop extension if exists amcheck; +-- a non-superuser can install it, and it lands in pg_catalog rather than public +set role postgres; +create extension amcheck; +reset role; +select extowner::regrole as owner, extnamespace::regnamespace as schema + from pg_extension + where extname = 'amcheck'; + owner | schema +----------------+------------ + supabase_admin | pg_catalog +(1 row) + +-- the override wins over an explicitly requested schema +drop extension amcheck; +set role postgres; +create extension amcheck with schema public; +reset role; +select extnamespace::regnamespace as schema_after_requesting_public + from pg_extension + where extname = 'amcheck'; + schema_after_requesting_public +-------------------------------- + pg_catalog +(1 row) + +-- The privilege matrix. Aggregated over every function the extension owns rather +-- than named signatures, so one expected file covers PG 15 (6 functions) and +-- PG 17 (8), and functions added by future versions are picked up automatically. +select r.rolname, + bool_and(has_function_privilege(r.rolname, p.oid, 'execute')) as all_functions, + bool_or(has_function_privilege(r.rolname, p.oid, 'execute')) as any_function + from pg_proc p + join pg_depend d on d.objid = p.oid and d.deptype = 'e' + join pg_extension e on e.oid = d.refobjid and e.extname = 'amcheck' + cross join (values ('postgres'), ('anon'), ('authenticated'), ('service_role')) as r(rolname) + group by r.rolname + order by r.rolname; + rolname | all_functions | any_function +---------------+---------------+-------------- + anon | f | f + authenticated | f | f + postgres | t | t + service_role | f | f +(4 rows) + +-- The support case: postgres checks an index it owns, then one it does not. +-- amcheck has no per relation gate, which is what makes it useful when an +-- upgrade corrupts an index in a schema the customer does not control. +-- +-- Both tables are heap tables in a schema created here, rather than existing +-- relations in public or auth. On orioledb, auth.users is orioledb backed and +-- its btree has no conventional storage. On multigres, postgres is not the +-- database owner and has no CREATE on public. +create schema amcheck_test; +grant usage, create on schema amcheck_test to postgres; +create table amcheck_test.notmine(i int primary key) using heap; +insert into amcheck_test.notmine select generate_series(1, 100); +revoke all on amcheck_test.notmine from postgres, anon, authenticated, service_role; +-- postgres neither owns this table nor can write to it. It keeps SELECT via +-- pg_read_all_data (migrations/20240606060239_grant_predefined_roles_to_postgres.sql), +-- which no revoke undoes. Ownership is the property that matters here. +select pg_get_userbyid(relowner) as notmine_owner, + has_table_privilege('postgres', 'amcheck_test.notmine', 'insert') as postgres_can_write + from pg_class + where oid = 'amcheck_test.notmine'::regclass; + notmine_owner | postgres_can_write +----------------+-------------------- + supabase_admin | f +(1 row) + +set role postgres; +create table amcheck_test.mine(i int primary key) using heap; +insert into amcheck_test.mine select generate_series(1, 100); +select pg_catalog.bt_index_check('amcheck_test.mine_pkey'::regclass); + bt_index_check +---------------- + +(1 row) + +select pg_catalog.bt_index_check('amcheck_test.notmine_pkey'::regclass); + bt_index_check +---------------- + +(1 row) + +reset role; +-- the API roles must not reach it. The denial is at the function level, so it +-- applies to every target relation. +set role anon; +select pg_catalog.bt_index_check('auth.users_pkey'::regclass); +ERROR: permission denied for function bt_index_check +reset role; +-- restore the state prime.sql created (amcheck present, in pg_catalog) +set role postgres; +drop table amcheck_test.mine; +reset role; +drop table amcheck_test.notmine; +drop schema amcheck_test; diff --git a/nix/tests/expected/z_15_ext_interface.out b/nix/tests/expected/z_15_ext_interface.out index 622080a4f..e53086bde 100644 --- a/nix/tests/expected/z_15_ext_interface.out +++ b/nix/tests/expected/z_15_ext_interface.out @@ -181,12 +181,12 @@ order by adminpack | pg_catalog | pg_file_unlink | text | boolean adminpack | pg_catalog | pg_file_write | text, text, boolean | bigint adminpack | pg_catalog | pg_logdir_ls | | SETOF record - amcheck | public | bt_index_check | index regclass | void - amcheck | public | bt_index_check | index regclass, heapallindexed boolean | void - amcheck | public | bt_index_parent_check | index regclass | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean | void - amcheck | public | verify_heapam | relation regclass, on_error_stop boolean, check_toast boolean, skip text, startblock bigint, endblock bigint, OUT blkno bigint, OUT offnum integer, OUT attnum integer, OUT msg text | SETOF record + amcheck | pg_catalog | bt_index_check | index regclass | void + amcheck | pg_catalog | bt_index_check | index regclass, heapallindexed boolean | void + amcheck | pg_catalog | bt_index_parent_check | index regclass | void + amcheck | pg_catalog | bt_index_parent_check | index regclass, heapallindexed boolean | void + amcheck | pg_catalog | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean | void + amcheck | pg_catalog | verify_heapam | relation regclass, on_error_stop boolean, check_toast boolean, skip text, startblock bigint, endblock bigint, OUT blkno bigint, OUT offnum integer, OUT attnum integer, OUT msg text | SETOF record autoinc | public | autoinc | | trigger bloom | public | blhandler | internal | index_am_handler btree_gin | public | gin_btree_consistent | internal, smallint, anyelement, integer, internal, internal | boolean diff --git a/nix/tests/expected/z_15_roles.out b/nix/tests/expected/z_15_roles.out index a051ab1d1..c7aaabf36 100644 --- a/nix/tests/expected/z_15_roles.out +++ b/nix/tests/expected/z_15_roles.out @@ -51,6 +51,11 @@ where p.pronamespace::regnamespace::text = 'pg_catalog' order by object_name, grantee, privilege_type; schema | object_name | grantee | privilege_type ------------+--------------------------------+-------------------+---------------- + pg_catalog | bt_index_check | postgres | EXECUTE + pg_catalog | bt_index_check | postgres | EXECUTE + pg_catalog | bt_index_parent_check | postgres | EXECUTE + pg_catalog | bt_index_parent_check | postgres | EXECUTE + pg_catalog | bt_index_parent_check | postgres | EXECUTE pg_catalog | pg_get_backend_memory_contexts | pg_read_all_stats | EXECUTE pg_catalog | pg_get_shmem_allocations | pg_read_all_stats | EXECUTE pg_catalog | pg_ls_archive_statusdir | pg_monitor | EXECUTE @@ -62,5 +67,6 @@ order by object_name, grantee, privilege_type; pg_catalog | pg_ls_tmpdir | pg_monitor | EXECUTE pg_catalog | pg_ls_waldir | pg_monitor | EXECUTE pg_catalog | pg_reload_conf | postgres | EXECUTE -(11 rows) + pg_catalog | verify_heapam | postgres | EXECUTE +(17 rows) diff --git a/nix/tests/expected/z_17_ext_interface.out b/nix/tests/expected/z_17_ext_interface.out index 6625cd8bc..d589b8a6d 100644 --- a/nix/tests/expected/z_17_ext_interface.out +++ b/nix/tests/expected/z_17_ext_interface.out @@ -162,14 +162,14 @@ order by address_standardizer | public | parse_address | text, OUT num text, OUT street text, OUT street2 text, OUT address1 text, OUT city text, OUT state text, OUT zip text, OUT zipplus text, OUT country text | record address_standardizer | public | standardize_address | lextab text, gaztab text, rultab text, address text | stdaddr address_standardizer | public | standardize_address | lextab text, gaztab text, rultab text, micro text, macro text | stdaddr - amcheck | public | bt_index_check | index regclass | void - amcheck | public | bt_index_check | index regclass, heapallindexed boolean, checkunique boolean | void - amcheck | public | bt_index_check | index regclass, heapallindexed boolean | void - amcheck | public | bt_index_parent_check | index regclass | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean, checkunique boolean | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean | void - amcheck | public | verify_heapam | relation regclass, on_error_stop boolean, check_toast boolean, skip text, startblock bigint, endblock bigint, OUT blkno bigint, OUT offnum integer, OUT attnum integer, OUT msg text | SETOF record + amcheck | pg_catalog | bt_index_check | index regclass | void + amcheck | pg_catalog | bt_index_check | index regclass, heapallindexed boolean, checkunique boolean | void + amcheck | pg_catalog | bt_index_check | index regclass, heapallindexed boolean | void + amcheck | pg_catalog | bt_index_parent_check | index regclass | void + amcheck | pg_catalog | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean, checkunique boolean | void + amcheck | pg_catalog | bt_index_parent_check | index regclass, heapallindexed boolean | void + amcheck | pg_catalog | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean | void + amcheck | pg_catalog | verify_heapam | relation regclass, on_error_stop boolean, check_toast boolean, skip text, startblock bigint, endblock bigint, OUT blkno bigint, OUT offnum integer, OUT attnum integer, OUT msg text | SETOF record autoinc | public | autoinc | | trigger bloom | public | blhandler | internal | index_am_handler btree_gin | public | gin_btree_consistent | internal, smallint, anyelement, integer, internal, internal | boolean diff --git a/nix/tests/expected/z_17_roles.out b/nix/tests/expected/z_17_roles.out index e7ee48070..b966f8f9d 100644 --- a/nix/tests/expected/z_17_roles.out +++ b/nix/tests/expected/z_17_roles.out @@ -184,6 +184,13 @@ where p.pronamespace::regnamespace::text = 'pg_catalog' order by object_name, grantee, privilege_type; schema | object_name | grantee | privilege_type ------------+--------------------------------+-------------------+---------------- + pg_catalog | bt_index_check | postgres | EXECUTE + pg_catalog | bt_index_check | postgres | EXECUTE + pg_catalog | bt_index_check | postgres | EXECUTE + pg_catalog | bt_index_parent_check | postgres | EXECUTE + pg_catalog | bt_index_parent_check | postgres | EXECUTE + pg_catalog | bt_index_parent_check | postgres | EXECUTE + pg_catalog | bt_index_parent_check | postgres | EXECUTE pg_catalog | pg_current_logfile | pg_monitor | EXECUTE pg_catalog | pg_current_logfile | pg_monitor | EXECUTE pg_catalog | pg_get_backend_memory_contexts | pg_read_all_stats | EXECUTE @@ -197,5 +204,6 @@ order by object_name, grantee, privilege_type; pg_catalog | pg_ls_tmpdir | pg_monitor | EXECUTE pg_catalog | pg_ls_waldir | pg_monitor | EXECUTE pg_catalog | pg_reload_conf | postgres | EXECUTE -(13 rows) + pg_catalog | verify_heapam | postgres | EXECUTE +(21 rows) diff --git a/nix/tests/expected/z_multigres-17_ext_interface.out b/nix/tests/expected/z_multigres-17_ext_interface.out index c66b18d7a..76a667573 100644 --- a/nix/tests/expected/z_multigres-17_ext_interface.out +++ b/nix/tests/expected/z_multigres-17_ext_interface.out @@ -162,14 +162,14 @@ order by address_standardizer | public | parse_address | text, OUT num text, OUT street text, OUT street2 text, OUT address1 text, OUT city text, OUT state text, OUT zip text, OUT zipplus text, OUT country text | record address_standardizer | public | standardize_address | lextab text, gaztab text, rultab text, address text | stdaddr address_standardizer | public | standardize_address | lextab text, gaztab text, rultab text, micro text, macro text | stdaddr - amcheck | public | bt_index_check | index regclass | void - amcheck | public | bt_index_check | index regclass, heapallindexed boolean, checkunique boolean | void - amcheck | public | bt_index_check | index regclass, heapallindexed boolean | void - amcheck | public | bt_index_parent_check | index regclass | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean, checkunique boolean | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean | void - amcheck | public | verify_heapam | relation regclass, on_error_stop boolean, check_toast boolean, skip text, startblock bigint, endblock bigint, OUT blkno bigint, OUT offnum integer, OUT attnum integer, OUT msg text | SETOF record + amcheck | extensions | bt_index_check | index regclass | void + amcheck | extensions | bt_index_check | index regclass, heapallindexed boolean, checkunique boolean | void + amcheck | extensions | bt_index_check | index regclass, heapallindexed boolean | void + amcheck | extensions | bt_index_parent_check | index regclass | void + amcheck | extensions | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean, checkunique boolean | void + amcheck | extensions | bt_index_parent_check | index regclass, heapallindexed boolean | void + amcheck | extensions | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean | void + amcheck | extensions | verify_heapam | relation regclass, on_error_stop boolean, check_toast boolean, skip text, startblock bigint, endblock bigint, OUT blkno bigint, OUT offnum integer, OUT attnum integer, OUT msg text | SETOF record autoinc | public | autoinc | | trigger bloom | public | blhandler | internal | index_am_handler btree_gin | public | gin_btree_consistent | internal, smallint, anyelement, integer, internal, internal | boolean diff --git a/nix/tests/expected/z_orioledb-17_ext_interface.out b/nix/tests/expected/z_orioledb-17_ext_interface.out index 0718c6ed3..0482d0b86 100644 --- a/nix/tests/expected/z_orioledb-17_ext_interface.out +++ b/nix/tests/expected/z_orioledb-17_ext_interface.out @@ -163,14 +163,14 @@ order by address_standardizer | public | parse_address | text, OUT num text, OUT street text, OUT street2 text, OUT address1 text, OUT city text, OUT state text, OUT zip text, OUT zipplus text, OUT country text | record address_standardizer | public | standardize_address | lextab text, gaztab text, rultab text, address text | stdaddr address_standardizer | public | standardize_address | lextab text, gaztab text, rultab text, micro text, macro text | stdaddr - amcheck | public | bt_index_check | index regclass | void - amcheck | public | bt_index_check | index regclass, heapallindexed boolean, checkunique boolean | void - amcheck | public | bt_index_check | index regclass, heapallindexed boolean | void - amcheck | public | bt_index_parent_check | index regclass | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean, checkunique boolean | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean | void - amcheck | public | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean | void - amcheck | public | verify_heapam | relation regclass, on_error_stop boolean, check_toast boolean, skip text, startblock bigint, endblock bigint, OUT blkno bigint, OUT offnum integer, OUT attnum integer, OUT msg text | SETOF record + amcheck | pg_catalog | bt_index_check | index regclass | void + amcheck | pg_catalog | bt_index_check | index regclass, heapallindexed boolean, checkunique boolean | void + amcheck | pg_catalog | bt_index_check | index regclass, heapallindexed boolean | void + amcheck | pg_catalog | bt_index_parent_check | index regclass | void + amcheck | pg_catalog | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean, checkunique boolean | void + amcheck | pg_catalog | bt_index_parent_check | index regclass, heapallindexed boolean | void + amcheck | pg_catalog | bt_index_parent_check | index regclass, heapallindexed boolean, rootdescend boolean | void + amcheck | pg_catalog | verify_heapam | relation regclass, on_error_stop boolean, check_toast boolean, skip text, startblock bigint, endblock bigint, OUT blkno bigint, OUT offnum integer, OUT attnum integer, OUT msg text | SETOF record autoinc | public | autoinc | | trigger bloom | public | blhandler | internal | index_am_handler btree_gin | public | gin_btree_consistent | internal, smallint, anyelement, integer, internal, internal | boolean diff --git a/nix/tests/prime-superuser.sql b/nix/tests/prime-superuser.sql index 3d3a2c8b7..435fc4fde 100644 --- a/nix/tests/prime-superuser.sql +++ b/nix/tests/prime-superuser.sql @@ -30,7 +30,6 @@ set client_min_messages = warning; -- Category 1: "may be unsafe" per supautils.conf.j2 -create extension if not exists amcheck; create extension if not exists file_fdw; create extension if not exists lo; create extension if not exists pageinspect; diff --git a/nix/tests/prime.sql b/nix/tests/prime.sql index 24fd8f2ea..9260e759a 100644 --- a/nix/tests/prime.sql +++ b/nix/tests/prime.sql @@ -9,12 +9,13 @@ set client_min_messages = warning; -- engines-with-smoke against hosted projects). They live in -- prime-superuser.sql instead — that covers: -- - the "may be unsafe" list from supautils.conf.j2 --- (amcheck, file_fdw, lo, pageinspect, pg_freespacemap, pg_surgery, pg_visibility) +-- (file_fdw, lo, pageinspect, pg_freespacemap, pg_surgery, pg_visibility) -- - the "deprecated" list from supautils.conf.j2 (intagg, xml2) -- Superuser-context harnesses (nix/checks.nix, docker-image-test, -- migrate-tool) source prime-superuser.sql in addition to this file. create extension if not exists address_standardizer; create extension if not exists address_standardizer_data_us; +create extension if not exists amcheck; create extension if not exists autoinc; create extension if not exists bloom; create extension if not exists btree_gin; diff --git a/nix/tests/sql/amcheck.sql b/nix/tests/sql/amcheck.sql new file mode 100644 index 000000000..04ee5aa45 --- /dev/null +++ b/nix/tests/sql/amcheck.sql @@ -0,0 +1,103 @@ +-- amcheck for customers (PSQL-1327). +-- +-- amcheck is in supautils.privileged_extensions, so the non-superuser postgres +-- role can install it and call bt_index_check() instead of running a full +-- REINDEX DATABASE. +-- +-- supautils.extensions_parameter_overrides pins it to pg_catalog, the same as pg_cron. +-- +-- Why not public: supautils creates privileged extensions as supabase_admin, and +-- ALTER DEFAULT PRIVILEGES (init-scripts/00000000000000-initial-schema.sql) grants +-- EXECUTE on anything supabase_admin creates in public to anon, authenticated and +-- service_role, the PostgREST API roles. amcheck does no permission check of its own +-- (upstream verify_nbtree.c: "Intentionally not checking permissions"), so in public +-- it would be callable over the REST API against any index, including auth's. +-- pg_catalog has no such default-privilege grant, so the API roles never get access. +-- +-- This suite asserts the placement (pg_catalog), that postgres can use it, and that +-- the API roles cannot. + +-- the pin lives in the platform config +show supautils.extensions_parameter_overrides; + +-- precondition: postgres must not be a superuser, or nothing below is meaningful +select rolsuper from pg_roles where rolname = 'postgres'; + +-- prime.sql already created amcheck; drop it so the creates below are observable +drop extension if exists amcheck; + +-- a non-superuser can install it, and it lands in pg_catalog rather than public +set role postgres; +create extension amcheck; +reset role; + +select extowner::regrole as owner, extnamespace::regnamespace as schema + from pg_extension + where extname = 'amcheck'; + +-- the override wins over an explicitly requested schema +drop extension amcheck; + +set role postgres; +create extension amcheck with schema public; +reset role; + +select extnamespace::regnamespace as schema_after_requesting_public + from pg_extension + where extname = 'amcheck'; + +-- The privilege matrix. Aggregated over every function the extension owns rather +-- than named signatures, so one expected file covers PG 15 (6 functions) and +-- PG 17 (8), and functions added by future versions are picked up automatically. +select r.rolname, + bool_and(has_function_privilege(r.rolname, p.oid, 'execute')) as all_functions, + bool_or(has_function_privilege(r.rolname, p.oid, 'execute')) as any_function + from pg_proc p + join pg_depend d on d.objid = p.oid and d.deptype = 'e' + join pg_extension e on e.oid = d.refobjid and e.extname = 'amcheck' + cross join (values ('postgres'), ('anon'), ('authenticated'), ('service_role')) as r(rolname) + group by r.rolname + order by r.rolname; + +-- The support case: postgres checks an index it owns, then one it does not. +-- amcheck has no per relation gate, which is what makes it useful when an +-- upgrade corrupts an index in a schema the customer does not control. +-- +-- Both tables are heap tables in a schema created here, rather than existing +-- relations in public or auth. On orioledb, auth.users is orioledb backed and +-- its btree has no conventional storage. On multigres, postgres is not the +-- database owner and has no CREATE on public. +create schema amcheck_test; +grant usage, create on schema amcheck_test to postgres; + +create table amcheck_test.notmine(i int primary key) using heap; +insert into amcheck_test.notmine select generate_series(1, 100); +revoke all on amcheck_test.notmine from postgres, anon, authenticated, service_role; + +-- postgres neither owns this table nor can write to it. It keeps SELECT via +-- pg_read_all_data (migrations/20240606060239_grant_predefined_roles_to_postgres.sql), +-- which no revoke undoes. Ownership is the property that matters here. +select pg_get_userbyid(relowner) as notmine_owner, + has_table_privilege('postgres', 'amcheck_test.notmine', 'insert') as postgres_can_write + from pg_class + where oid = 'amcheck_test.notmine'::regclass; + +set role postgres; +create table amcheck_test.mine(i int primary key) using heap; +insert into amcheck_test.mine select generate_series(1, 100); +select pg_catalog.bt_index_check('amcheck_test.mine_pkey'::regclass); +select pg_catalog.bt_index_check('amcheck_test.notmine_pkey'::regclass); +reset role; + +-- the API roles must not reach it. The denial is at the function level, so it +-- applies to every target relation. +set role anon; +select pg_catalog.bt_index_check('auth.users_pkey'::regclass); +reset role; + +-- restore the state prime.sql created (amcheck present, in pg_catalog) +set role postgres; +drop table amcheck_test.mine; +reset role; +drop table amcheck_test.notmine; +drop schema amcheck_test;