From 4ba9df54251baa60c13f146820f06076c585aa45 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 10 May 2026 15:47:05 -0400 Subject: [PATCH 1/3] fix(cliproxy/auth): generalize Plus OAuth credential guards for gemini and agy Refactor getGeminiPlusOAuthCredentialError / getGeminiAuthUrlCredentialError behind a provider-table-driven helper (PLUS_OAUTH_ENV_BY_PROVIDER) and add getPlusOAuthCredentialError / getPlusAuthUrlCredentialError exports covering both gemini and agy. Existing gemini-named exports preserved as aliases so PR #1131 test surface remains unchanged. New AGY-parity test cases mirror the original gemini diagnostics. CLI call sites in triggerOAuth / handlePasteCallbackMode stay gemini-only; dashboard handler in the follow-up commits will use the generalized exports for both providers. Refs #1208 --- .../oauth-handler-paste-callback.test.ts | 62 +++++++++ src/cliproxy/auth/oauth-handler.ts | 123 +++++++++++++++--- 2 files changed, 166 insertions(+), 19 deletions(-) diff --git a/src/cliproxy/auth/__tests__/oauth-handler-paste-callback.test.ts b/src/cliproxy/auth/__tests__/oauth-handler-paste-callback.test.ts index 025e03fc..33f62b38 100644 --- a/src/cliproxy/auth/__tests__/oauth-handler-paste-callback.test.ts +++ b/src/cliproxy/auth/__tests__/oauth-handler-paste-callback.test.ts @@ -143,6 +143,68 @@ describe('Gemini Plus OAuth credential diagnostics', () => { }); }); +describe('Antigravity Plus OAuth credential diagnostics', () => { + it('fails fast when AGY uses Plus without CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_ID/SECRET', async () => { + const { getPlusOAuthCredentialError } = await import( + `../oauth-handler?agy-plus-missing-env=${Date.now()}` + ); + + const error = getPlusOAuthCredentialError('agy', 'plus', {}); + + expect(error).toContain('Antigravity OAuth from CLIProxy Plus is missing'); + expect(error).toContain('CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_ID'); + expect(error).toContain('CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_SECRET'); + expect(error).toContain('Antigravity'); + }); + + it('allows AGY Plus when both AGY OAuth client env values exist', async () => { + const { getPlusOAuthCredentialError } = await import( + `../oauth-handler?agy-plus-env-present=${Date.now()}` + ); + + expect( + getPlusOAuthCredentialError('agy', 'plus', { + CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_ID: 'client-id', + CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_SECRET: 'client-secret', + }) + ).toBeNull(); + }); + + it('does not warn for AGY on the original backend', async () => { + const { getPlusOAuthCredentialError } = await import( + `../oauth-handler?agy-original-backend=${Date.now()}` + ); + + expect(getPlusOAuthCredentialError('agy', 'original', {})).toBeNull(); + }); + + it('detects AGY auth URLs missing client_id before display', async () => { + const { getPlusAuthUrlCredentialError } = await import( + `../oauth-handler?agy-auth-url-missing-client=${Date.now()}` + ); + + const error = getPlusAuthUrlCredentialError( + 'agy', + 'https://accounts.google.com/o/oauth2/v2/auth?client_id=&redirect_uri=http%3A%2F%2Flocalhost%3A8085%2Foauth2callback&state=test' + ); + + expect(error).toContain('Antigravity OAuth from CLIProxy Plus is missing'); + }); + + it('allows AGY auth URLs with client_id present', async () => { + const { getPlusAuthUrlCredentialError } = await import( + `../oauth-handler?agy-auth-url-client-present=${Date.now()}` + ); + + expect( + getPlusAuthUrlCredentialError( + 'agy', + 'https://accounts.google.com/o/oauth2/v2/auth?client_id=test-client&redirect_uri=http%3A%2F%2Flocalhost%3A8085%2Foauth2callback&state=test' + ) + ).toBeNull(); + }); +}); + describe('usesKiroLocalCallbackReplay', () => { it('limits local callback replay to CLI auth-code flows', async () => { const { usesKiroLocalCallbackReplay } = await import( diff --git a/src/cliproxy/auth/oauth-handler.ts b/src/cliproxy/auth/oauth-handler.ts index c8dd2f74..ac58e0bc 100644 --- a/src/cliproxy/auth/oauth-handler.ts +++ b/src/cliproxy/auth/oauth-handler.ts @@ -87,30 +87,122 @@ const GEMINI_PLUS_CLIENT_SECRET_ENV = 'CLIPROXY_GEMINI_OAUTH_CLIENT_SECRET'; const logger = createLogger('cliproxy:auth:oauth'); -function buildGeminiPlusOAuthCredentialMessage(missing?: string[]): string { +/** + * Table of providers that require Google OAuth client credentials when running + * against CLIProxy Plus. Keyed by CLIProxyProvider value. + * + * Used by the generalized helpers so the dashboard handler can guard any + * table-listed provider without duplicating env-var names. + */ +export const PLUS_OAUTH_ENV_BY_PROVIDER: Partial< + Record +> = { + gemini: { + idEnv: GEMINI_PLUS_CLIENT_ID_ENV, + secretEnv: GEMINI_PLUS_CLIENT_SECRET_ENV, + displayName: 'Gemini', + }, + agy: { + idEnv: 'CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_ID', + secretEnv: 'CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_SECRET', + displayName: 'Antigravity', + }, +}; + +/** + * Build a human-readable error message for a provider whose Plus OAuth client + * credentials are missing. + * + * @param displayName - Human-readable provider name (e.g. "Gemini", "Antigravity") + * @param idEnv - Name of the client-ID env var + * @param secretEnv - Name of the client-secret env var + * @param missing - Which of the two vars are absent (omit to suppress the "Missing:" prefix) + */ +function buildPlusOAuthCredentialMessage( + displayName: string, + idEnv: string, + secretEnv: string, + missing?: string[] +): string { const missingText = missing?.length ? ` Missing: ${missing.join(', ')}.` : ''; return ( - 'Gemini OAuth from CLIProxy Plus is missing Google OAuth client credentials.' + + `${displayName} OAuth from CLIProxy Plus is missing Google OAuth client credentials.` + missingText + - ` Set ${GEMINI_PLUS_CLIENT_ID_ENV} and ${GEMINI_PLUS_CLIENT_SECRET_ENV} before starting CLIProxy Plus,` + - ' or switch `cliproxy.backend` to `original` for Gemini.' + ` Set ${idEnv} and ${secretEnv} before starting CLIProxy Plus,` + + ` or switch \`cliproxy.backend\` to \`original\` for ${displayName}.` ); } +/** + * Generalized credential-missing guard for any provider in PLUS_OAUTH_ENV_BY_PROVIDER. + * + * Returns null when: + * - provider is not in the table (not a Plus-credentialed provider) + * - backend is not 'plus' + * - both credential env vars are set and non-empty + * + * Returns an error string when Plus is active and one or both vars are missing. + */ +export function getPlusOAuthCredentialError( + provider: CLIProxyProvider, + backend: CLIProxyBackend, + env: NodeJS.ProcessEnv = process.env +): string | null { + const entry = PLUS_OAUTH_ENV_BY_PROVIDER[provider]; + if (!entry || backend !== 'plus') { + return null; + } + + const missing = [entry.idEnv, entry.secretEnv].filter((name) => !env[name]?.trim()); + return missing.length > 0 + ? buildPlusOAuthCredentialMessage(entry.displayName, entry.idEnv, entry.secretEnv, missing) + : null; +} + +/** + * Generalized auth-URL guard for any provider in PLUS_OAUTH_ENV_BY_PROVIDER. + * + * Returns null when: + * - provider is not in the table + * - authUrl cannot be parsed as a URL (ignore malformed upstream responses) + * - client_id query param is present and non-empty + * + * Returns an error string when client_id is absent or empty. + */ +export function getPlusAuthUrlCredentialError( + provider: CLIProxyProvider, + authUrl: string +): string | null { + const entry = PLUS_OAUTH_ENV_BY_PROVIDER[provider]; + if (!entry) { + return null; + } + + try { + const parsed = new URL(authUrl); + const clientId = parsed.searchParams.get('client_id')?.trim(); + return clientId + ? null + : buildPlusOAuthCredentialMessage(entry.displayName, entry.idEnv, entry.secretEnv); + } catch { + return null; + } +} + +// --------------------------------------------------------------------------- +// Gemini-specific aliases — kept for backward-compat with PR #1131's callers. +// These lock the provider to 'gemini' and delegate to the generalized helpers. +// --------------------------------------------------------------------------- + export function getGeminiPlusOAuthCredentialError( provider: CLIProxyProvider, backend: CLIProxyBackend, env: NodeJS.ProcessEnv = process.env ): string | null { - if (provider !== 'gemini' || backend !== 'plus') { + if (provider !== 'gemini') { return null; } - - const missing = [GEMINI_PLUS_CLIENT_ID_ENV, GEMINI_PLUS_CLIENT_SECRET_ENV].filter( - (name) => !env[name]?.trim() - ); - - return missing.length > 0 ? buildGeminiPlusOAuthCredentialMessage(missing) : null; + return getPlusOAuthCredentialError(provider, backend, env); } export function getGeminiAuthUrlCredentialError( @@ -120,14 +212,7 @@ export function getGeminiAuthUrlCredentialError( if (provider !== 'gemini') { return null; } - - try { - const parsed = new URL(authUrl); - const clientId = parsed.searchParams.get('client_id')?.trim(); - return clientId ? null : buildGeminiPlusOAuthCredentialMessage(); - } catch { - return null; - } + return getPlusAuthUrlCredentialError(provider, authUrl); } export async function requestPasteCallbackStart( From 31076dff3e494645e8177e19aed64961d073b206 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 10 May 2026 15:47:22 -0400 Subject: [PATCH 2/3] fix(web-server): gate Gemini/AGY dashboard OAuth on Plus credential availability The dashboard Add Account flow hits POST /api/cliproxy/auth/:provider/start-url which fetches the OAuth consent URL directly from the CLIProxy Plus management API. With cliproxy.backend=plus and unset CLIPROXY_{GEMINI,ANTIGRAVITY}_OAUTH_CLIENT_ID/SECRET, Plus returns a URL with empty client_id= and Google rejects with 400 invalid_request. PR #1131 only guarded the CLI /start path; /start-url was unguarded. - Pre-fetch guard via getPlusOAuthCredentialError -> 400 plus_oauth_credentials_missing before contacting the Plus binary. - Post-fetch guard via getPlusAuthUrlCredentialError -> 502 plus_oauth_url_missing_client_id if Plus still emits a URL without client_id (logged with query string redacted). - New integration test (cliproxy-auth-routes-oauth-guard.test.ts) covers gemini/agy guard firing, non-table providers passing through, and both error body contracts. 15 cases pass. Closes #1208 --- .../cliproxy-auth-routes-oauth-guard.test.ts | 291 ++++++++++++++++++ src/web-server/routes/cliproxy-auth-routes.ts | 42 +++ 2 files changed, 333 insertions(+) create mode 100644 src/web-server/routes/__tests__/cliproxy-auth-routes-oauth-guard.test.ts diff --git a/src/web-server/routes/__tests__/cliproxy-auth-routes-oauth-guard.test.ts b/src/web-server/routes/__tests__/cliproxy-auth-routes-oauth-guard.test.ts new file mode 100644 index 00000000..a9db07c1 --- /dev/null +++ b/src/web-server/routes/__tests__/cliproxy-auth-routes-oauth-guard.test.ts @@ -0,0 +1,291 @@ +/** + * Integration tests for the OAuth credential guard wired into the + * /:provider/start-url route (Phase 3 + Phase 4). + * + * These tests verify the guard functions that are called inline by the route + * handler, using the same pattern as oauth-handler-paste-callback.test.ts. + * Dynamic imports with cache-busting query strings prevent module-cache + * interference between test cases. + * + * Test isolation: guard functions under test only read process.env and + * their CLIProxyProvider/CLIProxyBackend arguments — no disk access, + * no real ~/.ccs reads required. + */ + +import { afterEach, describe, expect, it } from 'bun:test'; + +// --------------------------------------------------------------------------- +// Restore any env vars mutated during tests +// --------------------------------------------------------------------------- +const GEMINI_ID_ENV = 'CLIPROXY_GEMINI_OAUTH_CLIENT_ID'; +const GEMINI_SECRET_ENV = 'CLIPROXY_GEMINI_OAUTH_CLIENT_SECRET'; +const AGY_ID_ENV = 'CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_ID'; +const AGY_SECRET_ENV = 'CLIPROXY_ANTIGRAVITY_OAUTH_CLIENT_SECRET'; + +function unsetGeminiEnv(): void { + delete process.env[GEMINI_ID_ENV]; + delete process.env[GEMINI_SECRET_ENV]; +} + +function setGeminiEnv(): void { + process.env[GEMINI_ID_ENV] = 'test-client-id'; + process.env[GEMINI_SECRET_ENV] = 'test-client-secret'; +} + +function unsetAgyEnv(): void { + delete process.env[AGY_ID_ENV]; + delete process.env[AGY_SECRET_ENV]; +} + +function setAgyEnv(): void { + process.env[AGY_ID_ENV] = 'test-agy-client-id'; + process.env[AGY_SECRET_ENV] = 'test-agy-client-secret'; +} + +afterEach(() => { + // Clean up any env vars set in tests + delete process.env[GEMINI_ID_ENV]; + delete process.env[GEMINI_SECRET_ENV]; + delete process.env[AGY_ID_ENV]; + delete process.env[AGY_SECRET_ENV]; +}); + +// --------------------------------------------------------------------------- +// Phase 3: pre-fetch credential guard (getPlusOAuthCredentialError) +// The route calls this before making any fetch to the Plus binary. +// --------------------------------------------------------------------------- + +describe('start-url route: Phase 3 pre-fetch credential guard', () => { + it('fires for gemini on plus backend when both env vars are missing', async () => { + unsetGeminiEnv(); + + const { getPlusOAuthCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-guard-gemini-missing-${Date.now()}` + ); + + const error = getPlusOAuthCredentialError('gemini', 'plus'); + + // Guard must return a non-null string (route returns 400 with this as message) + expect(error).not.toBeNull(); + expect(typeof error).toBe('string'); + expect(error).toContain('Gemini OAuth from CLIProxy Plus is missing'); + expect(error).toContain(GEMINI_ID_ENV); + expect(error).toContain(GEMINI_SECRET_ENV); + // Message must tell user how to fix (set env vars or switch backend) + expect(error).toContain('original'); + }); + + it('fires for gemini on plus backend when only client ID is missing', async () => { + delete process.env[GEMINI_ID_ENV]; + process.env[GEMINI_SECRET_ENV] = 'has-secret'; + + const { getPlusOAuthCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-guard-gemini-id-only-${Date.now()}` + ); + + const error = getPlusOAuthCredentialError('gemini', 'plus'); + expect(error).not.toBeNull(); + // Missing var should be listed + expect(error).toContain(GEMINI_ID_ENV); + delete process.env[GEMINI_SECRET_ENV]; + }); + + it('fires for agy on plus backend when both env vars are missing', async () => { + unsetAgyEnv(); + + const { getPlusOAuthCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-guard-agy-missing-${Date.now()}` + ); + + const error = getPlusOAuthCredentialError('agy', 'plus'); + + expect(error).not.toBeNull(); + expect(typeof error).toBe('string'); + expect(error).toContain('Antigravity OAuth from CLIProxy Plus is missing'); + expect(error).toContain(AGY_ID_ENV); + expect(error).toContain(AGY_SECRET_ENV); + }); + + it('returns null for gemini on plus when both env vars are present (guard does not fire)', async () => { + setGeminiEnv(); + + const { getPlusOAuthCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-guard-gemini-ok-${Date.now()}` + ); + + expect(getPlusOAuthCredentialError('gemini', 'plus')).toBeNull(); + }); + + it('returns null for agy on plus when both env vars are present (guard does not fire)', async () => { + setAgyEnv(); + + const { getPlusOAuthCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-guard-agy-ok-${Date.now()}` + ); + + expect(getPlusOAuthCredentialError('agy', 'plus')).toBeNull(); + }); + + it('returns null for ghcp provider on plus backend (not in guard table)', async () => { + // ghcp is NOT in PLUS_OAUTH_ENV_BY_PROVIDER — guard must not fire + const { getPlusOAuthCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-guard-ghcp-${Date.now()}` + ); + + expect(getPlusOAuthCredentialError('ghcp', 'plus')).toBeNull(); + }); + + it('returns null for gemini when backend is original (guard only applies to plus)', async () => { + unsetGeminiEnv(); // env vars absent, but backend is original + + const { getPlusOAuthCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-guard-gemini-original-${Date.now()}` + ); + + // original backend → guard returns null regardless of env + expect(getPlusOAuthCredentialError('gemini', 'original', {})).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// Phase 4: post-fetch auth-URL guard (getPlusAuthUrlCredentialError) +// The route calls this after fetching the authUrl from Plus, before responding. +// --------------------------------------------------------------------------- + +describe('start-url route: Phase 4 post-fetch auth-URL guard', () => { + it('fires for gemini when Plus emits auth URL with empty client_id (502 contract)', async () => { + const { getPlusAuthUrlCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-url-guard-gemini-empty-${Date.now()}` + ); + + const badUrl = + 'https://accounts.google.com/o/oauth2/v2/auth' + + '?client_id=&redirect_uri=http%3A%2F%2Flocalhost%3A8085%2Foauth2callback&state=abc'; + const error = getPlusAuthUrlCredentialError('gemini', badUrl); + + expect(error).not.toBeNull(); + expect(typeof error).toBe('string'); + expect(error).toContain('Gemini OAuth from CLIProxy Plus is missing'); + }); + + it('fires for agy when Plus emits auth URL with empty client_id (502 contract)', async () => { + const { getPlusAuthUrlCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-url-guard-agy-empty-${Date.now()}` + ); + + const badUrl = 'https://accounts.google.com/o/oauth2/v2/auth?client_id=&state=abc'; + const error = getPlusAuthUrlCredentialError('agy', badUrl); + + expect(error).not.toBeNull(); + expect(error).toContain('Antigravity OAuth from CLIProxy Plus is missing'); + }); + + it('returns null for gemini when client_id is present (guard must not fire)', async () => { + const { getPlusAuthUrlCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-url-guard-gemini-ok-${Date.now()}` + ); + + const goodUrl = 'https://accounts.google.com/o/oauth2/v2/auth?client_id=real-id&state=abc'; + expect(getPlusAuthUrlCredentialError('gemini', goodUrl)).toBeNull(); + }); + + it('returns null for ghcp (not in guard table) even with empty client_id', async () => { + const { getPlusAuthUrlCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-url-guard-ghcp-${Date.now()}` + ); + + // ghcp is not in PLUS_OAUTH_ENV_BY_PROVIDER — URL guard never fires + const anyUrl = 'https://example.com/oauth?client_id=&state=abc'; + expect(getPlusAuthUrlCredentialError('ghcp', anyUrl)).toBeNull(); + }); + + it('returns null for malformed authUrl (guard must not throw)', async () => { + const { getPlusAuthUrlCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?route-url-guard-malformed-${Date.now()}` + ); + + // Guard must swallow parse errors — route should not 502 on malformed URLs + expect(getPlusAuthUrlCredentialError('gemini', 'not-a-url')).toBeNull(); + expect(getPlusAuthUrlCredentialError('gemini', '')).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// Phase 3+4: HTTP response body contract +// Verifies the exact JSON shape the route would return so the UI hook can +// match on data.error and surface data.message to the user. +// --------------------------------------------------------------------------- + +describe('start-url route: response body contract', () => { + it('credential-missing 400 body shape: error=plus_oauth_credentials_missing, message=string, provider=string', async () => { + unsetGeminiEnv(); + + const { getPlusOAuthCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?body-shape-missing-${Date.now()}` + ); + + const message = getPlusOAuthCredentialError('gemini', 'plus'); + + // Replicate what the route handler does when credentialError is non-null + const body = { + error: 'plus_oauth_credentials_missing' as const, + provider: 'gemini' as const, + message, + }; + + expect(body.error).toBe('plus_oauth_credentials_missing'); + expect(typeof body.message).toBe('string'); + // Human-readable message must be meaningful + expect((body.message ?? '').length).toBeGreaterThan(10); + expect(body.provider).toBe('gemini'); + }); + + it('auth-url 502 body shape: error=plus_oauth_url_missing_client_id, message=string, provider=string', async () => { + const { getPlusAuthUrlCredentialError } = await import( + `../../../cliproxy/auth/oauth-handler?body-shape-url-${Date.now()}` + ); + + const badUrl = 'https://accounts.google.com/o/oauth2/v2/auth?client_id=&state=abc'; + const message = getPlusAuthUrlCredentialError('gemini', badUrl); + + // Replicate what the route handler does when authUrlError is non-null + const body = { + error: 'plus_oauth_url_missing_client_id' as const, + provider: 'gemini' as const, + message, + }; + + expect(body.error).toBe('plus_oauth_url_missing_client_id'); + expect(typeof body.message).toBe('string'); + expect((body.message ?? '').length).toBeGreaterThan(10); + expect(body.provider).toBe('gemini'); + }); + + it('UI hook can distinguish credential errors by data.error code', () => { + // The UI hook checks: data.error === 'plus_oauth_credentials_missing' + // or data.error === 'plus_oauth_url_missing_client_id' to decide + // whether to use data.message instead of data.error as the displayed text. + const missingCreds = { error: 'plus_oauth_credentials_missing', message: 'Friendly message' }; + const missingUrl = { + error: 'plus_oauth_url_missing_client_id', + message: 'Friendly URL message', + }; + const generic = { error: 'some_other_error' }; + + function simulateHookErrorResolution(data: Record): string { + const isPlusCredentialError = + data.error === 'plus_oauth_credentials_missing' || + data.error === 'plus_oauth_url_missing_client_id'; + return isPlusCredentialError && typeof data.message === 'string' + ? data.message + : typeof data.error === 'string' + ? data.error + : 'Unknown error'; + } + + expect(simulateHookErrorResolution(missingCreds)).toBe('Friendly message'); + expect(simulateHookErrorResolution(missingUrl)).toBe('Friendly URL message'); + // Generic errors still use data.error (the code) + expect(simulateHookErrorResolution(generic)).toBe('some_other_error'); + }); +}); diff --git a/src/web-server/routes/cliproxy-auth-routes.ts b/src/web-server/routes/cliproxy-auth-routes.ts index a1547ce7..90834888 100644 --- a/src/web-server/routes/cliproxy-auth-routes.ts +++ b/src/web-server/routes/cliproxy-auth-routes.ts @@ -70,6 +70,11 @@ import { import { createRouteErrorHelpers } from './route-helpers'; import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; import { loadOrCreateUnifiedConfig } from '../../config/config-loader-facade'; +import { + getPlusOAuthCredentialError, + getPlusAuthUrlCredentialError, +} from '../../cliproxy/auth/oauth-handler'; +import { getStoredConfiguredBackend } from '../../cliproxy/binary-manager'; const router = Router(); const MANUAL_AUTH_STATE_TTL_MS = 10 * 60 * 1000; @@ -1042,6 +1047,24 @@ router.post('/:provider/start-url', async (req: Request, res: Response): Promise return; } + // Phase 3: Pre-fetch credential guard for Plus-backend OAuth providers (gemini, agy). + // Returns null for providers not in the table or when backend is not 'plus'. + const credentialError = getPlusOAuthCredentialError( + provider as CLIProxyProvider, + getStoredConfiguredBackend() + ); + if (credentialError) { + console.error( + `[cliproxy-auth-routes] start-url credential guard fired for provider=${provider}: ${credentialError}` + ); + res.status(400).json({ + error: 'plus_oauth_credentials_missing', + provider, + message: credentialError, + }); + return; + } + try { const authUrlProvider = CLIPROXY_AUTH_URL_PROVIDER_MAP[provider as CLIProxyProvider] || provider; @@ -1078,6 +1101,25 @@ router.post('/:provider/start-url', async (req: Request, res: Response): Promise method?: string; }; const authUrl = data.url || data.auth_url; + + // Phase 4: Post-fetch auth-URL guard — detect Plus emitting an OAuth URL with empty client_id. + // Only fires for table-listed providers (gemini, agy); returns null for all others. + if (authUrl) { + const authUrlError = getPlusAuthUrlCredentialError(provider as CLIProxyProvider, authUrl); + if (authUrlError) { + const redactedUrl = authUrl.split('?')[0]; + console.error( + `[cliproxy-auth-routes] Plus emitted OAuth URL without client_id for provider=${provider} url=${redactedUrl}` + ); + res.status(502).json({ + error: 'plus_oauth_url_missing_client_id', + provider, + message: authUrlError, + }); + return; + } + } + const oauthState = data.state || parseAuthUrlState(authUrl); // Some upstream flows return state first and provide auth_url in subsequent status polling. From 497bca8684d77cd0242eddea6162b2986b976f80 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 10 May 2026 15:48:09 -0400 Subject: [PATCH 3/3] fix(ui): surface Plus OAuth credential diagnostics in Add Account dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When POST /api/cliproxy/auth/:provider/start-url returns 400 plus_oauth_credentials_missing or 502 plus_oauth_url_missing_client_id, surface the server-provided human-readable message (env var names and backend=original switch hint) instead of the raw error code. Error propagates through the existing toast/inline-alert path in add-account-dialog.tsx — no new component primitives. Refs #1208 --- ui/src/hooks/use-cliproxy-auth-flow.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ui/src/hooks/use-cliproxy-auth-flow.ts b/ui/src/hooks/use-cliproxy-auth-flow.ts index b9dca50d..bba9788a 100644 --- a/ui/src/hooks/use-cliproxy-auth-flow.ts +++ b/ui/src/hooks/use-cliproxy-auth-flow.ts @@ -371,8 +371,17 @@ export function useCliproxyAuthFlow() { const success = data.success === true; if (!response.ok || !success) { + // For Plus OAuth credential errors the server sends a human-readable + // explanation in `data.message`; prefer it over the machine error code. + const isPlusCredentialError = + data.error === 'plus_oauth_credentials_missing' || + data.error === 'plus_oauth_url_missing_client_id'; const errorMsg = - typeof data.error === 'string' ? data.error : t('toasts.providerStartOAuthFailed'); + isPlusCredentialError && typeof data.message === 'string' + ? data.message + : typeof data.error === 'string' + ? data.error + : t('toasts.providerStartOAuthFailed'); throw new Error(errorMsg); }