mirror of
https://github.com/openai/codex.git
synced 2026-09-04 15:08:45 +00:00
Fix long Windows working-directory launches
This commit is contained in:
@@ -1050,7 +1050,7 @@ impl UnifiedExecProcessManager {
|
||||
path: request.cwd.clone(),
|
||||
})?;
|
||||
|
||||
let (program, args) = request
|
||||
let (_program, args) = request
|
||||
.command
|
||||
.split_first()
|
||||
.ok_or(UnifiedExecError::MissingCommandLine)?;
|
||||
@@ -1069,7 +1069,7 @@ impl UnifiedExecProcessManager {
|
||||
))
|
||||
})?;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
let program = program.as_str();
|
||||
let program = _program.as_str();
|
||||
let spawn_result = if tty {
|
||||
codex_utils_pty::pty::spawn_process_with_inherited_fds(
|
||||
program,
|
||||
|
||||
@@ -107,7 +107,7 @@ fn windows_search_dirs(cwd: &Path, env_map: &HashMap<String, String>) -> Vec<Pat
|
||||
.filter(|dir| {
|
||||
!dir.as_os_str().is_empty()
|
||||
&& !is_drive_relative(dir)
|
||||
&& !(dir.has_root() && !dir.is_absolute())
|
||||
&& (!dir.has_root() || dir.is_absolute())
|
||||
})
|
||||
.map(|dir| {
|
||||
if dir.is_absolute() {
|
||||
|
||||
@@ -58,29 +58,6 @@ fn requested_cwd_precedes_child_path() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keeps_long_and_lexical_paths_unchanged() {
|
||||
let temp = TempDir::new().expect("tempdir");
|
||||
let mut cwd = temp.path().to_path_buf();
|
||||
while cwd.to_string_lossy().len() <= 270 {
|
||||
cwd.push("long-working-directory-segment");
|
||||
}
|
||||
fs::create_dir_all(cwd.join("nested")).expect("create long cwd");
|
||||
let executable = cwd.join("tool.EXE");
|
||||
fs::write(&executable, b"fixture").expect("write executable fixture");
|
||||
let lexical = cwd.join("nested").join("..").join("tool.EXE");
|
||||
|
||||
assert_eq!(
|
||||
resolve_windows_executable(
|
||||
&[lexical.to_string_lossy().into_owned()],
|
||||
&cwd,
|
||||
&HashMap::new(),
|
||||
)
|
||||
.expect("resolve long lexical executable"),
|
||||
lexical
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keeps_extended_length_paths_unchanged() {
|
||||
let executable = dunce::canonicalize(std::env::current_exe().expect("current executable"))
|
||||
|
||||
@@ -12,6 +12,7 @@ use crate::process::WindowsProcessLaunch;
|
||||
use crate::winutil::format_last_error;
|
||||
use crate::winutil::quote_windows_arg;
|
||||
use crate::winutil::to_wide;
|
||||
use crate::winutil::to_win32_path_wide;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_utils_pty::PsuedoCon;
|
||||
@@ -143,7 +144,7 @@ pub fn spawn_conpty_process_as_user(
|
||||
0,
|
||||
EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT,
|
||||
env_block.as_ptr() as *mut c_void,
|
||||
to_wide(cwd).as_ptr(),
|
||||
to_win32_path_wide(cwd).as_ptr(),
|
||||
&si.StartupInfo,
|
||||
&mut pi,
|
||||
)
|
||||
|
||||
@@ -16,6 +16,7 @@ use crate::runner_pipe::find_runner_exe;
|
||||
use crate::runner_pipe::pipe_pair;
|
||||
use crate::winutil::quote_windows_arg;
|
||||
use crate::winutil::to_wide;
|
||||
use crate::winutil::to_win32_path_wide;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use std::ffi::c_void;
|
||||
@@ -342,7 +343,7 @@ pub(crate) fn spawn_runner_transport(
|
||||
);
|
||||
let mut cmdline_vec = to_wide(&runner_full_cmd);
|
||||
let exe_w = to_wide(&runner_cmdline);
|
||||
let cwd_w = to_wide(cwd);
|
||||
let cwd_w = to_win32_path_wide(cwd);
|
||||
let user_w = to_wide(&sandbox_creds.username);
|
||||
let domain_w = to_wide(".");
|
||||
let password_w = to_wide(&sandbox_creds.password);
|
||||
|
||||
@@ -1,27 +1,8 @@
|
||||
use super::inherit_path_env;
|
||||
use super::prepend_path;
|
||||
use super::reorder_pathext_for_stubs;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
|
||||
#[test]
|
||||
fn child_path_spelling_prevents_parent_path_inheritance() {
|
||||
let mut env = HashMap::from([
|
||||
("Path".to_string(), r"C:\child".to_string()),
|
||||
("PathExt".to_string(), ".BIN".to_string()),
|
||||
]);
|
||||
|
||||
inherit_path_env(&mut env);
|
||||
|
||||
assert_eq!(
|
||||
env,
|
||||
HashMap::from([
|
||||
("Path".to_string(), r"C:\child".to_string()),
|
||||
("PathExt".to_string(), ".BIN".to_string()),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_mutations_are_case_insensitive_and_do_not_fall_back_to_parent_values() {
|
||||
let mut env = HashMap::from([
|
||||
|
||||
@@ -4,6 +4,7 @@ use crate::proc_thread_attr::ProcThreadAttributeList;
|
||||
use crate::winutil::argv_to_command_line;
|
||||
use crate::winutil::format_last_error;
|
||||
use crate::winutil::to_wide;
|
||||
use crate::winutil::to_win32_path_wide;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
@@ -117,7 +118,7 @@ pub(crate) unsafe fn create_process_as_user(
|
||||
let env_block = make_env_block(env_map);
|
||||
let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?;
|
||||
let mut pi: PROCESS_INFORMATION = std::mem::zeroed();
|
||||
let cwd_wide = to_wide(cwd);
|
||||
let cwd_wide = to_win32_path_wide(cwd);
|
||||
let env_block_len = env_block.len();
|
||||
match stdio {
|
||||
Some((stdin_h, stdout_h, stderr_h)) => {
|
||||
|
||||
@@ -380,7 +380,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
|
||||
|
||||
let process_handle = Arc::new(StdMutex::new(Some(pi.hProcess)));
|
||||
let wait_handle = Arc::clone(&process_handle);
|
||||
let command_for_wait = launch.command.clone();
|
||||
let command_for_wait = launch.command;
|
||||
let hpc_for_wait = hpc_handle.clone();
|
||||
std::thread::spawn(move || {
|
||||
let _desktop = desktop;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use anyhow::Result;
|
||||
use std::ffi::OsStr;
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::path::Path;
|
||||
use windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER;
|
||||
use windows_sys::Win32::Foundation::GetLastError;
|
||||
use windows_sys::Win32::Foundation::HLOCAL;
|
||||
@@ -22,6 +23,32 @@ pub fn to_wide<S: AsRef<OsStr>>(s: S) -> Vec<u16> {
|
||||
v
|
||||
}
|
||||
|
||||
const MAX_PATH_UTF16_UNITS: usize = 260;
|
||||
const EXTENDED_PATH_PREFIX: &[u16] = &[b'\\' as u16, b'\\' as u16, b'?' as u16, b'\\' as u16];
|
||||
const DEVICE_PATH_PREFIX: &[u16] = &[b'\\' as u16, b'\\' as u16, b'.' as u16, b'\\' as u16];
|
||||
|
||||
/// Encodes a CreateProcess path, adding an extended-length prefix only when needed.
|
||||
pub(crate) fn to_win32_path_wide(path: &Path) -> Vec<u16> {
|
||||
let wide = path.as_os_str().encode_wide().collect::<Vec<_>>();
|
||||
if !path.is_absolute()
|
||||
|| wide.len() < MAX_PATH_UTF16_UNITS
|
||||
|| wide.starts_with(EXTENDED_PATH_PREFIX)
|
||||
|| wide.starts_with(DEVICE_PATH_PREFIX)
|
||||
{
|
||||
return wide.into_iter().chain([0]).collect();
|
||||
}
|
||||
|
||||
let mut extended = EXTENDED_PATH_PREFIX.to_vec();
|
||||
if wide.starts_with(&[b'\\' as u16, b'\\' as u16]) {
|
||||
extended.extend("UNC\\".encode_utf16());
|
||||
extended.extend_from_slice(&wide[2..]);
|
||||
} else {
|
||||
extended.extend(wide);
|
||||
}
|
||||
extended.push(0);
|
||||
extended
|
||||
}
|
||||
|
||||
/// Quote a single Windows command-line argument following the rules used by
|
||||
/// CommandLineToArgvW/CRT so that spaces, quotes, and backslashes are preserved.
|
||||
/// Reference behavior matches Rust std::process::Command on Windows.
|
||||
@@ -208,7 +235,10 @@ fn sid_bytes_from_string(sid_str: &str) -> Result<Vec<u8>> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::argv_to_command_line;
|
||||
use super::to_wide;
|
||||
use super::to_win32_path_wide;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
|
||||
#[test]
|
||||
fn argv_to_command_line_quotes_each_argument_independently() {
|
||||
@@ -238,4 +268,23 @@ mod tests {
|
||||
"pwsh.exe -Command \"Write-Output \\\"hello world\\\"\""
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_process_paths_add_extended_prefixes_only_for_long_paths() {
|
||||
let long_tail = ["long-working-directory-segment"; 10].join(r"\");
|
||||
let long_drive = format!(r"C:\{long_tail}");
|
||||
let long_unc = format!(r"\\localhost\C$\{long_tail}");
|
||||
|
||||
assert_eq!(
|
||||
to_win32_path_wide(Path::new(&long_drive)),
|
||||
to_wide(format!(r"\\?\{long_drive}"))
|
||||
);
|
||||
assert_eq!(
|
||||
to_win32_path_wide(Path::new(&long_unc)),
|
||||
to_wide(format!(r"\\?\UNC\{}", &long_unc[2..]))
|
||||
);
|
||||
for path in [r"C:\short", r"\\localhost\C$\short", r"\\?\C:\already"] {
|
||||
assert_eq!(to_win32_path_wide(Path::new(path)), to_wide(path));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user