Skip to content

Commit 0bcb0f4

Browse files
committed
Removed HTML since we don't want navigation in the app
1 parent 78ec07b commit 0bcb0f4

8 files changed

Lines changed: 194 additions & 52 deletions

File tree

src/EventLogExpert.UI.Tests/Services/ReleaseNotesMarkdownRendererTests.cs

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,24 @@ namespace EventLogExpert.UI.Tests.Services;
77

88
public sealed class ReleaseNotesMarkdownRendererTests
99
{
10+
[Theory]
11+
[InlineData("[evil](javascript:alert(1))")]
12+
[InlineData("[evil](data:text/html,<script>)")]
13+
[InlineData("[evil](file:///etc/passwd)")]
14+
[InlineData("[evil](ftp://example.com)")]
15+
public void RenderToHtml_AllLinkSchemes_StrippedToTextOnly(string markdown)
16+
{
17+
var html = ReleaseNotesMarkdownRenderer.RenderToHtml(markdown);
18+
19+
Assert.DoesNotContain("<a ", html);
20+
Assert.DoesNotContain("href=", html);
21+
Assert.DoesNotContain("javascript:", html);
22+
Assert.DoesNotContain("data:", html);
23+
Assert.DoesNotContain("file://", html);
24+
Assert.DoesNotContain("ftp://", html);
25+
Assert.Contains("evil", html);
26+
}
27+
1028
[Fact]
1129
public void RenderToHtml_BlankLineBetweenBullets_StartsNewList()
1230
{
@@ -106,19 +124,25 @@ public void RenderToHtml_Headings(string markdown, string expected)
106124
}
107125

108126
[Fact]
109-
public void RenderToHtml_HttpLink_RendersAnchor()
127+
public void RenderToHtml_HttpLink_RendersTextOnlyWithoutAnchor()
110128
{
111129
var html = ReleaseNotesMarkdownRenderer.RenderToHtml("[link](http://example.com)");
112130

113-
Assert.Contains("<a href=\"http://example.com\"", html);
131+
Assert.DoesNotContain("<a ", html);
132+
Assert.DoesNotContain("href=", html);
133+
Assert.DoesNotContain("http://example.com", html);
134+
Assert.Contains("link", html);
114135
}
115136

116137
[Fact]
117-
public void RenderToHtml_HttpsLink_RendersAnchor()
138+
public void RenderToHtml_HttpsLink_RendersTextOnlyWithoutAnchor()
118139
{
119140
var html = ReleaseNotesMarkdownRenderer.RenderToHtml("see [docs](https://example.com/page)");
120141

121-
Assert.Contains("<a href=\"https://example.com/page\" target=\"_blank\" rel=\"noopener noreferrer\">docs</a>", html);
142+
Assert.DoesNotContain("<a ", html);
143+
Assert.DoesNotContain("href=", html);
144+
Assert.DoesNotContain("https://example.com/page", html);
145+
Assert.Contains("see docs", html);
122146
}
123147

124148
[Fact]
@@ -185,14 +209,14 @@ public void RenderToHtml_PlainTextLines_RenderAsParagraph()
185209
[Theory]
186210
[InlineData("[xss](https://example.com\"onload=alert(1))")]
187211
[InlineData("[xss](https://example.com'onclick='alert(1))")]
188-
public void RenderToHtml_QuotesInUrl_AreEscapedNotInjected(string markdown)
212+
public void RenderToHtml_QuotesInUrl_StrippedAlongWithUrl(string markdown)
189213
{
190214
var html = ReleaseNotesMarkdownRenderer.RenderToHtml(markdown);
191215

192-
Assert.DoesNotContain("\" onload=", html);
193-
Assert.DoesNotContain("\"onload=", html);
194-
Assert.DoesNotContain("\" onclick=", html);
195-
Assert.DoesNotContain("'onclick=", html);
216+
Assert.DoesNotContain("onload=", html);
217+
Assert.DoesNotContain("onclick=", html);
218+
Assert.DoesNotContain("href=", html);
219+
Assert.Contains("xss", html);
196220
}
197221

198222
[Fact]
@@ -224,7 +248,9 @@ public void RenderToHtml_RichLayoutSample_RendersAllElements()
224248
Assert.Contains("<h3>Features</h3>", html);
225249
Assert.Contains("<h3>Bug Fixes</h3>", html);
226250
Assert.Contains("<strong>Column reordering</strong>", html);
227-
Assert.Contains("<a href=\"https://example.com/docs\"", html);
251+
Assert.Contains("<li>Support for exported logs</li>", html);
252+
Assert.DoesNotContain("href=", html);
253+
Assert.DoesNotContain("https://example.com", html);
228254
Assert.Contains("<li>Fixed crash when opening empty file</li>", html);
229255
}
230256

