fix(hooks): add Windows .cmd support with shell injection guard

Handle Windows .cmd shim resolution via spawnSync with strict path
validation. Removes shell:true injection risk, uses strict equality,
and restores .cmd support with path injection guard.
This commit is contained in:
Jonghyeok Park
2026-03-10 22:37:57 +09:00
parent 66498ae9ac
commit 0a3afbe38f
4 changed files with 33 additions and 10 deletions

View File

@@ -17,9 +17,12 @@
* Fails silently if no formatter is found or installed.
*/
const { execFileSync } = require('child_process');
const { execFileSync, spawnSync } = require('child_process');
const path = require('path');
// Shell metacharacters that cmd.exe interprets as command separators/operators
const UNSAFE_PATH_CHARS = /[&|<>^%!]/;
const { findProjectRoot, detectFormatter, resolveFormatterBin } = require('../lib/resolve-formatter');
const MAX_STDIN = 1024 * 1024; // 1MB limit
@@ -50,11 +53,29 @@ function run(rawInput) {
// Prettier: `--write` = format only
const args = formatter === 'biome' ? [...resolved.prefix, 'check', '--write', resolvedFilePath] : [...resolved.prefix, '--write', resolvedFilePath];
execFileSync(resolved.bin, args, {
cwd: projectRoot,
stdio: ['pipe', 'pipe', 'pipe'],
timeout: 15000
});
if (process.platform === 'win32' && resolved.bin.endsWith('.cmd')) {
// Windows: .cmd files require shell to execute. Guard against
// command injection by rejecting paths with shell metacharacters.
if (UNSAFE_PATH_CHARS.test(resolvedFilePath)) {
throw new Error('File path contains unsafe shell characters');
}
const result = spawnSync(resolved.bin, args, {
cwd: projectRoot,
shell: true,
stdio: 'pipe',
timeout: 15000
});
if (result.error) throw result.error;
if (typeof result.status === 'number' && result.status !== 0) {
throw new Error(result.stderr?.toString() || `Formatter exited with status ${result.status}`);
}
} else {
execFileSync(resolved.bin, args, {
cwd: projectRoot,
stdio: ['pipe', 'pipe', 'pipe'],
timeout: 15000
});
}
} catch {
// Formatter not installed, file missing, or failed — non-blocking
}

View File

@@ -90,7 +90,7 @@ async function main() {
if (hookModule && typeof hookModule.run === 'function') {
try {
const output = hookModule.run(raw);
if (output != null) process.stdout.write(output);
if (output !== null && output !== undefined) process.stdout.write(output);
} catch (runErr) {
process.stderr.write(`[Hook] run() error for ${hookId}: ${runErr.message}\n`);
process.stdout.write(raw);

View File

@@ -86,7 +86,7 @@ function detectFormatter(projectRoot) {
const pkgPath = path.join(projectRoot, 'package.json');
if (fs.existsSync(pkgPath)) {
const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8'));
if (pkg.prettier != null) {
if ('prettier' in pkg) {
formatterCache.set(projectRoot, 'prettier');
return 'prettier';
}

View File

@@ -1966,8 +1966,10 @@ async function runTests() {
const formatSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-format.js'), 'utf8');
// Strip comments to avoid matching "shell: true" in comment text
const codeOnly = formatSource.replace(/\/\/.*$/gm, '').replace(/\/\*[\s\S]*?\*\//g, '');
assert.ok(!codeOnly.includes('shell:'), 'post-edit-format.js should not pass shell option in code');
// npx.cmd handling moved to shared resolve-formatter.js — verify it uses the shared module
assert.ok(!/execFileSync\([^)]*shell\s*:/.test(codeOnly), 'post-edit-format.js should not pass shell option to execFileSync');
assert.ok(codeOnly.includes("process.platform === 'win32' && resolved.bin.endsWith('.cmd')"), 'Windows shell execution must stay gated to .cmd shims');
assert.ok(codeOnly.includes('UNSAFE_PATH_CHARS'), 'Must guard against shell metacharacters before using shell: true');
// npx.cmd handling in shared resolve-formatter.js
const resolverSource = fs.readFileSync(path.join(scriptsDir, '..', 'lib', 'resolve-formatter.js'), 'utf8');
assert.ok(resolverSource.includes('npx.cmd'), 'resolve-formatter.js should use npx.cmd for Windows cross-platform safety');
assert.ok(formatSource.includes('resolveFormatterBin'), 'post-edit-format.js should use shared resolveFormatterBin');