From 14ff425db91d54836ad3f5143abd2f02859e74e3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 30 Aug 2026 05:46:17 -0700 Subject: [PATCH 1/2] Add tests for bounded OAuth exchange timeouts The four OAuth code-for-token exchange functions (github, gmail, huggingface, openrouter) fetch their provider's token endpoint with no signal at all -- a provider that never answers hangs the exchange (and the /callback request awaiting it) forever. These tests currently fail: they assert each exchange wires a bounded AbortSignal into its fetch, and cover a shared helper's cancel-not-abandon behavior directly. --- .../connections/src/github-connect.test.ts | 27 ++++++ .../connections/src/gmail-connect.test.ts | 28 ++++++ .../src/huggingface-connect.test.ts | 86 +++++++++++++++++++ .../src/oauth-exchange-fetch.test.ts | 82 ++++++++++++++++++ .../src/openrouter-connect.test.ts | 76 ++++++++++++++++ 5 files changed, 299 insertions(+) create mode 100644 packages/connections/src/huggingface-connect.test.ts create mode 100644 packages/connections/src/oauth-exchange-fetch.test.ts create mode 100644 packages/connections/src/openrouter-connect.test.ts diff --git a/packages/connections/src/github-connect.test.ts b/packages/connections/src/github-connect.test.ts index 9b4ce47ab..f7f7f9b73 100644 --- a/packages/connections/src/github-connect.test.ts +++ b/packages/connections/src/github-connect.test.ts @@ -119,4 +119,31 @@ describe("exchangeCodeForGithubToken", () => { expect(result.message).toContain("getaddrinfo ENOTFOUND"); } }); + + // CL-7235: a GitHub token endpoint that never answers used to leave + // this exchange awaiting `doFetch` forever. It now carries a bounded + // `AbortSignal`, so a stalled provider is caught the same way any + // other network failure already is instead of hanging the `/callback` + // request indefinitely. + test("wires a bounded AbortSignal into the exchange fetch so a stalled provider can't hang the exchange", async () => { + let capturedSignal: AbortSignal | undefined; + const fetchImpl: ExchangeFetch = (_url, init) => { + capturedSignal = init.signal; + return new Promise(() => { + // never resolves -- a provider that never answers. + }); + }; + + void exchangeCodeForGithubToken({ + code: "auth_code_1", + redirectUri: "https://hub.example.test/callback", + clientId: "client_1", + clientSecret: "secret_1", + fetchImpl, + }); + await Promise.resolve(); + + expect(capturedSignal).toBeInstanceOf(AbortSignal); + expect(capturedSignal?.aborted).toBe(false); + }); }); diff --git a/packages/connections/src/gmail-connect.test.ts b/packages/connections/src/gmail-connect.test.ts index ca59f32bb..e2b95968a 100644 --- a/packages/connections/src/gmail-connect.test.ts +++ b/packages/connections/src/gmail-connect.test.ts @@ -7,6 +7,7 @@ import { expect, test } from "bun:test"; import { exchangeCodeForGoogleToken, GOOGLE_TOKEN_EXCHANGE_URL, + type ExchangeFetch, } from "./gmail-connect"; function stubFetch( @@ -89,3 +90,30 @@ test("a Google error response maps to an honest failure that never echoes token expect(result.message).toContain("invalid_grant"); expect(result.message).not.toContain("secret-1"); }); + +// CL-7235: a Google token endpoint that never answers used to leave this +// exchange awaiting `doFetch` forever. It now carries a bounded +// `AbortSignal`, so a stalled provider is caught the same way any other +// network failure already is instead of hanging the `/callback` request +// indefinitely. +test("wires a bounded AbortSignal into the exchange fetch so a stalled provider can't hang the exchange", async () => { + let capturedSignal: AbortSignal | undefined; + const fetchImpl: ExchangeFetch = (_url, init) => { + capturedSignal = init.signal; + return new Promise(() => { + // never resolves -- a provider that never answers. + }); + }; + + void exchangeCodeForGoogleToken({ + code: "auth-code-1", + redirectUri: "https://bench.example.com/callback", + clientId: "client-1", + clientSecret: "secret-1", + fetchImpl, + }); + await Promise.resolve(); + + expect(capturedSignal).toBeInstanceOf(AbortSignal); + expect(capturedSignal?.aborted).toBe(false); +}); diff --git a/packages/connections/src/huggingface-connect.test.ts b/packages/connections/src/huggingface-connect.test.ts new file mode 100644 index 000000000..cbde44b49 --- /dev/null +++ b/packages/connections/src/huggingface-connect.test.ts @@ -0,0 +1,86 @@ +// Unit tests for the Hugging Face connector's code-for-token exchange, +// driven entirely against a stubbed fetch -- no Hugging Face credentials +// involved. +import { expect, test } from "bun:test"; + +import { + exchangeCodeForToken, + HUGGINGFACE_TOKEN_URL, + type ExchangeFetch, +} from "./huggingface-connect"; + +test("exchanges the code and verifier for an access token and expiry", async () => { + const requests: { url: string; body: string }[] = []; + const fetchImpl: ExchangeFetch = async (url, init) => { + requests.push({ url, body: init.body }); + return new Response( + JSON.stringify({ access_token: "hf_minted_token", expires_in: 3600 }), + { status: 200 }, + ); + }; + + const result = await exchangeCodeForToken({ + code: "auth_code_1", + codeVerifier: "verifier_1", + redirectUri: "https://hub.example.test/callback", + clientId: "client_1", + fetchImpl, + now: () => 1_000_000, + }); + + expect(result).toEqual({ + ok: true, + accessToken: "hf_minted_token", + expiresAt: new Date(1_000_000 + 3600 * 1000).toISOString(), + }); + expect(requests[0]?.url).toBe(HUGGINGFACE_TOKEN_URL); + const params = new URLSearchParams(requests[0]?.body ?? ""); + expect(params.get("code")).toBe("auth_code_1"); + expect(params.get("code_verifier")).toBe("verifier_1"); +}); + +test("a transport failure is reported honestly, never as a token", async () => { + const fetchImpl: ExchangeFetch = async () => { + throw new Error("getaddrinfo ENOTFOUND"); + }; + + const result = await exchangeCodeForToken({ + code: "auth_code_1", + codeVerifier: "verifier_1", + redirectUri: "https://hub.example.test/callback", + clientId: "client_1", + fetchImpl, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).toContain("Could not reach Hugging Face"); + } +}); + +// CL-7235: a Hugging Face token endpoint that never answers used to +// leave this exchange awaiting `doFetch` forever. It now carries a +// bounded `AbortSignal`, so a stalled provider is caught the same way +// any other network failure already is instead of hanging the +// `/callback` request indefinitely. +test("wires a bounded AbortSignal into the exchange fetch so a stalled provider can't hang the exchange", async () => { + let capturedSignal: AbortSignal | undefined; + const fetchImpl: ExchangeFetch = (_url, init) => { + capturedSignal = init.signal; + return new Promise(() => { + // never resolves -- a provider that never answers. + }); + }; + + void exchangeCodeForToken({ + code: "auth_code_1", + codeVerifier: "verifier_1", + redirectUri: "https://hub.example.test/callback", + clientId: "client_1", + fetchImpl, + }); + await Promise.resolve(); + + expect(capturedSignal).toBeInstanceOf(AbortSignal); + expect(capturedSignal?.aborted).toBe(false); +}); diff --git a/packages/connections/src/oauth-exchange-fetch.test.ts b/packages/connections/src/oauth-exchange-fetch.test.ts new file mode 100644 index 000000000..1d0dca7d7 --- /dev/null +++ b/packages/connections/src/oauth-exchange-fetch.test.ts @@ -0,0 +1,82 @@ +// Every OAuth connect module (github-connect.ts, gmail-connect.ts, +// huggingface-connect.ts, openrouter-connect.ts) routes its outbound +// token/key exchange through `postExchangeRequest`. These tests exercise +// that shared mechanism directly, standing in for a per-provider hang +// test: proving the timeout cancels an in-flight request (aborts the +// fetch a real implementation is waiting on) rather than racing a timer +// while the request keeps running unobserved. +import { expect, test } from "bun:test"; + +import { + OAUTH_EXCHANGE_TIMEOUT_MS, + postExchangeRequest, + type OAuthExchangeFetch, +} from "./oauth-exchange-fetch"; + +test("a provider that never answers is aborted once the timeout fires, not left hanging", async () => { + let sawAbort = false; + const fetchImpl: OAuthExchangeFetch = (_url, init) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener("abort", () => { + sawAbort = true; + reject(init.signal?.reason); + }); + }); + + await expect( + postExchangeRequest( + fetchImpl, + "https://provider.example.test/token", + { method: "POST", headers: {}, body: "" }, + 5, + ), + ).rejects.toBeDefined(); + expect(sawAbort).toBe(true); +}); + +test("passes the caller's timeoutMs through to the abort signal, not the default", async () => { + const start = performance.now(); + const fetchImpl: OAuthExchangeFetch = (_url, init) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener("abort", () => reject(init.signal?.reason)); + }); + + await expect( + postExchangeRequest( + fetchImpl, + "https://provider.example.test/token", + { method: "POST", headers: {}, body: "" }, + 5, + ), + ).rejects.toBeDefined(); + + expect(performance.now() - start).toBeLessThan(OAUTH_EXCHANGE_TIMEOUT_MS); +}); + +test("a provider that answers before the timeout resolves normally", async () => { + const fetchImpl: OAuthExchangeFetch = async () => + new Response(JSON.stringify({ ok: true }), { status: 200 }); + + const response = await postExchangeRequest( + fetchImpl, + "https://provider.example.test/token", + { method: "POST", headers: {}, body: "" }, + 5, + ); + + expect(response.status).toBe(200); +}); + +test("defaults to OAUTH_EXCHANGE_TIMEOUT_MS when no timeoutMs is given", async () => { + const fetchImpl: OAuthExchangeFetch = (_url, init) => { + expect(init.signal).toBeInstanceOf(AbortSignal); + expect(init.signal?.aborted).toBe(false); + return Promise.resolve(new Response(null, { status: 200 })); + }; + + await postExchangeRequest(fetchImpl, "https://provider.example.test/token", { + method: "POST", + headers: {}, + body: "", + }); +}); diff --git a/packages/connections/src/openrouter-connect.test.ts b/packages/connections/src/openrouter-connect.test.ts new file mode 100644 index 000000000..679b9bf96 --- /dev/null +++ b/packages/connections/src/openrouter-connect.test.ts @@ -0,0 +1,76 @@ +// Unit tests for the OpenRouter connector's code-for-key exchange, +// driven entirely against a stubbed fetch -- no OpenRouter credentials +// involved. +import { expect, test } from "bun:test"; + +import { + exchangeCodeForKey, + OPENROUTER_KEY_EXCHANGE_URL, + type ExchangeFetch, +} from "./openrouter-connect"; + +test("exchanges the code and verifier for the user-scoped API key", async () => { + const requests: { url: string; body: unknown }[] = []; + const fetchImpl: ExchangeFetch = async (url, init) => { + requests.push({ url, body: JSON.parse(init.body) }); + return new Response(JSON.stringify({ key: "sk-or-minted" }), { + status: 200, + }); + }; + + const result = await exchangeCodeForKey({ + code: "auth_code_1", + codeVerifier: "verifier_1", + fetchImpl, + }); + + expect(result).toEqual({ ok: true, key: "sk-or-minted" }); + expect(requests[0]?.url).toBe(OPENROUTER_KEY_EXCHANGE_URL); + expect(requests[0]?.body).toEqual({ + code: "auth_code_1", + code_verifier: "verifier_1", + code_challenge_method: "S256", + }); +}); + +test("a transport failure is reported honestly, never as a key", async () => { + const fetchImpl: ExchangeFetch = async () => { + throw new Error("getaddrinfo ENOTFOUND"); + }; + + const result = await exchangeCodeForKey({ + code: "auth_code_1", + codeVerifier: "verifier_1", + fetchImpl, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).toContain("Could not reach OpenRouter"); + } +}); + +// CL-7235: an OpenRouter key endpoint that never answers used to leave +// this exchange awaiting `doFetch` forever. It now carries a bounded +// `AbortSignal`, so a stalled provider is caught the same way any other +// network failure already is instead of hanging the `/callback` request +// indefinitely. +test("wires a bounded AbortSignal into the exchange fetch so a stalled provider can't hang the exchange", async () => { + let capturedSignal: AbortSignal | undefined; + const fetchImpl: ExchangeFetch = (_url, init) => { + capturedSignal = init.signal; + return new Promise(() => { + // never resolves -- a provider that never answers. + }); + }; + + void exchangeCodeForKey({ + code: "auth_code_1", + codeVerifier: "verifier_1", + fetchImpl, + }); + await Promise.resolve(); + + expect(capturedSignal).toBeInstanceOf(AbortSignal); + expect(capturedSignal?.aborted).toBe(false); +}); From 3bd86f041216c183165c04be1d68a59f6f53d1d8 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 30 Aug 2026 06:01:17 -0700 Subject: [PATCH 2/2] Bound OAuth token exchanges with a cancelling timeout The four OAuth connect modules each posted once to their provider's token endpoint with no timeout, so an unresponsive provider hung the exchange indefinitely. postExchangeRequest wraps that single call in AbortSignal.timeout, which aborts the request itself rather than racing a timer and leaving the fetch running unobserved. This mirrors the timeout already applied to every outbound call in probes.ts, and replaces four copies of the same wiring with one call site. --- packages/connections/src/github-connect.ts | 16 ++++----- packages/connections/src/gmail-connect.ts | 20 ++++++----- .../connections/src/huggingface-connect.ts | 16 ++++----- .../src/oauth-exchange-fetch.test.ts | 13 ++++--- .../connections/src/oauth-exchange-fetch.ts | 36 +++++++++++++++++++ .../connections/src/openrouter-connect.ts | 16 ++++----- 6 files changed, 77 insertions(+), 40 deletions(-) create mode 100644 packages/connections/src/oauth-exchange-fetch.ts diff --git a/packages/connections/src/github-connect.ts b/packages/connections/src/github-connect.ts index 0ce570c0f..9992106be 100644 --- a/packages/connections/src/github-connect.ts +++ b/packages/connections/src/github-connect.ts @@ -13,6 +13,11 @@ import { type } from "arktype"; +import { + postExchangeRequest, + type OAuthExchangeFetch, +} from "./oauth-exchange-fetch"; + export const GITHUB_AUTHORIZE_URL = "https://github.com/login/oauth/authorize"; export const GITHUB_TOKEN_EXCHANGE_URL = "https://github.com/login/oauth/access_token"; @@ -27,14 +32,7 @@ export type ExchangeResult = | { readonly ok: true; readonly key: string } | { readonly ok: false; readonly message: string }; -export type ExchangeFetch = ( - url: string, - init: { - method: "POST"; - headers: Record; - body: string; - }, -) => Promise; +export type ExchangeFetch = OAuthExchangeFetch; export type ExchangeCodeForGithubTokenArgs = { readonly code: string; @@ -55,7 +53,7 @@ export async function exchangeCodeForGithubToken( const doFetch = args.fetchImpl ?? fetch; let response: Response; try { - response = await doFetch(GITHUB_TOKEN_EXCHANGE_URL, { + response = await postExchangeRequest(doFetch, GITHUB_TOKEN_EXCHANGE_URL, { method: "POST", headers: { "content-type": "application/json", diff --git a/packages/connections/src/gmail-connect.ts b/packages/connections/src/gmail-connect.ts index b337fbf19..4adbe66b0 100644 --- a/packages/connections/src/gmail-connect.ts +++ b/packages/connections/src/gmail-connect.ts @@ -12,6 +12,11 @@ import { type } from "arktype"; +import { + postExchangeRequest, + type OAuthExchangeFetch, +} from "./oauth-exchange-fetch"; + export const GOOGLE_AUTHORIZE_URL = "https://accounts.google.com/o/oauth2/v2/auth"; export const GOOGLE_TOKEN_EXCHANGE_URL = "https://oauth2.googleapis.com/token"; @@ -38,20 +43,17 @@ export type GoogleExchangeResult = } | { readonly ok: false; readonly message: string }; +/** Re-exported for symmetry with the other three connect modules and so + * tests can type a stub `fetchImpl` explicitly. */ +export type ExchangeFetch = OAuthExchangeFetch; + export type ExchangeCodeForGoogleTokenArgs = { readonly code: string; readonly codeVerifier?: string; readonly redirectUri: string; readonly clientId: string; readonly clientSecret: string; - readonly fetchImpl?: ( - url: string, - init: { - method: "POST"; - headers: Record; - body: string; - }, - ) => Promise; + readonly fetchImpl?: ExchangeFetch; }; /** @@ -77,7 +79,7 @@ export async function exchangeCodeForGoogleToken( let response: Response; try { - response = await doFetch(GOOGLE_TOKEN_EXCHANGE_URL, { + response = await postExchangeRequest(doFetch, GOOGLE_TOKEN_EXCHANGE_URL, { method: "POST", headers: { "content-type": "application/x-www-form-urlencoded" }, body: params.toString(), diff --git a/packages/connections/src/huggingface-connect.ts b/packages/connections/src/huggingface-connect.ts index 977d0b1df..45d3bbcac 100644 --- a/packages/connections/src/huggingface-connect.ts +++ b/packages/connections/src/huggingface-connect.ts @@ -15,6 +15,11 @@ import { type } from "arktype"; +import { + postExchangeRequest, + type OAuthExchangeFetch, +} from "./oauth-exchange-fetch"; + export const HUGGINGFACE_AUTHORIZE_URL = "https://huggingface.co/oauth/authorize"; export const HUGGINGFACE_TOKEN_URL = "https://huggingface.co/oauth/token"; @@ -44,14 +49,7 @@ export type ExchangeResult = } | { readonly ok: false; readonly message: string }; -export type ExchangeFetch = ( - url: string, - init: { - method: "POST"; - headers: Record; - body: string; - }, -) => Promise; +export type ExchangeFetch = OAuthExchangeFetch; export type ExchangeCodeForTokenArgs = { readonly code: string; @@ -84,7 +82,7 @@ export async function exchangeCodeForToken( let response: Response; try { - response = await doFetch(HUGGINGFACE_TOKEN_URL, { + response = await postExchangeRequest(doFetch, HUGGINGFACE_TOKEN_URL, { method: "POST", headers: { "content-type": "application/x-www-form-urlencoded" }, body: body.toString(), diff --git a/packages/connections/src/oauth-exchange-fetch.test.ts b/packages/connections/src/oauth-exchange-fetch.test.ts index 1d0dca7d7..fbf6fefe2 100644 --- a/packages/connections/src/oauth-exchange-fetch.test.ts +++ b/packages/connections/src/oauth-exchange-fetch.test.ts @@ -13,6 +13,11 @@ import { type OAuthExchangeFetch, } from "./oauth-exchange-fetch"; +// The two real-timer tests below use a small but not razor-thin +// `timeoutMs` (100ms) and a generous per-test timeout (20s) so they +// prove the abort actually fires without false-failing on a busy CI +// runner -- the assertion is "clearly bounded, not that it happened in +// under a handful of milliseconds." test("a provider that never answers is aborted once the timeout fires, not left hanging", async () => { let sawAbort = false; const fetchImpl: OAuthExchangeFetch = (_url, init) => @@ -28,11 +33,11 @@ test("a provider that never answers is aborted once the timeout fires, not left fetchImpl, "https://provider.example.test/token", { method: "POST", headers: {}, body: "" }, - 5, + 100, ), ).rejects.toBeDefined(); expect(sawAbort).toBe(true); -}); +}, 20_000); test("passes the caller's timeoutMs through to the abort signal, not the default", async () => { const start = performance.now(); @@ -46,12 +51,12 @@ test("passes the caller's timeoutMs through to the abort signal, not the default fetchImpl, "https://provider.example.test/token", { method: "POST", headers: {}, body: "" }, - 5, + 100, ), ).rejects.toBeDefined(); expect(performance.now() - start).toBeLessThan(OAUTH_EXCHANGE_TIMEOUT_MS); -}); +}, 20_000); test("a provider that answers before the timeout resolves normally", async () => { const fetchImpl: OAuthExchangeFetch = async () => diff --git a/packages/connections/src/oauth-exchange-fetch.ts b/packages/connections/src/oauth-exchange-fetch.ts new file mode 100644 index 000000000..7c84cd9fb --- /dev/null +++ b/packages/connections/src/oauth-exchange-fetch.ts @@ -0,0 +1,36 @@ +// Shared code-for-token/key exchange fetch for every OAuth connect +// module (github-connect.ts, gmail-connect.ts, huggingface-connect.ts, +// openrouter-connect.ts). Each posts once to its provider's token +// endpoint and must not hang forever if that provider never answers -- +// this mirrors the `AbortSignal.timeout(...)` already applied to every +// outbound call in `./probes.ts`, so one shared call site replaces four +// separate copies of the same timeout wiring. The signal aborts the +// underlying request the instant the timer fires; it never races a +// timer while leaving the fetch itself running unobserved. + +export const OAUTH_EXCHANGE_TIMEOUT_MS = 10_000; + +export type OAuthExchangeFetch = ( + url: string, + init: { + method: "POST"; + headers: Record; + body: string; + signal?: AbortSignal; + }, +) => Promise; + +/** + * Posts to an OAuth token/key exchange endpoint bounded by `timeoutMs` + * (default `OAUTH_EXCHANGE_TIMEOUT_MS`). A timeout rejects the same way + * any other network failure does, so callers keep their existing + * catch-and-convert-to-`{ ok: false, message }` handling unchanged. + */ +export async function postExchangeRequest( + fetchImpl: OAuthExchangeFetch, + url: string, + init: { method: "POST"; headers: Record; body: string }, + timeoutMs: number = OAUTH_EXCHANGE_TIMEOUT_MS, +): Promise { + return fetchImpl(url, { ...init, signal: AbortSignal.timeout(timeoutMs) }); +} diff --git a/packages/connections/src/openrouter-connect.ts b/packages/connections/src/openrouter-connect.ts index cd432557d..e87b09126 100644 --- a/packages/connections/src/openrouter-connect.ts +++ b/packages/connections/src/openrouter-connect.ts @@ -8,6 +8,11 @@ import { type } from "arktype"; +import { + postExchangeRequest, + type OAuthExchangeFetch, +} from "./oauth-exchange-fetch"; + export const OPENROUTER_AUTH_URL = "https://openrouter.ai/auth"; export const OPENROUTER_KEY_EXCHANGE_URL = "https://openrouter.ai/api/v1/auth/keys"; @@ -22,14 +27,7 @@ export type ExchangeResult = | { readonly ok: true; readonly key: string } | { readonly ok: false; readonly message: string }; -export type ExchangeFetch = ( - url: string, - init: { - method: "POST"; - headers: Record; - body: string; - }, -) => Promise; +export type ExchangeFetch = OAuthExchangeFetch; export type ExchangeCodeForKeyArgs = { readonly code: string; @@ -49,7 +47,7 @@ export async function exchangeCodeForKey( const doFetch = args.fetchImpl ?? fetch; let response: Response; try { - response = await doFetch(OPENROUTER_KEY_EXCHANGE_URL, { + response = await postExchangeRequest(doFetch, OPENROUTER_KEY_EXCHANGE_URL, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({