Merge pull request #1210 from kaitranntt/kai/fix/dashboard-gemini-agy-oauth-guard

fix(cliproxy): gate Gemini/AGY dashboard OAuth when Plus credentials missing
This commit is contained in:
Kai (Tam Nhu) Tran
2026-05-10 22:01:23 -04:00
committed by GitHub
5 changed files with 509 additions and 20 deletions
@@ -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(
+104 -19
View File
@@ -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<CLIProxyProvider, { idEnv: string; secretEnv: string; displayName: string }>
> = {
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(
@@ -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, unknown>): 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');
});
});
@@ -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.
+10 -1
View File
@@ -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);
}