Repository navigation
fix(security): prevent structured-output format downgrade - #2394
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdcb84b7e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
[P1] The structured-output downgrade remains possible through an own enumerable metadata serializer. At
Line 110 in fdcb84b
toJSON even after forcing the in-memory format type to json_schema. Both Zod and Standard Schema helpers forward that callable, and FallbackEncoder then runs JSON.stringify(body), invoking it. Independently reproduced: the runtime format remains branded with type=json_schema and strict=true, while the actual HTTP request sends {"type":"text"}. This requires programmable metadata with an own enumerable callable; plain JSON input cannot introduce the function, and local response validation still runs. However, within this security fix, it preserves exactly the server-side schema downgrade the change is intended to prevent. Strip or neutralize metadata-provided serializer hooks in makeParseableTextFormat, and add direct-factory plus real responses.parse() wire-level regressions for both helper families.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the updated head with an independent second pass. The previously reported wire-format downgrade is fixed: the shared Responses text-format factory forces the trusted json_schema discriminator and removes copied toJSON hooks before request serialization. New public fetch regressions cover Zod v3/v4/Mini, Standard Schema, direct factory calls, callable/accessor serializers, frozen metadata, and inherited/non-enumerable hooks. Existing schema/name/strict precedence, parser branding, metadata compatibility, and validation behavior remain intact. Exact-head CI is green. No actionable findings.
Summary
type: 'json_schema'after copying caller-supplied metadata in the shared handwritten parser factory.textorjson_objectwhile their local parser remains deceptively branded.Security impact
Before this change, both
zodTextFormat(schema, name, { type: 'text' })andstandardTextFormat(schema, name, { type: 'json_object' })could silently replace the required format discriminator. Realclient.responses.parse(...)requests then serialized the downgraded discriminator while retaining an SDK-branded local parser, disabling API-side structured-output enforcement.The regression suite proves the outgoing HTTP request and locally validated parsed result across all four public validator families, including malicious
text,json_object, null, undefined, numeric, object, getter, inherited, non-enumerable, and direct-factory inputs.Validation
main; all 59 pass after the one-line fix.Only the handwritten shared parser and one new dedicated regression file are changed; active Zod helper work remains untouched.