fix(security): enable RLS automatically for plugin-created tenant tables
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
Plugin migrations run after core migration 0085 which sets up RLS for all known core tables. Plugin-created tables were left without RLS, creating a critical multi-tenant isolation gap (84 tables affected). The migration runner now automatically enables RLS on all newly created tenant tables after validation: - ENABLE + FORCE ROW LEVEL SECURITY - Idempotent DROP IF EXISTS + CREATE fail-closed tenant isolation policy - GRANT CRUD to crm_api and crm_worker - ALTER TABLE OWNER TO crm_migration Global tables (-- GLOBAL TABLE comment) are skipped.
This commit is contained in:
@@ -113,6 +113,12 @@ class MigrationRunner:
|
||||
f"If a table is intentionally global, add `-- GLOBAL TABLE: table_name` comment to the migration SQL."
|
||||
)
|
||||
|
||||
# Enable RLS on all newly created tenant tables.
|
||||
# Plugin migrations run AFTER core alembic migration 0085, which sets up RLS
|
||||
# for all known core tables. Without this step, plugin-created tables would
|
||||
# be left without RLS — a critical multi-tenant isolation gap.
|
||||
await self._enable_rls_for_tables(db, created_tables, sql_content)
|
||||
|
||||
# Record the migration in plugin_migrations table
|
||||
migration_record = PluginMigration(
|
||||
plugin_name=plugin_name,
|
||||
@@ -485,3 +491,83 @@ class MigrationRunner:
|
||||
|
||||
pattern = r'--\s*GLOBAL\s+TABLE:\s*(\w+)'
|
||||
return set(re.findall(pattern, sql, re.IGNORECASE))
|
||||
|
||||
async def _enable_rls_for_tables(
|
||||
self,
|
||||
db: AsyncSession,
|
||||
created_tables: set[str],
|
||||
sql_content: str,
|
||||
) -> None:
|
||||
"""Enable Row Level Security on all newly created tenant tables.
|
||||
|
||||
Plugin migrations run AFTER core alembic migration 0085, which sets up RLS
|
||||
for all known core tables. Without this step, plugin-created tables would
|
||||
be left without RLS — a critical multi-tenant isolation gap. This method
|
||||
replicates the exact RLS pattern from 0085 for every new tenant table:
|
||||
|
||||
1. ALTER TABLE ... ENABLE ROW LEVEL SECURITY
|
||||
2. ALTER TABLE ... FORCE ROW LEVEL SECURITY
|
||||
3. DROP old policies (idempotent) + CREATE fail-closed tenant isolation policy
|
||||
4. GRANT CRUD to crm_api and crm_worker
|
||||
5. ALTER TABLE ... OWNER TO crm_migration
|
||||
|
||||
Global tables (marked with `-- GLOBAL TABLE` in SQL) are skipped.
|
||||
"""
|
||||
global_tables = self._extract_global_table_names(sql_content)
|
||||
tenant_tables = created_tables - global_tables
|
||||
|
||||
for table_name in tenant_tables:
|
||||
# Idempotent: DROP old policies first, then ENABLE + FORCE RLS
|
||||
await db.execute(
|
||||
text(
|
||||
f"DO $$ BEGIN "
|
||||
f"IF EXISTS (SELECT 1 FROM information_schema.tables "
|
||||
f"WHERE table_schema = 'public' AND table_name = '{table_name}') THEN "
|
||||
f"DROP POLICY IF EXISTS tenant_isolation ON public.{table_name}; "
|
||||
f"DROP POLICY IF EXISTS {table_name}_tenant_isolation ON public.{table_name}; "
|
||||
f"ALTER TABLE public.{table_name} ENABLE ROW LEVEL SECURITY; "
|
||||
f"ALTER TABLE public.{table_name} FORCE ROW LEVEL SECURITY; "
|
||||
f"END IF; END $$"
|
||||
)
|
||||
)
|
||||
|
||||
# Create fail-closed tenant isolation policy (same pattern as 0085)
|
||||
await db.execute(
|
||||
text(
|
||||
f"DO $$ BEGIN "
|
||||
f"IF EXISTS (SELECT 1 FROM information_schema.tables "
|
||||
f"WHERE table_schema = 'public' AND table_name = '{table_name}') THEN "
|
||||
f"CREATE POLICY {table_name}_tenant_isolation "
|
||||
f"ON public.{table_name} "
|
||||
f"FOR ALL "
|
||||
f"TO crm_api, crm_worker "
|
||||
f"USING (tenant_id = NULLIF(current_setting('app.current_tenant_id', true), '')::uuid) "
|
||||
f"WITH CHECK (tenant_id = NULLIF(current_setting('app.current_tenant_id', true), '')::uuid); "
|
||||
f"END IF; END $$"
|
||||
)
|
||||
)
|
||||
|
||||
# Grant CRUD to runtime roles
|
||||
await db.execute(
|
||||
text(
|
||||
f"DO $$ BEGIN "
|
||||
f"IF EXISTS (SELECT 1 FROM information_schema.tables "
|
||||
f"WHERE table_schema = 'public' AND table_name = '{table_name}') THEN "
|
||||
f"GRANT SELECT, INSERT, UPDATE, DELETE ON public.{table_name} TO crm_api; "
|
||||
f"GRANT SELECT, INSERT, UPDATE, DELETE ON public.{table_name} TO crm_worker; "
|
||||
f"END IF; END $$"
|
||||
)
|
||||
)
|
||||
|
||||
# Transfer ownership to crm_migration (same as 0085)
|
||||
await db.execute(
|
||||
text(
|
||||
f"DO $$ BEGIN "
|
||||
f"IF EXISTS (SELECT 1 FROM information_schema.tables "
|
||||
f"WHERE table_schema = 'public' AND table_name = '{table_name}') THEN "
|
||||
f"ALTER TABLE public.{table_name} OWNER TO crm_migration; "
|
||||
f"END IF; END $$"
|
||||
)
|
||||
)
|
||||
|
||||
await db.flush()
|
||||
|
||||
Reference in New Issue
Block a user