Compare commits

...

3 Commits

Author SHA1 Message Date
Alex P 15963d39ef fix: primary session timeout promoting wrong session
When primary timed out, emergency promotion was re-promoting the same
timed-out session instead of promoting an observer. The emergency bypass
logic ignored the excludeSessionID parameter.

Fixed by applying session exclusion logic in all emergency promotion paths.
2025-10-23 14:19:46 +03:00
Alex P 8c1ebe35fd fix: primary session timeout not tracking RPC activity
The primary session inactivity timeout was only tracking HID-related
activity (keyboard, mouse, mass storage) but not general RPC messages.
This meant users watching video or interacting with the UI would be
timed out even though they were actively using the session.

Added UpdateLastActive call in onRPCMessage to track any valid RPC
activity, ensuring all user interactions reset the inactivity timer.
2025-10-23 11:47:30 +03:00
Alex P 1b007b76d7 fix: resolve critical concurrency and safety issues in session management
- Fix panic recovery in AddSession to log instead of re-throwing, preventing process crashes
- Fix integer overflow in trust score calculation by capping before int conversion
- Fix TOCTOU race condition in nickname uniqueness check with atomic UpdateSessionNickname method
2025-10-23 10:47:37 +03:00
3 changed files with 76 additions and 15 deletions

View File

@ -143,16 +143,10 @@ func handleUpdateSessionNicknameRPC(params map[string]any, session *Session) (an
return nil, errors.New("permission denied: can only update own nickname")
}
// Check nickname uniqueness
allSessions := sessionManager.GetAllSessions()
for _, existingSession := range allSessions {
if existingSession.ID != sessionID && existingSession.Nickname == nickname {
return nil, fmt.Errorf("nickname '%s' is already in use by another session", nickname)
}
if err := sessionManager.UpdateSessionNickname(sessionID, nickname); err != nil {
return nil, err
}
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 {

View File

@ -33,7 +33,27 @@ func (sm *SessionManager) attemptEmergencyPromotion(ctx emergencyPromotionContex
Str("triggerSessionID", ctx.triggerSessionID).
Str("triggerReason", ctx.triggerReason).
Msg("Bypassing emergency promotion rate limits - no primary exists")
promotedSessionID := sm.findMostTrustedSessionForEmergency()
// Find best session, excluding the specified session if provided
var promotedSessionID string
if excludeSessionID != "" {
bestSessionID := ""
bestScore := -1
for id, session := range sm.sessions {
if id != excludeSessionID &&
!sm.isSessionBlacklisted(id) &&
(session.Mode == SessionModeObserver || session.Mode == SessionModeQueued) {
score := sm.getSessionTrustScore(id)
if score > bestScore {
bestScore = score
bestSessionID = id
}
}
}
promotedSessionID = bestSessionID
} else {
promotedSessionID = sm.findMostTrustedSessionForEmergency()
}
return promotedSessionID, true, false
}
@ -266,6 +286,12 @@ func (sm *SessionManager) handlePrimarySessionTimeout(now time.Time) bool {
primary.Mode = SessionModeObserver
sm.primarySessionID = ""
sm.logger.Info().
Str("sessionID", timedOutSessionID).
Dur("inactiveFor", now.Sub(primary.LastActive)).
Dur("timeout", currentTimeout).
Msg("Primary session timed out due to inactivity - demoted to observer")
ctx := emergencyPromotionContext{
triggerSessionID: timedOutSessionID,
triggerReason: "timeout",
@ -274,7 +300,8 @@ func (sm *SessionManager) handlePrimarySessionTimeout(now time.Time) bool {
promotedSessionID, isEmergency, shouldSkip := sm.attemptEmergencyPromotion(ctx, timedOutSessionID)
if shouldSkip {
return false
sm.logger.Info().Msg("Promotion skipped after timeout - session demoted but no promotion")
return true // Still need to broadcast the demotion
}
if promotedSessionID != "" {
@ -307,8 +334,17 @@ func (sm *SessionManager) handlePrimarySessionTimeout(now time.Time) bool {
Bool("isEmergencyPromotion", isEmergency).
Msg("Auto-promoted session after primary timeout")
return true
} else {
sm.logger.Error().Err(err).
Str("timedOutSessionID", timedOutSessionID).
Str("promotedSessionID", promotedSessionID).
Msg("Failed to promote session after timeout - primary demoted")
return true // Still broadcast the demotion even if promotion failed
}
}
return false
sm.logger.Info().
Str("timedOutSessionID", timedOutSessionID).
Msg("Primary session timed out - demoted to observer, no eligible sessions to promote")
return true // Broadcast the demotion even if no promotion
}

View File

@ -195,7 +195,7 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe
delete(sm.nicknameIndex, session.Nickname)
}
}
panic(r)
sm.logger.Error().Interface("panic", r).Str("sessionID", session.ID).Msg("Recovered from panic in AddSession")
}
}()
@ -987,6 +987,35 @@ func (sm *SessionManager) UpdateLastActive(sessionID string) {
sm.mu.Unlock()
}
// UpdateSessionNickname atomically updates a session's nickname with uniqueness check
func (sm *SessionManager) UpdateSessionNickname(sessionID, nickname string) error {
sm.mu.Lock()
defer sm.mu.Unlock()
targetSession, exists := sm.sessions[sessionID]
if !exists {
return errors.New("session not found")
}
// Check nickname uniqueness under lock
if existingSession, nicknameInUse := sm.nicknameIndex[nickname]; nicknameInUse {
if existingSession.ID != sessionID {
return fmt.Errorf("nickname '%s' is already in use by another session", nickname)
}
}
// Remove old nickname from index
if targetSession.Nickname != "" {
delete(sm.nicknameIndex, targetSession.Nickname)
}
// Update nickname and index atomically
targetSession.Nickname = nickname
sm.nicknameIndex[nickname] = targetSession
return nil
}
// Internal helper methods
// validateSinglePrimary ensures there's only one primary session and fixes any inconsistencies
@ -1364,9 +1393,11 @@ func (sm *SessionManager) getSessionTrustScore(sessionID string) int {
// Longer session duration = more trust (up to 100 points for 100+ minutes)
sessionAge := now.Sub(session.CreatedAt)
score += int(sessionAge.Minutes())
if score > 100 {
score = 100 // Cap age bonus at 100 points
sessionAgeMinutes := sessionAge.Minutes()
if sessionAgeMinutes > 100 {
score += 100
} else {
score += int(sessionAgeMinutes)
}
// Recently successful primary sessions get higher trust