Skip to content

Commit 908da5e

Browse files
author
Deepak Kasu
committed
add changes for fixing node exec
1 parent 87a78ee commit 908da5e

File tree

8 files changed

+67
-17
lines changed

8 files changed

+67
-17
lines changed

src/lib/util.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ import argvSplit from 'argv-split';
22
import fs from 'fs-extra';
33
import E from './errors.js';
44
import path from 'path';
5+
import os from 'os';
56
import semver from 'semver';
67
import which from 'which';
8+
import child_process from 'child_process';
79
import { fileURLToPath } from 'url';
810
import { packageDirectorySync } from 'pkg-dir';
11+
import { execPath } from 'process';
912

1013
const __dirname = path.dirname(fileURLToPath(import.meta.url));
1114

@@ -264,3 +267,27 @@ export function wrap(str, width, indent) {
264267
})
265268
.join('\n');
266269
}
270+
271+
// cache to avoid extra lookups
272+
let _nodePath;
273+
export function nodePath() {
274+
if (!_nodePath) {
275+
const execPath = process.execPath;
276+
// cannot exec cmd on windows on new versions of node https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2
277+
// CVE-2024-27980. Can't pass shell: true to get around this on windows since it breaks non-shell executions.
278+
// Can't imagine node would be a bat but who knows. It's .cmd on windows often.
279+
if (os.platform() === 'win32' && [ 'cmd', 'bat' ].includes(path.extname(execPath))) {
280+
// try and see if the node.exe lives in the same dir
281+
const newNodePath = execPath.replace(new RegExp(`${path.extname(execPath)}$`), 'exe');
282+
try {
283+
fs.statSync(newNodePath);
284+
_nodePath = newNodePath;
285+
} catch (err) {
286+
_nodePath = 'node.exe';
287+
}
288+
} else {
289+
_nodePath = execPath;
290+
}
291+
}
292+
return _nodePath;
293+
}

src/parser/extension.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import E from '../lib/errors.js';
44
import helpCommand from '../commands/help.js';
55
import _path from 'path';
66

7-
import { declareCLIKitClass, filename, findPackage, isExecutable } from '../lib/util.js';
7+
import { declareCLIKitClass, filename, findPackage, isExecutable, nodePath } from '../lib/util.js';
88
import { spawn } from 'child_process';
99

