Check the address a supervisor hands back before calling it

server/src/computer/client.ts

@@ -22,7 +22,7 @@ import type {
   WriteFileInput,
   WriteFileResult,
 } from "./schema";
-import { checkNavigationTarget } from "./target";
+import { checkComputerAddress, checkNavigationTarget } from "./target";
 
 /**
  * How the server talks to a Bot's computer.
@@ -164,10 +164,24 @@ export function createComputerClient(options: ComputerClientOptions) {
       // Outside the try below on purpose: that catch reports "the computer is not running", which is
       // true of a computer that will not answer and misleading about a supervisor that could not be
       // reached or refused. Those are different operator-facing problems.
-      const target =
-        botId && options.resolveBaseUrl
-          ? (await options.resolveBaseUrl(botId)).replace(/\/$/, "")
-          : base;
+      let target: string;
+      if (botId && options.resolveBaseUrl) {
+        const located = (await options.resolveBaseUrl(botId)).replace(
+          /\/$/,
+          "",
+        );
+        // Checked because this address is not necessarily ours. A hosted provider answers from its
+        // own API, and whatever comes back is about to be called with this deployment's computer
+        // token. Our own supervisor answers with a private address, which is fine and why this is
+        // not the navigation check.
+        const verdict = checkComputerAddress(located);
+        if (!verdict.allowed) {
+          throw new ComputerUnavailableError(verdict.reason);
+        }
+        target = located;
+      } else {
+        target = base;
+      }
 
       // Already stopped before this left: do not dispatch at all. Relying on fetch to reject an
       // aborted signal makes "did the click happen" depend on how quickly the runtime notices, and

server/src/computer/target.ts

@@ -56,6 +56,50 @@ function isPrivateIpv4(hostname: string): boolean {
   return false;
 }
 
+/**
+ * Decide whether an address a supervisor handed back may be called at all.
+ *
+ * Deliberately not {@link checkNavigationTarget}. That one judges where a Bot may browse, and its
+ * private-host rule is exactly wrong here: our own supervisor answers with `http://127.0.0.1:<port>`
+ * for a container on this machine, so applying it would refuse the normal case.
+ *
+ * What survives is what holds however the address was produced. The scheme must be one we speak, and
+ * the cloud metadata addresses are refused whatever anything says, because that is how a container's
+ * credentials leave it and no supervisor has a reason to name one.
+ *
+ * This matters because the address stops being ours. With a hosted provider (A10) it arrives from a
+ * third party's API and goes straight into `fetch` carrying this deployment's computer token, so it
+ * is worth one check that it is an address rather than a surprise.
+ */
+export function checkComputerAddress(raw: string): TargetVerdict {
+  let url: URL;
+  try {
+    url = new URL(raw);
+  } catch {
+    return {
+      allowed: false,
+      reason: `The computer's address is not a URL: ${raw}`,
+    };
+  }
+
+  if (!ALLOWED_PROTOCOLS.has(url.protocol)) {
+    return {
+      allowed: false,
+      reason: `A computer must be reached over http or https, not ${url.protocol.replace(":", "")}.`,
+    };
+  }
+
+  if (NEVER_ALLOWED_HOSTNAMES.has(url.hostname.toLowerCase())) {
+    return {
+      allowed: false,
+      reason:
+        "That address holds this deployment's own cloud credentials, so it is never called as a computer.",
+    };
+  }
+
+  return { allowed: true, url: url.toString() };
+}
+
 /**
  * Decide whether a Bot may navigate here.
  *

server/tests/computer-target.test.ts

@@ -1,5 +1,8 @@
 import { describe, expect, test } from "bun:test";
-import { checkNavigationTarget } from "../src/computer/target";
+import {
+  checkComputerAddress,
+  checkNavigationTarget,
+} from "../src/computer/target";
 
 describe("navigation targets", () => {
   test("allows an ordinary public address", () => {
@@ -77,3 +80,51 @@ describe("navigation targets", () => {
     expect(checkNavigationTarget("http://172.31.255.255/").allowed).toBe(false);
   });
 });
+
+/**
+ * The address a supervisor hands back, before anything is called on it.
+ *
+ * Not the navigation check. Our own supervisor answers with a loopback address for a container on
+ * this machine, so refusing private hosts here would refuse the ordinary case. What is worth
+ * checking is that the thing is an address at all, and that it is not the one place a token must
+ * never be sent, because with a hosted provider it arrives from somebody else's API.
+ */
+describe("checkComputerAddress", () => {
+  test("allows the private address our own supervisor returns", () => {
+    expect(checkComputerAddress("http://127.0.0.1:49213")).toEqual({
+      allowed: true,
+      url: "http://127.0.0.1:49213/",
+    });
+  });
+
+  test("allows a hosted provider's public address", () => {
+    const verdict = checkComputerAddress("https://sandbox-abc123.daytona.app");
+    expect(verdict.allowed).toBe(true);
+  });
+
+  test.each(["169.254.169.254", "metadata.google.internal", "metadata.goog"])(
+    "refuses the cloud metadata address %s however it arrived",
+    (host) => {
+      const verdict = checkComputerAddress(`http://${host}/latest/meta-data/`);
+      expect(verdict.allowed).toBe(false);
+      if (!verdict.allowed) {
+        expect(verdict.reason).toContain("cloud credentials");
+      }
+    },
+  );
+
+  test.each(["file:///etc/passwd", "ftp://example.com", "gopher://x"])(
+    "refuses %s, which is not a scheme a computer speaks",
+    (raw) => {
+      expect(checkComputerAddress(raw).allowed).toBe(false);
+    },
+  );
+
+  test("refuses something that is not a URL", () => {
+    const verdict = checkComputerAddress("not-an-address");
+    expect(verdict.allowed).toBe(false);
+    if (!verdict.allowed) {
+      expect(verdict.reason).toContain("not a URL");
+    }
+  });
+});