diff --git a/src/EventLogExpert.Runtime/Common/Files/AtomicFileWriter.cs b/src/EventLogExpert.Runtime/Common/Files/AtomicFileWriter.cs new file mode 100644 index 00000000..bd103c41 --- /dev/null +++ b/src/EventLogExpert.Runtime/Common/Files/AtomicFileWriter.cs @@ -0,0 +1,92 @@ +// // Copyright (c) Microsoft Corporation. +// // Licensed under the MIT License. + +namespace EventLogExpert.Runtime.Common.Files; + +/// +/// Writes a file atomically: the caller's content is streamed into a fixed-name sibling temp file in the +/// destination's own directory, then committed with an atomic same-volume +/// (overwrite). Because the temp always lives on the same volume as the +/// destination, the commit is an atomic replace-by-rename (MoveFileEx MOVEFILE_REPLACE_EXISTING): the destination +/// always ends up as either its prior content or the fully written new content -- never missing or partial -- even +/// when the commit itself fails. A cancellation or a failed write leaves the destination untouched and always cleans +/// up the temp (the temp is deleted on every path that does not commit). This is pure with no +/// WinRT dependency, so it works unchanged under elevated (packaged or unpackaged) processes. +/// +public static class AtomicFileWriter +{ + public static async Task WriteAsync( + string destinationPath, + Func writeContent, + CancellationToken cancellationToken) + { + ArgumentException.ThrowIfNullOrEmpty(destinationPath); + ArgumentNullException.ThrowIfNull(writeContent); + + cancellationToken.ThrowIfCancellationRequested(); + + var directory = Path.GetDirectoryName(destinationPath); + + if (string.IsNullOrEmpty(directory)) + { + throw new ArgumentException( + "The destination path must include a directory component.", nameof(destinationPath)); + } + + // Fixed-length sibling name (independent of the destination filename) so that a near-maximum-length + // destination name cannot push the temp path past the file-system's path-component limit. + var tempPath = Path.Combine(directory, $".eventlogexpert-{Guid.NewGuid():N}.tmp"); + var committed = false; + + try + { + // Explicit braces: the temp stream MUST be disposed (its handle released) BEFORE the commit, otherwise the + // File.Move replace fails with a sharing violation. Do not collapse to `await using var`. + await using (var tempStream = new FileStream(tempPath, new FileStreamOptions + { + Mode = FileMode.CreateNew, + Access = FileAccess.Write, + Share = FileShare.None, + // Asynchronous: writeContent streams via WriteAsync, so use true overlapped I/O rather than sync I/O + // dispatched to the thread pool (matters for large exports; mirrors DebugLogFileReader's convention). + Options = FileOptions.Asynchronous, + })) + { + await writeContent(tempStream, cancellationToken); + } + + // Final cancellation gate: the last point at which a Cancel can abort the save. After this the commit is + // an uncancellable rename. + cancellationToken.ThrowIfCancellationRequested(); + + Commit(tempPath, destinationPath); + committed = true; + } + finally + { + if (!committed) { TryDelete(tempPath); } + } + } + + private static void Commit(string tempPath, string destinationPath) => + // Atomic same-volume replace-by-rename. MoveFileEx(MOVEFILE_REPLACE_EXISTING) either fully succeeds or leaves + // both files intact, so a failed commit can never destroy the existing destination. We deliberately do NOT use + // File.Replace: its underlying ReplaceFile has documented partial-failure states (e.g. + // ERROR_UNABLE_TO_MOVE_REPLACEMENT_2) that can delete the destination while orphaning the replacement under the + // temp name -- a data-loss hazard in a save path. The committed file takes the destination directory's inherited + // ACL, the correct outcome for freshly written export content (the comdlg32 Save dialog's OFN_OVERWRITEPROMPT + // already obtained the user's consent to overwrite an existing file). + File.Move(tempPath, destinationPath, overwrite: true); + + private static void TryDelete(string path) + { + try + { + File.Delete(path); + } + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) + { + // Best-effort: the temp is never the saved output, so a failed cleanup must not mask the real result. + } + } +} diff --git a/src/EventLogExpert/Adapters/FilePicker/MauiFolderPickerService.cs b/src/EventLogExpert/Adapters/FilePicker/MauiFolderPickerService.cs index 5ccd1259..cd89d736 100644 --- a/src/EventLogExpert/Adapters/FilePicker/MauiFolderPickerService.cs +++ b/src/EventLogExpert/Adapters/FilePicker/MauiFolderPickerService.cs @@ -6,8 +6,8 @@ namespace EventLogExpert.Adapters.FilePicker; -/// MAUI adapter that delegates to . internal sealed class MauiFolderPickerService : IFolderPickerService { - public Task PickFolderAsync() => WinUiFolderPicker.PickFolderAsync(); + public Task PickFolderAsync() => + MainThread.InvokeOnMainThreadAsync(() => Win32FileDialogService.PickFolderAsync("Select Folder")); } diff --git a/src/EventLogExpert/Adapters/FileSave/MauiFileSaveService.cs b/src/EventLogExpert/Adapters/FileSave/MauiFileSaveService.cs index 4d3b090c..cb1f0524 100644 --- a/src/EventLogExpert/Adapters/FileSave/MauiFileSaveService.cs +++ b/src/EventLogExpert/Adapters/FileSave/MauiFileSaveService.cs @@ -1,17 +1,14 @@ // // Copyright (c) Microsoft Corporation. // // Licensed under the MIT License. -using EventLogExpert.Platforms.Windows; using EventLogExpert.Runtime.Common.Files; -using System.Runtime.InteropServices; -using Windows.Storage; -using Windows.Storage.Pickers; -using Windows.Storage.Provider; namespace EventLogExpert.Adapters.FileSave; -public sealed class MauiFileSaveService : IFileSaveService +public sealed class MauiFileSaveService(IFilePickerService filePickerService) : IFileSaveService { + private readonly IFilePickerService _filePickerService = filePickerService; + public async Task SaveStreamingAsync( string suggestedFileName, IReadOnlyDictionary> fileTypes, @@ -24,177 +21,30 @@ public sealed class MauiFileSaveService : IFileSaveService cancellationToken.ThrowIfCancellationRequested(); - var pickedFile = await PickSaveFileAsync(suggestedFileName, fileTypes); + var extensions = FlattenExtensions(fileTypes); + var destinationPath = await _filePickerService.PickSaveAsync("Save As", extensions, suggestedFileName); - if (pickedFile is null) { return null; } + if (destinationPath is null) { return null; } - var parentFolder = await TryGetParentAsync(pickedFile); - var tempInParent = parentFolder is null - ? null - : await TryCreateTempFileAsync(parentFolder, pickedFile.Name); + await AtomicFileWriter.WriteAsync(destinationPath, writeContent, cancellationToken); - // Primary path requires write access to the picked file's folder; otherwise fall back to a local temp + copy. - return tempInParent is not null - ? await SaveByReplacingAsync(pickedFile, tempInParent, writeContent, cancellationToken) - : await SaveByLocalTempCopyAsync(pickedFile, writeContent, cancellationToken); + return destinationPath; } - private static async Task PickSaveFileAsync( - string suggestedFileName, IReadOnlyDictionary> fileTypes) + private static IReadOnlyList FlattenExtensions( + IReadOnlyDictionary> fileTypes) { if (fileTypes.Count == 0) { throw new ArgumentException("At least one file-type choice must be supplied.", nameof(fileTypes)); } - return await MainThread.InvokeOnMainThreadAsync(async () => - { - var picker = new FileSavePicker - { - SuggestedStartLocation = PickerLocationId.ComputerFolder, - SuggestedFileName = suggestedFileName - }; - - foreach ((string label, IReadOnlyList extensions) in fileTypes) - { - picker.FileTypeChoices.Add(label, [.. extensions]); - } - - PickerHostWindow.Initialize(picker); - - return await picker.PickSaveFileAsync(); - }); - } - - private static async Task SaveByLocalTempCopyAsync( - StorageFile pickedFile, Func writeContent, CancellationToken cancellationToken) - { - string localTempPath = Path.Combine(Path.GetTempPath(), $"eventlogexpert-export-{Guid.NewGuid():N}.tmp"); - bool deferred = false; - bool completed = false; - FileUpdateStatus status = FileUpdateStatus.Failed; - - try - { - await using (var tempStream = File.Create(localTempPath)) - { - await writeContent(tempStream, cancellationToken); - } - - cancellationToken.ThrowIfCancellationRequested(); - - var localTempFile = await StorageFile.GetFileFromPathAsync(localTempPath); - - CachedFileManager.DeferUpdates(pickedFile); - deferred = true; - - // Final cancellation gate before the uncancellable commit: if cancelled here, the finally resolves the - // deferred-update contract and the picked file is left untouched (CopyAndReplaceAsync never runs). - cancellationToken.ThrowIfCancellationRequested(); - - // Replace the picked file from the fully-staged temp in a single platform-mediated operation rather than - // truncating then re-filling it ourselves, which would guarantee a zero-length window on any copy fault. - // This is best-effort rather than atomic on provider-backed destinations (see IFileSaveService docs). - await localTempFile.CopyAndReplaceAsync(pickedFile); - - status = await CachedFileManager.CompleteUpdatesAsync(pickedFile); - completed = true; - } - finally - { - if (deferred && !completed) - { - try { await CachedFileManager.CompleteUpdatesAsync(pickedFile); } - catch (Exception) { /* best-effort: resolve the provider update contract on failure. */ } - } - - try { File.Delete(localTempPath); } - catch (Exception) { /* best-effort: the local temp is a copy source, never the saved output. */ } - } - - return ToResultPath(pickedFile, status); - } - - private static async Task SaveByReplacingAsync( - StorageFile pickedFile, - StorageFile tempFile, - Func writeContent, - CancellationToken cancellationToken) - { - bool deferred = false; - bool completed = false; - bool replaced = false; - FileUpdateStatus status = FileUpdateStatus.Failed; + var extensions = fileTypes.Values + .SelectMany(group => group) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList(); - try - { - CachedFileManager.DeferUpdates(pickedFile); - deferred = true; - - await using (var tempStream = await tempFile.OpenStreamForWriteAsync()) - { - tempStream.SetLength(0); - await writeContent(tempStream, cancellationToken); - } - - cancellationToken.ThrowIfCancellationRequested(); - - // Atomic commit: the picked file is replaced only after the write succeeds. - await tempFile.MoveAndReplaceAsync(pickedFile); - replaced = true; - - status = await CachedFileManager.CompleteUpdatesAsync(pickedFile); - completed = true; - } - finally - { - if (deferred && !completed) - { - try { await CachedFileManager.CompleteUpdatesAsync(pickedFile); } - catch (Exception) { /* best-effort: resolve the provider update contract on failure. */ } - } - - if (!replaced) - { - // After a successful replace the temp IS the saved output and must not be deleted. - try { await tempFile.DeleteAsync(); } - catch (Exception) { /* best-effort cleanup. */ } - } - } - - return ToResultPath(pickedFile, status); - } - - private static string ToResultPath(StorageFile pickedFile, FileUpdateStatus status) => - status is FileUpdateStatus.Complete or FileUpdateStatus.CompleteAndRenamed - ? pickedFile.Path - : throw new IOException($"Save failed with status '{status}'."); - - private static async Task TryCreateTempFileAsync(StorageFolder folder, string pickedName) - { - try - { - // The save picker may grant access only to the picked file, not its folder; fall back when creation is denied. - return await folder.CreateFileAsync($"{pickedName}.tmp", CreationCollisionOption.GenerateUniqueName); - } - catch (Exception exception) when (exception is UnauthorizedAccessException or COMException) - { - // The parent folder already resolved, so a creation failure here is an access/permission issue; fall back - // to the local-temp copy. Other (unexpected) exceptions propagate so a genuine fault is not masked by - // silently choosing the non-atomic path. - return null; - } - } - - private static async Task TryGetParentAsync(StorageFile file) - { - try - { - return await file.GetParentAsync(); - } - catch (Exception) - { - return null; - } + return extensions.Count == 0 ? + throw new ArgumentException("At least one extension must be supplied.", nameof(fileTypes)) : extensions; } } diff --git a/src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs b/src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs index e22b8225..42410ae9 100644 --- a/src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs +++ b/src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs @@ -30,7 +30,6 @@ using System.Globalization; using Application = Microsoft.Maui.Controls.Application; using IDispatcher = Fluxor.IDispatcher; -using MauiFilePicker = Microsoft.Maui.Storage.FilePicker; namespace EventLogExpert.Adapters.Menu; @@ -54,6 +53,7 @@ public sealed class MauiMenuActionService( IUpdateService updateService, ICurrentVersionProvider currentVersionProvider, ITraceLogger traceLogger, + IFilePickerService filePickerService, IFolderPickerService folderPickerService, IState eventLogState, IState logTableState, @@ -71,6 +71,7 @@ public sealed class MauiMenuActionService( private readonly IState _eventLogState = eventLogState; private readonly IEventTableExporter _eventTableExporter = eventTableExporter; private readonly IExportProgressBannerService _exportProgress = exportProgressBannerService; + private readonly IFilePickerService _filePickerService = filePickerService; private readonly IFileSaveService _fileSaveService = fileSaveService; private readonly IFilterLibraryCommands _filterLibraryCommands = filterLibraryCommands; private readonly IFilterPaneCommands _filterPaneCommands = filterPaneCommands; @@ -271,23 +272,22 @@ public Task OpenDocsAsync() => public async Task OpenFileAsync(bool combineLog) { - var options = new PickOptions - { - FileTypes = new FilePickerFileType( - new Dictionary> { { DevicePlatform.WinUI, [".evtx"] } }) - }; + IReadOnlyList paths; - var files = await MauiFilePicker.Default.PickMultipleAsync(options); + try + { + paths = await _filePickerService.PickMultipleAsync("Open Event Logs", [".evtx"]); + } + catch (InvalidOperationException ex) + { + await _dialogService.ShowAlert("Open File Failed", ex.Message, "Ok"); - if (!files.Any()) { return; } + return; + } - var paths = files - .OfType() - .Where(file => !string.IsNullOrEmpty(file.FullPath)) - .Select(file => (file.FullPath, LogPathType.File)) - .ToList(); + if (paths.Count == 0) { return; } - await OpenLogsBatchAsync(paths, combineLog); + await OpenLogsBatchAsync(paths.Select(path => (path, LogPathType.File)), combineLog); } public async Task OpenFolderAsync(bool combineLog) diff --git a/src/EventLogExpert/Platforms/Windows/PickerHostWindow.cs b/src/EventLogExpert/Platforms/Windows/PickerHostWindow.cs index f1fe183a..3c74f627 100644 --- a/src/EventLogExpert/Platforms/Windows/PickerHostWindow.cs +++ b/src/EventLogExpert/Platforms/Windows/PickerHostWindow.cs @@ -1,18 +1,8 @@ // // Copyright (c) Microsoft Corporation. // // Licensed under the MIT License. -using WinRT.Interop; - namespace EventLogExpert.Platforms.Windows; -/// -/// Single source of truth for the picker-windowing dance. WinUI's FileSavePicker and FolderPicker -/// need the active MAUI host window's HWND to present correctly — they associate via -/// (COM interop that fails with 0x80004005 when -/// the process is elevated and the picker isn't bound to a window). The Win32 also -/// takes an HWND, but through the hwndOwner field of the OPENFILENAMEW struct passed to the -/// procedural comdlg32!GetOpenFileNameW / GetSaveFileNameW APIs (no COM activation). -/// internal static class PickerHostWindow { /// @@ -32,16 +22,4 @@ public static IntPtr GetHandle() return window.WindowHandle; } - - /// - /// Associates with the active MAUI host window via the WinUI - /// COM interop. Use for WinUI FileSavePicker / - /// FolderPicker; for file-open / file-save use which takes the HWND - /// directly via and passes it as the OPENFILENAMEW hwndOwner field. - /// - public static void Initialize(object picker) - { - ArgumentNullException.ThrowIfNull(picker); - InitializeWithWindow.Initialize(picker, GetHandle()); - } } diff --git a/src/EventLogExpert/Platforms/Windows/Win32FileDialogService.cs b/src/EventLogExpert/Platforms/Windows/Win32FileDialogService.cs index e4f5ec6b..9f85659a 100644 --- a/src/EventLogExpert/Platforms/Windows/Win32FileDialogService.cs +++ b/src/EventLogExpert/Platforms/Windows/Win32FileDialogService.cs @@ -3,25 +3,6 @@ namespace EventLogExpert.Platforms.Windows; -/// -/// Presents the Win32 file dialogs for open (single + multi) and save scenarios. Delegates to -/// , which uses the procedural comdlg32!GetOpenFileNameW / -/// GetSaveFileNameW APIs (NO COM activation) rather than WinUI's FileOpenPicker / FileSavePicker -/// or the shell IFileOpenDialog COM class — both of which throw COMException 0x80004005 under elevated -/// processes (even with InitializeWithWindow applied) or REGDB_E_CLASSNOTREG under MSIX-packaged -/// elevated processes. Returns the selected file path(s), or null / an empty list when the user cancelled. -/// -/// -/// The dialogs MUST run on a dedicated thread. Although the procedural -/// GetOpenFileNameW / GetSaveFileNameW APIs don't require COM activation, they DO require an STA -/// apartment so the modern explorer-style dialog (OFN_EXPLORER) can host the shell common-controls that depend -/// on STA-only OLE drag-and-drop, IDataObject marshalling, and the explorer view's own threading model. Running them -/// on the MAUI (the dispatcher queue thread) is unreliable because its apartment state is -/// not guaranteed to be STA under elevated WinUI hosts. Creating our own STA thread via + -/// gives the dialog a clean, well-defined apartment to present -/// from. The dialog APIs pump their own message loop until dismissed, so the STA thread doesn't need a -/// Dispatcher.Run set up around it. -/// internal static class Win32FileDialogService { public static Task PickAsync(IReadOnlyList extensions, string? title = null) @@ -33,6 +14,13 @@ internal static class Win32FileDialogService return RunOnStaThreadAsync(() => Win32FileDialog.PickSingleFile(hwnd, extensions, title)); } + public static Task PickFolderAsync(string? title = null) + { + var hwnd = PickerHostWindow.GetHandle(); + + return RunOnStaThreadAsync(() => Win32FolderDialog.PickFolder(hwnd, title)); + } + public static Task> PickMultipleAsync(IReadOnlyList extensions, string? title = null) { ArgumentNullException.ThrowIfNull(extensions); diff --git a/src/EventLogExpert/Platforms/Windows/Win32FolderDialog.cs b/src/EventLogExpert/Platforms/Windows/Win32FolderDialog.cs new file mode 100644 index 00000000..fddddf72 --- /dev/null +++ b/src/EventLogExpert/Platforms/Windows/Win32FolderDialog.cs @@ -0,0 +1,159 @@ +// // Copyright (c) Microsoft Corporation. +// // Licensed under the MIT License. + +using System.Runtime.InteropServices; + +namespace EventLogExpert.Platforms.Windows; + +internal static partial class Win32FolderDialog +{ + private const uint BIF_EDITBOX = 0x00000010; + private const uint BIF_NEWDIALOGSTYLE = 0x00000040; + private const uint BIF_RETURNONLYFSDIRS = 0x00000001; + // MAX_PATH: SHBrowseForFolder writes the selected folder's display NAME (not path) into pszDisplayName and takes no + // length parameter, assuming a MAX_PATH buffer. A real >= MAX_PATH buffer is unambiguously overrun-safe. + private const int DisplayNameChars = 260; + private const int GPFIDL_DEFAULT = 0x00000000; + + // 256 chars bounds the stack allocation against pathologically long external title input. + private const int MaxTitleChars = 256; + + // 32K char = 64 KB: covers the documented Windows absolute maximum path length, mirroring Win32FileDialog and the + // native extension's SHGetPathFromIDListEx buffer -- avoids the MAX_PATH ceiling GPFIDL_DEFAULT otherwise escapes. + private const int PathBufferChars = 32 * 1024; + + /// + /// Presents the folder dialog. Returns the selected folder's file-system path, or null only when the user + /// cancelled. Throws when OLE cannot be initialized or when a selected item + /// cannot be resolved to a file-system path (so a genuine failure is never silently reported as a cancel). + /// + public static string? PickFolder(IntPtr hwndOwner, string? title = null) + { + // BIF_NEWDIALOGSTYLE requires the calling STA thread to be OLE-initialized. The runtime already CoInitializes + // the dedicated STA worker as apartment-threaded, so OleInitialize typically returns S_FALSE (COM already + // initialized) while still layering OLE on top. Throw on any failure (including RPC_E_CHANGED_MODE, which + // cannot occur on the fresh guaranteed-STA thread) so OleUninitialize is only ever called after a success. + var oleResult = OleInitialize(IntPtr.Zero); + + if (oleResult < 0) + { + throw new InvalidOperationException( + $"Could not initialize OLE to present the folder dialog (HRESULT 0x{oleResult:X8})."); + } + + try + { + return PickFolderCore(hwndOwner, title); + } + finally + { + OleUninitialize(); + } + } + + /// + /// Builds (or measures, when is empty) the null-terminated title buffer. Returns + /// 0 when is null or empty (the dialog uses no caption prompt in that case -- pass + /// IntPtr.Zero for lpszTitle). Titles longer than are silently truncated to bound + /// the stack allocation against external input (mirror of the helper in ). + /// + private static int CopyNullableTitle(Span destination, string? title) + { + if (string.IsNullOrEmpty(title)) { return 0; } + + var titleLen = Math.Min(title.Length, MaxTitleChars); + var needed = titleLen + 1; + + if (destination.IsEmpty) { return needed; } + + title.AsSpan(0, titleLen).CopyTo(destination); + destination[titleLen] = '\0'; + + return needed; + } + + [LibraryImport("ole32.dll")] + private static partial int OleInitialize(IntPtr pvReserved); + + [LibraryImport("ole32.dll")] + private static partial void OleUninitialize(); + + private static unsafe string? PickFolderCore(IntPtr hwndOwner, string? title) + { + Span displayNameBuffer = stackalloc char[DisplayNameChars]; + displayNameBuffer.Clear(); + + Span titleBuffer = stackalloc char[CopyNullableTitle(default, title)]; + CopyNullableTitle(titleBuffer, title); + + fixed (char* displayNamePtr = displayNameBuffer) + { + fixed (char* titlePtr = titleBuffer) + { + var browseInfo = new BrowseInfo + { + hwndOwner = hwndOwner, + pidlRoot = IntPtr.Zero, + pszDisplayName = (IntPtr)displayNamePtr, + lpszTitle = titleBuffer.IsEmpty ? IntPtr.Zero : (IntPtr)titlePtr, + ulFlags = BIF_RETURNONLYFSDIRS | BIF_NEWDIALOGSTYLE | BIF_EDITBOX, + lpfn = IntPtr.Zero, + lParam = IntPtr.Zero, + iImage = 0 + }; + + var pidl = SHBrowseForFolder(ref browseInfo); + + if (pidl == IntPtr.Zero) { return null; } // user cancelled + + try + { + Span pathBuffer = stackalloc char[PathBufferChars]; + pathBuffer.Clear(); + + fixed (char* pathPtr = pathBuffer) + { + if (!SHGetPathFromIDListEx(pidl, (IntPtr)pathPtr, PathBufferChars, GPFIDL_DEFAULT)) + { + throw new InvalidOperationException( + "The selected item is not a file-system folder."); + } + + var path = new string(pathPtr); + + return string.IsNullOrEmpty(path) + ? throw new InvalidOperationException( + "The selected folder could not be resolved to a file-system path.") + : path; + } + } + finally + { + // The PIDL is allocated by the shell task allocator (identical to the COM task allocator), so + // Marshal.FreeCoTaskMem is the correct release -- matching the native extension's CoTaskMemFree. + Marshal.FreeCoTaskMem(pidl); + } + } + } + } + + [LibraryImport("shell32.dll", EntryPoint = "SHBrowseForFolderW")] + private static partial IntPtr SHBrowseForFolder(ref BrowseInfo browseInfo); + + [LibraryImport("shell32.dll", EntryPoint = "SHGetPathFromIDListEx")] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool SHGetPathFromIDListEx(IntPtr pidl, IntPtr pszPath, uint cchPath, int uOpts); + + [StructLayout(LayoutKind.Sequential)] + private struct BrowseInfo + { + public IntPtr hwndOwner; + public IntPtr pidlRoot; + public IntPtr pszDisplayName; + public IntPtr lpszTitle; + public uint ulFlags; + public IntPtr lpfn; + public IntPtr lParam; + public int iImage; + } +} diff --git a/src/EventLogExpert/Platforms/Windows/WinUiFolderPicker.cs b/src/EventLogExpert/Platforms/Windows/WinUiFolderPicker.cs deleted file mode 100644 index e24f9d34..00000000 --- a/src/EventLogExpert/Platforms/Windows/WinUiFolderPicker.cs +++ /dev/null @@ -1,30 +0,0 @@ -// // Copyright (c) Microsoft Corporation. -// // Licensed under the MIT License. - -using Windows.Storage; -using Windows.Storage.Pickers; - -namespace EventLogExpert.Platforms.Windows; - -internal static class WinUiFolderPicker -{ - /// - /// Presents the WinUI folder picker. Returns the selected folder's path, or null only when the user - /// cancelled. Throws when no MAUI host window is available so callers can - /// surface the broken-host condition instead of silently treating it as a cancel. - /// - public static async Task PickFolderAsync() - { - FolderPicker picker = new() - { - SuggestedStartLocation = PickerLocationId.Desktop, - FileTypeFilter = { "*" } // Add a wildcard to allow folder selection - }; - - PickerHostWindow.Initialize(picker); - - StorageFolder? folder = await picker.PickSingleFolderAsync(); - - return folder?.Path; - } -} diff --git a/tests/Unit/EventLogExpert.Runtime.Tests/Common/Files/AtomicFileWriterTests.cs b/tests/Unit/EventLogExpert.Runtime.Tests/Common/Files/AtomicFileWriterTests.cs new file mode 100644 index 00000000..c1726d7e --- /dev/null +++ b/tests/Unit/EventLogExpert.Runtime.Tests/Common/Files/AtomicFileWriterTests.cs @@ -0,0 +1,136 @@ +// // Copyright (c) Microsoft Corporation. +// // Licensed under the MIT License. + +using EventLogExpert.Runtime.Common.Files; +using System.Text; + +namespace EventLogExpert.Runtime.Tests.Common.Files; + +public sealed class AtomicFileWriterTests : IDisposable +{ + private readonly string _directory; + + public AtomicFileWriterTests() + { + _directory = Path.Combine(Path.GetTempPath(), $"ele-atomic-{Guid.NewGuid():N}"); + Directory.CreateDirectory(_directory); + } + + public void Dispose() + { + try { Directory.Delete(_directory, recursive: true); } + catch (IOException) { /* best-effort test cleanup. */ } + catch (UnauthorizedAccessException) { /* best-effort test cleanup. */ } + } + + [Fact] + public async Task WriteAsync_CancelledAfterWrite_LeavesExistingDestinationUntouchedAndDeletesTemp() + { + var destination = Destination(); + await File.WriteAllTextAsync(destination, "original", TestContext.Current.CancellationToken); + using var cancellation = new CancellationTokenSource(); + + // Cancel from inside writeContent (after the content is written) so the post-write gate throws before commit. + Func writeThenCancel = async (stream, token) => + { + await stream.WriteAsync(Encoding.UTF8.GetBytes("new"), token); + await cancellation.CancelAsync(); + }; + + await Assert.ThrowsAnyAsync( + () => AtomicFileWriter.WriteAsync(destination, writeThenCancel, cancellation.Token)); + + Assert.Equal("original", await File.ReadAllTextAsync(destination, TestContext.Current.CancellationToken)); + Assert.Empty(LeakedTempFiles()); + } + + [Fact] + public async Task WriteAsync_CancelledBeforeWrite_ThrowsAndCreatesNoFile() + { + var destination = Destination(); + using var cancellation = new CancellationTokenSource(); + await cancellation.CancelAsync(); + + await Assert.ThrowsAnyAsync( + () => AtomicFileWriter.WriteAsync(destination, WriteText("x"), cancellation.Token)); + + Assert.False(File.Exists(destination)); + Assert.Empty(LeakedTempFiles()); + } + + [Fact] + public async Task WriteAsync_EmptyDestination_ThrowsArgumentException() => + await Assert.ThrowsAnyAsync( + () => AtomicFileWriter.WriteAsync("", WriteText("x"), TestContext.Current.CancellationToken)); + + [Fact] + public async Task WriteAsync_ExistingDestination_ReplacesContent() + { + var destination = Destination(); + await File.WriteAllTextAsync(destination, "original", TestContext.Current.CancellationToken); + + await AtomicFileWriter.WriteAsync(destination, WriteText("replaced"), TestContext.Current.CancellationToken); + + Assert.Equal("replaced", await File.ReadAllTextAsync(destination, TestContext.Current.CancellationToken)); + Assert.Empty(LeakedTempFiles()); + } + + [Fact] + public async Task WriteAsync_ExistingLongerDestination_IsFullyReplacedByShorterContent() + { + // The commit is a whole-file atomic rename, so a shorter payload fully supersedes a longer original with no + // trailing bytes (unlike a naive truncate-then-write). + var destination = Destination(); + await File.WriteAllTextAsync( + destination, "a considerably longer original content string", TestContext.Current.CancellationToken); + + await AtomicFileWriter.WriteAsync(destination, WriteText("short"), TestContext.Current.CancellationToken); + + Assert.Equal("short", await File.ReadAllTextAsync(destination, TestContext.Current.CancellationToken)); + Assert.Empty(LeakedTempFiles()); + } + + [Fact] + public async Task WriteAsync_NewDestination_CreatesFileWithContent() + { + var destination = Destination(); + + await AtomicFileWriter.WriteAsync(destination, WriteText("hello"), TestContext.Current.CancellationToken); + + Assert.True(File.Exists(destination)); + Assert.Equal("hello", await File.ReadAllTextAsync(destination, TestContext.Current.CancellationToken)); + Assert.Empty(LeakedTempFiles()); + } + + [Fact] + public async Task WriteAsync_NoDirectoryComponent_ThrowsArgumentException() => + await Assert.ThrowsAnyAsync( + () => AtomicFileWriter.WriteAsync("barefilename.log", WriteText("x"), TestContext.Current.CancellationToken)); + + [Fact] + public async Task WriteAsync_NullWriteContent_ThrowsArgumentNullException() => + await Assert.ThrowsAsync( + () => AtomicFileWriter.WriteAsync(Destination(), null!, TestContext.Current.CancellationToken)); + + [Fact] + public async Task WriteAsync_WriteContentThrows_LeavesExistingDestinationUntouchedAndDeletesTemp() + { + var destination = Destination(); + await File.WriteAllTextAsync(destination, "original", TestContext.Current.CancellationToken); + + Func throwing = (_, _) => throw new InvalidOperationException("boom"); + + await Assert.ThrowsAsync( + () => AtomicFileWriter.WriteAsync(destination, throwing, TestContext.Current.CancellationToken)); + + Assert.Equal("original", await File.ReadAllTextAsync(destination, TestContext.Current.CancellationToken)); + Assert.Empty(LeakedTempFiles()); + } + + private static Func WriteText(string text) => + (stream, token) => stream.WriteAsync(Encoding.UTF8.GetBytes(text), token).AsTask(); + + private string Destination(string name = "output.log") => Path.Combine(_directory, name); + + private IReadOnlyList LeakedTempFiles() => Directory.GetFiles(_directory, "*.tmp"); +} diff --git a/tests/Unit/EventLogExpert.Windows.Tests/PickerSafetyGuardTests.cs b/tests/Unit/EventLogExpert.Windows.Tests/PickerSafetyGuardTests.cs new file mode 100644 index 00000000..ca884941 --- /dev/null +++ b/tests/Unit/EventLogExpert.Windows.Tests/PickerSafetyGuardTests.cs @@ -0,0 +1,99 @@ +// // Copyright (c) Microsoft Corporation. +// // Licensed under the MIT License. + +using Xunit; + +namespace EventLogExpert.Windows.Tests; + +public sealed class PickerSafetyGuardTests +{ + private static readonly string[] s_forbiddenTokens = + [ + "Windows.Storage.Pickers", // WinRT FileOpenPicker / FileSavePicker / FolderPicker namespace + "FilePicker.Default", // MAUI Microsoft.Maui.Storage.FilePicker default instance + "Microsoft.Maui.Storage.FilePicker" // MAUI file picker (fully-qualified reference or using-alias target) + ]; + + // C#, Razor, and XAML sources are scanned: a picker API could be reintroduced via a .razor @using / @code block or + // a .xaml reference, not only in .cs files. + private static readonly HashSet s_scannedExtensions = + new(StringComparer.OrdinalIgnoreCase) { ".cs", ".razor", ".xaml" }; + + [Fact] + public void SourceTree_DoesNotReintroduceElevationUnsafePickers() + { + var sourceRoot = ResolveRepoRelativeDirectory("src"); + var offenders = new List(); + + foreach (var file in EnumerateScannedFiles(sourceRoot)) + { + var content = File.ReadAllText(file); + + foreach (var token in s_forbiddenTokens) + { + if (content.Contains(token, StringComparison.Ordinal)) + { + offenders.Add($"{token} -> {file}"); + } + } + } + + Assert.True( + offenders.Count == 0, + "Elevation-unsafe picker API(s) found in source. Route file/folder picking through IFilePickerService / " + + "IFolderPickerService (procedural comdlg32 / SHBrowseForFolder) instead of the WinRT/MAUI pickers:" + + Environment.NewLine + string.Join(Environment.NewLine, offenders)); + } + + private static IEnumerable EnumerateScannedFiles(string root) + { + var pending = new Stack(); + pending.Push(root); + + while (pending.Count > 0) + { + var directory = pending.Pop(); + + foreach (var file in Directory.EnumerateFiles(directory)) + { + if (s_scannedExtensions.Contains(Path.GetExtension(file))) + { + yield return file; + } + } + + foreach (var subdirectory in Directory.EnumerateDirectories(directory)) + { + var name = Path.GetFileName(subdirectory); + + // Skip obj/bin build outputs, and skip reparse points (directory junctions / symlinks) so a link + // targeting an ancestor cannot cycle the walk. + if (name.Equals("obj", StringComparison.OrdinalIgnoreCase) + || name.Equals("bin", StringComparison.OrdinalIgnoreCase) + || (File.GetAttributes(subdirectory) & FileAttributes.ReparsePoint) != 0) + { + continue; + } + + pending.Push(subdirectory); + } + } + } + + private static string ResolveRepoRelativeDirectory(params string[] segments) + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "EventLogExpert.slnx"))) + { + directory = directory.Parent; + } + + Assert.NotNull(directory); + + var combined = Path.Combine([directory.FullName, .. segments]); + Assert.True(Directory.Exists(combined), $"Expected directory at {combined} to exist."); + + return combined; + } +}