diff --git a/Client/tauri-client/src-tauri/Cargo.toml b/Client/tauri-client/src-tauri/Cargo.toml index f33837a0..c81d003b 100644 --- a/Client/tauri-client/src-tauri/Cargo.toml +++ b/Client/tauri-client/src-tauri/Cargo.toml @@ -13,7 +13,7 @@ tauri-build = { version = "2", features = [] } tauri-typegen = "0.5" [features] -default = ["devtools"] +default = [] devtools = ["tauri/devtools"] [dependencies] diff --git a/Client/tauri-client/src-tauri/capabilities/default.json b/Client/tauri-client/src-tauri/capabilities/default.json index 867fcccc..9f7831bf 100644 --- a/Client/tauri-client/src-tauri/capabilities/default.json +++ b/Client/tauri-client/src-tauri/capabilities/default.json @@ -28,7 +28,6 @@ "notification:allow-notify", "notification:allow-request-permission", "notification:allow-is-permission-granted", - "http:default", { "identifier": "http:allow-fetch", "allow": [ diff --git a/Client/tauri-client/src-tauri/src/ptt.rs b/Client/tauri-client/src-tauri/src/ptt.rs index 46c073b4..44283dfd 100644 --- a/Client/tauri-client/src-tauri/src/ptt.rs +++ b/Client/tauri-client/src-tauri/src/ptt.rs @@ -25,6 +25,41 @@ static PTT_SHUTDOWN: AtomicBool = AtomicBool::new(false); /// prevents duplicate thread spawns. static PTT_THREAD: Mutex>> = Mutex::new(None); +/// Returns true if a VK code is allowed for global capture in ptt_listen_for_key. +/// +/// Security hardening (BUG-136): only non-text keys are capturable to reduce +/// misuse potential if the renderer is compromised. +fn is_allowed_ptt_capture_vk(vk: i32) -> bool { + // Explicitly reject modifier keys. + if matches!(vk, 0x10 | 0x11 | 0x12 | 0x5B | 0x5C) { + return false; + } + + // Function keys F1-F24. + if (0x70..=0x87).contains(&vk) { + return true; + } + + // Non-text navigation/control keys. + matches!( + vk, + 0x1B | // Escape + 0x20 | // Space + 0x21 | // Page Up + 0x22 | // Page Down + 0x23 | // End + 0x24 | // Home + 0x25 | // Left + 0x26 | // Up + 0x27 | // Right + 0x28 | // Down + 0x2D | // Insert + 0x2E | // Delete + 0x05 | // Mouse X1 + 0x06 // Mouse X2 + ) +} + // --------------------------------------------------------------------------- // Platform-specific key detection // --------------------------------------------------------------------------- @@ -371,7 +406,7 @@ pub async fn ptt_listen_for_key() -> i32 { while std::time::Instant::now() < deadline { for key in device_state.get_keys() { let vk = linux::keycode_to_vk(&key); - if vk == 0 || linux::is_modifier_vk(vk) { + if vk == 0 || linux::is_modifier_vk(vk) || !is_allowed_ptt_capture_vk(vk) { continue; } // Wait for key release (with its own timeout) @@ -395,8 +430,8 @@ pub async fn ptt_listen_for_key() -> i32 { while std::time::Instant::now() < deadline { for vk in 1..=254i32 { - // Skip modifier keys - if matches!(vk, 0x10 | 0x11 | 0x12 | 0x5B | 0x5C) { + // Skip keys that are not explicitly allowed for capture. + if !is_allowed_ptt_capture_vk(vk) { continue; } if is_key_down(vk) { @@ -446,6 +481,28 @@ mod tests { assert!(ptt_set_key(300).is_err()); } + #[test] + fn allowed_capture_vk_accepts_safe_non_text_keys() { + assert!(is_allowed_ptt_capture_vk(0x70)); // F1 + assert!(is_allowed_ptt_capture_vk(0x7B)); // F12 + assert!(is_allowed_ptt_capture_vk(0x25)); // Left arrow + assert!(is_allowed_ptt_capture_vk(0x2E)); // Delete + assert!(is_allowed_ptt_capture_vk(0x05)); // Mouse X1 + assert!(is_allowed_ptt_capture_vk(0x06)); // Mouse X2 + } + + #[test] + fn allowed_capture_vk_rejects_text_and_modifier_keys() { + assert!(!is_allowed_ptt_capture_vk(0x41)); // A + assert!(!is_allowed_ptt_capture_vk(0x31)); // 1 + assert!(!is_allowed_ptt_capture_vk(0x0D)); // Enter + assert!(!is_allowed_ptt_capture_vk(0x08)); // Backspace + assert!(!is_allowed_ptt_capture_vk(0x10)); // Shift + assert!(!is_allowed_ptt_capture_vk(0x11)); // Ctrl + assert!(!is_allowed_ptt_capture_vk(0x12)); // Alt + assert!(!is_allowed_ptt_capture_vk(0x5B)); // Meta + } + #[test] fn ptt_get_key_reflects_set_key() { ptt_set_key(0x41).unwrap(); diff --git a/Client/tauri-client/src-tauri/tauri.conf.json b/Client/tauri-client/src-tauri/tauri.conf.json index f8d679e5..b1a62e77 100644 --- a/Client/tauri-client/src-tauri/tauri.conf.json +++ b/Client/tauri-client/src-tauri/tauri.conf.json @@ -22,7 +22,7 @@ "additionalBrowserArgs": "--autoplay-policy=no-user-gesture-required --use-fake-ui-for-media-stream" } ], - "withGlobalTauri": true, + "withGlobalTauri": false, "security": { "csp": "default-src 'self'; script-src 'self' 'wasm-unsafe-eval'; style-src 'self' 'unsafe-inline'; connect-src 'self' http://ipc.localhost https: wss: http://localhost:* ws://localhost:* http://127.0.0.1:* ws://127.0.0.1:*; img-src 'self' https: data:; media-src 'self' blob:; font-src 'self'; object-src 'none'; base-uri 'self'; frame-src https://www.youtube.com https://youtube.com" } diff --git a/Client/tauri-client/src/components/settings/AdvancedTab.ts b/Client/tauri-client/src/components/settings/AdvancedTab.ts index 6bded0eb..3cb0d496 100644 --- a/Client/tauri-client/src/components/settings/AdvancedTab.ts +++ b/Client/tauri-client/src/components/settings/AdvancedTab.ts @@ -66,32 +66,34 @@ export function buildAdvancedTab(signal: AbortSignal): HTMLDivElement { const debugTitle = createElement("div", { class: "settings-section-title" }, "Debug"); section.appendChild(debugTitle); - // DevTools button row - const devtoolsRow = createElement("div", { class: "setting-row" }); - const devtoolsInfo = createElement("div", {}); - const devtoolsLabel = createElement("div", { class: "setting-label" }, "Open DevTools"); - const devtoolsDesc = createElement( - "div", - { class: "setting-desc" }, - "Open the browser developer tools for debugging", - ); - appendChildren(devtoolsInfo, devtoolsLabel, devtoolsDesc); + if (import.meta.env.DEV) { + // DevTools button row + const devtoolsRow = createElement("div", { class: "setting-row" }); + const devtoolsInfo = createElement("div", {}); + const devtoolsLabel = createElement("div", { class: "setting-label" }, "Open DevTools"); + const devtoolsDesc = createElement( + "div", + { class: "setting-desc" }, + "Open the browser developer tools for debugging", + ); + appendChildren(devtoolsInfo, devtoolsLabel, devtoolsDesc); - const devtoolsBtn = createElement("button", { class: "ac-btn" }, "Open DevTools"); - devtoolsBtn.addEventListener( - "click", - () => { - void invoke("open_devtools").catch((err: unknown) => { - log.warn("DevTools not available", { - error: err instanceof Error ? err.message : String(err), + const devtoolsBtn = createElement("button", { class: "ac-btn" }, "Open DevTools"); + devtoolsBtn.addEventListener( + "click", + () => { + void invoke("open_devtools").catch((err: unknown) => { + log.warn("DevTools not available", { + error: err instanceof Error ? err.message : String(err), + }); }); - }); - }, - { signal }, - ); + }, + { signal }, + ); - appendChildren(devtoolsRow, devtoolsInfo, devtoolsBtn); - section.appendChild(devtoolsRow); + appendChildren(devtoolsRow, devtoolsInfo, devtoolsBtn); + section.appendChild(devtoolsRow); + } // ---- Storage & Cache section ------------------------------------------------ diff --git a/Client/tauri-client/src/components/settings/KeybindsTab.ts b/Client/tauri-client/src/components/settings/KeybindsTab.ts index cd8872b4..213b5930 100644 --- a/Client/tauri-client/src/components/settings/KeybindsTab.ts +++ b/Client/tauri-client/src/components/settings/KeybindsTab.ts @@ -40,7 +40,7 @@ export function buildKeybindsTab(signal: AbortSignal): HTMLDivElement { () => { if (capturing) return; capturing = true; - pttValue.textContent = "Press any key..."; + pttValue.textContent = "Press a supported key..."; pttValue.style.borderColor = "var(--accent)"; pttValue.style.color = "var(--accent)"; @@ -93,7 +93,7 @@ export function buildKeybindsTab(signal: AbortSignal): HTMLDivElement { { style: "font-size: 11px; color: var(--text-micro); margin: 4px 0 16px 0; line-height: 1.4;", }, - "PTT works globally and does not hijack the key \u2014 you can still type and use other apps normally. Mouse buttons (Mouse 4/5) also work.", + "PTT works globally and does not hijack the key. Capture supports function keys, navigation keys, and Mouse 4/5.", ); section.appendChild(pttHint); diff --git a/Client/tauri-client/src/lib/livekitSession.ts b/Client/tauri-client/src/lib/livekitSession.ts index a60b7fc2..46055459 100644 --- a/Client/tauri-client/src/lib/livekitSession.ts +++ b/Client/tauri-client/src/lib/livekitSession.ts @@ -361,6 +361,18 @@ export class LiveKitSession { } try { const newRoom = this.createRoom(); + const cleanupAbortedReconnect = async (): Promise => { + newRoom.removeAllListeners(); + try { + await newRoom.disconnect(); + } catch (disconnectErr) { + log.warn("Failed to disconnect room after reconnect abort", disconnectErr); + } + this._audioPipeline.setRoom(null); + this._audioElements.setRoom(null); + this._deviceManager.setRoom(null); + this._deviceManager.setAudioPipeline(null); + }; // Set state to reconnecting with the fresh room-less attempt info; // the actual room appears in "connected" state after connect succeeds. if (this._state.type === "reconnecting") { @@ -370,10 +382,31 @@ export class LiveKitSession { this._audioElements.setRoom(newRoom); this._deviceManager.setRoom(newRoom); this._deviceManager.setAudioPipeline(this._audioPipeline); + + if (signal.aborted || this._currentChannelId !== channelId) { + log.info("Auto-reconnect aborted after room creation"); + await cleanupAbortedReconnect(); + return; + } + // eslint-disable-next-line no-await-in-loop -- sequential reconnect: resolve URL then connect const resolvedUrl = await this.resolveLiveKitUrl(url, directUrl); + + if (signal.aborted || this._currentChannelId !== channelId) { + log.info("Auto-reconnect aborted before room connect"); + await cleanupAbortedReconnect(); + return; + } + // eslint-disable-next-line no-await-in-loop -- sequential reconnect: must connect before restoring state await newRoom.connect(resolvedUrl, token); + + if (signal.aborted || this._currentChannelId !== channelId) { + log.info("Auto-reconnect aborted after room connect"); + await cleanupAbortedReconnect(); + return; + } + log.info("Auto-reconnect succeeded", { attempt, channelId, url: resolvedUrl }); // Transition to "connected" — this is the single atomic write. this.setState({ diff --git a/Client/tauri-client/src/lib/ws.ts b/Client/tauri-client/src/lib/ws.ts index 324b0220..a2e45348 100644 --- a/Client/tauri-client/src/lib/ws.ts +++ b/Client/tauri-client/src/lib/ws.ts @@ -157,7 +157,13 @@ export function createWsClient() { setState("reconnecting"); reconnectTimer = setTimeout(() => { reconnectAttempt++; - void connect(config!); + const nextConfig = config; + if (!nextConfig) { + log.warn("Reconnect aborted: missing config"); + setState("disconnected"); + return; + } + void connect(nextConfig); }, delay); } @@ -463,6 +469,7 @@ export function createWsClient() { cleanupEventListeners(); void disconnectProxy(); setState("disconnected"); + config = null; // Reset lastSeq — disconnect() is only called for intentional close // (logout). Automatic reconnects go through scheduleReconnect() which // preserves lastSeq for server-side event replay. diff --git a/Client/tauri-client/src/main.ts b/Client/tauri-client/src/main.ts index f94fb626..0a527a13 100644 --- a/Client/tauri-client/src/main.ts +++ b/Client/tauri-client/src/main.ts @@ -39,7 +39,7 @@ document.addEventListener("contextmenu", (e) => { e.preventDefault(); }); -// F12 or Ctrl+Shift+I opens WebView2 DevTools. +// F12 or Ctrl+Shift+I opens WebView2 DevTools in development builds only. // F5 and Ctrl+R are blocked to prevent accidental page reloads which cause // ghost voice state (user appears in channel with no LiveKit connection). document.addEventListener("keydown", (e) => { @@ -47,7 +47,7 @@ document.addEventListener("keydown", (e) => { e.preventDefault(); return; } - if (e.key === "F12" || (e.ctrlKey && e.shiftKey && e.key === "I")) { + if (import.meta.env.DEV && (e.key === "F12" || (e.ctrlKey && e.shiftKey && e.key === "I"))) { e.preventDefault(); void import("@tauri-apps/api/core").then(({ invoke }) => { void invoke("open_devtools"); diff --git a/Client/tauri-client/tests/unit/livekit-session.test.ts b/Client/tauri-client/tests/unit/livekit-session.test.ts index 31078afe..649e948e 100644 --- a/Client/tauri-client/tests/unit/livekit-session.test.ts +++ b/Client/tauri-client/tests/unit/livekit-session.test.ts @@ -1789,6 +1789,37 @@ describe("LiveKitSession", () => { expect(leaveVoiceChannel).toHaveBeenCalled(); }); + + it("cleans up reconnect room when signal aborts after connect resolves (BUG-070)", async () => { + (session as any)._state = { + type: "reconnecting", + channelId: 5, + latestToken: "token", + lastUrl: "/livekit", + lastDirectUrl: "ws://localhost:7880", + ac: new AbortController(), + }; + session.setServerHost("localhost:7880"); + const ac = new AbortController(); + + mockRoom.connect.mockImplementationOnce(async () => { + ac.abort(); + }); + + const reconnectPromise = (session as any).attemptAutoReconnect( + "token", + "/livekit", + 5, + "ws://localhost:7880", + ac.signal, + ); + + await vi.advanceTimersByTimeAsync(3100); + await reconnectPromise; + + expect(mockRoom.disconnect).toHaveBeenCalled(); + expect((session as any)._state.type).not.toBe("connected"); + }); }); describe("token refresh timer", () => { diff --git a/Client/tauri-client/tests/unit/ws.test.ts b/Client/tauri-client/tests/unit/ws.test.ts index b7b3de69..6f686c93 100644 --- a/Client/tauri-client/tests/unit/ws.test.ts +++ b/Client/tauri-client/tests/unit/ws.test.ts @@ -1779,6 +1779,36 @@ describe("scheduleReconnect guard clauses", () => { const reconnects = mockInvoke.mock.calls.filter((c) => c[0] === "ws_connect"); expect(reconnects).toHaveLength(0); }); + + it("reconnect timer callback bails out safely when config is cleared", async () => { + client.connect({ host: "localhost:8443", token: "t" }); + await vi.advanceTimersByTimeAsync(10); + emitTauriEvent("ws-state", "open"); + + emitTauriEvent( + "ws-message", + JSON.stringify({ + type: "auth_ok", + payload: { + user: { id: 1, username: "a", avatar: null, role: "admin" }, + server_name: "S", + motd: "", + }, + }), + ); + + // Unexpected close schedules reconnect. + emitTauriEvent("ws-state", "closed"); + + // Simulate config being cleared before timer callback executes. + client.disconnect(); + mockInvoke.mockClear(); + + await vi.advanceTimersByTimeAsync(2_000); + const reconnects = mockInvoke.mock.calls.filter((c) => c[0] === "ws_connect"); + expect(reconnects).toHaveLength(0); + expect(client.getState()).toBe("disconnected"); + }); }); describe("cert-tofu non-mismatch statuses", () => { diff --git a/Server/ws/dm_handlers_test.go b/Server/ws/dm_handlers_test.go index 58f71c6c..c9204855 100644 --- a/Server/ws/dm_handlers_test.go +++ b/Server/ws/dm_handlers_test.go @@ -176,6 +176,45 @@ func TestDM_ChatSend_ParticipantSuccess(t *testing.T) { } } +func TestDM_ChatSend_SequencedAndReplayBuffered(t *testing.T) { + hub, database := newHandlerHub(t) + alice := seedOwnerUser(t, database, "dm-seq-alice") + bob := seedMemberUser(t, database, "dm-seq-bob") + dmChID := seedDMChannel(t, database, alice.ID, bob.ID) + + sendAlice := make(chan []byte, 128) + sendBob := make(chan []byte, 128) + cAlice := ws.NewTestClientWithUser(hub, alice, dmChID, sendAlice) + cBob := ws.NewTestClientWithUser(hub, bob, dmChID, sendBob) + hub.Register(cAlice) + hub.Register(cBob) + time.Sleep(20 * time.Millisecond) + + hub.HandleMessageForTest(cAlice, dmChatSendMsg(dmChID, "m1")) + hub.HandleMessageForTest(cAlice, dmChatSendMsg(dmChID, "m2")) + hub.HandleMessageForTest(cAlice, dmChatSendMsg(dmChID, "m3")) + time.Sleep(120 * time.Millisecond) + + bobMsgs := dmDrainAll(sendBob) + chat := dmFindMsgType(bobMsgs, "chat_message") + if chat == nil { + t.Fatal("Bob did not receive any DM chat_message") + } + if _, ok := chat["seq"]; !ok { + t.Fatal("DM chat_message is missing seq") + } + + oldest := hub.ReplayBuffer().OldestSeq() + if oldest == 0 { + t.Fatal("replay buffer did not record DM events (oldest seq is 0)") + } + + replayed := hub.ReplayBuffer().EventsSinceFiltered(oldest+1, map[int64]bool{dmChID: true}) + if len(replayed) == 0 { + t.Fatal("expected DM replay events after oldest+1, got none") + } +} + func TestDM_ChatSend_NonParticipantForbidden(t *testing.T) { hub, database := newHandlerHub(t) alice := seedOwnerUser(t, database, "dm-forbid-alice") diff --git a/Server/ws/handlers.go b/Server/ws/handlers.go index 26260407..961d5e1a 100644 --- a/Server/ws/handlers.go +++ b/Server/ws/handlers.go @@ -157,18 +157,15 @@ func (h *Hub) broadcastExclude(channelID, excludeUserID int64, msg []byte) { } // broadcastToDMParticipants sends a message to all participants of a DM channel -// using SendToUser for each participant. This bypasses the channel-subscription -// model used by BroadcastToChannel, which is correct for DMs since users may -// not be "focused" on the DM channel. +// while preserving DM semantics (delivery is by participant, not channel focus). +// Unlike broadcastToDMParticipantsExclude, this path is sequenced and replayable. func (h *Hub) broadcastToDMParticipants(channelID int64, msg []byte) { participantIDs, err := h.db.GetDMParticipantIDs(channelID) if err != nil { slog.Error("broadcastToDMParticipants GetDMParticipantIDs", "err", err, "channel_id", channelID) return } - for _, pid := range participantIDs { - h.SendToUser(pid, msg) - } + h.sendSequencedToUsers(channelID, participantIDs, msg) } // broadcastToDMParticipantsExclude sends a message to all participants of a DM diff --git a/Server/ws/handlers_chat.go b/Server/ws/handlers_chat.go index 0271941d..06cda90e 100644 --- a/Server/ws/handlers_chat.go +++ b/Server/ws/handlers_chat.go @@ -209,10 +209,6 @@ func (h *Hub) broadcastChatMessage(c *Client, channelID int64, isDM bool, broadc return } - for _, pid := range participantIDs { - h.SendToUser(pid, broadcast) - } - for _, pid := range participantIDs { if pid == c.userID { continue @@ -226,6 +222,8 @@ func (h *Hub) broadcastChatMessage(c *Client, channelID int64, isDM bool, broadc h.SendToUser(pid, buildDMChannelOpen(channelID, c.user)) } } + + h.sendSequencedToUsers(channelID, participantIDs, broadcast) } // handleChatEdit processes a chat_edit message. diff --git a/Server/ws/hub.go b/Server/ws/hub.go index 6712b4db..135e41ef 100644 --- a/Server/ws/hub.go +++ b/Server/ws/hub.go @@ -41,6 +41,7 @@ type Hub struct { permChecker *permissions.Checker seq uint64 // atomic monotonic sequence counter + seqMu syncutil.Mutex // serializes seq assignment + replay insertion + delivery order replayBuf *EventRingBuffer // recent broadcast events for reconnection replay broadcastDrops atomic.Uint64 // counts messages dropped due to full broadcast channel @@ -439,6 +440,24 @@ func (h *Hub) SendToUser(userID int64, msg []byte) bool { return c.trySendMsg(msg) } +// sendSequencedToUsers stamps msg with a monotonic seq, stores it in the replay +// buffer under channelID, and fanouts the wrapped payload to the provided users. +func (h *Hub) sendSequencedToUsers(channelID int64, userIDs []int64, msg []byte) { + h.seqMu.Lock() + defer h.seqMu.Unlock() + + seq := h.nextSeq() + wrapped := wrapWithSeq(msg, seq) + + // Store DM event for reconnect replay; filtering is channel-based and uses + // allowed channel IDs computed at auth time (including open DMs). + h.replayBuf.Push(seq, channelID, wrapped) + + for _, userID := range userIDs { + h.SendToUser(userID, wrapped) + } +} + // ClientCount returns the number of currently registered clients (test helper). func (h *Hub) ClientCount() int { h.mu.RLock() @@ -623,6 +642,9 @@ func (h *Hub) sweepStaleVoiceStates() { // deliverBroadcast stamps bm.msg with a monotonic sequence number, stores it // in the replay buffer, and sends it to the appropriate clients. func (h *Hub) deliverBroadcast(bm broadcastMsg) { + h.seqMu.Lock() + defer h.seqMu.Unlock() + seq := h.nextSeq() msg := wrapWithSeq(bm.msg, seq) diff --git a/Server/ws/serve.go b/Server/ws/serve.go index 10c2033d..205b4e9d 100644 --- a/Server/ws/serve.go +++ b/Server/ws/serve.go @@ -287,6 +287,9 @@ func (h *Hub) handleFreshConnect( slog.Error("buildReady failed", "user_id", c.userID, "err", readyErr) _ = conn.Write(ctx, websocket.MessageText, buildErrorMsg(ErrCodeInternal, "failed to build ready payload")) + h.unregisterNow(c) + _ = conn.Close(websocket.StatusInternalError, "failed to build ready payload") + return readyErr } if updateErr := database.UpdateUserStatus(c.userID, "online"); updateErr != nil {