fix(security): prepare-db drops the legacy explain_generic helper
Update 2026-09-30 (round-2 review):
- Rebased on !442 (closed) (the host metrics MR that contains !434 (closed), !436 (closed) and !440 (closed)), and retargeted to
feat/unified-host-metrics: it conflicted with !434 (closed) and !442 (closed) incli/bin/postgres-ai.ts. When !442 (closed) merges, GitLab retargets this MR tomain. No code change: the only conflict was the../lib/initimport line.- After the rebase:
init.integration.test.ts9/9 on PostgreSQL 15 (Debian,oven/bun:1container);init.test.tsandclickhouse-provider.test.ts141/141.- Real provider run (ClickHouse Managed Postgres, r6gd.medium, PostgreSQL 17.11, 2026-09-30). We could not use the Supabase test project: its access token lacks
database_read, and the Management API never returns the database password, so there was no database connection to runprepare-dbover. On ClickHouse, with the CLI from this branch:
- drop path: the service's superuser created the legacy
postgres_ai.explain_generic(text, text, text);prepare-dbexited 0, the function was gone (to_regprocedurenull), and no warning was printed;- warning path, in a separate database: a CREATEROLE non-superuser admin (
pg_monitor/pg_read_all_statswith admin option) ranprepare-dbagainst a helper owned by another role; it exited 0 and printed⚠ legacy SECURITY DEFINER helper postgres_ai.explain_generic(text, text, text) is still installed; its owner or a superuser must drop it;--verifyexited 0 and listed it; the helper was left in place.- The service was deleted; its API URL returns 404.
Update 2026-09-29 (Part of postgres-ai/internal#354):
- Review finding fixed (different owner). Step
06.helpersnow drops the helper inside adoblock that catchesinsufficient_privilegeand raises aWARNING. A CREATEROLE non-superuser that owns neitherpostgres_ainor the helper therefore no longer fails at06.helpers. After apply, and in--verify, the CLI checksto_regprocedure('postgres_ai.explain_generic(text,text,text)')itself. It works on both direct and Supabase connections, because Supabase does not relay server notices. It reportslegacy SECURITY DEFINER helper postgres_ai.explain_generic(text, text, text) is still installed; its owner or a superuser must drop iton stderr, in--jsonwarnings, and under--verify's optional items. The exit code stays 0. Other errors (for example dependent objects) still fail.- New red/green pair. Red: an integration test with a second admin and a legacy owner failed with
Failed at step "06.helpers": must be owner of function postgres_ai.explain_generic. The fix commit makes it pass. The full CLI suite passes except 8 tests that fail the same way onmainlocally (mon local-installon macOS, and one checkup-api timeout).- Merge order with !336: the two MRs used to conflict in
cli/test/init.test.ts. !336 moved its two assertions, so they now merge cleanly in either order. A local merge of both heads passesinit.test.tsandinit.integration.test.ts. Suggested order: !433 first, then !336.
Part of postgres-ai/internal#354. Companion to platform-all !880 (https://gitlab.com/postgres-ai/platform-all/-/merge_requests/880). Follow-up to the removal of postgres_ai.explain_generic, a SECURITY DEFINER EXPLAIN wrapper that allowed RCE as the superuser (removed from cli/sql on 2026-06-24).
Problem
Databases prepared by older CLI versions still carry postgres_ai.explain_generic(text, text, text). The current prepare-db neither creates nor removes it; only unprepare-db drops it. Managed and Supabase monitoring deploys run postgresai prepare-db through postgres_ai-ansible (deploy_monitoring.yml, prepare_monitoring_db=true), so this is the path that reaches those databases.
Change
cli/sql/06.helpers.sql now starts with drop function if exists postgres_ai.explain_generic(text, text, text);. That is the only signature the helper ever had. The next prepare-db run, or monitoring redeploy, removes it. Clean databases are unaffected. --print-sql shows the same statement.
Tests (red/green)
- Red commit, tests only:
- The
--print-sqloutput for the default and Supabase providers must include the drop. - Local-PG integration: create a harmless stub with the legacy signature, run
prepare-dbtwice, and assert the function is gone and--verifypasses.
- The
- Green commit: the one-line SQL change.