mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Preserve script quotes in Windows sandbox cmd invocations (#60624)
## Why CRT argument escaping adds literal backslashes before quotes in `cmd.exe /c` scripts, breaking quoted paths and executable names. ## What changed Serialize single-script `cmd /c` invocations with `/s /c` and an outer quote pair, preserving the script's quotes. Share this handling across Windows sandbox process creation, ConPTY, and MXC execution requests. ## Testing Add a regression test for `certutil -hashfile` with a quoted path containing spaces, and update the command-line test for a quoted PowerShell executable. GitOrigin-RevId: 32b4b7b55f6af4dba206813eac150310f37ae1d4
This commit is contained in:
@@ -15,6 +15,7 @@ use codex_protocol::protocol::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::FileSystemSpecialPath;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_windows_sandbox::cmd_script_command_line;
|
||||
use codex_windows_sandbox::expand_windows_deny_read_paths;
|
||||
use codex_windows_sandbox::resolve_windows_deny_read_paths;
|
||||
use mxc_sdk::mxc_common::cmdline::CommandLineContext;
|
||||
@@ -320,10 +321,13 @@ pub(super) fn build_request(
|
||||
});
|
||||
}
|
||||
Ok(ExecutionRequest {
|
||||
script_code: cmdline_from_argv_for_context(
|
||||
&command.command,
|
||||
CommandLineContext::WindowsCreateProcess,
|
||||
)?,
|
||||
script_code: match cmd_script_command_line(&command.command) {
|
||||
Some(script) => script,
|
||||
None => cmdline_from_argv_for_context(
|
||||
&command.command,
|
||||
CommandLineContext::WindowsCreateProcess,
|
||||
)?,
|
||||
},
|
||||
working_directory: command_cwd
|
||||
.to_str()
|
||||
.ok_or(PolicyError::NonUnicodeCommandCwd)?
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
//! Serialize the single script argument used by the Windows command interpreter.
|
||||
|
||||
/// Build the native command line for Codex's `cmd.exe /c SCRIPT` invocation.
|
||||
/// The script is shell syntax. CRT argument escaping would introduce literal
|
||||
/// backslashes before its quotes. With /s, cmd strips exactly the outer pair.
|
||||
pub fn cmd_script_command_line(argv: &[String]) -> Option<String> {
|
||||
let [program, switch, script] = argv else {
|
||||
return None;
|
||||
};
|
||||
let name = program.rsplit(['/', '\\']).next()?;
|
||||
if !(name.eq_ignore_ascii_case("cmd.exe") || name.eq_ignore_ascii_case("cmd"))
|
||||
|| !switch.eq_ignore_ascii_case("/c")
|
||||
|| program.contains(['"', '\0'])
|
||||
|| script.contains('\0')
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Some(format!(r#""{program}" /s /c "{script}""#))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "cmd_script_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,19 @@
|
||||
#![cfg(test)]
|
||||
|
||||
use super::cmd_script_command_line;
|
||||
|
||||
#[test]
|
||||
fn preserves_quotes_in_the_hash_command() {
|
||||
let argv = [
|
||||
r"C:\Windows\System32\CMD.EXE",
|
||||
"/C",
|
||||
r#"certutil -hashfile "C:\New folder\long name.pftrace" SHA256"#,
|
||||
]
|
||||
.map(str::to_owned);
|
||||
assert_eq!(
|
||||
cmd_script_command_line(&argv).as_deref(),
|
||||
Some(
|
||||
r#""C:\Windows\System32\CMD.EXE" /s /c "certutil -hashfile "C:\New folder\long name.pftrace" SHA256""#
|
||||
)
|
||||
);
|
||||
}
|
||||
@@ -8,8 +8,8 @@
|
||||
|
||||
use crate::desktop::LaunchDesktop;
|
||||
use crate::proc_thread_attr::ProcThreadAttributeList;
|
||||
use crate::winutil::argv_to_command_line;
|
||||
use crate::winutil::format_last_error;
|
||||
use crate::winutil::quote_windows_arg;
|
||||
use crate::winutil::to_wide;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
@@ -112,11 +112,7 @@ pub fn spawn_conpty_process_as_user(
|
||||
env_map: &HashMap<String, String>,
|
||||
desktop: LaunchDesktop,
|
||||
) -> Result<(PROCESS_INFORMATION, ConptyInstance)> {
|
||||
let cmdline_str = argv
|
||||
.iter()
|
||||
.map(|arg| quote_windows_arg(arg))
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ");
|
||||
let cmdline_str = argv_to_command_line(argv);
|
||||
let mut cmdline: Vec<u16> = to_wide(&cmdline_str);
|
||||
let env_block = make_env_block(env_map);
|
||||
let mut si: STARTUPINFOEXW = unsafe { std::mem::zeroed() };
|
||||
|
||||
@@ -155,6 +155,9 @@ mod winutil;
|
||||
#[cfg(target_os = "windows")]
|
||||
mod workspace_acl;
|
||||
|
||||
mod cmd_script;
|
||||
pub use cmd_script::cmd_script_command_line;
|
||||
|
||||
mod deny_read_resolver;
|
||||
#[cfg(target_os = "windows")]
|
||||
mod uninstall_windows;
|
||||
|
||||
@@ -114,6 +114,9 @@ pub fn quote_windows_arg(arg: &str) -> String {
|
||||
/// Build a Windows command line for CreateProcess-style APIs.
|
||||
#[cfg(target_os = "windows")]
|
||||
pub fn argv_to_command_line(argv: &[String]) -> String {
|
||||
if let Some(command_line) = crate::cmd_script_command_line(argv) {
|
||||
return command_line;
|
||||
}
|
||||
argv.iter()
|
||||
.map(|arg| quote_windows_arg(arg))
|
||||
.collect::<Vec<_>>()
|
||||
@@ -330,7 +333,7 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn argv_to_command_line_quotes_each_argument_independently() {
|
||||
fn argv_to_command_line_preserves_quoted_executable_in_cmd_script() {
|
||||
let argv = vec![
|
||||
"cmd.exe".to_string(),
|
||||
"/c".to_string(),
|
||||
@@ -340,7 +343,7 @@ mod tests {
|
||||
|
||||
assert_eq!(
|
||||
argv_to_command_line(&argv),
|
||||
"cmd.exe /c \"\\\"C:\\Program Files\\PowerShell\\7\\pwsh.exe\\\" -NoProfile -EncodedCommand abc==\""
|
||||
r#""cmd.exe" /s /c ""C:\Program Files\PowerShell\7\pwsh.exe" -NoProfile -EncodedCommand abc==""#
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user