fix(core): quote child arguments so MSYS children receive them intact - #3728
Open
esidorenko-sl wants to merge 1 commit into
Open
fix(core): quote child arguments so MSYS children receive them intact#3728esidorenko-sl wants to merge 1 commit into
esidorenko-sl wants to merge 1 commit into
Conversation
esidorenko-sl
force-pushed
the
fix/windows-msys-child-arg-quoting
branch
from
August 27, 2026 12:19
629f8be to
d176798
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows
std::process::Commandwraps an argument in quotes only when it holds a space or a tab, soa"breaches the raw command line bare, asa\"b. MSVCRT/UCRT children parse that back correctly; Cygwin/MSYS children read the\"as a backslash and glue the surrounding arguments together. Sincewhichresolvesgrep,find,ls,wcand the rest ofusr/binto the MSYS binaries Git for Windows ships,rtk grep -c '"type"' filereported 0 matches and exit 1 while the nativertk rgwas unaffected (#3727).quote_arg_for_childinsrc/core/utils.rsemits the encoding libuv'squote_cmd_arguses — the form both parsers agree on — and aChildArgExttrait (child_arg/child_args) puts it in front of the child's command line. std already produces those same bytes for every other argument, so only an argument that actually contains a"leaves theCommand::argpath; everything else is encoded exactly as before..bat/.cmdprograms stay on std's path unconditionally, since cmd.exe parses by its own rules andraw_argwould bypass the escaping std applies for them.Converted call sites:
rtk grep/rtk rg(structural, passthrough and--version/--help),rtk find,rtk ls,rtk wc,rtk proxy,runner::run_passthroughand the unknown-command fallback inmain.rs. That is every place where an argument from rtk's own command line reaches a child resolved through PATH, sosed,awk,sort,trand friends are covered too when they arrive through the fallback.Out of scope
rtk err,rtk runandrtk summaryhand a whole string tocmd /Cinstead of an argument vector, which is a separate problem (#2985, #2389, #3704). MSYS2's own POSIX-path conversion of arguments (MSYS2_ARG_CONV_EXCL) sits above the command line and is left alone.Test plan
cargo fmt --all -- --check(0),cargo clippy --all-targets -- -D warnings(0),cargo test— 2647 unit tests and every integration binary pass.src/core/utils.rsfor the libuv rules and the.batguard; the encoder itself compiles and is tested on every platform.tests/windows_child_quoting_test.rs(#![cfg(windows)], silently skipped when no MSYSgrep.exeis around) drivesrtk grep -c '"type"' q.jsonlat the Git-for-Windows grep. With the fix disabled it fails withrtk grep exited Some(1), expected 0; with it, green.rtk grep -c '"type"' q.jsonl→1(was0, exit 1), andrtk printf '<%s> ' 'a"b' 'c d' '"type"'→<a"b> <c d> <"type">.rtk find,rtk lsandrtk wcunchanged on ordinary arguments.Built and tested on both
stable-x86_64-pc-windows-gnuandstable-x86_64-pc-windows-msvc. On both,core::stream::tests::test_read_lines_lossy_preserves_lines_after_invalid_utf8andhooks::hook_cmd::tests::test_claude_pipeline_rewrites_only_safe_final_stagefail on unmodifieddevelop(322db4d) here as well, so those two are pre-existing.Fixes #3727