From e71971cab4afd9f8bf714199a148e68579547866 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sat, 26 Sep 2026 22:45:35 -0400 Subject: [PATCH] fix(webview): route webview:changed only to its owner in multi-user mode webview:changed carried only {action, id} and the SSE routing hint had no webview: branch, so every connected client received it: in multi-user mode any user saw the ids of other users' web-tab creates, edits and deletes. The event now carries the web tab's owner (from the stored record) and is routed to that owner plus admins. Single-user delivery is unchanged. --- src/web/routes/webview-routes.ts | 18 ++- src/web/server.ts | 7 + src/web/sse-events.ts | 6 +- src/web/webview-sse.ts | 6 + test/webview-sse.test.ts | 217 +++++++++++++++++++++++++++++++ 5 files changed, 249 insertions(+), 5 deletions(-) create mode 100644 src/web/webview-sse.ts create mode 100644 test/webview-sse.test.ts diff --git a/src/web/routes/webview-routes.ts b/src/web/routes/webview-routes.ts index 16204ba1b..99dc8420b 100644 --- a/src/web/routes/webview-routes.ts +++ b/src/web/routes/webview-routes.ts @@ -165,7 +165,11 @@ function registerCrudRoutes(app: FastifyInstance, ctx: EventPort & TabLayoutPort }); throw error; } - ctx.broadcast(SseEvent.WebviewChanged, { action: 'created', id: created.id }); + ctx.broadcast(SseEvent.WebviewChanged, { + action: 'created', + id: created.id, + owner: ownerLayoutKey(created.owner), + }); return { success: true, data: created }; }); @@ -195,7 +199,11 @@ function registerCrudRoutes(app: FastifyInstance, ctx: EventPort & TabLayoutPort // Any edit invalidates the outstanding capability. Otherwise a token minted // against the OLD url keeps proxying to it after the user repointed the tab. webviewCapabilities.revokeWebview(id); - ctx.broadcast(SseEvent.WebviewChanged, { action: 'updated', id }); + ctx.broadcast(SseEvent.WebviewChanged, { + action: 'updated', + id, + owner: ownerLayoutKey(updated.owner), + }); return { success: true, data: updated }; }); @@ -231,7 +239,11 @@ function registerCrudRoutes(app: FastifyInstance, ctx: EventPort & TabLayoutPort webviewCapabilities.revokeWebview(id); socketCounts.delete(id); - ctx.broadcast(SseEvent.WebviewChanged, { action: 'deleted', id }); + ctx.broadcast(SseEvent.WebviewChanged, { + action: 'deleted', + id, + owner: ownerLayoutKey(result.owner), + }); return { success: true, data: { id } }; }); diff --git a/src/web/server.ts b/src/web/server.ts index c9de58959..d70577e67 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -96,6 +96,7 @@ import { PushSubscriptionStore } from '../push-store.js'; import webpush from 'web-push'; import { SseStreamManager } from './sse-stream-manager.js'; import { deriveTabLayoutSseHint } from './tab-layout-sse.js'; +import { deriveWebviewSseHint } from './webview-sse.js'; import { type SessionListenerRefs, createSessionListeners, @@ -2461,6 +2462,12 @@ export class WebServer extends EventEmitter { if (event.startsWith('tab:')) { return deriveTabLayoutSseHint(data); } + // Saved-webview invalidations carry the trusted resource owner. Route them to + // that owner (plus admins), so an admin editing a user's web tab notifies the + // user, and no other user learns the ids of someone else's web tabs. + if (event.startsWith('webview:')) { + return deriveWebviewSseHint(data); + } // Session-scoped families: resolve the owner from the payload's session id. const SESSION_PREFIXES = [ 'session:', diff --git a/src/web/sse-events.ts b/src/web/sse-events.ts index 6359ed4f9..f18fbf6d3 100644 --- a/src/web/sse-events.ts +++ b/src/web/sse-events.ts @@ -481,8 +481,10 @@ export const AuthPasswordChangeRequired = 'auth:passwordChangeRequired' as const export const SessionOrderChanged = 'session:orderChanged' as const; /** A saved web tab (dashboard URL) was created, updated or deleted. - * Payload: `{ action: 'created' | 'updated' | 'deleted', id }`. The client - * re-fetches the list rather than patching from the payload. */ + * Payload: `{ action: 'created' | 'updated' | 'deleted', id, owner }`. The client + * re-fetches the list rather than patching from the payload. `owner` is the web + * tab's owner (`'@single'` when multi-user mode is off); in multi-user mode the + * event is delivered only to that owner and admins (`deriveWebviewSseHint`). */ export const WebviewChanged = 'webview:changed' as const; /** Owner-scoped layout invalidation. Payload contains only `{ owner, version }`. */ export const TabLayoutChanged = 'tab:layoutChanged' as const; diff --git a/src/web/webview-sse.ts b/src/web/webview-sse.ts new file mode 100644 index 000000000..12707cc37 --- /dev/null +++ b/src/web/webview-sse.ts @@ -0,0 +1,6 @@ +/** @fileoverview Trusted owner routing metadata for saved-webview invalidations. */ +import type { SseRoutingHint } from './sse-stream-manager.js'; + +export function deriveWebviewSseHint(data: unknown): SseRoutingHint { + return { username: (data as { owner?: string }).owner, sessionScoped: true }; +} diff --git a/test/webview-sse.test.ts b/test/webview-sse.test.ts new file mode 100644 index 000000000..bcc47cf99 --- /dev/null +++ b/test/webview-sse.test.ts @@ -0,0 +1,217 @@ +/** + * @fileoverview Owner routing of the `webview:changed` SSE event. + * + * Saved web tabs are owner-scoped in multi-user mode (`canAccessOwned` on every + * CRUD route), but their invalidation event used to carry no owner and fell + * through `deriveSseHint`'s global branch, so every connected user learned the + * ids of every other user's web-tab creates, edits and deletes. The event now + * carries the resource owner and routes to that owner plus admins; single-user + * mode (no SSE identity) still delivers it to every client. + */ +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import Fastify, { type FastifyInstance, type FastifyReply } from 'fastify'; +import fastifyCookie from '@fastify/cookie'; +import fastifyWebsocket from '@fastify/websocket'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { CleanupManager } from '../src/utils/index.js'; +import type { AuthUser } from '../src/types.js'; +import { installRouteErrorHandler } from '../src/web/route-error-handler.js'; +import { registerWebviewRoutes } from '../src/web/routes/webview-routes.js'; +import { WebServer } from '../src/web/server.js'; +import { SseEvent } from '../src/web/sse-events.js'; +import { SseStreamManager, type SseRoutingHint } from '../src/web/sse-stream-manager.js'; +import { deriveWebviewSseHint } from '../src/web/webview-sse.js'; + +function client() { + const writes: string[] = []; + return { + writes, + reply: { raw: { write: (chunk: string) => (writes.push(chunk), true) } } as unknown as FastifyReply, + }; +} + +/** The server's real event → routing-hint derivation, without starting the server. */ +function serverHint(event: string, data: unknown): SseRoutingHint | undefined { + const server = new WebServer(3999, false, true) as unknown as { + deriveSseHint(event: string, data: unknown): SseRoutingHint | undefined; + }; + return server.deriveSseHint(event, data); +} + +describe('deriveWebviewSseHint', () => { + it('routes to the exact owner and fails closed when the owner is missing', () => { + expect(deriveWebviewSseHint({ action: 'updated', id: 'w1', owner: 'alice' })).toEqual({ + username: 'alice', + sessionScoped: true, + }); + expect(deriveWebviewSseHint({ action: 'updated', id: 'w1' })).toEqual({ + username: undefined, + sessionScoped: true, + }); + }); + + it('is what the server derives for the webview: family (never the global branch)', () => { + const payload = { action: 'created', id: 'w1', owner: 'alice' }; + expect(serverHint(SseEvent.WebviewChanged, payload)).toEqual({ username: 'alice', sessionScoped: true }); + expect(serverHint(SseEvent.WebviewChanged, { action: 'deleted', id: 'w1' })).not.toBeUndefined(); + }); +}); + +describe('webview:changed delivery', () => { + it('reaches the owner and admins, never another ordinary user', () => { + const cleanup = new CleanupManager(); + const manager = new SseStreamManager({ getSessionStateWithRespawn: () => null }, cleanup); + const alice = client(); + const bob = client(); + const admin = client(); + manager.addClient(alice.reply, null, false, undefined, { username: 'alice', role: 'user' }); + manager.addClient(bob.reply, null, false, undefined, { username: 'bob', role: 'user' }); + manager.addClient(admin.reply, null, false, undefined, { username: 'root', role: 'admin' }); + const payload = { action: 'deleted', id: 'w1', owner: 'alice' }; + + manager.broadcast(SseEvent.WebviewChanged, payload, serverHint(SseEvent.WebviewChanged, payload)); + + expect(alice.writes).toEqual(['event: webview:changed\ndata: {"action":"deleted","id":"w1","owner":"alice"}\n\n']); + expect(admin.writes).toEqual(alice.writes); + expect(bob.writes).toEqual([]); + cleanup.dispose(); + }); + + it('single-user mode: clients without an identity all still receive it', () => { + const cleanup = new CleanupManager(); + const manager = new SseStreamManager({ getSessionStateWithRespawn: () => null }, cleanup); + const tabA = client(); + const tabB = client(); + manager.addClient(tabA.reply, null, false, undefined, undefined); + manager.addClient(tabB.reply, null, false, undefined, undefined); + const payload = { action: 'created', id: 'w1', owner: '@single' }; + + manager.broadcast(SseEvent.WebviewChanged, payload, serverHint(SseEvent.WebviewChanged, payload)); + + expect(tabA.writes).toHaveLength(1); + expect(tabB.writes).toEqual(tabA.writes); + cleanup.dispose(); + }); +}); + +describe('webview routes → SSE, end to end', () => { + let tmpDir: string; + let savedDataDir: string | undefined; + let savedMode: string | undefined; + let cleanup: CleanupManager; + let manager: SseStreamManager; + const apps: FastifyInstance[] = []; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'codeman-webview-sse-')); + savedDataDir = process.env.CODEMAN_DATA_DIR; + savedMode = process.env.CODEMAN_MULTIUSER; + process.env.CODEMAN_DATA_DIR = tmpDir; + cleanup = new CleanupManager(); + manager = new SseStreamManager({ getSessionStateWithRespawn: () => null }, cleanup); + }); + + afterEach(async () => { + for (const app of apps.splice(0)) await app.close(); + cleanup.dispose(); + if (savedDataDir === undefined) delete process.env.CODEMAN_DATA_DIR; + else process.env.CODEMAN_DATA_DIR = savedDataDir; + if (savedMode === undefined) delete process.env.CODEMAN_MULTIUSER; + else process.env.CODEMAN_MULTIUSER = savedMode; + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}); + }); + + /** A route app acting as `authUser`, whose broadcasts go through the server's routing. */ + async function appAs(authUser: AuthUser | undefined): Promise { + const app = Fastify({ logger: false }); + await app.register(fastifyCookie); + await app.register(fastifyWebsocket); + app.decorateRequest('authUser', undefined); + app.addHook('onRequest', async (req) => { + req.authUser = authUser; + }); + registerWebviewRoutes(app, { + broadcast: (event: string, data: unknown) => manager.broadcast(event, data, serverHint(event, data)), + tabLayouts: { webviewCreated: async () => {}, webviewDeleted: async () => {} }, + } as never); + installRouteErrorHandler(app); + await app.ready(); + apps.push(app); + return app; + } + + it("multi-user: another user's SSE stream never sees a web-tab create, edit or delete", async () => { + process.env.CODEMAN_MULTIUSER = '1'; + const aliceApp = await appAs({ username: 'alice', role: 'user' }); + const alice = client(); + const bob = client(); + const admin = client(); + manager.addClient(alice.reply, null, false, undefined, { username: 'alice', role: 'user' }); + manager.addClient(bob.reply, null, false, undefined, { username: 'bob', role: 'user' }); + manager.addClient(admin.reply, null, false, undefined, { username: 'root', role: 'admin' }); + + const created = await aliceApp.inject({ + method: 'POST', + url: '/api/webviews', + payload: { name: 'Grafana', url: 'http://127.0.0.1:4000/' }, + }); + expect(created.statusCode).toBe(200); + const id = created.json().data.id as string; + const patched = await aliceApp.inject({ method: 'PATCH', url: `/api/webviews/${id}`, payload: { name: 'G2' } }); + expect(patched.statusCode).toBe(200); + expect((await aliceApp.inject({ method: 'DELETE', url: `/api/webviews/${id}` })).statusCode).toBe(200); + + const expected = ['created', 'updated', 'deleted'].map( + (action) => `event: webview:changed\ndata: ${JSON.stringify({ action, id, owner: 'alice' })}\n\n` + ); + expect(alice.writes).toEqual(expected); + expect(admin.writes).toEqual(expected); + expect(bob.writes).toEqual([]); + }); + + it("multi-user: an admin editing a user's web tab notifies that user, not a bystander", async () => { + process.env.CODEMAN_MULTIUSER = '1'; + const aliceApp = await appAs({ username: 'alice', role: 'user' }); + const adminApp = await appAs({ username: 'root', role: 'admin' }); + const id = ( + await aliceApp.inject({ + method: 'POST', + url: '/api/webviews', + payload: { name: 'G', url: 'http://127.0.0.1:4000/' }, + }) + ).json().data.id as string; + const alice = client(); + const bob = client(); + manager.addClient(alice.reply, null, false, undefined, { username: 'alice', role: 'user' }); + manager.addClient(bob.reply, null, false, undefined, { username: 'bob', role: 'user' }); + + expect((await adminApp.inject({ method: 'DELETE', url: `/api/webviews/${id}` })).statusCode).toBe(200); + + expect(alice.writes).toEqual([ + `event: webview:changed\ndata: ${JSON.stringify({ action: 'deleted', id, owner: 'alice' })}\n\n`, + ]); + expect(bob.writes).toEqual([]); + }); + + it('single-user: every client still receives the event', async () => { + const soloApp = await appAs(undefined); + const tabA = client(); + const tabB = client(); + manager.addClient(tabA.reply, null, false, undefined, undefined); + manager.addClient(tabB.reply, null, false, undefined, undefined); + + const created = await soloApp.inject({ + method: 'POST', + url: '/api/webviews', + payload: { name: 'G', url: 'http://127.0.0.1:4000/' }, + }); + expect(created.statusCode).toBe(200); + + expect(tabA.writes).toEqual([ + `event: webview:changed\ndata: ${JSON.stringify({ action: 'created', id: created.json().data.id, owner: '@single' })}\n\n`, + ]); + expect(tabB.writes).toEqual(tabA.writes); + }); +});