fix(homecore-api): close WS auth bypass + reply-theater, harden dev bin (ADR-161 A1/A2/A8)

A1 (CRITICAL): the /api/websocket handshake accepted any non-empty token, ignoring the LongLivedTokenStore whitelist the REST path enforces — a full WS auth bypass. Now validates via state.tokens().is_valid() before auth_ok; wrong tokens get auth_invalid + close. A2 (HIGH): WS command replies were pushed into an mpsc whose only consumer logged and discarded them — no result/pong/event reached the client. Split the socket with futures StreamExt::split; a dedicated writer task drains the response channel onto the wire. A8 (HIGH): the homecore-api dev bin bound 0.0.0.0 with unconditional allow-any auth and no env path. Wired the HOMECORE_TOKENS env path (dev fallback warn-logged when unset) and defaulted the bind to 127.0.0.1 (HOMECORE_BIND to opt into LAN). Tests (fail on old source): - ws_handshake::wrong_token_is_rejected (old → auth_ok) - ws_handshake::result_reply_is_received / ping_pong_reply_is_received (old → timeout) - server_bin_auth::provisioned_bin_rejects_wrong_bearer / from_env_path_enforces_whitelist Co-Authored-By: claude-flow <ruv@ruv.net>
2026-08-05 19:41:44 +00:00 · 2026-06-12 00:55:16 -04:00
parent 8487192d0f
commit 9d52d49c0b
6 changed files with 387 additions and 62 deletions
@@ -0,0 +1,77 @@
+//! HC-WS-08 (ADR-161): the `homecore-api-server` bin must honor the
+//! `HOMECORE_TOKENS` env whitelist instead of unconditionally accepting
+//! any non-empty bearer.
+//!
+//! `main()` is not directly callable, so this reproduces the bin's exact
+//! token-provisioning path (`LongLivedTokenStore::from_env()` when
+//! `HOMECORE_TOKENS` is set) and drives a real HTTP request through the
+//! router. On the pre-fix bin — which used `SharedState::new()` →
+//! `allow_any_non_empty()` with NO env path — a wrong bearer was
+//! accepted; this test asserts it is now rejected with 401.
+
+use axum::body::Body;
+use axum::http::{Request, StatusCode};
+use homecore::HomeCore;
+use homecore_api::{router, LongLivedTokenStore, SharedState};
+use tower::ServiceExt; // for `oneshot`
+
+/// Build the same state the bin builds when HOMECORE_TOKENS is set.
+async fn provisioned_state(valid: &str) -> SharedState {
+    // Mirror `from_env()` deterministically without mutating process
+    // env (which would race other tests): an `empty()` store with the
+    // one provisioned token registered is exactly what
+    // `from_env()` produces for `HOMECORE_TOKENS=<valid>`.
+    let store = LongLivedTokenStore::empty();
+    store.register(valid).await;
+    SharedState::with_tokens(HomeCore::new(), "Home", "test", store)
+}
+
+#[tokio::test]
+async fn provisioned_bin_rejects_wrong_bearer() {
+    let app = router(provisioned_state("the_real_token").await);
+    let resp = app
+        .oneshot(
+            Request::builder()
+                .uri("/api/states")
+                .header("Authorization", "Bearer the_wrong_token")
+                .body(Body::empty())
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(
+        resp.status(),
+        StatusCode::UNAUTHORIZED,
+        "a provisioned token store must reject a wrong bearer (HC-WS-08)"
+    );
+}
+
+#[tokio::test]
+async fn provisioned_bin_accepts_correct_bearer() {
+    let app = router(provisioned_state("the_real_token").await);
+    let resp = app
+        .oneshot(
+            Request::builder()
+                .uri("/api/states")
+                .header("Authorization", "Bearer the_real_token")
+                .body(Body::empty())
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(resp.status(), StatusCode::OK);
+}
+
+#[tokio::test]
+async fn from_env_path_enforces_whitelist() {
+    // Exercise the literal `from_env()` constructor the bin uses, under
+    // a serialized env mutation, to prove the env path itself enforces.
+    std::env::set_var("HOMECORE_TOKENS", "env_token_1, env_token_2");
+    let store = LongLivedTokenStore::from_env();
+    std::env::remove_var("HOMECORE_TOKENS");
+
+    assert!(store.is_valid("env_token_1").await);
+    assert!(store.is_valid("env_token_2").await);
+    assert!(!store.is_valid("not_in_whitelist").await);
+    assert!(!store.is_dev_mode().await, "from_env must NOT be dev mode");
+}
@@ -0,0 +1,168 @@
+//! End-to-end WebSocket handshake + reply tests (ADR-161, HC-WS-01/02).
+//!
+//! These bind a real `TcpListener`, serve the full router, and connect
+//! with a real WS client (`tokio-tungstenite`). They exercise the wire
+//! path the in-crate unit tests cannot.
+//!
+//! - `wrong_token_is_rejected` — FAILS on the pre-fix `ws.rs` that only
+//!   checked `token.trim().is_empty()` and accepted any non-empty token
+//!   (HC-WS-01: WS auth bypass).
+//! - `result_reply_is_received` — FAILS on the pre-fix `ws.rs` that moved
+//!   the socket into a recv-only task and discarded every reply with
+//!   `debug!("ws emit: {msg}")` (HC-WS-02: reply theater).
+
+use std::net::SocketAddr;
+
+use futures_util::{SinkExt, StreamExt};
+use homecore::HomeCore;
+use homecore_api::{router, LongLivedTokenStore, SharedState};
+use tokio_tungstenite::connect_async;
+use tokio_tungstenite::tungstenite::Message;
+
+/// Spawn the API on an ephemeral port with a real (non-dev) token store
+/// containing exactly one valid token. Returns the bound address.
+async fn spawn_server_with_token(valid_token: &str) -> SocketAddr {
+    let hc = HomeCore::new();
+    let tokens = LongLivedTokenStore::empty();
+    tokens.register(valid_token).await;
+    let state = SharedState::with_tokens(hc, "Test", "test-version", tokens);
+    let app = router(state);
+
+    let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
+    let addr = listener.local_addr().unwrap();
+    tokio::spawn(async move {
+        axum::serve(listener, app).await.unwrap();
+    });
+    addr
+}
+
+/// Read text frames until one parses as JSON; returns the parsed value.
+async fn next_json<S>(ws: &mut S) -> serde_json::Value
+where
+    S: StreamExt<Item = Result<Message, tokio_tungstenite::tungstenite::Error>> + Unpin,
+{
+    loop {
+        match ws.next().await {
+            Some(Ok(Message::Text(raw))) => {
+                if let Ok(v) = serde_json::from_str::<serde_json::Value>(&raw) {
+                    return v;
+                }
+            }
+            Some(Ok(_)) => continue,
+            other => panic!("expected text frame, got {other:?}"),
+        }
+    }
+}
+
+#[tokio::test]
+async fn wrong_token_is_rejected() {
+    // HC-WS-01: a provisioned store with one good token must reject a
+    // DIFFERENT (non-empty) token over the WS handshake. The old code
+    // sent `auth_ok` for any non-empty token — this asserts the fix.
+    let addr = spawn_server_with_token("good_token_abc").await;
+    let url = format!("ws://{addr}/api/websocket");
+    let (mut ws, _resp) = connect_async(&url).await.unwrap();
+
+    // Server → auth_required
+    let req = next_json(&mut ws).await;
+    assert_eq!(req["type"], "auth_required");
+
+    // Client → auth with the WRONG token
+    ws.send(Message::Text(
+        serde_json::json!({"type":"auth","access_token":"wrong_token_xyz"}).to_string(),
+    ))
+    .await
+    .unwrap();
+
+    // Server → auth_invalid (NOT auth_ok)
+    let resp = next_json(&mut ws).await;
+    assert_eq!(
+        resp["type"], "auth_invalid",
+        "wrong token must be rejected with auth_invalid, got: {resp}"
+    );
+    assert_ne!(resp["type"], "auth_ok", "wrong token must NOT receive auth_ok");
+}
+
+#[tokio::test]
+async fn correct_token_is_accepted() {
+    let addr = spawn_server_with_token("good_token_abc").await;
+    let url = format!("ws://{addr}/api/websocket");
+    let (mut ws, _resp) = connect_async(&url).await.unwrap();
+
+    let req = next_json(&mut ws).await;
+    assert_eq!(req["type"], "auth_required");
+
+    ws.send(Message::Text(
+        serde_json::json!({"type":"auth","access_token":"good_token_abc"}).to_string(),
+    ))
+    .await
+    .unwrap();
+
+    let resp = next_json(&mut ws).await;
+    assert_eq!(resp["type"], "auth_ok", "correct token should be accepted, got: {resp}");
+}
+
+#[tokio::test]
+async fn result_reply_is_received() {
+    // HC-WS-02: after a successful auth, a `get_states` command must
+    // produce a `result` reply RECEIVED over the socket. The old code
+    // discarded all replies in the rx-draining task, so this hangs/
+    // fails on the pre-fix source.
+    let addr = spawn_server_with_token("good_token_abc").await;
+    let url = format!("ws://{addr}/api/websocket");
+    let (mut ws, _resp) = connect_async(&url).await.unwrap();
+
+    let req = next_json(&mut ws).await;
+    assert_eq!(req["type"], "auth_required");
+
+    ws.send(Message::Text(
+        serde_json::json!({"type":"auth","access_token":"good_token_abc"}).to_string(),
+    ))
+    .await
+    .unwrap();
+    let auth = next_json(&mut ws).await;
+    assert_eq!(auth["type"], "auth_ok");
+
+    // Send a command and assert we RECEIVE a result reply.
+    ws.send(Message::Text(
+        serde_json::json!({"id": 1, "type": "get_states"}).to_string(),
+    ))
+    .await
+    .unwrap();
+
+    let reply = tokio::time::timeout(std::time::Duration::from_secs(5), next_json(&mut ws))
+        .await
+        .expect("did not receive a reply within 5s — reply theater (HC-WS-02)");
+    assert_eq!(reply["type"], "result", "expected a result reply, got: {reply}");
+    assert_eq!(reply["id"], 1);
+    assert_eq!(reply["success"], true);
+}
+
+#[tokio::test]
+async fn ping_pong_reply_is_received() {
+    // The `ping` command must produce a `pong` reply on the wire — also
+    // exercises the writer task that HC-WS-02 introduced.
+    let addr = spawn_server_with_token("good_token_abc").await;
+    let url = format!("ws://{addr}/api/websocket");
+    let (mut ws, _resp) = connect_async(&url).await.unwrap();
+
+    let _ = next_json(&mut ws).await; // auth_required
+    ws.send(Message::Text(
+        serde_json::json!({"type":"auth","access_token":"good_token_abc"}).to_string(),
+    ))
+    .await
+    .unwrap();
+    let _ = next_json(&mut ws).await; // auth_ok
+
+    ws.send(Message::Text(
+        serde_json::json!({"id": 7, "type": "ping"}).to_string(),
+    ))
+    .await
+    .unwrap();
+
+    let reply = tokio::time::timeout(std::time::Duration::from_secs(5), next_json(&mut ws))
+        .await
+        .expect("did not receive pong within 5s");
+    assert_eq!(reply["type"], "pong");
+    assert_eq!(reply["id"], 7);
+}