@@ -263,24 +289,13 @@ public void RenderToHtml_UnmatchedBoldMarkers_LeftAsLiteral()
263289
Assert.Contains("**unclosed bold", html);
264290
}
265291

266-
[Theory]
267-
[InlineData("[evil](javascript:alert(1))")]
268-
[InlineData("[evil](data:text/html,<script>)")]
269-
[InlineData("[evil](file:///etc/passwd)")]
270-
[InlineData("[evil](ftp://example.com)")]
271-
public void RenderToHtml_UnsafeLinkSchemes_NotRenderedAsAnchor(string markdown)
272-
{
273-
var html = ReleaseNotesMarkdownRenderer.RenderToHtml(markdown);
274-
275-
Assert.DoesNotContain("<a ", html);
276-
Assert.DoesNotContain("href=", html);
277-
}
278-
279292
[Fact]
280-
public void RenderToHtml_UrlWithAmpersand_PreservesEscapedEntity()
293+
public void RenderToHtml_UrlWithAmpersand_NotRenderedInOutput()
281294
{
282295
var html = ReleaseNotesMarkdownRenderer.RenderToHtml("[link](https://example.com/path?a=1&b=2)");
283296

284-
Assert.Contains("href=\"https://example.com/path?a=1&amp;b=2\"", html);
297+
Assert.DoesNotContain("href=", html);
298+
Assert.DoesNotContain("https://example.com", html);
299+
Assert.Contains("link", html);
285300
}
286301
}

src/EventLogExpert.UI.Tests/Services/ReleaseNotesNormalizerTests.cs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ public void Normalize_BulletWithoutCommitPrefix_ConvertsToDashStyle()
1818
Assert.Equal("- Just a description with no commit id", result);
1919
}
2020

21+
[Fact]
22+
public void Normalize_EmptySummary_RemovedWithoutEmittingHeading()
23+
{
24+
var result = ReleaseNotesNormalizer.Normalize("before<summary></summary>after");
25+
26+
Assert.DoesNotContain("##", result);
27+
Assert.Contains("before", result);
28+
Assert.Contains("after", result);
29+
}
30+
2131
[Fact]
2232
public void Normalize_LegacyFixture_ProducesCleanBullets()
2333
{
@@ -30,6 +40,37 @@ public void Normalize_LegacyFixture_ProducesCleanBullets()
3040
Assert.DoesNotContain("66b7d6883807a5c518ffcd59f92e07e528a5636a", result);
3141
}
3242

