Skip to content

Commit baf3741

Browse files
paleitepaleite
committed
Fix potential security vulnerability
1 parent 67582d2 commit baf3741

File tree

7 files changed

+1180
-1358
lines changed

7 files changed

+1180
-1358
lines changed

‎husky.config.js

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎package.json

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,31 +40,31 @@
4040
"typecheck": "tsc --project tsconfig.json"
4141
},
4242
"devDependencies": {
43-
"@paleite/eslint-config": "^1.0.0",
44-
"@paleite/eslint-config-base": "^1.0.0",
45-
"@paleite/eslint-config-typescript": "^1.0.0",
46-
"@paleite/jest-config": "^1.0.0",
47-
"@paleite/prettier-config": "^1.0.0",
48-
"@paleite/tsconfig-node16": "^1.0.0",
49-
"@types/eslint": "^8.4.1",
50-
"@types/jest": "^27.4.1",
43+
"@paleite/eslint-config": "^1.0.2",
44+
"@paleite/eslint-config-base": "^1.0.2",
45+
"@paleite/eslint-config-typescript": "^1.0.2",
46+
"@paleite/jest-config": "^1.0.2",
47+
"@paleite/prettier-config": "^1.0.2",
48+
"@paleite/tsconfig-node16": "^1.0.2",
49+
"@types/eslint": "^8.4.2",
50+
"@types/jest": "^28.1.1",
5151
"@types/node": "^16.11.26",
52-
"@typescript-eslint/eslint-plugin": "^5.15.0",
53-
"@typescript-eslint/parser": "^5.15.0",
54-
"eslint": "^8.11.0",
52+
"@typescript-eslint/eslint-plugin": "^5.27.0",
53+
"@typescript-eslint/parser": "^5.27.0",
54+
"eslint": "^8.17.0",
5555
"eslint-config-prettier": "^8.5.0",
5656
"eslint-files": "^1.0.0",
57-
"eslint-plugin-import": "^2.25.4",
57+
"eslint-plugin-import": "^2.26.0",
5858
"eslint-plugin-promise": "^6.0.0",
59-
"husky": "^7.0.4",
60-
"jest": "^27.5.1",
61-
"jest-mock": "^27.5.1",
62-
"lint-staged": "^12.3.7",
59+
"husky": "^8.0.1",
60+
"jest": "^28.1.0",
61+
"jest-mock": "^28.1.0",
62+
"lint-staged": "^13.0.0",
6363
"np": "^7.6.1",
6464
"pinst": "^3.0.0",
65-
"prettier": "^2.6.0",
66-
"ts-jest": "^27.1.3",
67-
"typescript": "^4.6.2"
65+
"prettier": "^2.6.2",
66+
"ts-jest": "^28.0.4",
67+
"typescript": "^4.7.3"
6868
},
6969
"peerDependencies": {
7070
"eslint": ">=6.7.0"

‎src/git.test.ts

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -50,67 +50,70 @@ describe("getRangesForDiff", () => {
5050

5151
describe("getDiffForFile", () => {
5252
it("should get the staged diff of a file", () => {
53-
mockedChildProcess.execSync.mockReturnValueOnce(Buffer.from(hunks));
53+
mockedChildProcess.execFileSync.mockReturnValueOnce(Buffer.from(hunks));
5454
process.env.ESLINT_PLUGIN_DIFF_COMMIT = "1234567";
5555

5656
const diffFromFile = getDiffForFile("./mockfile.js", true);
5757

58-
const expectedArguments =
59-
'git diff --diff-algorithm=histogram --diff-filter=ACM -M100% --relative --staged --unified=0 "1234567"';
60-
expect(
61-
mockedChildProcess.execSync.mock.calls[
62-
mockedChildProcess.execSync.mock.calls.length - 1
63-
][0]
64-
.split(" -- ")
65-
.shift()
66-
).toEqual(expectedArguments);
58+
const expectedCommand = "git";
59+
const expectedArgs =
60+
'diff --diff-algorithm=histogram --diff-filter=ACM -M100% --relative --staged --unified=0 "1234567"';
61+
62+
const lastCall = mockedChildProcess.execFileSync.mock.calls.at(-1);
63+
const [command, argsIncludingFile = []] = lastCall ?? [""];
64+
const args = argsIncludingFile.slice(0, argsIncludingFile.length - 2);
65+
66+
expect(command).toBe(expectedCommand);
67+
expect(args.join(" ")).toEqual(expectedArgs);
6768
expect(diffFromFile).toContain("diff --git");
6869
expect(diffFromFile).toContain("@@");
6970
});
7071

7172
it("should hit the cached diff of a file", () => {
7273
jest.mock("child_process").resetAllMocks();
73-
mockedChildProcess.execSync.mockReturnValueOnce(Buffer.from(hunks));
74+
mockedChildProcess.execFileSync.mockReturnValueOnce(Buffer.from(hunks));
7475

75-
expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(0);
76+
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(0);
7677
const diffFromFileA = getDiffForFile("./mockfileCache.js");
7778
const diffFromFileB = getDiffForFile("./mockfileCache.js");
78-
expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(1);
79+
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(1);
7980
expect(diffFromFileA).toEqual(diffFromFileB);
8081

81-
mockedChildProcess.execSync.mockReturnValueOnce(Buffer.from(hunks));
82+
mockedChildProcess.execFileSync.mockReturnValueOnce(Buffer.from(hunks));
8283
getDiffForFile("./mockfileMiss.js");
83-
expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(2);
84+
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(2);
8485
});
8586
});
8687

8788
describe("hasCleanIndex", () => {
8889
it("returns false instead of throwing", () => {
8990
jest.mock("child_process").resetAllMocks();
90-
mockedChildProcess.execSync.mockImplementationOnce(() => {
91+
mockedChildProcess.execFileSync.mockImplementationOnce(() => {
9192
throw Error("mocked error");
9293
});
9394
expect(hasCleanIndex("")).toEqual(false);
94-
expect(mockedChildProcess.execSync).toHaveBeenCalled();
95+
expect(mockedChildProcess.execFileSync).toHaveBeenCalled();
9596
});
9697

9798
it("returns true otherwise", () => {
9899
jest.mock("child_process").resetAllMocks();
99-
mockedChildProcess.execSync.mockReturnValue(Buffer.from(""));
100+
mockedChildProcess.execFileSync.mockReturnValue(Buffer.from(""));
100101
expect(hasCleanIndex("")).toEqual(true);
101-
expect(mockedChildProcess.execSync).toHaveBeenCalled();
102+
expect(mockedChildProcess.execFileSync).toHaveBeenCalled();
102103
});
103104
});
104105

105106
describe("getDiffFileList", () => {
106107
it("should get the list of staged files", () => {
107108
jest.mock("child_process").resetAllMocks();
108-
mockedChildProcess.execSync.mockReturnValueOnce(Buffer.from(diffFileList));
109-
expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(0);
109+
mockedChildProcess.execFileSync.mockReturnValueOnce(
110+
Buffer.from(diffFileList)
111+
);
112+
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(0);
110113
const fileListA = getDiffFileList();
111114
const fileListB = getDiffFileList();
112115

113-
expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(1);
116+
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(1);
114117
expect(fileListA).toEqual(
115118
["file1", "file2", "file3"].map((p) => path.resolve(p))
116119
);
@@ -121,13 +124,15 @@ describe("getDiffFileList", () => {
121124
describe("getUntrackedFileList", () => {
122125
it("should get the list of untracked files", () => {
123126
jest.mock("child_process").resetAllMocks();
124-
mockedChildProcess.execSync.mockReturnValueOnce(Buffer.from(diffFileList));
125-
expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(0);
127+
mockedChildProcess.execFileSync.mockReturnValueOnce(
128+
Buffer.from(diffFileList)
129+
);
130+
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(0);
126131
const fileListA = getUntrackedFileList();
127132
const staged = false;
128133
const fileListB = getUntrackedFileList(staged);
129134

130-
expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(1);
135+
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(1);
131136
expect(fileListA).toEqual(
132137
["file1", "file2", "file3"].map((p) => path.resolve(p))
133138
);
@@ -142,7 +147,9 @@ describe("getUntrackedFileList", () => {
142147

143148
describe("getGitFileList", () => {
144149
it("should get the list of committed files", () => {
145-
mockedChildProcess.execSync.mockReturnValueOnce(Buffer.from(diffFileList));
150+
mockedChildProcess.execFileSync.mockReturnValueOnce(
151+
Buffer.from(diffFileList)
152+
);
146153
expect(getGitFileList()).toEqual(
147154
["file1", "file2", "file3"].map((p) => path.resolve(p))
148155
);

‎src/git.ts

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as path from "path";
33
import { Range } from "./Range";
44

55
const RUNNING_INSIDE_VSCODE = process.env.VSCODE_CLI !== undefined;
6+
const COMMAND = "git";
67

78
const sanitizeFilePath = (filePath: string) =>
89
JSON.stringify(path.resolve(filePath));
@@ -20,8 +21,7 @@ const getCachedDiff = (filePath: string, staged: boolean) =>
2021
const getDiffForFile = (filePath: string, staged = false): string => {
2122
let diff = getCachedDiff(filePath, staged);
2223
if (RUNNING_INSIDE_VSCODE || diff === undefined) {
23-
const command = [
24-
"git",
24+
const args = [
2525
"diff",
2626
"--diff-algorithm=histogram",
2727
"--diff-filter=ACM",
@@ -32,11 +32,12 @@ const getDiffForFile = (filePath: string, staged = false): string => {
3232
JSON.stringify(process.env.ESLINT_PLUGIN_DIFF_COMMIT ?? "HEAD"),
3333
"--",
3434
sanitizeFilePath(filePath),
35-
]
36-
.filter(Boolean)
37-
.join(" ");
35+
].reduce<string[]>(
36+
(acc, cur) => (typeof cur === "string" ? [...acc, cur] : acc),
37+
[]
38+
);
3839

39-
const result = child_process.execSync(command).toString();
40+
const result = child_process.execFileSync(COMMAND, args).toString();
4041
setCachedDiff(filePath, staged, result);
4142
diff = result;
4243
}
@@ -47,8 +48,7 @@ const getDiffForFile = (filePath: string, staged = false): string => {
4748
let diffFileListCache: string[] | undefined;
4849
const getDiffFileList = (): string[] => {
4950
if (RUNNING_INSIDE_VSCODE || diffFileListCache === undefined) {
50-
const command = [
51-
"git",
51+
const args = [
5252
"diff",
5353
"--diff-algorithm=histogram",
5454
"--diff-filter=ACM",
@@ -57,12 +57,10 @@ const getDiffFileList = (): string[] => {
5757
"--relative",
5858
"--staged",
5959
JSON.stringify(process.env.ESLINT_PLUGIN_DIFF_COMMIT ?? "HEAD"),
60-
]
61-
.filter(Boolean)
62-
.join(" ");
60+
];
6361

6462
diffFileListCache = child_process
65-
.execSync(command)
63+
.execFileSync(COMMAND, args)
6664
.toString()
6765
.trim()
6866
.split("\n")
@@ -75,10 +73,10 @@ const getDiffFileList = (): string[] => {
7573
let gitFileListCache: string[] | undefined;
7674
const getGitFileList = (): string[] => {
7775
if (RUNNING_INSIDE_VSCODE || gitFileListCache === undefined) {
78-
const command = ["git", "ls-files"].filter(Boolean).join(" ");
76+
const args = ["ls-files"];
7977

8078
gitFileListCache = child_process
81-
.execSync(command)
79+
.execFileSync(COMMAND, args)
8280
.toString()
8381
.trim()
8482
.split("\n")
@@ -89,19 +87,18 @@ const getGitFileList = (): string[] => {
8987
};
9088

9189
const hasCleanIndex = (filePath: string): boolean => {
92-
const command = [
93-
"git",
90+
const args = [
9491
"diff",
9592
"--quiet",
9693
"--relative",
9794
"--unified=0",
9895
"--",
9996
sanitizeFilePath(filePath),
100-
].join(" ");
97+
];
10198

10299
let result = true;
103100
try {
104-
child_process.execSync(command).toString();
101+
child_process.execFileSync(COMMAND, args).toString();
105102
} catch (err: unknown) {
106103
result = false;
107104
}
@@ -114,12 +111,10 @@ const getUntrackedFileList = (staged = false): string[] => {
114111
if (staged) {
115112
untrackedFileListCache = [];
116113
} else if (RUNNING_INSIDE_VSCODE || untrackedFileListCache === undefined) {
117-
const command = ["git", "ls-files", "--exclude-standard", "--others"]
118-
.filter(Boolean)
119-
.join(" ");
114+
const args = ["ls-files", "--exclude-standard", "--others"];
120115

121116
untrackedFileListCache = child_process
122-
.execSync(command)
117+
.execFileSync(COMMAND, args)
123118
.toString()
124119
.trim()
125120
.split("\n")

‎src/index.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import { mocked } from "jest-mock";
33

44
jest.mock("child_process");
55
const mockedChildProcess = mocked(child_process, true);
6-
mockedChildProcess.execSync.mockReturnValue(Buffer.from("line1\nline2\nline3"));
6+
mockedChildProcess.execFileSync.mockReturnValue(
7+
Buffer.from("line1\nline2\nline3")
8+
);
79

810
import { configs, processors } from "./index";
911

‎src/processors.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { postprocessArguments } from "./__fixtures__/postprocessArguments";
1010

1111
jest.mock("child_process");
1212
const mockedChildProcess = mocked(child_process, true);
13-
mockedChildProcess.execSync.mockReturnValue(
13+
mockedChildProcess.execFileSync.mockReturnValue(
1414
Buffer.from('/mock filename ", ; .js')
1515
);
1616
jest.mock("./git", () => ({
@@ -27,7 +27,7 @@ describe("processors", () => {
2727
const validFilename = filename;
2828
const sourceCode = "/** Some source code */";
2929

30-
mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureDiff));
30+
mockedChildProcess.execFileSync.mockReturnValue(Buffer.from(fixtureDiff));
3131

3232
expect(diff.preprocess(sourceCode, validFilename)).toEqual([sourceCode]);
3333
});
@@ -36,29 +36,29 @@ describe("processors", () => {
3636
const validFilename = filename;
3737
const sourceCode = "/** Some source code */";
3838

39-
mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureDiff));
39+
mockedChildProcess.execFileSync.mockReturnValue(Buffer.from(fixtureDiff));
4040

4141
expect(diff.preprocess(sourceCode, validFilename)).toEqual([sourceCode]);
4242
});
4343

4444
it("diff postprocess", () => {
45-
mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureDiff));
45+
mockedChildProcess.execFileSync.mockReturnValue(Buffer.from(fixtureDiff));
4646

4747
expect(diff.postprocess(messages, filename)).toMatchSnapshot();
4848

49-
expect(mockedChildProcess.execSync).toHaveBeenCalled();
49+
expect(mockedChildProcess.execFileSync).toHaveBeenCalled();
5050
});
5151

5252
it("staged postprocess", () => {
53-
mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureStaged));
53+
mockedChildProcess.execFileSync.mockReturnValue(Buffer.from(fixtureStaged));
5454

5555
expect(staged.postprocess(messages, filename)).toMatchSnapshot();
5656

57-
expect(mockedChildProcess.execSync).toHaveBeenCalled();
57+
expect(mockedChildProcess.execFileSync).toHaveBeenCalled();
5858
});
5959

6060
it("should report fatal errors", () => {
61-
mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureDiff));
61+
mockedChildProcess.execFileSync.mockReturnValue(Buffer.from(fixtureDiff));
6262

6363
const [[firstMessage, ...restMessage], ...restMessageArray] = messages;
6464
const messagesWithFatal: Linter.LintMessage[][] = [
@@ -69,7 +69,7 @@ describe("processors", () => {
6969
expect(diff.postprocess(messages, filename)).toHaveLength(2);
7070
expect(diff.postprocess(messagesWithFatal, filename)).toHaveLength(3);
7171

72-
expect(mockedChildProcess.execSync).toHaveBeenCalled();
72+
expect(mockedChildProcess.execFileSync).toHaveBeenCalled();
7373
});
7474
});
7575

0 commit comments

Comments
 (0)