Skip to content

Commit ad117a0

Browse files
[APS-19009] security(cli): --ignore-scripts + validate npm_dependencies names/versions
browserstack.json's npm_dependencies were merged into a temp package.json and installed with `npm install` and no `--ignore-scripts`, so a PR-supplied malicious package's lifecycle script (postinstall) executed on CI (RCE, credential theft). - Add `--ignore-scripts` to both npm install invocations (the RCE fix). npm_dependencies is documented pure-JS only. - Validate each dependency name (standard npm package-name regex) and version (semver/dist-tag charset only) before writing package.json, rejecting git-url / file: / path / alternate-registry specs (dependency confusion / code-exec via spec). - shell:true is retained deliberately: the command line is fully static (names live in package.json data, never on the command line -> no injection surface) and it is required for the output redirection and for invoking npm.cmd on Windows. Tested: validation rejects shell-metachar/git-url/file/$() specs, accepts normal semver; --ignore-scripts present in both installs; syntax clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e578fad commit ad117a0

1 file changed

Lines changed: 22 additions & 4 deletions

File tree

‎bin/helpers/packageInstaller.js‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,18 @@ const setupPackageFolder = (runSettings, directoryPath) => {
3333

3434
// Combine win and mac specific dependencies if present
3535
const combinedDependencies = combineMacWinNpmDependencies(runSettings);
36+
// APS-19009: only allow standard npm package names + semver/dist-tag versions
37+
// before writing them to package.json, so a browserstack.json cannot smuggle a
38+
// git-url / file: / path / alternate-registry spec (dependency confusion or code
39+
// execution) into `npm install`.
40+
const NPM_NAME_RE = /^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/;
41+
const NPM_VERSION_RE = /^[A-Za-z0-9.\-+~^><=|*\s]+$/;
42+
for (const depName of Object.keys(combinedDependencies || {})) {
43+
const depVersion = combinedDependencies[depName];
44+
if (!NPM_NAME_RE.test(depName) || typeof depVersion !== 'string' || !NPM_VERSION_RE.test(depVersion)) {
45+
return reject(`Invalid npm_dependencies entry "${depName}": only standard package names and semver/dist-tag versions are allowed.`);
46+
}
47+
}
3648
if (combinedDependencies && Object.keys(combinedDependencies).length > 0) {
3749
Object.assign(packageJSON, {
3850
devDependencies: combinedDependencies,
@@ -97,12 +109,18 @@ const packageInstall = (packageDir, bsConfig) => {
97109

98110
// add --legacy-peer-deps flag while installing dependencies for npm v7+
99111
// For more info please read "Peer Dependencies" section here -> https://github.blog/2021-02-02-npm-7-is-now-generally-available/
112+
// APS-19009: --ignore-scripts prevents a user-supplied npm_dependencies package from
113+
// executing lifecycle scripts (postinstall etc.) during this install, which was an RCE
114+
// on CI. npm_dependencies is documented as pure-JS only. shell:true is retained on
115+
// purpose: the command line is fully static (package names live in package.json, never
116+
// on the command line, so there is no injection surface) and it is required for the
117+
// output redirection and for invoking npm.cmd on Windows.
100118
if (parseInt(npm_major_version) >= 7) {
101-
logger.debug(`Running NPM install command: npm install --legacy-peer-deps --loglevel verbose > ../npm_install_debug.log`);
102-
nodeProcess = spawn(/^win/.test(process.platform) ? 'npm.cmd' : 'npm', ['install', '--legacy-peer-deps', '--loglevel', 'verbose', '>', '../npm_install_debug.log', '2>&1'], {cwd: packageDir, shell: true});
119+
logger.debug(`Running NPM install command: npm install --legacy-peer-deps --ignore-scripts --loglevel verbose > ../npm_install_debug.log`);
120+
nodeProcess = spawn(/^win/.test(process.platform) ? 'npm.cmd' : 'npm', ['install', '--legacy-peer-deps', '--ignore-scripts', '--loglevel', 'verbose', '>', '../npm_install_debug.log', '2>&1'], {cwd: packageDir, shell: true});
103121
} else {
104-
logger.debug(`Running NPM install command: 'npm install --loglevel verbose > ../npm_install_debug.log'`);
105-
nodeProcess = spawn(/^win/.test(process.platform) ? 'npm.cmd' : 'npm', ['install', '--loglevel', 'verbose', '>', '../npm_install_debug.log', '2>&1'], {cwd: packageDir, shell: true});
122+
logger.debug(`Running NPM install command: 'npm install --ignore-scripts --loglevel verbose > ../npm_install_debug.log'`);
123+
nodeProcess = spawn(/^win/.test(process.platform) ? 'npm.cmd' : 'npm', ['install', '--ignore-scripts', '--loglevel', 'verbose', '>', '../npm_install_debug.log', '2>&1'], {cwd: packageDir, shell: true});
106124
}
107125
nodeProcess.on('close', nodeProcessCloseCallback);
108126
nodeProcess.on('error', nodeProcessErrorCallback);

0 commit comments

Comments
 (0)