🛡️ Sentinel: [CRITICAL] Fix command injection in child process execution

.jules/sentinel.md

@@ -7,3 +7,8 @@
 **Vulnerability:** The daemon configuration file (`~/.agor/config.yaml`) and its parent directory (`~/.agor`) were created with default file permissions (e.g., `0o755`/`0o644`), which made them readable by other users on the system. This file stores extremely sensitive information such as API keys and master JWT secrets.
 **Learning:** Default Node.js filesystem operations (`fs.writeFile` and `fs.mkdir`) do not enforce strict permissions unless explicitly specified with a `mode` parameter. When handling sensitive files, relying on the system `umask` is insufficient.
 **Prevention:** Always specify `mode: 0o600` for sensitive files and `mode: 0o700` for their parent directories. Additionally, use `fs.chmod` to retroactively secure existing files and directories that might have been created with permissive defaults.
+
+## 2025-02-28 - [CRITICAL] Fix command injection in child process execution
+**Vulnerability:** Found `execSync` being used with string interpolation for dynamic variables, particularly `sudoUser` in `apps/agor-cli/src/commands/admin/sync-unix.ts` and `chownTo` / `envFile` in `apps/agor-daemon/src/services/terminals.ts`.
+**Learning:** Using `execSync` with dynamic user-controlled strings enables command injection because it executes commands within a shell (`/bin/sh`). In environments where inputs like `sudoUser` or API parameters are manipulatable, a threat actor can break out of the string boundary to execute arbitrary commands.
+**Prevention:** Instead of `execSync` or `exec` with string interpolation, use `execFileSync` or `execFile` exclusively with the executable passed as the first argument, and dynamic variables separated as array elements to bypass shell interpretation entirely. Place a double-dash `--` before the dynamic arguments to prevent argument injection vulnerabilities.

apps/agor-cli/src/commands/admin/sync-unix.ts

@@ -22,7 +22,7 @@
  * @see context/guides/rbac-and-unix-isolation.md
  */
 
-import { execSync } from 'node:child_process';
+import { execFileSync } from 'node:child_process';
 import { existsSync, readlinkSync } from 'node:fs';
 import { homedir } from 'node:os';
 import { join } from 'node:path';
@@ -236,7 +236,7 @@ export default class SyncUnix extends Command {
           // Running under sudo - use the invoking user's home directory
           // Try to get home directory from passwd entry
           try {
-            const passwdEntry = execSync(`getent passwd ${sudoUser}`, {
+            const passwdEntry = execFileSync('getent', ['passwd', '--', String(sudoUser)], {
               encoding: 'utf-8',
               stdio: ['pipe', 'pipe', 'ignore'],
             }).trim();

apps/agor-daemon/src/services/terminals.ts

@@ -18,7 +18,7 @@
  * - xterm.js frontend for rendering
  */
 
-import { execSync } from 'node:child_process';
+import { execFileSync } from 'node:child_process';
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
@@ -51,7 +51,7 @@ interface CreateTerminalData {
  */
 function isZellijAvailable(): boolean {
   try {
-    execSync('which zellij', { stdio: 'pipe', timeout: 2000 });
+    execFileSync('which', ['zellij'], { stdio: 'pipe', timeout: 2000 });
     return true;
   } catch {
     return false;
@@ -109,7 +109,10 @@ ${exportLines.join('\n')}
       try {
         // CRITICAL: Use -n flag to prevent password prompts that freeze the system
         // Also add timeout to prevent any hangs
-        execSync(`sudo -n chown "${chownTo}" "${envFile}"`, { stdio: 'pipe', timeout: 2000 });
+        execFileSync('sudo', ['-n', 'chown', '--', String(chownTo), String(envFile)], {
+          stdio: 'pipe',
+          timeout: 2000,
+        });
       } catch (chownError) {
         console.warn(`Failed to chown env file to ${chownTo}:`, chownError);
         // Continue anyway - file may still be readable in some configurations