From 513b4aacebe3e4b3de382824709feccb05e17197 Mon Sep 17 00:00:00 2001 From: Mario Loriedo Date: Tue, 28 Jan 2025 19:09:06 +0100 Subject: [PATCH] Safer use of `filepath.EvalSymlinks()` on Windows The behavior of function `path/filepath.EvalSymlinks()` has changed in Go v1.23: - https://go-review.googlesource.com/c/go/+/565136 - https://go.dev/doc/go1.23#minor_library_changes - https://tip.golang.org/doc/godebug As a consequences, starting with Podman 5.3.0, when installing on Windows (WSL) using scoop, Podman fails to start because it fails to find helper binaries. Scoop copies Podman binaries in a folder of type Junction and `EvalSymlinks` returns an error. The problem is described in #24557. To address this problem we are checking if a path is a `Symlink` before calling `EvalSymlinks` and, if it's not (hardlinks, mount points or canonical files), we are calling `path/filepath.Clean` for consistency. In fact `path/filepath.EvalSymlinks`, after evaluating a symlink target, calls `Clean` too. Signed-off-by: Mario Loriedo --- pkg/machine/machine_windows.go | 24 +++++- pkg/machine/machine_windows_test.go | 116 ++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 pkg/machine/machine_windows_test.go diff --git a/pkg/machine/machine_windows.go b/pkg/machine/machine_windows.go index 174980fc29..dd59876ad3 100644 --- a/pkg/machine/machine_windows.go +++ b/pkg/machine/machine_windows.go @@ -251,7 +251,7 @@ func FindExecutablePeer(name string) (string, error) { return "", err } - exe, err = filepath.EvalSymlinks(exe) + exe, err = EvalSymlinksOrClean(exe) if err != nil { return "", err } @@ -259,6 +259,28 @@ func FindExecutablePeer(name string) (string, error) { return filepath.Join(filepath.Dir(exe), name), nil } +func EvalSymlinksOrClean(filePath string) (string, error) { + fileInfo, err := os.Lstat(filePath) + if err != nil { + return "", err + } + if fileInfo.Mode()&fs.ModeSymlink != 0 { + // Only call filepath.EvalSymlinks if it is a symlink. + // Starting with v1.23, EvalSymlinks returns an error for mount points. + // See https://go-review.googlesource.com/c/go/+/565136 for reference. + filePath, err = filepath.EvalSymlinks(filePath) + if err != nil { + return "", err + } + } else { + // Call filepath.Clean when filePath is not a symlink. That's for + // consistency with the symlink case (filepath.EvalSymlinks calls + // Clean after evaluating filePath). + filePath = filepath.Clean(filePath) + } + return filePath, nil +} + func GetWinProxyStateDir(name string, vmtype define.VMType) (string, error) { dir, err := env.GetDataDir(vmtype) if err != nil { diff --git a/pkg/machine/machine_windows_test.go b/pkg/machine/machine_windows_test.go new file mode 100644 index 0000000000..48f3eafd53 --- /dev/null +++ b/pkg/machine/machine_windows_test.go @@ -0,0 +1,116 @@ +//go:build windows + +package machine + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// CreateNewItemWithPowerShell creates a new item using PowerShell. +// It's an helper to easily create junctions on Windows (as well as other file types). +// It constructs a PowerShell command to create a new item at the specified path with the given item type. +// If a target is provided, it includes it in the command. +// +// Parameters: +// - t: The testing.T instance. +// - path: The path where the new item will be created. +// - itemType: The type of the item to be created (e.g., "File", "SymbolicLink", "Junction"). +// - target: The target for the new item, if applicable. +func CreateNewItemWithPowerShell(t *testing.T, path string, itemType string, target string) { + var pwshCmd string + pwshCmd = "New-Item -Path " + path + " -ItemType " + itemType + if target != "" { + pwshCmd += " -Target " + target + } + cmd := exec.Command("pwsh", "-Command", pwshCmd) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err := cmd.Run() + require.NoError(t, err) +} + +// TestEvalSymlinksOrClean tests the EvalSymlinksOrClean function. +// In particular it verifies that EvalSymlinksOrClean behaves as +// filepath.EvalSymlink before Go 1.23 - with the exception of +// files under a mount point (juntion) that aren't resolved +// anymore. +// The old behavior of filepath.EvalSymlinks can be tested with +// the directive "//go:debug winsymlink=0" and replacing EvalSymlinksOrClean() +// with filepath.EvalSymlink(). +func TestEvalSymlinksOrClean(t *testing.T) { + // Create a temporary directory to store the normal file + normalFileDir := t.TempDir() + + // Create a temporary directory to store the (hard/sym)link files + linkFilesDir := t.TempDir() + + // Create a temporary directory where the mount point will be created + mountPointDir := t.TempDir() + + // Create a normal file + normalFile := filepath.Join(normalFileDir, "testFile") + CreateNewItemWithPowerShell(t, normalFile, "File", "") + + // Create a symlink file + symlinkFile := filepath.Join(linkFilesDir, "testSymbolicLink") + CreateNewItemWithPowerShell(t, symlinkFile, "SymbolicLink", normalFile) + + // Create a hardlink file + hardlinkFile := filepath.Join(linkFilesDir, "testHardLink") + CreateNewItemWithPowerShell(t, hardlinkFile, "HardLink", normalFile) + + // Create a mount point file + mountPoint := filepath.Join(mountPointDir, "testJunction") + mountPointFile := filepath.Join(mountPoint, "testFile") + CreateNewItemWithPowerShell(t, mountPoint, "Junction", normalFileDir) + + // Replaces the backslashes with forward slashes in the normal file path + normalFileWithBadSeparators := filepath.ToSlash(normalFile) + + tests := []struct { + name string + filePath string + want string + }{ + { + name: "Normal file", + filePath: normalFile, + want: normalFile, + }, + { + name: "File under a mount point (juntion)", + filePath: mountPointFile, + want: mountPointFile, + }, + { + name: "Symbolic link", + filePath: symlinkFile, + want: normalFile, + }, + { + name: "Hard link", + filePath: hardlinkFile, + want: hardlinkFile, + }, + { + name: "Bad separators in path", + filePath: normalFileWithBadSeparators, + want: normalFile, + }, + } + + for _, tt := range tests { + assert := assert.New(t) + t.Run(tt.name, func(t *testing.T) { + got, err := EvalSymlinksOrClean(tt.filePath) + require.NoError(t, err) + assert.Equal(tt.want, got) + }) + } +}