From 06502b9048be84b208dda7ef63fed7c174cafd19 Mon Sep 17 00:00:00 2001 From: Andrew Lytvynov Date: Mon, 22 Apr 2024 15:55:25 -0700 Subject: [PATCH] ipn/ipnlocal: reset auto-updates if unsupported on profile load (#11838) Prior to https://github.com/tailscale/tailscale/pull/11814/commits/1613b18f8280c2bce786980532d012c9f0454fa2#diff-314ba0d799f70c8998940903efb541e511f352b39a9eeeae8d475c921d66c2ac, nodes could set AutoUpdate.Apply=true on unsupported platforms via `EditPrefs`. Specifically, this affects tailnets where default auto-updates are on. Fix up those invalid prefs on profile reload, as a migration. Updates #11544 Signed-off-by: Andrew Lytvynov --- ipn/ipnlocal/profiles.go | 11 +++++++++++ ipn/ipnlocal/profiles_test.go | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index a7c6ab944..6acdacc30 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "tailscale.com/clientupdate" "tailscale.com/envknob" "tailscale.com/ipn" "tailscale.com/types/logger" @@ -366,6 +367,16 @@ func (pm *profileManager) loadSavedPrefs(key ipn.StateKey) (ipn.PrefsView, error ipn.IsLoginServerSynonym(savedPrefs.ControlURL) { savedPrefs.ControlURL = "" } + // Before + // https://github.com/tailscale/tailscale/pull/11814/commits/1613b18f8280c2bce786980532d012c9f0454fa2#diff-314ba0d799f70c8998940903efb541e511f352b39a9eeeae8d475c921d66c2ac + // prefs could set AutoUpdate.Apply=true via EditPrefs or tailnet + // auto-update defaults. After that change, such value is "invalid" and + // cause any EditPrefs calls to fail (other than disabling autp-updates). + // + // Reset AutoUpdate.Apply if we detect such invalid prefs. + if savedPrefs.AutoUpdate.Apply.EqualBool(true) && !clientupdate.CanAutoUpdate() { + savedPrefs.AutoUpdate.Apply.Clear() + } return savedPrefs.View(), nil } diff --git a/ipn/ipnlocal/profiles_test.go b/ipn/ipnlocal/profiles_test.go index 47f75baba..101aeb9c9 100644 --- a/ipn/ipnlocal/profiles_test.go +++ b/ipn/ipnlocal/profiles_test.go @@ -11,6 +11,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "tailscale.com/clientupdate" "tailscale.com/ipn" "tailscale.com/ipn/store/mem" "tailscale.com/tailcfg" @@ -339,6 +340,7 @@ func TestProfileManagement(t *testing.T) { t.Fatalf("Profiles = %v; want %v", profiles, wantProfiles) } p := pm.CurrentPrefs() + t.Logf("\tCurrentPrefs = %s", p.Pretty()) if !p.Valid() { t.Fatalf("CurrentPrefs = %v; want valid", p) } @@ -458,6 +460,27 @@ func TestProfileManagement(t *testing.T) { delete(wantProfiles, "tagged-node.2.ts.net") wantCurProfile = "user@2.example.com" checkProfiles(t) + + if !clientupdate.CanAutoUpdate() { + t.Logf("Save an invalid AutoUpdate pref value") + prefs := pm.CurrentPrefs().AsStruct() + prefs.AutoUpdate.Apply.Set(true) + if err := pm.SetPrefs(prefs.View(), ipn.NetworkProfile{}); err != nil { + t.Fatal(err) + } + if !pm.CurrentPrefs().AutoUpdate().Apply.EqualBool(true) { + t.Fatal("SetPrefs failed to save auto-update setting") + } + // Re-load profiles to trigger migration for invalid auto-update value. + pm, err = newProfileManagerWithGOOS(store, logger.Discard, "linux") + if err != nil { + t.Fatal(err) + } + checkProfiles(t) + if pm.CurrentPrefs().AutoUpdate().Apply.EqualBool(true) { + t.Fatal("invalid auto-update setting persisted after reload") + } + } } // TestProfileManagementWindows tests going into and out of Unattended mode on