fix(cliproxy): harden provider alias and refresh edge cases

- guard provider alias generation against ambiguous collisions

- add account-scoped Gemini token refresh with strict account checks

- warn and fallback on invalid management remote ports

- align remaining HTTP remote defaults and extend cliproxy tests
This commit is contained in:
Tam Nhu Tran
2026-02-18 04:18:39 +07:00
parent 7e527af777
commit a71496cc3d
8 changed files with 167 additions and 52 deletions
+55 -33
View File
@@ -108,42 +108,53 @@ function isValidCliproxyToken(data: unknown): data is CliproxyGeminiToken {
* Read Gemini token from CLIProxy auth directory * Read Gemini token from CLIProxy auth directory
* Returns credentials with source path, or null if no valid token found * Returns credentials with source path, or null if no valid token found
*/ */
function readCliproxyGeminiCreds(): GeminiCredsWithSource | null { function readCliproxyGeminiCreds(accountId?: string): GeminiCredsWithSource | null {
const authDir = getProviderAuthDir('gemini'); const authDir = getProviderAuthDir('gemini');
if (!fs.existsSync(authDir)) return null; if (!fs.existsSync(authDir)) return null;
// Try to find default account's token file
const defaultAccount = getDefaultAccount('gemini');
let tokenPath: string | null = null; let tokenPath: string | null = null;
const normalizedAccountId = accountId?.trim();
const accounts = getProviderAccounts('gemini');
if (defaultAccount) { // Account-specific refresh path (used by background worker)
tokenPath = path.join(authDir, defaultAccount.tokenFile); if (normalizedAccountId) {
if (!fs.existsSync(tokenPath)) tokenPath = null; const targetAccount = accounts.find((account) => account.id === normalizedAccountId);
if (!targetAccount) {
return null;
}
tokenPath = path.join(authDir, targetAccount.tokenFile);
} }
// Fallback: find any gemini token file by prefix or type if (!normalizedAccountId) {
if (!tokenPath) { // Try to find default account's token file
const accounts = getProviderAccounts('gemini'); const defaultAccount = getDefaultAccount('gemini');
if (accounts.length > 0) { if (defaultAccount) {
tokenPath = path.join(authDir, defaultAccount.tokenFile);
if (!fs.existsSync(tokenPath)) tokenPath = null;
}
// Fallback: find any gemini account token file
if (!tokenPath && accounts.length > 0) {
tokenPath = path.join(authDir, accounts[0].tokenFile); tokenPath = path.join(authDir, accounts[0].tokenFile);
if (!fs.existsSync(tokenPath)) tokenPath = null; if (!fs.existsSync(tokenPath)) tokenPath = null;
} }
}
// Last fallback: scan directory for gemini token files // Last fallback: scan directory for gemini token files
if (!tokenPath) { if (!tokenPath) {
try { try {
const files = fs.readdirSync(authDir).filter((f) => f.endsWith('.json')); const files = fs.readdirSync(authDir).filter((f) => f.endsWith('.json'));
for (const file of files) { for (const file of files) {
const filePath = path.join(authDir, file); const filePath = path.join(authDir, file);
if (file.startsWith('gemini-') || isTokenFileForProvider(filePath, 'gemini')) { if (file.startsWith('gemini-') || isTokenFileForProvider(filePath, 'gemini')) {
tokenPath = filePath; tokenPath = filePath;
break; break;
}
} }
} catch {
// Directory read failed - continue to return null
return null;
} }
} catch {
// Directory read failed - continue to return null
return null;
} }
} }
@@ -172,13 +183,19 @@ function readCliproxyGeminiCreds(): GeminiCredsWithSource | null {
* Priority: CLIProxy auth dir first, then ~/.gemini/oauth_creds.json * Priority: CLIProxy auth dir first, then ~/.gemini/oauth_creds.json
* Returns credentials with source path for correct write-back * Returns credentials with source path for correct write-back
*/ */
function readGeminiCreds(): GeminiCredsWithSource | null { function readGeminiCreds(accountId?: string): GeminiCredsWithSource | null {
// 1. Try CLIProxy auth directory first (CCS-managed tokens) // 1. Try CLIProxy auth directory first (CCS-managed tokens)
const cliproxyResult = readCliproxyGeminiCreds(); const cliproxyResult = readCliproxyGeminiCreds(accountId);
if (cliproxyResult) { if (cliproxyResult) {
return cliproxyResult; return cliproxyResult;
} }
// Account-scoped refresh is only supported for CLIProxy account files.
// Do not fall back to ~/.gemini for a specific accountId.
if (accountId?.trim()) {
return null;
}
// 2. Fall back to standard Gemini CLI location // 2. Fall back to standard Gemini CLI location
const oauthPath = getGeminiOAuthPath(); const oauthPath = getGeminiOAuthPath();
if (!fs.existsSync(oauthPath)) { if (!fs.existsSync(oauthPath)) {
@@ -249,8 +266,8 @@ function writeGeminiCreds(creds: GeminiOAuthCreds, sourcePath: string): string |
/** /**
* Check if Gemini token is expired or expiring soon * Check if Gemini token is expired or expiring soon
*/ */
export function isGeminiTokenExpiringSoon(): boolean { export function isGeminiTokenExpiringSoon(accountId?: string): boolean {
const result = readGeminiCreds(); const result = readGeminiCreds(accountId);
if (!result || !result.creds.access_token) { if (!result || !result.creds.access_token) {
return true; // No token = needs auth return true; // No token = needs auth
} }
@@ -263,14 +280,15 @@ export function isGeminiTokenExpiringSoon(): boolean {
/** /**
* Refresh Gemini access token using refresh_token * Refresh Gemini access token using refresh_token
* @param accountId Optional account ID for account-scoped refresh
* @returns Result with success status, optional error, and expiry time * @returns Result with success status, optional error, and expiry time
*/ */
export async function refreshGeminiToken(): Promise<{ export async function refreshGeminiToken(accountId?: string): Promise<{
success: boolean; success: boolean;
error?: string; error?: string;
expiresAt?: number; expiresAt?: number;
}> { }> {
const result = readGeminiCreds(); const result = readGeminiCreds(accountId);
if (!result || !result.creds.refresh_token) { if (!result || !result.creds.refresh_token) {
return { success: false, error: 'No refresh token available' }; return { success: false, error: 'No refresh token available' };
} }
@@ -334,19 +352,23 @@ export async function refreshGeminiToken(): Promise<{
/** /**
* Ensure Gemini token is valid, refreshing if needed * Ensure Gemini token is valid, refreshing if needed
* @param verbose Log progress if true * @param verbose Log progress if true
* @param accountId Optional account ID for account-scoped refresh
* @returns true if token is valid (or was refreshed), false if refresh failed * @returns true if token is valid (or was refreshed), false if refresh failed
*/ */
export async function ensureGeminiTokenValid(verbose = false): Promise<{ export async function ensureGeminiTokenValid(
verbose = false,
accountId?: string
): Promise<{
valid: boolean; valid: boolean;
refreshed: boolean; refreshed: boolean;
error?: string; error?: string;
}> { }> {
const result = readGeminiCreds(); const result = readGeminiCreds(accountId);
if (!result || !result.creds.access_token) { if (!result || !result.creds.access_token) {
return { valid: false, refreshed: false, error: 'No Gemini credentials found' }; return { valid: false, refreshed: false, error: 'No Gemini credentials found' };
} }
if (!isGeminiTokenExpiringSoon()) { if (!isGeminiTokenExpiringSoon(accountId)) {
return { valid: true, refreshed: false }; return { valid: true, refreshed: false };
} }
@@ -355,7 +377,7 @@ export async function ensureGeminiTokenValid(verbose = false): Promise<{
console.log('[i] Gemini token expired or expiring soon, refreshing...'); console.log('[i] Gemini token expired or expiring soon, refreshing...');
} }
const refreshResult = await refreshGeminiToken(); const refreshResult = await refreshGeminiToken(accountId);
if (refreshResult.success) { if (refreshResult.success) {
if (verbose) { if (verbose) {
console.log('[OK] Gemini token refreshed successfully'); console.log('[OK] Gemini token refreshed successfully');
+24 -5
View File
@@ -11,6 +11,7 @@
*/ */
import { CLIProxyProvider } from '../../types'; import { CLIProxyProvider } from '../../types';
import { getProviderAccounts } from '../../account-manager';
import { import {
getTokenRefreshOwnership, getTokenRefreshOwnership,
isRefreshDelegatedToCLIProxy, isRefreshDelegatedToCLIProxy,
@@ -40,15 +41,33 @@ export function isRefreshDelegated(provider: CLIProxyProvider): boolean {
/** /**
* Refresh token for a specific provider and account * Refresh token for a specific provider and account
* @param provider Provider to refresh * @param provider Provider to refresh
* @param _accountId Account ID (currently unused, multi-account not yet implemented) * @param accountId Account ID used to refresh the correct provider token
* @returns Refresh result with success status and optional error * @returns Refresh result with success status and optional error
*/ */
export async function refreshToken( export async function refreshToken(
provider: CLIProxyProvider, provider: CLIProxyProvider,
_accountId: string accountId: string
): Promise<ProviderRefreshResult> { ): Promise<ProviderRefreshResult> {
const normalizedAccountId = accountId.trim();
if (!normalizedAccountId) {
return {
success: false,
error: 'Account ID is required for token refresh',
};
}
const hasAccount = getProviderAccounts(provider).some(
(account) => account.id === normalizedAccountId
);
if (!hasAccount) {
return {
success: false,
error: `Account not found for ${provider}: ${normalizedAccountId}`,
};
}
if (provider === 'gemini') { if (provider === 'gemini') {
return await refreshGeminiTokenWrapper(); return await refreshGeminiTokenWrapper(normalizedAccountId);
} }
const ownership = getTokenRefreshOwnership(provider); const ownership = getTokenRefreshOwnership(provider);
@@ -73,8 +92,8 @@ export async function refreshToken(
* Wrapper for Gemini token refresh * Wrapper for Gemini token refresh
* Converts gemini-token-refresh.ts format to provider-refreshers format * Converts gemini-token-refresh.ts format to provider-refreshers format
*/ */
async function refreshGeminiTokenWrapper(): Promise<ProviderRefreshResult> { async function refreshGeminiTokenWrapper(accountId: string): Promise<ProviderRefreshResult> {
const result = await refreshGeminiToken(); const result = await refreshGeminiToken(accountId);
if (!result.success) { if (!result.success) {
return { return {
+21 -2
View File
@@ -24,14 +24,33 @@ const DEFAULT_TIMEOUT_MS = 5000;
/** Default port for HTTPS protocol */ /** Default port for HTTPS protocol */
const DEFAULT_HTTPS_PORT = 443; const DEFAULT_HTTPS_PORT = 443;
/** Avoid duplicate warnings for repeated invalid port inputs */
const WARNED_INVALID_PORTS = new Set<string>();
function isValidPort(port: number | undefined): port is number {
return port !== undefined && Number.isInteger(port) && port > 0 && port <= 65535;
}
/** /**
* Get effective port based on config and protocol. * Get effective port based on config and protocol.
*/ */
function getEffectivePort(port: number | undefined, protocol: 'http' | 'https'): number { function getEffectivePort(port: number | undefined, protocol: 'http' | 'https'): number {
if (port !== undefined && Number.isInteger(port) && port > 0 && port <= 65535) { if (isValidPort(port)) {
return port; return port;
} }
return protocol === 'https' ? DEFAULT_HTTPS_PORT : CLIPROXY_DEFAULT_PORT;
const fallbackPort = protocol === 'https' ? DEFAULT_HTTPS_PORT : CLIPROXY_DEFAULT_PORT;
if (port !== undefined) {
const warningKey = `${protocol}:${String(port)}`;
if (!WARNED_INVALID_PORTS.has(warningKey)) {
WARNED_INVALID_PORTS.add(warningKey);
console.warn(
`[management-api-client] Invalid port "${String(port)}", using default ${fallbackPort}`
);
}
}
return fallbackPort;
} }
/** /**
+31 -8
View File
@@ -154,16 +154,39 @@ export function buildProviderMap<T>(
const PROVIDER_ID_SET = new Set(CLIPROXY_PROVIDER_IDS); const PROVIDER_ID_SET = new Set(CLIPROXY_PROVIDER_IDS);
const PROVIDER_ALIAS_MAP: ReadonlyMap<string, CLIProxyProvider> = (() => { export function buildProviderAliasMap(
const entries: Array<[string, CLIProxyProvider]> = []; capabilities: Record<CLIProxyProvider, ProviderCapabilities> = PROVIDER_CAPABILITIES
for (const provider of CLIPROXY_PROVIDER_IDS) { ): ReadonlyMap<string, CLIProxyProvider> {
entries.push([provider, provider]); const aliasMap = new Map<string, CLIProxyProvider>();
for (const alias of PROVIDER_CAPABILITIES[provider].aliases) { const providers = Object.keys(capabilities) as CLIProxyProvider[];
entries.push([alias.toLowerCase(), provider]);
const registerAlias = (alias: string, provider: CLIProxyProvider): void => {
const normalized = alias.trim().toLowerCase();
if (!normalized) {
return;
}
const existingProvider = aliasMap.get(normalized);
if (existingProvider && existingProvider !== provider) {
throw new Error(
`Provider alias collision for "${normalized}": ${existingProvider} and ${provider}`
);
}
aliasMap.set(normalized, provider);
};
for (const provider of providers) {
registerAlias(provider, provider);
for (const alias of capabilities[provider].aliases) {
registerAlias(alias, provider);
} }
} }
return new Map(entries);
})(); return aliasMap;
}
const PROVIDER_ALIAS_MAP: ReadonlyMap<string, CLIProxyProvider> = buildProviderAliasMap();
export function isCLIProxyProvider(provider: string): provider is CLIProxyProvider { export function isCLIProxyProvider(provider: string): provider is CLIProxyProvider {
return PROVIDER_ID_SET.has(provider as CLIProxyProvider); return PROVIDER_ID_SET.has(provider as CLIProxyProvider);
+1 -1
View File
@@ -227,7 +227,7 @@ async function configureRemoteProxy(rl: readline.Interface): Promise<{
])) as 'http' | 'https'; ])) as 'http' | 'https';
// Port (optional) - with validation // Port (optional) - with validation
const defaultPort = protocol === 'https' ? '443' : '80'; const defaultPort = protocol === 'https' ? '443' : String(CLIPROXY_DEFAULT_PORT);
const portStr = await prompt(rl, `Port (leave empty for default ${defaultPort})`); const portStr = await prompt(rl, `Port (leave empty for default ${defaultPort})`);
let port: number | undefined; let port: number | undefined;
if (portStr) { if (portStr) {
+1 -1
View File
@@ -328,7 +328,7 @@ export interface ProxyRemoteConfig {
* Remote proxy port. * Remote proxy port.
* Optional - defaults based on protocol: * Optional - defaults based on protocol:
* - HTTPS: 443 * - HTTPS: 443
* - HTTP: 80 * - HTTP: 8317
* When empty/undefined, uses protocol default. * When empty/undefined, uses protocol default.
*/ */
port?: number; port?: number;
@@ -1,7 +1,7 @@
/** /**
* Unit tests for management-api-client module * Unit tests for management-api-client module
*/ */
import { describe, it, expect, beforeEach, mock } from 'bun:test'; import { describe, it, expect, beforeEach, mock, spyOn } from 'bun:test';
import { ManagementApiClient } from '../../../src/cliproxy/management-api-client'; import { ManagementApiClient } from '../../../src/cliproxy/management-api-client';
import type { import type {
ManagementClientConfig, ManagementClientConfig,
@@ -76,6 +76,18 @@ describe('management-api-client', () => {
const client = new ManagementApiClient(configNoPort); const client = new ManagementApiClient(configNoPort);
expect(client.getBaseUrl()).toBe('https://localhost'); expect(client.getBaseUrl()).toBe('https://localhost');
}); });
it('should warn and fall back when configured port is invalid', () => {
const warnSpy = spyOn(console, 'warn').mockImplementation(() => {});
const client = new ManagementApiClient({ ...config, port: 99999 });
expect(client.getBaseUrl()).toBe('http://localhost:8317');
expect(warnSpy).toHaveBeenCalledWith(
'[management-api-client] Invalid port "99999", using default 8317'
);
warnSpy.mockRestore();
});
}); });
describe('error code mapping', () => { describe('error code mapping', () => {
@@ -1,8 +1,10 @@
import { describe, expect, it } from 'bun:test'; import { describe, expect, it } from 'bun:test';
import { import {
buildProviderAliasMap,
CLIPROXY_PROVIDER_IDS, CLIPROXY_PROVIDER_IDS,
getOAuthCallbackPort, getOAuthCallbackPort,
getOAuthFlowType, getOAuthFlowType,
PROVIDER_CAPABILITIES,
getProviderDisplayName, getProviderDisplayName,
getProvidersByOAuthFlow, getProvidersByOAuthFlow,
isCLIProxyProvider, isCLIProxyProvider,
@@ -68,7 +70,25 @@ describe('provider-capabilities', () => {
expect(getOAuthCallbackPort('qwen')).toBeNull(); expect(getOAuthCallbackPort('qwen')).toBeNull();
expect(getOAuthCallbackPort('kiro')).toBeNull(); expect(getOAuthCallbackPort('kiro')).toBeNull();
expect(getOAuthCallbackPort('gemini')).toBe(8085); expect(getOAuthCallbackPort('gemini')).toBe(8085);
expect(getProviderDisplayName('agy')).toBe('AntiGravity'); expect(getProviderDisplayName('agy')).toBe('Antigravity');
});
it('throws when provider aliases collide across providers', () => {
const capabilitiesWithCollision = {
...PROVIDER_CAPABILITIES,
gemini: {
...PROVIDER_CAPABILITIES.gemini,
aliases: ['shared-alias'],
},
codex: {
...PROVIDER_CAPABILITIES.codex,
aliases: ['shared-alias'],
},
};
expect(() =>
buildProviderAliasMap(capabilitiesWithCollision as typeof PROVIDER_CAPABILITIES)
).toThrow(/shared-alias/i);
}); });
it('keeps diagnostics flow metadata in sync with provider capabilities', () => { it('keeps diagnostics flow metadata in sync with provider capabilities', () => {