Call safeSpawn iso safeSpawnPy

This commit is contained in:
Reinier Criel 2025-10-28 13:38:31 -07:00
parent ccd59a2f17
commit b886bb1cfe
3 changed files with 44 additions and 142 deletions

View file

@ -1,11 +1,11 @@
import { ui } from "../../environment/userInteraction.js"; import { ui } from "../../environment/userInteraction.js";
import { safeSpawnPy } from "../../utils/safeSpawn.js"; import { safeSpawn } from "../../utils/safeSpawn.js";
import { mergeSafeChainProxyEnvironmentVariables } from "../../registryProxy/registryProxy.js"; import { mergeSafeChainProxyEnvironmentVariables } from "../../registryProxy/registryProxy.js";
export async function runPip(command, args) { export async function runPip(command, args) {
try { try {
const result = await safeSpawnPy(command, args, { const result = await safeSpawn(command, args, {
stdio: "inherit", stdio: "inherit",
env: mergeSafeChainProxyEnvironmentVariables(process.env), env: mergeSafeChainProxyEnvironmentVariables(process.env),
}); });

View file

@ -98,65 +98,3 @@ export async function safeSpawn(command, args, options = {}) {
}); });
}); });
} }
/**
* To avoid any regression issues on the JS ecosystem,
* a py-friendly safeSpawn that avoids shell interpolation
* issues (e.g., '<', '>' in version specs).
*
* TL;DR: add support for shell::false
*/
export async function safeSpawnPy(command, args, options = {}) {
// 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) => {
let cmdToRun = command;
if (os.platform() !== "win32") {
try {
cmdToRun = resolveCommandPath(command);
} catch (e) {
if (options.stdio === "inherit") {
process.stderr.write(
`Error: Command '${command}' not found. Please ensure it is installed and available in your PATH.\n`
);
}
return resolve({ status: 1, stdout: "", stderr: e.message || String(e) });
}
}
const child = spawn(cmdToRun, args, { ...options, shell: false });
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, stderr });
});
child.on("error", (error) => {
// When stdio is inherited and spawn fails (e.g., command not found),
// we need to write the error to stderr manually since there's no child process
if (options.stdio === "inherit") {
if (error.code === "ENOENT") {
process.stderr.write(`Error: Command '${command}' not found. Please ensure it is installed and available in your PATH.\n`);
} else {
process.stderr.write(`Error: ${error.message}\n`);
}
}
resolve({ status: 1, stdout: "", stderr: error.message || String(error) });
});
});
}

View file

