Skip to content

fix(google): bound OAuth response reads to prevent OOM#99812

Closed
Pandah97 wants to merge 2 commits into
openclaw:mainfrom
Pandah97:fix/google-oauth-bound-response-reads
Closed

fix(google): bound OAuth response reads to prevent OOM#99812
Pandah97 wants to merge 2 commits into
openclaw:mainfrom
Pandah97:fix/google-oauth-bound-response-reads

Conversation

@Pandah97

@Pandah97 Pandah97 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Problem: Three Google OAuth modules read HTTP response bodies without size limits, creating OOM risk when upstream services return oversized or malicious payloads. Additionally, gunzipSync in the Vertex ADC parser lacks a decompressed-output cap, so a small compressed payload can still inflate past the wire limit.

Solution: Replace all unbounded response.arrayBuffer(), .json() calls with bounded SDK helpers (readResponseWithLimit, readProviderJsonResponse). Add maxOutputLength to gunzipSync to cap decompressed output at the same 1 MB bound.

What changed: extensions/google/vertex-adc.ts, extensions/google/oauth.token.ts, extensions/google/oauth.project.ts — 3 files, +27/-12 lines.

  • Wire reads bounded via SDK helpers (7 call sites)
  • gunzipSync bounded via maxOutputLength: GOOGLE_OAUTH_TOKEN_RESPONSE_MAX_BYTES
  • Token, project, and onboarding response paths all covered

What did NOT change: No behavior change for well-formed responses under the size limits. No API or config surface changes. No dependency additions.

What Problem This Solves

7 call sites across 3 files consume unbounded HTTP response bodies. This is the same unbounded-read class fixed across 30+ providers via the Plugin SDK, but the Google OAuth paths were missed.

Additionally, the Vertex ADC gzip decoder could amplify a small compressed payload (under the 1 MB wire cap) into an arbitrarily large decompressed buffer, leaving an OOM path open even after the wire cap.

Why This Change Was Made

The existing readProviderJsonResponse and readResponseWithLimit helpers already ship in the Plugin SDK with a default 16 MB cap. Using them here aligns with the established bounded-read pattern. The maxOutputLength option on gunzipSync (Node 15+) closes the decompression amplification path that the wire cap alone cannot address.

Real behavior proof

Behavior addressed: Bounded HTTP response reads and bounded gzip decompression for Google OAuth HTTP paths.

BEFORE FIX — unbounded response.arrayBuffer() reads entire body into memory:

const bytes = Buffer.from(await response.arrayBuffer());

AFTER FIX — bounded read with 1 MB cap and overflow error:

const bytes = await readResponseWithLimit(response, GOOGLE_OAUTH_TOKEN_RESPONSE_MAX_BYTES, {
  onOverflow: ({ maxBytes }) =>
    new Error(`Google OAuth token response exceeds ${maxBytes} bytes`),
});

BEFORE FIX — unbounded gunzipSync allocates full decompressed output:

return gunzipSync(bytes).toString("utf8");

AFTER FIX — bounded decompression with maxOutputLength:

return gunzipSync(bytes, { maxOutputLength: GOOGLE_OAUTH_TOKEN_RESPONSE_MAX_BYTES }).toString("utf8");

Real environment tested: Linux x86_64, Node v24.13.1.

Exact steps or command run after this patch:

# Unit tests
pnpm test extensions/google -- --reporter verbose
pnpm test:changed

# L2 proof: bounded gzip prevents decompression bomb
node evidence/pr-99812/bounded-gzip-proof.mjs

# L2 proof: loopback HTTP shows oversized response rejection
node evidence/pr-99812/bounded-http-proof.mjs

After-fix evidence: L2 runtime proof with loopback HTTP server and decompression bomb:

=== Bounded gzip decompression proof ===
Normal token response: 76 bytes uncompressed
  ✓ bounded: decompressed 76 bytes, parsed OK
Decompression bomb: 2068 bytes compressed → 2097153 bytes uncompressed
  unbounded gunzipSync: allocated 2097153 bytes (OOM risk)
  bounded gunzipSync: rejected — Cannot create a Buffer larger than 1048576 bytes

=== Bounded HTTP read proof ===
Normal response: 46 bytes — ✓ bounded read OK, parsed
Oversized response: caught — Response exceeds 1048576 bytes
  ✓ bounded read correctly rejected the oversized response

