Merge pull request #598 from razllivan/fix/cli-tools-detection

fix(cli): cross-platform CLI tool detection for custom npm prefixes

src/shared/services/cliRuntime.ts

@@ -1,7 +1,8 @@
 import fs from "fs/promises";
+import fsSync from "fs";
 import os from "os";
 import path from "path";
-import { spawn } from "child_process";
+import { spawn, execFileSync } from "child_process";
 
 const VALID_RUNTIME_MODES = new Set(["auto", "host", "container"]);
 const FALSE_VALUES = new Set(["0", "false", "no", "off"]);
@@ -258,6 +259,42 @@ const validateEnvPath = (value: string | undefined, allowedParents: string[]): s
   return normalized;
 };
 
+/**
+ * Detect the npm global bin directory.
+ * Cached on first call — `execFileSync` is expensive, only run once.
+ */
+let _npmGlobalPrefix: string | undefined;
+const getNpmGlobalPrefix = (): string => {
+  if (_npmGlobalPrefix !== undefined) return _npmGlobalPrefix;
+
+  const envPrefix = String(process.env.npm_config_prefix || "").trim();
+  if (envPrefix && path.isAbsolute(envPrefix)) {
+    _npmGlobalPrefix = envPrefix;
+    return _npmGlobalPrefix;
+  }
+
+  try {
+    const result = execFileSync("npm", ["config", "get", "prefix"], {
+      timeout: 5000,
+      encoding: "utf8",
+      stdio: ["ignore", "pipe", "ignore"],
+      ...(isWindows() ? { shell: true } : {}),
+    });
+    const prefix = result.trim();
+    if (
+      prefix &&
+      path.isAbsolute(prefix) &&
+      !DANGEROUS_PATH_CHARS.some((c) => prefix.includes(c))
+    ) {
+      _npmGlobalPrefix = prefix;
+      return _npmGlobalPrefix;
+    }
+  } catch {}
+
+  _npmGlobalPrefix = "";
+  return _npmGlobalPrefix;
+};
+
 /**
  * Pre-compute expected parent directories at module startup for performance.
  * These are the allowed directories for CLI binary installation locations.
@@ -281,6 +318,8 @@ const getExpectedParentPaths = (): string[] => {
     "C:\\Program Files (x86)",
   ]);
 
+  const npmPrefix = getNpmGlobalPrefix();
+
   return [
     home,
     userProfile,
@@ -288,6 +327,7 @@ const getExpectedParentPaths = (): string[] => {
     validatedLocalAppData,
     validatedProgramFiles,
     validatedProgramFilesX86,
+    npmPrefix,
   ].filter(Boolean);
 };
 
@@ -310,86 +350,94 @@ const getExtraPaths = () =>
     });
 
 /**
- * Get known installation paths for a specific CLI tool on Windows.
- * Returns ONLY verified, tool-specific paths - NOT generic user bin directories.
- * This is more secure than searching PATH as it checks known locations only.
+ * Get known installation paths for a specific CLI tool.
+ * Checks npm global prefix, NVM locations, standalone installer paths.
+ * Works on all platforms — Windows checks .cmd wrappers, Linux/macOS checks bare names.
  */
 const getKnownToolPaths = (toolId: string): string[] => {
-  if (!isWindows()) return [];
-
   const home = os.homedir();
-  const userProfile = process.env.USERPROFILE || home;
+  const paths: string[] = [];
 
-  // Validate environment paths against allowed parent directories
-  const appData = validateEnvPath(process.env.APPDATA, [home, userProfile]);
-  const localAppData = validateEnvPath(process.env.LOCALAPPDATA, [
-    path.join(home, "AppData", "Local"),
-    path.join(userProfile, "AppData", "Local"),
-    userProfile,
-  ]);
-
-  // Cache nvm node path to avoid duplicate detection calls
+  const npmPrefix = getNpmGlobalPrefix();
   const nvmNodePath = getNvmNodePath();
 
-  // Tool-specific known installation paths (verified locations only)
-  const knownPaths: Record<string, string[]> = {
+  const toolBins: Record<string, [string, string][]> = {
     claude: [
-      // Official Claude Code standalone installer locations
-      path.join(home, ".local", "bin", "claude.exe"),
-      ...(localAppData ? [path.join(localAppData, "Programs", "Claude", "claude.exe")] : []),
-      ...(localAppData ? [path.join(localAppData, "claude-code", "claude.exe")] : []),
-      // npm global (only if nvm-windows is detected)
-      ...(nvmNodePath ? [path.join(nvmNodePath, "claude-code.cmd")] : []),
-    ],
-    codex: [
-      path.join(home, ".local", "bin", "codex"),
-      // npm global (only if nvm-windows is detected)
-      ...(nvmNodePath ? [path.join(nvmNodePath, "codex.cmd")] : []),
-      ...(appData ? [path.join(appData, "npm", "codex.cmd")] : []),
-    ],
-    droid: [
-      path.join(home, ".local", "bin", "droid"),
-      // npm global (only if nvm-windows is detected)
-      ...(nvmNodePath ? [path.join(nvmNodePath, "droid.cmd")] : []),
-      ...(appData ? [path.join(appData, "npm", "droid.cmd")] : []),
-    ],
-    openclaw: [
-      path.join(home, ".local", "bin", "openclaw"),
-      // npm global (only if nvm-windows is detected)
-      ...(nvmNodePath ? [path.join(nvmNodePath, "openclaw.cmd")] : []),
-      ...(appData ? [path.join(appData, "npm", "openclaw.cmd")] : []),
+      ["claude.cmd", "claude"],
+      ["claude.exe", "claude"],
     ],
+    codex: [["codex.cmd", "codex"]],
+    droid: [["droid.cmd", "droid"]],
+    openclaw: [["openclaw.cmd", "openclaw"]],
     cursor: [
-      path.join(home, ".local", "bin", "agent"),
-      path.join(home, ".local", "bin", "cursor"),
-      // npm global (only if nvm-windows is detected)
-      ...(nvmNodePath ? [path.join(nvmNodePath, "agent.cmd")] : []),
-      ...(nvmNodePath ? [path.join(nvmNodePath, "cursor.cmd")] : []),
-      ...(appData ? [path.join(appData, "npm", "agent.cmd")] : []),
-      ...(appData ? [path.join(appData, "npm", "cursor.cmd")] : []),
+      ["agent.cmd", "agent"],
+      ["cursor.cmd", "cursor"],
     ],
-    cline: [
-      path.join(home, ".local", "bin", "cline"),
-      // npm global (only if nvm-windows is detected)
-      ...(nvmNodePath ? [path.join(nvmNodePath, "cline.cmd")] : []),
-      ...(appData ? [path.join(appData, "npm", "cline.cmd")] : []),
-    ],
-    kilo: [
-      path.join(home, ".local", "bin", "kilocode"),
-      // npm global (only if nvm-windows is detected)
-      ...(nvmNodePath ? [path.join(nvmNodePath, "kilocode.cmd")] : []),
-      ...(appData ? [path.join(appData, "npm", "kilocode.cmd")] : []),
-    ],
-    opencode: [
-      path.join(home, ".local", "bin", "opencode"),
-      // npm global (only if nvm-windows is detected)
-      ...(nvmNodePath ? [path.join(nvmNodePath, "opencode.cmd")] : []),
-      ...(appData ? [path.join(appData, "npm", "opencode.cmd")] : []),
-    ],
-    // Add other tools as needed with their specific known paths
+    cline: [["cline.cmd", "cline"]],
+    kilo: [["kilocode.cmd", "kilocode"]],
+    opencode: [["opencode.cmd", "opencode"]],
   };
 
-  return knownPaths[toolId] || [];
+  const bins = toolBins[toolId] || [];
+
+  if (isWindows()) {
+    const userProfile = process.env.USERPROFILE || home;
+    const appData = validateEnvPath(process.env.APPDATA, [home, userProfile]);
+    const localAppData = validateEnvPath(process.env.LOCALAPPDATA, [
+      path.join(home, "AppData", "Local"),
+      path.join(userProfile, "AppData", "Local"),
+      userProfile,
+    ]);
+
+    if (toolId === "claude") {
+      paths.push(path.join(home, ".local", "bin", "claude.exe"));
+      if (localAppData) {
+        paths.push(path.join(localAppData, "Programs", "Claude", "claude.exe"));
+        paths.push(path.join(localAppData, "claude-code", "claude.exe"));
+      }
+    }
+
+    for (const [winName] of bins) {
+      if (npmPrefix) paths.push(path.join(npmPrefix, winName));
+      if (appData) {
+        const appDataPath = path.join(appData, "npm", winName);
+        if (
+          !npmPrefix ||
+          path.normalize(appDataPath) !== path.normalize(path.join(npmPrefix, winName))
+        ) {
+          paths.push(appDataPath);
+        }
+      }
+      if (nvmNodePath) paths.push(path.join(nvmNodePath, winName));
+    }
+  } else {
+    for (const [, posixName] of bins) {
+      const nodeBinDir = path.dirname(process.execPath);
+      paths.push(path.join(nodeBinDir, posixName));
+
+      if (npmPrefix) {
+        paths.push(path.join(npmPrefix, "bin", posixName));
+      }
+
+      paths.push(path.join(home, ".local", "bin", posixName));
+      // Only add system paths if they exist (avoids unnecessary stat calls)
+      if (fsSync.existsSync("/usr/local/bin")) {
+        paths.push(path.join("/usr", "local", "bin", posixName));
+      }
+      if (fsSync.existsSync("/usr/bin")) {
+        paths.push(path.join("/usr", "bin", posixName));
+      }
+
+      if (toolId === "opencode") {
+        paths.push(path.join(home, ".opencode", "bin", posixName));
+      }
+      if (toolId === "claude") {
+        paths.push(path.join(home, ".claude", "bin", posixName));
+      }
+    }
+  }
+
+  return paths;
 };
 
 /**
@@ -492,7 +540,7 @@ const locateCommand = async (command: string, env: Record<string, string | undef
  * Security hardening:
  * - Resolves symlinks and verifies target stays within expected directories
  * - Verifies file is a regular file (not directory, pipe, or device)
- * - Checks file size bounds (1KB - 100MB) to detect suspicious binaries
+ * - Checks file size bounds (30B - 100MB) to detect suspicious binaries
  */
 const checkKnownPath = async (commandPath: string) => {
   if (!path.isAbsolute(commandPath)) {
@@ -521,9 +569,10 @@ const checkKnownPath = async (commandPath: string) => {
       return { installed: false, commandPath: null, reason: "not_file" };
     }
 
-    // CLI binaries should be > 1KB and < 100MB
-    // This catches suspicious files while allowing for wrapper scripts
-    if (stat.size < 1024 || stat.size > 100 * 1024 * 1024) {
+    // CLI binaries should be > 30 bytes and < 100MB
+    // npm .cmd wrappers on Windows are ~300-500 bytes, JS wrappers on Linux can be ~44 bytes
+    // Minimum catches empty/suspicious files while allowing legitimate thin wrappers
+    if (stat.size < 30 || stat.size > 100 * 1024 * 1024) {
       return { installed: false, commandPath: null, reason: "suspicious_size" };
     }
   } catch (error) {
@@ -556,7 +605,7 @@ const locateCommandCandidate = async (
 
   // SECURITY: First check known installation paths for this specific tool
   // This avoids searching PATH and reduces attack surface
-  if (toolId && isWindows()) {
+  if (toolId) {
     const knownPaths = getKnownToolPaths(toolId);
     for (const knownPath of knownPaths) {
       const result = await checkKnownPath(knownPath);
@@ -592,6 +641,7 @@ const checkRunnable = async (
     PATH: env.PATH,
     HOME: env.HOME || env.USERPROFILE,
     SystemRoot: env.SystemRoot, // Windows needs this
+    PATHEXT: env.PATHEXT, // Windows cmd.exe needs this to resolve .cmd/.bat/.exe extensions
   };
 
   for (const args of [["--version"], ["-v"]]) {

tests/unit/cli-runtime-detection.test.mjs

@@ -0,0 +1,202 @@
+/**
+ * Tests for CLI tool detection: cross-platform known paths, size threshold,
+ * npm prefix deduplication, and env var overrides.
+ */
+
+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";
+
+const { getCliRuntimeStatus, CLI_TOOL_IDS } =
+  await import("../../src/shared/services/cliRuntime.ts");
+
+// ─── Helpers ──────────────────────────────────────────────────
+
+function createTempDir() {
+  return fs.mkdtempSync(path.join(os.tmpdir(), "cli-test-"));
+}
+
+function createFile(dir, name, content) {
+  const filePath = path.join(dir, name);
+  fs.writeFileSync(filePath, content);
+  if (process.platform !== "win32") {
+    fs.chmodSync(filePath, 0o755);
+  }
+  return filePath;
+}
+
+// ─── CLI_TOOL_IDS ─────────────────────────────────────────────
+
+describe("CLI_TOOL_IDS", () => {
+  it("should include all expected tools", () => {
+    const expected = [
+      "claude",
+      "codex",
+      "droid",
+      "openclaw",
+      "cursor",
+      "cline",
+      "kilo",
+      "continue",
+      "opencode",
+    ];
+    for (const id of expected) {
+      assert.ok(CLI_TOOL_IDS.includes(id), `Missing tool: ${id}`);
+    }
+  });
+});
+
+// ─── Size Threshold (30 bytes) ────────────────────────────────
+
+describe("Size threshold — checkKnownPath", () => {
+  let tmpDir;
+
+  before(() => {
+    tmpDir = createTempDir();
+  });
+
+  after(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  it("should detect files >= 30 bytes via env var", async () => {
+    const prev = process.env.CLI_DROID_BIN;
+    // Create a valid 30-byte+ script
+    const content =
+      process.platform === "win32"
+        ? "@echo off\r\necho 1.0.0\r\nexit 0\r\n"
+        : "#!/bin/sh\r\necho 1.0.0\r\nexit 0\r\n";
+    const script = createFile(tmpDir, "droid-valid", content);
+    // Verify it's at least 30 bytes
+    const stat = fs.statSync(script);
+    assert.ok(stat.size >= 30, `File should be >= 30 bytes, got ${stat.size}`);
+
+    process.env.CLI_DROID_BIN = script;
+    try {
+      const result = await getCliRuntimeStatus("droid");
+      assert.ok(result.installed, `Expected installed=true, got reason=${result.reason}`);
+      assert.ok(result.commandPath === script, `Expected commandPath=${script}`);
+    } finally {
+      if (prev !== undefined) process.env.CLI_DROID_BIN = prev;
+      else delete process.env.CLI_DROID_BIN;
+    }
+  });
+
+  it("should detect a valid CLI script (>= 30 bytes) via env var", async () => {
+    const prev = process.env.CLI_DROID_BIN;
+    const script =
+      process.platform === "win32"
+        ? createFile(tmpDir, "droid.cmd", "@echo off\necho 1.0.0\n")
+        : createFile(tmpDir, "droid", "#!/bin/sh\necho 1.0.0\n");
+
+    process.env.CLI_DROID_BIN = script;
+    try {
+      const result = await getCliRuntimeStatus("droid");
+      assert.ok(result.installed, `Expected installed=true, got reason=${result.reason}`);
+      assert.ok(
+        result.commandPath === script,
+        `Expected commandPath=${script}, got ${result.commandPath}`
+      );
+    } finally {
+      if (prev !== undefined) process.env.CLI_DROID_BIN = prev;
+      else delete process.env.CLI_DROID_BIN;
+    }
+  });
+});
+
+// ─── Healthcheck with --version ───────────────────────────────
+
+describe("Healthcheck — checkRunnable", () => {
+  let tmpDir;
+
+  before(() => {
+    tmpDir = createTempDir();
+  });
+
+  after(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  it("should report runnable=true for a script that outputs version", async () => {
+    const prev = process.env.CLI_CLINE_BIN;
+    const script =
+      process.platform === "win32"
+        ? createFile(tmpDir, "good.cmd", "@echo off\necho 1.0.0\n")
+        : createFile(tmpDir, "good", "#!/bin/sh\necho 1.0.0\n");
+
+    process.env.CLI_CLINE_BIN = script;
+    try {
+      const result = await getCliRuntimeStatus("cline");
+      assert.ok(result.installed, `Expected installed=true, got reason=${result.reason}`);
+      if (result.runnable) {
+        assert.ok(result.reason === null, `Expected no reason, got ${result.reason}`);
+      }
+    } finally {
+      if (prev !== undefined) process.env.CLI_CLINE_BIN = prev;
+      else delete process.env.CLI_CLINE_BIN;
+    }
+  });
+});
+
+// ─── Unknown tool ─────────────────────────────────────────────
+
+describe("Unknown tool", () => {
+  it("should return unknown_tool for non-existent tool", async () => {
+    const result = await getCliRuntimeStatus("nonexistent-tool-xyz");
+    assert.equal(result.installed, false);
+    assert.equal(result.reason, "unknown_tool");
+  });
+});
+
+// ─── continue tool (requiresBinary: false) ────────────────────
+
+describe("continue tool — no binary required", () => {
+  it("should report installed=true without checking binary", async () => {
+    const result = await getCliRuntimeStatus("continue");
+    assert.equal(result.installed, true);
+    assert.equal(result.reason, "not_required");
+  });
+});
+
+// ─── resolveOpencodeConfigPath — cross-platform ─────────────────
+
+const { resolveOpencodeConfigPath: resolveOpencodeConfigPathFn } =
+  await import("../../src/shared/services/cliRuntime.ts");
+
+describe("resolveOpencodeConfigPath — cross-platform", () => {
+  it("should resolve on Linux with XDG_CONFIG_HOME", () => {
+    const result = resolveOpencodeConfigPathFn(
+      "linux",
+      { XDG_CONFIG_HOME: "/tmp/xdg" },
+      "/home/dev"
+    );
+    assert.equal(result, path.join("/tmp/xdg", "opencode", "opencode.json"));
+  });
+
+  it("should resolve on Linux with default .config", () => {
+    const result = resolveOpencodeConfigPathFn("linux", {}, "/home/dev");
+    assert.equal(result, path.join("/home/dev", ".config", "opencode", "opencode.json"));
+  });
+
+  it("should resolve on Windows with APPDATA", () => {
+    const result = resolveOpencodeConfigPathFn(
+      "win32",
+      { APPDATA: "C:\\Users\\dev\\AppData\\Roaming" },
+      "C:\\Users\\dev"
+    );
+    assert.equal(
+      result,
+      path.join("C:\\Users\\dev\\AppData\\Roaming", "opencode", "opencode.json")
+    );
+  });
+
+  it("should fallback to home/AppData/Roaming on Windows without APPDATA", () => {
+    const result = resolveOpencodeConfigPathFn("win32", {}, "C:\\Users\\dev");
+    assert.equal(
+      result,
+      path.join("C:\\Users\\dev", "AppData", "Roaming", "opencode", "opencode.json")
+    );
+  });
+});