feat(transport): websocket transport support#290
Conversation
commit: |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
👁️ Cyclops Security Review🧭 Auditing · mode=
⚙️ Controls
📜 3 events🔍 |
|
left some comments on overall design -- especially given sensitivity here for security flows kicked off a cyclops run too can you attach a diagram / flow of the state machine? is hard to understand from code alone |
|
@brendanjryan so I'm thinking of it as implementation-first, not protocol-final:
In other words, it's not “this is now the protocol”. It’s more “this is a concrete state machine that works, and we can use it to decide what the right generalized protocol shape should be.” Added a verbose diagram below since I agree it’s hard to reason about from code alone. stateDiagram-v2
[*] --> Idle
Idle --> Probe402: client GET /ws/chat (HTTP)
Probe402 --> WsConnect: 402 + Payment challenge
Probe402 --> Failed: no challenge / invalid challenge
WsConnect --> AwaitAuthReceipt: open websocket\n send initial authorization frame
WsConnect --> Failed: ws open fails
AwaitAuthReceipt --> Streaming: server verifies open\n sends payment-receipt
AwaitAuthReceipt --> Failed: payment-error / socket close
Streaming --> AwaitVoucher: server sends payment-need-voucher
AwaitVoucher --> Streaming: client sends higher cumulative voucher\nserver verifies + sends payment-receipt
Streaming --> AwaitCloseReady: client requests close
AwaitVoucher --> AwaitCloseReady: client requests close while paused for coverage
AwaitCloseReady --> AwaitCloseReceipt: server stops stream\ncomputes final spent\nsends payment-close-ready\nclient signs final close credential
AwaitCloseReady --> Failed: payment-error / socket close
AwaitCloseReceipt --> Settled: server verifies close\nsettles onchain\nsends payment-receipt
AwaitCloseReceipt --> Failed: payment-error / socket close
Settled --> Closed: websocket closes
Closed --> [*]
Failed --> [*]
|
👁️ Cyclops Security Review🧭 Consolidating · mode=
Findings
🔄 Consolidation running · Thread ⚙️ Controls
📜 16 events🔍 |
src/tempo/client/SessionManager.ts
Outdated
| if (type === 'close') { | ||
| if (emittedClose) return | ||
| emittedClose = true | ||
| readyState = 3 |
There was a problem hiding this comment.
what are these readyStates?
There was a problem hiding this comment.
These are the standard websocket readyState ints. I’ll make them explicit so this isn’t relying on magic numbers.
| rawSocket.send(Ws.formatAuthorizationMessage(voucher)) | ||
| break | ||
| } | ||
| case 'payment-receipt': |
There was a problem hiding this comment.
this smells a little complex -- is there a more functional / robust way to represent this state machine? in general this seems pretty fragile and I worry that it would be easy to mess up
There was a problem hiding this comment.
Fair call. The current shape is definitely explicit/procedural. The goal here was to get a working end-to-end websocket transport for tempo.session(), not to claim we’ve landed the final abstraction. I do think we should make the active socket/session state more explicit though and I’m tightening that in this PR so the current state machine is less fragile.
tempoxyz-bot
left a comment
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review
This PR adds experimental WebSocket transport for Tempo payment sessions. The implementation correctly serializes concurrent frames and reuses the existing HTTP challenge/verification flow, but introduces 3 high-severity and 1 medium-severity security issue in the new WebSocket transport layer.
| # | Tier | Finding | File |
|---|---|---|---|
| 1 | 🚨 SECURITY | Application frames parsed as payment control — injection enables fund drain | SessionManager.ts:500, Ws.ts:161 |
| 2 | 🚨 SECURITY | Client trusts server-supplied channelId without binding — voucher retargeting |
SessionManager.ts:541 |
| 3 | 🚨 SECURITY | Unbounded promise queue — CPU exhaustion DoS | Ws.ts:253-262 |
| 4 | HTTP-fallback close uses stale spent — funds locked after WS disconnect |
SessionManager.ts:600 |
Reviewer Callouts
- ⚡ WebSocket Bootstrap Invariant:
Ws.ts:222-239starts the application stream after any non-closeauthorization, includingtopUp, even thoughtopUpis documented as management-only inSession.ts:262-274. This deserves a manual pass. - ⚡ WS Stream Concurrency: The
actionpromise queue correctly serializes concurrent frames forchannel.highestVoucherAmountmutations, but the lack of backpressure and closed-state guards exposes the server to queue-flooding. - ⚡ WebSocket State Syncing: The decision not to send
payment-receiptduring streaming causes the client'sspentvariable to fall out of sync with the server. Ensure the client can reliably fall back tochannel.cumulativeAmountfor close transactions.
o-az
force-pushed
the
o-az/ws-transport-support
branch
from
53918e4 to
cfa18a8
Compare
👁️ Cyclops Security Review🧭 Auditing · mode=
Findings
⚙️ Controls
📜 24 events🔍 |
o-az
force-pushed
the
o-az/ws-transport-support
branch
from
67aa317 to
6da52c6
Compare
Addresses findings from Cyclops bot review, Amp security audit, and manual analysis: - bind close receipts to signed close amount (prevents open receipt replay) - commit spend only when streaming chunks are delivered (prevents overcharging) - enforce local maxDeposit on streamed voucher requests - track delivered chunks for fallback close (prevents overpayment on disconnect) - validate parseMessage data fields structurally instead of truthy checks - add amount validation to Ws.serve to prevent compose underbilling - buffer managed socket messages until first listener is installed - document synthetic POST behavior on Ws.serve route - use app-defined WS close code for browser compatibility - use isows for example client WebSocket
o-az
force-pushed
the
o-az/ws-transport-support
branch
from
0dc91e1 to
121425d
Compare
The Cyclops fix changed the non-WS fallback close from spent.toString() to max(cumulative, spent), which signs for the full voucher authorization instead of actual consumption. SSE keeps spent in sync via inline receipts, so the original spent.toString() is correct.
brendanjryan
left a comment
brendanjryan
left a comment
There was a problem hiding this comment.
LGTM -- much cleaner. Thank you!!
| "editor.formatOnSave": true | ||
| "editor.formatOnSave": true, | ||
| "typescript.enablePromptUseWorkspaceTsdk": true, | ||
| "typescript.tsdk": "node_modules/typescript/lib" |
| } | ||
|
|
||
| const WebSocketReadyState = { | ||
| CONNECTING: 0, |
3 participants
Adds a working end-to-end websocket transport for
tempo.session().This keeps the initial 402 bootstrap over HTTP, then moves the rest of the session flow onto the websocket: initial auth, mid-stream voucher top-ups, final stop/close, and settlement receipt. The app layer only sees streamed content.
Also fixes a few issues that showed up once the flow was exercised end to end:
close()no longer signs from stale / unvalidated websocket close statespentstate after websocket disconnectincludes a runnable
examples/session/wsdemo:CleanShot.2026-04-02.at.15.25.31.mp4
Additional hardening from security review
wsDeliveredChunks × wsTickCost) to prevent overpaying on unexpected WS disconnect instead of signing for the full cumulative voucher authorizationparseMessagestructural validation — malformed frames with truthy non-objectdata(e.g.{"mpp":"payment-receipt","data":42}) are rejected at parse time instead of propagating and killing the socket with a misleading errorWs.serve()accepts an optionalamountparameter that validates the echoed challenge price, preventing a client from selecting a cheaper composed offer for the same streamsession.ws()buffers messages until the first listener is installed, eliminating thesetTimeout(fn, 0)timing dependencymaxDepositenforcement on voucher requests — both SSE and WS paths validate voucher amounts against the localmaxDepositlimitWs.serve()and therouteoption documenting that credentials are verified via synthetic POST with only theAuthorizationheaderisowsmigration — example client usesisowsinstead ofwsfor isomorphic WebSocket support