diff --git a/src/codex-auth/codex-auth-dashboard-service.ts b/src/codex-auth/codex-auth-dashboard-service.ts index 79accaa8..aaa56bbd 100644 --- a/src/codex-auth/codex-auth-dashboard-service.ts +++ b/src/codex-auth/codex-auth-dashboard-service.ts @@ -9,10 +9,9 @@ * fields (email, plan, accountId) are extracted from JWT. auth.json is * read/decoded and then discarded. * - * Cache: 5-second in-memory single-key cache reduces fs reads during - * dashboard polling. Out-of-process callers rely on the TTL. In-process - * callers (Phase 2 CLI commands running in dev-server context) can call - * invalidateCodexAuthProfilesCache() to force an immediate re-read. + * Cache: 5-second in-memory cache reduces full registry/auth reads during + * dashboard polling, but each call still stats the registry so corruption or + * edits are surfaced immediately instead of serving stale success. */ import * as fs from 'fs'; @@ -53,7 +52,11 @@ export interface CodexAuthProfilesSummary { // ── Cache ─────────────────────────────────────────────────────────────────── -let cache: { value: CodexAuthProfilesSummary; expiresAt: number } | null = null; +let cache: { + value: CodexAuthProfilesSummary; + expiresAt: number; + registrySignature: string; +} | null = null; const TTL_MS = 5000; /** @@ -67,6 +70,21 @@ export function invalidateCodexAuthProfilesCache(): void { // ── Registry helpers ──────────────────────────────────────────────────────── +function getRegistryCacheSignature(): string { + const registryPath = getCodexAuthRegistryPath(); + try { + const stat = fs.statSync(registryPath); + return ['present', stat.dev, stat.ino, stat.size, stat.mtimeMs, stat.ctimeMs].join(':'); + } catch (err) { + if ((err as NodeJS.ErrnoException | undefined)?.code === 'ENOENT') { + return 'missing'; + } + const msg = err instanceof Error ? err.message : String(err); + logger.warn('codex-auth.dashboard.registry-stat-failed', `Registry stat failed: ${msg}`); + throw new Error('Codex auth profile registry could not be checked safely'); + } +} + function readRegistry(): CodexProfileData { const registryPath = getCodexAuthRegistryPath(); if (!fs.existsSync(registryPath)) { @@ -225,10 +243,11 @@ async function buildSummary(): Promise { */ export async function getCodexAuthProfilesSummary(): Promise { const now = Date.now(); - if (cache && cache.expiresAt > now) { + const registrySignature = getRegistryCacheSignature(); + if (cache && cache.expiresAt > now && cache.registrySignature === registrySignature) { return cache.value; } const value = await buildSummary(); - cache = { value, expiresAt: now + TTL_MS }; + cache = { value, expiresAt: now + TTL_MS, registrySignature }; return value; } diff --git a/src/codex-auth/codex-profile-registry.ts b/src/codex-auth/codex-profile-registry.ts index 4e90bba6..6ceaed9b 100644 --- a/src/codex-auth/codex-profile-registry.ts +++ b/src/codex-auth/codex-profile-registry.ts @@ -24,7 +24,7 @@ export class CodexProfileRegistryReadError extends Error { } } -function validateRegistryData(parsed: unknown): CodexProfileData { +export function validateCodexProfileRegistryData(parsed: unknown): CodexProfileData { if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { throw new Error('registry YAML root is not an object'); } @@ -164,7 +164,7 @@ export class CodexProfileRegistry { } try { const raw = fs.readFileSync(this.registryPath, 'utf8'); - return validateRegistryData(yaml.load(raw)); + return validateCodexProfileRegistryData(yaml.load(raw)); } catch (err) { const msg = safeRegistryReadMessage(err); const displayPath = registryDisplayPath(this.registryPath); @@ -309,13 +309,16 @@ export class CodexProfileRegistry { }); } - removeProfile(name: string): void { + removeProfile(name: string, options: { forceDefault?: boolean } = {}): void { assertValidProfileName(name); this._withRegistryWriteLock(() => { const data = this._read(); if (!data.profiles[name]) { throw new Error(`Profile not found: ${name}`); } + if (data.default === name && Object.keys(data.profiles).length > 1 && !options.forceDefault) { + throw new Error('Cannot remove default profile while other profiles exist without --force'); + } delete data.profiles[name]; if (data.default === name) { data.default = null; diff --git a/src/codex-auth/commands/remove-command.ts b/src/codex-auth/commands/remove-command.ts index f28a6224..a83b39f7 100644 --- a/src/codex-auth/commands/remove-command.ts +++ b/src/codex-auth/commands/remove-command.ts @@ -90,7 +90,16 @@ export async function handleRemoveCodex(ctx: CodexCommandContext, args: string[] // Ghost case: dir already gone if (!dirExists) { process.stderr.write(`[!] Profile dir was already missing; removing registry entry only.\n`); - registry.removeProfile(profileName); + try { + registry.removeProfile(profileName, { forceDefault: force }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + exitWithError( + `Profile registry update failed; profile dir was already missing.\n ${msg}`, + ExitCode.GENERAL_ERROR + ); + return; + } console.log(ok(`Profile removed: ${profileName}`)); return; } @@ -146,7 +155,7 @@ export async function handleRemoveCodex(ctx: CodexCommandContext, args: string[] } try { - registry.removeProfile(profileName); + registry.removeProfile(profileName, { forceDefault: force }); } catch (err) { const restored = _restoreProfileDir(stagedDeleteDir, profileDir); _removePathBestEffort(preservationDir); diff --git a/src/codex-auth/resolve-active-profile.ts b/src/codex-auth/resolve-active-profile.ts index 555209ef..f8b938f7 100644 --- a/src/codex-auth/resolve-active-profile.ts +++ b/src/codex-auth/resolve-active-profile.ts @@ -9,6 +9,8 @@ import * as yaml from 'js-yaml'; import { getCodexAuthRegistryPath, resolveCodexProfileDir } from './codex-profile-paths'; import { getCcsDirSource } from '../utils/config-manager'; import { getCodexProfileNameError } from './types'; +import { validateCodexProfileRegistryData } from './codex-profile-registry'; +import type { CodexProfileData } from './types'; export interface ResolvedProfile { name: string; @@ -23,12 +25,6 @@ export class CodexAuthProfileResolutionError extends Error { } } -interface RegistryShape { - version?: string; - default?: string | null; - profiles?: Record; -} - function quoteDiagnosticValue(value: string): string { const escaped = value .replace(/[\x00-\x1f\x7f]/g, (char) => `\\x${char.charCodeAt(0).toString(16).padStart(2, '0')}`) @@ -117,21 +113,28 @@ export function resolveActiveProfile(env: NodeJS.ProcessEnv = process.env): Reso return null; } - let registry: RegistryShape; + let parsed: unknown; try { const raw = fs.readFileSync(registryPath, 'utf8'); - const parsed = yaml.load(raw); - if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { - const msg = `registry at ${displayRegistryPath} is not a valid YAML object`; - resolutionFailure(msg, envName, displayEnvName); - } - registry = parsed as RegistryShape; + parsed = yaml.load(raw); } catch (err) { if (err instanceof CodexAuthProfileResolutionError) throw err; const msg = `registry YAML could not be parsed at ${displayRegistryPath}`; resolutionFailure(msg, envName, displayEnvName); } + let registry: CodexProfileData; + try { + registry = validateCodexProfileRegistryData(parsed); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + resolutionFailure( + `registry at ${displayRegistryPath} is invalid: ${msg}`, + envName, + displayEnvName + ); + } + const profiles = registry.profiles; if (!profiles || typeof profiles !== 'object' || Array.isArray(profiles)) { resolutionFailure( @@ -161,8 +164,8 @@ export function resolveActiveProfile(env: NodeJS.ProcessEnv = process.env): Reso } // F3: registry default - const defaultName = registry.default ?? null; - if (defaultName) { + const defaultName = registry.default; + if (defaultName !== null) { if (typeof defaultName !== 'string') { resolutionFailure( `registry default at ${displayRegistryPath} is not a valid profile name`, diff --git a/tests/integration/web-server/codex-profiles-endpoint.test.ts b/tests/integration/web-server/codex-profiles-endpoint.test.ts index 3651595d..44b3ce05 100644 --- a/tests/integration/web-server/codex-profiles-endpoint.test.ts +++ b/tests/integration/web-server/codex-profiles-endpoint.test.ts @@ -8,7 +8,7 @@ * - response contains no token substrings */ -import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from 'bun:test'; import * as fs from 'fs'; import * as http from 'http'; import * as os from 'os'; @@ -93,6 +93,7 @@ afterEach(async () => { delete process.env.CCS_HOME; delete process.env.CODEX_HOME; delete process.env.CCS_CODEX_PROFILE; + mock.restore(); fs.rmSync(tmpDir, { recursive: true, force: true }); }); @@ -123,6 +124,43 @@ describe('GET /api/codex/profiles', () => { expect((body as { error?: string }).error).toContain('could not be read safely'); }); + it('returns 500 when a malformed registry appears after an empty response was cached', async () => { + const first = await get('/api/codex/profiles'); + expect(first.status).toBe(200); + + const registryPath = path.join(ccsDir, 'codex-profiles.yaml'); + fs.writeFileSync(registryPath, '{ invalid: yaml: [', { mode: 0o600 }); + const future = new Date(Date.now() + 10_000); + fs.utimesSync(registryPath, future, future); + + const { status, body } = await get('/api/codex/profiles'); + + expect(status).toBe(500); + expect((body as { error?: string }).error).toContain('could not be read safely'); + }); + + it('returns a sanitized 500 when registry stat fails', async () => { + const registryPath = path.join(ccsDir, 'codex-profiles.yaml'); + const rawMessage = `EACCES: permission denied, stat '${registryPath}'`; + const realStatSync = fs.statSync; + spyOn(fs, 'statSync').mockImplementation((target) => { + if (target === registryPath) { + const err = new Error(rawMessage) as NodeJS.ErrnoException; + err.code = 'EACCES'; + throw err; + } + return realStatSync(target); + }); + + const { status, body } = await get('/api/codex/profiles'); + const error = (body as { error?: string }).error ?? ''; + + expect(status).toBe(500); + expect(error).toContain('could not be checked safely'); + expect(error).not.toContain(registryPath); + expect(error).not.toContain('EACCES'); + }); + it('returns 200 with decoded email and plan for a valid profile', async () => { const instancesDir = path.join(ccsDir, 'codex-instances'); const workDir = path.join(instancesDir, 'work'); diff --git a/tests/unit/bin/codex-runtime-router.test.ts b/tests/unit/bin/codex-runtime-router.test.ts index 5aadcd02..2562e969 100644 --- a/tests/unit/bin/codex-runtime-router.test.ts +++ b/tests/unit/bin/codex-runtime-router.test.ts @@ -249,7 +249,7 @@ describe('codex-runtime router — non-auth profile resolution', () => { expect(code).toBe(1); expect(process.env.CODEX_HOME).toBeUndefined(); - expect(stderr).toContain('not a valid YAML object'); + expect(stderr).toContain('registry YAML root is not an object'); }); it('fails fast for structural resolver errors with the expected name', async () => { diff --git a/tests/unit/codex-auth/codex-auth-dashboard-service.test.ts b/tests/unit/codex-auth/codex-auth-dashboard-service.test.ts index 87857c93..df32c116 100644 --- a/tests/unit/codex-auth/codex-auth-dashboard-service.test.ts +++ b/tests/unit/codex-auth/codex-auth-dashboard-service.test.ts @@ -6,7 +6,7 @@ * and token redaction from response. */ -import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from 'bun:test'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -71,6 +71,11 @@ function writeRegistry(registryPath: string, data: unknown): void { fs.writeFileSync(registryPath, yaml(data), { mode: 0o600 }); } +function bumpRegistryMtime(registryPath: string): void { + const future = new Date(Date.now() + 10_000); + fs.utimesSync(registryPath, future, future); +} + // Module cache helpers ----------------------------------------------------- async function importService() { @@ -100,6 +105,7 @@ afterEach(() => { delete process.env.CCS_HOME; delete process.env.CODEX_HOME; delete process.env.CCS_CODEX_PROFILE; + mock.restore(); fs.rmSync(tmpDir, { recursive: true, force: true }); }); @@ -125,6 +131,34 @@ describe('getCodexAuthProfilesSummary', () => { await expect(getCodexAuthProfilesSummary()).rejects.toThrow(/could not be read safely/i); }); + it('does not expose raw stat errors when the registry cannot be checked', async () => { + const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService(); + invalidateCodexAuthProfilesCache(); + + const registryPath = path.join(ccsDir, 'codex-profiles.yaml'); + const rawMessage = `EACCES: permission denied, stat '${registryPath}'`; + const realStatSync = fs.statSync; + spyOn(fs, 'statSync').mockImplementation((target) => { + if (target === registryPath) { + const err = new Error(rawMessage) as NodeJS.ErrnoException; + err.code = 'EACCES'; + throw err; + } + return realStatSync(target); + }); + + let message = ''; + try { + await getCodexAuthProfilesSummary(); + } catch (err) { + message = String(err); + } + + expect(message).toContain('could not be checked safely'); + expect(message).not.toContain(registryPath); + expect(message).not.toContain('EACCES'); + }); + it('returns decoded email, plan, accountId for valid registry with 2 profiles', async () => { const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService(); invalidateCodexAuthProfilesCache(); @@ -357,7 +391,7 @@ describe('getCodexAuthProfilesSummary', () => { expect(result.active).toBeNull(); }); - it('returns cached value on second call within 5s (no extra fs reads)', async () => { + it('returns cached value on second call within 5s when the registry file is unchanged', async () => { const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService(); invalidateCodexAuthProfilesCache(); @@ -373,14 +407,42 @@ describe('getCodexAuthProfilesSummary', () => { ); const first = await getCodexAuthProfilesSummary(); - // Modify registry after first call — should NOT be seen within cache TTL + const second = await getCodexAuthProfilesSummary(); + + // Both calls should return same reference (cache hit) + expect(second).toBe(first); + }); + + it('does not serve cached missing-registry success after a malformed registry appears', async () => { + const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService(); + invalidateCodexAuthProfilesCache(); + + const first = await getCodexAuthProfilesSummary(); + expect(first.profiles).toHaveLength(0); + + const registryPath = path.join(ccsDir, 'codex-profiles.yaml'); + fs.writeFileSync(registryPath, '{ invalid: yaml: [', { mode: 0o600 }); + bumpRegistryMtime(registryPath); + + await expect(getCodexAuthProfilesSummary()).rejects.toThrow(/could not be read safely/i); + }); + + it('does not serve cached valid-registry success after the registry becomes malformed', async () => { + const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService(); + invalidateCodexAuthProfilesCache(); + + const registryPath = path.join(ccsDir, 'codex-profiles.yaml'); fs.writeFileSync(registryPath, `version: "1.0"\ndefault: null\nprofiles: {}\n`, { mode: 0o600, }); - const second = await getCodexAuthProfilesSummary(); - // Both calls should return same reference (cache hit) - expect(second).toBe(first); + const first = await getCodexAuthProfilesSummary(); + expect(first.profiles).toHaveLength(0); + + fs.writeFileSync(registryPath, '{ invalid: yaml: [', { mode: 0o600 }); + bumpRegistryMtime(registryPath); + + await expect(getCodexAuthProfilesSummary()).rejects.toThrow(/could not be read safely/i); }); it('invalidateCodexAuthProfilesCache forces re-read on next call', async () => { diff --git a/tests/unit/codex-auth/codex-profile-registry.test.ts b/tests/unit/codex-auth/codex-profile-registry.test.ts index 7157f951..1045d3b5 100644 --- a/tests/unit/codex-auth/codex-profile-registry.test.ts +++ b/tests/unit/codex-auth/codex-profile-registry.test.ts @@ -10,7 +10,7 @@ let CodexProfileRegistry: new (registryPath?: string) => { createProfile(name: string, meta?: Record): void; getProfile(name: string): Record; updateProfile(name: string, partial: Record): void; - removeProfile(name: string): void; + removeProfile(name: string, options?: { forceDefault?: boolean }): void; listProfiles(): string[]; hasProfile(name: string): boolean; getDefault(): string | null; @@ -128,13 +128,25 @@ describe('CodexProfileRegistry — remove', () => { expect(reg.getDefault()).toBeNull(); }); - it('does not promote another profile when the default profile is removed', () => { + it('refuses to remove the default profile when other profiles remain without force', () => { const reg = new CodexProfileRegistry(registryPath); reg.createProfile('work'); reg.createProfile('personal'); reg.setDefault('work'); - reg.removeProfile('work'); + expect(() => reg.removeProfile('work')).toThrow(/default profile.*without --force/i); + + expect(reg.listProfiles()).toEqual(['work', 'personal']); + expect(reg.getDefault()).toBe('work'); + }); + + it('does not promote another profile when the default profile is force removed', () => { + const reg = new CodexProfileRegistry(registryPath); + reg.createProfile('work'); + reg.createProfile('personal'); + reg.setDefault('work'); + + reg.removeProfile('work', { forceDefault: true }); expect(reg.listProfiles()).toEqual(['personal']); expect(reg.getDefault()).toBeNull(); diff --git a/tests/unit/codex-auth/commands/remove-command.test.ts b/tests/unit/codex-auth/commands/remove-command.test.ts index f612686d..652caedc 100644 --- a/tests/unit/codex-auth/commands/remove-command.test.ts +++ b/tests/unit/codex-auth/commands/remove-command.test.ts @@ -125,6 +125,49 @@ describe('handleRemoveCodex — default guard', () => { await handleRemoveCodex(ctx, ['alpha', '--force', '--yes']); expect(ctx.registry.hasProfile('alpha')).toBe(false); }); + + it('restores data when the target becomes default between precheck and registry write', async () => { + const { handleRemoveCodex } = await import( + '../../../../src/codex-auth/commands/remove-command' + ); + const ctx = await makeCtx('alpha', 'beta'); + ctx.registry.setDefault('beta'); + const profileDir = path.join(ccsHome, '.ccs', 'codex-instances', 'alpha'); + const authJsonPath = path.join(profileDir, 'auth.json'); + fs.writeFileSync(authJsonPath, JSON.stringify({ tokens: { id_token: 'h.e30K.s' } })); + + const realCpSync = fs.cpSync; + spyOn(fs, 'cpSync').mockImplementation((src, dest, options) => { + const result = realCpSync(src, dest, options); + if (typeof dest === 'string' && dest.includes('.preserved.')) { + ctx.registry.setDefault('alpha'); + } + return result; + }); + + let exitCode = -1; + const origExit = process.exit; + const origErr = console.error; + process.exit = (code?: number) => { + exitCode = code ?? 0; + throw new Error('exit'); + }; + console.error = () => {}; + + try { + await handleRemoveCodex(ctx, ['alpha', '--yes']); + } catch { + /* process.exit */ + } finally { + process.exit = origExit; + console.error = origErr; + } + + expect(exitCode).toBeGreaterThan(0); + expect(fs.existsSync(authJsonPath)).toBe(true); + expect(ctx.registry.hasProfile('alpha')).toBe(true); + expect(ctx.registry.getDefault()).toBe('alpha'); + }); }); // ── only profile → allows removal ──────────────────────────────────────────── diff --git a/tests/unit/codex-auth/resolve-active-profile.test.ts b/tests/unit/codex-auth/resolve-active-profile.test.ts index 153f479e..5a65cb9b 100644 --- a/tests/unit/codex-auth/resolve-active-profile.test.ts +++ b/tests/unit/codex-auth/resolve-active-profile.test.ts @@ -95,7 +95,17 @@ describe('resolveActiveProfile', () => { mode: 0o600, }); - expect(() => resolveActiveProfile({})).toThrow(/valid profiles map/); + expect(() => resolveActiveProfile({})).toThrow(/object profiles map/); + }); + + it('throws instead of falling back when registry default has an invalid falsy type', () => { + writeRegistry({ + version: '1.0', + default: false, + profiles: {}, + }); + + expect(() => resolveActiveProfile({})).toThrow(/default must be a string or null/i); }); it('throws when CCS_CODEX_PROFILE contains an unsafe profile name', () => { @@ -129,7 +139,7 @@ describe('resolveActiveProfile', () => { profiles: {}, }); - expect(() => resolveActiveProfile({})).toThrow(/default 'ghost' is missing/i); + expect(() => resolveActiveProfile({})).toThrow(/default profile is missing from profiles map/i); }); it('throws when matched registry profile entry is malformed', () => { @@ -141,7 +151,36 @@ describe('resolveActiveProfile', () => { }, }); - expect(() => resolveActiveProfile({})).toThrow(/not a valid object/i); + expect(() => resolveActiveProfile({})).toThrow(/profile "work" must be an object/i); + }); + + it('throws when a registry profile is missing required metadata', () => { + writeRegistry({ + version: '1.0', + default: 'work', + profiles: { + work: { type: 'codex', last_used: null }, + }, + }); + + expect(() => resolveActiveProfile({})).toThrow(/created timestamp/i); + }); + + it('throws when a registry profile has invalid optional metadata types', () => { + writeRegistry({ + version: '1.0', + default: 'work', + profiles: { + work: { + type: 'codex', + created: '2026-01-01T00:00:00.000Z', + last_used: null, + account_id: 123, + }, + }, + }); + + expect(() => resolveActiveProfile({})).toThrow(/account_id must be a string/i); }); it('returns source=env when CCS_CODEX_PROFILE matches a registry entry', () => {