From c4bc479df6b74b12edc316738ec43f86bbc6b119 Mon Sep 17 00:00:00 2001
From: Viktor Liu <viktor@netbird.io>
Date: Wed, 29 Jul 2026 09:47:21 +0200
Subject: [PATCH 1/5] Detect Windows privilege from the token and group
 membership instead of name matching

---
 .../ssh/server/command_execution_windows.go   |   4 +-
 client/ssh/server/privileges_other.go         |  15 ++
 client/ssh/server/privileges_windows.go       | 198 ++++++++++++++++
 client/ssh/server/privileges_windows_test.go  | 182 +++++++++++++++
 client/ssh/server/server_config_test.go       |  74 +++---
 client/ssh/server/sftp_windows.go             |   2 +-
 client/ssh/server/user_utils.go               | 213 +++++-------------
 client/ssh/server/user_utils_test.go          | 101 +++------
 client/ssh/server/userswitching_windows.go    |   6 +-
 9 files changed, 533 insertions(+), 262 deletions(-)
 create mode 100644 client/ssh/server/privileges_other.go
 create mode 100644 client/ssh/server/privileges_windows.go
 create mode 100644 client/ssh/server/privileges_windows_test.go

diff --git a/client/ssh/server/command_execution_windows.go b/client/ssh/server/command_execution_windows.go
index e1ba777f606..feb8daa26c7 100644
--- a/client/ssh/server/command_execution_windows.go
+++ b/client/ssh/server/command_execution_windows.go
@@ -243,7 +243,7 @@ func (s *Server) setUserEnvironmentVariables(envMap map[string]string, userProfi
 
 // prepareCommandEnv prepares environment variables for command execution on Windows
 func (s *Server) prepareCommandEnv(logger *log.Entry, localUser *user.User, session ssh.Session) []string {
-	username, domain := s.parseUsername(localUser.Username)
+	username, domain := parseUsername(localUser.Username)
 	userEnv, err := s.getUserEnvironment(logger, username, domain)
 	if err != nil {
 		log.Debugf("failed to get user environment for %s\\%s, using fallback: %v", domain, username, err)
@@ -383,7 +383,7 @@ func (s *Server) executeCommandWithPty(logger *log.Entry, session ssh.Session, _
 		return false
 	}
 
-	username, domain := s.parseUsername(localUser.Username)
+	username, domain := parseUsername(localUser.Username)
 	shell := getUserShell(localUser.Uid)
 
 	req := PtyExecutionRequest{
diff --git a/client/ssh/server/privileges_other.go b/client/ssh/server/privileges_other.go
new file mode 100644
index 00000000000..5aaa461b083
--- /dev/null
+++ b/client/ssh/server/privileges_other.go
@@ -0,0 +1,15 @@
+//go:build !windows
+
+package server
+
+// isProcessElevated is only meaningful on Windows; other platforms use the
+// effective UID check in isCurrentProcessPrivileged.
+func isProcessElevated() bool {
+	return false
+}
+
+// isWindowsAccountPrivileged is only reachable on Windows. Fail closed if it
+// is ever called on another platform.
+func isWindowsAccountPrivileged(string) bool {
+	return true
+}
diff --git a/client/ssh/server/privileges_windows.go b/client/ssh/server/privileges_windows.go
new file mode 100644
index 00000000000..82da4523177
--- /dev/null
+++ b/client/ssh/server/privileges_windows.go
@@ -0,0 +1,198 @@
+//go:build windows
+
+package server
+
+import (
+	"fmt"
+	"unsafe"
+
+	log "github.com/sirupsen/logrus"
+	"golang.org/x/sys/windows"
+)
+
+var (
+	netapi32                  = windows.NewLazySystemDLL("netapi32.dll")
+	procNetUserGetLocalGroups = netapi32.NewProc("NetUserGetLocalGroups")
+)
+
+const (
+	// lgIncludeIndirect makes NetUserGetLocalGroups also return local groups
+	// the user belongs to through a global group.
+	lgIncludeIndirect  = 0x1
+	maxPreferredLength = 0xFFFFFFFF
+)
+
+// localGroupUsersInfo0 mirrors LOCALGROUP_USERS_INFO_0.
+type localGroupUsersInfo0 struct {
+	name *uint16
+}
+
+// isProcessElevated reports whether the current process token is elevated
+// (TokenElevation): true for elevated administrators, the built-in
+// Administrator, administrators with UAC disabled, and SYSTEM; false for
+// standard users and administrators running with a UAC-filtered token.
+func isProcessElevated() bool {
+	return windows.GetCurrentProcessToken().IsElevated()
+}
+
+// isWindowsAccountPrivileged reports whether the account is privileged on this
+// machine: a well-known service account, a built-in Administrator (RID 500),
+// or a member of the local Administrators group, directly or through nested
+// groups. Evaluation errors count as privileged so policy checks fail closed.
+func isWindowsAccountPrivileged(username string) bool {
+	sid, _, _, err := windows.LookupSID("", username)
+	if err != nil {
+		log.Warnf("privilege check: SID lookup for %q failed, treating as privileged: %v", username, err)
+		return true
+	}
+
+	if isPrivilegedUserSID(sid) {
+		return true
+	}
+
+	member, err := isLocalAdminsMember(username)
+	if err != nil {
+		log.Warnf("privilege check: cannot determine Administrators membership for %q, treating as privileged: %v", username, err)
+		return true
+	}
+	return member
+}
+
+// isPrivilegedUserSID reports whether the SID itself identifies a privileged
+// principal, without consulting group membership.
+func isPrivilegedUserSID(sid *windows.SID) bool {
+	wellKnown := []windows.WELL_KNOWN_SID_TYPE{
+		windows.WinLocalSystemSid,
+		windows.WinLocalServiceSid,
+		windows.WinNetworkServiceSid,
+		windows.WinBuiltinAdministratorsSid,
+	}
+	for _, sidType := range wellKnown {
+		if sid.IsWellKnown(sidType) {
+			return true
+		}
+	}
+	return isBuiltinAdministratorSID(sid)
+}
+
+// isBuiltinAdministratorSID reports whether the SID is a machine or domain
+// built-in Administrator account (S-1-5-21-...-500). RID 500 is reserved for
+// that account; it can be renamed but cannot be removed from the
+// Administrators group.
+func isBuiltinAdministratorSID(sid *windows.SID) bool {
+	if sid.IdentifierAuthority() != windows.SECURITY_NT_AUTHORITY {
+		return false
+	}
+	count := sid.SubAuthorityCount()
+	if count < 2 || sid.SubAuthority(0) != 21 {
+		return false
+	}
+	return sid.SubAuthority(uint32(count-1)) == 500
+}
+
+// isLocalAdminsMember reports whether the account is a member of the local
+// Administrators group. Local accounts are checked against the local SAM,
+// which is authoritative for them. For domain accounts an S4U identification
+// token is preferred because its group list is LSA's transitive expansion
+// (nested and universal groups included); NetUserGetLocalGroups expands only
+// one global-group hop but needs no logon, so it serves as fallback.
+func isLocalAdminsMember(username string) (bool, error) {
+	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
+	if err != nil {
+		return false, fmt.Errorf("create Administrators SID: %w", err)
+	}
+
+	account, domain := parseUsername(username)
+	if NewPrivilegeDropper().isLocalUser(domain) {
+		return localGroupsContainSID(account, adminSid)
+	}
+
+	member, s4uErr := s4uTokenIsMember(account, domain, adminSid)
+	if s4uErr == nil {
+		return member, nil
+	}
+	log.Debugf("privilege check: S4U membership check for %q failed, falling back to local group enumeration: %v", username, s4uErr)
+
+	member, err = localGroupsContainSID(buildUserCpn(account, domain), adminSid)
+	if err != nil {
+		return false, fmt.Errorf("S4U check: %w; local group enumeration: %w", s4uErr, err)
+	}
+	return member, nil
+}
+
+// s4uTokenIsMember obtains an S4U token for the account and checks whether the
+// given SID is enabled in it.
+func s4uTokenIsMember(account, domain string, sid *windows.SID) (bool, error) {
+	token, err := generateS4UUserToken(log.NewEntry(log.StandardLogger()), account, domain)
+	if err != nil {
+		return false, err
+	}
+	defer func() {
+		if err := windows.CloseHandle(token); err != nil {
+			log.Debugf("close S4U token: %v", err)
+		}
+	}()
+	return windows.Token(token).IsMember(sid)
+}
+
+// localGroupsContainSID enumerates the local groups the account belongs to
+// (including membership through global groups) and compares each group's SID
+// against the wanted SID. Group names are resolved back to SIDs so the
+// comparison is not sensitive to localization.
+func localGroupsContainSID(username string, want *windows.SID) (bool, error) {
+	groups, err := netUserGetLocalGroups(username)
+	if err != nil {
+		return false, err
+	}
+	for _, group := range groups {
+		groupSid, _, _, err := windows.LookupSID("", group)
+		if err != nil {
+			log.Debugf("resolve local group %q to SID: %v", group, err)
+			continue
+		}
+		if groupSid.Equals(want) {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+// netUserGetLocalGroups returns the names of the local groups the account is a
+// member of, including indirect membership through global groups.
+func netUserGetLocalGroups(username string) ([]string, error) {
+	name16, err := windows.UTF16PtrFromString(username)
+	if err != nil {
+		return nil, fmt.Errorf("convert username: %w", err)
+	}
+
+	var buf *byte
+	var entriesRead, totalEntries uint32
+	status, _, _ := procNetUserGetLocalGroups.Call(
+		0, // local server
+		uintptr(unsafe.Pointer(name16)),
+		0, // level 0: LOCALGROUP_USERS_INFO_0
+		lgIncludeIndirect,
+		uintptr(unsafe.Pointer(&buf)),
+		maxPreferredLength,
+		uintptr(unsafe.Pointer(&entriesRead)),
+		uintptr(unsafe.Pointer(&totalEntries)),
+	)
+	if status != 0 {
+		return nil, fmt.Errorf("NetUserGetLocalGroups for %q: status %d", username, status)
+	}
+	if buf == nil {
+		return nil, nil
+	}
+	defer func() {
+		if err := windows.NetApiBufferFree(buf); err != nil {
+			log.Debugf("free NetApi buffer: %v", err)
+		}
+	}()
+
+	entries := unsafe.Slice((*localGroupUsersInfo0)(unsafe.Pointer(buf)), entriesRead)
+	groups := make([]string, 0, entriesRead)
+	for _, entry := range entries {
+		groups = append(groups, windows.UTF16PtrToString(entry.name))
+	}
+	return groups, nil
+}
diff --git a/client/ssh/server/privileges_windows_test.go b/client/ssh/server/privileges_windows_test.go
new file mode 100644
index 00000000000..45831dbe715
--- /dev/null
+++ b/client/ssh/server/privileges_windows_test.go
@@ -0,0 +1,182 @@
+//go:build windows
+
+package server
+
+import (
+	"os/user"
+	"testing"
+	"unsafe"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/windows"
+)
+
+// filterNormalAccount limits NetUserEnum to normal user accounts.
+const filterNormalAccount = 0x2
+
+// userInfo0 mirrors USER_INFO_0.
+type userInfo0 struct {
+	name *uint16
+}
+
+func mustParseSID(t *testing.T, s string) *windows.SID {
+	t.Helper()
+	sid, err := windows.StringToSid(s)
+	require.NoError(t, err, "parse SID %s", s)
+	return sid
+}
+
+func TestIsBuiltinAdministratorSID(t *testing.T) {
+	tests := []struct {
+		name string
+		sid  string
+		want bool
+	}{
+		{"machine_administrator", "S-1-5-21-1111111111-2222222222-3333333333-500", true},
+		{"domain_administrator", "S-1-5-21-3390233681-4087452608-412898826-500", true},
+		{"regular_user", "S-1-5-21-1111111111-2222222222-3333333333-1001", false},
+		{"guest_account", "S-1-5-21-1111111111-2222222222-3333333333-501", false},
+		{"domain_admins_group", "S-1-5-21-1111111111-2222222222-3333333333-512", false},
+		{"system", "S-1-5-18", false},
+		{"administrators_group", "S-1-5-32-544", false},
+		{"non_nt_authority", "S-1-1-0", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := isBuiltinAdministratorSID(mustParseSID(t, tt.sid))
+			assert.Equal(t, tt.want, result, "RID 500 detection for %s", tt.sid)
+		})
+	}
+}
+
+func TestIsPrivilegedUserSID(t *testing.T) {
+	tests := []struct {
+		name string
+		sid  string
+		want bool
+	}{
+		{"local_system", "S-1-5-18", true},
+		{"local_service", "S-1-5-19", true},
+		{"network_service", "S-1-5-20", true},
+		{"administrators_group", "S-1-5-32-544", true},
+		{"builtin_administrator", "S-1-5-21-1111111111-2222222222-3333333333-500", true},
+		{"regular_user", "S-1-5-21-1111111111-2222222222-3333333333-1001", false},
+		{"users_group", "S-1-5-32-545", false},
+		{"everyone", "S-1-1-0", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := isPrivilegedUserSID(mustParseSID(t, tt.sid))
+			assert.Equal(t, tt.want, result, "SID privilege classification for %s", tt.sid)
+		})
+	}
+}
+
+func TestIsWindowsAccountPrivileged(t *testing.T) {
+	tests := []struct {
+		name     string
+		username string
+		want     bool
+	}{
+		{"system", "NT AUTHORITY\\SYSTEM", true},
+		{"local_service", "NT AUTHORITY\\LOCAL SERVICE", true},
+		{"network_service", "NT AUTHORITY\\NETWORK SERVICE", true},
+		{"administrators_group_name", "BUILTIN\\Administrators", true},
+		// The built-in Administrator and Guest accounts exist on every
+		// Windows installation, though they may be disabled.
+		{"builtin_administrator", "Administrator", true},
+		{"guest", "Guest", false},
+		// Unresolvable accounts fail closed.
+		{"nonexistent_user", "netbird-no-such-user", true},
+		{"empty_username", "", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := isWindowsAccountPrivileged(tt.username)
+			assert.Equal(t, tt.want, result, "account privilege classification for %q", tt.username)
+		})
+	}
+}
+
+func TestIsProcessElevated(t *testing.T) {
+	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
+	require.NoError(t, err, "create Administrators SID")
+
+	// Token(0) makes CheckTokenMembership evaluate the caller's own token.
+	member, err := windows.Token(0).IsMember(adminSid)
+	require.NoError(t, err, "check own Administrators membership")
+
+	elevated := isProcessElevated()
+	t.Logf("member of Administrators: %v, token elevated: %v", member, elevated)
+
+	// An enabled Administrators SID in the token implies an elevated token.
+	// CI runs this test as SYSTEM, which satisfies both.
+	if member {
+		assert.True(t, elevated, "token with enabled Administrators membership must report elevated")
+	}
+}
+
+// TestS4UMembershipAgreesWithLocalGroups exercises the S4U token path used
+// for domain accounts. S4U logons need the TCB privilege, so the test runs
+// only as SYSTEM (which is how CI executes the suite). For local accounts the
+// token's Administrators membership must agree with the SAM enumeration.
+func TestS4UMembershipAgreesWithLocalGroups(t *testing.T) {
+	system, err := windows.CreateWellKnownSid(windows.WinLocalSystemSid)
+	require.NoError(t, err, "create SYSTEM SID")
+	current, err := user.Current()
+	require.NoError(t, err, "get current user")
+	if current.Uid != system.String() {
+		t.Skipf("S4U logon requires SYSTEM (running as %s)", current.Username)
+	}
+
+	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
+	require.NoError(t, err, "create Administrators SID")
+
+	var buf *byte
+	var entriesRead, totalEntries uint32
+	var resume uint32
+	err = windows.NetUserEnum(nil, 0, filterNormalAccount, &buf, maxPreferredLength,
+		&entriesRead, &totalEntries, &resume)
+	require.NoError(t, err, "enumerate local users")
+	defer func() {
+		require.NoError(t, windows.NetApiBufferFree(buf), "free NetApi buffer")
+	}()
+
+	names := unsafe.Slice((*userInfo0)(unsafe.Pointer(buf)), entriesRead)
+	checked := 0
+	for _, entry := range names {
+		name := windows.UTF16PtrToString(entry.name)
+
+		viaToken, err := s4uTokenIsMember(name, ".", adminSid)
+		if err != nil {
+			// Disabled or logon-restricted accounts cannot get an S4U logon.
+			t.Logf("skipping %s: %v", name, err)
+			continue
+		}
+		viaSAM, err := localGroupsContainSID(name, adminSid)
+		require.NoError(t, err, "enumerate local groups for %s", name)
+
+		assert.Equal(t, viaSAM, viaToken, "S4U token and SAM enumeration must agree on Administrators membership for %s", name)
+		checked++
+	}
+	t.Logf("checked %d local accounts via S4U", checked)
+}
+
+func TestLocalGroupsContainSID_Guest(t *testing.T) {
+	guestsSid, err := windows.CreateWellKnownSid(windows.WinBuiltinGuestsSid)
+	require.NoError(t, err, "create Guests SID")
+	adminsSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
+	require.NoError(t, err, "create Administrators SID")
+
+	inGuests, err := localGroupsContainSID("Guest", guestsSid)
+	require.NoError(t, err, "enumerate local groups for Guest")
+	assert.True(t, inGuests, "Guest should be a member of BUILTIN\\Guests")
+
+	inAdmins, err := localGroupsContainSID("Guest", adminsSid)
+	require.NoError(t, err, "enumerate local groups for Guest")
+	assert.False(t, inAdmins, "Guest should not be a member of BUILTIN\\Administrators")
+}
diff --git a/client/ssh/server/server_config_test.go b/client/ssh/server/server_config_test.go
index f70e29963dd..3ce0110b614 100644
--- a/client/ssh/server/server_config_test.go
+++ b/client/ssh/server/server_config_test.go
@@ -420,35 +420,17 @@ func TestServer_PortConflictHandling(t *testing.T) {
 
 func TestServer_IsPrivilegedUser(t *testing.T) {
 
-	tests := []struct {
+	type privilegedUserCase struct {
 		username    string
 		expected    bool
 		description string
-	}{
-		{
-			username:    "root",
-			expected:    true,
-			description: "root should be considered privileged",
-		},
-		{
-			username:    "regular",
-			expected:    false,
-			description: "regular user should not be privileged",
-		},
-		{
-			username:    "",
-			expected:    false,
-			description: "empty username should not be privileged",
-		},
 	}
+	var tests []privilegedUserCase
 
-	// Add Windows-specific tests
 	if runtime.GOOS == "windows" {
-		tests = append(tests, []struct {
-			username    string
-			expected    bool
-			description string
-		}{
+		// On Windows, accounts that cannot be resolved or evaluated count as
+		// privileged (fail closed), including nonexistent and empty names.
+		tests = []privilegedUserCase{
 			{
 				username:    "Administrator",
 				expected:    true,
@@ -459,20 +441,50 @@ func TestServer_IsPrivilegedUser(t *testing.T) {
 				expected:    true,
 				description: "administrator should be considered privileged on Windows (case insensitive)",
 			},
-		}...)
+			{
+				username:    "NT AUTHORITY\\SYSTEM",
+				expected:    true,
+				description: "SYSTEM should be considered privileged on Windows",
+			},
+			{
+				username:    "Guest",
+				expected:    false,
+				description: "Guest should not be privileged on Windows",
+			},
+			{
+				username:    "netbird-no-such-user",
+				expected:    true,
+				description: "unresolvable account should fail closed on Windows",
+			},
+			{
+				username:    "",
+				expected:    true,
+				description: "empty username should fail closed on Windows",
+			},
+		}
 	} else {
-		// On non-Windows systems, Administrator should not be privileged
-		tests = append(tests, []struct {
-			username    string
-			expected    bool
-			description string
-		}{
+		tests = []privilegedUserCase{
+			{
+				username:    "root",
+				expected:    true,
+				description: "root should be considered privileged",
+			},
+			{
+				username:    "regular",
+				expected:    false,
+				description: "regular user should not be privileged",
+			},
+			{
+				username:    "",
+				expected:    false,
+				description: "empty username should not be privileged",
+			},
 			{
 				username:    "Administrator",
 				expected:    false,
 				description: "Administrator should not be privileged on non-Windows systems",
 			},
-		}...)
+		}
 	}
 
 	for _, tt := range tests {
diff --git a/client/ssh/server/sftp_windows.go b/client/ssh/server/sftp_windows.go
index dc532b9e766..25cd1729899 100644
--- a/client/ssh/server/sftp_windows.go
+++ b/client/ssh/server/sftp_windows.go
@@ -17,7 +17,7 @@ import (
 // createSftpCommand creates a Windows SFTP command with user switching.
 // The caller must close the returned token handle after starting the process.
 func (s *Server) createSftpCommand(targetUser *user.User, sess ssh.Session) (*exec.Cmd, windows.Token, error) {
-	username, domain := s.parseUsername(targetUser.Username)
+	username, domain := parseUsername(targetUser.Username)
 
 	netbirdPath, err := os.Executable()
 	if err != nil {
diff --git a/client/ssh/server/user_utils.go b/client/ssh/server/user_utils.go
index bc2aa2d7d82..5ffa3c8a16b 100644
--- a/client/ssh/server/user_utils.go
+++ b/client/ssh/server/user_utils.go
@@ -16,11 +16,6 @@ var (
 	ErrPrivilegedUserSwitch = errors.New("cannot switch to privileged user - current user lacks required privileges")
 )
 
-// isPlatformUnix returns true for Unix-like platforms (Linux, macOS, etc.)
-func isPlatformUnix() bool {
-	return getCurrentOS() != "windows"
-}
-
 // Dependency injection variables for testing - allows mocking dynamic runtime checks
 var (
 	getCurrentUser         = currentUserWithGetent
@@ -29,6 +24,9 @@ var (
 	getIsProcessPrivileged = isCurrentProcessPrivileged
 
 	getEuid = os.Geteuid
+
+	getProcessElevated          = isProcessElevated
+	getWindowsAccountPrivileged = isWindowsAccountPrivileged
 )
 
 const (
@@ -65,6 +63,13 @@ type PrivilegeCheckResult struct {
 	RequiresUserSwitching bool
 }
 
+// privilegeCheckContext holds all context needed for privilege checking
+type privilegeCheckContext struct {
+	currentUser           *user.User
+	currentUserPrivileged bool
+	allowRoot             bool
+}
+
 // CheckPrivileges performs comprehensive privilege checking for all SSH features.
 // This is the single source of truth for privilege decisions across the SSH server.
 func (s *Server) CheckPrivileges(req PrivilegeCheckRequest) PrivilegeCheckResult {
@@ -175,6 +180,48 @@ func (s *Server) resolveRequestedUser(requestedUsername string) (*user.User, err
 	return u, nil
 }
 
+// SetAllowRootLogin configures root login access
+func (s *Server) SetAllowRootLogin(allow bool) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.allowRootLogin = allow
+}
+
+// userNameLookup performs user lookup with root login permission check
+func (s *Server) userNameLookup(username string) (*user.User, error) {
+	result := s.CheckPrivileges(PrivilegeCheckRequest{
+		RequestedUsername:         username,
+		FeatureSupportsUserSwitch: true,
+		FeatureName:               FeatureSSHLogin,
+	})
+
+	if !result.Allowed {
+		return nil, result.Error
+	}
+
+	return result.User, nil
+}
+
+// userPrivilegeCheck performs user lookup with full privilege check result
+func (s *Server) userPrivilegeCheck(username string) (PrivilegeCheckResult, error) {
+	result := s.CheckPrivileges(PrivilegeCheckRequest{
+		RequestedUsername:         username,
+		FeatureSupportsUserSwitch: true,
+		FeatureName:               FeatureSSHLogin,
+	})
+
+	if !result.Allowed {
+		return result, result.Error
+	}
+
+	return result, nil
+}
+
+// isPlatformUnix returns true for Unix-like platforms (Linux, macOS, etc.)
+func isPlatformUnix() bool {
+	return getCurrentOS() != "windows"
+}
+
 // isSameResolvedUser compares two resolved user identities
 func isSameResolvedUser(user1, user2 *user.User) bool {
 	if user1 == nil || user2 == nil {
@@ -183,13 +230,6 @@ func isSameResolvedUser(user1, user2 *user.User) bool {
 	return user1.Uid == user2.Uid
 }
 
-// privilegeCheckContext holds all context needed for privilege checking
-type privilegeCheckContext struct {
-	currentUser           *user.User
-	currentUserPrivileged bool
-	allowRoot             bool
-}
-
 // isSameUser checks if two usernames refer to the same user
 // SECURITY: This function must be conservative - it should only return true
 // when we're certain both usernames refer to the exact same user identity
@@ -253,159 +293,24 @@ func isWindowsSameUser(requestedUsername, currentUsername string) bool {
 	return strings.EqualFold(reqDomain, curDomain)
 }
 
-// SetAllowRootLogin configures root login access
-func (s *Server) SetAllowRootLogin(allow bool) {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	s.allowRootLogin = allow
-}
-
-// userNameLookup performs user lookup with root login permission check
-func (s *Server) userNameLookup(username string) (*user.User, error) {
-	result := s.CheckPrivileges(PrivilegeCheckRequest{
-		RequestedUsername:         username,
-		FeatureSupportsUserSwitch: true,
-		FeatureName:               FeatureSSHLogin,
-	})
-
-	if !result.Allowed {
-		return nil, result.Error
-	}
-
-	return result.User, nil
-}
-
-// userPrivilegeCheck performs user lookup with full privilege check result
-func (s *Server) userPrivilegeCheck(username string) (PrivilegeCheckResult, error) {
-	result := s.CheckPrivileges(PrivilegeCheckRequest{
-		RequestedUsername:         username,
-		FeatureSupportsUserSwitch: true,
-		FeatureName:               FeatureSSHLogin,
-	})
-
-	if !result.Allowed {
-		return result, result.Error
-	}
-
-	return result, nil
-}
-
 // isPrivilegedUsername checks if the given username represents a privileged user across platforms.
-// On Unix: root
-// On Windows: Administrator, SYSTEM (case-insensitive)
-// Handles domain-qualified usernames like "DOMAIN\Administrator" or "user@domain.com"
+// On Unix: root.
+// On Windows: well-known service accounts, built-in Administrator accounts,
+// and members of the local Administrators group; handles domain-qualified
+// usernames like "DOMAIN\user" or "user@domain.com".
 func isPrivilegedUsername(username string) bool {
 	if getCurrentOS() != "windows" {
 		return username == "root"
 	}
-
-	bareUsername := username
-	// Handle Windows domain format: DOMAIN\username
-	if idx := strings.LastIndex(username, `\`); idx != -1 {
-		bareUsername = username[idx+1:]
-	}
-	// Handle email-style format: username@domain.com
-	if idx := strings.Index(bareUsername, "@"); idx != -1 {
-		bareUsername = bareUsername[:idx]
-	}
-
-	return isWindowsPrivilegedUser(bareUsername)
-}
-
-// isWindowsPrivilegedUser checks if a bare username (domain already stripped) represents a Windows privileged account
-func isWindowsPrivilegedUser(bareUsername string) bool {
-	// common privileged usernames (case insensitive)
-	privilegedNames := []string{
-		"administrator",
-		"admin",
-		"root",
-		"system",
-		"localsystem",
-		"networkservice",
-		"localservice",
-	}
-
-	usernameLower := strings.ToLower(bareUsername)
-	for _, privilegedName := range privilegedNames {
-		if usernameLower == privilegedName {
-			return true
-		}
-	}
-
-	// computer accounts (ending with $) are not privileged by themselves
-	// They only gain privileges through group membership or specific SIDs
-
-	if targetUser, err := lookupUser(bareUsername); err == nil {
-		return isWindowsPrivilegedSID(targetUser.Uid)
-	}
-
-	return false
-}
-
-// isWindowsPrivilegedSID checks if a Windows SID represents a privileged account
-func isWindowsPrivilegedSID(sid string) bool {
-	privilegedSIDs := []string{
-		"S-1-5-18",     // Local System (SYSTEM)
-		"S-1-5-19",     // Local Service (NT AUTHORITY\LOCAL SERVICE)
-		"S-1-5-20",     // Network Service (NT AUTHORITY\NETWORK SERVICE)
-		"S-1-5-32-544", // Administrators group (BUILTIN\Administrators)
-		"S-1-5-500",    // Built-in Administrator account (local machine RID 500)
-	}
-
-	for _, privilegedSID := range privilegedSIDs {
-		if sid == privilegedSID {
-			return true
-		}
-	}
-
-	// Check for domain administrator accounts (RID 500 in any domain)
-	// Format: S-1-5-21-domain-domain-domain-500
-	// This is reliable as RID 500 is reserved for the domain Administrator account
-	if strings.HasPrefix(sid, "S-1-5-21-") && strings.HasSuffix(sid, "-500") {
-		return true
-	}
-
-	// Check for other well-known privileged RIDs in domain contexts
-	// RID 512 = Domain Admins group, RID 516 = Domain Controllers group
-	if strings.HasPrefix(sid, "S-1-5-21-") {
-		if strings.HasSuffix(sid, "-512") || // Domain Admins group
-			strings.HasSuffix(sid, "-516") || // Domain Controllers group
-			strings.HasSuffix(sid, "-519") { // Enterprise Admins group
-			return true
-		}
-	}
-
-	return false
+	return getWindowsAccountPrivileged(username)
 }
 
 // isCurrentProcessPrivileged checks if the current process is running with elevated privileges.
 // On Unix systems, this means running as root (UID 0).
-// On Windows, this means running as Administrator or SYSTEM.
+// On Windows, this means the process token is elevated (administrators, SYSTEM).
 func isCurrentProcessPrivileged() bool {
 	if getCurrentOS() == "windows" {
-		return isWindowsElevated()
+		return getProcessElevated()
 	}
 	return getEuid() == 0
 }
-
-// isWindowsElevated checks if the current process is running with elevated privileges on Windows
-func isWindowsElevated() bool {
-	currentUser, err := getCurrentUser()
-	if err != nil {
-		log.Errorf("failed to get current user for privilege check, assuming non-privileged: %v", err)
-		return false
-	}
-
-	if isWindowsPrivilegedSID(currentUser.Uid) {
-		log.Debugf("Windows user switching supported: running as privileged SID %s", currentUser.Uid)
-		return true
-	}
-
-	if isPrivilegedUsername(currentUser.Username) {
-		log.Debugf("Windows user switching supported: running as privileged username %s", currentUser.Username)
-		return true
-	}
-
-	log.Debugf("Windows user switching not supported: not running as privileged user (current: %s)", currentUser.Uid)
-	return false
-}
diff --git a/client/ssh/server/user_utils_test.go b/client/ssh/server/user_utils_test.go
index 637dc10d0a6..c734b194713 100644
--- a/client/ssh/server/user_utils_test.go
+++ b/client/ssh/server/user_utils_test.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os/user"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -27,8 +28,8 @@ func setupTestDependencies(currentUser *user.User, currentUserErr error, os stri
 	originalLookupUser := lookupUser
 	originalGetCurrentOS := getCurrentOS
 	originalGetEuid := getEuid
-
-	// Reset caches to ensure clean test state
+	originalGetProcessElevated := getProcessElevated
+	originalGetWindowsAccountPrivileged := getWindowsAccountPrivileged
 
 	// Set test values - inject platform dependencies
 	getCurrentUser = func() (*user.User, error) {
@@ -53,16 +54,31 @@ func setupTestDependencies(currentUser *user.User, currentUserErr error, os stri
 		return euid
 	}
 
-	// Mock privilege detection based on the test user
-	getIsProcessPrivileged = func() bool {
+	// Simulate the Windows token elevation check based on the fixture user:
+	// the built-in Administrator (RID 500) and SYSTEM run elevated.
+	getProcessElevated = func() bool {
 		if currentUser == nil {
 			return false
 		}
-		// Check both username and SID for Windows systems
-		if os == "windows" && isWindowsPrivilegedSID(currentUser.Uid) {
+		return currentUser.Uid == "S-1-5-18" || strings.HasSuffix(currentUser.Uid, "-500")
+	}
+
+	// Simulate the Windows account classifier for the fixture accounts.
+	// "root" does not exist on Windows; the real classifier fails closed on
+	// unresolvable accounts, so it counts as privileged here too.
+	getWindowsAccountPrivileged = func(username string) bool {
+		bare := username
+		if idx := strings.LastIndex(bare, `\`); idx != -1 {
+			bare = bare[idx+1:]
+		}
+		if idx := strings.Index(bare, "@"); idx != -1 {
+			bare = bare[:idx]
+		}
+		switch strings.ToLower(bare) {
+		case "administrator", "system", "root":
 			return true
 		}
-		return isPrivilegedUsername(currentUser.Username)
+		return false
 	}
 
 	// Return cleanup function
@@ -71,10 +87,8 @@ func setupTestDependencies(currentUser *user.User, currentUserErr error, os stri
 		lookupUser = originalLookupUser
 		getCurrentOS = originalGetCurrentOS
 		getEuid = originalGetEuid
-
-		getIsProcessPrivileged = isCurrentProcessPrivileged
-
-		// Reset caches after test
+		getProcessElevated = originalGetProcessElevated
+		getWindowsAccountPrivileged = originalGetWindowsAccountPrivileged
 	}
 }
 
@@ -421,6 +435,9 @@ func TestUsedFallback_MeansNoPrivilegeDropping(t *testing.T) {
 }
 
 func TestPrivilegedUsernameDetection(t *testing.T) {
+	// Windows classification is syscall-backed (SID resolution, group
+	// membership) and is covered by privileges_windows_test.go; here only the
+	// Unix logic and the platform dispatch are exercised.
 	tests := []struct {
 		name       string
 		username   string
@@ -432,25 +449,9 @@ func TestPrivilegedUsernameDetection(t *testing.T) {
 		{"unix_regular_user", "alice", "linux", false},
 		{"unix_root_capital", "Root", "linux", false}, // Case-sensitive
 
-		// Windows tests
+		// Windows dispatch to the (mocked) account classifier
 		{"windows_administrator", "Administrator", "windows", true},
-		{"windows_system", "SYSTEM", "windows", true},
-		{"windows_admin", "admin", "windows", true},
-		{"windows_admin_lowercase", "administrator", "windows", true}, // Case-insensitive
-		{"windows_domain_admin", "DOMAIN\\Administrator", "windows", true},
-		{"windows_email_admin", "admin@domain.com", "windows", true},
 		{"windows_regular_user", "alice", "windows", false},
-		{"windows_domain_user", "DOMAIN\\alice", "windows", false},
-		{"windows_localsystem", "localsystem", "windows", true},
-		{"windows_networkservice", "networkservice", "windows", true},
-		{"windows_localservice", "localservice", "windows", true},
-
-		// Computer accounts (these depend on current user context in real implementation)
-		{"windows_computer_account", "WIN2K19-C2$", "windows", false},      // Computer account by itself not privileged
-		{"windows_domain_computer", "DOMAIN\\COMPUTER$", "windows", false}, // Domain computer account
-
-		// Cross-platform
-		{"root_on_windows", "root", "windows", true}, // Root should be privileged everywhere
 	}
 
 	for _, tt := range tests {
@@ -460,49 +461,7 @@ func TestPrivilegedUsernameDetection(t *testing.T) {
 			defer cleanup()
 
 			result := isPrivilegedUsername(tt.username)
-			assert.Equal(t, tt.privileged, result)
-		})
-	}
-}
-
-func TestWindowsPrivilegedSIDDetection(t *testing.T) {
-	tests := []struct {
-		name        string
-		sid         string
-		privileged  bool
-		description string
-	}{
-		// Well-known system accounts
-		{"system_account", "S-1-5-18", true, "Local System (SYSTEM)"},
-		{"local_service", "S-1-5-19", true, "Local Service"},
-		{"network_service", "S-1-5-20", true, "Network Service"},
-		{"administrators_group", "S-1-5-32-544", true, "Administrators group"},
-		{"builtin_administrator", "S-1-5-500", true, "Built-in Administrator"},
-
-		// Domain accounts
-		{"domain_administrator", "S-1-5-21-1234567890-1234567890-1234567890-500", true, "Domain Administrator (RID 500)"},
-		{"domain_admins_group", "S-1-5-21-1234567890-1234567890-1234567890-512", true, "Domain Admins group"},
-		{"domain_controllers_group", "S-1-5-21-1234567890-1234567890-1234567890-516", true, "Domain Controllers group"},
-		{"enterprise_admins_group", "S-1-5-21-1234567890-1234567890-1234567890-519", true, "Enterprise Admins group"},
-
-		// Regular users
-		{"regular_user", "S-1-5-21-1234567890-1234567890-1234567890-1001", false, "Regular domain user"},
-		{"another_regular_user", "S-1-5-21-1234567890-1234567890-1234567890-1234", false, "Another regular user"},
-		{"local_user", "S-1-5-21-1234567890-1234567890-1234567890-1000", false, "Local regular user"},
-
-		// Groups that are not privileged
-		{"domain_users", "S-1-5-21-1234567890-1234567890-1234567890-513", false, "Domain Users group"},
-		{"power_users", "S-1-5-32-547", false, "Power Users group"},
-
-		// Invalid SIDs
-		{"malformed_sid", "S-1-5-invalid", false, "Malformed SID"},
-		{"empty_sid", "", false, "Empty SID"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			result := isWindowsPrivilegedSID(tt.sid)
-			assert.Equal(t, tt.privileged, result, "Failed for %s: %s", tt.description, tt.sid)
+			assert.Equal(t, tt.privileged, result, "privilege classification for %s on %s", tt.username, tt.platform)
 		})
 	}
 }
diff --git a/client/ssh/server/userswitching_windows.go b/client/ssh/server/userswitching_windows.go
index 260e1301ed6..9e8cd5b3023 100644
--- a/client/ssh/server/userswitching_windows.go
+++ b/client/ssh/server/userswitching_windows.go
@@ -91,7 +91,7 @@ func validateUsernameFormat(username string) error {
 func (s *Server) createExecutorCommand(logger *log.Entry, session ssh.Session, localUser *user.User, hasPty bool) (*exec.Cmd, func(), error) {
 	logger.Debugf("creating Windows executor command for user %s (Pty: %v)", localUser.Username, hasPty)
 
-	username, _ := s.parseUsername(localUser.Username)
+	username, _ := parseUsername(localUser.Username)
 	if err := validateUsername(username); err != nil {
 		return nil, nil, fmt.Errorf("invalid username %q: %w", username, err)
 	}
@@ -102,7 +102,7 @@ func (s *Server) createExecutorCommand(logger *log.Entry, session ssh.Session, l
 // createUserSwitchCommand creates a command with Windows user switching.
 // Returns the command and a cleanup function that must be called after starting the process.
 func (s *Server) createUserSwitchCommand(logger *log.Entry, session ssh.Session, localUser *user.User) (*exec.Cmd, func(), error) {
-	username, domain := s.parseUsername(localUser.Username)
+	username, domain := parseUsername(localUser.Username)
 
 	shell := getUserShell(localUser.Uid)
 
@@ -138,7 +138,7 @@ func (s *Server) createUserSwitchCommand(logger *log.Entry, session ssh.Session,
 }
 
 // parseUsername extracts username and domain from a Windows username
-func (s *Server) parseUsername(fullUsername string) (username, domain string) {
+func parseUsername(fullUsername string) (username, domain string) {
 	// Handle DOMAIN\username format
 	if idx := strings.LastIndex(fullUsername, `\`); idx != -1 {
 		domain = fullUsername[:idx]

From 3c745a8228901a039d18a30fbecf5a409499ba24 Mon Sep 17 00:00:00 2001
From: Viktor Liu <viktor@netbird.io>
Date: Wed, 29 Jul 2026 18:00:04 +0200
Subject: [PATCH 2/5] Fail closed when a local group name cannot be resolved to
 a SID

---
 client/ssh/server/privileges_windows.go      | 47 +++++++++++++++++---
 client/ssh/server/privileges_windows_test.go | 38 ++++++++++++++++
 client/ssh/server/user_utils.go              | 12 ++---
 3 files changed, 81 insertions(+), 16 deletions(-)

diff --git a/client/ssh/server/privileges_windows.go b/client/ssh/server/privileges_windows.go
index 82da4523177..981e2f073ee 100644
--- a/client/ssh/server/privileges_windows.go
+++ b/client/ssh/server/privileges_windows.go
@@ -4,6 +4,7 @@ package server
 
 import (
 	"fmt"
+	"strings"
 	"unsafe"
 
 	log "github.com/sirupsen/logrus"
@@ -13,6 +14,12 @@ import (
 var (
 	netapi32                  = windows.NewLazySystemDLL("netapi32.dll")
 	procNetUserGetLocalGroups = netapi32.NewProc("NetUserGetLocalGroups")
+
+	// lookupGroupSID resolves a group name to its SID, indirected for testing.
+	lookupGroupSID = func(name string) (*windows.SID, error) {
+		sid, _, _, err := windows.LookupSID("", name)
+		return sid, err
+	}
 )
 
 const (
@@ -136,23 +143,42 @@ func s4uTokenIsMember(account, domain string, sid *windows.SID) (bool, error) {
 }
 
 // localGroupsContainSID enumerates the local groups the account belongs to
-// (including membership through global groups) and compares each group's SID
-// against the wanted SID. Group names are resolved back to SIDs so the
-// comparison is not sensitive to localization.
+// (including membership through global groups) and reports whether the wanted
+// SID is among them. Group names are resolved to SIDs so the comparison is not
+// sensitive to localization.
+//
+// A group whose name does not resolve is compared against the wanted SID's own
+// account name rather than skipped: skipping it would under-report membership,
+// which for a privilege check means reporting a privileged account as
+// unprivileged. If neither comparison is possible the error is returned so the
+// caller can fail closed.
 func localGroupsContainSID(username string, want *windows.SID) (bool, error) {
 	groups, err := netUserGetLocalGroups(username)
 	if err != nil {
 		return false, err
 	}
+
+	wantName, _, _, wantNameErr := want.LookupAccount("")
+
 	for _, group := range groups {
-		groupSid, _, _, err := windows.LookupSID("", group)
-		if err != nil {
-			log.Debugf("resolve local group %q to SID: %v", group, err)
+		groupSid, err := lookupGroupSID(group)
+		if err == nil {
+			if groupSid.Equals(want) {
+				return true, nil
+			}
 			continue
 		}
-		if groupSid.Equals(want) {
+
+		if wantNameErr != nil {
+			return false, fmt.Errorf("resolve group %q: %w (and resolve wanted SID to a name: %w)", group, err, wantNameErr)
+		}
+
+		// Local group names are unique per machine, so a name mismatch is
+		// conclusive even though the SID is unavailable.
+		if strings.EqualFold(group, wantName) {
 			return true, nil
 		}
+		log.Debugf("local group %q does not resolve to a SID and does not match %q: %v", group, wantName, err)
 	}
 	return false, nil
 }
@@ -189,6 +215,13 @@ func netUserGetLocalGroups(username string) ([]string, error) {
 		}
 	}()
 
+	// MAX_PREFERRED_LENGTH makes the API allocate as much as it needs, so a
+	// short read is not expected. Report it rather than silently returning a
+	// subset of the account's groups.
+	if entriesRead != totalEntries {
+		return nil, fmt.Errorf("NetUserGetLocalGroups for %q returned %d of %d groups", username, entriesRead, totalEntries)
+	}
+
 	entries := unsafe.Slice((*localGroupUsersInfo0)(unsafe.Pointer(buf)), entriesRead)
 	groups := make([]string, 0, entriesRead)
 	for _, entry := range entries {
diff --git a/client/ssh/server/privileges_windows_test.go b/client/ssh/server/privileges_windows_test.go
index 45831dbe715..e9bc37720d4 100644
--- a/client/ssh/server/privileges_windows_test.go
+++ b/client/ssh/server/privileges_windows_test.go
@@ -3,6 +3,7 @@
 package server
 
 import (
+	"errors"
 	"os/user"
 	"testing"
 	"unsafe"
@@ -166,6 +167,43 @@ func TestS4UMembershipAgreesWithLocalGroups(t *testing.T) {
 	t.Logf("checked %d local accounts via S4U", checked)
 }
 
+// TestLocalGroupsContainSID_UnresolvableGroupMatchesByName covers the case
+// where a group name cannot be resolved to a SID: membership must still be
+// found by name instead of the group being skipped, which would report a
+// privileged account as unprivileged.
+func TestLocalGroupsContainSID_UnresolvableGroupMatchesByName(t *testing.T) {
+	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
+	require.NoError(t, err, "create Administrators SID")
+
+	original := lookupGroupSID
+	t.Cleanup(func() { lookupGroupSID = original })
+	lookupGroupSID = func(string) (*windows.SID, error) {
+		return nil, errors.New("simulated SID resolution failure")
+	}
+
+	// The built-in Administrator is always a member of Administrators.
+	member, err := localGroupsContainSID("Administrator", adminSid)
+	require.NoError(t, err, "enumerate local groups for Administrator")
+	assert.True(t, member, "unresolvable group must be matched by name, not skipped")
+}
+
+// TestLocalGroupsContainSID_UnresolvableFailsClosed covers the case where
+// neither the group SID nor the wanted SID's name can be resolved: the error
+// must surface so the caller treats the account as privileged.
+func TestLocalGroupsContainSID_UnresolvableFailsClosed(t *testing.T) {
+	// A SID that resolves to no account, so LookupAccount fails.
+	unknown := mustParseSID(t, "S-1-5-21-1111111111-2222222222-3333333333-4444")
+
+	original := lookupGroupSID
+	t.Cleanup(func() { lookupGroupSID = original })
+	lookupGroupSID = func(string) (*windows.SID, error) {
+		return nil, errors.New("simulated SID resolution failure")
+	}
+
+	_, err := localGroupsContainSID("Administrator", unknown)
+	require.Error(t, err, "must report an error when membership cannot be determined either way")
+}
+
 func TestLocalGroupsContainSID_Guest(t *testing.T) {
 	guestsSid, err := windows.CreateWellKnownSid(windows.WinBuiltinGuestsSid)
 	require.NoError(t, err, "create Guests SID")
diff --git a/client/ssh/server/user_utils.go b/client/ssh/server/user_utils.go
index 5ffa3c8a16b..93a86fd1829 100644
--- a/client/ssh/server/user_utils.go
+++ b/client/ssh/server/user_utils.go
@@ -189,16 +189,10 @@ func (s *Server) SetAllowRootLogin(allow bool) {
 
 // userNameLookup performs user lookup with root login permission check
 func (s *Server) userNameLookup(username string) (*user.User, error) {
-	result := s.CheckPrivileges(PrivilegeCheckRequest{
-		RequestedUsername:         username,
-		FeatureSupportsUserSwitch: true,
-		FeatureName:               FeatureSSHLogin,
-	})
-
-	if !result.Allowed {
-		return nil, result.Error
+	result, err := s.userPrivilegeCheck(username)
+	if err != nil {
+		return nil, err
 	}
-
 	return result.User, nil
 }
 

From 6f8dd82ddc4ca1e2e382c85703fe4fe132fa624c Mon Sep 17 00:00:00 2001
From: Viktor Liu <viktor@netbird.io>
Date: Wed, 29 Jul 2026 18:14:19 +0200
Subject: [PATCH 3/5] Identify the Administrators group by the name resolved
 from its well-known SID

---
 client/ssh/server/privileges_windows.go      | 66 +++++++++-----------
 client/ssh/server/privileges_windows_test.go | 35 +++--------
 2 files changed, 38 insertions(+), 63 deletions(-)

diff --git a/client/ssh/server/privileges_windows.go b/client/ssh/server/privileges_windows.go
index 981e2f073ee..6b8d497089d 100644
--- a/client/ssh/server/privileges_windows.go
+++ b/client/ssh/server/privileges_windows.go
@@ -14,12 +14,6 @@ import (
 var (
 	netapi32                  = windows.NewLazySystemDLL("netapi32.dll")
 	procNetUserGetLocalGroups = netapi32.NewProc("NetUserGetLocalGroups")
-
-	// lookupGroupSID resolves a group name to its SID, indirected for testing.
-	lookupGroupSID = func(name string) (*windows.SID, error) {
-		sid, _, _, err := windows.LookupSID("", name)
-		return sid, err
-	}
 )
 
 const (
@@ -98,11 +92,17 @@ func isBuiltinAdministratorSID(sid *windows.SID) bool {
 }
 
 // isLocalAdminsMember reports whether the account is a member of the local
-// Administrators group. Local accounts are checked against the local SAM,
-// which is authoritative for them. For domain accounts an S4U identification
-// token is preferred because its group list is LSA's transitive expansion
-// (nested and universal groups included); NetUserGetLocalGroups expands only
-// one global-group hop but needs no logon, so it serves as fallback.
+// Administrators group.
+//
+// Local accounts are checked against the local SAM, which is authoritative for
+// them and, unlike a token, cannot under-report: UAC filters the tokens of
+// local administrators, and a filtered token carries Administrators as
+// deny-only, which a membership check on the token would read as "not a
+// member". Domain accounts are exempt from that filtering, so for them an S4U
+// token is preferred because its group list is LSA's transitive expansion and
+// therefore covers nested and universal groups plus the machine's own local
+// groups. NetUserGetLocalGroups expands only one global-group hop but needs no
+// logon, so it serves as the fallback when no token can be obtained.
 func isLocalAdminsMember(username string) (bool, error) {
 	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
 	if err != nil {
@@ -142,43 +142,35 @@ func s4uTokenIsMember(account, domain string, sid *windows.SID) (bool, error) {
 	return windows.Token(token).IsMember(sid)
 }
 
-// localGroupsContainSID enumerates the local groups the account belongs to
-// (including membership through global groups) and reports whether the wanted
-// SID is among them. Group names are resolved to SIDs so the comparison is not
-// sensitive to localization.
+// localGroupsContainSID reports whether the wanted group is among the local
+// groups the account belongs to, directly or through a global group.
 //
-// A group whose name does not resolve is compared against the wanted SID's own
-// account name rather than skipped: skipping it would under-report membership,
-// which for a privilege check means reporting a privileged account as
-// unprivileged. If neither comparison is possible the error is returned so the
-// caller can fail closed.
+// The wanted SID is resolved to its group name once and compared against the
+// enumerated names. Well-known SIDs resolve from a static table, so that lookup
+// needs no domain controller, and it keeps the comparison correct for a renamed
+// or localized group because both sides then carry the new name. Resolving each
+// enumerated name back to a SID instead would add a lookup per group that can
+// block until it times out while a domain controller is unreachable, and cannot
+// change the outcome: the names enumerated here are local groups of this
+// machine, whose names are unique, so a name match identifies the group.
+//
+// A failure to resolve the wanted SID is returned rather than reported as
+// "not a member", so a privilege check built on this fails closed.
 func localGroupsContainSID(username string, want *windows.SID) (bool, error) {
+	wantName, _, _, err := want.LookupAccount("")
+	if err != nil {
+		return false, fmt.Errorf("resolve group SID %s to a name: %w", want, err)
+	}
+
 	groups, err := netUserGetLocalGroups(username)
 	if err != nil {
 		return false, err
 	}
 
-	wantName, _, _, wantNameErr := want.LookupAccount("")
-
 	for _, group := range groups {
-		groupSid, err := lookupGroupSID(group)
-		if err == nil {
-			if groupSid.Equals(want) {
-				return true, nil
-			}
-			continue
-		}
-
-		if wantNameErr != nil {
-			return false, fmt.Errorf("resolve group %q: %w (and resolve wanted SID to a name: %w)", group, err, wantNameErr)
-		}
-
-		// Local group names are unique per machine, so a name mismatch is
-		// conclusive even though the SID is unavailable.
 		if strings.EqualFold(group, wantName) {
 			return true, nil
 		}
-		log.Debugf("local group %q does not resolve to a SID and does not match %q: %v", group, wantName, err)
 	}
 	return false, nil
 }
diff --git a/client/ssh/server/privileges_windows_test.go b/client/ssh/server/privileges_windows_test.go
index e9bc37720d4..2aa6fa88b16 100644
--- a/client/ssh/server/privileges_windows_test.go
+++ b/client/ssh/server/privileges_windows_test.go
@@ -3,7 +3,6 @@
 package server
 
 import (
-	"errors"
 	"os/user"
 	"testing"
 	"unsafe"
@@ -167,41 +166,25 @@ func TestS4UMembershipAgreesWithLocalGroups(t *testing.T) {
 	t.Logf("checked %d local accounts via S4U", checked)
 }
 
-// TestLocalGroupsContainSID_UnresolvableGroupMatchesByName covers the case
-// where a group name cannot be resolved to a SID: membership must still be
-// found by name instead of the group being skipped, which would report a
-// privileged account as unprivileged.
-func TestLocalGroupsContainSID_UnresolvableGroupMatchesByName(t *testing.T) {
+// TestLocalGroupsContainSID_Administrator checks the positive case against an
+// account that is a member of Administrators on every Windows installation.
+func TestLocalGroupsContainSID_Administrator(t *testing.T) {
 	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
 	require.NoError(t, err, "create Administrators SID")
 
-	original := lookupGroupSID
-	t.Cleanup(func() { lookupGroupSID = original })
-	lookupGroupSID = func(string) (*windows.SID, error) {
-		return nil, errors.New("simulated SID resolution failure")
-	}
-
-	// The built-in Administrator is always a member of Administrators.
 	member, err := localGroupsContainSID("Administrator", adminSid)
 	require.NoError(t, err, "enumerate local groups for Administrator")
-	assert.True(t, member, "unresolvable group must be matched by name, not skipped")
+	assert.True(t, member, "the built-in Administrator is a member of Administrators")
 }
 
-// TestLocalGroupsContainSID_UnresolvableFailsClosed covers the case where
-// neither the group SID nor the wanted SID's name can be resolved: the error
-// must surface so the caller treats the account as privileged.
-func TestLocalGroupsContainSID_UnresolvableFailsClosed(t *testing.T) {
-	// A SID that resolves to no account, so LookupAccount fails.
+// TestLocalGroupsContainSID_UnresolvableGroupFailsClosed covers a wanted SID
+// that resolves to no group: the error must surface rather than being reported
+// as "not a member", so the privilege check treats the account as privileged.
+func TestLocalGroupsContainSID_UnresolvableGroupFailsClosed(t *testing.T) {
 	unknown := mustParseSID(t, "S-1-5-21-1111111111-2222222222-3333333333-4444")
 
-	original := lookupGroupSID
-	t.Cleanup(func() { lookupGroupSID = original })
-	lookupGroupSID = func(string) (*windows.SID, error) {
-		return nil, errors.New("simulated SID resolution failure")
-	}
-
 	_, err := localGroupsContainSID("Administrator", unknown)
-	require.Error(t, err, "must report an error when membership cannot be determined either way")
+	require.Error(t, err, "must report an error when the wanted group cannot be identified")
 }
 
 func TestLocalGroupsContainSID_Guest(t *testing.T) {

From c4fec48a71b59e1e3d09c23419f19e392893ee73 Mon Sep 17 00:00:00 2001
From: Viktor Liu <viktor@netbird.io>
Date: Wed, 29 Jul 2026 18:33:52 +0200
Subject: [PATCH 4/5] Resolve test account names from well-known SIDs and RIDs
 instead of English literals

---
 client/ssh/server/privileges_windows_test.go | 121 +++++++++++++------
 client/ssh/server/server_config_test.go      |  91 +++++---------
 2 files changed, 116 insertions(+), 96 deletions(-)

diff --git a/client/ssh/server/privileges_windows_test.go b/client/ssh/server/privileges_windows_test.go
index 2aa6fa88b16..f23e1d26b63 100644
--- a/client/ssh/server/privileges_windows_test.go
+++ b/client/ssh/server/privileges_windows_test.go
@@ -27,6 +27,69 @@ func mustParseSID(t *testing.T, s string) *windows.SID {
 	return sid
 }
 
+// localAccountNames returns the names of the local user accounts.
+func localAccountNames(t *testing.T) []string {
+	t.Helper()
+
+	var buf *byte
+	var entriesRead, totalEntries, resume uint32
+	err := windows.NetUserEnum(nil, 0, filterNormalAccount, &buf, maxPreferredLength,
+		&entriesRead, &totalEntries, &resume)
+	require.NoError(t, err, "enumerate local users")
+	t.Cleanup(func() {
+		require.NoError(t, windows.NetApiBufferFree(buf), "free NetApi buffer")
+	})
+
+	entries := unsafe.Slice((*userInfo0)(unsafe.Pointer(buf)), entriesRead)
+	names := make([]string, 0, entriesRead)
+	for _, entry := range entries {
+		names = append(names, windows.UTF16PtrToString(entry.name))
+	}
+	return names
+}
+
+// localAccountNameByRID returns the name of the local account carrying the
+// given RID. Accounts such as Administrator and Guest can be renamed and are
+// localized, so tests must not name them literally.
+func localAccountNameByRID(t *testing.T, rid uint32) string {
+	t.Helper()
+
+	for _, name := range localAccountNames(t) {
+		sid, _, _, err := windows.LookupSID("", name)
+		if err != nil {
+			continue
+		}
+		if sid.IdentifierAuthority() != windows.SECURITY_NT_AUTHORITY {
+			continue
+		}
+		count := sid.SubAuthorityCount()
+		if count < 2 || sid.SubAuthority(0) != 21 {
+			continue
+		}
+		if sid.SubAuthority(uint32(count-1)) == rid {
+			return name
+		}
+	}
+
+	t.Fatalf("no local account with RID %d", rid)
+	return ""
+}
+
+// wellKnownAccountName resolves a well-known SID to the qualified account name
+// the local system uses for it, which is localized.
+func wellKnownAccountName(t *testing.T, sidType windows.WELL_KNOWN_SID_TYPE) string {
+	t.Helper()
+
+	sid, err := windows.CreateWellKnownSid(sidType)
+	require.NoError(t, err, "create well-known SID")
+	name, domain, _, err := sid.LookupAccount("")
+	require.NoError(t, err, "resolve %s to an account name", sid)
+	if domain == "" {
+		return name
+	}
+	return domain + `\` + name
+}
+
 func TestIsBuiltinAdministratorSID(t *testing.T) {
 	tests := []struct {
 		name string
@@ -81,14 +144,14 @@ func TestIsWindowsAccountPrivileged(t *testing.T) {
 		username string
 		want     bool
 	}{
-		{"system", "NT AUTHORITY\\SYSTEM", true},
-		{"local_service", "NT AUTHORITY\\LOCAL SERVICE", true},
-		{"network_service", "NT AUTHORITY\\NETWORK SERVICE", true},
-		{"administrators_group_name", "BUILTIN\\Administrators", true},
-		// The built-in Administrator and Guest accounts exist on every
-		// Windows installation, though they may be disabled.
-		{"builtin_administrator", "Administrator", true},
-		{"guest", "Guest", false},
+		{"system", wellKnownAccountName(t, windows.WinLocalSystemSid), true},
+		{"local_service", wellKnownAccountName(t, windows.WinLocalServiceSid), true},
+		{"network_service", wellKnownAccountName(t, windows.WinNetworkServiceSid), true},
+		{"administrators_group", wellKnownAccountName(t, windows.WinBuiltinAdministratorsSid), true},
+		// The built-in Administrator (RID 500) and Guest (RID 501) accounts
+		// exist on every Windows installation, though they may be disabled.
+		{"builtin_administrator", localAccountNameByRID(t, 500), true},
+		{"guest", localAccountNameByRID(t, 501), false},
 		// Unresolvable accounts fail closed.
 		{"nonexistent_user", "netbird-no-such-user", true},
 		{"empty_username", "", true},
@@ -136,21 +199,8 @@ func TestS4UMembershipAgreesWithLocalGroups(t *testing.T) {
 	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
 	require.NoError(t, err, "create Administrators SID")
 
-	var buf *byte
-	var entriesRead, totalEntries uint32
-	var resume uint32
-	err = windows.NetUserEnum(nil, 0, filterNormalAccount, &buf, maxPreferredLength,
-		&entriesRead, &totalEntries, &resume)
-	require.NoError(t, err, "enumerate local users")
-	defer func() {
-		require.NoError(t, windows.NetApiBufferFree(buf), "free NetApi buffer")
-	}()
-
-	names := unsafe.Slice((*userInfo0)(unsafe.Pointer(buf)), entriesRead)
 	checked := 0
-	for _, entry := range names {
-		name := windows.UTF16PtrToString(entry.name)
-
+	for _, name := range localAccountNames(t) {
 		viaToken, err := s4uTokenIsMember(name, ".", adminSid)
 		if err != nil {
 			// Disabled or logon-restricted accounts cannot get an S4U logon.
@@ -166,15 +216,16 @@ func TestS4UMembershipAgreesWithLocalGroups(t *testing.T) {
 	t.Logf("checked %d local accounts via S4U", checked)
 }
 
-// TestLocalGroupsContainSID_Administrator checks the positive case against an
-// account that is a member of Administrators on every Windows installation.
+// TestLocalGroupsContainSID_Administrator checks the positive case against the
+// built-in Administrator, a member of Administrators on every installation.
 func TestLocalGroupsContainSID_Administrator(t *testing.T) {
 	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
 	require.NoError(t, err, "create Administrators SID")
 
-	member, err := localGroupsContainSID("Administrator", adminSid)
-	require.NoError(t, err, "enumerate local groups for Administrator")
-	assert.True(t, member, "the built-in Administrator is a member of Administrators")
+	administrator := localAccountNameByRID(t, 500)
+	member, err := localGroupsContainSID(administrator, adminSid)
+	require.NoError(t, err, "enumerate local groups for %s", administrator)
+	assert.True(t, member, "%s is a member of the Administrators group", administrator)
 }
 
 // TestLocalGroupsContainSID_UnresolvableGroupFailsClosed covers a wanted SID
@@ -183,7 +234,7 @@ func TestLocalGroupsContainSID_Administrator(t *testing.T) {
 func TestLocalGroupsContainSID_UnresolvableGroupFailsClosed(t *testing.T) {
 	unknown := mustParseSID(t, "S-1-5-21-1111111111-2222222222-3333333333-4444")
 
-	_, err := localGroupsContainSID("Administrator", unknown)
+	_, err := localGroupsContainSID(localAccountNameByRID(t, 500), unknown)
 	require.Error(t, err, "must report an error when the wanted group cannot be identified")
 }
 
@@ -193,11 +244,13 @@ func TestLocalGroupsContainSID_Guest(t *testing.T) {
 	adminsSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
 	require.NoError(t, err, "create Administrators SID")
 
-	inGuests, err := localGroupsContainSID("Guest", guestsSid)
-	require.NoError(t, err, "enumerate local groups for Guest")
-	assert.True(t, inGuests, "Guest should be a member of BUILTIN\\Guests")
+	guest := localAccountNameByRID(t, 501)
+
+	inGuests, err := localGroupsContainSID(guest, guestsSid)
+	require.NoError(t, err, "enumerate local groups for %s", guest)
+	assert.True(t, inGuests, "%s is a member of the Guests group", guest)
 
-	inAdmins, err := localGroupsContainSID("Guest", adminsSid)
-	require.NoError(t, err, "enumerate local groups for Guest")
-	assert.False(t, inAdmins, "Guest should not be a member of BUILTIN\\Administrators")
+	inAdmins, err := localGroupsContainSID(guest, adminsSid)
+	require.NoError(t, err, "enumerate local groups for %s", guest)
+	assert.False(t, inAdmins, "%s is not a member of the Administrators group", guest)
 }
diff --git a/client/ssh/server/server_config_test.go b/client/ssh/server/server_config_test.go
index 3ce0110b614..0717c238d25 100644
--- a/client/ssh/server/server_config_test.go
+++ b/client/ssh/server/server_config_test.go
@@ -420,71 +420,38 @@ func TestServer_PortConflictHandling(t *testing.T) {
 
 func TestServer_IsPrivilegedUser(t *testing.T) {
 
-	type privilegedUserCase struct {
+	// Windows classification depends on account SIDs and group membership, and
+	// the accounts involved carry localized, renameable names. It is covered by
+	// TestIsWindowsAccountPrivileged, which resolves them from well-known SIDs.
+	if runtime.GOOS == "windows" {
+		t.Skip("covered by TestIsWindowsAccountPrivileged")
+	}
+
+	tests := []struct {
 		username    string
 		expected    bool
 		description string
-	}
-	var tests []privilegedUserCase
-
-	if runtime.GOOS == "windows" {
-		// On Windows, accounts that cannot be resolved or evaluated count as
-		// privileged (fail closed), including nonexistent and empty names.
-		tests = []privilegedUserCase{
-			{
-				username:    "Administrator",
-				expected:    true,
-				description: "Administrator should be considered privileged on Windows",
-			},
-			{
-				username:    "administrator",
-				expected:    true,
-				description: "administrator should be considered privileged on Windows (case insensitive)",
-			},
-			{
-				username:    "NT AUTHORITY\\SYSTEM",
-				expected:    true,
-				description: "SYSTEM should be considered privileged on Windows",
-			},
-			{
-				username:    "Guest",
-				expected:    false,
-				description: "Guest should not be privileged on Windows",
-			},
-			{
-				username:    "netbird-no-such-user",
-				expected:    true,
-				description: "unresolvable account should fail closed on Windows",
-			},
-			{
-				username:    "",
-				expected:    true,
-				description: "empty username should fail closed on Windows",
-			},
-		}
-	} else {
-		tests = []privilegedUserCase{
-			{
-				username:    "root",
-				expected:    true,
-				description: "root should be considered privileged",
-			},
-			{
-				username:    "regular",
-				expected:    false,
-				description: "regular user should not be privileged",
-			},
-			{
-				username:    "",
-				expected:    false,
-				description: "empty username should not be privileged",
-			},
-			{
-				username:    "Administrator",
-				expected:    false,
-				description: "Administrator should not be privileged on non-Windows systems",
-			},
-		}
+	}{
+		{
+			username:    "root",
+			expected:    true,
+			description: "root should be considered privileged",
+		},
+		{
+			username:    "regular",
+			expected:    false,
+			description: "regular user should not be privileged",
+		},
+		{
+			username:    "",
+			expected:    false,
+			description: "empty username should not be privileged",
+		},
+		{
+			username:    "Administrator",
+			expected:    false,
+			description: "Administrator should not be privileged on non-Windows systems",
+		},
 	}
 
 	for _, tt := range tests {

From 7aa93955d5e13082ade1c0ccadcf6563fdde766b Mon Sep 17 00:00:00 2001
From: Viktor Liu <viktor@netbird.io>
Date: Wed, 29 Jul 2026 21:05:55 +0200
Subject: [PATCH 5/5] Require an S4U comparison and check elevation against the
 token elevation type

---
 client/ssh/server/privileges_windows_test.go | 47 +++++++++++++++++---
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/client/ssh/server/privileges_windows_test.go b/client/ssh/server/privileges_windows_test.go
index f23e1d26b63..406ce613758 100644
--- a/client/ssh/server/privileges_windows_test.go
+++ b/client/ssh/server/privileges_windows_test.go
@@ -15,6 +15,24 @@ import (
 // filterNormalAccount limits NetUserEnum to normal user accounts.
 const filterNormalAccount = 0x2
 
+// TOKEN_ELEVATION_TYPE values.
+const (
+	tokenElevationTypeDefault = 1
+	tokenElevationTypeFull    = 2
+	tokenElevationTypeLimited = 3
+)
+
+// tokenElevationType reads TokenElevationType from a token.
+func tokenElevationType(token windows.Token) (uint32, error) {
+	var elevationType, returnedLen uint32
+	err := windows.GetTokenInformation(token, windows.TokenElevationType,
+		(*byte)(unsafe.Pointer(&elevationType)), uint32(unsafe.Sizeof(elevationType)), &returnedLen)
+	if err != nil {
+		return 0, err
+	}
+	return elevationType, nil
+}
+
 // userInfo0 mirrors USER_INFO_0.
 type userInfo0 struct {
 	name *uint16
@@ -166,18 +184,34 @@ func TestIsWindowsAccountPrivileged(t *testing.T) {
 }
 
 func TestIsProcessElevated(t *testing.T) {
+	elevated := isProcessElevated()
+
+	// TokenElevationType is a second, independent view of the same token:
+	// Full means elevated and Limited means a filtered administrator, while
+	// Default covers both a standard user and an administrator with no linked
+	// token (UAC off, the built-in Administrator, SYSTEM), so it implies nothing.
+	elevationType, err := tokenElevationType(windows.GetCurrentProcessToken())
+	require.NoError(t, err, "read token elevation type")
+
 	adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid)
 	require.NoError(t, err, "create Administrators SID")
 
-	// Token(0) makes CheckTokenMembership evaluate the caller's own token.
+	// Token(0) makes CheckTokenMembership evaluate the caller's own token. It
+	// counts only enabled SIDs, so a filtered administrator reports false here.
 	member, err := windows.Token(0).IsMember(adminSid)
 	require.NoError(t, err, "check own Administrators membership")
 
-	elevated := isProcessElevated()
-	t.Logf("member of Administrators: %v, token elevated: %v", member, elevated)
+	t.Logf("elevated=%v elevationType=%d memberOfAdministrators=%v", elevated, elevationType, member)
+
+	switch elevationType {
+	case tokenElevationTypeFull:
+		assert.True(t, elevated, "a token of elevation type Full must report elevated")
+	case tokenElevationTypeLimited:
+		assert.False(t, elevated, "a filtered administrator token must not report elevated")
+	}
 
-	// An enabled Administrators SID in the token implies an elevated token.
-	// CI runs this test as SYSTEM, which satisfies both.
+	// Administrators enabled in the token means the token wields administrative
+	// rights, which is what elevation reports.
 	if member {
 		assert.True(t, elevated, "token with enabled Administrators membership must report elevated")
 	}
@@ -213,6 +247,9 @@ func TestS4UMembershipAgreesWithLocalGroups(t *testing.T) {
 		assert.Equal(t, viaSAM, viaToken, "S4U token and SAM enumeration must agree on Administrators membership for %s", name)
 		checked++
 	}
+	// Ineligible accounts are skipped, so without this the test could report
+	// success while comparing nothing at all.
+	require.Positive(t, checked, "no local account completed an S4U logon, so nothing was compared")
 	t.Logf("checked %d local accounts via S4U", checked)
 }