From e4cf810e147708077a370dd8cafb3d4caeb02dd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?JiayuWang=28=E7=8E=8B=E5=98=89=E5=AE=87=29?= <151589547+JiayuuWang@users.noreply.github.com> Date: Sat, 4 Apr 2026 22:37:59 +0800 Subject: [PATCH] fix: guard rawBaseUrl against the literal string "undefined" from env vars (#340) On Windows, shells can set OPENAI_BASE_URL to the literal string "undefined" when the variable is referenced without quotes while unset. The nullish-coalescing operator (??) does not catch this because "undefined" is a truthy string, causing resolveProviderRequest() to treat it as a real base URL. This broke the Codex transport check: (!rawBaseUrl && isCodexAlias(model)) evaluated as (false || true) = false so the transport was incorrectly set to chat_completions (issue #336). Fix: introduce asEnvUrl() which trims the value and rejects both empty strings and the sentinel string "undefined". Use it for all three rawBaseUrl sources (options.baseUrl, OPENAI_BASE_URL, OPENAI_API_BASE). Tests: add three new cases to the 'Codex provider config' describe block covering the empty-string, "undefined"-string, and options-override scenarios. Also add beforeEach/afterEach guards so individual tests cannot contaminate each other via env var state. Co-authored-by: Claude Sonnet 4.6 --- src/services/api/codexShim.test.ts | 38 +++++++++++++++++++++++++++++- src/services/api/providerConfig.ts | 21 +++++++++++++---- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/services/api/codexShim.test.ts b/src/services/api/codexShim.test.ts index 44f900ef..79ab085e 100644 --- a/src/services/api/codexShim.test.ts +++ b/src/services/api/codexShim.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test } from 'bun:test' +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { tmpdir } from 'node:os' @@ -46,6 +46,21 @@ async function collectStreamEventTypes(responseText: string): Promise } describe('Codex provider config', () => { + const originalOpenaiBaseUrl = process.env.OPENAI_BASE_URL + const originalOpenaiApiBase = process.env.OPENAI_API_BASE + + beforeEach(() => { + delete process.env.OPENAI_BASE_URL + delete process.env.OPENAI_API_BASE + }) + + afterEach(() => { + if (originalOpenaiBaseUrl === undefined) delete process.env.OPENAI_BASE_URL + else process.env.OPENAI_BASE_URL = originalOpenaiBaseUrl + if (originalOpenaiApiBase === undefined) delete process.env.OPENAI_API_BASE + else process.env.OPENAI_API_BASE = originalOpenaiApiBase + }) + test('resolves codexplan alias to Codex transport with reasoning', () => { const resolved = resolveProviderRequest({ model: 'codexplan' }) expect(resolved.transport).toBe('codex_responses') @@ -53,6 +68,27 @@ describe('Codex provider config', () => { expect(resolved.reasoning).toEqual({ effort: 'high' }) }) + test('resolves codexplan to Codex transport even when OPENAI_BASE_URL is the string "undefined"', () => { + // On Windows, env vars can leak as the literal string "undefined" instead of + // the JS value undefined when not properly unset (issue #336). + process.env.OPENAI_BASE_URL = 'undefined' + const resolved = resolveProviderRequest({ model: 'codexplan' }) + expect(resolved.transport).toBe('codex_responses') + }) + + test('resolves codexplan to Codex transport even when OPENAI_BASE_URL is an empty string', () => { + process.env.OPENAI_BASE_URL = '' + const resolved = resolveProviderRequest({ model: 'codexplan' }) + expect(resolved.transport).toBe('codex_responses') + }) + + test('prefers explicit baseUrl option over env var', () => { + process.env.OPENAI_BASE_URL = 'https://example.com/v1' + const resolved = resolveProviderRequest({ model: 'codexplan', baseUrl: 'https://chatgpt.com/backend-api/codex' }) + expect(resolved.transport).toBe('codex_responses') + expect(resolved.baseUrl).toBe('https://chatgpt.com/backend-api/codex') + }) + test('loads Codex credentials from auth.json fallback', () => { const authPath = createTempAuthJson({ tokens: { diff --git a/src/services/api/providerConfig.ts b/src/services/api/providerConfig.ts index 24de14c3..c8dd717c 100644 --- a/src/services/api/providerConfig.ts +++ b/src/services/api/providerConfig.ts @@ -112,7 +112,19 @@ function isPrivateIpv6Address(hostname: string): boolean { } function asTrimmedString(value: unknown): string | undefined { - return typeof value === 'string' && value.trim() ? value.trim() : undefined + if (typeof value !== 'string') return undefined + const trimmed = value.trim() + return trimmed ? trimmed : undefined +} + +// Reads an env-var-style string intended as a URL or path, rejecting both +// empty strings and the literal string "undefined" that Windows shells can +// write when a variable is unset-then-referenced without quotes (issue #336). +function asEnvUrl(value: string | undefined): string | undefined { + if (!value) return undefined + const trimmed = value.trim() + if (!trimmed || trimmed === 'undefined') return undefined + return trimmed } function readNestedString( @@ -287,10 +299,9 @@ export function resolveProviderRequest(options?: { (isGithubMode ? 'github:copilot' : 'gpt-4o') const descriptor = parseModelDescriptor(requestedModel) const rawBaseUrl = - options?.baseUrl ?? - process.env.OPENAI_BASE_URL ?? - process.env.OPENAI_API_BASE ?? - undefined + asEnvUrl(options?.baseUrl) ?? + asEnvUrl(process.env.OPENAI_BASE_URL) ?? + asEnvUrl(process.env.OPENAI_API_BASE) // Use Codex transport only when: // - the base URL is explicitly the Codex endpoint, OR // - the model is a Codex alias AND no custom base URL has been set