fix(server): harden private-channel visibility propagation

Three review findings on the #93 feature:

- RefreshChannelVisibility targeted clients by their connect-time role
  snapshot; a user whose role changed mid-session was evaluated against the
  stale role. Resolve the current role from the DB per client (fail closed).
- Visibility updates are targeted, unsequenced messages, so a client that
  disconnected before an override change and later resumed via replay never
  converged (stale sidebar until a fresh connect). Track a visibility-change
  sequence watermark and force resumes from at/before it onto the
  full-ready path.
- The admin SPA interpolated channel/user names into single-quoted JS
  strings inside onclick attributes with HTML-escaping only; a name
  containing a quote broke out of the string literal (XSS in the admin
  panel, reachable by any user allowed to create channels). Add a jsq()
  helper (JS-escape then HTML-escape) and use it for every onclick name
  interpolation.

Follow-up to #93.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwtnpHAoSFr1ZibQgQkNQK

Server/admin/static/index.html

@@ -301,6 +301,10 @@ async function api(method,path,body){
 
 /* ═══ Utilities ═══ */
 function esc(s){if(s===null||s===undefined)return'';return String(s).replace(/&/g,'&amp;').replace(/</g,'&lt;').replace(/>/g,'&gt;').replace(/"/g,'&quot;')}
+/* Escape for embedding inside a single-quoted JS string in an inline onclick
+   attribute: JS-escape backslashes and single quotes first, then HTML-escape.
+   Without this a name containing ' breaks out of the string literal (XSS). */
+function jsq(s){return esc(String(s).replace(/\\/g,'\\\\').replace(/'/g,"\\'"))}
 function fmtBytes(b){if(b<1024)return b+' B';if(b<1048576)return(b/1024).toFixed(1)+' KB';if(b<1073741824)return(b/1048576).toFixed(1)+' MB';return(b/1073741824).toFixed(2)+' GB'}
 function actionBadge(a){if(!a)return'badge-muted';if(a.includes('ban')||a.includes('kick')||a.includes('delete'))return'badge-red';if(a.includes('create'))return'badge-green';if(a.includes('update'))return'badge-yellow';return'badge-accent'}
 function actionColor(a){if(!a)return'var(--accent)';if(a.includes('ban')||a.includes('kick')||a.includes('delete'))return'var(--red)';if(a.includes('create'))return'var(--green)';if(a.includes('update'))return'var(--yellow)';return'var(--accent)'}
@@ -467,10 +471,10 @@ async function renderUsers(){
     html+='<td><span class="dot '+statusDot+'"></span>'+statusLabel+'</td>';
     html+='<td>'+(banned?'<span class="badge badge-red">Yes</span>':'<span class="badge badge-muted">No</span>')+'</td>';
     html+='<td><div class="act-group" style="justify-content:flex-end">';
-    html+='<button class="act-btn" title="Edit role" onclick="openEditUser('+uid+',\''+esc(uname)+'\','+rid+')">'+I.edit+'</button>';
+    html+='<button class="act-btn" title="Edit role" onclick="openEditUser('+uid+',\''+jsq(uname)+'\','+rid+')">'+I.edit+'</button>';
     html+='<button class="act-btn" title="Force logout" onclick="forceLogout('+uid+')">'+I.disconnect+'</button>';
     if(banned)html+='<button class="act-btn" title="Unban" onclick="unbanUser('+uid+')">'+I.check+'</button>';
-    else html+='<button class="act-btn danger" title="Ban" onclick="openBanUser('+uid+',\''+esc(uname)+'\')">'+I.ban+'</button>';
+    else html+='<button class="act-btn danger" title="Ban" onclick="openBanUser('+uid+',\''+jsq(uname)+'\')">'+I.ban+'</button>';
     html+='</div></td></tr>';
   });
   html+='</tbody></table></div></div>';
@@ -528,8 +532,8 @@ async function renderChannels(){
     html+='<td><span class="badge '+(type==='voice'?'badge-yellow':type==='announcement'?'badge-accent':'badge-muted')+'">'+esc(type)+'</span></td>';
     html+='<td style="font-size:12px;color:var(--text-faint)">'+esc(cat)+'</td>';
     html+='<td>'+(archived?'<span class="badge badge-muted">Yes</span>':'<span class="badge badge-green">No</span>')+'</td>';
-    const lockBtn=type==='dm'?'':'<button class="act-btn" title="Access (private channel)" onclick="openChannelPermsModal('+id+',\''+esc(name)+'\')">'+I.lock+'</button>';
-    html+='<td><div class="act-group" style="justify-content:flex-end"><button class="act-btn" title="Edit" onclick="openChannelEditModal('+id+',\''+esc(name)+'\')">'+I.edit+'</button>'+lockBtn+'<button class="act-btn danger" title="Delete" onclick="openDeleteChannel('+id+',\''+esc(name)+'\')">'+I.trash+'</button></div></td></tr>';
+    const lockBtn=type==='dm'?'':'<button class="act-btn" title="Access (private channel)" onclick="openChannelPermsModal('+id+',\''+jsq(name)+'\')">'+I.lock+'</button>';
+    html+='<td><div class="act-group" style="justify-content:flex-end"><button class="act-btn" title="Edit" onclick="openChannelEditModal('+id+',\''+jsq(name)+'\')">'+I.edit+'</button>'+lockBtn+'<button class="act-btn danger" title="Delete" onclick="openDeleteChannel('+id+',\''+jsq(name)+'\')">'+I.trash+'</button></div></td></tr>';
   });
   html+='</tbody></table></div></div>';
   return html;
@@ -847,7 +851,7 @@ async function createBackup(){
 }
 
 function openRestoreModal(name){
-  openModal('<div class="modal-header"><h3>Restore Backup</h3><button class="modal-close" onclick="closeModal()">&times;</button></div><div class="modal-body"><p style="color:var(--text-muted)">Overwrite the current database with <strong style="color:white">'+esc(name)+'</strong>? A pre-restore backup will be created. Server restart recommended after restore.</p></div><div class="modal-footer"><button class="btn btn-ghost" onclick="closeModal()">Cancel</button><button class="btn btn-danger" onclick="confirmRestore(\''+esc(name)+'\')">Restore</button></div>');
+  openModal('<div class="modal-header"><h3>Restore Backup</h3><button class="modal-close" onclick="closeModal()">&times;</button></div><div class="modal-body"><p style="color:var(--text-muted)">Overwrite the current database with <strong style="color:white">'+esc(name)+'</strong>? A pre-restore backup will be created. Server restart recommended after restore.</p></div><div class="modal-footer"><button class="btn btn-ghost" onclick="closeModal()">Cancel</button><button class="btn btn-danger" onclick="confirmRestore(\''+jsq(name)+'\')">Restore</button></div>');
 }
 
 async function confirmRestore(name){

Server/ws/export_test.go

@@ -217,3 +217,8 @@ func (h *Hub) HandleWebhookParticipantLeftForTest(userID int64, channelID int64,
 	}
 	h.handleWebhookParticipantLeft(context.Background(), event)
 }
+
+// MustFullResyncForTest exposes mustFullResync for external tests.
+func (h *Hub) MustFullResyncForTest(lastSeq uint64) bool {
+	return h.mustFullResync(lastSeq)
+}

Server/ws/hub.go

@@ -74,6 +74,13 @@ type Hub struct {
 	reconnectTierDB   atomic.Uint64
 	reconnectTierFull atomic.Uint64
 
+	// Sequence watermark of the last channel-visibility change. Visibility
+	// updates are sent as targeted, unsequenced messages, so clients resuming
+	// from a seq at or before this point must take the full-ready path to
+	// converge (replay cannot deliver them). Reset on restart — a fresh
+	// connection always gets a correctly filtered ready payload anyway.
+	visibilityChangeSeq atomic.Uint64
+
 	// Settings cache — avoids per-connection DB queries for server_name/motd.
 	settingsMu         syncutil.RWMutex
 	settingsName       string
@@ -557,7 +564,16 @@ func (h *Hub) RefreshChannelVisibility(ch *db.Channel) {
 		if c.user == nil {
 			continue
 		}
-		if roleVisible(c.user.RoleID) {
+		// c.user is a connect-time snapshot; an admin may have changed the
+		// user's role mid-session, so resolve the current role from the DB.
+		// Fail closed: on error send nothing rather than mis-target.
+		fresh, err := h.db.GetUserByID(c.user.ID)
+		if err != nil || fresh == nil {
+			slog.Warn("hub: RefreshChannelVisibility could not resolve user role",
+				"user_id", c.user.ID, "err", err)
+			continue
+		}
+		if roleVisible(fresh.RoleID) {
 			// Idempotent add on the client; also refreshes channel metadata.
 			c.sendMsg(buildChannelCreate(ch))
 			continue
@@ -570,6 +586,20 @@ func (h *Hub) RefreshChannelVisibility(ch *db.Channel) {
 		}
 		c.mu.Unlock()
 	}
+
+	// Clients not connected right now missed the targeted sends above. Move
+	// the watermark so any resume from a seq at or before this point is
+	// forced onto the full-ready path instead of replay (stored after the
+	// sends so a concurrent seq advance errs toward re-syncing more clients).
+	h.visibilityChangeSeq.Store(atomic.LoadUint64(&h.seq))
+}
+
+// mustFullResync reports whether a client resuming from lastSeq predates the
+// most recent channel-visibility change and therefore cannot converge via
+// replay.
+func (h *Hub) mustFullResync(lastSeq uint64) bool {
+	w := h.visibilityChangeSeq.Load()
+	return w > 0 && lastSeq <= w
 }
 
 // BroadcastMemberBan sends a member_ban message to all connected clients

Server/ws/hub_test.go

@@ -954,6 +954,36 @@ func TestRefreshChannelVisibility_TargetedSends(t *testing.T) {
 	assertNoMsgType(t, memberSend, "channel_delete")
 }
 
+func TestRefreshChannelVisibility_ForcesFullResyncForStaleResumes(t *testing.T) {
+	hub, database := newTestHub(t)
+
+	chID := seedTestChannel(t, database, "watermark-room")
+	ch, err := database.GetChannel(chID)
+	if err != nil || ch == nil {
+		t.Fatalf("GetChannel: %v", err)
+	}
+
+	// No visibility change yet — resume is allowed regardless of seq.
+	if hub.MustFullResyncForTest(1) {
+		t.Error("expected replay allowed before any visibility change")
+	}
+
+	hub.SeedSeq(41)
+	hub.RefreshChannelVisibility(ch)
+
+	// Clients resuming from at/before the change must take the full path.
+	if !hub.MustFullResyncForTest(41) {
+		t.Error("expected forced full resync for lastSeq at the watermark")
+	}
+	if !hub.MustFullResyncForTest(10) {
+		t.Error("expected forced full resync for lastSeq before the watermark")
+	}
+	// Clients that saw sequenced traffic after the change may replay.
+	if hub.MustFullResyncForTest(42) {
+		t.Error("expected replay allowed for lastSeq after the watermark")
+	}
+}
+
 // hubTestSchema is the minimal schema needed for hub tests.
 var hubTestSchema = []byte(`
 CREATE TABLE IF NOT EXISTS roles (

Server/ws/serve.go

@@ -112,6 +112,16 @@ func (h *Hub) upgradeAndAuth(
 func (h *Hub) handleReconnect(
 	ctx context.Context, conn *websocket.Conn, c *Client, database *db.DB, lastSeq uint64,
 ) bool {
+	// Channel-visibility changes are delivered as targeted, unsequenced
+	// messages, so replay cannot bring a client that missed one back into a
+	// coherent state — force the full-ready path instead.
+	if h.mustFullResync(lastSeq) {
+		slog.Info("ws replay skipped (visibility changed since last_seq), sending full ready",
+			"user_id", c.userID, "last_seq", lastSeq)
+		h.reconnectTierFull.Add(1)
+		telemetry.NewAppMetrics().WSReconnectTierTotal.Add(ctx, 1, telemetry.String("tier", "full"))
+		return false
+	}
 	// Compute the set of channel IDs the reconnecting user can access so that
 	// channel-scoped replay events are filtered by current permissions (M3).
 	allowedChannelIDs, err := h.computeAllowedChannels(database, c.user)