diff --git a/plugins/codex-security/mcp-app/src/helpers/resolve-security-md.ts b/plugins/codex-security/mcp-app/src/helpers/resolve-security-md.ts index 6e87b6d09..51494a03b 100644 --- a/plugins/codex-security/mcp-app/src/helpers/resolve-security-md.ts +++ b/plugins/codex-security/mcp-app/src/helpers/resolve-security-md.ts @@ -10,7 +10,7 @@ import { type Stats, } from "node:fs"; import { homedir } from "node:os"; -import { basename, dirname, parse, sep } from "node:path"; +import { basename, dirname, parse, sep, win32 } from "node:path"; import { parseArgs } from "node:util"; import { unixBinding, windowsBinding } from "../native"; import { windowsFileSystem } from "../../../native/windows-files.mjs"; @@ -36,56 +36,38 @@ type FileInfo = Pick & { const statPath = (path: Buffer): FileInfo => windows ? windowsFiles().stat(path) : statSync(path); -function windowsParts(value: string): [string, string, string] { - const path = value.replaceAll("/", "\\"); - if (path.startsWith("\\\\")) { - const start = path.slice(0, 8).toUpperCase() === "\\\\?\\UNC\\" ? 8 : 2; - const server = path.indexOf("\\", start); - const share = server === -1 ? -1 : path.indexOf("\\", server + 1); - return share === -1 - ? [value, "", ""] - : [value.slice(0, share), value[share]!, value.slice(share + 1)]; - } - const drive = path[1] === ":" ? 2 : 0; - const root = path[drive] === "\\" ? 1 : 0; - return [ - value.slice(0, drive), - value.slice(drive, drive + root), - value.slice(drive + root), - ]; -} - function windowsJoin(left: string, right: string): string { - const [leftDrive, leftRoot, leftPath] = windowsParts(left); - const [rightDrive, rightRoot, rightPath] = windowsParts(right); - if (rightRoot) return (rightDrive || leftDrive) + rightRoot + rightPath; - if (rightDrive && rightDrive.toLowerCase() !== leftDrive.toLowerCase()) + if (right.startsWith("\\\\?\\") || right.startsWith("\\\\.\\")) return right; + const namespaced = left.startsWith("\\\\?\\"); + const base = left.startsWith("\\\\?\\UNC\\") + ? `\\\\${left.slice(8)}` + : namespaced + ? left.slice(4) + : left; + const drive = win32.parse(right).root; + if ( + drive.endsWith(":") && + drive.toLowerCase() !== base.slice(0, 2).toLowerCase() + ) return right; - const drive = rightDrive || leftDrive; - const path = - leftPath + (leftPath && !/[/\\]$/u.test(leftPath) ? "\\" : "") + rightPath; - const root = - leftRoot || (path && drive && !/[:/\\]$/u.test(drive) ? "\\" : ""); - return drive + root + path; + const joined = win32.resolve(base, right); + return namespaced && !win32.isAbsolute(right) + ? win32.toNamespacedPath(joined) + : joined; } function parsedPath(value: string): string { - // pathlib removes empty and '.' components while preserving symlink/.. pairs. - let root = windows - ? windowsParts(value).slice(0, 2).join("").replaceAll("/", "\\") + // Preserve symlink/.. pairs while removing empty and '.' components. + const root = windows + ? win32.parse(value).root.replaceAll("/", "\\") : value.startsWith("//") && !value.startsWith("///") ? "//" : parse(value).root; - if (windows && root.startsWith("\\\\") && !root.endsWith("\\")) { - const parts = root.split("\\"); - if ((parts.length === 4 && !"?.".includes(parts[2]!)) || parts.length === 6) - root += "\\"; - } const parts = value .slice(root.length) - .split(process.platform === "win32" ? /[/\\]/u : /\//u) + .split(windows ? /[/\\]/u : /\//u) .filter((part) => part !== "" && part !== "."); - if (windows && !root && windowsParts(parts[0] ?? "")[0]) parts.unshift("."); + if (windows && !root && win32.parse(parts[0] ?? "").root) parts.unshift("."); return root + parts.join(sep) || "."; } @@ -103,6 +85,12 @@ function resolvedPath(path: Buffer): Buffer { function expandHome(path: string, posixHome: string | undefined): string { if (!path.startsWith("~")) return path; if (process.platform === "win32") { + // path.join('C:', 'name') is rooted; 'C:.' keeps it drive-relative. + const joinHome = (home: string, child: string) => + win32.join( + home.length === 2 && home[1] === ":" ? `${home}.` : home, + child, + ); const environment = (name: string) => windowsBinding() .windowsEnvironment(Buffer.from(name, "utf16le")) @@ -114,23 +102,26 @@ function expandHome(path: string, posixHome: string | undefined): string { let home = environment("USERPROFILE"); const homePath = environment("HOMEPATH"); if (home === undefined && homePath !== undefined) { - home = windowsJoin(environment("HOMEDRIVE") ?? "", homePath); + home = `${environment("HOMEDRIVE") ?? ""}${homePath}`; } if (home === undefined) throw new HomeExpansionError("Could not determine home directory."); + // node:path recognizes share roots in ordinary UNC paths, not extended UNC. + const namespacedUnc = home.slice(0, 8).toUpperCase() === "\\\\?\\UNC\\"; + if (namespacedUnc) home = `\\\\${home.slice(8)}`; if (username !== "" && username !== currentUsername) { - const [drive, root, tail] = windowsParts(home); - const separator = Math.max(tail.lastIndexOf("/"), tail.lastIndexOf("\\")); - if (currentUsername !== tail.slice(separator + 1)) { + if (currentUsername !== win32.parse(home).base) { throw new HomeExpansionError("Could not determine home directory."); } - const parent = - drive + root + tail.slice(0, separator + 1).replace(/[/\\]+$/u, ""); - home = windowsJoin(parent, username); + home = joinHome(win32.dirname(home), username); } if (home.startsWith("~")) throw new HomeExpansionError("Could not determine home directory."); - return windowsJoin(home, separator === -1 ? "" : path.slice(end + 1)); + const expanded = joinHome( + home, + separator === -1 ? "" : path.slice(end + 1), + ); + return namespacedUnc ? win32.toNamespacedPath(expanded) : expanded; } if (path === "~" || path.startsWith("~/")) { const home = posixHome ?? homedir(); @@ -351,17 +342,28 @@ function resolveSecurityMd( ): string { const root = resolveRoot(repo, posixHome); const expandedScope = parsedPath(expandHome(scope, posixHome)); - const requestedScope = - process.platform === "win32" - ? windowsFiles().absolute( - encodePath(windowsJoin(decodePath(root), expandedScope)), - ) - : expandedScope.startsWith("/") - ? encodePosixPath(expandedScope) - : appendPath(root, encodePosixPath(expandedScope)); + let requestedScope: Buffer; + if (windows) { + const files = windowsFiles(); + const requestedRoot = decodePath( + files.absolute(encodePath(parsedPath(expandHome(repo, posixHome)))), + ); + // Keep ordinary paths for OS normalization; canonicalize explicit device roots. + const scopeRoot = + requestedRoot.startsWith("\\\\?\\") || requestedRoot.startsWith("\\\\.\\") + ? decodePath(root) + : requestedRoot; + requestedScope = files.absolute( + encodePath(windowsJoin(scopeRoot, expandedScope)), + ); + } else { + requestedScope = expandedScope.startsWith("/") + ? encodePosixPath(expandedScope) + : appendPath(root, encodePosixPath(expandedScope)); + } let resolvedScope: Buffer; try { - // Resolve links before '..', including Python's accepted file/.. paths. + // On POSIX, resolve links before '..', including accepted file/.. paths. resolvedScope = resolvedPath(requestedScope); } catch (error) { if (error instanceof SymlinkLoopError) throw error; diff --git a/plugins/codex-security/native/README.md b/plugins/codex-security/native/README.md index 83f1632d0..37f090393 100644 --- a/plugins/codex-security/native/README.md +++ b/plugins/codex-security/native/README.md @@ -41,7 +41,7 @@ The binding exposes synchronous file and directory creation, attributes and repa 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 enumerator through `entriesWithTypes`, which `resolve-security-md --list` uses on Windows. -`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. +`windows-files.mts` uses native absolute and final paths, preserving raw UTF-16 and the returned device prefix. Non-strict `realpath` resolves the nearest existing ancestor of a missing path; dangling links and other access errors remain errors. Callers must check containment independently. It follows native Windows path behavior rather than emulating Python's special cases for device names, whitespace, verbatim dot segments, and prefix removal. `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: @@ -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. 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. Adapter path and I/O tests run on the same matrix. The launcher cleans up the wide fixtures and is never included in the uploaded or bundled native payloads. Creating file and directory symbolic links requires Windows Developer Mode or the symbolic-link privilege. Without it, the proofs still run their other assertions, including directory junctions, and report the skipped symbolic-link assertions as `false` in their JSON output. CI requires real symbolic links on both architectures and also forces the restricted case to exercise both paths. diff --git a/plugins/codex-security/native/examples/windows-wide-launcher.rs b/plugins/codex-security/native/examples/windows-wide-launcher.rs index b0e66ab8c..395f8ef83 100644 --- a/plugins/codex-security/native/examples/windows-wide-launcher.rs +++ b/plugins/codex-security/native/examples/windows-wide-launcher.rs @@ -58,6 +58,8 @@ fn main() -> std::io::Result<()> { std::os::windows::fs::symlink_file(&names[0], cwd.join("file-link")) })?; if symlinks { + std::os::windows::fs::symlink_file(raw("missing-", 0xdfff), cwd.join("missing-link"))?; + std::os::windows::fs::symlink_file("loop-link", cwd.join("loop-link"))?; std::os::windows::fs::symlink_dir("empty", cwd.join("directory-link"))?; std::os::windows::fs::symlink_dir( raw("missing-", 0xdfff), @@ -203,6 +205,15 @@ fn main() -> std::io::Result<()> { } fs::remove_file(&output)?; } + for scope in [PathBuf::from("."), PathBuf::from(&scopes[0]).join("..")] { + let child = invoke(&["--repo".into(), repo.clone(), "--scope".into(), scope])?; + let expected = b"## SECURITY.md source: \"SECURITY.md\"\n\nroot raw\n"; + if !child.status.success() || !child.stderr.is_empty() || child.stdout != expected { + return Err(io::Error::other( + "Windows policy helper did not resolve the root scope", + )); + } + } let listing = invoke(&["--repo".into(), "~".into(), "--list".into()])?; let expected = b"[\"SECURITY.md\", \"scope-\\udfff/SECURITY.md\", \"scope-\\ufffd/SECURITY.md\"]\n"; diff --git a/plugins/codex-security/native/proof-policy-windows.mts b/plugins/codex-security/native/proof-policy-windows.mts index 5ca186a39..d649cde92 100644 --- a/plugins/codex-security/native/proof-policy-windows.mts +++ b/plugins/codex-security/native/proof-policy-windows.mts @@ -1,9 +1,20 @@ +import assert from "node:assert/strict"; import { execFileSync } from "node:child_process"; -import { copyFileSync, mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { + copyFileSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, win32 } from "node:path"; import { binaryPath, output, root } from "./binding.mjs"; import { nativeTarget } from "./platform.mjs"; +import { + loadWindowsBinding, + windowsFlags as flags, +} from "./windows-binding.mjs"; const testDirectory = join(output, "policy-proof"); const helper = join(testDirectory, "helpers.cjs"); @@ -28,6 +39,44 @@ if (process.argv[2] === "build") { } else { const fixture = mkdtempSync(join(tmpdir(), "codex-security-policy-proof-")); try { + const repo = join(fixture, "volume-repository"); + mkdirSync(repo); + writeFileSync(join(repo, "SECURITY.md"), "volume policy\n"); + const opened = loadWindowsBinding().openWindowsFile( + Buffer.from(repo, "utf16le"), + 0, + flags.FILE_SHARE_READ | flags.FILE_SHARE_WRITE | flags.FILE_SHARE_DELETE, + flags.OPEN_EXISTING, + flags.FILE_FLAG_BACKUP_SEMANTICS, + ); + assert.equal(opened.error, 0); + assert(opened.handle); + let volumePath: string; + try { + const final = opened.handle.finalPath(flags.VOLUME_NAME_GUID); + assert.equal(final.error, 0); + volumePath = final.path.toString("utf16le"); + } finally { + assert.equal(opened.handle.close(), 0); + } + for (const scope of [".", repo.slice(win32.parse(repo).root.length - 1)]) { + assert.equal( + execFileSync( + process.execPath, + [ + helper, + "--helper", + "resolve-security-md", + "--repo", + volumePath, + "--scope", + scope, + ], + { encoding: "utf8" }, + ), + '## SECURITY.md source: "SECURITY.md"\n\nvolume policy\n', + ); + } const proof: unknown = JSON.parse( execFileSync( join(output, "windows-wide-launcher.exe"), @@ -36,7 +85,12 @@ if (process.argv[2] === "build") { ), ); console.log( - JSON.stringify({ node: process.version, arch: process.arch, proof }), + JSON.stringify({ + node: process.version, + arch: process.arch, + proof, + volumeGuidScope: true, + }), ); } finally { rmSync(fixture, { recursive: true, force: true }); diff --git a/plugins/codex-security/native/proof-windows-wide.mts b/plugins/codex-security/native/proof-windows-wide.mts index edc1ff0db..cadd8ccd1 100644 --- a/plugins/codex-security/native/proof-windows-wide.mts +++ b/plugins/codex-security/native/proof-windows-wide.mts @@ -29,7 +29,9 @@ function worker(root: string): Record { const directoryLinks = symlinks ? ["dangling-directory-link", "directory-link"] : []; - const links = symlinks ? [...directoryLinks, "file-link"] : []; + const links = symlinks + ? [...directoryLinks, "file-link", "loop-link", "missing-link"] + : []; const cwd = win32.join(root, "cwd-\ud800"); const expectedArguments = [ "arg-high-\ud800", @@ -67,9 +69,11 @@ function worker(root: string): Record { assert.deepEqual(environment("USERPROFILE"), widePath(cwd)); function samePath(actual: Buffer, expected: string): void { + const namespaced = (path: string) => + path.startsWith("\\\\?\\") ? path : win32.toNamespacedPath(path); assert.equal( - win32.toNamespacedPath(pathText(actual)).toLowerCase(), - win32.toNamespacedPath(expected).toLowerCase(), + namespaced(pathText(actual)).toLowerCase(), + namespaced(expected).toLowerCase(), ); } samePath(files.absolute(widePath(".")), cwd); @@ -92,6 +96,7 @@ function worker(root: string): Record { `${drive}\\rooted-\ud800`, ); samePath(files.realpath(widePath(".")), cwd); + assert.throws(() => files.readFile(widePath(".")), { winerror: 5 }); const names = [ "high-\ud800", @@ -175,14 +180,6 @@ 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}/`, - `${win32.join(cwd, name)}\\`, - ]) { - samePath(files.realpath(widePath(input)), win32.join(cwd, name)); - } } samePath(files.realpath(widePath(`${drive}///`)), `${drive}\\`); samePath( @@ -190,6 +187,47 @@ function worker(root: string): Record { win32.join(root, "parent-\ud800"), ); assert(files.stat(widePath(".")).isDirectory()); + if (symlinks) { + assert.deepEqual( + files.readFile(widePath("file-link")), + Buffer.from("sentinel-0"), + ); + samePath( + files.realpath(widePath("directory-link\\missing-\udfff"), false), + win32.join(cwd, "empty", "missing-\udfff"), + ); + for (const path of [ + "missing-link", + "missing-link\\child", + "dangling-directory-link", + "dangling-directory-link\\child", + ]) { + assert.throws(() => files.realpath(widePath(path), false), { + code: "ENOENT", + }); + } + assert.throws(() => files.realpath(widePath("loop-link"), false), { + code: "ELOOP", + }); + } + + const streamFile = win32.join(cwd, "a"); + files.writeFile(widePath(streamFile), Buffer.from("base file")); + for (const parent of [cwd, win32.toNamespacedPath(cwd)]) { + const stream = `${parent}\\a:stream`; + const resolved = files.realpath(widePath(stream), false); + samePath(resolved, stream); + files.writeFile(resolved, Buffer.from("stream payload"), true); + assert.equal(files.readFile(widePath(stream)).toString(), "stream payload"); + files.unlink(resolved); + assert.equal(files.readFile(widePath(streamFile)).toString(), "base file"); + } + files.unlink(widePath(streamFile)); + const missingDeepPath = `${win32.toNamespacedPath(cwd)}\\${"a\\".repeat(8_000)}missing`; + assert.equal( + pathText(files.realpath(widePath(missingDeepPath), false)), + missingDeepPath, + ); const bounded = Buffer.alloc(4); assert.equal(files.readInto(widePath(names[0]!), bounded), 4); assert.equal(bounded.toString(), "sent"); @@ -200,13 +238,27 @@ function worker(root: string): Record { replacementOutput, Buffer.from("replacement output untouched"), ); - files.writeFile(rawOutput, Buffer.from("a longer initial output")); + files.writeFile(rawOutput, [ + Buffer.alloc(64 * 1024, 7), + Buffer.alloc(64 * 1024 + 1, 7), + ]); + assert.deepEqual(files.readFile(rawOutput), Buffer.alloc(128 * 1024 + 1, 7)); 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); + assert.throws(() => + files.writeFile(rawOutput, Buffer.from("no replacement"), true), + ); + assert.equal(files.readFile(rawOutput).length, 0); + const renamed = widePath("renamed-\udc80"); + files.writeFile(renamed, Buffer.from("previous destination")); + files.rename(rawOutput, renamed); + assert.equal(files.readFile(renamed).length, 0); + files.unlink(renamed); + assert.throws(() => files.stat(renamed), { code: "ENOENT" }); const replacementLength = files.readInto(replacementOutput, contents); assert.equal( contents.subarray(0, replacementLength).toString(), @@ -244,6 +296,15 @@ function worker(root: string): Record { win32.join(cwd, `directory-${name.slice(0, -1)}`), ); files.mkdir(ordinaryDirectory); + const missing = files.realpath( + widePath(`${pathText(directory)}\\new.json`), + false, + ); + files.writeFile(missing, Buffer.from("new literal child")); + assert.equal( + files.readFile(widePath(`${pathText(directory)}\\new.json`)).toString(), + "new literal child", + ); assert.deepEqual(files.entriesWithTypes(ordinaryDirectory), []); } @@ -283,7 +344,6 @@ function worker(root: string): Record { completeWideDirectoryIteration: true, cachedDirectoryAttributesWithoutFileAccess: true, cachedSymlinkTagsIncludingDanglingDirectories: symlinks, - existingFilesWithTrailingSeparators: true, distinctRawAndReplacementFiles: true, canonicalPathsBoundedReadsAndTruncation: true, verbatimTrailingDotsAndSpaces: true, diff --git a/plugins/codex-security/native/windows-files.mts b/plugins/codex-security/native/windows-files.mts index 37f42ca78..de57b7f89 100644 --- a/plugins/codex-security/native/windows-files.mts +++ b/plugins/codex-security/native/windows-files.mts @@ -38,106 +38,96 @@ export function windowsFileSystem(native: WindowsBinding) { ); } - function open( + function withFile( path: Buffer, - access = 0, + access: number, + action: (handle: WindowsHandle) => T, disposition: number = flags.OPEN_EXISTING, follow = true, - ): WindowsHandle { + ): T { const result = native.openWindowsFile( operationPath(path), access, flags.FILE_SHARE_READ | flags.FILE_SHARE_WRITE | flags.FILE_SHARE_DELETE, disposition, - flags.FILE_FLAG_BACKUP_SEMANTICS | + (access === flags.GENERIC_READ ? 0 : 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); + const handle = result.handle!; try { - const result = handle.finalPath(0); - check(result.error, path); - return result.path; + return action(handle); } finally { check(handle.close(), path); } } - function realpath(path: Buffer): Buffer { - 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 + - win32.join("\\", 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); - 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. + function realpath(path: Buffer, strict = true): Buffer { + let current = operationPath(path); + const missing: string[] = []; + while (true) { + try { + return withFile(current, 0, (handle) => { + const result = handle.finalPath(0); + check(result.error, current); + if (!missing.length) return result.path; + // Join filename components directly so a:stream remains a filename. + return widePath( + `${pathText(result.path).replace(/\\$/u, "")}\\${missing.reverse().join("\\")}`, + ); + }); + } catch (error) { + if (strict || (error as NodeJS.ErrnoException).code !== "ENOENT") + throw error; + try { + withFile(current, 0, () => {}, flags.OPEN_EXISTING, false); + } catch (sourceError) { + if ((sourceError as NodeJS.ErrnoException).code !== "ENOENT") + throw sourceError; + const text = pathText(current); + const parent = win32.dirname(text); + if (parent === text) throw error; + missing.push(win32.basename(text)); + current = widePath(parent); + continue; + } + // An existing entry that cannot be followed is not a missing output. + throw error; + } } - return resolved; } function stat(path: Buffer, follow = true) { - const handle = open( + return withFile( path, flags.FILE_READ_ATTRIBUTES, + (handle) => { + 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, + }; + }, 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 identity(path: Buffer) { - const handle = open(path, flags.FILE_READ_ATTRIBUTES); - try { + return withFile(path, flags.FILE_READ_ATTRIBUTES, (handle) => { const result = handle.identity(); check(result.error, path); return { volume: result.volume, fileId: result.fileId }; - } finally { - check(handle.close(), path); - } + }); } function entriesWithTypes(path: Buffer) { @@ -155,9 +145,8 @@ export function windowsFileSystem(native: WindowsBinding) { } function readInto(path: Buffer, buffer: Buffer): number { - const handle = open(path, flags.GENERIC_READ); - let length = 0; - try { + return withFile(path, flags.GENERIC_READ, (handle) => { + let length = 0; while (length < buffer.length) { const result = handle.read( buffer, @@ -168,32 +157,75 @@ export function windowsFileSystem(native: WindowsBinding) { if (result.value === 0) break; length += result.value; } - } finally { - check(handle.close(), path); - } - return length; + 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), - ); + function readFile(path: Buffer): Buffer { + return withFile(path, flags.GENERIC_READ, (handle) => { + const chunks: Buffer[] = []; + while (true) { + const chunk = Buffer.alloc(64 * 1024); + const result = handle.read(chunk, 0, chunk.length); check(result.error, path); - if (result.value === 0) - throw new Error( - `Windows file write made no progress: ${pathText(path)}`, - ); - offset += result.value; + if (result.value === 0) return Buffer.concat(chunks); + chunks.push(chunk.subarray(0, result.value)); } - } finally { - check(handle.close(), path); - } + }); + } + + function writeFile( + path: Buffer, + data: Buffer | Iterable, + exclusive = false, + ): void { + withFile( + path, + flags.GENERIC_WRITE, + (handle) => { + for (const buffer of Buffer.isBuffer(data) ? [data] : data) { + let offset = 0; + 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; + } + } + }, + exclusive ? flags.CREATE_NEW : flags.CREATE_ALWAYS, + ); + } + + function rename(source: Buffer, destination: Buffer): void { + withFile( + source, + flags.DELETE, + (handle) => { + check(handle.rename(operationPath(destination), true), destination); + }, + flags.OPEN_EXISTING, + false, + ); + } + + function unlink(path: Buffer): void { + withFile( + path, + flags.DELETE, + (handle) => { + check(handle.setDisposition(true), path); + }, + flags.OPEN_EXISTING, + false, + ); } return { @@ -204,6 +236,9 @@ export function windowsFileSystem(native: WindowsBinding) { entriesWithTypes, mkdir, readInto, + readFile, writeFile, + rename, + unlink, }; } diff --git a/plugins/codex-security/native/windows-files.test.mts b/plugins/codex-security/native/windows-files.test.mts index 29291731a..f869e69bd 100644 --- a/plugins/codex-security/native/windows-files.test.mts +++ b/plugins/codex-security/native/windows-files.test.mts @@ -3,65 +3,269 @@ 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"; +import { windowsFlags as flags } from "./windows-flags.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", - ], - ["\\\\?\\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 "], +for (const [input, absolute] of [ + ["C:.\\..\\sentinel", "C:\\parent\\sentinel"], + ["\\\\server\\share", "\\\\server\\share\\"], + ["\\\\?\\C:\\file-\ud800", "\\\\?\\C:\\file-\ud800"], + ["\\\\?\\C:\\trailing.", "\\\\?\\C:\\trailing."], ] as const) { - test(`verbatim realpath preserves its root: ${JSON.stringify(input)}`, () => { + test(`realpath uses native absolute and final paths: ${JSON.stringify(input)}`, () => { + const final = widePath("\\\\?\\C:\\canonical-\ud800"); + let closes = 0; + let resolutions = 0; const native = { windowsAbsolutePath(path: Buffer) { - return { error: 0, value: path }; + if (resolutions++ === 0) assert.deepEqual(path, widePath(input)); + return { error: 0, value: widePath(absolute) }; }, openWindowsFile(path: Buffer) { - assert.equal(pathText(path), expected); - throw opened; + assert.equal(pathText(path), win32.toNamespacedPath(absolute)); + return { + error: 0, + handle: { + finalPath: () => ({ error: 0, path: final }), + close: () => { + closes++; + return 0; + }, + }, + }; }, } as unknown as WindowsBinding; - assert.throws( - () => windowsFileSystem(native).realpath(widePath(input)), - (error) => error === opened, + assert.deepEqual( + windowsFileSystem(native).realpath(widePath(input)), + final, ); + assert.equal(closes, 1); }); } -for (const [input, absolute] of [ - ["C:.\\..\\sentinel", "C:\\parent\\sentinel"], - ["\\\\server\\share\\..\\file\\", "\\\\server\\share\\file\\"], - ["C:/", "C:\\"], - ["C:\\file\\", "C:\\file\\"], +for (const [parent, tail] of [ + ["C:\\alias", "missing-\udfff\\child"], + ["C:\\alias", "a:stream"], + ["\\\\server\\share\\alias", "missing"], + ["\\\\?\\C:\\trailing.", "new.json"], + ["\\\\?\\C:\\space ", "new.json"], + ["\\\\?\\C:\\alias", "a\\".repeat(8_000) + "missing"], ] as const) { - test(`ordinary realpath uses native absolute resolution: ${JSON.stringify(input)}`, () => { - let absoluteCalls = 0; + test(`non-strict realpath resolves the existing ancestor: ${JSON.stringify(parent)} (${tail.length} chars)`, () => { + const canonical = "\\\\?\\C:\\destination"; const native = { - windowsAbsolutePath(path: Buffer) { - if (absoluteCalls++ === 0) { - assert.equal(pathText(path), input); - return { error: 0, value: widePath(absolute) }; - } - return { error: 0, value: path }; - }, + windowsAbsolutePath: (path: Buffer) => ({ 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; + return pathText(path) === win32.toNamespacedPath(parent) + ? { + error: 0, + handle: { + finalPath: () => ({ error: 0, path: widePath(canonical) }), + close: () => 0, + }, + } + : { error: 3, handle: null }; + }, + } as unknown as WindowsBinding; + assert.equal( + pathText( + windowsFileSystem(native).realpath( + widePath(`${parent}\\${tail}`), + false, + ), + ), + `${canonical}\\${tail}`, + ); + assert.throws( + () => windowsFileSystem(native).realpath(widePath(`${parent}\\${tail}`)), + { code: "ENOENT" }, + ); + }); +} + +for (const suffix of ["", "\\child"]) { + test(`non-strict realpath rejects dangling reparse points${suffix}`, () => { + let closes = 0; + const native = { + windowsAbsolutePath: (path: Buffer) => ({ error: 0, value: path }), + openWindowsFile( + path: Buffer, + _access: number, + _share: number, + _disposition: number, + options: number, + ) { + return pathText(path) === "\\\\?\\C:\\link" && + options & flags.FILE_FLAG_OPEN_REPARSE_POINT + ? { + error: 0, + handle: { + close: () => { + closes++; + return 0; + }, + }, + } + : { error: 3, handle: null }; }, } as unknown as WindowsBinding; assert.throws( - () => windowsFileSystem(native).realpath(widePath(input)), - (error) => error === opened, + () => + windowsFileSystem(native).realpath( + widePath(`C:\\link${suffix}`), + false, + ), + { code: "ENOENT" }, + ); + assert.equal(closes, 1); + }); +} + +for (const error of [5, 32, 1921]) { + test(`non-strict realpath preserves native error ${error}`, () => { + const native = { + windowsAbsolutePath: (path: Buffer) => ({ error: 0, value: path }), + openWindowsFile: () => ({ error, handle: null }), + } as unknown as WindowsBinding; + assert.throws( + () => windowsFileSystem(native).realpath(widePath("C:\\file"), false), + { winerror: error }, + ); + }); +} + +for (const bounded of [true, false]) { + test(`${bounded ? "bounded" : "complete"} reads continue after short reads and close at EOF`, () => { + const chunks = [Buffer.from("ab"), Buffer.from("c"), Buffer.alloc(0)]; + let closes = 0; + const native = { + windowsAbsolutePath: (path: Buffer) => ({ error: 0, value: path }), + openWindowsFile( + path: Buffer, + access: number, + share: number, + disposition: number, + options: number, + ) { + assert.equal(pathText(path), "\\\\?\\C:\\file-\ud800"); + assert.equal(access, flags.GENERIC_READ); + assert.equal( + share, + flags.FILE_SHARE_READ | + flags.FILE_SHARE_WRITE | + flags.FILE_SHARE_DELETE, + ); + assert.equal(disposition, flags.OPEN_EXISTING); + assert.equal(options & flags.FILE_FLAG_BACKUP_SEMANTICS, 0); + return { + error: 0, + handle: { + read(buffer: Buffer, offset: number, length: number) { + const chunk = chunks.shift()!; + assert(chunk.length <= length); + chunk.copy(buffer, offset); + return { error: 0, value: chunk.length }; + }, + close: () => { + closes++; + return 0; + }, + }, + }; + }, + } as unknown as WindowsBinding; + const files = windowsFileSystem(native); + const path = widePath("C:\\file-\ud800"); + if (bounded) { + const buffer = Buffer.alloc(8); + assert.equal(files.readInto(path, buffer), 3); + assert.equal(buffer.subarray(0, 3).toString(), "abc"); + } else assert.equal(files.readFile(path).toString(), "abc"); + assert.equal(chunks.length, 0); + assert.equal(closes, 1); + }); +} + +test("exclusive writes handle short writes without consuming input on open failure", () => { + const written: Buffer[] = []; + let opens = 0; + let closes = 0; + let iterations = 0; + const native = { + windowsAbsolutePath: (path: Buffer) => ({ error: 0, value: path }), + openWindowsFile( + path: Buffer, + access: number, + share: number, + disposition: number, + ) { + assert.equal(pathText(path), "\\\\?\\C:\\output-\ud800"); + assert.equal(access, flags.GENERIC_WRITE); + assert.equal( + share, + flags.FILE_SHARE_READ | + flags.FILE_SHARE_WRITE | + flags.FILE_SHARE_DELETE, + ); + assert.equal(disposition, flags.CREATE_NEW); + if (opens++) return { error: 80, handle: null }; + return { + error: 0, + handle: { + write(buffer: Buffer, offset: number, length: number) { + const count = Math.min(length, 2); + written.push(buffer.subarray(offset, offset + count)); + return { error: 0, value: count }; + }, + close: () => { + closes++; + return 0; + }, + }, + }; + }, + } as unknown as WindowsBinding; + const data = { + *[Symbol.iterator]() { + iterations++; + yield Buffer.from("abc"); + yield Buffer.alloc(0); + yield Buffer.from("def"); + }, + }; + const files = windowsFileSystem(native); + const path = widePath("C:\\output-\ud800"); + files.writeFile(path, data, true); + assert.equal(Buffer.concat(written).toString(), "abcdef"); + assert.throws(() => files.writeFile(path, data, true), { winerror: 80 }); + assert.equal(iterations, 1); + assert.equal(closes, 1); +}); + +for (const closeError of [0, 5]) { + test(`write input failures close the handle, preserving close error ${closeError}`, () => { + const inputError = new Error("input failed"); + let closes = 0; + const native = { + windowsAbsolutePath: (path: Buffer) => ({ error: 0, value: path }), + openWindowsFile: () => ({ + error: 0, + handle: { + close: () => { + closes++; + return closeError; + }, + }, + }), + } as unknown as WindowsBinding; + const data = { + [Symbol.iterator](): Iterator { + throw inputError; + }, + }; + assert.throws( + () => windowsFileSystem(native).writeFile(widePath("C:\\output"), data), + closeError ? { winerror: closeError } : (error) => error === inputError, ); + assert.equal(closes, 1); }); } diff --git a/plugins/codex-security/native/windows-flags.mts b/plugins/codex-security/native/windows-flags.mts index fa88aab07..5d351facf 100644 --- a/plugins/codex-security/native/windows-flags.mts +++ b/plugins/codex-security/native/windows-flags.mts @@ -17,6 +17,7 @@ export const windowsFlags = { FILE_FLAG_OPEN_REPARSE_POINT: 0x00200000, FILE_FLAG_OVERLAPPED: 0x40000000, FILE_NAME_OPENED: 8, + VOLUME_NAME_GUID: 1, FILE_BEGIN: 0, FILE_CURRENT: 1, FILE_END: 2, diff --git a/sdk/typescript/tests-ts/security-policy-helper.test.ts b/sdk/typescript/tests-ts/security-policy-helper.test.ts index 74d2b78e0..303d4101d 100644 --- a/sdk/typescript/tests-ts/security-policy-helper.test.ts +++ b/sdk/typescript/tests-ts/security-policy-helper.test.ts @@ -162,6 +162,8 @@ describe("built SECURITY.md helper", () => { ], [{ HOMEDRIVE: drive, HOMEPATH: "current" }, "~/project", root], [{ USERPROFILE: `${drive}current` }, "~/project", root], + [{ USERPROFILE: drive }, "~/project", home], + [{ HOMEDRIVE: drive, HOMEPATH: "" }, "~/project", home], [{ USERPROFILE: "" }, "~", join(home, "project")], [{ HOMEDRIVE: drive, HOMEPATH: "" }, "~", join(home, "project")], ]; @@ -175,8 +177,18 @@ describe("built SECURITY.md helper", () => { expect(result.stdout).toContain("home-variable policy"); } expect(run(["--repo", root, "--scope", "~"], homeEnv({})).status).toBe(1); - const other = homeEnv({ USERPROFILE: `${home}\\`, USERNAME: "current" }); + const other = homeEnv({ USERPROFILE: home, USERNAME: "different" }); expect(run(["--repo", "~other", "--scope", "."], other).status).toBe(1); + for (const profile of [ + "\\\\host\\share\\", + "\\\\?\\UNC\\host\\share\\", + "\\\\?\\unc\\host\\share", + ]) { + const shareRoot = homeEnv({ USERPROFILE: profile, USERNAME: "share" }); + const result = run(["--repo", "~other", "--scope", "."], shareRoot); + expect(result.status, result.stderr).toBe(1); + expect(result.stderr).toContain("Could not determine home directory."); + } }, ); @@ -736,13 +748,23 @@ describe("built SECURITY.md helper", () => { const profiles = join(root, "profiles"); write(profiles, "current/SECURITY.md", "current policy\n"); write(profiles, "sibling/SECURITY.md", "sibling policy\n"); - const result = run(["--repo", "~sibling", "--scope", "~sibling"], { - ...process.env, - USERPROFILE: join(profiles, "current"), - USERNAME: "current", - }); - expect(result.status, result.stderr).toBe(0); - expect(result.stdout).toContain("sibling policy\n"); + for (const [home, cwd, scope] of [ + [join(profiles, "current"), undefined, "~sibling"], + [`${join(profiles, "current")}\\`, undefined, "~sibling"], + [`${root.slice(0, 2)}current`, profiles, "."], + ] as const) { + const result = run( + ["--repo", "~sibling", "--scope", scope], + { + ...process.env, + USERPROFILE: home, + USERNAME: "current", + }, + cwd, + ); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain("sibling policy\n"); + } }, ); @@ -787,15 +809,26 @@ describe("built SECURITY.md helper", () => { }); test.skipIf(process.platform !== "win32")( - "resolves drive-relative and rooted scopes using the repository drive", + "resolves Windows scopes using the repository drive and native normalization", () => { const { root } = fixture("İrepository"); write(root, "src/SECURITY.md", "component policy\n"); write(root, "src/app.ts", "export {};\n"); + const literalRoot = win32.toNamespacedPath(join(root, "src.")); + write(literalRoot, "SECURITY.md", "literal directory policy\n"); const drive = root.slice(0, 2); for (const scope of [ `${drive}src\\app.ts`, + "src/app.ts", + "src/../src/app.ts", + "src.", + "src ", + `${drive}src.`, + `${drive}src `, + win32.toNamespacedPath(join(root, "src", "app.ts")), join(root, "src", "app.ts").slice(2), + `${join(root, "src")}.`, + `${join(root, "src").slice(2)}.`, ]) { const result = run( ["--repo", root, "--scope", scope], @@ -807,6 +840,14 @@ describe("built SECURITY.md helper", () => { ["src/SECURITY.md", "component policy"], ]); } + for (const [repo, scope, source] of [ + [literalRoot, ".", "SECURITY.md"], + [win32.toNamespacedPath(root), "src.", "src./SECURITY.md"], + ] as const) { + const result = run(["--repo", repo, "--scope", scope]); + expect(result.status, result.stderr).toBe(0); + expectGuidance(result.stdout, [[source, "literal directory policy"]]); + } }, );