Make paths absolute before weakly_canonical and add regression tests

src/windows/common/filesystem.cpp

@@ -774,6 +774,35 @@ bool wsl::windows::common::filesystem::FileExists(_In_ LPCWSTR Path)
     return (Attributes != INVALID_FILE_ATTRIBUTES);
 }
 
+std::filesystem::path wsl::windows::common::filesystem::GetCanonicalPath(const std::filesystem::path& Path)
+{
+    std::error_code error;
+    auto canonicalPath = GetCanonicalPath(Path, error);
+    THROW_HR_IF_MSG(HRESULT_FROM_WIN32(error.value()), !!error, "GetCanonicalPath(%ls)", Path.c_str());
+
+    return canonicalPath;
+}
+
+std::filesystem::path wsl::windows::common::filesystem::GetCanonicalPath(const std::filesystem::path& Path, std::error_code& Error)
+{
+    // absolute() is applied first because weakly_canonical() does not resolve a relative path
+    // against the current directory on its own. Its result is checked before canonicalizing because
+    // weakly_canonical() clears Error on success, which would otherwise mask an absolute() failure.
+    const auto absolutePath = std::filesystem::absolute(Path, Error);
+    if (Error)
+    {
+        return {};
+    }
+
+    auto canonicalPath = std::filesystem::weakly_canonical(absolutePath, Error);
+    if (Error)
+    {
+        return {};
+    }
+
+    return canonicalPath;
+}
+
 std::filesystem::path wsl::windows::common::filesystem::GetFullPath(_In_ LPCWSTR Path)
 {
     DWORD Attributes = GetFileAttributesW(Path);

src/windows/common/filesystem.hpp

@@ -137,6 +137,21 @@ void EnsureDirectoryWithAttributes(_In_ PCWSTR Path, _In_ ULONG Mode, _In_ ULONG
 
 bool FileExists(_In_ LPCWSTR Path);
 
+/// <summary>
+/// Resolves Path to an absolute, canonical form. The path is made absolute against the current
+/// directory first because std::filesystem::weakly_canonical does not reliably resolve a relative
+/// path on its own. '..' components are collapsed and symlinks are resolved for the portion of the
+/// path that exists, so a path naming a file that does not exist yet still succeeds.
+/// Throws on failure.
+/// </summary>
+std::filesystem::path GetCanonicalPath(const std::filesystem::path& Path);
+
+/// <summary>
+/// Non-throwing overload of GetCanonicalPath. On failure Error is set and an empty path is
+/// returned; on success Error is cleared.
+/// </summary>
+std::filesystem::path GetCanonicalPath(const std::filesystem::path& Path, std::error_code& Error);
+
 std::filesystem::path GetFullPath(_In_ LPCWSTR Path);
 
 std::pair<std::string, std::string> GetHostAndDomainNames();

src/windows/service/exe/LxssUserSession.cpp

@@ -3852,7 +3852,7 @@ void LxssUserSessionImpl::_ValidateDistributionNameAndPathNotInUse(
 
     if (Path != nullptr)
     {
-        canonicalPath = std::filesystem::weakly_canonical(Path, error);
+        canonicalPath = wsl::windows::common::filesystem::GetCanonicalPath(Path, error);
         if (error)
         {
             LOG_WIN32(error.value());
@@ -3896,7 +3896,7 @@ void LxssUserSessionImpl::_ValidateDistributionNameAndPathNotInUse(
 
         if (Path != nullptr)
         {
-            auto canonicalDistroPath = std::filesystem::weakly_canonical(configuration.BasePath, error);
+            auto canonicalDistroPath = wsl::windows::common::filesystem::GetCanonicalPath(configuration.BasePath, error);
             if (error)
             {
                 LOG_WIN32(error.value());

src/windows/service/exe/WslCoreVm.cpp

@@ -2182,7 +2182,7 @@ std::tuple<std::wstring, std::wstring, std::wstring> WslCoreVm::AddVirtioFsShare
         sharePath.push_back(L'\\');
     }
 
-    sharePath = std::filesystem::weakly_canonical(sharePath).wstring();
+    sharePath = wsl::windows::common::filesystem::GetCanonicalPath(sharePath).wstring();
 
     std::wstring effectiveOptions(Options);
 

src/windows/wslc/arguments/SpecParsing.cpp

@@ -167,10 +167,10 @@ services::BuildSecret ParseSecretSpec(const std::wstring& spec)
         // Normalize to an absolute path (the service requires one to mount the file's directory) but do
         // not verify the file exists or is a regular file here: that would be a TOCTOU race with the
         // build, and the file may only be reachable from the service's context. Let the service/BuildKit
-        // reject an unmountable or unreadable file instead. weakly_canonical resolves a relative path
+        // reject an unmountable or unreadable file instead. GetCanonicalPath resolves a relative path
         // against the current directory, collapses '..', and resolves symlinks for the portion of the
         // path that exists; it succeeds for a missing file but still reports genuine errors.
-        auto absPath = std::filesystem::weakly_canonical(srcPath, ec);
+        auto absPath = wsl::windows::common::filesystem::GetCanonicalPath(srcPath, ec);
         if (ec.value() != 0)
         {
             throw ArgumentException(

src/windows/wslc/services/ImageService.cpp

@@ -16,6 +16,7 @@ Module Name:
 #include "SessionService.h"
 #include "SpecParsing.h"
 #include "WarningCallback.h"
+#include <filesystem.hpp>
 #include <wslutil.h>
 #include <HandleConsoleProgressBar.h>
 #include <relay.hpp>
@@ -266,7 +267,7 @@ void ImageService::Build(
     std::wstring iidPathStr;
     if (iidFilePath.has_value())
     {
-        iidPathStr = std::filesystem::weakly_canonical(std::filesystem::absolute(*iidFilePath)).wstring();
+        iidPathStr = wsl::windows::common::filesystem::GetCanonicalPath(*iidFilePath).wstring();
     }
 
     WSLCBuildImageOptions options{

src/windows/wslc/tasks/ContainerTasks.cpp

@@ -23,6 +23,7 @@ Module Name:
 #include "SessionService.h"
 #include "TableOutput.h"
 #include <wil/result_macros.h>
+#include <filesystem.hpp>
 #include <wslc_schema.h>
 #include <filesystem>
 
@@ -389,7 +390,7 @@ void ContainerCp(CLIExecutionContext& context)
 
         // Resolve any symlinks in the target path since tar.exe refuses to extract through a symlink.
         std::error_code canonicalError;
-        auto absTarget = std::filesystem::weakly_canonical(std::filesystem::absolute(target), canonicalError);
+        auto absTarget = wsl::windows::common::filesystem::GetCanonicalPath(target, canonicalError);
         if (canonicalError)
         {
             absTarget = std::filesystem::absolute(target); // Fall back to absolute if canonicalization fails.

src/windows/wslinstaller/exe/WslInstaller.cpp

@@ -24,7 +24,7 @@ std::wstring GetMsiPackagePath()
 
     static_assert(!wsl::shared::OfficialBuild);
 
-    return std::filesystem::weakly_canonical(WSL_DEV_THIN_MSI_PACKAGE).wstring();
+    return wsl::windows::common::filesystem::GetCanonicalPath(WSL_DEV_THIN_MSI_PACKAGE).wstring();
 
 #endif
 
@@ -50,7 +50,7 @@ try
     }
 
     // A canonical path is required because msiexec doesn't like symlinks.
-    return UpgradeLogInfo{std::filesystem::weakly_canonical(path), true};
+    return UpgradeLogInfo{wsl::windows::common::filesystem::GetCanonicalPath(path), true};
 }
 catch (...)
 {

test/windows/CMakeLists.txt

@@ -5,6 +5,7 @@ set(SOURCES
     NetworkTests.cpp
     Plan9Tests.cpp
     DrvFsTests.cpp
+    FilesystemUnitTests.cpp
     Common.cpp
     PluginTests.cpp
     PolicyTests.cpp

test/windows/FilesystemUnitTests.cpp

@@ -0,0 +1,147 @@
+/*++
+
+Copyright (c) Microsoft. All rights reserved.
+
+Module Name:
+
+    FilesystemUnitTests.cpp
+
+Abstract:
+
+    This file contains unit tests for the helpers in src/windows/common/filesystem.cpp.
+    These tests only read from the local filesystem so they do not require an installed distribution.
+
+--*/
+
+#include "precomp.h"
+#include "Common.h"
+
+using wsl::windows::common::filesystem::GetCanonicalPath;
+
+namespace {
+
+// Returns a file name that does not exist in the given directory.
+std::wstring UniqueMissingName(const std::filesystem::path& Directory)
+{
+    static int counter = 0;
+    const auto name = std::format(L"wsl_ut_canonical_{}_{}.txt", GetCurrentProcessId(), ++counter);
+    VERIFY_IS_FALSE(std::filesystem::exists(Directory / name));
+
+    return name;
+}
+
+// The canonical form of the current directory, which is what a relative path is expected to resolve
+// against. std::filesystem::canonical is used rather than weakly_canonical so the expected value is
+// computed independently of the API under test.
+std::filesystem::path CanonicalCurrentDirectory()
+{
+    return std::filesystem::canonical(std::filesystem::current_path());
+}
+
+// A path that std::filesystem::absolute is guaranteed to reject, because it exceeds the longest path
+// Win32 can express. An empty path is not used: whether absolute rejects one is implementation
+// defined, and some standard library versions accept it.
+std::filesystem::path UnresolvablePath()
+{
+    return {L"C:\\" + std::wstring(40000, L'a')};
+}
+
+// A file that is known to exist, used to cover paths that resolve to a real filesystem entry. The
+// test module itself is used so that no file has to be created.
+std::filesystem::path ExistingFile()
+{
+    return {wil::GetModuleFileNameW<std::wstring>(wil::GetModuleInstanceHandle())};
+}
+
+} // namespace
+
+namespace FilesystemUnitTests {
+class FilesystemUnitTests
+{
+    WSL_TEST_CLASS(FilesystemUnitTests)
+
+    // A relative path naming a file that does not exist must still resolve to an absolute path.
+    // std::filesystem::weakly_canonical cannot do this on its own: it builds its result from the
+    // longest leading sequence of elements that exist, so a bare missing file name has nothing to
+    // canonicalize and is returned unchanged.
+    TEST_METHOD(GetCanonicalPath_RelativeMissingPathIsMadeAbsolute)
+    {
+        const auto name = UniqueMissingName(std::filesystem::current_path());
+        VERIFY_IS_FALSE(std::filesystem::weakly_canonical(name).is_absolute());
+
+        const auto result = GetCanonicalPath(name);
+
+        VERIFY_IS_TRUE(result.is_absolute());
+        VERIFY_ARE_EQUAL((CanonicalCurrentDirectory() / name).wstring(), result.wstring());
+    }
+
+    // The same resolution must happen for a relative path whose target already exists.
+    TEST_METHOD(GetCanonicalPath_RelativeExistingPathIsMadeAbsolute)
+    {
+        const auto existing = ExistingFile();
+        const auto relativePath = std::filesystem::relative(existing, std::filesystem::current_path());
+        VERIFY_IS_FALSE(relativePath.empty());
+        VERIFY_IS_FALSE(relativePath.is_absolute());
+
+        const auto result = GetCanonicalPath(relativePath);
+
+        VERIFY_IS_TRUE(result.is_absolute());
+        VERIFY_ARE_EQUAL(std::filesystem::canonical(existing).wstring(), result.wstring());
+    }
+
+    // '.' and '..' components must be collapsed even when the intermediate directory does not exist.
+    TEST_METHOD(GetCanonicalPath_CollapsesDotSegments)
+    {
+        const auto name = UniqueMissingName(std::filesystem::current_path());
+
+        const auto result = GetCanonicalPath(L".\\nonexistent\\..\\" + name);
+
+        VERIFY_ARE_EQUAL((CanonicalCurrentDirectory() / name).wstring(), result.wstring());
+    }
+
+    // An already absolute path must be returned unchanged.
+    TEST_METHOD(GetCanonicalPath_AbsolutePathIsUnchanged)
+    {
+        const auto expected = CanonicalCurrentDirectory() / UniqueMissingName(std::filesystem::current_path());
+
+        VERIFY_ARE_EQUAL(expected.wstring(), GetCanonicalPath(expected).wstring());
+    }
+
+    // A failure from std::filesystem::absolute must be reported. absolute returns an empty path when
+    // it fails, and weakly_canonical succeeds on an empty path and clears the error_code, so calling
+    // the two in sequence without checking in between silently turns the failure into success.
+    TEST_METHOD(GetCanonicalPath_ErrorOverloadReportsFailure)
+    {
+        std::error_code error;
+        const auto result = GetCanonicalPath(UnresolvablePath(), error);
+
+        VERIFY_ARE_NOT_EQUAL(std::error_code{}, error);
+        VERIFY_IS_TRUE(result.empty());
+    }
+
+    // Error must be cleared when the call succeeds so callers can reuse the same variable.
+    TEST_METHOD(GetCanonicalPath_ErrorOverloadClearsErrorOnSuccess)
+    {
+        const auto expected = CanonicalCurrentDirectory() / UniqueMissingName(std::filesystem::current_path());
+
+        auto error = std::make_error_code(std::errc::permission_denied);
+        const auto result = GetCanonicalPath(expected, error);
+
+        VERIFY_ARE_EQUAL(std::error_code{}, error);
+        VERIFY_ARE_EQUAL(expected.wstring(), result.wstring());
+    }
+
+    // The throwing overload must surface the same failure the non-throwing overload reports.
+    TEST_METHOD(GetCanonicalPath_ThrowingOverloadSurfacesFailure)
+    {
+        std::error_code error;
+        (void)GetCanonicalPath(UnresolvablePath(), error);
+        VERIFY_ARE_NOT_EQUAL(std::error_code{}, error);
+
+        const auto expectedResult = HRESULT_FROM_WIN32(error.value());
+        VERIFY_THROWS_SPECIFIC(GetCanonicalPath(UnresolvablePath()), wil::ResultException, [&](const wil::ResultException& e) {
+            return e.GetErrorCode() == expectedResult;
+        });
+    }
+};
+} // namespace FilesystemUnitTests

test/windows/InstallerTests.cpp

@@ -50,7 +50,7 @@ class InstallerTests
 
         WEX::Common::String MsixPackagePath;
         WEX::TestExecution::RuntimeParameters::TryGetValue(L"Package", MsixPackagePath);
-        m_msixPackagePath = std::filesystem::weakly_canonical(static_cast<std::wstring>(MsixPackagePath)).wstring();
+        m_msixPackagePath = wsl::windows::common::filesystem::GetCanonicalPath(static_cast<std::wstring>(MsixPackagePath)).wstring();
         VERIFY_IS_FALSE(m_msixPackagePath.empty());
 
         for (const auto& e : m_packageManager.FindPackages(wsl::windows::common::wslutil::c_msixPackageFamilyName))
@@ -61,7 +61,7 @@ class InstallerTests
 
 #ifdef WSL_DEV_THIN_MSI_PACKAGE
 
-        m_msiPath = std::filesystem::weakly_canonical(WSL_DEV_THIN_MSI_PACKAGE).wstring();
+        m_msiPath = wsl::windows::common::filesystem::GetCanonicalPath(WSL_DEV_THIN_MSI_PACKAGE).wstring();
 
 #else
 
@@ -383,12 +383,12 @@ class InstallerTests
 
         if (auto found = L"wsl." + version + arch + L".msi"; PathFileExists(found.c_str()))
         {
-            installerFile = std::filesystem::weakly_canonical(found);
+            installerFile = wsl::windows::common::filesystem::GetCanonicalPath(found);
             cleanup.release();
         }
         else if (auto found = L"Microsoft.WSL_" + version + L".0_x64_ARM64.msixbundle"; PathFileExists(found.c_str()))
         {
-            installerFile = std::filesystem::weakly_canonical(found);
+            installerFile = wsl::windows::common::filesystem::GetCanonicalPath(found);
             cleanup.release();
         }
         else

test/windows/UnitTests.cpp

@@ -2622,7 +2622,7 @@ Error code: Wsl/InstallDistro/WSL_E_DISTRO_NOT_FOUND
     WSL2_TEST_METHOD(CorruptedVhd)
     {
         // Create a 100MB vhd without a filesystem.
-        auto distroPath = std::filesystem::weakly_canonical(wil::GetCurrentDirectoryW<std::wstring>());
+        auto distroPath = wsl::windows::common::filesystem::GetCanonicalPath(wil::GetCurrentDirectoryW<std::wstring>());
         auto vhdPath = distroPath / L"CorruptedTest.vhdx";
 
         VIRTUAL_STORAGE_TYPE storageType{};
@@ -3083,7 +3083,7 @@ Error code: Wsl/InstallDistro/WSL_E_DISTRO_NOT_FOUND
             VERIFY_IS_TRUE(std::filesystem::exists(std::format(L"{}\\ext4.vhdx", testFolder)));
         }
 
-        auto absolutePath = std::filesystem::weakly_canonical(".").wstring();
+        auto absolutePath = wsl::windows::common::filesystem::GetCanonicalPath(".").wstring();
 
         // Move the distro to a different folder (absolute path)
         {

test/windows/WSLCTests.cpp

@@ -9369,8 +9369,9 @@ class WSLCTests
 
     WSLC_TEST_METHOD(ContainerVolumesAdvanced)
     {
-        auto hostFolder = std::filesystem::weakly_canonical(std::filesystem::current_path() / "test-volume");
-        auto symlinkFolder = std::filesystem::weakly_canonical(std::filesystem::current_path() / "test-volume-symlink");
+        auto hostFolder = wsl::windows::common::filesystem::GetCanonicalPath(std::filesystem::current_path() / "test-volume");
+        auto symlinkFolder =
+            wsl::windows::common::filesystem::GetCanonicalPath(std::filesystem::current_path() / "test-volume-symlink");
         std::filesystem::create_directories(hostFolder);
 
         auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() {