Skip to content

fix(google): bound OAuth token error response reads#99605

Merged
steipete merged 2 commits into
openclaw:mainfrom
mushuiyu886:fix/followup-97808
Jul 3, 2026
Merged

fix(google): bound OAuth token error response reads#99605
steipete merged 2 commits into
openclaw:mainfrom
mushuiyu886:fix/followup-97808

Conversation

@mushuiyu886

@mushuiyu886 mushuiyu886 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Bound Google Gemini CLI OAuth token error response reads before including provider error text in failures.
  • Add regression coverage for exchange and refresh failures so oversized diagnostic bodies are truncated instead of fully buffered.

What Problem This Solves

Google OAuth token grant failures previously used response.text() on non-OK responses. That fully consumes an external HTTP response body before throwing, so a malformed upstream/proxy/provider failure body could force OpenClaw to buffer more diagnostic text than needed.

User Impact

Users who hit Google Gemini CLI OAuth exchange or refresh failures still receive the provider diagnostic prefix, but the failure path no longer depends on buffering an unbounded response body.

  • Why it matters / User impact: OAuth setup or token refresh can fail during normal Gemini CLI provider use; this keeps that auth-provider failure path diagnosable without letting an external diagnostic body define memory usage.

Origin / follow-up

Follows #97808.

  • Gap this fills: fix(chutes): bound OAuth token error response reads #97808 bounded Chutes OAuth token error response reads; the Google Gemini CLI OAuth token grant path had the same unbounded non-OK body read pattern.
  • Consistency: This uses the existing readResponseTextLimited provider HTTP helper with the same 8 KiB diagnostic budget used by related OAuth/provider integrations.

Competition / linked PR analysis

No linked issue is being closed. I searched open PRs for the Google OAuth token response/body-read target and found no existing open PR covering this exact follow-up. The change is limited to extensions/google/oauth.token.ts plus targeted regression tests in extensions/google/oauth.test.ts.

Real behavior proof

  • Behavior or issue addressed: Google Gemini CLI OAuth token exchange failures now return a bounded provider diagnostic while exercising the real Google OAuth token endpoint failure path.
  • Real environment tested: Local OpenClaw checkout on Linux, current PR head 6c1d7950f1b8e9eccdc1c2481509c2635d7805fd, calling production exchangeCodeForTokens against https://oauth2.googleapis.com/token with an invalid redacted client id and invalid code.
  • Exact steps or command run after this patch:
OPENCLAW_GEMINI_OAUTH_CLIENT_ID=<redacted-invalid-client> \
OPENCLAW_GEMINI_OAUTH_CLIENT_SECRET= \
pnpm -C /media/vdb/code/ai/aispace/openclaw-worktrees/followup-97808 exec tsx -e '<import exchangeCodeForTokens and call it with an invalid OAuth code/verifier>'
  • Evidence after fix:
