diff --git a/src/main/codex-accounts/runtime-home-service.test.ts b/src/main/codex-accounts/runtime-home-service.test.ts index 22ce038f45a..be90313c231 100644 --- a/src/main/codex-accounts/runtime-home-service.test.ts +++ b/src/main/codex-accounts/runtime-home-service.test.ts @@ -1893,6 +1893,30 @@ describe('CodexRuntimeHomeService', () => { ) }) + it('launches the system-default custom provider without requiring OAuth auth', async () => { + const systemCodexHome = getSystemCodexHomePath() + const canonicalConfigPath = join(systemCodexHome, 'config.toml') + const canonicalConfig = [ + 'model_provider = "codex-lb"', + '', + '[model_providers.codex-lb]', + 'base_url = "https://codex-lb.example.test/v1"', + 'env_key = "CODEX_LB_API_KEY"', + '' + ].join('\n') + writeFileSync(canonicalConfigPath, canonicalConfig, 'utf-8') + const store = createStore(createSettings({ codexSystemDefaultRealHomeEnabled: false })) + const { CodexRuntimeHomeService } = await import('./runtime-home-service') + const service = new CodexRuntimeHomeService(store as never) + + expect(service.prepareForCodexLaunch()).toBe(getRuntimeCodexHomePath()) + expect(readFileSync(join(getRuntimeCodexHomePath(), 'config.toml'), 'utf-8')).toBe( + canonicalConfig + ) + expect(existsSync(getRuntimeCodexAuthPath())).toBe(false) + expect(readFileSync(canonicalConfigPath, 'utf-8')).toBe(canonicalConfig) + }) + it('links system Codex user resources into the managed runtime home before launch', async () => { const systemCodexHome = getSystemCodexHomePath() mkdirSync(join(systemCodexHome, 'skills', 'review'), { recursive: true }) diff --git a/src/main/codex-accounts/service.test.ts b/src/main/codex-accounts/service.test.ts index 4b4511e3c8d..ba827190cfc 100644 --- a/src/main/codex-accounts/service.test.ts +++ b/src/main/codex-accounts/service.test.ts @@ -667,7 +667,8 @@ describe('CodexAccountService config sync', () => { vi.resetModules() const canonicalConfigPath = join(testState.fakeHomeDir, '.codex', 'config.toml') - const canonicalConfig = 'approval_policy = "never"\nsandbox_mode = "danger-full-access"\n' + const canonicalConfig = + 'model_provider = "openai"\napproval_policy = "never"\nsandbox_mode = "danger-full-access"\n' writeFileSync(canonicalConfigPath, canonicalConfig, 'utf-8') const spawnMock = vi.fn( @@ -788,6 +789,55 @@ describe('CodexAccountService config sync', () => { expect(spawnMock).toHaveBeenCalledTimes(1) }) + it('rejects OAuth account add when canonical config pins a custom provider', async () => { + vi.resetModules() + + const canonicalConfigPath = join(testState.fakeHomeDir, '.codex', 'config.toml') + const canonicalConfig = [ + 'model_provider = "codex-lb"', + 'model = "gpt-5.2-codex"', + '', + '[model_providers.codex-lb]', + 'name = "Codex load balancer"', + 'base_url = "https://codex-lb.example.test/v1"', + 'env_key = "CODEX_LB_API_KEY"', + '' + ].join('\n') + writeFileSync(canonicalConfigPath, canonicalConfig, 'utf-8') + + const spawnMock = vi.fn() + vi.doMock('node:crypto', () => ({ randomUUID: () => 'account-id-for-test' })) + vi.doMock('node:child_process', () => ({ execFileSync: vi.fn(), spawn: spawnMock })) + + try { + const settings = createSettings() + const store = createStore(settings) + const rateLimits = createRateLimits() + const runtimeHome = createRuntimeHome() + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + store as never, + rateLimits as never, + runtimeHome as never + ) + + await expect(service.addAccount()).rejects.toThrow( + 'Orca cannot add a Codex OAuth account while ~/.codex/config.toml pins the custom provider "codex-lb". Keep using the system-default account for this provider, or remove model_provider (or set it to "openai") before adding an OAuth account. Orca left your config unchanged.' + ) + + expect(spawnMock).not.toHaveBeenCalled() + expect(store.updateSettings).not.toHaveBeenCalled() + expect(runtimeHome.syncForCurrentSelection).not.toHaveBeenCalled() + expect(existsSync(join(testState.userDataDir, 'codex-accounts', 'account-id-for-test'))).toBe( + false + ) + expect(readFileSync(canonicalConfigPath, 'utf-8')).toBe(canonicalConfig) + } finally { + vi.doUnmock('node:crypto') + vi.doUnmock('node:child_process') + } + }) + it('recreates the expected missing managed home before reauthenticating', async () => { vi.resetModules() diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index cab0554de1d..4abb6ad19bd 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -23,6 +23,7 @@ import { getCodexManagedHookInstallMaterial } from '../codex/hook-service' import { syncSystemConfigIntoManagedCodexHome } from '../codex/codex-config-mirror' import { getSystemCodexHomePath } from '../codex/codex-home-paths' import { MANAGED_HOOK_TIMEOUT_SECONDS } from '../agent-hooks/installer-utils' +import { readCodexTopLevelModelProvider } from '../codex/codex-model-provider-config' import { resolveCodexCommand } from '../codex-cli/command' import type { Store } from '../persistence' import type { RateLimitService } from '../rate-limits/service' @@ -139,7 +140,9 @@ export class CodexAccountService { const { managedHomePath } = managedHome try { - this.safeSyncCanonicalConfigIntoManagedHome(managedHomePath) + const canonicalConfig = this.readCanonicalConfigForManagedHome(managedHomePath) + this.assertOAuthAccountAddAllowed(canonicalConfig) + this.safeSyncCanonicalConfigIntoManagedHome(managedHomePath, canonicalConfig) await this.runCodexLogin(managedHomePath) const identity = this.readIdentityFromHome(managedHomePath) if (!identity.email) { @@ -537,9 +540,12 @@ export class CodexAccountService { } } - private safeSyncCanonicalConfigIntoManagedHome(managedHomePath: string): void { + private safeSyncCanonicalConfigIntoManagedHome( + managedHomePath: string, + canonicalConfig?: CanonicalCodexConfig | null + ): void { try { - this.syncCanonicalConfigIntoManagedHome(managedHomePath) + this.syncCanonicalConfigIntoManagedHome(managedHomePath, canonicalConfig) } catch (error) { console.warn('[codex-accounts] Failed to seed managed config:', error) } @@ -659,6 +665,21 @@ export class CodexAccountService { } } + private assertOAuthAccountAddAllowed(canonicalConfig: CanonicalCodexConfig | null): void { + const modelProvider = canonicalConfig + ? readCodexTopLevelModelProvider(canonicalConfig.contents) + : null + if (!modelProvider || modelProvider === 'openai') { + return + } + + // Why: mirroring a custom-provider pin into an OAuth managed home makes + // the new OAuth credentials inert; fail before login and leave user config intact. + throw new Error( + `Orca cannot add a Codex OAuth account while ~/.codex/config.toml pins the custom provider ${JSON.stringify(modelProvider)}. Keep using the system-default account for this provider, or remove model_provider (or set it to "openai") before adding an OAuth account. Orca left your config unchanged.` + ) + } + private writeManagedConfig(managedHomePath: string, contents: string): void { const configPath = join(managedHomePath, 'config.toml') try { diff --git a/src/main/codex/codex-config-path-reference-rewrite.ts b/src/main/codex/codex-config-path-reference-rewrite.ts index 94e29db3d8f..542ce64a824 100644 --- a/src/main/codex/codex-config-path-reference-rewrite.ts +++ b/src/main/codex/codex-config-path-reference-rewrite.ts @@ -3,6 +3,7 @@ import { createTomlLineScanState, getTomlTableHeader, isTomlStructuralLine, + parseTomlSingleLineStringValue, updateTomlLineScanState } from './config-toml-line-scan' @@ -22,12 +23,6 @@ const EXACT_PATH_CONFIG_KEYS = new Set([ 'sqlite_home' ]) -type ParsedTomlString = { - value: string - start: number - end: number -} - // Why: Orca mirrors config.toml into a managed CODEX_HOME, but Codex resolves // path-valued config settings from the file it read. Keep user-owned assets in // ~/.codex reachable after the mirror moves the TOML. Best-effort by design: @@ -151,94 +146,6 @@ function quoteTomlBasicString(value: string): string { return `${quoted}"` } -function parseTomlSingleLineStringValue(line: string, offset: number): ParsedTomlString | null { - let index = offset - while (line[index] === ' ' || line[index] === '\t') { - index += 1 - } - - if (line.startsWith('"""', index) || line.startsWith("'''", index)) { - return null - } - - const quote = line[index] - if (quote !== '"' && quote !== "'") { - return null - } - - const start = index - index += 1 - let value = '' - while (index < line.length) { - const char = line[index] - if (char === quote) { - return { value, start, end: index + 1 } - } - if (quote === '"' && char === '\\') { - const escaped = parseTomlBasicStringEscape(line, index) - if (!escaped) { - return null - } - value += escaped.value - index = escaped.nextIndex - continue - } - value += char - index += 1 - } - return null -} - -function parseTomlBasicStringEscape( - line: string, - slashIndex: number -): { value: string; nextIndex: number } | null { - const escaped = line[slashIndex + 1] - switch (escaped) { - case 'b': - return { value: '\b', nextIndex: slashIndex + 2 } - case 't': - return { value: '\t', nextIndex: slashIndex + 2 } - case 'n': - return { value: '\n', nextIndex: slashIndex + 2 } - case 'f': - return { value: '\f', nextIndex: slashIndex + 2 } - case 'r': - return { value: '\r', nextIndex: slashIndex + 2 } - case '"': - case '\\': - return { value: escaped, nextIndex: slashIndex + 2 } - case 'u': - return parseTomlUnicodeEscape(line, slashIndex + 2, 4) - case 'U': - return parseTomlUnicodeEscape(line, slashIndex + 2, 8) - default: - return null - } -} - -function parseTomlUnicodeEscape( - line: string, - start: number, - length: number -): { value: string; nextIndex: number } | null { - const raw = line.slice(start, start + length) - if (!new RegExp(`^[0-9a-fA-F]{${length}}$`).test(raw)) { - return null - } - const codePoint = Number.parseInt(raw, 16) - // Why: TOML escapes must be Unicode scalar values; String.fromCodePoint - // accepts lone surrogates, which would round-trip into invalid TOML. - if (codePoint >= 0xd800 && codePoint <= 0xdfff) { - return null - } - try { - return { value: String.fromCodePoint(codePoint), nextIndex: start + length } - } catch { - return null - } -} - function getTomlHeaderPath(header: string): string { const trimmed = header.trim() if (trimmed.startsWith('[[') && trimmed.endsWith(']]')) { diff --git a/src/main/codex/codex-model-provider-config.test.ts b/src/main/codex/codex-model-provider-config.test.ts new file mode 100644 index 00000000000..1304946768e --- /dev/null +++ b/src/main/codex/codex-model-provider-config.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from 'vitest' +import { readCodexTopLevelModelProvider } from './codex-model-provider-config' + +describe('readCodexTopLevelModelProvider', () => { + it.each([ + ['missing provider', 'model = "gpt-5"\n', null], + ['OpenAI provider', 'model_provider = "openai" # default\n', 'openai'], + ['literal custom provider', "model_provider = 'codex-lb'\n", 'codex-lb'], + ['quoted key', '"model_provider" = "codex-lb"\n', 'codex-lb'], + ['escaped quoted key', '"model\\u005Fprovider" = "codex-lb"\n', 'codex-lb'], + ['escaped provider', 'model_provider = "codex\\u002Dlb"\n', 'codex-lb'], + ['single-line multiline OpenAI', 'model_provider = """openai"""\n', 'openai'], + ['single-line multiline custom', "model_provider = '''codex-lb'''\n", 'codex-lb'], + ['CRLF custom provider', 'model = "gpt-5"\r\nmodel_provider = "codex-lb"\r\n', 'codex-lb'], + [ + 'continued multiline custom', + ['model_provider = """\\', ' codex-lb"""', ''].join('\n'), + 'codex-lb' + ], + ['profile provider', ['[profiles.custom]', 'model_provider = "codex-lb"', ''].join('\n'), null], + [ + 'provider-like multiline data', + [ + 'developer_instructions = """', + '[profiles.custom]', + 'model_provider = "not-a-setting"', + '"""', + 'model_provider = "openai"', + '' + ].join('\n'), + 'openai' + ], + [ + 'provider-like array data', + [ + 'notify = [', + ' "model_provider = \\"not-a-setting\\"",', + ']', + 'model_provider = "openai"', + '' + ].join('\n'), + 'openai' + ] + ])('reads %s', (_description, config, expected) => { + expect(readCodexTopLevelModelProvider(config)).toBe(expected) + }) + + it('uses the first duplicate assignment deterministically', () => { + expect( + readCodexTopLevelModelProvider( + ['model_provider = "openai"', 'model_provider = "codex-lb"', ''].join('\n') + ) + ).toBe('openai') + }) + + it.each([ + ['unquoted value', 'model_provider = codex-lb\n'], + ['unterminated value', 'model_provider = "codex-lb\n'], + ['assignment after a table', '[profiles.default]\nmodel_provider = "codex-lb"\n'] + ])('ignores malformed or non-root %s', (_description, config) => { + expect(readCodexTopLevelModelProvider(config)).toBeNull() + }) +}) diff --git a/src/main/codex/codex-model-provider-config.ts b/src/main/codex/codex-model-provider-config.ts new file mode 100644 index 00000000000..6a659804858 --- /dev/null +++ b/src/main/codex/codex-model-provider-config.ts @@ -0,0 +1,51 @@ +import { + createTomlLineScanState, + getTomlTableHeader, + isTomlStructuralLine, + parseTomlSingleLineStringValue, + parseTomlStringValue, + updateTomlLineScanState +} from './config-toml-line-scan' + +// Why: only the root setting selects the provider for every OAuth login; +// similarly named keys inside profiles or provider tables are not global pins. +export function readCodexTopLevelModelProvider(config: string): string | null { + let state = createTomlLineScanState() + let lineOffset = 0 + for (const line of config.split('\n')) { + if (isTomlStructuralLine(state)) { + if (getTomlTableHeader(line)) { + return null + } + const valueOffset = getModelProviderValueOffset(line) + if (valueOffset !== null) { + return parseTomlStringValue(config, lineOffset + valueOffset)?.value ?? null + } + } + state = updateTomlLineScanState(state, line) + lineOffset += line.length + 1 + } + return null +} + +function getModelProviderValueOffset(line: string): number | null { + let index = 0 + while (line[index] === ' ' || line[index] === '\t') { + index += 1 + } + + if (line.startsWith('model_provider', index)) { + index += 'model_provider'.length + } else { + const parsedKey = parseTomlSingleLineStringValue(line, index) + if (parsedKey?.value !== 'model_provider') { + return null + } + index = parsedKey.end + } + + while (line[index] === ' ' || line[index] === '\t') { + index += 1 + } + return line[index] === '=' ? index + 1 : null +} diff --git a/src/main/codex/config-toml-line-scan.ts b/src/main/codex/config-toml-line-scan.ts index c0136be9e32..48216621636 100644 --- a/src/main/codex/config-toml-line-scan.ts +++ b/src/main/codex/config-toml-line-scan.ts @@ -9,6 +9,12 @@ export type TomlLineScanState = { arrayDepth: number } +export type ParsedTomlString = { + value: string + start: number + end: number +} + type TomlMultilineMode = 'basic' | 'literal' | null export function createTomlLineScanState(): TomlLineScanState { @@ -93,6 +99,102 @@ export function getTomlTableHeader(line: string): string | null { return match?.[1] ?? null } +export function parseTomlSingleLineStringValue( + line: string, + offset: number +): ParsedTomlString | null { + let index = offset + while (line[index] === ' ' || line[index] === '\t') { + index += 1 + } + + if (line.startsWith('"""', index) || line.startsWith("'''", index)) { + return null + } + + const quote = line[index] + if (quote !== '"' && quote !== "'") { + return null + } + + const start = index + index += 1 + let value = '' + while (index < line.length) { + const char = line[index] + if (char === '\n' || char === '\r') { + return null + } + if (char === quote) { + return { value, start, end: index + 1 } + } + if (quote === '"' && char === '\\') { + const escaped = parseTomlBasicStringEscape(line, index) + if (!escaped) { + return null + } + value += escaped.value + index = escaped.nextIndex + continue + } + value += char + index += 1 + } + return null +} + +// Why: serde accepts multiline TOML strings for String settings such as +// model_provider, so safety classifiers cannot treat triple quotes as absent. +export function parseTomlStringValue(source: string, offset: number): ParsedTomlString | null { + let index = offset + while (source[index] === ' ' || source[index] === '\t') { + index += 1 + } + + const delimiter = source.startsWith('"""', index) + ? '"""' + : source.startsWith("'''", index) + ? "'''" + : null + if (!delimiter) { + return parseTomlSingleLineStringValue(source, offset) + } + + const start = index + index += delimiter.length + index = skipInitialTomlMultilineNewline(source, index) + let value = '' + while (index < source.length) { + if (source.startsWith(delimiter, index)) { + return { value, start, end: index + delimiter.length } + } + + if (delimiter === '"""' && source[index] === '\\') { + const continuationEnd = getTomlMultilineContinuationEnd(source, index) + if (continuationEnd !== null) { + index = continuationEnd + continue + } + const escaped = parseTomlBasicStringEscape(source, index) + if (!escaped) { + return null + } + value += escaped.value + index = escaped.nextIndex + continue + } + + if (source[index] === '\r') { + value += '\n' + index += source[index + 1] === '\n' ? 2 : 1 + continue + } + value += source[index] + index += 1 + } + return null +} + function skipTomlBasicString(line: string, startIndex: number): number { let index = startIndex while (index < line.length) { @@ -113,3 +215,84 @@ function skipTomlLiteralString(line: string, startIndex: number): number { const endIndex = line.indexOf("'", startIndex) return endIndex === -1 ? line.length : endIndex + 1 } + +function skipInitialTomlMultilineNewline(source: string, index: number): number { + if (source[index] === '\r' && source[index + 1] === '\n') { + return index + 2 + } + return source[index] === '\n' ? index + 1 : index +} + +function getTomlMultilineContinuationEnd(source: string, slashIndex: number): number | null { + let index = slashIndex + 1 + while (source[index] === ' ' || source[index] === '\t') { + index += 1 + } + if (source[index] === '\r' && source[index + 1] === '\n') { + index += 2 + } else if (source[index] === '\n') { + index += 1 + } else { + return null + } + + while ( + source[index] === ' ' || + source[index] === '\t' || + source[index] === '\n' || + source[index] === '\r' + ) { + index += 1 + } + return index +} + +function parseTomlBasicStringEscape( + line: string, + slashIndex: number +): { value: string; nextIndex: number } | null { + const escaped = line[slashIndex + 1] + switch (escaped) { + case 'b': + return { value: '\b', nextIndex: slashIndex + 2 } + case 't': + return { value: '\t', nextIndex: slashIndex + 2 } + case 'n': + return { value: '\n', nextIndex: slashIndex + 2 } + case 'f': + return { value: '\f', nextIndex: slashIndex + 2 } + case 'r': + return { value: '\r', nextIndex: slashIndex + 2 } + case '"': + case '\\': + return { value: escaped, nextIndex: slashIndex + 2 } + case 'u': + return parseTomlUnicodeEscape(line, slashIndex + 2, 4) + case 'U': + return parseTomlUnicodeEscape(line, slashIndex + 2, 8) + default: + return null + } +} + +function parseTomlUnicodeEscape( + line: string, + start: number, + length: number +): { value: string; nextIndex: number } | null { + const raw = line.slice(start, start + length) + if (!new RegExp(`^[0-9a-fA-F]{${length}}$`).test(raw)) { + return null + } + const codePoint = Number.parseInt(raw, 16) + // Why: TOML escapes must be Unicode scalar values; String.fromCodePoint + // accepts lone surrogates, which would round-trip into invalid TOML. + if (codePoint >= 0xd800 && codePoint <= 0xdfff) { + return null + } + try { + return { value: String.fromCodePoint(codePoint), nextIndex: start + length } + } catch { + return null + } +}