1010
const { log, warn } = debug('cli-kit:extension');
@@ -34,6 +34,7 @@ export default class Extension {
3434
* @access public
3535
*/
3636
constructor(pathOrParams, params) {
37+
log({pathOrParams, params});
3738
let path = pathOrParams;
3839

3940
if (typeof path === 'string' && !params) {
@@ -83,7 +84,7 @@ export default class Extension {
8384

8485
// spawn the process
8586
log(`Running: ${highlight(`${bin} ${args.join(' ')}`)}`);
86-
const child = spawn(bin, args, { windowsHide: true, shell: true });
87+
const child = spawn(bin, args, { windowsHide: true });
8788
child.stdout.on('data', data => terminal.stdout.write(data.toString()));
8889
child.stderr.on('data', data => terminal.stderr.write(data.toString()));
8990
await new Promise(resolve => child.on('close', (code = 0) => resolve({ code })));
@@ -114,7 +115,7 @@ export default class Extension {
114115
const makeDefaultAction = main => {
115116
return async ({ __argv, cmd }) => {
116117
process.argv = [
117-
process.execPath,
118+
nodePath(),
118119
main
119120
];
120121

@@ -239,6 +240,8 @@ export default class Extension {
239240
*/
240241
registerExtension(name, meta, params) {
241242
log(`Registering extension command: ${highlight(`${this.name}:${name}`)}`);
243+
log(meta);
244+
log(params);
242245
const cmd = new Command(name, {
243246
parent: this,
244247
...params
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import CLI from '../../../src/index.js';
2+
import { nodePath } from '../../../src/lib/util.js';
23

34
new CLI({
4-
extensions: [ 'node' ]
5+
extensions: [ nodePath() ]
56
}).exec();

test/examples/run-node/run.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import CLI from '../../../src/index.js';
2+
import { nodePath } from '../../../src/lib/util.js';
23

34
new CLI({
45
extensions: {
5-
run: `"${process.execPath}" -e`
6+
run: `"${nodePath()}" -e`
67
}
78
}).exec();

test/test-argument.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ describe('Argument', () => {
5454
name: 'foo',
5555
type: 'bar'
5656
});
57+
throw new Error('Expected error');
5758
} catch (err) {
5859
expect(err).to.be.instanceof(Error);
5960
expect(err.message).to.equal('Unsupported type "bar"');

test/test-extension.js

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import CLI, { ansi, Extension, Terminal } from '../src/index.js';
44
import path from 'path';
55
import { expect } from 'chai';
66
import { fileURLToPath } from 'url';
7+
import { platform } from 'os';
78
import { spawnSync } from 'child_process';
89
import { WritableStream } from 'memory-streams';
10+
import { nodePath } from '../src/lib/util.js';
911

1012
const __dirname = path.dirname(fileURLToPath(import.meta.url));
1113

@@ -57,12 +59,17 @@ describe('Extension', () => {
5759
delete env.SNOOPLOGG;
5860
const args = [
5961
path.join(__dirname, 'examples', 'external-binary', 'extbin.js'),
62+
// this is the command name!
6063
'node',
6164
'-e',
62-
'\'console.log(\'foo\');\''
65+
'console.log(\'foo\');'
6366
];
64-
const cmd = `${process.execPath} ${args.join(' ')}`;
65-
const {status} = spawnSync(process.execPath, args, { env, shell: true });
67+
68+
const { status, stdout, stderr } = spawnSync(nodePath(), args, {
69+
env
70+
});
71+
expect(stdout.toString().trim()).to.equal('foo');
72+
expect(stderr.toString().trim()).to.equal('');
6673
expect(status).to.equal(0);
6774
});
6875

@@ -73,9 +80,11 @@ describe('Extension', () => {
7380
const env = { ...process.env };
7481
delete env.SNOOPLOGG;
7582

76-
const { status, stdout, stderr } = spawnSync(process.execPath, [
83+
const { status, stdout, stderr } = spawnSync(nodePath(), [
7784
path.join(__dirname, 'examples', 'run-node', 'run.js'), 'run', 'console.log(\'It works\')'
78-
], { env });
85+
], {
86+
env
87+
});
7988
expect(status).to.equal(0);
8089
expect(stdout.toString().trim() + stderr.toString().trim()).to.match(/It works/m);
8190
});
@@ -86,7 +95,7 @@ describe('Extension', () => {
8695
const cli = new CLI({
8796
colors: false,
8897
extensions: {
89-
echo: 'node -e \'console.log("hi " + process.argv.slice(1).join(" "))\''
98+
echo: nodePath() + ' -e \'console.log("hi " + process.argv.slice(1).join(" "))\''
9099
},
91100
help: true,
92101
name: 'test-cli',
@@ -166,9 +175,11 @@ describe('Extension', () => {
166175
const env = { ...process.env };
167176
delete env.SNOOPLOGG;
168177

169-
const { status, stdout, stderr } = spawnSync(process.execPath, [
178+
const { status, stdout, stderr } = spawnSync(nodePath(), [
170179
path.join(__dirname, 'examples', 'external-js-file', 'extjsfile.js'), 'simple', 'foo', 'bar'
171-
], { env, shell: true });
180+
], {
181+
env
182+
});
172183
expect(stdout.toString().trim() + stderr.toString().trim()).to.equal(`${process.version} foo bar`);
173184
expect(status).to.equal(0);
174185
});
@@ -180,9 +191,11 @@ describe('Extension', () => {
180191
const env = { ...process.env };
181192
delete env.SNOOPLOGG;
182193

183-
const { status, stdout, stderr } = spawnSync(process.execPath, [
194+
const { status, stdout, stderr } = spawnSync(nodePath(), [
184195
path.join(__dirname, 'examples', 'external-module', 'extmod.js'), 'foo', 'bar'
185-
], { env, shell: true });
196+
], {
197+
env
198+
});
186199
expect(stdout.toString().trim() + stderr.toString().trim()).to.equal(`${process.version} bar`);
187200
expect(status).to.equal(0);
188201
});

test/test-parser.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { expect } from 'chai';
44
import { fileURLToPath } from 'url';
55
import { spawnSync } from 'child_process';
66
import { WritableStream } from 'memory-streams';
7+
import { nodePath } from '../src/lib/util.js';
78

89
const __dirname = path.dirname(fileURLToPath(import.meta.url));
910

@@ -86,7 +87,9 @@ describe('Parser', () => {
8687
const env = Object.assign({}, process.env);
8788
delete env.SNOOPLOGG;
8889

89-
const { status, stdout } = spawnSync(process.execPath, [ path.join(__dirname, 'examples', 'version-test', 'ver.js'), '--version' ], { env });
90+
const { status, stdout } = spawnSync(nodePath(), [ path.join(__dirname, 'examples', 'version-test', 'ver.js'), '--version' ], {
91+
env
92+
});
9093
expect(status).to.equal(0);
9194
expect(stdout.toString()).to.equal('1.2.3\n');
9295
});

test/test-util.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ describe('util', () => {
1010
it('should throw error if package.json has syntax error', () => {
1111
const dir = path.resolve(__dirname, 'fixtures', 'bad-pkg-json');
1212
try {
13-
const result = findPackage(dir);
13+
findPackage(dir);
14+
throw new Error('Expected error');
1415
} catch (err) {
1516
expect(err.message).to.match(/Failed to parse package.json:/);
1617
}

0 commit comments

Comments
 (0)