VM console fails to connect when hero_compute is embedded in hero_os (works standalone) #102

Closed
opened 2026-04-22 13:40:57 +00:00 by rawan · 4 comments
Member

Summary

The VM console in the Hero Compute dashboard fails to establish a WebSocket connection when the dashboard is accessed embedded inside hero_os (via hero_router). Opening the same dashboard as a standalone hero_compute admin page, the console connects and works normally.

Observed symptom in the browser console:

--- Reconnecting (1/10)... ---
...
--- Reconnecting (10/10)... ---
--- Connection lost. Click to reconnect. ---

console disconnected

Steps to reproduce

  1. Deploy a VM through hero_compute and wait until it is running.
  2. Open the standalone hero_compute admin UI and click the console button → terminal connects successfully.
  3. Open the same admin UI from the hero_os desktop (served through hero_router) and click the console button → 10 reconnect attempts, then Connection lost.

Root cause

The console WebSocket handler (console_ws_handler in crates/hero_compute_ui/src/server.rs) performs an Origin/Host check:

let origin_host = origin
    .trim_start_matches("http://")
    .trim_start_matches("https://");
if !origin_host.starts_with(host) {
    return (StatusCode::FORBIDDEN, "WebSocket origin mismatch").into_response();
}

When hero_compute runs embedded, hero_router proxies the WebSocket upgrade over a Unix socket and rewrites the Host header to localhost before forwarding (see proxy_ws_tunnel in hero_router/crates/hero_router/src/server/routes.rs, which hard-codes .header("Host", "localhost")). The Origin header, however, is forwarded unchanged as the real browser origin (e.g. http://<os-host>:<port>).

Result: the backend compares origin_host = "<os-host>:<port>" against host = "localhost", the starts_with check fails, and every WebSocket upgrade is rejected with 403 Forbidden. The client then burns through its 10 reconnect attempts and gives up.

In standalone mode the backend receives the real Host header, so Origin and Host match and the check passes — which is why the bug only manifests when embedded.

Proposed fix

Two coordinated changes:

  1. hero_router — when tunneling a WebSocket upgrade, also inject an X-Forwarded-Host header containing the original Host value (alongside the existing X-Forwarded-Prefix / X-Hero-Context injections in ws_proxy_inner).
  2. hero_compute — in console_ws_handler, prefer X-Forwarded-Host over Host when present, so the Origin check compares against the external hostname the browser actually saw.

This keeps the security intent of the origin check (reject cross-origin WS upgrades) while making it correct for reverse-proxied traffic.

## Summary The VM console in the Hero Compute dashboard fails to establish a WebSocket connection when the dashboard is accessed **embedded inside hero_os** (via hero_router). Opening the same dashboard as a **standalone** hero_compute admin page, the console connects and works normally. Observed symptom in the browser console: ``` --- Reconnecting (1/10)... --- ... --- Reconnecting (10/10)... --- --- Connection lost. Click to reconnect. --- ``` ![console disconnected](https://forge.ourworld.tf/attachments/29d949ba-4300-4c47-9b64-aa721d92b248) ## Steps to reproduce 1. Deploy a VM through `hero_compute` and wait until it is `running`. 2. Open the **standalone** hero_compute admin UI and click the console button → terminal connects successfully. 3. Open the **same** admin UI from the hero_os desktop (served through hero_router) and click the console button → 10 reconnect attempts, then `Connection lost`. ## Root cause The console WebSocket handler (`console_ws_handler` in `crates/hero_compute_ui/src/server.rs`) performs an Origin/Host check: ```rust let origin_host = origin .trim_start_matches("http://") .trim_start_matches("https://"); if !origin_host.starts_with(host) { return (StatusCode::FORBIDDEN, "WebSocket origin mismatch").into_response(); } ``` When hero_compute runs embedded, hero_router proxies the WebSocket upgrade over a Unix socket and rewrites the `Host` header to `localhost` before forwarding (see `proxy_ws_tunnel` in `hero_router/crates/hero_router/src/server/routes.rs`, which hard-codes `.header("Host", "localhost")`). The `Origin` header, however, is forwarded unchanged as the real browser origin (e.g. `http://<os-host>:<port>`). Result: the backend compares `origin_host = "<os-host>:<port>"` against `host = "localhost"`, the `starts_with` check fails, and every WebSocket upgrade is rejected with `403 Forbidden`. The client then burns through its 10 reconnect attempts and gives up. In standalone mode the backend receives the real `Host` header, so Origin and Host match and the check passes — which is why the bug only manifests when embedded. ## Proposed fix Two coordinated changes: 1. **hero_router** — when tunneling a WebSocket upgrade, also inject an `X-Forwarded-Host` header containing the original `Host` value (alongside the existing `X-Forwarded-Prefix` / `X-Hero-Context` injections in `ws_proxy_inner`). 2. **hero_compute** — in `console_ws_handler`, prefer `X-Forwarded-Host` over `Host` when present, so the Origin check compares against the external hostname the browser actually saw. This keeps the security intent of the origin check (reject cross-origin WS upgrades) while making it correct for reverse-proxied traffic.
Author
Member

Companion router-side issue: lhumina_code/hero_router#47 — tracks the X-Forwarded-Host injection half of the fix.

Companion router-side issue: [lhumina_code/hero_router#47](https://forge.ourworld.tf/lhumina_code/hero_router/issues/47) — tracks the `X-Forwarded-Host` injection half of the fix.
Author
Member

Implementation Specification for Issue #102

Objective

Fix the VM console WebSocket handler (console_ws_handler in crates/hero_compute_ui/src/server.rs) so that the Origin/Host check remains correct when hero_compute runs embedded behind hero_router. Router rewrites Host to localhost but forwards the real browser Origin unchanged; the fix must prefer X-Forwarded-Host (injected by hero_router per issue hero_router#47) when present, falling back to Host when running standalone.

Requirements

  1. Compare Origin against the externally-visible hostname rather than the intra-proxy Host.
  2. Use the first non-empty value of X-Forwarded-Host if present, else fall back to Host.
  3. Continue to reject WebSocket upgrades where Origin does not match the chosen target host.
  4. Standalone behaviour (no X-Forwarded-Host) must be identical to today so standalone browsers continue to connect.
  5. No behavioural change for non-WebSocket requests on the same route.
  6. The Origin check remains defence-in-depth: any client that can set X-Forwarded-Host can equally set Origin, so in production hero_router is the trust boundary injecting/sanitising that header.
  7. Robust to existing edge cases (http/https prefix, host with port) and new ones (forwarded host, IPv6 literals, comma-separated forwarded chains).

Files to Modify/Create

  • crates/hero_compute_ui/src/server.rs — MODIFY. Extract a pure helper for the Origin/Host comparison and a helper for resolving the effective host, then switch console_ws_handler to use both. Extend the existing #[cfg(test)] module with tests for both helpers.

No other files need to be touched. There is only one WebSocket upgrade handler in this crate.

Step-by-step Implementation Plan

Step 1 — Extract origin_matches_host helper

File: crates/hero_compute_ui/src/server.rs
Dependencies: none

Add a private function near other small helpers:

fn origin_matches_host(origin: &str, host: &str) -> bool {
    if host.is_empty() { return false; }
    let o = origin
        .trim_start_matches("http://")
        .trim_start_matches("https://");
    let o = o.split('/').next().unwrap_or(o);
    o.eq_ignore_ascii_case(host)
}

Short doc comment explaining intent (CSRF defence-in-depth) and noting that the upstream proxy is the trust boundary for X-Forwarded-Host.

Strict equality closes the example.com.evil starts_with prefix bug; path-stripping keeps the common Origin: scheme://host[:port] case correct and defends against Origin: http://host/anything.

Step 2 — Extract effective_host helper

File: crates/hero_compute_ui/src/server.rs
Dependencies: none

Add a private function:

fn effective_host(headers: &HeaderMap) -> Option<String> {
    if let Some(v) = headers.get("x-forwarded-host").and_then(|v| v.to_str().ok()) {
        let first = v.split(',').next().unwrap_or("").trim();
        if !first.is_empty() {
            return Some(first.to_string());
        }
    }
    headers
        .get("host")
        .and_then(|v| v.to_str().ok())
        .filter(|s| !s.is_empty())
        .map(|s| s.to_string())
}

Step 3 — Update console_ws_handler

File: crates/hero_compute_ui/src/server.rs
Function: console_ws_handler
Dependencies: Steps 1 and 2

Replace the inline Origin/Host check with:

let origin = headers.get("origin").and_then(|v| v.to_str().ok());
let host = effective_host(&headers);
if let (Some(origin), Some(host)) = (origin, host.as_deref()) {
    if !origin_matches_host(origin, host) {
        return (StatusCode::FORBIDDEN, "WebSocket origin mismatch").into_response();
    }
}

Keep the same early-return location so control flow of the rest of the function is untouched. Preserve today's behaviour when either header is missing (do not reject).

Step 4 — Unit tests

File: crates/hero_compute_ui/src/server.rs (extend existing #[cfg(test)] mod tests)
Dependencies: Steps 1 and 2

origin_matches_host cases:

  • ("http://example.com", "example.com") → true
  • ("https://example.com", "example.com") → true
  • ("http://example.com:9001", "example.com:9001") → true
  • ("http://EXAMPLE.com", "example.com") → true (case-insensitive)
  • ("http://example.com", "evil.com") → false
  • ("http://example.com.evil", "example.com") → false (regression against prefix bug)
  • ("http://example.com/path", "example.com") → true
  • ("", "example.com") → false
  • ("http://example.com", "") → false
  • ("http://[::1]:9001", "[::1]:9001") → true
  • ("http://[::1]", "[::1]") → true

effective_host cases (construct a HeaderMap in each test):

  • x-forwarded-host: example.com, host: localhostSome("example.com")
  • x-forwarded-host absent, host: localhost:9001Some("localhost:9001")
  • x-forwarded-host: example.com, internal.lanSome("example.com") (first comma-entry, trimmed)
  • x-forwarded-host: empty, host: fooSome("foo")
  • neither header present → None

Step 5 — Build and test

Dependencies: Steps 1-4

From /home/rawan/codescalers/hero_compute:

cargo build -p hero_compute_ui
cargo test -p hero_compute_ui

Fix any compile errors or test failures.

Dependencies: Step 5

  • Standalone: start hero_compute_ui, open VM console, confirm it attaches.
  • Embedded (requires hero_router with the X-Forwarded-Host injection from hero_router#47): open the same console from hero_os desktop, confirm no 403 and terminal attaches.
  • Negative: upgrade with a mismatched Origin and confirm 403.

Acceptance Criteria

  • origin_matches_host helper exists, is private, has a doc comment, is unit-tested.
  • effective_host helper exists, is private, is unit-tested.
  • console_ws_handler uses both helpers and resolves the comparison host from X-Forwarded-Host first.
  • Standalone access (no X-Forwarded-Host) still accepts WS upgrades whose Origin matches Host.
  • Embedded access (hero_router injects X-Forwarded-Host, rewrites Host: localhost) accepts WS upgrades whose Origin matches the external host.
  • WS upgrade with genuinely mismatched Origin still returns 403.
  • cargo test -p hero_compute_ui passes including new tests.
  • cargo build -p hero_compute_ui succeeds with no new warnings.

Notes

  • Trust boundary: in production, hero_router injects X-Forwarded-Host at the edge; browsers cannot set it directly on same-origin WS upgrades. The Origin check is defence-in-depth, not a primary access control.
  • X-Forwarded-Host missing: standalone deployments — falls back to Host, behaviour unchanged.
  • Comma-separated chains: take the first (left-most) entry per common reverse-proxy convention.
  • Port in Origin but not in forwarded host: if the router forwards X-Forwarded-Host: example.com but the browser hit https://example.com:8443, the Origin carries the port and the comparison will fail. Router must forward host-with-port. This is the contract assumed by hero_router#47.
  • Empty Origin: retain current behaviour, do not reject (non-browser clients often omit it).
  • IPv6 literals: [::1]:9001 works byte-for-byte against the trimmed Origin.
  • starts_with → strict equality: minor behavioural tightening that closes a latent prefix-match bug. If reviewers prefer a minimal-diff fix, replace the equality in Step 1 with o.to_ascii_lowercase().starts_with(&host.to_ascii_lowercase()) and leave a TODO.
# Implementation Specification for Issue #102 ## Objective Fix the VM console WebSocket handler (`console_ws_handler` in `crates/hero_compute_ui/src/server.rs`) so that the Origin/Host check remains correct when hero_compute runs embedded behind hero_router. Router rewrites `Host` to `localhost` but forwards the real browser `Origin` unchanged; the fix must prefer `X-Forwarded-Host` (injected by hero_router per issue hero_router#47) when present, falling back to `Host` when running standalone. ## Requirements 1. Compare `Origin` against the externally-visible hostname rather than the intra-proxy `Host`. 2. Use the first non-empty value of `X-Forwarded-Host` if present, else fall back to `Host`. 3. Continue to reject WebSocket upgrades where `Origin` does not match the chosen target host. 4. Standalone behaviour (no `X-Forwarded-Host`) must be identical to today so standalone browsers continue to connect. 5. No behavioural change for non-WebSocket requests on the same route. 6. The Origin check remains defence-in-depth: any client that can set `X-Forwarded-Host` can equally set `Origin`, so in production hero_router is the trust boundary injecting/sanitising that header. 7. Robust to existing edge cases (http/https prefix, host with port) and new ones (forwarded host, IPv6 literals, comma-separated forwarded chains). ## Files to Modify/Create - `crates/hero_compute_ui/src/server.rs` — MODIFY. Extract a pure helper for the Origin/Host comparison and a helper for resolving the effective host, then switch `console_ws_handler` to use both. Extend the existing `#[cfg(test)]` module with tests for both helpers. No other files need to be touched. There is only one WebSocket upgrade handler in this crate. ## Step-by-step Implementation Plan ### Step 1 — Extract `origin_matches_host` helper File: `crates/hero_compute_ui/src/server.rs` Dependencies: none Add a private function near other small helpers: ```rust fn origin_matches_host(origin: &str, host: &str) -> bool { if host.is_empty() { return false; } let o = origin .trim_start_matches("http://") .trim_start_matches("https://"); let o = o.split('/').next().unwrap_or(o); o.eq_ignore_ascii_case(host) } ``` Short doc comment explaining intent (CSRF defence-in-depth) and noting that the upstream proxy is the trust boundary for `X-Forwarded-Host`. Strict equality closes the `example.com.evil` `starts_with` prefix bug; path-stripping keeps the common `Origin: scheme://host[:port]` case correct and defends against `Origin: http://host/anything`. ### Step 2 — Extract `effective_host` helper File: `crates/hero_compute_ui/src/server.rs` Dependencies: none Add a private function: ```rust fn effective_host(headers: &HeaderMap) -> Option<String> { if let Some(v) = headers.get("x-forwarded-host").and_then(|v| v.to_str().ok()) { let first = v.split(',').next().unwrap_or("").trim(); if !first.is_empty() { return Some(first.to_string()); } } headers .get("host") .and_then(|v| v.to_str().ok()) .filter(|s| !s.is_empty()) .map(|s| s.to_string()) } ``` ### Step 3 — Update `console_ws_handler` File: `crates/hero_compute_ui/src/server.rs` Function: `console_ws_handler` Dependencies: Steps 1 and 2 Replace the inline Origin/Host check with: ```rust let origin = headers.get("origin").and_then(|v| v.to_str().ok()); let host = effective_host(&headers); if let (Some(origin), Some(host)) = (origin, host.as_deref()) { if !origin_matches_host(origin, host) { return (StatusCode::FORBIDDEN, "WebSocket origin mismatch").into_response(); } } ``` Keep the same early-return location so control flow of the rest of the function is untouched. Preserve today's behaviour when either header is missing (do not reject). ### Step 4 — Unit tests File: `crates/hero_compute_ui/src/server.rs` (extend existing `#[cfg(test)] mod tests`) Dependencies: Steps 1 and 2 `origin_matches_host` cases: - `("http://example.com", "example.com")` → true - `("https://example.com", "example.com")` → true - `("http://example.com:9001", "example.com:9001")` → true - `("http://EXAMPLE.com", "example.com")` → true (case-insensitive) - `("http://example.com", "evil.com")` → false - `("http://example.com.evil", "example.com")` → false (regression against prefix bug) - `("http://example.com/path", "example.com")` → true - `("", "example.com")` → false - `("http://example.com", "")` → false - `("http://[::1]:9001", "[::1]:9001")` → true - `("http://[::1]", "[::1]")` → true `effective_host` cases (construct a `HeaderMap` in each test): - `x-forwarded-host: example.com`, `host: localhost` → `Some("example.com")` - `x-forwarded-host` absent, `host: localhost:9001` → `Some("localhost:9001")` - `x-forwarded-host: example.com, internal.lan` → `Some("example.com")` (first comma-entry, trimmed) - `x-forwarded-host:` empty, `host: foo` → `Some("foo")` - neither header present → `None` ### Step 5 — Build and test Dependencies: Steps 1-4 From `/home/rawan/codescalers/hero_compute`: ``` cargo build -p hero_compute_ui cargo test -p hero_compute_ui ``` Fix any compile errors or test failures. ### Step 6 — Manual verification (optional but recommended) Dependencies: Step 5 - Standalone: start hero_compute_ui, open VM console, confirm it attaches. - Embedded (requires hero_router with the X-Forwarded-Host injection from hero_router#47): open the same console from hero_os desktop, confirm no 403 and terminal attaches. - Negative: upgrade with a mismatched `Origin` and confirm 403. ## Acceptance Criteria - [ ] `origin_matches_host` helper exists, is private, has a doc comment, is unit-tested. - [ ] `effective_host` helper exists, is private, is unit-tested. - [ ] `console_ws_handler` uses both helpers and resolves the comparison host from `X-Forwarded-Host` first. - [ ] Standalone access (no `X-Forwarded-Host`) still accepts WS upgrades whose `Origin` matches `Host`. - [ ] Embedded access (hero_router injects `X-Forwarded-Host`, rewrites `Host: localhost`) accepts WS upgrades whose `Origin` matches the external host. - [ ] WS upgrade with genuinely mismatched `Origin` still returns 403. - [ ] `cargo test -p hero_compute_ui` passes including new tests. - [ ] `cargo build -p hero_compute_ui` succeeds with no new warnings. ## Notes - **Trust boundary:** in production, hero_router injects `X-Forwarded-Host` at the edge; browsers cannot set it directly on same-origin WS upgrades. The Origin check is defence-in-depth, not a primary access control. - **`X-Forwarded-Host` missing:** standalone deployments — falls back to `Host`, behaviour unchanged. - **Comma-separated chains:** take the first (left-most) entry per common reverse-proxy convention. - **Port in `Origin` but not in forwarded host:** if the router forwards `X-Forwarded-Host: example.com` but the browser hit `https://example.com:8443`, the Origin carries the port and the comparison will fail. Router must forward host-with-port. This is the contract assumed by hero_router#47. - **Empty `Origin`:** retain current behaviour, do not reject (non-browser clients often omit it). - **IPv6 literals:** `[::1]:9001` works byte-for-byte against the trimmed Origin. - **`starts_with` → strict equality:** minor behavioural tightening that closes a latent prefix-match bug. If reviewers prefer a minimal-diff fix, replace the equality in Step 1 with `o.to_ascii_lowercase().starts_with(&host.to_ascii_lowercase())` and leave a TODO.
Author
Member

Test Results

Command: cargo test -p hero_compute_ui
Branch: development_vm_console_forwarded_host

  • Total: 24
  • Passed: 24
  • Failed: 0
  • Ignored: 0

Build: PASS

## Test Results Command: `cargo test -p hero_compute_ui` Branch: `development_vm_console_forwarded_host` - Total: 24 - Passed: 24 - Failed: 0 - Ignored: 0 Build: PASS
Author
Member

Implementation Summary

Branch: development_vm_console_forwarded_host

Changes

  • crates/hero_compute_ui/src/server.rs
    • Added private helper origin_matches_host(origin, host) -> bool that strips http:// / https:// and any path suffix from Origin, then compares against host with case-insensitive equality. Strict equality closes a latent prefix-match bug in the previous starts_with check (e.g. example.com.evil no longer matches example.com).
    • Added private helper effective_host(&HeaderMap) -> Option<String> that prefers the first non-empty comma-separated entry of X-Forwarded-Host, falling back to Host.
    • console_ws_handler now uses both helpers for the Origin check. The ws.is_some() guard is preserved, so the check only runs on actual WebSocket upgrade requests (non-WS hits on this endpoint are unaffected).
    • Standalone behaviour is unchanged: without X-Forwarded-Host, the handler falls back to Host and the check passes exactly as before.
    • Embedded behaviour (hero_router injects X-Forwarded-Host via hero_router#47) now compares Origin against the external hostname, so the 403 on embedded console connections is resolved.

Test Results

  • Command: cargo test -p hero_compute_ui
  • Total: 24, Passed: 24, Failed: 0, Ignored: 0
  • Build: PASS, no new warnings
  • 16 new unit tests added: 11 for origin_matches_host (including http/https, port, IPv6 literal with and without port, case-insensitive, suffix-attack regression, empty origin/host, path-stripping) and 5 for effective_host (prefers forwarded host, falls back to host, picks first comma-separated entry, handles empty forwarded value, returns None when both headers absent).

Notes

  • No other WebSocket handlers exist in this crate — console_ws_handler is the only one, verified by grep -n "WebSocketUpgrade" crates/hero_compute_ui/src/.
  • Paired router-side change lives in hero_router#47 (injects X-Forwarded-Host on WS upgrades).
  • Requires both PRs to land together for the embedded VM console to work end-to-end; the hero_compute side alone preserves standalone behaviour.
## Implementation Summary Branch: `development_vm_console_forwarded_host` ### Changes - `crates/hero_compute_ui/src/server.rs` - Added private helper `origin_matches_host(origin, host) -> bool` that strips `http://` / `https://` and any path suffix from `Origin`, then compares against `host` with case-insensitive equality. Strict equality closes a latent prefix-match bug in the previous `starts_with` check (e.g. `example.com.evil` no longer matches `example.com`). - Added private helper `effective_host(&HeaderMap) -> Option<String>` that prefers the first non-empty comma-separated entry of `X-Forwarded-Host`, falling back to `Host`. - `console_ws_handler` now uses both helpers for the Origin check. The `ws.is_some()` guard is preserved, so the check only runs on actual WebSocket upgrade requests (non-WS hits on this endpoint are unaffected). - Standalone behaviour is unchanged: without `X-Forwarded-Host`, the handler falls back to `Host` and the check passes exactly as before. - Embedded behaviour (hero_router injects `X-Forwarded-Host` via hero_router#47) now compares `Origin` against the external hostname, so the 403 on embedded console connections is resolved. ### Test Results - Command: `cargo test -p hero_compute_ui` - Total: 24, Passed: 24, Failed: 0, Ignored: 0 - Build: PASS, no new warnings - 16 new unit tests added: 11 for `origin_matches_host` (including http/https, port, IPv6 literal with and without port, case-insensitive, suffix-attack regression, empty origin/host, path-stripping) and 5 for `effective_host` (prefers forwarded host, falls back to host, picks first comma-separated entry, handles empty forwarded value, returns None when both headers absent). ### Notes - No other WebSocket handlers exist in this crate — `console_ws_handler` is the only one, verified by `grep -n "WebSocketUpgrade" crates/hero_compute_ui/src/`. - Paired router-side change lives in hero_router#47 (injects `X-Forwarded-Host` on WS upgrades). - Requires both PRs to land together for the embedded VM console to work end-to-end; the hero_compute side alone preserves standalone behaviour.
rawan closed this issue 2026-04-23 09:30:58 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_compute#102
No description provided.