fix(oauth): Windows openBrowser URL quoting (v0.11.2)

docs/installation.ja.md

@@ -46,6 +46,8 @@ A browser window should have opened automatically. Sign in on GitHub, then retry
 
 > **ブラウザ自動オープンが失敗した場合:** 応答と stderr ログに URL がそのまま残るので、手動でコピーしてブラウザに貼り付けてください。Windows では `start`、macOS では `open`、Linux では `xdg-open` を使用します。
 
+> **v0.11.2 Windows hotfix:** v0.11.1 の Windows 版はブラウザ自動オープン時に authorize URL を cmd.exe にクォート無しで渡していたため、URL に含まれる `&` が command separator として解釈され、`state` パラメータが欠落して `/oauth/authorize` が 400 を返していました。v0.11.2 では URL を `"..."` で囲んで shell 経由で起動するようにしたので、`&` がリテラル扱いされ state が正しくブラウザに届きます。v0.11.1 で自動オープンが機能しなかった Windows ユーザーは v0.11.2 に更新してください（手動 URL コピーは引き続きフォールバックとして動作します）。
+
 > **旧バージョンからの移行:** v0.11.0 以前（localhost callback flow / device flow どちらも）の `~/.github-webhook-mcp/oauth-tokens.json` は flow marker が一致しないため自動で無視され、初回ツール呼び出し時に新しい web OAuth flow で再認証が走ります。特別な手作業は不要です。
 
 ### Claude Desktop — デスクトップ拡張 (.mcpb)

docs/installation.md

@@ -46,6 +46,8 @@ A browser window should have opened automatically. Sign in on GitHub, then retry
 
 > **ブラウザ自動オープンが失敗した場合:** 応答と stderr ログに URL がそのまま残るので、手動でコピーしてブラウザに貼り付けてください。Windows では `start`、macOS では `open`、Linux では `xdg-open` を使用します。
 
+> **v0.11.2 Windows hotfix:** v0.11.1 の Windows 版はブラウザ自動オープン時に authorize URL を cmd.exe にクォート無しで渡していたため、URL に含まれる `&` が command separator として解釈され、`state` パラメータが欠落して `/oauth/authorize` が 400 を返していました。v0.11.2 では URL を `"..."` で囲んで shell 経由で起動するようにしたので、`&` がリテラル扱いされ state が正しくブラウザに届きます。v0.11.1 で自動オープンが機能しなかった Windows ユーザーは v0.11.2 に更新してください（手動 URL コピーは引き続きフォールバックとして動作します）。
+
 > **旧バージョンからの移行:** v0.11.0 以前（localhost callback flow / device flow どちらも）の `~/.github-webhook-mcp/oauth-tokens.json` は flow marker が一致しないため自動で無視され、初回ツール呼び出し時に新しい web OAuth flow で再認証が走ります。特別な手作業は不要です。
 
 ### Claude Desktop — デスクトップ拡張 (.mcpb)

local-mcp/src/index.ts

