Merge pull request #99 from AikidoSec/remove-sync

Remove `safeSpawnSync` (unused)
This commit is contained in:
Sander Declerck 2025-10-10 15:04:39 +02:00 committed by GitHub
commit dc4352bffb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 44 additions and 69 deletions

View file

@ -1,4 +1,4 @@
import { spawnSync, spawn } from "child_process"; import { spawn } from "child_process";
function escapeArg(arg) { function escapeArg(arg) {
// If argument contains spaces or quotes, wrap in double quotes and escape double quotes // If argument contains spaces or quotes, wrap in double quotes and escape double quotes
@ -13,11 +13,6 @@ function buildCommand(command, args) {
return `${command} ${escapedArgs.join(" ")}`; return `${command} ${escapedArgs.join(" ")}`;
} }
export function safeSpawnSync(command, args, options = {}) {
const fullCommand = buildCommand(command, args);
return spawnSync(fullCommand, { ...options, shell: true });
}
export async function safeSpawn(command, args, options = {}) { export async function safeSpawn(command, args, options = {}) {
const fullCommand = buildCommand(command, args); const fullCommand = buildCommand(command, args);
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {

View file

@ -2,7 +2,7 @@ import { describe, it, beforeEach, afterEach, mock } from "node:test";
import assert from "node:assert"; import assert from "node:assert";
describe("safeSpawn", () => { describe("safeSpawn", () => {
let safeSpawnSync, safeSpawn; let safeSpawn;
let spawnCalls = []; let spawnCalls = [];
beforeEach(async () => { beforeEach(async () => {
@ -11,14 +11,6 @@ describe("safeSpawn", () => {
// Mock child_process module to capture what command string gets built // Mock child_process module to capture what command string gets built
mock.module("child_process", { mock.module("child_process", {
namedExports: { namedExports: {
spawnSync: (command, options) => {
spawnCalls.push({ command, options });
return {
status: 0,
stdout: Buffer.from(""),
stderr: Buffer.from(""),
};
},
spawn: (command, options) => { spawn: (command, options) => {
spawnCalls.push({ command, options }); spawnCalls.push({ command, options });
return { return {
@ -35,7 +27,6 @@ describe("safeSpawn", () => {
// Import after mocking // Import after mocking
const safeSpawnModule = await import("./safeSpawn.js"); const safeSpawnModule = await import("./safeSpawn.js");
safeSpawnSync = safeSpawnModule.safeSpawnSync;
safeSpawn = safeSpawnModule.safeSpawn; safeSpawn = safeSpawnModule.safeSpawn;
}); });
@ -43,67 +34,56 @@ describe("safeSpawn", () => {
mock.reset(); mock.reset();
}); });
// Helper to run either sync or async variant it("should pass basic command and arguments correctly", async () => {
async function runSafeSpawn(variant, command, args, options) { await safeSpawn("echo", ["hello"]);
if (variant === "sync") {
return safeSpawnSync(command, args, options);
} else {
return await safeSpawn(command, args, options);
}
}
for (let variant of ["sync", "async"]) { assert.strictEqual(spawnCalls.length, 1);
it(`should pass basic command and arguments correctly (${variant})`, async () => { assert.strictEqual(spawnCalls[0].command, "echo hello");
await runSafeSpawn(variant, "echo", ["hello"]); assert.strictEqual(spawnCalls[0].options.shell, true);
});
assert.strictEqual(spawnCalls.length, 1); it("should escape arguments containing spaces", async () => {
assert.strictEqual(spawnCalls[0].command, "echo hello"); await safeSpawn("echo", ["hello world"]);
assert.strictEqual(spawnCalls[0].options.shell, true);
});
it(`should escape arguments containing spaces (${variant})`, async () => { assert.strictEqual(spawnCalls.length, 1);
await runSafeSpawn(variant, "echo", ["hello world"]); // 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); it("should prevent shell injection attacks", async () => {
// Argument should be escaped to prevent shell interpretation await safeSpawn("ls", ["; rm test123.txt"]);
assert.strictEqual(spawnCalls[0].command, 'echo "hello world"');
assert.strictEqual(spawnCalls[0].options.shell, true);
});
it(`should prevent shell injection attacks (${variant})`, async () => { assert.strictEqual(spawnCalls.length, 1);
await runSafeSpawn(variant, "ls", ["; rm test123.txt"]); // 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); it("should escape single quotes in arguments", async () => {
// Malicious command should be escaped to prevent execution await safeSpawn("echo", ["don't break"]);
assert.strictEqual(spawnCalls[0].command, 'ls "; rm test123.txt"');
assert.strictEqual(spawnCalls[0].options.shell, true);
});
it(`should escape single quotes in arguments (${variant})`, async () => { assert.strictEqual(spawnCalls.length, 1);
await runSafeSpawn(variant, "echo", ["don't break"]); // 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); it("should handle double quotes with simpler escaping", async () => {
// Single quote should be properly escaped with double quotes await safeSpawn("echo", ['say "hello"']);
assert.strictEqual(spawnCalls[0].command, 'echo "don\'t break"');
assert.strictEqual(spawnCalls[0].options.shell, true);
});
it(`should handle double quotes with simpler escaping (${variant})`, async () => { assert.strictEqual(spawnCalls.length, 1);
await runSafeSpawn(variant, "echo", ['say "hello"']); // 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); it("should not escape arguments with only safe characters", async () => {
// If we switch to double quotes, this should be: "say \"hello\"" await safeSpawn("npm", ["install", "axios", "--save"]);
assert.strictEqual(spawnCalls[0].command, 'echo "say \\"hello\\""');
assert.strictEqual(spawnCalls[0].options.shell, true);
});
it(`should not escape arguments with only safe characters (${variant})`, async () => { assert.strictEqual(spawnCalls.length, 1);
await runSafeSpawn(variant, "npm", ["install", "axios", "--save"]); // Safe arguments (alphanumeric, dash, underscore, dot, slash) shouldn't be quoted
assert.strictEqual(spawnCalls[0].command, "npm install axios --save");
assert.strictEqual(spawnCalls.length, 1); assert.strictEqual(spawnCalls[0].options.shell, true);
// 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);
});
}
}); });