Skip to content

fix(openrouter): bound generation-cost JSON response reads#97490

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
hugenshen:fix/bound-openrouter-generation-cost
Jun 28, 2026
Merged

fix(openrouter): bound generation-cost JSON response reads#97490
vincentkoc merged 1 commit into
openclaw:mainfrom
hugenshen:fix/bound-openrouter-generation-cost

Conversation

@hugenshen

@hugenshen hugenshen commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

fetchOpenRouterGenerationTotalCost in extensions/openrouter/stream.ts reads the OpenRouter /generation metadata endpoint with an unbounded await response.json() on the success path. The error path was already protected — assertOkOrThrowHttpError routes through createProviderHttpError which uses the bounded reader — but the success path had no cap.

This is the same asymmetric pattern fixed for sibling OpenRouter paths:

The generation-cost lookup fires on every OpenRouter agent turn where usage.cost and responseId are present, making it the hottest remaining unbounded read in the extension. Because resolveOpenRouterGenerationUrl substitutes model.baseUrl when present, a misconfigured or hostile endpoint can stream an arbitrarily large body without Content-Length and force the process to buffer it before parsing — an OOM vector.

Why This Change Was Made

The error body on this path was already capped; leaving the success body uncapped is an asymmetric gap. readProviderJsonResponse applies the same 16 MiB cap used across every other provider JSON success path, and the lookup failure is already handled gracefully — errors are caught, logged at debug level, and the original streamed cost estimate is preserved — so the risk of this change is minimal.

User Impact

OpenRouter users with cost tracking enabled no longer risk OOM from an oversized generation-metadata response. Normal responses (well under 16 MiB) are unaffected.

Evidence

Proof script (scripts/proof-openrouter-generation-cost-bound.mjs) uses a real node:http server on 127.0.0.1, Node v22.22.0, no Content-Length, real fetch:

$ node scripts/proof-openrouter-generation-cost-bound.mjs
PASS oversized body rejected; threw="openrouter.generation-cost: JSON response exceeds 16777216 bytes"; serverSent=19202071; full=25165824
PASS stream cancelled at cap — server sent 19202071 < 25165824 (not full 25165824)
PASS negative control: raw response.json() caused server to send 25165850 bytes (> 20971520)
PASS small body parses correctly (total_cost=0.0042)

ALL PROOF ASSERTIONS PASSED

$ node scripts/run-vitest.mjs run extensions/openrouter/index.test.ts
 Test Files  1 passed (1)
      Tests  43 passed (43)
   Duration  11.98s

The new test "falls back to streamed cost estimate when generation metadata response is oversized" streams a body exceeding the cap via a real ReadableStream, verifies the bounded read rejects it, and confirms applyOpenRouterBilledCost falls back to the original streamed estimate rather than propagating the error.

What was not tested: live OpenRouter API calls.

AI-assisted.

@clawsweeper

clawsweeper Bot commented Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 28, 2026, 11:23 AM ET / 15:23 UTC.

Summary
The PR routes OpenRouter generation-cost success JSON parsing through readProviderJsonResponse and adds an oversized-response regression test that preserves the streamed cost estimate.

PR surface: Source +6, Tests +51. Total +57 across 2 files.

Reproducibility: yes. Current main has a direct unbounded response.json() on the generation metadata success path, and the PR body includes terminal loopback proof with a raw response.json() negative control; I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

  • Bounded JSON success reads: 1 changed. The only runtime parser change moves the OpenRouter generation-cost success body onto the existing shared provider JSON byte cap.

Stored data model
Persistent data-model change detected: vector/embedding metadata: extensions/openrouter/index.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #97490
Summary: This PR is the canonical current item for bounding the OpenRouter generation-cost success-body read; related items harden adjacent OpenRouter response surfaces.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

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.

Next step before merge

  • No ClawSweeper repair job is needed; the branch already contains the narrow code and regression-test fix, leaving maintainer review and normal merge gates.

Security
Cleared: The diff reduces provider-controlled response-body DoS exposure and adds no dependency, workflow, credential, lockfile, or supply-chain surface.

Review details

Best possible solution:

Land this focused helper reuse after normal maintainer review, keeping video submit/poll response bounding in its separate open PR.

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

Yes. Current main has a direct unbounded response.json() on the generation metadata success path, and the PR body includes terminal loopback proof with a raw response.json() negative control; I did not run tests because this review is read-only.

Is this the best way to solve the issue?

Yes. Reusing readProviderJsonResponse at the exact success parser is the narrowest maintainable fix and preserves the existing fail-open fallback to the streamed estimate when metadata lookup fails.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a focused OpenRouter provider hardening fix with limited blast radius and no evidence of an urgent live outage.
  • 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 includes terminal proof from a real Node loopback server with no Content-Length, a raw response.json() negative control, a small-body control, and focused OpenRouter test output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal proof from a real Node loopback server with no Content-Length, a raw response.json() negative control, a small-body control, and focused OpenRouter test output.
Evidence reviewed

PR surface:

Source +6, Tests +51. Total +57 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 7 1 +6
Tests 1 51 0 +51
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 58 1 +57

What I checked:

Likely related people:

  • steipete: Authored and merged the OpenRouter generation-cost reconciliation that added fetchOpenRouterGenerationTotalCost and the original success-path response.json() call. (role: introduced behavior; confidence: high; commits: 7c9a2eaff20a, 3006b85db022; files: extensions/openrouter/stream.ts, extensions/openrouter/index.test.ts)
  • Alix-007: Authored the merged OpenRouter model-scan bounded-body fix and the related open OpenRouter video response bounded-reader PR in the same response-body hardening family. (role: adjacent hardening contributor; confidence: high; commits: 285839b1e002, 06ca1235efb7, 0c9d84cfd2d0; files: src/agents/model-scan.ts, src/agents/model-scan.test.ts, extensions/openrouter/video-generation-provider.ts)
  • mushuiyu886: Authored the merged OpenRouter video catalog bounded JSON reader change using the same shared provider reader pattern. (role: sibling OpenRouter hardening contributor; confidence: medium; commits: 13d4c216e613, 5715b5500067; files: extensions/openrouter/video-model-catalog.ts, extensions/openrouter/video-generation-provider.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: 🦞 diamond lobster Very strong PR readiness with only minor 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. labels Jun 28, 2026
@vincentkoc
vincentkoc merged commit c0883a5 into openclaw:main Jun 28, 2026
130 of 138 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 28, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 29, 2026
@hugenshen
fix(openrouter): bound generation-cost JSON response reads (openclaw#…
0cc2fa1 
…97490)
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@hugenshen @QiuYuang
fix(openrouter): bound generation-cost JSON response reads (openclaw#…
9f3851c 
…97490)
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
@hugenshen chenyangjun-xy
fix(openrouter): bound generation-cost JSON response reads (openclaw#…
b9b7dd4 
…97490)
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 2, 2026
@hugenshen cron
fix(openrouter): bound generation-cost JSON response reads (openclaw#…
43ac536 
…97490)

(cherry picked from commit c0883a5)
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
@hugenshen @chenyangjun-xy
fix(openrouter): bound generation-cost JSON response reads (openclaw#…
71f8374 
…97490)
@mushuiyu886 mushuiyu886 mentioned this pull request Jul 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: openrouter 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hugenshen @vincentkoc