mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-26 11:49:16 +00:00
fix(remote): harden WS stream lifecycle, auth precedence, and proxy error handling
- Destroy Docker stats stream on WS close to prevent orphaned daemon polling - Guard all ws.send() calls with readyState === OPEN check - Add .catch() to unawaited streamStats/execContainer calls to prevent unhandled rejections crashing the process (Node >= 15) - Close per-connection WebSocket.Server instances after handleUpgrade to prevent listener accumulation over many connections - Invert auth token precedence to bearerToken || cookieToken in both authMiddleware and the WS upgrade handler so node-to-node Bearer tokens are never shadowed by a stale browser cookie - Narrow proxyRes type in remoteNodeProxy error handler before calling .status() to avoid throwing on raw Socket (WS/TCP-level proxy errors)
This commit is contained in:
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
## [Unreleased]
|
||||
- **Fixed:** LogViewer (container SSE log stream) returning 404 on remote nodes — the `?nodeId=` query param was forwarded by `remoteNodeProxy` to the remote server, where `nodeContextMiddleware` rejected it with `Node X not found` because the gateway's node IDs don't exist on the remote instance. Fixed by stripping `nodeId` from `proxyReq.path` in `onProxyReq`, mirroring the existing `x-node-id` header removal.
|
||||
- **Fixed:** Terminal logs and container stats WebSockets failing with "HTTP Authentication failed" on remote nodes — the WebSocket upgrade proxy was forwarding the browser's `cookie` header to the remote Sencho instance. The remote's `authMiddleware` picks `cookieToken` before `bearerToken`, and the cookie (signed with the local JWT secret) fails verification on the remote, returning 401. Fixed by deleting the `cookie` header before `wsProxyServer.ws()`, mirroring the `proxyReq.removeHeader('cookie')` already present in the HTTP proxy. Also strips the gateway's `nodeId` query param from the forwarded URL so the remote defaults cleanly to its own local node.
|
||||
- **Fixed:** `streamStats` Docker stats stream leaking after WebSocket client disconnect — the Docker daemon stream was never destroyed when the WS closed, causing orphaned streams to poll the daemon indefinitely. Fixed by adding a `ws.on('close')` handler that calls `stats.destroy()`. Also guards all `ws.send()` calls with a `readyState === OPEN` check to prevent silent errors on a closed socket.
|
||||
- **Fixed:** `streamStats` and `execContainer` called unawaited in the WS `connection` handler — unhandled promise rejections (e.g., invalid container ID, Docker daemon unavailable) would terminate the Node.js process in production (Node ≥ 15). Fixed by chaining `.catch()` on both calls, logging the error, and closing the WebSocket cleanly.
|
||||
- **Fixed:** Per-connection `WebSocket.Server` instances for stack logs and host console never closed after upgrade — each WS connection allocated a persistent `ws.Server` object accumulating listeners. Fixed by calling `wss.close()` immediately after `handleUpgrade` completes.
|
||||
- **Fixed:** `authMiddleware` and WS upgrade handler evaluated `cookieToken || bearerToken` — a browser cookie present alongside a valid Bearer token would shadow it, risking 401 on node-to-node proxy calls. Fixed by inverting precedence to `bearerToken || cookieToken` in both places.
|
||||
- **Fixed:** `remoteNodeProxy` error handler unsafely cast `proxyRes` to `Response` — on WebSocket or TCP-level proxy errors `proxyRes` is a raw `Socket`, so calling `.status()` would throw. Fixed by type-narrowing before sending the 502 response.
|
||||
- **Fixed:** Container stats (CPU/RAM/NET), bash exec terminal, and "Open App" button all broken for remote nodes — stats and exec WebSockets connected to the bare root (`ws://host`) with no `?nodeId=` query param, so the upgrade handler couldn't detect the remote node and skipped the WS proxy. Moved all generic WebSockets to `/ws?nodeId=` path; upgrade handler now correctly proxies them to the remote Sencho instance.
|
||||
- **Fixed:** Bash exec and stats WebSockets not reaching the backend in `npm run dev` — Vite proxy config now includes `ws: true` on `/api` and a new `/ws` proxy entry so all WebSocket upgrades are forwarded to `localhost:3000`.
|
||||
- **Fixed:** Backend WS message handler crashing when a proxied WebSocket arrives from a gateway and the forwarded `nodeId` doesn't exist in the remote instance's DB — now falls back to the default local node instead of throwing.
|
||||
|
||||
+26
-7
@@ -122,13 +122,15 @@ wsProxyServer.on('error', (err, _req, socket: any) => {
|
||||
});
|
||||
|
||||
// Authentication Middleware
|
||||
// Accepts both cookie auth (browser sessions) and Bearer token auth (Sencho-to-Sencho proxy)
|
||||
// Accepts both cookie auth (browser sessions) and Bearer token auth (Sencho-to-Sencho proxy).
|
||||
// Bearer token is evaluated first: node-to-node proxy calls always carry a Bearer token and
|
||||
// should never be shadowed by a stale or cross-instance cookie.
|
||||
const authMiddleware = async (req: Request, res: Response, next: NextFunction): Promise<void> => {
|
||||
const cookieToken = req.cookies[COOKIE_NAME];
|
||||
const bearerToken = req.headers.authorization?.startsWith('Bearer ')
|
||||
? req.headers.authorization.slice(7)
|
||||
: null;
|
||||
const token = cookieToken || bearerToken;
|
||||
const token = bearerToken || cookieToken;
|
||||
|
||||
if (!token) {
|
||||
res.status(401).json({ error: 'Authentication required' });
|
||||
@@ -371,8 +373,12 @@ const remoteNodeProxy = createProxyMiddleware<Request, Response>({
|
||||
},
|
||||
error: (err, _req, proxyRes) => {
|
||||
console.error('[Proxy] Remote node error:', (err as Error).message);
|
||||
if (!(proxyRes as Response).headersSent) {
|
||||
(proxyRes as Response).status(502).json({
|
||||
// proxyRes can be either a ServerResponse (HTTP) or a raw Socket (WS/TCP errors).
|
||||
// Only attempt to send an HTTP 502 if it is a proper ServerResponse with a
|
||||
// headersSent flag — otherwise silently drop (the socket will be destroyed).
|
||||
const res = proxyRes as any;
|
||||
if (typeof res?.headersSent === 'boolean' && !res.headersSent && typeof res.status === 'function') {
|
||||
res.status(502).json({
|
||||
error: 'Remote node is unreachable. Check the API URL and ensure Sencho is running on that host.'
|
||||
});
|
||||
}
|
||||
@@ -424,7 +430,9 @@ server.on('upgrade', async (req, socket, head) => {
|
||||
const cookieToken = cookies[COOKIE_NAME];
|
||||
const authHeader = req.headers['authorization'] as string | undefined;
|
||||
const bearerToken = authHeader?.startsWith('Bearer ') ? authHeader.slice(7) : null;
|
||||
const token = cookieToken || bearerToken;
|
||||
// Prefer Bearer over cookie: node-to-node proxy upgrades carry a Bearer token and must
|
||||
// not be shadowed by a browser cookie signed with a different instance's JWT secret.
|
||||
const token = bearerToken || cookieToken;
|
||||
|
||||
if (!token) {
|
||||
socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n');
|
||||
@@ -473,6 +481,10 @@ server.on('upgrade', async (req, socket, head) => {
|
||||
// Dedicated stack logs WebSocket - uses Supervisor loop for persistent logs
|
||||
const logsWss = new WebSocket.Server({ noServer: true });
|
||||
logsWss.handleUpgrade(req, socket, head, (ws) => {
|
||||
// Close the per-connection server immediately after the upgrade is complete.
|
||||
// The wss instance is only needed to negotiate the handshake; keeping it open
|
||||
// would accumulate listeners and allocate memory for every connection.
|
||||
logsWss.close();
|
||||
const stackName = decodeURIComponent(logsMatch[1]);
|
||||
try {
|
||||
ComposeService.getInstance(nodeId).streamLogs(stackName, ws);
|
||||
@@ -486,6 +498,7 @@ server.on('upgrade', async (req, socket, head) => {
|
||||
} else if (hostConsoleMatch) {
|
||||
const hostConsoleWss = new WebSocket.Server({ noServer: true });
|
||||
hostConsoleWss.handleUpgrade(req, socket, head, (ws) => {
|
||||
hostConsoleWss.close();
|
||||
let targetDirectory = '';
|
||||
try {
|
||||
targetDirectory = FileSystemService.getInstance(nodeId).getBaseDir();
|
||||
@@ -539,14 +552,20 @@ wss.on('connection', (ws) => {
|
||||
// message belongs to the gateway's DB and won't resolve locally. Fall back to local.
|
||||
let nodeId = requestedId;
|
||||
try { NodeRegistry.getInstance().getDocker(requestedId); } catch { nodeId = NodeRegistry.getInstance().getDefaultNodeId(); }
|
||||
DockerController.getInstance(nodeId).streamStats(data.containerId, ws);
|
||||
DockerController.getInstance(nodeId).streamStats(data.containerId, ws).catch((err: Error) => {
|
||||
console.error('[WS] streamStats error:', err.message);
|
||||
if (ws.readyState === WebSocket.OPEN) ws.close();
|
||||
});
|
||||
} else if (data.action === 'execContainer') {
|
||||
// Handle container exec for bash access
|
||||
// Input, resize, and cleanup are handled inside execContainer's closure
|
||||
const requestedId = data.nodeId ? parseInt(data.nodeId, 10) : NodeRegistry.getInstance().getDefaultNodeId();
|
||||
let nodeId = requestedId;
|
||||
try { NodeRegistry.getInstance().getDocker(requestedId); } catch { nodeId = NodeRegistry.getInstance().getDefaultNodeId(); }
|
||||
DockerController.getInstance(nodeId).execContainer(data.containerId, ws);
|
||||
DockerController.getInstance(nodeId).execContainer(data.containerId, ws).catch((err: Error) => {
|
||||
console.error('[WS] execContainer error:', err.message);
|
||||
if (ws.readyState === WebSocket.OPEN) ws.close();
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
// Malformed JSON - ignore silently
|
||||
|
||||
@@ -425,15 +425,27 @@ class DockerController {
|
||||
const stats = await container.stats({ stream: true });
|
||||
|
||||
stats.on('data', (chunk: Buffer) => {
|
||||
ws.send(chunk.toString());
|
||||
if (ws.readyState === WebSocket.OPEN) {
|
||||
ws.send(chunk.toString());
|
||||
}
|
||||
});
|
||||
|
||||
stats.on('error', (err: Error) => {
|
||||
ws.send(JSON.stringify({ error: err.message }));
|
||||
if (ws.readyState === WebSocket.OPEN) {
|
||||
ws.send(JSON.stringify({ error: err.message }));
|
||||
}
|
||||
});
|
||||
|
||||
stats.on('end', () => {
|
||||
ws.send(JSON.stringify({ end: true }));
|
||||
if (ws.readyState === WebSocket.OPEN) {
|
||||
ws.send(JSON.stringify({ end: true }));
|
||||
}
|
||||
});
|
||||
|
||||
// Destroy the Docker stats stream when the WebSocket closes to prevent
|
||||
// orphaned streams polling the daemon after client disconnect.
|
||||
ws.on('close', () => {
|
||||
try { (stats as any).destroy(); } catch { /* stream already ended */ }
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user