🛡️ Sentinel: Fix sub-process OS command injection risk

.jules/sentinel.md

@@ -0,0 +1,7 @@
+# Sentinel Security Journal
+
+## 2025-02-13 - Mitigating OS Command Injection in Sub-process Spawning
+
+**Vulnerability:** The helper utility `sub-process.execute` was hardcoded to run processes with `{ shell: true }` by default. Spawning child processes via a shell is a major security risk, as any unvalidated or improperly escaped input in parameters or option fields (such as dynamic `cwd` or `args` derived from project/user files) could lead to OS command injection and arbitrary code execution.
+**Learning:** Legacy design patterns often prioritized shell-level features (like global variable interpolation or shell built-ins like `echo`) by default at the cost of security. By enforcing `shell: false` as the default and requiring an explicit, conscious opt-in (`{ shell: true }`), we align with modern secure-by-default software engineering standards.
+**Prevention:** Avoid running processes inside shell environments unless absolutely necessary. When spawning processes, explicitly pass `{ shell: false }` or use APIs that do not invoke the shell interpreter. If shell integration is unavoidable, strictly validate and escape all inputs before passing them to the shell interpreter.

src/lib/sub-process.ts

@@ -3,9 +3,13 @@ import * as childProcess from 'child_process';
 export function execute(
   command: string,
   args: string[],
-  options?: { cwd: string | undefined },
+  options?: { cwd?: string; shell?: boolean },
 ): Promise<string> {
-  const spawnOptions: childProcess.SpawnOptions = { shell: true };
+  // Security Hardening: Default to shell: false to prevent OS command injection.
+  // Callers requiring shell features must explicitly opt-in with { shell: true }.
+  const spawnOptions: childProcess.SpawnOptions = {
+    shell: options?.shell ?? false,
+  };
   if (options && options.cwd) {
     spawnOptions.cwd = options.cwd;
   }

test/tap/sub-process.test.js

@@ -45,13 +45,38 @@ function isSupported() {
 
 test('sub-process.execute executes sub processes', function (t) {
   if (isSupported()) {
-    t.test('runs in shell', function (t) {
+    t.test(
+      'does not run in shell by default (mitigates command injection)',
+      function (t) {
+        t.plan(1);
+
+        subProcess
+          .execute('echo', [shellVar])
+          .then(function (result) {
+            t.equal(
+              result.trim(),
+              shellVar,
+              'does not evaluate shell variable by default',
+            );
+          })
+          .catch(function () {
+            // On Windows, running a shell built-in without shell: true results in failure (ENOENT), which is secure and correct.
+            t.pass('does not run shell-builtin or fails securely');
+          });
+      },
+    );
+
+    t.test('runs in shell if explicitly opted in', function (t) {
       t.plan(1);
 
       subProcess
-        .execute('echo', [shellVar])
+        .execute('echo', [shellVar], { shell: true })
         .then(function (result) {
-          t.not(result.trim(), shellVar, 'evaluates shell variable');
+          t.not(
+            result.trim(),
+            shellVar,
+            'evaluates shell variable when shell: true',
+          );
         })
         .catch(t.fail);
     });