diff --git a/src/oauth/callback-server.ts b/src/oauth/callback-server.ts index 3bccea7d..d99836e6 100644 --- a/src/oauth/callback-server.ts +++ b/src/oauth/callback-server.ts @@ -1,6 +1,10 @@ -// src/oauth/callback-server.ts — CLI fallback local callback server for PKCE OAuth flows. -// Primary path: the GUI server handles /oauth/callback when the UI is open. -// This is only used when running `clodex providers auth ` without the GUI. +// src/oauth/callback-server.ts — loopback callback server for PKCE OAuth flows. +// It serves the callback path the caller registered with the provider, on the loopback port the +// caller supplies, and hands the authorization code back to the flow. Today its only caller is +// browser sign-in (`clodex providers auth openai --browser`, or the Browser option in the +// interactive picker), and nothing else in src/ receives an OAuth callback. The UI server that +// used to own this module's job, for the since-removed Antigravity flow, went with the strip to +// the Claude-to-OpenAI bridge (d01d0eb). import http from 'node:http'; import { listenTcpServer, waitForTcpListener } from '../listener-ready.js'; diff --git a/src/oauth/openai.ts b/src/oauth/openai.ts index 3aa9c0e7..fcc3a4ea 100644 --- a/src/oauth/openai.ts +++ b/src/oauth/openai.ts @@ -191,8 +191,9 @@ export async function runOpenAiBrowserFlow( } catch (error) { if ((error as NodeJS.ErrnoException).code === 'EADDRINUSE') { throw new Error( - `Ports ${ports.join(' and ')} are in use — close any other OpenAI sign-in ` - + '(e.g. codex login) and try again.', + `Ports ${ports.join(' and ')} are in use — browser sign-in needs one free. ` + + 'Check for another OpenAI sign-in (e.g. `codex login`), or any other process ' + + 'holding them, then try again.', ); } throw new Error( @@ -206,7 +207,10 @@ export async function runOpenAiBrowserFlow( const params = await server.waitForCallback(opts?.timeoutMs); if (params.error) throw new Error(`OpenAI sign-in failed: ${params.error}`); if (!params.code) throw new Error('OpenAI sign-in returned no authorization code'); - // Defense in depth: the callback server already filters on expectedState. + // Unreachable as written: this flow always passes expectedState, so the callback server + // answers 400 and keeps waiting rather than delivering a mismatched state here. Kept as a + // backstop in case this flow ever stops passing it — but it is not what protects the flow + // today, so do not weaken the server-side check on the strength of this one. if (params.state !== state) { throw new Error('OpenAI sign-in returned a mismatched state — try again'); } diff --git a/tests/oauth-openai.test.ts b/tests/oauth-openai.test.ts index 02cc94f7..27737d86 100644 --- a/tests/oauth-openai.test.ts +++ b/tests/oauth-openai.test.ts @@ -239,8 +239,15 @@ describe('oauth/openai', () => { }), )); const busyPorts = blockers.map(srv => (srv.address() as { port: number }).port); + // Pin the message exactly, not as a substring: it must state the observed fact (the + // ports are busy) and never assert a cause we did not observe. toThrow(string) matches a + // substring, so appended copy would slip through — compare the Error for equality. await expect(runOpenAiBrowserFlow(vi.fn(), { ports: busyPorts, timeoutMs: 200 })) - .rejects.toThrow(new RegExp(`${busyPorts[0]} and ${busyPorts[1]} are in use`)); + .rejects.toThrowError(new Error( + `Ports ${busyPorts[0]} and ${busyPorts[1]} are in use — browser sign-in needs one free. ` + + 'Check for another OpenAI sign-in (e.g. `codex login`), or any other process ' + + 'holding them, then try again.', + )); } finally { for (const srv of blockers) srv.close(); dns.setDefaultResultOrder(previousOrder);