fix(auth): harden shared-context isolation edge cases

This commit is contained in:
Tam Nhu Tran
2026-02-25 04:09:46 +07:00
parent 3b7395b229
commit 7d5e604e53
12 changed files with 417 additions and 21 deletions
+11 -3
View File
@@ -29,6 +29,8 @@ export interface ResolvedCreateAccountContext {
export const DEFAULT_ACCOUNT_CONTEXT_MODE: AccountContextMode = 'isolated';
export const DEFAULT_ACCOUNT_CONTEXT_GROUP = 'default';
export const MAX_CONTEXT_GROUP_LENGTH = 64;
export const ACCOUNT_PROFILE_NAME_PATTERN = /^[a-zA-Z][a-zA-Z0-9_-]*$/;
const CONTEXT_GROUP_PATTERN = /^[a-zA-Z][a-zA-Z0-9_-]*$/;
@@ -43,7 +45,14 @@ export function normalizeContextGroupName(value: string): string {
* Validate context group naming constraints.
*/
export function isValidContextGroupName(value: string): boolean {
return CONTEXT_GROUP_PATTERN.test(value);
return value.length <= MAX_CONTEXT_GROUP_LENGTH && CONTEXT_GROUP_PATTERN.test(value);
}
/**
* Validate account profile naming constraints.
*/
export function isValidAccountProfileName(value: string): boolean {
return ACCOUNT_PROFILE_NAME_PATTERN.test(value);
}
/**
@@ -84,8 +93,7 @@ export function resolveCreateAccountContext(
if (!isValidContextGroupName(normalizedGroup)) {
return {
policy: { mode: 'isolated' },
error:
'Invalid context group. Use letters/numbers/dash/underscore and start with a letter.',
error: `Invalid context group. Use letters/numbers/dash/underscore, start with a letter, max ${MAX_CONTEXT_GROUP_LENGTH} chars.`,
};
}
+96 -3
View File
@@ -9,21 +9,27 @@ import { initUI, header, color, fail, warn, info, infoBox, warnBox } from '../..
import { getClaudeCliInfo } from '../../utils/claude-detector';
import { escapeShellArg, stripClaudeCodeEnv } from '../../utils/shell-executor';
import { isUnifiedMode } from '../../config/unified-config-loader';
import { ProfileMetadata } from '../../types';
import {
resolveCreateAccountContext,
policyToAccountContextMetadata,
formatAccountContextPolicy,
isValidAccountProfileName,
} from '../account-context';
import { exitWithError } from '../../errors';
import { ExitCode } from '../../errors/exit-codes';
import { CommandContext, parseArgs } from './types';
function sanitizeProfileNameForInstance(name: string): string {
return name.replace(/[^a-zA-Z0-9_-]/g, '-').toLowerCase();
}
/**
* Handle the create command
*/
export async function handleCreate(ctx: CommandContext, args: string[]): Promise<void> {
await initUI();
const { profileName, force, shareContext, contextGroup } = parseArgs(args);
const { profileName, force, shareContext, contextGroup, unknownFlags } = parseArgs(args);
if (!profileName) {
console.log(fail('Profile name is required'));
@@ -37,6 +43,21 @@ 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.';
console.log(fail(error));
console.log('');
exitWithError(error, ExitCode.PROFILE_ERROR);
}
// Check if profile already exists (check both legacy and unified)
const existsLegacy = ctx.registry.hasProfile(profileName);
const existsUnified = ctx.registry.hasAccountUnified(profileName);
@@ -46,6 +67,18 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise
exitWithError(`Profile already exists: ${profileName}`, ExitCode.PROFILE_ERROR);
}
const normalizedName = sanitizeProfileNameForInstance(profileName);
const collidingName = Object.keys(ctx.registry.getAllProfilesMerged()).find(
(name) => name !== profileName && sanitizeProfileNameForInstance(name) === normalizedName
);
if (collidingName) {
const error = `Profile "${profileName}" conflicts with existing profile "${collidingName}" on filesystem.`;
console.log(fail(error));
console.log('');
exitWithError(error, ExitCode.PROFILE_ERROR);
}
const resolvedContext = resolveCreateAccountContext({
shareContext: !!shareContext,
contextGroup,
@@ -59,14 +92,47 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise
const contextPolicy = resolvedContext.policy;
const contextMetadata = policyToAccountContextMetadata(contextPolicy);
const useUnifiedConfig = isUnifiedMode();
const createdProfile = useUnifiedConfig ? !existsUnified : !existsLegacy;
const previousLegacyProfile: ProfileMetadata | undefined =
!useUnifiedConfig && existsLegacy ? ctx.registry.getProfile(profileName) : undefined;
const previousUnifiedProfile =
useUnifiedConfig && existsUnified
? ctx.registry.getAllAccountsUnified()[profileName]
: undefined;
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);
// Create/update profile entry based on config mode
if (isUnifiedMode()) {
if (useUnifiedConfig) {
// Use unified config (config.yaml)
if (existsUnified) {
ctx.registry.updateAccountUnified(profileName, {
@@ -96,7 +162,11 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise
console.log(info(`Instance directory: ${instancePath}`));
console.log('');
console.log(warn('Starting Claude in isolated instance...'));
const launchDescription =
contextPolicy.mode === 'shared'
? `Starting Claude with shared context group "${contextPolicy.group || 'default'}"...`
: 'Starting Claude in isolated instance...';
console.log(warn(launchDescription));
console.log(warn('You will be prompted to login with your account.'));
console.log('');
@@ -112,6 +182,20 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise
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];
}
}
// 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
@@ -162,6 +246,11 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise
console.log('');
process.exit(0);
} else {
rollbackMetadata();
if (createdProfile) {
ctx.instanceMgr.deleteInstance(profileName);
}
console.log('');
console.log(fail('Login failed or cancelled'));
console.log('');
@@ -173,6 +262,10 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise
});
child.on('error', (err: Error) => {
rollbackMetadata();
if (createdProfile) {
ctx.instanceMgr.deleteInstance(profileName);
}
exitWithError(`Failed to execute Claude CLI: ${err.message}`, ExitCode.BINARY_ERROR);
});
} catch (error) {
+2
View File
@@ -30,6 +30,8 @@ export async function handleList(ctx: CommandContext, args: string[]): Promise<v
type: 'account',
created: account.created,
last_used: account.last_used,
context_mode: account.context_mode,
context_group: account.context_group,
};
}
+24
View File
@@ -21,6 +21,7 @@ export interface AuthCommandArgs {
yes?: boolean;
shareContext?: boolean;
contextGroup?: string;
unknownFlags?: string[];
}
/**
@@ -61,6 +62,16 @@ export interface CommandContext {
export function parseArgs(args: string[]): AuthCommandArgs {
let profileName: string | undefined;
let contextGroup: string | undefined;
const unknownFlags = new Set<string>();
const knownBooleanFlags = new Set([
'--force',
'--verbose',
'--json',
'--yes',
'-y',
'--share-context',
]);
const knownValueFlags = new Set(['--context-group']);
for (let i = 0; i < args.length; i++) {
const arg = args[i];
@@ -83,6 +94,18 @@ export function parseArgs(args: string[]): AuthCommandArgs {
}
if (arg.startsWith('-')) {
const normalizedFlag = arg.includes('=') ? arg.slice(0, arg.indexOf('=')) : arg;
const isKnownFlag =
knownBooleanFlags.has(normalizedFlag) || knownValueFlags.has(normalizedFlag);
if (!isKnownFlag) {
unknownFlags.add(normalizedFlag);
// Best effort: unknown flags often take a value token.
// Skip one following non-flag token to avoid mis-parsing profile name.
const next = args[i + 1];
if (!arg.includes('=') && next && !next.startsWith('-')) {
i++;
}
}
continue;
}
@@ -99,5 +122,6 @@ export function parseArgs(args: string[]): AuthCommandArgs {
yes: args.includes('--yes') || args.includes('-y'),
shareContext: args.includes('--share-context'),
contextGroup,
unknownFlags: [...unknownFlags],
};
}
+9
View File
@@ -148,9 +148,18 @@ export async function migrate(dryRun = false): Promise<MigrationResult> {
if (oldProfiles?.profiles) {
for (const [name, meta] of Object.entries(oldProfiles.profiles)) {
const metadata = meta as Record<string, unknown>;
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;
const account: AccountConfig = {
created: (metadata.created as string) || new Date().toISOString(),
last_used: (metadata.last_used as string) || null,
context_mode: contextMode,
context_group: contextMode === 'shared' ? contextGroup : undefined,
};
unifiedConfig.accounts[name] = account;
}
+71 -8
View File
@@ -17,10 +17,12 @@ import { getCcsDir } from '../utils/config-manager';
*/
class InstanceManager {
private readonly instancesDir: string;
private readonly locksDir: string;
private readonly sharedManager: SharedManager;
constructor() {
this.instancesDir = path.join(getCcsDir(), 'instances');
this.locksDir = path.join(this.instancesDir, '.locks');
this.sharedManager = new SharedManager();
}
@@ -33,16 +35,19 @@ class InstanceManager {
): Promise<string> {
const instancePath = this.getInstancePath(profileName);
// Lazy initialization
if (!fs.existsSync(instancePath)) {
this.initializeInstance(profileName, instancePath);
}
// Serialize context sync operations per profile across processes.
await this.withContextSyncLock(profileName, async () => {
// Lazy initialization
if (!fs.existsSync(instancePath)) {
this.initializeInstance(profileName, instancePath);
}
// Validate structure (auto-fix missing dirs)
this.validateInstance(instancePath);
// Validate structure (auto-fix missing dirs)
this.validateInstance(instancePath);
// Apply context policy (isolated by default, optional shared group).
await this.sharedManager.syncProjectContext(instancePath, contextPolicy);
// Apply context policy (isolated by default, optional shared group).
await this.sharedManager.syncProjectContext(instancePath, contextPolicy);
});
return instancePath;
}
@@ -195,6 +200,64 @@ class InstanceManager {
// Replace unsafe characters with dash
return name.replace(/[^a-zA-Z0-9_-]/g, '-').toLowerCase();
}
private getContextSyncLockPath(profileName: string): string {
const safeName = this.sanitizeName(profileName);
return path.join(this.locksDir, `${safeName}.lock`);
}
private async withContextSyncLock<T>(
profileName: string,
callback: () => Promise<T>
): Promise<T> {
const lockPath = this.getContextSyncLockPath(profileName);
const retryDelayMs = 50;
const timeoutMs = 5000;
const staleLockMs = 30000;
const start = Date.now();
fs.mkdirSync(this.locksDir, { recursive: true, mode: 0o700 });
while (true) {
try {
const fd = fs.openSync(lockPath, 'wx', 0o600);
fs.writeFileSync(fd, `${process.pid}`);
fs.closeSync(fd);
break;
} catch (error) {
const err = error as NodeJS.ErrnoException;
if (err.code !== 'EEXIST') {
throw error;
}
try {
const lockStats = fs.statSync(lockPath);
if (Date.now() - lockStats.mtimeMs > staleLockMs) {
fs.unlinkSync(lockPath);
continue;
}
} catch {
// Best-effort stale lock cleanup.
}
if (Date.now() - start > timeoutMs) {
throw new Error(`Timed out waiting for profile context lock: ${profileName}`);
}
await new Promise((resolve) => setTimeout(resolve, retryDelayMs));
}
}
try {
return await callback();
} finally {
try {
fs.unlinkSync(lockPath);
} catch {
// Best-effort cleanup.
}
}
}
}
export { InstanceManager };
+17 -6
View File
@@ -7,6 +7,7 @@
import { Router, Request, Response } from 'express';
import ProfileRegistry from '../../auth/profile-registry';
import InstanceManager from '../../management/instance-manager';
import { isUnifiedMode } from '../../config/unified-config-loader';
import {
getAllAccountsSummary,
@@ -21,19 +22,25 @@ import { isCLIProxyProvider } from '../../cliproxy/provider-capabilities';
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 colonIndex = key.indexOf(':');
const normalizedKey = key.startsWith('cliproxy:') ? key.slice('cliproxy:'.length) : key;
const colonIndex = normalizedKey.indexOf(':');
if (colonIndex === -1) return null;
const provider = key.slice(0, colonIndex);
const accountId = key.slice(colonIndex + 1);
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);
}
/**
* GET /api/accounts - List accounts from both profiles.json and config.yaml
*/
@@ -91,7 +98,8 @@ 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 key = `${provider}:${acct.id}`;
const rawKey = `${provider}:${acct.id}`;
const key = merged[rawKey] ? `cliproxy:${rawKey}` : rawKey;
merged[key] = {
type: 'cliproxy',
provider,
@@ -130,7 +138,7 @@ router.post('/default', (req: Request, res: Response): void => {
}
// Check if this is a CLIProxy account (format: "provider:accountId")
const cliproxyKey = parseCliproxyKey(name);
const cliproxyKey = !hasAuthAccount(name) ? parseCliproxyKey(name) : null;
if (cliproxyKey) {
const success = setCliproxyDefault(cliproxyKey.provider, cliproxyKey.accountId);
if (!success) {
@@ -192,7 +200,7 @@ router.delete('/:name', (req: Request, res: Response): void => {
}
// Check if this is a CLIProxy account (format: "provider:accountId")
const cliproxyKey = parseCliproxyKey(name);
const cliproxyKey = !hasAuthAccount(name) ? parseCliproxyKey(name) : null;
if (cliproxyKey) {
const success = removeCliproxyAccount(cliproxyKey.provider, cliproxyKey.accountId);
if (!success) {
@@ -219,6 +227,9 @@ router.delete('/:name', (req: Request, res: Response): void => {
return;
}
// Keep API delete behavior aligned with CLI remove command.
instanceMgr.deleteInstance(name);
res.json({ success: true, deleted: name });
} catch (error) {
res.status(500).json({ error: (error as Error).message });
+31
View File
@@ -0,0 +1,31 @@
import { describe, expect, it } from 'bun:test';
import {
MAX_CONTEXT_GROUP_LENGTH,
isValidAccountProfileName,
resolveAccountContextPolicy,
resolveCreateAccountContext,
} from '../../src/auth/account-context';
describe('account context helpers', () => {
it('rejects context groups that exceed the max length', () => {
const group = `a${'x'.repeat(MAX_CONTEXT_GROUP_LENGTH)}`;
const result = resolveCreateAccountContext({ shareContext: false, contextGroup: group });
expect(result.error).toContain('Invalid context group');
});
it('rejects profile names with unsupported characters', () => {
expect(isValidAccountProfileName('work')).toBe(true);
expect(isValidAccountProfileName('gemini:default')).toBe(false);
});
it('falls back to default shared group for invalid persisted metadata', () => {
const resolved = resolveAccountContextPolicy({
context_mode: 'shared',
context_group: '###',
});
expect(resolved.mode).toBe('shared');
expect(resolved.group).toBe('default');
});
});
+14
View File
@@ -31,4 +31,18 @@ describe('auth command args parsing', () => {
expect(parsed.profileName).toBe('work');
expect(parsed.contextGroup).toBe('');
});
it('flags empty inline context group as empty string', () => {
const parsed = parseArgs(['work', '--context-group=']);
expect(parsed.profileName).toBe('work');
expect(parsed.contextGroup).toBe('');
});
it('tracks unknown flags and keeps positional profile intact', () => {
const parsed = parseArgs(['--foo', 'bar', 'work']);
expect(parsed.profileName).toBe('work');
expect(parsed.unknownFlags).toEqual(['--foo']);
});
});
+88
View File
@@ -0,0 +1,88 @@
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 ProfileRegistry from '../../src/auth/profile-registry';
import InstanceManager from '../../src/management/instance-manager';
import { handleList } from '../../src/auth/commands/list-command';
describe('auth list context metadata', () => {
let tempRoot = '';
let originalCcsHome: string | undefined;
let originalCcsUnified: string | undefined;
beforeEach(() => {
tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-auth-list-context-'));
originalCcsHome = process.env.CCS_HOME;
originalCcsUnified = process.env.CCS_UNIFIED_CONFIG;
process.env.CCS_HOME = tempRoot;
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 (tempRoot && fs.existsSync(tempRoot)) {
fs.rmSync(tempRoot, { recursive: true, force: true });
}
});
it('keeps unified account context metadata in JSON list output', async () => {
const ccsDir = path.join(tempRoot, '.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: 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');
});
});
+38 -1
View File
@@ -3,7 +3,7 @@ import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { loadMigrationCheckData, migrate } from '../../../src/config/migration-manager';
import { saveUnifiedConfig } from '../../../src/config/unified-config-loader';
import { loadUnifiedConfig, saveUnifiedConfig } from '../../../src/config/unified-config-loader';
import { createEmptyUnifiedConfig } from '../../../src/config/unified-config-types';
describe('migration-manager legacy kimi compatibility', () => {
@@ -132,4 +132,41 @@ describe('migration-manager legacy kimi compatibility', () => {
const checkData = loadMigrationCheckData();
expect(checkData.needsMigration).toBe(false);
});
it('migrates account context metadata from profiles.json', 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',
},
personal: {
type: 'account',
created: '2026-02-02T00:00:00.000Z',
last_used: null,
},
},
},
null,
2
)
);
const result = await migrate(false);
expect(result.success).toBe(true);
const unified = loadUnifiedConfig();
expect(unified).toBeTruthy();
expect(unified?.accounts.work.context_mode).toBe('shared');
expect(unified?.accounts.work.context_group).toBe('sprint-a');
expect(unified?.accounts.personal.context_mode).toBe('isolated');
expect(unified?.accounts.personal.context_group).toBeUndefined();
});
});
+16
View File
@@ -3,6 +3,7 @@ import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import SharedManager from '../../src/management/shared-manager';
import InstanceManager from '../../src/management/instance-manager';
import type { AccountContextPolicy } from '../../src/auth/account-context';
function getTestCcsDir(): string {
@@ -118,4 +119,19 @@ describe('SharedManager context policy', () => {
expect(fs.existsSync(projectFile)).toBe(true);
expect(fs.readFileSync(projectFile, 'utf8')).toBe('shared history');
});
it('serializes concurrent context sync for the same profile', async () => {
const instanceMgr = new InstanceManager();
const jobs = Array.from({ length: 6 }, () =>
instanceMgr.ensureInstance('work', { mode: 'shared', group: 'sprint-a' })
);
await Promise.all(jobs);
const ccsDir = getTestCcsDir();
const projectsPath = path.join(ccsDir, 'instances', 'work', 'projects');
const stats = fs.lstatSync(projectsPath);
expect(stats.isDirectory() || stats.isSymbolicLink()).toBe(true);
});
});