43+
[Fact]
44+
public void Normalize_LegacyFixture_PromotesSummaryTextToHeading()
45+
{
46+
var result = ReleaseNotesNormalizer.Normalize(Constants.GitHubReleaseNotes);
47+
48+
Assert.Contains("## See More", result);
49+
Assert.Contains("- Reduce unnecessary sorting", result);
50+
}
51+
52+
[Fact]
53+
public void Normalize_LegacyFixture_StripsAutoGeneratedFooter()
54+
{
55+
var result = ReleaseNotesNormalizer.Normalize(Constants.GitHubReleaseNotes);
56+
57+
Assert.DoesNotContain("This list of changes was", result);
58+
Assert.DoesNotContain("auto generated", result);
59+
}
60+
61+
[Fact]
62+
public void Normalize_LegacyFixture_StripsRawHtmlWrappers()
63+
{
64+
var result = ReleaseNotesNormalizer.Normalize(Constants.GitHubReleaseNotes);
65+
66+
Assert.DoesNotContain("<details", result);
67+
Assert.DoesNotContain("</details>", result);
68+
Assert.DoesNotContain("<summary", result);
69+
Assert.DoesNotContain("</summary>", result);
70+
Assert.DoesNotContain("<b>", result);
71+
Assert.DoesNotContain("</b>", result);
72+
}
73+
3374
[Fact]
3475
public void Normalize_LegacyShaBullet_StripsCommitId()
3576
{
@@ -79,4 +120,21 @@ public void Normalize_RichAuthoredMarkdown_PassesThroughUnchanged()
79120
Assert.Contains("- **Column reordering** with persistent sizing", result);
80121
Assert.Contains("[exported logs](https://example.com/docs)", result);
81122
}
123+
124+
[Fact]
125+
public void Normalize_RichBodyWithBoldTag_PreservesInnerTextOnly()
126+
{
127+
var result = ReleaseNotesNormalizer.Normalize("Some <b>important</b> note");
128+
129+
Assert.Equal("Some important note", result);
130+
}
131+
132+
[Fact]
133+
public void Normalize_SummaryWithNonLegacyTags_StripsTagsFromHeading()
134+
{
135+
var result = ReleaseNotesNormalizer.Normalize("<summary>Click <iframe>here</summary>");
136+
137+
Assert.Contains("## Click here", result);
138+
Assert.DoesNotContain("<iframe", result);
139+
}
82140
}

src/EventLogExpert.UI.Tests/Services/UpdateServiceTests.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,36 @@ await mockAlertDialogService.DidNotReceive()
423423
.ShowAlert(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>());
424424
}
425425

426+
[Fact]
427+
public async Task GetReleaseNotes_WithCurrentChangesButNoSubscriber_ShouldFallBackToAlert()
428+
{
429+
// Arrange
430+
var mockCurrentVersionProvider = Substitute.For<ICurrentVersionProvider>();
431+
mockCurrentVersionProvider.CurrentVersion.Returns(new Version(Constants.GitHubLatestVersion[1..]));
432+
mockCurrentVersionProvider.IsDevBuild.Returns(false);
433+
434+
var mockGitHubService = Substitute.For<IGitHubService>();
435+
mockGitHubService.GetReleases().Returns(Task.FromResult(GitHubUtils.CreateGitReleaseModels()));
436+
437+
var mockAlertDialogService = Substitute.For<IAlertDialogService>();
438+
439+
var updateService = CreateUpdateService(
440+
mockCurrentVersionProvider,
441+
gitHubService: mockGitHubService,
442+
alertDialogService: mockAlertDialogService);
443+
444+
// Act — note: no subscriber attached to ReleaseNotesReady.
445+
await updateService.CheckForUpdates(false, false);
446+
await updateService.GetReleaseNotes();
447+
448+
// Assert — without a UI subscriber the service must surface notes via the alert dialog
449+
// so the user is never left with silent no-op feedback.
450+
await mockAlertDialogService.Received(1).ShowAlert(
451+
Arg.Is<string>(title => title.Contains("Release notes")),
452+
Arg.Is<string>(body => body.Contains("Updated Azure yml to .NET 8")),
453+
"Ok");
454+
}
455+
426456
private static UpdateService CreateUpdateService(
427457
ICurrentVersionProvider? currentVersionProvider = null,
428458
IAppTitleService? appTitleService = null,

src/EventLogExpert.UI/Services/ReleaseNotesMarkdownRenderer.cs

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,6 @@ void FlushList()
135135
[GeneratedRegex(@"^(#{1,6})\s+(.+)$")]
136136
private static partial Regex HeadingRegex();
137137

138-
private static bool IsSafeUrl(string url) =>
139-
url.StartsWith("http://", StringComparison.OrdinalIgnoreCase) ||
140-
url.StartsWith("https://", StringComparison.OrdinalIgnoreCase);
141-
142138
[GeneratedRegex(@"\*([^*\n]+)\*")]
143139
private static partial Regex ItalicRegex();
144140

@@ -160,20 +156,12 @@ private static string ProcessInline(string line)
160156
return $"{CodePlaceholderSentinel}{codeSpans.Count - 1}{CodePlaceholderSentinel}";
161157
});
162158

