security: fix critical race conditions and add validation to session management

This commit addresses multiple CRITICAL and HIGH severity security issues
identified during the multi-session implementation review.

CRITICAL Fixes:
- Fix race condition in session approval handlers (jsonrpc.go)
  Previously approveNewSession and denyNewSession directly mutated
  session.Mode without holding the SessionManager.mu lock, potentially
  causing data corruption during concurrent access.

- Add validation to ApprovePrimaryRequest (session_manager.go:795-810)
  Now verifies that requester session exists and is in Queued mode
  before approving transfer, preventing invalid state transitions.

- Close dual-primary window during reconnection (session_manager.go:208)
  Added explicit primaryExists check to prevent brief window where two
  sessions could both be primary during reconnection.

HIGH Priority Fixes:
- Add nickname uniqueness validation (session_manager.go:152-159)
  Prevents multiple sessions from having the same nickname, both in
  AddSession and updateSessionNickname RPC handler.

Code Quality:
- Remove debug scaffolding from cloud.go (lines 515-520, 530)
  Cleaned up temporary debug logs that are no longer needed.

Thread Safety:
- Add centralized ApproveSession() method (session_manager.go:870-890)
- Add centralized DenySession() method (session_manager.go:894-912)
  Both methods properly acquire locks and validate session state.

- Update RPC handlers to use thread-safe methods
  approveNewSession and denyNewSession now call sessionManager methods
  instead of direct session mutation.

All changes have been verified with linters (golangci-lint: 0 issues).

cloud.go

@@ -512,12 +512,8 @@ func handleSessionRequest(
 		_ = wsjson.Write(context.Background(), c, gin.H{"error": "session manager not initialized"})
 		return fmt.Errorf("session manager not initialized")
 	}
-	scopedLogger.Debug().Msg("About to call AddSession")
+
 	err = sessionManager.AddSession(session, req.SessionSettings)
