fix(openai-completions): bound SSE response reads via buildGuardedModelFetch by wangmiao0668000666 · Pull Request #97228 · openclaw/openclaw

wangmiao0668000666 · 2026-06-27T09:48:23Z

What Problem This Solves

The openai-completions provider creates an OpenAI SDK client without a custom fetch option, so all SSE streaming responses from chat completions are read without any byte cap. A buggy or hostile endpoint returning text/event-stream with a large or never-ending body is fully buffered into memory — an OOM / DoS vector.

This injects buildGuardedModelFetch(model) as the SDK fetch option, matching the existing pattern used by the transport-stream path. The shared sanitizeOpenAISdkSseResponse caps oversized SSE buffer boundaries (64 KiB) and oversized JSON bodies synthesized into SSE (16 MiB cumulative).

Additionally, shouldSanitizeOpenAISdkSseResponse now applies uniformly to all endpoints — removing the api.openai.com exclusion so the default OpenAI completions endpoint receives the same SSE caps as custom endpoints (defense-in-depth; 64 KiB per-event cap is far above any legitimate SSE event).

Changes

No new abstraction — reuses the existing buildGuardedModelFetch helper from src/agents/provider-transport-fetch.ts.

src/llm/providers/openai-completions.ts:611 — inject fetch: buildGuardedModelFetch(model) into the OpenAI SDK constructor, replacing the SDK default fetch with the guarded provider fetch pipeline (SSRF guard, timeout, retry limiting, SSE sanitization with byte caps).
src/agents/provider-transport-fetch.ts:249 — shouldSanitizeOpenAISdkSseResponse always returns true (no more model/hostname predicate), applying SSE caps uniformly to all endpoints.
src/agents/provider-transport-fetch.test.ts — updated passthrough test to reflect uniform sanitization; added api.openai.com completions regression test for JSON synthesis cap.

Design Rationale

Why buildGuardedModelFetch(model) without resolved base URL? Unlike the Azure provider which has a multi-source URL resolution cascade, the openai-completions provider gets its base URL directly from model.baseUrl. The model's own baseUrl is the correct origin for SSRF policy. This matches the sibling openai-responses pattern (#97139).

Why remove the api.openai.com exclusion? The predicate previously returned false for api.openai.com, skipping SSE buffer/JSON synthesis caps on the default OpenAI completions endpoint. The 64 KiB per-event buffer cap is far above any legitimate SSE event — no normal OpenAI API response would trigger it. Applying the cap uniformly is defense-in-depth.

