Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d69174a

Browse filesBrowse files
author
Akos Kitta
committed
fix: removed unsafe shell when executing process
Ref: PNX-3671 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
1 parent 117b2a4 commit d69174a
Copy full SHA for d69174a

11 files changed

+209
-183
lines changed

‎arduino-ide-extension/package.json

Copy file name to clipboardExpand all lines: arduino-ide-extension/package.json
-2Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,9 @@
110110
"devDependencies": {
111111
"@octokit/rest": "^18.12.0",
112112
"@types/chai": "^4.2.7",
113-
"@types/chai-string": "^1.4.2",
114113
"@types/mocha": "^5.2.7",
115114
"@types/react-window": "^1.8.5",
116115
"chai": "^4.2.0",
117-
"chai-string": "^1.5.0",
118116
"decompress": "^4.2.0",
119117
"decompress-tarbz2": "^4.1.1",
120118
"decompress-targz": "^4.1.1",

‎arduino-ide-extension/src/common/protocol/executable-service.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/common/protocol/executable-service.ts
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,5 @@ export interface ExecutableService {
55
clangdUri: string;
66
cliUri: string;
77
lsUri: string;
8-
fwuploaderUri: string;
98
}>;
109
}

‎arduino-ide-extension/src/node/arduino-daemon-impl.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/node/arduino-daemon-impl.ts
+6-13Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ export class ArduinoDaemonImpl
4444

4545
private _running = false;
4646
private _port = new Deferred<string>();
47-
private _execPath: string | undefined;
4847

4948
// Backend application lifecycle.
5049

