|
| 1 | +/** |
| 2 | + * Regression test: cross-tenant "server reference" misrouting. |
| 3 | + * |
| 4 | + * Historically the server reference used by elicitation helpers |
| 5 | + * (`utils/elicitation.ts`) was stored in a module-level `let _server` |
| 6 | + * singleton in `utils/server-ref.ts` (`setServerRef` / `getServerRef`), |
| 7 | + * set synchronously per request and read back later — including after |
| 8 | + * `await` gaps inside async tool handlers (e.g. after awaiting a Syncro |
| 9 | + * API call, before sending an elicitation/confirmation prompt back |
| 10 | + * through "the" server). |
| 11 | + * |
| 12 | + * In gateway (multi-tenant HTTP) mode a fresh `Server` is created per |
| 13 | + * request, so two concurrent requests raced through that shared global: |
| 14 | + * tenant A's request sets the ref and starts awaiting async work; before A |
| 15 | + * resumes, tenant B's request runs to completion and overwrites the |
| 16 | + * module-level ref with B's server/transport; when A's awaited work |
| 17 | + * resolves and it reads the ref back to call `elicitInput`, it gets B's |
| 18 | + * server — so A's confirmation prompt is sent down B's connection instead |
| 19 | + * of A's (or vice versa, depending on timing). |
| 20 | + * |
| 21 | + * The fix replaces the module-level singleton with an AsyncLocalStorage |
| 22 | + * context (`runWithServerRef` / `bindServerRef` / `getServerRef`), scoped |
| 23 | + * per request and correctly restored across await gaps. |
| 24 | + * |
| 25 | + * This test forces the exact interleave deterministically — via a |
| 26 | + * manually-resolved "gate" promise, not a timing-based stagger — and |
| 27 | + * asserts, BY VALUE, which tenant's mock server actually received the |
| 28 | + * elicitation call. (Verified by temporarily reinstating a module-singleton |
| 29 | + * implementation behind the same function names: this test fails with |
| 30 | + * tenant A's prompt observed on tenant B's mock `elicitInput`, and passes |
| 31 | + * again once the ALS-based fix is restored — see the PR description.) |
| 32 | + */ |
| 33 | +import { describe, it, expect, vi } from "vitest"; |
| 34 | +import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; |
| 35 | +import { |
| 36 | + runWithServerRef, |
| 37 | + bindServerRef, |
| 38 | + getServerRef, |
| 39 | +} from "../../src/utils/server-ref.js"; |
| 40 | +import { elicitConfirmation } from "../../src/utils/elicitation.js"; |
| 41 | + |
| 42 | +/** A deferred promise the test can resolve on demand, for a deterministic forced interleave. */ |
| 43 | +function createDeferred<T = void>() { |
| 44 | + let resolve!: (value: T) => void; |
| 45 | + const promise = new Promise<T>((res) => { |
| 46 | + resolve = res; |
| 47 | + }); |
| 48 | + return { promise, resolve }; |
| 49 | +} |
| 50 | + |
| 51 | +type FakeServer = Server & { tenantId: string; elicitInput: ReturnType<typeof vi.fn> }; |
| 52 | + |
| 53 | +/** Minimal fake MCP Server whose elicitInput is a per-instance spy (per-tenant mock). */ |
| 54 | +function createFakeServer(tenantId: string): FakeServer { |
| 55 | + const elicitInput = vi.fn().mockImplementation(async () => ({ |
| 56 | + action: "accept" as const, |
| 57 | + content: { confirm: true }, |
| 58 | + })); |
| 59 | + return { tenantId, elicitInput } as unknown as FakeServer; |
| 60 | +} |
| 61 | + |
| 62 | +describe("server-ref cross-tenant isolation", () => { |
| 63 | + it("getServerRef returns null outside of any bound context", () => { |
| 64 | + expect(getServerRef()).toBeNull(); |
| 65 | + }); |
| 66 | + |
| 67 | + it("getServerRef resolves the server bound by runWithServerRef within its scope", async () => { |
| 68 | + const server = createFakeServer("tenant-X"); |
| 69 | + await runWithServerRef(server, async () => { |
| 70 | + expect(getServerRef()).toBe(server); |
| 71 | + }); |
| 72 | + }); |
| 73 | + |
| 74 | + it( |
| 75 | + "routes each tenant's elicitation through its OWN server, even when a " + |
| 76 | + "second tenant's request runs to completion while the first is still " + |
| 77 | + "in flight (forced deterministic interleave, not a timing stagger)", |
| 78 | + async () => { |
| 79 | + const serverA = createFakeServer("tenant-A"); |
| 80 | + const serverB = createFakeServer("tenant-B"); |
| 81 | + const gate = createDeferred<void>(); |
| 82 | + |
| 83 | + // Tenant A: binds its server, then suspends on an await gap |
| 84 | + // (simulating an in-flight Syncro API call inside a tool handler) |
| 85 | + // BEFORE sending its elicitation/confirmation prompt. |
| 86 | + const tenantA = runWithServerRef(serverA, async () => { |
| 87 | + await gate.promise; // the exact await gap the original bug lost the ref across |
| 88 | + expect((getServerRef() as FakeServer | null)?.tenantId).toBe("tenant-A"); // must still be A's server after resuming |
| 89 | + return elicitConfirmation("Confirm tenant A's sensitive action?"); |
| 90 | + }); |
| 91 | + |
| 92 | + // Force the interleave: tenant B's ENTIRE request — bind, elicit, |
| 93 | + // resolve — runs to completion while tenant A is still suspended |
| 94 | + // above, exactly like a second concurrent HTTP request racing in. |
| 95 | + const tenantB = runWithServerRef(serverB, async () => { |
| 96 | + return elicitConfirmation("Confirm tenant B's sensitive action?"); |
| 97 | + }); |
| 98 | + await tenantB; |
| 99 | + |
| 100 | + // Only now let tenant A resume. |
| 101 | + gate.resolve(); |
| 102 | + const resultA = await tenantA; |
| 103 | + expect(resultA).toBe(true); |
| 104 | + |
| 105 | + // --- Per-tenant VALUE assertions ----------------------------------- |
| 106 | + // Each tenant's prompt must have gone out through THAT tenant's mock |
| 107 | + // server specifically, not the other tenant's. |
| 108 | + expect(serverA.elicitInput).toHaveBeenCalledTimes(1); |
| 109 | + expect(serverA.elicitInput).toHaveBeenCalledWith( |
| 110 | + expect.objectContaining({ |
| 111 | + message: "Confirm tenant A's sensitive action?", |
| 112 | + }) |
| 113 | + ); |
| 114 | + |
| 115 | + expect(serverB.elicitInput).toHaveBeenCalledTimes(1); |
| 116 | + expect(serverB.elicitInput).toHaveBeenCalledWith( |
| 117 | + expect.objectContaining({ |
| 118 | + message: "Confirm tenant B's sensitive action?", |
| 119 | + }) |
| 120 | + ); |
| 121 | + |
| 122 | + // Explicit negative checks: A's message must never have reached B's |
| 123 | + // transport, and B's must never have reached A's. |
| 124 | + for (const call of serverA.elicitInput.mock.calls) { |
| 125 | + expect(call[0].message).not.toBe( |
| 126 | + "Confirm tenant B's sensitive action?" |
| 127 | + ); |
| 128 | + } |
| 129 | + for (const call of serverB.elicitInput.mock.calls) { |
| 130 | + expect(call[0].message).not.toBe( |
| 131 | + "Confirm tenant A's sensitive action?" |
| 132 | + ); |
| 133 | + } |
| 134 | + } |
| 135 | + ); |
| 136 | + |
| 137 | + it("bindServerRef binds for the remainder of the current async execution (stdio single-session mode)", async () => { |
| 138 | + const server = createFakeServer("tenant-X"); |
| 139 | + bindServerRef(server); |
| 140 | + // Simulate work continuing across an await gap in the same "session". |
| 141 | + await Promise.resolve(); |
| 142 | + expect(getServerRef()).toBe(server); |
| 143 | + }); |
| 144 | +}); |
0 commit comments