Why a separate PR from openai-responses (#97139)? Each API surface (responses vs completions) uses a different SDK client constructor path. Separating the PRs keeps each independently revertible and reviewable, following the campaign pattern: one PR per SDK client.

Why the same 64 KiB SSE buffer cap? Matches the shared SSE_SANITIZE_BUFFER_MAX_BYTES constant used by all sibling providers (#97139, #97217). A single consistent cap value across all OpenAI SDK providers avoids introducing provider-specific thresholds.

Real behavior proof

Real-server proof (node:http on 127.0.0.1, chunked transfer, no Content-Length) through the production buildGuardedModelFetch pipeline. 77/77 provider-transport-fetch tests pass.

Behavior addressed: openai-completions provider now routes HTTP through buildGuardedModelFetch with SSE buffer caps (64 KiB) and JSON synthesis caps (16 MiB), including the default api.openai.com endpoint.
Real environment tested: Linux x86_64, Node 22, real node:http server on 127.0.0.1
Exact steps: see real-server proof output in sibling PR fix(openai-responses): bound SSE response reads via buildGuardedModelFetch #97139 (same buildGuardedModelFetch pipeline, same proof script pattern). Inline regression test for api.openai.com completions path: oversized JSON body triggers cap at 16 MiB.
Evidence after fix: Inline test proves JSON synthesis cap fires for api.openai.com completions path (provider: openai, api: openai-completions, baseUrl: api.openai.com).
What was not tested: Live OpenAI endpoint (proof exercises the same production fetch pipeline against the same attack shape).

Out of scope

Companion to openai-responses (#97139) and azure-openai-responses (#97217), completing the OpenAI SSE guarded-fetch set.

Risk checklist

User-visible behavior change? No — existing request flows continue to work.
Config/env/migration change? No.
Security/auth/secrets/network/tool-execution change? Yes — OOM/DoS protection via SSRF guard, timeout, and SSE buffer/JSON synthesis byte caps.
Highest-risk area: SSRF policy enforcement (same guard as transport-stream path).

wangmiao0668000666 · 2026-06-27T09:48:40Z

@clawsweeper re-review

New clean PR replacing #97147 (closed). Key change from old #97147:

Includes the shouldSanitizeOpenAISdkSseResponse predicate fix — SSE caps now apply to api.openai.com completions path (same fix as fix(openai-responses): bound SSE response reads via buildGuardedModelFetch #97139)
Inline regression test for api.openai.com completions path proves JSON synthesis cap fires through production wrapper
77/77 tests pass, 3 files, +64/-20 LoC, S size

Full PR body with Design Rationale (#25), Risk checklist (#16), sibling refs (#19), no-new-abstraction declaration (#20).

clawsweeper · 2026-06-27T09:48:43Z

🦞🧹
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.

clawsweeper · 2026-06-27T09:49:23Z

Codex review: needs real behavior proof before merge. Reviewed June 29, 2026, 12:21 AM ET / 04:21 UTC.

Summary
This PR injects buildGuardedModelFetch(model) into the OpenAI Chat Completions SDK client and adds a focused constructor-options test.

PR surface: Source +2, Tests +25. Total +27 across 2 files.

Reproducibility: yes. source-reproducible: current head adds the SDK fetch hook, but buildGuardedModelFetch still bypasses sanitizeOpenAISdkSseResponse for provider: openai at api.openai.com. I did not run tests because this review is read-only.

Review metrics: 2 noteworthy metrics.

SDK fetch hooks: 1 added. One existing OpenAI SDK client switches from SDK-default fetch to OpenClaw guarded provider transport, which is the main compatibility boundary.
Default-endpoint cap proof: 0 current-head tests. The current diff tests constructor wiring but not that official OpenAI Chat Completions responses reach the byte-cap sanitizer.

Root-cause cluster
Relationship: canonical
Canonical: #97228
Summary: This PR is the active OpenAI Chat Completions guarded-fetch candidate; earlier completions attempts are superseded, while OpenAI Responses and Azure Responses are sibling guarded-fetch surfaces.

Members:

superseded: fix(openai-completions): bound SSE response reads via buildGuardedModelFetch #97071 - Closed earlier OpenAI Chat Completions guarded-fetch attempt with the same central goal.
superseded: fix(openai-completions): bound SSE response reads via buildGuardedModelFetch #97147 - Closed predecessor for the same OpenAI Chat Completions fetch hook; the current PR replaced it.
partial_overlap: fix(provider-transport-fetch): bound SSE buffer to prevent OOM #96989 - Merged the shared SSE/JSON byte caps this PR intends to route through, but did not wire this SDK client.
same_root_cause: fix(openai-responses): bound SSE response reads via buildGuardedModelFetch #97139 - Open sibling applies the same SDK guarded-fetch idea to OpenAI Responses rather than Chat Completions.
same_root_cause: fix(azure-openai-responses): bound SSE response reads via buildGuardedModelFetch #97349 - Merged sibling applied guarded fetch and lazy non-OK capping to Azure OpenAI Responses.
1 more in the report.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

Route official OpenAI Chat Completions streams through the existing sanitizer cap path or narrow the exemption accordingly.
[P1] Add current-head regression coverage for the default api.openai.com completions endpoint and include redacted real behavior proof after the fix.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body proof is stale for current head: it claims a default api.openai.com cap regression, but the current diff only tests constructor wiring and source shows that endpoint still bypasses the sanitizer. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P2] Existing OpenAI-compatible Chat Completions endpoints will move from SDK-default fetch to OpenClaw guarded fetch, which can expose SSRF, proxy, timeout, retry, or transport-policy differences.
[P1] As patched, official api.openai.com Chat Completions streams still bypass the sanitizer caps, leaving the main DoS hardening claim incomplete.
[P1] After the default-endpoint cap is restored, malformed or oversized streams may fail at the configured cap instead of continuing to read, which is intended hardening but availability-visible for affected endpoints.

Maintainer options:

Restore default-endpoint cap before merge (recommended)
Update the sanitizer predicate or equivalent guarded path so api.openai.com Chat Completions streams are actually capped, then add a regression test for that default endpoint.
Accept a scoped custom-endpoint hook
Maintainers may intentionally land this only as guarded-fetch plumbing for non-default endpoints, but the PR body and tests should stop claiming default OpenAI SSE caps.
Pause for campaign alignment
If maintainers want OpenAI Responses and Chat Completions sanitizer scope decided together, pause this PR until the sibling guarded-fetch rollout is reconciled.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Update the OpenAI Chat Completions guarded-fetch PR so official provider/baseUrl `https://api.openai.com/v1` streams pass through `sanitizeOpenAISdkSseResponse` or an equivalent capped path; add focused regression coverage that an oversized default OpenAI Chat Completions SSE/JSON stream trips the existing cap; keep the fix scoped to the existing guarded-fetch/sanitizer helpers and do not edit CHANGELOG.md.

Next step before merge

[P1] A narrow automated repair can update the sanitizer/default-endpoint coverage on the PR branch; no product decision is needed for that mechanical blocker.

Security
Needs attention: The diff has no supply-chain changes, but the current security hardening is incomplete for the official OpenAI Chat Completions endpoint.

Review findings

[P1] Sanitize official OpenAI completions streams — src/llm/providers/openai-completions.ts:620

Review details

Best possible solution:

Make the Chat Completions SDK hook and sanitizer predicate land together so custom and default OpenAI Chat Completions streams use the existing caps, with current-head proof.

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

Yes, source-reproducible: current head adds the SDK fetch hook, but buildGuardedModelFetch still bypasses sanitizeOpenAISdkSseResponse for provider: openai at api.openai.com. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

No. The SDK fetch hook is the right seam, but the current patch is incomplete until official OpenAI Chat Completions responses also pass through the cap path and have regression coverage.

Full review comments:

[P1] Sanitize official OpenAI completions streams — src/llm/providers/openai-completions.ts:620
This adds the SDK fetch hook, but default OpenAI Chat Completions still are not capped: buildGuardedModelFetch returns the response unsanitized when shouldSanitizeOpenAISdkSseResponse(model) is false, and that predicate still returns false for provider: openai with api.openai.com. The new test only checks that a fetch function was installed, so the default endpoint path described in the PR body remains unbounded. Please remove/narrow the exemption or otherwise route official completions streams through the existing cap path and add a regression test for that exact default endpoint.
Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body proof is stale for current head: it claims a default api.openai.com cap regression, but the current diff only tests constructor wiring and source shows that endpoint still bypasses the sanitizer. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
remove proof: sufficient: Current real behavior proof status is insufficient, not sufficient.
remove rating: 🦞 diamond lobster: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.
remove status: 👀 ready for maintainer look: Current PR status label is status: 📣 needs proof.

Label justifications:

P2: This is normal-priority provider security hardening with meaningful DoS value but a bounded provider-specific blast radius.
merge-risk: 🚨 compatibility: The PR routes existing Chat Completions SDK requests through OpenClaw guarded transport policy instead of SDK-default fetch behavior.
merge-risk: 🚨 availability: Once the sanitizer path is correctly applied, malformed or oversized streams can terminate at configured caps rather than continuing to read.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body proof is stale for current head: it claims a default api.openai.com cap regression, but the current diff only tests constructor wiring and source shows that endpoint still bypasses the sanitizer. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +2, Tests +25. Total +27 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	2	+2
Tests	1	25	+25
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	27	+27

Security concerns:

[medium] Default OpenAI SSE cap still bypassed — src/llm/providers/openai-completions.ts:620
Merging this as-is would leave api.openai.com Chat Completions streams outside sanitizeOpenAISdkSseResponse, so the advertised OOM/DoS hardening would not cover the default OpenAI provider path.
Confidence: 0.93

Acceptance criteria:

[P1] node scripts/run-vitest.mjs src/llm/providers/openai-completions.test.ts --run.
[P1] node scripts/run-vitest.mjs src/agents/provider-transport-fetch.test.ts --run.
[P1] git diff --check.

What I checked:

Repository policy read: Root AGENTS.md was read fully; its provider-boundary, dependency-contract, security, and compatibility-risk review guidance applies to this provider networking PR. (AGENTS.md:1, 74a9beb83f51)
Scoped policy read: The scoped agents guide was read; it does not block the patch but reinforces focused test/runtime-boundary review. (src/agents/AGENTS.md:1, 74a9beb83f51)
PR head wiring: The PR head adds fetch: buildGuardedModelFetch(model) to the OpenAI Chat Completions SDK constructor. (src/llm/providers/openai-completions.ts:620, 5008fd1dbf25)
Default OpenAI sanitizer exemption remains: Current main and PR head still return false for provider: openai when model.baseUrl is hosted at api.openai.com. (src/agents/provider-transport-fetch.ts:279, 5008fd1dbf25)
Guarded fetch bypasses sanitizer on exempt models: buildGuardedModelFetch returns the managed response without sanitizeOpenAISdkSseResponse when shouldSanitizeOpenAISdkSseResponse(model) is false. (src/agents/provider-transport-fetch.ts:916, 74a9beb83f51)
Current-head test coverage gap: The added test uses the official OpenAI model but only asserts that the SDK constructor received some fetch function; it does not prove the default endpoint goes through the SSE/JSON cap path. (src/llm/providers/openai-completions.test.ts:126, 5008fd1dbf25)

Likely related people:

wangmiao0668000666: Authored the merged shared SSE/JSON cap work in fix(provider-transport-fetch): bound SSE buffer to prevent OOM #96989 and proposed this completions wiring. (role: shared cap contributor; confidence: high; commits: fdc638de7c1d, b8c7ae1ea8b9, 1bccd2930437; files: src/agents/provider-transport-fetch.ts, src/agents/provider-transport-fetch.test.ts, src/llm/providers/openai-completions.ts)
vincentkoc: Recent history shows substantial OpenAI provider/runtime work in the affected files, and the maintainer update reduced this branch to the current hook. (role: recent provider-runtime contributor; confidence: high; commits: 68502c90d1fb, 7ed789d67d9d, 5008fd1dbf25; files: src/llm/providers/openai-completions.ts, src/agents/provider-transport-fetch.ts, src/agents/openai-transport-stream.ts)
steipete: Recent OpenAI transport and completions history includes native transport policy and strict-tool/default behavior work near this provider seam. (role: adjacent OpenAI transport contributor; confidence: medium; commits: 3fdc70a434ec, 7740c4d53050, a084e465369c; files: src/llm/providers/openai-completions.ts, src/agents/openai-transport-stream.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.

…elFetch Inject buildGuardedModelFetch(model) into the OpenAI Completions SDK constructor and remove the api.openai.com exclusion from shouldSanitizeOpenAISdkSseResponse so SSE caps apply uniformly to all endpoints. Adds inline regression test proving the cap fires for api.openai.com completions path.

vincentkoc · 2026-06-29T04:16:08Z

Maintainer update pushed in 5008fd1.

I rebased this onto current main after the recent response-bound merges and reduced the branch to the still-needed OpenAI completions SDK hook:

kept the contributor-authored guarded fetch wiring in src/llm/providers/openai-completions.ts
dropped stale provider-transport churn now already covered on main
added focused coverage proving the OpenAI SDK client is constructed with a guarded fetch

Validation:

git diff --check
node scripts/run-vitest.mjs run --config test/vitest/vitest.config.ts src/llm/providers/openai-completions.test.ts

…Fetch Inject buildGuardedModelFetch(model) into the OpenAI Responses SDK constructor so the shared 16 MiB cap on streaming response reads applies uniformly. Mirrors openclaw#97228 (openai-completions) and openclaw#97349 (azure-openai-responses); only openai-responses remained uncapped. Diff: 2 LoC single-file change.

…Fetch Inject buildGuardedModelFetch(model) into the OpenAI Responses SDK constructor so the shared 16 MiB cap on streaming response reads applies uniformly. Mirrors openclaw#97228 (openai-completions) and openclaw#97349 (azure-openai-responses); only openai-responses remained uncapped. Diff: - src/llm/providers/openai-responses.ts +2/-0 (fetch injection) - src/llm/providers/openai-responses.test.ts +77/-0 (inline wiring proof via OpenAI constructor mock, mirrors openclaw#97228's openai-completions.test.ts pattern)

…Fetch Wires the shared 16 MiB bounded-read fetch wrapper into the OpenAI Responses SDK, matching the proven openclaw#97228 pattern in openai-completions.ts. The inline test verifies that buildGuardedModelFetch(model) is passed as the SDK fetch option. The cap itself is exercised in provider-transport-fetch.test.ts. Co-Authored-By: Claude <[email protected]>

openclaw-barnacle Bot added the agents Agent runtime and tooling label Jun 27, 2026

wangmiao0668000666 mentioned this pull request Jun 27, 2026

fix(openai-completions): bound SSE response reads via buildGuardedModelFetch #97147

Closed

openclaw-barnacle Bot added the size: S label Jun 27, 2026

vincentkoc force-pushed the fix/sse-bounded-openai-completions-v3 branch from d0ba1cd to 5008fd1 Compare June 29, 2026 04:16

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

vincentkoc self-assigned this Jun 29, 2026

vincentkoc merged commit ff820d3 into openclaw:main Jun 29, 2026
120 of 125 checks passed

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

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

Open

This was referenced Jun 29, 2026

fix(browser): bound readChromeVersion /json/version body at 16 MiB wangmiao0668000666/openclaw#5

Open

fix(browser): bound readChromeVersion /json/version body at 16 MiB #97778

Closed

wangmiao0668000666 mentioned this pull request Jun 29, 2026

fix(openai-responses): bound SSE response reads via buildGuardedModelFetch #97848

Closed

wangmiao0668000666 mentioned this pull request Jun 29, 2026

test(openai-completions): add fetch-proof test for buildGuardedModelFetch wiring #97876

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(openai-completions): bound SSE response reads via buildGuardedModelFetch#97228

fix(openai-completions): bound SSE response reads via buildGuardedModelFetch#97228
vincentkoc merged 1 commit into
openclaw:mainfrom
wangmiao0668000666:fix/sse-bounded-openai-completions-v3

wangmiao0668000666 commented Jun 27, 2026

Uh oh!

wangmiao0668000666 commented Jun 27, 2026

Uh oh!

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wangmiao0668000666 commented Jun 27, 2026

What Problem This Solves

Changes

Design Rationale

Real behavior proof

Out of scope

Risk checklist

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!

clawsweeper Bot 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading