Skip to content

Commit de89755

Browse files
committed
fix(sandbox): Windows 命令行改用 raw_arg,引号不再被 MSVC 规则改坏
bare_shell_command 用 Command::arg 把整条命令交给 cmd /C,而 arg 走 MSVC C 运行时的 引号规则:带空格或引号的参数会被包上 " 且内部引号转义成 \"。cmd.exe 不实现这套规则 —— 它把 \ 当普通字符、把 " 当开关 —— 所以 git commit -m "msg" 到达时已变成 git commit -m \"msg\",消息里带着字面反斜杠执行。std 为此专门提供了 os::windows::process::CommandExt::raw_arg,本项目一次都没用过。 两个后果都是真的:Windows 上任何带引号参数的命令都被静默改坏(可观测症状就是模型放弃 git commit -m、改成把消息写进文件再 -F 提交);以及审批面板展示的命令与 cmd 实际执行的 不是同一条 —— 用户批准了 A,跑的是 B。 现按平台拆成两个 #[cfg] 函数(raw_arg 仅 Windows 存在,无法用 cfg! 运行时分支): /C 仍走普通 arg(裸 token,正常引号规则对它是对的),命令本身走 raw_arg,参数间仍以空格 连接,最终命令行即 cmd /C <command>。非 Windows 分支逐字不变。 顺带把那个探测测试的定位从"只探测不改行为"改成回归守卫,并补一条多个引号参数的用例 (git commit -m "…" 在 cmd 眼里就是这个形状)。 重要更正:07-30 曾记录"CI Windows job 通过 → 引号传参理论被否、代码未动",那条记录是 错的 —— 理论一直成立。bare_shell_command 自最初 c3d0ca4 起从未改过,探测测试是 07-30 加的,本次 Windows CI 报的失败内容与当初的预测逐字一致。本机无法跑 Windows 测试,也因 ring 的构建脚本无法整包交叉编译,故用一个无依赖的最小 crate 抽出改动后的函数,对 x86_64-pc-windows-msvc 单独 cargo check 通过,确认 raw_arg 用法在 Windows 下能编译。
1 parent c8818ac commit de89755

1 file changed

Lines changed: 64 additions & 31 deletions

File tree

  • crates/deep-code-agent/src/sandbox

crates/deep-code-agent/src/sandbox/mod.rs

Lines changed: 64 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -274,16 +274,39 @@ fn refuse_bare_execution(
274274
policy_wants_sandbox && forced.is_none() && !available
275275
}
276276

277+
/// Build `cmd /C <command>` with the command line passed to `cmd.exe` verbatim.
278+
///
279+
/// `Command::arg` applies the MSVC C-runtime quoting rules: an argument holding
280+
/// spaces or quotes is wrapped in `"` and its inner quotes escaped as `\"`.
281+
/// `cmd.exe` implements none of that — it treats `\` as an ordinary character
282+
/// and `"` as a quote toggle — so `git commit -m "msg"` reached the shell as
283+
/// `git commit -m \"msg\"` and ran with literal backslashes in the message.
284+
/// `raw_arg` exists for exactly this case.
285+
///
286+
/// Two consequences of the old behaviour, both real: every Windows command
287+
/// carrying a quoted argument was silently corrupted (the observable being that
288+
/// the model gave up on `git commit -m` and started writing the message to a
289+
/// file to commit with `-F`), and what actually executed differed from the
290+
/// command string shown in the approval panel — the user approved one thing and
291+
/// `cmd` ran another.
292+
#[cfg(windows)]
277293
fn bare_shell_command(command: &str, cwd: &Path) -> Command {
278-
let mut cmd = if cfg!(windows) {
279-
let mut cmd = Command::new("cmd");
280-
cmd.arg("/C").arg(command);
281-
cmd
282-
} else {
283-
let mut cmd = Command::new("sh");
284-
cmd.arg("-c").arg(command);
285-
cmd
286-
};
294+
use std::os::windows::process::CommandExt;
295+
296+
let mut cmd = Command::new("cmd");
297+
// `/C` is a bare token, so normal quoting is correct for it; only the
298+
// command itself must bypass the escaping. Arguments are still joined with
299+
// a space, so this yields `cmd /C <command>`.
300+
cmd.arg("/C");
301+
cmd.raw_arg(command);
302+
cmd.current_dir(cwd);
303+
cmd
304+
}
305+
306+
#[cfg(not(windows))]
307+
fn bare_shell_command(command: &str, cwd: &Path) -> Command {
308+
let mut cmd = Command::new("sh");
309+
cmd.arg("-c").arg(command);
287310
cmd.current_dir(cwd);
288311
cmd
289312
}
@@ -312,41 +335,51 @@ mod tests {
312335
assert!(!refuse_bare_execution(true, Some(true), false));
313336
}
314337

