Skip to content

Commit fdc3413

Browse files
authored
fix(exec): Fails to Execute Binaries Named After Shell Keywords (#8221)
<!-- What / Why --> <!-- Describe the request in detail. What it does and why it's being changed. --> The best example of this bug is in the original issue (#8190). Basically, if an exec command has a shell keyword (i.e `npx select`) it will trigger a bash syntax error. Things I did: 1. Added a test that fails to execute a binary named after a shell keyword (`select`) 2. Wrap all execution commands (after handling parsing for locations/downloads/etc) in quotes to guarantee a programs execution and not a bash parse This is my first PR on this project so please let me know if I missed something! I'm happy to make whatever changes we need to get it fixed! Thanks! ## References Fixes #8190
1 parent 4b08e2e commit fdc3413

File tree

3 files changed

+63
-1
lines changed

3 files changed

+63
-1
lines changed

test/lib/commands/exec.js

+26-1
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,31 @@ t.test('packs from git spec', async t => {
275275
const exists = await fs.stat(path.join(npm.prefix, 'npm-exec-test-success'))
276276
t.ok(exists.isFile(), 'bin ran, creating file')
277277
} catch (err) {
278-
t.fail(err, "shouldn't throw")
278+
t.fail(err, 'should not throw')
279+
}
280+
})
281+
282+
t.test('can run packages with keywords', async t => {
283+
const { npm } = await loadMockNpm(t, {
284+
prefixDir: {
285+
'package.json': JSON.stringify({
286+
name: '@npmcli/npx-package-test',
287+
bin: { select: 'index.js' },
288+
}),
289+
'index.js': `#!/usr/bin/env node
290+
require('fs').writeFileSync('npm-exec-test-success', (process.argv.length).toString())`,
291+
},
292+
})
293+
294+
try {
295+
await npm.exec('exec', ['select'])
296+
297+
const testFilePath = path.join(npm.prefix, 'npm-exec-test-success')
298+
const exists = await fs.stat(testFilePath)
299+
t.ok(exists.isFile(), 'bin ran, creating file')
300+
const noExtraArgumentCount = await fs.readFile(testFilePath, 'utf8')
301+
t.equal(+noExtraArgumentCount, 2, 'should have no extra arguments')
302+
} catch (err) {
303+
t.fail(err, 'should not throw')
279304
}
280305
})

workspaces/libnpmexec/lib/run-script.js

+9
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const runScript = require('@npmcli/run-script')
33
const readPackageJson = require('read-package-json-fast')
44
const { log, output } = require('proc-log')
55
const noTTY = require('./no-tty.js')
6+
const isWindowsShell = require('./is-windows.js')
67

78
const run = async ({
89
args,
@@ -14,6 +15,14 @@ const run = async ({
1415
runPath,
1516
scriptShell,
1617
}) => {
18+
// escape executable path
19+
// necessary for preventing bash/cmd keywords from overriding
20+
if (!isWindowsShell) {
21+
if (args.length > 0) {
22+
args[0] = '"' + args[0] + '"'
23+
}
24+
}
25+
1726
// turn list of args into command string
1827
const script = call || args.shift() || scriptShell
1928

workspaces/libnpmexec/test/run-script.js

+28
Original file line numberDiff line numberDiff line change
@@ -115,3 +115,31 @@ t.test('ci env', async t => {
115115

116116
t.equal(logs[0], 'warn exec Interactive mode disabled in CI environment')
117117
})
118+
119+
t.test('isWindows', async t => {
120+
const { runScript } = await mockRunScript(t, {
121+
'ci-info': { isCI: true },
122+
'@npmcli/run-script': async () => {
123+
t.ok('should call run-script')
124+
},
125+
'../lib/is-windows.js': true,
126+
})
127+
128+
await runScript({ args: ['test'] })
129+
// need both arguments and no arguments for code coverage
130+
await runScript()
131+
})
132+
133+
t.test('isNotWindows', async t => {
134+
const { runScript } = await mockRunScript(t, {
135+
'ci-info': { isCI: true },
136+
'@npmcli/run-script': async () => {
137+
t.ok('should call run-script')
138+
},
139+
'../lib/is-windows.js': false,
140+
})
141+
142+
await runScript({ args: ['test'] })
143+
// need both arguments and no arguments for code coverage
144+
await runScript()
145+
})

0 commit comments

Comments
 (0)