Skip to content

Commit

Permalink
Merge pull request #4522 from Shopify/fix-not-found-in-check-command-…
Browse files Browse the repository at this point in the history
…safety

Fix dev for Ruby apps
  • Loading branch information
gonzaloriestra authored Dec 4, 2024
2 parents 4721974 + f559cb0 commit 084f555
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 6 deletions.
32 changes: 32 additions & 0 deletions packages/cli-kit/src/public/node/system.test.ts
Original file line number Diff line number Diff line change
@@ -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.')
})
})
18 changes: 12 additions & 6 deletions packages/cli-kit/src/public/node/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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})
Expand Down

0 comments on commit 084f555

Please sign in to comment.