fix: add room membership checks. Broadcast errors to room only#1391
fix: add room membership checks. Broadcast errors to room only#1391jiexi wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 93319fb. Configure here.
| ); | ||
| callback?.('not authorized', undefined); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Reject auth too broad
Medium Severity
The new off-room guard allows any socket to complete handleChannelRejected when channelConfig.clients.wallet is set, without tying the request to the wallet. A non-member who knows the channel UUID can mark the channel rejected and broadcast to the room, not only a reconnecting wallet.
Reviewed by Cursor Bugbot for commit 93319fb. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1391 +/- ##
=======================================
Coverage 74.99% 74.99%
=======================================
Files 184 184
Lines 4519 4519
Branches 1108 1108
=======================================
Hits 3389 3389
Misses 1130 1130 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|





Explanation
pingandackhandlers (socket-config.ts) now bail out with awarning if
socket.rooms.has(channelId)is false, mirroring the existingmessageandleave_channelguards. Non-member sockets can no longer reachhandlePing/handleAck.rejectedhandler (handleChannelRejected.ts) gets a participantcheck. A naive room-membership guard would break the legitimate flow because
rejectChannel.tsreconnects the socket before emittingrejectedand thereconnected socket is no longer in the channel room. Instead we accept the
request only if either:
channel_config.clients.walletentry is recorded in Redis(the wallet had previously joined and is rejecting after a reconnect).
An invalid channel ID also short-circuits with
error_idnow.handleMessage.ts:178error broadcast is scoped to the channel room(
socket.broadcast.to(channelId).emit(...)), so an in-room client's errorno longer leaks the channel ID to every other connected socket.
References
Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1549
Checklist
Note
High Risk
Changes authorization on live Socket.IO channel operations and Redis side effects; mistakes could break legitimate wallet reconnect/reject flows or leave channels open to abuse.
Overview
Addresses HackerOne 3604630 by tightening who can act on a channel in
sdk-socket-server-next.Socket layer:
ackandping(and existingmessagewiring covered by tests) now bail out unlesssocket.roomsincludes the channel ID, so unrelated connections cannot ack/delete or pull queued messages for arbitrary UUIDs.handleChannelRejected: ValidateschannelIdas a UUID, then requires the socket to be in the room or a known wallet participant (post-reconnect reject) before writing Redis or broadcasting; other callers getnot authorized/error_idwith nosetex.handleMessage: On failure, errors are emitted viabroadcast.to(channelId)instead of a global broadcast, avoiding leakage of active channel IDs.Adds package Jest config/setup (env for
analytics-api, logger init) and regression tests; e2e import updated toanalytics-api.Reviewed by Cursor Bugbot for commit 93319fb. Bugbot is set up for automated code reviews on this repo. Configure here.