From 5f371ba30ae4a4bf4527eb0729ab6b36e3a0c123 Mon Sep 17 00:00:00 2001 From: zm-oai Date: Wed, 23 Sep 2026 21:24:58 +0000 Subject: [PATCH] Fix no-reparse directory opens on Windows 10 (#47672) ## Why Windows 10 rejects the Object Manager drive-letter link when opening a directory with `OBJ_DONT_REPARSE`, preventing ordinary local-drive paths from opening successfully. ## What changed Resolve drive-letter aliases with `QueryDosDeviceW` before calling `NtCreateFile`. Accept only direct disk-volume mappings, rejecting other devices and volume subpaths. Leave filesystem components unresolved so `OBJ_DONT_REPARSE` continues to reject filesystem reparse points. ## Testing Add tests for drive-root opens and NT path construction using ordinary and verbatim drive paths, plus validation of accepted disk-volume mappings and rejected device or subpath mappings. GitOrigin-RevId: 2fe81913a7ac8d0122ec423db5750e80241def56 --- .../windows-sandbox-rs/src/no_reparse_dir.rs | 83 ++++++++++++++----- .../src/no_reparse_dir_tests.rs | 74 +++++++++++++++++ 2 files changed, 135 insertions(+), 22 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/no_reparse_dir.rs b/codex-rs/windows-sandbox-rs/src/no_reparse_dir.rs index da8cb2de0e..cb14621f6b 100644 --- a/codex-rs/windows-sandbox-rs/src/no_reparse_dir.rs +++ b/codex-rs/windows-sandbox-rs/src/no_reparse_dir.rs @@ -30,6 +30,7 @@ use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_NORMAL; use windows_sys::Win32::Storage::FileSystem::FILE_READ_DATA; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; +use windows_sys::Win32::Storage::FileSystem::QueryDosDeviceW; use windows_sys::Win32::System::IO::IO_STATUS_BLOCK; use windows_sys::Win32::System::IO::IO_STATUS_BLOCK_0; use windows_sys::Win32::System::Kernel::OBJ_CASE_INSENSITIVE; @@ -120,28 +121,7 @@ pub fn open_directory_no_reparse( share_access: u32, disposition: DirectoryOpenDisposition, ) -> Result { - validate_local_directory_path(path)?; - let source_offset = if matches!( - path.components().next(), - Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::VerbatimDisk(_)) - ) { - 4 - } else { - 0 - }; - // NtCreateFile expects an NT namespace path. Convert only the local-disk - // spellings accepted by provisioning, without canonicalizing and following - // the reparse points that OBJ_DONT_REPARSE must reject. - let source: Vec = path.as_os_str().encode_wide().collect(); - let mut nt_path: Vec = OsStr::new("\\??\\").encode_wide().collect(); - for unit in source.into_iter().skip(source_offset) { - nt_path.push(if unit == b'/' as u16 { - b'\\' as u16 - } else { - unit - }); - } - nt_path.push(0); + let mut nt_path = local_directory_nt_path(path)?; open_no_reparse( /*root_directory*/ 0, &mut nt_path, @@ -156,6 +136,65 @@ pub fn open_directory_no_reparse( .with_context(|| format!("open directory {}", path.display())) } +fn local_directory_nt_path(path: &Path) -> Result> { + validate_local_directory_path(path)?; + let source_offset = if matches!( + path.components().next(), + Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::VerbatimDisk(_)) + ) { + 4 + } else { + 0 + }; + // Windows 10 also rejects the Object Manager drive-letter link under + // OBJ_DONT_REPARSE. Resolve that alias only; never canonicalize filesystem + // components, which must still be checked by NtCreateFile. + let source: Vec = path.as_os_str().encode_wide().collect(); + let drive = [source[source_offset], b':' as u16, 0]; + let mut nt_path = vec![0_u16; 32_768]; + let length = + unsafe { QueryDosDeviceW(drive.as_ptr(), nt_path.as_mut_ptr(), nt_path.len() as u32) }; + if length == 0 { + return Err(std::io::Error::last_os_error()).context("resolve directory drive"); + } + nt_path.truncate( + nt_path + .iter() + .position(|unit| *unit == 0) + .unwrap_or(length as usize), + ); + ensure!( + is_disk_volume(&nt_path), + "directory drive must resolve directly to a disk volume" + ); + for unit in source.into_iter().skip(source_offset + 2) { + nt_path.push(if unit == b'/' as u16 { + b'\\' as u16 + } else { + unit + }); + } + nt_path.push(0); + Ok(nt_path) +} + +fn is_disk_volume(target: &[u16]) -> bool { + let Some((prefix, number)) = target.split_at_checked(br"\Device\HarddiskVolume".len()) else { + return false; + }; + // Do not pass other devices or volume subpaths to NtCreateFile. + prefix + .iter() + .zip(br"\Device\HarddiskVolume") + .all(|(&unit, expected)| { + u8::try_from(unit).is_ok_and(|unit| unit.eq_ignore_ascii_case(expected)) + }) + && !number.is_empty() + && number + .iter() + .all(|unit| (u16::from(b'0')..=u16::from(b'9')).contains(unit)) +} + /// Keeps a directory nonempty, preventing in-place reparse conversion while the /// returned handle is held. The fresh file cannot be renamed or deleted by other /// opens and uses delete-on-close cleanup after the guard and its duplicates close. diff --git a/codex-rs/windows-sandbox-rs/src/no_reparse_dir_tests.rs b/codex-rs/windows-sandbox-rs/src/no_reparse_dir_tests.rs index 829552e577..1ff5454def 100644 --- a/codex-rs/windows-sandbox-rs/src/no_reparse_dir_tests.rs +++ b/codex-rs/windows-sandbox-rs/src/no_reparse_dir_tests.rs @@ -2,17 +2,23 @@ use std::fs; use std::os::windows::io::OwnedHandle; use std::path::Path; use std::process::Command; +use windows_sys::Win32::Storage::FileSystem::FILE_READ_ATTRIBUTES; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; +use windows_sys::Win32::Storage::FileSystem::FILE_TRAVERSE; use windows_sys::Win32::Storage::FileSystem::READ_CONTROL; use windows_sys::Win32::Storage::FileSystem::WRITE_DAC; use anyhow::Result; +use pretty_assertions::assert_eq; use super::DirectoryOpenDisposition; +use super::is_disk_volume; +use super::local_directory_nt_path; use super::open_directory_no_reparse; use super::validate_local_directory_path; +use windows_sys::Win32::Storage::FileSystem::QueryDosDeviceW; fn open_for_acl(path: &Path, disposition: DirectoryOpenDisposition) -> Result { open_directory_no_reparse( @@ -54,6 +60,74 @@ fn creates_and_opens_plain_directory_leaf() -> Result<()> { Ok(()) } +#[test] +fn opens_drive_root_without_following_filesystem_reparses() -> Result<()> { + let temporary = tempfile::tempdir()?; + let directory = temporary.path().canonicalize()?; + let root = directory + .ancestors() + .last() + .expect("absolute path has a root"); + // canonicalize gives a verbatim drive path; exercise both supported forms. + let ordinary = root + .to_string_lossy() + .trim_start_matches(r"\\?\") + .to_owned(); + let drive: Vec<_> = ordinary.encode_utf16().take(2).chain(Some(0)).collect(); + let mut device = vec![0; 32_768]; + if unsafe { QueryDosDeviceW(drive.as_ptr(), device.as_mut_ptr(), device.len() as u32) } == 0 { + return Err(std::io::Error::last_os_error().into()); + } + device.truncate( + device + .iter() + .position(|unit| *unit == 0) + .expect("QueryDosDeviceW returns a NUL-terminated target"), + ); + assert!(is_disk_volume(&device)); + for path in [root, Path::new(&ordinary)] { + for suffix in ["", "no-reparse-test"] { + let mut expected = device.clone(); + expected.extend(format!(r"\{suffix}").encode_utf16()); + expected.push(0); + assert_eq!(local_directory_nt_path(&path.join(suffix))?, expected); + } + drop(open_directory_no_reparse( + path, + FILE_READ_ATTRIBUTES | FILE_TRAVERSE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + DirectoryOpenDisposition::OpenExisting, + )?); + } + Ok(()) +} + +#[test] +fn disk_volume_mapping_excludes_other_devices_and_subpaths() { + for accepted in [ + r"\Device\HarddiskVolume0", + r"\Device\HarddiskVolume1", + r"\device\harddiskvolume123", + ] { + assert!(is_disk_volume(&accepted.encode_utf16().collect::>())); + } + for rejected in [ + r"\Device\NamedPipe", + r"\Device\NamedPipe\pipe", + r"\Device\HarddiskVolume", + r"\Device\HarddiskVolume1\directory", + r"\Device\HarddiskVolume1suffix", + r"\Device\HarddiskVolumeShadowCopy1", + r"\Device\CdRom0", + r"\??\C:\directory", + ] { + assert!( + !is_disk_volume(&rejected.encode_utf16().collect::>()), + "accepted {rejected}" + ); + } +} + #[test] fn rejects_final_directory_junction() -> Result<()> { let temporary = tempfile::tempdir()?;