diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index f307e0f4e..643e466d1 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2361,6 +2361,14 @@ 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 + + + 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/service/exe/WSLCSessionManager.cpp b/src/windows/service/exe/WSLCSessionManager.cpp index fa39b5625..84cca0ca7 100644 --- a/src/windows/service/exe/WSLCSessionManager.cpp +++ b/src/windows/service/exe/WSLCSessionManager.cpp @@ -273,7 +273,18 @@ void WSLCSessionManagerImpl::CreateSession( const 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(), WarningCallback, &session, &serviceRef)); + { + const auto factoryHr = factory->CreateSession(&sessionSettings, vm.Get(), notifier.Get(), WarningCallback, &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{ @@ -591,6 +602,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 1ada37006..51b52950d 100644 --- a/src/windows/service/exe/WSLCSessionManager.h +++ b/src/windows/service/exe/WSLCSessionManager.h @@ -191,7 +191,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: @@ -208,4 +208,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 1d7b81210..f2d017950 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -34,6 +34,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; @@ -285,6 +286,57 @@ try TraceLoggingValue(m_displayName.c_str(), "DisplayName"), TraceLoggingValue(Settings->CreatorPid, "CreatorPid")); + // 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 auto vhdPath = storagePath / c_storageVhdFilename; + const bool vhdExists = std::filesystem::exists(vhdPath, ec); + throwOnRealFsError(ec, vhdPath, "exists"); + + THROW_HR_WITH_USER_ERROR_IF( + HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND), + Localization::MessageWslcSessionStorageNotFound(Settings->StoragePath), + !vhdExists && WI_IsFlagSet(Settings->StorageFlags, WSLCSessionStorageFlagsNoCreate)); + + // 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 + { + 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()); + } + } + } + // Create the VM. m_virtualMachine.emplace(Vm, Settings, m_sessionTerminatingEvent.get()); @@ -356,10 +408,9 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID return; } + // Storage path validation is done in Initialize() before any VM resources are created. 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"; + m_storageVhdPath = storagePath / c_storageVhdFilename; std::string diskDevice; std::optional diskLun{}; @@ -388,15 +439,10 @@ 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)); - // 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; diff --git a/src/windows/wslcsession/WSLCSessionFactory.cpp b/src/windows/wslcsession/WSLCSessionFactory.cpp index 3c9502fc0..e90ea205b 100644 --- a/src/windows/wslcsession/WSLCSessionFactory.cpp +++ b/src/windows/wslcsession/WSLCSessionFactory.cpp @@ -19,9 +19,7 @@ Module Name: #include "WSLCSessionFactory.h" #include "WSLCSession.h" #include "WSLCSessionReference.h" -#include "wslutil.h" -namespace wslutil = wsl::windows::common::wslutil; namespace wslc = wsl::windows::service::wslc; void wslc::WSLCSessionFactory::SetDestructionCallback(std::function&& callback) @@ -68,6 +66,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 ed98bdba7..fbfc6ffe8 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); @@ -53,6 +53,9 @@ class DECLSPEC_UUID("9FCD2067-9FC6-4EFA-9EB0-698169EBF7D3") WSLCSessionFactory IFACEMETHOD(GetProcessHandle)(_Out_ HANDLE* ProcessHandle) override; + // ISupportErrorInfo: enables IErrorInfo marshaling across COM boundaries. + IFACEMETHOD(InterfaceSupportsErrorInfo)(_In_ REFIID riid) override; + private: std::function m_destructionCallback; }; diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index ac98ce4df..6dc5025ad 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -451,6 +451,44 @@ class WSLCTests wil::com_ptr session; VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, nullptr, &session), E_INVALIDARG); } + + // 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()); + std::filesystem::create_directories(storagePath); + auto cleanup = wil::scope_exit([&]() { + std::error_code ignored; + std::filesystem::remove_all(storagePath, ignored); + }); + + std::ofstream{storagePath / L"userfile.txt"} << "data"; + + 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, 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 diff --git a/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp b/test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp index c10f43295..c04c4b767 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, }); }