315-
/// Diagnostic probe for Windows argument passing — changes no behaviour.
338+
/// Regression guard for Windows argument passing.
316339
///
317-
/// `bare_shell_command` builds `cmd /C <command>` with `Command::arg`, which
318-
/// applies the MSVC C-runtime quoting rules: an argument containing spaces or
319-
/// quotes is wrapped in `"` and its inner quotes escaped as `\"`. `cmd.exe`
320-
/// does not implement those rules — it treats `\` as a literal character and
321-
/// `"` as a quote toggle — so any command carrying a quoted argument can
322-
/// arrive mangled. `std::os::windows::process::CommandExt::raw_arg` exists
323-
/// precisely for this case and is used nowhere here.
340+
/// This started life as a diagnostic probe and it caught a real defect:
341+
/// `bare_shell_command` used `Command::arg` for the whole command line, which
342+
/// applies the MSVC C-runtime quoting rules (an argument holding spaces or
343+
/// quotes is wrapped in `"`, inner quotes escaped as `\"`). `cmd.exe`
344+
/// implements none of that — `\` is an ordinary character and `"` a quote
345+
/// toggle — so every command carrying a quoted argument arrived mangled. The
346+
/// fix is `raw_arg`; this test fails loudly if anyone reverts to `arg`.
324347
///
325-
/// `echo "a b"` is the minimal probe: cmd's `echo` emits its argument
326-
/// verbatim, quotes included, so a correct pass-through prints exactly
327-
/// `"a b"`. Backslashes in the output mean the escaping leaked through.
348+
/// `echo` is the right probe because cmd's `echo` emits its argument
349+
/// verbatim, quotes included a correct pass-through prints exactly what was
350+
/// typed, and a stray backslash means the escaping leaked through.
328351
///
329-
/// This is the observable behind a real report: on Windows the model stopped
330-
/// using `git commit -m "<message>"` and started writing the message to a
331-
/// file to commit with `-F`, i.e. it routed around broken quoting.
352+
/// The observable behind the original report: on Windows the model stopped
353+
/// using `git commit -m "<message>"` and started writing the message to a file
354+
/// to commit with `-F`, i.e. it routed around the broken quoting.
332355
#[cfg(windows)]
333356
#[test]
334357
fn windows_cmd_receives_quoted_arguments_verbatim() {
335358
let cwd = std::env::current_dir().expect("cwd");
336-
let output = bare_shell_command("echo \"a b\"", &cwd)
337-
.output()
338-
.expect("spawn cmd");
339-
let stdout = String::from_utf8_lossy(&output.stdout);
340-
let got = stdout.trim();
341-
359+
let run = |command: &str| {
360+
let output = bare_shell_command(command, &cwd)
361+
.output()
362+
.expect("spawn cmd");
363+
String::from_utf8_lossy(&output.stdout).trim().to_string()
364+
};
365+
366+
let got = run("echo \"a b\"");
342367
assert!(
343368
!got.contains('\\'),
344-
"cmd received escaped quotes: stdout={got:?}. Command::arg applied \
345-
MSVC quoting that cmd.exe cannot parse; the fix is raw_arg."
369+
"cmd received escaped quotes: stdout={got:?}. `Command::arg` applies \
370+
MSVC quoting that cmd.exe cannot parse; use raw_arg."
346371
);
347372
assert_eq!(
348373
got, "\"a b\"",
349374
"quoted argument did not survive the trip to cmd.exe: stdout={got:?}"
350375
);
376+
377+
// The shape from the real report: several quoted arguments in one command,
378+
// which is what `git commit -m "…"` looks like to cmd.
379+
let got = run("echo \"a b\" \"c d\"");
380+
assert_eq!(
381+
got, "\"a b\" \"c d\"",
382+
"multiple quoted arguments were mangled: stdout={got:?}"
383+
);
351384
}
352385
}

0 commit comments

Comments
 (0)