frank/ruvnet--RuView
1
0
Fork 0
mirror of https://github.com/ruvnet/RuView synced 2026-07-24 17:43:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ws): gate the dedicated WS port and the merged field routes (ADR-272 was incomplete)

Browse Source 
The previous commit claimed WebSocket upgrades were gated. They were not. Found
by an adversarial cross-vendor review, reproduced, and fixed here.

MEASURED BEFORE (auth ON via RUVIEW_API_TOKEN, no credential presented):

  HTTP port :3991  /ws/sensing  -> 401   (what the previous fix covered)
  HTTP port :3991  /ws/field    -> 101   HOLE
  WS   port :3990  /ws/sensing  -> 101   HOLE
  WS   port :3990  /ws/field    -> 101   HOLE
  control   :3991  /api/v1/models -> 401

Two independent defects, both from the same root cause — routes registered
AFTER an axum `.layer()` are silently exempt from it:

1. The dedicated WebSocket server on `--ws-port` was built with ONLY
   `host_validation::require_allowed_host`. `require_bearer` was never applied
   to it at all. My earlier verification only ever probed the HTTP port and I
   generalised from it.

   This is the worse of the two, because it is the port the UI actually uses:
   `ui/services/sensing.service.js` maps HTTP 8080 -> WS 8765. So the previous
   fix protected a path the browser never takes, while the path it does take
   stayed open.

2. On the HTTP router, `/ws/field` (ADR-262) was `.merge()`d AFTER the
   `require_bearer` layer, so it bypassed authentication entirely. The auth
   layer is now applied after the merge, and the comment says why the ordering
   is load-bearing.

MEASURED AFTER, same conditions:

  :3989 /ws/sensing -> 401 · :3989 /ws/field -> 401
  :3988 /ws/sensing -> 401 · :3988 /ws/field -> 401
  with bearer on the upgrade: 101 on both ports
  ticket minted at POST /api/v1/ws-ticket on the HTTP port and redeemed on the
    WS port: 101  (AuthState shares its TicketStore via Arc)
  auth OFF: 101 — unchanged, no regression

526 sensing-server tests still pass.

TEST GAP, stated rather than papered over: no automated test covers this. The
defect is in router WIRING in main.rs, not in logic a unit test reaches — both
holes were invisible to 526 green tests and to the ws_gate_tests suite, which
builds its own Router and therefore cannot see how the real one is assembled.
Catching this class needs an integration test that boots the binary and probes
BOTH ports; that is the honest follow-up.

Co-Authored-By: Ruflo & AQE
This commit is contained in:
Dragan Spiridonov
2026-07-23 09:14:07 +02:00
parent eb68e07a2c
commit 3347e258e6
1 changed files with 27 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
v2/crates/wifi-densepose-sensing-server/src/main.rs
+27 -10
View File
@@ -7993,6 +7993,18 @@ async fn main() {
// so a client on :8765 can stream signed RuField FieldEvents alongside
// `/ws/sensing`. Merged with its own FieldState (different state type).
.merge(rufield_surface::router(field_surface.clone()))
// ADR-272 FIX: this router had NO auth layer at all. `/ws/sensing` and
// `/ws/field` on the dedicated WS port accepted unauthenticated
// upgrades even with auth ON — and this is the port the UI actually
// uses (ui/services/sensing.service.js maps HTTP 8080 -> WS 8765), so
// gating only the HTTP port protected a path the browser never takes.
// Applied AFTER the merge so it covers the RuField routes too.
// AuthState shares its TicketStore via Arc, so a ticket minted at
// POST /api/v1/ws-ticket on the HTTP port is redeemable here.
.layer(axum::middleware::from_fn_with_state(
bearer_auth_state.clone(),
wifi_densepose_sensing_server::bearer_auth::require_bearer,
))
.layer(axum::middleware::from_fn_with_state(
host_allowlist.clone(),
wifi_densepose_sensing_server::host_validation::require_allowed_host,
@@ -8123,22 +8135,27 @@ async fn main() {
// is unset/empty the middleware is a no-op — the default stays
// LAN-mode-friendly. `/health*`, `/ws/sensing`, and `/ui/*` are never
// gated (orchestrator probes + local browsers).
.layer(axum::middleware::from_fn_with_state(
bearer_auth_state.clone(),
wifi_densepose_sensing_server::bearer_auth::require_bearer,
))
// ADR-272: the ws-ticket handler needs the store the middleware owns.
// Added AFTER the auth layer so it is still gated by it.
.layer(axum::Extension(bearer_auth_state.clone()))
.with_state(state.clone())
// ADR-262 P3: additive RuField surface (`/api/field` + `/ws/field`).
// Merged AFTER `.with_state` (so http_app is already `Router<()>` and
// can absorb the field router's own `FieldState`). These routes sit
// OUTSIDE `/api/v1/*` so they are not bearer-gated, but the
// host-validation layer below still applies (it is added last, so it
// runs first, over the whole merged router). The surface's own §10
// egress gate is what keeps above-policy classes off the wire.
// can absorb the field router's own `FieldState`).
.merge(rufield_surface::router(field_surface.clone()))
// Opt-in bearer auth (#443) + ADR-272 WebSocket gating.
//
// Applied AFTER the merge, and that ordering is load-bearing: axum
// `.layer()` wraps only what is already registered, so while this sat
// above the merge, `/ws/field` bypassed authentication entirely —
// measured 101 on an unauthenticated upgrade with auth ON. Adding
// routes after an auth layer silently exempts them, which is exactly
// the failure mode ADR-272 exists to prevent.
//
// Unset RUVIEW_API_TOKEN/RUVIEW_OAUTH_ISSUER still makes this a no-op.
.layer(axum::middleware::from_fn_with_state(
bearer_auth_state.clone(),
wifi_densepose_sensing_server::bearer_auth::require_bearer,
))
// DNS-rebinding defense: applied last so it runs first on the request
// path (axum layers run outermost-in). Rejects requests whose `Host`
// header is not in the allowlist before any handler — including