diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 37506ff..551daa2 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -43,7 +43,7 @@ physical directory. It does not authorize native commands or remote resource acc | Threat | Mitigation and evidence | | --- | --- | | Document script or native-command execution | Sanitizer regressions pass; host disables scripts/bridge; actual browser malicious-script and reader acceptance passed on 77dd04d | -| Image traversal, device/ADS paths or encoded separators | Core path-policy tests pass; denied before filesystem access | +| Image traversal, device/ADS paths or encoded separators | Core path-policy tests pass; denied before filesystem access. Reserved device names and forbidden characters are one written-out rule (`WindowsFileNames`), independent of the running Windows version: every device name in any letter case, with any extension, and with spaces before the extension | | Junction/symlink escape or check/reopen race | Check normalized physical path on opened image handle; read same handle; real junction rejection test passes | | Remote image tracking / resource exfiltration | No-referrer header, no document networking; actual listening-server browser denial passed on 77dd04d | | Oversized input / image denial of service | Pre-parse character bound and image byte bound; native limit tests pass; performance acceptance remains | diff --git a/src/MarkdownViewr.Windows.Core/Documents/DocumentResourcePolicy.cs b/src/MarkdownViewr.Windows.Core/Documents/DocumentResourcePolicy.cs index 7c9a983..6ea1565 100644 --- a/src/MarkdownViewr.Windows.Core/Documents/DocumentResourcePolicy.cs +++ b/src/MarkdownViewr.Windows.Core/Documents/DocumentResourcePolicy.cs @@ -19,12 +19,6 @@ public sealed class DocumentResourcePolicy ".md", ".markdown", ".mdown", ".mkd", ".txt", }; - private static readonly HashSet s_deviceNames = new(StringComparer.OrdinalIgnoreCase) - { - "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", - "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", "COM¹", "COM²", "COM³", "LPT¹", "LPT²", "LPT³", - }; - private readonly string _prefix; @@ -38,6 +32,16 @@ public sealed class DocumentResourcePolicy + /// The image file types served to the document surface. + internal static IReadOnlyCollection ImageExtensions => s_imageExtensions; + + + + /// The document file types a link may open in a new reader. + internal static IReadOnlyCollection DocumentExtensions => s_documentExtensions; + + + /// The only top-level document URI the browser host may serve. public Uri DocumentUri { get; } @@ -93,7 +97,7 @@ public sealed class DocumentResourcePolicy private static bool IsSafeSegment(string segment) { return segment.Length > 0 && segment is not ("." or "..") && !segment.EndsWith(' ') && !segment.EndsWith('.') && - !segment.Any(character => char.IsControl(character) || "<>:\\|?*".Contains(character, StringComparison.Ordinal)) && - !s_deviceNames.Contains(segment.Split('.')[0]); + !segment.Any(character => char.IsControl(character) || WindowsFileNames.InvalidCharacters.Contains(character, StringComparison.Ordinal)) && + !WindowsFileNames.IsReservedDeviceName(segment); } } diff --git a/src/MarkdownViewr.Windows.Core/Documents/MarkdownRenderer.cs b/src/MarkdownViewr.Windows.Core/Documents/MarkdownRenderer.cs index a911826..a5b1b62 100644 --- a/src/MarkdownViewr.Windows.Core/Documents/MarkdownRenderer.cs +++ b/src/MarkdownViewr.Windows.Core/Documents/MarkdownRenderer.cs @@ -59,6 +59,11 @@ public sealed class MarkdownRenderer + /// The configured sanitizer, for comparing its allowlists with an independent list. + internal HtmlSanitizer Policy => _sanitizer; + + + /// Preserves useful markup while sanitizing every parser result, including authored raw HTML. public string Render(string markdown, FrontmatterMode frontmatter = FrontmatterMode.Hide) => Render(markdown, frontmatter, null); diff --git a/src/MarkdownViewr.Windows.Core/Themes/ReaderTheme.cs b/src/MarkdownViewr.Windows.Core/Themes/ReaderTheme.cs index 3dc0eec..9c52487 100644 --- a/src/MarkdownViewr.Windows.Core/Themes/ReaderTheme.cs +++ b/src/MarkdownViewr.Windows.Core/Themes/ReaderTheme.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Text; +using System.Text.Encodings.Web; using System.Text.Json; using System.Text.Json.Nodes; @@ -15,8 +16,24 @@ namespace MarkdownViewr.Windows.Core.Themes; /// Validates Darin's version-one theme format before generating passive document CSS. public sealed class ReaderTheme { - /// Bounds schema input before a native file read allocates the complete JSON text. + /// The largest theme file accepted for import; bounds the read before the complete JSON text is allocated. public const int MaximumCharacters = 128 * 1024; + + /// + /// The largest theme the app keeps. Renaming or editing rewrites the whole theme, and JSON + /// writes some characters (emoji, no-break spaces, private-use glyphs) as six-character + /// escapes, so a kept theme can be several times the size of the file it was imported from. + /// The bound leaves room for that, so a theme that could be imported can always be renamed. + /// + public const int MaximumStoredCharacters = 8 * MaximumCharacters; + + /// + /// How a renamed or edited theme is written back. Quote marks, angle brackets and most + /// non-ASCII letters are written as themselves instead of as six-character escapes, which + /// keeps ordinary CSS and text close to its imported size. Theme JSON is stored and exported, + /// never placed in a page. + /// + private static readonly JsonSerializerOptions s_compact = new() { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping }; private static readonly string[] s_colors = ["background", "text", "heading1", "heading2", "heading3", "heading4", "heading5", "heading6", "link", "codeBackground", "codeText", "blockquoteBorder", "blockquoteBackground", "highlightBackground", "highlightText"]; private static readonly string[] s_sizes = ["baseFontSize", "h1Size", "h2Size", "h3Size", "h4Size", "h5Size", "h6Size", "codeFontSize", "lineHeight"]; private static readonly string s_baseStyles = LoadBaseStyles(); @@ -59,7 +76,7 @@ public sealed class ReaderTheme { JsonNode source = JsonNode.Parse(Json)!; source["name"] = name; - return ParseCompatible(source.ToJsonString()); + return ParseCompatible(source.ToJsonString(s_compact)); } @@ -103,7 +120,7 @@ public sealed class ReaderTheme } source["colors"]![key] = value?.Trim(); } - return ParseCompatible(source.ToJsonString()); + return ParseCompatible(source.ToJsonString(s_compact)); } @@ -150,13 +167,14 @@ public sealed class ReaderTheme private static ReaderTheme ParseSchema(string json, bool allowNewer) { ArgumentNullException.ThrowIfNull(json); - if(json.Length > MaximumCharacters) + if(json.Length > MaximumStoredCharacters) { - throw new JsonException("The theme exceeds the 128 Ki-character limit."); + throw new JsonException("The theme is too large."); } - using var document = JsonDocument.Parse(json, new JsonDocumentOptions { MaxDepth = 16 }); + using JsonDocument document = ParseDocument(json); JsonElement root = document.RootElement; RequireObject(root); + RequireReadableText(root); int schemaVersion = 1; if(root.TryGetProperty("schemaVersion", out JsonElement version) && (version.ValueKind != JsonValueKind.Number || !version.TryGetInt32(out schemaVersion) || schemaVersion < 1 || !allowNewer && schemaVersion != 1)) { @@ -278,6 +296,68 @@ public sealed class ReaderTheme + /// Reads the theme text as JSON; text that is not well-formed UTF-16 is an invalid theme like any other malformed input. + private static JsonDocument ParseDocument(string json) + { + try + { + return JsonDocument.Parse(json, new JsonDocumentOptions { MaxDepth = 16 }); + } + catch(ArgumentException exception) + { + throw new JsonException("The theme contains text that cannot be decoded.", exception); + } + } + + + + /// + /// Checks every property name and string in the theme once, including ones this version does + /// not read, because a kept theme is later renamed, edited and re-serialized as a whole. An + /// unpaired surrogate escape is valid JSON syntax with no string value, and a repeated + /// property name has no single value; both are invalid themes. + /// + private static void RequireReadableText(JsonElement root) + { + try + { + var pending = new Stack(); + pending.Push(root); + while(pending.TryPop(out JsonElement element)) + { + if(element.ValueKind == JsonValueKind.Object) + { + var names = new HashSet(StringComparer.Ordinal); + foreach(JsonProperty property in element.EnumerateObject()) + { + if(!names.Add(property.Name)) + { + throw new JsonException("The theme repeats a property: " + property.Name); + } + pending.Push(property.Value); + } + } + else if(element.ValueKind == JsonValueKind.Array) + { + foreach(JsonElement item in element.EnumerateArray()) + { + pending.Push(item); + } + } + else if(element.ValueKind == JsonValueKind.String) + { + _ = element.GetString(); + } + } + } + catch(InvalidOperationException exception) + { + throw new JsonException("The theme contains text that cannot be decoded.", exception); + } + } + + + /// Prevents wrong-shaped JSON from reaching property accessors. private static void RequireObject(JsonElement element) { diff --git a/src/MarkdownViewr.Windows.Core/Themes/ThemeJson.cs b/src/MarkdownViewr.Windows.Core/Themes/ThemeJson.cs index 4430db3..2e3ad0e 100644 --- a/src/MarkdownViewr.Windows.Core/Themes/ThemeJson.cs +++ b/src/MarkdownViewr.Windows.Core/Themes/ThemeJson.cs @@ -33,14 +33,13 @@ public static class ThemeJson public static string SuggestedFileName(ReaderTheme theme) { ArgumentNullException.ThrowIfNull(theme); - string stem = string.Concat(theme.Name.Select(character => "<>:\"/\\|?*".Contains(character, StringComparison.Ordinal) ? '_' : character)) + string stem = string.Concat(theme.Name.Select(character => WindowsFileNames.InvalidCharacters.Contains(character, StringComparison.Ordinal) ? '_' : character)) .Replace(' ', '-').ToLowerInvariant().Trim('.', '-'); if(stem.Length == 0) { stem = "theme"; } - string device = stem.Split('.')[0]; - if(device is "con" or "prn" or "aux" or "nul" || device.Length == 4 && (device.StartsWith("com", StringComparison.Ordinal) || device.StartsWith("lpt", StringComparison.Ordinal)) && device[3] is >= '1' and <= '9' or '¹' or '²' or '³') + if(WindowsFileNames.IsReservedDeviceName(stem)) { stem = "theme-" + stem; } diff --git a/src/MarkdownViewr.Windows.Core/WindowsFileNames.cs b/src/MarkdownViewr.Windows.Core/WindowsFileNames.cs new file mode 100644 index 0000000..d053552 --- /dev/null +++ b/src/MarkdownViewr.Windows.Core/WindowsFileNames.cs @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + + +namespace MarkdownViewr.Windows.Core; + +/// +/// The Windows file-name rules this application relies on, written out instead of asked of the +/// running platform: the characters a name cannot contain and the names that address a device. +/// +public static class WindowsFileNames +{ + /// Printable characters that cannot appear in a Windows file or folder name. Control characters are also forbidden; callers refuse those themselves. + public const string InvalidCharacters = "<>:\"/\\|?*"; + + private static readonly HashSet s_devices = new(StringComparer.OrdinalIgnoreCase) + { + "CON", "PRN", "AUX", "NUL", "CONIN$", "CONOUT$", + "COM0", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "COM¹", "COM²", "COM³", + "LPT0", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", "LPT¹", "LPT²", "LPT³", + }; + + + + /// + /// Whether a single name addresses a device under the legacy Win32 rule: any letter case, and + /// ignoring everything after the first dot and any spaces before it, so nul.txt and + /// COM1 .log count. Current Windows versions resolve fewer of these spellings to a + /// device; the rule is applied in full on every version so the answer never depends on it. + /// + public static bool IsReservedDeviceName(string name) + { + ArgumentNullException.ThrowIfNull(name); + int dot = name.IndexOf('.', StringComparison.Ordinal); + return s_devices.Contains((dot < 0 ? name : name[..dot]).TrimEnd(' ')); + } +} diff --git a/tests/MarkdownViewr.Windows.Core.Tests/DocumentResourcePolicyBoundaryTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/DocumentResourcePolicyBoundaryTests.cs new file mode 100644 index 0000000..f3bbed2 --- /dev/null +++ b/tests/MarkdownViewr.Windows.Core.Tests/DocumentResourcePolicyBoundaryTests.cs @@ -0,0 +1,217 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using MarkdownViewr.Windows.Core.Documents; + + +namespace MarkdownViewr.Windows.Core.Tests; + +/// +/// Each resource-policy check is exercised alone: a request that differs from a valid one in a +/// single component must be denied by that component's check, not by an unrelated one. +/// +public sealed class DocumentResourcePolicyBoundaryTests +{ + /// The valid baseline every single-component variation below is derived from. + [Fact] + public void GetRelativePath_OwnDocumentRequest_IsAccepted() + { + var policy = new DocumentResourcePolicy(); + Assert.Equal("image.png", policy.GetImageRelativePath(Vary(policy, "image.png", _ => { }))); + Assert.Equal("child.md", policy.GetDocumentRelativePath(Vary(policy, "child.md", _ => { }))); + } + + + + /// Plain HTTP on the same host, port and path is not the document's origin. + [Fact] + public void GetRelativePath_OtherScheme_IsDenied() + { + var policy = new DocumentResourcePolicy(); + Uri image = Vary(policy, "image.png", builder => + { + builder.Scheme = "http"; + builder.Port = 443; + }); + Assert.Equal(443, image.Port); + Assert.Null(policy.GetImageRelativePath(image)); + Assert.Null(policy.GetDocumentRelativePath(Vary(policy, "child.md", builder => + { + builder.Scheme = "http"; + builder.Port = 443; + }))); + } + + + + /// Another port on the same host is another origin. + [Fact] + public void GetRelativePath_OtherPort_IsDenied() + { + var policy = new DocumentResourcePolicy(); + Assert.Null(policy.GetImageRelativePath(Vary(policy, "image.png", builder => builder.Port = 444))); + Assert.Null(policy.GetDocumentRelativePath(Vary(policy, "child.md", builder => builder.Port = 8443))); + } + + + + /// Credentials in the request never reach a resource. + [Fact] + public void GetRelativePath_UserInformation_IsDenied() + { + var policy = new DocumentResourcePolicy(); + Assert.Null(policy.GetImageRelativePath(Vary(policy, "image.png", builder => builder.UserName = "user"))); + Assert.Null(policy.GetDocumentRelativePath(Vary(policy, "child.md", builder => + { + builder.UserName = "user"; + builder.Password = "secret"; + }))); + } + + + + /// Another host is denied, including a subdomain of the viewer's host and a host that merely starts with it. + [Theory] + [InlineData("example.com")] + [InlineData("sub.viewer.markdownviewr.invalid")] + [InlineData("viewer.markdownviewr.invalid.example.com")] + public void GetRelativePath_OtherHost_IsDenied(string host) + { + var policy = new DocumentResourcePolicy(); + Assert.Null(policy.GetImageRelativePath(Vary(policy, "image.png", builder => builder.Host = host))); + Assert.Null(policy.GetDocumentRelativePath(Vary(policy, "child.md", builder => builder.Host = host))); + } + + + + /// The served image types and the openable document types are exactly these sets. + [Fact] + public void Extensions_ConfiguredSets_AreExactlyTheListedTypes() + { + Assert.Equal([".avif", ".bmp", ".gif", ".ico", ".jpeg", ".jpg", ".png", ".svg", ".webp"], DocumentResourcePolicy.ImageExtensions.Order(StringComparer.Ordinal), StringComparer.Ordinal); + Assert.Equal([".markdown", ".md", ".mdown", ".mkd", ".txt"], DocumentResourcePolicy.DocumentExtensions.Order(StringComparer.Ordinal), StringComparer.Ordinal); + } + + + + /// The document's own path prefix is matched exactly; another letter case is another document. + [Fact] + public void GetRelativePath_PrefixInAnotherLetterCase_IsDenied() + { + var policy = new DocumentResourcePolicy(); + string shouted = policy.DocumentUri.AbsoluteUri.Replace("/documents/", "/DOCUMENTS/", StringComparison.Ordinal); + Assert.NotEqual(policy.DocumentUri.AbsoluteUri, shouted, StringComparer.Ordinal); + Assert.Null(policy.GetImageRelativePath(new Uri(new Uri(shouted), "image.png"))); + Assert.Null(policy.GetDocumentRelativePath(new Uri(new Uri(shouted), "child.md"))); + Assert.Null(policy.GetHeadingIndex(new Uri(shouted + "#mv-heading-0"), 3)); + Assert.Equal(0, policy.GetHeadingIndex(new Uri(policy.DocumentUri.AbsoluteUri + "#mv-heading-0"), 3)); + } + + + + /// Image types are served in any letter case; documents, pages, scripts and executables are not images. + [Theory] + [InlineData(".png", true)] + [InlineData(".jpg", true)] + [InlineData(".jpeg", true)] + [InlineData(".gif", true)] + [InlineData(".bmp", true)] + [InlineData(".webp", true)] + [InlineData(".svg", true)] + [InlineData(".avif", true)] + [InlineData(".ico", true)] + [InlineData(".PNG", true)] + [InlineData(".md", false)] + [InlineData(".markdown", false)] + [InlineData(".txt", false)] + [InlineData(".html", false)] + [InlineData(".htm", false)] + [InlineData(".js", false)] + [InlineData(".css", false)] + [InlineData(".json", false)] + [InlineData(".pdf", false)] + [InlineData(".tiff", false)] + [InlineData(".exe", false)] + [InlineData(".lnk", false)] + [InlineData(".url", false)] + [InlineData("", false)] + public void GetImageRelativePath_Extension_ServesOnlyImageTypes(string extension, bool served) + { + var policy = new DocumentResourcePolicy(); + Assert.Equal(served ? "file" + extension : null, policy.GetImageRelativePath(new Uri(policy.DocumentUri, "file" + extension))); + } + + + + /// Document types open in a new reader in any letter case; pages, images, scripts and executables do not. + [Theory] + [InlineData(".md", true)] + [InlineData(".markdown", true)] + [InlineData(".mdown", true)] + [InlineData(".mkd", true)] + [InlineData(".txt", true)] + [InlineData(".MD", true)] + [InlineData(".html", false)] + [InlineData(".htm", false)] + [InlineData(".png", false)] + [InlineData(".svg", false)] + [InlineData(".json", false)] + [InlineData(".pdf", false)] + [InlineData(".exe", false)] + [InlineData(".bat", false)] + [InlineData(".cmd", false)] + [InlineData(".ps1", false)] + [InlineData(".lnk", false)] + [InlineData(".url", false)] + [InlineData(".mdx", false)] + [InlineData("", false)] + public void GetDocumentRelativePath_Extension_OpensOnlyDocumentTypes(string extension, bool opened) + { + var policy = new DocumentResourcePolicy(); + Assert.Equal(opened ? "file" + extension : null, policy.GetDocumentRelativePath(new Uri(policy.DocumentUri, "file" + extension))); + } + + + + /// Each character Windows forbids in a name is refused on its own, for images and documents. + [Theory] + [InlineData("%3C")] + [InlineData("%3E")] + [InlineData("%3A")] + [InlineData("%22")] + [InlineData("%5C")] + [InlineData("%7C")] + [InlineData("%3F")] + [InlineData("%2A")] + public void GetRelativePath_ForbiddenCharacter_IsDenied(string encoded) + { + var policy = new DocumentResourcePolicy(); + Assert.Null(policy.GetImageRelativePath(new Uri(policy.DocumentUri, "images/a" + encoded + "b.png"))); + Assert.Null(policy.GetImageRelativePath(new Uri(policy.DocumentUri, "im" + encoded + "ages/ab.png"))); + Assert.Null(policy.GetDocumentRelativePath(new Uri(policy.DocumentUri, "docs/a" + encoded + "b.md"))); + } + + + + /// The generated heading fragment is matched exactly; another letter case is not a heading target. + [Theory] + [InlineData("#MV-HEADING-0")] + [InlineData("#Mv-Heading-0")] + public void GetHeadingIndex_OtherSpelling_IsNotAHeadingTarget(string fragment) + { + var policy = new DocumentResourcePolicy(); + Assert.Null(policy.GetHeadingIndex(new Uri(policy.DocumentUri.AbsoluteUri + fragment), 3)); + } + + + + /// Builds this document's own resource request and changes exactly one component of it. + private static Uri Vary(DocumentResourcePolicy policy, string file, Action change) + { + var builder = new UriBuilder(new Uri(policy.DocumentUri, file)); + change(builder); + return builder.Uri; + } +} diff --git a/tests/MarkdownViewr.Windows.Core.Tests/ReaderThemeTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/ReaderThemeTests.cs index 7b31b34..3d3f688 100644 --- a/tests/MarkdownViewr.Windows.Core.Tests/ReaderThemeTests.cs +++ b/tests/MarkdownViewr.Windows.Core.Tests/ReaderThemeTests.cs @@ -124,12 +124,15 @@ public sealed class ReaderThemeTests - /// Null and oversized input are rejected before parser work. + /// Null input is refused, and a theme is kept up to the stored size limit exactly and refused one character past it. [Fact] public void Parse_OutsideSizeContract_RejectsInput() { Assert.Throws(() => ReaderTheme.Parse(null!)); - Assert.Throws(() => ReaderTheme.Parse(new string(' ', 128 * 1024 + 1))); + string valid = ThemeCatalog.Find("Nord").Json; + Assert.Equal("Nord", ReaderTheme.Parse(valid.PadRight(ReaderTheme.MaximumStoredCharacters)).Name); + Assert.Throws(() => ReaderTheme.Parse(valid.PadRight(ReaderTheme.MaximumStoredCharacters + 1))); + Assert.Equal(8 * 128 * 1024, ReaderTheme.MaximumStoredCharacters); } diff --git a/tests/MarkdownViewr.Windows.Core.Tests/SanitizerAllowlistTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/SanitizerAllowlistTests.cs new file mode 100644 index 0000000..5b1e454 --- /dev/null +++ b/tests/MarkdownViewr.Windows.Core.Tests/SanitizerAllowlistTests.cs @@ -0,0 +1,220 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using AngleSharp.Dom; +using AngleSharp.Html.Dom; +using AngleSharp.Html.Parser; +using Ganss.Xss; +using MarkdownViewr.Windows.Core.Documents; + + +namespace MarkdownViewr.Windows.Core.Tests; + +/// +/// The elements, attributes and link schemes authored HTML may keep. The configured policy is +/// compared with the lists below as sets, so any change to it has to be made here as well, and +/// the rendering tests show the policy doing what the lists say. +/// +public sealed class SanitizerAllowlistTests +{ + private static readonly string[] s_allowedTags = + [ + "a", "abbr", "b", "blockquote", "br", "caption", "code", "dd", "del", "details", "div", "dl", "dt", "em", + "h1", "h2", "h3", "h4", "h5", "h6", "hr", "i", "img", "input", "ins", "kbd", "li", "mark", "ol", "p", "pre", + "s", "small", "span", "strong", "sub", "summary", "sup", "table", "tbody", "td", "th", "thead", "tr", "ul", + ]; + + private static readonly string[] s_otherTags = + [ + "acronym", "address", "applet", "area", "article", "aside", "audio", "base", "basefont", "bdi", "bdo", "bgsound", "big", + "blink", "button", "canvas", "center", "cite", "data", "datalist", "dfn", "dialog", "dir", "embed", "fencedframe", + "fieldset", "figcaption", "figure", "font", "footer", "form", "header", "hgroup", "iframe", "keygen", "label", "legend", + "link", "listing", "main", "map", "marquee", "math", "menu", "menuitem", "meta", "meter", "nav", "nobr", "noembed", + "noframes", "noscript", "object", "optgroup", "option", "output", "param", "picture", "plaintext", "portal", "progress", + "q", "rb", "rp", "rt", "rtc", "ruby", "samp", "script", "search", "section", "select", "selectedcontent", "slot", "source", + "strike", "style", "svg", "template", "textarea", "time", "title", "track", "tt", "u", "var", "video", "wbr", "xmp", + "custom-element", + ]; + + private static readonly string[] s_allowedAttributes = + [ + "align", "alt", "checked", "class", "colspan", "disabled", "height", "href", "rowspan", "src", "title", "type", "width", + ]; + + private static readonly string[] s_otherAttributes = + [ + "accesskey", "action", "allow", "as", "autocapitalize", "autofocus", "autoplay", "background", "bgcolor", "border", + "cellpadding", "cellspacing", "charset", "cite", "color", "content", "contenteditable", "controls", "coords", + "crossorigin", "data", "data-mv-body", "data-mv-source-fixed", "data-x", "datetime", "dir", "download", "draggable", + "enctype", "enterkeyhint", "face", "for", "form", "formaction", "formmethod", "formtarget", "headers", "hidden", + "http-equiv", "id", "inert", "inputmode", "integrity", "is", "ismap", "itemprop", "label", "lang", "list", "loading", + "longdesc", "loop", "lowsrc", "manifest", "max", "maxlength", "media", "method", "min", "multiple", "muted", "name", + "nonce", "novalidate", "nowrap", "onclick", "onerror", "onfocus", "onload", "onmouseover", "onpointerdown", "ontoggle", + "onwheel", "open", "part", "pattern", "ping", "placeholder", "popover", "popovertarget", "poster", "preload", "readonly", + "referrerpolicy", "rel", "required", "reversed", "role", "sandbox", "scope", "selected", "shape", "size", "sizes", "slot", + "span", "spellcheck", "srcdoc", "srclang", "srcset", "start", "step", "tabindex", "target", "translate", "usemap", + "valign", "value", "wrap", "xlink:href", "xml:lang", "xmlns", "aria-label", + ]; + + + + /// The policy the renderer configures is exactly these elements, attributes and schemes, with no styling or data attributes. + [Fact] + public void Constructor_ConfiguredPolicy_AllowsExactlyTheListedMarkup() + { + HtmlSanitizer policy = new MarkdownRenderer().Policy; + Assert.Equal(s_allowedTags.Order(StringComparer.Ordinal), policy.AllowedTags.Order(StringComparer.Ordinal), StringComparer.Ordinal); + Assert.Equal(s_allowedAttributes.Order(StringComparer.Ordinal), policy.AllowedAttributes.Order(StringComparer.Ordinal), StringComparer.Ordinal); + Assert.Equal(["http", "https", "mailto"], policy.AllowedSchemes.Order(StringComparer.Ordinal), StringComparer.Ordinal); + Assert.Empty(policy.AllowedCssProperties); + Assert.Empty(policy.AllowedAtRules); + Assert.False(policy.AllowDataAttributes); + Assert.False(policy.AllowCssCustomProperties); + } + + + + /// Every allowed element survives in a document that nests each one where HTML permits it. + [Fact] + public void Render_EveryAllowedElement_SurvivesSanitation() + { + const string Markup = """ +

