Refuse a scroll whose deltaY is not a finite number

CHANGELOG.md

@@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A scroll with an unusable `deltaY` is refused, rather than scrolling some other distance
+
+`POST /computers/:botId/scroll` and `POST /computers/:botId/human/scroll` accepted any JSON number
+as `deltaY`, and `1e999` is a JSON number: it parses to `Infinity`, passes the `typeof` check, and is
+turned back into `null` by the hop to the Bot's computer, which reads the field as absent and scrolls
+its own default distance. The caller was answered 200 for a scroll it had not asked for. A `deltaY`
+that is not a finite number now answers 400 and the page is not touched, the way the timeout on
+`exec` and the coordinates behind a person's click already did.
 ### An MCP call carrying `x-api-key` is stopped the same as one carrying `api-key`
 
 The check that keeps credentials out of MCP tool arguments compared each argument name against a

server/src/computer/routes.ts

@@ -331,11 +331,12 @@ export function createComputerRoutes(
   );
 
   routes.post("/:botId/scroll", (context) =>
-    act(context, (botId, actor, body) =>
-      gateway.scroll(botId, actor, {
+    act(context, (botId, actor, body) => {
+      if (!usableDeltaY(body?.deltaY)) return badDeltaY;
+      return gateway.scroll(botId, actor, {
         ...(typeof body?.deltaY === "number" ? { deltaY: body.deltaY } : {}),
-      }),
-    ),
+      });
+    }),
   );
 
   /**
@@ -481,6 +482,9 @@ export function createComputerRoutes(
       string,
       unknown
     > | null;
+    if (kind === "scroll" && !usableDeltaY(body?.deltaY)) {
+      return context.json(badDeltaY, 400);
+    }
     try {
       return context.json(
         await gateway.humanInput(context.req.param("botId"), {
@@ -701,6 +705,24 @@ const badRef: BadRequest = {
     "A ref and the snapshotId it came from are both required. Take a snapshot first.",
 };
 
+const badDeltaY: BadRequest = {
+  error: "deltaY must be a finite number of pixels.",
+};
+
+/**
+ * Whether a wheel delta from an untrusted body can be carried out.
+ *
+ * `typeof value === "number"` is true of `Infinity`, and JSON carries it: `1e999` parses to it. It
+ * then survives every comparison on the way down and is erased by `JSON.stringify` on the hop to the
+ * computer, which reads the missing field as absent and scrolls its own default distance instead --
+ * so the caller is answered 200 for a scroll nobody asked for. Every other number on this surface is
+ * already checked at the edge: the timeout on `exec`, the coordinates behind `human/click`. This one
+ * was not.
+ */
+function usableDeltaY(value: unknown): boolean {
+  return value === undefined || Number.isFinite(value);
+}
+
 /**
  * Shared plumbing for acting routes that use this helper: resolve who is asking, run, and map
  * failures onto statuses.

server/tests/computer-scroll-delta.test.ts

@@ -0,0 +1,105 @@
+import { describe, expect, test } from "bun:test";
+import type { MiddlewareHandler } from "hono";
+import type { AppVariables } from "../src/auth/guards";
+import type { ComputerGateway } from "../src/computer/gateway";
+import type { PolicyStore } from "../src/computer/policy-store";
+import { createComputerRoutes } from "../src/computer/routes";
+
+function appWith(calls: { deltaY?: number }[]) {
+  const gateway = {
+    scroll: async (
+      _botId: string,
+      _actor: unknown,
+      input: { deltaY?: number },
+    ) => {
+      calls.push({
+        ...(input.deltaY !== undefined ? { deltaY: input.deltaY } : {}),
+      });
+      return { action: "scroll", url: "https://openbot.test/" };
+    },
+    humanInput: async (_botId: string, input: { deltaY?: number }) => {
+      calls.push({
+        ...(input.deltaY !== undefined ? { deltaY: input.deltaY } : {}),
+      });
+      return { action: "human_scroll", url: "https://openbot.test/" };
+    },
+  } as unknown as ComputerGateway;
+  const requireUser: MiddlewareHandler<{ Variables: AppVariables }> = async (
+    context,
+    next,
+  ) => {
+    context.set("actor", {
+      id: "user-1",
+      email: "user@openbot.test",
+      role: "admin",
+    });
+    await next();
+  };
+  return createComputerRoutes(
+    gateway,
+    {} as PolicyStore,
+    requireUser,
+    async () => true,
+  );
+}
+
+/**
+ * The body is sent as text, not as a stringified object.
+ *
+ * `JSON.stringify({ deltaY: Infinity })` is `{"deltaY":null}`, so an object literal cannot express
+ * what a client actually puts on the wire. `1e999` is valid JSON and parses to `Infinity`, which is
+ * the value this endpoint has to answer for.
+ */
+async function postScroll(
+  app: ReturnType<typeof createComputerRoutes>,
+  path: string,
+  body: string,
+) {
+  return app.request(`http://openbot.test/bot-1${path}`, {
+    method: "POST",
+    headers: { "content-type": "application/json" },
+    body,
+  });
+}
+
+describe.each(["/scroll", "/human/scroll"])("POST %s deltaY", (path) => {
+  test("a valid delta reaches the gateway", async () => {
+    const calls: { deltaY?: number }[] = [];
+    const response = await postScroll(appWith(calls), path, '{"deltaY":400}');
+
+    expect(response.status).toBe(200);
+    expect(calls).toEqual([{ deltaY: 400 }]);
+  });
+
+  test("an absent delta still scrolls the computer's own default", async () => {
+    const calls: { deltaY?: number }[] = [];
+    const response = await postScroll(appWith(calls), path, "{}");
+
+    expect(response.status).toBe(200);
+    expect(calls).toEqual([{}]);
+  });
+
+  test.each([
+    ["Infinity", "1e999"],
+    ["negative Infinity", "-1e999"],
+    ["a string", '"400"'],
+    ["null", "null"],
+    ["a boolean", "true"],
+  ])(
+    "rejects %s with 400 and never reaches the gateway",
+    async (_name, raw) => {
+      const calls: { deltaY?: number }[] = [];
+      const response = await postScroll(
+        appWith(calls),
+        path,
+        `{"deltaY":${raw}}`,
+      );
+
+      expect(response.status).toBe(400);
+      await expect(response.json()).resolves.toEqual({
+        error: "deltaY must be a finite number of pixels.",
+      });
+      expect(calls).toEqual([]);
+    },
+  );
+});