Test if command is safe to execute

This commit is contained in:
Sander Declerck 2025-10-24 17:36:51 +02:00
parent 7a55be49f4
commit f5f3b91b40
No known key found for this signature in database
2 changed files with 37 additions and 1 deletions

View file

@ -27,6 +27,10 @@ function escapeDoubleQuoteContent(arg) {
} }
function buildCommand(command, args) { function buildCommand(command, args) {
if (args.length === 0) {
return command;
}
const escapedArgs = args.map(sanitizeShellArgument); const escapedArgs = args.map(sanitizeShellArgument);
return `${command} ${escapedArgs.join(" ")}`; return `${command} ${escapedArgs.join(" ")}`;
@ -48,6 +52,11 @@ function resolveCommandPath(command) {
} }
export async function safeSpawn(command, args, options = {}) { 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) => { return new Promise((resolve, reject) => {
// Windows requires shell: true because .bat and .cmd files are not executable // 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 // without a terminal. On Unix/macOS, we resolve the full path first, then use

View file

@ -4,9 +4,11 @@ import assert from "node:assert";
describe("safeSpawn", () => { describe("safeSpawn", () => {
let safeSpawn; let safeSpawn;
let spawnCalls = []; let spawnCalls = [];
let os;
beforeEach(async () => { beforeEach(async () => {
spawnCalls = []; spawnCalls = [];
os = "win32"; // Test Windows behavior by default
// 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", {
@ -35,7 +37,7 @@ describe("safeSpawn", () => {
mock.module("os", { mock.module("os", {
namedExports: { namedExports: {
platform: () => "win32", platform: () => os,
}, },
}); });
@ -184,4 +186,29 @@ describe("safeSpawn", () => {
'cmd safe "needs space" "\\$dangerous" also-safe' '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");
});
}); });