The Problem With Trusting API Responses in File Paths
The index.js file in this private Node.js application automates the management of Git submodules by fetching repository names from an external API and using them to clone dependencies into a libraries/ directory. It's a common pattern in monorepo tooling — but a subtle flaw in how those repository names were consumed created a meaningful path traversal risk.
At line 40, the loop over uniqueRepoFullNames passed each repoName value directly into both path.join('libraries', repoName) and as a path argument to git submodule add — without ever checking whether the value looked like a legitimate owner/repo string. This is the textbook setup for CWE-22.
The Vulnerability Explained
Here is the vulnerable loop as it existed before the fix:
// BEFORE — vulnerable code
for (const repoName of uniqueRepoFullNames) {
if (!fs.existsSync(path.join('libraries', repoName))) {
console.log('Adding git submodule for', repoName);
await execFile('git', ['submodule', 'add', '--depth', '1',
`https://github.com/${repoName}.git`, repoName], {
// ...
});
}
}
At first glance, using execFile() with an array of arguments looks safe — and for shell injection, it is. There's no shell interpreter involved, so a value like foo; rm -rf / won't execute arbitrary commands. However, execFile() does nothing to interpret or sanitize the content of the arguments it passes. The git binary receives exactly what you give it.
The real danger is in how repoName is used as a filesystem path:
path.join('libraries', repoName)— Node'spath.joinresolves..segments. A value like../../../home/user/.ssh/authorized_keyswould resolve to a path well outside thelibraries/directory.- The final positional argument to
git submodule add— This is the local path where git checks out the submodule. Supplying a traversal sequence here instructsgitto write the submodule into an attacker-chosen directory.
Concrete Attack Scenario
Imagine the external API that populates uniqueRepoFullNames is compromised, returns a misconfigured response, or is under an attacker's control. It returns this value:
../../../.git/hooks/pre-commit
When the code runs:
path.join('libraries', '../../../.git/hooks/pre-commit')
// resolves to: '.git/hooks/pre-commit'
fs.existsSync checks whether .git/hooks/pre-commit exists. If it doesn't, the code proceeds to run:
git submodule add --depth 1 \
https://github.com/../../../.git/hooks/pre-commit.git \
../../../.git/hooks/pre-commit
Git would attempt to clone an attacker-controlled repository into the .git/hooks/ directory — a location where git executes scripts automatically. This could result in arbitrary code execution on every subsequent git operation in the repository, affecting every developer who clones or pulls.
Even without reaching .git/hooks, an attacker could overwrite configuration files, inject malicious content into other submodules, or corrupt the repository state — all without shell injection.
The Fix
The fix adds a validation block at the very top of the loop, before any filesystem or process operation occurs:
// AFTER — patched code
for (const repoName of uniqueRepoFullNames) {
if (
typeof repoName !== 'string' ||
!/^(?!\.{1,2}\/)(?!.*\/\.{1,2}$)[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+$/.test(repoName)
) {
console.warn('Skipping invalid repo name:', repoName);
continue;
}
if (!fs.existsSync(path.join('libraries', repoName))) {
console.log('Adding git submodule for', repoName);
await execFile('git', ['submodule', 'add', '--depth', '1',
`https://github.com/${repoName}.git`, repoName], {
// ...
});
}
}
Let's break down exactly what the regex enforces:
/^(?!\.{1,2}\/)(?!.*\/\.{1,2}$)[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+$/
| Component | What it does |
|---|---|
^ |
Anchors to the start of the string |
(?!\.{1,2}\/) |
Negative lookahead: rejects strings starting with ./ or ../ |
(?!.*\/\.{1,2}$) |
Negative lookahead: rejects strings ending with /. or /.. |
[a-zA-Z0-9_.-]+ |
Owner segment: only alphanumeric, underscore, dot, hyphen |
\/ |
Exactly one forward slash separator |
[a-zA-Z0-9_.-]+ |
Repo segment: same character class |
$ |
Anchors to end of string |
This is an allowlist approach — it defines precisely what a valid owner/repo string looks like and rejects everything else. The negative lookaheads add explicit defense against dot-segment traversal even within otherwise-valid characters.
The typeof repoName !== 'string' check at the start guards against non-string values (null, undefined, objects) that would cause path.join to throw or behave unexpectedly.
A value like ../../../.git/hooks/pre-commit fails the first negative lookahead immediately. A value like legitimate-org/../../secrets fails the character class check (the repeated slashes and dots don't form a valid single-slash owner/repo pattern). A value like valid-org/valid-repo passes all checks and proceeds normally.
Prevention & Best Practices
1. Validate External Data at the Boundary
Any value arriving from an external API, HTTP request, or environment variable should be validated before it's used in a security-sensitive context. Don't wait until the data reaches path.join() — validate it as soon as it enters your application.
2. Use Allowlists, Not Blocklists
The fix uses a regex that defines what is allowed, not a list of things to reject. Blocklists (e.g., "reject strings containing ..") are notoriously incomplete — there are encoded variants (%2e%2e), Unicode representations, and null-byte tricks that bypass naive checks. Allowlists are far more robust.
3. Resolve and Assert the Final Path
For filesystem operations, add a second layer of defense using path.resolve():
const basePath = path.resolve('libraries');
const targetPath = path.resolve('libraries', repoName);
if (!targetPath.startsWith(basePath + path.sep)) {
throw new Error(`Path traversal detected: ${repoName}`);
}
This catches traversal attempts that might slip through regex validation due to URL encoding or platform-specific path handling.
4. execFile() Is Necessary But Not Sufficient
Using execFile() with an array of arguments (rather than exec() with a shell string) is the correct way to prevent shell injection in Node.js. But as this vulnerability demonstrates, it does not protect against path traversal in the argument values. Both defenses are needed.
5. Log and Monitor Rejected Values
The fix correctly uses console.warn('Skipping invalid repo name:', repoName) rather than silently ignoring invalid inputs. In production, these warnings should be forwarded to a monitoring system — a burst of invalid repo names from an API could indicate a supply chain compromise or API tampering.
Relevant Standards
- OWASP Path Traversal: https://owasp.org/www-community/attacks/Path_Traversal
- CWE-22: Improper Limitation of a Pathname to a Restricted Directory
- OWASP Input Validation Cheat Sheet: https://cheatsheetseries.owasp.org/cheatsheets/Input_Validation_Cheat_Sheet.html
Key Takeaways
execFile()with array arguments prevents shell injection, but not path traversal — both the argument structure and the argument content must be validated independently.- The
repoNamevariable inindex.jswas the exact trust boundary that needed a gate — it crossed from external API data into filesystem operations without any validation checkpoint. - The negative lookaheads in the fix (
(?!\.{1,2}\/)) are critical — a simpler character-class regex without them could still allow traversal sequences in edge cases. - Git submodule path arguments are especially dangerous — because git executes hooks from paths it manages, path traversal into
.git/hooks/can escalate directly to code execution. - Allowlist validation must happen before both
fs.existsSync()andexecFile()— the fix correctly places the check at the top of the loop, before either operation runs.
How Orbis AppSec Detected This
- Source:
repoNamevalues sourced fromuniqueRepoFullNames, populated by an external API response inindex.js - Sink:
path.join('libraries', repoName)at line 40 and the positional path argument inexecFile('git', ['submodule', 'add', ..., repoName], ...)— both consuming the unsanitized external value - Missing control: No type check, no format validation, and no path boundary assertion before the value was used in filesystem and child process operations
- CWE: CWE-22 — Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
- Fix: A strict allowlist regex was inserted at the top of the loop to validate the
owner/repoformat and explicitly reject dot-segment traversal sequences, with acontinueto skip invalid entries and aconsole.warnto surface them for monitoring
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
Path traversal vulnerabilities are easy to overlook precisely because the code around them often looks secure. In this case, the use of execFile() with array arguments was a correct and important security choice — but it addressed only half the problem. The repoName values flowing from an external API into path.join() and git's path argument represented an unguarded trust boundary.
The fix is elegant in its simplicity: a single regex validation block that enforces the known-good format of a GitHub repository name. It's a reminder that the most effective security controls are those applied closest to where untrusted data enters a sensitive operation — and that allowlist validation almost always beats blocklist approaches for path and identifier inputs.
If your Node.js tooling consumes external API data and uses it in filesystem paths, audit those data flows carefully. The gap between "prevents shell injection" and "prevents path traversal" is smaller than it looks, but the consequences of missing it can be severe.