diff --git a/packages/realm-server/tests/realm-endpoints/permissions-test.ts b/packages/realm-server/tests/realm-endpoints/permissions-test.ts index 0de2589888e..b5a65c8a3a5 100644 --- a/packages/realm-server/tests/realm-endpoints/permissions-test.ts +++ b/packages/realm-server/tests/realm-endpoints/permissions-test.ts @@ -2,7 +2,10 @@ import { module, test } from 'qunit'; import type { Test, SuperTest } from 'supertest'; import { basename } from 'path'; import type { Realm } from '@cardstack/runtime-common'; -import { fetchRealmPermissions } from '@cardstack/runtime-common'; +import { + fetchRealmPermissions, + insertPermissions, +} from '@cardstack/runtime-common'; import { setupPermissionedRealmCached, testRealmHref, @@ -344,6 +347,38 @@ module(`realm-endpoints/${basename(__filename)}`, function () { ); }); + // CS-11126: realm visibility / world-readable used to be memoized + // per Realm instance, so a `*: read` grant or revoke on replica A + // would be silently ignored by every other replica until restart. + // With the caches dropped, every visibility() call re-reads + // realm_permissions, so an out-of-band DB write (which is what a + // peer's commit looks like from this replica's vantage) is + // observed on the next call. Simulating "peer write" via direct + // insertPermissions/DELETE keeps the test single-process. + test('visibility() reflects out-of-band realm_permissions changes without restart', async function (assert) { + assert.strictEqual( + await testRealm.visibility(), + 'shared', + 'baseline: realm with mary+bob (no `*`) is shared', + ); + + // peer-side grant: *: read directly in the DB + await insertPermissions(dbAdapter, testRealmURL, { '*': ['read'] }); + assert.strictEqual( + await testRealm.visibility(), + 'public', + 'after out-of-band *: read grant, visibility is public on next call', + ); + + // peer-side revoke: drop *: read + await insertPermissions(dbAdapter, testRealmURL, { '*': [] }); + assert.strictEqual( + await testRealm.visibility(), + 'shared', + 'after out-of-band *: read revoke, visibility flips back without restart', + ); + }); + test('receive 400 error on invalid JSON API', async function (assert) { let response = await request .patch('/_permissions') diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 0e19fa20744..15abb90699a 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -733,17 +733,6 @@ export class Realm { readonly __fetchForTesting: typeof globalThis.fetch; readonly paths: RealmPaths; - private visibilityPromise?: Promise; - - // We are caching world readable permissions for realms such that we can avoid - // having to look up the realm permissions for world-read realms on every HTTP - // HEAD and GET request. the tradeoff is that if there is an out-of-band - // update to the permissions that effects world readability (e.g. directly - // updating the DB), that will mean we need to restart the realm server to - // pick up the permissions change. - #worldReadable?: boolean; - #worldReadablePromise?: Promise; - get url(): string { return this.paths.url; } @@ -2364,7 +2353,6 @@ export class Realm { }); } } - await this.isWorldReadable(); this.#perfLog.debug( `realm server ${this.url} startup in ${Date.now() - startTime} ms`, @@ -5769,12 +5757,6 @@ export class Realm { } await insertPermissions(this.#dbAdapter, new URL(this.url), patch); - if (Object.prototype.hasOwnProperty.call(patch, '*')) { - let worldReadable = - Array.isArray(patch['*']) && patch['*'].includes('read'); - this.#worldReadable = worldReadable; - this.#worldReadablePromise = Promise.resolve(worldReadable); - } return await this.getRealmPermissions(request, requestContext); } @@ -6457,45 +6439,31 @@ export class Realm { ); } - private async isWorldReadable(): Promise { - if (this.#worldReadable !== undefined) { - return this.#worldReadable; - } - if (!this.#worldReadablePromise) { - this.#worldReadablePromise = (async () => { - let permissions = await fetchRealmPermissions( - this.#dbAdapter, - new URL(this.url), - ); - let worldReadable = permissions['*']?.includes('read') ?? false; - if (this.#worldReadable === undefined) { - this.#worldReadable = worldReadable; - return worldReadable; - } - return this.#worldReadable; - })(); - } - return await this.#worldReadablePromise; - } - + // CS-11126: no memoization. `realm_permissions` is indexed by + // realm_url and a permissions PATCH from a peer replica must take + // effect here without a restart, so every read-path callsite fetches + // fresh. For requests, this is one extra indexed SELECT; for + // world-readable reads `createRequestContext` derives the flag from + // the same single fetch rather than calling this helper plus a + // second fetch. private async createRequestContext( requiredPermission: RealmAction, ): Promise { - let permissions: RealmPermissions; - let shouldUseWorldReadable = - requiredPermission === 'read' && (await this.isWorldReadable()); - - if (shouldUseWorldReadable) { - permissions = { - [this.#matrixClientUserId]: ['assume-user'], - '*': ['read'], - }; - } else { - permissions = { - [this.#matrixClientUserId]: ['assume-user'], - ...(await fetchRealmPermissions(this.#dbAdapter, new URL(this.url))), - }; - } + let fetched = await fetchRealmPermissions( + this.#dbAdapter, + new URL(this.url), + ); + let isWorldReadable = fetched['*']?.includes('read') ?? false; + let permissions: RealmPermissions = + requiredPermission === 'read' && isWorldReadable + ? { + [this.#matrixClientUserId]: ['assume-user'], + '*': ['read'], + } + : { + [this.#matrixClientUserId]: ['assume-user'], + ...fetched, + }; return { realm: this, @@ -6504,31 +6472,21 @@ export class Realm { } public async visibility(): Promise { - if (this.visibilityPromise) { - return this.visibilityPromise; - } - - this.visibilityPromise = (async () => { - let permissions = await fetchRealmPermissions( - this.#dbAdapter, - new URL(this.url), - ); - - let usernames = Object.keys(permissions).filter( - (username) => !username.startsWith('@realm/'), - ); - if (usernames.includes('*')) { - return 'public'; - } else if (usernames.includes('users')) { - return 'shared'; - } else if (usernames.length > 1) { - return 'shared'; - } else { - return 'private'; - } - })(); + let permissions = await fetchRealmPermissions( + this.#dbAdapter, + new URL(this.url), + ); - return this.visibilityPromise; + let usernames = Object.keys(permissions).filter( + (username) => !username.startsWith('@realm/'), + ); + if (usernames.includes('*')) { + return 'public'; + } else if (usernames.includes('users') || usernames.length > 1) { + return 'shared'; + } else { + return 'private'; + } } #logRequestPerformance(