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) in cli/bin/postgres-ai.ts. When !442 (closed) merges, GitLab retargets this MR to main. No code change: the only conflict was the ../lib/init import line.
  • After the rebase: init.integration.test.ts 9/9 on PostgreSQL 15 (Debian, oven/bun:1 container); init.test.ts and clickhouse-provider.test.ts 141/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 run prepare-db over. 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-db exited 0, the function was gone (to_regprocedure null), and no warning was printed;
    • warning path, in a separate database: a CREATEROLE non-superuser admin (pg_monitor/pg_read_all_stats with admin option) ran prepare-db against 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; --verify exited 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.helpers now drops the helper inside a do block that catches insufficient_privilege and raises a WARNING. A CREATEROLE non-superuser that owns neither postgres_ai nor the helper therefore no longer fails at 06.helpers. After apply, and in --verify, the CLI checks to_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 reports legacy SECURITY DEFINER helper postgres_ai.explain_generic(text, text, text) is still installed; its owner or a superuser must drop it on stderr, in --json warnings, 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 on main locally (mon local-install on 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 passes init.test.ts and init.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-sql output for the default and Supabase providers must include the drop.
    • Local-PG integration: create a harmless stub with the legacy signature, run prepare-db twice, and assert the function is gone and --verify passes.
  • Green commit: the one-line SQL change.
Edited by Nikolay Samokhvalov

Merge request reports

Loading
Loading