Merge remote-tracking branch 'origin/release/v3.8.50' into fix/release-v3.8.50-basereds-cluster

tests/unit/cli/_helpers/shellArgs.mjs

@@ -0,0 +1,41 @@
+/**
+ * Reverse the Windows `shell: true` argument escaping so assertions can be
+ * written against the logical argv on every platform.
+ *
+ * `bin/cli/commands/run.mjs` escapes argv before spawning, because on win32 the
+ * launchers must go through cmd.exe to run npm `.cmd` shims (CVE-2024-27980)
+ * and Node's `shell: true` joins argv with no escaping at all (DEP0190). That
+ * escaping is correct and deliberate, but it means `plan.args` holds
+ * `^^^"--model^^^"` on Windows where it holds `--model` elsewhere.
+ *
+ * Tests care about *which* arguments a plan carries, not about how they survive
+ * cmd.exe, so they normalise first. Keep this in sync with
+ * `escapeWindowsShellArg` in bin/cli/utils/winShellArgs.mjs.
+ */
+
+/**
+ * @param {unknown} arg
+ * @returns {string}
+ */
+export function unescapeWindowsShellArg(arg) {
+  let s = String(arg);
+  // 1. undo the two caret passes applied to cmd.exe metacharacters
+  s = s.replace(/\^(.)/g, "$1").replace(/\^(.)/g, "$1");
+  // 2. drop the wrapping quotes added by the CRT argv layer
+  if (s.length >= 2 && s.startsWith('"') && s.endsWith('"')) s = s.slice(1, -1);
+  // 3. undo the doubled backslashes and the escaped embedded quotes
+  s = s.replace(/\\\\/g, "\\").replace(/\\"/g, '"');
+  return s;
+}
+
+/**
+ * Normalise a plan's argv to its logical form. A no-op off Windows.
+ *
+ * @param {unknown[]} args
+ * @param {NodeJS.Platform|string} [platform]
+ * @returns {string[]}
+ */
+export function logicalArgs(args, platform = process.platform) {
+  const list = [...(args ?? [])].map(String);
+  return platform === "win32" ? list.map(unescapeWindowsShellArg) : list;
+}

tests/unit/cli/alias-resolver-7791.test.ts

@@ -18,7 +18,7 @@ import { spawnSync } from "node:child_process";
 import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { fileURLToPath } from "node:url";
+import { fileURLToPath, pathToFileURL } from "node:url";
 
 import {
   resolveAlias,
@@ -30,6 +30,13 @@ import {
 const __dirname = fileURLToPath(new URL(".", import.meta.url));
 const REPO_ROOT = join(__dirname, "..", "..", "..");
 
+// The child scripts below `import()` these paths, and `import()` resolves its
+// specifier as a URL. Replacing backslashes with forward slashes is not enough
+// on Windows: the leading drive letter is then parsed as the URL scheme `e:`,
+// which the ESM loader rejects with ERR_UNSUPPORTED_ESM_URL_SCHEME. Emit a
+// real file:// URL instead.
+const repoFileUrl = (relPath) => pathToFileURL(join(REPO_ROOT, relPath)).href;
+
 describe("aliasResolver.resolveAlias (pure)", () => {
   it("returns null for non-@/ specifiers (lets Node/tsx handle them)", () => {
     assert.equal(resolveAlias("node:fs", REPO_ROOT), null);
@@ -260,11 +267,11 @@ describe("aliasResolver end-to-end (#7791 regression)", () => {
     const script = `
       await import("tsx/esm");
       import { join } from "node:path";
-      import { registerAliasResolver } from "${join(REPO_ROOT, "bin/aliasResolver.mjs").replace(/\\/g, "/")}";
+      import { registerAliasResolver } from ${JSON.stringify(repoFileUrl("bin/aliasResolver.mjs"))};
       const ok = await registerAliasResolver(${JSON.stringify(REPO_ROOT)});
       if (!ok) { console.error("FAIL: registerAliasResolver returned false"); process.exit(2); }
       try {
-        const m = await import(${JSON.stringify(join(REPO_ROOT, "src/shared/network/outboundUrlGuard.ts").replace(/\\/g, "/"))});
+        const m = await import(${JSON.stringify(repoFileUrl("src/shared/network/outboundUrlGuard.ts"))});
         const keys = Object.keys(m).sort().join(",");
         console.log("OK:" + keys);
       } catch (err) {
@@ -286,7 +293,7 @@ describe("aliasResolver end-to-end (#7791 regression)", () => {
 
   it("does not interfere with bare/relative specifiers (regression guard)", () => {
     const script = `
-      import { registerAliasResolver } from "${join(REPO_ROOT, "bin/aliasResolver.mjs").replace(/\\/g, "/")}";
+      import { registerAliasResolver } from ${JSON.stringify(repoFileUrl("bin/aliasResolver.mjs"))};
       await registerAliasResolver(${JSON.stringify(REPO_ROOT)});
       // node:fs must still resolve via the default resolver
       const fs = await import("node:fs");

tests/unit/cli/run-command.test.ts

@@ -7,6 +7,7 @@ import {
   resolveModelFromTargetOptions,
   runCliTarget,
 } from "../../../bin/cli/commands/run.mjs";
+import { logicalArgs } from "./_helpers/shellArgs.mjs";
 
 test("resolveRunTarget resolves aliases", () => {
   assert.equal(resolveRunTarget("claude"), "claude");
@@ -39,7 +40,7 @@ test("buildRunPlan for claude includes env diff and model injection", async () =
   assert.equal(plan.target, "claude");
   assert.equal(plan.baseUrl, "http://localhost:20128");
   assert.equal(plan.model, "gpt-5");
-  assert.equal(plan.args.includes("--help"), true);
+  assert.equal(logicalArgs(plan.args).includes("--help"), true);
   assert.equal(plan.envDiff.changedOrAdded.includes("ANTHROPIC_AUTH_TOKEN"), true);
   assert.equal(plan.authSource, "option");
   assert.equal(plan.command.includes("claude"), true);
@@ -54,9 +55,9 @@ test("buildRunPlan for codex injects model into provider args", async () => {
   assert.equal(plan.target, "codex");
   assert.equal(plan.baseUrl, "http://localhost:20128");
   assert.equal(plan.model, "glm/glm-4.5");
-  assert.equal(plan.args.includes("--help"), true);
+  assert.equal(logicalArgs(plan.args).includes("--help"), true);
   assert.equal(
-    plan.args.some((a) => String(a).includes("model_providers.omniroute.model")),
+    logicalArgs(plan.args).some((a) => a.includes("model_providers.omniroute.model")),
     true
   );
   assert.equal(plan.authSource, "option");
@@ -70,7 +71,7 @@ test("buildRunPlan for Aider uses its OpenAI-compatible root endpoint", async ()
   );
   assert.equal(plan.target, "aider");
   assert.equal(plan.baseUrl, "https://relay.example.test");
-  assert.deepEqual(plan.args.slice(0, 2), ["--model", "openai/glm/glm-5.2"]);
+  assert.deepEqual(logicalArgs(plan.args).slice(0, 2), ["--model", "openai/glm/glm-5.2"]);
   assert.equal(plan.envDiff.changedOrAdded.includes("OPENAI_API_BASE"), true);
   assert.equal(plan.envDiff.changedOrAdded.includes("OPENAI_API_KEY"), true);
 });
@@ -82,7 +83,7 @@ test("buildRunPlan for Goose injects provider and model without writing config",
     ["session"]
   );
   assert.equal(plan.target, "goose");
-  assert.deepEqual(plan.args, ["session"]);
+  assert.deepEqual(logicalArgs(plan.args), ["session"]);
   assert.equal(plan.envDiff.changedOrAdded.includes("GOOSE_PROVIDER"), true);
   assert.equal(plan.envDiff.changedOrAdded.includes("GOOSE_MODEL"), true);
   assert.equal(plan.envDiff.changedOrAdded.includes("OPENAI_HOST"), true);
@@ -95,7 +96,7 @@ test("buildRunPlan for OpenCode uses an ephemeral compatible config", async () =
     ["run", "reply OK"]
   );
   assert.equal(plan.target, "opencode");
-  assert.deepEqual(plan.args.slice(0, 2), ["--model", "omniroute/glm/glm-5.2"]);
+  assert.deepEqual(logicalArgs(plan.args).slice(0, 2), ["--model", "omniroute/glm/glm-5.2"]);
   assert.equal(plan.envDiff.changedOrAdded.includes("OPENCODE_CONFIG_CONTENT"), true);
   assert.equal(plan.envDiff.changedOrAdded.includes("OMNIROUTE_API_KEY"), true);
   assert.equal(plan.configOverlay, "OPENCODE_CONFIG_CONTENT (process environment only)");
@@ -109,7 +110,7 @@ test("buildRunPlan for Qwen requires a deterministic model and injects only env
     ["-p", "reply OK"]
   );
   assert.equal(plan.target, "qwen");
-  assert.deepEqual(plan.args.slice(0, 2), ["--model", "glm/glm-5.2"]);
+  assert.deepEqual(logicalArgs(plan.args).slice(0, 2), ["--model", "glm/glm-5.2"]);
   assert.equal(plan.envDiff.changedOrAdded.includes("OMNIROUTE_API_KEY"), true);
   assert.equal(plan.configOverlay, "temporary QWEN_HOME (removed after exit)");
   await assert.rejects(
@@ -126,7 +127,7 @@ test("buildRunPlan for Gemini points the CLI at the /v1beta surface via env", as
   );
   assert.equal(plan.target, "gemini");
   assert.equal(plan.baseUrl, "https://relay.example.test");
-  assert.deepEqual(plan.args.slice(0, 2), ["--model", "glm/glm-5.2"]);
+  assert.deepEqual(logicalArgs(plan.args).slice(0, 2), ["--model", "glm/glm-5.2"]);
   assert.equal(plan.envDiff.changedOrAdded.includes("GOOGLE_GEMINI_BASE_URL"), true);
   assert.equal(plan.envDiff.changedOrAdded.includes("GEMINI_API_KEY"), true);
   assert.equal(plan.envDiff.changedOrAdded.includes("GEMINI_DEFAULT_AUTH_TYPE"), true);

tests/unit/cli/windows-esm-import-paths.test.ts

@@ -0,0 +1,68 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import path from "node:path";
+import { fileURLToPath, pathToFileURL } from "node:url";
+
+const PROJECT_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../..");
+
+// Regression guard for the Windows-only ESM loader failure:
+//
+//   Error: Only URLs with a scheme in: file, data, and node are supported by
+//   the default ESM loader. On Windows, absolute paths must be valid file://
+//   URLs. Received protocol 'e:'
+//
+// `import()` resolves its specifier as a URL. A POSIX absolute path like
+// /home/x/src/lib/db/combos.ts happens to also be a valid relative URL, so
+// interpolating it works by accident. A Windows absolute path is
+// E:\checkout\src\lib\db\combos.ts, whose leading drive letter the loader
+// parses as the URL scheme `e:` and rejects. Every such call site must go
+// through pathToFileURL().
+//
+// This broke `omniroute combo list/create/delete/switch` on Windows whenever
+// the CLI fell back to direct DB access with the server offline.
+
+const CLI_DIR = path.join(PROJECT_ROOT, "bin", "cli");
+
+function collectMjsFiles(dir: string): string[] {
+  const out: string[] = [];
+  for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) out.push(...collectMjsFiles(full));
+    else if (entry.name.endsWith(".mjs")) out.push(full);
+  }
+  return out;
+}
+
+test("bin/cli never passes an interpolated absolute path to dynamic import()", () => {
+  // Matches import(`${ANY_ROOT_CONST}/...`) — a raw filesystem path, not a URL.
+  const badImport = /\bimport\(\s*`\$\{[A-Za-z_$][\w$]*\}\//;
+
+  const offenders: string[] = [];
+  for (const file of collectMjsFiles(CLI_DIR)) {
+    const source = fs.readFileSync(file, "utf8");
+    source.split(/\r?\n/).forEach((line, i) => {
+      if (badImport.test(line)) {
+        offenders.push(`${path.relative(PROJECT_ROOT, file)}:${i + 1}: ${line.trim()}`);
+      }
+    });
+  }
+
+  assert.deepEqual(
+    offenders,
+    [],
+    "dynamic import() of an interpolated absolute path fails on Windows; " +
+      `wrap the path in pathToFileURL(...).href instead:\n${offenders.join("\n")}`,
+  );
+});
+
+test("runtime.mjs resolves db modules to a file:// URL", async () => {
+  const source = fs.readFileSync(path.join(CLI_DIR, "runtime.mjs"), "utf8");
+  assert.match(source, /pathToFileURL/, "runtime.mjs must build file:// URLs for dynamic imports");
+
+  // The real proof: the db fallback modules actually load on this platform.
+  const runtime = await import(pathToFileURL(path.join(CLI_DIR, "runtime.mjs")).href);
+  const ctx = await runtime.withDb(async (c: { kind: string; db: unknown }) => c);
+  assert.equal(ctx.kind, "db");
+  assert.ok(ctx.db);
+});