@@ -188,13 +188,26 @@ function openBrowser(url: string): void {
     const plat = osPlatform();
     let command: string;
     let args: string[];
-    const options = { detached: true, stdio: "ignore" as const };
+    const options: { detached: boolean; stdio: "ignore"; shell?: boolean } = {
+      detached: true,
+      stdio: "ignore" as const,
+    };
 
     if (plat === "win32") {
-      // `start` is a cmd.exe builtin. The empty "" argument is the window
-      // title placeholder expected when the URL is quoted.
-      command = "cmd.exe";
-      args = ["/c", "start", "", url];
+      // `start` is a cmd.exe builtin, not a standalone executable, so the
+      // command must run through a shell. The empty "" is the window-title
+      // placeholder expected by `start` when the following token is quoted.
+      //
+      // URL-quoting is mandatory on Windows: cmd.exe treats `&` as a command
+      // separator unless the argument is wrapped in double quotes, so an
+      // unquoted OAuth authorize URL like
+      //   https://worker/oauth/authorize?client_id=X&state=Y
+      // would truncate at `&state=` and drop the `state` parameter (v0.11.2
+      // fix for #211). `shell: true` hands the full command string to
+      // cmd.exe so the outer quotes survive argv reassembly intact.
+      command = `start "" "${url}"`;
+      args = [];
+      options.shell = true;
     } else if (plat === "darwin") {
       command = "open";
       args = [url];

mcp-server/manifest.json

@@ -2,7 +2,7 @@
   "manifest_version": "0.3",
   "name": "github-webhook-mcp",
   "display_name": "GitHub Webhook MCP",
-  "version": "0.11.1",
+  "version": "0.11.2",
   "description": "Real-time GitHub webhook notifications via Cloudflare Worker + Durable Object.",
   "long_description": "GitHub Webhook MCP bridges GitHub webhook events to Claude via a Cloudflare Worker backend. Events are stored in a Durable Object with SQLite, queried through MCP tools, and optionally pushed in real-time via SSE channel notifications. No local webhook receiver needed — just point your GitHub webhook at the Worker URL.",
   "author": {

mcp-server/package.json

@@ -1,6 +1,6 @@
 {
   "name": "github-webhook-mcp",
-  "version": "0.11.1",
+  "version": "0.11.2",
   "description": "MCP server bridging GitHub webhooks via Cloudflare Worker",
   "type": "module",
   "bin": {

mcp-server/server.json

@@ -2,7 +2,7 @@
   "$schema": "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json",
   "name": "io.github.Liplus-Project/github-webhook-mcp",
   "description": "MCP server bridging GitHub webhooks via Cloudflare Worker for real-time event streaming",
-  "version": "0.11.1",
+  "version": "0.11.2",
   "repository": {
     "url": "https://github.com/Liplus-Project/github-webhook-mcp",
     "source": "github",
@@ -13,7 +13,7 @@
     {
       "registryType": "npm",
       "identifier": "github-webhook-mcp",
-      "version": "0.11.1",
+      "version": "0.11.2",
       "runtimeHint": "npx",
       "transport": {
         "type": "stdio"

mcp-server/server/index.js

@@ -171,11 +171,20 @@ function openBrowser(url) {
     let options = { detached: true, stdio: "ignore" };
 
     if (plat === "win32") {
-      // `start` is a cmd.exe builtin, not a standalone executable.
-      // The empty "" argument is the window title placeholder expected by
-      // `start` when the URL is quoted.
-      command = "cmd.exe";
-      args = ["/c", "start", "", url];
+      // `start` is a cmd.exe builtin, not a standalone executable, so the
+      // command must run through a shell. The empty "" is the window-title
+      // placeholder expected by `start` when the following token is quoted.
+      //
+      // URL-quoting is mandatory on Windows: cmd.exe treats `&` as a command
+      // separator unless the argument is wrapped in double quotes, so an
+      // unquoted OAuth authorize URL like
+      //   https://worker/oauth/authorize?client_id=X&state=Y
+      // would truncate at `&state=` and drop the `state` parameter (v0.11.2
+      // fix for #211). `shell: true` hands the full command string to
+      // cmd.exe so the outer quotes survive argv reassembly intact.
+      command = `start "" "${url}"`;
+      args = [];
+      options.shell = true;
     } else if (plat === "darwin") {
       command = "open";
       args = [url];

mcp-server/test/open-browser.test.mjs

@@ -0,0 +1,191 @@
+/**
+ * Argument-construction tests for openBrowser() platform branches.
+ *
+ * Root cause of #211: on Windows the previous implementation passed the
+ * OAuth authorize URL to `cmd.exe /c start "" <URL>` unquoted. cmd.exe
+ * interprets `&` as a command separator, truncating URLs like
+ *   https://worker/oauth/authorize?client_id=X&state=Y
+ * at `&state=...`, dropping the state parameter and breaking the Worker's
+ * `/oauth/authorize` handler (`invalid_request: state parameter is required`).
+ *
+ * The v0.11.2 fix wraps the URL in double quotes and runs through a shell
+ * so that cmd.exe treats the quoted `&` as literal. macOS (`open`) and
+ * Linux (`xdg-open`) branches are unaffected because both launchers accept
+ * `&` as a normal argv character.
+ *
+ * We re-implement openBrowser() inline for the same reason migration.test.mjs
+ * and web-auth-required.test.mjs do: server/index.js cannot be imported
+ * without starting an MCP server (top-level await on server.connect).
+ * The assertions below lock down the argv shape for each platform and
+ * verify the `&`-containing URL survives intact to the spawn boundary.
+ */
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+/**
+ * Platform-parameterised mirror of openBrowser() from server/index.js.
+ * Returns the exact { command, args, options } tuple that would be passed
+ * to child_process.spawn, without actually spawning anything.
+ */
+function buildSpawnArgsForPlatform(plat, url) {
+  let command;
+  let args;
+  const options = { detached: true, stdio: "ignore" };
+
+  if (plat === "win32") {
+    command = `start "" "${url}"`;
+    args = [];
+    options.shell = true;
+  } else if (plat === "darwin") {
+    command = "open";
+    args = [url];
+  } else {
+    command = "xdg-open";
+    args = [url];
+  }
+
+  return { command, args, options };
+}
+
+const AUTHORIZE_URL_WITH_AMPERSAND =
+  "https://github-webhook.smgjp.com/oauth/authorize?client_id=abc123&state=xyz789";
+
+test("windows branch wraps URL in double quotes and runs through shell", () => {
+  const { command, args, options } = buildSpawnArgsForPlatform(
+    "win32",
+    AUTHORIZE_URL_WITH_AMPERSAND,
+  );
+
+  // The command string must quote the URL so cmd.exe treats `&` as literal.
+  assert.equal(
+    command,
+    `start "" "${AUTHORIZE_URL_WITH_AMPERSAND}"`,
+    "windows command must be `start \"\" \"<url>\"` with the URL in quotes",
+  );
+  assert.ok(
+    command.includes(`"${AUTHORIZE_URL_WITH_AMPERSAND}"`),
+    "authorize URL must appear inside the outer double-quote pair",
+  );
+  assert.deepEqual(args, [], "no separate argv when using shell: true");
+  assert.equal(options.shell, true, "shell: true is required for the cmd.exe builtin `start`");
+  assert.equal(options.detached, true);
+  assert.equal(options.stdio, "ignore");
+});
+
+test("windows branch preserves the state parameter across & (regression for #211)", () => {
+  const { command } = buildSpawnArgsForPlatform(
+    "win32",
+    AUTHORIZE_URL_WITH_AMPERSAND,
+  );
+
+  // The whole URL, including everything after `&`, must appear verbatim in
+  // the command string — this is the condition that lets cmd.exe's quote
+  // handling keep `&state=xyz789` attached to the authorize URL instead of
+  // treating it as a second command.
+  assert.ok(
+    command.includes("&state=xyz789"),
+    "state parameter must survive on the Windows command line",
+  );
+  assert.ok(
+    command.includes("client_id=abc123"),
+    "client_id must also survive on the Windows command line",
+  );
+});
+
+test("darwin branch uses `open` with the URL as a plain argv", () => {
+  const { command, args, options } = buildSpawnArgsForPlatform(
+    "darwin",
+    AUTHORIZE_URL_WITH_AMPERSAND,
+  );
+
+  assert.equal(command, "open");
+  assert.deepEqual(args, [AUTHORIZE_URL_WITH_AMPERSAND]);
+  assert.notEqual(options.shell, true, "no shell needed on macOS");
+});
+
+test("linux branch uses `xdg-open` with the URL as a plain argv", () => {
+  const { command, args, options } = buildSpawnArgsForPlatform(
+    "linux",
+    AUTHORIZE_URL_WITH_AMPERSAND,
+  );
+
+  assert.equal(command, "xdg-open");
+  assert.deepEqual(args, [AUTHORIZE_URL_WITH_AMPERSAND]);
+  assert.notEqual(options.shell, true, "no shell needed on Linux");
+});
+
+test("non-win32 platforms pass URL as single argv element (no cmd interpretation)", () => {
+  for (const plat of ["darwin", "linux", "freebsd", "openbsd"]) {
+    const { args } = buildSpawnArgsForPlatform(plat, AUTHORIZE_URL_WITH_AMPERSAND);
+    assert.equal(args.length, 1, `${plat}: URL must be a single argv element`);
+    assert.equal(
+      args[0],
+      AUTHORIZE_URL_WITH_AMPERSAND,
+      `${plat}: URL must survive verbatim (argv bypasses shell interpretation)`,
+    );
+  }
+});
+
+/**
+ * Integration-style test: swap in a fake spawn and verify openBrowser's
+ * caller-visible side effects. The inline openBrowser mirrors server/index.js
+ * exactly; the fake spawn captures the call shape and exposes a `.on` hook so
+ * the error-handler wiring is exercised too.
+ */
+test("openBrowser dispatches to spawn with platform-correct arguments", () => {
+  const calls = [];
+
+  function fakeSpawn(command, args, options) {
+    calls.push({ command, args, options });
+    return {
+      on: () => {},
+      unref: () => {},
+    };
+  }
+
+  function openBrowser(url, plat) {
+    if (!url || typeof url !== "string") return;
+    const { command, args, options } = buildSpawnArgsForPlatform(plat, url);
+    const child = fakeSpawn(command, args, options);
+    child.on("error", () => {});
+    if (typeof child.unref === "function") child.unref();
+  }
+
+  openBrowser(AUTHORIZE_URL_WITH_AMPERSAND, "win32");
+  openBrowser(AUTHORIZE_URL_WITH_AMPERSAND, "darwin");
+  openBrowser(AUTHORIZE_URL_WITH_AMPERSAND, "linux");
+
+  assert.equal(calls.length, 3);
+
+  // win32
+  assert.equal(calls[0].command, `start "" "${AUTHORIZE_URL_WITH_AMPERSAND}"`);
+  assert.equal(calls[0].options.shell, true);
+
+  // darwin
+  assert.equal(calls[1].command, "open");
+  assert.deepEqual(calls[1].args, [AUTHORIZE_URL_WITH_AMPERSAND]);
+
+  // linux
+  assert.equal(calls[2].command, "xdg-open");
+  assert.deepEqual(calls[2].args, [AUTHORIZE_URL_WITH_AMPERSAND]);
+});
+
+test("openBrowser is a no-op on empty/invalid input", () => {
+  const calls = [];
+  function fakeSpawn() {
+    calls.push("called");
+    return { on: () => {}, unref: () => {} };
+  }
+  function openBrowser(url, plat) {
+    if (!url || typeof url !== "string") return;
+    const { command, args, options } = buildSpawnArgsForPlatform(plat, url);
+    fakeSpawn(command, args, options);
+  }
+
+  openBrowser("", "win32");
+  openBrowser(null, "win32");
+  openBrowser(undefined, "win32");
+  openBrowser(42, "win32");
+
+  assert.equal(calls.length, 0, "invalid URLs must short-circuit before spawn");
+});