fix(proxy): bound SSE parser via complete-line cap at 1 MiB by wangmiao0668000666 · Pull Request #97191 · openclaw/openclaw

wangmiao0668000666 · 2026-06-27T06:29:05Z

What Problem This Solves

The proxy SSE parser (streamProxy in src/agents/runtime/proxy.ts) appends decoded chunks to a buffer and splits on \n. Without any line-size guard, a hostile or misconfigured proxy endpoint can force unbounded buffering through three paths: (a) a single data: line exceeding several MiB, (b) an unterminated tail that never receives \n, and (c) a final SSL frame without trailing newline at EOF — all OOM vectors.

The previous attempt (#96993, closed by maintainer) used a 64 KiB string-length tail cap, which failed non-deterministically: a 70 KiB line could cross or not cross the cap depending on TCP chunk boundaries, because 64 KiB is too close to legitimate data sizes.

This fix applies a byte-accurate 1 MiB policy to all three paths: complete lines (deterministic via split("\n") boundaries), the unterminated tail (now deterministic at 1 MiB because total accumulated bytes exceed the cap regardless of chunking), and the EOF final frame.

Changes

src/agents/runtime/proxy.ts:228,237,246 — three byte-accurate Buffer.byteLength checks at 1 MiB: (1) complete lines after split("\n"), (2) unterminated tail after draining complete lines, (3) final frame at EOF before processing.
src/agents/runtime/proxy.test.ts — 5 new tests: (1) oversized line in one chunk, (2) same line split across 3 chunks (deterministic), (3) 2000 small coalesced lines in one chunk (no false positive), (4) unterminated tail across 3 chunks without \n, (5) oversized final frame at EOF without trailing newline.

Design Rationale

Why complete-line cap + tail cap + EOF cap? The proxy parser has three distinct buffering paths. The complete-line cap covers split-delimited lines; the tail cap covers the accumulated unterminated data; the EOF cap covers the final frame. All three use the same 1 MiB byte-accurate limit for a single consistent policy.

Why is a 1 MiB tail cap deterministic when the old 64 KiB tail cap wasn't? Non-determinism arises when the cap is smaller than or similar to legitimate data sizes, so chunk boundaries decide whether accumulated bytes cross the threshold. At 1 MiB — over an order of magnitude larger than any legitimate proxy SSE line (text deltas and tool call payloads are well under 100 KiB) — the total accumulated bytes from a hostile payload will exceed the cap regardless of TCP chunking: a 1.1 MiB line reaches 1.1 MiB total bytes in 1 chunk or 3 chunks or any number of chunks. The threshold is high enough that chunk-boundary artifacts vanish.

Why 1 MiB? Aligned with sibling SSE handling in the codebase (e.g., readResponseWithLimit sibling bounds). ProxyAssistantMessageEvent has no event-size contract, so 1 MiB is a generous safe guard that no legitimate text-delta or tool-call payload should approach.

Why byte-accurate? Buffer.byteLength(line, "utf-8") correctly measures UTF-8 bytes, unlike String.length which counts UTF-16 code units and undercounts multibyte payloads. The previous tail-cap used .length while calling it "bytes", which was another finding in the maintainer's closure note.

Real behavior proof

Real node:http server + production streamProxy path (not mocked), driving 5 scenarios through real TCP sockets.

Behavior addressed: All three proxy SSE buffering paths (complete line, unterminated tail, EOF final frame) now have byte-accurate 1 MiB caps.
Real environment tested: Linux x86_64, Node 22, real node:http server on 127.0.0.1 (5 separate scenarios over real TCP sockets)

Exact steps or command run after this patch:

node --import tsx _proof_proxy_complete_line_cap.mts

Evidence after fix:

PASS  single-chunk: cap thrown -- msg="Proxy SSE line exceeds maximum allowed size of 1 MiB"
PASS  split-chunk (deterministic): cap thrown -- msg="Proxy SSE line exceeds maximum allowed size of 1 MiB"
PASS  coalesced valid: 1 events pass without cap
PASS  tail (no newline): cap thrown -- msg="Proxy SSE buffer exceeds maximum allowed size of 1 MiB"
PASS  EOF final frame: cap thrown -- msg="Proxy SSE buffer exceeds maximum allowed size of 1 MiB"
PASS  happy path: small event completes -- stopReason=stop

=== Results: 6 passed, 0 failed ===

Observed result after fix: (1) 1 MiB+ line in one TCP chunk → line cap, (2) same line across 3 chunks → line cap (deterministic), (3) 500 small coalesced lines → no false positive, (4) unterminated tail across 3 chunks → tail cap, (5) oversized EOF final frame → EOF cap, (6) happy path completes normally.
What was not tested: Live proxy endpoint (would not reproduce a hostile oversized line; the proof exercises the same production parser through a real HTTP response). Cross-platform Node differences (Node 22 only, matches CI).

Out of scope

Total proxy response body cap (separate concern, tracked in #96768). Other .json() bounded-read PRs in the campaign. The provider SSE layer (provider-transport-fetch.ts) has its own tail-cap mechanism operating at \n\n event boundaries and is not affected by this change.

Risk checklist

User-visible behavior change? Yes — proxy stream data over 1 MiB now fails (line, tail, or EOF) where it previously buffered silently.
Config/env/migration change? No.
Security/auth/secrets/network/tool-execution change? Yes — OOM protection via byte-accurate 1 MiB line/tail/EOF cap.
Highest-risk area: legitimate proxy events that exceed 1 MiB would now fail-closed. No OpenClaw proxy event today approaches that size (text deltas and tool call payloads are well under 100 KiB), and 1 MiB aligns with sibling SSE bounds. The 1 MiB tail cap is deterministic at this size — unlike the 64 KiB tail cap rejected in fix(proxy): bound SSE parser buffer to prevent OOM #96993 — because any hostile payload exceeding 1 MiB total bytes triggers the cap regardless of TCP chunk fragmentation.

clawsweeper · 2026-06-27T06:30:33Z

Codex review: needs maintainer review before merge. Reviewed June 27, 2026, 3:34 AM ET / 07:34 UTC.

Summary
The PR adds byte-accurate 1 MiB Buffer.byteLength guards to streamProxy complete-line, retained-tail, and EOF buffer paths, plus focused regression tests for oversized and coalesced SSE streams.

PR surface: Source +16, Tests +165. Total +181 across 2 files.

Reproducibility: yes. Current main has a source-visible path that appends proxy SSE data into an unbounded buffer, and the PR body supplies after-fix live node:http output for complete-line, tail, EOF, coalesced-valid, and happy-path cases.

Review metrics: 1 noteworthy metric.

Proxy SSE event-size caps: 3 added at 1 MiB. The PR creates a new fail-closed runtime boundary for complete lines, retained tails, and EOF final frames.

Stored data model
Persistent data-model change detected: serialized state: src/agents/runtime/proxy.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #97191
Summary: This PR is the current per-event proxy SSE parser cap candidate; the older 64 KiB attempt is superseded, and the total-body guard is overlapping but distinct.

Members:

superseded: fix(proxy): bound SSE parser buffer to prevent OOM #96993 - Closed predecessor for the same proxy SSE parser cap; maintainer feedback rejected its 64 KiB string-length boundary and requested a byte-accurate framing contract.
partial_overlap: fix(runtime/proxy): bound streaming success-body SSE reads at 16 MiB #96768 - Open PR against the same streamProxy reader that caps total streaming body bytes, but it does not replace this per-line/tail/EOF event-size policy.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Risk before merge

[P1] Existing proxy SSE events over 1 MiB now fail closed where current releases would continue parsing, so this is a compatibility boundary despite being availability hardening.
[P1] The same guard prevents OOM-style buffering but can turn unusually large legitimate proxied streaming responses into error results.
[P1] The open total-body guard at fix(runtime/proxy): bound streaming success-body SSE reads at 16 MiB #96768 overlaps the same reader and should be sequenced so total-stream and per-event limits stay coherent.

Maintainer options:

Accept The 1 MiB Event Boundary
Maintainers can proceed with the fail-closed behavior for proxy SSE frames above 1 MiB once normal merge gates pass.
Tune The Cap Before Merge
If legitimate proxy deployments may emit larger single-line events, adjust the cap and refresh oversized plus happy-path proof before landing.
Sequence With The Total-Body Guard
Coordinate this with fix(runtime/proxy): bound streaming success-body SSE reads at 16 MiB #96768 so per-event and total-stream policies do not drift during rebase or review.

Next step before merge

[P1] No narrow automated repair remains; maintainers need to accept or tune the 1 MiB fail-closed cap and coordinate normal sequencing with the total-body guard.

Security
Cleared: The diff hardens an availability-sensitive parser path and does not add dependencies, lockfile changes, CI permissions, secret handling, package metadata, or downloaded third-party execution.

Review details

Best possible solution:

Land this byte-accurate proxy SSE line/tail/EOF guard if maintainers accept the 1 MiB event boundary, while keeping the separate total-body guard coordinated but distinct.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main has a source-visible path that appends proxy SSE data into an unbounded buffer, and the PR body supplies after-fix live node:http output for complete-line, tail, EOF, coalesced-valid, and happy-path cases.

Is this the best way to solve the issue?

Yes, if maintainers accept the 1 MiB boundary. Capping complete lines, the retained tail, and the EOF final frame is the narrow parser fix; the total response-body cap remains separate in #96768.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against fbfadbd806e1.

Label changes

Label justifications:

P2: This is focused runtime availability hardening with limited blast radius and no evidence of an active widespread outage.
merge-risk: 🚨 compatibility: Existing proxy SSE events above 1 MiB would now error where current releases continue parsing.
merge-risk: 🚨 availability: The guard prevents unbounded buffering but can also make unusually large proxied streaming responses fail at runtime.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live output from a real node:http server exercising production streamProxy after the fix across oversized complete-line, split-line, open-tail, EOF final-frame, coalesced-valid, and happy-path cases.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live output from a real node:http server exercising production streamProxy after the fix across oversized complete-line, split-line, open-tail, EOF final-frame, coalesced-valid, and happy-path cases.

Evidence reviewed

PR surface:

Source +16, Tests +165. Total +181 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	16	+16
Tests	1	165	+165
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	181	+181

What I checked:

Current main parser remains unbounded: Current main appends decoded proxy SSE chunks into buffer, splits on newline, retains the tail, and processes the final buffer without a byte cap. (src/agents/runtime/proxy.ts:226, fbfadbd806e1)
PR guard placement covers the parser paths: The PR diff adds byte-accurate checks for the retained tail, complete lines, and the EOF final frame in the same parser loop. (src/agents/runtime/proxy.ts:227, caa27dc8b4ea)
Regression coverage targets the guarded behavior: The PR adds tests for oversized single-chunk lines, split oversized lines, many coalesced valid lines, an oversized unterminated tail, and an oversized no-newline final frame. (src/agents/runtime/proxy.test.ts:189, caa27dc8b4ea)
Latest release is not fixed: Release v2026.6.10 still has the same unbounded proxy SSE buffer path, so the PR is not obsolete on shipped code. (src/agents/runtime/proxy.ts:226, aa69b12d0086)
Prior maintainer context shaped this replacement: The predecessor was closed by maintainer feedback because the 64 KiB string-length limit was not a valid proxy protocol boundary and requested a byte-accurate framing contract.
Related work remains distinct: Search found the open total-body proxy guard, the closed 64 KiB predecessor, and other provider-body guard PRs; none supersedes this per-line/tail/EOF parser cap.

Likely related people:

vincentkoc: Recent commit history modified streamProxy and its tests, and this account provided the predecessor review that asked for a byte-accurate framing contract. (role: recent area contributor and reviewer; confidence: high; commits: 3c01716c828c; files: src/agents/runtime/proxy.ts, src/agents/runtime/proxy.test.ts)
steipete: The broad agent-runtime internalization commit added the current proxy runtime files to this path, making it a useful routing signal for owner-boundary review. (role: major runtime refactor contributor; confidence: medium; commits: bb46b79d3c14; files: src/agents/runtime/proxy.ts, src/agents/runtime/proxy.test.ts)
ferminquant: Recent path history changed proxy forwarding behavior and tests for prompt-cache affinity in the same module. (role: adjacent proxy option contributor; confidence: medium; commits: 3f9d2415acc8; files: src/agents/runtime/proxy.ts, src/agents/runtime/proxy.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

NianJiuZst

The proof for newline-delimited oversized lines is good; the remaining issue is that the same byte policy needs to cover every buffered SSE line path.

Extend the byte-accurate 1 MiB policy to the unterminated tail (data after last \n) and the EOF final frame, closing the bypass paths identified by ClawSweeper review. The 1 MiB tail cap is deterministic unlike the prior 64 KiB attempt (openclaw#96993) because any hostile payload exceeding 1 MiB total bytes triggers the cap regardless of TCP chunking. - Add tail buffer cap after draining complete lines (proxy.ts:237) - Add EOF final frame cap before processSseLine (proxy.ts:246) - 2 new tests: unterminated tail across chunks, no-newline EOF frame Co-Authored-By: Claude <[email protected]>

wangmiao0668000666 · 2026-06-27T07:22:56Z

@clawsweeper re-review

P1 fix applied: extended byte-accurate 1 MiB policy to all three buffering paths:

Complete lines (already present)
Unterminated tail after draining — added at proxy.ts:237
EOF final frame before processSseLine — added at proxy.ts:246

2 new tests: (1) unterminated tail across chunks without \n triggers tail cap,
(2) no-newline data: line triggers line cap at EOF.

Total: 9 tests pass (4 existing + 5 new: single-chunk, split-chunk,
coalesced, unterminated tail, EOF final frame).

clawsweeper · 2026-06-27T07:22:59Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

wangmiao0668000666 · 2026-06-27T08:20:07Z

@NianJiuZst Thanks for the review! This was addressed in commit caa27dc — the same byte-accurate 1 MiB policy now covers all three buffering paths:

Complete lines from split("\n") (was there)
Unterminated tail after draining complete lines (proxy.ts:237)
EOF final frame before processSseLine (proxy.ts:246)

Plus 2 new tests: oversized unterminated tail across chunks, and oversized EOF final frame without trailing newline.

vincentkoc · 2026-06-29T04:11:33Z

Closing this as superseded by #97235.

I reviewed the current branch against main. The OOM-hardening goal is now covered by the merged proxy path in #97235: bounded error-body reads, a 16 MiB total SSE byte guard, pending-buffer cap, and idle timeout in src/agents/runtime/proxy.ts.

This branch adds a separate 1 MiB per-line/tail/EOF protocol cap against the older reader shape. That is a new compatibility policy, not required for the OOM fix anymore, and I do not want to merge it stale. If we want that stricter per-frame policy, it should come back as a fresh PR against current streamProxy with current-main tests.

fix(proxy): bound SSE parser via complete-line cap at 1 MiB

50a14c4

openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jun 27, 2026

NianJiuZst reviewed Jun 27, 2026

View reviewed changes

Comment thread src/agents/runtime/proxy.ts

This was referenced Jun 27, 2026

fix(runtime/proxy): bound streaming success-body SSE reads at 16 MiB #96768

Closed

fix: proxy streams fail fast on oversized or stalled responses #97235

Merged

vincentkoc self-assigned this Jun 29, 2026

vincentkoc closed this Jun 29, 2026

github-actions Bot mentioned this pull request Jun 29, 2026

🦞 OpenClaw 生态日报 2026-06-29 96loveslife/big_model_radar#74

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(proxy): bound SSE parser via complete-line cap at 1 MiB#97191

fix(proxy): bound SSE parser via complete-line cap at 1 MiB#97191
wangmiao0668000666 wants to merge 2 commits into
openclaw:mainfrom
wangmiao0668000666:fix/proxy-sse-complete-line-cap

wangmiao0668000666 commented Jun 27, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

NianJiuZst left a comment •

edited

Loading

Uh oh!

Uh oh!

wangmiao0668000666 commented Jun 27, 2026

Uh oh!

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

wangmiao0668000666 commented Jun 27, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

wangmiao0668000666 commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Changes

Design Rationale

Real behavior proof

Out of scope

Risk checklist

Uh oh!

clawsweeper Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NianJiuZst left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wangmiao0668000666 commented Jun 27, 2026

Uh oh!

clawsweeper Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wangmiao0668000666 commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentkoc commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wangmiao0668000666 commented Jun 27, 2026 •

edited

Loading

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

NianJiuZst left a comment •

edited

Loading

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

wangmiao0668000666 commented Jun 27, 2026 •

edited

Loading