@ -13,8 +13,13 @@ 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: {
spawn: (command, options) => { spawn: (command, argsOrOptions, options) => {
spawnCalls.push({ command, options }); // Handle both signatures: spawn(cmd, {opts}) and spawn(cmd, [args], {opts})
if (Array.isArray(argsOrOptions)) {
spawnCalls.push({ command, args: argsOrOptions, options: options || {} });
} else {
spawnCalls.push({ command, options: argsOrOptions || {} });
}
return { return {
on: (event, callback) => { on: (event, callback) => {
if (event === "close") { if (event === "close") {
@ -211,84 +216,43 @@ describe("safeSpawn", () => {
assert.strictEqual(spawnCalls.length, 1); assert.strictEqual(spawnCalls.length, 1);
assert.strictEqual(spawnCalls[0].command, "valid_command-123"); assert.strictEqual(spawnCalls[0].command, "valid_command-123");
}); });
});
describe("safeSpawnPy", () => { it("should handle Python version specifiers with comparison operators on Windows", async () => {
let safeSpawnPy; os = "win32";
let spawnCalls = []; await safeSpawn("pip3", ["install", "Jinja2>=3.1,<3.2"]);
beforeEach(async () => {
spawnCalls = [];
// Mock child_process for argument-array spawn signature
mock.module("child_process", {
namedExports: {
spawn: (command, args = [], options = {}) => {
spawnCalls.push({ command, args, options });
const stdoutListeners = [];
const stderrListeners = [];
const stdout = { on: (event, cb) => { if (event === "data") stdoutListeners.push(cb); } };
const stderr = { on: (event, cb) => { if (event === "data") stderrListeners.push(cb); } };
const obj = {
stdout,
stderr,
on: (event, callback) => {
if (event === 'close') {
// Emit one chunk to stdout and stderr to verify piping works, then close with success
setTimeout(() => {
stdoutListeners.forEach((cb) => cb(Buffer.from("STDOUT-TEST")));
stderrListeners.forEach((cb) => cb(Buffer.from("")));
callback(0);
}, 0);
}
}
};
return obj;
},
// Provide execSync so the module under test can import it without ESM errors.
// We don't actually execute it in safeSpawnPy flows, but Node's module loader
// validates the presence of the named export during import.
execSync: (cmd) => {
// Minimal stub: emulate `command -v <cmd>` returning a path
const match = /command -v (.*)/.exec(String(cmd) || "");
const bin = match?.[1] || "mockbin";
return `/usr/bin/${bin}\n`;
},
},
});
// Import after mocking; use a query to avoid ESM cache collisions with previous import
const safeSpawnModule = await import("./safeSpawn.js?py");
safeSpawnPy = safeSpawnModule.safeSpawnPy;
});
afterEach(() => {
mock.reset();
});
it("spawns without a shell and preserves args (inherit)", async () => {
const result = await safeSpawnPy("pip3", ["install", "Jinja2>=3.1,<3.2"], { stdio: "inherit" });
// Verifies no throw and status 0
assert.strictEqual(result.status, 0);
// Verify spawn signature
assert.strictEqual(spawnCalls.length, 1);
// Allow either bare command or resolved full path
assert.match(spawnCalls[0].command, /(^|\/)pip3$/);
assert.deepStrictEqual(spawnCalls[0].args, ["install", "Jinja2>=3.1,<3.2"]);
assert.strictEqual(spawnCalls[0].options.shell, false);
assert.strictEqual(spawnCalls[0].options.stdio, "inherit");
});
it("captures stdout when stdio=pipe", async () => {
const result = await safeSpawnPy("pip3", ["install", "idna!=3.5,>=3.0", "--dry-run"], { stdio: "pipe" });
assert.strictEqual(result.status, 0);
assert.match(result.stdout || "", /STDOUT-TEST/);
assert.strictEqual(spawnCalls.length, 1); assert.strictEqual(spawnCalls.length, 1);
assert.strictEqual(spawnCalls[0].options.shell, false); // On Windows, args are built into a command string with proper escaping
assert.strictEqual(spawnCalls[0].options.stdio, "pipe"); assert.strictEqual(spawnCalls[0].command, 'pip3 install "Jinja2>=3.1,<3.2"');
assert.strictEqual(spawnCalls[0].options.shell, true);
});
it("should handle Python version specifiers with comparison operators on Unix", async () => {
os = "darwin"; // or "linux"
await safeSpawn("pip3", ["install", "Jinja2>=3.1,<3.2"]);
assert.strictEqual(spawnCalls.length, 1);
// On Unix, resolves full path and passes args as array (no shell interpretation)
assert.strictEqual(spawnCalls[0].command, "/usr/bin/pip3");
assert.deepStrictEqual(spawnCalls[0].args, ["install", "Jinja2>=3.1,<3.2"]);
assert.deepStrictEqual(spawnCalls[0].options, {});
});
it("should handle Python not-equal version specifiers", async () => {
os = "win32";
await safeSpawn("pip3", ["install", "idna!=3.5,>=3.0"]);
assert.strictEqual(spawnCalls.length, 1);
assert.strictEqual(spawnCalls[0].command, 'pip3 install "idna!=3.5,>=3.0"');
assert.strictEqual(spawnCalls[0].options.shell, true);
});
it("should handle Python extras with square brackets", async () => {
os = "win32";
await safeSpawn("pip3", ["install", "requests[socks]"]);
assert.strictEqual(spawnCalls.length, 1);
assert.strictEqual(spawnCalls[0].command, 'pip3 install "requests[socks]"');
assert.strictEqual(spawnCalls[0].options.shell, true);
}); });
}); });