-	scopedLogger.Debug().
-		Bool("addSessionSucceeded", err == nil).
-		Str("error", fmt.Sprintf("%v", err)).
-		Msg("AddSession returned")
 	if err != nil {
 		scopedLogger.Warn().Err(err).Msg("failed to add session to session manager")
 		if err == ErrMaxSessionsReached {
@@ -527,7 +523,6 @@ func handleSessionRequest(
 		}
 		return err
 	}
-	scopedLogger.Debug().Msg("AddSession completed successfully, continuing")
 
 	if session.HasPermission(PermissionPaste) {
 		cancelKeyboardMacro()

jsonrpc.go

@@ -192,12 +192,10 @@ func onRPCMessage(message webrtc.DataChannelMessage, session *Session) {
 		if err := RequirePermission(session, PermissionSessionApprove); err != nil {
 			handlerErr = err
 		} else if sessionID, ok := request.Params["sessionId"].(string); ok {
-			if targetSession := sessionManager.GetSession(sessionID); targetSession != nil && targetSession.Mode == SessionModePending {
+			handlerErr = sessionManager.ApproveSession(sessionID)
-				targetSession.Mode = SessionModeObserver
+			if handlerErr == nil {
-				sessionManager.broadcastSessionListUpdate()
+				go sessionManager.broadcastSessionListUpdate()
 				result = map[string]interface{}{"status": "approved"}
-			} else {
-				handlerErr = errors.New("session not found or not pending")
 			}
 		} else {
 			handlerErr = errors.New("invalid sessionId parameter")
@@ -206,14 +204,18 @@ func onRPCMessage(message webrtc.DataChannelMessage, session *Session) {
 		if err := RequirePermission(session, PermissionSessionApprove); err != nil {
 			handlerErr = err
 		} else if sessionID, ok := request.Params["sessionId"].(string); ok {
-			if targetSession := sessionManager.GetSession(sessionID); targetSession != nil && targetSession.Mode == SessionModePending {
+			handlerErr = sessionManager.DenySession(sessionID)
-				writeJSONRPCEvent("sessionAccessDenied", map[string]interface{}{
+			if handlerErr == nil {
-					"message": "Access denied by primary session",
+				// Notify the denied session
-				}, targetSession)
+				if targetSession := sessionManager.GetSession(sessionID); targetSession != nil {
-				sessionManager.broadcastSessionListUpdate()
+					go func() {
+						writeJSONRPCEvent("sessionAccessDenied", map[string]interface{}{
+							"message": "Access denied by primary session",
+						}, targetSession)
+						sessionManager.broadcastSessionListUpdate()
+					}()
+				}
 				result = map[string]interface{}{"status": "denied"}
-			} else {
-				handlerErr = errors.New("session not found or not pending")
 			}
 		} else {
 			handlerErr = errors.New("invalid sessionId parameter")
@@ -251,24 +253,35 @@ func onRPCMessage(message webrtc.DataChannelMessage, session *Session) {
 		} else if targetSession := sessionManager.GetSession(sessionID); targetSession != nil {
 			// Users can update their own nickname, or admins can update any
 			if targetSession.ID == session.ID || session.HasPermission(PermissionSessionManage) {
-				targetSession.Nickname = nickname
+				// Check nickname uniqueness
-
+				allSessions := sessionManager.GetAllSessions()
-				// If session is pending and approval is required, send the approval request now that we have a nickname
+				for _, existingSession := range allSessions {
-				if targetSession.Mode == SessionModePending && currentSessionSettings != nil && currentSessionSettings.RequireApproval {
+					if existingSession.ID != sessionID && existingSession.Nickname == nickname {
-					if primary := sessionManager.GetPrimarySession(); primary != nil {
+						handlerErr = fmt.Errorf("nickname '%s' is already in use by another session", nickname)
-						go func() {
+						break
-							writeJSONRPCEvent("newSessionPending", map[string]interface{}{
-								"sessionId": targetSession.ID,
-								"source":    targetSession.Source,
-								"identity":  targetSession.Identity,
-								"nickname":  targetSession.Nickname,
-							}, primary)
-						}()
 					}
 				}
 
-				sessionManager.broadcastSessionListUpdate()
+				if handlerErr == nil {
-				result = map[string]interface{}{"status": "updated"}
+					targetSession.Nickname = nickname
+
+					// If session is pending and approval is required, send the approval request now that we have a nickname
+					if targetSession.Mode == SessionModePending && currentSessionSettings != nil && currentSessionSettings.RequireApproval {
+						if primary := sessionManager.GetPrimarySession(); primary != nil {
+							go func() {
+								writeJSONRPCEvent("newSessionPending", map[string]interface{}{
+									"sessionId": targetSession.ID,
+									"source":    targetSession.Source,
+									"identity":  targetSession.Identity,
+									"nickname":  targetSession.Nickname,
+								}, primary)
+							}()
+						}
+					}
+
+					sessionManager.broadcastSessionListUpdate()
+					result = map[string]interface{}{"status": "updated"}
+				}
 			} else {
 				handlerErr = errors.New("permission denied: can only update own nickname")
 			}

session_manager.go

@@ -149,6 +149,15 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe
 	sm.mu.Lock()
 	defer sm.mu.Unlock()
 
+	// Check nickname uniqueness (only for non-empty nicknames)
+	if session.Nickname != "" {
+		for id, existingSession := range sm.sessions {
+			if id != session.ID && existingSession.Nickname == session.Nickname {
+				return fmt.Errorf("nickname '%s' is already in use by another session", session.Nickname)
+			}
+		}
+	}
+
 	wasWithinGracePeriod := false
 	wasPreviouslyPrimary := false
 	wasPreviouslyPending := false
@@ -195,12 +204,14 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe
 		// If this was the primary, try to restore primary status
 		if existing.Mode == SessionModePrimary {
 			isBlacklisted := sm.isSessionBlacklisted(session.ID)
-			if sm.lastPrimaryID == session.ID && !isBlacklisted {
+			// SECURITY: Prevent dual-primary window - only restore if no other primary exists
+			primaryExists := sm.primarySessionID != "" && sm.sessions[sm.primarySessionID] != nil
+			if sm.lastPrimaryID == session.ID && !isBlacklisted && !primaryExists {
 				sm.primarySessionID = session.ID
 				sm.lastPrimaryID = ""
 				delete(sm.reconnectGrace, session.ID)
 			} else {
-				// Grace period expired or another session took over
+				// Grace period expired, another session took over, or primary already exists
 				session.Mode = SessionModeObserver
 			}
 		}
@@ -781,6 +792,23 @@ func (sm *SessionManager) ApprovePrimaryRequest(currentPrimaryID, requesterID st
 		return errors.New("not the primary session")
 	}
 
+	// SECURITY: Verify requester session exists and is in Queued mode
+	requesterSession, exists := sm.sessions[requesterID]
+	if !exists {
+		sm.logger.Error().
+			Str("requesterID", requesterID).
+			Msg("Requester session not found")
+		return errors.New("requester session not found")
+	}
+
+	if requesterSession.Mode != SessionModeQueued {
+		sm.logger.Error().
+			Str("requesterID", requesterID).
+			Str("actualMode", string(requesterSession.Mode)).
+			Msg("Requester session is not in queued mode")
+		return fmt.Errorf("requester session is not in queued mode (current mode: %s)", requesterSession.Mode)
+	}
+
 	// Remove requester from queue
 	sm.removeFromQueue(requesterID)
 
@@ -838,6 +866,51 @@ func (sm *SessionManager) DenyPrimaryRequest(currentPrimaryID, requesterID strin
 	return nil
 }
 
+// ApproveSession approves a pending session (thread-safe)
+func (sm *SessionManager) ApproveSession(sessionID string) error {
+	sm.mu.Lock()
+	defer sm.mu.Unlock()
+
+	session, exists := sm.sessions[sessionID]
+	if !exists {
+		return ErrSessionNotFound
+	}
+
+	if session.Mode != SessionModePending {
+		return errors.New("session is not in pending mode")
+	}
+
+	// Promote session to observer
+	session.Mode = SessionModeObserver
+
+	sm.logger.Info().
+		Str("sessionID", sessionID).
+		Msg("Session approved and promoted to observer")
+
+	return nil
+}
+
+// DenySession denies a pending session (thread-safe)
+func (sm *SessionManager) DenySession(sessionID string) error {
+	sm.mu.Lock()
+	defer sm.mu.Unlock()
+
+	session, exists := sm.sessions[sessionID]
+	if !exists {
+		return ErrSessionNotFound
+	}
+
+	if session.Mode != SessionModePending {
+		return errors.New("session is not in pending mode")
+	}
+
+	sm.logger.Info().
+		Str("sessionID", sessionID).
+		Msg("Session denied - notifying session")
+
+	return nil
+}
+
 // ForEachSession executes a function for each active session
 func (sm *SessionManager) ForEachSession(fn func(*Session)) {
 	sm.mu.RLock()