Check where an agent address redirects to, not just where it starts

server/src/agents/connection-test.ts

@@ -1,4 +1,8 @@
-import { checkAgentEndpoint } from "./endpoint";
+import {
+  checkAgentEndpoint,
+  createAgentFetch,
+  EndpointRedirectError,
+} from "./endpoint";
 
 /**
  * Ask an endpoint whether it is really an agent before it is stored.
@@ -97,7 +101,14 @@ export async function testAgentConnection(
   });
   if (!verdict.allowed) return { ok: false, reason: verdict.reason };
 
-  const doFetch = options.fetchImpl ?? fetch;
+  // Wrapped rather than called directly, so the address the request finally lands on is checked too.
+  // Checking only what the person typed leaves the redirect as the way around it.
+  const doFetch = createAgentFetch({
+    ...(options.allowPrivateHosts !== undefined
+      ? { allowPrivateHosts: options.allowPrivateHosts }
+      : {}),
+    ...(options.fetchImpl ? { fetchImpl: options.fetchImpl } : {}),
+  });
   let response: Response;
   try {
     response = await doFetch(verdict.url, {
@@ -111,6 +122,11 @@ export async function testAgentConnection(
       signal: AbortSignal.timeout(options.timeoutMs ?? TEST_TIMEOUT_MS),
     });
   } catch (error) {
+    // A refused redirect is a specific thing that happened, and the person registering can act on it:
+    // it names where their address sent us.
+    if (error instanceof EndpointRedirectError) {
+      return { ok: false, reason: error.message };
+    }
     const timedOut = error instanceof Error && error.name === "TimeoutError";
     return {
       ok: false,

server/src/agents/endpoint.ts

@@ -55,3 +55,79 @@ export function checkAgentEndpoint(
 
   return { allowed: true, url: verdict.url };
 }
+
+/**
+ * How many redirects an agent is allowed before we stop believing it has somewhere to be.
+ *
+ * Three, which covers the ordinary shapes (`http` to `https`, a host rename, a trailing-slash
+ * canonicalisation) and stops a chain that has no end.
+ */
+const MAX_REDIRECTS = 3;
+
+/** A redirect this deployment will not follow, named so the person registering sees which hop. */
+export class EndpointRedirectError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = "EndpointRedirectError";
+  }
+}
+
+const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]);
+
+/**
+ * `fetch`, with the endpoint check applied to every hop rather than only the address a person typed.
+ *
+ * Checking the URL once and then handing it to a fetch that follows redirects is a check with a hole
+ * in it: `https://agent.example.com/ag-ui` passes, answers `307`, and the request lands wherever the
+ * `Location` header says, which is how a registrable agent becomes a way to read the deployment's own
+ * cloud metadata. The address that gets dialled is the one that must be allowed, and a redirect makes
+ * those two different addresses.
+ *
+ * Redirects are followed rather than refused, because a deployment that puts its agent behind one has
+ * done nothing wrong and `http` to `https` is the common case. Each destination goes through
+ * {@link checkAgentEndpoint} first, so following one can only ever reach somewhere registering it
+ * directly would have been allowed to reach.
+ *
+ * The method and body are carried across every hop. A browser turns a redirected `POST` into a `GET`;
+ * doing that here would only ever produce a confusing "that is not an AG-UI endpoint" from an agent
+ * that is one, because AG-UI is a POST protocol and this is a server talking to an API, not a person
+ * following a link.
+ */
+export function createAgentFetch(
+  options: { allowPrivateHosts?: boolean; fetchImpl?: typeof fetch } = {},
+): (url: string, init?: RequestInit) => Promise<Response> {
+  const doFetch = options.fetchImpl ?? fetch;
+
+  return async function guardedFetch(url: string, init?: RequestInit) {
+    let target = url;
+
+    for (let hop = 0; hop <= MAX_REDIRECTS; hop += 1) {
+      // `manual` is what makes this a check rather than a comment: the caller sees the redirect, and
+      // the underlying fetch cannot quietly follow one on its own.
+      const response = await doFetch(target, { ...init, redirect: "manual" });
+      if (!REDIRECT_STATUSES.has(response.status)) return response;
+
+      const location = response.headers.get("location");
+      // A redirect status with nowhere to go is just an answer. Whatever it means, it is the
+      // agent's own reply and not a hop.
+      if (!location) return response;
+
+      const next = new URL(location, target).toString();
+      const verdict = checkAgentEndpoint(next, {
+        ...(options.allowPrivateHosts !== undefined
+          ? { allowPrivateHosts: options.allowPrivateHosts }
+          : {}),
+      });
+      if (!verdict.allowed) {
+        throw new EndpointRedirectError(
+          `That address redirected to ${next}, and ${verdict.reason.charAt(0).toLowerCase()}${verdict.reason.slice(1)}`,
+        );
+      }
+      target = verdict.url;
+    }
+
+    throw new EndpointRedirectError(
+      `That address redirected more than ${MAX_REDIRECTS} times without arriving anywhere.`,
+    );
+  };
+}

