From 2ccd72180dd81e3af4e0de21c3d3c51169e13e0a Mon Sep 17 00:00:00 2001 From: Marx Date: Wed, 8 Apr 2026 20:03:44 +0800 Subject: [PATCH 1/2] updater: avoid in-place self-overwrite on darwin/linux --- component/updater/update_core.go | 102 +++++++++++++++++++++++--- component/updater/update_core_test.go | 88 ++++++++++++++++++++++ 2 files changed, 179 insertions(+), 11 deletions(-) diff --git a/component/updater/update_core.go b/component/updater/update_core.go index 83a303af5c..ec5737bd69 100644 --- a/component/updater/update_core.go +++ b/component/updater/update_core.go @@ -147,6 +147,11 @@ func (u *CoreUpdater) Update(currentExePath string, channel string, force bool) defer u.clean(updateDir) + err = u.prepareUpdateDir(updateDir) + if err != nil { + return fmt.Errorf("preparing update dir: %w", err) + } + err = u.download(updateDir, packagePath, packageURL) if err != nil { return fmt.Errorf("downloading: %w", err) @@ -162,7 +167,11 @@ func (u *CoreUpdater) Update(currentExePath string, channel string, force bool) return fmt.Errorf("backuping: %w", err) } - err = u.copyFile(updateExePath, currentExePath) + if runtime.GOOS == "windows" { + err = u.copyFile(updateExePath, currentExePath) + } else { + err = u.replaceFileAtomically(updateExePath, currentExePath) + } if err != nil { return fmt.Errorf("replacing: %w", err) } @@ -192,6 +201,18 @@ func (u *CoreUpdater) getLatestVersion(versionURL string) (version string, err e return content, nil } +func (u *CoreUpdater) prepareUpdateDir(updateDir string) error { + if err := os.RemoveAll(updateDir); err != nil { + return fmt.Errorf("os.RemoveAll(%s): %w", updateDir, err) + } + + if err := os.MkdirAll(updateDir, 0o755); err != nil { + return fmt.Errorf("os.MkdirAll(%s): %w", updateDir, err) + } + + return nil +} + // download package file and save it to disk func (u *CoreUpdater) download(updateDir, packagePath, packageURL string) (err error) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*90) @@ -208,12 +229,6 @@ func (u *CoreUpdater) download(updateDir, packagePath, packageURL string) (err e } }() - log.Debugln("updateDir %s", updateDir) - err = os.Mkdir(updateDir, 0o755) - if err != nil { - return fmt.Errorf("mkdir error: %w", err) - } - log.Debugln("updater: saving package to file %s", packagePath) // Create the output file wc, err := os.OpenFile(packagePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o755) @@ -462,13 +477,78 @@ func (u *CoreUpdater) copyFile(src, dst string) (err error) { return fmt.Errorf("io.Copy(): %w", err) } - if runtime.GOOS == "darwin" { - err = exec.Command("/usr/bin/codesign", "--sign", "-", dst).Run() + log.Infoln("updater: copy: %s to %s", src, dst) + return nil +} + +func (u *CoreUpdater) replaceFileAtomically(src, dst string) (err error) { + rc, err := os.Open(src) + if err != nil { + return fmt.Errorf("os.Open(%s): %w", src, err) + } + + defer func() { + closeErr := rc.Close() + if closeErr != nil && err == nil { + err = closeErr + } + }() + + info, err := rc.Stat() + if err != nil { + return fmt.Errorf("rc.Stat(): %w", err) + } + + dir := filepath.Dir(dst) + tmp, err := os.CreateTemp(dir, "."+filepath.Base(dst)+".tmp-*") + if err != nil { + return fmt.Errorf("os.CreateTemp(%s): %w", dir, err) + } + + tmpPath := tmp.Name() + defer func() { if err != nil { - log.Warnln("codesign failed: %v", err) + _ = os.Remove(tmpPath) + } + }() + + defer func() { + if tmp != nil { + closeErr := tmp.Close() + if closeErr != nil && err == nil { + err = closeErr + } } + }() + + if err = tmp.Chmod(info.Mode()); err != nil { + return fmt.Errorf("tmp.Chmod(%s): %w", tmpPath, err) } - log.Infoln("updater: copy: %s to %s", src, dst) + if _, err = io.Copy(tmp, rc); err != nil { + return fmt.Errorf("io.Copy(): %w", err) + } + + if err = tmp.Sync(); err != nil { + return fmt.Errorf("tmp.Sync(): %w", err) + } + + if err = tmp.Close(); err != nil { + return fmt.Errorf("tmp.Close(): %w", err) + } + tmp = nil + + if runtime.GOOS == "darwin" { + signErr := exec.Command("/usr/bin/codesign", "--sign", "-", tmpPath).Run() + if signErr != nil { + log.Warnln("codesign failed: %v", signErr) + } + } + + if err = os.Rename(tmpPath, dst); err != nil { + return fmt.Errorf("os.Rename(%s, %s): %w", tmpPath, dst, err) + } + + log.Infoln("updater: replace: %s to %s via %s", src, dst, tmpPath) return nil } diff --git a/component/updater/update_core_test.go b/component/updater/update_core_test.go index eb7c4a5565..d829f75e46 100644 --- a/component/updater/update_core_test.go +++ b/component/updater/update_core_test.go @@ -2,9 +2,97 @@ package updater import ( "fmt" + "os" + "path/filepath" + "runtime" "testing" ) func TestCoreBaseName(t *testing.T) { fmt.Println("Core base name =", DefaultCoreUpdater.CoreBaseName()) } + +func TestPrepareUpdateDirRemovesStaleContents(t *testing.T) { + root := t.TempDir() + updateDir := filepath.Join(root, "meta-update") + + if err := os.MkdirAll(updateDir, 0o755); err != nil { + t.Fatalf("mkdir update dir: %v", err) + } + + staleFile := filepath.Join(updateDir, "stale.txt") + if err := os.WriteFile(staleFile, []byte("stale"), 0o644); err != nil { + t.Fatalf("write stale file: %v", err) + } + + if err := DefaultCoreUpdater.prepareUpdateDir(updateDir); err != nil { + t.Fatalf("prepare update dir: %v", err) + } + + entries, err := os.ReadDir(updateDir) + if err != nil { + t.Fatalf("read update dir: %v", err) + } + + if len(entries) != 0 { + t.Fatalf("expected clean update dir, got %d entries", len(entries)) + } +} + +func TestReplaceFileAtomicallyReplacesDestination(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("replaceFileAtomically is used on non-Windows platforms") + } + + root := t.TempDir() + src := filepath.Join(root, "src") + dst := filepath.Join(root, "dst") + + if err := os.WriteFile(src, []byte("new-core"), 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + + if err := os.WriteFile(dst, []byte("old-core"), 0o755); err != nil { + t.Fatalf("write dst: %v", err) + } + + if err := DefaultCoreUpdater.replaceFileAtomically(src, dst); err != nil { + t.Fatalf("replace file atomically: %v", err) + } + + got, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("read dst: %v", err) + } + + if string(got) != "new-core" { + t.Fatalf("unexpected dst content: %q", string(got)) + } +} + +func TestReplaceFileAtomicallyKeepsDestinationOnSourceError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("replaceFileAtomically is used on non-Windows platforms") + } + + root := t.TempDir() + dst := filepath.Join(root, "dst") + + if err := os.WriteFile(dst, []byte("old-core"), 0o755); err != nil { + t.Fatalf("write dst: %v", err) + } + + err := DefaultCoreUpdater.replaceFileAtomically(filepath.Join(root, "missing-src"), dst) + if err == nil { + t.Fatal("expected replaceFileAtomically to fail for missing src") + } + + got, readErr := os.ReadFile(dst) + if readErr != nil { + t.Fatalf("read dst: %v", readErr) + } + + if string(got) != "old-core" { + t.Fatalf("destination was modified on failure: %q", string(got)) + } +} From 5fa5a757c743537859c0948afb95b4e5b80b8430 Mon Sep 17 00:00:00 2001 From: Marx Date: Fri, 17 Apr 2026 23:29:37 +0800 Subject: [PATCH 2/2] fix updater: preserve metadata when renaming with best effort --- component/updater/update_core.go | 32 ++++++- component/updater/update_core_darwin.go | 29 ++++++ component/updater/update_core_darwin_test.go | 47 ++++++++++ component/updater/update_core_test.go | 80 ++++++++++++++++ component/updater/update_core_unix.go | 96 ++++++++++++++++++++ component/updater/update_core_unix_test.go | 80 ++++++++++++++++ 6 files changed, 362 insertions(+), 2 deletions(-) create mode 100644 component/updater/update_core_darwin.go create mode 100644 component/updater/update_core_darwin_test.go create mode 100644 component/updater/update_core_unix.go create mode 100644 component/updater/update_core_unix_test.go diff --git a/component/updater/update_core.go b/component/updater/update_core.go index ec5737bd69..6562aaf1e8 100644 --- a/component/updater/update_core.go +++ b/component/updater/update_core.go @@ -49,6 +49,11 @@ type CoreUpdater struct { var DefaultCoreUpdater = CoreUpdater{} +// preservePosixAttrs is set via init() on linux/darwin to copy owner, mode, +// xattrs and (on darwin) file flags from the existing binary onto the staged +// replacement. Nil on other platforms. +var preservePosixAttrs func(src, dst string) + func (u *CoreUpdater) CoreBaseName() string { switch runtime.GOARCH { case "arm": @@ -482,6 +487,13 @@ func (u *CoreUpdater) copyFile(src, dst string) (err error) { } func (u *CoreUpdater) replaceFileAtomically(src, dst string) (err error) { + // Resolve symlinks so deployments that expose a stable path (e.g. + // /usr/local/bin/mihomo -> /opt/mihomo//mihomo) replace the real + // file rather than turning the symlink into a regular file. + if resolved, linkErr := filepath.EvalSymlinks(dst); linkErr == nil { + dst = resolved + } + rc, err := os.Open(src) if err != nil { return fmt.Errorf("os.Open(%s): %w", src, err) @@ -494,11 +506,21 @@ func (u *CoreUpdater) replaceFileAtomically(src, dst string) (err error) { } }() - info, err := rc.Stat() + srcInfo, err := rc.Stat() if err != nil { return fmt.Errorf("rc.Stat(): %w", err) } + // Prefer the existing dst's mode so local chmod survives updates. Fall + // back to src's mode on first install. + mode := srcInfo.Mode() + dstExists := true + if dstInfo, statErr := os.Stat(dst); statErr == nil { + mode = dstInfo.Mode() + } else if os.IsNotExist(statErr) { + dstExists = false + } + dir := filepath.Dir(dst) tmp, err := os.CreateTemp(dir, "."+filepath.Base(dst)+".tmp-*") if err != nil { @@ -521,7 +543,7 @@ func (u *CoreUpdater) replaceFileAtomically(src, dst string) (err error) { } }() - if err = tmp.Chmod(info.Mode()); err != nil { + if err = tmp.Chmod(mode); err != nil { return fmt.Errorf("tmp.Chmod(%s): %w", tmpPath, err) } @@ -538,6 +560,12 @@ func (u *CoreUpdater) replaceFileAtomically(src, dst string) (err error) { } tmp = nil + // Carry over ownership, xattrs (notably Linux security.capability for + // cap_net_admin / cap_net_bind_service), setuid/sgid and darwin flags. + if dstExists && preservePosixAttrs != nil { + preservePosixAttrs(dst, tmpPath) + } + if runtime.GOOS == "darwin" { signErr := exec.Command("/usr/bin/codesign", "--sign", "-", tmpPath).Run() if signErr != nil { diff --git a/component/updater/update_core_darwin.go b/component/updater/update_core_darwin.go new file mode 100644 index 0000000000..f8859462b9 --- /dev/null +++ b/component/updater/update_core_darwin.go @@ -0,0 +1,29 @@ +//go:build darwin + +package updater + +import ( + "syscall" + + "github.com/metacubex/mihomo/log" + + "golang.org/x/sys/unix" +) + +func init() { + flagsHook = preserveDarwinFlags +} + +// preserveDarwinFlags carries over macOS st_flags (uchg/schg/hidden). +func preserveDarwinFlags(src, dst string) { + var st syscall.Stat_t + if err := syscall.Stat(src, &st); err != nil { + return + } + if st.Flags == 0 { + return + } + if err := unix.Chflags(dst, int(st.Flags)); err != nil { + log.Warnln("updater: chflags %s: %v", dst, err) + } +} diff --git a/component/updater/update_core_darwin_test.go b/component/updater/update_core_darwin_test.go new file mode 100644 index 0000000000..b343298d29 --- /dev/null +++ b/component/updater/update_core_darwin_test.go @@ -0,0 +1,47 @@ +//go:build darwin + +package updater + +import ( + "errors" + "os" + "path/filepath" + "syscall" + "testing" + + "golang.org/x/sys/unix" +) + +func TestReplaceFileAtomicallyPreservesDarwinFlags(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "src") + dst := filepath.Join(root, "dst") + + if err := os.WriteFile(src, []byte("new-core"), 0o700); err != nil { + t.Fatalf("write src: %v", err) + } + + if err := os.WriteFile(dst, []byte("old-core"), 0o755); err != nil { + t.Fatalf("write dst: %v", err) + } + + if err := unix.Chflags(dst, unix.UF_HIDDEN); err != nil { + if errors.Is(err, syscall.EPERM) { + t.Skipf("cannot set darwin file flags in test environment: %v", err) + } + t.Fatalf("set darwin flags on dst: %v", err) + } + + if err := DefaultCoreUpdater.replaceFileAtomically(src, dst); err != nil { + t.Fatalf("replace file atomically: %v", err) + } + + var st syscall.Stat_t + if err := syscall.Stat(dst, &st); err != nil { + t.Fatalf("stat dst: %v", err) + } + + if st.Flags&unix.UF_HIDDEN == 0 { + t.Fatalf("expected UF_HIDDEN to be preserved, flags=%#x", st.Flags) + } +} diff --git a/component/updater/update_core_test.go b/component/updater/update_core_test.go index d829f75e46..51b636f854 100644 --- a/component/updater/update_core_test.go +++ b/component/updater/update_core_test.go @@ -70,6 +70,86 @@ func TestReplaceFileAtomicallyReplacesDestination(t *testing.T) { } } +func TestReplaceFileAtomicallyPreservesDestinationMode(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("replaceFileAtomically is used on non-Windows platforms") + } + + root := t.TempDir() + src := filepath.Join(root, "src") + dst := filepath.Join(root, "dst") + + if err := os.WriteFile(src, []byte("new-core"), 0o700); err != nil { + t.Fatalf("write src: %v", err) + } + + if err := os.WriteFile(dst, []byte("old-core"), 0o755); err != nil { + t.Fatalf("write dst: %v", err) + } + + if err := os.Chmod(dst, 0o751); err != nil { + t.Fatalf("chmod dst: %v", err) + } + + if err := DefaultCoreUpdater.replaceFileAtomically(src, dst); err != nil { + t.Fatalf("replace file atomically: %v", err) + } + + info, err := os.Stat(dst) + if err != nil { + t.Fatalf("stat dst: %v", err) + } + + if got, want := info.Mode().Perm(), os.FileMode(0o751); got != want { + t.Fatalf("unexpected dst mode: got %o want %o", got, want) + } +} + +func TestReplaceFileAtomicallyReplacesSymlinkTarget(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("replaceFileAtomically is used on non-Windows platforms") + } + + root := t.TempDir() + src := filepath.Join(root, "src") + realDst := filepath.Join(root, "real-dst") + linkDst := filepath.Join(root, "current") + + if err := os.WriteFile(src, []byte("new-core"), 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + + if err := os.WriteFile(realDst, []byte("old-core"), 0o755); err != nil { + t.Fatalf("write real dst: %v", err) + } + + if err := os.Symlink(realDst, linkDst); err != nil { + t.Fatalf("create symlink: %v", err) + } + + if err := DefaultCoreUpdater.replaceFileAtomically(src, linkDst); err != nil { + t.Fatalf("replace file atomically via symlink: %v", err) + } + + linkInfo, err := os.Lstat(linkDst) + if err != nil { + t.Fatalf("lstat symlink dst: %v", err) + } + + if linkInfo.Mode()&os.ModeSymlink == 0 { + t.Fatalf("expected %s to remain a symlink", linkDst) + } + + got, err := os.ReadFile(realDst) + if err != nil { + t.Fatalf("read real dst: %v", err) + } + + if string(got) != "new-core" { + t.Fatalf("unexpected real dst content: %q", string(got)) + } +} + func TestReplaceFileAtomicallyKeepsDestinationOnSourceError(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("replaceFileAtomically is used on non-Windows platforms") diff --git a/component/updater/update_core_unix.go b/component/updater/update_core_unix.go new file mode 100644 index 0000000000..4dc1f7f963 --- /dev/null +++ b/component/updater/update_core_unix.go @@ -0,0 +1,96 @@ +//go:build linux || darwin + +package updater + +import ( + "os" + "syscall" + + "github.com/metacubex/mihomo/log" + + "golang.org/x/sys/unix" +) + +func init() { + preservePosixAttrs = preservePosixAttrsUnix +} + +// flagsHook is set on darwin to carry over st_flags (chflags). +var flagsHook func(src, dst string) + +// preservePosixAttrsUnix copies owner, mode, file flags and xattrs from src to +// dst. Best-effort: errors are logged, not returned. Covers Linux file +// capabilities (security.capability), SELinux labels, POSIX ACLs, setuid/sgid, +// uid/gid, and macOS com.apple.* xattrs. chown(2) on Linux strips setuid/sgid +// when non-root, so chmod is reapplied after. +func preservePosixAttrsUnix(src, dst string) { + info, err := os.Stat(src) + if err != nil { + log.Warnln("updater: stat %s: %v", src, err) + return + } + + if sys, ok := info.Sys().(*syscall.Stat_t); ok { + if chownErr := os.Chown(dst, int(sys.Uid), int(sys.Gid)); chownErr != nil && !os.IsPermission(chownErr) { + log.Warnln("updater: chown %s: %v", dst, chownErr) + } + } + + if chmodErr := os.Chmod(dst, info.Mode()); chmodErr != nil { + log.Warnln("updater: chmod %s: %v", dst, chmodErr) + } + + if flagsHook != nil { + flagsHook(src, dst) + } + + copyXattrs(src, dst) +} + +func copyXattrs(src, dst string) { + size, err := unix.Llistxattr(src, nil) + if err != nil { + if err != unix.ENOTSUP { + log.Warnln("updater: llistxattr %s: %v", src, err) + } + return + } + if size == 0 { + return + } + + buf := make([]byte, size) + size, err = unix.Llistxattr(src, buf) + if err != nil { + log.Warnln("updater: llistxattr %s: %v", src, err) + return + } + + for _, name := range splitXattrNames(buf[:size]) { + valSize, err := unix.Lgetxattr(src, name, nil) + if err != nil { + continue + } + val := make([]byte, valSize) + if _, err = unix.Lgetxattr(src, name, val); err != nil { + continue + } + if err = unix.Lsetxattr(dst, name, val, 0); err != nil { + log.Warnln("updater: lsetxattr %s %s: %v", dst, name, err) + } + } +} + +func splitXattrNames(b []byte) []string { + var names []string + start := 0 + for i, c := range b { + if c == 0 { + if i > start { + names = append(names, string(b[start:i])) + } + start = i + 1 + } + } + return names +} diff --git a/component/updater/update_core_unix_test.go b/component/updater/update_core_unix_test.go new file mode 100644 index 0000000000..04f809923b --- /dev/null +++ b/component/updater/update_core_unix_test.go @@ -0,0 +1,80 @@ +//go:build linux || darwin + +package updater + +import ( + "bytes" + "errors" + "os" + "path/filepath" + "runtime" + "syscall" + "testing" + + "golang.org/x/sys/unix" +) + +func TestReplaceFileAtomicallyPreservesDestinationXattrs(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "src") + dst := filepath.Join(root, "dst") + name := testXattrName() + want := []byte("preserved") + + if err := os.WriteFile(src, []byte("new-core"), 0o700); err != nil { + t.Fatalf("write src: %v", err) + } + + if err := os.WriteFile(dst, []byte("old-core"), 0o755); err != nil { + t.Fatalf("write dst: %v", err) + } + + if err := unix.Lsetxattr(dst, name, want, 0); err != nil { + if isUnsupportedXattrErr(err) { + t.Skipf("xattrs not supported in test environment: %v", err) + } + t.Fatalf("set xattr on dst: %v", err) + } + + if err := DefaultCoreUpdater.replaceFileAtomically(src, dst); err != nil { + t.Fatalf("replace file atomically: %v", err) + } + + got, err := readXattr(dst, name) + if err != nil { + t.Fatalf("read xattr from dst: %v", err) + } + + if !bytes.Equal(got, want) { + t.Fatalf("unexpected xattr value: got %q want %q", got, want) + } +} + +func readXattr(path, name string) ([]byte, error) { + size, err := unix.Lgetxattr(path, name, nil) + if err != nil { + return nil, err + } + + value := make([]byte, size) + n, err := unix.Lgetxattr(path, name, value) + if err != nil { + return nil, err + } + + return value[:n], nil +} + +func testXattrName() string { + if runtime.GOOS == "darwin" { + return "com.mihomo.test" + } + + return "user.mihomo.test" +} + +func isUnsupportedXattrErr(err error) bool { + return errors.Is(err, unix.ENOTSUP) || + errors.Is(err, unix.EOPNOTSUPP) || + errors.Is(err, syscall.EPERM) +}