controlclient,health,ipnlocal,tailcfg: add DisplayMessage support

Updates tailscale/corp#27759

Signed-off-by: James Sanderson <[email protected]>

control/controlclient/map.go

@@ -90,6 +90,7 @@ type mapSession struct {
 	lastDomain             string
 	lastDomainAuditLogID   string
 	lastHealth             []string
+	lastDisplayMessages    map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage
 	lastPopBrowserURL      string
 	lastTKAInfo            *tailcfg.TKAInfo
 	lastNetmapSummary      string // from NetworkMap.VeryConcise
@@ -412,6 +413,21 @@ func (ms *mapSession) updateStateFromResponse(resp *tailcfg.MapResponse) {
 	if resp.Health != nil {
 		ms.lastHealth = resp.Health
 	}
+	if resp.DisplayMessages != nil {
+		if v, ok := resp.DisplayMessages["*"]; ok && v == nil {
+			ms.lastDisplayMessages = nil
+		}
+		for k, v := range resp.DisplayMessages {
+			if k == "*" {
+				continue
+			}
+			if v != nil {
+				mak.Set(&ms.lastDisplayMessages, k, *v)
+			} else {
+				delete(ms.lastDisplayMessages, k)
+			}
+		}
+	}
 	if resp.TKAInfo != nil {
 		ms.lastTKAInfo = resp.TKAInfo
 	}
@@ -831,14 +847,19 @@ func (ms *mapSession) sortedPeers() []tailcfg.NodeView {
 func (ms *mapSession) netmap() *netmap.NetworkMap {
 	peerViews := ms.sortedPeers()
 
-	// Convert all ms.lastHealth to the new [netmap.NetworkMap.DisplayMessages].
 	var msgs map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage
-	for _, h := range ms.lastHealth {
-		mak.Set(&msgs, tailcfg.DisplayMessageID("control-health-"+strhash(h)), tailcfg.DisplayMessage{
-			Title:    "Coordination server reports an issue",
-			Severity: tailcfg.SeverityMedium,
-			Text:     "The coordination server is reporting a health issue: " + h,
-		})
+	if len(ms.lastDisplayMessages) != 0 {
+		msgs = ms.lastDisplayMessages
+	} else if len(ms.lastHealth) > 0 {
+		// Convert all ms.lastHealth to the new [netmap.NetworkMap.DisplayMessages]
+		for _, h := range ms.lastHealth {
+			id := "control-health-" + strhash(h) // Unique ID in case there is more than one health message
+			mak.Set(&msgs, tailcfg.DisplayMessageID(id), tailcfg.DisplayMessage{
+				Title:    "Coordination server reports an issue",
+				Severity: tailcfg.SeverityMedium,
+				Text:     "The coordination server is reporting a health issue: " + h,
+			})
+		}
 	}
 
 	nm := &netmap.NetworkMap{

control/controlclient/map_test.go

@@ -16,6 +16,7 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"go4.org/mem"
 	"tailscale.com/control/controlknobs"
 	"tailscale.com/health"
@@ -1139,8 +1140,190 @@ func BenchmarkMapSessionDelta(b *testing.B) {
 	}
 }
 
+// TestNetmapDisplayMessage checks that the various diff operations
+// (add/update/delete/clear) for [tailcfg.DisplayMessage] in a
+// [tailcfg.MapResponse] work as expected.
+func TestNetmapDisplayMessage(t *testing.T) {
+	type test struct {
+		name         string
+		initialState *tailcfg.MapResponse
+		mapResponse  tailcfg.MapResponse
+		wantMessages map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage
+	}
+
+	tests := []test{
+		{
+			name: "basic-set",
+			mapResponse: tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"test-message": {
+						Title:               "Testing",
+						Text:                "This is a test message",
+						Severity:            tailcfg.SeverityHigh,
+						ImpactsConnectivity: true,
+						PrimaryAction: &tailcfg.DisplayMessageAction{
+							URL:   "https://www.example.com",
+							Label: "Learn more",
+						},
+					},
+				},
+			},
+			wantMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{
+				"test-message": {
+					Title:               "Testing",
+					Text:                "This is a test message",
+					Severity:            tailcfg.SeverityHigh,
+					ImpactsConnectivity: true,
+					PrimaryAction: &tailcfg.DisplayMessageAction{
+						URL:   "https://www.example.com",
+						Label: "Learn more",
+					},
+				},
+			},
+		},
+		{
+			name: "delete-one",
+			initialState: &tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"message-a": {
+						Title: "Message A",
+					},
+					"message-b": {
+						Title: "Message B",
+					},
+				},
+			},
+			mapResponse: tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"message-a": nil,
+				},
+			},
+			wantMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{
+				"message-b": {
+					Title: "Message B",
+				},
+			},
+		},
+		{
+			name: "update-one",
+			initialState: &tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"message-a": {
+						Title: "Message A",
+					},
+					"message-b": {
+						Title: "Message B",
+					},
+				},
+			},
+			mapResponse: tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"message-a": {
+						Title: "Message A updated",
+					},
+				},
+			},
+			wantMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{
+				"message-a": {
+					Title: "Message A updated",
+				},
+				"message-b": {
+					Title: "Message B",
+				},
+			},
+		},
+		{
+			name: "add-one",
+			initialState: &tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"message-a": {
+						Title: "Message A",
+					},
+				},
+			},
+			mapResponse: tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"message-b": {
+						Title: "Message B",
+					},
+				},
+			},
+			wantMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{
+				"message-a": {
+					Title: "Message A",
+				},
+				"message-b": {
+					Title: "Message B",
+				},
+			},
+		},
+		{
+			name: "delete-all",
+			initialState: &tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"message-a": {
+						Title: "Message A",
+					},
+					"message-b": {
+						Title: "Message B",
+					},
+				},
+			},
+			mapResponse: tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"*": nil,
+				},
+			},
+			wantMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{},
+		},
+		{
+			name: "delete-all-and-add",
+			initialState: &tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"message-a": {
+						Title: "Message A",
+					},
+					"message-b": {
+						Title: "Message B",
+					},
+				},
+			},
+			mapResponse: tailcfg.MapResponse{
+				DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+					"*": nil,
+					"message-c": {
+						Title: "Message C",
+					},
+				},
+			},
+			wantMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{
+				"message-c": {
+					Title: "Message C",
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ms := newTestMapSession(t, nil)
+
+			if test.initialState != nil {
+				ms.netmapForResponse(test.initialState)
+			}
+
+			nm := ms.netmapForResponse(&test.mapResponse)
+
+			if diff := cmp.Diff(test.wantMessages, nm.DisplayMessages, cmpopts.EquateEmpty()); diff != "" {
+				t.Errorf("unexpected warnings (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
 // TestNetmapHealthIntegration checks that we get the expected health warnings
-// from processing a map response and passing the NetworkMap to a health tracker
+// from processing a [tailcfg.MapResponse] containing health messages and passing the
+// [netmap.NetworkMap] to a [health.Tracker].
 func TestNetmapHealthIntegration(t *testing.T) {
 	ms := newTestMapSession(t, nil)
 	ht := health.Tracker{}
@@ -1182,3 +1365,56 @@ func TestNetmapHealthIntegration(t *testing.T) {
 		t.Fatalf("CurrentStatus().Warnings[\"control-health*\"] different than expected (-want +got)\n%s", d)
 	}
 }
+
+// TestNetmapDisplayMessageIntegration checks that we get the expected health
+// warnings from processing a [tailcfg.MapResponse] that contains DisplayMessages and
+// passing the [netmap.NetworkMap] to a [health.Tracker].
+func TestNetmapDisplayMessageIntegration(t *testing.T) {
+	ms := newTestMapSession(t, nil)
+	ht := health.Tracker{}
+
+	ht.SetIPNState("NeedsLogin", true)
+	ht.GotStreamedMapResponse()
+	baseWarnings := ht.CurrentState().Warnings
+
+	nm := ms.netmapForResponse(&tailcfg.MapResponse{
+		DisplayMessages: map[tailcfg.DisplayMessageID]*tailcfg.DisplayMessage{
+			"test-message": {
+				Title:               "Testing",
+				Text:                "This is a test message",
+				Severity:            tailcfg.SeverityHigh,
+				ImpactsConnectivity: true,
+				PrimaryAction: &tailcfg.DisplayMessageAction{
+					URL:   "https://www.example.com",
+					Label: "Learn more",
+				},
+			},
+		},
+	})
+	ht.SetControlHealth(nm.DisplayMessages)
+
+	state := ht.CurrentState()
+
+	// Ignore warnings that aren't from the netmap
+	for k := range baseWarnings {
+		delete(state.Warnings, k)
+	}
+
+	want := map[health.WarnableCode]health.UnhealthyState{
+		"test-message": {
+			WarnableCode:        "test-message",
+			Title:               "Testing",
+			Text:                "This is a test message",
+			Severity:            health.SeverityHigh,
+			ImpactsConnectivity: true,
+			PrimaryAction: &health.UnhealthyStateAction{
+				URL:   "https://www.example.com",
+				Label: "Learn more",
+			},
+		},
+	}
+
+	if diff := cmp.Diff(want, state.Warnings); diff != "" {
+		t.Errorf("unexpected message contents (-want +got):\n%s", diff)
+	}
+}

health/state.go

@@ -30,10 +30,19 @@ type UnhealthyState struct {
 	Severity            Severity
 	Title               string
 	Text                string
-	BrokenSince         *time.Time     `json:",omitempty"`
-	Args                Args           `json:",omitempty"`
-	DependsOn           []WarnableCode `json:",omitempty"`
-	ImpactsConnectivity bool           `json:",omitempty"`
+	BrokenSince         *time.Time            `json:",omitempty"`
+	Args                Args                  `json:",omitempty"`
+	DependsOn           []WarnableCode        `json:",omitempty"`
+	ImpactsConnectivity bool                  `json:",omitempty"`
+	PrimaryAction       *UnhealthyStateAction `json:",omitempty"`
+}
+
+// UnhealthyStateAction represents an action (URL and link) to be presented to
+// the user associated with an [UnhealthyState]. Analogous to
+// [tailcfg.DisplayMessageAction].
+type UnhealthyStateAction struct {
+	URL   string
+	Label string
 }
 
 // unhealthyState returns a unhealthyState of the Warnable given its current warningState.
@@ -102,15 +111,23 @@ func (t *Tracker) CurrentState() *State {
 	}
 
 	for id, msg := range t.lastNotifiedControlMessages {
-		code := WarnableCode(id)
-		wm[code] = UnhealthyState{
-			WarnableCode:        code,
+		state := UnhealthyState{
+			WarnableCode:        WarnableCode(id),
 			Severity:            severityFromTailcfg(msg.Severity),
 			Title:               msg.Title,
 			Text:                msg.Text,
 			ImpactsConnectivity: msg.ImpactsConnectivity,
 			// TODO(tailscale/corp#27759): DependsOn?
 		}
+
+		if msg.PrimaryAction != nil {
+			state.PrimaryAction = &UnhealthyStateAction{
+				URL:   msg.PrimaryAction.URL,
+				Label: msg.PrimaryAction.Label,
+			}
+		}
+
+		wm[state.WarnableCode] = state
 	}
 
 	return &State{

ipn/ipnlocal/local.go

@@ -5828,7 +5828,14 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
 	b.pauseOrResumeControlClientLocked()
 
 	if nm != nil {
-		b.health.SetControlHealth(nm.DisplayMessages)
+		messages := make(map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage)
+		for id, msg := range nm.DisplayMessages {
+			if msg.PrimaryAction != nil && !b.validPopBrowserURL(msg.PrimaryAction.URL) {
+				msg.PrimaryAction = nil
+			}
+			messages[id] = msg
+		}
+		b.health.SetControlHealth(messages)
 	} else {
 		b.health.SetControlHealth(nil)
 	}

ipn/ipnlocal/local_test.go

@@ -5339,3 +5339,68 @@ func TestSrcCapPacketFilter(t *testing.T) {
 		t.Error("IsDrop() for node without cap = false, want true")
 	}
 }
+
+func TestDisplayMessages(t *testing.T) {
+	b := newTestLocalBackend(t)
+
+	// Pretend we're in a map poll so health updates get processed
+	ht := b.HealthTracker()
+	ht.SetIPNState("NeedsLogin", true)
+	ht.GotStreamedMapResponse()
+
+	b.setNetMapLocked(&netmap.NetworkMap{
+		DisplayMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{
+			"test-message": {
+				Title: "Testing",
+			},
+		},
+	})
+
+	state := ht.CurrentState()
+	_, ok := state.Warnings["test-message"]
+
+	if !ok {
+		t.Error("no warning found with id 'test-message'")
+	}
+}
+
+// TestDisplayMessagesURLFilter tests that we filter out any URLs that are not
+// valid as a pop browser URL (see [LocalBackend.validPopBrowserURL]).
+func TestDisplayMessagesURLFilter(t *testing.T) {
+	b := newTestLocalBackend(t)
+
+	// Pretend we're in a map poll so health updates get processed
+	ht := b.HealthTracker()
+	ht.SetIPNState("NeedsLogin", true)
+	ht.GotStreamedMapResponse()
+
+	b.setNetMapLocked(&netmap.NetworkMap{
+		DisplayMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{
+			"test-message": {
+				Title:    "Testing",
+				Severity: tailcfg.SeverityHigh,
+				PrimaryAction: &tailcfg.DisplayMessageAction{
+					URL:   "https://www.evil.com",
+					Label: "Phishing Link",
+				},
+			},
+		},
+	})
+
+	state := ht.CurrentState()
+	got, ok := state.Warnings["test-message"]
+
+	if !ok {
+		t.Fatal("no warning found with id 'test-message'")
+	}
+
+	want := health.UnhealthyState{
+		WarnableCode: "test-message",
+		Title:        "Testing",
+		Severity:     health.SeverityHigh,
+	}
+
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("Unexpected message content (-want/+got):\n%s", diff)
+	}
+}

tailcfg/tailcfg.go

@@ -161,7 +161,8 @@ type CapabilityVersion int
 //   - 114: 2025-01-30: NodeAttrMaxKeyDuration CapMap defined, clients might use it (no tailscaled code change) (#14829)
 //   - 115: 2025-03-07: Client understands DERPRegion.NoMeasureNoHome.
 //   - 116: 2025-05-05: Client serves MagicDNS "AAAA" if NodeAttrMagicDNSPeerAAAA set on self node
-const CurrentCapabilityVersion CapabilityVersion = 116
+//   - 117: 2025-05-28: Client understands DisplayMessages (structured health messages), but not necessarily PrimaryAction.
+const CurrentCapabilityVersion CapabilityVersion = 117
 
 // ID is an integer ID for a user, node, or login allocated by the
 // control plane.
@@ -2030,11 +2031,29 @@ type MapResponse struct {
 	// known problems). A non-zero length slice are the list of problems that
 	// the control plane sees.
 	//
+	// Either this will be set, or DisplayMessages will be set, but not both.
+	//
 	// Note that this package's type, due its use of a slice and omitempty, is
 	// unable to marshal a zero-length non-nil slice. The control server needs
 	// to marshal this type using a separate type. See MapResponse docs.
 	Health []string `json:",omitempty"`
 
+	// DisplayMessages sets the health state of the node from the control
+	// plane's perspective.
+	//
+	// Either this will be set, or Health will be set, but not both.
+	//
+	// The map keys are IDs that uniquely identify the type of health issue. The
+	// map values are the messages. If the server sends down a map with entries,
+	// the client treats it as a patch: new entries are added, keys with a value
+	// of nil are deleted, existing entries with new values are updated. A nil
+	// map and an empty map both mean no change has occurred since the last
+	// update.
+	//
+	// As a special case, the map key "*" with a value of nil means to clear all
+	// prior display messages before processing the other map entries.
+	DisplayMessages map[DisplayMessageID]*DisplayMessage `json:",omitempty"`
+
 	// SSHPolicy, if non-nil, updates the SSH policy for how incoming
 	// SSH connections should be handled.
 	SSHPolicy *SSHPolicy `json:",omitempty"`
@@ -2079,24 +2098,53 @@ type MapResponse struct {
 }
 
 // DisplayMessage represents a health state of the node from the control plane's
-// perspective. It is deliberately similar to health.Warnable as both get
-// converted into health.UnhealthyState to be sent to the GUI.
+// perspective. It is deliberately similar to [health.Warnable] as both get
+// converted into [health.UnhealthyState] to be sent to the GUI.
 type DisplayMessage struct {
 	// Title is a string that the GUI uses as title for this message. The title
-	// should be short and fit in a single line.
+	// should be short and fit in a single line. It should not end in a period.
+	//
+	// Example: "Network may be blocking Tailscale".
+	//
+	// See the various instantiations of [health.Warnable] for more examples.
 	Title string
 
-	// Text is an extended string that the GUI will display to the user.
+	// Text is an extended string that the GUI will display to the user. This
+	// could be multiple sentences explaining the issue in more detail.
+	//
+	// Example: "macOS Screen Time seems to be blocking Tailscale. Try disabling
+	// Screen Time in System Settings > Screen Time > Content & Privacy > Access
+	// to Web Content."
+	//
+	// See the various instantiations of [health.Warnable] for more examples.
 	Text string
 
 	// Severity is the severity of the DisplayMessage, which the GUI can use to
-	// determine how to display it. Maps to health.Severity.
+	// determine how to display it. Maps to [health.Severity].
 	Severity DisplayMessageSeverity
 
 	// ImpactsConnectivity is whether the health problem will impact the user's
 	// ability to connect to the Internet or other nodes on the tailnet, which
 	// the GUI can use to determine how to display it.
 	ImpactsConnectivity bool `json:",omitempty"`
+
+	// Primary action, if present, represents the action to allow the user to
+	// take when interacting with this message. For example, if the
+	// DisplayMessage is shown via a notification, the action label might be a
+	// button on that notification and clicking the button would open the URL.
+	PrimaryAction *DisplayMessageAction `json:",omitempty"`
+}
+
+// DisplayMessageAction represents an action (URL and link) to be presented to
+// the user associated with a [DisplayMessage].
+type DisplayMessageAction struct {
+	// URL is the URL to navigate to when the user interacts with this action
+	URL string
+
+	// Label is the call to action for the UI to display on the UI element that
+	// will open the URL (such as a button or link). For example, "Sign in" or
+	// "Learn more".
+	Label string
 }
 
 // DisplayMessageID is a string that uniquely identifies the kind of health

types/netmap/nodemut.go

@@ -163,6 +163,7 @@ func mapResponseContainsNonPatchFields(res *tailcfg.MapResponse) bool {
 		res.PacketFilters != nil ||
 		res.UserProfiles != nil ||
 		res.Health != nil ||
+		res.DisplayMessages != nil ||
 		res.SSHPolicy != nil ||
 		res.TKAInfo != nil ||
 		res.DomainDataPlaneAuditLogID != "" ||