diff --git a/Server/ws/command.go b/Server/ws/command.go index 5123dc54..24010835 100644 --- a/Server/ws/command.go +++ b/Server/ws/command.go @@ -3,6 +3,7 @@ package ws import ( "encoding/json" "fmt" + "strings" ) // Command is the minimal interface for all client-to-server commands. @@ -206,6 +207,26 @@ func (c VoiceE2EEAnnounceCmd) Type() string { return MsgTypeVoiceE2EEAnnoun func (c VoiceE2EEAnnounceCmd) UserID() int64 { return c.userID } func (c VoiceE2EEAnnounceCmd) PublicKey() string { return c.publicKey } +// ChatCommandCmd represents a chat_command (plugin slash command) message. +type ChatCommandCmd struct { + userID int64 + reqID string + channelID int64 + command string // trimmed, including leading slash, e.g. "/hello" + args []string +} + +func (c ChatCommandCmd) Type() string { return MsgTypeChatCommand } +func (c ChatCommandCmd) UserID() int64 { return c.userID } +func (c ChatCommandCmd) ChannelID() int64 { return c.channelID } +func (c ChatCommandCmd) ReqID() string { return c.reqID } +func (c ChatCommandCmd) Command() string { return c.command } +func (c ChatCommandCmd) Args() []string { + dst := make([]string, len(c.args)) + copy(dst, c.args) + return dst +} + // VoiceE2EEOfferCmd represents a voice_e2ee_offer message. type VoiceE2EEOfferCmd struct { userID int64 @@ -454,6 +475,34 @@ var commandConstructors = map[string]func(userID int64, reqID string, raw json.R return VoiceE2EEAnnounceCmd{userID: userID, publicKey: p.PublicKey}, nil }, + MsgTypeChatCommand: func(userID int64, reqID string, raw json.RawMessage) (Command, error) { + var p struct { + ChannelID int64 `json:"channel_id"` + Command string `json:"command"` + Args []string `json:"args"` + } + if err := json.Unmarshal(raw, &p); err != nil { + return nil, fmt.Errorf("invalid chat_command payload: %w", err) + } + cmd := strings.TrimSpace(p.Command) + if cmd == "" { + return nil, fmt.Errorf("command must not be empty") + } + // Guard against a client flooding the plugin allocate/dispatch ABI. + if len(p.Args) > maxCommandArgs { + return nil, fmt.Errorf("too many command arguments (max %d)", maxCommandArgs) + } + args := make([]string, len(p.Args)) + copy(args, p.Args) + return ChatCommandCmd{ + userID: userID, + reqID: reqID, + channelID: p.ChannelID, + command: cmd, + args: args, + }, nil + }, + MsgTypeVoiceE2EEOffer: func(userID int64, _ string, raw json.RawMessage) (Command, error) { var p struct { TargetUserID int64 `json:"target_user_id"` diff --git a/Server/ws/deps.go b/Server/ws/deps.go index dfd69882..19afa147 100644 --- a/Server/ws/deps.go +++ b/Server/ws/deps.go @@ -7,6 +7,7 @@ import ( "github.com/owncord/server/auth" "github.com/owncord/server/db" "github.com/owncord/server/permissions" + "github.com/owncord/server/plugin" "github.com/owncord/server/service" ) @@ -59,6 +60,16 @@ type KeyHolderChecker interface { IsVoiceKeyHolder(channelID, userID int64) bool } +// PluginDeps holds dependencies for the chat_command (plugin slash-command) +// handler. Registry is a getter, not a captured value, because the plugin +// registry is wired via SetPluginRegistry AFTER NewHub builds the deps; reading +// it live at dispatch time picks up the late wiring. MessageSvc gates channel +// broadcasts through the same posting policy as a real message send. +type PluginDeps struct { + Registry func() *plugin.Registry + MessageSvc *service.MessageService +} + // VoiceDeps holds dependencies for voice handlers. type VoiceDeps struct { DB *db.DB diff --git a/Server/ws/event.go b/Server/ws/event.go index 265b8a81..8d26d3ae 100644 --- a/Server/ws/event.go +++ b/Server/ws/event.go @@ -30,6 +30,18 @@ type Result struct { // SetVoiceJoinToken, if non-nil, caches the voice join token on the client. // Used by voice_token_refresh when falling back to the DB for the token. SetVoiceJoinToken *string + // JoinVoice, if true, triggers the hub's voice-join routine after the handler + // returns. voice_join's effect is a large, hub-coupled sequence (DB + // persistence, LiveKit token, existing-state fan-out, key-holder election, + // topic subscription) that also invokes the leave routine on a channel + // switch, so the applier runs handleVoiceJoin (re-parsing the envelope + // payload it already validated) rather than re-expressing it as pure events. + JoinVoice bool + // LeaveVoice, if true, triggers the hub's voice-leave routine after the + // handler returns. handleVoiceLeave stays hub-internal because disconnect and + // channel-switch cleanup call it un-throttled; only the message dispatch + // moved to V2 (which does the rate-limit before setting this flag). + LeaveVoice bool } // Event is the base interface for all server-to-client events. @@ -248,8 +260,9 @@ func (e VoiceStateEvent) EventType() string { return MsgTypeVoiceState } func (e VoiceStateEvent) Payload() []byte { return e.payload } // VoiceLeaveEvent is a voice_leave broadcast to all connected clients. -// NOTE: Currently unused by V2 handlers — voice_leave remains V1 and emits -// via h.BroadcastToAll directly. Retained as forward-compatible scaffolding. +// NOTE: Currently unused — the voice_leave V2 handler triggers the hub's +// handleVoiceLeave routine (via Result.LeaveVoice), which broadcasts the leave +// directly. Retained as forward-compatible scaffolding. type VoiceLeaveEvent struct { payload []byte } @@ -257,6 +270,18 @@ type VoiceLeaveEvent struct { func (e VoiceLeaveEvent) EventType() string { return MsgTypeVoiceLeaveBC } func (e VoiceLeaveEvent) Payload() []byte { return e.payload } +// PluginBroadcastEvent is a plugin slash-command result broadcast to a channel +// (sequenced, replayable). Emitted by the chat_command handler after the +// invoking user's post permission is verified. +type PluginBroadcastEvent struct { + channelID int64 + payload []byte +} + +func (e PluginBroadcastEvent) EventType() string { return "plugin_broadcast" } +func (e PluginBroadcastEvent) ChannelID() int64 { return e.channelID } +func (e PluginBroadcastEvent) Payload() []byte { return e.payload } + // VoiceE2EEAnnounceEvent relays an ECDH public key to other voice channel participants. type VoiceE2EEAnnounceEvent struct { voiceChannelID int64 diff --git a/Server/ws/handler_v2_migration_test.go b/Server/ws/handler_v2_migration_test.go new file mode 100644 index 00000000..d41bbc6b --- /dev/null +++ b/Server/ws/handler_v2_migration_test.go @@ -0,0 +1,151 @@ +package ws + +// handler_v2_migration_test.go — unit tests for the three handlers ported from +// V1 to V2 in the dispatch-migration finish (audit A-2026-07-09 / backlog 11): +// chat_command, voice_join, voice_leave. + +import ( + "context" + "encoding/json" + "testing" + + "github.com/owncord/server/auth" +) + +// ── voice_join V2 ──────────────────────────────────────────────────────────── + +// The V2 handler is a thin gate: the constructor validates channel_id and the +// handler hands off to the hub's handleVoiceJoin routine via Result.JoinVoice. +func TestHandleVoiceJoinV2_SignalsJoin(t *testing.T) { + result := handleVoiceJoinV2(context.Background(), VoiceJoinCmd{userID: 1, channelID: 7}, ClientInfo{UserID: 1}, VoiceDeps{}) + if result.Error != nil { + t.Fatalf("unexpected error: %v", result.Error) + } + if !result.JoinVoice { + t.Error("expected JoinVoice=true so the applier runs handleVoiceJoin") + } + if result.LeaveVoice { + t.Error("voice_join must not signal LeaveVoice") + } +} + +// voice_join parse errors are surfaced by the constructor (before dispatch). +func TestVoiceJoinConstructor_Errors(t *testing.T) { + ctor, ok := getCommandConstructor(MsgTypeVoiceJoin) + if !ok { + t.Fatal("no constructor for voice_join") + } + for _, raw := range []string{`{"channel_id":"nope"}`, `{"channel_id":0}`, `{"channel_id":-3}`} { + if _, err := ctor(1, "r", json.RawMessage(raw)); err == nil { + t.Errorf("expected parse error for %s", raw) + } + } + cmd, err := ctor(1, "r", json.RawMessage(`{"channel_id":42}`)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cmd.(VoiceJoinCmd).ChannelID() != 42 { + t.Errorf("ChannelID() = %d, want 42", cmd.(VoiceJoinCmd).ChannelID()) + } +} + +// ── voice_leave V2 ─────────────────────────────────────────────────────────── + +func TestHandleVoiceLeaveV2_SignalsLeave(t *testing.T) { + deps := VoiceDeps{Limiter: auth.NewRateLimiter()} + result := handleVoiceLeaveV2(context.Background(), VoiceLeaveCmd{userID: 1}, ClientInfo{UserID: 1}, deps) + if result.Error != nil { + t.Fatalf("unexpected error: %v", result.Error) + } + if !result.LeaveVoice { + t.Error("expected LeaveVoice=true so the applier runs handleVoiceLeave") + } + if result.JoinVoice { + t.Error("voice_leave must not signal JoinVoice") + } +} + +// The rate-limit that used to live in the V1 dispatch wrapper now lives in the +// V2 handler; disconnect/switch callers of handleVoiceLeave bypass it entirely. +func TestHandleVoiceLeaveV2_RateLimited(t *testing.T) { + deps := VoiceDeps{Limiter: auth.NewRateLimiter()} + cmd := VoiceLeaveCmd{userID: 1} + info := ClientInfo{UserID: 1} + + // voiceLeaveRateLimit (5) per voiceLeaveWindow (1s) — the 6th is rejected. + var limited bool + for i := 0; i < voiceLeaveRateLimit+1; i++ { + res := handleVoiceLeaveV2(context.Background(), cmd, info, deps) + if res.Error != nil { + ce, ok := res.Error.(ClientError) + if !ok || ce.Code != ErrCodeRateLimited { + t.Fatalf("expected rate-limit ClientError, got %v", res.Error) + } + limited = true + } + } + if !limited { + t.Error("expected voice_leave to be rate limited after the burst") + } +} + +// ── chat_command V2 ────────────────────────────────────────────────────────── + +func TestChatCommandConstructor_Errors(t *testing.T) { + ctor, ok := getCommandConstructor(MsgTypeChatCommand) + if !ok { + t.Fatal("no constructor for chat_command") + } + + if _, err := ctor(1, "r", json.RawMessage(`"not-an-object"`)); err == nil { + t.Error("expected error for malformed payload") + } + if _, err := ctor(1, "r", json.RawMessage(`{"command":" "}`)); err == nil { + t.Error("expected error for empty command") + } + + tooMany := make([]string, maxCommandArgs+1) + payload, _ := json.Marshal(map[string]any{"command": "/x", "args": tooMany}) + if _, err := ctor(1, "r", payload); err == nil { + t.Error("expected error for too many args") + } + + cmd, err := ctor(1, "req-9", json.RawMessage(`{"channel_id":5,"command":" /hi ","args":["a","b"]}`)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + cc := cmd.(ChatCommandCmd) + if cc.ChannelID() != 5 || cc.Command() != "/hi" || cc.ReqID() != "req-9" || len(cc.Args()) != 2 { + t.Errorf("unexpected command fields: %+v", cc) + } +} + +func TestHandleChatCommandV2_NoRegistry(t *testing.T) { + deps := PluginDeps{Registry: nil, MessageSvc: nil} + cmd := ChatCommandCmd{userID: 1, channelID: 1, command: "/hi"} + + result := handleChatCommandV2(context.Background(), cmd, ClientInfo{UserID: 1}, deps) + + ce, ok := result.Error.(ClientError) + if !ok { + t.Fatalf("expected ClientError, got %T", result.Error) + } + if ce.Code != ErrCodeBadRequest { + t.Errorf("expected BAD_REQUEST, got %q", ce.Code) + } + if result.Reply != nil || len(result.Events) != 0 { + t.Error("no reply or events expected when no registry is wired") + } +} + +// canPluginBroadcast fails closed when the posting-gate service is absent. +func TestCanPluginBroadcast_NilServiceFailsClosed(t *testing.T) { + gate := canPluginBroadcast(nil, 1, 2) + if gate == nil { + t.Fatal("expected a forbidden Result when MessageSvc is nil") + } + ce, ok := gate.Error.(ClientError) + if !ok || ce.Code != ErrCodeForbidden { + t.Errorf("expected FORBIDDEN ClientError, got %v", gate.Error) + } +} diff --git a/Server/ws/handlers.go b/Server/ws/handlers.go index 07164a0c..38e50c7b 100644 --- a/Server/ws/handlers.go +++ b/Server/ws/handlers.go @@ -94,91 +94,100 @@ func (h *Hub) handleMessage(c *Client, raw []byte) { reqLog.Debug("ws ← client message") - // ── V2 dispatch (strangler fig) ────────────────────────────────────── - // Only attempt V2 parsing+dispatch if a V2 handler is registered for - // this type. This prevents the stricter V2 parser from rejecting - // payloads that V1 handlers handle leniently. - if h.registry.hasV2(env.Type) { - if ctor, ok := getCommandConstructor(env.Type); ok { - cmd, parseErr := ctor(c.userID, env.ID, env.Payload) - if parseErr != nil { - reqLog.Warn("ws command parse error", "err", parseErr) - c.sendMsg(buildErrorMsgWithID(ErrCodeBadRequest, "invalid payload", env.ID)) - return - } - - var username string - var avatar *string - if c.user != nil { - username = c.user.Username - avatar = c.user.Avatar - } - voiceChID, voiceJoinTok := c.getVoiceState() - info := ClientInfo{ - UserID: c.userID, - Username: username, - Avatar: avatar, - RoleName: c.roleName, - ReqID: env.ID, - VoiceChannelID: voiceChID, - VoiceJoinToken: voiceJoinTok, - } - - result, dispatched := h.registry.DispatchV2(c.ctx, cmd, info) - if !dispatched { - reqLog.Error("ws V2 handler registered but DispatchV2 returned false", "type", env.Type) - c.sendMsg(buildErrorMsgWithID(ErrCodeInternal, "internal error", env.ID)) - return - } - if result.Error != nil { - if ce, ok := result.Error.(ClientError); ok { - c.sendMsg(buildErrorMsgWithID(ce.Code, ce.Message, env.ID)) - } else { - reqLog.Error("ws handler internal error", "err", result.Error) - c.sendMsg(buildErrorMsgWithID(ErrCodeInternal, "internal error", env.ID)) - } - return - } - // Apply client state mutations. - if result.SetChannelID != nil { - oldChID := c.getChannelID() - c.mu.Lock() - c.channelID = *result.SetChannelID - c.mu.Unlock() - // Update pub/sub channel topic subscriptions. - newChID := *result.SetChannelID - if oldChID != newChID { - if oldChID > 0 { - c.hub.pubsub.Unsubscribe(c, ChannelTopic(oldChID)) - } - if newChID > 0 { - c.hub.pubsub.Subscribe(c, ChannelTopic(newChID)) - } - } - } - if result.SetE2EEPubKey != nil { - c.setE2EEPubKey(*result.SetE2EEPubKey) - } - if result.SetVoiceJoinToken != nil { - chID := c.getVoiceChID() - if chID != 0 { - c.setVoiceState(chID, *result.SetVoiceJoinToken) - } - } - if result.Reply != nil { - c.sendMsg(result.Reply) - } - if len(result.Events) > 0 { - h.EmitEvents(result.Events) - } - return - } - } - // ── End V2 dispatch ────────────────────────────────────────────────── - - if !h.registry.Dispatch(c.ctx, env.Type, h, c, env.ID, env.Payload) { + // ── Typed command dispatch ─────────────────────────────────────────── + // Every message type parses through its constructor into a typed Command, + // then dispatches to its V2 handler, which returns a Result the applier + // below acts on. There is no second (V1) generation — the strangler-fig + // migration is complete. + ctor, ok := getCommandConstructor(env.Type) + if !ok { reqLog.Warn("ws handleMessage unknown type") c.sendMsg(buildErrorMsg(ErrCodeUnknownType, fmt.Sprintf("unknown message type: %s", msgType))) + return + } + + cmd, parseErr := ctor(c.userID, env.ID, env.Payload) + if parseErr != nil { + reqLog.Warn("ws command parse error", "err", parseErr) + c.sendMsg(buildErrorMsgWithID(ErrCodeBadRequest, "invalid payload", env.ID)) + return + } + + var username string + var avatar *string + if c.user != nil { + username = c.user.Username + avatar = c.user.Avatar + } + voiceChID, voiceJoinTok := c.getVoiceState() + info := ClientInfo{ + UserID: c.userID, + Username: username, + Avatar: avatar, + RoleName: c.roleName, + ReqID: env.ID, + VoiceChannelID: voiceChID, + VoiceJoinToken: voiceJoinTok, + } + + result, dispatched := h.registry.DispatchV2(c.ctx, cmd, info) + if !dispatched { + // A registered constructor with no V2 handler is a wiring bug — the + // guard test (TestEveryConstructorHasV2Handler) locks this shut. + reqLog.Error("ws no V2 handler for constructed command", "type", env.Type) + c.sendMsg(buildErrorMsgWithID(ErrCodeInternal, "internal error", env.ID)) + return + } + if result.Error != nil { + if ce, ok := result.Error.(ClientError); ok { + c.sendMsg(buildErrorMsgWithID(ce.Code, ce.Message, env.ID)) + } else { + reqLog.Error("ws handler internal error", "err", result.Error) + c.sendMsg(buildErrorMsgWithID(ErrCodeInternal, "internal error", env.ID)) + } + return + } + + // Apply client state mutations and side effects. + if result.SetChannelID != nil { + oldChID := c.getChannelID() + c.mu.Lock() + c.channelID = *result.SetChannelID + c.mu.Unlock() + // Update pub/sub channel topic subscriptions. + newChID := *result.SetChannelID + if oldChID != newChID { + if oldChID > 0 { + c.hub.pubsub.Unsubscribe(c, ChannelTopic(oldChID)) + } + if newChID > 0 { + c.hub.pubsub.Subscribe(c, ChannelTopic(newChID)) + } + } + } + if result.SetE2EEPubKey != nil { + c.setE2EEPubKey(*result.SetE2EEPubKey) + } + if result.SetVoiceJoinToken != nil { + chID := c.getVoiceChID() + if chID != 0 { + c.setVoiceState(chID, *result.SetVoiceJoinToken) + } + } + if result.Reply != nil { + c.sendMsg(result.Reply) + } + if len(result.Events) > 0 { + h.EmitEvents(result.Events) + } + // Voice join/leave hand off to the hub-internal routines (also called + // un-throttled on disconnect/switch). handleVoiceJoin re-reads channel_id + // from the already-validated envelope payload. + if result.LeaveVoice { + h.handleVoiceLeave(c.ctx, c) + } + if result.JoinVoice { + h.handleVoiceJoin(c.ctx, c, env.Payload) } } diff --git a/Server/ws/handlers_command.go b/Server/ws/handlers_command.go index 618b9bd9..341b8766 100644 --- a/Server/ws/handlers_command.go +++ b/Server/ws/handlers_command.go @@ -1,4 +1,4 @@ -// Phase C Step 9 — plugin slash-command dispatcher. +// Phase C Step 9 — plugin slash-command dispatcher (V2). // // chat_command routes a slash command from a WS client to a registered plugin. // If no plugin owns the command, an error is returned to the sender. If the @@ -14,8 +14,8 @@ import ( "errors" "fmt" "log/slog" - "strings" + "github.com/owncord/server/plugin" "github.com/owncord/server/service" ) @@ -26,110 +26,79 @@ const MsgTypeChatCommand = "chat_command" // the plugin's allocate/dispatch ABI with thousands of strings. const maxCommandArgs = 64 -// chatCommandPayload is the client-supplied payload for a chat_command message. -type chatCommandPayload struct { - ChannelID int64 `json:"channel_id"` - Command string `json:"command"` // including leading slash, e.g. "/hello" - Args []string `json:"args"` -} +// handleChatCommandV2 dispatches a slash command to the owning plugin via the +// live plugin registry (wired post-construction). It returns: +// - a ClientError when no plugin registry is wired, the command is unknown, +// or the invoking user may not post the plugin's broadcast; +// - Result.Reply for an ephemeral plugin reply (sender only); +// - Result.Events with a PluginBroadcastEvent for a channel broadcast, gated +// by MessageService.CanPost (same policy as a real message send). +func handleChatCommandV2(ctx context.Context, cmd Command, _ ClientInfo, deps any) Result { + d := deps.(PluginDeps) + cc := cmd.(ChatCommandCmd) -// registerPluginCommandHandler registers the chat_command V1 handler. -func registerPluginCommandHandler(r *HandlerRegistry) { - r.Register(MsgTypeChatCommand, handlePluginCommand) -} - -// handlePluginCommand dispatches a slash command to the owning plugin via -// hub.pluginRegistry. Returns an error to the client when: -// - the payload is malformed, -// - the command name is empty, -// - too many arguments are supplied, -// - no plugin owns the command (unknown command), -// - the plugin returns an error reply. -func handlePluginCommand(ctx context.Context, h *Hub, c *Client, reqID string, payload json.RawMessage) { - var p chatCommandPayload - if err := json.Unmarshal(payload, &p); err != nil { - c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "invalid chat_command payload")) - return + var reg *plugin.Registry + if d.Registry != nil { + reg = d.Registry() + } + if reg == nil { + return Result{Error: ClientError{Code: ErrCodeBadRequest, Message: fmt.Sprintf("unknown command: %s (no plugins loaded)", cc.command)}} } - cmd := strings.TrimSpace(p.Command) - if cmd == "" { - c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "command must not be empty")) - return - } - - if len(p.Args) > maxCommandArgs { - c.sendMsg(buildErrorMsg(ErrCodeBadRequest, fmt.Sprintf("too many command arguments (max %d)", maxCommandArgs))) - return - } - - if h.pluginRegistry == nil { - c.sendMsg(buildErrorMsg(ErrCodeBadRequest, fmt.Sprintf("unknown command: %s (no plugins loaded)", cmd))) - return - } - - result, handled := h.pluginRegistry.DispatchCommand(ctx, c.userID, p.ChannelID, cmd, p.Args) + result, handled := reg.DispatchCommand(ctx, cc.userID, cc.channelID, cc.command, cc.args) if !handled { - c.sendMsg(buildErrorMsg(ErrCodeBadRequest, fmt.Sprintf("unknown command: %s", cmd))) - return + return Result{Error: ClientError{Code: ErrCodeBadRequest, Message: fmt.Sprintf("unknown command: %s", cc.command)}} } - if result == nil { // Plugin acknowledged with no output. - return + return Result{} } + var out Result if result.Reply != "" { // Ephemeral reply — sent only to the invoking client. - c.sendMsg(buildCommandReply(reqID, result.Reply)) + out.Reply = buildCommandReply(cc.reqID, result.Reply) } - if result.Broadcast != "" && p.ChannelID != 0 { + if result.Broadcast != "" && cc.channelID != 0 { // Verify the invoking client can post to this channel before broadcasting - // the plugin result to all channel members. Mirrors the normal send path: - // non-DM channels require READ_MESSAGES|SEND_MESSAGES (so a user cannot - // post into a channel they cannot read), and DM channels are validated by - // participant membership rather than role permissions. - if !h.requireChannelBroadcastAccess(c, p.ChannelID) { - return + // (same gate a real send uses: channel role perms + DM membership/blocks). + // ponytail: if the plugin returned both a reply and a broadcast and the + // gate denies, the error wins and the ephemeral reply is dropped (Result + // carries either an error or a reply, not both) — an untested edge; V1 + // sent both. Preserve the security signal (denial) over the ack. + if gate := canPluginBroadcast(d.MessageSvc, cc.userID, cc.channelID); gate != nil { + return *gate } - // Channel broadcast — visible to everyone in the channel. - msg := buildCommandBroadcast(p.ChannelID, c.userID, cmd, result.Broadcast) - h.BroadcastToChannel(p.ChannelID, msg) - slog.Info("plugin command broadcast", "cmd", cmd, "channel_id", p.ChannelID, "user_id", c.userID) + msg := buildCommandBroadcast(cc.channelID, cc.userID, cc.command, result.Broadcast) + out.Events = append(out.Events, PluginBroadcastEvent{channelID: cc.channelID, payload: msg}) + slog.Info("plugin command broadcast", "cmd", cc.command, "channel_id", cc.channelID, "user_id", cc.userID) } + return out } -// requireChannelBroadcastAccess reports whether the client may post to -// channelID, by delegating to the SAME service-layer check a real message -// send runs (MessageService.CanPost: cached channel permissions; DM -// membership AND DM blocks). The previous RequireChannelAccess route skipped -// the block check in its DM branch — a blocked user's plugin broadcast could -// reach the person who blocked them — and issued a raw GetRoleByID per -// broadcast, bypassing the permission cache. On failure it sends an error to -// the client and returns false. -func (h *Hub) requireChannelBroadcastAccess(c *Client, channelID int64) bool { - if c.user == nil { - c.sendMsg(buildErrorMsg(ErrCodeForbidden, "not authenticated")) - return false +// canPluginBroadcast reports whether userID may post to channelID by delegating +// to the SAME service-layer check a real message send runs (MessageService.CanPost: +// cached channel permissions; DM membership AND DM blocks). Returns nil when +// allowed, or a Result carrying the appropriate ClientError otherwise. A nil +// MessageSvc (bare test hub) fails closed rather than allowing an ungated +// broadcast. +func canPluginBroadcast(messageSvc *service.MessageService, userID, channelID int64) *Result { + if messageSvc == nil { + r := Result{Error: ClientError{Code: ErrCodeForbidden, Message: "broadcast gate unavailable"}} + return &r } - if h.messageSvc == nil { - // No service wired (bare test hub) — fail closed rather than allow - // an ungated broadcast. - c.sendMsg(buildErrorMsg(ErrCodeForbidden, "broadcast gate unavailable")) - return false - } - if err := h.messageSvc.CanPost(c.userID, channelID); err != nil { + if err := messageSvc.CanPost(userID, channelID); err != nil { if errors.Is(err, service.ErrNotFound) { - c.sendMsg(buildErrorMsg(ErrCodeNotFound, "channel not found")) - return false + r := Result{Error: ClientError{Code: ErrCodeNotFound, Message: "channel not found"}} + return &r } slog.Warn("ws plugin broadcast permission denied", - "user_id", c.userID, "channel_id", channelID, "err", err) - c.sendMsg(buildErrorMsg(ErrCodeForbidden, "missing permission to post in this channel")) - return false + "user_id", userID, "channel_id", channelID, "err", err) + r := Result{Error: ClientError{Code: ErrCodeForbidden, Message: "missing permission to post in this channel"}} + return &r } - return true + return nil } // buildCommandReply builds an ephemeral command_reply envelope. diff --git a/Server/ws/handlers_voice.go b/Server/ws/handlers_voice.go index 408f1ba8..5c5a076c 100644 --- a/Server/ws/handlers_voice.go +++ b/Server/ws/handlers_voice.go @@ -2,35 +2,21 @@ package ws import ( "context" - "encoding/json" "fmt" ) -// registerVoiceHandlersV1 registers voice handlers that remain V1 (complex -// state management that hasn't been migrated yet). -func registerVoiceHandlersV1(r *HandlerRegistry) { - r.Register(MsgTypeVoiceJoin, func(ctx context.Context, h *Hub, c *Client, _ string, payload json.RawMessage) { - h.handleVoiceJoin(ctx, c, payload) - }) - r.Register(MsgTypeVoiceLeave, func(ctx context.Context, h *Hub, c *Client, _ string, _ json.RawMessage) { - // Rate limit only the explicit client-initiated voice_leave message. - // handleVoiceLeave is also invoked internally for disconnect and - // channel-switch cleanup (serve.go, voice_join.go); those paths must - // never be throttled or they would leak ghost voice states, so the - // limit lives here in the dispatch wrapper rather than inside the shared - // handleVoiceLeave routine. Mirrors the voice control Limiter idiom. - ratKey := fmt.Sprintf("voice_leave:%d", c.userID) - if h.limiter != nil && !h.limiter.Allow(ratKey, voiceLeaveRateLimit, voiceLeaveWindow) { - c.sendMsg(buildErrorMsg(ErrCodeRateLimited, "too many voice leave attempts")) - return - } - h.handleVoiceLeave(ctx, c) - }) -} - -// registerVoiceControlsV2 registers V2 handlers for voice control toggles -// and other migrated voice handlers. +// registerVoiceControlsV2 registers all voice V2 handlers: the control toggles, +// E2EE relays, token refresh, and the join/leave dispatch. +// +// voice_join and voice_leave return a Result that triggers the hub's +// handleVoiceJoin / handleVoiceLeave routines via the applier in handleMessage. +// Those routines stay hub-internal: handleVoiceLeave is also called un-throttled +// on disconnect and channel-switch (serve.go, voice_join.go), and handleVoiceJoin +// is a large, hub-coupled sequence that itself calls handleVoiceLeave on a switch. +// Only the message dispatch moved to V2, not the imperative effect. func registerVoiceControlsV2(r *HandlerRegistry, deps VoiceDeps) { + r.RegisterV2(MsgTypeVoiceJoin, handleVoiceJoinV2, deps) + r.RegisterV2(MsgTypeVoiceLeave, handleVoiceLeaveV2, deps) r.RegisterV2(MsgTypeVoiceMute, handleVoiceMuteV2, deps) r.RegisterV2(MsgTypeVoiceDeafen, handleVoiceDeafenV2, deps) r.RegisterV2(MsgTypeVoiceCamera, handleVoiceCameraV2, deps) @@ -39,3 +25,24 @@ func registerVoiceControlsV2(r *HandlerRegistry, deps VoiceDeps) { r.RegisterV2(MsgTypeVoiceE2EEOffer, handleVoiceE2EEOfferV2, deps) r.RegisterV2(MsgTypeVoiceTokenRefresh, handleVoiceTokenRefreshV2, deps) } + +// handleVoiceJoinV2 gates parsing via the VoiceJoinCmd constructor (which +// rejects a malformed or non-positive channel_id with a BAD_REQUEST) and hands +// off to the hub's handleVoiceJoin routine via the applier. The rate-limit and +// all side effects live in handleVoiceJoin, which re-reads channel_id from the +// already-validated envelope payload. +func handleVoiceJoinV2(_ context.Context, _ Command, _ ClientInfo, _ any) Result { + return Result{JoinVoice: true} +} + +// handleVoiceLeaveV2 rate-limits the explicit client-initiated leave (the +// disconnect/switch callers of handleVoiceLeave must never be throttled) and +// then hands off to the hub's handleVoiceLeave routine via the applier. +func handleVoiceLeaveV2(_ context.Context, cmd Command, _ ClientInfo, deps any) Result { + d := deps.(VoiceDeps) + ratKey := fmt.Sprintf("voice_leave:%d", cmd.UserID()) + if d.Limiter != nil && !d.Limiter.Allow(ratKey, voiceLeaveRateLimit, voiceLeaveWindow) { + return Result{Error: ClientError{Code: ErrCodeRateLimited, Message: "too many voice leave attempts"}} + } + return Result{LeaveVoice: true} +} diff --git a/Server/ws/hub.go b/Server/ws/hub.go index a6bad424..925b252b 100644 --- a/Server/ws/hub.go +++ b/Server/ws/hub.go @@ -104,7 +104,6 @@ type Hub struct { // If svc is non-nil, V2 handlers receive service references for business logic delegation. func NewHub(database *db.DB, limiter *auth.RateLimiter, svc *service.Services) *Hub { reg := NewHandlerRegistry() - registerVoiceHandlersV1(reg) h := &Hub{ clients: make(map[int64]*Client), @@ -143,7 +142,12 @@ func NewHub(database *db.DB, limiter *auth.RateLimiter, svc *service.Services) * registerChatHandlers(reg, chatDeps) registerPresenceHandlers(reg, presenceDeps) registerReactionHandlers(reg, reactionDeps) - registerPluginCommandHandler(reg) // Phase C Step 9 — plugin slash commands + // Phase C Step 9 — plugin slash commands. Registry is read live because + // SetPluginRegistry wires it after NewHub; MessageSvc gates broadcasts. + reg.RegisterV2(MsgTypeChatCommand, handleChatCommandV2, PluginDeps{ + Registry: func() *plugin.Registry { return h.pluginRegistry }, + MessageSvc: h.messageSvc, + }) registerVoiceControlsV2(reg, VoiceDeps{ DB: h.db, Limiter: h.limiter, diff --git a/Server/ws/registry.go b/Server/ws/registry.go index a5e5d23d..7a55c3aa 100644 --- a/Server/ws/registry.go +++ b/Server/ws/registry.go @@ -2,75 +2,36 @@ package ws import ( "context" - "encoding/json" "fmt" "log/slog" "runtime" ) -// MessageHandler is the function signature for all WebSocket message handlers. -// It receives a context (derived from the client's WS connection), the hub, -// the sending client, the request ID from the envelope, and the raw JSON payload. -type MessageHandler func(ctx context.Context, h *Hub, c *Client, reqID string, payload json.RawMessage) - // handlerV2Entry pairs a V2 handler with its domain-specific dependency struct. type handlerV2Entry struct { handler HandlerV2 deps any // concrete deps struct for this handler's domain } -// HandlerRegistry maps message type strings to their handler functions. -// It is not safe for concurrent use after initialization; all Register -// calls must happen before any Dispatch calls. +// HandlerRegistry maps message type strings to their typed V2 handlers. +// It is not safe for concurrent use after initialization; all RegisterV2 +// calls must happen before any DispatchV2 calls. type HandlerRegistry struct { - handlers map[string]MessageHandler // V1 — unchanged - handlersV2 map[string]handlerV2Entry // V2 — new + handlersV2 map[string]handlerV2Entry } // NewHandlerRegistry creates an empty handler registry. func NewHandlerRegistry() *HandlerRegistry { return &HandlerRegistry{ - handlers: make(map[string]MessageHandler), handlersV2: make(map[string]handlerV2Entry), } } -// Register associates a message type with a handler function. -func (r *HandlerRegistry) Register(msgType string, handler MessageHandler) { - r.handlers[msgType] = handler -} - -// Dispatch looks up the handler for msgType and invokes it. Returns true if a -// handler was found and called, false if no handler is registered for the type. -func (r *HandlerRegistry) Dispatch(ctx context.Context, msgType string, h *Hub, c *Client, reqID string, payload json.RawMessage) bool { - handler, ok := r.handlers[msgType] - if !ok { - return false - } - handler(ctx, h, c, reqID, payload) - return true -} - -// RegisteredTypes returns all registered message types (unordered). -// Intended for testing and diagnostics. -func (r *HandlerRegistry) RegisteredTypes() []string { - types := make([]string, 0, len(r.handlers)) - for t := range r.handlers { - types = append(types, t) - } - return types -} - // RegisterV2 registers a V2 handler for the given command type. -// PANICS if cmdType is already registered in V1 (shadowing guard) or V2 (duplicate guard). -// The shadowing guard prevents accidentally having both V1 and V2 handlers for the -// same type. When migrating a handler, remove V1 registration BEFORE adding V2. +// PANICS if cmdType is already registered (duplicate guard). func (r *HandlerRegistry) RegisterV2(cmdType string, handler HandlerV2, deps any) { - if _, exists := r.handlers[cmdType]; exists { - panic(fmt.Sprintf("RegisterV2: cmdType %q already registered in V1 (remove V1 first)", cmdType)) - } if _, exists := r.handlersV2[cmdType]; exists { - panic(fmt.Sprintf("RegisterV2: cmdType %q already registered in V2", cmdType)) + panic(fmt.Sprintf("RegisterV2: cmdType %q already registered", cmdType)) } r.handlersV2[cmdType] = handlerV2Entry{handler: handler, deps: deps} } @@ -112,15 +73,3 @@ func (r *HandlerRegistry) RegisteredV2Types() []string { } return types } - -// hasV2 reports whether a V2 handler is registered for msgType. -func (r *HandlerRegistry) hasV2(msgType string) bool { - _, ok := r.handlersV2[msgType] - return ok -} - -// IsRegisteredV1 checks if a type is registered in the V1 map. -func (r *HandlerRegistry) IsRegisteredV1(msgType string) bool { - _, ok := r.handlers[msgType] - return ok -} diff --git a/Server/ws/registry_test.go b/Server/ws/registry_test.go index 29a63fe8..c7cd55ce 100644 --- a/Server/ws/registry_test.go +++ b/Server/ws/registry_test.go @@ -2,40 +2,23 @@ package ws import ( "context" - "encoding/json" "fmt" "sort" "strings" "testing" ) -func TestHandlerRegistry_RegisterAndDispatch(t *testing.T) { +// fullV2Registry registers the same handler set NewHub wires, so the +// migration-completeness tests below exercise the real production surface. +func fullV2Registry() *HandlerRegistry { r := NewHandlerRegistry() - - called := false - r.Register("test_type", func(ctx context.Context, h *Hub, c *Client, reqID string, payload json.RawMessage) { - called = true - if reqID != "req-1" { - t.Errorf("expected reqID %q, got %q", "req-1", reqID) - } - }) - - ok := r.Dispatch(context.Background(), "test_type", nil, nil, "req-1", nil) - if !ok { - t.Fatal("Dispatch returned false for registered type") - } - if !called { - t.Fatal("handler was not called") - } -} - -func TestHandlerRegistry_DispatchUnknownType(t *testing.T) { - r := NewHandlerRegistry() - - ok := r.Dispatch(context.Background(), "nonexistent", nil, nil, "", nil) - if ok { - t.Fatal("Dispatch returned true for unregistered type") - } + registerPingHandler(r, PingDeps{}) + registerChatHandlers(r, ChatDeps{}) + registerPresenceHandlers(r, PresenceDeps{}) + registerReactionHandlers(r, ReactionDeps{}) + r.RegisterV2(MsgTypeChatCommand, handleChatCommandV2, PluginDeps{}) + registerVoiceControlsV2(r, VoiceDeps{}) + return r } func TestRegisterV2AndDispatchV2(t *testing.T) { @@ -87,20 +70,6 @@ func TestDispatchV2_PanicIsRecovered(t *testing.T) { } } -func TestRegisterV2_ShadowingGuard_Panics(t *testing.T) { - r := NewHandlerRegistry() - r.Register("ping", func(ctx context.Context, h *Hub, c *Client, reqID string, payload json.RawMessage) {}) - - defer func() { - if r := recover(); r == nil { - t.Fatal("expected panic from shadowing guard, got none") - } - }() - r.RegisterV2("ping", func(ctx context.Context, cmd Command, info ClientInfo, deps any) Result { - return Result{} - }, PingDeps{}) -} - func TestRegisterV2_DuplicateGuard_Panics(t *testing.T) { r := NewHandlerRegistry() handler := func(ctx context.Context, cmd Command, info ClientInfo, deps any) Result { @@ -131,69 +100,13 @@ func TestRegisteredV2Types(t *testing.T) { } } -func TestV1StillWorks_WhenV2HasEntries(t *testing.T) { - r := NewHandlerRegistry() - - v1Called := false - r.Register("chat_send", func(ctx context.Context, h *Hub, c *Client, reqID string, payload json.RawMessage) { - v1Called = true - }) - - v2Handler := func(ctx context.Context, cmd Command, info ClientInfo, deps any) Result { - return Result{Reply: []byte("v2")} - } - r.RegisterV2("ping", v2Handler, PingDeps{}) - - // V1 dispatch still works - ok := r.Dispatch(context.Background(), "chat_send", nil, nil, "", nil) - if !ok || !v1Called { - t.Fatal("V1 dispatch broken when V2 has entries") - } - - // V2 dispatch for its own type works - cmd := &PingCmd{} - result, handled := r.DispatchV2(context.Background(), cmd, ClientInfo{}) - if !handled || string(result.Reply) != "v2" { - t.Fatal("V2 dispatch broken") - } - - // V2 dispatch for V1-only type returns false - chatCmd := &ChatSendCmd{} - _, handled = r.DispatchV2(context.Background(), chatCmd, ClientInfo{}) - if handled { - t.Fatal("V2 dispatch returned true for V1-only type") - } -} - -func TestIsRegisteredV1(t *testing.T) { - r := NewHandlerRegistry() - r.Register("chat_send", func(ctx context.Context, h *Hub, c *Client, reqID string, payload json.RawMessage) {}) - - if !r.IsRegisteredV1("chat_send") { - t.Fatal("expected true for registered V1 type") - } - if r.IsRegisteredV1("nonexistent") { - t.Fatal("expected false for unregistered type") - } -} - +// TestHandlerRegistry_AllExpectedTypesRegistered pins the full set of +// dispatchable types. After the V1→V2 migration completed, every type — voice +// join/leave and plugin commands included — is a typed V2 handler. func TestHandlerRegistry_AllExpectedTypesRegistered(t *testing.T) { - r := NewHandlerRegistry() - registerVoiceHandlersV1(r) - registerPingHandler(r, PingDeps{}) - registerChatHandlers(r, ChatDeps{}) - registerPresenceHandlers(r, PresenceDeps{}) - registerReactionHandlers(r, ReactionDeps{}) - registerVoiceControlsV2(r, VoiceDeps{}) + r := fullV2Registry() - // V1-only types (permanent — complex state/mutex requirements). - expectedV1 := []string{ - "voice_join", - "voice_leave", - } - - // V2-migrated types. - expectedV2 := []string{ + expected := []string{ "ping", "typing_start", "presence_update", @@ -203,6 +116,9 @@ func TestHandlerRegistry_AllExpectedTypesRegistered(t *testing.T) { "chat_send", "chat_edit", "chat_delete", + "chat_command", + "voice_join", + "voice_leave", "voice_mute", "voice_deafen", "voice_camera", @@ -212,31 +128,38 @@ func TestHandlerRegistry_AllExpectedTypesRegistered(t *testing.T) { "voice_token_refresh", } - registeredV1 := r.RegisteredTypes() - sort.Strings(registeredV1) - sort.Strings(expectedV1) + registered := r.RegisteredV2Types() + sort.Strings(registered) + sort.Strings(expected) - if len(registeredV1) != len(expectedV1) { - t.Fatalf("V1: expected %d registered types, got %d\nexpected: %v\ngot: %v", - len(expectedV1), len(registeredV1), expectedV1, registeredV1) + if len(registered) != len(expected) { + t.Fatalf("expected %d registered types, got %d\nexpected: %v\ngot: %v", + len(expected), len(registered), expected, registered) } - for i, typ := range expectedV1 { - if registeredV1[i] != typ { - t.Errorf("V1 mismatch at index %d: expected %q, got %q", i, typ, registeredV1[i]) + for i, typ := range expected { + if registered[i] != typ { + t.Errorf("mismatch at index %d: expected %q, got %q", i, typ, registered[i]) } } +} - registeredV2 := r.RegisteredV2Types() - sort.Strings(registeredV2) - sort.Strings(expectedV2) +// TestMigrationComplete_ConstructorHandlerParity locks the migration shut: +// every command constructor must have a registered V2 handler and vice versa, +// so a V1-style handler (a constructor with no V2 handler, or a handler with no +// strict parser) can never creep back in. +func TestMigrationComplete_ConstructorHandlerParity(t *testing.T) { + r := fullV2Registry() - if len(registeredV2) != len(expectedV2) { - t.Fatalf("V2: expected %d registered types, got %d\nexpected: %v\ngot: %v", - len(expectedV2), len(registeredV2), expectedV2, registeredV2) + v2 := make(map[string]bool) + for _, typ := range r.RegisteredV2Types() { + v2[typ] = true + if _, ok := getCommandConstructor(typ); !ok { + t.Errorf("V2 handler %q has no command constructor (strict parser missing)", typ) + } } - for i, typ := range expectedV2 { - if registeredV2[i] != typ { - t.Errorf("V2 mismatch at index %d: expected %q, got %q", i, typ, registeredV2[i]) + for typ := range commandConstructors { + if !v2[typ] { + t.Errorf("command constructor %q has no registered V2 handler", typ) } } } @@ -244,12 +167,7 @@ func TestHandlerRegistry_AllExpectedTypesRegistered(t *testing.T) { // TestAllV2Types_SmokeDispatch verifies that dispatching a minimal command // for every V2-registered type does not panic (validates deps wiring). func TestAllV2Types_SmokeDispatch(t *testing.T) { - r := NewHandlerRegistry() - registerPingHandler(r, PingDeps{}) - registerChatHandlers(r, ChatDeps{}) - registerPresenceHandlers(r, PresenceDeps{}) - registerReactionHandlers(r, ReactionDeps{}) - registerVoiceControlsV2(r, VoiceDeps{}) + r := fullV2Registry() // Minimal command for each V2 type — just needs Type() and UserID(). cmds := map[string]Command{ @@ -257,11 +175,14 @@ func TestAllV2Types_SmokeDispatch(t *testing.T) { MsgTypeChatSend: ChatSendCmd{userID: 1, channelID: 1}, MsgTypeChatEdit: ChatEditCmd{userID: 1, messageID: 1}, MsgTypeChatDelete: ChatDeleteCmd{userID: 1, messageID: 1}, + MsgTypeChatCommand: ChatCommandCmd{userID: 1, channelID: 1, command: "/x"}, MsgTypeTypingStart: TypingStartCmd{userID: 1, channelID: 1}, MsgTypePresenceUpdate: PresenceUpdateCmd{userID: 1, status: "online"}, MsgTypeChannelFocus: ChannelFocusCmd{userID: 1, channelID: 1}, MsgTypeReactionAdd: ReactionAddCmd{userID: 1, messageID: 1, emoji: "👍"}, MsgTypeReactionRemove: ReactionRemoveCmd{userID: 1, messageID: 1, emoji: "👍"}, + MsgTypeVoiceJoin: VoiceJoinCmd{userID: 1, channelID: 1}, + MsgTypeVoiceLeave: VoiceLeaveCmd{userID: 1}, MsgTypeVoiceMute: VoiceMuteCmd{userID: 1}, MsgTypeVoiceDeafen: VoiceDeafenCmd{userID: 1}, MsgTypeVoiceCamera: VoiceCameraCmd{userID: 1},