From 9c9b8df4ed578f548d222645d023d730e5e9fa88 Mon Sep 17 00:00:00 2001
From: Gonzalo Riestra <gonzalo.riestra@shopify.com>
Date: Wed, 25 Sep 2024 12:32:56 +0200
Subject: [PATCH 1/3] Avoid errors on checkCommandSafety when the command is
not found
---
packages/cli-kit/src/public/node/system.ts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts
index 4b13d273ee3..f4d3e630e0c 100644
--- a/packages/cli-kit/src/public/node/system.ts
+++ b/packages/cli-kit/src/public/node/system.ts
@@ -121,8 +121,8 @@ Running system process:
}
function checkCommandSafety(command: string) {
- const commandDirectory = dirname(which.sync(command))
- if (commandDirectory === cwd()) {
+ const commandPath = which.sync(command, {nothrow: true})
+ if (commandPath && dirname(commandPath) === cwd()) {
const headline = ['Skipped run of unsecure binary', {command}, 'found in the current directory.']
const body = 'Please remove that file or review your current PATH.'
renderWarning({headline, body})
From 07cd7c1b1388f34c914561539c8b6db7b19a4ba3 Mon Sep 17 00:00:00 2001
From: Ariel Caplan <ariel.caplan@shopify.com>
Date: Wed, 25 Sep 2024 14:49:27 +0300
Subject: [PATCH 2/3] Fix check to enforce everything
---
packages/cli-kit/src/public/node/system.ts | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts
index f4d3e630e0c..32ba4f34697 100644
--- a/packages/cli-kit/src/public/node/system.ts
+++ b/packages/cli-kit/src/public/node/system.ts
@@ -7,6 +7,7 @@ import {renderWarning} from './ui.js'
import {shouldDisplayColors, outputDebug} from '../../public/node/output.js'
import {execa, ExecaChildProcess} from 'execa'
import which from 'which'
+import {delimiter} from 'pathe'
import type {Writable, Readable} from 'stream'
export interface ExecOptions {
@@ -99,10 +100,11 @@ function buildExec(command: string, args: string[], options?: ExecOptions): Exec
if (shouldDisplayColors()) {
env.FORCE_COLOR = '1'
}
- checkCommandSafety(command)
+ const executionCwd = options?.cwd ?? cwd()
+ checkCommandSafety(command, {cwd: executionCwd})
const commandProcess = execa(command, args, {
env,
- cwd: options?.cwd,
+ cwd: executionCwd,
input: options?.input,
stdio: options?.stdio,
stdin: options?.stdin,
@@ -115,14 +117,18 @@ function buildExec(command: string, args: string[], options?: ExecOptions): Exec
outputDebug(`
Running system process:
· Command: ${command} ${args.join(' ')}
- · Working directory: ${options?.cwd ?? cwd()}
+ · Working directory: ${executionCwd}
`)
return commandProcess
}
-function checkCommandSafety(command: string) {
- const commandPath = which.sync(command, {nothrow: true})
- if (commandPath && dirname(commandPath) === cwd()) {
+function checkCommandSafety(command: string, _options: {cwd: string}): void {
+ const pathIncludingLocal = `${_options.cwd}${delimiter}${process.env.PATH}`
+ const commandPath = which.sync(command, {
+ nothrow: true,
+ path: pathIncludingLocal,
+ })
+ if (commandPath && dirname(commandPath) === _options.cwd) {
const headline = ['Skipped run of unsecure binary', {command}, 'found in the current directory.']
const body = 'Please remove that file or review your current PATH.'
renderWarning({headline, body})
From a84792f30c81dedd2ff0c6444bb37d8cc6f741e9 Mon Sep 17 00:00:00 2001
From: Gonzalo Riestra <gonzalo.riestra@shopify.com>
Date: Wed, 25 Sep 2024 17:32:11 +0200
Subject: [PATCH 3/3] Add tests
---
.../cli-kit/src/public/node/system.test.ts | 32 +++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 packages/cli-kit/src/public/node/system.test.ts
diff --git a/packages/cli-kit/src/public/node/system.test.ts b/packages/cli-kit/src/public/node/system.test.ts
new file mode 100644
index 00000000000..2c266263ec8
--- /dev/null
+++ b/packages/cli-kit/src/public/node/system.test.ts
@@ -0,0 +1,32 @@
+import * as system from './system.js'
+import {execa} from 'execa'
+import {describe, expect, test, vi} from 'vitest'
+import which from 'which'
+
+vi.mock('which')
+vi.mock('execa')
+
+describe('captureOutput', () => {
+ test('runs the command when it is not found in the current directory', async () => {
+ // Given
+ vi.mocked(which.sync).mockReturnValueOnce('/system/command')
+ vi.mocked(execa).mockResolvedValueOnce({stdout: undefined} as any)
+
+ // When
+ const got = await system.captureOutput('command', [], {cwd: '/currentDirectory'})
+
+ // Then
+ expect(got).toEqual(undefined)
+ })
+
+ test('raises an error if the command to run is found in the current directory', async () => {
+ // Given
+ vi.mocked(which.sync).mockReturnValueOnce('/currentDirectory/command')
+
+ // When
+ const got = system.captureOutput('command', [], {cwd: '/currentDirectory'})
+
+ // Then
+ await expect(got).rejects.toThrowError('Skipped run of unsecure binary command found in the current directory.')
+ })
+})