J3vb/OwnCord
1
0
Fork 0
mirror of https://github.com/J3vb/OwnCord.git synced 2026-09-03 03:50:00 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: resolve the six blocked batch-4 ledger findings (#1393)

Browse Source 
* fix(ws): resolve an empty READ audience for a channel whose row is gone

channelReadAudience already failed closed on a GetChannel error; a
deleted channel returns (nil, nil) and fell through to the role scan.
Return nobody for a missing row too — voice teardown callers union the
room's participants and the leaver back in, so their signals still land.

Test locks both halves: the non-participant hears nothing, the leaver
still gets voice_leave. (OC-0090)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ws): re-elect the key holder in CleanupVoiceForChannel

Every other voice-removal path re-elects (finishVoiceLeave, the LiveKit
webhook, registerNow, rollbackVoiceJoin, sweepStaleVoiceStates); the
channel delete/archive path did not, so a torn-down channel's
voiceKeyHolders entry lived for the process lifetime. One updateKeyHolder
call at the end of the teardown deletes it. (OC-0012)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ws): implement BroadcastMemberUnban so unban reaches connected clients

The admin unban path reaches the hub through an optional-capability type
assertion that *ws.Hub never satisfied, so it always missed silently and
clients connected during a ban kept the user missing from their member
store. Implement the mirror of BroadcastMemberBan: fan out the same
member_join a fresh connect sends (clients already map it to addMember),
reporting offline since the unbanned user cannot be connected. A
compile-time assertion in admin pins the wiring so the assertion can
never silently miss again. (OC-0058)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(db): exclude the requester's own flag from the video-cap stream count

EnableCameraIfUnderLimit and EnableScreenshareIfUnderLimit counted every
stream in the channel including the very flag the UPDATE sets, so a user
whose server-side flag was already 1 (client lost track and retried) was
refused at the cap against their own stream, with no path out. Subtract
the outer row's own bit from the correlated count: re-enable becomes
idempotent while the requester's other stream and everyone else's still
count. sqlc layer regenerated. (OC-0081)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(ledger): resolve the six blocked batch-4 findings

