Do not convert volume target path to Windows style, implement tilde expansion for Windows (#2488)

Since we deal with Linux containers, the volume target path should always be a Unix style path, even on Windows. Prior to this, the code would convert the target path to a Windows style path on Windows hosts due to the use of filepath.Clean. This PR changes the code to use path.Clean for the target instead, which always produces a Unix style path. Pre-existing validation ensures that the user cannot pass Windows style paths as input.

Another issue which was discovered by @danbarr as part of this was inconsistent handling of tilde expansion (i.e. converting ~/foo to an absolute path with the user's home folder). This was exacerbated by ToolHive using hand-rolled code to do absolute path expansion when there is a function in the Go standard library to do this. The new code explicitly checks to see if the path starts with ~ on Windows hosts, replaces it with the contents of the USERPROFILE env var if true, and then uses the Go standard library to convert to an absolute path.

Author: dmjb

pkg/container/docker/client.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	rt "runtime"
 	"slices"
 	"strconv"
 	"strings"
@@ -756,15 +757,18 @@ func convertRelativePathToAbsolute(source string, mountDecl permissions.MountDec
 		return source, true
 	}
 
-	// Get the current working directory
-	cwd, err := os.Getwd()
+	// Special case for Windows: expand ~ to user profile directory.
+	if rt.GOOS == "windows" && strings.HasPrefix(source, "~") {
+		homeDir := os.Getenv("USERPROFILE")
+		source = strings.Replace(source, "~", homeDir, 1)
+	}
+
+	absPath, err := filepath.Abs(source)
 	if err != nil {
-		logger.Warnf("Warning: Failed to get current working directory: %v", err)
+		logger.Warnf("Warning: Failed to convert to absolute path: %s (%v)", mountDecl, err)
 		return "", false
 	}
 
-	// Convert relative path to absolute path
-	absPath := filepath.Join(cwd, source)
 	logger.Infof("Converting relative path to absolute: %s -> %s", mountDecl, absPath)
 	return absPath, true
 }

pkg/permissions/profile.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	pkgpath "path"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -198,11 +199,6 @@ func isWindowsPath(path string) bool {
 	return false
 }
 
-// cleanPath cleans a path using filepath.Clean
-func cleanPath(path string) string {
-	return filepath.Clean(path)
-}
-
 // validateResourceScheme checks if a resource URI scheme is valid
 func validateResourceScheme(scheme string) bool {
 	return regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9_-]*$`).MatchString(scheme)
@@ -299,8 +295,10 @@ func parseResourceURI(declaration string) (source, target string, err error) {
 	}
 
 	// Clean paths
-	cleanedResource := cleanPath(resourceName)
-	cleanedTarget := cleanPath(containerPath)
+	cleanedResource := filepath.Clean(resourceName)
+	// For the target, we explicitly use path.Clean so that we do not convert
+	// Unix style paths into Windows style paths on Windows hosts
+	cleanedTarget := pkgpath.Clean(containerPath)
 
 	return scheme + "://" + cleanedResource, cleanedTarget, nil
 }
@@ -324,7 +322,7 @@ func parseWindowsPath(declaration string, colonPositions []int) (source, target
 		if err := validatePath(declaration); err != nil {
 			return "", "", err
 		}
-		cleanedPath := cleanPath(declaration)
+		cleanedPath := filepath.Clean(declaration)
 		return cleanedPath, cleanedPath, nil
 	}
 
@@ -346,8 +344,9 @@ func parseWindowsPath(declaration string, colonPositions []int) (source, target
 			return "", "", err
 		}
 
-		cleanedSource := cleanPath(hostPath)
-		cleanedTarget := cleanPath(containerPath)
+		cleanedSource := filepath.Clean(hostPath)
+		// See comment above about using path.Clean instead of filepath.Clean.
+		cleanedTarget := pkgpath.Clean(containerPath)
 		return cleanedSource, cleanedTarget, nil
 	}
 
@@ -381,8 +380,9 @@ func parseHostContainerPath(declaration string, colonPositions []int) (source, t
 			return "", "", err
 		}
 
-		cleanedSource := cleanPath(hostPath)
-		cleanedTarget := cleanPath(containerPath)
+		cleanedSource := filepath.Clean(hostPath)
+		// See comment above about using path.Clean instead of filepath.Clean.
+		cleanedTarget := pkgpath.Clean(containerPath)
 		return cleanedSource, cleanedTarget, nil
 	}
 
@@ -401,7 +401,8 @@ func parseSinglePath(declaration string) (source, target string, err error) {
 		return "", "", err
 	}
 
-	cleanedPath := cleanPath(declaration)
+	// Single path should always be converted to OS-specific cleaned path.
+	cleanedPath := filepath.Clean(declaration)
 	return cleanedPath, cleanedPath, nil
 }