mirror of
https://github.com/ruvnet/RuView
synced 2026-07-25 17:51:48 +00:00
feat(ui): fetch WebSocket tickets, prefix-match WS paths, and write ADR-272
Completes ADR-272. Three parts. 1. PREFIX MATCHING, not an allowlist. Anything under `/ws/` is a WebSocket path, plus `/api/v1/stream/pose` which lives outside it. An allowlist means every WebSocket route added later ships ungated until someone remembers to extend it — the same bug reintroduced on a delay. Not hypothetical: `/ws/train/progress` (ADR-186, arriving with PR #1387) is ALREADY referenced by ui/services/training.service.js and would have shipped unauthenticated. Pinned by a test that asserts it is gated before it exists. 2. UI wiring. A shared `withWsTicket()` helper mints a ticket immediately before each connection attempt — never cached, because a ticket is single-use and expires in seconds, so reusing one across reconnects fails on the second attempt. Wired into the three sites that open gated sockets: sensing.service.js, websocket-client.js, observatory/js/main.js. It degrades in both directions on purpose: no stored token means auth is off and no ticket is needed; a 404 from /api/v1/ws-ticket means a server predating this ADR, which still exempts WebSockets, so connecting without a ticket is correct there. The same UI therefore works against old and new servers, which is what makes the escape hatch removable later rather than permanent. The long-lived bearer token is still never put in a URL — only the ticket is. 3. ADR-272 itself. Previously cited in five places without existing; the same dangling-reference mistake made with ADR-271 earlier today, so it is written before this lands rather than after someone notices. It records the measured before/after, why a credential in a URL is acceptable here specifically (single use, seconds-long, not the credential), why the escape hatch exists and why it is deliberately uncomfortable, and what is deliberately NOT done — including that /health/metrics stays ungated, with the caveat that this should be revisited if metrics ever carry occupancy-derived values, since that would make them sensing data wearing an ops label. Tests: 526 sensing-server (4 new path-matching), 82 ruview-auth. JS syntax-checked with `node --check`; there is no UI test suite to extend. Co-Authored-By: Ruflo & AQE
This commit is contained in:
@@ -86,6 +86,10 @@ pub const PROTECTED_PREFIX: &str = "/api/v1/";
|
||||
///
|
||||
/// They are now gated, and accept **either** a bearer (native clients, which
|
||||
/// are not browser-constrained) **or** a single-use ticket (ADR-272).
|
||||
///
|
||||
/// This list is the set that exists today, used for the boot warning and tests.
|
||||
/// The runtime rule is [`is_ws_path`], which matches by prefix so routes added
|
||||
/// later are gated without an edit here.
|
||||
pub const WS_PATHS: &[&str] = &[
|
||||
"/ws/sensing",
|
||||
"/ws/introspection",
|
||||
@@ -109,8 +113,27 @@ fn legacy_ws_unauthenticated() -> bool {
|
||||
)
|
||||
}
|
||||
|
||||
/// WebSocket upgrade paths that do NOT live under [`WS_PREFIX`] and so must be
|
||||
/// named explicitly.
|
||||
pub const WS_PATHS_OUTSIDE_PREFIX: &[&str] = &["/api/v1/stream/pose"];
|
||||
|
||||
/// Everything under here is treated as a WebSocket upgrade.
|
||||
pub const WS_PREFIX: &str = "/ws/";
|
||||
|
||||
/// Is this a WebSocket upgrade path?
|
||||
///
|
||||
/// Matched by **prefix**, not by an allowlist, and that choice is the whole
|
||||
/// point. An allowlist means every WebSocket route added later is ungated until
|
||||
/// someone remembers to add it here — which is exactly the bug this module just
|
||||
/// fixed, reintroduced on a delay. `/ws/train/progress` (ADR-186, arriving with
|
||||
/// PR #1387) is already referenced by `ui/services/training.service.js` and
|
||||
/// would have shipped unauthenticated under an allowlist.
|
||||
///
|
||||
/// `/api/v1/stream/pose` is the one upgrade endpoint outside the prefix, so it
|
||||
/// is named. New WebSocket routes should go under `/ws/` and inherit gating for
|
||||
/// free.
|
||||
fn is_ws_path(path: &str) -> bool {
|
||||
WS_PATHS.contains(&path)
|
||||
path.starts_with(WS_PREFIX) || WS_PATHS_OUTSIDE_PREFIX.contains(&path)
|
||||
}
|
||||
|
||||
/// Cognitum OAuth verification state. Built once at boot and shared.
|
||||
@@ -1229,3 +1252,44 @@ mod ws_gate_tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod ws_path_matching_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn every_currently_known_websocket_path_matches() {
|
||||
for p in WS_PATHS {
|
||||
assert!(is_ws_path(p), "{p} must be recognised as a WebSocket path");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_websocket_route_that_does_not_exist_yet_is_already_gated() {
|
||||
// `/ws/train/progress` arrives with ADR-186 (PR #1387) and is already
|
||||
// referenced by the UI. Under an exact-match allowlist it would ship
|
||||
// unauthenticated. Prefix matching means it is gated on arrival.
|
||||
assert!(is_ws_path("/ws/train/progress"));
|
||||
assert!(is_ws_path("/ws/anything-added-in-future"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ordinary_rest_paths_are_not_treated_as_websockets() {
|
||||
for p in [
|
||||
"/api/v1/models",
|
||||
"/api/v1/stream/status", // a plain GET, not an upgrade
|
||||
"/health",
|
||||
"/ui/index.html",
|
||||
"/",
|
||||
] {
|
||||
assert!(!is_ws_path(p), "{p} must not be treated as a WebSocket path");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_path_merely_starting_with_ws_is_not_the_ws_prefix() {
|
||||
// `/wsx/...` must not match `/ws/`.
|
||||
assert!(!is_ws_path("/wsx/sensing"));
|
||||
assert!(!is_ws_path("/ws"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user