From 9b102412af95b1f992b550524686d16eb2640941 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Tue, 11 Nov 2025 10:37:39 -0800 Subject: [PATCH 01/23] Add extra ENV vars --- .../src/packagemanager/pip/runPipCommand.js | 31 +++++++++++++++++++ .../packagemanager/pip/runPipCommand.spec.js | 20 ++++++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 793302d..4da7ab4 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -2,6 +2,10 @@ import { ui } from "../../environment/userInteraction.js"; import { safeSpawn } from "../../utils/safeSpawn.js"; import { mergeSafeChainProxyEnvironmentVariables } from "../../registryProxy/registryProxy.js"; import { getCombinedCaBundlePath } from "../../registryProxy/certBundle.js"; +import { knownPipRegistries } from "../../registryProxy/parsePackageFromUrl.js"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; /** * @param {string} command @@ -20,6 +24,33 @@ export async function runPip(command, args) { env.REQUESTS_CA_BUNDLE = combinedCaPath; env.SSL_CERT_FILE = combinedCaPath; + // To counter behavior that is sometimes seen where pip ignores REQUESTS_CA_BUNDLE/SSL_CERT_FILE, + // 1. Set additional env vars for pip + // 2. Create a pip config file that specifies the cert and trusted hosts + + env.PIP_CERT = combinedCaPath; + + // Create a temporary pip config file + const tmpDir = os.tmpdir(); + const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); + + // Trusted hosts: use knownPipRegistries from parsePackageFromUrl + const trustedHosts = Array.from(new Set(knownPipRegistries)); + + // Proxy settings + const httpProxy = env.HTTP_PROXY || ''; + const httpsProxy = env.HTTPS_PROXY || ''; + + // Build pip config INI + let pipConfig = '[global]\n'; + pipConfig += `cert = ${combinedCaPath}\n`; + if (httpProxy) pipConfig += `proxy = ${httpProxy}\n`; + if (httpsProxy) pipConfig += `proxy = ${httpsProxy}\n`; + if (trustedHosts.length) pipConfig += `trusted-host = ${trustedHosts.join(' ')}\n`; + + await fs.writeFile(pipConfigPath, pipConfig); + env.PIP_CONFIG_FILE = pipConfigPath; + const result = await safeSpawn(command, args, { stdio: "inherit", env, diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index d7a0f93..9d330da 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -43,6 +43,23 @@ describe("runPipCommand environment variable handling", () => { mock.reset(); }); + it("should set PIP_CERT env var and create config file", async () => { + const res = await runPip("pip3", ["install", "requests"]); + assert.strictEqual(res.status, 0); + assert.ok(capturedArgs, "safeSpawn should have been called"); + // Check PIP_CERT env var + assert.strictEqual( + capturedArgs.options.env.PIP_CERT, + "/tmp/test-combined-ca.pem", + "PIP_CERT should be set to combined bundle path" + ); + // Check PIP_CONFIG_FILE env var exists and is a non-empty string + const configPath = capturedArgs.options.env.PIP_CONFIG_FILE; + assert.ok(configPath, "PIP_CONFIG_FILE should be set"); + assert.strictEqual(typeof configPath, "string", "PIP_CONFIG_FILE should be a string"); + assert.ok(configPath.length > 0, "PIP_CONFIG_FILE should be a non-empty path"); + }); + it("should set REQUESTS_CA_BUNDLE and SSL_CERT_FILE for default PyPI (no explicit index)", async () => { const res = await runPip("pip3", ["install", "requests"]); assert.strictEqual(res.status, 0); @@ -60,9 +77,6 @@ describe("runPipCommand environment variable handling", () => { "/tmp/test-combined-ca.pem", "SSL_CERT_FILE should be set to combined bundle path" ); - - // Args should be unchanged (no arg injection) - assert.deepStrictEqual(capturedArgs.args, ["install", "requests"]); }); it("should set CA environment variables even for external/test PyPI mirror (covers non-CLI traffic)", async () => { From 6a94271a101b523ccc9fa585272a5c65910d85bb Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Tue, 11 Nov 2025 14:28:31 -0800 Subject: [PATCH 02/23] Do not add list of trusted hosts, is security risk --- .../safe-chain/src/packagemanager/pip/runPipCommand.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 4da7ab4..30dade4 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -25,18 +25,13 @@ export async function runPip(command, args) { env.SSL_CERT_FILE = combinedCaPath; // To counter behavior that is sometimes seen where pip ignores REQUESTS_CA_BUNDLE/SSL_CERT_FILE, - // 1. Set additional env vars for pip - // 2. Create a pip config file that specifies the cert and trusted hosts - + // We will set additional env vars for pip env.PIP_CERT = combinedCaPath; // Create a temporary pip config file const tmpDir = os.tmpdir(); const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); - // Trusted hosts: use knownPipRegistries from parsePackageFromUrl - const trustedHosts = Array.from(new Set(knownPipRegistries)); - // Proxy settings const httpProxy = env.HTTP_PROXY || ''; const httpsProxy = env.HTTPS_PROXY || ''; @@ -46,7 +41,6 @@ export async function runPip(command, args) { pipConfig += `cert = ${combinedCaPath}\n`; if (httpProxy) pipConfig += `proxy = ${httpProxy}\n`; if (httpsProxy) pipConfig += `proxy = ${httpsProxy}\n`; - if (trustedHosts.length) pipConfig += `trusted-host = ${trustedHosts.join(' ')}\n`; await fs.writeFile(pipConfigPath, pipConfig); env.PIP_CONFIG_FILE = pipConfigPath; From f9d241e4747558de313806a31e76a870338ea7a9 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Tue, 11 Nov 2025 14:32:12 -0800 Subject: [PATCH 03/23] Fix unused import --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 30dade4..2e8bcbf 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -2,7 +2,6 @@ import { ui } from "../../environment/userInteraction.js"; import { safeSpawn } from "../../utils/safeSpawn.js"; import { mergeSafeChainProxyEnvironmentVariables } from "../../registryProxy/registryProxy.js"; import { getCombinedCaBundlePath } from "../../registryProxy/certBundle.js"; -import { knownPipRegistries } from "../../registryProxy/parsePackageFromUrl.js"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; From 6bcd3d3b8f08dc051a3edb382e4aca8ed4d48bb3 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Tue, 11 Nov 2025 15:22:06 -0800 Subject: [PATCH 04/23] Make sure we don't override any environments --- .../src/packagemanager/pip/runPipCommand.js | 42 ++++++++++++------- .../packagemanager/pip/runPipCommand.spec.js | 23 +++++++++- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 2e8bcbf..5024655 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -20,29 +20,39 @@ export async function runPip(command, args) { // so that any network request made by pip, including those outside explicit CLI args, // validates correctly under both MITM'd and tunneled HTTPS. const combinedCaPath = getCombinedCaBundlePath(); - env.REQUESTS_CA_BUNDLE = combinedCaPath; - env.SSL_CERT_FILE = combinedCaPath; + if (!env.REQUESTS_CA_BUNDLE) { + env.REQUESTS_CA_BUNDLE = combinedCaPath; + } + + if (!env.SSL_CERT_FILE) { + env.SSL_CERT_FILE = combinedCaPath; + } + // To counter behavior that is sometimes seen where pip ignores REQUESTS_CA_BUNDLE/SSL_CERT_FILE, // We will set additional env vars for pip - env.PIP_CERT = combinedCaPath; + if (!env.PIP_CERT) { + env.PIP_CERT = combinedCaPath; + } - // Create a temporary pip config file - const tmpDir = os.tmpdir(); - const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); + // Only create and set PIP_CONFIG_FILE if not already set + if (!env.PIP_CONFIG_FILE) { + const tmpDir = os.tmpdir(); + const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); - // Proxy settings - const httpProxy = env.HTTP_PROXY || ''; - const httpsProxy = env.HTTPS_PROXY || ''; + // Proxy settings + const httpProxy = env.HTTP_PROXY || ''; + const httpsProxy = env.HTTPS_PROXY || ''; - // Build pip config INI - let pipConfig = '[global]\n'; - pipConfig += `cert = ${combinedCaPath}\n`; - if (httpProxy) pipConfig += `proxy = ${httpProxy}\n`; - if (httpsProxy) pipConfig += `proxy = ${httpsProxy}\n`; + // Build pip config INI + let pipConfig = '[global]\n'; + pipConfig += `cert = ${combinedCaPath}\n`; + if (httpProxy) pipConfig += `proxy = ${httpProxy}\n`; + if (httpsProxy) pipConfig += `proxy = ${httpsProxy}\n`; - await fs.writeFile(pipConfigPath, pipConfig); - env.PIP_CONFIG_FILE = pipConfigPath; + await fs.writeFile(pipConfigPath, pipConfig); + env.PIP_CONFIG_FILE = pipConfigPath; + } const result = await safeSpawn(command, args, { stdio: "inherit", diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index 9d330da..7128bde 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -4,6 +4,7 @@ import assert from "node:assert"; describe("runPipCommand environment variable handling", () => { let runPip; let capturedArgs = null; + let customEnv = null; beforeEach(async () => { capturedArgs = null; @@ -18,11 +19,12 @@ describe("runPipCommand environment variable handling", () => { }, }); - // Mock proxy env merge + // Mock proxy env merge, allow custom env override mock.module("../../registryProxy/registryProxy.js", { namedExports: { mergeSafeChainProxyEnvironmentVariables: (env) => ({ ...env, + ...(customEnv || {}), HTTPS_PROXY: "http://localhost:8080", }), }, @@ -43,6 +45,25 @@ describe("runPipCommand environment variable handling", () => { mock.reset(); }); + it("should not overwrite existing env vars for certs and config", async () => { + // Set custom env vars before merge + customEnv = { + REQUESTS_CA_BUNDLE: "/custom/ca-bundle.pem", + SSL_CERT_FILE: "/custom/ssl-cert.pem", + PIP_CERT: "/custom/pip-cert.pem", + PIP_CONFIG_FILE: "/custom/pip.conf" + }; + const res = await runPip("pip3", ["install", "requests"]); + assert.strictEqual(res.status, 0); + assert.ok(capturedArgs, "safeSpawn should have been called"); + // Should preserve custom env vars + assert.strictEqual(capturedArgs.options.env.REQUESTS_CA_BUNDLE, "/custom/ca-bundle.pem"); + assert.strictEqual(capturedArgs.options.env.SSL_CERT_FILE, "/custom/ssl-cert.pem"); + assert.strictEqual(capturedArgs.options.env.PIP_CERT, "/custom/pip-cert.pem"); + assert.strictEqual(capturedArgs.options.env.PIP_CONFIG_FILE, "/custom/pip.conf"); + customEnv = null; + }); + it("should set PIP_CERT env var and create config file", async () => { const res = await runPip("pip3", ["install", "requests"]); assert.strictEqual(res.status, 0); From a3d57cbd240e9d6f785cdc42096a90c6709116bb Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Tue, 11 Nov 2025 15:24:59 -0800 Subject: [PATCH 05/23] Cleanup --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 5024655..f2eccc5 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -24,18 +24,16 @@ export async function runPip(command, args) { if (!env.REQUESTS_CA_BUNDLE) { env.REQUESTS_CA_BUNDLE = combinedCaPath; } - if (!env.SSL_CERT_FILE) { env.SSL_CERT_FILE = combinedCaPath; } - + // To counter behavior that is sometimes seen where pip ignores REQUESTS_CA_BUNDLE/SSL_CERT_FILE, // We will set additional env vars for pip + if (!env.PIP_CERT) { env.PIP_CERT = combinedCaPath; } - - // Only create and set PIP_CONFIG_FILE if not already set if (!env.PIP_CONFIG_FILE) { const tmpDir = os.tmpdir(); const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); From f2bf5869ba0072243fa35ea4d87d8520253a56ec Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Tue, 11 Nov 2025 15:49:25 -0800 Subject: [PATCH 06/23] Fix linting issue --- .../safe-chain/src/packagemanager/pip/runPipCommand.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index 7128bde..627d909 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -24,7 +24,7 @@ describe("runPipCommand environment variable handling", () => { namedExports: { mergeSafeChainProxyEnvironmentVariables: (env) => ({ ...env, - ...(customEnv || {}), + ...customEnv, HTTPS_PROXY: "http://localhost:8080", }), }, From fdef9e0766aa603c09b5bfeee9acc49357b87fca Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Wed, 12 Nov 2025 13:11:02 -0800 Subject: [PATCH 07/23] Some tweaks --- .../safe-chain/src/packagemanager/pip/runPipCommand.js | 9 +++------ packages/safe-chain/src/registryProxy/certUtils.js | 4 ++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index f2eccc5..440c02b 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -30,7 +30,6 @@ export async function runPip(command, args) { // To counter behavior that is sometimes seen where pip ignores REQUESTS_CA_BUNDLE/SSL_CERT_FILE, // We will set additional env vars for pip - if (!env.PIP_CERT) { env.PIP_CERT = combinedCaPath; } @@ -38,15 +37,13 @@ export async function runPip(command, args) { const tmpDir = os.tmpdir(); const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); - // Proxy settings - const httpProxy = env.HTTP_PROXY || ''; - const httpsProxy = env.HTTPS_PROXY || ''; + // Proxy settings: prefer GLOBAL_AGENT_HTTP_PROXY, then HTTPS_PROXY, then HTTP_PROXY + const proxy = env.GLOBAL_AGENT_HTTP_PROXY || env.HTTPS_PROXY || env.HTTP_PROXY || ''; // Build pip config INI let pipConfig = '[global]\n'; pipConfig += `cert = ${combinedCaPath}\n`; - if (httpProxy) pipConfig += `proxy = ${httpProxy}\n`; - if (httpsProxy) pipConfig += `proxy = ${httpsProxy}\n`; + if (proxy) pipConfig += `proxy = ${proxy}\n`; await fs.writeFile(pipConfigPath, pipConfig); env.PIP_CONFIG_FILE = pipConfigPath; diff --git a/packages/safe-chain/src/registryProxy/certUtils.js b/packages/safe-chain/src/registryProxy/certUtils.js index a2fb7bb..aa23d79 100644 --- a/packages/safe-chain/src/registryProxy/certUtils.js +++ b/packages/safe-chain/src/registryProxy/certUtils.js @@ -48,6 +48,10 @@ export function generateCertForHost(hostname) { digitalSignature: true, keyEncipherment: true, }, + { + name: "extKeyUsage", + serverAuth: true, + }, ]); cert.sign(ca.privateKey, forge.md.sha256.create()); From f215368c4a97531e2875044347174145c04fb84b Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Wed, 12 Nov 2025 13:30:22 -0800 Subject: [PATCH 08/23] Some small fixes --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 3 +++ packages/safe-chain/src/registryProxy/certUtils.js | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 440c02b..12a3748 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -24,6 +24,7 @@ export async function runPip(command, args) { if (!env.REQUESTS_CA_BUNDLE) { env.REQUESTS_CA_BUNDLE = combinedCaPath; } + if (!env.SSL_CERT_FILE) { env.SSL_CERT_FILE = combinedCaPath; } @@ -33,6 +34,8 @@ export async function runPip(command, args) { if (!env.PIP_CERT) { env.PIP_CERT = combinedCaPath; } + + // PIP_CONFIG file is created to ensure proxy and cert settings are applied even if env vars are ignored for certificates (e.g. Python 3.11 and up). if (!env.PIP_CONFIG_FILE) { const tmpDir = os.tmpdir(); const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); diff --git a/packages/safe-chain/src/registryProxy/certUtils.js b/packages/safe-chain/src/registryProxy/certUtils.js index aa23d79..6b326c8 100644 --- a/packages/safe-chain/src/registryProxy/certUtils.js +++ b/packages/safe-chain/src/registryProxy/certUtils.js @@ -49,6 +49,12 @@ export function generateCertForHost(hostname) { keyEncipherment: true, }, { + /* + extKeyUsage serverAuth is required for TLS server authentication. + This is especially important for Python venv environments, which use their own + certificate validation logic and will reject certificates lacking the serverAuth EKU. + Adding serverAuth does not impact other usages + */ name: "extKeyUsage", serverAuth: true, }, From 285906ea9d610d9960928d1e41a10a109f531f27 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Wed, 12 Nov 2025 13:39:58 -0800 Subject: [PATCH 09/23] Update doc --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 12a3748..0f5e697 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -29,13 +29,12 @@ export async function runPip(command, args) { env.SSL_CERT_FILE = combinedCaPath; } - // To counter behavior that is sometimes seen where pip ignores REQUESTS_CA_BUNDLE/SSL_CERT_FILE, - // We will set additional env vars for pip + // https://pip.pypa.io/en/stable/topics/https-certificates/ explains that the --cert option (which we're providing via both INI and PIP_CERT) + // is necessary for pip to use custom certs for later versions. if (!env.PIP_CERT) { env.PIP_CERT = combinedCaPath; } - // PIP_CONFIG file is created to ensure proxy and cert settings are applied even if env vars are ignored for certificates (e.g. Python 3.11 and up). if (!env.PIP_CONFIG_FILE) { const tmpDir = os.tmpdir(); const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); From fbd11c6d443ea93ab90cd011a34f8078e3cffd31 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Wed, 12 Nov 2025 14:01:06 -0800 Subject: [PATCH 10/23] Update --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 0f5e697..f3e7aa9 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -24,13 +24,14 @@ export async function runPip(command, args) { if (!env.REQUESTS_CA_BUNDLE) { env.REQUESTS_CA_BUNDLE = combinedCaPath; } + + // https://pip.pypa.io/en/stable/topics/https-certificates/ explains that the --cert option (which we're providing via both INI and PIP_CERT) + // Testing has shown that REQUESTS_CA_BUNDLE alone is not sufficient; PIP_CERT, SSL_CERT_FILE, or pip config cert is also needed in some cases. if (!env.SSL_CERT_FILE) { env.SSL_CERT_FILE = combinedCaPath; } - // https://pip.pypa.io/en/stable/topics/https-certificates/ explains that the --cert option (which we're providing via both INI and PIP_CERT) - // is necessary for pip to use custom certs for later versions. if (!env.PIP_CERT) { env.PIP_CERT = combinedCaPath; } From 61c9f1a1efe6a173f2872a5aa70770918082b572 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 13 Nov 2025 11:14:45 -0800 Subject: [PATCH 11/23] Merge config file if it exists --- package-lock.json | 10 ++ packages/safe-chain-bun/package.json | 2 +- packages/safe-chain/package.json | 1 + .../src/packagemanager/pip/runPipCommand.js | 59 ++++++-- .../packagemanager/pip/runPipCommand.spec.js | 130 ++++++++++++++++++ test/e2e/DockerTestContainer.js | 2 +- 6 files changed, 193 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index a9c32df..068e544 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1090,6 +1090,15 @@ "node": ">=0.8.19" } }, + "node_modules/ini": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/ini/-/ini-6.0.0.tgz", + "integrity": "sha512-IBTdIkzZNOpqm7q3dRqJvMaldXjDHWkEDfrwGEQTs5eaQMWV+djAhR+wahyNNMAa+qpbDUhBMVt4ZKNwpPm7xQ==", + "license": "ISC", + "engines": { + "node": "^20.17.0 || >=22.9.0" + } + }, "node_modules/ip-address": { "version": "9.0.5", "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-9.0.5.tgz", @@ -2083,6 +2092,7 @@ "certifi": "^14.5.15", "chalk": "5.4.1", "https-proxy-agent": "7.0.6", + "ini": "^6.0.0", "make-fetch-happen": "14.0.3", "node-forge": "1.3.1", "npm-registry-fetch": "18.0.2", diff --git a/packages/safe-chain-bun/package.json b/packages/safe-chain-bun/package.json index b5a9e3e..ca154b8 100644 --- a/packages/safe-chain-bun/package.json +++ b/packages/safe-chain-bun/package.json @@ -27,4 +27,4 @@ "peerDependencies": { "bun": ">=1.2.21" } -} \ No newline at end of file +} diff --git a/packages/safe-chain/package.json b/packages/safe-chain/package.json index f21a372..5f8da60 100644 --- a/packages/safe-chain/package.json +++ b/packages/safe-chain/package.json @@ -38,6 +38,7 @@ "certifi": "^14.5.15", "chalk": "5.4.1", "https-proxy-agent": "7.0.6", + "ini": "^6.0.0", "make-fetch-happen": "14.0.3", "node-forge": "1.3.1", "npm-registry-fetch": "18.0.2", diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index f3e7aa9..96cdcf8 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -3,8 +3,10 @@ import { safeSpawn } from "../../utils/safeSpawn.js"; import { mergeSafeChainProxyEnvironmentVariables } from "../../registryProxy/registryProxy.js"; import { getCombinedCaBundlePath } from "../../registryProxy/certBundle.js"; import fs from "node:fs/promises"; +import fsSync from "node:fs"; import os from "node:os"; import path from "node:path"; +import ini from "ini"; /** * @param {string} command @@ -36,20 +38,59 @@ export async function runPip(command, args) { env.PIP_CERT = combinedCaPath; } - if (!env.PIP_CONFIG_FILE) { - const tmpDir = os.tmpdir(); - const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); + // Proxy settings: prefer GLOBAL_AGENT_HTTP_PROXY, then HTTPS_PROXY, then HTTP_PROXY + const proxy = env.GLOBAL_AGENT_HTTP_PROXY || env.HTTPS_PROXY || env.HTTP_PROXY || ''; - // Proxy settings: prefer GLOBAL_AGENT_HTTP_PROXY, then HTTPS_PROXY, then HTTP_PROXY - 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`); + + if (!env.PIP_CONFIG_FILE) { // Build pip config INI - let pipConfig = '[global]\n'; - pipConfig += `cert = ${combinedCaPath}\n`; - if (proxy) pipConfig += `proxy = ${proxy}\n`; - + /** @type {{ global: { cert: string, proxy?: string } }} */ + const configObj = { global: { cert: combinedCaPath } }; + if (proxy) { + configObj.global.proxy = proxy; + } + const pipConfig = ini.stringify(configObj); await fs.writeFile(pipConfigPath, pipConfig); env.PIP_CONFIG_FILE = pipConfigPath; + } else if (fsSync.existsSync(env.PIP_CONFIG_FILE)) { + // Existing pip config file present and exists on disk. + // Lets merge in our cert and proxy settings if not already present + const userConfig = env.PIP_CONFIG_FILE; + + ui.writeVerbose("Safe-chain: Merging user provided PIP_CONFIG_FILE with safe-chain certificate and proxy settings."); + + // Read the existing config without modifying it + let content = await fs.readFile(userConfig, "utf-8"); + const parsed = ini.parse(content); + + // Ensure [global] section exists + parsed.global = parsed.global || {}; + + // Adding CERT and PROXY + // If either is already set, there's no neeed to throw an error; mitm might fail and throw later if the proxy config is invalid + + // Cert + if (typeof parsed.global.cert === "undefined") { + ui.writeVerbose("Safe-chain: Adding cert to existing PIP_CONFIG_FILE."); + parsed.global.cert = combinedCaPath; + } + + // Proxy + if (typeof parsed.global.proxy === "undefined") { + if (proxy) { + ui.writeVerbose("Safe-chain: Adding proxy to existing PIP_CONFIG_FILE."); + parsed.global.proxy = proxy; + } + } + + const updated = ini.stringify(parsed); + + // 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; } const result = await safeSpawn(command, args, { diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index 627d909..b3d7b2e 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -1,5 +1,9 @@ import { describe, it, beforeEach, afterEach, mock } from "node:test"; import assert from "node:assert"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import ini from "ini"; describe("runPipCommand environment variable handling", () => { let runPip; @@ -145,4 +149,130 @@ describe("runPipCommand environment variable handling", () => { "HTTPS_PROXY should be set by proxy merge" ); }); + + it("should create a new temp config when existing config exists (original file untouched)", async () => { + const tmpDir = os.tmpdir(); + const userCfgPath = path.join(tmpDir, `safe-chain-test-pip-${Date.now()}.ini`); + const initial = "[global]\nindex-url = https://example.com/simple\n"; + await fs.writeFile(userCfgPath, initial, "utf-8"); + + customEnv = { PIP_CONFIG_FILE: userCfgPath }; + const res = await runPip("pip3", ["install", "requests"]); + assert.strictEqual(res.status, 0); + const newCfgPath = capturedArgs.options.env.PIP_CONFIG_FILE; + assert.notStrictEqual(newCfgPath, userCfgPath, "should point to a new temp config file"); + + // Original file unchanged + const originalContent = await fs.readFile(userCfgPath, "utf-8"); + 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); + 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"); + customEnv = null; + }); + + it("should create new config with proxy set from env (ini-validated)", async () => { + // No PIP_CONFIG_FILE in env => creation path + const res = await runPip("pip3", ["install", "requests"]); + 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(parsed.global, "[global] should exist after creation"); + assert.strictEqual( + parsed.global.proxy, + "http://localhost:8080", + "proxy should be set from merged env" + ); + assert.strictEqual( + parsed.global.cert, + "/tmp/test-combined-ca.pem", + "cert should be set during creation" + ); + }); + + it("should create new temp config adding cert but preserving existing proxy (original file unchanged)", async () => { + const tmpDir = os.tmpdir(); + const userCfgPath = path.join(tmpDir, `safe-chain-test-pip-${Date.now()}.ini`); + const initial = "[global]\nproxy = http://original:9999\n"; + await fs.writeFile(userCfgPath, initial, "utf-8"); + + customEnv = { PIP_CONFIG_FILE: userCfgPath }; + const res = await runPip("pip3", ["install", "requests"]); + assert.strictEqual(res.status, 0); + const newCfgPath = capturedArgs.options.env.PIP_CONFIG_FILE; + assert.notStrictEqual(newCfgPath, userCfgPath, "should use a new temp config file"); + + // Original file unchanged + const originalParsed = ini.parse(await fs.readFile(userCfgPath, "utf-8")); + 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 merged: cert added (was missing), proxy preserved (was present) + const newParsed = ini.parse(await fs.readFile(newCfgPath, "utf-8")); + assert.strictEqual(newParsed.global.cert, "/tmp/test-combined-ca.pem", "new cert injected"); + assert.strictEqual(newParsed.global.proxy, "http://original:9999", "existing proxy should be preserved in new file"); + customEnv = null; + }); + + it("should create new temp config preserving existing cert and proxy while leaving original file unchanged", async () => { + const tmpDir = os.tmpdir(); + const cfgPath = path.join(tmpDir, `safe-chain-test-pip-${Date.now()}.ini`); + const initialIni = [ + "[global]", + "cert = /path/to/existing.pem", + "proxy = http://original:9999", + "" + ].join("\n"); + await fs.writeFile(cfgPath, initialIni, "utf-8"); + + customEnv = { PIP_CONFIG_FILE: cfgPath }; + const res = await runPip("pip3", ["install", "requests"]); + assert.strictEqual(res.status, 0, "execution should succeed"); + const newCfgPath = capturedArgs.options.env.PIP_CONFIG_FILE; + assert.notStrictEqual(newCfgPath, cfgPath, "should use a newly generated temp config file"); + + // Original file stays untouched + const originalContent = await fs.readFile(cfgPath, "utf-8"); + const originalParsed = ini.parse(originalContent); + 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 preserves existing values (no override when already set) + const newContent = await fs.readFile(newCfgPath, "utf-8"); + const newParsed = ini.parse(newContent); + assert.strictEqual(newParsed.global.cert, "/path/to/existing.pem", "existing cert preserved in new temp config"); + assert.strictEqual(newParsed.global.proxy, "http://original:9999", "existing proxy preserved in new temp config"); + customEnv = null; + }); + + it("should create new temp config preserving existing cert and adding missing proxy", async () => { + const tmpDir = os.tmpdir(); + const userCfgPath = path.join(tmpDir, `safe-chain-test-pip-${Date.now()}.ini`); + const initial = "[global]\ncert = /path/to/existing.pem\n"; + await fs.writeFile(userCfgPath, initial, "utf-8"); + + customEnv = { PIP_CONFIG_FILE: userCfgPath }; + const res = await runPip("pip3", ["install", "requests"]); + assert.strictEqual(res.status, 0); + const newCfgPath = capturedArgs.options.env.PIP_CONFIG_FILE; + assert.notStrictEqual(newCfgPath, userCfgPath, "should produce a new temp config file"); + + // Original remains unchanged + const originalParsed = ini.parse(await fs.readFile(userCfgPath, "utf-8")); + assert.strictEqual(originalParsed.global.cert, "/path/to/existing.pem", "original cert unchanged"); + assert.strictEqual(originalParsed.global.proxy, undefined, "original proxy still missing"); + + // New file preserves existing cert and adds proxy (since it was missing) + const newParsed = ini.parse(await fs.readFile(newCfgPath, "utf-8")); + assert.strictEqual(newParsed.global.cert, "/path/to/existing.pem", "existing cert preserved (not overridden)"); + assert.strictEqual(newParsed.global.proxy, "http://localhost:8080", "proxy added from env"); + customEnv = null; + }); }); diff --git a/test/e2e/DockerTestContainer.js b/test/e2e/DockerTestContainer.js index ec1af3c..289b451 100644 --- a/test/e2e/DockerTestContainer.js +++ b/test/e2e/DockerTestContainer.js @@ -33,7 +33,7 @@ export class DockerTestContainer { ].join(" "); execSync( - `docker build -t ${imageName} -f ${dockerFile} ${contextPath} ${buildArgs}`, + `docker build --no-cache -t ${imageName} -f ${dockerFile} ${contextPath} ${buildArgs}`, { stdio: "ignore", } From a0e24b172275ef450e14dd1625977645ee070465 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 13 Nov 2025 11:21:53 -0800 Subject: [PATCH 12/23] Update comments --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 96cdcf8..4198686 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -70,7 +70,9 @@ export async function runPip(command, args) { parsed.global = parsed.global || {}; // Adding CERT and PROXY - // If either is already set, there's no neeed to throw an error; mitm might fail and throw later if the proxy config is invalid + // If either is already set, there's no neeed to throw an error + // MITM might fail and throw later if the proxy config is invalid + // This ensures that no malware will be installed by safe-chain // Cert if (typeof parsed.global.cert === "undefined") { From 4ee18973dee7dc53ae8f764f3fc21056af002caa Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 13 Nov 2025 12:48:04 -0800 Subject: [PATCH 13/23] Fix unit test --- .../safe-chain/src/packagemanager/pip/runPipCommand.spec.js | 3 +++ test/e2e/DockerTestContainer.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index b3d7b2e..ecd8e0f 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -29,7 +29,10 @@ describe("runPipCommand environment variable handling", () => { mergeSafeChainProxyEnvironmentVariables: (env) => ({ ...env, ...customEnv, + // Force deterministic proxy for tests regardless of ambient env + GLOBAL_AGENT_HTTP_PROXY: "http://localhost:8080", HTTPS_PROXY: "http://localhost:8080", + HTTP_PROXY: "", }), }, }); diff --git a/test/e2e/DockerTestContainer.js b/test/e2e/DockerTestContainer.js index 289b451..ec1af3c 100644 --- a/test/e2e/DockerTestContainer.js +++ b/test/e2e/DockerTestContainer.js @@ -33,7 +33,7 @@ export class DockerTestContainer { ].join(" "); execSync( - `docker build --no-cache -t ${imageName} -f ${dockerFile} ${contextPath} ${buildArgs}`, + `docker build -t ${imageName} -f ${dockerFile} ${contextPath} ${buildArgs}`, { stdio: "ignore", } From f4ff18304aa50a1c201c41fe76b387400388670d Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 13 Nov 2025 13:20:11 -0800 Subject: [PATCH 14/23] Fix imports --- package-lock.json | 8 ++++++++ packages/safe-chain/package.json | 1 + 2 files changed, 9 insertions(+) diff --git a/package-lock.json b/package-lock.json index 068e544..94b6921 100644 --- a/package-lock.json +++ b/package-lock.json @@ -411,6 +411,13 @@ "node": ">=14" } }, + "node_modules/@types/ini": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/@types/ini/-/ini-4.1.1.tgz", + "integrity": "sha512-MIyNUZipBTbyUNnhvuXJTY7B6qNI78meck9Jbv3wk0OgNwRyOOVEKDutAkOs1snB/tx0FafyR6/SN4Ps0hZPeg==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/make-fetch-happen": { "version": "10.0.4", "resolved": "https://registry.npmjs.org/@types/make-fetch-happen/-/make-fetch-happen-10.0.4.tgz", @@ -2114,6 +2121,7 @@ "safe-chain": "bin/safe-chain.js" }, "devDependencies": { + "@types/ini": "^4.1.1", "@types/make-fetch-happen": "^10.0.4", "@types/node": "^18.19.130", "@types/node-forge": "^1.3.14", diff --git a/packages/safe-chain/package.json b/packages/safe-chain/package.json index 5f8da60..6394bc6 100644 --- a/packages/safe-chain/package.json +++ b/packages/safe-chain/package.json @@ -46,6 +46,7 @@ "semver": "7.7.2" }, "devDependencies": { + "@types/ini": "^4.1.1", "@types/make-fetch-happen": "^10.0.4", "@types/node": "^18.19.130", "@types/npm-registry-fetch": "^8.0.9", From 474d91d29ae1bb87741875274edfaa63d3501066 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 13 Nov 2025 13:32:49 -0800 Subject: [PATCH 15/23] Indentation --- packages/safe-chain/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/safe-chain/package.json b/packages/safe-chain/package.json index 6394bc6..186d810 100644 --- a/packages/safe-chain/package.json +++ b/packages/safe-chain/package.json @@ -46,7 +46,7 @@ "semver": "7.7.2" }, "devDependencies": { - "@types/ini": "^4.1.1", + "@types/ini": "^4.1.1", "@types/make-fetch-happen": "^10.0.4", "@types/node": "^18.19.130", "@types/npm-registry-fetch": "^8.0.9", From 0b3cc1c17543ebb864df0f0d72a8751dfd4756ff Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 13 Nov 2025 15:50:14 -0800 Subject: [PATCH 16/23] Some more cleanup --- .../src/packagemanager/pip/runPipCommand.js | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 4198686..9d68c81 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -23,20 +23,8 @@ export async function runPip(command, args) { // validates correctly under both MITM'd and tunneled HTTPS. const combinedCaPath = getCombinedCaBundlePath(); - if (!env.REQUESTS_CA_BUNDLE) { - env.REQUESTS_CA_BUNDLE = combinedCaPath; - } - - // https://pip.pypa.io/en/stable/topics/https-certificates/ explains that the --cert option (which we're providing via both INI and PIP_CERT) - // Testing has shown that REQUESTS_CA_BUNDLE alone is not sufficient; PIP_CERT, SSL_CERT_FILE, or pip config cert is also needed in some cases. - - if (!env.SSL_CERT_FILE) { - env.SSL_CERT_FILE = combinedCaPath; - } - - if (!env.PIP_CERT) { - env.PIP_CERT = combinedCaPath; - } + // https://pip.pypa.io/en/stable/topics/https-certificates/ explains that the --cert option (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 const proxy = env.GLOBAL_AGENT_HTTP_PROXY || env.HTTPS_PROXY || env.HTTP_PROXY || ''; @@ -45,7 +33,6 @@ export async function runPip(command, args) { const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); if (!env.PIP_CONFIG_FILE) { - // Build pip config INI /** @type {{ global: { cert: string, proxy?: string } }} */ const configObj = { global: { cert: combinedCaPath } }; @@ -55,6 +42,7 @@ export async function runPip(command, args) { const pipConfig = ini.stringify(configObj); await fs.writeFile(pipConfigPath, pipConfig); env.PIP_CONFIG_FILE = pipConfigPath; + } else if (fsSync.existsSync(env.PIP_CONFIG_FILE)) { // Existing pip config file present and exists on disk. // Lets merge in our cert and proxy settings if not already present @@ -72,18 +60,17 @@ export async function runPip(command, args) { // Adding CERT and PROXY // If either is already set, there's no neeed to throw an error // MITM might fail and throw later if the proxy config is invalid - // This ensures that no malware will be installed by safe-chain // Cert if (typeof parsed.global.cert === "undefined") { - ui.writeVerbose("Safe-chain: Adding cert to existing PIP_CONFIG_FILE."); + ui.writeVerbose("Safe-chain: Adding cert to temporary PIP_CONFIG_FILE."); parsed.global.cert = combinedCaPath; } // Proxy if (typeof parsed.global.proxy === "undefined") { if (proxy) { - ui.writeVerbose("Safe-chain: Adding proxy to existing PIP_CONFIG_FILE."); + ui.writeVerbose("Safe-chain: Adding proxy to temporary PIP_CONFIG_FILE."); parsed.global.proxy = proxy; } } @@ -93,6 +80,21 @@ 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; + + } 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) { + env.REQUESTS_CA_BUNDLE = combinedCaPath; + } + if (!env.SSL_CERT_FILE) { + env.SSL_CERT_FILE = combinedCaPath; + } + if (!env.PIP_CERT) { + env.PIP_CERT = combinedCaPath; } const result = await safeSpawn(command, args, { From 7039961d4ccd50054c55876cd8f7812838e485db Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 13 Nov 2025 15:50:37 -0800 Subject: [PATCH 17/23] Bugfix --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 9d68c81..309ee47 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -23,7 +23,7 @@ export async function runPip(command, args) { // validates correctly under both MITM'd and tunneled HTTPS. const combinedCaPath = getCombinedCaBundlePath(); - // https://pip.pypa.io/en/stable/topics/https-certificates/ explains that the --cert option (which we're providing via INI file) + // 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 @@ -46,9 +46,8 @@ export async function runPip(command, args) { } else if (fsSync.existsSync(env.PIP_CONFIG_FILE)) { // Existing pip config file present and exists on disk. // Lets merge in our cert and proxy settings if not already present - const userConfig = env.PIP_CONFIG_FILE; - ui.writeVerbose("Safe-chain: Merging user provided PIP_CONFIG_FILE with safe-chain certificate and proxy settings."); + const userConfig = env.PIP_CONFIG_FILE; // Read the existing config without modifying it let content = await fs.readFile(userConfig, "utf-8"); From 87fcb7239a34a19d9fbd65b3f0bf75ec839cb729 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Mon, 17 Nov 2025 10:03:38 -0800 Subject: [PATCH 18/23] Adapt per review --- .../src/packagemanager/pip/runPipCommand.js | 46 +++++++++---------- .../packagemanager/pip/runPipCommand.spec.js | 24 +++++----- 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 309ee47..f37e9b0 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -32,8 +32,7 @@ export async function runPip(command, args) { const tmpDir = os.tmpdir(); const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); - if (!env.PIP_CONFIG_FILE) { - // Build pip config INI + if (!env.PIP_CONFIG_FILE) { // Build pip config INI /** @type {{ global: { cert: string, proxy?: string } }} */ const configObj = { global: { cert: combinedCaPath } }; if (proxy) { @@ -43,9 +42,7 @@ export async function runPip(command, args) { await fs.writeFile(pipConfigPath, pipConfig); env.PIP_CONFIG_FILE = pipConfigPath; - } else if (fsSync.existsSync(env.PIP_CONFIG_FILE)) { - // Existing pip config file present and exists on disk. - // Lets merge in our cert and proxy settings if not already present + } else if (fsSync.existsSync(env.PIP_CONFIG_FILE)) { // Merge pip config INI ui.writeVerbose("Safe-chain: Merging user provided PIP_CONFIG_FILE with safe-chain certificate and proxy settings."); const userConfig = env.PIP_CONFIG_FILE; @@ -56,24 +53,20 @@ export async function runPip(command, args) { // Ensure [global] section exists parsed.global = parsed.global || {}; - // Adding CERT and PROXY - // If either is already set, there's no neeed to throw an error - // MITM might fail and throw later if the proxy config is invalid - // Cert - if (typeof parsed.global.cert === "undefined") { - ui.writeVerbose("Safe-chain: Adding cert to temporary PIP_CONFIG_FILE."); - parsed.global.cert = combinedCaPath; + if (typeof parsed.global.cert !== "undefined") { + ui.writeWarning("Safe-chain: User defined cert found in PIP_CONFIG_FILE. It will be overwritten in the temporary config."); } + parsed.global.cert = combinedCaPath; // Proxy - if (typeof parsed.global.proxy === "undefined") { - if (proxy) { - ui.writeVerbose("Safe-chain: Adding proxy to temporary PIP_CONFIG_FILE."); - parsed.global.proxy = proxy; - } + if (typeof parsed.global.proxy !== "undefined") { + ui.writeWarning("Safe-chain: User defined proxy found in PIP_CONFIG_FILE. It will be overwritten in the temporary config."); } - + if (proxy) { + parsed.global.proxy = proxy; + } + const updated = ini.stringify(parsed); // Save to a new temp file to avoid overwriting user's original config @@ -86,15 +79,20 @@ export async function runPip(command, args) { } // REQUESTS_CA_BUNDLE, SSL_CERT_FILE and PIP_CERT as extra safety nets. - if (!env.REQUESTS_CA_BUNDLE) { - env.REQUESTS_CA_BUNDLE = combinedCaPath; + if (env.REQUESTS_CA_BUNDLE) { + ui.writeWarning("Safe-chain: User defined REQUESTS_CA_BUNDLE found in environment. It will be overwritten."); } - if (!env.SSL_CERT_FILE) { - env.SSL_CERT_FILE = combinedCaPath; + 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."); } - if (!env.PIP_CERT) { - env.PIP_CERT = combinedCaPath; + 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; const result = await safeSpawn(command, args, { stdio: "inherit", diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index ecd8e0f..f67077a 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -217,10 +217,10 @@ 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 merged: cert added (was missing), proxy preserved (was present) + // New file: cert and proxy always overwritten const newParsed = ini.parse(await fs.readFile(newCfgPath, "utf-8")); - assert.strictEqual(newParsed.global.cert, "/tmp/test-combined-ca.pem", "new cert injected"); - assert.strictEqual(newParsed.global.proxy, "http://original:9999", "existing proxy should be preserved in new file"); + 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; }); @@ -247,11 +247,11 @@ 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 preserves existing values (no override when already set) - const newContent = await fs.readFile(newCfgPath, "utf-8"); - const newParsed = ini.parse(newContent); - assert.strictEqual(newParsed.global.cert, "/path/to/existing.pem", "existing cert preserved in new temp config"); - assert.strictEqual(newParsed.global.proxy, "http://original:9999", "existing proxy preserved in new temp config"); + // New temp config: cert and proxy always overwritten + const newContent = await fs.readFile(newCfgPath, "utf-8"); + const newParsed = ini.parse(newContent); + 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; }); @@ -272,10 +272,10 @@ 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 preserves existing cert and adds proxy (since it was missing) - const newParsed = ini.parse(await fs.readFile(newCfgPath, "utf-8")); - assert.strictEqual(newParsed.global.cert, "/path/to/existing.pem", "existing cert preserved (not overridden)"); - assert.strictEqual(newParsed.global.proxy, "http://localhost:8080", "proxy added from env"); + // New file: cert and proxy always overwritten + const newParsed = ini.parse(await fs.readFile(newCfgPath, "utf-8")); + 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; }); }); From 0e5b9b23f16a460631afe9fe27f9edb6bd56d9fd Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Mon, 17 Nov 2025 10:18:47 -0800 Subject: [PATCH 19/23] Fix tests --- .../packagemanager/pip/runPipCommand.spec.js | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index f67077a..4af0b40 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -52,25 +52,6 @@ describe("runPipCommand environment variable handling", () => { mock.reset(); }); - it("should not overwrite existing env vars for certs and config", async () => { - // Set custom env vars before merge - customEnv = { - REQUESTS_CA_BUNDLE: "/custom/ca-bundle.pem", - SSL_CERT_FILE: "/custom/ssl-cert.pem", - PIP_CERT: "/custom/pip-cert.pem", - PIP_CONFIG_FILE: "/custom/pip.conf" - }; - const res = await runPip("pip3", ["install", "requests"]); - assert.strictEqual(res.status, 0); - assert.ok(capturedArgs, "safeSpawn should have been called"); - // Should preserve custom env vars - assert.strictEqual(capturedArgs.options.env.REQUESTS_CA_BUNDLE, "/custom/ca-bundle.pem"); - assert.strictEqual(capturedArgs.options.env.SSL_CERT_FILE, "/custom/ssl-cert.pem"); - assert.strictEqual(capturedArgs.options.env.PIP_CERT, "/custom/pip-cert.pem"); - assert.strictEqual(capturedArgs.options.env.PIP_CONFIG_FILE, "/custom/pip.conf"); - customEnv = null; - }); - it("should set PIP_CERT env var and create config file", async () => { const res = await runPip("pip3", ["install", "requests"]); assert.strictEqual(res.status, 0); @@ -278,4 +259,35 @@ describe("runPipCommand environment variable handling", () => { assert.strictEqual(newParsed.global.proxy, "http://localhost:8080", "proxy always overwritten in temp config"); customEnv = null; }); + + it("should log warnings when cert and proxy are already set in user config file", async () => { + const tmpDir = os.tmpdir(); + const cfgPath = path.join(tmpDir, `safe-chain-test-pip-warn-${Date.now()}.ini`); + const initialIni = [ + "[global]", + "cert = /user/cert.pem", + "proxy = http://user-proxy:9999", + "" + ].join("\n"); + await fs.writeFile(cfgPath, initialIni, "utf-8"); + + process.env.PIP_CONFIG_FILE = cfgPath; + const mod = await import("./runPipCommand.js"); + // Capture stdout/stderr + let output = ""; + const originalWrite = process.stdout.write; + const originalError = process.stderr.write; + 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"]); + + 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; + }); }); From f030b16adf49620202141be8ac435ee1988a27c3 Mon Sep 17 00:00:00 2001 From: bitterpanda Date: Fri, 21 Nov 2025 13:33:33 +0100 Subject: [PATCH 20/23] rm obvious comments --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index f37e9b0..cb85c89 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -32,7 +32,7 @@ export async function runPip(command, args) { const tmpDir = os.tmpdir(); const pipConfigPath = path.join(tmpDir, `safe-chain-pip-${Date.now()}.ini`); - if (!env.PIP_CONFIG_FILE) { // Build pip config INI + if (!env.PIP_CONFIG_FILE) { /** @type {{ global: { cert: string, proxy?: string } }} */ const configObj = { global: { cert: combinedCaPath } }; if (proxy) { @@ -42,7 +42,7 @@ export async function runPip(command, args) { await fs.writeFile(pipConfigPath, pipConfig); env.PIP_CONFIG_FILE = pipConfigPath; - } else if (fsSync.existsSync(env.PIP_CONFIG_FILE)) { // Merge pip config INI + } 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."); const userConfig = env.PIP_CONFIG_FILE; From 0a0ac85542ab471e6b6d75e279e46238f9bf734d Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Fri, 21 Nov 2025 09:41:07 -0800 Subject: [PATCH 21/23] Adapt per review --- .../src/packagemanager/pip/runPipCommand.js | 79 ++++++++++++++----- .../packagemanager/pip/runPipCommand.spec.js | 45 +++++++---- 2 files changed, 87 insertions(+), 37 deletions(-) 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; }); }); From ab1aa0dce9f0f72a94b2d472694290288d1513b7 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Fri, 21 Nov 2025 09:58:43 -0800 Subject: [PATCH 22/23] Little cleanup --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 8efa000..3e83346 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -39,9 +39,9 @@ function setFallbackCaBundleEnvironmentVariables(env, 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 + * Creates a temporary pip config file to configure: + * - Cert bundle for HTTPS verification + * - Proxy settings * * 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. From 72bf44cb6d5b1f92a025605d757a20deb77095a0 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Fri, 21 Nov 2025 10:31:57 -0800 Subject: [PATCH 23/23] Fix linting issue --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 2 +- .../safe-chain/src/packagemanager/pip/runPipCommand.spec.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 3e83346..23485ff 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -131,7 +131,7 @@ export async function runPip(command, args) { if (cleanupConfigPath) { try { await fs.unlink(cleanupConfigPath); - } catch (error) { + } catch { // 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 } diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index 5d88b0e..d0df961 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -24,7 +24,7 @@ describe("runPipCommand environment variable handling", () => { if (options.env.PIP_CONFIG_FILE) { try { capturedConfigContent = await fs.readFile(options.env.PIP_CONFIG_FILE, "utf-8"); - } catch (e) { + } catch { // Ignore if file doesn't exist or can't be read } } @@ -175,7 +175,6 @@ describe("runPipCommand environment variable handling", () => { const res = await runPip("pip3", ["install", "requests"]); assert.strictEqual(res.status, 0); - const configPath = capturedArgs.options.env.PIP_CONFIG_FILE; assert.ok(capturedConfigContent, "config content should have been captured"); const parsed = ini.parse(capturedConfigContent); assert.ok(parsed.global, "[global] should exist after creation");