Skip to content

Commit 083bb25

Browse files
Hristo313Hristo Hristov
Hristo313
and
Hristo Hristov
authored
resolve code scanning vulnerability alerts (#1331)
* fix(cli): resolve alerts for code scanning * fix(cli): add fix for Util code scanning problem for shell command * fix(cli): add spawnSync and fix tests * fix(cli): resolve alerts for code scanning in package manager and fix tests * fix(cli): remove getSecureRandomNumber function * fix(cli): add spawnSync function to Util --------- Co-authored-by: Hristo Hristov <[email protected]>
1 parent 277ac97 commit 083bb25

File tree

5 files changed

+135
-60
lines changed

5 files changed

+135
-60
lines changed

‎packages/core/packages/PackageManager.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { exec, spawnSync } from "child_process";
1+
import { exec } from "child_process";
22
import * as path from "path";
33
import { TemplateManager } from "../../cli/lib/TemplateManager";
44
import { Config, FS_TOKEN, IFileSystem, ProjectTemplate } from "../types";
@@ -123,18 +123,18 @@ export class PackageManager {
123123
}
124124

125125
public static removePackage(packageName: string, verbose: boolean = false): boolean {
126-
let command: string;
127126
const managerCommand = this.getManager();
127+
let args: string[];
128128
switch (managerCommand) {
129129
case "npm":
130130
/* passes through */
131131
default:
132-
command = `${managerCommand} uninstall ${packageName} --quiet --save`;
132+
args = ['uninstall', packageName, '--quiet', '--save'];
133133
break;
134134
}
135135
try {
136136
// tslint:disable-next-line:object-literal-sort-keys
137-
Util.execSync(command, { stdio: "pipe", encoding: "utf8" });
137+
Util.spawnSync(managerCommand, args, { stdio: "pipe", encoding: "utf8" });
138138
} catch (error) {
139139
Util.log(`Error uninstalling package ${packageName} with ${managerCommand}`);
140140
if (verbose) {
@@ -225,7 +225,7 @@ export class PackageManager {
225225
const fullPackageRegistry = config.igPackageRegistry;
226226
try {
227227
// tslint:disable-next-line:object-literal-sort-keys
228-
Util.execSync(`npm whoami --registry=${fullPackageRegistry}`, { stdio: "pipe", encoding: "utf8" });
228+
Util.spawnSync('npm', ['whoami', `--registry=${fullPackageRegistry}`], { stdio: 'pipe', encoding: 'utf8' });
229229
} catch (error) {
230230
// try registering the user:
231231
Util.log(
@@ -247,14 +247,14 @@ export class PackageManager {
247247
process.stdin.setRawMode(true);
248248
}
249249
const cmd = /^win/.test(process.platform) ? "npm.cmd" : "npm"; //https://github.com/nodejs/node/issues/3675
250-
const login = spawnSync(cmd,
250+
const login = Util.spawnSync(cmd,
251251
["adduser", `--registry=${fullPackageRegistry}`, `--scope=@infragistics`, `--always-auth`],
252252
{ stdio: "inherit" }
253253
);
254254
if (login?.status === 0) {
255255
//make sure scope is configured:
256256
try {
257-
Util.execSync(`npm config set @infragistics:registry ${fullPackageRegistry}`);
257+
Util.spawnSync('npm', ['config', 'set', `@infragistics:registry`, fullPackageRegistry]);
258258
return true;
259259
} catch (error) {
260260
return false;

‎packages/core/util/Util.ts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import chalk from "chalk";
2-
import { execSync, ExecSyncOptions } from "child_process";
2+
import { execSync, ExecSyncOptions, spawnSync, SpawnSyncOptions } from "child_process";
33
import * as fs from "fs";
44
import * as glob from "glob";
55
import * as path from "path";
@@ -282,6 +282,11 @@ export class Util {
282282
}
283283

284284
for (const key of Object.keys(source)) {
285+
// Skip keys that can affect the prototype of objects to avoid prototype pollution vulnerabilities
286+
if (key === "__proto__" || key === "constructor") {
287+
continue;
288+
}
289+
285290
const sourceKeyIsArray = Array.isArray(source[key]);
286291
const targetHasThisKey = target.hasOwnProperty(key);
287292

@@ -346,6 +351,32 @@ export class Util {
346351
}
347352
}
348353

354+
/**
355+
* Execute synchronous command with options using spawnSync
356+
* @param command Command to be executed
357+
* @param args Command arguments
358+
* @param options Command options
359+
* @throws {Error} On non-zero exit code. Error has 'status', 'signal', 'output', 'stdout', 'stderr'
360+
*/
361+
public static spawnSync(command: string, args: string[], options?: SpawnSyncOptions) {
362+
try {
363+
return spawnSync(command, args, options);
364+
} catch (error) {
365+
// Handle potential process interruption
366+
// Check if the error output ends with "^C"
367+
if (error.stderr && error.stderr.toString().endsWith() === "^C") {
368+
return process.exit();
369+
}
370+
371+
// Handle specific exit codes for different signals
372+
if (error.status === 3221225786 || error.status > 128) {
373+
return process.exit();
374+
}
375+
376+
throw error;
377+
}
378+
}
379+
349380
/**
350381
* Initialize git for a project, located in the provided directory and commit it.
351382
* @param parentRoot Parent directory root of the project.
@@ -356,7 +387,8 @@ export class Util {
356387
const options: any = { cwd: path.join(parentRoot, projectName), stdio: [process.stdin, "ignore", "ignore"] };
357388
Util.execSync("git init", options);
358389
Util.execSync("git add .", options);
359-
Util.execSync("git commit -m " + "\"Initial commit for project: " + projectName + "\"", options);
390+
const commitMessage = `"Initial commit for project: ${projectName}"`;
391+
Util.spawnSync('git', ['commit', '-m', commitMessage], options);
360392
Util.log(Util.greenCheck() + " Git Initialized and Project '" + projectName + "' Committed");
361393
} catch (error) {
362394
Util.error("Git initialization failed. Install Git in order to automatically commit the project.", "yellow");

‎spec/acceptance/help-spec.ts‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { GoogleAnalytics } from "@igniteui/cli-core";
2-
import { spawnSync } from "child_process";
1+
import { GoogleAnalytics, Util } from "@igniteui/cli-core";
32
import * as cli from "../../packages/cli/lib/cli";
43

54
const execLocation = "packages/cli/bin/execute.js";
@@ -37,7 +36,7 @@ describe("Help command", () => {
3736
});
3837

3938
it("should show help for the new command", async () => {
40-
const child = spawnSync("node", [execLocation, "new", "--help"], {
39+
const child = Util.spawnSync("node", [execLocation, "new", "--help"], {
4140
encoding: "utf-8"
4241
});
4342
const originalNewHelpText: string = `Options:
@@ -60,7 +59,7 @@ describe("Help command", () => {
6059
});
6160

6261
it("should show help for the config sub-commands", async () => {
63-
const child = spawnSync("node", [execLocation, "config", "--help"], {
62+
const child = Util.spawnSync("node", [execLocation, "config", "--help"], {
6463
encoding: "utf-8"
6564
});
6665
const originalNewHelpText: string = `Commands:
@@ -80,7 +79,7 @@ describe("Help command", () => {
8079
});
8180

8281
it("should show help for the generate sub-commands", async () => {
83-
const child = spawnSync("node", [execLocation, "generate", "--help"], {
82+
const child = Util.spawnSync("node", [execLocation, "generate", "--help"], {
8483
encoding: "utf-8"
8584
});
8685
const originalNewHelpText: string = `Commands:
@@ -97,7 +96,7 @@ describe("Help command", () => {
9796
});
9897

9998
it("should show help for the generate template sub-commands", async () => {
100-
const child = spawnSync("node", [execLocation, "g", "t", "-h"], {
99+
const child = Util.spawnSync("node", [execLocation, "g", "t", "-h"], {
101100
encoding: "utf-8"
102101
});
103102
const originalNewHelpText: string = `
@@ -119,7 +118,7 @@ describe("Help command", () => {
119118
});
120119

121120
it("should show help for the list command", async () => {
122-
const child = spawnSync("node", [execLocation, "list", "-h"], {
121+
const child = Util.spawnSync("node", [execLocation, "list", "-h"], {
123122
encoding: "utf-8"
124123
});
125124
const originalNewHelpText: string = `
@@ -137,7 +136,7 @@ describe("Help command", () => {
137136
});
138137

139138
it("should show help for the quickstart command", async () => {
140-
const child = spawnSync("node", [execLocation, "quickstart", "-h"], {
139+
const child = Util.spawnSync("node", [execLocation, "quickstart", "-h"], {
141140
encoding: "utf-8"
142141
});
143142

@@ -156,7 +155,7 @@ describe("Help command", () => {
156155
});
157156

158157
it("should show help for the test command", async () => {
159-
const child = spawnSync("node", [execLocation, "test", "-h"], {
158+
const child = Util.spawnSync("node", [execLocation, "test", "-h"], {
160159
encoding: "utf-8"
161160
});
162161

@@ -174,7 +173,7 @@ describe("Help command", () => {
174173
});
175174

176175
it("should show help for the doc command", async () => {
177-
const child = spawnSync("node", [execLocation, "doc", "-h"], {
176+
const child = Util.spawnSync("node", [execLocation, "doc", "-h"], {
178177
encoding: "utf-8"
179178
});
180179

@@ -192,7 +191,7 @@ describe("Help command", () => {
192191
});
193192

194193
it("should show help for the build command", async () => {
195-
const child = spawnSync("node", [execLocation, "build", "-h"], {
194+
const child = Util.spawnSync("node", [execLocation, "build", "-h"], {
196195
encoding: "utf-8"
197196
});
198197

@@ -210,7 +209,7 @@ describe("Help command", () => {
210209
});
211210

212211
it("should show help for the start command", async () => {
213-
const child = spawnSync("node", [execLocation, "start", "-h"], {
212+
const child = Util.spawnSync("node", [execLocation, "start", "-h"], {
214213
encoding: "utf-8"
215214
});
216215

@@ -228,7 +227,7 @@ describe("Help command", () => {
228227
});
229228

230229
it("should show help for the upgrade command", async () => {
231-
const child = spawnSync("node", [execLocation, "upgrade-packages", "-h"], {
230+
const child = Util.spawnSync("node", [execLocation, "upgrade-packages", "-h"], {
232231
encoding: "utf-8"
233232
});
234233

‎spec/unit/new-spec.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,24 @@ describe("Unit - New command", () => {
284284
getProjectLibrary: mockProjLib
285285
});
286286

287+
spyOn(Util, "spawnSync").and.returnValues({
288+
status: 0,
289+
pid: 0,
290+
output: [],
291+
stdout: "",
292+
stderr: "",
293+
signal: "SIGABRT"
294+
});
295+
287296
await newCmd.handler({ name: projectName, framework: "jq", _: ["new"], $0: "new" });
288297

289298
expect(Util.execSync).toHaveBeenCalledWith("git init", jasmine.any(Object));
290299
expect(Util.execSync).toHaveBeenCalledWith("git add .", jasmine.any(Object));
291-
expect(Util.execSync).toHaveBeenCalledWith("git commit -m " + "\"Initial commit for project: " + projectName + "\"",
292-
jasmine.any(Object));
300+
expect(Util.spawnSync).toHaveBeenCalledWith(
301+
"git",
302+
['commit', '-m', `"Initial commit for project: ${projectName}"`],
303+
{ cwd: path.join(process.cwd(), projectName), stdio: [process.stdin, "ignore", "ignore"] }
304+
);
293305
expect(Util.log).toHaveBeenCalledWith(
294306
jasmine.stringMatching("Git Initialized and Project '" + projectName + "' Committed")
295307
);

0 commit comments

Comments
 (0)