From 0a3afbe38fd67801ef56e16b31e1a7b4815352c7 Mon Sep 17 00:00:00 2001
From: Jonghyeok Park <jonghyeok.park@unitblack.co.kr>
Date: Tue, 10 Mar 2026 22:37:57 +0900
Subject: [PATCH] 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.
---
 scripts/hooks/post-edit-format.js | 33 +++++++++++++++++++++++++------
 scripts/hooks/run-with-flags.js   |  2 +-
 scripts/lib/resolve-formatter.js  |  2 +-
 tests/hooks/hooks.test.js         |  6 ++++--
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/scripts/hooks/post-edit-format.js b/scripts/hooks/post-edit-format.js
index 7e098132..d6486866 100644
--- a/scripts/hooks/post-edit-format.js
+++ b/scripts/hooks/post-edit-format.js
@@ -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
       }
diff --git a/scripts/hooks/run-with-flags.js b/scripts/hooks/run-with-flags.js
index d69e8131..b665fe28 100755
--- a/scripts/hooks/run-with-flags.js
+++ b/scripts/hooks/run-with-flags.js
@@ -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);
diff --git a/scripts/lib/resolve-formatter.js b/scripts/lib/resolve-formatter.js
index f940a1fa..22d04819 100644
--- a/scripts/lib/resolve-formatter.js
+++ b/scripts/lib/resolve-formatter.js
@@ -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';
       }
diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js
index 81766ea2..311ae21f 100644
--- a/tests/hooks/hooks.test.js
+++ b/tests/hooks/hooks.test.js
@@ -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');