From 1a40efe9ae2edd50cfcea2b589c2b2639c7a12b6 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 21 Oct 2026 17:23:27 +0000 Subject: [PATCH] policy/v2: refuse autogroup:member as an sshTests destination Tailscale SaaS refuses it when parsing; headscale evaluated it. --- hscontrol/policy/v2/sshtester_compat_test.go | 4 ---- hscontrol/policy/v2/types.go | 8 ++++---- hscontrol/policy/v2/types_test.go | 12 ++++++++++++ integration/cli_policy_test.go | 8 ++++---- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/hscontrol/policy/v2/sshtester_compat_test.go b/hscontrol/policy/v2/sshtester_compat_test.go index c75bdc9f3..49befe3e8 100644 --- a/hscontrol/policy/v2/sshtester_compat_test.go +++ b/hscontrol/policy/v2/sshtester_compat_test.go @@ -31,9 +31,6 @@ var knownSSHTesterDivergences = map[string]string{ "sshtest-report-multi/label": sshTestNodeLabel, "sshtest-src-unassigned-ip-accept/label": sshTestNodeLabel, - "sshtest-fail-label-autogroup-member/message": sshTestDstMember, - "sshtest-check-reason-mixed-alias/message": sshTestDstMember, - "sshtest-fail-label-same-node-twice/breakdown": "SSHTEST_PER_ALIAS: SaaS reports each dst alias, also two naming one node; headscale checks each node once", "sshtest-fail-label-ip-no-device/breakdown": sshTestNoDevice, "sshtest-report-multi/breakdown": "SSHTEST_PER_ALIAS+SSHTEST_NO_DEVICE: SaaS reports one line per dst alias and a deviceless alias as not reached; headscale one per node and an error", @@ -42,7 +39,6 @@ var knownSSHTesterDivergences = map[string]string{ const ( sshTestNodeLabel = "SSHTEST_NODE_LABEL: headscale names each failing dst node by hostname; SaaS names the dst as written" - sshTestDstMember = "SSHTEST_DST_MEMBER: SaaS refuses autogroup:member as an sshTests destination when parsing; headscale evaluates it" sshTestNoDevice = "SSHTEST_NO_DEVICE: SaaS reports a dst that selects no device as not reached; headscale as an error" ) diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 96011ece4..9e40e4b47 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -3455,7 +3455,8 @@ func isAddressRange(pol *Policy, alias Alias) bool { // // - `host:port` suffixes (parsed as an unknown tag), // - multi-host CIDRs (raw `/N` or a hosts: entry resolving wider), -// - autogroup:internet (valid as ACL dst only). +// - autogroup:internet (valid as ACL dst only), +// - autogroup:member, which Tailscale SaaS refuses here. // // A bare IP literal (single-host /BitLen prefix) is accepted. Tag // entries must exist in tagOwners. @@ -3468,9 +3469,8 @@ func validateSSHTestDestination(pol *Policy, alias Alias) error { switch a := alias.(type) { case *AutoGroup: - // autogroup:internet is the only autogroup not valid here; - // member/tagged/self/nonroot pass to engine evaluation. - if *a == AutoGroupInternet { + // tagged/self/nonroot pass to engine evaluation. + if *a == AutoGroupInternet || *a == AutoGroupMember { return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst) } diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 79d13a9c3..f0f19a911 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -6077,6 +6077,18 @@ func TestUnmarshalPolicySSHTests(t *testing.T) { {"src": "thor@example.org", "dst": ["autogroup:internet"], "accept": ["root"]} ] } +`, + wantErr: ErrSSHTestDstDisallowedElement, + }, + { + name: "dst-autogroup-member", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["autogroup:member"], "accept": ["root"]} + ] +} `, wantErr: ErrSSHTestDstDisallowedElement, }, diff --git a/integration/cli_policy_test.go b/integration/cli_policy_test.go index 13f20dad1..4811355c1 100644 --- a/integration/cli_policy_test.go +++ b/integration/cli_policy_test.go @@ -201,8 +201,8 @@ func TestPolicyCheckCommand(t *testing.T) { // TestSSHTestsRejectFailingPolicy asserts `headscale policy set` rejects // a policy whose sshTests fail, surfaces the engine's "test(s) failed" -// sentinel, and leaves the stored policy unchanged. autogroup:member as -// dst lets every scenario node count, so no tagged node is needed. +// sentinel, and leaves the stored policy unchanged. The tests name user1's +// node by its user, so no tagged node is needed. func TestSSHTestsRejectFailingPolicy(t *testing.T) { IntegrationSkip(t) @@ -226,7 +226,7 @@ func TestSSHTestsRejectFailingPolicy(t *testing.T) { SSHTests: []policyv2.SSHPolicyTest{ { Src: usernamep(user1), - Dst: policyv2.SSHTestDestinations{new(policyv2.AutoGroupMember)}, + Dst: policyv2.SSHTestDestinations{usernamep(user1)}, Accept: []policyv2.SSHUser{policyv2.SSHUser("root")}, }, }, @@ -238,7 +238,7 @@ func TestSSHTestsRejectFailingPolicy(t *testing.T) { badPolicy.SSHTests = []policyv2.SSHPolicyTest{ { Src: usernamep(user2), - Dst: policyv2.SSHTestDestinations{new(policyv2.AutoGroupMember)}, + Dst: policyv2.SSHTestDestinations{usernamep(user1)}, Accept: []policyv2.SSHUser{policyv2.SSHUser("root")}, }, }