diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1a4b39c1..764cd408 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -152,7 +152,7 @@ jobs: - name: TypeScript check (Playwright specs) # The main tsconfig excludes tests/e2e from the app graph; this - # project typechecks the 47 spec files + fixtures + the three + # project typechecks every tests/e2e spec + fixtures + the # playwright configs so type rot cannot hide there. run: npx tsc -p tsconfig.e2e.json --noEmit diff --git a/Client/tauri-client/src/lib/dispatcher.ts b/Client/tauri-client/src/lib/dispatcher.ts index 8dc64ab5..b39d9bd0 100644 --- a/Client/tauri-client/src/lib/dispatcher.ts +++ b/Client/tauri-client/src/lib/dispatcher.ts @@ -567,17 +567,18 @@ export function wireDispatcher( const isOwnMessage = currentUserId !== null && payload.user.id === currentUserId; // Increment channel-level unread for non-active, non-own-message channels. - // Skip during reconnection replay to avoid inflating counts — the - // server's ready payload already contains accurate unread_count values. - // DM channel IDs are not in channelsStore (they use dmStore), so - // incrementUnread is a no-op for DMs, but the own-message guard is + // Replayed frames increment unread counts like live ones — the burst + // is exactly the messages missed while away (a full-ready resume sends + // no burst at all; ready's unread_count values are authoritative + // there). DM channel IDs are not in channelsStore (they use dmStore), + // so incrementUnread is a no-op for DMs, but the own-message guard is // applied here for defence-in-depth. const isMention = highlightsCurrentUser(payload.content, { mentions: payload.mentions, mentionsEveryone: payload.mentions_everyone, }); - if (payload.channel_id !== activeId && !isOwnMessage && !ws.isReplaying()) { + if (payload.channel_id !== activeId && !isOwnMessage) { incrementUnread(payload.channel_id); // A mention is an unread too — the mention badge just outranks it. if (isMention) { @@ -586,10 +587,10 @@ export function wireDispatcher( } // Update DM store last message if this message belongs to a DM channel. - // Skip unread increment for own messages, currently focused DM, and replay. + // Skip unread increment for own messages and the currently focused DM. if (isDm) { const isDmActive = payload.channel_id === activeId; - if (isOwnMessage || isDmActive || ws.isReplaying()) { + if (isOwnMessage || isDmActive) { // Update last message preview but don't increment unread count. updateDmLastMessagePreview( payload.channel_id, @@ -609,13 +610,13 @@ export function wireDispatcher( } // Fire desktop notification, taskbar flash, and sound — but not for a - // reconnect's replayed burst. ws.isReplaying() cannot gate this the way - // it gates the unread counter above: ws.ts clears it as soon as auth_ok - // is processed, before the replay burst itself even arrives. A replay - // frame's timestamp instead predates the reconnect handshake that - // preceded it, unlike a genuinely new live message — compared in - // server-clock terms (see serverClockSkewMs above) so a lagging or - // skewed server clock cannot make a live message look like a replay. + // reconnect's replayed burst. No connection-state flag can gate this: + // the server writes auth_ok before the burst, so by the time replayed + // frames arrive the client is already "connected". A replay frame's + // timestamp instead predates the reconnect handshake that preceded it, + // unlike a genuinely new live message — compared in server-clock terms + // (see serverClockSkewMs above) so a lagging or skewed server clock + // cannot make a live message look like a replay. // The wall-clock window additionally bounds a cold (never-sampled) // skew's damage — see REPLAY_GATE_WINDOW_MS. const isReplayFrame = diff --git a/Client/tauri-client/src/lib/ws.ts b/Client/tauri-client/src/lib/ws.ts index 2312b21f..c90fb4eb 100644 --- a/Client/tauri-client/src/lib/ws.ts +++ b/Client/tauri-client/src/lib/ws.ts @@ -159,11 +159,6 @@ export function createWsClient() { let proxyOpen = false; let lastSeq = 0; - // Deduplication cache for reconnection replay. - // Active when reconnecting (reconnectAttempt > 0) until auth_ok. - let replayDedup: Set | null = null; - const MAX_DEDUP_SIZE = 1000; - // Tauri event unsubscribe functions const eventUnsubs: Array<() => void> = []; @@ -302,28 +297,6 @@ export function createWsClient() { log.debug("WS ←", { type: msg.type, id: msg.id }); - // Deduplication during reconnection replay - if ( - replayDedup !== null && - msg.type !== "auth_ok" && - msg.type !== "auth_error" && - msg.type !== "ready" - ) { - const dedupKey = msg.id ?? `${msg.type}:${seq}`; - if (replayDedup.has(dedupKey)) { - log.debug("Dedup: skipping duplicate message", { type: msg.type, key: dedupKey }); - return; - } - replayDedup.add(dedupKey); - if (replayDedup.size > MAX_DEDUP_SIZE) { - const targetSize = Math.floor(MAX_DEDUP_SIZE * 0.8); - for (const key of replayDedup) { - if (replayDedup.size <= targetSize) break; - replayDedup.delete(key); - } - } - } - // auth_error — non-recoverable if (msg.type === "auth_error") { log.error("Authentication failed", { message: msg.payload.message }); @@ -354,8 +327,6 @@ export function createWsClient() { if (msg.payload.replay_source === "none") { lastSeq = 0; } - // Clear dedup cache — replay is complete - replayDedup = null; setState("connected"); reconnectAttempt = 0; startHeartbeat(); @@ -459,10 +430,6 @@ export function createWsClient() { isReconnect: reconnectAttempt > 0, lastSeq, }); - // Enable dedup during reconnection replay - if (reconnectAttempt > 0 && lastSeq > 0) { - replayDedup = new Set(); - } setState("authenticating"); if (config === null) return; // active_channel_id only matters on a resume (last_seq > 0); on a @@ -783,11 +750,6 @@ export function createWsClient() { return state; }, - /** True while processing reconnection replay messages (dedup active). */ - isReplaying(): boolean { - return replayDedup !== null; - }, - /** @internal for testing */ _getWs(): WebSocket | null { return null; diff --git a/Client/tauri-client/tests/helpers/mock-ws.ts b/Client/tauri-client/tests/helpers/mock-ws.ts index abb7ce0f..8226d92d 100644 --- a/Client/tauri-client/tests/helpers/mock-ws.ts +++ b/Client/tauri-client/tests/helpers/mock-ws.ts @@ -98,10 +98,6 @@ export function createMockWsClient() { return state; }, - isReplaying(): boolean { - return false; - }, - // --------------------------------------------------------------- // Test-only helpers // --------------------------------------------------------------- diff --git a/Client/tauri-client/tests/integration/stores.test.ts b/Client/tauri-client/tests/integration/stores.test.ts index e8ae8217..078e11e4 100644 --- a/Client/tauri-client/tests/integration/stores.test.ts +++ b/Client/tauri-client/tests/integration/stores.test.ts @@ -85,10 +85,6 @@ function createMockWsClient(): MockWsClient { return currentState; }, - isReplaying() { - return false; - }, - _getWs() { return null; }, diff --git a/Client/tauri-client/tests/unit/dispatcher.test.ts b/Client/tauri-client/tests/unit/dispatcher.test.ts index ee9bc64f..354e31d9 100644 --- a/Client/tauri-client/tests/unit/dispatcher.test.ts +++ b/Client/tauri-client/tests/unit/dispatcher.test.ts @@ -128,7 +128,6 @@ function createMockWs() { onCertMismatch: vi.fn(() => () => {}), acceptCertFingerprint: vi.fn(async () => {}), getState: vi.fn(() => "disconnected" as const), - isReplaying: vi.fn(() => false), _getWs: vi.fn(() => null), }; @@ -378,11 +377,11 @@ describe("WS Dispatcher", () => { }); describe("chat_message notifications during a reconnect replay burst", () => { - // ws.ts clears isReplaying() as soon as auth_ok is processed — before the - // replay burst of chat_message frames the server sends right after it - // even arrives — so it cannot gate notifications the way it gates the - // unread counter above. A second auth_ok in this dispatcher's lifetime is - // always a reconnect handshake; its timestamp is the gate instead. + // The server writes auth_ok before the replay burst, so by the time + // replayed chat_message frames arrive the client is already "connected" + // — no connection-state flag can distinguish them. A second auth_ok in + // this dispatcher's lifetime is always a reconnect handshake; its + // timestamp is the gate instead. beforeEach(() => { vi.mocked(mockNotifyIncomingMessage).mockClear(); }); @@ -3052,45 +3051,6 @@ describe("WS Dispatcher", () => { expect(channelsStore.getState().channels.get(5)?.unreadCount).toBe(0); }); - it("does not increment unread during replay", () => { - (mock.ws.isReplaying as ReturnType).mockReturnValue(true); - - channelsStore.setState((prev) => { - const ch = new Map(prev.channels); - ch.set(5, { - id: 5, - name: "other-ch", - type: "text" as const, - category: null, - position: 0, - unreadCount: 0, - mentionCount: 0, - lastMessageId: null, - canSend: true, - topic: "", - slowMode: 0, - nsfw: false, - voiceMaxUsers: 0, - voiceMaxVideo: 0, - }); - return { ...prev, channels: ch, activeChannelId: 1 }; - }); - - mock.dispatch("chat_message", { - id: 300, - channel_id: 5, - user: { id: 2, username: "bob", avatar: null }, - content: "replayed message", - reply_to: null, - attachments: [], - timestamp: "2026-03-15T10:00:00Z", - }); - - expect(channelsStore.getState().channels.get(5)?.unreadCount).toBe(0); - - (mock.ws.isReplaying as ReturnType).mockReturnValue(false); - }); - describe("chat_message DM store updates", () => { const dmChannel = { channelId: 50, @@ -3178,33 +3138,6 @@ describe("WS Dispatcher", () => { expect(dm?.unreadCount).toBe(0); }); - it("updates DM preview (no unread) during replay", () => { - (mock.ws.isReplaying as ReturnType).mockReturnValue(true); - - channelsStore.setState((prev) => ({ ...prev, activeChannelId: 1 })); - authStore.setState((prev) => ({ - ...prev, - user: { id: 5, username: "me", avatar: null, role: "member" }, - })); - - mock.dispatch("chat_message", { - id: 503, - channel_id: 50, - user: { id: 10, username: "bob", avatar: "" }, - content: "replayed DM", - reply_to: null, - attachments: [], - timestamp: "2026-03-15T10:00:00Z", - }); - - const dms = dmStore.getState().channels; - const dm = dms.find((c) => c.channelId === 50); - expect(dm?.lastMessage).toBe("replayed DM"); - expect(dm?.unreadCount).toBe(0); - - (mock.ws.isReplaying as ReturnType).mockReturnValue(false); - }); - it("increments the DM mention badge for an incoming @mention", () => { channelsStore.setState((prev) => ({ ...prev, activeChannelId: 1 })); authStore.setState((prev) => ({ diff --git a/Client/tauri-client/tests/unit/main-page.test.ts b/Client/tauri-client/tests/unit/main-page.test.ts index c08667a8..5716833d 100644 --- a/Client/tauri-client/tests/unit/main-page.test.ts +++ b/Client/tauri-client/tests/unit/main-page.test.ts @@ -228,7 +228,6 @@ function fakeWs(): FakeWsClient { startCertListener: vi.fn(async () => {}), acceptCertFingerprint: vi.fn(async () => {}), getState: vi.fn(() => "connected" as ConnectionState), - isReplaying: vi.fn(() => false), _getWs: vi.fn(() => null), }; } diff --git a/Client/tauri-client/tests/unit/message-list.test.ts b/Client/tauri-client/tests/unit/message-list.test.ts index dc43f408..a72781cc 100644 --- a/Client/tauri-client/tests/unit/message-list.test.ts +++ b/Client/tauri-client/tests/unit/message-list.test.ts @@ -730,30 +730,37 @@ describe("MessageList", () => { vi.useRealTimers(); }); - it("does not report success when renderWindow's own >30-in-2s breaker drops the rebuild", () => { - const many = Array.from({ length: 100 }, (_, i) => makeMessage({ id: i + 1 })); - setMessages(1, many); - msgList.mount(container); // mount's renderAll -> renderWindow: rebuild count = 1 + // 30 synchronous full renderWindow rebuilds of a 100-row list can exceed + // vitest's default 5s on a loaded CI runner (audit-2026-08-19 F-5), so + // this test carries its own timeout. + it( + "does not report success when renderWindow's own >30-in-2s breaker drops the rebuild", + { timeout: 20_000 }, + () => { + const many = Array.from({ length: 100 }, (_, i) => makeMessage({ id: i + 1 })); + setMessages(1, many); + msgList.mount(container); // mount's renderAll -> renderWindow: rebuild count = 1 - // Every scrollToMessage call forces renderedStart = -1 and calls - // renderWindow() directly, bypassing renderAll's own (lower) rapid-fire - // limit. 29 more calls bring the shared renderWindow rebuild counter to - // 30 (still under the >30 breaker), each one landing normally. - for (let i = 0; i < 29; i++) { - expect(msgList.scrollToMessage(i + 1)).toBe(true); - } + // Every scrollToMessage call forces renderedStart = -1 and calls + // renderWindow() directly, bypassing renderAll's own (lower) rapid-fire + // limit. 29 more calls bring the shared renderWindow rebuild counter to + // 30 (still under the >30 breaker), each one landing normally. + for (let i = 0; i < 29; i++) { + expect(msgList.scrollToMessage(i + 1)).toBe(true); + } - // The 30th call pushes the counter to 31 and trips the breaker inside - // renderWindow: it returns before reassigning renderedStart/renderedEnd - // or touching the DOM, so the target (far outside the last rendered - // window) never actually renders. - const result = msgList.scrollToMessage(90); + // The 30th call pushes the counter to 31 and trips the breaker inside + // renderWindow: it returns before reassigning renderedStart/renderedEnd + // or touching the DOM, so the target (far outside the last rendered + // window) never actually renders. + const result = msgList.scrollToMessage(90); - // The rebuild did not happen — the row is not in the DOM — so this must - // be reported as a failed jump (matching the "false if the message is - // not in the loaded window" contract), not a false "true". - expect(container.querySelector('[data-testid="message-90"]')).toBeNull(); - expect(result).toBe(false); - }); + // The rebuild did not happen — the row is not in the DOM — so this must + // be reported as a failed jump (matching the "false if the message is + // not in the loaded window" contract), not a false "true". + expect(container.querySelector('[data-testid="message-90"]')).toBeNull(); + expect(result).toBe(false); + }, + ); }); }); diff --git a/Client/tauri-client/tests/unit/presence-sender.test.ts b/Client/tauri-client/tests/unit/presence-sender.test.ts index 10ddca5e..fc008ef4 100644 --- a/Client/tauri-client/tests/unit/presence-sender.test.ts +++ b/Client/tauri-client/tests/unit/presence-sender.test.ts @@ -24,7 +24,6 @@ function createMockWs(): WsClient { onCertMismatch: vi.fn().mockReturnValue(() => {}), acceptCertFingerprint: vi.fn(), getState: vi.fn(() => "connected"), - isReplaying: vi.fn(() => false), _getWs: vi.fn(() => null), } as unknown as WsClient; } diff --git a/Client/tauri-client/tests/unit/status-picker-userbar.test.ts b/Client/tauri-client/tests/unit/status-picker-userbar.test.ts index a3cfab2d..7c4e1634 100644 --- a/Client/tauri-client/tests/unit/status-picker-userbar.test.ts +++ b/Client/tauri-client/tests/unit/status-picker-userbar.test.ts @@ -49,7 +49,6 @@ function createMockWs(state: "connected" | "disconnected" = "connected"): WsClie onCertMismatch: vi.fn().mockReturnValue(() => {}), acceptCertFingerprint: vi.fn(), getState: vi.fn(() => currentState), - isReplaying: vi.fn(() => false), _getWs: vi.fn(() => null), _setState(s: "connected" | "disconnected") { currentState = s; diff --git a/Client/tauri-client/tests/unit/ws-active-channel.test.ts b/Client/tauri-client/tests/unit/ws-active-channel.test.ts index f8bc5fac..5992452c 100644 --- a/Client/tauri-client/tests/unit/ws-active-channel.test.ts +++ b/Client/tauri-client/tests/unit/ws-active-channel.test.ts @@ -28,7 +28,7 @@ function getAuthPayload(): Record { return parsed.payload; } -describe("auth frame: active_channel_id + reconnect-replay dedup arming", () => { +describe("auth frame: active_channel_id", () => { let client: ReturnType; beforeEach(() => { @@ -50,7 +50,7 @@ describe("auth frame: active_channel_id + reconnect-replay dedup arming", () => vi.useRealTimers(); }); - it("fresh connect (reconnectAttempt=0, lastSeq=0): no active_channel_id key even with a provider registered, and dedup is not armed", async () => { + it("fresh connect (reconnectAttempt=0, lastSeq=0): no active_channel_id key even with a provider registered", async () => { setActiveChannelProvider(() => 99); client.connect({ host: "localhost:8443", token: "t" }); await vi.advanceTimersByTimeAsync(10); @@ -59,7 +59,6 @@ describe("auth frame: active_channel_id + reconnect-replay dedup arming", () => const payload = getAuthPayload(); expect(payload.last_seq).toBe(0); expect(Object.prototype.hasOwnProperty.call(payload, "active_channel_id")).toBe(false); - expect(client.isReplaying()).toBe(false); }); it("reconnect with lastSeq > 0 and a registered provider: active_channel_id carries the provider's id", async () => { @@ -151,7 +150,7 @@ describe("auth frame: active_channel_id + reconnect-replay dedup arming", () => expect(Object.prototype.hasOwnProperty.call(payload, "active_channel_id")).toBe(false); }); - it("lastSeq > 0 but reconnectAttempt === 0: the provider is still consulted (gated on lastSeq, not reconnect count) and dedup stays unarmed", async () => { + it("lastSeq > 0 but reconnectAttempt === 0: the provider is still consulted (gated on lastSeq, not reconnect count)", async () => { client.connect({ host: "localhost:8443", token: "t" }); await vi.advanceTimersByTimeAsync(10); // First "open": reconnectAttempt=0, lastSeq=0 — irrelevant, just gets us started. @@ -177,11 +176,9 @@ describe("auth frame: active_channel_id + reconnect-replay dedup arming", () => const payload = getAuthPayload(); expect(payload.last_seq).toBe(9); expect(payload.active_channel_id).toBe(5); - // Dedup requires reconnectAttempt > 0 too — must still be unarmed. - expect(client.isReplaying()).toBe(false); }); - it("reconnectAttempt > 0 but lastSeq === 0: no active_channel_id key and dedup stays unarmed", async () => { + it("reconnectAttempt > 0 but lastSeq === 0: no active_channel_id key", async () => { setActiveChannelProvider(() => 5); client.connect({ host: "localhost:8443", token: "t" }); @@ -197,54 +194,5 @@ describe("auth frame: active_channel_id + reconnect-replay dedup arming", () => const payload = getAuthPayload(); expect(payload.last_seq).toBe(0); expect(Object.prototype.hasOwnProperty.call(payload, "active_channel_id")).toBe(false); - expect(client.isReplaying()).toBe(false); - }); - - it("dedup is armed only when BOTH reconnectAttempt > 0 AND lastSeq > 0: a genuine reconnect replay dedups a repeated message (fresh-connect non-arming is covered above)", async () => { - client.connect({ host: "localhost:8443", token: "t" }); - await vi.advanceTimersByTimeAsync(10); - emitTauriEvent("ws-state", "open"); - - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "auth_ok", - seq: 1, - payload: { - user: { id: 1, username: "a", avatar: null, role: "admin" }, - server_name: "S", - motd: "", - }, - }), - ); - - emitTauriEvent("ws-state", "closed"); - await vi.advanceTimersByTimeAsync(1100); - emitTauriEvent("ws-state", "open"); - - // Both conditions true now: reconnectAttempt=1, lastSeq=1. - expect(client.isReplaying()).toBe(true); - - const replayed: unknown[] = []; - client.on("chat_message", (p) => replayed.push(p)); - - const dupMsg = JSON.stringify({ - type: "chat_message", - seq: 5, - id: "dup-msg", - payload: { - id: 1, - channel_id: 1, - user: { id: 1, username: "a", avatar: null }, - content: "replayed", - reply_to: null, - attachments: [], - timestamp: "2026-01-01T00:00:00Z", - }, - }); - emitTauriEvent("ws-message", dupMsg); - emitTauriEvent("ws-message", dupMsg); - - expect(replayed).toHaveLength(1); }); }); diff --git a/Client/tauri-client/tests/unit/ws-lifecycle.test.ts b/Client/tauri-client/tests/unit/ws-lifecycle.test.ts index 1f6a524e..db6395b9 100644 --- a/Client/tauri-client/tests/unit/ws-lifecycle.test.ts +++ b/Client/tauri-client/tests/unit/ws-lifecycle.test.ts @@ -1012,107 +1012,6 @@ describe("cleanupEventListeners edge cases", () => { }); }); -describe("dedup does not filter auth_ok, auth_error, or ready during replay", () => { - let client: ReturnType; - - beforeEach(() => { - vi.useFakeTimers(); - mockInvoke.mockReset(); - mockInvoke.mockResolvedValue(undefined); - mockListen.mockClear(); - eventHandlers.clear(); - client = createWsClient(); - }); - - afterEach(() => { - client.disconnect(); - vi.useRealTimers(); - }); - - it("ready message is not deduped during replay", async () => { - client.connect({ host: "localhost:8443", token: "t" }); - await vi.advanceTimersByTimeAsync(10); - emitTauriEvent("ws-state", "open"); - - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "auth_ok", - seq: 5, - payload: { - user: { id: 1, username: "a", avatar: null, role: "admin" }, - server_name: "S", - motd: "", - }, - }), - ); - - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "chat_message", - seq: 10, - payload: { - id: 1, - channel_id: 1, - user: { id: 1, username: "a", avatar: null }, - content: "hi", - reply_to: null, - attachments: [], - timestamp: "2026-01-01T00:00:00Z", - }, - }), - ); - - // Disconnect and reconnect - emitTauriEvent("ws-state", "closed"); - await vi.advanceTimersByTimeAsync(1100); - emitTauriEvent("ws-state", "open"); - expect(client.isReplaying()).toBe(true); - - const readyPayloads: unknown[] = []; - client.on("ready", (p) => readyPayloads.push(p)); - - // Send ready during replay BEFORE auth_ok — should NOT be deduped - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "ready", - seq: 11, - payload: { - channels: [], - members: [], - voice_states: [], - roles: [], - }, - }), - ); - - expect(readyPayloads).toHaveLength(1); - - // Send ready again with same seq — ready is exempt from dedup, so it passes - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "ready", - seq: 11, - payload: { - channels: [], - members: [], - voice_states: [], - roles: [], - }, - }), - ); - - expect(readyPayloads).toHaveLength(2); - }); -}); - -// --------------------------------------------------------------------------- -// Listener registry mechanics (no Tauri connection needed) -// --------------------------------------------------------------------------- - describe("listener registry mechanics (on/off/dispatch)", () => { let client: ReturnType; diff --git a/Client/tauri-client/tests/unit/ws-messaging.test.ts b/Client/tauri-client/tests/unit/ws-messaging.test.ts index bdf7785c..f8416458 100644 --- a/Client/tauri-client/tests/unit/ws-messaging.test.ts +++ b/Client/tauri-client/tests/unit/ws-messaging.test.ts @@ -244,10 +244,6 @@ describe("message handling edge cases", () => { expect(client.getState()).toBe("connecting"); }); - it("isReplaying returns false when not reconnecting", () => { - expect(client.isReplaying()).toBe(false); - }); - it("_getWs returns null", () => { expect(client._getWs()).toBeNull(); }); diff --git a/Client/tauri-client/tests/unit/ws-reconnect.test.ts b/Client/tauri-client/tests/unit/ws-reconnect.test.ts index c187f53f..4bf51df4 100644 --- a/Client/tauri-client/tests/unit/ws-reconnect.test.ts +++ b/Client/tauri-client/tests/unit/ws-reconnect.test.ts @@ -389,7 +389,7 @@ describe("lastSeq tracking", () => { }); }); -describe("reconnection dedup", () => { +describe("reconnection replay handling", () => { let client: ReturnType; beforeEach(() => { @@ -406,12 +406,11 @@ describe("reconnection dedup", () => { vi.useRealTimers(); }); - it("deduplicates messages during reconnection replay", async () => { + it("dispatches replay-burst frames arriving after auth_ok verbatim (no client-side dedup layer)", async () => { client.connect({ host: "localhost:8443", token: "t" }); await vi.advanceTimersByTimeAsync(10); emitTauriEvent("ws-state", "open"); - // Auth and get some messages to advance lastSeq emitTauriEvent( "ws-message", JSON.stringify({ @@ -424,7 +423,6 @@ describe("reconnection dedup", () => { }, }), ); - emitTauriEvent( "ws-message", JSON.stringify({ @@ -435,7 +433,7 @@ describe("reconnection dedup", () => { id: 1, channel_id: 1, user: { id: 1, username: "a", avatar: null }, - content: "original", + content: "before disconnect", reply_to: null, attachments: [], timestamp: "2026-01-01T00:00:00Z", @@ -443,186 +441,51 @@ describe("reconnection dedup", () => { }), ); - // Disconnect unexpectedly emitTauriEvent("ws-state", "closed"); - - // Wait for reconnect await vi.advanceTimersByTimeAsync(1100); emitTauriEvent("ws-state", "open"); - // During reconnect, replay dedup is active - expect(client.isReplaying()).toBe(true); + // The server writes auth_ok BEFORE the replay burst + // (reconnectWriteReplay), so replayed frames land in the connected state + // exactly like live ones. The ws layer applies them verbatim — the old + // replayDedup machinery keyed off a pre-auth_ok window that never exists + // against a spec-compliant server and was removed (audit-2026-08-19 + // F-6); duplicate handling belongs to the stores. + emitTauriEvent( + "ws-message", + JSON.stringify({ + type: "auth_ok", + seq: 5, + payload: { + user: { id: 1, username: "a", avatar: null, role: "admin" }, + server_name: "S", + motd: "", + replay_source: "buffer", + }, + }), + ); const messages: unknown[] = []; client.on("chat_message", (p) => messages.push(p)); - // Send a message during replay -- first occurrence passes - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "chat_message", - seq: 5, - id: "msg-5", - payload: { - id: 1, - channel_id: 1, - user: { id: 1, username: "a", avatar: null }, - content: "original", - reply_to: null, - attachments: [], - timestamp: "2026-01-01T00:00:00Z", - }, - }), - ); + const replayFrame = JSON.stringify({ + type: "chat_message", + seq: 6, + id: "msg-6", + payload: { + id: 2, + channel_id: 1, + user: { id: 1, username: "a", avatar: null }, + content: "replayed", + reply_to: null, + attachments: [], + timestamp: "2026-01-01T00:00:00Z", + }, + }); + emitTauriEvent("ws-message", replayFrame); + emitTauriEvent("ws-message", replayFrame); - // Send the SAME message ID again — should be deduped - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "chat_message", - seq: 5, - id: "msg-5", - payload: { - id: 1, - channel_id: 1, - user: { id: 1, username: "a", avatar: null }, - content: "original", - reply_to: null, - attachments: [], - timestamp: "2026-01-01T00:00:00Z", - }, - }), - ); - - // Only the first occurrence should pass through - expect(messages).toHaveLength(1); - expect((messages[0] as { content: string }).content).toBe("original"); - }); - - it("auth_ok and ready messages are not deduped during replay", async () => { - client.connect({ host: "localhost:8443", token: "t" }); - await vi.advanceTimersByTimeAsync(10); - emitTauriEvent("ws-state", "open"); - - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "auth_ok", - seq: 5, - payload: { - user: { id: 1, username: "a", avatar: null, role: "admin" }, - server_name: "S", - motd: "", - }, - }), - ); - - // Disconnect - emitTauriEvent("ws-state", "closed"); - await vi.advanceTimersByTimeAsync(1100); - emitTauriEvent("ws-state", "open"); - - expect(client.isReplaying()).toBe(true); - - const authPayloads: unknown[] = []; - client.on("auth_ok", (p) => authPayloads.push(p)); - - // auth_ok during replay should NOT be deduped - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "auth_ok", - seq: 6, - payload: { - user: { id: 1, username: "a", avatar: null, role: "admin" }, - server_name: "S", - motd: "", - }, - }), - ); - - expect(authPayloads).toHaveLength(1); - // After auth_ok, replay dedup should be cleared - expect(client.isReplaying()).toBe(false); - }); - - it("dedup uses type:seq as key when message has no id", async () => { - client.connect({ host: "localhost:8443", token: "t" }); - await vi.advanceTimersByTimeAsync(10); - emitTauriEvent("ws-state", "open"); - - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "auth_ok", - seq: 1, - payload: { - user: { id: 1, username: "a", avatar: null, role: "admin" }, - server_name: "S", - motd: "", - }, - }), - ); - - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "presence", - seq: 10, - payload: { user_id: 1, status: "idle" }, - }), - ); - - // Disconnect - emitTauriEvent("ws-state", "closed"); - await vi.advanceTimersByTimeAsync(1100); - emitTauriEvent("ws-state", "open"); - - const presences: unknown[] = []; - client.on("presence", (p) => presences.push(p)); - - // First presence during replay — passes through - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "presence", - seq: 10, - payload: { user_id: 1, status: "idle" }, - }), - ); - - // Same type:seq — should be deduped - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "presence", - seq: 10, - payload: { user_id: 1, status: "idle" }, - }), - ); - - // Different seq — should pass through - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "presence", - seq: 11, - payload: { user_id: 1, status: "online" }, - }), - ); - - expect(presences).toHaveLength(2); - expect((presences[0] as { status: string }).status).toBe("idle"); - expect((presences[1] as { status: string }).status).toBe("online"); - }); - - it("dedup is not active for first connection (lastSeq=0)", async () => { - client.connect({ host: "localhost:8443", token: "t" }); - await vi.advanceTimersByTimeAsync(10); - emitTauriEvent("ws-state", "open"); - - // First connect should NOT enable dedup - expect(client.isReplaying()).toBe(false); + expect(messages).toHaveLength(2); }); }); @@ -854,115 +717,6 @@ describe("scheduleReconnect guard clauses", () => { }); }); -describe("dedup eviction when exceeding MAX_DEDUP_SIZE", () => { - let client: ReturnType; - - beforeEach(() => { - vi.useFakeTimers(); - mockInvoke.mockReset(); - mockInvoke.mockResolvedValue(undefined); - mockListen.mockClear(); - eventHandlers.clear(); - client = createWsClient(); - }); - - afterEach(() => { - client.disconnect(); - vi.useRealTimers(); - }); - - it("evicts oldest entry when dedup set exceeds 1000 entries", async () => { - client.connect({ host: "localhost:8443", token: "t" }); - await vi.advanceTimersByTimeAsync(10); - emitTauriEvent("ws-state", "open"); - - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "auth_ok", - seq: 1, - payload: { - user: { id: 1, username: "a", avatar: null, role: "admin" }, - server_name: "S", - motd: "", - }, - }), - ); - - // Get past lastSeq > 0 condition - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "chat_message", - seq: 100, - payload: { - id: 99, - channel_id: 1, - user: { id: 1, username: "a", avatar: null }, - content: "bump seq", - reply_to: null, - attachments: [], - timestamp: "2026-01-01T00:00:00Z", - }, - }), - ); - - // Disconnect to trigger dedup mode - emitTauriEvent("ws-state", "closed"); - await vi.advanceTimersByTimeAsync(1100); - emitTauriEvent("ws-state", "open"); - expect(client.isReplaying()).toBe(true); - - const messages: unknown[] = []; - client.on("chat_message", (p) => messages.push(p)); - - // Send 1002 unique messages to trigger eviction (MAX_DEDUP_SIZE = 1000) - for (let i = 0; i < 1002; i++) { - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "chat_message", - seq: 101 + i, - id: `msg-${i}`, - payload: { - id: i, - channel_id: 1, - user: { id: 1, username: "a", avatar: null }, - content: `msg ${i}`, - reply_to: null, - attachments: [], - timestamp: "2026-01-01T00:00:00Z", - }, - }), - ); - } - - // All 1002 should have been dispatched (first occurrence of each) - expect(messages).toHaveLength(1002); - - // Now re-send the very first message (msg-0) — it was evicted, so it should pass again - const countBefore = messages.length; - emitTauriEvent( - "ws-message", - JSON.stringify({ - type: "chat_message", - seq: 101, - id: "msg-0", - payload: { - id: 0, - channel_id: 1, - user: { id: 1, username: "a", avatar: null }, - content: "msg 0", - reply_to: null, - attachments: [], - timestamp: "2026-01-01T00:00:00Z", - }, - }), - ); - expect(messages).toHaveLength(countBefore + 1); - }); -}); - describe("auth_error during reconnection replay", () => { let client: ReturnType; @@ -980,7 +734,7 @@ describe("auth_error during reconnection replay", () => { vi.useRealTimers(); }); - it("auth_error is not deduped during replay and stops reconnect", async () => { + it("auth_error after a reconnect handshake stops reconnection", async () => { client.connect({ host: "localhost:8443", token: "t" }); await vi.advanceTimersByTimeAsync(10); emitTauriEvent("ws-state", "open"); @@ -1002,12 +756,10 @@ describe("auth_error during reconnection replay", () => { emitTauriEvent("ws-state", "closed"); await vi.advanceTimersByTimeAsync(1100); emitTauriEvent("ws-state", "open"); - expect(client.isReplaying()).toBe(true); const errors: unknown[] = []; client.on("auth_error", (p) => errors.push(p)); - // auth_error during replay — should NOT be deduped emitTauriEvent( "ws-message", JSON.stringify({ diff --git a/Client/tauri-client/tsconfig.e2e.json b/Client/tauri-client/tsconfig.e2e.json index 0936137f..13c98951 100644 --- a/Client/tauri-client/tsconfig.e2e.json +++ b/Client/tauri-client/tsconfig.e2e.json @@ -1,5 +1,5 @@ { - // Typechecks the Playwright layer (47 spec files + fixtures + the three + // Typechecks the Playwright layer (every tests/e2e spec + fixtures + the // playwright configs), which the main tsconfig deliberately excludes from // the app graph. "exclude": [] is required to clear the inherited // "tests/e2e" exclusion — same trick tsconfig.build.json uses. diff --git a/Server/api/dm_handler.go b/Server/api/dm_handler.go index 6e408361..b7dee469 100644 --- a/Server/api/dm_handler.go +++ b/Server/api/dm_handler.go @@ -83,7 +83,7 @@ func MountDMRoutes(r chi.Router, database *db.DB, svc *service.Services, broadca r.Route("/api/v1/blocks", func(r chi.Router) { r.Use(AuthMiddleware(database)) r.Get("/", handleListBlocks(svc)) - r.Put("/{userId}", handleBlockUser(svc)) + r.Put("/{userId}", handleBlockUser(svc, broadcaster)) r.Delete("/{userId}", handleUnblockUser(svc)) }) } @@ -381,7 +381,7 @@ func handleRenameGroupDM(svc *service.Services, broadcaster DMBroadcaster) http. } // handleBlockUser blocks a user. -func handleBlockUser(svc *service.Services) http.HandlerFunc { +func handleBlockUser(svc *service.Services, broadcaster DMBroadcaster) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { user, _ := r.Context().Value(UserKey).(*db.User) if user == nil { @@ -398,6 +398,23 @@ func handleBlockUser(svc *service.Services) http.HandlerFunc { writeServiceError(r.Context(), w, err) return } + + // Revocation must evict a live session, not merely block the next + // join (the same invariant the voice sweep states): without this, a + // blocked user already in the pair's 1:1 DM voice call stays in it + // indefinitely — the block gate otherwise runs only on voice_join and + // voluntary voice_token_refresh, both of which the blocked client + // controls. Group DM calls are deliberately untouched, matching + // requireDMNotBlocked's group exemption. + if ve, evictable := broadcaster.(dmVoiceEvictor); evictable { + if chID, exists, err := svc.DMs.SharedOneToOneDM(r.Context(), user.ID, targetID); err != nil { + slog.Warn("block: shared-DM lookup for voice eviction failed", + "blocker_id", user.ID, "target_id", targetID, "err", err) + } else if exists { + ve.DisconnectFromVoiceInChannel(context.WithoutCancel(r.Context()), targetID, chID) + } + } + writeJSON(w, http.StatusOK, map[string]string{"message": "user blocked"}) } } diff --git a/Server/api/dm_handler_watermark_voice_test.go b/Server/api/dm_handler_watermark_voice_test.go index c5b1a8b0..66cd2b4a 100644 --- a/Server/api/dm_handler_watermark_voice_test.go +++ b/Server/api/dm_handler_watermark_voice_test.go @@ -110,3 +110,77 @@ func TestCloseDM_OneToOneDoesNotEvictVoice(t *testing.T) { t.Fatalf("DisconnectFromVoiceInChannel calls = %+v, want none for a 1:1 close", bc.evictCalls) } } + +// Blocking a user must evict them from the pair's live 1:1 DM voice call +// (audit-2026-08-19 F-1): the block gate otherwise runs only on voice_join +// and voluntary voice_token_refresh, so a blocked user already in the call +// would stay in it indefinitely. +func TestBlockUser_EvictsBlockedUserFromSharedDMVoice(t *testing.T) { + database := newDMTestDB(t) + bc := &watermarkVoiceBroadcaster{mockBroadcaster: &mockBroadcaster{}} + router := buildDMRouter(database, bc) + tokens := []string{ + dmCreateToken(t, database, "alice", 4), + dmCreateToken(t, database, "bob", 4), + } + rr := dmPost(t, router, "/api/v1/dms", tokens[0], map[string]any{"recipient_id": 2}) + var created struct { + ChannelID int64 `json:"channel_id"` + } + _ = json.Unmarshal(rr.Body.Bytes(), &created) + bc.evictCalls = nil + + if blockRR := dmPut(t, router, "/api/v1/blocks/2", tokens[0]); blockRR.Code != http.StatusOK { + t.Fatalf("block: %d %s", blockRR.Code, blockRR.Body.String()) + } + + if len(bc.evictCalls) != 1 || bc.evictCalls[0].userID != 2 || bc.evictCalls[0].channelID != created.ChannelID { + t.Fatalf("DisconnectFromVoiceInChannel calls = %+v, want exactly one for user=2 channel=%d", bc.evictCalls, created.ChannelID) + } +} + +// A block between users with no shared 1:1 DM has no call to sever — the +// eviction capability must not fire at all. +func TestBlockUser_NoSharedDM_NoEviction(t *testing.T) { + database := newDMTestDB(t) + bc := &watermarkVoiceBroadcaster{mockBroadcaster: &mockBroadcaster{}} + router := buildDMRouter(database, bc) + alice := dmCreateToken(t, database, "alice", 4) + dmCreateToken(t, database, "bob", 4) + + if blockRR := dmPut(t, router, "/api/v1/blocks/2", alice); blockRR.Code != http.StatusOK { + t.Fatalf("block: %d %s", blockRR.Code, blockRR.Body.String()) + } + + if len(bc.evictCalls) != 0 { + t.Fatalf("DisconnectFromVoiceInChannel calls = %+v, want none without a shared 1:1 DM", bc.evictCalls) + } +} + +// Group DM calls are exempt from block enforcement (matching +// requireDMNotBlocked's group exemption), so blocking a co-member of a group +// must not evict them from the group's call. +func TestBlockUser_GroupDMOnly_NoEviction(t *testing.T) { + database := newDMTestDB(t) + bc := &watermarkVoiceBroadcaster{mockBroadcaster: &mockBroadcaster{}} + router := buildDMRouter(database, bc) + tokens := []string{ + dmCreateToken(t, database, "alice", 4), + dmCreateToken(t, database, "bob", 4), + dmCreateToken(t, database, "carol", 4), + } + if rr := dmPost(t, router, "/api/v1/dms/group", tokens[0], map[string]any{ + "recipient_ids": []int64{2, 3}, + }); rr.Code != http.StatusCreated { + t.Fatalf("create group dm: %d %s", rr.Code, rr.Body.String()) + } + bc.evictCalls = nil + + if blockRR := dmPut(t, router, "/api/v1/blocks/2", tokens[0]); blockRR.Code != http.StatusOK { + t.Fatalf("block: %d %s", blockRR.Code, blockRR.Body.String()) + } + + if len(bc.evictCalls) != 0 { + t.Fatalf("DisconnectFromVoiceInChannel calls = %+v, want none for a group-only relationship", bc.evictCalls) + } +} diff --git a/Server/auth/ratelimit.go b/Server/auth/ratelimit.go index 800a4fa0..188a79f3 100644 --- a/Server/auth/ratelimit.go +++ b/Server/auth/ratelimit.go @@ -169,7 +169,10 @@ func (r *RateLimiter) Lockout(ctx context.Context, key string, duration time.Dur expiresAt := time.Now().Add(duration) s.lockouts[key] = &lockoutEntry{expiresAt: expiresAt} if r.store != nil { - _ = r.store.UpsertLockout(context.WithoutCancel(ctx), key, expiresAt) + if err := r.store.UpsertLockout(context.WithoutCancel(ctx), key, expiresAt); err != nil { + slog.Warn("ratelimit: failed to persist lockout; it will not survive a restart", + "key", key, "err", err) + } } } @@ -231,7 +234,10 @@ func (r *RateLimiter) Reset(ctx context.Context, key string) { delete(s.windows, key) delete(s.lockouts, key) if r.store != nil { - _ = r.store.DeleteLockout(context.WithoutCancel(ctx), key) + if err := r.store.DeleteLockout(context.WithoutCancel(ctx), key); err != nil { + slog.Warn("ratelimit: failed to delete persisted lockout; it may reappear after a restart", + "key", key, "err", err) + } } } @@ -280,7 +286,9 @@ func (r *RateLimiter) Cleanup(maxWindow time.Duration) { if r.store != nil { // Runs from the StartCleanup background goroutine — no request ctx. - _ = r.store.CleanupExpiredLockouts(context.Background()) + if err := r.store.CleanupExpiredLockouts(context.Background()); err != nil { + slog.Warn("ratelimit: failed to clean up expired persisted lockouts", "err", err) + } } } diff --git a/Server/auth/ratelimit_persist_test.go b/Server/auth/ratelimit_persist_test.go index e16bf651..aef6b24e 100644 --- a/Server/auth/ratelimit_persist_test.go +++ b/Server/auth/ratelimit_persist_test.go @@ -46,3 +46,72 @@ func TestNewPersistentRateLimiter_LoadErrorIsLogged(t *testing.T) { t.Errorf("expected log output to mention the load error, got: %q", out) } } + +// failingWriteLockoutStore fails every write, simulating a transient DB +// failure once the limiter is already running (audit-2026-08-19 F-3 — the +// write-path twin of OC-0061's load-path store). +type failingWriteLockoutStore struct{} + +func (failingWriteLockoutStore) UpsertLockout(context.Context, string, time.Time) error { + return errors.New("upsert: disk I/O error") +} +func (failingWriteLockoutStore) DeleteLockout(context.Context, string) error { + return errors.New("delete: disk I/O error") +} +func (failingWriteLockoutStore) CleanupExpiredLockouts(context.Context) error { + return errors.New("cleanup: disk I/O error") +} +func (failingWriteLockoutStore) LoadActiveLockouts(context.Context) ([]string, []time.Time, error) { + return nil, nil, nil +} + +// TestLockout_PersistErrorIsLoggedAndInMemoryLockoutHolds pins F-3's +// contract: a failed persist write is logged, and the in-memory lockout +// still applies for the life of the process. +func TestLockout_PersistErrorIsLoggedAndInMemoryLockoutHolds(t *testing.T) { + rl := auth.NewPersistentRateLimiter(failingWriteLockoutStore{}) + + out := captureLogs(t, func() { + rl.Lockout(context.Background(), "login:198.51.100.7", time.Minute) + }) + + if !strings.Contains(out, "upsert: disk I/O error") { + t.Errorf("expected log output to mention the persist error, got: %q", out) + } + if !rl.IsLockedOut("login:198.51.100.7") { + t.Error("in-memory lockout must hold even when the persist write fails") + } +} + +// TestReset_DeleteErrorIsLoggedAndInMemoryStateClears pins the same +// contract on the delete path: the in-memory clear wins, the store failure +// is visible. +func TestReset_DeleteErrorIsLoggedAndInMemoryStateClears(t *testing.T) { + rl := auth.NewPersistentRateLimiter(failingWriteLockoutStore{}) + rl.Lockout(context.Background(), "login:198.51.100.7", time.Minute) + + out := captureLogs(t, func() { + rl.Reset(context.Background(), "login:198.51.100.7") + }) + + if !strings.Contains(out, "delete: disk I/O error") { + t.Errorf("expected log output to mention the delete error, got: %q", out) + } + if rl.IsLockedOut("login:198.51.100.7") { + t.Error("in-memory lockout must clear even when the store delete fails") + } +} + +// TestCleanup_StoreErrorIsLogged pins the third write path: the periodic +// expired-lockout cleanup logging its store failure instead of dropping it. +func TestCleanup_StoreErrorIsLogged(t *testing.T) { + rl := auth.NewPersistentRateLimiter(failingWriteLockoutStore{}) + + out := captureLogs(t, func() { + rl.Cleanup(time.Minute) + }) + + if !strings.Contains(out, "cleanup: disk I/O error") { + t.Errorf("expected log output to mention the cleanup error, got: %q", out) + } +} diff --git a/Server/db/auth_queries.go b/Server/db/auth_queries.go index f0c3d899..612a0c72 100644 --- a/Server/db/auth_queries.go +++ b/Server/db/auth_queries.go @@ -7,6 +7,7 @@ import ( "encoding/hex" "errors" "fmt" + "log/slog" "strings" "time" @@ -240,11 +241,18 @@ const maxSessionsPerUser = 25 // H-6: Enforces a per-user session cap by evicting the oldest session when // the limit is reached. func (d *DB) CreateSession(ctx context.Context, userID int64, tokenHash, device, ip string) (int64, error) { - // Evict oldest sessions if at or above the cap. - _ = d.q.EvictOldestSessions(ctx, dbgen.EvictOldestSessionsParams{ + // Evict oldest sessions if at or above the cap. A failed eviction must + // not block the login (the DELETE trims to the cap again on the next + // successful CreateSession, and a persistent DB failure fails the + // InsertSession below anyway), but an H-6 control failing is never + // allowed to be invisible. + if err := d.q.EvictOldestSessions(ctx, dbgen.EvictOldestSessionsParams{ UserID: userID, Offset: maxSessionsPerUser - 1, - }) + }); err != nil { + slog.Warn("session cap: failed to evict oldest sessions", + "user_id", userID, "err", err) + } expiresAt := time.Now().Add(sessionTTL).UTC().Format(sessionTimeLayout) deviceCopy, ipCopy := device, ip diff --git a/Server/db/dbgen/dm.sql.go b/Server/db/dbgen/dm.sql.go index 38f4f58c..4a153ac5 100644 --- a/Server/db/dbgen/dm.sql.go +++ b/Server/db/dbgen/dm.sql.go @@ -34,6 +34,31 @@ func (q *Queries) CountDMParticipants(ctx context.Context, channelID int64) (int return count, err } +const findDMChannelIDBetween = `-- name: FindDMChannelIDBetween :one +SELECT dp1.channel_id FROM dm_participants dp1 +JOIN dm_participants dp2 ON dp1.channel_id = dp2.channel_id +JOIN channels c ON c.id = dp1.channel_id +WHERE dp1.user_id = ? AND dp2.user_id = ? AND c.type = 'dm' AND c.is_group = 0 +ORDER BY dp1.channel_id ASC +` + +type FindDMChannelIDBetweenParams struct { + UserID int64 `json:"userId"` + UserID_2 int64 `json:"userId2"` +} + +// The 1:1 DM channel between two users, if one exists. Mirrors the lookup +// inside GetOrCreateDMChannel (raw, transactional) without creating anything: +// the is_group clause keeps group DMs out, matching the block-enforcement +// boundary (blocks never gate group DMs). ORDER BY makes the row choice +// deterministic should duplicates ever exist. +func (q *Queries) FindDMChannelIDBetween(ctx context.Context, arg FindDMChannelIDBetweenParams) (int64, error) { + row := q.db.QueryRowContext(ctx, findDMChannelIDBetween, arg.UserID, arg.UserID_2) + var channel_id int64 + err := row.Scan(&channel_id) + return channel_id, err +} + const getDMParticipantIDs = `-- name: GetDMParticipantIDs :many SELECT user_id FROM dm_participants WHERE channel_id = ? ` diff --git a/Server/db/dbgen/querier.go b/Server/db/dbgen/querier.go index bb8b458d..8aeb4bb0 100644 --- a/Server/db/dbgen/querier.go +++ b/Server/db/dbgen/querier.go @@ -91,6 +91,12 @@ type Querier interface { // own screenshare flag so re-enable is idempotent at the cap (OC-0081). EnableScreenshareIfUnderLimit(ctx context.Context, arg EnableScreenshareIfUnderLimitParams) (sql.Result, error) EvictOldestSessions(ctx context.Context, arg EvictOldestSessionsParams) error + // The 1:1 DM channel between two users, if one exists. Mirrors the lookup + // inside GetOrCreateDMChannel (raw, transactional) without creating anything: + // the is_group clause keeps group DMs out, matching the block-enforcement + // boundary (blocks never gate group DMs). ORDER BY makes the row choice + // deterministic should duplicates ever exist. + FindDMChannelIDBetween(ctx context.Context, arg FindDMChannelIDBetweenParams) (int64, error) ForceLogoutUser(ctx context.Context, userID int64) error // Auth-hot lookup: returns the token only if it is neither revoked nor expired, // so a resolved row is always usable. Matches the sessions never-expiring diff --git a/Server/db/dm_queries.go b/Server/db/dm_queries.go index 18205491..314f5c95 100644 --- a/Server/db/dm_queries.go +++ b/Server/db/dm_queries.go @@ -188,6 +188,24 @@ func (d *DB) GetOrCreateDMChannel(ctx context.Context, user1ID, user2ID int64) ( return ch, true, nil } +// FindDMChannelIDBetween returns the id of the 1:1 DM channel the two users +// share, or ok=false when none exists. It never creates anything, and group +// DMs never match — blocks do not gate them, so side effects keyed off a +// block (like voice eviction) must not reach a group call. +func (d *DB) FindDMChannelIDBetween(ctx context.Context, user1ID, user2ID int64) (int64, bool, error) { + id, err := d.q.FindDMChannelIDBetween(ctx, dbgen.FindDMChannelIDBetweenParams{ + UserID: user1ID, + UserID_2: user2ID, + }) + if errors.Is(err, sql.ErrNoRows) { + return 0, false, nil + } + if err != nil { + return 0, false, fmt.Errorf("FindDMChannelIDBetween: %w", err) + } + return id, true, nil +} + // ─── GetUserDMChannels ────────────────────────────────────────────────────── // GetUserDMChannels returns all open DM channels for a user with the full diff --git a/Server/db/queries/sqlite/dm.sql b/Server/db/queries/sqlite/dm.sql index 7e474e62..1342d7f4 100644 --- a/Server/db/queries/sqlite/dm.sql +++ b/Server/db/queries/sqlite/dm.sql @@ -78,3 +78,15 @@ JOIN dm_participants dp ON dp.channel_id = dos.channel_id JOIN users u ON u.id = dp.user_id WHERE dos.user_id = ? ORDER BY dp.channel_id ASC, u.id ASC; + +-- The 1:1 DM channel between two users, if one exists. Mirrors the lookup +-- inside GetOrCreateDMChannel (raw, transactional) without creating anything: +-- the is_group clause keeps group DMs out, matching the block-enforcement +-- boundary (blocks never gate group DMs). ORDER BY makes the row choice +-- deterministic should duplicates ever exist. +-- name: FindDMChannelIDBetween :one +SELECT dp1.channel_id FROM dm_participants dp1 +JOIN dm_participants dp2 ON dp1.channel_id = dp2.channel_id +JOIN channels c ON c.id = dp1.channel_id +WHERE dp1.user_id = ? AND dp2.user_id = ? AND c.type = 'dm' AND c.is_group = 0 +ORDER BY dp1.channel_id ASC; diff --git a/Server/logctx/logctx.go b/Server/logctx/logctx.go index 0bacffc6..a8e72800 100644 --- a/Server/logctx/logctx.go +++ b/Server/logctx/logctx.go @@ -46,7 +46,7 @@ func (h handler) WithAttrs(attrs []slog.Attr) slog.Handler { } // WithGroup re-wraps so enrichment survives logger.WithGroup. -// ponytail: req_id/trace_id are added at the record's top level; the codebase +// req_id/trace_id are added at the record's top level; the codebase // opens no logger-level groups, so there is no group-nesting concern to handle // here. Revisit if slog group usage is introduced. func (h handler) WithGroup(name string) slog.Handler { diff --git a/Server/service/channel.go b/Server/service/channel.go index ebd2ec50..91bf0e1a 100644 --- a/Server/service/channel.go +++ b/Server/service/channel.go @@ -282,7 +282,12 @@ func (s *ChannelService) HandleChannelFocus(ctx context.Context, userID, channel "user_id", userID, "channel_id", channelID) return ch, nil } - _ = s.st.UpdateReadState(ctx, userID, channelID, latestID) + if wErr := s.st.UpdateReadState(ctx, userID, channelID, latestID); wErr != nil { + // Self-heals on the next focus, but a persistently failing write + // means unread badges never clear — it must not be invisible. + slog.Warn("channel_focus: read-state write failed", + "user_id", userID, "channel_id", channelID, "err", wErr) + } } slog.Debug("channel_focus", "user_id", userID, "channel_id", channelID) diff --git a/Server/service/datastore.go b/Server/service/datastore.go index 572a4fe4..efd5116d 100644 --- a/Server/service/datastore.go +++ b/Server/service/datastore.go @@ -153,6 +153,7 @@ type Store interface { // ── Direct messages ── GetOrCreateDMChannel(ctx context.Context, user1ID, user2ID int64) (*db.Channel, bool, error) + FindDMChannelIDBetween(ctx context.Context, user1ID, user2ID int64) (int64, bool, error) GetUserDMChannels(ctx context.Context, userID int64) ([]db.DMChannelInfo, error) GetUserDMChannelIDs(ctx context.Context, userID int64) ([]int64, error) OpenDM(ctx context.Context, userID, channelID int64) (bool, error) diff --git a/Server/service/dm.go b/Server/service/dm.go index 240cdbd1..4f47b0c2 100644 --- a/Server/service/dm.go +++ b/Server/service/dm.go @@ -363,6 +363,17 @@ func (s *DMService) DMSummaryFor(ctx context.Context, viewerID, channelID int64) return db.NewDMChannelInfo(channelID, ch.Name, isGroup, participants, viewerID), nil } +// SharedOneToOneDM returns the id of the 1:1 DM channel the two users share, +// or ok=false when they have none. Group DMs never match, mirroring the +// block-enforcement boundary (requireDMNotBlocked exempts groups). +func (s *DMService) SharedOneToOneDM(ctx context.Context, userA, userB int64) (int64, bool, error) { + id, ok, err := s.st.FindDMChannelIDBetween(ctx, userA, userB) + if err != nil { + return 0, false, fmt.Errorf("%w: failed to look up shared DM: %v", ErrInternal, err) + } + return id, ok, nil +} + // RingTargets returns the other participants of a DM the caller is in — the // people a call_ring or call_decline is addressed to. // diff --git a/Server/ws/hub_broadcast.go b/Server/ws/hub_broadcast.go index 94245b2a..36d66468 100644 --- a/Server/ws/hub_broadcast.go +++ b/Server/ws/hub_broadcast.go @@ -794,6 +794,13 @@ func (h *Hub) BroadcastMemberUpdate(userID int64, roleName string) { // // Only the topics the socket actually holds are examined — a blanket sweep over // every channel would disclose the full channel-ID list to a demoted user. +// +// revokeUnreadableChannelsPreActRaceHook, when non-nil, runs once per revoked +// topic immediately before the live-client re-resolve. Test-only (nil in +// production); pins the replaced-mid-loop hazard deterministically — same +// pattern as refreshChannelVisibilityRaceHook. +var revokeUnreadableChannelsPreActRaceHook func(userID int64) + func (h *Hub) revokeUnreadableChannels(userID int64) { // Ratcheted upward only (see bumpVisibilityWatermark), and evaluated at // defer-RUN time — not the plain Store(Load(&h.seq)) this used to be, @@ -843,7 +850,12 @@ func (h *Hub) revokeUnreadableChannels(userID int64) { // makes the client clear its credentials instead of reconnecting. slog.Warn("hub: role change visibility unresolved, closing socket", "user_id", userID, "err", err) - h.kickClient(c) + // Re-resolve before kicking: the lookups above are DB round trips a + // reconnect can overlap, and kicking the stale snapshot would close a + // dead socket while the replacement keeps its subscriptions. + if live := h.GetClient(userID); live != nil { + h.kickClient(live) + } return } @@ -862,19 +874,36 @@ func (h *Hub) revokeUnreadableChannels(userID int64) { if chErr != nil { slog.Warn("hub: role change channel lookup failed, closing socket", "user_id", userID, "channel_id", chID, "err", chErr) - h.kickClient(c) + if live := h.GetClient(userID); live != nil { + h.kickClient(live) + } return } if ch != nil && ch.Type == "dm" { continue } - c.sendMsg(buildChannelDelete(chID)) - h.pubsub.Unsubscribe(c, topic) - c.mu.Lock() - if c.channelID == chID { - c.channelID = 0 + if revokeUnreadableChannelsPreActRaceHook != nil { + revokeUnreadableChannelsPreActRaceHook(userID) } - c.mu.Unlock() + // Re-resolve the live client immediately before acting: the DB round + // trips above (and computeAllowedChannels before the loop) give a + // reconnect room to replace this user's *Client in h.clients. Acting + // on the snapshot c would target the dead socket, and Unsubscribe + // would no-op on unsubscribeLocked's identity guard — stranding the + // replacement with the revoked topic (audit-2026-08-19 F-2; mirrors + // RefreshChannelVisibility's live re-resolve). A nil result means the + // user disconnected entirely; nothing left to revoke. + live := h.GetClient(userID) + if live == nil { + return + } + live.sendMsg(buildChannelDelete(chID)) + h.pubsub.Unsubscribe(live, topic) + live.mu.Lock() + if live.channelID == chID { + live.channelID = 0 + } + live.mu.Unlock() } } diff --git a/Server/ws/role_reassign_handshake_test.go b/Server/ws/role_reassign_handshake_test.go new file mode 100644 index 00000000..afe0d58b --- /dev/null +++ b/Server/ws/role_reassign_handshake_test.go @@ -0,0 +1,381 @@ +package ws + +// role_reassign_handshake_test.go — regression tests for audit-2026-08-19 +// F-2: a role reassignment racing a WS handshake must not leave the socket +// on subscriptions resolved from the auth-time role snapshot. +// +// The defect had three cooperating halves: +// 1. reconnectPrecheck / handleFreshConnect resolved permissions from +// c.user, the row fetched at auth time, so a reassignment landing after +// auth was invisible to the whole handshake. +// 2. revokeUnreadableChannels early-returns for a user not yet in +// h.clients, so the reassignment's own revocation pass cannot reach a +// mid-handshake socket. +// 3. revokeUnreadableChannels acted on its entry-time *Client snapshot; +// when a reconnect replaced the client mid-loop, unsubscribeLocked's +// identity guard turned the revocation into a no-op on the replacement. + +import ( + "context" + "encoding/json" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/coder/websocket" + + "github.com/owncord/server/auth" + "github.com/owncord/server/db" +) + +// demotedRoleID carries no permissions at all, so a member reassigned to it +// loses READ_MESSAGES on every channel. +const demotedRoleID = int64(201) + +func seedDemotedRole(t *testing.T, database *db.DB) { + t.Helper() + if _, err := database.ExecContext(context.Background(), + `INSERT INTO roles (id, name, color, permissions, position, is_default) + VALUES (?, 'harvest-demoted', NULL, 0, 4, 0)`, demotedRoleID); err != nil { + t.Fatalf("seed demoted role: %v", err) + } +} + +func reassignRole(t *testing.T, database *db.DB, uid, roleID int64) { + t.Helper() + if _, err := database.ExecContext(context.Background(), + `UPDATE users SET role_id = ? WHERE id = ?`, roleID, uid); err != nil { + t.Fatalf("reassign role: %v", err) + } +} + +func mustCreateTextChannel(t *testing.T, database *db.DB, name string) int64 { + t.Helper() + chID, err := database.CreateChannel(context.Background(), name, "text", "", "", 0) + if err != nil { + t.Fatalf("CreateChannel %s: %v", name, err) + } + return chID +} + +// dialAndAuth opens a WS connection against srv and sends the auth frame. +func dialAndAuth(t *testing.T, ctx context.Context, srvURL, token string, lastSeq uint64, activeChannelID int64) *websocket.Conn { + t.Helper() + dialCtx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + conn, dialResp, dialErr := websocket.Dial(dialCtx, "ws"+strings.TrimPrefix(srvURL, "http"), nil) + if dialResp != nil && dialResp.Body != nil { + _ = dialResp.Body.Close() + } + if dialErr != nil { + t.Fatalf("websocket.Dial: %v", dialErr) + } + payload := map[string]any{"token": token, "last_seq": lastSeq} + if activeChannelID != 0 { + payload["active_channel_id"] = activeChannelID + } + raw, _ := json.Marshal(map[string]any{"type": "auth", "payload": payload}) + if err := conn.Write(dialCtx, websocket.MessageText, raw); err != nil { + t.Fatalf("write auth: %v", err) + } + return conn +} + +// readFrameType reads one frame and returns its type field. +func readFrameType(t *testing.T, ctx context.Context, conn *websocket.Conn) (string, []byte) { + t.Helper() + readCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + _, msg, err := conn.Read(readCtx) + if err != nil { + t.Fatalf("read frame: %v", err) + } + var parsed struct { + Type string `json:"type"` + } + if err := json.Unmarshal(msg, &parsed); err != nil { + t.Fatalf("unmarshal frame %q: %v", msg, err) + } + return parsed.Type, msg +} + +func waitForRegisteredClient(t *testing.T, hub *Hub, uid int64) *Client { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for { + hub.mu.Lock() + c := hub.clients[uid] + hub.mu.Unlock() + if c != nil { + return c + } + if time.Now().After(deadline) { + t.Fatal("client was never registered") + } + time.Sleep(10 * time.Millisecond) + } +} + +func assertNotSubscribed(t *testing.T, hub *Hub, uid, chID int64, when string) { + t.Helper() + hub.mu.Lock() + c := hub.clients[uid] + hub.mu.Unlock() + if c != nil && c.getChannelID() == chID { + t.Errorf("%s: client kept focus on channel %d resolved from the stale role", when, chID) + } + deadline := time.Now().Add(2 * time.Second) + for { + hub.pubsub.mu.RLock() + sub := hub.pubsub.topics[ChannelTopic(chID)][uid] + hub.pubsub.mu.RUnlock() + if sub == nil { + return + } + if time.Now().After(deadline) { + t.Errorf("%s: client is still subscribed to ChannelTopic(%d) after the role reassignment — "+ + "every broadcast to that channel keeps reaching a socket whose role cannot read it", when, chID) + return + } + time.Sleep(10 * time.Millisecond) + } +} + +// refreshUserSnapshot is the primitive both handshake paths now call: it must +// replace the auth-time user row (and role name) with the current one. +func TestRefreshUserSnapshot_PicksUpRoleReassignment(t *testing.T) { + database := newHarvestVoiceDB(t) + seedDemotedRole(t, database) + ctx := context.Background() + uid := seedHarvestVoiceUser(t, database, "refresh-snapshot-user") + user, err := database.GetUserByID(ctx, uid) + if err != nil || user == nil { + t.Fatalf("GetUserByID: %v", err) + } + + hub := NewHub(database, auth.NewRateLimiter(), nil) + c := newClient(hub, nil, user, "", 0, ctx) + c.roleName = "harvest-voice" + + reassignRole(t, database, uid, demotedRoleID) + + if err := hub.refreshUserSnapshot(ctx, database, c); err != nil { + t.Fatalf("refreshUserSnapshot: %v", err) + } + if c.user.RoleID != demotedRoleID { + t.Errorf("c.user.RoleID = %d, want %d (stale auth-time snapshot kept)", c.user.RoleID, demotedRoleID) + } + if c.roleName != "harvest-demoted" { + t.Errorf("c.roleName = %q, want %q", c.roleName, "harvest-demoted") + } +} + +// A role reassignment landing mid-reconnect (inside the seqMu window the +// existing hook exposes) bumps the visibility watermark and forces the +// full-ready fallback — which must then resolve everything from the NEW role. +// Before the fix, handleFreshConnect reused the auth-time c.user, recomputed +// the same stale answer, and re-subscribed the revoked channel. +func TestFreshConnectFallback_RoleReassignMidReconnect_ResolvesFreshRole(t *testing.T) { + database := newHarvestVoiceDB(t) + seedDemotedRole(t, database) + ctx := context.Background() + uid := seedHarvestVoiceUser(t, database, "role-race-user") + chID := mustCreateTextChannel(t, database, "role-race-channel") + + token, err := auth.GenerateToken() + if err != nil { + t.Fatalf("GenerateToken: %v", err) + } + if _, err := database.CreateSession(ctx, uid, auth.HashToken(token), "test", "127.0.0.1"); err != nil { + t.Fatalf("CreateSession: %v", err) + } + + hub := NewHub(database, auth.NewRateLimiter(), nil) + go hub.Run() + defer hub.Stop() + + // Bracket last_seq so the resume takes the buffer tier and reaches the + // final mustFullResync re-check, where the hook fires. + rb := hub.ReplayBuffer() + rb.Push(98, chID, []byte(`{"seq":98,"type":"chat_message","payload":{}}`)) + rb.Push(99, chID, []byte(`{"seq":99,"type":"chat_message","payload":{}}`)) + rb.Push(100, chID, []byte(`{"seq":100,"type":"chat_message","payload":{}}`)) + hub.SeedSeq(100) + + var hookRan bool + handleReconnectPreRegisterRaceHook = func() { + if hookRan { + return + } + hookRan = true + reassignRole(t, database, uid, demotedRoleID) + // The real admin path: member_update fan-out + revocation pass. The + // revocation cannot reach this not-yet-registered socket — that is + // the hazard — but its watermark bump forces the full-ready fallback. + hub.BroadcastMemberUpdate(uid, "harvest-demoted") + } + defer func() { handleReconnectPreRegisterRaceHook = nil }() + + srv := httptest.NewServer(ServeWS(hub, database, []string{"*"}, 0)) + defer srv.Close() + + conn := dialAndAuth(t, ctx, srv.URL, token, 99, chID) + defer func() { _ = conn.Close(websocket.StatusNormalClosure, "") }() + + // The forced fallback writes auth_ok then ready. + if typ, _ := readFrameType(t, ctx, conn); typ != MsgTypeAuthOK { + t.Fatalf("expected auth_ok, got %v", typ) + } + typ, readyMsg := readFrameType(t, ctx, conn) + if typ != MsgTypeReady { + t.Fatalf("expected ready (forced fallback), got %v", typ) + } + if !hookRan { + t.Fatal("handleReconnectPreRegisterRaceHook never fired — not exercising the race window") + } + + // The ready payload must be the demoted role's view: no revoked channel. + var ready struct { + Payload struct { + Channels []struct { + ID int64 `json:"id"` + } `json:"channels"` + } `json:"payload"` + } + if err := json.Unmarshal(readyMsg, &ready); err != nil { + t.Fatalf("unmarshal ready: %v", err) + } + for _, ch := range ready.Payload.Channels { + if ch.ID == chID { + t.Errorf("ready payload contains channel %d, which the reassigned role cannot read", chID) + } + } + + waitForRegisteredClient(t, hub, uid) + assertNotSubscribed(t, hub, uid, chID, "post-fallback") +} + +// A reassignment landing in the residual window — after handleFreshConnect's +// own user re-read but before registerNow — finds the socket absent from +// h.clients (so the admin path's revocation pass early-returns) yet the +// inherited subscription is built from the pre-change read. The post-register +// re-read must close exactly this: either ordering ends with the revoked +// channel unsubscribed. +func TestFreshConnectFallback_RoleReassignPreRegister_PostRegisterVerifyRevokes(t *testing.T) { + database := newHarvestVoiceDB(t) + seedDemotedRole(t, database) + ctx := context.Background() + uid := seedHarvestVoiceUser(t, database, "role-race-late-user") + chID := mustCreateTextChannel(t, database, "role-race-late-channel") + + token, err := auth.GenerateToken() + if err != nil { + t.Fatalf("GenerateToken: %v", err) + } + if _, err := database.CreateSession(ctx, uid, auth.HashToken(token), "test", "127.0.0.1"); err != nil { + t.Fatalf("CreateSession: %v", err) + } + + hub := NewHub(database, auth.NewRateLimiter(), nil) + go hub.Run() + defer hub.Stop() + + rb := hub.ReplayBuffer() + rb.Push(98, chID, []byte(`{"seq":98,"type":"chat_message","payload":{}}`)) + rb.Push(99, chID, []byte(`{"seq":99,"type":"chat_message","payload":{}}`)) + rb.Push(100, chID, []byte(`{"seq":100,"type":"chat_message","payload":{}}`)) + hub.SeedSeq(100) + + // First hook: force the full-ready fallback WITHOUT touching the role, + // so the auth-hint promotion into c.channelID survives into + // handleFreshConnect. + handleReconnectPreRegisterRaceHook = func() { + hub.MarkVisibilityChanged() + } + defer func() { handleReconnectPreRegisterRaceHook = nil }() + + // Second hook: the reassignment lands after handleFreshConnect's re-read + // and before registerNow — the exact window the post-register verify + // exists for. + var lateHookRan bool + freshConnectPreRegisterRaceHook = func() { + if lateHookRan { + return + } + lateHookRan = true + reassignRole(t, database, uid, demotedRoleID) + hub.BroadcastMemberUpdate(uid, "harvest-demoted") + } + defer func() { freshConnectPreRegisterRaceHook = nil }() + + srv := httptest.NewServer(ServeWS(hub, database, []string{"*"}, 0)) + defer srv.Close() + + conn := dialAndAuth(t, ctx, srv.URL, token, 99, chID) + defer func() { _ = conn.Close(websocket.StatusNormalClosure, "") }() + + if typ, _ := readFrameType(t, ctx, conn); typ != MsgTypeAuthOK { + t.Fatalf("expected auth_ok, got %v", typ) + } + if typ, _ := readFrameType(t, ctx, conn); typ != MsgTypeReady { + t.Fatalf("expected ready (forced fallback), got %v", typ) + } + if !lateHookRan { + t.Fatal("freshConnectPreRegisterRaceHook never fired — not exercising the race window") + } + + waitForRegisteredClient(t, hub, uid) + assertNotSubscribed(t, hub, uid, chID, "post-register-verify") +} + +// revokeUnreadableChannels must act on the CURRENT holder of the user's +// slot: its per-topic DB round trips give a reconnect room to replace the +// *Client it looked up at entry, and unsubscribeLocked's identity guard +// makes an Unsubscribe on the stale pointer a silent no-op — stranding the +// replacement with the revoked topic. +func TestRevokeUnreadableChannels_ActsOnReplacementClient(t *testing.T) { + database := newHarvestVoiceDB(t) + seedDemotedRole(t, database) + ctx := context.Background() + uid := seedHarvestVoiceUser(t, database, "revoke-replaced-user") + chID := mustCreateTextChannel(t, database, "revoke-replaced-channel") + + user, err := database.GetUserByID(ctx, uid) + if err != nil || user == nil { + t.Fatalf("GetUserByID: %v", err) + } + + hub := NewHub(database, auth.NewRateLimiter(), nil) + c1 := newClient(hub, nil, user, "", 0, ctx) + c2 := newClient(hub, nil, user, "", 0, ctx) + + hub.mu.Lock() + hub.clients[uid] = c1 + hub.mu.Unlock() + hub.pubsub.Subscribe(c1, ChannelTopic(chID)) + + // The user is demoted; the revocation pass will find chID unreadable. + reassignRole(t, database, uid, demotedRoleID) + + // Mid-loop, a reconnect replaces the client — the replacement holds the + // topic (its own handshake subscribed it before the demotion was + // visible to it). + revokeUnreadableChannelsPreActRaceHook = func(int64) { + hub.mu.Lock() + hub.clients[uid] = c2 + hub.mu.Unlock() + hub.pubsub.Subscribe(c2, ChannelTopic(chID)) + } + defer func() { revokeUnreadableChannelsPreActRaceHook = nil }() + + hub.revokeUnreadableChannels(uid) + + hub.pubsub.mu.RLock() + sub := hub.pubsub.topics[ChannelTopic(chID)][uid] + hub.pubsub.mu.RUnlock() + if sub != nil { + t.Errorf("replacement client is still subscribed to ChannelTopic(%d): "+ + "the revocation acted on the stale snapshot and no-opped on the identity guard", chID) + } +} diff --git a/Server/ws/serve.go b/Server/ws/serve.go index 8672b373..6a97d447 100644 --- a/Server/ws/serve.go +++ b/Server/ws/serve.go @@ -158,6 +158,13 @@ func (h *Hub) upgradeAndAuth( // for the analogous races elsewhere in this package (OC-0206). var handleReconnectPreRegisterRaceHook func() +// freshConnectPreRegisterRaceHook, when non-nil, runs once inside +// handleFreshConnect after refreshUserSnapshot has re-read the user row but +// before registerNow. Test-only (nil in production); pins the +// role-reassignment-vs-handshake window (audit-2026-08-19 F-2) +// deterministically, same pattern as handleReconnectPreRegisterRaceHook. +var freshConnectPreRegisterRaceHook func() + // handleReconnect attempts to resume a client via replay. Its two return // values are independent signals for ServeWS: // - handled reports whether this function owns the outcome of the @@ -301,6 +308,18 @@ func (h *Hub) reconnectPrecheck( telemetry.NewAppMetrics().WSReconnectTierTotal.Add(ctx, 1, telemetry.String("tier", "full")) return nil, false } + // c.user is the auth-time snapshot; a role reassignment landing between + // authenticateConn and here would otherwise be resolved from the OLD + // RoleID for the rest of this socket's life — revokeUnreadableChannels + // cannot reach a mid-handshake socket (it early-returns when the user is + // not yet in h.clients), and nothing revalidates handshake-time + // subscriptions afterwards (audit-2026-08-19 F-2). Re-read the row so + // the permission set below is computed from the CURRENT role. + if err := h.refreshUserSnapshot(ctx, database, c); err != nil { + slog.Warn("ws handleReconnect: user re-read failed, falling back to full ready", + "user_id", c.userID, "err", err) + return nil, false + } // Compute the set of channel IDs the reconnecting user can access so that // channel-scoped replay events are filtered by current permissions (M3). allowedChannelIDs, err := h.computeAllowedChannels(ctx, database, c.user) @@ -312,6 +331,30 @@ func (h *Hub) reconnectPrecheck( return allowedChannelIDs, true } +// refreshUserSnapshot replaces c.user (and, when the role changed, c.roleName) +// with a fresh read of the user row. Handshake paths call it before +// registerNow, while c is still invisible to every other goroutine, so the +// plain field writes are safe. Fail closed: callers must not proceed on the +// stale snapshot when the re-read fails. +func (h *Hub) refreshUserSnapshot(ctx context.Context, database *db.DB, c *Client) error { + user, err := database.GetUserByID(ctx, c.userID) + if err != nil { + return fmt.Errorf("refreshUserSnapshot GetUserByID: %w", err) + } + if user == nil { + return fmt.Errorf("refreshUserSnapshot: user %d vanished", c.userID) + } + if user.RoleID != c.user.RoleID { + roleName := "member" + if role, roleErr := database.GetRoleByID(ctx, user.RoleID); roleErr == nil && role != nil { + roleName = strings.ToLower(role.Name) + } + c.roleName = roleName + } + c.user = user + return nil +} + // reconnectSelectReplay picks the tier that serves this resume — the ring // buffer when it still covers lastSeq, otherwise the cold-tier EventStore — and // returns the events found, the tier name, and (cold tier only) the persisted @@ -712,6 +755,17 @@ func (h *Hub) handleFreshConnect( h.freshConnectCleanStaleVoice(ctx, database, c, vs) } + // c.user is the auth-time snapshot — re-read it so the ready payload and + // any inherited subscriptions resolve from the user's CURRENT role, not + // the one they held when the auth frame was evaluated (audit-2026-08-19 + // F-2; the resume path does the same in reconnectPrecheck). Fail closed + // like the role lookup below. + if err := h.refreshUserSnapshot(ctx, database, c); err != nil { + slog.Error("ws: user re-read failed, disconnecting", "user_id", c.userID, "err", err) + _ = conn.Close(websocket.StatusInternalError, "user lookup failed") + return err + } + // Look up role for permission-filtered ready payload. // Fail closed: if the role lookup fails, disconnect rather than serving // a permissive ready payload with nil role (BUG-094). @@ -755,8 +809,28 @@ func (h *Hub) handleFreshConnect( c.channelID = 0 c.mu.Unlock() } + if freshConnectPreRegisterRaceHook != nil { + freshConnectPreRegisterRaceHook() + } h.registerNow(c, allowedChannelIDs) + // The re-read above and registerNow are not atomic: a role reassignment + // committing in between finds this socket absent from h.clients (so its + // revokeUnreadableChannels pass early-returns) yet builds our inherited + // subscriptions from the pre-change role. One PK re-read after + // registration makes the two orderings meet: a commit visible here is + // pruned by our own revoke pass, and a commit that is not yet visible + // necessarily runs its own revoke lookup after our registerNow and + // finds us. + // Scoped to the resume-fallback path — a pure fresh connect (lastSeq==0) + // inherits no subscriptions; channel_focus and voice_join re-check live. + if c.lastSeq > 0 { + if fresh, err := database.GetUserByID(ctx, c.userID); err != nil || fresh == nil || fresh.RoleID != c.user.RoleID { + //nolint:contextcheck // revokeUnreadableChannels takes no context by design (admin HubBroadcaster interface). + h.revokeUnreadableChannels(c.userID) + } + } + // Settle the session's status before buildReady reads the member list, so // the ready payload and the presence broadcast below cannot disagree. applyConnectStatus(ctx, database, c) diff --git a/Server/ws/serve_ready.go b/Server/ws/serve_ready.go index 5d5619ef..3aaa1ebd 100644 --- a/Server/ws/serve_ready.go +++ b/Server/ws/serve_ready.go @@ -311,8 +311,8 @@ func (h *Hub) readyVoiceStates(ctx context.Context, database *db.DB, channels [] // buildReady constructs the ready server→client message. // Per docs/protocol.md, channels include unread_count and last_message_id per -// user, and only protocol-specified fields (no slow_mode, archived, voice_* -// extras). +// user plus the channelPayloadFrom fields (slow_mode, nsfw, voice_* caps); +// archived is the one stored field deliberately not shipped. func (h *Hub) buildReady(ctx context.Context, database *db.DB, userID int64, role *db.Role) ([]byte, error) { channels, err := database.ListChannels(ctx) if err != nil { diff --git a/docs/api.md b/docs/api.md index fcdd3880..810c31b7 100644 --- a/docs/api.md +++ b/docs/api.md @@ -25,7 +25,7 @@ In mount order (`Server/api/router.go`): 5. **Request Logger** -- structured logging of method, path, status, duration. 6. **Telemetry HTTP middleware** -- OpenTelemetry tracing; a no-op unless the server was built with `-tags otel` and telemetry is enabled. 7. **SecurityHeadersWithTLS** -- (adds `Strict-Transport-Security` when TLS is on) sets `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `X-XSS-Protection: 0`, `Referrer-Policy: strict-origin-when-cross-origin`, `Content-Security-Policy: default-src 'self'`, `Permissions-Policy: camera=(), microphone=(), geolocation=()`, `Cache-Control: no-store`. -8. **MaxBodySize** -- 1 MiB default for all routes except `/api/v1/uploads` (which has its own 100 MiB limit). +8. **MaxBodySize** -- 1 MiB default for all routes except `/api/v1/uploads` (100 MiB), `/api/v1/admin/plugins/install` (16 MiB envelope), and `/api/v1/users/me/avatar` (2 MiB envelope). 9. **Coraza WAF** (optional) -- OWASP Core Rule Set request filtering, mounted only when `server.waf_enabled: true` (see `docs/server-configuration.md`). Note: chi's `middleware.RealIP` is deliberately **not** used -- client IPs are resolved from `X-Forwarded-For` only when the peer is listed in `server.trusted_proxies`. @@ -34,7 +34,8 @@ Note: chi's `middleware.RealIP` is deliberately **not** used -- client IPs are r ## Standard Error Response -All error responses use this JSON envelope: +Error responses use this JSON envelope (one exception: the plugin admin +endpoints return plain-text errors — see their section): ```json { @@ -52,10 +53,10 @@ All error responses use this JSON envelope: | `FORBIDDEN` | 403 | Insufficient permissions, banned account, or admin IP restriction | | `NOT_FOUND` | 404 | Resource (channel, message, user, invite, file, backup) not found | | `RATE_LIMITED` | 429 | Too many requests; response includes `Retry-After` header (seconds) | -| `INVALID_INPUT` / `BAD_REQUEST` | 400 | Malformed body, missing required fields, invalid query params | +| `INVALID_INPUT` / `BAD_REQUEST` | 400 | Malformed body, missing required fields, invalid query params, or an upload exceeding the size limit (oversize uploads are rejected 400, not 413; the only 413 in the API is the plugin-install endpoint's plain-text "plugin upload too large") | | `CONFLICT` | 409 | Duplicate username on register, or server already up-to-date on update | -| `TOO_LARGE` | 413 | File exceeds upload size limit | -| `SERVER_ERROR` / `INTERNAL` | 500 | Internal server error | +| `INTERNAL_ERROR` | 500 | Internal server error | +| `STORAGE_ERROR` | 507 | Upload could not be persisted (storage backend write failure) | | `BAD_GATEWAY` | 502 | Upstream failure (GitHub API, LiveKit, GIF provider, asset download) | | `GIF_DISABLED` | 503 | GIF proxy is not configured on this server (no `gif.api_key`) | @@ -116,7 +117,7 @@ See [GET /api/v1/auth/me](#get-apiv1authme) for the full user-object field table | 400 | `INVALID_CREDENTIALS` | Bad invite code, expired/revoked invite, or duplicate username | | 403 | `FORBIDDEN` | Registration is closed or unavailable while server-wide 2FA is required | | 429 | `RATE_LIMITED` | Exceeded 3 registrations/minute from this IP | -| 500 | `SERVER_ERROR` | Hashing failure, session creation failure, or DB error | +| 500 | `INTERNAL_ERROR` | Hashing failure, session creation failure, or DB error | --- @@ -178,7 +179,7 @@ If the account has TOTP enabled: | 401 | `UNAUTHORIZED` | Wrong username or password | | 403 | `FORBIDDEN` | Account is banned/suspended | | 429 | `RATE_LIMITED` | IP locked out after 10 consecutive failures (15 min cooldown) | -| 500 | `SERVER_ERROR` | Session creation failure | +| 500 | `INTERNAL_ERROR` | Session creation failure | --- @@ -226,7 +227,7 @@ See [GET /api/v1/auth/me](#get-apiv1authme) for the full user-object field table | ------ | ---- | ----- | | 400 | `INVALID_INPUT` | Malformed request body | | 401 | `UNAUTHORIZED` | Missing/expired challenge, invalid TOTP code, or challenge consumed | -| 500 | `SERVER_ERROR` | Session creation failure | +| 500 | `INTERNAL_ERROR` | Session creation failure | --- @@ -307,7 +308,7 @@ Account deleted successfully. All sessions, messages (soft-deleted), and associa | 400 | `INVALID_INPUT` | Missing or incorrect password | | 403 | `FORBIDDEN` | Cannot delete the last admin account | | 429 | `RATE_LIMITED` | Locked out after 3 failed password attempts (15 min cooldown) | -| 500 | `SERVER_ERROR` | Database error during deletion | +| 500 | `INTERNAL_ERROR` | Database error during deletion | --- @@ -404,6 +405,7 @@ event replaces the client's copy rather than patching it). | `avatar` | Optional. Must be an `https://` URL (max 512 chars) or `""` to clear. Upload a file instead with `POST /api/v1/users/me/avatar`. | | `display_name` | Optional, 1–32 characters. Shown instead of `username` everywhere; `""` clears it and falls back to the username. Rejected if it contains control or invisible (bidi-override) characters. | | `about` | Optional, max 300 characters. `""` clears it. | +| `identity_public_key` | Optional, base64, max 128 characters. Publishes the client's long-term E2EE identity public key for voice TOFU pinning (see [protocol.md](protocol.md), Voice End-to-End Encryption). | Omitting a field leaves it unchanged; sending `""` clears the nullable ones. `display_name` and `about` are HTML-sanitized and trimmed server-side, and the @@ -1442,6 +1444,14 @@ is deliberately not exposed here (anti-fingerprinting hardening, C-2). } ``` +When a health probe fails, the endpoint answers **503** with +`"status": "degraded"` and a `reason` field naming the failing subsystem — +`"hub"` (WS dispatch loop dead), `"database"`, or `"disk"`: + +```json +{ "status": "degraded", "reason": "database", "uptime": 86400, "online_users": 3 } +``` + --- ## Server Info @@ -1465,9 +1475,9 @@ unauthenticated endpoint (anti-fingerprinting hardening, C-2). ### GET /api/v1/metrics -Runtime server metrics. Restricted to admin-allowed CIDRs. +Runtime server metrics. IP-restricted (not token-based): allowed CIDRs come from `server.metrics_allowed_cidrs`, falling back to `server.admin_allowed_cidrs` when unset — set the dedicated key to admit a scraper without widening the admin perimeter. -**Auth:** Admin IP restriction (not token-based) +**Auth:** IP restriction (metrics CIDRs, admin fallback) ```json { @@ -2066,6 +2076,8 @@ re-verification against TOCTOU swaps), spawns the new process and shuts down. | ------ | ---- | ----- | | 503 | `CONTAINER_DEPLOYMENT` | Container deployment — the binary is image content; upgrade by pulling the new image (opt back in with `OWNCORD_CONTAINER=0` if the binary is bind-mounted) | | 503 | `UPDATE_UNAVAILABLE` | Update checking is not configured | +| 409 | `RESTART_PENDING` | A restart from an earlier apply/restore is already pending | +| 409 | `UPDATE_IN_PROGRESS` | Another restart-sensitive operation (update apply or backup restore) is running | | 409 | `NO_UPDATE` | Already up to date | | 502 | `UPDATE_CHECK_FAILED` / `MISSING_ASSETS` / `DOWNLOAD_FAILED` | Check, asset or download/verification failure | @@ -2388,6 +2400,15 @@ not open them). Plugin execution additionally requires a server built with `-tags wazero` and `plugins.enabled: true` in config. +Unlike the rest of the API, these endpoints answer errors as **plain text** +(`http.Error`), not the standard JSON envelope — the one envelope exception is +install's `400 INSTALL_FAILED`. When the runtime is unavailable, mutating +endpoints answer `503` `plugin runtime disabled`; other plain-text statuses +are `400` (bad multipart/zip), `415` (not a `.zip`), `413` (`plugin upload +too large` — the API's only 413), and `500`. `GET /api/v1/admin/plugins` +always answers `200` and reports the runtime state in an `X-Plugin-Runtime` +response header instead. + ### GET /api/v1/admin/plugins List installed plugins. @@ -2427,13 +2448,13 @@ These endpoints are only registered when LiveKit voice is configured. ### POST /api/v1/livekit/webhook -LiveKit webhook receiver. Uses LiveKit JWT verification. Admin-IP-restricted. Called by the LiveKit server, not by clients. +LiveKit webhook receiver. Uses LiveKit JWT verification. IP-restricted via `server.livekit_webhook_allowed_cidrs` (falls back to `server.admin_allowed_cidrs` when unset). Called by the LiveKit server, not by clients. ### GET /api/v1/livekit/health Check whether the LiveKit server is reachable. -**Auth:** Admin IP restriction +**Auth:** IP restriction (`server.livekit_webhook_allowed_cidrs`, admin fallback) #### Response 200 OK @@ -2469,8 +2490,8 @@ All requests to `/livekit/*` are reverse-proxied to the LiveKit server URL. The Returns connectivity diagnostics for debugging voice/network issues. -**Auth:** Required (any authenticated user) -**Rate limit:** 5 requests/minute per user +**Auth:** Required — `ADMINISTRATOR` only (H-8 hardening: the response reveals network topology) +**Rate limit:** 5 requests/minute per IP ```json { @@ -2482,7 +2503,7 @@ Returns connectivity diagnostics for debugging voice/network issues. }, "voice": { "enabled": true, - "livekit_url": "ws://localhost:7880", + "livekit_url": "localhost:7880", "livekit_health": true, "node_ip": "203.0.113.1", "proxy_path": "/livekit" diff --git a/docs/plans/audit-2026-08-19-remediation.md b/docs/plans/audit-2026-08-19-remediation.md new file mode 100644 index 00000000..d4f3f096 --- /dev/null +++ b/docs/plans/audit-2026-08-19-remediation.md @@ -0,0 +1,37 @@ +# Audit 2026-08-19 Remediation — Phased Plan + +**Status:** in progress 2026-08-19 — phases execute in order; each phase's +status is updated in place when it lands. +**Source:** [audit-2026-08-19.md](../audit-2026-08-19.md) — this plan executes +its §8 MUST-fix verdict and §9.1 fix order verbatim. Items outside that list +(§6 DEBT beyond D-01..D-05, §9.2 alpha-exit work) are deliberately NOT in +scope here; they stay tracked by the audit. +**Branch/PR:** `claude/repo-health-audit-s0xnyo` (restarted from `main` after +the audit-report PR #1395 merged), one commit per phase, single PR to `main`. + +## Phases + +| # | Closes | Change | Verification | Status | +|---|--------|--------|--------------|--------| +| 1 | F-5 | Give the `renderWindow >30-in-2s breaker` test in `tests/unit/message-list.test.ts` an explicit timeout so CI under load cannot produce a spurious red (it performs 30 synchronous 100-row jsdom rebuilds inside vitest's default 5 s) | run the file 3× | done 2026-08-20 | +| 2 | B-01..B-10, D-01..D-05 | Reference-doc refresh: schema.md (migrations 030/031, attachments `ON DELETE SET NULL`, index inventory, pool split, default-roles snapshot, dbgen preamble), protocol.md (DM/plugin_broadcast seq + replay tiers, retry_after, five "None" rate limits, E2EE prose + inner per-target cap, missing error codes, ready/member_join field gaps), api.md (diagnostics auth/limiter/example, error-code table, body-cap exemptions, identity_public_key, plugin text errors + header, /health 503, CIDR keys, restart-conflict 409s); stale comments (`serve_ready.go` buildReady, `tsconfig.e2e.json` + `ci.yml` spec counts, `logctx.go` stray word); three stale plan headers (bug-detection-improvements, security-scan-2026-07-22-remediation, discord-parity) | every edit re-checked against the cited code | done 2026-08-20 | +| 3 | F-3, F-4, D-16 | `slog.Warn` on the discarded errors: lockout Upsert/Delete/Cleanup (`auth/ratelimit.go`), `EvictOldestSessions` in `CreateSession` (`db/auth_queries.go`), `UpdateReadState` in `HandleChannelFocus` (`service/channel.go`) — mirrors the shipped OC-0061 pattern; in-memory behavior unchanged | unit tests pin the warn-and-continue contract | done 2026-08-20 | +| 4 | F-1 | Blocking a user evicts them from the pair's live 1:1 DM voice call via the existing `dmVoiceEvictor` seam `CloseDM` already exercises (group DMs stay exempt, matching `requireDMNotBlocked`) | failing-first service/API test | done 2026-08-20 | +| 5 | F-2 | Close the role-reassign/WS-handshake race: handshake paths re-read the user row instead of trusting the auth-time snapshot, and `revokeUnreadableChannels` re-resolves the live client before acting (mirrors `RefreshChannelVisibility`'s OC-0206 hazard notes) | failing-first ws tests + `-tags deadlock` run | done 2026-08-20 | +| 6 | F-6 | Remove the client's inert replay-dedup machinery (`replayDedup`, `isReplaying()`, the two dispatcher gates) — the server sends `auth_ok` before the burst, so the gates can never engage and their no-op behavior is the verified-correct behavior; rewrite the non-representative tests to pin the real frame ordering | client unit suite green | done 2026-08-20 (5036/5036) | +| 7 | — | `ci-check` local CI mirror, push, PR, drive green | CI | pending | + +## Decisions taken + +- **F-6 resolved by deletion, not repair.** The audit offered + delete-or-move-the-clear; the adversarial verification established the + gates' no-op behavior is the correct behavior for unread counts, so making + them fire would change behavior for the worse. The duplicate-voice-frame + window on resume (`serve.go` voice supplement) is benign — voice_state + application is idempotent — and is recorded in the audit, not patched here. +- **F-1 fixed at the mutation site, not the sweep.** Immediate eviction on + block matches `CloseDM`'s existing semantics and avoids adding a per-minute + DM/block query to the sweep for every voice state. +- **F-4 is log-only by design** — the audit's verifier established the cap + self-heals and persistent failures already abort the login; visibility is + the whole gap. diff --git a/docs/plans/bug-detection-improvements.md b/docs/plans/bug-detection-improvements.md index de0ca834..99e26674 100644 --- a/docs/plans/bug-detection-improvements.md +++ b/docs/plans/bug-detection-improvements.md @@ -1,7 +1,11 @@ # Bug-detection improvements — design Date: 2026-08-08 -Status: approved, not implemented +Status: partially implemented (verified 2026-08-19) — Tier 1a's `make fuzz` +target exists (`Server/Makefile`) and Tier 2's five custom ESLint rules +shipped 2026-08-08 (`Client/tauri-client/eslint-rules.js`), so the gap table +below is stale for those two rows; Tiers 1b/1c are on-demand npm scripts; +Tiers 3–4 remain unimplemented. ## Problem diff --git a/docs/plans/discord-parity.md b/docs/plans/discord-parity.md index 6c989986..db1314ca 100644 --- a/docs/plans/discord-parity.md +++ b/docs/plans/discord-parity.md @@ -10,8 +10,8 @@ > `permissions/checker.go:116-121`). Named leftovers remain open and are listed > in-line: role hoist/mentionable flags + `@RoleName` mentions (Phase 5), > categories as real entities (Phase 5), and the §"still-dead code" cleanup -> list (`sounds` table, `voice_speakers`, `voice_config.bitrate`, macOS PTT -> stub). +> list (`voice_speakers`, `voice_config.bitrate`, macOS PTT stub — the +> `sounds` table was dropped by migration 029 on 2026-08-04, A-2026-07-13). Status: phase 6 complete (2026-08-01) diff --git a/docs/plans/security-scan-2026-07-22-remediation.md b/docs/plans/security-scan-2026-07-22-remediation.md index fb4d858a..9b4bb0e0 100644 --- a/docs/plans/security-scan-2026-07-22-remediation.md +++ b/docs/plans/security-scan-2026-07-22-remediation.md @@ -5,9 +5,11 @@ > number is rendered (voice-roster shield badge title, `ChannelSidebar.ts:45-60`) > and the re-pin affordance exists (mismatch badge click → identity-mismatch > modal → `rePinPeerIdentity`, `ChannelSidebar.ts:84-135`). Follow-up 3 -> (`getIdentityPin` fail-open on a transient keyring read error, -> `identity.ts:106-118`) remains open; follow-up 4 is accepted behavior -> (degrades to *unverified*, never wrongly-*verified*). The scan artifact +> (`getIdentityPin` fail-open on a transient keyring read error) closed +> 2026-08-05 (DC-08): the lookup is now tri-state +> (pinned/unpinned/**unavailable**, `identity.ts` `getIdentityPin`) and +> `verifyPeerAnnounce` fails closed on "unavailable"; follow-up 4 is accepted +> behavior (degrades to *unverified*, never wrongly-*verified*). The scan artifact > directory `CLAUDE-SECURITY-20260722-184557/` referenced below is not part of > this repository. diff --git a/docs/protocol.md b/docs/protocol.md index 1d3be620..40ba96e6 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -95,7 +95,8 @@ The sequence number system enables reconnection with state recovery. | Channel broadcasts | Yes | `chat_message`, `chat_edited`, `chat_deleted`, `chat_bulk_deleted`, `reaction_update` | | Global broadcasts | Yes | `member_join`, `member_leave`, `member_update`, `member_ban`, `roles_update`, `emoji_update`, `voice_state`, `voice_leave`, `channel_create`, `channel_update`, `channel_delete`, `server_restart` | | Ephemeral | No | `typing`, `presence` from a `presence_update` (see below) | -| DM messages | No | DM `chat_message`, `chat_edited`, `chat_deleted`, `reaction_update`, `dm_channel_open`, `dm_channel_close` | +| DM chat events | Yes | DM `chat_message`, `chat_edited`, `chat_deleted`, `reaction_update` — sequenced and replayable exactly like channel broadcasts, delivered only to the DM's participants | +| DM lifecycle | No | `dm_channel_open`, `dm_channel_close` | | Call signalling | No | `call_incoming`, `call_declined` | | Direct responses | No | `auth_ok`, `auth_error`, `chat_send_ok`, `error`, `voice_config`, `voice_token`, `pong` | @@ -211,7 +212,7 @@ After `auth_ok`, the server sends a `ready` message containing all initial state The server broadcasts to all connected clients: ```json -{ "type": "member_join", "payload": { "user": { "id": 1, "username": "alex", "avatar": "uuid.png", "role": "admin" } } } +{ "type": "member_join", "payload": { "user": { "id": 1, "username": "alex", "avatar": "uuid.png", "role": "admin" }, "status": "online" } } { "type": "presence", "payload": { "user_id": 1, "status": "online", "custom_status": null } } ``` @@ -260,9 +261,12 @@ A visibility watermark forces the tier-3 full re-sync whenever channel visibility changed while the client was disconnected, so permission changes can never be replayed around. -DM events are not stored in the ring buffer; DM history persisted to the -`events` table is replayable via tier 2, and everything is always recoverable -via the full `ready` payload. +DM chat events (`chat_message`, `chat_edited`, `chat_deleted`, +`reaction_update` in DM channels) are sequenced into the same ring buffer and +`events` table as channel broadcasts, so they replay at tiers 1 and 2 — +filtered to the DM's participants. The unsequenced DM lifecycle events +(`dm_channel_open`/`dm_channel_close`) are not replayed; that state is always +recoverable via the full `ready` payload. --- @@ -319,9 +323,9 @@ to reconstruct them: 2. An `"invisible"` member is `"offline"` to everyone but themselves. The viewer's own entry carries their true status. -**voice_states[]:** All users currently in any voice channel: `channel_id`, `user_id`, `muted`, `deafened`, `server_muted`, `server_deafened` +**voice_states[]:** All users currently in any voice channel: `channel_id`, `user_id`, `username`, `muted`, `deafened`, `server_muted`, `server_deafened`, `speaking`, `camera`, `screenshare` -**roles[]:** All server roles with `id`, `name`, `color`, `permissions` (bitfield) +**roles[]:** All server roles with `id`, `name`, `color`, `permissions` (bitfield), `position`, `is_default` --- @@ -736,14 +740,19 @@ Sent when a user first connects (fresh connection, not reconnect replay). "role": "member", "display_name": "New User", "identity_public_key": "base64-identity-pubkey" - } + }, + "status": "online" } } ``` `display_name` is the nickname to render instead of `username`; omitted when unset. `identity_public_key` is the user's long-term E2EE identity public key -(see voice E2EE TOFU); omitted when the user has not published one. +(see voice E2EE TOFU); omitted when the user has not published one. The +top-level `status` is the viewer-safe presence the user comes online as (an +invisible connector reports `"offline"` here) — clients must render presence +from this field rather than assuming `"online"` because a `member_join` +arrived. ### member_update (Server -> Client, broadcast) @@ -1109,8 +1118,13 @@ ID in the channel). The joiner learns whether they are the key holder from `voice_token.is_key_holder`. When a participant leaves, the key holder rotates the room key so departed members cannot decrypt future media. -Both E2EE message types are rate limited at 5 per second per user. Key -material must be standard-alphabet base64 (padded or unpadded). +`voice_e2ee_announce` is rate limited at 5 per second per user. +`voice_e2ee_offer` has a higher outer budget of 64 per second per (sender, +voice channel) — the key holder fans one offer per peer on a rotation — plus +an inner cap of 5 per second per (sender, channel, target) so no single +recipient can be flooded (the W1-2 per-victim cap). Both answer +`RATE_LIMITED` when exceeded. Key material must be standard-alphabet base64 +(padded or unpadded). **Identity keys + TOFU:** each client holds a long-term ECDSA P-256 identity keypair, published via `PATCH /api/v1/users/me` (`identity_public_key`) and @@ -1393,10 +1407,12 @@ and the ringer's own 30s window already covers it. | Code | Description | |------|-------------| | `BAD_REQUEST` | Invalid payload format or field values | +| `BAD_PAYLOAD` | Structurally valid message with a field that fails validation (E2EE announce/offer key material, signatures, targets) | | `INTERNAL` | Server-side error | | `NOT_FOUND` | Channel or message not found | | `FORBIDDEN` | Missing required permission | -| `RATE_LIMITED` | Too many requests (includes `retry_after` in seconds) | +| `NOT_KEY_HOLDER` | `voice_e2ee_offer` sent by a participant who is not the channel's key holder | +| `RATE_LIMITED` | Too many requests (the error carries only `code` and `message`; REST 429s carry a `Retry-After` header, WS errors do not) | | `ALREADY_JOINED` | Already in this voice channel | | `CHANNEL_FULL` | Voice channel at capacity | | `VOICE_ERROR` | Voice-specific error | @@ -1433,10 +1449,17 @@ All rate limits are enforced server-side using a token bucket rate limiter. | Voice E2EE offer | 64 | 1 second | `RATE_LIMITED` error | | Voice moderation (mute/deafen/move/kick) | 5 | 1 second | `RATE_LIMITED` error | | Call ring | 1 | 3 seconds | `RATE_LIMITED` error | +| Call decline | 1 | 3 seconds | `RATE_LIMITED` error | +| Plugin command (`chat_command`) | 5 | 1 second | `RATE_LIMITED` error | +| Channel focus | 5 | 1 second | Silently dropped | +| Mark read | 5 | 1 second (own budget, separate from focus) | Silently dropped | +| Ping | 2 | 1 second | Silently dropped | The E2EE offer budget is deliberately higher than the announce budget: a key rotation fires one offer per peer in a single burst, so the limit is sized to -a whole rotation rather than to a single frame. +a whole rotation rather than to a single frame. Within that outer budget an +inner cap of 5 per second per (sender, channel, target) stops any single +recipient from being flooded. --- @@ -1458,8 +1481,8 @@ tables below add per-type behavioral notes. | `reaction_add` | 5/sec | | | `reaction_remove` | 5/sec | | | `typing_start` | 1/3sec/channel | Silently dropped | -| `channel_focus` | None | Updates read state | -| `mark_read` | None | Updates read state without moving focus | +| `channel_focus` | 5/sec (silently dropped) | Updates read state | +| `mark_read` | 5/sec, own budget (silently dropped) | Updates read state without moving focus | | `presence_update` | 1/10sec | | | `voice_join` | 5/sec | | | `voice_leave` | 5/sec | Empty payload | @@ -1473,11 +1496,11 @@ tables below add per-type behavioral notes. | `voice_mod_kick` | 5/sec | Requires MUTE_MEMBERS + outranks target | | `voice_token_refresh` | 1/60sec | Must be in voice | | `voice_e2ee_announce` | 5/sec | ECDH pubkey announce | -| `voice_e2ee_offer` | 64/sec | Wrapped room key to target (budgeted per key rotation) | +| `voice_e2ee_offer` | 64/sec outer, 5/sec per target | Wrapped room key to target (budgeted per key rotation) | | `call_ring` | 1/3sec | DM participants only; fans out as `call_incoming` | -| `call_decline` | None | DM participants only; fans out as `call_declined` | -| `chat_command` | None | Plugin slash command; max 64 args; broadcast gated by `CanPost` | -| `ping` | None | Heartbeat | +| `call_decline` | 1/3sec | DM participants only; fans out as `call_declined` | +| `chat_command` | 5/sec | Plugin slash command; max 64 args; broadcast gated by `CanPost` | +| `ping` | 2/sec (silently dropped) | Heartbeat | ### Server -> Client (39 types) @@ -1486,12 +1509,12 @@ tables below add per-type behavioral notes. | `auth_ok` | No | Direct | | `auth_error` | No | Direct (then close) | | `ready` | No | Direct | -| `chat_message` | Non-DM only | Channel or DM participants | +| `chat_message` | Yes | Channel or DM participants | | `chat_send_ok` | No | Direct to sender | -| `chat_edited` | Non-DM only | Channel or DM participants | -| `chat_deleted` | Non-DM only | Channel or DM participants | +| `chat_edited` | Yes | Channel or DM participants | +| `chat_deleted` | Yes | Channel or DM participants | | `chat_bulk_deleted` | Yes | Channel | -| `reaction_update` | Non-DM only | Channel or DM participants | +| `reaction_update` | Yes | Channel or DM participants | | `typing` | No | Channel (excl. sender) or DM | | `presence` | Yes | All clients | | `channel_create` | Yes | All clients | @@ -1521,7 +1544,7 @@ tables below add per-type behavioral notes. | `error` | No | Direct to requester | | `pong` | No | Direct to pinger | | `command_reply` | No | Direct to invoking client (ephemeral plugin reply) | -| `plugin_broadcast` | No | Channel (plugin output posted as a broadcast) | +| `plugin_broadcast` | Yes | Channel (plugin output posted as a broadcast; sequenced and replayable) | ### Plugin command types @@ -1532,6 +1555,6 @@ the generated constants cover them and `make protocol-verify` plus the | Type | Direction | Notes | |------|-----------|-------| -| `chat_command` | Client -> Server | `{command, args[], channel_id, req_id?}`; max 64 args; unknown commands return an `error`. No dedicated rate limit; a channel broadcast is gated by the same `CanPost` policy as a real message send. | +| `chat_command` | Client -> Server | `{command, args[], channel_id, req_id?}`; max 64 args; unknown commands return an `error`. Rate limited at 5/sec (`RATE_LIMITED`); a channel broadcast is gated by the same `CanPost` policy as a real message send. | | `command_reply` | Server -> Client | Ephemeral plugin reply, sent only to the invoking client; echoes `req_id`. Payload: `{text}`. | | `plugin_broadcast` | Server -> Client | Plugin output posted to a channel. Payload: `{channel_id, user_id, command, text}`. | diff --git a/docs/schema.md b/docs/schema.md index 806fb8bc..873393a1 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -2,12 +2,14 @@ OwnCord uses a single SQLite database file (`data/chatserver.db`) with the pure-Go driver `modernc.org/sqlite` (no CGO). Migrations run automatically on startup. -> **Data-access layers:** queries currently run as hand-written SQL in -> `Server/db`; an sqlc-generated layer (`Server/db/dbgen`, from -> `Server/db/queries/`) exists and is slated to become the real query layer -> per decision D2 in -> [plans/audit-2026-07-19-decisions.md](plans/audit-2026-07-19-decisions.md). -> See [architecture/data-model.md](architecture/data-model.md) for the full +> **Data-access layers:** most `Server/db` methods delegate to the +> sqlc-generated layer (`Server/db/dbgen`, from `Server/db/queries/`) per +> decision D2 in +> [plans/audit-2026-07-19-decisions.md](plans/audit-2026-07-19-decisions.md); +> a deliberate remainder (variable `IN` lists, FTS, multi-statement +> transactions) still runs as hand-written SQL — tracked in +> [plans/sqlc-adoption.md](plans/sqlc-adoption.md). See +> [architecture/data-model.md](architecture/data-model.md) for the full > picture. --- @@ -24,7 +26,12 @@ OwnCord uses a single SQLite database file (`data/chatserver.db`) with the pure- | `mmap_size` | `268435456` | 256 MB memory-mapped I/O | | `cache_size` | `-64000` | 64 MB page cache | -SQLite only allows one writer at a time. The connection pool is pinned to a single connection. +SQLite only allows one writer at a time. File-backed databases (the production +mode) therefore run a split pool: a single-connection writer pool +(`SetMaxOpenConns(1)`) plus a multi-connection read-only pool sized +`max(4, NumCPU)` and clamped to 1–64, configurable via `database.max_readers` +(`Server/db/db.go`). Only in-memory databases (tests) keep the historical +single shared connection. --- @@ -72,6 +79,8 @@ CREATE TABLE IF NOT EXISTS schema_versions ( | `027_user_profile_fields.sql` | Adds `users.display_name`, `users.about`, `users.custom_status`, and a partial index on `users(avatar)` for the file route's avatar-authorization probe | | `028_group_dms.sql` | Adds `channels.is_group` + a partial index — marks a DM channel as a group so group-ness survives people leaving | | `029_drop_sounds_table.sql` | Drops `sounds` — dead since 001; the soundboard it was created for was never built (A-2026-07-13) | +| `030_attachments_unlink_on_message_delete.sql` | Rebuilds `attachments` with `message_id ON DELETE SET NULL` (was CASCADE) — cascaded message deletes now unlink rows instead of removing them, so the periodic orphan sweep can still find and reclaim the stored files | +| `031_sessions_expiry_index.sql` | Normalizes legacy `sessions.expires_at` values to RFC3339 UTC and adds `idx_sessions_expires_at` so the 15-minute expiry sweep is sargable | --- @@ -92,14 +101,15 @@ CREATE TABLE roles ( ); ``` -**Default roles** (seeded by migration `001`, but no longer fixed — see -*Role semantics* below): +**Default roles** — current values after the full migration set (001 seeds +different masks: 005/007 raise Member's, 022 grants `MENTION_EVERYONE` to +Owner/Admin/Moderator). Not fixed at runtime — see *Role semantics* below: | id | name | color | permissions | position | Notes | |----|------|-------|-------------|----------|-------| | 1 | Owner | `#E74C3C` | `0x7FFFFFFF` | 100 | All 31 permission bits set | | 2 | Admin | `#F39C12` | `0x3FFFFFFF` | 80 | Everything except ADMINISTRATOR | -| 3 | Moderator | `#3498DB` | `0x000FFFFF` | 60 | All message + voice + moderation | +| 3 | Moderator | `#3498DB` | `0x002FFFFF` | 60 | All message + voice + moderation + mention-everyone | | 4 | Member | NULL | `0x1E63` | 40 | Send, read, attach, react, voice, video, screen share | **Role semantics:** @@ -417,7 +427,7 @@ Supports FTS5 query syntax: simple terms, phrase queries, prefix queries, boolea ```sql CREATE TABLE attachments ( id TEXT PRIMARY KEY, - message_id INTEGER REFERENCES messages(id) ON DELETE CASCADE, + message_id INTEGER REFERENCES messages(id) ON DELETE SET NULL, filename TEXT NOT NULL, stored_as TEXT NOT NULL, mime_type TEXT NOT NULL, @@ -432,6 +442,10 @@ CREATE TABLE attachments ( Uses UUID primary keys. `message_id` is NULL during upload, linked when the message is sent. `uploader_id` (added by migration 010) records who uploaded the file and backs the ownership check when attaching an upload to a message. +`ON DELETE SET NULL` (migration 030, was CASCADE) means a cascaded message +delete unlinks the row instead of removing it, leaving the periodic orphan +sweep (`DeleteOrphanedAttachments`) a handle on the stored file so the bytes +are reclaimed rather than stranded. --- @@ -716,24 +730,32 @@ CREATE TABLE plugin_kv ( | Index Name | Table | Columns | Purpose | |------------|-------|---------|---------| -| `idx_sessions_token` | sessions | `(token)` | Fast session lookup by token hash | | `idx_sessions_user` | sessions | `(user_id)` | Fast deletion of all sessions for a user | +| `idx_sessions_expires_at` | sessions | `(expires_at)` | Sargable 15-minute session-expiry sweep (031) | | `idx_messages_channel` | messages | `(channel_id, id DESC)` | Latest messages in channel query | | `idx_messages_user` | messages | `(user_id)` | Filter by author | -| `idx_invites_code` | invites | `(code)` | Fast invite validation | +| `idx_messages_pinned` | messages | `(channel_id, id DESC)` partial: `WHERE pinned = 1 AND deleted = 0` | Pinned-message listing without scanning channel history (019) | | `idx_audit_timestamp` | audit_log | `(created_at DESC)` | Pagination of audit log | | `idx_audit_log_actor` | audit_log | `(actor_id)` | Filter by actor | | `idx_login_ip` | login_attempts | `(ip_address, timestamp)` | Rate limiting queries | | `idx_voice_states_channel` | voice_states | `(channel_id)` | All users in a voice channel | -| `idx_channel_overrides_channel_role` | channel_overrides | `(channel_id, role_id)` | Permission lookup | +| `idx_channel_overrides_role` | channel_overrides | `(role_id, channel_id, allow, deny)` | Covering per-role override fetch (019; replaced `idx_channel_overrides_channel_role`, which duplicated the UNIQUE auto-index) | | `idx_dm_participants_user` | dm_participants | `(user_id)` | DM channel lookup | | `idx_attachments_uploader` | attachments | `(uploader_id)` | Upload-ownership checks | +| `idx_attachments_message` | attachments | `(message_id)` | Message → attachments fetch (019, recreated by 030's rebuild) | | `idx_user_blocks_blocked` | user_blocks | `(blocked_id, blocker_id)` | Reverse block lookup | | `idx_events_channel_seq` | events | `(channel_id, seq)` | Cold-tier replay per channel | | `idx_events_created_at` | events | `(created_at)` | Retention pruning | +| `idx_api_tokens_user` | api_tokens | `(user_id)` | Per-user token listing/revocation (018) | | `idx_message_mentions_user` | message_mentions | `(mentioned_user_id)` | Per-user mention lookup | | `idx_channel_user_overrides_user` | channel_user_overrides | `(user_id)` | "every override this member carries" — the direction the permission cache populates from (the PK covers the per-channel direction) | | `idx_roles_name_nocase` | roles | `(name COLLATE NOCASE)` UNIQUE | Case-insensitive role-name uniqueness | +| `idx_users_avatar` | users | `(avatar)` partial: `WHERE avatar IS NOT NULL` | File route's avatar-authorization probe (027) | +| `idx_channels_dm_group` | channels | `(is_group)` partial: `WHERE type = 'dm'` | Group-DM filtering (028) | + +Sessions are looked up by token and invites by code through their `UNIQUE` +auto-indexes; the duplicating `idx_sessions_token` / `idx_invites_code` were +dropped by migration 020. ---