Fix review findings for embedded VNC server

client/cmd/vnc_agent_dropprivs_darwin.go

@@ -5,6 +5,8 @@ package cmd
 import (
 	"fmt"
 	"os"
+	"os/user"
+	"strconv"
 	"syscall"
 )
 
@@ -31,14 +33,21 @@ func dropAgentPrivileges(targetUID uint32) error {
 	if cur != 0 {
 		return fmt.Errorf("agent uid %d does not match expected %d and we lack root to fix it", cur, targetUID)
 	}
+	// Resolve the target user's real primary group rather than reusing
+	// targetUID as the gid: a user's primary group on macOS is typically
+	// staff(20), not gid==uid. Fail closed if the lookup fails.
+	targetGID, err := primaryGroupID(targetUID)
+	if err != nil {
+		return err
+	}
 	// Drop supplementary groups first: setgid alone doesn't touch the
 	// auxiliary group list, leaving root's groups attached would let the
 	// dropped process write to root-only group-writable files.
 	if err := syscall.Setgroups([]int{}); err != nil {
 		return fmt.Errorf("setgroups([]): %w", err)
 	}
-	if err := syscall.Setgid(int(targetUID)); err != nil {
-		return fmt.Errorf("setgid(%d): %w", targetUID, err)
+	if err := syscall.Setgid(targetGID); err != nil {
+		return fmt.Errorf("setgid(%d): %w", targetGID, err)
 	}
 	if err := syscall.Setuid(int(targetUID)); err != nil {
 		return fmt.Errorf("setuid(%d): %w", targetUID, err)
@@ -48,3 +57,18 @@ func dropAgentPrivileges(targetUID uint32) error {
 	}
 	return nil
 }
+
+// primaryGroupID resolves the real primary group id of the user with the
+// given uid. Fails closed: a lookup or parse error returns an error so the
+// caller never falls back to using uid as the gid.
+func primaryGroupID(targetUID uint32) (int, error) {
+	u, err := user.LookupId(strconv.Itoa(int(targetUID)))
+	if err != nil {
+		return 0, fmt.Errorf("look up uid %d: %w", targetUID, err)
+	}
+	gid, err := strconv.Atoi(u.Gid)
+	if err != nil {
+		return 0, fmt.Errorf("parse gid %q for uid %d: %w", u.Gid, targetUID, err)
+	}
+	return gid, nil
+}

client/cmd/vnc_agent_dropprivs_windows.go

@@ -6,9 +6,9 @@ package cmd
 // both run as SYSTEM (the daemon spawns the agent into the interactive
 // session via CreateProcessAsUser with an impersonation token, but the
 // resulting process still runs under SYSTEM, not under the user's
-// account). The Windows path relies on the C:\Windows\Temp socket
-// location (admin/SYSTEM-write-only) and the per-spawn token for
-// integrity instead.
+// account). The Windows path relies on the DACL-restricted socket
+// directory, the unpredictable per-spawn socket name, the listen-readiness
+// gate, and the per-spawn token for integrity instead.
 func dropAgentPrivileges(_ uint32) error {
 	return nil
 }

client/internal/engine_ssh.go

@@ -12,10 +12,10 @@ import (
 	firewallManager "github.com/netbirdio/netbird/client/firewall/manager"
 	"github.com/netbirdio/netbird/client/iface/netstack"
 	nftypes "github.com/netbirdio/netbird/client/internal/netflow/types"
-	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	sshconfig "github.com/netbirdio/netbird/client/ssh/config"
 	sshserver "github.com/netbirdio/netbird/client/ssh/server"
 	mgmProto "github.com/netbirdio/netbird/shared/management/proto"
+	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	sshuserhash "github.com/netbirdio/netbird/shared/sshauth"
 )
 

client/internal/engine_vnc.go

@@ -17,12 +17,11 @@ import (
 	"github.com/netbirdio/netbird/client/internal/peer"
 	"github.com/netbirdio/netbird/client/vnc"
 	vncserver "github.com/netbirdio/netbird/client/vnc/server"
-	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	mgmProto "github.com/netbirdio/netbird/shared/management/proto"
+	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	sshuserhash "github.com/netbirdio/netbird/shared/sshauth"
 )
 
-
 type vncServer interface {
 	Start(ctx context.Context, addr netip.AddrPort, network netip.Prefix) error
 	Stop() error
@@ -188,13 +187,13 @@ func (e *Engine) updateVNCServerAuth(vncAuth *mgmProto.VNCAuth) {
 	}
 
 	sessionPubKeys := make([]sshauth.SessionPubKey, 0, len(vncAuth.GetSessionPubKeys()))
-	for _, e := range vncAuth.GetSessionPubKeys() {
-		pub := e.GetPubKey()
+	for _, pk := range vncAuth.GetSessionPubKeys() {
+		pub := pk.GetPubKey()
 		if len(pub) != 32 {
 			log.Warnf("VNC session pubkey wrong length %d", len(pub))
 			continue
 		}
-		hash := e.GetUserIdHash()
+		hash := pk.GetUserIdHash()
 		if len(hash) != 16 {
 			log.Warnf("VNC session user id hash wrong length %d", len(hash))
 			continue
@@ -202,7 +201,7 @@ func (e *Engine) updateVNCServerAuth(vncAuth *mgmProto.VNCAuth) {
 		sessionPubKeys = append(sessionPubKeys, sshauth.SessionPubKey{
 			PubKey:      pub,
 			UserIDHash:  sshuserhash.UserIDHash(hash),
-			DisplayName: e.GetDisplayName(),
+			DisplayName: pk.GetDisplayName(),
 		})
 	}
 
@@ -236,7 +235,7 @@ func (e *Engine) stopVNCServer() error {
 		log.Warnf("cleanup VNC port redirection: %v", err)
 	}
 
-	if netstackNet := e.wgInterface.GetNet(); netstackNet != nil {
+	if e.wgInterface != nil && e.wgInterface.GetNet() != nil {
 		if registrar, ok := e.firewall.(interface {
 			UnregisterNetstackService(protocol nftypes.Protocol, port uint16)
 		}); ok {

client/ssh/proxy/proxy_test.go

@@ -28,10 +28,10 @@ import (
 
 	"github.com/netbirdio/netbird/client/proto"
 	nbssh "github.com/netbirdio/netbird/client/ssh"
-	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	"github.com/netbirdio/netbird/client/ssh/server"
 	"github.com/netbirdio/netbird/client/ssh/testutil"
 	nbjwt "github.com/netbirdio/netbird/shared/auth/jwt"
+	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	sshuserhash "github.com/netbirdio/netbird/shared/sshauth"
 )
 

client/ssh/server/jwt_test.go

@@ -23,11 +23,11 @@ import (
 	"github.com/stretchr/testify/require"
 
 	nbssh "github.com/netbirdio/netbird/client/ssh"
-	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	"github.com/netbirdio/netbird/client/ssh/client"
 	"github.com/netbirdio/netbird/client/ssh/detection"
 	"github.com/netbirdio/netbird/client/ssh/testutil"
 	nbjwt "github.com/netbirdio/netbird/shared/auth/jwt"
+	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	sshuserhash "github.com/netbirdio/netbird/shared/sshauth"
 )
 

client/ssh/server/server.go

@@ -23,10 +23,10 @@ import (
 	"golang.zx2c4.com/wireguard/tun/netstack"
 
 	"github.com/netbirdio/netbird/client/iface/wgaddr"
-	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	"github.com/netbirdio/netbird/client/ssh/detection"
 	"github.com/netbirdio/netbird/shared/auth"
 	"github.com/netbirdio/netbird/shared/auth/jwt"
+	sshauth "github.com/netbirdio/netbird/shared/sessionauth"
 	"github.com/netbirdio/netbird/util/netrelay"
 	"github.com/netbirdio/netbird/version"
 )

client/ui/approval.go

@@ -4,7 +4,10 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os"
+	"os/exec"
 	"strings"
 	"time"
 
@@ -17,6 +20,18 @@ import (
 	"github.com/netbirdio/netbird/client/proto"
 )
 
+// Approval metadata that is remote-peer or dashboard controlled is passed to
+// the forked netbird-ui via environment variables rather than argv, so it is
+// not exposed to other local users through ps.
+const (
+	envApprovalInitiator      = "NB_APPROVAL_INITIATOR"
+	envApprovalPeerName       = "NB_APPROVAL_PEER_NAME"
+	envApprovalSourceIP       = "NB_APPROVAL_SOURCE_IP"
+	envApprovalUsername       = "NB_APPROVAL_USERNAME"
+	envApprovalKeyFingerprint = "NB_APPROVAL_KEY_FINGERPRINT"
+	envApprovalSubject        = "NB_APPROVAL_SUBJECT"
+)
+
 // handleApprovalEvent forks a netbird-ui child process to render the
 // dialog on its own fyne main loop. Top-level windows opened from a
 // background goroutine of the tray process don't render reliably on
@@ -31,18 +46,56 @@ func (s *serviceClient) handleApprovalEvent(ev *proto.SystemEvent) {
 		log.Warnf("approval event missing request_id: %v", ev.Metadata)
 		return
 	}
+
+	// Only the request id, kind, and deadline stay on argv: they are
+	// daemon-issued and non-sensitive. The remote-influenced fields go
+	// through the child's environment.
 	args := []string{
 		"--approval-request-id=" + requestID,
 		"--approval-kind=" + ev.Metadata["kind"],
-		"--approval-initiator=" + ev.Metadata["initiator"],
-		"--approval-peer-name=" + ev.Metadata["peer_name"],
-		"--approval-source-ip=" + ev.Metadata["source_ip"],
-		"--approval-username=" + ev.Metadata["username"],
 		"--approval-expires-at=" + ev.Metadata["expires_at"],
-		"--approval-key-fingerprint=" + ev.Metadata["peer_pubkey"],
-		"--approval-subject=" + ev.UserMessage,
 	}
-	go s.eventHandler.runSelfCommand(s.ctx, "approval", args...)
+	env := append(os.Environ(),
+		envApprovalInitiator+"="+ev.Metadata["initiator"],
+		envApprovalPeerName+"="+ev.Metadata["peer_name"],
+		envApprovalSourceIP+"="+ev.Metadata["source_ip"],
+		envApprovalUsername+"="+ev.Metadata["username"],
+		envApprovalKeyFingerprint+"="+ev.Metadata["peer_pubkey"],
+		envApprovalSubject+"="+ev.UserMessage,
+	)
+	go s.runApprovalCommand(s.ctx, env, args)
+}
+
+// runApprovalCommand forks netbird-ui to render the approval dialog,
+// inheriting the parent environment plus the approval-specific variables. It
+// mirrors runSelfCommand but sets cmd.Env so the sensitive metadata never
+// appears on the child's argv.
+func (s *serviceClient) runApprovalCommand(ctx context.Context, env, args []string) {
+	proc, err := os.Executable()
+	if err != nil {
+		log.Errorf("get executable path: %v", err)
+		return
+	}
+
+	cmdArgs := append([]string{"--approval=true", "--daemon-addr=" + s.addr}, args...)
+	cmd := exec.CommandContext(ctx, proc, cmdArgs...)
+	cmd.Env = env
+
+	if out := s.attachOutput(cmd); out != nil {
+		defer func() {
+			if err := out.Close(); err != nil {
+				log.Errorf("close log file %s: %v", s.logFile, err)
+			}
+		}()
+	}
+
+	log.Printf("running approval command: %s", cmd.String())
+	if err := cmd.Run(); err != nil {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
+			log.Printf("approval command failed with exit code %d", exitErr.ExitCode())
+		}
+	}
 }
 
 // showApprovalUI runs the dialog on the forked process's fyne main loop

client/ui/client_ui.go

@@ -101,13 +101,13 @@ func main() {
 		approvalRequest: approvalRequest{
 			requestID:      flags.approvalRequestID,
 			kind:           flags.approvalKind,
-			initiator:      flags.approvalInitiator,
-			peerName:       flags.approvalPeerName,
-			sourceIP:       flags.approvalSourceIP,
-			username:       flags.approvalUsername,
-			subject:        flags.approvalSubject,
+			initiator:      os.Getenv(envApprovalInitiator),
+			peerName:       os.Getenv(envApprovalPeerName),
+			sourceIP:       os.Getenv(envApprovalSourceIP),
+			username:       os.Getenv(envApprovalUsername),
+			subject:        os.Getenv(envApprovalSubject),
 			expiresAt:      flags.approvalExpiresAt,
-			keyFingerprint: flags.approvalKeyFingerprint,
+			keyFingerprint: os.Getenv(envApprovalKeyFingerprint),
 		},
 	})
 
@@ -154,15 +154,9 @@ type cliFlags struct {
 	showUpdateVersion string
 	showApproval      bool
 
-	approvalRequestID      string
-	approvalKind           string
-	approvalInitiator      string
-	approvalPeerName       string
-	approvalSourceIP       string
-	approvalUsername       string
-	approvalSubject        string
-	approvalExpiresAt      string
-	approvalKeyFingerprint string
+	approvalRequestID string
+	approvalKind      string
+	approvalExpiresAt string
 }
 
 // parseFlags reads and returns all needed command-line flags.
@@ -187,13 +181,7 @@ func parseFlags() *cliFlags {
 	flag.BoolVar(&flags.showApproval, "approval", false, "show inbound-connection approval prompt window")
 	flag.StringVar(&flags.approvalRequestID, "approval-request-id", "", "approval prompt: daemon-issued request id")
 	flag.StringVar(&flags.approvalKind, "approval-kind", "", "approval prompt: subsystem kind (vnc, ssh, ...)")
-	flag.StringVar(&flags.approvalInitiator, "approval-initiator", "", "approval prompt: display name of the user who initiated the connection")
-	flag.StringVar(&flags.approvalPeerName, "approval-peer-name", "", "approval prompt: remote peer FQDN")
-	flag.StringVar(&flags.approvalSourceIP, "approval-source-ip", "", "approval prompt: remote source IP")
-	flag.StringVar(&flags.approvalUsername, "approval-username", "", "approval prompt: requested OS username")
-	flag.StringVar(&flags.approvalSubject, "approval-subject", "", "approval prompt: human-readable subject line")
 	flag.StringVar(&flags.approvalExpiresAt, "approval-expires-at", "", "approval prompt: RFC3339 deadline at which the daemon auto-denies")
-	flag.StringVar(&flags.approvalKeyFingerprint, "approval-key-fingerprint", "", "approval prompt: hex-encoded Noise static pubkey of the connecting client")
 	flag.Parse()
 	return &flags
 }

client/vnc/ports.go

@@ -11,9 +11,9 @@ package vnc
 // sockets; kept here so packet captures from older builds still get
 // tagged, and so any future on-wire agent variant has a reserved port.
 const (
-	ExternalPort     uint16 = 5900
-	InternalPort     uint16 = 25900
-	AgentLegacyPort  uint16 = 15900
+	ExternalPort    uint16 = 5900
+	InternalPort    uint16 = 25900
+	AgentLegacyPort uint16 = 15900
 )
 
 // WellKnownPorts is the unordered set of ports a packet capture should

client/vnc/server/agent_peercred_windows.go

@@ -6,14 +6,26 @@ import (
 	"net"
 )
 
-// validateAgentPeer is a best-effort no-op on Windows: AF_UNIX sockets on
-// Windows do not expose SO_PEERCRED equivalents, and both the daemon and
-// the spawned agent run as SYSTEM in distinct sessions. The remaining
-// trust comes from the location of the socket file (under
-// C:\Windows\Temp, writable only by SYSTEM/Administrators) and from the
-// per-spawn auth token preamble that follows this call. Documented as a
-// known gap; a future hardening pass could interrogate the connected
-// pipe's PID via process-token APIs.
+// validateAgentPeer is a documented no-op on Windows. AF_UNIX on Windows
+// exposes no SO_PEERCRED equivalent and no supported API to recover the
+// peer process from an accepted AF_UNIX connection, so the daemon cannot
+// match the connected peer against the agent PID it spawned the way the
+// darwin path does via LOCAL_PEERCRED. The Windows trust model therefore
+// rests on three other measures, none of which assume the socket path is
+// secret:
+//
+//   - the socket lives in a dedicated directory (agentSocketDir) created
+//     with a DACL granting only SYSTEM and Administrators, so an
+//     unprivileged local user cannot create or squat a socket there;
+//   - each spawn uses a cryptographically random socket name, so the path
+//     is unguessable before the agent binds it;
+//   - the daemon publishes the path only after confirming the spawned
+//     agent is listening (see waitForAgentListening), and gates every
+//     connection on the per-spawn auth-token preamble that follows this
+//     call.
+//
+// If a future Windows release exposes peer-PID retrieval for AF_UNIX,
+// this function should verify the peer against the spawned agent PID.
 func validateAgentPeer(_ net.Conn, _ uint32) error {
 	return nil
 }

client/vnc/server/agent_windows.go

@@ -4,10 +4,14 @@ package server
 
 import (
 	"context"
+	crand "crypto/rand"
 	"encoding/binary"
+	"encoding/hex"
 	"errors"
 	"fmt"
+	"net"
 	"os"
+	"path/filepath"
 	"runtime"
 	"sync"
 	"time"
@@ -362,10 +366,33 @@ type sessionManager struct {
 	jobHandle windows.Handle
 }
 
-// agentSocketPathFmt parameterizes the per-session agent socket path by
-// the Windows session id. C:\Windows\Temp is writable to both the daemon
-// (SYSTEM) and the spawned agent (SYSTEM token impersonating the session).
-const agentSocketPathFmt = `C:\Windows\Temp\netbird-vnc-%d.sock`
+const (
+	// agentSocketDir is a dedicated subdirectory under C:\Windows\Temp that
+	// the daemon creates with a restrictive DACL (SYSTEM + Administrators
+	// only). The default ACL on C:\Windows\Temp grants BUILTIN\Users
+	// create-file rights, so the agent socket must not live directly there:
+	// an unprivileged local user could pre-create a predictable path and
+	// intercept the daemon→agent stream. Both the daemon and the agent run
+	// as SYSTEM, so a SYSTEM-write-only directory is sufficient.
+	agentSocketDir = `C:\Windows\Temp\netbird-vnc`
+
+	// agentSocketDirSDDL grants full access to Local System (SY) and the
+	// Builtin Administrators group (BA) only, with the DACL protected
+	// (P) from inheritance so the parent's BUILTIN\Users grant does not
+	// flow in. AI is omitted; PAI marks the DACL protected and auto-
+	// inherited entries cleared.
+	agentSocketDirSDDL = "D:PAI(A;;FA;;;SY)(A;;FA;;;BA)"
+
+	// agentSocketRandomLen is the number of random bytes mixed into each
+	// per-spawn socket name so the path is unguessable before the agent
+	// owns it.
+	agentSocketRandomLen = 16
+
+	// agentReadyTimeout bounds how long the daemon waits for the freshly
+	// spawned agent to bind and accept on its socket before treating the
+	// spawn as failed.
+	agentReadyTimeout = 5 * time.Second
+)
 
 func newSessionManager() *sessionManager {
 	m := &sessionManager{sessionID: ^uint32(0), done: make(chan struct{})}
@@ -427,11 +454,14 @@ func createKillOnCloseJob() (windows.Handle, error) {
 
 // Resolve returns the current agent socket path, shared token, and the
 // uid the agent runs under (0 on Windows since the agent runs as
-// SYSTEM in the interactive session; validateAgentPeer is a no-op
-// there). When no agent is spawned yet (initial boot, between session
-// switches, or permanently disabled when SE_TCB_NAME is missing) it
-// surfaces a distinct error so the daemon can reject the connection
-// with a meaningful message instead of timing out the proxy dial.
+// SYSTEM in the interactive session; see validateAgentPeer for the
+// Windows trust model). The path is only published after the spawned
+// agent is confirmed listening, so a caller never receives a socket a
+// squatter could be holding. When no agent is spawned yet (initial
+// boot, between session switches, or permanently disabled when
+// SE_TCB_NAME is missing) it surfaces a distinct error so the daemon
+// can reject the connection with a meaningful message instead of timing
+// out the proxy dial.
 func (m *sessionManager) Resolve(_ context.Context) (string, string, uint32, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -547,13 +577,21 @@ func (m *sessionManager) maybeSpawnAgent(sid uint32) bool {
 	if m.agentProc != 0 || sid == 0xFFFFFFFF || !time.Now().After(m.nextSpawnAt) {
 		return true
 	}
-	// Reap any orphan still holding the agent port from a previous
-	// service instance, only on our very first spawn. Once we own
-	// an agent, we manage its lifecycle ourselves and never need to
-	// kill an unknown listener; if a kill+respawn races on port
-	// release, the spawn-failure backoff handles it without forcing
-	// a synchronous wait or duplicate kill.
-	socketPath := fmt.Sprintf(agentSocketPathFmt, sid)
+
+	if err := ensureAgentSocketDir(); err != nil {
+		log.Warnf("prepare agent socket dir: %v", err)
+		m.nextSpawnAt = time.Now().Add(5 * time.Second)
+		return true
+	}
+
+	// The leaf name carries a cryptographically random component so a local
+	// user cannot pre-create the path at a guessable location. The session
+	// id is kept for diagnostics only; security does not rely on it.
+	socketPath, err := newAgentSocketPath(sid)
+	if err != nil {
+		log.Warnf("generate agent socket path: %v", err)
+		return true
+	}
 	// Covers a previous-run crash that escaped Job Object kill-on-close.
 	if err := os.Remove(socketPath); err != nil && !os.IsNotExist(err) {
 		log.Debugf("clear stale agent socket %s: %v", socketPath, err)
@@ -563,12 +601,8 @@ func (m *sessionManager) maybeSpawnAgent(sid uint32) bool {
 		log.Warnf("generate agent auth token: %v", err)
 		return true
 	}
-	m.authToken = token
-	m.socketPath = socketPath
-	h, err := spawnAgentInSession(sid, socketPath, m.authToken, m.jobHandle)
+	h, err := spawnAgentInSession(sid, socketPath, token, m.jobHandle)
 	if err != nil {
-		m.authToken = ""
-		m.socketPath = ""
 		if errors.Is(err, windows.ERROR_PRIVILEGE_NOT_HELD) {
 			// SE_TCB_NAME (token-impersonation across sessions) is only
 			// granted to SYSTEM. Without it spawnAgent will fail every 2
@@ -579,12 +613,97 @@ func (m *sessionManager) maybeSpawnAgent(sid uint32) bool {
 		log.Warnf("spawn agent in session %d: %v", sid, err)
 		return true
 	}
+
+	// Gate on listen-readiness before publishing the path: do not hand a
+	// caller a socket the agent has not bound yet. On timeout, fail closed
+	// by killing the agent and leaving socketPath/authToken unset so
+	// Resolve keeps returning errAgentNotReady.
+	if err := waitForAgentListening(socketPath, agentReadyTimeout); err != nil {
+		log.Warnf("agent in session %d did not start listening: %v", sid, err)
+		_ = windows.TerminateProcess(h, 1)
+		_ = windows.CloseHandle(h)
+		if rmErr := os.Remove(socketPath); rmErr != nil && !os.IsNotExist(rmErr) {
+			log.Debugf("clear unready agent socket %s: %v", socketPath, rmErr)
+		}
+		m.scheduleNextSpawn(0, 0)
+		return true
+	}
+
+	m.authToken = token
+	m.socketPath = socketPath
 	m.agentProc = h
 	m.agentStartedAt = time.Now()
 	m.everSpawned = true
 	return true
 }
 
+// ensureAgentSocketDir creates the dedicated socket directory with a
+// restrictive DACL (SYSTEM + Administrators only). A pre-existing directory
+// is torn down and recreated rather than reused: it may have been created by
+// an unprivileged user with a permissive ACL, and it only ever holds our
+// transient sockets, so removing it loses nothing. Fails closed: returns an
+// error if the directory cannot be created with the intended security.
+func ensureAgentSocketDir() error {
+	sd, err := windows.SecurityDescriptorFromString(agentSocketDirSDDL)
+	if err != nil {
+		return fmt.Errorf("parse socket dir SDDL: %w", err)
+	}
+	var sa windows.SecurityAttributes
+	sa.Length = uint32(unsafe.Sizeof(sa))
+	sa.SecurityDescriptor = sd
+
+	dirW, err := windows.UTF16PtrFromString(agentSocketDir)
+	if err != nil {
+		return fmt.Errorf("encode socket dir path: %w", err)
+	}
+	err = windows.CreateDirectory(dirW, &sa)
+	if errors.Is(err, windows.ERROR_ALREADY_EXISTS) {
+		if rmErr := os.RemoveAll(agentSocketDir); rmErr != nil {
+			return fmt.Errorf("remove pre-existing socket dir %s: %w", agentSocketDir, rmErr)
+		}
+		err = windows.CreateDirectory(dirW, &sa)
+	}
+	if err != nil {
+		return fmt.Errorf("create socket dir %s: %w", agentSocketDir, err)
+	}
+	return nil
+}
+
+// newAgentSocketPath returns a per-spawn socket path inside the secured
+// socket directory. The leaf name mixes a cryptographically random component
+// with the session id (for diagnostics) so the path is unguessable before the
+// agent binds it.
+func newAgentSocketPath(sessionID uint32) (string, error) {
+	b := make([]byte, agentSocketRandomLen)
+	if _, err := crand.Read(b); err != nil {
+		return "", fmt.Errorf("read random: %w", err)
+	}
+	name := fmt.Sprintf("netbird-vnc-%d-%s.sock", sessionID, hex.EncodeToString(b))
+	return filepath.Join(agentSocketDir, name), nil
+}
+
+// waitForAgentListening dials the agent's Unix socket until it answers or the
+// timeout elapses. Mirrors the darwin readiness gate so the daemon never
+// exposes a socket path before the legitimate agent owns it.
+func waitForAgentListening(socketPath string, wait time.Duration) error {
+	var d net.Dialer
+	deadline := time.Now().Add(wait)
+	var lastErr error
+	for time.Now().Before(deadline) {
+		c, err := d.Dial("unix", socketPath)
+		if err == nil {
+			_ = c.Close()
+			return nil
+		}
+		lastErr = err
+		time.Sleep(100 * time.Millisecond)
+	}
+	if lastErr == nil {
+		lastErr = fmt.Errorf("timeout")
+	}
+	return fmt.Errorf("dial %s: %w", socketPath, lastErr)
+}
+
 func (m *sessionManager) killAgent() {
 	if m.agentProc == 0 {
 		return

client/vnc/server/capture_darwin.go

@@ -204,10 +204,11 @@ func (c *CGCapturer) Width() int { return c.w }
 // Height returns the screen height.
 func (c *CGCapturer) Height() int { return c.h }
 
-// Capture returns the current screen as an RGBA image.
 // CaptureInto writes a fresh frame directly into dst, skipping the
-// per-frame image.RGBA allocation that Capture() does. Returns
-// errFrameUnchanged when the screen hash matches the prior call.
+// per-frame image.RGBA allocation that Capture() does. It always fills
+// dst: the capturer is shared across all sessions, so dedup here would
+// starve every consumer but the first one to poll after a change.
+// Per-session prevFrame diffing in the session layer handles no-op frames.
 func (c *CGCapturer) CaptureInto(dst *image.RGBA) error {
 	cgImage := cgDisplayCreateImage(c.displayID)
 	if cgImage == 0 {
@@ -233,12 +234,6 @@ func (c *CGCapturer) CaptureInto(dst *image.RGBA) error {
 		return fmt.Errorf("empty image data")
 	}
 	src := unsafe.Slice((*byte)(unsafe.Pointer(dataPtr)), dataLen)
-	hash := maphash.Bytes(c.hashSeed, src)
-	if c.hasHash && hash == c.lastHash {
-		return errFrameUnchanged
-	}
-	c.lastHash = hash
-	c.hasHash = true
 
 	ds := c.downscale
 	if ds < 1 {
@@ -565,14 +560,7 @@ func (p *MacPoller) CaptureInto(dst *image.RGBA) error {
 	if err := p.ensureCapturerLocked(); err != nil {
 		return err
 	}
-	err := p.capturer.CaptureInto(dst)
-	if errors.Is(err, errFrameUnchanged) {
-		// Caller (session) treats this as "no change"; the dst buffer
-		// keeps its prior contents from the previous capture cycle so
-		// the diff stays meaningful.
-		return err
-	}
-	if err != nil {
+	if err := p.capturer.CaptureInto(dst); err != nil {
 		p.capturer = nil
 		return fmt.Errorf("macos capture: %w", err)
 	}

client/vnc/server/capture_fb_unix.go

@@ -1,4 +1,4 @@
-//go:build unix && !darwin && !ios && !android
+//go:build (linux && !android) || freebsd
 
 package server
 

client/vnc/server/capture_x11.go

@@ -1,4 +1,4 @@
-//go:build unix && !darwin && !ios && !android
+//go:build (linux && !android) || freebsd
 
 package server
 

client/vnc/server/copyrect.go

@@ -191,6 +191,16 @@ func (d *copyRectDetector) extractCopyRectTiles(cur *image.RGBA, dirtyTiles [][4
 	for _, r := range dirtyTiles {
 		if r[2] == ts && r[3] == ts {
 			if sx, sy, ok := d.findTileMatch(cur, r[0], r[1]); ok {
+				// The client applies moves sequentially against its live
+				// framebuffer. If this move's source overlaps the
+				// destination of any move already queued, that destination
+				// has overwritten the source pixels client-side, so the
+				// copy would read corrupted data. Drop it and let the tile
+				// fall through to normal pixel encoding instead.
+				if tileOverlapsPriorDst(moves, sx, sy, ts) {
+					remaining = append(remaining, r)
+					continue
+				}
 				moves = append(moves, copyRectMove{
 					srcX: sx, srcY: sy, dstX: r[0], dstY: r[1],
 				})
@@ -201,3 +211,18 @@ func (d *copyRectDetector) extractCopyRectTiles(cur *image.RGBA, dirtyTiles [][4
 	}
 	return moves, remaining
 }
+
+// tileOverlapsPriorDst reports whether the tileSize-square source rectangle
+// at (srcX, srcY) intersects the destination rectangle of any move already
+// emitted. All move rectangles are ts×ts, so the test reduces to a
+// per-axis distance check.
+func tileOverlapsPriorDst(moves []copyRectMove, srcX, srcY, ts int) bool {
+	for _, m := range moves {
+		dx := srcX - m.dstX
+		dy := srcY - m.dstY
+		if dx > -ts && dx < ts && dy > -ts && dy < ts {
+			return true
+		}
+	}
+	return false
+}

client/vnc/server/copyrect_test.go

@@ -83,6 +83,69 @@ func TestCopyRectDetector_DetectsVerticalScroll(t *testing.T) {
 	}
 }
 
+// rectsOverlap reports whether two ts×ts tiles at the given origins overlap.
+func tilesOverlap(ax, ay, bx, by, ts int) bool {
+	return ax < bx+ts && bx < ax+ts && ay < by+ts && by < ay+ts
+}
+
+// TestCopyRectDetector_DownwardScrollNoOverlap exercises a downward scroll,
+// where each move's source is the destination of the move one row above it.
+// Emitting all of them in order would corrupt the client framebuffer because
+// the earlier move overwrites the source pixels the later move reads. The
+// detector must drop any move whose source overlaps a prior move's
+// destination and route that tile to pixel encoding instead.
+func TestCopyRectDetector_DownwardScrollNoOverlap(t *testing.T) {
+	const w, h = 256, 192 // 4×3 tiles at 64px
+	const ts = 64
+
+	prev := image.NewRGBA(image.Rect(0, 0, w, h))
+	cur := image.NewRGBA(image.Rect(0, 0, w, h))
+
+	// prev: 12 tiles each with a unique colour.
+	for ty := 0; ty < 3; ty++ {
+		for tx := 0; tx < 4; tx++ {
+			fillTile(prev, tx*ts, ty*ts, ts, byte(tx*40), byte(ty*60), 0x80)
+		}
+	}
+	// cur: scroll downward by one row. Rows 1 and 2 are copied from prev
+	// rows 0 and 1; the top row is new content.
+	for ty := 1; ty < 3; ty++ {
+		for tx := 0; tx < 4; tx++ {
+			copyTile(cur, prev, tx*ts, (ty-1)*ts, tx*ts, ty*ts, ts)
+		}
+	}
+	for tx := 0; tx < 4; tx++ {
+		fillTile(cur, tx*ts, 0, ts, 0xff, 0xff, 0xff)
+	}
+
+	d := newCopyRectDetector(ts)
+	d.rebuild(prev, w, h)
+
+	tiles := diffTiles(prev, cur, w, h, ts)
+	wantTiles := len(tiles)
+	moves, remaining := d.extractCopyRectTiles(cur, tiles)
+
+	// No move's source may overlap an earlier move's destination.
+	for i, m := range moves {
+		for _, prior := range moves[:i] {
+			if tilesOverlap(m.srcX, m.srcY, prior.dstX, prior.dstY, ts) {
+				t.Fatalf("move %d src (%d,%d) overlaps prior dst (%d,%d)",
+					i, m.srcX, m.srcY, prior.dstX, prior.dstY)
+			}
+		}
+	}
+
+	// The dropped row-2 moves must fall through to pixel encoding rather than
+	// being silently skipped, so the region still updates correctly.
+	if len(moves)+len(remaining) != wantTiles {
+		t.Fatalf("moves(%d)+remaining(%d) != dirty tiles(%d): a tile was lost",
+			len(moves), len(remaining), wantTiles)
+	}
+	if len(moves) != 4 {
+		t.Fatalf("moves: want 4 (top scrolled row only), got %d", len(moves))
+	}
+}
+
 func TestCopyRectDetector_RejectsSelfMatch(t *testing.T) {
 	const w, h = 128, 128
 	const ts = 64

client/vnc/server/cursor_x11.go

@@ -1,4 +1,4 @@
-//go:build unix && !darwin && !ios && !android
+//go:build (linux && !android) || freebsd
 
 package server
 

client/vnc/server/handshake.go

@@ -3,7 +3,6 @@
 package server
 
 import (
-	"bufio"
 	"bytes"
 	"crypto/subtle"
 	"encoding/binary"
@@ -119,21 +118,35 @@ func (s *Server) readConnectionHeader(conn net.Conn) (*connectionHeader, error)
 		username = string(buf)
 	}
 
-	br := bufio.NewReader(conn)
-	clientStatic, identityVerified, err := s.maybeRunNoiseHandshake(conn, br, mode, username)
+	// Read the 4-byte magic candidate directly off the wire instead of
+	// buffering ahead with a bufio.Reader: the session reads the raw conn
+	// after this returns, so any bytes a bufio.Reader buffered past the
+	// header would be silently dropped. When the bytes aren't the v3 magic
+	// they are the start of the session_id field and feed straight into it.
+	var magicBuf [4]byte
+	if _, err := io.ReadFull(conn, magicBuf[:]); err != nil {
+		return &connectionHeader{mode: mode, username: username}, nil
+	}
+
+	clientStatic, identityVerified, magicConsumed, err := s.maybeRunNoiseHandshake(conn, magicBuf, mode, username)
 	if err != nil {
 		return nil, err
 	}
 
 	var sessionID uint32
-	var sidBuf [4]byte
-	if _, err := io.ReadFull(br, sidBuf[:]); err == nil {
-		sessionID = binary.BigEndian.Uint32(sidBuf[:])
+	var width, height uint16
+	if magicConsumed {
+		var sidBuf [4]byte
+		if _, err := io.ReadFull(conn, sidBuf[:]); err == nil {
+			sessionID = binary.BigEndian.Uint32(sidBuf[:])
+		}
+	} else {
+		// No magic: the 4 bytes we already read are the session_id.
+		sessionID = binary.BigEndian.Uint32(magicBuf[:])
 	}
 
-	var width, height uint16
 	var geomBuf [4]byte
-	if _, err := io.ReadFull(br, geomBuf[:]); err == nil {
+	if _, err := io.ReadFull(conn, geomBuf[:]); err == nil {
 		width = binary.BigEndian.Uint16(geomBuf[0:2])
 		height = binary.BigEndian.Uint16(geomBuf[2:4])
 	}
@@ -155,18 +168,14 @@ func (s *Server) readConnectionHeader(conn net.Conn) (*connectionHeader, error)
 // (fail closed). headerMode and headerUsername are mixed into the Noise
 // prologue so the client cannot lie in the cleartext header prefix
 // without making its own AEAD MAC verify-fail on the responder side.
-func (s *Server) maybeRunNoiseHandshake(conn net.Conn, br *bufio.Reader, headerMode byte, headerUsername string) ([]byte, bool, error) {
-	peek, _ := br.Peek(len(vncIdentityMagic))
-	if !bytes.Equal(peek, vncIdentityMagic) {
-		return nil, false, nil
-	}
-	if _, err := br.Discard(len(vncIdentityMagic)); err != nil {
-		return nil, false, fmt.Errorf("discard identity magic: %w", err)
+func (s *Server) maybeRunNoiseHandshake(conn net.Conn, magic [4]byte, headerMode byte, headerUsername string) (clientStatic []byte, identityVerified, magicConsumed bool, err error) {
+	if !bytes.Equal(magic[:], vncIdentityMagic) {
+		return nil, false, false, nil
 	}
 
 	msg1 := make([]byte, noiseInitiatorMsgLen)
-	if _, err := io.ReadFull(br, msg1); err != nil {
-		return nil, false, fmt.Errorf("read noise msg1: %w", err)
+	if _, err := io.ReadFull(conn, msg1); err != nil {
+		return nil, false, true, fmt.Errorf("read noise msg1: %w", err)
 	}
 
 	// Agents on loopback authenticate via the agent token, not this
@@ -179,11 +188,11 @@ func (s *Server) maybeRunNoiseHandshake(conn net.Conn, br *bufio.Reader, headerM
 	// short-circuit will see the truthful "no Noise identity proved
 	// here" rather than a stale true.
 	if s.disableAuth {
-		return nil, false, nil
+		return nil, false, true, nil
 	}
 
 	if len(s.identityKey) != 32 || len(s.identityPublic) != 32 {
-		return nil, false, errors.New("identity key not configured")
+		return nil, false, true, errors.New("identity key not configured")
 	}
 	state, err := noise.NewHandshakeState(noise.Config{
 		CipherSuite:   vncNoiseSuite,
@@ -193,27 +202,27 @@ func (s *Server) maybeRunNoiseHandshake(conn net.Conn, br *bufio.Reader, headerM
 		StaticKeypair: noise.DHKey{Private: s.identityKey, Public: s.identityPublic},
 	})
 	if err != nil {
-		return nil, false, fmt.Errorf("noise responder init: %w", err)
+		return nil, false, true, fmt.Errorf("noise responder init: %w", err)
 	}
 	if _, _, _, err := state.ReadMessage(nil, msg1); err != nil {
-		return nil, false, fmt.Errorf("noise read msg1: %w", err)
+		return nil, false, true, fmt.Errorf("noise read msg1: %w", err)
 	}
 	msg2, _, _, err := state.WriteMessage(nil, nil)
 	if err != nil {
-		return nil, false, fmt.Errorf("noise write msg2: %w", err)
+		return nil, false, true, fmt.Errorf("noise write msg2: %w", err)
 	}
 	if len(msg2) != noiseResponderMsgLen {
-		return nil, false, fmt.Errorf("noise responder produced %d bytes, expected %d", len(msg2), noiseResponderMsgLen)
+		return nil, false, true, fmt.Errorf("noise responder produced %d bytes, expected %d", len(msg2), noiseResponderMsgLen)
 	}
 	if _, err := conn.Write(msg2); err != nil {
-		return nil, false, fmt.Errorf("write noise msg2: %w", err)
+		return nil, false, true, fmt.Errorf("write noise msg2: %w", err)
 	}
 
-	clientStatic := state.PeerStatic()
-	if len(clientStatic) != 32 {
-		return nil, false, errors.New("noise peer static missing")
+	peerStatic := state.PeerStatic()
+	if len(peerStatic) != 32 {
+		return nil, false, true, errors.New("noise peer static missing")
 	}
-	return clientStatic, true, nil
+	return peerStatic, true, true, nil
 }
 
 // verifyAgentToken validates the agent token prefix when configured and

client/vnc/server/input_darwin.go

@@ -281,6 +281,8 @@ func releasePreventIdleSleep() {
 }
 
 func ensureEventSource() uintptr {
+	pmMu.Lock()
+	defer pmMu.Unlock()
 	if darwinEventSource != 0 {
 		return darwinEventSource
 	}

client/vnc/server/input_x11.go

@@ -1,4 +1,4 @@
-//go:build unix && !darwin && !ios && !android
+//go:build (linux && !android) || freebsd
 
 package server
 

client/vnc/server/metrics_conn.go

@@ -3,6 +3,7 @@
 package server
 
 import (
+	"encoding/binary"
 	"net"
 	"sync"
 	"sync/atomic"
@@ -169,26 +170,20 @@ func (m *metricsConn) BusyFraction() float64 {
 	return m.busyFraction
 }
 
-// isFBUHeader reports whether the given Write payload is the 4-byte
-// FramebufferUpdate header (message type 0, padding 0, rect-count high
-// byte). Rect bodies are written separately by sendDirtyAndMoves, so the
-// FBU/rect boundary lines up with Write boundaries.
-func isFBUHeader(p []byte) bool {
-	return len(p) == 4 && p[0] == serverFramebufferUpdate
+// startsFBU reports whether the Write payload begins a FramebufferUpdate
+// message (message type byte 0). This holds both for the standalone 4-byte
+// header that sendDirtyAndMoves writes before its rect bodies and for the
+// single framed Write that sendFullUpdate / sendEmptyUpdate use to emit a
+// whole FBU (header plus body) at once. Either way the FBU boundary lines
+// up with this Write boundary.
+func startsFBU(p []byte) bool {
+	return len(p) >= 1 && p[0] == serverFramebufferUpdate
 }
 
 func (m *metricsConn) Write(p []byte) (int, error) {
-	if isFBUHeader(p) {
-		if b := m.fbuBytes.Swap(0); b > 0 {
-			if b > m.maxFBUBytes.Load() {
-				m.maxFBUBytes.Store(b)
-			}
-		}
-		if r := m.fbuRects.Swap(0); r > 0 {
-			if r > m.maxFBURects.Load() {
-				m.maxFBURects.Store(r)
-			}
-		}
+	fbuStart := startsFBU(p)
+	if fbuStart {
+		m.flushFBUMax()
 		m.fbus.Add(1)
 	}
 
@@ -197,28 +192,41 @@ func (m *metricsConn) Write(p []byte) (int, error) {
 	m.writeNanos.Add(uint64(time.Since(t0).Nanoseconds()))
 	m.bytesOut.Add(uint64(n))
 	m.writes.Add(1)
-	if !isFBUHeader(p) {
-		m.fbuBytes.Add(uint64(n))
-		m.fbuRects.Add(1)
+
+	m.fbuBytes.Add(uint64(n))
+	if fbuStart {
+		// Rect count is carried in bytes 2:3 of the FBU header. A standalone
+		// header records it here; the rect bodies that follow only add bytes.
+		if len(p) >= 4 {
+			m.fbuRects.Add(uint64(binary.BigEndian.Uint16(p[2:4])))
+		}
 	}
+
 	if uint64(n) > m.largestPkt.Load() {
 		m.largestPkt.Store(uint64(n))
 	}
 	return n, err
 }
 
+// flushFBUMax folds the bytes and rects accumulated for the FBU that just
+// ended into the per-tick high-water marks, then resets the accumulators
+// for the next FBU.
+func (m *metricsConn) flushFBUMax() {
+	if b := m.fbuBytes.Swap(0); b > m.maxFBUBytes.Load() {
+		m.maxFBUBytes.Store(b)
+	}
+	if r := m.fbuRects.Swap(0); r > m.maxFBURects.Load() {
+		m.maxFBURects.Store(r)
+	}
+}
+
 func (m *metricsConn) Close() error {
 	m.closeOnce.Do(func() {
 		close(m.done)
 		if m.recorder == nil {
 			return
 		}
-		if b := m.fbuBytes.Swap(0); b > m.maxFBUBytes.Load() {
-			m.maxFBUBytes.Store(b)
-		}
-		if r := m.fbuRects.Swap(0); r > m.maxFBURects.Load() {
-			m.maxFBURects.Store(r)
-		}
+		m.flushFBUMax()
 		m.flushTick(true)
 	})
 	return m.Conn.Close()

client/vnc/server/server_test.go

@@ -406,7 +406,7 @@ func TestGateApproval_Disabled_NoApproverCall(t *testing.T) {
 	header := &connectionHeader{mode: ModeAttach}
 
 	_, err := srv.gateApproval(conn, header)
-		allowed := err == nil
+	allowed := err == nil
 	assert.True(t, allowed, "gate must pass through when requireApproval is false")
 	assert.Equal(t, int32(0), app.calls.Load(), "approver must not be called when disabled")
 }
@@ -475,7 +475,7 @@ func TestGateApproval_ApproverDenies(t *testing.T) {
 
 			header := &connectionHeader{mode: ModeAttach}
 			_, err := srv.gateApproval(conn, header)
-		allowed := err == nil
+			allowed := err == nil
 			assert.False(t, allowed, "approver error %v must deny", tc.err)
 			assert.Equal(t, int32(1), app.calls.Load())
 		})
@@ -493,7 +493,7 @@ func TestGateApproval_ApproverAccepts(t *testing.T) {
 
 	header := &connectionHeader{mode: ModeAttach, username: "alice"}
 	_, err := srv.gateApproval(conn, header)
-		allowed := err == nil
+	allowed := err == nil
 	assert.True(t, allowed, "approver returning nil must let the gate pass")
 	assert.Equal(t, int32(1), app.calls.Load())
 	assert.Equal(t, "alice", app.lastIn.Username, "header username must reach the approver")
@@ -515,7 +515,7 @@ func TestGateApproval_PassesPubKeyHex(t *testing.T) {
 	}
 	header := &connectionHeader{mode: ModeAttach, clientStatic: pub}
 	_, err := srv.gateApproval(conn, header)
-		allowed := err == nil
+	allowed := err == nil
 	assert.True(t, allowed)
 	assert.Equal(t, hex.EncodeToString(pub), app.lastIn.PeerPubKey)
 }

client/vnc/server/server_x11.go

@@ -1,4 +1,4 @@
-//go:build unix && !darwin && !ios && !android
+//go:build (linux && !android) || freebsd
 
 package server
 

client/vnc/server/session.go

@@ -20,6 +20,12 @@ const (
 	maxCutTextBytes = 1 << 20 // 1 MiB
 )
 
+// handshakeDeadline bounds the RFB handshake exchange (version, security,
+// ClientInit). Without it an authenticated peer can park a connection
+// between the connection-header deadlines and messageLoop's own deadline,
+// pinning a connSem slot.
+const handshakeDeadline = 10 * time.Second
+
 const tileSize = 64 // pixels per tile for dirty-rect detection
 
 // fullFramePromoteNum/Den trigger full-frame encoding when the dirty area
@@ -48,9 +54,12 @@ const (
 )
 
 type session struct {
-	conn        net.Conn
-	capturer    ScreenCapturer
-	injector    InputInjector
+	conn     net.Conn
+	capturer ScreenCapturer
+	injector InputInjector
+	// serverW and serverH are the current framebuffer dimensions. The
+	// encoder goroutine updates them on resize while the message loop reads
+	// them for pointer scaling, so both accesses are guarded by encMu.
 	serverW     int
 	serverH     int
 	desktopName string
@@ -200,6 +209,11 @@ func (s *session) serve() {
 }
 
 func (s *session) handshake() error {
+	if err := s.conn.SetDeadline(time.Now().Add(handshakeDeadline)); err != nil {
+		return fmt.Errorf("set handshake deadline: %w", err)
+	}
+	defer s.conn.SetDeadline(time.Time{}) //nolint:errcheck
+
 	// Send protocol version.
 	if _, err := io.WriteString(s.conn, rfbProtocolVersion); err != nil {
 		return fmt.Errorf("send version: %w", err)
@@ -540,38 +554,6 @@ func (s *session) handleFBUpdateRequest() error {
 	return nil
 }
 
-// SendDesktopName pushes a DesktopName pseudo-encoded update to the
-// client if it advertised support. Lets the client keep its window title
-// 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()
-	if !supported {
-		s.desktopName = name
-		return nil
-	}
-	s.desktopName = name
-	header := make([]byte, 4)
-	header[0] = serverFramebufferUpdate
-	binary.BigEndian.PutUint16(header[2:4], 1)
-
-	body := encodeDesktopNameBody(name)
-	s.writeMu.Lock()
-	defer s.writeMu.Unlock()
-	if _, err := s.conn.Write(header); err != nil {
-		return err
-	}
-	if _, err := s.conn.Write(body); err != nil {
-		return err
-	}
-	return nil
-}
-
 func (s *session) handleKeyEvent() error {
 	var data [7]byte
 	if _, err := io.ReadFull(s.conn, data[:]); err != nil {
@@ -639,7 +621,10 @@ func (s *session) handlePointerEvent() error {
 	s.lastPointerX = x
 	s.lastPointerY = y
 	s.pointerMu.Unlock()
-	s.injector.InjectPointer(mask, x, y, s.serverW, s.serverH)
+	s.encMu.RLock()
+	w, h := s.serverW, s.serverH
+	s.encMu.RUnlock()
+	s.injector.InjectPointer(mask, x, y, w, h)
 	return nil
 }
 
@@ -673,5 +658,8 @@ func (s *session) releaseStickyInput() {
 	s.pointerMu.Lock()
 	x, y := s.lastPointerX, s.lastPointerY
 	s.pointerMu.Unlock()
-	s.injector.InjectPointer(0, x, y, s.serverW, s.serverH)
+	s.encMu.RLock()
+	w, h := s.serverW, s.serverH
+	s.encMu.RUnlock()
+	s.injector.InjectPointer(0, x, y, w, h)
 }

client/vnc/server/session_encode.go

@@ -257,8 +257,10 @@ func (s *session) handleResize() error {
 		return nil
 	}
 	s.log.Debugf("framebuffer resized: %dx%d -> %dx%d", s.serverW, s.serverH, w, h)
+	s.encMu.Lock()
 	s.serverW = w
 	s.serverH = h
+	s.encMu.Unlock()
 	// Drop the prev frame so the next encode produces a full update at
 	// the new dimensions rather than diffing against a stale-sized buffer.
 	s.prevFrame = nil
@@ -405,7 +407,7 @@ func promoteToBoundingBox(rects [][4]int) ([][4]int, bool) {
 	if bbox < bboxPromoteMinArea {
 		return nil, false
 	}
-	if dirty*100 < bbox*bboxPromoteDensityPct {
+	if int64(dirty)*100 < int64(bbox)*bboxPromoteDensityPct {
 		return nil, false
 	}
 	return [][4]int{{x0, y0, w, h}}, true
@@ -423,7 +425,7 @@ func (s *session) shouldPromoteToFullFrame(rects [][4]int) bool {
 	for _, r := range rects {
 		dirty += r[2] * r[3]
 	}
-	return dirty*fullFramePromoteDen > s.serverW*s.serverH*fullFramePromoteNum
+	return int64(dirty)*fullFramePromoteDen > int64(s.serverW)*int64(s.serverH)*fullFramePromoteNum
 }
 
 // swapPrevCur makes the just-encoded frame the new prevFrame (for the next

client/vnc/server/virtual_x11.go

@@ -1,4 +1,4 @@
-//go:build unix && !darwin && !ios && !android
+//go:build (linux && !android) || freebsd
 
 package server
 

client/vnc/server/xauth_x11.go

@@ -1,4 +1,4 @@
-//go:build unix && !darwin && !ios && !android
+//go:build (linux && !android) || freebsd
 
 package server
 

client/wasm/cmd/main.go

@@ -409,12 +409,13 @@ func createGenerateVNCSessionKeyMethod() js.Func {
 
 // createVNCProxyMethod creates the VNC proxy method for raw TCP-over-WebSocket bridging.
 // JS signature: createVNCProxy(hostname, port, mode?, username?, keySessionID?, sessionID?, width?, height?, peerPublicKey?)
-//   mode:           "attach" (default) or "session"
-//   username:       required when mode is "session"
-//   keySessionID:   handle for the wasm-resident session keypair minted by netbirdGenerateVNCSessionKey
-//   sessionID:      Windows session ID (0 = console/auto)
-//   width/height:   requested viewport size for session mode (0 = server default)
-//   peerPublicKey:  base64 X25519 static pubkey of the destination peer (required for auth)
+//
+//	mode:           "attach" (default) or "session"
+//	username:       required when mode is "session"
+//	keySessionID:   handle for the wasm-resident session keypair minted by netbirdGenerateVNCSessionKey
+//	sessionID:      Windows session ID (0 = console/auto)
+//	width/height:   requested viewport size for session mode (0 = server default)
+//	peerPublicKey:  base64 X25519 static pubkey of the destination peer (required for auth)
 func createVNCProxyMethod(client *netbird.Client) js.Func {
 	return js.FuncOf(func(_ js.Value, args []js.Value) any {
 		params, err := parseVNCProxyArgs(args)

client/wasm/internal/vnc/proxy.go

@@ -21,6 +21,12 @@ import (
 
 var cryptoRandRead = crand.Read
 
+// proxyIDCounter is process-unique across every createVNCProxy call so each
+// proxy/connection registers a distinct global handler name. A per-proxy
+// counter would restart at 1 for every new VNCProxy, letting a reconnect's
+// cleanup delete the new proxy's handler.
+var proxyIDCounter atomic.Uint64
+
 // vncIdentityMagic mirrors the server side in client/vnc/server/server.go.
 var vncIdentityMagic = []byte("NBV3")
 
@@ -115,7 +121,7 @@ type vncNBClient interface {
 }
 
 type VNCProxy struct {
-	nbClient vncNBClient
+	nbClient          vncNBClient
 	activeConnections map[string]*vncConnection
 	destinations      map[string]vncDestination
 	// pendingHandlers holds the js.Func for handleVNCWebSocket_<id> between
@@ -123,19 +129,18 @@ type VNCProxy struct {
 	// vncConnection for later release.
 	pendingHandlers map[string]js.Func
 	mu              sync.Mutex
-	nextID          atomic.Uint64
 }
 
 type vncDestination struct {
-	address      string
-	mode         byte
-	username     string
-	sessionPriv  []byte
-	sessionPub   []byte
-	sessionID    uint32
-	width        uint16
-	height       uint16
-	peerPubKey []byte
+	address     string
+	mode        byte
+	username    string
+	sessionPriv []byte
+	sessionPub  []byte
+	sessionID   uint32
+	width       uint16
+	height      uint16
+	peerPubKey  []byte
 }
 
 type vncConnection struct {
@@ -152,6 +157,10 @@ type vncConnection struct {
 	wsHandlerFn js.Func
 	onMessageFn js.Func
 	onCloseFn   js.Func
+	// writeQueue carries inbound WS payloads to a single writer goroutine so
+	// vncConn.Write calls stay serialized in arrival order.
+	writeQueue  chan []byte
+	cleanupOnce sync.Once
 }
 
 // NewVNCProxy creates a new VNC proxy.
@@ -253,7 +262,7 @@ func (p *VNCProxy) newProxyPromise(address, mode, username string, dest vncDesti
 		go func() {
 			defer executor.Release()
 
-			proxyID := fmt.Sprintf("vnc_proxy_%d", p.nextID.Add(1))
+			proxyID := fmt.Sprintf("vnc_proxy_%d", proxyIDCounter.Add(1))
 
 			p.mu.Lock()
 			if p.destinations == nil {
@@ -309,6 +318,7 @@ func (p *VNCProxy) handleWebSocketConnection(ws js.Value, proxyID string) {
 		ctx:         ctx,
 		cancel:      cancel,
 		wsHandlerFn: handlerFn,
+		writeQueue:  make(chan []byte, 256),
 	}
 
 	p.mu.Lock()
@@ -326,8 +336,7 @@ func (p *VNCProxy) setupWebSocketHandlers(ws js.Value, conn *vncConnection) {
 		if len(args) < 1 {
 			return nil
 		}
-		data := args[0]
-		go p.handleWebSocketMessage(conn, data)
+		p.enqueueWebSocketMessage(conn, args[0])
 		return nil
 	})
 	ws.Set("onGoMessage", conn.onMessageFn)
@@ -340,7 +349,12 @@ func (p *VNCProxy) setupWebSocketHandlers(ws js.Value, conn *vncConnection) {
 	ws.Set("onGoClose", conn.onCloseFn)
 }
 
-func (p *VNCProxy) handleWebSocketMessage(conn *vncConnection, data js.Value) {
+// enqueueWebSocketMessage copies an inbound WS payload into Go memory and
+// hands it to the writer goroutine in arrival order. JS onmessage events are
+// delivered single-threaded on the event loop, so copying here preserves
+// stream order. When the queue is full the connection is torn down rather
+// than dropping bytes, which would corrupt the RFB stream.
+func (p *VNCProxy) enqueueWebSocketMessage(conn *vncConnection, data js.Value) {
 	if !data.InstanceOf(js.Global().Get("Uint8Array")) {
 		return
 	}
@@ -349,16 +363,30 @@ func (p *VNCProxy) handleWebSocketMessage(conn *vncConnection, data js.Value) {
 	buf := make([]byte, length)
 	js.CopyBytesToGo(buf, data)
 
-	conn.mu.Lock()
-	vncConn := conn.vncConn
-	conn.mu.Unlock()
-
-	if vncConn == nil {
-		return
+	select {
+	case <-conn.ctx.Done():
+	case conn.writeQueue <- buf:
+	default:
+		log.Debugf("VNC write queue full for %s; closing connection", conn.id)
+		conn.cancel()
 	}
+}
 
-	if _, err := vncConn.Write(buf); err != nil {
-		log.Debugf("write to VNC server: %v", err)
+// writeQueueLoop drains the ordered write queue and performs the blocking
+// vncConn.Write sequentially, serializing WS→TCP writes. It exits when the
+// connection context is cancelled.
+func (p *VNCProxy) writeQueueLoop(conn *vncConnection, vncConn net.Conn) {
+	for {
+		select {
+		case <-conn.ctx.Done():
+			return
+		case buf := <-conn.writeQueue:
+			if _, err := vncConn.Write(buf); err != nil {
+				log.Debugf("write to VNC server: %v", err)
+				conn.cancel()
+				return
+			}
+		}
 	}
 }
 
@@ -394,9 +422,10 @@ func (p *VNCProxy) connectToVNC(conn *vncConnection) {
 		return
 	}
 
-	// WS→TCP is handled by the onGoMessage handler set in setupWebSocketHandlers,
-	// which writes directly to the VNC connection as data arrives from JS.
-	// Only the TCP→WS direction needs a read loop here.
+	// WS→TCP payloads are enqueued in arrival order by the onGoMessage handler
+	// and drained sequentially by a single writer goroutine, keeping the RFB
+	// stream ordered. The TCP→WS direction has its own read loop.
+	go p.writeQueueLoop(conn, vncConn)
 	go p.forwardConnToWS(conn)
 
 	<-conn.ctx.Done()
@@ -573,33 +602,47 @@ func (p *VNCProxy) sendToWebSocket(conn *vncConnection, data []byte) {
 }
 
 func (p *VNCProxy) cleanupConnection(conn *vncConnection) {
-	log.Debugf("cleaning up VNC connection %s", conn.id)
-	conn.cancel()
+	conn.cleanupOnce.Do(func() {
+		log.Debugf("cleaning up VNC connection %s", conn.id)
+		conn.cancel()
 
-	conn.mu.Lock()
-	vncConn := conn.vncConn
-	conn.vncConn = nil
-	conn.mu.Unlock()
+		conn.mu.Lock()
+		vncConn := conn.vncConn
+		conn.vncConn = nil
+		conn.mu.Unlock()
 
-	if vncConn != nil {
-		if err := vncConn.Close(); err != nil {
-			log.Debugf("close VNC connection: %v", err)
+		if vncConn != nil {
+			if err := vncConn.Close(); err != nil {
+				log.Debugf("close VNC connection: %v", err)
+			}
 		}
-	}
 
-	// Remove the global JS handler registered in CreateProxy.
-	globalName := fmt.Sprintf("handleVNCWebSocket_%s", conn.id)
-	js.Global().Delete(globalName)
+		// Remove the global JS handler registered in CreateProxy.
+		js.Global().Delete(fmt.Sprintf("handleVNCWebSocket_%s", conn.id))
 
-	// Release all js.Func handles; js.FuncOf pins the Go closure and the
-	// allocations it captures until Release is called.
-	conn.wsHandlerFn.Release()
-	conn.onMessageFn.Release()
-	conn.onCloseFn.Release()
+		// Detach before releasing so a late WS event surfaces as a TypeError
+		// instead of calling a released js.Func and panicking the runtime.
+		if conn.wsHandlers.Truthy() {
+			conn.wsHandlers.Set("onGoMessage", js.Undefined())
+			conn.wsHandlers.Set("onGoClose", js.Undefined())
+		}
 
-	p.mu.Lock()
-	delete(p.activeConnections, conn.id)
-	delete(p.destinations, conn.id)
-	delete(p.pendingHandlers, conn.id)
-	p.mu.Unlock()
+		// wsHandlerFn is the zero js.Func when the pendingHandlers lookup
+		// missed on a second connect.
+		if conn.wsHandlerFn.Truthy() {
+			conn.wsHandlerFn.Release()
+		}
+		if conn.onMessageFn.Truthy() {
+			conn.onMessageFn.Release()
+		}
+		if conn.onCloseFn.Truthy() {
+			conn.onCloseFn.Release()
+		}
+
+		p.mu.Lock()
+		delete(p.activeConnections, conn.id)
+		delete(p.destinations, conn.id)
+		delete(p.pendingHandlers, conn.id)
+		p.mu.Unlock()
+	})
 }

management/server/http/handlers/peers/temporary_access_permission_test.go

@@ -2,6 +2,7 @@ package peers
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
@@ -34,7 +35,7 @@ func TestCreateTemporaryAccess_RejectsCallerWithoutPeersCreate(t *testing.T) {
 	// nil so the test fails loudly if the handler tries to call it.
 	permMgr.EXPECT().
 		ValidateUserPermissions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Eq(modules.Peers), gomock.Eq(operations.Create)).
-		Return(false, nil).
+		Return(false, context.Background(), nil).
 		Times(1)
 
 	h := &Handler{
@@ -74,11 +75,11 @@ func TestCreateTemporaryAccess_RejectsCallerWithoutPoliciesCreate(t *testing.T)
 
 	permMgr.EXPECT().
 		ValidateUserPermissions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Eq(modules.Peers), gomock.Eq(operations.Create)).
-		Return(true, nil).
+		Return(true, context.Background(), nil).
 		Times(1)
 	permMgr.EXPECT().
 		ValidateUserPermissions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Eq(modules.Policies), gomock.Eq(operations.Create)).
-		Return(false, nil).
+		Return(false, context.Background(), nil).
 		Times(1)
 
 	h := &Handler{

management/server/peer/peer.go

@@ -138,7 +138,6 @@ type Flags struct {
 	DisableIPv6         bool
 
 	LazyConnectionEnabled bool
-
 }
 
 // PeerSystemMeta is a metadata of a Peer machine system

management/server/store/sql_store.go

@@ -2536,7 +2536,7 @@ func (s *SqlStore) getPolicyRules(ctx context.Context, policyIDs []string) ([]*t
 	if len(policyIDs) == 0 {
 		return nil, nil
 	}
-	const query = `SELECT id, policy_id, name, description, enabled, action, destinations, destination_resource, sources, source_resource, bidirectional, protocol, ports, port_ranges, authorized_groups, authorized_user FROM policy_rules WHERE policy_id = ANY($1)`
+	const query = `SELECT id, policy_id, name, description, enabled, action, destinations, destination_resource, sources, source_resource, bidirectional, protocol, ports, port_ranges, authorized_groups, authorized_user, session_pub_key, session_display_name FROM policy_rules WHERE policy_id = ANY($1)`
 	rows, err := s.pool.Query(ctx, query, policyIDs)
 	if err != nil {
 		return nil, err
@@ -2545,8 +2545,8 @@ func (s *SqlStore) getPolicyRules(ctx context.Context, policyIDs []string) ([]*t
 		var r types.PolicyRule
 		var dest, destRes, sources, sourceRes, ports, portRanges, authorizedGroups []byte
 		var enabled, bidirectional sql.NullBool
-		var authorizedUser sql.NullString
-		err := row.Scan(&r.ID, &r.PolicyID, &r.Name, &r.Description, &enabled, &r.Action, &dest, &destRes, &sources, &sourceRes, &bidirectional, &r.Protocol, &ports, &portRanges, &authorizedGroups, &authorizedUser)
+		var authorizedUser, sessionPubKey, sessionDisplayName sql.NullString
+		err := row.Scan(&r.ID, &r.PolicyID, &r.Name, &r.Description, &enabled, &r.Action, &dest, &destRes, &sources, &sourceRes, &bidirectional, &r.Protocol, &ports, &portRanges, &authorizedGroups, &authorizedUser, &sessionPubKey, &sessionDisplayName)
 		if err == nil {
 			if enabled.Valid {
 				r.Enabled = enabled.Bool
@@ -2578,6 +2578,12 @@ func (s *SqlStore) getPolicyRules(ctx context.Context, policyIDs []string) ([]*t
 			if authorizedUser.Valid {
 				r.AuthorizedUser = authorizedUser.String
 			}
+			if sessionPubKey.Valid {
+				r.SessionPubKey = sessionPubKey.String
+			}
+			if sessionDisplayName.Valid {
+				r.SessionDisplayName = sessionDisplayName.String
+			}
 		}
 		return &r, err
 	})

management/server/types/account.go

@@ -14,7 +14,6 @@ import (
 	"github.com/rs/xid"
 	log "github.com/sirupsen/logrus"
 
-	auth "github.com/netbirdio/netbird/shared/sessionauth"
 	nbdns "github.com/netbirdio/netbird/dns"
 	proxydomain "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
 	"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
@@ -29,6 +28,7 @@ import (
 	"github.com/netbirdio/netbird/route"
 	"github.com/netbirdio/netbird/shared/management/domain"
 	"github.com/netbirdio/netbird/shared/management/status"
+	auth "github.com/netbirdio/netbird/shared/sessionauth"
 	"github.com/netbirdio/netbird/version"
 )
 
@@ -170,7 +170,6 @@ func (a *Account) GetGroup(groupID string) *Group {
 	return a.Groups[groupID]
 }
 
-
 func (a *Account) addNetworksRoutingPeers(
 	networkResourcesRoutes []*route.Route,
 	peer *nbpeer.Peer,

management/server/types/networkmap_components.go

@@ -9,13 +9,13 @@ import (
 	"strings"
 	"time"
 
-	auth "github.com/netbirdio/netbird/shared/sessionauth"
 	nbdns "github.com/netbirdio/netbird/dns"
 	resourceTypes "github.com/netbirdio/netbird/management/server/networks/resources/types"
 	routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types"
 	nbpeer "github.com/netbirdio/netbird/management/server/peer"
 	"github.com/netbirdio/netbird/route"
 	"github.com/netbirdio/netbird/shared/management/domain"
+	auth "github.com/netbirdio/netbird/shared/sessionauth"
 )
 
 type NetworkMapComponents struct {
@@ -109,7 +109,7 @@ func (c *NetworkMapComponents) Calculate(ctx context.Context) *NetworkMap {
 
 	peerGroups := c.GetPeerGroups(targetPeerID)
 
-	connRes := c.getPeerConnectionResources(targetPeerID)
+	connRes := c.getPeerConnectionResources(ctx, targetPeerID)
 	aclPeers := connRes.peers
 
 	peersToConnect, expiredPeers := c.filterPeersByLoginExpiration(aclPeers)
@@ -182,7 +182,7 @@ type peerConnectionResult struct {
 	sshEnabled         bool
 }
 
-func (c *NetworkMapComponents) getPeerConnectionResources(targetPeerID string) peerConnectionResult {
+func (c *NetworkMapComponents) getPeerConnectionResources(ctx context.Context, targetPeerID string) peerConnectionResult {
 	targetPeer := c.GetPeerInfo(targetPeerID)
 	if targetPeer == nil {
 		return peerConnectionResult{}
@@ -202,7 +202,7 @@ func (c *NetworkMapComponents) getPeerConnectionResources(targetPeerID string) p
 			if !rule.Enabled {
 				continue
 			}
-			c.applyPolicyRule(rule, policy.SourcePostureChecks, targetPeer, targetPeerID, generateResources, state)
+			c.applyPolicyRule(ctx, rule, policy.SourcePostureChecks, targetPeer, targetPeerID, generateResources, state)
 		}
 	}
 
@@ -218,6 +218,7 @@ func (c *NetworkMapComponents) getPeerConnectionResources(targetPeerID string) p
 }
 
 func (c *NetworkMapComponents) applyPolicyRule(
+	ctx context.Context,
 	rule *PolicyRule,
 	sourcePostureChecks []string,
 	targetPeer *nbpeer.Peer,
@@ -229,8 +230,12 @@ func (c *NetworkMapComponents) applyPolicyRule(
 	destinationPeers, peerInDestinations := c.resolveRuleEndpoint(rule.DestinationResource, rule.Destinations, targetPeerID, nil)
 
 	cb := ruleAuthCallbacks{
-		collectSSHUsers:   c.collectAuthorizedUsers,
-		collectVNCUsers:   c.collectAuthorizedUsers,
+		collectSSHUsers: func(r *PolicyRule, t map[string]map[string]struct{}) {
+			c.collectAuthorizedUsers(ctx, r, t)
+		},
+		collectVNCUsers: func(r *PolicyRule, t map[string]map[string]struct{}) {
+			c.collectAuthorizedUsers(ctx, r, t)
+		},
 		getAllowedUserIDs: c.getAllowedUserIDs,
 	}
 	applyResolvedRuleToState(rule, sourcePeers, destinationPeers, peerInSources, peerInDestinations, targetPeer.SSHEnabled, generateResources, cb, state)
@@ -249,10 +254,10 @@ func (c *NetworkMapComponents) resolveRuleEndpoint(
 }
 
 // collectAuthorizedUsers populates the target map with authorized user mappings from the rule.
-func (c *NetworkMapComponents) collectAuthorizedUsers(rule *PolicyRule, target map[string]map[string]struct{}) {
+func (c *NetworkMapComponents) collectAuthorizedUsers(ctx context.Context, rule *PolicyRule, target map[string]map[string]struct{}) {
 	switch {
 	case len(rule.AuthorizedGroups) > 0:
-		mergeAuthorizedGroupUsers(context.Background(), rule.AuthorizedGroups, c.GroupIDToUserIDs, target)
+		mergeAuthorizedGroupUsers(ctx, rule.AuthorizedGroups, c.GroupIDToUserIDs, target)
 	case rule.AuthorizedUser != "":
 		ensureWildcardUser(target, rule.AuthorizedUser)
 	default:

management/server/types/policy_authorized_users.go

@@ -6,8 +6,8 @@ import (
 
 	log "github.com/sirupsen/logrus"
 
-	auth "github.com/netbirdio/netbird/shared/sessionauth"
 	nbpeer "github.com/netbirdio/netbird/management/server/peer"
+	auth "github.com/netbirdio/netbird/shared/sessionauth"
 )
 
 // peerConnResolveState carries the in-progress maps mutated by per-rule

management/server/types/policy_authorized_users_security_test.go

@@ -1,6 +1,10 @@
 package types
 
-import "testing"
+import (
+	"testing"
+
+	nbpeer "github.com/netbirdio/netbird/management/server/peer"
+)
 
 // TestHandleVNCRule_BidirectionalDistributesPubkeyToSourcePeer covers the
 // latent bug where a bidirectional VNC rule used to drop the
@@ -83,3 +87,68 @@ func TestHandleVNCRule_DestinationAlwaysGetsPubkey(t *testing.T) {
 		t.Fatalf("expected 1 session pubkey for destination peer, got %d", len(state.vncSessionPubKeys))
 	}
 }
+
+// TestApplyResolvedRule_BidirectionalSSHEnablesSourcePeer locks the
+// bidirectional widening for netbird-ssh rules: a peer that appears only
+// in the rule's sources of a bidirectional SSH rule must get SSH enabled
+// and its authorized users collected, because the rule grants access in
+// both directions. A unidirectional rule must not do this for a
+// source-only peer.
+func TestApplyResolvedRule_BidirectionalSSHEnablesSourcePeer(t *testing.T) {
+	collected := false
+	cb := ruleAuthCallbacks{
+		collectSSHUsers: func(_ *PolicyRule, target map[string]map[string]struct{}) {
+			collected = true
+			target["local"] = map[string]struct{}{"user1": {}}
+		},
+	}
+	rule := &PolicyRule{
+		Protocol:      PolicyRuleProtocolNetbirdSSH,
+		Bidirectional: true,
+	}
+	state := &peerConnResolveState{
+		authorizedUsers:    make(map[string]map[string]struct{}),
+		vncAuthorizedUsers: make(map[string]map[string]struct{}),
+	}
+
+	applyResolvedRuleToState(rule, nil, nil, true /*peerInSources*/, false /*peerInDestinations*/, false, func(*PolicyRule, []*nbpeer.Peer, int) {}, cb, state)
+
+	if !state.sshEnabled {
+		t.Fatal("expected SSH enabled on source-side peer of bidirectional SSH rule")
+	}
+	if !collected {
+		t.Fatal("expected authorized users collected on source-side peer of bidirectional SSH rule")
+	}
+	if _, ok := state.authorizedUsers["local"]; !ok {
+		t.Fatal("expected authorized users map populated for source-side peer")
+	}
+}
+
+// TestApplyResolvedRule_UnidirectionalSSHSkipsSourcePeer is the negative
+// counterpart: a unidirectional SSH rule must not enable SSH for a peer
+// that appears only in sources.
+func TestApplyResolvedRule_UnidirectionalSSHSkipsSourcePeer(t *testing.T) {
+	collected := false
+	cb := ruleAuthCallbacks{
+		collectSSHUsers: func(_ *PolicyRule, _ map[string]map[string]struct{}) {
+			collected = true
+		},
+	}
+	rule := &PolicyRule{
+		Protocol:      PolicyRuleProtocolNetbirdSSH,
+		Bidirectional: false,
+	}
+	state := &peerConnResolveState{
+		authorizedUsers:    make(map[string]map[string]struct{}),
+		vncAuthorizedUsers: make(map[string]map[string]struct{}),
+	}
+
+	applyResolvedRuleToState(rule, nil, nil, true /*peerInSources*/, false /*peerInDestinations*/, false, func(*PolicyRule, []*nbpeer.Peer, int) {}, cb, state)
+
+	if state.sshEnabled {
+		t.Fatal("expected SSH NOT enabled on source-only peer of unidirectional SSH rule")
+	}
+	if collected {
+		t.Fatal("expected NO authorized users collected on source-only peer of unidirectional SSH rule")
+	}
+}

shared/sessionauth/auth.go

@@ -20,11 +20,11 @@ const (
 )
 
 var (
-	ErrEmptyUserID            = errors.New("JWT user ID is empty")
-	ErrUserNotAuthorized      = errors.New("user is not authorized to access this peer")
-	ErrNoMachineUserMapping   = errors.New("no authorization mapping for OS user")
-	ErrUserNotMappedToOSUser  = errors.New("user is not authorized to login as OS user")
-	ErrSessionKeyNotKnown = errors.New("session pubkey not registered")
+	ErrEmptyUserID           = errors.New("JWT user ID is empty")
+	ErrUserNotAuthorized     = errors.New("user is not authorized to access this peer")
+	ErrNoMachineUserMapping  = errors.New("no authorization mapping for OS user")
+	ErrUserNotMappedToOSUser = errors.New("user is not authorized to login as OS user")
+	ErrSessionKeyNotKnown    = errors.New("session pubkey not registered")
 )
 
 // Authorizer handles SSH fine-grained access control authorization
@@ -83,8 +83,8 @@ type SessionPubKey struct {
 // NewAuthorizer creates a new SSH authorizer with empty configuration
 func NewAuthorizer() *Authorizer {
 	a := &Authorizer{
-		userIDClaim:    DefaultUserIDClaim,
-		machineUsers:   make(map[string][]uint32),
+		userIDClaim:         DefaultUserIDClaim,
+		machineUsers:        make(map[string][]uint32),
 		sessionPubKeys:      make(map[[sessionPubKeyLen]byte]sshuserhash.UserIDHash),
 		sessionDisplayNames: make(map[[sessionPubKeyLen]byte]string),
 	}

util/capture/text.go

@@ -466,19 +466,19 @@ func isWellKnownVNCPort(p uint16) bool {
 // message-type recognitions fire only when length matches the fixed
 // size for that type, to avoid mis-tagging Noise handshake bytes.
 func annotateVNCClientToServer(p []byte) string {
-	if len(p) >= 10 && (p[0] == 0 || p[0] == 1) {
-		userLen := int(p[1])
-		// width and height are uint16 fields the dashboard often leaves
-		// zero (default). A header without an OS user has total length
-		// 10; with one, 10+userLen.
-		if 10+userLen <= len(p) {
+	if len(p) >= 11 && (p[0] == 0 || p[0] == 1) {
+		// Connection header layout: mode(1) + u16 BE username length +
+		// username(N), then sessionID(4) + width(2) + height(2). Prefix is
+		// 3+N, full header 11+N.
+		userLen := int(binary.BigEndian.Uint16(p[1:3]))
+		if 11+userLen <= len(p) {
 			mode := "attach"
 			if p[0] == 1 {
 				mode = "session"
 			}
 			tag := fmt.Sprintf("connect mode=%s", mode)
-			if userLen > 0 {
-				tag += fmt.Sprintf(" user(%d)", userLen)
+			if userLen > 0 && 3+userLen <= len(p) {
+				tag += fmt.Sprintf(" user(%s)", p[3:3+userLen])
 			}
 			return tag
 		}
@@ -528,9 +528,11 @@ func annotateVNCServerToClient(p []byte) string {
 
 // matchRFBSecurityFailure recognises the RFB 3.8 security-result body the
 // server sends when authentication or session setup fails. Format:
-//   byte 0  : 0x00 (security types count = 0 = failure)
-//   bytes 1-4: uint32 reason length
-//   bytes 5+: reason text
+//
+//	byte 0  : 0x00 (security types count = 0 = failure)
+//	bytes 1-4: uint32 reason length
+//	bytes 5+: reason text
+//
 // Returns the reason text and ok=true when the length self-checks.
 func matchRFBSecurityFailure(p []byte) (string, bool) {
 	if len(p) < 5 || p[0] != 0 {