From b3d642ed4669bccde7671fa450b765f31c55b203 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Tue, 29 Sep 2026 23:48:43 +0900 Subject: [PATCH 1/3] fix(server): enforce CORS origin policy on /v1/realtime upgrade Browsers do not apply CORS to WebSocket upgrades, so a page on any site could open /v1/realtime, take the single session slot, and read every event even when --cors-origins or --allowed-origins restricted browser access. CorsPolicy::permits_websocket_origin checks a present Origin against the configured rule (any under *, localhost hosts under localhost, byte equality under a literal, membership under --allowed-origins) and accepts an absent Origin, which only non-browser clients send. realtime_router now takes the policy, and the handler answers 403 with an invalid_request_error body before the upgrade, so no reservation is taken. The origin check runs before upgrade validation; the API-key layer stays outside, so failing both checks is still 401. The check lives in the route, so router-mode sub-apps built without the CORS layer enforce it too. Tests: websocket_origin_matrix covers every policy against absent, allowed, disallowed, null and non-UTF-8 origins; an app-level test covers create_app and create_app_without_cors including the 401 ordering; tests/realtime_ws.rs gains start_server_with_policy and end-to-end handshake tests. Closes #2042 --- docs/nemotron-voicechat.md | 2 +- src/server/app.rs | 7 +- src/server/cors.rs | 21 ++++++ src/server/cors_tests.rs | 77 ++++++++++++++++++++++ src/server/routes/realtime.rs | 54 +++++++++++++--- src/server/routes/realtime_route_tests.rs | 78 +++++++++++++++++++++++ tests/realtime_ws.rs | 69 +++++++++++++++++++- 7 files changed, 293 insertions(+), 15 deletions(-) diff --git a/docs/nemotron-voicechat.md b/docs/nemotron-voicechat.md index b88d8ed57..7b0801216 100644 --- a/docs/nemotron-voicechat.md +++ b/docs/nemotron-voicechat.md @@ -93,7 +93,7 @@ The streamed encoder equals the offline encoder for the first frames and then dr ## Realtime WebSocket (`/v1/realtime`) -`mlxcel-server -m models/NemotronLabs-VoiceChat-11B-4bit --port 8080` (or `mlxcel serve`) loads the checkpoint on a dedicated realtime engine thread and serves the online session over a WebSocket at `/v1/realtime`. The chat worker, the batch scheduler and the text warmup are not started for this checkpoint; `/v1/chat/completions`, `/v1/completions`, `/v1/responses` and `/v1/messages` answer `501 Not Implemented` with a message naming `/v1/realtime`. `--api-key` applies to the upgrade request (send `Authorization: Bearer `), and `--api-prefix` prefixes the path like every other route. The protocol follows the mlx-vlm reference server (`mlx_vlm/server/realtime.py`). +`mlxcel-server -m models/NemotronLabs-VoiceChat-11B-4bit --port 8080` (or `mlxcel serve`) loads the checkpoint on a dedicated realtime engine thread and serves the online session over a WebSocket at `/v1/realtime`. The chat worker, the batch scheduler and the text warmup are not started for this checkpoint; `/v1/chat/completions`, `/v1/completions`, `/v1/responses` and `/v1/messages` answer `501 Not Implemented` with a message naming `/v1/realtime`. `--api-key` applies to the upgrade request (send `Authorization: Bearer `), and `--api-prefix` prefixes the path like every other route. `--cors-origins` and `--allowed-origins` also gate the upgrade by its `Origin` header: a browser page whose origin the policy does not allow gets `403 Forbidden`, while a client that sends no `Origin` (a non-browser client) is accepted. The protocol follows the mlx-vlm reference server (`mlx_vlm/server/realtime.py`). All MLX work runs on the engine thread: the socket task sends `open`, `push`, `flush`, `cancel` and `close` commands over a channel and awaits a one-shot reply, and the engine serializes each event (base64 audio included) before replying. The MLX buffer cache is cleared when a session closes, not per frame. diff --git a/src/server/app.rs b/src/server/app.rs index b6249fcfd..e9b66652e 100644 --- a/src/server/app.rs +++ b/src/server/app.rs @@ -558,9 +558,12 @@ fn build_routes(state: &AppState) -> Router { // Nemotron VoiceChat (#1376): the realtime WebSocket exists only when the // engine does. Merged here so the API-key and `--api-prefix` layers wrap - // it like every other route; the upgrade request carries the key. + // it like every other route; the upgrade request carries the key. The + // route checks `Origin` against the CORS policy itself (#2042), which + // also covers router-mode sub-apps built without the CORS layer. if let Some(engine) = state.realtime_engine.clone() { - app = app.merge(routes::realtime::realtime_router(engine)); + let cors = std::sync::Arc::new(state.config.cors_policy.clone()); + app = app.merge(routes::realtime::realtime_router(engine, cors)); } app diff --git a/src/server/cors.rs b/src/server/cors.rs index aaffbd735..d3299e2a8 100644 --- a/src/server/cors.rs +++ b/src/server/cors.rs @@ -165,6 +165,27 @@ impl CorsPolicy { } } + /// Whether a WebSocket upgrade carrying `origin` may proceed (#2042). + /// + /// Browsers do not apply CORS to WebSocket upgrades, so the server has to + /// check `Origin` itself. An absent `Origin` is accepted: browsers always + /// send one on an upgrade, and non-browser clients do not. A present value + /// (including `null`) must satisfy the same origin rule the HTTP routes + /// reflect: any value under `Wildcard`, a localhost host under + /// `Localhost`, byte equality under `Literal`, membership under + /// `AllowList`. `credentials` plays no part. + pub(crate) fn permits_websocket_origin(&self, origin: Option<&HeaderValue>) -> bool { + let Some(value) = origin else { + return true; + }; + match &self.origins { + OriginPolicy::Wildcard => true, + OriginPolicy::Localhost => origin_is_localhost(value), + OriginPolicy::Literal(literal) => literal.as_bytes() == value.as_bytes(), + OriginPolicy::AllowList(list) => list.contains(value), + } + } + /// The three headers b10621 adds to a preflight response. pub(crate) fn preflight_headers(&self) -> [(HeaderName, HeaderValue); 3] { [ diff --git a/src/server/cors_tests.rs b/src/server/cors_tests.rs index ed112e84f..efc9bab5b 100644 --- a/src/server/cors_tests.rs +++ b/src/server/cors_tests.rs @@ -248,6 +248,83 @@ fn the_mlxcel_allow_list_reflects_only_a_matching_origin() { assert_eq!(p.allow_origin(None), None); } +#[test] +fn websocket_origin_matrix() { + // #2042: every OriginPolicy variant against an absent Origin, an allowed + // value, a disallowed value and `null`. Credentials never matter. + let non_utf8 = HeaderValue::from_bytes(b"http://localhost\xff").expect("obs-text is valid"); + for credentials in [true, false] { + let wildcard = policy("*", credentials); + assert_eq!(wildcard.origins, OriginPolicy::Wildcard); + assert!(wildcard.permits_websocket_origin(None)); + for any in ["https://evil.example", "null", "http://localhost:3000"] { + assert!( + wildcard.permits_websocket_origin(Some(&origin(any))), + "{any}" + ); + } + assert!(wildcard.permits_websocket_origin(Some(&non_utf8))); + + let localhost = policy("localhost", credentials); + assert_eq!(localhost.origins, OriginPolicy::Localhost); + assert!(localhost.permits_websocket_origin(None)); + for ok in [ + "http://localhost:3000", + "http://127.0.0.1", + "http://[::1]:8080", + ] { + assert!( + localhost.permits_websocket_origin(Some(&origin(ok))), + "{ok}" + ); + } + for bad in [ + "https://localhost.evil.com", + "https://evil.example", + "null", + "localhost", + ] { + assert!( + !localhost.permits_websocket_origin(Some(&origin(bad))), + "{bad}" + ); + } + assert!(!localhost.permits_websocket_origin(Some(&non_utf8))); + + let literal = policy("https://app.example.com", credentials); + assert!(matches!(literal.origins, OriginPolicy::Literal(_))); + assert!(literal.permits_websocket_origin(None)); + assert!(literal.permits_websocket_origin(Some(&origin("https://app.example.com")))); + for bad in ["https://evil.example", "https://APP.example.com", "null"] { + assert!( + !literal.permits_websocket_origin(Some(&origin(bad))), + "{bad}" + ); + } + assert!(!literal.permits_websocket_origin(Some(&non_utf8))); + let null_literal = policy("null", credentials); + assert!(null_literal.permits_websocket_origin(Some(&origin("null")))); + assert!(!null_literal.permits_websocket_origin(Some(&origin("https://a.example")))); + + let list = parse_allowed_origins(&origins(&[ + "https://app.example.com", + "http://localhost:3000", + ])) + .unwrap(); + let allow = CorsPolicy::resolve("*", "GET", "*", credentials, Some(list)).expect("valid"); + assert!(matches!(allow.origins, OriginPolicy::AllowList(_))); + assert!(allow.permits_websocket_origin(None)); + for ok in ["https://app.example.com", "http://localhost:3000"] { + assert!(allow.permits_websocket_origin(Some(&origin(ok))), "{ok}"); + } + for bad in ["https://evil.example", "http://localhost:3001", "null"] { + assert!(!allow.permits_websocket_origin(Some(&origin(bad))), "{bad}"); + } + assert!(!allow.permits_websocket_origin(Some(&non_utf8))); + } + assert!(CorsPolicy::default().permits_websocket_origin(Some(&origin("https://evil.example")))); +} + #[test] fn preflight_headers_carry_the_configured_methods_and_credentials() { let p = CorsPolicy::resolve("*", "GET, POST", "X-Custom", false, None).expect("valid"); diff --git a/src/server/routes/realtime.rs b/src/server/routes/realtime.rs index 9de05b960..6d026b204 100644 --- a/src/server/routes/realtime.rs +++ b/src/server/routes/realtime.rs @@ -28,12 +28,16 @@ use std::sync::Arc; -use axum::Router; use axum::extract::State; +use axum::extract::ws::rejection::WebSocketUpgradeRejection; use axum::extract::ws::{CloseFrame, Message, WebSocket, WebSocketUpgrade}; -use axum::response::Response; +use axum::http::{HeaderMap, StatusCode, header}; +use axum::response::{IntoResponse, Response}; use axum::routing::get; +use axum::{Json, Router}; +use serde_json::json; +use crate::server::CorsPolicy; use crate::server::realtime_engine::{RealtimeSessionConfig, RealtimeVoiceChatEngine}; use crate::server::realtime_protocol::{ AudioFormat, CLOSE_CODE_POLICY, CLOSE_CODE_TRY_AGAIN_LATER, CODE_INFERENCE_ERROR, @@ -45,22 +49,52 @@ use crate::server::realtime_protocol::{ /// Route path of the realtime socket. pub const REALTIME_PATH: &str = "/v1/realtime"; -/// A router serving only [`REALTIME_PATH`], with the engine as its state. -/// `server/app.rs` merges it inside the auth and `--api-prefix` layers. -pub fn realtime_router(engine: Arc) -> Router +/// State of the realtime router: the engine and the origin policy the +/// upgrade is checked against. +#[derive(Clone)] +struct RealtimeRouteState { + engine: Arc, + cors: Arc, +} + +/// A router serving only [`REALTIME_PATH`]. `server/app.rs` merges it inside +/// the auth and `--api-prefix` layers, so a request failing both the API key +/// and the origin check gets 401. `cors` is the server's `--cors-origins` / +/// `--allowed-origins` policy: browsers do not apply CORS to WebSocket +/// upgrades, so the handler checks `Origin` itself (#2042). +pub fn realtime_router(engine: Arc, cors: Arc) -> Router where S: Clone + Send + Sync + 'static, { Router::new() .route(REALTIME_PATH, get(realtime_ws)) - .with_state(engine) + .with_state(RealtimeRouteState { engine, cors }) } -/// Upgrade handler. -pub async fn realtime_ws( - ws: WebSocketUpgrade, - State(engine): State>, +/// Upgrade handler. A disallowed `Origin` gets 403 before the upgrade is +/// even validated, so no reservation is taken and no session is created. +async fn realtime_ws( + State(state): State, + headers: HeaderMap, + ws: Result, ) -> Response { + let origin = headers.get(header::ORIGIN); + if !state.cors.permits_websocket_origin(origin) { + let shown = origin.map(|value| String::from_utf8_lossy(value.as_bytes()).into_owned()); + tracing::warn!("rejected /v1/realtime upgrade from origin {shown:?}"); + let body = json!({ + "error": { + "message": "origin not allowed for /v1/realtime", + "type": "invalid_request_error", + } + }); + return (StatusCode::FORBIDDEN, Json(body)).into_response(); + } + let ws = match ws { + Ok(ws) => ws, + Err(rejection) => return rejection.into_response(), + }; + let engine = state.engine; // Clients send 80 ms appends (about 3.5 KB of base64); 1 MiB (about // 24 s of audio per message) bounds what one message can make the // server decode and push before the socket is read again. diff --git a/src/server/routes/realtime_route_tests.rs b/src/server/routes/realtime_route_tests.rs index 18cb03cdd..5ba06536d 100644 --- a/src/server/routes/realtime_route_tests.rs +++ b/src/server/routes/realtime_route_tests.rs @@ -126,3 +126,81 @@ async fn realtime_route_is_mounted_only_with_an_engine_and_behind_api_keys() { let (status, _) = status_and_body(app, Method::GET, "/v1/realtime").await; assert_eq!(status, StatusCode::UNAUTHORIZED); } + +async fn upgrade_status(app: axum::Router, origin: Option<&str>) -> (StatusCode, String) { + let mut request = Request::builder() + .method(Method::GET) + .uri("/v1/realtime") + .header("connection", "upgrade") + .header("upgrade", "websocket") + .header("sec-websocket-version", "13") + .header("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="); + if let Some(origin) = origin { + request = request.header("origin", origin); + } + let response = app + .oneshot(request.body(Body::empty()).expect("request builds")) + .await + .expect("route responds"); + let status = response.status(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("body reads"); + (status, String::from_utf8_lossy(&bytes).into_owned()) +} + +fn allow_list_config(api_key: Option<&str>) -> ServerConfig { + let list = crate::server::cors::parse_allowed_origins(&["https://app.example.com".to_string()]) + .expect("valid origin"); + let cors_policy = crate::server::CorsPolicy::resolve("*", "GET", "*", true, Some(list)) + .expect("valid policy"); + let api_keys = api_key.map_or_else(Default::default, |key| { + crate::server::resolve_api_keys(&[key.to_string()], &[]).expect("valid key set") + }); + ServerConfig { + cors_policy, + api_keys, + ..Default::default() + } +} + +#[tokio::test] +async fn realtime_upgrade_enforces_the_origin_policy_in_both_app_builds() { + // #2042: the check lives in the route, so router-mode sub-apps (built + // without the CORS layer) enforce it too. + for with_cors in [true, false] { + let build = |config| { + let state = state(config, true); + if with_cors { + create_app(state) + } else { + crate::server::app::create_app_without_cors(state) + } + }; + let (status, body) = + upgrade_status(build(allow_list_config(None)), Some("https://evil.example")).await; + assert_eq!(status, StatusCode::FORBIDDEN, "cors={with_cors}: {body}"); + assert!( + body.contains("origin not allowed for /v1/realtime"), + "{body}" + ); + assert!(body.contains("invalid_request_error"), "{body}"); + + // Allowed and absent origins pass the check and reach the upgrade + // extractor, which refuses the in-process request (no hyper upgrade + // handle) with something other than 403. + for origin in [Some("https://app.example.com"), None] { + let (status, body) = upgrade_status(build(allow_list_config(None)), origin).await; + assert_ne!(status, StatusCode::FORBIDDEN, "{origin:?}: {body}"); + assert_ne!(status, StatusCode::NOT_FOUND, "{origin:?}: {body}"); + } + + // The API-key layer stays outside: failing both checks is 401. + let (status, _) = upgrade_status( + build(allow_list_config(Some("realtime-key"))), + Some("https://evil.example"), + ) + .await; + assert_eq!(status, StatusCode::UNAUTHORIZED, "cors={with_cors}"); + } +} diff --git a/tests/realtime_ws.rs b/tests/realtime_ws.rs index 82df22241..deb336d41 100644 --- a/tests/realtime_ws.rs +++ b/tests/realtime_ws.rs @@ -19,18 +19,21 @@ use std::sync::Arc; +use axum::http::HeaderValue; use base64::Engine as _; use futures::{SinkExt, StreamExt}; use mlxcel::models::nemotron_voicechat::VoiceChatEvent; use mlxcel::models::nemotron_voicechat::streaming::VoiceChatError; +use mlxcel::server::CorsPolicy; use mlxcel::server::realtime_engine::{ RealtimeModel, RealtimeSession, RealtimeSessionConfig, RealtimeSessionInfo, RealtimeVoiceChatEngine, }; use serde_json::{Value, json}; use tokio::net::TcpStream; -use tokio_tungstenite::tungstenite::Message; +use tokio_tungstenite::tungstenite::client::IntoClientRequest; use tokio_tungstenite::tungstenite::protocol::frame::coding::CloseCode; +use tokio_tungstenite::tungstenite::{Error as WsError, Message}; use tokio_tungstenite::{MaybeTlsStream, WebSocketStream}; type Socket = WebSocketStream>; @@ -116,9 +119,15 @@ impl RealtimeSession for FakeSession { /// Serve the realtime router on a free port; returns the socket URL. async fn start_server() -> String { + start_server_with_policy(CorsPolicy::default()).await +} + +/// [`start_server`] with an explicit origin policy (#2042). +async fn start_server_with_policy(policy: CorsPolicy) -> String { let engine = RealtimeVoiceChatEngine::spawn_with_loader("fake-voicechat", || Ok(FakeModel)) .expect("fake engine"); - let app = mlxcel::server::routes::realtime::realtime_router::<()>(Arc::new(engine)); + let app = + mlxcel::server::routes::realtime::realtime_router::<()>(Arc::new(engine), Arc::new(policy)); let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr = listener.local_addr().unwrap(); tokio::spawn(async move { @@ -432,3 +441,59 @@ async fn whole_float_sample_rate_is_accepted() { let event = next_event(&mut socket).await; assert_ne!(event["type"], "error", "16000.0 must be accepted: {event}"); } + +/// Connect with an optional `Origin` header; the handshake result as is. +async fn connect_from(url: &str, origin: Option<&str>) -> Result { + let mut request = url.into_client_request().unwrap(); + if let Some(origin) = origin { + let value = HeaderValue::from_str(origin).unwrap(); + request.headers_mut().insert("origin", value); + } + Ok(tokio_tungstenite::connect_async(request).await?.0) +} + +fn assert_forbidden(result: Result) { + let err = result.err().expect("a disallowed origin must not upgrade"); + assert!( + matches!(&err, WsError::Http(r) if r.status() == 403), + "{err:?}" + ); +} + +/// Retry until any previous session's reservation is released. +async fn created_after_release(url: &str, origin: Option<&str>) -> Socket { + for _ in 0..50 { + let mut socket = connect_from(url, origin).await.expect("origin upgrades"); + if next_event(&mut socket).await["type"] == "session.created" { + return socket; + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + panic!("no session.created for origin {origin:?}"); +} + +#[tokio::test] +async fn disallowed_origin_is_rejected_before_session() { + let allowed = vec![HeaderValue::from_static("https://app.example.com")]; + let policy = CorsPolicy::resolve("*", "GET", "*", true, Some(allowed)).unwrap(); + let url = start_server_with_policy(policy).await; + assert_forbidden(connect_from(&url, Some("https://evil.example")).await); + + // The rejected attempt held no reservation; allowed and absent Origin pass. + let mut first = connect_from(&url, Some("https://app.example.com")) + .await + .unwrap(); + assert_eq!(next_event(&mut first).await["type"], "session.created"); + drop(first); + created_after_release(&url, None).await; +} + +#[tokio::test] +async fn localhost_policy_gates_and_default_policy_accepts_any_origin() { + let policy = CorsPolicy::resolve("localhost", "GET", "*", true, None).unwrap(); + let url = start_server_with_policy(policy).await; + assert_forbidden(connect_from(&url, Some("https://localhost.evil.com")).await); + created_after_release(&url, Some("http://localhost:3000")).await; + let url = start_server().await; + created_after_release(&url, Some("https://evil.example")).await; +} From d43409c98e1df7de9b800353dd51ec5e4ea63666 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Tue, 29 Sep 2026 23:57:28 +0900 Subject: [PATCH 2/3] test: use expect_err in realtime origin handshake helper --- tests/realtime_ws.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/realtime_ws.rs b/tests/realtime_ws.rs index deb336d41..3b1b0bf00 100644 --- a/tests/realtime_ws.rs +++ b/tests/realtime_ws.rs @@ -453,7 +453,7 @@ async fn connect_from(url: &str, origin: Option<&str>) -> Result) { - let err = result.err().expect("a disallowed origin must not upgrade"); + let err = result.expect_err("a disallowed origin must not upgrade"); assert!( matches!(&err, WsError::Http(r) if r.status() == 403), "{err:?}" From 2fbdee6d698a0c2dcef3cdba280fd7a2b29250f4 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Tue, 29 Sep 2026 23:58:56 +0900 Subject: [PATCH 3/3] docs: add technical report for PR #2047 --- ...ime-websocket-origin-policy-20260929.en.md | 50 +++++++++++++++++++ ...ime-websocket-origin-policy-20260929.ko.md | 50 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 TECHNICAL_REPORTS/2047-realtime-websocket-origin-policy-20260929.en.md create mode 100644 TECHNICAL_REPORTS/2047-realtime-websocket-origin-policy-20260929.ko.md diff --git a/TECHNICAL_REPORTS/2047-realtime-websocket-origin-policy-20260929.en.md b/TECHNICAL_REPORTS/2047-realtime-websocket-origin-policy-20260929.en.md new file mode 100644 index 000000000..7bc8d66ef --- /dev/null +++ b/TECHNICAL_REPORTS/2047-realtime-websocket-origin-policy-20260929.en.md @@ -0,0 +1,50 @@ +# Technical Report: PR #2047 - Enforce CORS origin policy on /v1/realtime upgrade + +**Date**: 2026-09-29 + +**Status**: Implemented and validated with fake-engine tests; pending merge. + +**Languages**: Rust + +**Risk Level**: Low. The change is confined to the realtime route and one `CorsPolicy` method. The default `--cors-origins *` posture is unchanged. + +## Executive Summary + +`GET /v1/realtime` accepted a WebSocket upgrade from any `Origin`, whatever `--cors-origins` or `--allowed-origins` said. Browsers do not apply CORS to WebSocket upgrades, so any web page could open the socket, take the single VoiceChat session slot, and read every event (cross-site WebSocket hijacking). The route now checks `Origin` against the configured policy and answers `403` before the upgrade (issue #2042). + +## 1. Problem Statement + +`cors_middleware` only stamps `Access-Control-Allow-Origin` on responses. It never rejects a request, and router-mode sub-apps do not run it at all. A browser completes a WebSocket handshake whatever that header says, so an operator who set `--cors-origins localhost` or `--allowed-origins https://app.example.com` had no browser restriction on `/v1/realtime`. The exposed case is a server without `--api-key` (browsers cannot set `Authorization` on `new WebSocket`) reached from a page the user visits. The impact is occupation of the only session slot and compute use. + +## 2. Change Summary + +- **Policy** (`server::cors`): `CorsPolicy::permits_websocket_origin(Option<&HeaderValue>)`. An absent `Origin` is accepted. A present one, including `null`, must match the origin rule: any value under `Wildcard`, a localhost host (the existing `origin_is_localhost`) under `Localhost`, byte equality under `Literal`, membership under `AllowList`. `credentials` plays no part. +- **Route** (`server::routes::realtime`): `realtime_router(engine, cors)` with a private `RealtimeRouteState`. The handler checks the origin first and returns `403` with an `invalid_request_error` JSON body and one `warn!` log. No reservation is taken. +- **Wiring** (`server::app::build_routes`): passes `Arc::new(config.cors_policy.clone())`. Because the check is in the route, `create_app_without_cors` (router-mode sub-apps) enforces it too. +- **Docs**: one sentence in `docs/nemotron-voicechat.md`. + +## 3. Technical Decisions + +### Check in the route, not in `cors_middleware` + +Rejecting in the middleware would change HTTP CORS behavior that follows llama-server b10621, and the middleware is skipped for router-mode sub-apps. Only the one route that needs the check has it. + +### Origin check before upgrade validation + +The issue sketched the handler with `ws: WebSocketUpgrade` as the first extractor. The handler instead takes `Result` last and checks `Origin` first. A disallowed origin is refused even when the handshake is malformed, and the in-process app test can see the `403` through `create_app` without a hyper upgrade handle. A valid-origin request with a bad handshake still gets axum's own rejection. + +### Absent Origin accepted, API key stays outer + +Browsers always send `Origin` on an upgrade; `websocat`, the example clients, and the tests do not. Accepting an absent header keeps those clients working without weakening the browser case. The API-key layer wraps the route, so a request failing both checks gets `401`, as before. + +## 4. Validation + +- `websocket_origin_matrix` unit test: every policy variant against absent, allowed, disallowed, `null`, and non-UTF-8 origins, with credentials on and off. +- App-level test over `create_app` and `create_app_without_cors`: `403` for a disallowed origin, not `403` for allowed or absent, `401` when an API key is also missing. +- `tests/realtime_ws.rs`: `start_server_with_policy`; a disallowed origin fails the handshake with HTTP 403, then an allowed origin gets `session.created` on the first try (the rejected attempt held no reservation) and a client with no `Origin` gets one after release; the localhost policy rejects `https://localhost.evil.com` and accepts `http://localhost:3000`; the default policy accepts any origin. +- `cargo fmt --check`, `cargo clippy --release -p mlxcel --lib --tests --examples -D warnings`, the three contract tests, and `cargo test --release -p mlxcel --lib server::` (3270 passed, 0 failed) pass locally. GitHub CI was not awaited because the GB10 runner is down. + +## 5. Known Limitations + +- No real-checkpoint run; the fake-engine tests exercise the same route and handshake. +- Router mode does not proxy WebSocket upgrades today, so no router-level change was needed. diff --git a/TECHNICAL_REPORTS/2047-realtime-websocket-origin-policy-20260929.ko.md b/TECHNICAL_REPORTS/2047-realtime-websocket-origin-policy-20260929.ko.md new file mode 100644 index 000000000..21e7b8231 --- /dev/null +++ b/TECHNICAL_REPORTS/2047-realtime-websocket-origin-policy-20260929.ko.md @@ -0,0 +1,50 @@ +# 기술 보고서: PR #2047 - /v1/realtime 업그레이드에 CORS origin 정책 적용 + +**날짜**: 2026-09-29 + +**상태**: 구현 완료, fake 엔진 테스트로 검증, 머지 대기. + +**언어**: Rust + +**위험도**: 낮음. 변경은 realtime 라우트와 `CorsPolicy` 메서드 하나에 한정된다. 기본 `--cors-origins *` 동작은 바뀌지 않는다. + +## 요약 + +`GET /v1/realtime`은 `--cors-origins`나 `--allowed-origins` 설정과 무관하게 모든 `Origin`의 WebSocket 업그레이드를 받아들였다. 브라우저는 WebSocket 업그레이드에 CORS를 적용하지 않으므로, 어떤 웹 페이지든 소켓을 열어 하나뿐인 VoiceChat 세션 슬롯을 차지하고 모든 이벤트를 읽을 수 있었다(cross-site WebSocket hijacking). 이제 라우트가 설정된 정책으로 `Origin`을 검사하고, 업그레이드 전에 `403`으로 응답한다(이슈 #2042). + +## 1. 문제 정의 + +`cors_middleware`는 응답에 `Access-Control-Allow-Origin`을 붙이기만 하고 요청을 거부하지 않으며, 라우터 모드 서브 앱에서는 아예 실행되지 않는다. 브라우저는 이 헤더와 상관없이 WebSocket 핸드셰이크를 완료하므로, `--cors-origins localhost`나 `--allowed-origins https://app.example.com`을 설정한 운영자도 `/v1/realtime`에 대해서는 브라우저 제한이 없었다. 노출되는 경우는 `--api-key` 없이 실행된 서버(브라우저는 `new WebSocket`에 `Authorization`을 설정할 수 없다)에 사용자가 방문한 페이지가 접근하는 경우다. 영향은 유일한 세션 슬롯 점유와 연산 자원 사용이다. + +## 2. 변경 요약 + +- **정책** (`server::cors`): `CorsPolicy::permits_websocket_origin(Option<&HeaderValue>)`. `Origin`이 없으면 허용한다. 있으면(`null` 포함) origin 규칙을 만족해야 한다: `Wildcard`는 모든 값, `Localhost`는 localhost 호스트(기존 `origin_is_localhost`), `Literal`은 바이트 단위 일치, `AllowList`는 목록 포함. `credentials`는 관여하지 않는다. +- **라우트** (`server::routes::realtime`): 비공개 `RealtimeRouteState`를 쓰는 `realtime_router(engine, cors)`. 핸들러는 origin을 먼저 검사하고, 거부 시 `invalid_request_error` JSON 본문과 함께 `403`을 반환하며 `warn!` 로그를 한 번 남긴다. 예약은 잡지 않는다. +- **연결** (`server::app::build_routes`): `Arc::new(config.cors_policy.clone())`을 전달한다. 검사가 라우트에 있으므로 `create_app_without_cors`(라우터 모드 서브 앱)에서도 적용된다. +- **문서**: `docs/nemotron-voicechat.md`에 한 문장 추가. + +## 3. 기술적 결정 + +### `cors_middleware`가 아닌 라우트에서 검사 + +미들웨어에서 거부하면 llama-server b10621을 따르는 HTTP CORS 동작이 바뀌고, 라우터 모드 서브 앱에서는 미들웨어가 생략된다. 검사가 필요한 라우트 하나에만 둔다. + +### 업그레이드 검증보다 origin 검사를 먼저 + +이슈는 핸들러의 첫 추출자로 `ws: WebSocketUpgrade`를 제시했다. 실제 구현은 `Result`을 마지막 추출자로 받고 `Origin`을 먼저 검사한다. 핸드셰이크가 잘못된 경우에도 허용되지 않은 origin은 거부되며, 프로세스 내 앱 테스트가 hyper 업그레이드 핸들 없이 `create_app`을 통해 `403`을 확인할 수 있다. 허용된 origin에 잘못된 핸드셰이크는 여전히 axum 자체의 거부 응답을 받는다. + +### Origin 없는 요청 허용, API 키 검사가 바깥 + +브라우저는 업그레이드 시 항상 `Origin`을 보내지만 `websocat`, 예제 클라이언트, 테스트는 보내지 않는다. 헤더가 없는 요청을 허용해도 브라우저 경우의 보호는 약해지지 않는다. API 키 레이어가 라우트를 감싸므로 두 검사 모두 실패하면 이전처럼 `401`을 받는다. + +## 4. 검증 + +- `websocket_origin_matrix` 단위 테스트: 모든 정책 변형을 origin 없음, 허용, 비허용, `null`, 비 UTF-8 값에 대해 credentials 켜짐과 꺼짐 모두로 검사. +- `create_app`과 `create_app_without_cors`에 대한 앱 수준 테스트: 비허용 origin은 `403`, 허용 또는 없음은 `403` 아님, API 키가 없으면 `401`. +- `tests/realtime_ws.rs`: `start_server_with_policy` 추가. 비허용 origin은 HTTP 403으로 핸드셰이크 실패, 이후 허용 origin은 첫 시도에 `session.created`를 받고(거부된 시도가 예약을 잡지 않았음을 확인) `Origin` 없는 클라이언트는 해제 후 세션을 받는다. localhost 정책은 `https://localhost.evil.com`을 거부하고 `http://localhost:3000`을 허용한다. 기본 정책은 모든 origin을 허용한다. +- `cargo fmt --check`, `cargo clippy --release -p mlxcel --lib --tests --examples -D warnings`, 세 계약 테스트, `cargo test --release -p mlxcel --lib server::`(3270 통과, 0 실패)가 로컬에서 통과했다. GB10 러너가 중단되어 GitHub CI는 기다리지 않았다. + +## 5. 알려진 한계 + +- 실제 체크포인트로는 실행하지 않았다. fake 엔진 테스트가 같은 라우트와 핸드셰이크를 검증한다. +- 라우터 모드는 현재 WebSocket 업그레이드를 프록시하지 않으므로 라우터 수준 변경은 필요 없었다.