Skip to content

fix: node WebSocket connection and UI connection string#382

Merged
penso merged 5 commits intomoltis-org:mainfrom
lijunle:fix/node-ws-and-ui
Mar 10, 2026
Merged

fix: node WebSocket connection and UI connection string#382
penso merged 5 commits intomoltis-org:mainfrom
lijunle:fix/node-ws-and-ui

Conversation

@lijunle
Copy link
Copy Markdown
Contributor

@lijunle lijunle commented Mar 10, 2026

Fixes #380 and #381

Changes

1. Add /ws route for node connections (#381)

Node client (runner.rs) connects to /ws, but gateway only registers /ws/chat. Auth middleware intercepts the request and returns 303 redirect to /login — the WebSocket upgrade never happens.

The WebSocket handler (ws.rs lines 148–165) already supports device-token auth at the protocol level, but gets blocked at the HTTP middleware layer.

Fix: Register /ws route pointing to the same ws_upgrade_handler and add it to is_public_path(). Includes test verifying /ws bypasses auth.

2. Fix UI connection string (#380)

Nodes settings page (page-nodes.js) uses gon.get("port") (server bind port) + location.hostname to construct the WebSocket URL. Behind a reverse proxy, this produces unreachable URLs like wss://hostname:38309/ws.

Every other WebSocket page (ws-connect.js, page-settings.js, page-terminal.js) correctly uses location.host.

Also: the generated shell command said moltis node run but the CLI subcommand is moltis node add.

Fix: Use location.host (consistent with all other pages), fix command to node add, remove unused gon import.

3. Update docs

Add /ws to the public paths table in authentication.md (with note about device-token auth at protocol level) and to the rate-limiting table in security.md.

4. Add moltis node run subcommand (#381)

moltis node add saves config to node.json and generates a service file (launchd plist / systemd unit) that invokes moltis node run. But the run subcommand did not exist — the generated service file would fail.

Fix: Add NodeAction::Run that reads gateway URL and credentials from node.json and starts the node connection. Also simplify the generated plist/systemd files to use node run instead of embedding all connection args inline.

lijunle and others added 2 commits March 10, 2026 01:56
…ctions (moltis-org#381)

Add /ws as a public WebSocket endpoint so remote nodes can connect
using device-token authentication at the WebSocket protocol layer.

- Register /ws route in both build_gateway_base variants
- Add /ws to is_public_path() in auth_middleware
- Add test verifying /ws bypasses auth middleware

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…oltis-org#380)

- Use location.host instead of gon.get('port') + location.hostname for
  the WebSocket URL, matching the pattern used by all other pages
  (ws-connect.js, page-settings.js, page-terminal.js)
- Change generated command from 'moltis node run' to 'moltis node add'
  to match the actual CLI subcommand
- Remove unused gon import
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 10, 2026

Greptile Summary

This PR fixes two independent bugs: a node WebSocket connectivity regression where runner.rs clients connecting to /ws were blocked by the HTTP auth middleware (which redirected them to /login instead of allowing WS-level device-token auth to run), and an incorrect WebSocket URL construction on the Nodes page that broke behind reverse proxies.

Key changes:

  • auth_middleware.rs / server.rs: Registers a /ws route bound to the existing ws_upgrade_handler and adds it to is_public_path(). Auth for this path now happens entirely at the WebSocket protocol layer inside handle_connection (device token, API key, password, or local-connection checks), consistent with how /ws/chat auth already works at the WS layer for header-authenticated sessions. /ws/chat is deliberately unchanged and remains protected at the HTTP middleware level.
  • page-nodes.js: Replaces gon.get("port") + location.hostname with location.host (consistent with ws-connect.js, page-settings.js, and page-terminal.js), fixes the CLI command string from moltis node run to moltis node add, and removes the now-unused gon import.
  • tests/auth_middleware.rs: Adds an integration test verifying /ws does not return a 303 redirect when auth is configured. The test assertion only checks the negative case (!= 303); adding a positive assert_eq!(status, 400) check would make the intent clearer and the test more robust against false passes.

Confidence Score: 4/5

  • Safe to merge — the security model is preserved since WS-level auth in handle_connection still validates all connections to /ws.
  • All three auth paths (device token, API key, password) are enforced inside handle_connection, so bypassing the HTTP middleware for /ws does not weaken security. The route registration is symmetric across both push-notifications feature variants. The JS fix is straightforward and consistent with other pages. Minor test quality gap (weak assertion, missing unit test) accounts for the score not being 5.
  • No files require special attention beyond the minor test improvements noted in comments.

Important Files Changed

Filename Overview
crates/gateway/src/auth_middleware.rs Adds /ws to is_public_path() so device-token-authenticated node connections bypass HTTP auth middleware and get auth at the WebSocket protocol layer instead. Logically correct; one minor unit test gap.
crates/gateway/src/server.rs Registers /ws route (same ws_upgrade_handler) in both push-notifications and not(push-notifications) feature variants. Change is symmetric and straightforward.
crates/gateway/tests/auth_middleware.rs Adds integration test verifying /ws does not return 303 redirect when auth is required. The assertion is weaker than ideal — it only checks the negative case (not 303) without also asserting the expected 400.
crates/web/src/assets/js/page-nodes.js Correctly switches from gon.get("port") + location.hostname to location.host for WebSocket URL construction (consistent with all other pages), fixes CLI subcommand name from node run to node add, and removes the now-unused gon import.

Sequence Diagram

sequenceDiagram
    participant NodeClient as Node Client
    participant Middleware as auth_gate Middleware
    participant Handler as ws_upgrade_handler
    participant Connection as handle_connection

    Note over NodeClient,Connection: Before fix
    NodeClient->>Middleware: GET /ws (WebSocket Upgrade)
    Middleware->>Middleware: is_public_path /ws = false
    Middleware-->>NodeClient: 303 Redirect to /login

    Note over NodeClient,Connection: After fix
    NodeClient->>Middleware: GET /ws (WebSocket Upgrade)
    Middleware->>Middleware: is_public_path /ws = true
    Middleware->>Handler: pass through
    Handler->>Handler: websocket_header_authenticated
    Handler->>Connection: header_authenticated = false
    Connection->>Connection: verify device_token in WS handshake
    Connection-->>NodeClient: HelloOk

    Note over NodeClient,Connection: /ws/chat (browser) unchanged
    participant Browser as Browser UI
    Browser->>Middleware: GET /ws/chat (session cookie)
    Middleware->>Middleware: check_auth = Allowed
    Middleware->>Handler: pass through
    Handler->>Connection: header_authenticated = true
    Connection-->>Browser: HelloOk
Loading

Comments Outside Diff (1)

  1. crates/gateway/src/auth_middleware.rs, line 326-349 (link)

    Missing unit test for is_public_path("/ws")

    The existing inline unit tests verify path-classification logic directly against is_public_path(), e.g. chat_ws_path_is_not_public asserts !is_public_path("/ws/chat"). There is no symmetric test asserting is_public_path("/ws") returns true for the newly-added path.

    A one-liner unit test here would document the intent and catch any accidental reversion of the is_public_path change independently of the integration test:

    #[cfg(feature = "web-ui")]
    #[test]
    fn node_ws_path_is_public() {
        assert!(is_public_path("/ws"));
    }

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Last reviewed commit: c72b67e

Comment thread crates/gateway/tests/auth_middleware.rs Outdated
Update authentication.md and security.md to document the new /ws
endpoint as a public path (device token auth at WebSocket protocol
level) and include it in the rate-limiting tables alongside /ws/chat.
Junle Li and others added 2 commits March 10, 2026 17:25
Add NodeAction::Run that reads connection config from node.json
(written by 'moltis node add') and launches the node host. This is
the command invoked by the generated launchd plist / systemd unit.

Also simplify generated service files to just 'moltis node run'
instead of passing all connection args inline — config comes from
node.json.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Fabien Penso <github@pen.so>
@penso penso merged commit fb221c2 into moltis-org:main Mar 10, 2026
@lijunle lijunle deleted the fix/node-ws-and-ui branch March 11, 2026 00:17
penso added a commit that referenced this pull request Mar 23, 2026
* fix(gateway): add /ws route and bypass auth middleware for node connections (#381)

Add /ws as a public WebSocket endpoint so remote nodes can connect
using device-token authentication at the WebSocket protocol layer.

- Register /ws route in both build_gateway_base variants
- Add /ws to is_public_path() in auth_middleware
- Add test verifying /ws bypasses auth middleware

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(web): correct node connection string and command in nodes page (#380)

- Use location.host instead of gon.get('port') + location.hostname for
  the WebSocket URL, matching the pattern used by all other pages
  (ws-connect.js, page-settings.js, page-terminal.js)
- Change generated command from 'moltis node run' to 'moltis node add'
  to match the actual CLI subcommand
- Remove unused gon import

* docs: add /ws to public paths and rate-limiting tables

Update authentication.md and security.md to document the new /ws
endpoint as a public path (device token auth at WebSocket protocol
level) and include it in the rate-limiting tables alongside /ws/chat.

* feat(node): add 'moltis node run' subcommand (#381)

Add NodeAction::Run that reads connection config from node.json
(written by 'moltis node add') and launches the node host. This is
the command invoked by the generated launchd plist / systemd unit.

Also simplify generated service files to just 'moltis node run'
instead of passing all connection args inline — config comes from
node.json.

* Update crates/gateway/tests/auth_middleware.rs

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Fabien Penso <github@pen.so>

---------

Signed-off-by: Fabien Penso <github@pen.so>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Junle Li <nickleli@gmail.com>
Co-authored-by: Fabien Penso <github@pen.so>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
jmikedupont2 pushed a commit to meta-introspector/moltis that referenced this pull request Mar 23, 2026
* fix(gateway): add /ws route and bypass auth middleware for node connections (moltis-org#381)

Add /ws as a public WebSocket endpoint so remote nodes can connect
using device-token authentication at the WebSocket protocol layer.

- Register /ws route in both build_gateway_base variants
- Add /ws to is_public_path() in auth_middleware
- Add test verifying /ws bypasses auth middleware

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(web): correct node connection string and command in nodes page (moltis-org#380)

- Use location.host instead of gon.get('port') + location.hostname for
  the WebSocket URL, matching the pattern used by all other pages
  (ws-connect.js, page-settings.js, page-terminal.js)
- Change generated command from 'moltis node run' to 'moltis node add'
  to match the actual CLI subcommand
- Remove unused gon import

* docs: add /ws to public paths and rate-limiting tables

Update authentication.md and security.md to document the new /ws
endpoint as a public path (device token auth at WebSocket protocol
level) and include it in the rate-limiting tables alongside /ws/chat.

* feat(node): add 'moltis node run' subcommand (moltis-org#381)

Add NodeAction::Run that reads connection config from node.json
(written by 'moltis node add') and launches the node host. This is
the command invoked by the generated launchd plist / systemd unit.

Also simplify generated service files to just 'moltis node run'
instead of passing all connection args inline — config comes from
node.json.

* Update crates/gateway/tests/auth_middleware.rs

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Fabien Penso <github@pen.so>

---------

Signed-off-by: Fabien Penso <github@pen.so>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Junle Li <nickleli@gmail.com>
Co-authored-by: Fabien Penso <github@pen.so>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node settings page: WebSocket connection string uses internal bind port instead of location.host

2 participants