From 184c1913c848c094d34482854906f402c8131634 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 15 Apr 2026 14:25:06 -0700 Subject: [PATCH 1/4] Image save optional output --- localization/strings/en-US/Resources.resw | 3 ++ .../wslc/commands/ImageSaveCommand.cpp | 2 +- src/windows/wslc/services/ImageService.cpp | 9 +++- src/windows/wslc/services/ImageService.h | 1 + src/windows/wslc/tasks/ImageTasks.cpp | 20 +++++++-- .../wslc/e2e/WSLCE2EImageSaveTests.cpp | 41 +++++++++++++++--- test/windows/wslc/e2e/WSLCExecutor.cpp | 42 +++++++++++++++++++ test/windows/wslc/e2e/WSLCExecutor.h | 1 + 8 files changed, 107 insertions(+), 12 deletions(-) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 3607f1f0c..ceea86e2b 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2554,4 +2554,7 @@ On first run, creates the file with all settings commented out at their defaults Requested load but no input provided. + + Cannot write image to terminal. Use the -o flag or redirect stdout. + diff --git a/src/windows/wslc/commands/ImageSaveCommand.cpp b/src/windows/wslc/commands/ImageSaveCommand.cpp index f4056c894..5159bab6d 100644 --- a/src/windows/wslc/commands/ImageSaveCommand.cpp +++ b/src/windows/wslc/commands/ImageSaveCommand.cpp @@ -28,7 +28,7 @@ std::vector ImageSaveCommand::GetArguments() const { return { Argument::Create(ArgType::ImageId, true), - Argument::Create(ArgType::Output, true), + Argument::Create(ArgType::Output), Argument::Create(ArgType::Session), }; } diff --git a/src/windows/wslc/services/ImageService.cpp b/src/windows/wslc/services/ImageService.cpp index 34d4aadd6..0baf96dd8 100644 --- a/src/windows/wslc/services/ImageService.cpp +++ b/src/windows/wslc/services/ImageService.cpp @@ -233,9 +233,14 @@ void ImageService::Save(wsl::windows::wslc::models::Session& session, const std: CreateFileW(output.c_str(), GENERIC_WRITE, FILE_SHARE_READ, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; THROW_LAST_ERROR_IF(!outputFile); + Save(session, image, outputFile.get(), cancelEvent); +} + +void ImageService::Save(wsl::windows::wslc::models::Session& session, const std::string& image, HANDLE outputHandle, HANDLE cancelEvent) +{ wsl::windows::common::HandleConsoleProgressBar progressBar( - outputFile.get(), L"Save in progress.", wsl::windows::common::HandleConsoleProgressBar::Format::FileSize); - THROW_IF_FAILED(session.Get()->SaveImage(ToCOMInputHandle(outputFile.get()), image.c_str(), nullptr, cancelEvent)); + outputHandle, L"Save in progress.", wsl::windows::common::HandleConsoleProgressBar::Format::FileSize); + THROW_IF_FAILED(session.Get()->SaveImage(ToCOMInputHandle(outputHandle), image.c_str(), nullptr, cancelEvent)); } void ImageService::Prune() diff --git a/src/windows/wslc/services/ImageService.h b/src/windows/wslc/services/ImageService.h index 4d97b55e2..1d1e9e0e8 100644 --- a/src/windows/wslc/services/ImageService.h +++ b/src/windows/wslc/services/ImageService.h @@ -37,6 +37,7 @@ class ImageService static wsl::windows::common::wslc_schema::InspectImage Inspect(wsl::windows::wslc::models::Session& session, const std::string& image); static void Pull(wsl::windows::wslc::models::Session& session, const std::string& image, IProgressCallback* callback); static void Save(wsl::windows::wslc::models::Session& session, const std::string& image, const std::wstring& output, HANDLE cancelEvent = nullptr); + static void Save(wsl::windows::wslc::models::Session& session, const std::string& image, HANDLE outputHandle, HANDLE cancelEvent = nullptr); static void Tag(wsl::windows::wslc::models::Session& session, const std::string& sourceImage, const std::string& targetImage); void Push(); void Prune(); diff --git a/src/windows/wslc/tasks/ImageTasks.cpp b/src/windows/wslc/tasks/ImageTasks.cpp index 461e00589..33d662b00 100644 --- a/src/windows/wslc/tasks/ImageTasks.cpp +++ b/src/windows/wslc/tasks/ImageTasks.cpp @@ -23,6 +23,7 @@ Module Name: #include "TableOutput.h" #include "Task.h" #include +#include using namespace wsl::shared; using namespace wsl::windows::common::string; @@ -190,11 +191,24 @@ void SaveImage(CLIExecutionContext& context) { WI_ASSERT(context.Data.Contains(Data::Session)); WI_ASSERT(context.Args.Contains(ArgType::ImageId)); - WI_ASSERT(context.Args.Contains(ArgType::Output)); auto& session = context.Data.Get(); auto& imageId = context.Args.Get(); - auto& output = context.Args.Get(); - services::ImageService::Save(session, WideToMultiByte(imageId), output, context.CreateCancelEvent()); + + if (context.Args.Contains(ArgType::Output)) + { + auto& output = context.Args.Get(); + services::ImageService::Save(session, WideToMultiByte(imageId), output, context.CreateCancelEvent()); + } + else + { + auto stdoutHandle = GetStdHandle(STD_OUTPUT_HANDLE); + if (wsl::windows::common::wslutil::IsConsoleHandle(stdoutHandle)) + { + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_ImageSaveStdoutIsTerminalError()); + } + + services::ImageService::Save(session, WideToMultiByte(imageId), stdoutHandle, context.CreateCancelEvent()); + } } void TagImage(CLIExecutionContext& context) diff --git a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp index 99d6d1c9c..a8e9ff4d0 100644 --- a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp @@ -54,12 +54,6 @@ class WSLCE2EImageSaveTests result.Verify({.Stdout = GetHelpMessage(), .Stderr = L"Required argument not provided: 'image'\r\n", .ExitCode = 1}); } - WSLC_TEST_METHOD(WSLCE2E_Image_Save_MissingOutputPath) - { - const auto result = RunWslc(std::format(L"image save {}", DebianImage.NameAndTag())); - result.Verify({.Stderr = L"Required argument not provided: 'output'\r\n", .ExitCode = 1}); - } - WSLC_TEST_METHOD(WSLCE2E_Image_Save_ImageNotFound) { const auto result = RunWslc(std::format(L"image save --output \"{}\" {}", SavedArchivePath.wstring(), InvalidImage.NameAndTag())); @@ -95,6 +89,41 @@ class WSLCE2EImageSaveTests runResult.Verify({.Stdout = L"Hello from saved image!\n", .Stderr = L"", .ExitCode = 0}); } + WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Success) + { + const auto result = RunWslcAndRedirectToFile(format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath); + result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); + + VERIFY_IS_TRUE(std::filesystem::exists(SavedArchivePath)); + auto sourceFileSize = std::filesystem::file_size(DebianImage.Path); + auto archiveFileSize = std::filesystem::file_size(SavedArchivePath); + VERIFY_ARE_EQUAL(sourceFileSize, archiveFileSize); + } + + WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToTerminal_Fail) + { + const auto result = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag())); + result.Verify({.Stderr = L"Cannot write image to terminal. Use the -o flag or redirect stdout.\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); + } + + WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Load) + { + // Save source image + auto saveResult = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath.wstring()); + saveResult.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); + + // Delete source image + EnsureImageIsDeleted(DebianImage); + + // Load from saved archive + auto loadResult = RunWslc(std::format(L"image load --input \"{}\"", SavedArchivePath.wstring())); + loadResult.Verify({.Stderr = L"", .ExitCode = 0}); + + // Run a container from the loaded image to verify it works + auto runResult = RunWslc(std::format(L"container run --rm {} echo Hello from saved image!", DebianImage.NameAndTag())); + runResult.Verify({.Stdout = L"Hello from saved image!\n", .Stderr = L"", .ExitCode = 0}); + } + private: const TestImage DebianImage = DebianTestImage(); const TestImage& InvalidImage = InvalidTestImage(); diff --git a/test/windows/wslc/e2e/WSLCExecutor.cpp b/test/windows/wslc/e2e/WSLCExecutor.cpp index 3472d4eba..21e580555 100644 --- a/test/windows/wslc/e2e/WSLCExecutor.cpp +++ b/test/windows/wslc/e2e/WSLCExecutor.cpp @@ -163,6 +163,48 @@ void RunWslcAndVerify(const std::wstring& cmd, const WSLCExecutionResult& expect RunWslc(cmd, elevationType).Verify(expected); } +WSLCExecutionResult RunWslcAndRedirectToFile(const std::wstring& commandLine, std::optional outputPath, ElevationType elevationType) +{ + auto cmd = L"\"" + GetWslcPath() + L"\" " + commandLine; + wsl::windows::common::SubProcess process(nullptr, cmd.c_str()); + + // If running non-elevated we need to keep the token alive until it completes. + wil::unique_handle nonElevatedToken; + if (elevationType == ElevationType::NonElevated) + { + nonElevatedToken = GetNonElevatedPrimaryToken(); + process.SetToken(nonElevatedToken.get()); + } + + auto [parentStderrRead, childStderrWrite] = wsl::windows::common::wslutil::OpenAnonymousPipe(0, true, false); + THROW_IF_WIN32_BOOL_FALSE(SetHandleInformation(childStderrWrite.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)); + + wil::unique_hfile redirectedStdout; + HANDLE stdoutHandle = GetStdHandle(STD_OUTPUT_HANDLE); + + std::wstring effectiveCommandLine = commandLine; + if (outputPath.has_value()) + { + SECURITY_ATTRIBUTES securityAttributes{}; + securityAttributes.nLength = sizeof(securityAttributes); + securityAttributes.bInheritHandle = TRUE; + redirectedStdout.reset(CreateFileW(outputPath->c_str(), GENERIC_WRITE, FILE_SHARE_READ, &securityAttributes, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)); + THROW_LAST_ERROR_IF(!redirectedStdout); + stdoutHandle = redirectedStdout.get(); + effectiveCommandLine = std::format(L"{} > {}", commandLine, outputPath->wstring()); + } + + process.SetStdHandles(nullptr, stdoutHandle, childStderrWrite.get()); + + const auto processHandle = process.Start(); + childStderrWrite.reset(); + + const auto exitCode = wsl::windows::common::SubProcess::GetExitCode(processHandle.get()); + const auto stdErrOutput = wsl::shared::string::MultiByteToWide(ReadToString(parentStderrRead.get())); + + return {.CommandLine = std::move(effectiveCommandLine), .Stdout = L"", .Stderr = stdErrOutput, .ExitCode = exitCode}; +} + std::wstring GetWslcHeader() { std::wstringstream header; diff --git a/test/windows/wslc/e2e/WSLCExecutor.h b/test/windows/wslc/e2e/WSLCExecutor.h index 5f1612f71..575f2657a 100644 --- a/test/windows/wslc/e2e/WSLCExecutor.h +++ b/test/windows/wslc/e2e/WSLCExecutor.h @@ -94,6 +94,7 @@ struct WSLCInteractiveSession }; WSLCExecutionResult RunWslc(const std::wstring& commandLine, ElevationType elevationType = ElevationType::Elevated); +WSLCExecutionResult RunWslcAndRedirectToFile(const std::wstring& commandLine, std::optional outputPath = std::nullopt, ElevationType elevationType = ElevationType::Elevated); void RunWslcAndVerify(const std::wstring& cmd, const WSLCExecutionResult& expected, ElevationType elevationType = ElevationType::Elevated); std::wstring GetWslcHeader(); From 6395742927fd87b68c46dd05c5c4abbb3241278f Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 15 Apr 2026 14:25:54 -0700 Subject: [PATCH 2/4] Clang format --- test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp | 3 ++- test/windows/wslc/e2e/WSLCExecutor.cpp | 3 ++- test/windows/wslc/e2e/WSLCExecutor.h | 5 ++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp index a8e9ff4d0..4d24c5f09 100644 --- a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp @@ -103,7 +103,8 @@ class WSLCE2EImageSaveTests WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToTerminal_Fail) { const auto result = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag())); - result.Verify({.Stderr = L"Cannot write image to terminal. Use the -o flag or redirect stdout.\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); + result.Verify( + {.Stderr = L"Cannot write image to terminal. Use the -o flag or redirect stdout.\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); } WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Load) diff --git a/test/windows/wslc/e2e/WSLCExecutor.cpp b/test/windows/wslc/e2e/WSLCExecutor.cpp index 21e580555..759495838 100644 --- a/test/windows/wslc/e2e/WSLCExecutor.cpp +++ b/test/windows/wslc/e2e/WSLCExecutor.cpp @@ -188,7 +188,8 @@ WSLCExecutionResult RunWslcAndRedirectToFile(const std::wstring& commandLine, st SECURITY_ATTRIBUTES securityAttributes{}; securityAttributes.nLength = sizeof(securityAttributes); securityAttributes.bInheritHandle = TRUE; - redirectedStdout.reset(CreateFileW(outputPath->c_str(), GENERIC_WRITE, FILE_SHARE_READ, &securityAttributes, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)); + redirectedStdout.reset(CreateFileW( + outputPath->c_str(), GENERIC_WRITE, FILE_SHARE_READ, &securityAttributes, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)); THROW_LAST_ERROR_IF(!redirectedStdout); stdoutHandle = redirectedStdout.get(); effectiveCommandLine = std::format(L"{} > {}", commandLine, outputPath->wstring()); diff --git a/test/windows/wslc/e2e/WSLCExecutor.h b/test/windows/wslc/e2e/WSLCExecutor.h index 575f2657a..c7a4fccba 100644 --- a/test/windows/wslc/e2e/WSLCExecutor.h +++ b/test/windows/wslc/e2e/WSLCExecutor.h @@ -94,7 +94,10 @@ struct WSLCInteractiveSession }; WSLCExecutionResult RunWslc(const std::wstring& commandLine, ElevationType elevationType = ElevationType::Elevated); -WSLCExecutionResult RunWslcAndRedirectToFile(const std::wstring& commandLine, std::optional outputPath = std::nullopt, ElevationType elevationType = ElevationType::Elevated); +WSLCExecutionResult RunWslcAndRedirectToFile( + const std::wstring& commandLine, + std::optional outputPath = std::nullopt, + ElevationType elevationType = ElevationType::Elevated); void RunWslcAndVerify(const std::wstring& cmd, const WSLCExecutionResult& expected, ElevationType elevationType = ElevationType::Elevated); std::wstring GetWslcHeader(); From 9ff15e26fe188f62660f8d769489d2fe71de644e Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 15 Apr 2026 14:57:03 -0700 Subject: [PATCH 3/4] Update test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp index 4d24c5f09..065bf618d 100644 --- a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp @@ -91,7 +91,7 @@ class WSLCE2EImageSaveTests WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Success) { - const auto result = RunWslcAndRedirectToFile(format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath); + const auto result = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath); result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); VERIFY_IS_TRUE(std::filesystem::exists(SavedArchivePath)); From cb72befcb8c9266445857f86cb0ad29ba2e3bcbe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:59:26 +0000 Subject: [PATCH 4/4] Fix terminal detection in CI for WSLCE2E_Image_Save_ToTerminal_Fail test - Open CONOUT$ as child stdout in RunWslcAndRedirectToFile when no output path is given, so IsConsoleHandle() returns true in the child process regardless of how CI redirects the test runner's stdout - Fix SavedArchivePath.wstring() -> SavedArchivePath in WSLCE2E_Image_Save_ToStdout_Load - Quote output path in effectiveCommandLine for diagnostic clarity Agent-Logs-Url: https://github.com/microsoft/WSL/sessions/f94556e1-9acd-40f1-89bd-349e238e92f3 Co-authored-by: AmelBawa-msft <104940545+AmelBawa-msft@users.noreply.github.com> --- .../windows/wslc/e2e/WSLCE2EImageSaveTests.cpp | 2 +- test/windows/wslc/e2e/WSLCExecutor.cpp | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp index 065bf618d..361f493a6 100644 --- a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp @@ -110,7 +110,7 @@ class WSLCE2EImageSaveTests WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Load) { // Save source image - auto saveResult = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath.wstring()); + auto saveResult = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath); saveResult.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); // Delete source image diff --git a/test/windows/wslc/e2e/WSLCExecutor.cpp b/test/windows/wslc/e2e/WSLCExecutor.cpp index 759495838..5711713ae 100644 --- a/test/windows/wslc/e2e/WSLCExecutor.cpp +++ b/test/windows/wslc/e2e/WSLCExecutor.cpp @@ -180,7 +180,7 @@ WSLCExecutionResult RunWslcAndRedirectToFile(const std::wstring& commandLine, st THROW_IF_WIN32_BOOL_FALSE(SetHandleInformation(childStderrWrite.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)); wil::unique_hfile redirectedStdout; - HANDLE stdoutHandle = GetStdHandle(STD_OUTPUT_HANDLE); + HANDLE stdoutHandle = nullptr; std::wstring effectiveCommandLine = commandLine; if (outputPath.has_value()) @@ -192,7 +192,21 @@ WSLCExecutionResult RunWslcAndRedirectToFile(const std::wstring& commandLine, st outputPath->c_str(), GENERIC_WRITE, FILE_SHARE_READ, &securityAttributes, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)); THROW_LAST_ERROR_IF(!redirectedStdout); stdoutHandle = redirectedStdout.get(); - effectiveCommandLine = std::format(L"{} > {}", commandLine, outputPath->wstring()); + effectiveCommandLine = std::format(L"{} > \"{}\"", commandLine, outputPath->wstring()); + } + else + { + // Open CONOUT$ so the child process receives a real console handle regardless of + // how the test runner has configured its own stdout (e.g. piped in CI). This + // makes IsConsoleHandle() return true inside wslc, which is the condition under + // test in WSLCE2E_Image_Save_ToTerminal_Fail. + SECURITY_ATTRIBUTES securityAttributes{}; + securityAttributes.nLength = sizeof(securityAttributes); + securityAttributes.bInheritHandle = TRUE; + redirectedStdout.reset( + CreateFileW(L"CONOUT$", GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, &securityAttributes, OPEN_EXISTING, 0, nullptr)); + THROW_LAST_ERROR_IF(!redirectedStdout); + stdoutHandle = redirectedStdout.get(); } process.SetStdHandles(nullptr, stdoutHandle, childStderrWrite.get());