From 7b5b42399554a6981b6d88466b5c958faa9a27c4 Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Wed, 27 May 2026 10:34:20 -0700 Subject: [PATCH 1/9] Add marker-file validation for WSLC session storage --- localization/strings/en-US/Resources.resw | 4 ++ .../service/exe/WSLCSessionManager.cpp | 18 ++++- src/windows/service/exe/WSLCSessionManager.h | 5 +- src/windows/wslcsession/WSLCSession.cpp | 66 ++++++++++++++++++- .../wslcsession/WSLCSessionFactory.cpp | 10 +++ src/windows/wslcsession/WSLCSessionFactory.h | 6 +- test/windows/WSLCTests.cpp | 34 ++++++++++ .../wslc/e2e/WSLCE2ESessionEnterTests.cpp | 23 +++++++ 8 files changed, 162 insertions(+), 4 deletions(-) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 99293088d..be578a2bb 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2359,6 +2359,10 @@ For privacy information about this product please visit https://aka.ms/privacy.< No WSLC session found in '{}' {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + + Cannot use '{}' as session storage because the directory is not empty + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + Invalid image tag format: '{}'. Expected format is 'name:tag' {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated diff --git a/src/windows/service/exe/WSLCSessionManager.cpp b/src/windows/service/exe/WSLCSessionManager.cpp index 700d570d7..fcae1c19b 100644 --- a/src/windows/service/exe/WSLCSessionManager.cpp +++ b/src/windows/service/exe/WSLCSessionManager.cpp @@ -272,7 +272,18 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings, auto sessionSettings = CreateSessionSettings(sessionId, creatorPid, Settings, resolvedDisplayName.c_str()); wil::com_ptr session; wil::com_ptr serviceRef; - THROW_IF_FAILED(factory->CreateSession(&sessionSettings, vm.Get(), notifier.Get(), &session, &serviceRef)); + { + const auto factoryHr = factory->CreateSession(&sessionSettings, vm.Get(), notifier.Get(), &session, &serviceRef); + if (FAILED(factoryHr)) + { + if (auto comError = wslutil::GetCOMErrorInfo(); comError && comError->Message) + { + THROW_HR_WITH_USER_ERROR(factoryHr, comError->Message.get()); + } + + THROW_IF_FAILED(factoryHr); + } + } // Track the session via its service ref, along with metadata and security info. m_sessions.push_back(SessionEntry{ @@ -588,6 +599,11 @@ HRESULT WSLCSessionManager::OpenSessionByName(_In_ LPCWSTR DisplayName, _Out_ IW return CallImpl(&WSLCSessionManagerImpl::OpenSessionByName, DisplayName, Session); } +HRESULT WSLCSessionManager::InterfaceSupportsErrorInfo(_In_ REFIID riid) +{ + return riid == __uuidof(IWSLCSessionManager) ? S_OK : S_FALSE; +} + namespace wsl::windows::service::wslc { WSLCSessionManagerImpl* WSLCSessionManagerImpl::Instance() noexcept diff --git a/src/windows/service/exe/WSLCSessionManager.h b/src/windows/service/exe/WSLCSessionManager.h index ce5d31eab..025ae4f7b 100644 --- a/src/windows/service/exe/WSLCSessionManager.h +++ b/src/windows/service/exe/WSLCSessionManager.h @@ -187,7 +187,7 @@ class WSLCSessionManagerImpl } // namespace wsl::windows::service::wslc class DECLSPEC_UUID("a9b7a1b9-0671-405c-95f1-e0612cb4ce8f") WSLCSessionManager - : public Microsoft::WRL::RuntimeClass, IWSLCSessionManager, IFastRundown>, + : public Microsoft::WRL::RuntimeClass, IWSLCSessionManager, IFastRundown, ISupportErrorInfo>, public wsl::windows::service::wslc::COMImplClass { public: @@ -203,4 +203,7 @@ class DECLSPEC_UUID("a9b7a1b9-0671-405c-95f1-e0612cb4ce8f") WSLCSessionManager IFACEMETHOD(ListSessions)(_Out_ WSLCSessionListEntry** Sessions, _Out_ ULONG* SessionsCount) override; IFACEMETHOD(OpenSession)(_In_ ULONG Id, _Out_ IWSLCSession** Session) override; IFACEMETHOD(OpenSessionByName)(_In_ LPCWSTR DisplayName, _Out_ IWSLCSession** Session) override; + + // ISupportErrorInfo: enables IErrorInfo marshaling across COM boundaries. + IFACEMETHOD(InterfaceSupportsErrorInfo)(_In_ REFIID riid) override; }; diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index f9ca9ffd6..c141df8c3 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -340,6 +340,63 @@ void WSLCSession::SetDestructionCallback(std::function&& callback) m_destructionCallback = std::move(callback); } +namespace { + + // Zero-byte sentinel written at the root of a session storage directory so we can + // distinguish a directory we own from one the user is using for something else. + constexpr auto c_sessionMarkerFile = L"wslcsession"; + + // Validates StoragePath and stamps the marker if needed. + // IsExistingStorage = true when a session VHD was just attached (legacy directories + // pre-dating the marker are tolerated and upgraded). When false the directory must + // be empty or non-existent so we never overwrite unrelated user files. + void EnsureSessionMarker(const std::filesystem::path& StoragePath, bool IsExistingStorage) + { + const auto markerPath = StoragePath / c_sessionMarkerFile; + + const auto markerAttrs = GetFileAttributesW(markerPath.c_str()); + if (markerAttrs != INVALID_FILE_ATTRIBUTES) + { + // A directory at the marker name means the storage path is used for something else. + THROW_HR_WITH_USER_ERROR_IF( + E_INVALIDARG, + Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), + WI_IsFlagSet(markerAttrs, FILE_ATTRIBUTE_DIRECTORY)); + + return; + } + + std::error_code ec; + const bool isDir = std::filesystem::is_directory(StoragePath, ec); + + // is_directory sets ec when the path does not exist (ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND). + // That is the normal case for new sessions — only throw on unexpected errors. + if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND) + { + THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_directory failed for %ls", StoragePath.c_str()); + } + + if (isDir) + { + if (!IsExistingStorage) + { + // New session into an existing directory: require it to be empty. + const bool empty = std::filesystem::is_empty(StoragePath, ec); + THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", StoragePath.c_str()); + THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), !empty); + } + } + else + { + std::filesystem::create_directories(StoragePath); + } + + wil::unique_hfile marker{CreateFileW(markerPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; + THROW_LAST_ERROR_IF_MSG(!marker, "Failed to create marker file: %ls", markerPath.c_str()); + } + +} // namespace + void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID UserSid) { if (Settings.StoragePath == nullptr) @@ -386,10 +443,12 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID Localization::MessageWslcSessionStorageNotFound(Settings.StoragePath), WI_IsFlagSet(Settings.StorageFlags, WSLCSessionStorageFlagsNoCreate)); + // Validate the directory is safe to claim and stamp the marker before creating the VHD. + EnsureSessionMarker(storagePath, /*IsExistingStorage=*/false); + // If the VHD wasn't found, create it. WSL_LOG("CreateStorageVhd", TraceLoggingValue(m_storageVhdPath.c_str(), "StorageVhdPath")); - std::filesystem::create_directories(storagePath); wsl::core::filesystem::CreateVhd(m_storageVhdPath.c_str(), Settings.MaximumStorageSizeMb * _1MB, UserSid, false, false); vhdCreated = true; @@ -399,6 +458,11 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID // Then format it. m_virtualMachine->Ext4Format(diskDevice); } + else + { + // Stamp the marker for legacy directories created before the marker convention. + EnsureSessionMarker(storagePath, /*IsExistingStorage=*/true); + } // Mount the device to /root. m_virtualMachine->Mount(diskDevice.c_str(), c_containerdStorage, "ext4", "", 0); diff --git a/src/windows/wslcsession/WSLCSessionFactory.cpp b/src/windows/wslcsession/WSLCSessionFactory.cpp index e08f72090..17945dfed 100644 --- a/src/windows/wslcsession/WSLCSessionFactory.cpp +++ b/src/windows/wslcsession/WSLCSessionFactory.cpp @@ -37,6 +37,11 @@ HRESULT wslc::WSLCSessionFactory::CreateSession( _Out_ IWSLCSessionReference** ServiceRef) try { + // Establish a COM execution context so any user-error message set during session init + // (e.g. by ConfigureStorage's THROW_HR_WITH_USER_ERROR) is serialized to IErrorInfo before + // returning to the caller (wslservice -> wslc CLI). + wsl::windows::common::COMServiceExecutionContext context; + *Session = nullptr; *ServiceRef = nullptr; @@ -67,6 +72,11 @@ try } CATCH_RETURN() +HRESULT wslc::WSLCSessionFactory::InterfaceSupportsErrorInfo(_In_ REFIID riid) +{ + return riid == __uuidof(IWSLCSessionFactory) ? S_OK : S_FALSE; +} + HRESULT wslc::WSLCSessionFactory::GetProcessHandle(_Out_ HANDLE* ProcessHandle) try { diff --git a/src/windows/wslcsession/WSLCSessionFactory.h b/src/windows/wslcsession/WSLCSessionFactory.h index 266517d28..df95c928a 100644 --- a/src/windows/wslcsession/WSLCSessionFactory.h +++ b/src/windows/wslcsession/WSLCSessionFactory.h @@ -30,7 +30,7 @@ Module Name: namespace wsl::windows::service::wslc { class DECLSPEC_UUID("9FCD2067-9FC6-4EFA-9EB0-698169EBF7D3") WSLCSessionFactory - : public Microsoft::WRL::RuntimeClass, IWSLCSessionFactory, IFastRundown> + : public Microsoft::WRL::RuntimeClass, IWSLCSessionFactory, IFastRundown, ISupportErrorInfo> { public: NON_COPYABLE(WSLCSessionFactory); @@ -52,6 +52,10 @@ class DECLSPEC_UUID("9FCD2067-9FC6-4EFA-9EB0-698169EBF7D3") WSLCSessionFactory IFACEMETHOD(GetProcessHandle)(_Out_ HANDLE* ProcessHandle) override; + // ISupportErrorInfo: enables IErrorInfo marshaling across COM process boundaries so that + // user-facing error messages set via THROW_HR_WITH_USER_ERROR propagate to the caller. + IFACEMETHOD(InterfaceSupportsErrorInfo)(_In_ REFIID riid) override; + private: std::function m_destructionCallback; }; diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index b0da4a00f..82e7c4046 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -451,6 +451,40 @@ class WSLCTests wil::com_ptr session; VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), E_INVALIDARG); } + + // Marker-file scenarios use unique temp paths so the test never touches shared default storage. + auto uniqueTempStoragePath = [] { + auto path = std::filesystem::temp_directory_path() / + std::format(L"wslc-test-storage-{}-{}", GetCurrentProcessId(), GetTickCount64()); + std::filesystem::create_directories(path); + return path; + }; + + auto expectMarkerRejection = [&](LPCWSTR name, auto&& populateDir) { + const auto storagePath = uniqueTempStoragePath(); + auto cleanup = wil::scope_exit([&]() { + std::error_code ignored; + std::filesystem::remove_all(storagePath, ignored); + }); + + populateDir(storagePath); + + auto settings = GetDefaultSessionSettings(name); + const auto storagePathString = storagePath.wstring(); + settings.StoragePath = storagePathString.c_str(); + wil::com_ptr session; + VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), E_INVALIDARG); + ValidateCOMErrorMessage(std::format(L"Cannot use '{}' as session storage because the directory is not empty", storagePathString)); + }; + + // Reject non-empty storage directory without the wslcsession marker. + expectMarkerRejection( + L"storage-not-empty", [](const std::filesystem::path& dir) { std::ofstream{dir / L"userfile.txt"} << "data"; }); + + // Reject directory whose marker name is occupied by a sub-directory. + expectMarkerRejection(L"marker-is-directory", [](const std::filesystem::path& dir) { + std::filesystem::create_directory(dir / L"wslcsession"); + }); } struct VmInfo diff --git a/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp b/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp index c10f43295..720a90deb 100644 --- a/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp @@ -114,5 +114,28 @@ class WSLCE2ESessionEnterTests .ExitCode = 1, }); } + + WSLC_TEST_METHOD(WSLCE2E_SessionEnter_MarkerFileValidation) + { + // The non-empty-rejection case is covered by WSLCTests::CreateSessionValidation against a + // temp directory; reproducing it here would require mutating the shared default storage. + const auto storagePath = GetDefaultStoragePath(); + const auto markerPath = storagePath / L"wslcsession"; + + auto createSessionAndExpectMarker = [&]() { + auto terminateOnExit = wil::scope_exit([&]() { RunWslc(L"system session terminate"); }); + RunWslc(L"image ls").Verify({.ExitCode = 0}); + VERIFY_IS_TRUE(std::filesystem::exists(markerPath)); + }; + + // Default session creation writes the marker file. + createSessionAndExpectMarker(); + + // Legacy directory auto-upgrade: removing the marker then re-entering recreates it. + std::error_code ec; + VERIFY_IS_TRUE(std::filesystem::remove(markerPath, ec)); + VERIFY_IS_FALSE(std::filesystem::exists(markerPath)); + createSessionAndExpectMarker(); + } }; } // namespace WSLCE2ETests From d88f7357e15705d86eef94356e5f26e850f84c64 Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Wed, 27 May 2026 20:37:02 -0700 Subject: [PATCH 2/9] Fix CreateSessionValidation test --- localization/strings/en-US/Resources.resw | 2 +- src/windows/wslcsession/WSLCSession.cpp | 153 +++++++++--------- .../wslcsession/WSLCSessionFactory.cpp | 14 +- 3 files changed, 90 insertions(+), 79 deletions(-) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index be578a2bb..516a7b39b 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2359,7 +2359,7 @@ For privacy information about this product please visit https://aka.ms/privacy.< No WSLC session found in '{}' {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated - + Cannot use '{}' as session storage because the directory is not empty {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index c141df8c3..442c76ecc 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -257,6 +257,63 @@ try } CATCH_RETURN(); +namespace { + + // Zero-byte sentinel written at the root of a session storage directory so we can + // distinguish a directory we own from one the user is using for something else. + constexpr auto c_sessionMarkerFile = L"wslcsession"; + + // Validates StoragePath and stamps the marker if needed. + // IsExistingStorage = true when a session VHD was just attached (legacy directories + // pre-dating the marker are tolerated and upgraded). When false the directory must + // be empty or non-existent so we never overwrite unrelated user files. + void EnsureSessionMarker(const std::filesystem::path& StoragePath, bool IsExistingStorage) + { + const auto markerPath = StoragePath / c_sessionMarkerFile; + + const auto markerAttrs = GetFileAttributesW(markerPath.c_str()); + if (markerAttrs != INVALID_FILE_ATTRIBUTES) + { + // A directory at the marker name means the storage path is used for something else. + THROW_HR_WITH_USER_ERROR_IF( + E_INVALIDARG, + Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), + WI_IsFlagSet(markerAttrs, FILE_ATTRIBUTE_DIRECTORY)); + + return; + } + + std::error_code ec; + const bool isDir = std::filesystem::is_directory(StoragePath, ec); + + // is_directory sets ec when the path does not exist (ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND). + // That is the normal case for new sessions — only throw on unexpected errors. + if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND) + { + THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_directory failed for %ls", StoragePath.c_str()); + } + + if (isDir) + { + if (!IsExistingStorage) + { + // New session into an existing directory: require it to be empty. + const bool empty = std::filesystem::is_empty(StoragePath, ec); + THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", StoragePath.c_str()); + THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), !empty); + } + } + else + { + std::filesystem::create_directories(StoragePath); + } + + wil::unique_hfile marker{CreateFileW(markerPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; + THROW_LAST_ERROR_IF_MSG(!marker, "Failed to create marker file: %ls", markerPath.c_str()); + } + +} // namespace + HRESULT WSLCSession::Initialize(_In_ const WSLCSessionInitSettings* Settings, _In_ IWSLCVirtualMachine* Vm, _In_ IWSLCPluginNotifier* PluginNotifier) try { @@ -278,6 +335,27 @@ try TraceLoggingValue(m_displayName.c_str(), "DisplayName"), TraceLoggingValue(Settings->CreatorPid, "CreatorPid")); + // Pre-validate the storage path and stamp the marker file before any VM resources + // are created. The check must run before errorCleanup is established because + // Terminate()'s cleanup of partially-initialized state can overwrite the current + // error context during stack unwinding, swallowing the user-facing message. + if (Settings->StoragePath != nullptr) + { + std::filesystem::path storagePath{Settings->StoragePath}; + THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessagePathNotAbsolute(Settings->StoragePath), !storagePath.is_absolute()); + + std::error_code ec; + const bool vhdExists = std::filesystem::exists(storagePath / L"storage.vhdx", ec); + + // Reject up-front if the caller forbade creating new storage and there is no VHD to attach. + THROW_HR_WITH_USER_ERROR_IF( + HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND), + Localization::MessageWslcSessionStorageNotFound(Settings->StoragePath), + !vhdExists && WI_IsFlagSet(Settings->StorageFlags, WSLCSessionStorageFlagsNoCreate)); + + EnsureSessionMarker(storagePath, /*IsExistingStorage=*/vhdExists); + } + // Create the VM. m_virtualMachine.emplace(Vm, Settings, m_sessionTerminatingEvent.get()); @@ -340,63 +418,6 @@ void WSLCSession::SetDestructionCallback(std::function&& callback) m_destructionCallback = std::move(callback); } -namespace { - - // Zero-byte sentinel written at the root of a session storage directory so we can - // distinguish a directory we own from one the user is using for something else. - constexpr auto c_sessionMarkerFile = L"wslcsession"; - - // Validates StoragePath and stamps the marker if needed. - // IsExistingStorage = true when a session VHD was just attached (legacy directories - // pre-dating the marker are tolerated and upgraded). When false the directory must - // be empty or non-existent so we never overwrite unrelated user files. - void EnsureSessionMarker(const std::filesystem::path& StoragePath, bool IsExistingStorage) - { - const auto markerPath = StoragePath / c_sessionMarkerFile; - - const auto markerAttrs = GetFileAttributesW(markerPath.c_str()); - if (markerAttrs != INVALID_FILE_ATTRIBUTES) - { - // A directory at the marker name means the storage path is used for something else. - THROW_HR_WITH_USER_ERROR_IF( - E_INVALIDARG, - Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), - WI_IsFlagSet(markerAttrs, FILE_ATTRIBUTE_DIRECTORY)); - - return; - } - - std::error_code ec; - const bool isDir = std::filesystem::is_directory(StoragePath, ec); - - // is_directory sets ec when the path does not exist (ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND). - // That is the normal case for new sessions — only throw on unexpected errors. - if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND) - { - THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_directory failed for %ls", StoragePath.c_str()); - } - - if (isDir) - { - if (!IsExistingStorage) - { - // New session into an existing directory: require it to be empty. - const bool empty = std::filesystem::is_empty(StoragePath, ec); - THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", StoragePath.c_str()); - THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), !empty); - } - } - else - { - std::filesystem::create_directories(StoragePath); - } - - wil::unique_hfile marker{CreateFileW(markerPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; - THROW_LAST_ERROR_IF_MSG(!marker, "Failed to create marker file: %ls", markerPath.c_str()); - } - -} // namespace - void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID UserSid) { if (Settings.StoragePath == nullptr) @@ -406,9 +427,10 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID return; } + // N.B. The storage path is validated and the marker file is stamped earlier in + // Initialize(), before errorCleanup is established. This is required so user-facing + // rejection errors propagate cleanly to the caller. std::filesystem::path storagePath{Settings.StoragePath}; - THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessagePathNotAbsolute(Settings.StoragePath), !storagePath.is_absolute()); - m_storageVhdPath = storagePath / "storage.vhdx"; std::string diskDevice; @@ -438,14 +460,6 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID "Failed to attach vhd: %ls", m_storageVhdPath.c_str()); - THROW_HR_WITH_USER_ERROR_IF( - HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND), - Localization::MessageWslcSessionStorageNotFound(Settings.StoragePath), - WI_IsFlagSet(Settings.StorageFlags, WSLCSessionStorageFlagsNoCreate)); - - // Validate the directory is safe to claim and stamp the marker before creating the VHD. - EnsureSessionMarker(storagePath, /*IsExistingStorage=*/false); - // If the VHD wasn't found, create it. WSL_LOG("CreateStorageVhd", TraceLoggingValue(m_storageVhdPath.c_str(), "StorageVhdPath")); @@ -458,11 +472,6 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID // Then format it. m_virtualMachine->Ext4Format(diskDevice); } - else - { - // Stamp the marker for legacy directories created before the marker convention. - EnsureSessionMarker(storagePath, /*IsExistingStorage=*/true); - } // Mount the device to /root. m_virtualMachine->Mount(diskDevice.c_str(), c_containerdStorage, "ext4", "", 0); diff --git a/src/windows/wslcsession/WSLCSessionFactory.cpp b/src/windows/wslcsession/WSLCSessionFactory.cpp index 17945dfed..bbe69e709 100644 --- a/src/windows/wslcsession/WSLCSessionFactory.cpp +++ b/src/windows/wslcsession/WSLCSessionFactory.cpp @@ -19,9 +19,8 @@ Module Name: #include "WSLCSessionFactory.h" #include "WSLCSession.h" #include "WSLCSessionReference.h" -#include "wslutil.h" +#include "ExecutionContext.h" -namespace wslutil = wsl::windows::common::wslutil; namespace wslc = wsl::windows::service::wslc; void wslc::WSLCSessionFactory::SetDestructionCallback(std::function&& callback) @@ -37,9 +36,9 @@ HRESULT wslc::WSLCSessionFactory::CreateSession( _Out_ IWSLCSessionReference** ServiceRef) try { - // Establish a COM execution context so any user-error message set during session init - // (e.g. by ConfigureStorage's THROW_HR_WITH_USER_ERROR) is serialized to IErrorInfo before - // returning to the caller (wslservice -> wslc CLI). + // Establish a COM execution context so any user-error message set during + // session init (e.g. by ConfigureStorage's THROW_HR_WITH_USER_ERROR) is + // captured and serialized to IErrorInfo before returning to the caller. wsl::windows::common::COMServiceExecutionContext context; *Session = nullptr; @@ -53,7 +52,10 @@ try session->SetDestructionCallback(std::move(m_destructionCallback)); // Initialize the session with the VM. - RETURN_IF_FAILED(session->Initialize(Settings, Vm, PluginNotifier)); + // N.B. THROW_IF_FAILED (not RETURN_IF_FAILED) is required here so WIL's + // error callback invokes CollectError, which consumes the error string + // before ~COMServiceExecutionContext runs. + THROW_IF_FAILED(session->Initialize(Settings, Vm, PluginNotifier)); // Create the service session ref. It extracts metadata and a weak reference from the session. auto serviceRef = Microsoft::WRL::Make(session.Get()); From ba8a228a8bc873abec3519e48883482aa3dfecd0 Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Thu, 28 May 2026 09:35:50 -0700 Subject: [PATCH 3/9] fix test --- src/windows/wslcsession/WSLCSession.cpp | 108 +++++++++--------- .../wslc/e2e/WSLCE2ESessionEnterTests.cpp | 3 +- 2 files changed, 53 insertions(+), 58 deletions(-) diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 442c76ecc..07be07312 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -154,6 +154,57 @@ std::string GenerateContainerName(int retry) return name; } +// Zero-byte sentinel written at the root of a session storage directory so we can +// distinguish a directory we own from one the user is using for something else. +constexpr auto c_sessionMarkerFile = L"wslcsession"; + +// Validates StoragePath and stamps the marker if needed. +// IsExistingStorage = true when a session VHD was just attached (legacy directories +// pre-dating the marker are tolerated and upgraded). When false the directory must +// be empty or non-existent so we never overwrite unrelated user files. +void EnsureSessionMarker(const std::filesystem::path& StoragePath, bool IsExistingStorage) +{ + const auto markerPath = StoragePath / c_sessionMarkerFile; + + const auto markerAttrs = GetFileAttributesW(markerPath.c_str()); + if (markerAttrs != INVALID_FILE_ATTRIBUTES) + { + // A directory at the marker name means the storage path is used for something else. + THROW_HR_WITH_USER_ERROR_IF( + E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), WI_IsFlagSet(markerAttrs, FILE_ATTRIBUTE_DIRECTORY)); + + return; + } + + std::error_code ec; + const bool isDir = std::filesystem::is_directory(StoragePath, ec); + + // is_directory sets ec when the path does not exist (ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND). + // That is the normal case for new sessions — only throw on unexpected errors. + if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND) + { + THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_directory failed for %ls", StoragePath.c_str()); + } + + if (isDir) + { + if (!IsExistingStorage) + { + // New session into an existing directory: require it to be empty. + const bool empty = std::filesystem::is_empty(StoragePath, ec); + THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", StoragePath.c_str()); + THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), !empty); + } + } + else + { + std::filesystem::create_directories(StoragePath); + } + + wil::unique_hfile marker{CreateFileW(markerPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; + THROW_LAST_ERROR_IF_MSG(!marker, "Failed to create marker file: %ls", markerPath.c_str()); +} + } // namespace namespace wsl::windows::service::wslc { @@ -257,63 +308,6 @@ try } CATCH_RETURN(); -namespace { - - // Zero-byte sentinel written at the root of a session storage directory so we can - // distinguish a directory we own from one the user is using for something else. - constexpr auto c_sessionMarkerFile = L"wslcsession"; - - // Validates StoragePath and stamps the marker if needed. - // IsExistingStorage = true when a session VHD was just attached (legacy directories - // pre-dating the marker are tolerated and upgraded). When false the directory must - // be empty or non-existent so we never overwrite unrelated user files. - void EnsureSessionMarker(const std::filesystem::path& StoragePath, bool IsExistingStorage) - { - const auto markerPath = StoragePath / c_sessionMarkerFile; - - const auto markerAttrs = GetFileAttributesW(markerPath.c_str()); - if (markerAttrs != INVALID_FILE_ATTRIBUTES) - { - // A directory at the marker name means the storage path is used for something else. - THROW_HR_WITH_USER_ERROR_IF( - E_INVALIDARG, - Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), - WI_IsFlagSet(markerAttrs, FILE_ATTRIBUTE_DIRECTORY)); - - return; - } - - std::error_code ec; - const bool isDir = std::filesystem::is_directory(StoragePath, ec); - - // is_directory sets ec when the path does not exist (ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND). - // That is the normal case for new sessions — only throw on unexpected errors. - if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND) - { - THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_directory failed for %ls", StoragePath.c_str()); - } - - if (isDir) - { - if (!IsExistingStorage) - { - // New session into an existing directory: require it to be empty. - const bool empty = std::filesystem::is_empty(StoragePath, ec); - THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", StoragePath.c_str()); - THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), !empty); - } - } - else - { - std::filesystem::create_directories(StoragePath); - } - - wil::unique_hfile marker{CreateFileW(markerPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; - THROW_LAST_ERROR_IF_MSG(!marker, "Failed to create marker file: %ls", markerPath.c_str()); - } - -} // namespace - HRESULT WSLCSession::Initialize(_In_ const WSLCSessionInitSettings* Settings, _In_ IWSLCVirtualMachine* Vm, _In_ IWSLCPluginNotifier* PluginNotifier) try { diff --git a/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp b/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp index 720a90deb..c8bcf9263 100644 --- a/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp @@ -109,8 +109,9 @@ class WSLCE2ESessionEnterTests WSLC_TEST_METHOD(WSLCE2E_SessionEnter_StoragePathNotFound) { auto result = RunWslc(L"system session enter does-not-exist"); + const auto expectedPath = std::filesystem::absolute(L"does-not-exist").wstring(); result.Verify({ - .Stderr = L"The system cannot find the path specified. \r\nError code: ERROR_PATH_NOT_FOUND\r\n", + .Stderr = std::format(L"No WSLC session found in '{}'\r\nError code: ERROR_PATH_NOT_FOUND\r\n", expectedPath), .ExitCode = 1, }); } From 81268fad4fefdfb809abde762a6739d7ae315310 Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Thu, 28 May 2026 09:48:03 -0700 Subject: [PATCH 4/9] simplify comments and guard is_empty error check --- src/windows/wslcsession/WSLCSession.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 07be07312..d860623f1 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -192,7 +192,10 @@ void EnsureSessionMarker(const std::filesystem::path& StoragePath, bool IsExisti { // New session into an existing directory: require it to be empty. const bool empty = std::filesystem::is_empty(StoragePath, ec); - THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", StoragePath.c_str()); + if (ec) + { + THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", StoragePath.c_str()); + } THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), !empty); } } @@ -329,10 +332,8 @@ try TraceLoggingValue(m_displayName.c_str(), "DisplayName"), TraceLoggingValue(Settings->CreatorPid, "CreatorPid")); - // Pre-validate the storage path and stamp the marker file before any VM resources - // are created. The check must run before errorCleanup is established because - // Terminate()'s cleanup of partially-initialized state can overwrite the current - // error context during stack unwinding, swallowing the user-facing message. + // Validate the storage path before creating VM resources so rejection errors + // propagate cleanly (Terminate()'s cleanup can overwrite the error context). if (Settings->StoragePath != nullptr) { std::filesystem::path storagePath{Settings->StoragePath}; @@ -421,9 +422,7 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID return; } - // N.B. The storage path is validated and the marker file is stamped earlier in - // Initialize(), before errorCleanup is established. This is required so user-facing - // rejection errors propagate cleanly to the caller. + // Storage path validation and marker stamping are done in Initialize(). std::filesystem::path storagePath{Settings.StoragePath}; m_storageVhdPath = storagePath / "storage.vhdx"; From 48ed43509179204219b2f55ede3869ef37194aba Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Tue, 2 Jun 2026 20:37:53 -0700 Subject: [PATCH 5/9] Prevent session storage reuse by validating directory emptiness for new sessions --- src/windows/wslcsession/WSLCSession.cpp | 95 +++++++------------ .../wslcsession/WSLCSessionFactory.cpp | 9 +- src/windows/wslcsession/WSLCSessionFactory.h | 3 +- test/windows/WSLCTests.cpp | 31 ++---- .../wslc/e2e/WSLCE2ESessionEnterTests.cpp | 23 ----- 5 files changed, 47 insertions(+), 114 deletions(-) diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 6f944c105..46bb702c6 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -32,6 +32,7 @@ using wsl::windows::service::wslc::WSLCVirtualMachine; constexpr auto c_containerdStorage = "/var/lib/docker"; constexpr auto c_containerdSocket = "/run/containerd/containerd.sock"; constexpr auto c_dockerdReadyLogLine = "API listen on /var/run/docker.sock"; +constexpr auto c_storageVhdFilename = L"storage.vhdx"; constexpr DWORD c_processTerminateTimeoutMs = 30 * 1000; constexpr DWORD c_processKillTimeoutMs = 10 * 1000; @@ -154,60 +155,6 @@ std::string GenerateContainerName(int retry) return name; } -// Zero-byte sentinel written at the root of a session storage directory so we can -// distinguish a directory we own from one the user is using for something else. -constexpr auto c_sessionMarkerFile = L"wslcsession"; - -// Validates StoragePath and stamps the marker if needed. -// IsExistingStorage = true when a session VHD was just attached (legacy directories -// pre-dating the marker are tolerated and upgraded). When false the directory must -// be empty or non-existent so we never overwrite unrelated user files. -void EnsureSessionMarker(const std::filesystem::path& StoragePath, bool IsExistingStorage) -{ - const auto markerPath = StoragePath / c_sessionMarkerFile; - - const auto markerAttrs = GetFileAttributesW(markerPath.c_str()); - if (markerAttrs != INVALID_FILE_ATTRIBUTES) - { - // A directory at the marker name means the storage path is used for something else. - THROW_HR_WITH_USER_ERROR_IF( - E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), WI_IsFlagSet(markerAttrs, FILE_ATTRIBUTE_DIRECTORY)); - - return; - } - - std::error_code ec; - const bool isDir = std::filesystem::is_directory(StoragePath, ec); - - // is_directory sets ec when the path does not exist (ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND). - // That is the normal case for new sessions — only throw on unexpected errors. - if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND) - { - THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_directory failed for %ls", StoragePath.c_str()); - } - - if (isDir) - { - if (!IsExistingStorage) - { - // New session into an existing directory: require it to be empty. - const bool empty = std::filesystem::is_empty(StoragePath, ec); - if (ec) - { - THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", StoragePath.c_str()); - } - THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), !empty); - } - } - else - { - std::filesystem::create_directories(StoragePath); - } - - wil::unique_hfile marker{CreateFileW(markerPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; - THROW_LAST_ERROR_IF_MSG(!marker, "Failed to create marker file: %ls", markerPath.c_str()); -} - } // namespace namespace wsl::windows::service::wslc { @@ -332,23 +279,49 @@ try TraceLoggingValue(m_displayName.c_str(), "DisplayName"), TraceLoggingValue(Settings->CreatorPid, "CreatorPid")); - // Validate the storage path before creating VM resources so rejection errors - // propagate cleanly (Terminate()'s cleanup can overwrite the error context). + // Validate storage path before creating VM resources (Terminate() would clobber the error). if (Settings->StoragePath != nullptr) { std::filesystem::path storagePath{Settings->StoragePath}; THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessagePathNotAbsolute(Settings->StoragePath), !storagePath.is_absolute()); + // MSVC's std::filesystem sets ec to ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND + // for non-existent paths. Filter those out — only throw on real failures. + auto throwOnRealFsError = [](const std::error_code& ec, const std::filesystem::path& path, const char* op) { + if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND) + { + THROW_IF_WIN32_ERROR_MSG(ec.value(), "%hs failed for %ls", op, path.c_str()); + } + }; + std::error_code ec; - const bool vhdExists = std::filesystem::exists(storagePath / L"storage.vhdx", ec); + const auto vhdPath = storagePath / c_storageVhdFilename; + const bool vhdExists = std::filesystem::exists(vhdPath, ec); + throwOnRealFsError(ec, vhdPath, "exists"); - // Reject up-front if the caller forbade creating new storage and there is no VHD to attach. THROW_HR_WITH_USER_ERROR_IF( HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND), Localization::MessageWslcSessionStorageNotFound(Settings->StoragePath), !vhdExists && WI_IsFlagSet(Settings->StorageFlags, WSLCSessionStorageFlagsNoCreate)); - EnsureSessionMarker(storagePath, /*IsExistingStorage=*/vhdExists); + // Only check emptiness for new sessions. Existing sessions are identified by their VHD. + if (!vhdExists) + { + const bool isDir = std::filesystem::is_directory(storagePath, ec); + throwOnRealFsError(ec, storagePath, "is_directory"); + + if (isDir) + { + const bool empty = std::filesystem::is_empty(storagePath, ec); + THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", storagePath.c_str()); + + THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(storagePath.c_str()), !empty); + } + else + { + std::filesystem::create_directories(storagePath); + } + } } // Create the VM. @@ -422,9 +395,9 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID return; } - // Storage path validation and marker stamping are done in Initialize(). + // Storage path validation is done in Initialize() before any VM resources are created. std::filesystem::path storagePath{Settings.StoragePath}; - m_storageVhdPath = storagePath / "storage.vhdx"; + m_storageVhdPath = storagePath / c_storageVhdFilename; std::string diskDevice; std::optional diskLun{}; diff --git a/src/windows/wslcsession/WSLCSessionFactory.cpp b/src/windows/wslcsession/WSLCSessionFactory.cpp index bbe69e709..806fe9bbe 100644 --- a/src/windows/wslcsession/WSLCSessionFactory.cpp +++ b/src/windows/wslcsession/WSLCSessionFactory.cpp @@ -36,9 +36,7 @@ HRESULT wslc::WSLCSessionFactory::CreateSession( _Out_ IWSLCSessionReference** ServiceRef) try { - // Establish a COM execution context so any user-error message set during - // session init (e.g. by ConfigureStorage's THROW_HR_WITH_USER_ERROR) is - // captured and serialized to IErrorInfo before returning to the caller. + // Capture user-error messages and serialize them to IErrorInfo for the caller. wsl::windows::common::COMServiceExecutionContext context; *Session = nullptr; @@ -52,9 +50,8 @@ try session->SetDestructionCallback(std::move(m_destructionCallback)); // Initialize the session with the VM. - // N.B. THROW_IF_FAILED (not RETURN_IF_FAILED) is required here so WIL's - // error callback invokes CollectError, which consumes the error string - // before ~COMServiceExecutionContext runs. + // N.B. THROW_IF_FAILED is required (not RETURN_IF_FAILED) so WIL's error + // callback fires CollectError before ~COMServiceExecutionContext runs. THROW_IF_FAILED(session->Initialize(Settings, Vm, PluginNotifier)); // Create the service session ref. It extracts metadata and a weak reference from the session. diff --git a/src/windows/wslcsession/WSLCSessionFactory.h b/src/windows/wslcsession/WSLCSessionFactory.h index df95c928a..e5fe60b88 100644 --- a/src/windows/wslcsession/WSLCSessionFactory.h +++ b/src/windows/wslcsession/WSLCSessionFactory.h @@ -52,8 +52,7 @@ class DECLSPEC_UUID("9FCD2067-9FC6-4EFA-9EB0-698169EBF7D3") WSLCSessionFactory IFACEMETHOD(GetProcessHandle)(_Out_ HANDLE* ProcessHandle) override; - // ISupportErrorInfo: enables IErrorInfo marshaling across COM process boundaries so that - // user-facing error messages set via THROW_HR_WITH_USER_ERROR propagate to the caller. + // ISupportErrorInfo: enables IErrorInfo marshaling across COM boundaries. IFACEMETHOD(InterfaceSupportsErrorInfo)(_In_ REFIID riid) override; private: diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 61457b14d..1cdde5159 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -452,39 +452,26 @@ class WSLCTests VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), E_INVALIDARG); } - // Marker-file scenarios use unique temp paths so the test never touches shared default storage. - auto uniqueTempStoragePath = [] { - auto path = std::filesystem::temp_directory_path() / - std::format(L"wslc-test-storage-{}-{}", GetCurrentProcessId(), GetTickCount64()); - std::filesystem::create_directories(path); - return path; - }; - - auto expectMarkerRejection = [&](LPCWSTR name, auto&& populateDir) { - const auto storagePath = uniqueTempStoragePath(); + // Reject non-empty storage directories that don't already contain a session VHD + // (would otherwise risk overwriting unrelated user files). + { + const auto storagePath = std::filesystem::temp_directory_path() / + std::format(L"wslc-test-storage-{}-{}", GetCurrentProcessId(), GetTickCount64()); + std::filesystem::create_directories(storagePath); auto cleanup = wil::scope_exit([&]() { std::error_code ignored; std::filesystem::remove_all(storagePath, ignored); }); - populateDir(storagePath); + std::ofstream{storagePath / L"userfile.txt"} << "data"; - auto settings = GetDefaultSessionSettings(name); + auto settings = GetDefaultSessionSettings(L"storage-not-empty"); const auto storagePathString = storagePath.wstring(); settings.StoragePath = storagePathString.c_str(); wil::com_ptr session; VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), E_INVALIDARG); ValidateCOMErrorMessage(std::format(L"Cannot use '{}' as session storage because the directory is not empty", storagePathString)); - }; - - // Reject non-empty storage directory without the wslcsession marker. - expectMarkerRejection( - L"storage-not-empty", [](const std::filesystem::path& dir) { std::ofstream{dir / L"userfile.txt"} << "data"; }); - - // Reject directory whose marker name is occupied by a sub-directory. - expectMarkerRejection(L"marker-is-directory", [](const std::filesystem::path& dir) { - std::filesystem::create_directory(dir / L"wslcsession"); - }); + } } struct VmInfo diff --git a/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp b/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp index c8bcf9263..c04c4b767 100644 --- a/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp @@ -115,28 +115,5 @@ class WSLCE2ESessionEnterTests .ExitCode = 1, }); } - - WSLC_TEST_METHOD(WSLCE2E_SessionEnter_MarkerFileValidation) - { - // The non-empty-rejection case is covered by WSLCTests::CreateSessionValidation against a - // temp directory; reproducing it here would require mutating the shared default storage. - const auto storagePath = GetDefaultStoragePath(); - const auto markerPath = storagePath / L"wslcsession"; - - auto createSessionAndExpectMarker = [&]() { - auto terminateOnExit = wil::scope_exit([&]() { RunWslc(L"system session terminate"); }); - RunWslc(L"image ls").Verify({.ExitCode = 0}); - VERIFY_IS_TRUE(std::filesystem::exists(markerPath)); - }; - - // Default session creation writes the marker file. - createSessionAndExpectMarker(); - - // Legacy directory auto-upgrade: removing the marker then re-entering recreates it. - std::error_code ec; - VERIFY_IS_TRUE(std::filesystem::remove(markerPath, ec)); - VERIFY_IS_FALSE(std::filesystem::exists(markerPath)); - createSessionAndExpectMarker(); - } }; } // namespace WSLCE2ETests From 8c998866c8b765e8c4699fad5369858c90547341 Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Tue, 2 Jun 2026 21:12:15 -0700 Subject: [PATCH 6/9] Use error_code overload for creating storage directories --- src/windows/wslcsession/WSLCSession.cpp | 3 ++- test/windows/WSLCTests.cpp | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 46bb702c6..7b929ca5d 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -319,7 +319,8 @@ try } else { - std::filesystem::create_directories(storagePath); + std::filesystem::create_directories(storagePath, ec); + THROW_IF_WIN32_ERROR_MSG(ec.value(), "create_directories failed for %ls", storagePath.c_str()); } } } diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 1cdde5159..68761edd8 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -452,8 +452,7 @@ class WSLCTests VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), E_INVALIDARG); } - // Reject non-empty storage directories that don't already contain a session VHD - // (would otherwise risk overwriting unrelated user files). + // Reject non-empty storage directory that doesn't contain a session VHD. { const auto storagePath = std::filesystem::temp_directory_path() / std::format(L"wslc-test-storage-{}-{}", GetCurrentProcessId(), GetTickCount64()); From b3b908bb49aa6f8b687b60560b41d9c0b11d7b19 Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Fri, 5 Jun 2026 08:43:48 -0700 Subject: [PATCH 7/9] Validate that session storage path is a directory, not a file --- localization/strings/en-US/Resources.resw | 4 ++++ src/windows/wslcsession/WSLCSession.cpp | 6 ++++++ test/windows/WSLCTests.cpp | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 4f5554cef..643e466d1 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2365,6 +2365,10 @@ For privacy information about this product please visit https://aka.ms/privacy.< Cannot use '{}' as session storage because the directory is not empty {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + + Cannot use '{}' as session storage because the path is not a directory + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + Invalid image tag format: '{}'. Expected format is 'name:tag' {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 9b0ddc464..58b83ae68 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -326,6 +326,12 @@ try } else { + const bool pathExists = std::filesystem::exists(storagePath, ec); + throwOnRealFsError(ec, storagePath, "exists"); + + THROW_HR_WITH_USER_ERROR_IF( + E_INVALIDARG, Localization::MessageWslcSessionStorageNotADirectory(storagePath.c_str()), pathExists); + std::filesystem::create_directories(storagePath, ec); THROW_IF_WIN32_ERROR_MSG(ec.value(), "create_directories failed for %ls", storagePath.c_str()); } diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 477a030ce..6dc5025ad 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -471,6 +471,24 @@ class WSLCTests VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, nullptr, &session), E_INVALIDARG); ValidateCOMErrorMessage(std::format(L"Cannot use '{}' as session storage because the directory is not empty", storagePathString)); } + + // Reject storage path that points to an existing file (not a directory). + { + const auto storagePath = std::filesystem::temp_directory_path() / + std::format(L"wslc-test-storage-file-{}-{}", GetCurrentProcessId(), GetTickCount64()); + std::ofstream{storagePath} << "data"; + auto cleanup = wil::scope_exit([&]() { + std::error_code ignored; + std::filesystem::remove(storagePath, ignored); + }); + + auto settings = GetDefaultSessionSettings(L"storage-is-file"); + const auto storagePathString = storagePath.wstring(); + settings.StoragePath = storagePathString.c_str(); + wil::com_ptr session; + VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, nullptr, &session), E_INVALIDARG); + ValidateCOMErrorMessage(std::format(L"Cannot use '{}' as session storage because the path is not a directory", storagePathString)); + } } struct VmInfo From 61a3c6b8af45045c3f36149cc5b9db455c2a9a22 Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Fri, 5 Jun 2026 08:45:08 -0700 Subject: [PATCH 8/9] Format --- src/windows/wslcsession/WSLCSession.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 58b83ae68..6af3b987f 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -329,8 +329,7 @@ try const bool pathExists = std::filesystem::exists(storagePath, ec); throwOnRealFsError(ec, storagePath, "exists"); - THROW_HR_WITH_USER_ERROR_IF( - E_INVALIDARG, Localization::MessageWslcSessionStorageNotADirectory(storagePath.c_str()), pathExists); + THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageNotADirectory(storagePath.c_str()), pathExists); std::filesystem::create_directories(storagePath, ec); THROW_IF_WIN32_ERROR_MSG(ec.value(), "create_directories failed for %ls", storagePath.c_str()); From 45b2731139a718a7587ca64ec220c29f323c74a4 Mon Sep 17 00:00:00 2001 From: Beena Chauhan Date: Fri, 5 Jun 2026 10:47:44 -0700 Subject: [PATCH 9/9] Restore defensive create_directories call before VHD creation --- src/windows/wslcsession/WSLCSession.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 6af3b987f..f2d017950 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -441,6 +441,7 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID // If the VHD wasn't found, create it. WSL_LOG("CreateStorageVhd", TraceLoggingValue(m_storageVhdPath.c_str(), "StorageVhdPath")); + std::filesystem::create_directories(storagePath); wsl::core::filesystem::CreateVhd(m_storageVhdPath.c_str(), Settings.MaximumStorageSizeMb * _1MB, UserSid, false, false); vhdCreated = true;