From f5f3b91b40dc1bad0050ef1d4339672ecd2cfc5f Mon Sep 17 00:00:00 2001
From: Sander Declerck <sander@sdsoftware.be>
Date: Fri, 24 Oct 2025 17:36:51 +0200
Subject: [PATCH] Test if command is safe to execute

---
 packages/safe-chain/src/utils/safeSpawn.js    |  9 ++++++
 .../safe-chain/src/utils/safeSpawn.spec.js    | 29 ++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js
index c85b91e..8642b07 100644
--- a/packages/safe-chain/src/utils/safeSpawn.js
+++ b/packages/safe-chain/src/utils/safeSpawn.js
@@ -27,6 +27,10 @@ function escapeDoubleQuoteContent(arg) {
 }
 
 function buildCommand(command, args) {
+  if (args.length === 0) {
+    return command;
+  }
+
   const escapedArgs = args.map(sanitizeShellArgument);
 
   return `${command} ${escapedArgs.join(" ")}`;
@@ -48,6 +52,11 @@ function resolveCommandPath(command) {
 }
 
 export async function safeSpawn(command, args, options = {}) {
+  // command should always be alphanumeric or _ or - to avoid injection
+  if (!/^[a-zA-Z0-9_-]+$/.test(command)) {
+    throw new Error(`Invalid command name: ${command}`);
+  }
+
   return new Promise((resolve, reject) => {
     // Windows requires shell: true because .bat and .cmd files are not executable
     // without a terminal. On Unix/macOS, we resolve the full path first, then use
diff --git a/packages/safe-chain/src/utils/safeSpawn.spec.js b/packages/safe-chain/src/utils/safeSpawn.spec.js
index cbdb7cb..d4180b7 100644
--- a/packages/safe-chain/src/utils/safeSpawn.spec.js
+++ b/packages/safe-chain/src/utils/safeSpawn.spec.js
@@ -4,9 +4,11 @@ import assert from "node:assert";
 describe("safeSpawn", () => {
   let safeSpawn;
   let spawnCalls = [];
+  let os;
 
   beforeEach(async () => {
     spawnCalls = [];
+    os = "win32"; // Test Windows behavior by default
 
     // Mock child_process module to capture what command string gets built
     mock.module("child_process", {
@@ -35,7 +37,7 @@ describe("safeSpawn", () => {
 
     mock.module("os", {
       namedExports: {
-        platform: () => "win32",
+        platform: () => os,
       },
     });
 
@@ -184,4 +186,29 @@ describe("safeSpawn", () => {
       'cmd safe "needs space" "\\$dangerous" also-safe'
     );
   });
+
+  it("should reject command names with special characters", async () => {
+    await assert.rejects(async () => await safeSpawn("npm; echo hacked", []), {
+      message: "Invalid command name: npm; echo hacked",
+    });
+  });
+
+  it("should reject command names with spaces", async () => {
+    await assert.rejects(async () => await safeSpawn("npm install", []), {
+      message: "Invalid command name: npm install",
+    });
+  });
+
+  it("should reject command names with slashes", async () => {
+    await assert.rejects(async () => await safeSpawn("../../malicious", []), {
+      message: "Invalid command name: ../../malicious",
+    });
+  });
+
+  it("should accept valid command names with letters, numbers, underscores and hyphens", async () => {
+    await safeSpawn("valid_command-123", []);
+
+    assert.strictEqual(spawnCalls.length, 1);
+    assert.strictEqual(spawnCalls[0].command, "valid_command-123");
+  });
 });