Merge branch 'main' into escape-special-chars-in-shell

packages/safe-chain/src/utils/safeSpawn.js

@@ -9,7 +9,7 @@ function escapeArg(arg) {
   // and escape characters that are special even inside double quotes
   if (shellMetaChars.test(arg)) {
     // Inside double quotes, we need to escape: " $ ` \
-    return '"' + arg.replace(/(["`$\\])/g, '\\$1') + '"';
+    return '"' + arg.replace(/(["`$\\])/g, "\\$1") + '"';
   }
   return arg;
 }
@@ -50,11 +50,23 @@ export async function safeSpawn(command, args, options = {}) {
       child = spawn(fullPath, args, options);
     }
 
+    // When stdio is piped, we need to collect the output
+    let stdout = "";
+    let stderr = "";
+
+    child.stdout?.on("data", (data) => {
+      stdout += data.toString();
+    });
+
+    child.stderr?.on("data", (data) => {
+      stderr += data.toString();
+    });
+
     child.on("close", (code) => {
       resolve({
         status: code,
-        stdout: Buffer.from(""),
-        stderr: Buffer.from(""),
+        stdout: stdout,
+        stderr: stderr,
       });
     });
 

packages/safe-chain/src/utils/safeSpawn.spec.js

@@ -2,7 +2,7 @@ import { describe, it, beforeEach, afterEach, mock } from "node:test";
 import assert from "node:assert";
 
 describe("safeSpawn", () => {
-  let safeSpawnSync, safeSpawn;
+  let safeSpawn;
   let spawnCalls = [];
 
   beforeEach(async () => {
@@ -11,31 +11,30 @@ describe("safeSpawn", () => {
     // Mock child_process module to capture what command string gets built
     mock.module("child_process", {
       namedExports: {
-        spawnSync: (command, options) => {
-          spawnCalls.push({ command, options });
-          return {
-            status: 0,
-            stdout: Buffer.from(""),
-            stderr: Buffer.from(""),
-          };
-        },
         spawn: (command, options) => {
           spawnCalls.push({ command, options });
           return {
             on: (event, callback) => {
-              if (event === 'close') {
+              if (event === "close") {
                 // Simulate immediate success
                 setTimeout(() => callback(0), 0);
               }
-            }
+            },
           };
         },
+        execSync: (cmd, opts) => {
+          // Simulate 'command -v' returning full path
+          const match = cmd.match(/command -v (.+)/);
+          if (match) {
+            return `/usr/bin/${match[1]}\n`;
+          }
+          return "";
+        },
       },
     });
 
     // Import after mocking
     const safeSpawnModule = await import("./safeSpawn.js");
-    safeSpawnSync = safeSpawnModule.safeSpawnSync;
     safeSpawn = safeSpawnModule.safeSpawn;
   });
 
@@ -43,76 +42,68 @@ describe("safeSpawn", () => {
     mock.reset();
   });
 
-  // Helper to run either sync or async variant
-  async function runSafeSpawn(variant, command, args, options) {
-    if (variant === "sync") {
-      return safeSpawnSync(command, args, options);
-    } else {
-      return await safeSpawn(command, args, options);
-    }
-  }
+  it("should pass basic command and arguments correctly", async () => {
+    await safeSpawn("echo", ["hello"]);
 
-  for (let variant of ["sync", "async"]) {
-    it(`should pass basic command and arguments correctly (${variant})`, async () => {
-      await runSafeSpawn(variant, "echo", ["hello"]);
+    assert.strictEqual(spawnCalls.length, 1);
+    assert.strictEqual(spawnCalls[0].command, "echo hello");
+    assert.strictEqual(spawnCalls[0].options.shell, true);
+  });
 
-      assert.strictEqual(spawnCalls.length, 1);
-      assert.strictEqual(spawnCalls[0].command, "echo hello");
-      assert.strictEqual(spawnCalls[0].options.shell, true);
-    });
+  it("should escape arguments containing spaces", async () => {
+    await safeSpawn("echo", ["hello world"]);
 
-    it(`should escape arguments containing spaces (${variant})`, async () => {
-      await runSafeSpawn(variant, "echo", ["hello world"]);
+    assert.strictEqual(spawnCalls.length, 1);
+    // Argument should be escaped to prevent shell interpretation
+    assert.strictEqual(spawnCalls[0].command, 'echo "hello world"');
+    assert.strictEqual(spawnCalls[0].options.shell, true);
+  });
 
-      assert.strictEqual(spawnCalls.length, 1);
-      // Argument should be escaped to prevent shell interpretation
-      assert.strictEqual(spawnCalls[0].command, 'echo "hello world"');
-      assert.strictEqual(spawnCalls[0].options.shell, true);
-    });
+  it("should prevent shell injection attacks", async () => {
+    await safeSpawn("ls", ["; rm test123.txt"]);
 
-    it(`should prevent shell injection attacks (${variant})`, async () => {
-      await runSafeSpawn(variant, "ls", ["; rm test123.txt"]);
+    assert.strictEqual(spawnCalls.length, 1);
+    // Malicious command should be escaped to prevent execution
+    assert.strictEqual(spawnCalls[0].command, 'ls "; rm test123.txt"');
+    assert.strictEqual(spawnCalls[0].options.shell, true);
+  });
 
-      assert.strictEqual(spawnCalls.length, 1);
-      // Malicious command should be escaped to prevent execution
-      assert.strictEqual(spawnCalls[0].command, 'ls "; rm test123.txt"');
-      assert.strictEqual(spawnCalls[0].options.shell, true);
-    });
+  it("should escape single quotes in arguments", async () => {
+    await safeSpawn("echo", ["don't break"]);
 
-    it(`should escape single quotes in arguments (${variant})`, async () => {
-      await runSafeSpawn(variant, "echo", ["don't break"]);
+    assert.strictEqual(spawnCalls.length, 1);
+    // Single quote should be properly escaped with double quotes
+    assert.strictEqual(spawnCalls[0].command, 'echo "don\'t break"');
+    assert.strictEqual(spawnCalls[0].options.shell, true);
+  });
 
-      assert.strictEqual(spawnCalls.length, 1);
-      // Single quote should be properly escaped with double quotes
-      assert.strictEqual(spawnCalls[0].command, 'echo "don\'t break"');
-      assert.strictEqual(spawnCalls[0].options.shell, true);
-    });
+  it("should handle double quotes with simpler escaping", async () => {
+    await safeSpawn("echo", ['say "hello"']);
 
-    it(`should handle double quotes with simpler escaping (${variant})`, async () => {
-      await runSafeSpawn(variant, "echo", ['say "hello"']);
+    assert.strictEqual(spawnCalls.length, 1);
+    // If we switch to double quotes, this should be: "say \"hello\""
+    assert.strictEqual(spawnCalls[0].command, 'echo "say \\"hello\\""');
+    assert.strictEqual(spawnCalls[0].options.shell, true);
+  });
 
-      assert.strictEqual(spawnCalls.length, 1);
-      // If we switch to double quotes, this should be: "say \"hello\""
-      assert.strictEqual(spawnCalls[0].command, 'echo "say \\"hello\\""');
-      assert.strictEqual(spawnCalls[0].options.shell, true);
-    });
+  it("should not escape arguments with only safe characters", async () => {
+    await safeSpawn("npm", ["install", "axios", "--save"]);
 
-    it(`should not escape arguments with only safe characters (${variant})`, async () => {
-      await runSafeSpawn(variant, "npm", ["install", "axios", "--save"]);
+    assert.strictEqual(spawnCalls.length, 1);
+    // Safe arguments (alphanumeric, dash, underscore, dot, slash) shouldn't be quoted
+    assert.strictEqual(spawnCalls[0].command, "npm install axios --save");
+    assert.strictEqual(spawnCalls[0].options.shell, true);
+  });
 
-      assert.strictEqual(spawnCalls.length, 1);
-      // Safe arguments (alphanumeric, dash, underscore, dot, slash) shouldn't be quoted
-      assert.strictEqual(spawnCalls[0].command, "npm install axios --save");
-      assert.strictEqual(spawnCalls[0].options.shell, true);
-    });
+  it(`should escape ampersand character`, async () => {
+    await safeSpawn("npx", ["cypress", "run", "--env", "password=foo&bar"]);
 
-    it(`should escape ampersand character (${variant})`, async () => {
-      await runSafeSpawn(variant, "npx", ["cypress", "run", "--env", "password=foo&bar"]);
-
-      assert.strictEqual(spawnCalls.length, 1);
-      // & should be escaped by wrapping the arg in quotes
-      assert.strictEqual(spawnCalls[0].command, 'npx cypress run --env "password=foo&bar"');
-      assert.strictEqual(spawnCalls[0].options.shell, true);
-    });
-  }
-});
+    assert.strictEqual(spawnCalls.length, 1);
+    // & should be escaped by wrapping the arg in quotes
+    assert.strictEqual(
+      spawnCalls[0].command,
+      'npx cypress run --env "password=foo&bar"'
+    );
+    assert.strictEqual(spawnCalls[0].options.shell, true);
+  });
+});