From ce988724c100e9d77bf45d39d97cefcb56a84df6 Mon Sep 17 00:00:00 2001 From: UNITRONIX <36471318+UNITRONIX@users.noreply.github.com> Date: Wed, 22 Jul 2026 18:21:18 +0200 Subject: [PATCH] fix(console): share one WebSocket upgrade dispatcher (#295) Collapse 11 per-service upgrade listeners into wsUpgradeRouter so Node no longer emits a false MaxListenersExceededWarning at panel startup. --- CHANGELOG.md | 3 + web-nodejs/server.js | 6 + web-nodejs/services/bdRelay.js | 37 ++--- web-nodejs/services/cdapMediaProxy.js | 75 +++++----- web-nodejs/services/cdapTerminalProxy.js | 73 +++++----- web-nodejs/services/chatRelay.js | 54 ++++---- web-nodejs/services/deviceStatusPush.js | 34 ++--- web-nodejs/services/meshAshxProxy.js | 92 +++++++------ web-nodejs/services/remoteRelay.js | 96 ++++++------- web-nodejs/services/serverTerminalProxy.js | 52 +++---- web-nodejs/services/wsRelay.js | 151 ++++++++++----------- web-nodejs/services/wsUpgradeRouter.js | 85 ++++++++++++ web-nodejs/tests/wsUpgradeRouter.test.js | 95 +++++++++++++ 13 files changed, 535 insertions(+), 318 deletions(-) create mode 100644 web-nodejs/services/wsUpgradeRouter.js create mode 100644 web-nodejs/tests/wsUpgradeRouter.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d21594..046f58b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## [Unreleased] +### Fixed +- **False MaxListenersExceededWarning on panel WebSocket upgrades (#295):** console WebSocket services now share a single HTTP `upgrade` dispatcher instead of stacking 11 separate listeners (Node's default max is 10). Reconnects never added listeners — the warning was a startup false positive, not a reconnect leak. Session `MemoryStore` remains intentional for the single-process console (shared store is for future multi-instance HA only). + ### Changed - _(none yet)_ diff --git a/web-nodejs/server.js b/web-nodejs/server.js index da913bc1..4c799bf3 100644 --- a/web-nodejs/server.js +++ b/web-nodejs/server.js @@ -107,12 +107,18 @@ app.use(cookieParser()); // Session management — also kept as a standalone middleware ref for WebSocket upgrades // Use a different cookie name in HTTP mode to avoid collision with stale // Secure cookies left over from a previous HTTPS configuration (Issue #82). +// +// MemoryStore is intentional for the single-process console (GitHub #295). +// express-session warns in production that MemoryStore is not for multi-process +// or HA; BetterDesk runs one Node panel per host. Shared store (PostgreSQL/Redis) +// is planned only for multi-instance HA — see docs/enterprise/IMPLEMENTATION_PLAN.md. const SESSION_COOKIE = config.httpsEnabled ? 'betterdesk.sid' : 'bd.sid'; const sessionMiddleware = session({ secret: config.sessionSecret, name: SESSION_COOKIE, resave: false, saveUninitialized: false, + store: new session.MemoryStore(), cookie: { secure: config.httpsEnabled, httpOnly: true, diff --git a/web-nodejs/services/bdRelay.js b/web-nodejs/services/bdRelay.js index d5abec12..aea82a82 100644 --- a/web-nodejs/services/bdRelay.js +++ b/web-nodejs/services/bdRelay.js @@ -208,26 +208,29 @@ function initBdRelay(server) { const relayWss = new WebSocket.Server({ noServer: true, maxPayload: MAX_FRAME_BYTES }); const signalWss = new WebSocket.Server({ noServer: true, maxPayload: 64 * 1024 }); const { enforceOrigin } = require('../middleware/wsOrigin'); + const { registerUpgradeHandler } = require('./wsUpgradeRouter'); - // Attach to server upgrade — only handle /ws/bd-relay and /ws/bd-signal, - // let other handlers (remoteRelay, chatRelay, cdap) handle their paths. - server.on('upgrade', (request, socket, head) => { - const url = new URL(request.url, `http://${request.headers.host}`); - const pathname = url.pathname; + // Shared upgrade router — paths /ws/bd-relay and /ws/bd-signal (#295). + registerUpgradeHandler( + server, + (pathname) => pathname === '/ws/bd-relay' || pathname === '/ws/bd-signal', + (request, socket, head) => { + const url = new URL(request.url, `http://${request.headers.host}`); + const pathname = url.pathname; - if (pathname === '/ws/bd-relay') { - if (!enforceOrigin(request, socket, `bd-relay ${pathname}`)) return; - relayWss.handleUpgrade(request, socket, head, (ws) => { - relayWss.emit('connection', ws, request); - }); - } else if (pathname === '/ws/bd-signal') { - if (!enforceOrigin(request, socket, `bd-signal ${pathname}`)) return; - signalWss.handleUpgrade(request, socket, head, (ws) => { - signalWss.emit('connection', ws, request); - }); + if (pathname === '/ws/bd-relay') { + if (!enforceOrigin(request, socket, `bd-relay ${pathname}`)) return; + relayWss.handleUpgrade(request, socket, head, (ws) => { + relayWss.emit('connection', ws, request); + }); + } else if (pathname === '/ws/bd-signal') { + if (!enforceOrigin(request, socket, `bd-signal ${pathname}`)) return; + signalWss.handleUpgrade(request, socket, head, (ws) => { + signalWss.emit('connection', ws, request); + }); + } } - // Other paths: do nothing — let other upgrade handlers deal with them - }); + ); // ---- Relay connections ---- diff --git a/web-nodejs/services/cdapMediaProxy.js b/web-nodejs/services/cdapMediaProxy.js index 664b5ed5..69572127 100644 --- a/web-nodejs/services/cdapMediaProxy.js +++ b/web-nodejs/services/cdapMediaProxy.js @@ -41,51 +41,56 @@ function createCdapMediaProxy(server, sessionMiddleware, opts) { const wss = new WebSocket.Server({ noServer: true }); const { enforceOrigin } = require('../middleware/wsOrigin'); + const { registerUpgradeHandler } = require('./wsUpgradeRouter'); - server.on('upgrade', (req, socket, head) => { - const url = new URL(req.url, `http://${req.headers.host}`); - const match = url.pathname.match(pattern); - if (!match) return; + registerUpgradeHandler( + server, + (pathname) => pattern.test(pathname), + (req, socket, head) => { + const url = new URL(req.url, `http://${req.headers.host}`); + const match = url.pathname.match(pattern); + if (!match) return; - const deviceId = match[1]; + const deviceId = match[1]; - // CSWSH protection — reject before validating session. - if (!enforceOrigin(req, socket, `cdap-${label} ${url.pathname}`)) return; + // CSWSH protection — reject before validating session. + if (!enforceOrigin(req, socket, `cdap-${label} ${url.pathname}`)) return; - sessionMiddleware(req, {}, () => { - if (!req.session || !req.session.userId) { - console.warn(`[CDAP ${label}] 401 upgrade rejected for ${url.pathname} (no session; ip=${req.socket?.remoteAddress})`); - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - socket.destroy(); - return; - } + sessionMiddleware(req, {}, () => { + if (!req.session || !req.session.userId) { + console.warn(`[CDAP ${label}] 401 upgrade rejected for ${url.pathname} (no session; ip=${req.socket?.remoteAddress})`); + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } - // Session stores the user under req.session.user; older code paths - // also write flat fields. Accept either shape. - const sessUser = req.session.user || {}; - const userRole = sessUser.role || req.session.role || ''; - const userName = sessUser.username || req.session.username || `user#${req.session.userId}`; + // Session stores the user under req.session.user; older code paths + // also write flat fields. Accept either shape. + const sessUser = req.session.user || {}; + const userRole = sessUser.role || req.session.role || ''; + const userName = sessUser.username || req.session.username || `user#${req.session.userId}`; - const userLevel = roleLevel[userRole] || 0; - const requiredLevel = roleLevel[minRole] || 3; - if (userLevel < requiredLevel) { - console.warn(`[CDAP ${label}] 403 upgrade rejected for ${url.pathname} (user=${userName} role=${userRole} level=${userLevel} < required=${requiredLevel})`); - socket.write('HTTP/1.1 403 Forbidden\r\n\r\n'); - socket.destroy(); - return; - } + const userLevel = roleLevel[userRole] || 0; + const requiredLevel = roleLevel[minRole] || 3; + if (userLevel < requiredLevel) { + console.warn(`[CDAP ${label}] 403 upgrade rejected for ${url.pathname} (user=${userName} role=${userRole} level=${userLevel} < required=${requiredLevel})`); + socket.write('HTTP/1.1 403 Forbidden\r\n\r\n'); + socket.destroy(); + return; + } - console.log(`[CDAP ${label}] Upgrade accepted for device=${deviceId} user=${userName} role=${userRole}`); + console.log(`[CDAP ${label}] Upgrade accepted for device=${deviceId} user=${userName} role=${userRole}`); - // Attach normalized fields so the connection handler can use them. - req._cdapUserName = userName; - req._cdapUserRole = userRole; + // Attach normalized fields so the connection handler can use them. + req._cdapUserName = userName; + req._cdapUserRole = userRole; - wss.handleUpgrade(req, socket, head, (ws) => { - wss.emit('connection', ws, req, deviceId); + wss.handleUpgrade(req, socket, head, (ws) => { + wss.emit('connection', ws, req, deviceId); + }); }); - }); - }); + } + ); wss.on('connection', (browserWs, req, deviceId) => { const username = req._cdapUserName || req.session?.user?.username || req.session?.username || 'admin'; diff --git a/web-nodejs/services/cdapTerminalProxy.js b/web-nodejs/services/cdapTerminalProxy.js index 9300082f..d5e96c63 100644 --- a/web-nodejs/services/cdapTerminalProxy.js +++ b/web-nodejs/services/cdapTerminalProxy.js @@ -18,50 +18,55 @@ const config = require('../config/config'); function initCdapTerminalProxy(server, sessionMiddleware) { const wss = new WebSocket.Server({ noServer: true }); const { enforceOrigin } = require('../middleware/wsOrigin'); + const { registerUpgradeHandler } = require('./wsUpgradeRouter'); - server.on('upgrade', (req, socket, head) => { - const url = new URL(req.url, `http://${req.headers.host}`); - const pathname = url.pathname; + registerUpgradeHandler( + server, + (pathname) => /^\/api\/cdap\/devices\/[A-Za-z0-9_-]{6,30}\/terminal$/.test(pathname), + (req, socket, head) => { + const url = new URL(req.url, `http://${req.headers.host}`); + const pathname = url.pathname; - // Match /api/cdap/devices/:id/terminal - const match = pathname.match(/^\/api\/cdap\/devices\/([A-Za-z0-9_-]{6,30})\/terminal$/); - if (!match) return; // Let other upgrade handlers deal with it + // Match /api/cdap/devices/:id/terminal + const match = pathname.match(/^\/api\/cdap\/devices\/([A-Za-z0-9_-]{6,30})\/terminal$/); + if (!match) return; - const deviceId = match[1]; + const deviceId = match[1]; - // CSWSH protection — reject before validating session. - if (!enforceOrigin(req, socket, `cdap-terminal ${pathname}`)) return; + // CSWSH protection — reject before validating session. + if (!enforceOrigin(req, socket, `cdap-terminal ${pathname}`)) return; - // Require session authentication - sessionMiddleware(req, {}, () => { - if (!req.session || !req.session.userId) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - socket.destroy(); - return; - } + // Require session authentication + sessionMiddleware(req, {}, () => { + if (!req.session || !req.session.userId) { + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } - // Session may store user under req.session.user (object) or as - // flat fields. Accept either; treat super_admin/admin as admin. - const sessUser = req.session.user || {}; - const userRole = sessUser.role || req.session.role || ''; - const userName = sessUser.username || req.session.username || `user#${req.session.userId}`; + // Session may store user under req.session.user (object) or as + // flat fields. Accept either; treat super_admin/admin as admin. + const sessUser = req.session.user || {}; + const userRole = sessUser.role || req.session.role || ''; + const userName = sessUser.username || req.session.username || `user#${req.session.userId}`; - // RBAC: only admin / super_admin users can access terminal - if (userRole !== 'admin' && userRole !== 'super_admin') { - console.warn(`[CDAP Terminal] 403 upgrade rejected (user=${userName} role=${userRole})`); - socket.write('HTTP/1.1 403 Forbidden\r\n\r\n'); - socket.destroy(); - return; - } + // RBAC: only admin / super_admin users can access terminal + if (userRole !== 'admin' && userRole !== 'super_admin') { + console.warn(`[CDAP Terminal] 403 upgrade rejected (user=${userName} role=${userRole})`); + socket.write('HTTP/1.1 403 Forbidden\r\n\r\n'); + socket.destroy(); + return; + } - req._cdapUserName = userName; - req._cdapUserRole = userRole; + req._cdapUserName = userName; + req._cdapUserRole = userRole; - wss.handleUpgrade(req, socket, head, (ws) => { - wss.emit('connection', ws, req, deviceId); + wss.handleUpgrade(req, socket, head, (ws) => { + wss.emit('connection', ws, req, deviceId); + }); }); - }); - }); + } + ); wss.on('connection', (browserWs, req, deviceId) => { const username = req._cdapUserName || req.session?.user?.username || 'admin'; diff --git a/web-nodejs/services/chatRelay.js b/web-nodejs/services/chatRelay.js index fe1dd999..de696d84 100644 --- a/web-nodejs/services/chatRelay.js +++ b/web-nodejs/services/chatRelay.js @@ -496,36 +496,40 @@ function initChatRelay(server, sessionMiddleware, betterdeskApi) { const wss = new WebSocket.Server({ noServer: true }); const { enforceOrigin } = require('../middleware/wsOrigin'); + const { registerUpgradeHandler } = require('./wsUpgradeRouter'); - server.on('upgrade', (req, socket, head) => { - const url = new URL(req.url, `http://${req.headers.host}`); - const pathname = url.pathname; + registerUpgradeHandler( + server, + (pathname) => /^\/ws\/chat\/[^/]+$/.test(pathname) || /^\/ws\/chat-operator\/[^/]+$/.test(pathname), + (req, socket, head) => { + const url = new URL(req.url, `http://${req.headers.host}`); + const pathname = url.pathname; - const agentMatch = pathname.match(/^\/ws\/chat\/([^/]+)$/); - if (agentMatch) { - if (!enforceOrigin(req, socket, `chat-agent ${pathname}`)) return; - wss.handleUpgrade(req, socket, head, (ws) => { - wss.emit('connection', ws, req, 'agent', agentMatch[1]); - }); - return; - } - - const opMatch = pathname.match(/^\/ws\/chat-operator\/([^/]+)$/); - if (opMatch) { - if (!enforceOrigin(req, socket, `chat-operator ${pathname}`)) return; - sessionMiddleware(req, {}, () => { - if (!req.session || !req.session.userId) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - socket.destroy(); - return; - } + const agentMatch = pathname.match(/^\/ws\/chat\/([^/]+)$/); + if (agentMatch) { + if (!enforceOrigin(req, socket, `chat-agent ${pathname}`)) return; wss.handleUpgrade(req, socket, head, (ws) => { - wss.emit('connection', ws, req, 'operator', opMatch[1]); + wss.emit('connection', ws, req, 'agent', agentMatch[1]); }); - }); - return; + return; + } + + const opMatch = pathname.match(/^\/ws\/chat-operator\/([^/]+)$/); + if (opMatch) { + if (!enforceOrigin(req, socket, `chat-operator ${pathname}`)) return; + sessionMiddleware(req, {}, () => { + if (!req.session || !req.session.userId) { + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } + wss.handleUpgrade(req, socket, head, (ws) => { + wss.emit('connection', ws, req, 'operator', opMatch[1]); + }); + }); + } } - }); + ); wss.on('connection', (ws, req, role, deviceId) => { if (role === 'agent') { diff --git a/web-nodejs/services/deviceStatusPush.js b/web-nodejs/services/deviceStatusPush.js index 5f5c6428..fee9dca5 100644 --- a/web-nodejs/services/deviceStatusPush.js +++ b/web-nodejs/services/deviceStatusPush.js @@ -34,24 +34,26 @@ function initDeviceStatusPush(httpServer, sessionMiddleware, goApiUrl, apiKey) { // Browser-facing WebSocket server const wss = new WebSocket.Server({ noServer: true }); const clients = new Set(); + const { registerUpgradeHandler } = require('./wsUpgradeRouter'); - // Handle upgrade requests - httpServer.on('upgrade', (req, socket, head) => { - const url = new URL(req.url, `http://${req.headers.host}`); - if (url.pathname !== '/ws/device-status') return; - - // Authenticate via session - sessionMiddleware(req, {}, () => { - if (!req.session || !req.session.userId) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - socket.destroy(); - return; - } - wss.handleUpgrade(req, socket, head, (ws) => { - wss.emit('connection', ws, req); + // Handle upgrade requests via shared router (#295) + registerUpgradeHandler( + httpServer, + (pathname) => pathname === '/ws/device-status', + (req, socket, head) => { + // Authenticate via session + sessionMiddleware(req, {}, () => { + if (!req.session || !req.session.userId) { + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } + wss.handleUpgrade(req, socket, head, (ws) => { + wss.emit('connection', ws, req); + }); }); - }); - }); + } + ); wss.on('connection', (ws) => { clients.add(ws); diff --git a/web-nodejs/services/meshAshxProxy.js b/web-nodejs/services/meshAshxProxy.js index 1436ca60..9eea3a44 100644 --- a/web-nodejs/services/meshAshxProxy.js +++ b/web-nodejs/services/meshAshxProxy.js @@ -6,6 +6,7 @@ const WebSocket = require('ws'); const config = require('../config/config'); const { enforceOrigin } = require('../middleware/wsOrigin'); +const { registerUpgradeHandler } = require('./wsUpgradeRouter'); const MESH_PATHS = new Set([ '/agent.ashx', @@ -21,60 +22,63 @@ function goWsBase() { function initMeshAshxProxy(server, sessionMiddleware) { const wss = new WebSocket.Server({ noServer: true }); - server.on('upgrade', (req, socket, head) => { - const url = new URL(req.url, `http://${req.headers.host}`); - if (!MESH_PATHS.has(url.pathname)) return; + registerUpgradeHandler( + server, + (pathname) => MESH_PATHS.has(pathname), + (req, socket, head) => { + const url = new URL(req.url, `http://${req.headers.host}`); - const needsSession = url.pathname === '/control.ashx'; - const label = `mesh-${url.pathname}`; + const needsSession = url.pathname === '/control.ashx'; + const label = `mesh-${url.pathname}`; - if (!enforceOrigin(req, socket, label)) return; + if (!enforceOrigin(req, socket, label)) return; - const connect = () => { - wss.handleUpgrade(req, socket, head, (browserWs) => { - const target = goWsBase() + url.pathname + (url.search || ''); - const goWs = new WebSocket(target, { - headers: { - 'x-forwarded-for': req.socket?.remoteAddress || '', - }, - }); - - goWs.on('open', () => { - browserWs.on('message', (data, isBinary) => { - if (goWs.readyState === WebSocket.OPEN) { - goWs.send(data, { binary: isBinary }); - } + const connect = () => { + wss.handleUpgrade(req, socket, head, (browserWs) => { + const target = goWsBase() + url.pathname + (url.search || ''); + const goWs = new WebSocket(target, { + headers: { + 'x-forwarded-for': req.socket?.remoteAddress || '', + }, }); - goWs.on('message', (data, isBinary) => { - if (browserWs.readyState === WebSocket.OPEN) { - browserWs.send(data, { binary: isBinary }); - } + + goWs.on('open', () => { + browserWs.on('message', (data, isBinary) => { + if (goWs.readyState === WebSocket.OPEN) { + goWs.send(data, { binary: isBinary }); + } + }); + goWs.on('message', (data, isBinary) => { + if (browserWs.readyState === WebSocket.OPEN) { + browserWs.send(data, { binary: isBinary }); + } + }); }); - }); - goWs.on('error', (err) => { - console.warn('[mesh proxy]', url.pathname, err.message); - browserWs.close(); + goWs.on('error', (err) => { + console.warn('[mesh proxy]', url.pathname, err.message); + browserWs.close(); + }); + browserWs.on('close', () => goWs.close()); + goWs.on('close', () => browserWs.close()); + browserWs.on('error', () => goWs.close()); }); - browserWs.on('close', () => goWs.close()); - goWs.on('close', () => browserWs.close()); - browserWs.on('error', () => goWs.close()); - }); - }; + }; - if (needsSession) { - sessionMiddleware(req, {}, () => { - if (!req.session || !req.session.userId) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - socket.destroy(); - return; - } + if (needsSession) { + sessionMiddleware(req, {}, () => { + if (!req.session || !req.session.userId) { + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } + connect(); + }); + } else { connect(); - }); - } else { - connect(); + } } - }); + ); } module.exports = { initMeshAshxProxy }; diff --git a/web-nodejs/services/remoteRelay.js b/web-nodejs/services/remoteRelay.js index 31473662..5922a12b 100644 --- a/web-nodejs/services/remoteRelay.js +++ b/web-nodejs/services/remoteRelay.js @@ -283,59 +283,63 @@ function handleViewerConnection(ws, deviceId, operatorName) { function initRemoteRelay(server, sessionMiddleware) { const wss = new WebSocket.Server({ noServer: true }); const { enforceOrigin } = require('../middleware/wsOrigin'); + const { registerUpgradeHandler } = require('./wsUpgradeRouter'); - server.on('upgrade', (req, socket, head) => { - const url = new URL(req.url, `http://${req.headers.host}`); - const path = url.pathname; + registerUpgradeHandler( + server, + (path) => /^\/ws\/remote-agent\/[^/]+$/.test(path) || /^\/ws\/remote-viewer\/[^/]+$/.test(path), + (req, socket, head) => { + const url = new URL(req.url, `http://${req.headers.host}`); + const path = url.pathname; - // Agent: /ws/remote-agent/ - const agentMatch = path.match(/^\/ws\/remote-agent\/([^/]+)$/); - if (agentMatch) { - if (!enforceOrigin(req, socket, `remote-agent ${path}`)) return; - const deviceId = decodeURIComponent(agentMatch[1]); - const token = url.searchParams.get('token') || ''; - // Validate device ID format (reject path traversal etc.) - if (!/^[A-Za-z0-9_-]{3,64}$/.test(deviceId)) { - socket.write('HTTP/1.1 400 Bad Request\r\n\r\n'); - socket.destroy(); + // Agent: /ws/remote-agent/ + const agentMatch = path.match(/^\/ws\/remote-agent\/([^/]+)$/); + if (agentMatch) { + if (!enforceOrigin(req, socket, `remote-agent ${path}`)) return; + const deviceId = decodeURIComponent(agentMatch[1]); + const token = url.searchParams.get('token') || ''; + // Validate device ID format (reject path traversal etc.) + if (!/^[A-Za-z0-9_-]{3,64}$/.test(deviceId)) { + socket.write('HTTP/1.1 400 Bad Request\r\n\r\n'); + socket.destroy(); + return; + } + verifyAgentConnection(deviceId, token).then((ok) => { + if (!ok) { + socket.write('HTTP/1.1 403 Forbidden\r\n\r\n'); + socket.destroy(); + return; + } + wss.handleUpgrade(req, socket, head, (ws) => { + handleAgentConnection(ws, deviceId); + }); + }).catch(() => { + try { + socket.write('HTTP/1.1 500 Internal Server Error\r\n\r\n'); + socket.destroy(); + } catch (_) { /* closed */ } + }); return; } - verifyAgentConnection(deviceId, token).then((ok) => { - if (!ok) { - socket.write('HTTP/1.1 403 Forbidden\r\n\r\n'); - socket.destroy(); - return; - } - wss.handleUpgrade(req, socket, head, (ws) => { - handleAgentConnection(ws, deviceId); - }); - }).catch(() => { - try { - socket.write('HTTP/1.1 500 Internal Server Error\r\n\r\n'); - socket.destroy(); - } catch (_) { /* closed */ } - }); - return; - } - // Viewer (operator): /ws/remote-viewer/ - const viewerMatch = path.match(/^\/ws\/remote-viewer\/([^/]+)$/); - if (viewerMatch) { - if (!enforceOrigin(req, socket, `remote-viewer ${path}`)) return; - sessionMiddleware(req, {}, () => { - if (!req.session || !req.session.userId) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - socket.destroy(); - return; - } - wss.handleUpgrade(req, socket, head, (ws) => { - const opName = req.session.username || 'operator'; - handleViewerConnection(ws, viewerMatch[1], opName); + // Viewer (operator): /ws/remote-viewer/ + const viewerMatch = path.match(/^\/ws\/remote-viewer\/([^/]+)$/); + if (viewerMatch) { + if (!enforceOrigin(req, socket, `remote-viewer ${path}`)) return; + sessionMiddleware(req, {}, () => { + if (!req.session || !req.session.userId) { + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } + wss.handleUpgrade(req, socket, head, (ws) => { + const opName = req.session.username || 'operator'; + handleViewerConnection(ws, viewerMatch[1], opName); + }); }); - }); - return; + } } - }); + ); log.info('Remote relay initialized (/ws/remote-agent/:id, /ws/remote-viewer/:id)'); return wss; diff --git a/web-nodejs/services/serverTerminalProxy.js b/web-nodejs/services/serverTerminalProxy.js index d4e763a9..8294dfa4 100644 --- a/web-nodejs/services/serverTerminalProxy.js +++ b/web-nodejs/services/serverTerminalProxy.js @@ -233,32 +233,34 @@ function startShell(cols, rows, userInfo) { function initServerTerminalProxy(server, sessionMiddleware, opts) { const wss = new WebSocket.Server({ noServer: true }); const audit = opts && typeof opts.logAction === 'function' ? opts.logAction : null; + const { registerUpgradeHandler } = require('./wsUpgradeRouter'); - server.on('upgrade', (req, socket, head) => { - const url = new URL(req.url, `http://${req.headers.host}`); - if (url.pathname !== '/ws/server-management/terminal') return; - - sessionMiddleware(req, {}, () => { - if (!req.session || !req.session.userId) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - return socket.destroy(); - } - const sessUser = req.session.user || {}; - const role = sessUser.role || req.session.role || ''; - const username = sessUser.username || `user#${req.session.userId}`; - // RBAC: only super_admin / admin / server_admin - if (!(role === 'super_admin' || role === 'admin' || role === 'server_admin')) { - console.warn(`[srv-term] 403 upgrade rejected (user=${username} role=${role})`); - socket.write('HTTP/1.1 403 Forbidden\r\n\r\n'); - return socket.destroy(); - } - req._smUserName = username; - req._smUserRole = role; - req._smUserId = req.session.userId; - req._smIp = (req.headers['x-forwarded-for'] || req.socket.remoteAddress || '').split(',')[0].trim(); - wss.handleUpgrade(req, socket, head, (ws) => wss.emit('connection', ws, req)); - }); - }); + registerUpgradeHandler( + server, + (pathname) => pathname === '/ws/server-management/terminal', + (req, socket, head) => { + sessionMiddleware(req, {}, () => { + if (!req.session || !req.session.userId) { + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + return socket.destroy(); + } + const sessUser = req.session.user || {}; + const role = sessUser.role || req.session.role || ''; + const username = sessUser.username || `user#${req.session.userId}`; + // RBAC: only super_admin / admin / server_admin + if (!(role === 'super_admin' || role === 'admin' || role === 'server_admin')) { + console.warn(`[srv-term] 403 upgrade rejected (user=${username} role=${role})`); + socket.write('HTTP/1.1 403 Forbidden\r\n\r\n'); + return socket.destroy(); + } + req._smUserName = username; + req._smUserRole = role; + req._smUserId = req.session.userId; + req._smIp = (req.headers['x-forwarded-for'] || req.socket.remoteAddress || '').split(',')[0].trim(); + wss.handleUpgrade(req, socket, head, (ws) => wss.emit('connection', ws, req)); + }); + } + ); wss.on('connection', (ws, req) => { const sessionId = makeSessionId(); diff --git a/web-nodejs/services/wsRelay.js b/web-nodejs/services/wsRelay.js index 51dd6d7d..7119d4bf 100644 --- a/web-nodejs/services/wsRelay.js +++ b/web-nodejs/services/wsRelay.js @@ -16,6 +16,7 @@ const net = require('net'); const os = require('os'); const config = require('../config/config'); const { enforceOrigin } = require('../middleware/wsOrigin'); +const { registerUpgradeHandler } = require('./wsUpgradeRouter'); // Maximum concurrent relay connections per IP const MAX_CONNECTIONS_PER_IP = 5; @@ -65,88 +66,86 @@ function initWsProxy(server, sessionMiddleware) { // Relay proxy (hbbr) const relayWss = new WebSocket.Server({ noServer: true }); - // Handle upgrade requests — verify session cookie before allowing WebSocket - // Only handles /ws/rendezvous and /ws/relay; other paths are left for - // downstream handlers (chatRelay, remoteRelay, cdapProxy, etc.) - server.on('upgrade', (request, socket, head) => { - const url = new URL(request.url, `http://${request.headers.host}`); - const pathname = url.pathname; + // Handle upgrade requests — verify session cookie before allowing WebSocket. + // Paths owned: /ws/rendezvous, /ws/relay (shared upgrade router — #295). + registerUpgradeHandler( + server, + (pathname) => pathname === '/ws/rendezvous' || pathname === '/ws/relay', + (request, socket, head) => { + const url = new URL(request.url, `http://${request.headers.host}`); + const pathname = url.pathname; - // Only handle paths this proxy owns - if (pathname !== '/ws/rendezvous' && pathname !== '/ws/relay') { - return; // let other upgrade handlers deal with it - } + // CSWSH protection: reject cross-origin upgrades before touching session + if (!enforceOrigin(request, socket, `ws-proxy ${pathname}`)) return; - // CSWSH protection: reject cross-origin upgrades before touching session - if (!enforceOrigin(request, socket, `ws-proxy ${pathname}`)) return; - - // Validate the session against the real Express session store. - // Using sessionMiddleware (from server.js) populates req.session, which - // we then check for an authenticated userId. This replaces the old - // cookie-name-only check that could be bypassed with a fake cookie. - if (typeof sessionMiddleware !== 'function') { - console.warn('WS proxy: sessionMiddleware not provided — rejecting upgrade'); - socket.write('HTTP/1.1 503 Service Unavailable\r\n\r\n'); - socket.destroy(); - return; - } - - // Attach a minimal fake response so session middleware can call next() - const fakeRes = Object.create(null); - fakeRes.getHeader = () => undefined; - fakeRes.setHeader = () => {}; - fakeRes.end = () => {}; - fakeRes.on = () => {}; - - sessionMiddleware(request, fakeRes, () => { - const hasUser = request.session && request.session.userId; - let hasGuest = false; - if (!hasUser) { - try { - // Prefer ?guest= on WS URL (session pages always append it for guests) - const g = url.searchParams.get('guest') || url.searchParams.get('t'); - if (g) { - hasGuest = true; - request.guestToken = g; - } - if (!hasGuest) { - const { GUEST_COOKIE } = require('../middleware/guestAccess'); - const raw = request.headers.cookie || ''; - const names = [GUEST_COOKIE, 'bd.guest', 'betterdesk.guest']; - for (const cookieName of names) { - const match = raw.split(';').map((p) => p.trim()).find((p) => p.startsWith(cookieName + '=')); - if (match) { - const val = decodeURIComponent(match.slice(cookieName.length + 1) || ''); - if (val) { - hasGuest = true; - request.guestToken = val; - break; - } - } - } - } - } catch { - hasGuest = false; - } - } - if (!hasUser && !hasGuest) { - console.warn(`WS proxy: Rejected upgrade to ${pathname} — no authenticated session (ip: ${request.socket?.remoteAddress})`); - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + // Validate the session against the real Express session store. + // Using sessionMiddleware (from server.js) populates req.session, which + // we then check for an authenticated userId. This replaces the old + // cookie-name-only check that could be bypassed with a fake cookie. + if (typeof sessionMiddleware !== 'function') { + console.warn('WS proxy: sessionMiddleware not provided — rejecting upgrade'); + socket.write('HTTP/1.1 503 Service Unavailable\r\n\r\n'); socket.destroy(); return; } - if (pathname === '/ws/rendezvous') { - rendezvousWss.handleUpgrade(request, socket, head, (ws) => { - rendezvousWss.emit('connection', ws, request); - }); - } else { - relayWss.handleUpgrade(request, socket, head, (ws) => { - relayWss.emit('connection', ws, request); - }); - } - }); - }); // server.on('upgrade') + // Attach a minimal fake response so session middleware can call next() + const fakeRes = Object.create(null); + fakeRes.getHeader = () => undefined; + fakeRes.setHeader = () => {}; + fakeRes.end = () => {}; + fakeRes.on = () => {}; + + sessionMiddleware(request, fakeRes, () => { + const hasUser = request.session && request.session.userId; + let hasGuest = false; + if (!hasUser) { + try { + // Prefer ?guest= on WS URL (session pages always append it for guests) + const g = url.searchParams.get('guest') || url.searchParams.get('t'); + if (g) { + hasGuest = true; + request.guestToken = g; + } + if (!hasGuest) { + const { GUEST_COOKIE } = require('../middleware/guestAccess'); + const raw = request.headers.cookie || ''; + const names = [GUEST_COOKIE, 'bd.guest', 'betterdesk.guest']; + for (const cookieName of names) { + const match = raw.split(';').map((p) => p.trim()).find((p) => p.startsWith(cookieName + '=')); + if (match) { + const val = decodeURIComponent(match.slice(cookieName.length + 1) || ''); + if (val) { + hasGuest = true; + request.guestToken = val; + break; + } + } + } + } + } catch { + hasGuest = false; + } + } + if (!hasUser && !hasGuest) { + console.warn(`WS proxy: Rejected upgrade to ${pathname} — no authenticated session (ip: ${request.socket?.remoteAddress})`); + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } + + if (pathname === '/ws/rendezvous') { + rendezvousWss.handleUpgrade(request, socket, head, (ws) => { + rendezvousWss.emit('connection', ws, request); + }); + } else { + relayWss.handleUpgrade(request, socket, head, (ws) => { + relayWss.emit('connection', ws, request); + }); + } + }); + } + ); // Parse target host/port from config const hbbsHost = config.wsProxy?.hbbsHost || 'localhost'; diff --git a/web-nodejs/services/wsUpgradeRouter.js b/web-nodejs/services/wsUpgradeRouter.js new file mode 100644 index 00000000..c94e273a --- /dev/null +++ b/web-nodejs/services/wsUpgradeRouter.js @@ -0,0 +1,85 @@ +/** + * Shared WebSocket upgrade router for the BetterDesk console HTTP server. + * + * Previously each WS service called server.on('upgrade'), which stacked 11 + * listeners and triggered Node's MaxListenersExceededWarning (default max=10). + * Reconnects never added listeners — the warning was a false-positive leak + * signal (GitHub #295). This module attaches exactly one upgrade listener per + * server and dispatches by pathname. + */ + +'use strict'; + +/** @type {WeakMap }>} */ +const routers = new WeakMap(); + +/** + * Ensure a single upgrade dispatcher is attached to the given server. + * @param {import('http').Server|import('events').EventEmitter} server + */ +function ensureRouter(server) { + let state = routers.get(server); + if (state) return state; + + state = { routes: [] }; + routers.set(server, state); + + server.on('upgrade', (req, socket, head) => { + let pathname; + try { + pathname = new URL(req.url, `http://${req.headers.host || 'localhost'}`).pathname; + } catch { + try { + socket.write('HTTP/1.1 400 Bad Request\r\n\r\n'); + } catch (_) { /* closed */ } + try { socket.destroy(); } catch (_) { /* closed */ } + return; + } + + for (const route of state.routes) { + if (route.match(pathname, req)) { + route.handle(req, socket, head); + return; + } + } + // No registered route — leave the socket alone (same as the old + // multi-listener chain when every handler early-returned). + }); + + return state; +} + +/** + * Register a WebSocket upgrade handler on the shared per-server router. + * Idempotent with respect to the HTTP listener: only one server.on('upgrade') + * is ever attached per server instance. + * + * @param {import('http').Server|import('events').EventEmitter} server + * @param {(pathname: string, req: import('http').IncomingMessage) => boolean} match + * @param {(req: import('http').IncomingMessage, socket: import('net').Socket, head: Buffer) => void} handle + */ +function registerUpgradeHandler(server, match, handle) { + if (!server || typeof server.on !== 'function') { + throw new TypeError('registerUpgradeHandler: server must be an EventEmitter'); + } + if (typeof match !== 'function' || typeof handle !== 'function') { + throw new TypeError('registerUpgradeHandler: match and handle must be functions'); + } + const state = ensureRouter(server); + state.routes.push({ match, handle }); +} + +/** + * Number of routes registered for this server (for tests / diagnostics). + * @param {object} server + * @returns {number} + */ +function registeredRouteCount(server) { + const state = routers.get(server); + return state ? state.routes.length : 0; +} + +module.exports = { + registerUpgradeHandler, + registeredRouteCount, +}; diff --git a/web-nodejs/tests/wsUpgradeRouter.test.js b/web-nodejs/tests/wsUpgradeRouter.test.js new file mode 100644 index 00000000..264c11d1 --- /dev/null +++ b/web-nodejs/tests/wsUpgradeRouter.test.js @@ -0,0 +1,95 @@ +/** + * Shared WS upgrade router — prevents MaxListenersExceededWarning (#295). + * + * All panel WebSocket services must register via registerUpgradeHandler so the + * HTTP server has exactly one 'upgrade' listener regardless of how many routes + * are registered. + */ + +'use strict'; + +const http = require('http'); +const { EventEmitter } = require('events'); + +describe('wsUpgradeRouter', () => { + beforeEach(() => { + jest.resetModules(); + }); + + test('attaches exactly one upgrade listener for many route registrations', () => { + const { registerUpgradeHandler, registeredRouteCount } = require('../services/wsUpgradeRouter'); + const server = new EventEmitter(); + + for (let i = 0; i < 11; i += 1) { + const path = `/ws/route-${i}`; + registerUpgradeHandler(server, (pathname) => pathname === path, () => {}); + } + + expect(server.listenerCount('upgrade')).toBe(1); + expect(registeredRouteCount(server)).toBe(11); + }); + + test('dispatches to the matching route only', () => { + const { registerUpgradeHandler } = require('../services/wsUpgradeRouter'); + const server = new EventEmitter(); + const hits = []; + + registerUpgradeHandler(server, (p) => p === '/ws/a', () => { hits.push('a'); }); + registerUpgradeHandler(server, (p) => p === '/ws/b', () => { hits.push('b'); }); + + const socket = { write: jest.fn(), destroy: jest.fn() }; + server.emit('upgrade', { url: '/ws/b', headers: { host: 'localhost' } }, socket, Buffer.alloc(0)); + + expect(hits).toEqual(['b']); + expect(socket.write).not.toHaveBeenCalled(); + }); + + test('leaves socket alone when no route matches', () => { + const { registerUpgradeHandler } = require('../services/wsUpgradeRouter'); + const server = new EventEmitter(); + registerUpgradeHandler(server, (p) => p === '/ws/owned', () => {}); + + const socket = { write: jest.fn(), destroy: jest.fn() }; + server.emit('upgrade', { url: '/ws/other', headers: { host: 'localhost' } }, socket, Buffer.alloc(0)); + + expect(socket.write).not.toHaveBeenCalled(); + expect(socket.destroy).not.toHaveBeenCalled(); + }); + + test('production WS inits share a single upgrade listener on one HTTP server', () => { + const sessionStub = (req, _res, next) => { + req.session = { userId: 1 }; + next(); + }; + + const { initWsProxy } = require('../services/wsRelay'); + const { initBdRelay } = require('../services/bdRelay'); + const { initChatRelay } = require('../services/chatRelay'); + const { initRemoteRelay } = require('../services/remoteRelay'); + const { initCdapTerminalProxy } = require('../services/cdapTerminalProxy'); + const { initCdapMediaProxies } = require('../services/cdapMediaProxy'); + const { initMeshAshxProxy } = require('../services/meshAshxProxy'); + const { registerUpgradeHandler, registeredRouteCount } = require('../services/wsUpgradeRouter'); + + const server = http.createServer((req, res) => { + res.writeHead(404); + res.end(); + }); + + initWsProxy(server, sessionStub); + initBdRelay(server); + initChatRelay(server, sessionStub, null); + initRemoteRelay(server, sessionStub); + initCdapTerminalProxy(server, sessionStub); + initCdapMediaProxies(server, sessionStub); + initMeshAshxProxy(server, sessionStub); + // deviceStatusPush uses the same registerUpgradeHandler API; register its + // path without starting the Go event-bus reconnect loop (keeps Jest clean). + registerUpgradeHandler(server, (pathname) => pathname === '/ws/device-status', () => {}); + + // 1 wsRelay + 1 bdRelay + 1 chat + 1 remote + 1 cdap terminal + // + 4 cdap media + 1 mesh + 1 device-status = 11 routes, 1 listener + expect(server.listenerCount('upgrade')).toBe(1); + expect(registeredRouteCount(server)).toBe(11); + }); +});