fix(security): bound SDK event streams and validate streamed item identities - #2415
fix(security): bound SDK event streams and validate streamed item identities#2415HAYDEN-OAI wants to merge 23 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f220cce2b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8283fd463c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0ed2b4614
ℹ️ 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
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Independent two-pass review confirms the existing cross-realm Map accounting and inherited routing-getter discussions are actionable; please resolve those existing threads. Both are reachable through programmatic/custom event inputs rather than normal JSON-decoded provider events, so Medium severity is better calibrated than P1.
The third existing claim that iterator overflow skips later listeners is not reproducible: executing the exact implementation confirms later listeners still run because dispatch uses an earlier listener-array snapshot while cleanup mutates the replacement registry. Please avoid changing correct dispatch behavior based solely on that false positive.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f2c2f95d8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5498320c05
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
openai-node/src/lib/ChatCompletionStream.ts
Lines 993 to 995 in 2cd10b7
Allow valid tool-call indices beyond the choice limit
When a streamed completion contains 129 or more parallel tool calls, the call at index 128 is rejected even though the generated Chat Completions schema places no 128-item bound on delta.tool_calls or the finalized tool-call array. The comment derives this ceiling from the unrelated n limit, so the streaming helper now rejects a schema-valid response that the non-streaming path accepts; prevent sparse allocation with a contiguity or memory check rather than imposing an undocumented tool-count capability limit.
AGENTS.md reference: AGENTS.md:L46-L48
ℹ️ 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
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Two independently validated, existing unresolved threads still need fixes: [P1] accessor-backed buffered event objects evade retained-size accounting and can retain unbounded closure data: #2415 (comment) ; [P2] an unrelated strict tool incorrectly applies structured parsing limits to identifiable non-strict tool arguments: #2415 (comment) . Prior cross-realm collection and inherited-routing issues are fixed. Avoiding duplicate inline comments.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 266cd10056
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4903a36b96
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ad4b0529c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be87d1f170
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c99abcd72
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bafad1baa
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acd97e7d3f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de9c49c9c4
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39697164fc
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06d4b1c205
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
openai-node/src/lib/ChatCompletionStream.ts
Line 1437 in 6f26d1a
Validate choices against the serialized n value
When an enumerable request-level toJSON() changes n—for example, the caller object contains n: 1 but serializes with n: 2—the server legitimately returns choice index 1, while this limit is still derived from the pre-serialization parser snapshot and rejects that choice. The serialization observer only updates tools and response_format (and is not installed at all when both are absent), so the value enforced here can differ from the actual request body. Capture the serialized n from the same encoding pass or retain the schema-wide ceiling instead of narrowing with an undispatched value.
AGENTS.md reference: AGENTS.md:L86-L90
ℹ️ 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".
Castiron custom code✅ No new custom-code files detected. 34 mixed files remain; 0 existing customizations changed. Compared 34 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 32319309694 --repo openai/openai-node \
--name castiron-custom-code-32319309694-1 --dir /tmp/castiron-custom-code-32319309694-1
git apply --stat /tmp/castiron-custom-code-32319309694-1/custom-code.patch
cat /tmp/castiron-custom-code-32319309694-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin a0d68cc53125c2cb82eab31271b8984b8d65d4b2 a5a1cf194af1205de5f62e593aeb164020120141
python3 scripts/castiron/custom_code_report.py report \
--base a0d68cc53125c2cb82eab31271b8984b8d65d4b2 \
--head a5a1cf194af1205de5f62e593aeb164020120141 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-a5a1cf194af1
cat /tmp/castiron-custom-code-a5a1cf194af1/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
There was a problem hiding this comment.
💡 Codex Review
openai-node/src/lib/ChatCompletionStream.ts
Lines 1447 to 1451 in 929ded3
Bind the choice limit to the serialized n value
When a request-level toJSON() changes n from 1 to 2, the wire request legitimately asks the server for two choices, but #params.n remains the pre-serialization value because the serializer observer captures only tools and response format. The first chunk for choice index 1 is then rejected here as invalid. Capture the serialized n alongside the other wire settings, or enforce only the schema-wide 128-choice ceiling when the dispatched value is unavailable.
AGENTS.md reference: AGENTS.md:L86-L90
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b18e1a6cee
ℹ️ 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
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Independent two-pass review confirms the new existing Medium finding in discussion #3817576174: AssistantStream repeatedly queues events referencing the same accumulated message, but each event uses a fresh WeakSet for retained-byte estimation. The shared message graph is repeatedly charged, so ordinary streams can exceed the 8 MiB cap and abort despite retaining far less memory. Track shared reachability across the entire iterator queue. Other newly opened related threads also warrant review. The Castiron reporter check failure is separately an inherited stale-base infrastructure issue; no duplicate inline comment added.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 836c27c547
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd41cae973
ℹ️ 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".
2 participants
Security findings
csf_f748424407d7f9a4c8cd94f9: cap detached chat, assistant, and Responses iterators at 4,096 events / 8 MiB, including nested objects, collections, shared buffers, and hidden error data; fail closed and abort the transport.csf_184ca4005f95edde69d61854: reject unknown, mismatched, duplicate, out-of-order, and post-terminal assistant message IDs before mutation or emission.csf_f90473f146356354c5bfe391: replace quadratic structured-output/tool prefix reparsing with adaptive coalescing and enforce aggregate UTF-8 byte, fragment, nesting-depth, and parse-work budgets.csf_dd139bc420237c584e757348: require matching ownitem_idand event-family item types across all 42 item-scoped Responses events; validate created, added, completed, duplicate, and lifecycle item identities.Compatibility and scope
call_id.csf_8c5b2acede7459533e2038cc) was already fixed by merged fix(security): stop retaining sensitive assistant stream events #2407 and is intentionally excluded.Verification
./node_modules/.bin/tsc --noEmit./scripts/lintgit diff --check origin/main...HEAD