Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions xtask/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,18 @@ pub fn spawn_child(label: &str, cmd: &str, args: &[&str], cwd: &Path) -> Option<
console::style(label).cyan().bold(),
);

let mut command = Command::new(cmd);
// On Windows, non-.exe commands (e.g. .cmd/.bat scripts like pnpm) must be
// launched through cmd.exe because CreateProcessW only resolves .exe files.
let mut command = if cfg!(windows) {
let mut c = Command::new("cmd");
c.arg("/c").arg(cmd).args(args);
c
} else {

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When cmd.exe /c is used, individual arguments in the args slice are passed as separate arg() calls, but cmd.exe reassembles them into a single command line string using its own quoting rules. If any argument or the cmd value contains spaces, special characters (&, |, >, <, ^, %, !), or quotes, cmd.exe may misinterpret them, leading to incorrect behavior or potential argument injection. The current callers only pass simple strings like "start:server", so this is not an immediate problem, but it is a latent hazard as the function is public and could be called with arbitrary arguments in the future. Consider documenting this limitation or using std::os::windows::process::CommandExt::raw_arg to bypass cmd.exe's re-quoting.

Suggested change
// On Windows, non-.exe commands (e.g. .cmd/.bat scripts like pnpm) must be
// launched through cmd.exe because CreateProcessW only resolves .exe files.
let mut command = if cfg!(windows) {
let mut c = Command::new("cmd");
c.arg("/c").arg(cmd).args(args);
c
} else {
// Spawn the requested command directly; on Windows we rely on PATH/PATHEXT
// resolution so that shell wrapper scripts (e.g. .cmd/.bat) are located
// without going through `cmd.exe`.
let mut command = {

Copilot uses AI. Check for mistakes.
let mut c = Command::new(cmd);
c.args(args);
c
};

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When cmd.exe /c is used to wrap the command on Windows, the Child handle returned by command.spawn() refers to the cmd.exe process, not the actual target process (e.g. pnpm). As a result, send_graceful_stop sends CTRL_BREAK_EVENT to cmd.exe's process group ID (the cmd.exe PID), not to the real subprocess's group. Because cmd.exe /c spawns the real process in its own context and does not forward CTRL_BREAK_EVENT, the graceful shutdown mechanism (lines 229–238) silently fails: only cmd.exe is signalled, and the real long-running process (e.g. a pnpm dev server) is left running or must wait for the 3-second timeout to be force-killed.

A more robust approach for Windows is to use where.exe or std::env::var("PATHEXT") to locate the actual .cmd/.bat file and pass its full path to Command::new, or to use the which crate to resolve the executable. Alternatively, if cmd.exe /c must be kept, the process group setup and graceful-stop logic need to account for the extra cmd.exe wrapper layer.

Copilot uses AI. Check for mistakes.
command
.args(args)
.current_dir(cwd)
.stdin(Stdio::inherit())
.stdout(Stdio::inherit())
Expand Down
Loading