Skip to content

Commit e76d9ab

Browse files
fix: find the command file on Windows like cross-spawn
Run .exe and .com files without a shell and escape the arguments twice for the .cmd shims of packages, they pass them to another shell. Claude-Session: https://claude.ai/code/session_013FhutRFjuQemBzekJMYcqL
1 parent b7ba111 commit e76d9ab

3 files changed

Lines changed: 115 additions & 18 deletions

File tree

‎.cspell.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
"Nitin",
6565
"nwjs",
6666
"Oikawa",
67+
"PATHEXT",
6768
"pathinfo",
6869
"plopfile",
6970
"pnpm",

‎packages/webpack-cli/src/webpack-cli.ts‎

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -647,20 +647,65 @@ class WebpackCLI {
647647
return spawnSync(command, args, options);
648648
}
649649

650-
// Package managers are `.cmd` files on Windows, which only a shell runs, and the shell doesn't
651-
// escape arguments
650+
// Find the file of the command like `which`, on Windows the current directory goes first
651+
const extensions = (process.env.PATHEXT || ".EXE;.CMD;.BAT;.COM").split(";");
652+
653+
if (command.includes(".") && extensions[0] !== "") {
654+
extensions.unshift("");
655+
}
656+
657+
const directories = /[/\\]/.test(command)
658+
? [""]
659+
: [process.cwd(), ...(process.env.PATH || "").split(";")];
660+
let file: string | undefined;
661+
662+
search: for (const directory of directories) {
663+
for (const extension of extensions) {
664+
const candidate = path.resolve(
665+
path.join(directory.replace(/^"(.*)"$/, "$1"), command) + extension,
666+
);
667+
668+
try {
669+
if (
670+
fs.statSync(candidate).isFile() &&
671+
extensions.some(
672+
(extension) =>
673+
extension !== "" && candidate.toLowerCase().endsWith(extension.toLowerCase()),
674+
)
675+
) {
676+
file = candidate;
677+
678+
break search;
679+
}
680+
} catch {
681+
// Not found
682+
}
683+
}
684+
}
685+
686+
// Executables run directly
687+
if (file && /\.(?:com|exe)$/i.test(file)) {
688+
return spawnSync(command, args, options);
689+
}
690+
691+
// Other files, e.g. `npm.cmd`, only run in a shell, which doesn't escape arguments, and the
692+
// `.cmd` shims of packages pass them to another shell, so they are escaped twice
693+
const isShim = Boolean(file && /node_modules[\\/].bin[\\/][^\\/]+\.cmd$/i.test(file));
694+
const metaChars = /([()\][%!^"`<>&|;, *?])/g;
652695
const escapedArgs = args.map((arg) => {
653696
const quoted = `"${arg
654697
// Double the backslashes before a quote and escape the quote
655698
.replaceAll(/(?=(\\+?)?)\1"/g, '$1$1\\"')
656699
// Double the backslashes at the end, they precede the closing quote
657-
.replace(/(?=(\\+?)?)\1$/, "$1$1")}"`;
700+
.replace(/(?=(\\+?)?)\1$/, "$1$1")}"`.replaceAll(metaChars, "^$1");
658701

659-
// Escape the characters `cmd.exe` treats specially
660-
return quoted.replaceAll(/([()\][%!^"`<>&|;, *?])/g, "^$1");
702+
return isShim ? quoted.replaceAll(metaChars, "^$1") : quoted;
661703
});
662704

663-
return spawnSync([command, ...escapedArgs].join(" "), { ...options, shell: true });
705+
return spawnSync(
706+
[path.normalize(command).replaceAll(metaChars, "^$1"), ...escapedArgs].join(" "),
707+
{ ...options, shell: true },
708+
);
664709
}
665710

666711
async getDefaultPackageManager(): Promise<PackageManager | undefined> {

‎test/api/install-package.test.js‎

Lines changed: 63 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
"use strict";
22

3+
const fs = require("node:fs");
4+
const os = require("node:os");
5+
const path = require("node:path");
36
const { stripVTControlCharacters } = require("node:util");
47
const CLI = require("../../packages/webpack-cli/lib/webpack-cli").default;
58

@@ -99,23 +102,71 @@ describe("installPackage", () => {
99102
expectInstallCommand("pnpm", ["install", "-D", "test-package"]);
100103
});
101104

102-
it("should escape the arguments for the shell on Windows", async () => {
103-
readlineQuestionMock.mockImplementation((_questionTest, cb) => cb("y"));
104-
getDefaultPackageManagerSpy.mockReturnValue("npm");
105+
describe("on Windows", () => {
106+
let directory;
107+
let platform;
108+
let env;
109+
110+
beforeEach(() => {
111+
directory = fs.mkdtempSync(path.join(os.tmpdir(), "webpack-cli-install-package-"));
112+
platform = Object.getOwnPropertyDescriptor(process, "platform");
113+
env = { PATH: process.env.PATH, PATHEXT: process.env.PATHEXT };
114+
115+
Object.defineProperty(process, "platform", { value: "win32" });
116+
process.env.PATH = directory;
117+
process.env.PATHEXT = ".exe;.cmd";
118+
readlineQuestionMock.mockImplementation((_questionTest, cb) => cb("y"));
119+
getDefaultPackageManagerSpy.mockReturnValue("npm");
120+
});
105121

106-
const platform = Object.getOwnPropertyDescriptor(process, "platform");
122+
afterEach(() => {
123+
Object.defineProperty(process, "platform", platform);
107124

108-
Object.defineProperty(process, "platform", { value: "win32" });
125+
for (const [name, value] of Object.entries(env)) {
126+
if (typeof value === "undefined") {
127+
delete process.env[name];
128+
} else {
129+
process.env[name] = value;
130+
}
131+
}
132+
133+
fs.rmSync(directory, { recursive: true, force: true });
134+
});
135+
136+
it("should escape the arguments for the shell", async () => {
137+
fs.writeFileSync(path.join(directory, "npm.cmd"), "");
109138

110-
try {
111139
await cli.installPackage('a"b&c');
112-
} finally {
113-
Object.defineProperty(process, "platform", platform);
114-
}
115140

116-
expect(spawnMock).toHaveBeenCalledWith('npm ^"install^" ^"-D^" ^"a\\^"b^&c^"', {
117-
stdio: "inherit",
118-
shell: true,
141+
expect(spawnMock).toHaveBeenCalledWith('npm ^"install^" ^"-D^" ^"a\\^"b^&c^"', {
142+
stdio: "inherit",
143+
shell: true,
144+
});
145+
});
146+
147+
it("should escape the arguments twice for the shims of packages", async () => {
148+
const bin = path.join(directory, "node_modules", ".bin");
149+
150+
fs.mkdirSync(bin, { recursive: true });
151+
fs.writeFileSync(path.join(bin, "npm.cmd"), "");
152+
process.env.PATH = bin;
153+
154+
await cli.installPackage('a"b&c');
155+
156+
expect(spawnMock).toHaveBeenCalledWith(
157+
'npm ^^^"install^^^" ^^^"-D^^^" ^^^"a\\^^^"b^^^&c^^^"',
158+
{ stdio: "inherit", shell: true },
159+
);
160+
});
161+
162+
it("should run executables without a shell", async () => {
163+
fs.writeFileSync(path.join(directory, "npm.exe"), "");
164+
165+
await cli.installPackage("test-package");
166+
167+
expect(spawnMock).toHaveBeenCalledWith("npm", ["install", "-D", "test-package"], {
168+
stdio: "inherit",
169+
});
119170
});
120171
});
121172

0 commit comments

Comments
 (0)