fix(mcp): follow redirects with per-hop SSRF re-validation

backend/src/lib/mcp/__tests__/client.ssrf.test.ts

@@ -143,3 +143,121 @@ describe("guardedFetch", () => {
         expect(nextInit.dispatcher).toBe(init.dispatcher);
     });
 });
+
+describe("guardedFetch redirect following", () => {
+    function redirectTo(location: string, status = 302) {
+        return new Response(null, { status, headers: { location } });
+    }
+
+    it("follows a redirect and returns the final response", async () => {
+        resolvesTo("93.184.216.34");
+        const fetchSpy = vi
+            .spyOn(globalThis, "fetch")
+            .mockResolvedValueOnce(
+                redirectTo("https://public.example.com/mcp/.well-known/x"),
+            )
+            .mockResolvedValueOnce(new Response("{}", { status: 200 }));
+
+        const res = await guardedFetch(
+            "https://public.example.com/.well-known/x/mcp",
+        );
+
+        expect(res.status).toBe(200);
+        expect(fetchSpy).toHaveBeenCalledTimes(2);
+        expect(fetchSpy.mock.calls[1][0]).toBe(
+            "https://public.example.com/mcp/.well-known/x",
+        );
+    });
+
+    it("resolves a relative Location against the current URL", async () => {
+        resolvesTo("93.184.216.34");
+        const fetchSpy = vi
+            .spyOn(globalThis, "fetch")
+            .mockResolvedValueOnce(redirectTo("/elsewhere", 307))
+            .mockResolvedValueOnce(new Response("{}", { status: 200 }));
+
+        await guardedFetch("https://public.example.com/a/b");
+
+        expect(fetchSpy.mock.calls[1][0]).toBe(
+            "https://public.example.com/elsewhere",
+        );
+    });
+
+    it("re-validates each hop and refuses a redirect to a private address", async () => {
+        lookupMock.mockImplementation(async (hostname: string) =>
+            hostname === "public.example.com"
+                ? [{ address: "93.184.216.34", family: 4 }]
+                : [{ address: "169.254.169.254", family: 4 }],
+        );
+        vi.spyOn(globalThis, "fetch").mockResolvedValueOnce(
+            redirectTo("https://internal.example.com/latest/meta-data/"),
+        );
+
+        await expect(
+            guardedFetch("https://public.example.com/mcp"),
+        ).rejects.toThrow(/blocked network address/);
+    });
+
+    it("drops the Authorization header when the origin changes", async () => {
+        resolvesTo("93.184.216.34");
+        const fetchSpy = vi
+            .spyOn(globalThis, "fetch")
+            .mockResolvedValueOnce(redirectTo("https://other.example.com/x"))
+            .mockResolvedValueOnce(new Response("{}", { status: 200 }));
+
+        await guardedFetch("https://public.example.com/mcp", {
+            headers: { authorization: "Bearer secret", accept: "application/json" },
+        });
+
+        const headers = new Headers(
+            (fetchSpy.mock.calls[1][1] as RequestInit).headers,
+        );
+        expect(headers.get("authorization")).toBeNull();
+        expect(headers.get("accept")).toBe("application/json");
+    });
+
+    it("keeps the Authorization header on a same-origin redirect", async () => {
+        resolvesTo("93.184.216.34");
+        const fetchSpy = vi
+            .spyOn(globalThis, "fetch")
+            .mockResolvedValueOnce(redirectTo("https://public.example.com/x"))
+            .mockResolvedValueOnce(new Response("{}", { status: 200 }));
+
+        await guardedFetch("https://public.example.com/mcp", {
+            headers: { authorization: "Bearer secret" },
+        });
+
+        const headers = new Headers(
+            (fetchSpy.mock.calls[1][1] as RequestInit).headers,
+        );
+        expect(headers.get("authorization")).toBe("Bearer secret");
+    });
+
+    it("does not follow redirects for non-GET requests", async () => {
+        resolvesTo("93.184.216.34");
+        const fetchSpy = vi
+            .spyOn(globalThis, "fetch")
+            .mockResolvedValue(redirectTo("https://public.example.com/x"));
+
+        const res = await guardedFetch("https://public.example.com/mcp", {
+            method: "POST",
+            body: "{}",
+        });
+
+        expect(res.status).toBe(302);
+        expect(fetchSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it("stops after the redirect cap instead of looping forever", async () => {
+        resolvesTo("93.184.216.34");
+        const fetchSpy = vi
+            .spyOn(globalThis, "fetch")
+            .mockResolvedValue(redirectTo("https://public.example.com/loop"));
+
+        const res = await guardedFetch("https://public.example.com/loop");
+
+        expect(res.status).toBe(302);
+        // 1 initial + MAX_MCP_REDIRECTS follows.
+        expect(fetchSpy).toHaveBeenCalledTimes(6);
+    });
+});

backend/src/lib/mcp/client.ts

@@ -371,22 +371,76 @@ const guardedAgent = new Agent({
 // connection to a connect-time-validated address, and refuses to auto-follow
 // redirects (`redirect: "manual"`) so a 3xx to an internal host cannot smuggle
 // egress past the guard.
+// Redirects are followed here rather than by the runtime so that every hop is
+// re-checked by validateRemoteMcpUrl — `redirect: "follow"` would let a public
+// URL bounce us to a private address the guard never saw. Refusing outright is
+// not an option either: RFC 8414 well-known discovery paths are commonly served
+// as redirects, and the MCP SDK treats any non-4xx as fatal, so a single 302
+// aborts discovery even when a later candidate URL would have worked.
+const MAX_MCP_REDIRECTS = 5;
+
 export async function guardedFetch(
     input: Parameters<typeof fetch>[0],
     init?: Parameters<typeof fetch>[1],
 ) {
-    const url =
+    const isRequest = typeof input === "object" && input instanceof Request;
+    let url =
         typeof input === "string"
             ? input
             : input instanceof URL
               ? input.toString()
               : input.url;
     await validateRemoteMcpUrl(url);
-    return fetch(input, {
+    let response = await fetch(input, {
         ...init,
         redirect: "manual",
         dispatcher: guardedAgent,
     } as RequestInit);
+
+    const method = (
+        init?.method ??
+        (isRequest ? input.method : null) ??
+        "GET"
+    ).toUpperCase();
+    // Only bodyless methods are followed. Replaying a POST body across a
+    // redirect is not something any MCP flow needs, and skipping it avoids
+    // having to reason about 307/308 body semantics.
+    if (method !== "GET" && method !== "HEAD") return response;
+
+    const baseHeaders = new Headers(
+        (init?.headers as HeadersInit | undefined) ??
+            (isRequest ? input.headers : undefined),
+    );
+
+    for (let hop = 0; hop < MAX_MCP_REDIRECTS; hop++) {
+        if (response.status < 300 || response.status > 399) return response;
+        const location = response.headers.get("location");
+        if (!location) return response;
+
+        let target: string;
+        try {
+            target = new URL(location, url).toString();
+        } catch {
+            return response;
+        }
+        await response.body?.cancel().catch(() => undefined);
+
+        const validated = await validateRemoteMcpUrl(target);
+        const headers = new Headers(baseHeaders);
+        // Never carry credentials to a different origin.
+        if (new URL(validated).origin !== new URL(url).origin) {
+            headers.delete("authorization");
+        }
+        url = validated;
+        response = await fetch(validated, {
+            ...init,
+            method,
+            headers,
+            redirect: "manual",
+            dispatcher: guardedAgent,
+        } as RequestInit);
+    }
+    return response;
 }
 
 export function base64Url(buffer: Buffer) {