Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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<WebSocketUpgrade, WebSocketUpgradeRejection>` 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.
Original file line number Diff line number Diff line change
@@ -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<WebSocketUpgrade, WebSocketUpgradeRejection>`을 마지막 추출자로 받고 `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 업그레이드를 프록시하지 않으므로 라우터 수준 변경은 필요 없었다.
2 changes: 1 addition & 1 deletion docs/nemotron-voicechat.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <key>`), 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 <key>`), 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.

Expand Down
7 changes: 5 additions & 2 deletions src/server/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,12 @@ fn build_routes(state: &AppState) -> Router<AppState> {

// 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
Expand Down
21 changes: 21 additions & 0 deletions src/server/cors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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] {
[
Expand Down
77 changes: 77 additions & 0 deletions src/server/cors_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading
Loading