diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 3a5ef27423..e7bbb941d4 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -7,3 +7,7 @@ **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-07-31 - [Command Injection Risk via execSync in ID Lookups] +**Vulnerability:** Command injection vulnerability in `packages/core/src/unix/id-lookups.ts`. Functions looking up user and group IDs used `execSync` with string interpolation for dynamic arguments (`username`, `groupName`). Even with double quotes, this allowed potential shell command injection. +**Learning:** Using `execSync` with dynamic arguments is inherently unsafe because it spawns a shell which evaluates metacharacters. Double quotes do not prevent execution of backticks or properly escaped commands. +**Prevention:** Always use `execFileSync` (or `spawn`/`execFile`) instead of `execSync` for dynamic arguments to avoid shell execution. Explicitly cast arguments to strings and use `--` to prevent argument injection vulnerabilities. diff --git a/packages/core/src/unix/id-lookups.ts b/packages/core/src/unix/id-lookups.ts index 4a7f16390d..b9b591d4f8 100644 --- a/packages/core/src/unix/id-lookups.ts +++ b/packages/core/src/unix/id-lookups.ts @@ -4,7 +4,7 @@ * Supports both Linux (using getent) and macOS (parsing /etc/group and /etc/passwd) */ -import { execSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; /** @@ -26,7 +26,7 @@ export function getGidFromGroupName(groupName: string | undefined | null): numbe try { // Try getent first (Linux, some BSD) try { - const result = execSync(`getent group "${groupName}"`, { + const result = execFileSync('getent', ['group', '--', String(groupName)], { encoding: 'utf-8', stdio: 'pipe', timeout: 2000, @@ -91,7 +91,7 @@ export function getUidFromUsername(username: string | undefined | null): number try { // Try `id -u username` first (most reliable) try { - const result = execSync(`id -u "${username}"`, { + const result = execFileSync('id', ['-u', '--', String(username)], { encoding: 'utf-8', stdio: 'pipe', timeout: 2000, @@ -107,7 +107,7 @@ export function getUidFromUsername(username: string | undefined | null): number // Try getent (Linux, some BSD) try { - const result = execSync(`getent passwd "${username}"`, { + const result = execFileSync('getent', ['passwd', '--', String(username)], { encoding: 'utf-8', stdio: 'pipe', timeout: 2000, @@ -171,7 +171,7 @@ export function getHomedirFromUsername(username: string | undefined | null): str try { // Try getent first (Linux, some BSD) try { - const result = execSync(`getent passwd "${username}"`, { + const result = execFileSync('getent', ['passwd', '--', String(username)], { encoding: 'utf-8', stdio: 'pipe', timeout: 2000,