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