mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-06 00:47:52 +00:00
fix(deploy-progress): decouple deploys from the live progress stream (#1246)
* fix(deploy-progress): decouple deploys from the live progress stream The deploy progress modal streamed compose output over a WebSocket, but the deploy itself was coupled to that socket in two ways that could break or silently abort a deploy: - The deploy request was gated on the progress socket connecting, so any upgrade failure (a reverse proxy blocking WebSocket upgrades, or the admin-only stream rejecting a scoped deployer) left the modal stuck on "Connecting..." and the deploy never fired. - The backend terminated the running compose process when that socket closed, so minimizing the modal, navigating away, or a network blip aborted an in-flight deploy. Make the progress socket output-only: the deploy is owned by its request and runs to completion (or the existing command timeout) regardless of the stream. The modal now degrades to a "Live progress unavailable" state and still reports success or failure from the request result. Connect failures, drops, and a connect timeout all release the deploy instead of blocking it. Also route progress output per deploy: the frontend sends a correlation id on both the connectTerminal message and the deploy request header, and the backend keys progress sockets by that id so concurrent deploys from different tabs or users no longer cross-stream each other's output. Cap the in-memory parsed log rows so a very long deploy cannot grow the modal's state unbounded. * fix(deploy-progress): generate the deploy session id with a CSPRNG The per-deploy correlation id keys which WebSocket receives a deploy's live output, so a guessable id lets one authenticated client register a victim's id and read its compose output. It was built from Math.random() plus a timestamp, which is not cryptographically secure. Generate it with crypto.getRandomValues (128 bits, hex). That is the one Crypto member available in insecure contexts, so it still works over LAN HTTP where crypto.randomUUID is unavailable. * fix(deploy-progress): stop headerless ops bleeding into a keyed progress modal Address review findings on the progress-stream routing: - Only an id-less connectTerminal registration may become the id-less fallback socket. Previously every connectTerminal (including keyed deploy modals) set the fallback, so a headerless operation (bulk update, rollback, or a legacy client) resolved via getTerminalWs() into another user's keyed deploy modal. Keyed sockets are now excluded from the fallback, and a socket that adopts a session id is removed from it. - The connect-timeout fallback now also flags the modal as "Live progress unavailable" instead of leaving it on "Connecting..." while the deploy runs. - Log only a short prefix of the deploy session id in developer diagnostics, not the full capability value.
This commit is contained in:
@@ -254,22 +254,21 @@ describe('ComposeService - runCommand', () => {
|
||||
await expectation;
|
||||
});
|
||||
|
||||
it('kills and rejects running commands when the WebSocket disconnects', async () => {
|
||||
it('keeps the command running when the WebSocket disconnects (progress socket is output-only)', async () => {
|
||||
const proc = createMockProcess();
|
||||
mockSpawn.mockReturnValue(proc);
|
||||
const ws = createMockWs();
|
||||
|
||||
const svc = ComposeService.getInstance(1);
|
||||
const promise = svc.runCommand('my-stack', 'restart', ws);
|
||||
const expectation = expect(promise).rejects.toThrow('client disconnected');
|
||||
let settled = false;
|
||||
promise.finally(() => { settled = true; }).catch(() => undefined);
|
||||
// The deploy is owned by its HTTP request; closing the progress socket
|
||||
// (panel minimized, navigated away, connection blip) must not abort it.
|
||||
ws.emit('close');
|
||||
expect(proc.kill).not.toHaveBeenCalled();
|
||||
|
||||
expect(proc.kill).toHaveBeenCalledWith('SIGTERM');
|
||||
expect(settled).toBe(false);
|
||||
proc.emit('close', null);
|
||||
await expectation;
|
||||
// The command still completes on its own exit, not the socket close.
|
||||
proc.emit('close', 0);
|
||||
await expect(promise).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it('rewrites ENOMEM spawn failures as host out-of-memory', async () => {
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { EventEmitter } from 'events';
|
||||
import WebSocket, { type WebSocketServer } from 'ws';
|
||||
import { attachGenericConnectionHandlers, getTerminalWs } from '../websocket/generic';
|
||||
|
||||
class FakeWs extends EventEmitter {
|
||||
readyState: number = WebSocket.OPEN;
|
||||
}
|
||||
|
||||
/** Simulate a client opening the generic /ws socket and registering for deploy
|
||||
* output via the connectTerminal handshake. */
|
||||
function connect(wss: EventEmitter, sessionId?: string): FakeWs {
|
||||
const ws = new FakeWs();
|
||||
wss.emit('connection', ws);
|
||||
ws.emit('message', Buffer.from(JSON.stringify({ action: 'connectTerminal', ...(sessionId ? { sessionId } : {}) })));
|
||||
return ws;
|
||||
}
|
||||
|
||||
describe('generic ws terminal registry', () => {
|
||||
let wss: EventEmitter;
|
||||
beforeEach(() => {
|
||||
wss = new EventEmitter();
|
||||
attachGenericConnectionHandlers(wss as unknown as WebSocketServer);
|
||||
});
|
||||
|
||||
it('routes output to the socket matching the deploy session id', () => {
|
||||
const a = connect(wss, 'sess-a');
|
||||
const b = connect(wss, 'sess-b');
|
||||
expect(getTerminalWs('sess-a')).toBe(a);
|
||||
expect(getTerminalWs('sess-b')).toBe(b);
|
||||
expect(getTerminalWs('not-a-session')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('falls back to the most recent id-less socket', () => {
|
||||
const a = connect(wss);
|
||||
expect(getTerminalWs()).toBe(a);
|
||||
const b = connect(wss);
|
||||
expect(getTerminalWs()).toBe(b);
|
||||
});
|
||||
|
||||
it('drops a session from the registry when its socket closes', () => {
|
||||
const a = connect(wss, 'sess-x');
|
||||
expect(getTerminalWs('sess-x')).toBe(a);
|
||||
a.emit('close');
|
||||
expect(getTerminalWs('sess-x')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('ignores a registered socket that is no longer open', () => {
|
||||
const a = connect(wss, 'sess-y');
|
||||
a.readyState = WebSocket.CLOSED;
|
||||
expect(getTerminalWs('sess-y')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('rebinds a socket to a new session id and drops the old mapping', () => {
|
||||
const a = connect(wss, 'sess-1');
|
||||
// Same socket re-registers under a new id (a second deploy in the same tab).
|
||||
a.emit('message', Buffer.from(JSON.stringify({ action: 'connectTerminal', sessionId: 'sess-2' })));
|
||||
expect(getTerminalWs('sess-1')).toBeUndefined();
|
||||
expect(getTerminalWs('sess-2')).toBe(a);
|
||||
});
|
||||
|
||||
it('clears the id-less fallback when its socket closes', () => {
|
||||
const a = connect(wss);
|
||||
expect(getTerminalWs()).toBe(a);
|
||||
a.emit('close');
|
||||
expect(getTerminalWs()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not expose a keyed socket as the id-less fallback', () => {
|
||||
// A headerless operation (bulk / rollback / legacy) resolves via getTerminalWs()
|
||||
// with no id; it must never reach a keyed deploy modal's socket.
|
||||
const a = connect(wss, 'sess-keyed');
|
||||
expect(getTerminalWs('sess-keyed')).toBe(a);
|
||||
expect(getTerminalWs()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('drops the id-less fallback when that socket later adopts a session id', () => {
|
||||
const a = connect(wss);
|
||||
expect(getTerminalWs()).toBe(a);
|
||||
a.emit('message', Buffer.from(JSON.stringify({ action: 'connectTerminal', sessionId: 'sess-late' })));
|
||||
expect(getTerminalWs()).toBeUndefined();
|
||||
expect(getTerminalWs('sess-late')).toBe(a);
|
||||
});
|
||||
});
|
||||
@@ -25,7 +25,7 @@ import { sendGitSourceError } from '../utils/gitSourceHttp';
|
||||
import { buildPolicyGateOptions, runPolicyGate, triggerPostDeployScan } from '../helpers/policyGate';
|
||||
import { invalidateNodeCaches } from '../helpers/cacheInvalidation';
|
||||
import { STACK_STATUSES_CACHE_TTL_MS } from '../helpers/constants';
|
||||
import { getTerminalWs } from '../websocket/generic';
|
||||
import { getTerminalWs, DEPLOY_SESSION_HEADER } from '../websocket/generic';
|
||||
|
||||
// Authenticated users with edit permission can write arbitrarily large compose
|
||||
// files. Refuse to YAML.parse anything beyond this bound so a malformed (or
|
||||
@@ -311,7 +311,7 @@ async function runStackBulkOp(
|
||||
};
|
||||
}
|
||||
const atomic = effectiveTier(req) === 'paid';
|
||||
await ComposeService.getInstance(req.nodeId).updateStack(stackName, getTerminalWs(), atomic);
|
||||
await ComposeService.getInstance(req.nodeId).updateStack(stackName, getTerminalWs(req.get(DEPLOY_SESSION_HEADER)), atomic);
|
||||
DatabaseService.getInstance().clearStackUpdateStatus(req.nodeId, stackName);
|
||||
NotificationService.getInstance().broadcastEvent({
|
||||
type: 'state-invalidate',
|
||||
@@ -892,7 +892,7 @@ stacksRouter.post('/:stackName/deploy', async (req: Request, res: Response) => {
|
||||
const debug = isDebugEnabled();
|
||||
const atomic = effectiveTier(req) === 'paid';
|
||||
if (debug) console.debug('[Stacks:debug] Deploy starting', { stackName, atomic, nodeId: req.nodeId });
|
||||
await ComposeService.getInstance(req.nodeId).deployStack(stackName, getTerminalWs(), atomic);
|
||||
await ComposeService.getInstance(req.nodeId).deployStack(stackName, getTerminalWs(req.get(DEPLOY_SESSION_HEADER)), atomic);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
dlog(`[Stacks] Deploy completed: ${sanitizeForLog(stackName)}`);
|
||||
if (debug) console.debug(`[Stacks:debug] Deploy finished in ${Date.now() - t0}ms`);
|
||||
@@ -938,7 +938,7 @@ stacksRouter.post('/:stackName/down', async (req: Request, res: Response) => {
|
||||
let ok = false;
|
||||
try {
|
||||
if (isDebugEnabled()) console.debug(`[Stacks:debug] Down starting`, { stackName: sanitizeForLog(stackName), nodeId: req.nodeId });
|
||||
await ComposeService.getInstance(req.nodeId).runCommand(stackName, 'down', getTerminalWs());
|
||||
await ComposeService.getInstance(req.nodeId).runCommand(stackName, 'down', getTerminalWs(req.get(DEPLOY_SESSION_HEADER)));
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
dlog(`[Stacks] Down completed: ${sanitizeForLog(stackName)}`);
|
||||
ok = true;
|
||||
@@ -1152,7 +1152,7 @@ stacksRouter.post('/:stackName/update', async (req: Request, res: Response) => {
|
||||
const debug = isDebugEnabled();
|
||||
const atomic = effectiveTier(req) === 'paid';
|
||||
if (debug) console.debug('[Stacks:debug] Update starting', { stackName, atomic, nodeId: req.nodeId });
|
||||
await ComposeService.getInstance(req.nodeId).updateStack(stackName, getTerminalWs(), atomic);
|
||||
await ComposeService.getInstance(req.nodeId).updateStack(stackName, getTerminalWs(req.get(DEPLOY_SESSION_HEADER)), atomic);
|
||||
DatabaseService.getInstance().clearStackUpdateStatus(req.nodeId, stackName);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
NotificationService.getInstance().broadcastEvent({
|
||||
@@ -1209,7 +1209,7 @@ stacksRouter.post('/:stackName/rollback', async (req: Request, res: Response) =>
|
||||
dlog(`[Stacks] Rollback initiated: ${sanitizeForLog(stackName)}`);
|
||||
await fsSvc.restoreStackFiles(stackName);
|
||||
if (!(await runPolicyGate(req, res, stackName, req.nodeId))) return;
|
||||
await ComposeService.getInstance(req.nodeId).deployStack(stackName, getTerminalWs(), false);
|
||||
await ComposeService.getInstance(req.nodeId).deployStack(stackName, getTerminalWs(req.get(DEPLOY_SESSION_HEADER)), false);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
dlog(`[Stacks] Rollback completed: ${sanitizeForLog(stackName)}`);
|
||||
res.json({ message: 'Stack rolled back successfully.' });
|
||||
|
||||
@@ -14,7 +14,7 @@ import { isDebugEnabled } from '../utils/debug';
|
||||
import { getErrorMessage } from '../utils/errors';
|
||||
import { runPolicyGate, triggerPostDeployScan } from '../helpers/policyGate';
|
||||
import { invalidateNodeCaches } from '../helpers/cacheInvalidation';
|
||||
import { getTerminalWs } from '../websocket/generic';
|
||||
import { getTerminalWs, DEPLOY_SESSION_HEADER } from '../websocket/generic';
|
||||
|
||||
export const templatesRouter = Router();
|
||||
|
||||
@@ -125,7 +125,7 @@ templatesRouter.post('/deploy', authMiddleware, async (req: Request, res: Respon
|
||||
return;
|
||||
}
|
||||
const atomic = effectiveTier(req) === 'paid';
|
||||
await ComposeService.getInstance(req.nodeId).deployStack(stackName, getTerminalWs(), atomic);
|
||||
await ComposeService.getInstance(req.nodeId).deployStack(stackName, getTerminalWs(req.get(DEPLOY_SESSION_HEADER)), atomic);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
console.log(`[Templates] Deploy completed: ${stackName}`);
|
||||
res.json({ success: true, message: 'Template deployed successfully' });
|
||||
|
||||
@@ -131,9 +131,6 @@ export class ComposeService {
|
||||
clearTimeout(forceKillTimeout);
|
||||
forceKillTimeout = null;
|
||||
}
|
||||
if (ws) {
|
||||
ws.removeListener('close', onClientDisconnect);
|
||||
}
|
||||
};
|
||||
|
||||
const finish = (complete: () => void) => {
|
||||
@@ -161,21 +158,17 @@ export class ComposeService {
|
||||
}, 5000);
|
||||
};
|
||||
|
||||
const onClientDisconnect = () => {
|
||||
const message = 'Command cancelled because the client disconnected';
|
||||
terminateChild(new Error(message));
|
||||
};
|
||||
|
||||
// The progress socket is output-only: a deploy/update/down is owned by the
|
||||
// HTTP request that started it, so closing or losing the socket (the user
|
||||
// minimizes the panel, navigates away, or the connection blips) must not
|
||||
// terminate the compose process. Termination is driven solely by the
|
||||
// command timeout below.
|
||||
timeout = setTimeout(() => {
|
||||
const message = `Command timed out after ${Math.round(timeoutMs / 1000)}s`;
|
||||
sendOutput(`${message}\n`);
|
||||
terminateChild(new Error(message));
|
||||
}, timeoutMs);
|
||||
|
||||
if (ws) {
|
||||
ws.once('close', onClientDisconnect);
|
||||
}
|
||||
|
||||
const onData = (data: Buffer) => {
|
||||
const text = data.toString();
|
||||
errorLog += text;
|
||||
|
||||
@@ -8,19 +8,28 @@ import { isDebugEnabled } from '../utils/debug';
|
||||
import { rejectUpgrade as reject } from './reject';
|
||||
|
||||
/**
|
||||
* Module-scope singleton: the most recent WebSocket to send
|
||||
* `{action: 'connectTerminal'}` receives streaming output from any subsequent
|
||||
* compose deploy/down/update. Routes that want to echo compose progress read
|
||||
* the current value via `getTerminalWs()`.
|
||||
*
|
||||
* Intentionally single-instance. If multiple clients connect, the last one
|
||||
* wins. This matches pre-refactor behavior; race-hardening is a separate
|
||||
* concern.
|
||||
* Header the deploy/update/down routes carry the per-deploy correlation id on,
|
||||
* mirroring the `sessionId` the frontend sends in `{action:'connectTerminal'}`.
|
||||
* Must stay in sync with `DEPLOY_SESSION_HEADER` in `frontend/src/lib/api.ts`.
|
||||
*/
|
||||
let terminalWs: WebSocket | undefined;
|
||||
export const DEPLOY_SESSION_HEADER = 'x-deploy-session-id';
|
||||
|
||||
export function getTerminalWs(): WebSocket | undefined {
|
||||
return terminalWs;
|
||||
/**
|
||||
* Registry of compose-progress sockets keyed by the per-deploy correlation id
|
||||
* the frontend sends on `{action:'connectTerminal', sessionId}` and echoes on
|
||||
* the deploy/update/down POST via {@link DEPLOY_SESSION_HEADER}. A route resolves
|
||||
* the socket for its own deploy with `getTerminalWs(sessionId)`, so concurrent
|
||||
* deploys from different tabs or users never cross-stream output to each other.
|
||||
*
|
||||
* `lastTerminalWs` is the fallback for callers that connect or stream without a
|
||||
* session id (bulk operations, legacy clients): the most recent such socket wins.
|
||||
*/
|
||||
const terminalRegistry = new Map<string, WebSocket>();
|
||||
let lastTerminalWs: WebSocket | undefined;
|
||||
|
||||
export function getTerminalWs(sessionId?: string): WebSocket | undefined {
|
||||
const ws = sessionId ? terminalRegistry.get(sessionId) : lastTerminalWs;
|
||||
return ws && ws.readyState === WebSocket.OPEN ? ws : undefined;
|
||||
}
|
||||
|
||||
interface GenericContext {
|
||||
@@ -85,6 +94,7 @@ export function handleGenericWs(
|
||||
export function attachGenericConnectionHandlers(wss: WebSocketServer): void {
|
||||
wss.on('connection', (ws) => {
|
||||
console.log('WebSocket connected');
|
||||
let registeredSessionId: string | undefined;
|
||||
|
||||
ws.on('message', (message) => {
|
||||
try {
|
||||
@@ -92,7 +102,24 @@ export function attachGenericConnectionHandlers(wss: WebSocketServer): void {
|
||||
if (!data.action) return;
|
||||
|
||||
if (data.action === 'connectTerminal') {
|
||||
terminalWs = ws;
|
||||
const sessionId = typeof data.sessionId === 'string' && data.sessionId ? data.sessionId : undefined;
|
||||
// Rebind this socket to the new id, dropping any prior mapping it held.
|
||||
if (registeredSessionId && registeredSessionId !== sessionId && terminalRegistry.get(registeredSessionId) === ws) {
|
||||
terminalRegistry.delete(registeredSessionId);
|
||||
}
|
||||
registeredSessionId = sessionId;
|
||||
if (sessionId) {
|
||||
terminalRegistry.set(sessionId, ws);
|
||||
// A keyed socket must never be the id-less fallback, or a headerless
|
||||
// operation (bulk / rollback / legacy) would stream into this user's
|
||||
// keyed deploy modal.
|
||||
if (lastTerminalWs === ws) lastTerminalWs = undefined;
|
||||
} else {
|
||||
lastTerminalWs = ws;
|
||||
}
|
||||
if (isDebugEnabled()) {
|
||||
console.debug('[Deploy:diag] progress stream registered', { session: sessionId ? `${sessionId.slice(0, 8)}…` : '(none)' });
|
||||
}
|
||||
} else if (data.action === 'streamStats') {
|
||||
const requestedId = data.nodeId ? parseInt(data.nodeId, 10) : NodeRegistry.getInstance().getDefaultNodeId();
|
||||
// When a WS is proxied from a gateway to this remote instance, the
|
||||
@@ -117,5 +144,12 @@ export function attachGenericConnectionHandlers(wss: WebSocketServer): void {
|
||||
// Malformed JSON - ignore silently
|
||||
}
|
||||
});
|
||||
|
||||
ws.on('close', () => {
|
||||
if (registeredSessionId && terminalRegistry.get(registeredSessionId) === ws) {
|
||||
terminalRegistry.delete(registeredSessionId);
|
||||
}
|
||||
if (lastTerminalWs === ws) lastTerminalWs = undefined;
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user