From f793bb84670de534362ebf358c9d08edca2f9727 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Wed, 17 Sep 2025 15:26:06 +0200 Subject: [PATCH 1/6] Check if directory exists before creating a new shell startup file --- .../src/shell-integration/helpers.js | 29 +++++++++++++++---- .../safe-chain/src/shell-integration/setup.js | 11 ++++++- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/safe-chain/src/shell-integration/helpers.js b/packages/safe-chain/src/shell-integration/helpers.js index 7efface..9785e0a 100644 --- a/packages/safe-chain/src/shell-integration/helpers.js +++ b/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" }, @@ -43,12 +44,17 @@ function shouldRemoveLine(line, pattern) { if (line.length > maxLineLength) { // safe-chain only adds lines shorter than maxLineLength - // so if the line is longer, it must be from a different + // so if the line is longer, it must be from a different // source and could be dangerous to remove return false; } - if (line.includes("\n") || line.includes("\r") || line.includes("\u2028") || line.includes("\u2029")) { + if ( + line.includes("\n") || + line.includes("\r") || + line.includes("\u2028") || + line.includes("\u2029") + ) { // If the line contains newlines, something has gone wrong in splitting // \u2028 and \u2029 are Unicode line separator characters (line and paragraph separators) return false; @@ -58,11 +64,22 @@ function shouldRemoveLine(line, pattern) { } export function addLineToFile(filePath, line) { - if (!fs.existsSync(filePath)) { - fs.writeFileSync(filePath, "", "utf-8"); - } + createFileIfNotExists(filePath); const fileContent = fs.readFileSync(filePath, "utf-8"); - const updatedContent = fileContent + os.EOL + line; + const updatedContent = fileContent + os.EOL + line + os.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"); +} diff --git a/packages/safe-chain/src/shell-integration/setup.js b/packages/safe-chain/src/shell-integration/setup.js index 792aa3c..b58eb51 100644 --- a/packages/safe-chain/src/shell-integration/setup.js +++ b/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; From 7ccae17473e4f57537b80ef5cc49b7325f5deed1 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Thu, 18 Sep 2025 07:56:49 +0200 Subject: [PATCH 2/6] Better reflect how package managers are scanning in README --- README.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index e6fb9b5..63bbaf5 100644 --- a/README.md +++ b/README.md @@ -8,12 +8,14 @@ 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** -- ✅ **npx** -- ✅ **yarn** -- ✅ **pnpm** -- ✅ **pnpx** -- 🚧 **bun** Coming soon +- ✅ full coverage: **npm >= 10.4.0**: +- ⚠️ limited to scanning the install command arguments (broader scanning coming soon): + - **npm < 10.4.0** + - **npx** + - **yarn** + - **pnpm** + - **pnpx** +- 🚧 **bun**: coming soon # Usage From 8dda3190e33ed7f424e44fc5bfd65bb6449968cf Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Thu, 18 Sep 2025 09:35:02 +0200 Subject: [PATCH 3/6] Add extra note on the limited supported package managers. --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 63bbaf5..45317a0 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,8 @@ Aikido Safe Chain works on Node.js version 18 and above and supports the followi - **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 ## Installation From 528a60c1662778e4cbb7baf9a0817e55c6e6ffc9 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Fri, 19 Sep 2025 08:52:42 +0200 Subject: [PATCH 4/6] Exit installation when detecting changes failed due to non-zero exit code in dry-run --- packages/safe-chain/src/main.js | 1 + .../npm/dependencyScanner/dryRunScanner.js | 21 ++- .../dependencyScanner/dryRunScanner.spec.js | 139 ++++++++++++++++++ .../src/packagemanager/npm/runNpmCommand.js | 2 +- packages/safe-chain/src/scanning/index.js | 16 +- 5 files changed, 169 insertions(+), 10 deletions(-) create mode 100644 packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.spec.js diff --git a/packages/safe-chain/src/main.js b/packages/safe-chain/src/main.js index 501dd95..81b1b3a 100644 --- a/packages/safe-chain/src/main.js +++ b/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); diff --git a/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js b/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js index cd7f251..79baac0 100644 --- a/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js +++ b/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js @@ -37,10 +37,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 && !doesCommandReturnNonZero(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."); - return []; + throw new Error( + `Dry-run command failed with exit code ${dryRunOutput.status} and produced no output.` + ); } const parsedOutput = parseDryRunOutput(dryRunOutput.output); @@ -48,3 +55,11 @@ function checkChangesWithDryRun(args) { // reverse the array to have the top-level packages first return parsedOutput.reverse(); } + +function doesCommandReturnNonZero(args) { + if (args.length < 2) { + return false; + } + + return args[0] === "audit" && args[1] === "fix"; +} diff --git a/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.spec.js b/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.spec.js new file mode 100644 index 0000000..4fb6272 --- /dev/null +++ b/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); + }); + }); +}); diff --git a/packages/safe-chain/src/packagemanager/npm/runNpmCommand.js b/packages/safe-chain/src/packagemanager/npm/runNpmCommand.js index 97390da..70a0d17 100644 --- a/packages/safe-chain/src/packagemanager/npm/runNpmCommand.js +++ b/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) { diff --git a/packages/safe-chain/src/scanning/index.js b/packages/safe-chain/src/scanning/index.js index 2905bd6..48a3e3a 100644 --- a/packages/safe-chain/src/scanning/index.js +++ b/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}`); From 5a5afc1810cbb60e814fe019b41bb09ab11aa2c1 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Fri, 19 Sep 2025 08:55:34 +0200 Subject: [PATCH 5/6] Fix liniting error --- .../src/packagemanager/npm/dependencyScanner/dryRunScanner.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js b/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js index 79baac0..8b16872 100644 --- a/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js +++ b/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"; From ea7ee5c6b9ddfe6f4d649dc588e0ffd1df151b56 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Fri, 19 Sep 2025 13:13:28 +0200 Subject: [PATCH 6/6] Clarify doesCommandReturnNonZero function with a comment. --- .../packagemanager/npm/dependencyScanner/dryRunScanner.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js b/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js index 8b16872..0db23cb 100644 --- a/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js +++ b/packages/safe-chain/src/packagemanager/npm/dependencyScanner/dryRunScanner.js @@ -37,7 +37,7 @@ 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 vulnerabilities that can be fixed. - if (dryRunOutput.status !== 0 && !doesCommandReturnNonZero(args)) { + if (dryRunOutput.status !== 0 && !canCommandReturnNonZeroOnSuccess(args)) { throw new Error( `Dry-run command failed with exit code ${dryRunOutput.status} and output:\n${dryRunOutput.output}` ); @@ -55,10 +55,12 @@ function checkChangesWithDryRun(args) { return parsedOutput.reverse(); } -function doesCommandReturnNonZero(args) { +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"; }