Skip to content
Merged
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
30 changes: 20 additions & 10 deletions src/orb/relay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string | null> {
const declared = parseContentLength(contentLengthHeader);
if (declared !== null && declared > MAX_ORB_RELAY_REGISTER_BODY_BYTES) return null;
Expand All @@ -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 };
Expand Down
16 changes: 16 additions & 0 deletions test/integration/orb-relay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Uint8Array>({
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", () => {
Expand Down
Loading