Resolve an agent's host before dialling it

server/src/agents/connection-test.ts

@@ -2,6 +2,7 @@ import {
   checkAgentEndpoint,
   createAgentFetch,
   EndpointNotAllowedError,
+  type ResolveHost,
 } from "./endpoint";
 
 /**
@@ -140,6 +141,9 @@ export async function testAgentConnection(
     headers?: Record<string, string>;
     allowPrivateHosts?: boolean;
     allowedHosts?: ReadonlySet<string>;
+    /** Hosts this deployment's own configuration names. See `checkResolvedAgentHost`. */
+    trustedHosts?: ReadonlySet<string>;
+    resolveHost?: ResolveHost;
     fetchImpl?: typeof fetch;
     timeoutMs?: number;
   } = {},
@@ -159,6 +163,8 @@ export async function testAgentConnection(
     ...(options.allowPrivateHosts !== undefined
       ? { allowPrivateHosts: options.allowPrivateHosts }
       : {}),
+    ...(options.trustedHosts ? { trustedHosts: options.trustedHosts } : {}),
+    ...(options.resolveHost ? { resolveHost: options.resolveHost } : {}),
     ...(options.fetchImpl ? { fetchImpl: options.fetchImpl } : {}),
   });
   let response: Response;

server/src/agents/endpoint.ts

@@ -1,4 +1,6 @@
-import { checkNavigationTarget } from "../computer/target";
+import { lookup } from "node:dns/promises";
+import { isIP } from "node:net";
+import { checkNavigationTarget, isInternalAddress } from "../computer/target";
 
 /**
  * Where an external agent lives, and whether we are willing to talk to it.
@@ -113,6 +115,110 @@ export function checkAgentEndpoint(
   return { allowed: true, url: verdict.url };
 }
 
+/** Every address a name resolves to, which is what a dial will actually reach. */
+export type ResolveHost = (hostname: string) => Promise<string[]>;
+
+const resolveWithDns: ResolveHost = async (hostname) =>
+  (await lookup(hostname, { all: true, verbatim: true })).map(
+    (entry) => entry.address,
+  );
+
+/** The host of an address in the spelling `allowedHosts` uses, with and without its port. */
+function hostKeys(url: URL): string[] {
+  const hostname = url.hostname.toLowerCase();
+  return url.port ? [`${hostname}:${url.port}`, hostname] : [hostname];
+}
+
+/**
+ * The hosts of addresses this deployment's own configuration names, which the resolved check below
+ * does not apply to. An operator pointing the managed Bot or a tenant package's coworker at a
+ * service name on its own network has decided that, and is not somebody registering an address.
+ */
+export function trustedHostsOf(
+  addresses: readonly (string | URL | undefined)[],
+): ReadonlySet<string> {
+  const hosts = new Set<string>();
+  for (const address of addresses) {
+    if (!address) continue;
+    try {
+      for (const key of hostKeys(new URL(address))) hosts.add(key);
+    } catch {
+      // Not a URL: nothing to trust.
+    }
+  }
+  return hosts;
+}
+
+/**
+ * The half of the endpoint check that text cannot do: where the name actually points.
+ *
+ * `checkAgentEndpoint` reads the address as written, so `https://agent.example.com` passes however
+ * its DNS answers, including with 10.0.0.5 or the metadata address, and a bare service name such as
+ * `http://supervisor:4300` passes because it is neither an IP nor `localhost`. This server then
+ * POSTs to it with headers the person chose. So before a dial the name is resolved and every address
+ * it gives must be one the textual check would have allowed, and a single-label name is refused
+ * outright: nothing public is reached that way, and inside a deployment almost everything is.
+ *
+ * Skipped where the floor is down (`allowPrivateHosts`), for a host the deployment named
+ * (`allowedHosts`) or configured itself (`trustedHosts`), and for an IP literal, which the textual
+ * check has already judged. Resolution failing is a refusal: a name that does not resolve is not
+ * somewhere a run can go, and answering otherwise would turn a resolver hiccup into a dial.
+ *
+ * What this does not close is a name that answers differently between this lookup and the dial
+ * (DNS rebinding). Pinning the connection to the checked address needs a dialler of its own; a
+ * network policy around the server is the boundary that holds regardless.
+ */
+export async function checkResolvedAgentHost(
+  raw: string,
+  options: {
+    allowPrivateHosts?: boolean;
+    allowedHosts?: ReadonlySet<string>;
+    trustedHosts?: ReadonlySet<string>;
+    resolveHost?: ResolveHost;
+  } = {},
+): Promise<EndpointVerdict> {
+  let url: URL;
+  try {
+    url = new URL(raw);
+  } catch {
+    return { allowed: false, reason: "An agent needs a web address." };
+  }
+  if (options.allowPrivateHosts) return { allowed: true, url: url.toString() };
+  const keys = hostKeys(url);
+  if (
+    keys.some(
+      (key) => options.allowedHosts?.has(key) || options.trustedHosts?.has(key),
+    )
+  ) {
+    return { allowed: true, url: url.toString() };
+  }
+  const hostname = url.hostname.replace(/^\[|\]$/g, "").replace(/\.+$/, "");
+  if (isIP(hostname)) return { allowed: true, url: url.toString() };
+  if (!hostname.includes(".")) {
+    return {
+      allowed: false,
+      reason:
+        "That is a name inside OpenBot's own network, so an agent may not live there.",
+    };
+  }
+  let addresses: string[];
+  try {
+    addresses = await (options.resolveHost ?? resolveWithDns)(hostname);
+  } catch {
+    return {
+      allowed: false,
+      reason: `${hostname} could not be resolved, so an agent cannot live there.`,
+    };
+  }
+  if (addresses.length === 0 || addresses.some(isInternalAddress)) {
+    return {
+      allowed: false,
+      reason: `${hostname} points inside OpenBot's own network, so an agent may not live there.`,
+    };
+  }
+  return { allowed: true, url: url.toString() };
+}
+
 /**
  * How many redirects an agent is allowedbefore we stop believing it has somewhere to be.
  *
@@ -258,6 +364,16 @@ export function createAgentFetch(
      * not be a way to arrive somewhere registration would have declined.
      */
     allowedHosts?: ReadonlySet<string>;
