Fix some issues

This commit is contained in:
Reinier Criel 2025-12-08 15:18:06 -08:00 committed by Sander Declerck
parent 3de53e1f8a
commit 7b5a700655
No known key found for this signature in database
4 changed files with 78 additions and 79 deletions

View file

@ -93,7 +93,7 @@ export async function runPip(command, args) {
try { try {
const env = mergeSafeChainProxyEnvironmentVariables(process.env); const env = mergeSafeChainProxyEnvironmentVariables(process.env);
// Always provide Python with a complete CA bundle (Safe Chain CA + Mozilla + Node built-in roots) // Always provide Python with a complete CA bundle (Safe Chain CA + Mozilla + Node built-in roots + user certs)
// so that any network request made by pip, including those outside explicit CLI args, // so that any network request made by pip, including those outside explicit CLI args,
// validates correctly under both MITM'd and tunneled HTTPS. // validates correctly under both MITM'd and tunneled HTTPS.
const combinedCaPath = getCombinedCaBundlePath(); const combinedCaPath = getCombinedCaBundlePath();

View file

@ -48,16 +48,16 @@ function isParsable(pem) {
let cachedPath = null; let cachedPath = null;
/** /**
* Build a combined CA bundle for Python and Node HTTPS flows. * Build a combined CA bundle.
* - Includes Safe Chain CA (for MITM of known registries) * Automatically includes:
* - Includes Mozilla roots via npm `certifi` (public HTTPS) * - Safe Chain CA (for MITM of known registries)
* - Includes Node's built-in root certificates as a portable fallback * - Mozilla roots via certifi (for public HTTPS)
* - Node's built-in root certificates (fallback)
* - User's custom certificates (if NODE_EXTRA_CA_CERTS environment variable is set)
*
* @returns {string} Path to the combined CA bundle PEM file * @returns {string} Path to the combined CA bundle PEM file
*/ */
export function getCombinedCaBundlePath() { export function getCombinedCaBundlePath() {
if (cachedPath && fs.existsSync(cachedPath)) return cachedPath;
// Concatenate PEM files
const parts = []; const parts = [];
// 1) Safe Chain CA (for MITM'd registries) // 1) Safe Chain CA (for MITM'd registries)
@ -90,11 +90,23 @@ export function getCombinedCaBundlePath() {
// Ignore if unavailable // Ignore if unavailable
} }
// 4) User's NODE_EXTRA_CA_CERTS (if set)
const userCertPath = process.env.NODE_EXTRA_CA_CERTS;
if (userCertPath) {
const userPem = readUserCertificateFile(userCertPath);
if (userPem) {
parts.push(userPem.trim());
ui.writeVerbose(`Safe-chain: Merging user's NODE_EXTRA_CA_CERTS from ${userCertPath}`);
} else {
ui.writeWarning(`Safe-chain: Could not read or parse user's NODE_EXTRA_CA_CERTS from ${userCertPath}`);
}
}
const combined = parts.filter(Boolean).join("\n"); const combined = parts.filter(Boolean).join("\n");
const target = path.join(os.tmpdir(), "safe-chain-ca-bundle.pem"); const target = path.join(os.tmpdir(), `safe-chain-ca-bundle-${Date.now()}.pem`);
fs.writeFileSync(target, combined, { encoding: "utf8" }); fs.writeFileSync(target, combined, { encoding: "utf8" });
cachedPath = target; cachedPath = target;
return cachedPath; return target;
} }
/** /**
@ -166,38 +178,4 @@ function readUserCertificateFile(certPath) {
} }
} }
/**
* Combine user's existing NODE_EXTRA_CA_CERTS with Safe Chain's CA certificate.
* If user has NODE_EXTRA_CA_CERTS set, it's merged with Safe Chain CA.
*
* @param {string | undefined} userCertPath - User's existing NODE_EXTRA_CA_CERTS path (if any)
* @returns {string} Path to the final CA bundle
*/
export function getCombinedCaBundlePathWithUserCerts(userCertPath) {
const parts = [];
// 1) Safe Chain CA
const safeChainPath = getCaCertPath();
try {
const safeChainPem = fs.readFileSync(safeChainPath, "utf8");
if (isParsable(safeChainPem)) parts.push(safeChainPem.trim());
} catch {
// Ignore if Safe Chain CA is not available
}
// 2) User's certificates
if (userCertPath) {
const userPem = readUserCertificateFile(userCertPath);
if (userPem) {
parts.push(userPem.trim());
ui.writeVerbose(`Safe-chain: Merging user's NODE_EXTRA_CA_CERTS from ${userCertPath}`);
} else {
ui.writeWarning(`Safe-chain: Could not read or parse user's NODE_EXTRA_CA_CERTS from ${userCertPath}`);
}
}
const finalCombined = parts.filter(Boolean).join("\n");
const target = path.join(os.tmpdir(), `safe-chain-ca-bundle-${Date.now()}.pem`);
fs.writeFileSync(target, finalCombined, { encoding: "utf8" });
return target;
}

View file

@ -77,12 +77,13 @@ describe("certBundle.getCombinedCaBundlePath", () => {
}); });
}); });
describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { describe("certBundle.getCombinedCaBundlePath with user certs", () => {
beforeEach(() => { beforeEach(() => {
mock.restoreAll(); mock.restoreAll();
delete process.env.NODE_EXTRA_CA_CERTS;
}); });
it("returns a path with Safe Chain CA when no user cert provided", async () => { it("returns a path with full CA bundle (Safe Chain + Mozilla + Node roots) when no user cert in env", async () => {
// Mock getCaCertPath to return valid cert // Mock getCaCertPath to return valid cert
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-"));
const safeChainPath = path.join(tmpDir, "safechain.pem"); const safeChainPath = path.join(tmpDir, "safechain.pem");
@ -94,15 +95,17 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts(undefined); const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain certificate blocks");
// Should include base bundle (Safe Chain + Mozilla/Node roots)
assert.ok(contents.length > 1000, "Bundle should be substantial with Mozilla/Node roots included");
}); });
it("merges user cert with Safe Chain CA", async () => { it("merges user cert with full base bundle (Safe Chain CA + Mozilla + Node roots)", async () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-"));
// Create Safe Chain CA // Create Safe Chain CA
@ -114,6 +117,7 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
const userCertPath = path.join(tmpDir, "user-cert.pem"); const userCertPath = path.join(tmpDir, "user-cert.pem");
const userCert = getValidCert(); const userCert = getValidCert();
fs.writeFileSync(userCertPath, userCert, "utf8"); fs.writeFileSync(userCertPath, userCert, "utf8");
process.env.NODE_EXTRA_CA_CERTS = userCertPath;
mock.module("./certUtils.js", { mock.module("./certUtils.js", {
namedExports: { namedExports: {
@ -121,8 +125,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
@ -136,6 +140,7 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-"));
const safeChainPath = path.join(tmpDir, "safechain.pem"); const safeChainPath = path.join(tmpDir, "safechain.pem");
fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); fs.writeFileSync(safeChainPath, getValidCert(), "utf8");
process.env.NODE_EXTRA_CA_CERTS = "/nonexistent/path.pem";
mock.module("./certUtils.js", { mock.module("./certUtils.js", {
namedExports: { namedExports: {
@ -143,8 +148,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts("/nonexistent/path.pem"); const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
@ -159,7 +164,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); fs.writeFileSync(safeChainPath, getValidCert(), "utf8");
const userCertPath = path.join(tmpDir, "invalid.pem"); const userCertPath = path.join(tmpDir, "invalid.pem");
fs.writeFileSync(userCertPath, "NOT A VALID CERTIFICATE", "utf8"); fs.writeFileSync(userCertPath, "NOT A VALID PEM", "utf8");
process.env.NODE_EXTRA_CA_CERTS = userCertPath;
mock.module("./certUtils.js", { mock.module("./certUtils.js", {
namedExports: { namedExports: {
@ -167,8 +173,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
@ -188,8 +194,9 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts("../../../etc/passwd"); process.env.NODE_EXTRA_CA_CERTS = "../../../etc/passwd";
const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
@ -221,8 +228,9 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts(symlinkPath); process.env.NODE_EXTRA_CA_CERTS = symlinkPath;
const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
@ -245,8 +253,9 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts(certDir); process.env.NODE_EXTRA_CA_CERTS = certDir;
const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
@ -265,8 +274,9 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts(" "); process.env.NODE_EXTRA_CA_CERTS = " ";
const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
@ -280,8 +290,10 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
// Create a real file with CRLF content to test Windows line ending support // Create a real file with CRLF content to test Windows line ending support
const userCertPath = path.join(tmpDir, "user-cert-crlf.pem"); const userCertPath = path.join(tmpDir, "user-cert-crlf.pem");
const crlfCert = getValidCert().replace(/\n/g, "\r\n"); const userCert = getValidCert();
fs.writeFileSync(userCertPath, crlfCert, "utf8"); const certWithCRLF = userCert.replace(/\n/g, "\r\n");
fs.writeFileSync(userCertPath, certWithCRLF, "utf8");
process.env.NODE_EXTRA_CA_CERTS = userCertPath;
mock.module("./certUtils.js", { mock.module("./certUtils.js", {
namedExports: { namedExports: {
@ -289,8 +301,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length; const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length;
@ -308,7 +320,7 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
// Test that Windows path syntax is recognized (even if files don't exist on macOS/Linux) // Test that Windows path syntax is recognized (even if files don't exist on macOS/Linux)
// These should gracefully fail (return Safe Chain CA only) rather than crash // These should gracefully fail (return Safe Chain CA only) rather than crash
@ -319,7 +331,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
]; ];
for (const winPath of winPaths) { for (const winPath of winPaths) {
const bundlePath = getCombinedCaBundlePathWithUserCerts(winPath); process.env.NODE_EXTRA_CA_CERTS = winPath;
const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), `Bundle should exist for ${winPath}`); assert.ok(fs.existsSync(bundlePath), `Bundle should exist for ${winPath}`);
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA");
@ -337,7 +350,7 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
}, },
}); });
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); const { getCombinedCaBundlePath } = await import("./certBundle.js");
// Test various Windows-style traversal attempts // Test various Windows-style traversal attempts
const traversalPaths = [ const traversalPaths = [
@ -347,13 +360,20 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
"../../../etc/passwd", // Unix-style for comparison "../../../etc/passwd", // Unix-style for comparison
]; ];
// First, get baseline bundle without user certs to know expected cert count
delete process.env.NODE_EXTRA_CA_CERTS;
const baselineBundlePath = getCombinedCaBundlePath();
const baselineContents = fs.readFileSync(baselineBundlePath, "utf8");
const baselineCertCount = (baselineContents.match(/-----BEGIN CERTIFICATE-----/g) || []).length;
for (const badPath of traversalPaths) { for (const badPath of traversalPaths) {
const bundlePath = getCombinedCaBundlePathWithUserCerts(badPath); process.env.NODE_EXTRA_CA_CERTS = badPath;
const bundlePath = getCombinedCaBundlePath();
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
const contents = fs.readFileSync(bundlePath, "utf8"); const contents = fs.readFileSync(bundlePath, "utf8");
// Only Safe Chain CA should be present (user cert rejected due to traversal) // Should contain base bundle (Safe Chain + Mozilla + Node roots) but NOT user cert
const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length; const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length;
assert.strictEqual(certCount, 1, `Traversal path ${badPath} should be rejected; only Safe Chain CA included`); assert.strictEqual(certCount, baselineCertCount, `Traversal path ${badPath} should be rejected; base bundle only (no user cert added)`);
} }
}); });
}); });

View file

@ -2,7 +2,7 @@ import * as http from "http";
import { tunnelRequest } from "./tunnelRequestHandler.js"; import { tunnelRequest } from "./tunnelRequestHandler.js";
import { mitmConnect } from "./mitmRequestHandler.js"; import { mitmConnect } from "./mitmRequestHandler.js";
import { handleHttpProxyRequest } from "./plainHttpProxy.js"; import { handleHttpProxyRequest } from "./plainHttpProxy.js";
import { getCombinedCaBundlePathWithUserCerts } from "./certBundle.js"; import { getCombinedCaBundlePath } from "./certBundle.js";
import { ui } from "../environment/userInteraction.js"; import { ui } from "../environment/userInteraction.js";
import chalk from "chalk"; import chalk from "chalk";
import { createInterceptorForUrl } from "./interceptors/createInterceptorForEcoSystem.js"; import { createInterceptorForUrl } from "./interceptors/createInterceptorForEcoSystem.js";
@ -37,8 +37,7 @@ function getSafeChainProxyEnvironmentVariables() {
} }
const proxyUrl = `http://localhost:${state.port}`; const proxyUrl = `http://localhost:${state.port}`;
const userNodeExtraCaCerts = process.env.NODE_EXTRA_CA_CERTS; const caCertPath = getCombinedCaBundlePath();
const caCertPath = getCombinedCaBundlePathWithUserCerts(userNodeExtraCaCerts);
return { return {
HTTPS_PROXY: proxyUrl, HTTPS_PROXY: proxyUrl,
@ -121,7 +120,9 @@ function stopServer(server) {
} catch { } catch {
resolve(); resolve();
} }
setTimeout(() => resolve(), SERVER_STOP_TIMEOUT_MS); setTimeout(() => {
resolve();
}, SERVER_STOP_TIMEOUT_MS);
}); });
} }