diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 4de1512..e8d8d26 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -7,10 +7,16 @@ export async function runPip(command, args) { try { const env = mergeSafeChainProxyEnvironmentVariables(process.env); - // Pass --cert with our CA to pip so it trusts our MITM for known registries. - // pip will append this to its default CA bundle, so it still validates - // non-registry HTTPS (GitHub, custom mirrors) against system CAs. - const finalArgs = [...args, "--cert", getCaCertPath()]; + // If the user already provided --cert, respect their choice and do not override. + // Support both "--cert " and "--cert=" forms. + const hasUserCert = args.some((a, i) => { + if (a === "--cert") return true; + return typeof a === "string" && a.startsWith("--cert="); + }); + + // By default, pass --cert with our CA so pip trusts our MITM for known registries. + // Note: pip treats --cert as the CA bundle to use for TLS (it does not merge with system CAs). + const finalArgs = hasUserCert ? [...args] : [...args, "--cert", getCaCertPath()]; const result = await safeSpawn(command, finalArgs, { 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 52a4148..f9ef43d 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -60,4 +60,28 @@ describe("runPipCommand --cert handling", () => { assert.strictEqual(capturedArgs.args[0], "install"); assert.strictEqual(capturedArgs.args[1], "requests"); }); + + it("should not override user-provided --cert ", async () => { + const res = await runPip("pip3", ["install", "requests", "--cert", "/tmp/user-ca.pem"]); + assert.strictEqual(res.status, 0); + + // Ensure only the user-provided --cert is present + const certIndices = capturedArgs.args + .map((a, i) => (a === "--cert" ? i : -1)) + .filter((i) => i >= 0); + assert.strictEqual(certIndices.length, 1, "should not inject an extra --cert"); + const userPath = capturedArgs.args[certIndices[0] + 1]; + assert.strictEqual(userPath, "/tmp/user-ca.pem", "should preserve user-provided cert path"); + }); + + it("should not override user-provided --cert=", async () => { + const res = await runPip("pip3", ["install", "requests", "--cert=/tmp/user-ca.pem"]); + assert.strictEqual(res.status, 0); + + // Ensure args contain the inline --cert= and no extra --cert token + const hasInline = capturedArgs.args.some((a) => typeof a === "string" && a.startsWith("--cert=")); + assert.ok(hasInline, "should keep inline --cert="); + const injectedIndex = capturedArgs.args.indexOf("--cert"); + assert.strictEqual(injectedIndex, -1, "should not inject separate --cert when inline is provided"); + }); }); diff --git a/test/e2e/pip.e2e.spec.js b/test/e2e/pip.e2e.spec.js index 7013121..05bdde9 100644 --- a/test/e2e/pip.e2e.spec.js +++ b/test/e2e/pip.e2e.spec.js @@ -193,4 +193,73 @@ describe("E2E: pip coverage", () => { ); }); + it(`pip3 can install from GitHub URL using system CAs`, async () => { + const shell = await container.openShell("zsh"); + // Install a simple package from GitHub - this should use TCP tunnel, not MITM + // Using a popular, small package for testing + const result = await shell.runCommand('pip3 install --break-system-packages git+https://github.com/psf/requests.git@v2.32.3'); + + assert.ok( + result.output.includes("no malicious packages found."), + `Output did not include expected text. Output was:\n${result.output}` + ); + + // Verify installation succeeded (would fail if certificate validation broke) + assert.ok( + result.output.includes("Successfully installed") || result.output.includes("Requirement already satisfied"), + `Installation from GitHub failed - system CAs may not be working. Output was:\n${result.output}` + ); + + // Verify package was actually installed + const listResult = await shell.runCommand("pip3 list"); + assert.ok( + listResult.output.includes("requests"), + `Package from GitHub was not installed. Output was:\n${listResult.output}` + ); + }); + + it(`pip3 successfully validates certificates for HTTPS downloads`, async () => { + const shell = await container.openShell("zsh"); + // Clear cache to force network download through proxy + await shell.runCommand("pip3 cache purge"); + + const result = await shell.runCommand('pip3 install --break-system-packages certifi'); + + assert.ok( + result.output.includes("no malicious packages found."), + `Output did not include expected text. Output was:\n${result.output}` + ); + + // Verify successful installation (would fail with SSL/certificate errors if --cert wasn't working) + assert.ok( + result.output.includes("Successfully installed"), + `Installation should succeed with proper certificate validation. Output was:\n${result.output}` + ); + + // Should NOT contain SSL or certificate errors + assert.ok( + !result.output.match(/SSL|certificate verify failed|CERTIFICATE_VERIFY_FAILED/i), + `Should not have SSL/certificate errors. Output was:\n${result.output}` + ); + }); + + it(`pip3 handles external HTTPS correctly (e.g., downloading from CDN)`, async () => { + const shell = await container.openShell("zsh"); + // Test installing from a direct HTTPS URL (not a registry) + // This validates that non-registry HTTPS traffic works with system CAs + const result = await shell.runCommand('pip3 install --break-system-packages https://files.pythonhosted.org/packages/70/8e/0e2d847013cb52cd35b38c009bb167a1a26b2ce6cd6965bf26b47bc0bf44/requests-2.31.0-py3-none-any.whl'); + + assert.ok( + result.output.includes("no malicious packages found."), + `Output did not include expected text. Output was:\n${result.output}` + ); + + // Since this is from pythonhosted.org, it should be MITM'd by safe-chain + // But the certificate validation should still work + assert.ok( + result.output.includes("Successfully installed") || result.output.includes("Requirement already satisfied"), + `Installation from direct HTTPS URL failed. Output was:\n${result.output}` + ); + }); + });