feat(channels): add WebSocket channel for local web-based chat#641
feat(channels): add WebSocket channel for local web-based chat#641likeaturtle wants to merge 10 commits intosipeed:mainfrom
Conversation
nikolasdehor
left a comment
There was a problem hiding this comment.
Nice feature for local LAN testing and embedded devices. The chat UI is polished with responsive design, theme toggle, and markdown rendering. Several issues to address:
Critical
-
Concurrent WebSocket writes are not safe:
Send()callsconn.WriteMessagefrom the manager dispatch goroutine, whilehandleClientcallsconn.WriteControl(PingMessage)from a separate goroutine. The gorilla/websocket documentation explicitly states: "Connections support one concurrent reader and one concurrent writer." You need a write mutex per connection, or use a per-client write channel. This will cause panics under load. -
No authentication/authorization on the HTTP endpoint: The WebSocket server listens on 0.0.0.0:8080 by default with no authentication. Anyone on the LAN can open the chat page and interact with the AI agent. The
allow_fromlist checksr.RemoteAddrwhich includes the port (e.g.192.168.1.5:54321), so it will never match a configured IP like192.168.1.5. The IsAllowed check is effectively broken. -
External CDN dependencies in embedded HTML: The chat.html loads Google Fonts and FontAwesome from CDNs. This breaks the "zero-dependency deployment" claim and will not work on air-gapped LANs (which is a common use case for embedded devices like NanoKVM). Either embed these assets or use system fonts.
Security
- XSS via markdown rendering: The chat HTML renders markdown with
innerHTML. If the LLM response contains HTML (which it can), it will be rendered directly. The markdown rendering function should sanitize HTML or use textContent for non-markdown parts. Looking at therenderMarkdownfunction in the HTML, it does replace<with<inside code blocks, but not in regular text.
Architecture
-
Does not extend BaseChannel: Unlike every other channel, this one implements the Channel interface from scratch instead of embedding BaseChannel. This means it misses any future additions to BaseChannel (like the group trigger logic, media store, or sender identity from PR #662). Consider embedding BaseChannel.
-
Port conflict with health server: The default WebSocket port is 8080, but the health server uses the gateway port (default 18790). If a user sets the gateway port to 8080, these will conflict silently. Add a check or at least document the constraint.
-
1268 lines of HTML embedded in Go binary: The chat.html is large. Consider putting it in a separate embedded filesystem with
//go:embedon a directory instead of a single file, so it can be maintained more easily.
Minor
-
The
min()helper function is unnecessary in Go 1.21+ which hasminas a builtin. -
The
registry_test.gochanges (interface{}toany) are unrelated cleanup and should be in a separate PR. -
The
cmd_status.gochannel listing is another instance of the growing switch/if-chain pattern. Same maintenance concern as noted in PR #646.
Please address items 1 (concurrent writes), 2 (broken allow_from), and 4 (XSS) before merging.
|
Hi @nikolasdehor , thanks for your review! |
…) to improve cross-browser compatibility.
…bustness of IP verification, and added front-end and back-end token validation
…ys processed with sanitizeHtml() to avoid direct use of innerHTML.
…ed Google Fonts with the system's native font stack, and replaced FontAwesome loaded via CDN with built-in files.
1842d11 to
a50bb97
Compare
|
Hello, we are currently working on a channel refactoring plan, which includes a plan for a WebSocket server channel. In PR #662, I have also preliminarily implemented a WebSocket channel. |
|
Hi @alexhoshina. PR #662 is an important and massive project. If this work can be split into modular parts, I would be more than happy to contribute. |
|
Hi! @likeaturtle I've opened a new refactor branch. You can check the WebSocket server I implemented in the refactor branch. I'm really looking forward to you improving the WebSocket channel-related content. |
|
@alexhoshina Great! I will check your branch later. If your branch can be merged, I'll be able to implement the front-end UI capabilities in other projects using this WebSocket protocol. By the way, I noticed that it has been merged into the refactor/channel-system branch. Do you have a timeline for when it will be merged into the main branch? |
|
Hello! @likeaturtle Sorry for the late reply! The refactoring branch is expected to be merged on February 28th Beijing time, but the specific time is unknown. |
|
Thank you for all your efforts!
…---- 回复的原邮件 ----
| 发件人 | ***@***.***> |
| 日期 | 2026年02月26日 20:49 |
| 收件人 | ***@***.***> |
| 抄送至 | ***@***.***>***@***.***> |
| 主题 | Re: [sipeed/picoclaw] feat(channels): add WebSocket channel for local web-based chat (PR #641) |
alexhoshina left a comment (sipeed/picoclaw#641)
Hello! @likeaturtle Sorry for the late reply! The refactoring branch is expected to be merged on February 28th Beijing time, but the specific time is unknown.
I will close this PR first. Contributions related to webui can be submitted to #806.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Add WebSocket channel implementation to support local LAN web chat interface with mobile-optimized responsive design.
Features:
Add WebSocket channel implementation in pkg/channels/websocket/
Integration with channel manager
Mobile-first responsive design
Configuration
Technical Details:
📝 Description
🗣️ Type of Change
🤖 AI Code Generation
📚 Technical Context (Skip for Docs)
🧪 Test Environment
Hardware: [NanoKVM Cube]
OS: [NAME=Buildroot, VERSION=-g3649fe90d, ID=buildroot, VERSION_ID=2023.11.2, PRETTY_NAME="Buildroot 2023.11.2"]
Model/Provider: [Volcengine]
Channels: [Discord, Telegram, Feishu, DingTalk, WebSocket]
☑️ Checklist