Harden runtime env, tokens, control text and model content

agent-computer/src/authorisation.ts

@@ -31,11 +31,15 @@ export function matchesToken(expected: string, offered: string): boolean {
  * parameter.
  */
 export function offeredToken(headers: Headers, url: URL): string {
-  if (url.pathname === "/stream") return url.searchParams.get("token") ?? "";
+  // The header path trims; the query path must too, or `?token=%20SECRET` 401s while the same
+  // value in a header succeeds and the failure looks stream-specific.
+  if (url.pathname === "/stream")
+    return url.searchParams.get("token")?.trim() ?? "";
   const header = headers.get("x-openbot-computer-token")?.trim();
   if (header) return header;
   const authorization = headers.get("authorization")?.trim() ?? "";
-  return authorization.replace(/^Bearer /i, "");
+  // The remainder needs trimming too: `Bearer   SECRET  ` left leading spaces behind.
+  return authorization.replace(/^Bearer /i, "").trim();
 }
 
 /**

agent-computer/src/control.ts

@@ -186,9 +186,11 @@ export function createControl(
         ...state,
         requested: true,
         requestedAt: now(),
+        // Polled ~1Hz by every viewer for HELP_REQUEST_TTL_MS: a model-generated megabyte reason
+        // would be retained and re-served the whole time. Capped like form fields are.
         reason:
           typeof reason === "string" && reason.trim()
-            ? reason.trim()
+            ? reason.trim().slice(0, 500)
             : "The assistant needs a person to continue.",
       };
       return this.get();
@@ -210,7 +212,7 @@ export function createControl(
         ...state,
         secretWanted:
           typeof input.label === "string" && input.label.trim()
-            ? input.label.trim()
+            ? input.label.trim().slice(0, 500)
             : "the value this page is asking for",
         secretRef: input.ref.trim(),
         secretSnapshotId:

agent-computer/src/index.ts

@@ -204,7 +204,9 @@ function botIdOf(request: Request, fallback?: string | null): string {
  * would only add a syscall to every call. Everything about why confinement is harder than it looks
  * lives in workspace.ts.
  */
-const workspace = createWorkspace(process.env.WORKSPACE_DIR ?? "/workspace");
+const workspace = createWorkspace(
+  process.env.WORKSPACE_DIR?.trim() || "/workspace",
+);
 
 /**
  * Who has the wheel, as a state machine in its own module.
@@ -232,12 +234,12 @@ const workspace = createWorkspace(process.env.WORKSPACE_DIR ?? "/workspace");
  * meant to shrink.
  */
 const profiles = createProfiles(
-  process.env.PROFILES_DIR ?? "/profiles",
+  process.env.PROFILES_DIR?.trim() || "/profiles",
   (botId) => sessions.get(botId)?.viewer.releaseAll(COMPUTER_STOPPED),
 );
 // Rooted in the same workspace the file tools use, so a command and a written file see one
 // directory rather than two.
-const shell = createShell(process.env.WORKSPACE_DIR ?? "/workspace");
+const shell = createShell(process.env.WORKSPACE_DIR?.trim() || "/workspace");
 
 /**
  * The id normally arrives as a header on every request. This is the fallback for a caller that has no
@@ -515,6 +517,9 @@ serve<StreamData>({
         }
         message = validated.message;
       } catch {
+        // The validated-but-wrong branch above sends an error frame; unparseable input used to
+        // be dropped silently, so a buggy surface saw input "ignored" with no diagnostic.
+        ws.send(JSON.stringify({ type: "error", error: "Input is not JSON." }));
         return;
       }
       // A person's input is accepted only while they hold the wheel. The socket being open is not permission:

agent-computer/tests/runtime-hardening.test.ts

@@ -0,0 +1,45 @@
+import { describe, expect, test } from "bun:test";
+import { offeredToken } from "../src/authorisation";
+import { createControl } from "../src/control";
+
+const SECRET = "a-long-development-secret";
+const url = (path: string) => new URL(`http://computer.test${path}`);
+
+/**
+ * The header path trimmed while `/stream` did not, so `?token=%20SECRET` 401d while the same
+ * value in a header succeeded. `Bearer   SECRET  ` left leading spaces behind the regex.
+ */
+describe("offered token trimming", () => {
+  test("trims a padded stream query token", () => {
+    const headers = new Headers();
+    expect(
+      offeredToken(
+        headers,
+        url(`/stream?token=${encodeURIComponent(`  ${SECRET}  `)}`),
+      ),
+    ).toBe(SECRET);
+  });
+
+  test("trims the bearer remainder", () => {
+    const headers = new Headers({ authorization: `Bearer   ${SECRET}  ` });
+    expect(offeredToken(headers, url("/snapshot"))).toBe(SECRET);
+  });
+});
+
+/**
+ * `reason` and `label` are stored and polled ~1Hz by every viewer for the request TTL. A
+ * model-generated megabyte string would be retained and re-served the whole time; capped at 500.
+ */
+describe("control reason/label caps", () => {
+  test("caps a long help reason at 500 characters", () => {
+    const control = createControl();
+    const state = control.requestHelp("r".repeat(2000));
+    expect(state.reason).toHaveLength(500);
+  });
+
+  test("caps a long secret label at 500 characters", () => {
+    const control = createControl();
+    const state = control.requestSecret({ label: "l".repeat(2000), ref: "e1" });
+    expect(state.secretWanted).toHaveLength(500);
+  });
+});

shared/user-content-hardening.test.ts

@@ -0,0 +1,65 @@
+import { describe, expect, test } from "bun:test";
+import { userContent } from "./user-content";
+
+const png = Buffer.from([137, 80, 78, 71]).toString("base64");
+
+/**
+ * The mime type reaches a `data:` URL sent to model providers with no allowlist, and the value
+ * with no shape check. `text/html`, smuggling whitespace, and empty values now degrade to a
+ * named part instead of a provider payload.
+ */
+describe("userContent image hardening", () => {
+  test("passes an allowlisted png through", () => {
+    expect(
+      userContent([
+        {
+          type: "image",
+          source: { type: "data", value: png, mimeType: "image/png" },
+        },
+      ]),
+    ).toEqual([
+      { type: "image_url", image_url: { url: `data:image/png;base64,${png}` } },
+    ]);
+  });
+
+  test.each([["text/html"], ["application/javascript"], ["text/plain"]])(
+    "names a %s attachment instead of sending it",
+    (mimeType) => {
+      expect(
+        userContent([
+          {
+            type: "image",
+            source: { type: "data", value: png, mimeType },
+          },
+        ]),
+      ).toEqual([{ type: "text", text: "[image]" }]);
+    },
+  );
+
+  test("names an empty value instead of sending it", () => {
+    expect(
+      userContent([
+        {
+          type: "image",
+          source: { type: "data", value: "   ", mimeType: "image/png" },
+        },
+      ]),
+    ).toEqual([{ type: "text", text: "[image]" }]);
+  });
+
+  test("normalises a padded, upper-case mime type", () => {
+    expect(
+      userContent([
+        {
+          type: "image",
+          source: { type: "data", value: png, mimeType: "  IMAGE/JPEG  " },
+        },
+      ]),
+    ).toEqual([
+      {
+        type: "image_url",
+        image_url: { url: `data:image/jpeg;base64,${png}` },
+      },
+    ]);
+  });
+});

shared/user-content.ts

@@ -33,15 +33,25 @@ export function userContent(content: unknown): string | UserContentPart[] {
       return { type: "text", text: item.text };
     }
     const source = item.source;
+    // The mime type reaches a `data:` URL sent to model providers. Allowlisted to images and
+    // base64-shape-checked, so `text/html` (or whitespace/quotes/CRLF smuggling) and empty values
+    // degrade to a named part instead of a provider payload.
     if (
       item.type === "image" &&
       source?.type === "data" &&
       typeof source.value === "string" &&
-      typeof source.mimeType === "string"
+      source.value.trim() &&
+      /^[A-Za-z0-9+/]*={0,2}$/.test(source.value.replace(/\s/g, "")) &&
+      typeof source.mimeType === "string" &&
+      ["image/png", "image/jpeg", "image/gif", "image/webp"].includes(
+        source.mimeType.trim().toLowerCase(),
+      )
     ) {
       return {
         type: "image_url",
-        image_url: { url: `data:${source.mimeType};base64,${source.value}` },
+        image_url: {
+          url: `data:${source.mimeType.trim().toLowerCase()};base64,${source.value}`,
+        },
       };
     }
     const name =

supervisor/src/environment.ts

@@ -12,9 +12,22 @@ export function environmentFor(
   const passthrough = Object.entries(env).filter(([key]) =>
     key.startsWith("EGRESS_PROXY"),
   );
-  const computerToken = env.COMPUTER_TOKEN;
+  const computerToken = env.COMPUTER_TOKEN?.trim() || undefined;
   const spireSocketVolume = env.SPIRE_AGENT_SOCKET_VOLUME;
-  const browserMode = env.COMPUTER_BROWSER_MODE;
+  // Fail fast here rather than forwarding an invalid mode that crashes the child at
+  // `browserModeFromEnv`: whitespace-only is falsy after trim, anything else must be headless
+  // or headed.
+  const rawBrowserMode = env.COMPUTER_BROWSER_MODE?.trim() || undefined;
+  if (
+    rawBrowserMode !== undefined &&
+    rawBrowserMode !== "headless" &&
+    rawBrowserMode !== "headed"
+  ) {
+    throw new Error(
+      `COMPUTER_BROWSER_MODE must be headless or headed, not ${JSON.stringify(env.COMPUTER_BROWSER_MODE)}.`,
+    );
+  }
+  const browserMode = rawBrowserMode;
   return [
     `COMPUTER_BOT_ID=${botId}`,
     ...(computerToken ? [`COMPUTER_TOKEN=${computerToken}`] : []),

supervisor/src/index.ts

@@ -59,16 +59,18 @@ if (!token) {
   );
   process.exit(1);
 }
-const image = process.env.COMPUTER_IMAGE ?? "openbot-agent-computer:latest";
-const network = process.env.COMPUTER_NETWORK;
-const runtime = process.env.COMPUTER_RUNTIME;
+const image =
+  process.env.COMPUTER_IMAGE?.trim() || "openbot-agent-computer:latest";
+const network = process.env.COMPUTER_NETWORK?.trim() || undefined;
+const runtime = process.env.COMPUTER_RUNTIME?.trim() || undefined;
 const resolvedMemory = computerMemoryBytes(process.env.COMPUTER_MEMORY_BYTES);
 if (!resolvedMemory.ok) {
   console.error(resolvedMemory.reason);
   process.exit(1);
 }
 const memoryBytes = resolvedMemory.bytes;
-const spireSocketVolume = process.env.SPIRE_AGENT_SOCKET_VOLUME;
+const spireSocketVolume =
+  process.env.SPIRE_AGENT_SOCKET_VOLUME?.trim() || undefined;
 
 const app = new Hono();
 

supervisor/tests/environment-hardening.test.ts

@@ -0,0 +1,44 @@
+import { describe, expect, test } from "bun:test";
+import { environmentFor } from "../src/environment";
+
+/**
+ * Whitespace-only `COMPUTER_TOKEN` used to be forwarded verbatim while the child trims and then
+ * exits, a boot crash loop from a value the supervisor accepted. Invalid `COMPUTER_BROWSER_MODE`
+ * was likewise forwarded to crash the child instead of failing fast here.
+ */
+describe("supervisor environment hardening", () => {
+  test("omits a whitespace-only computer token", () => {
+    expect(
+      environmentFor("bot-1", {
+        COMPUTER_TOKEN: "   ",
+        COMPUTER_BROWSER_MODE: "headless",
+      }),
+    ).toEqual(["COMPUTER_BOT_ID=bot-1", "COMPUTER_BROWSER_MODE=headless"]);
+  });
+
+  test("trims a padded token", () => {
+    expect(environmentFor("bot-1", { COMPUTER_TOKEN: "  secret  " })).toContain(
+      "COMPUTER_TOKEN=secret",
+    );
+  });
+
+  test("refuses an invalid browser mode instead of forwarding it", () => {
+    expect(() =>
+      environmentFor("bot-1", {
+        COMPUTER_TOKEN: "s",
+        COMPUTER_BROWSER_MODE: "fullscreen",
+      }),
+    ).toThrow(/headless or headed/);
+  });
+
+  test("accepts both valid modes", () => {
+    for (const mode of ["headless", "headed"]) {
+      expect(
+        environmentFor("bot-1", {
+          COMPUTER_TOKEN: "s",
+          COMPUTER_BROWSER_MODE: mode,
+        }),
+      ).toContain(`COMPUTER_BROWSER_MODE=${mode}`);
+    }
+  });
+});