From 486a4b8f680f60100cebd1dcd0aa750e3924229b Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Mon, 6 Oct 2025 16:25:12 +0200 Subject: [PATCH 01/11] Escape special chars in shell scripts --- packages/safe-chain/src/utils/safeSpawn.js | 12 +++++++++--- packages/safe-chain/src/utils/safeSpawn.spec.js | 9 +++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js index 826ab7d..f45e4ff 100644 --- a/packages/safe-chain/src/utils/safeSpawn.js +++ b/packages/safe-chain/src/utils/safeSpawn.js @@ -1,9 +1,15 @@ import { spawnSync, spawn } from "child_process"; function escapeArg(arg) { - // If argument contains spaces or quotes, wrap in double quotes and escape double quotes - if (arg.includes(" ") || arg.includes('"') || arg.includes("'")) { - return '"' + arg.replaceAll('"', '\\"') + '"'; + // Shell metacharacters that need escaping + // These characters have special meaning in shells and need to be quoted + const shellMetaChars = /[ "&'|;<>()$`\\!*?[\]{}~#]/; + + // If argument contains shell metacharacters, wrap in double quotes + // and escape characters that are special even inside double quotes + if (shellMetaChars.test(arg)) { + // Inside double quotes, we need to escape: " $ ` \ + return '"' + arg.replace(/(["`$\\])/g, '\\$1') + '"'; } return arg; } diff --git a/packages/safe-chain/src/utils/safeSpawn.spec.js b/packages/safe-chain/src/utils/safeSpawn.spec.js index d325f8a..020b59a 100644 --- a/packages/safe-chain/src/utils/safeSpawn.spec.js +++ b/packages/safe-chain/src/utils/safeSpawn.spec.js @@ -105,5 +105,14 @@ describe("safeSpawn", () => { assert.strictEqual(spawnCalls[0].command, "npm install axios --save"); assert.strictEqual(spawnCalls[0].options.shell, true); }); + + it(`should escape ampersand character (${variant})`, async () => { + await runSafeSpawn(variant, "npx", ["cypress", "run", "--env", "password=foo&bar"]); + + assert.strictEqual(spawnCalls.length, 1); + // & should be escaped by wrapping the arg in quotes + assert.strictEqual(spawnCalls[0].command, 'npx cypress run --env "password=foo&bar"'); + assert.strictEqual(spawnCalls[0].options.shell, true); + }); } }); \ No newline at end of file From 7e72ae7d3d281a248afd5d926b9f2afaa41dd2e9 Mon Sep 17 00:00:00 2001 From: Hans Ott Date: Thu, 9 Oct 2025 16:33:40 +0200 Subject: [PATCH 02/11] On Unix/macOS, pass args to `spawn` to avoid escaping issues --- packages/safe-chain/src/utils/safeSpawn.js | 33 ++++++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js index f45e4ff..410f455 100644 --- a/packages/safe-chain/src/utils/safeSpawn.js +++ b/packages/safe-chain/src/utils/safeSpawn.js @@ -1,4 +1,4 @@ -import { spawnSync, spawn } from "child_process"; +import { spawn, execSync } from "child_process"; function escapeArg(arg) { // Shell metacharacters that need escaping @@ -16,18 +16,39 @@ function escapeArg(arg) { function buildCommand(command, args) { const escapedArgs = args.map(escapeArg); + return `${command} ${escapedArgs.join(" ")}`; } -export function safeSpawnSync(command, args, options = {}) { - const fullCommand = buildCommand(command, args); - return spawnSync(fullCommand, { ...options, shell: true }); +function resolveCommandPath(command) { + // command will be "npm", "yarn", etc. + // Use 'command -v' to find the full path + const fullPath = execSync(`command -v ${command}`, { + encoding: "utf8", + shell: true, + }).trim(); + + if (!fullPath) { + throw new Error(`Command not found: ${command}`); + } + + return fullPath; } export async function safeSpawn(command, args, options = {}) { - const fullCommand = buildCommand(command, args); return new Promise((resolve, reject) => { - const child = spawn(fullCommand, { ...options, shell: true }); + // Windows requires shell: true because .bat and .cmd files are not executable + // without a terminal. On Unix/macOS, we resolve the full path first, then use + // array args (safer, no escaping needed). + // See: https://nodejs.org/api/child_process.html#child_processspawncommand-args-options + let child; + if (process.platform === "win32") { + const fullCommand = buildCommand(command, args); + child = spawn(fullCommand, { ...options, shell: true }); + } else { + const fullPath = resolveCommandPath(command); + child = spawn(fullPath, args, options); + } child.on("close", (code) => { resolve({ From c74c23b0ffada047507409d4cd39535b30d8f040 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Thu, 23 Oct 2025 10:52:03 +0200 Subject: [PATCH 03/11] Fix unit tests --- packages/safe-chain/src/utils/safeSpawn.js | 3 ++- packages/safe-chain/src/utils/safeSpawn.spec.js | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js index 32669b3..96c0603 100644 --- a/packages/safe-chain/src/utils/safeSpawn.js +++ b/packages/safe-chain/src/utils/safeSpawn.js @@ -1,4 +1,5 @@ import { spawn, execSync } from "child_process"; +import os from "os"; function escapeArg(arg) { // Shell metacharacters that need escaping @@ -42,7 +43,7 @@ export async function safeSpawn(command, args, options = {}) { // array args (safer, no escaping needed). // See: https://nodejs.org/api/child_process.html#child_processspawncommand-args-options let child; - if (process.platform === "win32") { + if (os.platform() === "win32") { const fullCommand = buildCommand(command, args); child = spawn(fullCommand, { ...options, shell: true }); } else { diff --git a/packages/safe-chain/src/utils/safeSpawn.spec.js b/packages/safe-chain/src/utils/safeSpawn.spec.js index 6d8dd26..4ad005e 100644 --- a/packages/safe-chain/src/utils/safeSpawn.spec.js +++ b/packages/safe-chain/src/utils/safeSpawn.spec.js @@ -33,6 +33,12 @@ describe("safeSpawn", () => { }, }); + mock.module("os", { + namedExports: { + platform: () => "win32", + }, + }); + // Import after mocking const safeSpawnModule = await import("./safeSpawn.js"); safeSpawn = safeSpawnModule.safeSpawn; From 08c1328b521cb5fb0872e8107be62932e736c2e2 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Thu, 23 Oct 2025 13:23:08 +0200 Subject: [PATCH 04/11] Cleanup code, add some tests --- packages/safe-chain/src/utils/safeSpawn.js | 27 ++++--- .../safe-chain/src/utils/safeSpawn.spec.js | 72 +++++++++++++++++++ 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js index 96c0603..c85b91e 100644 --- a/packages/safe-chain/src/utils/safeSpawn.js +++ b/packages/safe-chain/src/utils/safeSpawn.js @@ -1,22 +1,33 @@ import { spawn, execSync } from "child_process"; import os from "os"; -function escapeArg(arg) { - // Shell metacharacters that need escaping - // These characters have special meaning in shells and need to be quoted - const shellMetaChars = /[ "&'|;<>()$`\\!*?[\]{}~#]/; - +function sanitizeShellArgument(arg) { // If argument contains shell metacharacters, wrap in double quotes // and escape characters that are special even inside double quotes - if (shellMetaChars.test(arg)) { + if (hasShellMetaChars(arg)) { // Inside double quotes, we need to escape: " $ ` \ - return '"' + arg.replace(/(["`$\\])/g, "\\$1") + '"'; + return '"' + escapeDoubleQuoteContent(arg) + '"'; } return arg; } +function hasShellMetaChars(arg) { + // Shell metacharacters that need escaping + // These characters have special meaning in shells and need to be quoted + // Whenever one of these characters is present, we should quote the argument + // Characters: space, ", &, ', |, ;, <, >, (, ), $, `, \, !, *, ?, [, ], {, }, ~, # + const shellMetaChars = /[ "&'|;<>()$`\\!*?[\]{}~#]/; + return shellMetaChars.test(arg); +} + +function escapeDoubleQuoteContent(arg) { + // Escape special characters for shell safety + // This escapes ", $, `, and \ by prefixing them with a backslash + return arg.replace(/(["`$\\])/g, "\\$1"); +} + function buildCommand(command, args) { - const escapedArgs = args.map(escapeArg); + const escapedArgs = args.map(sanitizeShellArgument); return `${command} ${escapedArgs.join(" ")}`; } diff --git a/packages/safe-chain/src/utils/safeSpawn.spec.js b/packages/safe-chain/src/utils/safeSpawn.spec.js index 4ad005e..cf7bd41 100644 --- a/packages/safe-chain/src/utils/safeSpawn.spec.js +++ b/packages/safe-chain/src/utils/safeSpawn.spec.js @@ -112,4 +112,76 @@ describe("safeSpawn", () => { ); assert.strictEqual(spawnCalls[0].options.shell, true); }); + + it("should escape dollar signs to prevent variable expansion", async () => { + await safeSpawn("echo", ["$HOME/test"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "\\$HOME/test"'); + }); + + it("should escape backticks to prevent command substitution", async () => { + await safeSpawn("echo", ["file`whoami`.txt"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "file\\`whoami\\`.txt"'); + }); + + it("should escape backslashes properly", async () => { + await safeSpawn("echo", ["path\\with\\backslash"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual( + spawnCalls[0].command, + 'echo "path\\\\with\\\\backslash"' + ); + }); + + it("should handle multiple special characters in one argument", async () => { + await safeSpawn("cmd", ['test "quoted" $var `cmd` & more']); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual( + spawnCalls[0].command, + 'cmd "test \\"quoted\\" \\$var \\`cmd\\` & more"' + ); + }); + + it("should handle pipe character", async () => { + await safeSpawn("echo", ["foo|bar"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "foo|bar"'); + }); + + it("should handle parentheses", async () => { + await safeSpawn("echo", ["(test)"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "(test)"'); + }); + + it("should handle angle brackets for redirection", async () => { + await safeSpawn("echo", ["foo>output.txt"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "foo>output.txt"'); + }); + + it("should handle wildcard characters", async () => { + await safeSpawn("echo", ["*.txt"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, 'echo "*.txt"'); + }); + + it("should handle multiple arguments with mixed escaping needs", async () => { + await safeSpawn("cmd", ["safe", "needs space", "$dangerous", "also-safe"]); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual( + spawnCalls[0].command, + 'cmd safe "needs space" "\\$dangerous" also-safe' + ); + }); }); From 7a55be49f4a35a9fca262c82e588996ebf4b46a0 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Thu, 23 Oct 2025 13:29:14 +0200 Subject: [PATCH 05/11] Fix linting error --- packages/safe-chain/src/utils/safeSpawn.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/safe-chain/src/utils/safeSpawn.spec.js b/packages/safe-chain/src/utils/safeSpawn.spec.js index cf7bd41..cbdb7cb 100644 --- a/packages/safe-chain/src/utils/safeSpawn.spec.js +++ b/packages/safe-chain/src/utils/safeSpawn.spec.js @@ -22,7 +22,7 @@ describe("safeSpawn", () => { }, }; }, - execSync: (cmd, opts) => { + execSync: (cmd) => { // Simulate 'command -v' returning full path const match = cmd.match(/command -v (.+)/); if (match) { From 9a78cafbfdd1b71dd7ffac0319e84f359602cd3c Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Thu, 23 Oct 2025 17:45:03 +0200 Subject: [PATCH 06/11] Introduce silent mode to disable logging --- README.md | 12 ++++ .../safe-chain/src/config/cliArguments.js | 17 +++++ .../src/config/cliArguments.spec.js | 69 ++++++++++++++++++- packages/safe-chain/src/config/settings.js | 13 ++++ .../src/environment/userInteraction.js | 31 ++++++++- .../src/registryProxy/registryProxy.js | 2 +- packages/safe-chain/src/scanning/index.js | 2 +- 7 files changed, 142 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 1083c0e..7b3f038 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,18 @@ Example usage: npm install suspicious-package --safe-chain-malware-action=prompt ``` +## Logging + +You can control the output from Aikido Safe Chain using the `--safe-chain-logging` flag: + +- `--safe-chain-logging=silent` - Suppresses all Aikido Safe Chain output except when malware is blocked. The package manager output is written to stdout as normal, and Safe Chain only writes a short message if it has blocked malware and causes the process to exit. + +Example usage: + +```shell +npm install express --safe-chain-logging=silent +``` + # Usage in CI/CD You can protect your CI/CD pipelines from malicious packages by integrating Aikido Safe Chain into your build process. This ensures that any packages installed during your automated builds are checked for malware before installation. diff --git a/packages/safe-chain/src/config/cliArguments.js b/packages/safe-chain/src/config/cliArguments.js index 87abb7b..70c56b1 100644 --- a/packages/safe-chain/src/config/cliArguments.js +++ b/packages/safe-chain/src/config/cliArguments.js @@ -1,5 +1,6 @@ const state = { malwareAction: undefined, + loggingLevel: undefined, }; const SAFE_CHAIN_ARG_PREFIX = "--safe-chain-"; @@ -7,6 +8,7 @@ const SAFE_CHAIN_ARG_PREFIX = "--safe-chain-"; export function initializeCliArguments(args) { // Reset state on each call state.malwareAction = undefined; + state.loggingLevel = undefined; const safeChainArgs = []; const remainingArgs = []; @@ -20,6 +22,7 @@ export function initializeCliArguments(args) { } setMalwareAction(safeChainArgs); + setLoggingLevel(safeChainArgs); return remainingArgs; } @@ -48,3 +51,17 @@ function getLastArgEqualsValue(args, prefix) { export function getMalwareAction() { return state.malwareAction; } + +function setLoggingLevel(args) { + const safeChainLoggingArg = SAFE_CHAIN_ARG_PREFIX + "logging="; + + const level = getLastArgEqualsValue(args, safeChainLoggingArg); + if (!level) { + return; + } + state.loggingLevel = level.toLowerCase(); +} + +export function getLoggingLevel() { + return state.loggingLevel; +} diff --git a/packages/safe-chain/src/config/cliArguments.spec.js b/packages/safe-chain/src/config/cliArguments.spec.js index 9d5c0ba..c7d9a84 100644 --- a/packages/safe-chain/src/config/cliArguments.spec.js +++ b/packages/safe-chain/src/config/cliArguments.spec.js @@ -1,6 +1,10 @@ import { describe, it } from "node:test"; import assert from "node:assert"; -import { initializeCliArguments, getMalwareAction } from "./cliArguments.js"; +import { + initializeCliArguments, + getMalwareAction, + getLoggingLevel, +} from "./cliArguments.js"; describe("initializeCliArguments", () => { it("should return all args when no safe-chain args are present", () => { @@ -105,4 +109,67 @@ describe("initializeCliArguments", () => { assert.deepEqual(result, ["install"]); assert.strictEqual(getMalwareAction(), "block"); }); + + it("should not set loggingLevel when no logging argument is passed", () => { + const args = ["install", "express", "--save"]; + initializeCliArguments(args); + + assert.strictEqual(getLoggingLevel(), undefined); + }); + + it("should parse logging=silent and set state", () => { + const args = ["--safe-chain-logging=silent", "install", "package"]; + const result = initializeCliArguments(args); + + assert.deepEqual(result, ["install", "package"]); + assert.strictEqual(getLoggingLevel(), "silent"); + }); + + it("should parse logging=normal and set state", () => { + const args = ["--safe-chain-logging=normal", "install", "package"]; + const result = initializeCliArguments(args); + + assert.deepEqual(result, ["install", "package"]); + assert.strictEqual(getLoggingLevel(), "normal"); + }); + + it("should handle multiple logging args, using the last one", () => { + const args = [ + "--safe-chain-logging=normal", + "--safe-chain-logging=silent", + "install", + ]; + const result = initializeCliArguments(args); + + assert.deepEqual(result, ["install"]); + assert.strictEqual(getLoggingLevel(), "silent"); + }); + + it("should handle logging level case-insensitively", () => { + const args = ["--safe-chain-logging=SILENT", "install"]; + initializeCliArguments(args); + + assert.strictEqual(getLoggingLevel(), "silent"); + }); + + it("should capture invalid logging level as-is (lowercased)", () => { + const args = ["--safe-chain-logging=invalid", "install"]; + initializeCliArguments(args); + + assert.strictEqual(getLoggingLevel(), "invalid"); + }); + + it("should handle logging with other safe-chain args", () => { + const args = [ + "--safe-chain-debug", + "--safe-chain-logging=silent", + "--safe-chain-malware-action=block", + "install", + ]; + const result = initializeCliArguments(args); + + assert.deepEqual(result, ["install"]); + assert.strictEqual(getLoggingLevel(), "silent"); + assert.strictEqual(getMalwareAction(), "block"); + }); }); diff --git a/packages/safe-chain/src/config/settings.js b/packages/safe-chain/src/config/settings.js index ed2cae2..17c1cdb 100644 --- a/packages/safe-chain/src/config/settings.js +++ b/packages/safe-chain/src/config/settings.js @@ -10,5 +10,18 @@ export function getMalwareAction() { return MALWARE_ACTION_BLOCK; } +export function getLoggingLevel() { + const level = cliArguments.getLoggingLevel(); + + if (level === LOGGING_SILENT) { + return LOGGING_SILENT; + } + + return LOGGING_NORMAL; +} + export const MALWARE_ACTION_BLOCK = "block"; export const MALWARE_ACTION_PROMPT = "prompt"; + +export const LOGGING_SILENT = "silent"; +export const LOGGING_NORMAL = "normal"; diff --git a/packages/safe-chain/src/environment/userInteraction.js b/packages/safe-chain/src/environment/userInteraction.js index 829afa1..99fe90f 100644 --- a/packages/safe-chain/src/environment/userInteraction.js +++ b/packages/safe-chain/src/environment/userInteraction.js @@ -3,16 +3,27 @@ import chalk from "chalk"; import ora from "ora"; import { createInterface } from "readline"; import { isCi } from "./environment.js"; +import { getLoggingLevel, LOGGING_SILENT } from "../config/settings.js"; + +function isSilentMode() { + return getLoggingLevel() === LOGGING_SILENT; +} function emptyLine() { + if (isSilentMode()) return; + writeInformation(""); } function writeInformation(message, ...optionalParams) { + if (isSilentMode()) return; + console.log(message, ...optionalParams); } function writeWarning(message, ...optionalParams) { + if (isSilentMode()) return; + if (!isCi()) { message = chalk.yellow(message); } @@ -26,7 +37,24 @@ function writeError(message, ...optionalParams) { console.error(message, ...optionalParams); } +function writeExitWithoutInstallingMaliciousPackages() { + let message = "Safe-chain: Exiting without installing malicious packages."; + if (!isCi()) { + message = chalk.red(message); + } + console.error(message); +} + function startProcess(message) { + if (isSilentMode()) { + return { + succeed: () => {}, + fail: () => {}, + stop: () => {}, + setText: () => {}, + }; + } + if (isCi()) { return { succeed: (message) => { @@ -60,7 +88,7 @@ function startProcess(message) { } async function confirm(config) { - if (isCi()) { + if (isCi() || isSilentMode()) { return Promise.resolve(config.default); } @@ -91,6 +119,7 @@ export const ui = { writeInformation, writeWarning, writeError, + writeExitWithoutInstallingMaliciousPackages, emptyLine, startProcess, confirm, diff --git a/packages/safe-chain/src/registryProxy/registryProxy.js b/packages/safe-chain/src/registryProxy/registryProxy.js index b0e8dd1..887fd47 100644 --- a/packages/safe-chain/src/registryProxy/registryProxy.js +++ b/packages/safe-chain/src/registryProxy/registryProxy.js @@ -153,7 +153,7 @@ function verifyNoMaliciousPackages() { } ui.emptyLine(); - ui.writeError("Exiting without installing malicious packages."); + ui.writeExitWithoutInstallingMaliciousPackages(); ui.emptyLine(); return false; diff --git a/packages/safe-chain/src/scanning/index.js b/packages/safe-chain/src/scanning/index.js index 36f62ca..4b0f654 100644 --- a/packages/safe-chain/src/scanning/index.js +++ b/packages/safe-chain/src/scanning/index.js @@ -93,7 +93,7 @@ async function onMalwareFound() { } } - ui.writeError("Exiting without installing malicious packages."); + ui.writeExitWithoutInstallingMaliciousPackages(); ui.emptyLine(); return 1; } From 0f164d055ff16c9faeb79d661379cb797e622bf7 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Thu, 23 Oct 2025 17:48:26 +0200 Subject: [PATCH 07/11] Fix mocking in tests --- packages/safe-chain/src/scanning/index.scanCommand.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/safe-chain/src/scanning/index.scanCommand.spec.js b/packages/safe-chain/src/scanning/index.scanCommand.spec.js index 1858d10..abcdc97 100644 --- a/packages/safe-chain/src/scanning/index.scanCommand.spec.js +++ b/packages/safe-chain/src/scanning/index.scanCommand.spec.js @@ -46,6 +46,7 @@ describe("scanCommand", async () => { writeError: () => {}, writeInformation: () => {}, writeWarning: () => {}, + writeExitWithoutInstallingMaliciousPackages: () => {}, emptyLine: () => {}, confirm: mockConfirm, }, From f5f3b91b40dc1bad0050ef1d4339672ecd2cfc5f Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Fri, 24 Oct 2025 17:36:51 +0200 Subject: [PATCH 08/11] Test if command is safe to execute --- packages/safe-chain/src/utils/safeSpawn.js | 9 ++++++ .../safe-chain/src/utils/safeSpawn.spec.js | 29 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js index c85b91e..8642b07 100644 --- a/packages/safe-chain/src/utils/safeSpawn.js +++ b/packages/safe-chain/src/utils/safeSpawn.js @@ -27,6 +27,10 @@ function escapeDoubleQuoteContent(arg) { } function buildCommand(command, args) { + if (args.length === 0) { + return command; + } + const escapedArgs = args.map(sanitizeShellArgument); return `${command} ${escapedArgs.join(" ")}`; @@ -48,6 +52,11 @@ function resolveCommandPath(command) { } export async function safeSpawn(command, args, options = {}) { + // command should always be alphanumeric or _ or - to avoid injection + if (!/^[a-zA-Z0-9_-]+$/.test(command)) { + throw new Error(`Invalid command name: ${command}`); + } + return new Promise((resolve, reject) => { // Windows requires shell: true because .bat and .cmd files are not executable // without a terminal. On Unix/macOS, we resolve the full path first, then use diff --git a/packages/safe-chain/src/utils/safeSpawn.spec.js b/packages/safe-chain/src/utils/safeSpawn.spec.js index cbdb7cb..d4180b7 100644 --- a/packages/safe-chain/src/utils/safeSpawn.spec.js +++ b/packages/safe-chain/src/utils/safeSpawn.spec.js @@ -4,9 +4,11 @@ import assert from "node:assert"; describe("safeSpawn", () => { let safeSpawn; let spawnCalls = []; + let os; beforeEach(async () => { spawnCalls = []; + os = "win32"; // Test Windows behavior by default // Mock child_process module to capture what command string gets built mock.module("child_process", { @@ -35,7 +37,7 @@ describe("safeSpawn", () => { mock.module("os", { namedExports: { - platform: () => "win32", + platform: () => os, }, }); @@ -184,4 +186,29 @@ describe("safeSpawn", () => { 'cmd safe "needs space" "\\$dangerous" also-safe' ); }); + + it("should reject command names with special characters", async () => { + await assert.rejects(async () => await safeSpawn("npm; echo hacked", []), { + message: "Invalid command name: npm; echo hacked", + }); + }); + + it("should reject command names with spaces", async () => { + await assert.rejects(async () => await safeSpawn("npm install", []), { + message: "Invalid command name: npm install", + }); + }); + + it("should reject command names with slashes", async () => { + await assert.rejects(async () => await safeSpawn("../../malicious", []), { + message: "Invalid command name: ../../malicious", + }); + }); + + it("should accept valid command names with letters, numbers, underscores and hyphens", async () => { + await safeSpawn("valid_command-123", []); + + assert.strictEqual(spawnCalls.length, 1); + assert.strictEqual(spawnCalls[0].command, "valid_command-123"); + }); }); From 0029a7e1c1fa8f9a16ce3ef3d913c46cea360c63 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Mon, 27 Oct 2025 10:49:26 +0100 Subject: [PATCH 09/11] Add extra comments for regex clarification --- packages/safe-chain/src/utils/safeSpawn.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js index 8642b07..c398ac2 100644 --- a/packages/safe-chain/src/utils/safeSpawn.js +++ b/packages/safe-chain/src/utils/safeSpawn.js @@ -52,7 +52,9 @@ function resolveCommandPath(command) { } export async function safeSpawn(command, args, options = {}) { - // command should always be alphanumeric or _ or - to avoid injection + // The command is always one of our supported package managers. + // It should always be alphanumeric or _ or - + // Reject any command names with suspicious characters if (!/^[a-zA-Z0-9_-]+$/.test(command)) { throw new Error(`Invalid command name: ${command}`); } From ab3319a3104dfe5c1ce015362986501f63fa95ac Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Mon, 27 Oct 2025 11:51:19 +0100 Subject: [PATCH 10/11] Remove --safe-chain-malware-action flag --- .../safe-chain/src/config/cliArguments.js | 17 ------ .../src/config/cliArguments.spec.js | 56 +----------------- packages/safe-chain/src/config/settings.js | 13 ---- .../src/environment/userInteraction.js | 30 ---------- packages/safe-chain/src/scanning/index.js | 21 +------ .../src/scanning/index.scanCommand.spec.js | 59 ++----------------- 6 files changed, 8 insertions(+), 188 deletions(-) diff --git a/packages/safe-chain/src/config/cliArguments.js b/packages/safe-chain/src/config/cliArguments.js index 70c56b1..f234bbb 100644 --- a/packages/safe-chain/src/config/cliArguments.js +++ b/packages/safe-chain/src/config/cliArguments.js @@ -1,5 +1,4 @@ const state = { - malwareAction: undefined, loggingLevel: undefined, }; @@ -7,7 +6,6 @@ const SAFE_CHAIN_ARG_PREFIX = "--safe-chain-"; export function initializeCliArguments(args) { // Reset state on each call - state.malwareAction = undefined; state.loggingLevel = undefined; const safeChainArgs = []; @@ -21,22 +19,11 @@ export function initializeCliArguments(args) { } } - setMalwareAction(safeChainArgs); setLoggingLevel(safeChainArgs); return remainingArgs; } -function setMalwareAction(args) { - const safeChainMalwareActionArg = SAFE_CHAIN_ARG_PREFIX + "malware-action="; - - const action = getLastArgEqualsValue(args, safeChainMalwareActionArg); - if (!action) { - return; - } - state.malwareAction = action.toLowerCase(); -} - function getLastArgEqualsValue(args, prefix) { for (var i = args.length - 1; i >= 0; i--) { const arg = args[i]; @@ -48,10 +35,6 @@ function getLastArgEqualsValue(args, prefix) { return undefined; } -export function getMalwareAction() { - return state.malwareAction; -} - function setLoggingLevel(args) { const safeChainLoggingArg = SAFE_CHAIN_ARG_PREFIX + "logging="; diff --git a/packages/safe-chain/src/config/cliArguments.spec.js b/packages/safe-chain/src/config/cliArguments.spec.js index c7d9a84..415d34a 100644 --- a/packages/safe-chain/src/config/cliArguments.spec.js +++ b/packages/safe-chain/src/config/cliArguments.spec.js @@ -1,10 +1,6 @@ import { describe, it } from "node:test"; import assert from "node:assert"; -import { - initializeCliArguments, - getMalwareAction, - getLoggingLevel, -} from "./cliArguments.js"; +import { initializeCliArguments, getLoggingLevel } from "./cliArguments.js"; describe("initializeCliArguments", () => { it("should return all args when no safe-chain args are present", () => { @@ -61,55 +57,6 @@ describe("initializeCliArguments", () => { assert.deepEqual(result, ["install", "my--safe-chain-package", "--save"]); }); - it("should not set malwareAction when no safe-chain arguments are passed", () => { - const args = ["install", "express", "--save"]; - const result = initializeCliArguments(args); - - assert.deepEqual(result, ["install", "express", "--save"]); - assert.strictEqual(getMalwareAction(), undefined); - }); - - it("should parse malware-action=block and set state", () => { - const args = ["--safe-chain-malware-action=block", "install", "package"]; - const result = initializeCliArguments(args); - - assert.deepEqual(result, ["install", "package"]); - assert.strictEqual(getMalwareAction(), "block"); - }); - - it("should parse malware-action=prompt and set state", () => { - const args = ["--safe-chain-malware-action=prompt", "install", "package"]; - const result = initializeCliArguments(args); - - assert.deepEqual(result, ["install", "package"]); - assert.strictEqual(getMalwareAction(), "prompt"); - }); - - it("should handle multiple malware-action args, using the last valid one", () => { - const args = [ - "--safe-chain-malware-action=block", - "--safe-chain-malware-action=prompt", - "install", - ]; - const result = initializeCliArguments(args); - - assert.deepEqual(result, ["install"]); - assert.strictEqual(getMalwareAction(), "prompt"); - }); - - it("should handle malware-action with other safe-chain args", () => { - const args = [ - "--safe-chain-debug", - "--safe-chain-malware-action=block", - "--safe-chain-verbose", - "install", - ]; - const result = initializeCliArguments(args); - - assert.deepEqual(result, ["install"]); - assert.strictEqual(getMalwareAction(), "block"); - }); - it("should not set loggingLevel when no logging argument is passed", () => { const args = ["install", "express", "--save"]; initializeCliArguments(args); @@ -170,6 +117,5 @@ describe("initializeCliArguments", () => { assert.deepEqual(result, ["install"]); assert.strictEqual(getLoggingLevel(), "silent"); - assert.strictEqual(getMalwareAction(), "block"); }); }); diff --git a/packages/safe-chain/src/config/settings.js b/packages/safe-chain/src/config/settings.js index 17c1cdb..ad150b2 100644 --- a/packages/safe-chain/src/config/settings.js +++ b/packages/safe-chain/src/config/settings.js @@ -1,15 +1,5 @@ import * as cliArguments from "./cliArguments.js"; -export function getMalwareAction() { - const action = cliArguments.getMalwareAction(); - - if (action === MALWARE_ACTION_PROMPT) { - return MALWARE_ACTION_PROMPT; - } - - return MALWARE_ACTION_BLOCK; -} - export function getLoggingLevel() { const level = cliArguments.getLoggingLevel(); @@ -20,8 +10,5 @@ export function getLoggingLevel() { return LOGGING_NORMAL; } -export const MALWARE_ACTION_BLOCK = "block"; -export const MALWARE_ACTION_PROMPT = "prompt"; - export const LOGGING_SILENT = "silent"; export const LOGGING_NORMAL = "normal"; diff --git a/packages/safe-chain/src/environment/userInteraction.js b/packages/safe-chain/src/environment/userInteraction.js index 99fe90f..e1a4f93 100644 --- a/packages/safe-chain/src/environment/userInteraction.js +++ b/packages/safe-chain/src/environment/userInteraction.js @@ -1,7 +1,6 @@ // oxlint-disable no-console import chalk from "chalk"; import ora from "ora"; -import { createInterface } from "readline"; import { isCi } from "./environment.js"; import { getLoggingLevel, LOGGING_SILENT } from "../config/settings.js"; @@ -87,34 +86,6 @@ function startProcess(message) { } } -async function confirm(config) { - if (isCi() || isSilentMode()) { - return Promise.resolve(config.default); - } - - const rl = createInterface({ - input: process.stdin, - output: process.stdout, - }); - - return new Promise((resolve) => { - const defaultText = config.default ? " (Y/n)" : " (y/N)"; - rl.question(`${config.message}${defaultText} `, (answer) => { - rl.close(); - - const normalizedAnswer = answer.trim().toLowerCase(); - - if (normalizedAnswer === "y" || normalizedAnswer === "yes") { - resolve(true); - } else if (normalizedAnswer === "n" || normalizedAnswer === "no") { - resolve(false); - } else { - resolve(config.default); - } - }); - }); -} - export const ui = { writeInformation, writeWarning, @@ -122,5 +93,4 @@ export const ui = { writeExitWithoutInstallingMaliciousPackages, emptyLine, startProcess, - confirm, }; diff --git a/packages/safe-chain/src/scanning/index.js b/packages/safe-chain/src/scanning/index.js index 4b0f654..969c994 100644 --- a/packages/safe-chain/src/scanning/index.js +++ b/packages/safe-chain/src/scanning/index.js @@ -4,7 +4,6 @@ import { setTimeout } from "timers/promises"; import chalk from "chalk"; import { getPackageManager } from "../packagemanager/currentPackageManager.js"; import { ui } from "../environment/userInteraction.js"; -import { getMalwareAction, MALWARE_ACTION_PROMPT } from "../config/settings.js"; export function shouldScanCommand(args) { if (!args || args.length === 0) { @@ -65,7 +64,8 @@ export async function scanCommand(args) { return 0; } else { printMaliciousChanges(audit.disallowedChanges, spinner); - return await onMalwareFound(); + onMalwareFound(); + return 1; } } @@ -77,23 +77,8 @@ function printMaliciousChanges(changes, spinner) { } } -async function onMalwareFound() { +function onMalwareFound() { ui.emptyLine(); - - if (getMalwareAction() === MALWARE_ACTION_PROMPT) { - const continueInstall = await ui.confirm({ - message: - "Malicious packages were found. Do you want to continue with the installation?", - default: false, - }); - - if (continueInstall) { - ui.writeWarning("Continuing with the installation despite the risks..."); - return 0; - } - } - ui.writeExitWithoutInstallingMaliciousPackages(); ui.emptyLine(); - return 1; } diff --git a/packages/safe-chain/src/scanning/index.scanCommand.spec.js b/packages/safe-chain/src/scanning/index.scanCommand.spec.js index abcdc97..c47555f 100644 --- a/packages/safe-chain/src/scanning/index.scanCommand.spec.js +++ b/packages/safe-chain/src/scanning/index.scanCommand.spec.js @@ -1,10 +1,6 @@ import assert from "node:assert/strict"; -import { beforeEach, describe, it, mock } from "node:test"; +import { describe, it, mock } from "node:test"; import { setTimeout } from "node:timers/promises"; -import { - MALWARE_ACTION_PROMPT, - MALWARE_ACTION_BLOCK, -} from "../config/settings.js"; describe("scanCommand", async () => { const getScanTimeoutMock = mock.fn(() => 1000); @@ -15,8 +11,6 @@ describe("scanCommand", async () => { fail: () => {}, stop: () => {}, })); - const mockConfirm = mock.fn(() => true); - let malwareAction = MALWARE_ACTION_PROMPT; // import { getPackageManager } from "../packagemanager/currentPackageManager.js"; mock.module("../packagemanager/currentPackageManager.js", { @@ -48,19 +42,10 @@ describe("scanCommand", async () => { writeWarning: () => {}, writeExitWithoutInstallingMaliciousPackages: () => {}, emptyLine: () => {}, - confirm: mockConfirm, }, }, }); - mock.module("../config/settings.js", { - namedExports: { - getMalwareAction: () => malwareAction, - MALWARE_ACTION_PROMPT, - MALWARE_ACTION_BLOCK, - }, - }); - // import { auditChanges, MAX_LENGTH_EXCEEDED } from "./audit/index.js"; mock.module("./audit/index.js", { namedExports: { @@ -89,11 +74,6 @@ describe("scanCommand", async () => { const { scanCommand } = await import("./index.js"); - beforeEach(() => { - // Reset malware action back to prompt mode for other tests - malwareAction = MALWARE_ACTION_PROMPT; - }); - it("should succeed when there are no changes", async () => { let progressWasStopped = false; mockStartProcess.mock.mockImplementationOnce(() => ({ @@ -151,7 +131,7 @@ describe("scanCommand", async () => { assert.equal(failureMessageWasSet, true); }); - it("should fail and prompt the user when malicious changes are detected", async () => { + it("should fail and return 1 malicious changes are detected", async () => { let failureMessageWasSet = false; mockStartProcess.mock.mockImplementationOnce(() => ({ setText: () => {}, @@ -164,16 +144,11 @@ describe("scanCommand", async () => { mockGetDependencyUpdatesForCommand.mock.mockImplementation(() => [ { name: "malicious", version: "1.0.0" }, ]); - let userWasPrompted = false; - mockConfirm.mock.mockImplementationOnce(() => { - userWasPrompted = true; - return true; // Simulate user accepting the risk, otherwise the process would exit - }); - await scanCommand(["install", "malicious"]); + const result = await scanCommand(["install", "malicious"]); assert.equal(failureMessageWasSet, true); - assert.equal(userWasPrompted, true); + assert.equal(result, 1); }); it("should not report a timeout when the user takes a long time to respond (it should not affect the timeout)", async () => { @@ -190,10 +165,6 @@ describe("scanCommand", async () => { mockGetDependencyUpdatesForCommand.mock.mockImplementation(async () => { return [{ name: "malicious", version: "4.17.21" }]; }); - mockConfirm.mock.mockImplementationOnce(async () => { - await setTimeout(200); - return true; // Simulate user accepting the risk, otherwise the process would exit - }); await scanCommand(["install", "malicious"]); @@ -204,12 +175,6 @@ describe("scanCommand", async () => { }); it("should exit immediately when malicious changes are detected in block mode", async () => { - // Set malware action to block mode for this test - malwareAction = MALWARE_ACTION_BLOCK; - - // Reset mock call count - mockConfirm.mock.resetCalls(); - let failureMessageWasSet = false; mockStartProcess.mock.mockImplementationOnce(() => ({ @@ -229,19 +194,9 @@ describe("scanCommand", async () => { assert.equal(failureMessageWasSet, true); assert.equal(result, 1); - // Confirm should not have been called in block mode - assert.equal(mockConfirm.mock.callCount(), 0); }); it("should exit immediately when malicious changes are detected in block mode without prompting", async () => { - // Set malware action to block mode for this test - malwareAction = MALWARE_ACTION_BLOCK; - - // Reset mock call count - mockConfirm.mock.resetCalls(); - - let userWasPrompted = false; - mockStartProcess.mock.mockImplementationOnce(() => ({ setText: () => {}, succeed: () => {}, @@ -253,14 +208,8 @@ describe("scanCommand", async () => { { name: "malicious", version: "1.0.0" }, ]); - mockConfirm.mock.mockImplementationOnce(() => { - userWasPrompted = true; - return false; - }); - const result = await scanCommand(["install", "malicious"]); assert.equal(result, 1); - assert.equal(userWasPrompted, false); }); }); From ff724154fbfe176593955086e3a2638c734413e9 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Mon, 27 Oct 2025 13:49:29 +0100 Subject: [PATCH 11/11] Remove --safe-chain-malware-action documentation --- README.md | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/README.md b/README.md index 7b3f038..e385ec4 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ Installing the Aikido Safe Chain is easy. You just need 3 simple steps: When running `npm`, `npx`, `yarn`, `pnpm`, `pnpx`, `bun`, or `bunx` commands, the Aikido Safe Chain will automatically check for malware in the packages you are trying to install. If any malware is detected, it will prompt you to exit the command. You can check the installed version by running: + ```shell safe-chain --version ``` @@ -75,19 +76,6 @@ To uninstall the Aikido Safe Chain, you can run the following command: # Configuration -## Malware Action - -You can control how Aikido Safe Chain responds when malware is detected using the `--safe-chain-malware-action` flag: - -- `--safe-chain-malware-action=block` (**default**) - Automatically blocks installation and exits with an error when malware is detected -- `--safe-chain-malware-action=prompt` - Prompts the user to decide whether to continue despite the malware detection - -Example usage: - -```shell -npm install suspicious-package --safe-chain-malware-action=prompt -``` - ## Logging You can control the output from Aikido Safe Chain using the `--safe-chain-logging` flag: