fix(codex-auth): harden profile review findings

This commit is contained in:
Tam Nhu Tran
2026-05-17 15:32:29 -04:00
parent 4e7d648967
commit a3fe2c63d8
15 changed files with 643 additions and 86 deletions
@@ -34,6 +34,20 @@ function writeAuthJson(profileDir: string, idTokenPayload: Record<string, unknow
});
}
function writeRawAuthJson(profileDir: string, idToken: string): void {
fs.writeFileSync(
path.join(profileDir, 'auth.json'),
JSON.stringify({
tokens: {
id_token: idToken,
access_token: 'access-token-should-not-appear',
refresh_token: 'refresh-token-should-not-appear',
},
}),
{ mode: 0o600 }
);
}
function writeRegistry(registryPath: string, data: unknown): void {
const dir = path.dirname(registryPath);
if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true });
@@ -177,6 +191,75 @@ describe('getCodexAuthProfilesSummary', () => {
expect(broken?.accountId).toBeNull();
});
it('sets authValid=false when id_token is non-empty but malformed', async () => {
const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService();
invalidateCodexAuthProfilesCache();
const instancesDir = path.join(ccsDir, 'codex-instances');
const brokenDir = path.join(instancesDir, 'broken-jwt');
fs.mkdirSync(brokenDir, { recursive: true });
writeRawAuthJson(brokenDir, 'not-a-jwt');
const registryPath = path.join(ccsDir, 'codex-profiles.yaml');
fs.writeFileSync(
registryPath,
`version: "1.0"\ndefault: broken-jwt\nprofiles:\n broken-jwt:\n type: codex\n created: "2026-01-01T00:00:00Z"\n last_used: null\n`,
{ mode: 0o600 }
);
const result = await getCodexAuthProfilesSummary();
const broken = result.profiles[0];
expect(broken?.authValid).toBe(false);
expect(broken?.email).toBeNull();
expect(broken?.plan).toBeNull();
expect(broken?.accountId).toBeNull();
});
it('sets authValid=false when id_token contains invalid base64url characters', async () => {
const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService();
invalidateCodexAuthProfilesCache();
const instancesDir = path.join(ccsDir, 'codex-instances');
const brokenDir = path.join(instancesDir, 'broken-base64url');
fs.mkdirSync(brokenDir, { recursive: true });
writeRawAuthJson(brokenDir, 'h.e30$.s');
const registryPath = path.join(ccsDir, 'codex-profiles.yaml');
fs.writeFileSync(
registryPath,
`version: "1.0"\ndefault: broken-base64url\nprofiles:\n broken-base64url:\n type: codex\n created: "2026-01-01T00:00:00Z"\n last_used: null\n`,
{ mode: 0o600 }
);
const result = await getCodexAuthProfilesSummary();
const broken = result.profiles[0];
expect(broken?.authValid).toBe(false);
});
it('sets authValid=true for a valid but sparse JWT payload', async () => {
const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService();
invalidateCodexAuthProfilesCache();
const instancesDir = path.join(ccsDir, 'codex-instances');
const sparseDir = path.join(instancesDir, 'sparse');
fs.mkdirSync(sparseDir, { recursive: true });
writeRawAuthJson(sparseDir, buildToken({}));
const registryPath = path.join(ccsDir, 'codex-profiles.yaml');
fs.writeFileSync(
registryPath,
`version: "1.0"\ndefault: sparse\nprofiles:\n sparse:\n type: codex\n created: "2026-01-01T00:00:00Z"\n last_used: null\n`,
{ mode: 0o600 }
);
const result = await getCodexAuthProfilesSummary();
const sparse = result.profiles[0];
expect(sparse?.authValid).toBe(true);
expect(sparse?.email).toBeNull();
expect(sparse?.plan).toBeNull();
expect(sparse?.accountId).toBeNull();
});
it('sets authValid=false and nulls identity fields when auth.json is missing', async () => {
const { getCodexAuthProfilesSummary, invalidateCodexAuthProfilesCache } = await importService();
invalidateCodexAuthProfilesCache();
@@ -1,8 +1,10 @@
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';
import * as yaml from 'js-yaml';
import * as lockfile from 'proper-lockfile';
import { spawn } from 'child_process';
let CodexProfileRegistry: new (registryPath?: string) => {
createProfile(name: string, meta?: Record<string, unknown>): void;
@@ -41,6 +43,7 @@ afterEach(() => {
process.env.CCS_HOME = ORIGINAL_CCS_HOME;
}
fs.rmSync(tempDir, { recursive: true, force: true });
mock.restore();
});
describe('CodexProfileRegistry — empty state', () => {
@@ -212,3 +215,83 @@ describe('CodexProfileRegistry — registry file permissions', () => {
expect(stat.mode & 0o777).toBe(0o600);
});
});
describe('CodexProfileRegistry — write lock', () => {
it('serializes read-modify-write mutations through a registry lock', () => {
const release = () => {};
const lockSpy = spyOn(lockfile, 'lockSync').mockReturnValue(release);
const reg = new CodexProfileRegistry(registryPath);
reg.createProfile('work');
expect(lockSpy).toHaveBeenCalled();
const [lockTarget, options] = lockSpy.mock.calls[0] ?? [];
expect(lockTarget).toBe(path.dirname(registryPath));
expect(options).toMatchObject({ stale: 10000 });
});
it('waits for a contended registry lock before writing', async () => {
const registryDir = path.dirname(registryPath);
const readyPath = path.join(tempDir, 'holder-ready');
const holderScript = path.join(tempDir, 'hold-registry-lock.cjs');
fs.writeFileSync(
holderScript,
`
const fs = require('fs');
const lockfile = require(process.argv[4]);
const release = lockfile.lockSync(process.argv[2], { stale: 10000 });
fs.writeFileSync(process.argv[3], String(process.pid));
setTimeout(() => {
release();
process.exit(0);
}, 150);
setTimeout(() => process.exit(2), 5000);
process.on('SIGTERM', () => {
try { release(); } finally { process.exit(0); }
});
`,
'utf8'
);
const child = spawn(
process.execPath,
[
holderScript,
registryDir,
readyPath,
path.join(process.cwd(), 'node_modules', 'proper-lockfile'),
],
{
cwd: process.cwd(),
stdio: ['ignore', 'ignore', 'pipe'],
}
);
try {
await waitForFile(readyPath);
const reg = new CodexProfileRegistry(registryPath);
reg.createProfile('work');
expect(reg.hasProfile('work')).toBe(true);
} finally {
if (!child.killed) child.kill();
await waitForChildExit(child);
}
});
});
async function waitForFile(filePath: string, timeoutMs = 1000): Promise<void> {
const started = Date.now();
while (!fs.existsSync(filePath)) {
if (Date.now() - started > timeoutMs) {
throw new Error(`Timed out waiting for ${filePath}`);
}
await new Promise((resolve) => setTimeout(resolve, 10));
}
}
async function waitForChildExit(child: ReturnType<typeof spawn>): Promise<void> {
if (child.exitCode !== null || child.signalCode !== null) return;
await new Promise<void>((resolve) => child.once('exit', () => resolve()));
}
@@ -34,6 +34,12 @@ async function makeCtx() {
return { registry: reg, version: '0.0.0-test' };
}
function buildToken(payload: Record<string, unknown>): string {
const header = Buffer.from(JSON.stringify({ alg: 'RS256', typ: 'JWT' })).toString('base64url');
const body = Buffer.from(JSON.stringify(payload)).toString('base64url');
return `${header}.${body}.fakesig`;
}
/** Suppress console output during test. */
function silenceConsole(): () => void {
const origLog = console.log;
@@ -306,4 +312,54 @@ describe('handleCreateCodex — auto-spawn login (D11)', () => {
expect(fs.existsSync(profileDir)).toBe(true);
expect(ctx.registry.hasProfile('faillogin')).toBe(true);
});
it('persists last_used and account_id when login token has account_id only', async () => {
const detectorMod = await import('../../../../src/targets/codex-detector');
spyOn(detectorMod, 'detectCodexCli').mockReturnValue('/usr/bin/codex');
spyOn(childProcess, 'spawn').mockImplementation(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(_cmd: string, _args: string[], opts: any) => {
const dir = (opts?.env?.CODEX_HOME as string) ?? '';
if (dir) {
fs.mkdirSync(dir, { recursive: true });
fs.writeFileSync(
path.join(dir, 'auth.json'),
JSON.stringify({
tokens: {
id_token: buildToken({
'https://api.openai.com/auth': {
chatgpt_account_id: 'acct-account-only',
},
}),
},
})
);
}
const ee = {
on: (evt: string, cb: (n: number) => void) => {
if (evt === 'exit') setImmediate(() => cb(0));
return ee;
},
};
return ee as ReturnType<typeof childProcess.spawn>;
}
);
const { handleCreateCodex } = await import(
'../../../../src/codex-auth/commands/create-command'
);
const ctx = await makeCtx();
const restore = silenceConsole();
try {
await handleCreateCodex(ctx, ['accountonly']);
} finally {
restore();
}
const meta = ctx.registry.getProfile('accountonly');
expect(meta.last_used).toBeTruthy();
expect(meta.account_id).toBe('acct-account-only');
});
});
@@ -327,6 +327,64 @@ describe('import-default — torn-write retry', () => {
expect(exitCalled).toBe(true);
expect(ctx.registry.hasProfile('torntest')).toBe(false);
});
it('rejects a malformed 3-segment id_token payload', async () => {
const authPath = path.join(legacyCodexHome, 'auth.json');
fs.writeFileSync(authPath, JSON.stringify({ tokens: { id_token: 'header.not-json.sig' } }));
const { handleImportDefaultCodex } = await import(
'../../../../src/codex-auth/commands/import-default-command'
);
const ctx = await makeCtx();
let exitCalled = false;
const origExit = process.exit;
process.exit = () => {
exitCalled = true;
throw new Error('exit');
};
const restore = silenceConsole();
try {
await handleImportDefaultCodex(ctx, ['badjwt']);
} catch {
/* expected */
} finally {
restore();
process.exit = origExit;
}
expect(exitCalled).toBe(true);
expect(ctx.registry.hasProfile('badjwt')).toBe(false);
});
it('rejects an id_token payload with invalid base64url characters', async () => {
const authPath = path.join(legacyCodexHome, 'auth.json');
fs.writeFileSync(authPath, JSON.stringify({ tokens: { id_token: 'h.e30$.s' } }));
const { handleImportDefaultCodex } = await import(
'../../../../src/codex-auth/commands/import-default-command'
);
const ctx = await makeCtx();
let exitCalled = false;
const origExit = process.exit;
process.exit = () => {
exitCalled = true;
throw new Error('exit');
};
const restore = silenceConsole();
try {
await handleImportDefaultCodex(ctx, ['bad-base64url']);
} catch {
/* expected */
} finally {
restore();
process.exit = origExit;
}
expect(exitCalled).toBe(true);
expect(ctx.registry.hasProfile('bad-base64url')).toBe(false);
});
});
describe('import-default — Codex running detection', () => {
@@ -191,4 +191,82 @@ describe('handleRemoveCodex — confirmation', () => {
expect(promptCalled).toBe(false);
expect(ctx.registry.hasProfile('skipconfirm')).toBe(false);
});
it('preserves profile data when registry removal fails', async () => {
const { handleRemoveCodex } = await import(
'../../../../src/codex-auth/commands/remove-command'
);
const ctx = await makeCtx('preserveme');
const profileDir = path.join(ccsHome, '.ccs', 'codex-instances', 'preserveme');
const authJsonPath = path.join(profileDir, 'auth.json');
fs.writeFileSync(authJsonPath, JSON.stringify({ tokens: { id_token: 'h.e30K.s' } }));
spyOn(ctx.registry, 'removeProfile').mockImplementation(() => {
throw new Error('registry write denied');
});
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, ['preserveme', '--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('preserveme')).toBe(true);
});
it('restores profile data and registry when final deletion fails', async () => {
const { handleRemoveCodex } = await import(
'../../../../src/codex-auth/commands/remove-command'
);
const ctx = await makeCtx('restoreme');
ctx.registry.setDefault('restoreme');
const profileDir = path.join(ccsHome, '.ccs', 'codex-instances', 'restoreme');
const authJsonPath = path.join(profileDir, 'auth.json');
fs.writeFileSync(authJsonPath, JSON.stringify({ tokens: { id_token: 'h.e30K.s' } }));
const realRmSync = fs.rmSync;
spyOn(fs, 'rmSync').mockImplementation((target, options) => {
if (typeof target === 'string' && target.includes('.deleting.')) {
throw new Error('delete denied');
}
return realRmSync(target, options);
});
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, ['restoreme', '--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('restoreme')).toBe(true);
expect(ctx.registry.getDefault()).toBe('restoreme');
});
});
@@ -143,12 +143,12 @@ describe('handleUseCodex — shell syntax', () => {
expect(stdout).toContain('$env:CCS_CODEX_PROFILE');
});
it('cmd: set KEY=value (no quotes)', async () => {
it('cmd: quoted set assignment syntax', async () => {
const { handleUseCodex } = await import('../../../../src/codex-auth/commands/use-command');
const ctx = await makeCtxWithProfile('work');
const { stdout } = await captureStreams(() => handleUseCodex(ctx, ['work', '--shell', 'cmd']));
expect(stdout).toContain('set CODEX_HOME=');
expect(stdout).toContain('set CCS_CODEX_PROFILE=work');
expect(stdout).toContain('set "CODEX_HOME=');
expect(stdout).toContain('set "CCS_CODEX_PROFILE=work"');
});
it('invalid --shell value → stderr error, empty stdout', async () => {
@@ -5,6 +5,7 @@ import * as path from 'path';
// Lazy import so tests can run before implementation
let decodeIdToken: (idToken: string) => { email?: string; plan_type?: string; account_id?: string };
let hasStructurallyValidIdToken: (idToken: string) => boolean;
// Fixture: real-shape JWT with nested claims
// Payload (base64url): {"email":"user@example.com","https://api.openai.com/auth":{"chatgpt_plan_type":"pro","chatgpt_account_id":"4b0448c0-e4a2-4cc0-a70d-77065d613553"}}
@@ -62,6 +63,7 @@ function buildToken(payload: Record<string, unknown>): string {
beforeEach(async () => {
const mod = await import('../../../src/codex-auth/decode-id-token');
decodeIdToken = mod.decodeIdToken;
hasStructurallyValidIdToken = mod.hasStructurallyValidIdToken;
});
describe('decodeIdToken', () => {
@@ -121,4 +123,14 @@ describe('decodeIdToken', () => {
expect(() => decodeIdToken('')).not.toThrow();
expect(decodeIdToken('')).toEqual({});
});
it('reports valid sparse JWT payloads as structurally valid', () => {
expect(hasStructurallyValidIdToken(buildToken({}))).toBe(true);
});
it('rejects JWT segments with invalid base64url characters', () => {
const [header, payload, signature] = buildToken({}).split('.');
expect(hasStructurallyValidIdToken(`${header}.${payload}$.${signature}`)).toBe(false);
expect(hasStructurallyValidIdToken(`${header}=.${payload}.${signature}`)).toBe(false);
});
});
+14 -2
View File
@@ -95,9 +95,21 @@ describe('formatExport — pwsh', () => {
});
describe('formatExport — cmd', () => {
it('uses set KEY=VALUE syntax without quotes', () => {
it('uses quoted set assignment syntax', () => {
expect(formatExport('cmd', 'CODEX_HOME', 'C:\\Users\\foo\\.ccs\\codex-instances\\work')).toBe(
'set CODEX_HOME=C:\\Users\\foo\\.ccs\\codex-instances\\work'
'set "CODEX_HOME=C:\\Users\\foo\\.ccs\\codex-instances\\work"'
);
});
it('keeps cmd metacharacters inside the quoted set assignment', () => {
expect(formatExport('cmd', 'CODEX_HOME', 'C:\\Users\\Kai & Co\\x|y<z>')).toBe(
'set "CODEX_HOME=C:\\Users\\Kai & Co\\x|y<z>"'
);
});
it('escapes cmd expansion-sensitive characters', () => {
expect(formatExport('cmd', 'CODEX_HOME', 'C:\\Users\\100% ^ "quoted"')).toBe(
'set "CODEX_HOME=C:\\Users\\100%% ^^ ^"quoted^""'
);
});
});