Address CodeRabbit review and fix CI on embedded-vnc

client/vnc/server/agent_windows.go

@@ -47,8 +47,6 @@ var (
 	procWTSEnumerateSessionsW      = wtsapi32.NewProc("WTSEnumerateSessionsW")
 	procWTSFreeMemory              = wtsapi32.NewProc("WTSFreeMemory")
 	procWTSQuerySessionInformation = wtsapi32.NewProc("WTSQuerySessionInformationW")
-
-	iphlpapi                = windows.NewLazySystemDLL("iphlpapi.dll")
 )
 
 // GetCurrentSessionID returns the session ID of the current process.
@@ -514,6 +512,8 @@ func (m *sessionManager) reapExitedAgent() {
 		log.Debugf("close agent handle: %v", err)
 	}
 	m.agentProc = 0
+	m.authToken = ""
+	m.socketPath = ""
 }
 
 // scheduleNextSpawn applies an exponential backoff on fast crashes (<5s) and
@@ -586,6 +586,8 @@ func (m *sessionManager) killAgent() {
 	_ = windows.TerminateProcess(m.agentProc, 0)
 	_ = windows.CloseHandle(m.agentProc)
 	m.agentProc = 0
+	m.authToken = ""
+	m.socketPath = ""
 	log.Info("killed old agent")
 }
 

client/vnc/server/console_user_darwin.go

@@ -1,19 +1,10 @@
 package server
 
-import "errors"
-
-// consoleHasInteractiveUser returns true when a user is logged into the
-// console (i.e. an Aqua session is active). At the loginwindow there is
-// nobody to display an approval prompt to, so callers can decline
-// without waiting on the broker.
-func consoleHasInteractiveUser() bool {
-	if _, err := consoleUserID(); err != nil {
-		if errors.Is(err, errNoConsoleUser) {
-			return false
-		}
-		// Unknown error: fail closed so a probe-time glitch does not
-		// silently let an unattended console accept VNC sessions.
-		return false
-	}
-	return true
+// interactiveUserError returns nil when a user is logged into the console
+// (i.e. an Aqua session is active). At the loginwindow there is nobody to
+// display an approval prompt to, so callers can decline without waiting on
+// the broker. Any error (including errNoConsoleUser) is treated as decline.
+func interactiveUserError() error {
+	_, err := consoleUserID()
+	return err
 }

client/vnc/server/console_user_other.go

@@ -2,6 +2,6 @@
 
 package server
 
-// consoleHasInteractiveUser is unused outside service mode (darwin/windows)
-// but the symbol must exist so gateApproval compiles on all platforms.
-func consoleHasInteractiveUser() bool { return true }
+// interactiveUserError is unused outside service mode (darwin/windows) but
+// the symbol must exist so gateApproval compiles on all platforms.
+func interactiveUserError() error { return nil }

client/vnc/server/console_user_windows.go

@@ -1,13 +1,15 @@
 package server
 
-// consoleHasInteractiveUser returns true when there is a logged-in user
-// session on the box. At the lock/login screen WTSQueryUserName is empty,
-// which means there is nobody to display an approval prompt to. Callers
-// should decline without waiting on the broker in that case.
-func consoleHasInteractiveUser() bool {
+// interactiveUserError returns nil when there is a logged-in user session
+// on the box. At the lock/login screen WTSQueryUserName is empty, which
+// means there is nobody to display an approval prompt to.
+func interactiveUserError() error {
 	sid := getActiveSessionID()
 	if sid == 0 {
-		return false
+		return errNoConsoleUser
 	}
-	return wtsSessionHasUser(sid)
+	if !wtsSessionHasUser(sid) {
+		return errNoConsoleUser
+	}
+	return nil
 }

client/vnc/server/server.go

@@ -431,10 +431,12 @@ func (s *Server) gateApproval(conn net.Conn, header *connectionHeader, connLog *
 		connLog.Warn("VNC connection rejected: approval required but no approver")
 		return false, ApprovalDecision{}
 	}
-	if s.serviceMode && !consoleHasInteractiveUser() {
-		rejectConnection(conn, codeMessage(RejectCodeNoConsoleUser, "no interactive user session"))
-		connLog.Info("VNC connection rejected: no interactive user session to approve")
-		return false, ApprovalDecision{}
+	if s.serviceMode {
+		if err := interactiveUserError(); err != nil {
+			rejectConnection(conn, codeMessage(RejectCodeNoConsoleUser, "no interactive user session"))
+			connLog.Infof("VNC connection rejected: no interactive user session to approve: %v", err)
+			return false, ApprovalDecision{}
+		}
 	}
 	info := ApprovalInfo{
 		SourceIP: sourceIPString(conn.RemoteAddr()),
@@ -449,7 +451,7 @@ func (s *Server) gateApproval(conn net.Conn, header *connectionHeader, connLog *
 	}
 	decision, err := s.approver.Request(s.ctx, info)
 	if err != nil {
-		rejectConnection(conn, codeMessage(RejectCodeApprovalDenied, err.Error()))
+		rejectConnection(conn, codeMessage(RejectCodeApprovalDenied, "approval denied"))
 		connLog.Infof("VNC connection rejected: approval %v", err)
 		return false, ApprovalDecision{}
 	}
@@ -737,9 +739,13 @@ func (s *Server) validateCapturer(capturer ScreenCapturer) error {
 func (s *Server) isAllowedSource(addr net.Addr) bool {
 	// Unix-socket remotes (the agent path) are local IPC, gated by the
 	// token, not by overlay membership.
+	if _, ok := addr.(*net.UnixAddr); ok {
+		return true
+	}
 	tcpAddr, ok := addr.(*net.TCPAddr)
 	if !ok {
-		return true
+		s.log.Warnf("connection rejected: unsupported remote address type %T", addr)
+		return false
 	}
 
 	remoteIP, ok := netip.AddrFromSlice(tcpAddr.IP)

client/vnc/server/server_windows.go

@@ -269,4 +269,3 @@ func (s *Server) serviceAcceptLoop() {
 		}(conn)
 	}
 }
-

client/vnc/server/session.go

@@ -545,6 +545,9 @@ func (s *session) handleFBUpdateRequest() error {
 // in sync with the active session (e.g. username changes after login on
 // a virtual session).
 func (s *session) SendDesktopName(name string) error {
+	if s.viewOnly {
+		name = ViewOnlyDesktopNamePrefix + name
+	}
 	s.encMu.RLock()
 	supported := s.clientSupportsDesktopName
 	s.encMu.RUnlock()
@@ -629,13 +632,13 @@ func (s *session) handlePointerEvent() error {
 		mask = (mask & 0x7f) | uint16(hi[0])<<7
 	}
 
+	if s.viewOnly {
+		return nil
+	}
 	s.pointerMu.Lock()
 	s.lastPointerX = x
 	s.lastPointerY = y
 	s.pointerMu.Unlock()
-	if s.viewOnly {
-		return nil
-	}
 	s.injector.InjectPointer(mask, x, y, s.serverW, s.serverH)
 	return nil
 }
@@ -661,6 +664,9 @@ var stickyModifierKeysyms = [...]uint32{
 // when the client disconnects mid-press. Mouse coordinates are reused
 // from the last PointerEvent so we don't warp the cursor.
 func (s *session) releaseStickyInput() {
+	if s.viewOnly {
+		return
+	}
 	for _, ks := range stickyModifierKeysyms {
 		s.injector.InjectKey(ks, false)
 	}