VM console fails to connect when hero_compute is embedded in hero_os (works standalone) #102
Labels
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
lhumina_code/hero_compute#102
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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:
Steps to reproduce
hero_computeand wait until it isrunning.Connection lost.Root cause
The console WebSocket handler (
console_ws_handlerincrates/hero_compute_ui/src/server.rs) performs an Origin/Host check:When hero_compute runs embedded, hero_router proxies the WebSocket upgrade over a Unix socket and rewrites the
Hostheader tolocalhostbefore forwarding (seeproxy_ws_tunnelinhero_router/crates/hero_router/src/server/routes.rs, which hard-codes.header("Host", "localhost")). TheOriginheader, however, is forwarded unchanged as the real browser origin (e.g.http://<os-host>:<port>).Result: the backend compares
origin_host = "<os-host>:<port>"againsthost = "localhost", thestarts_withcheck fails, and every WebSocket upgrade is rejected with403 Forbidden. The client then burns through its 10 reconnect attempts and gives up.In standalone mode the backend receives the real
Hostheader, so Origin and Host match and the check passes — which is why the bug only manifests when embedded.Proposed fix
Two coordinated changes:
X-Forwarded-Hostheader containing the originalHostvalue (alongside the existingX-Forwarded-Prefix/X-Hero-Contextinjections inws_proxy_inner).console_ws_handler, preferX-Forwarded-HostoverHostwhen 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.
Companion router-side issue: lhumina_code/hero_router#47 — tracks the
X-Forwarded-Hostinjection half of the fix.Implementation Specification for Issue #102
Objective
Fix the VM console WebSocket handler (
console_ws_handlerincrates/hero_compute_ui/src/server.rs) so that the Origin/Host check remains correct when hero_compute runs embedded behind hero_router. Router rewritesHosttolocalhostbut forwards the real browserOriginunchanged; the fix must preferX-Forwarded-Host(injected by hero_router per issue hero_router#47) when present, falling back toHostwhen running standalone.Requirements
Originagainst the externally-visible hostname rather than the intra-proxyHost.X-Forwarded-Hostif present, else fall back toHost.Origindoes not match the chosen target host.X-Forwarded-Host) must be identical to today so standalone browsers continue to connect.X-Forwarded-Hostcan equally setOrigin, so in production hero_router is the trust boundary injecting/sanitising that header.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 switchconsole_ws_handlerto 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_hosthelperFile:
crates/hero_compute_ui/src/server.rsDependencies: none
Add a private function near other small helpers:
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.evilstarts_withprefix bug; path-stripping keeps the commonOrigin: scheme://host[:port]case correct and defends againstOrigin: http://host/anything.Step 2 — Extract
effective_hosthelperFile:
crates/hero_compute_ui/src/server.rsDependencies: none
Add a private function:
Step 3 — Update
console_ws_handlerFile:
crates/hero_compute_ui/src/server.rsFunction:
console_ws_handlerDependencies: Steps 1 and 2
Replace the inline Origin/Host check with:
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_hostcases:("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]")→ trueeffective_hostcases (construct aHeaderMapin each test):x-forwarded-host: example.com,host: localhost→Some("example.com")x-forwarded-hostabsent,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")NoneStep 5 — Build and test
Dependencies: Steps 1-4
From
/home/rawan/codescalers/hero_compute:Fix any compile errors or test failures.
Step 6 — Manual verification (optional but recommended)
Dependencies: Step 5
Originand confirm 403.Acceptance Criteria
origin_matches_hosthelper exists, is private, has a doc comment, is unit-tested.effective_hosthelper exists, is private, is unit-tested.console_ws_handleruses both helpers and resolves the comparison host fromX-Forwarded-Hostfirst.X-Forwarded-Host) still accepts WS upgrades whoseOriginmatchesHost.X-Forwarded-Host, rewritesHost: localhost) accepts WS upgrades whoseOriginmatches the external host.Originstill returns 403.cargo test -p hero_compute_uipasses including new tests.cargo build -p hero_compute_uisucceeds with no new warnings.Notes
X-Forwarded-Hostat 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-Hostmissing: standalone deployments — falls back toHost, behaviour unchanged.Originbut not in forwarded host: if the router forwardsX-Forwarded-Host: example.combut the browser hithttps://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.Origin: retain current behaviour, do not reject (non-browser clients often omit it).[::1]:9001works 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 witho.to_ascii_lowercase().starts_with(&host.to_ascii_lowercase())and leave a TODO.Test Results
Command:
cargo test -p hero_compute_uiBranch:
development_vm_console_forwarded_hostBuild: PASS
Implementation Summary
Branch:
development_vm_console_forwarded_hostChanges
crates/hero_compute_ui/src/server.rsorigin_matches_host(origin, host) -> boolthat stripshttp:///https://and any path suffix fromOrigin, then compares againsthostwith case-insensitive equality. Strict equality closes a latent prefix-match bug in the previousstarts_withcheck (e.g.example.com.evilno longer matchesexample.com).effective_host(&HeaderMap) -> Option<String>that prefers the first non-empty comma-separated entry ofX-Forwarded-Host, falling back toHost.console_ws_handlernow uses both helpers for the Origin check. Thews.is_some()guard is preserved, so the check only runs on actual WebSocket upgrade requests (non-WS hits on this endpoint are unaffected).X-Forwarded-Host, the handler falls back toHostand the check passes exactly as before.X-Forwarded-Hostvia hero_router#47) now comparesOriginagainst the external hostname, so the 403 on embedded console connections is resolved.Test Results
cargo test -p hero_compute_uiorigin_matches_host(including http/https, port, IPv6 literal with and without port, case-insensitive, suffix-attack regression, empty origin/host, path-stripping) and 5 foreffective_host(prefers forwarded host, falls back to host, picks first comma-separated entry, handles empty forwarded value, returns None when both headers absent).Notes
console_ws_handleris the only one, verified bygrep -n "WebSocketUpgrade" crates/hero_compute_ui/src/.X-Forwarded-Hoston WS upgrades).