From 19f5a4a0bb030f9fb482bf37fedfd3b29d9b0f79 Mon Sep 17 00:00:00 2001 From: NIO Date: Mon, 6 Jul 2026 10:22:59 +0800 Subject: [PATCH] fix(tlon): bound urbit scry JSON response reads (#100376) * fix(tlon): bound urbit scry JSON response reads * fix(tlon): preserve bounded scry diagnostics * refactor(tlon): tighten scry boundary proof * docs(changelog): stabilize Tlon response entry * docs(changelog): close out maintainer batch * chore(changelog): defer batch closeout --------- Co-authored-by: NIO Co-authored-by: Peter Steinberger --- extensions/tlon/src/urbit/channel-ops.test.ts | 6 +- extensions/tlon/src/urbit/channel-ops.ts | 17 +++-- .../src/urbit/error-body-boundary.test.ts | 73 ++++++++++++++++--- 3 files changed, 76 insertions(+), 20 deletions(-) diff --git a/extensions/tlon/src/urbit/channel-ops.test.ts b/extensions/tlon/src/urbit/channel-ops.test.ts index 157b5d3ac8c0..1c42056e940a 100644 --- a/extensions/tlon/src/urbit/channel-ops.test.ts +++ b/extensions/tlon/src/urbit/channel-ops.test.ts @@ -12,7 +12,7 @@ describe("Urbit channel operations", () => { vi.mocked(urbitFetch).mockReset(); }); - it("wraps malformed scry response JSON", async () => { + it("rejects malformed scry response JSON", async () => { const release = vi.fn().mockResolvedValue(undefined); vi.mocked(urbitFetch).mockResolvedValue({ response: new Response("{not json", { @@ -31,7 +31,9 @@ describe("Urbit channel operations", () => { }, { path: "/chat/inbox.json", auditContext: "test" }, ), - ).rejects.toThrow("Urbit scry response was malformed JSON for path /chat/inbox.json"); + ).rejects.toThrow( + "Tlon scry response for path /chat/inbox.json: malformed JSON response", + ); expect(release).toHaveBeenCalledTimes(1); }); }); diff --git a/extensions/tlon/src/urbit/channel-ops.ts b/extensions/tlon/src/urbit/channel-ops.ts index d0ffe927ba19..a9117e49ada8 100644 --- a/extensions/tlon/src/urbit/channel-ops.ts +++ b/extensions/tlon/src/urbit/channel-ops.ts @@ -1,5 +1,8 @@ // Tlon plugin module implements channel ops behavior. -import { readResponseTextLimited } from "openclaw/plugin-sdk/provider-http"; +import { + readProviderJsonResponse, + readResponseTextLimited, +} from "openclaw/plugin-sdk/provider-http"; import type { LookupFn, SsrFPolicy } from "openclaw/plugin-sdk/ssrf-runtime"; import { UrbitHttpError } from "./errors.js"; import { urbitFetch } from "./fetch.js"; @@ -60,7 +63,9 @@ export async function pokeUrbitChannel( try { if (!response.ok && response.status !== 204) { - const errorText = await readResponseTextLimited(response, TLON_ERROR_BODY_LIMIT_BYTES).catch(() => ""); + const errorText = await readResponseTextLimited(response, TLON_ERROR_BODY_LIMIT_BYTES).catch( + () => "", + ); throw new Error(`Poke failed: ${response.status}${errorText ? ` - ${errorText}` : ""}`); } return pokeId; @@ -92,11 +97,9 @@ export async function scryUrbitPath( if (!response.ok) { throw new Error(`Scry failed: ${response.status} for path ${params.path}`); } - try { - return await response.json(); - } catch (cause) { - throw new Error(`Urbit scry response was malformed JSON for path ${params.path}`, { cause }); - } + // Successful scry bodies come from a remote Urbit and have no protocol size bound. + // Keep the shared JSON ceiling while retaining the path needed to identify the endpoint. + return await readProviderJsonResponse(response, `Tlon scry response for path ${params.path}`); } finally { await release(); } diff --git a/extensions/tlon/src/urbit/error-body-boundary.test.ts b/extensions/tlon/src/urbit/error-body-boundary.test.ts index 09a84c32c027..9c0aa972ffb6 100644 --- a/extensions/tlon/src/urbit/error-body-boundary.test.ts +++ b/extensions/tlon/src/urbit/error-body-boundary.test.ts @@ -19,9 +19,18 @@ vi.mock("openclaw/plugin-sdk/ssrf-runtime", async () => { }; }); -const { pokeUrbitChannel } = await import("./channel-ops.js"); +const { pokeUrbitChannel, scryUrbitPath } = await import("./channel-ops.js"); const CHUNK = Buffer.alloc(64 * 1024, "X"); +const SCRY_PATH = "/groups-ui/v6/init.json"; + +async function listen(server: http.Server): Promise { + return await new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => { + resolve((server.address() as { port: number }).port); + }); + }); +} describe("tlon error body boundary", () => { let server: http.Server; @@ -52,11 +61,7 @@ describe("tlon error body boundary", () => { } write(); }); - const port = await new Promise((resolve) => { - server.listen(0, "127.0.0.1", () => { - resolve((server.address() as { port: number }).port); - }); - }); + const port = await listen(server); const err = await pokeUrbitChannel( { @@ -79,11 +84,7 @@ describe("tlon error body boundary", () => { res.writeHead(500, { "Content-Type": "text/plain" }); res.end("session expired"); }); - const port = await new Promise((resolve) => { - server.listen(0, "127.0.0.1", () => { - resolve((server.address() as { port: number }).port); - }); - }); + const port = await listen(server); const err = await pokeUrbitChannel( { @@ -98,4 +99,54 @@ describe("tlon error body boundary", () => { expect(err).toBeInstanceOf(Error); expect((err as Error).message).toContain("session expired"); }); + + it("parses a normal scry response over HTTP", async () => { + server = http.createServer((_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ groups: {} })); + }); + const port = await listen(server); + + await expect( + scryUrbitPath( + { baseUrl: `http://127.0.0.1:${port}`, cookie: "urbit=cookie" }, + { path: SCRY_PATH, auditContext: "test" }, + ), + ).resolves.toEqual({ groups: {} }); + }); + + it("bounds a streaming successful scry response over HTTP", async () => { + server = http.createServer((_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.write('{"payload":"'); + let written = 0; + const write = () => { + if (res.destroyed) { + return; + } + if (written >= 18 * 1024 * 1024) { + res.end('"}'); + return; + } + written += CHUNK.length; + if (res.write(CHUNK)) { + setImmediate(write); + } else { + res.once("drain", write); + } + }; + write(); + }); + const port = await listen(server); + + const error = await scryUrbitPath( + { baseUrl: `http://127.0.0.1:${port}`, cookie: "urbit=cookie" }, + { path: SCRY_PATH, auditContext: "test" }, + ).catch((cause: unknown) => cause); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + `Tlon scry response for path ${SCRY_PATH}: JSON response exceeds 16777216 bytes`, + ); + }); });