From 0ff31427e6815ac58c0906065b41efb631686db0 Mon Sep 17 00:00:00 2001
From: Donach <39565367+Donach@users.noreply.github.com>
Date: Sat, 1 Aug 2026 18:25:32 +0000
Subject: [PATCH] Fix command injection vulnerability in unix system queries

Replaced `execSync` with `execFileSync` in `packages/core/src/unix/system-queries.ts` to execute shell commands like `id` and `getent`. This mitigates command injection risks because arguments are passed directly to the executable instead of being evaluated inside an implicit shell environment.

Appended a journal entry in `.jules/sentinel.md` documenting this security pattern.
---
 .jules/sentinel.md                       |  5 +++++
 packages/core/src/unix/system-queries.ts | 22 +++++++++-------------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 3a5ef27423..f1fca47762 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.
+
+## 2024-05-22 - [CRITICAL] Fix command injection vulnerability in unix system queries
+**Vulnerability:** The daemon used `execSync` from `node:child_process` in `packages/core/src/unix/system-queries.ts` to execute shell commands like `getent` and `id` which took dynamic parameters like `username` or `groupName`. Due to the usage of double quotes around parameters being interpreted by a shell environment, this resulted in a command injection vulnerability.
+**Learning:** Avoid using `execSync` with string interpolation for dynamic arguments (like usernames, group names, or paths). Always use `execFileSync` to prevent shell command injection, as double quoting in `execSync` does not fully protect against backticks or escaped quotes.
+**Prevention:** Always use `execFileSync` with its arguments array whenever possible instead of formatting command strings for the implicit shell environment to process.
diff --git a/packages/core/src/unix/system-queries.ts b/packages/core/src/unix/system-queries.ts
index 29b8d4120c..226c416b0e 100644
--- a/packages/core/src/unix/system-queries.ts
+++ b/packages/core/src/unix/system-queries.ts
@@ -9,9 +9,7 @@
  * @see context/guides/rbac-and-unix-isolation.md
  */
 
-import { execSync } from 'node:child_process';
-import { UnixGroupCommands } from './group-manager.js';
-import { UnixUserCommands } from './user-manager.js';
+import { execFileSync } from 'node:child_process';
 
 // ============================================================
 // USER QUERIES
@@ -27,7 +25,7 @@ import { UnixUserCommands } from './user-manager.js';
  */
 export function getUserGroups(username: string): string[] {
   try {
-    const output = execSync(UnixUserCommands.getUserGroups(username), {
+    const output = execFileSync('id', ['-nG', '--', String(username)], {
       encoding: 'utf-8',
       stdio: ['pipe', 'pipe', 'ignore'],
     });
@@ -37,6 +35,8 @@ export function getUserGroups(username: string): string[] {
   }
 }
 
+import { execSync } from 'node:child_process';
+
 /**
  * List all agor_* users on the system (auto-generated format: agor_<8-hex>)
  *
@@ -69,7 +69,7 @@ export function listAgorUsers(): string[] {
  */
 export function groupExists(groupName: string): boolean {
   try {
-    execSync(UnixGroupCommands.groupExists(groupName), { stdio: 'ignore' });
+    execFileSync('getent', ['group', '--', String(groupName)], { stdio: 'ignore' });
     return true;
   } catch {
     return false;
@@ -84,12 +84,7 @@ export function groupExists(groupName: string): boolean {
  * @returns true if user is in group
  */
 export function isUserInGroup(username: string, groupName: string): boolean {
-  try {
-    execSync(UnixGroupCommands.isUserInGroup(username, groupName), { stdio: 'ignore' });
-    return true;
-  } catch {
-    return false;
-  }
+  return getUserGroups(username).includes(groupName);
 }
 
 /**
@@ -100,11 +95,12 @@ export function isUserInGroup(username: string, groupName: string): boolean {
  */
 export function getGroupMembers(groupName: string): string[] {
   try {
-    const output = execSync(UnixGroupCommands.listGroupMembers(groupName), {
+    const output = execFileSync('getent', ['group', '--', String(groupName)], {
       encoding: 'utf-8',
       stdio: ['pipe', 'pipe', 'ignore'],
     });
-    return output.trim().split(',').filter(Boolean);
+    const parts = output.trim().split(':');
+    return parts[3] ? parts[3].split(',').filter(Boolean) : [];
   } catch {
     return [];
   }