Unit tests:

 ✓ |extension-providers| extensions/google/... (28 test files, 378 tests passed)
 ✓ |extension-providers| test:changed (15 test files, 228 tests passed)

Observed result after the fix: All tests pass. Both loopback proof scripts confirm oversized responses and decompression bombs are safely rejected at the 1 MB boundary. Normal-sized responses pass through unchanged.

What was not tested: Actual Google OAuth HTTP round-trips (requires live credentials). The SDK helpers readResponseWithLimit and readProviderJsonResponse are integration-tested in src/agents/provider-http-errors.test.ts and packages/media-core/src/read-response-with-limit.test.ts. This PR only swaps the call sites and adds a maxOutputLength option, preserving all existing error handling and test seams.

Risk checklist

merge-risk: Low

  • Low risk — bounded-read wrapper replaces unbounded call with same return type and compatible error semantics
  • Decompression bomb path closed via maxOutputLength on gunzipSync
  • No behavior change for normal-sized responses under the cap
  • No API, config, or dependency changes
  • L2 runtime proof with loopback HTTP server (oversized response rejection) and decompression bomb (gzip amplification blocked)
  • Existing tests green (28 test files, 378 tests; 15 test:changed files, 228 tests)

Replace all unbounded response.arrayBuffer() and response.json() calls
in Google OAuth HTTP paths with bounded SDK helpers (readResponseWithLimit,
readProviderJsonResponse) that enforce configurable size caps.

7 call sites across 3 files fixed:
- extensions/google/vertex-adc.ts: token refresh response
- extensions/google/oauth.token.ts: token grant response
- extensions/google/oauth.project.ts: userinfo, poll, and project discovery
@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper review: did not complete due to Codex infrastructure failure. Reviewed July 4, 2026, 1:28 AM ET / 05:28 UTC.

Summary
Review failed before ClawSweeper could summarize the requested change.

PR surface: Source +17. Total +17 across 3 files.

Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path.

Review metrics: none identified.

Merge readiness
Not assessed.
Failure reason: timeout.

This is a ClawSweeper/Codex infrastructure failure, not a PR readiness or patch-quality verdict.
Keep any merge decision on the normal maintainer review path until ClawSweeper can complete a fresh review.

Risk before merge

  • [P1] No close action taken because the review did not complete.

Maintainer options:

  1. Decide the mitigation before merge
    Retry the Codex review after fixing the execution failure.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • Review did not complete, so no work-lane recommendation was made.
Review details

Best possible solution:

Retry the Codex review after fixing the execution failure.

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

Unclear. The review failed before ClawSweeper could establish a reproduction path.

Is this the best way to solve the issue?

Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against 8822b66952b7.

Label changes

Label changes:

  • remove P2: Current review triage priority is none.
  • remove rating: 🧂 unranked krab: Current review failed before PR readiness was assessed, so no rating label should remain.
  • remove merge-risk: 🚨 auth-provider: Current PR review selected no merge-risk labels.
  • remove status: 📣 needs proof: Current PR status no longer selects a status label.
Evidence reviewed

PR surface:

Source +17. Total +17 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 3 29 12 +17
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 29 12 +17

What I checked:

  • failure reason: timeout.
  • codex failure detail: Codex review failed for this PR: Codex process timed out after 1199999ms.
  • codex stderr: No stderr captured.
  • codex stdout: {"type":"thread.started","thread_id":"019f2b87-0f0d-7fc3-96bf-a69b7693934f"}.
  • process error code: ETIMEDOUT.

Likely related people:

  • unknown: Codex failed before it could trace repository history. (role: review did not complete; confidence: low)
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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels Jul 4, 2026
Add maxOutputLength to gunzipSync so a small compressed token response
cannot inflate past the 1 MB wire cap. Closes the decompression bomb
path that the wire cap alone cannot address.

Ref. openclaw#99812
@Pandah97

Pandah97 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Pushed commit f7745fc that adds maxOutputLength to gunzipSync and updated PR body with L2 loopback HTTP proof and decompression bomb demonstration.

@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

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

@Pandah97

Pandah97 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 4, 2026
@steipete steipete self-assigned this Jul 5, 2026
steipete added a commit that referenced this pull request Jul 5, 2026
steipete added a commit that referenced this pull request Jul 5, 2026
* fix(agents): harden LSP process failures

