Skip to content

fix(realtime-transcription): bound inbound websocket payloads#102443

Merged
steipete merged 2 commits into
openclaw:mainfrom
sunlit-deng:sunlit/fix/realtime-transcription-ws-maxpayload
Jul 9, 2026
Merged

fix(realtime-transcription): bound inbound websocket payloads#102443
steipete merged 2 commits into
openclaw:mainfrom
sunlit-deng:sunlit/fix/realtime-transcription-ws-maxpayload

Conversation

@sunlit-deng

@sunlit-deng sunlit-deng commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where a realtime transcription websocket session would buffer an arbitrarily large inbound frame from the upstream provider before parsing it. The generic session (src/realtime-transcription/websocket-session.ts) backs every websocket transcription provider (OpenAI, Mistral, xAI, ElevenLabs, Deepgram), so a buggy or hostile upstream could push an unbounded frame straight into JSON.parse with no size ceiling.

Why This Change Was Made

The session now passes maxPayload to the ws client, matching the existing realtime voice cap. ws rejects an over-limit frame with an error and a 1009 close before it assembles the message, so an oversized frame never reaches onMessage/parseMessage. The 16 MiB cap leaves large headroom over real transcript frames (KB-scale) while replacing ws's 100 MiB default.

User Impact

Operators running any websocket transcription provider get a bounded failure on a malformed oversized frame instead of unbounded buffering; legitimate transcript traffic is unaffected.

Evidence

  • pnpm test src/realtime-transcription/websocket-session.test.ts — 12 passed, incl. two new boundary tests (legit large frame delivered, oversized frame dropped before the parser).
  • tsgo:core and tsgo:core:test clean; oxlint on changed files clean.
  • Live loopback proof importing the real changed module (createRealtimeTranscriptionWebSocketSession) over a real ws connection, both boundary sides:
$ tsx proof-live.ts
cap REALTIME_TRANSCRIPTION_WS_MAX_PAYLOAD_BYTES = 16777216 bytes (16 MiB)
legit-2MiB:        frameBytes=2097183     reachedParser=true  onError=none
oversized-16MiB+1: frameBytes=16777217    reachedParser=false onError=Max payload size exceeded
proof-live.ts 
import { createServer } from "node:http";
import type { AddressInfo } from "node:net";
import { WebSocketServer } from "ws";
import {
  createRealtimeTranscriptionWebSocketSession,
  REALTIME_TRANSCRIPTION_WS_MAX_PAYLOAD_BYTES,
} from "./src/realtime-transcription/websocket-session.js";

async function startServer(sendFrame: string) {
  const server = createServer();
  const wss = new WebSocketServer({ noServer: true, maxPayload: 64 * 1024 * 1024 });
  server.on("upgrade", (req, socket, head) => {
    wss.handleUpgrade(req, socket, head, (ws) => ws.send(sendFrame));
  });
  await new Promise<void>((r) => server.listen(0, "127.0.0.1", r));
  const port = (server.address() as AddressInfo).port;
  return { url: `ws://127.0.0.1:${port}` };
}

async function run(label: string, frame: string) {
  const server = await startServer(frame);
  let delivered = false;
  let errored: string | undefined;
  const session = createRealtimeTranscriptionWebSocketSession({
    providerId: "proof",
    callbacks: { onError: (e) => (errored = e.message) },
    url: server.url,
    readyOnOpen: true,
    onMessage: () => (delivered = true),
    sendAudio: () => {},
  });
  await session.connect();
  await new Promise((r) => setTimeout(r, 400));
  session.close();
  console.log(`${label}: frameBytes=${frame.length} reachedParser=${delivered} onError=${errored ?? "none"}`);
}

async function main() {
  console.log(`cap = ${REALTIME_TRANSCRIPTION_WS_MAX_PAYLOAD_BYTES} bytes (16 MiB)`);
  await run("legit-2MiB", JSON.stringify({ type: "transcript", text: "x".repeat(2 * 1024 * 1024) }));
  await run("oversized-16MiB+1", "x".repeat(REALTIME_TRANSCRIPTION_WS_MAX_PAYLOAD_BYTES + 1));
}

void main().then(() => process.exit(0));

AI-assisted: built with Codex

@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 6:36 AM ET / 10:36 UTC.

Summary
The PR adds a 16 MiB ws inbound message cap to the shared realtime transcription websocket session and adds boundary tests for below-cap delivery and over-cap rejection.

PR surface: Source +5, Tests +63. Total +68 across 2 files.

