From 2cd2d43186afc64abe7e8cb8e54c6a8907b9fa60 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 17 May 2026 17:14:54 -0400 Subject: [PATCH] fix(codex-auth): fail closed on registry corruption --- src/bin/codex-runtime-router.ts | 4 +- src/codex-auth/codex-profile-registry.ts | 41 +++++++++++++++---- src/codex-auth/resolve-active-profile.ts | 32 ++++++++------- tests/unit/bin/codex-runtime-router.test.ts | 41 +++++++++++++++++++ .../codex-auth/codex-profile-registry.test.ts | 21 +++++++--- .../codex-auth/resolve-active-profile.test.ts | 39 +++++++++--------- 6 files changed, 128 insertions(+), 50 deletions(-) diff --git a/src/bin/codex-runtime-router.ts b/src/bin/codex-runtime-router.ts index 89135bd5..04d29764 100644 --- a/src/bin/codex-runtime-router.ts +++ b/src/bin/codex-runtime-router.ts @@ -85,8 +85,8 @@ export async function main(argv: string[]): Promise { process.stderr.write(`[X] codex-auth: ${msg}\n`); return 1; } - // Resolver module threw unexpectedly — degrade to legacy mode. - process.stderr.write(`[!] codex-auth: profile resolution skipped (${msg})\n`); + process.stderr.write(`[X] codex-auth: profile resolution failed (${msg})\n`); + return 1; } } diff --git a/src/codex-auth/codex-profile-registry.ts b/src/codex-auth/codex-profile-registry.ts index c532a0ef..f742fb5f 100644 --- a/src/codex-auth/codex-profile-registry.ts +++ b/src/codex-auth/codex-profile-registry.ts @@ -16,6 +16,33 @@ function emptyRegistry(): CodexProfileData { return { version: CODEX_PROFILE_SCHEMA_VERSION, default: null, profiles: {} }; } +export class CodexProfileRegistryReadError extends Error { + constructor(message: string) { + super(message); + this.name = 'CodexProfileRegistryReadError'; + } +} + +function validateRegistryData(parsed: unknown): CodexProfileData { + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new Error('registry YAML root is not an object'); + } + + const data = parsed as Partial; + if (!data.profiles || typeof data.profiles !== 'object' || Array.isArray(data.profiles)) { + throw new Error('registry YAML is missing an object profiles map'); + } + if (data.default !== undefined && data.default !== null && typeof data.default !== 'string') { + throw new Error('registry YAML default must be a string or null'); + } + + return { + version: typeof data.version === 'string' ? data.version : CODEX_PROFILE_SCHEMA_VERSION, + default: data.default ?? null, + profiles: data.profiles as Record, + }; +} + function sleepSync(ms: number): void { try { Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); @@ -55,18 +82,16 @@ export class CodexProfileRegistry { } try { const raw = fs.readFileSync(this.registryPath, 'utf8'); - const parsed = yaml.load(raw) as CodexProfileData | null; - if (!parsed || typeof parsed !== 'object' || !parsed.profiles) { - return emptyRegistry(); - } - return parsed; + return validateRegistryData(yaml.load(raw)); } catch (err) { const msg = err instanceof Error ? err.message : String(err); logger.warn( - 'codex-auth.registry.corrupt', - `Corrupt registry at ${this.registryPath}, returning empty state: ${msg}` + 'codex-auth.registry.read-failed', + `Registry at ${this.registryPath} could not be read safely; refusing empty-state rewrite: ${msg}` + ); + throw new CodexProfileRegistryReadError( + `Codex profile registry at ${this.registryPath} could not be read safely: ${msg}. Refusing to rewrite it.` ); - return emptyRegistry(); } } diff --git a/src/codex-auth/resolve-active-profile.ts b/src/codex-auth/resolve-active-profile.ts index 71e6e791..65e7964f 100644 --- a/src/codex-auth/resolve-active-profile.ts +++ b/src/codex-auth/resolve-active-profile.ts @@ -51,6 +51,13 @@ function registryDisplayPath(registryPath: string): string { return registryPath; } +function resolutionFailure(message: string, envName: string, displayEnvName: string): never { + const prefix = envName ? `CCS_CODEX_PROFILE=${displayEnvName} is set but ` : ''; + throw new CodexAuthProfileResolutionError( + `${prefix}${message}. Refusing to fall back to ~/.codex.` + ); +} + /** @param env - Process env map; defaults to process.env. Injectable for tests. */ export function resolveActiveProfile(env: NodeJS.ProcessEnv = process.env): ResolvedProfile | null { const registryPath = getCodexAuthRegistryPath(); @@ -74,28 +81,23 @@ export function resolveActiveProfile(env: NodeJS.ProcessEnv = process.env): Reso const parsed = yaml.load(raw); if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { const msg = `registry at ${displayRegistryPath} is not a valid YAML object`; - if (envName) { - throw new CodexAuthProfileResolutionError( - `CCS_CODEX_PROFILE=${displayEnvName} is set but ${msg}. Refusing to fall back to ~/.codex.` - ); - } - process.stderr.write(`[!] codex-auth: ${msg}, falling back to ~/.codex\n`); - return null; + resolutionFailure(msg, envName, displayEnvName); } registry = parsed as RegistryShape; } catch (err) { if (err instanceof CodexAuthProfileResolutionError) throw err; const msg = `registry YAML could not be parsed at ${displayRegistryPath}`; - if (envName) { - throw new CodexAuthProfileResolutionError( - `CCS_CODEX_PROFILE=${displayEnvName} is set but ${msg}. Refusing to fall back to ~/.codex.` - ); - } - process.stderr.write(`[!] codex-auth: ${msg}, falling back to ~/.codex\n`); - return null; + resolutionFailure(msg, envName, displayEnvName); } - const profiles = registry.profiles ?? {}; + const profiles = registry.profiles; + if (!profiles || typeof profiles !== 'object' || Array.isArray(profiles)) { + resolutionFailure( + `registry at ${displayRegistryPath} is missing a valid profiles map`, + envName, + displayEnvName + ); + } // F2: explicit env override if (envName) { diff --git a/tests/unit/bin/codex-runtime-router.test.ts b/tests/unit/bin/codex-runtime-router.test.ts index 4f83c858..33091b9e 100644 --- a/tests/unit/bin/codex-runtime-router.test.ts +++ b/tests/unit/bin/codex-runtime-router.test.ts @@ -215,6 +215,24 @@ describe('codex-runtime router — non-auth profile resolution', () => { expect(stderr).toContain('registry YAML could not be parsed'); }); + it('fails fast when registry YAML is corrupt even without CCS_CODEX_PROFILE', async () => { + fs.mkdirSync(path.dirname(registryPath), { recursive: true }); + fs.writeFileSync(registryPath, 'profiles: [unterminated\n'); + + const { result: code, stderr } = await withCapturedStderr(async () => { + require.cache[ccsPath] = { exports: {} } as NodeJS.Module; + flushRouterCache(); + require.cache[ccsPath] = { exports: {} } as NodeJS.Module; + + const { main } = require(routerPath) as { main: (argv: string[]) => Promise }; + return main(['node', 'codex-runtime', 'chat']); + }); + + expect(code).toBe(1); + expect(process.env.CODEX_HOME).toBeUndefined(); + expect(stderr).toContain('registry YAML could not be parsed'); + }); + it('fails fast when CCS_CODEX_PROFILE is set and registry is not an object', async () => { fs.mkdirSync(path.dirname(registryPath), { recursive: true }); fs.writeFileSync(registryPath, '- not\n- an\n- object\n'); @@ -256,6 +274,29 @@ describe('codex-runtime router — non-auth profile resolution', () => { expect(stderr).toContain('boundary failure'); }); + it('fails closed for unexpected resolver errors instead of falling back to legacy mode', async () => { + const { result: code, stderr } = await withCapturedStderr(async () => { + require.cache[ccsPath] = { exports: {} } as NodeJS.Module; + flushRouterCache(); + require.cache[ccsPath] = { exports: {} } as NodeJS.Module; + require.cache[resolveProfilePath] = { + exports: { + resolveActiveProfile: () => { + throw new Error('permission denied'); + }, + }, + } as NodeJS.Module; + + const { main } = require(routerPath) as { main: (argv: string[]) => Promise }; + return main(['node', 'codex-runtime', 'chat']); + }); + + expect(code).toBe(1); + expect(process.env.CODEX_HOME).toBeUndefined(); + expect(stderr).toContain('profile resolution failed'); + expect(stderr).toContain('permission denied'); + }); + it('preserves an explicit CODEX_HOME already in env — does not overwrite', async () => { const explicitHome = path.join(tempDir, 'explicit-codex-home'); fs.mkdirSync(explicitHome, { recursive: true }); diff --git a/tests/unit/codex-auth/codex-profile-registry.test.ts b/tests/unit/codex-auth/codex-profile-registry.test.ts index b526b186..ed05c0f1 100644 --- a/tests/unit/codex-auth/codex-profile-registry.test.ts +++ b/tests/unit/codex-auth/codex-profile-registry.test.ts @@ -156,13 +156,24 @@ describe('CodexProfileRegistry — listProfiles', () => { }); }); -describe('CodexProfileRegistry — corrupt YAML recovery', () => { - it('returns empty state on corrupt YAML without throwing', () => { +describe('CodexProfileRegistry — corrupt YAML safety', () => { + it('throws on corrupt YAML without rewriting the registry', () => { fs.mkdirSync(path.dirname(registryPath), { recursive: true }); - fs.writeFileSync(registryPath, '{ invalid: yaml: content: [', { mode: 0o600 }); + const corrupt = '{ invalid: yaml: content: ['; + fs.writeFileSync(registryPath, corrupt, { mode: 0o600 }); const reg = new CodexProfileRegistry(registryPath); - expect(reg.listProfiles()).toEqual([]); - expect(reg.getDefault()).toBeNull(); + expect(() => reg.listProfiles()).toThrow(/could not be read safely/i); + expect(fs.readFileSync(registryPath, 'utf8')).toBe(corrupt); + }); + + it('refuses mutating writes when the registry shape is invalid', () => { + fs.mkdirSync(path.dirname(registryPath), { recursive: true }); + const invalidShape = 'version: "1.0"\ndefault: null\nprofiles: []\n'; + fs.writeFileSync(registryPath, invalidShape, { mode: 0o600 }); + const reg = new CodexProfileRegistry(registryPath); + + expect(() => reg.createProfile('work')).toThrow(/profiles map/i); + expect(fs.readFileSync(registryPath, 'utf8')).toBe(invalidShape); }); }); diff --git a/tests/unit/codex-auth/resolve-active-profile.test.ts b/tests/unit/codex-auth/resolve-active-profile.test.ts index ed8ed3d8..85493949 100644 --- a/tests/unit/codex-auth/resolve-active-profile.test.ts +++ b/tests/unit/codex-auth/resolve-active-profile.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, spyOn } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -57,28 +57,18 @@ describe('resolveActiveProfile', () => { expect(result).toBeNull(); }); - it('returns null and warns to stderr when registry YAML is corrupt', () => { + it('throws when registry YAML is corrupt even without an env override', () => { fs.mkdirSync(path.dirname(registryPath), { recursive: true }); fs.writeFileSync(registryPath, '{ invalid yaml: [[[', { mode: 0o600 }); - const stderrMessages: string[] = []; - const origWrite = process.stderr.write.bind(process.stderr); - const spy = spyOn(process.stderr, 'write').mockImplementation( - (msg: string | Uint8Array, ...rest: unknown[]) => { - stderrMessages.push(typeof msg === 'string' ? msg : String(msg)); - return origWrite(msg as string, ...(rest as Parameters).slice(1)); - } - ); - - const result = resolveActiveProfile({}); - - spy.mockRestore(); - - expect(result).toBeNull(); - expect(stderrMessages.some((m) => m.includes('codex-auth'))).toBe(true); - expect(stderrMessages.join('')).toContain('$CCS_HOME/.ccs/codex-profiles.yaml'); - expect(stderrMessages.join('')).not.toContain(registryPath); - expect(stderrMessages.join('')).not.toContain('invalid yaml'); + expect(() => resolveActiveProfile({})).toThrow(/registry YAML could not be parsed/); + try { + resolveActiveProfile({}); + } catch (err) { + expect(String(err)).toContain('$CCS_HOME/.ccs/codex-profiles.yaml'); + expect(String(err)).not.toContain(registryPath); + expect(String(err)).not.toContain('invalid yaml'); + } }); it('throws when CCS_CODEX_PROFILE is set and registry YAML is corrupt', () => { @@ -99,6 +89,15 @@ describe('resolveActiveProfile', () => { expect(String(thrown)).not.toContain('invalid yaml'); }); + it('throws when registry is missing a valid profiles map', () => { + fs.mkdirSync(path.dirname(registryPath), { recursive: true }); + fs.writeFileSync(registryPath, 'version: "1.0"\ndefault: null\nprofiles: []\n', { + mode: 0o600, + }); + + expect(() => resolveActiveProfile({})).toThrow(/valid profiles map/); + }); + it('returns source=env when CCS_CODEX_PROFILE matches a registry entry', () => { const profileDir = makeProfileDir('work'); writeRegistry({