fix(core): prevent SIGHUP kills in PTY environments (WSL2/Kitty/Alacritty) (#27267)

packages/core/src/sandbox/utils/commandUtils.ts

@@ -56,7 +56,11 @@ export async function getCommandName(req: SandboxRequest): Promise<string> {
   const roots = getCommandRoots(stripped).filter(
     (r) => r !== 'shopt' && r !== 'set',
   );
-  if (roots.length > 0) {
+  // Single-root enforcement: only grant named-command permissions when the
+  // command is unambiguous. Multi-root chains fall back to basename so that
+  // a chained command like `git; malicious_cmd` never inherits `git`'s
+  // sandbox policy for the entire chain.
+  if (roots.length === 1) {
     return roots[0];
   }
   return path.basename(req.command);

packages/core/src/services/shellExecutionService.test.ts

@@ -289,7 +289,7 @@ describe('ShellExecutionService', () => {
         'bash',
         [
           '-c',
-          'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls -l',
+          "trap '' HUP; shopt -u promptvars nullglob extglob nocaseglob dotglob; ls -l",
         ],
         expect.any(Object),
       );
@@ -1049,7 +1049,7 @@ describe('ShellExecutionService', () => {
         'bash',
         [
           '-c',
-          'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls "foo bar"',
+          'trap \'\' HUP; shopt -u promptvars nullglob extglob nocaseglob dotglob; ls "foo bar"',
         ],
         expect.any(Object),
       );
@@ -1305,7 +1305,7 @@ describe('ShellExecutionService child_process fallback', () => {
         'bash',
         [
           '-c',
-          'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls -l',
+          "trap '' HUP; shopt -u promptvars nullglob extglob nocaseglob dotglob; ls -l",
         ],
         expect.objectContaining({ shell: false, detached: true }),
       );
@@ -1664,7 +1664,7 @@ describe('ShellExecutionService child_process fallback', () => {
         'bash',
         [
           '-c',
-          'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls "foo bar"',
+          'trap \'\' HUP; shopt -u promptvars nullglob extglob nocaseglob dotglob; ls "foo bar"',
         ],
         expect.objectContaining({
           shell: false,

packages/core/src/services/shellExecutionService.ts

@@ -17,6 +17,7 @@ import {
   getShellConfiguration,
   resolveExecutable,
   type ShellType,
+  BASH_HUP_GUARD,
 } from '../utils/shell-utils.js';
 import { isBinary, truncateString } from '../utils/textUtils.js';
 import pkg from '@xterm/headless';
@@ -114,6 +115,32 @@ function injectUtf8CodepageForPty(
   return command;
 }
 
+/**
+ * Prepends a POSIX SIGHUP-ignore guard to bash commands on non-Windows platforms.
+ *
+ * PTY environments such as WSL2, Kitty, and Alacritty aggressively send SIGHUP
+ * to process groups that lose their controlling terminal. By prepending
+ * `trap '' HUP;` we apply the same mechanism as the POSIX `nohup` utility:
+ * SIG_IGN is inherited across exec(), so every child spawned by the command
+ * also ignores SIGHUP — making the guard genuinely effective even in subshells.
+ *
+ * The guard is bash-only and idempotent (won't be doubled if already present).
+ * It is stripped back out by stripShellWrapper() / stripHupGuard() before any
+ * sandbox or permission-check logic sees the command, so there is no
+ * privilege-escalation surface from the preamble itself.
+ */
+function ensureHupIgnored(command: string, shell: ShellType): string {
+  if (shell !== 'bash') {
+    return command;
+  }
+  const trimmed = command.trimStart();
+  const prefix = `${BASH_HUP_GUARD} `;
+  if (trimmed.startsWith(prefix) || trimmed === BASH_HUP_GUARD) {
+    return command; // Already guarded — idempotent
+  }
+  return `${BASH_HUP_GUARD} ${command}`;
+}
+
 /** A structured result from a shell command execution. */
 export type ShellExecutionResult = ExecutionResult;
 
@@ -450,9 +477,16 @@ export class ShellExecutionService {
 
     const resolvedExecutable = resolveExecutable(executable) ?? executable;
 
-    const guardedCommand = ensurePromptvarsDisabled(commandToExecute, shell);
+    const promptGuarded = ensurePromptvarsDisabled(commandToExecute, shell);
+    // Prepend the SIGHUP-ignore guard for bash on non-Windows. This uses the
+    // same mechanism as POSIX `nohup`: SIG_IGN is inherited across exec(), so
+    // child processes spawned by the command also ignore SIGHUP. The guard is
+    // stripped by stripShellWrapper() before any sandbox permission checks.
+    const hupGuarded = !isWindows
+      ? ensureHupIgnored(promptGuarded, shell)
+      : promptGuarded;
     const finalCommand = injectUtf8CodepageForPty(
-      guardedCommand,
+      hupGuarded,
       shell,
       isWindows,
       usingPty,

packages/core/src/utils/shell-utils.test.ts

@@ -21,6 +21,8 @@ import {
   parseCommandDetails,
   splitCommands,
   stripShellWrapper,
+  stripHupGuard,
+  BASH_HUP_GUARD,
   normalizeCommand,
   hasRedirection,
   resolveExecutable,
@@ -646,3 +648,58 @@ describe('resolveExecutable', () => {
     expect(resolveExecutable('anything')).toBeUndefined();
   });
 });
+
+describe('stripHupGuard', () => {
+  it('should remove our own HUP guard prefix from the start of a command', () => {
+    expect(stripHupGuard(`${BASH_HUP_GUARD} git status`)).toBe('git status');
+  });
+
+  it('should be idempotent: if no guard present, return command unchanged', () => {
+    expect(stripHupGuard('git status')).toBe('git status');
+  });
+
+  it('should return empty string when command is exactly the guard itself', () => {
+    expect(stripHupGuard(BASH_HUP_GUARD)).toBe('');
+  });
+
+  it('should handle leading whitespace before the guard', () => {
+    expect(stripHupGuard(`   ${BASH_HUP_GUARD} git status`)).toBe('git status');
+  });
+
+  // Security regression: must NOT strip user-supplied trap commands
+  it('should NOT strip a user-supplied trap command (only strips our exact preamble)', () => {
+    // A user attempting to use trap as a sandbox bypass
+    const maliciousCmd = `trap 'rm -rf /' EXIT; git status`;
+    // stripHupGuard looks for the exact BASH_HUP_GUARD prefix (trap '' HUP;), not 'trap'
+    expect(stripHupGuard(maliciousCmd)).toBe(maliciousCmd);
+  });
+
+  it('should NOT strip a trap command with different arguments', () => {
+    const cmd = `trap '' TERM; git status`;
+    expect(stripHupGuard(cmd)).toBe(cmd);
+  });
+});
+
+describe('stripShellWrapper (with HUP guard integration)', () => {
+  it('should strip bash -c wrapper AND then the HUP guard preamble', () => {
+    const wrapped = `bash -c "${BASH_HUP_GUARD} git status"`;
+    expect(stripShellWrapper(wrapped)).toBe('git status');
+  });
+
+  it('should strip the HUP guard alone when no shell wrapper', () => {
+    expect(stripShellWrapper(`${BASH_HUP_GUARD} git status`)).toBe(
+      'git status',
+    );
+  });
+
+  it('should leave user-supplied trap commands intact (security check)', () => {
+    // This ensures that a user command starting with a different trap
+    // is not incorrectly stripped by the HUP guard removal logic
+    const maliciousCmd = `trap 'rm -rf /' EXIT; git status`;
+    expect(stripShellWrapper(maliciousCmd)).toBe(maliciousCmd);
+  });
+
+  it('should still strip a plain shell wrapper without HUP guard', () => {
+    expect(stripShellWrapper('sh -c "ls -l"')).toEqual('ls -l');
+  });
+});

packages/core/src/utils/shell-utils.ts

@@ -14,9 +14,40 @@ import {
   type SpawnOptionsWithoutStdio,
 } from 'node:child_process';
 
+/**
+ * The exact HUP-signal guard preamble injected by ensureHupIgnored().
+ * Exported so shellExecutionService and stripShellWrapper stay in sync.
+ */
+export const BASH_HUP_GUARD = `trap '' HUP;`;
+
+/**
+ * Strips the SIGHUP guard prepended by ensureHupIgnored() from a command.
+ *
+ * This is intentionally narrow: it only removes the exact literal string
+ * `trap '' HUP; ` from the very start of a command. It does NOT
+ * skip or ignore arbitrary user-supplied `trap` commands, which would be a
+ * sandbox-bypass vector (e.g., `trap 'rm -rf /' EXIT; git status`).
+ */
+export function stripHupGuard(command: string): string {
+  const trimmed = command.trimStart();
+  const prefix = `${BASH_HUP_GUARD} `;
+  if (trimmed.startsWith(prefix)) {
+    return trimmed.slice(prefix.length);
+  }
+  // Handle case where there's no trailing space (e.g., guard is the whole command)
+  if (trimmed === BASH_HUP_GUARD) {
+    return '';
+  }
+  return command;
+}
+
 /**
  * Extracts the primary command name from a potentially wrapped shell command.
- * Strips shell wrappers and handles shopt/set/etc.
+ * Strips shell wrappers (including our own HUP guard) and handles shopt/set/etc.
+ *
+ * Returns the command name only when there is exactly ONE non-builtin root so
+ * that chained commands (e.g. `git; malicious_cmd`) never silently inherit the
+ * first command's sandbox permissions.
  *
  * @param command - The full command string.
  * @param args - The arguments for the command.
@@ -32,7 +63,10 @@ export async function getCommandName(
   const roots = getCommandRoots(stripped).filter(
     (r) => r !== 'shopt' && r !== 'set',
   );
-  if (roots.length > 0) {
+  // Single-root enforcement: only grant named-command permissions when the
+  // command is unambiguous. Multi-root chains fall back to basename so that
+  // `git; malicious_cmd` never inherits `git`'s sandbox policy.
+  if (roots.length === 1) {
     return roots[0];
   }
   return path.basename(command);
@@ -838,6 +872,7 @@ export function stripShellWrapper(command: string): string {
   const pattern =
     /^\s*(?:(?:(?:\S+\/)?(?:sh|bash|zsh))\s+-c|cmd\.exe\s+\/c|powershell(?:\.exe)?\s+(?:-NoProfile\s+)?-Command|pwsh(?:\.exe)?\s+(?:-NoProfile\s+)?-Command)\s+/i;
   const match = command.match(pattern);
+  let result: string;
   if (match) {
     let newCommand = command.substring(match[0].length).trim();
     if (
@@ -846,9 +881,13 @@ export function stripShellWrapper(command: string): string {
     ) {
       newCommand = newCommand.substring(1, newCommand.length - 1);
     }
-    return newCommand;
+    result = newCommand;
+  } else {
+    result = command.trim();
   }
-  return command.trim();
+  // Peel off the SIGHUP guard that ensureHupIgnored() prepends so that
+  // sandbox managers see the actual user command, not our preamble.
+  return stripHupGuard(result);
 }
 
 /**