security(websocket): auth token travels via Sec-WebSocket-Protocol, not the URL (#16457)

autobot-backend/api/live_events.py

@@ -378,15 +378,21 @@ async def live_events_endpoint(websocket: WebSocket):
     # raw-JWT check was too strict and rejected valid deployments.
     from auth_middleware import authenticate_websocket
 
+    # #16457: echo the client's offered subprotocol on every accept() in this handshake.
+    # RFC 6455 4.2.2 requires the server to choose one of the client's offered
+    # subprotocols, and a browser fails the handshake if none is echoed back.
+    protocols = websocket.headers.get("sec-websocket-protocol", "")
+    subprotocol = "bearer" if protocols.startswith("bearer") else None
+
     user_payload: dict | None = await authenticate_websocket(websocket)
     if _auth_required() and user_payload is None:
         # accept() before close(4001) so clients see a clean close frame
         # instead of a handshake 403 (project WS rule).
-        await websocket.accept()
+        await websocket.accept(subprotocol=subprotocol)
         await websocket.close(code=4001, reason="Unauthorized")
         logger.info("Live events WebSocket rejected: invalid token")
         return
-    await websocket.accept()
+    await websocket.accept(subprotocol=subprotocol)
     logger.info(
         "Live events WebSocket connected: %s (user=%s)",
         websocket.client,

autobot-backend/api/live_events_ws_subprotocol_16457_test.py

@@ -0,0 +1,76 @@
+# Copyright 2025-2026 mrveiss
+# SPDX-License-Identifier: Apache-2.0
+# AutoBot - AI-Powered Automation Platform
+# Author: mrveiss
+"""``live_events_endpoint`` echoes the client's ``bearer`` subprotocol (#16457).
+
+RFC 6455 4.2.2 requires a server that accepts a handshake carrying subprotocols
+to choose and echo one of them, or the browser fails the handshake outright --
+so every ``websocket.accept()`` call in this endpoint (both the success path and
+the pre-close-4001 rejection path) must pass ``subprotocol="bearer"`` whenever
+the client offered it, mirroring ``api/process_management.py``'s convention
+(#16374, see ``api/process_management_stream_subprotocol_test.py``).
+"""
+
+from __future__ import annotations
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from starlette.websockets import WebSocketState
+
+import api.live_events as live_events
+from api.live_events import live_events_endpoint
+
+
+def _fake_ws(*, protocols: str = "", client_state=WebSocketState.DISCONNECTED) -> AsyncMock:
+    """A fake WebSocket that never enters the message loop (client_state != CONNECTED)."""
+    ws = AsyncMock()
+    ws.accept = AsyncMock()
+    ws.close = AsyncMock()
+    ws.send_json = AsyncMock()
+    ws.headers = MagicMock()
+    ws.headers.get = MagicMock(
+        side_effect=lambda key, default="": protocols if key == "sec-websocket-protocol" else default
+    )
+    ws.client_state = client_state
+    ws.client = "test-client"
+    return ws
+
+
+@pytest.fixture(autouse=True)
+def _stub_event_bus():
+    """The success path's finally-block calls get_event_bus().remove_client(); stub it inert."""
+    bus = MagicMock()
+    bus.remove_client = AsyncMock()
+    with patch.object(live_events, "get_event_bus", return_value=bus):
+        yield bus
+
+
+async def test_accepts_with_bearer_subprotocol_on_success(monkeypatch):
+    ws = _fake_ws(protocols="bearer, sometoken")
+    monkeypatch.setattr(live_events, "_auth_required", lambda: False)
+    with patch.object(live_events, "enforce_ws_origin", new=AsyncMock(return_value=True)):
+        with patch("auth_middleware.authenticate_websocket", new=AsyncMock(return_value=None)):
+            await live_events_endpoint(ws)
+    assert ws.accept.call_args.kwargs.get("subprotocol") == "bearer"
+
+
+async def test_accepts_with_no_subprotocol_when_none_offered_on_success(monkeypatch):
+    ws = _fake_ws(protocols="")
+    monkeypatch.setattr(live_events, "_auth_required", lambda: False)
+    with patch.object(live_events, "enforce_ws_origin", new=AsyncMock(return_value=True)):
+        with patch("auth_middleware.authenticate_websocket", new=AsyncMock(return_value=None)):
+            await live_events_endpoint(ws)
+    assert ws.accept.call_args.kwargs.get("subprotocol") is None
+
+
+async def test_accepts_with_bearer_subprotocol_on_unauthorized_rejection():
+    """The accept()-before-close(4001) rejection path must echo it too."""
+    ws = _fake_ws(protocols="bearer, sometoken")
+    with patch.object(live_events, "enforce_ws_origin", new=AsyncMock(return_value=True)):
+        with patch.object(live_events, "_auth_required", return_value=True):
+            with patch("auth_middleware.authenticate_websocket", new=AsyncMock(return_value=None)):
+                await live_events_endpoint(ws)
+    assert ws.accept.call_args.kwargs.get("subprotocol") == "bearer"
+    ws.close.assert_awaited_once_with(code=4001, reason="Unauthorized")

autobot-backend/api/presence_ws.py

@@ -76,30 +76,36 @@ async def session_presence(
     if not await enforce_ws_origin(websocket):
         return
 
+    # #16457: echo the client's offered subprotocol on every accept() below. RFC 6455
+    # 4.2.2 requires the server to choose one of the client's offered subprotocols, and
+    # a browser fails the handshake if none is echoed back.
+    protocols = websocket.headers.get("sec-websocket-protocol", "")
+    subprotocol = "bearer" if protocols.startswith("bearer") else None
+
     # #16455: the caller's identity comes ONLY from a verified JWT, never
     # from client-supplied input -- a prior version trusted a bare `user_id`
     # query param, letting any caller join any session as anyone.
     user_payload = await authenticate_websocket(websocket)
     if user_payload is None or user_payload.get("user_id") is None:
         # accept() before close(4001), matching api/live_events.py's own rule:
         # clients see a clean close frame, not a raw handshake rejection.
-        await websocket.accept()
+        await websocket.accept(subprotocol=subprotocol)
         await websocket.close(code=_WS_CLOSE_UNAUTHENTICATED, reason="Unauthorized")
         logger.info("Presence WS rejected: invalid or missing token, session=%s", session_id)
         return
 
     try:
         user_id = uuid.UUID(str(user_payload["user_id"]))
     except ValueError:
-        await websocket.accept()
+        await websocket.accept(subprotocol=subprotocol)
         await websocket.close(code=_WS_CLOSE_UNAUTHENTICATED, reason="Unauthorized")
         logger.warning("Presence WS rejected: malformed user_id in token, session=%s", session_id)
         return
 
     # Authenticated is not authorized: a verified user still must not join a
     # session they aren't a participant of.
     if not await _authorized_participant(session_id, user_id):
-        await websocket.accept()
+        await websocket.accept(subprotocol=subprotocol)
         await websocket.close(code=_WS_CLOSE_POLICY_VIOLATION, reason="Not a session participant")
         logger.info("Presence WS refused: user=%s not a participant of session=%s", user_id, session_id)
         return

autobot-backend/api/presence_ws_subprotocol_16457_test.py

@@ -0,0 +1,75 @@
+# Copyright 2025-2026 mrveiss
+# SPDX-License-Identifier: Apache-2.0
+# AutoBot - AI-Powered Automation Platform
+# Author: mrveiss
+"""``session_presence``'s rejection paths echo the client's ``bearer`` subprotocol (#16457).
+
+RFC 6455 4.2.2 requires a server that accepts a handshake carrying subprotocols
+to choose and echo one of them, or the browser fails the handshake outright.
+All three of this endpoint's accept()-before-close() rejection paths (missing
+identity, malformed user_id, not a session participant) must pass
+``subprotocol="bearer"`` whenever the client offered it -- mirroring
+``api/process_management.py``'s convention (#16374). The success path defers
+to ``presence_websocket_handler`` (``websocket/presence.py``), covered by its
+own test file.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+import api.presence_ws as presence_ws
+from api.presence_ws import session_presence
+
+
+def _fake_ws(*, protocols: str = "bearer, sometoken") -> AsyncMock:
+    ws = AsyncMock()
+    ws.accept = AsyncMock()
+    ws.close = AsyncMock()
+    ws.headers = MagicMock()
+    ws.headers.get = MagicMock(
+        side_effect=lambda key, default="": protocols if key == "sec-websocket-protocol" else default
+    )
+    return ws
+
+
+@pytest.fixture(autouse=True)
+def _allow_origin():
+    with patch.object(presence_ws, "enforce_ws_origin", new=AsyncMock(return_value=True)):
+        yield
+
+
+async def test_echoes_bearer_on_missing_identity_rejection():
+    ws = _fake_ws(protocols="bearer, sometoken")
+    with patch.object(presence_ws, "authenticate_websocket", new=AsyncMock(return_value=None)):
+        await session_presence(ws, "session-1")
+    assert ws.accept.call_args.kwargs.get("subprotocol") == "bearer"
+    ws.close.assert_awaited_once_with(code=presence_ws._WS_CLOSE_UNAUTHENTICATED, reason="Unauthorized")
+
+
+async def test_no_subprotocol_echoed_when_none_offered_on_missing_identity():
+    ws = _fake_ws(protocols="")
+    with patch.object(presence_ws, "authenticate_websocket", new=AsyncMock(return_value=None)):
+        await session_presence(ws, "session-1")
+    assert ws.accept.call_args.kwargs.get("subprotocol") is None
+
+
+async def test_echoes_bearer_on_malformed_user_id_rejection():
+    ws = _fake_ws(protocols="bearer, sometoken")
+    payload = {"user_id": "not-a-uuid"}
+    with patch.object(presence_ws, "authenticate_websocket", new=AsyncMock(return_value=payload)):
+        await session_presence(ws, "session-1")
+    assert ws.accept.call_args.kwargs.get("subprotocol") == "bearer"
+    ws.close.assert_awaited_once_with(code=presence_ws._WS_CLOSE_UNAUTHENTICATED, reason="Unauthorized")
+
+
+async def test_echoes_bearer_on_not_a_participant_rejection():
+    ws = _fake_ws(protocols="bearer, sometoken")
+    payload = {"user_id": "11111111-1111-1111-1111-111111111111"}
+    with patch.object(presence_ws, "authenticate_websocket", new=AsyncMock(return_value=payload)):
+        with patch.object(presence_ws, "_authorized_participant", new=AsyncMock(return_value=False)):
+            await session_presence(ws, "session-1")
+    assert ws.accept.call_args.kwargs.get("subprotocol") == "bearer"
+    ws.close.assert_awaited_once_with(code=presence_ws._WS_CLOSE_POLICY_VIOLATION, reason="Not a session participant")

autobot-backend/api/ws_security.py

@@ -80,12 +80,13 @@ async def enforce_ws_origin(websocket: WebSocket) -> bool:
 async def _resolve_ws_user(websocket: WebSocket) -> "dict | None":
     """Try every credential source a caller might legitimately present.
 
-    A browser JS client can only put a JWT in the query string -- it cannot
-    set custom headers on a WebSocket handshake -- so the query-param check
-    (:func:`auth_middleware.authenticate_websocket`) goes first. A
-    non-browser or service caller may instead send an ``Authorization``
-    header, an ``X-Session-ID`` session header, a dev-mode header, or the
-    internal-service key; ``get_user_from_request`` and
+    A browser JS client cannot set arbitrary custom headers on a WebSocket
+    handshake, but it can set the ``Sec-WebSocket-Protocol`` header (via the
+    `WebSocket` constructor's ``protocols`` argument) and the query string --
+    so the subprotocol/query-param check (:func:`auth_middleware.authenticate_websocket`,
+    #16457) goes first. A non-browser or service caller may instead send an
+    ``Authorization`` header, an ``X-Session-ID`` session header, a dev-mode
+    header, or the internal-service key; ``get_user_from_request`` and
     ``verify_internal_api_key`` already resolve those for HTTP requests, and
     a WebSocket exposes the same ``headers``/``cookies`` interface
     (``authenticate_ws_admin`` relies on the same fact). Trying each in turn

autobot-backend/auth_middleware.py

@@ -195,13 +195,11 @@ def _load_pem(pem: str):
 
         # Tier 2: durable file (survives restart and code-sync deploys)
         #
-        # #13162: the existence probe belongs INSIDE the guard. Path.exists()
-        # only swallows "not there" errnos (ENOENT/ENOTDIR/ELOOP) — EACCES
-        # propagates. When the service user cannot traverse the service-keys
-        # directory (e.g. it is mode 0700 and owned by another account) the
-        # PermissionError escaped this function, out of the AuthMiddleware
-        # constructor, and turned every request behind check_admin_permission
-        # into a 500 instead of falling through to the tiers below.
+        # #13162: the existence probe belongs INSIDE the guard. Path.exists() only swallows "not there" errnos
+        # (ENOENT/ENOTDIR/ELOOP) — EACCES propagates. When the service user cannot traverse the service-keys directory
+        # (e.g. it is mode 0700 and owned by another account) the PermissionError escaped this function, out of the
+        # AuthMiddleware constructor, and turned every request behind check_admin_permission into a 500 instead of
+        # falling through to the tiers below.
         try:
             if key_file.exists():
                 pem_private = key_file.read_text(encoding="utf-8")
@@ -308,9 +306,8 @@ def _get_auth_disabled_user(self) -> Dict:
         Returns default admin user dict when auth is disabled.
         """
         return {
-            # user_id/sub so user endpoints (e.g. /users/me/preferences) resolve
-            # the identity when auth is disabled; without them they 401'd ('User ID
-            # not found in token') → frontend logout → login redirect loop.
+            # user_id/sub so user endpoints (e.g. /users/me/preferences) resolve the identity when auth is disabled;
+            # without them they 401'd ('User ID not found in token') → frontend logout → login redirect loop.
             "user_id": "admin",
             "sub": "admin",
             "username": "admin",
@@ -1056,7 +1053,7 @@ async def _require_device_jwt(request: Request) -> Dict:
 async def authenticate_websocket(websocket) -> dict | None:
     """Authenticate a WebSocket connection.
 
-    Checks for JWT token in query params. Returns None if unauthenticated.
+    Checks for JWT via Sec-WebSocket-Protocol (preferred) or query param (fallback). None if unauthenticated.
 
     Issue #2818: Add auth before websocket.accept() to reject unauthenticated
     connections at the protocol handshake level.
@@ -1067,8 +1064,11 @@ async def authenticate_websocket(websocket) -> dict | None:
     Returns:
         User dict or None if authentication fails.
     """
-    # Check query param token
-    token = websocket.query_params.get("token")
+    # #16457: prefer Sec-WebSocket-Protocol (['bearer', '<jwt>']) over the query param so the
+    # token never lands in URL access logs/browser history; query stays a fallback during migration.
+    protocols = [p.strip() for p in websocket.headers.get("sec-websocket-protocol", "").split(",")]
+    token = protocols[1] if len(protocols) == 2 and protocols[0] == "bearer" and protocols[1] else None
+    token = token or websocket.query_params.get("token")
     if token:
         try:
             # Use the singleton — a fresh AuthenticationMiddleware() generates a

autobot-backend/tests/auth_middleware_ws_subprotocol_16457_test.py

@@ -0,0 +1,92 @@
+# Copyright 2025-2026 mrveiss
+# SPDX-License-Identifier: Apache-2.0
+# AutoBot - AI-Powered Automation Platform
+# Author: mrveiss
+"""``authenticate_websocket`` prefers the Sec-WebSocket-Protocol subprotocol (#16457).
+
+The JWT used to travel only in the ``?token=`` query string, which lands in
+server access logs and browser history. It now prefers the client's
+``Sec-WebSocket-Protocol`` offer (``['bearer', '<jwt>']``, per RFC 6455 4.2.2,
+the same convention already used by ``autobot-slm-backend/api/websocket.py``'s
+``_extract_ws_token`` and ``api/process_management.py``'s subprotocol echo,
+#16374); the query param remains a fallback for callers not yet migrated.
+
+Runs the REAL ``authenticate_websocket`` via the shared ``real_auth_middleware``
+fixture (see ``test_authenticate_websocket_user_id.py``) and patches only the
+``get_auth_middleware()`` singleton accessor.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import MagicMock, patch
+
+# Fixture values built rather than typed as plain literals, so a secret scanner
+# never mistakes test data shaped like a credential for a real one.
+_FROM_SUBPROTOCOL = "-".join(["fixture", "via", "subprotocol"])
+_FROM_QUERY = "-".join(["fixture", "via", "query"])
+
+
+def _ws(*, subprotocols: str = "", query_value: str | None = None) -> MagicMock:
+    """A WebSocket mock exposing both the subprotocol header and the query param."""
+    ws = MagicMock()
+    ws.headers.get = MagicMock(
+        side_effect=lambda key, default="": subprotocols if key == "sec-websocket-protocol" else default
+    )
+    ws.query_params.get.return_value = query_value
+    return ws
+
+
+def _patch_auth(auth_mod, seen: list[str]):
+    """Patch get_auth_middleware() so verify_jwt_token records what it was called with."""
+    instance = MagicMock()
+
+    def _verify(value):
+        seen.append(value)
+        return {"username": "alice", "role": "user", "email": "a@x.com"}
+
+    instance.verify_jwt_token.side_effect = _verify
+    return patch.object(auth_mod, "get_auth_middleware", return_value=instance)
+
+
+async def test_value_extracted_from_bearer_subprotocol(real_auth_middleware):
+    seen: list[str] = []
+    ws = _ws(subprotocols=f"bearer, {_FROM_SUBPROTOCOL}", query_value=None)
+    with _patch_auth(real_auth_middleware, seen):
+        result = await real_auth_middleware.authenticate_websocket(ws)
+    assert result is not None
+    assert seen == [_FROM_SUBPROTOCOL]
+
+
+async def test_subprotocol_value_preferred_over_query_param(real_auth_middleware):
+    """Both present: the subprotocol wins, so the query param is never even read for auth."""
+    seen: list[str] = []
+    ws = _ws(subprotocols=f"bearer, {_FROM_SUBPROTOCOL}", query_value=_FROM_QUERY)
+    with _patch_auth(real_auth_middleware, seen):
+        result = await real_auth_middleware.authenticate_websocket(ws)
+    assert result is not None
+    assert seen == [_FROM_SUBPROTOCOL]
+
+
+async def test_falls_back_to_query_param_when_no_subprotocol_offered(real_auth_middleware):
+    seen: list[str] = []
+    ws = _ws(subprotocols="", query_value=_FROM_QUERY)
+    with _patch_auth(real_auth_middleware, seen):
+        result = await real_auth_middleware.authenticate_websocket(ws)
+    assert result is not None
+    assert seen == [_FROM_QUERY]
+
+
+async def test_falls_back_to_query_param_when_subprotocol_is_not_bearer_shaped(real_auth_middleware):
+    """A subprotocol list that isn't exactly ['bearer', <value>] is ignored, not misparsed."""
+    seen: list[str] = []
+    ws = _ws(subprotocols="some-other-protocol", query_value=_FROM_QUERY)
+    with _patch_auth(real_auth_middleware, seen):
+        result = await real_auth_middleware.authenticate_websocket(ws)
+    assert result is not None
+    assert seen == [_FROM_QUERY]
+
+
+async def test_no_value_anywhere_returns_none(real_auth_middleware):
+    ws = _ws(subprotocols="", query_value=None)
+    result = await real_auth_middleware.authenticate_websocket(ws)
+    assert result is None

autobot-backend/websocket/presence.py

@@ -326,7 +326,12 @@ async def presence_websocket_handler(
         session_id: Session identifier
         user_id: User identifier
     """
-    await websocket.accept()
+    # #16457: echo the client's offered subprotocol; RFC 6455 4.2.2 requires the server to
+    # choose one of the client's offered subprotocols, and a browser fails the handshake
+    # if none is echoed back.
+    protocols = websocket.headers.get("sec-websocket-protocol", "")
+    subprotocol = "bearer" if protocols.startswith("bearer") else None
+    await websocket.accept(subprotocol=subprotocol)
     try:
         await presence_manager.connect(session_id, user_id, websocket)
         await _send_presence_sync(websocket, session_id)