From 4f364575630d43edbc25d582a389d06e6e8d356d Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Tue, 22 Sep 2026 13:02:00 -0400 Subject: [PATCH] fix(updater): probe swap path before enabling self-updates The 2.0.0 update helper renames staged files from os.TempDir() onto the running executable's directory. On the common Arch/Fedora layout where /tmp is tmpfs and $HOME is persistent, that rename crosses filesystems and fails with EXDEV ("invalid cross-device link"). The helper's recovery path then deletes the working binary while failing to restore it: twenty observed attempts, an orphaned .bak, and no running app. Guard against this at startup instead of mid-swap: - swapPathProven() exercises the exact move the helper will perform (os.Rename from os.TempDir() into the executable's directory) with a throwaway probe file, then cleans up after itself. - maybeEnableUpdater() skips the updater entirely when the probe fails or the build version is empty, logging the reason. A loud skip beats a destructive attempt that can strand the user with no app. - renameProbeOK() is the testable core, taking explicit tmp/target dirs. - updater_probe_test.go covers same-filesystem success with no leftover files, and fail-closed behavior for missing tmp dir, missing target dir, and unwritable target dir. - V3_MIGRATION.md documents the probe and the incident that motivated it. --- docs/V3_MIGRATION.md | 7 +++++- updater.go | 52 +++++++++++++++++++++++++++++++++++++++ updater_probe_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 updater_probe_test.go diff --git a/docs/V3_MIGRATION.md b/docs/V3_MIGRATION.md index e0657a3..00348b6 100644 --- a/docs/V3_MIGRATION.md +++ b/docs/V3_MIGRATION.md @@ -49,7 +49,12 @@ headless; the builtin window opens only when an update is found. Only the binary self-swaps; icons/`.desktop` still come from the tarball's `install_linux.sh`. Signing: public half `updater.pub` is embedded; the private key lives in the password manager + the `UPDATER_SIGNING_KEY` CI -secret, never in the repo (see `.gitignore`). +secret, never in the repo (see `.gitignore`). Startup also guards the swap +itself: it proves `os.TempDir()` can be renamed into the executable directory +before offering an update, because the 2.0.0 helper showed twenty EXDEV +`invalid cross-device link` attempts on tmpfs `/tmp` versus persistent `$HOME`, +then orphaned `.bak` with nothing running. A failed probe disables updates +with a log line instead of risking the installed binary. ## Future plans on v3 diff --git a/updater.go b/updater.go index c11c11b..e5d95d4 100644 --- a/updater.go +++ b/updater.go @@ -3,8 +3,10 @@ package main import ( "context" _ "embed" + "fmt" "log" "os" + "path/filepath" "github.com/wailsapp/wails/v3/pkg/application" "github.com/wailsapp/wails/v3/pkg/updater" @@ -15,6 +17,48 @@ import ( //go:embed updater.pub var updaterPublicKey []byte +// swapPathProven verifies the exact move the update helper will perform: +// a rename from the framework staging area (os.TempDir, where it creates +// wails-update-*) onto the running executable's directory. A bare Rename +// fails across filesystems (EXDEV: tmpfs /tmp vs persistent $HOME is the +// Arch/Fedora default), and the helper's recovery path then deletes the +// working binary while failing to restore it — stranding the user with no +// app. If this probe fails we skip updates entirely: a loud skip beats a +// destructive attempt. Observed in the wild on 2.0.0 (helper log: twenty +// "invalid cross-device link" attempts, orphaned .bak, nothing running). +func swapPathProven() error { + exe, err := os.Executable() + if err != nil { + return fmt.Errorf("cannot locate executable: %w", err) + } + return renameProbeOK(os.TempDir(), filepath.Dir(exe)) +} + +// renameProbeOK proves a file can be renamed from tmpDir into targetDir and +// cleans up after itself. It exercises the same call (os.Rename) across the +// same two directories the helper will use, so any EXDEV or permission +// failure surfaces here, harmlessly, instead of mid-swap. +func renameProbeOK(tmpDir, targetDir string) error { + tmp, err := os.CreateTemp(tmpDir, "anitrack-update-probe-*") + if err != nil { + return fmt.Errorf("cannot stage update files: %w", err) + } + tmpName := tmp.Name() + if err := tmp.Close(); err != nil { + os.Remove(tmpName) + return fmt.Errorf("cannot stage update files: %w", err) + } + probe := filepath.Join(targetDir, ".anitrack-update-probe") + if err := os.Rename(tmpName, probe); err != nil { + os.Remove(tmpName) + return fmt.Errorf("cannot move update into %s: %w", targetDir, err) + } + if err := os.Remove(probe); err != nil { + return fmt.Errorf("cannot clean up probe in %s: %w", targetDir, err) + } + return nil +} + // maybeEnableUpdater wires self-updates: desktop production builds only. // Mobile stays on Obtainium; dev builds never phone home. The startup check // is headless — the builtin window opens only when an update is found, via a @@ -26,6 +70,14 @@ func maybeEnableUpdater(app *application.App) { if !application.System.IsDesktop() { return } + if appVersion() == "" { + log.Println("updater: disabled: empty version") + return + } + if err := swapPathProven(); err != nil { + log.Printf("updater: disabled: %s", err) + return + } gh, err := giteaprovider.New(giteaprovider.Config{ BaseURL: "https://git.linuxhg.com", Owner: "john-okeefe", diff --git a/updater_probe_test.go b/updater_probe_test.go new file mode 100644 index 0000000..d8db185 --- /dev/null +++ b/updater_probe_test.go @@ -0,0 +1,57 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func TestRenameProbeOKSameFilesystem(t *testing.T) { + tmpDir := t.TempDir() + targetDir := t.TempDir() + if err := renameProbeOK(tmpDir, targetDir); err != nil { + t.Fatalf("same-filesystem probe: %s", err) + } + // No litter: probe file removed, nothing else created. + entries, err := os.ReadDir(targetDir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Errorf("probe left %d entries in target dir", len(entries)) + } + entries, err = os.ReadDir(tmpDir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Errorf("probe left %d entries in tmp dir", len(entries)) + } +} + +func TestRenameProbeFailsClosed(t *testing.T) { + tmpDir := t.TempDir() + + t.Run("missing tmp dir", func(t *testing.T) { + if err := renameProbeOK(filepath.Join(tmpDir, "nope"), tmpDir); err == nil { + t.Error("expected error for missing tmp dir, got nil") + } + }) + + t.Run("unwritable target dir", func(t *testing.T) { + targetDir := t.TempDir() + if err := os.Chmod(targetDir, 0o500); err != nil { + t.Fatal(err) + } + defer os.Chmod(targetDir, 0o700) + if err := renameProbeOK(tmpDir, targetDir); err == nil { + t.Error("expected error for unwritable target dir, got nil") + } + }) + + t.Run("missing target dir", func(t *testing.T) { + if err := renameProbeOK(tmpDir, filepath.Join(tmpDir, "nope")); err == nil { + t.Error("expected error for missing target dir, got nil") + } + }) +}