diff --git a/packages/host/config/schema/1792100613068_schema.sql b/packages/host/config/schema/1792193305492_schema.sql similarity index 100% rename from packages/host/config/schema/1792100613068_schema.sql rename to packages/host/config/schema/1792193305492_schema.sql 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 new file mode 100644 index 00000000000..2b5ef6308d9 --- /dev/null +++ b/packages/postgres/migrations/1792193305492_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 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. +// +// `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..b2a1132e445 --- /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', + '1792193305492_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'); + }); + }); + }); +});