Merge branch 'main' into non-interactive-terminal-support

README.md

@@ -8,12 +8,16 @@ The Aikido Safe Chain wraps around the [npm cli](https://github.com/npm/cli), [n
 
 Aikido Safe Chain works on Node.js version 18 and above and supports the following package managers:
 
-- ✅ **npm**
+- ✅ full coverage: **npm >= 10.4.0**:
-- ✅ **npx**
+- ⚠️ limited to scanning the install command arguments (broader scanning coming soon):
-- ✅ **yarn**
+  - **npm < 10.4.0**
-- ✅ **pnpm**
+  - **npx**
-- ✅ **pnpx**
+  - **yarn**
-- 🚧 **bun** Coming soon
+  - **pnpm**
+  - **pnpx**
+- 🚧 **bun**: coming soon
+
+Note on the limited support for npm < 10.4.0, npx, yarn, pnpm and pnpx: adding **full support for these package managers is a high priority**. In the meantime, we offer limited support already, which means that the Aikido Safe Chain will scan the package names passed as arguments to the install commands. However, it will not scan the full dependency tree of these packages.
 
 # Usage
 

packages/safe-chain/src/main.js

@@ -15,6 +15,7 @@ export async function main(args) {
     }
   } catch (error) {
     ui.writeError("Failed to check for malicious packages:", error.message);
+    process.exit(1);
   }
 
   var result = getPackageManager().runCommand(args);

packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js

@@ -1,4 +1,3 @@
-import { ui } from "../../../environment/userInteraction.js";
 import { parseDryRunOutput } from "../parsing/parseNpmInstallDryRunOutput.js";
 import { dryRunNpmCommandAndOutput } from "../runNpmCommand.js";
 import { hasDryRunArg } from "../utils/npmCommands.js";