Four fixed in this branch (OC-0012, OC-0058, OC-0081, OC-0090), each
with an independent revert-proof pass. Two were already fixed on main by
later sibling fixes and are recorded as such: OC-0086 by the OC-0017
pre-delete re-check (#1374), OC-0101 by the OC-0206 early watermark bump
(#1375). The ledger holds zero open and zero blocked findings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
J3vb
2026-08-19 18:28:00 +02:00
committed by GitHub
co-authored by Claude Fable 5
parent 8cf019c03f
commit c86d803a18
13 changed files with 560 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.superpowers/FINDINGS.md
Generated
+139 -129
View File
@@ -2,135 +2,7 @@
Generated by `render-ledger.mjs`. Do not hand-edit — edit `findings-ledger.json`.
**0 open** · 6 blocked · 180 fixed · 4 declined · 0 refuted · 1 duplicate
## Blocked — fix attempted, revert-proof failed
### OC-0012 — low — CleanupVoiceForChannel never clears voiceKeyHolders
`Server/ws/hub_sweep.go:290` · found 2026-08-09 · hunt `voice-e2ee-2026-08-09` · lens `keyholder-election`
Every other removal path re-elects (finishVoiceLeave, the LiveKit webhook, registerNow, rollbackVoiceJoin, sweepStaleVoiceStates). Channel delete and archive do not, leaving h.voiceKeyHolders[channelID] populated.
**Repro:** Delete a channel that had an elected holder. The map entry is never reachable and never freed — an unbounded per-deleted-channel leak for the process lifetime. On archive the next join's own updateKeyHolder overwrites it before any client can act, so there is no live desync.
**Evidence:** Server/ws/hub_sweep.go:290-346; contrast Server/ws/voice_leave.go:102
### OC-0058 — low — Unban emits no WS event — *ws.Hub does not implement memberUnbanBroadcaster
`Server/admin/handlers_users.go:169` · found 2026-08-12 · hunt `general-2026-08-12` · lens `state-desync`
The ban path calls hub.BroadcastMemberBan(id) directly (a method *ws.Hub really has), but the unban path routes through a type assertion to memberUnbanBroadcaster, and BroadcastMemberUnban exists nowhere on *ws.Hub (grep finds it only in this file and admin/handlers_users_broadcast_test.go). The assertion always misses, so the DB (users.banned=0, the user is back in ListMembers) and every already-connected client's membersStore — which hard-deleted the row on member_ban — permanently disagree.
**Repro:** Admin bans user U: BroadcastMemberBan fans member_ban out, every connected client runs removeMember(U) and drops U from membersStore, and U's socket is kicked. Admin then unbans U via PATCH /api/v1/admin/users/{id} {"banned": false}. ModerationService.UnbanUser commits, then the `case !*req.Banned && hub != nil` branch type-asserts and silently does nothing. U is absent from the member list, from mention autocomplete and from getTypingUsers on every client that was connected during the ban, while any client that connects afterwards gets U in its ready payload — two clients side by side showing different rosters. It only converges for the stale clients if U reconnects (handleFreshConnect broadcasts member_join) or they reconnect themselves; an unbanned user who never comes back online stays missing indefinitely. admin/handlers_users_broadcast_test.go:51 asserts the call happens against a double that implements the interface, so the suite stays green.
**Evidence:** case *req.Banned && hub != nil:
hub.BroadcastMemberBan(id) // real method on *ws.Hub
case !*req.Banned && hub != nil:
if mub, ok := hub.(memberUnbanBroadcaster); ok {
mub.BroadcastMemberUnban(id) // *ws.Hub has no such method — assertion always false
}
**Suggested fix:** Implement `func (h *Hub) BroadcastMemberUnban(userID int64)` on *ws.Hub that loads the user and role from h.db and calls h.BroadcastToAll(buildMemberJoin(user, roleName)) — the client already maps member_join to addMember, so no protocol change is needed. Add a compile-time `var _ memberUnbanBroadcaster = (*ws.Hub)(nil)` where admin is wired to the real hub so the assertion cannot silently miss again.
### OC-0081 — low — voice_max_video cap counts the requester's own camera row, so a user whose server-side camera flag is already 1 can never re-enable
`Server/db/queries/sqlite/voice.sql:92` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-ws`
EnableCameraIfUnderLimit's guard subquery counts every camera=1 row in the channel, including the very row the UPDATE targets. An enable request from a user whose row already has camera=1 therefore needs maxVideo-1 other publishers to pass, so at the cap it is refused against the requester's own stream. The zero-rows result is also indistinguishable from "no voice_states row for this channel", and handleVoiceCameraV2 maps both to VIDEO_LIMIT "maximum N video streams reached".
**Repro:** Channel with voice_max_video = 1. User A enables their camera: COUNT(camera=1)=0 < 1, row updated to camera=1. A's client-side localCamera then falls out of sync with the row while the row stays 1 — the confirmed enableCamera supersession gap (Client/tauri-client/src/lib/screenShare.ts:243) does exactly this: a disableCamera that lands during publishTrack resets localCamera to false but the server row keeps camera=1. A now presses the camera button (VoiceCallbacks.ts:110 computes next = !localCamera = true) and the server runs EnableCameraIfUnderLimit(A, ch, 1): the subquery counts A's own row, 1 < 1 is false, 0 rows affected, ok=false. A receives VIDEO_LIMIT "maximum 1 video streams reached" while being the only video publisher in the room, and every retry repeats it — there is no path that clears camera back to 0 except A sending voice_camera{enabled:false}, which the UI will not do because it believes the camera is already off.
**Evidence:** Server/db/queries/sqlite/voice.sql:90-92 — `UPDATE voice_states SET camera = 1 WHERE voice_states.user_id = ? AND voice_states.channel_id = ? AND (SELECT COUNT(*) FROM voice_states AS vs2 WHERE vs2.channel_id = ? AND vs2.camera = 1) < ?;` (no `AND vs2.user_id <> ?` exclusion, and no `AND camera = 0` on the outer UPDATE). Consumed at Server/ws/voice_controls.go:116 `ok, limitErr := d.DB.EnableCameraIfUnderLimit(ctx, userID, voiceChID, ch.VoiceMaxVideo)` with the refusal at voice_controls.go:121-126 returning ErrCodeVideoLimit.
**Suggested fix:** Exclude the requester's own row from the count in EnableCameraIfUnderLimit: change the subquery to `WHERE vs2.channel_id = ? AND vs2.camera = 1 AND vs2.user_id <> voice_states.user_id` (or bind userID again with `AND vs2.user_id <> ?`), then regenerate the sqlc layer via the db-change workflow. This makes re-enable idempotent while still refusing a genuinely new publisher at the cap.
### OC-0086 — low — sweepStaleVoiceStates classifies ghost rows from a snapshot and then deletes them without re-checking, so an in-flight voice_join is ejected from the SFU while the hub still believes the user is in the room
`Server/ws/hub_sweep.go:220` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-ws`
The ghost loop decides staleness under one h.mu.RLock (lines 202-218) and then acts on that decision in a second, much longer loop that performs a DB delete, a channelReadAudience resolution, a broadcast and a LiveKit RemoveParticipant (5s timeout) per entry. The LeaveVoiceChannelIfMatch guard only protects against the user having moved to a *different* row — it matches the very row the in-flight join just created, because joined_at is identical. The revocation loop in the same function (lines 182-187) applies exactly the opposite discipline via handleVoiceLeaveIfStillIn, with a comment explaining why ("a voice_join to a still-permitted channel may have committed while it ran"); that reasoning was never carried into the ghost loop below it.
**Repro:** Voice channel V already holds several genuine ghost rows (e.g. after a server restart, or several crashed clients), so the `stale` loop has work to do and each iteration costs a channelReadAudience resolution plus a LiveKit RemoveParticipant with a 5s timeout.
1. The 60s voiceSweepTicker fires. sweepStaleVoiceStates reads GetAllVoiceStates and takes the h.mu.RLock snapshot at T0.
2. At T0, user X's voice_join has committed its row via JoinVoiceChannel (voice_join.go:196) but has not yet reached c.setVoiceState (voice_join.go:222) — one GetVoiceState round trip away. X's client still reports getVoiceChID() == 0, so X's row is appended to `stale`.
3. X's join then completes normally: setVoiceState, Subscribe(VoiceTopic(V)), updateKeyHolder(V), broadcastVoiceEvent(voice_state) — every other client is told X joined, and X's client connects to the SFU.
4. Seconds later the sweep reaches X's entry. LeaveVoiceChannelIfMatch(X, V, joinedAt) matches — it is literally the row X's join created — and deletes it. voice_leave is broadcast for X. RemoveParticipant(V, X, joinedAt) uses the same identity and kicks X out of the LiveKit room.
5. Nothing clears X's in-memory state: c.voiceChID is still V and c.voiceJoinToken is still joinedAt, and X remains subscribed to VoiceTopic(V).
Outcome: X is silently ejected from the SFU seconds after joining while the hub and X's own client both still believe X is in voice. updateKeyHolder(V) at line 245 scans h.clients, sees X's live voiceChID == V, and can elect X key holder for a room X is not in — every other participant's voice_e2ee_offer is then rejected with NOT_KEY_HOLDER. X's voice_mute/voice_camera writes hit zero rows and voiceStateBroadcast returns Result{} (state == nil), so the toggles silently no-op, and no later sweep can heal it because the sweep only iterates DB rows.
**Evidence:** hub_sweep.go:202-249
h.mu.RLock()
var stale []struct{ userID, channelID int64; joinedAt string }
for _, vs := range allStates {
c, ok := h.clients[vs.UserID]
if !ok || c.getVoiceChID() != vs.ChannelID { // <-- decided here, under one RLock
stale = append(stale, ...)
}
}
h.mu.RUnlock()
for _, s := range stale { // <-- acted on here, seconds later, no re-check
deleted, err := h.db.LeaveVoiceChannelIfMatch(ctx, s.userID, s.channelID, s.joinedAt)
...
h.broadcastVoiceEvent(ctx, s.channelID, buildVoiceLeave(s.channelID, s.userID))
h.updateKeyHolder(s.channelID)
if h.livekit != nil { _ = h.livekit.RemoveParticipant(ctx, s.channelID, s.userID, s.joinedAt) }
}
contrast, same function, hub_sweep.go:181-187
// The permission check is a DB round-trip; a voice_join to a
// still-permitted channel may have committed while it ran. The
// eviction is conditional on the client still being in the checked
// channel — never on whatever channel it is in by now.
if !h.handleVoiceLeaveIfStillIn(ctx, c, chID) { continue }
the window being raced, voice_join.go:196-222
if err := h.db.JoinVoiceChannel(ctx, c.userID, channelID); ... // row committed
state, err := h.db.GetVoiceState(ctx, c.userID) // still voiceChID == 0 here
...
c.setVoiceState(channelID, state.JoinedAt) // only now non-zero
**Suggested fix:** In the ghost-classification loop (hub_sweep.go:208-217), add the grace-period guard the codebase's own cross_batch note prescribes: parse vs.JoinedAt and `continue` for rows younger than a grace window (e.g. 2x the 60s sweep interval), so a just-committed join can never be classified as a ghost regardless of when c.setVoiceState lands.
### OC-0090 — low — channelReadAudience falls through to a server-wide role scan when the channel row is gone, leaking a private group-DM's voice_leave to every connected member
`Server/ws/hub_broadcast.go:149` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-ws`
The DM-audience branch is guarded on `ch != nil && ch.Type == "dm"`. A deleted channel yields `ch == nil` (db.GetChannel returns (nil, nil) on sql.ErrNoRows), so control drops into the generic role scan, which — for an id whose channel row and ON DELETE CASCADE'd channel_overrides no longer exist — resolves to "every connected user whose base role holds READ_MESSAGES". The read error above is deliberately failed closed; the missing-row case is not.
**Repro:** Group DM #G with alice, bob, carol. bob and carol leave. alice is alone in #G and is in its voice call (voice_states row + hub voiceChID set). alice calls DELETE /api/v1/dms/G. service.CloseDM -> db.LeaveGroupDM removes the last dm_participants row and, with remaining == 0, deletes the channels row, returning ChannelDeleted=true. api/dm_handler.go:236 then calls hub.DisconnectFromVoiceInChannel(alice, G) -> handleVoiceLeaveIfStillIn -> finishVoiceLeave (Server/ws/voice_leave.go:76) -> h.channelReadAudience(ctx, G). GetChannel(G) now returns (nil, nil), the `ch.Type == "dm"` branch is skipped, and the loop at hub_broadcast.go:171-178 calls h.perms.HasChannelPerm(uid, G, ReadMessages) for every connected user — with no override rows left for G, every ordinary member passes. Each of them receives {"type":"voice_leave","payload":{"channel_id":G,"user_id":alice}} for a private conversation they were never part of. The live-row form of exactly this leak is locked shut by Server/ws/voice_dm_access_test.go:179 (TestVoiceJoin_DMCall_VoiceStateNotLeakedToThirdConnectedUser); the nil-row branch bypasses it.
**Evidence:** ch, err := h.db.GetChannel(ctx, channelID)
if err != nil {
slog.Error("ws: channelReadAudience GetChannel failed, denying", ...)
return []int64{}
}
if ch != nil && ch.Type == "dm" { // <- nil (deleted) channel skips the DM audience entirely
participantIDs, err := h.db.GetDMParticipantIDs(ctx, channelID)
...
return audience
}
...
for _, uid := range userIDs {
if h.perms.HasChannelPerm(ctx, uid, channelID, permissions.ReadMessages) {
audience = append(audience, uid)
}
}
**Suggested fix:** In channelReadAudience (Server/ws/hub_broadcast.go), fail closed on a missing row the same way the lookup-error branch does: after the `if err != nil` block, add `if ch == nil { return []int64{} }` before the `ch.Type == "dm"` check. This is safe for every caller: finishVoiceLeave (voice_leave.go:88-98) and CleanupVoiceForChannel (hub_sweep.go:333-342) both union the room's remaining participants and the leaver into the audience after the call, so eviction/E2EE-teardown signals still reach everyone entitled to them, while non-participants get nothing.
### OC-0101 — medium — Channel-visibility watermark is bumped after the fan-out loop, so a client reconnecting during the loop replays past the change and never converges
`Server/ws/hub_broadcast.go:365` · found 2026-08-13 · hunt `2026-08-13-postopt` · lens `ws-hub`
RefreshChannelVisibility delivers the visibility change as targeted, unsequenced channel_create/channel_delete frames to a snapshot of clients taken at line 252, and only calls bumpVisibilityWatermark() at line 365 after the whole per-client loop finishes. A client that reconnects while the loop is still running is in neither audience: it was not in the snapshot, and handleReconnect's mustFullResync check (serve.go:125) reads the still-unbumped watermark and admits it to the replay path, which by construction cannot carry an unsequenced targeted message. revokeUnreadableChannels has the identical gap via `defer h.bumpVisibilityWatermark()` at hub_broadcast.go:497.
**Repro:** Server has N connected clients. Admin edits a channel_override on channel C (or edits a role, which drives RefreshAllChannelVisibility over every channel). RefreshChannelVisibility(C) snapshots h.clients at hub_broadcast.go:252-257, then loops lines 315-357 running 1-4 permission lookups plus a sendMsg + pubsub mutation per client. User U is offline at snapshot time (or drops immediately after) and reconnects mid-loop with last_seq = L, where L exceeds the previous watermark. handleReconnect evaluates mustFullResync(L) against the unbumped visibilityChangeSeq -> false -> U resumes from the ring buffer, never receives a ready payload, and never receives the targeted channel_create/channel_delete (it was not in the snapshot). Line 365 bumps the watermark only after U's handshake has already committed to replay. Revoked case: U's sidebar keeps rendering channel C forever, and clicking it is dead because registerNow's READ-gated re-subscribe correctly refused the topic. Granted case: U's computeAllowedChannels includes C so chat_message frames for C start arriving for a channel the client has no entry for. Neither converges until U reconnects with last_seq=0 or an unrelated visibility change pushes the watermark past L.
**Evidence:** hub_broadcast.go:252 `h.mu.RLock(); clients := make([]*Client, 0, len(h.clients))` … loop 315-357 … 365 `h.bumpVisibilityWatermark()` // comment at 359-364: "Clients not connected right now missed the targeted sends above. Move the watermark so any resume from a seq at or before this point is forced onto the full-ready path" — but the bump happens after, not before, the sends. Same shape at hub_broadcast.go:497 `defer h.bumpVisibilityWatermark()`.
**Suggested fix:** Bump before the audience is snapshotted instead of after it is served, in the one place each: in RefreshChannelVisibility move h.bumpVisibilityWatermark() from line 365 to immediately before the h.mu.RLock() snapshot at line 252; in revokeUnreadableChannels drop the `defer` on line 497 so the existing first-statement call runs eagerly (it already sits at the top, so early returns stay covered). That shrinks the hole from 'the whole fan-out loop' to the few instructions between the bump and the RLock. Residual, if you want it fully closed: re-check h.mustFullResync(lastSeq) inside the h.seqMu section right before h.registerNow in serve.go and fall back to full ready.
**0 open** · 0 blocked · 186 fixed · 4 declined · 0 refuted · 1 duplicate
## Fixed
@@ -242,6 +114,18 @@ The signed announce message is domain || userId || ephemeralPubRaw with no chann
**Fixed:** `8787b906` · test `Client/tauri-client/tests/unit/livekit-e2ee.test.ts` · revert-proof pass
### OC-0012 — low — CleanupVoiceForChannel never clears voiceKeyHolders
`Server/ws/hub_sweep.go:290` · found 2026-08-09 · hunt `voice-e2ee-2026-08-09` · lens `keyholder-election`
Every other removal path re-elects (finishVoiceLeave, the LiveKit webhook, registerNow, rollbackVoiceJoin, sweepStaleVoiceStates). Channel delete and archive do not, leaving h.voiceKeyHolders[channelID] populated.
**Repro:** Delete a channel that had an elected holder. The map entry is never reachable and never freed — an unbounded per-deleted-channel leak for the process lifetime. On archive the next join's own updateKeyHolder overwrites it before any client can act, so there is no live desync.
**Evidence:** Server/ws/hub_sweep.go:290-346; contrast Server/ws/voice_leave.go:102
**Fixed:** `6f3485e7` · test `TestCleanupVoiceForChannel_ClearsKeyHolder` · revert-proof pass
### OC-0013 — high — REST DM events never bump the visibility watermark — *ws.Hub does not implement dmVisibilityMarker
`Server/api/dm_handler.go:45` · found 2026-08-12 · hunt `general-2026-08-12` · lens `state-desync`
@@ -1300,6 +1184,25 @@ showContextMenu({ x: e.clientX, y: e.clientY, items, signal, className: "dm-cont
**Fixed:** `c3837fa` · test `Client/tauri-client/tests/unit/context-menu.test.ts` · revert-proof self-reported
### OC-0058 — low — Unban emits no WS event — *ws.Hub does not implement memberUnbanBroadcaster
`Server/admin/handlers_users.go:169` · found 2026-08-12 · hunt `general-2026-08-12` · lens `state-desync`
The ban path calls hub.BroadcastMemberBan(id) directly (a method *ws.Hub really has), but the unban path routes through a type assertion to memberUnbanBroadcaster, and BroadcastMemberUnban exists nowhere on *ws.Hub (grep finds it only in this file and admin/handlers_users_broadcast_test.go). The assertion always misses, so the DB (users.banned=0, the user is back in ListMembers) and every already-connected client's membersStore — which hard-deleted the row on member_ban — permanently disagree.
**Repro:** Admin bans user U: BroadcastMemberBan fans member_ban out, every connected client runs removeMember(U) and drops U from membersStore, and U's socket is kicked. Admin then unbans U via PATCH /api/v1/admin/users/{id} {"banned": false}. ModerationService.UnbanUser commits, then the `case !*req.Banned && hub != nil` branch type-asserts and silently does nothing. U is absent from the member list, from mention autocomplete and from getTypingUsers on every client that was connected during the ban, while any client that connects afterwards gets U in its ready payload — two clients side by side showing different rosters. It only converges for the stale clients if U reconnects (handleFreshConnect broadcasts member_join) or they reconnect themselves; an unbanned user who never comes back online stays missing indefinitely. admin/handlers_users_broadcast_test.go:51 asserts the call happens against a double that implements the interface, so the suite stays green.
**Evidence:** case *req.Banned && hub != nil:
hub.BroadcastMemberBan(id) // real method on *ws.Hub
case !*req.Banned && hub != nil:
if mub, ok := hub.(memberUnbanBroadcaster); ok {
mub.BroadcastMemberUnban(id) // *ws.Hub has no such method — assertion always false
}
**Suggested fix:** Implement `func (h *Hub) BroadcastMemberUnban(userID int64)` on *ws.Hub that loads the user and role from h.db and calls h.BroadcastToAll(buildMemberJoin(user, roleName)) — the client already maps member_join to addMember, so no protocol change is needed. Add a compile-time `var _ memberUnbanBroadcaster = (*ws.Hub)(nil)` where admin is wired to the real hub so the assertion cannot silently miss again.
**Fixed:** `d2289560` · test `TestBroadcastMemberUnban_FansOutMemberJoin` · revert-proof pass
### OC-0059 — low — Composer slow-mode cooldown is applied to whichever channel happens to be mounted when a chat_send_ok/SLOW_MODE frame arrives, not the channel the message was actually sent to
`Client/tauri-client/src/pages/main-page/ChannelController.ts:450` · found 2026-08-12 · hunt `general-2026-08-12` · lens `state-desync`
@@ -1783,6 +1686,20 @@ roomEventHandlers.ts's handleDisconnected (line 184) calls deps.teardownForRecon
**Fixed:** `7be9ccd2` · test `Client/tauri-client/tests/unit/livekit-session.test.ts` · revert-proof pass
### OC-0081 — low — voice_max_video cap counts the requester's own camera row, so a user whose server-side camera flag is already 1 can never re-enable
`Server/db/queries/sqlite/voice.sql:92` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-ws`
EnableCameraIfUnderLimit's guard subquery counts every camera=1 row in the channel, including the very row the UPDATE targets. An enable request from a user whose row already has camera=1 therefore needs maxVideo-1 other publishers to pass, so at the cap it is refused against the requester's own stream. The zero-rows result is also indistinguishable from "no voice_states row for this channel", and handleVoiceCameraV2 maps both to VIDEO_LIMIT "maximum N video streams reached".
**Repro:** Channel with voice_max_video = 1. User A enables their camera: COUNT(camera=1)=0 < 1, row updated to camera=1. A's client-side localCamera then falls out of sync with the row while the row stays 1 — the confirmed enableCamera supersession gap (Client/tauri-client/src/lib/screenShare.ts:243) does exactly this: a disableCamera that lands during publishTrack resets localCamera to false but the server row keeps camera=1. A now presses the camera button (VoiceCallbacks.ts:110 computes next = !localCamera = true) and the server runs EnableCameraIfUnderLimit(A, ch, 1): the subquery counts A's own row, 1 < 1 is false, 0 rows affected, ok=false. A receives VIDEO_LIMIT "maximum 1 video streams reached" while being the only video publisher in the room, and every retry repeats it — there is no path that clears camera back to 0 except A sending voice_camera{enabled:false}, which the UI will not do because it believes the camera is already off.
**Evidence:** Server/db/queries/sqlite/voice.sql:90-92 — `UPDATE voice_states SET camera = 1 WHERE voice_states.user_id = ? AND voice_states.channel_id = ? AND (SELECT COUNT(*) FROM voice_states AS vs2 WHERE vs2.channel_id = ? AND vs2.camera = 1) < ?;` (no `AND vs2.user_id <> ?` exclusion, and no `AND camera = 0` on the outer UPDATE). Consumed at Server/ws/voice_controls.go:116 `ok, limitErr := d.DB.EnableCameraIfUnderLimit(ctx, userID, voiceChID, ch.VoiceMaxVideo)` with the refusal at voice_controls.go:121-126 returning ErrCodeVideoLimit.
**Suggested fix:** Exclude the requester's own row from the count in EnableCameraIfUnderLimit: change the subquery to `WHERE vs2.channel_id = ? AND vs2.camera = 1 AND vs2.user_id <> voice_states.user_id` (or bind userID again with `AND vs2.user_id <> ?`), then regenerate the sqlc layer via the db-change workflow. This makes re-enable idempotent while still refusing a genuinely new publisher at the cap.
**Fixed:** `6a5a3a7c` · test `TestVoice_EnableCameraIfUnderLimit_ReEnableIdempotentAtCap` · revert-proof pass
### OC-0082 — low — Pinning a soft-deleted message returns HTTP 500: SetMessagePinned leaks db.ErrNotFound unwrapped, and lacks the deleted-message guard its siblings have
`Server/service/message_query.go:227` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-service`
@@ -1877,6 +1794,56 @@ setAudioVolumeHost(apiConfig.host ?? null);
**Fixed:** `8787b906` · test `Client/tauri-client/tests/unit/sidebar-area.test.ts` · revert-proof pass
### OC-0086 — low — sweepStaleVoiceStates classifies ghost rows from a snapshot and then deletes them without re-checking, so an in-flight voice_join is ejected from the SFU while the hub still believes the user is in the room
`Server/ws/hub_sweep.go:220` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-ws`
The ghost loop decides staleness under one h.mu.RLock (lines 202-218) and then acts on that decision in a second, much longer loop that performs a DB delete, a channelReadAudience resolution, a broadcast and a LiveKit RemoveParticipant (5s timeout) per entry. The LeaveVoiceChannelIfMatch guard only protects against the user having moved to a *different* row — it matches the very row the in-flight join just created, because joined_at is identical. The revocation loop in the same function (lines 182-187) applies exactly the opposite discipline via handleVoiceLeaveIfStillIn, with a comment explaining why ("a voice_join to a still-permitted channel may have committed while it ran"); that reasoning was never carried into the ghost loop below it.
**Repro:** Voice channel V already holds several genuine ghost rows (e.g. after a server restart, or several crashed clients), so the `stale` loop has work to do and each iteration costs a channelReadAudience resolution plus a LiveKit RemoveParticipant with a 5s timeout.
1. The 60s voiceSweepTicker fires. sweepStaleVoiceStates reads GetAllVoiceStates and takes the h.mu.RLock snapshot at T0.
2. At T0, user X's voice_join has committed its row via JoinVoiceChannel (voice_join.go:196) but has not yet reached c.setVoiceState (voice_join.go:222) — one GetVoiceState round trip away. X's client still reports getVoiceChID() == 0, so X's row is appended to `stale`.
3. X's join then completes normally: setVoiceState, Subscribe(VoiceTopic(V)), updateKeyHolder(V), broadcastVoiceEvent(voice_state) — every other client is told X joined, and X's client connects to the SFU.
4. Seconds later the sweep reaches X's entry. LeaveVoiceChannelIfMatch(X, V, joinedAt) matches — it is literally the row X's join created — and deletes it. voice_leave is broadcast for X. RemoveParticipant(V, X, joinedAt) uses the same identity and kicks X out of the LiveKit room.
5. Nothing clears X's in-memory state: c.voiceChID is still V and c.voiceJoinToken is still joinedAt, and X remains subscribed to VoiceTopic(V).
Outcome: X is silently ejected from the SFU seconds after joining while the hub and X's own client both still believe X is in voice. updateKeyHolder(V) at line 245 scans h.clients, sees X's live voiceChID == V, and can elect X key holder for a room X is not in — every other participant's voice_e2ee_offer is then rejected with NOT_KEY_HOLDER. X's voice_mute/voice_camera writes hit zero rows and voiceStateBroadcast returns Result{} (state == nil), so the toggles silently no-op, and no later sweep can heal it because the sweep only iterates DB rows.
**Evidence:** hub_sweep.go:202-249
h.mu.RLock()
var stale []struct{ userID, channelID int64; joinedAt string }
for _, vs := range allStates {
c, ok := h.clients[vs.UserID]
if !ok || c.getVoiceChID() != vs.ChannelID { // <-- decided here, under one RLock
stale = append(stale, ...)
}
}
h.mu.RUnlock()
for _, s := range stale { // <-- acted on here, seconds later, no re-check
deleted, err := h.db.LeaveVoiceChannelIfMatch(ctx, s.userID, s.channelID, s.joinedAt)
...
h.broadcastVoiceEvent(ctx, s.channelID, buildVoiceLeave(s.channelID, s.userID))
h.updateKeyHolder(s.channelID)
if h.livekit != nil { _ = h.livekit.RemoveParticipant(ctx, s.channelID, s.userID, s.joinedAt) }
}
contrast, same function, hub_sweep.go:181-187
// The permission check is a DB round-trip; a voice_join to a
// still-permitted channel may have committed while it ran. The
// eviction is conditional on the client still being in the checked
// channel — never on whatever channel it is in by now.
if !h.handleVoiceLeaveIfStillIn(ctx, c, chID) { continue }
the window being raced, voice_join.go:196-222
if err := h.db.JoinVoiceChannel(ctx, c.userID, channelID); ... // row committed
state, err := h.db.GetVoiceState(ctx, c.userID) // still voiceChID == 0 here
...
c.setVoiceState(channelID, state.JoinedAt) // only now non-zero
**Suggested fix:** In the ghost-classification loop (hub_sweep.go:208-217), add the grace-period guard the codebase's own cross_batch note prescribes: parse vs.JoinedAt and `continue` for rows younger than a grace window (e.g. 2x the 60s sweep interval), so a just-committed join can never be classified as a ghost regardless of when c.setVoiceState lands.
**Fixed:** `b8b7a2a1` · test `TestSweepStaleVoiceStates_JoinCatchesUpDuringDeleteWindow` · revert-proof covered-by-OC-0017
### OC-0087 — low — Global search silently drops every DM hit when the DM-id lookup fails, and reports success
`Server/service/message_perms.go:44` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-service`
@@ -1955,6 +1922,35 @@ tauri = 2.11.5 (src-tauri/Cargo.lock:4809-4810), which carries the replace-not-a
**Fixed:** `8787b906` · test `Client/tauri-client/tests/unit/tauri-conf-webview2-args.test.ts` · revert-proof pass
### OC-0090 — low — channelReadAudience falls through to a server-wide role scan when the channel row is gone, leaking a private group-DM's voice_leave to every connected member
`Server/ws/hub_broadcast.go:149` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-ws`
The DM-audience branch is guarded on `ch != nil && ch.Type == "dm"`. A deleted channel yields `ch == nil` (db.GetChannel returns (nil, nil) on sql.ErrNoRows), so control drops into the generic role scan, which — for an id whose channel row and ON DELETE CASCADE'd channel_overrides no longer exist — resolves to "every connected user whose base role holds READ_MESSAGES". The read error above is deliberately failed closed; the missing-row case is not.
**Repro:** Group DM #G with alice, bob, carol. bob and carol leave. alice is alone in #G and is in its voice call (voice_states row + hub voiceChID set). alice calls DELETE /api/v1/dms/G. service.CloseDM -> db.LeaveGroupDM removes the last dm_participants row and, with remaining == 0, deletes the channels row, returning ChannelDeleted=true. api/dm_handler.go:236 then calls hub.DisconnectFromVoiceInChannel(alice, G) -> handleVoiceLeaveIfStillIn -> finishVoiceLeave (Server/ws/voice_leave.go:76) -> h.channelReadAudience(ctx, G). GetChannel(G) now returns (nil, nil), the `ch.Type == "dm"` branch is skipped, and the loop at hub_broadcast.go:171-178 calls h.perms.HasChannelPerm(uid, G, ReadMessages) for every connected user — with no override rows left for G, every ordinary member passes. Each of them receives {"type":"voice_leave","payload":{"channel_id":G,"user_id":alice}} for a private conversation they were never part of. The live-row form of exactly this leak is locked shut by Server/ws/voice_dm_access_test.go:179 (TestVoiceJoin_DMCall_VoiceStateNotLeakedToThirdConnectedUser); the nil-row branch bypasses it.
**Evidence:** ch, err := h.db.GetChannel(ctx, channelID)
if err != nil {
slog.Error("ws: channelReadAudience GetChannel failed, denying", ...)
return []int64{}
}
if ch != nil && ch.Type == "dm" { // <- nil (deleted) channel skips the DM audience entirely
participantIDs, err := h.db.GetDMParticipantIDs(ctx, channelID)
...
return audience
}
...
for _, uid := range userIDs {
if h.perms.HasChannelPerm(ctx, uid, channelID, permissions.ReadMessages) {
audience = append(audience, uid)
}
}
**Suggested fix:** In channelReadAudience (Server/ws/hub_broadcast.go), fail closed on a missing row the same way the lookup-error branch does: after the `if err != nil` block, add `if ch == nil { return []int64{} }` before the `ch.Type == "dm"` check. This is safe for every caller: finishVoiceLeave (voice_leave.go:88-98) and CleanupVoiceForChannel (hub_sweep.go:333-342) both union the room's remaining participants and the leaver into the audience after the call, so eviction/E2EE-teardown signals still reach everyone entitled to them, while non-participants get nothing.
**Fixed:** `b22ad84d` · test `TestVoiceLeave_DeletedChannel_NotLeakedToNonParticipant` · revert-proof pass
### OC-0091 — low — chat_command is the only client message type registered without a rate limiter, and each frame runs a WASM plugin invocation
`Server/ws/hub.go:155` · found 2026-08-12 · hunt `general-2026-08-12` · lens `hotspot-server-service`
@@ -2177,6 +2173,20 @@ service/user.go:107-110 // "a plain sanitizer.Sanitize call would persist and d
**Fixed:** `8579cb5d` · test `Server/api/profile_handler_test.go` · revert-proof pass
### OC-0101 — medium — Channel-visibility watermark is bumped after the fan-out loop, so a client reconnecting during the loop replays past the change and never converges
`Server/ws/hub_broadcast.go:365` · found 2026-08-13 · hunt `2026-08-13-postopt` · lens `ws-hub`
RefreshChannelVisibility delivers the visibility change as targeted, unsequenced channel_create/channel_delete frames to a snapshot of clients taken at line 252, and only calls bumpVisibilityWatermark() at line 365 after the whole per-client loop finishes. A client that reconnects while the loop is still running is in neither audience: it was not in the snapshot, and handleReconnect's mustFullResync check (serve.go:125) reads the still-unbumped watermark and admits it to the replay path, which by construction cannot carry an unsequenced targeted message. revokeUnreadableChannels has the identical gap via `defer h.bumpVisibilityWatermark()` at hub_broadcast.go:497.
**Repro:** Server has N connected clients. Admin edits a channel_override on channel C (or edits a role, which drives RefreshAllChannelVisibility over every channel). RefreshChannelVisibility(C) snapshots h.clients at hub_broadcast.go:252-257, then loops lines 315-357 running 1-4 permission lookups plus a sendMsg + pubsub mutation per client. User U is offline at snapshot time (or drops immediately after) and reconnects mid-loop with last_seq = L, where L exceeds the previous watermark. handleReconnect evaluates mustFullResync(L) against the unbumped visibilityChangeSeq -> false -> U resumes from the ring buffer, never receives a ready payload, and never receives the targeted channel_create/channel_delete (it was not in the snapshot). Line 365 bumps the watermark only after U's handshake has already committed to replay. Revoked case: U's sidebar keeps rendering channel C forever, and clicking it is dead because registerNow's READ-gated re-subscribe correctly refused the topic. Granted case: U's computeAllowedChannels includes C so chat_message frames for C start arriving for a channel the client has no entry for. Neither converges until U reconnects with last_seq=0 or an unrelated visibility change pushes the watermark past L.
**Evidence:** hub_broadcast.go:252 `h.mu.RLock(); clients := make([]*Client, 0, len(h.clients))` … loop 315-357 … 365 `h.bumpVisibilityWatermark()` // comment at 359-364: "Clients not connected right now missed the targeted sends above. Move the watermark so any resume from a seq at or before this point is forced onto the full-ready path" — but the bump happens after, not before, the sends. Same shape at hub_broadcast.go:497 `defer h.bumpVisibilityWatermark()`.
**Suggested fix:** Bump before the audience is snapshotted instead of after it is served, in the one place each: in RefreshChannelVisibility move h.bumpVisibilityWatermark() from line 365 to immediately before the h.mu.RLock() snapshot at line 252; in revokeUnreadableChannels drop the `defer` on line 497 so the existing first-statement call runs eagerly (it already sits at the top, so early returns stay covered). That shrinks the hole from 'the whole fan-out loop' to the few instructions between the bump and the RLock. Residual, if you want it fully closed: re-check h.mustFullResync(lastSeq) inside the h.seqMu section right before h.registerNow in serve.go and fall back to full ready.
**Fixed:** `ea0430c5` · test `TestHandleReconnect_VisibilityChangeDuringHandshake_ForcesFullReady` · revert-proof covered-by-OC-0206
### OC-0102 — medium — Avatar upload rewrites the username from a pre-lock snapshot, silently reverting a concurrent rename
`Server/api/profile_handler.go:575` · found 2026-08-13 · hunt `2026-08-13-postopt` · lens `api-authz`
.superpowers/findings-ledger.json
+48 -24
View File
@@ -231,13 +231,17 @@
"why": "Every other removal path re-elects (finishVoiceLeave, the LiveKit webhook, registerNow, rollbackVoiceJoin, sweepStaleVoiceStates). Channel delete and archive do not, leaving h.voiceKeyHolders[channelID] populated.",
"repro": "Delete a channel that had an elected holder. The map entry is never reachable and never freed — an unbounded per-deleted-channel leak for the process lifetime. On archive the next join's own updateKeyHolder overwrites it before any client can act, so there is no live desync.",
"evidence": "Server/ws/hub_sweep.go:290-346; contrast Server/ws/voice_leave.go:102",
"status": "blocked",
"status": "fixed",
"found": "2026-08-09",
"hunt": "voice-e2ee-2026-08-09",
"lens": "keyholder-election",
"fix": null,
"rationale": "cross-cluster edit: shares Server/db/voice_queries.go with Server/db/queries/sqlite/voice.sql - needs a human (agent edits saved in .superpowers/debris-2026-08-14.patch)",
"blockedDate": "2026-08-14"
"fix": {
"commit": "6f3485e7",
"test": "TestCleanupVoiceForChannel_ClearsKeyHolder",
"revertProof": "pass"
},
"fixed": "2026-08-19",
"note": "CleanupVoiceForChannel now re-elects via updateKeyHolder"
},
{
"id": "OC-0013",
@@ -1386,16 +1390,20 @@
"repro": "Admin bans user U: BroadcastMemberBan fans member_ban out, every connected client runs removeMember(U) and drops U from membersStore, and U's socket is kicked. Admin then unbans U via PATCH /api/v1/admin/users/{id} {\"banned\": false}. ModerationService.UnbanUser commits, then the `case !*req.Banned && hub != nil` branch type-asserts and silently does nothing. U is absent from the member list, from mention autocomplete and from getTypingUsers on every client that was connected during the ban, while any client that connects afterwards gets U in its ready payload — two clients side by side showing different rosters. It only converges for the stale clients if U reconnects (handleFreshConnect broadcasts member_join) or they reconnect themselves; an unbanned user who never comes back online stays missing indefinitely. admin/handlers_users_broadcast_test.go:51 asserts the call happens against a double that implements the interface, so the suite stays green.",
"evidence": "case *req.Banned && hub != nil:\n\thub.BroadcastMemberBan(id) // real method on *ws.Hub\ncase !*req.Banned && hub != nil:\n\tif mub, ok := hub.(memberUnbanBroadcaster); ok {\n\t\tmub.BroadcastMemberUnban(id) // *ws.Hub has no such method — assertion always false\n\t}",
"suggestedFix": "Implement `func (h *Hub) BroadcastMemberUnban(userID int64)` on *ws.Hub that loads the user and role from h.db and calls h.BroadcastToAll(buildMemberJoin(user, roleName)) — the client already maps member_join to addMember, so no protocol change is needed. Add a compile-time `var _ memberUnbanBroadcaster = (*ws.Hub)(nil)` where admin is wired to the real hub so the assertion cannot silently miss again.",
"status": "blocked",
"status": "fixed",
"found": "2026-08-12",
"hunt": "general-2026-08-12",
"lens": "state-desync",
"finder": "opus",
"round": 2,
"confidence": "high",
"fix": null,
"rationale": "cross-cluster edit: shares Server/ws/hub_broadcast.go with the hub_broadcast cluster - needs a human (agent edits saved in .superpowers/debris-2026-08-14.patch)",
"blockedDate": "2026-08-14"
"fix": {
"commit": "d2289560",
"test": "TestBroadcastMemberUnban_FansOutMemberJoin",
"revertProof": "pass"
},
"fixed": "2026-08-19",
"note": "BroadcastMemberUnban implemented on *ws.Hub; compile-time wiring assertion added in admin"
},
{
"id": "OC-0059",
@@ -1965,16 +1973,20 @@
"repro": "Channel with voice_max_video = 1. User A enables their camera: COUNT(camera=1)=0 < 1, row updated to camera=1. A's client-side localCamera then falls out of sync with the row while the row stays 1 — the confirmed enableCamera supersession gap (Client/tauri-client/src/lib/screenShare.ts:243) does exactly this: a disableCamera that lands during publishTrack resets localCamera to false but the server row keeps camera=1. A now presses the camera button (VoiceCallbacks.ts:110 computes next = !localCamera = true) and the server runs EnableCameraIfUnderLimit(A, ch, 1): the subquery counts A's own row, 1 < 1 is false, 0 rows affected, ok=false. A receives VIDEO_LIMIT \"maximum 1 video streams reached\" while being the only video publisher in the room, and every retry repeats it — there is no path that clears camera back to 0 except A sending voice_camera{enabled:false}, which the UI will not do because it believes the camera is already off.",
"evidence": "Server/db/queries/sqlite/voice.sql:90-92 — `UPDATE voice_states SET camera = 1 WHERE voice_states.user_id = ? AND voice_states.channel_id = ? AND (SELECT COUNT(*) FROM voice_states AS vs2 WHERE vs2.channel_id = ? AND vs2.camera = 1) < ?;` (no `AND vs2.user_id <> ?` exclusion, and no `AND camera = 0` on the outer UPDATE). Consumed at Server/ws/voice_controls.go:116 `ok, limitErr := d.DB.EnableCameraIfUnderLimit(ctx, userID, voiceChID, ch.VoiceMaxVideo)` with the refusal at voice_controls.go:121-126 returning ErrCodeVideoLimit.",
"suggestedFix": "Exclude the requester's own row from the count in EnableCameraIfUnderLimit: change the subquery to `WHERE vs2.channel_id = ? AND vs2.camera = 1 AND vs2.user_id <> voice_states.user_id` (or bind userID again with `AND vs2.user_id <> ?`), then regenerate the sqlc layer via the db-change workflow. This makes re-enable idempotent while still refusing a genuinely new publisher at the cap.",
"status": "blocked",
"status": "fixed",
"found": "2026-08-12",
"hunt": "general-2026-08-12",
"lens": "hotspot-server-ws",
"finder": "opus",
"round": 6,
"confidence": "high",
"fix": null,
"rationale": "cross-cluster edit: shares Server/db/voice_queries.go with Server/ws/hub_sweep.go - needs a human (agent edits saved in .superpowers/debris-2026-08-14.patch)",
"blockedDate": "2026-08-14"
"fix": {
"commit": "6a5a3a7c",
"test": "TestVoice_EnableCameraIfUnderLimit_ReEnableIdempotentAtCap",
"revertProof": "pass"
},
"fixed": "2026-08-19",
"note": "own-flag exclusion in both Enable*IfUnderLimit gates; sqlc regenerated"
},
{
"id": "OC-0082",
@@ -2088,16 +2100,20 @@
"repro": "Voice channel V already holds several genuine ghost rows (e.g. after a server restart, or several crashed clients), so the `stale` loop has work to do and each iteration costs a channelReadAudience resolution plus a LiveKit RemoveParticipant with a 5s timeout.\n1. The 60s voiceSweepTicker fires. sweepStaleVoiceStates reads GetAllVoiceStates and takes the h.mu.RLock snapshot at T0.\n2. At T0, user X's voice_join has committed its row via JoinVoiceChannel (voice_join.go:196) but has not yet reached c.setVoiceState (voice_join.go:222) — one GetVoiceState round trip away. X's client still reports getVoiceChID() == 0, so X's row is appended to `stale`.\n3. X's join then completes normally: setVoiceState, Subscribe(VoiceTopic(V)), updateKeyHolder(V), broadcastVoiceEvent(voice_state) — every other client is told X joined, and X's client connects to the SFU.\n4. Seconds later the sweep reaches X's entry. LeaveVoiceChannelIfMatch(X, V, joinedAt) matches — it is literally the row X's join created — and deletes it. voice_leave is broadcast for X. RemoveParticipant(V, X, joinedAt) uses the same identity and kicks X out of the LiveKit room.\n5. Nothing clears X's in-memory state: c.voiceChID is still V and c.voiceJoinToken is still joinedAt, and X remains subscribed to VoiceTopic(V).\nOutcome: X is silently ejected from the SFU seconds after joining while the hub and X's own client both still believe X is in voice. updateKeyHolder(V) at line 245 scans h.clients, sees X's live voiceChID == V, and can elect X key holder for a room X is not in — every other participant's voice_e2ee_offer is then rejected with NOT_KEY_HOLDER. X's voice_mute/voice_camera writes hit zero rows and voiceStateBroadcast returns Result{} (state == nil), so the toggles silently no-op, and no later sweep can heal it because the sweep only iterates DB rows.",
"evidence": "hub_sweep.go:202-249\n\th.mu.RLock()\n\tvar stale []struct{ userID, channelID int64; joinedAt string }\n\tfor _, vs := range allStates {\n\t\tc, ok := h.clients[vs.UserID]\n\t\tif !ok || c.getVoiceChID() != vs.ChannelID { // <-- decided here, under one RLock\n\t\t\tstale = append(stale, ...)\n\t\t}\n\t}\n\th.mu.RUnlock()\n\n\tfor _, s := range stale { // <-- acted on here, seconds later, no re-check\n\t\tdeleted, err := h.db.LeaveVoiceChannelIfMatch(ctx, s.userID, s.channelID, s.joinedAt)\n\t\t...\n\t\th.broadcastVoiceEvent(ctx, s.channelID, buildVoiceLeave(s.channelID, s.userID))\n\t\th.updateKeyHolder(s.channelID)\n\t\tif h.livekit != nil { _ = h.livekit.RemoveParticipant(ctx, s.channelID, s.userID, s.joinedAt) }\n\t}\n\ncontrast, same function, hub_sweep.go:181-187\n\t\t// The permission check is a DB round-trip; a voice_join to a\n\t\t// still-permitted channel may have committed while it ran. The\n\t\t// eviction is conditional on the client still being in the checked\n\t\t// channel — never on whatever channel it is in by now.\n\t\tif !h.handleVoiceLeaveIfStillIn(ctx, c, chID) { continue }\n\nthe window being raced, voice_join.go:196-222\n\t\tif err := h.db.JoinVoiceChannel(ctx, c.userID, channelID); ... // row committed\n\t\tstate, err := h.db.GetVoiceState(ctx, c.userID) // still voiceChID == 0 here\n\t\t...\n\t\tc.setVoiceState(channelID, state.JoinedAt) // only now non-zero",
"suggestedFix": "In the ghost-classification loop (hub_sweep.go:208-217), add the grace-period guard the codebase's own cross_batch note prescribes: parse vs.JoinedAt and `continue` for rows younger than a grace window (e.g. 2x the 60s sweep interval), so a just-committed join can never be classified as a ghost regardless of when c.setVoiceState lands.",
"status": "blocked",
"status": "fixed",
"found": "2026-08-12",
"hunt": "general-2026-08-12",
"lens": "hotspot-server-ws",
"finder": "opus",
"round": 7,
"confidence": "high",
"fix": null,
"rationale": "cross-cluster edit: shares Server/db/voice_queries.go with Server/db/queries/sqlite/voice.sql - needs a human (agent edits saved in .superpowers/debris-2026-08-14.patch)",
"blockedDate": "2026-08-14"
"fix": {
"commit": "b8b7a2a1",
"test": "TestSweepStaleVoiceStates_JoinCatchesUpDuringDeleteWindow",
"revertProof": "covered-by-OC-0017"
},
"fixed": "2026-08-19",
"note": "already fixed on main by the OC-0017 pre-delete live-client re-check (PR #1374); verified against HEAD 2026-08-19"
},
{
"id": "OC-0087",
@@ -2186,16 +2202,20 @@
"repro": "Group DM #G with alice, bob, carol. bob and carol leave. alice is alone in #G and is in its voice call (voice_states row + hub voiceChID set). alice calls DELETE /api/v1/dms/G. service.CloseDM -> db.LeaveGroupDM removes the last dm_participants row and, with remaining == 0, deletes the channels row, returning ChannelDeleted=true. api/dm_handler.go:236 then calls hub.DisconnectFromVoiceInChannel(alice, G) -> handleVoiceLeaveIfStillIn -> finishVoiceLeave (Server/ws/voice_leave.go:76) -> h.channelReadAudience(ctx, G). GetChannel(G) now returns (nil, nil), the `ch.Type == \"dm\"` branch is skipped, and the loop at hub_broadcast.go:171-178 calls h.perms.HasChannelPerm(uid, G, ReadMessages) for every connected user — with no override rows left for G, every ordinary member passes. Each of them receives {\"type\":\"voice_leave\",\"payload\":{\"channel_id\":G,\"user_id\":alice}} for a private conversation they were never part of. The live-row form of exactly this leak is locked shut by Server/ws/voice_dm_access_test.go:179 (TestVoiceJoin_DMCall_VoiceStateNotLeakedToThirdConnectedUser); the nil-row branch bypasses it.",
"evidence": "ch, err := h.db.GetChannel(ctx, channelID)\nif err != nil {\n slog.Error(\"ws: channelReadAudience GetChannel failed, denying\", ...)\n return []int64{}\n}\nif ch != nil && ch.Type == \"dm\" { // <- nil (deleted) channel skips the DM audience entirely\n participantIDs, err := h.db.GetDMParticipantIDs(ctx, channelID)\n ...\n return audience\n}\n...\nfor _, uid := range userIDs {\n if h.perms.HasChannelPerm(ctx, uid, channelID, permissions.ReadMessages) {\n audience = append(audience, uid)\n }\n}",
"suggestedFix": "In channelReadAudience (Server/ws/hub_broadcast.go), fail closed on a missing row the same way the lookup-error branch does: after the `if err != nil` block, add `if ch == nil { return []int64{} }` before the `ch.Type == \"dm\"` check. This is safe for every caller: finishVoiceLeave (voice_leave.go:88-98) and CleanupVoiceForChannel (hub_sweep.go:333-342) both union the room's remaining participants and the leaver into the audience after the call, so eviction/E2EE-teardown signals still reach everyone entitled to them, while non-participants get nothing.",
"status": "blocked",
"status": "fixed",
"found": "2026-08-12",
"hunt": "general-2026-08-12",
"lens": "hotspot-server-ws",
"finder": "opus",
"round": 8,
"confidence": "high",
"fix": null,
"rationale": "cross-cluster edit: shares Server/ws/hub_broadcast.go with Server/admin/handlers_users.go - needs a human (agent edits saved in .superpowers/debris-2026-08-14.patch)",
"blockedDate": "2026-08-14"
"fix": {
"commit": "b22ad84d",
"test": "TestVoiceLeave_DeletedChannel_NotLeakedToNonParticipant",
"revertProof": "pass"
},
"fixed": "2026-08-19",
"note": "fail closed on a missing channel row in channelReadAudienceImpl"
},
{
"id": "OC-0091",
@@ -2444,16 +2464,20 @@
"why": "RefreshChannelVisibility delivers the visibility change as targeted, unsequenced channel_create/channel_delete frames to a snapshot of clients taken at line 252, and only calls bumpVisibilityWatermark() at line 365 after the whole per-client loop finishes. A client that reconnects while the loop is still running is in neither audience: it was not in the snapshot, and handleReconnect's mustFullResync check (serve.go:125) reads the still-unbumped watermark and admits it to the replay path, which by construction cannot carry an unsequenced targeted message. revokeUnreadableChannels has the identical gap via `defer h.bumpVisibilityWatermark()` at hub_broadcast.go:497.",
"repro": "Server has N connected clients. Admin edits a channel_override on channel C (or edits a role, which drives RefreshAllChannelVisibility over every channel). RefreshChannelVisibility(C) snapshots h.clients at hub_broadcast.go:252-257, then loops lines 315-357 running 1-4 permission lookups plus a sendMsg + pubsub mutation per client. User U is offline at snapshot time (or drops immediately after) and reconnects mid-loop with last_seq = L, where L exceeds the previous watermark. handleReconnect evaluates mustFullResync(L) against the unbumped visibilityChangeSeq -> false -> U resumes from the ring buffer, never receives a ready payload, and never receives the targeted channel_create/channel_delete (it was not in the snapshot). Line 365 bumps the watermark only after U's handshake has already committed to replay. Revoked case: U's sidebar keeps rendering channel C forever, and clicking it is dead because registerNow's READ-gated re-subscribe correctly refused the topic. Granted case: U's computeAllowedChannels includes C so chat_message frames for C start arriving for a channel the client has no entry for. Neither converges until U reconnects with last_seq=0 or an unrelated visibility change pushes the watermark past L.",
"evidence": "hub_broadcast.go:252 `h.mu.RLock(); clients := make([]*Client, 0, len(h.clients))` … loop 315-357 … 365 `h.bumpVisibilityWatermark()` // comment at 359-364: \"Clients not connected right now missed the targeted sends above. Move the watermark so any resume from a seq at or before this point is forced onto the full-ready path\" — but the bump happens after, not before, the sends. Same shape at hub_broadcast.go:497 `defer h.bumpVisibilityWatermark()`.",
"status": "blocked",
"status": "fixed",
"found": "2026-08-13",
"hunt": "2026-08-13-postopt",
"lens": "ws-hub",
"finder": "opus",
"confidence": "medium",
"fix": null,
"fix": {
"commit": "ea0430c5",
"test": "TestHandleReconnect_VisibilityChangeDuringHandshake_ForcesFullReady",
"revertProof": "covered-by-OC-0206"
},
"suggestedFix": "Bump before the audience is snapshotted instead of after it is served, in the one place each: in RefreshChannelVisibility move h.bumpVisibilityWatermark() from line 365 to immediately before the h.mu.RLock() snapshot at line 252; in revokeUnreadableChannels drop the `defer` on line 497 so the existing first-statement call runs eagerly (it already sits at the top, so early returns stay covered). That shrinks the hole from 'the whole fan-out loop' to the few instructions between the bump and the RLock. Residual, if you want it fully closed: re-check h.mustFullResync(lastSeq) inside the h.seqMu section right before h.registerNow in serve.go and fall back to full ready.",
"rationale": "cross-cluster edit: shares Server/ws/hub_broadcast.go with Server/admin/handlers_users.go - needs a human (agent edits saved in .superpowers/debris-2026-08-14.patch)",
"blockedDate": "2026-08-14"
"fixed": "2026-08-19",
"note": "already fixed on main by the OC-0206 early watermark bump in both fan-out paths (PR #1375); verified against HEAD 2026-08-19"
},
{
"id": "OC-0102",
Server/admin/hub_wiring_test.go
+10
View File
@@ -0,0 +1,10 @@
package admin
import "github.com/owncord/server/ws"
// OC-0058: handlePatchUser reaches BroadcastMemberUnban through a type
// assertion, which fails silently if *ws.Hub ever loses (or never had) the
// method. This compile-time check turns that silent miss into a build error.
// In-package (not admin_test) so it can see the unexported interface; a test
// file so production admin still depends only on HubBroadcaster.
var _ memberUnbanBroadcaster = (*ws.Hub)(nil)
Server/db/dbgen/querier.go
+11 -2
View File
@@ -76,10 +76,19 @@ type Querier interface {
// OC-0023), and a single user with both flags set must consume two of the N
// slots, not one (OC-0006) -- so both gates sum `vs2.camera + vs2.screenshare`
// across the channel's rows rather than counting rows where either is set.
// The enabling user's own bit is still 0 at gate time, so no self-exclusion
// term is needed.
// The enabling user's own bit for the flag being set CAN already be 1 at
// gate time (a client that lost track of the server-side flag retries the
// enable), so each gate excludes exactly that one bit from the count --
// see the per-query comments below (OC-0081).
// The channel-wide stream count excludes the requester's own camera flag
// (subtracted via the correlated outer-row reference), so re-enabling an
// already-set camera is idempotent at the cap instead of being refused
// against the requester's own stream (OC-0081). Their screenshare, and
// every other user's streams, still count.
EnableCameraIfUnderLimit(ctx context.Context, arg EnableCameraIfUnderLimitParams) (sql.Result, error)
EnablePlugin(ctx context.Context, id int64) error
// Mirror of EnableCameraIfUnderLimit: the count excludes the requester's
// 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
ForceLogoutUser(ctx context.Context, userID int64) error
Server/db/dbgen/voice.sql.go
+13 -4
View File
@@ -102,7 +102,7 @@ const enableCameraIfUnderLimit = `-- name: EnableCameraIfUnderLimit :execresult
UPDATE voice_states SET camera = 1
WHERE voice_states.user_id = ? AND voice_states.channel_id = ?
AND (SELECT COALESCE(SUM(vs2.camera), 0) + COALESCE(SUM(vs2.screenshare), 0) FROM voice_states AS vs2 WHERE vs2.channel_id = ?) < ?4
AND (SELECT COALESCE(SUM(vs2.camera), 0) + COALESCE(SUM(vs2.screenshare), 0) FROM voice_states AS vs2 WHERE vs2.channel_id = ?) - voice_states.camera < ?4
`
type EnableCameraIfUnderLimitParams struct {
@@ -118,8 +118,15 @@ type EnableCameraIfUnderLimitParams struct {
// OC-0023), and a single user with both flags set must consume two of the N
// slots, not one (OC-0006) -- so both gates sum `vs2.camera + vs2.screenshare`
// across the channel's rows rather than counting rows where either is set.
// The enabling user's own bit is still 0 at gate time, so no self-exclusion
// term is needed.
// The enabling user's own bit for the flag being set CAN already be 1 at
// gate time (a client that lost track of the server-side flag retries the
// enable), so each gate excludes exactly that one bit from the count --
// see the per-query comments below (OC-0081).
// The channel-wide stream count excludes the requester's own camera flag
// (subtracted via the correlated outer-row reference), so re-enabling an
// already-set camera is idempotent at the cap instead of being refused
// against the requester's own stream (OC-0081). Their screenshare, and
// every other user's streams, still count.
func (q *Queries) EnableCameraIfUnderLimit(ctx context.Context, arg EnableCameraIfUnderLimitParams) (sql.Result, error) {
return q.db.ExecContext(ctx, enableCameraIfUnderLimit,
arg.UserID,
@@ -132,7 +139,7 @@ func (q *Queries) EnableCameraIfUnderLimit(ctx context.Context, arg EnableCamera
const enableScreenshareIfUnderLimit = `-- name: EnableScreenshareIfUnderLimit :execresult
UPDATE voice_states SET screenshare = 1
WHERE voice_states.user_id = ? AND voice_states.channel_id = ?
AND (SELECT COALESCE(SUM(vs2.camera), 0) + COALESCE(SUM(vs2.screenshare), 0) FROM voice_states AS vs2 WHERE vs2.channel_id = ?) < ?4
AND (SELECT COALESCE(SUM(vs2.camera), 0) + COALESCE(SUM(vs2.screenshare), 0) FROM voice_states AS vs2 WHERE vs2.channel_id = ?) - voice_states.screenshare < ?4
`
type EnableScreenshareIfUnderLimitParams struct {
@@ -142,6 +149,8 @@ type EnableScreenshareIfUnderLimitParams struct {
MaxVideo int64 `json:"maxVideo"`
}
// Mirror of EnableCameraIfUnderLimit: the count excludes the requester's
// own screenshare flag so re-enable is idempotent at the cap (OC-0081).
func (q *Queries) EnableScreenshareIfUnderLimit(ctx context.Context, arg EnableScreenshareIfUnderLimitParams) (sql.Result, error) {
return q.db.ExecContext(ctx, enableScreenshareIfUnderLimit,
arg.UserID,
Server/db/oc_0081_video_cap_own_row_test.go
+93
View File
@@ -0,0 +1,93 @@
package db_test
import (
"context"
"testing"
)
// OC-0081: EnableCameraIfUnderLimit's guard subquery counted every video
// stream in the channel INCLUDING the requester's own camera row, so a user
// whose server-side camera flag was already 1 could never re-enable at the
// cap: the sole publisher in a max_video=1 room got VIDEO_LIMIT against
// their own stream, with no path out (the client believes the camera is off
// and never sends a disable). Re-enable must be idempotent; a genuinely new
// publisher at the cap must still be refused; the requester's OTHER stream
// (screenshare) must still count against enabling their camera.
func TestVoice_EnableCameraIfUnderLimit_ReEnableIdempotentAtCap(t *testing.T) {
database := newVoiceTestDB(t)
u1 := seedVoiceUser(t, database, "cam-reen-u1")
chanID := seedVoiceChannel(t, database, "cam-reen-ch")
_ = database.JoinVoiceChannel(context.Background(), u1, chanID)
ok, err := database.EnableCameraIfUnderLimit(context.Background(), u1, chanID, 1)
if err != nil || !ok {
t.Fatalf("first enable: ok=%v err=%v, want true", ok, err)
}
// Same user, camera row already 1, still the only stream in the room.
ok, err = database.EnableCameraIfUnderLimit(context.Background(), u1, chanID, 1)
if err != nil {
t.Fatalf("re-enable: %v", err)
}
if !ok {
t.Error("re-enable at the cap was refused against the requester's own camera row")
}
}
func TestVoice_EnableCameraIfUnderLimit_OtherUserStillRefusedAtCap(t *testing.T) {
database := newVoiceTestDB(t)
u1 := seedVoiceUser(t, database, "cam-cap-u1")
u2 := seedVoiceUser(t, database, "cam-cap-u2")
chanID := seedVoiceChannel(t, database, "cam-cap-ch")
_ = database.JoinVoiceChannel(context.Background(), u1, chanID)
_ = database.JoinVoiceChannel(context.Background(), u2, chanID)
if ok, err := database.EnableCameraIfUnderLimit(context.Background(), u1, chanID, 1); err != nil || !ok {
t.Fatalf("u1 enable: ok=%v err=%v, want true", ok, err)
}
ok, err := database.EnableCameraIfUnderLimit(context.Background(), u2, chanID, 1)
if err != nil {
t.Fatalf("u2 enable: %v", err)
}
if ok {
t.Error("a new publisher was admitted past the video cap")
}
}
func TestVoice_EnableCameraIfUnderLimit_OwnScreenshareStillCounts(t *testing.T) {
database := newVoiceTestDB(t)
u1 := seedVoiceUser(t, database, "cam-ss-u1")
chanID := seedVoiceChannel(t, database, "cam-ss-ch")
_ = database.JoinVoiceChannel(context.Background(), u1, chanID)
if ok, err := database.EnableScreenshareIfUnderLimit(context.Background(), u1, chanID, 1); err != nil || !ok {
t.Fatalf("screenshare enable: ok=%v err=%v, want true", ok, err)
}
// The camera would be a SECOND stream from this user; the cap is 1.
ok, err := database.EnableCameraIfUnderLimit(context.Background(), u1, chanID, 1)
if err != nil {
t.Fatalf("camera enable: %v", err)
}
if ok {
t.Error("own screenshare must still count toward the cap when enabling the camera")
}
}
func TestVoice_EnableScreenshareIfUnderLimit_ReEnableIdempotentAtCap(t *testing.T) {
database := newVoiceTestDB(t)
u1 := seedVoiceUser(t, database, "ss-reen-u1")
chanID := seedVoiceChannel(t, database, "ss-reen-ch")
_ = database.JoinVoiceChannel(context.Background(), u1, chanID)
if ok, err := database.EnableScreenshareIfUnderLimit(context.Background(), u1, chanID, 1); err != nil || !ok {
t.Fatalf("first enable: ok=%v err=%v, want true", ok, err)
}
ok, err := database.EnableScreenshareIfUnderLimit(context.Background(), u1, chanID, 1)
if err != nil {
t.Fatalf("re-enable: %v", err)
}
if !ok {
t.Error("screenshare re-enable at the cap was refused against the requester's own row")
}
}
Server/db/queries/sqlite/voice.sql
+13 -4
View File
@@ -99,18 +99,27 @@ UPDATE voice_states SET server_deafened = 0 WHERE user_id = ? AND channel_id = ?
-- OC-0023), and a single user with both flags set must consume two of the N
-- slots, not one (OC-0006) -- so both gates sum `vs2.camera + vs2.screenshare`
-- across the channel's rows rather than counting rows where either is set.
-- The enabling user's own bit is still 0 at gate time, so no self-exclusion
-- term is needed.
-- The enabling user's own bit for the flag being set CAN already be 1 at
-- gate time (a client that lost track of the server-side flag retries the
-- enable), so each gate excludes exactly that one bit from the count --
-- see the per-query comments below (OC-0081).
-- name: EnableCameraIfUnderLimit :execresult
-- The channel-wide stream count excludes the requester's own camera flag
-- (subtracted via the correlated outer-row reference), so re-enabling an
-- already-set camera is idempotent at the cap instead of being refused
-- against the requester's own stream (OC-0081). Their screenshare, and
-- every other user's streams, still count.
UPDATE voice_states SET camera = 1
WHERE voice_states.user_id = ? AND voice_states.channel_id = ?
AND (SELECT COALESCE(SUM(vs2.camera), 0) + COALESCE(SUM(vs2.screenshare), 0) FROM voice_states AS vs2 WHERE vs2.channel_id = ?) < sqlc.arg(max_video);
AND (SELECT COALESCE(SUM(vs2.camera), 0) + COALESCE(SUM(vs2.screenshare), 0) FROM voice_states AS vs2 WHERE vs2.channel_id = ?) - voice_states.camera < sqlc.arg(max_video);
-- name: EnableScreenshareIfUnderLimit :execresult
-- Mirror of EnableCameraIfUnderLimit: the count excludes the requester's
-- own screenshare flag so re-enable is idempotent at the cap (OC-0081).
UPDATE voice_states SET screenshare = 1
WHERE voice_states.user_id = ? AND voice_states.channel_id = ?
AND (SELECT COALESCE(SUM(vs2.camera), 0) + COALESCE(SUM(vs2.screenshare), 0) FROM voice_states AS vs2 WHERE vs2.channel_id = ?) < sqlc.arg(max_video);
AND (SELECT COALESCE(SUM(vs2.camera), 0) + COALESCE(SUM(vs2.screenshare), 0) FROM voice_states AS vs2 WHERE vs2.channel_id = ?) - voice_states.screenshare < sqlc.arg(max_video);
-- name: ClearVoiceState :exec
DELETE FROM voice_states WHERE user_id = ?;
Server/ws/hub_broadcast.go
+38 -2
View File
@@ -234,6 +234,16 @@ func (h *Hub) channelReadAudienceImpl(ctx context.Context, channelID int64, igno
"channel_id", channelID, "err", err)
return []int64{}
}
// Fail closed on a missing row too (OC-0090): GetChannel returns
// (nil, nil) for a deleted channel, and falling through would hand a
// channel with no override rows left to the role scan below — which
// resolves to every connected user with base READ_MESSAGES, leaking
// e.g. a closed group-DM's voice_leave server-wide. Callers that
// tear down voice union the room's participants and the leaver back
// in afterwards, so eviction/E2EE-teardown signals still arrive.
if ch == nil {
return []int64{}
}
// Archived channels are hidden from every client regardless of
// permissions, mirroring RefreshChannelVisibility and VisibleChannelIDs.
// Without this, an admin edit to an archived channel (or a voice
@@ -241,10 +251,10 @@ func (h *Hub) channelReadAudienceImpl(ctx context.Context, channelID int64, igno
// base role holds READ_MESSAGES, none of whom have the channel in their
// ready payload or sidebar. ignoreArchived opts a caller out of this
// specific check only — see channelReadAudienceIgnoringArchived.
if ch != nil && ch.Archived && !ignoreArchived {
if ch.Archived && !ignoreArchived {
return []int64{}
}
if ch != nil && ch.Type == "dm" {
if ch.Type == "dm" {
return h.channelReadAudienceDM(ctx, channelID, userIDs)
}
}
@@ -580,6 +590,32 @@ func (h *Hub) BroadcastMemberBan(userID int64) {
h.DisconnectUser(userID)
}
// BroadcastMemberUnban is the mirror of BroadcastMemberBan: member_ban
// hard-deletes the row on every connected client, so an unban must re-add it
// or clients connected through the ban permanently disagree with freshly
// connecting ones. Fans out the same member_join a fresh connect would
// (clients map it to addMember — no protocol change), satisfying the admin
// package's memberUnbanBroadcaster capability; admin's hub_wiring_test.go
// pins that at compile time. (OC-0058)
func (h *Hub) BroadcastMemberUnban(userID int64) {
ctx := context.Background()
user, err := h.db.GetUserByID(ctx, userID)
if err != nil || user == nil {
slog.Error("hub: BroadcastMemberUnban GetUserByID failed", "user_id", userID, "err", err)
return
}
roleName := ""
if role, err := h.db.GetRoleForUser(ctx, userID); err == nil && role != nil {
roleName = role.Name
}
// The ban disconnected them and reconnecting was refused while banned,
// so they cannot be online at unban time — report offline regardless of
// the stale status the row carries (serve_ready's "no live connection is
// offline, whatever the row says" rule).
user.Status = "offline"
h.BroadcastToAll(buildMemberJoin(user, roleName))
}
// DisconnectUser forcibly disconnects the client identified by userID.
// No-op if the user is not currently connected.
func (h *Hub) DisconnectUser(userID int64) {
Server/ws/hub_sweep.go
+9
View File
@@ -440,4 +440,13 @@ func (h *Hub) CleanupVoiceForChannel(channelID int64) {
for _, vs := range states {
h.broadcastChannelScopedTo(channelID, buildVoiceLeave(channelID, vs.UserID), audience, "voice event")
}
// Re-elect the key holder now that the room is torn down — every other
// removal path does this (finishVoiceLeave, the LiveKit webhook,
// registerNow, rollbackVoiceJoin, sweepStaleVoiceStates). All client
// voice states for this channel were cleared above, so this deletes the
// voiceKeyHolders entry; without it a deleted channel's entry lived for
// the process lifetime (OC-0012). No locks are held here, as
// updateKeyHolder requires.
h.updateKeyHolder(channelID)
}
Server/ws/oc_0012_cleanup_keyholder_test.go
+38
View File
@@ -0,0 +1,38 @@
package ws_test
import (
"testing"
"time"
"github.com/owncord/server/ws"
)
// OC-0012: every other voice-removal path re-elects the key holder
// (finishVoiceLeave, the LiveKit webhook, registerNow, rollbackVoiceJoin,
// sweepStaleVoiceStates) — CleanupVoiceForChannel, the channel delete/archive
// path, did not. The voiceKeyHolders entry for a deleted channel then stayed
// populated forever: an unbounded per-deleted-channel leak, and a stale
// IsVoiceKeyHolder verdict for a room that no longer exists.
func TestCleanupVoiceForChannel_ClearsKeyHolder(t *testing.T) {
hub, database := newVoiceHub(t)
alice := seedMemberUser(t, database, "ckh-alice")
vcID := seedVoiceChannel(t, database, "ckh-vc")
send := make(chan []byte, 32)
c := ws.NewTestClientWithUser(hub, alice, 0, send)
hub.Register(c)
waitRegistered(t, hub, c)
// Real join flow: elects alice key holder of the one-person room.
hub.HandleMessageForTest(c, voiceJoinMsg(vcID))
drainChanTimeout(send, 200*time.Millisecond)
if !hub.IsVoiceKeyHolder(vcID, alice.ID) {
t.Fatal("precondition: the sole participant must be the key holder")
}
hub.CleanupVoiceForChannel(vcID)
if hub.IsVoiceKeyHolder(vcID, alice.ID) {
t.Fatal("voiceKeyHolders still names a holder for a cleaned-up channel")
}
}
Server/ws/oc_0058_member_unban_broadcast_test.go
+62
View File
@@ -0,0 +1,62 @@
package ws_test
import (
"encoding/json"
"testing"
"time"
"github.com/owncord/server/ws"
)
// OC-0058: the admin unban path type-asserts its hub against an optional
// BroadcastMemberUnban capability, but *ws.Hub never implemented it — the
// assertion always missed, so clients connected during the ban (which
// hard-deleted the member row on member_ban) never learned the user was
// back, permanently disagreeing with freshly connecting clients. The hub
// must implement the mirror of BroadcastMemberBan: a member_join fan-out
// that re-adds the user to every connected member store. The unbanned user
// has no live connection (the ban kicked them), so the payload must report
// them offline regardless of the stale status their row carries.
func TestBroadcastMemberUnban_FansOutMemberJoin(t *testing.T) {
hub, database := newVoiceHub(t)
alice := seedMemberUser(t, database, "unban-alice")
bob := seedMemberUser(t, database, "unban-bob")
send := make(chan []byte, 32)
c := ws.NewTestClientWithUser(hub, bob, 0, send)
hub.Register(c)
waitRegistered(t, hub, c)
drainChanTimeout(send, 100*time.Millisecond)
hub.BroadcastMemberUnban(alice.ID)
msgs := drainChanTimeout(send, 300*time.Millisecond)
for _, m := range msgs {
if extractType(t, m) != "member_join" {
continue
}
var parsed struct {
Payload struct {
User struct {
ID int64 `json:"id"`
Username string `json:"username"`
} `json:"user"`
Status string `json:"status"`
} `json:"payload"`
}
if err := json.Unmarshal(m, &parsed); err != nil {
t.Fatalf("unmarshal member_join: %v", err)
}
if parsed.Payload.User.ID != alice.ID {
t.Fatalf("member_join user id = %d, want %d", parsed.Payload.User.ID, alice.ID)
}
if parsed.Payload.User.Username != alice.Username {
t.Fatalf("member_join username = %q, want %q", parsed.Payload.User.Username, alice.Username)
}
if parsed.Payload.Status != "offline" {
t.Fatalf("member_join status = %q, want offline — the unbanned user cannot be connected", parsed.Payload.Status)
}
return
}
t.Fatal("no member_join reached a connected client after BroadcastMemberUnban")
}
Server/ws/oc_0090_deleted_channel_audience_test.go
+65
View File
@@ -0,0 +1,65 @@
package ws_test
import (
"context"
"testing"
"time"
"github.com/owncord/server/ws"
)
// OC-0090: channelReadAudience fails closed on a GetChannel *error*, but a
// deleted channel returns (nil, nil) — and that nil row used to skip the DM
// branch and fall through to the server-wide role scan, broadcasting a
// private DM call's voice_leave to every connected member with base
// READ_MESSAGES. The last-leaver CloseDM path hits exactly this: the channel
// row is already gone when DisconnectFromVoiceInChannel tears the call down.
// A missing row must resolve to an empty READ audience; the leaver still
// hears their own teardown via broadcastVoiceEventWithLeaver's union.
func TestVoiceLeave_DeletedChannel_NotLeakedToNonParticipant(t *testing.T) {
hub, database := newVoiceHub(t)
alice := seedMemberUser(t, database, "delchan-alice")
bob := seedMemberUser(t, database, "delchan-bob")
mallory := seedMemberUser(t, database, "delchan-mallory") // connected, READ_MESSAGES, not a participant
dmID := seedDMChannel(t, database, alice.ID, bob.ID)
aliceSend := make(chan []byte, 32)
mallorySend := make(chan []byte, 32)
aliceClient := ws.NewTestClientWithUser(hub, alice, 0, aliceSend)
malloryClient := ws.NewTestClientWithUser(hub, mallory, 0, mallorySend)
hub.Register(aliceClient)
hub.Register(malloryClient)
waitRegistered(t, hub, malloryClient)
hub.HandleMessageForTest(aliceClient, voiceJoinMsg(dmID))
drainChanTimeout(aliceSend, 200*time.Millisecond)
drainChanTimeout(mallorySend, 200*time.Millisecond)
// The DM is closed out from under the call: the channels row (and its
// CASCADE'd overrides) are gone before the voice teardown runs.
if err := database.DeleteChannel(context.Background(), dmID); err != nil {
t.Fatalf("DeleteChannel: %v", err)
}
if !hub.DisconnectFromVoiceInChannel(context.Background(), alice.ID, dmID) {
t.Fatal("DisconnectFromVoiceInChannel reported the user was not in the channel")
}
aliceMsgs := drainChanTimeout(aliceSend, 300*time.Millisecond)
foundLeave := false
for _, m := range aliceMsgs {
if extractType(t, m) == "voice_leave" {
foundLeave = true
}
}
if !foundLeave {
t.Error("the evicted participant must still receive voice_leave — it is their only teardown signal")
}
malloryMsgs := drainChanTimeout(mallorySend, 300*time.Millisecond)
for _, m := range malloryMsgs {
if extractType(t, m) == "voice_leave" {
t.Fatal("voice_leave for a deleted DM channel leaked to a connected non-participant")
}
}
}
Server/ws/serve_test.go
+20 -2
View File
@@ -610,7 +610,17 @@ func TestHub_BroadcastChannelCreate_DeliversToAllClients(t *testing.T) {
hub.Register(c1)
waitRegistered(t, hub, c1)
ch := &db.Channel{ID: 77, Name: "announcements", Type: "text", Category: "News", Position: 1}
// Seed a real row: every production caller broadcasts a channel already
// committed to the DB, and channelReadAudience fails closed on a missing
// row (OC-0090) — a fabricated id would resolve to an empty audience.
chID, err := database.CreateChannel(context.Background(), "announcements", "text", "News", "", 1)
if err != nil {
t.Fatalf("CreateChannel: %v", err)
}
ch, err := database.GetChannel(context.Background(), chID)
if err != nil || ch == nil {
t.Fatalf("GetChannel: ch=%v err=%v", ch, err)
}
hub.BroadcastChannelCreate(ch)
// The receive select below blocks with its own timeout.
@@ -649,7 +659,15 @@ func TestHub_BroadcastChannelUpdate_DeliversToAllClients(t *testing.T) {
hub.Register(c1)
waitRegistered(t, hub, c1)
ch := &db.Channel{ID: 88, Name: "updated-channel", Type: "text", Category: "General", Position: 2}
// Seed a real row — see the channel_create test above (OC-0090).
chID, err := database.CreateChannel(context.Background(), "updated-channel", "text", "General", "", 2)
if err != nil {
t.Fatalf("CreateChannel: %v", err)
}
ch, err := database.GetChannel(context.Background(), chID)
if err != nil || ch == nil {
t.Fatalf("GetChannel: ch=%v err=%v", ch, err)
}
hub.BroadcastChannelUpdate(ch)
// The receive select below blocks with its own timeout.