The getPermissions useEffect had send and pollPermissions in its dependency
array. Since send gets recreated when rpcDataChannel changes, this caused
multiple getPermissions RPC calls (5 observed) on page load.
Fix:
- Add rpcDataChannel readiness check to prevent calls before channel is open
- Remove send and pollPermissions from dependency array
- Keep only currentMode and rpcDataChannel.readyState as dependencies
This ensures getPermissions is called only when:
1. The RPC channel becomes ready (readyState changes to "open")
2. The session mode changes (observer <-> primary)
Eliminates duplicate RPC calls while maintaining correct behavior for
mode changes and initial connection.
The getLocalVersion useEffect had getLocalVersion and hasPermission in
its dependency array. Since these functions are recreated on every render,
this caused an infinite loop of RPC calls when refreshing the primary
session, resulting in 100+ identical getLocalVersion requests.
Fix: Remove function references from dependency array, only keep appVersion
which is the actual data dependency. The effect now only runs once when
appVersion changes from null to a value.
This is the same pattern as the previous fix for getVideoState,
getKeyboardLedState, and getKeyDownState.
Changes:
- Add permission checks before making getVideoState, getKeyboardLedState,
and getKeyDownState RPC calls to prevent rejected requests for sessions
without VIDEO_VIEW permission
- Fix infinite loop issue by excluding hasPermission from useEffect
dependency arrays (functions recreated on render cause infinite loops)
- Increase RPC rate limit from 100 to 500 per second to support 10+
concurrent sessions with broadcasts and state updates
This eliminates console spam from permission denied errors and log spam
from continuous RPC calls, while improving multi-session performance.
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).
The session manager had backwards logic that prevented sessions from
restoring their primary status when reconnecting within the grace period.
This caused browser refreshes to demote primary sessions to observers.
Changes:
- Fix conditional in AddSession to allow primary restoration within grace
- Remove excessive debug logging throughout session manager
- Clean up unused decrActiveSessions function
- Remove unnecessary leading newline in NewSessionManager
- Update lastPrimaryID handling to support WebRTC reconnections
- Preserve grace periods during transfers to allow browser refreshes
The fix ensures that when a primary session refreshes their browser:
1. RemoveSession adds a grace period entry
2. New connection checks wasWithinGracePeriod and wasPreviouslyPrimary
3. Session correctly reclaims primary status
Blacklist system prevents demoted sessions from immediate re-promotion
while grace periods allow legitimate reconnections.
When a user explicitly logs out via the logout button, the session should
be removed immediately without grace period, allowing observers to be
promoted right away instead of waiting for the grace period to expire.
Changes:
- Close WebRTC connection immediately on logout
- Clear grace period marker for intentional logout detection
- Add logging to track logout vs disconnect differentiation
This complements the accidental disconnect handling which uses grace period.
When a primary session disconnects accidentally (not intentional logout), the
60-second transfer blacklist from previous role transfers was blocking observer
sessions from being promoted after the grace period expires (~10s).
The blacklist is intended to prevent immediate re-promotion during manual
transfers (user-initiated), but should not interfere with emergency promotion
after accidental disconnects (system-initiated).
Changes:
- Clear all transfer blacklist entries when primary enters grace period
- Add logging to track blacklist clearing for debugging
- Preserve blacklist during intentional logout to maintain manual transfer protection
This ensures observers are promoted after grace period (~10s) instead of
waiting for blacklist expiration (~40-60s).
1. Terminal access permission check:
- Add Permission.TERMINAL_ACCESS check to Web Terminal button
- Prevents observer sessions from accessing terminal
2. Immediate websocket cleanup:
- Close peer connection immediately when websocket errors
- Previously waited 24+ seconds for ICE to transition from disconnected to failed
- Now triggers session cleanup immediately on tab close
3. Immediate grace period validation:
- Trigger validateSinglePrimary() immediately when grace period expires
- Previously waited up to 10 seconds for next periodic validation
- Eliminates unnecessary delay in observer promotion
Timeline improvement:
Before: Tab close → 6s (ICE disconnect) → 24s (ICE fail) → RemoveSession → 10s grace → up to 10s validation = ~50s total
After: Tab close → immediate peerConnection.Close() → immediate RemoveSession → 10s grace → immediate validation = ~11s total
The previous limit of 20 RPC/second per session was too aggressive for
multi-session scenarios. During normal operation with multiple sessions,
legitimate RPC calls would frequently hit the rate limit, especially
during page refreshes or reconnections when sessions make bursts of calls
like getSessions, getPermissions, getLocalVersion, and getVideoState.
Increased the limit to 100 RPC/second per session, which still provides
DoS protection while accommodating legitimate multi-session usage patterns.
Add comprehensive logging to identify why sessions fail to be added to
the session manager:
- Log entry/exit points in AddSession
- Track reconnection path execution
- Log max sessions limit checks
- Trace AddSession call and return in handleSessionRequest
This will help diagnose why sessions get stuck at ICE checking state
without being properly registered in the session manager.
Observer-to-primary promotion protections:
- Block auto-promotion during active primary grace periods
- Prevent creating multiple primary sessions simultaneously
- Validate transfer source is actual current primary
- Check for duplicate primaries before promotion
Immediate promotion on logout:
- Trigger validateSinglePrimary() immediately when primary disconnects
- Smart grace period bypass: allow promotion within 2 seconds of disconnect
- Provides instant promotion on logout while protecting against network blips
Enhanced validation and logging:
- Log session additions/removals with counts
- Display session IDs in validation logs for debugging
- Track grace period timing for smart bypass decisions
Sessions in pending mode do not have PermissionVideoView and should not
attempt to call getLocalVersion RPC method. Add permission check before
calling getLocalVersion to prevent unnecessary permission denied errors.
Backend improvements:
- Keep denied sessions alive in pending mode instead of removing them
- Add requestSessionApproval RPC method for re-requesting access
- Fix security issue: preserve pending mode on reconnection for denied sessions
- Add MaxRejectionAttempts field to SessionSettings (default: 3, configurable 1-10)
Frontend improvements:
- Change "Try Again" button to "Request Access Again" that re-requests approval
- Add rejection counter with configurable maximum attempts
- Hide modal after max rejections; session stays pending in SessionPopover
- Add "Dismiss" button for primary to hide approval requests without deciding
- Add MaxRejectionAttempts control in multi-session settings page
- Reset rejection count when session is approved
This improves the user experience by allowing denied users to retry without
page reloads, while preventing spam with configurable rejection limits.
CRITICAL SECURITY FIX: Pending sessions (awaiting approval) were granted
video.view permission, allowing denied sessions to see video when they
reconnected.
**Vulnerability:**
1. Session requests access and enters pending mode
2. Primary session denies the request
3. Denied session clicks "Try Again" and reconnects
4. New session enters pending mode but has video.view permission
5. User can see video stream despite being denied
**Fix:**
Remove PermissionVideoView from SessionModePending. Pending sessions now
have NO permissions until explicitly approved by the primary session.
This ensures:
- Denied sessions cannot access video on reconnection
- Only approved sessions (observer/queued/primary) can view video
- CanReceiveVideo() properly blocks video frames for pending sessions
Address all linting warnings and errors in both backend and frontend code:
**Go (golangci-lint):**
- Add error checking for ignored return values (errcheck)
- Remove unused RPC functions (unused)
- Fix import formatting (goimports)
**TypeScript/React (eslint):**
- Replace all 'any' and 'Function' types with proper type definitions
- Add RpcSendFunction type for consistent JSON-RPC callback signatures
- Fix React Hook exhaustive-deps warnings by adding missing dependencies
- Wrap functions in useCallback where needed to stabilize dependencies
- Remove unused variables and imports
- Remove empty code blocks
- Suppress exhaustive-deps warnings where intentional (with comments)
All linting now passes with 0 errors and 0 warnings.
* fix: update delay handling in PasteModal component
- Changed default delay value to 20 and adjusted validation to allow values between 0 and 65534.
- Cleaned up code formatting for better readability.
* fix: formatting
* feat: release keyPress automatically
* send keepalive when pressing the key
* remove logging
* clean up logging
* chore: use unreliable channel to send keepalive events
* chore: use ordered unreliable channel for pointer events
* chore: adjust auto release key interval
* chore: update logging for kbdAutoReleaseLock
* chore: update comment for KEEPALIVE_INTERVAL
* fix: should cancelAutorelease when pressed is true
* fix: handshake won't happen if webrtc reconnects
* chore: add trace log for writeWithTimeout
* chore: add timeout for KeypressReport
* chore: use the proper key to send release command
* refactor: simplify HID RPC keyboard input handling and improve key state management
- Updated `handleHidRPCKeyboardInput` to return errors directly instead of keys down state.
- Refactored `rpcKeyboardReport` and `rpcKeypressReport` to return errors instead of states.
- Introduced a queue for managing key down state updates in the `Session` struct to prevent input handling stalls.
- Adjusted the `UpdateKeysDown` method to handle state changes more efficiently.
- Removed unnecessary logging and commented-out code for clarity.
* refactor: enhance keyboard auto-release functionality and key state management
* fix: correct Windows default auto-repeat delay comment from 1ms to 1s
* refactor: send keypress as early as possible
* refactor: replace console.warn with console.info for HID RPC channel events
* refactor: remove unused NewKeypressKeepAliveMessage function from HID RPC
* fix: handle error in key release process and log warnings
* fix: log warning on keypress report failure
* fix: update auto-release keyboard interval to 225
* refactor: enhance keep-alive handling and jitter compensation in HID RPC
- Implemented staleness guard to ignore outdated keep-alive packets.
- Added jitter compensation logic to adjust timer extensions based on packet arrival times.
- Introduced new methods for managing keep-alive state and reset functionality in the Session struct.
- Updated auto-release delay mechanism to use dynamic durations based on keep-alive timing.
- Adjusted keep-alive interval in the UI to improve responsiveness.
* gofmt
* clean up code
* chore: use dynamic duration for scheduleAutoRelease
* Use harcoded timer reset value for now
* fix: prevent nil pointer dereference when stopping timers in Close method
* refactor: remove nil check for kbdAutoReleaseTimers in DelayAutoReleaseWithDuration
* refactor: optimize dependencies in useHidRpc hooks
* refactor: streamline keep-alive timer management in useKeyboard hook
* refactor: clarify comments in useKeyboard hook for resetKeyboardState function
* refactor: reduce keysDownStateQueueSize
* refactor: close and reset keysDownStateQueue in newSession function
* chore: resolve conflicts
* resolve conflicts
---------
Co-authored-by: Adam Shiervani <adam.shiervani@gmail.com>
Alex P
Aveline
Marc Brooks
Aylen
Alex
dependabot[bot]
Alexander
Adam Shiervani
Siyuan Miao