From 73d72944d32d1d34f2e37a2cb64b91d9973107f9 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 4 Dec 2024 10:54:03 +0100 Subject: [PATCH 1/2] Avoid errors on checkCommandSafety when the command is not found --- .../cli-kit/src/public/node/system.test.ts | 32 +++++++++++++++++++ packages/cli-kit/src/public/node/system.ts | 18 +++++++---- 2 files changed, 44 insertions(+), 6 deletions(-) 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.') + }) +}) diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index fe77bdb0600..673fba6c422 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 commandDirectory = dirname(which.sync(command)) - if (commandDirectory === 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 a395820e7b3c5cfa3479332cbb67807616db4743 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 4 Dec 2024 10:54:58 +0100 Subject: [PATCH 2/2] Add changeset --- .changeset/wise-crabs-retire.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/wise-crabs-retire.md diff --git a/.changeset/wise-crabs-retire.md b/.changeset/wise-crabs-retire.md new file mode 100644 index 00000000000..565f040c88d --- /dev/null +++ b/.changeset/wise-crabs-retire.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Fix dev for Ruby apps by avoiding the command safety check to raise an error when not found