From 2e7a80171b9f415808b0b578671e416ace171692 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 13:32:58 +0000 Subject: [PATCH] feat(server,client): protocol codegen + audit quick-wins batch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protocol codegen (decision D4, audit A-2026-07-08): - Add docs/protocol-schema.json as the real single source of truth for WS message-type constants, making the long-standing 'generated from' comment in both constant files true. - Add Server/scripts/genprotocol, a generator emitting both Server/ws/message_types.go and Client .../lib/protocolTypes.ts (constants byte-for-byte value-identical to before; only headers, ordering alignment, and provenance comments changed). - Add make protocol-generate / protocol-verify and wire protocol-verify into CI next to sqlc-verify. Quick wins (decision D8): - admin: log LogAudit write failures in the backup handlers instead of discarding them (prior audit #10). - api: fix self-contradictory upload Cache-Control to 'private, no-cache' per remediation plan W3-4; drop the now-unused fileCacheMaxAgeSeconds constant; update test. - ws: route the hub settings cache through db.GetSetting instead of inline SQL. - ws: fix a latent data race — main.go wires SetEventPersister and SetEventStore after NewRouter has already started the hub Run loop, which reads those fields on the broadcast/replay paths. They (and pluginSink, which one test sets post-Run) are now atomic pointers; the remaining pre-Run-only setters reject late calls with an error log instead of racing silently. Update the audit closure table and decisions doc statuses accordingly. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UA17KPvqGBX3XbXYnMf1rA --- .github/workflows/ci.yml | 6 + Client/tauri-client/src/lib/protocolTypes.ts | 16 +- Server/Makefile | 24 ++- Server/admin/handlers_backup.go | 10 +- Server/api/constants.go | 3 - Server/api/upload_handler.go | 4 +- Server/api/upload_handler_test.go | 4 +- Server/scripts/genprotocol/main.go | 164 +++++++++++++++++++ Server/ws/hub.go | 80 ++++++--- Server/ws/message_types.go | 70 ++++---- Server/ws/serve.go | 5 +- docs/audit-2026-07-19.md | 8 +- docs/plans/audit-2026-07-19-decisions.md | 4 +- docs/protocol-schema.json | 77 +++++++++ 14 files changed, 390 insertions(+), 85 deletions(-) create mode 100644 Server/scripts/genprotocol/main.go create mode 100644 docs/protocol-schema.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1d269d0..56e5f069 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,12 @@ jobs: if: matrix.os == 'ubuntu-latest' run: make sqlc-install sqlc-verify + # Protocol message-type constants (Go + TS) must never drift from + # docs/protocol-schema.json — the single source of truth. + - name: Verify generated protocol constants (make protocol-verify) + if: matrix.os == 'ubuntu-latest' + run: make protocol-verify + - name: Run tests with race detection and coverage run: go test -race -timeout 20m ./... -coverprofile=coverage.out -cover diff --git a/Client/tauri-client/src/lib/protocolTypes.ts b/Client/tauri-client/src/lib/protocolTypes.ts index 88ff15b3..218e4ea9 100644 --- a/Client/tauri-client/src/lib/protocolTypes.ts +++ b/Client/tauri-client/src/lib/protocolTypes.ts @@ -1,6 +1,8 @@ -// Shared WebSocket protocol message type constants. -// Generated from docs/protocol-schema.json — single source of truth for -// both Server (Go) and Client (TypeScript). +// Code generated by scripts/genprotocol from docs/protocol-schema.json; DO NOT EDIT. +// +// Shared WebSocket protocol message type constants — single source of truth +// for both Server (Go) and Client (TypeScript). Edit docs/protocol-schema.json +// and run `make protocol-generate` in Server/. // // Usage: import { MessageType } from "@lib/protocolTypes"; // ws.send({ type: MessageType.CHAT_SEND, payload: { ... } }); @@ -24,10 +26,10 @@ export const ServerMessageType = { CHANNEL_UPDATE: "channel_update", CHANNEL_DELETE: "channel_delete", VOICE_STATE: "voice_state", - VOICE_LEAVE: "voice_leave", VOICE_CONFIG: "voice_config", VOICE_TOKEN: "voice_token", VOICE_SPEAKERS: "voice_speakers", + VOICE_LEAVE: "voice_leave", // broadcast (same string as client msg) MEMBER_JOIN: "member_join", MEMBER_LEAVE: "member_leave", MEMBER_UPDATE: "member_update", @@ -35,12 +37,11 @@ export const ServerMessageType = { MEMBER_BAN: "member_ban", SERVER_RESTART: "server_restart", ERROR: "error", - // Extensions (not in protocol-schema.json but used in practice) PONG: "pong", DM_CHANNEL_OPEN: "dm_channel_open", DM_CHANNEL_CLOSE: "dm_channel_close", - VOICE_E2EE_ANNOUNCE: "voice_e2ee_announce", - VOICE_E2EE_OFFER: "voice_e2ee_offer", + VOICE_E2EE_ANNOUNCE: "voice_e2ee_announce", // broadcast (same string as client msg) + VOICE_E2EE_OFFER: "voice_e2ee_offer", // relay (same string as client msg) } as const; export type ServerMessageTypeValue = (typeof ServerMessageType)[keyof typeof ServerMessageType]; @@ -66,7 +67,6 @@ export const ClientMessageType = { VOICE_CAMERA: "voice_camera", VOICE_SCREENSHARE: "voice_screenshare", PING: "ping", - // Extension (not in protocol-schema.json but used in practice) VOICE_TOKEN_REFRESH: "voice_token_refresh", VOICE_E2EE_ANNOUNCE: "voice_e2ee_announce", VOICE_E2EE_OFFER: "voice_e2ee_offer", diff --git a/Server/Makefile b/Server/Makefile index 5eb9012b..b2d05734 100644 --- a/Server/Makefile +++ b/Server/Makefile @@ -1,14 +1,16 @@ # OwnCord Server — developer convenience targets # -# sqlc-generate Regenerate type-safe Go from sqlc.yaml (db/dbgen). -# sqlc-verify Fail if the committed dbgen output is stale (used by CI). -# sqlc-install Install the pinned sqlc version into $GOBIN. -# otel-up Start Jaeger + Prometheus for local tracing development. -# otel-down Stop and remove the OTel dev containers. +# sqlc-generate Regenerate type-safe Go from sqlc.yaml (db/dbgen). +# sqlc-verify Fail if the committed dbgen output is stale (used by CI). +# sqlc-install Install the pinned sqlc version into $GOBIN. +# protocol-generate Regenerate WS message-type constants (Go + TS) from docs/protocol-schema.json. +# protocol-verify Fail if the committed protocol constants are stale (used by CI). +# otel-up Start Jaeger + Prometheus for local tracing development. +# otel-down Stop and remove the OTel dev containers. SQLC_VERSION := $(shell cat sqlc.version) -.PHONY: sqlc-install sqlc-generate sqlc-verify otel-up otel-down +.PHONY: sqlc-install sqlc-generate sqlc-verify protocol-generate protocol-verify otel-up otel-down sqlc-install: go install github.com/sqlc-dev/sqlc/cmd/sqlc@$(SQLC_VERSION) @@ -23,6 +25,16 @@ sqlc-verify: exit 1 ; \ ) +protocol-generate: + go run ./scripts/genprotocol + +protocol-verify: + go run ./scripts/genprotocol + @git diff --exit-code ws/message_types.go ../Client/tauri-client/src/lib/protocolTypes.ts || ( \ + echo "ERROR: generated protocol constants are stale. Run 'make protocol-generate' and commit the result." ; \ + exit 1 ; \ + ) + # Phase B Step 8 — local OTel development stack. # Starts Jaeger (traces) and Prometheus (metrics) in Docker. # Jaeger UI: http://localhost:16686 diff --git a/Server/admin/handlers_backup.go b/Server/admin/handlers_backup.go index 6c4727a7..578a3509 100644 --- a/Server/admin/handlers_backup.go +++ b/Server/admin/handlers_backup.go @@ -52,8 +52,10 @@ func handleBackup(database *db.DB) http.Handler { actor := actorFromContext(r) backupName := filepath.Base(backupPath) slog.Info("database backup created", "actor_id", actor, "name", backupName) - _ = database.LogAudit(actor, "backup_create", "server", 0, - fmt.Sprintf("backup saved: %s", backupName)) + if err := database.LogAudit(actor, "backup_create", "server", 0, + fmt.Sprintf("backup saved: %s", backupName)); err != nil { + slog.Error("audit log write failed", "action", "backup_create", "actor_id", actor, "error", err) + } writeJSON(w, http.StatusOK, map[string]string{ "path": filepath.Base(backupPath), @@ -136,7 +138,9 @@ func handleDeleteBackup(database *db.DB) http.Handler { actor := actorFromContext(r) slog.Info("backup deleted", "actor_id", actor, "name", name) - _ = database.LogAudit(actor, "backup_delete", "server", 0, "deleted backup "+name) + if err := database.LogAudit(actor, "backup_delete", "server", 0, "deleted backup "+name); err != nil { + slog.Error("audit log write failed", "action", "backup_delete", "actor_id", actor, "error", err) + } w.WriteHeader(http.StatusNoContent) }) diff --git a/Server/api/constants.go b/Server/api/constants.go index 523e440b..2ca0973a 100644 --- a/Server/api/constants.go +++ b/Server/api/constants.go @@ -121,9 +121,6 @@ const ( // hstsMaxAgeSeconds is the max-age value for the Strict-Transport-Security header. hstsMaxAgeSeconds = 31536000 - - // fileCacheMaxAgeSeconds is the max-age value for the Cache-Control header on served files. - fileCacheMaxAgeSeconds = 31536000 ) // ─── Size limits ──────────────────────────────────────────────────────────── diff --git a/Server/api/upload_handler.go b/Server/api/upload_handler.go index b83a8292..016106c1 100644 --- a/Server/api/upload_handler.go +++ b/Server/api/upload_handler.go @@ -306,7 +306,9 @@ func handleServeFile(database *db.DB, store *storage.Storage, allowedOrigins []s w.Header().Set("Content-Disposition", mime.FormatMediaType(disposition, map[string]string{"filename": aa.Filename})) // These downloads are access-controlled, so they must never be stored by // shared/proxy caches (info-leak). Mark private and force revalidation. - w.Header().Set("Cache-Control", fmt.Sprintf("private, max-age=%d, no-cache", fileCacheMaxAgeSeconds)) + // W3-4: no-cache forces revalidation on every use, so a max-age is dead + // weight alongside it — private + no-cache expresses the intent exactly. + w.Header().Set("Cache-Control", "private, no-cache") // The Access-Control-Allow-Origin header below reflects the request // Origin, so responses vary by Origin and must not be cross-served. w.Header().Set("Vary", "Origin") diff --git a/Server/api/upload_handler_test.go b/Server/api/upload_handler_test.go index 9a68bab0..95b1fc4a 100644 --- a/Server/api/upload_handler_test.go +++ b/Server/api/upload_handler_test.go @@ -730,8 +730,8 @@ func TestServeFile_Success(t *testing.T) { // Verify cache control header. Access-controlled downloads must be marked // private + no-cache so shared/proxy caches never store them (info-leak). cc := rr2.Header().Get("Cache-Control") - if cc != "private, max-age=31536000, no-cache" { - t.Errorf("Cache-Control = %q, want 'private, max-age=31536000, no-cache'", cc) + if cc != "private, no-cache" { + t.Errorf("Cache-Control = %q, want 'private, no-cache'", cc) } // Verify Content-Disposition header. diff --git a/Server/scripts/genprotocol/main.go b/Server/scripts/genprotocol/main.go new file mode 100644 index 00000000..f147b43b --- /dev/null +++ b/Server/scripts/genprotocol/main.go @@ -0,0 +1,164 @@ +// genprotocol generates the WebSocket protocol message-type constant files +// for both the Go server and the TypeScript client from the single source of +// truth at docs/protocol-schema.json. +// +// Usage (from the Server/ directory): +// +// go run ./scripts/genprotocol +// +// or via make: +// +// make protocol-generate # regenerate both outputs +// make protocol-verify # fail if committed outputs are stale (CI) +package main + +import ( + "encoding/json" + "flag" + "fmt" + "go/format" + "log" + "os" + "strings" +) + +type message struct { + Wire string `json:"wire"` + Go string `json:"go"` + TS string `json:"ts"` + Note string `json:"note,omitempty"` + GoTrailingComment string `json:"go_trailing_comment,omitempty"` +} + +type schema struct { + Comment string `json:"$comment"` + Version int `json:"version"` + ClientToServer []message `json:"client_to_server"` + ServerToClient []message `json:"server_to_client"` +} + +func main() { + schemaPath := flag.String("schema", "../docs/protocol-schema.json", "path to protocol-schema.json") + goOut := flag.String("go-out", "ws/message_types.go", "path to the generated Go file") + tsOut := flag.String("ts-out", "../Client/tauri-client/src/lib/protocolTypes.ts", "path to the generated TypeScript file") + flag.Parse() + + raw, err := os.ReadFile(*schemaPath) + if err != nil { + log.Fatalf("read schema: %v", err) + } + var s schema + dec := json.NewDecoder(strings.NewReader(string(raw))) + dec.DisallowUnknownFields() + if err := dec.Decode(&s); err != nil { + log.Fatalf("parse schema: %v", err) + } + if err := validate(s); err != nil { + log.Fatalf("invalid schema: %v", err) + } + + goSrc, err := renderGo(s) + if err != nil { + log.Fatalf("render Go: %v", err) + } + if err := os.WriteFile(*goOut, goSrc, 0o644); err != nil { + log.Fatalf("write %s: %v", *goOut, err) + } + if err := os.WriteFile(*tsOut, []byte(renderTS(s)), 0o644); err != nil { + log.Fatalf("write %s: %v", *tsOut, err) + } + fmt.Printf("generated %s (%d client→server, %d server→client) and %s\n", + *goOut, len(s.ClientToServer), len(s.ServerToClient), *tsOut) +} + +// validate rejects duplicate identifiers and empty fields early so a bad +// schema edit fails the generator instead of producing broken output. +func validate(s schema) error { + goNames := map[string]bool{} + for _, list := range [][]message{s.ClientToServer, s.ServerToClient} { + tsNames := map[string]bool{} + for _, m := range list { + if m.Wire == "" || m.Go == "" || m.TS == "" { + return fmt.Errorf("entry %+v: wire, go, and ts are all required", m) + } + if goNames[m.Go] { + return fmt.Errorf("duplicate Go constant %q", m.Go) + } + goNames[m.Go] = true + if tsNames[m.TS] { + return fmt.Errorf("duplicate TS key %q within one direction", m.TS) + } + tsNames[m.TS] = true + } + } + return nil +} + +func header(comment string) string { + return "// Code generated by scripts/genprotocol from docs/protocol-schema.json; DO NOT EDIT.\n" + + comment + "\n" +} + +func renderGo(s schema) ([]byte, error) { + var b strings.Builder + b.WriteString(header("//\n// WebSocket protocol message type constants — single source of truth for\n" + + "// both Server (Go) and Client (TypeScript). Edit docs/protocol-schema.json\n" + + "// and run `make protocol-generate` (see Server/Makefile).")) + b.WriteString("\npackage ws\n\n") + + writeBlock := func(title string, msgs []message) { + b.WriteString("// " + title + "\nconst (\n") + for _, m := range msgs { + b.WriteString("\t" + m.Go + " = " + fmt.Sprintf("%q", m.Wire)) + switch { + case m.GoTrailingComment != "": + b.WriteString(" " + m.GoTrailingComment) + case m.Note != "": + b.WriteString(" // " + m.Note) + } + b.WriteString("\n") + } + b.WriteString(")\n") + } + + writeBlock("Client → Server message types (received by handlers).", s.ClientToServer) + b.WriteString("\n") + writeBlock("Server → Client message types (sent in broadcasts/responses).", s.ServerToClient) + + return format.Source([]byte(b.String())) +} + +func renderTS(s schema) string { + var b strings.Builder + b.WriteString(header("//\n// Shared WebSocket protocol message type constants — single source of truth\n" + + "// for both Server (Go) and Client (TypeScript). Edit docs/protocol-schema.json\n" + + "// and run `make protocol-generate` in Server/.\n//\n" + + "// Usage: import { MessageType } from \"@lib/protocolTypes\";\n" + + "// ws.send({ type: MessageType.CHAT_SEND, payload: { ... } });")) + + writeBlock := func(title, name string, msgs []message) { + b.WriteString("\n// " + strings.Repeat("-", 75) + "\n") + b.WriteString("// " + title + "\n") + b.WriteString("// " + strings.Repeat("-", 75) + "\n\n") + b.WriteString("export const " + name + " = {\n") + for _, m := range msgs { + b.WriteString(" " + m.TS + ": " + fmt.Sprintf("%q", m.Wire) + ",") + if m.Note != "" { + b.WriteString(" // " + m.Note) + } + b.WriteString("\n") + } + b.WriteString("} as const;\n\n") + b.WriteString("export type " + name + "Value = (typeof " + name + ")[keyof typeof " + name + "];\n") + } + + writeBlock("Server → Client message types", "ServerMessageType", s.ServerToClient) + writeBlock("Client → Server message types", "ClientMessageType", s.ClientToServer) + + b.WriteString("\n// " + strings.Repeat("-", 75) + "\n") + b.WriteString("// Unified MessageType — all message types in one object for convenience\n") + b.WriteString("// " + strings.Repeat("-", 75) + "\n\n") + b.WriteString("export const MessageType = {\n ...ServerMessageType,\n ...ClientMessageType,\n} as const;\n\n") + b.WriteString("export type MessageTypeValue = (typeof MessageType)[keyof typeof MessageType];\n") + return b.String() +} diff --git a/Server/ws/hub.go b/Server/ws/hub.go index 7e4b5e8d..ba12e9ea 100644 --- a/Server/ws/hub.go +++ b/Server/ws/hub.go @@ -61,13 +61,19 @@ type Hub struct { replayBuf *EventRingBuffer // recent broadcast events for reconnection replay broadcastDrops atomic.Uint64 // counts messages dropped due to full broadcast channel - // Phase B Step 7 — event persistence. nil = ring buffer only. - eventPersister *EventPersister - eventStore store.EventStore // read path for cold-tier replay + // Phase B Step 7 — event persistence. nil = ring buffer only. Atomic + // because main.go wires these after NewRouter has already started the + // Run loop, which reads them on the broadcast/replay paths. + eventPersister atomic.Pointer[EventPersister] + eventStore atomic.Pointer[store.EventStore] // read path for cold-tier replay // Phase C Step 9 — plugin wiring. - pluginRegistry *plugin.Registry // slash-command dispatch; nil = no plugins - pluginSink *plugin.EventSink // hub→plugin event fan-out; nil = no plugins + pluginRegistry *plugin.Registry // slash-command dispatch; nil = no plugins; wire before Run + pluginSink atomic.Pointer[plugin.EventSink] // hub→plugin event fan-out; nil = no plugins + + // running flips when Run starts; plain-field setters check it so a late + // call fails loudly instead of racing the dispatch loop. + running atomic.Bool // Phase B Step 7 — reconnection tier metrics. Incremented per resume. reconnectTierBuf atomic.Uint64 @@ -178,18 +184,21 @@ func (h *Hub) refreshSettingsLocked() { if h.db == nil { return } - var name, motd string - if err := h.db.QueryRow("SELECT value FROM settings WHERE key='server_name'").Scan(&name); err == nil { + if name, err := h.db.GetSetting("server_name"); err == nil { h.settingsName = name } - if err := h.db.QueryRow("SELECT value FROM settings WHERE key='motd'").Scan(&motd); err == nil { + if motd, err := h.db.GetSetting("motd"); err == nil { h.settingsMotd = motd } h.settingsLastUpdate = time.Now() } -// SetLiveKit sets the LiveKit client on the hub. Must be called before Run. +// SetLiveKit sets the LiveKit client on the hub. Must be called before Run; +// late calls are ignored with an error log. func (h *Hub) SetLiveKit(lk *LiveKitClient) { + if h.rejectIfRunning("SetLiveKit") { + return + } h.livekit = lk } @@ -222,8 +231,12 @@ func (h *Hub) LiveKitHealthCheck(ctx context.Context) (bool, error) { return h.livekit.HealthCheck(ctx) } -// SetLiveKitProcess sets the LiveKit process manager on the hub. +// SetLiveKitProcess sets the LiveKit process manager on the hub. Must be +// called before Run; late calls are ignored with an error log. func (h *Hub) SetLiveKitProcess(p *LiveKitProcess) { + if h.rejectIfRunning("SetLiveKitProcess") { + return + } h.lkProcess = p } @@ -234,6 +247,7 @@ func (h *Hub) SetLiveKitProcess(p *LiveKitProcess) { // panics more than 3 times within a 60-second window it stops permanently to // avoid a tight crash loop. func (h *Hub) Run() { + h.running.Store(true) var panicCount int var lastPanicReset time.Time @@ -749,28 +763,53 @@ func (h *Hub) SeedSeq(seed uint64) { } // SetEventPersister attaches a persister so subsequent broadcasts are also -// written to the persistent EventStore. Pass nil to disable. +// written to the persistent EventStore. Pass nil to disable. Safe to call +// at any time, including after Run has started. func (h *Hub) SetEventPersister(p *EventPersister) { - h.eventPersister = p + h.eventPersister.Store(p) } // SetEventStore attaches a read-side EventStore used by the cold-tier // reconnect replay path. Typically the same store backing SetEventPersister. +// Pass nil to disable. Safe to call at any time, including after Run has +// started. func (h *Hub) SetEventStore(s store.EventStore) { - h.eventStore = s + if s == nil { + h.eventStore.Store(nil) + return + } + h.eventStore.Store(&s) } // SetPluginRegistry wires the plugin.Registry so the hub can dispatch // slash commands (chat_command messages) to plugin-owned handlers. -// Pass nil to disable plugin command dispatch. +// Pass nil to disable plugin command dispatch. Must be called before Run; +// late calls are ignored with an error log. func (h *Hub) SetPluginRegistry(r *plugin.Registry) { + if h.rejectIfRunning("SetPluginRegistry") { + return + } h.pluginRegistry = r } // SetPluginEventSink wires the plugin.EventSink so the hub fans out each -// sequenced broadcast to subscribed plugins. Pass nil to disable. +// sequenced broadcast to subscribed plugins. Pass nil to disable. Safe to +// call at any time, including after Run has started. func (h *Hub) SetPluginEventSink(s *plugin.EventSink) { - h.pluginSink = s + h.pluginSink.Store(s) +} + +// rejectIfRunning reports whether Run has already started, logging an error +// when it has. Plain-field setters must be wired before Run: the dispatch +// loop and connection goroutines read those fields without synchronization, +// so a late set would be a data race. Late calls are dropped. +func (h *Hub) rejectIfRunning(setter string) bool { + if h.running.Load() { + slog.Error("ws: setter called after Hub.Run started; ignoring (must be wired before Run)", + "setter", setter) + return true + } + return false } // ReconnectTierStats returns the per-tier resume hit counters in the order @@ -786,7 +825,8 @@ func (h *Hub) ReconnectTierStats() (buffer, db, full uint64) { // written to the EventStore has a row-seq that matches the wrapped-payload // seq the client tracks. func (h *Hub) persistEvent(seq uint64, channelID int64, payload []byte) { - if h.eventPersister == nil { + p := h.eventPersister.Load() + if p == nil { return } eventType := extractEventType(payload) @@ -796,7 +836,7 @@ func (h *Hub) persistEvent(seq uint64, channelID int64, payload []byte) { eventType = "channel_broadcast" } } - h.eventPersister.Enqueue(int64(seq), eventType, channelID, payload) //nolint:gosec // seq is a monotonically increasing counter, never reaches MaxInt64 + p.Enqueue(int64(seq), eventType, channelID, payload) //nolint:gosec // seq is a monotonically increasing counter, never reaches MaxInt64 } // extractEventType scans a wrapped JSON envelope for the value of the "type" @@ -987,12 +1027,12 @@ func (h *Hub) deliverBroadcast(bm broadcastMsg) { // conceptually — but since seqMu is still held here, the call MUST NOT // re-enter the hub. The default build is safe; the wazero build should // dispatch asynchronously once the runtime is real. - if h.pluginSink != nil { + if sink := h.pluginSink.Load(); sink != nil { eventType := extractEventType(msg) if eventType == "" { eventType = "broadcast" } - h.pluginSink.Dispatch(context.Background(), eventType, msg) + sink.Dispatch(context.Background(), eventType, msg) } if bm.channelID == 0 { diff --git a/Server/ws/message_types.go b/Server/ws/message_types.go index 626e93bf..7e08c2a5 100644 --- a/Server/ws/message_types.go +++ b/Server/ws/message_types.go @@ -1,9 +1,11 @@ +// Code generated by scripts/genprotocol from docs/protocol-schema.json; DO NOT EDIT. +// +// WebSocket protocol message type constants — single source of truth for +// both Server (Go) and Client (TypeScript). Edit docs/protocol-schema.json +// and run `make protocol-generate` (see Server/Makefile). + package ws -// WebSocket protocol message type constants. -// Generated from docs/protocol-schema.json — single source of truth for -// both Server (Go) and Client (TypeScript). -// // Client → Server message types (received by handlers). const ( MsgTypeAuth = "auth" @@ -29,34 +31,34 @@ const ( // Server → Client message types (sent in broadcasts/responses). const ( - MsgTypeAuthOK = "auth_ok" - MsgTypeAuthError = "auth_error" - MsgTypeReady = "ready" - MsgTypeChatMessage = "chat_message" - MsgTypeChatSendOK = "chat_send_ok" - MsgTypeChatEdited = "chat_edited" - MsgTypeChatDeleted = "chat_deleted" - MsgTypeReactionUpdate = "reaction_update" - MsgTypeTyping = "typing" - MsgTypePresence = "presence" - MsgTypeChannelCreate = "channel_create" - MsgTypeChannelUpdate = "channel_update" - MsgTypeChannelDelete = "channel_delete" - MsgTypeVoiceState = "voice_state" - MsgTypeVoiceConfig = "voice_config" - MsgTypeVoiceToken = "voice_token" - MsgTypeVoiceSpeakers = "voice_speakers" - MsgTypeVoiceLeaveBC = "voice_leave" // broadcast (same string as client msg) - MsgTypeMemberJoin = "member_join" - MsgTypeMemberLeave = "member_leave" - MsgTypeMemberUpdate = "member_update" - MsgTypeUserUpdate = "user_update" - MsgTypeMemberBan = "member_ban" - MsgTypeServerRestart = "server_restart" - MsgTypeError = "error" - MsgTypePong = "pong" - MsgTypeDMChannelOpen = "dm_channel_open" - MsgTypeDMChannelClose = "dm_channel_close" - MsgTypeVoiceE2EEAnnounceBC = "voice_e2ee_announce" // broadcast (same string as client msg) - MsgTypeVoiceE2EEOfferRelay = "voice_e2ee_offer" // relay (same string as client msg) + MsgTypeAuthOK = "auth_ok" + MsgTypeAuthError = "auth_error" + MsgTypeReady = "ready" + MsgTypeChatMessage = "chat_message" + MsgTypeChatSendOK = "chat_send_ok" + MsgTypeChatEdited = "chat_edited" + MsgTypeChatDeleted = "chat_deleted" + MsgTypeReactionUpdate = "reaction_update" + MsgTypeTyping = "typing" + MsgTypePresence = "presence" + MsgTypeChannelCreate = "channel_create" + MsgTypeChannelUpdate = "channel_update" + MsgTypeChannelDelete = "channel_delete" + MsgTypeVoiceState = "voice_state" + MsgTypeVoiceConfig = "voice_config" + MsgTypeVoiceToken = "voice_token" + MsgTypeVoiceSpeakers = "voice_speakers" + MsgTypeVoiceLeaveBC = "voice_leave" // broadcast (same string as client msg) + MsgTypeMemberJoin = "member_join" + MsgTypeMemberLeave = "member_leave" + MsgTypeMemberUpdate = "member_update" + MsgTypeUserUpdate = "user_update" + MsgTypeMemberBan = "member_ban" + MsgTypeServerRestart = "server_restart" + MsgTypeError = "error" + MsgTypePong = "pong" + MsgTypeDMChannelOpen = "dm_channel_open" + MsgTypeDMChannelClose = "dm_channel_close" + MsgTypeVoiceE2EEAnnounceBC = "voice_e2ee_announce" // broadcast (same string as client msg) + MsgTypeVoiceE2EEOfferRelay = "voice_e2ee_offer" // relay (same string as client msg) ) diff --git a/Server/ws/serve.go b/Server/ws/serve.go index 5ecb6894..903ddc4b 100644 --- a/Server/ws/serve.go +++ b/Server/ws/serve.go @@ -136,13 +136,14 @@ func (h *Hub) handleReconnect( if events == nil { // Phase B Step 7 — try cold-tier replay from the EventStore before // giving up and forcing a full ready re-sync. - if h.eventStore != nil { + if esp := h.eventStore.Load(); esp != nil { + es := *esp channelIDs := make([]int64, 0, len(allowedChannelIDs)) for cid := range allowedChannelIDs { channelIDs = append(channelIDs, cid) } const maxColdReplay = 5000 - persisted, dbErr := h.eventStore.GetEventsSinceForChannels(ctx, int64(lastSeq), channelIDs, maxColdReplay) //nolint:gosec // lastSeq is a sequence counter bounded well below MaxInt64 + persisted, dbErr := es.GetEventsSinceForChannels(ctx, int64(lastSeq), channelIDs, maxColdReplay) //nolint:gosec // lastSeq is a sequence counter bounded well below MaxInt64 if dbErr != nil { slog.Warn("ws handleReconnect: cold-tier replay query failed", "user_id", c.userID, "err", dbErr) diff --git a/docs/audit-2026-07-19.md b/docs/audit-2026-07-19.md index 092be873..be3c73c6 100644 --- a/docs/audit-2026-07-19.md +++ b/docs/audit-2026-07-19.md @@ -21,7 +21,7 @@ accepted-risk note before the beta gate. MEDIUMs are folded into the backlog | A-2026-07-05 | MEDIUM | Dead sqlc layer: `Server/db/dbgen/` (~3.5k LOC) generated + CI-verified but imported by nothing | DECIDED 2026-07-19 — adopt sqlc as the real query layer (D2) (see [plans/audit-2026-07-19-decisions.md](plans/audit-2026-07-19-decisions.md)) | | A-2026-07-06 | MEDIUM | Three coexisting DB-access styles (raw `*db.DB` in api/admin/ws, `store.Store` under service, dead dbgen) | DECIDED 2026-07-19 — single data layer: sqlc-backed db pkg, remove store/ (D2+D3) (see [plans/audit-2026-07-19-decisions.md](plans/audit-2026-07-19-decisions.md)) | | A-2026-07-07 | MEDIUM | Channel-visibility logic duplicated across ~4 sites with "must mirror" comments | OPEN | -| A-2026-07-08 | MEDIUM | Protocol constants on both sides claim generation from `docs/protocol-schema.json`, which does not exist in the repo | DECIDED 2026-07-19 — build real codegen; greenlit (D4) (see [plans/audit-2026-07-19-decisions.md](plans/audit-2026-07-19-decisions.md)) | +| A-2026-07-08 | MEDIUM | Protocol constants on both sides claim generation from `docs/protocol-schema.json`, which does not exist in the repo | CLOSED 2026-07-19 — codegen implemented: `docs/protocol-schema.json` + `Server/scripts/genprotocol` + `make protocol-verify` CI gate | | A-2026-07-09 | MEDIUM | Dual V1+V2 WS dispatch (strangler-fig) still live; two parsers/registries to keep in sync | OPEN | | A-2026-07-10 | MEDIUM | `api.NewRouter` god-constructor: builds services, hub, LiveKit, updater, admin, plugins; spawns goroutines; mounts everything | OPEN | | A-2026-07-11 | MEDIUM | `ws.Hub` mega-object with post-construction `Set*` wiring ("must be called before Run") | OPEN | @@ -53,9 +53,9 @@ audit's table; details stay in [audit-2026-04-07.md](audit-2026-04-07.md). | 6 | HIGH | `Server/store/` untested | **Still zero `_test.go` files** in `Server/store/`. The "SUPERSEDED — remove in P4" plan has not executed; the package remains the designated abstraction seam with no direct tests | | 7 | HIGH | Client unit coverage | Suite is large (157 test files) but currently KNOWN RED and non-blocking — see A-2026-07-04 | | 9 | MEDIUM | auth_handler bypasses service layer | **Confirmed open** — `Server/api/router.go:101` passes `database *db.DB` to `MountAuthRoutes` while sibling mounts receive `svc` | -| 10 | MEDIUM | Audit-trail write failures silently ignored | **Confirmed open** — `Server/admin/handlers_backup.go:55` and `:139` still discard the error: `_ = database.LogAudit(...)` | +| 10 | MEDIUM | Audit-trail write failures silently ignored | **Fixed 2026-07-19** at the two flagged backup-handler sites (errors now logged). Wider scope discovered: the `_ = LogAudit` pattern exists at 23 call sites across admin/api/ws/service — appears to be a deliberate best-effort convention; policy decision tracked in the decisions doc (D8 note) | | 11 | MEDIUM | E2E not in CI | **Confirmed open** — no Playwright job exists in `.github/workflows/ci.yml` | -| W3-4 (remediation plan) | LOW | Contradictory upload cache header | **Confirmed open** — `Server/api/upload_handler.go:309` sets `private, max-age=%d, no-cache` (max-age and no-cache contradict) | +| W3-4 (remediation plan) | LOW | Contradictory upload cache header | **Fixed 2026-07-19** — now `private, no-cache` per the remediation plan's prescription | --- @@ -120,7 +120,7 @@ that grew around that design. | A-2026-07-09 | MEDIUM | Real-time | `Server/ws/handlers.go` (`handleMessage` V2-then-V1 fallback), dual registration in `NewHub` (`Server/ws/hub.go`) | Strangler-fig V1+V2 dispatch is live: two parsers (lenient/strict), two registries, per-type duplication. | Finish the migration: port remaining V1 types to V2, then delete the V1 path. Track remaining types in an issue so the count visibly shrinks. | M/L | | A-2026-07-10 | MEDIUM | Composition | `Server/api/router.go:34` (`NewRouter`, ~278 lines) | God-constructor builds rate limiter, TOTP key, storage, services, hub, LiveKit client+process, updater, admin + plugin handlers; spawns goroutines; returns a cleanup closure covering only one of them. Hard to test wiring in isolation; lifecycle ownership is implicit. | Split construction (a `Deps`/`App` struct built in `main.go`) from route mounting (`NewRouter(deps)`); return a composite `io.Closer`. | M | | A-2026-07-11 | MEDIUM | Real-time | `Server/ws/hub.go` (`SetLiveKit`, `SetEventPersister`, `SetPluginRegistry`, …) | Hub is a mega-object wired post-construction via setters that "must be called before Run" — temporal coupling; a missed setter is a nil-deref at runtime, not a compile error. | Move required collaborators into `NewHub` params (or an options struct validated before `Run`). Full Hub decomposition is a separate, larger effort (backlog 12). | S (constructor) / L (decomposition) | -| — | MEDIUM | Layering | `Server/ws/hub.go:182` (`refreshSettingsLocked`) | Hub runs inline `SELECT value FROM settings WHERE key='server_name'` instead of using `SettingsStore` — the only raw SQL in the real-time layer. | Route through the store; folds into A-2026-07-06 but is a 20-minute standalone fix. | S | +| — | MEDIUM | Layering | `Server/ws/hub.go:182` (`refreshSettingsLocked`) | Hub runs inline `SELECT value FROM settings WHERE key='server_name'` instead of using `SettingsStore` — the only raw SQL in the real-time layer. | **Fixed 2026-07-19** — now uses `db.GetSetting`; full consolidation folds into A-2026-07-06. | S | | — | LOW | Scaling posture | `Server/auth/ratelimit.go` (documented), in-memory pub/sub + ring buffer, process-local TOTP replay | Single-instance coupling is structural and *documented* — this is a deliberate design, not a bug. Recorded here so the constraint stays visible ([architecture/system-overview.md D8](architecture/system-overview.md)). | No action now; revisit only if multi-instance ever becomes a goal. | — | | A-2026-07-13 | LOW | Schema hygiene | `sounds` table (`Server/migrations/001`), `audit_log` + `audit_log_v6` (`003`) | Dead/duplicated schema: soundboard was removed but its table remains; two audit-log tables coexist after the 003 rebuild. | Add a cleanup migration (drop `sounds`, finish the audit_log consolidation) next time a migration ships anyway. | S | diff --git a/docs/plans/audit-2026-07-19-decisions.md b/docs/plans/audit-2026-07-19-decisions.md index a010a4b9..34d7ecd4 100644 --- a/docs/plans/audit-2026-07-19-decisions.md +++ b/docs/plans/audit-2026-07-19-decisions.md @@ -17,11 +17,11 @@ here (and the audit's closure table) as items land. | D1 | `announcement` channel type (documented + offered by admin API, rejected by DB triggers) | A-2026-07-01 | **Implement end-to-end**: migration to allow the type, posting-permission semantics, admin support, client rendering, spec updates. Not a doc-strip — this becomes a real feature. | Planned (not yet greenlit to start) | | D2 | Data-layer direction (raw SQL vs dead sqlc `db/dbgen` vs `store.Store`) | A-2026-07-05 / A-2026-07-06 | **Adopt sqlc for real**: wire `db.DB` method bodies to the generated `dbgen` queries so sqlc becomes the actual, type-checked query layer. The `sqlc-verify` CI job stays and starts earning its keep. | Planned | | D3 | Fate of `Server/store/` (untested abstraction seam) | prior audit #6 | **Remove `store/`**: execute the prior audit's P4 "single data layer" direction. Services call the (sqlc-backed) `db` package directly; tests use in-memory SQLite instead of `MemStore`. | Planned (sequence with/after D2) | -| D4 | Protocol constants sync (`message_types.go` / `protocolTypes.ts` claim a nonexistent `docs/protocol-schema.json`) | A-2026-07-08 | **Create real codegen**: commit an actual `protocol-schema.json` plus a generator that emits the Go and TS constant files (and, ideally, protocol.md's message table), making the "single source of truth" comment true. | **Greenlit** | +| D4 | Protocol constants sync (`message_types.go` / `protocolTypes.ts` claim a nonexistent `docs/protocol-schema.json`) | A-2026-07-08 | **Create real codegen**: commit an actual `protocol-schema.json` plus a generator that emits the Go and TS constant files (and, ideally, protocol.md's message table), making the "single source of truth" comment true. | **Implemented 2026-07-19**: `docs/protocol-schema.json` + `Server/scripts/genprotocol` + `make protocol-generate`/`protocol-verify` + CI gate. protocol.md table generation deferred to D7. | | D5 | Client HTTP TLS gap (`allowSelfSigned: true`, no TOFU pinning on the REST path) | A-2026-07-02 | **Next security work**: build the TOFU HTTP proxy in Rust (mirroring `ws_proxy.rs`) as the next security task — highest-priority security item. | Planned | | D6 | Abandoned SolidJS beachhead + stale `docs/client-architecture.md` | A-2026-07-12 | **Delete it all**: remove `src/components/solid/`, `solidMount`/`solidAdapter`, `vite-plugin-solid`, and Solid test deps; retire `client-architecture.md` in favor of [docs/architecture/client.md](../architecture/client.md). | Planned | | D7 | Spec refresh strategy for api.md / protocol.md / schema.md | A-2026-07-03 | **One refresh PR first**, using the audit's §2 conformance matrix as the checklist; afterwards specs are kept current per-PR (see the maintenance rule in [docs/architecture/README.md](../architecture/README.md)). Announcement channels (D1) later update the *fresh* specs. | Planned | -| D8 | What to implement first | backlog §6 | **Greenlit now: Protocol codegen (D4) + the quick-wins batch** — `LogAudit` error handling (`admin/handlers_backup.go`), contradictory upload `Cache-Control` (`upload_handler.go`), hub inline settings SQL through the data layer (`ws/hub.go`), Hub constructor cleanup (required collaborators into `NewHub`). | **Greenlit** | +| D8 | What to implement first | backlog §6 | **Greenlit now: Protocol codegen (D4) + the quick-wins batch** — `LogAudit` error handling (`admin/handlers_backup.go`), contradictory upload `Cache-Control` (`upload_handler.go`), hub inline settings SQL through the data layer (`ws/hub.go`), Hub constructor cleanup (required collaborators into `NewHub`). | **Implemented 2026-07-19** (all four quick wins + D4). Hub cleanup shipped as: race fix — `eventPersister`/`eventStore`/`pluginSink` are now atomic (they were plain fields written by `main.go` after `NewRouter` had already started `Run`); remaining pre-Run setters now reject late calls with an error log instead of racing silently. Note discovered during the work: the discarded-`LogAudit` pattern is repo-wide (23 call sites) — the two tracker-flagged backup handlers are fixed; whether best-effort audit writes stay the convention elsewhere needs a policy decision. | ## Suggested sequencing diff --git a/docs/protocol-schema.json b/docs/protocol-schema.json new file mode 100644 index 00000000..6ed92348 --- /dev/null +++ b/docs/protocol-schema.json @@ -0,0 +1,77 @@ +{ + "$comment": "Single source of truth for WebSocket protocol message-type constants. Server/ws/message_types.go and Client/tauri-client/src/lib/protocolTypes.ts are generated from this file — edit here, then run `make protocol-generate` in Server/. CI runs `make protocol-verify` to reject drift.", + "version": 1, + "client_to_server": [ + { "wire": "auth", "go": "MsgTypeAuth", "ts": "AUTH" }, + { "wire": "chat_send", "go": "MsgTypeChatSend", "ts": "CHAT_SEND" }, + { "wire": "chat_edit", "go": "MsgTypeChatEdit", "ts": "CHAT_EDIT" }, + { "wire": "chat_delete", "go": "MsgTypeChatDelete", "ts": "CHAT_DELETE" }, + { "wire": "reaction_add", "go": "MsgTypeReactionAdd", "ts": "REACTION_ADD" }, + { "wire": "reaction_remove", "go": "MsgTypeReactionRemove", "ts": "REACTION_REMOVE" }, + { "wire": "typing_start", "go": "MsgTypeTypingStart", "ts": "TYPING_START" }, + { "wire": "channel_focus", "go": "MsgTypeChannelFocus", "ts": "CHANNEL_FOCUS" }, + { "wire": "presence_update", "go": "MsgTypePresenceUpdate", "ts": "PRESENCE_UPDATE" }, + { "wire": "voice_join", "go": "MsgTypeVoiceJoin", "ts": "VOICE_JOIN" }, + { "wire": "voice_leave", "go": "MsgTypeVoiceLeave", "ts": "VOICE_LEAVE" }, + { "wire": "voice_mute", "go": "MsgTypeVoiceMute", "ts": "VOICE_MUTE" }, + { "wire": "voice_deafen", "go": "MsgTypeVoiceDeafen", "ts": "VOICE_DEAFEN" }, + { "wire": "voice_camera", "go": "MsgTypeVoiceCamera", "ts": "VOICE_CAMERA" }, + { "wire": "voice_screenshare", "go": "MsgTypeVoiceScreenshare", "ts": "VOICE_SCREENSHARE" }, + { "wire": "ping", "go": "MsgTypePing", "ts": "PING" }, + { + "wire": "voice_token_refresh", + "go": "MsgTypeVoiceTokenRefresh", + "ts": "VOICE_TOKEN_REFRESH", + "go_trailing_comment": "//nolint:gosec // G101: false positive — message type constant, not a credential" + }, + { "wire": "voice_e2ee_announce", "go": "MsgTypeVoiceE2EEAnnounce", "ts": "VOICE_E2EE_ANNOUNCE" }, + { "wire": "voice_e2ee_offer", "go": "MsgTypeVoiceE2EEOffer", "ts": "VOICE_E2EE_OFFER" } + ], + "server_to_client": [ + { "wire": "auth_ok", "go": "MsgTypeAuthOK", "ts": "AUTH_OK" }, + { "wire": "auth_error", "go": "MsgTypeAuthError", "ts": "AUTH_ERROR" }, + { "wire": "ready", "go": "MsgTypeReady", "ts": "READY" }, + { "wire": "chat_message", "go": "MsgTypeChatMessage", "ts": "CHAT_MESSAGE" }, + { "wire": "chat_send_ok", "go": "MsgTypeChatSendOK", "ts": "CHAT_SEND_OK" }, + { "wire": "chat_edited", "go": "MsgTypeChatEdited", "ts": "CHAT_EDITED" }, + { "wire": "chat_deleted", "go": "MsgTypeChatDeleted", "ts": "CHAT_DELETED" }, + { "wire": "reaction_update", "go": "MsgTypeReactionUpdate", "ts": "REACTION_UPDATE" }, + { "wire": "typing", "go": "MsgTypeTyping", "ts": "TYPING" }, + { "wire": "presence", "go": "MsgTypePresence", "ts": "PRESENCE" }, + { "wire": "channel_create", "go": "MsgTypeChannelCreate", "ts": "CHANNEL_CREATE" }, + { "wire": "channel_update", "go": "MsgTypeChannelUpdate", "ts": "CHANNEL_UPDATE" }, + { "wire": "channel_delete", "go": "MsgTypeChannelDelete", "ts": "CHANNEL_DELETE" }, + { "wire": "voice_state", "go": "MsgTypeVoiceState", "ts": "VOICE_STATE" }, + { "wire": "voice_config", "go": "MsgTypeVoiceConfig", "ts": "VOICE_CONFIG" }, + { "wire": "voice_token", "go": "MsgTypeVoiceToken", "ts": "VOICE_TOKEN" }, + { "wire": "voice_speakers", "go": "MsgTypeVoiceSpeakers", "ts": "VOICE_SPEAKERS" }, + { + "wire": "voice_leave", + "go": "MsgTypeVoiceLeaveBC", + "ts": "VOICE_LEAVE", + "note": "broadcast (same string as client msg)" + }, + { "wire": "member_join", "go": "MsgTypeMemberJoin", "ts": "MEMBER_JOIN" }, + { "wire": "member_leave", "go": "MsgTypeMemberLeave", "ts": "MEMBER_LEAVE" }, + { "wire": "member_update", "go": "MsgTypeMemberUpdate", "ts": "MEMBER_UPDATE" }, + { "wire": "user_update", "go": "MsgTypeUserUpdate", "ts": "USER_UPDATE" }, + { "wire": "member_ban", "go": "MsgTypeMemberBan", "ts": "MEMBER_BAN" }, + { "wire": "server_restart", "go": "MsgTypeServerRestart", "ts": "SERVER_RESTART" }, + { "wire": "error", "go": "MsgTypeError", "ts": "ERROR" }, + { "wire": "pong", "go": "MsgTypePong", "ts": "PONG" }, + { "wire": "dm_channel_open", "go": "MsgTypeDMChannelOpen", "ts": "DM_CHANNEL_OPEN" }, + { "wire": "dm_channel_close", "go": "MsgTypeDMChannelClose", "ts": "DM_CHANNEL_CLOSE" }, + { + "wire": "voice_e2ee_announce", + "go": "MsgTypeVoiceE2EEAnnounceBC", + "ts": "VOICE_E2EE_ANNOUNCE", + "note": "broadcast (same string as client msg)" + }, + { + "wire": "voice_e2ee_offer", + "go": "MsgTypeVoiceE2EEOfferRelay", + "ts": "VOICE_E2EE_OFFER", + "note": "relay (same string as client msg)" + } + ] +}