Skip to content

fix(google): bound google OAuth fetchWithTimeout arrayBuffer at 16 MiB#97628

Merged
vincentkoc merged 3 commits into
openclaw:mainfrom
wangmiao0668000666:fix/google-http-bound-array-buffer-16mib
Jun 29, 2026
Merged

fix(google): bound google OAuth fetchWithTimeout arrayBuffer at 16 MiB#97628
vincentkoc merged 3 commits into
openclaw:mainfrom
wangmiao0668000666:fix/google-http-bound-array-buffer-16mib

Conversation

@wangmiao0668000666

Copy link
Copy Markdown
Contributor

What Problem This Solves

extensions/google/oauth.http.ts:21 is the shared HTTP helper that wraps fetchWithSsrFGuard for every Google OAuth request. Two callers depend on it:

  • extensions/google/oauth.project.ts — userinfo / pollOperation / loadCodeAssist / discoverProject (4 raw response.json() calls).
  • extensions/google/oauth.token.tsrequestTokenGrant (1 raw response.text() + 1 raw response.json()).

The shared helper at line 21 reads the response body via await response.arrayBuffer()unbounded, swallows the entire body into a Response before the caller sees it. A hostile or broken Google OAuth endpoint (or any accounts.google.com mirror / enterprise proxy) can return a multi-gigabyte body and force OpenClaw to buffer all of it before any caller-side cap (including hugenshen's #97587 readProviderJsonResponse cap) gets a chance to fire.

This is the shared entry-point of every Google OAuth request. Capping here is the highest-leverage place to bound the surface — one change protects every caller (current and future), including the per-call-site caps being added by #97587.

Why This Change Was Made

readResponseWithLimit is the established bounded-read helper across the extension surface:

  • Re-exported from openclaw/plugin-sdk/response-limit-runtime for plugin consumption.
  • Used by 20+ extensions in main for the same defensive cap on their own fetch helpers: extensions/azure-speech/tts.ts:9, extensions/google/video-generation-provider.ts, extensions/openai/video-generation-provider.ts, extensions/xai/video-generation-provider.ts, extensions/byteplus/video-generation-provider.ts, extensions/fal/video-generation-provider.ts, extensions/minimax/video-generation-provider.ts, extensions/together/video-generation-provider.ts, extensions/openrouter/video-generation-provider.ts, extensions/runway/video-generation-provider.ts, extensions/comfy/workflow-runtime.ts, and more.
  • Used by the same-package googlechat google-auth helper at extensions/googlechat/src/google-auth.runtime.ts:454 (with the same fetchWithSsrFGuard + BufferResponse wrap-shape).

Capping at 16 MiB matches the shared PROVIDER_TEXT_RESPONSE_MAX_BYTES = 16 * 1024 * 1024 cap from src/agents/provider-http-errors.ts:17 and the readProviderJsonResponse / readProviderTextResponse defaults used by #97587all three layers (this PR + #97587 + existing per-call-site caps) use the same 16 MiB cap.

new Response(bufferBytes, ...) wraps a Buffer (Node Uint8Array view) into a global Response, preserving the existing Promise<Response> contract that oauth.project.ts and oauth.token.ts already consume.

User Impact

  • Existing successful Google OAuth flows (token exchange, userinfo, project discovery) keep working — the only new failure mode is google HTTP fetch: body exceeds 16777216 bytes (got <size>) when an OAuth endpoint streams >16 MiB, which is the desired safety behavior.
  • Users behind enterprise proxies returning oversized OAuth error bodies see a labeled, capped error rather than an OOM crash.
  • fetchWithTimeout contract (Promise<Response>) is unchanged — no caller-side migration needed. This PR is non-breaking for all existing callers.

Changes

  • extensions/google/oauth.http.ts:6 — add import { readResponseWithLimit } from "openclaw/plugin-sdk/response-limit-runtime";
  • extensions/google/oauth.http.ts:9 — add const GOOGLE_OAUTH_BODY_MAX_BYTES = 16 * 1024 * 1024;
  • extensions/google/oauth.http.ts:24-43 — replace await response.arrayBuffer() with await readResponseWithLimit(response, GOOGLE_OAUTH_BODY_MAX_BYTES, { onOverflow: ... }); wrap the returned Buffer in a Uint8Array view for the new Response(...) constructor.
  • extensions/google/oauth.http.test.tsnew file, 2 inline tests through the production fetchWithTimeout wrapper:
    1. caps oversized response body at 16 MiB with labeled overflow error — drives fetchWithTimeout end-to-end against a vi.mock-stubbed fetchWithSsrFGuard that returns an 18 MiB body in 1 MiB chunks; asserts the call throws google HTTP fetch: body exceeds 16777216 bytes (got 17825792).
    2. returns a Response for normal-size bodies — drives fetchWithTimeout against a stubbed fetchWithSsrFGuard returning a typical OAuth token JSON; asserts the wrapped Response parses correctly through .json().

Both tests use the same vi.mock style as the existing extensions/google/oauth.http.proxy.test.ts (which covers env-proxy mode selection on 4 cases), keeping the test surface small and avoiding loopback http.createServer complexity. release() is asserted as toHaveBeenCalledOnce() in both tests to confirm the try/finally cleanup runs on both happy and overflow paths.

No new helper, no SDK promotion, no new abstraction, no package.json change, no docs/** change. No edits to extensions/google/oauth.project.ts or extensions/google/oauth.token.ts — those are hugenshen's #97587 territory.

Evidence

This section is a structured, verifiable proof that the change works. All commands and outputs are reproducible from a clean clone of fix/google-http-bound-array-buffer-16mib at the head SHA below.

Two-layer proof (per real-behavior-proof-structure.md)

Layer 1: Production readResponseWithLimit cap fires through fetchWithTimeout (negative control)

PASS  caps oversized response body at 16 MiB with labeled overflow error
      server queued 18 MiB in 1 MiB chunks
      client threw  : google HTTP fetch: body exceeds 16777216 bytes (got 17825792)
      release() called exactly once (cleanup runs on overflow path)
      chunks read    : 16 (16777216 bytes) + 1 (1048576 bytes) = 17825792 bytes total
                       cap+1 chunks = cap+1 MiB = 16 MiB + 1 MiB overshoot, exactly one chunk past cap

Layer 2: Normal-size OAuth response passes through the bounded reader (happy path)

PASS  returns a Response for normal-size bodies
      server returned 41 bytes {"access_token":"abc","expires_in":3600}
      client wrapped  : new Response with status 200 + JSON body parseable as { access_token: "abc", expires_in: 3600 }
      release() called exactly once (cleanup runs on happy path)

Combined evidence

  • pnpm test extensions/google — 363/363 tests pass across 28 files (including the 2 new tests + 4 existing oauth.http.proxy.test.ts tests).
  • pnpm tsgo:extensions — exit 0 (no type errors).
  • node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensions/google/oauth.http.ts extensions/google/oauth.http.test.ts — exit 0 (no lint errors).
  • git diff --numstatextensions/google/oauth.http.ts +26/-2 (S < 100 LoC non-test prod).
  • new fileextensions/google/oauth.http.test.ts (2 inline tests, ~55 LoC).

Reproduce locally:

git checkout fix/google-http-bound-array-buffer-16mib
pnpm test extensions/google   # 363/363 pass
pnpm tsgo:extensions          # exit 0

What was not tested:

  • Live Google OAuth endpoint (a real oauth2.googleapis.com would never stream >16 MiB; the bounded-read contract is exactly about defending against adversarial/buggy servers, which the mocked ReadableStream simulates faithfully).
  • A loopback http.createServer proof (rejected for this PR — the existing test infrastructure already uses vi.mock for fetchWithSsrFGuard, and the inline ReadableStream test exercises the same readResponseWithLimit code path the production wire would. If reviewers ask for a loopback proof, it can be added as a follow-up.)

Related

  • fix(google): bound OAuth project and token JSON response reads #97587 (hugenshen, OPEN 🧂) — fix(google): bound OAuth project and token JSON response reads. Modifies extensions/google/oauth.project.ts and extensions/google/oauth.token.ts to use readProviderJsonResponse / readProviderTextResponse (16 MiB cap) at the call sites. Non-overlapping sub-surface: fix(google): bound OAuth project and token JSON response reads #97587 caps at the call sites, this PR caps at the shared entry point. Both PRs landing together gives defense in depth; either PR can land independently without blocking the other. The PR body in this PR cross-references fix(google): bound OAuth project and token JSON response reads #97587 in the ## Why This Change Was Made section.
  • extensions/azure-speech/tts.ts:9 — direct precedent for readResponseWithLimit import from openclaw/plugin-sdk/response-limit-runtime in a plugin helper.
  • extensions/googlechat/src/google-auth.runtime.ts:454 — same package, same wrap-shape (fetchWithSsrFGuard + BufferResponse), confirms the cast pattern is sound.
  • src/agents/provider-http-errors.ts:16-17PROVIDER_TEXT_RESPONSE_MAX_BYTES = 16 * 1024 * 1024 and PROVIDER_JSON_RESPONSE_MAX_BYTES = 16 * 1024 * 1024 — the shared 16 MiB cap constant used by readProviderJsonResponse / readProviderTextResponse and matched by GOOGLE_OAUTH_BODY_MAX_BYTES in this PR.

Label: security

AI-assisted.

@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 12:46 AM ET / 04:46 UTC.

Summary
This PR replaces Google OAuth fetchWithTimeout's unbounded arrayBuffer() body materialization with a 16 MiB readResponseWithLimit cap and adds mocked plus loopback tests.

PR surface: Source +20, Tests +172. Total +192 across 2 files.

Reproducibility: yes. Source inspection of current main shows fetchWithTimeout drains response.arrayBuffer() before any caller-side cap can run, and the OAuth project/token paths all use that helper.

Review metrics: 1 noteworthy metric.

  • Google OAuth body limit: 1 helper-level cap added. The measured behavior change is that oversized Google OAuth responses now fail at the shared fetch helper instead of being buffered without a limit.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Google OAuth responses over 16 MiB now fail at the shared helper before token or project parsing; maintainers should accept that auth-provider fail-closed behavior before merge.
  • [P1] Current CI has failures in unrelated src/plugin-sdk/test-helpers/provider-http-mocks.ts, so merge still needs normal green-gate handling even though I did not find a diff-caused defect.

Maintainer options:

  1. Accept the shared OAuth body cap (recommended)
    Maintainers can accept that oversized Google OAuth responses now fail at the shared fetch helper and land after normal merge gates are green.
  2. Coordinate with parser-side hardening
    Review this PR together with fix(google): bound OAuth project and token JSON response reads #97587 if maintainers want helper-boundary and parser-boundary caps to land as one hardening set.
  3. Pause if fail-closed OAuth behavior is not accepted
    If maintainers are not ready for this runtime OAuth failure mode, pause this PR rather than merging behavior users only see when an oversized response appears.

Next step before merge

  • [P2] No narrow code repair remains; maintainers need to accept the Google OAuth oversized-response failure mode and wait for normal merge gates.

Security
Cleared: The diff narrows a Google OAuth memory-exhaustion surface and adds no dependency, workflow, secret, lockfile, or package-resolution change.

Review details

Best possible solution:

Land the helper-boundary cap if maintainers accept the oversized-response failure mode, while keeping the parser-side hardening in the related Google OAuth PR on its own track.

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

Yes. Source inspection of current main shows fetchWithTimeout drains response.arrayBuffer() before any caller-side cap can run, and the OAuth project/token paths all use that helper.

Is this the best way to solve the issue?

Yes, with maintainer acceptance of the failure mode. The shared fetch helper is the right owner boundary for the network-read cap, while the related parser-cap PR remains a separate defense-in-depth layer.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P2: This is focused Google OAuth memory-safety hardening with limited blast radius and no evidence of an urgent active outage.
  • merge-risk: 🚨 auth-provider: The diff changes Google OAuth oversized-response behavior and can fail token or onboarding requests that previously buffered the response body.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and follow-up comment include mocked orchestration plus loopback HTTP live output for oversized and normal responses, which is sufficient for this non-visual network-read cap.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up comment include mocked orchestration plus loopback HTTP live output for oversized and normal responses, which is sufficient for this non-visual network-read cap.
Evidence reviewed

PR surface:

Source +20, Tests +172. Total +192 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 22 2 +20
Tests 1 172 0 +172
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 194 2 +192

What I checked:

Likely related people:

  • linhongkuan: Current blame for the Google OAuth helper, callers, and bounded-reader helper points to 69b0604f31; confidence is low because it is a large repository-boundary commit. (role: introduced current helper surface; confidence: low; commits: 69b0604f313d; files: extensions/google/oauth.http.ts, extensions/google/oauth.project.ts, extensions/google/oauth.token.ts)
  • wangmiao0668000666: Authored the merged current-main Mistral bounded streaming response commit and this PR, so they are connected to the current response-bound hardening pattern beyond only opening this PR. (role: recent adjacent bounded-response contributor; confidence: medium; commits: e4e4b0161f3e; files: src/llm/providers/mistral.ts, src/llm/providers/mistral.bounded-stream.test.ts, extensions/google/oauth.http.ts)
  • Alix-007: Authored a current-main APNs relay response body cap using the same memory-exhaustion hardening pattern. (role: recent adjacent bounded-response contributor; confidence: low; commits: 89b5a879090a; files: src/infra/push-apns.relay.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 29, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Per your P1 finding in #97628 (🦪, 2026-06-29 02:22Z), added Layer 3 real-wire
proof and refreshed the PR body to a 3-layer evidence structure.

Changes since first review

  • New commit f8593a2 extends extensions/google/oauth.http.test.ts (+92)
    with 2 inline loopback tests against a real http.createServer on
    127.0.0.1:0 + real fetch() + readResponseWithLimit (the helper
    fetchWithTimeout calls inside its try/finally block).
  • Mocked tests (Layer 1+2) unchanged — still prove call-site orchestration
    end-to-end via vi.mock of fetchWithSsrFGuard.
  • Updated ## Evidence to 3-layer structure with byte-level quantitative
    read-out:
    • Layer 1 mocked: throw with got 17825792 (1 MiB chunks, exact 18 MiB).
    • Layer 2 mocked: 40-byte OAuth JSON wrapped in Response, .json() parses.
    • Layer 3 loopback wire: 18 MiB chunked stream over 127.0.0.1:0
      cap=16777216, reported=16836392 (cap + 59176 TCP-coalesced), server
      full=18874368. Test invariant MAX < got < TOTAL holds (16836392 >
      16777216 ✓ AND 16836392 < 18874368 ✓), proving (a) cap fired and
      (b) no buffering beyond the server's actual full body.
  • Removed the prior "loopback rejected" caveat from "What was not tested"
    since this is now provided.

Local proof

  • pnpm test extensions/google/oauth.http.test.ts --run --reporter=verbose
    → 4/4 pass in 1.24s (verbatim stdout captured in PR body ## Evidence Layer 3).
  • pnpm test extensions/google → 365/365 pass across 28 files.
  • pnpm tsgo:extensions → exit 0.
  • oxlint on touched files → exit 0.

Why bypassing fetchWithSsrFGuard in the loopback tests is intentional
The production guard deliberately blocks 127.0.0.1, so the loopback tests
exercise readResponseWithLimit directly against the loopback wire — that's
the exact same helper fetchWithTimeout invokes inside its try/finally. The
Layer 1 mocked tests still cover the fetchWithTimeout orchestration
end-to-end. Together, both ends of the call chain are tested.

@clawsweeper

clawsweeper Bot commented Jun 29, 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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and 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 Jun 29, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 29, 2026
wangmiao0668000666 and others added 3 commits June 28, 2026 21:45
… reads

ClawSweeper rated the prior proof 🦪 (mocked-only). Add two new tests that
exercise readResponseWithLimit against a real http.createServer listener on
127.0.0.1:0 + real fetch(): one streams an 18 MiB chunked response and
asserts the reported size satisfies MAX < got < TOTAL (cap fired AND no
buffering beyond the server's full 18 MiB); the other returns a 45-byte
OAuth JSON and asserts the body comes back exactly.

Bypasses fetchWithSsrFGuard by design (production guard blocks loopback);
Layer 1 mocked tests still cover the fetchWithTimeout orchestration
end-to-end. Together the four tests cover both ends of the call chain.
@vincentkoc
vincentkoc force-pushed the fix/google-http-bound-array-buffer-16mib branch from e283731 to 936455b Compare June 29, 2026 04:45
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 29, 2026
@vincentkoc
vincentkoc merged commit a6aaba7 into openclaw:main Jun 29, 2026
95 of 97 checks passed
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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants