From 6a3bf2342c5d24798794024492572cef077a48dd Mon Sep 17 00:00:00 2001 From: shaun0927 <70629228+shaun0927@users.noreply.github.com> Date: Sun, 17 May 2026 18:58:56 +0900 Subject: [PATCH] Prevent stale HTTP session interceptors from leaking across sessions Register the HTTP session-delete lifecycle hook with the network interceptor cache so deleted streamable HTTP sessions drop their per-session rule state instead of retaining stale NetworkInterceptor instances. Constraint: HTTP transport already emits session deletion but the MCP transport abstraction did not expose that lifecycle hook. Rejected: Static import from mcp-server to network-intercept | it would introduce a stronger circular dependency because network-intercept already imports mcp-server for client resolution. Confidence: high Scope-risk: narrow Directive: Keep transport-owned lifecycle cleanup optional so stdio remains stateless and unaffected. Tested: npx jest --runTestsByPath tests/unit/network-intercept-tool.test.ts tests/unit/mcp-server-session-cleanup.test.ts --runInBand; npx eslint --max-warnings=0 src/tools/network-intercept.ts src/transports/index.ts src/mcp-server.ts tests/unit/network-intercept-tool.test.ts tests/unit/mcp-server-session-cleanup.test.ts; npx tsc --noEmit --pretty false Not-tested: live HTTP client DELETE against a running MCP server --- src/mcp-server.ts | 5 +++ src/tools/network-intercept-cache.ts | 38 +++++++++++++++++++ src/tools/network-intercept.ts | 32 ++++++---------- src/transports/index.ts | 6 +++ tests/unit/mcp-server-session-cleanup.test.ts | 36 ++++++++++++++++++ tests/unit/network-intercept-tool.test.ts | 29 +++++++++++++- 6 files changed, 125 insertions(+), 21 deletions(-) create mode 100644 src/tools/network-intercept-cache.ts create mode 100644 tests/unit/mcp-server-session-cleanup.test.ts diff --git a/src/mcp-server.ts b/src/mcp-server.ts index d31bfd95..83c8c70d 100644 --- a/src/mcp-server.ts +++ b/src/mcp-server.ts @@ -17,6 +17,7 @@ import { MCPMessageContext, } from './types/mcp'; import { createTransport, MCPTransport, TransportMode } from './transports'; +import { removeNetworkInterceptorForSession } from './tools/network-intercept-cache'; import { TOOL_TIERS, getToolTier } from './config/tool-tiers'; import { BrowserBackend } from './types/browser-backend'; import { logAuditEntry } from './security/audit-logger'; @@ -196,6 +197,10 @@ export class MCPServer { allowedOrigins: options.allowedOrigins, }); + this.transport.onSessionDelete?.((sessionId) => { + removeNetworkInterceptorForSession(sessionId); + }); + this.transport.onMessage((msg, context) => this.handleMessage(msg, context)); await this.transport.start(); diff --git a/src/tools/network-intercept-cache.ts b/src/tools/network-intercept-cache.ts new file mode 100644 index 00000000..60c0f869 --- /dev/null +++ b/src/tools/network-intercept-cache.ts @@ -0,0 +1,38 @@ +import { NetworkInterceptor } from '../network-interceptor'; + +const DEFAULT_INTERCEPTOR_SCOPE = '__default__'; +const interceptorsBySession = new Map(); + +function resolveInterceptorScope(sessionId?: string): string { + return sessionId || DEFAULT_INTERCEPTOR_SCOPE; +} + +export function getNetworkInterceptorForSession(sessionId?: string): NetworkInterceptor { + const key = resolveInterceptorScope(sessionId); + let interceptor = interceptorsBySession.get(key); + if (!interceptor) { + interceptor = new NetworkInterceptor(); + interceptorsBySession.set(key, interceptor); + } + return interceptor; +} + +export function removeNetworkInterceptorForSession(sessionId: string): number { + if (!sessionId) return 0; + + let removed = 0; + for (const key of Array.from(interceptorsBySession.keys())) { + if (key === sessionId || key.startsWith(`${sessionId}|`)) { + interceptorsBySession.delete(key); + removed += 1; + } + } + return removed; +} + +export function resetNetworkInterceptorsForTest(): void { + interceptorsBySession.clear(); +} + +/** Legacy singleton for callers that are not yet session-aware. */ +export const networkInterceptor = getNetworkInterceptorForSession(DEFAULT_INTERCEPTOR_SCOPE); diff --git a/src/tools/network-intercept.ts b/src/tools/network-intercept.ts index bf71bdac..9b6cedce 100644 --- a/src/tools/network-intercept.ts +++ b/src/tools/network-intercept.ts @@ -1,29 +1,21 @@ import { MCPServer, getWebKitClient } from '../mcp-server'; import { - NetworkInterceptor, type InterceptorClient, type InterceptRule, } from '../network-interceptor'; +import { + getNetworkInterceptorForSession, + networkInterceptor, + removeNetworkInterceptorForSession, + resetNetworkInterceptorsForTest, +} from './network-intercept-cache'; -const DEFAULT_INTERCEPTOR_SCOPE = '__default__'; -const interceptorsBySession = new Map(); - -export function getNetworkInterceptorForSession(sessionId?: string): NetworkInterceptor { - const key = sessionId || DEFAULT_INTERCEPTOR_SCOPE; - let interceptor = interceptorsBySession.get(key); - if (!interceptor) { - interceptor = new NetworkInterceptor(); - interceptorsBySession.set(key, interceptor); - } - return interceptor; -} - -export function resetNetworkInterceptorsForTest(): void { - interceptorsBySession.clear(); -} - -/** Legacy singleton for callers that are not yet session-aware. */ -export const networkInterceptor = getNetworkInterceptorForSession(DEFAULT_INTERCEPTOR_SCOPE); +export { + getNetworkInterceptorForSession, + networkInterceptor, + removeNetworkInterceptorForSession, + resetNetworkInterceptorsForTest, +}; function resolveClient(deviceId: unknown): InterceptorClient | null { return getWebKitClient(typeof deviceId === 'string' ? deviceId : undefined); diff --git a/src/transports/index.ts b/src/transports/index.ts index 1040c409..4b3a838a 100644 --- a/src/transports/index.ts +++ b/src/transports/index.ts @@ -26,6 +26,12 @@ export interface MCPTransport { */ send(response: MCPResponse): void; + /** + * Register cleanup that runs when a stateful transport deletes an MCP session. + * Stateless transports can omit this hook. + */ + onSessionDelete?(handler: (sessionId: string) => void): void; + /** Start listening for messages (bind port or attach readline). */ start(): void | Promise; diff --git a/tests/unit/mcp-server-session-cleanup.test.ts b/tests/unit/mcp-server-session-cleanup.test.ts new file mode 100644 index 00000000..7352d23f --- /dev/null +++ b/tests/unit/mcp-server-session-cleanup.test.ts @@ -0,0 +1,36 @@ +describe('MCPServer HTTP session cleanup', () => { + beforeEach(() => { + jest.resetModules(); + }); + + it('registers network interceptor eviction for transport session deletion', async () => { + const onSessionDelete = jest.fn(); + const transport = { + onMessage: jest.fn(), + onSessionDelete, + send: jest.fn(), + start: jest.fn(), + close: jest.fn(), + }; + const removeNetworkInterceptorForSession = jest.fn(); + + jest.doMock('../../src/transports', () => ({ + createTransport: jest.fn(async () => transport), + })); + jest.doMock('../../src/tools/network-intercept-cache', () => ({ + removeNetworkInterceptorForSession, + })); + + const { MCPServer } = await import('../../src/mcp-server'); + const server = new MCPServer(); + + await server.start({ transport: 'http' }); + + expect(onSessionDelete).toHaveBeenCalledWith(expect.any(Function)); + const handler = onSessionDelete.mock.calls[0][0] as (sessionId: string) => void; + handler('session-a'); + await new Promise((resolve) => setImmediate(resolve)); + + expect(removeNetworkInterceptorForSession).toHaveBeenCalledWith('session-a'); + }); +}); diff --git a/tests/unit/network-intercept-tool.test.ts b/tests/unit/network-intercept-tool.test.ts index bb2289ab..0575e11d 100644 --- a/tests/unit/network-intercept-tool.test.ts +++ b/tests/unit/network-intercept-tool.test.ts @@ -1,4 +1,8 @@ -import { getNetworkInterceptorForSession, resetNetworkInterceptorsForTest } from '../../src/tools/network-intercept'; +import { + getNetworkInterceptorForSession, + removeNetworkInterceptorForSession, + resetNetworkInterceptorsForTest, +} from '../../src/tools/network-intercept'; describe('network_intercept session scoping', () => { beforeEach(() => resetNetworkInterceptorsForTest()); @@ -26,6 +30,29 @@ describe('network_intercept session scoping', () => { expect(a2.findMatchingRule('https://example.com/login')?.action).toBe('mock'); }); + it('evicts only the selected MCP session cache on session deletion', () => { + const a1 = getNetworkInterceptorForSession('session-a'); + const b = getNetworkInterceptorForSession('session-b'); + + a1.addRule({ urlPattern: '/a', action: 'block' }); + b.addRule({ urlPattern: '/b', action: 'block' }); + + expect(removeNetworkInterceptorForSession('session-a')).toBe(1); + + const a2 = getNetworkInterceptorForSession('session-a'); + expect(a2).not.toBe(a1); + expect(a2.listRules()).toHaveLength(0); + expect(b.listRules()).toHaveLength(1); + }); + + it('reports no eviction for unknown or empty session ids', () => { + getNetworkInterceptorForSession('session-a').addRule({ urlPattern: '/a', action: 'block' }); + + expect(removeNetworkInterceptorForSession('missing')).toBe(0); + expect(removeNetworkInterceptorForSession('')).toBe(0); + expect(getNetworkInterceptorForSession('session-a').listRules()).toHaveLength(1); + }); + it('clear/disable restores and clears only the selected session', async () => { const a = getNetworkInterceptorForSession('session-a'); const b = getNetworkInterceptorForSession('session-b');