feat(rls): flip tenant_isolation fail-closed with WITH CHECK and FORCE RLS #188

Merged
David merged 2 commits from feat/pms-257-rls-fail-closed into main 2026-06-13 12:59:02 +02:00
Owner

Migration 038 rewrites the tenant_isolation RLS policy on every tenant_id table. The 024 policy was fail-open (USING (tenant_id = COALESCE(NULLIF(current_setting('app.current_tenant', true), '')::uuid, tenant_id))), so an unset GUC matched every row, and it was USING-only so it never constrained INSERT/UPDATE. The new policy uses USING (tenant_id = NULLIF(current_setting('app.current_tenant', true), '')::uuid) plus the same expression as WITH CHECK, so an unset or empty GUC matches no rows (read) and rejects the write, and a write cannot set or move a row into another tenant. Each table also gets FORCE ROW LEVEL SECURITY so the owner is not exempt.

The loop mirrors 024's information_schema selection, so it also covers tables created after 024 (027/028/031/032/...) that never received the original policy. DROP POLICY IF EXISTS makes it idempotent and lets it replace the 024 policy in place.

FORCE RLS does not apply to superusers or BYPASSRLS roles, so the migration / owner role keeps operating unrestricted while the application role must run without BYPASSRLS for the policy to bite. Roles are cluster-global and environment-specific, so the migration documents this posture rather than creating or altering roles; pointing the app connection at an unprivileged role is a deployment step gated on migrating the remaining read paths onto begin_with_tenant (parent PMS-255). Until then the app still connects as the bypassing role and relies on explicit WHERE tenant_id = $1 filters, so this flip is a no-op for the running app and safe to land now.

tests/rls_isolation.rs proves the behaviour against an explicitly NOBYPASSRLS role via SET ROLE: an unset GUC exposes zero rows, the matching GUC exposes exactly the tenant's row, a write with the wrong GUC is rejected (42501), and a matching write passes.

#PMS-257

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Migration 038 rewrites the `tenant_isolation` RLS policy on every tenant_id table. The 024 policy was fail-open (`USING (tenant_id = COALESCE(NULLIF(current_setting('app.current_tenant', true), '')::uuid, tenant_id))`), so an unset GUC matched every row, and it was USING-only so it never constrained INSERT/UPDATE. The new policy uses `USING (tenant_id = NULLIF(current_setting('app.current_tenant', true), '')::uuid)` plus the same expression as `WITH CHECK`, so an unset or empty GUC matches no rows (read) and rejects the write, and a write cannot set or move a row into another tenant. Each table also gets `FORCE ROW LEVEL SECURITY` so the owner is not exempt. The loop mirrors 024's information_schema selection, so it also covers tables created after 024 (027/028/031/032/...) that never received the original policy. `DROP POLICY IF EXISTS` makes it idempotent and lets it replace the 024 policy in place. FORCE RLS does not apply to superusers or BYPASSRLS roles, so the migration / owner role keeps operating unrestricted while the application role must run without BYPASSRLS for the policy to bite. Roles are cluster-global and environment-specific, so the migration documents this posture rather than creating or altering roles; pointing the app connection at an unprivileged role is a deployment step gated on migrating the remaining read paths onto `begin_with_tenant` (parent PMS-255). Until then the app still connects as the bypassing role and relies on explicit `WHERE tenant_id = $1` filters, so this flip is a no-op for the running app and safe to land now. tests/rls_isolation.rs proves the behaviour against an explicitly NOBYPASSRLS role via `SET ROLE`: an unset GUC exposes zero rows, the matching GUC exposes exactly the tenant's row, a write with the wrong GUC is rejected (42501), and a matching write passes. #PMS-257 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
feat(rls): flip tenant_isolation fail-closed with WITH CHECK and FORCE RLS
All checks were successful
E2E (staging) / Playwright against staging (pull_request) Successful in 45s
Check / fmt + clippy + compile + tests (pull_request) Successful in 2m24s
Build OCI container / Build and push mokosh-api image (push) Successful in 3m42s
3476ed6ae7
Migration 038 rewrites the `tenant_isolation` RLS policy on every tenant_id table. The 024 policy was fail-open (`USING (tenant_id = COALESCE(NULLIF(current_setting('app.current_tenant', true), '')::uuid, tenant_id))`), so an unset GUC matched every row, and it was USING-only so it never constrained INSERT/UPDATE. The new policy uses `USING (tenant_id = NULLIF(current_setting('app.current_tenant', true), '')::uuid)` plus the same expression as `WITH CHECK`, so an unset or empty GUC matches no rows (read) and rejects the write, and a write cannot set or move a row into another tenant. Each table also gets `FORCE ROW LEVEL SECURITY` so the owner is not exempt.

The loop mirrors 024's information_schema selection, so it also covers tables created after 024 (027/028/031/032/...) that never received the original policy. `DROP POLICY IF EXISTS` makes it idempotent and lets it replace the 024 policy in place.

FORCE RLS does not apply to superusers or BYPASSRLS roles, so the migration / owner role keeps operating unrestricted while the application role must run without BYPASSRLS for the policy to bite. Roles are cluster-global and environment-specific, so the migration documents this posture rather than creating or altering roles; pointing the app connection at an unprivileged role is a deployment step gated on migrating the remaining read paths onto `begin_with_tenant` (parent PMS-255). Until then the app still connects as the bypassing role and relies on explicit `WHERE tenant_id = $1` filters, so this flip is a no-op for the running app and safe to land now.

tests/rls_isolation.rs proves the behaviour against an explicitly NOBYPASSRLS role via `SET ROLE`: an unset GUC exposes zero rows, the matching GUC exposes exactly the tenant's row, a write with the wrong GUC is rejected (42501), and a matching write passes.

#PMS-257

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merge branch 'main' into feat/pms-257-rls-fail-closed
All checks were successful
Create release / Create release from merged PR (pull_request) Has been skipped
Check / fmt + clippy + compile + tests (pull_request) Successful in 1m53s
E2E (staging) / Playwright against staging (pull_request) Successful in 1m11s
Build OCI container / Build and push mokosh-api image (push) Successful in 3m54s
6175e84623
David merged commit 100c59f2c1 into main 2026-06-13 12:59:02 +02:00
David deleted branch feat/pms-257-rls-fail-closed 2026-06-13 12:59:02 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!188
No description provided.