From db03d4e19da508fe29c96043e5fd49156e577f60 Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Mon, 19 Oct 2026 15:34:25 -0700 Subject: [PATCH] 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 --- clientupdate/clientupdate.go | 15 +++-- clientupdate/clientupdate_test.go | 97 +++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 5 deletions(-) diff --git a/clientupdate/clientupdate.go b/clientupdate/clientupdate.go index 20d2e7b0a..f6a51b4e0 100644 --- a/clientupdate/clientupdate.go +++ b/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 diff --git a/clientupdate/clientupdate_test.go b/clientupdate/clientupdate_test.go index d2e7601be..713aa80b5 100644 --- a/clientupdate/clientupdate_test.go +++ b/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 })