Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions app/src/components/computer/live-screen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion app/src/lib/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,11 @@ export async function client<T>(

if (key === undefined) return response;

return ((await response.json()) as Record<string, T>)[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<string, T>)[key];
}
8 changes: 7 additions & 1 deletion app/src/lib/computers/control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
46 changes: 43 additions & 3 deletions app/src/lib/computers/screen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand All @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion app/src/lib/relative-time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
13 changes: 12 additions & 1 deletion app/src/lib/socket-url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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}`;
}
100 changes: 100 additions & 0 deletions app/tests/app-resilience-guards.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
Loading