Feature: Re-enable the Preview Pane - #16
Closed
0x5bfa wants to merge 2 commits into
Closed
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It touches high-risk Windows Shell/COM preview hosting, process/token isolation, and UI lifecycle paths that require careful manual validation on real Windows environments.
Pull request overview
This PR re-enables the Preview Pane end-to-end in the WinUI app and restores the Windows Shell preview-handler pipeline, adding policy/registration validation, low-integrity activation, and stronger lifecycle/stream handling to better match Explorer behavior.
Changes:
- Re-enable Preview Pane UI wiring (root view, pane view-model creation, runtime storage previews) and add a user-facing “Preview anyway” flow for untrusted items.
- Rework Windows Shell preview hosting to add activation-time policy revalidation, registration allowlisting/validation, low-integrity local-server activation, and improved host visuals/layout updates.
- Add/expand unit + UI tests covering handler registration, preview access policy, stream ownership/leasing, and stale-selection / retry behavior.
File summaries
| File | Description |
|---|---|
| tests/Files.UnitTests/WindowsShellPreviewTests.cs | Expands Shell session tests for activation policy, ordering, revalidation, and target/handler validation. |
| tests/Files.UnitTests/WindowsPreviewHandlerRegistrationTests.cs | Adds tests for association parsing and registration allowlist behavior. |
| tests/Files.UnitTests/WindowsPreviewAccessPolicyTests.cs | Adds tests for Windows trust/hydration/size/enterprise gating and retry binding. |
| tests/Files.UnitTests/StreamPreviewResultTests.cs | Adds tests for new stream lease/disposal behavior. |
| tests/Files.UnitTests/BrowsePreviewModelTests.cs | Adds tests for disposal failures, untrusted retry snapshot binding, and activation-time block reporting. |
| tests/Files.UITests/PresentationTests/PreviewPaneViewModelTests.cs | Adds tests ensuring queued UI updates retain streams and validate exact-snapshot retry behavior. |
| src/Files/Views/RootView.xaml.cs | Re-wires PreviewPaneView session factory/window attachment/disposal and hides preview pane in Settings view. |
| src/Files/Views/RootView.xaml | Re-enables PreviewPane column + control in the layout. |
| src/Files/Views/PreviewPane.xaml.cs | Restores preview rendering lifecycle; adds “Preview anyway”, focus transfer, layout updates without reloading, and image safety limits. |
| src/Files/Views/PreviewPane.xaml | Adds a button for untrusted-preview retry and adjusts status text wrapping constraints. |
| src/Files/ViewModels/PreviewPaneViewModel.cs | Retains stream leases across dispatcher boundaries and surfaces untrusted-retry + activation-block reporting APIs. |
| src/Files/ViewModels/PaneViewModel.cs | Re-enables creation and disposal of the preview pane view-model. |
| src/Files/Strings/en-US/Resources.resw | Adds localized strings for untrusted preview messaging and “Preview anyway”. |
| src/Files/App.xaml.cs | Re-enables Windows storage previews at runtime construction. |
| src/Files.Core/NativeMethods.txt | Adds Win32/COM/security APIs needed for policy gates, visuals, and low-integrity activation. |
| src/Files.Core/Composition/WindowsFilesCoreBuilderExtensions.cs | Switches to WindowsPreviewAccessPolicy default and wires session factory with policy. |
| src/Files.Core/Capabilities/Previews/WindowsShellPreviewSessionFactory.cs | Adds activation-time target context requirement, policy revalidation, and handler-registration validation. |
| src/Files.Core/Capabilities/Previews/WindowsShellPreviewResult.cs | Carries the PreviewRequest through Shell preview results for revalidation and retry binding. |
| src/Files.Core/Capabilities/Previews/WindowsShellPreviewLoader.cs | Updates loader to use async request-aware policy gate and preserve request on Shell results. |
| src/Files.Core/Capabilities/Previews/WindowsShellPreviewHandlerController.cs | Enforces cloaked local-server activation, adds low-integrity impersonation path, improves init failure handling, and applies system visuals. |
| src/Files.Core/Capabilities/Previews/WindowsShellPreviewHandlerAssociation.cs | Hardens handler association lookup with bounded allocations and injectable query for tests. |
| src/Files.Core/Capabilities/Previews/WindowsShellPreviewBlockedException.cs | Introduces a dedicated exception for activation-time policy blocks. |
| src/Files.Core/Capabilities/Previews/WindowsPreviewTargetResolver.cs | Ensures resolved preview targets carry ItemContext for activation-time policy checks. |
| src/Files.Core/Capabilities/Previews/WindowsPreviewTarget.cs | Adds optional ItemContext to preview targets and validates it matches the owned item. |
| src/Files.Core/Capabilities/Previews/WindowsPreviewHandlerResolver.cs | Adds registration allowlisting and moves extension normalization to a reusable helper. |
| src/Files.Core/Capabilities/Previews/WindowsPreviewHandlerRegistrationValidator.cs | Adds activation-time validator to ensure handler association/registration still matches before activation. |
| src/Files.Core/Capabilities/Previews/WindowsPreviewHandlerRegistrationAllowlist.cs | Adds per-user + machine allowlist based on PreviewHandlers registry keys. |
| src/Files.Core/Capabilities/Previews/WindowsPreviewHandlerActivationPolicy.cs | Adds EnableCloaking activation context and updates default local-server policy. |
| src/Files.Core/Capabilities/Previews/WindowsPreviewAccessPolicy.cs | Implements Windows-specific hydration/size/trust/enterprise gating for stream + Shell previews with retry authorization. |
| src/Files.Core/Capabilities/Previews/StreamPreviewResult.cs | Adds lease-based stream lifetime management and disposal coordination. |
| src/Files.Core/Capabilities/Previews/PreviewRequest.cs | Adds internal trust-authorization binding to requests to support exact-target retry semantics. |
| src/Files.Core/Capabilities/Previews/PreviewBlockReason.cs | Adds explicit enum values and introduces Untrusted block reason. |
| src/Files.Core/Capabilities/Previews/IWindowsShellPreviewPolicy.cs | Adds request-aware sync/async policy hooks while preserving compatibility default behavior. |
| src/Files.Core/Capabilities/Previews/IWindowsPreviewTargetResolver.cs | Updates contract documentation to require context for activation-time revalidation. |
| src/Files.Core/Capabilities/Previews/IWindowsPreviewHandlerRegistrationAllowlist.cs | Adds an interface for handler registration allowlisting. |
| src/Files.Core/Capabilities/Previews/IWindowsPreviewHandlerController.cs | Adds optional system-visuals application hook to the controller interface. |
| src/Files.Core/Capabilities/Previews/AllowWindowsShellPreviewPolicy.cs | Updates allow policy to support request-aware async evaluation. |
| src/Files.Core/Browsing/IBrowsePreviewModel.cs | Adds default interface methods for untrusted retry and activation-block reporting. |
| src/Files.Core/Browsing/BrowsePreviewModel.cs | Implements exact-snapshot untrusted retry binding, activation-block publishing, and hardens disposal of prior results. |
Review details
Suppressed comments (4)
tests/Files.UnitTests/WindowsShellPreviewTests.cs:251
- This constructor call is split across lines even though it fits comfortably on one line; the repo’s C# formatting rules prefer keeping invocations on a single line unless the line would exceed 200 characters.
tests/Files.UnitTests/WindowsShellPreviewTests.cs:385 - This constructor call is split across lines even though it fits comfortably on one line; the repo’s C# formatting rules prefer keeping invocations on a single line unless the line would exceed 200 characters.
tests/Files.UnitTests/WindowsShellPreviewTests.cs:270 - This constructor call is split across lines even though it fits comfortably on one line; the repo’s C# formatting rules prefer keeping invocations on a single line unless the line would exceed 200 characters.
tests/Files.UnitTests/WindowsShellPreviewTests.cs:319 - This constructor call is split across lines even though it fits comfortably on one line; the repo’s C# formatting rules prefer keeping invocations on a single line unless the line would exceed 200 characters.
- Files reviewed: 39/39 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+212
to
+213
| var factory = new WindowsShellPreviewSessionFactory( | ||
| new FakeTargetResolver(target), scheduler, new FakeControllerFactory(controller), AllowWindowsShellPreviewPolicy.Instance, AllowHandlerRegistrationValidator.Instance); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolved / Related Issues
Summary
DisableLowILProcessIsolationusing the same registry-view fallback behavior.ZoneCheckUrlExCacheWpath through the private SHLWAPI ordinal 233, interpret its raw policy exactly, and fail closed. If the ordinal is unavailable, fall back through the file's zone ADS and then public URLMON policy evaluation.MSGhandler-to-host forwarding, known-host HWND normalization, bounded ordered Tab/F6 processing, five-secondQueryFocustimeout, child-focus validation, and keyboard focus cues.IPreviewHandler::TranslateAcceleratordowncall. By contract, that method is called only by the preview handler process message pump; unhandled input returns throughIPreviewHandlerFrame::TranslateAccelerator.Zone.Identifiercoverage for local, Internet-zone, and unblocked files across stream and Shell preview policy paths.Explorer compatibility notes
The implementation was compared with the recovered x64
shell32.dllpreview path (file version 10.0.26100.9168) and the installed 10.0.26100.9278 build. SHLWAPI ordinal 233 was confirmed on x64 and x86; ARM64 was not directly recovered, so the runtime fallback remains necessary.Explorer's additional WDAG/HVSI trust gate uses private, deprecated Application Guard components and is not reproduced here. The Preview Pane also retains Files-specific stream renderers, size/hydration limits, and UI, so this PR does not claim complete Explorer UI parity.
Steps used to test these changes
src/Files.Core/Files.Core.csprojfor Debug x64.src/Files/Files.csprojfor Debug x64.tests/Files.UnitTests/Files.UnitTests.csprojfor Debug x64: 269 passed, 0 failed, 0 skipped.tests/Files.UITests/Files.UITests.csprojfor Debug x64.git diff --check.