fix: harden stack management operations (#1046)

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<typeof vi.fn>;
+    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<typeof vi.fn>;
+  close: ReturnType<typeof vi.fn>;
+  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;

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/);
+  });
+});

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);
+  });
 });

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; }
 

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<string, string | undefined>
-  ): Promise<void> {
-    return new Promise((resolve, reject) => {
-      const child = spawn(command, args, {
-        cwd,
-        env: env ?? {
+  ): Promise<void> {
+    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<typeof setTimeout> | null = null;
+      let forceKillTimeout: ReturnType<typeof setTimeout> | 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<T>(
     fn: (env: Record<string, string | undefined>) => Promise<T>,