Merge branch 'main' into disable-mac-unit-tests

packages/safe-chain/bin/safe-chain.js

@@ -3,7 +3,7 @@
 import chalk from "chalk";
 import { ui } from "../src/environment/userInteraction.js";
 import { setup } from "../src/shell-integration/setup.js";
-import { teardown } from "../src/shell-integration/teardown.js";
+import { teardown, teardownDirectories } from "../src/shell-integration/teardown.js";
 import { setupCi } from "../src/shell-integration/setup-ci.js";
 import { initializeCliArguments } from "../src/config/cliArguments.js";
 import { setEcoSystem } from "../src/config/settings.js";
@@ -60,6 +60,7 @@ if (tool) {
 } else if (command === "setup") {
   setup();
 } else if (command === "teardown") {
+  teardownDirectories();
   teardown();
 } else if (command === "setup-ci") {
   setupCi();

packages/safe-chain/src/config/configFile.js

@@ -67,7 +67,7 @@ function validateMinimumPackageAgeHours(value) {
  */
 export function getMinimumPackageAgeHours() {
   const config = readConfigFile();
-  if (config.minimumPackageAgeHours) {
+  if (config.minimumPackageAgeHours !== undefined) {
     const validated = validateMinimumPackageAgeHours(
       config.minimumPackageAgeHours
     );

packages/safe-chain/src/config/settings.js

@@ -81,7 +81,7 @@ function validateMinimumPackageAgeHours(value) {
     return undefined;
   }
 
-  if (numericValue > 0) {
+  if (numericValue >= 0) {
     return numericValue;
   }
 

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

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.
@@ -16,7 +17,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)) {
@@ -77,14 +78,16 @@ 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) => {
+        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);
       });
     });

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);
+  });
+
 });

packages/safe-chain/src/shell-integration/helpers.js

@@ -113,6 +113,20 @@ export function getPackageManagerList() {
   return `${tools.join(", ")}, and ${lastTool} commands`;
 }
 
+/**
+ * @returns {string}
+ */
+export function getShimsDir() {
+  return path.join(os.homedir(), ".safe-chain", "shims");
+}
+
+/**
+ * @returns {string}
+ */
+export function getScriptsDir() {
+  return path.join(os.homedir(), ".safe-chain", "scripts");
+}
+
 /**
  * @param {string} executableName
  *

packages/safe-chain/src/shell-integration/setup-ci.js

@@ -1,6 +1,6 @@
 import chalk from "chalk";
 import { ui } from "../environment/userInteraction.js";
-import { getPackageManagerList, knownAikidoTools } from "./helpers.js";
+import { getPackageManagerList, knownAikidoTools, getShimsDir } from "./helpers.js";
 import fs from "fs";
 import os from "os";
 import path from "path";
@@ -32,7 +32,7 @@ export async function setupCi() {
   );
   ui.emptyLine();
 
-  const shimsDir = path.join(os.homedir(), ".safe-chain", "shims");
+  const shimsDir = getShimsDir();
   const binDir = path.join(os.homedir(), ".safe-chain", "bin");
   // Create the shims directory if it doesn't exist
   if (!fs.existsSync(shimsDir)) {

packages/safe-chain/src/shell-integration/setup-ci.spec.js

@@ -50,6 +50,7 @@ describe("Setup CI shell integration", () => {
           { tool: "yarn", aikidoCommand: "aikido-yarn" },
         ],
         getPackageManagerList: () => "npm, yarn",
+        getShimsDir: () => mockShimsDir,
       },
     });
 

packages/safe-chain/src/shell-integration/setup.js

@@ -1,9 +1,8 @@
 import chalk from "chalk";
 import { ui } from "../environment/userInteraction.js";
 import { detectShells } from "./shellDetection.js";
-import { knownAikidoTools, getPackageManagerList } from "./helpers.js";
+import { knownAikidoTools, getPackageManagerList, getScriptsDir } from "./helpers.js";
 import fs from "fs";
-import os from "os";
 import path from "path";
 import { includePython } from "../config/cliArguments.js";
 import { fileURLToPath } from "url";
@@ -107,10 +106,10 @@ function setupShell(shell) {
 
 function copyStartupFiles() {
   const startupFiles = ["init-posix.sh", "init-pwsh.ps1", "init-fish.fish"];
+  const targetDir = getScriptsDir();
 
   for (const file of startupFiles) {
-    const targetDir = path.join(os.homedir(), ".safe-chain", "scripts");
+    const targetPath = path.join(targetDir, file);
-    const targetPath = path.join(os.homedir(), ".safe-chain", "scripts", file);
 
     if (!fs.existsSync(targetDir)) {
       fs.mkdirSync(targetDir, { recursive: true });

packages/safe-chain/src/shell-integration/teardown.js

@@ -1,7 +1,8 @@
 import chalk from "chalk";
 import { ui } from "../environment/userInteraction.js";
 import { detectShells } from "./shellDetection.js";
-import { knownAikidoTools, getPackageManagerList } from "./helpers.js";
+import { knownAikidoTools, getPackageManagerList, getShimsDir, getScriptsDir } from "./helpers.js";
+import fs from "fs";
 
 /**
  * @returns {Promise<void>}
@@ -62,3 +63,44 @@ export async function teardown() {
     return;
   }
 }
+
+/**
+ * Removes directories created by setup-ci and setup commands
+ * @returns {Promise<void>}
+ */
+export async function teardownDirectories() {
+  const shimsDir = getShimsDir();
+  const scriptsDir = getScriptsDir();
+
+  // Remove CI shims directory
+  if (fs.existsSync(shimsDir)) {
+    try {
+      fs.rmSync(shimsDir, { recursive: true, force: true });
+      ui.writeInformation(
+        `${chalk.bold("- CI Shims:")} ${chalk.green("Removed successfully")}`
+      );
+    } catch (/** @type {any} */ error) {
+      ui.writeError(
+        `${chalk.bold("- CI Shims:")} ${chalk.red(
+          "Failed to remove"
+        )}. Error: ${error.message}`
+      );
+    }
+  }
+
+  // Remove scripts directory
+  if (fs.existsSync(scriptsDir)) {
+    try {
+      fs.rmSync(scriptsDir, { recursive: true, force: true });
+      ui.writeInformation(
+        `${chalk.bold("- Scripts:")} ${chalk.green("Removed successfully")}`
+      );
+    } catch (/** @type {any} */ error) {
+      ui.writeError(
+        `${chalk.bold("- Scripts:")} ${chalk.red(
+          "Failed to remove"
+        )}. Error: ${error.message}`
+      );
+    }
+  }
+}

