Refuse a non-integer snapshotId as malformed, not stale

CHANGELOG.md

@@ -8,6 +8,12 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A fractional or infinite snapshot id is refused as malformed, not stale
+
+A `snapshotId` of `1.5` or `Infinity` passed the acting routes and never matched the stored
+integer, so the answer was a 409 stale snapshot and the caller retried a request that was
+malformed. Non-integer ids are refused with a 400 naming the ref and its snapshot before any
+decision or audit row.
 ### A `DATABASE_URL` with a port of zero is refused at start-up
 
 `postgres://…:0/…` parsed and booted, and every query then failed against a port nothing listens

server/src/computer/routes.ts

@@ -431,7 +431,10 @@ export function createComputerRoutes(
             "Say which field the value goes in, using a ref from your snapshot.",
         };
       }
-      if (typeof body?.snapshotId !== "number") {
+      if (
+        typeof body?.snapshotId !== "number" ||
+        !Number.isInteger(body.snapshotId)
+      ) {
         return { error: "The snapshotId the ref came from is required." };
       }
       return gateway.requestSecret(botId, actor, {
@@ -816,7 +819,18 @@ function asRef(
   body: Record<string, unknown> | null,
 ): { ref: string; snapshotId: number } | undefined {
   if (typeof body?.ref !== "string" || !body.ref) return undefined;
-  if (typeof body?.snapshotId !== "number") return undefined;
+  /*
+   * A snapshot id is an integer the snapshot store handed out. `typeof` alone accepts `1.5` and
+   * `Infinity` (valid JSON: `1e999` parses to it), which then never equals the stored integer, so
+   * the gateway reports a stale snapshot and the caller retries a request that was malformed.
+   * Malformed input is a 400 here, not a 409 staleness.
+   */
+  if (
+    typeof body?.snapshotId !== "number" ||
+    !Number.isInteger(body.snapshotId)
+  ) {
+    return undefined;
+  }
   return { ref: body.ref, snapshotId: body.snapshotId };
 }
 

server/tests/computer-snapshot-id.test.ts

@@ -0,0 +1,85 @@
+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: unknown[]) {
+  const gateway = {
+    click: async (_botId: string, _actor: unknown, ref: unknown) => {
+      calls.push(ref);
+      return { action: "click", 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,
+  );
+}
+
+/**
+ * A snapshot id is an integer the store handed out, not any number.
+ *
+ * `typeof 1.5 === "number"` and `1e999` parses to `Infinity`, so both used to pass the edge
+ * check and then never equal the stored integer. The gateway answered a stale snapshot (409)
+ * and the caller retried a request that was malformed. Malformed input is a 400 here.
+ */
+async function postClick(
+  app: ReturnType<typeof createComputerRoutes>,
+  body: string,
+) {
+  return app.request("http://openbot.test/bot-1/click", {
+    method: "POST",
+    headers: { "content-type": "application/json" },
+    body,
+  });
+}
+
+describe("POST /:botId/click snapshotId", () => {
+  test("an integer snapshotId reaches the gateway", async () => {
+    const calls: unknown[] = [];
+    const response = await postClick(
+      appWith(calls),
+      '{"ref":"e5","snapshotId":12}',
+    );
+
+    expect(response.status).toBe(200);
+    expect(calls).toEqual([{ ref: "e5", snapshotId: 12 }]);
+  });
+
+  test.each([
+    ["a float", "1.5"],
+    ["Infinity", "1e999"],
+    ["negative Infinity", "-1e999"],
+    ["a string", '"12"'],
+    ["null", "null"],
+    ["a boolean", "true"],
+  ])("rejects %s with 400 and never reaches the gateway", async (_n, raw) => {
+    const calls: unknown[] = [];
+    const response = await postClick(
+      appWith(calls),
+      `{"ref":"e5","snapshotId":${raw}}`,
+    );
+
+    expect(response.status).toBe(400);
+    await expect(response.json()).resolves.toEqual({
+      error:
+        "A ref and the snapshotId it came from are both required. Take a snapshot first.",
+    });
+    expect(calls).toEqual([]);
+  });
+});