Skip to content

Commit 5bf75a2

Browse files
edelaunaedelauna
committed
fix(openai): harden custom request field validation
1 parent ec09c39 commit 5bf75a2

9 files changed

Lines changed: 196 additions & 42 deletions

File tree

‎packages/types/src/__tests__/provider-settings.test.ts‎

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { ANTHROPIC_API_PROTOCOL, OPENAI_API_PROTOCOL, providerIdentifiers, provi
22
import {
33
getApiProtocol,
44
OPEN_AI_CODEX_SERVICE_TIER_KEY,
5+
parseOpenAiExtraBody,
56
PROVIDER_SETTINGS_KEYS,
67
providerSettingsSchema,
78
providerSettingsSchemaDiscriminated,
@@ -42,17 +43,62 @@ describe("OpenAI-compatible extra body settings", () => {
4243
).toBe(false)
4344
})
4445

45-
it.each(["model", "messages", "stream", "tools", "max_tokens", "__proto__"])(
46-
"rejects the reserved request key %s",
47-
(reservedKey) => {
48-
expect(
49-
providerSettingsSchemaDiscriminated.safeParse({
50-
apiProvider: providerIdentifiers.openai,
51-
openAiExtraBody: JSON.stringify({ [reservedKey]: "override" }),
52-
}).success,
53-
).toBe(false)
54-
},
55-
)
46+
it.each([
47+
"__proto__",
48+
"constructor",
49+
"prototype",
50+
"max_completion_tokens",
51+
"max_tokens",
52+
"messages",
53+
"model",
54+
"parallel_tool_calls",
55+
"reasoning",
56+
"reasoning_effort",
57+
"response_format",
58+
"stream",
59+
"stream_options",
60+
"temperature",
61+
"tool_choice",
62+
"tools",
63+
])("rejects the reserved extra-body key %s", (reservedKey) => {
64+
expect(
65+
providerSettingsSchemaDiscriminated.safeParse({
66+
apiProvider: providerIdentifiers.openai,
67+
openAiExtraBody: JSON.stringify({ [reservedKey]: "override" }),
68+
}).success,
69+
).toBe(false)
70+
})
71+
72+
it("reports and filters reserved keys while preserving allowed nested fields", () => {
73+
const result = parseOpenAiExtraBody(
74+
JSON.stringify({
75+
metadata: { completion_window: "balanced" },
76+
model: "overridden-model",
77+
response_format: { type: "json_object" },
78+
stream: false,
79+
}),
80+
)
81+
82+
expect(result).toEqual({
83+
success: false,
84+
reason: "reservedKeys",
85+
reservedKeys: ["model", "response_format", "stream"],
86+
data: { metadata: { completion_window: "balanced" } },
87+
})
88+
})
89+
90+
it("accepts stop and n as provider-specific request fields", () => {
91+
const settings = {
92+
apiProvider: providerIdentifiers.openai,
93+
openAiExtraBody: JSON.stringify({ stop: ["DONE"], n: 2 }),
94+
}
95+
96+
expect(providerSettingsSchemaDiscriminated.parse(settings)).toEqual(settings)
97+
expect(parseOpenAiExtraBody(settings.openAiExtraBody)).toEqual({
98+
success: true,
99+
data: { stop: ["DONE"], n: 2 },
100+
})
101+
})
56102
})
57103

