fix(codex-auth): close remaining review gaps

This commit is contained in:
Tam Nhu Tran committed 2026-05-17 16:51:22 -04:00
1 parent 08fe63c626
commit 5c79df4311
20 files changed
+823 -93

No files matched your search

@@ -123,6 +123,45 @@ function captureOutput(): { stderr: string[]; restore: () => void } {
};
}
function mockProcessTable(pgrepStdout: string, psStdout: string) {
spyOn(childProcess, 'spawnSync').mockImplementation(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(cmd: string, _args: string[]): any => {
if (cmd === 'pgrep') {
return {
status: pgrepStdout.trim().length > 0 ? 0 : 1,
stdout: pgrepStdout,
stderr: '',
pid: 0,
output: [],
signal: null,
error: undefined,
};
}
if (cmd === 'ps') {
return {
status: psStdout.trim().length > 0 ? 0 : 1,
stdout: psStdout,
stderr: '',
pid: 0,
output: [],
signal: null,
error: undefined,
};
}
return {
status: 1,
stdout: '',
stderr: '',
pid: 0,
output: [],
signal: null,
error: undefined,
};
}
);
}
// ─────────────────────────────────────────────────────────────────────────────
describe('import-default — missing legacy auth.json', () => {
@@ -385,22 +424,43 @@ describe('import-default — torn-write retry', () => {
expect(exitCalled).toBe(true);
expect(ctx.registry.hasProfile('bad-base64url')).toBe(false);
});
it('rejects an id_token signature with impossible base64url length', async () => {
const authPath = path.join(legacyCodexHome, 'auth.json');
const [header, payload] = VALID_JWT.split('.');
fs.writeFileSync(authPath, JSON.stringify({ tokens: { id_token: `${header}.${payload}.a` } }));
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-signature']);
} catch {
/* expected */
} finally {
restore();
process.exit = origExit;
}
expect(exitCalled).toBe(true);
expect(ctx.registry.hasProfile('bad-signature')).toBe(false);
});
});
describe('import-default — Codex running detection', () => {
it('warns and refuses when pgrep finds a codex PID', async () => {
fs.writeFileSync(path.join(legacyCodexHome, 'auth.json'), VALID_AUTH_JSON);
// Mock spawnSync to simulate pgrep finding codex
spyOn(childProcess, 'spawnSync').mockReturnValue({
status: 0,
stdout: '12345\n',
stderr: '',
pid: 0,
output: [],
signal: null,
error: undefined,
});
mockProcessTable('12345\n', '12345 /usr/local/bin/codex login\n');
const { handleImportDefaultCodex } = await import(
'../../../../src/codex-auth/commands/import-default-command'
@@ -431,15 +491,7 @@ describe('import-default — Codex running detection', () => {
it('proceeds with --force-while-running even when Codex is running', async () => {
fs.writeFileSync(path.join(legacyCodexHome, 'auth.json'), VALID_AUTH_JSON);
spyOn(childProcess, 'spawnSync').mockReturnValue({
status: 0,
stdout: '12345\n',
stderr: '',
pid: 0,
output: [],
signal: null,
error: undefined,
});
mockProcessTable('12345\n', '12345 /usr/local/bin/codex login\n');
const { handleImportDefaultCodex } = await import(
'../../../../src/codex-auth/commands/import-default-command'
@@ -456,6 +508,57 @@ describe('import-default — Codex running detection', () => {
// Should have proceeded and created the profile
expect(ctx.registry.hasProfile('forcerunning')).toBe(true);
});
it('warns and refuses when Codex is running through a node shim', async () => {
fs.writeFileSync(path.join(legacyCodexHome, 'auth.json'), VALID_AUTH_JSON);
mockProcessTable('12345\n', '12345 /usr/bin/node /usr/local/bin/codex login\n');
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 captured = captureOutput();
try {
await handleImportDefaultCodex(ctx, ['nodeshim']);
} catch {
/* expected */
} finally {
captured.restore();
process.exit = origExit;
}
expect(exitCalled).toBe(true);
expect(captured.stderr.join('')).toContain('12345');
expect(ctx.registry.hasProfile('nodeshim')).toBe(false);
});
it('ignores pgrep false positives that are not Codex executables', async () => {
fs.writeFileSync(path.join(legacyCodexHome, 'auth.json'), VALID_AUTH_JSON);
mockProcessTable('11111\n', '11111 /usr/bin/node /tmp/codex-auth-helper.js\n');
const { handleImportDefaultCodex } = await import(
'../../../../src/codex-auth/commands/import-default-command'
);
const ctx = await makeCtx();
const restore = silenceConsole();
try {
await handleImportDefaultCodex(ctx, ['falsepositive']);
} finally {
restore();
}
expect(ctx.registry.hasProfile('falsepositive')).toBe(true);
});
});
describe('import-default — --with-history', () => {
@@ -53,6 +53,12 @@ function spawnReturnsCode(code: number, writeAuth = false) {
});
}
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`;
}
describe('handleLoginCodex — binary missing', () => {
it('exits with BINARY_ERROR when codex not found', async () => {
const detectorMod = await import('../../../../src/targets/codex-detector');
@@ -149,4 +155,57 @@ describe('handleLoginCodex — clean exit updates registry', () => {
// last_used should now be set
expect(meta.last_used).toBeTruthy();
});
it('persists 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-login-only',
},
}),
},
})
);
}
const ee = {
on: (evt: string, cb: (code: number) => void) => {
if (evt === 'exit') setTimeout(() => cb(0), 0);
return ee;
},
};
return ee as ReturnType<typeof childProcess.spawn>;
}
);
const { handleLoginCodex } = await import('../../../../src/codex-auth/commands/login-command');
const ctx = await makeCtx();
ctx.registry.createProfile('accountonly', {
created: new Date().toISOString(),
last_used: null,
});
const origLog = console.log;
console.log = () => {};
try {
await handleLoginCodex(ctx, ['accountonly']);
} finally {
console.log = origLog;
}
const meta = ctx.registry.getProfile('accountonly');
expect(meta.last_used).toBeTruthy();
expect(meta.account_id).toBe('acct-login-only');
});
});
@@ -228,6 +228,52 @@ describe('handleRemoveCodex — confirmation', () => {
expect(ctx.registry.hasProfile('preserveme')).toBe(true);
});
it('cleans a partial preservation copy when delete preparation fails', async () => {
const { handleRemoveCodex } = await import(
'../../../../src/codex-auth/commands/remove-command'
);
const ctx = await makeCtx('copyfail');
const profileDir = path.join(ccsHome, '.ccs', 'codex-instances', 'copyfail');
const parentDir = path.dirname(profileDir);
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) => {
if (typeof dest === 'string' && dest.includes('.preserved.')) {
fs.mkdirSync(dest, { recursive: true });
fs.writeFileSync(path.join(dest, 'auth.json'), '{}');
throw new Error('copy failed after partial write');
}
return realCpSync(src, dest, 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, ['copyfail', '--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('copyfail')).toBe(true);
expect(fs.readdirSync(parentDir).some((entry) => entry.startsWith('copyfail.preserved.'))).toBe(
false
);
});
it('restores profile data and registry when final deletion fails', async () => {
const { handleRemoveCodex } = await import(
'../../../../src/codex-auth/commands/remove-command'
@@ -269,4 +315,45 @@ describe('handleRemoveCodex — confirmation', () => {
expect(ctx.registry.hasProfile('restoreme')).toBe(true);
expect(ctx.registry.getDefault()).toBe('restoreme');
});
it('restores from preserved copy when final deletion partially removes auth.json', async () => {
const { handleRemoveCodex } = await import(
'../../../../src/codex-auth/commands/remove-command'
);
const ctx = await makeCtx('partialrestore');
const profileDir = path.join(ccsHome, '.ccs', 'codex-instances', 'partialrestore');
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.')) {
realRmSync(path.join(target, 'auth.json'), { force: true });
throw new Error('delete failed after auth removal');
}
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, ['partialrestore', '--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('partialrestore')).toBe(true);
});
});
@@ -56,6 +56,12 @@ async function captureStdout(fn: () => Promise<void>): Promise<string> {
return chunks.join('');
}
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`;
}
// ── empty list ────────────────────────────────────────────────────────────────
describe('handleShowCodex — empty list', () => {
@@ -82,6 +88,48 @@ describe('handleShowCodex — default marker', () => {
});
});
// ── JSON account metadata ───────────────────────────────────────────────────
describe('handleShowCodex — JSON account_id', () => {
it('includes account_id from registry metadata or auth.json identity', async () => {
const { handleShowCodex } = await import('../../../../src/codex-auth/commands/show-command');
const ctx = await makeCtx('registryid', 'authid');
ctx.registry.updateProfile('registryid', {
account_id: 'acct-from-registry',
email: 'registry@example.com',
});
const authProfileDir = path.join(ccsHome, '.ccs', 'codex-instances', 'authid');
fs.mkdirSync(authProfileDir, { recursive: true });
fs.writeFileSync(
path.join(authProfileDir, 'auth.json'),
JSON.stringify({
tokens: {
id_token: buildToken({
email: 'auth@example.com',
'https://api.openai.com/auth': {
chatgpt_plan_type: 'plus',
chatgpt_account_id: 'acct-from-auth-json',
},
}),
},
})
);
const out = await captureStdout(() => handleShowCodex(ctx, ['--json']));
const parsed = JSON.parse(out) as {
profiles: Array<{ name: string; account_id: string | null; email: string | null }>;
};
expect(parsed.profiles.find((p) => p.name === 'registryid')?.account_id).toBe(
'acct-from-registry'
);
expect(parsed.profiles.find((p) => p.name === 'authid')?.account_id).toBe(
'acct-from-auth-json'
);
});
});
// ── active(missing) row at top (D14) ─────────────────────────────────────────
describe('handleShowCodex — active(missing) at top', () => {
@@ -120,6 +168,19 @@ describe('handleShowCodex — detail view', () => {
expect(out).toContain('<unknown>');
});
it('detail JSON includes account_id from registry metadata when auth.json is missing', async () => {
const { handleShowCodex } = await import('../../../../src/codex-auth/commands/show-command');
const ctx = await makeCtx('registrydetail');
ctx.registry.updateProfile('registrydetail', {
account_id: 'acct-from-registry-detail',
});
const out = await captureStdout(() => handleShowCodex(ctx, ['registrydetail', '--json']));
const parsed = JSON.parse(out) as { account_id: string | null };
expect(parsed.account_id).toBe('acct-from-registry-detail');
});
it('does not crash with malformed auth.json', async () => {
const { handleShowCodex } = await import('../../../../src/codex-auth/commands/show-command');
const ctx = await makeCtx('malformed');
@@ -133,4 +133,10 @@ describe('decodeIdToken', () => {
expect(hasStructurallyValidIdToken(`${header}.${payload}$.${signature}`)).toBe(false);
expect(hasStructurallyValidIdToken(`${header}=.${payload}.${signature}`)).toBe(false);
});
it('rejects JWT segments with impossible base64url length', () => {
const [header, payload] = buildToken({}).split('.');
expect(hasStructurallyValidIdToken(`${header}.${payload}.a`)).toBe(false);
expect(decodeIdToken(`${header}.${payload}.a`)).toEqual({});
});
});
@@ -76,15 +76,27 @@ describe('resolveActiveProfile', () => {
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');
});
it('throws when CCS_CODEX_PROFILE is set and registry YAML is corrupt', () => {
fs.mkdirSync(path.dirname(registryPath), { recursive: true });
fs.writeFileSync(registryPath, '{ invalid yaml: [[[', { mode: 0o600 });
expect(() => resolveActiveProfile({ CCS_CODEX_PROFILE: 'work' })).toThrow(
/Refusing to fall back to ~\/\.codex/
);
let thrown: unknown;
try {
resolveActiveProfile({ CCS_CODEX_PROFILE: 'work' });
} catch (err) {
thrown = err;
}
expect(thrown).toBeDefined();
expect(String(thrown)).toContain('Refusing to fall back to ~/.codex');
expect(String(thrown)).toContain('$CCS_HOME/.ccs/codex-profiles.yaml');
expect(String(thrown)).not.toContain(registryPath);
expect(String(thrown)).not.toContain('invalid yaml');
});
it('returns source=env when CCS_CODEX_PROFILE matches a registry entry', () => {
+40 -8
View File
@@ -31,16 +31,43 @@ describe('detectShell — Unix', () => {
});
describe('detectShell — Windows', () => {
it('returns pwsh when PSModulePath is set', () => {
expect(detectShell({ PSModulePath: 'C:\\Windows\\system32\\...' }, 'win32')).toBe('pwsh');
it('does not treat PSModulePath alone as PowerShell', () => {
expect(
detectShell(
{ PSModulePath: 'C:\\Windows\\system32\\...', ComSpec: 'C:\\Windows\\System32\\cmd.exe' },
'win32'
)
).toBe('cmd');
});
it('returns cmd when PSModulePath is absent', () => {
expect(detectShell({}, 'win32')).toBe('cmd');
it('returns pwsh when SHELL points to PowerShell', () => {
expect(
detectShell(
{
SHELL: 'C:\\Program Files\\PowerShell\\7\\pwsh.exe',
PSModulePath: 'C:\\Windows\\system32\\...',
},
'win32'
)
).toBe('pwsh');
});
it('ignores SHELL on Windows — uses PSModulePath heuristic', () => {
expect(detectShell({ SHELL: '/bin/bash', PSModulePath: 'C:\\ps' }, 'win32')).toBe('pwsh');
it('returns pwsh when the parent process is PowerShell and ComSpec points to cmd', () => {
expect(
detectShell(
{ PSModulePath: 'C:\\Windows\\system32\\...', ComSpec: 'C:\\Windows\\System32\\cmd.exe' },
'win32',
'pwsh.exe'
)
).toBe('pwsh');
});
it('honors Git Bash style SHELL on Windows', () => {
expect(detectShell({ SHELL: '/usr/bin/bash', PSModulePath: 'C:\\ps' }, 'win32')).toBe('bash');
});
it('returns cmd when no explicit shell hint is available', () => {
expect(detectShell({ PSModulePath: 'C:\\ps' }, 'win32')).toBe('cmd');
});
});
@@ -92,6 +119,11 @@ describe('formatExport — pwsh', () => {
const result = formatExport('pwsh', 'X', 'say "hello"');
expect(result).toBe('$env:X = "say ""hello"""');
});
it('escapes backticks before interpolation-sensitive characters', () => {
const result = formatExport('pwsh', 'CODEX_HOME', 'C:\\Users\\kai`$tmp');
expect(result).toBe('$env:CODEX_HOME = "C:\\Users\\kai```$tmp"');
});
});
describe('formatExport — cmd', () => {
@@ -108,8 +140,8 @@ describe('formatExport — cmd', () => {
});
it('escapes cmd expansion-sensitive characters', () => {
expect(formatExport('cmd', 'CODEX_HOME', 'C:\\Users\\100% ^ "quoted"')).toBe(
'set "CODEX_HOME=C:\\Users\\100%% ^^ ^"quoted^""'
expect(formatExport('cmd', 'CODEX_HOME', 'C:\\Users\\100% ^ "quoted" !bang!')).toBe(
'set "CODEX_HOME=C:\\Users\\100%% ^^ ^"quoted^" ^^!bang^^!"'
);
});
});