feat: enable customer installation of amcheck (PSQL-1327) - #2240
Open
brainrake wants to merge 10 commits into
Open
feat: enable customer installation of amcheck (PSQL-1327)#2240brainrake wants to merge 10 commits into
brainrake wants to merge 10 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 |
Collaborator
Author
|
#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. |
Collaborator
Author
|
I just used the issue template. I only wrote the first two paragraphs. |
Collaborator
Author
|
@hanefi can you review wether amcheck is safe to enable for users? |
Collaborator
|
I am taking over the review and testing of this PR |
samrose
force-pushed
the
psql-1327-enable-amcheck
branch
2 times, most recently
from
August 18, 2026 21:05
fbb6fd8 to
ac4b183
Compare
samrose
marked this pull request as ready for review
August 18, 2026 21:06
Collaborator
|
Will document next step of supadev smoke testing prior requesting review |
samrose
force-pushed
the
psql-1327-enable-amcheck
branch
2 times, most recently
from
August 21, 2026 15:04
44432cb to
77b6c1f
Compare
soedirgo
reviewed
Aug 21, 2026
samrose
force-pushed
the
psql-1327-enable-amcheck
branch
3 times, most recently
from
August 26, 2026 15:12
aa7d360 to
b4d6400
Compare
This comment has been minimized.
This comment has been minimized.
samrose
force-pushed
the
psql-1327-enable-amcheck
branch
from
August 27, 2026 16:31
b4d6400 to
e7101d2
Compare
This comment has been minimized.
This comment has been minimized.
Collaborator
|
latest changes as of this writing in this PR confirmed in staging |
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
samrose
force-pushed
the
psql-1327-enable-amcheck
branch
from
August 27, 2026 20:35
e7101d2 to
813ae9d
Compare
soedirgo
reviewed
Aug 28, 2026
Comment on lines
-14
to
+16
| postgresorioledb-17: "17.9.0.019-orioledb" | ||
| postgres17: "17.6.1.166" | ||
| postgres15: "15.14.1.166" | ||
| postgresorioledb-17: "17.9.0.018-orioledb-amcheck-2" | ||
| postgres17: "17.6.1.165-amcheck-2" | ||
| postgres15: "15.14.1.165-amcheck-2" |
soedirgo
reviewed
Aug 28, 2026
| 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); |
Member
There was a problem hiding this comment.
This is not causing issues for now, but use routine to make it work for both functions and procedures:
Suggested change
| execute format('grant execute on function %s to postgres', r.sig); | |
| execute format('grant execute on routine %s to postgres', r.sig); |
soedirgo
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
extensionsschema.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. Inextensionsthe 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