From e0437d4d8d4e4934aa7a5bf64bfd2066dcd089e8 Mon Sep 17 00:00:00 2001 From: jevb Date: Tue, 24 Mar 2026 21:30:23 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20LiveKit=20migration=20=E2=80=94=20permi?= =?UTF-8?q?ssions,=20auth=20hardening,=20voice=20improvements?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-review snapshot of LiveKit migration changes including: - Permission computation fix (allow-wins semantics) - Timing-safe password comparison with dummy hash - Rate limiter window fix - Dev credential clearing for LiveKit - Voice leave/join broadcast improvements - Migration transaction wrapping - Chat edit/delete permission guards - TOTP verification endpoint - Embed regex injection fix --- Client/tauri-client/src-tauri/Cargo.lock | 2 +- Client/tauri-client/src-tauri/src/commands.rs | 5 +- .../src/components/ChannelSidebar.ts | 12 +++-- .../src/components/MessageInput.ts | 9 +--- .../components/message-list/attachments.ts | 5 ++ .../src/components/message-list/embeds.ts | 10 +++- .../src/components/message-list/media.ts | 6 ++- Client/tauri-client/src/lib/api.ts | 47 ++++++++++++++----- Client/tauri-client/src/lib/dispatcher.ts | 4 ++ Client/tauri-client/src/lib/permissions.ts | 6 +-- Client/tauri-client/src/lib/ws.ts | 4 +- Client/tauri-client/src/pages/MainPage.ts | 10 ++-- .../tauri-client/src/stores/messages.store.ts | 4 +- .../tests/unit/permissions.test.ts | 4 +- Server/api/auth_handler.go | 25 +++++----- Server/api/auth_handler_test.go | 2 +- Server/api/invite_handler_test.go | 2 +- Server/api/router.go | 2 +- Server/auth/password.go | 13 ++++- Server/auth/ratelimit.go | 5 +- Server/config/config.go | 4 ++ Server/db/channel_queries.go | 10 +++- Server/db/migrate.go | 13 ++++- Server/ws/handlers.go | 37 +++++++++++++-- Server/ws/hub.go | 4 +- Server/ws/livekit_webhook.go | 3 ++ Server/ws/voice_leave.go | 4 +- 27 files changed, 188 insertions(+), 64 deletions(-) diff --git a/Client/tauri-client/src-tauri/Cargo.lock b/Client/tauri-client/src-tauri/Cargo.lock index 5dbd52e8..48f7d29d 100644 --- a/Client/tauri-client/src-tauri/Cargo.lock +++ b/Client/tauri-client/src-tauri/Cargo.lock @@ -2594,7 +2594,7 @@ dependencies = [ [[package]] name = "owncord-client" -version = "1.2.0" +version = "1.3.0" dependencies = [ "futures-util", "ring", diff --git a/Client/tauri-client/src-tauri/src/commands.rs b/Client/tauri-client/src-tauri/src/commands.rs index 67634e51..6c8634cc 100644 --- a/Client/tauri-client/src-tauri/src/commands.rs +++ b/Client/tauri-client/src-tauri/src/commands.rs @@ -75,6 +75,9 @@ pub fn store_cert_fingerprint( host: String, fingerprint: String, ) -> Result<(), String> { + // Normalize to lowercase for consistent comparison with ws_proxy fingerprints + let fingerprint = fingerprint.to_lowercase(); + if host.is_empty() { return Err("host must not be empty".into()); } @@ -82,7 +85,7 @@ pub fn store_cert_fingerprint( return Err("fingerprint must not be empty".into()); } - // Validate SHA-256 colon-hex format: "AA:BB:CC:..." (95 chars, 32 hex pairs) + // Validate SHA-256 colon-hex format: "aa:bb:cc:..." (95 chars, 32 hex pairs) if fingerprint.len() != 95 { return Err("fingerprint must be a SHA-256 colon-hex string (95 chars)".into()); } diff --git a/Client/tauri-client/src/components/ChannelSidebar.ts b/Client/tauri-client/src/components/ChannelSidebar.ts index 98e1bcbc..49a4be63 100644 --- a/Client/tauri-client/src/components/ChannelSidebar.ts +++ b/Client/tauri-client/src/components/ChannelSidebar.ts @@ -386,13 +386,13 @@ function attachChannelContextMenu( } /** Global mousemove/mouseup handlers for drag reordering. Registered once. */ -let globalDragListenersAttached = false; +let globalDragAc: AbortController | null = null; function ensureGlobalDragListeners(): void { - if (globalDragListenersAttached) { + if (globalDragAc !== null) { return; } - globalDragListenersAttached = true; + globalDragAc = new AbortController(); document.addEventListener("mousemove", (e) => { if (activeDrag === null) { @@ -415,7 +415,7 @@ function ensureGlobalDragListeners(): void { break; } } - }); + }, { signal: globalDragAc.signal }); document.addEventListener("mouseup", (e) => { if (activeDrag === null) { @@ -480,7 +480,7 @@ function ensureGlobalDragListeners(): void { if (reorders.length > 0) { drag.onReorder(reorders); } - }); + }, { signal: globalDragAc.signal }); } /** Make a channel element draggable via mousedown (admin/owner only). */ @@ -785,6 +785,8 @@ export function createChannelSidebar(options: ChannelSidebarOptions): MountableC function destroy(): void { ac.abort(); + globalDragAc?.abort(); + globalDragAc = null; for (const unsub of unsubscribers) { unsub(); } diff --git a/Client/tauri-client/src/components/MessageInput.ts b/Client/tauri-client/src/components/MessageInput.ts index f5a5a578..51042628 100644 --- a/Client/tauri-client/src/components/MessageInput.ts +++ b/Client/tauri-client/src/components/MessageInput.ts @@ -445,13 +445,8 @@ export function createMessageInput( function destroy(): void { ac.abort(); - // Revoke any blob URLs for image previews - for (const att of pendingAttachments) { - const img = att.previewEl.querySelector("img"); - if (img !== null && img.src.startsWith("blob:")) { - URL.revokeObjectURL(img.src); - } - } + // Image previews now use data: URLs (via readFileAsDataUrl) which don't + // require revocation — just clear the array and let GC reclaim them. pendingAttachments.length = 0; root?.remove(); root = null; diff --git a/Client/tauri-client/src/components/message-list/attachments.ts b/Client/tauri-client/src/components/message-list/attachments.ts index 0bd79a7c..35ecd8f8 100644 --- a/Client/tauri-client/src/components/message-list/attachments.ts +++ b/Client/tauri-client/src/components/message-list/attachments.ts @@ -153,6 +153,11 @@ export function fetchImageAsDataUrl(url: string): Promise { } // 4. Network fetch via Tauri HTTP plugin + // acceptInvalidCerts is required for self-hosted OwnCord servers with self-signed + // TLS certificates. This means the client will accept any certificate from any server + // for image fetching, which could enable SSRF to internal endpoints via malicious + // chat messages containing internal URLs. Mitigated by: (1) isSafeUrl only allows + // http/https, (2) responses are only used as image data, not executed. try { const res = await tauriFetch(url, { danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false }, diff --git a/Client/tauri-client/src/components/message-list/embeds.ts b/Client/tauri-client/src/components/message-list/embeds.ts index 1617113b..72d59fc4 100644 --- a/Client/tauri-client/src/components/message-list/embeds.ts +++ b/Client/tauri-client/src/components/message-list/embeds.ts @@ -33,13 +33,19 @@ const ogInFlight = new Set(); // -- OG tag parsing ----------------------------------------------------------- +/** Escape special regex characters in a string for safe use in `new RegExp()`. */ +function escapeRegex(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + /** Extract Open Graph meta tags from raw HTML using regex (no DOM parser needed). */ export function parseOgTags(html: string): OgMeta { function getMetaContent(property: string): string | null { // Match both property="og:X" and name="og:X" patterns + const escaped = escapeRegex(property); const regex = new RegExp( - `]*(?:property|name)=["']${property}["'][^>]*content=["']([^"']*)["']` + - `|]*content=["']([^"']*)["'][^>]*(?:property|name)=["']${property}["']`, + `]*(?:property|name)=["']${escaped}["'][^>]*content=["']([^"']*)["']` + + `|]*content=["']([^"']*)["'][^>]*(?:property|name)=["']${escaped}["']`, "i", ); const match = html.match(regex); diff --git a/Client/tauri-client/src/components/message-list/media.ts b/Client/tauri-client/src/components/message-list/media.ts index 248a92bf..f7751059 100644 --- a/Client/tauri-client/src/components/message-list/media.ts +++ b/Client/tauri-client/src/components/message-list/media.ts @@ -12,6 +12,7 @@ import { createIcon } from "@lib/icons"; import { createLogger } from "@lib/logger"; import { observeMedia } from "@lib/media-visibility"; import { loadPref } from "@components/settings/helpers"; +import { fetch as tauriFetch } from "@tauri-apps/plugin-http"; import { isSafeUrl } from "./attachments"; import { CODE_BLOCK_REGEX, INLINE_CODE_REGEX, URL_REGEX } from "./content-parser"; import { renderGenericLinkPreview } from "./embeds"; @@ -120,7 +121,10 @@ export function renderYouTubeEmbed(videoId: string, originalUrl: string): HTMLDi } else { setText(titleLink, "Loading..."); const oembedUrl = `https://www.youtube.com/oembed?url=https://www.youtube.com/watch?v=${encodeURIComponent(videoId)}&format=json`; - fetch(oembedUrl, { signal: AbortSignal.timeout(5000) }) + tauriFetch(oembedUrl, { + signal: AbortSignal.timeout(5000), + danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false }, + } as RequestInit) .then((res) => (res.ok ? (res.json() as Promise<{ title?: string } | null>) : null)) .then((data) => { const title = data?.title ?? "YouTube Video"; diff --git a/Client/tauri-client/src/lib/api.ts b/Client/tauri-client/src/lib/api.ts index 049a8257..a2840d92 100644 --- a/Client/tauri-client/src/lib/api.ts +++ b/Client/tauri-client/src/lib/api.ts @@ -232,22 +232,47 @@ export function createApiClient( return request("POST", "/auth/logout", undefined, signal); }, - verifyTotp( + async verifyTotp( code: string, partialToken: string, signal?: AbortSignal, ): Promise { - // Temporarily set token for this request; restore in .finally() - const prevToken = config.token; - config = { ...config, token: partialToken }; - return request( - "POST", - "/auth/verify-totp", - { code }, + // Don't mutate shared config — make direct fetch with the partial token + const url = `${baseUrl()}/auth/verify-totp`; + const init: RequestInit & { danger?: { acceptInvalidCerts: boolean; acceptInvalidHostnames: boolean } } = { + method: "POST", + headers: { + "Content-Type": "application/json", + "Authorization": `Bearer ${partialToken}`, + }, + body: JSON.stringify({ code }), signal, - ).finally(() => { - config = { ...config, token: prevToken }; - }); + danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false }, + }; + + let res: Response; + try { + res = await fetch(url, init as RequestInit); + } catch (fetchErr) { + log.error("API fetch failed", { method: "POST", path: "/auth/verify-totp", error: String(fetchErr) }); + if (fetchErr instanceof Error) { + throw fetchErr; + } + throw new Error(typeof fetchErr === "string" ? fetchErr : String(fetchErr)); + } + + if (res.status === 401) { + onUnauthorized?.(); + const err = await parseError(res); + throw new ApiClientError(401, err.error, err.message); + } + + if (!res.ok) { + const err = await parseError(res); + throw new ApiClientError(res.status, err.error, err.message); + } + + return res.json() as Promise; }, // ── Users ───────────────────────────────────────────── diff --git a/Client/tauri-client/src/lib/dispatcher.ts b/Client/tauri-client/src/lib/dispatcher.ts index a2e1ab75..ef38c442 100644 --- a/Client/tauri-client/src/lib/dispatcher.ts +++ b/Client/tauri-client/src/lib/dispatcher.ts @@ -278,6 +278,7 @@ export function wireDispatcher(ws: WsClient): DispatcherCleanup { reason: payload.reason, delaySeconds: payload.delay_seconds, }); + setTransientError(`Server is restarting: ${payload.reason ?? "maintenance"}`); }), ); @@ -287,6 +288,9 @@ export function wireDispatcher(ws: WsClient): DispatcherCleanup { code: payload.code, message: payload.message, }); + if (payload.code === "RATE_LIMITED" || payload.code === "FORBIDDEN") { + setTransientError(payload.message || "Server error"); + } }), ); diff --git a/Client/tauri-client/src/lib/permissions.ts b/Client/tauri-client/src/lib/permissions.ts index cb01da59..d23cda70 100644 --- a/Client/tauri-client/src/lib/permissions.ts +++ b/Client/tauri-client/src/lib/permissions.ts @@ -41,14 +41,14 @@ export function hasAllPermissions(userPerms: number, ...perms: Permission[]): bo * * - If the base permissions contain ADMINISTRATOR the result is all bits set * (deny/allow are ignored). - * - Otherwise: start with `basePerms`, add `allow` bits, then remove `deny` bits. - * Deny takes precedence over allow. + * - Otherwise: remove `deny` bits first, then add `allow` bits. + * Allow takes precedence over deny (matches server semantics). */ export function computeEffective(basePerms: number, allow: number, deny: number): number { if ((basePerms & Permission.ADMINISTRATOR) === Permission.ADMINISTRATOR) { return ALL_PERMISSIONS; } - return (basePerms | allow) & ~deny; + return (basePerms & ~deny) | allow; } /** Shorthand check for the ADMINISTRATOR bit. */ diff --git a/Client/tauri-client/src/lib/ws.ts b/Client/tauri-client/src/lib/ws.ts index 15cd1f1c..1ced2733 100644 --- a/Client/tauri-client/src/lib/ws.ts +++ b/Client/tauri-client/src/lib/ws.ts @@ -365,12 +365,14 @@ export function createWsClient() { function disconnect(): void { intentionalClose = true; certMismatchBlock = false; - lastSeq = 0; cancelReconnect(); stopHeartbeat(); cleanupEventListeners(); void disconnectProxy(); setState("disconnected"); + // Only reset lastSeq on intentional disconnect (e.g. logout) + // so reconnect scenarios preserve replay ability. + lastSeq = 0; } return { diff --git a/Client/tauri-client/src/pages/MainPage.ts b/Client/tauri-client/src/pages/MainPage.ts index bf1d5c1e..d8766a76 100644 --- a/Client/tauri-client/src/pages/MainPage.ts +++ b/Client/tauri-client/src/pages/MainPage.ts @@ -332,9 +332,13 @@ export function createMainPage(options: MainPageOptions): MountableComponent { const unsubChannels = channelsStore.subscribeSelector( (s) => s.activeChannelId, () => { - const active = getActiveChannel(); - if (active !== null) { - channelCtrl!.mountChannel(active.id, active.name); + try { + const active = getActiveChannel(); + if (active !== null) { + channelCtrl!.mountChannel(active.id, active.name); + } + } catch (err) { + log.error("Channel mount failed", err); } }, ); diff --git a/Client/tauri-client/src/stores/messages.store.ts b/Client/tauri-client/src/stores/messages.store.ts index 393413d9..099afd38 100644 --- a/Client/tauri-client/src/stores/messages.store.ts +++ b/Client/tauri-client/src/stores/messages.store.ts @@ -168,9 +168,9 @@ export function prependMessages( messagesStore.setState((prev) => { const existing = prev.messagesByChannel.get(channelId) ?? []; let combined = [...converted, ...existing]; - // Keep only the newest messages if combined exceeds the cap + // Keep oldest messages (start of array) since we're loading history if (combined.length > MAX_MESSAGES_PER_CHANNEL) { - combined = combined.slice(combined.length - MAX_MESSAGES_PER_CHANNEL); + combined = combined.slice(0, MAX_MESSAGES_PER_CHANNEL); } const updatedMessages = new Map(prev.messagesByChannel); updatedMessages.set(channelId, combined); diff --git a/Client/tauri-client/tests/unit/permissions.test.ts b/Client/tauri-client/tests/unit/permissions.test.ts index 80c3c49e..d5e2806a 100644 --- a/Client/tauri-client/tests/unit/permissions.test.ts +++ b/Client/tauri-client/tests/unit/permissions.test.ts @@ -82,12 +82,12 @@ describe('hasAllPermissions', () => { }); describe('computeEffective', () => { - it('deny overrides allow', () => { + it('allow overrides deny (allow-wins, matches server semantics)', () => { const base = MEMBER_PERMS; const allow = Permission.MANAGE_MESSAGES; const deny = Permission.MANAGE_MESSAGES; const effective = computeEffective(base, allow, deny); - expect(effective & Permission.MANAGE_MESSAGES).toBe(0); + expect(effective & Permission.MANAGE_MESSAGES).toBe(Permission.MANAGE_MESSAGES); }); it('ADMINISTRATOR ignores deny and returns all bits', () => { diff --git a/Server/api/auth_handler.go b/Server/api/auth_handler.go index 38cb684e..71cdc7b6 100644 --- a/Server/api/auth_handler.go +++ b/Server/api/auth_handler.go @@ -54,16 +54,18 @@ type authSuccessResponse struct { } // MountAuthRoutes registers all auth endpoints on the given router. -// Rate limiters are applied per-endpoint as specified. -func MountAuthRoutes(r chi.Router, database *db.DB, limiter *auth.RateLimiter) { +// Rate limiters are applied per-endpoint as specified. trustedProxies is the +// list of CIDRs whose X-Forwarded-For / X-Real-IP headers are honoured for +// rate-limiting IP resolution. +func MountAuthRoutes(r chi.Router, database *db.DB, limiter *auth.RateLimiter, trustedProxies []string) { registerLimiter := limiter loginLimiter := limiter r.Route("/api/v1/auth", func(r chi.Router) { - r.With(RateLimitMiddleware(registerLimiter, 3, time.Minute)). + r.With(RateLimitMiddleware(registerLimiter, 3, time.Minute, trustedProxies)). Post("/register", handleRegister(database)) - r.With(RateLimitMiddleware(loginLimiter, 5, time.Minute)). + r.With(RateLimitMiddleware(loginLimiter, 5, time.Minute, trustedProxies)). Post("/login", handleLogin(database, limiter)) r.With(AuthMiddleware(database)). @@ -106,13 +108,8 @@ func handleRegister(database *db.DB) http.HandlerFunc { return } - // Validate and consume invite atomically to prevent TOCTOU races. - if err := database.UseInviteAtomic(req.InviteCode); err != nil { - writeJSON(w, http.StatusBadRequest, genericAuthError) - return - } - - // Hash password. + // Hash password before consuming the invite so that a hashing failure + // does not burn a valid invite code. hash, err := auth.HashPassword(req.Password) if err != nil { writeJSON(w, http.StatusInternalServerError, errorResponse{ @@ -122,6 +119,12 @@ func handleRegister(database *db.DB) http.HandlerFunc { return } + // Validate and consume invite atomically to prevent TOCTOU races. + if err := database.UseInviteAtomic(req.InviteCode); err != nil { + writeJSON(w, http.StatusBadRequest, genericAuthError) + return + } + // Create user with default Member role. uid, err := database.CreateUser(req.Username, hash, int(permissions.MemberRoleID)) if err != nil { diff --git a/Server/api/auth_handler_test.go b/Server/api/auth_handler_test.go index 75beb1c9..f9ef17fc 100644 --- a/Server/api/auth_handler_test.go +++ b/Server/api/auth_handler_test.go @@ -36,7 +36,7 @@ func newAuthTestDB(t *testing.T) *db.DB { // buildAuthRouter returns a chi router with auth routes mounted on /api/v1/auth. func buildAuthRouter(database *db.DB, limiter *auth.RateLimiter) http.Handler { r := chi.NewRouter() - api.MountAuthRoutes(r, database, limiter) + api.MountAuthRoutes(r, database, limiter, nil) return r } diff --git a/Server/api/invite_handler_test.go b/Server/api/invite_handler_test.go index bf8c7452..e173eba3 100644 --- a/Server/api/invite_handler_test.go +++ b/Server/api/invite_handler_test.go @@ -15,7 +15,7 @@ import ( // buildInviteRouter returns a chi router with invite routes and auth middleware. func buildInviteRouter(database *db.DB, limiter *auth.RateLimiter) http.Handler { r := chi.NewRouter() - api.MountAuthRoutes(r, database, limiter) + api.MountAuthRoutes(r, database, limiter, nil) api.MountInviteRoutes(r, database) return r } diff --git a/Server/api/router.go b/Server/api/router.go index ec361d1e..5315c8ea 100644 --- a/Server/api/router.go +++ b/Server/api/router.go @@ -47,7 +47,7 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri }) // Auth routes: register, login, logout, me. - MountAuthRoutes(r, database, limiter) + MountAuthRoutes(r, database, limiter, cfg.Server.TrustedProxies) // Invite management routes (require MANAGE_INVITES permission). MountInviteRoutes(r, database) diff --git a/Server/auth/password.go b/Server/auth/password.go index f1cffb07..5ca533ce 100644 --- a/Server/auth/password.go +++ b/Server/auth/password.go @@ -27,10 +27,21 @@ func HashPassword(password string) (string, error) { return string(hash), nil } +// dummyHash is a pre-computed bcrypt hash used to prevent timing side-channels +// when the user does not exist. Comparing against this dummy ensures that +// CheckPassword takes roughly constant time regardless of whether a valid hash +// was supplied. +var dummyHash, _ = bcrypt.GenerateFromPassword([]byte("dummy-timing-pad"), bcryptCost) + // CheckPassword reports whether password matches hash. Returns false on any -// error, including an empty or malformed hash. +// error, including an empty or malformed hash. When hash is empty (user does +// not exist), a dummy bcrypt comparison is performed to prevent timing-based +// username enumeration. func CheckPassword(hash, password string) bool { if hash == "" { + // Perform a dummy comparison so the response time is indistinguishable + // from a real check, preventing timing-based username enumeration. + bcrypt.CompareHashAndPassword(dummyHash, []byte(password)) //nolint:errcheck return false } err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password)) diff --git a/Server/auth/ratelimit.go b/Server/auth/ratelimit.go index 7dbf48cd..f569cbd4 100644 --- a/Server/auth/ratelimit.go +++ b/Server/auth/ratelimit.go @@ -32,8 +32,9 @@ func NewRateLimiter() *RateLimiter { } // Allow reports whether a request from key is permitted given the limit and -// window. It records the current request timestamp regardless of the outcome. -// Returns false when key is locked out or has exceeded limit within window. +// window. It records the current request timestamp only when the request is +// permitted. Returns false when key is locked out or has exceeded limit within +// window. func (r *RateLimiter) Allow(key string, limit int, window time.Duration) bool { r.mu.Lock() defer r.mu.Unlock() diff --git a/Server/config/config.go b/Server/config/config.go index 61f1e374..a8e11f72 100644 --- a/Server/config/config.go +++ b/Server/config/config.go @@ -209,8 +209,12 @@ func Load(cfgPath string) (*Config, error) { applyVoiceDefaults(&cfg.Voice) // Warn if using default dev credentials — these are public and insecure. + // Clear credentials so downstream consumers (e.g. NewLiveKitClient) see + // empty values and refuse to start voice. if IsDefaultVoiceCredentials(&cfg.Voice) { slog.Warn("using default LiveKit dev credentials — voice will be disabled; set voice.livekit_api_key and voice.livekit_api_secret in config.yaml") + cfg.Voice.LiveKitAPIKey = "" + cfg.Voice.LiveKitAPISecret = "" } return &cfg, nil diff --git a/Server/db/channel_queries.go b/Server/db/channel_queries.go index 99a31cda..f4544fb3 100644 --- a/Server/db/channel_queries.go +++ b/Server/db/channel_queries.go @@ -10,7 +10,11 @@ import ( func (d *DB) ListChannels() ([]Channel, error) { rows, err := d.sqlDB.Query( `SELECT id, name, type, COALESCE(category,''), COALESCE(topic,''), - position, slow_mode, archived, created_at + position, slow_mode, archived, created_at, + COALESCE(voice_max_users, 0), + voice_quality, + mixing_threshold, + COALESCE(voice_max_video, 0) FROM channels ORDER BY position ASC, id ASC`, ) if err != nil { @@ -172,12 +176,16 @@ func (d *DB) GetAllChannelPermissionsForRole(roleID int64) (map[int64]ChannelOve // ─── helpers ────────────────────────────────────────────────────────────────── // scanChannel scans a single channel row from *sql.Rows. +// The query must select the 13 columns: id, name, type, category, topic, +// position, slow_mode, archived, created_at, voice_max_users, +// voice_quality, mixing_threshold, voice_max_video. func scanChannel(rows *sql.Rows) (Channel, error) { var ch Channel var archived int err := rows.Scan( &ch.ID, &ch.Name, &ch.Type, &ch.Category, &ch.Topic, &ch.Position, &ch.SlowMode, &archived, &ch.CreatedAt, + &ch.VoiceMaxUsers, &ch.VoiceQuality, &ch.MixingThreshold, &ch.VoiceMaxVideo, ) if err != nil { return Channel{}, err diff --git a/Server/db/migrate.go b/Server/db/migrate.go index f1f147d8..c66b6228 100644 --- a/Server/db/migrate.go +++ b/Server/db/migrate.go @@ -168,15 +168,26 @@ func MigrateFS(database *DB, fsys fs.FS) error { continue } + tx, txErr := database.sqlDB.Begin() + if txErr != nil { + return fmt.Errorf("begin tx for %s: %w", name, txErr) + } + raw, readErr := fs.ReadFile(fsys, name) if readErr != nil { + tx.Rollback() //nolint:errcheck return fmt.Errorf("reading migration %s: %w", name, readErr) } - if _, execErr := database.sqlDB.Exec(string(raw)); execErr != nil { + if _, execErr := tx.Exec(string(raw)); execErr != nil { + tx.Rollback() //nolint:errcheck return fmt.Errorf("executing migration %s: %w", name, execErr) } + if commitErr := tx.Commit(); commitErr != nil { + return fmt.Errorf("commit migration %s: %w", name, commitErr) + } + if err := recordApplied(database, name); err != nil { return err } diff --git a/Server/ws/handlers.go b/Server/ws/handlers.go index fe516d23..63dc11f4 100644 --- a/Server/ws/handlers.go +++ b/Server/ws/handlers.go @@ -24,6 +24,9 @@ const ( reactionWindow = time.Second ) +// maxMessageLen is the maximum allowed message length in runes (Unicode code points). +const maxMessageLen = 4000 + var sanitizer = bluemonday.StrictPolicy() // HandleMessageForTest dispatches a raw WebSocket message from client c. @@ -191,7 +194,7 @@ func (h *Hub) handleChatSend(c *Client, reqID string, payload json.RawMessage) { c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "message content cannot be empty")) return } - if len([]rune(content)) > 4000 { + if len([]rune(content)) > maxMessageLen { c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "message content exceeds maximum length of 4000 characters")) return } @@ -294,6 +297,23 @@ func (h *Hub) handleChatEdit(c *Client, _ string, payload json.RawMessage) { c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "content cannot be empty")) return } + if len([]rune(content)) > maxMessageLen { + c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "message too long")) + return + } + + // Fetch message first to get the channel ID for the permission check. + msg, err := h.db.GetMessage(msgID) + if err != nil || msg == nil { + c.sendMsg(buildErrorMsg(ErrCodeNotFound, "message not found")) + return + } + + // Re-check that the user still has SendMessages permission on this channel. + if !h.hasChannelPerm(c, msg.ChannelID, permissions.SendMessages) { + c.sendMsg(buildErrorMsg(ErrCodeForbidden, "no send permission in this channel")) + return + } // EditMessage checks ownership internally. if err := h.db.EditMessage(msgID, c.userID, content); err != nil { @@ -301,7 +321,8 @@ func (h *Hub) handleChatEdit(c *Client, _ string, payload json.RawMessage) { return } - msg, err := h.db.GetMessage(msgID) + // Re-fetch to get the updated edited_at timestamp. + msg, err = h.db.GetMessage(msgID) if err != nil || msg == nil { slog.Error("ws handleChatEdit GetMessage after edit", "err", err, "msg_id", msgID) c.sendMsg(buildErrorMsg(ErrCodeInternal, "edit saved but broadcast failed")) @@ -343,6 +364,12 @@ func (h *Hub) handleChatDelete(c *Client, _ string, payload json.RawMessage) { return } + // Ensure the user still has at least ReadMessages on this channel. + if !h.hasChannelPerm(c, msg.ChannelID, permissions.ReadMessages) { + c.sendMsg(buildErrorMsg(ErrCodeForbidden, "no read permission in this channel")) + return + } + isMod := h.hasChannelPerm(c, msg.ChannelID, permissions.ManageMessages) if err := h.db.DeleteMessage(msgID, c.userID, isMod); err != nil { c.sendMsg(buildErrorMsg(ErrCodeForbidden, "cannot delete this message")) @@ -507,7 +534,11 @@ func (h *Hub) requireChannelPerm(c *Client, channelID int64, perm int64, permLab return false } -// broadcastExclude sends msg to all channel members except excludeUserID. +// broadcastExclude sends a message to all clients in the sender's channel +// EXCEPT the sender. Unlike hub.BroadcastToChannel, messages sent via this +// function are NOT stored in the replay ring buffer — they are ephemeral. +// This is correct for typing indicators but would be incorrect for messages +// that should survive reconnection replay. func (h *Hub) broadcastExclude(channelID, excludeUserID int64, msg []byte) { h.mu.RLock() defer h.mu.RUnlock() diff --git a/Server/ws/hub.go b/Server/ws/hub.go index 02297b88..58591166 100644 --- a/Server/ws/hub.go +++ b/Server/ws/hub.go @@ -137,12 +137,12 @@ func (h *Hub) Run() { defer func() { if r := recover(); r != nil { - panicCount++ now := time.Now() if lastPanicReset.IsZero() || now.Sub(lastPanicReset) > 60*time.Second { - panicCount = 1 + panicCount = 0 lastPanicReset = now } + panicCount++ buf := make([]byte, 4096) n := runtime.Stack(buf, false) diff --git a/Server/ws/livekit_webhook.go b/Server/ws/livekit_webhook.go index e097661c..4d8d8ebd 100644 --- a/Server/ws/livekit_webhook.go +++ b/Server/ws/livekit_webhook.go @@ -53,6 +53,9 @@ func (h *Hub) NewLiveKitWebhookHandler(apiKey, apiSecret string) http.HandlerFun return } + // Verify checks both the HMAC signature and the exp/nbf claims + // (via jwt.Claims.Validate with Time: time.Now() inside the SDK). + // Expired tokens are rejected with an error here. if _, _, err := verifier.Verify(apiSecret); err != nil { slog.Warn("livekit webhook: token verification failed", "error", err) http.Error(w, "unauthorized", http.StatusUnauthorized) diff --git a/Server/ws/voice_leave.go b/Server/ws/voice_leave.go index b5931ef5..80b09ad4 100644 --- a/Server/ws/voice_leave.go +++ b/Server/ws/voice_leave.go @@ -18,8 +18,10 @@ func (h *Hub) handleVoiceLeave(c *Client) { if leaveErr := h.db.LeaveVoiceChannel(c.userID); leaveErr != nil { slog.Error("ws handleVoiceLeave LeaveVoiceChannel — ghost session may remain in DB", "err", leaveErr, "user_id", c.userID, "channel_id", oldChID) - c.sendMsg(buildErrorMsg(ErrCodeInternal, "voice leave partially failed — please rejoin if issues persist")) + c.sendMsg(buildErrorMsg(ErrCodeInternal, "voice leave failed — please rejoin if issues persist")) + return } + h.BroadcastToAll(buildVoiceLeave(oldChID, c.userID)) // Remove from LiveKit (best-effort).