History

Clean MCP per-user auth test boundaries

Rhys Sullivan committed 6 months ago

Commit 7656b2e1 parent b039b6e1 file · +18 −23Browse files

Unified · Split

packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts+18−23
@@ -19,7 +19,7 @@ · 18 unmodified lines
19import * as http from "node:http";19import * as http from "node:http";
20 20
21import { describe, expect, it } from "@effect/vitest";21import { describe, expect, it } from "@effect/vitest";
22import { Cause, Effect, Exit } from "effect";22import { Cause, Effect, Exit, Predicate } from "effect";
23import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";23import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
24import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js";24import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js";
25import { z } from "zod";25import { z } from "zod";
@@ -37,6 +37,7 @@ import { · 11 unmodified lines
37 definePlugin,37 definePlugin,
38 makeInMemoryBlobStore,38 makeInMemoryBlobStore,
39 type SecretProvider,39 type SecretProvider,
40 type ToolInvocationError,
40} from "@executor-js/sdk";41} from "@executor-js/sdk";
41import { makeMemoryAdapter } from "@executor-js/storage-core/testing/memory";42import { makeMemoryAdapter } from "@executor-js/storage-core/testing/memory";
42 43
@@ -96,6 +97,12 @@ type TestServer = { · 53 unmodified lines
96 readonly recorded: () => readonly RecordedRequest[];97 readonly recorded: () => readonly RecordedRequest[];
97};98};
98 99
100const failureError = <E>(exit: Exit.Exit<unknown, E>): E | undefined =>
101 Exit.isFailure(exit) ? exit.cause.reasons.find(Cause.isFailReason)?.error : undefined;
102
103const isToolInvocationError = (error: unknown): error is ToolInvocationError =>
104 Predicate.isTagged(error, "ToolInvocationError");
105
99const createAuthRecordingServer: Effect.Effect<TestServer, Error, never> =106const createAuthRecordingServer: Effect.Effect<TestServer, Error, never> =
100 Effect.callback<TestServer, Error>((resume) => {107 Effect.callback<TestServer, Error>((resume) => {
101 const transports = new Map<string, StreamableHTTPServerTransport>();108 const transports = new Map<string, StreamableHTTPServerTransport>();
@@ -258,7 +265,7 @@ describe("per-user MCP auth isolation", () => { · 156 unmodified lines
258 // stack [userB, org] can see them via fall-through.265 // stack [userB, org] can see them via fall-through.
259 yield* execUserA.mcp.addSource({266 yield* execUserA.mcp.addSource({
260 transport: "remote",267 transport: "remote",
261 scope: ORG as string,268 scope: ORG,
262 name: "Shared MCP",269 name: "Shared MCP",
263 endpoint: server.url,270 endpoint: server.url,
264 namespace: "iso_test",271 namespace: "iso_test",
@@ -312,21 +319,15 @@ describe("per-user MCP auth isolation", () => { · 47 unmodified lines
312 // Pin the exact error tag so a future regression that swaps319 // Pin the exact error tag so a future regression that swaps
313 // the "connection not found" check for a silent `auth: { kind:320 // the "connection not found" check for a silent `auth: { kind:
314 // "none" }` fallback would fail here, not silently connect.321 // "none" }` fallback would fail here, not silently connect.
315 if (!Exit.isFailure(userBResult)) return;
316 // tools.invoke wraps plugin failures in ToolInvocationError322 // tools.invoke wraps plugin failures in ToolInvocationError
317 // with the original error carried on `cause`. Pin the exact323 // with the original error carried on `cause`. Pin the exact
318 // inner tag — a regression that swapped the "no connection324 // inner tag — a regression that swapped the "no connection
319 // found" check for a silent no-auth fallback would either325 // found" check for a silent no-auth fallback would either
320 // succeed outright (leaking) or surface a different tag here.326 // succeed outright (leaking) or surface a different tag here.
321 const failure = userBResult.cause.reasons.find(Cause.isFailReason);327 const outer = failureError(userBResult);
322 const outer = failure?.error as328 expect(isToolInvocationError(outer)).toBe(true);
323 | {329 const inner = isToolInvocationError(outer) ? outer.cause : undefined;
324 _tag?: string;330 expect(Predicate.isTagged(inner, "McpConnectionError")).toBe(true);
325 cause?: { _tag?: string };
326 }
327 | undefined;
328 expect(outer?._tag).toBe("ToolInvocationError");
329 expect(outer?.cause?._tag).toBe("McpConnectionError");
330 331
331 // CRITICAL: no outbound MCP request was made on user B's behalf332 // CRITICAL: no outbound MCP request was made on user B's behalf
332 // carrying user A's bearer token. Auth resolution must have333 // carrying user A's bearer token. Auth resolution must have
@@ -366,7 +367,7 @@ describe("per-user MCP auth isolation", () => { · 33 unmodified lines
366 367
367 yield* execUserA.mcp.addSource({368 yield* execUserA.mcp.addSource({
368 transport: "remote",369 transport: "remote",
369 scope: ORG as string,370 scope: ORG,
370 name: "Shared MCP (header)",371 name: "Shared MCP (header)",
371 endpoint: server.url,372 endpoint: server.url,
372 namespace: "iso_header",373 namespace: "iso_header",
@@ -418,21 +419,15 @@ describe("per-user MCP auth isolation", () => { · 45 unmodified lines
418 );419 );
419 420
420 expect(Exit.isFailure(userBResult)).toBe(true);421 expect(Exit.isFailure(userBResult)).toBe(true);
421 if (!Exit.isFailure(userBResult)) return;
422 // tools.invoke wraps plugin failures in ToolInvocationError422 // tools.invoke wraps plugin failures in ToolInvocationError
423 // with the original error carried on `cause`. Pin the exact423 // with the original error carried on `cause`. Pin the exact
424 // inner tag — a regression that swapped the "no connection424 // inner tag — a regression that swapped the "no connection
425 // found" check for a silent no-auth fallback would either425 // found" check for a silent no-auth fallback would either
426 // succeed outright (leaking) or surface a different tag here.426 // succeed outright (leaking) or surface a different tag here.
427 const failure = userBResult.cause.reasons.find(Cause.isFailReason);427 const outer = failureError(userBResult);
428 const outer = failure?.error as428 expect(isToolInvocationError(outer)).toBe(true);
429 | {429 const inner = isToolInvocationError(outer) ? outer.cause : undefined;
430 _tag?: string;430 expect(Predicate.isTagged(inner, "McpConnectionError")).toBe(true);
431 cause?: { _tag?: string };
432 }
433 | undefined;
434 expect(outer?._tag).toBe("ToolInvocationError");
435 expect(outer?.cause?._tag).toBe("McpConnectionError");
436 431
437 const afterUserB = server.recorded().slice(recordedBeforeUserB);432 const afterUserB = server.recorded().slice(recordedBeforeUserB);
438 for (const req of afterUserB) {433 for (const req of afterUserB) {