From 69a600e249cf3398571317a792a9fc047efef9a5 Mon Sep 17 00:00:00 2001 From: st-gr <38470677+st-gr@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:12:04 -0700 Subject: [PATCH] feat(File Explorer): Add configurable local image rendering to Markdown previewer (#47857) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Adds a "Show local images" toggle in PowerToys Settings (File Explorer > Markdown) - When enabled, renders images referenced via relative paths or local file paths in the Markdown preview pane - Serves validated local image files on a WebView2 virtual host (`https://localmdimages/`) directly from the handler's resource filter - Supports local paths and UNC/network share paths - Default: OFF (preserves existing behavior) - GPO support: Admins can force-enable or force-disable via Group Policy Fixes #40787 Fixes #3713 ## Security model | Scenario | Behavior | |----------|----------| | Setting OFF (default) | All images blocked, info bar shown. Raw HTML `src` is rewritten to `#` in this state too, so a `data:` image cannot render (it is resolved internally and never reaches the resource filter) | | Setting ON + relative path (`media/img.png`) | Resolved against .md directory, rendered if under that tree | | Setting ON + path traversal (`../../secret.png`) | Blocked — resolved with `Path.GetFullPath` and checked with `Path.GetRelativePath`, including percent-encoded traversal on the serving side | | Setting ON + junction/symlink below the allowed path | Blocked — each component of the resolved path is rejected if it carries `FileAttributes.ReparsePoint`, since lexical containment alone does not prevent redirection | | Setting ON + UNC relative path (`images/pic.png` on `\\server\share`) | Allowed within share root | | Setting ON + remote URL (`https://evil.com/track.png`) | Always blocked | | data:/javascript: URI | Always blocked, in both setting states | | `srcset` on a raw HTML `` | Attribute removed, in both setting states — its candidates are not validated by the `src` sanitizer | | Script execution | Always disabled (`IsScriptEnabled = false`) | | Mark-of-the-Web (MotW) | Explorer blocks preview of MotW-tagged files before our code runs (OS-level protection) | ## GPO Policy - Policy name: `MarkdownAllowLocalImages` - Registry: `HKLM\SOFTWARE\Policies\PowerToys\MarkdownAllowLocalImages` (DWORD: 1=enabled, 0=disabled) - ADMX category: **PowerToys > File Explorer Preview** - Uses `getConfiguredValue()` (individual module setting pattern, no global utility fallback) ## Screenshots ### Settings UI — new toggle _"Show local images" toggle nested under the Markdown preview section (File Explorer add-ons). Captured from a Debug build of this branch (the Settings app only runs standalone in Debug builds):_ 07-settings-ui-toggle ### Settings UI — locked by GPO _With the `MarkdownAllowLocalImages` policy set to Disabled, the toggle is forced Off and grayed out, and the "managed by your organization" info bar appears:_ 08-settings-ui-gpo-locked ### GPO in Group Policy Editor _New "File Explorer Preview" category under PowerToys, showing the policy and its description:_ 01-gpedit-category ### GPO set to Enabled _Policy enabled state in gpedit.msc:_ 02-gpedit-policy-enabled ### Preview with local images rendered _Markdown preview with local image rendering enabled — relative path image renders:_ 03-preview-images-shown ### Info bar for blocked remote images _When the document contains remote (http/https) image URLs, they are always blocked and an info bar is shown:_ 04-preview-infobar ### GPO disabled — all images blocked _With GPO set to disabled, all images (local and remote) are blocked. Info bar reads "Some pictures have been blocked...":_ 05-gpo-disabled-blocked ### Mark-of-the-Web protection _Files copied from a network source carry a Zone Identifier (MotW). Explorer blocks the preview entirely before our code runs — an OS-level security layer:_ 06-MotW-tagged ## Implementation Two layers were blocking images: 1. **Markdig AST layer** (`HTMLParsingExtension.cs`): replaced image URLs with `#` 2. **WebView2 layer** (`MarkdownPreviewHandlerControl.cs`): returned HTTP 403 for all non-HTML requests Changes: - `HTMLParsingExtension`: conditionally resolves markdown `![](path)` images to virtual host URLs with path traversal protection - `MarkdownHelper`: regex-rewrites relative `src=""` in raw HTML `` tags to virtual host URLs - `MarkdownPreviewHandlerControl`: serves `https://localmdimages/` requests in the `WebResourceRequested` handler — the URL is resolved back to a file path, re-validated for containment against the allowed base path (document directory, or share root for UNC), and the bytes are returned via `CreateWebResourceResponse` with the proper content type. Note: `SetVirtualHostNameToFolderMapping` is deliberately NOT used for images — WebView2 Runtime 150+ no longer serves files from UNC/network folder mappings (verified by A/B test on 150.0.4078.48); serving from the handler works uniformly for local and UNC paths - Settings UI: new toggle nested under the Markdown preview expander, with GPO lock support - Handler `Settings.cs`: reads `EnableMdLocalImages` via `SettingsUtils`, GPO override via `GPOWrapper` - GPO: `gpo.h` individual module setting, ADMX/ADML with `FileExplorerPreview` category ## Known limitation Peek also renders Markdown through `FilePreviewCommon.MarkdownHelper`, but calls it without the local-images arguments, so **Peek does not show local images even when the setting is enabled** — it keeps the existing behavior of blocking every image. With the setting on, the same file therefore renders differently in the preview pane (images shown) and in Peek (images blocked). This is deliberate for now: wiring the setting through Peek means changing a module that is otherwise untouched by this PR. Verified that Peek itself is unaffected — it still renders Markdown correctly against the shared assembly, with images blocked as before. ## Test plan - [x] Toggle OFF: images blocked, "pictures blocked" info bar shows (existing behavior) - [x] Toggle ON with relative paths: `![](images/test.png)` renders - [x] Toggle ON with HTML img: `` renders - [x] Toggle ON with path traversal: `![](../../secret.png)` — blocked - [x] Toggle ON with remote URL: `![](https://...)` — blocked, info bar shown - [x] UNC path: preview works on `\\server\share\...\file.md` with relative images - [x] UNC path with `../` within share: allowed (resolves within share root) - [x] Regression tested on WebView2 Runtime 150.0.4078.48: local + UNC images render, blocked cases (traversal, data:, remote, encoded traversal) stay blocked - [x] GPO Enabled: images forced on - [x] GPO Disabled: images forced off - [x] GPO Not Configured: user controls toggle - [x] gpedit.msc: policy appears under PowerToys > File Explorer Preview - [x] MotW-tagged files: Explorer blocks preview before our code runs - [x] Settings UI toggle locked when GPO configured (grayed out for both forced states) - [x] Other preview handlers (Monaco, SVG, PDF) unaffected 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- .github/actions/spell-check/expect.txt | 1 + .../FilePreviewCommon/HTMLParsingExtension.cs | 211 +++++++++++- .../FilePreviewCommon/MarkdownHelper.cs | 59 +++- src/common/GPOWrapper/GPOWrapper.cpp | 4 + src/common/GPOWrapper/GPOWrapper.h | 1 + src/common/GPOWrapper/GPOWrapper.idl | 1 + src/common/utils/gpo.h | 6 + src/gpo/assets/PowerToys.admx | 14 + src/gpo/assets/en-US/PowerToys.adml | 13 + .../MarkdownPreviewHandler.csproj | 1 + .../MarkdownPreviewHandlerControl.cs | 97 +++++- .../Properties/Resources.Designer.cs | 9 + .../Properties/Resources.resx | 4 + .../MarkdownPreviewHandler/Settings.cs | 32 ++ .../TestFiles/GPO-TESTING.md | 60 ++++ .../TestFiles/images/test.png | Bin 0 -> 6287 bytes .../TestFiles/test-local-images.md | 46 +++ .../HTMLParsingExtensionTest.cs | 324 ++++++++++++++++++ .../PowerPreviewProperties.cs | 17 + .../SettingsXAML/Views/PowerPreviewPage.xaml | 31 +- .../Settings.UI/Strings/en-us/Resources.resw | 6 + .../ViewModels/PowerPreviewViewModel.cs | 46 +++ 22 files changed, 967 insertions(+), 16 deletions(-) create mode 100644 src/modules/previewpane/MarkdownPreviewHandler/TestFiles/GPO-TESTING.md create mode 100644 src/modules/previewpane/MarkdownPreviewHandler/TestFiles/images/test.png create mode 100644 src/modules/previewpane/MarkdownPreviewHandler/TestFiles/test-local-images.md diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 8434397411..86129ca3ce 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -966,6 +966,7 @@ lng LOADFROMFILE LOBYTE localappdata +localmdimages localpackage LOCALSYSTEM LOCATIONCHANGE diff --git a/src/common/FilePreviewCommon/HTMLParsingExtension.cs b/src/common/FilePreviewCommon/HTMLParsingExtension.cs index b0dae4dcbe..a48ecb274a 100644 --- a/src/common/FilePreviewCommon/HTMLParsingExtension.cs +++ b/src/common/FilePreviewCommon/HTMLParsingExtension.cs @@ -2,6 +2,10 @@ // The Microsoft Corporation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; +using System.Diagnostics.CodeAnalysis; +using System.IO; + using Markdig; using Markdig.Extensions.Figures; using Markdig.Extensions.Tables; @@ -43,6 +47,199 @@ namespace Microsoft.PowerToys.FilePreviewCommon /// public string FilePath { get; set; } + /// + /// Gets or sets the base path used for path validation and relative URL computation. + /// For local files this equals FilePath. For UNC paths this is the share root. + /// + public string? AllowedBasePath { get; set; } + + /// + /// Gets or sets a value indicating whether local images should be rendered. + /// + public bool AllowLocalImages { get; set; } + + private static bool IsLocalImage([NotNullWhen(true)] string? url) + { + if (string.IsNullOrEmpty(url)) + { + return false; + } + + // Reject any URI-like scheme (http:, https:, data:, javascript:, file:, ...). + // A colon is only permitted as part of a drive path like "C:\" or "C:/". + int colonIndex = url.IndexOf(':'); + if (colonIndex >= 0) + { + bool isDrivePath = colonIndex == 1 && char.IsLetter(url[0]) && url.Length > 2 && (url[2] == '\\' || url[2] == '/'); + if (!isDrivePath) + { + return false; + } + } + + return true; + } + + /// + /// Validates that a local image URL resolves to a path inside the allowed base path and + /// computes the corresponding virtual host URL. Returns false for remote URLs, URI schemes + /// (data:, javascript:, file:, ...), path traversal outside the base path and malformed paths. + /// + /// Image URL from the markdown document. + /// Directory containing the markdown file; relative URLs resolve against it. + /// Base path the resolved path must be contained in. Falls back to if empty. + /// The rewritten virtual host URL on success. + /// True if the URL is a contained local image and was set. + public static bool TryGetLocalImageVirtualUrl(string? url, string markdownDirectory, string? allowedBasePath, [NotNullWhen(true)] out string? virtualUrl) + { + virtualUrl = null; + + if (!IsLocalImage(url)) + { + return false; + } + + try + { + string basePath = Path.GetFullPath(string.IsNullOrEmpty(allowedBasePath) ? markdownDirectory : allowedBasePath); + string resolvedPath = Path.GetFullPath(Path.Combine(markdownDirectory, url)); + string relativePath = Path.GetRelativePath(basePath, resolvedPath); + + if (relativePath == "." || relativePath == ".." || + relativePath.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || + relativePath.StartsWith(".." + Path.AltDirectorySeparatorChar, StringComparison.Ordinal) || + Path.IsPathRooted(relativePath)) + { + return false; + } + + // Reserved characters in a filename (#, %, ...) are not valid in a URL path, so + // escape it while keeping the directory separators intact. TryResolveVirtualUrl + // unescapes symmetrically when the request comes back. + string escapedPath = Uri.EscapeDataString(relativePath.Replace('\\', '/')).Replace("%2F", "/", StringComparison.OrdinalIgnoreCase); + virtualUrl = "https://localmdimages/" + escapedPath; + return true; + } + catch (ArgumentException) + { + return false; + } + catch (PathTooLongException) + { + return false; + } + catch (NotSupportedException) + { + return false; + } + } + + /// + /// Resolves a virtual host image request URL back to a file path and validates that it is + /// contained in the allowed base path. Used when serving the image bytes for a WebView2 + /// resource request. Returns false for foreign hosts, empty paths, path traversal outside + /// the base path (including percent-encoded traversal) and malformed paths. + /// + /// The request URL, expected on the localmdimages virtual host. + /// Base path the resolved file must be contained in. + /// The validated absolute file path on success. + /// True if the URL maps to a contained file path and was set. + /// Each path component is inspected for reparse points, so a path that does not + /// exist or cannot be read fails closed and returns false. + public static bool TryResolveVirtualUrl(string? requestUri, string? allowedBasePath, [NotNullWhen(true)] out string? resolvedPath) + { + resolvedPath = null; + + if (string.IsNullOrEmpty(requestUri) || string.IsNullOrEmpty(allowedBasePath)) + { + return false; + } + + try + { + var uri = new Uri(requestUri); + if (!string.Equals(uri.Host, "localmdimages", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + string relativePath = Uri.UnescapeDataString(uri.AbsolutePath).TrimStart('/').Replace('/', Path.DirectorySeparatorChar); + if (relativePath.Length == 0) + { + return false; + } + + string basePath = Path.GetFullPath(allowedBasePath); + string fullPath = Path.GetFullPath(Path.Combine(basePath, relativePath)); + string containmentCheck = Path.GetRelativePath(basePath, fullPath); + + if (containmentCheck == "." || containmentCheck == ".." || + containmentCheck.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || + containmentCheck.StartsWith(".." + Path.AltDirectorySeparatorChar, StringComparison.Ordinal) || + Path.IsPathRooted(containmentCheck)) + { + return false; + } + + // The check above proves only lexical containment. A junction or symbolic link + // anywhere below the base path can still redirect the read outside it, so walk the + // resolved path back up to the base and reject any reparse point on the way. The + // base path itself is not checked: that is the document's own location. + string baseComparand = TrimTrailingSeparators(basePath); + string current = fullPath; + while (!string.Equals(TrimTrailingSeparators(current), baseComparand, StringComparison.OrdinalIgnoreCase)) + { + if ((File.GetAttributes(current) & FileAttributes.ReparsePoint) != 0) + { + return false; + } + + string? parent = Path.GetDirectoryName(current); + if (string.IsNullOrEmpty(parent) || parent.Length >= current.Length) + { + return false; + } + + current = parent; + } + + resolvedPath = fullPath; + return true; + } + catch (ArgumentException) + { + return false; + } + catch (UriFormatException) + { + return false; + } + catch (PathTooLongException) + { + return false; + } + catch (NotSupportedException) + { + return false; + } + catch (IOException) + { + return false; + } + catch (UnauthorizedAccessException) + { + return false; + } + } + + private static string TrimTrailingSeparators(string path) + { + string trimmed = path.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + + // Keep the separator for roots such as "C:\", where trimming changes the meaning. + return trimmed.Length == 0 || trimmed.EndsWith(Path.VolumeSeparatorChar) ? path : trimmed; + } + /// public void Setup(MarkdownPipelineBuilder pipeline) { @@ -92,9 +289,17 @@ namespace Microsoft.PowerToys.FilePreviewCommon { if (link.IsImage) { - link.Url = "#"; - link.GetAttributes().AddClass("img-fluid"); - imagesBlockedCallBack(); + if (AllowLocalImages && TryGetLocalImageVirtualUrl(link.Url, FilePath, AllowedBasePath, out string? virtualUrl)) + { + link.Url = virtualUrl; + link.GetAttributes().AddClass("img-fluid"); + } + else + { + link.Url = "#"; + link.GetAttributes().AddClass("img-fluid"); + imagesBlockedCallBack(); + } } } } diff --git a/src/common/FilePreviewCommon/MarkdownHelper.cs b/src/common/FilePreviewCommon/MarkdownHelper.cs index 2003df3340..3392676b7a 100644 --- a/src/common/FilePreviewCommon/MarkdownHelper.cs +++ b/src/common/FilePreviewCommon/MarkdownHelper.cs @@ -2,7 +2,9 @@ // The Microsoft Corporation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.IO; +using System.Text.RegularExpressions; using Markdig; @@ -27,11 +29,23 @@ namespace Microsoft.PowerToys.FilePreviewCommon public static string MarkdownHtml(string fileContent, string theme, string filePath, ImagesBlockedCallBack imagesBlockedCallBack) { - var htmlHeader = theme == "dark" ? HtmlDarkHeader : HtmlLightHeader; + return MarkdownHtml(fileContent, theme, filePath, imagesBlockedCallBack, false, null); + } + + public static string MarkdownHtml(string fileContent, string theme, string filePath, ImagesBlockedCallBack imagesBlockedCallBack, bool allowLocalImages, string? allowedBasePath) + { + // Enforce the resource policy in the browser as well as in the rewriting below: regex + // cannot cover every resource-bearing construct (objects, frames, styles, malformed + // markup), so WebView2 blocks anything the sanitizers do not catch. + string imageSourcePolicy = allowLocalImages ? "https://localmdimages" : "'none'"; + string contentSecurityPolicy = $""; + var htmlHeader = (theme == "dark" ? HtmlDarkHeader : HtmlLightHeader).Insert("".Length, contentSecurityPolicy); // Extension to modify markdown AST. HTMLParsingExtension extension = new HTMLParsingExtension(imagesBlockedCallBack); extension.FilePath = Path.GetDirectoryName(filePath) ?? string.Empty; + extension.AllowedBasePath = allowedBasePath ?? extension.FilePath; + extension.AllowLocalImages = allowLocalImages; // if you have a string with double space, some people view it as a new line. // while this is against spec, even GH supports this. Technically looks like GH just trims whitespace @@ -46,6 +60,49 @@ namespace Microsoft.PowerToys.FilePreviewCommon MarkdownPipeline pipeline = pipelineBuilder.Build(); string parsedMarkdown = Markdown.ToHtml(fileContent, pipeline); + // srcset supports multiple candidates and descriptors, none of which the src sanitizer + // below validates. Remove the attribute rather than let a candidate through unchecked. + parsedMarkdown = Regex.Replace( + parsedMarkdown, + @"(]*?)\s+srcset\s*=\s*(?:""[^""]*""|'[^']*'|[^\s>]+)", + m => + { + imagesBlockedCallBack(); + return m.Groups[1].Value; + }, + RegexOptions.IgnoreCase); + + // Sanitize src on raw HTML tags in both setting states. Markdown images were + // already handled by the Markdig AST layer (rewritten to the virtual host URL or "#") + // and pass through unchanged. When local images are disabled everything else is blocked; + // when enabled it is validated the same way as the AST layer. Matches double-quoted, + // single-quoted and unquoted values, so every form an author can write is covered. + parsedMarkdown = Regex.Replace( + parsedMarkdown, + @"(]*?\ssrc\s*=\s*)(?:(""|')(.+?)\2|([^\s""'>]+))", + m => + { + bool isQuoted = m.Groups[2].Success; + string quote = isQuoted ? m.Groups[2].Value : "\""; + string src = isQuoted ? m.Groups[3].Value : m.Groups[4].Value; + + if (src == "#" || + (allowLocalImages && src.StartsWith("https://localmdimages/", StringComparison.OrdinalIgnoreCase))) + { + return m.Value; + } + + if (allowLocalImages && + HTMLParsingExtension.TryGetLocalImageVirtualUrl(src, extension.FilePath, extension.AllowedBasePath, out string? virtualUrl)) + { + return m.Groups[1].Value + quote + virtualUrl + quote; + } + + imagesBlockedCallBack(); + return m.Groups[1].Value + quote + "#" + quote; + }, + RegexOptions.IgnoreCase); + string markdownHTML = $"{htmlHeader}{parsedMarkdown}{HtmlFooter}"; return markdownHTML; } diff --git a/src/common/GPOWrapper/GPOWrapper.cpp b/src/common/GPOWrapper/GPOWrapper.cpp index 8d6844c3df..b2bccf29a5 100644 --- a/src/common/GPOWrapper/GPOWrapper.cpp +++ b/src/common/GPOWrapper/GPOWrapper.cpp @@ -60,6 +60,10 @@ namespace winrt::PowerToys::GPOWrapper::implementation { return static_cast(powertoys_gpo::getConfiguredMarkdownPreviewEnabledValue()); } + GpoRuleConfigured GPOWrapper::GetConfiguredMarkdownLocalImagesEnabledValue() + { + return static_cast(powertoys_gpo::getConfiguredMarkdownLocalImagesEnabledValue()); + } GpoRuleConfigured GPOWrapper::GetConfiguredMonacoPreviewEnabledValue() { return static_cast(powertoys_gpo::getConfiguredMonacoPreviewEnabledValue()); diff --git a/src/common/GPOWrapper/GPOWrapper.h b/src/common/GPOWrapper/GPOWrapper.h index 616b523e16..3a29982f74 100644 --- a/src/common/GPOWrapper/GPOWrapper.h +++ b/src/common/GPOWrapper/GPOWrapper.h @@ -21,6 +21,7 @@ namespace winrt::PowerToys::GPOWrapper::implementation static GpoRuleConfigured GetConfiguredFileLocksmithEnabledValue(); static GpoRuleConfigured GetConfiguredSvgPreviewEnabledValue(); static GpoRuleConfigured GetConfiguredMarkdownPreviewEnabledValue(); + static GpoRuleConfigured GetConfiguredMarkdownLocalImagesEnabledValue(); static GpoRuleConfigured GetConfiguredMonacoPreviewEnabledValue(); static GpoRuleConfigured GetConfiguredMouseWithoutBordersEnabledValue(); static GpoRuleConfigured GetConfiguredPdfPreviewEnabledValue(); diff --git a/src/common/GPOWrapper/GPOWrapper.idl b/src/common/GPOWrapper/GPOWrapper.idl index 33d6821673..5b5bed82fa 100644 --- a/src/common/GPOWrapper/GPOWrapper.idl +++ b/src/common/GPOWrapper/GPOWrapper.idl @@ -25,6 +25,7 @@ namespace PowerToys static GpoRuleConfigured GetConfiguredFileLocksmithEnabledValue(); static GpoRuleConfigured GetConfiguredSvgPreviewEnabledValue(); static GpoRuleConfigured GetConfiguredMarkdownPreviewEnabledValue(); + static GpoRuleConfigured GetConfiguredMarkdownLocalImagesEnabledValue(); static GpoRuleConfigured GetConfiguredMonacoPreviewEnabledValue(); static GpoRuleConfigured GetConfiguredPdfPreviewEnabledValue(); static GpoRuleConfigured GetConfiguredGcodePreviewEnabledValue(); diff --git a/src/common/utils/gpo.h b/src/common/utils/gpo.h index a7ae4bb00f..5434b6df82 100644 --- a/src/common/utils/gpo.h +++ b/src/common/utils/gpo.h @@ -107,6 +107,7 @@ namespace powertoys_gpo const std::wstring POLICY_NEW_PLUS_HIDE_TEMPLATE_FILENAME_EXTENSION = L"NewPlusHideTemplateFilenameExtension"; const std::wstring POLICY_NEW_PLUS_REPLACE_VARIABLES = L"NewPlusReplaceVariablesInTemplateFilenames"; const std::wstring POLICY_NEW_PLUS_HIDE_BUILT_IN_NEW_CONTEXT_MENU = L"NewPlusHideBuiltInNewContextMenu"; + const std::wstring POLICY_MARKDOWN_ALLOW_LOCAL_IMAGES = L"MarkdownAllowLocalImages"; // Methods used for reading the registry #pragma region ReadRegistryMethods @@ -724,5 +725,10 @@ namespace powertoys_gpo return getConfiguredValue(POLICY_NEW_PLUS_HIDE_BUILT_IN_NEW_CONTEXT_MENU); } + inline gpo_rule_configured_t getConfiguredMarkdownLocalImagesEnabledValue() + { + return getConfiguredValue(POLICY_MARKDOWN_ALLOW_LOCAL_IMAGES); + } + #pragma endregion IndividualModuleSettingPolicies } diff --git a/src/gpo/assets/PowerToys.admx b/src/gpo/assets/PowerToys.admx index bc71d9229f..43de800243 100644 --- a/src/gpo/assets/PowerToys.admx +++ b/src/gpo/assets/PowerToys.admx @@ -31,6 +31,7 @@ + @@ -57,6 +58,9 @@ + + + @@ -222,6 +226,16 @@ + + + + + + + + + + diff --git a/src/gpo/assets/en-US/PowerToys.adml b/src/gpo/assets/en-US/PowerToys.adml index f7d8068c2e..9b1c4f9534 100644 --- a/src/gpo/assets/en-US/PowerToys.adml +++ b/src/gpo/assets/en-US/PowerToys.adml @@ -14,6 +14,7 @@ General settings New+ Deprecated policies + File Explorer Preview PowerToys version 0.64.0 or later PowerToys version 0.68.0 or later @@ -38,6 +39,7 @@ PowerToys version 0.98.0 or later PowerToys version 0.99.0 or later PowerToys version 0.100.0 or later + PowerToys version 0.101.0 or later From PowerToys version 0.64.0 until PowerToys version 0.87.1 This policy configures the enabled state for all PowerToys utilities. @@ -263,6 +265,17 @@ If you don't configure this policy, the user will be able to control the setting File Locksmith: Configure enabled state SVG file preview: Configure enabled state Markdown file preview: Configure enabled state + Markdown preview: Show images from local and network sources + This policy configures whether images from local and network (UNC) sources should be displayed in Markdown file previews. + +If you enable this setting, images from the document's folder and network shares will always be shown and the user won't be able to disable it. + +If you disable this setting, all images will be blocked and the user won't be able to enable local image rendering. + +If you don't configure this setting, users are able to enable or disable showing local images. + +Note: Online/remote images (http/https URLs) are always blocked regardless of this setting. Images are rendered inside a sandboxed WebView2 control with scripts disabled. The Mark-of-the-Web (Zone Identifier) is not checked; access is controlled by path validation against the document's directory (local) or share root (UNC). + Source code file preview: Configure enabled state PDF file preview: Configure enabled state Gcode file preview: Configure enabled state diff --git a/src/modules/previewpane/MarkdownPreviewHandler/MarkdownPreviewHandler.csproj b/src/modules/previewpane/MarkdownPreviewHandler/MarkdownPreviewHandler.csproj index 6dc4006e53..11f6291171 100644 --- a/src/modules/previewpane/MarkdownPreviewHandler/MarkdownPreviewHandler.csproj +++ b/src/modules/previewpane/MarkdownPreviewHandler/MarkdownPreviewHandler.csproj @@ -62,6 +62,7 @@ + diff --git a/src/modules/previewpane/MarkdownPreviewHandler/MarkdownPreviewHandlerControl.cs b/src/modules/previewpane/MarkdownPreviewHandler/MarkdownPreviewHandlerControl.cs index 875e1e98d7..e8b4beede0 100644 --- a/src/modules/previewpane/MarkdownPreviewHandler/MarkdownPreviewHandlerControl.cs +++ b/src/modules/previewpane/MarkdownPreviewHandler/MarkdownPreviewHandlerControl.cs @@ -56,6 +56,10 @@ namespace Microsoft.PowerToys.PreviewHandler.Markdown /// private bool _infoBarDisplayed; + private string _markdownDirectory; + private string _allowedBasePath; + private bool _allowLocalImages; + /// /// Gets the path of the current assembly. /// @@ -116,14 +120,33 @@ namespace Microsoft.PowerToys.PreviewHandler.Markdown throw new ArgumentException($"{nameof(dataSource)} for {nameof(MarkdownPreviewHandlerControl)} must be a string but was a '{typeof(T)}'"); } - string fileText = File.ReadAllText(filePath); - Regex imageTagRegex = new Regex(@"<[ ]*img.*>"); - if (imageTagRegex.IsMatch(fileText)) + _allowLocalImages = Settings.GetLocalImagesEnabled(); + _markdownDirectory = Path.GetDirectoryName(filePath) ?? string.Empty; + + _allowedBasePath = _markdownDirectory; + if (_markdownDirectory.StartsWith(@"\\", StringComparison.Ordinal)) { - _infoBarDisplayed = true; + string trimmed = _markdownDirectory.Substring(2); + int firstSep = trimmed.IndexOf('\\'); + int secondSep = firstSep >= 0 ? trimmed.IndexOf('\\', firstSep + 1) : -1; + if (secondSep >= 0) + { + _allowedBasePath = string.Concat(@"\\", trimmed.AsSpan(0, secondSep)); + } } - string markdownHTML = FilePreviewCommon.MarkdownHelper.MarkdownHtml(fileText, Settings.GetTheme(), filePath, ImagesBlockedCallBack); + string fileText = File.ReadAllText(filePath); + + if (!_allowLocalImages) + { + Regex imageTagRegex = new Regex(@"<[ ]*img.*>"); + if (imageTagRegex.IsMatch(fileText)) + { + _infoBarDisplayed = true; + } + } + + string markdownHTML = FilePreviewCommon.MarkdownHelper.MarkdownHtml(fileText, Settings.GetTheme(), filePath, ImagesBlockedCallBack, _allowLocalImages, _allowedBasePath); _browser = new WebView2() { @@ -152,15 +175,44 @@ namespace Microsoft.PowerToys.PreviewHandler.Markdown _browser.CoreWebView2.Settings.IsScriptEnabled = false; _browser.CoreWebView2.Settings.IsWebMessageEnabled = false; - // Don't load any resources. + // Don't load any resources except virtual host mapped ones. _browser.CoreWebView2.AddWebResourceRequestedFilter("*", CoreWebView2WebResourceContext.All); _browser.CoreWebView2.WebResourceRequested += (object sender, CoreWebView2WebResourceRequestedEventArgs e) => { - // Show local file we've saved with the markdown contents. Block all else. - if (new Uri(e.Request.Uri) != _localFileURI) + // Allow the local HTML file + if (_localFileURI != null && new Uri(e.Request.Uri) == _localFileURI) { - e.Response = _browser.CoreWebView2.Environment.CreateWebResourceResponse(null, 403, "Forbidden", null); + return; } + + // Serve virtual host image requests (localmdimages) directly. WebView2 + // Runtime 150+ no longer serves UNC/network paths through + // SetVirtualHostNameToFolderMapping, so the image bytes are read here + // after re-validating the resolved path against the allowed base path. + if (_allowLocalImages && e.Request.Uri.StartsWith("https://localmdimages/", StringComparison.OrdinalIgnoreCase)) + { + if (FilePreviewCommon.HTMLParsingExtension.TryResolveVirtualUrl(e.Request.Uri, _allowedBasePath, out string imagePath) && File.Exists(imagePath)) + { + try + { + var imageStream = new MemoryStream(File.ReadAllBytes(imagePath)); + e.Response = _browser.CoreWebView2.Environment.CreateWebResourceResponse(imageStream, 200, "OK", "Content-Type: " + GetImageContentType(imagePath)); + return; + } + catch (IOException) + { + } + catch (UnauthorizedAccessException) + { + } + } + + e.Response = _browser.CoreWebView2.Environment.CreateWebResourceResponse(null, 404, "Not Found", null); + return; + } + + // Block everything else + e.Response = _browser.CoreWebView2.Environment.CreateWebResourceResponse(null, 403, "Forbidden", null); }; _browser.CoreWebView2.ContextMenuRequested += (object sender, CoreWebView2ContextMenuRequestedEventArgs args) => @@ -217,7 +269,10 @@ namespace Microsoft.PowerToys.PreviewHandler.Markdown if (_infoBarDisplayed) { - _infoBar = GetTextBoxControl(Resources.BlockedImageInfoText); + string message = _allowLocalImages + ? Resources.RemoteImagesBlockedInfoText + : Resources.BlockedImageInfoText; + _infoBar = GetTextBoxControl(message); Resize += FormResized; Controls.Add(_infoBar); } @@ -303,6 +358,28 @@ namespace Microsoft.PowerToys.PreviewHandler.Markdown } } + /// + /// Returns the HTTP Content-Type for an image file based on its extension. + /// + /// Path of the image file. + /// The content type string. + private static string GetImageContentType(string imagePath) + { + return Path.GetExtension(imagePath).ToUpperInvariant() switch + { + ".PNG" => "image/png", + ".JPG" or ".JPEG" => "image/jpeg", + ".GIF" => "image/gif", + ".BMP" => "image/bmp", + ".WEBP" => "image/webp", + ".SVG" => "image/svg+xml", + ".ICO" => "image/x-icon", + ".TIF" or ".TIFF" => "image/tiff", + ".AVIF" => "image/avif", + _ => "application/octet-stream", + }; + } + /// /// Callback when image is blocked by extension. /// diff --git a/src/modules/previewpane/MarkdownPreviewHandler/Properties/Resources.Designer.cs b/src/modules/previewpane/MarkdownPreviewHandler/Properties/Resources.Designer.cs index c15e32ce06..b37dec1397 100644 --- a/src/modules/previewpane/MarkdownPreviewHandler/Properties/Resources.Designer.cs +++ b/src/modules/previewpane/MarkdownPreviewHandler/Properties/Resources.Designer.cs @@ -78,6 +78,15 @@ namespace Microsoft.PowerToys.PreviewHandler.Markdown.Properties { } } + /// + /// Looks up a localized string similar to Some online images have been blocked. Only local and network images from the document's folder are shown.. + /// + internal static string RemoteImagesBlockedInfoText { + get { + return ResourceManager.GetString("RemoteImagesBlockedInfoText", resourceCulture); + } + } + /// /// Looks up a localized string for an error when Gpo has the utility disabled. /// diff --git a/src/modules/previewpane/MarkdownPreviewHandler/Properties/Resources.resx b/src/modules/previewpane/MarkdownPreviewHandler/Properties/Resources.resx index 7c4d1b6065..24fd4ed866 100644 --- a/src/modules/previewpane/MarkdownPreviewHandler/Properties/Resources.resx +++ b/src/modules/previewpane/MarkdownPreviewHandler/Properties/Resources.resx @@ -125,6 +125,10 @@ The markdown could not be preview due to an internal error. This text is displayed if markdown fails to preview + + Some online images have been blocked. Only local images from the document's folder and images on the same network share are shown. + This text is displayed when local images are enabled but the document contains remote/online image URLs that were blocked. + Tried to start with a GPO policy setting the utility to always be disabled. Please contact your systems administrator. GPO stands for the Windows Group Policy Object feature. diff --git a/src/modules/previewpane/MarkdownPreviewHandler/Settings.cs b/src/modules/previewpane/MarkdownPreviewHandler/Settings.cs index a69808d2ed..b8a3a83e79 100644 --- a/src/modules/previewpane/MarkdownPreviewHandler/Settings.cs +++ b/src/modules/previewpane/MarkdownPreviewHandler/Settings.cs @@ -4,10 +4,13 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Text; using System.Threading.Tasks; +using Microsoft.PowerToys.Settings.UI.Library; + namespace Microsoft.PowerToys.PreviewHandler.Markdown { internal sealed class Settings @@ -40,5 +43,34 @@ namespace Microsoft.PowerToys.PreviewHandler.Markdown { return Common.UI.ThemeManager.GetWindowsBaseColor().ToLowerInvariant(); } + + private static readonly SettingsUtils ModuleSettings = SettingsUtils.Default; + + /// + /// Returns whether local images should be displayed in the Markdown preview. + /// GPO policy takes precedence over user setting. + /// + public static bool GetLocalImagesEnabled() + { + var gpo = global::PowerToys.GPOWrapper.GPOWrapper.GetConfiguredMarkdownLocalImagesEnabledValue(); + if (gpo == global::PowerToys.GPOWrapper.GpoRuleConfigured.Enabled) + { + return true; + } + + if (gpo == global::PowerToys.GPOWrapper.GpoRuleConfigured.Disabled) + { + return false; + } + + try + { + return ModuleSettings.GetSettings(PowerPreviewSettings.ModuleName).Properties.EnableMdLocalImages; + } + catch (FileNotFoundException) + { + return false; + } + } } } diff --git a/src/modules/previewpane/MarkdownPreviewHandler/TestFiles/GPO-TESTING.md b/src/modules/previewpane/MarkdownPreviewHandler/TestFiles/GPO-TESTING.md new file mode 100644 index 0000000000..ac896541e1 --- /dev/null +++ b/src/modules/previewpane/MarkdownPreviewHandler/TestFiles/GPO-TESTING.md @@ -0,0 +1,60 @@ +# GPO Testing Instructions for Markdown Local Images + +## Manual Registry Testing + +Run these commands in an elevated (Administrator) command prompt: + +```cmd +REM Force-enable local images (toggle locked ON in Settings UI): +reg add "HKLM\SOFTWARE\Policies\PowerToys" /v MarkdownAllowLocalImages /t REG_DWORD /d 1 /f + +REM Force-disable local images (toggle locked OFF in Settings UI): +reg add "HKLM\SOFTWARE\Policies\PowerToys" /v MarkdownAllowLocalImages /t REG_DWORD /d 0 /f + +REM Remove policy (user controls the setting): +reg delete "HKLM\SOFTWARE\Policies\PowerToys" /v MarkdownAllowLocalImages /f +``` + +After changing the registry value, restart PowerToys for the setting to take effect. + +## Group Policy Editor Testing (gpedit.msc) + +1. Copy `src/gpo/assets/PowerToys.admx` to `C:\Windows\PolicyDefinitions\` +2. Copy `src/gpo/assets/en-US/PowerToys.adml` to `C:\Windows\PolicyDefinitions\en-US\` +3. Open `gpedit.msc` +4. Navigate to: Computer Configuration > Administrative Templates > PowerToys > File Explorer Preview +5. Find "Markdown preview: Show local images - Configure enabled state" +6. Set to Enabled, Disabled, or Not Configured + +## Expected Behavior Matrix + +| GPO State | User Setting | Images Shown? | Settings UI Toggle | Info Bar | +|-----------|-------------|---------------|-------------------|----------| +| Not configured | OFF | No | Editable, OFF | "Some pictures have been blocked..." | +| Not configured | ON | Local/UNC only | Editable, ON | "Some online images have been blocked..." (if remote images in file) | +| Enabled | (ignored) | Local/UNC only | Locked ON (grayed out) | "Some online images have been blocked..." (if remote images in file) | +| Disabled | (ignored) | No | Locked OFF (grayed out) | "Some pictures have been blocked..." | + +## Verification Steps + +1. Set GPO to "Enabled" via registry +2. Open PowerToys Settings > File Explorer > Markdown +3. Verify the "Show local images" toggle is ON and grayed out (cannot be changed) +4. Preview `test-local-images.md` in File Explorer +5. Verify local image renders, remote image is blocked with info bar + +6. Set GPO to "Disabled" via registry +7. Restart PowerToys +8. Verify the toggle is OFF and grayed out +9. Preview the same file +10. Verify all images are blocked + +11. Remove the GPO registry value +12. Restart PowerToys +13. Verify the toggle is editable again + +## Notes + +- Machine scope (HKLM) takes precedence over user scope (HKCU) +- The "Some preview handlers are managed by your organization" info bar will appear in Settings when any GPO is configured +- Remote/online images (http/https URLs) are ALWAYS blocked regardless of this setting diff --git a/src/modules/previewpane/MarkdownPreviewHandler/TestFiles/images/test.png b/src/modules/previewpane/MarkdownPreviewHandler/TestFiles/images/test.png new file mode 100644 index 0000000000000000000000000000000000000000..6ae44344c5e57901e128449491f8ff98a88d8ed1 GIT binary patch literal 6287 zcmb_hcT`hbmk)vvI))yakRUt|P?|s>v;a|*UIYZBgLI{YCcPUW4-_m&3(}iZrFX?b z?@~gMjzM}6=ED2Fw`R?}nfYhd%FSIjx!LEQefF>HlUN-s4O%KF6$k{Py@^KY0q1Gp zXakc2?-$nFzW^taM|v76pptK#%fJPhgYq3^5U4zX`q=6sa82ofHhBaB(Y2i&Bwg-# zwjdB2{3c4-0AsP9X6eXb+}PFhaQx5USCDs!(O1hCOeysQ$2_dWBB6Y*M#WlcAI=~r zSYB>R*inO~7v4Zaw86&qN3{oYa=xwY~B*IO>n@P4C-&GvOwy0%v@fhX{q zDuOKm@x|}yv)OMtd_ymOlz@IZ3{q;Bm3t{G(Qmx?C4CE5*wA23qRil(ixs&H&O++! zgIjq81IY*sOvo?~FcpuoOoZmP%VN1r7qN|Av8==FNB1+~(WYQUvwOIaQp>si98H(& z%96~jgjcU#wK=2*+53Uqzt@Jwvx>dzV1DnvBlIm@v4@2e#Ka$xpUb%9D(tgz&~feQ zn>67<2Z|};g&)y~G3)t}x7SB+#4@ME2gNyBMMS67`K-6@@5?;pLUbbSx-<2&_b013 zANz_#tLCbY<-Wg%c$_3s5PLi>N&ex(%qC_9A`iM7yW2Lm**v$IQRoaBaP|W!J%|g_ zV7o-e18TRvP5^C;R?$Zvs&DRfRqP+lnQ*Q!QcwG|u*O=|R?r?)Io(UQOXmlFA_nbE z`s$I8g;Xg%jsuQv7LHn<@n z$A@K~91I!SinF(qQr#2Q-R4m!prdSycU(|j67Sa zh3X88&L?4!(V3zM!+DY?R&L;(htm9|+M$0f9u? z?Tm`z;;VN-W%5lCRe{`GZeoF?3QJ0Knp>ZhthlZhj|j87)9oCPfb#LCZ$;XsZ{SA= zRV1oGORmnUX6{VDS`==zEnR(m+w@a`<%PDbLVvMalJt56gwGmVXIIyy_zvQv?{vAg z*@cNY6DcOz_JO-;JmmJ|easfN=_P3CY;sKpKW z)%9X}7cJ=SEAC$HAC3**Z&g}OKmSHJc$X>Ew~vnO*_bIefW~!a2wGsiDn(_EJkM?J!j$^11lNcYbx8vCGaJ*w?qfc1-4GtA! z%%!xpt&g_isXQC)G7d>x>g`GDUq4VjtcgQ40?IFU>39g5~n0z%snx* zTrFiCphAFwqZyC{6^4kaf1(xQm{y%6q{e-^vn8BjY&NbQvw6Yc+iR*I5bzww+xIB@4kuQEr`6OF2?=%68gE%YitCCe%fADrV|K z(#iFrX-{Dpg0FpIcP_gm=pP2VW0Wo5R3~8Uo-sHVO2dsDUf}0Qw7AOGo$T?g6daO} z#h&!8S2x^g=CZrVArQG* z+Cpie$iYQ1iRAo>FVEBovyYdVaA_5^*;%|`pLI=BUHX`cv{mr;d8!{>qcB0eOf)IR$?Porv2FQN zETFOhHhDGbmkpOWOqYQ}h zx7sWBN@VY)d%~DDTUaSu%&%z2@y4(}c^5hf`6GfF3Ds8S&7{m!zr?sQgwnm>c^eqw zQTLbp;hB)u35>rJuF6Df40+>F0w)r{WebEQsv=`)|*r*&UBeA?6m@K z_>w-&IAu|2h8sqCMp{Tq%8({k@^aB9%PKLp6nQ$jguNYH!m=bY1%*6F=^M8+xQ}OT zjq&LI0>2~f>izyl;_ZA@g9^C`E<*g8j}u&rCcSwgeQI{BrpIF`TUBO8Hblcxp>;|j z;7&l3dM`S|W4BVw59J}F&aeQE?xCae<%v3daF9O2wTN*b0U}|01(NTfp_ag;j1fdWj?+kUi}Kie2w|>AH$+8UO98I~!7$xsZiY5GZ15g2XQS zcp_t^Vf+Qi8XY#GGa(_ky)Rgn zs**&MmAJ9tORMk()8h~!2|;D^G9&B4C_urnrx)w!)>KYUx3d@N8}@gio5-(HarZ)@ zl={ylKKhVd*SlLI2AQQn$PSL%tANidymx z>R@w2WwcP#rYP@ej8!aR&31^cs`>?k>ONncIh3+c?A-`sKbpZzStih@uc#&krye#c z^fbD3>@t?jiuv5I5*S6vl&MbNHi>Si89w|WyGmym~xR2esDBB9+#LkBncUp z?_9fk&`?tHOfbu^5oF4oPXr!C65{eo3frz@HS3wPtUmOdnT{zvfr1IsMjiE=SKrt% z?aEO-H#!msav2*Dt8UE#syz6U6ZM>#h9q0#X@+op%+!*QHJj8L{fiGOtMhAH{=8hs zesrRGs{yQ!Kt2KG3bk8KN_0rN`=yjQ#r%V!?MhrxQccflgaI(4Ocf|4U8Kn=Kk0=m zDuBI&F~BIYcd~FE?*|DOcdx2r%qWMtOa@xyv-s<~xn!*b*jc_|PVgtSfGj>O96SV! zl(tRtv5O}Hm11{Mt4N<5Ks_FJ4Tx+le1@8rJGOkp|obb_NvoUpO+P*Y*FcyBXze*gpS@p@vR37%=-g{pi08eW=ken(YrmH3n?B z$Nv2ab?ds!%2f5RL=B-pp9k;~uBQAw|D0g6n}rYH+Y2dixcW;$FFGmTqZH1dGKjv1 zc}U0l=C#Bz_GMI7aS@Io6V-O@8ZDWP6vyY7aom}wpmcV!!$yGBXv537Pz{Mx>1j|( z+Ul-~2ZFbtl*qW*$gZ!?Av;RIcoYda{W=CO&z@*WsYfZZG2~Z583jY?q%D!N8Wb0@#R4|4-$a*4c}`-JA}cE9v2|P~rnGuO`|1Q?cOuAEY7S z38TK6uOKltQsePw4G_ov74Y9g70dZ>qyK%J(8`@@KHsDp=~SMU4u!o=hG#NwX4_2)n)o!~E3VYmilq>I&+`VWDC zkGx@DBCsQu*UP>8-nL!;s)Ij#_cVt%ZoXUp$%mDmYmlAqFrdTaP4yE@AjykXl+(KR zIC8$>=gQ&1QDO}@0*Hx*^}(UBCUAZJri2X+CHm(&sXkx>C;FrBg8uD~)jyC8_;6fv z+hef5Bqtu#aSVM+@xfzz?cTEm@8&eFp=mQ9B7yv8&|*_fiO^~}%Q67CELllN#fZZU zo!X2c6AhLOm-Z4hW@PXnp@-DBr1s9|B>i7)h7~~AF}LS&pFb}%zV!2$_N~~fp7k$M z6AHxRbeyQL?Z4!o_lAdNf>rn}>2^9sH;Q?i>c^*i4|WE_0MAFqt(lXVCn@q4fRWKP z*=HRdUTlH~4}0?m-z?C)<5GUY^kz#+j!wW z(eU2M$*(M=;|471b~>ig@8$8%G}m^R`Ydb~E$P=$p5IJYmqo$Ma{@gP{!Z|fo6GWI zu5_sydI()7aVKw66JKS3FQuK}fStyn;dt)V)zvlY7^p7ipn0Vnz)wJhY2|tzZad{` z$fen{+Q`a33(-Z~ALL?N{kRt+zaBc(_A>tp&8lnQq*@`289ADPlL!3awG zNuaJ7QnC%wJ|dqCE!WDK!Vh_~7Gu5ki-ys!4}UQomhPn}j8Ea*o>!&jlNID~fIL=- z{$$Mk=9S|A=pHQ*!kw}2ko4a_^4YxBHzmowl~M2^lW1kP>S)KEK%tTFrWoU5_-Z#q zTYpK^lr5iV!X+=UyFNi-97*rUf3Jku_8k|v)4-1b1XN-l?v~MUZ;GxO|NXaaf@*j= zR*7|G{x8YkI~jG3cIT+I|#H@g!z)|*j6ch;eK^xW5( zmHhDHR!FYfeQ%0-IlWXzdLgT~s1Ruv7q3aRc)mc~B(R*Z1V+r!fSLT!E<0V27nnkx zg1A(uFhyC&5E^SA>cxtU_1IsV@z}H4FNIDT(>G#5cCkJK2uq6w|H#iNVig@^ z6#eFsqW_x*D>~qvRGXV~+}C>i7_xriQ_a@HgY255_8qeh8#R)FBEx`*_4q*kTj{Cn zM^HP#!L6ndZ5n^EFPZxUYnyo#!lQfY#<|sU;xMm}eEjP+pyE9J=s~Jrxo7MvYcYV? z8ccU92}-{~*GeFDf>-n7aUOK%QftVUsGc>Bx3Z}d`kByg2Zz0}em^zAz9)MmXYp!o z^P)Xp#n!)-)`BR%I{S#1 zP{Odl>ZJd!x~&5D)&Mbm|L`KvJ3GV+|2o`pzW2c^R^&>|TNNOx6Ws0Zy}5j^Xw7{8 zkX&4zSA*te)E&c%p_rG+jQoR|vgK*lAHTev2Qb3;<|`N7$vnK!<%Ni(?$~vplI|y| zhdFXH>1o}KJMW4e0qxfRxpV8Z`sFR5hzA81Brr*K*h8{8ajf@zK)aEVmLx)t5KarF z#8PX2BeW(xifXd@tdm@-B4};EoXT6GeR%=3eir*2zB1vlRGBRJ9ZG(W6#%l&fZw}l z<69k~)x#p!H|L|XjGW-Md3VyA3st*gyN_HDXJ$0KOV3I0K@>lflKpD0K+GVazq>ou zH3TU8e)MRIkrZSmkLpWepFr8?`5|1ET{7bY<6j+3QR7D~7g{~7L3)5vFHTwv%OpO` zYLwwdw)&xE2{iw=r>xY3c^>V$a>`?XeTt93uBXS3FYMK+mtRtyyWQI0)5c|Y@l=2V z|3#>hto`V_Q3FuQscNyPIvU@Zt65il~z zqA>PNrq6}-2}5T+#x=$l>&ilbWy&zmlViVnzpYN9j?1J+<5ULN2@chS>3-j}*7-c5 z5Xb#Z8_hAT@vqLpP(a))^dFcOe#JlBbTkeUsvkW9%OfTPn5qIHd=!`fR()^Hks^>= ztH|rBl<{>OWnG6mOrFTZ)ov|w&ENP{d*d + +## Remote Image (should always be blocked) + +This image should never render (online URL): + +![Remote Image](https://example.com/nonexistent-image.png) + +## UNC Path Image (for network testing) + +When the markdown file is on a network share, the following would render: + + + +Note: UNC images are allowed when: +- The markdown file itself is on a UNC share, AND +- The image is on the same share root (e.g., same `\\server\share\`) + +## Path Traversal (should always be blocked) + +This attempts to escape the document directory and should be blocked: + +![Traversal](../../../../../../Windows/System32/notepad.exe) diff --git a/src/modules/previewpane/UnitTests-MarkdownPreviewHandler/HTMLParsingExtensionTest.cs b/src/modules/previewpane/UnitTests-MarkdownPreviewHandler/HTMLParsingExtensionTest.cs index d8fe7dc9bc..21e9484806 100644 --- a/src/modules/previewpane/UnitTests-MarkdownPreviewHandler/HTMLParsingExtensionTest.cs +++ b/src/modules/previewpane/UnitTests-MarkdownPreviewHandler/HTMLParsingExtensionTest.cs @@ -2,6 +2,9 @@ // The Microsoft Corporation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; +using System.IO; + using Markdig; using Microsoft.PowerToys.PreviewHandler.Markdown; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -100,5 +103,326 @@ namespace PreviewPaneUnitTests const string expected = "

\"text\"

\n"; Assert.AreEqual(expected, html); } + + [DataTestMethod] + [DataRow("images/test.png", @"C:\docs", @"C:\docs", "https://localmdimages/images/test.png")] + [DataRow(@"C:\docs\images\test.png", @"C:\docs", @"C:\docs", "https://localmdimages/images/test.png")] + [DataRow("images/test.png", @"\\server\share\sub\dir", @"\\server\share", "https://localmdimages/sub/dir/images/test.png")] + [DataRow("../test.png", @"\\server\share\sub", @"\\server\share", "https://localmdimages/test.png")] + public void TryGetLocalImageVirtualUrlAllowsContainedPaths(string url, string markdownDirectory, string basePath, string expectedVirtualUrl) + { + bool result = Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension.TryGetLocalImageVirtualUrl(url, markdownDirectory, basePath, out string virtualUrl); + + Assert.IsTrue(result); + Assert.AreEqual(expectedVirtualUrl, virtualUrl); + } + + [DataTestMethod] + [DataRow("http://example.com/a.png", @"C:\docs", @"C:\docs")] + [DataRow("https://example.com/a.png", @"C:\docs", @"C:\docs")] + [DataRow("data:image/png;base64,iVBORw0KGgo=", @"C:\docs", @"C:\docs")] + [DataRow("javascript:alert(1)", @"C:\docs", @"C:\docs")] + [DataRow("file:///C:/secret.png", @"C:\docs", @"C:\docs")] + [DataRow("../secret.png", @"C:\docs", @"C:\docs")] + [DataRow(@"..\..\secret.png", @"C:\docs\sub", @"C:\docs\sub")] + [DataRow(@"C:\other\secret.png", @"C:\docs", @"C:\docs")] + [DataRow(@"C:\docsBackup\secret.png", @"C:\docs", @"C:\docs")] + [DataRow(@"\\server\share2\secret.png", @"\\server\share\sub", @"\\server\share")] + [DataRow("", @"C:\docs", @"C:\docs")] + public void TryGetLocalImageVirtualUrlBlocksUnsafeUrls(string url, string markdownDirectory, string basePath) + { + bool result = Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension.TryGetLocalImageVirtualUrl(url, markdownDirectory, basePath, out string virtualUrl); + + Assert.IsFalse(result); + Assert.IsNull(virtualUrl); + } + + // Resolving checks each path component for reparse points, so these cases operate on real + // files rather than notional paths. + [DataTestMethod] + [DataRow("images/test.png", "https://localmdimages/images/test.png")] + [DataRow("sub/dir/images/test.png", "https://localmdimages/sub/dir/images/test.png")] + [DataRow("my image.png", "https://localmdimages/my%20image.png")] + public void TryResolveVirtualUrlAllowsContainedRequests(string relativePath, string requestUri) + { + string root = Path.Combine(Path.GetTempPath(), "ptmd-" + Guid.NewGuid().ToString("N")); + string expectedPath = Path.Combine(root, relativePath.Replace('/', Path.DirectorySeparatorChar)); + Directory.CreateDirectory(Path.GetDirectoryName(expectedPath)); + File.WriteAllText(expectedPath, "not really an image"); + + try + { + bool result = Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension.TryResolveVirtualUrl(requestUri, root, out string resolvedPath); + + Assert.IsTrue(result); + Assert.AreEqual(expectedPath, resolvedPath); + } + finally + { + try + { + Directory.Delete(root, true); + } + catch (IOException) + { + } + } + } + + [TestMethod] + public void TryResolveVirtualUrlRejectsMissingFile() + { + string root = Path.Combine(Path.GetTempPath(), "ptmd-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(root); + + try + { + bool result = Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension.TryResolveVirtualUrl( + "https://localmdimages/does-not-exist.png", root, out string resolvedPath); + + Assert.IsFalse(result, "a path that cannot be inspected must fail closed"); + Assert.IsNull(resolvedPath); + } + finally + { + try + { + Directory.Delete(root, true); + } + catch (IOException) + { + } + } + } + + [DataTestMethod] + [DataRow("https://localmdimages/.." + "%2F" + "secret.png", @"C:\docs")] + [DataRow("https://localmdimages/.." + "%5C" + "secret.png", @"C:\docs")] + [DataRow("https://localmdimages/C" + "%3A%5C" + "other" + "%5C" + "secret.png", @"C:\docs")] + [DataRow("https://example.com/images/test.png", @"C:\docs")] + [DataRow("https://localmdimages/", @"C:\docs")] + [DataRow("not a url", @"C:\docs")] + [DataRow("", @"C:\docs")] + public void TryResolveVirtualUrlBlocksUnsafeRequests(string requestUri, string basePath) + { + bool result = Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension.TryResolveVirtualUrl(requestUri, basePath, out string resolvedPath); + + Assert.IsFalse(result); + Assert.IsNull(resolvedPath); + } + + [DataTestMethod] + [DataRow("")] + [DataRow("")] + [DataRow("")] + [DataRow("\"x\"")] + public void RawHtmlImageIsRewrittenForEveryQuoteStyle(string mdString) + { + string html = Microsoft.PowerToys.FilePreviewCommon.MarkdownHelper.MarkdownHtml( + mdString, "light", @"C:\docs\doc.md", () => { }, true, @"C:\docs"); + + StringAssert.Contains(html, "https://localmdimages/images/test.png"); + } + + [DataTestMethod] + [DataRow("")] + [DataRow("")] + [DataRow("")] + [DataRow("")] + [DataRow("")] + public void RawHtmlImageIsBlockedForEveryQuoteStyle(string mdString) + { + int blockedCount = 0; + string html = Microsoft.PowerToys.FilePreviewCommon.MarkdownHelper.MarkdownHtml( + mdString, "light", @"C:\docs\doc.md", () => { blockedCount++; }, true, @"C:\docs"); + + Assert.AreNotEqual(0, blockedCount, "the blocked-images callback should fire so the info bar is shown"); + StringAssert.Contains(html, "src="); + Assert.IsFalse(html.Contains("example.com"), "remote source must not survive the rewrite"); + Assert.IsFalse(html.Contains("secret.png"), "traversal source must not survive the rewrite"); + Assert.IsFalse(html.Contains("base64"), "data URI must not survive the rewrite"); + } + + [DataTestMethod] + [DataRow(true)] + [DataRow(false)] + public void RawHtmlSrcsetIsRemovedInBothSettingStates(bool allowLocalImages) + { + int blockedCount = 0; + string mdString = ""; + + string html = Microsoft.PowerToys.FilePreviewCommon.MarkdownHelper.MarkdownHtml( + mdString, "light", @"C:\docs\doc.md", () => { blockedCount++; }, allowLocalImages, @"C:\docs"); + + Assert.IsFalse(html.Contains("srcset"), "srcset must be removed so its candidates cannot bypass the src sanitizer"); + Assert.IsFalse(html.Contains("base64"), "the srcset data URI must not survive"); + Assert.AreNotEqual(0, blockedCount); + } + + [DataTestMethod] + [DataRow("", "base64")] + [DataRow("", "example.com")] + [DataRow("", "images/test.png")] + public void RawHtmlSrcIsSanitizedWhenLocalImagesDisabled(string mdString, string forbidden) + { + int blockedCount = 0; + + string html = Microsoft.PowerToys.FilePreviewCommon.MarkdownHelper.MarkdownHtml( + mdString, "light", @"C:\docs\doc.md", () => { blockedCount++; }, false, @"C:\docs"); + + Assert.IsFalse(html.Contains(forbidden), "raw HTML img sources must be blocked while the setting is off"); + StringAssert.Contains(html, "src=\"#\""); + Assert.AreNotEqual(0, blockedCount); + } + + [TestMethod] + public void TryResolveVirtualUrlRejectsPathBehindDirectoryLink() + { + string root = Path.Combine(Path.GetTempPath(), "ptmd-" + Guid.NewGuid().ToString("N")); + string allowed = Path.Combine(root, "allowed"); + string outside = Path.Combine(root, "outside"); + Directory.CreateDirectory(allowed); + Directory.CreateDirectory(outside); + File.WriteAllText(Path.Combine(outside, "secret.png"), "not really an image"); + + try + { + string link = Path.Combine(allowed, "link"); + try + { + Directory.CreateSymbolicLink(link, outside); + } + catch (Exception ex) when (ex is IOException || ex is UnauthorizedAccessException) + { + Assert.Inconclusive("Creating a directory link requires privilege on this machine."); + return; + } + + // Lexically "link/secret.png" sits inside the allowed directory, but the read would + // follow the link outside it. + bool result = Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension.TryResolveVirtualUrl( + "https://localmdimages/link/secret.png", allowed, out string resolvedPath); + + Assert.IsFalse(result, "a path traversing a reparse point must be rejected"); + Assert.IsNull(resolvedPath); + } + finally + { + try + { + Directory.Delete(root, true); + } + catch (IOException) + { + } + } + } + + [DataTestMethod] + [DataRow(true, "img-src https://localmdimages")] + [DataRow(false, "img-src 'none'")] + public void ContentSecurityPolicyMatchesTheSettingState(bool allowLocalImages, string expectedImgSrc) + { + string html = Microsoft.PowerToys.FilePreviewCommon.MarkdownHelper.MarkdownHtml( + "# heading", "light", @"C:\docs\doc.md", () => { }, allowLocalImages, @"C:\docs"); + + StringAssert.Contains(html, "http-equiv=\"Content-Security-Policy\""); + StringAssert.Contains(html, expectedImgSrc); + StringAssert.Contains(html, "default-src 'none'"); + StringAssert.Contains(html, "object-src 'none'"); + StringAssert.Contains(html, "frame-src 'none'"); + + // The policy has to precede the content it governs. + Assert.IsTrue( + html.IndexOf("Content-Security-Policy", StringComparison.Ordinal) < html.IndexOf(" { blockedCount++; }); + + StringAssert.Contains(html, "src=\"#\""); + StringAssert.Contains(html, "img-src 'none'"); + Assert.AreNotEqual(0, blockedCount); + } + + [DataTestMethod] + [DataRow("images/a#b.png")] + [DataRow("images/a%20b.png")] + [DataRow("images/a b.png")] + [DataRow("images/a&b.png")] + public void VirtualUrlRoundTripsFilenamesWithReservedCharacters(string relativePath) + { + string root = Path.Combine(Path.GetTempPath(), "ptmd-" + Guid.NewGuid().ToString("N")); + string onDisk = Path.Combine(root, relativePath.Replace('/', Path.DirectorySeparatorChar)); + Directory.CreateDirectory(Path.GetDirectoryName(onDisk)); + File.WriteAllText(onDisk, "not really an image"); + + try + { + bool built = Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension.TryGetLocalImageVirtualUrl( + relativePath, root, root, out string virtualUrl); + Assert.IsTrue(built, "the URL should be produced"); + + // A URL carrying a raw '#' or '%' would be truncated or misparsed here. + bool resolved = Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension.TryResolveVirtualUrl( + virtualUrl, root, out string resolvedPath); + + Assert.IsTrue(resolved, $"the escaped URL '{virtualUrl}' should resolve back"); + Assert.AreEqual(onDisk, resolvedPath); + } + finally + { + try + { + Directory.Delete(root, true); + } + catch (IOException) + { + } + } + } + + [TestMethod] + public void ExtensionRewritesLocalImageToVirtualHostWhenLocalImagesAllowed() + { + // arrange + string mdString = "![text](images/test.png)"; + Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension htmlParsingExtension = new Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension(() => { }, @"C:\docs"); + htmlParsingExtension.AllowLocalImages = true; + MarkdownPipeline markdownPipeline = BuildPipeline(htmlParsingExtension); + + // Act + string html = Markdown.ToHtml(mdString, markdownPipeline); + + // Assert + const string expected = "

\"text\"

\n"; + Assert.AreEqual(expected, html); + } + + [TestMethod] + public void ExtensionBlocksPathTraversalAndMakesCallbackWhenLocalImagesAllowed() + { + // arrange + int count = 0; + string mdString = "![text](../secret.png)"; + Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension htmlParsingExtension = new Microsoft.PowerToys.FilePreviewCommon.HTMLParsingExtension(() => { count++; }, @"C:\docs"); + htmlParsingExtension.AllowLocalImages = true; + MarkdownPipeline markdownPipeline = BuildPipeline(htmlParsingExtension); + + // Act + string html = Markdown.ToHtml(mdString, markdownPipeline); + + // Assert + Assert.AreEqual(1, count); + const string expected = "

\"text\"

\n"; + Assert.AreEqual(expected, html); + } } } diff --git a/src/settings-ui/Settings.UI.Library/PowerPreviewProperties.cs b/src/settings-ui/Settings.UI.Library/PowerPreviewProperties.cs index b99b19aae7..5ada3c53b9 100644 --- a/src/settings-ui/Settings.UI.Library/PowerPreviewProperties.cs +++ b/src/settings-ui/Settings.UI.Library/PowerPreviewProperties.cs @@ -81,6 +81,23 @@ namespace Microsoft.PowerToys.Settings.UI.Library } } + private bool enableMdLocalImages; + + [JsonPropertyName("md-previewer-local-images-setting")] + [JsonConverter(typeof(BoolPropertyJsonConverter))] + public bool EnableMdLocalImages + { + get => enableMdLocalImages; + set + { + if (value != enableMdLocalImages) + { + LogTelemetryEvent(value); + enableMdLocalImages = value; + } + } + } + private bool enableMonacoPreview = true; [JsonPropertyName("monaco-previewer-toggle-setting")] diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/PowerPreviewPage.xaml b/src/settings-ui/Settings.UI/SettingsXAML/Views/PowerPreviewPage.xaml index 85f27ce030..81999a16ba 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/PowerPreviewPage.xaml +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/PowerPreviewPage.xaml @@ -132,7 +132,7 @@ - - + + + + + + + + + + + + + .md, .markdown, .mdown, .mkdn, .mkd, .mdwn, .mdtxt, .mdtext {Locked} + + Show local images + + + Display images stored in the Markdown document's folder, including images on the same network share, in the preview. Remote/online images remain blocked. + Source code files (Monaco) File type, do not translate diff --git a/src/settings-ui/Settings.UI/ViewModels/PowerPreviewViewModel.cs b/src/settings-ui/Settings.UI/ViewModels/PowerPreviewViewModel.cs index 75c22dd855..2f26a4bef7 100644 --- a/src/settings-ui/Settings.UI/ViewModels/PowerPreviewViewModel.cs +++ b/src/settings-ui/Settings.UI/ViewModels/PowerPreviewViewModel.cs @@ -76,6 +76,17 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels _mdRenderIsEnabled = Settings.Properties.EnableMdPreview; } + _mdLocalImagesEnabledGpoRuleConfiguration = GPOWrapper.GetConfiguredMarkdownLocalImagesEnabledValue(); + if (_mdLocalImagesEnabledGpoRuleConfiguration == GpoRuleConfigured.Disabled || _mdLocalImagesEnabledGpoRuleConfiguration == GpoRuleConfigured.Enabled) + { + _mdLocalImagesEnabledStateIsGPOConfigured = true; + _mdLocalImagesEnabled = _mdLocalImagesEnabledGpoRuleConfiguration == GpoRuleConfigured.Enabled; + } + else + { + _mdLocalImagesEnabled = Settings.Properties.EnableMdLocalImages; + } + _monacoRenderEnabledGpoRuleConfiguration = GPOWrapper.GetConfiguredMonacoPreviewEnabledValue(); if (_monacoRenderEnabledGpoRuleConfiguration == GpoRuleConfigured.Disabled || _monacoRenderEnabledGpoRuleConfiguration == GpoRuleConfigured.Enabled) { @@ -254,6 +265,9 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels private bool _mdRenderIsGpoEnabled; private bool _mdRenderIsGpoDisabled; private bool _mdRenderIsEnabled; + private GpoRuleConfigured _mdLocalImagesEnabledGpoRuleConfiguration; + private bool _mdLocalImagesEnabledStateIsGPOConfigured; + private bool _mdLocalImagesEnabled; private GpoRuleConfigured _monacoRenderEnabledGpoRuleConfiguration; private bool _monacoRenderEnabledStateIsGPOConfigured; @@ -547,6 +561,38 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels } } + public bool MDLocalImagesIsEnabled + { + get + { + return _mdLocalImagesEnabled; + } + + set + { + if (_mdLocalImagesEnabledStateIsGPOConfigured) + { + return; + } + + if (_mdLocalImagesEnabled != value) + { + _mdLocalImagesEnabled = value; + Settings.Properties.EnableMdLocalImages = value; + RaisePropertyChanged(); + } + } + } + + // Used to disable the toggle when the setting is forced by GPO (enabled or disabled). + public bool MDLocalImagesIsGPOConfigured + { + get + { + return _mdLocalImagesEnabledStateIsGPOConfigured; + } + } + public bool MonacoRenderIsEnabled { get