From 627c60916723049890a6a13cfc7e66c7acdb27f5 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Thu, 25 Jun 2026 15:40:45 -0700 Subject: [PATCH 1/2] fix(orb): revalidate relay DNS before forwarding --- src/orb/relay.ts | 32 ++++++++++++++++++++++++++ test/integration/orb-relay.test.ts | 37 ++++++++++++++++++++++++++---- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/orb/relay.ts b/src/orb/relay.ts index ef9fe1f2c..f6d536416 100644 --- a/src/orb/relay.ts +++ b/src/orb/relay.ts @@ -8,6 +8,36 @@ import { hashToken } from "../auth/security"; import { isSafeHttpUrl } from "../review/content-lane/safe-url"; import { decryptSecret, encryptSecret } from "../utils/crypto"; +type RelayDnsResolver = (hostname: string) => Promise; + +function addressAsUrlHost(address: string): string { + return address.includes(":") ? `[${address.replace(/^\[|\]$/g, "")}]` : address; +} + +function resolvedAddressIsSafe(address: string): boolean { + return isSafeHttpUrl(`https://${addressAsUrlHost(address)}/`); +} + +async function resolveRelayHostname(hostname: string): Promise { + if (/^\d+\.\d+\.\d+\.\d+$/.test(hostname) || hostname.includes(":")) return [hostname]; + const answers: string[] = []; + for (const type of ["A", "AAAA"]) { + const res = await fetch(`https://cloudflare-dns.com/dns-query?name=${encodeURIComponent(hostname)}&type=${type}`, { headers: { accept: "application/dns-json" }, signal: AbortSignal.timeout(3_000) }); + if (!res.ok) throw new Error("dns_resolution_failed"); + const json = (await res.json()) as { Answer?: { data?: string; type?: number }[] }; + for (const answer of json.Answer ?? []) { + if ((answer.type === 1 || answer.type === 28) && answer.data) answers.push(answer.data); + } + } + return answers; +} + +async function relayDestinationIsSafe(raw: string, resolveHostname: RelayDnsResolver): Promise { + if (!isSafeHttpUrl(raw)) return false; + const addresses = await resolveHostname(new URL(raw).hostname.toLowerCase()); + return addresses.length > 0 && addresses.every(resolvedAddressIsSafe); +} + // The events a brokered container needs to review/act on. Installation-lifecycle + other Orb-internal events are // deliberately NOT forwarded (the container runs under the CENTRAL Orb App, not its own, so it must not treat // those as its own installation state). @@ -154,6 +184,7 @@ export async function forwardOrbEvent( env: Env, args: { eventName: string; installationId: number | null | undefined; deliveryId: string; rawBody: string }, fetchImpl: typeof fetch = fetch, + resolveHostname: RelayDnsResolver = resolveRelayHostname, ): Promise<"forwarded" | "skipped" | "failed"> { if (!args.installationId || !RELAY_FORWARD_EVENTS.has(args.eventName)) return "skipped"; const row = await env.DB @@ -162,6 +193,7 @@ export async function forwardOrbEvent( .first<{ relay_url: string; relay_secret_enc: string; relay_secret_iv: string; relay_secret_salt: string | null }>(); if (!row || !env.TOKEN_ENCRYPTION_SECRET) return "skipped"; try { + if (!(await relayDestinationIsSafe(row.relay_url, resolveHostname))) return "failed"; const secret = await decryptSecret(row.relay_secret_enc, row.relay_secret_iv, env.TOKEN_ENCRYPTION_SECRET, row.relay_secret_salt); const signature = await relaySignature(secret, args.rawBody); const res = await fetchImpl(row.relay_url, { diff --git a/test/integration/orb-relay.test.ts b/test/integration/orb-relay.test.ts index 997c4ab04..3f9acc36e 100644 --- a/test/integration/orb-relay.test.ts +++ b/test/integration/orb-relay.test.ts @@ -1,9 +1,11 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createApp } from "../../src/api/routes"; import { issueOrbEnrollment } from "../../src/orb/broker"; import { forwardOrbEvent, registerOrbRelay, relaySignature, relayVerify, retryFailedRelays, storeRelayFailure } from "../../src/orb/relay"; import { createTestEnv, type TestD1Database } from "../helpers/d1"; +afterEach(() => vi.unstubAllGlobals()); + const db = (e: Env) => e.DB as unknown as TestD1Database; const seedInstall = (e: Env, id: number, cols: Record = {}) => { const all: Record = { installation_id: id, registered: 1, ...cols }; @@ -103,6 +105,7 @@ describe("relaySignature", () => { }); describe("forwardOrbEvent", () => { + const publicDns = async () => ["203.0.113.10"]; const capture = (resp: Response) => { const calls: { url: string; init?: RequestInit | undefined }[] = []; const fetchImpl = ((u: RequestInfo | URL, init?: RequestInit) => { @@ -127,7 +130,7 @@ describe("forwardOrbEvent", () => { await registerOrbRelay(e, secret, "https://c.example/v1/orb/relay"); const { fetchImpl, calls } = capture(new Response("ok")); const body = '{"action":"opened","number":7}'; - expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 800, deliveryId: "del-1", rawBody: body }, fetchImpl)).toBe("forwarded"); + expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 800, deliveryId: "del-1", rawBody: body }, fetchImpl, publicDns)).toBe("forwarded"); expect(calls[0]?.url).toBe("https://c.example/v1/orb/relay"); const h = calls[0]?.init?.headers as Record; expect(h["x-github-event"]).toBe("pull_request"); @@ -136,12 +139,38 @@ describe("forwardOrbEvent", () => { expect(calls[0]?.init?.body).toBe(body); }); + it("revalidates the relay DNS destination at forward time before posting", async () => { + const e = brokeredEnv(); + const secret = await enroll(e, 805); + await registerOrbRelay(e, secret, "https://dns-checked.example/v1/orb/relay"); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = new URL(String(input)); + const type = url.searchParams.get("type"); + return Response.json({ Answer: type === "A" ? [{ type: 1, data: "203.0.113.20" }] : [{ type: 28, data: "2001:db8::20" }] }); + }); + const { fetchImpl, calls } = capture(new Response("ok")); + expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 805, deliveryId: "d", rawBody: "{}" }, fetchImpl)).toBe("forwarded"); + expect(calls[0]?.url).toBe("https://dns-checked.example/v1/orb/relay"); + }); + it("returns FAILED (never throws) on a non-ok response or a thrown fetch — the Orb 202 always stands", async () => { const e = brokeredEnv(); const secret = await enroll(e, 802); await registerOrbRelay(e, secret, "https://c.example/v1/orb/relay"); - expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 802, deliveryId: "d", rawBody: "{}" }, (() => Promise.resolve(new Response("no", { status: 503 }))) as typeof fetch)).toBe("failed"); - expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 802, deliveryId: "d", rawBody: "{}" }, (() => Promise.reject(new Error("down"))) as typeof fetch)).toBe("failed"); + expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 802, deliveryId: "d", rawBody: "{}" }, (() => Promise.resolve(new Response("no", { status: 503 }))) as typeof fetch, publicDns)).toBe("failed"); + expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 802, deliveryId: "d", rawBody: "{}" }, (() => Promise.reject(new Error("down"))) as typeof fetch, publicDns)).toBe("failed"); + }); + + it("rejects a registered relay destination that resolves to loopback/private or has no DNS answers", async () => { + const e = brokeredEnv(); + const secret = await enroll(e, 804); + await registerOrbRelay(e, secret, "https://rebinding.example/v1/orb/relay"); + const { fetchImpl, calls } = capture(new Response("ok")); + const args = { eventName: "pull_request", installationId: 804, deliveryId: "d", rawBody: "{}" }; + expect(await forwardOrbEvent(e, args, fetchImpl, async () => ["127.0.0.1"])).toBe("failed"); + expect(await forwardOrbEvent(e, args, fetchImpl, async () => ["10.0.0.5"])).toBe("failed"); + expect(await forwardOrbEvent(e, args, fetchImpl, async () => [])).toBe("failed"); + expect(calls).toHaveLength(0); }); it("SKIPS when the server's encryption secret is gone (can't decrypt the stored secret)", async () => { From 33e252850feb3ba4149103308986767fa53bd956 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:50:59 -0700 Subject: [PATCH 2/2] fix(orb): thread resolveHostname through retryFailedRelays to forwardOrbEvent --- src/orb/relay.ts | 3 ++- test/integration/orb-relay.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/orb/relay.ts b/src/orb/relay.ts index f6d536416..30a0168eb 100644 --- a/src/orb/relay.ts +++ b/src/orb/relay.ts @@ -134,7 +134,7 @@ export async function storeRelayFailure( * Each row gets up to RELAY_RETRY_MAX_ATTEMPTS (5) retries within a 1-hour TTL; on success or expiry the row * is removed. Never throws — a bad DB row or a persistently-down container is dropped (with an alertable log, * below) after exhaustion. */ -export async function retryFailedRelays(env: Env, opts?: { fetchImpl?: typeof fetch }): Promise { +export async function retryFailedRelays(env: Env, opts?: { fetchImpl?: typeof fetch; resolveHostname?: RelayDnsResolver }): Promise { // Prune rows whose TTL has elapsed or whose attempt budget is exhausted. const pruned = await env.DB .prepare("DELETE FROM orb_relay_failures WHERE expires_at < datetime('now') OR attempts >= ?") @@ -159,6 +159,7 @@ export async function retryFailedRelays(env: Env, opts?: { fetchImpl?: typeof fe env, { eventName: row.event_name, installationId: row.installation_id, deliveryId: row.delivery_id, rawBody: row.raw_body }, opts?.fetchImpl, + opts?.resolveHostname, ); if (result === "forwarded" || result === "skipped") { await env.DB.prepare("DELETE FROM orb_relay_failures WHERE delivery_id = ?").bind(row.delivery_id).run(); diff --git a/test/integration/orb-relay.test.ts b/test/integration/orb-relay.test.ts index 3f9acc36e..94f4f6fce 100644 --- a/test/integration/orb-relay.test.ts +++ b/test/integration/orb-relay.test.ts @@ -254,7 +254,7 @@ describe("retryFailedRelays", () => { await registerOrbRelay(e, secret, "https://c.example/v1/orb/relay"); await storeRelayFailure(e, { deliveryId: "retry-ok", eventName: "pull_request", installationId: 9100, rawBody: "{}" }); const fetchOk = (() => Promise.resolve(new Response("ok", { status: 200 }))) as typeof fetch; - await retryFailedRelays(e, { fetchImpl: fetchOk }); + await retryFailedRelays(e, { fetchImpl: fetchOk, resolveHostname: async () => ["203.0.113.10"] }); const row = await db(e).prepare("SELECT delivery_id FROM orb_relay_failures WHERE delivery_id='retry-ok'").first(); expect(row ?? null).toBeNull(); // row removed on success }); @@ -265,7 +265,7 @@ describe("retryFailedRelays", () => { await registerOrbRelay(e, secret, "https://c.example/v1/orb/relay"); await storeRelayFailure(e, { deliveryId: "retry-fail", eventName: "pull_request", installationId: 9101, rawBody: "{}" }); const fetchFail = (() => Promise.resolve(new Response("bad", { status: 503 }))) as typeof fetch; - await retryFailedRelays(e, { fetchImpl: fetchFail }); + await retryFailedRelays(e, { fetchImpl: fetchFail, resolveHostname: async () => ["203.0.113.10"] }); const row = await db(e).prepare("SELECT attempts FROM orb_relay_failures WHERE delivery_id='retry-fail'").first<{ attempts: number }>(); expect(row?.attempts).toBe(1); }); @@ -300,7 +300,7 @@ describe("retryFailedRelays", () => { return new Response("bad", { status: 503 }); }) as typeof fetch; - await retryFailedRelays(e, { fetchImpl: fetchFail }); + await retryFailedRelays(e, { fetchImpl: fetchFail, resolveHostname: async () => ["203.0.113.10"] }); expect(calls).toBe(25); expect(maxActive).toBeLessThanOrEqual(5);