fix(openai): bound SSE parser buffer to prevent OOM#96972
fix(openai): bound SSE parser buffer to prevent OOM#96972wangmiao0668000666 wants to merge 2 commits into
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep this PR open: the runtime guard is in the right post-drain location, but the branch still has a weak regression test and lacks real behavior proof from the exact current head. Canonical path: Close this PR as superseded by #96762. So I’m closing this here and keeping the remaining discussion on #96762. Review detailsBest possible solution: Close this PR as superseded by #96762. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection: current main keeps an unterminated OpenAI/Codex SSE tail in Is this the best way to solve the issue? Not yet. The guard location is a plausible fix, but the best mergeable version needs a valid-only coalesced-chunk regression test plus exact-head real behavior proof; #96762 remains a broader total-byte alternative. Security review: Security review cleared: The diff hardens an availability-sensitive parser path and adds no dependency, lockfile, CI, permission, secret-handling, package, or downloaded-code surface. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against ec737ee74d9b. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Updated Real behavior proof with real node:http server proof:
Pre-flight: oxlint shards ✅ tsgo ✅ 17/17 vitest ✅ |
|
@clawsweeper re-review Real behavior proof has been updated:
|
|
@clawsweeper re-review Real behavior proof has been updated: Replaced in-memory ReadableStream proof with real node:http server proof |
|
@clawsweeper re-review P1 finding addressed:
Commit: 763757e |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
ClawSweeper applied the proposed close for this PR.
|
What Problem This Solves
The
parseSSEgenerator insrc/llm/providers/openai-chatgpt-responses.ts:719accumulates decoded SSE chunks into an internalbufferstring (line 726) that only shrinks when a\n\nevent boundary is found. If a hostile or misconfigured SSE endpoint sends data without double-newline delimiters, the buffer grows unboundedly — consuming all available memory and OOM-killing the gateway process.This affects every streaming Codex/OpenAI chat completion call routed through the SSE transport path (
transport: "sse"or WebSocket failure fallback).Changes
src/llm/providers/openai-chatgpt-responses.ts(+10 LoC): AddedSSE_PARSE_MAX_BUFFER_BYTES = 64 KiBconstant. After each chunk decode, the buffer check throwsCodexProtocolErrorif the accumulated buffer exceeds the cap — preventing unbounded memory growth.src/llm/providers/openai-chatgpt-responses.test.ts(+35 LoC): Added inline test"rejects oversized SSE response that exceeds buffer cap"— creates aResponsewith aReadableStreambody of 65 KiB (no\n\n), runs it through the productionstreamOpenAICodexResponses->parseSSEchain, and asserts the error shows the buffer cap message.Real behavior proof
pnpm test src/llm/providers/openai-chatgpt-responses.test.ts --runnode:httpserver on 127.0.0.1 delivered a 65 KiB SSE body without\n\n→ client threwCodexProtocolError: Codex SSE response exceeded max buffer size (65536 bytes). Server-side bytes-sent: 65.0 KiB.\n\n) → parsed 1 event correctly.\n\n) → accumulated without throwing (cap uses strict>, not>=).Out of scope
src/llm/providers/anthropic.ts:345—iterateSseMessages) has the same pattern plusdata[]/raw[]array accumulation. Follow-up PR.src/agents/provider-transport-fetch.ts,src/agents/runtime/proxy.ts) — separate PRs.Risk
Low. The cap (64 KiB) is well above any reasonable SSE event payload. If triggered, the stream errors with a clear
CodexProtocolErrormessage rather than silently terminating. The change is additive — no existing behavior is modified, only a guard added.AI-assisted; reviewed before submission.