From b74fc7de2d9c0cb0696ecec18d2c7bb83c009923 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 00:40:14 -0700 Subject: [PATCH] fix(orb): fail safe on a stream read error in readOrbRelayRegisterBody Found via Sentry (GITTENSORY-J): a dropped connection or network reset mid-read threw uncaught out of readOrbRelayRegisterBody, which all three callers (POST /v1/orb/token, /v1/orb/relay/register, /v1/orb/relay pull) invoke before their own try/catch -- the exception escaped as a bare framework 500 instead of each route's own clean 4xx/503 JSON response. Wrap the read loop and return null (the same sentinel already used for an oversized payload) on any stream error, fixing all three call sites at the source. --- src/orb/relay.ts | 30 ++++++++++++++++++++---------- test/integration/orb-relay.test.ts | 16 ++++++++++++++++ 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/orb/relay.ts b/src/orb/relay.ts index fe2c06ddec..a2f5be6e26 100644 --- a/src/orb/relay.ts +++ b/src/orb/relay.ts @@ -163,7 +163,13 @@ function parseContentLength(header: string | null | undefined): number | null { return Number.isInteger(n) && n >= 0 ? n : null; } -/** Read the relay-registration JSON with a small hard ceiling; returns null when the sender exceeds it. */ +/** Read the relay-registration JSON with a small hard ceiling; returns null when the sender exceeds it OR when + * the underlying stream itself errors (a dropped connection / network reset mid-read, #orb-broker-500 — every + * caller already treats null identically to "reject this request", so a transient read failure degrades the + * same way an oversized payload does, instead of throwing UNCAUGHT out of this function. Each of this + * function's three route call sites (POST /v1/orb/token, /v1/orb/relay/register, /v1/orb/relay pull) calls it + * BEFORE its own try/catch, so an uncaught throw here previously escaped as a bare framework 500 instead of the + * route's own clean 4xx/503 JSON error response — fixed once here rather than wrapping all three callers. */ export async function readOrbRelayRegisterBody(request: Request, contentLengthHeader: string | null | undefined): Promise { const declared = parseContentLength(contentLengthHeader); if (declared !== null && declared > MAX_ORB_RELAY_REGISTER_BODY_BYTES) return null; @@ -174,17 +180,21 @@ export async function readOrbRelayRegisterBody(request: Request, contentLengthHe const decoder = new TextDecoder(); let total = 0; let out = ""; - for (;;) { - const { done, value } = await reader.read(); - if (done) break; - total += value.byteLength; - if (total > MAX_ORB_RELAY_REGISTER_BODY_BYTES) { - await reader.cancel(); - return null; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + total += value.byteLength; + if (total > MAX_ORB_RELAY_REGISTER_BODY_BYTES) { + await reader.cancel(); + return null; + } + out += decoder.decode(value, { stream: true }); } - out += decoder.decode(value, { stream: true }); + return out + decoder.decode(); + } catch { + return null; } - return out + decoder.decode(); } export type RelayEnrollment = { enrollId: string; installationId: number }; diff --git a/test/integration/orb-relay.test.ts b/test/integration/orb-relay.test.ts index 72e531441a..d8aee540e4 100644 --- a/test/integration/orb-relay.test.ts +++ b/test/integration/orb-relay.test.ts @@ -141,6 +141,22 @@ describe("readOrbRelayRegisterBody", () => { const req = new Request("http://localhost/r", { method: "POST", body: "x".repeat(MAX_ORB_RELAY_REGISTER_BODY_BYTES + 1) }); expect(await readOrbRelayRegisterBody(req, null)).toBeNull(); }); + + it("REGRESSION (#orb-broker-500): returns null instead of throwing when the underlying stream errors mid-read", async () => { + // Simulates a dropped connection / network reset while GitHub Sentry observed as "Orb broker token exchange + // failed (500)" (GITTENSORY-J) — this function is called BEFORE each of its three route call sites' own + // try/catch, so an uncaught throw here previously escaped as a bare framework 500 rather than the route's + // own clean 4xx/503 JSON response. A first successful chunk (some bytes already arrived) followed by a + // stream error is the realistic shape of a mid-read network drop, not an error on the very first read. + const stream = new ReadableStream({ + pull(controller) { + controller.enqueue(new TextEncoder().encode('{"relayUrl":')); + controller.error(new Error("simulated network reset")); + }, + }); + const req = new Request("http://localhost/r", { method: "POST", body: stream, duplex: "half" } as RequestInit); + await expect(readOrbRelayRegisterBody(req, null)).resolves.toBeNull(); + }); }); describe("relaySignature", () => {