Reproducibility: yes. Source inspection shows current main opens the shared realtime transcription WebSocket without maxPayload and parses inbound messages afterward; the PR body includes live loopback output for both sides of the new boundary.

Review metrics: 1 noteworthy metric.

  • Runtime Payload Cap: 1 added: 16 MiB inbound websocket message cap. It lowers the accepted inbound message size from ws's 100 MiB default across every provider using the shared realtime transcription session.

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

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

Rank-up moves:

  • [P2] Have the assigned maintainer accept or tune the 16 MiB shared transcription cap before merge.

Risk before merge

  • [P1] Existing deployments receiving a legitimate realtime transcription provider message above 16 MiB would now get a ws payload error and close/reconnect instead of being accepted up to the previous 100 MiB default.

Maintainer options:

  1. Accept The Shared Cap (recommended)
    Maintainers can accept 16 MiB as the shared safety boundary because it matches the realtime voice precedent and has below/above boundary proof.
  2. Adjust The Cap First
    If supported providers can emit larger legitimate messages, update the constant and tests before merge.
  3. Pause Until Provider Contracts Are Checked
    If the provider frame-size contract is unknown, pause the PR until owner review confirms the limit.

Next step before merge

  • Manual review is the right lane because the implementation is mechanically narrow, but accepting the 16 MiB all-provider cap is a maintainer compatibility and resource-safety decision.

Maintainer decision needed

  • Question: Should the shared realtime transcription websocket helper fail closed at 16 MiB for every bundled websocket transcription provider?
  • Rationale: The code and dependency contract are clear, but choosing a lower-than-ws default cap across all supported transcription providers is a maintainer-owned resource-safety and compatibility decision.
  • Likely owner: steipete — This user is assigned to the PR and has the strongest history on the shared realtime transcription helper and SDK surface.
  • Options:
    • Accept 16 MiB Cap (recommended): Merge the hardening once final mergeability is refreshed, accepting bounded failures for provider messages above 16 MiB as the intended safety default.
    • Tune The Ceiling: Ask for a different constant and matching boundary tests if any supported provider can legitimately send larger realtime transcription messages.
    • Pause For Provider Evidence: Hold the PR until provider docs, telemetry, or owner review confirms the all-provider message-size ceiling.

Security
Cleared: No concrete supply-chain or secret-handling regression was found; the diff tightens a websocket resource boundary without dependency, workflow, lockfile, or credential changes.

Review details

Best possible solution:

Land the shared transport-level cap after the assigned maintainer accepts 16 MiB as the intended all-provider transcription ceiling, or tune the constant and boundary tests to a provider-backed ceiling before merge.

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

Yes. Source inspection shows current main opens the shared realtime transcription WebSocket without maxPayload and parses inbound messages afterward; the PR body includes live loopback output for both sides of the new boundary.

Is this the best way to solve the issue?

Yes, subject to maintainer acceptance of the cap value. The shared session helper is the narrowest maintainable boundary because all bundled websocket transcription providers route through it; per-provider caps would duplicate policy.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR hardens a focused realtime transcription runtime path with limited blast radius and no evidence of an active outage.
  • merge-risk: 🚨 compatibility: A provider message above 16 MiB would now fail with a websocket payload error instead of being accepted up to the previous ws 100 MiB default.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes live loopback output using the real changed module over a real ws connection, showing a 2 MiB message reaches the parser and a 16 MiB + 1 message is rejected before parsing.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes live loopback output using the real changed module over a real ws connection, showing a 2 MiB message reaches the parser and a 16 MiB + 1 message is rejected before parsing.
Evidence reviewed

PR surface:

Source +5, Tests +63. Total +68 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 5 0 +5
Tests 1 64 1 +63
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 69 1 +68

What I checked:

