Uh oh!
There was an error while loading. Please reload this page.
Prefix hardening for low-privilege multi-schema installs, runtime schema-prefix support + daisyUI modal-gutter fix - #631
Merged
Merged
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.… :prefix Until now only the MIGRATIONS honored the prefix — every runtime Ecto query issued unqualified table names, so a prefixed install only worked when the DB role's search_path happened to include the schema (the field-report install relied on exactly that, undocumented). New PhoenixKit.SchemaPrefix sets @schema_prefix from Application.compile_env(:phoenix_kit, :prefix); all 21 table-backed schemas use it. Because the prefix rides on the schema meta, every pathway inherits it — RepoHelper delegators, direct repo() calls, update_all/insert_all, Ecto.Multi steps, preloads, and joins — with no call-site changes. Unset config compiles to nil = behavior unchanged for public installs. Compile-time by design (an install's prefix is fixed at install time); Mix recompiles the dep when it changes. Oban rides the same prefix: V27 creates oban_jobs inside the named schema, so the installer now writes prefix: into the generated Oban config for prefixed installs and mix phoenix_kit.update warns when an existing Oban config lacks it. A conformance test scans for table-backed schemas missing the use — a schema without it falls back to search_path resolution, invisible on public installs and broken on prefixed ones. Verified end-to-end by recompiling the test build with a prefix configured: register_user (user + token + owner role assignment + activity) landed entirely in a scratch schema with public untouched, and loaded structs carry the prefix in __meta__.
…GENTS.md section dev_docs/investigations/2026-07-12-daisyui-version-management-investigation.md records the full arc: the dashboards create-modal scrollbar report, the frame-by-frame reproduction, the root cause (daisyUI 5.0.x's unconditional modal gutter vs core's accreted compensations), the ecosystem research (hosts own the vendored plugin; every workspace host was on 5.0.35; upstream fixed the gutter conditionally in >= 5.1), the options analysis (npm / pin-and-download / deps-path @plugin / vendor-in-core custody / advisory warnings), the custody implementation that was built, verified, and deliberately NOT adopted (hosts own their assets), what shipped instead (compensations removed + PhoenixKit.Install.DaisyUI advisory warnings), and the measurement gotchas (clientWidth vs reserved gutters, scrollbar-mode browser traps, the overlay+stable Chromium quirk). AGENTS.md's daisyUI section shrinks to the operative rules plus a pointer to the investigation file.
Multi-AI review (Codex, GLM, Kimi) of the prefix hardening surfaced five majors and a batch of minors — all addressed: - uuid_generate_v7()'s body called pgcrypto's gen_random_bytes/1 unqualified. A plpgsql body resolves identifiers via the CALLER's search_path at execution, so the schema-qualified function failed for roles whose search_path excludes pgcrypto's schema — defeating the point of qualifying it. The body now interpolates pgcrypto's actual installation schema (pg_extension lookup, public fallback while the extension's own CREATE is still queued on fresh chains). The prefix integration test now calls the function under SET LOCAL search_path TO '' to pin this. - The Oban-prefix warning grepped the whole config for prefix:"..." — which the config :phoenix_kit, prefix: entry itself satisfies, so the warning never fired exactly when needed. Replaced with a block-scoped check (ObanConfig.oban_block_missing_prefix?/1, unit-tested) that scans config.exs AND runtime.exs, treats any prefix: inside an Oban block (including computed values) as configured, and runs from both install and update. - Prefix validation now has no bypass: resolve_prefix/1 validates the resolved value (so --prefix "" and quoting-requiring values fail at the task boundary), add_prefix_configuration validates before persisting (outside its rescue), UUIDRepair validates at both entry points, migrated_version_runtime/Common.migrated_version reraise ArgumentError instead of swallowing it into "not installed", and Common's fallback queries are parameterized. - Multi-repo detection now adds an Igniter issue (halts the install) instead of a warning that let the pipeline continue onto guesses. - gen.migration: from_version == 0 means the generated migration IS the fresh install, so a non-public prefix keeps create_schema: true (the blanket false only holds for genuine upgrades). - Minors: pg_proc existence check pins pronargs = 0; ensure_extension! rejects unknown extension names (interpolated into DDL); the installer resolves the prefix from config for Oban wiring; the UUID-repair manual-fix message emits schema-qualified SQL; stale moduledocs updated; upgrade docs note the CREATE-on-schema requirement for locked-down prefixed installs.
Four parallel triage passes (security, delta-audit/docs, cleanliness, host-integration boundaries) over the prefix commits. All chains verified end to end; the fixes: Correctness / robustness: - Extension creation in migration context now runs IMMEDIATELY (repo().query!) instead of queued via execute — so the pgcrypto-schema lookup inside ensure_uuid_v7_function sees an extension created earlier in the same version (fresh chains) and resolves its real schema instead of falling back to public; also guarantees citext exists before any queued CREATE TABLE that uses the type. - heal_version_comment gained a local validate_prefix! guard (defense in depth — it interpolates the prefix into DDL and must not rely on every future caller pre-validating). - Multi-repo fallthrough in find_or_detect_repo made explicit (else clause) instead of riding with-semantics silently. Host-facing docs (the compile_env contract, verified loud-not-silent: Config.Provider.validate_compile_env raises at boot on a stale build): - PhoenixKit.Migration + PhoenixKit.SchemaPrefix document the recompile-after-config-change behavior and the boot-time check. - The installer's completion notice explains it for prefixed installs. - All four mix tasks document the --prefix config fallback, validation, persistence, and Oban wiring; removed the ancient 'Current version: V17' blurb from update's docs; fixed the garbled --create-schema help entry; V27's moduledoc and the install error message no longer show Oban config under :phoenix_kit (it belongs to the host app). Consistency / cleanliness: - @SPEC on resolve_prefix, add_prefix_configuration, oban_block_missing_prefix?; new shared Helpers.public_prefix?/1; ensure_uuid_v7_function's repo variant documents that it raises; Helpers moduledoc no longer overstates qualify_table adoption; prose comments render <prefix> instead of a literal interpolation. New pinning tests (delta audit): - prefix_validation_test: invalid prefix raises at Postgres.up/down, migrated_version_runtime, Common.check_installation_status, UUIDRepair (both entries), add_prefix_configuration; unknown extension name raises; migration_opts emits create_schema explicitly. - Integration: legacy-table (ALTER TABLE path) uuid default pinned to the prefixed function; upgrade-path re-ensure pinned by rolling the marker back one version with the function renamed away.
… detection
check_installation_status treated 'no version marker found' and 'the
database cannot be queried at all' identically — both fell through to
{:not_installed}. That answer drives real behavior: the status task
displayed 'Not installed' and the update task suggested a fresh install
while the actual state was unknowable.
Common.check_installation_status now probes connectivity (SELECT 1)
before concluding absence and returns {:unreachable, reason} when the
database can't be queried (including when no repo is configured).
mix phoenix_kit.status renders 'Unknown — database unreachable' with a
fix-the-connection next step; mix phoenix_kit.update warns and skips
migration generation instead of claiming PhoenixKit isn't installed.ddon pushed a commit
that referenced
this pull request
Jul 12, 2026
Review of the QR device-handoff login (#630) and prefix-hardening (#631) PRs surfaced real gaps: the qr_login_enabled setting wasn't an immediate kill switch on the phone-approval/completion paths, the public QR mint endpoint had no rate limiting, and V26's pgcrypto digest() backfill and the new Oban prefix-detection regex both had the same class of bugs the prefix-hardening PR was written to fix elsewhere. See CLAUDE_REVIEW.md in each PR's dev_docs directory for full findings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
2 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.
Supersedes #629 (closed before review to extend the batch). 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). Every finding was reproduced against source before fixing, and the whole batch went through a multi-AI review (Codex / GLM / Kimi) plus a four-dimension quality sweep whose findings are also fixed here.Migration chain
CREATE EXTENSION/CREATE SCHEMAcheck the CREATE privilege before the IF-NOT-EXISTS short-circuit, failing low-privilege roles even when the object exists. 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 create only when genuinely missing; a missing extension the role cannot create raises an operator-facing message listing the three required extensions (documented inPhoenixKit.Migrationfor DBA pre-provisioning). Extension creation runs immediately (not queued) so same-version lookups see it.create_schemaflag at the Oban handoff — Oban re-defaults it to true for non-public prefixes and executed the failingCREATE SCHEMAmid-chain (the report's "dies around v27–v31"). V27 now passescreate_schema: false; by V27 the schema always exists.uuid_generate_v7()was created unqualified — it landed whereversearch_pathpointed (pollutespublic; fails wherepublicisn't writable). Now created inside the install's schema with all ~89 call sites qualified, and — per review — the function body qualifies pgcrypto'sgen_random_byteswith the extension's actual schema (a plpgsql body resolves via the caller's search_path, which would have defeated the qualification).Postgres.up/1re-ensures the function at the prefix for upgrade chains starting ≥ V40. Existing installs keep their OID-pinned column defaults.[a-z_][a-z0-9_]*) at every entry point —up/down, the prefix-resolving tooling,UUIDRepair,Install.Common— and validation errors re-raise instead of being swallowed into "not installed". New sharedPhoenixKit.Migrations.Postgres.Helperscentralizes these patterns.Tooling
mix phoenix_kit.install --prefix Xpersistsconfig :phoenix_kit, prefix: "X"(newPhoenixKit.Install.PrefixConfig); update/status/gen.migration resolve--prefix→ config →"public", so a prefixed install no longer reads as "Not installed".Install.Commonno longer fabricates{:current_version, 1}from the mere existence of migration files — the bug that mademix phoenix_kit.updategenerate a from-scratch v01→v142 migration into the wrong schema. It now also distinguishes{:unreachable, reason}from{:not_installed}(a down DB must not read as an absent install); status renders "Unknown — database unreachable" and update refuses to generate migrations.create_schema: false;gen.migrationkeepscreate_schema: trueonly for a genuinely fresh (from_version 0) prefixed project.migration_optsalways emits the flag explicitly (omitting it silently discarded--create-schema=false).--repo— the field report's project got a migration-only repo silently wired into config.prefix:(V27 putsoban_jobsinside the named schema); both install and update warn when a pre-existing Oban block lacks it (block-scoped check across config.exs + runtime.exs — a whole-file grep is defeated by theconfig :phoenix_kit, prefix:entry itself).Runtime schema prefix
Until now only migrations honored the prefix — runtime queries relied on the DB role's
search_path(undocumented; the reporter's install depends on it). NewPhoenixKit.SchemaPrefixsets@schema_prefixfromApplication.compile_env(:phoenix_kit, :prefix); all 21 table-backed schemas use it, so every pathway (RepoHelper delegators, directrepo()calls,update_all/insert_all, Multi steps, preloads, joins) targets the named schema with zero call-site churn. Unset config compiles to nil — public installs are byte-for-byte unchanged. A conformance test blocks future schemas from skipping it. Compile-time by design; a stale build fails loudly at boot (Config.Provider.validate_compile_env) rather than silently queryingpublic— documented, and the installer notice explains it for prefixed installs.Disclosed limitation (documented in
PhoenixKit.Migration+ AGENTS.md): feature modules' own schemas need the same one-line adoption; until then prefixed installs running feature modules still needsearch_path.Verification
test/integration/prefix_migration_test.exsextended to five pins: function lives in the prefixed schema; CREATE TABLE-path and ALTER TABLE-path uuid defaults bound to it; the function works underSET LOCAL search_path TO ''; upgrade chains starting ≥ V40 re-ensure the function.public. Fresh public install from a dropped DB also clean.register_user(user + token + owner role + activity) landed entirely in a scratch schema,publicuntouched.PrefixConfig.resolve_prefix, Oban block-scoping, gen.migration content, SchemaPrefix conformance.mix precommitclean; full-suite failures identical to the pre-existing multi-session/sitemap set (verified by stash-rerun on the same base).daisyUI modal scrollbar-gutter (user-reported bug)
rootscrollgutter.css), which handles both page types correctly on its own.:root:has(.modal-open, …) { scrollbar-gutter: auto }rules (admin LayoutWrapper +layouts/root.html.heex) and PkDialog's inline override +_PkDialogOpenCountrefcount machinery (~90 lines of JS). Do not re-addscrollbar-gutteroverrides in layouts, PkDialog, or modules.assets/vendor/daisyui.js, scaffolded by phx.new) — a vendor-in-core auto-sync design was built, verified, and deliberately not adopted. Instead the newPhoenixKit.Install.DaisyUIdeclares a designed-for minimum (5.6.0; verified against 5.6.17/5.6.18) and warns when a host is behind:phoenix_kit.install(Igniter warning with curl upgrade steps),phoenix_kit.update(shell warning),phoenix_kit.doctor(new "daisyUI Version" check with pass / outdated / unversioned / missing states). Advisory only — nothing touches host files. Unit-tested.assets/vendor/files, not core CSS. This knowingly re-trades the 1.7.179 counter-rule's fix for old-daisyUI hosts.dev_docs/investigations/2026-07-12-daisyui-version-management-investigation.md.Notes for the reviewer
DROP INDEX schema.namesites the report warns about are deliberately untouched (qualified names are valid on DROP).