From 13f32d49653f3a3be5fc3135e4b8e445900c7443 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 17 Jul 2026 02:26:38 -0700 Subject: [PATCH 1/2] fix(codex): guard OAuth add for custom providers --- .../runtime-home-service.test.ts | 24 +++++ src/main/codex-accounts/service.test.ts | 52 +++++++++- src/main/codex-accounts/service.ts | 27 +++++- .../codex-config-path-reference-rewrite.ts | 95 +----------------- src/main/codex/codex-model-provider-config.ts | 26 +++++ src/main/codex/config-toml-line-scan.ts | 97 +++++++++++++++++++ 6 files changed, 223 insertions(+), 98 deletions(-) create mode 100644 src/main/codex/codex-model-provider-config.ts diff --git a/src/main/codex-accounts/runtime-home-service.test.ts b/src/main/codex-accounts/runtime-home-service.test.ts index 513385b5f24..fa1e111706f 100644 --- a/src/main/codex-accounts/runtime-home-service.test.ts +++ b/src/main/codex-accounts/runtime-home-service.test.ts @@ -1441,6 +1441,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 9b185e4c357..17c4de9a3eb 100644 --- a/src/main/codex-accounts/service.test.ts +++ b/src/main/codex-accounts/service.test.ts @@ -502,7 +502,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( @@ -564,6 +565,55 @@ describe('CodexAccountService config sync', () => { expect(runtimeHome.syncForCurrentSelection).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 2a0fb54bdc4..dff595168b1 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -16,6 +16,7 @@ import type { import type { CodexRuntimeHomeService } from './runtime-home-service' import { writeFileAtomically } from './fs-utils' import { rewriteRelativePathConfigValues } from '../codex/codex-config-path-reference-rewrite' +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' @@ -130,7 +131,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) { @@ -434,9 +437,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) } @@ -516,6 +522,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.ts b/src/main/codex/codex-model-provider-config.ts new file mode 100644 index 00000000000..9851e994aa4 --- /dev/null +++ b/src/main/codex/codex-model-provider-config.ts @@ -0,0 +1,26 @@ +import { + createTomlLineScanState, + getTomlTableHeader, + isTomlStructuralLine, + parseTomlSingleLineStringValue, + 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() + for (const line of config.split('\n')) { + if (isTomlStructuralLine(state)) { + if (getTomlTableHeader(line)) { + return null + } + const match = /^[ \t]*(?:model_provider|"model_provider"|'model_provider')[ \t]*=/.exec(line) + if (match) { + return parseTomlSingleLineStringValue(line, match[0].length)?.value ?? null + } + } + state = updateTomlLineScanState(state, line) + } + return null +} diff --git a/src/main/codex/config-toml-line-scan.ts b/src/main/codex/config-toml-line-scan.ts index c0136be9e32..542b9f8db56 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,47 @@ 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 === 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 skipTomlBasicString(line: string, startIndex: number): number { let index = startIndex while (index < line.length) { @@ -113,3 +160,53 @@ function skipTomlLiteralString(line: string, startIndex: number): number { const endIndex = line.indexOf("'", startIndex) return endIndex === -1 ? line.length : endIndex + 1 } + +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 + } +} From 1c328c56cbeeaf3f237a976f994a45eff3ec5ce3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 17 Jul 2026 02:37:30 -0700 Subject: [PATCH 2/2] fix(codex): parse multiline provider pins in OAuth guard --- .../codex/codex-model-provider-config.test.ts | 63 ++++++++++++++ src/main/codex/codex-model-provider-config.ts | 31 ++++++- src/main/codex/config-toml-line-scan.ts | 86 +++++++++++++++++++ 3 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 src/main/codex/codex-model-provider-config.test.ts 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 index 9851e994aa4..6a659804858 100644 --- a/src/main/codex/codex-model-provider-config.ts +++ b/src/main/codex/codex-model-provider-config.ts @@ -3,6 +3,7 @@ import { getTomlTableHeader, isTomlStructuralLine, parseTomlSingleLineStringValue, + parseTomlStringValue, updateTomlLineScanState } from './config-toml-line-scan' @@ -10,17 +11,41 @@ import { // 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 match = /^[ \t]*(?:model_provider|"model_provider"|'model_provider')[ \t]*=/.exec(line) - if (match) { - return parseTomlSingleLineStringValue(line, match[0].length)?.value ?? 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 542b9f8db56..48216621636 100644 --- a/src/main/codex/config-toml-line-scan.ts +++ b/src/main/codex/config-toml-line-scan.ts @@ -122,6 +122,9 @@ export function parseTomlSingleLineStringValue( 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 } } @@ -140,6 +143,58 @@ export function parseTomlSingleLineStringValue( 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) { @@ -161,6 +216,37 @@ function skipTomlLiteralString(line: string, startIndex: number): number { 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