From 60b483e03a361e597d381483c1b3d5ac0ff5923d Mon Sep 17 00:00:00 2001
From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com>
Date: Fri, 8 May 2026 00:20:04 +0000
Subject: [PATCH] [Security] Fix command injection in tree-kill.ts on Windows

- Validate PID format using a strict numeric regex to prevent injection.
- Switch from `exec` to `execFile` for Windows taskkill to avoid shell evaluation.
- Add comprehensive tests for tree-kill logic.
- Perform knip maintenance on root package.json.
---
 package.json                                  |  5 +-
 .../cli-kit/src/public/node/tree-kill.test.ts | 66 +++++++++++++++++++
 packages/cli-kit/src/public/node/tree-kill.ts |  6 +-
 3 files changed, 70 insertions(+), 7 deletions(-)
 create mode 100644 packages/cli-kit/src/public/node/tree-kill.test.ts

diff --git a/package.json b/package.json
index 19baee5e6eb..a226cbded2a 100644
--- a/package.json
+++ b/package.json
@@ -199,7 +199,6 @@
           "**/graphql/**/generated/*.ts"
         ],
         "ignoreDependencies": [
-          "@graphql-typed-document-node/core",
           "@shopify/plugin-cloudflare"
         ],
         "vite": {
@@ -292,9 +291,7 @@
         "ignoreBinaries": [
           "open"
         ],
-        "ignoreDependencies": [
-          "@graphql-typed-document-node/core"
-        ],
+        "ignoreDependencies": [],
         "vite": {
           "config": [
             "vite.config.ts"
diff --git a/packages/cli-kit/src/public/node/tree-kill.test.ts b/packages/cli-kit/src/public/node/tree-kill.test.ts
new file mode 100644
index 00000000000..484995b4a99
--- /dev/null
+++ b/packages/cli-kit/src/public/node/tree-kill.test.ts
@@ -0,0 +1,66 @@
+/* eslint-disable no-restricted-imports */
+import {treeKill} from './tree-kill.js'
+import {describe, expect, test, vi, beforeEach, afterEach} from 'vitest'
+import {execFile} from 'child_process'
+
+vi.mock('child_process', () => ({
+  exec: vi.fn(),
+  execFile: vi.fn(),
+  spawn: vi.fn(() => ({
+    stdout: {on: vi.fn()},
+    on: vi.fn(),
+  })),
+}))
+
+describe('treeKill', () => {
+  const originalPlatform = process.platform
+
+  beforeEach(() => {
+    Object.defineProperty(process, 'platform', {
+      value: originalPlatform,
+    })
+  })
+
+  afterEach(() => {
+    Object.defineProperty(process, 'platform', {
+      value: originalPlatform,
+    })
+  })
+
+  test('calls taskkill on Windows with numeric PID', () => {
+    Object.defineProperty(process, 'platform', {
+      value: 'win32',
+    })
+
+    treeKill(123)
+
+    expect(execFile).toHaveBeenCalledWith('taskkill', ['/pid', '123', '/T', '/F'], expect.any(Function))
+  })
+
+  test('throws or calls callback with error for non-numeric PID', () => {
+    Object.defineProperty(process, 'platform', {
+      value: 'win32',
+    })
+
+    const callback = vi.fn()
+
+    treeKill('123 & calc', 'SIGTERM', true, callback)
+
+    expect(callback).toHaveBeenCalledWith(
+      expect.objectContaining({
+        message: 'pid must be a number',
+      }),
+    )
+    expect(execFile).not.toHaveBeenCalled()
+  })
+
+  test('handles string numeric PID', () => {
+    Object.defineProperty(process, 'platform', {
+      value: 'win32',
+    })
+
+    treeKill('123')
+
+    expect(execFile).toHaveBeenCalledWith('taskkill', ['/pid', '123', '/T', '/F'], expect.any(Function))
+  })
+})
diff --git a/packages/cli-kit/src/public/node/tree-kill.ts b/packages/cli-kit/src/public/node/tree-kill.ts
index 1bd7bdee30d..3183c1efe87 100644
--- a/packages/cli-kit/src/public/node/tree-kill.ts
+++ b/packages/cli-kit/src/public/node/tree-kill.ts
@@ -3,7 +3,7 @@
 /* eslint-disable no-restricted-imports */
 
 import {outputDebug} from './output.js'
-import {exec, spawn} from 'child_process'
+import {execFile, spawn} from 'child_process'
 
 type ProcessTree = Record<string, string[]>
 
@@ -52,7 +52,7 @@ function adaptedTreeKill(
 ): void {
   const rootPid = typeof pid === 'number' ? pid.toString() : pid
 
-  if (Number.isNaN(rootPid)) {
+  if (!/^\d+$/.test(rootPid)) {
     if (callback) {
       callback(new Error('pid must be a number'))
       return
@@ -73,7 +73,7 @@ function adaptedTreeKill(
   switch (process.platform) {
     case 'win32':
       // @ts-ignore
-      exec(`taskkill /pid ${pid} /T /F`, callback)
+      execFile('taskkill', ['/pid', rootPid, '/T', '/F'], callback)
       break
     case 'darwin':
       buildProcessTree(