diff --git a/app/src/components/computer/live-screen.tsx b/app/src/components/computer/live-screen.tsx index cd9e1784f..0a05d62aa 100644 --- a/app/src/components/computer/live-screen.tsx +++ b/app/src/components/computer/live-screen.tsx @@ -101,6 +101,29 @@ export function LiveScreen({ computerId, driving, onProblem }: Props) { return; } if (message.type !== "frame" || !message.data) return; + // Live-run JSON only checks `typeof type === "string"` upstream. Non-finite or negative + // dimensions would poison frameSize and every coordinate scaled from it; a huge payload + // would hit `atob` before any bound. Both are dropped as corrupt frames. + const width = message.width ?? 1280; + const height = message.height ?? 800; + if ( + typeof width !== "number" || + typeof height !== "number" || + !Number.isFinite(width) || + !Number.isFinite(height) || + width <= 0 || + height <= 0 || + width > 8192 || + height > 8192 + ) { + return; + } + if ( + typeof message.data !== "string" || + message.data.length > 20_000_000 + ) { + return; + } const canvas = canvasRef.current; if (!canvas || closed) return; diff --git a/app/src/lib/client.ts b/app/src/lib/client.ts index 2abae4c40..2e92b282b 100644 --- a/app/src/lib/client.ts +++ b/app/src/lib/client.ts @@ -98,5 +98,11 @@ export async function client( if (key === undefined) return response; - return ((await response.json()) as Record)[key]; + // A 204 or a non-JSON body used to throw a bare SyntaxError, and `null` or an array body threw + // a TypeError on property access. Malformed success is a failed request with the fallback. + const body = (await response.json().catch(() => null)) as unknown; + if (!body || typeof body !== "object" || Array.isArray(body)) { + throw new Error(options.fallback ?? "That request failed."); + } + return (body as Record)[key]; } diff --git a/app/src/lib/computers/control.ts b/app/src/lib/computers/control.ts index eddca0965..686c2547d 100644 --- a/app/src/lib/computers/control.ts +++ b/app/src/lib/computers/control.ts @@ -30,7 +30,13 @@ async function callControl( method ? { method } : {}, ); if (!response.ok) return null; - return (await response.json()) as ControlState; + // A non-JSON or wrong-shaped body used to throw out of the readers and reject the panel's + // poll. Reads answer null on failure, so malformed succeeds as missing. + const body = (await response.json().catch(() => null)) as unknown; + if (!body || typeof body !== "object" || Array.isArray(body)) return null; + const holder = (body as { holder?: unknown }).holder; + if (holder !== "bot" && holder !== "human") return null; + return body as ControlState; } export function readControl(computerId: string) { diff --git a/app/src/lib/computers/screen.ts b/app/src/lib/computers/screen.ts index df3025e20..d52608af9 100644 --- a/app/src/lib/computers/screen.ts +++ b/app/src/lib/computers/screen.ts @@ -35,12 +35,45 @@ export async function readScreenshot( } | null; return { error: body?.error ?? unavailable }; } - return { frame: (await response.json()) as Screenshot }; + return parseScreenshot(await response.json().catch(() => null)); } catch { return { error: unavailable }; } } +function parseScreenshot(body: unknown): { + frame?: Screenshot; + error?: string; +} { + const unavailable = "The screen is not available right now."; + if (!body || typeof body !== "object" || Array.isArray(body)) { + return { error: unavailable }; + } + const frame = (body as { frame?: unknown }).frame ?? body; + if (!frame || typeof frame !== "object" || Array.isArray(frame)) { + return { error: unavailable }; + } + const { base64, width, height } = frame as { + base64?: unknown; + width?: unknown; + height?: unknown; + }; + // A mistyped frame used to reach `atob` in the viewer and throw there. Refused here instead. + if ( + typeof base64 !== "string" || + !base64 || + typeof width !== "number" || + !Number.isFinite(width) || + width <= 0 || + typeof height !== "number" || + !Number.isFinite(height) || + height <= 0 + ) { + return { error: unavailable }; + } + return { frame: frame as Screenshot }; +} + /** The frame a page was showing when a Bot opened it. */ export type PageFrame = { url: string; title: string | null; frame: string }; @@ -61,8 +94,15 @@ export async function readPageFrame( `/api/computers/${computerId}/page-frame/${encodeURIComponent(toolCallId)}`, ); if (!response.ok) return null; - const body = (await response.json()) as { frame?: PageFrame | null }; - return body.frame ?? null; + const body = (await response.json().catch(() => null)) as unknown; + if (!body || typeof body !== "object" || Array.isArray(body)) return null; + const frame = (body as { frame?: unknown }).frame; + if (!frame || typeof frame !== "object" || Array.isArray(frame)) { + return null; + } + const { frame: image } = frame as { frame?: unknown }; + if (typeof image !== "string" || !image) return null; + return frame as PageFrame; } catch { // A missing picture is a smaller sentence, not a broken conversation. return null; diff --git a/app/src/lib/relative-time.ts b/app/src/lib/relative-time.ts index d05c076dd..f9c17d3f3 100644 --- a/app/src/lib/relative-time.ts +++ b/app/src/lib/relative-time.ts @@ -12,7 +12,11 @@ const relativeFormat = new Intl.RelativeTimeFormat(undefined, { /** Locale-aware relative timestamp, e.g. "2 minutes ago". */ export function relativeTime(iso: string): string { - const elapsed = Date.now() - new Date(iso).getTime(); + const time = new Date(iso).getTime(); + // An invalid date used to flow into `Math.abs(NaN)` comparisons and `format(-NaN)`, which + // answers "NaN weeks ago" or throws depending on ICU. Return the input unchanged instead. + if (!Number.isFinite(time)) return iso; + const elapsed = Date.now() - time; const scale = RELATIVE_UNITS.find(({ limit }) => Math.abs(elapsed) < limit) ?? RELATIVE_UNITS[RELATIVE_UNITS.length - 1]; diff --git a/app/src/lib/socket-url.ts b/app/src/lib/socket-url.ts index 0822c0fb8..5d0d47db5 100644 --- a/app/src/lib/socket-url.ts +++ b/app/src/lib/socket-url.ts @@ -27,6 +27,16 @@ function announcedPort(): string { : ""; } +function validPort(port: string): string { + // A whitespace, alphabetic, or out-of-range value used to be interpolated into the authority, + // so `new WebSocket()` threw synchronously inside the effects that open it. Only whole digits + // in range override same-origin. + if (!/^\d+$/.test(port.trim())) return ""; + const n = Number(port.trim()); + if (!Number.isInteger(n) || n < 1 || n > 65535) return ""; + return String(n); +} + export function socketUrl( path: string, location: { @@ -39,6 +49,7 @@ export function socketUrl( const scheme = location.protocol === "https:" ? "wss:" : "ws:"; // A port only when a Vite runtime named one: the app is not same-origin with the server there. // Otherwise the browser's own host, which is the server's own origin in production. - const authority = port ? `${location.hostname}:${port}` : location.host; + const usable = validPort(port); + const authority = usable ? `${location.hostname}:${usable}` : location.host; return `${scheme}//${authority}${path}`; } diff --git a/app/tests/app-resilience-guards.test.ts b/app/tests/app-resilience-guards.test.ts new file mode 100644 index 000000000..d1df29a2f --- /dev/null +++ b/app/tests/app-resilience-guards.test.ts @@ -0,0 +1,100 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { socketUrl } from "../src/lib/socket-url"; +import { relativeTime } from "../src/lib/relative-time"; + +const at = (protocol: string, hostname: string, host: string) => ({ + protocol, + hostname, + host, +}); + +/** + * `__OPENBOT_WS_PORT__` was interpolated into the WebSocket authority unchecked, so `" "`, + * `"abc"` or `"99999"` produced `ws://host:abc/path` and `new WebSocket()` threw synchronously + * inside the effects that open it. Only whole digits in range override same-origin now. + */ +describe("socketUrl port validation", () => { + test.each([ + ["whitespace", " "], + ["letters", "abc"], + ["too large", "99999"], + ["zero", "0"], + ])("falls back to same-origin on %s", (_n, port) => { + expect( + socketUrl("/api/channels/events", at("http:", "h", "h:3010"), port), + ).toBe("ws://h:3010/api/channels/events"); + }); + + test("keeps a valid override", () => { + expect( + socketUrl("/api/channels/events", at("http:", "h", "h:3010"), "3001"), + ).toBe("ws://h:3001/api/channels/events"); + }); +}); + +/** + * An invalid date used to flow into `Math.abs(NaN)` comparisons and `format(-NaN)`, answering + * "NaN weeks ago" or throwing depending on ICU. The input is returned unchanged instead. + */ +describe("relativeTime", () => { + test("returns the input for an invalid date", () => { + expect(relativeTime("not-a-date")).toBe("not-a-date"); + }); + + test("still formats a valid date", () => { + const iso = new Date(Date.now() - 30_000).toISOString(); + expect(relativeTime(iso)).toMatch(/second/); + }); +}); + +describe("control/screen/client guards", () => { + const realFetch = globalThis.fetch; + + afterEach(() => { + globalThis.fetch = realFetch; + }); + + test("readControl answers null on a malformed body", async () => { + globalThis.fetch = (async () => + new Response(JSON.stringify([1, 2]), { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof fetch; + const { readControl } = await import("../src/lib/computers/control"); + await expect(readControl("bot-1")).resolves.toBeNull(); + }); + + test("client throws the fallback on a malformed envelope", async () => { + globalThis.fetch = (async () => + new Response("null", { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof fetch; + const { client } = await import("../src/lib/client"); + await expect( + client("http://x.test/thing", "thing", { fallback: "Nope." }), + ).rejects.toThrow("Nope."); + }); + + test("readScreenshot reports unavailable on a mistyped frame", async () => { + globalThis.fetch = (async () => + new Response(JSON.stringify({ base64: 42, width: "x", height: 1 }), { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof fetch; + const { readScreenshot } = await import("../src/lib/computers/screen"); + const result = await readScreenshot("bot-1"); + expect(result.frame).toBeUndefined(); + expect(typeof result.error).toBe("string"); + }); + + test("readPageFrame answers null on an array frame", async () => { + globalThis.fetch = (async () => + new Response(JSON.stringify({ frame: [] }), { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof fetch; + const { readPageFrame } = await import("../src/lib/computers/screen"); + await expect(readPageFrame("bot-1", "turn-1")).resolves.toBeNull(); + }); +});