From 50dda444bc384cffbb3ef48aad17a4ecf0c9d658 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Thu, 3 Sep 2026 01:47:42 +0000 Subject: [PATCH 1/7] refactor(plugin): preserve Windows wide paths for typed helpers --- .github/workflows/native-windows.yml | 2 +- plugins/codex-security/native/README.md | 4 + plugins/codex-security/native/build.mts | 20 +- .../native/examples/windows-wide-launcher.rs | 90 ++++++++ .../native/proof-windows-wide.mts | 208 ++++++++++++++++++ .../codex-security/native/proof-windows.mts | 11 + plugins/codex-security/native/src/windows.rs | 85 ++++++- .../codex-security/native/windows-binding.mts | 5 + .../codex-security/native/windows-files.mts | 183 +++++++++++++++ 9 files changed, 605 insertions(+), 3 deletions(-) create mode 100644 plugins/codex-security/native/examples/windows-wide-launcher.rs create mode 100644 plugins/codex-security/native/proof-windows-wide.mts create mode 100644 plugins/codex-security/native/windows-files.mts diff --git a/.github/workflows/native-windows.yml b/.github/workflows/native-windows.yml index 258ddf40ae..9de21022bc 100644 --- a/.github/workflows/native-windows.yml +++ b/.github/workflows/native-windows.yml @@ -51,7 +51,7 @@ jobs: run: | cargo fmt --check if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - cargo clippy --locked --target ${{ matrix.target }} -- -D warnings + cargo clippy --locked --target ${{ matrix.target }} --lib --example windows-wide-launcher -- -D warnings - name: Build and verify with Node.js 22 run: | node build.mjs diff --git a/plugins/codex-security/native/README.md b/plugins/codex-security/native/README.md index bbc3175dee..cd25fb4783 100644 --- a/plugins/codex-security/native/README.md +++ b/plugins/codex-security/native/README.md @@ -39,6 +39,8 @@ Windows uses `windows-binding.mts` and the same Rust crate. `WindowsHandle` owns The binding exposes synchronous file and directory creation, attributes and reparse tags, identity and final/opened names, read/write/seek/size/EOF/flush, exact-handle rename and deletion, and exclusive whole-file locking. Rust's `File` supplies ordinary I/O, cursor-preserving truncation, `sync_all` for flush, and locks. Calls return numeric Windows errors, including 6 for closed handles and 33 for nonblocking lock contention. Buffer ranges, path encoding, and 64-bit seek arguments are checked before use. Overlapped handles are unsupported because pending operations could retain native buffers beyond the call. Path authorization, ancestor traversal, and reparse-point policy remain the caller's responsibility. +Four additional operations avoid Node's lossy Windows string conversions. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryNames` returns every name as UTF-16LE; iteration failures return their Windows error and an empty array. `windows-files.mts` composes these operations and the existing handles into typed filesystem helpers; product commands do not use this adapter yet. + Build on Windows after compiling the TypeScript tools, then run: ```sh @@ -53,6 +55,8 @@ The `native-windows` workflow builds x64 and arm64 with MSVC and a static CRT. I node --expose-gc plugins/codex-security/native/proof-windows.mjs python plugins/codex-security/scripts ``` +The build also compiles the test-only `windows-wide-launcher` Rust example. It starts a Node proof child with lone surrogates in arguments, environment values, and its working directory. That child checks complete directory iteration, distinct surrogate and replacement-character files, canonical paths, bounded reads, output truncation, and recursive long paths through the typed adapter. The launcher cleans up the wide fixtures and is never included in the uploaded or bundled native payloads. + ## Package inputs The `native-artifacts` workflow calls all three platform workflows and combines their eight verified payloads into `native-universal-`. PR validation jobs share one artifact assembled by `node-ci`; release and standalone validation runs assemble their own. The standalone MCP builder and npm package include the same complete `mcp/native` tree; neither compiles nor downloads code at runtime. diff --git a/plugins/codex-security/native/build.mts b/plugins/codex-security/native/build.mts index 4b4b61663a..ca07cd0c01 100644 --- a/plugins/codex-security/native/build.mts +++ b/plugins/codex-security/native/build.mts @@ -40,7 +40,14 @@ const flags = [ ]; const target = resolve(root, process.env["CARGO_TARGET_DIR"] ?? "target"); const args = ["build", "--release", "--locked"]; -if (windowsTarget !== undefined) args.push("--target", windowsTarget); +if (windowsTarget !== undefined) + args.push( + "--target", + windowsTarget, + "--lib", + "--example", + "windows-wide-launcher", + ); execFileSync("cargo", args, { cwd: root, stdio: "inherit", @@ -63,4 +70,15 @@ const library = join( checkPrivatePaths(readFileSync(library), [root, cargoHome, sysroot, target]); mkdirSync(output, { recursive: true }); copyFileSync(library, binaryPath); +if (windowsTarget !== undefined) + copyFileSync( + join( + target, + windowsTarget, + "release", + "examples", + "windows-wide-launcher.exe", + ), + join(output, "windows-wide-launcher.exe"), + ); console.log(`Built ${nativeTarget} Node-API 8 primitives.`); diff --git a/plugins/codex-security/native/examples/windows-wide-launcher.rs b/plugins/codex-security/native/examples/windows-wide-launcher.rs new file mode 100644 index 0000000000..b916cbcb6f --- /dev/null +++ b/plugins/codex-security/native/examples/windows-wide-launcher.rs @@ -0,0 +1,90 @@ +// Test-only launcher: Node's Windows startup has already replaced lone surrogates. +#[cfg(not(windows))] +fn main() {} + +#[cfg(windows)] +fn main() -> std::io::Result<()> { + use std::{ + env, + ffi::OsString, + fs, io, + os::windows::ffi::OsStringExt, + path::{Path, PathBuf}, + process::Command, + }; + + fn raw(prefix: &str, unit: u16) -> OsString { + OsString::from_wide(&prefix.encode_utf16().chain([unit]).collect::>()) + } + + fn run(node: OsString, script: OsString, root: &Path) -> io::Result<()> { + let cwd = root.join(raw("cwd-", 0xd800)); + fs::create_dir(&cwd)?; + let replacement = root.join("cwd-\u{fffd}"); + fs::create_dir(&replacement)?; + fs::write(replacement.join("sentinel"), "replacement cwd untouched")?; + let names = [ + raw("high-", 0xd800), + raw("high-", 0xfffd), + raw("low-", 0xdc80), + raw("low-", 0xfffd), + raw("tail-", 0xdfff), + raw("tail-", 0xfffd), + OsString::from("unicode-馃攼-鏉变含"), + ]; + for (index, name) in names.iter().enumerate() { + fs::write(cwd.join(name), format!("sentinel-{index}"))?; + } + fs::create_dir(cwd.join("empty"))?; + let verbatim = fs::canonicalize(&cwd)?; + for (name, contents) in [ + ("trailing", "ordinary dot sibling"), + ("trailing.", "literal dot file"), + ("space", "ordinary space sibling"), + ("space ", "literal space file"), + ] { + fs::write(verbatim.join(name), contents)?; + } + let arguments = [ + raw("arg-high-", 0xd800), + raw("arg-low-", 0xdc80), + raw("arg-tail-", 0xdfff), + OsString::from("replacement-\u{fffd}"), + OsString::from("Unicode 馃攼 鏉变含"), + OsString::from(""), + OsString::from("space and\ttab"), + OsString::from("quoted \"value\" and trailing\\"), + OsString::from("backslash\\\"quote"), + ]; + let status = Command::new(node) + .arg(script) + .arg("wide-worker") + .arg(root) + .args(arguments) + .current_dir(&cwd) + .env("CODEX_SECURITY_WIDE_VALUE", raw("value-", 0xd800)) + .env("CODEX_SECURITY_WIDE_EMPTY", "") + .env_remove("CODEX_SECURITY_WIDE_ABSENT") + .env(raw("CODEX_SECURITY_WIDE_NAME_", 0xdfff), "wide name value") + .env("CODEX_SECURITY_WIDE_LONG", "x".repeat(1024)) + .env("USERPROFILE", &cwd) + .status()?; + if !status.success() { + return Err(io::Error::other("Wide Windows child proof failed")); + } + if fs::read(replacement.join("sentinel"))? != b"replacement cwd untouched" { + return Err(io::Error::other("Replacement cwd was changed")); + } + Ok(()) + } + + let mut args = env::args_os().skip(1); + let node = args.next().expect("Node executable path"); + let script = args.next().expect("Windows wide proof script"); + let root = PathBuf::from(args.next().expect("Proof fixture directory")).join("wide-process"); + fs::create_dir(&root)?; + let result = run(node, script, &root); + let cleanup = fs::remove_dir_all(&root); + result?; + cleanup +} diff --git a/plugins/codex-security/native/proof-windows-wide.mts b/plugins/codex-security/native/proof-windows-wide.mts new file mode 100644 index 0000000000..bb67b01148 --- /dev/null +++ b/plugins/codex-security/native/proof-windows-wide.mts @@ -0,0 +1,208 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { join, win32 } from "node:path"; +import { fileURLToPath } from "node:url"; +import { output } from "./binding.mjs"; +import { loadWindowsBinding } from "./windows-binding.mjs"; +import { pathText, widePath, windowsFileSystem } from "./windows-files.mjs"; + +const self = fileURLToPath(import.meta.url); + +export function wideProcessProof(root: string): Record { + const child = spawnSync( + join(output, "windows-wide-launcher.exe"), + [process.execPath, self, root], + { encoding: "utf8", maxBuffer: Infinity, timeout: 30_000 }, + ); + assert.equal(child.error, undefined); + assert.equal(child.status, 0, child.stderr); + assert.equal(child.stderr, ""); + return JSON.parse(child.stdout) as Record; +} + +function worker(root: string): Record { + const native = loadWindowsBinding(); + const files = windowsFileSystem(native); + const cwd = win32.join(root, "cwd-\ud800"); + const expectedArguments = [ + "arg-high-\ud800", + "arg-low-\udc80", + "arg-tail-\udfff", + "replacement-\ufffd", + "Unicode 馃攼 鏉变含", + "", + "space and\ttab", + 'quoted "value" and trailing\\', + 'backslash\\"quote', + ]; + const arguments_ = native.windowsArguments().map(pathText); + assert.deepEqual(arguments_.slice(4), expectedArguments); + assert.equal(arguments_[2], "wide-worker"); + assert.equal(arguments_[3], root); + + function environment(name: string): Buffer | null { + return native.windowsEnvironment(widePath(name)); + } + assert.deepEqual( + environment("CODEX_SECURITY_WIDE_VALUE"), + widePath("value-\ud800"), + ); + assert.deepEqual(environment("CODEX_SECURITY_WIDE_EMPTY"), Buffer.alloc(0)); + assert.equal(environment("CODEX_SECURITY_WIDE_ABSENT"), null); + assert.deepEqual( + environment("CODEX_SECURITY_WIDE_NAME_\udfff"), + widePath("wide name value"), + ); + assert.deepEqual( + environment("CODEX_SECURITY_WIDE_LONG"), + widePath("x".repeat(1024)), + ); + assert.deepEqual(environment("USERPROFILE"), widePath(cwd)); + + function samePath(actual: Buffer, expected: string): void { + assert.equal( + win32.toNamespacedPath(pathText(actual)).toLowerCase(), + win32.toNamespacedPath(expected).toLowerCase(), + ); + } + samePath(files.absolute(widePath(".")), cwd); + samePath( + files.absolute(widePath("missing/../high-\ud800")), + win32.join(cwd, "high-\ud800"), + ); + samePath(files.absolute(widePath(cwd)), cwd); + const drive = win32.parse(cwd).root.slice(0, 2); + assert.match(drive, /^[a-z]:$/iu); + samePath( + files.absolute(widePath(`${drive}high-\ud800`)), + win32.join(cwd, "high-\ud800"), + ); + samePath( + files.absolute(widePath("\\rooted-\ud800")), + `${drive}\\rooted-\ud800`, + ); + samePath(files.realpath(widePath(".")), cwd); + + const names = [ + "high-\ud800", + "high-\ufffd", + "low-\udc80", + "low-\ufffd", + "tail-\udfff", + "tail-\ufffd", + "unicode-馃攼-鏉变含", + ]; + assert.deepEqual( + files.entries(widePath(".")).map(pathText).sort(), + [...names, "empty", "trailing", "trailing.", "space", "space "].sort(), + ); + assert.deepEqual(native.windowsDirectoryNames(widePath("empty")), { + error: 0, + value: [], + }); + assert.deepEqual(native.windowsDirectoryNames(widePath("missing")), { + error: 3, + value: [], + }); + const notDirectory = native.windowsDirectoryNames(widePath(names[0]!)); + assert.notEqual(notDirectory.error, 0); + assert(Number.isInteger(notDirectory.error)); + assert.deepEqual(notDirectory.value, []); + for (const [index, name] of names.entries()) { + const buffer = Buffer.alloc(64); + const length = files.readInto(widePath(name), buffer); + assert.equal(buffer.subarray(0, length).toString(), `sentinel-${index}`); + assert(files.stat(widePath(name)).isFile()); + assert(!files.stat(widePath(name), false).isSymbolicLink()); + samePath(files.realpath(widePath(name)), win32.join(cwd, name)); + } + assert(files.stat(widePath(".")).isDirectory()); + const bounded = Buffer.alloc(4); + assert.equal(files.readInto(widePath(names[0]!), bounded), 4); + assert.equal(bounded.toString(), "sent"); + + const rawOutput = widePath("output-\ud800"); + const replacementOutput = widePath("output-\ufffd"); + files.writeFile( + replacementOutput, + Buffer.from("replacement output untouched"), + ); + files.writeFile(rawOutput, Buffer.from("a longer initial output")); + files.writeFile(rawOutput, Buffer.from("short")); + const contents = Buffer.alloc(64); + assert.equal(files.readInto(rawOutput, contents), 5); + assert.equal(contents.subarray(0, 5).toString(), "short"); + files.writeFile(rawOutput, Buffer.alloc(0)); + assert.equal(files.readInto(rawOutput, contents), 0); + const replacementLength = files.readInto(replacementOutput, contents); + assert.equal( + contents.subarray(0, replacementLength).toString(), + "replacement output untouched", + ); + + for (const [name, literal, ordinary] of [ + ["trailing.", "literal dot file", "ordinary dot sibling"], + ["space ", "literal space file", "ordinary space sibling"], + ] as const) { + const exact = widePath(win32.toNamespacedPath(win32.join(cwd, name))); + assert.deepEqual(files.absolute(exact), exact); + const length = files.readInto(exact, contents); + assert.equal(contents.subarray(0, length).toString(), literal); + samePath(files.realpath(exact), pathText(exact)); + files.writeFile(exact, Buffer.from("updated literal file")); + const ordinaryLength = files.readInto( + widePath(name.slice(0, -1)), + contents, + ); + assert.equal(contents.subarray(0, ordinaryLength).toString(), ordinary); + const directory = widePath( + win32.toNamespacedPath(win32.join(cwd, `directory-${name}`)), + ); + files.mkdir(directory); + files.writeFile( + widePath(`${pathText(directory)}\\child`), + Buffer.from("literal directory"), + ); + assert.deepEqual(files.entries(directory).map(pathText), ["child"]); + const ordinaryDirectory = widePath( + win32.join(cwd, `directory-${name.slice(0, -1)}`), + ); + files.mkdir(ordinaryDirectory); + assert.deepEqual(files.entries(ordinaryDirectory), []); + } + + const longDirectory = win32.join( + cwd, + ...Array.from({ length: 6 }, (_, i) => `${i}-${"x".repeat(48)}`), + "directory-\udfff", + ); + assert(files.absolute(widePath(longDirectory)).length > 512); + files.mkdir(widePath(longDirectory)); + files.mkdir(widePath(longDirectory)); + assert(files.stat(widePath(longDirectory)).isDirectory()); + const longFile = widePath(win32.join(longDirectory, "file-\udc80")); + files.writeFile(longFile, Buffer.from("long raw path")); + const longLength = files.readInto(longFile, contents); + assert.equal(contents.subarray(0, longLength).toString(), "long raw path"); + samePath(files.realpath(longFile), pathText(longFile)); + + for (const malformed of [Buffer.from([0x61]), widePath("bad\0value")]) { + assert.throws(() => native.windowsEnvironment(malformed)); + assert.throws(() => native.windowsAbsolutePath(malformed)); + assert.throws(() => native.windowsDirectoryNames(malformed)); + } + return { + rawArgumentsAndCrtQuoting: true, + rawEnvironmentEmptyAndUnset: true, + rawCwdAndDriveRelativePaths: true, + completeWideDirectoryIteration: true, + distinctRawAndReplacementFiles: true, + canonicalPathsBoundedReadsAndTruncation: true, + verbatimTrailingDotsAndSpaces: true, + recursiveLongWideDirectories: true, + numericErrorsAndFfiRepresentations: true, + }; +} + +if (process.argv[2] === "wide-worker") + console.log(JSON.stringify(worker(process.argv[3]!))); diff --git a/plugins/codex-security/native/proof-windows.mts b/plugins/codex-security/native/proof-windows.mts index 6c19978267..0aed2bcc28 100644 --- a/plugins/codex-security/native/proof-windows.mts +++ b/plugins/codex-security/native/proof-windows.mts @@ -17,6 +17,8 @@ import { basename, join, win32 } from "node:path"; import { createInterface } from "node:readline"; import { fileURLToPath } from "node:url"; import { setImmediate } from "node:timers/promises"; +import { wideProcessProof } from "./proof-windows-wide.mjs"; +import { windowsFileSystem } from "./windows-files.mjs"; import { loadWindowsBinding, windowsFlags as flags, @@ -282,6 +284,14 @@ function handleProof(root: string) { assert(attributes.attributes & flags.FILE_ATTRIBUTE_REPARSE_POINT); assert(attributes.attributes & flags.FILE_ATTRIBUTE_DIRECTORY); assert.equal(attributes.reparseTag, 0xa0000003); + const files = windowsFileSystem(native); + const junctionStat = files.stat(pathBytes(ancestor), false); + assert(junctionStat.isDirectory()); + assert(junctionStat.isReparsePoint()); + assert(!junctionStat.isSymbolicLink()); + const targetStat = files.stat(pathBytes(ancestor)); + assert(targetStat.isDirectory()); + assert(!targetStat.isReparsePoint()); samePath( checked(junction.finalPath(flags.FILE_NAME_OPENED)).path, ancestor, @@ -662,6 +672,7 @@ if (process.argv[2] === "worker") { architecture: process.arch, nodeApi: 8, handles: handleProof(root), + wideProcessAndPaths: wideProcessProof(root), garbageCollectionClosesHandle: await ownershipProof(root), locks: await lockProof(root), pythonCompatibility: diff --git a/plugins/codex-security/native/src/windows.rs b/plugins/codex-security/native/src/windows.rs index aee44ba648..b5fd9d6f5e 100644 --- a/plugins/codex-security/native/src/windows.rs +++ b/plugins/codex-security/native/src/windows.rs @@ -1,10 +1,14 @@ use napi::bindgen_prelude::{BigInt, Buffer}; use napi_derive::napi; use std::{ + ffi::OsString, fs::{File, TryLockError}, io::{self, Read, Seek, SeekFrom, Write}, mem::{offset_of, size_of, MaybeUninit}, - os::windows::io::{AsRawHandle, FromRawHandle}, + os::windows::{ + ffi::{OsStrExt, OsStringExt}, + io::{AsRawHandle, FromRawHandle}, + }, ptr::{copy_nonoverlapping, null, null_mut}, }; use windows_sys::Win32::{ @@ -63,6 +67,85 @@ fn wide_path(bytes: Buffer) -> napi::Result> { Ok(path) } +fn wide_bytes(units: impl IntoIterator) -> Buffer { + units + .into_iter() + .flat_map(u16::to_le_bytes) + .collect::>() + .into() +} + +fn os_string(bytes: Buffer) -> napi::Result { + let path = wide_path(bytes)?; + Ok(OsString::from_wide(&path[..path.len() - 1])) +} + +#[napi(object)] +pub struct BufferResult { + pub error: u32, + pub value: Buffer, +} + +#[napi(object)] +pub struct DirectoryResult { + pub error: u32, + pub value: Vec, +} + +#[napi] +pub fn windows_arguments() -> Vec { + std::env::args_os() + .map(|argument| wide_bytes(argument.encode_wide())) + .collect() +} + +#[napi] +pub fn windows_environment(name: Buffer) -> napi::Result> { + Ok(std::env::var_os(os_string(name)?).map(|value| wide_bytes(value.encode_wide()))) +} + +#[napi] +pub fn windows_absolute_path(path: Buffer) -> napi::Result { + let path = wide_path(path)?; + let mut absolute = vec![0_u16; 256]; + loop { + let capacity = u32::try_from(absolute.len()) + .map_err(|_| invalid("Absolute path exceeds the Win32 buffer size"))?; + let length = + unsafe { GetFullPathNameW(path.as_ptr(), capacity, absolute.as_mut_ptr(), null_mut()) }; + if length == 0 { + return Ok(BufferResult { + error: unsafe { GetLastError() }, + value: Vec::new().into(), + }); + } + if length < capacity { + return Ok(BufferResult { + error: 0, + value: wide_bytes(absolute[..length as usize].iter().copied()), + }); + } + absolute.resize(length as usize + 1, 0); + } +} + +#[napi] +pub fn windows_directory_names(path: Buffer) -> napi::Result { + let path = os_string(path)?; + let names = std::fs::read_dir(path).and_then(|entries| { + entries + .map(|entry| entry.map(|entry| wide_bytes(entry.file_name().encode_wide()))) + .collect::>>() + }); + Ok(match names { + Ok(value) => DirectoryResult { error: 0, value }, + Err(error) => DirectoryResult { + error: error.raw_os_error().unwrap() as u32, + value: Vec::new(), + }, + }) +} + fn io_range(buffer: &Buffer, offset: f64, length: f64) -> napi::Result<(usize, u32)> { if !offset.is_finite() || !length.is_finite() diff --git a/plugins/codex-security/native/windows-binding.mts b/plugins/codex-security/native/windows-binding.mts index b80facf215..99f352541a 100644 --- a/plugins/codex-security/native/windows-binding.mts +++ b/plugins/codex-security/native/windows-binding.mts @@ -28,6 +28,10 @@ export interface WindowsHandle { /** Paths are UTF-16LE code units without a terminator, including lone surrogates. */ export interface WindowsBinding { + windowsArguments(): Buffer[]; + windowsEnvironment(name: Buffer): Buffer | null; + windowsAbsolutePath(path: Buffer): WindowsResult; + windowsDirectoryNames(path: Buffer): WindowsResult; openWindowsFile( path: Buffer, access: number, @@ -47,6 +51,7 @@ export const windowsFlags = { FILE_SHARE_WRITE: 2, FILE_SHARE_DELETE: 4, CREATE_NEW: 1, + CREATE_ALWAYS: 2, OPEN_EXISTING: 3, OPEN_ALWAYS: 4, FILE_ATTRIBUTE_DIRECTORY: 0x00000010, diff --git a/plugins/codex-security/native/windows-files.mts b/plugins/codex-security/native/windows-files.mts new file mode 100644 index 0000000000..a11e11552e --- /dev/null +++ b/plugins/codex-security/native/windows-files.mts @@ -0,0 +1,183 @@ +import { win32 } from "node:path"; +import { + windowsFlags as flags, + type WindowsBinding, + type WindowsHandle, +} from "./windows-binding.mjs"; + +export const widePath = (path: string): Buffer => Buffer.from(path, "utf16le"); +export const pathText = (path: Buffer): string => path.toString("utf16le"); + +export function windowsFileSystem(native: WindowsBinding) { + function check(error: number, path: Buffer): void { + if (error === 0) return; + const code = new Map([ + [2, "ENOENT"], + [3, "ENOENT"], + [267, "ENOTDIR"], + [1921, "ELOOP"], + ]).get(error); + throw Object.assign( + new Error(`Windows filesystem error ${error}: ${pathText(path)}`), + { code, winerror: error }, + ); + } + + function absolute(path: Buffer): Buffer { + // GetFullPathNameW normalizes even explicit verbatim paths. + if (pathText(path).startsWith("\\\\?\\")) return path; + const result = native.windowsAbsolutePath(path); + check(result.error, path); + return result.value; + } + + function open( + path: Buffer, + access = 0, + disposition: number = flags.OPEN_EXISTING, + follow = true, + ): WindowsHandle { + const result = native.openWindowsFile( + absolute(path), + access, + flags.FILE_SHARE_READ | flags.FILE_SHARE_WRITE | flags.FILE_SHARE_DELETE, + disposition, + flags.FILE_FLAG_BACKUP_SEMANTICS | + (follow ? 0 : flags.FILE_FLAG_OPEN_REPARSE_POINT), + ); + check(result.error, path); + return result.handle!; + } + + function finalPath(path: Buffer): Buffer { + const handle = open(path); + try { + const result = handle.finalPath(0); + check(result.error, path); + return result.path; + } finally { + check(handle.close(), path); + } + } + + function realpath(path: Buffer): Buffer { + const textPath = pathText(path).replaceAll("/", "\\"); + const normalized = widePath( + textPath.slice(0, 8).toUpperCase() === "\\\\?\\UNC\\" + ? textPath.slice(0, 8) + + win32.normalize(`\\\\${textPath.slice(8)}`).slice(2) + : win32.normalize(textPath), + ); + const resolved = finalPath(normalized); + if (pathText(normalized).startsWith("\\\\?\\")) return resolved; + const text = pathText(resolved); + const shortened = text.startsWith("\\\\?\\UNC\\") + ? `\\\\${text.slice(8)}` + : text.startsWith("\\\\?\\") + ? text.slice(4) + : text; + // Like pathlib, remove the device prefix only if that spelling resolves too. + const candidate = widePath(shortened); + try { + if (finalPath(candidate).equals(resolved)) return candidate; + } catch { + // Extended paths can be valid when their ordinary spelling is not. + } + return resolved; + } + + function stat(path: Buffer, follow = true) { + const handle = open( + path, + flags.FILE_READ_ATTRIBUTES, + flags.OPEN_EXISTING, + follow, + ); + try { + const info = handle.attributes(); + check(info.error, path); + const type = handle.fileType(); + check(type.error, path); + const link = !follow && info.reparseTag === 0xa000000c; + const directory = + (info.attributes & flags.FILE_ATTRIBUTE_DIRECTORY) !== 0; + return { + isDirectory: () => !link && directory, + isFile: () => !link && !directory && type.value === 1, + isSymbolicLink: () => link, + isReparsePoint: () => + (info.attributes & flags.FILE_ATTRIBUTE_REPARSE_POINT) !== 0, + }; + } finally { + check(handle.close(), path); + } + } + + function entries(path: Buffer): Buffer[] { + const result = native.windowsDirectoryNames(absolute(path)); + check(result.error, path); + return result.value; + } + + function mkdir(path: Buffer): void { + const resolved = absolute(path); + const parent = widePath(win32.dirname(pathText(resolved))); + let error = native.createWindowsDirectory(resolved); + if (error === 3 && !parent.equals(resolved)) { + mkdir(parent); + error = native.createWindowsDirectory(resolved); + } + if (error !== 0) { + try { + if (stat(resolved).isDirectory()) return; + } catch { + // Report the original creation error. + } + check(error, path); + } + } + + function readInto(path: Buffer, buffer: Buffer): number { + const handle = open(path, flags.GENERIC_READ); + let length = 0; + try { + while (length < buffer.length) { + const result = handle.read( + buffer, + length, + Math.min(buffer.length - length, 0xffffffff), + ); + check(result.error, path); + if (result.value === 0) break; + length += result.value; + } + } finally { + check(handle.close(), path); + } + return length; + } + + function writeFile(path: Buffer, buffer: Buffer): void { + const handle = open(path, flags.GENERIC_WRITE, flags.CREATE_ALWAYS); + let offset = 0; + try { + while (offset < buffer.length) { + const result = handle.write( + buffer, + offset, + Math.min(buffer.length - offset, 0xffffffff), + ); + check(result.error, path); + if (result.value === 0) + throw new Error( + `Windows file write made no progress: ${pathText(path)}`, + ); + offset += result.value; + } + } finally { + check(handle.close(), path); + } + } + + return { absolute, realpath, stat, entries, mkdir, readInto, writeFile }; +} From 10ece770d4d5eb9928694731a25d769648b0b6ad Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Thu, 3 Sep 2026 01:55:24 +0000 Subject: [PATCH 2/7] fix(plugin): namespace native Windows filesystem operations --- .../codex-security/native/windows-files.mts | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/plugins/codex-security/native/windows-files.mts b/plugins/codex-security/native/windows-files.mts index a11e11552e..638432fe97 100644 --- a/plugins/codex-security/native/windows-files.mts +++ b/plugins/codex-security/native/windows-files.mts @@ -31,6 +31,18 @@ export function windowsFileSystem(native: WindowsBinding) { return result.value; } + function operationPath(path: Buffer): Buffer { + const resolved = absolute(path); + const text = pathText(resolved); + if (text.startsWith("\\\\?\\") || text.startsWith("\\\\.\\")) + return resolved; + return widePath( + text.startsWith("\\\\") + ? `\\\\?\\UNC\\${text.slice(2)}` + : `\\\\?\\${text}`, + ); + } + function open( path: Buffer, access = 0, @@ -38,7 +50,7 @@ export function windowsFileSystem(native: WindowsBinding) { follow = true, ): WindowsHandle { const result = native.openWindowsFile( - absolute(path), + operationPath(path), access, flags.FILE_SHARE_READ | flags.FILE_SHARE_WRITE | flags.FILE_SHARE_DELETE, disposition, @@ -114,7 +126,7 @@ export function windowsFileSystem(native: WindowsBinding) { } function entries(path: Buffer): Buffer[] { - const result = native.windowsDirectoryNames(absolute(path)); + const result = native.windowsDirectoryNames(operationPath(path)); check(result.error, path); return result.value; } @@ -122,10 +134,10 @@ export function windowsFileSystem(native: WindowsBinding) { function mkdir(path: Buffer): void { const resolved = absolute(path); const parent = widePath(win32.dirname(pathText(resolved))); - let error = native.createWindowsDirectory(resolved); + let error = native.createWindowsDirectory(operationPath(resolved)); if (error === 3 && !parent.equals(resolved)) { mkdir(parent); - error = native.createWindowsDirectory(resolved); + error = native.createWindowsDirectory(operationPath(resolved)); } if (error !== 0) { try { From 1a2ecda637c47cba0a05c02e1e707ca60dd868df Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Thu, 3 Sep 2026 02:39:43 +0000 Subject: [PATCH 3/7] fix(plugin): preserve Windows path and directory metadata --- plugins/codex-security/native/README.md | 2 +- .../native/examples/windows-wide-launcher.rs | 118 +++++++++++++++--- .../native/proof-windows-wide.mts | 44 ++++++- .../codex-security/native/proof-windows.mts | 8 ++ plugins/codex-security/native/src/windows.rs | 40 +++++- .../codex-security/native/windows-binding.mts | 3 + .../codex-security/native/windows-files.mts | 39 ++++-- 7 files changed, 223 insertions(+), 31 deletions(-) diff --git a/plugins/codex-security/native/README.md b/plugins/codex-security/native/README.md index cd25fb4783..d24bfb6f54 100644 --- a/plugins/codex-security/native/README.md +++ b/plugins/codex-security/native/README.md @@ -39,7 +39,7 @@ Windows uses `windows-binding.mts` and the same Rust crate. `WindowsHandle` owns The binding exposes synchronous file and directory creation, attributes and reparse tags, identity and final/opened names, read/write/seek/size/EOF/flush, exact-handle rename and deletion, and exclusive whole-file locking. Rust's `File` supplies ordinary I/O, cursor-preserving truncation, `sync_all` for flush, and locks. Calls return numeric Windows errors, including 6 for closed handles and 33 for nonblocking lock contention. Buffer ranges, path encoding, and 64-bit seek arguments are checked before use. Overlapped handles are unsupported because pending operations could retain native buffers beyond the call. Path authorization, ancestor traversal, and reparse-point policy remain the caller's responsibility. -Four additional operations avoid Node's lossy Windows string conversions. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryNames` returns every name as UTF-16LE; iteration failures return their Windows error and an empty array. `windows-files.mts` composes these operations and the existing handles into typed filesystem helpers; product commands do not use this adapter yet. +Five additional operations avoid Node's lossy Windows string conversions. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryNames` returns every name as UTF-16LE; iteration failures return their Windows error and an empty array. `windowsDirectoryEntries` also returns the cached directory attribute, including directory reparse points, without opening each entry. `windows-files.mts` exposes this through `entriesWithTypes` alongside the existing names-only `entries`; product commands do not use this adapter yet. Build on Windows after compiling the TypeScript tools, then run: diff --git a/plugins/codex-security/native/examples/windows-wide-launcher.rs b/plugins/codex-security/native/examples/windows-wide-launcher.rs index b916cbcb6f..d93da419f4 100644 --- a/plugins/codex-security/native/examples/windows-wide-launcher.rs +++ b/plugins/codex-security/native/examples/windows-wide-launcher.rs @@ -8,15 +8,91 @@ fn main() -> std::io::Result<()> { env, ffi::OsString, fs, io, - os::windows::ffi::OsStringExt, + mem::size_of, + os::windows::ffi::{OsStrExt, OsStringExt}, path::{Path, PathBuf}, process::Command, + ptr::null_mut, }; + use windows_sys::Win32::Security::*; fn raw(prefix: &str, unit: u16) -> OsString { OsString::from_wide(&prefix.encode_utf16().chain([unit]).collect::>()) } + fn deny_file_access(path: &Path, operation: impl FnOnce() -> io::Result<()>) -> io::Result<()> { + let path = path + .as_os_str() + .encode_wide() + .chain([0]) + .collect::>(); + let mut length = 0; + unsafe { + GetFileSecurityW( + path.as_ptr(), + DACL_SECURITY_INFORMATION, + null_mut(), + 0, + &mut length, + ) + }; + if length == 0 { + return Err(io::Error::last_os_error()); + } + let mut saved = vec![0_usize; (length as usize).div_ceil(size_of::())]; + let mut control = 0; + let mut revision = 0; + if unsafe { + GetFileSecurityW( + path.as_ptr(), + DACL_SECURITY_INFORMATION, + saved.as_mut_ptr().cast(), + length, + &mut length, + ) == 0 + || GetSecurityDescriptorControl( + saved.as_mut_ptr().cast(), + &mut control, + &mut revision, + ) == 0 + } { + return Err(io::Error::last_os_error()); + } + let mut acl = ACL::default(); + let mut descriptor = SECURITY_DESCRIPTOR::default(); + let descriptor = (&mut descriptor as *mut SECURITY_DESCRIPTOR).cast(); + if unsafe { + InitializeAcl(&mut acl, size_of::() as u32, ACL_REVISION) == 0 + || InitializeSecurityDescriptor(descriptor, 1) == 0 + || SetSecurityDescriptorDacl(descriptor, 1, &acl, 0) == 0 + || SetFileSecurityW( + path.as_ptr(), + DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION, + descriptor, + ) == 0 + } { + return Err(io::Error::last_os_error()); + } + let result = operation(); + let protection = if control & SE_DACL_PROTECTED != 0 { + PROTECTED_DACL_SECURITY_INFORMATION + } else { + UNPROTECTED_DACL_SECURITY_INFORMATION + }; + // Restore the owned fixture's original DACL even if the child proof fails. + if unsafe { + SetFileSecurityW( + path.as_ptr(), + DACL_SECURITY_INFORMATION | protection, + saved.as_mut_ptr().cast(), + ) + } == 0 + { + return Err(io::Error::last_os_error()); + } + result + } + fn run(node: OsString, script: OsString, root: &Path) -> io::Result<()> { let cwd = root.join(raw("cwd-", 0xd800)); fs::create_dir(&cwd)?; @@ -36,6 +112,11 @@ fn main() -> std::io::Result<()> { fs::write(cwd.join(name), format!("sentinel-{index}"))?; } fs::create_dir(cwd.join("empty"))?; + fs::create_dir(cwd.join(raw("directory-", 0xdc80)))?; + std::os::windows::fs::symlink_file(&names[0], cwd.join("file-link"))?; + std::os::windows::fs::symlink_dir("empty", cwd.join("directory-link"))?; + let denied = cwd.join(raw("denied-", 0xdfff)); + fs::write(&denied, "directory enumeration does not open this file")?; let verbatim = fs::canonicalize(&cwd)?; for (name, contents) in [ ("trailing", "ordinary dot sibling"), @@ -56,22 +137,25 @@ fn main() -> std::io::Result<()> { OsString::from("quoted \"value\" and trailing\\"), OsString::from("backslash\\\"quote"), ]; - let status = Command::new(node) - .arg(script) - .arg("wide-worker") - .arg(root) - .args(arguments) - .current_dir(&cwd) - .env("CODEX_SECURITY_WIDE_VALUE", raw("value-", 0xd800)) - .env("CODEX_SECURITY_WIDE_EMPTY", "") - .env_remove("CODEX_SECURITY_WIDE_ABSENT") - .env(raw("CODEX_SECURITY_WIDE_NAME_", 0xdfff), "wide name value") - .env("CODEX_SECURITY_WIDE_LONG", "x".repeat(1024)) - .env("USERPROFILE", &cwd) - .status()?; - if !status.success() { - return Err(io::Error::other("Wide Windows child proof failed")); - } + deny_file_access(&denied, || { + let status = Command::new(node) + .arg(script) + .arg("wide-worker") + .arg(root) + .args(arguments) + .current_dir(&cwd) + .env("CODEX_SECURITY_WIDE_VALUE", raw("value-", 0xd800)) + .env("CODEX_SECURITY_WIDE_EMPTY", "") + .env_remove("CODEX_SECURITY_WIDE_ABSENT") + .env(raw("CODEX_SECURITY_WIDE_NAME_", 0xdfff), "wide name value") + .env("CODEX_SECURITY_WIDE_LONG", "x".repeat(1024)) + .env("USERPROFILE", &cwd) + .status()?; + if !status.success() { + return Err(io::Error::other("Wide Windows child proof failed")); + } + Ok(()) + })?; if fs::read(replacement.join("sentinel"))? != b"replacement cwd untouched" { return Err(io::Error::other("Replacement cwd was changed")); } diff --git a/plugins/codex-security/native/proof-windows-wide.mts b/plugins/codex-security/native/proof-windows-wide.mts index bb67b01148..3baf9817ae 100644 --- a/plugins/codex-security/native/proof-windows-wide.mts +++ b/plugins/codex-security/native/proof-windows-wide.mts @@ -94,8 +94,48 @@ function worker(root: string): Record { ]; assert.deepEqual( files.entries(widePath(".")).map(pathText).sort(), - [...names, "empty", "trailing", "trailing.", "space", "space "].sort(), + [ + ...names, + "empty", + "trailing", + "trailing.", + "space", + "space ", + "directory-\udc80", + "file-link", + "directory-link", + "denied-\udfff", + ].sort(), ); + const listed = files.entriesWithTypes(widePath(".")); + assert.deepEqual( + listed.map((entry) => pathText(entry.name)).sort(), + files.entries(widePath(".")).map(pathText).sort(), + ); + const directories = listed + .filter((entry) => entry.isDirectory()) + .map((entry) => pathText(entry.name)) + .sort(); + assert.deepEqual(directories, [ + "directory-link", + "directory-\udc80", + "empty", + ]); + assert.throws(() => files.stat(widePath("denied-\udfff")), { winerror: 5 }); + assert.equal( + listed + .find((entry) => entry.name.equals(widePath("denied-\udfff"))) + ?.isDirectory(), + false, + ); + assert.deepEqual(native.windowsDirectoryEntries(widePath("empty")), { + error: 0, + value: [], + }); + assert.deepEqual(native.windowsDirectoryEntries(widePath("missing")), { + error: 3, + value: [], + }); assert.deepEqual(native.windowsDirectoryNames(widePath("empty")), { error: 0, value: [], @@ -190,12 +230,14 @@ function worker(root: string): Record { assert.throws(() => native.windowsEnvironment(malformed)); assert.throws(() => native.windowsAbsolutePath(malformed)); assert.throws(() => native.windowsDirectoryNames(malformed)); + assert.throws(() => native.windowsDirectoryEntries(malformed)); } return { rawArgumentsAndCrtQuoting: true, rawEnvironmentEmptyAndUnset: true, rawCwdAndDriveRelativePaths: true, completeWideDirectoryIteration: true, + cachedDirectoryAttributesWithoutFileAccess: true, distinctRawAndReplacementFiles: true, canonicalPathsBoundedReadsAndTruncation: true, verbatimTrailingDotsAndSpaces: true, diff --git a/plugins/codex-security/native/proof-windows.mts b/plugins/codex-security/native/proof-windows.mts index 0aed2bcc28..5400acc280 100644 --- a/plugins/codex-security/native/proof-windows.mts +++ b/plugins/codex-security/native/proof-windows.mts @@ -285,6 +285,14 @@ function handleProof(root: string) { assert(attributes.attributes & flags.FILE_ATTRIBUTE_DIRECTORY); assert.equal(attributes.reparseTag, 0xa0000003); const files = windowsFileSystem(native); + assert( + files + .entriesWithTypes(pathBytes(root)) + .find((entry) => + entry.name.equals(Buffer.from(basename(ancestor), "utf16le")), + ) + ?.isDirectory(), + ); const junctionStat = files.stat(pathBytes(ancestor), false); assert(junctionStat.isDirectory()); assert(junctionStat.isReparsePoint()); diff --git a/plugins/codex-security/native/src/windows.rs b/plugins/codex-security/native/src/windows.rs index b5fd9d6f5e..ff4b7186b0 100644 --- a/plugins/codex-security/native/src/windows.rs +++ b/plugins/codex-security/native/src/windows.rs @@ -7,6 +7,7 @@ use std::{ mem::{offset_of, size_of, MaybeUninit}, os::windows::{ ffi::{OsStrExt, OsStringExt}, + fs::MetadataExt, io::{AsRawHandle, FromRawHandle}, }, ptr::{copy_nonoverlapping, null, null_mut}, @@ -92,6 +93,18 @@ pub struct DirectoryResult { pub value: Vec, } +#[napi(object)] +pub struct DirectoryEntry { + pub name: Buffer, + pub is_directory: bool, +} + +#[napi(object)] +pub struct DirectoryEntriesResult { + pub error: u32, + pub value: Vec, +} + #[napi] pub fn windows_arguments() -> Vec { std::env::args_os() @@ -131,15 +144,32 @@ pub fn windows_absolute_path(path: Buffer) -> napi::Result { #[napi] pub fn windows_directory_names(path: Buffer) -> napi::Result { + let result = windows_directory_entries(path)?; + Ok(DirectoryResult { + error: result.error, + value: result.value.into_iter().map(|entry| entry.name).collect(), + }) +} + +#[napi] +pub fn windows_directory_entries(path: Buffer) -> napi::Result { let path = os_string(path)?; - let names = std::fs::read_dir(path).and_then(|entries| { + let entries = std::fs::read_dir(path).and_then(|entries| { entries - .map(|entry| entry.map(|entry| wide_bytes(entry.file_name().encode_wide()))) + .map(|entry| { + let entry = entry?; + // Windows DirEntry metadata comes from cached WIN32_FIND_DATAW. + let attributes = entry.metadata()?.file_attributes(); + Ok(DirectoryEntry { + name: wide_bytes(entry.file_name().encode_wide()), + is_directory: attributes & FILE_ATTRIBUTE_DIRECTORY != 0, + }) + }) .collect::>>() }); - Ok(match names { - Ok(value) => DirectoryResult { error: 0, value }, - Err(error) => DirectoryResult { + Ok(match entries { + Ok(value) => DirectoryEntriesResult { error: 0, value }, + Err(error) => DirectoryEntriesResult { error: error.raw_os_error().unwrap() as u32, value: Vec::new(), }, diff --git a/plugins/codex-security/native/windows-binding.mts b/plugins/codex-security/native/windows-binding.mts index 99f352541a..7561c3bfca 100644 --- a/plugins/codex-security/native/windows-binding.mts +++ b/plugins/codex-security/native/windows-binding.mts @@ -32,6 +32,9 @@ export interface WindowsBinding { windowsEnvironment(name: Buffer): Buffer | null; windowsAbsolutePath(path: Buffer): WindowsResult; windowsDirectoryNames(path: Buffer): WindowsResult; + windowsDirectoryEntries( + path: Buffer, + ): WindowsResult<{ name: Buffer; isDirectory: boolean }[]>; openWindowsFile( path: Buffer, access: number, diff --git a/plugins/codex-security/native/windows-files.mts b/plugins/codex-security/native/windows-files.mts index 638432fe97..d2df3fbcb8 100644 --- a/plugins/codex-security/native/windows-files.mts +++ b/plugins/codex-security/native/windows-files.mts @@ -73,13 +73,20 @@ export function windowsFileSystem(native: WindowsBinding) { } function realpath(path: Buffer): Buffer { + // Match CPython's Windows normalization of UNC and device prefixes. const textPath = pathText(path).replaceAll("/", "\\"); - const normalized = widePath( - textPath.slice(0, 8).toUpperCase() === "\\\\?\\UNC\\" - ? textPath.slice(0, 8) + - win32.normalize(`\\\\${textPath.slice(8)}`).slice(2) - : win32.normalize(textPath), - ); + let normalizedText = textPath; + if (textPath.startsWith("\\\\")) { + const first = textPath.indexOf("\\", 2); + const end = first === -1 ? -1 : textPath.indexOf("\\", first + 1); + if (end !== -1) { + const tail = textPath.slice(end + 1).replace(/^\\+/u, ""); + normalizedText = + textPath.slice(0, end + 1) + + win32.normalize(`\\${tail}`).slice(1).replace(/\\+$/u, ""); + } + } else normalizedText = win32.normalize(textPath); + const normalized = widePath(normalizedText); const resolved = finalPath(normalized); if (pathText(normalized).startsWith("\\\\?\\")) return resolved; const text = pathText(resolved); @@ -131,6 +138,15 @@ export function windowsFileSystem(native: WindowsBinding) { return result.value; } + function entriesWithTypes(path: Buffer) { + const result = native.windowsDirectoryEntries(operationPath(path)); + check(result.error, path); + return result.value.map(({ name, isDirectory }) => ({ + name, + isDirectory: () => isDirectory, + })); + } + function mkdir(path: Buffer): void { const resolved = absolute(path); const parent = widePath(win32.dirname(pathText(resolved))); @@ -191,5 +207,14 @@ export function windowsFileSystem(native: WindowsBinding) { } } - return { absolute, realpath, stat, entries, mkdir, readInto, writeFile }; + return { + absolute, + realpath, + stat, + entries, + entriesWithTypes, + mkdir, + readInto, + writeFile, + }; } From ef56277aa03dd362d9e4d0ade542e36c42b6b242 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Thu, 3 Sep 2026 02:56:08 +0000 Subject: [PATCH 4/7] fix(native): preserve cached Windows entry types and path normalization --- plugins/codex-security/native/README.md | 2 +- .../native/examples/windows-wide-launcher.rs | 4 + .../native/proof-windows-wide.mts | 25 ++++ .../codex-security/native/proof-windows.mts | 15 +- plugins/codex-security/native/src/windows.rs | 128 +++++++++++++++--- .../codex-security/native/windows-binding.mts | 4 +- .../codex-security/native/windows-files.mts | 20 ++- 7 files changed, 164 insertions(+), 34 deletions(-) diff --git a/plugins/codex-security/native/README.md b/plugins/codex-security/native/README.md index d24bfb6f54..87bee840d7 100644 --- a/plugins/codex-security/native/README.md +++ b/plugins/codex-security/native/README.md @@ -39,7 +39,7 @@ Windows uses `windows-binding.mts` and the same Rust crate. `WindowsHandle` owns The binding exposes synchronous file and directory creation, attributes and reparse tags, identity and final/opened names, read/write/seek/size/EOF/flush, exact-handle rename and deletion, and exclusive whole-file locking. Rust's `File` supplies ordinary I/O, cursor-preserving truncation, `sync_all` for flush, and locks. Calls return numeric Windows errors, including 6 for closed handles and 33 for nonblocking lock contention. Buffer ranges, path encoding, and 64-bit seek arguments are checked before use. Overlapped handles are unsupported because pending operations could retain native buffers beyond the call. Path authorization, ancestor traversal, and reparse-point policy remain the caller's responsibility. -Five additional operations avoid Node's lossy Windows string conversions. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryNames` returns every name as UTF-16LE; iteration failures return their Windows error and an empty array. `windowsDirectoryEntries` also returns the cached directory attribute, including directory reparse points, without opening each entry. `windows-files.mts` exposes this through `entriesWithTypes` alongside the existing names-only `entries`; product commands do not use this adapter yet. +Five additional operations avoid Node's lossy Windows string conversions. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryNames` returns every name as UTF-16LE; iteration failures return their Windows error and an empty array. `windowsDirectoryEntries` also returns the cached directory attribute and exact symbolic-link status, distinguishing directory symlinks from junctions without opening each entry. `windows-files.mts` exposes these through `entriesWithTypes` with `isDirectory()` and `isSymbolicLink()` alongside the existing names-only `entries`; product commands do not use this adapter yet. Build on Windows after compiling the TypeScript tools, then run: diff --git a/plugins/codex-security/native/examples/windows-wide-launcher.rs b/plugins/codex-security/native/examples/windows-wide-launcher.rs index d93da419f4..f804baab6d 100644 --- a/plugins/codex-security/native/examples/windows-wide-launcher.rs +++ b/plugins/codex-security/native/examples/windows-wide-launcher.rs @@ -115,6 +115,10 @@ fn main() -> std::io::Result<()> { fs::create_dir(cwd.join(raw("directory-", 0xdc80)))?; std::os::windows::fs::symlink_file(&names[0], cwd.join("file-link"))?; std::os::windows::fs::symlink_dir("empty", cwd.join("directory-link"))?; + std::os::windows::fs::symlink_dir( + raw("missing-", 0xdfff), + cwd.join("dangling-directory-link"), + )?; let denied = cwd.join(raw("denied-", 0xdfff)); fs::write(&denied, "directory enumeration does not open this file")?; let verbatim = fs::canonicalize(&cwd)?; diff --git a/plugins/codex-security/native/proof-windows-wide.mts b/plugins/codex-security/native/proof-windows-wide.mts index 3baf9817ae..a1f2cd5ab3 100644 --- a/plugins/codex-security/native/proof-windows-wide.mts +++ b/plugins/codex-security/native/proof-windows-wide.mts @@ -104,6 +104,7 @@ function worker(root: string): Record { "directory-\udc80", "file-link", "directory-link", + "dangling-directory-link", "denied-\udfff", ].sort(), ); @@ -117,10 +118,18 @@ function worker(root: string): Record { .map((entry) => pathText(entry.name)) .sort(); assert.deepEqual(directories, [ + "dangling-directory-link", "directory-link", "directory-\udc80", "empty", ]); + assert.deepEqual( + listed + .filter((entry) => entry.isSymbolicLink()) + .map((entry) => pathText(entry.name)) + .sort(), + ["dangling-directory-link", "directory-link", "file-link"], + ); assert.throws(() => files.stat(widePath("denied-\udfff")), { winerror: 5 }); assert.equal( listed @@ -136,6 +145,10 @@ function worker(root: string): Record { error: 3, value: [], }); + assert.deepEqual(native.windowsDirectoryEntries(Buffer.alloc(0)), { + error: 3, + value: [], + }); assert.deepEqual(native.windowsDirectoryNames(widePath("empty")), { error: 0, value: [], @@ -155,7 +168,17 @@ function worker(root: string): Record { assert(files.stat(widePath(name)).isFile()); assert(!files.stat(widePath(name), false).isSymbolicLink()); samePath(files.realpath(widePath(name)), win32.join(cwd, name)); + for (const input of [ + `${name}/`, + `${name}\\`, + `${drive}${name}/`, + `${drive}.\\..\\${name}\\`, + `${win32.join(cwd, name)}\\`, + ]) { + samePath(files.realpath(widePath(input)), win32.join(cwd, name)); + } } + samePath(files.realpath(widePath(`${drive}///`)), `${drive}\\`); assert(files.stat(widePath(".")).isDirectory()); const bounded = Buffer.alloc(4); assert.equal(files.readInto(widePath(names[0]!), bounded), 4); @@ -238,6 +261,8 @@ function worker(root: string): Record { rawCwdAndDriveRelativePaths: true, completeWideDirectoryIteration: true, cachedDirectoryAttributesWithoutFileAccess: true, + cachedSymlinkTagsIncludingDanglingDirectories: true, + existingFilesWithTrailingSeparators: true, distinctRawAndReplacementFiles: true, canonicalPathsBoundedReadsAndTruncation: true, verbatimTrailingDotsAndSpaces: true, diff --git a/plugins/codex-security/native/proof-windows.mts b/plugins/codex-security/native/proof-windows.mts index 5400acc280..c3512f2f99 100644 --- a/plugins/codex-security/native/proof-windows.mts +++ b/plugins/codex-security/native/proof-windows.mts @@ -285,14 +285,13 @@ function handleProof(root: string) { assert(attributes.attributes & flags.FILE_ATTRIBUTE_DIRECTORY); assert.equal(attributes.reparseTag, 0xa0000003); const files = windowsFileSystem(native); - assert( - files - .entriesWithTypes(pathBytes(root)) - .find((entry) => - entry.name.equals(Buffer.from(basename(ancestor), "utf16le")), - ) - ?.isDirectory(), - ); + const junctionEntry = files + .entriesWithTypes(pathBytes(root)) + .find((entry) => + entry.name.equals(Buffer.from(basename(ancestor), "utf16le")), + ); + assert(junctionEntry?.isDirectory()); + assert.equal(junctionEntry?.isSymbolicLink(), false); const junctionStat = files.stat(pathBytes(ancestor), false); assert(junctionStat.isDirectory()); assert(junctionStat.isReparsePoint()); diff --git a/plugins/codex-security/native/src/windows.rs b/plugins/codex-security/native/src/windows.rs index ff4b7186b0..14f84e2c8e 100644 --- a/plugins/codex-security/native/src/windows.rs +++ b/plugins/codex-security/native/src/windows.rs @@ -7,15 +7,16 @@ use std::{ mem::{offset_of, size_of, MaybeUninit}, os::windows::{ ffi::{OsStrExt, OsStringExt}, - fs::MetadataExt, io::{AsRawHandle, FromRawHandle}, }, + path::Path, ptr::{copy_nonoverlapping, null, null_mut}, }; use windows_sys::Win32::{ Foundation::{ - GetLastError, SetLastError, ERROR_INVALID_HANDLE, ERROR_INVALID_PARAMETER, - ERROR_LOCK_VIOLATION, HANDLE, INVALID_HANDLE_VALUE, + GetLastError, SetLastError, ERROR_FILE_NOT_FOUND, ERROR_INVALID_HANDLE, + ERROR_INVALID_PARAMETER, ERROR_LOCK_VIOLATION, ERROR_NO_MORE_FILES, + ERROR_PATH_NOT_FOUND, HANDLE, INVALID_HANDLE_VALUE, }, Storage::FileSystem::*, }; @@ -97,6 +98,7 @@ pub struct DirectoryResult { pub struct DirectoryEntry { pub name: Buffer, pub is_directory: bool, + pub is_symbolic_link: bool, } #[napi(object)] @@ -154,25 +156,107 @@ pub fn windows_directory_names(path: Buffer) -> napi::Result { #[napi] pub fn windows_directory_entries(path: Buffer) -> napi::Result { let path = os_string(path)?; - let entries = std::fs::read_dir(path).and_then(|entries| { - entries - .map(|entry| { - let entry = entry?; - // Windows DirEntry metadata comes from cached WIN32_FIND_DATAW. - let attributes = entry.metadata()?.file_attributes(); - Ok(DirectoryEntry { - name: wide_bytes(entry.file_name().encode_wide()), - is_directory: attributes & FILE_ATTRIBUTE_DIRECTORY != 0, - }) - }) - .collect::>>() - }); - Ok(match entries { - Ok(value) => DirectoryEntriesResult { error: 0, value }, - Err(error) => DirectoryEntriesResult { - error: error.raw_os_error().unwrap() as u32, - value: Vec::new(), - }, + let failure = |error| DirectoryEntriesResult { + error, + value: Vec::new(), + }; + if path.is_empty() { + return Ok(failure(ERROR_PATH_NOT_FOUND)); + } + let pattern = Path::new(&path).join("*"); + let pattern = directory_search_path(wide_bytes(pattern.as_os_str().encode_wide()))?; + if pattern.error != 0 { + return Ok(failure(pattern.error)); + } + let pattern = wide_path(pattern.value)?; + let mut data = WIN32_FIND_DATAW::default(); + let handle = unsafe { FindFirstFileW(pattern.as_ptr(), &mut data) }; + if handle == INVALID_HANDLE_VALUE { + let error = unsafe { GetLastError() }; + // Like read_dir, a successful empty search is an empty iterator. + return Ok(failure(if error == ERROR_FILE_NOT_FOUND { + 0 + } else { + error + })); + } + struct FindHandle(HANDLE); + impl Drop for FindHandle { + fn drop(&mut self) { + unsafe { FindClose(self.0) }; + } + } + let handle = FindHandle(handle); + let mut value = Vec::new(); + loop { + let end = data.cFileName.iter().position(|unit| *unit == 0).unwrap(); + let name = &data.cFileName[..end]; + if name != [b'.' as u16] && name != [b'.' as u16, b'.' as u16] { + value.push(DirectoryEntry { + name: wide_bytes(name.iter().copied()), + is_directory: data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0, + is_symbolic_link: data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != 0 + && data.dwReserved0 == 0xa000000c, + }); + } + if unsafe { FindNextFileW(handle.0, &mut data) } == 0 { + let error = unsafe { GetLastError() }; + return Ok(if error == ERROR_NO_MORE_FILES { + DirectoryEntriesResult { error: 0, value } + } else { + failure(error) + }); + } + } +} + +fn directory_search_path(path: Buffer) -> napi::Result { + const SEP: u16 = b'\\' as u16; + const ALT: u16 = b'/' as u16; + const COLON: u16 = b':' as u16; + const DOT: u16 = b'.' as u16; + const QUERY: u16 = b'?' as u16; + let units = wide_path(path.to_vec().into())?; + let verbatim = + units.starts_with(&[SEP, SEP, QUERY, SEP]) || units.starts_with(&[SEP, QUERY, QUERY, SEP]); + let short_absolute = units.len() < 248 + && (matches!(units.as_slice(), [drive, COLON, SEP | ALT, ..] if ![SEP, ALT].contains(drive)) + || matches!(units.as_slice(), [SEP | ALT, SEP | ALT, ..])); + if verbatim || short_absolute { + return Ok(BufferResult { + error: 0, + value: path, + }); + } + // Match read_dir's conversion of relative and long search paths to verbatim paths. + let absolute = windows_absolute_path(path)?; + if absolute.error != 0 { + return Ok(absolute); + } + let units = wide_path(absolute.value)?; + let units = &units[..units.len() - 1]; + let (prefix, rest): (&[u16], &[u16]) = match units { + [_, COLON, SEP, ..] => (&[SEP, SEP, QUERY, SEP], units), + [SEP, SEP, DOT, SEP, rest @ ..] => (&[SEP, SEP, QUERY, SEP], rest), + [SEP, SEP, QUERY, SEP, ..] | [SEP, QUERY, QUERY, SEP, ..] => (&[], units), + [SEP, SEP, rest @ ..] => ( + &[ + SEP, + SEP, + QUERY, + SEP, + b'U' as u16, + b'N' as u16, + b'C' as u16, + SEP, + ], + rest, + ), + _ => (&[], units), + }; + Ok(BufferResult { + error: 0, + value: wide_bytes(prefix.iter().chain(rest).copied()), }) } diff --git a/plugins/codex-security/native/windows-binding.mts b/plugins/codex-security/native/windows-binding.mts index 7561c3bfca..36c9d90bc9 100644 --- a/plugins/codex-security/native/windows-binding.mts +++ b/plugins/codex-security/native/windows-binding.mts @@ -34,7 +34,9 @@ export interface WindowsBinding { windowsDirectoryNames(path: Buffer): WindowsResult; windowsDirectoryEntries( path: Buffer, - ): WindowsResult<{ name: Buffer; isDirectory: boolean }[]>; + ): WindowsResult< + { name: Buffer; isDirectory: boolean; isSymbolicLink: boolean }[] + >; openWindowsFile( path: Buffer, access: number, diff --git a/plugins/codex-security/native/windows-files.mts b/plugins/codex-security/native/windows-files.mts index d2df3fbcb8..d188a53102 100644 --- a/plugins/codex-security/native/windows-files.mts +++ b/plugins/codex-security/native/windows-files.mts @@ -85,7 +85,22 @@ export function windowsFileSystem(native: WindowsBinding) { textPath.slice(0, end + 1) + win32.normalize(`\\${tail}`).slice(1).replace(/\\+$/u, ""); } - } else normalizedText = win32.normalize(textPath); + } else { + if (textPath[1] === ":" && textPath.slice(2, 4) === ".\\") { + // Windows normpath retains the first drive-relative dot until a parent consumes it. + const parts = ["."]; + for (const part of textPath.slice(4).split("\\")) { + if (part === "" || part === ".") continue; + if (part === ".." && parts.length && parts.at(-1) !== "..") + parts.pop(); + else parts.push(part); + } + normalizedText = textPath.slice(0, 2) + parts.join("\\"); + } else normalizedText = win32.normalize(textPath); + const root = win32.parse(normalizedText).root; + normalizedText = + root + normalizedText.slice(root.length).replace(/\\+$/u, ""); + } const normalized = widePath(normalizedText); const resolved = finalPath(normalized); if (pathText(normalized).startsWith("\\\\?\\")) return resolved; @@ -141,9 +156,10 @@ export function windowsFileSystem(native: WindowsBinding) { function entriesWithTypes(path: Buffer) { const result = native.windowsDirectoryEntries(operationPath(path)); check(result.error, path); - return result.value.map(({ name, isDirectory }) => ({ + return result.value.map(({ name, isDirectory, isSymbolicLink }) => ({ name, isDirectory: () => isDirectory, + isSymbolicLink: () => isSymbolicLink, })); } From d89ce971f83a9a7d04e54dcc6f0fee2822c40a75 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Fri, 4 Sep 2026 22:34:05 +0000 Subject: [PATCH 5/7] refactor(native): simplify Windows path and directory handling --- .github/workflows/native-windows.yml | 4 + plugins/codex-security/native/README.md | 6 +- .../native/examples/windows-wide-launcher.rs | 127 ++++----------- .../native/proof-windows-wide.mts | 101 +++++++++--- .../codex-security/native/proof-windows.mts | 2 +- plugins/codex-security/native/src/windows.rs | 144 +++--------------- .../codex-security/native/windows-binding.mts | 1 - .../codex-security/native/windows-files.mts | 49 ++---- .../native/windows-files.test.mts | 62 ++++++++ 9 files changed, 219 insertions(+), 277 deletions(-) create mode 100644 plugins/codex-security/native/windows-files.test.mts diff --git a/.github/workflows/native-windows.yml b/.github/workflows/native-windows.yml index 9de21022bc..c51516d61c 100644 --- a/.github/workflows/native-windows.yml +++ b/.github/workflows/native-windows.yml @@ -58,6 +58,8 @@ jobs: if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } node check.mjs if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + node --test windows-files.test.mjs + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } $nativeNode = (Get-Command node).Source $env:PATH = "" & $nativeNode --expose-gc proof-windows.mjs @@ -74,6 +76,8 @@ jobs: run: | node check.mjs if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + node --test windows-files.test.mjs + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } $nativeNode = (Get-Command node).Source $env:PATH = "" & $nativeNode --expose-gc proof-windows.mjs diff --git a/plugins/codex-security/native/README.md b/plugins/codex-security/native/README.md index 87bee840d7..809ef2adcf 100644 --- a/plugins/codex-security/native/README.md +++ b/plugins/codex-security/native/README.md @@ -39,7 +39,9 @@ Windows uses `windows-binding.mts` and the same Rust crate. `WindowsHandle` owns The binding exposes synchronous file and directory creation, attributes and reparse tags, identity and final/opened names, read/write/seek/size/EOF/flush, exact-handle rename and deletion, and exclusive whole-file locking. Rust's `File` supplies ordinary I/O, cursor-preserving truncation, `sync_all` for flush, and locks. Calls return numeric Windows errors, including 6 for closed handles and 33 for nonblocking lock contention. Buffer ranges, path encoding, and 64-bit seek arguments are checked before use. Overlapped handles are unsupported because pending operations could retain native buffers beyond the call. Path authorization, ancestor traversal, and reparse-point policy remain the caller's responsibility. -Five additional operations avoid Node's lossy Windows string conversions. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryNames` returns every name as UTF-16LE; iteration failures return their Windows error and an empty array. `windowsDirectoryEntries` also returns the cached directory attribute and exact symbolic-link status, distinguishing directory symlinks from junctions without opening each entry. `windows-files.mts` exposes these through `entriesWithTypes` with `isDirectory()` and `isSymbolicLink()` alongside the existing names-only `entries`; product commands do not use this adapter yet. +Four additional operations preserve Windows strings at the Node boundary. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryEntries` uses `std::fs::read_dir` and cached `DirEntry::file_type()` values without opening each child; names remain UTF-16LE, and construction or iteration failures return their numeric Windows error and an empty array. Directory symlinks and junctions have both directory and symbolic-link flags. The typed adapter exposes this one enumerator through `entriesWithTypes`; product commands do not use it yet. + +`windows-files.mts` leaves ordinary absolute-path resolution and canonicalization to `GetFullPathNameW` and `GetFinalPathNameByHandleW`, trimming trailing separators below the root. Its small verbatim-path normalizer preserves drive and UNC share roots when resolving dot segments, including literal trailing dots and spaces. `stat(path, false)` retains exact symbolic-link and reparse-point metadata so callers can reject junction traversal independently of the enumerator's link label. The SDK's public runtime floor remains Node 22.13.0. Node 20.0.0 is an additional native-foundation compatibility proof; it does not change the SDK engine requirement. Build on Windows after compiling the TypeScript tools, then run: @@ -55,7 +57,7 @@ The `native-windows` workflow builds x64 and arm64 with MSVC and a static CRT. I node --expose-gc plugins/codex-security/native/proof-windows.mjs python plugins/codex-security/scripts ``` -The build also compiles the test-only `windows-wide-launcher` Rust example. It starts a Node proof child with lone surrogates in arguments, environment values, and its working directory. That child checks complete directory iteration, distinct surrogate and replacement-character files, canonical paths, bounded reads, output truncation, and recursive long paths through the typed adapter. The launcher cleans up the wide fixtures and is never included in the uploaded or bundled native payloads. +The build also compiles the test-only `windows-wide-launcher` Rust example. It starts a Node proof child with lone surrogates in arguments, environment values, and its working directory. That child checks complete directory iteration, distinct surrogate and replacement-character files, canonical paths, bounded reads, output truncation, and recursive long paths through the typed adapter. A Rust file guard with sharing disabled remains open while the child enumerates its name; an explicit data read fails with a sharing violation. Attribute-only access is not blocked by Windows file sharing. The child also compares `node:fs` string paths and WTF-8 buffers against the Rust-created names on both pinned Node versions. Node string conversion can still replace lone surrogates before or after libuv; the buffer results distinguish libuv support from JavaScript string support. Root-normalization tables run on the same matrix. The launcher cleans up the wide fixtures and is never included in the uploaded or bundled native payloads. ## Package inputs diff --git a/plugins/codex-security/native/examples/windows-wide-launcher.rs b/plugins/codex-security/native/examples/windows-wide-launcher.rs index f804baab6d..cada8b3b2c 100644 --- a/plugins/codex-security/native/examples/windows-wide-launcher.rs +++ b/plugins/codex-security/native/examples/windows-wide-launcher.rs @@ -8,97 +8,25 @@ fn main() -> std::io::Result<()> { env, ffi::OsString, fs, io, - mem::size_of, - os::windows::ffi::{OsStrExt, OsStringExt}, + os::windows::{ffi::OsStringExt, fs::OpenOptionsExt}, path::{Path, PathBuf}, process::Command, - ptr::null_mut, }; - use windows_sys::Win32::Security::*; fn raw(prefix: &str, unit: u16) -> OsString { OsString::from_wide(&prefix.encode_utf16().chain([unit]).collect::>()) } - fn deny_file_access(path: &Path, operation: impl FnOnce() -> io::Result<()>) -> io::Result<()> { - let path = path - .as_os_str() - .encode_wide() - .chain([0]) - .collect::>(); - let mut length = 0; - unsafe { - GetFileSecurityW( - path.as_ptr(), - DACL_SECURITY_INFORMATION, - null_mut(), - 0, - &mut length, - ) - }; - if length == 0 { - return Err(io::Error::last_os_error()); - } - let mut saved = vec![0_usize; (length as usize).div_ceil(size_of::())]; - let mut control = 0; - let mut revision = 0; - if unsafe { - GetFileSecurityW( - path.as_ptr(), - DACL_SECURITY_INFORMATION, - saved.as_mut_ptr().cast(), - length, - &mut length, - ) == 0 - || GetSecurityDescriptorControl( - saved.as_mut_ptr().cast(), - &mut control, - &mut revision, - ) == 0 - } { - return Err(io::Error::last_os_error()); - } - let mut acl = ACL::default(); - let mut descriptor = SECURITY_DESCRIPTOR::default(); - let descriptor = (&mut descriptor as *mut SECURITY_DESCRIPTOR).cast(); - if unsafe { - InitializeAcl(&mut acl, size_of::() as u32, ACL_REVISION) == 0 - || InitializeSecurityDescriptor(descriptor, 1) == 0 - || SetSecurityDescriptorDacl(descriptor, 1, &acl, 0) == 0 - || SetFileSecurityW( - path.as_ptr(), - DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION, - descriptor, - ) == 0 - } { - return Err(io::Error::last_os_error()); - } - let result = operation(); - let protection = if control & SE_DACL_PROTECTED != 0 { - PROTECTED_DACL_SECURITY_INFORMATION - } else { - UNPROTECTED_DACL_SECURITY_INFORMATION - }; - // Restore the owned fixture's original DACL even if the child proof fails. - if unsafe { - SetFileSecurityW( - path.as_ptr(), - DACL_SECURITY_INFORMATION | protection, - saved.as_mut_ptr().cast(), - ) - } == 0 - { - return Err(io::Error::last_os_error()); - } - result - } - fn run(node: OsString, script: OsString, root: &Path) -> io::Result<()> { let cwd = root.join(raw("cwd-", 0xd800)); fs::create_dir(&cwd)?; let replacement = root.join("cwd-\u{fffd}"); fs::create_dir(&replacement)?; fs::write(replacement.join("sentinel"), "replacement cwd untouched")?; + fs::write( + replacement.join("high-\u{fffd}"), + "core replacement sentinel", + )?; let names = [ raw("high-", 0xd800), raw("high-", 0xfffd), @@ -119,8 +47,9 @@ fn main() -> std::io::Result<()> { raw("missing-", 0xdfff), cwd.join("dangling-directory-link"), )?; - let denied = cwd.join(raw("denied-", 0xdfff)); - fs::write(&denied, "directory enumeration does not open this file")?; + let locked = cwd.join(raw("locked-", 0xdfff)); + fs::write(&locked, "directory enumeration does not open this file")?; + fs::write(root.join(raw("parent-", 0xd800)), "parent sentinel")?; let verbatim = fs::canonicalize(&cwd)?; for (name, contents) in [ ("trailing", "ordinary dot sibling"), @@ -141,25 +70,27 @@ fn main() -> std::io::Result<()> { OsString::from("quoted \"value\" and trailing\\"), OsString::from("backslash\\\"quote"), ]; - deny_file_access(&denied, || { - let status = Command::new(node) - .arg(script) - .arg("wide-worker") - .arg(root) - .args(arguments) - .current_dir(&cwd) - .env("CODEX_SECURITY_WIDE_VALUE", raw("value-", 0xd800)) - .env("CODEX_SECURITY_WIDE_EMPTY", "") - .env_remove("CODEX_SECURITY_WIDE_ABSENT") - .env(raw("CODEX_SECURITY_WIDE_NAME_", 0xdfff), "wide name value") - .env("CODEX_SECURITY_WIDE_LONG", "x".repeat(1024)) - .env("USERPROFILE", &cwd) - .status()?; - if !status.success() { - return Err(io::Error::other("Wide Windows child proof failed")); - } - Ok(()) - })?; + let guard = fs::OpenOptions::new() + .read(true) + .share_mode(0) + .open(&locked)?; + let status = Command::new(node) + .arg(script) + .arg("wide-worker") + .arg(root) + .args(arguments) + .current_dir(&cwd) + .env("CODEX_SECURITY_WIDE_VALUE", raw("value-", 0xd800)) + .env("CODEX_SECURITY_WIDE_EMPTY", "") + .env_remove("CODEX_SECURITY_WIDE_ABSENT") + .env(raw("CODEX_SECURITY_WIDE_NAME_", 0xdfff), "wide name value") + .env("CODEX_SECURITY_WIDE_LONG", "x".repeat(1024)) + .env("USERPROFILE", &cwd) + .status()?; + if !status.success() { + return Err(io::Error::other("Wide Windows child proof failed")); + } + drop(guard); if fs::read(replacement.join("sentinel"))? != b"replacement cwd untouched" { return Err(io::Error::other("Replacement cwd was changed")); } diff --git a/plugins/codex-security/native/proof-windows-wide.mts b/plugins/codex-security/native/proof-windows-wide.mts index a1f2cd5ab3..8c2044f289 100644 --- a/plugins/codex-security/native/proof-windows-wide.mts +++ b/plugins/codex-security/native/proof-windows-wide.mts @@ -1,6 +1,7 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import { join, win32 } from "node:path"; +import { readFileSync, readdirSync, realpathSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { output } from "./binding.mjs"; import { loadWindowsBinding } from "./windows-binding.mjs"; @@ -92,8 +93,9 @@ function worker(root: string): Record { "tail-\ufffd", "unicode-馃攼-鏉变含", ]; + const listed = files.entriesWithTypes(widePath(".")); assert.deepEqual( - files.entries(widePath(".")).map(pathText).sort(), + listed.map((entry) => pathText(entry.name)).sort(), [ ...names, "empty", @@ -105,14 +107,17 @@ function worker(root: string): Record { "file-link", "directory-link", "dangling-directory-link", - "denied-\udfff", + "locked-\udfff", ].sort(), ); - const listed = files.entriesWithTypes(widePath(".")); - assert.deepEqual( - listed.map((entry) => pathText(entry.name)).sort(), - files.entries(widePath(".")).map(pathText).sort(), - ); + for (const spelling of [".", `${drive}.`, cwd, win32.toNamespacedPath(cwd)]) { + const result = native.windowsDirectoryEntries(widePath(spelling)); + assert.equal(result.error, 0); + assert.deepEqual( + result.value.map((entry) => pathText(entry.name)).sort(), + listed.map((entry) => pathText(entry.name)).sort(), + ); + } const directories = listed .filter((entry) => entry.isDirectory()) .map((entry) => pathText(entry.name)) @@ -130,10 +135,13 @@ function worker(root: string): Record { .sort(), ["dangling-directory-link", "directory-link", "file-link"], ); - assert.throws(() => files.stat(widePath("denied-\udfff")), { winerror: 5 }); + assert.throws( + () => files.readInto(widePath("locked-\udfff"), Buffer.alloc(1)), + { winerror: 32 }, + ); assert.equal( listed - .find((entry) => entry.name.equals(widePath("denied-\udfff"))) + .find((entry) => entry.name.equals(widePath("locked-\udfff"))) ?.isDirectory(), false, ); @@ -149,15 +157,7 @@ function worker(root: string): Record { error: 3, value: [], }); - assert.deepEqual(native.windowsDirectoryNames(widePath("empty")), { - error: 0, - value: [], - }); - assert.deepEqual(native.windowsDirectoryNames(widePath("missing")), { - error: 3, - value: [], - }); - const notDirectory = native.windowsDirectoryNames(widePath(names[0]!)); + const notDirectory = native.windowsDirectoryEntries(widePath(names[0]!)); assert.notEqual(notDirectory.error, 0); assert(Number.isInteger(notDirectory.error)); assert.deepEqual(notDirectory.value, []); @@ -172,13 +172,16 @@ function worker(root: string): Record { `${name}/`, `${name}\\`, `${drive}${name}/`, - `${drive}.\\..\\${name}\\`, `${win32.join(cwd, name)}\\`, ]) { samePath(files.realpath(widePath(input)), win32.join(cwd, name)); } } samePath(files.realpath(widePath(`${drive}///`)), `${drive}\\`); + samePath( + files.realpath(widePath(`${drive}.\\..\\parent-\ud800`)), + win32.join(root, "parent-\ud800"), + ); assert(files.stat(widePath(".")).isDirectory()); const bounded = Buffer.alloc(4); assert.equal(files.readInto(widePath(names[0]!), bounded), 4); @@ -226,12 +229,15 @@ function worker(root: string): Record { widePath(`${pathText(directory)}\\child`), Buffer.from("literal directory"), ); - assert.deepEqual(files.entries(directory).map(pathText), ["child"]); + assert.deepEqual( + files.entriesWithTypes(directory).map((entry) => pathText(entry.name)), + ["child"], + ); const ordinaryDirectory = widePath( win32.join(cwd, `directory-${name.slice(0, -1)}`), ); files.mkdir(ordinaryDirectory); - assert.deepEqual(files.entries(ordinaryDirectory), []); + assert.deepEqual(files.entriesWithTypes(ordinaryDirectory), []); } const longDirectory = win32.join( @@ -248,14 +254,65 @@ function worker(root: string): Record { const longLength = files.readInto(longFile, contents); assert.equal(contents.subarray(0, longLength).toString(), "long raw path"); samePath(files.realpath(longFile), pathText(longFile)); + const longEntries = native.windowsDirectoryEntries(widePath(longDirectory)); + assert.equal(longEntries.error, 0); + assert.deepEqual( + longEntries.value.map((entry) => pathText(entry.name)), + ["file-\udc80"], + ); for (const malformed of [Buffer.from([0x61]), widePath("bad\0value")]) { assert.throws(() => native.windowsEnvironment(malformed)); assert.throws(() => native.windowsAbsolutePath(malformed)); - assert.throws(() => native.windowsDirectoryNames(malformed)); assert.throws(() => native.windowsDirectoryEntries(malformed)); } + // libuv's WTF-8 support does not bypass Node's JS string conversion. + assert.equal( + readFileSync(win32.join(cwd, "high-\ud800"), "utf8"), + "core replacement sentinel", + ); + const rawName = Buffer.concat([ + Buffer.from("high-"), + Buffer.from([0xed, 0xa0, 0x80]), + ]); + const rawCwd = Buffer.concat([ + Buffer.from(win32.join(root, "cwd-")), + Buffer.from([0xed, 0xa0, 0x80]), + ]); + const rawFile = Buffer.concat([rawCwd, Buffer.from("\\"), rawName]); + function coreSupports(operation: () => boolean): boolean { + try { + return operation(); + } catch (error) { + assert.equal(typeof (error as NodeJS.ErrnoException).code, "string"); + return false; + } + } + const coreStringNames = coreSupports(() => + readdirSync(rawCwd).includes("high-\ud800"), + ); + const coreStringRealpath = coreSupports( + () => realpathSync.native(rawCwd) === cwd, + ); + assert.equal(coreStringNames, false); + assert.equal(coreStringRealpath, false); + const coreBufferRead = coreSupports( + () => readFileSync(rawFile, "utf8") === "sentinel-0", + ); + const coreBufferNames = coreSupports(() => + readdirSync(rawCwd, { encoding: "buffer" }).some((name) => + name.equals(rawName), + ), + ); + const coreBufferRealpath = coreSupports(() => + realpathSync.native(rawCwd, { encoding: "buffer" }).equals(rawCwd), + ); return { + nodeStringPathsReadReplacementSibling: true, + nodeStringNamesAndRealpathLoseSurrogates: true, + nodeWtf8BufferReads: coreBufferRead, + nodeWtf8BufferDirectoryNames: coreBufferNames, + nodeWtf8BufferRealpath: coreBufferRealpath, rawArgumentsAndCrtQuoting: true, rawEnvironmentEmptyAndUnset: true, rawCwdAndDriveRelativePaths: true, diff --git a/plugins/codex-security/native/proof-windows.mts b/plugins/codex-security/native/proof-windows.mts index c3512f2f99..dd62780292 100644 --- a/plugins/codex-security/native/proof-windows.mts +++ b/plugins/codex-security/native/proof-windows.mts @@ -291,7 +291,7 @@ function handleProof(root: string) { entry.name.equals(Buffer.from(basename(ancestor), "utf16le")), ); assert(junctionEntry?.isDirectory()); - assert.equal(junctionEntry?.isSymbolicLink(), false); + assert.equal(junctionEntry?.isSymbolicLink(), true); const junctionStat = files.stat(pathBytes(ancestor), false); assert(junctionStat.isDirectory()); assert(junctionStat.isReparsePoint()); diff --git a/plugins/codex-security/native/src/windows.rs b/plugins/codex-security/native/src/windows.rs index 14f84e2c8e..a6c6670103 100644 --- a/plugins/codex-security/native/src/windows.rs +++ b/plugins/codex-security/native/src/windows.rs @@ -2,21 +2,20 @@ use napi::bindgen_prelude::{BigInt, Buffer}; use napi_derive::napi; use std::{ ffi::OsString, - fs::{File, TryLockError}, + fs::{self, File, TryLockError}, io::{self, Read, Seek, SeekFrom, Write}, mem::{offset_of, size_of, MaybeUninit}, os::windows::{ ffi::{OsStrExt, OsStringExt}, + fs::FileTypeExt, io::{AsRawHandle, FromRawHandle}, }, - path::Path, ptr::{copy_nonoverlapping, null, null_mut}, }; use windows_sys::Win32::{ Foundation::{ - GetLastError, SetLastError, ERROR_FILE_NOT_FOUND, ERROR_INVALID_HANDLE, - ERROR_INVALID_PARAMETER, ERROR_LOCK_VIOLATION, ERROR_NO_MORE_FILES, - ERROR_PATH_NOT_FOUND, HANDLE, INVALID_HANDLE_VALUE, + GetLastError, SetLastError, ERROR_INVALID_HANDLE, ERROR_INVALID_PARAMETER, + ERROR_LOCK_VIOLATION, HANDLE, INVALID_HANDLE_VALUE, }, Storage::FileSystem::*, }; @@ -88,12 +87,6 @@ pub struct BufferResult { pub value: Buffer, } -#[napi(object)] -pub struct DirectoryResult { - pub error: u32, - pub value: Vec, -} - #[napi(object)] pub struct DirectoryEntry { pub name: Buffer, @@ -144,120 +137,31 @@ pub fn windows_absolute_path(path: Buffer) -> napi::Result { } } -#[napi] -pub fn windows_directory_names(path: Buffer) -> napi::Result { - let result = windows_directory_entries(path)?; - Ok(DirectoryResult { - error: result.error, - value: result.value.into_iter().map(|entry| entry.name).collect(), - }) -} - #[napi] pub fn windows_directory_entries(path: Buffer) -> napi::Result { let path = os_string(path)?; - let failure = |error| DirectoryEntriesResult { - error, - value: Vec::new(), + let entries = || -> io::Result> { + fs::read_dir(path)? + .map(|entry| { + let entry = entry?; + let kind = entry.file_type()?; + Ok(DirectoryEntry { + name: wide_bytes(entry.file_name().encode_wide()), + is_directory: kind.is_dir() || kind.is_symlink_dir(), + is_symbolic_link: kind.is_symlink(), + }) + }) + .collect() }; - if path.is_empty() { - return Ok(failure(ERROR_PATH_NOT_FOUND)); - } - let pattern = Path::new(&path).join("*"); - let pattern = directory_search_path(wide_bytes(pattern.as_os_str().encode_wide()))?; - if pattern.error != 0 { - return Ok(failure(pattern.error)); - } - let pattern = wide_path(pattern.value)?; - let mut data = WIN32_FIND_DATAW::default(); - let handle = unsafe { FindFirstFileW(pattern.as_ptr(), &mut data) }; - if handle == INVALID_HANDLE_VALUE { - let error = unsafe { GetLastError() }; - // Like read_dir, a successful empty search is an empty iterator. - return Ok(failure(if error == ERROR_FILE_NOT_FOUND { - 0 - } else { - error - })); - } - struct FindHandle(HANDLE); - impl Drop for FindHandle { - fn drop(&mut self) { - unsafe { FindClose(self.0) }; - } - } - let handle = FindHandle(handle); - let mut value = Vec::new(); - loop { - let end = data.cFileName.iter().position(|unit| *unit == 0).unwrap(); - let name = &data.cFileName[..end]; - if name != [b'.' as u16] && name != [b'.' as u16, b'.' as u16] { - value.push(DirectoryEntry { - name: wide_bytes(name.iter().copied()), - is_directory: data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0, - is_symbolic_link: data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != 0 - && data.dwReserved0 == 0xa000000c, - }); - } - if unsafe { FindNextFileW(handle.0, &mut data) } == 0 { - let error = unsafe { GetLastError() }; - return Ok(if error == ERROR_NO_MORE_FILES { - DirectoryEntriesResult { error: 0, value } - } else { - failure(error) - }); - } - } -} - -fn directory_search_path(path: Buffer) -> napi::Result { - const SEP: u16 = b'\\' as u16; - const ALT: u16 = b'/' as u16; - const COLON: u16 = b':' as u16; - const DOT: u16 = b'.' as u16; - const QUERY: u16 = b'?' as u16; - let units = wide_path(path.to_vec().into())?; - let verbatim = - units.starts_with(&[SEP, SEP, QUERY, SEP]) || units.starts_with(&[SEP, QUERY, QUERY, SEP]); - let short_absolute = units.len() < 248 - && (matches!(units.as_slice(), [drive, COLON, SEP | ALT, ..] if ![SEP, ALT].contains(drive)) - || matches!(units.as_slice(), [SEP | ALT, SEP | ALT, ..])); - if verbatim || short_absolute { - return Ok(BufferResult { - error: 0, - value: path, - }); - } - // Match read_dir's conversion of relative and long search paths to verbatim paths. - let absolute = windows_absolute_path(path)?; - if absolute.error != 0 { - return Ok(absolute); + match entries() { + Ok(value) => Ok(DirectoryEntriesResult { error: 0, value }), + Err(error) => Ok(DirectoryEntriesResult { + error: error + .raw_os_error() + .ok_or_else(|| invalid(&error.to_string()))? as u32, + value: Vec::new(), + }), } - let units = wide_path(absolute.value)?; - let units = &units[..units.len() - 1]; - let (prefix, rest): (&[u16], &[u16]) = match units { - [_, COLON, SEP, ..] => (&[SEP, SEP, QUERY, SEP], units), - [SEP, SEP, DOT, SEP, rest @ ..] => (&[SEP, SEP, QUERY, SEP], rest), - [SEP, SEP, QUERY, SEP, ..] | [SEP, QUERY, QUERY, SEP, ..] => (&[], units), - [SEP, SEP, rest @ ..] => ( - &[ - SEP, - SEP, - QUERY, - SEP, - b'U' as u16, - b'N' as u16, - b'C' as u16, - SEP, - ], - rest, - ), - _ => (&[], units), - }; - Ok(BufferResult { - error: 0, - value: wide_bytes(prefix.iter().chain(rest).copied()), - }) } fn io_range(buffer: &Buffer, offset: f64, length: f64) -> napi::Result<(usize, u32)> { diff --git a/plugins/codex-security/native/windows-binding.mts b/plugins/codex-security/native/windows-binding.mts index 36c9d90bc9..1a393ad5ad 100644 --- a/plugins/codex-security/native/windows-binding.mts +++ b/plugins/codex-security/native/windows-binding.mts @@ -31,7 +31,6 @@ export interface WindowsBinding { windowsArguments(): Buffer[]; windowsEnvironment(name: Buffer): Buffer | null; windowsAbsolutePath(path: Buffer): WindowsResult; - windowsDirectoryNames(path: Buffer): WindowsResult; windowsDirectoryEntries( path: Buffer, ): WindowsResult< diff --git a/plugins/codex-security/native/windows-files.mts b/plugins/codex-security/native/windows-files.mts index d188a53102..daadd33d92 100644 --- a/plugins/codex-security/native/windows-files.mts +++ b/plugins/codex-security/native/windows-files.mts @@ -73,33 +73,23 @@ export function windowsFileSystem(native: WindowsBinding) { } function realpath(path: Buffer): Buffer { - // Match CPython's Windows normalization of UNC and device prefixes. - const textPath = pathText(path).replaceAll("/", "\\"); - let normalizedText = textPath; - if (textPath.startsWith("\\\\")) { - const first = textPath.indexOf("\\", 2); - const end = first === -1 ? -1 : textPath.indexOf("\\", first + 1); - if (end !== -1) { - const tail = textPath.slice(end + 1).replace(/^\\+/u, ""); - normalizedText = - textPath.slice(0, end + 1) + - win32.normalize(`\\${tail}`).slice(1).replace(/\\+$/u, ""); - } - } else { - if (textPath[1] === ":" && textPath.slice(2, 4) === ".\\") { - // Windows normpath retains the first drive-relative dot until a parent consumes it. - const parts = ["."]; - for (const part of textPath.slice(4).split("\\")) { - if (part === "" || part === ".") continue; - if (part === ".." && parts.length && parts.at(-1) !== "..") - parts.pop(); - else parts.push(part); - } - normalizedText = textPath.slice(0, 2) + parts.join("\\"); - } else normalizedText = win32.normalize(textPath); - const root = win32.parse(normalizedText).root; + let normalizedText: string; + if (pathText(path).startsWith("\\\\?\\")) { + // Verbatim paths bypass Win32 dot parsing; normalize only below their root. + const text = pathText(path).replaceAll("/", "\\"); + const root = + /^\\\\\?\\(?:UNC\\[^\\]+\\[^\\]+(?:\\|$)|[^\\]+\\)/iu.exec(text)?.[0] ?? + win32.parse(text).root; normalizedText = - root + normalizedText.slice(root.length).replace(/\\+$/u, ""); + root + + win32 + .normalize(`\\${text.slice(root.length)}`) + .slice(1) + .replace(/\\+$/u, ""); + } else { + const text = pathText(absolute(path)); + const root = win32.parse(text).root; + normalizedText = root + text.slice(root.length).replace(/\\+$/u, ""); } const normalized = widePath(normalizedText); const resolved = finalPath(normalized); @@ -147,12 +137,6 @@ export function windowsFileSystem(native: WindowsBinding) { } } - function entries(path: Buffer): Buffer[] { - const result = native.windowsDirectoryNames(operationPath(path)); - check(result.error, path); - return result.value; - } - function entriesWithTypes(path: Buffer) { const result = native.windowsDirectoryEntries(operationPath(path)); check(result.error, path); @@ -227,7 +211,6 @@ export function windowsFileSystem(native: WindowsBinding) { absolute, realpath, stat, - entries, entriesWithTypes, mkdir, readInto, diff --git a/plugins/codex-security/native/windows-files.test.mts b/plugins/codex-security/native/windows-files.test.mts new file mode 100644 index 0000000000..ec39709b95 --- /dev/null +++ b/plugins/codex-security/native/windows-files.test.mts @@ -0,0 +1,62 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { win32 } from "node:path"; +import { type WindowsBinding } from "./windows-binding.mjs"; +import { pathText, widePath, windowsFileSystem } from "./windows-files.mjs"; + +const opened = new Error("Captured native open"); + +for (const [input, expected] of [ + [ + "\\\\?\\UNC\\server\\share\\..\\other\\file", + "\\\\?\\UNC\\server\\share\\other\\file", + ], + ["\\\\?\\UNC\\server\\share\\child\\..\\..\\", "\\\\?\\UNC\\server\\share\\"], + ["\\\\?\\C:\\..\\file-\ud800", "\\\\?\\C:\\file-\ud800"], + ["\\\\?\\C:\\child\\.\\..\\", "\\\\?\\C:\\"], + ["\\\\?\\C:\\trailing.\\", "\\\\?\\C:\\trailing."], + ["\\\\?\\UNC\\server\\share\\space \\", "\\\\?\\UNC\\server\\share\\space "], +] as const) { + test(`verbatim realpath preserves its root: ${JSON.stringify(input)}`, () => { + const native = { + openWindowsFile(path: Buffer) { + assert.equal(pathText(path), expected); + throw opened; + }, + } as unknown as WindowsBinding; + assert.throws( + () => windowsFileSystem(native).realpath(widePath(input)), + (error) => error === opened, + ); + }); +} + +for (const [input, absolute] of [ + ["C:.\\..\\sentinel", "C:\\parent\\sentinel"], + ["\\\\server\\share\\..\\file\\", "\\\\server\\share\\file\\"], + ["C:/", "C:\\"], + ["C:\\file\\", "C:\\file\\"], +] as const) { + test(`ordinary realpath uses native absolute resolution: ${JSON.stringify(input)}`, () => { + let absoluteCalls = 0; + const native = { + windowsAbsolutePath(path: Buffer) { + if (absoluteCalls++ === 0) { + assert.equal(pathText(path), input); + return { error: 0, value: widePath(absolute) }; + } + return { error: 0, value: path }; + }, + openWindowsFile(path: Buffer) { + const root = win32.parse(absolute).root; + const trimmed = root + absolute.slice(root.length).replace(/\\+$/u, ""); + assert.equal(pathText(path), win32.toNamespacedPath(trimmed)); + throw opened; + }, + } as unknown as WindowsBinding; + assert.throws( + () => windowsFileSystem(native).realpath(widePath(input)), + (error) => error === opened, + ); + }); +} From f87db381acb0621bbb5d60cd319f6d21e1cc5a37 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Sat, 5 Sep 2026 00:28:40 +0000 Subject: [PATCH 6/7] fix(plugin): preserve verbatim roots and simplify native paths --- plugins/codex-security/native/README.md | 2 +- .../native/examples/windows-wide-launcher.rs | 4 -- .../native/proof-windows-wide.mts | 50 ++----------------- plugins/codex-security/native/src/windows.rs | 44 ++++++++-------- .../codex-security/native/windows-files.mts | 7 +-- .../native/windows-files.test.mts | 5 ++ 6 files changed, 34 insertions(+), 78 deletions(-) diff --git a/plugins/codex-security/native/README.md b/plugins/codex-security/native/README.md index 809ef2adcf..9762066209 100644 --- a/plugins/codex-security/native/README.md +++ b/plugins/codex-security/native/README.md @@ -57,7 +57,7 @@ The `native-windows` workflow builds x64 and arm64 with MSVC and a static CRT. I node --expose-gc plugins/codex-security/native/proof-windows.mjs python plugins/codex-security/scripts ``` -The build also compiles the test-only `windows-wide-launcher` Rust example. It starts a Node proof child with lone surrogates in arguments, environment values, and its working directory. That child checks complete directory iteration, distinct surrogate and replacement-character files, canonical paths, bounded reads, output truncation, and recursive long paths through the typed adapter. A Rust file guard with sharing disabled remains open while the child enumerates its name; an explicit data read fails with a sharing violation. Attribute-only access is not blocked by Windows file sharing. The child also compares `node:fs` string paths and WTF-8 buffers against the Rust-created names on both pinned Node versions. Node string conversion can still replace lone surrogates before or after libuv; the buffer results distinguish libuv support from JavaScript string support. Root-normalization tables run on the same matrix. The launcher cleans up the wide fixtures and is never included in the uploaded or bundled native payloads. +The build also compiles the test-only `windows-wide-launcher` Rust example. It starts a Node proof child with lone surrogates in arguments, environment values, and its working directory. That child checks complete directory iteration, distinct surrogate and replacement-character files, canonical paths, bounded reads, output truncation, and recursive long paths through the typed adapter. A Rust file guard with sharing disabled remains open while the child enumerates its name; an explicit data read fails with a sharing violation. Attribute-only access is not blocked by Windows file sharing. Root-normalization tables run on the same matrix. The launcher cleans up the wide fixtures and is never included in the uploaded or bundled native payloads. ## Package inputs diff --git a/plugins/codex-security/native/examples/windows-wide-launcher.rs b/plugins/codex-security/native/examples/windows-wide-launcher.rs index cada8b3b2c..d9f3bfc695 100644 --- a/plugins/codex-security/native/examples/windows-wide-launcher.rs +++ b/plugins/codex-security/native/examples/windows-wide-launcher.rs @@ -23,10 +23,6 @@ fn main() -> std::io::Result<()> { let replacement = root.join("cwd-\u{fffd}"); fs::create_dir(&replacement)?; fs::write(replacement.join("sentinel"), "replacement cwd untouched")?; - fs::write( - replacement.join("high-\u{fffd}"), - "core replacement sentinel", - )?; let names = [ raw("high-", 0xd800), raw("high-", 0xfffd), diff --git a/plugins/codex-security/native/proof-windows-wide.mts b/plugins/codex-security/native/proof-windows-wide.mts index 8c2044f289..fd14f83152 100644 --- a/plugins/codex-security/native/proof-windows-wide.mts +++ b/plugins/codex-security/native/proof-windows-wide.mts @@ -1,7 +1,6 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import { join, win32 } from "node:path"; -import { readFileSync, readdirSync, realpathSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { output } from "./binding.mjs"; import { loadWindowsBinding } from "./windows-binding.mjs"; @@ -72,6 +71,9 @@ function worker(root: string): Record { win32.join(cwd, "high-\ud800"), ); samePath(files.absolute(widePath(cwd)), cwd); + const emptyAbsolute = native.windowsAbsolutePath(Buffer.alloc(0)); + assert(Number.isInteger(emptyAbsolute.error)); + assert.deepEqual(emptyAbsolute.value, Buffer.alloc(0)); const drive = win32.parse(cwd).root.slice(0, 2); assert.match(drive, /^[a-z]:$/iu); samePath( @@ -266,53 +268,7 @@ function worker(root: string): Record { assert.throws(() => native.windowsAbsolutePath(malformed)); assert.throws(() => native.windowsDirectoryEntries(malformed)); } - // libuv's WTF-8 support does not bypass Node's JS string conversion. - assert.equal( - readFileSync(win32.join(cwd, "high-\ud800"), "utf8"), - "core replacement sentinel", - ); - const rawName = Buffer.concat([ - Buffer.from("high-"), - Buffer.from([0xed, 0xa0, 0x80]), - ]); - const rawCwd = Buffer.concat([ - Buffer.from(win32.join(root, "cwd-")), - Buffer.from([0xed, 0xa0, 0x80]), - ]); - const rawFile = Buffer.concat([rawCwd, Buffer.from("\\"), rawName]); - function coreSupports(operation: () => boolean): boolean { - try { - return operation(); - } catch (error) { - assert.equal(typeof (error as NodeJS.ErrnoException).code, "string"); - return false; - } - } - const coreStringNames = coreSupports(() => - readdirSync(rawCwd).includes("high-\ud800"), - ); - const coreStringRealpath = coreSupports( - () => realpathSync.native(rawCwd) === cwd, - ); - assert.equal(coreStringNames, false); - assert.equal(coreStringRealpath, false); - const coreBufferRead = coreSupports( - () => readFileSync(rawFile, "utf8") === "sentinel-0", - ); - const coreBufferNames = coreSupports(() => - readdirSync(rawCwd, { encoding: "buffer" }).some((name) => - name.equals(rawName), - ), - ); - const coreBufferRealpath = coreSupports(() => - realpathSync.native(rawCwd, { encoding: "buffer" }).equals(rawCwd), - ); return { - nodeStringPathsReadReplacementSibling: true, - nodeStringNamesAndRealpathLoseSurrogates: true, - nodeWtf8BufferReads: coreBufferRead, - nodeWtf8BufferDirectoryNames: coreBufferNames, - nodeWtf8BufferRealpath: coreBufferRealpath, rawArgumentsAndCrtQuoting: true, rawEnvironmentEmptyAndUnset: true, rawCwdAndDriveRelativePaths: true, diff --git a/plugins/codex-security/native/src/windows.rs b/plugins/codex-security/native/src/windows.rs index a6c6670103..f89f325343 100644 --- a/plugins/codex-security/native/src/windows.rs +++ b/plugins/codex-security/native/src/windows.rs @@ -114,26 +114,30 @@ pub fn windows_environment(name: Buffer) -> napi::Result> { #[napi] pub fn windows_absolute_path(path: Buffer) -> napi::Result { - let path = wide_path(path)?; - let mut absolute = vec![0_u16; 256]; - loop { - let capacity = u32::try_from(absolute.len()) - .map_err(|_| invalid("Absolute path exceeds the Win32 buffer size"))?; - let length = - unsafe { GetFullPathNameW(path.as_ptr(), capacity, absolute.as_mut_ptr(), null_mut()) }; - if length == 0 { - return Ok(BufferResult { - error: unsafe { GetLastError() }, - value: Vec::new().into(), - }); - } - if length < capacity { - return Ok(BufferResult { - error: 0, - value: wide_bytes(absolute[..length as usize].iter().copied()), - }); - } - absolute.resize(length as usize + 1, 0); + let path = os_string(path)?; + if path.is_empty() { + // Rust rejects empty paths before Win32; retain the native error contract. + let mut value = [0_u16; 256]; + let error = unsafe { + GetFullPathNameW([0_u16].as_ptr(), 256, value.as_mut_ptr(), null_mut()); + GetLastError() + }; + return Ok(BufferResult { + error, + value: Vec::new().into(), + }); + } + match std::path::absolute(path) { + Ok(value) => Ok(BufferResult { + error: 0, + value: wide_bytes(value.as_os_str().encode_wide()), + }), + Err(error) => Ok(BufferResult { + error: error + .raw_os_error() + .ok_or_else(|| invalid(&error.to_string()))? as u32, + value: Vec::new().into(), + }), } } diff --git a/plugins/codex-security/native/windows-files.mts b/plugins/codex-security/native/windows-files.mts index daadd33d92..1a1d7db274 100644 --- a/plugins/codex-security/native/windows-files.mts +++ b/plugins/codex-security/native/windows-files.mts @@ -24,8 +24,6 @@ export function windowsFileSystem(native: WindowsBinding) { } function absolute(path: Buffer): Buffer { - // GetFullPathNameW normalizes even explicit verbatim paths. - if (pathText(path).startsWith("\\\\?\\")) return path; const result = native.windowsAbsolutePath(path); check(result.error, path); return result.value; @@ -82,10 +80,7 @@ export function windowsFileSystem(native: WindowsBinding) { win32.parse(text).root; normalizedText = root + - win32 - .normalize(`\\${text.slice(root.length)}`) - .slice(1) - .replace(/\\+$/u, ""); + win32.join("\\", text.slice(root.length)).slice(1).replace(/\\+$/u, ""); } else { const text = pathText(absolute(path)); const root = win32.parse(text).root; diff --git a/plugins/codex-security/native/windows-files.test.mts b/plugins/codex-security/native/windows-files.test.mts index ec39709b95..29291731a3 100644 --- a/plugins/codex-security/native/windows-files.test.mts +++ b/plugins/codex-security/native/windows-files.test.mts @@ -7,6 +7,8 @@ import { pathText, widePath, windowsFileSystem } from "./windows-files.mjs"; const opened = new Error("Captured native open"); for (const [input, expected] of [ + ["\\\\?\\C:\\\\..\\file", "\\\\?\\C:\\file"], + ["\\\\?\\UNC\\server\\share\\\\..\\file", "\\\\?\\UNC\\server\\share\\file"], [ "\\\\?\\UNC\\server\\share\\..\\other\\file", "\\\\?\\UNC\\server\\share\\other\\file", @@ -19,6 +21,9 @@ for (const [input, expected] of [ ] as const) { test(`verbatim realpath preserves its root: ${JSON.stringify(input)}`, () => { const native = { + windowsAbsolutePath(path: Buffer) { + return { error: 0, value: path }; + }, openWindowsFile(path: Buffer) { assert.equal(pathText(path), expected); throw opened; From ec4dd0805762c548c7234759acdebab239a404a7 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Mon, 7 Sep 2026 11:17:17 +0000 Subject: [PATCH 7/7] refactor(plugin): use native recursive Windows directory creation --- .../codex-security/native/proof-windows-wide.mts | 4 ++++ plugins/codex-security/native/src/windows.rs | 8 +++++++- .../codex-security/native/windows-binding.mts | 1 + plugins/codex-security/native/windows-files.mts | 16 +--------------- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/plugins/codex-security/native/proof-windows-wide.mts b/plugins/codex-security/native/proof-windows-wide.mts index fd14f83152..5ec3c1ccbf 100644 --- a/plugins/codex-security/native/proof-windows-wide.mts +++ b/plugins/codex-security/native/proof-windows-wide.mts @@ -159,6 +159,7 @@ function worker(root: string): Record { error: 3, value: [], }); + assert.deepEqual(native.windowsAbsolutePath(Buffer.alloc(0)), emptyAbsolute); const notDirectory = native.windowsDirectoryEntries(widePath(names[0]!)); assert.notEqual(notDirectory.error, 0); assert(Number.isInteger(notDirectory.error)); @@ -251,6 +252,9 @@ function worker(root: string): Record { files.mkdir(widePath(longDirectory)); files.mkdir(widePath(longDirectory)); assert(files.stat(widePath(longDirectory)).isDirectory()); + assert.throws(() => files.mkdir(Buffer.alloc(0))); + assert.throws(() => files.mkdir(widePath(names[0]!))); + assert.throws(() => files.mkdir(widePath(`${names[0]}\\child`))); const longFile = widePath(win32.join(longDirectory, "file-\udc80")); files.writeFile(longFile, Buffer.from("long raw path")); const longLength = files.readInto(longFile, contents); diff --git a/plugins/codex-security/native/src/windows.rs b/plugins/codex-security/native/src/windows.rs index f89f325343..1ab3485e3f 100644 --- a/plugins/codex-security/native/src/windows.rs +++ b/plugins/codex-security/native/src/windows.rs @@ -116,9 +116,10 @@ pub fn windows_environment(name: Buffer) -> napi::Result> { pub fn windows_absolute_path(path: Buffer) -> napi::Result { let path = os_string(path)?; if path.is_empty() { - // Rust rejects empty paths before Win32; retain the native error contract. + // The public Rust path API rejects empty input before reaching Win32. let mut value = [0_u16; 256]; let error = unsafe { + SetLastError(0); GetFullPathNameW([0_u16].as_ptr(), 256, value.as_mut_ptr(), null_mut()); GetLastError() }; @@ -284,6 +285,11 @@ pub fn create_windows_directory(path: Buffer) -> napi::Result { Ok(status(unsafe { CreateDirectoryW(path.as_ptr(), null()) })) } +#[napi] +pub fn create_windows_directories(path: Buffer) -> napi::Result { + Ok(io_status(fs::create_dir_all(os_string(path)?))) +} + #[napi] impl WindowsHandle { #[napi] diff --git a/plugins/codex-security/native/windows-binding.mts b/plugins/codex-security/native/windows-binding.mts index 1a393ad5ad..e4662d7cf2 100644 --- a/plugins/codex-security/native/windows-binding.mts +++ b/plugins/codex-security/native/windows-binding.mts @@ -44,6 +44,7 @@ export interface WindowsBinding { flags: number, ): { error: number; handle?: WindowsHandle | null }; createWindowsDirectory(path: Buffer): number; + createWindowsDirectories(path: Buffer): number; } export const windowsFlags = { diff --git a/plugins/codex-security/native/windows-files.mts b/plugins/codex-security/native/windows-files.mts index 1a1d7db274..25c0deb366 100644 --- a/plugins/codex-security/native/windows-files.mts +++ b/plugins/codex-security/native/windows-files.mts @@ -143,21 +143,7 @@ export function windowsFileSystem(native: WindowsBinding) { } function mkdir(path: Buffer): void { - const resolved = absolute(path); - const parent = widePath(win32.dirname(pathText(resolved))); - let error = native.createWindowsDirectory(operationPath(resolved)); - if (error === 3 && !parent.equals(resolved)) { - mkdir(parent); - error = native.createWindowsDirectory(operationPath(resolved)); - } - if (error !== 0) { - try { - if (stat(resolved).isDirectory()) return; - } catch { - // Report the original creation error. - } - check(error, path); - } + check(native.createWindowsDirectories(operationPath(path)), path); } function readInto(path: Buffer, buffer: Buffer): number {