diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index cb85c89..8efa000 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -9,10 +9,46 @@ import path from "node:path"; import ini from "ini"; /** - * @param {string} command - * @param {string[]} args - * - * @returns {Promise<{status: number}>} + * Sets fallback CA bundle environment variables used by Python libraries. + * These are applied in addition to the PIP_CONFIG_FILE to ensure all Python + * network libraries respect the combined CA bundle, even if they don't read pip's config. + * + * @param {NodeJS.ProcessEnv} env - Environment object to modify + * @param {string} combinedCaPath - Path to the combined CA bundle + */ +function setFallbackCaBundleEnvironmentVariables(env, combinedCaPath) { + // REQUESTS_CA_BUNDLE: Used by the popular 'requests' library + if (env.REQUESTS_CA_BUNDLE) { + ui.writeWarning("Safe-chain: User defined REQUESTS_CA_BUNDLE found in environment. It will be overwritten."); + } + env.REQUESTS_CA_BUNDLE = combinedCaPath; + + // SSL_CERT_FILE: Used by some Python SSL libraries and urllib + if (env.SSL_CERT_FILE) { + ui.writeWarning("Safe-chain: User defined SSL_CERT_FILE found in environment. It will be overwritten."); + } + env.SSL_CERT_FILE = combinedCaPath; + + // PIP_CERT: Pip's own environment variable for certificate verification + if (env.PIP_CERT) { + ui.writeWarning("Safe-chain: User defined PIP_CERT found in environment. It will be overwritten."); + } + env.PIP_CERT = combinedCaPath; +} + +/** + * Runs a pip command with safe-chain's certificate bundle and proxy configuration. + * + * Creates a temporary pip config file (cleaned up automatically after execution) to configure: + * - Certificate bundle for HTTPS verification + * - Proxy settings if available + * + * If the user has an existing PIP_CONFIG_FILE, a new temporary config is created that merges + * their settings with safe-chain's, leaving the original file unchanged. + * + * @param {string} command - The pip command to execute (e.g., 'pip3') + * @param {string[]} args - Command line arguments to pass to pip + * @returns {Promise<{status: number}>} Exit status of the pip command */ export async function runPip(command, args) { try { @@ -26,12 +62,15 @@ export async function runPip(command, args) { // https://pip.pypa.io/en/stable/topics/https-certificates/ explains that the 'cert' param (which we're providing via INI file) // will tell pip to use the provided CA bundle for HTTPS verification. - // Proxy settings: prefer GLOBAL_AGENT_HTTP_PROXY, then HTTPS_PROXY, then HTTP_PROXY + // Proxy settings: GLOBAL_AGENT_HTTP_PROXY is our safe-chain proxy (if active), + // otherwise fall back to user-defined HTTPS_PROXY or HTTP_PROXY environment variables const proxy = env.GLOBAL_AGENT_HTTP_PROXY || env.HTTPS_PROXY || env.HTTP_PROXY || ''; const tmpDir = os.tmpdir(); const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); + let cleanupConfigPath = null; // Track temp file for cleanup + // Note: Setting PIP_CONFIG_FILE overrides all pip config levels (Global/User/Site) per pip's loading order if (!env.PIP_CONFIG_FILE) { /** @type {{ global: { cert: string, proxy?: string } }} */ const configObj = { global: { cert: combinedCaPath } }; @@ -41,6 +80,7 @@ export async function runPip(command, args) { const pipConfig = ini.stringify(configObj); await fs.writeFile(pipConfigPath, pipConfig); env.PIP_CONFIG_FILE = pipConfigPath; + cleanupConfigPath = pipConfigPath; } else if (fsSync.existsSync(env.PIP_CONFIG_FILE)) { ui.writeVerbose("Safe-chain: Merging user provided PIP_CONFIG_FILE with safe-chain certificate and proxy settings."); @@ -72,32 +112,31 @@ export async function runPip(command, args) { // Save to a new temp file to avoid overwriting user's original config await fs.writeFile(pipConfigPath, updated, "utf-8"); env.PIP_CONFIG_FILE = pipConfigPath; + cleanupConfigPath = pipConfigPath; } else { // The user provided PIP_CONFIG_FILE does not exist on disk // PIP will handle this as an error and inform the user } - // REQUESTS_CA_BUNDLE, SSL_CERT_FILE and PIP_CERT as extra safety nets. - if (env.REQUESTS_CA_BUNDLE) { - ui.writeWarning("Safe-chain: User defined REQUESTS_CA_BUNDLE found in environment. It will be overwritten."); - } - env.REQUESTS_CA_BUNDLE = combinedCaPath; - - if (env.SSL_CERT_FILE) { - ui.writeWarning("Safe-chain: User defined SSL_CERT_FILE found in environment. It will be overwritten."); - } - env.SSL_CERT_FILE = combinedCaPath; - - if (env.PIP_CERT) { - ui.writeWarning("Safe-chain: User defined PIP_CERT found in environment. It will be overwritten."); - } - env.PIP_CERT = combinedCaPath; + // Set fallback CA bundle environment variables for Python libraries that don't read pip config + setFallbackCaBundleEnvironmentVariables(env, combinedCaPath); const result = await safeSpawn(command, args, { stdio: "inherit", env, }); + + // Cleanup temporary config file if we created one + if (cleanupConfigPath) { + try { + await fs.unlink(cleanupConfigPath); + } catch (error) { + // Ignore cleanup errors - the file may have already been deleted or is inaccessible + // Temp files in os.tmpdir() may eventually be cleaned by the OS, but timing varies by platform + } + } + return { status: result.status }; } catch (/** @type any */ error) { if (error.status) { diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index 4af0b40..5d88b0e 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -9,15 +9,25 @@ describe("runPipCommand environment variable handling", () => { let runPip; let capturedArgs = null; let customEnv = null; + let capturedConfigContent = null; // Capture config file content before cleanup beforeEach(async () => { capturedArgs = null; + capturedConfigContent = null; - // Mock safeSpawn to capture args + // Mock safeSpawn to capture args and config file content before cleanup mock.module("../../utils/safeSpawn.js", { namedExports: { safeSpawn: async (command, args, options) => { capturedArgs = { command, args, options }; + // Capture the config file content before the function cleans it up + if (options.env.PIP_CONFIG_FILE) { + try { + capturedConfigContent = await fs.readFile(options.env.PIP_CONFIG_FILE, "utf-8"); + } catch (e) { + // Ignore if file doesn't exist or can't be read + } + } return { status: 0 }; }, }, @@ -151,9 +161,9 @@ describe("runPipCommand environment variable handling", () => { const originalParsed = ini.parse(originalContent); assert.strictEqual(originalParsed.global.cert, undefined, "original file should not gain cert"); - // New file has merged settings - const newContent = await fs.readFile(newCfgPath, "utf-8"); - const newParsed = ini.parse(newContent); + // New file has merged settings (read from captured content before cleanup) + assert.ok(capturedConfigContent, "config content should have been captured"); + const newParsed = ini.parse(capturedConfigContent); assert.strictEqual(newParsed.global.cert, "/tmp/test-combined-ca.pem", "new config should include cert"); assert.strictEqual(newParsed.global.proxy, "http://localhost:8080", "new config should include proxy from env"); assert.strictEqual(newParsed.global["index-url"], "https://example.com/simple", "index-url should be preserved"); @@ -166,8 +176,8 @@ describe("runPipCommand environment variable handling", () => { assert.strictEqual(res.status, 0); const configPath = capturedArgs.options.env.PIP_CONFIG_FILE; - const content = await fs.readFile(configPath, "utf-8"); - const parsed = ini.parse(content); + assert.ok(capturedConfigContent, "config content should have been captured"); + const parsed = ini.parse(capturedConfigContent); assert.ok(parsed.global, "[global] should exist after creation"); assert.strictEqual( parsed.global.proxy, @@ -198,8 +208,9 @@ describe("runPipCommand environment variable handling", () => { assert.strictEqual(originalParsed.global.cert, undefined, "original file should not gain cert"); assert.strictEqual(originalParsed.global.proxy, "http://original:9999", "original proxy remains"); - // New file: cert and proxy always overwritten - const newParsed = ini.parse(await fs.readFile(newCfgPath, "utf-8")); + // New file: cert and proxy always overwritten (read from captured content) + assert.ok(capturedConfigContent, "config content should have been captured"); + const newParsed = ini.parse(capturedConfigContent); assert.strictEqual(newParsed.global.cert, "/tmp/test-combined-ca.pem", "cert always overwritten in temp config"); assert.strictEqual(newParsed.global.proxy, "http://localhost:8080", "proxy always overwritten in temp config"); customEnv = null; @@ -228,9 +239,9 @@ describe("runPipCommand environment variable handling", () => { assert.strictEqual(originalParsed.global.cert, "/path/to/existing.pem", "original cert preserved"); assert.strictEqual(originalParsed.global.proxy, "http://original:9999", "original proxy preserved"); - // New temp config: cert and proxy always overwritten - const newContent = await fs.readFile(newCfgPath, "utf-8"); - const newParsed = ini.parse(newContent); + // New temp config: cert and proxy always overwritten (read from captured content) + assert.ok(capturedConfigContent, "config content should have been captured"); + const newParsed = ini.parse(capturedConfigContent); assert.strictEqual(newParsed.global.cert, "/tmp/test-combined-ca.pem", "cert always overwritten in temp config"); assert.strictEqual(newParsed.global.proxy, "http://localhost:8080", "proxy always overwritten in temp config"); customEnv = null; @@ -253,8 +264,9 @@ describe("runPipCommand environment variable handling", () => { assert.strictEqual(originalParsed.global.cert, "/path/to/existing.pem", "original cert unchanged"); assert.strictEqual(originalParsed.global.proxy, undefined, "original proxy still missing"); - // New file: cert and proxy always overwritten - const newParsed = ini.parse(await fs.readFile(newCfgPath, "utf-8")); + // New file: cert and proxy always overwritten (read from captured content) + assert.ok(capturedConfigContent, "config content should have been captured"); + const newParsed = ini.parse(capturedConfigContent); assert.strictEqual(newParsed.global.cert, "/tmp/test-combined-ca.pem", "cert always overwritten in temp config"); assert.strictEqual(newParsed.global.proxy, "http://localhost:8080", "proxy always overwritten in temp config"); customEnv = null; @@ -271,8 +283,8 @@ describe("runPipCommand environment variable handling", () => { ].join("\n"); await fs.writeFile(cfgPath, initialIni, "utf-8"); - process.env.PIP_CONFIG_FILE = cfgPath; - const mod = await import("./runPipCommand.js"); + customEnv = { PIP_CONFIG_FILE: cfgPath }; + // Capture stdout/stderr let output = ""; const originalWrite = process.stdout.write; @@ -280,14 +292,13 @@ describe("runPipCommand environment variable handling", () => { process.stdout.write = (chunk, ...args) => { output += chunk; return originalWrite.apply(process.stdout, [chunk, ...args]); }; process.stderr.write = (chunk, ...args) => { output += chunk; return originalError.apply(process.stderr, [chunk, ...args]); }; - await mod.runPip("pip3", ["install", "requests"]); + await runPip("pip3", ["install", "requests"]); process.stdout.write = originalWrite; process.stderr.write = originalError; assert.ok(output.includes("cert found in PIP_CONFIG_FILE"), "Should warn about cert overwrite in output"); assert.ok(output.includes("proxy found in PIP_CONFIG_FILE"), "Should warn about proxy overwrite in output"); - delete process.env.PIP_CONFIG_FILE; customEnv = null; }); });