diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js index 253417c..c398ac2 100644 --- a/packages/safe-chain/src/utils/safeSpawn.js +++ b/packages/safe-chain/src/utils/safeSpawn.js @@ -1,22 +1,77 @@ -import { spawn } from "child_process"; +import { spawn, execSync } from "child_process"; +import os from "os"; -function escapeArg(arg) { - // If argument contains spaces or quotes, wrap in double quotes and escape double quotes - if (arg.includes(" ") || arg.includes('"') || arg.includes("'")) { - return '"' + arg.replaceAll('"', '\\"') + '"'; +function sanitizeShellArgument(arg) { + // If argument contains shell metacharacters, wrap in double quotes + // and escape characters that are special even inside double quotes + if (hasShellMetaChars(arg)) { + // Inside double quotes, we need to escape: " $ ` \ + return '"' + escapeDoubleQuoteContent(arg) + '"'; } return arg; } +function hasShellMetaChars(arg) { + // Shell metacharacters that need escaping + // These characters have special meaning in shells and need to be quoted + // Whenever one of these characters is present, we should quote the argument + // Characters: space, ", &, ', |, ;, <, >, (, ), $, `, \, !, *, ?, [, ], {, }, ~, # + const shellMetaChars = /[ "&'|;<>()$`\\!*?[\]{}~#]/; + return shellMetaChars.test(arg); +} + +function escapeDoubleQuoteContent(arg) { + // Escape special characters for shell safety + // This escapes ", $, `, and \ by prefixing them with a backslash + return arg.replace(/(["`$\\])/g, "\\$1"); +} + function buildCommand(command, args) { - const escapedArgs = args.map(escapeArg); + if (args.length === 0) { + return command; + } + + const escapedArgs = args.map(sanitizeShellArgument); + return `${command} ${escapedArgs.join(" ")}`; } +function resolveCommandPath(command) { + // command will be "npm", "yarn", etc. + // Use 'command -v' to find the full path + const fullPath = execSync(`command -v ${command}`, { + encoding: "utf8", + shell: true, + }).trim(); + + if (!fullPath) { + throw new Error(`Command not found: ${command}`); + } + + return fullPath; +} + export async function safeSpawn(command, args, options = {}) { - const fullCommand = buildCommand(command, args); + // The command is always one of our supported package managers. + // It should always be alphanumeric or _ or - + // Reject any command names with suspicious characters + if (!/^[a-zA-Z0-9_-]+$/.test(command)) { + throw new Error(`Invalid command name: ${command}`); + } + return new Promise((resolve, reject) => { - const child = spawn(fullCommand, { ...options, shell: true }); + // 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 + // array args (safer, no escaping needed). + // See: https://nodejs.org/api/child_process.html#child_processspawncommand-args-options + let child; + if (os.platform() === "win32") { + const fullCommand = buildCommand(command, args); + child = spawn(fullCommand, { ...options, shell: true }); + } else { + const fullPath = resolveCommandPath(command); + child = spawn(fullPath, args, options); + } // When stdio is piped, we need to collect the output let stdout = ""; diff --git a/packages/safe-chain/src/utils/safeSpawn.spec.js b/packages/safe-chain/src/utils/safeSpawn.spec.js index 6417084..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", { @@ -15,13 +17,27 @@ describe("safeSpawn", () => { spawnCalls.push({ command, options }); return { on: (event, callback) => { - if (event === 'close') { + if (event === "close") { // Simulate immediate success setTimeout(() => callback(0), 0); } - } + }, }; }, + execSync: (cmd) => { + // Simulate 'command -v' returning full path + const match = cmd.match(/command -v (.+)/); + if (match) { + return `/usr/bin/${match[1]}\n`; + } + return ""; + }, + }, + }); + + mock.module("os", { + namedExports: { + platform: () => os, }, }); @@ -86,4 +102,113 @@ describe("safeSpawn", () => { 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"]); + + 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); + }); + + it("should escape dollar signs to prevent variable expansion", async () => { + await safeSpawn("echo", ["$HOME/test"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "\\$HOME/test"'); + }); + + it("should escape backticks to prevent command substitution", async () => { + await safeSpawn("echo", ["file`whoami`.txt"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "file\\`whoami\\`.txt"'); + }); + + it("should escape backslashes properly", async () => { + await safeSpawn("echo", ["path\\with\\backslash"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual( + spawnCalls[0].command, + 'echo "path\\\\with\\\\backslash"' + ); + }); + + it("should handle multiple special characters in one argument", async () => { + await safeSpawn("cmd", ['test "quoted" $var `cmd` & more']); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual( + spawnCalls[0].command, + 'cmd "test \\"quoted\\" \\$var \\`cmd\\` & more"' + ); + }); + + it("should handle pipe character", async () => { + await safeSpawn("echo", ["foo|bar"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "foo|bar"'); + }); + + it("should handle parentheses", async () => { + await safeSpawn("echo", ["(test)"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "(test)"'); + }); + + it("should handle angle brackets for redirection", async () => { + await safeSpawn("echo", ["foo>output.txt"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "foo>output.txt"'); + }); + + it("should handle wildcard characters", async () => { + await safeSpawn("echo", ["*.txt"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "*.txt"'); + }); + + it("should handle multiple arguments with mixed escaping needs", async () => { + await safeSpawn("cmd", ["safe", "needs space", "$dangerous", "also-safe"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual( + spawnCalls[0].command, + '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"); + }); });