diff --git a/packages/safe-chain/bin/safe-chain.js b/packages/safe-chain/bin/safe-chain.js index 2793987..802005b 100755 --- a/packages/safe-chain/bin/safe-chain.js +++ b/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(); diff --git a/packages/safe-chain/src/config/configFile.js b/packages/safe-chain/src/config/configFile.js index ae25a1d..23387f5 100644 --- a/packages/safe-chain/src/config/configFile.js +++ b/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 ); diff --git a/packages/safe-chain/src/config/settings.js b/packages/safe-chain/src/config/settings.js index 7c20358..e1cec34 100644 --- a/packages/safe-chain/src/config/settings.js +++ b/packages/safe-chain/src/config/settings.js @@ -81,7 +81,7 @@ function validateMinimumPackageAgeHours(value) { return undefined; } - if (numericValue > 0) { + if (numericValue >= 0) { return numericValue; } 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..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. @@ -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); }); }); 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); + }); + }); diff --git a/packages/safe-chain/src/shell-integration/helpers.js b/packages/safe-chain/src/shell-integration/helpers.js index 50cea5d..3b08bf2 100644 --- a/packages/safe-chain/src/shell-integration/helpers.js +++ b/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 * diff --git a/packages/safe-chain/src/shell-integration/setup-ci.js b/packages/safe-chain/src/shell-integration/setup-ci.js index bc5c5e6..b0a8c83 100644 --- a/packages/safe-chain/src/shell-integration/setup-ci.js +++ b/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)) { diff --git a/packages/safe-chain/src/shell-integration/setup-ci.spec.js b/packages/safe-chain/src/shell-integration/setup-ci.spec.js index 92ef82e..b437157 100644 --- a/packages/safe-chain/src/shell-integration/setup-ci.spec.js +++ b/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, }, }); diff --git a/packages/safe-chain/src/shell-integration/setup.js b/packages/safe-chain/src/shell-integration/setup.js index d5c4be9..065de75 100644 --- a/packages/safe-chain/src/shell-integration/setup.js +++ b/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(os.homedir(), ".safe-chain", "scripts", file); + const targetPath = path.join(targetDir, file); if (!fs.existsSync(targetDir)) { fs.mkdirSync(targetDir, { recursive: true }); diff --git a/packages/safe-chain/src/shell-integration/teardown.js b/packages/safe-chain/src/shell-integration/teardown.js index bc83b48..de3fbd7 100644 --- a/packages/safe-chain/src/shell-integration/teardown.js +++ b/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} @@ -62,3 +63,44 @@ export async function teardown() { return; } } + +/** + * Removes directories created by setup-ci and setup commands + * @returns {Promise} + */ +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}` + ); + } + } +} diff --git a/test/e2e/teardown-dirs.e2e.spec.js b/test/e2e/teardown-dirs.e2e.spec.js new file mode 100644 index 0000000..0ed8bf6 --- /dev/null +++ b/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"); + }); +});