From 9abfc29038f709d914854ffc787fc955db706362 Mon Sep 17 00:00:00 2001
From: Donach <39565367+Donach@users.noreply.github.com>
Date: Tue, 4 Aug 2026 06:48:13 +0000
Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?=
 =?UTF-8?q?=20Fix=20command=20injection=20in=20terminal=20service?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replaced execSync with execFileSync when running sudo chown in terminal
service to prevent command injection from user-controlled chownTo input.
---
 .jules/sentinel.md                         | 5 +++++
 apps/agor-daemon/src/services/terminals.ts | 7 +++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 3a5ef27423..5e03995960 100644
--- a/.jules/sentinel.md
+++ b/.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-05-14 - [CRITICAL] Fix command injection in sudo chown
+**Vulnerability:** A critical command injection vulnerability in `apps/agor-daemon/src/services/terminals.ts` where user-controlled input (`chownTo`) was interpolated directly into a shell command executed via `execSync`. Even with double quotes, shell execution permits injection via backticks or escaped quotes, potentially leading to root privilege escalation because of the `sudo` command.
+**Learning:** Double-quoting dynamic arguments in `execSync` is insufficient to prevent command injection in a shell environment.
+**Prevention:** Always use `execFileSync` with an array of arguments, prefixing dynamic parameters with `--`, and casting dynamic parameters to `String()` to avoid shell injection entirely.
diff --git a/apps/agor-daemon/src/services/terminals.ts b/apps/agor-daemon/src/services/terminals.ts
index f343eddde8..faeb7d5fcb 100644
--- a/apps/agor-daemon/src/services/terminals.ts
+++ b/apps/agor-daemon/src/services/terminals.ts
@@ -18,7 +18,7 @@
  * - xterm.js frontend for rendering
  */
 
-import { execSync } from 'node:child_process';
+import { execFileSync, execSync } from 'node:child_process';
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
@@ -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