mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
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
This commit is contained in:
@@ -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<OwnedHandle> {
|
||||
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<u16> = path.as_os_str().encode_wide().collect();
|
||||
let mut nt_path: Vec<u16> = 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<Vec<u16>> {
|
||||
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<u16> = 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.
|
||||
|
||||
@@ -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<OwnedHandle> {
|
||||
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::<Vec<_>>()));
|
||||
}
|
||||
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::<Vec<_>>()),
|
||||
"accepted {rejected}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_final_directory_junction() -> Result<()> {
|
||||
let temporary = tempfile::tempdir()?;
|
||||
|
||||
Reference in New Issue
Block a user