From 4ba9df54251baa60c13f146820f06076c585aa45 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 10 May 2026 15:47:05 -0400 Subject: [PATCH] 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(