fix(extensions): bound CDP and ClawRouter JSON response reads to prevent OOM#100192
fix(extensions): bound CDP and ClawRouter JSON response reads to prevent OOM#100192hailory wants to merge 1 commit into
Conversation
…ent OOM Replace unbounded `response.json()` with `readProviderJsonResponse` at three remaining call sites: - `extensions/browser/cdp.helpers.ts:fetchJson` — generic CDP HTTP endpoint reader (used for /json/version and similar small payloads) - `extensions/browser/chrome.diagnostics.ts:readChromeVersion` — Chrome version probe via /json/version - `extensions/clawrouter/usage.ts:fetchClawRouterUsage` — usage data fetch All three now read the response body under a 16 MiB cap and throw a descriptive error on overflow or malformed JSON, matching the same pattern used across the rest of the codebase (PRs openclaw#96321–openclaw#96620). Test changes: update CDP mock to provide `arrayBuffer()` and `body: null` since `readProviderJsonResponse` reads via `readResponseWithLimit` (internal `response.arrayBuffer()`) rather than `response.json()`. Co-Authored-By: Claude <[email protected]>
openclaw-barnacle
Bot
added
extensions: clawrouter
|
Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 2:47 AM ET / 06:47 UTC. Summary PR surface: Source +6, Tests +1. Total +7 across 4 files. Reproducibility: yes. Source inspection shows current Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Keep the bounded-read direction, preserve CDP malformed-JSON diagnostics, add real oversized-response proof, and reconcile the ClawRouter hunk with the overlapping ClawRouter-specific PR. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current Is this the best way to solve the issue? No as currently proposed. The owner-boundary bounded reader is the right fix shape, but the PR must preserve existing CDP diagnostic classification and provide real behavior proof before merge. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8be1d366a1f7. Label changesLabel justifications:
Evidence reviewedPR surface: Source +6, Tests +1. Total +7 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
rating: 🦪 silver shellfish
|
Traced this through and it is a genuine stream-and-bound fix, not buffer-then-check, which is the part that actually matters for an OOM guard. One note on the test: the |
|
Thank you for the contribution. This fix landed with contributor co-authorship preserved in #100258 ( I consolidated the source fixes into a maintainer takeover because the contributor branches were based on rewritten pre- |
3 participants
Summary
Three remaining unbounded
response.json()calls in CDP and ClawRouter code paths have no size protection, risking OOM from compromised or misconfigured upstream endpoints.response.json()buffers the entire body before parsing; a multi-GB response from a hostile or buggy CDP endpoint or ClawRouter instance would exhaust process memory.readProviderJsonResponsefrom plugin-sdk, which enforces a 16 MiB cap and cancels the stream on overflow.extensions/browser/src/browser/cdp.helpers.ts(1 call site infetchJson),extensions/browser/src/browser/chrome.diagnostics.ts(1 call site inreadChromeVersion),extensions/clawrouter/usage.ts(1 call site infetchClawRouterUsage),extensions/browser/src/browser/cdp.helpers.test.ts(update mock to providearrayBuffer()forreadResponseWithLimit)response.json()call sites in browser extension; SSRF guard logic; functional behavior for well-formed responses.Change Type
Scope
Linked Issue/PR
Motivation
Same pattern already merged across the codebase (PRs #96321–#96620, #95416, and many more). CDP helpers (
fetchJson,readChromeVersion) and ClawRouter usage fetch were the last remaining unboundedresponse.json()success-path reads in bundled extensions, completing the coverage of this OOM vector.Real behavior proof
response.json()in three CDP and ClawRouter call sites, OOM risk from huge API responses.fix/bound-cdp-clawrouter-json.arrayBuffer()—readResponseWithLimitreads viaresponse.arrayBuffer()rather thanresponse.json()readProviderJsonResponse's default limit).User-visible / Behavior Changes
None for well-formed responses. Oversized or malformed responses now throw a descriptive error instead of buffering unboundedly in memory.
Security Impact
Human Verification
readResponseWithLimitcompatibility.bodystream (falls through toarrayBuffer()), non-object JSON (ChromeVersionnull-check retained), oversized body (capped at 16 MiB).Compatibility / Migration
Best-fix Verdict
readProviderJsonResponsehelper already used across the codebase for bounded JSON response parsing.readResponseWithLimit(more verbose, duplicates error handling already inreadProviderJsonResponse).AI Assistance
Risks and Mitigations
/json/versionis < 1 KiB) or ClawRouter (also small payloads) would throw instead of parsing. Realistic responses are well under this threshold.readProviderJsonResponsecap has been the standard across the codebase.