server/src/copilot.ts

@@ -7,7 +7,7 @@ import {
 } from "@copilotkit/runtime/v2";
 import { createCopilotHonoHandler } from "@copilotkit/runtime/v2/hono";
 import type { AgentActor } from "./agents/profile-types";
-import type { StallGuard } from "./channels/stall-guard";
+import type { AgentFetch, StallGuard } from "./channels/stall-guard";
 import type { DeploymentConfig } from "./config";
 
 /**
@@ -196,11 +196,19 @@ export function buildAgents(
   apiKey: string | null,
   /** Absent leaves every stream unwatched, which is what an unconfigured timeout means. */
   stallGuard?: StallGuard,
+  /**
+   * The fetch a remote agent is dialled with.
+   *
+   * Absent uses the runtime's own, which follows redirects wherever they point. A deployment passes
+   * one that re-checks each hop, because the address a registration was validated against and the
+   * address a run finally reaches are only the same address while nobody redirects.
+   */
+  agentFetch?: AgentFetch,
 ): Record<string, AbstractAgent> {
   return Object.fromEntries(
     agents.map((agent) => [
       agent.id,
-      buildAgent(agent, model, apiKey, stallGuard),
+      buildAgent(agent, model, apiKey, stallGuard, agentFetch),
     ]),
   );
 }
@@ -210,14 +218,15 @@ function buildAgent(
   model: RuntimeModel,
   apiKey: string | null,
   stallGuard?: StallGuard,
+  agentFetch?: AgentFetch,
 ): AbstractAgent {
   if (agent.type === "built_in") {
     return new BuiltInAgent(builtInAgentConfiguration(agent, model, apiKey));
   }
   if (agent.type === "unavailable") {
     return new UnavailableAgent(agent);
   }
-  return remoteAgentWithStandingRole(agent, stallGuard);
+  return remoteAgentWithStandingRole(agent, stallGuard, agentFetch);
 }
 
 /**
@@ -235,16 +244,26 @@ function buildAgent(
 function remoteAgentWithStandingRole(
   agent: RegisteredRemoteAgent,
   stallGuard?: StallGuard,
+  agentFetch?: AgentFetch,
 ) {
   const remote = new HttpAgent({
     url: agent.endpoint,
     agentId: agent.id,
     // The customer's own key, if their agent sits behind one. `HttpAgentConfig` is
     // `{ url, headers?, fetch? }`, verified against @ag-ui/client 0.0.57.
     ...(agent.headers ? { headers: agent.headers } : {}),
+    // The watch wraps whichever fetch is underneath, so a deployment gets both the stall timeout and
+    // the redirect check rather than having to choose.
     ...(stallGuard
-      ? { fetch: stallGuard.watch({ id: agent.id, name: agent.name }) }
-      : {}),
+      ? {
+          fetch: stallGuard.watch(
+            { id: agent.id, name: agent.name },
+            agentFetch,
+          ),
+        }
+      : agentFetch
+        ? { fetch: agentFetch }
+        : {}),
   });
   remote.use((input, next) =>
     next.run({
@@ -280,6 +299,7 @@ export async function resolveRuntimeAgents(
   model: RuntimeModel,
   resolveModelApiKey: () => Promise<string | null>,
   stallGuard?: StallGuard,
+  agentFetch?: AgentFetch,
 ): Promise<Record<string, AbstractAgent>> {
   const registered = await loadAgents();
   if (registered.length === 0) {
@@ -291,7 +311,7 @@ export async function resolveRuntimeAgents(
   const apiKey = registered.some((agent) => agent.type === "built_in")
     ? await resolveModelApiKey()
     : null;
-  return buildAgents(registered, model, apiKey, stallGuard);
+  return buildAgents(registered, model, apiKey, stallGuard, agentFetch);
 }
 
 /** Who is asking. Agent visibility is decided per person, so a run has to know this first. */
@@ -320,6 +340,7 @@ export function createRequestAgents(
    * that opened it has been answered.
    */
   stallGuard?: StallGuard,
+  agentFetch?: AgentFetch,
 ) {
   return async ({ request }: { request: Request }) => {
     const actor = await identifyActor(request);
@@ -328,6 +349,7 @@ export function createRequestAgents(
       model,
       resolveModelApiKey,
       stallGuard,
+      agentFetch,
     );
   };
 }
