diff --git a/README.md b/README.md index 9c173863..a50d7a6f 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ Want to run the dashboard in Docker? See `docker/README.md`. The dashboard provides visual management for all account types: -- **Claude Accounts**: Create isolated instances (work, personal, client) +- **Claude Accounts**: Isolation-first by default (work, personal, client), with explicit shared context opt-in - **OAuth Providers**: One-click auth for Gemini, Codex, Antigravity, Kiro, Copilot - **API Profiles**: Configure GLM, Kimi with your keys - **Health Monitor**: Real-time status across all profiles @@ -221,17 +221,45 @@ ccs work "implement feature" # Terminal 1 ccs "review code" # Terminal 2 (personal account) ``` -Need continuity between two accounts for the same project? Opt in to shared context: +#### Account Context Modes (Isolation-First) + +Account profiles are isolated by default. + +| Mode | Default | Requirements | +|------|---------|--------------| +| `isolated` | Yes | No `context_group` required | +| `shared` | No (explicit opt-in) | Valid non-empty `context_group` | + +Opt in to shared context when needed: ```bash # Share context with default group ccs auth create backup --share-context -# Or isolate by named group (only accounts in this group share context) +# Share context only within named group ccs auth create backup2 --context-group sprint-a ``` -Isolation remains the default. Shared context only links project workspace data; credentials stay per-account. +Shared mode metadata in `~/.ccs/config.yaml`: + +```yaml +accounts: + work: + created: "2026-02-24T00:00:00.000Z" + last_used: null + context_mode: "shared" + context_group: "team-alpha" +``` + +`context_group` rules: + +- lowercase letters, numbers, `_`, `-` +- must start with a letter +- max length `64` +- non-empty after normalization +- normalized by trim + lowercase + whitespace collapse (`" Team Alpha "` -> `"team-alpha"`) + +Shared context links project workspace data only. Credentials remain isolated per account.
diff --git a/docs/codebase-summary.md b/docs/codebase-summary.md index 972b56c6..d138f34b 100644 --- a/docs/codebase-summary.md +++ b/docs/codebase-summary.md @@ -1,8 +1,8 @@ # CCS Codebase Summary -Last Updated: 2026-02-04 +Last Updated: 2026-02-24 -Comprehensive overview of the modularized CCS codebase structure following the Phase 9 modularization effort (Settings, Analytics, Auth Monitor splits + Test Infrastructure), v7.1 Remote CLIProxy feature, v7.2 Kiro + GitHub Copilot (ghcp) OAuth providers, v7.14 Hybrid Quota Management, and v7.34 Image Analysis Hook. +Comprehensive overview of the modularized CCS codebase structure following the Phase 9 modularization effort (Settings, Analytics, Auth Monitor splits + Test Infrastructure), v7.1 Remote CLIProxy feature, v7.2 Kiro + GitHub Copilot (ghcp) OAuth providers, v7.14 Hybrid Quota Management, v7.34 Image Analysis Hook, and account-context validation hardening. ## Repository Structure @@ -201,6 +201,19 @@ src/ | Services | `web-server/`, `api/` | HTTP server, API services | | Utilities | `utils/`, `management/` | Helpers, diagnostics | +### Account Context Metadata Flow + +- Source fields: `accounts..context_mode` and `accounts..context_group` in `~/.ccs/config.yaml`. +- Runtime policy resolver: `src/auth/account-context.ts`. +- Metadata storage normalization: `src/auth/profile-registry.ts`. +- API write validation: `PUT /api/config` in `src/web-server/routes/config-routes.ts`. +- Rules: + - mode is isolation-first (`isolated` default, `shared` opt-in) + - shared mode requires non-empty valid `context_group` + - `context_group` is normalized (trim + lowercase + whitespace collapse to `-`) + - API route rejects `context_group` when mode is not `shared` + - registry normalization drops malformed persisted `context_group` values + ### Target Adapter Module The targets module provides an extensible interface for dispatching profiles to different CLI implementations. diff --git a/docs/dashboard-auth-cli.md b/docs/dashboard-auth-cli.md index 60c442e4..85d1a7ed 100644 --- a/docs/dashboard-auth-cli.md +++ b/docs/dashboard-auth-cli.md @@ -1,6 +1,6 @@ # Dashboard Authentication CLI -Last Updated: 2026-02-04 +Last Updated: 2026-02-24 CLI commands for managing CCS dashboard authentication. @@ -10,6 +10,35 @@ 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. +## Account Context Modes (Related Feature) + +Dashboard auth and account context metadata are separate: + +- `dashboard_auth`: protects dashboard access with username/password +- `accounts..context_mode/context_group`: controls isolated vs shared account context + +Account context is isolation-first: + +| Mode | Default | Requirement | +|------|---------|-------------| +| `isolated` | Yes | No `context_group` required | +| `shared` | No (opt-in) | Valid non-empty `context_group` | + +`context_group` normalization and validation: + +- trim + lowercase + collapse internal whitespace to `-` +- allowed characters: lowercase letters, numbers, `_`, `-` +- must start with a letter +- max length: 64 +- shared mode requires non-empty value after normalization + +`PUT /api/config` behavior for account context: + +- rejects invalid unified payloads +- rejects explicit `context_mode: shared` with invalid/empty `context_group` +- normalizes valid shared `context_group` before save +- rejects `context_group` when mode is not `shared` + ## Commands ### `ccs config auth setup` @@ -162,6 +191,10 @@ export CCS_DASHBOARD_PASSWORD_HASH='$2b$10$...' Check `session_timeout_hours` in config. Default is 24 hours. +### "Invalid ... context_group ..." + +This error comes from `PUT /api/config` when an account explicitly sets shared mode with an invalid group. Use a canonical group value (for example: `team-alpha`). + ## See Also - [Dashboard Auth Feature](https://ccs.kaitran.ca/features/dashboard-auth) - Full documentation diff --git a/src/auth/account-context.ts b/src/auth/account-context.ts index f97a89b5..c30a0f7c 100644 --- a/src/auth/account-context.ts +++ b/src/auth/account-context.ts @@ -38,7 +38,7 @@ const CONTEXT_GROUP_PATTERN = /^[a-zA-Z][a-zA-Z0-9_-]*$/; * Normalize context group names so paths and config stay consistent. */ export function normalizeContextGroupName(value: string): string { - return value.trim().toLowerCase(); + return value.trim().toLowerCase().replace(/\s+/g, '-'); } /** diff --git a/src/auth/auth-commands.ts b/src/auth/auth-commands.ts index e94c47f8..2ed849ee 100644 --- a/src/auth/auth-commands.ts +++ b/src/auth/auth-commands.ts @@ -14,6 +14,7 @@ import ProfileRegistry from './profile-registry'; import { InstanceManager } from '../management/instance-manager'; import { initUI, header, subheader, color, dim, warn, fail } from '../utils/ui'; +import { MAX_CONTEXT_GROUP_LENGTH } from './account-context'; import packageJson from '../../package.json'; // Import command handlers from modular structure @@ -127,6 +128,10 @@ class AuthCommands { console.log( ` Account profiles stay isolated unless you opt in with ${color('--share-context', 'command')}.` ); + console.log(` Shared context groups are normalized (trim + lowercase) and spaces become "-".`); + console.log( + ` ${color('context_group', 'path')} must be non-empty and <= ${MAX_CONTEXT_GROUP_LENGTH} chars in shared mode.` + ); console.log(''); } diff --git a/src/auth/commands/create-command-env.ts b/src/auth/commands/create-command-env.ts new file mode 100644 index 00000000..630e9a0e --- /dev/null +++ b/src/auth/commands/create-command-env.ts @@ -0,0 +1,57 @@ +const AMBIENT_PROVIDER_PREFIXES = [ + 'ANTHROPIC_', + 'OPENAI_', + 'GOOGLE_', + 'GEMINI_', + 'MINIMAX_', + 'QWEN_', + 'DEEPSEEK_', + 'KIMI_', + 'AZURE_', + 'OLLAMA_', + 'OPENROUTER_', + 'XAI_', + 'MISTRAL_', + 'COHERE_', + 'PERPLEXITY_', + 'TOGETHER_', + 'FIREWORKS_', +]; +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', + '_API_TOKEN', + '_BEARER_TOKEN', + '_SESSION_TOKEN', +]; + +export function stripAmbientProviderCredentials(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const sanitized: NodeJS.ProcessEnv = { ...env }; + + for (const envKey of Object.keys(sanitized)) { + const normalizedKey = envKey.toUpperCase(); + + if (normalizedKey === 'CLAUDE_CONFIG_DIR') { + continue; + } + + if ( + AMBIENT_PROVIDER_PREFIXES.some((prefix) => normalizedKey.startsWith(prefix)) || + AMBIENT_PROVIDER_EXACT_KEYS.has(normalizedKey) || + AMBIENT_PROVIDER_SUFFIXES.some((suffix) => normalizedKey.endsWith(suffix)) + ) { + delete sanitized[envKey]; + } + } + + return sanitized; +} diff --git a/src/auth/commands/create-command.ts b/src/auth/commands/create-command.ts index 614a0ec8..8c169c1d 100644 --- a/src/auth/commands/create-command.ts +++ b/src/auth/commands/create-command.ts @@ -20,52 +20,12 @@ import { import { exitWithError } from '../../errors'; import { ExitCode } from '../../errors/exit-codes'; import { CommandContext, parseArgs } from './types'; +import { stripAmbientProviderCredentials } from './create-command-env'; 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 */ @@ -74,9 +34,14 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise const { profileName, force, shareContext, contextGroup, unknownFlags } = parseArgs(args); if (unknownFlags && unknownFlags.length > 0) { - const unknownList = unknownFlags.join(', '); + const unknownList = unknownFlags.map((flag) => `"${flag}"`).join(', '); console.log(fail(`Unknown option(s): ${unknownList}`)); console.log(''); + console.log( + `Usage: ${color('ccs auth create [--force] [--share-context] [--context-group ]', 'command')}` + ); + console.log(`Help: ${color('ccs auth --help', 'command')}`); + console.log(''); exitWithError(`Unknown option(s): ${unknownList}`, ExitCode.PROFILE_ERROR); } @@ -135,15 +100,17 @@ 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; + const profileExistedBeforeCreate = existsLegacy || existsUnified; + const createdUnifiedProfile = useUnifiedConfig && !existsUnified; + const createdLegacyProfile = !useUnifiedConfig && !existsLegacy; + const previousLegacyProfile: ProfileMetadata | undefined = existsLegacy + ? ctx.registry.getProfile(profileName) + : undefined; + const previousUnifiedProfile = existsUnified + ? ctx.registry.getAllAccountsUnified()[profileName] + : undefined; const previousContextPolicy = - !createdProfile && (previousUnifiedProfile || previousLegacyProfile) + profileExistedBeforeCreate && (previousUnifiedProfile || previousLegacyProfile) ? resolveAccountContextPolicy(previousUnifiedProfile || previousLegacyProfile) : undefined; @@ -160,22 +127,21 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise const rollbackMetadata = (): void => { try { if (useUnifiedConfig) { - if (createdProfile) { + if (createdUnifiedProfile) { 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 (createdLegacyProfile) { + if (ctx.registry.hasProfile(profileName)) { + ctx.registry.deleteProfile(profileName); + } + } else if (previousLegacyProfile) { + ctx.registry.updateProfile(profileName, previousLegacyProfile); } - } else if (previousLegacyProfile) { - ctx.registry.updateProfile(profileName, previousLegacyProfile); } } catch { // Best-effort rollback to avoid leaving stale accounts after failed login. @@ -190,7 +156,7 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise rollbackMetadata(); - if (createdProfile) { + if (!profileExistedBeforeCreate) { try { ctx.instanceMgr.deleteInstance(profileName); } catch { diff --git a/src/auth/profile-registry.ts b/src/auth/profile-registry.ts index 5870db61..f70f7e79 100644 --- a/src/auth/profile-registry.ts +++ b/src/auth/profile-registry.ts @@ -8,6 +8,7 @@ import { } from '../config/unified-config-loader'; import type { AccountConfig } from '../config/unified-config-types'; import { getCcsDir } from '../utils/config-manager'; +import { isValidContextGroupName, normalizeContextGroupName } from './account-context'; /** * Profile Registry (Simplified) @@ -51,13 +52,31 @@ export class ProfileRegistry { this.profilesPath = path.join(getCcsDir(), 'profiles.json'); } + private normalizeContextGroupValue(value: unknown): string | undefined { + if (typeof value !== 'string') { + return undefined; + } + + const normalized = normalizeContextGroupName(value); + if (normalized.length === 0 || !isValidContextGroupName(normalized)) { + return undefined; + } + + return normalized; + } + private normalizeLegacyProfileMetadata(metadata: ProfileMetadata): ProfileMetadata { const normalized: ProfileMetadata = { ...metadata }; if (normalized.context_mode !== 'shared') { delete normalized.context_group; - } else if (!normalized.context_group || normalized.context_group.trim().length === 0) { - delete normalized.context_group; + } else { + const normalizedGroup = this.normalizeContextGroupValue(normalized.context_group); + if (normalizedGroup) { + normalized.context_group = normalizedGroup; + } else { + delete normalized.context_group; + } } return normalized; @@ -68,8 +87,13 @@ export class ProfileRegistry { if (normalized.context_mode !== 'shared') { delete normalized.context_group; - } else if (!normalized.context_group || normalized.context_group.trim().length === 0) { - delete normalized.context_group; + } else { + const normalizedGroup = this.normalizeContextGroupValue(normalized.context_group); + if (normalizedGroup) { + normalized.context_group = normalizedGroup; + } else { + delete normalized.context_group; + } } return normalized; diff --git a/src/management/instance-manager.ts b/src/management/instance-manager.ts index e39acbb1..c0142f06 100644 --- a/src/management/instance-manager.ts +++ b/src/management/instance-manager.ts @@ -8,8 +8,8 @@ import * as fs from 'fs'; import * as path from 'path'; -import { createHash } from 'crypto'; import SharedManager from './shared-manager'; +import ProfileContextSyncLock from './profile-context-sync-lock'; import { AccountContextPolicy, DEFAULT_ACCOUNT_CONTEXT_MODE } from '../auth/account-context'; import { getCcsDir } from '../utils/config-manager'; @@ -18,13 +18,13 @@ import { getCcsDir } from '../utils/config-manager'; */ class InstanceManager { private readonly instancesDir: string; - private readonly locksDir: string; private readonly sharedManager: SharedManager; + private readonly contextSyncLock: ProfileContextSyncLock; constructor() { this.instancesDir = path.join(getCcsDir(), 'instances'); - this.locksDir = path.join(this.instancesDir, '.locks'); this.sharedManager = new SharedManager(); + this.contextSyncLock = new ProfileContextSyncLock(this.instancesDir); } /** @@ -37,7 +37,7 @@ class InstanceManager { const instancePath = this.getInstancePath(profileName); // Serialize context sync operations per profile across processes. - await this.withContextSyncLock(profileName, async () => { + await this.contextSyncLock.withLock(profileName, async () => { // Lazy initialization if (!fs.existsSync(instancePath)) { this.initializeInstance(profileName, instancePath); @@ -201,100 +201,6 @@ 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); - // 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( - profileName: string, - callback: () => Promise - ): Promise { - const lockPath = this.getContextSyncLockPath(profileName); - const retryDelayMs = 50; - const staleLockMs = 30000; - const timeoutMs = staleLockMs + 5000; - 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; - } - - if (this.tryRemoveDeadOwnerLock(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 }; diff --git a/src/management/profile-context-sync-lock.ts b/src/management/profile-context-sync-lock.ts new file mode 100644 index 00000000..c8158bc3 --- /dev/null +++ b/src/management/profile-context-sync-lock.ts @@ -0,0 +1,177 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import { createHash } from 'crypto'; + +interface ContextSyncLockPayload { + version: 1; + pid: number; + nonce: string; + acquiredAtMs: number; +} + +interface ContextSyncLockSnapshot { + raw: string; + owner: { pid: number; nonce?: string } | null; +} + +class ProfileContextSyncLock { + private readonly locksDir: string; + + constructor(instancesDir: string) { + this.locksDir = path.join(instancesDir, '.locks'); + } + + private sanitizeName(name: string): string { + return name.replace(/[^a-zA-Z0-9_-]/g, '-').toLowerCase(); + } + + private getLockPath(profileName: string): string { + const safeName = this.sanitizeName(profileName); + 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 parseContextSyncLock(raw: string): { pid: number; nonce?: string } | null { + const trimmed = raw.trim(); + if (trimmed.length === 0) { + return null; + } + + try { + const parsed = JSON.parse(trimmed) as Partial; + if (typeof parsed.pid === 'number' && Number.isInteger(parsed.pid) && parsed.pid > 0) { + const nonce = + typeof parsed.nonce === 'string' && parsed.nonce.length > 0 ? parsed.nonce : undefined; + return { pid: parsed.pid, nonce }; + } + } catch { + const legacyPid = Number.parseInt(trimmed, 10); + if (Number.isInteger(legacyPid) && legacyPid > 0) { + return { pid: legacyPid }; + } + } + + return null; + } + + private readContextSyncLockSnapshot(lockPath: string): ContextSyncLockSnapshot | null { + try { + const raw = fs.readFileSync(lockPath, 'utf8'); + return { + raw, + owner: this.parseContextSyncLock(raw), + }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return null; + } + return null; + } + } + + private tryRemoveLockIfUnchanged(lockPath: string, expectedRaw: string): boolean { + try { + const currentRaw = fs.readFileSync(lockPath, 'utf8'); + if (currentRaw !== expectedRaw) { + return false; + } + fs.unlinkSync(lockPath); + return true; + } catch { + return false; + } + } + + private tryRemoveDeadOwnerLock(lockPath: string, snapshot: ContextSyncLockSnapshot): boolean { + if (!snapshot.owner || this.isProcessAlive(snapshot.owner.pid)) { + return false; + } + + return this.tryRemoveLockIfUnchanged(lockPath, snapshot.raw); + } + + async withLock(profileName: string, callback: () => Promise): Promise { + const lockPath = this.getLockPath(profileName); + const retryDelayMs = 50; + const staleLockMs = 30000; + const timeoutMs = staleLockMs + 5000; + const start = Date.now(); + const ownerPayload: ContextSyncLockPayload = { + version: 1, + pid: process.pid, + nonce: createHash('sha1') + .update(`${process.pid}:${Date.now()}:${Math.random()}`) + .digest('hex') + .slice(0, 16), + acquiredAtMs: Date.now(), + }; + const ownerPayloadRaw = JSON.stringify(ownerPayload); + + fs.mkdirSync(this.locksDir, { recursive: true, mode: 0o700 }); + + while (true) { + try { + const fd = fs.openSync(lockPath, 'wx', 0o600); + fs.writeFileSync(fd, ownerPayloadRaw, 'utf8'); + fs.closeSync(fd); + break; + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code !== 'EEXIST') { + throw error; + } + + const lockSnapshot = this.readContextSyncLockSnapshot(lockPath); + if (lockSnapshot) { + if (this.tryRemoveDeadOwnerLock(lockPath, lockSnapshot)) { + continue; + } + + // For malformed lock payloads, fall back to age-based stale cleanup. + if (!lockSnapshot.owner) { + try { + const lockStats = fs.statSync(lockPath); + if (Date.now() - lockStats.mtimeMs > staleLockMs) { + if (this.tryRemoveLockIfUnchanged(lockPath, lockSnapshot.raw)) { + 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 { + this.tryRemoveLockIfUnchanged(lockPath, ownerPayloadRaw); + } + } +} + +export default ProfileContextSyncLock; diff --git a/src/management/shared-manager.ts b/src/management/shared-manager.ts index e2ae5ad8..4fc0e394 100644 --- a/src/management/shared-manager.ts +++ b/src/management/shared-manager.ts @@ -63,8 +63,14 @@ class SharedManager { const resolvedTarget = path.resolve(path.dirname(target), targetLink); // Check if target points back to our shared dir or link path - const sharedDir = path.join(getCcsDir(), 'shared'); - if (resolvedTarget.startsWith(sharedDir) || resolvedTarget === linkPath) { + const sharedDir = this.resolveCanonicalPath(path.join(getCcsDir(), 'shared')); + const canonicalResolvedTarget = this.resolveCanonicalPath(resolvedTarget); + const canonicalLinkPath = this.resolveCanonicalPath(linkPath); + + if ( + this.isPathWithinDirectory(canonicalResolvedTarget, sharedDir) || + canonicalResolvedTarget === canonicalLinkPath + ) { console.log(warn(`Circular symlink detected: ${target} → ${resolvedTarget}`)); return true; } @@ -692,17 +698,17 @@ 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( + const resolvedSource = this.resolveCanonicalPath(sourcePath); + const sharedContextRoot = this.resolveCanonicalPath( + path.join(this.sharedDir, 'context-groups') + ); + const instanceProjectsRoot = this.resolveCanonicalPath( 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) + this.isPathWithinDirectory(resolvedSource, sharedContextRoot) || + this.isPathWithinDirectory(resolvedSource, instanceProjectsRoot) ); } @@ -736,7 +742,7 @@ class SharedManager { projectsPath: string, instanceName: string ): Promise { - const sharedMemoryRoot = path.resolve(path.join(this.sharedDir, 'memory')); + const sharedMemoryRoot = this.resolveCanonicalPath(path.join(this.sharedDir, 'memory')); let projectEntries: fs.Dirent[] = []; try { @@ -763,15 +769,20 @@ class SharedManager { continue; } - if (!path.resolve(memoryTarget).startsWith(sharedMemoryRoot)) { + const canonicalMemoryTarget = this.resolveCanonicalPath(memoryTarget); + if (!this.isPathWithinDirectory(canonicalMemoryTarget, sharedMemoryRoot)) { continue; } await fs.promises.unlink(memoryPath); await this.ensureDirectory(memoryPath); - if (await this.pathExists(memoryTarget)) { - await this.mergeDirectoryWithConflictCopies(memoryTarget, memoryPath, instanceName); + if (await this.pathExists(canonicalMemoryTarget)) { + await this.mergeDirectoryWithConflictCopies( + canonicalMemoryTarget, + memoryPath, + instanceName + ); } } } @@ -879,6 +890,27 @@ class SharedManager { return candidate; } + private resolveCanonicalPath(targetPath: string): string { + try { + return fs.realpathSync.native(targetPath); + } catch { + return path.resolve(targetPath); + } + } + + private isPathWithinDirectory(candidatePath: string, rootPath: string): boolean { + const normalizeForCompare = (inputPath: string): string => { + const resolved = path.resolve(inputPath); + return process.platform === 'win32' ? resolved.toLowerCase() : resolved; + }; + + const normalizedCandidate = normalizeForCompare(candidatePath); + const normalizedRoot = normalizeForCompare(rootPath); + const relative = path.relative(normalizedRoot, normalizedCandidate); + + return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative)); + } + private async pathExists(targetPath: string): Promise { try { await fs.promises.access(targetPath); diff --git a/src/web-server/routes/account-routes.ts b/src/web-server/routes/account-routes.ts index 90ace229..b6ebc359 100644 --- a/src/web-server/routes/account-routes.ts +++ b/src/web-server/routes/account-routes.ts @@ -206,25 +206,24 @@ router.delete('/:name', (req: Request, res: Response): void => { return; } - // Delete from appropriate config (unified and/or legacy) - let deleted = false; - if (isUnifiedMode() && registry.hasAccountUnified(name)) { - registry.removeAccountUnified(name); - deleted = true; - } - if (registry.hasProfile(name)) { - registry.deleteProfile(name); - deleted = true; - } + const existsUnified = isUnifiedMode() && registry.hasAccountUnified(name); + const existsLegacy = registry.hasProfile(name); - if (!deleted) { + if (!existsUnified && !existsLegacy) { res.status(404).json({ error: `Account not found: ${name}` }); return; } - // Keep API delete behavior aligned with CLI remove command. + // Match CLI remove ordering: delete instance first, metadata second. instanceMgr.deleteInstance(name); + if (existsUnified) { + registry.removeAccountUnified(name); + } + if (existsLegacy) { + registry.deleteProfile(name); + } + res.json({ success: true, deleted: name }); } catch (error) { res.status(500).json({ error: (error as Error).message }); diff --git a/src/web-server/routes/config-routes.ts b/src/web-server/routes/config-routes.ts index 537f7348..31a25cc7 100644 --- a/src/web-server/routes/config-routes.ts +++ b/src/web-server/routes/config-routes.ts @@ -22,7 +22,7 @@ import { isValidContextGroupName, normalizeContextGroupName } from '../../auth/a const router = Router(); -function validateAccountContextMetadata(config: unknown): string | null { +function validateAndNormalizeAccountContextMetadata(config: unknown): string | null { if (typeof config !== 'object' || config === null) { return 'Invalid config payload'; } @@ -63,6 +63,15 @@ function validateAccountContextMetadata(config: unknown): string | null { if (!isValidContextGroupName(normalizedGroup)) { return `Invalid config.accounts.${accountName}.context_group`; } + account.context_group = normalizedGroup; + } + + if (mode === 'shared' && typeof group === 'string' && group.trim().length === 0) { + return `Invalid config.accounts.${accountName}.context_group: shared mode requires a non-empty value`; + } + + if (mode === 'isolated' && group !== undefined) { + delete account.context_group; } } @@ -130,7 +139,7 @@ router.put('/', (req: Request, res: Response): void => { return; } - const accountContextError = validateAccountContextMetadata(config); + const accountContextError = validateAndNormalizeAccountContextMetadata(config); if (accountContextError) { res.status(400).json({ error: accountContextError }); return; @@ -150,6 +159,15 @@ router.put('/', (req: Request, res: Response): void => { router.post('/migrate', async (req: Request, res: Response): Promise => { try { const dryRun = req.query.dryRun === 'true'; + if (!needsMigration()) { + res.json({ + success: true, + migratedFiles: [], + warnings: [], + alreadyMigrated: true, + }); + return; + } const result = await migrate(dryRun); res.json(result); } catch (error) { diff --git a/tests/unit/account-context.test.ts b/tests/unit/account-context.test.ts index 7e282a58..2c8f658f 100644 --- a/tests/unit/account-context.test.ts +++ b/tests/unit/account-context.test.ts @@ -40,4 +40,15 @@ describe('account context helpers', () => { expect(resolved.mode).toBe('shared'); expect(resolved.group).toBe('sprint-a'); }); + + it('normalizes whitespace in explicit shared context group names', () => { + const result = resolveCreateAccountContext({ + shareContext: false, + contextGroup: ' Team Alpha ', + }); + + expect(result.error).toBeUndefined(); + expect(result.policy.mode).toBe('shared'); + expect(result.policy.group).toBe('team-alpha'); + }); }); diff --git a/tests/unit/auth/profile-registry-context-normalization.test.ts b/tests/unit/auth/profile-registry-context-normalization.test.ts new file mode 100644 index 00000000..95882b35 --- /dev/null +++ b/tests/unit/auth/profile-registry-context-normalization.test.ts @@ -0,0 +1,92 @@ +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'; + +describe('profile-registry context normalization', () => { + let tempHome = ''; + let originalCcsHome: string | undefined; + let originalUnifiedMode: string | undefined; + + beforeEach(() => { + tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-profile-registry-context-')); + originalCcsHome = process.env.CCS_HOME; + originalUnifiedMode = process.env.CCS_UNIFIED_CONFIG; + process.env.CCS_HOME = tempHome; + }); + + afterEach(() => { + if (originalCcsHome !== undefined) process.env.CCS_HOME = originalCcsHome; + else delete process.env.CCS_HOME; + + if (originalUnifiedMode !== undefined) process.env.CCS_UNIFIED_CONFIG = originalUnifiedMode; + else delete process.env.CCS_UNIFIED_CONFIG; + + if (tempHome && fs.existsSync(tempHome)) { + fs.rmSync(tempHome, { recursive: true, force: true }); + } + }); + + it('drops non-string legacy context_group values without throwing', () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + fs.writeFileSync( + path.join(ccsDir, 'profiles.json'), + JSON.stringify( + { + version: '2.0.0', + default: null, + profiles: { + work: { + type: 'account', + created: '2026-02-24T00:00:00.000Z', + last_used: null, + context_mode: 'shared', + context_group: { invalid: true }, + }, + }, + }, + null, + 2 + ), + 'utf8' + ); + + const registry = new ProfileRegistry(); + const profile = registry.getProfile('work'); + + expect(profile.context_mode).toBe('shared'); + expect(profile.context_group).toBeUndefined(); + }); + + it('drops non-string unified context_group values without throwing', () => { + process.env.CCS_UNIFIED_CONFIG = '1'; + 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-24T00:00:00.000Z"', + ' last_used: null', + ' context_mode: shared', + ' context_group: 123', + 'profiles: {}', + 'cliproxy:', + ' oauth_accounts: {}', + ' providers: {}', + ' variants: {}', + ].join('\n'), + 'utf8' + ); + + const registry = new ProfileRegistry(); + const accounts = registry.getAllAccountsUnified(); + + expect(accounts.work.context_mode).toBe('shared'); + expect(accounts.work.context_group).toBeUndefined(); + }); +}); diff --git a/tests/unit/shared-context-policy.test.ts b/tests/unit/shared-context-policy.test.ts index dffbd37f..b882b5d5 100644 --- a/tests/unit/shared-context-policy.test.ts +++ b/tests/unit/shared-context-policy.test.ts @@ -13,6 +13,12 @@ function getTestCcsDir(): string { return path.join(path.resolve(process.env.CCS_HOME), '.ccs'); } +function createDirectorySymlink(targetPath: string, linkPath: string): void { + const symlinkType: 'dir' | 'junction' = process.platform === 'win32' ? 'junction' : 'dir'; + const linkTarget = process.platform === 'win32' ? path.resolve(targetPath) : targetPath; + fs.symlinkSync(linkTarget, linkPath, symlinkType); +} + describe('SharedManager context policy', () => { let tempRoot = ''; let originalHome: string | undefined; @@ -134,4 +140,42 @@ describe('SharedManager context policy', () => { expect(stats.isDirectory() || stats.isSymbolicLink()).toBe(true); }); + + it('skips merge when projects symlink target is outside canonical CCS roots', async () => { + const ccsDir = getTestCcsDir(); + const instancePath = path.join(ccsDir, 'instances', 'work'); + const projectsPath = path.join(instancePath, 'projects'); + const unsafeProjectsPath = path.join(ccsDir, 'shared', 'context-groups-evil', 'projects'); + const unsafeFile = path.join(unsafeProjectsPath, '-tmp-project', 'notes.md'); + + fs.mkdirSync(path.dirname(unsafeFile), { recursive: true }); + fs.writeFileSync(unsafeFile, 'unsafe source', 'utf8'); + fs.mkdirSync(instancePath, { recursive: true }); + createDirectorySymlink(unsafeProjectsPath, projectsPath); + + const manager = new SharedManager(); + await manager.syncProjectContext(instancePath, { mode: 'isolated' }); + + expect(fs.lstatSync(projectsPath).isDirectory()).toBe(true); + expect(fs.existsSync(path.join(projectsPath, '-tmp-project', 'notes.md'))).toBe(false); + }); + + it('does not detach project memory symlink from lookalike shared path prefixes', async () => { + const ccsDir = getTestCcsDir(); + const instancePath = path.join(ccsDir, 'instances', 'work'); + const projectPath = path.join(instancePath, 'projects', '-tmp-project'); + const memoryPath = path.join(projectPath, 'memory'); + const unsafeMemoryTarget = path.join(ccsDir, 'shared', 'memory-evil', '-tmp-project'); + const unsafeMemoryFile = path.join(unsafeMemoryTarget, 'MEMORY.md'); + + fs.mkdirSync(path.dirname(unsafeMemoryFile), { recursive: true }); + fs.writeFileSync(unsafeMemoryFile, 'unsafe memory', 'utf8'); + fs.mkdirSync(projectPath, { recursive: true }); + createDirectorySymlink(unsafeMemoryTarget, memoryPath); + + const manager = new SharedManager(); + await manager.syncProjectContext(instancePath, { mode: 'isolated' }); + + expect(fs.lstatSync(memoryPath).isSymbolicLink()).toBe(true); + }); }); diff --git a/tests/unit/web-server/account-routes-context.test.ts b/tests/unit/web-server/account-routes-context.test.ts index 0563c129..c3b27999 100644 --- a/tests/unit/web-server/account-routes-context.test.ts +++ b/tests/unit/web-server/account-routes-context.test.ts @@ -5,6 +5,8 @@ import * as os from 'os'; import * as path from 'path'; import type { Server } from 'http'; import accountRoutes from '../../../src/web-server/routes/account-routes'; +import ProfileRegistry from '../../../src/auth/profile-registry'; +import { InstanceManager } from '../../../src/management/instance-manager'; async function getJson(baseUrl: string, routePath: string): Promise { const response = await fetch(`${baseUrl}${routePath}`); @@ -12,6 +14,10 @@ async function getJson(baseUrl: string, routePath: string): Promise { return (await response.json()) as T; } +async function deletePath(baseUrl: string, routePath: string): Promise { + return fetch(`${baseUrl}${routePath}`, { method: 'DELETE' }); +} + describe('web-server account-routes context normalization', () => { let server: Server; let baseUrl = ''; @@ -130,4 +136,41 @@ describe('web-server account-routes context normalization', () => { expect(work?.context_mode).toBe('shared'); expect(work?.context_group).toBe('default'); }); + + it('does not delete metadata when instance deletion fails', 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: sprint-a', + 'profiles: {}', + 'cliproxy:', + ' oauth_accounts: {}', + ' providers: {}', + ' variants: {}', + ].join('\n'), + 'utf8' + ); + const registry = new ProfileRegistry(); + + const originalDeleteInstance = InstanceManager.prototype.deleteInstance; + InstanceManager.prototype.deleteInstance = () => { + throw new Error('simulated instance delete failure'); + }; + + try { + const response = await deletePath(baseUrl, '/api/accounts/work'); + expect(response.status).toBe(500); + expect(registry.hasAccountUnified('work')).toBe(true); + } finally { + InstanceManager.prototype.deleteInstance = originalDeleteInstance; + } + }); }); diff --git a/tests/unit/web-server/config-routes-account-context.test.ts b/tests/unit/web-server/config-routes-account-context.test.ts index 850f654f..a1bcb728 100644 --- a/tests/unit/web-server/config-routes-account-context.test.ts +++ b/tests/unit/web-server/config-routes-account-context.test.ts @@ -6,6 +6,7 @@ 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'; +import { loadUnifiedConfig } from '../../../src/config/unified-config-loader'; async function putJson(baseUrl: string, routePath: string, body: unknown): Promise { return fetch(`${baseUrl}${routePath}`, { @@ -17,6 +18,16 @@ async function putJson(baseUrl: string, routePath: string, body: unknown): Promi }); } +async function postJson(baseUrl: string, routePath: string, body?: unknown): Promise { + return fetch(`${baseUrl}${routePath}`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: body === undefined ? undefined : JSON.stringify(body), + }); +} + describe('web-server config-routes account context validation', () => { let server: Server; let baseUrl = ''; @@ -124,6 +135,26 @@ describe('web-server config-routes account context validation', () => { expect(payload.error).toContain('context_group'); }); + it('rejects whitespace-only shared context_group 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: '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('requires a non-empty value'); + }); + it('accepts valid shared context metadata', async () => { const config = createEmptyUnifiedConfig(); config.accounts.work = { @@ -137,5 +168,24 @@ describe('web-server config-routes account context validation', () => { expect(response.status).toBe(200); const payload = (await response.json()) as { success: boolean }; expect(payload.success).toBe(true); + + const savedConfig = loadUnifiedConfig(); + expect(savedConfig?.accounts.work.context_group).toBe('sprint-a'); + }); + + it('returns alreadyMigrated when migration is not needed', async () => { + const response = await postJson(baseUrl, '/api/config/migrate'); + + expect(response.status).toBe(200); + const payload = (await response.json()) as { + success: boolean; + migratedFiles: string[]; + warnings: string[]; + alreadyMigrated?: boolean; + }; + expect(payload.success).toBe(true); + expect(payload.migratedFiles).toEqual([]); + expect(payload.warnings).toEqual([]); + expect(payload.alreadyMigrated).toBe(true); }); });