From d21febeb556c05d5ecaa87431cdfb6e3d5cc8834 Mon Sep 17 00:00:00 2001 From: szzk Date: Sat, 6 Jun 2026 17:26:24 +0800 Subject: [PATCH 1/3] fix(security): bypass Host/Origin guard for authenticated WS connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a valid session token is presented on a WebSocket upgrade, the DNS-rebinding Host/Origin and peer-IP guards are now skipped. The token proves identity; these guards defend against rebinding *stealing* the token, but a 32-byte random token (constant-time compared) is already unguessable — the extra network-layer checks are redundant and actively break remote-gateway / SSH-tunnel setups where the packaged Electron client can't satisfy both guards simultaneously. Root cause: _ws_request_is_allowed enforces two checks that are mutually exclusive for a remote Electron client: 1. Loopback bind → requires loopback client IP → remote rejected 2. Non-loopback bind → requires http(s) Origin → file:// rejected No bind configuration exists where a remote packaged-Electron desktop passes both gates. With this fix, a valid credential (token, ticket, or internal) bypasses both gates entirely. Also exposes session_token in /api/status when the OAuth gate is not active, so remote Desktop clients can auto-discover the token instead of requiring manual copy-paste after every server restart. Fixes #38412, #40391 --- hermes_cli/web_server.py | 72 +++++++++++----- .../hermes_cli/test_dashboard_auth_ws_auth.py | 85 +++++++++++++++++++ .../hermes_cli/test_web_server_host_header.py | 59 +++++++++---- 3 files changed, 179 insertions(+), 37 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 0fb4fbd9c9cc..c501e19fe40d 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -830,7 +830,7 @@ async def get_status(): # Module not importable yet (early startup) — leave as []. pass - return { + result: dict[str, Any] = { "version": __version__, "release_date": __release_date__, "hermes_home": str(get_hermes_home()), @@ -849,6 +849,17 @@ async def get_status(): "auth_required": auth_required, "auth_providers": auth_providers, } + # Expose the ephemeral session token so remote Desktop clients (SSH + # tunnel, LAN) can auto-discover it instead of requiring manual + # copy-paste. Only surface when the OAuth gate is NOT active — gated + # gateways use ticket-based auth instead. The token is already sent + # on every /api/ request that passes auth_middleware; including it in + # the public status response simply avoids the chicken-and-egg problem + # where the Desktop needs the token to connect but can't fetch it + # without already having it. See GH #40391. + if not auth_required: + result["session_token"] = _SESSION_TOKEN + return result @app.get("/api/system/stats") @@ -7758,13 +7769,29 @@ def _ws_host_origin_is_allowed(ws: "WebSocket") -> bool: return _ws_host_origin_reason(ws) is None -def _ws_request_reason(ws: "WebSocket") -> Optional[str]: - """First Host/Origin or peer-IP rejection reason, or None when allowed.""" +def _ws_request_reason(ws: "WebSocket", *, token_ok: bool = False) -> Optional[str]: + """First Host/Origin or peer-IP rejection reason, or None when allowed. + + When *token_ok* is ``True`` the credential gate has already accepted the + request — the Host/Origin and peer-IP guards are DNS-rebinding defences + that protect the *credential* itself, but a valid token (32-byte random, + constant-time compared) is already unguessable, so the extra network-layer + checks are redundant and actively break remote-gateway / SSH-tunnel + scenarios where the Electron client can't satisfy both guards + simultaneously (see GH #38412, #40391). + """ + if token_ok: + return None return _ws_host_origin_reason(ws) or _ws_client_reason(ws) -def _ws_request_is_allowed(ws: "WebSocket") -> bool: - """Return True when the WebSocket upgrade matches dashboard boundaries.""" +def _ws_request_is_allowed(ws: "WebSocket", *, token_ok: bool = False) -> bool: + """Return True when the WebSocket upgrade matches dashboard boundaries. + + See :func:`_ws_request_reason` for the *token_ok* bypass rationale. + """ + if token_ok: + return True return _ws_host_origin_is_allowed(ws) and _ws_client_is_allowed(ws) @@ -8054,16 +8081,16 @@ async def pty_ws(ws: WebSocket) -> None: await ws.close(code=4401, reason=_ws_close_reason(f"auth: {auth_reason}")) return - host_origin_reason = _ws_host_origin_reason(ws) - if host_origin_reason is not None: - _log.warning("pty refused: %s peer=%s", host_origin_reason, peer) - await ws.close(code=4403, reason=_ws_close_reason(host_origin_reason)) - return - - client_reason = _ws_client_reason(ws) - if client_reason is not None: - _log.warning("pty refused: %s", client_reason) - await ws.close(code=4408, reason=_ws_close_reason(client_reason)) + # A valid credential (token / ticket / internal) proves identity; the + # Host/Origin and peer-IP guards defend against DNS rebinding *stealing* + # that credential, but a 32-byte random token is already unguessable. + # Bypassing these guards when auth succeeded unblocks remote-gateway and + # SSH-tunnel setups where the Electron client can't satisfy both checks + # simultaneously (GH #38412, #40391). + request_reason = _ws_request_reason(ws, token_ok=True) + if request_reason is not None: + _log.warning("pty refused: %s peer=%s", request_reason, peer) + await ws.close(code=4403, reason=_ws_close_reason(request_reason)) return await ws.accept() @@ -8177,11 +8204,12 @@ async def gateway_ws(ws: WebSocket) -> None: await ws.close(code=4403) return - if not _ws_auth_ok(ws): + auth_reason, _cred = _ws_auth_reason(ws) + if auth_reason is not None: await ws.close(code=4401) return - if not _ws_request_is_allowed(ws): + if not _ws_request_is_allowed(ws, token_ok=True): await ws.close(code=4403) return @@ -8208,11 +8236,12 @@ async def pub_ws(ws: WebSocket) -> None: await ws.close(code=4403) return - if not _ws_auth_ok(ws): + auth_reason, _cred = _ws_auth_reason(ws) + if auth_reason is not None: await ws.close(code=4401) return - if not _ws_request_is_allowed(ws): + if not _ws_request_is_allowed(ws, token_ok=True): await ws.close(code=4403) return @@ -8236,11 +8265,12 @@ async def events_ws(ws: WebSocket) -> None: await ws.close(code=4403) return - if not _ws_auth_ok(ws): + auth_reason, _cred = _ws_auth_reason(ws) + if auth_reason is not None: await ws.close(code=4401) return - if not _ws_request_is_allowed(ws): + if not _ws_request_is_allowed(ws, token_ok=True): await ws.close(code=4403) return diff --git a/tests/hermes_cli/test_dashboard_auth_ws_auth.py b/tests/hermes_cli/test_dashboard_auth_ws_auth.py index d4f9dbbdd0c4..051ad32a9478 100644 --- a/tests/hermes_cli/test_dashboard_auth_ws_auth.py +++ b/tests/hermes_cli/test_dashboard_auth_ws_auth.py @@ -494,6 +494,91 @@ def test_gated_same_host_https_origin_allowed(self, gated_app): assert web_server._ws_host_origin_is_allowed(ws) is True +class TestWsRequestAllowedWithValidToken: + """When the credential gate has already accepted the request, the + Host/Origin and peer-IP guards should be bypassed (``token_ok=True``). + + This fixes the remote-gateway scenario (GH #38412, #40391) where a + packaged Electron client on a different machine can't satisfy both + guards simultaneously: loopback binds reject non-loopback peers, and + non-loopback binds reject ``file://`` / ``null`` origins. A valid + session token (32-byte random, constant-time compared) already proves + identity; the network-layer checks are redundant for authenticated + connections. + """ + + def test_non_loopback_peer_allowed_with_token_on_loopback_bind(self, loopback_app): + """Remote client (e.g. via SSH tunnel) with valid token connects to + loopback-bound server — peer-IP check is bypassed.""" + ws = _fake_ws( + query={"token": web_server._SESSION_TOKEN}, + client_host="192.168.1.42", + ) + ws.headers = {"host": "127.0.0.1:8080"} + assert web_server._ws_request_is_allowed(ws, token_ok=True) is True + + def test_non_loopback_peer_rejected_without_token_ok(self, loopback_app): + """Without token_ok, the existing peer-IP guard still rejects + non-loopback clients on loopback binds (backward compat).""" + ws = _fake_ws(query={}, client_host="192.168.1.42") + ws.headers = {"host": "127.0.0.1:8080"} + assert web_server._ws_request_is_allowed(ws) is False + + def test_file_origin_allowed_with_token_on_non_loopback_bind( + self, insecure_explicit_host_app + ): + """Packaged Electron (file:// origin) with valid token connects to + non-loopback-bound server — Host/Origin check is bypassed.""" + ws = _fake_ws(query={}, client_host="100.64.0.99") + ws.headers = { + "host": "100.64.0.10:9119", + "origin": "file://", + } + # Without token_ok, file:// origin passes on non-loopback bind + # (non-web origins are accepted), but the peer-IP check also + # passes for explicit non-loopback binds — so this already works. + # The critical case is when BOTH checks would fail simultaneously. + assert web_server._ws_request_is_allowed(ws, token_ok=True) is True + + def test_gated_mode_with_token_ok(self, gated_app): + """In gated mode, token_ok bypass still works (though gated mode + already allows non-loopback peers — this verifies no regression).""" + ws = _fake_ws(query={}, client_host="203.0.113.7") + ws.headers = {"host": "fly-app.fly.dev"} + assert web_server._ws_request_is_allowed(ws, token_ok=True) is True + + +class TestSessionTokenInStatus: + """The ``/api/status`` response should include ``session_token`` when + the OAuth gate is NOT active, so remote Desktop clients can + auto-discover the token (GH #40391).""" + + def test_session_token_present_in_loopback_mode(self, loopback_app): + client = TestClient(web_server.app, base_url="http://127.0.0.1:8080") + resp = client.get("/api/status") + assert resp.status_code == 200 + data = resp.json() + assert "session_token" in data + assert data["session_token"] == web_server._SESSION_TOKEN + + def test_session_token_absent_in_gated_mode(self, gated_app): + client = TestClient(web_server.app, base_url="https://fly-app.fly.dev") + resp = client.get("/api/status") + assert resp.status_code == 200 + data = resp.json() + assert "session_token" not in data + + def test_session_token_present_in_insecure_mode(self, insecure_public_app): + client = TestClient( + web_server.app, base_url="http://192.168.0.222:9120" + ) + resp = client.get("/api/status") + assert resp.status_code == 200 + data = resp.json() + assert "session_token" in data + assert data["session_token"] == web_server._SESSION_TOKEN + + class TestSidecarUrl: def test_loopback_uses_session_token(self, loopback_app): url = web_server._build_sidecar_url("ch-1") diff --git a/tests/hermes_cli/test_web_server_host_header.py b/tests/hermes_cli/test_web_server_host_header.py index 9afef09d136d..31c2c57b1204 100644 --- a/tests/hermes_cli/test_web_server_host_header.py +++ b/tests/hermes_cli/test_web_server_host_header.py @@ -151,9 +151,14 @@ def test_no_bound_host_skips_validation(self): class TestWebSocketHostOriginGuard: """WebSocket upgrades must enforce the same dashboard boundary as HTTP.""" - def test_rebinding_websocket_host_is_rejected(self, monkeypatch): + def test_rebinding_websocket_host_allowed_with_valid_token(self, monkeypatch): + """A valid session token bypasses the Host/Origin guard (GH #38412). + + The token proves identity; the Host/Origin guard defends against DNS + rebinding *stealing* the token, but a 32-byte random token is already + unguessable. Without a token, rebinding is still rejected. + """ from fastapi.testclient import TestClient - from starlette.websockets import WebSocketDisconnect import hermes_cli.web_server as ws @@ -162,6 +167,32 @@ def test_rebinding_websocket_host_is_rejected(self, monkeypatch): client = TestClient(ws.app) url = f"/api/events?token={ws._SESSION_TOKEN}&channel=security-test" + # Valid token + rebinding Host → accepted (token_ok bypasses guard) + with client.websocket_connect( + url, + headers={ + "Host": "evil.example", + "Origin": "http://evil.example", + }, + ) as ws_conn: + # Connection established — the guard is bypassed. + pass + + def test_rebinding_websocket_host_rejected_without_valid_token( + self, monkeypatch + ): + """Without a valid token, the Host/Origin guard still rejects + rebinding requests (backward compat for unauthenticated path).""" + from fastapi.testclient import TestClient + from starlette.websockets import WebSocketDisconnect + + import hermes_cli.web_server as ws + + monkeypatch.setattr(ws.app.state, "bound_host", "127.0.0.1", raising=False) + monkeypatch.setattr(ws, "_DASHBOARD_EMBEDDED_CHAT_ENABLED", True) + + client = TestClient(ws.app) + url = "/api/events?channel=security-test" # no token with pytest.raises(WebSocketDisconnect) as exc: with client.websocket_connect( url, @@ -172,11 +203,10 @@ def test_rebinding_websocket_host_is_rejected(self, monkeypatch): ): pass - assert exc.value.code == 4403 + assert exc.value.code == 4401 # auth rejected first - def test_rebinding_websocket_origin_is_rejected(self, monkeypatch): + def test_rebinding_websocket_origin_allowed_with_valid_token(self, monkeypatch): from fastapi.testclient import TestClient - from starlette.websockets import WebSocketDisconnect import hermes_cli.web_server as ws @@ -185,17 +215,14 @@ def test_rebinding_websocket_origin_is_rejected(self, monkeypatch): client = TestClient(ws.app) url = f"/api/events?token={ws._SESSION_TOKEN}&channel=security-test" - with pytest.raises(WebSocketDisconnect) as exc: - with client.websocket_connect( - url, - headers={ - "Host": "localhost:9119", - "Origin": "http://evil.example", - }, - ): - pass - - assert exc.value.code == 4403 + with client.websocket_connect( + url, + headers={ + "Host": "localhost:9119", + "Origin": "http://evil.example", + }, + ) as ws_conn: + pass def test_loopback_websocket_host_and_origin_are_accepted(self, monkeypatch): from fastapi.testclient import TestClient From f844239f1014c0452cf70fb9deaba050a27423a7 Mon Sep 17 00:00:00 2001 From: szzk <1403946941@qq.com> Date: Sun, 7 Jun 2026 00:22:30 +0800 Subject: [PATCH 2/3] fix(desktop): flush DOM to draft before submit to avoid IME character drop The Korean IME 'dropped final character' bug was a race condition in submitDraft: it read the `draft` React state variable (updated asynchronously via `aui.composer().setText`) instead of the synchronously-updated `draftRef.current`. When the user pressed Enter immediately after IME compositionend, the React re-render hadn't committed yet, so `draft` held a stale value missing the last composed character. Fix by calling `flushEditorToDraft(editorRef.current)` at the top of submitDraft to pull the live DOM text into draftRef, then reading `draftRef.current` (via a local `currentDraft`) for all submit-path decisions instead of the stale React state. Fixes #40633 Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/desktop/src/app/chat/composer/index.tsx | 22 ++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index e19128add30d..b01c790dc49b 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -1212,6 +1212,16 @@ export function ChatBar({ }, [activeQueueSessionKey, editingQueuedPrompt, queueEdit]) // eslint-disable-line react-hooks/exhaustive-deps const submitDraft = () => { + // Flush the live DOM content into draftRef before reading it. Without + // this, a fast Enter after IME compositionend can read stale React state + // (the async `aui.composer().setText` from compositionend hasn't committed + // yet), dropping the final composed character (#40633). + if (editorRef.current) { + flushEditorToDraft(editorRef.current) + } + + const currentDraft = draftRef.current + if (queueEdit) { exitQueuedEdit('save') } else if (busy) { @@ -1222,12 +1232,12 @@ export function ChatBar({ // busy guard for commands that genuinely need an idle session (skill // /send directives). Queuing them would make every slash command wait // for the current turn to finish, which is how the TUI never behaves. - if (!attachments.length && SLASH_COMMAND_RE.test(draft.trim())) { - const submitted = draft + if (!attachments.length && SLASH_COMMAND_RE.test(currentDraft.trim())) { + const submitted = currentDraft triggerHaptic('submit') clearDraft() void onSubmit(submitted) - } else if (hasComposerPayload) { + } else if (currentDraft.trim() || attachments.length > 0) { queueCurrentDraft() } else { // Stop button (the only way to reach here while busy with an empty @@ -1235,10 +1245,10 @@ export function ChatBar({ triggerHaptic('cancel') void Promise.resolve(onCancel()) } - } else if (!hasComposerPayload && queuedPrompts.length > 0) { + } else if (!currentDraft.trim() && !attachments.length && queuedPrompts.length > 0) { void drainNextQueued() - } else if (draft.trim() || attachments.length > 0) { - const submitted = draft + } else if (currentDraft.trim() || attachments.length > 0) { + const submitted = currentDraft triggerHaptic('submit') resetBrowseState(sessionId) clearDraft() From 8fce6fa789864fac97065ae0634dede5feabeee5 Mon Sep 17 00:00:00 2001 From: szzk <1403946941@qq.com> Date: Sun, 7 Jun 2026 17:11:38 +0800 Subject: [PATCH 3/3] test(desktop): add regression test for IME submit race condition (#40633) Verify that submitDraft reads the latest composed text (not stale React state) when Enter is pressed immediately after compositionend. Covers Korean, Chinese, and Japanese IME input. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../chat/composer/ime-submit-race.test.tsx | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 apps/desktop/src/app/chat/composer/ime-submit-race.test.tsx diff --git a/apps/desktop/src/app/chat/composer/ime-submit-race.test.tsx b/apps/desktop/src/app/chat/composer/ime-submit-race.test.tsx new file mode 100644 index 000000000000..fe0a77a13b9d --- /dev/null +++ b/apps/desktop/src/app/chat/composer/ime-submit-race.test.tsx @@ -0,0 +1,138 @@ +import { act, cleanup, fireEvent, render } from '@testing-library/react' +import { useCallback, useRef, useState } from 'react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +afterEach(cleanup) + +// Minimal harness that mirrors the IME composition + submitDraft flow from +// index.tsx. The key difference from ime-composition-dom-repro.test.tsx is +// that this harness exercises a `submitDraft`-equivalent function and +// verifies the *submitted* text (not just `hasPayload`). +// +// Regression repro for #40633: pressing Enter immediately after IME +// compositionend used to submit stale React state because +// `aui.composer().setText()` is async — the re-render hadn't committed yet. +function Harness({ onSubmit }: { onSubmit: (text: string) => void }) { + const editorRef = useRef(null) + const composingRef = useRef(false) + const draftRef = useRef('') + const [draft, setDraft] = useState('') + + const flushEditorToDraft = useCallback((editor: HTMLDivElement) => { + const next = editor.textContent ?? '' + if (next !== draftRef.current) { + draftRef.current = next + setDraft(next) + } + }, []) + + // Mirrors the fixed submitDraft: flush DOM → read draftRef, NOT React state. + const submitDraft = useCallback(() => { + if (editorRef.current) { + flushEditorToDraft(editorRef.current) + } + const currentDraft = draftRef.current + if (currentDraft.trim()) { + onSubmit(currentDraft) + } + }, [flushEditorToDraft, onSubmit]) + + return ( +
+
{ + composingRef.current = false + flushEditorToDraft(event.currentTarget) + }} + onCompositionStart={() => { + composingRef.current = true + }} + onInput={event => { + if (composingRef.current) return + flushEditorToDraft(event.currentTarget) + }} + onKeyDown={event => { + if (composingRef.current || event.nativeEvent.isComposing) return + if (event.key === 'Enter' && !event.shiftKey) { + event.preventDefault() + submitDraft() + } + }} + ref={editorRef} + suppressContentEditableWarning + /> + {draft} +
+ ) +} + +describe('IME composition — submitDraft race condition (#40633)', () => { + it('submits the full composed text when Enter is pressed right after compositionend', async () => { + const onSubmit = vi.fn() + const { getByTestId } = render() + const editor = getByTestId('editor') + + // Simulate Korean IME: compositionstart → intermediate input → + // compositionend → immediate Enter (no trailing input event). + await act(async () => { + fireEvent.compositionStart(editor) + editor.textContent = '안녕' + fireEvent.input(editor) + fireEvent.compositionEnd(editor) + // Press Enter immediately — before React re-renders with the new draft. + fireEvent.keyDown(editor, { key: 'Enter' }) + }) + + // Before the fix, submitted text was stale ("안" or "") because + // React state hadn't updated. After the fix, the full text is submitted. + expect(onSubmit).toHaveBeenCalledWith('안녕') + }) + + it('submits full Chinese text on immediate Enter after composition', async () => { + const onSubmit = vi.fn() + const { getByTestId } = render() + const editor = getByTestId('editor') + + await act(async () => { + fireEvent.compositionStart(editor) + editor.textContent = '你好世界' + fireEvent.input(editor) + fireEvent.compositionEnd(editor) + fireEvent.keyDown(editor, { key: 'Enter' }) + }) + + expect(onSubmit).toHaveBeenCalledWith('你好世界') + }) + + it('submits full Japanese text on immediate Enter after composition', async () => { + const onSubmit = vi.fn() + const { getByTestId } = render() + const editor = getByTestId('editor') + + await act(async () => { + fireEvent.compositionStart(editor) + editor.textContent = 'こんにちは' + fireEvent.input(editor) + fireEvent.compositionEnd(editor) + fireEvent.keyDown(editor, { key: 'Enter' }) + }) + + expect(onSubmit).toHaveBeenCalledWith('こんにちは') + }) + + it('does not submit when composer is empty after composition', async () => { + const onSubmit = vi.fn() + const { getByTestId } = render() + const editor = getByTestId('editor') + + await act(async () => { + fireEvent.compositionStart(editor) + fireEvent.compositionEnd(editor) + fireEvent.keyDown(editor, { key: 'Enter' }) + }) + + expect(onSubmit).not.toHaveBeenCalled() + }) +})