Skip to content

fix(core): quote arguments passed to external commands on Windows - #17

Merged
cataggar merged 1 commit into
kfrom
fix/windows-external-command-arg-quoting
Aug 20, 2026
Merged

cataggar merged 1 commit into
kfrom
fix/windows-external-command-arg-quoting

Conversation

@cataggar

Copy link
Copy Markdown
Owner

Fixes #16.

Root cause

Windows has no argv array: a child process receives a single command-line string and parses it itself. std::process::Command only wraps an argument in double quotes when it is empty or contains whitespace or a quote, so every other argument is emitted bare.

Programs built on the MSYS2/Cygwin runtime — including all the tools shipped with Git for Windows — re-parse that raw command line and run Cygwin's globify on it, applying glob, brace, and tilde expansion plus quote removal. Words that brush had already finished expanding got expanded a second time by the child.

Git-for-Windows bash is immune because it is itself an MSYS program and hands argv to MSYS children over the Cygwin IPC channel, so no re-parse happens.

Impact is wider than the issue reported

Measured by passing each argument through to Git's printf.exe:

argument received before received after
[0-9]{3} [0-9]3 [0-9]{3}
MAC\([A-Fa-f0-9]{12} MAC([A-Fa-f0-9]12 unchanged
x{a,b}y xay, xby (two arguments) unchanged
* every file in the working directory *
~ /c/Users/<user> ~
a'b ab a'b

The * and ~ rows are not in the original report but are arguably worse: find . -name '*.txt' and grep -r x * were silently operating on the wrong input.

Fix

Quote every argument explicitly using the rules understood by CommandLineToArgvW and the Microsoft C runtime, via CommandExt::raw_arg. Native programs are unaffected because they strip the quotes while parsing. (CommandExt::force_quotes would be the natural API, but it is still unstable — rust-lang/rust#82227 — and the MSRV here is 1.88 stable.)

Argument encoding is now routed through a new sys::commands::set_args, which is a straight passthrough to Command::args on unix and wasm.

Two carve-outs, both verified necessary

Arguments bound for cmd.exe are left to the standard library to encode:

  1. cmd.exe rejects quotes around its own options. A command line of cmd.exe "/c" "ver" fails with '"ver' is not recognized as an internal or external command.
  2. Batch files lose the standard library's extra escaping. std runs .bat/.cmd through cmd.exe and neutralises metacharacters such as % (the CVE-2024-24576 mitigation). Encoding a raw command line bypasses that: a literal %CD% argument was observed being expanded to the working directory.

Neither cmd.exe nor a batch file is an MSYS2/Cygwin program, so no protection is lost by deferring to the standard library for them.

Testing

New unit tests in brush-core/src/sys/windows/commands.rs cover the quoting rules (metacharacters, embedded quotes, trailing backslashes, empty arguments) and the carve-out detection, plus three end-to-end tests:

  • cmd.exe /c "echo hello world" still runs
  • a generated .bat still receives %CD% literally
  • an MSYS2 child receives every argument byte-for-byte (skipped when Git for Windows is not installed)

CI runs the suite on windows-2025, so these execute there.

Verified manually against the issue's repro:

$ echo "abc123" | grep -oE '[0-9]{3}'
123
$ echo "MAC(AABBCCDDEEFF,0x1)" | grep -oE 'MAC\([A-Fa-f0-9]{12}'
MAC(AABBCCDDEEFF

Also checked: cargo xtask test integration (401 passed), cargo clippy --workspace --all-features --all-targets, cargo fmt --check, and cargo check against x86_64-unknown-linux-gnu and wasm32-unknown-unknown to confirm the non-Windows paths are unaffected.

Assisted-by: GitHub Copilot CLI:claude-opus-5

Windows has no argv array: a child process receives a single command-line
string and parses it itself. The standard library only quotes an argument
when it is empty or contains whitespace or a quote, which leaves every
other argument exposed to a second round of interpretation in the child.

Programs built on the MSYS2/Cygwin runtime -- including the tools shipped
with Git for Windows -- re-expand the arguments they receive from a native
parent, applying glob, brace, and tilde expansion along with quote removal.
Words the shell had already finished expanding were silently corrupted:

    grep -oE '[0-9]{3}'   ->  child saw  [0-9]3
    grep -oE 'MAC\(x'     ->  child saw  MAC(x
    find . -name '*.txt'  ->  child saw  every file in the directory
    printf '%s' '~'       ->  child saw  the home directory

Explicitly quote every argument using the rules understood by
CommandLineToArgvW so the child recovers exactly what the shell expanded.
Native programs are unaffected, since they strip the quotes while parsing.

Arguments bound for cmd.exe are left to the standard library to encode:
cmd.exe rejects quotes around its own options, and the standard library
applies extra escaping when spawning a batch file that a raw command line
would bypass. Neither is an MSYS2/Cygwin program, so no protection is lost.

Fixes #16

Assisted-by: GitHub Copilot CLI:claude-opus-5
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@cataggar
cataggar force-pushed the fix/windows-external-command-arg-quoting branch from 51d1783 to 7f9c1c8 Compare August 20, 2026 18:13
@cataggar
cataggar merged commit 84f87b0 into k Aug 20, 2026
25 of 35 checks passed
@cataggar
cataggar deleted the fix/windows-external-command-arg-quoting branch August 20, 2026 18:14
cataggar added a commit that referenced this pull request Aug 22, 2026
Windows has no argv array: a child process receives a single command-line
string and parses it itself. The standard library only quotes an argument
when it is empty or contains whitespace or a quote, which leaves every
other argument exposed to a second round of interpretation in the child.

Programs built on the MSYS2/Cygwin runtime -- including the tools shipped
with Git for Windows -- re-expand the arguments they receive from a native
parent, applying glob, brace, and tilde expansion along with quote removal.
Words the shell had already finished expanding were silently corrupted:

    grep -oE '[0-9]{3}'   ->  child saw  [0-9]3
    grep -oE 'MAC\(x'     ->  child saw  MAC(x
    find . -name '*.txt'  ->  child saw  every file in the directory
    printf '%s' '~'       ->  child saw  the home directory

Explicitly quote every argument using the rules understood by
CommandLineToArgvW so the child recovers exactly what the shell expanded.
Native programs are unaffected, since they strip the quotes while parsing.

Arguments bound for cmd.exe are left to the standard library to encode:
cmd.exe rejects quotes around its own options, and the standard library
applies extra escaping when spawning a batch file that a raw command line
would bypass. Neither is an MSYS2/Cygwin program, so no protection is lost.

Fixes #16

Assisted-by: GitHub Copilot CLI:claude-opus-5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: arguments containing braces or escaped parens are corrupted when passed to external commands

1 participant