How Command Injection happens in Node.js and how to fix it

Introduction

The scripts/postinstall.js file in this repository runs automatically every time npm install executes, and its job is to prepare native Electron dependencies — rebuilding better-sqlite3 and running electron-builder install-app-deps. To do that, it shells out using Node's child_process module. A semgrep scan (javascript.lang.security.detect-child-process.detect-child-process) flagged line 28 of this file, warning that calls to child_process originating from a function argument named file could become a command injection vector if that input were ever user-controllable.

This matters because postinstall.js isn't a normal application code path — it's a lifecycle script that npm executes with elevated trust, at install time, often without a developer double-checking the output. If a pattern like this is left unguarded, it becomes exactly the kind of "exploit primitive" that automated attack tooling looks for: a working code path that could be chained with a future change (a new CLI flag, a config value, a dependency update) to achieve arbitrary command execution.

The Vulnerability Explained

Here's what the script looked like before the fix:

#!/usr/bin/env node
const { execSync } = require('child_process');
const path = require('path');
const fs = require('fs');

// Install native dependencies for Electron
try {
  execSync('npx electron-builder install-app-deps', { stdio: 'inherit' });
} catch (err) {
  console.error('electron-builder install-app-deps failed:', err.message);
  // Fallback: rebuild only better-sqlite3 for Electron (node-pty uses prebuilds)
  console.log('Attempting fallback: rebuilding better-sqlite3 for Electron...');
  try {
    execSync('npx @electron/rebuild -f -m . -o better-sqlite3', { stdio: 'inherit' });
    console.log('Fallback rebuild succeeded.');
  } catch (err2) {
    console.error('Fallback rebuild also failed:', err2.message);
  }
}

Two structural problems stand out:

1. Everything runs unconditionally at module load time. As soon as this file is require()'d or executed by Node, both execSync calls fire — there is no separation between "this module was imported for testing/reuse" and "this module was invoked as the actual postinstall step." That means any code path that pulls this file in (a test harness, a build tool, a future refactor that imports helper functions from it) automatically triggers a shell command with stdio: 'inherit'.
2. execSync takes a full shell string. execSync('npx electron-builder install-app-deps', ...) is parsed and executed by a shell (/bin/sh or cmd.exe), not passed as a discrete argument list. Shell strings are exactly the pattern semgrep's detect-child-process rule exists to catch, because the moment any part of that string becomes dynamic — say, a version pulled from package.json, a path built from path.join(), or a CLI flag — an attacker who controls that fragment can inject ;, &&, backticks, or $() to run arbitrary commands.

