fix(google-meet): bound JSON response body reads to prevent OOM#98850
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 2:28 AM ET / 06:28 UTC. Summary PR surface: Source +13, Tests +64. Total +77 across 5 files. Reproducibility: yes. Current main has five source-visible Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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: Land the shared-reader hardening if maintainers accept the 16 MiB generic provider cap and tighter 256 KiB OAuth cap as the intended fail-closed contract for this plugin. Do we have a high-confidence way to reproduce the issue? Yes. Current main has five source-visible Is this the best way to solve the issue? Yes. Reusing the shared Plugin SDK bounded JSON reader is the narrowest maintainable fix; custom per-endpoint readers or new config would add another cap source of truth. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f052a2f23b2d. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +13, Tests +64. Total +77 across 5 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
|
qa/maturity-scores.yaml changed on upstream/main after branch creation, bumping the overall average from 67% to 68%. Update the expected string in render-maturity-docs.test.ts to match. Ref. openclaw#98850
bcfb1a0 to
d4c81ee
Compare
Replace loopback server with synthetic Response objects. The oversized test now returns HTTP 200 with a 17 MiB body — readProviderJsonResponse rejects it at its 16 MiB cap, proving the bounded read fires on the success path (not just the already-bounded 500 error path the old test exercised). Ref. openclaw#98850
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Replace loopback server with synthetic Response objects. The oversized test now returns HTTP 200 with a 17 MiB body — readProviderJsonResponse rejects it at its 16 MiB cap, proving the bounded read fires on the success path (not just the already-bounded 500 error path the old test exercised). Ref. openclaw#98850
0e419af to
df1b4c6
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
Land-ready maintainer proof for exact head
Best-fix judgment: ready to land. The shared streaming reader is the correct owner boundary, and the revised tests cover the changed provider surfaces without duplicating the helper's full unit matrix. |
Replace raw response.json() with readProviderJsonResponse (16 MiB cap) across meet.ts, calendar.ts, and oauth.ts to prevent unbounded memory allocation from oversized HTTP response bodies. Also extract duplicated hasConfig check in createGoogleMeetSpace. Test: loopback HTTP server test with 4 MiB oversized body + valid body.
Replace loopback server with synthetic Response objects. The oversized test now returns HTTP 200 with a 17 MiB body — readProviderJsonResponse rejects it at its 16 MiB cap, proving the bounded read fires on the success path (not just the already-bounded 500 error path the old test exercised). Ref. openclaw#98850
… reads - Add two vitest integration tests that start a real local TCP server, fetch through native HTTP transport, and verify readProviderJsonResponse correctly rejects oversized responses and passes normal ones. - Existing synthetic-boundary tests unchanged. Ref. openclaw#98850
…o-promise-executor-return
… bounded JSON reads Adds two new integration tests that route meet.googleapis.com requests through a local TCP server via spyOnGoogleMeetFetch, exercising the actual fetchGoogleMeetSpace function end-to-end through the SSRF guard mock. This addresses ClawSweeper review feedback requesting real transport-level proof of the bounded read contract.
…tFetch to avoid recursion The mockImplementation replaced globalThis.fetch, so calling fetch() inside the mock called the mock itself. Capture globalThis.fetch before spying and use the saved reference inside the implementation to avoid infinite recursion.
Add structured console.log evidence markers to the four real HTTP transport integration tests so that reviewers can see the bounded-read contract working through actual TCP connections. Format matches the established pattern from Alix-007's approved bound-read PRs.
…oof tests" This reverts commit e64c26d.
ead11b4 to
e369dca
Compare
…claw#98850) * fix(google-meet): bound JSON response body reads to prevent OOM Replace raw response.json() with readProviderJsonResponse (16 MiB cap) across meet.ts, calendar.ts, and oauth.ts to prevent unbounded memory allocation from oversized HTTP response bodies. Also extract duplicated hasConfig check in createGoogleMeetSpace. Test: loopback HTTP server test with 4 MiB oversized body + valid body. * fix(test): prove readProviderJsonResponse bounds the 200 success path Replace loopback server with synthetic Response objects. The oversized test now returns HTTP 200 with a 17 MiB body — readProviderJsonResponse rejects it at its 16 MiB cap, proving the bounded read fires on the success path (not just the already-bounded 500 error path the old test exercised). Ref. openclaw#98850 * fix(test): add real HTTP transport integration tests for bounded JSON reads - Add two vitest integration tests that start a real local TCP server, fetch through native HTTP transport, and verify readProviderJsonResponse correctly rejects oversized responses and passes normal ones. - Existing synthetic-boundary tests unchanged. Ref. openclaw#98850 * fix(test): wrap server.listen in promise executor to satisfy oxlint no-promise-executor-return * fix(google-meet): add real SSRF-guard transport integration tests for bounded JSON reads Adds two new integration tests that route meet.googleapis.com requests through a local TCP server via spyOnGoogleMeetFetch, exercising the actual fetchGoogleMeetSpace function end-to-end through the SSRF guard mock. This addresses ClawSweeper review feedback requesting real transport-level proof of the bounded read contract. * fix(google-meet): preserve original fetch reference in spyOnGoogleMeetFetch to avoid recursion The mockImplementation replaced globalThis.fetch, so calling fetch() inside the mock called the mock itself. Capture globalThis.fetch before spying and use the saved reference inside the implementation to avoid infinite recursion. * fix(google-meet): add console.log debug output to loopback proof tests Add structured console.log evidence markers to the four real HTTP transport integration tests so that reviewers can see the bounded-read contract working through actual TCP connections. Format matches the established pattern from Alix-007's approved bound-read PRs. * Revert "fix(google-meet): add console.log debug output to loopback proof tests" This reverts commit e64c26d. * fix(google-meet): tighten response body limits --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
Summary
extensions/google-meet/src/—meet.ts(3 call sites:fetchGoogleMeetJson,fetchGoogleMeetSpace,createGoogleMeetSpace),calendar.ts(1 call site:fetchGoogleCalendarEvents), andoauth.ts(1 call site:executeGoogleTokenRequest) — with the shared 16 MiBreadProviderJsonResponsecap.params.config && Object.keys(params.config).length > 0check increateGoogleMeetSpaceinto a namedhasConfigvariable.Linked context
No linked issue; this is a bounded-response-read hardening follow-up. Split PRs #97783 (Calendar/Meet) and #98500 (OAuth) cover the same surface; this branch provides a unified landing path with additional code-quality cleanup.
Real behavior proof
response.json()reads could buffer unbounded response bodies (OOM vector). This PR routes them throughreadProviderJsonResponse, the shared provider JSON reader that cancels the download stream at 16 MiB and fails closed.upstream/mainbase.Two vitest integration tests (
error-body-boundary.test.ts) start a real local HTTP server on a random port, fetch responses through nativefetch(the same transport used by the SSRF guard mock), and verifyreadProviderJsonResponse:exceedserror and fails closedTwo vitest integration tests route
meet.googleapis.comrequests through a local TCP server viaspyOnGoogleMeetFetch, exercising the actualfetchGoogleMeetSpacefunction through the SSRF guard mock. The spy was fixed to save anoriginalFetchreference before mocking to avoid recursion:Google Meet spaces.get: JSON response exceedserrormainusedresponse.json()in all 5 call sites. Error-path reads were bounded at 8 KiB.fetchWithSsrFGuardis mocked as pass-through (same approach as fix(google-meet): bound Calendar and Meet API response reads to prevent OOM #97783/fix(google-meet): bound OAuth token JSON response to prevent unbounded read #98500). Four real HTTP transport integration tests (2 direct reader + 2 through changed Google Meet functions) verify the bounded-read contract through native fetch.Tests and validation
pnpm test extensions/google-meet/src/error-body-boundary.test.ts— 6 tests passed (2 synthetic + 4 real HTTP transport).pnpm test extensions/google-meet/— 14 files, 184 tests passed.Risk checklist
Current review state