Skip to content

Commit fb3210b

Browse files
committed
fix(onnx): reject unversioned System32 runtime
1 parent 62b1c8c commit fb3210b

8 files changed

Lines changed: 264 additions & 67 deletions

File tree

packages/aft-bridge/src/__tests__/onnx-version-filter.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,31 @@ describe("findSystemOnnxRuntime (Windows PATH)", () => {
224224
);
225225
});
226226

227+
test("rejects an unversioned ONNX Runtime from Windows System32", async () => {
228+
const windowsDir = join(workDir, "Windows");
229+
const systemDir = join(windowsDir, "System32");
230+
mkdirSync(systemDir, { recursive: true });
231+
writeFileSync(join(systemDir, "onnxruntime.dll"), "Windows component");
232+
233+
await withEnv(
234+
{
235+
PATH: systemDir,
236+
Path: undefined,
237+
path: undefined,
238+
SystemRoot: windowsDir,
239+
windir: undefined,
240+
ProgramFiles: join(workDir, "program-files"),
241+
"ProgramFiles(x86)": join(workDir, "program-files-x86"),
242+
USERPROFILE: join(workDir, "profile-without-nuget"),
243+
},
244+
async () => {
245+
const found = withPlatform("win32", () => findSystemOnnxRuntime("onnxruntime.dll"));
246+
247+
expect(found).toBeNull();
248+
},
249+
);
250+
});
251+
227252
test("ignores non-existent PATH directories without throwing", async () => {
228253
await withEnv(
229254
{

packages/aft-bridge/src/onnx-runtime.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,26 @@ function pathEntriesForPlatform(): string[] {
470470
});
471471
}
472472

473+
function isWindowsSystem32Directory(dir: string): boolean {
474+
if (process.platform !== "win32") return false;
475+
476+
const normalizedDir = win32
477+
.resolve(dir)
478+
.replace(/[\\/]+$/, "")
479+
.toLowerCase();
480+
const windowsRoots = [process.env.SystemRoot, process.env.windir, "C:\\Windows"];
481+
return windowsRoots.some((root) => {
482+
if (!root) return false;
483+
return (
484+
normalizedDir ===
485+
win32
486+
.resolve(root, "System32")
487+
.replace(/[\\/]+$/, "")
488+
.toLowerCase()
489+
);
490+
});
491+
}
492+
473493
function directoryContainsLibrary(dir: string, libName: string): boolean {
474494
try {
475495
const entries = readdirSync(dir);
@@ -583,6 +603,15 @@ function findSystemOnnxRuntime(libName?: string): string | null {
583603
// it shadow a later candidate with a known compatible version.
584604
const version = detectOnnxVersion(dir, libName);
585605
if (!version) {
606+
// Windows ships an unversioned ONNX Runtime in System32 on some releases.
607+
// Rust can discover that DLL is too old only after startup, so never let
608+
// an unverifiable OS copy prevent AFT from downloading its managed runtime.
609+
if (isWindowsSystem32Directory(dir)) {
610+
warn(
611+
`Skipping unversioned Windows system ONNX Runtime at ${dir}; falling through to AFT-managed download.`,
612+
);
613+
continue;
614+
}
586615
unknownVersionPaths.push(dir);
587616
continue;
588617
}

packages/aft-cli/src/__tests__/onnx-fix.test.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,26 @@ describe("findOnnxFixCandidates", () => {
157157
expect(candidates).toHaveLength(1);
158158
expect(candidates[0].reason).toContain("system ONNX Runtime");
159159
expect(candidates[0].reason).toContain("1.9.0");
160-
expect(candidates[0].reason).toContain("v0.19.5+ skips incompatible system installs");
160+
expect(candidates[0].reason).toContain("download v1.24");
161+
});
162+
163+
test("flags a missing runtime after an unreadable Windows system copy is ignored", () => {
164+
const report = makeReport([
165+
makeHarness({
166+
onnxRuntime: {
167+
...makeHarness().onnxRuntime,
168+
ignoredSystemPath: "C:\\Windows\\System32",
169+
ignoredSystemReason: "version unreadable (Windows system copy) — ignored",
170+
platform: "win32-x64",
171+
},
172+
}),
173+
]);
174+
175+
const candidates = findOnnxFixCandidates(report);
176+
177+
expect(candidates).toHaveLength(1);
178+
expect(candidates[0].reason).toContain("version is unreadable");
179+
expect(candidates[0].reason).toContain("download v1.24");
161180
});
162181

163182
test("does NOT flag system install when a compatible cached install exists", () => {
@@ -247,10 +266,12 @@ describe("runOnnxFix", () => {
247266
rmFn: (path) => {
248267
removed.push(path);
249268
},
269+
ensureFn: async () => join(storagePath, "onnxruntime", "1.24.4"),
250270
});
251271

252272
expect(result).not.toBe(null);
253273
expect(result?.cleared).toBe(1);
274+
expect(result?.installed).toBe(1);
254275
expect(result?.errors).toEqual([]);
255276
// Critical safety: the path deleted must be inside our test workDir,
256277
// never `/usr/lib/...`.
@@ -279,6 +300,7 @@ describe("runOnnxFix", () => {
279300
await runOnnxFix([], report, {
280301
confirmFn: async () => true,
281302
rmFn: (path) => removed.push(path),
303+
ensureFn: async () => join(storagePath, "onnxruntime", "1.24.4"),
282304
});
283305

284306
// The candidate's storage dir doesn't exist (we never downloaded
@@ -290,4 +312,32 @@ describe("runOnnxFix", () => {
290312
expect(path).toContain(workDir);
291313
}
292314
});
315+
316+
test("downloads a managed runtime immediately when none is installed", async () => {
317+
const storagePath = join(workDir, "storage");
318+
const report = makeReport([
319+
makeHarness({
320+
storageDir: { path: storagePath, exists: false, accessible: false, sizesByKey: {} },
321+
onnxRuntime: {
322+
...makeHarness().onnxRuntime,
323+
ignoredSystemPath: "C:\\Windows\\System32",
324+
ignoredSystemReason: "version unreadable (Windows system copy) — ignored",
325+
platform: "win32-x64",
326+
},
327+
}),
328+
]);
329+
const ensured: string[] = [];
330+
331+
const result = await runOnnxFix([], report, {
332+
yes: true,
333+
ensureFn: async (storageDir) => {
334+
ensured.push(storageDir);
335+
return join(storageDir, "onnxruntime", "1.24.4");
336+
},
337+
});
338+
339+
expect(ensured).toEqual([storagePath]);
340+
expect(result?.installed).toBe(1);
341+
expect(result?.errors).toEqual([]);
342+
});
293343
});

packages/aft-cli/src/__tests__/onnx.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { join } from "node:path";
77
import { __test__ as bridgeOnnxTest } from "../../../aft-bridge/src/onnx-runtime.js";
88
import {
99
findCachedOnnxRuntime,
10+
findIgnoredWindowsSystemOnnxRuntime,
1011
findSystemOnnxRuntime,
1112
getOnnxLibraryName,
1213
ONNX_RUNTIME_VERSION,
@@ -26,6 +27,8 @@ beforeEach(() => {
2627
["USERPROFILE", process.env.USERPROFILE],
2728
["ProgramFiles", process.env.ProgramFiles],
2829
["ProgramFiles(x86)", process.env["ProgramFiles(x86)"]],
30+
["SystemRoot", process.env.SystemRoot],
31+
["windir", process.env.windir],
2932
]);
3033
});
3134

@@ -64,6 +67,29 @@ describe("CLI ONNX system detection", () => {
6467
expect(found).toBe(runtimeDir);
6568
});
6669

70+
test("doctor rejects an unversioned ONNX Runtime from Windows System32", () => {
71+
const windowsDir = join(workDir, "Windows");
72+
const systemDir = join(windowsDir, "System32");
73+
mkdirSync(systemDir, { recursive: true });
74+
writeFileSync(join(systemDir, "onnxruntime.dll"), "Windows component");
75+
process.env.PATH = systemDir;
76+
process.env.SystemRoot = windowsDir;
77+
process.env.ProgramFiles = join(workDir, "program-files");
78+
process.env["ProgramFiles(x86)"] = join(workDir, "program-files-x86");
79+
process.env.USERPROFILE = join(workDir, "profile-without-nuget");
80+
delete process.env.Path;
81+
delete process.env.path;
82+
delete process.env.windir;
83+
84+
const result = withPlatform("win32", () => ({
85+
found: findSystemOnnxRuntime(),
86+
ignored: findIgnoredWindowsSystemOnnxRuntime(),
87+
}));
88+
89+
expect(result.found).toBeNull();
90+
expect(result.ignored).toBe(systemDir);
91+
});
92+
6793
test("doctor skips incompatible Windows PATH DLLs and selects a compatible candidate", () => {
6894
const oldRuntimeDir = join(workDir, "onnxruntime-1.9.0", "bin");
6995
const newRuntimeDir = join(workDir, "onnxruntime-1.24.4", "bin");

packages/aft-cli/src/commands/doctor.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,10 @@ export async function runDoctor(options: DoctorOptions): Promise<number> {
194194
parts.push(
195195
`system: ${h.onnxRuntime.systemVersion ?? "unknown"}${h.onnxRuntime.systemCompatible === false ? " (incompatible)" : ""}`,
196196
);
197+
} else if (h.onnxRuntime.ignoredSystemPath) {
198+
parts.push(
199+
`system: ${h.onnxRuntime.ignoredSystemReason} at ${h.onnxRuntime.ignoredSystemPath}`,
200+
);
197201
}
198202
if (!h.onnxRuntime.cachedPath && !h.onnxRuntime.systemPath) {
199203
parts.push(`not installed — ${h.onnxRuntime.installHint}`);
@@ -606,12 +610,12 @@ export function buildDoctorFixPlan(
606610
if (candidate.storageOnnxBytes > 0) {
607611
items.push({
608612
kind: "onnx",
609-
message: `Will delete AFT-managed ONNX cache at ${candidate.storageOnnxDir} (${formatBytes(candidate.storageOnnxBytes)})`,
613+
message: `Will replace AFT-managed ONNX cache at ${candidate.storageOnnxDir} (${formatBytes(candidate.storageOnnxBytes)}) and download a compatible runtime`,
610614
});
611615
} else {
612616
items.push({
613617
kind: "onnx",
614-
message: `Will leave system ONNX untouched and refresh AFT-managed ONNX state for ${candidate.harness.displayName} on next start`,
618+
message: `Will leave system ONNX untouched and download a compatible AFT-managed runtime for ${candidate.harness.displayName}`,
615619
});
616620
}
617621
}

packages/aft-cli/src/lib/diagnostics.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { getLspCacheReport, type LspCacheReport } from "./lsp-cache.js";
1818
import {
1919
detectOrtVersion,
2020
findCachedOnnxRuntime,
21+
findIgnoredWindowsSystemOnnxRuntime,
2122
findSystemOnnxRuntime,
2223
getManualInstallHint,
2324
isOrtVersionCompatible,
@@ -86,6 +87,8 @@ export interface HarnessDiagnostic {
8687
systemPath: string | null;
8788
systemVersion: string | null;
8889
systemCompatible: boolean | null;
90+
ignoredSystemPath?: string | null;
91+
ignoredSystemReason?: string | null;
8992
cachedPath: string | null;
9093
cachedVersion: string | null;
9194
cachedCompatible: boolean | null;
@@ -174,6 +177,7 @@ async function diagnoseHarness(adapter: HarnessAdapter): Promise<HarnessDiagnost
174177
true);
175178

176179
const systemOrtDir = findSystemOnnxRuntime();
180+
const ignoredSystemOrtDir = systemOrtDir ? null : findIgnoredWindowsSystemOnnxRuntime();
177181
const cachedOrtDir = findCachedOnnxRuntime(storage);
178182
const systemVersion = systemOrtDir ? detectOrtVersion(systemOrtDir) : null;
179183
const cachedVersion = cachedOrtDir ? detectOrtVersion(cachedOrtDir) : null;
@@ -205,6 +209,10 @@ async function diagnoseHarness(adapter: HarnessAdapter): Promise<HarnessDiagnost
205209
systemPath: systemOrtDir,
206210
systemVersion,
207211
systemCompatible: systemVersion ? isOrtVersionCompatible(systemVersion) : null,
212+
ignoredSystemPath: ignoredSystemOrtDir,
213+
ignoredSystemReason: ignoredSystemOrtDir
214+
? "version unreadable (Windows system copy) — ignored"
215+
: null,
208216
cachedPath: cachedOrtDir,
209217
cachedVersion,
210218
cachedCompatible: cachedVersion ? isOrtVersionCompatible(cachedVersion) : null,
@@ -410,7 +418,9 @@ export function collectDiagnosticIssues(report: DiagnosticReport): DiagnosticIss
410418
code: "onnx_missing",
411419
severity: "medium",
412420
scope: h.displayName,
413-
message: "ONNX Runtime is required for semantic search but was not detected.",
421+
message: h.onnxRuntime.ignoredSystemPath
422+
? `ONNX Runtime at ${h.onnxRuntime.ignoredSystemPath} has a ${h.onnxRuntime.ignoredSystemReason}; no compatible runtime was detected.`
423+
: "ONNX Runtime is required for semantic search but was not detected.",
414424
remediation: `Run \`aft doctor --fix\` or install ONNX Runtime manually (${h.onnxRuntime.installHint}).`,
415425
});
416426
}

0 commit comments

Comments
 (0)