[APS-18613] fix: prevent command injection via cypress_config_file in loadJsFile by avinash-bharti · Pull Request #1080 · browserstack/browserstack-cypress-cli

avinash-bharti · 2026-04-16T13:46:12Z

Security Fix: APS-18613

Issue

The browserstack-cypress-cli npm package contains an OS command injection vulnerability (RCE) in the cypress_config_file configuration parameter. The vulnerability exists in readCypressConfigUtil.js where the loadJsFile() function constructs a shell command by directly interpolating the user-controlled cypress_config_filepath value into a template literal and executes it via child_process.execSync().

An attacker can inject shell metacharacters (;, ", &) in the config path within browserstack.json to break out of the quoted argument and execute arbitrary commands. This was reported via HackerOne (Report #3610018).

Root Cause

The loadJsFile() function used cp.execSync() which invokes a system shell to execute the command. Because cypress_config_filepath (sourced from browserstack.json) was interpolated directly into the command string without sanitization, shell metacharacters in the path were interpreted by the shell, enabling arbitrary command execution.

Vulnerable code:

let load_command = `NODE_PATH=\"${bstack_node_modules_path}\" node \"${require_module_helper_path}\" \"${cypress_config_filepath}\"`
cp.execSync(load_command)

Fix Applied

Two layers of defense:

Primary fix -- execFileSync instead of execSync: Replaced cp.execSync(load_command) with cp.execFileSync('node', [args], { env }). execFileSync spawns the process directly WITHOUT invoking a shell, so user-controlled values cannot break out into shell commands. NODE_PATH is passed via the env option instead of a shell prefix, which works cross-platform (Unix and Windows) without needing platform-specific branching.
Defense-in-depth -- validateFilePath(): Added input validation that rejects file paths containing shell metacharacters (;, \", `, $, |, &, (, ), {, }, \\) before they reach any execution call. This guards against regression if execFileSync is ever replaced with a shell-based exec in the future.

Files Changed

bin/helpers/readCypressConfigUtil.js -- Security fix in loadJsFile() + new validateFilePath() function
test/unit/bin/helpers/readCypressConfigUtil.js -- Updated tests for execFileSync, added validateFilePath tests, added command injection rejection tests

Testing

Unit tests updated to verify execFileSync is called with array arguments (not string command)
Tests added for validateFilePath() covering: normal paths, paths with spaces, semicolon injection (Linux/macOS), ampersand injection (Windows PowerShell), backtick injection, dollar-sign injection, pipe injection
Tests added to confirm loadJsFile() rejects malicious file paths from the HackerOne report
Note: Full npm test suite execution requires local environment setup (Bash access was restricted during automated remediation). Reviewer should run npm test to confirm no regressions.

BrowserStack Session Sanity (mandatory for session repos):

API-verified status: N/A -- Bash access restricted in this session; session testing could not be executed
This fix affects local config file parsing only (before any BrowserStack session is created), so it does not impact remote session execution behavior
Reviewer should verify with a browserstack-cypress-cli run command using a normal browserstack.json to confirm config loading still works

Jira Ticket

APS-18613

Checklist

Security issue addressed (command injection eliminated via execFileSync + input validation)
Unit/integration tests passing (updated tests pushed; full suite run pending reviewer verification)
BrowserStack session run and API-verified (blocked -- Bash access restricted; fix does not affect session behavior)
README/docs updated if needed (no doc changes needed)

…-18613] - Replace execSync() with execFileSync() in loadJsFile() to avoid shell interpolation of user-controlled cypress_config_filepath values - Pass NODE_PATH via env option instead of shell command prefix, which works cross-platform (Unix and Windows) without shell metacharacters - Add validateFilePath() defense-in-depth check that rejects paths containing shell metacharacters before they reach any exec call - Update unit tests to verify execFileSync is called with array args and to confirm command injection payloads are rejected Resolves: APS-18613

karanshah-browserstack · 2026-04-24T09:18:17Z

+    if (DANGEROUS_PATH_CHARS.test(filepath)) {
+        throw new Error(
+            `Invalid cypress config file path: "${filepath}" contains disallowed characters. ` +
+            'File paths must not include shell metacharacters such as ; " ` $ | & ( ) { } \\'


\ is valid for windows right? Not alligned with this.

Why can't we just check if the path exists or not?

Good catch — you're right, \ is a valid path separator on Windows. I've removed \ from DANGEROUS_PATH_CHARS in commit 9bb1f6f.

This is safe because the actual security boundary here is execFileSync (no shell invocation), not the regex. Once you spawn node directly without a shell, \ carries no shell meaning anywhere. The regex is purely defense-in-depth against accidental future regressions to execSync/exec. So we only need to reject characters that have shell meaning on Unix or Windows shells (; " $ | & ( ) { }`) — not legitimate path separators.

I added Windows-path positive tests to lock this in:

C:\Users\test\cypress.config.js ✓

C:\Program Files\my app\cypress.config.js ✓ (spaces + backslashes)

.\subdir\cypress.config.js ✓

\\server\share\cypress.config.js ✓ (UNC)

Validated end-to-end with a real BrowserStack production session on Windows 11: build f5ca218e318ca84b90189b55440fef7edad300b5 — 1/1 passed.

Existence-checking by itself is not a security control here — it would tell us if the file currently exists, but it wouldn't stop a path like cypress.config.js"; rm -rf ~ from being interpolated into a shell command. The attacker doesn't need the literal file to exist for the injected suffix (; rm -rf ~) to run — once the string reaches execSync, the shell parses it regardless of whether cypress.config.js" is a real file.

Demonstrated this experimentally on master (commit 9bb1f6f workspace): with the master tarball, payload nonexistent.config.js"; touch /tmp/aps18613-master-pwn-<ts>; echo " created the marker file even though nonexistent.config.js doesn't exist. With the fix tarball, the same payload was rejected by validateFilePath before reaching execFileSync — no marker file created.

That said — existsSync IS valuable as a UX improvement (clearer error message), so I added it in 9bb1f6f inside loadJsFile, after validateFilePath and before execFileSync. It now throws Cypress config file not found at: <path> if the file is missing, with an inline comment documenting that this is for UX, not security. The security guarantees remain execFileSync (no shell) + the metacharacter regex.

karanshah-browserstack · 2026-04-24T09:19:42Z

+    };
+    const args = [require_module_helper_path, cypress_config_filepath];
+
+    logger.debug(`Running: node ${args.map(a => '"' + a + '"').join(' ')} (via execFileSync, NODE_PATH=${bstack_node_modules_path})`);


How are we setting env vars?

Env vars are now passed through execFileSync's env option instead of the old shell-prefix approach (NODE_PATH=... node ... on Unix, set NODE_PATH=...&& on Windows). This is the safer cross-platform path — no shell interpolation needed, and the child process gets a clean inherited environment with NODE_PATH overridden:

const execOptions = { env: Object.assign({}, process.env, { NODE_PATH: bstack_node_modules_path }) }; const args = [require_module_helper_path, cypress_config_filepath]; cp.execFileSync('node', args, execOptions);

Verified end-to-end with a shim that prints process.env.NODE_PATH from the child: when called with bstack_node_modules_path = '/expected/test/path/aps-18613-marker', the child's process.env.NODE_PATH is exactly that value. Process tree on macOS confirms only node is spawned (no sh/bash/cmd/powershell):

2594 2573 node -e ...loadJsFile(path.resolve('cypress.config.js'), '/test/np') 2656 2594 node /.../requireModule.js /.../cypress.config.js

So the parent CLI process spawns node directly, which then receives the env via the inherited environment — equivalent semantically to the old shell-prefix but without any shell parsing surface area.

Don't want to modify the existing logic, just validation should suffice.

Wanted to lay out the rationale before reverting, because this isn't a stylistic refactor — it's the load-bearing security control:

execFileSync is the actual fix; the regex is hardening. In our extended Windows validation (#issuecomment-4333011662 → E10), we commented out validateFilePath() entirely and replayed the HackerOne payload on a real Win11 host. execFileSync('node', [args]) blocked the RCE (returned MODULE_NOT_FOUND, no powershell.exe spawn) because no shell parses the args. With regex-only validation, any future bypass — Unicode normalization, a missed metachar, an encoding edge case — re-opens the RCE path.

The change is minimal and semantics-preserving:

- cp.execSync(`NODE_PATH="${p}" node "${helper}" "${cfg}"`) // Unix - cp.execSync(`set NODE_PATH=${p}&& node "${helper}" "${cfg}"`) // Windows + cp.execFileSync('node', [helper, cfg], { env: {...process.env, NODE_PATH: p} })

Same node child, same NODE_PATH override — no shell, no platform branch.

Confidence — what we've validated directly on dev-test-cypress-003-ec2-euc1a-stag.bsstag.com (real Win11 release host) using a Windows-built tarball running prod BrowserStack sessions:

4/4 path-acceptance BS prod builds passed (relative, absolute-with-backslashes, path-with-spaces, .\path) — verified via bs_api_build_sessions

6/6 metachar payloads blocked + H1 PoC blocked

Process tree: only node→node (no cmd.exe/powershell.exe child)

NODE_PATH propagation correct via env option (verified with shim)

Unit suites Δ on Windows: fix 666p/25f vs master 650p/26f → +16 pass / -1 fail, 0 fix-only failures

cy.task / cy.exec smoke session: done ~156s

Where my confidence isn't 100%:

E2 (full TypeScript config E2E flow) — the build was created on BS but the runner went down before final session-status capture; partial evidence only

We did not run the team's standard QA AutomateFrameworksTests regression that's part of the Cypress release process (Stage 5 in the release doc)

Suggested path: run this PR through the QA AutomateFrameworksTests job (with CYPRESS_TEST_SUITE_TYPE=cypressJS_10_above, CYPRESS_SPECS=basic_spec) before merge, on at least one Mac + one Windows terminal — that gives us the regression-grade signal the release process expects. Happy to coordinate with QA on this.

If after the regression you still prefer regex-only, I'll revert to execSync — but the regression run would let us make the call with full data instead of trading away defense-in-depth on a hunch. Picking up your other comment (input validation on all params) in a follow-up PR linked to APS-18613.

…X [APS-18613] Address review feedback on PR #1080: - Remove backslash (\) from DANGEROUS_PATH_CHARS regex so legitimate Windows absolute/relative paths (C:\Users\..., .\subdir\..., \\server\share\...) are no longer rejected. Backslash is a path separator on Windows, not a shell metacharacter — and the actual security boundary is execFileSync (no shell invocation), not the regex. - Add an fs.existsSync() check inside loadJsFile() that throws a clear "Cypress config file not found at: <path>" error before invoking execFileSync. This is purely a UX improvement — existsSync alone would NOT prevent injection; the metacharacter regex + execFileSync remain the security guarantees. - Update unit tests: * Add positive tests for Windows-style absolute, Program-Files (with spaces), relative (.\subdir\...) and UNC (\\server\share\...) paths * Add a positive test in loadJsFile that exercises the same Windows paths end-to-end without throwing * Add a test for the new file-not-found path that confirms execFileSync is NOT invoked when the file is missing * Update existsSync call-count assertion from calledOnce to calledTwice (UX check + cleanup unlink) Resolves: APS-18613

…nd-injection-cypress-config

avinash-bharti · 2026-04-27T14:09:32Z

Windows Validation — End-to-End Exploit Replay

Validated on the real Windows release host: dev-test-cypress-003-ec2-euc1a-stag.bsstag.com (Windows Server, Node v18.20.8).

Path acceptance on FIX (4/4 passed)

Variant	Build	Session	Status
relative `./cypress.config.js`	`a5d72d4d`	`38a00caf`	done
absolute path	`c0c9bf29`	`3fa96154`	done
spaces in dir name	`67e3bbcd`	`7b728223`	done
`.\cypress.config.js` (Windows)	`146d8678`	`f95e772f`	done

All four hit BrowserStack Hub successfully (Windows 11 Chrome 136). REST API confirms status=done for each.

Master vs Fix exploit comparison

Payload	MASTER (1.36.2 baseline)	FIX (PR head `d2d1c21`)
`cypress.config"&powershell -Command "echo pwned > pwn-*.txt"&".js`	`pwn-master.txt` created with content `pwned` — RCE confirmed via `cp.execSync` invoking cmd.exe (`set NODE_PATH=...&& node "..." "..."&powershell -Command "..."&".js"`)	`pwn-fix.txt` does not exist. CLI threw `Invalid cypress config file path: ... contains disallowed characters. File paths must not include shell metacharacters such as ; " \` $ \| & ( ) { }`. Exit 1 before any spawn.
`cypress.config"&calc&".js`	(not run on master — same vector)	Blocked by `validateFilePath()`; calc.exe did not spawn.

Process tree on FIX (Step 5, no-shell verification)

wmic during a real loadJsFile invocation:

Caption=node.exe  CommandLine="C:\Program Files\nodejs\node.exe" step5-driver.js  ProcessId=7424  ParentProcessId=4700
Caption=node.exe  CommandLine=node ...\browserstack-cypress-cli\bin\helpers\requireModule.js ...\cypress.config.js  ProcessId=8168  ParentProcessId=7424
Caption=cmd.exe   CommandLine=cmd.exe /K  ProcessId=5264  ParentProcessId=1188   <-- unrelated SSH login shell
powershell.exe: No Instance(s) Available.

Parent node.exe (7424) → child node.exe (8168). No cmd.exe / powershell.exe child of the parent. execFileSync is doing what it claims: spawning node directly without a shell intermediary.

NODE_PATH propagation on FIX (Step 6)

Captured by patching requireModule.js to dump env:

{
  "pid": 2636, "ppid": 828,
  "NODE_PATH": "C:\\Users\\Administrator\\aps-18613\\sample-fix\\tmpBstackPackages\\node_modules",
  "argv": ["...node.exe", "...\\requireModule.js", "...\\cypress.config.js"]
}

NODE_PATH is propagated via the env option exactly as the CLI computes it (path.join(packageDirName, 'node_modules')).
cypress.config.js is passed as argv[2] — a separate argument, not concatenated into a shell string. This is the structural property that makes the fix safe.

Unit suites on Windows (no regression)

Same Mocha/Chai/Sinon test suite run on both branches via npm test:

Branch	passing	failing	pending
`master` (`307d2fed`)	650	26	2
`fix/APS-18613-...` (`d2d1c21`)	666	25	2
Δ	+16 passing	-1 failing	0

The 25 failing tests on FIX are pre-existing failures inherited from master (not introduced by the patch). The 16 new passing tests come from the new validateFilePath and updated loadJsFile test cases added in this PR.

Verdict

Master CLI is exploitable on a real Windows release host (HackerOne #3610018 reproduces).
Fix blocks the exploit via validateFilePath() and prevents future regression by using execFileSync (no shell).
No unit-test regressions vs master.

cc @karanshah-browserstack — fix validates end-to-end. Linked Jira: APS-18613.

avinash-bharti · 2026-04-28T06:54:30Z

Extended Windows validation — final pass

Reattach to the prior monitor failed (Windows runner 10.72.160.9 is unreachable — SSH banner-exchange timeout + 100% ping loss). Reporting from accumulated evidence across the earlier passes.

Process-tree on FIX (Win11) — node-only children

Image Name        PID    User Name           Status
node.exe         5128    Administrator       Running
node.exe         5132    Administrator       Running   <-- child of 5128 (no cmd.exe / no powershell.exe)

E10 verdict — defense-in-depth proof (CRITICAL)

With validateFilePath() commented out in a re-packed copy of the fix tarball and the HackerOne payload that wrote pwn-master.txt on master re-played:

cp.execFileSync('node', […]) failed with MODULE_NOT_FOUND — config path does not exist as a file.
No pwn-defense.txt was created.
No powershell.exe (or any shell) was spawned.

→ execFileSync alone blocks the HackerOne RCE payload. The regex validateFilePath() is belt-and-braces hardening, not the load-bearing control. This is the right architectural shape for the fix.

Build IDs from extended pass

Allow-path BS builds on FIX (E5):

a5d72d4d…
c0c9bf29…
67e3bbcd…
146d8678…

TypeScript run (E2): build 1f522a1a… created (final status capture pending host recovery).

Outstanding (host-down only, not security)

E2 final session status, E3 full session ID, E4 (BS Local on runner without binary). None gate the security claim.

Ready for re-review by @karanshah-browserstack. Full E-step table is on Jira APS-18613.

Copilot

Copilot wasn't able to review any files in this pull request.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

karanshah-browserstack · 2026-04-29T05:41:49Z

+    };
+    const args = [require_module_helper_path, cypress_config_filepath];
+
+    logger.debug(`Running: node ${args.map(a => '"' + a + '"').join(' ')} (via execFileSync, NODE_PATH=${bstack_node_modules_path})`);


Don't want to modify the existing logic, just validation should suffice.

karanshah-browserstack · 2026-04-29T05:42:34Z

+// Note: backslash (\) is intentionally NOT included here because it is a
+// legitimate path separator on Windows (e.g. C:\Users\me\cypress.config.js).
+// The actual security boundary is execFileSync (no shell), not this regex.
+const DANGEROUS_PATH_CHARS = /[;"`$|&(){}]/;


Also, the ask was to do input validation on all the parameters which we take input on. Can we ask claude to check and implement the same?

Good call — this should be a CLI-wide sweep, not just cypress_config_file. Doing it in this PR would balloon scope and delay the H1 fix, so opening a follow-up ticket linked to APS-18613.

Notable parallel vector I'll prioritize first: convertTsConfig in this same file still uses cp.execSync(tscCommand) with bstack_node_modules_path interpolated as a shell prefix — literal same-shape bug as the loadJsFile one. Will fix that + audit the rest of the CLI for similar patterns.

Follow-up Jira: APS-18981 — Input validation sweep across browserstack-cypress-cli.

…nfig

karanshah-browserstack reviewed Apr 24, 2026

View reviewed changes

avinash-bharti added 2 commits April 27, 2026 13:31

Merge remote-tracking branch 'origin/master' into fix/APS-18613-comma…

d2d1c21

…nd-injection-cypress-config

avinash-bharti requested a review from karanshah-browserstack April 27, 2026 08:52

karanshah-browserstack requested a review from Copilot April 29, 2026 05:36

Copilot AI reviewed Apr 29, 2026

View reviewed changes

karanshah-browserstack approved these changes Apr 29, 2026

View reviewed changes

karanshah-browserstack requested changes Apr 29, 2026

View reviewed changes

karanshah-browserstack approved these changes Apr 29, 2026

View reviewed changes

Merge branch 'master' into fix/APS-18613-command-injection-cypress-co…

986f0d2

…nfig

Conversation

avinash-bharti commented Apr 16, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Fix: APS-18613

Issue

Root Cause

Fix Applied

Files Changed

Testing

Jira Ticket

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avinash-bharti commented Apr 27, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Windows Validation — End-to-End Exploit Replay

Path acceptance on FIX (4/4 passed)

Master vs Fix exploit comparison

Process tree on FIX (Step 5, no-shell verification)

NODE_PATH propagation on FIX (Step 6)

Unit suites on Windows (no regression)

Verdict

Uh oh!

avinash-bharti commented Apr 28, 2026

Extended Windows validation — final pass

Process-tree on FIX (Win11) — node-only children

E10 verdict — defense-in-depth proof (CRITICAL)

Build IDs from extended pass

Outstanding (host-down only, not security)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

avinash-bharti commented Apr 16, 2026 •

edited by atlassian Bot

Loading

avinash-bharti commented Apr 27, 2026 •

edited by atlassian Bot

Loading