58104
describe("OpenAI Codex provider settings", () => {

‎packages/types/src/provider-settings.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { providerDefinitionList, type ProviderDefinition } from "./provider-sett
44
import { API_PROVIDER_FIELD, SETTINGS_SHAPE_FIELD } from "./provider-settings/common.js"
55
export {
66
OPEN_AI_CODEX_SERVICE_TIER_KEY,
7-
OPENAI_EXTRA_BODY_RESERVED_KEYS,
87
parseOpenAiExtraBody,
98
kimiCodeAuthMethodSchema,
109
type KimiCodeAuthMethod,

‎packages/types/src/provider-settings/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import { basetenProviderDefinition } from "./baseten.js"
3737
import type { ProviderDefinition } from "./common.js"
3838

3939
export { OPEN_AI_CODEX_SERVICE_TIER_KEY } from "./openai-codex.js"
40-
export { OPENAI_EXTRA_BODY_RESERVED_KEYS, parseOpenAiExtraBody } from "./openai.js"
40+
export { parseOpenAiExtraBody } from "./openai.js"
4141
export { kimiCodeAuthMethodSchema, type KimiCodeAuthMethod } from "./kimi-code.js"
4242
export { zaiApiLineSchema, type ZaiApiLine } from "./zai.js"
4343
export {

‎packages/types/src/provider-settings/openai.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,19 @@ import { baseProviderSettingsShape, createModelIdAccessor, createProviderDefinit
66

77
export const OPEN_AI_MODEL_ID_FIELD = "openAiModelId"
88

9-
export const OPENAI_EXTRA_BODY_RESERVED_KEYS = [
9+
const OPENAI_EXTRA_BODY_RESERVED_KEYS = [
10+
// Prototype-pollution defenses; remaining keys are request-owned, including tool-call protocol controls.
1011
"__proto__",
1112
"constructor",
13+
"prototype",
1214
"max_completion_tokens",
1315
"max_tokens",
1416
"messages",
1517
"model",
1618
"parallel_tool_calls",
17-
"prototype",
1819
"reasoning",
1920
"reasoning_effort",
21+
"response_format",
2022
"stream",
2123
"stream_options",
2224
"temperature",

‎src/api/providers/__tests__/openai.spec.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,20 @@ describe("OpenAiHandler", () => {
171171
})
172172
})
173173

174+
describe("withExtraBody", () => {
175+
it("gives request-owned options precedence when an allowed Extra Body field collides", () => {
176+
const extraBodyHandler = new OpenAiHandler({
177+
...mockOptions,
178+
openAiExtraBody: JSON.stringify({ service_tier: "flex" }),
179+
})
180+
181+
expect(extraBodyHandler["withExtraBody"]({})).toEqual({ service_tier: "flex" })
182+
expect(extraBodyHandler["withExtraBody"]({ service_tier: "default" })).toEqual({
183+
service_tier: "default",
184+
})
185+
})
186+
})
187+
174188
describe("createMessage", () => {
175189
const systemPrompt = "You are a helpful assistant."
176190
const messages: Anthropic.Messages.MessageParam[] = [

‎webview-ui/src/components/settings/providers/OpenAICompatible.tsx‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useState, useCallback, useEffect, type FormEvent } from "react"
1+
import { useState, useCallback, useEffect } from "react"
22
import { useEvent } from "react-use"
33
import { Checkbox } from "vscrui"
44
import { VSCodeButton, VSCodeTextArea, VSCodeTextField } from "@vscode/webview-ui-toolkit/react"
@@ -18,6 +18,7 @@ import {
1818

1919
import { useAppTranslation } from "@src/i18n/TranslationContext"
2020
import { Button, StandardTooltip } from "@src/components/ui"
21+
import { formatOpenAiExtraBodyValidationError } from "@src/utils/validate"
2122

2223
import { convertHeadersToObject } from "../utils/headers"
2324
import { inputEventTransform, noTransform } from "../transforms"
@@ -56,13 +57,7 @@ export const OpenAICompatible = ({
5657
return Object.entries(headers)
5758
})
5859
const extraBodyResult = parseOpenAiExtraBody(apiConfiguration.openAiExtraBody)
59-
const extraBodyError = extraBodyResult.success
60-
? undefined
61-
: extraBodyResult.reason === "reservedKeys"
62-
? t("settings:validation.openAiExtraBody.reservedKeys", {
63-
keys: extraBodyResult.reservedKeys?.join(", ") ?? "",
64-
})
65-
: t(`settings:validation.openAiExtraBody.${extraBodyResult.reason}`)
60+
const extraBodyError = formatOpenAiExtraBodyValidationError(extraBodyResult, t)
6661

6762
const handleAddCustomHeader = useCallback(() => {
6863
// Only update the local state to show the new row in the UI.
@@ -121,14 +116,6 @@ export const OpenAICompatible = ({
121116
[setApiConfigurationField],
122117
)
123118

124-
const handleExtraBodyChange = useCallback(
125-
(event: Event | FormEvent<HTMLElement>) => {
126-
const target = event.currentTarget as (HTMLElement & { value?: string }) | null
127-
setApiConfigurationField("openAiExtraBody", target?.value ?? "")
128-
},
129-
[setApiConfigurationField],
130-
)
131-
132119
const onMessage = useCallback((event: MessageEvent) => {
133120
const message: ExtensionMessage = event.data
134121

@@ -279,7 +266,7 @@ export const OpenAICompatible = ({
279266
resize="vertical"
280267
rows={5}
281268
value={apiConfiguration.openAiExtraBody ?? ""}
282-
onInput={handleExtraBodyChange}
269+
onInput={handleInputChange("openAiExtraBody")}
283270
placeholder={'{\n "metadata": {\n "completion_window": "balanced"\n }\n}'}
284271
className="w-full font-mono"
285272
aria-labelledby="openai-extra-body-label"

‎webview-ui/src/components/settings/providers/__tests__/OpenAICompatible.spec.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,25 @@ describe("OpenAICompatible Component - includeMaxTokens checkbox", () => {
437437
})
438438

439439
describe("Extra Body", () => {
440+
it("renders an undefined value as an editable empty field without a validation error", () => {
441+
render(
442+
<OpenAICompatible
443+
apiConfiguration={{ openAiExtraBody: undefined } as ProviderSettings}
444+
setApiConfigurationField={mockSetApiConfigurationField}
445+
organizationAllowList={mockOrganizationAllowList}
446+
/>,
447+
)
448+
449+
const input = screen.getByTestId("openai-extra-body-input")
450+
expect(input).toHaveValue("")
451+
expect(input).not.toHaveAttribute("aria-invalid", "true")
452+
expect(screen.queryByRole("alert")).not.toBeInTheDocument()
453+
454+
fireEvent.change(input, { target: { value: '{"store":false}' } })
455+
456+
expect(mockSetApiConfigurationField).toHaveBeenCalledWith("openAiExtraBody", '{"store":false}')
457+
})
458+
440459
it("renders the saved JSON and updates the cached provider field", () => {
441460
const openAiExtraBody = JSON.stringify({ metadata: { completion_window: "balanced" } }, null, 2)
442461

‎webview-ui/src/utils/__tests__/validate.spec.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,10 @@ describe("Model Validation Functions", () => {
244244
["not json", "settings:validation.openAiExtraBody.invalidJson"],
245245
["[]", "settings:validation.openAiExtraBody.objectRequired"],
246246
[JSON.stringify({ model: "override" }), "settings:validation.openAiExtraBody.reservedKeys keys=model"],
247+
[
248+
JSON.stringify({ response_format: { type: "json_object" } }),
249+
"settings:validation.openAiExtraBody.reservedKeys keys=response_format",
250+
],
247251
])("rejects invalid OpenAI-compatible Extra Body input", (openAiExtraBody, expectedError) => {
248252
const config: ProviderSettings = {
249253
apiProvider: providerIdentifiers.openai,
@@ -259,6 +263,58 @@ describe("Model Validation Functions", () => {
259263
})
260264
})
261265

266+
describe("validateApiConfiguration OpenAI-compatible Extra Body validation", () => {
267+
const createConfig = (openAiExtraBody: string): ProviderSettings => ({
268+
apiProvider: providerIdentifiers.openai,
269+
openAiBaseUrl: "https://api.sailresearch.com/v1",
270+
openAiApiKey: "valid-key",
271+
openAiModelId: "zai-org/GLM-5.2-FP8",
272+
openAiExtraBody,
273+
})
274+
275+
it("accepts a valid Extra Body object through both validation entry points", () => {
276+
const config = createConfig(JSON.stringify({ metadata: { completion_window: "balanced" } }))
277+
278+
expect(validateApiConfiguration(config, mockRouterModels, allowAllOrganization)).toBeUndefined()
279+
expect(
280+
validateApiConfigurationExcludingModelErrors(config, mockRouterModels, allowAllOrganization),
281+
).toBeUndefined()
282+
})
283+
284+
it.each([
285+
["invalid JSON", "not json", "settings:validation.openAiExtraBody.invalidJson"],
286+
["non-object JSON", "[]", "settings:validation.openAiExtraBody.objectRequired"],
287+
[
288+
"a reserved request API key",
289+
JSON.stringify({ model: "override" }),
290+
"settings:validation.openAiExtraBody.reservedKeys keys=model",
291+
],
292+
[
293+
"a reserved security key",
294+
'{"constructor":{"prototype":{"polluted":true}}}',
295+
"settings:validation.openAiExtraBody.reservedKeys keys=constructor",
296+
],
297+
[
298+
"response_format",
299+
JSON.stringify({ response_format: { type: "json_object" } }),
300+
"settings:validation.openAiExtraBody.reservedKeys keys=response_format",
301+
],
302+
])("rejects %s with the localized error", (_name, openAiExtraBody, expectedError) => {
303+
const config = createConfig(openAiExtraBody)
304+
305+
expect(validateApiConfiguration(config, mockRouterModels, allowAllOrganization)).toBe(expectedError)
306+
})
307+
308+
it("returns required-field errors before Extra Body errors", () => {
309+
const config = createConfig("not json")
310+
config.openAiApiKey = undefined
311+
312+
expect(validateApiConfiguration(config, mockRouterModels, allowAllOrganization)).toBe(
313+
"settings:validation.openAi",
314+
)
315+
})
316+
})
317+
262318
describe("Opencode Go validation", () => {
263319
it("returns an apiKey error when the Opencode Go API key is missing", () => {
264320
const config: ProviderSettings = {

‎webview-ui/src/utils/validate.ts‎

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,38 @@ import {
1010
providerIdentifiers,
1111
} from "@roo-code/types"
1212

13+
type OpenAiExtraBodyParseResult = ReturnType<typeof parseOpenAiExtraBody>
14+
type OpenAiExtraBodyTranslationKey = `settings:validation.openAiExtraBody.${
15+
| "invalidJson"
16+
| "objectRequired"
17+
| "reservedKeys"}`
18+
type OpenAiExtraBodyTranslationFunction = (key: OpenAiExtraBodyTranslationKey, options?: { keys: string }) => string
19+
20+
export function formatOpenAiExtraBodyValidationError(
21+
result: OpenAiExtraBodyParseResult,
22+
t: OpenAiExtraBodyTranslationFunction,
23+
): string | undefined {
24+
if (result.success) {
25+
return undefined
26+
}
27+
28+
if (result.reason === "reservedKeys") {
29+
return t("settings:validation.openAiExtraBody.reservedKeys", {
30+
keys: result.reservedKeys?.join(", ") ?? "",
31+
})
32+
}
33+
34+
return t(`settings:validation.openAiExtraBody.${result.reason}`)
35+
}
36+
37+
function validateOpenAiExtraBody(apiConfiguration: ProviderSettings): string | undefined {
38+
if (apiConfiguration.apiProvider !== providerIdentifiers.openai) {
39+
return undefined
40+
}
41+
42+
return formatOpenAiExtraBodyValidationError(parseOpenAiExtraBody(apiConfiguration.openAiExtraBody), i18next.t)
43+
}
44+
1345
export function validateApiConfiguration(
1446
apiConfiguration: ProviderSettings,
1547
routerModels?: RouterModels,
@@ -22,6 +54,12 @@ export function validateApiConfiguration(
2254
return keysAndIdsPresentErrorMessage
2355
}
2456

57+
const extraBodyError = validateOpenAiExtraBody(apiConfiguration)
58+
59+
if (extraBodyError) {
60+
return extraBodyError
61+
}
62+
2563
const organizationAllowListError = validateProviderAgainstOrganizationSettings(
2664
apiConfiguration,
2765
organizationAllowList,
@@ -312,17 +350,10 @@ export function validateApiConfigurationExcludingModelErrors(
312350
}
313351
}
314352

315-
if (apiConfiguration.apiProvider === providerIdentifiers.openai) {
316-
const extraBodyResult = parseOpenAiExtraBody(apiConfiguration.openAiExtraBody)
317-
if (!extraBodyResult.success) {
318-
if (extraBodyResult.reason === "reservedKeys") {
319-
return i18next.t("settings:validation.openAiExtraBody.reservedKeys", {
320-
keys: extraBodyResult.reservedKeys?.join(", ") ?? "",
321-
})
322-
}
353+
const extraBodyError = validateOpenAiExtraBody(apiConfiguration)
323354

324-
return i18next.t(`settings:validation.openAiExtraBody.${extraBodyResult.reason}`)
325-
}
355+
if (extraBodyError) {
356+
return extraBodyError
326357
}
327358

328359
const organizationAllowListError = validateProviderAgainstOrganizationSettings(

0 commit comments

Comments
 (0)