From 650dde4c84902dd96ddd548b405ce8e55ac1cfc4 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Fri, 12 Dec 2025 15:51:48 +0100 Subject: [PATCH 01/23] Remove mac unit test runner --- .github/workflows/test-on-pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-on-pr.yml b/.github/workflows/test-on-pr.yml index f754931..6680269 100644 --- a/.github/workflows/test-on-pr.yml +++ b/.github/workflows/test-on-pr.yml @@ -11,7 +11,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest, macos-latest] + os: [ubuntu-latest, windows-latest] steps: - name: Checkout code From 09809d29bcc9804df35a9303e615ee6e47f0fc96 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Mon, 15 Dec 2025 10:49:52 +0100 Subject: [PATCH 02/23] Refactor mocking in configFile.spec.js --- .github/workflows/test-on-pr.yml | 2 +- .../safe-chain/src/config/configFile.spec.js | 188 +++++++----------- 2 files changed, 69 insertions(+), 121 deletions(-) diff --git a/.github/workflows/test-on-pr.yml b/.github/workflows/test-on-pr.yml index 6680269..f754931 100644 --- a/.github/workflows/test-on-pr.yml +++ b/.github/workflows/test-on-pr.yml @@ -11,7 +11,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest, windows-latest, macos-latest] steps: - name: Checkout code diff --git a/packages/safe-chain/src/config/configFile.spec.js b/packages/safe-chain/src/config/configFile.spec.js index 18415bc..7da7e8d 100644 --- a/packages/safe-chain/src/config/configFile.spec.js +++ b/packages/safe-chain/src/config/configFile.spec.js @@ -1,32 +1,24 @@ import { describe, it, beforeEach, afterEach, mock } from "node:test"; import assert from "node:assert"; -describe("getScanTimeout", () => { +let configFileContent = undefined; +mock.module("fs", { + namedExports: { + existsSync: () => configFileContent !== undefined, + readFileSync: () => configFileContent, + writeFileSync: (content) => (configFileContent = content), + mkdirSync: () => {}, + }, +}); + +describe("getScanTimeout", async () => { let originalEnv; - let fsMock; - let getScanTimeout; + + const { getScanTimeout } = await import("./configFile.js"); beforeEach(async () => { // Save original environment originalEnv = process.env.AIKIDO_SCAN_TIMEOUT_MS; - - // Mock fs module - fsMock = { - existsSync: mock.fn(() => false), - readFileSync: mock.fn(() => "{}"), - writeFileSync: mock.fn(), - mkdirSync: mock.fn(), - }; - - mock.module("fs", { - namedExports: fsMock, - }); - - // Re-import the module to get the mocked version - const configFileModule = await import( - `./configFile.js?update=${Date.now()}` - ); - getScanTimeout = configFileModule.getScanTimeout; }); afterEach(() => { @@ -37,14 +29,12 @@ describe("getScanTimeout", () => { delete process.env.AIKIDO_SCAN_TIMEOUT_MS; } - // Reset all mocks - mock.restoreAll(); + configFileContent = undefined; }); it("should return default timeout of 10000ms when no config or env var is set", () => { delete process.env.AIKIDO_SCAN_TIMEOUT_MS; - // Mock: config file doesn't exist - fsMock.existsSync.mock.mockImplementation(() => false); + configFileContent = undefined; const timeout = getScanTimeout(); @@ -53,11 +43,7 @@ describe("getScanTimeout", () => { it("should return timeout from config file when set", () => { delete process.env.AIKIDO_SCAN_TIMEOUT_MS; - // Mock: config file exists with scanTimeout: 5000 - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: 5000 }) - ); + configFileContent = JSON.stringify({ scanTimeout: 5000 }); const timeout = getScanTimeout(); @@ -66,11 +52,7 @@ describe("getScanTimeout", () => { it("should prioritize environment variable over config file", () => { process.env.AIKIDO_SCAN_TIMEOUT_MS = "20000"; - // Mock: config file exists with scanTimeout: 5000 - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: 5000 }) - ); + configFileContent = JSON.stringify({ scanTimeout: 5000 }); const timeout = getScanTimeout(); @@ -79,11 +61,7 @@ describe("getScanTimeout", () => { it("should handle invalid environment variable and fall back to config", () => { process.env.AIKIDO_SCAN_TIMEOUT_MS = "invalid"; - // Mock: config file exists with scanTimeout: 7000 - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: 7000 }) - ); + configFileContent = JSON.stringify({ scanTimeout: 7000 }); const timeout = getScanTimeout(); @@ -91,8 +69,7 @@ describe("getScanTimeout", () => { }); it("should ignore zero and negative values and fall back to default", () => { - // Mock: config file doesn't exist - fsMock.existsSync.mock.mockImplementation(() => false); + configFileContent = undefined; process.env.AIKIDO_SCAN_TIMEOUT_MS = "0"; @@ -107,11 +84,7 @@ describe("getScanTimeout", () => { it("should ignore textual non-numeric values in environment variable and fall back to config", () => { process.env.AIKIDO_SCAN_TIMEOUT_MS = "fast"; - // Mock: config file exists with scanTimeout: 8000 - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: 8000 }) - ); + configFileContent = JSON.stringify({ scanTimeout: 8000 }); const timeout = getScanTimeout(); @@ -120,11 +93,7 @@ describe("getScanTimeout", () => { it("should ignore textual non-numeric values in config file and fall back to default", () => { delete process.env.AIKIDO_SCAN_TIMEOUT_MS; - // Mock: config file exists with scanTimeout: "slow" - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: "slow" }) - ); + configFileContent = JSON.stringify({ scanTimeout: "slow" }); const timeout = getScanTimeout(); @@ -133,11 +102,7 @@ describe("getScanTimeout", () => { it("should ignore textual non-numeric values in both env and config, fall back to default", () => { process.env.AIKIDO_SCAN_TIMEOUT_MS = "quick"; - // Mock: config file exists with scanTimeout: "medium" - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: "medium" }) - ); + configFileContent = JSON.stringify({ scanTimeout: "medium" }); const timeout = getScanTimeout(); @@ -146,11 +111,7 @@ describe("getScanTimeout", () => { it("should ignore mixed alphanumeric strings in environment variable", () => { process.env.AIKIDO_SCAN_TIMEOUT_MS = "5000ms"; - // Mock: config file exists with scanTimeout: 6000 - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: 6000 }) - ); + configFileContent = JSON.stringify({ scanTimeout: 6000 }); const timeout = getScanTimeout(); @@ -159,11 +120,7 @@ describe("getScanTimeout", () => { it("should ignore mixed alphanumeric strings in config file", () => { delete process.env.AIKIDO_SCAN_TIMEOUT_MS; - // Mock: config file exists with scanTimeout: "3000ms" - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: "3000ms" }) - ); + configFileContent = JSON.stringify({ scanTimeout: "3000ms" }); const timeout = getScanTimeout(); @@ -171,37 +128,15 @@ describe("getScanTimeout", () => { }); }); -describe("getMinimumPackageAgeHours", () => { - let fsMock; - let getMinimumPackageAgeHours; - - beforeEach(async () => { - // Mock fs module - fsMock = { - existsSync: mock.fn(() => false), - readFileSync: mock.fn(() => "{}"), - writeFileSync: mock.fn(), - mkdirSync: mock.fn(), - }; - - mock.module("fs", { - namedExports: fsMock, - }); - - // Re-import the module to get the mocked version - const configFileModule = await import( - `./configFile.js?update=${Date.now()}` - ); - getMinimumPackageAgeHours = configFileModule.getMinimumPackageAgeHours; - }); +describe("getMinimumPackageAgeHours", async () => { + const { getMinimumPackageAgeHours } = await import("./configFile.js"); afterEach(() => { - // Reset all mocks - mock.restoreAll(); + configFileContent = undefined; }); it("should return null when config file doesn't exist", () => { - fsMock.existsSync.mock.mockImplementation(() => false); + configFileContent = undefined; const hours = getMinimumPackageAgeHours(); @@ -209,10 +144,7 @@ describe("getMinimumPackageAgeHours", () => { }); it("should return null when config file exists but minimumPackageAgeHours is not set", () => { - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ scanTimeout: 5000 }) - ); + configFileContent = JSON.stringify({ scanTimeout: 5000 }); const hours = getMinimumPackageAgeHours(); @@ -220,10 +152,7 @@ describe("getMinimumPackageAgeHours", () => { }); it("should return value from config file when set to valid number", () => { - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ minimumPackageAgeHours: 48 }) - ); + configFileContent = JSON.stringify({ minimumPackageAgeHours: 48 }); const hours = getMinimumPackageAgeHours(); @@ -231,10 +160,7 @@ describe("getMinimumPackageAgeHours", () => { }); it("should handle string numbers in config file", () => { - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ minimumPackageAgeHours: "72" }) - ); + configFileContent = JSON.stringify({ minimumPackageAgeHours: "72" }); const hours = getMinimumPackageAgeHours(); @@ -242,10 +168,7 @@ describe("getMinimumPackageAgeHours", () => { }); it("should handle decimal values", () => { - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ minimumPackageAgeHours: 1.5 }) - ); + configFileContent = JSON.stringify({ minimumPackageAgeHours: 1.5 }); const hours = getMinimumPackageAgeHours(); @@ -253,21 +176,15 @@ describe("getMinimumPackageAgeHours", () => { }); it("should return null for non-numeric strings", () => { - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ minimumPackageAgeHours: "invalid" }) - ); + configFileContent = JSON.stringify({ minimumPackageAgeHours: "invalid" }); const hours = getMinimumPackageAgeHours(); assert.strictEqual(hours, undefined); }); - it("should return null for values with units suffix", () => { - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => - JSON.stringify({ minimumPackageAgeHours: "48h" }) - ); + it("should return undefined for values with units suffix", () => { + configFileContent = JSON.stringify({ minimumPackageAgeHours: "48h" }); const hours = getMinimumPackageAgeHours(); @@ -275,11 +192,42 @@ describe("getMinimumPackageAgeHours", () => { }); it("should handle malformed JSON and return null", () => { - fsMock.existsSync.mock.mockImplementation(() => true); - fsMock.readFileSync.mock.mockImplementation(() => "{ invalid json"); + configFileContent = "{ invalid json"; const hours = getMinimumPackageAgeHours(); assert.strictEqual(hours, undefined); }); + + it("should return 0 when minimumPackageAgeHours is set to 0", () => { + configFileContent = JSON.stringify({ minimumPackageAgeHours: 0 }); + + const hours = getMinimumPackageAgeHours(); + + assert.strictEqual(hours, 0); + }); + + it("should return 0 when minimumPackageAgeHours is set to string '0'", () => { + configFileContent = JSON.stringify({ minimumPackageAgeHours: "0" }); + + const hours = getMinimumPackageAgeHours(); + + assert.strictEqual(hours, 0); + }); + + it("should handle negative numeric values", () => { + configFileContent = JSON.stringify({ minimumPackageAgeHours: -24 }); + + const hours = getMinimumPackageAgeHours(); + + assert.strictEqual(hours, -24); + }); + + it("should handle negative string values", () => { + configFileContent = JSON.stringify({ minimumPackageAgeHours: "-48" }); + + const hours = getMinimumPackageAgeHours(); + + assert.strictEqual(hours, -48); + }); }); From 02c30a2544805edd404d7ae4e321deab8fe614d7 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Fri, 5 Dec 2025 14:23:57 -0800 Subject: [PATCH 03/23] Combine NODE_EXTRA_CA_CERTS with Safe Chain's certificate bundle --- .../src/registryProxy/certBundle.js | 66 +++++++++++++++++++ .../src/registryProxy/registryProxy.js | 7 +- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/certBundle.js b/packages/safe-chain/src/registryProxy/certBundle.js index 956279d..9b0c7bf 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.js +++ b/packages/safe-chain/src/registryProxy/certBundle.js @@ -6,6 +6,7 @@ import certifi from "certifi"; import tls from "node:tls"; import { X509Certificate } from "node:crypto"; import { getCaCertPath } from "./certUtils.js"; +import { ui } from "../environment/userInteraction.js"; /** * Check if a PEM string contains only parsable cert blocks. @@ -93,3 +94,68 @@ export function getCombinedCaBundlePath() { cachedPath = target; return cachedPath; } + +/** + * Read user certificate file. + * @param {string} certPath - Path to certificate file + * @returns {string | null} Certificate PEM content or null if invalid/unreadable + */ +function readUserCertificateFile(certPath) { + try { + // Validate path is a string and not attempting path traversal + if (typeof certPath !== "string" || certPath.includes("..") || certPath.startsWith("/")) { + return null; + } + + if (!fs.existsSync(certPath)) { + return null; + } + + const certPathAbsolute = path.resolve(certPath); + // Verify it's an absolute path (cross-platform) + if (!path.isAbsolute(certPathAbsolute)) { + return null; + } + + const content = fs.readFileSync(certPathAbsolute, "utf8"); + return content && isParsable(content) ? content : null; + } catch { + return null; + } +} + +/** + * 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) Add Safe Chain CA (for MITM'd registries) + 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) Add user's certificates if provided + if (userCertPath) { + const userPem = readUserCertificateFile(userCertPath); + if (userPem) { + parts.push(userPem.trim()); + ui.writeWarning(`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; +} diff --git a/packages/safe-chain/src/registryProxy/registryProxy.js b/packages/safe-chain/src/registryProxy/registryProxy.js index 497def8..9402830 100644 --- a/packages/safe-chain/src/registryProxy/registryProxy.js +++ b/packages/safe-chain/src/registryProxy/registryProxy.js @@ -2,7 +2,7 @@ import * as http from "http"; import { tunnelRequest } from "./tunnelRequestHandler.js"; import { mitmConnect } from "./mitmRequestHandler.js"; import { handleHttpProxyRequest } from "./plainHttpProxy.js"; -import { getCaCertPath } from "./certUtils.js"; +import { getCombinedCaBundlePathWithUserCerts } from "./certBundle.js"; import { ui } from "../environment/userInteraction.js"; import chalk from "chalk"; import { createInterceptorForUrl } from "./interceptors/createInterceptorForEcoSystem.js"; @@ -37,10 +37,13 @@ function getSafeChainProxyEnvironmentVariables() { } const proxyUrl = `http://localhost:${state.port}`; + const userNodeExtraCaCerts = process.env.NODE_EXTRA_CA_CERTS; + const caCertPath = getCombinedCaBundlePathWithUserCerts(userNodeExtraCaCerts); + return { HTTPS_PROXY: proxyUrl, GLOBAL_AGENT_HTTP_PROXY: proxyUrl, - NODE_EXTRA_CA_CERTS: getCaCertPath(), + NODE_EXTRA_CA_CERTS: caCertPath, }; } From 314001eb0c6920fc124905f3fe77ce6d5e0797c2 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Fri, 5 Dec 2025 15:13:12 -0800 Subject: [PATCH 04/23] Some improvements --- .../src/registryProxy/certBundle.js | 2 +- test/e2e/certbundle.e2e.spec.js | 347 ++++++++++++++++++ 2 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 test/e2e/certbundle.e2e.spec.js diff --git a/packages/safe-chain/src/registryProxy/certBundle.js b/packages/safe-chain/src/registryProxy/certBundle.js index 9b0c7bf..6dc9a51 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.js +++ b/packages/safe-chain/src/registryProxy/certBundle.js @@ -103,7 +103,7 @@ export function getCombinedCaBundlePath() { function readUserCertificateFile(certPath) { try { // Validate path is a string and not attempting path traversal - if (typeof certPath !== "string" || certPath.includes("..") || certPath.startsWith("/")) { + if (typeof certPath !== "string" || certPath.includes("..")) { return null; } diff --git a/test/e2e/certbundle.e2e.spec.js b/test/e2e/certbundle.e2e.spec.js new file mode 100644 index 0000000..a60dc3b --- /dev/null +++ b/test/e2e/certbundle.e2e.spec.js @@ -0,0 +1,347 @@ +import { describe, it, before, beforeEach, afterEach } from "node:test"; +import { DockerTestContainer } from "./DockerTestContainer.js"; +import assert from "node:assert"; + +describe("E2E: NODE_EXTRA_CA_CERTS merging", () => { + let container; + + before(async () => { + DockerTestContainer.buildImage(); + }); + + beforeEach(async () => { + // Run a new Docker container for each test + container = new DockerTestContainer(); + await container.start(); + + const installationShell = await container.openShell("zsh"); + await installationShell.runCommand("safe-chain setup"); + }); + + afterEach(async () => { + // Stop and clean up the container after each test + if (container) { + await container.stop(); + container = null; + } + }); + + it(`npm install works without NODE_EXTRA_CA_CERTS set`, async () => { + const shell = await container.openShell("zsh"); + + // Ensure NODE_EXTRA_CA_CERTS is not set + await shell.runCommand("unset NODE_EXTRA_CA_CERTS"); + + const result = await shell.runCommand("npm install axios"); + + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed without NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`npm install works with valid NODE_EXTRA_CA_CERTS set`, async () => { + const shell = await container.openShell("zsh"); + + // Create a temporary valid certificate (using the system's Mozilla CA bundle) + await shell.runCommand("cp /etc/ssl/certs/ca-certificates.crt /tmp/valid-certs.pem"); + + // Verify the cert file was created + const { output: checkOutput } = await shell.runCommand("test -f /tmp/valid-certs.pem && echo exists"); + assert.ok( + checkOutput.includes("exists"), + `Certificate file was not created at /tmp/valid-certs.pem` + ); + + // Set NODE_EXTRA_CA_CERTS and run npm install + const result = await shell.runCommand( + "NODE_EXTRA_CA_CERTS=/tmp/valid-certs.pem npm install axios" + ); + + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`npm install works with non-existent NODE_EXTRA_CA_CERTS path`, async () => { + const shell = await container.openShell("zsh"); + + // Set NODE_EXTRA_CA_CERTS to a non-existent path + const result = await shell.runCommand( + 'export NODE_EXTRA_CA_CERTS="/tmp/nonexistent-certs.pem" && npm install axios' + ); + + // Should still succeed - safe-chain should gracefully handle missing user certs + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed with non-existent NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + + // Should show a warning + assert.ok( + result.output.includes("Safe-chain") || result.output.includes("Could not read"), + `Expected safe-chain warning about missing certs. Output was:\n${result.output}` + ); + }); + + it(`npm install works with invalid (non-PEM) NODE_EXTRA_CA_CERTS`, async () => { + const shell = await container.openShell("zsh"); + + // Create an invalid certificate file (not valid PEM) + await shell.runCommand( + 'echo "This is not a valid PEM certificate" > /tmp/invalid-certs.pem' + ); + + // Set NODE_EXTRA_CA_CERTS to invalid cert + const result = await shell.runCommand( + 'export NODE_EXTRA_CA_CERTS="/tmp/invalid-certs.pem" && npm install axios' + ); + + // Should still succeed - safe-chain should skip invalid user certs + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed with invalid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + + // Should show a warning about invalid cert + assert.ok( + result.output.includes("Safe-chain") || result.output.includes("Could not read"), + `Expected safe-chain warning about invalid certs. Output was:\n${result.output}` + ); + }); + + it(`npm install handles NODE_EXTRA_CA_CERTS with path traversal attempt`, async () => { + const shell = await container.openShell("zsh"); + + // Try to set NODE_EXTRA_CA_CERTS with path traversal + const result = await shell.runCommand( + 'export NODE_EXTRA_CA_CERTS="/tmp/../../../etc/passwd" && npm install axios' + ); + + // Should still succeed - safe-chain should reject path traversal + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed with path traversal NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`npm install handles empty NODE_EXTRA_CA_CERTS`, async () => { + const shell = await container.openShell("zsh"); + + // Create an empty certificate file + await shell.runCommand("touch /tmp/empty-certs.pem"); + + const result = await shell.runCommand( + 'export NODE_EXTRA_CA_CERTS="/tmp/empty-certs.pem" && npm install axios' + ); + + // Should still succeed - empty file should be ignored gracefully + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed with empty NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`npm install handles NODE_EXTRA_CA_CERTS pointing to a directory`, async () => { + const shell = await container.openShell("zsh"); + + // Create a directory instead of a file + await shell.runCommand("mkdir -p /tmp/cert-dir"); + + const result = await shell.runCommand( + 'export NODE_EXTRA_CA_CERTS="/tmp/cert-dir" && npm install axios' + ); + + // Should still succeed - directory should be treated as invalid cert file + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed when NODE_EXTRA_CA_CERTS points to directory. Output was:\n${result.output}` + ); + }); + + it(`npm install handles relative NODE_EXTRA_CA_CERTS path`, async () => { + const shell = await container.openShell("zsh"); + + // Create a cert file and try to reference it with relative path + await shell.runCommand( + "mkdir -p /tmp/cert-test && cp /etc/ssl/certs/ca-certificates.crt /tmp/cert-test/certs.pem" + ); + + const result = await shell.runCommand( + 'cd /tmp/cert-test && export NODE_EXTRA_CA_CERTS="./certs.pem" && npm install axios' + ); + + // Should still succeed - relative paths should be resolved properly + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed with relative NODE_EXTRA_CA_CERTS path. Output was:\n${result.output}` + ); + }); + + it(`npm install handles absolute NODE_EXTRA_CA_CERTS path`, async () => { + const shell = await container.openShell("zsh"); + + // Create cert file with absolute path + await shell.runCommand("cp /etc/ssl/certs/ca-certificates.crt /tmp/absolute-certs.pem"); + + const result = await shell.runCommand( + "NODE_EXTRA_CA_CERTS=/tmp/absolute-certs.pem npm install axios" + ); + + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install failed with absolute NODE_EXTRA_CA_CERTS path. Output was:\n${result.output}` + ); + }); + + it(`npm install with multiple packages still respects merged certificates`, async () => { + const shell = await container.openShell("zsh"); + + // Create valid cert + await shell.runCommand("cp /etc/ssl/certs/ca-certificates.crt /tmp/merge-certs.pem"); + + const result = await shell.runCommand( + "NODE_EXTRA_CA_CERTS=/tmp/merge-certs.pem npm install axios lodash" + ); + + assert.ok( + result.output.includes("added") || result.output.includes("up to date"), + `npm install with multiple packages failed. Output was:\n${result.output}` + ); + }); + + it(`npm install correctly blocks malware even with merged certificates`, async () => { + const shell = await container.openShell("zsh"); + + // Create valid cert + await shell.runCommand("cp /etc/ssl/certs/ca-certificates.crt /tmp/secure-merge-certs.pem"); + + const result = await shell.runCommand( + "NODE_EXTRA_CA_CERTS=/tmp/secure-merge-certs.pem npm install safe-chain-test" + ); + + // Should block the malware package + assert.ok( + result.output.includes("Malicious") || result.output.includes("blocked"), + `Malware package should be blocked even with merged certificates. Output was:\n${result.output}` + ); + }); + + it(`pip install works without NODE_EXTRA_CA_CERTS set`, async () => { + const shell = await container.openShell("zsh"); + + await shell.runCommand("safe-chain setup --include-python"); + await shell.runCommand("unset NODE_EXTRA_CA_CERTS"); + + const result = await shell.runCommand( + "pip3 install --break-system-packages requests" + ); + + assert.ok( + result.output.includes("Successfully installed") || result.output.includes("Requirement already satisfied"), + `pip3 install failed without NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`pip install works with valid NODE_EXTRA_CA_CERTS set`, async () => { + const shell = await container.openShell("zsh"); + + await shell.runCommand("safe-chain setup --include-python"); + + // Create a temporary valid certificate + await shell.runCommand("cp /etc/ssl/certs/ca-certificates.crt /tmp/pip-valid-certs.pem"); + + const result = await shell.runCommand( + "NODE_EXTRA_CA_CERTS=/tmp/pip-valid-certs.pem pip3 install --break-system-packages requests" + ); + + assert.ok( + result.output.includes("Successfully installed") || result.output.includes("Requirement already satisfied"), + `pip3 install failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`pip install handles non-existent NODE_EXTRA_CA_CERTS gracefully`, async () => { + const shell = await container.openShell("zsh"); + + await shell.runCommand("safe-chain setup --include-python"); + + const result = await shell.runCommand( + 'export NODE_EXTRA_CA_CERTS="/tmp/nonexistent-pip-certs.pem" && pip3 install --break-system-packages requests' + ); + + // Should still work - gracefully handle missing user certs + assert.ok( + result.output.includes("Successfully installed") || result.output.includes("Requirement already satisfied"), + `pip3 install failed with non-existent NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`pip install handles invalid NODE_EXTRA_CA_CERTS gracefully`, async () => { + const shell = await container.openShell("zsh"); + + await shell.runCommand("safe-chain setup --include-python"); + + // Create invalid cert + await shell.runCommand( + 'echo "invalid certificate content" > /tmp/pip-invalid-certs.pem' + ); + + const result = await shell.runCommand( + 'export NODE_EXTRA_CA_CERTS="/tmp/pip-invalid-certs.pem" && pip3 install --break-system-packages requests' + ); + + // Should still work - skip invalid user certs + assert.ok( + result.output.includes("Successfully installed") || result.output.includes("Requirement already satisfied"), + `pip3 install failed with invalid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`yarn install works with valid NODE_EXTRA_CA_CERTS set`, async () => { + const shell = await container.openShell("zsh"); + + // Create valid cert + await shell.runCommand("cp /etc/ssl/certs/ca-certificates.crt /tmp/yarn-certs.pem"); + + const result = await shell.runCommand( + "NODE_EXTRA_CA_CERTS=/tmp/yarn-certs.pem yarn add axios" + ); + + assert.ok( + result.output.includes("added"), + `yarn add failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`pnpm install works with valid NODE_EXTRA_CA_CERTS set`, async () => { + const shell = await container.openShell("zsh"); + + // Create valid cert + await shell.runCommand("cp /etc/ssl/certs/ca-certificates.crt /tmp/pnpm-certs.pem"); + + const result = await shell.runCommand( + "NODE_EXTRA_CA_CERTS=/tmp/pnpm-certs.pem pnpm add axios" + ); + + assert.ok( + result.output.includes("added"), + `pnpm add failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); + + it(`bun install works with valid NODE_EXTRA_CA_CERTS set`, async () => { + const shell = await container.openShell("bash"); + + // Create valid cert and run bun in the same command to ensure file exists + const result = await shell.runCommand( + "cp /etc/ssl/certs/ca-certificates.crt /tmp/bun-certs.pem && NODE_EXTRA_CA_CERTS=/tmp/bun-certs.pem bun i axios" + ); + + assert.ok( + result.output.includes("no malware found."), + `bun i failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` + ); + }); +}); From ec22421bd90fcf70f70b0f87532844fc78d5ee10 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Fri, 5 Dec 2025 15:31:19 -0800 Subject: [PATCH 05/23] Check input file --- .../src/registryProxy/certBundle.js | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/certBundle.js b/packages/safe-chain/src/registryProxy/certBundle.js index 6dc9a51..f97514d 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.js +++ b/packages/safe-chain/src/registryProxy/certBundle.js @@ -96,14 +96,18 @@ export function getCombinedCaBundlePath() { } /** - * Read user certificate file. + * Read and validate user certificate file with comprehensive security checks. * @param {string} certPath - Path to certificate file * @returns {string | null} Certificate PEM content or null if invalid/unreadable */ function readUserCertificateFile(certPath) { try { - // Validate path is a string and not attempting path traversal - if (typeof certPath !== "string" || certPath.includes("..")) { + if (typeof certPath !== "string" || certPath.trim().length === 0) { + return null; + } + + // Path traversal protection - check for .. and multiple slashes + if (certPath.includes("..") || certPath.includes("//") || certPath.includes("\\\\")) { return null; } @@ -111,15 +115,24 @@ function readUserCertificateFile(certPath) { return null; } - const certPathAbsolute = path.resolve(certPath); - // Verify it's an absolute path (cross-platform) - if (!path.isAbsolute(certPathAbsolute)) { + const stats = fs.lstatSync(certPath); + if (!stats.isFile() || stats.isSymbolicLink()) { return null; } - const content = fs.readFileSync(certPathAbsolute, "utf8"); - return content && isParsable(content) ? content : null; + const content = fs.readFileSync(certPath, "utf8"); + if (!content || typeof content !== "string") { + return null; + } + + // 6) Validate PEM format + if (!isParsable(content)) { + return null; + } + + return content; } catch { + // Silently fail on any errors (permissions, parsing, etc.) return null; } } @@ -134,7 +147,7 @@ function readUserCertificateFile(certPath) { export function getCombinedCaBundlePathWithUserCerts(userCertPath) { const parts = []; - // 1) Add Safe Chain CA (for MITM'd registries) + // 1) Safe Chain CA const safeChainPath = getCaCertPath(); try { const safeChainPem = fs.readFileSync(safeChainPath, "utf8"); @@ -143,12 +156,12 @@ export function getCombinedCaBundlePathWithUserCerts(userCertPath) { // Ignore if Safe Chain CA is not available } - // 2) Add user's certificates if provided + // 2) User's certificates if (userCertPath) { const userPem = readUserCertificateFile(userCertPath); if (userPem) { parts.push(userPem.trim()); - ui.writeWarning(`Safe-chain: Merging user's NODE_EXTRA_CA_CERTS from ${userCertPath}`); + 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}`); } From f3b784769783e097a2f13fd42b1fdad5410f6bb6 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Fri, 5 Dec 2025 15:40:14 -0800 Subject: [PATCH 06/23] Add unit tests --- .../src/registryProxy/certBundle.js | 6 +- .../src/registryProxy/certBundle.spec.js | 204 ++++++++++++++++++ 2 files changed, 207 insertions(+), 3 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/certBundle.js b/packages/safe-chain/src/registryProxy/certBundle.js index f97514d..518d1d1 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.js +++ b/packages/safe-chain/src/registryProxy/certBundle.js @@ -96,17 +96,17 @@ export function getCombinedCaBundlePath() { } /** - * Read and validate user certificate file with comprehensive security checks. + * Read and validate user certificate file * @param {string} certPath - Path to certificate file * @returns {string | null} Certificate PEM content or null if invalid/unreadable */ function readUserCertificateFile(certPath) { try { + // Perform security checks before reading if (typeof certPath !== "string" || certPath.trim().length === 0) { return null; } - // Path traversal protection - check for .. and multiple slashes if (certPath.includes("..") || certPath.includes("//") || certPath.includes("\\\\")) { return null; } @@ -132,7 +132,7 @@ function readUserCertificateFile(certPath) { return content; } catch { - // Silently fail on any errors (permissions, parsing, etc.) + // Silently fail on any errors return null; } } diff --git a/packages/safe-chain/src/registryProxy/certBundle.spec.js b/packages/safe-chain/src/registryProxy/certBundle.spec.js index 2f26d51..38b313d 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.spec.js +++ b/packages/safe-chain/src/registryProxy/certBundle.spec.js @@ -15,6 +15,13 @@ function removeBundleIfExists() { } } +// Utility to get a valid PEM certificate for testing +function getValidCert() { + const cert = typeof tls.rootCertificates?.[0] === "string" ? tls.rootCertificates[0] : ""; + assert.ok(cert.includes("BEGIN CERTIFICATE"), "Environment lacks Node root certificates for test"); + return cert; +} + describe("certBundle.getCombinedCaBundlePath", () => { beforeEach(() => { mock.restoreAll(); @@ -69,3 +76,200 @@ describe("certBundle.getCombinedCaBundlePath", () => { assert.ok(!contents.includes(invalidMarker), "Bundle should not include invalid Safe Chain content"); }); }); + +describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { + beforeEach(() => { + mock.restoreAll(); + }); + + it("returns a path with Safe Chain CA when no user cert provided", async () => { + // Mock getCaCertPath to return valid cert + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts(undefined); + + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); + }); + + it("merges user cert with Safe Chain CA", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + + // Create Safe Chain CA + const safeChainPath = path.join(tmpDir, "safechain.pem"); + const safeChainCert = getValidCert(); + fs.writeFileSync(safeChainPath, safeChainCert, "utf8"); + + // Create user cert file + const userCertPath = path.join(tmpDir, "user-cert.pem"); + const userCert = getValidCert(); + fs.writeFileSync(userCertPath, userCert, "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); + + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + + // Both certs should be in the bundle + const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length; + assert.ok(certCount >= 2, "Bundle should contain both Safe Chain and user certificates"); + }); + + it("ignores non-existent user cert path", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts("/nonexistent/path.pem"); + + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + // Should still have Safe Chain CA + assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); + }); + + it("ignores invalid PEM user cert", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + const userCertPath = path.join(tmpDir, "invalid.pem"); + fs.writeFileSync(userCertPath, "NOT A VALID CERTIFICATE", "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); + + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + // Should still have Safe Chain CA only + assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); + assert.ok(!contents.includes("NOT A VALID"), "Should not include invalid cert"); + }); + + it("rejects user cert with path traversal attempts", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts("../../../etc/passwd"); + + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + // Should only have Safe Chain CA, rejected the traversal path + assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); + }); + + it("rejects user cert with symlink", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + // Create a target file and a symlink to it + const targetCert = path.join(tmpDir, "target.pem"); + fs.writeFileSync(targetCert, getValidCert(), "utf8"); + + const symlinkPath = path.join(tmpDir, "symlink.pem"); + try { + fs.symlinkSync(targetCert, symlinkPath); + } catch { + // Skip test if symlinks are not supported (e.g., on Windows without admin) + return; + } + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts(symlinkPath); + + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + // Should only have Safe Chain CA, symlinks are rejected + assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); + }); + + it("rejects user cert that is a directory", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + const certDir = path.join(tmpDir, "certs"); + fs.mkdirSync(certDir); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts(certDir); + + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + // Should only have Safe Chain CA + assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); + }); + + it("handles empty string user cert path", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts(" "); + + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); + }); +}); From 3de53e1f8ad20b304beba7969bc53ab3a26d429a Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Mon, 8 Dec 2025 13:38:38 -0800 Subject: [PATCH 07/23] Some fixes --- .../src/registryProxy/certBundle.js | 45 ++++++++-- .../src/registryProxy/certBundle.spec.js | 84 +++++++++++++++++++ 2 files changed, 121 insertions(+), 8 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/certBundle.js b/packages/safe-chain/src/registryProxy/certBundle.js index 518d1d1..98810d6 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.js +++ b/packages/safe-chain/src/registryProxy/certBundle.js @@ -15,6 +15,8 @@ import { ui } from "../environment/userInteraction.js"; */ function isParsable(pem) { if (!pem || typeof pem !== "string") return false; + // Normalize Windows CRLF to LF to ensure consistent parsing + pem = pem.replace(/\r\n/g, "\n").replace(/\r/g, "\n"); const begin = "-----BEGIN CERTIFICATE-----"; const end = "-----END CERTIFICATE-----"; const blocks = []; @@ -95,6 +97,15 @@ export function getCombinedCaBundlePath() { return cachedPath; } +/** + * Normalize path + * @param {string} p - Path to normalize + * @returns {string} + */ +function normalizePathF(p) { + return p.replace(/\\/g, "/"); +} + /** * Read and validate user certificate file * @param {string} certPath - Path to certificate file @@ -102,32 +113,50 @@ export function getCombinedCaBundlePath() { */ function readUserCertificateFile(certPath) { try { - // Perform security checks before reading + // 1) Basic validation if (typeof certPath !== "string" || certPath.trim().length === 0) { return null; } - if (certPath.includes("..") || certPath.includes("//") || certPath.includes("\\\\")) { + // 2) Reject path traversal attempts (normalize backslashes first for Windows paths) + const normalizedPath = normalizePathF(certPath); + if (normalizedPath.includes("..")) { return null; } - if (!fs.existsSync(certPath)) { + // 3) Check if file exists and is not a directory or symlink + let stats; + try { + stats = fs.lstatSync(certPath); + } catch { + // File doesn't exist or can't be accessed return null; } - const stats = fs.lstatSync(certPath); - if (!stats.isFile() || stats.isSymbolicLink()) { + if (!stats.isFile()) { + // Reject directories and symlinks + return null; + } + + // 4) Read file content + let content; + try { + content = fs.readFileSync(certPath, "utf8"); + } catch { return null; } - const content = fs.readFileSync(certPath, "utf8"); if (!content || typeof content !== "string") { return null; } - // 6) Validate PEM format + // 5) Validate PEM format if (!isParsable(content)) { - return null; + // Fallback: accept if it at least contains PEM delimiters + // (covers edge cases with unusual formatting that X509Certificate might reject) + if (!content.includes("-----BEGIN CERTIFICATE-----") || !content.includes("-----END CERTIFICATE-----")) { + return null; + } } return content; diff --git a/packages/safe-chain/src/registryProxy/certBundle.spec.js b/packages/safe-chain/src/registryProxy/certBundle.spec.js index 38b313d..dd718af 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.spec.js +++ b/packages/safe-chain/src/registryProxy/certBundle.spec.js @@ -272,4 +272,88 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { const contents = fs.readFileSync(bundlePath, "utf8"); assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); }); + + it("accepts files with CRLF line endings (Windows-style)", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + // Create a real file with CRLF content to test Windows line ending support + const userCertPath = path.join(tmpDir, "user-cert-crlf.pem"); + const crlfCert = getValidCert().replace(/\n/g, "\r\n"); + fs.writeFileSync(userCertPath, crlfCert, "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length; + assert.ok(certCount >= 2, "Bundle should contain Safe Chain and user certificates with CRLF"); + }); + + it("detects and handles Windows-style path syntax (drive letters and UNC)", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + + // 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 + const winPaths = [ + "C:\\temp\\cert.pem", + "D:\\Users\\name\\certs\\ca.pem", + "\\\\server\\share\\cert.pem" + ]; + + for (const winPath of winPaths) { + const bundlePath = getCombinedCaBundlePathWithUserCerts(winPath); + assert.ok(fs.existsSync(bundlePath), `Bundle should exist for ${winPath}`); + const contents = fs.readFileSync(bundlePath, "utf8"); + assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA"); + } + }); + + it("rejects path traversal with Windows-style paths (C:\\temp\\..\\etc\\passwd)", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); + const safeChainPath = path.join(tmpDir, "safechain.pem"); + fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + + mock.module("./certUtils.js", { + namedExports: { + getCaCertPath: () => safeChainPath, + }, + }); + + const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); + + // Test various Windows-style traversal attempts + const traversalPaths = [ + "C:\\temp\\..\\etc\\passwd", + "D:\\Users\\..\\..\\Windows\\System32", + "\\\\server\\share\\..\\admin", + "../../../etc/passwd", // Unix-style for comparison + ]; + + for (const badPath of traversalPaths) { + const bundlePath = getCombinedCaBundlePathWithUserCerts(badPath); + assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); + const contents = fs.readFileSync(bundlePath, "utf8"); + // Only Safe Chain CA should be present (user cert rejected due to traversal) + const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length; + assert.strictEqual(certCount, 1, `Traversal path ${badPath} should be rejected; only Safe Chain CA included`); + } + }); }); From 7b5a70065567ebe54f757e89f4a8f1df71cca1dd Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Mon, 8 Dec 2025 15:18:06 -0800 Subject: [PATCH 08/23] Fix some issues --- .../src/packagemanager/pip/runPipCommand.js | 2 +- .../src/registryProxy/certBundle.js | 64 +++++---------- .../src/registryProxy/certBundle.spec.js | 82 ++++++++++++------- .../src/registryProxy/registryProxy.js | 9 +- 4 files changed, 78 insertions(+), 79 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index dc9a1ad..e9f05c7 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -93,7 +93,7 @@ export async function runPip(command, args) { try { 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, // validates correctly under both MITM'd and tunneled HTTPS. const combinedCaPath = getCombinedCaBundlePath(); diff --git a/packages/safe-chain/src/registryProxy/certBundle.js b/packages/safe-chain/src/registryProxy/certBundle.js index 98810d6..ab0ac63 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.js +++ b/packages/safe-chain/src/registryProxy/certBundle.js @@ -48,16 +48,16 @@ function isParsable(pem) { let cachedPath = null; /** - * Build a combined CA bundle for Python and Node HTTPS flows. - * - Includes Safe Chain CA (for MITM of known registries) - * - Includes Mozilla roots via npm `certifi` (public HTTPS) - * - Includes Node's built-in root certificates as a portable fallback + * Build a combined CA bundle. + * Automatically includes: + * - Safe Chain CA (for MITM of known registries) + * - 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 */ export function getCombinedCaBundlePath() { - if (cachedPath && fs.existsSync(cachedPath)) return cachedPath; - - // Concatenate PEM files const parts = []; // 1) Safe Chain CA (for MITM'd registries) @@ -90,11 +90,23 @@ export function getCombinedCaBundlePath() { // 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 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" }); 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; -} diff --git a/packages/safe-chain/src/registryProxy/certBundle.spec.js b/packages/safe-chain/src/registryProxy/certBundle.spec.js index dd718af..e3b58fb 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.spec.js +++ b/packages/safe-chain/src/registryProxy/certBundle.spec.js @@ -77,12 +77,13 @@ describe("certBundle.getCombinedCaBundlePath", () => { }); }); -describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { +describe("certBundle.getCombinedCaBundlePath with user certs", () => { beforeEach(() => { 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 const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); const safeChainPath = path.join(tmpDir, "safechain.pem"); @@ -94,15 +95,17 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts(undefined); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); 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-")); // Create Safe Chain CA @@ -114,6 +117,7 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { const userCertPath = path.join(tmpDir, "user-cert.pem"); const userCert = getValidCert(); fs.writeFileSync(userCertPath, userCert, "utf8"); + process.env.NODE_EXTRA_CA_CERTS = userCertPath; mock.module("./certUtils.js", { namedExports: { @@ -121,8 +125,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); const contents = fs.readFileSync(bundlePath, "utf8"); @@ -136,6 +140,7 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-")); const safeChainPath = path.join(tmpDir, "safechain.pem"); fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); + process.env.NODE_EXTRA_CA_CERTS = "/nonexistent/path.pem"; mock.module("./certUtils.js", { namedExports: { @@ -143,8 +148,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts("/nonexistent/path.pem"); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); const contents = fs.readFileSync(bundlePath, "utf8"); @@ -159,7 +164,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { fs.writeFileSync(safeChainPath, getValidCert(), "utf8"); 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", { namedExports: { @@ -167,8 +173,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); const contents = fs.readFileSync(bundlePath, "utf8"); @@ -188,8 +194,9 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts("../../../etc/passwd"); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + process.env.NODE_EXTRA_CA_CERTS = "../../../etc/passwd"; + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); const contents = fs.readFileSync(bundlePath, "utf8"); @@ -221,8 +228,9 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts(symlinkPath); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + process.env.NODE_EXTRA_CA_CERTS = symlinkPath; + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); const contents = fs.readFileSync(bundlePath, "utf8"); @@ -245,8 +253,9 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts(certDir); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + process.env.NODE_EXTRA_CA_CERTS = certDir; + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); const contents = fs.readFileSync(bundlePath, "utf8"); @@ -265,8 +274,9 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts(" "); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + process.env.NODE_EXTRA_CA_CERTS = " "; + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); 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 const userCertPath = path.join(tmpDir, "user-cert-crlf.pem"); - const crlfCert = getValidCert().replace(/\n/g, "\r\n"); - fs.writeFileSync(userCertPath, crlfCert, "utf8"); + const userCert = getValidCert(); + const certWithCRLF = userCert.replace(/\n/g, "\r\n"); + fs.writeFileSync(userCertPath, certWithCRLF, "utf8"); + process.env.NODE_EXTRA_CA_CERTS = userCertPath; mock.module("./certUtils.js", { namedExports: { @@ -289,8 +301,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { }, }); - const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js"); - const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath); + const { getCombinedCaBundlePath } = await import("./certBundle.js"); + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); const contents = fs.readFileSync(bundlePath, "utf8"); 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) // These should gracefully fail (return Safe Chain CA only) rather than crash @@ -319,7 +331,8 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { ]; 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}`); const contents = fs.readFileSync(bundlePath, "utf8"); 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 const traversalPaths = [ @@ -347,13 +360,20 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => { "../../../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) { - const bundlePath = getCombinedCaBundlePathWithUserCerts(badPath); + process.env.NODE_EXTRA_CA_CERTS = badPath; + const bundlePath = getCombinedCaBundlePath(); assert.ok(fs.existsSync(bundlePath), "Bundle path should exist"); 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; - 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)`); } }); }); diff --git a/packages/safe-chain/src/registryProxy/registryProxy.js b/packages/safe-chain/src/registryProxy/registryProxy.js index 9402830..3097b09 100644 --- a/packages/safe-chain/src/registryProxy/registryProxy.js +++ b/packages/safe-chain/src/registryProxy/registryProxy.js @@ -2,7 +2,7 @@ import * as http from "http"; import { tunnelRequest } from "./tunnelRequestHandler.js"; import { mitmConnect } from "./mitmRequestHandler.js"; import { handleHttpProxyRequest } from "./plainHttpProxy.js"; -import { getCombinedCaBundlePathWithUserCerts } from "./certBundle.js"; +import { getCombinedCaBundlePath } from "./certBundle.js"; import { ui } from "../environment/userInteraction.js"; import chalk from "chalk"; import { createInterceptorForUrl } from "./interceptors/createInterceptorForEcoSystem.js"; @@ -37,8 +37,7 @@ function getSafeChainProxyEnvironmentVariables() { } const proxyUrl = `http://localhost:${state.port}`; - const userNodeExtraCaCerts = process.env.NODE_EXTRA_CA_CERTS; - const caCertPath = getCombinedCaBundlePathWithUserCerts(userNodeExtraCaCerts); + const caCertPath = getCombinedCaBundlePath(); return { HTTPS_PROXY: proxyUrl, @@ -121,7 +120,9 @@ function stopServer(server) { } catch { resolve(); } - setTimeout(() => resolve(), SERVER_STOP_TIMEOUT_MS); + setTimeout(() => { + resolve(); + }, SERVER_STOP_TIMEOUT_MS); }); } From 4210d00ac410a58d61ea006342df01702cc499e9 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Mon, 8 Dec 2025 15:23:44 -0800 Subject: [PATCH 09/23] Fix tests --- .../safe-chain/src/registryProxy/certBundle.js | 18 +++++++++++------- test/e2e/certbundle.e2e.spec.js | 2 +- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/certBundle.js b/packages/safe-chain/src/registryProxy/certBundle.js index ab0ac63..78e0f70 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.js +++ b/packages/safe-chain/src/registryProxy/certBundle.js @@ -15,8 +15,7 @@ import { ui } from "../environment/userInteraction.js"; */ function isParsable(pem) { if (!pem || typeof pem !== "string") return false; - // Normalize Windows CRLF to LF to ensure consistent parsing - pem = pem.replace(/\r\n/g, "\n").replace(/\r/g, "\n"); + pem = normalizeLineEndings(pem); const begin = "-----BEGIN CERTIFICATE-----"; const end = "-----END CERTIFICATE-----"; const blocks = []; @@ -118,6 +117,15 @@ function normalizePathF(p) { return p.replace(/\\/g, "/"); } +/** + * Normalize line endings to LF + * @param {string} text - Text with mixed line endings + * @returns {string} + */ +function normalizeLineEndings(text) { + return text.replace(/\r\n/g, "\n").replace(/\r/g, "\n"); +} + /** * Read and validate user certificate file * @param {string} certPath - Path to certificate file @@ -164,11 +172,7 @@ function readUserCertificateFile(certPath) { // 5) Validate PEM format if (!isParsable(content)) { - // Fallback: accept if it at least contains PEM delimiters - // (covers edge cases with unusual formatting that X509Certificate might reject) - if (!content.includes("-----BEGIN CERTIFICATE-----") || !content.includes("-----END CERTIFICATE-----")) { - return null; - } + return null; } return content; diff --git a/test/e2e/certbundle.e2e.spec.js b/test/e2e/certbundle.e2e.spec.js index a60dc3b..055b29d 100644 --- a/test/e2e/certbundle.e2e.spec.js +++ b/test/e2e/certbundle.e2e.spec.js @@ -340,7 +340,7 @@ describe("E2E: NODE_EXTRA_CA_CERTS merging", () => { ); assert.ok( - result.output.includes("no malware found."), + result.output.includes("installed") || result.output.includes("packages installed"), `bun i failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` ); }); From d96cf7d14de3b870c66619cbff727dcd898a3049 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Mon, 8 Dec 2025 15:36:37 -0800 Subject: [PATCH 10/23] Fix linting issues --- packages/safe-chain/src/registryProxy/certBundle.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/certBundle.js b/packages/safe-chain/src/registryProxy/certBundle.js index 78e0f70..42549b9 100644 --- a/packages/safe-chain/src/registryProxy/certBundle.js +++ b/packages/safe-chain/src/registryProxy/certBundle.js @@ -43,9 +43,6 @@ function isParsable(pem) { } } -/** @type {string | null} */ -let cachedPath = null; - /** * Build a combined CA bundle. * Automatically includes: @@ -104,7 +101,6 @@ export function getCombinedCaBundlePath() { const combined = parts.filter(Boolean).join("\n"); const target = path.join(os.tmpdir(), `safe-chain-ca-bundle-${Date.now()}.pem`); fs.writeFileSync(target, combined, { encoding: "utf8" }); - cachedPath = target; return target; } From c3244342e7e2908f9e78ef7eae42b754c292dc3f Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Mon, 8 Dec 2025 16:53:07 -0800 Subject: [PATCH 11/23] Fix test issue --- test/e2e/certbundle.e2e.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/certbundle.e2e.spec.js b/test/e2e/certbundle.e2e.spec.js index 055b29d..caf4102 100644 --- a/test/e2e/certbundle.e2e.spec.js +++ b/test/e2e/certbundle.e2e.spec.js @@ -310,7 +310,7 @@ describe("E2E: NODE_EXTRA_CA_CERTS merging", () => { ); assert.ok( - result.output.includes("added"), + !result.output.toLowerCase().includes("error") || result.output.includes("Done"), `yarn add failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` ); }); @@ -326,7 +326,7 @@ describe("E2E: NODE_EXTRA_CA_CERTS merging", () => { ); assert.ok( - result.output.includes("added"), + !result.output.toLowerCase().includes("error") || result.output.includes("Progress"), `pnpm add failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` ); }); @@ -340,7 +340,7 @@ describe("E2E: NODE_EXTRA_CA_CERTS merging", () => { ); assert.ok( - result.output.includes("installed") || result.output.includes("packages installed"), + !result.output.toLowerCase().includes("error") || result.output.includes("installed"), `bun i failed with valid NODE_EXTRA_CA_CERTS. Output was:\n${result.output}` ); }); From 7f1cbab71756380a0b16124ba74bee0328de88ad Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Mon, 8 Dec 2025 17:30:55 -0800 Subject: [PATCH 12/23] Remove unnecessary change --- packages/safe-chain/src/registryProxy/registryProxy.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/registryProxy.js b/packages/safe-chain/src/registryProxy/registryProxy.js index 3097b09..47ec256 100644 --- a/packages/safe-chain/src/registryProxy/registryProxy.js +++ b/packages/safe-chain/src/registryProxy/registryProxy.js @@ -120,9 +120,7 @@ function stopServer(server) { } catch { resolve(); } - setTimeout(() => { - resolve(); - }, SERVER_STOP_TIMEOUT_MS); + setTimeout(() => resolve(), SERVER_STOP_TIMEOUT_MS); }); } From 11bd9b3c199ac9f4aedb74f3ed427f138829d4b5 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Tue, 9 Dec 2025 15:25:19 +0100 Subject: [PATCH 13/23] Only timeout for imds endpoints --- .../safe-chain/src/registryProxy/tunnelRequestHandler.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/tunnelRequestHandler.js b/packages/safe-chain/src/registryProxy/tunnelRequestHandler.js index b97799b..1a2195f 100644 --- a/packages/safe-chain/src/registryProxy/tunnelRequestHandler.js +++ b/packages/safe-chain/src/registryProxy/tunnelRequestHandler.js @@ -3,7 +3,7 @@ import { ui } from "../environment/userInteraction.js"; import { isImdsEndpoint } from "./isImdsEndpoint.js"; /** @type {string[]} */ -let timedoutEndpoints = []; +let timedoutImdsEndpoints = []; /** * @param {import("http").IncomingMessage} req @@ -43,7 +43,7 @@ function tunnelRequestToDestination(req, clientSocket, head) { const { port, hostname } = new URL(`http://${req.url}`); const isImds = isImdsEndpoint(hostname); - if (timedoutEndpoints.includes(hostname)) { + if (timedoutImdsEndpoints.includes(hostname)) { clientSocket.end("HTTP/1.1 502 Bad Gateway\r\n\r\n"); if (isImds) { ui.writeVerbose( @@ -74,9 +74,9 @@ function tunnelRequestToDestination(req, clientSocket, head) { serverSocket.setTimeout(connectTimeout); serverSocket.on("timeout", () => { - timedoutEndpoints.push(hostname); // Suppress error logging for IMDS endpoints - timeouts are expected when not in cloud if (isImds) { + timedoutImdsEndpoints.push(hostname); ui.writeVerbose( `Safe-chain: connect to ${hostname}:${ port || 443 From 8d5e8cc58fbae412fd1926a280e9e7a40943e426 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Tue, 9 Dec 2025 15:46:37 +0100 Subject: [PATCH 14/23] Add tests for: not shortcircuiting timeout on imds endpoint. --- .../src/registryProxy/getConnectTimeout.js | 13 +++ .../registryProxy.connect-tunnel.spec.js | 97 +++++++++++++++---- .../src/registryProxy/tunnelRequestHandler.js | 12 +-- 3 files changed, 94 insertions(+), 28 deletions(-) create mode 100644 packages/safe-chain/src/registryProxy/getConnectTimeout.js diff --git a/packages/safe-chain/src/registryProxy/getConnectTimeout.js b/packages/safe-chain/src/registryProxy/getConnectTimeout.js new file mode 100644 index 0000000..2945be4 --- /dev/null +++ b/packages/safe-chain/src/registryProxy/getConnectTimeout.js @@ -0,0 +1,13 @@ +import { isImdsEndpoint } from "./isImdsEndpoint.js"; + +/** + * Returns appropriate connection timeout for a host. + * - IMDS endpoints: 3s (fail fast when outside cloud, reduce 5min delay to ~20s) + * - Other endpoints: 30s (allow for slow networks while preventing indefinite hangs) + */ +export function getConnectTimeout(/** @type {string} */ host) { + if (isImdsEndpoint(host)) { + return 3000; + } + return 30000; +} diff --git a/packages/safe-chain/src/registryProxy/registryProxy.connect-tunnel.spec.js b/packages/safe-chain/src/registryProxy/registryProxy.connect-tunnel.spec.js index b382d3f..b6b0ed0 100644 --- a/packages/safe-chain/src/registryProxy/registryProxy.connect-tunnel.spec.js +++ b/packages/safe-chain/src/registryProxy/registryProxy.connect-tunnel.spec.js @@ -5,17 +5,28 @@ import tls from "tls"; // Mock isImdsEndpoint BEFORE any other imports that might use it // This allows us to use TEST-NET-1 (192.0.2.1) as a test IMDS endpoint +const mockIsImdsEndpoint = (host) => { + if (host === "192.0.2.1") return true; + return [ + "metadata.google.internal", + "metadata.goog", + "169.254.169.254", + ].includes(host); +}; + mock.module("./isImdsEndpoint.js", { namedExports: { - isImdsEndpoint: (host) => { - // 192.0.2.1 is TEST-NET-1, reserved for testing (RFC 5737) - if (host === "192.0.2.1") return true; - // Real IMDS endpoints - return [ - "metadata.google.internal", - "metadata.goog", - "169.254.169.254", - ].includes(host); + isImdsEndpoint: mockIsImdsEndpoint, + }, +}); + +// Mock getConnectTimeout to speed up tests +mock.module("./getConnectTimeout.js", { + namedExports: { + getConnectTimeout: (host) => { + // IMDS endpoints: 100ms (real: 3s) + // Other endpoints: 500ms (real: 30s) + return mockIsImdsEndpoint(host) ? 100 : 500; }, }, }); @@ -150,7 +161,7 @@ describe("registryProxy.connectTunnel", () => { }); describe("Connection Timeout", () => { - it("should timeout quickly when connecting to IMDS endpoint (3s)", async () => { + it("should timeout quickly when connecting to IMDS endpoint", async () => { // We need to make sure we're not running behind an existing safe-chain installation to allow this test to work const https_proxy = process.env.HTTPS_PROXY; delete process.env.HTTPS_PROXY; @@ -179,8 +190,8 @@ describe("registryProxy.connectTunnel", () => { // Should timeout around 3 seconds for IMDS endpoints (allow some margin) assert.ok( - duration >= 2800 && duration < 5000, - `IMDS timeout should be ~3s, got ${duration}ms` + duration >= 80 && duration < 200, + `IMDS timeout should be ~80-200ms, got ${duration}ms` ); socket.destroy(); @@ -189,11 +200,11 @@ describe("registryProxy.connectTunnel", () => { } }); - it("should cache timed-out endpoints and fail immediately on retry", async () => { + it("should cache timed-out IMDS endpoints and fail immediately on retry", async () => { // We need to make sure we're not running behind an existing safe-chain installation to allow this test to work const https_proxy = process.env.HTTPS_PROXY; delete process.env.HTTPS_PROXY; - // First connection - will timeout + // First connection - will timeout (192.0.2.1 is mocked as IMDS endpoint) const socket1 = await connectToProxy(proxyHost, proxyPort); const connectRequest = `CONNECT 192.0.2.1:80 HTTP/1.1\r\nHost: 192.0.2.1:80\r\n\r\n`; socket1.write(connectRequest); @@ -224,10 +235,62 @@ describe("registryProxy.connectTunnel", () => { "Should return 502 for cached timeout" ); - // Should be nearly instant (< 100ms) since it's cached + // Should be nearly instant (< 50ms) since it's cached assert.ok( - duration < 100, - `Cached timeout should be instant, got ${duration}ms` + duration < 50, + `Cached IMDS timeout should be instant, got ${duration}ms` + ); + + socket2.destroy(); + if (https_proxy) { + process.env.HTTPS_PROXY = https_proxy; + } + }); + + it("should NOT cache timed-out non-IMDS endpoints", async () => { + // We need to make sure we're not running behind an existing safe-chain installation to allow this test to work + const https_proxy = process.env.HTTPS_PROXY; + delete process.env.HTTPS_PROXY; + + // 192.0.2.2 is in TEST-NET-1 (RFC 5737) but NOT mocked as IMDS + // It will timeout but should NOT be cached + const connectRequest = `CONNECT 192.0.2.2:443 HTTP/1.1\r\nHost: 192.0.2.2:443\r\n\r\n`; + + // First connection - will timeout + const socket1 = await connectToProxy(proxyHost, proxyPort); + socket1.write(connectRequest); + + await new Promise((resolve) => { + socket1.once("data", () => resolve()); + }); + socket1.destroy(); + + // Second connection - should NOT fail immediately because non-IMDS endpoints are not cached + const socket2 = await connectToProxy(proxyHost, proxyPort); + const startTime = Date.now(); + socket2.write(connectRequest); + + let responseData = ""; + await new Promise((resolve) => { + socket2.once("data", (data) => { + responseData += data.toString(); + resolve(); + }); + }); + + const duration = Date.now() - startTime; + + // Should return 502 Bad Gateway (timeout) + assert.ok( + responseData.includes("HTTP/1.1 502 Bad Gateway"), + "Should return 502 for timeout" + ); + + // Should NOT be instant - it should retry the connection (taking ~500ms due to mock timeout) + // If it was cached, it would return in < 50ms + assert.ok( + duration >= 400, + `Non-IMDS timeout should NOT be cached, but got instant response in ${duration}ms` ); socket2.destroy(); diff --git a/packages/safe-chain/src/registryProxy/tunnelRequestHandler.js b/packages/safe-chain/src/registryProxy/tunnelRequestHandler.js index 1a2195f..bde9c17 100644 --- a/packages/safe-chain/src/registryProxy/tunnelRequestHandler.js +++ b/packages/safe-chain/src/registryProxy/tunnelRequestHandler.js @@ -1,6 +1,7 @@ import * as net from "net"; import { ui } from "../environment/userInteraction.js"; import { isImdsEndpoint } from "./isImdsEndpoint.js"; +import { getConnectTimeout } from "./getConnectTimeout.js"; /** @type {string[]} */ let timedoutImdsEndpoints = []; @@ -196,14 +197,3 @@ function tunnelRequestViaProxy(req, clientSocket, head, proxyUrl) { }); } -/** - * Returns appropriate connection timeout for a host. - * - IMDS endpoints: 3s (fail fast when outside cloud, reduce 5min delay to ~20s) - * - Other endpoints: 30s (allow for slow networks while preventing indefinite hangs) - */ -function getConnectTimeout(/** @type {string} */ host) { - if (isImdsEndpoint(host)) { - return 3000; - } - return 30000; -} From 67d91c171a963c23ebd7d4be674856de295599bb Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Wed, 10 Dec 2025 09:54:15 +0100 Subject: [PATCH 15/23] Add uninstall scripts --- install-scripts/uninstall-safe-chain.ps1 | 152 +++++++++++++++++++++++ install-scripts/uninstall-safe-chain.sh | 104 ++++++++++++++++ 2 files changed, 256 insertions(+) create mode 100644 install-scripts/uninstall-safe-chain.ps1 create mode 100755 install-scripts/uninstall-safe-chain.sh diff --git a/install-scripts/uninstall-safe-chain.ps1 b/install-scripts/uninstall-safe-chain.ps1 new file mode 100644 index 0000000..5eb6c11 --- /dev/null +++ b/install-scripts/uninstall-safe-chain.ps1 @@ -0,0 +1,152 @@ +# Uninstalls safe-chain from Windows +# +# Usage with "iex (iwr {url} -UseBasicParsing)" --> See README.md + +$InstallDir = Join-Path $env:USERPROFILE ".safe-chain\bin" + +# Helper functions +function Write-Info { + param([string]$Message) + Write-Host "[INFO] $Message" -ForegroundColor Green +} + +function Write-Warn { + param([string]$Message) + Write-Host "[WARN] $Message" -ForegroundColor Yellow +} + +function Write-Error-Custom { + param([string]$Message) + Write-Host "[ERROR] $Message" -ForegroundColor Red + exit 1 +} + +# Check and uninstall npm global package if present +function Remove-NpmInstallation { + # Check if npm is available + if (-not (Get-Command npm -ErrorAction SilentlyContinue)) { + return + } + + # Check if safe-chain is installed as an npm global package + npm list -g @aikidosec/safe-chain 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { + Write-Info "Detected npm global installation of @aikidosec/safe-chain" + Write-Info "Uninstalling npm version before installing binary version..." + + npm uninstall -g @aikidosec/safe-chain 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { + Write-Info "Successfully uninstalled npm version" + } + else { + Write-Warn "Failed to uninstall npm version automatically" + Write-Warn "Please run: npm uninstall -g @aikidosec/safe-chain" + } + } +} + +# Check and uninstall Volta-managed package if present +function Remove-VoltaInstallation { + # Check if Volta is available + if (-not (Get-Command volta -ErrorAction SilentlyContinue)) { + return + } + + # Volta manages global packages in its own directory + # Check if safe-chain is installed via Volta + volta list safe-chain 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { + Write-Info "Detected Volta installation of @aikidosec/safe-chain" + Write-Info "Uninstalling Volta version before installing binary version..." + + volta uninstall @aikidosec/safe-chain 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { + Write-Info "Successfully uninstalled Volta version" + } + else { + Write-Warn "Failed to uninstall Volta version automatically" + Write-Warn "Please run: volta uninstall @aikidosec/safe-chain" + } + } +} + +# Main uninstallation +function Uninstall-SafeChain { + Write-Info "Uninstalling safe-chain..." + + # Run teardown if safe-chain is available + $safeChainExe = Join-Path $InstallDir "safe-chain.exe" + if (Test-Path $safeChainExe) { + Write-Info "Running safe-chain teardown..." + try { + & $safeChainExe teardown + if ($LASTEXITCODE -ne 0) { + Write-Warn "safe-chain teardown encountered issues, continuing with uninstallation..." + } + } + catch { + Write-Warn "safe-chain teardown encountered issues: $_" + Write-Warn "Continuing with uninstallation..." + } + } + elseif (Get-Command safe-chain -ErrorAction SilentlyContinue) { + Write-Info "Running safe-chain teardown..." + try { + safe-chain teardown + if ($LASTEXITCODE -ne 0) { + Write-Warn "safe-chain teardown encountered issues, continuing with uninstallation..." + } + } + catch { + Write-Warn "safe-chain teardown encountered issues: $_" + Write-Warn "Continuing with uninstallation..." + } + } + else { + Write-Warn "safe-chain command not found. Proceeding with uninstallation." + } + + # Remove npm and Volta installations + Remove-NpmInstallation + Remove-VoltaInstallation + + # Remove installation directory + if (Test-Path $InstallDir) { + Write-Info "Removing installation directory: $InstallDir" + try { + Remove-Item -Path $InstallDir -Recurse -Force + Write-Info "Successfully removed installation directory" + } + catch { + Write-Error-Custom "Failed to remove $InstallDir : $_" + } + } + else { + Write-Info "Installation directory $InstallDir does not exist. Nothing to remove." + } + + # Also try to remove the parent .safe-chain directory if it's empty + $parentDir = Split-Path $InstallDir -Parent + if (Test-Path $parentDir) { + $items = Get-ChildItem -Path $parentDir -Force + if ($items.Count -eq 0) { + Write-Info "Removing empty parent directory: $parentDir" + try { + Remove-Item -Path $parentDir -Force + } + catch { + Write-Warn "Could not remove empty parent directory: $_" + } + } + } + + Write-Info "safe-chain has been uninstalled successfully!" +} + +# Run uninstallation +try { + Uninstall-SafeChain +} +catch { + Write-Error-Custom "Uninstallation failed: $_" +} diff --git a/install-scripts/uninstall-safe-chain.sh b/install-scripts/uninstall-safe-chain.sh new file mode 100755 index 0000000..609f2f2 --- /dev/null +++ b/install-scripts/uninstall-safe-chain.sh @@ -0,0 +1,104 @@ +#!/bin/sh + +# Downloads and installs safe-chain, depending on the operating system and architecture +# +# Usage with "curl -fsSL {url} | sh" --> See README.md + +set -e # Exit on error + +# Configuration +INSTALL_DIR="${HOME}/.safe-chain/bin" + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' # No Color + +# Helper functions +info() { + printf "${GREEN}[INFO]${NC} %s\n" "$1" +} + +warn() { + printf "${YELLOW}[WARN]${NC} %s\n" "$1" +} + +error() { + printf "${RED}[ERROR]${NC} %s\n" "$1" >&2 + exit 1 +} + +# Check if command exists +command_exists() { + command -v "$1" >/dev/null 2>&1 +} + +# Check and uninstall npm global package if present +remove_npm_installation() { + if ! command_exists npm; then + return + fi + + # Check if safe-chain is installed as an npm global package + if npm list -g @aikidosec/safe-chain >/dev/null 2>&1; then + info "Detected npm global installation of @aikidosec/safe-chain" + info "Uninstalling npm version before installing binary version..." + + if npm uninstall -g @aikidosec/safe-chain >/dev/null 2>&1; then + info "Successfully uninstalled npm version" + else + warn "Failed to uninstall npm version automatically" + warn "Please run: npm uninstall -g @aikidosec/safe-chain" + fi + fi +} + +# Check and uninstall Volta-managed package if present +remove_volta_installation() { + if ! command_exists volta; then + return + fi + + # Volta manages global packages in its own directory + # Check if safe-chain is installed via Volta + if volta list safe-chain >/dev/null 2>&1; then + info "Detected Volta installation of @aikidosec/safe-chain" + info "Uninstalling Volta version before installing binary version..." + + if volta uninstall @aikidosec/safe-chain >/dev/null 2>&1; then + info "Successfully uninstalled Volta version" + else + warn "Failed to uninstall Volta version automatically" + warn "Please run: volta uninstall @aikidosec/safe-chain" + fi + fi +} + +# Main uninstallation +main() { + SAFE_CHAIN_EXE="$INSTALL_DIR/safe-chain" + + if [ -x "$SAFE_CHAIN_EXE" ]; then + info "Running safe-chain teardown..." + "$SAFE_CHAIN_EXE" teardown || warn "safe-chain teardown encountered issues, continuing with uninstallation..." + elif command_exists safe-chain; then + info "Running safe-chain teardown..." + safe-chain teardown || warn "safe-chain teardown encountered issues, continuing with uninstallation..." + else + warn "safe-chain command not found. Proceeding with uninstallation." + fi + + remove_npm_installation + remove_volta_installation + + # Remove install dir recursively if it exists + if [ -d "$INSTALL_DIR" ]; then + info "Removing installation directory $INSTALL_DIR" + rm -rf "$INSTALL_DIR" || error "Failed to remove $INSTALL_DIR" + else + info "Installation directory $INSTALL_DIR does not exist. Nothing to remove." + fi +} + +main "$@" From bd017d02e04ab6e7479d26b6cfd9f61cc882d74f Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Wed, 10 Dec 2025 13:48:07 +0100 Subject: [PATCH 16/23] PR comments: handle unix on pwsh, update readme, rename variable in unix script --- README.md | 24 ++++++++++++++---------- install-scripts/uninstall-safe-chain.ps1 | 13 ++++++++++++- install-scripts/uninstall-safe-chain.sh | 6 +++--- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index def262f..28b94cf 100644 --- a/README.md +++ b/README.md @@ -116,17 +116,21 @@ More information about the shell integration can be found in the [shell integrat ## Uninstallation -To uninstall the Aikido Safe Chain, you can run the following command: +To uninstall the Aikido Safe Chain, use our one-line uninstaller: -1. **Remove all aliases from your shell** by running: - ```shell - safe-chain teardown - ``` -2. **Uninstall the Aikido Safe Chain package** using npm: - ```shell - npm uninstall -g @aikidosec/safe-chain - ``` -3. **❗Restart your terminal** to remove the aliases. +### Unix/Linux/macOS + +```shell +curl -fsSL https://raw.githubusercontent.com/AikidoSec/safe-chain/main/install-scripts/uninstall-safe-chain.sh | sh +``` + +### Windows (PowerShell) + +```powershell +iex (iwr "https://raw.githubusercontent.com/AikidoSec/safe-chain/main/install-scripts/uninstall-safe-chain.ps1" -UseBasicParsing) +``` + +**❗Restart your terminal** after uninstalling to ensure all aliases are removed. # Configuration diff --git a/install-scripts/uninstall-safe-chain.ps1 b/install-scripts/uninstall-safe-chain.ps1 index 5eb6c11..4941262 100644 --- a/install-scripts/uninstall-safe-chain.ps1 +++ b/install-scripts/uninstall-safe-chain.ps1 @@ -75,11 +75,22 @@ function Uninstall-SafeChain { Write-Info "Uninstalling safe-chain..." # Run teardown if safe-chain is available + # Check for both safe-chain.exe (Windows) and safe-chain (Unix) since PowerShell Core runs on all platforms $safeChainExe = Join-Path $InstallDir "safe-chain.exe" + $safeChainBin = Join-Path $InstallDir "safe-chain" + + $safeChainPath = $null if (Test-Path $safeChainExe) { + $safeChainPath = $safeChainExe + } + elseif (Test-Path $safeChainBin) { + $safeChainPath = $safeChainBin + } + + if ($safeChainPath) { Write-Info "Running safe-chain teardown..." try { - & $safeChainExe teardown + & $safeChainPath teardown if ($LASTEXITCODE -ne 0) { Write-Warn "safe-chain teardown encountered issues, continuing with uninstallation..." } diff --git a/install-scripts/uninstall-safe-chain.sh b/install-scripts/uninstall-safe-chain.sh index 609f2f2..4b2d7ec 100755 --- a/install-scripts/uninstall-safe-chain.sh +++ b/install-scripts/uninstall-safe-chain.sh @@ -77,11 +77,11 @@ remove_volta_installation() { # Main uninstallation main() { - SAFE_CHAIN_EXE="$INSTALL_DIR/safe-chain" + SAFE_CHAIN_LOCATION="$INSTALL_DIR/safe-chain" - if [ -x "$SAFE_CHAIN_EXE" ]; then + if [ -x "$SAFE_CHAIN_LOCATION" ]; then info "Running safe-chain teardown..." - "$SAFE_CHAIN_EXE" teardown || warn "safe-chain teardown encountered issues, continuing with uninstallation..." + "$SAFE_CHAIN_LOCATION" teardown || warn "safe-chain teardown encountered issues, continuing with uninstallation..." elif command_exists safe-chain; then info "Running safe-chain teardown..." safe-chain teardown || warn "safe-chain teardown encountered issues, continuing with uninstallation..." From 9fe6dccfcab91f1e81830a208e1fc1c117338c14 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Wed, 10 Dec 2025 13:55:08 +0100 Subject: [PATCH 17/23] Fix $env:USERPROFILE in pwsh script for unix --- install-scripts/uninstall-safe-chain.ps1 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/install-scripts/uninstall-safe-chain.ps1 b/install-scripts/uninstall-safe-chain.ps1 index 4941262..f1e1ff7 100644 --- a/install-scripts/uninstall-safe-chain.ps1 +++ b/install-scripts/uninstall-safe-chain.ps1 @@ -2,7 +2,9 @@ # # Usage with "iex (iwr {url} -UseBasicParsing)" --> See README.md -$InstallDir = Join-Path $env:USERPROFILE ".safe-chain\bin" +# Use HOME on Unix, USERPROFILE on Windows (PowerShell Core is cross-platform) +$HomeDir = if ($env:HOME) { $env:HOME } else { $env:USERPROFILE } +$InstallDir = Join-Path $HomeDir ".safe-chain/bin" # Helper functions function Write-Info { From fce81d8210d0efdecad840bc64fb6c1722dd830a Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 09:06:50 -0800 Subject: [PATCH 18/23] Better logging for e2e tests + allow buffering of logs --- test/e2e/DockerTestContainer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/DockerTestContainer.js b/test/e2e/DockerTestContainer.js index ec1af3c..54b0f64 100644 --- a/test/e2e/DockerTestContainer.js +++ b/test/e2e/DockerTestContainer.js @@ -33,9 +33,9 @@ export class DockerTestContainer { ].join(" "); execSync( - `docker build -t ${imageName} -f ${dockerFile} ${contextPath} ${buildArgs}`, + `docker build --progress=plain -t ${imageName} -f ${dockerFile} ${contextPath} ${buildArgs}`, { - stdio: "ignore", + stdio: "inherit", } ); } catch (error) { From 0d1283a0fca9b568b0f4b3e1e507f34a2dece719 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 09:32:53 -0800 Subject: [PATCH 19/23] Pipe output for better logging --- test/e2e/DockerTestContainer.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/e2e/DockerTestContainer.js b/test/e2e/DockerTestContainer.js index 54b0f64..a7df63c 100644 --- a/test/e2e/DockerTestContainer.js +++ b/test/e2e/DockerTestContainer.js @@ -35,10 +35,14 @@ export class DockerTestContainer { execSync( `docker build --progress=plain -t ${imageName} -f ${dockerFile} ${contextPath} ${buildArgs}`, { - stdio: "inherit", + stdio: "pipe", + maxBuffer: 50 * 1024 * 1024, // 50MB buffer to capture verbose build logs } ); } catch (error) { + // Only print the build logs if the build fails + if (error.stdout) console.log(error.stdout.toString()); + if (error.stderr) console.error(error.stderr.toString()); throw new Error(`Failed to build Docker image: ${error.message}`); } } From cba1fc36af5b93e5b4d52d97777b65c202291228 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 10:45:24 -0800 Subject: [PATCH 20/23] Adapt DockerFile --- test/e2e/Dockerfile | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/e2e/Dockerfile b/test/e2e/Dockerfile index c8d9c9c..7813164 100644 --- a/test/e2e/Dockerfile +++ b/test/e2e/Dockerfile @@ -41,11 +41,12 @@ RUN apt-get install -y fish && \ touch /root/.config/fish/config.fish # Install Volta and Node.js -RUN curl https://get.volta.sh | bash -RUN volta install node@${NODE_VERSION} -RUN volta install npm@${NPM_VERSION} -RUN volta install yarn@${YARN_VERSION} -RUN volta install pnpm@${PNPM_VERSION} +RUN curl -sSL https://get.volta.sh | bash +ENV VOLTA_HOME="/root/.volta" +RUN ${VOLTA_HOME}/bin/volta install node@${NODE_VERSION} +RUN ${VOLTA_HOME}/bin/volta install npm@${NPM_VERSION} +RUN ${VOLTA_HOME}/bin/volta install yarn@${YARN_VERSION} +RUN ${VOLTA_HOME}/bin/volta install pnpm@${PNPM_VERSION} # Install Bun RUN curl -fsSL https://bun.sh/install | bash From 77408f90b6b00447cd20534bd7e17927600f2dc8 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 10:57:18 -0800 Subject: [PATCH 21/23] Fix flag --- test/e2e/Dockerfile | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/e2e/Dockerfile b/test/e2e/Dockerfile index 7813164..fdb645a 100644 --- a/test/e2e/Dockerfile +++ b/test/e2e/Dockerfile @@ -42,11 +42,10 @@ RUN apt-get install -y fish && \ # Install Volta and Node.js RUN curl -sSL https://get.volta.sh | bash -ENV VOLTA_HOME="/root/.volta" -RUN ${VOLTA_HOME}/bin/volta install node@${NODE_VERSION} -RUN ${VOLTA_HOME}/bin/volta install npm@${NPM_VERSION} -RUN ${VOLTA_HOME}/bin/volta install yarn@${YARN_VERSION} -RUN ${VOLTA_HOME}/bin/volta install pnpm@${PNPM_VERSION} +RUN /root/.volta/bin/volta install node@${NODE_VERSION} +RUN /root/.volta/bin/volta install npm@${NPM_VERSION} +RUN /root/.volta/bin/volta install yarn@${YARN_VERSION} +RUN /root/.volta/bin/volta install pnpm@${PNPM_VERSION} # Install Bun RUN curl -fsSL https://bun.sh/install | bash From dc25345b7ca0c886f67360877609957a9323bebe Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 13:08:23 -0800 Subject: [PATCH 22/23] Some tweaks --- test/e2e/DockerTestContainer.js | 2 +- test/e2e/Dockerfile | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/e2e/DockerTestContainer.js b/test/e2e/DockerTestContainer.js index a7df63c..95a467c 100644 --- a/test/e2e/DockerTestContainer.js +++ b/test/e2e/DockerTestContainer.js @@ -36,7 +36,7 @@ export class DockerTestContainer { `docker build --progress=plain -t ${imageName} -f ${dockerFile} ${contextPath} ${buildArgs}`, { stdio: "pipe", - maxBuffer: 50 * 1024 * 1024, // 50MB buffer to capture verbose build logs + maxBuffer: 10 * 1024 * 1024, // Default is 1MB, increase to 10MB to account for large build logs } ); } catch (error) { diff --git a/test/e2e/Dockerfile b/test/e2e/Dockerfile index fdb645a..bc7ffc2 100644 --- a/test/e2e/Dockerfile +++ b/test/e2e/Dockerfile @@ -41,11 +41,11 @@ RUN apt-get install -y fish && \ touch /root/.config/fish/config.fish # Install Volta and Node.js -RUN curl -sSL https://get.volta.sh | bash -RUN /root/.volta/bin/volta install node@${NODE_VERSION} -RUN /root/.volta/bin/volta install npm@${NPM_VERSION} -RUN /root/.volta/bin/volta install yarn@${YARN_VERSION} -RUN /root/.volta/bin/volta install pnpm@${PNPM_VERSION} +RUN curl -fsSL https://get.volta.sh | bash +RUN volta install node@${NODE_VERSION} +RUN volta install npm@${NPM_VERSION} +RUN volta install yarn@${YARN_VERSION} +RUN volta install pnpm@${PNPM_VERSION} # Install Bun RUN curl -fsSL https://bun.sh/install | bash From 7b2e8eef46cd8f6a56d800862c5ea3ac54ced450 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Mon, 15 Dec 2025 16:33:48 +0100 Subject: [PATCH 23/23] Fix build: install packages before setting the version --- .github/workflows/create-artifact.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/create-artifact.yml b/.github/workflows/create-artifact.yml index ad43a9d..5aa6422 100644 --- a/.github/workflows/create-artifact.yml +++ b/.github/workflows/create-artifact.yml @@ -5,7 +5,7 @@ on: workflow_call: inputs: version: - description: 'Version to set in package.json' + description: "Version to set in package.json" required: false type: string @@ -64,13 +64,13 @@ jobs: npm i -g @aikidosec/safe-chain safe-chain setup-ci - - name: Set the version in safe-chain package - if: inputs.version != '' - run: npm --no-git-tag-version version ${{ inputs.version }} --workspace=packages/safe-chain - - name: Install dependencies run: npm ci --ignore-scripts + - name: Set the version in safe-chain package + if: inputs.version != '' + run: npm --no-git-tag-version version ${{ inputs.version }} --workspace=packages/safe-chain --ignore-scripts + - name: Create binary run: | node build.js ${{ matrix.target }}