Likely related people:

  • steipete: GitHub history shows this user introduced the shared realtime transcription websocket helper and recently maintained OpenAI realtime transcription; the live PR is assigned to this user. (role: feature introducer and likely decision owner; confidence: high; commits: 0e7bcf7588d2, 9566aded5cf8, 675b834a99b3; files: src/realtime-transcription/websocket-session.ts, src/plugin-sdk/realtime-transcription.ts, extensions/openai/realtime-transcription-provider.ts)
  • vincentkoc: Recent commits touched websocket-session error handling and OpenAI realtime transcription payload handling near the affected provider/session boundary. (role: recent area contributor; confidence: medium; commits: f2a83a7a7125, 278e3eabf29d, 0d604f160d06; files: src/realtime-transcription/websocket-session.ts, extensions/openai/realtime-transcription-provider.ts)
  • sunlit-deng: This contributor authored the merged sibling realtime voice payload-cap PR and authored the current focused transcription cap branch. (role: adjacent hardening contributor; confidence: medium; commits: 2525ea41fd09, a1282d62c239; files: extensions/openai/realtime-voice-provider.ts, extensions/openai/realtime-voice-provider.test.ts, src/realtime-transcription/websocket-session.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.
Review history (4 earlier review cycles)
  • reviewed 2026-07-09T06:03:42.472Z sha cd25a4b :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T06:09:57.104Z sha cd25a4b :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T06:16:54.362Z sha cd25a4b :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T10:17:38.630Z sha 6296f52 :: needs maintainer review before merge. :: none

@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: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 9, 2026
@steipete steipete self-assigned this Jul 9, 2026
@steipete
steipete requested a review from a team as a code owner July 9, 2026 10:09
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: discord Channel integration: discord channel: line Channel integration: line channel: matrix Channel integration: matrix channel: msteams Channel integration: msteams channel: slack Channel integration: slack channel: telegram Channel integration: telegram channel: zalouser Channel integration: zalouser app: android App: android app: ios App: ios app: macos App: macos app: web-ui App: web-ui gateway Gateway runtime extensions: memory-core Extension: memory-core cli CLI command changes scripts Repository scripts commands Command implementations docker Docker and sandbox tooling agents Agent runtime and tooling extensions: openai channel: qqbot labels Jul 9, 2026
@openclaw-barnacle openclaw-barnacle Bot removed channel: line Channel integration: line channel: matrix Channel integration: matrix channel: msteams Channel integration: msteams channel: slack Channel integration: slack channel: telegram Channel integration: telegram channel: zalouser Channel integration: zalouser app: android App: android app: ios App: ios app: macos App: macos app: web-ui App: web-ui gateway Gateway runtime extensions: memory-core Extension: memory-core cli CLI command changes scripts Repository scripts commands Command implementations docker Docker and sandbox tooling agents Agent runtime and tooling extensions: openai channel: qqbot extensions: stepfun extensions: qa-lab extensions: codex plugin: google-meet extensions: tts-local-cli extensions: diagnostics-prometheus extensions: oc-path labels Jul 9, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Land-ready proof for 675b834a99b3a471bddeb3db9b88779f102041d3:

  • Rebuilt the stale fork history into a clean two-commit stack on current main; GitHub now reports exactly 2 files and +69/−1. Contributor authorship is preserved.
  • Deep review covered the complete shared session, reconnect path, all five bundled callers (OpenAI, xAI, Deepgram, ElevenLabs, Mistral), the realtime-voice sibling cap, SDK exposure, tests, and [email protected] client/receiver/permessage-deflate source.
  • Maintainer refinement uses the dependency's exact “message” terminology, shortens the invariant comment, and asserts WS_ERR_UNSUPPORTED_MESSAGE_LENGTH as well as the overflow text.
  • Dependency behavior: maxPayload bounds assembled and decompressed messages before normal message delivery; overflow emits the stable error code and closes with 1009. The 16 MiB cap matches the shipped realtime-voice path; a real 2 MiB transcript message still reaches the provider parser.
  • Current provider contracts use incremental or segmented transcript events; none documents a legitimate response-message requirement near 16 MiB. Residual compatibility risk is therefore low, though the providers do not publish explicit maximum response sizes.
  • Sanitized AWS Crabbox proof (public network, no Tailscale, no instance profile, no secret hydration): provider aws, lease cbx_e2a85b9b6711, run run_0351ff27c430; pnpm test src/realtime-transcription/websocket-session.test.ts — 12/12 passed on this exact head. Lease stopped after proof.
  • Fresh exact-tree autoreview: no findings; patch correct (0.97 confidence). Repo-native review artifacts reinitialized and validated at the final PR head.
  • Exact-SHA hosted CI release gate: run 29011509522 — success (113 jobs, zero failures).
  • Repo-native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 102443 passed; changelog not required for this changed-file set.

Known proof gaps: no live external transcription-provider call. That is intentional: this untrusted contributor head was kept secretless, while real loopback WebSocket boundary tests exercise the changed transport behavior directly and deterministically.

@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 9, 2026
Open
@steipete steipete mentioned this pull request Jul 9, 2026
Merged
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

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary 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

@sunlit-deng @steipete