From a62a3a241603fc5f2948f581473caa673f04bc9e Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 13 Feb 2026 02:27:04 -0800 Subject: [PATCH] fix: sanitize getExecCommand args, escape regex in getCommandPattern, clean up readStdinJson timeout, add 10 tests Validate args parameter in getExecCommand() against SAFE_ARGS_REGEX to prevent command injection when returned string is passed to a shell. Escape regex metacharacters in getCommandPattern() generic action branch to prevent malformed patterns and unintended matching. Clean up stdin listeners in readStdinJson() timeout path to prevent process hanging. --- scripts/lib/package-manager.js | 25 ++++++++++---- scripts/lib/utils.js | 5 +++ tests/lib/package-manager.test.js | 56 +++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/scripts/lib/package-manager.js b/scripts/lib/package-manager.js index c2580d60..c12537f0 100644 --- a/scripts/lib/package-manager.js +++ b/scripts/lib/package-manager.js @@ -314,11 +314,15 @@ function getRunCommand(script, options = {}) { } } +// Allowed characters in arguments: alphanumeric, whitespace, dashes, dots, slashes, +// equals, colons, commas, quotes, @. Rejects shell metacharacters like ; | & ` $ ( ) { } < > ! +const SAFE_ARGS_REGEX = /^[@a-zA-Z0-9\s_.\/:=,'"*+-]+$/; + /** * Get the command to execute a package binary * @param {string} binary - Binary name (e.g., "prettier", "eslint") * @param {string} args - Arguments to pass - * @throws {Error} If binary name contains unsafe characters + * @throws {Error} If binary name or args contain unsafe characters */ function getExecCommand(binary, args = '', options = {}) { if (!binary || typeof binary !== 'string') { @@ -327,6 +331,9 @@ function getExecCommand(binary, args = '', options = {}) { if (!SAFE_NAME_REGEX.test(binary)) { throw new Error(`Binary name contains unsafe characters: ${binary}`); } + if (args && typeof args === 'string' && !SAFE_ARGS_REGEX.test(args)) { + throw new Error(`Arguments contain unsafe characters: ${args}`); + } const pm = getPackageManager(options); return `${pm.config.execCmd} ${binary}${args ? ' ' + args : ''}`; @@ -351,6 +358,11 @@ function getSelectionPrompt() { return message; } +// Escape regex metacharacters in a string before interpolating into a pattern +function escapeRegex(str) { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + /** * Generate a regex pattern that matches commands for all package managers * @param {string} action - Action pattern (e.g., "run dev", "install", "test") @@ -387,12 +399,13 @@ function getCommandPattern(action) { 'bun run build' ); } else { - // Generic run command + // Generic run command — escape regex metacharacters in action + const escaped = escapeRegex(action); patterns.push( - `npm run ${action}`, - `pnpm( run)? ${action}`, - `yarn ${action}`, - `bun run ${action}` + `npm run ${escaped}`, + `pnpm( run)? ${escaped}`, + `yarn ${escaped}`, + `bun run ${escaped}` ); } diff --git a/scripts/lib/utils.js b/scripts/lib/utils.js index c9d08418..4c497546 100644 --- a/scripts/lib/utils.js +++ b/scripts/lib/utils.js @@ -215,6 +215,11 @@ async function readStdinJson(options = {}) { const timer = setTimeout(() => { if (!settled) { settled = true; + // Clean up stdin listeners so the event loop can exit + process.stdin.removeAllListeners('data'); + process.stdin.removeAllListeners('end'); + process.stdin.removeAllListeners('error'); + if (process.stdin.unref) process.stdin.unref(); // Resolve with whatever we have so far rather than hanging try { resolve(data.trim() ? JSON.parse(data) : {}); diff --git a/tests/lib/package-manager.test.js b/tests/lib/package-manager.test.js index 67195e24..02e125a8 100644 --- a/tests/lib/package-manager.test.js +++ b/tests/lib/package-manager.test.js @@ -874,6 +874,62 @@ function runTests() { } })) passed++; else failed++; + // ─── Round 21: getExecCommand args validation ─── + console.log('\ngetExecCommand (args validation):'); + + if (test('rejects args with shell metacharacter semicolon', () => { + assert.throws(() => pm.getExecCommand('prettier', '; rm -rf /'), /unsafe characters/); + })) passed++; else failed++; + + if (test('rejects args with pipe character', () => { + assert.throws(() => pm.getExecCommand('prettier', '--write . | cat'), /unsafe characters/); + })) passed++; else failed++; + + if (test('rejects args with backtick injection', () => { + assert.throws(() => pm.getExecCommand('prettier', '`whoami`'), /unsafe characters/); + })) passed++; else failed++; + + if (test('rejects args with dollar sign', () => { + assert.throws(() => pm.getExecCommand('prettier', '$HOME'), /unsafe characters/); + })) passed++; else failed++; + + if (test('rejects args with ampersand', () => { + assert.throws(() => pm.getExecCommand('prettier', '--write . && echo pwned'), /unsafe characters/); + })) passed++; else failed++; + + if (test('allows safe args like --write .', () => { + const cmd = pm.getExecCommand('prettier', '--write .'); + assert.ok(cmd.includes('--write .'), 'Should include safe args'); + })) passed++; else failed++; + + if (test('allows empty args without trailing space', () => { + const cmd = pm.getExecCommand('prettier', ''); + assert.ok(!cmd.endsWith(' '), 'Should not have trailing space for empty args'); + })) passed++; else failed++; + + // ─── Round 21: getCommandPattern regex escaping ─── + console.log('\ngetCommandPattern (regex escaping):'); + + if (test('escapes dot in action name for regex safety', () => { + const pattern = pm.getCommandPattern('test.all'); + // The dot should be escaped to \\. in the pattern + const regex = new RegExp(pattern); + assert.ok(regex.test('npm run test.all'), 'Should match literal dot'); + assert.ok(!regex.test('npm run testXall'), 'Should NOT match arbitrary character in place of dot'); + })) passed++; else failed++; + + if (test('escapes brackets in action name', () => { + const pattern = pm.getCommandPattern('build[prod]'); + const regex = new RegExp(pattern); + assert.ok(regex.test('npm run build[prod]'), 'Should match literal brackets'); + })) passed++; else failed++; + + if (test('escapes parentheses in action name', () => { + // Should not throw when compiled as regex + const pattern = pm.getCommandPattern('foo(bar)'); + assert.doesNotThrow(() => new RegExp(pattern), 'Should produce valid regex with escaped parens'); + })) passed++; else failed++; + // Summary console.log('\n=== Test Results ==='); console.log(`Passed: ${passed}`);