RI-8321: Enforce x-window-id auth on WebSocket connections#6225
Conversation
The SessionMetadataAdapter was set as the web socket adapter unconditionally after WindowsAuthAdapter, and NestJS keeps only the last adapter — so the x-window-id handshake check never ran and every socket was granted the default session. Unauthenticated cross-origin clients could reach the monitor, pub-sub and bulk-actions namespaces. WindowsAuthAdapter now extends SessionMetadataAdapter so an authorized socket gets both the window-id gate and the session metadata, and it is the only adapter wired for the desktop build. Web/dev builds keep the metadata-only adapter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Disconnect unauthorized sockets
When an Electron production client supplies a missing or invalid x-window-id, this returns without closing the already-established Socket.IO connection; it merely skips registration of @SubscribeMessage handlers. The unauthenticated socket can still join the root namespace and receive broadcasts such as FeatureGateway and NotificationGateway, both of which use wss.of('/').emit(...). Explicitly disconnect or reject the socket here so the authorization check actually gates the WebSocket connection and its server-pushed data.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Code Coverage - Backend unit tests
Test suite run success3688 tests passing in 322 suites. Report generated by 🧪jest coverage report action from b87342f |
Code Coverage - Integration Tests
|
Returning early only skipped message-handler registration; the socket
stayed connected to the root namespace and still received broadcasts
such as FeatureGateway and NotificationGateway (wss.of('/').emit(...)).
Disconnect the socket when the window id is not authorized so the auth
check gates the connection, not just message sending.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
2 participants
What
The local WebSocket layer did not enforce the
x-window-idauthorizationthat the HTTP API requires.
SessionMetadataAdapterwas registered as theWebSocket adapter unconditionally after
WindowsAuthAdapter, and NestJS keepsonly the last adapter set — so the auth gate never ran and every socket was
granted the default session. Unauthenticated cross-origin clients could reach
the
monitor,pub-sub, andbulk-actionsnamespaces, observe live commands,and trigger destructive bulk actions against saved databases.
WindowsAuthAdapternow extendsSessionMetadataAdapter, so an authorizedsocket gets both the window-id check and its session metadata, and it is the
only adapter wired for the desktop build. Web/dev builds keep the
metadata-only adapter, matching the Electron-only scope of the HTTP
WindowAuthMiddleware.Note
High Risk
Changes WebSocket authentication and connection teardown for the desktop build, closing a path where unauthenticated clients could use monitor/pub-sub/bulk-actions namespaces.
Overview
Fixes a WebSocket auth bypass where registering
SessionMetadataAdapterafterWindowsAuthAdapterin bootstrap meant only the metadata adapter ran, so every socket got default session without anx-window-idcheck.For Electron builds, bootstrap now registers only
WindowsAuthAdapter(with an inline comment). Web/dev builds useSessionMetadataAdapteralone, matching HTTP window-auth scope.WindowsAuthAdapterextendsSessionMetadataAdapterand callssuper.bindMessageHandlersafter a successful window-id check, so authorized sockets still get session metadata and user-room join. Unauthorized sockets aredisconnect(true)so they cannot keep receiving namespace broadcasts.Adds unit tests for authorized vs unauthorized
bindMessageHandlersbehavior.Reviewed by Cursor Bugbot for commit b87342f. Bugbot is set up for automated code reviews on this repo. Configure here.