+    /**
+     * Hosts of addresses this deployment's own configuration names (the managed Bot, a tenant
+     * package's coworkers), exempt from the resolved check. See {@link checkResolvedAgentHost}.
+     */
+    trustedHosts?: ReadonlySet<string>;
+    /**
+     * How a name is resolved before it is dialled. DNS by default. A caller that substitutes
+     * `fetchImpl` substitutes the network, so with no resolver of its own nothing is resolved.
+     */
+    resolveHost?: ResolveHost;
     fetchImpl?: typeof fetch;
     /**
      * Told about every address this refused to dial, and why.
@@ -295,6 +411,24 @@ export function createAgentFetch(
       ...(options.allowedHosts ? { allowedHosts: options.allowedHosts } : {}),
     });
 
+  const resolveHost =
+    options.resolveHost ?? (options.fetchImpl ? undefined : resolveWithDns);
+  const resolved = (address: string): Promise<EndpointVerdict> =>
+    resolveHost
+      ? checkResolvedAgentHost(address, {
+          ...(options.allowPrivateHosts !== undefined
+            ? { allowPrivateHosts: options.allowPrivateHosts }
+            : {}),
+          ...(options.allowedHosts
+            ? { allowedHosts: options.allowedHosts }
+            : {}),
+          ...(options.trustedHosts
+            ? { trustedHosts: options.trustedHosts }
+            : {}),
+          resolveHost,
+        })
+      : Promise.resolve({ allowed: true, url: address });
+
   return async function guardedFetch(url: string, init?: RequestInit) {
     const stored = check(url);
     if (!stored.allowed) {
@@ -303,6 +437,13 @@ export function createAgentFetch(
         `${NOT_DIALLED}${url}: ${stored.reason.charAt(0).toLowerCase()}${stored.reason.slice(1)}`,
       );
     }
+    const storedResolved = await resolved(stored.url);
+    if (!storedResolved.allowed) {
+      throw refuse(
+        url,
+        `${NOT_DIALLED}${url}: ${storedResolved.reason.charAt(0).toLowerCase()}${storedResolved.reason.slice(1)}`,
+      );
+    }
 
     const origin = stored.url;
     let target = stored.url;
@@ -330,6 +471,13 @@ export function createAgentFetch(
           `That address redirected to ${next}, and ${verdict.reason.charAt(0).toLowerCase()}${verdict.reason.slice(1)}`,
         );
       }
+      const hopResolved = await resolved(verdict.url);
+      if (!hopResolved.allowed) {
+        throw refuse(
+          next,
+          `That address redirected to ${next}, and ${hopResolved.reason.charAt(0).toLowerCase()}${hopResolved.reason.slice(1)}`,
+        );
+      }
       if (!sameCredentialScope(origin, verdict.url)) {
         // A body this cannot strip refuses the hop rather than forwarding it, and that refusal is
         // worth the same row as any other: it means a run was carrying something unreadable to a

server/src/agents/routes.ts

@@ -6,7 +6,7 @@ import type { AppVariables } from "../auth/guards";
 import { afterExposureChange } from "../plugins/exposure-change";
 import type { SharedUseStore } from "../plugins/shared-use-store";
 import { testAgentConnection } from "./connection-test";
-import { checkAgentEndpoint } from "./endpoint";
+import { checkAgentEndpoint, trustedHostsOf } from "./endpoint";
 import { isolationState, mayIsolate } from "./isolation";
 import { isManagedEndpoint } from "./managed-endpoint";
 import { canManageAgent } from "./profile-policy";
@@ -524,6 +524,11 @@ export function createAgentRoutes(
       ...(Object.keys(headers).length > 0 ? { headers } : {}),
       allowPrivateHosts,
       allowedHosts,
+      // The deployment's own Bot and harness live at names on its own network, and are not an
+      // address somebody is registering.
+      ...(managedRun
+        ? { trustedHosts: trustedHostsOf(managedRun.endpoints) }
+        : {}),
     });
     // 200 either way: the request succeeded, and the verdict is the payload. A failed connection test
     // is an answer, not an error, and a 4xx here would have the surface render it as a broken button.

server/src/computer/target.ts

@@ -72,6 +72,41 @@ function isLocalhostName(hostname: string): boolean {
   return hostname === "localhost" || hostname.endsWith(".localhost");
 }
 
+/**
+ * A name with no dot in it: `postgres`, `supervisor`, `agent-computer`.
+ *
+ * Nothing on the public internet is reached by a single label. Inside a deployment almost everything
+ * is: Docker Compose and Kubernetes name each service this way, so `http://supervisor:4300` is this
+ * deployment's own Docker-socket holder however ordinary it looks as text. IP literals are excluded
+ * (an IPv4 address has dots, an IPv6 one colons) because they have their own checks above.
+ */
+function isSingleLabelHostname(hostname: string): boolean {
+  return hostname !== "" && !hostname.includes(".") && !hostname.includes(":");
+}
+
+/**
+ * Whether an address a name resolved to is one a public check would refuse.
+ *
+ * For callers that resolve before they dial (agents/endpoint.ts): the textual check above cannot see
+ * that `agent.example.com` points at 10.0.0.5, and the resolved address is the one that is dialled.
+ */
+export function isInternalAddress(address: string): boolean {
+  // An IPv4-mapped address with a dotted tail is the IPv4 address it carries.
+  const mapped = /^::ffff:(\d+\.\d+\.\d+\.\d+)$/i.exec(address);
+  if (mapped?.[1]) return isInternalAddress(mapped[1]);
+  const host = canonicalHostname(
+    address.includes(":") && !address.startsWith("[")
+      ? `[${address.toLowerCase()}]`
+      : address.toLowerCase(),
+  );
+  return (
+    isNeverAllowedHostname(host) ||
+    INTERNAL_HOSTNAMES.has(host) ||
+    isPrivateIpv4(host) ||
+    isPrivateIpv6(host)
+  );
+}
+
 export type TargetVerdict =
   | { allowed: true; url: string }
   | { allowed: false; reason: string };
