From 7a9a6418a5aa9c5dfaf47a45a82a8201f181a956 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 09:06:50 -0800 Subject: [PATCH 1/8] 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 2daddace31ff5e5987f72f4ee9be9054a9bdd898 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 09:32:53 -0800 Subject: [PATCH 2/8] 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 c385f9b371e24a0de0d694e0c30284b2c043bf8f Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 10:45:24 -0800 Subject: [PATCH 3/8] 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 a9a7a37f6a868a0e16e0f29f5982f203cb5e182d Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 10:57:18 -0800 Subject: [PATCH 4/8] 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 df66863ae5d4853803c6bfa182e5c231e7edb6da Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Thu, 11 Dec 2025 13:08:23 -0800 Subject: [PATCH 5/8] 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 64d87ae1e127e7a68ed005a2207dac74800670e5 Mon Sep 17 00:00:00 2001 From: Uriel Corfa Date: Thu, 11 Dec 2025 13:56:58 +0100 Subject: [PATCH 6/8] Flush buffered logs before exiting --- packages/safe-chain/src/main.js | 3 +++ packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 3 +++ 2 files changed, 6 insertions(+) diff --git a/packages/safe-chain/src/main.js b/packages/safe-chain/src/main.js index 38bb8ff..0e895b3 100644 --- a/packages/safe-chain/src/main.js +++ b/packages/safe-chain/src/main.js @@ -23,6 +23,7 @@ export async function main(args) { process.on("uncaughtException", (error) => { ui.writeError(`Safe-chain: Uncaught exception: ${error.message}`); ui.writeVerbose(`Stack trace: ${error.stack}`); + ui.writeBufferedLogsAndStopBuffering(); process.exit(1); }); @@ -31,6 +32,7 @@ export async function main(args) { if (reason instanceof Error) { ui.writeVerbose(`Stack trace: ${reason.stack}`); } + ui.writeBufferedLogsAndStopBuffering(); process.exit(1); }); @@ -89,6 +91,7 @@ export async function main(args) { return packageManagerResult.status; } catch (/** @type any */ error) { ui.writeError("Failed to check for malicious packages:", error.message); + ui.writeBufferedLogsAndStopBuffering(); // Returning the exit code back to the caller allows the promise // to be awaited in the bin files and return the correct exit code diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index e9f05c7..0e08b13 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -81,10 +81,13 @@ export async function runPip(command, args) { return new Promise((_resolve) => { const proc = spawn(command, args, { stdio: "inherit" }); proc.on("exit", (/** @type {number | null} */ code) => { + ui.writeVerbose(`${command} ${args.join(" ")} exited with status ${code}`); + ui.writeBufferedLogsAndStopBuffering(); process.exit(code ?? 0); }); proc.on("error", (/** @type {Error} */ err) => { ui.writeError(`Error executing command: ${err.message}`); + ui.writeBufferedLogsAndStopBuffering(); process.exit(1); }); }); From db2c272aea8a07154a2993308c2c95da29640124 Mon Sep 17 00:00:00 2001 From: Uriel Corfa Date: Thu, 11 Dec 2025 13:58:46 +0100 Subject: [PATCH 7/8] Add a unit test for shouldBypassSafeChain --- .../src/packagemanager/pip/runPipCommand.js | 2 +- .../packagemanager/pip/runPipCommand.spec.js | 43 +++++++++++++------ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index 0e08b13..ad0d76d 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -16,7 +16,7 @@ import ini from "ini"; * @param {string[]} args - The arguments * @returns {boolean} */ -function shouldBypassSafeChain(command, args) { +export function shouldBypassSafeChain(command, args) { if (command === PYTHON_COMMAND || command === PYTHON3_COMMAND) { // Check if args start with -m pip if (args.length >= 2 && args[0] === "-m" && (args[1] === PIP_COMMAND || args[1] === PIP3_COMMAND)) { diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js index cf121f6..0707333 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.spec.js @@ -7,6 +7,7 @@ import ini from "ini"; describe("runPipCommand environment variable handling", () => { let runPip; + let shouldBypassSafeChain; let capturedArgs = null; let customEnv = null; let capturedConfigContent = null; // Capture config file content before cleanup @@ -56,6 +57,7 @@ describe("runPipCommand environment variable handling", () => { const mod = await import("./runPipCommand.js"); runPip = mod.runPip; + shouldBypassSafeChain = mod.shouldBypassSafeChain; }); afterEach(() => { @@ -66,14 +68,14 @@ describe("runPipCommand environment variable handling", () => { const res = await runPip("pip3", ["config", "set", "global.index-url", "https://test.pypi.org/simple"]); assert.strictEqual(res.status, 0); assert.ok(capturedArgs, "safeSpawn should have been called"); - + // PIP_CONFIG_FILE should NOT be set for config commands assert.strictEqual( capturedArgs.options.env.PIP_CONFIG_FILE, undefined, "PIP_CONFIG_FILE should NOT be set for pip config commands" ); - + // But CA environment variables should still be set assert.strictEqual( capturedArgs.options.env.REQUESTS_CA_BUNDLE, @@ -96,7 +98,7 @@ describe("runPipCommand environment variable handling", () => { const res = await runPip("pip3", ["config", "get", "global.index-url"]); assert.strictEqual(res.status, 0); assert.ok(capturedArgs, "safeSpawn should have been called"); - + assert.strictEqual( capturedArgs.options.env.PIP_CONFIG_FILE, undefined, @@ -108,7 +110,7 @@ describe("runPipCommand environment variable handling", () => { const res = await runPip("pip3", ["config", "list"]); assert.strictEqual(res.status, 0); assert.ok(capturedArgs, "safeSpawn should have been called"); - + assert.strictEqual( capturedArgs.options.env.PIP_CONFIG_FILE, undefined, @@ -120,13 +122,13 @@ describe("runPipCommand environment variable handling", () => { const res = await runPip("pip3", ["cache", "dir"]); assert.strictEqual(res.status, 0); assert.ok(capturedArgs, "safeSpawn should have been called"); - + assert.strictEqual( capturedArgs.options.env.PIP_CONFIG_FILE, undefined, "PIP_CONFIG_FILE should NOT be set for pip cache commands" ); - + // CA env vars should still be set assert.strictEqual( capturedArgs.options.env.SSL_CERT_FILE, @@ -139,7 +141,7 @@ describe("runPipCommand environment variable handling", () => { const res = await runPip("pip3", ["debug"]); assert.strictEqual(res.status, 0); assert.ok(capturedArgs, "safeSpawn should have been called"); - + assert.strictEqual( capturedArgs.options.env.PIP_CONFIG_FILE, undefined, @@ -151,7 +153,7 @@ describe("runPipCommand environment variable handling", () => { const res = await runPip("pip3", ["completion", "--bash"]); assert.strictEqual(res.status, 0); assert.ok(capturedArgs, "safeSpawn should have been called"); - + assert.strictEqual( capturedArgs.options.env.PIP_CONFIG_FILE, undefined, @@ -181,7 +183,7 @@ describe("runPipCommand environment variable handling", () => { assert.strictEqual(res.status, 0); assert.ok(capturedArgs, "safeSpawn should have been called"); - + // Check environment variables are set assert.strictEqual( capturedArgs.options.env.REQUESTS_CA_BUNDLE, @@ -218,7 +220,7 @@ describe("runPipCommand environment variable handling", () => { // For default PyPI, we still set env vars; pip CLI --cert takes precedence const res = await runPip("pip3", ["install", "requests"]); assert.strictEqual(res.status, 0); - + // Environment variables still set (pip CLI --cert takes precedence) assert.strictEqual( capturedArgs.options.env.REQUESTS_CA_BUNDLE, @@ -233,7 +235,7 @@ describe("runPipCommand environment variable handling", () => { it("should preserve HTTPS_PROXY from proxy merge", async () => { const res = await runPip("pip3", ["install", "requests"]); assert.strictEqual(res.status, 0); - + assert.strictEqual( capturedArgs.options.env.HTTPS_PROXY, "http://localhost:8080", @@ -380,7 +382,7 @@ describe("runPipCommand environment variable handling", () => { await fs.writeFile(cfgPath, initialIni, "utf-8"); customEnv = { PIP_CONFIG_FILE: cfgPath }; - + // Capture stdout/stderr let output = ""; const originalWrite = process.stdout.write; @@ -397,4 +399,21 @@ describe("runPipCommand environment variable handling", () => { assert.ok(output.includes("proxy found in PIP_CONFIG_FILE"), "Should warn about proxy overwrite in output"); customEnv = null; }); + + it("should bypass safe-chain for python correctly", async () => { + assert.strictEqual(shouldBypassSafeChain("python", []), true); + assert.strictEqual(shouldBypassSafeChain("python3", []), true); + + assert.strictEqual(shouldBypassSafeChain("python", ["--version"]), true); + assert.strictEqual(shouldBypassSafeChain("python3", ["--version"]), true); + + assert.strictEqual(shouldBypassSafeChain("python", ["-m", "http.server"]), true); + assert.strictEqual(shouldBypassSafeChain("python3", ["-m", "http.server"]), true); + + assert.strictEqual(shouldBypassSafeChain("python", ["-m", "pip"]), false); + assert.strictEqual(shouldBypassSafeChain("python3", ["-m", "pip"]), false); + assert.strictEqual(shouldBypassSafeChain("python", ["-m", "pip3"]), false); + assert.strictEqual(shouldBypassSafeChain("python3", ["-m", "pip3"]), false); + }); + }); From cb9f3ee145cbb5e133fe2b0fdca309b2c1b9b68c Mon Sep 17 00:00:00 2001 From: Uriel Corfa Date: Thu, 11 Dec 2025 13:58:56 +0100 Subject: [PATCH 8/8] Do not rely on asynchronous import of child_process. Importing child_process asynchronously causes loader errors when running the binary dist: $ ./dist/safe-chain python --safe-chain-logging=verbose Safe-chain: Bypassing safe-chain for non-pip invocation: python Failed to check for malicious packages: A dynamic import callback was not specified. $ Relying on a regular import does not cause this issue. There is no obvious reason for this import to be dynamic (in particular, there are no tests using this to mock the spawn function), so let's simplify. --- packages/safe-chain/src/packagemanager/pip/runPipCommand.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js index ad0d76d..83bc03e 100644 --- a/packages/safe-chain/src/packagemanager/pip/runPipCommand.js +++ b/packages/safe-chain/src/packagemanager/pip/runPipCommand.js @@ -8,6 +8,7 @@ import fsSync from "node:fs"; import os from "node:os"; import path from "node:path"; import ini from "ini"; +import { spawn } from "child_process"; /** * Checks if this pip invocation should bypass safe-chain and spawn directly. @@ -77,7 +78,6 @@ export async function runPip(command, args) { if (shouldBypassSafeChain(command, args)) { ui.writeVerbose(`Safe-chain: Bypassing safe-chain for non-pip invocation: ${command} ${args.join(" ")}`); // Spawn the ORIGINAL command with ORIGINAL args - const { spawn } = await import("child_process"); return new Promise((_resolve) => { const proc = spawn(command, args, { stdio: "inherit" }); proc.on("exit", (/** @type {number | null} */ code) => {