From c4b5185c63eb3773b804c03dda597f51c4121959 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Mon, 7 Sep 2026 21:05:44 +0530 Subject: [PATCH] Validate timeoutMs on POST /computers/:botId/exec A NaN, Infinity, negative, fractional, or ten-hour timeout travelled to the computer, where AbortSignal.timeout threw a RangeError 500 or the run outlasted the 615s transport backstop the gateway documents. The shell's real bounds are 1s to 600s. Reject anything outside a whole number of milliseconds in 1000..600000 with 400 before the gateway decides and records, so the refusal is a caller error rather than a failed action on the trail. --- server/src/computer/routes.ts | 18 +++- server/tests/computer-exec-timeout.test.ts | 101 +++++++++++++++++++++ 2 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 server/tests/computer-exec-timeout.test.ts diff --git a/server/src/computer/routes.ts b/server/src/computer/routes.ts index 21b24632c..2ff5ceaa3 100644 --- a/server/src/computer/routes.ts +++ b/server/src/computer/routes.ts @@ -537,14 +537,28 @@ export function createComputerRoutes( * A command on the Bot's computer. * * Same shape as every other acting route: the gateway decides and records, this only shapes the - * request. `timeoutMs` is passed through and capped by the computer rather than here, so one place - * owns the limit. + * request. `timeoutMs` is validated here against the shell's own bounds (1s floor, 600s ceiling), + * so a NaN, an Infinity, a negative, or a ten-hour value answers 400 instead of travelling to the + * computer as a RangeError 500 or a run that outlasts the transport backstop. */ routes.post("/:botId/exec", (context) => act(context, (botId, actor, body, signal) => { if (typeof body?.command !== "string" || !body.command.trim()) { return { error: "A command is required." }; } + if (body.timeoutMs !== undefined) { + if ( + typeof body.timeoutMs !== "number" || + !Number.isInteger(body.timeoutMs) || + body.timeoutMs < 1_000 || + body.timeoutMs > 600_000 + ) { + return { + error: + "timeoutMs must be a whole number of milliseconds between 1000 and 600000.", + }; + } + } // The fourth argument, like every other acting route. Without it the plumbing through // gateway.runCommand and into the shell's own abort listener was dead code, and Stop ended the // run in the transcript while the command carried on to completion inside the container. diff --git a/server/tests/computer-exec-timeout.test.ts b/server/tests/computer-exec-timeout.test.ts new file mode 100644 index 000000000..8ee92d1f0 --- /dev/null +++ b/server/tests/computer-exec-timeout.test.ts @@ -0,0 +1,101 @@ +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: { timeoutMs?: number }[]) { + const gateway = { + runCommand: async ( + _botId: string, + _actor: unknown, + input: { command: string; timeoutMs?: number }, + ) => { + calls.push({ + ...(input.timeoutMs !== undefined + ? { timeoutMs: input.timeoutMs } + : {}), + }); + return { output: "hi", timedOut: false, elapsedMs: 1 }; + }, + } 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, + ); +} + +async function postExec( + app: ReturnType, + body: unknown, +) { + return app.request("http://openbot.test/bot-1/exec", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +describe("POST /:botId/exec timeoutMs", () => { + test("a valid timeout reaches the gateway", async () => { + const calls: { timeoutMs?: number }[] = []; + const response = await postExec(appWith(calls), { + command: "echo hi", + timeoutMs: 5000, + }); + + expect(response.status).toBe(200); + expect(calls).toEqual([{ timeoutMs: 5000 }]); + }); + + test("an absent timeout still runs with the computer default", async () => { + const calls: { timeoutMs?: number }[] = []; + const response = await postExec(appWith(calls), { command: "echo hi" }); + + expect(response.status).toBe(200); + expect(calls).toEqual([{}]); + }); + + test.each([ + ["NaN", Number.NaN], + ["Infinity", Number.POSITIVE_INFINITY], + ["a negative", -1], + ["zero", 0], + ["below the shell floor", 999], + ["above the shell ceiling", 600_001], + ["a ten-hour value", 36_000_000], + ["a fraction", 1500.5], + ["a string", "3000"], + ["null", null], + ])( + "rejects %s with 400 and never reaches the gateway", + async (_name, timeoutMs) => { + const calls: { timeoutMs?: number }[] = []; + const response = await postExec(appWith(calls), { + command: "echo hi", + timeoutMs, + }); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ + error: + "timeoutMs must be a whole number of milliseconds between 1000 and 600000.", + }); + expect(calls).toEqual([]); + }, + ); +});