diff --git a/src/auth/commands/create-command.ts b/src/auth/commands/create-command.ts index bc40fea4..614a0ec8 100644 --- a/src/auth/commands/create-command.ts +++ b/src/auth/commands/create-command.ts @@ -15,6 +15,7 @@ import { policyToAccountContextMetadata, formatAccountContextPolicy, isValidAccountProfileName, + resolveAccountContextPolicy, } from '../account-context'; import { exitWithError } from '../../errors'; import { ExitCode } from '../../errors/exit-codes'; @@ -24,6 +25,47 @@ function sanitizeProfileNameForInstance(name: string): string { return name.replace(/[^a-zA-Z0-9_-]/g, '-').toLowerCase(); } +const AMBIENT_PROVIDER_PREFIXES = [ + 'ANTHROPIC_', + 'OPENAI_', + 'GOOGLE_', + 'GEMINI_', + 'MINIMAX_', + 'QWEN_', + 'DEEPSEEK_', + 'KIMI_', + 'AZURE_', + 'OLLAMA_', +]; +const AMBIENT_PROVIDER_EXACT_KEYS = new Set([ + 'OPENROUTER_API_KEY', + 'OPENROUTER_KEY', + 'XAI_API_KEY', + 'MISTRAL_API_KEY', + 'COHERE_API_KEY', +]); +const AMBIENT_PROVIDER_SUFFIXES = ['_API_KEY', '_AUTH_TOKEN', '_ACCESS_TOKEN', '_SECRET_KEY']; + +function stripAmbientProviderCredentials(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const sanitized: NodeJS.ProcessEnv = { ...env }; + + for (const envKey of Object.keys(sanitized)) { + if (envKey === 'CLAUDE_CONFIG_DIR') { + continue; + } + + if ( + AMBIENT_PROVIDER_PREFIXES.some((prefix) => envKey.startsWith(prefix)) || + AMBIENT_PROVIDER_EXACT_KEYS.has(envKey) || + AMBIENT_PROVIDER_SUFFIXES.some((suffix) => envKey.endsWith(suffix)) + ) { + delete sanitized[envKey]; + } + } + + return sanitized; +} + /** * Handle the create command */ @@ -31,6 +73,13 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise await initUI(); const { profileName, force, shareContext, contextGroup, unknownFlags } = parseArgs(args); + if (unknownFlags && unknownFlags.length > 0) { + const unknownList = unknownFlags.join(', '); + console.log(fail(`Unknown option(s): ${unknownList}`)); + console.log(''); + exitWithError(`Unknown option(s): ${unknownList}`, ExitCode.PROFILE_ERROR); + } + if (!profileName) { console.log(fail('Profile name is required')); console.log(''); @@ -43,13 +92,6 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise exitWithError('Profile name is required', ExitCode.PROFILE_ERROR); } - if (unknownFlags && unknownFlags.length > 0) { - const unknownList = unknownFlags.join(', '); - console.log(fail(`Unknown option(s): ${unknownList}`)); - console.log(''); - exitWithError(`Unknown option(s): ${unknownList}`, ExitCode.PROFILE_ERROR); - } - if (!isValidAccountProfileName(profileName)) { const error = 'Invalid profile name. Use letters/numbers/dash/underscore and start with a letter.'; @@ -100,33 +142,73 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise useUnifiedConfig && existsUnified ? ctx.registry.getAllAccountsUnified()[profileName] : undefined; + const previousContextPolicy = + !createdProfile && (previousUnifiedProfile || previousLegacyProfile) + ? resolveAccountContextPolicy(previousUnifiedProfile || previousLegacyProfile) + : undefined; + + const claudeInfo = getClaudeCliInfo(); + if (!claudeInfo) { + console.log(fail('Claude CLI not found')); + console.log(''); + console.log('Please install Claude CLI first:'); + console.log(` ${color('https://claude.ai/download', 'path')}`); + exitWithError('Claude CLI not found', ExitCode.BINARY_ERROR); + } + + let rollbackCompleted = false; + const rollbackMetadata = (): void => { + try { + if (useUnifiedConfig) { + if (createdProfile) { + if (ctx.registry.hasAccountUnified(profileName)) { + ctx.registry.removeAccountUnified(profileName); + } + } else if (previousUnifiedProfile) { + ctx.registry.updateAccountUnified(profileName, previousUnifiedProfile); + } + return; + } + + if (createdProfile) { + if (ctx.registry.hasProfile(profileName)) { + ctx.registry.deleteProfile(profileName); + } + } else if (previousLegacyProfile) { + ctx.registry.updateProfile(profileName, previousLegacyProfile); + } + } catch { + // Best-effort rollback to avoid leaving stale accounts after failed login. + } + }; + + const rollbackFailedCreate = async (): Promise => { + if (rollbackCompleted) { + return; + } + rollbackCompleted = true; + + rollbackMetadata(); + + if (createdProfile) { + try { + ctx.instanceMgr.deleteInstance(profileName); + } catch { + // Best-effort cleanup. + } + return; + } + + if (previousContextPolicy) { + try { + await ctx.instanceMgr.ensureInstance(profileName, previousContextPolicy); + } catch { + // Best-effort rollback for context mode/group. + } + } + }; try { - const rollbackMetadata = (): void => { - try { - if (useUnifiedConfig) { - if (createdProfile) { - if (ctx.registry.hasAccountUnified(profileName)) { - ctx.registry.removeAccountUnified(profileName); - } - } else if (previousUnifiedProfile) { - ctx.registry.updateAccountUnified(profileName, previousUnifiedProfile); - } - return; - } - - if (createdProfile) { - if (ctx.registry.hasProfile(profileName)) { - ctx.registry.deleteProfile(profileName); - } - } else if (previousLegacyProfile) { - ctx.registry.updateProfile(profileName, previousLegacyProfile); - } - } catch { - // Best-effort rollback to avoid leaving stale accounts after failed login. - } - }; - // Create instance directory console.log(info(`Creating profile: ${profileName}`)); const instancePath = await ctx.instanceMgr.ensureInstance(profileName, contextPolicy); @@ -170,53 +252,39 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise console.log(warn('You will be prompted to login with your account.')); console.log(''); - // Detect Claude CLI - const claudeInfo = getClaudeCliInfo(); - if (!claudeInfo) { - console.log(fail('Claude CLI not found')); - console.log(''); - console.log('Please install Claude CLI first:'); - console.log(` ${color('https://claude.ai/download', 'path')}`); - exitWithError('Claude CLI not found', ExitCode.BINARY_ERROR); - } - const { path: claudeCli, needsShell } = claudeInfo; - const childEnv = stripClaudeCodeEnv({ ...process.env, CLAUDE_CONFIG_DIR: instancePath }); - // Avoid ambient provider credentials influencing account-login bootstrap behavior. - const ambientProviderPrefixes = ['ANTHROPIC_', 'OPENAI_', 'GOOGLE_', 'GEMINI_', 'MINIMAX_']; - for (const envKey of Object.keys(childEnv)) { - if (envKey === 'CLAUDE_CONFIG_DIR') { - continue; - } - - if ( - ambientProviderPrefixes.some((prefix) => envKey.startsWith(prefix)) || - envKey === 'OPENROUTER_API_KEY' - ) { - delete childEnv[envKey]; - } - } + const childEnv = stripAmbientProviderCredentials( + stripClaudeCodeEnv({ ...process.env, CLAUDE_CONFIG_DIR: instancePath }) + ); // Execute Claude in isolated instance (will auto-prompt for login if no credentials) // On Windows, .cmd/.bat/.ps1 files need shell: true to execute properly let child: ChildProcess; - if (needsShell) { - const cmdString = escapeShellArg(claudeCli); - child = spawn(cmdString, { - stdio: 'inherit', - windowsHide: true, - shell: true, - env: childEnv, - }); - } else { - child = spawn(claudeCli, [], { - stdio: 'inherit', - windowsHide: true, - env: childEnv, - }); + try { + if (needsShell) { + const cmdString = escapeShellArg(claudeCli); + child = spawn(cmdString, { + stdio: 'inherit', + windowsHide: true, + shell: true, + env: childEnv, + }); + } else { + child = spawn(claudeCli, [], { + stdio: 'inherit', + windowsHide: true, + env: childEnv, + }); + } + } catch (error) { + await rollbackFailedCreate(); + exitWithError( + `Failed to execute Claude CLI: ${(error as Error).message}`, + ExitCode.BINARY_ERROR + ); } - child.on('exit', (code: number | null) => { + child.on('exit', async (code: number | null) => { if (code === 0) { console.log(''); console.log( @@ -246,10 +314,7 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise console.log(''); process.exit(0); } else { - rollbackMetadata(); - if (createdProfile) { - ctx.instanceMgr.deleteInstance(profileName); - } + await rollbackFailedCreate(); console.log(''); console.log(fail('Login failed or cancelled')); @@ -261,14 +326,12 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise } }); - child.on('error', (err: Error) => { - rollbackMetadata(); - if (createdProfile) { - ctx.instanceMgr.deleteInstance(profileName); - } + child.on('error', async (err: Error) => { + await rollbackFailedCreate(); exitWithError(`Failed to execute Claude CLI: ${err.message}`, ExitCode.BINARY_ERROR); }); } catch (error) { + await rollbackFailedCreate(); exitWithError(`Failed to create profile: ${(error as Error).message}`, ExitCode.GENERAL_ERROR); } } diff --git a/src/auth/profile-registry.ts b/src/auth/profile-registry.ts index e6d7beda..5870db61 100644 --- a/src/auth/profile-registry.ts +++ b/src/auth/profile-registry.ts @@ -159,7 +159,7 @@ export class ProfileRegistry { throw new Error(`Profile not found: ${name}`); } - return data.profiles[name]; + return this.normalizeLegacyProfileMetadata(data.profiles[name]); } /** @@ -215,7 +215,11 @@ export class ProfileRegistry { */ getAllProfiles(): Record { const data = this._read(); - return data.profiles; + const normalized: Record = {}; + for (const [name, profile] of Object.entries(data.profiles)) { + normalized[name] = this.normalizeLegacyProfileMetadata(profile); + } + return normalized; } /** @@ -357,7 +361,11 @@ export class ProfileRegistry { getAllAccountsUnified(): Record { if (!isUnifiedMode()) return {}; const config = loadOrCreateUnifiedConfig(); - return config.accounts; + const normalized: Record = {}; + for (const [name, account] of Object.entries(config.accounts)) { + normalized[name] = this.normalizeUnifiedAccountConfig(account); + } + return normalized; } /** diff --git a/src/config/migration-manager.ts b/src/config/migration-manager.ts index 2a94f78b..cf9628d2 100644 --- a/src/config/migration-manager.ts +++ b/src/config/migration-manager.ts @@ -21,6 +21,7 @@ import type { ProfileConfig, AccountConfig, CLIProxyVariantConfig } from './unif import { createEmptyUnifiedConfig } from './unified-config-types'; import { CLIPROXY_PROVIDER_IDS } from '../cliproxy/provider-capabilities'; import { saveUnifiedConfig, hasUnifiedConfig, loadUnifiedConfig } from './unified-config-loader'; +import { isValidContextGroupName, normalizeContextGroupName } from '../auth/account-context'; import { infoBox, warn } from '../utils/ui'; const BACKUP_DIR_PREFIX = 'backup-v1-'; @@ -151,10 +152,17 @@ export async function migrate(dryRun = false): Promise { const rawContextMode = metadata.context_mode; const rawContextGroup = metadata.context_group; const contextMode = rawContextMode === 'shared' ? 'shared' : 'isolated'; - const contextGroup = - typeof rawContextGroup === 'string' && rawContextGroup.trim().length > 0 - ? rawContextGroup - : undefined; + let contextGroup: string | undefined; + if (typeof rawContextGroup === 'string' && rawContextGroup.trim().length > 0) { + const normalizedGroup = normalizeContextGroupName(rawContextGroup); + if (isValidContextGroupName(normalizedGroup)) { + contextGroup = normalizedGroup; + } else { + warnings.push( + `Skipped invalid context group for account "${name}": "${rawContextGroup}" (fallback to default shared group)` + ); + } + } const account: AccountConfig = { created: (metadata.created as string) || new Date().toISOString(), last_used: (metadata.last_used as string) || null, diff --git a/src/management/instance-manager.ts b/src/management/instance-manager.ts index b51abae3..e39acbb1 100644 --- a/src/management/instance-manager.ts +++ b/src/management/instance-manager.ts @@ -8,6 +8,7 @@ import * as fs from 'fs'; import * as path from 'path'; +import { createHash } from 'crypto'; import SharedManager from './shared-manager'; import { AccountContextPolicy, DEFAULT_ACCOUNT_CONTEXT_MODE } from '../auth/account-context'; import { getCcsDir } from '../utils/config-manager'; @@ -203,7 +204,39 @@ class InstanceManager { private getContextSyncLockPath(profileName: string): string { const safeName = this.sanitizeName(profileName); - return path.join(this.locksDir, `${safeName}.lock`); + // Keep lock filenames deterministic while preventing normalized-name collisions. + const profileHash = createHash('sha1').update(profileName).digest('hex').slice(0, 8); + return path.join(this.locksDir, `${safeName}-${profileHash}.lock`); + } + + private isProcessAlive(pid: number): boolean { + if (!Number.isInteger(pid) || pid <= 0) { + return false; + } + + try { + process.kill(pid, 0); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EPERM') { + return true; + } + return false; + } + } + + private tryRemoveDeadOwnerLock(lockPath: string): boolean { + try { + const lockContent = fs.readFileSync(lockPath, 'utf8').trim(); + const lockPid = Number.parseInt(lockContent, 10); + if (!this.isProcessAlive(lockPid)) { + fs.unlinkSync(lockPath); + return true; + } + } catch { + // Best-effort stale lock cleanup. + } + return false; } private async withContextSyncLock( @@ -212,8 +245,8 @@ class InstanceManager { ): Promise { const lockPath = this.getContextSyncLockPath(profileName); const retryDelayMs = 50; - const timeoutMs = 5000; const staleLockMs = 30000; + const timeoutMs = staleLockMs + 5000; const start = Date.now(); fs.mkdirSync(this.locksDir, { recursive: true, mode: 0o700 }); @@ -236,6 +269,10 @@ class InstanceManager { fs.unlinkSync(lockPath); continue; } + + if (this.tryRemoveDeadOwnerLock(lockPath)) { + continue; + } } catch { // Best-effort stale lock cleanup. } diff --git a/src/management/shared-manager.ts b/src/management/shared-manager.ts index b48aeb54..e2ae5ad8 100644 --- a/src/management/shared-manager.ts +++ b/src/management/shared-manager.ts @@ -240,6 +240,7 @@ class SharedManager { if ( currentTarget && path.resolve(currentTarget) !== path.resolve(sharedProjectsPath) && + this.isSafeProjectsMergeSource(currentTarget, instanceName) && (await this.pathExists(currentTarget)) ) { await this.mergeDirectoryWithConflictCopies( @@ -247,6 +248,10 @@ class SharedManager { sharedProjectsPath, instanceName ); + } else if (currentTarget && !this.isSafeProjectsMergeSource(currentTarget, instanceName)) { + console.log( + warn(`Skipping unsafe project merge source outside CCS roots: ${currentTarget}`) + ); } await fs.promises.unlink(projectsPath); @@ -286,9 +291,14 @@ class SharedManager { if ( currentTarget && path.resolve(currentTarget) !== path.resolve(projectsPath) && + this.isSafeProjectsMergeSource(currentTarget, instanceName) && (await this.pathExists(currentTarget)) ) { await this.mergeDirectoryWithConflictCopies(currentTarget, projectsPath, instanceName); + } else if (currentTarget && !this.isSafeProjectsMergeSource(currentTarget, instanceName)) { + console.log( + warn(`Skipping unsafe project merge source outside CCS roots: ${currentTarget}`) + ); } return; @@ -678,6 +688,24 @@ class SharedManager { } } + /** + * Guard project merge operations to known CCS-managed roots only. + */ + private isSafeProjectsMergeSource(sourcePath: string, instanceName: string): boolean { + const resolvedSource = path.resolve(sourcePath); + const sharedContextRoot = path.resolve(path.join(this.sharedDir, 'context-groups')); + const instanceProjectsRoot = path.resolve( + path.join(this.instancesDir, instanceName, 'projects') + ); + + const isWithin = (child: string, root: string): boolean => + child === root || child.startsWith(`${root}${path.sep}`); + + return ( + isWithin(resolvedSource, sharedContextRoot) || isWithin(resolvedSource, instanceProjectsRoot) + ); + } + /** * Link directory with Windows fallback to recursive copy. */ diff --git a/src/web-server/routes/account-route-helpers.ts b/src/web-server/routes/account-route-helpers.ts new file mode 100644 index 00000000..645262be --- /dev/null +++ b/src/web-server/routes/account-route-helpers.ts @@ -0,0 +1,45 @@ +import type { CLIProxyProvider } from '../../cliproxy/types'; +import { isCLIProxyProvider } from '../../cliproxy/provider-capabilities'; + +export interface MergedAccountEntry { + type: string; + created: string; + last_used: string | null; + context_mode?: 'isolated' | 'shared'; + context_group?: string; + provider?: string; + displayName?: string; +} + +/** Parse CLIProxy account key format: "provider:accountId" */ +export function parseCliproxyKey( + key: string +): { provider: CLIProxyProvider; accountId: string } | null { + let normalizedKey = key; + if (key.startsWith('cliproxy:')) { + normalizedKey = key.slice('cliproxy:'.length); + } else if (key.startsWith('cliproxy+')) { + normalizedKey = key.slice('cliproxy+'.length); + } + const colonIndex = normalizedKey.indexOf(':'); + if (colonIndex === -1) return null; + + const provider = normalizedKey.slice(0, colonIndex); + const accountId = normalizedKey.slice(colonIndex + 1); + + if (!isCLIProxyProvider(provider) || !accountId) return null; + return { provider, accountId }; +} + +export function buildCliproxyAccountKey( + rawKey: string, + merged: Record +): string | null { + const candidateKeys = [rawKey, `cliproxy:${rawKey}`, `cliproxy+${rawKey}`]; + for (const key of candidateKeys) { + if (!merged[key]) { + return key; + } + } + return null; +} diff --git a/src/web-server/routes/account-routes.ts b/src/web-server/routes/account-routes.ts index a92422aa..90ace229 100644 --- a/src/web-server/routes/account-routes.ts +++ b/src/web-server/routes/account-routes.ts @@ -12,31 +12,24 @@ import { isUnifiedMode } from '../../config/unified-config-loader'; import { getAllAccountsSummary, setDefaultAccount as setCliproxyDefault, + getDefaultAccount as getCliproxyDefaultAccount, removeAccount as removeCliproxyAccount, bulkPauseAccounts, bulkResumeAccounts, soloAccount, } from '../../cliproxy/account-manager'; -import type { CLIProxyProvider } from '../../cliproxy/types'; import { isCLIProxyProvider } from '../../cliproxy/provider-capabilities'; +import { resolveAccountContextPolicy } from '../../auth/account-context'; +import { + buildCliproxyAccountKey, + parseCliproxyKey, + type MergedAccountEntry, +} from './account-route-helpers'; const router = Router(); const registry = new ProfileRegistry(); const instanceMgr = new InstanceManager(); -/** Parse CLIProxy account key format: "provider:accountId" */ -function parseCliproxyKey(key: string): { provider: CLIProxyProvider; accountId: string } | null { - const normalizedKey = key.startsWith('cliproxy:') ? key.slice('cliproxy:'.length) : key; - const colonIndex = normalizedKey.indexOf(':'); - if (colonIndex === -1) return null; - - const provider = normalizedKey.slice(0, colonIndex); - const accountId = normalizedKey.slice(colonIndex + 1); - - if (!isCLIProxyProvider(provider) || !accountId) return null; - return { provider, accountId }; -} - function hasAuthAccount(name: string): boolean { return registry.hasAccountUnified(name) || registry.hasProfile(name); } @@ -54,38 +47,29 @@ router.get('/', (_req: Request, res: Response): void => { const cliproxyAccounts = getAllAccountsSummary(); // Merge profiles: unified config takes precedence - const merged: Record< - string, - { - type: string; - created: string; - last_used: string | null; - context_mode?: 'isolated' | 'shared'; - context_group?: string; - provider?: string; - displayName?: string; - } - > = {}; + const merged: Record = {}; // Add legacy profiles first for (const [name, meta] of Object.entries(legacyProfiles)) { + const contextPolicy = resolveAccountContextPolicy(meta); merged[name] = { type: meta.type || 'account', created: meta.created, last_used: meta.last_used || null, - context_mode: meta.context_mode, - context_group: meta.context_group, + context_mode: contextPolicy.mode, + context_group: contextPolicy.group, }; } // Override with unified config accounts (takes precedence) for (const [name, account] of Object.entries(unifiedAccounts)) { + const contextPolicy = resolveAccountContextPolicy(account); merged[name] = { type: 'account', created: account.created, last_used: account.last_used, - context_mode: account.context_mode, - context_group: account.context_group, + context_mode: contextPolicy.mode, + context_group: contextPolicy.group, }; } @@ -99,7 +83,10 @@ router.get('/', (_req: Request, res: Response): void => { // Use unique ID for key to prevent collisions between accounts with same nickname/email const displayName = acct.nickname || acct.email || acct.id; const rawKey = `${provider}:${acct.id}`; - const key = merged[rawKey] ? `cliproxy:${rawKey}` : rawKey; + const key = buildCliproxyAccountKey(rawKey, merged); + if (!key) { + continue; + } merged[key] = { type: 'cliproxy', provider, @@ -202,6 +189,14 @@ router.delete('/:name', (req: Request, res: Response): void => { // Check if this is a CLIProxy account (format: "provider:accountId") const cliproxyKey = !hasAuthAccount(name) ? parseCliproxyKey(name) : null; if (cliproxyKey) { + const defaultCliproxyAccount = getCliproxyDefaultAccount(cliproxyKey.provider); + if (defaultCliproxyAccount?.id === cliproxyKey.accountId) { + res.status(400).json({ + error: `Cannot delete default CLIProxy account: ${name}. Set another default first.`, + }); + return; + } + const success = removeCliproxyAccount(cliproxyKey.provider, cliproxyKey.accountId); if (!success) { res.status(404).json({ error: `CLIProxy account not found: ${name}` }); diff --git a/src/web-server/routes/config-routes.ts b/src/web-server/routes/config-routes.ts index d00985ca..537f7348 100644 --- a/src/web-server/routes/config-routes.ts +++ b/src/web-server/routes/config-routes.ts @@ -18,9 +18,57 @@ import { getBackupDirectories, } from '../../config/migration-manager'; import { isUnifiedConfig } from '../../config/unified-config-types'; +import { isValidContextGroupName, normalizeContextGroupName } from '../../auth/account-context'; const router = Router(); +function validateAccountContextMetadata(config: unknown): string | null { + if (typeof config !== 'object' || config === null) { + return 'Invalid config payload'; + } + + const candidate = config as Record; + const accounts = candidate.accounts; + if (accounts === undefined) { + return null; + } + + if (typeof accounts !== 'object' || accounts === null || Array.isArray(accounts)) { + return 'Invalid config.accounts: expected object'; + } + + for (const [accountName, accountValue] of Object.entries(accounts as Record)) { + if (typeof accountValue !== 'object' || accountValue === null || Array.isArray(accountValue)) { + return `Invalid config.accounts.${accountName}: expected object`; + } + + const account = accountValue as Record; + const mode = account.context_mode; + const group = account.context_group; + + if (mode !== undefined && mode !== 'isolated' && mode !== 'shared') { + return `Invalid config.accounts.${accountName}.context_mode: expected isolated|shared`; + } + + if (group !== undefined && typeof group !== 'string') { + return `Invalid config.accounts.${accountName}.context_group: expected string`; + } + + if (mode !== 'shared' && group !== undefined) { + return `Invalid config.accounts.${accountName}: context_group requires context_mode=shared`; + } + + if (mode === 'shared' && typeof group === 'string' && group.trim().length > 0) { + const normalizedGroup = normalizeContextGroupName(group); + if (!isValidContextGroupName(normalizedGroup)) { + return `Invalid config.accounts.${accountName}.context_group`; + } + } + } + + return null; +} + /** * GET /api/config/format - Return current config format and migration status */ @@ -82,6 +130,12 @@ router.put('/', (req: Request, res: Response): void => { return; } + const accountContextError = validateAccountContextMetadata(config); + if (accountContextError) { + res.status(400).json({ error: accountContextError }); + return; + } + try { saveUnifiedConfig(config); res.json({ success: true }); diff --git a/tests/unit/account-context.test.ts b/tests/unit/account-context.test.ts index 8b18b55d..7e282a58 100644 --- a/tests/unit/account-context.test.ts +++ b/tests/unit/account-context.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'bun:test'; import { MAX_CONTEXT_GROUP_LENGTH, isValidAccountProfileName, + policyToAccountContextMetadata, resolveAccountContextPolicy, resolveCreateAccountContext, } from '../../src/auth/account-context'; @@ -28,4 +29,15 @@ describe('account context helpers', () => { expect(resolved.mode).toBe('shared'); expect(resolved.group).toBe('default'); }); + + it('round-trips shared policy metadata with normalized context group', () => { + const metadata = policyToAccountContextMetadata({ + mode: 'shared', + group: 'Sprint-A', + }); + + const resolved = resolveAccountContextPolicy(metadata); + expect(resolved.mode).toBe('shared'); + expect(resolved.group).toBe('sprint-a'); + }); }); diff --git a/tests/unit/auth-list-context.test.ts b/tests/unit/auth-list-context.test.ts index 7f24b531..b62cb798 100644 --- a/tests/unit/auth-list-context.test.ts +++ b/tests/unit/auth-list-context.test.ts @@ -85,4 +85,78 @@ describe('auth list context metadata', () => { expect(work?.context_mode).toBe('shared'); expect(work?.context_group).toBe('sprint-a'); }); + + it('prefers unified context metadata over legacy when profile names overlap', async () => { + const ccsDir = path.join(tempRoot, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + + fs.writeFileSync( + path.join(ccsDir, 'profiles.json'), + JSON.stringify( + { + version: '2.0.0', + profiles: { + work: { + type: 'account', + created: '2026-01-01T00:00:00.000Z', + last_used: null, + context_mode: 'isolated', + }, + }, + default: null, + }, + null, + 2 + ) + ); + + fs.writeFileSync( + path.join(ccsDir, 'config.yaml'), + [ + 'version: 8', + 'accounts:', + ' work:', + ' created: "2026-02-01T00:00:00.000Z"', + ' last_used: null', + ' context_mode: shared', + ' context_group: sprint-a', + 'profiles: {}', + 'cliproxy:', + ' oauth_accounts: {}', + ' providers: {}', + ' variants: {}', + ].join('\n'), + 'utf8' + ); + + const registry = new ProfileRegistry(); + const instanceMgr = new InstanceManager(); + const lines: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { + lines.push(args.map(String).join(' ')); + }; + + try { + await handleList( + { + registry, + instanceMgr, + version: 'test', + }, + ['--json'] + ); + } finally { + console.log = originalLog; + } + + const payload = JSON.parse(lines.join('\n')) as { + profiles: Array<{ name: string; context_mode?: string; context_group?: string | null }>; + }; + const work = payload.profiles.find((profile) => profile.name === 'work'); + + expect(work).toBeTruthy(); + expect(work?.context_mode).toBe('shared'); + expect(work?.context_group).toBe('sprint-a'); + }); }); diff --git a/tests/unit/config/migration-manager.test.ts b/tests/unit/config/migration-manager.test.ts index ec446d35..1e09fcaf 100644 --- a/tests/unit/config/migration-manager.test.ts +++ b/tests/unit/config/migration-manager.test.ts @@ -169,4 +169,46 @@ describe('migration-manager legacy kimi compatibility', () => { expect(unified?.accounts.personal.context_mode).toBe('isolated'); expect(unified?.accounts.personal.context_group).toBeUndefined(); }); + + it('normalizes valid legacy shared groups and drops invalid ones during migration', async () => { + fs.writeFileSync( + path.join(ccsDir, 'profiles.json'), + JSON.stringify( + { + default: 'work', + profiles: { + work: { + type: 'account', + created: '2026-02-01T00:00:00.000Z', + last_used: null, + context_mode: 'shared', + context_group: 'Sprint-A', + }, + broken: { + type: 'account', + created: '2026-02-02T00:00:00.000Z', + last_used: null, + context_mode: 'shared', + context_group: '###', + }, + }, + }, + null, + 2 + ) + ); + + const result = await migrate(false); + expect(result.success).toBe(true); + expect( + result.warnings.some((warning) => + warning.includes('Skipped invalid context group for account "broken"') + ) + ).toBe(true); + + const unified = loadUnifiedConfig(); + expect(unified?.accounts.work.context_group).toBe('sprint-a'); + expect(unified?.accounts.broken.context_mode).toBe('shared'); + expect(unified?.accounts.broken.context_group).toBeUndefined(); + }); }); diff --git a/tests/unit/web-server/account-routes-context.test.ts b/tests/unit/web-server/account-routes-context.test.ts new file mode 100644 index 00000000..0563c129 --- /dev/null +++ b/tests/unit/web-server/account-routes-context.test.ts @@ -0,0 +1,133 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'bun:test'; +import express from 'express'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { Server } from 'http'; +import accountRoutes from '../../../src/web-server/routes/account-routes'; + +async function getJson(baseUrl: string, routePath: string): Promise { + const response = await fetch(`${baseUrl}${routePath}`); + expect(response.status).toBe(200); + return (await response.json()) as T; +} + +describe('web-server account-routes context normalization', () => { + let server: Server; + let baseUrl = ''; + let tempHome = ''; + let originalCcsHome: string | undefined; + let originalCcsUnified: string | undefined; + + beforeAll(async () => { + const app = express(); + app.use('/api/accounts', accountRoutes); + + await new Promise((resolve, reject) => { + server = app.listen(0, '127.0.0.1'); + const handleError = (error: Error) => reject(error); + server.once('error', handleError); + server.once('listening', () => { + server.off('error', handleError); + 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(() => { + tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-account-routes-context-')); + originalCcsHome = process.env.CCS_HOME; + originalCcsUnified = process.env.CCS_UNIFIED_CONFIG; + + process.env.CCS_HOME = tempHome; + process.env.CCS_UNIFIED_CONFIG = '1'; + }); + + afterEach(() => { + if (originalCcsHome !== undefined) process.env.CCS_HOME = originalCcsHome; + else delete process.env.CCS_HOME; + + if (originalCcsUnified !== undefined) process.env.CCS_UNIFIED_CONFIG = originalCcsUnified; + else delete process.env.CCS_UNIFIED_CONFIG; + + if (tempHome && fs.existsSync(tempHome)) { + fs.rmSync(tempHome, { recursive: true, force: true }); + } + }); + + it('normalizes invalid persisted account context metadata in API response', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + + fs.writeFileSync( + path.join(ccsDir, 'config.yaml'), + [ + 'version: 8', + 'accounts:', + ' work:', + ' created: "2026-02-01T00:00:00.000Z"', + ' last_used: null', + ' context_mode: weird', + ' context_group: "###"', + 'profiles: {}', + 'cliproxy:', + ' oauth_accounts: {}', + ' providers: {}', + ' variants: {}', + ].join('\n'), + 'utf8' + ); + + const payload = await getJson<{ + accounts: Array<{ name: string; context_mode?: string; context_group?: string }>; + }>(baseUrl, '/api/accounts'); + + const work = payload.accounts.find((account) => account.name === 'work'); + expect(work).toBeTruthy(); + expect(work?.context_mode).toBe('isolated'); + expect(work && 'context_group' in work).toBe(false); + }); + + it('falls back shared accounts with invalid groups to default shared group', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + + fs.writeFileSync( + path.join(ccsDir, 'config.yaml'), + [ + 'version: 8', + 'accounts:', + ' work:', + ' created: "2026-02-01T00:00:00.000Z"', + ' last_used: null', + ' context_mode: shared', + ' context_group: "###"', + 'profiles: {}', + 'cliproxy:', + ' oauth_accounts: {}', + ' providers: {}', + ' variants: {}', + ].join('\n'), + 'utf8' + ); + + const payload = await getJson<{ + accounts: Array<{ name: string; context_mode?: string; context_group?: string }>; + }>(baseUrl, '/api/accounts'); + + const work = payload.accounts.find((account) => account.name === 'work'); + expect(work).toBeTruthy(); + expect(work?.context_mode).toBe('shared'); + expect(work?.context_group).toBe('default'); + }); +}); diff --git a/tests/unit/web-server/config-routes-account-context.test.ts b/tests/unit/web-server/config-routes-account-context.test.ts new file mode 100644 index 00000000..850f654f --- /dev/null +++ b/tests/unit/web-server/config-routes-account-context.test.ts @@ -0,0 +1,141 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'bun:test'; +import express from 'express'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { Server } from 'http'; +import configRoutes from '../../../src/web-server/routes/config-routes'; +import { createEmptyUnifiedConfig } from '../../../src/config/unified-config-types'; + +async function putJson(baseUrl: string, routePath: string, body: unknown): Promise { + return fetch(`${baseUrl}${routePath}`, { + method: 'PUT', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(body), + }); +} + +describe('web-server config-routes account context validation', () => { + let server: Server; + let baseUrl = ''; + let tempHome = ''; + let originalCcsHome: string | undefined; + + beforeAll(async () => { + const app = express(); + app.use(express.json()); + app.use('/api/config', configRoutes); + + await new Promise((resolve, reject) => { + server = app.listen(0, '127.0.0.1'); + const handleError = (error: Error) => reject(error); + server.once('error', handleError); + server.once('listening', () => { + server.off('error', handleError); + 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(() => { + tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-config-routes-context-')); + originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tempHome; + fs.mkdirSync(path.join(tempHome, '.ccs'), { recursive: true }); + }); + + 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('rejects invalid account context_mode values', async () => { + const response = await putJson(baseUrl, '/api/config', { + version: 8, + accounts: { + work: { + created: '2026-01-01T00:00:00.000Z', + last_used: null, + context_mode: 'weird', + }, + }, + profiles: {}, + cliproxy: { oauth_accounts: {}, providers: [], variants: {} }, + }); + + expect(response.status).toBe(400); + const payload = (await response.json()) as { error: string }; + expect(payload.error).toContain('context_mode'); + }); + + it('rejects context_group when mode is not shared', async () => { + const response = await putJson(baseUrl, '/api/config', { + version: 8, + accounts: { + work: { + created: '2026-01-01T00:00:00.000Z', + last_used: null, + context_mode: 'isolated', + context_group: 'sprint-a', + }, + }, + profiles: {}, + cliproxy: { oauth_accounts: {}, providers: [], variants: {} }, + }); + + expect(response.status).toBe(400); + const payload = (await response.json()) as { error: string }; + expect(payload.error).toContain('context_group requires context_mode=shared'); + }); + + it('rejects invalid shared context_group names', async () => { + const response = await putJson(baseUrl, '/api/config', { + version: 8, + accounts: { + work: { + created: '2026-01-01T00:00:00.000Z', + last_used: null, + context_mode: 'shared', + context_group: '###', + }, + }, + profiles: {}, + cliproxy: { oauth_accounts: {}, providers: [], variants: {} }, + }); + + expect(response.status).toBe(400); + const payload = (await response.json()) as { error: string }; + expect(payload.error).toContain('context_group'); + }); + + it('accepts valid shared context metadata', async () => { + const config = createEmptyUnifiedConfig(); + config.accounts.work = { + created: '2026-01-01T00:00:00.000Z', + last_used: null, + context_mode: 'shared', + context_group: 'Sprint-A', + }; + const response = await putJson(baseUrl, '/api/config', config); + + expect(response.status).toBe(200); + const payload = (await response.json()) as { success: boolean }; + expect(payload.success).toBe(true); + }); +}); diff --git a/ui/src/components/account/create-auth-profile-dialog.tsx b/ui/src/components/account/create-auth-profile-dialog.tsx index a82dc72f..51b7b997 100644 --- a/ui/src/components/account/create-auth-profile-dialog.tsx +++ b/ui/src/components/account/create-auth-profile-dialog.tsx @@ -22,6 +22,8 @@ interface CreateAuthProfileDialogProps { onClose: () => void; } +const MAX_CONTEXT_GROUP_LENGTH = 64; + export function CreateAuthProfileDialog({ open, onClose }: CreateAuthProfileDialogProps) { const [profileName, setProfileName] = useState(''); const [shareContext, setShareContext] = useState(false); @@ -32,7 +34,9 @@ export function CreateAuthProfileDialog({ open, onClose }: CreateAuthProfileDial const isValidName = /^[a-zA-Z][a-zA-Z0-9_-]*$/.test(profileName); const normalizedGroup = contextGroup.trim().toLowerCase(); const isValidContextGroup = - normalizedGroup.length === 0 || /^[a-zA-Z][a-zA-Z0-9_-]*$/.test(normalizedGroup); + normalizedGroup.length === 0 || + (normalizedGroup.length <= MAX_CONTEXT_GROUP_LENGTH && + /^[a-zA-Z][a-zA-Z0-9_-]*$/.test(normalizedGroup)); const command = profileName && isValidName @@ -119,7 +123,7 @@ export function CreateAuthProfileDialog({ open, onClose }: CreateAuthProfileDial {contextGroup.trim().length > 0 && !isValidContextGroup && (

Group must start with a letter and use only letters, numbers, dashes, or - underscores. + underscores (max {MAX_CONTEXT_GROUP_LENGTH} chars).

)}