Uh oh!
There was an error while loading. Please reload this page.
Fix prefixed migrations + tooling for low-privilege multi-schema installs - #629
Closed
mdon wants to merge 5 commits into
Closed
Fix prefixed migrations + tooling for low-privilege multi-schema installs#629mdon wants to merge 5 commits into
mdon wants to merge 5 commits into
Conversation
… docs - Database section gains the prefix-safe migration rules (bare index names on CREATE, schema-anchored existence checks, late flush() failures) with the new integration test as the oracle - New Permissions section covering the allowlist model and the 1.7.182 sub-permissions (dotted keys, sub-implies-base, edit protection, advisory-lock serialization) - pointer-style, moduledoc is the source of truth - Tests section no longer claims test/support/postgres/migrations/ exists (retired 2026-05-05 for ensure_current/2) - Component-coverage TODO drops checkbox from the remaining gaps (checkbox_test.exs exists)
…d daisyUI
daisyUI 5.0.x reserves the modal scrollbar gutter unconditionally while a
modal/drawer is open. Core countered it with an unlayered
:root:has(.modal-open, ...) { scrollbar-gutter: auto } rule (admin
LayoutWrapper + layouts/root.html.heex) plus an inline PkDialog override —
killing the phantom right-edge strip on non-scrolling pages, but defeating
daisyUI's anti-jump reservation on pages that DO scroll: classic-scrollbar
users saw content reflow ~15px and the scrollbar pop in/out around every
modal open/close (user-reported as 'clicked cancel and a scroll bar showed
up' on the dashboards create modal).
daisyUI >= 5.1 reserves the gutter only when the page really has a scrollbar
(rootscrollgutter.css), which handles both page types correctly — so the
compensations are removed rather than made smarter:
- both CSS counter-rules deleted
- PkDialog's inline scrollbar-gutter override and its open-refcount
machinery (window._PkDialogOpenCount) deleted from phoenix_kit.js
The vendored daisyUI stays HOST-OWNED (assets/vendor/daisyui.js, scaffolded
by phx.new) — core does not manage or replace it. Instead the new
PhoenixKit.Install.DaisyUI declares the designed-for minimum (5.6.0; verified
against 5.6.17) and warns when a host is behind:
- mix phoenix_kit.install: Igniter warning with upgrade instructions
- mix phoenix_kit.update: shell warning, next to its CSS/JS refresh steps
- mix phoenix_kit.doctor: new 'daisyUI Version' check
(pass / outdated / unversioned / missing states)
Hosts still on daisyUI < 5.1 get daisyUI's stock old modal behavior (the
gutter strip while a modal is open on non-scrolling pages) plus the warning;
the cure is updating the two vendored files, not core CSS. Do not re-add
scrollbar-gutter overrides in layouts, PkDialog, or modules.
Verified in phoenix_kit_parent (vendored daisyUI updated to 5.6.17) with
native classic scrollbars: on the scrolling dashboards list, body width and
container position stay pixel-identical through the whole open -> cancel
cycle; on the viewport-locked builder, no gutter is reserved and the dialog
covers the full viewport width. Unit tests cover the version parsing and
comparison.…ns, guarded DDL Field report (2026-07-12): a hardened multi-schema install — schema pre-created by a DBA and owned by the app role, no CREATE on the database, PG15+ non-writable public — failed at three points the superuser-driven test suite never exercises: - CREATE EXTENSION IF NOT EXISTS and CREATE SCHEMA IF NOT EXISTS check the CREATE privilege BEFORE the IF-NOT-EXISTS short-circuit, so both fail for low-privilege roles even when the object already exists. V01/V26/V40/V111/UUIDRepair now check pg_extension / information_schema.schemata first and only attempt creation when the object is genuinely missing (raising an operator-facing message listing citext/pgcrypto/pg_trgm when missing and uncreatable). - V27 dropped the caller's create_schema flag at the Oban handoff: Oban.Migration.up defaults it to true for any non-public prefix and executed the failing CREATE SCHEMA mid-chain. By V27 the schema always exists (V01 owns creation), so it now passes create_schema: false. - uuid_generate_v7() was created unqualified — it landed wherever search_path pointed, polluting public on prefixed installs and failing outright where public isn't writable. The function is now created inside the install's schema and all 89 call sites (DEFAULTs, backfill UPDATEs, fragments) are schema-qualified. Postgres.up/1 re-ensures the function at the prefix for upgrade chains starting >= V40, which skip the V40/V56/V61/V63 creation sites. New PhoenixKit.Migrations.Postgres.Helpers centralizes these patterns (qualify_table/2, uuid_v7_call/1, ensure_extension!/1-2, ensure_uuid_v7_function/1-2) plus validate_prefix!/1 — the prefix is interpolated into SQL mostly unquoted, so up/down now reject anything outside [a-z_][a-z0-9_]*. The prefix integration test additionally asserts the function lives in the prefixed schema and that uuid DEFAULTs are pinned to it. Verified end-to-end against the report's repro recipe: full v01->v142 chain as a role with no superuser and no database CREATE into a pre-created schema — zero objects created in public.
Same field report, tooling half — a prefixed install was invisible to
every mix task because nothing persisted or resolved the prefix:
- mix phoenix_kit.install --prefix X now writes
config :phoenix_kit, prefix: "X" into config.exs/test.exs (as the
migration docs already instructed hosts to do by hand), via the new
PhoenixKit.Install.PrefixConfig.
- update/status/gen.migration resolve the prefix as
--prefix option -> config :phoenix_kit, :prefix -> "public"
(PrefixConfig.resolve_prefix/1), so version detection stops silently
looking at public and reporting a prefixed install as missing.
- Install.Common no longer fabricates {:current_version, 1} from the
mere existence of migration FILES when the DB has no marker at the
prefix — that fallback made mix phoenix_kit.update generate a
from-scratch v01->vN migration into the wrong schema. The updater's
not-installed notice now names the resolved prefix and hints at
--prefix / the config key.
- Update-migration generators (update task + gen.migration) always emit
create_schema: false — updating implies the schema exists, and the
flag previously re-defaulted to true for prefixed installs.
- MigrationStrategy no longer hardcodes "public" when checking an
existing install's version, and migration_opts/2 emits create_schema
explicitly for non-public prefixes (omitting it silently discarded
--create-schema=false through the chain's put_new default).
- Repo auto-detection refuses to pick among multiple detected Ecto
repos (it silently wired a migration-only repo in the field report);
it now lists them and asks for --repo.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up to #628, driven by a field report from a hardened multi-schema install (schema pre-created by a DBA and owned by the app role, no CREATE on the database, PG 15+ non-writable
public, PgBouncer). The report confirmed #628's index/existence-check fixes and surfaced five more bug families — all reproduced against source and fixed here, plus three adjacent bugs found while verifying.Migration chain
CREATE EXTENSION/CREATE SCHEMAfail on privilege before the IF-NOT-EXISTS short-circuit. All five extension sites (V01 citext, V26/V40 pgcrypto, V111 pg_trgm, UUIDRepair) and V01's schema creation now checkpg_extension/information_schema.schematafirst and only attempt creation when the object is genuinely missing. A missing extension that the role cannot create raises an operator-facing message listing the three required extensions (also documented inPhoenixKit.Migration's moduledoc for DBAs to pre-provision).create_schemaflag at the Oban handoff —Oban.Migration.up(prefix: prefix)re-defaults it to true for any non-public prefix and executed the failingCREATE SCHEMAmid-chain (the report's "dies around v27–v31"; queued SQL surfaces at a later flush). V27 now passescreate_schema: false— by V27 the schema always exists, V01 owns creation.uuid_generate_v7()was created unqualified — it landed whereversearch_pathpointed, pollutingpublicon prefixed installs and failing outright wherepublicisn't writable. The function is now created inside the install's schema (all 4 creation sites + UUIDRepair) and all ~89 call sites across 35 files (DEFAULTs, backfillUPDATEs,fragment/1) are schema-qualified. For upgrade chains starting ≥ V40 (which skip the creation sites),Postgres.up/1re-ensures the function at the prefix before running steps. Existing installs are unaffected: existing columns keep their OID-pinned defaults; the ensure step only creates the function where missing (neverREPLACEs a function owned by someone else).up/downentry points ([a-z_][a-z0-9_]*) — it's interpolated into hundreds of statements, mostly unquoted, so anything needing quoting only ever half-worked.PhoenixKit.Migrations.Postgres.Helperscentralizes these patterns (qualify_table/2,uuid_v7_call/1,ensure_extension!/1-2,ensure_uuid_v7_function/1-2,validate_prefix!/1) — the per-version hand-rolled helpers were exactly why Fix prefixed migrations: schema-qualified index names + cross-schema existence checks #628's index bug existed in four independent places. Migrating old versions to it is opportunistic; new code should use it.Tooling
mix phoenix_kit.install --prefix Xnow persistsconfig :phoenix_kit, prefix: "X"(newPhoenixKit.Install.PrefixConfig) — the migration docs always told hosts to add it, but nothing did, soupdate/statuslooked atpublicand reported a prefixed install as "Not installed".update/status/gen.migrationresolve the prefix as--prefix→config :phoenix_kit, :prefix→"public".Install.Commonno longer fabricates{:current_version, 1}from the mere existence of migration files when the DB has no marker at the prefix — that fallback mademix phoenix_kit.updategenerate a from-scratch v01→v142 migration into the wrong schema (the report caught it by eye before running it). The not-installed notice now names the resolved prefix and hints at--prefix/the config key.create_schema: false— updating implies the schema exists. Also fixed:MigrationStrategyhardcoded"public"when version-checking an existing install, andmigration_opts/2silently dropped--create-schema=false(omitting the key let the chain re-default it to true).CoreRepothat got wired intoconfig :phoenix_kit, repo:); it now lists them and requires--repo.Verification
test/integration/prefix_migration_test.exsextended: assertsuuid_generate_v7lives in the prefixed schema and uuidDEFAULTs are pinned to it. Full chain into a scratch schema passes.142, function + Oban tables in the prefix, zero objects created inpublic.Helpers(prefix validation, qualification) andPrefixConfig.resolve_prefix/1.mix precommitclean; full suite failures identical to the pre-existing multi-session/sitemap/settings set (verified by stash-rerun on the same base).Notes for the reviewer
uuid_fk_columns.exdrop_uuid_fk_index/4and V56drop_uuid_unique_indexes(DROP INDEX schema.nameis valid) — deliberately untouched.config :phoenix_kit, :prefix— Ecto queries run unprefixed, so prefixed installs still need the DB role'ssearch_pathto include the schema (the report's setup relied on this). Documented in AGENTS.md.