From 5aded15bf2e53f21892decc702370346dd39876e Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@tailscale.com>
Date: Wed, 23 Sep 2026 08:52:20 +0000
Subject: [PATCH] policy/v2: suggest approved exit nodes by default

Matches SaaS; Apple clients hide the exit-node list without a suggestion.

Fixes #3415

0.29's tailscale has no tailcfg/nodecap package, so the constant keeps its
tailcfg name here and in the test.

(cherry picked from commit c604874dba808bec6b5f9497cf2ff63171205804)
---
 hscontrol/policy/v2/tailnet_state_caps.go | 36 +++++-------
 hscontrol/servertest/nodeattrs_test.go    | 67 +++++++++++++++++++++++
 2 files changed, 81 insertions(+), 22 deletions(-)

diff --git a/hscontrol/policy/v2/tailnet_state_caps.go b/hscontrol/policy/v2/tailnet_state_caps.go
index e4b0c312..3d7ba0f8 100644
--- a/hscontrol/policy/v2/tailnet_state_caps.go
+++ b/hscontrol/policy/v2/tailnet_state_caps.go
@@ -27,35 +27,27 @@ import (
 // most peers.
 //
 // Caps the client reads from the peer view rather than the self view
-// (suggest-exit-node, dns-subdomain-resolve — see
-// ipn/ipnlocal/local.go:7534 and node_backend.go:745) are emitted only
-// when the peer satisfies the cap's emission condition. This function
-// encodes those conditions; the mapper calls it from
+// ([tailcfg.NodeAttrSuggestExitNode], read by
+// [tailscale.com/ipn/ipnlocal.LocalBackend.SuggestExitNode], and
+// [tailcfg.NodeAttrDNSSubdomainResolve]) are emitted only when the
+// peer satisfies the cap's emission condition. This function encodes
+// those conditions; the mapper calls it from
 // [mapper.MapResponseBuilder.buildTailPeers] and the compat test calls
 // it to compute the expected per-peer wire shape.
 func PeerCapMap(peer types.NodeView, peerSelfCaps tailcfg.NodeCapMap) tailcfg.NodeCapMap {
-	if len(peerSelfCaps) == 0 {
+	// suggest-exit-node — surfaced on Peer.CapMap when the peer
+	// advertises exit routes AND those routes are approved, with or
+	// without a nodeAttrs grant: SaaS stamps it by default, and Apple
+	// clients hide the exit-node list without a suggestion. A policy
+	// value, if any, wins. Approval gating prevents the suggestion from
+	// following an advertised-but-not-yet-trusted node.
+	if !peer.IsExitNode() {
 		return nil
 	}
 
-	var out tailcfg.NodeCapMap
-
-	// suggest-exit-node — surfaced on Peer.CapMap when the peer
-	// advertises exit routes AND those routes are approved. Client
-	// reads at ipn/ipnlocal/local.go:7534. Approval gating prevents
-	// the suggestion from following an advertised-but-not-yet-trusted
-	// node.
-	if peer.IsExitNode() {
-		if v, ok := peerSelfCaps[tailcfg.NodeAttrSuggestExitNode]; ok {
-			if out == nil {
-				out = tailcfg.NodeCapMap{}
-			}
-
-			out[tailcfg.NodeAttrSuggestExitNode] = v
-		}
+	return tailcfg.NodeCapMap{
+		tailcfg.NodeAttrSuggestExitNode: peerSelfCaps[tailcfg.NodeAttrSuggestExitNode],
 	}
-
-	return out
 }
 
 // unmodelledTailnetStateCaps lists [tailcfg.NodeCapability] values
diff --git a/hscontrol/servertest/nodeattrs_test.go b/hscontrol/servertest/nodeattrs_test.go
index 26179bf6..00e93c80 100644
--- a/hscontrol/servertest/nodeattrs_test.go
+++ b/hscontrol/servertest/nodeattrs_test.go
@@ -395,3 +395,70 @@ func TestNodeAttrsSuggestExitNodeOnPeerCapMap(t *testing.T) {
 			return false
 		})
 }
+
+// TestSuggestExitNodeDefaultOnPeerCapMap covers the SaaS default: an
+// approved exit node carries suggest-exit-node on its peer view with
+// no nodeAttrs grant, and loses it when approval is withdrawn. Apple
+// clients hide the exit-node list without it (issue #3415).
+func TestSuggestExitNodeDefaultOnPeerCapMap(t *testing.T) {
+	t.Parallel()
+
+	srv := servertest.NewServer(t)
+	user := srv.CreateUser(t, "sed-user")
+
+	exit := servertest.NewClient(t, srv, "sed-exit", servertest.WithUser(user))
+	viewer := servertest.NewClient(t, srv, "sed-viewer", servertest.WithUser(user))
+
+	exit.WaitForPeers(t, 1, 10*time.Second)
+	viewer.WaitForPeers(t, 1, 10*time.Second)
+
+	exitRoutes := []netip.Prefix{
+		netip.MustParsePrefix("0.0.0.0/0"),
+		netip.MustParsePrefix("::/0"),
+	}
+
+	exit.Direct().SetHostinfo(&tailcfg.Hostinfo{
+		BackendLogID: "servertest-sed-exit",
+		Hostname:     "sed-exit",
+		RoutableIPs:  exitRoutes,
+	})
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	require.NoError(t, exit.Direct().SendUpdate(ctx))
+	cancel()
+
+	peerHasCap := func(want bool) func(*netmap.NetworkMap) bool {
+		return func(nm *netmap.NetworkMap) bool {
+			if nm == nil {
+				return false
+			}
+
+			for _, peer := range nm.Peers {
+				if peer.ComputedName() == "sed-exit" {
+					return peer.CapMap().Contains(tailcfg.NodeAttrSuggestExitNode) == want
+				}
+			}
+
+			return false
+		}
+	}
+
+	exitID := findNodeID(t, srv, "sed-exit")
+	_, ch, err := srv.State().SetApprovedRoutes(exitID, exitRoutes)
+	require.NoError(t, err)
+	srv.App.Change(ch)
+
+	viewer.WaitForCondition(t, "peer suggest-exit-node without nodeAttrs",
+		10*time.Second, peerHasCap(true))
+
+	// SaaS never stamps it on the exit node's own view.
+	require.False(t, hasCap(exit.Netmap(), tailcfg.NodeAttrSuggestExitNode),
+		"suggest-exit-node must not appear on SelfNode by default")
+
+	_, ch, err = srv.State().SetApprovedRoutes(exitID, nil)
+	require.NoError(t, err)
+	srv.App.Change(ch)
+
+	viewer.WaitForCondition(t, "peer suggest-exit-node gone after unapprove",
+		10*time.Second, peerHasCap(false))
+}