fix(cli): stop cmd.exe mangling the OAuth URL on Windows (auth login silently fails)

.changeset/lucky-poems-argue.md

@@ -0,0 +1,7 @@
+---
+"@call-e/cli": patch
+---
+
+Open the browser without cmd.exe on Windows during `auth login`.
+
+`cmd /c start "" <url>` let cmd parse the OAuth URL, and `&` is a command separator there, so the URL was truncated at the first `&` (losing `redirect_uri`, `state` and `scope`) and the rest of the query string was run as shell commands. The opener is now `rundll32`, which takes the URL as a single argument, named by its fully qualified `%SystemRoot%\System32` path so the executable is not resolved through the current working directory.

packages/cli/lib/cli.js

@@ -914,14 +914,41 @@ async function handleCallCommand({ command, positional, options, config, deps, s
   }
 }
 
+// Picks the program that opens a URL in the user's browser.
+//
+// Windows is handled without cmd.exe on purpose. `cmd /c start "" <url>` lets cmd
+// parse the URL, and `&` is a command separator there -- an OAuth URL is truncated
+// at the first `&` (losing redirect_uri, state and scope) and the remaining query
+// fragments are run as shell commands. rundll32 takes the URL as a single argument
+// and never parses it.
+//
+// The Windows opener is named by its fully qualified path. An unqualified
+// "rundll32" would be looked up with the CreateProcess search order, which checks
+// the current directory before System32, so running `calle auth login` from an
+// untrusted checkout holding a planted rundll32.exe would execute it.
+// https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa
+//
+// Exported so the tests can assert the exact executable and argv the CLI uses.
+export function browserOpenCommand(url, platform = process.platform, env = process.env) {
+  if (platform === "darwin") return ["open", [url]];
+  if (platform !== "win32") return ["xdg-open", [url]];
+
+  const systemRoot = env.SystemRoot || env.SYSTEMROOT || env.windir || "C:\\Windows";
+  const rundll32 = `${systemRoot.replace(/[\\/]+$/, "")}\\System32\\rundll32.exe`;
+  return [rundll32, ["url.dll,FileProtocolHandler", url]];
+}
+
 export async function runCli(argv, deps = {}) {
   const stdout = deps.stdout || ((text) => process.stdout.write(text));
   const stderr = deps.stderr || ((text) => process.stderr.write(`${text}\n`));
   const openBrowser = deps.openBrowser || (async (url) => {
     const { spawn } = await import("node:child_process");
-    const command = process.platform === "darwin" ? "open" : process.platform === "win32" ? "cmd" : "xdg-open";
-    const args = process.platform === "win32" ? ["/c", "start", "", url] : [url];
+    const [command, args] = browserOpenCommand(url, process.platform, process.env);
     const child = spawn(command, args, { detached: true, stdio: "ignore" });
+    // Without a listener an 'error' event (a missing opener, say) becomes an
+    // uncaught exception. Failing to open a browser should not take the CLI
+    // down -- the URL is printed for the user either way.
+    child.on("error", () => {});
     child.unref();
   });
 

packages/cli/test/open-browser.test.js

@@ -0,0 +1,87 @@
+import assert from "node:assert/strict";
+import { spawnSync } from "node:child_process";
+import test from "node:test";
+
+import { browserOpenCommand } from "../lib/cli.js";
+
+// Regression: `auth login` opened the browser through cmd.exe on Windows.
+//
+//   spawn("cmd", ["/c", "start", "", url], { detached: true, stdio: "ignore" })
+//
+// cmd treats `&` as a command separator, and an OAuth URL is full of them.
+// Measured with the real URL shape:
+//
+//   in   ...authorize?client_id=calle&redirect_uri=http://localhost:9999/cb&state=abc&scope=mcp
+//   out  ...authorize?client_id=calle
+//   err  'redirect_uri' is not recognized as an internal or external command
+//
+// So the browser got a truncated URL -- no redirect_uri, no state, no scope, so
+// login could not complete -- and the rest of the query string was executed as
+// shell commands. `stdio: "ignore"` meant the user saw none of it.
+//
+// The assertions below run against browserOpenCommand, the function the CLI
+// actually calls, so putting cmd.exe back makes them fail on every platform.
+
+const OAUTH_URL =
+  "https://example.invalid/oauth/authorize" +
+  "?client_id=calle&redirect_uri=http://localhost:9999/cb&state=abc123&scope=mcp";
+
+const onWindows = process.platform === "win32";
+
+test("windows opens the URL with System32's rundll32, by absolute path", () => {
+  const [command, args] = browserOpenCommand(OAUTH_URL, "win32", { SystemRoot: "C:\\Windows" });
+
+  assert.equal(command, "C:\\Windows\\System32\\rundll32.exe");
+  assert.deepEqual(args, ["url.dll,FileProtocolHandler", OAUTH_URL]);
+});
+
+test("windows never routes the URL through a shell", () => {
+  const [command, args] = browserOpenCommand(OAUTH_URL, "win32", { SystemRoot: "C:\\Windows" });
+
+  assert.doesNotMatch(command, /(^|[\\/])cmd(\.exe)?$/i);
+  assert.ok(!args.includes("/c"), "no cmd switch in the argv");
+  assert.ok(!args.includes("start"), "no start builtin in the argv");
+});
+
+test("windows names the opener by a qualified path, not a bare executable", () => {
+  const [command] = browserOpenCommand(OAUTH_URL, "win32", { SystemRoot: "C:\\Windows" });
+
+  // A bare "rundll32" would be resolved with the CreateProcess search order,
+  // which looks in the current directory before System32.
+  assert.ok(/^[A-Za-z]:\\/.test(command), `expected an absolute path, got ${command}`);
+  assert.match(command, /\\System32\\rundll32\.exe$/i);
+});
+
+test("windows takes the system directory from the environment", () => {
+  const [command] = browserOpenCommand(OAUTH_URL, "win32", { SystemRoot: "D:\\WINNT" });
+
+  assert.equal(command, "D:\\WINNT\\System32\\rundll32.exe");
+});
+
+test("the URL travels as one argv entry, so nothing can split it", () => {
+  const [, args] = browserOpenCommand(OAUTH_URL, "win32", { SystemRoot: "C:\\Windows" });
+  const urlArgs = args.filter((arg) => arg.includes("example.invalid"));
+
+  assert.equal(urlArgs.length, 1);
+  assert.equal(urlArgs[0], OAUTH_URL);
+  assert.ok(urlArgs[0].includes("&scope=mcp"), "the query survives whole");
+});
+
+test("macos and linux keep their openers", () => {
+  assert.deepEqual(browserOpenCommand(OAUTH_URL, "darwin", {}), ["open", [OAUTH_URL]]);
+  assert.deepEqual(browserOpenCommand(OAUTH_URL, "linux", {}), ["xdg-open", [OAUTH_URL]]);
+});
+
+test("cmd.exe mangles a URL containing &", { skip: !onWindows }, () => {
+  const result = spawnSync("cmd", ["/c", "echo", OAUTH_URL], { encoding: "utf8" });
+  const received = (result.stdout || "").trim();
+
+  // This is the bug, asserted so its absence would be noticed: cmd stops at the
+  // first `&` and treats what follows as separate commands.
+  assert.ok(
+    !received.includes("scope=mcp"),
+    "expected cmd to truncate the URL -- if this now passes the whole URL, " +
+      "the platform changed and the comment above needs revisiting",
+  );
+  assert.ok(received.startsWith("https://example.invalid/oauth/authorize?client_id=calle"));
+});