diff --git a/Server/invariants/db_import_boundary.go b/Server/invariants/db_import_boundary.go index d51fe4d3..938df552 100644 --- a/Server/invariants/db_import_boundary.go +++ b/Server/invariants/db_import_boundary.go @@ -96,9 +96,9 @@ var DBImportAllow = map[string]DBImportEntry{ "ws/hub_sweep.go": {"move", "voice", "stale-voice sweep reads and leaves"}, "ws/messages.go": {"adapter", "", "wire types + pure status helpers"}, "ws/serve.go": {"move", "connection", "connect/disconnect lifecycle; B3-5 splits it by family first"}, - "ws/serve_auth.go": {"move", "auth", "session lookup on the WebSocket handshake"}, + "ws/serve_auth.go": {"move", "auth", "handshake auth: session, user and role lookups, connect audit, failed-handshake teardown"}, "ws/serve_pumps.go": {"move", "user", "MarkUserDisconnected on pump exit"}, - "ws/serve_ready.go": {"move", "channel", "ready snapshot: channels, overrides, unreads, DMs, members"}, + "ws/serve_ready.go": {"move", "channel", "ready snapshot and fresh-connect: channels, overrides, unreads, DMs, members, stale-voice cleanup"}, "ws/voice_join.go": {"move", "voice", "voice state reads and writes"}, "ws/voice_moderation.go": {"move", "voice", "mute/deafen/move persist voice state"}, } diff --git a/Server/ws/serve.go b/Server/ws/serve.go index 078a0b75..d4b712b9 100644 --- a/Server/ws/serve.go +++ b/Server/ws/serve.go @@ -33,24 +33,6 @@ const ( maxColdReplay = 5000 ) -// handshakeWrite writes one handshake-phase message (auth_ok, ready, or a -// replay event) under writeTimeout, instead of the bare ctx every caller here -// otherwise has on hand. -// -// Every handshake write runs against ctx = r.Context() from ServeWS. -// websocket.Accept hijacks the connection, which stops net/http's own -// mechanism for cancelling that context on client disconnect, so without this -// wrapper ctx is never cancelled while the handler is blocked inside -// conn.Write — a peer that stops reading (or whose receive window closes) -// pins the write, the handler goroutine, and the socket forever (OC-0152). -// writePumpWrite (serve_pumps.go) already bounds its writes the same way; -// this brings the handshake writes in serve.go up to the same guarantee. -func handshakeWrite(ctx context.Context, conn *websocket.Conn, msg []byte) error { - wCtx, cancel := context.WithTimeout(ctx, writeTimeout) - defer cancel() - return conn.Write(wCtx, websocket.MessageText, msg) -} - // ServeWS upgrades an HTTP connection to WebSocket, performs in-band auth, // then drives the client's read/write loops. // Do not wrap with AuthMiddleware — WS does its own auth. @@ -119,45 +101,6 @@ func ServeWS(hub *Hub, database *db.DB, allowedOrigins []string, maxConns int) h } } -func (h *Hub) upgradeAndAuth( - conn *websocket.Conn, database *db.DB, r *http.Request, -) (*Client, uint64, error) { - user, tokenHash, hint, err := authenticateConn(r.Context(), conn, database) - if err != nil { - slog.Warn("ws auth failed", "err", err, "remote", r.RemoteAddr) - _ = conn.Close(websocket.StatusPolicyViolation, "authentication failed") - return nil, 0, err - } - lastSeq := hint.LastSeq - - c := newClient(h, conn, user, tokenHash, lastSeq, r.Context()) - c.remoteAddr = r.RemoteAddr - // Untrusted until handleReconnect checks it against the allowed set. - c.authChannelID = hint.ChannelID - - // Look up role name for protocol-compliant payloads and cache on client. - // Fail closed like the sibling lookup in handleFreshConnect (BUG-094): - // this value is authoritative on the wire — auth_ok reports it as the - // user's own role, member_join broadcasts it to every other client, and - // every chat_message carries it — so a lookup failure must not silently - // substitute "member" and pin the whole session to a fabricated role - // (OC-0269). - role, roleErr := database.GetRoleByID(r.Context(), user.RoleID) - if roleErr != nil || role == nil { - slog.Error("ws: role lookup failed during handshake, closing connection", - "user_id", user.ID, "role_id", user.RoleID, "err", roleErr) - _ = conn.Close(websocket.StatusInternalError, "role lookup failed") - return nil, 0, fmt.Errorf("upgradeAndAuth: role lookup failed for user %d: %w", user.ID, roleErr) - } - c.roleName = strings.ToLower(role.Name) - - slog.Info("websocket connected", "username", user.Username, "user_id", user.ID, "remote", r.RemoteAddr) - db.WriteAudit(context.WithoutCancel(r.Context()), database, user.ID, "ws_connect", "user", user.ID, - "WebSocket connected from "+r.RemoteAddr) - - return c, lastSeq, nil -} - // handleReconnectPreRegisterRaceHook, when non-nil, runs once inside // handleReconnect's h.seqMu critical section immediately before the // mustFullResync re-check that guards registerNow. Test-only (nil in @@ -680,48 +623,6 @@ func (h *Hub) liveVoiceEventsSince(ctx context.Context, afterSeq uint64, chID in return filtered } -// unregisterFailedHandshake removes c after a post-registerNow handshake -// write failure. No readPump ever starts for this connection — the -// fresh-connect callers return an error that stops ServeWS before it starts -// the pumps, and handleReconnect's callers report startPumps=false for the -// same reason (OC-0051) — and the old connection this one replaced already -// ran its defer (skipping teardown because this client held the slot) — so -// when no replacement remains, the standard disconnect teardown must run -// here or the user stays online forever. -func (h *Hub) unregisterFailedHandshake(ctx context.Context, c *Client) { - // Snapshot voice state BEFORE unregister, mirroring readPump's defer - // (serve_pumps.go): once unregisterNow removes c, there is no way to tell - // whether it still owned a (possibly just-transferred) voice session. - voiceChID := c.getVoiceChID() - replaced := h.unregisterNow(c) - if !replaced { - cleanupCtx := context.WithoutCancel(ctx) - // A connection that inherited a transferred voice session (the - // replay-failure fallback in handleFreshConnect deliberately keeps - // the voice_states row and registerNow transfers it onto c) must have - // that session torn down here too, or the row, the LiveKit - // participant, and a stale E2EE key-holder entry all survive this - // connection's death until the next sweep (up to 60s). - if voiceChID != 0 { - h.handleVoiceLeave(cleanupCtx, c) - } - } - // shouldMarkOffline re-checks h.clients rather than trusting the - // `replaced` snapshot alone: it was sampled before handleVoiceLeave, - // which can block for seconds, so a reconnect landing during that window - // would otherwise be invisible here and mark the live session's user - // offline (OC-0019, mirrored from readPump's defer in serve_pumps.go). - if h.shouldMarkOffline(c, replaced) { - cleanupCtx := context.WithoutCancel(ctx) - _ = h.db.MarkUserDisconnected(cleanupCtx, c.userID) - // custom_status is nil, not c.user.CustomStatus: see the identical - // note in serve_pumps.go's readPump defer — that field is an - // auth-time snapshot, never updated, so broadcasting it here can - // resurrect a status the user already changed or cleared. - h.QueuePresence(c.userID, db.StatusOffline, nil) - } -} - // applyConnectStatus writes the status this session comes online as and caches // it on the client. // @@ -801,190 +702,3 @@ func (h *Hub) computeAllowedChannels(ctx context.Context, database *db.DB, user return allowed, nil } - -func (h *Hub) handleFreshConnect( - ctx context.Context, conn *websocket.Conn, c *Client, database *db.DB, -) error { - // Clean stale voice state BEFORE building ready and registering. - // When a user F5-reloads while in voice, the DB row from the previous - // session must be removed so the ready payload doesn't include it and - // other clients see a voice_leave broadcast. - if vs, err := database.GetVoiceState(ctx, c.userID); err == nil && vs != nil { - 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). - userRole, roleErr := database.GetRoleByID(ctx, c.user.RoleID) - if roleErr != nil || userRole == nil { - slog.Error("ws: role lookup failed, disconnecting", "user_id", c.userID, "role_id", c.user.RoleID, "err", roleErr) - _ = conn.Close(websocket.StatusInternalError, "role lookup failed") - return fmt.Errorf("role lookup failed for user %d: %w", c.userID, roleErr) - } - - // Register BEFORE writing auth_ok + ready so broadcasts that arrive during - // the write window are queued in the client's send buffer instead of - // being lost (BUG-123). writePump hasn't started yet, so queued messages - // will be drained once the pumps begin. - // - // Only the replay-failure fallback (lastSeq > 0) can inherit voice state - // from the previous connection, so that is the only case where registerNow - // needs the read-permission set. Fail closed on error: nil denies the - // inherited voice-channel subscription. - var allowedChannelIDs map[int64]bool - if c.lastSeq > 0 { - allowed, allowedErr := h.computeAllowedChannels(ctx, database, c.user) - if allowedErr != nil { - slog.Warn("ws handleFreshConnect: computeAllowedChannels failed, skipping voice channel subscription", - "user_id", c.userID, "err", allowedErr) - } else { - allowedChannelIDs = allowed - } - } - // handleReconnect may have promoted an auth-frame active_channel_id into - // c.channelID (serve.go, honoured only when it was READ-visible at that - // moment) and then aborted on one of its own re-checks — most notably the - // final mustFullResync check, tripped by a permission revocation that - // landed mid-handshake. None of those abort paths undo the c.channelID - // write. registerNow subscribes c.channelID's ChannelTopic - // unconditionally, so re-gate it here against the freshly recomputed - // permission set before registering. Fail closed: a nil allowedChannelIDs - // (lastSeq == 0, or the computeAllowedChannels error branch above) denies. - if chID := c.getChannelID(); chID != 0 && !allowedChannelIDs[chID] { - c.mu.Lock() - 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) - - // Fresh connection or replay fallback: full auth_ok + ready flow. - slog.Info("ws sending auth_ok", "user_id", c.userID, "username", c.user.Username, "role", c.roleName) - if err := handshakeWrite(ctx, conn, h.buildAuthOK(ctx, c.user, c.roleName, "none")); err != nil { - slog.Warn("ws: failed to send auth_ok", "user_id", c.userID, "err", err) - h.unregisterFailedHandshake(ctx, c) - _ = conn.Close(websocket.StatusInternalError, "handshake failed") - return err - } - if ready, readyErr := h.buildReady(ctx, database, c.userID, userRole); readyErr == nil { - slog.Info("ws sending ready payload", "user_id", c.userID, "payload_bytes", len(ready)) - if err := handshakeWrite(ctx, conn, ready); err != nil { - slog.Warn("ws: failed to send ready payload", "user_id", c.userID, "err", err) - h.unregisterFailedHandshake(ctx, c) - _ = conn.Close(websocket.StatusInternalError, "handshake failed") - return err - } - } else { - slog.Error("buildReady failed", "user_id", c.userID, "err", readyErr) - _ = handshakeWrite(ctx, conn, buildErrorMsg(ErrCodeInternal, "failed to build ready payload")) - h.unregisterFailedHandshake(ctx, c) - _ = conn.Close(websocket.StatusInternalError, "failed to build ready payload") - return readyErr - } - - slog.Info("ws broadcasting member_join and presence", "user_id", c.userID, "username", c.user.Username) - h.BroadcastToAll(buildMemberJoin(c.user, c.roleName)) - h.announceConnectPresence(c) - - return nil -} - -// freshConnectCleanStaleVoice removes the voice state left behind by this -// user's previous session, unless that session is the still-registered -// connection this one is about to inherit from. -func (h *Hub) freshConnectCleanStaleVoice(ctx context.Context, database *db.DB, c *Client, vs *db.VoiceState) { - // Replay-failure fallback (lastSeq > 0): registerNow below transfers - // the still-registered old connection's live voice state into this - // client. Deleting the DB row here — and the LiveKit participant, - // whose removal token is the very JoinedAt being transferred — would - // leave the user "in voice" on the hub only: voice_join bounces off - // ALREADY_JOINED and sweepStaleVoiceStates never heals - // memory-without-row. Keep the row so ready stays consistent. If the - // old client unregisters before registerNow runs, the transfer is - // skipped and the next sweep reaps the then-truly-stale row. - if old := h.GetClient(c.userID); c.lastSeq > 0 && old != nil && old.getVoiceChID() == vs.ChannelID { - slog.Info("ws fresh connect: keeping voice state for replay-failure fallback", - "user_id", c.userID, "channel_id", vs.ChannelID) - return - } - slog.Info("ws fresh connect: cleaning stale voice state", - "user_id", c.userID, "channel_id", vs.ChannelID) - if _, delErr := database.LeaveVoiceChannelIfMatch(ctx, c.userID, vs.ChannelID, vs.JoinedAt); delErr != nil { - slog.Warn("ws fresh connect: LeaveVoiceChannelIfMatch failed", "err", delErr) - } - // The DB row is gone, but the still-registered OLD *Client (if any) is - // otherwise only cleared by registerNow — which two early-return paths - // further down handleFreshConnect (the refreshUserSnapshot and - // GetRoleByID failure branches) can skip entirely. Without this, that - // old client's in-memory voiceChID and the E2EE key-holder election for - // this room survive as a memory-without-row ghost that - // sweepStaleVoiceStates can never see, since it iterates DB rows - // (OC-0252). Clearing here makes freshConnectCleanStaleVoice self - // sufficient regardless of whether registerNow ever runs; registerNow's - // own replacedVoiceChID re-election later becomes a redundant no-op - // (clearVoiceState finds nothing left to clear), not a conflict. - if old := h.GetClient(c.userID); old != nil { - if _, cleared := old.clearVoiceStateIfMatch(vs.ChannelID); cleared { - h.pubsub.Unsubscribe(old, VoiceTopic(vs.ChannelID)) - } - } - h.updateKeyHolder(vs.ChannelID) - h.broadcastVoiceEvent(ctx, vs.ChannelID, buildVoiceLeave(vs.ChannelID, c.userID)) - if h.livekit == nil { - return - } - // BUG-089: Capture stale join token so the goroutine only removes - // the exact stale participant. The identity includes joinedAt, so - // even if the user rejoins voice quickly, the new session has a - // different identity and won't be removed. The removal must - // complete even if this connection drops mid-handshake, so detach - // from cancellation (values kept); shutdown is handled via h.stop. - staleChID, staleUserID, staleJoinToken := vs.ChannelID, c.userID, vs.JoinedAt - lkCtx := context.WithoutCancel(ctx) - go func() { - select { - case <-h.stop: - return - default: - } - if err := h.livekit.RemoveParticipant(lkCtx, staleChID, staleUserID, staleJoinToken); err != nil { - slog.Warn("ws fresh connect: RemoveParticipant failed (may already be gone)", - "err", err, "user_id", staleUserID, "channel_id", staleChID) - } - }() -} diff --git a/Server/ws/serve_auth.go b/Server/ws/serve_auth.go index 64bb2dba..74e3d188 100644 --- a/Server/ws/serve_auth.go +++ b/Server/ws/serve_auth.go @@ -4,6 +4,9 @@ import ( "context" "encoding/json" "fmt" + "log/slog" + "net/http" + "strings" "github.com/coder/websocket" @@ -103,3 +106,102 @@ func authenticateConn(parent context.Context, conn *websocket.Conn, database *db return user, hash, resumeHint{LastSeq: p.LastSeq, ChannelID: p.ActiveChannelID}, nil } + +// handshakeWrite writes one handshake-phase message (auth_ok, ready, or a +// replay event) under writeTimeout, instead of the bare ctx every caller here +// otherwise has on hand. +// +// Every handshake write runs against ctx = r.Context() from ServeWS. +// websocket.Accept hijacks the connection, which stops net/http's own +// mechanism for cancelling that context on client disconnect, so without this +// wrapper ctx is never cancelled while the handler is blocked inside +// conn.Write — a peer that stops reading (or whose receive window closes) +// pins the write, the handler goroutine, and the socket forever (OC-0152). +// writePumpWrite (serve_pumps.go) already bounds its writes the same way; +// this brings the handshake writes in serve.go up to the same guarantee. +func handshakeWrite(ctx context.Context, conn *websocket.Conn, msg []byte) error { + wCtx, cancel := context.WithTimeout(ctx, writeTimeout) + defer cancel() + return conn.Write(wCtx, websocket.MessageText, msg) +} + +func (h *Hub) upgradeAndAuth( + conn *websocket.Conn, database *db.DB, r *http.Request, +) (*Client, uint64, error) { + user, tokenHash, hint, err := authenticateConn(r.Context(), conn, database) + if err != nil { + slog.Warn("ws auth failed", "err", err, "remote", r.RemoteAddr) + _ = conn.Close(websocket.StatusPolicyViolation, "authentication failed") + return nil, 0, err + } + lastSeq := hint.LastSeq + + c := newClient(h, conn, user, tokenHash, lastSeq, r.Context()) + c.remoteAddr = r.RemoteAddr + // Untrusted until handleReconnect checks it against the allowed set. + c.authChannelID = hint.ChannelID + + // Look up role name for protocol-compliant payloads and cache on client. + // Fail closed like the sibling lookup in handleFreshConnect (BUG-094): + // this value is authoritative on the wire — auth_ok reports it as the + // user's own role, member_join broadcasts it to every other client, and + // every chat_message carries it — so a lookup failure must not silently + // substitute "member" and pin the whole session to a fabricated role + // (OC-0269). + role, roleErr := database.GetRoleByID(r.Context(), user.RoleID) + if roleErr != nil || role == nil { + slog.Error("ws: role lookup failed during handshake, closing connection", + "user_id", user.ID, "role_id", user.RoleID, "err", roleErr) + _ = conn.Close(websocket.StatusInternalError, "role lookup failed") + return nil, 0, fmt.Errorf("upgradeAndAuth: role lookup failed for user %d: %w", user.ID, roleErr) + } + c.roleName = strings.ToLower(role.Name) + + slog.Info("websocket connected", "username", user.Username, "user_id", user.ID, "remote", r.RemoteAddr) + db.WriteAudit(context.WithoutCancel(r.Context()), database, user.ID, "ws_connect", "user", user.ID, + "WebSocket connected from "+r.RemoteAddr) + + return c, lastSeq, nil +} + +// unregisterFailedHandshake removes c after a post-registerNow handshake +// write failure. No readPump ever starts for this connection — the +// fresh-connect callers return an error that stops ServeWS before it starts +// the pumps, and handleReconnect's callers report startPumps=false for the +// same reason (OC-0051) — and the old connection this one replaced already +// ran its defer (skipping teardown because this client held the slot) — so +// when no replacement remains, the standard disconnect teardown must run +// here or the user stays online forever. +func (h *Hub) unregisterFailedHandshake(ctx context.Context, c *Client) { + // Snapshot voice state BEFORE unregister, mirroring readPump's defer + // (serve_pumps.go): once unregisterNow removes c, there is no way to tell + // whether it still owned a (possibly just-transferred) voice session. + voiceChID := c.getVoiceChID() + replaced := h.unregisterNow(c) + if !replaced { + cleanupCtx := context.WithoutCancel(ctx) + // A connection that inherited a transferred voice session (the + // replay-failure fallback in handleFreshConnect deliberately keeps + // the voice_states row and registerNow transfers it onto c) must have + // that session torn down here too, or the row, the LiveKit + // participant, and a stale E2EE key-holder entry all survive this + // connection's death until the next sweep (up to 60s). + if voiceChID != 0 { + h.handleVoiceLeave(cleanupCtx, c) + } + } + // shouldMarkOffline re-checks h.clients rather than trusting the + // `replaced` snapshot alone: it was sampled before handleVoiceLeave, + // which can block for seconds, so a reconnect landing during that window + // would otherwise be invisible here and mark the live session's user + // offline (OC-0019, mirrored from readPump's defer in serve_pumps.go). + if h.shouldMarkOffline(c, replaced) { + cleanupCtx := context.WithoutCancel(ctx) + _ = h.db.MarkUserDisconnected(cleanupCtx, c.userID) + // custom_status is nil, not c.user.CustomStatus: see the identical + // note in serve_pumps.go's readPump defer — that field is an + // auth-time snapshot, never updated, so broadcasting it here can + // resurrect a status the user already changed or cleared. + h.QueuePresence(c.userID, db.StatusOffline, nil) + } +} diff --git a/Server/ws/serve_ready.go b/Server/ws/serve_ready.go index 53a56367..dec22dbd 100644 --- a/Server/ws/serve_ready.go +++ b/Server/ws/serve_ready.go @@ -5,6 +5,8 @@ import ( "fmt" "log/slog" + "github.com/coder/websocket" + "github.com/J3vb/OwnCord/Server/db" "github.com/J3vb/OwnCord/Server/permissions" ) @@ -373,3 +375,190 @@ func (h *Hub) buildReady(ctx context.Context, database *db.DB, userID int64, rol func collectAllVoiceStates(ctx context.Context, database *db.DB, _ []db.Channel) ([]db.VoiceState, error) { return database.GetAllVoiceStates(ctx) } + +func (h *Hub) handleFreshConnect( + ctx context.Context, conn *websocket.Conn, c *Client, database *db.DB, +) error { + // Clean stale voice state BEFORE building ready and registering. + // When a user F5-reloads while in voice, the DB row from the previous + // session must be removed so the ready payload doesn't include it and + // other clients see a voice_leave broadcast. + if vs, err := database.GetVoiceState(ctx, c.userID); err == nil && vs != nil { + 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). + userRole, roleErr := database.GetRoleByID(ctx, c.user.RoleID) + if roleErr != nil || userRole == nil { + slog.Error("ws: role lookup failed, disconnecting", "user_id", c.userID, "role_id", c.user.RoleID, "err", roleErr) + _ = conn.Close(websocket.StatusInternalError, "role lookup failed") + return fmt.Errorf("role lookup failed for user %d: %w", c.userID, roleErr) + } + + // Register BEFORE writing auth_ok + ready so broadcasts that arrive during + // the write window are queued in the client's send buffer instead of + // being lost (BUG-123). writePump hasn't started yet, so queued messages + // will be drained once the pumps begin. + // + // Only the replay-failure fallback (lastSeq > 0) can inherit voice state + // from the previous connection, so that is the only case where registerNow + // needs the read-permission set. Fail closed on error: nil denies the + // inherited voice-channel subscription. + var allowedChannelIDs map[int64]bool + if c.lastSeq > 0 { + allowed, allowedErr := h.computeAllowedChannels(ctx, database, c.user) + if allowedErr != nil { + slog.Warn("ws handleFreshConnect: computeAllowedChannels failed, skipping voice channel subscription", + "user_id", c.userID, "err", allowedErr) + } else { + allowedChannelIDs = allowed + } + } + // handleReconnect may have promoted an auth-frame active_channel_id into + // c.channelID (serve.go, honoured only when it was READ-visible at that + // moment) and then aborted on one of its own re-checks — most notably the + // final mustFullResync check, tripped by a permission revocation that + // landed mid-handshake. None of those abort paths undo the c.channelID + // write. registerNow subscribes c.channelID's ChannelTopic + // unconditionally, so re-gate it here against the freshly recomputed + // permission set before registering. Fail closed: a nil allowedChannelIDs + // (lastSeq == 0, or the computeAllowedChannels error branch above) denies. + if chID := c.getChannelID(); chID != 0 && !allowedChannelIDs[chID] { + c.mu.Lock() + 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) + + // Fresh connection or replay fallback: full auth_ok + ready flow. + slog.Info("ws sending auth_ok", "user_id", c.userID, "username", c.user.Username, "role", c.roleName) + if err := handshakeWrite(ctx, conn, h.buildAuthOK(ctx, c.user, c.roleName, "none")); err != nil { + slog.Warn("ws: failed to send auth_ok", "user_id", c.userID, "err", err) + h.unregisterFailedHandshake(ctx, c) + _ = conn.Close(websocket.StatusInternalError, "handshake failed") + return err + } + if ready, readyErr := h.buildReady(ctx, database, c.userID, userRole); readyErr == nil { + slog.Info("ws sending ready payload", "user_id", c.userID, "payload_bytes", len(ready)) + if err := handshakeWrite(ctx, conn, ready); err != nil { + slog.Warn("ws: failed to send ready payload", "user_id", c.userID, "err", err) + h.unregisterFailedHandshake(ctx, c) + _ = conn.Close(websocket.StatusInternalError, "handshake failed") + return err + } + } else { + slog.Error("buildReady failed", "user_id", c.userID, "err", readyErr) + _ = handshakeWrite(ctx, conn, buildErrorMsg(ErrCodeInternal, "failed to build ready payload")) + h.unregisterFailedHandshake(ctx, c) + _ = conn.Close(websocket.StatusInternalError, "failed to build ready payload") + return readyErr + } + + slog.Info("ws broadcasting member_join and presence", "user_id", c.userID, "username", c.user.Username) + h.BroadcastToAll(buildMemberJoin(c.user, c.roleName)) + h.announceConnectPresence(c) + + return nil +} + +// freshConnectCleanStaleVoice removes the voice state left behind by this +// user's previous session, unless that session is the still-registered +// connection this one is about to inherit from. +func (h *Hub) freshConnectCleanStaleVoice(ctx context.Context, database *db.DB, c *Client, vs *db.VoiceState) { + // Replay-failure fallback (lastSeq > 0): registerNow below transfers + // the still-registered old connection's live voice state into this + // client. Deleting the DB row here — and the LiveKit participant, + // whose removal token is the very JoinedAt being transferred — would + // leave the user "in voice" on the hub only: voice_join bounces off + // ALREADY_JOINED and sweepStaleVoiceStates never heals + // memory-without-row. Keep the row so ready stays consistent. If the + // old client unregisters before registerNow runs, the transfer is + // skipped and the next sweep reaps the then-truly-stale row. + if old := h.GetClient(c.userID); c.lastSeq > 0 && old != nil && old.getVoiceChID() == vs.ChannelID { + slog.Info("ws fresh connect: keeping voice state for replay-failure fallback", + "user_id", c.userID, "channel_id", vs.ChannelID) + return + } + slog.Info("ws fresh connect: cleaning stale voice state", + "user_id", c.userID, "channel_id", vs.ChannelID) + if _, delErr := database.LeaveVoiceChannelIfMatch(ctx, c.userID, vs.ChannelID, vs.JoinedAt); delErr != nil { + slog.Warn("ws fresh connect: LeaveVoiceChannelIfMatch failed", "err", delErr) + } + // The DB row is gone, but the still-registered OLD *Client (if any) is + // otherwise only cleared by registerNow — which two early-return paths + // further down handleFreshConnect (the refreshUserSnapshot and + // GetRoleByID failure branches) can skip entirely. Without this, that + // old client's in-memory voiceChID and the E2EE key-holder election for + // this room survive as a memory-without-row ghost that + // sweepStaleVoiceStates can never see, since it iterates DB rows + // (OC-0252). Clearing here makes freshConnectCleanStaleVoice self + // sufficient regardless of whether registerNow ever runs; registerNow's + // own replacedVoiceChID re-election later becomes a redundant no-op + // (clearVoiceState finds nothing left to clear), not a conflict. + if old := h.GetClient(c.userID); old != nil { + if _, cleared := old.clearVoiceStateIfMatch(vs.ChannelID); cleared { + h.pubsub.Unsubscribe(old, VoiceTopic(vs.ChannelID)) + } + } + h.updateKeyHolder(vs.ChannelID) + h.broadcastVoiceEvent(ctx, vs.ChannelID, buildVoiceLeave(vs.ChannelID, c.userID)) + if h.livekit == nil { + return + } + // BUG-089: Capture stale join token so the goroutine only removes + // the exact stale participant. The identity includes joinedAt, so + // even if the user rejoins voice quickly, the new session has a + // different identity and won't be removed. The removal must + // complete even if this connection drops mid-handshake, so detach + // from cancellation (values kept); shutdown is handled via h.stop. + staleChID, staleUserID, staleJoinToken := vs.ChannelID, c.userID, vs.JoinedAt + lkCtx := context.WithoutCancel(ctx) + go func() { + select { + case <-h.stop: + return + default: + } + if err := h.livekit.RemoveParticipant(lkCtx, staleChID, staleUserID, staleJoinToken); err != nil { + slog.Warn("ws fresh connect: RemoveParticipant failed (may already be gone)", + "err", err, "user_id", staleUserID, "channel_id", staleChID) + } + }() +} diff --git a/docs/architecture/server-boundaries.md b/docs/architecture/server-boundaries.md index d1aa35ed..d67bdcb0 100644 --- a/docs/architecture/server-boundaries.md +++ b/docs/architecture/server-boundaries.md @@ -6,7 +6,9 @@ after-state at `fe1d11b8` (pre-squash; the squash SHA is in the plan's B3-2 evidence block); 2026-08-30 (B3-3) — the first table again, and the hub lifecycle section's after-state rows, on `feat/b3-3-lifecycle`; 2026-08-31 (B3-4) — the construction-and-setters after-state, on -`feat/b3-4-hub-options`. +`feat/b3-4-hub-options`; 2026-08-31 (B3-5, first split PR) — the first table, +on `feat/b3-5-ws-split` (handshake auth and fresh-connect rows moved with +their code). **Owner:** the B3 plan, [plans/b3-server-architecture-guardrails-2026-08-29.md](../plans/b3-server-architecture-guardrails-2026-08-29.md). **Regenerate the first table:** `cd Server && go run ./cmd/dbinventory` and @@ -61,66 +63,66 @@ which is a row worth reading, and none exists today. -| File | `db.*` types | `db.*` funcs and sentinels | `*db.DB` method calls | Shape | Disposition | Family | Why | -| --------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ----------- | ------------ | ------------------------------------------------------------------ | -| `admin/admin.go` | `DB` | — | — | type-only | boundary | — | holds the handle for the admin mux; no calls | -| `admin/api.go` | `DB` | — | — | type-only | boundary | — | passes the handle to handlers; no calls | -| `admin/backup_maintenance.go` | `DB×3` | `CheckBackupIntegrity()` `ErrNotFound×2` `WriteAudit()×2` | `BackupToSafe` `GetSetting×2` | calls | move | settings-ops | BackupToSafe, integrity check, settings reads | -| `admin/handlers_backup.go` | `DB×5` | `CheckBackupIntegrity()×2` `WriteAudit()×2` | `BackupToSafe×2` `Close` `LogAudit` `SQLDb` | calls | move | settings-ops | backup trigger and download; raw SQLDb for VACUUM INTO | -| `admin/handlers_channel_perms.go` | `Channel` `ChannelRoleOverride×2` `ChannelUserOverride×2` `DB×8` `Role×2` `User×2` | `WriteAudit()×4` | `DeleteChannelOverride` `DeleteChannelUserOverride` `GetChannel` `GetChannelPermissions×2` `GetRoleByID×3` `GetUserByID` `GetUserChannelPermissions×2` `ListChannelRoleOverrides` `ListChannelUserOverrides` `ListUserIDsByRole×2` `UpsertChannelOverride` `UpsertChannelUserOverride` | calls | move | channel | override CRUD decides permission policy in the handler | -| `admin/handlers_channels.go` | `Channel×2` `ChannelUpdate×2` `DB×6` | `WriteAudit()×3` | `AdminCreateChannel` `AdminDeleteChannel` `AdminUpdateChannel×2` `GetAuditLog` `GetChannel×4` `ListChannels` | calls | move | channel | channel CRUD + audit | -| `admin/handlers_roles.go` | `DB×4` | — | `GetRoleByID` `ListRoles` | calls | move | role | two reads; service/role.go already owns the writes | -| `admin/handlers_settings.go` | `DB×4` | `ErrNotFound` `WriteAudit()` | `BeginTx` `CountUsersWithoutTOTP` `GetAllSettings×2` `GetSetting` | calls | move | settings-ops | BeginTx in a handler; TOTP census | -| `admin/handlers_tokens.go` | `DB×3` `User` | `WriteAudit()×2` | `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` | calls | move | auth | API-token CRUD duplicated in token_cli.go | -| `admin/handlers_users.go` | `DB×4` `Role` `User` | — | `GetServerStats` `GetUserByID×2` `ListAllUsers` | calls | move | user | user list, stats, lookups | -| `admin/helpers.go` | `Role×2` `User` | — | — | type-only | adapter | — | Role/User types in response helpers | -| `admin/logstream.go` | `DB×3` | — | — | type-only | boundary | — | handle threaded to the SSE stream's auth check; no calls | -| `admin/middleware.go` | `DB×3` `Role` `User` | — | `GetRoleByID` | calls | move | auth | owner gate re-reads the role — OC-0345 | -| `admin/setup_handler.go` | `DB×4` | `ErrConflict` `WriteAudit()×2` | `CreateChannel×2` `CreateInvite` `CreateOwnerIfEmpty` `CreateSession` `GetSetting×3` `UserCount` | calls | move | auth | first-run owner creation (setup sub-family) | -| `admin/setup_wizard.go` | `DB` | — | `BeginTx` | calls | move | auth | BeginTx for the wizard; setup sub-family | -| `admin/types.go` | `Channel×3` `DB` `Role` `User` `UserWithRole` | — | `GetRoleByID` | calls | adapter | — | response DTOs; the one GetRoleByID moves with handlers_users | -| `api/channel_handler.go` | `DB` `MessageAPIResponse×2` `MessageSearchResult×2` `ReactionUser` `User×8` | — | — | type-only | adapter | — | response types only; service owns the calls | -| `api/dm_handler.go` | `DB` `DMChannelInfo` `DMUser×2` `User×8` | `NewDMChannelInfo()` `StatusForViewer()` | — | calls | adapter | — | DM response types + pure status helpers | -| `api/emoji_handler.go` | `DB` `Emoji×3` `User×2` | — | — | type-only | adapter | — | Emoji/User types only | -| `api/gif_handler.go` | `DB` | — | — | type-only | adapter | — | handle in the signature, unused for calls | -| `api/invite_handler.go` | `DB` `Invite` `User×2` | — | — | type-only | adapter | — | Invite/User types only | -| `api/middleware.go` | `DB` `Role` `Session` `User` | — | `DeleteSession` `TouchAPIToken` `TouchSession` | calls | move | auth | session/API-token touch and revoke | -| `api/plugins_handler.go` | `Auditor×2` | `WriteAudit()` | — | calls | adapter | — | db.Auditor is the seam; WriteAudit only | -| `api/profile_handler.go` | `DB×2` `Session×2` `User×7` | — | `CreateAttachment` | calls | move | upload | avatar upload creates the attachment row | -| `api/router.go` | `DB×4` | — | `PingRead` `SQLDb` | calls | boundary | — | health probe (PingRead, SQLDb); hub construction left in B3-3 | -| `api/upload_handler.go` | `AttachmentAccess×2` `DB×5` `Role` `User×3` | — | `CreateAttachment` `GetAttachmentWithChannel` `IsAvatarFileURL` `IsDMParticipant` `QueryRowContext` | calls | move | upload | attachment access + a raw QueryRowContext | -| `auth/helpers.go` | `User` | — | — | type-only | adapter | — | db.User type in a helper signature | -| `auth/resolve.go` | `APIToken` `Role×2` `Session×2` `User×2` | — | — | type-only | adapter | — | Session/APIToken/Role/User types; resolution is injected | -| `cmd/gendocs/main.go` | `DB×3` | `Migrate()` `Open()` | `Close×2` `QueryContext×2` | calls | boundary | — | docs generator migrates its own in-memory catalog | -| `cmd/seed/main.go` | `DB×6` | `Migrate()` `Open()` | `Close` `CreateChannel` `CreateMessage×2` `CreateUser` `GetOrCreateDMChannel` `GetUserByUsername` `ListChannels` `QueryRowContext` | calls | boundary | — | developer seeding tool owns its handle | -| `cmd/seed/profile_alpha.go` | `DB×3` | `Migrate()` | `BeginTx` `ExecContext×2` `QueryRowContext` | calls | boundary | — | the alpha profile writes through the handle main.go owns | -| `internal/app/app.go` | `AuditWriter` `DB` | — | — | type-only | boundary | — | the App holds the handle for its lifetime; no calls | -| `internal/app/database.go` | `DB×2` | `Migrate()` `OpenWithMaxReaders()` | `ClearAllVoiceStates` `ResetAllUserStatuses` | calls | boundary | — | opens the handle, migrates, clears stale state at boot | -| `internal/app/hub.go` | `DB` | — | — | type-only | boundary | — | hands the handle to the hub and the service layer it builds | -| `internal/app/maintenance.go` | `DB×3` | — | `DeleteExpiredSessions` `DeleteOrphanedAttachments` | calls | boundary | — | periodic worker: expired sessions, backups, orphan attachments | -| `internal/app/persistence.go` | `AuditWriter×2` `DB×4` | `ErrNotFound` `NewAuditWriter()` | `GetMaxEventSeq` `GetSetting` `SetAuditWriter` `SetSetting` | calls | boundary | — | event persister, audit writer and the boot seq seed own the handle | -| `internal/app/plugins.go` | `DB` | — | — | type-only | boundary | — | passes the handle to the plugin registry as its store; no calls | -| `plugin/pluginstore.go` | `PluginRow×3` | — | — | type-only | adapter | — | PluginRow type only; the store is injected | -| `token_cli.go` | `DB×3` `User` | `Migrate()` `OpenShared()` `WriteAudit()×3` | `Close` `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` `RevokeAPITokenByLabel` | calls | move | auth | API-token CLI duplicates admin/handlers_tokens.go | -| `ws/client.go` | `User×2` | — | — | type-only | adapter | — | db.User type on the connection | -| `ws/deps.go` | `Channel` `DB×5` | — | `GetRoleForUser×3` `IsDMParticipant` | calls | move | channel | role and DM-membership reads behind the hub's deps | -| `ws/event.go` | — | `BroadcastStatus()` | — | calls | adapter | — | pure BroadcastStatus helper | -| `ws/event_persister.go` | `PersistedEvent×2` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | -| `ws/eventstore.go` | `PersistedEvent×3` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | -| `ws/handlers.go` | `User` | — | `GetChannel` `GetRoleForUser` `GetSessionWithBanStatus` `IsDMParticipant` | calls | move | channel | channel, role, session-ban and DM reads in command handlers | -| `ws/handlers_chat.go` | — | `NewDMChannelInfo()` | — | calls | adapter | — | pure NewDMChannelInfo helper | -| `ws/hub.go` | `DB×2` | — | `GetSetting×2` | calls | move | settings-ops | GetSetting at construction | -| `ws/hub_broadcast.go` | `Channel×4` `Emoji` `Role` | `BroadcastStatus()` | `GetChannel×2` `GetDMParticipantIDs` `GetRoleForUser` `GetUserByID×2` `ListChannels` | calls | move | channel | visibility refresh reads channels, roles, users | -| `ws/hub_sweep.go` | `User` | — | `GetAllVoiceStates` `GetChannel` `GetChannelVoiceStates` `GetSessionsWithBanStatusBatch` `LeaveVoiceChannelIfMatch×2` | calls | move | voice | stale-voice sweep reads and leaves | -| `ws/messages.go` | `Channel×4` `DMChannelInfo×2` `DMUser×2` `Emoji` `Role×3` `User×2` `VoiceState` | `BroadcastStatus()` `StatusForViewer()` | — | calls | adapter | — | wire types + pure status helpers | -| `ws/serve.go` | `ChannelOverride` `DB×9` `PersistedEvent` `User` `VoiceState` | `ConnectStatus()` `StatusOffline` `WriteAudit()` | `GetChannelOverridesFor` `GetRoleByID×4` `GetUserByID×2` `GetUserDMChannelIDs` `GetVoiceState` `LeaveVoiceChannelIfMatch` `ListChannels` `MarkUserDisconnected` `UpdateUserStatus` | calls | move | connection | connect/disconnect lifecycle; B3-5 splits it by family first | -| `ws/serve_auth.go` | `DB` `User` | — | `GetSessionByTokenHash` `GetUserByID` | calls | move | auth | session lookup on the WebSocket handshake | -| `ws/serve_pumps.go` | — | `StatusOffline` | `MarkUserDisconnected` | calls | move | user | MarkUserDisconnected on pump exit | -| `ws/serve_ready.go` | `Channel×10` `ChannelOverride×6` `ChannelUnread×2` `DB×5` `DMChannelInfo×4` `MemberSummary×3` `Role×4` `User` `VoiceState×4` | `StatusOffline×3` | `GetAllVoiceStates` `GetChannelOverridesFor` `GetChannelUnreadCounts` `GetUserDMChannels` `ListChannels` `ListMembers` `ListRoles` | calls | move | channel | ready snapshot: channels, overrides, unreads, DMs, members | -| `ws/voice_join.go` | `Channel×3` `ChannelOverride` `VoiceState×5` | `ErrChannelFull` | `GetChannel×2` `GetChannelOverridesFor` `GetChannelVoiceStates` `GetRoleForUser` `GetVoiceState×6` `JoinVoiceChannel` `JoinVoiceChannelIfCapacity` `LeaveVoiceChannelIfMatch` `SetVoiceServerDeafen` `SetVoiceServerMute` | calls | move | voice | voice state reads and writes | -| `ws/voice_moderation.go` | `Role` `VoiceState×3` | `WriteAudit()` | `CountChannelVoiceUsers` `GetChannel×2` `GetRoleForUser` `GetVoiceState×2` `SetVoiceServerDeafen×2` `SetVoiceServerMute×2` | calls | move | voice | mute/deafen/move persist voice state | +| File | `db.*` types | `db.*` funcs and sentinels | `*db.DB` method calls | Shape | Disposition | Family | Why | +| --------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ----------- | ------------ | ------------------------------------------------------------------------------------------------- | +| `admin/admin.go` | `DB` | — | — | type-only | boundary | — | holds the handle for the admin mux; no calls | +| `admin/api.go` | `DB` | — | — | type-only | boundary | — | passes the handle to handlers; no calls | +| `admin/backup_maintenance.go` | `DB×3` | `CheckBackupIntegrity()` `ErrNotFound×2` `WriteAudit()×2` | `BackupToSafe` `GetSetting×2` | calls | move | settings-ops | BackupToSafe, integrity check, settings reads | +| `admin/handlers_backup.go` | `DB×5` | `CheckBackupIntegrity()×2` `WriteAudit()×2` | `BackupToSafe×2` `Close` `LogAudit` `SQLDb` | calls | move | settings-ops | backup trigger and download; raw SQLDb for VACUUM INTO | +| `admin/handlers_channel_perms.go` | `Channel` `ChannelRoleOverride×2` `ChannelUserOverride×2` `DB×8` `Role×2` `User×2` | `WriteAudit()×4` | `DeleteChannelOverride` `DeleteChannelUserOverride` `GetChannel` `GetChannelPermissions×2` `GetRoleByID×3` `GetUserByID` `GetUserChannelPermissions×2` `ListChannelRoleOverrides` `ListChannelUserOverrides` `ListUserIDsByRole×2` `UpsertChannelOverride` `UpsertChannelUserOverride` | calls | move | channel | override CRUD decides permission policy in the handler | +| `admin/handlers_channels.go` | `Channel×2` `ChannelUpdate×2` `DB×6` | `WriteAudit()×3` | `AdminCreateChannel` `AdminDeleteChannel` `AdminUpdateChannel×2` `GetAuditLog` `GetChannel×4` `ListChannels` | calls | move | channel | channel CRUD + audit | +| `admin/handlers_roles.go` | `DB×4` | — | `GetRoleByID` `ListRoles` | calls | move | role | two reads; service/role.go already owns the writes | +| `admin/handlers_settings.go` | `DB×4` | `ErrNotFound` `WriteAudit()` | `BeginTx` `CountUsersWithoutTOTP` `GetAllSettings×2` `GetSetting` | calls | move | settings-ops | BeginTx in a handler; TOTP census | +| `admin/handlers_tokens.go` | `DB×3` `User` | `WriteAudit()×2` | `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` | calls | move | auth | API-token CRUD duplicated in token_cli.go | +| `admin/handlers_users.go` | `DB×4` `Role` `User` | — | `GetServerStats` `GetUserByID×2` `ListAllUsers` | calls | move | user | user list, stats, lookups | +| `admin/helpers.go` | `Role×2` `User` | — | — | type-only | adapter | — | Role/User types in response helpers | +| `admin/logstream.go` | `DB×3` | — | — | type-only | boundary | — | handle threaded to the SSE stream's auth check; no calls | +| `admin/middleware.go` | `DB×2` `Role×2` | — | — | type-only | move | auth | owner gate re-reads the role — OC-0345 | +| `admin/setup_handler.go` | `DB×4` | `ErrConflict` `WriteAudit()×2` | `CreateChannel×2` `CreateInvite` `CreateOwnerIfEmpty` `CreateSession` `GetSetting×3` `UserCount` | calls | move | auth | first-run owner creation (setup sub-family) | +| `admin/setup_wizard.go` | `DB` | — | `BeginTx` | calls | move | auth | BeginTx for the wizard; setup sub-family | +| `admin/types.go` | `Channel×3` `DB` `Role` `User` `UserWithRole` | — | `GetRoleByID` | calls | adapter | — | response DTOs; the one GetRoleByID moves with handlers_users | +| `api/channel_handler.go` | `DB` `MessageAPIResponse×2` `MessageSearchResult×2` `ReactionUser` `User×8` | — | — | type-only | adapter | — | response types only; service owns the calls | +| `api/dm_handler.go` | `DB` `DMChannelInfo` `DMUser×2` `User×8` | `NewDMChannelInfo()` `StatusForViewer()` | — | calls | adapter | — | DM response types + pure status helpers | +| `api/emoji_handler.go` | `DB` `Emoji×3` `User×2` | — | — | type-only | adapter | — | Emoji/User types only | +| `api/gif_handler.go` | `DB` | — | — | type-only | adapter | — | handle in the signature, unused for calls | +| `api/invite_handler.go` | `DB` `Invite` `User×2` | — | — | type-only | adapter | — | Invite/User types only | +| `api/middleware.go` | `DB` `Role` `Session` `User` | — | `DeleteSession` `TouchAPIToken` `TouchSession` | calls | move | auth | session/API-token touch and revoke | +| `api/plugins_handler.go` | `Auditor×2` | `WriteAudit()` | — | calls | adapter | — | db.Auditor is the seam; WriteAudit only | +| `api/profile_handler.go` | `DB×2` `Session×2` `User×7` | — | `CreateAttachment` | calls | move | upload | avatar upload creates the attachment row | +| `api/router.go` | `DB×4` | — | `PingRead` `SQLDb` | calls | boundary | — | health probe (PingRead, SQLDb); hub construction left in B3-3 | +| `api/upload_handler.go` | `AttachmentAccess×2` `DB×5` `Role` `User×3` | — | `CreateAttachment` `GetAttachmentWithChannel` `IsAvatarFileURL` `IsDMParticipant` `QueryRowContext` | calls | move | upload | attachment access + a raw QueryRowContext | +| `auth/helpers.go` | `User` | — | — | type-only | adapter | — | db.User type in a helper signature | +| `auth/resolve.go` | `APIToken` `Role×2` `Session×2` `User×2` | — | — | type-only | adapter | — | Session/APIToken/Role/User types; resolution is injected | +| `cmd/gendocs/main.go` | `DB×3` | `Migrate()` `Open()` | `Close×2` `QueryContext×2` | calls | boundary | — | docs generator migrates its own in-memory catalog | +| `cmd/seed/main.go` | `DB×6` | `Migrate()` `Open()` | `Close` `CreateChannel` `CreateMessage×2` `CreateUser` `GetOrCreateDMChannel` `GetUserByUsername` `ListChannels` `QueryRowContext` | calls | boundary | — | developer seeding tool owns its handle | +| `cmd/seed/profile_alpha.go` | `DB×3` | `Migrate()` | `BeginTx` `ExecContext×2` `QueryRowContext` | calls | boundary | — | the alpha profile writes through the handle main.go owns | +| `internal/app/app.go` | `AuditWriter` `DB` | — | — | type-only | boundary | — | the App holds the handle for its lifetime; no calls | +| `internal/app/database.go` | `DB×2` | `Migrate()` `OpenWithMaxReaders()` | `ClearAllVoiceStates` `ResetAllUserStatuses` | calls | boundary | — | opens the handle, migrates, clears stale state at boot | +| `internal/app/hub.go` | `DB` | — | — | type-only | boundary | — | hands the handle to the hub and the service layer it builds | +| `internal/app/maintenance.go` | `DB×3` | — | `DeleteExpiredSessions` `DeleteOrphanedAttachments` | calls | boundary | — | periodic worker: expired sessions, backups, orphan attachments | +| `internal/app/persistence.go` | `AuditWriter×2` `DB×4` | `ErrNotFound` `NewAuditWriter()` | `GetMaxEventSeq` `GetSetting` `SetAuditWriter` `SetSetting` | calls | boundary | — | event persister, audit writer and the boot seq seed own the handle | +| `internal/app/plugins.go` | `DB` | — | — | type-only | boundary | — | passes the handle to the plugin registry as its store; no calls | +| `plugin/pluginstore.go` | `PluginRow×3` | — | — | type-only | adapter | — | PluginRow type only; the store is injected | +| `token_cli.go` | `DB×3` `User` | `Migrate()` `OpenShared()` `WriteAudit()×3` | `Close` `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` `RevokeAPITokenByLabel` | calls | move | auth | API-token CLI duplicates admin/handlers_tokens.go | +| `ws/client.go` | `User×2` | — | — | type-only | adapter | — | db.User type on the connection | +| `ws/deps.go` | `Channel` `DB×5` | — | `GetRoleForUser×3` `IsDMParticipant` | calls | move | channel | role and DM-membership reads behind the hub's deps | +| `ws/event.go` | — | `BroadcastStatus()` | — | calls | adapter | — | pure BroadcastStatus helper | +| `ws/event_persister.go` | `PersistedEvent×2` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | +| `ws/eventstore.go` | `PersistedEvent×3` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | +| `ws/handlers.go` | `User` | — | `GetChannel` `GetRoleForUser` `GetSessionWithBanStatus` `IsDMParticipant` | calls | move | channel | channel, role, session-ban and DM reads in command handlers | +| `ws/handlers_chat.go` | — | `NewDMChannelInfo()` | — | calls | adapter | — | pure NewDMChannelInfo helper | +| `ws/hub.go` | `DB×2` | — | `GetSetting×2` | calls | move | settings-ops | GetSetting at construction | +| `ws/hub_broadcast.go` | `Channel×4` `Emoji` `Role` | `BroadcastStatus()` | `GetChannel×2` `GetDMParticipantIDs` `GetRoleForUser` `GetUserByID×2` `ListChannels` | calls | move | channel | visibility refresh reads channels, roles, users | +| `ws/hub_sweep.go` | `User` | — | `GetAllVoiceStates` `GetChannel` `GetChannelVoiceStates` `GetSessionsWithBanStatusBatch` `LeaveVoiceChannelIfMatch×2` | calls | move | voice | stale-voice sweep reads and leaves | +| `ws/messages.go` | `Channel×4` `DMChannelInfo×2` `DMUser×2` `Emoji` `Role×3` `User×2` `VoiceState` | `BroadcastStatus()` `StatusForViewer()` | — | calls | adapter | — | wire types + pure status helpers | +| `ws/serve.go` | `ChannelOverride` `DB×6` `PersistedEvent` `User` | `ConnectStatus()` | `GetChannelOverridesFor` `GetRoleByID×2` `GetUserByID` `GetUserDMChannelIDs` `ListChannels` `UpdateUserStatus` | calls | move | connection | connect/disconnect lifecycle; B3-5 splits it by family first | +| `ws/serve_auth.go` | `DB×2` `User` | `StatusOffline` `WriteAudit()` | `GetRoleByID` `GetSessionByTokenHash` `GetUserByID` `MarkUserDisconnected` | calls | move | auth | handshake auth: session, user and role lookups, connect audit, failed-handshake teardown | +| `ws/serve_pumps.go` | — | `StatusOffline` | `MarkUserDisconnected` | calls | move | user | MarkUserDisconnected on pump exit | +| `ws/serve_ready.go` | `Channel×10` `ChannelOverride×6` `ChannelUnread×2` `DB×7` `DMChannelInfo×4` `MemberSummary×3` `Role×4` `User` `VoiceState×5` | `StatusOffline×3` | `GetAllVoiceStates` `GetChannelOverridesFor` `GetChannelUnreadCounts` `GetRoleByID` `GetUserByID` `GetUserDMChannels` `GetVoiceState` `LeaveVoiceChannelIfMatch` `ListChannels` `ListMembers` `ListRoles` | calls | move | channel | ready snapshot and fresh-connect: channels, overrides, unreads, DMs, members, stale-voice cleanup | +| `ws/voice_join.go` | `Channel×3` `ChannelOverride` `VoiceState×5` | `ErrChannelFull` | `GetChannel×2` `GetChannelOverridesFor` `GetChannelVoiceStates` `GetRoleForUser` `GetVoiceState×6` `JoinVoiceChannel` `JoinVoiceChannelIfCapacity` `LeaveVoiceChannelIfMatch` `SetVoiceServerDeafen` `SetVoiceServerMute` | calls | move | voice | voice state reads and writes | +| `ws/voice_moderation.go` | `Role` `VoiceState×3` | `WriteAudit()` | `CountChannelVoiceUsers` `GetChannel×2` `GetRoleForUser` `GetVoiceState×2` `SetVoiceServerDeafen×2` `SetVoiceServerMute×2` | calls | move | voice | mute/deafen/move persist voice state | -56 files import `db` outside `db/` and `service/` (. 1, admin 16, api 10, auth 2, cmd/gendocs 1, cmd/seed 2, internal/app 6, plugin 1, ws 17); 17 are type-only; 0 unlisted. +56 files import `db` outside `db/` and `service/` (. 1, admin 16, api 10, auth 2, cmd/gendocs 1, cmd/seed 2, internal/app 6, plugin 1, ws 17); 18 are type-only; 0 unlisted. Dispositions: adapter 17, boundary 13, move 26. Move targets: auth 7, channel 6, connection 1, role 1, settings-ops 4, upload 2, user 2, voice 3. @@ -130,11 +132,12 @@ Reading the table: - **Type-only files (14)** need no service; they stay `adapter`. The `db` types they use are the wire and response shapes. Whether those types should live outside `db` is a B3-8 question per family, not a boundary violation. -- **`ws/serve_ready.go`** (45 references, 7 distinct queries) is the single +- **`ws/serve_ready.go`** (48 references, 11 distinct queries) is the single heaviest reader: the ready snapshot reads channels, overrides, unreads, DM - channels, members, roles and voice states in one place. B3-5 keeps it as - the "fresh-connect initialisation" file and B3-8's channel family gives it - a snapshot service. + channels, members, roles and voice states in one place. B3-5 made it the + "fresh-connect initialisation" file (`handleFreshConnect` and its + stale-voice cleanup moved in from `serve.go`) and B3-8's channel family + gives it a snapshot service. - **Two raw SQL escapes** exist above the domain layer: `api/upload_handler.go` (`QueryRowContext`) and `admin/handlers_backup.go` (`SQLDb` for `VACUUM INTO`). Both are `move`; the backup one may end as an diff --git a/docs/plans/README.md b/docs/plans/README.md index aac31d25..618dd7d3 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -10,22 +10,22 @@ authority**. ## Active — these drive current work -| Plan | State | -| ----------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| [beta-product-requirements-2026-08-23](beta-product-requirements-2026-08-23.md) | Approved beta scope, frozen. 57 `BPR-*` requirements. **Amended 2026-08-31:** BPR-032 reworded to the slim single-current-epoch policy (owner decision; the scope HP-2 accepted). | -| [repo-health-roadmap-2026-08-23](repo-health-roadmap-2026-08-23.md) | Phase order and gates, B0–B10. **B0, B1 and B2 complete** (HP-0, HP-1 and HP-2 all accepted); **B3 in progress** — the B3 plan row below tracks it. B4–B10 not started. **Amended 2026-08-28:** dated lines in B3–B10, a "Phase execution pattern" section, and a truthful status header. **Amended 2026-08-29:** B3/B7/B9 lines binding the layout-refactor supplement below; current slice updated for B2-2. **Amended 2026-08-31:** B1 exit reworded to identical-tree integration evidence; B6/B8/B10 `server-info` and epoch-window lines aligned with the B2-2 slim decision; B10 gains the BPR-051 comprehension-read row (owner decisions 2026-08-31). | -| [repo-health-issue-register-2026-08-23](repo-health-issue-register-2026-08-23.md) | 88 planning rows. Public-safe; not a replacement for the ledger. | -| [beta-requirements-traceability-2026-08-23](beta-requirements-traceability-2026-08-23.md) | Requirement → phase → evidence map. No row is release-qualified. | -| [b0-baseline-2026-08-25](b0-baseline-2026-08-25.md) | **Supersedes the roadmap's "current evidence snapshot."** B0 measurements and dispositions. | -| [b1-repository-foundation-2026-08-25](b1-repository-foundation-2026-08-25.md) | **B1-0 through B1-8 all done.** B1 execution plan. Re-verifies every RL-\* claim against HEAD; several are refuted. | -| [hp-0-scorecard-2026-08-25](hp-0-scorecard-2026-08-25.md) | **HP-0 accepted 2026-08-25.** The single baseline-acceptance artifact. Part-closes `R-08`. | -| [hp-1-scorecard-2026-08-27](hp-1-scorecard-2026-08-27.md) | **HP-1 accepted 2026-08-27.** Structural-diff proofs for the flatten and module rename, plus the B1 exit gate. | -| [b2-protocol-trust-compat-2026-08-28](b2-protocol-trust-compat-2026-08-28.md) | **B2 complete — HP-2 accepted 2026-08-29.** B2-0, B2-1, B2-8 done 2026-08-28; B2-2 (B2-3/B2-4 folded in), B2-5, B2-6, B2-7, B2-9 done 2026-08-29. Scorecard below. | -| [hp-2-scorecard-2026-08-29](hp-2-scorecard-2026-08-29.md) | **HP-2 accepted 2026-08-29.** Seven questions answered with commands; B2 exit gate, nine conditions met (1 at the slim epoch scope, 4 with one E2EE gap disclaimed). Owner follow-ups that do not gate B3: BPR-051 reader line, SEC-01/SEC-04 advisory IDs. | -| [b3-server-architecture-guardrails-2026-08-29](b3-server-architecture-guardrails-2026-08-29.md) | **B3 in progress from 2026-08-29.** Execution plan: B3-0 inventory → B3-1/B3-2 auth slice → HP-3 → lifecycle, hub options, `ws` split, families; guardrails and the alpha dataset beside the slice. B3-0 (inventory + `db-import-boundary` rule) and B3-1 (auth characterization) merged 2026-08-29; B3-2 (auth vertical slice) merged 2026-08-30; B3-9 (five of the six B3-tagged findings; OC-0323 rides B3-8) merged 2026-08-30; HP-3 accepted 2026-08-30; B3-3 (lifecycle extraction into `Server/internal/app/`, one composite close, hub construction out of `api.NewRouter`) is PR #1464, opened 2026-08-30 — B3-4 next. | -| [hp-3-scorecard-2026-08-29](hp-3-scorecard-2026-08-29.md) | **HP-3 accepted 2026-08-30 by the owner.** Five questions on the auth vertical slice answered with commands: frozen set green at every pre-squash SHA, `api` db importers 12 → 10, B2 contracts unchanged, the pattern written as D4 in server.md, guardrails as they exist. | -| [b3-bench-baseline-2026-08-30](b3-bench-baseline-2026-08-30.md) | **Recorded 2026-08-30, not gated.** The six B3-6 `Benchmark*` through `benchstat` at `ec8ef24a`, produced by `make bench-baseline`. Nothing in CI reads these numbers; the performance gate is B6's. Regenerating writes a new dated file — replace this row and delete the superseded document, so only the newest baseline is kept. | -| [audit-2026-08-19-remediation](audit-2026-08-19-remediation.md) | Phases 1–6 done 2026-08-20; **phase 7 pending**. Its header still reads "in progress 2026-08-19" — stale; the phase table is correct. | +| Plan | State | +| ----------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [beta-product-requirements-2026-08-23](beta-product-requirements-2026-08-23.md) | Approved beta scope, frozen. 57 `BPR-*` requirements. **Amended 2026-08-31:** BPR-032 reworded to the slim single-current-epoch policy (owner decision; the scope HP-2 accepted). | +| [repo-health-roadmap-2026-08-23](repo-health-roadmap-2026-08-23.md) | Phase order and gates, B0–B10. **B0, B1 and B2 complete** (HP-0, HP-1 and HP-2 all accepted); **B3 in progress** — the B3 plan row below tracks it. B4–B10 not started. **Amended 2026-08-28:** dated lines in B3–B10, a "Phase execution pattern" section, and a truthful status header. **Amended 2026-08-29:** B3/B7/B9 lines binding the layout-refactor supplement below; current slice updated for B2-2. **Amended 2026-08-31:** B1 exit reworded to identical-tree integration evidence; B6/B8/B10 `server-info` and epoch-window lines aligned with the B2-2 slim decision; B10 gains the BPR-051 comprehension-read row (owner decisions 2026-08-31). | +| [repo-health-issue-register-2026-08-23](repo-health-issue-register-2026-08-23.md) | 88 planning rows. Public-safe; not a replacement for the ledger. | +| [beta-requirements-traceability-2026-08-23](beta-requirements-traceability-2026-08-23.md) | Requirement → phase → evidence map. No row is release-qualified. | +| [b0-baseline-2026-08-25](b0-baseline-2026-08-25.md) | **Supersedes the roadmap's "current evidence snapshot."** B0 measurements and dispositions. | +| [b1-repository-foundation-2026-08-25](b1-repository-foundation-2026-08-25.md) | **B1-0 through B1-8 all done.** B1 execution plan. Re-verifies every RL-\* claim against HEAD; several are refuted. | +| [hp-0-scorecard-2026-08-25](hp-0-scorecard-2026-08-25.md) | **HP-0 accepted 2026-08-25.** The single baseline-acceptance artifact. Part-closes `R-08`. | +| [hp-1-scorecard-2026-08-27](hp-1-scorecard-2026-08-27.md) | **HP-1 accepted 2026-08-27.** Structural-diff proofs for the flatten and module rename, plus the B1 exit gate. | +| [b2-protocol-trust-compat-2026-08-28](b2-protocol-trust-compat-2026-08-28.md) | **B2 complete — HP-2 accepted 2026-08-29.** B2-0, B2-1, B2-8 done 2026-08-28; B2-2 (B2-3/B2-4 folded in), B2-5, B2-6, B2-7, B2-9 done 2026-08-29. Scorecard below. | +| [hp-2-scorecard-2026-08-29](hp-2-scorecard-2026-08-29.md) | **HP-2 accepted 2026-08-29.** Seven questions answered with commands; B2 exit gate, nine conditions met (1 at the slim epoch scope, 4 with one E2EE gap disclaimed). Owner follow-ups that do not gate B3: BPR-051 reader line, SEC-01/SEC-04 advisory IDs. | +| [b3-server-architecture-guardrails-2026-08-29](b3-server-architecture-guardrails-2026-08-29.md) | **B3 in progress from 2026-08-29.** Execution plan: B3-0 inventory → B3-1/B3-2 auth slice → HP-3 → lifecycle, hub options, `ws` split, families; guardrails and the alpha dataset beside the slice. B3-0 (inventory + `db-import-boundary` rule) and B3-1 (auth characterization) merged 2026-08-29; B3-2 (auth vertical slice) merged 2026-08-30; B3-9 (five of the six B3-tagged findings; OC-0323 rides B3-8) merged 2026-08-30; HP-3 accepted 2026-08-30; B3-3 (lifecycle extraction into `Server/internal/app/`) merged 2026-08-30 (#1464); B3-4 (hub constructor options) merged 2026-08-31 (#1470); B3-7 (deterministic alpha dataset) merged 2026-08-31 (#1469); B3-5 (`ws` split) in progress — split PR 1 (handshake auth + fresh-connect moves) opened 2026-08-31. | +| [hp-3-scorecard-2026-08-29](hp-3-scorecard-2026-08-29.md) | **HP-3 accepted 2026-08-30 by the owner.** Five questions on the auth vertical slice answered with commands: frozen set green at every pre-squash SHA, `api` db importers 12 → 10, B2 contracts unchanged, the pattern written as D4 in server.md, guardrails as they exist. | +| [b3-bench-baseline-2026-08-30](b3-bench-baseline-2026-08-30.md) | **Recorded 2026-08-30, not gated.** The six B3-6 `Benchmark*` through `benchstat` at `ec8ef24a`, produced by `make bench-baseline`. Nothing in CI reads these numbers; the performance gate is B6's. Regenerating writes a new dated file — replace this row and delete the superseded document, so only the newest baseline is kept. | +| [audit-2026-08-19-remediation](audit-2026-08-19-remediation.md) | Phases 1–6 done 2026-08-20; **phase 7 pending**. Its header still reads "in progress 2026-08-19" — stale; the phase table is correct. | ## Partially implemented diff --git a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md index 5dcae64c..65425536 100644 --- a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md +++ b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md @@ -800,6 +800,36 @@ Exit: the file table in `docs/architecture/server-boundaries.md` §hub updated; race + deadlock + `TestEpoch1Fixtures` green on every commit. One PR per two responsibilities at most, so each is reviewable. +**Evidence — split PR 1 of the series, 2026-08-31** — branch +`feat/b3-5-ws-split` from `dev` `e13adaf8`. Responsibilities 1 and 2 +(handshake authentication, fresh-connect initialisation); both destination +files already existed, so `git diff -M --summary` records no file-level +rename and the purity proof is the normalised line-diff per move: + +- **Move 1** — `handshakeWrite`, `upgradeAndAuth`, + `unregisterFailedHandshake` (99 lines) → `serve_auth.go`, beside + `authenticateConn`. Residue: three import lines (`log/slog`, `net/http`, + `strings`) added to the destination; `serve.go` keeps its copies for the + remaining code. Gate: `-tags deadlock -count=10 ./ws/` ok 648.7 s, + `-race` ok 228.8 s. +- **Move 2** — `handleFreshConnect`, `freshConnectCleanStaleVoice` + (187 lines) → `serve_ready.go`, beside the `buildReady`/`buildAuthOK` + family they drive. Residue: one import line (`github.com/coder/websocket`). + Gate: `-tags deadlock -count=10 ./ws/` ok 649.8 s, `-race` ok 233.6 s. +- **Deliberately not moved**: `applyConnectStatus`, + `announceConnectPresence`, `refreshUserSnapshot` are called from both the + reconnect and fresh-connect paths, so they stay in `serve.go` with the + composition point; `computeAllowedChannels` (also called from + `hub_broadcast.go`) waits for the visibility responsibility. +- **Sizes**: `serve.go` 990 → 704, `serve_auth.go` 105 → 207, + `serve_ready.go` 375 → 564. The `serve.go` < 500 exit figure lands when + the replay/reconnect family moves (next PR). +- Boundaries doc re-measured (dbinventory table, two `DBImportAllow` + reasons, the reader note: 45 → 48 references, 7 → 11 distinct queries). +- Pre-squash SHAs recorded at merge time in the PR; the mechanical-rewrite + half of each pair was empty (no identifier changed), so each move is one + commit. + ## B3-6 — Permanent guardrails Roadmap workstreams 1, 2, 3, 10, 11, 13, 14, 15, 16. Runs beside B3-0..B3-2;