Merge pull request 'fix: escape html notification variables' (#18) from feature/it-1115-html-template-escaping into main

Reviewed-on: #18

HrynCo.NotificationService.Services.Tests/EmailProcessing/EmailTemplateRenderingServiceTests.cs

@@ -40,6 +40,27 @@ public sealed class EmailTemplateRenderingServiceTests
         Assert.Equal("Required template variables are missing: VerificationUrl.", exception.Message);
     }
 
+    [Fact]
+    public void Render_HtmlEncodesVariablesWithoutChangingSubjectOrTextBody()
+    {
+        EmailTemplate template = CreateTemplate();
+        SendEmailMessageData data = CreateData(new Dictionary<string, string>
+        {
+            ["AppName"] = "Invemory <script>alert('xss')</script>",
+            ["VerificationUrl"] = "https://example.invalid/verify?next=\" onclick=\"alert('xss')"
+        });
+
+        RenderedEmail result = _service.Render(template, data);
+
+        Assert.Equal("Verify Invemory <script>alert('xss')</script>", result.Subject);
+        Assert.Equal(
+            "<a href=\"https://example.invalid/verify?next=&quot; onclick=&quot;alert(&#39;xss&#39;)\">Verify</a>",
+            result.HtmlBody);
+        Assert.Equal(
+            "Verify at https://example.invalid/verify?next=\" onclick=\"alert('xss')",
+            result.TextBody);
+    }
+
     private static EmailTemplate CreateTemplate() => new()
     {
         ServiceName = "StoreMate-Prod",

HrynCo.NotificationService.Web.IntegrationTests/AdminTemplatesIndexViewTests.cs

@@ -0,0 +1,32 @@
+namespace HrynCo.NotificationService.Web.IntegrationTests;
+
+public sealed class AdminTemplatesIndexViewTests
+{
+    [Fact]
+    public void CreateAndEditLinks_UseExplicitFilterQueryInterpolation()
+    {
+        string view = File.ReadAllText(FindIndexView());
+
+        Assert.Contains("/admin/templates/create@(filterQuery)", view);
+        Assert.Contains("@t.LanguageCode@(filterQuery)", view);
+        Assert.DoesNotContain("create@filterQuery", view);
+        Assert.DoesNotContain("LanguageCode@filterQuery", view);
+    }
+
+    private static string FindIndexView()
+    {
+        DirectoryInfo? directory = new(AppContext.BaseDirectory);
+        while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "HrynCo.NotificationService.slnx")))
+        {
+            directory = directory.Parent;
+        }
+
+        Assert.NotNull(directory);
+        return Path.Combine(
+            directory.FullName,
+            "HrynCo.NotificationService.Web",
+            "Views",
+            "AdminTemplates",
+            "Index.cshtml");
+    }
+}

HrynCo.NotificationService.Web.IntegrationTests/UnitTest1.cs

@@ -1,10 +0,0 @@
-namespace HrynCo.NotificationService.Web.IntegrationTests;
-
-public class UnitTest1
-{
-    [Fact]
-    public void Test1()
-    {
-
-    }
-}

HrynCo.NotificationService.Web/Views/AdminTemplates/Index.cshtml

@@ -14,7 +14,7 @@
 
 <div class="page-header">
     <h2><i class="bi bi-envelope-paper"></i> Email Templates</h2>
-    <a href="/admin/templates/create@filterQuery" class="btn btn-primary btn-sm">
+    <a href="/admin/templates/create@(filterQuery)" class="btn btn-primary btn-sm">
         <i class="bi bi-plus-lg me-1"></i> Create New Template
     </a>
 </div>
@@ -146,7 +146,7 @@ else
                             <td>@t.LanguageCode</td>
                             <td>@t.Subject</td>
                             <td class="text-end">
-                                <a href="/admin/templates/@t.ServiceName/@t.Key/@t.LanguageCode@filterQuery"
+                                <a href="/admin/templates/@t.ServiceName/@t.Key/@t.LanguageCode@(filterQuery)"
                                    class="btn btn-sm btn-outline-primary me-1">
                                     <i class="bi bi-pencil"></i> Edit
                                 </a>

HrynCo.NotificationService.Worker.Services/EmailProcessing/EmailTemplateRenderingService.cs

@@ -1,5 +1,6 @@
 namespace HrynCo.NotificationService.Worker.Services.EmailProcessing;
 
+using System.Net;
 using System.Text;
 using HrynCo.NotificationService.Contracts.Messages;
 using HrynCo.NotificationService.DAL.Abstract.Templates;
@@ -22,15 +23,28 @@ internal sealed class EmailTemplateRenderingService : IEmailTemplateRenderingSer
 
         return new RenderedEmail(
             Interpolate(template.Subject, data.Variables),
-            Interpolate(template.HtmlBody, data.Variables),
+            InterpolateHtml(template.HtmlBody, data.Variables),
             Interpolate(template.TextBody, data.Variables));
     }
 
+    private static string InterpolateHtml(string text, IReadOnlyDictionary<string, string> variables)
+    {
+        return Interpolate(text, variables, WebUtility.HtmlEncode);
+    }
+
     private static string Interpolate(string text, IReadOnlyDictionary<string, string> variables)
+    {
+        return Interpolate(text, variables, static value => value);
+    }
+
+    private static string Interpolate(
+        string text,
+        IReadOnlyDictionary<string, string> variables,
+        Func<string, string> encodeValue)
     {
         var sb = new StringBuilder(text);
         foreach (var (key, value) in variables)
-            sb.Replace($"{{{{{key}}}}}", value);
+            sb.Replace($"{{{{{key}}}}}", encodeValue(value));
         return sb.ToString();
     }
 }

README.md

@@ -112,3 +112,7 @@ flowchart TD
     M -->|retries exhausted| N[Publish terminal failure result]
     N --> O[Nack original delivery without requeue]
 ```
+
+Template variables are treated as plain text. The worker HTML-encodes every variable while
+rendering `HtmlBody`; subject and plain-text body interpolation preserve the original value.
+Templates must express markup in `body.html` instead of supplying HTML through variables.

docs/itemtracker-outbox-email-consumer.md

@@ -112,4 +112,10 @@ version.
 
 Before enabling the Worker, verify that the target service has an active SMTP channel and exact-language templates for every queued ItemTracker template key. Inspect any delayed ItemTracker backlog for expired password-reset, verification, or invitation messages before draining it.
 
+The template administration list preserves optional Service Name and Key filters when an
+administrator opens the create or edit screen. These links use explicit Razor expression
+boundaries so the filter query is appended as query parameters rather than rendered as a
+literal `@filterQuery` path segment. With no active filters, the create route is exactly
+`/admin/templates/create`.
+
 For the smoke test, use an owned test account and a non-sensitive notification template. Record the Outbox `Id` and `CorrelationId`, trigger only one message, follow those identifiers through Outbox publication and Notification Service logs, and confirm receipt with the configured SMTP provider. Do not copy payloads, tokens, credentials, full recipient addresses, or rendered URLs into tickets or logs.