feat(rls): activate RLS at runtime via split migrator/app DB roles #224

Merged

David merged 2 commits from feat/pms-285-rls-runtime-role-split into main

2026-06-14 00:52:11 +02:00

David commented

2026-06-13 20:28:42 +02:00

Owner

Database now holds two pools: app_pool (role mokosh_app, NOSUPERUSER NOBYPASSRLS) returned by pool() and begin_with_tenant, used for every request-serving query; and migrator_pool (role mokosh_migrator, BYPASSRLS) used for migrations, bootstrap, the cross-tenant background workers, and the explicitly-justified pre-auth / cross-tenant paths. AppConfig gains MOKOSH_APP_DATABASE_URL (falls back to DATABASE_URL so a single-role dev box still boots with RLS inert). run_migrations and the SSO bootstrap move to the migrator pool.

Every raw self.pool() serving query is routed: tenant-scoped reads/writes go through begin_with_tenant (GUC set); genuinely cross-tenant / pre-auth / worker sites move to migrator_pool(), each with a // SAFETY: note. Named paths on the migrator pool: the Google (provider,subject) lookup + last_used_at update, password-reset token verify, find_user_placement, rehome_user_between_tenants, invitations::newest_pending_for + accept, the tenants super-admin handlers, copy_default_config (reads the default tenant, writes the new one), the notifications dispatcher / SLA sweep / contract lifecycle / recurring-invoicing / calendar-reminder / RMM-sync workers, and the startup admin bootstrap. Reads of the RLS-exempt tenants root and tenant-scoped reads of RLS-covered sla_targets are handled explicitly. Two latent bugs fixed: sla::upsert_target and sla clock read dropped the tenant tx before touching sla_targets, which would have failed WITH CHECK / fail-closed under the app role.

Step 9 (newest_pending_for): resolved as decision (a) - kept as a deliberate pre-auth cross-tenant bridge on the migrator pool with a SAFETY note, because it runs before the user is placed in any tenant and reshaping it to a tenant-scoped query is not possible there; under the app role with no GUC it would fail closed and break invite acceptance.