HEAD=6c1d7950f1b8e9eccdc1c2481509c2635d7805fd
Command: OPENCLAW_GEMINI_OAUTH_CLIENT_ID=<redacted-invalid-client> pnpm -C "$TASK_WORKTREE" exec tsx -e <exchangeCodeForTokens invalid code>
observed error prefix: Token exchange failed: {
  "error": "invalid_client",
  "error_d
contains Token exchange failed: true
message length: 112
  • Observed result after fix: The production Google OAuth token exchange path reached the real Google token endpoint, surfaced a Token exchange failed: diagnostic, and completed without requiring credentials or a successful account login.
  • What was not tested: I did not perform a successful live Google OAuth browser login or force Google to return an oversized live error body; Google controls the live error body size. Oversized-body truncation is covered separately by regression tests, not claimed as the real behavior proof.
  • Fix classification: Root cause fix

Review findings addressed

N/A — new follow-up PR, no review findings yet.

Regression Test Plan

  • Target test file: extensions/google/oauth.test.ts
  • Scenario locked in: Token exchange and token refresh failures with oversized diagnostic bodies return the 8 KiB bounded prefix and do not call response.text().
  • Why this is the smallest reliable guardrail: The test stays at the Google OAuth token helper boundary, covering both callers of requestTokenGrant without changing OAuth request construction, identity discovery, or project discovery behavior.
pnpm -C /media/vdb/code/ai/aispace/openclaw-worktrees/followup-97808 exec vitest run extensions/google/oauth.test.ts

Test Files  1 passed (1)
     Tests  29 passed (29)

Merge risk

  • Risk labels considered: auth-provider, security-boundary
  • Risk explanation: The touched file owns Google Gemini CLI OAuth token exchange/refresh handling, and OAuth failure diagnostics cross an external provider boundary.
  • Why acceptable: The diff only changes non-OK diagnostic body reading to the shared bounded helper; successful token JSON parsing, request parameters, headers, credential lookup, and identity/project discovery are unchanged, and the live proof exercises the real Google token endpoint failure path.

Risk / Compatibility

Low risk. Successful token responses still use the existing JSON parsing path. Only non-OK diagnostic text is capped, so callers keep the same error prefix with a bounded body preview.

What was not changed

  • What did NOT change: OAuth request parameters, headers, client credential discovery, identity discovery, token success parsing, Google project discovery, and personal OAuth mode behavior are all out of scope and unchanged.
  • Public API / schema / protocol / defaults: unchanged.
  • Channel/provider/session/security behavior outside the fixed path: unchanged.
  • Unrelated cleanup, docs, formatting, lockfiles: none.

Root Cause

  • Root cause: The source-of-truth Google OAuth token grant helper, requestTokenGrant, read non-OK Google OAuth token responses with response.text(), which fully buffered external diagnostic bodies before the auth-provider failure was reported.
  • Why this is root-cause fix: The non-OK token grant path now reads provider diagnostics through readResponseTextLimited, bounding the body read at the source before the error is constructed, rather than trimming after an unbounded buffer already exists.
  • Maintainer-ready confidence: High; the patch is a two-file auth-provider follow-up, uses the shared provider HTTP helper, has fresh live endpoint proof for the failure path, and has targeted regression coverage for the oversized-body guardrail.
  • Patch quality notes: Patch quality warning considered: this touches an auth-provider/security-boundary file, but it is not a fallback, catch-all, or symptom mask; it removes the unbounded read at the owning helper and leaves successful OAuth behavior unchanged.
  • Architecture / source-of-truth check: extensions/google/oauth.token.ts is the owner for Gemini CLI OAuth token exchange and refresh grants; the change happens before downstream credential construction, identity discovery, and project discovery, so the diagnostic body is bounded at the provider boundary.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 6:44 PM ET / 22:44 UTC.

Summary
The PR changes the Google plugin's Gemini CLI OAuth token grant error path to read non-OK diagnostic bodies through readResponseTextLimited with an 8 KiB budget and adds exchange/refresh regression coverage.

Reproducibility: yes. Current main and v2026.6.11 still use response.text() in the Google OAuth token grant failure path, and the PR discussion plus tests exercise exchange and refresh failures with bounded diagnostics.

Review metrics: 1 noteworthy metric.

  • OAuth Diagnostic Cap: 1 added: 8 KiB non-OK token error body cap. This is the intentional auth-provider behavior change maintainers should notice before merge.

Stored data model
Persistent data-model change detected: migration/backfill/repair: scripts/check-kysely-guardrails.mjs, migration/backfill/repair: src/commands/doctor-whatsapp-responsiveness.test.ts, migration/backfill/repair: src/commands/doctor-whatsapp-responsiveness.ts, migration/backfill/repair: src/commands/doctor/cron/index.test.ts, migration/backfill/repair: src/commands/doctor/cron/index.ts, migration/backfill/repair: src/commands/doctor/cron/migration-ledger.ts, and 23 more. Confirm migration or upgrade compatibility proof before merge.

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

Maintainer options:

  1. Accept The 8 KiB Diagnostic Budget (recommended)
    Land the focused cap after required checks, accepting that oversized Google OAuth token failure diagnostics beyond 8 KiB are intentionally omitted.
  2. Fold Into Parser Hardening
    If maintainers want one Google OAuth hardening branch, move this error-text cap into fix(google): bound OAuth project and token JSON response reads #97587 and close or supersede duplicate attempts.
  3. Pause For Canonical Cleanup
    If the overlapping Google OAuth PRs are still unsettled, pause this PR until maintainers choose the canonical landing order.

Next step before merge

  • [P2] Maintainers should choose the Google OAuth hardening landing order and wait for exact-head gates; no narrow repair lane is needed.

Security
Cleared: The actual merge result reduces external-provider diagnostic body materialization and adds tests, with no new dependencies, workflows, permissions, package metadata, or secret-handling surface.

Review details

Best possible solution:

Land this focused diagnostic cap after exact-head checks are acceptable, while keeping JSON parser hardening in the separate Google OAuth PR unless maintainers choose to fold the changes together.

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

Yes. Current main and v2026.6.11 still use response.text() in the Google OAuth token grant failure path, and the PR discussion plus tests exercise exchange and refresh failures with bounded diagnostics.

Is this the best way to solve the issue?

Yes. Replacing the Google plugin's non-OK diagnostic read with the existing shared bounded reader is the narrow owner-boundary fix; broader JSON parser hardening should remain coordinated through the separate open PR.

AGENTS.md: found and applied where relevant.

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

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 a focused Google auth-provider hardening fix with limited blast radius and no evidence of an active outage.
  • merge-risk: 🚨 auth-provider: The PR changes Google OAuth token failure diagnostics by capping provider error text, so maintainers should explicitly accept that auth-provider behavior.
  • 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 (terminal): The PR body and maintainer comment provide redacted terminal/live-output proof of the Google token failure path plus focused local and remote Testbox validation for the oversized-body guardrail.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and maintainer comment provide redacted terminal/live-output proof of the Google token failure path plus focused local and remote Testbox validation for the oversized-body guardrail.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Recent history shows the Google OAuth module split that owns the current oauth.token.ts shape and the maintainer proof comment on this PR. (role: recent area contributor; confidence: high; commits: 92e765cdee15; files: extensions/google/oauth.token.ts, extensions/google/oauth.test.ts)
  • jason-allen-oneal: The merged Gemini CLI OAuth refresh PR added the shared token grant helper/error path now being hardened. (role: introduced behavior; confidence: high; commits: d5035bad6231; files: extensions/google/oauth.token.ts, extensions/google/oauth.test.ts)
  • vincentkoc: Merged the Gemini CLI OAuth refresh PR and authored adjacent Google personal OAuth work in the same token/test area. (role: merger and adjacent contributor; confidence: high; commits: d5035bad6231, f02e43518826; files: extensions/google/oauth.token.ts, extensions/google/oauth.test.ts)
  • wangmiao0668000666: Authored the current shared Google OAuth fetchWithTimeout 16 MiB response cap adjacent to this tighter diagnostic cap. (role: adjacent bounded-read contributor; confidence: medium; commits: a6aaba76ac66; files: extensions/google/oauth.http.ts, extensions/google/oauth.http.test.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 proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels Jul 3, 2026
@steipete steipete self-assigned this Jul 3, 2026
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation app: android App: android app: ios App: ios app: macos App: macos gateway Gateway runtime scripts Repository scripts commands Command implementations extensions: qa-lab size: XL and removed size: XS labels Jul 3, 2026
@steipete

steipete commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Maintainer proof for clean exact head cfc513cf86834ccb96b176e264ed77fd86832a74:

  • Best-fix review: the Google plugin is the correct owner for its 8 KiB diagnostic budget; readResponseTextLimited is the canonical shared byte-bounded decoder. Successful token JSON parsing, request parameters, identity discovery, and credential storage are unchanged.
  • Improvement: consolidated the exchange/refresh regressions into one parameterized test while retaining distinct cases and the response.text() trap.
  • Stack hygiene: rewrote the contributor branch onto current main after detecting that an additive sync had carried unrelated mainline history. The PR is now exactly two commits and two files: extensions/google/oauth.token.ts and extensions/google/oauth.test.ts.
  • Focused local proof after the clean rewrite: node scripts/run-vitest.mjs extensions/google/oauth.test.ts — 29 tests passed.
  • Real endpoint proof: production exchangeCodeForTokens reached https://oauth2.googleapis.com/token with a non-secret invalid client id and returned the expected Token exchange failed: { "error": "invalid_client" ... } path.
  • Remote scenario proof: Blacksmith Testbox-through-Crabbox tbx_01kwn1c7t6tmyn49em87sfzjej, Actions run 28685932959 — focused test 29/29 plus the live Google endpoint path, wrapper exitCode: 0.
  • Remote changed gate: Blacksmith Testbox-through-Crabbox tbx_01kwn1h5mkc6dscb6kw3yn6x44, Actions run 28686012238 — extension production/test typechecks, changed-file lint, guards, and import-cycle check passed; wrapper exitCode: 0.
  • Fresh autoreview after the rewrite: .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --stream-engine-output — clean, no accepted/actionable findings; correctness confidence 0.94.
  • Provenance: the unbounded diagnostic read came from fix(google): refresh Gemini CLI OAuth tokens #78030, authored by @jason-allen-oneal and merged by @vincentkoc on 2026-05-16. This is ordinary follow-up hardening, not a fault attribution.

Proof boundary: Google controls live error-body size, so the live call proves the production endpoint/error path while the focused regression proves oversized-body truncation. The shared Google transport already imposed a 16 MiB response cap; this change specifically prevents materializing more than 8 KiB as diagnostic text.

The local prepared tree and pushed PR tree are identical (d643f062523d7780658db86f2d96d76699d35a24). Fresh hosted CI is running on the rewritten exact head; merge remains gated on that head turning green.

@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 Jul 3, 2026
@openclaw-barnacle openclaw-barnacle Bot added the channel: slack Channel integration: slack label Jul 3, 2026
@steipete
steipete force-pushed the fix/followup-97808 branch from 456cf8b to cfc513c Compare July 3, 2026 22:51
@openclaw-barnacle openclaw-barnacle Bot removed docs Improvements or additions to documentation channel: slack Channel integration: slack app: android App: android app: ios App: ios labels Jul 3, 2026
@openclaw-barnacle openclaw-barnacle Bot added size: XS and removed app: macos App: macos gateway Gateway runtime scripts Repository scripts commands Command implementations extensions: qa-lab size: XL labels Jul 3, 2026
@steipete
steipete merged commit 6207a4e into openclaw:main Jul 3, 2026
101 of 105 checks passed
@steipete

steipete commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 4, 2026
@mushuiyu886 @steipete
fix(google): bound OAuth token error response reads (openclaw#99605)
cd00a30 
* fix(google): bound OAuth token error response reads

* test(google): consolidate OAuth error cases

---------

Co-authored-by: Peter Steinberger <[email protected]>
@mushuiyu886 mushuiyu886 mentioned this pull request Jul 4, 2026
Closed
This was referenced Jul 4, 2026
Closed
Merged
This was referenced Jul 6, 2026
Closed
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jul 17, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

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: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mushuiyu886 @steipete