fix(qa-channel): bound QA bus JSON response reads#99169
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 10:54 PM ET / 02:54 UTC. Summary PR surface: Source +18, Tests +116. Total +134 across 2 files. Reproducibility: yes. Current main clearly buffers node:http QA bus POST responses and uses unbounded response.json() for state, while the PR body supplies a concrete local streaming-server proof; I did not run commands because this review is read-only. 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:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge this bounded-reader shape if maintainers accept 16 MiB as the QA bus success-body contract; otherwise choose a named QA-specific limit and keep the same poll/state regression coverage aligned. Do we have a high-confidence way to reproduce the issue? Yes. Current main clearly buffers node:http QA bus POST responses and uses unbounded response.json() for state, while the PR body supplies a concrete local streaming-server proof; I did not run commands because this review is read-only. Is this the best way to solve the issue? Yes, conditionally. The patch is the narrow maintainable fix because it reuses existing bounded-response helpers and adds only a local node:http adapter, but the 16 MiB fail-closed contract still needs maintainer acceptance. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b34c188f8f0d. Label changesLabel justifications:
Evidence reviewedPR surface: Source +18, Tests +116. Total +134 across 2 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
Review history (3 earlier review cycles) |
b43e609 to
6f83858
Compare
6f83858 to
f07d612
Compare
f07d612 to
d31eba2
Compare
|
Merge-ready from local maintainer review. Autoreview is clean at head SHA d31eba2, CI is green, and I found no compatibility concerns or breaking changes. This is the best narrow fix for the QA bus JSON parse-boundary gap: it applies the existing 16 MiB JSON cap to the owning bus response read paths while preserving normal QA channel behavior. |
Co-authored-by: NIO <nocodet@mail.com>
Co-authored-by: NIO <nocodet@mail.com>
What Problem This Solves
extensions/qa-channel/src/bus-client.tsused unbounded JSON reads on the QA busHTTP success paths:
postJson(used bypollQaBusand other bus calls) buffered the fullnode:httpresponse body into memory beforeJSON.parse.getQaBusStateused unboundedawait response.json()on the fetch success path.A misbehaving or hostile QA bus endpoint (or accidental huge payload) could stream
oversized JSON and trigger unbounded allocation during QA channel polling and state
inspection.
This change caps both paths at 16 MiB with labeled errors such as
qa-bus /v1/poll: JSON response exceeds 16777216 bytesandqa-channel.bus-state: JSON response exceeds 16777216 bytes.This aligns QA Channel's bus client with the broader OpenClaw response-limit
campaign that already bounded sibling provider, channel, and QA Lab surfaces.
Changes
extensions/qa-channel/src/bus-client.ts:readQaBusNodeJsonResponseusingreadByteStreamWithLimitfornode:httpPOST responses (pollQaBus, etc.)response.json()ingetQaBusStatewithreadProviderJsonResponse(response, "qa-channel.bus-state")"qa-bus <path>: malformed JSON response"extensions/qa-channel/src/bus-client.test.ts:/v1/polland/v1/stateresponsesscripts/proof-qa-channel-bus-bound.mjs:node:httpstreaming poll bodies and fetch state readsReal behavior proof
Behavior addressed: Oversized QA bus JSON success bodies fail closed instead of
unbounded heap growth on poll and state reads.
Real environment tested: Node v22.22.0, macOS 15.3.1,
node:httpstreamingserver on
127.0.0.1, built helpers fromdist/plugin-sdk/provider-http.jsanddist/plugin-sdk/response-limit-runtime.js.Exact steps:
Content-Length).readByteStreamWithLimiton anode:httppoll response → assert labeled cap.readProviderJsonResponseon a fetch state response → assert labeled cap.Observed result:
qa-bus /v1/poll: JSON response exceeds 16777216 bytesqa-channel.bus-state: JSON response exceeds 16777216 bytesaccountsandthreadsarrays intactALL PROOF ASSERTIONS PASSEDWhat was not tested: live QA bus server integration against a running OpenClaw gateway.
Evidence
New bound regression tests (2 added):
bounds oversized poll responses and closes the stream early✓bounds oversized qa-bus state responses✓