fix(cli): fix Windows CLI detection false negatives (#7753, #7774) (#7831)

checkKnownPath() only validated the RESOLVED realpath target against
EXPECTED_PARENT_PATHS, never crediting that the candidate path itself was
already constructed from a trusted root by getKnownToolPaths(). Version
managers that install via symlinks/junctions (nvm-windows and more broadly
nvm/asdf/pyenv-style tools) place the shim inside a trusted root but its
resolved target lives in a private per-version store outside the allowlist,
so it was misreported as symlink_escape (#7753). Fix: trust a candidate
location if EITHER its original path OR its resolved target falls within
EXPECTED_PARENT_PATHS.

Separately, locateCommandCandidate() short-circuited on the FIRST known-path
candidate that returned any non-not_found failure reason, without trying the
remaining candidates or ever falling back to a real PATH search. A single
stray artifact at one guessed Windows install location for claude therefore
poisoned detection entirely even when the real binary was resolvable via PATH
(#7774). Fix: remember non-fatal known-path failures but keep walking every
candidate, and always fall through to the PATH-based search before giving up.

Extracted both helpers into a new cliRuntimeKnownPath.ts module (dependency
injected, no circular import) to keep cliRuntime.ts under its frozen
file-size budget.

changelog.d/fixes/7753-windows-cli-path.md

@@ -0,0 +1 @@
+- fix(cli): resolve Windows CLI detection false negatives — nvm-windows symlinked binaries no longer rejected as symlink_escape, and a stray known-path artifact no longer hides a genuinely runnable Claude CLI on the PATH fallback (#7753, #7774)

src/shared/services/cliRuntime.ts

@@ -7,6 +7,7 @@ import { getHermesHome } from "@/lib/cli-helper/config-generator/hermesHome";
 import { getCachedLoginShellPath, mergeShellPath } from "./loginShellPath";
 import { withSettingsFallback } from "./cliInstallFallback";
 import { GROK_BUILD_RUNTIME_ENTRY, AMP_RUNTIME_ENTRY } from "./cliRuntimeGrokBuild";
+import { isLocationTrusted, findKnownPathMatch } from "./cliRuntimeKnownPath";
 const VALID_RUNTIME_MODES = new Set(["auto", "host", "container"]);
 const FALSE_VALUES = new Set(["0", "false", "no", "off"]);
 
@@ -791,7 +792,7 @@ export const locateCommand = async (command: string, env: Record<string, string
  * - Verifies file is a regular file (not directory, pipe, or device)
  * - Checks file size bounds (30B - 100MB) to detect suspicious binaries
  */
-const checkKnownPath = async (commandPath: string) => {
+export const checkKnownPath = async (commandPath: string) => {
   if (!path.isAbsolute(commandPath)) {
     return { installed: false, commandPath: null, reason: "not_absolute" };
   }
@@ -804,27 +805,21 @@ const checkKnownPath = async (commandPath: string) => {
     // Resolve symlinks to get the real path and detect symlink escapes
     const realPath = await fs.realpath(commandPath);
 
-    // Verify the resolved path is still within expected directories
-    // Use pre-computed expected parent paths (cached at module startup for performance).
-    // On macOS temp directories often resolve from /var -> /private/var, so compare both
-    // the configured parent and its canonical realpath when available.
-    let isWithinExpected = false;
-    for (const parent of EXPECTED_PARENT_PATHS) {
-      if (isPathWithin(realPath, parent)) {
-        isWithinExpected = true;
-        break;
-      }
-
-      try {
-        const resolvedParent = await fs.realpath(parent);
-        if (isPathWithin(realPath, resolvedParent)) {
-          isWithinExpected = true;
-          break;
-        }
-      } catch {
-        // Ignore missing/unresolvable parents and continue checking the remaining ones.
-      }
-    }
+    // Verify the resolved path — OR the original pre-resolution path — is within
+    // expected directories. Use pre-computed expected parent paths (cached at module
+    // startup for performance). On macOS temp directories often resolve from
+    // /var -> /private/var, so compare both the configured parent and its canonical
+    // realpath when available.
+    //
+    // #7753: also trust the pre-resolution `commandPath` itself, not just `realPath`
+    // — see isLocationTrusted() in cliRuntimeKnownPath.ts for the rationale.
+    const isWithinExpected = await isLocationTrusted(
+      commandPath,
+      realPath,
+      EXPECTED_PARENT_PATHS,
+      isPathWithin,
+      fs.realpath
+    );
 
     if (!isWithinExpected) {
       return { installed: false, commandPath: null, reason: "symlink_escape" };
@@ -862,6 +857,8 @@ const checkKnownPath = async (commandPath: string) => {
   }
 };
 
+type KnownPathResult = Awaited<ReturnType<typeof checkKnownPath>>;
+
 const locateCommandCandidate = async (
   commands: string[],
   env: Record<string, string | undefined>,
@@ -873,35 +870,22 @@ const locateCommandCandidate = async (
 
   // SECURITY: First check known installation paths for this specific tool
   // This avoids searching PATH and reduces attack surface
+  let bestKnownPathFailure: KnownPathResult | null = null;
   if (toolId) {
-    const knownPaths = getKnownToolPaths(toolId);
-    for (const knownPath of knownPaths) {
-      const result = await checkKnownPath(knownPath);
-      if (result.installed && result.reason === null) {
-        return {
-          command: commands[0],
-          installed: true,
-          commandPath: result.commandPath,
-          reason: null,
-        };
-      }
-
-      if (result.installed && result.reason === "not_executable") {
-        return {
-          command: commands[0],
-          installed: true,
-          commandPath: result.commandPath,
-          reason: "not_executable",
-        };
-      }
-
-      if (result.reason && result.reason !== "not_found") {
-        return { command: commands[0], ...result };
-      }
+    const { match, bestFailure } = await findKnownPathMatch(getKnownToolPaths(toolId), checkKnownPath);
+    if (match) {
+      return {
+        command: commands[0],
+        installed: true,
+        commandPath: match.commandPath,
+        reason: match.reason,
+      };
     }
+    bestKnownPathFailure = bestFailure;
   }
 
-  // Fallback: search PATH (user can set CLI_EXTRA_PATHS if needed)
+  // Always try PATH — a stray/broken known-path guess must never hide a genuinely
+  // PATH-resolvable binary (#7774). User can also set CLI_EXTRA_PATHS if needed.
   for (const command of commands) {
     const located = await locateCommand(command, env);
     if (located.installed || located.reason !== "not_found") {
@@ -909,6 +893,9 @@ const locateCommandCandidate = async (
     }
   }
 
+  if (bestKnownPathFailure) {
+    return { command: commands[0], ...bestKnownPathFailure };
+  }
   return { command: commands[0], installed: false, commandPath: null, reason: "not_found" };
 };
 

src/shared/services/cliRuntimeKnownPath.ts

@@ -0,0 +1,79 @@
+/**
+ * Known-install-path trust & candidate-matching helpers for cliRuntime.ts.
+ *
+ * Extracted from cliRuntime.ts (which is at its frozen file-size budget) so the
+ * #7753/#7774 fixes don't grow that file. Both helpers are dependency-injected
+ * (no import back into cliRuntime.ts) to avoid a circular module reference.
+ */
+
+export type PathWithinFn = (childPath: string, parentPath: string) => boolean;
+
+/**
+ * #7753 — nvm-windows/nvm/asdf/pyenv symlink-escape false positive.
+ *
+ * Check whether either the original (pre-resolution) candidate path or its
+ * resolved realpath target falls within a trusted parent directory. Checking
+ * both — not just the resolved target — credits that `commandPath` was already
+ * constructed from a trusted root by `getKnownToolPaths()`, so a symlink/junction
+ * placed there by a version manager (nvm-windows, nvm, asdf, pyenv) whose
+ * resolved target lives in a private per-version store outside
+ * `EXPECTED_PARENT_PATHS` is still recognized as legitimate. A symlink whose
+ * ORIGINAL location is also untrusted (not reachable through the trusted-root
+ * candidate generator) stays rejected.
+ */
+export const isLocationTrusted = async (
+  commandPath: string,
+  realPath: string,
+  expectedParentPaths: string[],
+  isPathWithin: PathWithinFn,
+  realpath: (targetPath: string) => Promise<string>
+): Promise<boolean> => {
+  for (const parent of expectedParentPaths) {
+    if (isPathWithin(commandPath, parent) || isPathWithin(realPath, parent)) {
+      return true;
+    }
+
+    try {
+      const resolvedParent = await realpath(parent);
+      if (isPathWithin(commandPath, resolvedParent) || isPathWithin(realPath, resolvedParent)) {
+        return true;
+      }
+    } catch {
+      // Ignore missing/unresolvable parents and continue checking the remaining ones.
+    }
+  }
+  return false;
+};
+
+export type KnownPathCheckResult = {
+  installed: boolean;
+  commandPath: string | null;
+  reason: string | null;
+};
+
+/**
+ * #7774 — known-path short-circuit hides a genuinely runnable binary.
+ *
+ * Walk the known-install-path candidates for a tool. A genuine positive
+ * identification (installed, or installed-but-not-executable) returns
+ * immediately. Any other failure reason (unsafe_path/symlink_escape/
+ * suspicious_size/not_file/…) is only REMEMBERED, not returned — a single stray
+ * artifact at one guessed location must not stop the remaining candidates (or
+ * the PATH fallback in the caller) from being tried.
+ */
+export const findKnownPathMatch = async <T extends KnownPathCheckResult>(
+  knownPaths: string[],
+  checkKnownPath: (candidatePath: string) => Promise<T>
+): Promise<{ match: T | null; bestFailure: T | null }> => {
+  let bestFailure: T | null = null;
+  for (const knownPath of knownPaths) {
+    const result = await checkKnownPath(knownPath);
+    if (result.installed) {
+      return { match: result, bestFailure: null };
+    }
+    if (result.reason && result.reason !== "not_found" && !bestFailure) {
+      bestFailure = result;
+    }
+  }
+  return { match: null, bestFailure };
+};

tests/unit/cli-runtime-extended.test.ts

@@ -330,11 +330,21 @@ test("getCliRuntimeStatus ignores suspicious known-path binaries and symlink esc
     fs.symlinkSync(outsideTarget, path.join(escapeBinDir, "qodercli"));
     process.env.npm_config_prefix = escapePrefix;
 
+    // #7753: every candidate checkKnownPath() ever receives is constructed by
+    // getKnownToolPaths() from a trusted root (here: npm_config_prefix/bin) — so a
+    // symlink placed exactly at that generated candidate path (the version-manager
+    // shim pattern used by nvm-windows/nvm/asdf/pyenv: trusted shim location,
+    // private per-version store target) is legitimate and must now be ACCEPTED even
+    // though its resolved target lives outside every EXPECTED_PARENT_PATHS entry.
+    // A genuinely unsafe symlink (one whose ORIGINAL location is also untrusted, not
+    // reachable through this known-path candidate generator) is covered separately
+    // by tests/unit/cliRuntime-symlink-escape-7753.test.ts via the exported
+    // checkKnownPath().
     const escapedRuntime = await importFresh("symlink-escape");
     const escapedStatus = await escapedRuntime.getCliRuntimeStatus("qoder");
 
-    assert.equal(escapedStatus.installed, false);
-    assert.equal(escapedStatus.reason, "symlink_escape");
+    assert.equal(escapedStatus.installed, true);
+    assert.equal(escapedStatus.reason, null);
   }
 });
 

tests/unit/cli-runtime-known-path-shortcircuit-7774.test.ts

@@ -0,0 +1,67 @@
+import { describe, it, before, after } from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+// HOME must be overridden BEFORE importing cliRuntime.ts — the module computes
+// EXPECTED_PARENT_PATHS (the known-path realpath containment check) once at
+// import time from os.homedir().
+const fakeHome = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-7774-home-"));
+const realBinDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-7774-realbin-"));
+
+const savedEnv: Record<string, string | undefined> = {
+  HOME: process.env.HOME,
+  USERPROFILE: process.env.USERPROFILE,
+  PATH: process.env.PATH,
+  CLI_CLAUDE_BIN: process.env.CLI_CLAUDE_BIN,
+  CLI_EXTRA_PATHS: process.env.CLI_EXTRA_PATHS,
+  npm_config_prefix: process.env.npm_config_prefix,
+};
+
+process.env.HOME = fakeHome;
+process.env.USERPROFILE = fakeHome;
+delete process.env.CLI_CLAUDE_BIN;
+delete process.env.CLI_EXTRA_PATHS;
+process.env.npm_config_prefix = path.join(fakeHome, "npm-prefix-unused");
+
+const { getCliRuntimeStatus, getKnownToolPaths } = await import(
+  "../../src/shared/services/cliRuntime.ts"
+);
+
+function makeExecutable(filePath: string, content: string) {
+  fs.writeFileSync(filePath, content);
+  if (process.platform !== "win32") fs.chmodSync(filePath, 0o755);
+}
+
+describe("#7774 — known-path short-circuit hides a genuinely runnable Claude binary", () => {
+  before(() => {
+    const poisonedCandidate = path.join(fakeHome, ".local", "bin", "claude");
+    fs.mkdirSync(poisonedCandidate, { recursive: true });
+
+    const known = getKnownToolPaths("claude");
+    assert.ok(known.includes(poisonedCandidate));
+
+    const realClaude = path.join(realBinDir, "claude");
+    makeExecutable(realClaude, "#!/bin/sh\necho '2.1.215 (Claude Code)'\n");
+    // Prepend realBinDir rather than replacing PATH outright — locateCommand()
+    // spawns `sh`/`where.exe` itself using this same PATH, so the standard
+    // system bin dirs (containing `sh`) must stay resolvable too.
+    process.env.PATH = [realBinDir, savedEnv.PATH].filter(Boolean).join(path.delimiter);
+  });
+
+  after(() => {
+    for (const [key, value] of Object.entries(savedEnv)) {
+      if (value === undefined) delete (process.env as Record<string, string | undefined>)[key];
+      else process.env[key] = value;
+    }
+    fs.rmSync(fakeHome, { recursive: true, force: true });
+    fs.rmSync(realBinDir, { recursive: true, force: true });
+  });
+
+  it("should still find and report Claude as installed+runnable via PATH fallback", async () => {
+    const result = await getCliRuntimeStatus("claude");
+    assert.equal(result.installed, true, `expected installed=true, got reason=${result.reason}`);
+    assert.equal(result.runnable, true, `expected runnable=true, got reason=${result.reason}`);
+  });
+});

tests/unit/cliRuntime-symlink-escape-7753.test.ts

@@ -0,0 +1,60 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import fsp from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
+
+// cliRuntime.ts computes EXPECTED_PARENT_PATHS from os.homedir() at MODULE LOAD
+// time, so HOME must be redirected before the module is imported.
+const sandboxHome = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-7753-home-"));
+const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-7753-outside-"));
+
+process.env.HOME = sandboxHome;
+process.env.USERPROFILE = sandboxHome;
+process.env.npm_config_prefix = path.join(sandboxHome, "npm-prefix-unused");
+delete process.env.CLI_OPENCODE_BIN;
+
+const localBinDir = path.join(sandboxHome, ".local", "bin");
+fs.mkdirSync(localBinDir, { recursive: true });
+
+const realBinaryPath = path.join(outsideDir, "opencode-real-binary");
+fs.writeFileSync(realBinaryPath, "#!/bin/sh\necho fake-opencode-binary-body-padding\n");
+fs.chmodSync(realBinaryPath, 0o755);
+
+// symlink lives INSIDE a trusted parent (HOME/.local/bin) — like nvm-windows's
+// opencode.cmd under AppData\Local\nvm\<ver>\ — but its resolved target is outside.
+const symlinkPath = path.join(localBinDir, "opencode");
+fs.symlinkSync(realBinaryPath, symlinkPath);
+
+const { getCliRuntimeStatus, checkKnownPath } = await import(
+  "../../src/shared/services/cliRuntime.ts"
+);
+
+test("#7753: a CLI symlink located inside an expected parent dir is wrongly reported not-installed when its resolved target escapes EXPECTED_PARENT_PATHS", async () => {
+  const status = await getCliRuntimeStatus("opencode");
+  assert.equal(
+    status.installed,
+    true,
+    `expected installed=true but got installed=${status.installed} reason=${status.reason}`
+  );
+});
+
+test("#7753: a genuinely unsafe symlink whose ORIGINAL location is also untrusted must still be rejected", async () => {
+  // Neither the symlink's own location nor its resolved target is inside any
+  // EXPECTED_PARENT_PATHS entry — this must stay rejected as symlink_escape.
+  const untrustedDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-7753-untrusted-"));
+  const untrustedSymlink = path.join(untrustedDir, "opencode");
+  fs.symlinkSync(realBinaryPath, untrustedSymlink);
+
+  const result = await checkKnownPath(untrustedSymlink);
+  assert.equal(result.installed, false);
+  assert.equal(result.reason, "symlink_escape");
+
+  await fsp.rm(untrustedDir, { recursive: true, force: true });
+});
+
+test.after(async () => {
+  await fsp.rm(sandboxHome, { recursive: true, force: true });
+  await fsp.rm(outsideDir, { recursive: true, force: true });
+});