From d64bdc015cdf7b7fd9b59a74454613b94afca9a9 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 25 Jun 2026 09:45:02 -0700 Subject: [PATCH] fix(security): correct plpgsql alias collision in #1385 BYPASSRLS gate (real-PG) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The broadened BYPASSRLS preflight aliased `pg_roles r`, but several RLS DO-blocks already declare `r record` for their backfill FOR loop, so plpgsql resolved `r.oid`/`r.rolbypassrls` to the unassigned record variable → "record \"r\" is not assigned yet" on real Postgres (PGLite tolerated it; the DATABASE_URL-gated e2e jobs are the backstop). Renamed the subquery alias to `pr` at all 10 migrate.ts sites; also broadened the schema.sql base RLS gate the same way (with the `pr` alias) for #1385 consistency on superuser fresh installs, and regenerated schema-embedded.ts. Also fixes a PRE-EXISTING engine-parity bug (confirmed failing on clean origin/master): the relationalFanout shape compared `canonical_chunk_id`, a serial id that diverges between a fresh PGLite engine and a shared Postgres DB (setupDB TRUNCATEs without RESTART IDENTITY). Compare its presence, not the exact value. Validated on real Postgres (pgvector/pg16): migration v120 applies, the v35 RLS backfill runs, and engine-parity + postgres-bootstrap + jsonb-parity are green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/migrate.ts | 20 ++++++++++---------- src/core/schema-embedded.ts | 4 +++- src/schema.sql | 4 +++- test/e2e/engine-parity.test.ts | 7 ++++++- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/core/migrate.ts b/src/core/migrate.ts index 54155c58e..61ee2da07 100644 --- a/src/core/migrate.ts +++ b/src/core/migrate.ts @@ -862,7 +862,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF NOT has_bypass THEN -- Fail the migration loudly instead of WARNING + version-bump. -- The runner unconditionally records schema_version on success, @@ -1151,7 +1151,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF NOT has_bypass THEN RAISE EXCEPTION 'v29 cathedral_ii_code_edges_rls: 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; @@ -1239,7 +1239,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF has_bypass THEN ALTER TABLE takes ENABLE ROW LEVEL SECURITY; ALTER TABLE synthesis_evidence ENABLE ROW LEVEL SECURITY; @@ -1341,7 +1341,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF has_bypass THEN ALTER TABLE dream_verdicts ENABLE ROW LEVEL SECURITY; END IF; @@ -1380,7 +1380,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF NOT has_bypass THEN RAISE EXCEPTION 'v31 eval_capture_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; @@ -1499,7 +1499,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF has_bypass THEN ALTER TABLE oauth_clients ENABLE ROW LEVEL SECURITY; ALTER TABLE oauth_tokens ENABLE ROW LEVEL SECURITY; @@ -1720,7 +1720,7 @@ export const MIGRATIONS: Migration[] = [ has_bypass BOOLEAN; r record; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF NOT has_bypass THEN -- Same posture as v24: raise to abort the migration so the runner -- leaves config.version unbumped and retries on the next call. @@ -2112,7 +2112,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF has_bypass THEN ALTER TABLE drift_decisions ENABLE ROW LEVEL SECURITY; END IF; @@ -2365,7 +2365,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF has_bypass THEN ALTER TABLE facts ENABLE ROW LEVEL SECURITY; END IF; @@ -4389,7 +4389,7 @@ export const MIGRATIONS: Migration[] = [ DECLARE has_bypass BOOLEAN; BEGIN - SELECT EXISTS (SELECT 1 FROM pg_roles r WHERE pg_has_role(current_user, r.oid, 'USAGE') AND (r.rolbypassrls OR r.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; -- #1385: superuser + inherited-role BYPASSRLS, not just the role's own rolbypassrls IF has_bypass THEN ALTER TABLE take_domain_assignments ENABLE ROW LEVEL SECURITY; END IF; diff --git a/src/core/schema-embedded.ts b/src/core/schema-embedded.ts index 8fb66be2d..e5ed5768b 100644 --- a/src/core/schema-embedded.ts +++ b/src/core/schema-embedded.ts @@ -1383,7 +1383,9 @@ DO \$\$ DECLARE has_bypass BOOLEAN; BEGIN - SELECT rolbypassrls INTO has_bypass FROM pg_roles WHERE rolname = current_user; + -- #1385: recognize superuser + inherited-role BYPASSRLS, not just the role's + -- own rolbypassrls (alias \`pr\` avoids any plpgsql record-variable collision). + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; IF has_bypass THEN ALTER TABLE pages ENABLE ROW LEVEL SECURITY; ALTER TABLE content_chunks ENABLE ROW LEVEL SECURITY; diff --git a/src/schema.sql b/src/schema.sql index 292240f2f..463d27403 100644 --- a/src/schema.sql +++ b/src/schema.sql @@ -1379,7 +1379,9 @@ DO $$ DECLARE has_bypass BOOLEAN; BEGIN - SELECT rolbypassrls INTO has_bypass FROM pg_roles WHERE rolname = current_user; + -- #1385: recognize superuser + inherited-role BYPASSRLS, not just the role's + -- own rolbypassrls (alias `pr` avoids any plpgsql record-variable collision). + SELECT EXISTS (SELECT 1 FROM pg_roles pr WHERE pg_has_role(current_user, pr.oid, 'USAGE') AND (pr.rolbypassrls OR pr.rolsuper)) INTO has_bypass; IF has_bypass THEN ALTER TABLE pages ENABLE ROW LEVEL SECURITY; ALTER TABLE content_chunks ENABLE ROW LEVEL SECURITY; diff --git a/test/e2e/engine-parity.test.ts b/test/e2e/engine-parity.test.ts index 7b27424d4..f0de50e98 100644 --- a/test/e2e/engine-parity.test.ts +++ b/test/e2e/engine-parity.test.ts @@ -588,7 +588,12 @@ describeBoth('Engine parity — relationalFanout', () => { }, 30_000); const shape = (rows: Awaited>) => - rows.map(r => `${r.source_id}:${r.slug}:${r.hop}:${r.edge_count}:${r.via_link_types.join(',')}:${r.path.join('>')}:${r.canonical_chunk_id ?? 'null'}`); + // canonical_chunk_id is a serial id — its absolute value diverges between a + // fresh PGLite engine and a shared Postgres DB whose content_chunks sequence + // advanced earlier (setupDB TRUNCATEs without RESTART IDENTITY). Compare its + // PRESENCE, not the exact id, so the parity check verifies graph structure + + // canonical-chunk resolution without depending on cross-engine sequence state. + rows.map(r => `${r.source_id}:${r.slug}:${r.hop}:${r.edge_count}:${r.via_link_types.join(',')}:${r.path.join('>')}:${r.canonical_chunk_id != null ? 'set' : 'null'}`); test('typed-edge fan-out is identical across engines', async () => { const opts = { direction: 'in' as const, linkTypes: ['invested_in'] };