diff --git a/backend/src/__tests__/compose-service.test.ts b/backend/src/__tests__/compose-service.test.ts index 98d607c1..0aefd2c4 100644 --- a/backend/src/__tests__/compose-service.test.ts +++ b/backend/src/__tests__/compose-service.test.ts @@ -4,6 +4,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { EventEmitter } from 'events'; +import type WebSocket from 'ws'; // ── Hoisted mocks ────────────────────────────────────────────────────── @@ -102,16 +103,23 @@ vi.mock('../services/LogFormatter', () => ({ import { ComposeService, getComposeRollbackInfo } from '../services/ComposeService'; +const originalComposeTimeout = process.env.SENCHO_COMPOSE_COMMAND_TIMEOUT_MS; + /** Creates an EventEmitter that mimics a child_process spawn result */ function createMockProcess() { const proc = new EventEmitter() as EventEmitter & { stdout: EventEmitter; stderr: EventEmitter; kill: ReturnType; + killed: boolean; }; proc.stdout = new EventEmitter(); proc.stderr = new EventEmitter(); - proc.kill = vi.fn(); + proc.killed = false; + proc.kill = vi.fn(() => { + proc.killed = true; + return true; + }); return proc; } @@ -125,14 +133,22 @@ function setupAutoCloseSpawn(exitCode = 0) { }); } -function createMockWs() { - return { +type MockWebSocket = EventEmitter & { + readyState: number; + send: ReturnType; + close: ReturnType; + OPEN: number; +} & WebSocket; + +function createMockWs(): MockWebSocket { + const ws = new EventEmitter() as MockWebSocket; + Object.assign(ws, { readyState: 1, send: vi.fn(), - on: vi.fn(), close: vi.fn(), OPEN: 1, - }; + }); + return ws; } beforeEach(() => { @@ -142,6 +158,11 @@ beforeEach(() => { afterEach(() => { vi.useRealTimers(); + if (originalComposeTimeout === undefined) { + delete process.env.SENCHO_COMPOSE_COMMAND_TIMEOUT_MS; + } else { + process.env.SENCHO_COMPOSE_COMMAND_TIMEOUT_MS = originalComposeTimeout; + } }); // ── runCommand ───────────────────────────────────────────────────────── @@ -206,13 +227,49 @@ describe('ComposeService - runCommand', () => { const ws = createMockWs(); const svc = ComposeService.getInstance(1); - const promise = svc.runCommand('my-stack', 'restart', ws as any); + const promise = svc.runCommand('my-stack', 'restart', ws); proc.stdout.emit('data', Buffer.from('Restarting...')); proc.emit('close', 0); await promise; expect(ws.send).toHaveBeenCalledWith('Restarting...'); }); + + it('kills and rejects commands that exceed the compose timeout', async () => { + process.env.SENCHO_COMPOSE_COMMAND_TIMEOUT_MS = '1000'; + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const svc = ComposeService.getInstance(1); + const promise = svc.runCommand('my-stack', 'restart'); + const expectation = expect(promise).rejects.toThrow('Command timed out after 1s'); + let settled = false; + promise.finally(() => { settled = true; }).catch(() => undefined); + await vi.advanceTimersByTimeAsync(1000); + + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + expect(settled).toBe(false); + proc.emit('close', null); + await expectation; + }); + + it('kills and rejects running commands when the WebSocket disconnects', 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); + ws.emit('close'); + + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + expect(settled).toBe(false); + proc.emit('close', null); + await expectation; + }); }); // ── deployStack ──────────────────────────────────────────────────────── @@ -389,7 +446,7 @@ describe('ComposeService - withRegistryAuth', () => { const ws = createMockWs(); const svc = ComposeService.getInstance(1); - const promise = svc.deployStack('my-stack', ws as any); + const promise = svc.deployStack('my-stack', ws); await vi.advanceTimersByTimeAsync(3100); await promise; diff --git a/backend/src/__tests__/scheduled-tasks-routes.test.ts b/backend/src/__tests__/scheduled-tasks-routes.test.ts index a9aaaf3c..ce1cb58c 100644 --- a/backend/src/__tests__/scheduled-tasks-routes.test.ts +++ b/backend/src/__tests__/scheduled-tasks-routes.test.ts @@ -246,6 +246,30 @@ describe('POST /api/scheduled-tasks', () => { expect(res.status).toBe(400); expect(res.body.error).toMatch(/target_services can only be used with restart/); }); + + it('rejects invalid stack target_id values', async () => { + const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({ + ...basePayload, target_id: '../etc/passwd', + }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/valid stack name/); + }); + + it('rejects stack target_id values with surrounding whitespace', async () => { + const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({ + ...basePayload, target_id: ' my-stack ', + }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/valid stack name/); + }); + + it('rejects invalid stack node_id values', async () => { + const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({ + ...basePayload, node_id: 'not-a-node', + }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/valid node_id/); + }); }); describe('GET /api/scheduled-tasks/:id', () => { @@ -410,3 +434,67 @@ describe('PUT /api/scheduled-tasks/:id - delete_after_run', () => { expect(res2.body.delete_after_run).toBe(0); }); }); + +describe('PUT /api/scheduled-tasks/:id - stack target validation', () => { + let taskId: number; + + beforeEach(() => { + const now = Date.now(); + taskId = DatabaseService.getInstance().createScheduledTask({ + name: 't', target_type: 'stack', target_id: 's', node_id: 1, action: 'update', + cron_expression: '0 3 * * *', enabled: 1, created_by: 'admin', created_at: now, updated_at: now, + last_run_at: null, next_run_at: null, last_status: null, last_error: null, + prune_targets: null, target_services: null, prune_label_filter: null, delete_after_run: 0, + }); + }); + + it('rejects updates that introduce path traversal in stack target_id', async () => { + const res = await request(app) + .put(`/api/scheduled-tasks/${taskId}`) + .set('Cookie', adminCookie) + .send({ target_id: '../bad' }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/valid stack name/); + }); + + it('rejects updates that introduce whitespace in stack target_id', async () => { + const res = await request(app) + .put(`/api/scheduled-tasks/${taskId}`) + .set('Cookie', adminCookie) + .send({ target_id: ' s ' }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/valid stack name/); + }); + + it('rejects updates that clear node_id for a stack target', async () => { + const res = await request(app) + .put(`/api/scheduled-tasks/${taskId}`) + .set('Cookie', adminCookie) + .send({ node_id: null }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/node_id/); + }); + + it('rejects updates that clear target_type', async () => { + const res = await request(app) + .put(`/api/scheduled-tasks/${taskId}`) + .set('Cookie', adminCookie) + .send({ target_type: null }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/Invalid target_type/); + }); + + it('rejects updates that clear action', async () => { + const res = await request(app) + .put(`/api/scheduled-tasks/${taskId}`) + .set('Cookie', adminCookie) + .send({ action: null }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/Invalid action/); + }); +}); diff --git a/backend/src/__tests__/stacks-failure-notifications.test.ts b/backend/src/__tests__/stacks-failure-notifications.test.ts index 0aa5f991..f68545b9 100644 --- a/backend/src/__tests__/stacks-failure-notifications.test.ts +++ b/backend/src/__tests__/stacks-failure-notifications.test.ts @@ -340,4 +340,18 @@ describe('deploy_failure notification on /update error', () => { expect(res.status).toBe(500); expect(res.body).toMatchObject({ rolledBack: false }); }); + + it('uses trusted proxy tier headers for remote atomic updates', async () => { + mockUpdateStack.mockResolvedValue(undefined); + const token = jwt.sign({ scope: 'node_proxy' }, TEST_JWT_SECRET, { expiresIn: '1m' }); + + const res = await request(app) + .post('/api/stacks/myapp/update') + .set('Authorization', `Bearer ${token}`) + .set('x-sencho-tier', 'paid') + .set('x-sencho-variant', 'skipper'); + + expect(res.status).toBe(200); + expect(mockUpdateStack.mock.calls[0][2]).toBe(true); + }); }); diff --git a/backend/src/routes/scheduledTasks.ts b/backend/src/routes/scheduledTasks.ts index c485a102..dc4fcf5e 100644 --- a/backend/src/routes/scheduledTasks.ts +++ b/backend/src/routes/scheduledTasks.ts @@ -8,6 +8,7 @@ import { escapeCsvField } from '../utils/csv'; import { getErrorMessage } from '../utils/errors'; import { parseIntParam } from '../utils/parseIntParam'; import { sanitizeForLog } from '../utils/safeLog'; +import { isValidStackName } from '../utils/validation'; const VALID_TARGET_TYPES = ['stack', 'fleet', 'system'] as const; const VALID_ACTIONS = ['restart', 'snapshot', 'prune', 'update', 'scan', 'auto_backup', 'auto_stop', 'auto_down', 'auto_start'] as const; @@ -37,6 +38,25 @@ function validateActionTarget(action: ScheduledAction, targetType: TargetType): return null; } +function validateStackTarget(targetType: TargetType, targetId: unknown, nodeId: unknown): string | null { + if (targetType !== 'stack') return null; + + if (typeof targetId !== 'string' || !targetId.trim() || nodeId === null || nodeId === undefined) { + return 'Stack operations require target_id and node_id.'; + } + + if (targetId !== targetId.trim() || !isValidStackName(targetId)) { + return 'Stack target_id must be a valid stack name.'; + } + + const parsedNodeId = Number(nodeId); + if (!Number.isInteger(parsedNodeId) || parsedNodeId <= 0) { + return 'Stack operations require a valid node_id.'; + } + + return null; +} + function validateScanNode(nodeId: unknown): string | null { if (nodeId == null) return 'Scan action requires node_id.'; const parsedNodeId = Number(nodeId); @@ -150,9 +170,8 @@ scheduledTasksRouter.post('/', (req: Request, res: Response): void => { if (action === 'update' && target_type === 'fleet' && !node_id) { res.status(400).json({ error: ERR_FLEET_NODE_REQUIRED }); return; } - if (target_type === 'stack' && (!target_id || !node_id)) { - res.status(400).json({ error: 'Stack operations require target_id and node_id.' }); return; - } + const stackTargetErr = validateStackTarget(target_type, target_id, node_id); + if (stackTargetErr) { res.status(400).json({ error: stackTargetErr }); return; } const optionalErr = validateOptionalFields(action, target_type, prune_targets, target_services, prune_label_filter); if (optionalErr) { res.status(400).json({ error: optionalErr }); return; } @@ -226,30 +245,33 @@ scheduledTasksRouter.put('/:id', (req: Request, res: Response): void => { const { name, target_type, target_id, node_id, action, cron_expression, enabled, prune_targets, target_services, prune_label_filter, delete_after_run } = req.body; - if (target_type && !(VALID_TARGET_TYPES as readonly string[]).includes(target_type)) { + if (target_type !== undefined && !(VALID_TARGET_TYPES as readonly string[]).includes(target_type)) { res.status(400).json({ error: 'Invalid target_type' }); return; } - if (action && !(VALID_ACTIONS as readonly string[]).includes(action)) { + if (action !== undefined && !(VALID_ACTIONS as readonly string[]).includes(action)) { res.status(400).json({ error: 'Invalid action' }); return; } - const finalAction = (action || existing.action) as ScheduledAction; - const finalTargetType = (target_type || existing.target_type) as TargetType; + const finalAction = (action ?? existing.action) as ScheduledAction; + const finalTargetType = (target_type ?? existing.target_type) as TargetType; + const finalTargetId = target_id !== undefined ? target_id : existing.target_id; + const finalNodeId = node_id !== undefined ? node_id : existing.node_id; const targetErr = validateActionTarget(finalAction, finalTargetType); if (targetErr) { res.status(400).json({ error: targetErr }); return; } if (finalAction === 'scan') { - const finalNodeId = node_id !== undefined ? node_id : existing.node_id; const nodeErr = validateScanNode(finalNodeId); if (nodeErr) { res.status(400).json({ error: nodeErr }); return; } } if (finalAction === 'update' && finalTargetType === 'fleet') { - const finalNodeId = node_id !== undefined ? node_id : existing.node_id; if (!finalNodeId) { res.status(400).json({ error: ERR_FLEET_NODE_REQUIRED }); return; } } + const stackTargetErr = validateStackTarget(finalTargetType, finalTargetId, finalNodeId); + if (stackTargetErr) { res.status(400).json({ error: stackTargetErr }); return; } + const optionalErr = validateOptionalFields(finalAction, finalTargetType, prune_targets, target_services, prune_label_filter); if (optionalErr) { res.status(400).json({ error: optionalErr }); return; } diff --git a/backend/src/services/ComposeService.ts b/backend/src/services/ComposeService.ts index 8a32653f..583a60a4 100644 --- a/backend/src/services/ComposeService.ts +++ b/backend/src/services/ComposeService.ts @@ -31,12 +31,22 @@ export class ComposeRollbackError extends Error { } } -export function getComposeRollbackInfo(error: unknown): { attempted: boolean; rolledBack: boolean } | null { - if (!(error instanceof ComposeRollbackError)) { - return null; - } - return { attempted: error.rollbackAttempted, rolledBack: error.rolledBack }; -} +export function getComposeRollbackInfo(error: unknown): { attempted: boolean; rolledBack: boolean } | null { + if (!(error instanceof ComposeRollbackError)) { + return null; + } + return { attempted: error.rollbackAttempted, rolledBack: error.rolledBack }; +} + +const DEFAULT_COMPOSE_COMMAND_TIMEOUT_MS = 30 * 60 * 1000; + +function getComposeCommandTimeoutMs(): number { + const configured = Number(process.env.SENCHO_COMPOSE_COMMAND_TIMEOUT_MS); + if (Number.isFinite(configured) && configured > 0) { + return configured; + } + return DEFAULT_COMPOSE_COMMAND_TIMEOUT_MS; +} /** * ComposeService - local docker compose CLI execution. @@ -87,47 +97,123 @@ export class ComposeService { ws?: WebSocket, throwOnError = true, env?: Record - ): Promise { - return new Promise((resolve, reject) => { - const child = spawn(command, args, { - cwd, - env: env ?? { + ): Promise { + return new Promise((resolve, reject) => { + const child = spawn(command, args, { + cwd, + env: env ?? { ...process.env, PATH: process.env.PATH || '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin' } - }); - - let errorLog = ''; - - const onData = (data: Buffer) => { - const text = data.toString(); - errorLog += text; - if (ws && ws.readyState === WebSocket.OPEN) { - ws.send(text); - } - }; + }); + + let errorLog = ''; + let settled = false; + let exited = false; + let pendingTerminationError: Error | null = null; + const timeoutMs = getComposeCommandTimeoutMs(); + let timeout: ReturnType | null = null; + let forceKillTimeout: ReturnType | null = null; + + const sendOutput = (text: string) => { + if (ws && ws.readyState === WebSocket.OPEN) { + ws.send(text); + } + }; + + const cleanup = () => { + if (timeout) { + clearTimeout(timeout); + timeout = null; + } + if (forceKillTimeout) { + clearTimeout(forceKillTimeout); + forceKillTimeout = null; + } + if (ws) { + ws.removeListener('close', onClientDisconnect); + } + }; + + const finish = (complete: () => void) => { + if (settled) return; + settled = true; + cleanup(); + complete(); + }; + + const terminateChild = (error: Error) => { + pendingTerminationError = pendingTerminationError ?? error; + if (exited) return; + try { + child.kill('SIGTERM'); + } catch (error) { + console.warn('[ComposeService] Failed to terminate compose command:', sanitizeForLog(getErrorMessage(error, 'unknown'))); + } + forceKillTimeout = setTimeout(() => { + if (exited) return; + try { + child.kill('SIGKILL'); + } catch (error) { + console.warn('[ComposeService] Failed to force terminate compose command:', sanitizeForLog(getErrorMessage(error, 'unknown'))); + } + }, 5000); + }; + + const onClientDisconnect = () => { + const message = 'Command cancelled because the client disconnected'; + terminateChild(new Error(message)); + }; + + 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; + sendOutput(text); + }; child.stdout.on('data', onData); - child.stderr.on('data', onData); - - child.on('close', (code: number | null) => { - if (ws && ws.readyState === WebSocket.OPEN) { - ws.send(`Command exited with code ${code}\n`); - } - if (code === 0) resolve(); - else if (throwOnError) reject(new Error(redactSensitiveText(errorLog.trim()) || `Command failed with code ${code}`)); - else resolve(); - }); - - child.on('error', (error: Error) => { - if (ws && ws.readyState === WebSocket.OPEN) { - ws.send(`Error: ${redactSensitiveText(error.message)}\n`); - } - if (throwOnError) reject(new Error(redactSensitiveText(error.message))); - else resolve(); - }); - }); - } + child.stderr.on('data', onData); + + child.on('close', (code: number | null) => { + exited = true; + finish(() => { + sendOutput(`Command exited with code ${code}\n`); + if (pendingTerminationError) { + if (throwOnError) reject(pendingTerminationError); + else resolve(); + return; + } + if (code === 0) resolve(); + else if (throwOnError) reject(new Error(redactSensitiveText(errorLog.trim()) || `Command failed with code ${code}`)); + else resolve(); + }); + }); + + child.on('error', (error: Error) => { + exited = true; + finish(() => { + sendOutput(`Error: ${redactSensitiveText(error.message)}\n`); + if (pendingTerminationError) { + if (throwOnError) reject(pendingTerminationError); + else resolve(); + return; + } + if (throwOnError) reject(new Error(redactSensitiveText(error.message))); + else resolve(); + }); + }); + }); + } private async withRegistryAuth( fn: (env: Record) => Promise,