From 61c9f1a1efe6a173f2872a5aa70770918082b572 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 13 Nov 2025 11:14:45 -0800 Subject: [PATCH] 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", }