Skip to content

Route all file and folder pickers through elevation-safe Win32 dialogs#664

Merged
jschick04 merged 1 commit into
mainfrom
jschick/fix-elevated-pickers
Jul 23, 2026
Merged

Route all file and folder pickers through elevation-safe Win32 dialogs#664
jschick04 merged 1 commit into
mainfrom
jschick/fix-elevated-pickers

Conversation

@jschick04

Copy link
Copy Markdown
Collaborator

Summary

Fixes #661. When EventLogExpert runs elevated, the WinRT/WinUI file and folder pickers throw COMException 0x80004005 (the broker-activation path is unavailable to elevated callers), so File > Open, File > Open Folder, and every Save failed. This routes every remaining picker through the procedural Win32 dialogs the codebase already uses for elevation-safe file open/save, so they work for standard-user, unpackaged-elevated, and MSIX-packaged-elevated processes alike.

Changes

  • File > Open (OpenFileAsync) now uses the injected IFilePickerService (comdlg32 GetOpenFileNameW) instead of MauiFilePicker.Default (WinUI FileOpenPicker).
  • File save (MauiFileSaveService) picks via IFilePickerService.PickSaveAsync (comdlg32 GetSaveFileNameW) and commits through a new, testable AtomicFileWriter (temp-in-parent + atomic File.Move(overwrite)), replacing the WinRT FileSavePicker + StorageFile/CachedFileManager broker machinery.
  • Folder open (OpenFolderAsync + scenario launch) uses a new Win32FolderDialog (procedural shell32!SHBrowseForFolderW + SHGetPathFromIDListEx on an STA thread), replacing the WinRT FolderPicker.
  • Removed the now-dead WinUiFolderPicker and PickerHostWindow.Initialize (WinRT InitializeWithWindow).
  • Added PickerSafetyGuardTests, a source-scan guard that fails the build if any WinRT/MAUI picker API is reintroduced.

Testing

  • New AtomicFileWriterTests (9): new-file create, replace-existing, whole-file replace of a longer original, cancel-before/after-write, writeContent-throws cleanup, and argument validation.
  • New PickerSafetyGuardTests (1): asserts no Windows.Storage.Pickers / FilePicker.Default / Microsoft.Maui.Storage.FilePicker reference remains under src/.
  • Build 0 warnings / 0 errors; Runtime.Tests 2008/2008, Windows.Tests 47/47.
  • Native interop (Win32FolderDialog) and the host adapters are not unit-testable (real modal dialogs / WinRT-coupled host), consistent with the existing Win32FileDialog; owed a manual elevated packaged x64 + ARM64 acceptance pass (File > Open, File > Open Folder, scenario launch, and each Save: export CSV/JSON, debug log, database-tools log).

Notes

  • The atomic save uses File.Move(overwrite: true) (MoveFileEx MOVEFILE_REPLACE_EXISTING) rather than File.Replace, whose underlying ReplaceFile has documented partial-failure states that can delete the destination while orphaning the replacement, a data-loss hazard in a save path. The design and post-code review panels converged unanimously.

Copilot AI review requested due to automatic review settings July 23, 2026 14:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Routes all remaining file/folder picker flows away from WinRT/WinUI brokered pickers (which fail under elevated execution) and onto elevation-safe procedural Win32 dialogs, while also making file saves atomic and testable. This aligns with the app’s needs when run as Administrator (per #661) and reduces reliance on broker activation.

Changes:

  • Switches “Open File” to use IFilePickerService (Win32 comdlg32 open dialogs) instead of Microsoft.Maui.Storage.FilePicker.
  • Reworks file saving to pick destinations via IFilePickerService.PickSaveAsync and commit content via the new AtomicFileWriter (temp-in-parent + atomic File.Move(overwrite: true)).
  • Replaces the WinRT folder picker with a new Win32FolderDialog (SHBrowseForFolderW + SHGetPathFromIDListEx) and adds a source-scan unit test guard against reintroducing unsafe picker APIs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/Unit/EventLogExpert.Windows.Tests/PickerSafetyGuardTests.cs Adds a source-scan guard test preventing reintroduction of WinRT/MAUI picker APIs under src/.
tests/Unit/EventLogExpert.Runtime.Tests/Common/Files/AtomicFileWriterTests.cs Adds unit coverage for atomic write semantics, cancellation, and cleanup behavior.
src/EventLogExpert/Platforms/Windows/WinUiFolderPicker.cs Removes WinRT FolderPicker implementation now replaced by Win32 dialog.
src/EventLogExpert/Platforms/Windows/Win32FolderDialog.cs Introduces elevation-safe Win32 folder picker implementation.
src/EventLogExpert/Platforms/Windows/Win32FileDialogService.cs Adds PickFolderAsync and routes folder selection through STA-threaded Win32 dialog execution.
src/EventLogExpert/Platforms/Windows/PickerHostWindow.cs Removes WinRT InitializeWithWindow helper, keeping only HWND acquisition.
src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs Routes File > Open through IFilePickerService with improved error surfacing.
src/EventLogExpert/Adapters/FileSave/MauiFileSaveService.cs Switches save flow to Win32 destination picking + AtomicFileWriter commit.
src/EventLogExpert/Adapters/FilePicker/MauiFolderPickerService.cs Routes folder picking through Win32 folder dialog via the Win32 service.
src/EventLogExpert.Runtime/Common/Files/AtomicFileWriter.cs Adds a reusable atomic file writer abstraction for save paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/EventLogExpert.Runtime/Common/Files/AtomicFileWriter.cs Outdated
Copilot AI review requested due to automatic review settings July 23, 2026 15:06
@jschick04
jschick04 force-pushed the jschick/fix-elevated-pickers branch from d60726f to 37b19a0 Compare July 23, 2026 15:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment thread src/EventLogExpert.Runtime/Common/Files/AtomicFileWriter.cs Outdated
Comment thread tests/Unit/EventLogExpert.Windows.Tests/PickerSafetyGuardTests.cs Outdated
Copilot AI review requested due to automatic review settings July 23, 2026 15:31
@jschick04
jschick04 force-pushed the jschick/fix-elevated-pickers branch from 37b19a0 to 3d61bfc Compare July 23, 2026 15:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comment thread tests/Unit/EventLogExpert.Windows.Tests/PickerSafetyGuardTests.cs Outdated
Copilot AI review requested due to automatic review settings July 23, 2026 16:00
@jschick04
jschick04 force-pushed the jschick/fix-elevated-pickers branch from 3d61bfc to 8a47b77 Compare July 23, 2026 16:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

@jschick04
jschick04 marked this pull request as ready for review July 23, 2026 16:08
@jschick04
jschick04 requested a review from a team as a code owner July 23, 2026 16:08

@ryanriesMSFT ryanriesMSFT left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jschick04
jschick04 merged commit c87ddef into main Jul 23, 2026
8 checks passed
@jschick04
jschick04 deleted the jschick/fix-elevated-pickers branch July 23, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File->Open does not work when running elevated

3 participants