@@ -68,7 +67,7 @@ export class ArduinoDaemonImpl
6867
async start(): Promise<string> {
6968
try {
7069
this.toDispose.dispose(); // This will `kill` the previously started daemon process, if any.
71-
const cliPath = await this.getExecPath();
70+
const cliPath = this.getExecPath();
7271
this.onData(`Starting daemon from ${cliPath}...`);
7372
const { daemon, port } = await this.spawnDaemonProcess();
7473
// Watchdog process for terminating the daemon process when the backend app terminates.
@@ -132,12 +131,8 @@ export class ArduinoDaemonImpl
132131
return this.onDaemonStoppedEmitter.event;
133132
}
134133

135-
async getExecPath(): Promise<string> {
136-
if (this._execPath) {
137-
return this._execPath;
138-
}
139-
this._execPath = await getExecPath('arduino-cli', this.onError.bind(this));
140-
return this._execPath;
134+
getExecPath(): string {
135+
return getExecPath('arduino-cli');
141136
}
142137

143138
protected async getSpawnArgs(): Promise<string[]> {
@@ -151,7 +146,7 @@ export class ArduinoDaemonImpl
151146
'--port',
152147
'0',
153148
'--config-file',
154-
`"${cliConfigPath}"`,
149+
cliConfigPath,
155150
'-v',
156151
];
157152
if (debug) {
@@ -173,10 +168,8 @@ export class ArduinoDaemonImpl
173168
daemon: ChildProcess;
174169
port: string;
175170
}> {
176-
const [cliPath, args] = await Promise.all([
177-
this.getExecPath(),
178-
this.getSpawnArgs(),
179-
]);
171+
const args = await this.getSpawnArgs();
172+
const cliPath = this.getExecPath();
180173
const ready = new Deferred<{ daemon: ChildProcess; port: string }>();
181174
const options = { shell: true };
182175
const daemon = spawn(`"${cliPath}"`, args, options);

‎arduino-ide-extension/src/node/arduino-firmware-uploader-impl.ts

Copy file name to clipboard
+17-34Lines changed: 17 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,22 @@
1+
import { ILogger } from '@theia/core/lib/common/logger';
2+
import { inject, injectable, named } from '@theia/core/shared/inversify';
3+
import type { Port } from '../common/protocol';
14
import {
25
ArduinoFirmwareUploader,
36
FirmwareInfo,
47
} from '../common/protocol/arduino-firmware-uploader';
5-
import { injectable, inject, named } from '@theia/core/shared/inversify';
6-
import { ExecutableService, Port } from '../common/protocol';
78
import { getExecPath, spawnCommand } from './exec-util';
8-
import { ILogger } from '@theia/core/lib/common/logger';
99
import { MonitorManager } from './monitor-manager';
1010

1111
@injectable()
1212
export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
13-
@inject(ExecutableService)
14-
protected executableService: ExecutableService;
15-
16-
protected _execPath: string | undefined;
17-
1813
@inject(ILogger)
1914
@named('fwuploader')
20-
protected readonly logger: ILogger;
21-
15+
private readonly logger: ILogger;
2216
@inject(MonitorManager)
23-
protected readonly monitorManager: MonitorManager;
24-
25-
protected onError(error: any): void {
26-
this.logger.error(error);
27-
}
28-
29-
async getExecPath(): Promise<string> {
30-
if (this._execPath) {
31-
return this._execPath;
32-
}
33-
this._execPath = await getExecPath('arduino-fwuploader');
34-
return this._execPath;
35-
}
36-
37-
async runCommand(args: string[]): Promise<any> {
38-
const execPath = await this.getExecPath();
39-
return await spawnCommand(`"${execPath}"`, args, this.onError.bind(this));
40-
}
17+
private readonly monitorManager: MonitorManager;
4118

42-
async uploadCertificates(command: string): Promise<any> {
19+
async uploadCertificates(command: string): Promise<string> {
4320
return await this.runCommand(['certificates', 'flash', command]);
4421
}
4522

@@ -70,14 +47,13 @@ export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
7047
}
7148

7249
async flash(firmware: FirmwareInfo, port: Port): Promise<string> {
73-
let output;
7450
const board = {
7551
name: firmware.board_name,
7652
fqbn: firmware.board_fqbn,
7753
};
7854
try {
7955
await this.monitorManager.notifyUploadStarted(board.fqbn, port);
80-
output = await this.runCommand([
56+
const output = await this.runCommand([
8157
'firmware',
8258
'flash',
8359
'--fqbn',
@@ -87,11 +63,18 @@ export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
8763
'--module',
8864
`${firmware.module}@${firmware.firmware_version}`,
8965
]);
90-
} catch (e) {
91-
throw e;
66+
return output;
9267
} finally {
9368
await this.monitorManager.notifyUploadFinished(board.fqbn, port);
94-
return output;
9569
}
9670
}
71+
72+
private onError(error: Error): void {
73+
this.logger.error(error);
74+
}
75+
76+
private async runCommand(args: string[]): Promise<string> {
77+
const execPath = getExecPath('arduino-fwuploader');
78+
return await spawnCommand(execPath, args, this.onError.bind(this));
79+
}
9780
}

‎arduino-ide-extension/src/node/clang-formatter.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/node/clang-formatter.ts
+2-7Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,16 @@ export class ClangFormatter implements Formatter {
3131
this.style(formatterConfigFolderUris, options),
3232
]);
3333
const formatted = await spawnCommand(
34-
`"${execPath}"`,
34+
execPath,
3535
[style],
3636
console.error,
3737
content
3838
);
3939
return formatted;
4040
}
4141

42-
private _execPath: string | undefined;
4342
private async execPath(): Promise<string> {
44-
if (this._execPath) {
45-
return this._execPath;
46-
}
47-
this._execPath = await getExecPath('clang-format');
48-
return this._execPath;
43+
return getExecPath('clang-format');
4944
}
5045

5146
/**

‎arduino-ide-extension/src/node/config-service-impl.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/node/config-service-impl.ts
+4-9Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ export class ConfigServiceImpl
222222
}
223223

224224
private async getFallbackCliConfig(): Promise<DefaultCliConfig> {
225-
const cliPath = await this.daemon.getExecPath();
226-
const rawJson = await spawnCommand(`"${cliPath}"`, [
225+
const cliPath = this.daemon.getExecPath();
226+
const rawJson = await spawnCommand(cliPath, [
227227
'config',
228228
'dump',
229229
'format',
@@ -233,13 +233,8 @@ export class ConfigServiceImpl
233233
}
234234

235235
private async initCliConfigTo(fsPathToDir: string): Promise<void> {
236-
const cliPath = await this.daemon.getExecPath();
237-
await spawnCommand(`"${cliPath}"`, [
238-
'config',
239-
'init',
240-
'--dest-dir',
241-
`"${fsPathToDir}"`,
242-
]);
236+
const cliPath = this.daemon.getExecPath();
237+
await spawnCommand(cliPath, ['config', 'init', '--dest-dir', fsPathToDir]);
243238
}
244239

245240
private async mapCliConfigToAppConfig(

‎arduino-ide-extension/src/node/exec-util.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/node/exec-util.ts
+12-46Lines changed: 12 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,17 @@
1+
import { spawn } from 'node:child_process';
12
import os from 'node:os';
2-
import which from 'which';
3-
import semver from 'semver';
43
import { join } from 'node:path';
5-
import { spawn } from 'node:child_process';
64

7-
export async function getExecPath(
8-
commandName: string,
9-
onError: (error: Error) => void = (error) => console.log(error),
10-
versionArg?: string | undefined,
11-
inBinDir?: boolean
12-
): Promise<string> {
13-
const execName = `${commandName}${os.platform() === 'win32' ? '.exe' : ''}`;
14-
const relativePath = ['..', '..', 'build'];
15-
if (inBinDir) {
16-
relativePath.push('bin');
17-
}
18-
const buildCommand = join(__dirname, ...relativePath, execName);
19-
if (!versionArg) {
20-
return buildCommand;
21-
}
22-
const versionRegexp = /\d+\.\d+\.\d+/;
23-
const buildVersion = await spawnCommand(
24-
`"${buildCommand}"`,
25-
[versionArg],
26-
onError
27-
);
28-
const buildShortVersion = (buildVersion.match(versionRegexp) || [])[0];
29-
const pathCommand = await new Promise<string | undefined>((resolve) =>
30-
which(execName, (error, path) => resolve(error ? undefined : path))
31-
);
32-
if (!pathCommand) {
33-
return buildCommand;
34-
}
35-
const pathVersion = await spawnCommand(
36-
`"${pathCommand}"`,
37-
[versionArg],
38-
onError
39-
);
40-
const pathShortVersion = (pathVersion.match(versionRegexp) || [])[0];
41-
if (
42-
pathShortVersion &&
43-
buildShortVersion &&
44-
semver.gt(pathShortVersion, buildShortVersion)
45-
) {
46-
return pathCommand;
47-
}
48-
return buildCommand;
5+
export type ArduinoBinaryName =
6+
| 'arduino-cli'
7+
| 'arduino-fwuploader'
8+
| 'arduino-language-server';
9+
export type ClangBinaryName = 'clangd' | 'clang-format';
10+
export type BinaryName = ArduinoBinaryName | ClangBinaryName;
11+
12+
export function getExecPath(binaryName: BinaryName): string {
13+
const filename = `${binaryName}${os.platform() === 'win32' ? '.exe' : ''}`;
14+
return join(__dirname, '..', '..', 'build', filename);
4915
}
5016

5117
export function spawnCommand(
@@ -55,7 +21,7 @@ export function spawnCommand(
5521
stdIn?: string
5622
): Promise<string> {
5723
return new Promise<string>((resolve, reject) => {
58-
const cp = spawn(command, args, { windowsHide: true, shell: true });
24+
const cp = spawn(command, args, { windowsHide: true });
5925
const outBuffers: Buffer[] = [];
6026
const errBuffers: Buffer[] = [];
6127
cp.stdout.on('data', (b: Buffer) => outBuffers.push(b));

‎arduino-ide-extension/src/node/executable-service-impl.ts

Copy file name to clipboard
+5-21Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,19 @@
1-
import { injectable, inject } from '@theia/core/shared/inversify';
2-
import { ILogger } from '@theia/core/lib/common/logger';
31
import { FileUri } from '@theia/core/lib/node/file-uri';
4-
import { getExecPath } from './exec-util';
2+
import { injectable } from '@theia/core/shared/inversify';
53
import { ExecutableService } from '../common/protocol/executable-service';
4+
import { getExecPath } from './exec-util';
65

76
@injectable()
87
export class ExecutableServiceImpl implements ExecutableService {
9-
@inject(ILogger)
10-
protected logger: ILogger;
11-
128
async list(): Promise<{
139
clangdUri: string;
1410
cliUri: string;
1511
lsUri: string;
16-
fwuploaderUri: string;
1712
}> {
18-
const [ls, clangd, cli, fwuploader] = await Promise.all([
19-
getExecPath('arduino-language-server', this.onError.bind(this)),
20-
getExecPath('clangd', this.onError.bind(this), undefined),
21-
getExecPath('arduino-cli', this.onError.bind(this)),
22-
getExecPath('arduino-fwuploader', this.onError.bind(this)),
23-
]);
2413
return {
25-
clangdUri: FileUri.create(clangd).toString(),
26-
cliUri: FileUri.create(cli).toString(),
27-
lsUri: FileUri.create(ls).toString(),
28-
fwuploaderUri: FileUri.create(fwuploader).toString(),
14+
clangdUri: FileUri.create(getExecPath('clangd')).toString(),
15+
cliUri: FileUri.create(getExecPath('arduino-cli')).toString(),
16+
lsUri: FileUri.create(getExecPath('arduino-language-server')).toString(),
2917
};
3018
}
31-
32-
protected onError(error: Error): void {
33-
this.logger.error(error);
34-
}
3519
}

‎arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts
+3-9Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,13 @@ class SilentArduinoDaemonImpl extends ArduinoDaemonImpl {
4343
}
4444

4545
private async initCliConfig(): Promise<string> {
46-
const cliPath = await this.getExecPath();
46+
const cliPath = this.getExecPath();
4747
const destDir = track.mkdirSync();
48-
await spawnCommand(`"${cliPath}"`, [
49-
'config',
50-
'init',
51-
'--dest-dir',
52-
destDir,
53-
]);
48+
await spawnCommand(cliPath, ['config', 'init', '--dest-dir', destDir]);
5449
const content = fs.readFileSync(path.join(destDir, CLI_CONFIG), {
5550
encoding: 'utf8',
5651
});
57-
const cliConfig = safeLoad(content) as any;
58-
// cliConfig.daemon.port = String(this.port);
52+
const cliConfig = safeLoad(content);
5953
const modifiedContent = safeDump(cliConfig);
6054
fs.writeFileSync(path.join(destDir, CLI_CONFIG), modifiedContent, {
6155
encoding: 'utf8',

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.