From d5a7c2db46c2409081e7158b3ce08f288e194721 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Fri, 16 Oct 2026 19:24:51 -0400 Subject: [PATCH 1/3] Check in a new migration that the read-only roles can't read withheld tables The migrations that create or seal proxy_endpoints, incoming_webhooks, user_keys and key_write_tickets are already recorded on staging and production, so a check added to them would never run there. This new migration asks has_column_privilege about every column of those tables for readonly_role and every role that inherits it or can SET ROLE to it (transitively), excluding each table's owner and ADMIN-only members, and raises if any can still read one through PUBLIC or another role. The schema snapshot is renamed to the new migration's timestamp; the migration changes no schema. Co-Authored-By: Claude Opus 5.5 --- ...68_schema.sql => 1792192933698_schema.sql} | 0 ...readonly-role-cant-read-withheld-tables.js | 74 +++++++ .../readonly-role-withheld-tables-test.ts | 201 ++++++++++++++++++ 3 files changed, 275 insertions(+) rename packages/host/config/schema/{1792100613068_schema.sql => 1792192933698_schema.sql} (100%) create mode 100644 packages/postgres/migrations/1792192933698_check-readonly-role-cant-read-withheld-tables.js create mode 100644 packages/realm-server/tests/readonly-role-withheld-tables-test.ts diff --git a/packages/host/config/schema/1792100613068_schema.sql b/packages/host/config/schema/1792192933698_schema.sql similarity index 100% rename from packages/host/config/schema/1792100613068_schema.sql rename to packages/host/config/schema/1792192933698_schema.sql diff --git a/packages/postgres/migrations/1792192933698_check-readonly-role-cant-read-withheld-tables.js b/packages/postgres/migrations/1792192933698_check-readonly-role-cant-read-withheld-tables.js new file mode 100644 index 00000000000..622203235da --- /dev/null +++ b/packages/postgres/migrations/1792192933698_check-readonly-role-cant-read-withheld-tables.js @@ -0,0 +1,74 @@ +// Checks that the read-only role used by dashboards and read-only database +// access, and every role that reads through it, can't read the tables withheld +// from it: `proxy_endpoints` and `incoming_webhooks`, which hold the platform's +// vendor keys and the secrets that verify incoming webhooks, and `user_keys` +// and `key_write_tickets`. The migrations that create or seal those tables +// revoke the SELECT grants made to the role and its members by name. SELECT +// granted to PUBLIC, or to another role a member also belongs to, still lets +// them read, and revoking it isn't a migration's call. So `up` asks +// `has_column_privilege` about every column of each table and stops, naming +// the role and the column, if any of those roles can still read one. +// +// The roles checked are `readonly_role` and every role that inherits its +// privileges or can SET ROLE to it, directly or through other roles. A member +// holding only the ADMIN option reads nothing through the role (on Postgres 16 +// and later the role that creates `readonly_role` holds that membership), and +// a table's owner reads it by owning it, so neither is checked. The check +// covers what each role reads as itself; a member that can SET ROLE to some +// other role reads as that role. +// +// `readonly_role` exists only where it was provisioned (staging and +// production), so the check is skipped wherever it doesn't. The migration +// changes nothing, so `down` has nothing to undo. + +exports.up = (pgm) => { + pgm.sql(` + DO $$ + DECLARE + readable_by name; + readable_table name; + readable_column name; + BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'readonly_role') THEN + WITH RECURSIVE reader (oid) AS ( + SELECT oid FROM pg_roles WHERE rolname = 'readonly_role' + UNION + SELECT membership.member + FROM pg_auth_members membership + JOIN reader ON reader.oid = membership.roleid + WHERE membership.inherit_option OR membership.set_option + ) + SELECT reader_role.rolname, tbl.relname, col.attname + INTO readable_by, readable_table, readable_column + FROM reader + JOIN pg_roles reader_role ON reader_role.oid = reader.oid + CROSS JOIN pg_class tbl + JOIN pg_attribute col ON col.attrelid = tbl.oid + WHERE tbl.oid IN ( + to_regclass('proxy_endpoints'), + to_regclass('incoming_webhooks'), + to_regclass('user_keys'), + to_regclass('key_write_tickets') + ) + AND reader.oid <> tbl.relowner + AND col.attnum > 0 + AND NOT col.attisdropped + AND has_column_privilege(reader.oid, tbl.oid, col.attnum, 'SELECT') + ORDER BY tbl.relname, reader_role.rolname, col.attnum + LIMIT 1; + IF FOUND THEN + RAISE EXCEPTION + '% can read %.%, which is withheld from readonly_role and the roles that read through it', + readable_by, readable_table, readable_column + USING HINT = format( + 'Revoke the grant or role membership that lets %I read %I.%I, then run the migration again.', + readable_by, readable_table, readable_column + ); + END IF; + END IF; + END + $$ + `); +}; + +exports.down = () => {}; diff --git a/packages/realm-server/tests/readonly-role-withheld-tables-test.ts b/packages/realm-server/tests/readonly-role-withheld-tables-test.ts new file mode 100644 index 00000000000..61d5c4ce63f --- /dev/null +++ b/packages/realm-server/tests/readonly-role-withheld-tables-test.ts @@ -0,0 +1,201 @@ +import QUnit from 'qunit'; +const { module, test } = QUnit; +import { basename, dirname, join } from 'path'; +import { createRequire } from 'module'; +import { setupDB } from './helpers/index.ts'; + +const require = createRequire(import.meta.url); +const CHECK_MIGRATION = join( + dirname(require.resolve('@cardstack/postgres/package.json')), + 'migrations', + '1792192933698_check-readonly-role-cant-read-withheld-tables.js', +); + +const WITHHELD_TABLES = [ + 'proxy_endpoints', + 'incoming_webhooks', + 'user_keys', + 'key_write_tickets', +]; + +// Roles and memberships are shared by every database on the server, so each +// test removes the memberships it makes, whatever happens. +const MEMBER = 'readonly_member_for_withheld_tables_test'; +const GROUP = 'readonly_group_for_withheld_tables_test'; +const LEAF = 'readonly_leaf_for_withheld_tables_test'; +const ADMIN_MEMBER = 'readonly_admin_member_for_withheld_tables_test'; +const OWNER = 'readonly_owner_for_withheld_tables_test'; +const OTHER_ROLE = 'other_role_for_withheld_tables_test'; +const OUTSIDER = 'outsider_for_withheld_tables_test'; + +module(basename(import.meta.filename), function () { + module('the read-only role check', function (hooks) { + let dbAdapter: any; + + setupDB(hooks, { + beforeEach: async (_dbAdapter) => { + dbAdapter = _dbAdapter; + // The role and its blanket grant as staging and production have them, + // with SELECT revoked on the withheld tables as the migrations that + // create or seal them leave it. + await createRole('readonly_role'); + await dbAdapter.execute( + `GRANT SELECT ON ALL TABLES IN SCHEMA public TO readonly_role`, + ); + await dbAdapter.execute( + `REVOKE SELECT ON TABLE ${WITHHELD_TABLES.join(', ')} FROM readonly_role`, + ); + }, + }); + + async function createRole(name: string) { + await dbAdapter.execute( + `DO $$ + BEGIN + CREATE ROLE ${name}; + EXCEPTION WHEN duplicate_object OR unique_violation THEN NULL; + END + $$`, + ); + } + + async function runUp() { + let statements: string[] = []; + require(CHECK_MIGRATION).up({ + sql: (statement: string) => statements.push(statement), + }); + for (let statement of statements) { + await dbAdapter.execute(statement); + } + } + + async function withMemberships( + memberships: [role: string, member: string, options?: string][], + fn: () => Promise, + ) { + for (let [role, member] of memberships) { + await createRole(role); + await createRole(member); + } + try { + for (let [role, member, options] of memberships) { + await dbAdapter.execute( + `GRANT ${role} TO ${member}${options ? ` WITH ${options}` : ''}`, + ); + } + await fn(); + } finally { + for (let [role, member] of [...memberships].reverse()) { + await dbAdapter.execute(`REVOKE ${role} FROM ${member}`); + } + } + } + + test('passes when only a role outside the read-only role reads the tables', async function (assert) { + await createRole(OUTSIDER); + await dbAdapter.execute( + `GRANT SELECT ON TABLE ${WITHHELD_TABLES.join(', ')} TO ${OUTSIDER}`, + ); + + await runUp(); + + assert.ok(true, 'up completes'); + }); + + test('stops when PUBLIC can read a column', async function (assert) { + await dbAdapter.execute( + `GRANT SELECT (api_key) ON TABLE proxy_endpoints TO PUBLIC`, + ); + + await assert.rejects( + runUp(), + /can read proxy_endpoints\.api_key, which is withheld from readonly_role/, + ); + }); + + test('stops when a member reads a table through another role', async function (assert) { + await withMemberships( + [ + ['readonly_role', MEMBER], + [OTHER_ROLE, MEMBER], + ], + async () => { + await dbAdapter.execute( + `GRANT SELECT ON TABLE user_keys TO ${OTHER_ROLE}`, + ); + + await assert.rejects( + runUp(), + new RegExp( + `${MEMBER} can read user_keys\\.\\w+, which is withheld`, + ), + ); + }, + ); + }); + + test('stops when a role that inherits the read-only role through a group reads a table', async function (assert) { + await withMemberships( + [ + ['readonly_role', GROUP], + [GROUP, LEAF], + [OTHER_ROLE, LEAF], + ], + async () => { + await dbAdapter.execute( + `GRANT SELECT (owner) ON TABLE key_write_tickets TO ${OTHER_ROLE}`, + ); + + await assert.rejects( + runUp(), + new RegExp( + `${LEAF} can read key_write_tickets\\.owner, which is withheld`, + ), + ); + }, + ); + }); + + test('leaves a member that holds only the ADMIN option on the role', async function (assert) { + // The role that creates the read-only role holds this membership on + // Postgres 16 and later, and reads nothing through it. + await withMemberships( + [ + [ + 'readonly_role', + ADMIN_MEMBER, + 'ADMIN TRUE, INHERIT FALSE, SET FALSE', + ], + [OTHER_ROLE, ADMIN_MEMBER], + ], + async () => { + await dbAdapter.execute( + `GRANT SELECT ON TABLE ${WITHHELD_TABLES.join(', ')} TO ${OTHER_ROLE}`, + ); + + await runUp(); + + let [{ readable }] = await dbAdapter.execute( + `SELECT has_table_privilege('${ADMIN_MEMBER}', 'proxy_endpoints', 'SELECT') AS readable`, + ); + assert.true( + readable, + 'the member still reads through the other role', + ); + }, + ); + }); + + test('leaves a member that owns a table reading it', async function (assert) { + await createRole(OWNER); + await dbAdapter.execute( + `ALTER TABLE incoming_webhooks OWNER TO ${OWNER}`, + ); + await withMemberships([['readonly_role', OWNER]], async () => { + await runUp(); + + assert.ok(true, 'up completes'); + }); + }); + }); +}); From 74add027f5dc0394609975950c9a4d5ef4e3fded Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Fri, 16 Oct 2026 19:28:37 -0400 Subject: [PATCH 2/3] Re-stamp the read-only role check to run after the grant repair The migration that repairs readonly_role's withheld-table grants revokes any SELECT a reader still holds; checking afterwards confirms none is left rather than stopping on a grant the repair would remove. The schema snapshot moves to the new timestamp with it. Co-Authored-By: Claude Opus 5.5 --- .../{1792192933698_schema.sql => 1792193305492_schema.sql} | 0 ...2193305492_check-readonly-role-cant-read-withheld-tables.js} | 0 .../realm-server/tests/readonly-role-withheld-tables-test.ts | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename packages/host/config/schema/{1792192933698_schema.sql => 1792193305492_schema.sql} (100%) rename packages/postgres/migrations/{1792192933698_check-readonly-role-cant-read-withheld-tables.js => 1792193305492_check-readonly-role-cant-read-withheld-tables.js} (100%) diff --git a/packages/host/config/schema/1792192933698_schema.sql b/packages/host/config/schema/1792193305492_schema.sql similarity index 100% rename from packages/host/config/schema/1792192933698_schema.sql rename to packages/host/config/schema/1792193305492_schema.sql diff --git a/packages/postgres/migrations/1792192933698_check-readonly-role-cant-read-withheld-tables.js b/packages/postgres/migrations/1792193305492_check-readonly-role-cant-read-withheld-tables.js similarity index 100% rename from packages/postgres/migrations/1792192933698_check-readonly-role-cant-read-withheld-tables.js rename to packages/postgres/migrations/1792193305492_check-readonly-role-cant-read-withheld-tables.js diff --git a/packages/realm-server/tests/readonly-role-withheld-tables-test.ts b/packages/realm-server/tests/readonly-role-withheld-tables-test.ts index 61d5c4ce63f..b2a1132e445 100644 --- a/packages/realm-server/tests/readonly-role-withheld-tables-test.ts +++ b/packages/realm-server/tests/readonly-role-withheld-tables-test.ts @@ -8,7 +8,7 @@ const require = createRequire(import.meta.url); const CHECK_MIGRATION = join( dirname(require.resolve('@cardstack/postgres/package.json')), 'migrations', - '1792192933698_check-readonly-role-cant-read-withheld-tables.js', + '1792193305492_check-readonly-role-cant-read-withheld-tables.js', ); const WITHHELD_TABLES = [ From 7c129d319d795bb9a731c4f514e1ad98dfb0c83a Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Mon, 19 Oct 2026 03:40:07 -0400 Subject: [PATCH 3/3] Say only a non-superuser creator holds the ADMIN-only membership A superuser that runs CREATE ROLE gets no automatic membership in the new role, so the comment justifying the exclusion states the condition exactly. Co-Authored-By: Claude Opus 5.5 --- ...92193305492_check-readonly-role-cant-read-withheld-tables.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/postgres/migrations/1792193305492_check-readonly-role-cant-read-withheld-tables.js b/packages/postgres/migrations/1792193305492_check-readonly-role-cant-read-withheld-tables.js index 622203235da..2b5ef6308d9 100644 --- a/packages/postgres/migrations/1792193305492_check-readonly-role-cant-read-withheld-tables.js +++ b/packages/postgres/migrations/1792193305492_check-readonly-role-cant-read-withheld-tables.js @@ -12,7 +12,7 @@ // The roles checked are `readonly_role` and every role that inherits its // privileges or can SET ROLE to it, directly or through other roles. A member // holding only the ADMIN option reads nothing through the role (on Postgres 16 -// and later the role that creates `readonly_role` holds that membership), and +// and later a non-superuser that creates `readonly_role` holds that membership), and // a table's owner reads it by owning it, so neither is checked. The check // covers what each role reads as itself; a member that can SET ROLE to some // other role reads as that role.