Take the kind of human input from the path, not the body

server/src/computer/gateway.ts

@@ -623,6 +623,21 @@ export function createComputerGateway(
       input: HumanInput,
     ): Promise<HumanInputResult> {
       const { kind, ...payload } = input;
+      /*
+       * Checked here as well as at the route, because this is where it becomes a path.
+       *
+       * `kind` is typed as one of four gestures and a type is not a check: the route casts a parsed
+       * body to this shape, so whatever arrived is whatever the caller sent. Interpolated into the
+       * path below, a value like `../exec` reaches a different endpoint of the computer's API
+       * altogether, carrying this deployment's computer token. This method is also the one acting
+       * path that writes no audit row, deliberately, so a call that went somewhere else leaves
+       * nothing behind that would say so.
+       */
+      if (!HUMAN_GESTURES.has(kind)) {
+        throw new Error(
+          `A person's input is one of ${[...HUMAN_GESTURES].join(", ")}, not ${JSON.stringify(kind)}.`,
+        );
+      }
       return post<HumanInputResult>(botId, `/human/${kind}`, payload);
     },
 
@@ -824,6 +839,15 @@ function describeFile(path: string): {
  */
 const ACTIVATING_KEYS = new Set(["Enter", "NumpadEnter", "Space", " "]);
 
+/**
+ * What a person's mouse and keyboard produce, and the whole of what `/human/<kind>` may name.
+ *
+ * A set rather than the union type alone, because the type is erased before the value gets here: the
+ * route parses a JSON body and casts it to the input shape, so the check has to exist at runtime on
+ * the side that builds the path.
+ */
+const HUMAN_GESTURES = new Set(["click", "type", "key", "scroll"]);
+
 function intentOf(
   toolName: string,
   key: string | undefined,

server/src/computer/routes.ts

@@ -324,8 +324,11 @@ export function createComputerRoutes(
     try {
       return context.json(
         await gateway.humanInput(context.req.param("botId"), {
-          kind,
           ...(body ?? {}),
+          // Last, so the checked value wins. Spread over it, a body carrying its own `kind` replaced
+          // the one this route had just checked, and the gateway puts that value into the path it
+          // calls on the computer.
+          kind,
         } as Parameters<typeof gateway.humanInput>[1]),
       );
     } catch (error) {

server/tests/computer-gateway.test.ts

@@ -163,6 +163,9 @@ function fakeComputer(options?: {
         calls.push("supplySecret");
         return Response.json({ supplied: true });
       case "/human/click":
+      case "/human/type":
+      case "/human/key":
+      case "/human/scroll":
       case "/human/move":
       case "/human/button":
       case "/human/wheel":
@@ -743,6 +746,48 @@ describe("the computer gateway", () => {
   });
 });
 
+/**
+ * The gateway builds the computer path from `kind`, so `kind` cannot be a caller's string.
+ *
+ * The route ahead of it checks the four gestures, and this is the layer that holds when something
+ * gets past that: a value reaching here decides which endpoint of the computer's API the deployment
+ * calls, with its computer token attached. `humanInput` is also the one acting method that writes no
+ * audit row, by design, so a redirected call leaves nothing behind to read afterwards.
+ */
+describe("human input names a gesture, not a path", () => {
+  test.each([
+    ["../computers/reset", "another endpoint of the computer's API"],
+    ["../exec", "the shell"],
+    ["click/../../health", "a traversal in the middle"],
+    ["", "nothing at all"],
+  ])("refuses %s (%s), and sends nothing", async (kind) => {
+    const { gateway, requests } = await gatewayWith(PERMISSIVE);
+    const before = requests.length;
+
+    await expect(
+      gateway.humanInput("bot-1", { kind, x: 1, y: 1 } as never),
+    ).rejects.toThrow();
+    // Nothing left this process. Asserting only that it threw would pass just as well when the
+    // request went out and the far side answered 404, which is the failure being fixed.
+    expect(requests.slice(before)).toEqual([]);
+  });
+
+  test.each(["click", "type", "key", "scroll"])(
+    "carries %s through, because that is what a person's hands do",
+    async (kind) => {
+      const { gateway, requests } = await gatewayWith(PERMISSIVE);
+
+      await gateway.humanInput("bot-1", { kind, x: 1, y: 1 } as never);
+
+      expect(
+        requests.some(
+          (request) => new URL(request.url).pathname === `/human/${kind}`,
+        ),
+      ).toBe(true);
+    },
+  );
+});
+
 describe("resolving a computer's address", () => {
   test("a foreign address is refused, so nothing is sent to it", async () => {
     /*

server/tests/computer-routes.test.ts

@@ -132,3 +132,84 @@ describe("computer fleet listing", () => {
     expect(listed()).toBe(1);
   });
 });
+
+/**
+ * The kind of human input is what the path says, and only what the path says.
+ *
+ * The route checks `:kind` against the four gestures a person's mouse and keyboard produce, and then
+ * built the call as `{ kind, ...body }`, so a body carrying its own `kind` replaced the value that
+ * had just been checked. The gateway puts that value straight into the path it calls on the
+ * computer, so the check decided one thing and the request went somewhere else.
+ *
+ * This route is the one that deliberately skips the policy decision and the audit row, because a
+ * takeover exists so a person can type the thing nothing should keep. That makes it the worst one to
+ * be able to redirect: nothing downstream writes the row that would have shown where it went.
+ */
+describe("human input", () => {
+  function recordingGateway() {
+    const calls: Array<{ botId: string; input: Record<string, unknown> }> = [];
+    const gateway = {
+      humanInput: async (botId: string, input: Record<string, unknown>) => {
+        calls.push({ botId, input });
+        return { ok: true };
+      },
+    } as unknown as ComputerGateway;
+    return {
+      calls,
+      app: createComputerRoutes(
+        gateway,
+        {} as PolicyStore,
+        asActor(member),
+        async () => true,
+      ),
+    };
+  }
+
+  async function send(body: unknown, kind = "click") {
+    const { app, calls } = recordingGateway();
+    const response = await app.request(
+      `http://openbot.test/bot-1/human/${kind}`,
+      {
+        method: "POST",
+        headers: { "content-type": "application/json" },
+        body: JSON.stringify(body),
+      },
+    );
+    return { response, calls };
+  }
+
+  test("carries a real gesture through with its coordinates", async () => {
+    const { response, calls } = await send({ x: 10, y: 20 });
+
+    expect(response.status).toBe(200);
+    expect(calls).toHaveLength(1);
+    expect(calls[0]?.input.kind).toBe("click");
+    expect(calls[0]?.input.x).toBe(10);
+  });
+
+  test("a body naming its own kind does not decide where the call goes", async () => {
+    // `../computers/reset` is the shape that matters: the gateway interpolates this into the path it
+    // calls, and a fetch resolves the `..` away, so the request lands on a different endpoint of the
+    // computer's API carrying the deployment's computer token.
+    const { calls } = await send({ kind: "../computers/reset", x: 1, y: 1 });
+
+    expect(calls[0]?.input.kind).toBe("click");
+  });
+
+  test("a body naming its own kind cannot reach the shell either", async () => {
+    const { calls } = await send(
+      { kind: "../exec", command: "cat /workspace/notes" },
+      "type",
+    );
+
+    expect(calls[0]?.input.kind).toBe("type");
+  }, 10_000);
+
+  test("a kind the path does not allow is still refused", async () => {
+    // The existing check, which must go on working: the four gestures are the whole surface.
+    const { response, calls } = await send({ x: 1 }, "screenshot");
+
+    expect(response.status).toBe(400);
+    expect(calls).toHaveLength(0);
+  });
+});

server/tests/human-input-end-to-end.test.ts

@@ -0,0 +1,157 @@
+import { afterEach, describe, expect, test } from "bun:test";
+import type { MiddlewareHandler } from "hono";
+import type { AuditEventInput, AuditStore } from "../src/audit";
+import type { AppVariables, AuthenticatedActor } from "../src/auth/guards";
+import { createComputerGateway } from "../src/computer/gateway";
+import type { PolicyStore } from "../src/computer/policy-store";
+import type { ComputerProvider } from "../src/computer/provider";
+import { createComputerRoutes } from "../src/computer/routes";
+
+/**
+ * The whole path a person's input takes, over a real socket.
+ *
+ * The unit tests either side of this one stub the fetch, which means the URL they assert on is a
+ * string this process built rather than a request anything received. The thing being prevented here
+ * is a request ARRIVING somewhere it should not, and only a listener can say where one arrived: the
+ * `..` in `/human/../exec` is resolved by URL parsing inside fetch, so a test that reads the string
+ * it passed in is describing its own argument. This one starts a server, drives the real router
+ * through the real gateway and the real transport, and asks the server what it got.
+ *
+ * It stands in for `agent-computer` rather than running it, because that process opens Chromium at
+ * import time. What it reproduces is the part that matters: an HTTP API on the other end of the
+ * transport, with endpoints beyond the four this route is allowed to reach.
+ */
+
+const servers: Array<{ stop: (force?: boolean) => void }> = [];
+
+afterEach(() => {
+  for (const server of servers.splice(0)) server.stop(true);
+});
+
+const TOKEN = "computer-token-for-this-test";
+
+/** What the far side actually received, in the order it arrived. */
+type Received = { path: string; token: string | null; body: unknown };
+
+function serveComputer() {
+  const received: Received[] = [];
+  const server = Bun.serve({
+    port: 0,
+    fetch: async (request) => {
+      received.push({
+        path: new URL(request.url).pathname,
+        token: request.headers.get("x-openbot-computer-token"),
+        body: await request.json().catch(() => null),
+      });
+      return Response.json({ ok: true });
+    },
+  });
+  servers.push(server);
+  return { received, baseUrl: `http://127.0.0.1:${server.port}` };
+}
+
+function appFor(baseUrl: string) {
+  const rows: AuditEventInput[] = [];
+  const provider: ComputerProvider = {
+    name: "test",
+    isolation: "per-bot",
+    locate: async () => baseUrl,
+    status: async (botId) => ({ botId, state: "ready" }),
+    stop: async () => ({ wasRunning: true }),
+    reset: async () => ({ cleared: true }),
+    list: async () => [],
+  };
+  // No fetchImpl: the transport uses the platform's own, which is what resolves the path.
+  const gateway = createComputerGateway({
+    provider,
+    auditStore: {
+      insert: async (event: AuditEventInput) => {
+        rows.push(event);
+      },
+    } as unknown as AuditStore,
+    policy: () => ({ mode: "enforce", deny: [], allow: ["true"] }),
+    token: TOKEN,
+  });
+  const actor: AuthenticatedActor = {
+    id: "user-1",
+    email: "member@openbot.test",
+    role: "user",
+  };
+  const asActor: MiddlewareHandler<{ Variables: AppVariables }> = async (
+    context,
+    next,
+  ) => {
+    context.set("actor", actor);
+    await next();
+  };
+  return {
+    rows,
+    app: createComputerRoutes(
+      gateway,
+      {} as PolicyStore,
+      asActor,
+      async () => true,
+    ),
+  };
+}
+
+async function drive(kind: string, body: unknown) {
+  const { received, baseUrl } = serveComputer();
+  const { app, rows } = appFor(baseUrl);
+  const response = await app.request(
+    `http://openbot.test/bot-1/human/${kind}`,
+    {
+      method: "POST",
+      headers: { "content-type": "application/json" },
+      body: JSON.stringify(body),
+    },
+  );
+  return { response, received, rows };
+}
+
+describe("a person's input, end to end", () => {
+  test("a click arrives at the click endpoint, carrying the computer token", async () => {
+    const { response, received } = await drive("click", { x: 10, y: 20 });
+
+    expect(response.status).toBe(200);
+    expect(received).toHaveLength(1);
+    expect(received[0]?.path).toBe("/human/click");
+    expect(received[0]?.token).toBe(TOKEN);
+    expect(received[0]?.body).toEqual({ x: 10, y: 20 });
+  });
+
+  test.each([
+    [
+      "../computers/reset",
+      "/computers/reset",
+      "wipes the profile and every login in it",
+    ],
+    ["../exec", "/exec", "runs a command on the computer"],
+    ["../../health", "/health", "leaves the Bot's own surface entirely"],
+  ])(
+    "a body asking for %s does not arrive at %s, which %s",
+    async (kind, forbidden) => {
+      const { received } = await drive("click", {
+        kind,
+        command: "cat /workspace/notes",
+        x: 1,
+        y: 1,
+      });
+
+      // The assertion is about what the far side was asked to do, not about what this process
+      // intended: every path below is one the transport would have reached with the deployment's
+      // token attached, and none of them is guarded by the takeover check that protects `/human/*`.
+      expect(received.map((request) => request.path)).not.toContain(forbidden);
+    },
+  );
+
+  test("the four gestures still reach their own endpoints", async () => {
+    for (const kind of ["click", "type", "key", "scroll"]) {
+      const { received } = await drive(kind, { x: 1, y: 1, text: "hello" });
+
+      expect(received.map((request) => request.path)).toEqual([
+        `/human/${kind}`,
+      ]);
+    }
+  });
+});