Source: #100450

Co-authored-by: morluto <[email protected]>

* fix(sandbox): report effective workspace layout

Sources: #100435, #100439

Co-authored-by: Aniruddha Adak <[email protected]>

Co-authored-by: ZengWen-DT <[email protected]>

* fix(security): fail install checks on stream errors

Source: #100413

Co-authored-by: 陈宪彪0668000387 <[email protected]>

* fix(android): normalize all-day calendar events

Source: #100032

Co-authored-by: NianJiuZst <[email protected]>

* fix(ios): serialize push-to-talk lifecycle

Source: #99942

Co-authored-by: NianJiuZst <[email protected]>

* fix(talk): reject inherited provider names

Source: #99849

Co-authored-by: zenglingbiao <[email protected]>

* fix(android): stop voice capture in background

Source: #99840

Co-authored-by: xialonglee <[email protected]>

* fix(cron): preserve fallback result classification

Source: #99913

Co-authored-by: jincheng-xydt <[email protected]>

* fix(google): bound Vertex response decompression

Source: #99812

Co-authored-by: 黄剑雄0668001315 <[email protected]>

* fix(plugins): report malformed discovery JSON

Source: #99892

Co-authored-by: 陈宪彪0668000387 <[email protected]>

* test(sandbox): configure non-default workspace fixture

* test: fix small-fix batch validation

---------

Co-authored-by: morluto <[email protected]>
Co-authored-by: ZengWen-DT <[email protected]>
Co-authored-by: 陈宪彪0668000387 <[email protected]>
Co-authored-by: NianJiuZst <[email protected]>
Co-authored-by: zenglingbiao <[email protected]>
Co-authored-by: xialonglee <[email protected]>
Co-authored-by: jincheng-xydt <[email protected]>
Co-authored-by: 黄剑雄0668001315 <[email protected]>
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Thanks @Pandah97 — your Vertex response-size finding was incorporated into the narrower canonical fix in #100483, landed as aaf5ab9. OAuth token/project responses are already bounded by the shared fetch helper, so the landed change targets the genuinely unbounded Vertex stream path and independently caps raw and gzip-decompressed output. Contributor credit is preserved. Closing this superseded PR.

@steipete steipete closed this Jul 5, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 6, 2026
* fix(agents): harden LSP process failures

Source: openclaw#100450

Co-authored-by: morluto <[email protected]>

* fix(sandbox): report effective workspace layout

Sources: openclaw#100435, openclaw#100439

Co-authored-by: Aniruddha Adak <[email protected]>

Co-authored-by: ZengWen-DT <[email protected]>

* fix(security): fail install checks on stream errors

Source: openclaw#100413

Co-authored-by: 陈宪彪0668000387 <[email protected]>

* fix(android): normalize all-day calendar events

Source: openclaw#100032

Co-authored-by: NianJiuZst <[email protected]>

* fix(ios): serialize push-to-talk lifecycle

Source: openclaw#99942

Co-authored-by: NianJiuZst <[email protected]>

* fix(talk): reject inherited provider names

Source: openclaw#99849

Co-authored-by: zenglingbiao <[email protected]>

* fix(android): stop voice capture in background

Source: openclaw#99840

Co-authored-by: xialonglee <[email protected]>

* fix(cron): preserve fallback result classification

Source: openclaw#99913

Co-authored-by: jincheng-xydt <[email protected]>

* fix(google): bound Vertex response decompression

Source: openclaw#99812

Co-authored-by: 黄剑雄0668001315 <[email protected]>

* fix(plugins): report malformed discovery JSON

Source: openclaw#99892

Co-authored-by: 陈宪彪0668000387 <[email protected]>

* test(sandbox): configure non-default workspace fixture

* test: fix small-fix batch validation

---------

Co-authored-by: morluto <[email protected]>
Co-authored-by: ZengWen-DT <[email protected]>
Co-authored-by: 陈宪彪0668000387 <[email protected]>
Co-authored-by: NianJiuZst <[email protected]>
Co-authored-by: zenglingbiao <[email protected]>
Co-authored-by: xialonglee <[email protected]>
Co-authored-by: jincheng-xydt <[email protected]>
Co-authored-by: 黄剑雄0668001315 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensions: google merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. P2 Normal backlog priority with limited blast radius. size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants