policy: fix ssh usermap, fixing autogroup:nonroot (#2768)

hscontrol/policy/policy_test.go

@@ -1612,13 +1612,7 @@ func TestSSHPolicyRules(t *testing.T) {
 		UserID:   2,
 		User:     users[1],
 	}
-	taggedServer := types.Node{
+
-		Hostname:   "tagged-server",
-		IPv4:       ap("100.64.0.3"),
-		UserID:     3,
-		User:       users[2],
-		ForcedTags: []string{"tag:server"},
-	}
 	taggedClient := types.Node{
 		Hostname:   "tagged-client",
 		IPv4:       ap("100.64.0.4"),
@@ -1659,149 +1653,14 @@ func TestSSHPolicyRules(t *testing.T) {
 						{NodeIP: "100.64.0.2"},
 					},
 					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
+						"*":    "=",
+						"root": "",
 					},
 					Action: &tailcfg.SSHAction{
-						Accept:                   true,
+						Accept:                    true,
-						AllowAgentForwarding:     true,
+						AllowAgentForwarding:      true,
-						AllowLocalPortForwarding: true,
+						AllowLocalPortForwarding:  true,
-					},
+						AllowRemotePortForwarding: true,
-				},
-			}},
-		},
-		{
-			name:       "group-to-tag",
-			targetNode: taggedServer,
-			peers:      types.Nodes{&nodeUser1, &nodeUser2},
-			policy: `{
-				"tagOwners": {
-				  "tag:server": ["user3@"],
-				},
-				"groups": {
-					"group:users": ["user1@", "user2@"]
-				},
-				"ssh": [
-					{
-						"action": "accept",
-						"src": ["group:users"],
-						"dst": ["tag:server"],
-						"users": ["autogroup:nonroot"]
-					}
-				]
-			}`,
-			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
-				{
-					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.1"},
-						{NodeIP: "100.64.0.2"},
-					},
-					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
-					},
-					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
-					},
-				},
-			}},
-		},
-		{
-			name:       "tag-to-user",
-			targetNode: nodeUser1,
-			peers:      types.Nodes{&taggedClient},
-			policy: `{
-			    "tagOwners": {
-					"tag:client": ["user1@"],
-				},
-				"ssh": [
-					{
-						"action": "accept",
-						"src": ["tag:client"],
-						"dst": ["user1@"],
-						"users": ["autogroup:nonroot"]
-					}
-				]
-			}`,
-			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
-				{
-					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.4"},
-					},
-					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
-					},
-					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
-					},
-				},
-			}},
-		},
-		{
-			name:       "tag-to-tag",
-			targetNode: taggedServer,
-			peers:      types.Nodes{&taggedClient},
-			policy: `{
-				"tagOwners": {
-					"tag:client": ["user2@"],
-					"tag:server": ["user3@"],
-				},
-				"ssh": [
-					{
-						"action": "accept",
-						"src": ["tag:client"],
-						"dst": ["tag:server"],
-						"users": ["autogroup:nonroot"]
-					}
-				]
-			}`,
-			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
-				{
-					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.4"},
-					},
-					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
-					},
-					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
-					},
-				},
-			}},
-		},
-		{
-			name:       "group-to-wildcard",
-			targetNode: nodeUser1,
-			peers:      types.Nodes{&nodeUser2, &taggedClient},
-			policy: `{
-				"groups": {
-					"group:admins": ["user2@"]
-				},
-				"ssh": [
-					{
-						"action": "accept",
-						"src": ["group:admins"],
-						"dst": ["*"],
-						"users": ["autogroup:nonroot"]
-					}
-				]
-			}`,
-			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
-				{
-					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.2"},
-					},
-					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
-					},
-					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
 					},
 				},
 			}},
@@ -1830,13 +1689,15 @@ func TestSSHPolicyRules(t *testing.T) {
 						{NodeIP: "100.64.0.4"},
 					},
 					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
+						"*":    "=",
+						"root": "",
 					},
 					Action: &tailcfg.SSHAction{
-						Accept:                   true,
+						Accept:                    true,
-						SessionDuration:          24 * time.Hour,
+						SessionDuration:           24 * time.Hour,
-						AllowAgentForwarding:     true,
+						AllowAgentForwarding:      true,
-						AllowLocalPortForwarding: true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
 					},
 				},
 			}},
@@ -1895,40 +1756,6 @@ func TestSSHPolicyRules(t *testing.T) {
 			expectErr:    true,
 			errorMessage: "not a valid duration string",
 		},
-		{
-			name:       "multiple-ssh-users-with-autogroup",
-			targetNode: nodeUser1,
-			peers:      types.Nodes{&taggedClient},
-			policy: `{
-			"tagOwners": {
-				"tag:client": ["user1@"],
-			},
-        	"ssh": [
-        	    {
-        	        "action": "accept",
-        	        "src": ["tag:client"],
-        	        "dst": ["user1@"],
-        	        "users": ["alice", "bob"]
-        	    }
-        	]
-    }`,
-			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
-				{
-					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.4"},
-					},
-					SSHUsers: map[string]string{
-						"alice": "=",
-						"bob":   "=",
-					},
-					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
-					},
-				},
-			}},
-		},
 		{
 			name:       "unsupported-autogroup",
 			targetNode: nodeUser1,
@@ -1946,6 +1773,114 @@ func TestSSHPolicyRules(t *testing.T) {
 			expectErr:    true,
 			errorMessage: "autogroup \"autogroup:invalid\" is not supported",
 		},
+		{
+			name:       "autogroup-nonroot-should-use-wildcard-with-root-excluded",
+			targetNode: nodeUser1,
+			peers:      types.Nodes{&nodeUser2},
+			policy: `{
+				"groups": {
+					"group:admins": ["user2@"]
+				},
+				"ssh": [
+					{
+						"action": "accept",
+						"src": ["group:admins"],
+						"dst": ["user1@"],
+						"users": ["autogroup:nonroot"]
+					}
+				]
+			}`,
+			// autogroup:nonroot should map to wildcard "*" with root excluded
+			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
+				{
+					Principals: []*tailcfg.SSHPrincipal{
+						{NodeIP: "100.64.0.2"},
+					},
+					SSHUsers: map[string]string{
+						"*":    "=",
+						"root": "",
+					},
+					Action: &tailcfg.SSHAction{
+						Accept:                    true,
+						AllowAgentForwarding:      true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
+					},
+				},
+			}},
+		},
+		{
+			name:       "autogroup-nonroot-plus-root-should-use-wildcard-with-root-mapped",
+			targetNode: nodeUser1,
+			peers:      types.Nodes{&nodeUser2},
+			policy: `{
+				"groups": {
+					"group:admins": ["user2@"]
+				},
+				"ssh": [
+					{
+						"action": "accept",
+						"src": ["group:admins"],
+						"dst": ["user1@"],
+						"users": ["autogroup:nonroot", "root"]
+					}
+				]
+			}`,
+			// autogroup:nonroot + root should map to wildcard "*" with root mapped to itself
+			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
+				{
+					Principals: []*tailcfg.SSHPrincipal{
+						{NodeIP: "100.64.0.2"},
+					},
+					SSHUsers: map[string]string{
+						"*":    "=",
+						"root": "root",
+					},
+					Action: &tailcfg.SSHAction{
+						Accept:                    true,
+						AllowAgentForwarding:      true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
+					},
+				},
+			}},
+		},
+		{
+			name:       "specific-users-should-map-to-themselves-not-equals",
+			targetNode: nodeUser1,
+			peers:      types.Nodes{&nodeUser2},
+			policy: `{
+				"groups": {
+					"group:admins": ["user2@"]
+				},
+				"ssh": [
+					{
+						"action": "accept",
+						"src": ["group:admins"],
+						"dst": ["user1@"],
+						"users": ["ubuntu", "root"]
+					}
+				]
+			}`,
+			// specific usernames should map to themselves, not "="
+			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
+				{
+					Principals: []*tailcfg.SSHPrincipal{
+						{NodeIP: "100.64.0.2"},
+					},
+					SSHUsers: map[string]string{
+						"root":   "root",
+						"ubuntu": "ubuntu",
+					},
+					Action: &tailcfg.SSHAction{
+						Accept:                    true,
+						AllowAgentForwarding:      true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
+					},
+				},
+			}},
+		},
 	}
 
 	for _, tt := range tests {

hscontrol/policy/v2/filter.go

@@ -89,11 +89,12 @@ func (pol *Policy) compileFilterRules(
 
 func sshAction(accept bool, duration time.Duration) tailcfg.SSHAction {
 	return tailcfg.SSHAction{
-		Reject:                   !accept,
+		Reject:                    !accept,
-		Accept:                   accept,
+		Accept:                    accept,
-		SessionDuration:          duration,
+		SessionDuration:           duration,
-		AllowAgentForwarding:     true,
+		AllowAgentForwarding:      true,
-		AllowLocalPortForwarding: true,
+		AllowLocalPortForwarding:  true,
+		AllowRemotePortForwarding: true,
 	}
 }
 
@@ -153,8 +154,17 @@ func (pol *Policy) compileSSHPolicy(
 		}
 
 		userMap := make(map[string]string, len(rule.Users))
-		for _, user := range rule.Users {
+		if rule.Users.ContainsNonRoot() {
-			userMap[user.String()] = "="
+			userMap["*"] = "="
+
+			// by default, we do not allow root unless explicitly stated
+			userMap["root"] = ""
+		}
+		if rule.Users.ContainsRoot() {
+			userMap["root"] = "root"
+		}
+		for _, u := range rule.Users.NormalUsers() {
+			userMap[u.String()] = u.String()
 		}
 		rules = append(rules, &tailcfg.SSHRule{
 			Principals: principals,

hscontrol/policy/v2/filter_test.go

@@ -1,10 +1,16 @@
 package v2
 
 import (
+	"encoding/json"
+	"net/netip"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/juanfont/headscale/hscontrol/types"
+	"github.com/prometheus/common/model"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gorm.io/gorm"
 	"tailscale.com/tailcfg"
 )
@@ -376,3 +382,406 @@ func TestParsing(t *testing.T) {
 		})
 	}
 }
+
+func TestCompileSSHPolicy_UserMapping(t *testing.T) {
+	users := types.Users{
+		{Name: "user1", Model: gorm.Model{ID: 1}},
+		{Name: "user2", Model: gorm.Model{ID: 2}},
+	}
+
+	// Create test nodes
+	nodeUser1 := types.Node{
+		Hostname: "user1-device",
+		IPv4:     createAddr("100.64.0.1"),
+		UserID:   1,
+		User:     users[0],
+	}
+	nodeUser2 := types.Node{
+		Hostname: "user2-device",
+		IPv4:     createAddr("100.64.0.2"),
+		UserID:   2,
+		User:     users[1],
+	}
+
+	nodes := types.Nodes{&nodeUser1, &nodeUser2}
+
+	tests := []struct {
+		name         string
+		targetNode   types.Node
+		policy       *Policy
+		wantSSHUsers map[string]string
+		wantEmpty    bool
+	}{
+		{
+			name:       "specific user mapping",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{"ssh-it-user"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"ssh-it-user": "ssh-it-user",
+			},
+		},
+		{
+			name:       "multiple specific users",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{"ubuntu", "admin", "deploy"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"ubuntu": "ubuntu",
+				"admin":  "admin",
+				"deploy": "deploy",
+			},
+		},
+		{
+			name:       "autogroup:nonroot only",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{SSHUser(AutoGroupNonRoot)},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"*":    "=",
+				"root": "",
+			},
+		},
+		{
+			name:       "root only",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{"root"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"root": "root",
+			},
+		},
+		{
+			name:       "autogroup:nonroot plus root",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{SSHUser(AutoGroupNonRoot), "root"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"*":    "=",
+				"root": "root",
+			},
+		},
+		{
+			name:       "mixed specific users and autogroups",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{SSHUser(AutoGroupNonRoot), "root", "ubuntu", "admin"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"*":      "=",
+				"root":   "root",
+				"ubuntu": "ubuntu",
+				"admin":  "admin",
+			},
+		},
+		{
+			name:       "no matching destination",
+			targetNode: nodeUser2, // Target node2, but policy only allows user1
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")}, // Only user1, not user2
+						Users:        []SSHUser{"ssh-it-user"},
+					},
+				},
+			},
+			wantEmpty: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Validate the policy
+			err := tt.policy.validate()
+			require.NoError(t, err)
+
+			// Compile SSH policy
+			sshPolicy, err := tt.policy.compileSSHPolicy(users, tt.targetNode.View(), nodes.ViewSlice())
+			require.NoError(t, err)
+
+			if tt.wantEmpty {
+				if sshPolicy == nil {
+					return // Expected empty result
+				}
+				assert.Empty(t, sshPolicy.Rules, "SSH policy should be empty when no rules match")
+				return
+			}
+
+			require.NotNil(t, sshPolicy)
+			require.Len(t, sshPolicy.Rules, 1, "Should have exactly one SSH rule")
+
+			rule := sshPolicy.Rules[0]
+			assert.Equal(t, tt.wantSSHUsers, rule.SSHUsers, "SSH users mapping should match expected")
+
+			// Verify principals are set correctly (should contain user2's IP since that's the source)
+			require.Len(t, rule.Principals, 1)
+			assert.Equal(t, "100.64.0.2", rule.Principals[0].NodeIP)
+
+			// Verify action is set correctly
+			assert.True(t, rule.Action.Accept)
+			assert.True(t, rule.Action.AllowAgentForwarding)
+			assert.True(t, rule.Action.AllowLocalPortForwarding)
+			assert.True(t, rule.Action.AllowRemotePortForwarding)
+		})
+	}
+}
+
+func TestCompileSSHPolicy_CheckAction(t *testing.T) {
+	users := types.Users{
+		{Name: "user1", Model: gorm.Model{ID: 1}},
+		{Name: "user2", Model: gorm.Model{ID: 2}},
+	}
+
+	nodeUser1 := types.Node{
+		Hostname: "user1-device",
+		IPv4:     createAddr("100.64.0.1"),
+		UserID:   1,
+		User:     users[0],
+	}
+	nodeUser2 := types.Node{
+		Hostname: "user2-device",
+		IPv4:     createAddr("100.64.0.2"),
+		UserID:   2,
+		User:     users[1],
+	}
+
+	nodes := types.Nodes{&nodeUser1, &nodeUser2}
+
+	policy := &Policy{
+		Groups: Groups{
+			Group("group:admins"): []Username{Username("user2@")},
+		},
+		SSHs: []SSH{
+			{
+				Action:       "check",
+				CheckPeriod:  model.Duration(24 * time.Hour),
+				Sources:      SSHSrcAliases{gp("group:admins")},
+				Destinations: SSHDstAliases{up("user1@")},
+				Users:        []SSHUser{"ssh-it-user"},
+			},
+		},
+	}
+
+	err := policy.validate()
+	require.NoError(t, err)
+
+	sshPolicy, err := policy.compileSSHPolicy(users, nodeUser1.View(), nodes.ViewSlice())
+	require.NoError(t, err)
+	require.NotNil(t, sshPolicy)
+	require.Len(t, sshPolicy.Rules, 1)
+
+	rule := sshPolicy.Rules[0]
+
+	// Verify SSH users are correctly mapped
+	expectedUsers := map[string]string{
+		"ssh-it-user": "ssh-it-user",
+	}
+	assert.Equal(t, expectedUsers, rule.SSHUsers)
+
+	// Verify check action with session duration
+	assert.True(t, rule.Action.Accept)
+	assert.Equal(t, 24*time.Hour, rule.Action.SessionDuration)
+}
+
+// TestSSHIntegrationReproduction reproduces the exact scenario from the integration test
+// TestSSHOneUserToAll that was failing with empty sshUsers
+func TestSSHIntegrationReproduction(t *testing.T) {
+	// Create users matching the integration test
+	users := types.Users{
+		{Name: "user1", Model: gorm.Model{ID: 1}},
+		{Name: "user2", Model: gorm.Model{ID: 2}},
+	}
+
+	// Create simple nodes for testing
+	node1 := &types.Node{
+		Hostname: "user1-node",
+		IPv4:     createAddr("100.64.0.1"),
+		UserID:   1,
+		User:     users[0],
+	}
+
+	node2 := &types.Node{
+		Hostname: "user2-node",
+		IPv4:     createAddr("100.64.0.2"),
+		UserID:   2,
+		User:     users[1],
+	}
+
+	nodes := types.Nodes{node1, node2}
+
+	// Create a simple policy that reproduces the issue
+	policy := &Policy{
+		Groups: Groups{
+			Group("group:integration-test"): []Username{Username("user1@")},
+		},
+		SSHs: []SSH{
+			{
+				Action:       "accept",
+				Sources:      SSHSrcAliases{gp("group:integration-test")},
+				Destinations: SSHDstAliases{up("user2@")},       // Target user2
+				Users:        []SSHUser{SSHUser("ssh-it-user")}, // This is the key - specific user
+			},
+		},
+	}
+
+	// Validate policy
+	err := policy.validate()
+	require.NoError(t, err)
+
+	// Test SSH policy compilation for node2 (target)
+	sshPolicy, err := policy.compileSSHPolicy(users, node2.View(), nodes.ViewSlice())
+	require.NoError(t, err)
+	require.NotNil(t, sshPolicy)
+	require.Len(t, sshPolicy.Rules, 1)
+
+	rule := sshPolicy.Rules[0]
+
+	// This was the failing assertion in integration test - sshUsers was empty
+	assert.NotEmpty(t, rule.SSHUsers, "SSH users should not be empty")
+	assert.Contains(t, rule.SSHUsers, "ssh-it-user", "ssh-it-user should be present in SSH users")
+	assert.Equal(t, "ssh-it-user", rule.SSHUsers["ssh-it-user"], "ssh-it-user should map to itself")
+
+	// Verify that ssh-it-user is correctly mapped
+	expectedUsers := map[string]string{
+		"ssh-it-user": "ssh-it-user",
+	}
+	assert.Equal(t, expectedUsers, rule.SSHUsers, "ssh-it-user should be mapped to itself")
+}
+
+// TestSSHJSONSerialization verifies that the SSH policy can be properly serialized
+// to JSON and that the sshUsers field is not empty
+func TestSSHJSONSerialization(t *testing.T) {
+	users := types.Users{
+		{Name: "user1", Model: gorm.Model{ID: 1}},
+	}
+
+	node := &types.Node{
+		Hostname: "test-node",
+		IPv4:     createAddr("100.64.0.1"),
+		UserID:   1,
+		User:     users[0],
+	}
+
+	nodes := types.Nodes{node}
+
+	policy := &Policy{
+		SSHs: []SSH{
+			{
+				Action:       "accept",
+				Sources:      SSHSrcAliases{up("user1@")},
+				Destinations: SSHDstAliases{up("user1@")},
+				Users:        []SSHUser{"ssh-it-user", "ubuntu", "admin"},
+			},
+		},
+	}
+
+	err := policy.validate()
+	require.NoError(t, err)
+
+	sshPolicy, err := policy.compileSSHPolicy(users, node.View(), nodes.ViewSlice())
+	require.NoError(t, err)
+	require.NotNil(t, sshPolicy)
+
+	// Serialize to JSON to verify structure
+	jsonData, err := json.MarshalIndent(sshPolicy, "", "  ")
+	require.NoError(t, err)
+
+	// Parse back to verify structure
+	var parsed tailcfg.SSHPolicy
+	err = json.Unmarshal(jsonData, &parsed)
+	require.NoError(t, err)
+
+	// Verify the parsed structure has the expected SSH users
+	require.Len(t, parsed.Rules, 1)
+	rule := parsed.Rules[0]
+
+	expectedUsers := map[string]string{
+		"ssh-it-user": "ssh-it-user",
+		"ubuntu":      "ubuntu",
+		"admin":       "admin",
+	}
+	assert.Equal(t, expectedUsers, rule.SSHUsers, "SSH users should survive JSON round-trip")
+
+	// Verify JSON contains the SSH users (not empty)
+	assert.Contains(t, string(jsonData), `"ssh-it-user"`)
+	assert.Contains(t, string(jsonData), `"ubuntu"`)
+	assert.Contains(t, string(jsonData), `"admin"`)
+	assert.NotContains(t, string(jsonData), `"sshUsers": {}`, "SSH users should not be empty")
+	assert.NotContains(t, string(jsonData), `"sshUsers": null`, "SSH users should not be null")
+}
+
+// Helper function to create IP addresses for testing
+func createAddr(ip string) *netip.Addr {
+	addr, _ := netip.ParseAddr(ip)
+	return &addr
+}

hscontrol/policy/v2/types.go

@@ -20,6 +20,7 @@ import (
 	"tailscale.com/types/ptr"
 	"tailscale.com/types/views"
 	"tailscale.com/util/multierr"
+	"tailscale.com/util/slicesx"
 )
 
 const Wildcard = Asterix(0)
@@ -506,6 +507,10 @@ func (ag *AutoGroup) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
+func (ag AutoGroup) String() string {
+	return string(ag)
+}
+
 // MarshalJSON marshals the AutoGroup to JSON.
 func (ag AutoGroup) MarshalJSON() ([]byte, error) {
 	return json.Marshal(string(ag))
@@ -1562,7 +1567,7 @@ type SSH struct {
 	Action       string         `json:"action"`
 	Sources      SSHSrcAliases  `json:"src"`
 	Destinations SSHDstAliases  `json:"dst"`
-	Users        []SSHUser      `json:"users"`
+	Users        SSHUsers       `json:"users"`
 	CheckPeriod  model.Duration `json:"checkPeriod,omitempty"`
 }
 
@@ -1715,6 +1720,22 @@ func (a SSHSrcAliases) Resolve(p *Policy, users types.Users, nodes views.Slice[t
 // It can be a list of usernames, tags or autogroups.
 type SSHDstAliases []Alias
 
+type SSHUsers []SSHUser
+
+func (u SSHUsers) ContainsRoot() bool {
+	return slices.Contains(u, "root")
+}
+
+func (u SSHUsers) ContainsNonRoot() bool {
+	return slices.Contains(u, SSHUser(AutoGroupNonRoot))
+}
+
+func (u SSHUsers) NormalUsers() []SSHUser {
+	return slicesx.Filter(nil, u, func(user SSHUser) bool {
+		return user != "root" && user != SSHUser(AutoGroupNonRoot)
+	})
+}
+
 type SSHUser string
 
 func (u SSHUser) String() string {

hscontrol/policy/v2/types_test.go

@@ -1615,6 +1615,134 @@ func TestResolveAutoApprovers(t *testing.T) {
 	}
 }
 
+func TestSSHUsers_NormalUsers(t *testing.T) {
+	tests := []struct {
+		name     string
+		users    SSHUsers
+		expected []SSHUser
+	}{
+		{
+			name:     "empty users",
+			users:    SSHUsers{},
+			expected: []SSHUser{},
+		},
+		{
+			name:     "only root",
+			users:    SSHUsers{"root"},
+			expected: []SSHUser{},
+		},
+		{
+			name:     "only autogroup:nonroot",
+			users:    SSHUsers{SSHUser(AutoGroupNonRoot)},
+			expected: []SSHUser{},
+		},
+		{
+			name:     "only normal user",
+			users:    SSHUsers{"ssh-it-user"},
+			expected: []SSHUser{"ssh-it-user"},
+		},
+		{
+			name:     "multiple normal users",
+			users:    SSHUsers{"ubuntu", "admin", "user1"},
+			expected: []SSHUser{"ubuntu", "admin", "user1"},
+		},
+		{
+			name:     "mixed users with root",
+			users:    SSHUsers{"ubuntu", "root", "admin"},
+			expected: []SSHUser{"ubuntu", "admin"},
+		},
+		{
+			name:     "mixed users with autogroup:nonroot",
+			users:    SSHUsers{"ubuntu", SSHUser(AutoGroupNonRoot), "admin"},
+			expected: []SSHUser{"ubuntu", "admin"},
+		},
+		{
+			name:     "mixed users with both root and autogroup:nonroot",
+			users:    SSHUsers{"ubuntu", "root", SSHUser(AutoGroupNonRoot), "admin"},
+			expected: []SSHUser{"ubuntu", "admin"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.users.NormalUsers()
+			assert.ElementsMatch(t, tt.expected, result, "NormalUsers() should return expected normal users")
+		})
+	}
+}
+
+func TestSSHUsers_ContainsRoot(t *testing.T) {
+	tests := []struct {
+		name     string
+		users    SSHUsers
+		expected bool
+	}{
+		{
+			name:     "empty users",
+			users:    SSHUsers{},
+			expected: false,
+		},
+		{
+			name:     "contains root",
+			users:    SSHUsers{"root"},
+			expected: true,
+		},
+		{
+			name:     "does not contain root",
+			users:    SSHUsers{"ubuntu", "admin"},
+			expected: false,
+		},
+		{
+			name:     "contains root among others",
+			users:    SSHUsers{"ubuntu", "root", "admin"},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.users.ContainsRoot()
+			assert.Equal(t, tt.expected, result, "ContainsRoot() should return expected result")
+		})
+	}
+}
+
+func TestSSHUsers_ContainsNonRoot(t *testing.T) {
+	tests := []struct {
+		name     string
+		users    SSHUsers
+		expected bool
+	}{
+		{
+			name:     "empty users",
+			users:    SSHUsers{},
+			expected: false,
+		},
+		{
+			name:     "contains autogroup:nonroot",
+			users:    SSHUsers{SSHUser(AutoGroupNonRoot)},
+			expected: true,
+		},
+		{
+			name:     "does not contain autogroup:nonroot",
+			users:    SSHUsers{"ubuntu", "admin", "root"},
+			expected: false,
+		},
+		{
+			name:     "contains autogroup:nonroot among others",
+			users:    SSHUsers{"ubuntu", SSHUser(AutoGroupNonRoot), "admin"},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.users.ContainsNonRoot()
+			assert.Equal(t, tt.expected, result, "ContainsNonRoot() should return expected result")
+		})
+	}
+}
+
 func mustIPSet(prefixes ...string) *netipx.IPSet {
 	var builder netipx.IPSetBuilder
 	for _, p := range prefixes {