From c9fb05b6f5120a54e95ffb903a20451aa2f507ba Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 27 May 2026 21:23:58 +0000 Subject: [PATCH] ipn/ipnlocal: don't dup-suppress UserProfiles on IPNBus on profile switches Fixes #19889 Change-Id: I324a735c13772c0c79ed7392c0baa5064b34823b Signed-off-by: Brad Fitzpatrick --- ipn/ipnlocal/local.go | 32 +++++++++++++++++++++++++ ipn/ipnlocal/local_test.go | 48 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index ff49c4b06..57733a66b 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -165,6 +165,12 @@ type watchSession struct { // "control re-announces the same profile" case is a pointer-cheap // equality check. lastSentUserProfile map[tailcfg.UserID]tailcfg.UserProfileView + + // lastSentSelf identifies the self node most recently delivered to + // this session via [Notify.SelfChange]. A change to self is a profile + // boundary for implicit bus state, so per-session dedup state such as + // lastSentUserProfile must be reset when this identity changes. + lastSentSelf tailcfg.NodeView } var ( @@ -3873,6 +3879,10 @@ func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify) stripPatches := len(n.PeerChangedPatch) > 0 && !wantsPeerPatches promotePatches := len(n.PeerChangedPatch) > 0 && wantsPeerChanges && !wantsPeerPatches + if sess.selfChangeResetsImplicitState(n.SelfChange.View()) { + sess.lastSentUserProfile = nil + } + // UserProfiles ride alongside peer changes and are gated on the // same opt-in. Sessions that didn't ask for peer changes get the // field stripped entirely; opted-in sessions get a per-session @@ -3934,6 +3944,28 @@ func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify) return &nCopy } +// selfChangeResetsImplicitState reports whether self changes the identity of +// the self node last delivered to sess. WatchIPNBus sessions survive profile +// switches, but a self-node identity change is a boundary for implicit bus +// state: publishers must resend any implicit state, such as UserProfiles, +// needed to interpret subsequent peer deltas. +func (sess *watchSession) selfChangeResetsImplicitState(self tailcfg.NodeView) bool { + if !self.Valid() { + return false + } + reset := !sess.lastSentSelf.Valid() || !sameSelfNode(sess.lastSentSelf, self) + sess.lastSentSelf = self + return reset +} + +func sameSelfNode(a, b tailcfg.NodeView) bool { + // Include User in the identity even though a node can move users, for + // example when an untagged auth-key node is later tagged. Treating that as + // a boundary only causes redundant implicit-state delivery; missing the + // boundary would leave consumers unable to resolve newly referenced users. + return a.ID() == b.ID() && a.StableID() == b.StableID() && a.User() == b.User() +} + // hasPeerChangeWatcherLocked reports whether any active watcher wants peer-set // notifications. b.mu must be held. func (b *LocalBackend) hasPeerChangeWatcherLocked() bool { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 3c31a1dd6..e8d7e0743 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2319,6 +2319,54 @@ func TestNotifyForSessionUserProfilesGating(t *testing.T) { }) } +func TestNotifyForSessionUserProfilesDedupResetsOnSelfChange(t *testing.T) { + b := newTestLocalBackend(t) + + deliver := func(sess *watchSession, n *ipn.Notify) *ipn.Notify { + b.mu.Lock() + defer b.mu.Unlock() + return b.notifyForSessionLocked(sess, n) + } + + sess := &watchSession{mask: ipn.NotifyPeerChanges} + self1 := &tailcfg.Node{ID: 1, StableID: "self1", User: 10} + self2 := &tailcfg.Node{ID: 2, StableID: "self2", User: 20} + profiles1 := map[tailcfg.UserID]tailcfg.UserProfileView{ + 10: (&tailcfg.UserProfile{ID: 10, LoginName: "alice@example.com", DisplayName: "Alice"}).View(), + 11: (&tailcfg.UserProfile{ID: 11, LoginName: "peer@example.com", DisplayName: "Peer"}).View(), + } + profiles2 := map[tailcfg.UserID]tailcfg.UserProfileView{ + 20: (&tailcfg.UserProfile{ID: 20, LoginName: "bob@example.com", DisplayName: "Bob"}).View(), + } + + n := deliver(sess, &ipn.Notify{SelfChange: self1, UserProfiles: profiles1}) + if got, want := len(n.UserProfiles), len(profiles1); got != want { + t.Fatalf("initial UserProfiles len = %d; want %d", got, want) + } + n = deliver(sess, &ipn.Notify{SelfChange: self1, UserProfiles: profiles1}) + if len(n.UserProfiles) != 0 { + t.Fatalf("same self duplicate UserProfiles = %v; want empty", n.UserProfiles) + } + n = deliver(sess, &ipn.Notify{SelfChange: self2, UserProfiles: profiles2}) + if got, want := len(n.UserProfiles), len(profiles2); got != want { + t.Fatalf("new self UserProfiles len = %d; want %d", got, want) + } + n = deliver(sess, &ipn.Notify{SelfChange: self1, UserProfiles: profiles1}) + if got, want := len(n.UserProfiles), len(profiles1); got != want { + t.Fatalf("returned self UserProfiles len = %d; want %d", got, want) + } + + sess = &watchSession{mask: ipn.NotifyPeerChanges} + n = deliver(sess, &ipn.Notify{UserProfiles: profiles1}) + if got, want := len(n.UserProfiles), len(profiles1); got != want { + t.Fatalf("pre-self UserProfiles len = %d; want %d", got, want) + } + n = deliver(sess, &ipn.Notify{SelfChange: self1, UserProfiles: profiles1}) + if got, want := len(n.UserProfiles), len(profiles1); got != want { + t.Fatalf("first self UserProfiles len = %d; want %d", got, want) + } +} + // tests LocalBackend.updateNetmapDeltaLocked func TestUpdateNetmapDelta(t *testing.T) { b := newTestLocalBackend(t)