Gateway: Implemented WebSocket interface referencing OpenClaw#751
Gateway: Implemented WebSocket interface referencing OpenClaw#751seanly wants to merge 2 commits intosipeed:mainfrom
Conversation
- Add --help/-h: print gatewayHelp() and exit before loading config - Default enableWebSocket=true (WebSocket gateway + /health, /ready) - Add --no-websocket for health-only mode - Remove -e; keep --enable-websocket as no-op for compatibility - Add gatewayHelp() with usage, options, examples Co-authored-by: Cursor <cursoragent@cursor.com>
|
Hey, @seanly I think this feature should be added as a channel function~. We have already opened the refactor branch, which includes an implementation of a WebSocket server. Perhaps you could make improvements based on this. Looking forward to your modifications and suggestions! |
nikolasdehor
left a comment
There was a problem hiding this comment.
This is a significant feature (WebSocket gateway). The architecture is reasonable — JSON-framed req/res/event protocol over WebSocket with session management. However, there are several issues that should be addressed before merging:
Security Issues:
-
CheckOriginallows all origins:CheckOrigin: func(r *http.Request) bool { return true }is a textbook WebSocket CSRF vulnerability. Any website the user visits can connect to the local gateway and interact with the agent. This should at minimum check Origin against allowed origins, or require token auth on the upgrade request itself (not just in a post-upgradeconnectframe). -
Auth happens after connection is established: The
connectmethod validates token/password, but by then the WebSocket is already open. Beforeconnectsucceeds, the client can still send frames — the only check isframe.Type != "req"which skips non-request frames, but request frames for other methods (likechat.send) are processed without checking ifconnectwas called first. There is no per-connection auth state tracking. -
Empty token AND password means "deny all": If both
cfg.Tokenandcfg.Passwordare empty strings (the default!),handleConnectreturns"UNAUTHORIZED", "Missing gateway auth". This means the WebSocket gateway is unusable by default until the user configures auth. This is arguably safe but the user experience will be confusing — the gateway starts, says "WebSocket Gateway available", but nobody can connect. Consider either requiring auth config to enable WS, or allowing unauthenticated local access when bound to 127.0.0.1.
Correctness Issues:
-
SubscribeOutboundis a blocking poll:consumeOutboundcallss.bus.SubscribeOutbound(ctx)in a loop. If this is a channel-based subscription, only one consumer gets each message. If other channels (Telegram, Discord) also subscribe, the web gateway would steal messages from them. Need to verify this is a fan-out subscription, not a single-consumer queue. -
Session key displayed without
agent:prefix can collide: The display key stripsagent:main:, but if two agents have sessions with the same suffix, the display keys collide. TheresolveSessionKeyround-trip may resolve to the wrong agent. -
No connection cleanup on close: When
serveWebSocketexits, subscriptions are cleaned up, but in-flightchat.sendmessages (already published to the bus) will generate outbound events that go to dead connections. The goroutine inconsumeOutboundwill callwriteJSONLockedon a closed connection — this should be handled (checkconnis still alive).
Minor:
-
Missing newline at end of
server.go: The file does not end with a newline. -
SHA1 for message IDs: Using SHA1 in
msgIDis fine for non-security purposes but worth noting it is not a security hash — it is just a stable ID generator.
Good test coverage (395 lines), which is appreciated for a feature of this scope.
📝 Description
🗣️ Type of Change
🤖 AI Code Generation
🔗 Related Issue
📚 Technical Context (Skip for Docs)
🧪 Test Environment
📸 Evidence (Optional)
Click to view Logs/Screenshots
☑️ Checklist