fix(google): bound OAuth token error body reads to prevent OOM#98722
fix(google): bound OAuth token error body reads to prevent OOM#98722wings1029 wants to merge 2 commits into
Conversation
openclaw-barnacle
Bot
added
channel: discord
|
Codex review: needs maintainer review before merge. Reviewed July 1, 2026, 1:09 PM ET / 17:09 UTC. Summary PR surface: Source +14, Tests +376. Total +390 across 4 files. Reproducibility: yes. Current main and Review metrics: 2 noteworthy metrics.
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: Keep this PR open until a linked canonical PR proves it covers this PR's unique work, or a maintainer confirms closure. Do we have a high-confidence way to reproduce the issue? Yes. Current main and Is this the best way to solve the issue? Mostly yes. Reusing the existing bounded-response helpers is the right implementation layer, but the best merge shape is to split or explicitly resolve the overlapping Discord work before landing. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4895a00b94f0. Label changesLabel justifications:
Evidence reviewedPR surface: Source +14, Tests +376. Total +390 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
proof: sufficient
1ad38c1 to
dddcaa6
Compare
wings1029
changed the title
openclaw-barnacle
Bot
added
triage: blank-template
The requestTokenGrant helper in the Google OAuth token exchange path read error response bodies via response.text() without any size cap. A misconfigured or malfunctioning Google OAuth endpoint that returns an oversized error body could exhaust gateway memory. Replace response.text() with readResponseTextLimited(8 KiB) on the error path, consistent with the Discord API error body limit. Replace the success-path response.json() cast with readProviderJsonResponse for defense-in-depth type-safe parsing. The oauth.http.ts fetchWithTimeout wrapper already bounds the response at 16 MiB, so these are defense-in-depth bounds that prevent oversized error text from being embedded in downstream Error messages. Co-Authored-By: Claude <[email protected]>
Addresses CI lint failures: curly(if) and no-unnecessary-type-conversion. Co-Authored-By: Claude <[email protected]>
dddcaa6 to
cde4fcf
Compare
|
Thanks for the PR. I’m closing this as superseded by #97587. The Google OAuth token error-body bound landed on |
2 participants
What Problem This Solves
The
requestTokenGranthelper in the Google OAuth token exchange path read error response bodies viaresponse.text()without any size cap. A misconfigured or malfunctioning Google OAuth endpoint (or intermediate proxy) that returns an oversized error body — e.g. a misrouted HTML page or a proxy error splash — could exhaust gateway memory before the error was surfaced.The
fetchWithTimeoutwrapper inoauth.http.tsalready bounds the response at 16 MiB, so the raw body cannot overflow memory. However, an unbounded 16 MiB error body can still be embedded verbatim into the downstreamErrormessage, producing excessive log payloads and redundant memory pressure.Why This Change Was Made
Error path (
response.status !== 200): replaceresponse.text()withreadResponseTextLimited(response, 8 KiB). The cap is generous for a structured OAuth error payload (~100 bytes of JSON) while keeping a runaway HTML page from polluting downstream diagnostics.Success path: replace the bare
(await response.json()) as {…}cast withreadProviderJsonResponse<T>(response, …). The upstreamfetchWithTimeoutalready bounds the body at 16 MiB, so this is defense-in-depth type-safe parsing with a consistent provider error surface.No new dependencies, APIs, config keys, or protocol changes.
User Impact
Normal OAuth errors (invalid grant, expired token, etc.) preserve their complete diagnostic payload. Oversized error streams are truncated at 8 KiB — the error message still identifies the failure, and the upstream 16 MiB bound ensures memory safety at the transport layer.
Evidence
Real behavior proof
response.text()on error path → boundedreadResponseTextLimited(8 KiB)node:http.createServer) streaming 1 MiB of chunked body over127.0.0.1, driven through the exportedexchangeCodeForTokensandrefreshTokensForGeminiClipublic APIs with mockedfetchWithTimeoutfetchWithTimeoutto return the loopback ResponseexchangeCodeForTokens("test-code", "test-verifier"){"error":"invalid_grant"}) is preserved exactlyexchangeCodeForTokens(authorization code) andrefreshTokensForGeminiCli(refresh) paths are covered[google oauth.token loopback proof] oversized path: cap=8192 streamed=1048576 message_length=8215fetchWithTimeoutwrapper already has an independent 16 MiB bound; this PR adds a tighter text cap at the call site.Validation Evidence
Security & Privacy / Compatibility
readResponseTextLimitedreads at most maxBytes from the stream; oversized bodies are truncated, not silently consumedCHANGELOG.mdchanges🤖 Generated with Claude Code