From 8b40cfca90f50d670159f1956f1757d01bf591ab Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:06:52 +0100 Subject: [PATCH] fix: update the plugin frame origins in the CSP right after an install, also on other instances #1066 --- .../__tests__/csp-frame-origins-cache.test.ts | 69 ++++++++++++++ .../csp-frame-origins-shared.test.ts | 93 +++++++++++++++++++ lib/admin/csp-frame-origins.ts | 48 +++++++--- 3 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 lib/admin/__tests__/csp-frame-origins-cache.test.ts create mode 100644 lib/admin/__tests__/csp-frame-origins-shared.test.ts diff --git a/lib/admin/__tests__/csp-frame-origins-cache.test.ts b/lib/admin/__tests__/csp-frame-origins-cache.test.ts new file mode 100644 index 000000000..f975b6536 --- /dev/null +++ b/lib/admin/__tests__/csp-frame-origins-cache.test.ts @@ -0,0 +1,69 @@ +// @vitest-environment node +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +// The proxy builds frame-src from the plugins' frame origins and caches +// them for 5 s. It has its own copy of csp-frame-origins.ts, so clearing +// the cache from an install route used to clear only the route's copy: the +// proxy kept the old frame-src until its 5 s ran out. The cache is one per +// process now, and an install, update or removal clears it at once. + +vi.mock('../third-party-mode', () => ({ isNoThirdPartyTrafficOn: vi.fn(async () => false) })); +vi.mock('../plugin-dev', () => ({ listDevPlugins: vi.fn(async () => []) })); + +const PLUGIN = { + id: 'video-embed', name: 'Video', version: '1.0.0', author: 'a', description: '', type: 'hook', + permissions: [], entrypoint: 'index.js', enabled: true, installedAt: 'x', updatedAt: 'x', +}; + +let dir: string; + +beforeEach(async () => { + dir = await mkdtemp(path.join(tmpdir(), 'bw-csp-cache-')); + vi.stubEnv('ADMIN_CONFIG_DIR', dir); + delete (globalThis as Record)[Symbol.for('bulwark.admin.cspFrameOrigins')]; +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + await rm(dir, { recursive: true, force: true }); +}); + +/** A separate copy of the modules, as each bundle entry (proxy, route) has. */ +async function moduleCopy() { + vi.resetModules(); + return { + origins: await import('../csp-frame-origins'), + registry: await import('../plugin-registry'), + }; +} + +async function writeRegistry(frameOrigins: string[]) { + await mkdir(path.join(dir, 'plugins'), { recursive: true }); + await writeFile(path.join(dir, 'plugins', 'registry.json'), JSON.stringify({ plugins: [{ ...PLUGIN, frameOrigins }] })); +} + +describe('plugin frame origins cache', () => { + it('is cleared in the proxy copy when a route clears it', async () => { + const proxyCopy = await moduleCopy(); + const routeCopy = await moduleCopy(); + await writeRegistry(['https://video.example.com']); + expect(await proxyCopy.origins.getEnabledPluginFrameOrigins()).toEqual(['https://video.example.com']); + + await writeRegistry(['https://other.example.com']); + routeCopy.origins.invalidateFrameOriginsCache(); + expect(await proxyCopy.origins.getEnabledPluginFrameOrigins()).toEqual(['https://other.example.com']); + }); + + it('is cleared in the proxy copy when a plugin is switched off', async () => { + const proxyCopy = await moduleCopy(); + const routeCopy = await moduleCopy(); + await routeCopy.registry.savePlugin({ ...PLUGIN, frameOrigins: ['https://video.example.com'] }, 'code()'); + expect(await proxyCopy.origins.getEnabledPluginFrameOrigins()).toEqual(['https://video.example.com']); + + await routeCopy.registry.updatePluginMeta(PLUGIN.id, { enabled: false }); + expect(await proxyCopy.origins.getEnabledPluginFrameOrigins()).toEqual([]); + }); +}); diff --git a/lib/admin/__tests__/csp-frame-origins-shared.test.ts b/lib/admin/__tests__/csp-frame-origins-shared.test.ts new file mode 100644 index 000000000..975f511ac --- /dev/null +++ b/lib/admin/__tests__/csp-frame-origins-shared.test.ts @@ -0,0 +1,93 @@ +// @vitest-environment node +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { NextRequest } from 'next/server'; + +// Several replicas share one store (#1066): after a plugin with frame +// origins is installed through another replica, this replica's proxy must +// put them into frame-src on the next change poll, not only after its +// 5-second cache runs out. + +vi.mock('@/lib/auth/instance-details', () => ({ + canSeeInstanceDetails: async () => true, + signedInIdentity: async () => ({ admin: true }), +})); +vi.mock('@/lib/admin/session', () => ({ + requireAdminAuth: async () => ({ payload: { role: 'admin' } }), + getClientIP: () => '127.0.0.1', +})); +vi.mock('@/lib/admin/audit', () => ({ auditLog: async () => {} })); +vi.mock('@/lib/admin/plugin-dev', () => ({ listDevPlugins: vi.fn(async () => []) })); +vi.mock('@/lib/wopi/discovery', () => ({ getWopiEditorOrigins: vi.fn(async () => []) })); +vi.mock('next-intl/middleware', async () => { + const { NextResponse } = await import('next/server'); + return { default: () => () => NextResponse.next() }; +}); + +import { configManager } from '../config-manager'; +import { adminConfigDocs } from '../admin-store'; +import { pluginDocs } from '../extension-store'; +import { setBootState, setStoreForTests } from '@/lib/shared-store'; +import { resetLocalStoreForTests } from '@/lib/shared-store/local'; +import { INIT_EPOCH, markEpochsSeen, pollChanges, resetChangesForTests } from '@/lib/shared-store/changes'; +import { separateFakeStore } from './shared-store-fake'; + +const PLUGIN = { + id: 'video-embed', name: 'Video', version: '1.0.0', author: 'a', description: '', type: 'hook', + permissions: [], entrypoint: 'index.js', enabled: true, installedAt: 'x', updatedAt: 'x', +}; + +let dir: string; +let store: ReturnType; + +const registryWith = (frameOrigins: string[]) => JSON.stringify({ plugins: [{ ...PLUGIN, frameOrigins }] }); + +beforeEach(async () => { + dir = await mkdtemp(path.join(tmpdir(), 'bw-csp-shared-')); + vi.stubEnv('ADMIN_CONFIG_DIR', path.join(dir, 'config')); + vi.stubEnv('ADMIN_STATE_DIR', path.join(dir, 'state')); + resetLocalStoreForTests(); + resetChangesForTests(); + delete (globalThis as Record)[Symbol.for('bulwark.admin.cspFrameOrigins')]; + store = separateFakeStore(path.join(dir, 'store')); + setStoreForTests(store); + await store.epochs.bump(INIT_EPOCH); + await adminConfigDocs().put('config.json', JSON.stringify({ setupComplete: true })); + await adminConfigDocs().put('policy.json', JSON.stringify({ features: { pluginsEnabled: true } })); + await pluginDocs().put('registry.json', registryWith(['https://first.example.com'])); + await markEpochsSeen(); + await configManager.load(); + setBootState('ready'); +}); + +afterEach(async () => { + setStoreForTests(null); + resetChangesForTests(); + resetLocalStoreForTests(); + vi.unstubAllEnvs(); + await rm(dir, { recursive: true, force: true }); +}); + +describe('plugin frame origins with a shared store', () => { + it('reach the proxy within one poll after an install on another replica', async () => { + // The proxy's own module copies, loaded now (as at process start). + vi.resetModules(); + const { proxy } = await import('@/proxy'); + const frameSrc = async () => { + const res = await proxy(new NextRequest('http://localhost:3000/')); + return /frame-src [^;]*/.exec(res.headers.get('content-security-policy') ?? '')?.[0] ?? ''; + }; + expect(await frameSrc()).toContain('https://first.example.com'); + + // Another replica installs: the store changes and the epoch moves. + await pluginDocs().put('registry.json', registryWith(['https://second.example.com'])); + await store.epochs.bump('plugins'); + await pollChanges(); + + const after = await frameSrc(); + expect(after).toContain('https://second.example.com'); + expect(after).not.toContain('https://first.example.com'); + }); +}); diff --git a/lib/admin/csp-frame-origins.ts b/lib/admin/csp-frame-origins.ts index d3a8e6548..f80b9b1c7 100644 --- a/lib/admin/csp-frame-origins.ts +++ b/lib/admin/csp-frame-origins.ts @@ -12,6 +12,7 @@ import { getPluginRegistry } from './plugin-registry'; import { sanitizeFrameOrigins } from '../extension-origins'; import { isNoThirdPartyTrafficOn } from './third-party-mode'; +import { onSharedChange } from '@/lib/shared-store/changes'; export { isValidFrameOrigin, @@ -25,10 +26,27 @@ export { // registry JSON every time is fine but cheap to skip when nothing has // changed. Five seconds is short enough to make plugin install/uninstall // feel snappy without measurable overhead. -let cachedAt = 0; -let cachedOrigins: string[] = []; +// +// On globalThis: the proxy has its own copy of this module, and an install +// handled by a route must clear the proxy's cache too, not only the route's. +// Dropped at once when the plugins (or the config and policy that gate +// them) change, on this replica or another one (#1066). +interface FrameOriginsCache { + at: number; + origins: string[]; + /** Bumped on every invalidation, so a read that started before can't store stale origins. */ + generation: number; +} +const CACHE_KEY = Symbol.for('bulwark.admin.cspFrameOrigins'); const CACHE_TTL_MS = 5_000; +function cache(): FrameOriginsCache { + const g = globalThis as typeof globalThis & { [CACHE_KEY]?: FrameOriginsCache }; + return (g[CACHE_KEY] ??= { at: 0, origins: [], generation: 0 }); +} + +for (const area of ['plugins', 'config', 'policy']) onSharedChange(area, () => invalidateFrameOriginsCache()); + /** * Returns the union of frame origins declared by every enabled plugin in * the server-side registry, deduped and validated. @@ -37,14 +55,20 @@ const CACHE_TTL_MS = 5_000; * a broken registry only ever shrinks the CSP - never widens it. */ export async function getEnabledPluginFrameOrigins(): Promise { + const c = cache(); const now = Date.now(); - if (now - cachedAt < CACHE_TTL_MS) return cachedOrigins; + if (now - c.at < CACHE_TTL_MS) return c.origins; + const generation = c.generation; + const remember = (origins: string[]) => { + if (c.generation !== generation) return; + c.origins = origins; + c.at = now; + }; try { // No outside frames at all while the server stays off outside hosts (#903). if (await isNoThirdPartyTrafficOn()) { - cachedOrigins = []; - cachedAt = now; + remember([]); return []; } const registry = await getPluginRegistry(); @@ -64,18 +88,18 @@ export async function getEnabledPluginFrameOrigins(): Promise { out.push(o); } } - cachedOrigins = out; - cachedAt = now; + remember(out); return out; } catch { - cachedOrigins = []; - cachedAt = now; + remember([]); return []; } } -/** Force the next call to re-read the registry. Used by install/uninstall. */ +/** Force the next call (in every module copy) to re-read the registry. Used by install/uninstall. */ export function invalidateFrameOriginsCache(): void { - cachedAt = 0; - cachedOrigins = []; + const c = cache(); + c.at = 0; + c.origins = []; + c.generation++; }