163-
var withLinks = LinkRegex().Replace(withCodePlaceholders, match =>
164-
{
165-
var text = match.Groups[1].Value;
166-
var url = match.Groups[2].Value;
167-
168-
if (!IsSafeUrl(url))
169-
{
170-
return match.Value;
171-
}
172-
173-
return $"<a href=\"{url}\" target=\"_blank\" rel=\"noopener noreferrer\">{text}</a>";
174-
});
159+
// Render Markdown links as their visible text only. The release notes
160+
// modal is intentionally read-only; we do not want users navigating to
161+
// external URLs from inside the app.
162+
var withLinkTextOnly = LinkRegex().Replace(withCodePlaceholders, match => match.Groups[1].Value);
175163

176-
var withBold = BoldRegex().Replace(withLinks, "<strong>$1</strong>");
164+
var withBold = BoldRegex().Replace(withLinkTextOnly, "<strong>$1</strong>");
177165
var withItalic = ItalicRegex().Replace(withBold, "<em>$1</em>");
178166

179167
var restored = CodePlaceholderRegex().Replace(withItalic, match =>

src/EventLogExpert.UI/Services/ReleaseNotesNormalizer.cs

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,14 @@ namespace EventLogExpert.UI.Services;
1010
/// rendered by <see cref="ReleaseNotesMarkdownRenderer"/>.
1111
///
1212
/// Future releases are hand-authored in rich Markdown (headings, bold, lists,
13-
/// links) and pass through unchanged. Legacy releases used a flat list of
14-
/// <c>* &lt;commit-id&gt; description</c> bullets; this normalizer strips the
15-
/// commit-id prefix from those lines so they render as clean bullet items.
13+
/// links). Hand-authored content largely passes through; the only rewrites
14+
/// applied universally are converting <c>*</c> bullet markers to <c>-</c> for
15+
/// consistency and collapsing runs of 3+ blank lines. Legacy releases used a
16+
/// flat list of <c>* &lt;commit-id&gt; description</c> bullets, sometimes
17+
/// wrapped in raw HTML (<c>&lt;details&gt;</c>/<c>&lt;summary&gt;</c>/
18+
/// <c>&lt;b&gt;</c>) and followed by an auto-generated build-pipeline footer.
19+
/// This normalizer strips that noise so the renderer (which escapes raw HTML)
20+
/// does not surface the markup as literal text.
1621
/// </summary>
1722
public static partial class ReleaseNotesNormalizer
1823
{
@@ -25,11 +30,28 @@ public static string Normalize(string? rawBody)
2530

2631
var working = rawBody.Replace("\r\n", "\n").Replace('\r', '\n');
2732

33+
working = SummaryTagRegex().Replace(working, match =>
34+
{
35+
// Strip any nested HTML tags (legacy or otherwise) so they do not
36+
// surface as literal text inside the promoted heading.
37+
var inner = AnyHtmlTagRegex().Replace(match.Groups[1].Value, string.Empty).Trim();
38+
return string.IsNullOrEmpty(inner) ? string.Empty : $"\n\n## {inner}\n";
39+
});
40+
41+
working = LegacyHtmlTagRegex().Replace(working, string.Empty);
42+
working = AutoGeneratedFooterRegex().Replace(working, string.Empty);
2843
working = BulletRegex().Replace(working, match => $"- {match.Groups[1].Value.Trim()}");
44+
working = CollapseBlankLinesRegex().Replace(working, "\n\n");
2945

3046
return working.Trim();
3147
}
3248

49+
[GeneratedRegex(@"<[^>]+>")]
50+
private static partial Regex AnyHtmlTagRegex();
51+
52+
[GeneratedRegex(@"^This list of changes was \[auto generated\][^\n]*$", RegexOptions.Multiline)]
53+
private static partial Regex AutoGeneratedFooterRegex();
54+
3355
/// <summary>
3456
/// Matches a <c>* </c> bullet whose content optionally begins with a commit
3557
/// identifier — either a 40-character SHA or a Markdown link
@@ -40,5 +62,27 @@ public static string Normalize(string? rawBody)
4062
[GeneratedRegex(@"^\*\s+(?:\[[0-9a-f]{6,}\]\([^)\s]+\)\s+|[0-9a-f]{40}\s+)?(.+)$",
4163
RegexOptions.Multiline | RegexOptions.IgnoreCase)]
4264
private static partial Regex BulletRegex();
65+
66+
[GeneratedRegex(@"\n{3,}")]
67+
private static partial Regex CollapseBlankLinesRegex();
68+
69+
/// <summary>
70+
/// Strips the structural HTML wrappers and inline emphasis tags that
71+
/// historically appeared in GitHub release bodies. Inner text is preserved.
72+
/// </summary>
73+
[GeneratedRegex(@"</?(?:details|summary|b|i|em|strong)[^>]*>", RegexOptions.IgnoreCase)]
74+
private static partial Regex LegacyHtmlTagRegex();
75+
76+
/// <summary>
77+
/// Captures a <c>&lt;summary&gt;</c> block and promotes its inner text to a
78+
/// section heading so the collapsible "See More" label is preserved as a
79+
/// visible divider in the rendered notes. Any nested HTML tags inside the
80+
/// captured text are removed by <see cref="AnyHtmlTagRegex"/> before the
81+
/// heading is emitted, so even non-legacy markup (e.g. <c>&lt;iframe&gt;</c>)
82+
/// cannot surface as literal text in the heading.
83+
/// </summary>
84+
[GeneratedRegex(@"<summary[^>]*>(.*?)</summary>", RegexOptions.IgnoreCase | RegexOptions.Singleline)]
85+
private static partial Regex SummaryTagRegex();
4386
}
4487