Step 8 (from_trusted audit): every TenantId::from_trusted call site now carries a current SAFETY note (28 sites on this branch; the issue's "53" predates the PMS-258/PMS-236 merges), completing PMS-261 AC2.

Roles are provisioned by a new mokosh-bootstrap provision-roles subcommand (idempotent: creates the roles, grants mokosh_app SELECT/INSERT/UPDATE/DELETE on tables, USAGE/SELECT on sequences, EXECUTE on functions, plus ALTER DEFAULT PRIVILEGES FOR ROLE mokosh_migrator so future migrations auto-grant; mokosh_app owns nothing). The dev compose.dev.yml postgres entrypoint provisions both roles on boot and the server points DATABASE_URL / MOKOSH_APP_DATABASE_URL at them, so just dev boots with the split; .env.example documents the new knobs.

Tests: the integration harness gains boot_rls, which builds the app pool as a fresh NOBYPASSRLS role against the per-test database; tests/per_user_isolation.rs runs its request queries through it so the HTTP suite exercises RLS rather than only the app-layer WHERE filters, plus a new app_role_read_is_fail_closed_without_guc regression proving a no-GUC read returns zero rows through the actual application role.

#PMS-285

Activate the fail-closed RLS policy (PMS-257) at runtime by splitting the database connection into two roles so the request-serving connection physically cannot bypass per-tenant Row Level Security. A forgotten `WHERE tenant_id` filter now fail-closes to zero rows instead of leaking across the personal-tenant boundary. `Database` now holds two pools: `app_pool` (role `mokosh_app`, NOSUPERUSER NOBYPASSRLS) returned by `pool()` and `begin_with_tenant`, used for every request-serving query; and `migrator_pool` (role `mokosh_migrator`, BYPASSRLS) used for migrations, bootstrap, the cross-tenant background workers, and the explicitly-justified pre-auth / cross-tenant paths. `AppConfig` gains `MOKOSH_APP_DATABASE_URL` (falls back to `DATABASE_URL` so a single-role dev box still boots with RLS inert). `run_migrations` and the SSO bootstrap move to the migrator pool. Every raw `self.pool()` serving query is routed: tenant-scoped reads/writes go through `begin_with_tenant` (GUC set); genuinely cross-tenant / pre-auth / worker sites move to `migrator_pool()`, each with a `// SAFETY:` note. Named paths on the migrator pool: the Google `(provider,subject)` lookup + `last_used_at` update, password-reset token verify, `find_user_placement`, `rehome_user_between_tenants`, `invitations::newest_pending_for` + `accept`, the `tenants` super-admin handlers, `copy_default_config` (reads the default tenant, writes the new one), the notifications dispatcher / SLA sweep / contract lifecycle / recurring-invoicing / calendar-reminder / RMM-sync workers, and the startup admin bootstrap. Reads of the RLS-exempt `tenants` root and tenant-scoped reads of RLS-covered `sla_targets` are handled explicitly. Two latent bugs fixed: `sla::upsert_target` and `sla` clock read dropped the tenant tx before touching `sla_targets`, which would have failed WITH CHECK / fail-closed under the app role. Step 9 (`newest_pending_for`): resolved as decision (a) - kept as a deliberate pre-auth cross-tenant bridge on the migrator pool with a SAFETY note, because it runs before the user is placed in any tenant and reshaping it to a tenant-scoped query is not possible there; under the app role with no GUC it would fail closed and break invite acceptance. Step 8 (`from_trusted` audit): every `TenantId::from_trusted` call site now carries a current SAFETY note (28 sites on this branch; the issue's "53" predates the PMS-258/PMS-236 merges), completing PMS-261 AC2. Roles are provisioned by a new `mokosh-bootstrap provision-roles` subcommand (idempotent: creates the roles, grants `mokosh_app` SELECT/INSERT/UPDATE/DELETE on tables, USAGE/SELECT on sequences, EXECUTE on functions, plus `ALTER DEFAULT PRIVILEGES FOR ROLE mokosh_migrator` so future migrations auto-grant; `mokosh_app` owns nothing). The dev `compose.dev.yml` postgres entrypoint provisions both roles on boot and the server points `DATABASE_URL` / `MOKOSH_APP_DATABASE_URL` at them, so `just dev` boots with the split; `.env.example` documents the new knobs. Tests: the integration harness gains `boot_rls`, which builds the app pool as a fresh NOBYPASSRLS role against the per-test database; `tests/per_user_isolation.rs` runs its request queries through it so the HTTP suite exercises RLS rather than only the app-layer `WHERE` filters, plus a new `app_role_read_is_fail_closed_without_guc` regression proving a no-GUC read returns zero rows through the actual application role. #PMS-285

David added 1 commit

2026-06-13 20:28:43 +02:00

feat(rls): activate RLS at runtime via split migrator/app DB roles

E2E / Playwright against staging (pull_request) Failing after 41s

Details

Check / fmt + clippy + compile + tests (pull_request) Failing after 3m44s

Details

f67057c74f

Activate the fail-closed RLS policy (PMS-257) at runtime by splitting the database connection into two roles so the request-serving connection physically cannot bypass per-tenant Row Level Security. A forgotten `WHERE tenant_id` filter now fail-closes to zero rows instead of leaking across the personal-tenant boundary.

`Database` now holds two pools: `app_pool` (role `mokosh_app`, NOSUPERUSER NOBYPASSRLS) returned by `pool()` and `begin_with_tenant`, used for every request-serving query; and `migrator_pool` (role `mokosh_migrator`, BYPASSRLS) used for migrations, bootstrap, the cross-tenant background workers, and the explicitly-justified pre-auth / cross-tenant paths. `AppConfig` gains `MOKOSH_APP_DATABASE_URL` (falls back to `DATABASE_URL` so a single-role dev box still boots with RLS inert). `run_migrations` and the SSO bootstrap move to the migrator pool.

Every raw `self.pool()` serving query is routed: tenant-scoped reads/writes go through `begin_with_tenant` (GUC set); genuinely cross-tenant / pre-auth / worker sites move to `migrator_pool()`, each with a `// SAFETY:` note. Named paths on the migrator pool: the Google `(provider,subject)` lookup + `last_used_at` update, password-reset token verify, `find_user_placement`, `rehome_user_between_tenants`, `invitations::newest_pending_for` + `accept`, the `tenants` super-admin handlers, `copy_default_config` (reads the default tenant, writes the new one), the notifications dispatcher / SLA sweep / contract lifecycle / recurring-invoicing / calendar-reminder / RMM-sync workers, and the startup admin bootstrap. Reads of the RLS-exempt `tenants` root and tenant-scoped reads of RLS-covered `sla_targets` are handled explicitly. Two latent bugs fixed: `sla::upsert_target` and `sla` clock read dropped the tenant tx before touching `sla_targets`, which would have failed WITH CHECK / fail-closed under the app role.

Step 9 (`newest_pending_for`): resolved as decision (a) - kept as a deliberate pre-auth cross-tenant bridge on the migrator pool with a SAFETY note, because it runs before the user is placed in any tenant and reshaping it to a tenant-scoped query is not possible there; under the app role with no GUC it would fail closed and break invite acceptance.

Step 8 (`from_trusted` audit): every `TenantId::from_trusted` call site now carries a current SAFETY note (28 sites on this branch; the issue's "53" predates the PMS-258/PMS-236 merges), completing PMS-261 AC2.

Roles are provisioned by a new `mokosh-bootstrap provision-roles` subcommand (idempotent: creates the roles, grants `mokosh_app` SELECT/INSERT/UPDATE/DELETE on tables, USAGE/SELECT on sequences, EXECUTE on functions, plus `ALTER DEFAULT PRIVILEGES FOR ROLE mokosh_migrator` so future migrations auto-grant; `mokosh_app` owns nothing). The dev `compose.dev.yml` postgres entrypoint provisions both roles on boot and the server points `DATABASE_URL` / `MOKOSH_APP_DATABASE_URL` at them, so `just dev` boots with the split; `.env.example` documents the new knobs.

Tests: the integration harness gains `boot_rls`, which builds the app pool as a fresh NOBYPASSRLS role against the per-test database; `tests/per_user_isolation.rs` runs its request queries through it so the HTTP suite exercises RLS rather than only the app-layer `WHERE` filters, plus a new `app_role_read_is_fail_closed_without_guc` regression proving a no-GUC read returns zero rows through the actual application role.

#PMS-285

David added 1 commit

2026-06-14 00:47:47 +02:00

Merge origin/main into feat/pms-285-rls-runtime-role-split

E2E / Playwright against staging (pull_request) Failing after 16s

Details

Check / fmt + clippy + compile + tests (pull_request) Successful in 3m11s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

714658e4f4

Resolve conflicts where PMS-285's runtime-RLS role split met newer merged work:
- src/main.rs: keep migrator_pool() for SSO bootstrap (PMS-285) and main's SsoSetup::Mounted match arm together.
- src/modules/contracts/service.rs: keep PMS-194's per-tenant bounded-transaction sweep but route it through migrator_pool() (BYPASSRLS) per PMS-285, since the cross-tenant cadence cannot set a single tenant GUC.
- src/modules/tenants/service.rs: drop get_tenant_by_slug (dead, removed by PMS-202; PMS-285 only re-annotated it, no caller exists).
- tests/common/mod.rs: keep both PgPoolOptions (PMS-285 harness) and rust_decimal::Decimal (main) imports.

#PMS-285