do not merge until upgrade & restore are settled feat: enable customer installation of amcheck (PSQL-1327) - #2240
do not merge until upgrade & restore are settled feat: enable customer installation of amcheck (PSQL-1327)#2240brainrake wants to merge 11 commits into
Conversation
PostgreSQL Extension Dependency Analysis: PR #2240
SummaryNo extensions had dependencies with MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Extension DependenciesPostgreSQL 17 Extension DependenciesOrioleDB 17 Extension Dependencies |
PostgreSQL Package Dependency Analysis: PR #2240
SummaryNo packages had MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Dependency ChangesExtracting PostgreSQL 15 dependencies...
Runtime Closure Size
Raw Dependency ClosurePostgreSQL 17 Dependency ChangesExtracting PostgreSQL 17 dependencies...
Runtime Closure Size
Raw Dependency Closure |
|
#2240 (comment) |
|
Backup Testing section in current PR description has an item that does not sound reasonable to me.
Assuming old and new correspond to Postgres 15 and 17, this is not an easy task. I do not yet know what options do our users have for restoring but the most probable methods do not work. Postgres 15 was shipped with amcheck Official support for downgrading to an older major version is explicitly not supported by PostgreSQL via standard restore tools. One can always try to manually fix issues in the dump and attempt to restore. It is highly likely that many iterations of edits and retries are required until you can actually downgrade your database version. |
|
I just used the issue template. I only wrote the first two paragraphs. |
|
@hanefi can you review wether amcheck is safe to enable for users? |
|
I am taking over the review and testing of this PR |
fbb6fd8 to
ac4b183
Compare
|
Will document next step of supadev smoke testing prior requesting review |
44432cb to
77b6c1f
Compare
aa7d360 to
b4d6400
Compare
This comment has been minimized.
This comment has been minimized.
b4d6400 to
e7101d2
Compare
This comment has been minimized.
This comment has been minimized.
|
latest changes as of this writing in this PR confirmed in staging |
e7101d2 to
813ae9d
Compare
amcheck was in supautils' "may be unsafe" list based on upstream PostgreSQL's general caution around physical data structure exposure in error messages. No Supabase-specific exploit was ever documented. The restriction created a support gap: customers hit corrupted indexes after 15→17 upgrades and could not use bt_index_check() to enumerate and fix individual indexes, forcing full REINDEX DATABASE instead. Move amcheck from the superuser-only list to privileged_extensions so customers can install it. Update nix/tests/prime.sql and prime-superuser.sql to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move amcheck from supautils' superuser-only list into privileged_extensions, pinned to the `extensions` schema. The pin is required. In `public`, ALTER DEFAULT PRIVILEGES grants EXECUTE to anon/authenticated/service_role, and amcheck does no permission check of its own — so unauthenticated PostgREST callers could run bt_index_parent_check() and verify_heapam() against any table. In `extensions` the equivalent rule grants to postgres only
Co-authored-by: Bobbie Soedirgo <31685197+soedirgo@users.noreply.github.com>
a90f098 to
d53c8a1
Compare
|
his issue with be handed off to managed postgres team to make decisions about how to handle upgrades, pause/restore etc cc @KevinBrolly |
|
Taking over drive to merge this. Reviewing and testing upgrade. |
|
Pause/restore risk is low: Supabase doesn't support restoring a project onto a different major Postgres version, so the amcheck 1.3↔1.4 version-pin issue (dump/restore across 15↔17) doesn't apply there — pause/resume is same-version stop/start only. The pg_upgrade path is the real risk; tracked separately. |
|
Upgrade risk: PG15 ships amcheck 1.3, PG17 ships 1.4. pg_upgrade runs Fix: add |
Restores the KNOWN LIMITATION comment in after-create.sql and the amcheck-upgrade.nix VM test removed earlier in this branch's history. Adds a hard assertion (previously just a printed note) that postgres can call amcheck 1.4's PG17-only bt_index_check overload after a real 15 -> 17 pg_upgrade — this fails today, since the harness's bare pg_upgrade never runs the drop-before/recreate-after fix that lives in production's initiate.sh. Expected red until that gap is closed, either via a supautils after-update hook or by making this harness replay the real admin-API scripts instead of a bare pg_upgrade.
|
Restored |
|
Found 1 test failure on Blacksmith runners: Failure
|
Closes PSQL-1327
What kind of change does this PR introduce?
Moves the amcheck contrib extension into
supautils.privileged_extensionssocustomers can install it, pinned to the
pg_catalogschema.Additional context
amcheck was on supautils' "may be unsafe" list based on upstream
caution around physical data
structure exposure. Enabling it turned out to require the schema pin:
supabase_admin, whose search_pathstarts with
public.ALTER DEFAULT PRIVILEGES ... IN SCHEMA public(initial-schema.sql) grantsEXECUTE to
postgres, anon, authenticated, service_role. Those are explicitgrants, so amcheck's
REVOKE ALL FROM PUBLICdoes not strip them.verify_nbtree.c:"Intentionally not checking permissions" so EXECUTE means "check any index in
the database".
Verified before the pin:
set role anon; select bt_index_check('auth.users_pkey')succeeded. That would have exposed
bt_index_parent_check()(ShareLock, blockswrites) and
verify_heapam()to unauthenticated PostgREST callers. Inpg_catalogthe equivalent rule grants to
postgresonly with grant option, so customers canstill share it deliberately.
Tests
nix/tests/sql/amcheck.sql— placement, override enforcement (with schema publicis redirected), the four-role privilege matrix, and the functional path.
nix/ext/tests/amcheck-upgrade.nix15→17pg_upgradepreserves schema andgrants, including through the 1.3→1.4 extension update that creates new functions.
Action Items
supadev trigger-smoke-tests <PR> engines extensionsupgradetarget covers pause-on-old/restore-on-new;single-file backup/restore still manual