From b555a32ee9bd30f8588aa5dec3efa43653fa7cd4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 16 Aug 2026 20:11:04 -0700 Subject: [PATCH] fix(openrouter): keep OAuth loopback open after invalid callbacks (#124922) Amp-Thread-ID: https://ampcode.com/threads/T-01a00ae0-190d-718b-8a76-b75f3e8d1fae Co-authored-by: Amp --- docs/plugins/sdk-subpaths.md | 2 +- extensions/openrouter/oauth.test.ts | 135 +++++++++++----- extensions/openrouter/oauth.ts | 197 ++++++++---------------- src/plugin-sdk/provider-auth-runtime.ts | 31 ++++ 4 files changed, 187 insertions(+), 178 deletions(-) diff --git a/docs/plugins/sdk-subpaths.md b/docs/plugins/sdk-subpaths.md index bdbababb57ab..129a5586ab6f 100644 --- a/docs/plugins/sdk-subpaths.md +++ b/docs/plugins/sdk-subpaths.md @@ -138,7 +138,7 @@ are private-local. | `plugin-sdk/provider-entry` | Private-local after July 2026; `defineSingleProviderPluginEntry` | | `plugin-sdk/provider-setup` | Private-local after July 2026; Curated local/self-hosted provider setup helpers | | `plugin-sdk/cli-backend` | Private-local after July 2026; CLI backend defaults + watchdog constants | - | `plugin-sdk/provider-auth-runtime` | Private-local after July 2026; Provider auth runtime helpers: OAuth loopback flow, token exchange, auth persistence, and API-key resolution | + | `plugin-sdk/provider-auth-runtime` | Private-local after July 2026; provider auth runtime helpers including `startProviderOAuthLoopbackCallbackServer`, token exchange, auth persistence, and API-key resolution | | `plugin-sdk/provider-oauth-runtime` | Private-local after July 2026; Generic provider OAuth callback types, callback-page rendering, PKCE/state helpers, authorization-input parsing, token-expiry helpers, and abort helpers | | `plugin-sdk/provider-auth-api-key` | Private-local after July 2026; API-key onboarding/profile-write helpers such as `upsertApiKeyProfile` | | `plugin-sdk/provider-auth-result` | Private-local after July 2026; Standard OAuth auth-result builder | diff --git a/extensions/openrouter/oauth.test.ts b/extensions/openrouter/oauth.test.ts index 98b095e830b2..7857a4989aa1 100644 --- a/extensions/openrouter/oauth.test.ts +++ b/extensions/openrouter/oauth.test.ts @@ -137,15 +137,13 @@ function createOpenRouterOAuthContext(params: { return { ctx, progress, note, text, log, openUrl }; } -async function requestLocalOpenRouterOAuthCallback( - query: string, -): Promise<{ callback: Promise; response: Response; body: string }> { +async function startLocalOpenRouterOAuthLogin() { let markReady = () => {}; const ready = new Promise((resolve) => { markReady = resolve; }); const controller = new AbortController(); - const redirectInput = `${OPENROUTER_OAUTH_REDIRECT_URI}?${query}`; + const fetchImpl = vi.fn(async () => jsonResponse({ key: "sk-or-v1-test" })); const { ctx } = createOpenRouterOAuthContext({ isRemote: false, onProgress: (message) => { @@ -153,18 +151,17 @@ async function requestLocalOpenRouterOAuthCallback( markReady(); } }, - redirectInput, signal: controller.signal, }); - const callback = loginOpenRouterOAuth(ctx, { + const login = loginOpenRouterOAuth(ctx, { createPkce: () => ({ verifier: "verifier-1", challenge: "challenge-1" }), createState: () => "state-1", - fetchImpl: vi.fn(async () => jsonResponse({ key: "sk-or-v1-test" })), + fetchImpl, }); - callback.catch(() => undefined); + login.catch(() => undefined); await Promise.race([ ready, - callback.then( + login.then( () => { throw new Error("OpenRouter OAuth completed before callback server started"); }, @@ -174,13 +171,20 @@ async function requestLocalOpenRouterOAuthCallback( ), ]); - try { - const response = await fetch(redirectInput, { headers: { Connection: "close" } }); - return { callback, response, body: await response.text() }; - } catch (error) { - controller.abort(); - throw error; - } + return { + abort: () => controller.abort(), + fetchImpl, + login, + request: async (pathOrQuery: string, init?: RequestInit) => { + const url = pathOrQuery.startsWith("http") + ? pathOrQuery + : `${OPENROUTER_OAUTH_REDIRECT_URI}?${pathOrQuery}`; + const headers = new Headers(init?.headers); + headers.set("Connection", "close"); + const response = await fetch(url, { ...init, headers }); + return { response, body: await response.text() }; + }, + }; } function runRemoteOpenRouterOAuthRedirect(redirectInput: string) { @@ -411,53 +415,102 @@ describe("OpenRouter OAuth", () => { expect(progress.stop).toHaveBeenCalledWith("OpenRouter OAuth complete"); }); - it("uses the local callback path before opening the browser locally", async () => { + it("binds the local callback before opening the browser", async () => { const fetchImpl = vi.fn(async () => jsonResponse({ key: "sk-or-v1-test" })); - const waitForCallback = vi.fn(async (_params: { expectedState: string }) => ({ + const waitForCallback = vi.fn(async () => ({ + type: "authorization_code" as const, code: "AUTHCODE", state: "state-1", })); + const close = vi.fn(async () => undefined); + const startCallback = vi.fn(async () => ({ waitForCallback, close })); const { ctx, openUrl, text } = createOpenRouterOAuthContext({ isRemote: false }); await loginOpenRouterOAuth(ctx, { createPkce: () => ({ verifier: "verifier-1", challenge: "challenge-1" }), createState: () => "state-1", fetchImpl, - waitForCallback, + startCallback, }); - expect(waitForCallback).toHaveBeenCalledWith( - expect.objectContaining({ expectedState: "state-1" }), + expect(startCallback).toHaveBeenCalledWith( + expect.objectContaining({ + expectedState: "state-1", + redirectUrl: OPENROUTER_OAUTH_REDIRECT_URI, + }), ); - - expect(waitForCallback.mock.invocationCallOrder[0]).toBeLessThan( + expect(startCallback.mock.invocationCallOrder[0]).toBeLessThan( (openUrl as ReturnType).mock.invocationCallOrder[0] ?? Number.MAX_SAFE_INTEGER, ); + expect(waitForCallback).toHaveBeenCalledTimes(1); + expect(close).toHaveBeenCalledTimes(1); expect(openUrl).toHaveBeenCalledWith(expect.stringContaining("https://openrouter.ai/auth?")); expect(text).not.toHaveBeenCalled(); }); - it("validates local callback state before surfacing OpenRouter OAuth errors", async () => { - const denied = await requestLocalOpenRouterOAuthCallback( - "state=state-1&error=access_denied&error_description=Denied", - ); - expect(denied.response.status).toBe(400); - expect(denied.body).toBe("OpenRouter authentication failed: access_denied: Denied"); - await expect(denied.callback).rejects.toThrow("OpenRouter OAuth error: access_denied: Denied"); + it("falls back to a pasted redirect when the local listener cannot start", async () => { + const fetchImpl = vi.fn(async () => jsonResponse({ key: "sk-or-v1-test" })); + const startCallback = vi.fn(async () => { + throw new Error("listen EADDRINUSE: address already in use localhost:3000"); + }); + const { ctx, openUrl, text } = createOpenRouterOAuthContext({ isRemote: false }); - const missingState = await requestLocalOpenRouterOAuthCallback( - "error=access_denied&error_description=Denied", - ); - expect(missingState.response.status).toBe(400); - expect(missingState.body).toBe("Invalid OAuth state"); - await expect(missingState.callback).rejects.toThrow("Missing OpenRouter OAuth state"); + await expect( + loginOpenRouterOAuth(ctx, { + createPkce: () => ({ verifier: "verifier-1", challenge: "challenge-1" }), + createState: () => "state-1", + fetchImpl, + startCallback, + }), + ).resolves.toMatchObject({ defaultModel: "openrouter/auto" }); - const wrongState = await requestLocalOpenRouterOAuthCallback( - "state=wrong&error=access_denied&error_description=Denied", + expect(startCallback).toHaveBeenCalledTimes(1); + expect(openUrl).toHaveBeenCalledTimes(1); + expect(text).toHaveBeenCalledWith( + expect.objectContaining({ message: "Paste the OpenRouter redirect URL" }), ); - expect(wrongState.response.status).toBe(400); - expect(wrongState.body).toBe("Invalid OAuth state"); - await expect(wrongState.callback).rejects.toThrow("OpenRouter OAuth state mismatch"); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it("keeps waiting after rejected callback candidates and exchanges one valid code", async () => { + const local = await startLocalOpenRouterOAuthLogin(); + try { + expect((await local.request("http://localhost:3000/wrong")).response.status).toBe(404); + expect((await local.request("", { method: "POST" })).response.status).toBe(405); + expect((await local.request("code=missing-state")).response.status).toBe(400); + expect((await local.request("state=wrong&code=wrong-state")).response.status).toBe(400); + expect((await local.request("state=state-1")).response.status).toBe(400); + + const validCallbacks = await Promise.allSettled([ + local.request("state=state-1&code=AUTHCODE"), + local.request("state=state-1&code=REPLAY"), + ]); + const statuses = validCallbacks.flatMap((result) => + result.status === "fulfilled" ? [result.value.response.status] : [], + ); + expect(statuses.filter((status) => status === 200)).toHaveLength(1); + await expect(local.login).resolves.toMatchObject({ defaultModel: "openrouter/auto" }); + expect(local.fetchImpl).toHaveBeenCalledTimes(1); + } finally { + local.abort(); + await local.login.catch(() => undefined); + } + }); + + it("terminates a state-bound provider denial without exchanging a code", async () => { + const local = await startLocalOpenRouterOAuthLogin(); + try { + const denied = await local.request( + "state=state-1&error=access_denied&error_description=Denied", + ); + expect(denied.response.status).toBe(400); + expect(denied.body).toBe("Authorization was not completed."); + await expect(local.login).rejects.toThrow("OpenRouter OAuth error: access_denied: Denied"); + expect(local.fetchImpl).not.toHaveBeenCalled(); + } finally { + local.abort(); + await local.login.catch(() => undefined); + } }); it("exposes stable auth choice metadata", () => { diff --git a/extensions/openrouter/oauth.ts b/extensions/openrouter/oauth.ts index 206435d7f2c0..c60b00011def 100644 --- a/extensions/openrouter/oauth.ts +++ b/extensions/openrouter/oauth.ts @@ -1,5 +1,4 @@ // OpenRouter OAuth support exchanges PKCE browser login codes for API keys. -import { createServer } from "node:http"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; import type { ProviderAuthContext, ProviderAuthMethod } from "openclaw/plugin-sdk/plugin-entry"; import { @@ -7,7 +6,10 @@ import { generatePkceVerifierChallenge, type ProviderAuthResult, } from "openclaw/plugin-sdk/provider-auth"; -import { generateOAuthState } from "openclaw/plugin-sdk/provider-auth-runtime"; +import { + generateOAuthState, + startProviderOAuthLoopbackCallbackServer, +} from "openclaw/plugin-sdk/provider-auth-runtime"; import { readProviderJsonResponse, readResponseTextLimited, @@ -36,6 +38,13 @@ type OpenRouterOAuthCallbackResult = { state: string; }; +type OpenRouterOAuthCallbackServer = Awaited< + ReturnType +>; +type OpenRouterOAuthLoopbackResult = Awaited< + ReturnType +>; + type OpenRouterOAuthKeyResult = { key: string; userId?: string; @@ -45,7 +54,7 @@ type OpenRouterOAuthLoginOptions = { createPkce?: () => { verifier: string; challenge: string }; createState?: () => string; fetchImpl?: typeof fetch; - waitForCallback?: typeof waitForOpenRouterOAuthCallback; + startCallback?: typeof startProviderOAuthLoopbackCallbackServer; }; function extractOpenRouterError(value: unknown): string | undefined { @@ -206,125 +215,6 @@ async function exchangeOpenRouterOAuthCode(params: { return parseOpenRouterKeyResponse(body); } -async function waitForOpenRouterOAuthCallback(params: { - expectedState: string; - timeoutMs?: number; - onProgress?: (message: string) => void; - signal?: AbortSignal; -}): Promise { - const timeoutMs = params.timeoutMs ?? OPENROUTER_OAUTH_TIMEOUT_MS; - return new Promise((resolve, reject) => { - let settled = false; - const timeout = setTimeout(() => { - finish(new Error("OpenRouter OAuth callback timeout")); - }, timeoutMs); - const server = createServer((req, res) => { - try { - const requestUrl = new URL( - req.url ?? "/", - `http://${OPENROUTER_OAUTH_CALLBACK_HOST}:${OPENROUTER_OAUTH_CALLBACK_PORT}`, - ); - if (requestUrl.pathname !== OPENROUTER_OAUTH_CALLBACK_PATH) { - res.statusCode = 404; - res.setHeader("Content-Type", "text/plain"); - res.end("Not found"); - return; - } - if (req.method !== "GET") { - res.statusCode = 405; - res.setHeader("Allow", "GET"); - res.setHeader("Content-Type", "text/plain"); - res.end("Method not allowed"); - return; - } - - const state = normalizeOptionalString(requestUrl.searchParams.get("state")); - try { - requireOpenRouterOAuthState(state, params.expectedState); - } catch (err) { - res.statusCode = 400; - res.setHeader("Content-Type", "text/plain"); - res.end("Invalid OAuth state"); - finish(err instanceof Error ? err : new Error("OpenRouter OAuth state mismatch")); - return; - } - - const error = normalizeOptionalString(requestUrl.searchParams.get("error")); - if (error) { - const description = normalizeOptionalString( - requestUrl.searchParams.get("error_description"), - ); - const detail = description ? `${error}: ${description}` : error; - res.statusCode = 400; - res.setHeader("Content-Type", "text/plain"); - res.end(`OpenRouter authentication failed: ${detail}`); - finish(new Error(`OpenRouter OAuth error: ${detail}`)); - return; - } - - const code = normalizeOptionalString(requestUrl.searchParams.get("code")); - if (!code) { - res.statusCode = 400; - res.setHeader("Content-Type", "text/plain"); - res.end("Missing OAuth code"); - finish(new Error("Missing OpenRouter OAuth code")); - return; - } - - res.statusCode = 200; - res.setHeader("Content-Type", "text/html; charset=utf-8"); - res.end( - "" + - "

OpenRouter OAuth complete

" + - "

You can close this window and return to OpenClaw.

", - ); - finish(undefined, { code, state: params.expectedState }); - } catch (err) { - finish(err instanceof Error ? err : new Error("OpenRouter OAuth callback failed")); - } - }); - - const finish = (err?: Error, result?: OpenRouterOAuthCallbackResult) => { - if (settled) { - return; - } - settled = true; - if (timeout) { - clearTimeout(timeout); - } - params.signal?.removeEventListener("abort", onAbort); - try { - server.close(); - } catch { - // Best-effort cleanup after callback completion or timeout. - } - if (err) { - reject(err); - return; - } - if (result) { - resolve(result); - } - }; - - const onAbort = () => finish(new Error("OpenRouter OAuth cancelled")); - params.signal?.addEventListener("abort", onAbort, { once: true }); - if (params.signal?.aborted) { - onAbort(); - return; - } - - server.once("error", (err) => { - finish(err instanceof Error ? err : new Error("OpenRouter OAuth callback server error")); - }); - server.listen(OPENROUTER_OAUTH_CALLBACK_PORT, OPENROUTER_OAUTH_CALLBACK_HOST, () => { - params.onProgress?.( - `Waiting for OpenRouter OAuth callback on ${OPENROUTER_OAUTH_REDIRECT_URI}...`, - ); - }); - }); -} - async function promptForOpenRouterRedirect( ctx: ProviderAuthContext, expectedState: string, @@ -342,7 +232,7 @@ async function resolveOpenRouterOAuthCode( params: { authorizeUrl: string; state: string; - waitForCallback: typeof waitForOpenRouterOAuthCallback; + startCallback: typeof startProviderOAuthLoopbackCallbackServer; onProgress: (message: string) => void; }, ): Promise { @@ -373,20 +263,30 @@ async function resolveOpenRouterOAuthCode( return await promptForOpenRouterRedirect(ctx, params.state); } - const callbackPromise = params - .waitForCallback({ + let callback: OpenRouterOAuthCallbackServer | undefined; + try { + callback = await params.startCallback({ + redirectUrl: OPENROUTER_OAUTH_REDIRECT_URI, expectedState: params.state, - onProgress: params.onProgress, + timeoutMs: OPENROUTER_OAUTH_TIMEOUT_MS, ...(ctx.signal ? { signal: ctx.signal } : {}), - }) - .catch(async (error: unknown) => { - if (ctx.signal?.aborted) { - throw error; - } - params.onProgress("OAuth callback not detected; waiting for redirect URL..."); - return { code: await promptForOpenRouterRedirect(ctx, params.state), state: params.state }; + renderSuccess: () => ({ + body: + "" + + "

OpenRouter OAuth complete

" + + "

You can close this window and return to OpenClaw.

", + contentType: "text/html; charset=utf-8", + }), }); - void callbackPromise.catch(() => undefined); + params.onProgress( + `Waiting for OpenRouter OAuth callback on ${OPENROUTER_OAUTH_REDIRECT_URI}...`, + ); + } catch (error) { + if (ctx.signal?.aborted) { + throw error; + } + params.onProgress("OAuth callback not detected; waiting for redirect URL..."); + } try { await ctx.openUrl(params.authorizeUrl); @@ -395,7 +295,32 @@ async function resolveOpenRouterOAuthCode( ctx.runtime.log(`Open manually: ${params.authorizeUrl}`); } - return (await callbackPromise).code; + if (!callback) { + return await promptForOpenRouterRedirect(ctx, params.state); + } + + let result: OpenRouterOAuthLoopbackResult; + try { + try { + result = await callback.waitForCallback(); + } finally { + await callback.close(); + } + } catch (error) { + if (ctx.signal?.aborted) { + throw error; + } + params.onProgress("OAuth callback not detected; waiting for redirect URL..."); + return await promptForOpenRouterRedirect(ctx, params.state); + } + + if (result.type === "oauth_error") { + const detail = result.errorDescription + ? `${result.error}: ${result.errorDescription}` + : result.error; + throw new Error(`OpenRouter OAuth error: ${detail}`); + } + return result.code; } async function loginOpenRouterOAuth( @@ -413,7 +338,7 @@ async function loginOpenRouterOAuth( const code = await resolveOpenRouterOAuthCode(ctx, { authorizeUrl, state, - waitForCallback: options.waitForCallback ?? waitForOpenRouterOAuthCallback, + startCallback: options.startCallback ?? startProviderOAuthLoopbackCallbackServer, onProgress: (message) => progress.update(message), }); progress.update("Exchanging OpenRouter OAuth code..."); diff --git a/src/plugin-sdk/provider-auth-runtime.ts b/src/plugin-sdk/provider-auth-runtime.ts index a6dac75d1106..78d7f2c95df8 100644 --- a/src/plugin-sdk/provider-auth-runtime.ts +++ b/src/plugin-sdk/provider-auth-runtime.ts @@ -36,6 +36,37 @@ export type OAuthCallbackResult = { state: string; }; +type ProviderOAuthLoopbackCallbackResult = + | { type: "authorization_code"; code: string; state: string } + | { type: "oauth_error"; error: string; errorDescription?: string }; + +type ProviderOAuthLoopbackCallbackServer = { + waitForCallback: () => Promise; + close: () => Promise; +}; + +type ProviderOAuthLoopbackRenderedResponse = { body: string; contentType: string }; +type ProviderOAuthLoopbackCorsOriginResolver = ( + originHeader: string | string[] | undefined, +) => string | undefined; + +/** + * Binds a hardened loopback listener before returning so provider plugins can open the browser + * only after the callback route is ready. Invalid request candidates remain nonterminal. + */ +export async function startProviderOAuthLoopbackCallbackServer(params: { + redirectUrl: string | URL; + expectedState: string; + timeoutMs: number; + signal?: AbortSignal; + bindHostname?: string; + resolveCorsOrigin?: ProviderOAuthLoopbackCorsOriginResolver; + renderSuccess?: () => ProviderOAuthLoopbackRenderedResponse; + renderError?: (message: string) => ProviderOAuthLoopbackRenderedResponse; +}): Promise { + return await startOAuthLoopbackCallbackServer(params); +} + /** * Non-secret auth profile metadata used by provider discovery helpers. */