fix: batch of 34 correctness fixes across server and client (#1372)

* fix(client): 3 defect(s) (OC-0037, OC-0063, OC-0116)

Route the tray Status submenu through saveUserStatus() (mapping the legacy
"offline" to "invisible") so notifications, autoIdle, and reconnect presence
restore all agree with the tray's choice; build the connected overlay from
the auth_ok payload instead of a pre-dispatch authStore snapshot; keep the
TOTP overlay open across a rejected verify (totpPending latch) and retain
the partial token for the retry instead of clearing it in finally.

Hand-applied combined cluster preserved from the previous fix run's
overlap-guard block (both clusters edit main.ts).

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

* fix(voice): 2 defect(s) (OC-0010, OC-0011)

* fix(ws): 1 defect(s) (OC-0050)

* fix(db): 1 defect(s) (OC-0052)

* fix(client): 1 defect(s) (OC-0054)

* fix(client): 1 defect(s) (OC-0059)

* fix(auth): 1 defect(s) (OC-0061)

* fix(ws): 1 defect(s) (OC-0062)

* fix(client): 1 defect(s) (OC-0064)

* fix(service): 1 defect(s) (OC-0070)

* fix(ws): 1 defect(s) (OC-0073)

* fix(service): 2 defect(s) (OC-0075, OC-0120)

* fix(admin): 1 defect(s) (OC-0076)

* fix(voice): 1 defect(s) (OC-0084)

* fix(client): 2 defect(s) (OC-0085, OC-0094)

Scope collapsed-category persistence to the connected host instead of the
server display name, and stop the DM back button from jumping to the first
text channel when DM mode was entered without recording channelBeforeDm.

* fix(service): 1 defect(s) (OC-0087)

* fix(client): 1 defect(s) (OC-0089)

* fix(ws): 1 defect(s) (OC-0091)

* fix(api): 1 defect(s) (OC-0093)

* fix(identity): 1 defect(s) (OC-0118)

* fix(dm): 1 defect(s) (OC-0119)

* fix(voice): 1 defect(s) (OC-0135)

* fix(api): 1 defect(s) (OC-0137)

* fix(client): 1 defect(s) (OC-0142)

* fix(client): 1 defect(s) (OC-0144)

* fix(admin): 1 defect(s) (OC-0145)

* fix(updater): 1 defect(s) (OC-0146)

* fix(client): 1 defect(s) (OC-0150)

* fix(mentions): 1 defect(s) (OC-0131)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Server/admin/api.go

@@ -2,6 +2,7 @@ package admin
 
 import (
 	"net/http"
+	"time"
 
 	"github.com/go-chi/chi/v5"
 	"github.com/owncord/server/auth"
@@ -11,6 +12,48 @@ import (
 	"github.com/owncord/server/updater"
 )
 
+// setupLimiterReapInterval and setupLimiterReapMaxWindow control how often
+// the setup endpoint's dedicated rate limiter reaps stale window entries.
+// Vars, not consts, so tests can shrink them instead of waiting on the real
+// interval (see export_test.go).
+var (
+	setupLimiterReapInterval  = 5 * time.Minute
+	setupLimiterReapMaxWindow = 15 * time.Minute
+)
+
+// setupLimiterHook, when non-nil, receives the *auth.RateLimiter NewAdminAPI
+// creates for the /setup endpoint. Test-only seam: NewAdminAPI returns only
+// an http.Handler, so tests otherwise have no way to reach that limiter to
+// verify it gets reaped.
+var setupLimiterHook func(*auth.RateLimiter)
+
+// startSetupLimiterReap keeps rl's window map bounded for the life of the
+// process. Every distinct source IP that ever hits POST /setup leaves an
+// entry that Allow itself only prunes on a repeat call from that same key —
+// a one-shot caller's entry sits forever unless something sweeps the whole
+// map. api/router.go reaps its own limiter with RateLimiter.StartCleanup, a
+// goroutine parked in a ticker select until a stop channel closes — but
+// NewAdminAPI has no shutdown hook and is called directly by ~180 tests that
+// never capture one, so a parked goroutine here would leak under every
+// test's goleak check. time.AfterFunc self-rescheduling avoids that: between
+// fires there is no live goroutine, only a runtime timer, so nothing needs
+// to stop it.
+func startSetupLimiterReap(rl *auth.RateLimiter) {
+	// Capture the timing once, synchronously, on the caller's goroutine.
+	// The rescheduled AfterFunc callbacks below must never re-read the
+	// package vars themselves: those callbacks run on their own goroutine
+	// indefinitely (nothing stops the chain), so a later test's
+	// SetSetupLimiterReapTiming restoring the vars on its own goroutine
+	// would otherwise race an in-flight reap here.
+	interval, maxWindow := setupLimiterReapInterval, setupLimiterReapMaxWindow
+	var reap func()
+	reap = func() {
+		rl.Cleanup(maxWindow)
+		time.AfterFunc(interval, reap)
+	}
+	time.AfterFunc(interval, reap)
+}
+
 // ─── NewAdminAPI ──────────────────────────────────────────────────────────────
 
 // NewAdminAPI returns a chi router with all /admin/api/* routes. All routes
@@ -32,6 +75,10 @@ func NewAdminAPI(database *db.DB, version string, hub HubBroadcaster, u *updater
 
 	// Setup endpoints — unauthenticated, only functional when no users exist.
 	setupLimiter := auth.NewRateLimiter()
+	if setupLimiterHook != nil {
+		setupLimiterHook(setupLimiter)
+	}
+	startSetupLimiterReap(setupLimiter)
 	r.Get("/setup/status", handleSetupStatus(database, setupOpts))
 	r.Post("/setup", handleSetup(database, setupLimiter, allowedOrigins, hub, setupOpts))
 

Server/admin/api_test.go

@@ -1479,6 +1479,49 @@ func TestAdminAPI_CreateAPIToken_MissingLabel(t *testing.T) {
 	}
 }
 
+// TestAdminAPI_CreateAPIToken_NegativeExpiresHours pins OC-0145: a caller that
+// asks for a bounded credential (negative expires_hours) must not silently
+// receive a permanent one. The `> 0` check in handleCreateAPIToken sends any
+// negative value down the nil-expiresAt ("never expires") branch.
+func TestAdminAPI_CreateAPIToken_NegativeExpiresHours(t *testing.T) {
+	database := openAdminTestDB(t)
+	handler := admin.NewAdminAPI(database, "1.0.0", &mockHub{}, nil, nil, nil, nil, newTestModService(database), newTestRoleService(database))
+	token := createAdminUser(t, database)
+
+	w := doRequest(t, handler, http.MethodPost, "/tokens", token, map[string]any{"label": "neg-hours", "expires_hours": -1})
+	if w.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400; body: %s", w.Code, w.Body.String())
+	}
+
+	tokens, _ := database.ListAPITokens(context.Background())
+	for _, tok := range tokens {
+		if tok.Label == "neg-hours" {
+			t.Fatalf("negative expires_hours must not mint a token, got %+v", tok)
+		}
+	}
+}
+
+// TestAdminAPI_CreateAPIToken_HugeExpiresHours pins OC-0145's overflow half: a
+// huge expires_hours must not silently overflow time.Duration into a past
+// timestamp and hand back a token that 401s on first use.
+func TestAdminAPI_CreateAPIToken_HugeExpiresHours(t *testing.T) {
+	database := openAdminTestDB(t)
+	handler := admin.NewAdminAPI(database, "1.0.0", &mockHub{}, nil, nil, nil, nil, newTestModService(database), newTestRoleService(database))
+	token := createAdminUser(t, database)
+
+	w := doRequest(t, handler, http.MethodPost, "/tokens", token, map[string]any{"label": "huge-hours", "expires_hours": 3000000})
+	if w.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400; body: %s", w.Code, w.Body.String())
+	}
+
+	tokens, _ := database.ListAPITokens(context.Background())
+	for _, tok := range tokens {
+		if tok.Label == "huge-hours" {
+			t.Fatalf("out-of-range expires_hours must not mint a token, got %+v", tok)
+		}
+	}
+}
+
 func TestAdminAPI_ListAPITokens_OK(t *testing.T) {
 	database := openAdminTestDB(t)
 	handler := admin.NewAdminAPI(database, "1.0.0", &mockHub{}, nil, nil, nil, nil, newTestModService(database), newTestRoleService(database))

Server/admin/export_test.go

@@ -1,6 +1,34 @@
 package admin
 
-import "sync/atomic"
+import (
+	"sync/atomic"
+	"time"
+
+	"github.com/owncord/server/auth"
+)
+
+// CaptureSetupLimiter installs h so the next NewAdminAPI call reports the
+// *auth.RateLimiter it creates for the /setup endpoint. NewAdminAPI returns
+// only an http.Handler, so this is the only way tests can reach that limiter
+// to check whether its stale entries get reaped.
+func CaptureSetupLimiter(h func(*auth.RateLimiter)) (restore func()) {
+	prev := setupLimiterHook
+	setupLimiterHook = h
+	return func() { setupLimiterHook = prev }
+}
+
+// SetSetupLimiterReapTiming overrides the interval and max-window the setup
+// endpoint's rate-limiter reaper uses, so tests don't wait on the real
+// 5-minute interval.
+func SetSetupLimiterReapTiming(interval, maxWindow time.Duration) (restore func()) {
+	prevI, prevW := setupLimiterReapInterval, setupLimiterReapMaxWindow
+	setupLimiterReapInterval = interval
+	setupLimiterReapMaxWindow = maxWindow
+	return func() {
+		setupLimiterReapInterval = prevI
+		setupLimiterReapMaxWindow = prevW
+	}
+}
 
 // SetBackupBaseDir overrides backupBaseDir so tests can point backup handlers
 // at a temp dir. Lives here so it stays out of the production binary.

Server/admin/handlers_tokens.go

@@ -63,6 +63,14 @@ func handleCreateAPIToken(database *db.DB) http.HandlerFunc {
 			writeErr(w, http.StatusBadRequest, "BAD_REQUEST", "label is required")
 			return
 		}
+		// expires_hours=0 means "never expires" (see createTokenRequest doc).
+		// Negatives must not fall into that same nil-expiresAt branch, and the
+		// upper bound keeps time.Duration(hours)*time.Hour from overflowing
+		// int64 nanoseconds into a past timestamp. 87600h = 10 years.
+		if req.ExpiresHours < 0 || req.ExpiresHours > 24*365*10 {
+			writeErr(w, http.StatusBadRequest, "BAD_REQUEST", "expires_hours must be between 0 and 87600")
+			return
+		}
 
 		var user *db.User
 		var err error

Server/admin/setup_limiter_reap_test.go

@@ -0,0 +1,65 @@
+package admin_test
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/owncord/server/admin"
+	"github.com/owncord/server/auth"
+)
+
+// TestSetupLimiter_ReapsStaleEntries pins OC-0076: setupLimiter — the
+// dedicated auth.RateLimiter behind POST /setup — is never reaped, so a
+// distinct one-shot source IP (the common case once the server is already
+// configured: every unauthenticated caller 403s but still records a rate
+// limit entry before the CreateOwnerIfEmpty check rejects them) leaves a
+// windows[] entry that lives forever. Unlike a repeat caller, whose entry
+// self-prunes on its next Allow() call, a one-shot caller never revisits its
+// key, so only a periodic sweep (RateLimiter.Cleanup) can ever evict it.
+func TestSetupLimiter_ReapsStaleEntries(t *testing.T) {
+	restoreTiming := admin.SetSetupLimiterReapTiming(5*time.Millisecond, 5*time.Millisecond)
+	defer restoreTiming()
+
+	var limiter *auth.RateLimiter
+	restoreHook := admin.CaptureSetupLimiter(func(rl *auth.RateLimiter) { limiter = rl })
+	defer restoreHook()
+
+	database := openAdminTestDB(t)
+	handler := admin.NewAdminAPI(database, "1.0.0", &mockHub{}, nil, nil, nil, nil, newTestModService(database), newTestRoleService(database))
+
+	if limiter == nil {
+		t.Fatal("setup limiter was not captured — CaptureSetupLimiter hook not wired into NewAdminAPI")
+	}
+
+	// Simulate 20 distinct source IPs each making one POST /setup request —
+	// each leaves its own windows[] entry that nothing but a reap can evict.
+	const n = 20
+	for i := range n {
+		req := httptest.NewRequest(http.MethodPost, "/setup", strings.NewReader(`{}`))
+		req.RemoteAddr = fmt.Sprintf("203.0.113.%d:1234", i)
+		w := httptest.NewRecorder()
+		handler.ServeHTTP(w, req)
+	}
+
+	if wins, _ := limiter.Len(); wins != n {
+		t.Fatalf("Len().windows = %d immediately after %d one-shot requests, want %d", wins, n, n)
+	}
+
+	// Wait well past the (shrunk) reap interval + max window for the sweep
+	// to evict every now-stale entry.
+	deadline := time.Now().Add(2 * time.Second)
+	for {
+		wins, _ := limiter.Len()
+		if wins == 0 {
+			return
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("Len().windows = %d after waiting past the reap interval, want 0 — setupLimiter is never reaped (OC-0076)", wins)
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+}