From 4df08f6d99e3006107a0431609b7651f13ebc083 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Mon, 23 Mar 2026 11:01:38 -0400 Subject: [PATCH] feat: make cliproxy provider nicknames optional by default --- docs/dashboard-auth-cli.md | 7 +- docs/project-roadmap.md | 4 +- docs/system-architecture/provider-flows.md | 6 + src/api/services/profile-lifecycle-service.ts | 26 +- src/api/services/profile-writer.ts | 74 +- src/auth/profile-registry.ts | 98 ++- src/cliproxy/accounts/query.ts | 8 +- src/cliproxy/accounts/registry.ts | 638 +++++++++--------- src/cliproxy/accounts/token-file-ops.ts | 9 +- src/cliproxy/ai-providers/config-store.ts | 64 +- src/cliproxy/ai-providers/service.ts | 59 +- src/cliproxy/ai-providers/types.ts | 2 + src/cliproxy/auth-token-manager.ts | 102 ++- src/cliproxy/auth/token-manager.ts | 93 ++- .../services/variant-config-adapter.ts | 89 +-- .../cliproxy/proxy-lifecycle-subcommand.ts | 12 +- .../cliproxy/resolve-lifecycle-port.ts | 11 + src/commands/config-auth/disable-command.ts | 23 +- src/commands/config-auth/setup-command.ts | 25 +- src/commands/copilot-command.ts | 26 +- src/commands/cursor-command.ts | 30 +- src/commands/setup-command.ts | 9 +- src/config/unified-config-loader.ts | 73 +- src/web-server/health-service.ts | 8 +- src/web-server/middleware/auth-middleware.ts | 29 + src/web-server/routes/ai-provider-routes.ts | 39 +- src/web-server/routes/cliproxy-auth-routes.ts | 53 +- .../routes/cliproxy-stats-routes.ts | 13 + src/web-server/routes/cliproxy-sync-routes.ts | 20 +- .../routes/cursor-settings-routes.ts | 86 ++- src/web-server/routes/misc-routes.ts | 69 +- src/web-server/routes/proxy-routes.ts | 13 + src/web-server/routes/settings-routes.ts | 307 +++++---- src/web-server/routes/websearch-routes.ts | 98 ++- src/web-server/shared-routes.ts | 13 + .../account-registry-integrity.test.ts | 102 +++ ...account-registry-optional-nickname.test.ts | 36 +- .../ai-provider-service-stable-id.test.ts | 147 ++++ .../proxy-lifecycle-subcommand.test.ts | 86 ++- .../web-server/ai-provider-routes.test.ts | 173 +++++ ...iproxy-auth-routes-manual-callback.test.ts | 43 +- ui/src/hooks/use-cliproxy-ai-providers.ts | 10 +- ui/src/hooks/use-cliproxy-auth-flow.ts | 110 ++- ui/src/lib/api-client.ts | 24 +- ui/src/pages/cliproxy-ai-providers.tsx | 6 +- .../hooks/use-cliproxy-auth-flow.test.tsx | 199 ++++++ 46 files changed, 2076 insertions(+), 1096 deletions(-) create mode 100644 src/commands/cliproxy/resolve-lifecycle-port.ts create mode 100644 tests/unit/cliproxy/account-registry-integrity.test.ts create mode 100644 tests/unit/cliproxy/ai-provider-service-stable-id.test.ts create mode 100644 tests/unit/web-server/ai-provider-routes.test.ts create mode 100644 ui/tests/unit/hooks/use-cliproxy-auth-flow.test.tsx diff --git a/docs/dashboard-auth-cli.md b/docs/dashboard-auth-cli.md index df381442..62c77709 100644 --- a/docs/dashboard-auth-cli.md +++ b/docs/dashboard-auth-cli.md @@ -1,6 +1,6 @@ # Dashboard Authentication CLI -Last Updated: 2026-03-17 +Last Updated: 2026-03-23 CLI commands for managing CCS dashboard authentication. @@ -10,6 +10,8 @@ The CCS dashboard (`ccs config`) can be protected with username/password authent Authentication is **disabled by default** for backward compatibility. Use the CLI to configure and enable it. +When auth stays disabled, CCS now applies a localhost-only fallback on sensitive management endpoints. Remote devices can still open the dashboard UI when you intentionally bind it beyond loopback, but write-capable routes such as AI Provider management and CLIProxy auth/status helpers reject non-loopback requests until you enable dashboard auth. + ## Account Context Modes (Related Feature) Dashboard auth and account context metadata are separate: @@ -180,7 +182,8 @@ dashboard_auth: 1. **Bcrypt hashing**: Passwords are hashed with bcrypt (10 rounds) before storage 2. **Session cookies**: Sessions use HTTP-only cookies (not accessible via JavaScript) 3. **Rate limiting**: Login attempts are rate-limited (5 per 15 minutes) -4. **File permissions**: Config file is created with 0o600 permissions +4. **Fail-closed remote writes**: When auth is disabled, sensitive management routes allow localhost only +5. **File permissions**: Config file is created with 0o600 permissions ## Troubleshooting diff --git a/docs/project-roadmap.md b/docs/project-roadmap.md index 4ca846c8..e93db791 100644 --- a/docs/project-roadmap.md +++ b/docs/project-roadmap.md @@ -1,6 +1,6 @@ # CCS Project Roadmap -Last Updated: 2026-03-19 +Last Updated: 2026-03-23 Forward-looking roadmap documenting current priorities, GitHub issues, and future feature plans. @@ -41,6 +41,8 @@ All major modularization work is complete. The codebase evolved from monolithic ### Recent Fixes +- **2026-03-23**: CLIProxy providers that do not expose an email no longer require a user-supplied nickname on first auth. CCS now derives a stable internal account identifier for Kiro/Copilot-style flows, preserves later rename support, hardens account discovery/registry sync around that identifier, and updates AI Provider CRUD to use stable entry IDs instead of dashboard list indexes. +- **2026-03-23**: Sensitive dashboard management routes now fail closed to localhost-only access whenever dashboard auth is disabled. Remote access remains available after `ccs config auth setup`, but AI Provider management, CLIProxy auth/status helpers, and other write-capable settings endpoints no longer trust unauthenticated non-loopback requests. - **2026-03-19**: **#649** CCS splits CLIProxy provider-key authoring into a dedicated `CLIProxy -> AI Providers` dashboard route. `/cliproxy` now stays focused on OAuth accounts and variants, `/cliproxy/ai-providers` owns Gemini/Codex/Claude/Vertex/OpenAI-compatible key management, and `/providers` stays reserved for CCS-native API Profiles. - **2026-03-18**: **#755** Marketplace refresh no longer reuses one shared `known_marketplaces.json` across isolated instances. CCS now keeps marketplace payload directories shared while reconciling per-instance marketplace metadata so Claude Code validation succeeds for alternating or concurrent profiles, including Windows copy fallback. - **2026-03-17**: Deprecated user-facing GLMT discovery across CLI help, completions, presets, and docs. Existing `glmt` profiles now run through a compatibility path that normalizes legacy proxy settings to the direct GLM endpoint. diff --git a/docs/system-architecture/provider-flows.md b/docs/system-architecture/provider-flows.md index 8ccec44c..69d9f317 100644 --- a/docs/system-architecture/provider-flows.md +++ b/docs/system-architecture/provider-flows.md @@ -361,6 +361,10 @@ function selectBestAccount(accounts: AccountInfo[]): AccountInfo | null { **Providers**: GitHub Copilot (ghcp) +Provider identity note: +- Providers that do not expose a reliable email no longer require a manual nickname during first auth. +- CCS derives a stable internal account identifier from the token/cache context and still allows the user to rename the account later. + ``` +===========================================================================+ | OAuth - Device Code Flow (No Port Needed) | @@ -432,6 +436,8 @@ function selectBestAccount(accounts: AccountInfo[]): AccountInfo | null { - Device Code method uses /start route (no callback port) - Callback/social methods use /start-url + status polling - Some management flows return state first, auth_url later + - Manual nicknames are optional when the upstream provider does not return an email + - Account storage uses a stable internal identifier so reauth/update flows do not depend on dashboard list order ``` ### API Key Profiles (GLM, Kimi) diff --git a/src/api/services/profile-lifecycle-service.ts b/src/api/services/profile-lifecycle-service.ts index 5e91f455..a3e69a7a 100644 --- a/src/api/services/profile-lifecycle-service.ts +++ b/src/api/services/profile-lifecycle-service.ts @@ -12,11 +12,7 @@ import { getCcsDir, getConfigPath, loadConfigSafe } from '../../utils/config-man import { ensureProfileHooks } from '../../utils/websearch/profile-hook-injector'; import { isSensitiveKey } from '../../utils/sensitive-keys'; import { isReservedName } from '../../config/reserved-names'; -import { - isUnifiedMode, - loadOrCreateUnifiedConfig, - saveUnifiedConfig, -} from '../../config/unified-config-loader'; +import { isUnifiedMode, mutateUnifiedConfig } from '../../config/unified-config-loader'; import { validateApiName } from './validation-service'; import { listApiProfiles } from './profile-reader'; import { validateApiProfileSettingsPayload } from './profile-lifecycle-validation'; @@ -64,17 +60,17 @@ function writeJsonObjectAtomically(filePath: string, value: unknown): void { function registerApiProfileInConfig(name: string, target: TargetType, force = false): void { if (isUnifiedMode()) { - const config = loadOrCreateUnifiedConfig(); - if (config.profiles[name] && !force) { - throw new Error(`API profile already exists: ${name}`); - } + mutateUnifiedConfig((config) => { + if (config.profiles[name] && !force) { + throw new Error(`API profile already exists: ${name}`); + } - config.profiles[name] = { - type: 'api', - settings: `~/.ccs/${name}${SETTINGS_FILE_SUFFIX}`, - ...(target !== 'claude' && { target }), - }; - saveUnifiedConfig(config); + config.profiles[name] = { + type: 'api', + settings: `~/.ccs/${name}${SETTINGS_FILE_SUFFIX}`, + ...(target !== 'claude' && { target }), + }; + }); return; } diff --git a/src/api/services/profile-writer.ts b/src/api/services/profile-writer.ts index ae77908c..a0e230de 100644 --- a/src/api/services/profile-writer.ts +++ b/src/api/services/profile-writer.ts @@ -7,11 +7,7 @@ import * as path from 'path'; import { getCcsDir, getConfigPath, loadConfigSafe } from '../../utils/config-manager'; import { expandPath } from '../../utils/helpers'; import { validateApiName } from './validation-service'; -import { - loadOrCreateUnifiedConfig, - saveUnifiedConfig, - isUnifiedMode, -} from '../../config/unified-config-loader'; +import { mutateUnifiedConfig, isUnifiedMode } from '../../config/unified-config-loader'; import { ensureProfileHooks } from '../../utils/websearch/profile-hook-injector'; import type { TargetType } from '../../targets/target-adapter'; import { resolveDroidProvider } from '../../targets/droid-provider'; @@ -198,13 +194,13 @@ function createApiProfileUnified( // Inject WebSearch hooks into profile settings ensureProfileHooks(name); - const config = loadOrCreateUnifiedConfig(); - config.profiles[name] = { - type: 'api', - settings: `~/.ccs/${settingsFile}`, - ...(target !== 'claude' && { target }), - }; - saveUnifiedConfig(config); + mutateUnifiedConfig((config) => { + config.profiles[name] = { + type: 'api', + settings: `~/.ccs/${settingsFile}`, + ...(target !== 'claude' && { target }), + }; + }); } /** Create a new API profile */ @@ -308,17 +304,17 @@ export function updateApiProfileTarget( ): UpdateApiProfileTargetResult { try { if (isUnifiedMode()) { - const config = loadOrCreateUnifiedConfig(); - if (!config.profiles[name]) { - return { success: false, error: `API profile not found: ${name}` }; - } + mutateUnifiedConfig((config) => { + if (!config.profiles[name]) { + throw new Error(`API profile not found: ${name}`); + } - if (target === 'claude') { - delete config.profiles[name].target; - } else { - config.profiles[name].target = target; - } - saveUnifiedConfig(config); + if (target === 'claude') { + delete config.profiles[name].target; + } else { + config.profiles[name].target = target; + } + }); return { success: true, target }; } @@ -357,30 +353,26 @@ export function updateApiProfileTarget( /** Remove API profile from unified config */ function removeApiProfileUnified(name: string): void { - const config = loadOrCreateUnifiedConfig(); - const profile = config.profiles[name]; + mutateUnifiedConfig((config) => { + const profile = config.profiles[name]; - if (!profile) { - throw new Error(`API profile not found: ${name}`); - } - - // Delete the settings file if it exists. - // Uses expandPath() for cross-platform path handling. - if (profile.settings) { - const settingsPath = expandPath(profile.settings); - if (fs.existsSync(settingsPath)) { - fs.unlinkSync(settingsPath); + if (!profile) { + throw new Error(`API profile not found: ${name}`); } - } - delete config.profiles[name]; + if (profile.settings) { + const settingsPath = expandPath(profile.settings); + if (fs.existsSync(settingsPath)) { + fs.unlinkSync(settingsPath); + } + } - // Clear default if it was the deleted profile - if (config.default === name) { - config.default = undefined; - } + delete config.profiles[name]; - saveUnifiedConfig(config); + if (config.default === name) { + config.default = undefined; + } + }); } /** Remove API profile from legacy config */ diff --git a/src/auth/profile-registry.ts b/src/auth/profile-registry.ts index fb47da90..a0b944b2 100644 --- a/src/auth/profile-registry.ts +++ b/src/auth/profile-registry.ts @@ -3,7 +3,7 @@ import * as path from 'path'; import { ProfileMetadata } from '../types'; import { loadOrCreateUnifiedConfig, - saveUnifiedConfig, + mutateUnifiedConfig, isUnifiedMode, } from '../config/unified-config-loader'; import type { AccountConfig } from '../config/unified-config-types'; @@ -313,74 +313,72 @@ export class ProfileRegistry { * Create account in unified config (config.yaml) */ createAccountUnified(name: string, metadata: CreateMetadata = {}): void { - const config = loadOrCreateUnifiedConfig(); - if (config.accounts[name]) { - throw new Error(`Account already exists: ${name}`); - } - config.accounts[name] = this.normalizeUnifiedAccountConfig({ - created: new Date().toISOString(), - last_used: null, - context_mode: metadata.context_mode, - context_group: metadata.context_group, - continuity_mode: metadata.continuity_mode, - bare: metadata.bare, + mutateUnifiedConfig((config) => { + if (config.accounts[name]) { + throw new Error(`Account already exists: ${name}`); + } + config.accounts[name] = this.normalizeUnifiedAccountConfig({ + created: new Date().toISOString(), + last_used: null, + context_mode: metadata.context_mode, + context_group: metadata.context_group, + continuity_mode: metadata.continuity_mode, + bare: metadata.bare, + }); }); - saveUnifiedConfig(config); } /** * Update account metadata in unified config */ updateAccountUnified(name: string, updates: Partial): void { - const config = loadOrCreateUnifiedConfig(); - if (!config.accounts[name]) { - throw new Error(`Account not found: ${name}`); - } - config.accounts[name] = this.normalizeUnifiedAccountConfig({ - ...config.accounts[name], - ...updates, + mutateUnifiedConfig((config) => { + if (!config.accounts[name]) { + throw new Error(`Account not found: ${name}`); + } + config.accounts[name] = this.normalizeUnifiedAccountConfig({ + ...config.accounts[name], + ...updates, + }); }); - saveUnifiedConfig(config); } /** * Remove account from unified config */ removeAccountUnified(name: string): void { - const config = loadOrCreateUnifiedConfig(); - if (!config.accounts[name]) { - throw new Error(`Account not found: ${name}`); - } - delete config.accounts[name]; - // Clear default if it was the deleted account - if (config.default === name) { - config.default = undefined; - } - saveUnifiedConfig(config); + mutateUnifiedConfig((config) => { + if (!config.accounts[name]) { + throw new Error(`Account not found: ${name}`); + } + delete config.accounts[name]; + if (config.default === name) { + config.default = undefined; + } + }); } /** * Set default profile in unified config */ setDefaultUnified(name: string): void { - const config = loadOrCreateUnifiedConfig(); - // Check if exists in accounts, profiles, or cliproxy variants - const exists = - config.accounts[name] || config.profiles[name] || config.cliproxy?.variants?.[name]; - if (!exists) { - throw new Error(`Profile not found: ${name}`); - } - config.default = name; - saveUnifiedConfig(config); + mutateUnifiedConfig((config) => { + const exists = + config.accounts[name] || config.profiles[name] || config.cliproxy?.variants?.[name]; + if (!exists) { + throw new Error(`Profile not found: ${name}`); + } + config.default = name; + }); } /** * Clear default profile in unified config (restore original CCS behavior) */ clearDefaultUnified(): void { - const config = loadOrCreateUnifiedConfig(); - config.default = undefined; - saveUnifiedConfig(config); + mutateUnifiedConfig((config) => { + config.default = undefined; + }); } /** @@ -418,13 +416,13 @@ export class ProfileRegistry { * Update account last_used in unified config */ touchAccountUnified(name: string): void { - const config = loadOrCreateUnifiedConfig(); - if (!config.accounts[name]) { - throw new Error(`Account not found: ${name}`); - } - config.accounts[name].last_used = new Date().toISOString(); - config.accounts[name] = this.normalizeUnifiedAccountConfig(config.accounts[name]); - saveUnifiedConfig(config); + mutateUnifiedConfig((config) => { + if (!config.accounts[name]) { + throw new Error(`Account not found: ${name}`); + } + config.accounts[name].last_used = new Date().toISOString(); + config.accounts[name] = this.normalizeUnifiedAccountConfig(config.accounts[name]); + }); } // ========================================== diff --git a/src/cliproxy/accounts/query.ts b/src/cliproxy/accounts/query.ts index c1189129..4cee1123 100644 --- a/src/cliproxy/accounts/query.ts +++ b/src/cliproxy/accounts/query.ts @@ -6,7 +6,7 @@ import { CLIProxyProvider } from '../types'; import { CLIPROXY_PROFILES } from '../../auth/profile-detector'; import { AccountInfo } from './types'; -import { loadAccountsRegistry, syncRegistryWithTokenFiles, saveAccountsRegistry } from './registry'; +import { loadAccountsRegistry, syncRegistryWithTokenFiles } from './registry'; /** * Get all accounts for a provider @@ -14,10 +14,8 @@ import { loadAccountsRegistry, syncRegistryWithTokenFiles, saveAccountsRegistry export function getProviderAccounts(provider: CLIProxyProvider): AccountInfo[] { const registry = loadAccountsRegistry(); - // Sync with actual token files (removes stale entries) - if (syncRegistryWithTokenFiles(registry)) { - saveAccountsRegistry(registry); - } + // Sync in-memory view with actual token files without mutating disk on read. + syncRegistryWithTokenFiles(registry); const providerAccounts = registry.providers[provider]; diff --git a/src/cliproxy/accounts/registry.ts b/src/cliproxy/accounts/registry.ts index 6287d50d..0c7228a1 100644 --- a/src/cliproxy/accounts/registry.ts +++ b/src/cliproxy/accounts/registry.ts @@ -5,9 +5,10 @@ import * as fs from 'fs'; import * as path from 'path'; +import * as lockfile from 'proper-lockfile'; import { CLIProxyProvider } from '../types'; import { PROVIDER_TYPE_VALUES } from '../auth/auth-types'; -import { getAuthDir } from '../config-generator'; +import { getAuthDir, getCliproxyDir } from '../config-generator'; import { AccountsRegistry, AccountInfo, PROVIDERS_WITHOUT_EMAIL } from './types'; import { getAccountsRegistryPath, @@ -30,32 +31,58 @@ function createDefaultRegistry(): AccountsRegistry { }; } -/** - * Load accounts registry - */ -export function loadAccountsRegistry(): AccountsRegistry { +function withAccountsRegistryLock(callback: () => T): T { + const lockTarget = getCliproxyDir(); + let release: (() => void) | undefined; + + if (!fs.existsSync(lockTarget)) { + fs.mkdirSync(lockTarget, { recursive: true, mode: 0o700 }); + } + + try { + release = lockfile.lockSync(lockTarget, { stale: 10000 }) as () => void; + return callback(); + } finally { + if (release) { + try { + release(); + } catch { + // Best-effort release + } + } + } +} + +function readAccountsRegistryFromDisk(): AccountsRegistry { const registryPath = getAccountsRegistryPath(); if (!fs.existsSync(registryPath)) { return createDefaultRegistry(); } + const content = fs.readFileSync(registryPath, 'utf-8'); + let data: unknown; try { - const content = fs.readFileSync(registryPath, 'utf-8'); - const data = JSON.parse(content); - return { - version: data.version || 1, - providers: data.providers || {}, - }; - } catch { - return createDefaultRegistry(); + data = JSON.parse(content); + } catch (error) { + throw new Error(`Accounts registry is corrupted: ${(error as Error).message}`); } + + if (!data || typeof data !== 'object') { + throw new Error('Accounts registry is corrupted: expected object'); + } + + const parsed = data as { version?: unknown; providers?: unknown }; + return { + version: typeof parsed.version === 'number' ? parsed.version : 1, + providers: + parsed.providers && typeof parsed.providers === 'object' + ? (parsed.providers as AccountsRegistry['providers']) + : {}, + }; } -/** - * Save accounts registry - */ -export function saveAccountsRegistry(registry: AccountsRegistry): void { +function writeAccountsRegistryToDisk(registry: AccountsRegistry): void { const registryPath = getAccountsRegistryPath(); const dir = path.dirname(registryPath); @@ -63,9 +90,39 @@ export function saveAccountsRegistry(registry: AccountsRegistry): void { fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); } - fs.writeFileSync(registryPath, JSON.stringify(registry, null, 2) + '\n', { + const tempPath = `${registryPath}.tmp.${process.pid}`; + fs.writeFileSync(tempPath, JSON.stringify(registry, null, 2) + '\n', { mode: 0o600, }); + fs.renameSync(tempPath, registryPath); +} + +function mutateAccountsRegistry(mutator: (registry: AccountsRegistry) => T): T { + return withAccountsRegistryLock(() => { + const registry = readAccountsRegistryFromDisk(); + const initialSnapshot = JSON.stringify(registry); + const result = mutator(registry); + if (JSON.stringify(registry) !== initialSnapshot) { + writeAccountsRegistryToDisk(registry); + } + return result; + }); +} + +/** + * Load accounts registry + */ +export function loadAccountsRegistry(): AccountsRegistry { + return readAccountsRegistryFromDisk(); +} + +/** + * Save accounts registry + */ +export function saveAccountsRegistry(registry: AccountsRegistry): void { + withAccountsRegistryLock(() => { + writeAccountsRegistryToDisk(registry); + }); } /** @@ -133,110 +190,103 @@ export function registerAccount( nickname?: string, projectId?: string ): AccountInfo { - const registry = loadAccountsRegistry(); + return mutateAccountsRegistry((registry) => { + syncRegistryWithTokenFiles(registry); - // Initialize provider section if needed - if (!registry.providers[provider]) { - registry.providers[provider] = { - default: 'default', - accounts: {}, - }; - } - - const providerAccounts = registry.providers[provider]; - if (!providerAccounts) { - throw new Error('Failed to initialize provider accounts'); - } - - // Determine account ID based on provider type - let accountId: string; - let accountNickname: string; - - if (PROVIDERS_WITHOUT_EMAIL.includes(provider)) { - accountId = email - ? extractAccountIdFromTokenFile(tokenFile, email) - : deriveNoEmailProviderAccountId(provider, tokenFile, providerAccounts.accounts); - const existingAccount = providerAccounts.accounts[accountId]; - - if (nickname) { - const validationError = validateNickname(nickname); - if (validationError) { - throw new Error(validationError); - } - - const existingAccounts = Object.entries(providerAccounts.accounts).map(([id, account]) => ({ - id, - nickname: account.nickname, - })); - if (hasAccountNameConflict(existingAccounts, nickname, accountId)) { - throw new Error( - `An account with nickname "${nickname}" already exists for ${provider}. ` + - `Choose a different nickname.` - ); - } + if (!registry.providers[provider]) { + registry.providers[provider] = { + default: 'default', + accounts: {}, + }; } - accountNickname = - nickname || existingAccount?.nickname || (email ? generateNickname(email) : accountId); - } else { - // For other providers: use email as accountId, fallback to filename extraction - accountId = extractAccountIdFromTokenFile(tokenFile, email); - accountNickname = nickname || generateNickname(email); - } + const providerAccounts = registry.providers[provider]; + if (!providerAccounts) { + throw new Error('Failed to initialize provider accounts'); + } - const isFirstAccount = Object.keys(providerAccounts.accounts).length === 0; + let accountId: string; + let accountNickname: string; - // Create or update account - const existingAccount = providerAccounts.accounts[accountId]; - const accountMeta: Omit = { - email, - nickname: accountNickname, - tokenFile, - createdAt: existingAccount?.createdAt || new Date().toISOString(), - lastUsedAt: new Date().toISOString(), - }; + if (PROVIDERS_WITHOUT_EMAIL.includes(provider)) { + accountId = email + ? extractAccountIdFromTokenFile(tokenFile, email) + : deriveNoEmailProviderAccountId(provider, tokenFile, providerAccounts.accounts); + const existingAccount = providerAccounts.accounts[accountId]; - // Include projectId for Antigravity accounts - if (provider === 'agy' && projectId) { - accountMeta.projectId = projectId; - } + if (nickname) { + const validationError = validateNickname(nickname); + if (validationError) { + throw new Error(validationError); + } - providerAccounts.accounts[accountId] = accountMeta; + const existingAccounts = Object.entries(providerAccounts.accounts).map(([id, account]) => ({ + id, + nickname: account.nickname, + })); + if (hasAccountNameConflict(existingAccounts, nickname, accountId)) { + throw new Error( + `An account with nickname "${nickname}" already exists for ${provider}. ` + + `Choose a different nickname.` + ); + } + } - // Set as default if first account - if (isFirstAccount) { - providerAccounts.default = accountId; - } + accountNickname = + nickname || existingAccount?.nickname || (email ? generateNickname(email) : accountId); + } else { + accountId = extractAccountIdFromTokenFile(tokenFile, email); + accountNickname = nickname || generateNickname(email); + } - saveAccountsRegistry(registry); + const isFirstAccount = Object.keys(providerAccounts.accounts).length === 0; + const existingAccount = providerAccounts.accounts[accountId]; + const accountMeta: Omit = { + email, + nickname: accountNickname, + tokenFile, + createdAt: existingAccount?.createdAt || new Date().toISOString(), + lastUsedAt: new Date().toISOString(), + }; - return { - id: accountId, - provider, - isDefault: accountId === providerAccounts.default, - email, - nickname: accountNickname, - tokenFile, - createdAt: providerAccounts.accounts[accountId].createdAt, - lastUsedAt: providerAccounts.accounts[accountId].lastUsedAt, - projectId: providerAccounts.accounts[accountId].projectId, - }; + if (provider === 'agy' && projectId) { + accountMeta.projectId = projectId; + } + + providerAccounts.accounts[accountId] = accountMeta; + + if (isFirstAccount) { + providerAccounts.default = accountId; + } + + return { + id: accountId, + provider, + isDefault: accountId === providerAccounts.default, + email, + nickname: accountNickname, + tokenFile, + createdAt: providerAccounts.accounts[accountId].createdAt, + lastUsedAt: providerAccounts.accounts[accountId].lastUsedAt, + projectId: providerAccounts.accounts[accountId].projectId, + }; + }); } /** * Set default account for a provider */ export function setDefaultAccount(provider: CLIProxyProvider, accountId: string): boolean { - const registry = loadAccountsRegistry(); - const providerAccounts = registry.providers[provider]; + return mutateAccountsRegistry((registry) => { + const providerAccounts = registry.providers[provider]; - if (!providerAccounts || !providerAccounts.accounts[accountId]) { - return false; - } + if (!providerAccounts || !providerAccounts.accounts[accountId]) { + return false; + } - providerAccounts.default = accountId; - saveAccountsRegistry(registry); - return true; + providerAccounts.default = accountId; + return true; + }); } /** @@ -244,27 +294,26 @@ export function setDefaultAccount(provider: CLIProxyProvider, accountId: string) * Moves token file to paused/ subdir so CLIProxyAPI won't discover it */ export function pauseAccount(provider: CLIProxyProvider, accountId: string): boolean { - const registry = loadAccountsRegistry(); - const providerAccounts = registry.providers[provider]; + return mutateAccountsRegistry((registry) => { + const providerAccounts = registry.providers[provider]; - if (!providerAccounts?.accounts[accountId]) { - return false; - } + if (!providerAccounts?.accounts[accountId]) { + return false; + } - const accountMeta = providerAccounts.accounts[accountId]; + const accountMeta = providerAccounts.accounts[accountId]; + if (accountMeta.paused) { + return true; + } - // Skip if already paused (idempotent) - if (accountMeta.paused) { + if (!moveTokenToPaused(accountMeta.tokenFile)) { + return false; + } + + providerAccounts.accounts[accountId].paused = true; + providerAccounts.accounts[accountId].pausedAt = new Date().toISOString(); return true; - } - - // Move token file to paused directory (if it exists in auth dir) - moveTokenToPaused(accountMeta.tokenFile); - - providerAccounts.accounts[accountId].paused = true; - providerAccounts.accounts[accountId].pausedAt = new Date().toISOString(); - saveAccountsRegistry(registry); - return true; + }); } /** @@ -272,55 +321,53 @@ export function pauseAccount(provider: CLIProxyProvider, accountId: string): boo * Moves token file back from paused/ to auth/ so CLIProxyAPI can discover it */ export function resumeAccount(provider: CLIProxyProvider, accountId: string): boolean { - const registry = loadAccountsRegistry(); - const providerAccounts = registry.providers[provider]; + return mutateAccountsRegistry((registry) => { + const providerAccounts = registry.providers[provider]; - if (!providerAccounts?.accounts[accountId]) { - return false; - } + if (!providerAccounts?.accounts[accountId]) { + return false; + } - const accountMeta = providerAccounts.accounts[accountId]; + const accountMeta = providerAccounts.accounts[accountId]; + if (!accountMeta.paused) { + return true; + } - // Skip if already active (idempotent) - if (!accountMeta.paused) { + if (!moveTokenFromPaused(accountMeta.tokenFile)) { + return false; + } + + providerAccounts.accounts[accountId].paused = false; + providerAccounts.accounts[accountId].pausedAt = undefined; return true; - } - - // Move token file back from paused directory (if it exists in paused dir) - moveTokenFromPaused(accountMeta.tokenFile); - - providerAccounts.accounts[accountId].paused = false; - providerAccounts.accounts[accountId].pausedAt = undefined; - saveAccountsRegistry(registry); - return true; + }); } /** * Remove an account */ export function removeAccount(provider: CLIProxyProvider, accountId: string): boolean { - const registry = loadAccountsRegistry(); - const providerAccounts = registry.providers[provider]; + return mutateAccountsRegistry((registry) => { + const providerAccounts = registry.providers[provider]; - if (!providerAccounts || !providerAccounts.accounts[accountId]) { - return false; - } + if (!providerAccounts || !providerAccounts.accounts[accountId]) { + return false; + } - // Delete token file from both auth and paused directories - const tokenFile = providerAccounts.accounts[accountId].tokenFile; - deleteTokenFile(tokenFile); + const tokenFile = providerAccounts.accounts[accountId].tokenFile; + if (!deleteTokenFile(tokenFile)) { + return false; + } - // Remove from registry - delete providerAccounts.accounts[accountId]; + delete providerAccounts.accounts[accountId]; - // Update default if needed - const remainingAccounts = Object.keys(providerAccounts.accounts); - if (providerAccounts.default === accountId && remainingAccounts.length > 0) { - providerAccounts.default = remainingAccounts[0]; - } + const remainingAccounts = Object.keys(providerAccounts.accounts); + if (providerAccounts.default === accountId && remainingAccounts.length > 0) { + providerAccounts.default = remainingAccounts[0]; + } - saveAccountsRegistry(registry); - return true; + return true; + }); } /** @@ -336,37 +383,36 @@ export function renameAccount( throw new Error(validationError); } - const registry = loadAccountsRegistry(); - const providerAccounts = registry.providers[provider]; + return mutateAccountsRegistry((registry) => { + const providerAccounts = registry.providers[provider]; - if (!providerAccounts?.accounts[accountId]) { - return false; - } + if (!providerAccounts?.accounts[accountId]) { + return false; + } - const existingAccounts = Object.entries(providerAccounts.accounts).map(([id, account]) => ({ - id, - nickname: account.nickname, - })); - if (hasAccountNameConflict(existingAccounts, newNickname, accountId)) { - throw new Error(`Nickname "${newNickname}" is already used by another account`); - } + const existingAccounts = Object.entries(providerAccounts.accounts).map(([id, account]) => ({ + id, + nickname: account.nickname, + })); + if (hasAccountNameConflict(existingAccounts, newNickname, accountId)) { + throw new Error(`Nickname "${newNickname}" is already used by another account`); + } - providerAccounts.accounts[accountId].nickname = newNickname; - saveAccountsRegistry(registry); - return true; + providerAccounts.accounts[accountId].nickname = newNickname; + return true; + }); } /** * Update last used timestamp for an account */ export function touchAccount(provider: CLIProxyProvider, accountId: string): void { - const registry = loadAccountsRegistry(); - const providerAccounts = registry.providers[provider]; - - if (providerAccounts?.accounts[accountId]) { - providerAccounts.accounts[accountId].lastUsedAt = new Date().toISOString(); - saveAccountsRegistry(registry); - } + mutateAccountsRegistry((registry) => { + const providerAccounts = registry.providers[provider]; + if (providerAccounts?.accounts[accountId]) { + providerAccounts.accounts[accountId].lastUsedAt = new Date().toISOString(); + } + }); } /** @@ -377,16 +423,16 @@ export function setAccountTier( accountId: string, tier: 'free' | 'pro' | 'ultra' | 'unknown' ): boolean { - const registry = loadAccountsRegistry(); - const providerAccounts = registry.providers[provider]; + return mutateAccountsRegistry((registry) => { + const providerAccounts = registry.providers[provider]; - if (!providerAccounts?.accounts[accountId]) { - return false; - } + if (!providerAccounts?.accounts[accountId]) { + return false; + } - providerAccounts.accounts[accountId].tier = tier; - saveAccountsRegistry(registry); - return true; + providerAccounts.accounts[accountId].tier = tier; + return true; + }); } /** @@ -404,163 +450,103 @@ export function discoverExistingAccounts(): void { return; } - const registry = loadAccountsRegistry(); const files = fs.readdirSync(authDir); + mutateAccountsRegistry((registry) => { + syncRegistryWithTokenFiles(registry); - // Track whether any accounts were discovered (to avoid saving empty registry) - let discoveredCount = 0; + for (const file of files) { + if (!file.endsWith('.json')) continue; - for (const file of files) { - if (!file.endsWith('.json')) continue; + const filePath = path.join(authDir, file); - const filePath = path.join(authDir, file); + try { + const content = fs.readFileSync(filePath, 'utf-8'); + const data = JSON.parse(content); + if (!data.type) continue; - try { - const content = fs.readFileSync(filePath, 'utf-8'); - const data = JSON.parse(content); - - // Skip if no type field - if (!data.type) continue; - - // Build reverse mapping from PROVIDER_TYPE_VALUES (type value -> provider) - // e.g., "antigravity" -> "agy", "kiro" -> "kiro", "codewhisperer" -> "kiro" - const typeValue = data.type.toLowerCase(); - let provider: CLIProxyProvider | undefined; - for (const [prov, typeValues] of Object.entries(PROVIDER_TYPE_VALUES)) { - if (typeValues.includes(typeValue)) { - provider = prov as CLIProxyProvider; - break; + const typeValue = data.type.toLowerCase(); + let provider: CLIProxyProvider | undefined; + for (const [prov, typeValues] of Object.entries(PROVIDER_TYPE_VALUES)) { + if (typeValues.includes(typeValue)) { + provider = prov as CLIProxyProvider; + break; + } } - } - // Skip if unknown provider type - if (!provider) { - continue; - } - - // Extract email if available, fallback to filename-based ID - let email = data.email || undefined; - - // Fallback: extract email from filename (e.g., "kiro-google-user@example.com.json") - if (!email && file.includes('@')) { - const match = file.match(/([^-]+@[^.]+\.[^.]+)(?=\.json$)/); - if (match) { - email = match[1]; + if (!provider) { + continue; } - } - // Initialize provider section if needed - if (!registry.providers[provider]) { - registry.providers[provider] = { - default: 'default', - accounts: {}, + let email = data.email || undefined; + if (!email && file.includes('@')) { + const match = file.match(/([^-]+@[^.]+\.[^.]+)(?=\.json$)/); + if (match) { + email = match[1]; + } + } + + if (!registry.providers[provider]) { + registry.providers[provider] = { + default: 'default', + accounts: {}, + }; + } + + const providerAccounts = registry.providers[provider]; + if (!providerAccounts) continue; + + const existingTokenFiles = Object.values(providerAccounts.accounts).map((a) => a.tokenFile); + if (existingTokenFiles.includes(file)) { + const projectIdValue = + typeof data.project_id === 'string' && data.project_id.trim() + ? data.project_id.trim() + : null; + if (provider === 'agy' && projectIdValue) { + const existingEntry = Object.entries(providerAccounts.accounts).find( + ([, meta]) => meta.tokenFile === file + ); + if (existingEntry && existingEntry[1].projectId !== projectIdValue) { + existingEntry[1].projectId = projectIdValue; + } + } + continue; + } + + const accountId = + PROVIDERS_WITHOUT_EMAIL.includes(provider) && !email + ? deriveNoEmailProviderAccountId(provider, file, providerAccounts.accounts) + : extractAccountIdFromTokenFile(file, email); + + if (providerAccounts.accounts[accountId]) { + continue; + } + + if (Object.keys(providerAccounts.accounts).length === 0) { + providerAccounts.default = accountId; + } + + const stats = fs.statSync(filePath); + const lastModified = stats.mtime || stats.birthtime || new Date(); + const accountMeta: Omit = { + email, + nickname: email ? generateNickname(email) : accountId, + tokenFile: file, + createdAt: stats.birthtime?.toISOString() || new Date().toISOString(), + lastUsedAt: lastModified.toISOString(), }; - } - const providerAccounts = registry.providers[provider]; - if (!providerAccounts) continue; - - // Skip if token file already registered (under any accountId) - const existingTokenFiles = Object.values(providerAccounts.accounts).map((a) => a.tokenFile); - if (existingTokenFiles.includes(file)) { - // Token file exists - check if we need to update projectId for agy accounts - const projectIdValue = + const discoveredProjectId = typeof data.project_id === 'string' && data.project_id.trim() ? data.project_id.trim() : null; - if (provider === 'agy' && projectIdValue) { - const existingEntry = Object.entries(providerAccounts.accounts).find( - ([, meta]) => meta.tokenFile === file - ); - // Update if missing or changed - if (existingEntry && existingEntry[1].projectId !== projectIdValue) { - existingEntry[1].projectId = projectIdValue; - discoveredCount++; // Count projectId updates as changes - } + if (provider === 'agy' && discoveredProjectId) { + accountMeta.projectId = discoveredProjectId; } + + providerAccounts.accounts[accountId] = accountMeta; + } catch { continue; } - - const accountId = - PROVIDERS_WITHOUT_EMAIL.includes(provider) && !email - ? deriveNoEmailProviderAccountId(provider, file, providerAccounts.accounts) - : extractAccountIdFromTokenFile(file, email); - - // Skip if account already registered - if (providerAccounts.accounts[accountId]) { - continue; - } - - // Set as default if first account - if (Object.keys(providerAccounts.accounts).length === 0) { - providerAccounts.default = accountId; - } - - // Get file stats for creation time - const stats = fs.statSync(filePath); - - // Register account with auto-generated nickname - // Use mtime as lastUsedAt (when token was last modified = last auth/refresh) - const lastModified = stats.mtime || stats.birthtime || new Date(); - const accountMeta: Omit = { - email, - nickname: email ? generateNickname(email) : accountId, - tokenFile: file, - createdAt: stats.birthtime?.toISOString() || new Date().toISOString(), - lastUsedAt: lastModified.toISOString(), - }; - - // Read project_id for Antigravity accounts (read-only field from auth token) - const discoveredProjectId = - typeof data.project_id === 'string' && data.project_id.trim() - ? data.project_id.trim() - : null; - if (provider === 'agy' && discoveredProjectId) { - accountMeta.projectId = discoveredProjectId; - } - - providerAccounts.accounts[accountId] = accountMeta; - discoveredCount++; - } catch { - // Skip invalid files - continue; } - } - - // Only save if at least one account was discovered or updated - // This prevents creating accounts.json with empty provider sections - if (discoveredCount === 0) { - return; - } - - // Reload-merge pattern: reduce race condition with concurrent OAuth registration - // Reload fresh registry and merge discovered accounts (fresh registry wins on conflicts) - const freshRegistry = loadAccountsRegistry(); - for (const [providerName, discovered] of Object.entries(registry.providers)) { - if (!discovered) continue; - // Skip empty provider sections (no accounts discovered) - if (Object.keys(discovered.accounts).length === 0) continue; - - const prov = providerName as CLIProxyProvider; - if (!freshRegistry.providers[prov]) { - freshRegistry.providers[prov] = discovered; - } else { - // Merge accounts, preferring fresh registry's existing entries but updating projectId - const freshProviderAccounts = freshRegistry.providers[prov]; - if (!freshProviderAccounts) continue; - for (const [id, meta] of Object.entries(discovered.accounts)) { - if (!freshProviderAccounts.accounts[id]) { - freshProviderAccounts.accounts[id] = meta; - // Set default if none exists - if (!freshProviderAccounts.default || freshProviderAccounts.default === 'default') { - freshProviderAccounts.default = id; - } - } else if (meta.projectId && !freshProviderAccounts.accounts[id].projectId) { - // Update existing account with projectId if discovered from auth file - freshProviderAccounts.accounts[id].projectId = meta.projectId; - } - } - } - } - saveAccountsRegistry(freshRegistry); + }); } diff --git a/src/cliproxy/accounts/token-file-ops.ts b/src/cliproxy/accounts/token-file-ops.ts index 2f35d95d..74b51e85 100644 --- a/src/cliproxy/accounts/token-file-ops.ts +++ b/src/cliproxy/accounts/token-file-ops.ts @@ -93,16 +93,17 @@ export function moveTokenFromPaused(tokenFile: string): boolean { * Delete token file from both auth and paused directories * Idempotent */ -export function deleteTokenFile(tokenFile: string): void { +export function deleteTokenFile(tokenFile: string): boolean { const tokenPath = path.join(getAuthDir(), tokenFile); const pausedPath = path.join(getPausedDir(), tokenFile); + let success = true; // Delete from auth directory if (fs.existsSync(tokenPath)) { try { fs.unlinkSync(tokenPath); } catch { - // Ignore deletion errors + success = false; } } @@ -111,9 +112,11 @@ export function deleteTokenFile(tokenFile: string): void { try { fs.unlinkSync(pausedPath); } catch { - // Ignore deletion errors + success = false; } } + + return success; } /** diff --git a/src/cliproxy/ai-providers/config-store.ts b/src/cliproxy/ai-providers/config-store.ts index 2cc665f4..e1d1f79d 100644 --- a/src/cliproxy/ai-providers/config-store.ts +++ b/src/cliproxy/ai-providers/config-store.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'crypto'; import * as fs from 'fs'; import * as yaml from 'js-yaml'; import { configExists, getCliproxyConfigPath, regenerateConfig } from '../config-generator'; @@ -21,6 +22,41 @@ type FamilyEntriesMap = { export type FamilyEntries = FamilyEntriesMap[F]; +type EntryWithOptionalId = { + id?: string; +}; + +function createFamilyEntryId(family: AiProviderFamilyId): string { + return `${family}-${randomUUID()}`; +} + +function ensureStableEntryIds( + family: F, + entries: FamilyEntries +): { entries: FamilyEntries; changed: boolean } { + const seenIds = new Set(); + let changed = false; + + const normalizedEntries = entries.map((entry) => { + const rawId = (entry as EntryWithOptionalId).id; + const normalizedId = typeof rawId === 'string' && rawId.trim().length > 0 ? rawId.trim() : null; + const nextId = + normalizedId && !seenIds.has(normalizedId) ? normalizedId : createFamilyEntryId(family); + + if (normalizedId !== nextId) { + changed = true; + } + + seenIds.add(nextId); + return { + ...entry, + id: nextId, + }; + }) as FamilyEntries; + + return { entries: normalizedEntries, changed }; +} + function ensureLocalConfigPath(): string { if (!configExists()) { regenerateConfig(); @@ -91,7 +127,12 @@ export async function readFamilyEntries( if (!target.isRemote) { const config = readLocalConfig(); - return (config[family] || []) as FamilyEntries; + const entries = (Array.isArray(config[family]) ? config[family] : []) as FamilyEntries; + const normalized = ensureStableEntryIds(family, entries); + if (normalized.changed) { + writeLocalFamilySection(family, normalized.entries); + } + return normalized.entries; } const client = createManagementClient({ @@ -102,17 +143,29 @@ export async function readFamilyEntries( auth_token: target.authToken, }); - return client.getSection[number]>(family) as Promise>; + const entries = (await client.getSection[number]>(family)) as FamilyEntries; + const normalized = ensureStableEntryIds( + family, + Array.isArray(entries) ? entries : ([] as unknown as FamilyEntries) + ); + if (normalized.changed) { + await client.putSection[number]>( + family, + normalized.entries as FamilyEntries[number][] + ); + } + return normalized.entries; } export async function writeFamilyEntries( family: F, entries: FamilyEntries ): Promise { + const normalized = ensureStableEntryIds(family, entries); const target = getProxyTarget(); if (!target.isRemote) { - writeLocalFamilySection(family, entries); + writeLocalFamilySection(family, normalized.entries); return; } @@ -124,5 +177,8 @@ export async function writeFamilyEntries( auth_token: target.authToken, }); - await client.putSection[number]>(family, entries as FamilyEntries[number][]); + await client.putSection[number]>( + family, + normalized.entries as FamilyEntries[number][] + ); } diff --git a/src/cliproxy/ai-providers/service.ts b/src/cliproxy/ai-providers/service.ts index a03f959a..12a5e496 100644 --- a/src/cliproxy/ai-providers/service.ts +++ b/src/cliproxy/ai-providers/service.ts @@ -51,7 +51,7 @@ function buildApiKeyEntryView( index: number ): AiProviderEntryView { return { - id: `${family}:${index}`, + id: entry.id || `${family}:${index}`, index, label: entry.prefix?.trim() || entry['base-url']?.trim() || `Entry ${index + 1}`, baseUrl: entry['base-url']?.trim() || undefined, @@ -67,7 +67,7 @@ function buildApiKeyEntryView( function buildOpenAiCompatEntryView(entry: OpenAICompatEntry, index: number): AiProviderEntryView { return { - id: `openai-compatibility:${index}`, + id: entry.id || `openai-compatibility:${index}`, index, name: entry.name, label: entry.name, @@ -131,6 +131,7 @@ function toApiKeyEntry( : existing?.['api-key'] || ''; return { + id: existing?.id, 'api-key': nextSecret, 'base-url': input.baseUrl?.trim() || undefined, 'proxy-url': input.proxyUrl?.trim() || undefined, @@ -155,6 +156,7 @@ function toOpenAiCompatEntry( : (existing?.['api-key-entries'] || []).map((entry) => entry['api-key']); return { + id: existing?.id, name: input.name?.trim() || existing?.name || 'connector', 'base-url': input.baseUrl?.trim() || existing?.['base-url'] || '', headers: normalizeHeaders(input.headers), @@ -163,8 +165,41 @@ function toOpenAiCompatEntry( }; } -function assertIndex(entries: unknown[], index: number): void { - if (!Number.isInteger(index) || index < 0 || index >= entries.length) { +function resolveEntryIndex(entries: Array<{ id?: string }>, entryId: string): number { + const normalizedEntryId = entryId.trim(); + const matchedIndex = entries.findIndex((entry) => entry.id === normalizedEntryId); + if (matchedIndex !== -1) { + return matchedIndex; + } + + const legacyIndex = Number.parseInt(normalizedEntryId, 10); + if ( + Number.isInteger(legacyIndex) && + legacyIndex >= 0 && + legacyIndex < entries.length && + String(legacyIndex) === normalizedEntryId + ) { + return legacyIndex; + } + + if (normalizedEntryId.startsWith('openai-compatibility:') || normalizedEntryId.includes(':')) { + const legacySuffix = normalizedEntryId.split(':').at(-1) || ''; + const legacySuffixIndex = Number.parseInt(legacySuffix, 10); + if ( + Number.isInteger(legacySuffixIndex) && + legacySuffixIndex >= 0 && + legacySuffixIndex < entries.length && + String(legacySuffixIndex) === legacySuffix + ) { + return legacySuffixIndex; + } + } + + throw new Error('Entry not found'); +} + +function assertEntryId(entryId: string): void { + if (!entryId.trim()) { throw new Error('Entry not found'); } } @@ -208,12 +243,14 @@ export async function createAiProviderEntry( export async function updateAiProviderEntry( family: AiProviderFamilyId, - index: number, + entryId: string, input: UpsertAiProviderEntryInput ): Promise { + assertEntryId(entryId); + if (family === 'openai-compatibility') { const entries = await readFamilyEntries(family); - assertIndex(entries, index); + const index = resolveEntryIndex(entries, entryId); validateFamilyInput(family, input); entries[index] = toOpenAiCompatEntry(input, entries[index]); await writeFamilyEntries(family, entries); @@ -221,7 +258,7 @@ export async function updateAiProviderEntry( } const entries = await readFamilyEntries(family); - assertIndex(entries, index); + const index = resolveEntryIndex(entries, entryId); validateFamilyInput(family, input); entries[index] = toApiKeyEntry(input, entries[index]); await writeFamilyEntries(family, entries); @@ -229,18 +266,20 @@ export async function updateAiProviderEntry( export async function deleteAiProviderEntry( family: AiProviderFamilyId, - index: number + entryId: string ): Promise { + assertEntryId(entryId); + if (family === 'openai-compatibility') { const entries = await readFamilyEntries(family); - assertIndex(entries, index); + const index = resolveEntryIndex(entries, entryId); entries.splice(index, 1); await writeFamilyEntries(family, entries); return; } const entries = await readFamilyEntries(family); - assertIndex(entries, index); + const index = resolveEntryIndex(entries, entryId); entries.splice(index, 1); await writeFamilyEntries(family, entries); } diff --git a/src/cliproxy/ai-providers/types.ts b/src/cliproxy/ai-providers/types.ts index 3fe9cff6..a6d0bf7e 100644 --- a/src/cliproxy/ai-providers/types.ts +++ b/src/cliproxy/ai-providers/types.ts @@ -14,6 +14,7 @@ export interface AiProviderModelAlias { } export interface AiProviderApiKeyEntry { + id?: string; 'api-key': string; 'base-url'?: string; 'proxy-url'?: string; @@ -29,6 +30,7 @@ export interface OpenAICompatApiKeyEntry { } export interface OpenAICompatEntry { + id?: string; name: string; 'base-url': string; headers?: Record; diff --git a/src/cliproxy/auth-token-manager.ts b/src/cliproxy/auth-token-manager.ts index 4daebb13..ead0ce99 100644 --- a/src/cliproxy/auth-token-manager.ts +++ b/src/cliproxy/auth-token-manager.ts @@ -8,7 +8,7 @@ */ import { randomBytes } from 'crypto'; -import { loadOrCreateUnifiedConfig, saveUnifiedConfig } from '../config/unified-config-loader'; +import { loadOrCreateUnifiedConfig, mutateUnifiedConfig } from '../config/unified-config-loader'; import { CCS_INTERNAL_API_KEY, CCS_CONTROL_PANEL_SECRET } from './config-generator'; /** @@ -87,19 +87,17 @@ export function getEffectiveManagementSecret(): string { * @param apiKey - New API key (or undefined to reset to default) */ export function setGlobalApiKey(apiKey: string | undefined): void { - const config = loadOrCreateUnifiedConfig(); + mutateUnifiedConfig((config) => { + if (!config.cliproxy.auth) { + config.cliproxy.auth = {}; + } - if (!config.cliproxy.auth) { - config.cliproxy.auth = {}; - } - - if (apiKey === undefined) { - delete config.cliproxy.auth.api_key; - } else { - config.cliproxy.auth.api_key = apiKey; - } - - saveUnifiedConfig(config); + if (apiKey === undefined) { + delete config.cliproxy.auth.api_key; + } else { + config.cliproxy.auth.api_key = apiKey; + } + }); } /** @@ -109,19 +107,17 @@ export function setGlobalApiKey(apiKey: string | undefined): void { * @param secret - New management secret (or undefined to reset to default) */ export function setGlobalManagementSecret(secret: string | undefined): void { - const config = loadOrCreateUnifiedConfig(); + mutateUnifiedConfig((config) => { + if (!config.cliproxy.auth) { + config.cliproxy.auth = {}; + } - if (!config.cliproxy.auth) { - config.cliproxy.auth = {}; - } - - if (secret === undefined) { - delete config.cliproxy.auth.management_secret; - } else { - config.cliproxy.auth.management_secret = secret; - } - - saveUnifiedConfig(config); + if (secret === undefined) { + delete config.cliproxy.auth.management_secret; + } else { + config.cliproxy.auth.management_secret = secret; + } + }); } /** @@ -132,28 +128,26 @@ export function setGlobalManagementSecret(secret: string | undefined): void { * @param apiKey - New API key (or undefined to remove override) */ export function setVariantApiKey(variantName: string, apiKey: string | undefined): void { - const config = loadOrCreateUnifiedConfig(); - const variant = config.cliproxy.variants[variantName]; + mutateUnifiedConfig((config) => { + const variant = config.cliproxy.variants[variantName]; - if (!variant) { - throw new Error(`Variant '${variantName}' not found`); - } - - if (!variant.auth) { - variant.auth = {}; - } - - if (apiKey === undefined) { - delete variant.auth.api_key; - // Clean up empty auth object - if (Object.keys(variant.auth).length === 0) { - delete variant.auth; + if (!variant) { + throw new Error(`Variant '${variantName}' not found`); } - } else { - variant.auth.api_key = apiKey; - } - saveUnifiedConfig(config); + if (!variant.auth) { + variant.auth = {}; + } + + if (apiKey === undefined) { + delete variant.auth.api_key; + if (Object.keys(variant.auth).length === 0) { + delete variant.auth; + } + } else { + variant.auth.api_key = apiKey; + } + }); } /** @@ -161,20 +155,16 @@ export function setVariantApiKey(variantName: string, apiKey: string | undefined * Removes cliproxy.auth and all variant auth overrides. */ export function resetAuthToDefaults(): void { - const config = loadOrCreateUnifiedConfig(); + mutateUnifiedConfig((config) => { + delete config.cliproxy.auth; - // Remove global auth - delete config.cliproxy.auth; - - // Remove all variant auth overrides - for (const variantName of Object.keys(config.cliproxy.variants)) { - const variant = config.cliproxy.variants[variantName]; - if (variant.auth) { - delete variant.auth; + for (const variantName of Object.keys(config.cliproxy.variants)) { + const variant = config.cliproxy.variants[variantName]; + if (variant.auth) { + delete variant.auth; + } } - } - - saveUnifiedConfig(config); + }); } /** diff --git a/src/cliproxy/auth/token-manager.ts b/src/cliproxy/auth/token-manager.ts index 7ed68a36..788ebe73 100644 --- a/src/cliproxy/auth/token-manager.ts +++ b/src/cliproxy/auth/token-manager.ts @@ -11,7 +11,7 @@ import { CLIProxyProvider } from '../types'; import { CLIPROXY_PROFILES } from '../../auth/profile-detector'; import { getProviderAuthDir } from '../config-generator'; import { getProviderAccounts, getDefaultAccount } from '../account-manager'; -import { deleteTokenFile } from '../accounts/token-file-ops'; +import { deleteTokenFile, extractAccountIdFromTokenFile } from '../accounts/token-file-ops'; import { AuthStatus, PROVIDER_AUTH_PREFIXES, @@ -206,40 +206,78 @@ export function registerAccountFromToken( verbose = false, expectedAccountId?: string ): import('../account-manager').AccountInfo | null { + type TokenCandidate = { + file: string; + filePath: string; + email?: string; + projectId?: string; + accountId: string; + mtimeMs: number; + alreadyRegistered: boolean; + }; + const { registerAccount } = require('../account-manager'); - let newestFile: string | null = null; + let selectedCandidate: Omit | null = null; try { const files = fs.readdirSync(tokenDir); const jsonFiles = files.filter((f: string) => f.endsWith('.json')); + const existingAccounts = getProviderAccounts(provider); + const candidates: TokenCandidate[] = jsonFiles + .map((file): TokenCandidate | null => { + const filePath = path.join(tokenDir, file); + if (!isTokenFileForProvider(filePath, provider)) return null; - let newestMtime = 0; + const content = fs.readFileSync(filePath, 'utf-8'); + const data = JSON.parse(content) as { email?: string; project_id?: string }; + const email = data.email || undefined; + const projectId = data.project_id || undefined; + const accountId = extractAccountIdFromTokenFile(file, email); + const stats = fs.statSync(filePath); + return { + file, + filePath, + email, + projectId, + accountId, + mtimeMs: stats.mtimeMs, + alreadyRegistered: existingAccounts.some((account) => account.tokenFile === file), + }; + }) + .filter((candidate): candidate is TokenCandidate => candidate !== null) + .sort((a, b) => b.mtimeMs - a.mtimeMs); - for (const file of jsonFiles) { - const filePath = path.join(tokenDir, file); - if (!isTokenFileForProvider(filePath, provider)) continue; - - const stats = fs.statSync(filePath); - if (stats.mtimeMs > newestMtime) { - newestMtime = stats.mtimeMs; - newestFile = file; - } + if (expectedAccountId) { + selectedCandidate = + candidates.find((candidate) => candidate.accountId === expectedAccountId) || + candidates.find((candidate) => { + const existingAccount = existingAccounts.find( + (account) => account.id === expectedAccountId + ); + return !!existingAccount && existingAccount.tokenFile === candidate.file; + }) || + null; + } else { + selectedCandidate = candidates[0] || null; } - if (!newestFile) { + if (!selectedCandidate) { + if (verbose && expectedAccountId) { + console.error( + `[auth] No token matched the expected account ${expectedAccountId}; refusing ambiguous registration` + ); + } return null; } - const tokenPath = path.join(tokenDir, newestFile); - const content = fs.readFileSync(tokenPath, 'utf-8'); - const data = JSON.parse(content); - const email = data.email || undefined; - const projectId = data.project_id || undefined; - - const account = registerAccount(provider, newestFile, email, nickname, projectId); - - // Upload token to remote server if configured (async, don't block) - uploadTokenToRemoteAsync(tokenPath, verbose); + const account = registerAccount( + provider, + selectedCandidate.file, + selectedCandidate.email, + nickname, + selectedCandidate.projectId + ); + uploadTokenToRemoteAsync(selectedCandidate.filePath, verbose); return account; } catch (error) { const message = error instanceof Error ? error.message : String(error); @@ -247,13 +285,8 @@ export function registerAccountFromToken( console.error(`[auth] Failed to register token-backed account: ${message}`); } - if (typeof newestFile === 'string') { - const hasExistingRegistration = getProviderAccounts(provider).some( - (account) => account.tokenFile === newestFile - ); - if (!hasExistingRegistration) { - deleteTokenFile(newestFile); - } + if (selectedCandidate && !selectedCandidate.alreadyRegistered && !expectedAccountId) { + deleteTokenFile(selectedCandidate.file); } if (expectedAccountId && verbose) { diff --git a/src/cliproxy/services/variant-config-adapter.ts b/src/cliproxy/services/variant-config-adapter.ts index e8179b51..2f4814f4 100644 --- a/src/cliproxy/services/variant-config-adapter.ts +++ b/src/cliproxy/services/variant-config-adapter.ts @@ -10,7 +10,7 @@ import { } from '../../config/unified-config-types'; import { loadOrCreateUnifiedConfig, - saveUnifiedConfig, + mutateUnifiedConfig, isUnifiedMode, } from '../../config/unified-config-loader'; import { CLIPROXY_DEFAULT_PORT } from '../config-generator'; @@ -171,21 +171,20 @@ export function listVariantsFromConfig(): Record { } export function saveCompositeVariantUnified(name: string, config: CompositeVariantConfig): void { - const unifiedConfig = loadOrCreateUnifiedConfig(); + mutateUnifiedConfig((unifiedConfig) => { + if (!unifiedConfig.cliproxy) { + unifiedConfig.cliproxy = { + oauth_accounts: {}, + providers: [...CLIPROXY_SUPPORTED_PROVIDERS], + variants: {}, + }; + } + if (!unifiedConfig.cliproxy.variants) { + unifiedConfig.cliproxy.variants = {}; + } - if (!unifiedConfig.cliproxy) { - unifiedConfig.cliproxy = { - oauth_accounts: {}, - providers: [...CLIPROXY_SUPPORTED_PROVIDERS], - variants: {}, - }; - } - if (!unifiedConfig.cliproxy.variants) { - unifiedConfig.cliproxy.variants = {}; - } - - unifiedConfig.cliproxy.variants[name] = config; - saveUnifiedConfig(unifiedConfig); + unifiedConfig.cliproxy.variants[name] = config; + }); } export function saveVariantUnified( @@ -196,28 +195,26 @@ export function saveVariantUnified( port?: number, target: TargetType = 'claude' ): void { - const config = loadOrCreateUnifiedConfig(); + mutateUnifiedConfig((config) => { + if (!config.cliproxy) { + config.cliproxy = { + oauth_accounts: {}, + providers: [...CLIPROXY_SUPPORTED_PROVIDERS], + variants: {}, + }; + } + if (!config.cliproxy.variants) { + config.cliproxy.variants = {}; + } - if (!config.cliproxy) { - config.cliproxy = { - oauth_accounts: {}, - providers: [...CLIPROXY_SUPPORTED_PROVIDERS], - variants: {}, + config.cliproxy.variants[name] = { + provider, + account, + settings: settingsPath, + port, + ...(target !== 'claude' && { target }), }; - } - if (!config.cliproxy.variants) { - config.cliproxy.variants = {}; - } - - config.cliproxy.variants[name] = { - provider, - account, - settings: settingsPath, - port, - ...(target !== 'claude' && { target }), - }; - - saveUnifiedConfig(config); + }); } export function saveVariantLegacy( @@ -268,18 +265,22 @@ export function saveVariantLegacy( } export function removeVariantFromUnifiedConfig(name: string): VariantConfig | null { - const config = loadOrCreateUnifiedConfig(); + let removedVariant: CLIProxyVariantConfig | CompositeVariantConfig | null = null; + mutateUnifiedConfig((config) => { + if (!config.cliproxy?.variants || !(name in config.cliproxy.variants)) { + return; + } - if (!config.cliproxy?.variants || !(name in config.cliproxy.variants)) { + removedVariant = config.cliproxy.variants[name]; + delete config.cliproxy.variants[name]; + }); + + if (!removedVariant) { return null; } - const variant = config.cliproxy.variants[name]; - delete config.cliproxy.variants[name]; - saveUnifiedConfig(config); - - if ('type' in variant && variant.type === 'composite') { - const composite = variant as CompositeVariantConfig; + const composite = removedVariant as CompositeVariantConfig; + if (composite.type === 'composite') { return { provider: composite.tiers[composite.default_tier].provider, settings: composite.settings, @@ -290,7 +291,7 @@ export function removeVariantFromUnifiedConfig(name: string): VariantConfig | nu tiers: composite.tiers, }; } - const singleVariant = variant as CLIProxyVariantConfig; + const singleVariant = removedVariant as CLIProxyVariantConfig; return { provider: singleVariant.provider, settings: singleVariant.settings, diff --git a/src/commands/cliproxy/proxy-lifecycle-subcommand.ts b/src/commands/cliproxy/proxy-lifecycle-subcommand.ts index 1dad5b5a..1d289475 100644 --- a/src/commands/cliproxy/proxy-lifecycle-subcommand.ts +++ b/src/commands/cliproxy/proxy-lifecycle-subcommand.ts @@ -11,17 +11,7 @@ import { initUI, header, color, dim, ok, warn, info } from '../../utils/ui'; import { getProxyStatus, startProxy, stopProxy } from '../../cliproxy/services'; import { detectRunningProxy } from '../../cliproxy/proxy-detector'; -import { CLIPROXY_DEFAULT_PORT, validatePort } from '../../cliproxy/config/port-manager'; -import { loadOrCreateUnifiedConfig } from '../../config/unified-config-loader'; - -/** - * Resolve the local CLIProxy lifecycle port from unified config. - * Falls back to default port when unset/invalid. - */ -export function resolveLifecyclePort(): number { - const config = loadOrCreateUnifiedConfig(); - return validatePort(config.cliproxy_server?.local?.port ?? CLIPROXY_DEFAULT_PORT); -} +import { resolveLifecyclePort } from './resolve-lifecycle-port'; export async function handleStart(verbose = false): Promise { await initUI(); diff --git a/src/commands/cliproxy/resolve-lifecycle-port.ts b/src/commands/cliproxy/resolve-lifecycle-port.ts new file mode 100644 index 00000000..85e0b33c --- /dev/null +++ b/src/commands/cliproxy/resolve-lifecycle-port.ts @@ -0,0 +1,11 @@ +import { CLIPROXY_DEFAULT_PORT, validatePort } from '../../cliproxy/config/port-manager'; +import { loadOrCreateUnifiedConfig } from '../../config/unified-config-loader'; + +/** + * Resolve the local CLIProxy lifecycle port from unified config. + * Falls back to default port when unset/invalid. + */ +export function resolveLifecyclePort(): number { + const config = loadOrCreateUnifiedConfig(); + return validatePort(config.cliproxy_server?.local?.port ?? CLIPROXY_DEFAULT_PORT); +} diff --git a/src/commands/config-auth/disable-command.ts b/src/commands/config-auth/disable-command.ts index fd66cb46..d6efb0c2 100644 --- a/src/commands/config-auth/disable-command.ts +++ b/src/commands/config-auth/disable-command.ts @@ -5,11 +5,7 @@ */ import { InteractivePrompt } from '../../utils/prompt'; -import { - getDashboardAuthConfig, - loadOrCreateUnifiedConfig, - saveUnifiedConfig, -} from '../../config/unified-config-loader'; +import { getDashboardAuthConfig, mutateUnifiedConfig } from '../../config/unified-config-loader'; import { initUI, header, ok, info, warn, dim } from '../../utils/ui'; /** @@ -57,15 +53,14 @@ export async function handleDisable(): Promise { } // Disable auth - const fullConfig = loadOrCreateUnifiedConfig(); - fullConfig.dashboard_auth = { - enabled: false, - username: fullConfig.dashboard_auth?.username ?? '', - password_hash: fullConfig.dashboard_auth?.password_hash ?? '', - session_timeout_hours: fullConfig.dashboard_auth?.session_timeout_hours ?? 24, - }; - - saveUnifiedConfig(fullConfig); + mutateUnifiedConfig((fullConfig) => { + fullConfig.dashboard_auth = { + enabled: false, + username: fullConfig.dashboard_auth?.username ?? '', + password_hash: fullConfig.dashboard_auth?.password_hash ?? '', + session_timeout_hours: fullConfig.dashboard_auth?.session_timeout_hours ?? 24, + }; + }); console.log(''); console.log(ok('Dashboard authentication disabled')); diff --git a/src/commands/config-auth/setup-command.ts b/src/commands/config-auth/setup-command.ts index de40c32d..75df5d39 100644 --- a/src/commands/config-auth/setup-command.ts +++ b/src/commands/config-auth/setup-command.ts @@ -8,7 +8,7 @@ import bcrypt from 'bcrypt'; import { InteractivePrompt } from '../../utils/prompt'; -import { loadOrCreateUnifiedConfig, saveUnifiedConfig } from '../../config/unified-config-loader'; +import { mutateUnifiedConfig } from '../../config/unified-config-loader'; import { initUI, header, subheader, ok, fail, info, warn, dim } from '../../utils/ui'; import type { AuthSetupResult } from './types'; @@ -99,24 +99,23 @@ export async function handleSetup(): Promise { // Hash password const passwordHash = await bcrypt.hash(password, BCRYPT_ROUNDS); - // Load existing config and update - const config = loadOrCreateUnifiedConfig(); - config.dashboard_auth = { - enabled: true, - username, - password_hash: passwordHash, - session_timeout_hours: config.dashboard_auth?.session_timeout_hours ?? 24, - }; - - // Save config - saveUnifiedConfig(config); + const config = mutateUnifiedConfig((currentConfig) => { + currentConfig.dashboard_auth = { + enabled: true, + username, + password_hash: passwordHash, + session_timeout_hours: currentConfig.dashboard_auth?.session_timeout_hours ?? 24, + }; + }); console.log(''); console.log(ok('Dashboard authentication configured')); console.log(''); console.log(info('Settings saved to ~/.ccs/config.yaml')); console.log(info(`Username: ${username}`)); - console.log(info(`Session timeout: ${config.dashboard_auth.session_timeout_hours} hours`)); + console.log( + info(`Session timeout: ${config.dashboard_auth?.session_timeout_hours ?? 24} hours`) + ); console.log(''); console.log(dim(' Start dashboard: ccs config')); console.log(dim(' Show status: ccs config auth show')); diff --git a/src/commands/copilot-command.ts b/src/commands/copilot-command.ts index 40d36bcc..f25ebd52 100644 --- a/src/commands/copilot-command.ts +++ b/src/commands/copilot-command.ts @@ -15,7 +15,7 @@ import { normalizeCopilotConfigWithWarnings, } from '../copilot'; import type { CopilotModel } from '../copilot'; -import { loadOrCreateUnifiedConfig, saveUnifiedConfig } from '../config/unified-config-loader'; +import { loadOrCreateUnifiedConfig, mutateUnifiedConfig } from '../config/unified-config-loader'; import { DEFAULT_COPILOT_CONFIG } from '../config/unified-config-types'; import { ok, fail, info, color, warn } from '../utils/ui'; import { normalizeCopilotSubcommand } from '../copilot/constants'; @@ -361,14 +361,13 @@ async function handleStop(): Promise { * Handle enable subcommand. */ async function handleEnable(): Promise { - const config = loadOrCreateUnifiedConfig(); + mutateUnifiedConfig((config) => { + if (!config.copilot) { + config.copilot = { ...DEFAULT_COPILOT_CONFIG }; + } - if (!config.copilot) { - config.copilot = { ...DEFAULT_COPILOT_CONFIG }; - } - - config.copilot.enabled = true; - saveUnifiedConfig(config); + config.copilot.enabled = true; + }); console.log(ok('Copilot integration enabled')); console.log(''); @@ -384,12 +383,11 @@ async function handleEnable(): Promise { * Handle disable subcommand. */ async function handleDisable(): Promise { - const config = loadOrCreateUnifiedConfig(); - - if (config.copilot) { - config.copilot.enabled = false; - saveUnifiedConfig(config); - } + mutateUnifiedConfig((config) => { + if (config.copilot) { + config.copilot.enabled = false; + } + }); console.log(ok('Copilot integration disabled')); diff --git a/src/commands/cursor-command.ts b/src/commands/cursor-command.ts index 585c395e..ae8d5363 100644 --- a/src/commands/cursor-command.ts +++ b/src/commands/cursor-command.ts @@ -15,11 +15,7 @@ import { getAvailableModels, getDefaultModel, } from '../cursor'; -import { - getCursorConfig, - loadOrCreateUnifiedConfig, - saveUnifiedConfig, -} from '../config/unified-config-loader'; +import { getCursorConfig, mutateUnifiedConfig } from '../config/unified-config-loader'; import { DEFAULT_CURSOR_CONFIG } from '../config/unified-config-types'; import { renderCursorHelp, renderCursorModels, renderCursorStatus } from './cursor-command-display'; import { ok, fail, info } from '../utils/ui'; @@ -239,14 +235,13 @@ async function handleStop(): Promise { * Handle enable subcommand. */ async function handleEnable(): Promise { - const config = loadOrCreateUnifiedConfig(); + mutateUnifiedConfig((config) => { + if (!config.cursor) { + config.cursor = { ...DEFAULT_CURSOR_CONFIG }; + } - if (!config.cursor) { - config.cursor = { ...DEFAULT_CURSOR_CONFIG }; - } - - config.cursor.enabled = true; - saveUnifiedConfig(config); + config.cursor.enabled = true; + }); console.log(ok('Cursor integration enabled')); console.log(''); @@ -262,12 +257,11 @@ async function handleEnable(): Promise { * Handle disable subcommand. */ async function handleDisable(): Promise { - const config = loadOrCreateUnifiedConfig(); - - if (config.cursor) { - config.cursor.enabled = false; - saveUnifiedConfig(config); - } + mutateUnifiedConfig((config) => { + if (config.cursor) { + config.cursor.enabled = false; + } + }); console.log(ok('Cursor integration disabled')); return 0; diff --git a/src/commands/setup-command.ts b/src/commands/setup-command.ts index 681ea9ad..ed1f884d 100644 --- a/src/commands/setup-command.ts +++ b/src/commands/setup-command.ts @@ -19,7 +19,7 @@ import { initUI, header, ok, info, warn } from '../utils/ui'; import { loadOrCreateUnifiedConfig, loadUnifiedConfig, - saveUnifiedConfig, + mutateUnifiedConfig, hasUnifiedConfig, } from '../config/unified-config-loader'; import { DEFAULT_CLIPROXY_SERVER_CONFIG } from '../config/unified-config-types'; @@ -377,9 +377,10 @@ async function runSetupWizard(force: boolean = false): Promise { console.log(' After creating, edit the settings file to add your API key.'); } - // Save config - config.setup_completed = true; - saveUnifiedConfig(config); + mutateUnifiedConfig((currentConfig) => { + currentConfig.setup_completed = true; + currentConfig.cliproxy_server = config.cliproxy_server; + }); // Final summary console.log(''); diff --git a/src/config/unified-config-loader.ts b/src/config/unified-config-loader.ts index 19bd3266..108f9f20 100644 --- a/src/config/unified-config-loader.ts +++ b/src/config/unified-config-loader.ts @@ -7,6 +7,7 @@ import * as fs from 'fs'; import * as path from 'path'; +import * as crypto from 'crypto'; import * as yaml from 'js-yaml'; import { getCcsDir } from '../utils/config-manager'; import { @@ -64,65 +65,73 @@ function getLockFilePath(): string { /** * Acquire lockfile for config write operations. - * Returns true if lock acquired, false if already locked by another process. + * Returns a lock token if acquired, null if already locked by another process. * Cleans up stale locks (older than LOCK_STALE_MS). */ -function acquireLock(): boolean { +function acquireLock(): string | null { const lockPath = getLockFilePath(); - const lockData = `${process.pid}\n${Date.now()}`; + const lockToken = crypto.randomUUID(); + const lockData = `${process.pid}\n${Date.now()}\n${lockToken}`; try { // Check if lock exists if (fs.existsSync(lockPath)) { const content = fs.readFileSync(lockPath, 'utf8'); const [pidStr, timestampStr] = content.trim().split('\n'); - const timestamp = parseInt(timestampStr, 10); + const pid = Number.parseInt(pidStr, 10); + const timestamp = Number.parseInt(timestampStr, 10); + const hasLiveOwner = Number.isInteger(pid) && pid > 0 && processExists(pid); + const isStale = !Number.isFinite(timestamp) || Date.now() - timestamp > LOCK_STALE_MS; - // Check if lock is stale - if (Date.now() - timestamp > LOCK_STALE_MS) { - // Stale lock - remove and acquire + if (hasLiveOwner) { + return null; + } + + if (isStale || !hasLiveOwner) { fs.unlinkSync(lockPath); - } else { - // Check if process still exists - try { - process.kill(parseInt(pidStr, 10), 0); // Signal 0 checks if process exists - // Process exists - lock is valid - return false; - } catch { - // Process doesn't exist - remove stale lock - fs.unlinkSync(lockPath); - } } } // Acquire lock fs.writeFileSync(lockPath, lockData, { flag: 'wx', mode: 0o600 }); - return true; + return lockToken; } catch (error) { // EEXIST means another process acquired the lock between our check and write if ((error as NodeJS.ErrnoException).code === 'EEXIST') { - return false; + return null; } - return false; + return null; } } /** * Release lockfile after config write operation. */ - -function releaseLock(): void { +function releaseLock(lockToken: string): void { const lockPath = getLockFilePath(); try { if (fs.existsSync(lockPath)) { - fs.unlinkSync(lockPath); + const content = fs.readFileSync(lockPath, 'utf8'); + const fileToken = content.trim().split('\n')[2]; + if (fileToken === lockToken) { + fs.unlinkSync(lockPath); + } } } catch { // Ignore cleanup errors } } +function processExists(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + /** * Check if unified config.yaml exists */ @@ -152,7 +161,7 @@ export function getConfigFormat(): 'yaml' | 'json' | 'none' { /** * Load unified config from YAML file. - * Returns null if file doesn't exist or format check fails. + * Returns null if file doesn't exist. * Auto-upgrades config if version is outdated (regenerates comments). */ export function loadUnifiedConfig(): UnifiedConfig | null { @@ -168,8 +177,7 @@ export function loadUnifiedConfig(): UnifiedConfig | null { const parsed = yaml.load(content); if (!isUnifiedConfig(parsed)) { - console.error(`[!] Invalid config format in ${yamlPath}`); - return null; + throw new Error(`Invalid config format in ${yamlPath}`); } // Auto-upgrade if version is outdated (regenerates YAML with new comments and fields) @@ -208,7 +216,7 @@ export function loadUnifiedConfig(): UnifiedConfig | null { const error = err instanceof Error ? err.message : 'Unknown error'; console.error(`[X] Failed to load config: ${error}`); } - return null; + throw err; } } @@ -808,16 +816,17 @@ function withConfigWriteLock(callback: () => T): T { // Acquire lock (retry for up to 1 second) const maxRetries = 10; const retryDelayMs = 100; - let lockAcquired = false; + let lockToken: string | null = null; for (let i = 0; i < maxRetries; i++) { - if (acquireLock()) { - lockAcquired = true; + const acquiredToken = acquireLock(); + if (acquiredToken) { + lockToken = acquiredToken; break; } sleepSync(retryDelayMs); } - if (!lockAcquired) { + if (!lockToken) { throw new Error('Config file is locked by another process. Wait a moment and try again.'); } @@ -825,7 +834,7 @@ function withConfigWriteLock(callback: () => T): T { return callback(); } finally { // Always release lock - releaseLock(); + releaseLock(lockToken); } } diff --git a/src/web-server/health-service.ts b/src/web-server/health-service.ts index 69703484..c76ae57f 100644 --- a/src/web-server/health-service.ts +++ b/src/web-server/health-service.ts @@ -145,12 +145,8 @@ export function fixHealthIssue(checkId: string): { success: boolean; message: st // Use appropriate config based on unified mode const { isUnifiedMode } = require('../config/unified-config-loader'); if (isUnifiedMode()) { - const { - loadOrCreateUnifiedConfig, - saveUnifiedConfig, - } = require('../config/unified-config-loader'); - const config = loadOrCreateUnifiedConfig(); - saveUnifiedConfig(config); + const { mutateUnifiedConfig } = require('../config/unified-config-loader'); + mutateUnifiedConfig(() => {}); return { success: true, message: 'Created/updated config.yaml' }; } const configPath = getConfigPath(); diff --git a/src/web-server/middleware/auth-middleware.ts b/src/web-server/middleware/auth-middleware.ts index 044963d7..cbdff588 100644 --- a/src/web-server/middleware/auth-middleware.ts +++ b/src/web-server/middleware/auth-middleware.ts @@ -132,3 +132,32 @@ export function authMiddleware(req: Request, res: Response, next: NextFunction): // Unauthorized res.status(401).json({ error: 'Authentication required' }); } + +export function isLoopbackRemoteAddress(value: string | undefined): boolean { + if (!value) return false; + const normalized = value.trim().replace(/^\[|\]$/g, ''); + return ( + normalized === '::1' || + normalized === '127.0.0.1' || + normalized.startsWith('127.') || + normalized === '::ffff:127.0.0.1' || + normalized.startsWith('::ffff:127.') + ); +} + +export function requireLocalAccessWhenAuthDisabled( + req: Request, + res: Response, + error = 'This endpoint requires localhost access when dashboard auth is disabled.' +): boolean { + if (getDashboardAuthConfig().enabled) { + return true; + } + + if (isLoopbackRemoteAddress(req.socket.remoteAddress)) { + return true; + } + + res.status(403).json({ error }); + return false; +} diff --git a/src/web-server/routes/ai-provider-routes.ts b/src/web-server/routes/ai-provider-routes.ts index cb2a3492..a7772dfe 100644 --- a/src/web-server/routes/ai-provider-routes.ts +++ b/src/web-server/routes/ai-provider-routes.ts @@ -8,9 +8,22 @@ import { type AiProviderFamilyId, type UpsertAiProviderEntryInput, } from '../../cliproxy/ai-providers'; +import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; const router = Router(); +router.use((req: Request, res: Response, next) => { + if ( + requireLocalAccessWhenAuthDisabled( + req, + res, + 'AI provider endpoints require localhost access when dashboard auth is disabled.' + ) + ) { + next(); + } +}); + function isAiProviderFamilyId(value: string): value is AiProviderFamilyId { return AI_PROVIDER_FAMILY_IDS.includes(value as AiProviderFamilyId); } @@ -24,13 +37,13 @@ function parseFamily(req: Request, res: Response): AiProviderFamilyId | null { return family; } -function parseIndex(req: Request, res: Response): number | null { - const index = Number.parseInt(req.params.index || '', 10); - if (!Number.isInteger(index) || index < 0) { - res.status(400).json({ error: 'Invalid entry index' }); +function parseEntryId(req: Request, res: Response): string | null { + const entryId = req.params.entryId?.trim(); + if (!entryId) { + res.status(400).json({ error: 'Invalid entry id' }); return null; } - return index; + return entryId; } function parseInput(body: unknown): UpsertAiProviderEntryInput { @@ -95,14 +108,14 @@ router.post('/:family', async (req: Request, res: Response) => { } }); -router.put('/:family/:index', async (req: Request, res: Response) => { +router.put('/:family/:entryId', async (req: Request, res: Response) => { const family = parseFamily(req, res); if (!family) return; - const index = parseIndex(req, res); - if (index === null) return; + const entryId = parseEntryId(req, res); + if (!entryId) return; try { - await updateAiProviderEntry(family, index, parseInput(req.body)); + await updateAiProviderEntry(family, entryId, parseInput(req.body)); res.json({ success: true }); } catch (error) { const message = (error as Error).message; @@ -110,14 +123,14 @@ router.put('/:family/:index', async (req: Request, res: Response) => { } }); -router.delete('/:family/:index', async (req: Request, res: Response) => { +router.delete('/:family/:entryId', async (req: Request, res: Response) => { const family = parseFamily(req, res); if (!family) return; - const index = parseIndex(req, res); - if (index === null) return; + const entryId = parseEntryId(req, res); + if (!entryId) return; try { - await deleteAiProviderEntry(family, index); + await deleteAiProviderEntry(family, entryId); res.json({ success: true }); } catch (error) { const message = (error as Error).message; diff --git a/src/web-server/routes/cliproxy-auth-routes.ts b/src/web-server/routes/cliproxy-auth-routes.ts index 0072fc67..b6a96a88 100644 --- a/src/web-server/routes/cliproxy-auth-routes.ts +++ b/src/web-server/routes/cliproxy-auth-routes.ts @@ -23,7 +23,6 @@ import { resumeAccount as resumeAccountFn, touchAccount, hasAccountNameConflict, - findAccountNameMatch, PROVIDERS_WITHOUT_EMAIL, validateNickname, } from '../../cliproxy/account-manager'; @@ -53,6 +52,7 @@ import { isAntigravityResponsibilityBypassEnabled, } from '../../cliproxy/antigravity-responsibility'; import { createRouteErrorHelpers } from './route-helpers'; +import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; const router = Router(); const MANUAL_AUTH_STATE_TTL_MS = 10 * 60 * 1000; @@ -66,6 +66,18 @@ const validProviders: CLIProxyProvider[] = [...CLIPROXY_PROFILES]; const { respondInternalError } = createRouteErrorHelpers('cliproxy-auth-routes'); +router.use((req: Request, res: Response, next) => { + if ( + requireLocalAccessWhenAuthDisabled( + req, + res, + 'CLIProxy auth endpoints require localhost access when dashboard auth is disabled.' + ) + ) { + next(); + } +}); + function pruneExpiredManualAuthState(now = Date.now()): void { for (const [state, pending] of pendingManualAuthState.entries()) { if (now - pending.createdAt > MANUAL_AUTH_STATE_TTL_MS) { @@ -149,6 +161,13 @@ export function getStartAuthFailureMessage(provider: CLIProxyProvider): string { return 'Authentication failed or was cancelled'; } +function getManualCallbackRegistrationError(provider: CLIProxyProvider): string { + if (PROVIDERS_WITHOUT_EMAIL.includes(provider)) { + return 'Authenticated token could not be matched to a new account. Retry the flow and choose a different nickname if needed.'; + } + return 'Authenticated token could not be registered. Retry the flow.'; +} + export function getStartAuthNicknameError( provider: CLIProxyProvider, nickname: string | undefined, @@ -504,12 +523,10 @@ router.post('/:provider/start', async (req: Request, res: Response): Promise(); +router.use((req: Request, res: Response, next) => { + if ( + requireLocalAccessWhenAuthDisabled( + req, + res, + 'CLIProxy management endpoints require localhost access when dashboard auth is disabled.' + ) + ) { + next(); + } +}); + function buildQuotaRateLimitKey(req: Request, provider: string): string { const clientIp = req.ip || req.socket.remoteAddress || 'unknown'; return `${clientIp}:${provider}`; diff --git a/src/web-server/routes/cliproxy-sync-routes.ts b/src/web-server/routes/cliproxy-sync-routes.ts index 1bbec5fc..d414bbda 100644 --- a/src/web-server/routes/cliproxy-sync-routes.ts +++ b/src/web-server/routes/cliproxy-sync-routes.ts @@ -3,7 +3,7 @@ */ import { Router, Request, Response } from 'express'; -import { loadOrCreateUnifiedConfig } from '../../config/unified-config-loader'; +import { mutateUnifiedConfig } from '../../config/unified-config-loader'; import { generateSyncPayload, generateSyncPreview, @@ -12,7 +12,6 @@ import { syncToLocalConfig, getLocalSyncStatus, } from '../../cliproxy/sync'; -import { saveUnifiedConfig } from '../../config/unified-config-loader'; const router = Router(); @@ -130,18 +129,13 @@ router.put('/auto-sync', async (req: Request, res: Response): Promise => { return; } - // Update config - const config = loadOrCreateUnifiedConfig(); - if (!config.cliproxy) { - // Should not happen as loadOrCreate initializes it, but handle gracefully - res.status(500).json({ error: 'CLIProxy config not initialized' }); - return; - } - - // Save config try { - config.cliproxy.auto_sync = enabled; - saveUnifiedConfig(config); + mutateUnifiedConfig((config) => { + if (!config.cliproxy) { + throw new Error('CLIProxy config not initialized'); + } + config.cliproxy.auto_sync = enabled; + }); } catch (error) { res.status(500).json({ error: `Failed to save config: ${(error as Error).message}` }); return; diff --git a/src/web-server/routes/cursor-settings-routes.ts b/src/web-server/routes/cursor-settings-routes.ts index 8a120dfe..56c9dcc1 100644 --- a/src/web-server/routes/cursor-settings-routes.ts +++ b/src/web-server/routes/cursor-settings-routes.ts @@ -9,11 +9,7 @@ import { promises as fsp } from 'fs'; import * as path from 'path'; import { getCcsDir } from '../../utils/config-manager'; import { DEFAULT_CURSOR_CONFIG } from '../../config/unified-config-types'; -import { - loadOrCreateUnifiedConfig, - saveUnifiedConfig, - getCursorConfig, -} from '../../config/unified-config-loader'; +import { mutateUnifiedConfig, getCursorConfig } from '../../config/unified-config-loader'; import type { CursorConfig } from '../../config/unified-config-types'; const router = Router(); @@ -184,36 +180,38 @@ router.put('/', async (req: Request, res: Response): Promise => { return; } - const config = loadOrCreateUnifiedConfig(); const normalizedModel = parseRequiredModel(updates.model); + const config = mutateUnifiedConfig((currentConfig) => { + currentConfig.cursor = { + enabled: updates.enabled ?? currentConfig.cursor?.enabled ?? DEFAULT_CURSOR_CONFIG.enabled, + port: updates.port ?? currentConfig.cursor?.port ?? DEFAULT_CURSOR_CONFIG.port, + auto_start: + updates.auto_start ?? + currentConfig.cursor?.auto_start ?? + DEFAULT_CURSOR_CONFIG.auto_start, + ghost_mode: + updates.ghost_mode ?? + currentConfig.cursor?.ghost_mode ?? + DEFAULT_CURSOR_CONFIG.ghost_mode, + model: normalizedModel ?? currentConfig.cursor?.model ?? DEFAULT_CURSOR_CONFIG.model, + opus_model: + 'opus_model' in updates + ? parseOptionalModel(updates.opus_model) + : currentConfig.cursor?.opus_model, + sonnet_model: + 'sonnet_model' in updates + ? parseOptionalModel(updates.sonnet_model) + : currentConfig.cursor?.sonnet_model, + haiku_model: + 'haiku_model' in updates + ? parseOptionalModel(updates.haiku_model) + : currentConfig.cursor?.haiku_model, + }; + }); - // Merge updates with existing config - // Only known fields are merged — unknown properties are ignored - config.cursor = { - enabled: updates.enabled ?? config.cursor?.enabled ?? DEFAULT_CURSOR_CONFIG.enabled, - port: updates.port ?? config.cursor?.port ?? DEFAULT_CURSOR_CONFIG.port, - auto_start: - updates.auto_start ?? config.cursor?.auto_start ?? DEFAULT_CURSOR_CONFIG.auto_start, - ghost_mode: - updates.ghost_mode ?? config.cursor?.ghost_mode ?? DEFAULT_CURSOR_CONFIG.ghost_mode, - model: normalizedModel ?? config.cursor?.model ?? DEFAULT_CURSOR_CONFIG.model, - opus_model: - 'opus_model' in updates - ? parseOptionalModel(updates.opus_model) - : config.cursor?.opus_model, - sonnet_model: - 'sonnet_model' in updates - ? parseOptionalModel(updates.sonnet_model) - : config.cursor?.sonnet_model, - haiku_model: - 'haiku_model' in updates - ? parseOptionalModel(updates.haiku_model) - : config.cursor?.haiku_model, - }; - - saveUnifiedConfig(config); - await syncRawSettingsFromCursorConfig(config.cursor); - res.json({ success: true, cursor: config.cursor }); + const cursorConfig = config.cursor ?? DEFAULT_CURSOR_CONFIG; + await syncRawSettingsFromCursorConfig(cursorConfig); + res.json({ success: true, cursor: cursorConfig }); } catch (error) { res.status(500).json({ error: (error as Error).message }); } @@ -295,19 +293,19 @@ router.put('/raw', (req: Request, res: Response): void => { // Keep unified config aligned with raw settings edits (parity with Copilot raw editor). const parsedPort = parseLocalCursorPort(settings); - const config = loadOrCreateUnifiedConfig(); const env = (settings as { env?: Record }).env ?? {}; - const model = parseRequiredModel(env.ANTHROPIC_MODEL) ?? config.cursor?.model; + mutateUnifiedConfig((config) => { + const model = parseRequiredModel(env.ANTHROPIC_MODEL) ?? config.cursor?.model; - config.cursor = { - ...(config.cursor ?? DEFAULT_CURSOR_CONFIG), - ...(parsedPort !== null ? { port: parsedPort } : {}), - ...(model ? { model } : {}), - opus_model: parseOptionalModel(env.ANTHROPIC_DEFAULT_OPUS_MODEL), - sonnet_model: parseOptionalModel(env.ANTHROPIC_DEFAULT_SONNET_MODEL), - haiku_model: parseOptionalModel(env.ANTHROPIC_DEFAULT_HAIKU_MODEL), - }; - saveUnifiedConfig(config); + config.cursor = { + ...(config.cursor ?? DEFAULT_CURSOR_CONFIG), + ...(parsedPort !== null ? { port: parsedPort } : {}), + ...(model ? { model } : {}), + opus_model: parseOptionalModel(env.ANTHROPIC_DEFAULT_OPUS_MODEL), + sonnet_model: parseOptionalModel(env.ANTHROPIC_DEFAULT_SONNET_MODEL), + haiku_model: parseOptionalModel(env.ANTHROPIC_DEFAULT_HAIKU_MODEL), + }; + }); const stat = fs.statSync(settingsPath); res.json({ success: true, mtime: stat.mtimeMs }); diff --git a/src/web-server/routes/misc-routes.ts b/src/web-server/routes/misc-routes.ts index a3c7fbdc..ce95c457 100644 --- a/src/web-server/routes/misc-routes.ts +++ b/src/web-server/routes/misc-routes.ts @@ -8,8 +8,6 @@ import * as path from 'path'; import { getCcsDir } from '../../utils/config-manager'; import { expandPath } from '../../utils/helpers'; import { - loadOrCreateUnifiedConfig, - saveUnifiedConfig, mutateUnifiedConfig, getGlobalEnvConfig, getThinkingConfig, @@ -24,9 +22,22 @@ import { THINKING_OFF_VALUES, } from '../../cliproxy'; import { validateFilePath } from './route-helpers'; +import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; const router = Router(); +router.use((req: Request, res: Response, next) => { + if ( + requireLocalAccessWhenAuthDisabled( + req, + res, + 'Local configuration endpoints require localhost access when dashboard auth is disabled.' + ) + ) { + next(); + } +}); + export function resolveThinkingProviderOverridesForSave( currentProviderOverrides: ThinkingConfig['provider_overrides'] | undefined, updatesProviderOverrides: Record> | undefined, @@ -304,14 +315,10 @@ router.put('/thinking', (req: Request, res: Response): void => { } } - const config = loadOrCreateUnifiedConfig(); const shouldClearOverride = clearOverrideFlag === true || updates.override === null; const shouldClearProviderOverrides = clearProviderOverridesFlag === true || updates.provider_overrides === null; - let normalizedOverride: string | number | undefined = config.thinking?.override as - | string - | number - | undefined; + let normalizedOverride: string | number | undefined; let normalizedProviderOverrides: | Record> | undefined; @@ -438,28 +445,32 @@ router.put('/thinking', (req: Request, res: Response): void => { Object.keys(sanitizedOverrides).length > 0 ? sanitizedOverrides : undefined; } - // Update thinking section - config.thinking = { - mode: updates.mode ?? config.thinking?.mode ?? 'auto', - override: shouldClearOverride - ? undefined - : updates.override !== undefined - ? normalizedOverride - : config.thinking?.override, - tier_defaults: { - opus: updates.tier_defaults?.opus ?? config.thinking?.tier_defaults?.opus ?? 'high', - sonnet: updates.tier_defaults?.sonnet ?? config.thinking?.tier_defaults?.sonnet ?? 'medium', - haiku: updates.tier_defaults?.haiku ?? config.thinking?.tier_defaults?.haiku ?? 'low', - }, - provider_overrides: resolveThinkingProviderOverridesForSave( - config.thinking?.provider_overrides, - updates.provider_overrides !== undefined ? normalizedProviderOverrides : undefined, - shouldClearProviderOverrides - ), - show_warnings: updates.show_warnings ?? config.thinking?.show_warnings ?? true, - }; - - saveUnifiedConfig(config); + const config = mutateUnifiedConfig((currentConfig) => { + currentConfig.thinking = { + mode: updates.mode ?? currentConfig.thinking?.mode ?? 'auto', + override: shouldClearOverride + ? undefined + : updates.override !== undefined + ? normalizedOverride + : currentConfig.thinking?.override, + tier_defaults: { + opus: + updates.tier_defaults?.opus ?? currentConfig.thinking?.tier_defaults?.opus ?? 'high', + sonnet: + updates.tier_defaults?.sonnet ?? + currentConfig.thinking?.tier_defaults?.sonnet ?? + 'medium', + haiku: + updates.tier_defaults?.haiku ?? currentConfig.thinking?.tier_defaults?.haiku ?? 'low', + }, + provider_overrides: resolveThinkingProviderOverridesForSave( + currentConfig.thinking?.provider_overrides, + updates.provider_overrides !== undefined ? normalizedProviderOverrides : undefined, + shouldClearProviderOverrides + ), + show_warnings: updates.show_warnings ?? currentConfig.thinking?.show_warnings ?? true, + }; + }); // W4: Return new mtime for subsequent requests let newMtime: number | undefined; diff --git a/src/web-server/routes/proxy-routes.ts b/src/web-server/routes/proxy-routes.ts index 2d9ba46d..3b6a34d7 100644 --- a/src/web-server/routes/proxy-routes.ts +++ b/src/web-server/routes/proxy-routes.ts @@ -17,9 +17,22 @@ import { CliproxyServerConfig, } from '../../config/unified-config-types'; import { CLIPROXY_PROVIDER_IDS } from '../../cliproxy/provider-capabilities'; +import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; const router = Router(); +router.use((req: Request, res: Response, next) => { + if ( + requireLocalAccessWhenAuthDisabled( + req, + res, + 'CLIProxy server endpoints require localhost access when dashboard auth is disabled.' + ) + ) { + next(); + } +}); + /** * GET /api/cliproxy-server - Get proxy configuration */ diff --git a/src/web-server/routes/settings-routes.ts b/src/web-server/routes/settings-routes.ts index 9f2eec1d..6d60fd32 100644 --- a/src/web-server/routes/settings-routes.ts +++ b/src/web-server/routes/settings-routes.ts @@ -6,6 +6,7 @@ import { Router, Request, Response } from 'express'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +import * as lockfile from 'proper-lockfile'; import { getCcsDir, loadSettings } from '../../utils/config-manager'; import { isSensitiveKey, maskSensitiveValue } from '../../utils/sensitive-keys'; import { listVariants } from '../../cliproxy/services/variant-service'; @@ -20,11 +21,8 @@ import { import { regenerateConfig } from '../../cliproxy/config-generator'; import { deduplicateCcsHooks } from '../../utils/websearch/hook-utils'; import { resolveCliproxyBridgeMetadata } from '../../api/services'; -import { - getDashboardAuthConfig, - loadOrCreateUnifiedConfig, - mutateUnifiedConfig, -} from '../../config/unified-config-loader'; +import { loadOrCreateUnifiedConfig, mutateUnifiedConfig } from '../../config/unified-config-loader'; +import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; import type { Settings } from '../../types/config'; import type { CLIProxyProvider } from '../../cliproxy/types'; import { mapExternalProviderName } from '../../cliproxy/provider-capabilities'; @@ -43,40 +41,25 @@ const MODEL_ENV_KEYS = [ 'ANTHROPIC_DEFAULT_HAIKU_MODEL', ] as const; const PRESET_MODEL_KEYS = ['default', 'opus', 'sonnet', 'haiku'] as const; +const SETTINGS_IDENTIFIER_PATTERN = /^[a-zA-Z][a-zA-Z0-9._-]*$/; const { logRouteError, respondInternalError } = createRouteErrorHelpers('settings-routes'); -function isLoopbackAddress(value: string | undefined): boolean { - if (!value) return false; - const normalized = value.trim().replace(/^\[|\]$/g, ''); - return ( - normalized === '::1' || - normalized === '127.0.0.1' || - normalized.startsWith('127.') || - normalized === '::ffff:127.0.0.1' || - normalized.startsWith('::ffff:127.') - ); +function resolvePathWithin(basePath: string, targetPath: string): string { + const resolvedBase = path.resolve(basePath); + const resolvedTarget = path.resolve(targetPath); + if (resolvedTarget !== resolvedBase && !resolvedTarget.startsWith(`${resolvedBase}${path.sep}`)) { + throw new Error('Invalid settings path'); + } + return resolvedTarget; } function requireSensitiveLocalAccess(req: Request, res: Response): boolean { - const dashboardAuth = getDashboardAuthConfig(); - if (dashboardAuth.enabled) { - return true; - } - - // Use only socket-level address for security decisions. - // X-Forwarded-For is trivially spoofable and must NOT be trusted - // without an explicit trust proxy configuration. - const candidateAddress = req.socket.remoteAddress; - - if (isLoopbackAddress(candidateAddress)) { - return true; - } - - res.status(403).json({ - error: 'Sensitive settings endpoints require localhost access when dashboard auth is disabled.', - }); - return false; + return requireLocalAccessWhenAuthDisabled( + req, + res, + 'Sensitive settings endpoints require localhost access when dashboard auth is disabled.' + ); } function classifyConfigSaveFailure(error: unknown): { statusCode: number; message: string } { @@ -100,18 +83,26 @@ function classifyConfigSaveFailure(error: unknown): { statusCode: number; messag * Variants have settings paths in config, regular profiles use {name}.settings.json */ function resolveSettingsPath(profileOrVariant: string): string { + if (!SETTINGS_IDENTIFIER_PATTERN.test(profileOrVariant)) { + throw new Error('Invalid profile name'); + } + const ccsDir = getCcsDir(); + const resolvedCcsDir = path.resolve(ccsDir); // Check if this is a variant const variants = listVariants(); const variant = variants[profileOrVariant]; if (variant?.settings) { // Variant settings path (e.g., ~/.ccs/agy-g3.settings.json) - return variant.settings.replace(/^~/, os.homedir()); + return resolvePathWithin(resolvedCcsDir, variant.settings.replace(/^~/, os.homedir())); } // Regular profile settings - return path.join(ccsDir, `${profileOrVariant}.settings.json`); + return resolvePathWithin( + resolvedCcsDir, + path.join(resolvedCcsDir, `${profileOrVariant}.settings.json`) + ); } function resolveProviderForProfile(profileOrVariant: string): CLIProxyProvider | null { @@ -261,11 +252,29 @@ function canonicalizeProfileSettings(profileOrVariant: string, settings: Setting } function writeSettingsAtomically(settingsPath: string, settings: Settings): void { - const tempPath = settingsPath + '.tmp'; + const tempPath = `${settingsPath}.tmp.${process.pid}`; fs.writeFileSync(tempPath, JSON.stringify(settings, null, 2) + '\n'); fs.renameSync(tempPath, settingsPath); } +function withSettingsFileLock(settingsPath: string, callback: () => T): T { + const lockTarget = fs.existsSync(settingsPath) ? settingsPath : path.dirname(settingsPath); + let release: (() => void) | undefined; + + try { + release = lockfile.lockSync(lockTarget, { stale: 10000 }) as () => void; + return callback(); + } finally { + if (release) { + try { + release(); + } catch { + // Best-effort release + } + } + } +} + function loadCanonicalProfileSettings( profileOrVariant: string, settingsPath: string, @@ -339,6 +348,8 @@ router.get('/:profile', (req: Request, res: Response): void => { * GET /api/settings/:profile/raw - Get full settings (for editing) */ router.get('/:profile/raw', (req: Request, res: Response): void => { + if (!requireSensitiveLocalAccess(req, res)) return; + try { const { profile } = req.params; const settingsPath = resolveSettingsPath(profile); @@ -379,6 +390,8 @@ function checkRequiredEnvVars(settings: Settings): string[] { * PUT /api/settings/:profile - Update settings with conflict detection and backup */ router.put('/:profile', (req: Request, res: Response): void => { + if (!requireSensitiveLocalAccess(req, res)) return; + try { const { profile } = req.params; const { settings, expectedMtime } = req.body; @@ -407,53 +420,56 @@ router.put('/:profile', (req: Request, res: Response): void => { const missingFields = checkRequiredEnvVars(normalizedSettings); const settingsPath = resolveSettingsPath(profile); - const fileExists = fs.existsSync(settingsPath); - - // Only check conflict if file exists and expectedMtime was provided - if (fileExists && expectedMtime) { - const stat = fs.statSync(settingsPath); - if (stat.mtime.getTime() !== expectedMtime) { - res.status(409).json({ - error: 'File modified externally', - currentMtime: stat.mtime.getTime(), - }); - return; - } - } - - // Create backup only if file exists AND content actually changed let backupPath: string | undefined; - const newContent = JSON.stringify(normalizedSettings, null, 2) + '\n'; - if (fileExists) { - const existingContent = fs.readFileSync(settingsPath, 'utf8'); - // Only create backup if content differs - if (existingContent !== newContent) { - const backupDir = path.join(ccsDir, 'backups'); - if (!fs.existsSync(backupDir)) { - fs.mkdirSync(backupDir, { recursive: true }); + let created = false; + let newMtime = 0; + + withSettingsFileLock(settingsPath, () => { + const fileExists = fs.existsSync(settingsPath); + + if (fileExists && expectedMtime) { + const stat = fs.statSync(settingsPath); + if (stat.mtime.getTime() !== expectedMtime) { + res.status(409).json({ + error: 'File modified externally', + currentMtime: stat.mtime.getTime(), + }); + return; } - const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); - backupPath = path.join(backupDir, `${profile}.${timestamp}.settings.json`); - fs.copyFileSync(settingsPath, backupPath); } + + const newContent = JSON.stringify(normalizedSettings, null, 2) + '\n'; + if (fileExists) { + const existingContent = fs.readFileSync(settingsPath, 'utf8'); + if (existingContent !== newContent) { + const backupDir = path.join(ccsDir, 'backups'); + if (!fs.existsSync(backupDir)) { + fs.mkdirSync(backupDir, { recursive: true }); + } + const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); + backupPath = path.join(backupDir, `${profile}.${timestamp}.settings.json`); + fs.copyFileSync(settingsPath, backupPath); + } + } else { + created = true; + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + } + + const tempPath = `${settingsPath}.tmp.${process.pid}`; + fs.writeFileSync(tempPath, newContent); + fs.renameSync(tempPath, settingsPath); + newMtime = fs.statSync(settingsPath).mtime.getTime(); + }); + + if (res.headersSent) { + return; } - // Ensure directory exists for new files - if (!fileExists) { - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - } - - // Write new settings atomically - const tempPath = settingsPath + '.tmp'; - fs.writeFileSync(tempPath, newContent); - fs.renameSync(tempPath, settingsPath); - - const newStat = fs.statSync(settingsPath); res.json({ profile, - mtime: newStat.mtime.getTime(), + mtime: newMtime, backupPath, - created: !fileExists, + created, // Include warning if fields missing (runtime will use defaults) ...(missingFields.length > 0 && { warning: `Missing fields will use defaults: ${missingFields.join(', ')}`, @@ -491,6 +507,8 @@ router.get('/:profile/presets', (req: Request, res: Response): void => { * POST /api/settings/:profile/presets - Create a new preset */ router.post('/:profile/presets', (req: Request, res: Response): void => { + if (!requireSensitiveLocalAccess(req, res)) return; + try { const { profile } = req.params; const { name, default: defaultModel, opus, sonnet, haiku } = req.body; @@ -502,56 +520,65 @@ router.post('/:profile/presets', (req: Request, res: Response): void => { const settingsPath = resolveSettingsPath(profile); - // Create settings file if it doesn't exist - if (!fs.existsSync(settingsPath)) { - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - fs.writeFileSync(settingsPath, JSON.stringify({ env: {}, presets: [] }, null, 2) + '\n'); - } + let persistedPreset: + | { + name: string; + default: string; + opus: string; + sonnet: string; + haiku: string; + } + | undefined; - const settings = loadCanonicalProfileSettings(profile, settingsPath, false); - settings.presets = settings.presets || []; + withSettingsFileLock(settingsPath, () => { + if (!fs.existsSync(settingsPath)) { + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync(settingsPath, JSON.stringify({ env: {}, presets: [] }, null, 2) + '\n'); + } - // Check for duplicate name - if (settings.presets.some((p) => p.name === name)) { - res.status(409).json({ error: 'Preset with this name already exists' }); + const settings = loadCanonicalProfileSettings(profile, settingsPath, false); + settings.presets = settings.presets || []; + + if (settings.presets.some((p) => p.name === name)) { + res.status(409).json({ error: 'Preset with this name already exists' }); + return; + } + + const normalizePresetModel = (modelId: string): string => + canonicalizeProfileModelId(profile, modelId, settings); + + for (const modelId of [ + defaultModel, + opus || defaultModel, + sonnet || defaultModel, + haiku || defaultModel, + ]) { + const deniedReason = findDeniedProfileModel(profile, modelId, settings); + if (deniedReason) { + res.status(400).json({ error: deniedReason }); + return; + } + } + + const preset = { + name, + default: normalizePresetModel(defaultModel), + opus: normalizePresetModel(opus || defaultModel), + sonnet: normalizePresetModel(sonnet || defaultModel), + haiku: normalizePresetModel(haiku || defaultModel), + }; + + settings.presets.push(preset); + const canonicalizedSettings = canonicalizeProfileSettings(profile, settings); + writeSettingsAtomically(settingsPath, canonicalizedSettings); + persistedPreset = + canonicalizedSettings.presets?.find((entry) => entry.name === name) || preset; + }); + + if (res.headersSent) { return; } - const normalizePresetModel = (modelId: string): string => - canonicalizeProfileModelId(profile, modelId, settings); - - for (const modelId of [ - defaultModel, - opus || defaultModel, - sonnet || defaultModel, - haiku || defaultModel, - ]) { - const deniedReason = findDeniedProfileModel(profile, modelId, settings); - if (deniedReason) { - res.status(400).json({ error: deniedReason }); - return; - } - } - - const normalizedDefaultModel = normalizePresetModel(defaultModel); - const normalizedOpusModel = normalizePresetModel(opus || defaultModel); - const normalizedSonnetModel = normalizePresetModel(sonnet || defaultModel); - const normalizedHaikuModel = normalizePresetModel(haiku || defaultModel); - - const preset = { - name, - default: normalizedDefaultModel, - opus: normalizedOpusModel, - sonnet: normalizedSonnetModel, - haiku: normalizedHaikuModel, - }; - - settings.presets.push(preset); - const canonicalizedSettings = canonicalizeProfileSettings(profile, settings); - writeSettingsAtomically(settingsPath, canonicalizedSettings); - - const persistedPreset = - canonicalizedSettings.presets?.find((entry) => entry.name === name) || preset; res.status(201).json({ preset: persistedPreset }); } catch (error) { respondInternalError(res, error, 'Internal server error.'); @@ -562,25 +589,33 @@ router.post('/:profile/presets', (req: Request, res: Response): void => { * DELETE /api/settings/:profile/presets/:name - Delete a preset */ router.delete('/:profile/presets/:name', (req: Request, res: Response): void => { + if (!requireSensitiveLocalAccess(req, res)) return; + try { const { profile, name } = req.params; const settingsPath = resolveSettingsPath(profile); - if (!fs.existsSync(settingsPath)) { - res.status(404).json({ error: 'Settings not found' }); + withSettingsFileLock(settingsPath, () => { + if (!fs.existsSync(settingsPath)) { + res.status(404).json({ error: 'Settings not found' }); + return; + } + + const settings = loadCanonicalProfileSettings(profile, settingsPath, false); + if (!settings.presets || !settings.presets.some((p) => p.name === name)) { + res.status(404).json({ error: 'Preset not found' }); + return; + } + + settings.presets = settings.presets.filter((p) => p.name !== name); + const canonicalizedSettings = canonicalizeProfileSettings(profile, settings); + writeSettingsAtomically(settingsPath, canonicalizedSettings); + }); + + if (res.headersSent) { return; } - const settings = loadCanonicalProfileSettings(profile, settingsPath, false); - if (!settings.presets || !settings.presets.some((p) => p.name === name)) { - res.status(404).json({ error: 'Preset not found' }); - return; - } - - settings.presets = settings.presets.filter((p) => p.name !== name); - const canonicalizedSettings = canonicalizeProfileSettings(profile, settings); - writeSettingsAtomically(settingsPath, canonicalizedSettings); - res.json({ success: true }); } catch (error) { respondInternalError(res, error, 'Internal server error.'); @@ -642,6 +677,8 @@ router.put('/auth/antigravity-risk', (req: Request, res: Response): void => { * GET /api/settings/auth/tokens - Get current auth token status (masked) */ router.get('/auth/tokens', (_req: Request, res: Response): void => { + if (!requireSensitiveLocalAccess(_req, res)) return; + try { const summary = getAuthSummary(); @@ -692,6 +729,8 @@ router.get('/auth/tokens/raw', (req: Request, res: Response): void => { * PUT /api/settings/auth/tokens - Update auth tokens */ router.put('/auth/tokens', (req: Request, res: Response): void => { + if (!requireSensitiveLocalAccess(req, res)) return; + try { const { apiKey, managementSecret } = req.body; @@ -728,6 +767,8 @@ router.put('/auth/tokens', (req: Request, res: Response): void => { * POST /api/settings/auth/tokens/regenerate-secret - Generate new management secret */ router.post('/auth/tokens/regenerate-secret', (_req: Request, res: Response): void => { + if (!requireSensitiveLocalAccess(_req, res)) return; + try { const newSecret = generateSecureToken(32); setGlobalManagementSecret(newSecret); @@ -752,6 +793,8 @@ router.post('/auth/tokens/regenerate-secret', (_req: Request, res: Response): vo * POST /api/settings/auth/tokens/reset - Reset auth tokens to defaults */ router.post('/auth/tokens/reset', (_req: Request, res: Response): void => { + if (!requireSensitiveLocalAccess(_req, res)) return; + try { resetAuthToDefaults(); diff --git a/src/web-server/routes/websearch-routes.ts b/src/web-server/routes/websearch-routes.ts index 564ca508..ae8e9054 100644 --- a/src/web-server/routes/websearch-routes.ts +++ b/src/web-server/routes/websearch-routes.ts @@ -3,11 +3,7 @@ */ import { Router, Request, Response } from 'express'; -import { - loadUnifiedConfig, - saveUnifiedConfig, - getWebSearchConfig, -} from '../../config/unified-config-loader'; +import { mutateUnifiedConfig, getWebSearchConfig } from '../../config/unified-config-loader'; import type { WebSearchConfig } from '../../config/unified-config-types'; import { getWebSearchReadiness, @@ -52,59 +48,45 @@ router.put('/', (req: Request, res: Response): void => { } try { - // Load existing config and update websearch section - const existingConfig = loadUnifiedConfig(); - if (!existingConfig) { - res.status(500).json({ error: 'Failed to load config' }); - return; - } - - // Merge updates - supports Gemini CLI and Grok CLI - existingConfig.websearch = { - enabled: enabled ?? existingConfig.websearch?.enabled ?? true, - providers: providers - ? { - gemini: { - enabled: - providers.gemini?.enabled ?? - existingConfig.websearch?.providers?.gemini?.enabled ?? - true, - model: - providers.gemini?.model ?? - existingConfig.websearch?.providers?.gemini?.model ?? - 'gemini-2.5-flash', - timeout: - providers.gemini?.timeout ?? - existingConfig.websearch?.providers?.gemini?.timeout ?? - 55, - }, - grok: { - enabled: - providers.grok?.enabled ?? - existingConfig.websearch?.providers?.grok?.enabled ?? - false, - timeout: - providers.grok?.timeout ?? existingConfig.websearch?.providers?.grok?.timeout ?? 55, - }, - opencode: { - enabled: - providers.opencode?.enabled ?? - existingConfig.websearch?.providers?.opencode?.enabled ?? - false, - model: - providers.opencode?.model ?? - existingConfig.websearch?.providers?.opencode?.model ?? - 'opencode/grok-code', - timeout: - providers.opencode?.timeout ?? - existingConfig.websearch?.providers?.opencode?.timeout ?? - 60, - }, - } - : existingConfig.websearch?.providers, - }; - - saveUnifiedConfig(existingConfig); + const existingConfig = mutateUnifiedConfig((config) => { + config.websearch = { + enabled: enabled ?? config.websearch?.enabled ?? true, + providers: providers + ? { + gemini: { + enabled: + providers.gemini?.enabled ?? config.websearch?.providers?.gemini?.enabled ?? true, + model: + providers.gemini?.model ?? + config.websearch?.providers?.gemini?.model ?? + 'gemini-2.5-flash', + timeout: + providers.gemini?.timeout ?? config.websearch?.providers?.gemini?.timeout ?? 55, + }, + grok: { + enabled: + providers.grok?.enabled ?? config.websearch?.providers?.grok?.enabled ?? false, + timeout: + providers.grok?.timeout ?? config.websearch?.providers?.grok?.timeout ?? 55, + }, + opencode: { + enabled: + providers.opencode?.enabled ?? + config.websearch?.providers?.opencode?.enabled ?? + false, + model: + providers.opencode?.model ?? + config.websearch?.providers?.opencode?.model ?? + 'opencode/grok-code', + timeout: + providers.opencode?.timeout ?? + config.websearch?.providers?.opencode?.timeout ?? + 60, + }, + } + : config.websearch?.providers, + }; + }); res.json({ success: true, diff --git a/src/web-server/shared-routes.ts b/src/web-server/shared-routes.ts index af876196..ade2e0c3 100644 --- a/src/web-server/shared-routes.ts +++ b/src/web-server/shared-routes.ts @@ -10,9 +10,22 @@ import * as path from 'path'; import * as yaml from 'js-yaml'; import { getCcsDir } from '../utils/config-manager'; import { getClaudeConfigDir } from '../utils/claude-config-path'; +import { requireLocalAccessWhenAuthDisabled } from './middleware/auth-middleware'; export const sharedRoutes = Router(); +sharedRoutes.use((req: Request, res: Response, next) => { + if ( + requireLocalAccessWhenAuthDisabled( + req, + res, + 'Shared-content endpoints require localhost access when dashboard auth is disabled.' + ) + ) { + next(); + } +}); + const MAX_DIRECTORY_TRAVERSAL_DEPTH = 10; const MAX_DESCRIPTION_LENGTH = 140; const MAX_MARKDOWN_FILE_BYTES = 1024 * 1024; // 1 MiB diff --git a/tests/unit/cliproxy/account-registry-integrity.test.ts b/tests/unit/cliproxy/account-registry-integrity.test.ts new file mode 100644 index 00000000..6d0eb81d --- /dev/null +++ b/tests/unit/cliproxy/account-registry-integrity.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { runWithScopedCcsHome } from '../../../src/utils/config-manager'; + +async function withIsolatedHome(fn: (homeDir: string) => Promise | T): Promise { + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-account-registry-')); + try { + return await runWithScopedCcsHome(homeDir, () => fn(homeDir)); + } finally { + fs.rmSync(homeDir, { recursive: true, force: true }); + } +} + +async function loadRegistryModule() { + return import(`../../../src/cliproxy/accounts/registry?registry-integrity=${Date.now()}`); +} + +async function loadAccountManager() { + return import(`../../../src/cliproxy/account-manager?account-registry-integrity=${Date.now()}`); +} + +describe('account registry integrity', () => { + it('does not create accounts.json during no-op discovery', async () => { + await withIsolatedHome(async (homeDir) => { + const authDir = path.join(homeDir, '.ccs', 'cliproxy', 'auth'); + const registryPath = path.join(homeDir, '.ccs', 'cliproxy', 'accounts.json'); + fs.mkdirSync(authDir, { recursive: true }); + + const { discoverExistingAccounts } = await loadRegistryModule(); + discoverExistingAccounts(); + + expect(fs.existsSync(registryPath)).toBe(false); + }); + }); + + it('does not write accounts.json during provider account reads', async () => { + await withIsolatedHome(async (homeDir) => { + const authDir = path.join(homeDir, '.ccs', 'cliproxy', 'auth'); + const registryPath = path.join(homeDir, '.ccs', 'cliproxy', 'accounts.json'); + fs.mkdirSync(authDir, { recursive: true }); + + const { getProviderAccounts } = await loadAccountManager(); + expect(getProviderAccounts('kiro')).toEqual([]); + expect(fs.existsSync(registryPath)).toBe(false); + }); + }); + + it('removes stale accounts before choosing the next default during registration', async () => { + await withIsolatedHome(async (homeDir) => { + const cliproxyDir = path.join(homeDir, '.ccs', 'cliproxy'); + const authDir = path.join(cliproxyDir, 'auth'); + const registryPath = path.join(cliproxyDir, 'accounts.json'); + fs.mkdirSync(authDir, { recursive: true }); + fs.writeFileSync(path.join(authDir, 'kiro-github-ABC123.json'), JSON.stringify({ type: 'kiro' })); + fs.writeFileSync( + registryPath, + JSON.stringify({ + version: 1, + providers: { + kiro: { + default: 'github-OLD999', + accounts: { + 'github-OLD999': { + nickname: 'old', + tokenFile: 'kiro-github-OLD999.json', + createdAt: '2025-01-01T00:00:00.000Z', + lastUsedAt: '2025-01-01T00:00:00.000Z', + }, + }, + }, + }, + }), + 'utf8' + ); + + const { registerAccount } = await loadAccountManager(); + const account = registerAccount('kiro', 'kiro-github-ABC123.json'); + + const { loadAccountsRegistry } = await loadRegistryModule(); + const registry = loadAccountsRegistry(); + const kiroAccounts = registry.providers.kiro; + + expect(account.id).toBe('github-ABC123'); + expect(kiroAccounts?.default).toBe('github-ABC123'); + expect(kiroAccounts?.accounts['github-OLD999']).toBeUndefined(); + expect(Object.keys(kiroAccounts?.accounts ?? {})).toEqual(['github-ABC123']); + }); + }); + + it('fails closed on corrupted accounts.json', async () => { + await withIsolatedHome(async (homeDir) => { + const registryPath = path.join(homeDir, '.ccs', 'cliproxy', 'accounts.json'); + fs.mkdirSync(path.dirname(registryPath), { recursive: true }); + fs.writeFileSync(registryPath, '{not-valid-json', 'utf8'); + + const { loadAccountsRegistry } = await loadRegistryModule(); + expect(() => loadAccountsRegistry()).toThrow(/corrupted/i); + }); + }); +}); diff --git a/tests/unit/cliproxy/account-registry-optional-nickname.test.ts b/tests/unit/cliproxy/account-registry-optional-nickname.test.ts index 04ef857a..5fb576e9 100644 --- a/tests/unit/cliproxy/account-registry-optional-nickname.test.ts +++ b/tests/unit/cliproxy/account-registry-optional-nickname.test.ts @@ -8,10 +8,16 @@ async function loadAccountManager() { return import(`../../../src/cliproxy/account-manager?optional-nickname=${Date.now()}`); } -async function withIsolatedHome(fn: () => Promise | T): Promise { +function writeTokenFile(homeDir: string, tokenFile: string): void { + const authDir = path.join(homeDir, '.ccs', 'cliproxy', 'auth'); + fs.mkdirSync(authDir, { recursive: true }); + fs.writeFileSync(path.join(authDir, tokenFile), '{}', 'utf8'); +} + +async function withIsolatedHome(fn: (homeDir: string) => Promise | T): Promise { const testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-optional-nickname-')); try { - return await runWithScopedCcsHome(testDir, fn); + return await runWithScopedCcsHome(testDir, () => fn(testDir)); } finally { fs.rmSync(testDir, { recursive: true, force: true }); } @@ -19,7 +25,8 @@ async function withIsolatedHome(fn: () => Promise | T): Promise { describe('registerAccount optional nickname flow', () => { it('uses a filename-derived id when Kiro/GHCP nickname is omitted', async () => { - const account = await withIsolatedHome(async () => { + const account = await withIsolatedHome(async (homeDir) => { + writeTokenFile(homeDir, 'kiro-github-ABC123.json'); const { registerAccount } = await loadAccountManager(); return registerAccount('kiro', 'kiro-github-ABC123.json'); }); @@ -29,7 +36,9 @@ describe('registerAccount optional nickname flow', () => { }); it('falls back to provider-scoped sequential ids when the filename is not descriptive', async () => { - const { first, second } = await withIsolatedHome(async () => { + const { first, second } = await withIsolatedHome(async (homeDir) => { + writeTokenFile(homeDir, 'kiro-nomail.json'); + writeTokenFile(homeDir, 'kiro-second.json'); const { registerAccount } = await loadAccountManager(); return { first: registerAccount('kiro', 'kiro-nomail.json'), @@ -44,7 +53,8 @@ describe('registerAccount optional nickname flow', () => { }); it('keeps user nicknames optional metadata separate from internal ids', async () => { - const account = await withIsolatedHome(async () => { + const account = await withIsolatedHome(async (homeDir) => { + writeTokenFile(homeDir, 'ghcp-amazon-XYZ789.json'); const { registerAccount } = await loadAccountManager(); return registerAccount('ghcp', 'ghcp-amazon-XYZ789.json', undefined, 'work'); }); @@ -54,7 +64,8 @@ describe('registerAccount optional nickname flow', () => { }); it('preserves an existing custom nickname when the same token file is re-registered', async () => { - const reauthenticated = await withIsolatedHome(async () => { + const reauthenticated = await withIsolatedHome(async (homeDir) => { + writeTokenFile(homeDir, 'kiro-github-ABC123.json'); const { registerAccount } = await loadAccountManager(); registerAccount('kiro', 'kiro-github-ABC123.json', undefined, 'work'); return registerAccount('kiro', 'kiro-github-ABC123.json'); @@ -65,7 +76,10 @@ describe('registerAccount optional nickname flow', () => { }); it('rejects nickname collisions against existing account ids and nicknames', async () => { - await withIsolatedHome(async () => { + await withIsolatedHome(async (homeDir) => { + writeTokenFile(homeDir, 'kiro-github-ABC123.json'); + writeTokenFile(homeDir, 'kiro-google-XYZ789.json'); + writeTokenFile(homeDir, 'kiro-google-NEW123.json'); const { registerAccount, renameAccount } = await loadAccountManager(); registerAccount('kiro', 'kiro-github-ABC123.json'); const second = registerAccount('kiro', 'kiro-google-XYZ789.json', undefined, 'personal'); @@ -78,7 +92,9 @@ describe('registerAccount optional nickname flow', () => { }); it('avoids auto-generated ids that would collide with an existing nickname', async () => { - const added = await withIsolatedHome(async () => { + const added = await withIsolatedHome(async (homeDir) => { + writeTokenFile(homeDir, 'kiro-github-ABC123.json'); + writeTokenFile(homeDir, 'kiro-google-XYZ789.json'); const { registerAccount } = await loadAccountManager(); registerAccount('kiro', 'kiro-github-ABC123.json', undefined, 'google-XYZ789'); return registerAccount('kiro', 'kiro-google-XYZ789.json'); @@ -89,7 +105,9 @@ describe('registerAccount optional nickname flow', () => { }); it('does not resolve ambiguous nickname prefixes to the first generated account', async () => { - const match = await withIsolatedHome(async () => { + const match = await withIsolatedHome(async (homeDir) => { + writeTokenFile(homeDir, 'kiro-github-ABC123.json'); + writeTokenFile(homeDir, 'kiro-github-DEF456.json'); const { registerAccount, findAccountByQuery } = await loadAccountManager(); registerAccount('kiro', 'kiro-github-ABC123.json'); registerAccount('kiro', 'kiro-github-DEF456.json'); diff --git a/tests/unit/cliproxy/ai-provider-service-stable-id.test.ts b/tests/unit/cliproxy/ai-provider-service-stable-id.test.ts new file mode 100644 index 00000000..a4de2cf6 --- /dev/null +++ b/tests/unit/cliproxy/ai-provider-service-stable-id.test.ts @@ -0,0 +1,147 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import * as yaml from 'js-yaml'; + +function getCliproxyConfigPath(homeDir: string): string { + return path.join(homeDir, '.ccs', 'cliproxy', 'config.yaml'); +} + +function writeCliproxyConfig(homeDir: string, value: Record): void { + const configPath = getCliproxyConfigPath(homeDir); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, yaml.dump(value), 'utf8'); +} + +function readCliproxyConfig(homeDir: string): Record { + return ( + (yaml.load(fs.readFileSync(getCliproxyConfigPath(homeDir), 'utf8')) as Record) || {} + ); +} + +async function loadAiProviderService() { + return import(`../../../src/cliproxy/ai-providers/service?stable-id=${Date.now()}`); +} + +describe('ai-provider service stable ids', () => { + let tempHome = ''; + let originalCcsHome: string | undefined; + + beforeEach(() => { + tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-ai-provider-ids-')); + originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tempHome; + }); + + afterEach(() => { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + + if (tempHome && fs.existsSync(tempHome)) { + fs.rmSync(tempHome, { recursive: true, force: true }); + } + }); + + it('backfills and persists stable ids for api-key provider entries', async () => { + const { listAiProviders } = await loadAiProviderService(); + + writeCliproxyConfig(tempHome, { + 'gemini-api-key': [{ 'api-key': 'alpha' }, { 'api-key': 'beta' }], + }); + + const listed = await listAiProviders(); + const family = listed.families.find((entry) => entry.id === 'gemini-api-key'); + + expect(family).toBeDefined(); + expect(family?.entries).toHaveLength(2); + expect(family?.entries[0]?.id).toBeTruthy(); + expect(family?.entries[1]?.id).toBeTruthy(); + expect(family?.entries[0]?.id).not.toBe(family?.entries[1]?.id); + + const persisted = readCliproxyConfig(tempHome)['gemini-api-key'] as Array>; + expect(persisted[0]?.id).toBe(family?.entries[0]?.id); + expect(persisted[1]?.id).toBe(family?.entries[1]?.id); + }); + + it('updates and deletes api-key entries by stable id while preserving the id', async () => { + const { updateAiProviderEntry, deleteAiProviderEntry } = await loadAiProviderService(); + + writeCliproxyConfig(tempHome, { + 'gemini-api-key': [ + { 'api-key': 'alpha', id: 'gemini-a' }, + { 'api-key': 'beta', id: 'gemini-b' }, + ], + }); + + await updateAiProviderEntry('gemini-api-key', 'gemini-a', { + apiKey: 'gamma', + baseUrl: 'https://example.test/gemini', + }); + + let persisted = readCliproxyConfig(tempHome)['gemini-api-key'] as Array>; + expect(persisted[0]?.id).toBe('gemini-a'); + expect(persisted[0]?.['api-key']).toBe('gamma'); + expect(persisted[0]?.['base-url']).toBe('https://example.test/gemini'); + + await deleteAiProviderEntry('gemini-api-key', 'gemini-a'); + + persisted = readCliproxyConfig(tempHome)['gemini-api-key'] as Array>; + expect(persisted).toHaveLength(1); + expect(persisted[0]?.id).toBe('gemini-b'); + }); + + it('keeps legacy numeric index updates working during the route transition', async () => { + const { updateAiProviderEntry } = await loadAiProviderService(); + + writeCliproxyConfig(tempHome, { + 'gemini-api-key': [{ 'api-key': 'alpha', id: 'gemini-a' }], + }); + + await updateAiProviderEntry('gemini-api-key', '0', { + apiKey: 'legacy-index-update', + }); + + const persisted = readCliproxyConfig(tempHome)['gemini-api-key'] as Array>; + expect(persisted[0]?.id).toBe('gemini-a'); + expect(persisted[0]?.['api-key']).toBe('legacy-index-update'); + }); + + it('backfills and preserves stable ids for openai-compatible connectors', async () => { + const { listAiProviders, updateAiProviderEntry } = await loadAiProviderService(); + + writeCliproxyConfig(tempHome, { + 'openai-compatibility': [ + { + name: 'openrouter', + 'base-url': 'https://openrouter.ai/api/v1', + 'api-key-entries': [{ 'api-key': 'sk-openrouter' }], + }, + ], + }); + + const listed = await listAiProviders(); + const family = listed.families.find((entry) => entry.id === 'openai-compatibility'); + const connectorId = family?.entries[0]?.id; + + expect(connectorId).toBeTruthy(); + + await updateAiProviderEntry('openai-compatibility', connectorId!, { + name: 'openrouter', + baseUrl: 'https://router.example/v1', + preserveSecrets: true, + }); + + const persisted = readCliproxyConfig(tempHome)['openai-compatibility'] as Array< + Record + >; + expect(persisted[0]?.id).toBe(connectorId); + expect(persisted[0]?.['base-url']).toBe('https://router.example/v1'); + expect((persisted[0]?.['api-key-entries'] as Array>)[0]?.['api-key']).toBe( + 'sk-openrouter' + ); + }); +}); diff --git a/tests/unit/commands/proxy-lifecycle-subcommand.test.ts b/tests/unit/commands/proxy-lifecycle-subcommand.test.ts index 0cc8835b..32da192c 100644 --- a/tests/unit/commands/proxy-lifecycle-subcommand.test.ts +++ b/tests/unit/commands/proxy-lifecycle-subcommand.test.ts @@ -1,64 +1,62 @@ -import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; -import * as fs from 'fs'; -import * as os from 'os'; -import * as path from 'path'; -import { resolveLifecyclePort } from '../../../src/commands/cliproxy/proxy-lifecycle-subcommand'; +import { afterEach, describe, expect, it, mock } from 'bun:test'; import { CLIPROXY_DEFAULT_PORT } from '../../../src/cliproxy/config/port-manager'; -import { runWithScopedConfigDir } from '../../../src/utils/config-manager'; -let tempDir: string; +type MockUnifiedConfig = { + cliproxy_server?: { + local?: { + port?: number; + }; + }; +}; -function writeUnifiedConfig(localPort: number): void { - const configPath = path.join(tempDir, 'config.yaml'); - const yaml = `version: 2 -accounts: {} -profiles: {} -preferences: - theme: system - telemetry: false - auto_update: true -cliproxy: - oauth_accounts: {} - providers: - - gemini - - codex - - agy - variants: {} -cliproxy_server: - local: - port: ${localPort} -`; - fs.writeFileSync(configPath, yaml, 'utf8'); +function mockUnifiedConfig(config: MockUnifiedConfig): void { + mock.module('../../../src/config/unified-config-loader', () => ({ + loadOrCreateUnifiedConfig: () => config, + })); +} + +async function loadResolveLifecyclePort() { + const mod = await import( + `../../../src/commands/cliproxy/resolve-lifecycle-port?proxy-lifecycle-port=${Date.now()}-${Math.random()}` + ); + return mod.resolveLifecyclePort; } describe('resolveLifecyclePort', () => { - beforeEach(() => { - tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-proxy-lifecycle-')); - }); - afterEach(() => { - if (tempDir && fs.existsSync(tempDir)) { - fs.rmSync(tempDir, { recursive: true, force: true }); - } + mock.restore(); }); it('uses configured cliproxy_server.local.port', async () => { - writeUnifiedConfig(9456); - await runWithScopedConfigDir(tempDir, () => { - expect(resolveLifecyclePort()).toBe(9456); + mockUnifiedConfig({ + cliproxy_server: { + local: { + port: 9456, + }, + }, }); + + const resolveLifecyclePort = await loadResolveLifecyclePort(); + expect(resolveLifecyclePort()).toBe(9456); }); it('falls back to default port when configured local port is invalid', async () => { - writeUnifiedConfig(70000); - await runWithScopedConfigDir(tempDir, () => { - expect(resolveLifecyclePort()).toBe(CLIPROXY_DEFAULT_PORT); + mockUnifiedConfig({ + cliproxy_server: { + local: { + port: 70000, + }, + }, }); + + const resolveLifecyclePort = await loadResolveLifecyclePort(); + expect(resolveLifecyclePort()).toBe(CLIPROXY_DEFAULT_PORT); }); it('falls back to default port when config file is missing', async () => { - await runWithScopedConfigDir(tempDir, () => { - expect(resolveLifecyclePort()).toBe(CLIPROXY_DEFAULT_PORT); - }); + mockUnifiedConfig({}); + + const resolveLifecyclePort = await loadResolveLifecyclePort(); + expect(resolveLifecyclePort()).toBe(CLIPROXY_DEFAULT_PORT); }); }); diff --git a/tests/unit/web-server/ai-provider-routes.test.ts b/tests/unit/web-server/ai-provider-routes.test.ts new file mode 100644 index 00000000..8b638f6f --- /dev/null +++ b/tests/unit/web-server/ai-provider-routes.test.ts @@ -0,0 +1,173 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, mock } from 'bun:test'; +import express from 'express'; +import type { Server } from 'http'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +const listCalls: string[] = []; +const updateCalls: Array<{ family: string; entryId: string; data: Record }> = []; + +mock.module('../../../src/cliproxy/ai-providers', () => ({ + AI_PROVIDER_FAMILY_DEFINITIONS: { + 'gemini-api-key': { + id: 'gemini-api-key', + displayName: 'Gemini', + description: 'Mock Gemini family', + authMode: 'hybrid', + supportsNamedEntries: false, + routePath: '/api/provider/gemini', + }, + 'openai-compatibility': { + id: 'openai-compatibility', + displayName: 'OpenAI-Compatible', + description: 'Mock connector family', + authMode: 'connector', + supportsNamedEntries: true, + routePath: '/api/provider/openai-compat', + }, + }, + AI_PROVIDER_FAMILY_IDS: ['gemini-api-key', 'openai-compatibility'], + listAiProviders: async () => { + listCalls.push('list'); + return { + source: { + mode: 'local', + label: 'Local CLIProxy', + target: 'http://127.0.0.1:8317', + managementAuth: 'configured', + }, + families: [], + }; + }, + createAiProviderEntry: async () => {}, + updateAiProviderEntry: async (family: string, entryId: string, data: Record) => { + updateCalls.push({ family, entryId, data }); + }, + deleteAiProviderEntry: async () => {}, +})); + +describe('ai-provider-routes', () => { + let router: typeof import('../../../src/web-server/routes/ai-provider-routes').default; + let server: Server; + let baseUrl = ''; + let forcedRemoteAddress = '127.0.0.1'; + let tempHome = ''; + let originalDashboardAuthEnabled: string | undefined; + let originalCcsHome: string | undefined; + + beforeAll(async () => { + ({ default: router } = await import( + `../../../src/web-server/routes/ai-provider-routes?ai-provider-routes=${Date.now()}` + )); + + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + Object.defineProperty(req.socket, 'remoteAddress', { + value: forcedRemoteAddress, + configurable: true, + }); + next(); + }); + app.use('/api/cliproxy/ai-providers', router); + + await new Promise((resolve, reject) => { + server = app.listen(0, '127.0.0.1'); + server.once('error', reject); + server.once('listening', () => resolve()); + }); + + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Unable to resolve test server port'); + } + baseUrl = `http://127.0.0.1:${address.port}`; + }); + + afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); + + beforeEach(() => { + originalDashboardAuthEnabled = process.env.CCS_DASHBOARD_AUTH_ENABLED; + originalCcsHome = process.env.CCS_HOME; + if (tempHome && fs.existsSync(tempHome)) { + fs.rmSync(tempHome, { recursive: true, force: true }); + } + tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-ai-provider-routes-')); + process.env.CCS_HOME = tempHome; + forcedRemoteAddress = '127.0.0.1'; + process.env.CCS_DASHBOARD_AUTH_ENABLED = 'false'; + listCalls.length = 0; + updateCalls.length = 0; + }); + + afterEach(() => { + if (originalDashboardAuthEnabled !== undefined) { + process.env.CCS_DASHBOARD_AUTH_ENABLED = originalDashboardAuthEnabled; + } else { + delete process.env.CCS_DASHBOARD_AUTH_ENABLED; + } + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + if (tempHome && fs.existsSync(tempHome)) { + fs.rmSync(tempHome, { recursive: true, force: true }); + tempHome = ''; + } + }); + + it('blocks remote access when dashboard auth is disabled', async () => { + forcedRemoteAddress = '10.10.0.24'; + + const response = await fetch(`${baseUrl}/api/cliproxy/ai-providers`); + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ + error: 'AI provider endpoints require localhost access when dashboard auth is disabled.', + }); + expect(listCalls).toHaveLength(0); + }); + + it('allows non-local access when dashboard auth is enabled', async () => { + forcedRemoteAddress = '10.10.0.24'; + process.env.CCS_DASHBOARD_AUTH_ENABLED = 'true'; + + const response = await fetch(`${baseUrl}/api/cliproxy/ai-providers`); + expect(response.status).toBe(200); + expect(listCalls).toHaveLength(1); + }); + + it('passes stable entry ids through update routes', async () => { + const response = await fetch( + `${baseUrl}/api/cliproxy/ai-providers/gemini-api-key/entry-alpha-123`, + { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ apiKey: 'sk-test' }), + } + ); + + expect(response.status).toBe(200); + expect(updateCalls).toEqual([ + { + family: 'gemini-api-key', + entryId: 'entry-alpha-123', + data: { + apiKey: 'sk-test', + apiKeys: undefined, + baseUrl: undefined, + excludedModels: undefined, + headers: undefined, + models: undefined, + name: undefined, + prefix: undefined, + preserveSecrets: false, + proxyUrl: undefined, + }, + }, + ]); + }); +}); diff --git a/tests/unit/web-server/cliproxy-auth-routes-manual-callback.test.ts b/tests/unit/web-server/cliproxy-auth-routes-manual-callback.test.ts index eb7ccb03..b61e785b 100644 --- a/tests/unit/web-server/cliproxy-auth-routes-manual-callback.test.ts +++ b/tests/unit/web-server/cliproxy-auth-routes-manual-callback.test.ts @@ -112,9 +112,9 @@ describe('cliproxy-auth-routes manual callback nickname persistence', () => { ]); const startResponse = await postJson('/api/cliproxy/auth/kiro/start-url', { - nickname: 'work', - kiroMethod: 'google', - }); + nickname: 'work', + kiroMethod: 'google', + }); expect(startResponse.status).toBe(200); const tokenDir = path.join(tempHome, '.ccs', 'cliproxy', 'auth'); @@ -126,8 +126,8 @@ describe('cliproxy-auth-routes manual callback nickname persistence', () => { ); const callbackResponse = await postJson('/api/cliproxy/auth/kiro/submit-callback', { - redirectUrl: 'http://localhost/callback?code=abc123&state=state-123', - }); + redirectUrl: 'http://localhost/callback?code=abc123&state=state-123', + }); expect(callbackResponse.status).toBe(200); @@ -142,4 +142,37 @@ describe('cliproxy-auth-routes manual callback nickname persistence', () => { expect(registry.providers.kiro.accounts['github-ABC123']?.nickname).toBe('work'); }); + + it('returns 409 when callback completes upstream but no account can be registered locally', async () => { + mockFetch([ + { + url: /\/v0\/management\/kiro-auth-url\?is_webui=true&method=google$/, + response: { + auth_url: 'https://auth.example.com/authorize?state=state-409', + state: 'state-409', + }, + }, + { + url: /\/v0\/management\/oauth-callback$/, + method: 'POST', + response: { status: 'ok' }, + }, + ]); + + const startResponse = await postJson('/api/cliproxy/auth/kiro/start-url', { + nickname: 'work', + kiroMethod: 'google', + }); + expect(startResponse.status).toBe(200); + + const callbackResponse = await postJson('/api/cliproxy/auth/kiro/submit-callback', { + redirectUrl: 'http://localhost/callback?code=abc123&state=state-409', + }); + + expect(callbackResponse.status).toBe(409); + expect(callbackResponse.body).toEqual({ + error: + 'Authenticated token could not be matched to a new account. Retry the flow and choose a different nickname if needed.', + }); + }); }); diff --git a/ui/src/hooks/use-cliproxy-ai-providers.ts b/ui/src/hooks/use-cliproxy-ai-providers.ts index e95598b5..8493493c 100644 --- a/ui/src/hooks/use-cliproxy-ai-providers.ts +++ b/ui/src/hooks/use-cliproxy-ai-providers.ts @@ -42,13 +42,13 @@ export function useUpdateCliproxyAiProviderEntry() { return useMutation({ mutationFn: ({ family, - index, + entryId, data, }: { family: AiProviderFamilyId; - index: number; + entryId: string; data: UpsertAiProviderEntryInput; - }) => api.cliproxy.aiProviders.update(family, index, data), + }) => api.cliproxy.aiProviders.update(family, entryId, data), onSuccess: () => { queryClient.invalidateQueries({ queryKey: QUERY_KEY }); toast.success('Provider entry updated'); @@ -63,8 +63,8 @@ export function useDeleteCliproxyAiProviderEntry() { const queryClient = useQueryClient(); return useMutation({ - mutationFn: ({ family, index }: { family: AiProviderFamilyId; index: number }) => - api.cliproxy.aiProviders.delete(family, index), + mutationFn: ({ family, entryId }: { family: AiProviderFamilyId; entryId: string }) => + api.cliproxy.aiProviders.delete(family, entryId), onSuccess: () => { queryClient.invalidateQueries({ queryKey: QUERY_KEY }); toast.success('Provider entry removed'); diff --git a/ui/src/hooks/use-cliproxy-auth-flow.ts b/ui/src/hooks/use-cliproxy-auth-flow.ts index c31c42e8..a2a9b3bd 100644 --- a/ui/src/hooks/use-cliproxy-auth-flow.ts +++ b/ui/src/hooks/use-cliproxy-auth-flow.ts @@ -41,6 +41,8 @@ interface StartAuthOptions { const POLL_INTERVAL = 3000; /** Maximum polling duration (5 minutes) */ const MAX_POLL_DURATION = 5 * 60 * 1000; +/** Fail visibly after repeated poll transport errors instead of retrying forever */ +const MAX_POLL_FAILURES = 3; async function parseResponseBody(response: Response): Promise> { const text = await response.text(); @@ -69,18 +71,26 @@ const INITIAL_STATE: AuthFlowState = { export function useCliproxyAuthFlow() { const [state, setState] = useState(INITIAL_STATE); + const attemptIdRef = useRef(0); const abortControllerRef = useRef(null); const pollIntervalRef = useRef | null>(null); const pollStartRef = useRef(0); + const pollFailureCountRef = useRef(0); const openedAuthUrlRef = useRef(false); const queryClient = useQueryClient(); + const isActiveAttempt = useCallback( + (attemptId: number) => attemptId === attemptIdRef.current, + [] + ); + // Clear polling const stopPolling = useCallback(() => { if (pollIntervalRef.current) { clearInterval(pollIntervalRef.current); pollIntervalRef.current = null; } + pollFailureCountRef.current = 0; }, []); // Cleanup on unmount @@ -94,15 +104,21 @@ export function useCliproxyAuthFlow() { // Poll OAuth status const pollStatus = useCallback( - async (provider: string, oauthState: string) => { + async (provider: string, oauthState: string, attemptId: number) => { + if (!isActiveAttempt(attemptId)) { + return; + } + // Check timeout if (Date.now() - pollStartRef.current > MAX_POLL_DURATION) { stopPolling(); - setState((prev) => ({ - ...prev, - isAuthenticating: false, - error: 'Authentication timed out. Please try again.', - })); + if (isActiveAttempt(attemptId)) { + setState((prev) => ({ + ...prev, + isAuthenticating: false, + error: 'Authentication timed out. Please try again.', + })); + } return; } @@ -110,6 +126,10 @@ export function useCliproxyAuthFlow() { const response = await fetch( `/api/cliproxy/auth/${provider}/status?state=${encodeURIComponent(oauthState)}` ); + if (!isActiveAttempt(attemptId)) { + return; + } + const data = (await response.json()) as { status?: string; error?: string; @@ -118,6 +138,7 @@ export function useCliproxyAuthFlow() { verification_url?: string; user_code?: string; }; + pollFailureCountRef.current = 0; if (data.status === 'ok') { stopPolling(); @@ -161,11 +182,30 @@ export function useCliproxyAuthFlow() { })); } // status === 'wait' (or pending) means continue polling - } catch { - // Network error - continue polling + } catch (error) { + if (!isActiveAttempt(attemptId)) { + return; + } + + pollFailureCountRef.current += 1; + if (pollFailureCountRef.current < MAX_POLL_FAILURES) { + return; + } + + stopPolling(); + const message = + error instanceof Error && error.message.trim().length > 0 + ? error.message + : 'Lost contact with the auth status endpoint'; + toast.error(message); + setState((prev) => ({ + ...prev, + isAuthenticating: false, + error: message, + })); } }, - [queryClient, stopPolling] + [isActiveAttempt, queryClient, stopPolling] ); const startAuth = useCallback( @@ -182,9 +222,12 @@ export function useCliproxyAuthFlow() { abortControllerRef.current?.abort(); stopPolling(); openedAuthUrlRef.current = false; + pollFailureCountRef.current = 0; // Create fresh controller and capture locally to avoid race with cancelAuth const controller = new AbortController(); + const attemptId = attemptIdRef.current + 1; + attemptIdRef.current = attemptId; abortControllerRef.current = controller; const flowType = @@ -219,7 +262,13 @@ export function useCliproxyAuthFlow() { signal: controller.signal, }) .then(async (response) => { + if (!isActiveAttempt(attemptId)) { + return; + } const data = await parseResponseBody(response); + if (!isActiveAttempt(attemptId)) { + return; + } const success = data.success === true; if (response.ok && success) { queryClient.invalidateQueries({ queryKey: ['cliproxy-auth'] }); @@ -240,6 +289,9 @@ export function useCliproxyAuthFlow() { } }) .catch((error) => { + if (!isActiveAttempt(attemptId)) { + return; + } if (error instanceof Error && error.name === 'AbortError') { // Cancelled - state already reset by cancelAuth return; @@ -261,8 +313,14 @@ export function useCliproxyAuthFlow() { body: JSON.stringify(payload), signal: controller.signal, }); + if (!isActiveAttempt(attemptId)) { + return; + } const data = await parseResponseBody(response); + if (!isActiveAttempt(attemptId)) { + return; + } const success = data.success === true; if (!response.ok || !success) { @@ -290,11 +348,14 @@ export function useCliproxyAuthFlow() { if (oauthState) { pollStartRef.current = Date.now(); pollIntervalRef.current = setInterval(() => { - pollStatus(provider, oauthState); + void pollStatus(provider, oauthState, attemptId); }, POLL_INTERVAL); } } } catch (error) { + if (!isActiveAttempt(attemptId)) { + return; + } if (error instanceof Error && error.name === 'AbortError') { openedAuthUrlRef.current = false; setState(INITIAL_STATE); @@ -309,11 +370,12 @@ export function useCliproxyAuthFlow() { })); } }, - [pollStatus, stopPolling, queryClient] + [isActiveAttempt, pollStatus, stopPolling, queryClient] ); const cancelAuth = useCallback(() => { const currentProvider = state.provider; + attemptIdRef.current += 1; abortControllerRef.current?.abort(); stopPolling(); openedAuthUrlRef.current = false; @@ -329,37 +391,53 @@ export function useCliproxyAuthFlow() { const submitCallback = useCallback( async (redirectUrl: string) => { if (!state.provider) return; + const attemptId = attemptIdRef.current; + const currentProvider = state.provider; setState((prev) => ({ ...prev, isSubmittingCallback: true, error: null })); try { - const response = await fetch(`/api/cliproxy/auth/${state.provider}/submit-callback`, { + const response = await fetch(`/api/cliproxy/auth/${currentProvider}/submit-callback`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ redirectUrl }), }); + if (!isActiveAttempt(attemptId)) { + return; + } const data = await parseResponseBody(response); + if (!isActiveAttempt(attemptId)) { + return; + } const success = data.success === true; + const hasAccount = typeof data.account === 'object' && data.account !== null; - if (response.ok && success) { + if (response.ok && success && hasAccount) { stopPolling(); queryClient.invalidateQueries({ queryKey: ['cliproxy-auth'] }); queryClient.invalidateQueries({ queryKey: ['account-quota'] }); - toast.success(`${state.provider} authentication successful`); + toast.success(`${currentProvider} authentication successful`); setState(INITIAL_STATE); } else { const errorMsg = - typeof data.error === 'string' ? data.error : 'Callback submission failed'; + typeof data.error === 'string' + ? data.error + : success + ? 'Authenticated account could not be registered' + : 'Callback submission failed'; throw new Error(errorMsg); } } catch (error) { + if (!isActiveAttempt(attemptId)) { + return; + } const message = error instanceof Error ? error.message : 'Failed to submit callback'; toast.error(message); setState((prev) => ({ ...prev, isSubmittingCallback: false, error: message })); } }, - [state.provider, queryClient, stopPolling] + [isActiveAttempt, state.provider, queryClient, stopPolling] ); return useMemo( diff --git a/ui/src/lib/api-client.ts b/ui/src/lib/api-client.ts index 86faae54..e6c18c2c 100644 --- a/ui/src/lib/api-client.ts +++ b/ui/src/lib/api-client.ts @@ -854,15 +854,21 @@ export const api = { method: 'POST', body: JSON.stringify(data), }), - update: (family: AiProviderFamilyId, index: number, data: UpsertAiProviderEntryInput) => - request(`/cliproxy/ai-providers/${encodeURIComponent(family)}/${index}`, { - method: 'PUT', - body: JSON.stringify(data), - }), - delete: (family: AiProviderFamilyId, index: number) => - request(`/cliproxy/ai-providers/${encodeURIComponent(family)}/${index}`, { - method: 'DELETE', - }), + update: (family: AiProviderFamilyId, entryId: string, data: UpsertAiProviderEntryInput) => + request( + `/cliproxy/ai-providers/${encodeURIComponent(family)}/${encodeURIComponent(entryId)}`, + { + method: 'PUT', + body: JSON.stringify(data), + } + ), + delete: (family: AiProviderFamilyId, entryId: string) => + request( + `/cliproxy/ai-providers/${encodeURIComponent(family)}/${encodeURIComponent(entryId)}`, + { + method: 'DELETE', + } + ), }, // Config YAML for Config tab diff --git a/ui/src/pages/cliproxy-ai-providers.tsx b/ui/src/pages/cliproxy-ai-providers.tsx index 78deca16..7b747da4 100644 --- a/ui/src/pages/cliproxy-ai-providers.tsx +++ b/ui/src/pages/cliproxy-ai-providers.tsx @@ -1759,7 +1759,7 @@ export function CliproxyAiProvidersPage() { onSave={async (payload) => { await updateMutation.mutateAsync({ family: selectedFamily, - index: selectedEntry.index, + entryId: selectedEntry.id, data: payload, }); void refetch(); @@ -1793,7 +1793,7 @@ export function CliproxyAiProvidersPage() { if (editingEntry) { await updateMutation.mutateAsync({ family: selectedFamily, - index: editingEntry.index, + entryId: editingEntry.id, data: payload, }); } else { @@ -1820,7 +1820,7 @@ export function CliproxyAiProvidersPage() { if (!deleteEntry) return; await deleteMutation.mutateAsync({ family: selectedFamily, - index: deleteEntry.index, + entryId: deleteEntry.id, }); setDeleteEntry(null); }} diff --git a/ui/tests/unit/hooks/use-cliproxy-auth-flow.test.tsx b/ui/tests/unit/hooks/use-cliproxy-auth-flow.test.tsx new file mode 100644 index 00000000..8498ce9f --- /dev/null +++ b/ui/tests/unit/hooks/use-cliproxy-auth-flow.test.tsx @@ -0,0 +1,199 @@ +import { act, renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { toast } from 'sonner'; +import { AllProviders } from '../../setup/test-utils'; +import { useCliproxyAuthFlow } from '@/hooks/use-cliproxy-auth-flow'; + +vi.mock('sonner', () => ({ + toast: { + success: vi.fn(), + error: vi.fn(), + }, +})); + +function createJsonResponse(body: Record, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }); +} + +function createDeferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} +); + +describe('useCliproxyAuthFlow', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + vi.spyOn(window, 'open').mockImplementation(() => null); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('ignores stale poll completions from a superseded auth attempt', async () => { + const firstPoll = createDeferred(); + let startCount = 0; + + vi.stubGlobal( + 'fetch', + vi.fn((input: RequestInfo | URL) => { + const url = String(input); + + if (url.includes('/start-url')) { + startCount += 1; + return Promise.resolve( + createJsonResponse({ + success: true, + authUrl: `https://auth.example/${startCount}`, + state: `state-${startCount}`, + }) + ); + } + + if (url.includes('/status?state=state-1')) { + return firstPoll.promise; + } + + if (url.includes('/status?state=state-2')) { + return Promise.resolve(createJsonResponse({ status: 'wait' })); + } + + return Promise.reject(new Error(`Unexpected fetch: ${url}`)); + }) + ); + + const { result } = renderHook(() => useCliproxyAuthFlow(), { wrapper }); + + await act(async () => { + await result.current.startAuth('gemini', { startEndpoint: 'start-url' }); + }); + + expect(result.current.oauthState).toBe('state-1'); + + await act(async () => { + vi.advanceTimersByTime(3000); + await Promise.resolve(); + }); + + await act(async () => { + await result.current.startAuth('gemini', { startEndpoint: 'start-url' }); + }); + + expect(result.current.oauthState).toBe('state-2'); + expect(result.current.isAuthenticating).toBe(true); + + await act(async () => { + firstPoll.resolve(createJsonResponse({ status: 'ok' })); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(result.current.oauthState).toBe('state-2'); + expect(result.current.isAuthenticating).toBe(true); + expect(toast.success).not.toHaveBeenCalled(); + }); + + it('surfaces repeated poll transport failures instead of retrying forever', async () => { + vi.stubGlobal( + 'fetch', + vi.fn((input: RequestInfo | URL) => { + const url = String(input); + + if (url.includes('/start-url')) { + return Promise.resolve( + createJsonResponse({ + success: true, + authUrl: 'https://auth.example/fail', + state: 'state-fail', + }) + ); + } + + if (url.includes('/status?state=state-fail')) { + return Promise.reject(new Error('poll failed')); + } + + return Promise.reject(new Error(`Unexpected fetch: ${url}`)); + }) + ); + + const { result } = renderHook(() => useCliproxyAuthFlow(), { wrapper }); + + await act(async () => { + await result.current.startAuth('gemini', { startEndpoint: 'start-url' }); + }); + + for (let attempt = 0; attempt < 3; attempt += 1) { + await act(async () => { + vi.advanceTimersByTime(3000); + await Promise.resolve(); + await Promise.resolve(); + }); + } + + expect(result.current.error).toBe('poll failed'); + expect(result.current.isAuthenticating).toBe(false); + expect(toast.error).toHaveBeenCalledWith('poll failed'); + }); + + it('treats callback responses without an account as failures', async () => { + vi.stubGlobal( + 'fetch', + vi.fn((input: RequestInfo | URL) => { + const url = String(input); + + if (url.includes('/start-url')) { + return Promise.resolve( + createJsonResponse({ + success: true, + authUrl: 'https://auth.example/callback', + state: 'state-callback', + }) + ); + } + + if (url.includes('/submit-callback')) { + return Promise.resolve(createJsonResponse({ success: true, account: null })); + } + + if (url.includes('/status?state=state-callback')) { + return Promise.resolve(createJsonResponse({ status: 'wait' })); + } + + return Promise.reject(new Error(`Unexpected fetch: ${url}`)); + }) + ); + + const { result } = renderHook(() => useCliproxyAuthFlow(), { wrapper }); + + await act(async () => { + await result.current.startAuth('gemini', { startEndpoint: 'start-url' }); + }); + + await act(async () => { + await result.current.submitCallback( + 'http://localhost/callback?code=abc123&state=state-callback' + ); + }); + + expect(result.current.error).toBe('Authenticated account could not be registered'); + expect(result.current.isSubmittingCallback).toBe(false); + expect(toast.error).toHaveBeenCalledWith('Authenticated account could not be registered'); + expect(toast.success).not.toHaveBeenCalled(); + }); +});