From 40042be522787aea59c801b435b83bcf9e07a59e Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 7 Nov 2026 19:41:58 +0000 Subject: [PATCH] fix(server): linked-environment MCP calls no longer follow a peer's redirects Tool calls to a linked environment went through the default fetch client, which follows redirects. A redirect from the peer's /mcp endpoint could move the call, its arguments included, to an origin that never passed the link checks, and the answer from that origin was taken as the peer's. MCP calls now refuse redirects the same way link calls already do, through one shared helper, so a redirect surfaces as the existing "HTTP 3xx" failure. Co-authored-by: Claude Opus 5.5 --- apps/server/src/peer/PeerLinks.test.ts | 28 +-------- apps/server/src/peer/PeerLinks.testkit.ts | 21 +++++++ apps/server/src/peer/PeerLinks.ts | 15 +++-- apps/server/src/peer/PeerMcpClient.test.ts | 66 +++++++++++++++++++++- apps/server/src/peer/PeerMcpClient.ts | 2 +- 5 files changed, 98 insertions(+), 34 deletions(-) diff --git a/apps/server/src/peer/PeerLinks.test.ts b/apps/server/src/peer/PeerLinks.test.ts index 36c9b09fb4459..94792f43d88a3 100644 --- a/apps/server/src/peer/PeerLinks.test.ts +++ b/apps/server/src/peer/PeerLinks.test.ts @@ -14,13 +14,7 @@ import * as HttpRouter from "effect/http/HttpRouter"; import * as SqlClient from "effect/sql/SqlClient"; import * as TestClock from "effect/testing/TestClock"; import { Tool, Toolkit } from "effect/ai"; -import { - FetchHttpClient, - HttpClient, - HttpServer, - HttpServerRequest, - HttpServerResponse, -} from "effect/http"; +import { FetchHttpClient, HttpClient, HttpServerRequest, HttpServerResponse } from "effect/http"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import * as McpHttpServer from "../mcp/McpHttpServer.ts"; @@ -32,6 +26,7 @@ import { descriptorOf, layerLinkingEnvironment, servePeer, + serveRaw, type ServedPeer, } from "./PeerLinks.testkit.ts"; import * as PeerMcpClient from "./PeerMcpClient.ts"; @@ -306,25 +301,6 @@ it.effect("two links racing to one peer leave the stored access matching the tok ).pipe(Effect.provide(NodeHttpServer.layerTest)), ); -/** A bare HTTP server on a socket of its own, answering with `handle`. */ -const serveRaw = ( - handle: ( - request: HttpServerRequest.HttpServerRequest, - ) => Effect.Effect, -) => - Layer.build(Layer.fresh(NodeHttpServer.layerTest)).pipe( - Effect.flatMap((server) => - Effect.gen(function* () { - yield* HttpServer.serve(Effect.flatMap(HttpServerRequest.HttpServerRequest, handle)).pipe( - Layer.build, - ); - const address = (yield* HttpServer.HttpServer).address; - if (address._tag === "UnixPathAddress") return yield* Effect.die("expected a TCP address"); - return `http://127.0.0.1:${address.port}`; - }).pipe(Effect.provideContext(server)), - ), - ); - it.effect("never follows a peer's redirect with the pairing code", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/peer/PeerLinks.testkit.ts b/apps/server/src/peer/PeerLinks.testkit.ts index b2969bd86332e..c8c769bf75457 100644 --- a/apps/server/src/peer/PeerLinks.testkit.ts +++ b/apps/server/src/peer/PeerLinks.testkit.ts @@ -1,4 +1,5 @@ import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; +import * as NodeHttpServer from "@effect/platform-node/NodeHttpServer"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { EnvironmentHttpApi, @@ -14,6 +15,7 @@ import * as FetchHttpClient from "effect/http/FetchHttpClient"; import * as HttpRouter from "effect/http/HttpRouter"; import * as HttpServer from "effect/http/HttpServer"; import * as HttpServerRequest from "effect/http/HttpServerRequest"; +import type * as HttpServerResponse from "effect/http/HttpServerResponse"; import * as HttpApi from "effect/http-api/HttpApi"; import * as HttpApiBuilder from "effect/http-api/HttpApiBuilder"; @@ -150,6 +152,25 @@ export const servePeer = ( export type ServedPeer = Effect.Success>; +/** A bare HTTP server on a socket of its own, answering with `handle`. */ +export const serveRaw = ( + handle: ( + request: HttpServerRequest.HttpServerRequest, + ) => Effect.Effect, +) => + Layer.build(Layer.fresh(NodeHttpServer.layerTest)).pipe( + Effect.flatMap((server) => + Effect.gen(function* () { + yield* HttpServer.serve(Effect.flatMap(HttpServerRequest.HttpServerRequest, handle)).pipe( + Layer.build, + ); + const address = (yield* HttpServer.HttpServer).address; + if (address._tag === "UnixPathAddress") return yield* Effect.die("expected a TCP address"); + return `http://127.0.0.1:${address.port}`; + }).pipe(Effect.provideContext(server)), + ), + ); + /** * The environment that links: its own database and secret store, reaching * peers over plain HTTP. Provide it to anything built on its links. diff --git a/apps/server/src/peer/PeerLinks.ts b/apps/server/src/peer/PeerLinks.ts index dfcab9f11f273..4d5152500c2e6 100644 --- a/apps/server/src/peer/PeerLinks.ts +++ b/apps/server/src/peer/PeerLinks.ts @@ -163,6 +163,15 @@ const peerOrigin = (url: string) => { return Effect.succeed(parsed.origin); }; +/** + * Only the origin `peerOrigin` checked may receive a pairing code, token or + * tool call: a redirect could forward them anywhere, including plain http. + */ +export const withoutRedirects = (client: HttpClient.HttpClient) => + HttpClient.transform(client, (response) => + Effect.provideService(response, FetchHttpClient.RequestInit, { redirect: "manual" }), + ); + /** Whether a peer holds a linked agent to its limits and keeps its work within its link. */ const linkLimitsHold = (peer: { readonly capabilities: ExecutionEnvironmentCapabilities }) => peer.capabilities.mcpModeLimitHeader === true && peer.capabilities.linkFence === true; @@ -186,11 +195,7 @@ const make = Effect.gen(function* () { Effect.orDie, ); - // Only the origin `peerOrigin` checked may receive a pairing code or token: - // a redirect could forward them anywhere, including plain http. - const peerHttpClient = HttpClient.transform(httpClient, (response) => - Effect.provideService(response, FetchHttpClient.RequestInit, { redirect: "manual" }), - ); + const peerHttpClient = withoutRedirects(httpClient); const clientFor = (origin: string) => HttpApiClient.make(PeerHttpApi, { baseUrl: origin }).pipe( diff --git a/apps/server/src/peer/PeerMcpClient.test.ts b/apps/server/src/peer/PeerMcpClient.test.ts index cd2b0a9e52b16..c584448d7fa24 100644 --- a/apps/server/src/peer/PeerMcpClient.test.ts +++ b/apps/server/src/peer/PeerMcpClient.test.ts @@ -10,14 +10,26 @@ import * as Layer from "effect/Layer"; import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; import { Tool, Toolkit } from "effect/ai"; -import { HttpClient, HttpClientResponse } from "effect/http"; +import { + HttpClient, + HttpClientResponse, + HttpRouter, + HttpServerRequest, + HttpServerResponse, +} from "effect/http"; import * as McpHttpServer from "../mcp/McpHttpServer.ts"; import * as McpInvocationContext from "../mcp/McpInvocationContext.ts"; import * as McpToolAccess from "../mcp/McpToolAccess.ts"; import * as PeerLinks from "./PeerLinks.ts"; import * as PeerMcpClient from "./PeerMcpClient.ts"; -import { descriptorOf, layerLinkingEnvironment, linkTo, servePeer } from "./PeerLinks.testkit.ts"; +import { + descriptorOf, + layerLinkingEnvironment, + linkTo, + servePeer, + serveRaw, +} from "./PeerLinks.testkit.ts"; const Result = Schema.Struct({ modes: Schema.String }); const toolkit = Toolkit.make( @@ -138,6 +150,56 @@ it.effect("keeps a link healthy when the peer refuses a call", () => ).pipe(Effect.provide(NodeHttpServer.layerTest)), ); +it.effect("never follows a peer's redirect on its MCP endpoint", () => + Effect.scoped( + Effect.gen(function* () { + const received = yield* Ref.make>([]); + const elsewhere = yield* serveRaw((request) => + Ref.update(received, (seen) => [...seen, `${request.method} ${request.url}`]).pipe( + Effect.as(HttpServerResponse.empty({ status: 404 })), + ), + ); + const redirectMcp = HttpRouter.middleware( + (httpEffect) => + Effect.gen(function* () { + const request = yield* HttpServerRequest.HttpServerRequest; + return request.url.startsWith("/mcp") + ? HttpServerResponse.redirect(`${elsewhere}${request.url}`, { status: 307 }) + : yield* httpEffect; + }), + { global: true }, + ); + const handlers = McpToolAccess.toLayer(toolkit, { + probe: McpToolAccess.reads(() => Effect.succeed({ modes: "" })), + hold: McpToolAccess.reads(() => Effect.succeed({ modes: "" })), + refuse: McpToolAccess.reads(() => Effect.fail(refusal)), + }); + const box = descriptorOf("environment-box", "Box"); + const b = yield* servePeer( + box, + Layer.merge(McpHttpServer.toolkitRegistration(toolkit, handlers), redirectMcp), + ); + const context = yield* Layer.build( + layerLinkingEnvironment(descriptorOf("environment-a", "A")), + ); + const links = Context.get(context, PeerLinks.PeerLinks); + const client = Context.get(context, PeerMcpClient.PeerMcpClient); + yield* linkTo(links, b); + const failed = yield* client + .call({ + environmentId: box.environmentId, + tool: "probe", + arguments: {}, + limits: { runtimeMode: "full-access", interactionMode: "default" }, + success: Result, + }) + .pipe(Effect.flip); + expect(yield* Ref.get(received)).toEqual([]); + expect(failed.message).toContain("HTTP 307"); + }), + ).pipe(Effect.provide(NodeHttpServer.layerTest)), +); + it.effect("renews a missing peer session once and reuses its replacement", () => Effect.gen(function* () { const sessionHeaders: Array = []; diff --git a/apps/server/src/peer/PeerMcpClient.ts b/apps/server/src/peer/PeerMcpClient.ts index 885c91dce8665..898e0a19c440b 100644 --- a/apps/server/src/peer/PeerMcpClient.ts +++ b/apps/server/src/peer/PeerMcpClient.ts @@ -96,7 +96,7 @@ const responseJson = (contentType: string | undefined, text: string) => const make = Effect.gen(function* () { const links = yield* PeerLinks.PeerLinks; - const httpClient = yield* HttpClient.HttpClient; + const httpClient = PeerLinks.withoutRedirects(yield* HttpClient.HttpClient); // One MCP session per peer and address, reused across calls. The peer keeps // sessions in memory, so a restart there answers 404 and we start a new one. const sessions = yield* Ref.make>(new Map());