diff --git a/src/core/migrate.ts b/src/core/migrate.ts index 2f2d82fc3..6694bb894 100644 --- a/src/core/migrate.ts +++ b/src/core/migrate.ts @@ -713,19 +713,7 @@ export const MIGRATIONS: Migration[] = [ has_bypass BOOLEAN; BEGIN SELECT rolbypassrls INTO has_bypass FROM pg_roles WHERE rolname = current_user; - IF has_bypass THEN - ALTER TABLE access_tokens ENABLE ROW LEVEL SECURITY; - ALTER TABLE mcp_request_log ENABLE ROW LEVEL SECURITY; - ALTER TABLE minion_inbox ENABLE ROW LEVEL SECURITY; - ALTER TABLE minion_attachments ENABLE ROW LEVEL SECURITY; - ALTER TABLE subagent_messages ENABLE ROW LEVEL SECURITY; - ALTER TABLE subagent_tool_executions ENABLE ROW LEVEL SECURITY; - ALTER TABLE subagent_rate_leases ENABLE ROW LEVEL SECURITY; - ALTER TABLE gbrain_cycle_locks ENABLE ROW LEVEL SECURITY; - ALTER TABLE budget_ledger ENABLE ROW LEVEL SECURITY; - ALTER TABLE budget_reservations ENABLE ROW LEVEL SECURITY; - RAISE NOTICE 'v24: RLS enabled on 10 backfill tables (role % has BYPASSRLS)', current_user; - ELSE + IF NOT has_bypass THEN -- Fail the migration loudly instead of WARNING + version-bump. -- The runner unconditionally records schema_version on success, -- so a silent WARNING here would permanently lock the backfill out @@ -734,6 +722,36 @@ export const MIGRATIONS: Migration[] = [ -- and lets the next invocation retry after the role is fixed. RAISE EXCEPTION 'v24 rls_backfill_missing_tables: role % does not have BYPASSRLS privilege — cannot enable RLS safely. Re-run as postgres (or another BYPASSRLS role). The migration will retry automatically on the next initSchema call.', current_user; END IF; + + -- These 8 are guaranteed to exist: schema.sql creates them (idempotent + -- via IF NOT EXISTS) on every initSchema call, and initSchema runs + -- before this migration. Bare ALTER TABLE is safe. + ALTER TABLE access_tokens ENABLE ROW LEVEL SECURITY; + ALTER TABLE mcp_request_log ENABLE ROW LEVEL SECURITY; + ALTER TABLE minion_inbox ENABLE ROW LEVEL SECURITY; + ALTER TABLE minion_attachments ENABLE ROW LEVEL SECURITY; + ALTER TABLE subagent_messages ENABLE ROW LEVEL SECURITY; + ALTER TABLE subagent_tool_executions ENABLE ROW LEVEL SECURITY; + ALTER TABLE subagent_rate_leases ENABLE ROW LEVEL SECURITY; + ALTER TABLE gbrain_cycle_locks ENABLE ROW LEVEL SECURITY; + + -- budget_ledger + budget_reservations are migration-only (v12). Not + -- in schema.sql, not re-created on every initSchema. In normal flow + -- v12 runs before v24 so they exist, but if an operator manually + -- dropped them (unusual — budget data is regenerable from resolver + -- logs) or was pinned to a pre-v12 gbrain version when the table + -- went away, the bare ALTER would fail with 42P01 and abort v24. + -- information_schema.tables lookup makes the statement self-healing. + IF EXISTS (SELECT 1 FROM information_schema.tables + WHERE table_schema = 'public' AND table_name = 'budget_ledger') THEN + ALTER TABLE budget_ledger ENABLE ROW LEVEL SECURITY; + END IF; + IF EXISTS (SELECT 1 FROM information_schema.tables + WHERE table_schema = 'public' AND table_name = 'budget_reservations') THEN + ALTER TABLE budget_reservations ENABLE ROW LEVEL SECURITY; + END IF; + + RAISE NOTICE 'v24: RLS backfill complete (role % has BYPASSRLS)', current_user; END $$; `, }, diff --git a/test/migrate.test.ts b/test/migrate.test.ts index 047e9c4e7..6ebbbc31c 100644 --- a/test/migrate.test.ts +++ b/test/migrate.test.ts @@ -218,7 +218,28 @@ describe('migration v24 — rls_backfill_missing_tables', () => { const v24 = MIGRATIONS.find(m => m.version === 24); const sql = v24!.sql || ''; expect(sql).toContain('rolbypassrls'); - expect(sql).toContain('IF has_bypass THEN'); + // The gate can be either IF has_bypass / early-raise pattern. + expect(sql).toMatch(/IF (NOT )?has_bypass/); + }); + + // Self-healing guard: the budget_* tables are migration-only (v12). If an + // operator manually dropped them, or if a brain was somehow pinned to a + // pre-v12 version when those tables didn't exist, a bare `ALTER TABLE + // budget_ledger ...` would fail with 42P01 and abort v24. Wrapping those + // two ALTERs in an `IF EXISTS (information_schema.tables ...)` check lets + // the migration skip them silently instead of erroring out. The other 8 + // tables are created by schema.sql on every initSchema and don't need + // the guard — bare ALTER is fine. + test('guards budget_ledger + budget_reservations with information_schema.tables IF EXISTS', () => { + const v24 = MIGRATIONS.find(m => m.version === 24); + const sql = v24!.sql || ''; + // Both budget tables must be wrapped in an existence check. + expect(sql).toMatch( + /IF EXISTS \(SELECT 1 FROM information_schema\.tables[\s\S]{0,200}table_name = 'budget_ledger'\)[\s\S]{0,200}ALTER TABLE budget_ledger ENABLE ROW LEVEL SECURITY/, + ); + expect(sql).toMatch( + /IF EXISTS \(SELECT 1 FROM information_schema\.tables[\s\S]{0,200}table_name = 'budget_reservations'\)[\s\S]{0,200}ALTER TABLE budget_reservations ENABLE ROW LEVEL SECURITY/, + ); }); // Codex found: if v24 RAISE WARNINGs instead of raising on non-BYPASSRLS,