Attack scenario: Imagine a future change adds a --only=<module> style flag to the fallback rebuild command, sourced from an environment variable or a config file (execSync(\npx @electron/rebuild -f -m . -o ${moduleName}`)). IfmoduleNameis ever influenced by anything outside the developer's direct control — a compromised dependency's install script, a maliciouspackage.jsonfield, or a CI variable — that single string concatenation turnspostinstall.jsinto a remote code execution primitive that runs with the full privileges of whoever runsnpm install`. Because postinstall scripts execute automatically and silently, this is precisely the kind of primitive that's valuable to an attacker even before it's "fully exploitable" — it's a foothold waiting for the next weak link.

The Fix

The PR makes three coordinated changes to close this gap:

1. Import the safer execFileSync alongside execSync:

-const { execSync } = require('child_process');
+const { execSync, execFileSync } = require('child_process');

execFileSync executes a binary directly with an argument array — no shell is invoked to parse the string, so shell metacharacters (;, &&, |, backticks) in any argument are treated as literal data, not command syntax. This is the standard Node.js mitigation for the exact pattern semgrep flagged.

2. Guard all side effects behind a require.main === module check:

-// Install native dependencies for Electron
-try {
-  execSync('npx electron-builder install-app-deps', { stdio: 'inherit' });
-} catch (err) {
-  ...
+if (require.main === module) {
+  // Install native dependencies for Electron
+  try {
+    execSync('npx electron-builder install-app-deps', { stdio: 'inherit' });
+  } catch (err) {
+    ...
+  }
+}

Now the execSync calls only fire when postinstall.js is executed directly by npm's lifecycle hook — not merely by being imported. This removes the "load-time side effect" hazard and, just as importantly, makes the file safely testable: helper logic can be exercised in isolation without triggering real shell commands.

3. Add regression tests and wire them into CI:

+      - name: Test postinstall helpers
+        run: node --test test/postinstall.test.js

A dedicated test/postinstall.test.js file, run via node --test on every build, now locks in the hardened behavior so a future contributor can't silently reintroduce an unguarded shell call without a test failure surfacing it.

Together, these changes don't remove the need to shell out (the script legitimately needs to invoke electron-builder and @electron/rebuild), but they eliminate the unconditional execution and give the codebase a safer primitive (execFileSync) to build on if these commands ever need dynamic arguments.

Prevention & Best Practices

• Never build shell command strings from variables. If a command needs dynamic arguments, use execFileSync(cmd, [arg1, arg2]) or spawn(cmd, args) instead of execSync(\cmd ${arg}`)`.
• Guard lifecycle scripts with require.main === module. Any script that npm invokes automatically (preinstall, postinstall, prepare) should separate "importable module" behavior from "executed as entry point" behavior, both for safety and testability.
• Treat child_process calls as high-risk by default, even when today's inputs are hardcoded — code evolves, and a hardcoded string today can become a template literal with a variable tomorrow.
• Run static analysis (Semgrep, CodeQL, ESLint security plugins) in CI to catch child_process usage early; the rule that caught this issue, javascript.lang.security.detect-child-process.detect-child-process, is specifically designed for this pattern.
• Add tests for install/build scripts, as this PR did with test/postinstall.test.js, so hardening doesn't regress silently in future refactors.

Key Takeaways

• scripts/postinstall.js previously executed execSync() shell commands as a side effect of module load — now they only run when the file is npm's actual entry point.
• The addition of execFileSync gives the codebase a shell-free execution path ready to use if these commands ever need dynamic arguments.
• Semgrep's detect-child-process rule caught this before it became exploitable — a reminder that "not exploitable today" doesn't mean "safe to leave unguarded."
• A new CI step (node --test test/postinstall.test.js) now enforces this hardened behavior on every build, closing the door on silent regressions.

How Orbis AppSec Detected This

• Source: Function argument file flowing into a child_process call in scripts/postinstall.js:28, flagged by Semgrep as originating from a potentially non-constant input path.
• Sink: execSync('npx electron-builder install-app-deps', { stdio: 'inherit' }) and the fallback execSync('npx @electron/rebuild -f -m . -o better-sqlite3', { stdio: 'inherit' }) calls, executed unconditionally at module load.
• Missing control: No guard separating "module imported" from "script executed as entry point," and no use of an argument-array exec API (execFileSync) to eliminate shell interpretation of command strings.
• CWE: CWE-78 (Improper Neutralization of Special Elements used in an OS Command / OS Command Injection).
• Fix: Imported execFileSync, wrapped the install logic in a require.main === module guard, and added test/postinstall.test.js run in CI to verify the hardened script.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

This wasn't a vulnerability being actively exploited — it was a latent primitive sitting in an install-time script that ran shell commands unconditionally. By introducing execFileSync, gating side effects behind require.main === module, and backing it with a CI-enforced test, the fix removes the primitive before it could ever be chained into something worse. The lesson generalizes well beyond this one file: any script that shells out — especially one npm runs automatically — deserves the same scrutiny you'd give user-facing input handling, because today's hardcoded string is tomorrow's injectable variable.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
• OWASP Command Injection Prevention Cheat Sheet
• Node.js child_process documentation
• Semgrep rule: detect-child-process
• harden: sanitize child_process call in postinstall.js...