Refactor uninstall on Windows

internal/pkg/agent/install/uninstall.go

@@ -282,18 +282,19 @@ func checkForUnprivilegedVault(ctx context.Context, opts ...vault.OptionFunc) (b
 // seconds if it keeps getting that error.
 func RemovePath(path string) error {
 	const arbitraryTimeout = 60 * time.Second
+	tmpDir := tempDirOnSameVolume(path)
 	start := time.Now()
 	var lastErr error
 	for time.Since(start) <= arbitraryTimeout {
-		lastErr = removeAll(path)
+		lastErr = os.RemoveAll(path)
 
 		if lastErr == nil || !isRetryableError(lastErr) {
 			return lastErr
 		}
 
 		if isBlockingOnExe(lastErr) {
-			// try to remove the blocking exe and try again to clean up the path
-			_ = removeBlockingExe(lastErr)
+			// try to move the blocking exe out of the way and try again
+			_ = removeBlockingExe(lastErr, tmpDir)
 		}
 
 		time.Sleep(500 * time.Millisecond)
@@ -326,70 +327,6 @@ func RemoveBut(path string, bestEffort bool, exceptions ...string) error {
 	return err
 }
 
-// TODO: Replace this with a more robust and less mysterious approach.
-// removeAll is a reimplementation of Go 1.24's os.RemoveAll. Go 1.25 switched
-// to directory-relative unlinkat/openat syscalls (removeall_at.go) which on
-// Windows use NtCreateFile with DELETE access — these are stricter about file
-// state and fail on files that have been ADS-renamed. The simple path-based
-// approach using os.Remove works correctly with the ADS rename trick that
-// RemovePath uses to delete running executables on Windows.
-// Taken from: https://cs.opensource.google/go/go/+/refs/tags/go1.24.13:src/os/removeall_noat.go;drc=a2baae6851a157d662dff7cc508659f66249698a;l=15
-// The implementation which breaks our install is here:
-// https://cs.opensource.google/go/go/+/refs/tags/go1.25.8:src/os/removeall_at.go;drc=e81c624656e415626c7ac3a97768f5c2717979a4;l=15
-func removeAll(path string) error {
-	// Try simple remove first (handles files and empty directories).
-	err := os.Remove(path)
-	if err == nil || errors.Is(err, fs.ErrNotExist) {
-		return nil
-	}
-
-	// Check if it's a directory.
-	info, serr := os.Lstat(path)
-	if serr != nil {
-		if errors.Is(serr, fs.ErrNotExist) {
-			return nil
-		}
-		return serr
-	}
-	if !info.IsDir() {
-		// Not a directory — return the original Remove error.
-		return err
-	}
-
-	// Remove directory contents recursively.
-	err = removeAllChildren(path)
-	if err != nil {
-		return err
-	}
-
-	// Remove the now-empty directory.
-	err = os.Remove(path)
-	if errors.Is(err, fs.ErrNotExist) {
-		return nil
-	}
-	return err
-}
-
-func removeAllChildren(path string) error {
-	entries, err := os.ReadDir(path)
-	if err != nil {
-		if errors.Is(err, fs.ErrNotExist) {
-			return nil
-		}
-		return err
-	}
-
-	var firstErr error
-	for _, entry := range entries {
-		child := filepath.Join(path, entry.Name())
-		err := removeAll(child)
-		if err != nil && firstErr == nil {
-			firstErr = err
-		}
-	}
-	return firstErr
-}
-
 func containsString(str string, a []string, caseSensitive bool) bool {
 	if !caseSensitive {
 		str = strings.ToLower(str)

internal/pkg/agent/install/uninstall_unix.go

@@ -6,20 +6,27 @@
 
 package install
 
-import "os"
+import (
+	"os"
+	"path/filepath"
+)
 
 func isBlockingOnExe(_ error) bool {
 	return false
 }
 
-func removeBlockingExe(_ error) error {
+func removeBlockingExe(_ error, _ string) error {
 	return nil
 }
 
 func isRetryableError(_ error) bool {
 	return false
 }
 
+func tempDirOnSameVolume(path string) string {
+	return filepath.Dir(path)
+}
+
 // killNoneChildProcess provides a way of killing a process that is not started as a child of this process.
 //
 // On Unix systems it just calls the native golang kill.

internal/pkg/agent/install/uninstall_windows.go

@@ -11,8 +11,9 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
+	"path/filepath"
+	"strings"
 	"syscall"
-	"unsafe"
 
 	"golang.org/x/sys/windows"
 )
@@ -39,40 +40,61 @@ func isRetryableError(err error) bool {
 	return errno == syscall.ERROR_ACCESS_DENIED || errno == windows.ERROR_SHARING_VIOLATION
 }
 
-func removeBlockingExe(blockingErr error) error {
+// removeBlockingExe moves a locked executable out of the way so that
+// os.RemoveAll can remove the directory it lived in. On Windows a running exe
+// cannot be deleted, but it *can* be renamed/moved (even across directories,
+// as long as it stays on the same volume). After the move we schedule the temp
+// file for deletion on the next reboot via MoveFileEx/MOVEFILE_DELAY_UNTIL_REBOOT.
+//
+// tempDir is the directory where the temp file will be created. It must be on
+// the same volume as the blocked file (cross-volume rename won't work for
+// in-use files).
+func removeBlockingExe(blockingErr error, tempDir string) error {
 	path, _ := getPathFromError(blockingErr)
 	if path == "" {
 		return nil
 	}
 
-	// open handle for delete only
-	h, err := openDeleteHandle(path)
+	tmp, err := os.CreateTemp(tempDir, ".elastic-agent-rm-*.exe")
 	if err != nil {
-		return fmt.Errorf("failed to open handle for %q: %w", path, err)
+		return fmt.Errorf("failed to create temp file in %q: %w", tempDir, err)
 	}
+	tmpPath := tmp.Name()
+	_ = tmp.Close()
 
-	// rename handle
-	err = renameHandle(h)
-	_ = windows.CloseHandle(h)
-	if err != nil {
-		return fmt.Errorf("failed to rename handle for %q: %w", path, err)
+	// os.Rename uses MoveFileEx(MOVEFILE_REPLACE_EXISTING) under the hood.
+	if err := os.Rename(path, tmpPath); err != nil {
+		_ = os.Remove(tmpPath)
+		return fmt.Errorf("failed to rename %q to %q: %w", path, tmpPath, err)
 	}
 
-	// re-open handle
-	h, err = openDeleteHandle(path)
+	// Schedule the temp file for deletion on next reboot.
+	tmpPathPtr, err := windows.UTF16PtrFromString(tmpPath)
 	if err != nil {
-		return fmt.Errorf("failed to open handle after rename for %q: %w", path, err)
+		return fmt.Errorf("failed to convert temp path: %w", err)
 	}
-
-	// dispose of the handle
-	err = disposeHandle(h)
-	_ = windows.CloseHandle(h)
-	if err != nil {
-		return fmt.Errorf("failed to dispose handle for %q: %w", path, err)
+	if err := windows.MoveFileEx(tmpPathPtr, nil, windows.MOVEFILE_DELAY_UNTIL_REBOOT); err != nil {
+		// Non-fatal: the file is already out of the way, it just won't be
+		// auto-cleaned. Log-worthy but not worth failing the uninstall.
+		return fmt.Errorf("failed to schedule %q for deletion on reboot: %w", tmpPath, err)
 	}
+
 	return nil
 }
 
+// tempDirOnSameVolume returns os.TempDir() if it is on the same volume as
+// path, otherwise falls back to the parent of path. Rename/move of an in-use
+// file only works within the same volume.
+func tempDirOnSameVolume(path string) string {
+	sysTemp := os.TempDir()
+	pathVol := filepath.VolumeName(path)
+	tempVol := filepath.VolumeName(sysTemp)
+	if strings.EqualFold(pathVol, tempVol) {
+		return sysTemp
+	}
+	return filepath.Dir(path)
+}
+
 func getPathFromError(blockingErr error) (string, syscall.Errno) {
 	var perr *fs.PathError
 	if errors.As(blockingErr, &perr) {
@@ -84,86 +106,6 @@ func getPathFromError(blockingErr error) (string, syscall.Errno) {
 	return "", 0
 }
 
-func openDeleteHandle(path string) (windows.Handle, error) {
-	wPath, err := windows.UTF16PtrFromString(path)
-	if err != nil {
-		return 0, err
-	}
-	handle, err := windows.CreateFile(
-		wPath,
-		windows.DELETE,
-		0,
-		nil,
-		windows.OPEN_EXISTING,
-		windows.FILE_ATTRIBUTE_NORMAL,
-		0,
-	)
-	if err != nil {
-		return 0, err
-	}
-	return handle, nil
-}
-
-func renameHandle(hHandle windows.Handle) error {
-	wRename, err := windows.UTF16FromString(":agentrm")
-	if err != nil {
-		return err
-	}
-
-	var rename fileRenameInfo
-	lpwStream := &wRename[0]
-	rename.FileNameLength = uint32(unsafe.Sizeof(lpwStream))
-
-	// RtlCopyMemory (https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlcopymemory)
-	//   Return value - None
-	_, _, _ = windows.NewLazyDLL("kernel32.dll").NewProc("RtlCopyMemory").Call(
-		uintptr(unsafe.Pointer(&rename.FileName[0])),
-		uintptr(unsafe.Pointer(lpwStream)),
-		unsafe.Sizeof(lpwStream),
-	)
-
-	err = windows.SetFileInformationByHandle(
-		hHandle,
-		windows.FileRenameInfo,
-		(*byte)(unsafe.Pointer(&rename)),
-		uint32(unsafe.Sizeof(rename)+unsafe.Sizeof(lpwStream)),
-	)
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
-func disposeHandle(hHandle windows.Handle) error {
-	var deleteFile fileDispositionInfo
-	deleteFile.DeleteFile = true
-
-	err := windows.SetFileInformationByHandle(
-		hHandle,
-		windows.FileDispositionInfo,
-		(*byte)(unsafe.Pointer(&deleteFile)),
-		uint32(unsafe.Sizeof(deleteFile)),
-	)
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
-type fileRenameInfo struct {
-	Union struct {
-		ReplaceIfExists bool
-		Flags           uint32
-	}
-	RootDirectory  windows.Handle
-	FileNameLength uint32
-	FileName       [1]uint16
-}
-
-type fileDispositionInfo struct {
-	DeleteFile bool
-}
-
 // killNoneChildProcess provides a way of killing a process that is not started as a child of this process.
 //
 // On Windows when running in unprivileged mode the internal way that golang uses DuplicateHandle to perform the kill