fix(security): close cross-tenant server-ref misroute via AsyncLocalStorage

src/utils/server-ref.ts held the current request's MCP Server instance in
a module-level `let _server` singleton, set synchronously per request
(setServerRef, called from server.ts's createServer()) and read back later
by elicitation helpers (utils/elicitation.ts) — including after await gaps
inside async tool handlers.

In gateway (multi-tenant HTTP) mode, http.ts creates a fresh Server per
request. Under concurrent load this let two requests race through the
shared global: tenant A's request sets the ref and suspends on an await
(e.g. an in-flight N-central API call); before A resumes, tenant B's
request runs to completion and overwrites the module-level ref with B's
server; when A's awaited work resolves and calls elicitInput via
getServerRef(), it gets B's server — A's confirmation prompt is sent down
B's connection instead of A's. Confirmed live-exposed: AUTH_MODE=gateway /
MCP_TRANSPORT=http on rg-conduit-prod today, though not yet observed live
(distinct from the confirmed-live avanan-mcp incident).

Fix: replace the module-level singleton with an AsyncLocalStorage context
(runWithServerRef / bindServerRef / getServerRef), same shape as the
existing itglue-mcp / atera-mcp / blackpoint-mcp / avanan-mcp / datto-rmm-mcp
fixes for this identical shared-scaffold bug. http.ts now wraps the entire
per-request connect/handleRequest/catch chain inside runWithServerRef so
the bound context survives every await gap; index.ts's stdio entry point
uses bindServerRef once for the process lifetime (single-session, no
concurrent tenants to isolate). server.ts no longer touches server-ref at
all — callers own binding scope, not the factory.

Adds src/__tests__/server-ref.test.ts (ported from itglue-mcp's identical
regression test): forces the exact cross-tenant interleave deterministically
via a manually-resolved gate promise (not a timing stagger), and asserts BY
VALUE which tenant's mock server received the elicitation call. Verified
this test genuinely catches the bug, not just passes vacuously: temporarily
reinstated the old module-singleton implementation behind the same function
names — the new race-detection test failed with tenant A observing tenant
B's server ref, exactly the described misroute; passes again with the ALS
fix restored.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: asachs01

src/__tests__/server-ref.test.ts

@@ -0,0 +1,138 @@
+/**
+ * Regression test: cross-tenant "server reference" misrouting.
+ *
+ * Historically the server reference used by elicitation helpers
+ * (`utils/elicitation.ts`) was stored in a module-level `let _server`
+ * singleton in `utils/server-ref.ts` (`setServerRef` / `getServerRef`),
+ * set synchronously per request and read back later — including after
+ * `await` gaps inside async tool handlers (e.g. after awaiting an N-central
+ * API call, before sending an elicitation/confirmation prompt back through
+ * "the" server).
+ *
+ * In gateway (multi-tenant HTTP) mode a fresh `Server` is created per
+ * request, so two concurrent requests raced through that shared global:
+ * tenant A's request sets the ref and starts awaiting async work; before A
+ * resumes, tenant B's request runs to completion and overwrites the
+ * module-level ref with B's server/transport; when A's awaited work
+ * resolves and it reads the ref back to call `elicitInput`, it gets B's
+ * server — so A's confirmation prompt is sent down B's connection instead
+ * of A's (or vice versa, depending on timing).
+ *
+ * The fix replaces the module-level singleton with an AsyncLocalStorage
+ * context (`runWithServerRef` / `bindServerRef` / `getServerRef`), scoped
+ * per request and correctly restored across await gaps.
+ *
+ * This test forces the exact interleave deterministically — via a
+ * manually-resolved "gate" promise, not a timing-based stagger — and
+ * asserts, BY VALUE, which tenant's mock server actually received the
+ * elicitation call. Ported from itglue-mcp's identical regression test
+ * (same bug, same fix pattern, same shared server-ref.ts template).
+ */
+import { describe, it, expect, vi } from "vitest";
+import type { Server } from "@modelcontextprotocol/sdk/server/index.js";
+import { runWithServerRef, bindServerRef, getServerRef } from "../utils/server-ref.js";
+import { elicitConfirmation } from "../utils/elicitation.js";
+
+/** A deferred promise the test can resolve on demand, for a deterministic forced interleave. */
+function createDeferred<T = void>() {
+  let resolve!: (value: T) => void;
+  const promise = new Promise<T>((res) => {
+    resolve = res;
+  });
+  return { promise, resolve };
+}
+
+type FakeServer = Server & { tenantId: string; elicitInput: ReturnType<typeof vi.fn> };
+
+/** Minimal fake MCP Server whose elicitInput is a per-instance spy (per-tenant mock). */
+function createFakeServer(tenantId: string): FakeServer {
+  const elicitInput = vi.fn().mockImplementation(async () => ({
+    action: "accept" as const,
+    content: { confirm: true },
+  }));
+  return { tenantId, elicitInput } as unknown as FakeServer;
+}
+
+describe("server-ref cross-tenant isolation", () => {
+  it("getServerRef returns null outside of any bound context", () => {
+    expect(getServerRef()).toBeNull();
+  });
+
+  it("getServerRef resolves the server bound by runWithServerRef within its scope", async () => {
+    const server = createFakeServer("tenant-X");
+    await runWithServerRef(server, async () => {
+      expect(getServerRef()).toBe(server);
+    });
+  });
+
+  it(
+    "routes each tenant's elicitation through its OWN server, even when a " +
+      "second tenant's request runs to completion while the first is still " +
+      "in flight (forced deterministic interleave, not a timing stagger)",
+    async () => {
+      const serverA = createFakeServer("tenant-A");
+      const serverB = createFakeServer("tenant-B");
+      const gate = createDeferred<void>();
+
+      // Tenant A: binds its server, then suspends on an await gap
+      // (simulating an in-flight N-central API call inside a tool handler)
+      // BEFORE sending its elicitation/confirmation prompt.
+      const tenantA = runWithServerRef(serverA, async () => {
+        await gate.promise; // the exact await gap the original bug lost the ref across
+        expect((getServerRef() as FakeServer | null)?.tenantId).toBe("tenant-A"); // must still be A's server after resuming
+        return elicitConfirmation("Confirm tenant A's sensitive action?");
+      });
+
+      // Force the interleave: tenant B's ENTIRE request — bind, elicit,
+      // resolve — runs to completion while tenant A is still suspended
+      // above, exactly like a second concurrent HTTP request racing in.
+      const tenantB = runWithServerRef(serverB, async () => {
+        return elicitConfirmation("Confirm tenant B's sensitive action?");
+      });
+      await tenantB;
+
+      // Only now let tenant A resume.
+      gate.resolve();
+      const resultA = await tenantA;
+      expect(resultA).toBe(true);
+
+      // --- Per-tenant VALUE assertions -----------------------------------
+      // Each tenant's prompt must have gone out through THAT tenant's mock
+      // server specifically, not the other tenant's.
+      expect(serverA.elicitInput).toHaveBeenCalledTimes(1);
+      expect(serverA.elicitInput).toHaveBeenCalledWith(
+        expect.objectContaining({
+          message: "Confirm tenant A's sensitive action?",
+        })
+      );
+
+      expect(serverB.elicitInput).toHaveBeenCalledTimes(1);
+      expect(serverB.elicitInput).toHaveBeenCalledWith(
+        expect.objectContaining({
+          message: "Confirm tenant B's sensitive action?",
+        })
+      );
+
+      // Explicit negative checks: A's message must never have reached B's
+      // transport, and B's must never have reached A's.
+      for (const call of serverA.elicitInput.mock.calls) {
+        expect(call[0].message).not.toBe(
+          "Confirm tenant B's sensitive action?"
+        );
+      }
+      for (const call of serverB.elicitInput.mock.calls) {
+        expect(call[0].message).not.toBe(
+          "Confirm tenant A's sensitive action?"
+        );
+      }
+    }
+  );
+
+  it("bindServerRef binds for the remainder of the current async execution (stdio single-session mode)", async () => {
+    const server = createFakeServer("tenant-X");
+    bindServerRef(server);
+    // Simulate work continuing across an await gap in the same "session".
+    await Promise.resolve();
+    expect(getServerRef()).toBe(server);
+  });
+});

src/http.ts

@@ -14,6 +14,7 @@ import { createServer as createHttpServer } from 'node:http';
 import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';
 import { createServer } from './server.js';
 import { getCredentials, runWithCredentials } from './utils/client.js';
+import { runWithServerRef } from './utils/server-ref.js';
 import { logger } from './utils/logger.js';
 
 function headerValue(value: string | string[] | undefined): string | undefined {
@@ -65,36 +66,46 @@ function startHttpServer(): void {
     }
 
     const handle = async () => {
-      try {
-        // Stateless: fresh Server + Transport per request.
-        const server = createServer();
-        const transport = new StreamableHTTPServerTransport({
-          sessionIdGenerator: undefined,
-          enableJsonResponse: true,
-        });
+      // Stateless: fresh Server + Transport per request.
+      const server = createServer();
 
-        res.on('close', () => {
-          transport.close();
-          server.close();
-        });
+      // Bind this request's server into the per-request async context (not
+      // a module-level global) so elicitation helpers resolve *this*
+      // server/transport even after await gaps, and never a concurrent
+      // request's — see utils/server-ref.ts. The whole connect/handleRequest/
+      // catch chain must stay inside this callback so the bound context
+      // survives every await gap between here and any later getServerRef()
+      // call.
+      await runWithServerRef(server, async () => {
+        try {
+          const transport = new StreamableHTTPServerTransport({
+            sessionIdGenerator: undefined,
+            enableJsonResponse: true,
+          });
 
-        await server.connect(transport);
-        await transport.handleRequest(req, res);
-      } catch (error) {
-        logger.error('MCP transport error', {
-          error: error instanceof Error ? error.message : String(error),
-        });
-        if (!res.headersSent) {
-          res.writeHead(500, { 'Content-Type': 'application/json' });
-          res.end(
-            JSON.stringify({
-              jsonrpc: '2.0',
-              error: { code: -32603, message: 'Internal error' },
-              id: null,
-            })
-          );
+          res.on('close', () => {
+            transport.close();
+            server.close();
+          });
+
+          await server.connect(transport);
+          await transport.handleRequest(req, res);
+        } catch (error) {
+          logger.error('MCP transport error', {
+            error: error instanceof Error ? error.message : String(error),
+          });
+          if (!res.headersSent) {
+            res.writeHead(500, { 'Content-Type': 'application/json' });
+            res.end(
+              JSON.stringify({
+                jsonrpc: '2.0',
+                error: { code: -32603, message: 'Internal error' },
+                id: null,
+              })
+            );
+          }
         }
-      }
+      });
     };
 
     // Gateway mode: scope this request's credentials to an AsyncLocalStorage

src/index.ts

@@ -5,9 +5,14 @@
  */
 import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js';
 import { createServer } from './server.js';
+import { bindServerRef } from './utils/server-ref.js';
 import { logger } from './utils/logger.js';
 
 const server = createServer();
+// stdio is single-session (one process = one caller), so there is no
+// concurrent tenant to isolate from — bind once for the process lifetime
+// rather than per-request. See utils/server-ref.ts.
+bindServerRef(server);
 const transport = new StdioServerTransport();
 await server.connect(transport);
 logger.info('N-central MCP server started (stdio)');

src/server.ts

@@ -5,7 +5,6 @@ import {
 } from '@modelcontextprotocol/sdk/types.js';
 import { getNavigationTools, handleNavigate, handleStatus } from './domains/navigation.js';
 import { getAllDomainTools, getHandlerForTool } from './domains/index.js';
-import { setServerRef } from './utils/server-ref.js';
 import { logger } from './utils/logger.js';
 import type { DomainName } from './utils/types.js';
 
@@ -15,7 +14,10 @@ export function createServer(): Server {
     { capabilities: { tools: {} } }
   );
 
-  setServerRef(server);
+  // Caller (index.ts / http.ts) is responsible for binding this server into
+  // the per-request async context via runWithServerRef/bindServerRef — see
+  // utils/server-ref.ts. Doing it here would set a module-level ref, the
+  // exact cross-tenant bug this file used to have.
 
   // Expose ALL tools flat, always: the informational helpers plus every
   // domain's tools. This matches the deployed WYRE fleet and lets one-shot

src/utils/server-ref.ts

@@ -1,15 +1,72 @@
 /**
- * Shared MCP Server reference for elicitation support.
- * Avoids circular imports by decoupling the server instance from domain handlers.
+ * Per-request MCP Server reference for elicitation support.
+ *
+ * Avoids circular imports by decoupling the server instance from domain
+ * handlers, without leaking that reference across concurrent requests.
+ *
+ * SECURITY (cross-tenant misroute): this used to be a module-level
+ * `let _server` singleton, set synchronously per request via `setServerRef`
+ * and read back later via `getServerRef` — including after `await` gaps
+ * inside async tool handlers (e.g. after awaiting an N-central API call,
+ * before sending an elicitation/confirmation prompt back through "the"
+ * server).
+ *
+ * In gateway (multi-tenant HTTP) mode a fresh `Server` is created per
+ * request, so two concurrent requests race through that shared global:
+ * tenant A's request sets the ref and starts awaiting async work; before A
+ * resumes, tenant B's request runs and overwrites the module-level ref with
+ * B's server/transport; when A's awaited work resolves and it reads the ref
+ * back to call `elicitInput`, it gets B's server and A's confirmation prompt
+ * is sent down B's connection instead of A's.
+ *
+ * Fixed by binding the server reference to an AsyncLocalStorage context
+ * instead of a shared mutable variable. ALS scopes the value to the async
+ * call graph it was entered from, and correctly restores it after arbitrary
+ * `await` gaps, so concurrent requests can never observe each other's
+ * server.
+ *
+ * There is intentionally no module-level mutable server/transport state in
+ * this file.
  */
+import { AsyncLocalStorage } from 'node:async_hooks';
 import type { Server } from '@modelcontextprotocol/sdk/server/index.js';
 
-let _server: Server | null = null;
+/**
+ * Per-request server store.
+ */
+const serverRefStore = new AsyncLocalStorage<Server>();
+
+/**
+ * Run a callback with `server` bound to the async context for the duration
+ * of that callback — including anything it `await`s or schedules (promise
+ * chains, timers, etc). Use this for transports that create a fresh
+ * `Server` per inbound request (HTTP, Workers), one call per request, so
+ * concurrent requests never observe each other's server reference.
+ */
+export function runWithServerRef<T>(server: Server, fn: () => T): T {
+  return serverRefStore.run(server, fn);
+}
 
-export function setServerRef(server: Server): void {
-  _server = server;
+/**
+ * Bind `server` for the remainder of the current synchronous execution and
+ * all following async work, without requiring a wrapping callback.
+ *
+ * Only safe for single-session transports (stdio) where exactly one
+ * `Server` instance lives for the whole process and there are no
+ * concurrent tenants to isolate from each other. Do NOT use this for
+ * per-request transports (HTTP / Workers) — use `runWithServerRef` there,
+ * since `enterWith` has no natural "scope end" and would leak across
+ * requests just like the old module-level singleton.
+ */
+export function bindServerRef(server: Server): void {
+  serverRefStore.enterWith(server);
 }
 
+/**
+ * Get the server bound to the current request's async context, or `null`
+ * if none is bound (e.g. called outside of a request/session, before a
+ * server ref has been established).
+ */
 export function getServerRef(): Server | null {
-  return _server;
+  return serverRefStore.getStore() ?? null;
 }