diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cda274df..a89fc360 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -53,7 +53,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: latest + node-version: lts/* - uses: salesforcecli/github-workflows/.github/actions/yarnInstallWithRetries@main - run: yarn build - name: Remove package managers diff --git a/src/spawn.ts b/src/spawn.ts index 97d3e926..7d462d69 100644 --- a/src/spawn.ts +++ b/src/spawn.ts @@ -19,10 +19,14 @@ const debug = makeDebug('@oclif/plugin-plugins:spawn') export async function spawn(modulePath: string, args: string[] = [], {cwd, logLevel}: ExecOptions): Promise { return new Promise((resolve, reject) => { + let argv0: string | undefined if (modulePath.endsWith('.js')) { const quote = process.platform === 'win32' ? `"${modulePath}"` : modulePath args.unshift(quote) modulePath = process.execPath + if (process.platform === 'win32' && modulePath.includes(' ')) { + argv0 = `"${modulePath}"` + } } debug('modulePath', modulePath) @@ -37,6 +41,7 @@ export async function spawn(modulePath: string, args: string[] = [], {cwd, logLe }, stdio: 'pipe', windowsVerbatimArguments: true, + ...(argv0 && {argv0}), ...(process.platform === 'win32' && modulePath.toLowerCase().endsWith('.cmd') && {shell: true}), }) diff --git a/test/spawn.test.ts b/test/spawn.test.ts index 39eb6cff..97999d43 100644 --- a/test/spawn.test.ts +++ b/test/spawn.test.ts @@ -1,7 +1,8 @@ import {expect} from 'chai' -import {chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync} from 'node:fs' +import {chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync} from 'node:fs' import {tmpdir} from 'node:os' -import {join} from 'node:path' +import {dirname, join} from 'node:path' +import {fileURLToPath} from 'node:url' import {spawn} from '../src/spawn.js' @@ -48,7 +49,51 @@ describe('spawn', () => { expect(result.stdout).to.include('spaces-ok') }) - it('should not modify non-.js module paths', async () => { + it('should handle process.execPath containing spaces', async function () { + if (process.platform === 'win32') return this.skip() + const nodeDir = join(tempDir, 'path with spaces', 'bin') + mkdirSync(nodeDir, {recursive: true}) + const nodeLink = join(nodeDir, 'node') + symlinkSync(process.execPath, nodeLink) + + const script = join(tempDir, 'exec-path-test.js') + writeFileSync(script, 'console.log("execpath-ok")\n') + chmodSync(script, '755') + + const originalExecPath = process.execPath + try { + Object.defineProperty(process, 'execPath', {configurable: true, value: nodeLink, writable: true}) + const result = await spawn(script, [], {cwd: tempDir, logLevel: 'silent'}) + expect(result.stdout).to.include('execpath-ok') + } finally { + Object.defineProperty(process, 'execPath', {configurable: true, value: originalExecPath, writable: true}) + } + }) + + it('must use windowsVerbatimArguments to prevent argument injection (security)', () => { + const spawnSrc = readFileSync(join(dirname(fileURLToPath(import.meta.url)), '..', 'src', 'spawn.ts'), 'utf8') + expect(spawnSrc).to.include('windowsVerbatimArguments: true') + }) + + it('should not interpret shell metacharacters in arguments', async () => { + const script = join(tempDir, 'echo-args.js') + writeFileSync(script, 'console.log(JSON.stringify(process.argv.slice(2)))\n') + chmodSync(script, '755') + + const result = await spawn(script, ['$(whoami)', '`whoami`', '%PATH%', '|calc.exe'], { + cwd: tempDir, + logLevel: 'silent', + }) + const output = result.stdout.join(' ') + + expect(output).to.include('$(whoami)') + expect(output).to.include('`whoami`') + expect(output).to.include('%PATH%') + expect(output).to.include('|calc.exe') + }) + + it('should not modify non-.js module paths', async function () { + if (process.platform === 'win32') return this.skip() const script = join(tempDir, 'test-bin') writeFileSync(script, `#!/usr/bin/env bash\necho "bin-ok"\n`) chmodSync(script, '755')