Skip to content

Commit d95b16d

Browse files
jahoomajahooma
committed
Reviewer fixes
1 parent 0bd0e09 commit d95b16d

File tree

6 files changed

+45
-65
lines changed

6 files changed

+45
-65
lines changed

‎cli/src/utils/codex-oauth.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,8 @@ export function startOAuthFlowWithCallback(
355355
}
356356
})
357357

358-
callbackServer.listen(OAUTH_CALLBACK_PORT, async () => {
358+
// Bind to loopback only for security - prevents remote access to the callback server
359+
callbackServer.listen(OAUTH_CALLBACK_PORT, '127.0.0.1', async () => {
359360
onStatusChange?.('waiting')
360361
try {
361362
await open(authUrl)

‎common/src/constants/codex-oauth.ts‎

Lines changed: 24 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -68,85 +68,53 @@ export const CODEX_MODEL_MAP: Record<string, string> = {
6868

6969
/**
7070
* Check if a model is an OpenAI model that can use Codex OAuth.
71-
* Matches models in the CODEX_MODEL_MAP or with openai/ prefix.
71+
* Only returns true for models explicitly in the CODEX_MODEL_MAP.
72+
* This prevents unknown openai/* models from accidentally routing through Codex OAuth.
7273
*/
7374
export function isOpenAIModel(model: string): boolean {
74-
// Check if it's in the model map
75+
// Only return true for models explicitly in the model map
76+
// This is an allowlist approach - unknown models fall back to Codebuff backend
7577
if (CODEX_MODEL_MAP[model]) {
7678
return true
7779
}
78-
// Check if it has openai/ prefix
79-
if (model.startsWith('openai/')) {
80-
return true
80+
// Check without provider prefix
81+
if (model.includes('/')) {
82+
const modelId = model.split('/').pop()!
83+
if (CODEX_MODEL_MAP[modelId]) {
84+
return true
85+
}
8186
}
82-
// Check if it's a known Codex model
83-
const modelId = model.startsWith('openai/') ? model.slice(7) : model
84-
return (CODEX_OAUTH_MODELS as readonly string[]).includes(modelId)
87+
return false
8588
}
8689

8790
/**
8891
* Normalize a model ID to the Codex API format.
89-
* Uses the model map for known models, with fallback pattern matching.
92+
* Uses the model map for known models. Returns null for unknown models
93+
* to signal that the model cannot be used with Codex OAuth.
9094
*/
91-
export function toCodexModelId(model: string): string {
95+
export function toCodexModelId(model: string): string | null {
9296
// Check the mapping table first
9397
const mapped = CODEX_MODEL_MAP[model]
9498
if (mapped) {
9599
return mapped
96100
}
97101

98-
// Strip provider prefix if present
99-
const modelId = model.includes('/') ? model.split('/').pop()! : model
100-
101-
// Check again without prefix
102-
const mappedWithoutPrefix = CODEX_MODEL_MAP[modelId]
103-
if (mappedWithoutPrefix) {
104-
return mappedWithoutPrefix
105-
}
106-
107-
// Pattern-based fallback for unknown models
108-
const normalized = modelId.toLowerCase()
109-
110-
// GPT-5.2 Codex
111-
if (normalized.includes('gpt-5.2-codex') || normalized.includes('gpt 5.2 codex')) {
112-
return 'gpt-5.2-codex'
113-
}
114-
// GPT-5.2
115-
if (normalized.includes('gpt-5.2') || normalized.includes('gpt 5.2')) {
116-
return 'gpt-5.2'
117-
}
118-
// GPT-5.1 Codex Max
119-
if (normalized.includes('gpt-5.1-codex-max') || normalized.includes('codex-max')) {
120-
return 'gpt-5.1-codex-max'
121-
}
122-
// GPT-5.1 Codex Mini
123-
if (normalized.includes('gpt-5.1-codex-mini') || normalized.includes('codex-mini')) {
124-
return 'gpt-5.1-codex-mini'
125-
}
126-
// GPT-5.1 Codex
127-
if (normalized.includes('gpt-5.1-codex') || normalized.includes('gpt 5.1 codex')) {
128-
return 'gpt-5.1-codex'
129-
}
130-
// GPT-5.1
131-
if (normalized.includes('gpt-5.1') || normalized.includes('gpt 5.1')) {
132-
return 'gpt-5.1'
133-
}
134-
// Any codex model defaults to gpt-5.1-codex
135-
if (normalized.includes('codex')) {
136-
return 'gpt-5.1-codex'
137-
}
138-
// GPT-5 family defaults to gpt-5.1
139-
if (normalized.includes('gpt-5') || normalized.includes('gpt 5')) {
140-
return 'gpt-5.1'
102+
// Strip provider prefix if present and check again
103+
if (model.includes('/')) {
104+
const modelId = model.split('/').pop()!
105+
const mappedWithoutPrefix = CODEX_MODEL_MAP[modelId]
106+
if (mappedWithoutPrefix) {
107+
return mappedWithoutPrefix
108+
}
141109
}
142110

143-
// Default fallback
144-
return 'gpt-5.1'
111+
// Unknown model - return null to signal fallback to Codebuff backend
112+
return null
145113
}
146114

147115
/**
148116
* @deprecated Use toCodexModelId instead
149117
*/
150-
export function toOpenAIModelId(openrouterModel: string): string {
118+
export function toOpenAIModelId(openrouterModel: string): string | null {
151119
return toCodexModelId(openrouterModel)
152120
}

‎sdk/src/__tests__/codex-message-transform.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,9 +653,10 @@ describe('transformMessagesToCodexInput', () => {
653653
// ============================================================================
654654

655655
describe('transformCodexEventToOpenAI', () => {
656-
const createToolCallState = (): ToolCallState => ({
656+
const createToolCallState = (modelId = 'gpt-5.1'): ToolCallState => ({
657657
currentToolCallId: null,
658658
currentToolCallIndex: 0,
659+
modelId,
659660
})
660661

661662
describe('text delta events', () => {

‎sdk/src/credentials.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ const credentialsFileSchema = z.object({
4545

4646
const ensureDirectoryExistsSync = (dir: string) => {
4747
if (!fs.existsSync(dir)) {
48-
fs.mkdirSync(dir, { recursive: true })
48+
fs.mkdirSync(dir, { recursive: true, mode: 0o700 })
4949
}
5050
}
5151

@@ -180,7 +180,7 @@ export const saveClaudeOAuthCredentials = (
180180
claudeOAuth: credentials,
181181
}
182182

183-
fs.writeFileSync(credentialsPath, JSON.stringify(updatedData, null, 2))
183+
fs.writeFileSync(credentialsPath, JSON.stringify(updatedData, null, 2), { mode: 0o600 })
184184
}
185185

186186
/**
@@ -391,7 +391,7 @@ export const saveCodexOAuthCredentials = (
391391
codexOAuth: credentials,
392392
}
393393

394-
fs.writeFileSync(credentialsPath, JSON.stringify(updatedData, null, 2))
394+
fs.writeFileSync(credentialsPath, JSON.stringify(updatedData, null, 2), { mode: 0o600 })
395395
}
396396

397397
/**

‎sdk/src/impl/llm.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,7 @@ export async function promptAiSdk(
749749
apiKey: params.apiKey,
750750
model: params.model,
751751
skipClaudeOAuth: true, // Always use Codebuff backend for non-streaming
752+
skipCodexOAuth: true, // Always use Codebuff backend for non-streaming
752753
}
753754
const { model: aiSDKModel } = await getModelForRequest(modelParams)
754755

@@ -806,6 +807,7 @@ export async function promptAiSdkStructured<T>(
806807
apiKey: params.apiKey,
807808
model: params.model,
808809
skipClaudeOAuth: true, // Always use Codebuff backend for non-streaming
810+
skipCodexOAuth: true, // Always use Codebuff backend for non-streaming
809811
}
810812
const { model: aiSDKModel } = await getModelForRequest(modelParams)
811813

‎sdk/src/impl/model-provider.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,7 @@ export function transformMessagesToCodexInput(
551551
export interface ToolCallState {
552552
currentToolCallId: string | null
553553
currentToolCallIndex: number
554+
modelId: string
554555
}
555556

556557
/**
@@ -566,13 +567,15 @@ export function transformCodexEventToOpenAI(
566567
event: Record<string, unknown>,
567568
toolCallState: ToolCallState,
568569
): string | null {
570+
const { modelId } = toolCallState
571+
569572
// Handle text delta events - these contain the actual content
570573
if (event.type === 'response.output_text.delta' && event.delta) {
571574
const transformed = {
572575
id: event.response_id || 'chatcmpl-codex',
573576
object: 'chat.completion.chunk',
574577
created: Math.floor(Date.now() / 1000),
575-
model: 'gpt-5.2',
578+
model: modelId,
576579
choices: [
577580
{
578581
index: 0,
@@ -595,7 +598,7 @@ export function transformCodexEventToOpenAI(
595598
id: 'chatcmpl-codex',
596599
object: 'chat.completion.chunk',
597600
created: Math.floor(Date.now() / 1000),
598-
model: 'gpt-5.2',
601+
model: modelId,
599602
choices: [
600603
{
601604
index: 0,
@@ -626,7 +629,7 @@ export function transformCodexEventToOpenAI(
626629
id: 'chatcmpl-codex',
627630
object: 'chat.completion.chunk',
628631
created: Math.floor(Date.now() / 1000),
629-
model: 'gpt-5.2',
632+
model: modelId,
630633
choices: [
631634
{
632635
index: 0,
@@ -669,7 +672,7 @@ export function transformCodexEventToOpenAI(
669672
id: response?.id || 'chatcmpl-codex',
670673
object: 'chat.completion.chunk',
671674
created: Math.floor(Date.now() / 1000),
672-
model: response?.model || 'gpt-5.2',
675+
model: (response?.model as string) || modelId,
673676
choices: [
674677
{
675678
index: 0,
@@ -808,6 +811,7 @@ function createCodexFetch(
808811
const toolCallState: ToolCallState = {
809812
currentToolCallId: null,
810813
currentToolCallIndex: 0,
814+
modelId,
811815
}
812816

813817
const transformStream = new TransformStream({
@@ -887,6 +891,10 @@ function createCodexOAuthModel(
887891
): LanguageModel | null {
888892
// Convert to normalized Codex model ID
889893
const codexModelId = toCodexModelId(model)
894+
if (!codexModelId) {
895+
// Unknown model - fall back to Codebuff backend
896+
return null
897+
}
890898

891899
// Extract the ChatGPT account ID from the JWT token
892900
// This is REQUIRED for the chatgpt-account-id header

0 commit comments

Comments
 (0)