[Security] Fix command injection risk in cloudflared installer

packages/plugin-cloudflare/src/install-cloudflared.test.ts

@@ -1,18 +1,19 @@
 import install, {CURRENT_CLOUDFLARE_VERSION, versionIsGreaterThan} from './install-cloudflared.js'
 import * as fsActions from '@shopify/cli-kit/node/fs'
 import * as http from '@shopify/cli-kit/node/http'
+import * as system from '@shopify/cli-kit/node/system'
 import {beforeEach, describe, expect, test, vi} from 'vitest'
 import util from 'util'
 
 import {WriteStream} from 'fs'
-// eslint-disable-next-line no-restricted-imports
-import * as childProcess from 'child_process'
 
-vi.mock('child_process')
+vi.mock('@shopify/cli-kit/node/system')
 vi.mock('stream')
 
 describe('install-cloudflare', () => {
   beforeEach(() => {
+    vi.mocked(system.captureOutputWithExitCode).mockResolvedValue({stdout: '', stderr: '', exitCode: 0})
+    vi.mocked(system.exec).mockResolvedValue(undefined)
     vi.spyOn(util, 'promisify').mockReturnValue(vi.fn().mockReturnValue(Promise.resolve()))
     vi.spyOn(http, 'fetch').mockReturnValue(Promise.resolve({ok: true, body: {pipe: vi.fn()}} as any))
     vi.spyOn(fsActions, 'fileExistsSync').mockReturnValueOnce(false)
@@ -47,6 +48,11 @@ describe('install-cloudflare', () => {
       expect.anything(),
       'slow-request',
     )
+    expect(system.exec).toHaveBeenCalledWith(
+      'tar',
+      ['-xzf', expect.stringMatching(/\.tgz$/)],
+      expect.objectContaining({cwd: expect.any(String)}),
+    )
   })
 
   test('install works when system is mac and arm64', async () => {
@@ -98,28 +104,68 @@ describe('install-cloudflare', () => {
     // Given
     const env = {}
     vi.spyOn(fsActions, 'fileExistsSync').mockReturnValueOnce(true)
-    vi.spyOn(childProcess, 'execFileSync').mockReturnValue(
-      `cloudflared version ${CURRENT_CLOUDFLARE_VERSION} (built 2023-03-13-1444 UTC)`,
-    )
+    vi.mocked(system.captureOutputWithExitCode).mockResolvedValue({
+      stdout: `cloudflared version ${CURRENT_CLOUDFLARE_VERSION} (built 2023-03-13-1444 UTC)`,
+      stderr: '',
+      exitCode: 0,
+    })
 
     // When
     await install(env, 'win32', 'x64')
 
     // Then
     expect(http.fetch).not.toHaveBeenCalled()
+    expect(system.captureOutputWithExitCode).toHaveBeenCalledWith(expect.any(String), ['--version'])
   })
 
   test('install works if bin exists and current version is not up to date', async () => {
     // Given
     const env = {}
     vi.spyOn(fsActions, 'fileExistsSync').mockReturnValueOnce(true)
-    vi.spyOn(childProcess, 'execFileSync').mockReturnValue(`cloudflared version 2000.0.0 (built 2023-03-13-1444 UTC)`)
+    vi.mocked(system.captureOutputWithExitCode).mockResolvedValue({
+      stdout: `cloudflared version 2000.0.0 (built 2023-03-13-1444 UTC)`,
+      stderr: '',
+      exitCode: 0,
+    })
 
     // When
     await install(env, 'darwin', 'x64')
 
     // Then
     expect(http.fetch).toHaveBeenCalled()
+    expect(system.captureOutputWithExitCode).toHaveBeenCalledWith(expect.any(String), ['--version'])
+  })
+
+  test('install reinstalls if version check throws', async () => {
+    // Given
+    const env = {}
+    vi.spyOn(fsActions, 'fileExistsSync').mockReturnValueOnce(true)
+    vi.mocked(system.captureOutputWithExitCode).mockRejectedValue(new Error('ENOENT'))
+
+    // When
+    await install(env, 'linux', 'x64')
+
+    // Then
+    expect(http.fetch).toHaveBeenCalled()
+    expect(system.captureOutputWithExitCode).toHaveBeenCalledWith(expect.any(String), ['--version'])
+  })
+
+  test('install reinstalls if version check exits with non-zero code', async () => {
+    // Given
+    const env = {}
+    vi.spyOn(fsActions, 'fileExistsSync').mockReturnValueOnce(true)
+    vi.mocked(system.captureOutputWithExitCode).mockResolvedValue({
+      stdout: `cloudflared version ${CURRENT_CLOUDFLARE_VERSION} (built 2023-03-13-1444 UTC)`,
+      stderr: 'permission denied',
+      exitCode: 1,
+    })
+
+    // When
+    await install(env, 'linux', 'x64')
+
+    // Then
+    expect(http.fetch).toHaveBeenCalled()
+    expect(system.captureOutputWithExitCode).toHaveBeenCalledWith(expect.any(String), ['--version'])
   })
 
   test('install fails if unsupported platform', async () => {

packages/plugin-cloudflare/src/install-cloudflared.ts

@@ -10,11 +10,10 @@ import {
   unlinkFileSync,
   createFileWriteStream,
 } from '@shopify/cli-kit/node/fs'
+import {captureOutputWithExitCode, exec} from '@shopify/cli-kit/node/system'
 import {fileURLToPath} from 'url'
 import util from 'util'
 import {pipeline} from 'stream'
-// eslint-disable-next-line no-restricted-imports
-import {execSync, execFileSync} from 'child_process'
 
 export const CURRENT_CLOUDFLARE_VERSION = '2024.8.2'
 const CLOUDFLARE_REPO = `https://github.com/cloudflare/cloudflared/releases/download/${CURRENT_CLOUDFLARE_VERSION}/`
@@ -82,7 +81,9 @@ export default async function install(env = process.env, platform = process.plat
   if (fileExistsSync(binTarget)) {
     // --version returns an string like "cloudflared version 2023.3.1 (built 2023-03-13-1444 UTC)"
     try {
-      const versionArray = execFileSync(binTarget, ['--version'], {encoding: 'utf8'}).split(' ')
+      const {stdout, exitCode} = await captureOutputWithExitCode(binTarget, ['--version'])
+      if (exitCode !== 0) throw new Error(`cloudflared --version exited with code ${exitCode}`)
+      const versionArray = stdout.split(' ')
       const versionNumber = versionArray.length > 2 ? versionArray[2] : '0.0.0'
       const needsUpdate = versionIsGreaterThan(CURRENT_CLOUDFLARE_VERSION, versionNumber!)
       if (!needsUpdate) {
@@ -132,7 +133,7 @@ async function installWindows(file: string, binTarget: string) {
 async function installMacos(file: string, binTarget: string) {
   await downloadFile(file, `${binTarget}.tgz`)
   const filename = basename(`${binTarget}.tgz`)
-  execSync(`tar -xzf ${filename}`, {cwd: dirname(binTarget)})
+  await exec('tar', ['-xzf', filename], {cwd: dirname(binTarget)})
   unlinkFileSync(`${binTarget}.tgz`)
   await renameFile(`${dirname(binTarget)}/cloudflared`, binTarget)
 }