clientupdate: use a per-update Linux download directory (#21853)

Create a fresh directory for each Linux tarball download and remove it
after unpacking or a failed download. Continue to prefer the user cache
directory and use the temporary directory when the cache path is
unavailable.

Fixes tailscale/corp#50119
Fixes tailscale/corp#48680

Change-Id: Iebe13d922f8fa691f26fe17d908b295b7153aadb

Credit: Winfunc

Signed-off-by: Patrick O'Doherty <patrick@tailscale.com>

clientupdate/clientupdate.go

@@ -963,11 +963,12 @@ func (up *Updater) updateLinuxBinary() error {
 		return err
 	}
 	up.Logf("Extracting %q", dlPath)
-	if err := up.unpackLinuxTarball(dlPath); err != nil {
-		return err
+	err = up.unpackLinuxTarball(dlPath)
+	if err := os.RemoveAll(filepath.Dir(dlPath)); err != nil {
+		up.Logf("failed to clean up: %v", err)
 	}
-	if err := os.Remove(dlPath); err != nil {
-		up.Logf("failed to clean up %q: %v", dlPath, err)
+	if err != nil {
+		return err
 	}
 
 	err = restartSystemdUnit(up.Logf)
@@ -1040,13 +1041,17 @@ func (up *Updater) downloadLinuxTarball(ver string) (string, error) {
 	if err != nil {
 		dlDir = os.TempDir()
 	}
-	dlDir = filepath.Join(dlDir, "tailscale-update")
 	if err := os.MkdirAll(dlDir, 0700); err != nil {
 		return "", err
 	}
+	dlDir, err = os.MkdirTemp(dlDir, "tailscale-update")
+	if err != nil {
+		return "", err
+	}
 	pkgsPath := fmt.Sprintf("%s/tailscale_%s_%s.tgz", up.Track, ver, runtime.GOARCH)
 	dlPath := filepath.Join(dlDir, path.Base(pkgsPath))
 	if err := up.downloadURLToFile(pkgsPath, dlPath); err != nil {
+		os.RemoveAll(dlDir)
 		return "", err
 	}
 	return dlPath, nil

clientupdate/clientupdate_test.go

@@ -14,6 +14,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"runtime"
 	"slices"
 	"sort"
 	"strings"
@@ -586,6 +587,102 @@ unique="synology_88f6281_213air"
 	}
 }
 
+func TestDownloadLinuxTarballDirectory(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("Linux tarball downloads are only supported on Linux")
+	}
+	for _, name := range []string{"cache", "missing-cache", "temp"} {
+		t.Run(name, func(t *testing.T) {
+			base := t.TempDir()
+			t.Setenv("HOME", "")
+			t.Setenv("XDG_CACHE_HOME", "")
+			t.Setenv("TMPDIR", base)
+			if name != "temp" {
+				if name == "missing-cache" {
+					base = filepath.Join(base, "cache")
+				}
+				t.Setenv("XDG_CACHE_HOME", base)
+			}
+			// Leave an existing update directory and file untouched.
+			legacyDir := filepath.Join(base, "tailscale-update")
+			if name != "missing-cache" {
+				if err := os.Mkdir(legacyDir, 0700); err != nil {
+					t.Fatal(err)
+				}
+				if err := os.WriteFile(filepath.Join(legacyDir, "keep"), []byte("keep"), 0600); err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			dirs := make(chan []fs.FileInfo, 1)
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Path != "/distsign.pub" {
+					http.Error(w, "unavailable", http.StatusServiceUnavailable)
+					return
+				}
+				entries, err := os.ReadDir(base)
+				if err != nil {
+					t.Error(err)
+				}
+				var created []fs.FileInfo
+				for _, e := range entries {
+					if e.Name() == "tailscale-update" {
+						continue
+					}
+					info, err := e.Info()
+					if err != nil {
+						t.Error(err)
+						continue
+					}
+					created = append(created, info)
+				}
+				dirs <- created
+				http.Error(w, "unavailable", http.StatusServiceUnavailable)
+			}))
+			defer srv.Close()
+			up := &Updater{Arguments: Arguments{Logf: t.Logf, PkgsAddr: srv.URL, Track: StableTrack}}
+			var previous string
+			for range 2 {
+				if _, err := up.downloadLinuxTarball("1.0.0"); err == nil {
+					t.Fatal("download unexpectedly succeeded")
+				}
+				var created []fs.FileInfo
+				select {
+				case created = <-dirs:
+				default:
+					t.Fatal("download failed before reaching the package server")
+				}
+				if len(created) != 1 {
+					t.Errorf("got %d new download directories, want 1", len(created))
+				} else {
+					info := created[0]
+					if !info.IsDir() || info.Mode().Perm() != 0700 {
+						t.Errorf("download directory mode = %v, want drwx------", info.Mode())
+					}
+					if info.Name() == previous {
+						t.Errorf("reused download directory %q", previous)
+					}
+					previous = info.Name()
+				}
+				entries, err := os.ReadDir(base)
+				if err != nil {
+					t.Fatal(err)
+				}
+				for _, e := range entries {
+					if name == "missing-cache" || e.Name() != "tailscale-update" {
+						t.Errorf("download left behind %q", e.Name())
+					}
+				}
+			}
+			if name != "missing-cache" {
+				if got, err := os.ReadFile(filepath.Join(legacyDir, "keep")); err != nil || string(got) != "keep" {
+					t.Errorf("existing file = %q, %v; want keep, nil", got, err)
+				}
+			}
+		})
+	}
+}
+
 func TestUnpackLinuxTarball(t *testing.T) {
 	oldBinaryPaths := binaryPaths
 	t.Cleanup(func() { binaryPaths = oldBinaryPaths })