abc
defghjklmnopqr

s
t

1

2

3

4

5
6
u
v
wx
  • y
  1. z
c
h
d
+ """; + using IHtmlDocument document = Parse(new MarkdownRenderer().Render(Markup)); + Assert.Equal(s_allowedTags.Order(StringComparer.Ordinal), ElementNames(document), StringComparer.Ordinal); + } + + + + /// No other element survives between allowed elements; its text may remain but the element does not. + [Fact] + public void Render_EveryOtherElement_IsRemoved() + { + var renderer = new MarkdownRenderer(); + Assert.All(s_otherTags, tag => + { + using IHtmlDocument document = Parse(renderer.Render($"
<{tag} title=\"t\">innerafter
")); + Assert.Empty(ElementNames(document).Except(["div", "span"], StringComparer.Ordinal)); + }); + Assert.Empty(s_allowedTags.Intersect(s_otherTags, StringComparer.Ordinal)); + } + + + + /// Table parts that only exist inside a table, and are not allowed, are removed there while the allowed parts stay. + [Fact] + public void Render_TablePartsOutsideTheAllowlist_AreRemovedInsideATable() + { + const string Markup = "
h
d
f
"; + using IHtmlDocument document = Parse(new MarkdownRenderer().Render(Markup)); + Assert.Empty(document.Body!.QuerySelectorAll("colgroup,col,tfoot")); + Assert.NotNull(document.Body.QuerySelector("table > thead > tr > th")); + Assert.NotNull(document.Body.QuerySelector("table > tbody > tr > td")); + } + + + + /// Exactly the allowed attributes survive on an allowed element; identity, behaviour, styling and data attributes do not. + [Fact] + public void Render_EveryAttribute_KeepsExactlyTheAllowedOnes() + { + string attributes = string.Join(' ', s_allowedAttributes.Concat(s_otherAttributes).Select(name => string.Equals(name, "class", StringComparison.Ordinal) ? "class=\"frontmatter\"" : name + "=\"1\"")) + " style=\"color:red\""; + using IHtmlDocument document = Parse(new MarkdownRenderer().Render($"
text
")); + IElement span = Assert.Single(document.Body!.QuerySelectorAll("span")); + Assert.Equal(s_allowedAttributes, span.Attributes.Select(attribute => attribute.Name).Order(StringComparer.Ordinal), StringComparer.Ordinal); + Assert.Empty(s_allowedAttributes.Intersect(s_otherAttributes, StringComparer.Ordinal)); + } + + + + /// Only the fixed styling classes and bounded code-language classes survive, spelled exactly. + [Theory] + [InlineData("frontmatter fm-key fm-value task-list task-list-item", "frontmatter fm-key fm-value task-list task-list-item")] + [InlineData("language-csharp hljs keyword", "language-csharp")] + [InlineData("LANGUAGE-csharp Language-go language-rust", "language-rust")] + [InlineData("Frontmatter FM-KEY", null)] + [InlineData("language- language-a;b language-", null)] + public void Render_ClassNames_KeepsOnlyTheStylingClasses(string authored, string? kept) + { + using IHtmlDocument document = Parse(new MarkdownRenderer().Render($"text")); + Assert.Equal(kept, Assert.Single(document.Body!.QuerySelectorAll("span")).GetAttribute("class")); + } + + + + /// A code-language class is kept up to 80 characters and dropped one character past that. + [Fact] + public void Render_CodeLanguageClassAtAndPastTheLengthLimit_IsBoundedExactly() + { + string atLimit = "language-" + new string('x', 71); + Assert.Equal(80, atLimit.Length); + using IHtmlDocument kept = Parse(new MarkdownRenderer().Render($"text")); + Assert.Equal(atLimit, Assert.Single(kept.Body!.QuerySelectorAll("span")).GetAttribute("class")); + using IHtmlDocument dropped = Parse(new MarkdownRenderer().Render($"text")); + Assert.Null(Assert.Single(dropped.Body!.QuerySelectorAll("span")).GetAttribute("class")); + } + + + + /// The attribute that carries source positions is allowed only during a source-aware render, whether that render finishes or fails. + [Fact] + public void RenderWithSources_AfterTheRender_NoLongerAllowsTheSourceAttribute() + { + var renderer = new MarkdownRenderer(); + var sources = new DocumentSourceMap("# Title"); + Assert.Contains("Title", renderer.RenderWithSources("# Title", FrontmatterMode.Hide, sources), StringComparison.Ordinal); + Assert.DoesNotContain(sources.AttributeName, renderer.Policy.AllowedAttributes); + Assert.Throws(() => renderer.RenderWithSources("# Title", (FrontmatterMode)99, sources)); + Assert.DoesNotContain(sources.AttributeName, renderer.Policy.AllowedAttributes); + } + + + + /// Only web and mail links keep a target; every other scheme, including ones Windows would hand to another application, is removed. + [Theory] + [InlineData("http://example.com/a", true)] + [InlineData("https://example.com/a", true)] + [InlineData("mailto:someone@example.com", true)] + [InlineData("child.md", true)] + [InlineData("#section", true)] + [InlineData("HTTPS://example.com/a", true)] + [InlineData("ftp://example.com/a", false)] + [InlineData("file:///C:/private.txt", false)] + [InlineData("FILE:///C:/private.txt", false)] + [InlineData("javascript:run()", false)] + [InlineData("vbscript:run()", false)] + [InlineData("data:text/html,

x

", false)] + [InlineData("blob:https://example.com/1", false)] + [InlineData("about:blank", false)] + [InlineData("ws://example.com", false)] + [InlineData("tel:5551234", false)] + [InlineData("sms:5551234", false)] + [InlineData("ms-settings:defaultapps", false)] + [InlineData("ms-msdt:/id", false)] + [InlineData("ms-word:ofe|u|https://example.com/a.docx", false)] + [InlineData("search-ms:query=x", false)] + [InlineData("shell:startup", false)] + [InlineData("res://ieframe.dll/x", false)] + [InlineData("view-source:https://example.com", false)] + [InlineData("onenote:x", false)] + public void Render_LinkScheme_KeepsOnlyWebAndMailTargets(string target, bool kept) + { + using IHtmlDocument document = Parse(new MarkdownRenderer().Render($"link")); + IElement link = Assert.Single(document.Body!.QuerySelectorAll("a")); + Assert.Equal(kept, link.HasAttribute("href")); + } + + + + /// The distinct element names in the sanitized body, in ordinal order. + private static string[] ElementNames(IHtmlDocument document) + => document.Body!.QuerySelectorAll("*").Select(element => element.LocalName).Distinct(StringComparer.Ordinal).Order(StringComparer.Ordinal).ToArray(); + + + + /// Parses renderer output the way the browser surface does. + private static IHtmlDocument Parse(string html) => new HtmlParser().ParseDocument("" + html + ""); +} diff --git a/tests/MarkdownViewr.Windows.Core.Tests/SecurityFuzzTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/SecurityFuzzTests.cs index 9be1570..15b3d42 100644 --- a/tests/MarkdownViewr.Windows.Core.Tests/SecurityFuzzTests.cs +++ b/tests/MarkdownViewr.Windows.Core.Tests/SecurityFuzzTests.cs @@ -35,6 +35,10 @@ public sealed class SecurityFuzzTests "[section](#heading) ![image](images/hello%20world.png)\n", "| A | B |\n|---|---|\n| x | y |\n", "- [x] Task\n> Quote\n", "😀 café\0\t\r\n", "", "", "", "<script>literal</script>", "\"'<>:=[]()\\/!?%", + "", + "x", + "", + "x", ]; @@ -242,11 +246,12 @@ public sealed class SecurityFuzzTests /// Uses the parsed DOM's executable capabilities as the oracle, not the renderer's private allowlist. private static void AssertPassive(IHtmlDocument document) { - Assert.Empty(document.QuerySelectorAll("script,svg,math,iframe,object,embed,form,button,style,base,meta[http-equiv],audio,video")); + Assert.Empty(document.QuerySelectorAll("script,svg,math,iframe,object,embed,form,button,style,base,meta,link,audio,video,source,param,textarea,select,option")); foreach(IElement element in document.QuerySelectorAll("*")) { Assert.DoesNotContain(element.Attributes, attribute => attribute.Name.StartsWith("on", StringComparison.OrdinalIgnoreCase)); Assert.False(element.HasAttribute("srcdoc") || element.HasAttribute("srcset") || element.HasAttribute("style")); + Assert.False(element.HasAttribute("target") || element.HasAttribute("ping") || element.HasAttribute("name") || element.HasAttribute("autofocus")); foreach(string name in new[] { "href", "src" }) { string? value = element.GetAttribute(name); diff --git a/tests/MarkdownViewr.Windows.Core.Tests/UntrustedInputValidationTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/UntrustedInputValidationTests.cs new file mode 100644 index 0000000..1840f31 --- /dev/null +++ b/tests/MarkdownViewr.Windows.Core.Tests/UntrustedInputValidationTests.cs @@ -0,0 +1,431 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using System.Text.Json; +using System.Text.Json.Nodes; +using AngleSharp.Dom; +using AngleSharp.Html.Dom; +using AngleSharp.Html.Parser; +using MarkdownViewr.Windows.Core.Documents; +using MarkdownViewr.Windows.Core.Editors; +using MarkdownViewr.Windows.Core.Printing; +using MarkdownViewr.Windows.Core.Themes; + + +namespace MarkdownViewr.Windows.Core.Tests; + +/// +/// Validation of text that comes from a document, a settings file, an imported theme or the +/// browser surface: each check is pinned by an input that only that check rejects. +/// +public sealed class UntrustedInputValidationTests +{ + /// Metadata keys and values are display text: markup in either is escaped, never parsed. + [Fact] + public void RenderMetadata_MarkupInKeyAndValue_IsEscapedText() + { + const string Key = "key"; + const string Value = " & \"quoted\""; + using IHtmlDocument parsed = new HtmlParser().ParseDocument(DocumentFrontmatter.RenderMetadata(Key + ": " + Value)); + Assert.Equal(Key, Assert.Single(parsed.QuerySelectorAll("th.fm-key")).TextContent); + Assert.Equal(Value, Assert.Single(parsed.QuerySelectorAll("td.fm-value")).TextContent); + Assert.Empty(parsed.QuerySelectorAll("b,img")); + } + + + + /// A line that starts with its colon still has a key cell, so the table keeps two columns. + [Fact] + public void RenderMetadata_LeadingColon_KeepsAnEmptyKeyCell() + { + using IHtmlDocument parsed = new HtmlParser().ParseDocument(DocumentFrontmatter.RenderMetadata(": value")); + Assert.Equal("", Assert.Single(parsed.QuerySelectorAll("th.fm-key")).TextContent); + IElement value = Assert.Single(parsed.QuerySelectorAll("td.fm-value")); + Assert.Equal("value", value.TextContent); + Assert.False(value.HasAttribute("colspan")); + } + + + + /// The private source marker is new for every render, so an authored attribute cannot predict it. + [Fact] + public void AttributeName_EachSourceMap_IsUnpredictable() + { + string first = new DocumentSourceMap("# Title").AttributeName; + string second = new DocumentSourceMap("# Title").AttributeName; + Assert.Matches("^data-mv-source-[0-9a-f]{32}$", first); + Assert.Matches("^data-mv-source-[0-9a-f]{32}$", second); + Assert.NotEqual(first, second, StringComparer.Ordinal); + } + + + + /// Raw HTML headings map to their source at every level, including the sixth. + [Theory] + [InlineData(1)] + [InlineData(5)] + [InlineData(6)] + public void SourceHeadings_RawHtmlHeadingLevel_MapsItsSource(int level) + { + string heading = $"Raw"; + string source = "Intro\n\n" + heading + "\n\nAfter\n"; + var outline = new DocumentOutline(source); + Assert.Equal(level, Assert.Single(outline.Headings).Level); + DocumentSourceHeading mapped = Assert.Single(outline.SourceHeadings); + Assert.Equal(heading, source[mapped.Start..mapped.End]); + } + + + + /// A level the HTML standard does not define is not a heading and gets no source mapping. + [Fact] + public void SourceHeadings_RawHtmlLevelSeven_IsNotAHeading() + { + var outline = new DocumentOutline("Not a heading\n"); + Assert.Empty(outline.Headings); + Assert.Empty(outline.SourceHeadings); + } + + + + /// Loading repairs every stored list and object, not only the ones the window happens to read first. + [Fact] + public void Sanitize_CorruptPrintEditorsAndThemeLists_RepairsEachProperty() + { + var valid = new EditorConfiguration(Guid.NewGuid(), "Editor", @"C:\Editor\editor.exe"); + var settings = new AppSettings + { + Print = new PrintOptions(Theme: "Bad\u0001Theme", PaddingPixels: 5000), + ConfiguredEditors = [valid, new EditorConfiguration(Guid.NewGuid(), "Script", @"C:\Editor\run.cmd"), new EditorConfiguration(Guid.Empty, "Empty", @"C:\Editor\other.exe")], + DisabledThemes = ["Nord", "Nord", "Bad\u0001Name", ""], + DeletedBuiltInThemes = ["Dracula", "Dracula", "Bad\u0002Name"], + ThemeOrder = ["Nord", "Nord", "Bad\u0003Name"], + DocumentTheme = "Bad\u0001Theme", + }; + settings.Sanitize(); + Assert.Equal("Clean Printing", settings.Print.Theme); + Assert.Equal(96, settings.Print.PaddingPixels); + Assert.Equal([valid], settings.ConfiguredEditors); + Assert.Equal(valid.Executable, settings.ExternalEditor); + Assert.Equal(["Nord"], settings.DisabledThemes); + Assert.Equal(["Dracula"], settings.DeletedBuiltInThemes); + Assert.Equal(["Nord"], settings.ThemeOrder); + Assert.Equal("Catppuccin Mocha", settings.DocumentTheme); + } + + + + /// A missing print object, as an older file or a hand edit leaves it, becomes the defaults. + [Fact] + public void Sanitize_MissingPrintObject_UsesDefaults() + { + var settings = new AppSettings { Print = null! }; + settings.Sanitize(); + Assert.Equal(new PrintOptions(), settings.Print); + } + + + + /// Editing a theme cannot silently raise it to a newer schema; that needs the import confirmation. + [Fact] + public void SaveEdited_SchemaVersionRaised_RequiresConfirmation() + { + ReaderTheme original = ThemeCatalog.Find(null); + JsonNode raised = JsonNode.Parse(original.Json)!; + raised["schemaVersion"] = 2; + var edited = ReaderTheme.ParseCompatible(raised.ToJsonString()); + Assert.Equal(1, original.SchemaVersion); + Assert.Equal(2, edited.SchemaVersion); + Assert.Throws(() => ThemeManagement.SaveEdited(new AppSettings(), original, edited)); + } + + + + /// An already confirmed newer-schema theme stays editable at its own version. + [Fact] + public void SaveEdited_SameNewerSchema_IsSaved() + { + JsonNode newer = JsonNode.Parse(ThemeCatalog.Find(null).Json)!; + newer["schemaVersion"] = 2; + newer["name"] = "Owned future"; + var theme = ReaderTheme.ParseCompatible(newer.ToJsonString()); + AppSettings imported = ThemeManagement.Import(new AppSettings(), theme, approveNewerSchema: true); + AppSettings saved = ThemeManagement.SaveEdited(imported, theme, theme); + Assert.Equal("Owned future", saved.DocumentTheme); + Assert.Equal(2, ThemeManagement.Active(saved).SchemaVersion); + } + + + + /// A copy of a theme whose name is already at the limit still has a name the theme parser accepts. + [Fact] + public void CopyTheme_LongestNameTakenTwice_StaysWithinTheNameLimit() + { + ReaderTheme longest = ThemeCatalog.Find(null).Rename(new string('N', 80)); + AppSettings settings = ThemeManagement.Import(new AppSettings(), longest); + ReaderTheme firstCopy = ThemeManagement.CopyTheme(settings, longest); + settings = ThemeManagement.Import(settings, firstCopy); + ReaderTheme secondCopy = ThemeManagement.CopyTheme(settings, longest); + Assert.Equal(80, firstCopy.Name.Length); + Assert.Equal(80, secondCopy.Name.Length); + Assert.EndsWith(" (2)", secondCopy.Name, StringComparison.Ordinal); + Assert.Equal(secondCopy.Name, ReaderTheme.ParseCompatible(secondCopy.Json).Name); + } + + + + /// Editor and document paths are accepted up to the Windows limit and refused one character past it. + [Fact] + public void IsExecutablePath_PathAtAndPastTheWindowsLimit_IsBoundedExactly() + { + string atLimit = @"C:\" + new string('e', 32767 - 7) + ".exe"; + string pastLimit = @"C:\" + new string('e', 32768 - 7) + ".exe"; + Assert.Equal(32767, atLimit.Length); + Assert.True(EditorLaunch.IsExecutablePath(atLimit)); + Assert.False(EditorLaunch.IsExecutablePath(pastLimit)); + string document = @"C:\" + new string('d', 32767 - 6) + ".md"; + Assert.Equal(32767, document.Length); + Assert.Equal(document, Assert.Single(EditorLaunch.Create(@"C:\Editor\editor.exe", document).ArgumentList)); + Assert.Throws(() => EditorLaunch.Create(@"C:\Editor\editor.exe", document + "x")); + } + + + + /// Recent paths are kept up to the Windows limit and dropped one character past it. + [Fact] + public void Sanitize_RecentPathAtAndPastTheWindowsLimit_IsBoundedExactly() + { + string atLimit = @"C:\" + new string('d', 32767 - 6) + ".md"; + Assert.Equal([atLimit], RecentDocuments.Sanitize([atLimit, atLimit + "x"])); + } + + + + /// A find term is accepted at its limit and refused one character past it. + [Fact] + public void Constructor_FindTermAtAndPastTheLimit_IsBoundedExactly() + { + string atLimit = new('q', DocumentFind.MaximumQueryCharacters); + Assert.Equal(atLimit, new DocumentFind(atLimit + " tail", atLimit).Query); + Assert.Throws(() => new DocumentFind("source", atLimit + "q")); + } + + + + /// The viewport result is read up to its size cap and ignored past it, even when it would parse. + [Fact] + public void FromBrowserResult_ViewportAtAndPastTheSizeCap_IsBoundedExactly() + { + string atCap = "[0.25," + new string(' ', 128 - 11) + "0.75]"; + Assert.Equal(128, atCap.Length); + Assert.Equal(0.75, DocumentViewport.FromBrowserResult(atCap).VerticalFraction); + Assert.Equal(default, DocumentViewport.FromBrowserResult(atCap.Replace("[", "[ ", StringComparison.Ordinal))); + } + + + + /// The scroll-state result is read up to its size cap for the heading count and ignored past it. + [Theory] + [InlineData(0)] + [InlineData(3)] + public void FromBrowserResult_ScrollStateAtAndPastTheSizeCap_IsBoundedExactly(int headings) + { + int cap = headings * 128 + 512; + string atCap = "[1,2,[],[]" + new string(' ', cap - 11) + "]"; + Assert.Equal(cap, atCap.Length); + Assert.NotNull(DocumentScrollState.FromBrowserResult(atCap, headings)); + Assert.Null(DocumentScrollState.FromBrowserResult(atCap.Replace("[1,", "[ 1,", StringComparison.Ordinal), headings)); + } + + + + /// The scroll-state result has exactly four parts; a longer array is not the application's own query result. + [Theory] + [InlineData("[1,2,[]]")] + [InlineData("[1,2,[],[],[]]")] + [InlineData("[1,2,[],[],0]")] + public void FromBrowserResult_ScrollStateWithWrongNumberOfParts_IsRejected(string json) + => Assert.Null(DocumentScrollState.FromBrowserResult(json, 2)); + + + + /// Print styles are built from repaired options, so a corrupt padding value cannot reach the stylesheet. + [Theory] + [InlineData(double.NaN, "padding:0px")] + [InlineData(double.PositiveInfinity, "padding:0px")] + [InlineData(5000, "padding:96px")] + [InlineData(-3, "padding:0px")] + public void Build_CorruptPadding_UsesTheRepairedValue(double padding, string expected) + { + ReaderTheme theme = ThemeCatalog.Find(null); + string css = PrintStyles.Build(new PrintOptions(PaddingPixels: padding), theme, [theme], 1); + Assert.Contains(expected, css, StringComparison.Ordinal); + Assert.DoesNotContain("NaN", css, StringComparison.Ordinal); + Assert.DoesNotContain("Infinity", css, StringComparison.Ordinal); + Assert.DoesNotContain("∞", css, StringComparison.Ordinal); + } + + + + /// + /// Text the JSON reader cannot decode, an unpaired surrogate escape, makes the theme invalid + /// wherever it appears: in a value the theme reads, in a property name, or in a value this + /// version ignores but would later re-serialize. So does a repeated property name. + /// + [Theory] + [InlineData("\"name\": \"Nord\"", "\"name\": \"\\uD83D\"")] + [InlineData("\"background\": \"#2e3440\"", "\"background\": \"\\uD800\"")] + [InlineData("\"body\": \"System\"", "\"body\": \"A\\uDC00B\"")] + [InlineData("\"name\": \"Nord\"", "\"name\": \"Nord\", \"\\uD83D\": 1")] + [InlineData("\"name\": \"Nord\"", "\"name\": \"Nord\", \"note\": \"\\uD83D\"")] + [InlineData("\"name\": \"Nord\"", "\"name\": \"Nord\", \"notes\": [1, [\"ok\", {\"deep\": \"\\uDFFF\"}]]")] + [InlineData("\"body\": \"System\"", "\"body\": \"System\", \"\\uD800x\": \"y\"")] + [InlineData("\"name\": \"Nord\"", "\"name\": \"Nord\", \"name\": \"Nord\"")] + [InlineData("\"body\": \"System\"", "\"body\": \"System\", \"body\": \"Other\"")] + [InlineData("\"name\": \"Nord\"", "\"name\": \"Nord\", \"extra\": {\"a\": 1, \"a\": 2}")] + [InlineData("\"name\": \"Nord\"", "\"name\": \"Nord\", \"na\\u006de\": \"Nord\"")] + [InlineData("\"name\": \"Nord\"", "\"name\": \"Nord\", \"notes\": [\"\\uD83D\"]")] + public void Parse_UndecodableTextOrRepeatedProperty_IsRejectedAsAnInvalidTheme(string valid, string invalid) + { + string original = ThemeCatalog.Find("Nord").Json; + Assert.Contains(valid, original, StringComparison.Ordinal); + string json = original.Replace(valid, invalid, StringComparison.Ordinal); + Assert.Throws(() => ReaderTheme.Parse(json)); + Assert.Throws(() => ReaderTheme.ParseCompatible(json)); + var settings = new AppSettings { UserThemes = [json, original] }; + settings.Sanitize(); + Assert.Equal([original], settings.UserThemes); + } + + + + /// + /// A theme that parses can be renamed, copied, edited and imported under a new name with its + /// extra properties intact: those operations re-serialize the whole document. Names that differ + /// only in case, and the same name in two different objects, are not repeats. + /// + [Fact] + public void Parse_AcceptedThemeWithExtraProperties_SurvivesRenameCopyEditAndImport() + { + string json = ThemeCatalog.Find("Nord").Json.Replace("\"name\": \"Nord\"", "\"name\": \"Nord\", \"Name\": \"other case\", \"note\": \"caf\\u00e9 \\ud83d\\ude00\", \"future\": {\"name\": \"inner\", \"list\": [1, \"two\", null]}", StringComparison.Ordinal); + var theme = ReaderTheme.Parse(json); + Assert.Equal("Renamed", theme.Rename("Renamed").Name); + AppSettings imported = ThemeManagement.Import(new AppSettings(), theme); + Assert.Contains(ThemeManagement.All(imported), item => item.Name.StartsWith("Nord (", StringComparison.Ordinal)); + Assert.EndsWith(" Copy", ThemeManagement.CopyTheme(imported, theme).Name, StringComparison.Ordinal); + ReaderTheme edited = theme.Edit("Edited", new Dictionary(StringComparer.Ordinal), new Dictionary(StringComparer.Ordinal), new Dictionary(StringComparer.Ordinal), ""); + Assert.Equal("Edited", edited.Name); + Assert.Contains("caf", edited.Json, StringComparison.Ordinal); + Assert.Contains("\"Name\":\"other case\"", edited.Json, StringComparison.Ordinal); + Assert.Contains("\"name\":\"inner\"", edited.Json, StringComparison.Ordinal); + } + + + + /// + /// Renaming, copying and editing write characters as themselves, so a theme near the size + /// limit whose text is full of quote marks, angle brackets or non-ASCII characters stays + /// within the limit instead of growing sixfold and being refused. + /// + [Theory] + [InlineData("a>b+c{font:'x'}")] + [InlineData("/* \u6f22\u5b57 caf\u00e9 & */")] + public void Rename_ThemeNearTheSizeLimit_DoesNotGrowPastIt(string fragment) + { + string css = string.Concat(Enumerable.Repeat(fragment, 60_000 / fragment.Length)); + string original = ThemeCatalog.Find("Nord").Json; + Assert.Contains("\"name\": \"Nord\"", original, StringComparison.Ordinal); + string json = original.Replace("\"name\": \"Nord\"", "\"name\": \"Nord\", \"note\": \"" + css + "\"", StringComparison.Ordinal); + var theme = ReaderTheme.Parse(json); + ReaderTheme renamed = theme.Rename("Renamed"); + Assert.True(renamed.Json.Length <= json.Length); + Assert.Contains(fragment, renamed.Json, StringComparison.Ordinal); + ReaderTheme edited = renamed.Edit("Edited", new Dictionary(StringComparer.Ordinal), new Dictionary(StringComparer.Ordinal), new Dictionary(StringComparer.Ordinal), ""); + Assert.True(edited.Json.Length <= json.Length); + Assert.EndsWith(" Copy", ThemeManagement.CopyTheme(new AppSettings(), theme).Name, StringComparison.Ordinal); + } + + + + /// + /// JSON writes emoji, no-break and ideographic spaces and private-use glyphs as six-character + /// escapes whatever the encoder. A theme as large as an import may be, made of nothing else, + /// can still be renamed, copied and edited: the stored limit leaves room for the growth. + /// + [Theory] + [InlineData("😀")] + [InlineData(" ")] + [InlineData("　")] + [InlineData("")] + public void Rename_ThemeAtTheImportLimitMadeOfEscapedCharacters_IsKept(string character) + { + string original = ThemeCatalog.Find("Nord").Json; + string opening = "\"name\": \"Nord\", \"note\": \""; + int room = ReaderTheme.MaximumCharacters - original.Length - (opening.Length - "\"name\": \"Nord\"".Length) - 1; + string filler = string.Concat(Enumerable.Repeat(character, room / character.Length)).PadRight(room, 'x'); + string json = original.Replace("\"name\": \"Nord\"", opening + filler + "\"", StringComparison.Ordinal); + Assert.Equal(ReaderTheme.MaximumCharacters, json.Length); + var theme = ReaderTheme.Parse(json); + ReaderTheme renamed = theme.Rename("A considerably longer name than the one it had"); + Assert.InRange(renamed.Json.Length, json.Length + 1, ReaderTheme.MaximumStoredCharacters); + Assert.Equal(renamed.Json.Length, renamed.Rename(renamed.Name).Json.Length); + Assert.EndsWith(" Copy", ThemeManagement.CopyTheme(new AppSettings(), renamed).Name, StringComparison.Ordinal); + ReaderTheme edited = renamed.Edit("Edited", new Dictionary(StringComparer.Ordinal), new Dictionary(StringComparer.Ordinal), new Dictionary(StringComparer.Ordinal), "body { color: red }"); + Assert.Equal("Edited", edited.Name); + var settings = new AppSettings { UserThemes = [edited.Json] }; + settings.Sanitize(); + Assert.Equal([edited.Json], settings.UserThemes); + } + + + + /// A string that is not well-formed UTF-16 is refused as an invalid theme, and a stored one is dropped, rather than failing as a programming error. + [Fact] + public void Parse_TextWithAnUnpairedSurrogate_IsRejectedAsAnInvalidTheme() + { + string original = ThemeCatalog.Find("Nord").Json; + string json = original.Replace("\"name\": \"Nord\"", "\"name\": \"No\uD83Drd\"", StringComparison.Ordinal); + Assert.DoesNotContain("\"name\": \"Nord\"", json, StringComparison.Ordinal); + Assert.Throws(() => ReaderTheme.Parse(json)); + Assert.Throws(() => ReaderTheme.ParseCompatible(json)); + var settings = new AppSettings { UserThemes = [json, original] }; + settings.Sanitize(); + Assert.Equal([original], settings.UserThemes); + } + + + + /// Only the exact lower-case channel name selects the development identity. + [Theory] + [InlineData("DEV")] + [InlineData("Dev")] + [InlineData("dev ")] + [InlineData("development")] + public void ForChannel_OtherSpellingOfDev_KeepsTheStableIdentity(string channel) + => Assert.Equal("markdownviewr-windows", AppIdentity.ForChannel(channel).Id); + + + + /// A colour that is not six hexadecimal digits after a hash, or a label containing a control character, is refused. + [Theory] + [InlineData("colors.background", "\";ffffff\"")] + [InlineData("colors.background", "\"#fffffff\"")] + [InlineData("colors.background", "\"#ffffffff\"")] + [InlineData("colors.background", "\"#fffff\"")] + [InlineData("colors.background", "\"ffffff#\"")] + [InlineData("colors.text", "\"#12345g\"")] + [InlineData("name", "\"Theme\\u0001Name\"")] + [InlineData("name", "\"Theme\\u007fName\"")] + [InlineData("fonts.heading", "\"Font\\u0001Name\"")] + [InlineData("fonts.code", "\"Font\\u009bName\"")] + public void Parse_MalformedColorOrLabel_IsRejected(string path, string json) + { + JsonNode root = JsonNode.Parse(ThemeCatalog.Find(null).Json)!; + string[] parts = path.Split('.'); + JsonNode parent = parts.Length == 1 ? root : root[parts[0]]!; + parent[parts[^1]] = JsonNode.Parse(json); + Assert.Throws(() => ReaderTheme.Parse(root.ToJsonString())); + } +} diff --git a/tests/MarkdownViewr.Windows.Core.Tests/WindowsFileNamesTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/WindowsFileNamesTests.cs new file mode 100644 index 0000000..f6637da --- /dev/null +++ b/tests/MarkdownViewr.Windows.Core.Tests/WindowsFileNamesTests.cs @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using MarkdownViewr.Windows.Core.Documents; +using MarkdownViewr.Windows.Core.Themes; + + +namespace MarkdownViewr.Windows.Core.Tests; + +/// The written-out Windows name rules, and the two places that depend on them. +public sealed class WindowsFileNamesTests +{ + private static readonly string[] s_devices = + [ + "CON", "PRN", "AUX", "NUL", "CONIN$", "CONOUT$", + "COM0", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "COM¹", "COM²", "COM³", + "LPT0", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", "LPT¹", "LPT²", "LPT³", + ]; + + + + /// A device is a device in any letter case, with any extension, and with spaces before the extension or at the end. + [Fact] + public void IsReservedDeviceName_EveryDeviceSpelling_IsReserved() + { + Assert.All(Spellings(), spelling => + { + Assert.True(WindowsFileNames.IsReservedDeviceName(spelling), spelling); + Assert.True(WindowsFileNames.IsReservedDeviceName(spelling + ".png"), spelling); + Assert.True(WindowsFileNames.IsReservedDeviceName(spelling + ".tar.md"), spelling); + Assert.True(WindowsFileNames.IsReservedDeviceName(spelling + " .png"), spelling); + Assert.True(WindowsFileNames.IsReservedDeviceName(spelling + " "), spelling); + }); + Assert.Equal(96, Spellings().Count()); + } + + + + /// Names that only resemble a device stay ordinary file names; only spaces are ignored before the extension. + [Theory] + [InlineData("")] + [InlineData("console.md")] + [InlineData("COM10.png")] + [InlineData("COM")] + [InlineData("LPT.txt")] + [InlineData("xCON.png")] + [InlineData("CON-notes.md")] + [InlineData("CON-.md")] + [InlineData("null.md")] + [InlineData(" NUL.png")] + [InlineData("NUL\u00A0.png")] + [InlineData("NUL\t.png")] + [InlineData("readme.NUL")] + [InlineData("COM⁴.png")] + [InlineData("CLOCK$")] + [InlineData("CLOCK$.txt")] + public void IsReservedDeviceName_OrdinaryName_IsNotReserved(string name) => Assert.False(WindowsFileNames.IsReservedDeviceName(name)); + + + + /// A missing name is a contract error, not an ordinary file. + [Fact] + public void IsReservedDeviceName_Null_RejectsInput() => Assert.Throws(() => WindowsFileNames.IsReservedDeviceName(null!)); + + + + /// The printable characters Windows forbids in a name; control characters are refused separately by each caller. + [Fact] + public void InvalidCharacters_Constant_ListsThePrintableForbiddenCharacters() => Assert.Equal("\"*/:<>?\\|", new string(WindowsFileNames.InvalidCharacters.Order().ToArray())); + + + + /// No image or linked-document request can name a device in any spelling, as the file or as a folder on the way to it. + [Fact] + public void GetRelativePath_DeviceSegment_IsDenied() + { + var policy = new DocumentResourcePolicy(); + Assert.All(Spellings(), spelling => + { + string segment = Uri.EscapeDataString(spelling); + Assert.Null(policy.GetImageRelativePath(new Uri(policy.DocumentUri, segment + ".png"))); + Assert.Null(policy.GetImageRelativePath(new Uri(policy.DocumentUri, segment + "%20.png"))); + Assert.Null(policy.GetImageRelativePath(new Uri(policy.DocumentUri, segment + "/picture.png"))); + Assert.Null(policy.GetDocumentRelativePath(new Uri(policy.DocumentUri, segment + ".md"))); + Assert.Null(policy.GetDocumentRelativePath(new Uri(policy.DocumentUri, segment + ".tar.md"))); + Assert.Null(policy.GetDocumentRelativePath(new Uri(policy.DocumentUri, "docs/" + segment + "/child.md"))); + }); + Assert.Equal("console.png", policy.GetImageRelativePath(new Uri(policy.DocumentUri, "console.png"))); + Assert.Equal("docs/com10/child.md", policy.GetDocumentRelativePath(new Uri(policy.DocumentUri, "docs/com10/child.md"))); + } + + + + /// A theme named after a device, in any case, never becomes the suggested export file name. + [Fact] + public void SuggestedFileName_EveryDeviceName_IsPrefixed() + { + Assert.All(s_devices, device => + { + Assert.Equal("theme-" + device.ToLowerInvariant() + ".json", ThemeJson.SuggestedFileName(ThemeCatalog.Find(null).Rename(device))); + Assert.Equal("theme-" + device.ToLowerInvariant() + ".json", ThemeJson.SuggestedFileName(ThemeCatalog.Find(null).Rename(device.ToLowerInvariant()))); + Assert.Equal("theme-" + device.ToLowerInvariant() + ".backup.json", ThemeJson.SuggestedFileName(ThemeCatalog.Find(null).Rename(device + ".backup"))); + }); + } + + + + /// Leading and trailing dots and hyphens are dropped from a suggested name, and a name made only of them falls back to a plain one. + [Theory] + [InlineData("-Edge-", "edge.json")] + [InlineData(".Dotted.", "dotted.json")] + [InlineData("--", "theme.json")] + [InlineData(" spaced ", "spaced.json")] + [InlineData("-con-", "theme-con.json")] + public void SuggestedFileName_LeadingAndTrailingSeparators_AreTrimmed(string name, string expected) + => Assert.Equal(expected, ThemeJson.SuggestedFileName(ThemeCatalog.Find(null).Rename(name))); + + + + /// Each device name in upper, lower and mixed case. + private static IEnumerable Spellings() + => s_devices.SelectMany(device => new[] { device, device.ToLowerInvariant(), char.ToUpperInvariant(device[0]) + device[1..].ToLowerInvariant() }); +}