fix(db): harden the RLS tenant scope against an empty app.tenant_id (coreci-chat#13)

2026-10-27 06:57:51 +00:00
parent 311dfe5ac0
commit 9b4bbd01af
1 changed files with 66 additions and 0 deletions
@@ -0,0 +1,66 @@
+-- 0004_rls_scope_hardening.sql - RLS tenant-scope hardening (coreci-chat#13).
+--
+-- The tenant-scoped policies created by 0001_init.sql and 0003_mcp_adapters.sql
+-- cast the session's tenant scope straight to uuid:
+--
+--   USING (tenant_id = current_setting('app.tenant_id', true)::uuid)
+--
+-- `current_setting(name, missing_ok := true)` returns NULL when the setting was
+-- never set in this session - but it returns an EMPTY STRING, not NULL, once
+-- the session holds a placeholder for it. Postgres creates that placeholder the
+-- first time a transaction-local `set_config('app.tenant_id', ..., true)` runs
+-- (that is withTenant), and when that transaction ends the placeholder's value
+-- is ''. So on any pooled connection that has already run a withTenant
+-- transaction, the NEXT statement that touches a tenant-scoped table casts ''
+-- to uuid and the database raises:
+--
+--   invalid input syntax for type uuid: ""
+--
+-- That is the crash the control-plane dashboard suite hit under real Postgres
+-- (run 55595, job 57855: 16 assertions, provisioning.ts:177 via :84). PGlite
+-- never showed it because PGlite 0.5.7 does not enforce RLS on SELECT, so the
+-- policy expression is never evaluated there.
+--
+-- The fix keeps the documented semantics ("a query outside withTenant sees
+-- zero tenant-scoped rows") and removes the crash: nullif(..., '') maps the
+-- empty scope to NULL, so the comparison is NULL - not true - and the row is
+-- filtered out instead of the statement dying.
+--
+-- DROP + CREATE keeps this file idempotent: re-running it replaces the
+-- policies instead of failing on "policy already exists". The migrator role
+-- owns the tables, so it can drop and recreate them.
+--
+-- Cross-tenant behaviour is unchanged: a row whose tenant_id differs from the
+-- session's scope still fails the WITH CHECK clause - the R-009 pen test
+-- asserts exactly that.
+
+DROP POLICY IF EXISTS tenant_isolation_tenant_memberships ON tenant_memberships;
+CREATE POLICY tenant_isolation_tenant_memberships ON tenant_memberships
+  USING (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid)
+  WITH CHECK (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid);
+
+DROP POLICY IF EXISTS tenant_isolation_invitations ON invitations;
+CREATE POLICY tenant_isolation_invitations ON invitations
+  USING (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid)
+  WITH CHECK (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid);
+
+DROP POLICY IF EXISTS tenant_isolation_targets ON targets;
+DROP POLICY IF EXISTS tenant_isolation_targets ON targets;
+CREATE POLICY tenant_isolation_targets ON targets
+  USING (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid)
+  WITH CHECK (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid);
+
+DROP POLICY IF EXISTS tenant_isolation_byom_endpoints ON byom_endpoints;
+CREATE POLICY tenant_isolation_byom_endpoints ON byom_endpoints
+  USING (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid)
+  WITH CHECK (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid);
+
+DROP POLICY IF EXISTS tenant_isolation_audit_log ON audit_log;
+CREATE POLICY tenant_isolation_audit_log ON audit_log
+  USING (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid)
+  WITH CHECK (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid);
+
+DROP POLICY IF EXISTS tenant_isolation_mcp_adapters ON mcp_adapters;
+CREATE POLICY tenant_isolation_mcp_adapters ON mcp_adapters
+  USING (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid)
+  WITH CHECK (tenant_id = nullif(current_setting('app.tenant_id', true), '')::uuid);