88+

src/EventLogExpert.UI/Services/UpdateService.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,18 @@ await alertDialogService.ShowAlert("Release Notes Failure",
159159
var markdown = ReleaseNotesNormalizer.Normalize(_currentRawChanges);
160160
var title = $"Release notes for v{versionProvider.CurrentVersion}";
161161

162-
ReleaseNotesReady?.Invoke(new ReleaseNotesContent(title, markdown));
162+
var subscribers = ReleaseNotesReady;
163+
164+
if (subscribers is null)
165+
{
166+
// No UI is wired up to render the rich modal (e.g., service used
167+
// outside the Blazor host). Fall back to a plain alert so the user
168+
// still sees something rather than getting silent no-op feedback.
169+
await alertDialogService.ShowAlert(title, markdown, "Ok");
170+
171+
return;
172+
}
173+
174+
subscribers.Invoke(new ReleaseNotesContent(title, markdown));
163175
}
164176
}

src/EventLogExpert/Shared/Components/ReleaseNotesModal.razor.css

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,3 @@
5151
font-family: Consolas, monospace;
5252
font-size: 0.95em;
5353
}
54-
55-
.release-notes-content a {
56-
color: var(--clr-lightblue);
57-
text-decoration: underline;
58-
}

src/EventLogExpert/wwwroot/js/modals.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
window.openModal = (ref) => {
2-
if (ref != null) {
2+
if (ref != null && !ref.open) {
33
ref.showModal();
44
}
55
};

0 commit comments

Comments
 (0)