fix(windows): NuGet ONNX scan, LoadLibraryExW validation, storage mkdir, file-replacement retry (#66)

Zireael · web-flow · commit 4dae5cf1f4d4 · 2026-05-27T15:41:32.000+08:00
Windows hardening contributed by @Zireael in PR #66. Closes the remaining Windows sub-issues from #35 and #64: - ONNX Runtime discovery now scans NuGet Microsoft.ML.OnnxRuntime cache paths in addition to system installs, with case-insensitive dedup - LoadLibraryExW validation on Windows confirms the resolved ONNX library actually loads before AFT commits to it - 'aft-bridge' downloader handles EEXIST on Windows by unlinking before retry, fixing rare cache-write races - 'configure' creates storage directories with 'fs::create_dir_all' so first-run on Windows can't fail on missing parents - Semantic-index file replacement uses rename-with-retry to tolerate transient Windows file locks (AV scanners, Defender) - CLI 'diagnostics' creates its storage subdirectory before writing, matching the bridge behavior - PATH parsing in both bridge and CLI shares 'pathEntriesForPlatform' helper that rejects empty/'.'/null-byte/relative entries, preserving the v0.26.1 Windows DLL-hijacking defense
diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs
@@ -1,3 +1,4 @@
+use std::fs;
 use std::panic::{catch_unwind, AssertUnwindSafe};
 use std::path::{Component, Path, PathBuf};
 use std::process::Command;
@@ -1365,6 +1366,19 @@ pub fn handle_configure(req: &RawRequest, ctx: &AppContext) -> Response {
             &canonical_cache_root,
         ));
     if let Some(storage_dir) = next_config.storage_dir.clone() {
+        // Ensure the storage root directory exists so subsystems (trust,
+        // backups, checkpoints, DB, persistence) can create their sub-trees
+        // without a separate create_dir_all per subsystem. On fresh installs
+        // this directory hasn't been created yet, and every subsystem
+        // currently creates its own subdirectory lazily — but the root must
+        // exist for status/diagnostics to report a valid path.
+        if let Err(err) = fs::create_dir_all(&storage_dir) {
+            slog_warn!(
+                "failed to create storage directory {}: {}",
+                storage_dir.display(),
+                err
+            );
+        }
         ctx.backup().borrow_mut().set_storage_dir_for_harness(
             storage_dir,
             harness,
diff --git a/crates/aft/src/semantic_index.rs b/crates/aft/src/semantic_index.rs
@@ -30,7 +30,9 @@ const F32_BYTES: usize = std::mem::size_of::<f32>();
 const HEADER_BYTES_V1: usize = 9;
 const HEADER_BYTES_V2: usize = 13;
 const ONNX_RUNTIME_INSTALL_HINT: &str =
-    "ONNX Runtime not found. Install via: brew install onnxruntime (macOS) or apt install libonnxruntime (Linux).";
+    "ONNX Runtime not found. Install via: brew install onnxruntime (macOS), \
+     apt install libonnxruntime (Linux), or place onnxruntime.dll in your PATH (Windows). \
+     AFT can auto-download ONNX Runtime — run `npx @cortexkit/aft doctor` to diagnose.";
 
 const SEMANTIC_INDEX_VERSION_V1: u8 = 1;
 const SEMANTIC_INDEX_VERSION_V2: u8 = 2;
@@ -788,8 +790,136 @@ pub fn pre_validate_onnx_runtime() -> Result<(), String> {
 
     #[cfg(target_os = "windows")]
     {
-        // On Windows, skip pre-validation — let ort handle LoadLibrary
-        let _ = dylib_path;
+        // Validate ONNX Runtime availability on Windows by loading the DLL
+        // via LoadLibraryExW before the ort crate attempts its own LoadLibrary.
+        // This way we can produce a friendly error (with installation hints)
+        // instead of a raw LoadLibrary failure from deep inside fastembed.
+        let lib_name = dylib_path.as_deref().unwrap_or("onnxruntime.dll");
+
+        // Use kernel32 LoadLibraryExW for the validation — built-in, no
+        // crate dependency required. GetModuleFileNameW resolves the loaded
+        // DLL path for version probing via the version.dll API.
+        #[link(name = "kernel32")]
+        extern "system" {
+            fn LoadLibraryExW(
+                lpLibFileName: *const u16,
+                hFile: *mut std::ffi::c_void,
+                dwFlags: u32,
+            ) -> *mut std::ffi::c_void;
+            fn FreeLibrary(hLibModule: *mut std::ffi::c_void) -> i32;
+            fn GetModuleFileNameW(
+                hModule: *mut std::ffi::c_void,
+                lpFilename: *mut u16,
+                nSize: u32,
+            ) -> u32;
+        }
+
+        #[link(name = "version")]
+        extern "system" {
+            fn GetFileVersionInfoSizeW(
+                lptstrFilename: *const u16,
+                lpdwHandle: *mut u32,
+            ) -> u32;
+            fn GetFileVersionInfoW(
+                lptstrFilename: *const u16,
+                dwHandle: u32,
+                dwLen: u32,
+                lpData: *mut std::ffi::c_void,
+            ) -> i32;
+            fn VerQueryValueW(
+                pBlock: *mut std::ffi::c_void,
+                lpSubBlock: *const u16,
+                lplpBuffer: *mut *mut std::ffi::c_void,
+                puLen: *mut u32,
+            ) -> i32;
+        }
+
+        #[repr(C)]
+        struct VS_FIXEDFILEINFO {
+            dw_signature: u32,
+            dw_struc_version: u32,
+            dw_file_version_ms: u32,   // HIWORD major, LOWORD minor
+            dw_file_version_ls: u32,   // HIWORD build, LOWORD revision
+            dw_product_version_ms: u32,
+            dw_product_version_ls: u32,
+            dw_file_flags_mask: u32,
+            dw_file_flags: u32,
+            dw_file_os: u32,
+            dw_file_type: u32,
+            dw_file_subtype: u32,
+            dw_file_date_ms: u32,
+            dw_file_date_ls: u32,
+        }
+
+        unsafe {
+            use std::os::windows::ffi::OsStrExt;
+            let wide: Vec<u16> = std::ffi::OsStr::new(lib_name)
+                .encode_wide()
+                .chain(std::iter::once(0))
+                .collect();
+
+            let handle = LoadLibraryExW(wide.as_ptr(), std::ptr::null_mut(), 0);
+            if handle.is_null() {
+                let err = std::io::Error::last_os_error();
+                return Err(format!(
+                    "ONNX Runtime not found. LoadLibraryExW('{}') failed: {}. \
+                     Run `npx @cortexkit/aft doctor` to diagnose.",
+                    lib_name, err
+                ));
+            }
+
+            // Probe the file version from PE resources so we can reject
+            // outdated DLLs (e.g. v1.9.x) before the ort crate panics.
+            let mut detected_major: u32 = 0;
+            let mut detected_minor: u32 = 0;
+            let mut path_buf = [0u16; 260];
+            let path_len = GetModuleFileNameW(handle, path_buf.as_mut_ptr(), 260);
+            if path_len > 0 {
+                let mut dummy_handle: u32 = 0;
+                let info_size =
+                    GetFileVersionInfoSizeW(path_buf.as_ptr(), &mut dummy_handle);
+                if info_size > 0 {
+                    let mut info = vec![0u8; info_size as usize];
+                    if GetFileVersionInfoW(
+                        path_buf.as_ptr(),
+                        0,
+                        info_size,
+                        info.as_mut_ptr() as *mut std::ffi::c_void,
+                    ) != 0
+                    {
+                        let sub_block = "\\\0".encode_utf16().collect::<Vec<u16>>();
+                        let mut vs_info: *mut std::ffi::c_void =
+                            std::ptr::null_mut();
+                        let mut vs_len: u32 = 0;
+                        if VerQueryValueW(
+                            info.as_mut_ptr() as *mut std::ffi::c_void,
+                            sub_block.as_ptr(),
+                            &mut vs_info,
+                            &mut vs_len,
+                        ) != 0
+                            && !vs_info.is_null()
+                        {
+                            let fixed = vs_info as *const VS_FIXEDFILEINFO;
+                            detected_major = (*fixed).dw_file_version_ms >> 16;
+                            detected_minor =
+                                (*fixed).dw_file_version_ms & 0xFFFF;
+                        }
+                    }
+                }
+            }
+
+            FreeLibrary(handle);
+
+            // Version compatibility check (mirrors the Linux/macOS path).
+            // If version could not be detected (detected_major == 0) we let
+            // the load succeed — the ort crate will diagnose further.
+            if detected_major != 0
+                && (detected_major != 1 || detected_minor < 20)
+            {
+                let ver = format!("{}.{}", detected_major, detected_minor);
+                return Err(format_ort_version_mismatch(&ver, lib_name));
+            }
+        }
     }
 
     Ok(())
@@ -839,7 +969,6 @@ fn extract_version_from_filename(name: &str) -> Option<String> {
     re.find(name).map(|m| m.as_str().to_string())
 }
 
-#[cfg(any(test, target_os = "linux", target_os = "macos"))]
 fn suggest_removal_command(lib_path: &str) -> String {
     if lib_path.starts_with("/usr/local/lib")
         || lib_path == "libonnxruntime.so"
@@ -860,7 +989,6 @@ fn suggest_removal_command(lib_path: &str) -> String {
 /// stability — the auto-fix recommendation must always come first because
 /// it's the only safe option, and the system-rm step must remain present
 /// because some users prefer the system-wide cleanup path.
-#[cfg(any(test, target_os = "linux", target_os = "macos"))]
 pub(crate) fn format_ort_version_mismatch(version: &str, lib_name: &str) -> String {
     format!(
         "ONNX Runtime version mismatch: found v{} at '{}', but AFT requires v1.20+. \
diff --git a/packages/aft-bridge/src/downloader.ts b/packages/aft-bridge/src/downloader.ts
@@ -269,7 +269,16 @@ export async function downloadBinary(version?: string): Promise<string | null> {
       chmodSync(tmpPath, 0o755);
     }
 
-    // Atomic rename
+    // Replace binary — on Windows renameSync fails (EEXIST) when the target
+    // exists, so unlink first. This creates a brief window where no binary
+    // exists at binaryPath — callers should handle a missing binary gracefully.
+    if (process.platform === "win32" && existsSync(binaryPath)) {
+      try {
+        unlinkSync(binaryPath);
+      } catch {
+        // best-effort; renameSync will surface the error if unlink fails
+      }
+    }
     renameSync(tmpPath, binaryPath);
 
     log(`AFT binary ready at ${binaryPath}`);
diff --git a/packages/aft-bridge/src/onnx-runtime.ts b/packages/aft-bridge/src/onnx-runtime.ts
@@ -459,36 +459,89 @@ function findSystemOnnxRuntime(libName?: string): string | null {
       "/usr/local/lib",
     );
   } else if (process.platform === "win32") {
-    // Windows users often install ONNX Runtime via Scoop or a manual zip and
-    // put the directory containing `onnxruntime.dll` on PATH. We only consider
-    // absolute PATH entries (never the current directory or relative paths).
-    // Returning one of those directories means the plugin may pass it to Rust
-    // as the ORT DLL directory; that mirrors Windows loader semantics for a
-    // user-controlled PATH while avoiding accidental current-dir DLL loads.
-    // Doctor-only probes may be looser, but the bridge path can become a real
-    // load path and should stay conservative.
+    // Common Windows install locations for ONNX Runtime
+    const programFiles = process.env.ProgramFiles ?? "C:\\Program Files";
+    const programFilesX86 = process.env["ProgramFiles(x86)"] ?? "C:\\Program Files (x86)";
+    searchPaths.push(
+      join(programFiles, "onnxruntime", "lib"),
+      join(programFiles, "Microsoft ONNX Runtime", "lib"),
+      join(programFiles, "Microsoft Machine Learning", "lib"),
+      join(programFilesX86, "onnxruntime", "lib"),
+      // Windows NuGet package layout:
+      //   <user>\.nuget\packages\microsoft.ml.onnxruntime\<version>\runtimes\win-{x64,arm64}\native\
+      // Scan all installed versions since we don't know which one is present.
+      ...(() => {
+        const nugetPaths: string[] = [];
+        const userProfile = process.env.USERPROFILE ?? "";
+        if (!userProfile) return nugetPaths;
+        const nugetPackageDir = join(userProfile, ".nuget", "packages", "microsoft.ml.onnxruntime");
+        if (!existsSync(nugetPackageDir)) return nugetPaths;
+        try {
+          for (const entry of readdirSync(nugetPackageDir, { withFileTypes: true })) {
+            if (!entry.isDirectory()) continue;
+            // Skip well-known non-version entries
+            if (entry.name === "__globalPackagesFolder" || entry.name.startsWith(".")) continue;
+            nugetPaths.push(
+              join(nugetPackageDir, entry.name, "runtimes", "win-x64", "native"),
+              join(nugetPackageDir, entry.name, "runtimes", "win-arm64", "native"),
+            );
+          }
+        } catch {
+          // best-effort scan
+        }
+        return nugetPaths;
+      })(),
+    );
+    // Also include absolute PATH entries (reuses the existing helper that
+    // validates absolute paths, rejects null bytes, strips quotes, excludes ".").
     searchPaths.push(...pathEntriesForPlatform());
   }
 
+  // Deduplicate paths while preserving order.
+  // On case-insensitive filesystems (Windows, macOS) normalize casing for
+  // comparison; on Linux the raw path casing is the authority.
+  const normalizeCase = process.platform === "win32" || process.platform === "darwin";
   const seen = new Set<string>();
-  for (const dir of searchPaths) {
-    if (seen.has(dir)) continue;
-    seen.add(dir);
-    if (!directoryContainsLibrary(dir, libName)) continue;
-
-    // Reject system installs that the Rust pre-validator will refuse. Without
-    // this filter, a stale distro package (e.g. libonnxruntime1.9 on Ubuntu
-    // 22.04) shadows our auto-downloaded v1.24 forever and semantic search
-    // stays "failed" until the user hand-deletes the system library.
-    const version = detectOnnxVersion(dir, libName);
-    if (version && !isOnnxVersionCompatible(version)) {
-      warn(
-        `Skipping system ONNX Runtime at ${dir} (v${version}); AFT requires ` +
-          `v${REQUIRED_ORT_MAJOR}.${REQUIRED_ORT_MIN_MINOR}+. Falling through to AFT-managed download.`,
-      );
+  const uniquePaths = searchPaths.filter((p) => {
+    let key = resolve(p).replace(/[/\\]+$/, "");
+    if (normalizeCase) key = key.toLowerCase();
+    if (seen.has(key)) return false;
+    seen.add(key);
+    return true;
+  });
+
+  for (const dir of uniquePaths) {
+    const libPath = join(dir, libName);
+    if (process.platform === "win32") {
+      if (!directoryContainsLibrary(dir, libName)) continue;
+    } else if (!existsSync(libPath)) {
       continue;
     }
 
+    // Skip the version check for PATH entries — version-suffixed filenames
+    // are less common on Windows and we want PATH discovery to succeed.
+    let skipVersionCheck = false;
+    if (process.platform === "win32") {
+      // Only do version check for common install paths, not arbitrary PATH entries
+      const isCommonPath = dir.includes("Program Files") || dir.includes("onnxruntime");
+      if (!isCommonPath) skipVersionCheck = true;
+    }
+
+    if (!skipVersionCheck) {
+      // Reject system installs that the Rust pre-validator will refuse. Without
+      // this filter, a stale distro package (e.g. libonnxruntime1.9 on Ubuntu
+      // 22.04) shadows our auto-downloaded v1.24 forever and semantic search
+      // stays "failed" until the user hand-deletes the system library.
+      const version = detectOnnxVersion(dir, libName);
+      if (version && !isOnnxVersionCompatible(version)) {
+        warn(
+          `Skipping system ONNX Runtime at ${dir} (v${version}); AFT requires ` +
+            `v${REQUIRED_ORT_MAJOR}.${REQUIRED_ORT_MIN_MINOR}+. Falling through to AFT-managed download.`,
+        );
+        continue;
+      }
+    }
+
     return dir;
   }
 
diff --git a/packages/aft-bridge/src/resolver.ts b/packages/aft-bridge/src/resolver.ts
@@ -1,5 +1,5 @@
 import { execSync } from "node:child_process";
-import { chmodSync, copyFileSync, existsSync, mkdirSync, renameSync } from "node:fs";
+import { chmodSync, copyFileSync, existsSync, mkdirSync, renameSync, unlinkSync } from "node:fs";
 import { createRequire } from "node:module";
 import { homedir } from "node:os";
 import { join } from "node:path";
@@ -55,6 +55,14 @@ function copyToVersionedCache(npmBinaryPath: string, knownVersion?: string): str
     if (process.platform !== "win32") {
       chmodSync(tmpPath, 0o755);
     }
+    // Best-effort replace — unlink first on Windows where renameSync fails if target exists
+    if (process.platform === "win32" && existsSync(cachedPath)) {
+      try {
+        unlinkSync(cachedPath);
+      } catch {
+        // best-effort; renameSync will surface the error if unlink fails
+      }
+    }
     renameSync(tmpPath, cachedPath);
     log(`Copied npm binary to versioned cache: ${cachedPath}`);
     return cachedPath;
diff --git a/packages/aft-cli/src/lib/diagnostics.ts b/packages/aft-cli/src/lib/diagnostics.ts
@@ -1,4 +1,4 @@
-import { closeSync, existsSync, openSync, readSync, statSync } from "node:fs";
+import { closeSync, existsSync, mkdirSync, openSync, readSync, statSync } from "node:fs";
 import type { HarnessAdapter } from "../adapters/types.js";
 import { type BinaryCacheInfo, getBinaryCacheInfo } from "./binary-cache.js";
 import { probeBinaryVersion } from "./binary-probe.js";
@@ -113,6 +113,18 @@ async function diagnoseHarness(adapter: HarnessAdapter): Promise<HarnessDiagnost
   const logPath = adapter.getLogFile();
   const pluginCache = adapter.getPluginCacheInfo();
 
+  // Ensure the storage directory exists so diagnostics report useful info
+  // instead of "(not created)" on fresh installs. The storage directory is
+  // normally created lazily by the bridge on first tool call, but the doctor
+  // command is a read-only diagnostic that should still display it.
+  if (!existsSync(storage)) {
+    try {
+      mkdirSync(storage, { recursive: true });
+    } catch {
+      // best-effort — diagnostics can still report the path as (not created)
+    }
+  }
+
   const describeStorage =
     "describeStorageSubtrees" in adapter &&
     typeof (adapter as unknown as { describeStorageSubtrees: () => Record<string, number> })
diff --git a/packages/aft-cli/src/lib/onnx.ts b/packages/aft-cli/src/lib/onnx.ts
