Bound OAuth token exchanges with a cancelling timeout (CL-7235)

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);
+  });
 });

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<string, string>;
-    body: string;
-  },
-) => Promise<Response>;
+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",

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);
+});

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<string, string>;
-      body: string;
-    },
-  ) => Promise<Response>;
+  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(),

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);
+});

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<string, string>;
-    body: string;
-  },
-) => Promise<Response>;
+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(),

packages/connections/src/oauth-exchange-fetch.test.ts

@@ -0,0 +1,87 @@
+// 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";
+
+// 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) =>
+    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: "" },
+      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();
+  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: "" },
+      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 () =>
+    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: "",
+  });
+});