@@ -186,7 +221,12 @@ function isPrivateIpv4(hostname: string): boolean {
     return false;
   }
   const [a, b] = octets as [number, number, number, number];
+  // "This network": 0.0.0.0 reaches the local host on Linux, and the rest of 0/8 is never a server.
+  if (a === 0) return true;
   if (a === 10) return true;
+  // Shared address space (RFC 6598). Carrier-grade NAT, and the range cloud providers use for
+  // internal services, so it is no more a public address than 10/8 is.
+  if (a === 100 && b >= 64 && b <= 127) return true;
   if (a === 127) return true;
   if (a === 169 && b === 254) return true; // link-local, includes metadata
   if (a === 172 && b >= 16 && b <= 31) return true;
@@ -284,6 +324,7 @@ export function checkNavigationTarget(
   if (
     INTERNAL_HOSTNAMES.has(hostname) ||
     isLocalhostName(hostname) ||
+    isSingleLabelHostname(hostname) ||
     isPrivateIpv4(hostname) ||
     isPrivateIpv6(hostname)
   ) {

server/src/index.ts

@@ -31,7 +31,7 @@ import {
 } from "./agents/callback-token";
 import { canUseComputer } from "./agents/computer-access";
 import { wireCoordination } from "./agents/coordination-wiring";
-import { createAgentFetch } from "./agents/endpoint";
+import { createAgentFetch, trustedHostsOf } from "./agents/endpoint";
 import { createHandoffDesk } from "./agents/handoff";
 import { createHandoffRunner } from "./agents/handoff-runner";
 import { startHandoffDelivery } from "./agents/handoff-wiring";
@@ -1678,6 +1678,18 @@ const agentFetch = createAgentFetch({
   allowPrivateHosts: config.computer?.allowPrivateHosts === true,
   // Named addresses are reachable on every hop, not only the one that was registered.
   allowedHosts: config.agentEndpointAllowedHosts,
+  // Addresses this deployment's own configuration points at: the managed Bot, a harness picked during
+  // setup, and a tenant package's remote coworkers. Exempt from resolving where a name points,
+  // because they are the operator's decision rather than an address somebody registered.
+  trustedHosts: trustedHostsOf([
+    config.managedAgent?.endpoint,
+    config.managedAgent?.alsoRun,
+    ...tenantPackage.agents.map((agent) =>
+      typeof agent.configuration.endpoint === "string"
+        ? agent.configuration.endpoint
+        : undefined,
+    ),
+  ]),
   // The refusal is what the run already knows; this is what the deployment knows. Written here
   // rather than in `endpoint.ts` so that file keeps deciding and nothing else, the way the
   // target check it reuses does.