@@ -37,10 +36,17 @@ function checkChangesWithDryRun(args) {
 
   // Dry-run can return a non-zero status code in some cases
   //  e.g., when running "npm audit fix --dry-run", it returns exit code 1
-  //  when there are vulnurabilities that can be fixed.
+  //  when there are vulnerabilities that can be fixed.
+  if (dryRunOutput.status !== 0 && !canCommandReturnNonZeroOnSuccess(args)) {
+    throw new Error(
+      `Dry-run command failed with exit code ${dryRunOutput.status} and output:\n${dryRunOutput.output}`
+    );
+  }
+
   if (dryRunOutput.status !== 0 && !dryRunOutput.output) {
-    ui.writeError("Detecting changes failed.");
+    throw new Error(
-    return [];
+      `Dry-run command failed with exit code ${dryRunOutput.status} and produced no output.`
+    );
   }
 
   const parsedOutput = parseDryRunOutput(dryRunOutput.output);
@@ -48,3 +54,13 @@ function checkChangesWithDryRun(args) {
   // reverse the array to have the top-level packages first
   return parsedOutput.reverse();
 }
+
+function canCommandReturnNonZeroOnSuccess(args) {
+  if (args.length < 2) {
+    return false;
+  }
+
+  // `npm audit fix --dry-run` can return exit code 1 when it succesfully ran and
+  // there were vulnerabilities that could be fixed
+  return args[0] === "audit" && args[1] === "fix";
+}

packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.spec.js

@@ -0,0 +1,139 @@
+import { describe, it, mock } from "node:test";
+import assert from "node:assert/strict";
+
+describe("dryRunScanner", async () => {
+  const mockWriteError = mock.fn();
+  const mockDryRunNpmCommandAndOutput = mock.fn();
+
+  // Mock ui module
+  mock.module("../../../environment/userInteraction.js", {
+    namedExports: {
+      ui: {
+        writeError: mockWriteError,
+      },
+    },
+  });
+
+  // Mock dryRunNpmCommandAndOutput function
+  mock.module("../runNpmCommand.js", {
+    namedExports: {
+      dryRunNpmCommandAndOutput: mockDryRunNpmCommandAndOutput,
+    },
+  });
+
+  const { dryRunScanner } = await import("./dryRunScanner.js");
+
+  describe("doesCommandReturnNonZero", () => {
+    // We need to access the internal function for testing
+    // Since it's not exported, we'll test it indirectly through the main functionality
+
+    it("should handle npm audit fix commands that return non-zero", async () => {
+      mockDryRunNpmCommandAndOutput.mock.resetCalls();
+      mockWriteError.mock.resetCalls();
+      mockDryRunNpmCommandAndOutput.mock.mockImplementationOnce(() => ({
+        status: 1,
+        output: "found 5 vulnerabilities that can be fixed",
+      }));
+
+      const scanner = dryRunScanner();
+      const result = scanner.scan(["audit", "fix"]);
+
+      // Should not throw an error for audit fix commands
+      assert.ok(Array.isArray(result));
+      assert.equal(mockWriteError.mock.callCount(), 0);
+    });
+
+    it("should throw error for unexpected non-zero exit codes", async () => {
+      mockDryRunNpmCommandAndOutput.mock.resetCalls();
+      mockWriteError.mock.resetCalls();
+      mockDryRunNpmCommandAndOutput.mock.mockImplementationOnce(() => ({
+        status: 1,
+        output: "some error output",
+      }));
+
+      const scanner = dryRunScanner();
+
+      assert.throws(() => {
+        scanner.scan(["install", "lodash"]);
+      }, /Dry-run command failed with exit code 1/);
+    });
+
+    it("should handle zero exit codes normally", async () => {
+      mockDryRunNpmCommandAndOutput.mock.resetCalls();
+      mockWriteError.mock.resetCalls();
+      mockDryRunNpmCommandAndOutput.mock.mockImplementationOnce(() => ({
+        status: 0,
+        output: "added 1 package",
+      }));
+
+      const scanner = dryRunScanner();
+      const result = scanner.scan(["install", "lodash"]);
+
+      assert.ok(Array.isArray(result));
+      assert.equal(mockWriteError.mock.callCount(), 0);
+    });
+
+    it("should throw error for non-zero exit with no output for audit fix", async () => {
+      mockDryRunNpmCommandAndOutput.mock.resetCalls();
+      mockWriteError.mock.resetCalls();
+      mockDryRunNpmCommandAndOutput.mock.mockImplementationOnce(() => ({
+        status: 1,
+        output: "",
+      }));
+
+      const scanner = dryRunScanner();
+
+      assert.throws(() => {
+        scanner.scan(["audit", "fix"]);
+      }, /Dry-run command failed with exit code 1/);
+    });
+  });
+
+  describe("scanner functionality", () => {
+    it("should use dryRunCommand option when provided", async () => {
+      mockDryRunNpmCommandAndOutput.mock.resetCalls();
+      mockWriteError.mock.resetCalls();
+      mockDryRunNpmCommandAndOutput.mock.mockImplementationOnce(() => ({
+        status: 0,
+        output: "no changes",
+      }));
+
+      const scanner = dryRunScanner({ dryRunCommand: "install" });
+      scanner.scan(["install-test", "lodash"]);
+
+      // Should call with "install" instead of "install-test"
+      assert.equal(mockDryRunNpmCommandAndOutput.mock.callCount(), 1);
+      const calledArgs =
+        mockDryRunNpmCommandAndOutput.mock.calls[0].arguments[0];
+      assert.deepEqual(calledArgs, ["install", "lodash"]);
+    });
+
+    it("should skip scanning when hasDryRunArg returns true", async () => {
+      mockDryRunNpmCommandAndOutput.mock.resetCalls();
+      mockWriteError.mock.resetCalls();
+
+      const scanner = dryRunScanner();
+      const shouldScan = scanner.shouldScan(["install", "--dry-run"]);
+
+      assert.equal(shouldScan, false);
+      // Should not call dryRunNpmCommandAndOutput since scanning is skipped
+      assert.equal(mockDryRunNpmCommandAndOutput.mock.callCount(), 0);
+    });
+
+    it("should skip scanning when skipScanWhen returns true", async () => {
+      const scanner = dryRunScanner({
+        skipScanWhen: (args) => args.includes("--skip"),
+      });
+      const shouldScan = scanner.shouldScan(["install", "--skip"]);
+
+      assert.equal(shouldScan, false);
+    });
+
+    it("should scan when conditions are met", async () => {
+      const scanner = dryRunScanner();
+      const shouldScan = scanner.shouldScan(["install", "lodash"]);
+
+      assert.equal(shouldScan, true);
+    });
+  });
+});

packages/safe-chain/src/packagemanager/npm/runNpmCommand.js

@@ -18,7 +18,7 @@ export function runNpm(args) {
 
 export function dryRunNpmCommandAndOutput(args) {
   try {
-    const npmCommand = `npm ${args.join(" ")} --dry-run`;
+    const npmCommand = `npm ${args.join(" ")} --ignore-scripts --dry-run`;
     const output = execSync(npmCommand, { stdio: "pipe" });
     return { status: 0, output: output.toString() };
   } catch (error) {

packages/safe-chain/src/scanning/index.js

@@ -21,7 +21,9 @@ export async function scanCommand(args) {
 
   let timedOut = false;
 
-  const spinner = ui.startProcess("Scanning for malicious packages...");
+  const spinner = ui.startProcess(
+    "Safe-chain: Scanning for malicious packages..."
+  );
   let audit;
 
   await Promise.race([
@@ -37,12 +39,14 @@ export async function scanCommand(args) {
         }
 
         if (changes.length > 0) {
-          spinner.setText(`Scanning ${changes.length} package(s)...`);
+          spinner.setText(
+            `Safe-chain: Scanning ${changes.length} package(s)...`
+          );
         }
 
         audit = await auditChanges(changes);
       } catch (error) {
-        spinner.fail(`Error while scanning: ${error.message}`);
+        spinner.fail(`Safe-chain: Error while scanning.`);
         throw error;
       }
     })(),
@@ -52,12 +56,12 @@ export async function scanCommand(args) {
   ]);
 
   if (timedOut) {
-    spinner.fail("Timeout exceeded while scanning.");
+    spinner.fail("Safe-chain: Timeout exceeded while scanning.");
     throw new Error("Timeout exceeded while scanning npm install command.");
   }
 
   if (!audit || audit.isAllowed) {
-    spinner.succeed("No malicious packages detected.");
+    spinner.succeed("Safe-chain: No malicious packages detected.");
   } else {
     printMaliciousChanges(audit.disallowedChanges, spinner);
     await onMalwareFound();
@@ -65,7 +69,7 @@ export async function scanCommand(args) {
 }
 
 function printMaliciousChanges(changes, spinner) {
-  spinner.fail(chalk.bold("Malicious changes detected:"));
+  spinner.fail("Safe-chain: " + chalk.bold("Malicious changes detected:"));
 
   for (const change of changes) {
     ui.writeInformation(` - ${change.name}@${change.version}`);

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

@@ -1,6 +1,7 @@
 import { spawnSync } from "child_process";
 import * as os from "os";
 import fs from "fs";
+import path from "path";
 
 export const knownAikidoTools = [
   { tool: "npm", aikidoCommand: "aikido-npm" },
@@ -81,13 +82,24 @@ function shouldRemoveLine(line, pattern) {
 }
 
 export function addLineToFile(filePath, line, eol) {
-  if (!fs.existsSync(filePath)) {
+  createFileIfNotExists(filePath);
-    fs.writeFileSync(filePath, "", "utf-8");
-  }
 
   eol = eol || os.EOL;
 
   const fileContent = fs.readFileSync(filePath, "utf-8");
-  const updatedContent = fileContent + eol + line;
+  const updatedContent = fileContent + eol + line + eol;
   fs.writeFileSync(filePath, updatedContent, "utf-8");
 }
+
+function createFileIfNotExists(filePath) {
+  if (fs.existsSync(filePath)) {
+    return;
+  }
+
+  const dir = path.dirname(filePath);
+  if (!fs.existsSync(dir)) {
+    fs.mkdirSync(dir, { recursive: true });
+  }
+
+  fs.writeFileSync(filePath, "", "utf-8");
+}

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

@@ -56,11 +56,13 @@ export async function setup() {
  */
 function setupShell(shell) {
   let success = false;
+  let error;
   try {
     shell.teardown(knownAikidoTools); // First, tear down to prevent duplicate aliases
     success = shell.setup(knownAikidoTools);
-  } catch {
+  } catch (err) {
     success = false;
+    error = err;
   }
 
   if (success) {
@@ -75,6 +77,13 @@ function setupShell(shell) {
         "Setup failed"
       )}. Please check your ${shell.name} configuration.`
     );
+    if (error) {
+      let message = `  Error: ${error.message}`;
+      if (error.code) {
+        message += ` (code: ${error.code})`;
+      }
+      ui.writeError(message);
+    }
   }
 
   return success;