diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 3a5ef27423..0ce8a7a1c2 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-07-27 - [Command Injection Risk via execSync in ID Lookups] +**Vulnerability:** Shell command injection vulnerability in `packages/core/src/unix/id-lookups.ts` caused by passing user-controlled usernames and group names to `execSync` using string interpolation (e.g., `execSync(\`id -u "${username}"\`)`). +**Learning:** Double quotes inside `execSync` do not fully protect against shell injection if the input contains backticks, escaped quotes, or other shell metacharacters. Additionally, without the `--` separator, inputs starting with a hyphen could be treated as command flags. +**Prevention:** Always use `execFileSync` instead of `execSync` for dynamic arguments, pass arguments as an array, and use `--` to explicitly terminate options processing. 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,