test/e2e/teardown-dirs.e2e.spec.js

@@ -0,0 +1,99 @@
+import { describe, it, before, beforeEach, afterEach } from "node:test";
+import { DockerTestContainer } from "./DockerTestContainer.js";
+import assert from "node:assert";
+
+describe("E2E: safe-chain teardown command", () => {
+  let container;
+
+  before(async () => {
+    DockerTestContainer.buildImage();
+  });
+
+  beforeEach(async () => {
+    container = new DockerTestContainer();
+    await container.start();
+  });
+
+  afterEach(async () => {
+    if (container) {
+      await container.stop();
+      container = null;
+    }
+  });
+
+  it("safe-chain teardown removes shims directory created by setup-ci", async () => {
+    const shell = await container.openShell("bash");
+
+    // Run setup-ci
+    await shell.runCommand("safe-chain setup-ci");
+
+    // Verify shims directory exists
+    const checkShimsExist = await shell.runCommand("test -d ~/.safe-chain/shims && echo 'exists' || echo 'missing'");
+    assert.ok(checkShimsExist.output.includes("exists"), "Shims directory should exist after setup-ci");
+
+    // Run teardown
+    await shell.runCommand("safe-chain teardown");
+
+    // Verify shims directory is gone
+    const checkShimsGone = await shell.runCommand("test -d ~/.safe-chain/shims && echo 'exists' || echo 'missing'");
+    assert.ok(checkShimsGone.output.includes("missing"), "Shims directory should be removed after teardown");
+  });
+
+  it("safe-chain teardown removes scripts directory created by setup", async () => {
+    const shell = await container.openShell("bash");
+
+    // Run setup
+    await shell.runCommand("safe-chain setup");
+
+    // Verify scripts directory exists
+    const checkScriptsExist = await shell.runCommand("test -d ~/.safe-chain/scripts && echo 'exists' || echo 'missing'");
+    assert.ok(checkScriptsExist.output.includes("exists"), "Scripts directory should exist after setup");
+
+    // Run teardown
+    await shell.runCommand("safe-chain teardown");
+
+    // Verify scripts directory is gone
+    const checkScriptsGone = await shell.runCommand("test -d ~/.safe-chain/scripts && echo 'exists' || echo 'missing'");
+    assert.ok(checkScriptsGone.output.includes("missing"), "Scripts directory should be removed after teardown");
+  });
+
+  it("safe-chain teardown removes shims directory created by setup-ci --include-python", async () => {
+    const shell = await container.openShell("bash");
+
+    // Run setup-ci with --include-python
+    await shell.runCommand("safe-chain setup-ci --include-python");
+
+    // Verify shims directory exists
+    const checkShimsExist = await shell.runCommand("test -d ~/.safe-chain/shims && echo 'exists' || echo 'missing'");
+    assert.ok(checkShimsExist.output.includes("exists"), "Shims directory should exist after setup-ci --include-python");
+
+    // Verify Python shims were created
+    const checkPythonShims = await shell.runCommand("test -f ~/.safe-chain/shims/pip && echo 'exists' || echo 'missing'");
+    assert.ok(checkPythonShims.output.includes("exists"), "Python shims should exist after setup-ci --include-python");
+
+    // Run teardown
+    await shell.runCommand("safe-chain teardown");
+
+    // Verify shims directory is gone
+    const checkShimsGone = await shell.runCommand("test -d ~/.safe-chain/shims && echo 'exists' || echo 'missing'");
+    assert.ok(checkShimsGone.output.includes("missing"), "Shims directory should be removed after teardown");
+  });
+
+  it("safe-chain teardown removes scripts directory created by setup --include-python", async () => {
+    const shell = await container.openShell("bash");
+
+    // Run setup with --include-python
+    await shell.runCommand("safe-chain setup --include-python");
+
+    // Verify scripts directory exists
+    const checkScriptsExist = await shell.runCommand("test -d ~/.safe-chain/scripts && echo 'exists' || echo 'missing'");
+    assert.ok(checkScriptsExist.output.includes("exists"), "Scripts directory should exist after setup --include-python");
+
+    // Run teardown
+    await shell.runCommand("safe-chain teardown");
+
+    // Verify scripts directory is gone
+    const checkScriptsGone = await shell.runCommand("test -d ~/.safe-chain/scripts && echo 'exists' || echo 'missing'");
+    assert.ok(checkScriptsGone.output.includes("missing"), "Scripts directory should be removed after teardown");
+  });
+});