fix(cli): honor --insecure for dashboard WebSocket connections#17887
Closed
ycros wants to merge 1 commit into
Closed
fix(cli): honor --insecure for dashboard WebSocket connections#17887ycros wants to merge 1 commit into
ycros wants to merge 1 commit into
Conversation
Collaborator
|
Related to #17440 — both fix WebSocket rejection when --insecure is used on non-loopback binds. This PR adds additional sidecar URL normalization and tests. |
Collaborator
|
Related to #17440 — both fix WebSocket rejection when --insecure is used on non-loopback binds. |
Author
|
yup, my agent was over-eager and just created the PR before I had done my own review - I wasn't aware of the other PR, I don't think it existed when I did the fix a day or so ago |
Dashboard WebSocket endpoints unconditionally rejected non-loopback peers even when the operator explicitly started the server with --insecure on a public bind. This broke reverse-proxied and Tailscale access, since HTTP honored --insecure but WebSockets did not. Changes: - _is_accepted_ws_client(): new helper that respects app.state.allow_public - _build_sidecar_url(): normalize 0.0.0.0 / :: to loopback for the PTY sidecar URL so it remains reachable - start_server(): set app.state.allow_public when --insecure + non-loopback - Update docstrings to reflect the new behavior
1b3b6de to
8aaf428
Compare
1 task
Author
|
I guess this was fixed by some other commit. |
19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the dashboard is started with
--insecureon a non-loopback bind (e.g.0.0.0.0), HTTP requests from remote clients are accepted as expected, but WebSocket connections are unconditionally rejected with a 4403 close code. This breaks reverse-proxied or Tailscale-based access where the dashboard needs to be reachable from non-loopback addresses.What changed
_is_accepted_ws_client(ws): new helper that checksapp.state.allow_publicbefore applying the loopback defense-in-depth check. When the operator explicitly passed--insecureon a non-loopback bind, remote WebSocket peers are now accepted._build_sidecar_url(): normalize0.0.0.0→127.0.0.1and::→::1when building the PTY sidecar URL, so the spawned chat process can still reach the dashboard's pub endpoint even when bound to all interfaces.start_server(): setapp.state.allow_publicwhen--insecureis combined with a non-loopback host.tests/hermes_cli/test_web_server_ws_security.pycovering:allow_public=Falseallow_public=True0.0.0.0/::normalization in_build_sidecar_url()TestClientSecurity posture
Default behavior is unchanged: loopback-only binds still reject non-loopback WebSocket clients. The only change is that
--insecure(already an explicit operator opt-in for HTTP) now consistently applies to WebSockets as well.How to test
Run the new tests:
Platforms tested
Use-case: this was originally needed to expose the dashboard over Tailscale to a home LAN, and to make it work behind a Caddy/nginx reverse proxy with basic auth.