@@ -352,6 +374,8 @@ export function mountCopilotRuntime(
    * there is no reason for a caller to have to say `undefined` here to reach `basePath`.
    */
   stallGuard: StallGuard,
+  /** The fetch remote agents are dialled with. See {@link buildAgents}. */
+  agentFetch?: AgentFetch,
   basePath = "/api/copilotkit",
 ) {
   const { intelligence } = config.runtime;
@@ -377,6 +401,7 @@ export function mountCopilotRuntime(
       model,
       resolveModelApiKey,
       stallGuard,
+      agentFetch,
     ) as never,
   });
 

server/src/index.ts

@@ -1,4 +1,5 @@
 import { serve } from "bun";
+import { createAgentFetch } from "./agents/endpoint";
 import { createAgentProfileStore } from "./agents/profile-store";
 import { createRuntimeAgentLoader } from "./agents/runtime-agents";
 import { createApp } from "./app";
@@ -12,9 +13,9 @@ import {
   startChannelActivityListener,
 } from "./channels/events";
 import { createChannelStore } from "./channels/routes";
+import { websocket as channelSocket } from "./channels/socket";
 import { createStallGuard } from "./channels/stall-guard";
 import { createThreadIdentity } from "./channels/thread-identity";
-import { websocket as channelSocket } from "./channels/socket";
 import { createSandboxedStore } from "./components/sandboxed";
 import { createComponentStore } from "./components/store";
 import { createComputerClient } from "./computer/client";
@@ -333,6 +334,13 @@ const app = createApp(
     identifyUser,
     identifyActor,
     stallGuard,
+    // Every run dials the stored endpoint again, so the check that was applied when it was
+    // registered has to be applied to wherever it redirects now.
+    // Absent computer configuration means nothing opted into private hosts, which is the safe
+    // reading and the same one `createApp` takes.
+    createAgentFetch({
+      allowPrivateHosts: config.computer?.allowPrivateHosts === true,
+    }),
   ),
   computerClient,
   // The only path to an acting call.

server/tests/agent-connection-live.test.ts

@@ -60,6 +60,78 @@ describe("registering an agent that really answers", () => {
     expect(result.ok).toBe(false);
   });
 
+  test("a redirect cannot carry the request somewhere the check refuses", async () => {
+    // The check only ever sees the URL a person typed. Following a redirect blindly makes that check
+    // decorative: anything registrable can bounce the server at the metadata endpoint.
+    const bounced: string[] = [];
+    const redirector = Bun.serve({
+      port: 0,
+      fetch: (request) => {
+        bounced.push(request.url);
+        return new Response(null, {
+          status: 307,
+          headers: { location: "http://169.254.169.254/latest/meta-data/" },
+        });
+      },
+    });
+
+    try {
+      const result = await testAgentConnection(
+        `http://127.0.0.1:${redirector.port}/ag-ui`,
+        { allowPrivateHosts: true, timeoutMs: 4_000 },
+      );
+
+      expect(result.ok).toBe(false);
+      if (!result.ok) expect(result.reason).toMatch(/redirect/i);
+      // The first hop is the registered address and is expected. Nothing beyond it should have been
+      // dialled, which is what the refusal is for.
+      expect(bounced.length).toBe(1);
+    } finally {
+      redirector.stop(true);
+    }
+  });
+
+  test("a redirect to an address the check permits is still followed", async () => {
+    // A deployment that puts its agent behind a redirect, http to https being the ordinary case, has
+    // done nothing wrong. Refusing every redirect would break it, so the destination is checked
+    // rather than the hop count.
+    const redirector = Bun.serve({
+      port: 0,
+      fetch: () =>
+        new Response(null, { status: 307, headers: { location: url } }),
+    });
+
+    try {
+      const result = await testAgentConnection(
+        `http://127.0.0.1:${redirector.port}/ag-ui`,
+        { allowPrivateHosts: true, timeoutMs: 8_000 },
+      );
+
+      expect(result.ok).toBe(true);
+      if (result.ok) expect(result.events).toContain("RUN_STARTED");
+    } finally {
+      redirector.stop(true);
+    }
+  });
+
+  test("a redirect that never arrives anywhere gives up rather than looping", async () => {
+    const looper = Bun.serve({
+      port: 0,
+      fetch: (request) =>
+        new Response(null, { status: 307, headers: { location: request.url } }),
+    });
+
+    try {
+      const result = await testAgentConnection(
+        `http://127.0.0.1:${looper.port}/ag-ui`,
+        { allowPrivateHosts: true, timeoutMs: 8_000 },
+      );
+      expect(result.ok).toBe(false);
+    } finally {
+      looper.stop(true);
+    }
+  });
+
   test("a port with nothing on it reports the direction of the connection", async () => {
     // The server dials the agent, so localhost must be tested from the server side.
     const dead = await testAgentConnection("http://127.0.0.1:9/", {