From 63534280b1de796dfcde43ab04cdf88cbab18820 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Thu, 3 Sep 2026 04:21:18 +0000 Subject: [PATCH] refactor(plugin): expose Unix directory entry metadata --- plugins/codex-security/native/README.md | 8 +- plugins/codex-security/native/binding.mts | 19 ++- plugins/codex-security/native/proof.mts | 137 ++++++++++++++++++++++ plugins/codex-security/native/src/unix.rs | 55 ++++++++- sdk/typescript/scripts/smoke-package.mjs | 7 +- 5 files changed, 219 insertions(+), 7 deletions(-) diff --git a/plugins/codex-security/native/README.md b/plugins/codex-security/native/README.md index 585aa5c91..8eee15c3b 100644 --- a/plugins/codex-security/native/README.md +++ b/plugins/codex-security/native/README.md @@ -2,9 +2,11 @@ These bindings supply OS operations that Node does not expose. The `resolve-security-md` helper uses native account lookup on Unix and native path, file, and directory operations on Windows. -The nine Node-API 8 functions are typed in `binding.mts`. Paths remain byte buffers. `statAt` never follows the final symlink; device and inode numbers are decimal strings so JavaScript does not round them. `openAt` and `duplicate` create descriptors with close-on-exec set. Node owns subsequent reads, writes, `fstat`, `fsync`, and close calls. `userHome` looks up raw username bytes through the operating system and returns raw home-directory bytes or a missing result, without Git. +The ten Node-API 8 functions are typed in `binding.mts`. Paths remain byte buffers. `statAt` never follows the final symlink; device and inode numbers are decimal strings so JavaScript does not round them. `openAt` and `duplicate` create descriptors with close-on-exec set. Node owns subsequent reads, writes, `fstat`, `fsync`, and close calls. `userHome` looks up raw username bytes through the operating system and returns raw home-directory bytes or a missing result, without Git. -`openAt` and `fileLock` retry EINTR, matching the current Python helpers. Other operations return their native errno. `readDescriptor` retries one interrupted Node read without losing earlier chunks. Blocking locks must run outside the main JavaScript event loop; a process that holds a lock releases it on close or exit. A Python signal handler can raise during a blocked call, so later routing must preserve cancellation through the worker lifecycle. +`directoryEntries` returns raw names in filesystem order. With `withTypes: true`, it uses cached directory and symlink types where available and returns any individual type-query errno beside that entry. Symlinks are not followed. With `withTypes: false`, it never queries entry metadata; the unused type flags are false and entry errnos are zero. A directory-open or iteration failure returns its errno and an empty array. Rust closes the directory on success or failure. + +`openAt` and `fileLock` retry EINTR, matching the current Python helpers. Other descriptor operations return their native errno. Directory enumeration uses the Rust standard library's OS behavior. `readDescriptor` retries one interrupted Node read without losing earlier chunks. Blocking locks must run outside the main JavaScript event loop; a process that holds a lock releases it on close or exit. A Python signal handler can raise during a blocked call, so later routing must preserve cancellation through the worker lifecycle. Install the pinned Rust toolchain and the existing TypeScript dependencies, then run from the repository root: @@ -17,7 +19,7 @@ cargo +1.97.1 fmt --check --manifest-path plugins/codex-security/native/Cargo.to cargo +1.97.1 clippy --locked --manifest-path plugins/codex-security/native/Cargo.toml -- -D warnings ``` -The proof runs without Python. It checks directory replacement, byte paths, unreadable-file metadata, long raw symlinks, descriptor duplication, Node descriptor I/O, account lookup, contention, unlock, and process-death release. Linux exercises undecodable filename bytes; macOS uses valid UTF-8 filenames required by APFS. CI invokes it with an empty `PATH`. During migration, the same protocol can compare the existing Python lock helper: +The proof runs without Python. It checks directory replacement, byte paths, unreadable-file metadata, long raw symlinks, descriptor duplication, Node descriptor I/O, account lookup, directory names and types, names-only enumeration, nonsearchable directories, contention, unlock, and process-death release. Linux exercises undecodable filename bytes; macOS uses valid UTF-8 filenames required by APFS. CI invokes it with an empty `PATH`. During migration, the same protocol can compare the existing Python lock helper: ```sh node plugins/codex-security/native/proof.mjs python3 plugins/codex-security/scripts diff --git a/plugins/codex-security/native/binding.mts b/plugins/codex-security/native/binding.mts index bd6f40526..4a35b8056 100644 --- a/plugins/codex-security/native/binding.mts +++ b/plugins/codex-security/native/binding.mts @@ -23,8 +23,25 @@ export interface MetadataResult { inode: string; } -/** Paths are uninterpreted POSIX bytes. Only openAt and fileLock retry EINTR. */ +export interface DirectoryEntry { + name: Buffer; + isDirectory: boolean; + isSymbolicLink: boolean; + errno: number; +} + +/** Paths are uninterpreted POSIX bytes. */ export interface UnixBinding { + /** + * Filesystem order; known types are cached and symlinks are not followed. + * Entry errno reports type-query errors; outer errno reports enumeration + * failure with an empty value. With types disabled, no type query runs and + * both flags are false with entry errno zero. + */ + directoryEntries( + name: Buffer, + withTypes: boolean, + ): { errno: number; value: DirectoryEntry[] }; openAt( directory: number, name: Buffer, diff --git a/plugins/codex-security/native/proof.mts b/plugins/codex-security/native/proof.mts index 981e9129c..e954dda9f 100644 --- a/plugins/codex-security/native/proof.mts +++ b/plugins/codex-security/native/proof.mts @@ -1,6 +1,7 @@ import assert from "node:assert/strict"; import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; import { + chmodSync, closeSync, constants, existsSync, @@ -9,7 +10,9 @@ import { mkdirSync, mkdtempSync, openSync, + opendirSync, lstatSync, + readdirSync, readFileSync, renameSync, rmSync, @@ -100,6 +103,138 @@ function accountProof() { }; } +function directoryProof(root: string) { + const directory = rawPath(root, fixtureName("e", 0xf9)); + mkdirSync(directory); + const names = [ + fixtureName("f", 0xf8), + Buffer.from("nested"), + Buffer.from("directory-link"), + fixtureName("l", 0xf7), + ]; + const child = (name: Buffer) => + Buffer.concat([directory, Buffer.from("/"), name]); + writeFileSync(child(names[0]!), "file"); + mkdirSync(child(names[1]!)); + symlinkSync(names[1]!, child(names[2]!)); + symlinkSync(Buffer.from("missing"), child(names[3]!)); + const expected = new Map( + names.map((name, index) => [ + name.toString("hex"), + { + name, + isDirectory: index === 1, + isSymbolicLink: index >= 2, + errno: 0, + }, + ]), + ); + const typed = checked(native.directoryEntries(directory, true)).value; + assert.equal(typed.length, names.length); + for (const entry of typed) + assert.deepEqual(entry, expected.get(entry.name.toString("hex"))); + assert.deepEqual(native.directoryEntries(child(names[1]!), true), { + errno: 0, + value: [], + }); + + // Node's unknown-type fallback cannot handle raw names. Use ASCII here to + // compare filesystem order without that fallback changing the path bytes. + const orderDirectory = join(root, "directory-order"); + mkdirSync(orderDirectory); + for (const name of ["z-last", "a-first", "middle"]) + writeFileSync(join(orderDirectory, name), "file"); + const reference = opendirSync(orderDirectory); + const order: Buffer[] = []; + try { + for (let entry; (entry = reference.readSync()) !== null; ) + order.push(Buffer.from(entry.name)); + } finally { + reference.closeSync(); + } + assert.deepEqual( + checked( + native.directoryEntries(Buffer.from(orderDirectory), true), + ).value.map((entry) => entry.name), + order, + ); + const namesOnly = typed.map(({ name }) => ({ + name, + isDirectory: false, + isSymbolicLink: false, + errno: 0, + })); + assert.deepEqual( + checked(native.directoryEntries(directory, false)).value, + namesOnly, + ); + let nonsearchableTypes: { cached: number; denied: number } | null = null; + chmodSync(directory, 0o400); + try { + assert.deepEqual( + checked(native.directoryEntries(directory, false)).value, + namesOnly, + ); + if (process.geteuid?.() !== 0) { + assert.throws(() => lstatSync(child(names[1]!)), { code: "EACCES" }); + const withoutSearch = checked( + native.directoryEntries(directory, true), + ).value; + assert.equal(withoutSearch.length, typed.length); + for (const [index, entry] of withoutSearch.entries()) + assert.deepEqual( + entry, + entry.errno === errno.EACCES + ? { + name: typed[index]!.name, + isDirectory: false, + isSymbolicLink: false, + errno: errno.EACCES, + } + : typed[index], + ); + nonsearchableTypes = { + cached: withoutSearch.filter((entry) => entry.errno === 0).length, + denied: withoutSearch.filter((entry) => entry.errno === errno.EACCES) + .length, + }; + chmodSync(directory, 0); + assert.deepEqual(native.directoryEntries(directory, false), { + errno: errno.EACCES, + value: [], + }); + } + } finally { + chmodSync(directory, 0o700); + } + const descriptors = + process.platform === "linux" ? "/proc/self/fd" : "/dev/fd"; + const descriptorCount = readdirSync(descriptors).length; + for (const withTypes of [false, true]) { + assert.deepEqual(native.directoryEntries(child(names[0]!), withTypes), { + errno: errno.ENOTDIR, + value: [], + }); + assert.deepEqual(native.directoryEntries(child(names[3]!), withTypes), { + errno: errno.ENOENT, + value: [], + }); + assert.throws(() => native.directoryEntries(Buffer.from([0]), withTypes)); + for (let index = 0; index < 32; index++) + checked(native.directoryEntries(directory, withTypes)); + } + assert.equal(readdirSync(descriptors).length, descriptorCount); + return { + rawNamesAndPath: true, + filesystemOrder: true, + directoryAndSymlinkTypes: true, + namesOnly: true, + nonsearchableTypes, + enumerationErrors: true, + descriptorsClosed: true, + }; +} + const directoryFlags = constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW; const ownFileFlags = @@ -482,6 +617,7 @@ if (process.argv[2] === "lock-worker") { try { const descriptors = descriptorProof(root); const accounts = accountProof(); + const directories = directoryProof(root); const locks = await lockProof(root); const pythonCompatibility = python && scripts ? await lockProof(root, python, scripts) : undefined; @@ -494,6 +630,7 @@ if (process.argv[2] === "lock-worker") { nodeApi: 8, descriptors, accounts, + directories, locks, pythonCompatibility, fixture: basename(root), diff --git a/plugins/codex-security/native/src/unix.rs b/plugins/codex-security/native/src/unix.rs index 713b39abd..14606d739 100644 --- a/plugins/codex-security/native/src/unix.rs +++ b/plugins/codex-security/native/src/unix.rs @@ -1,8 +1,10 @@ use napi::bindgen_prelude::Buffer; use napi_derive::napi; use std::{ - ffi::{CStr, CString}, - io, + ffi::{CStr, CString, OsStr}, + fs, io, + os::unix::ffi::OsStrExt, + path::Path, }; #[napi(object)] @@ -35,6 +37,55 @@ fn path(value: Buffer) -> napi::Result { CString::new(value.as_ref()).map_err(|_| napi::Error::from_reason("Path contains a NUL byte")) } +#[napi(object)] +pub struct DirectoryEntry { + pub name: Buffer, + pub is_directory: bool, + pub is_symbolic_link: bool, + pub errno: i32, +} + +#[napi(object)] +pub struct DirectoryResult { + pub value: Vec, + pub errno: i32, +} + +#[napi] +pub fn directory_entries(name: Buffer, with_types: bool) -> napi::Result { + let name = path(name)?; + let entries = fs::read_dir(Path::new(OsStr::from_bytes(name.to_bytes()))).and_then(|entries| { + entries + .map(|entry| { + let entry = entry?; + let mut value = DirectoryEntry { + name: entry.file_name().as_bytes().to_vec().into(), + is_directory: false, + is_symbolic_link: false, + errno: 0, + }; + if with_types { + match entry.file_type() { + Ok(kind) => { + value.is_directory = kind.is_dir(); + value.is_symbolic_link = kind.is_symlink(); + } + Err(error) => value.errno = error.raw_os_error().unwrap(), + } + } + Ok(value) + }) + .collect::>>() + }); + Ok(match entries { + Ok(value) => DirectoryResult { value, errno: 0 }, + Err(error) => DirectoryResult { + value: Vec::new(), + errno: error.raw_os_error().unwrap(), + }, + }) +} + #[napi] pub fn open_at(directory: i32, name: Buffer, flags: i32, mode: u32) -> napi::Result { let name = path(name)?; diff --git a/sdk/typescript/scripts/smoke-package.mjs b/sdk/typescript/scripts/smoke-package.mjs index eea6fc99a..ebc8d7428 100644 --- a/sdk/typescript/scripts/smoke-package.mjs +++ b/sdk/typescript/scripts/smoke-package.mjs @@ -413,8 +413,13 @@ try { [ "--input-type=commonjs", "--eval", - "require(process.argv[1])", + `const assert = require("node:assert/strict"); +const native = require(process.argv[1]); +if (process.platform !== "win32") { + assert.deepEqual(native.directoryEntries(Buffer.from(process.argv[2]), false), { value: [], errno: 2 }); +}`, nativeLibrary, + join(consumer, "missing-native-file"), ], { cwd: consumer, env: { ...process.env, PATH: "" } }, );