[PM-27575] Add support for loading Mailer templates from disk (#6520)

Hinton · web-flow · commit 7d39efe29ff1 · 2025-11-10T08:40:40.000+01:00
Adds support for overloading mail templates from disk.
diff --git a/src/Core/Platform/Mail/Mailer/HandlebarMailRenderer.cs b/src/Core/Platform/Mail/Mailer/HandlebarMailRenderer.cs
@@ -1,15 +1,17 @@
 ﻿#nullable enable
 using System.Collections.Concurrent;
 using System.Reflection;
+using Bit.Core.Settings;
 using HandlebarsDotNet;
+using Microsoft.Extensions.Logging;
 
 namespace Bit.Core.Platform.Mail.Mailer;
 public class HandlebarMailRenderer : IMailRenderer
 {
     /// <summary>
     /// Lazy-initialized Handlebars instance. Thread-safe and ensures initialization occurs only once.
     /// </summary>
-    private readonly Lazy<Task<IHandlebars>> _handlebarsTask = new(InitializeHandlebarsAsync, LazyThreadSafetyMode.ExecutionAndPublication);
+    private readonly Lazy<Task<IHandlebars>> _handlebarsTask;
 
     /// <summary>
     /// Helper function that returns the handlebar instance.
@@ -21,6 +23,17 @@ public class HandlebarMailRenderer : IMailRenderer
     /// </summary>
     private readonly ConcurrentDictionary<string, Lazy<Task<HandlebarsTemplate<object, object>>>> _templateCache = new();
 
+    private readonly ILogger<HandlebarMailRenderer> _logger;
+    private readonly GlobalSettings _globalSettings;
+
+    public HandlebarMailRenderer(ILogger<HandlebarMailRenderer> logger, GlobalSettings globalSettings)
+    {
+        _logger = logger;
+        _globalSettings = globalSettings;
+
+        _handlebarsTask = new Lazy<Task<IHandlebars>>(InitializeHandlebarsAsync, LazyThreadSafetyMode.ExecutionAndPublication);
+    }
+
     public async Task<(string html, string txt)> RenderAsync(BaseMailView model)
     {
         var html = await CompileTemplateAsync(model, "html");
@@ -53,19 +66,59 @@ private async Task<HandlebarsTemplate<object, object>> CompileTemplateInternalAs
         return handlebars.Compile(source);
     }
 
-    private static async Task<string> ReadSourceAsync(Assembly assembly, string template)
+    private async Task<string> ReadSourceAsync(Assembly assembly, string template)
     {
         if (assembly.GetManifestResourceNames().All(f => f != template))
         {
             throw new FileNotFoundException("Template not found: " + template);
         }
 
+        var diskSource = await ReadSourceFromDiskAsync(template);
+        if (!string.IsNullOrWhiteSpace(diskSource))
+        {
+            return diskSource;
+        }
+
         await using var s = assembly.GetManifestResourceStream(template)!;
         using var sr = new StreamReader(s);
         return await sr.ReadToEndAsync();
     }
 
-    private static async Task<IHandlebars> InitializeHandlebarsAsync()
+    private async Task<string?> ReadSourceFromDiskAsync(string template)
+    {
+        if (!_globalSettings.SelfHosted)
+        {
+            return null;
+        }
+
+        try
+        {
+            var diskPath = Path.GetFullPath(Path.Combine(_globalSettings.MailTemplateDirectory, template));
+            var baseDirectory = Path.GetFullPath(_globalSettings.MailTemplateDirectory);
+
+            // Ensure the resolved path is within the configured directory
+            if (!diskPath.StartsWith(baseDirectory + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase) &&
+                !diskPath.Equals(baseDirectory, StringComparison.OrdinalIgnoreCase))
+            {
+                _logger.LogWarning("Template path traversal attempt detected: {Template}", template);
+                return null;
+            }
+
+            if (File.Exists(diskPath))
+            {
+                var fileContents = await File.ReadAllTextAsync(diskPath);
+                return fileContents;
+            }
+        }
+        catch (Exception e)
+        {
+            _logger.LogError(e, "Failed to read mail template from disk: {TemplateName}", template);
+        }
+
+        return null;
+    }
+
+    private async Task<IHandlebars> InitializeHandlebarsAsync()
     {
         var handlebars = Handlebars.Create();
 
diff --git a/test/Core.Test/Platform/Mailer/HandlebarMailRendererTests.cs b/test/Core.Test/Platform/Mailer/HandlebarMailRendererTests.cs
@@ -1,5 +1,8 @@
 ﻿using Bit.Core.Platform.Mail.Mailer;
+using Bit.Core.Settings;
 using Bit.Core.Test.Platform.Mailer.TestMail;
+using Microsoft.Extensions.Logging;
+using NSubstitute;
 using Xunit;
 
 namespace Bit.Core.Test.Platform.Mailer;
@@ -9,12 +12,161 @@ public class HandlebarMailRendererTests
     [Fact]
     public async Task RenderAsync_ReturnsExpectedHtmlAndTxt()
     {
-        var renderer = new HandlebarMailRenderer();
+        var logger = Substitute.For<ILogger<HandlebarMailRenderer>>();
+        var globalSettings = new GlobalSettings { SelfHosted = false };
+        var renderer = new HandlebarMailRenderer(logger, globalSettings);
+
         var view = new TestMailView { Name = "John Smith" };
 
         var (html, txt) = await renderer.RenderAsync(view);
 
         Assert.Equal("Hello <b>John Smith</b>", html.Trim());
         Assert.Equal("Hello John Smith", txt.Trim());
     }
+
+    [Fact]
+    public async Task RenderAsync_LoadsFromDisk_WhenSelfHostedAndFileExists()
+    {
+        var logger = Substitute.For<ILogger<HandlebarMailRenderer>>();
+        var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+        Directory.CreateDirectory(tempDir);
+
+        try
+        {
+            var globalSettings = new GlobalSettings
+            {
+                SelfHosted = true,
+                MailTemplateDirectory = tempDir
+            };
+
+            // Create test template files on disk
+            var htmlTemplatePath = Path.Combine(tempDir, "Bit.Core.Test.Platform.Mailer.TestMail.TestMailView.html.hbs");
+            var txtTemplatePath = Path.Combine(tempDir, "Bit.Core.Test.Platform.Mailer.TestMail.TestMailView.text.hbs");
+            await File.WriteAllTextAsync(htmlTemplatePath, "Custom HTML: <b>{{Name}}</b>");
+            await File.WriteAllTextAsync(txtTemplatePath, "Custom TXT: {{Name}}");
+
+            var renderer = new HandlebarMailRenderer(logger, globalSettings);
+            var view = new TestMailView { Name = "Jane Doe" };
+
+            var (html, txt) = await renderer.RenderAsync(view);
+
+            Assert.Equal("Custom HTML: <b>Jane Doe</b>", html.Trim());
+            Assert.Equal("Custom TXT: Jane Doe", txt.Trim());
+        }
+        finally
+        {
+            // Cleanup
+            if (Directory.Exists(tempDir))
+            {
+                Directory.Delete(tempDir, true);
+            }
+        }
+    }
+
+    [Theory]
+    [InlineData("../../../etc/passwd")]
+    [InlineData("../../../../malicious.txt")]
+    [InlineData("../../malicious.txt")]
+    [InlineData("../malicious.txt")]
+    public async Task ReadSourceFromDiskAsync_PrevenetsPathTraversal_WhenMaliciousPathProvided(string maliciousPath)
+    {
+        var logger = Substitute.For<ILogger<HandlebarMailRenderer>>();
+        var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+        Directory.CreateDirectory(tempDir);
+
+        try
+        {
+            var globalSettings = new GlobalSettings
+            {
+                SelfHosted = true,
+                MailTemplateDirectory = tempDir
+            };
+
+            // Create a malicious file outside the template directory
+            var maliciousFile = Path.Combine(Path.GetTempPath(), "malicious.txt");
+            await File.WriteAllTextAsync(maliciousFile, "Malicious Content");
+
+            var renderer = new HandlebarMailRenderer(logger, globalSettings);
+
+            // Use reflection to call the private ReadSourceFromDiskAsync method
+            var method = typeof(HandlebarMailRenderer).GetMethod("ReadSourceFromDiskAsync",
+                System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
+            var task = (Task<string?>)method!.Invoke(renderer, new object[] { maliciousPath })!;
+            var result = await task;
+
+            // Should return null and not load the malicious file
+            Assert.Null(result);
+
+            // Verify that a warning was logged for the path traversal attempt
+            logger.Received(1).Log(
+                LogLevel.Warning,
+                Arg.Any<EventId>(),
+                Arg.Any<object>(),
+                Arg.Any<Exception>(),
+                Arg.Any<Func<object, Exception, string>>());
+
+            // Cleanup malicious file
+            if (File.Exists(maliciousFile))
+            {
+                File.Delete(maliciousFile);
+            }
+        }
+        finally
+        {
+            // Cleanup
+            if (Directory.Exists(tempDir))
+            {
+                Directory.Delete(tempDir, true);
+            }
+        }
+    }
+
+    [Fact]
+    public async Task ReadSourceFromDiskAsync_AllowsValidFileWithDifferentCase_WhenCaseInsensitiveFileSystem()
+    {
+        var logger = Substitute.For<ILogger<HandlebarMailRenderer>>();
+        var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+        Directory.CreateDirectory(tempDir);
+
+        try
+        {
+            var globalSettings = new GlobalSettings
+            {
+                SelfHosted = true,
+                MailTemplateDirectory = tempDir
+            };
+
+            // Create a test template file
+            var templateFileName = "TestTemplate.hbs";
+            var templatePath = Path.Combine(tempDir, templateFileName);
+            await File.WriteAllTextAsync(templatePath, "Test Content");
+
+            var renderer = new HandlebarMailRenderer(logger, globalSettings);
+
+            // Try to read with different case (should work on case-insensitive file systems like Windows/macOS)
+            var method = typeof(HandlebarMailRenderer).GetMethod("ReadSourceFromDiskAsync",
+                System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
+            var task = (Task<string?>)method!.Invoke(renderer, new object[] { templateFileName })!;
+            var result = await task;
+
+            // Should successfully read the file
+            Assert.Equal("Test Content", result);
+
+            // Verify no warning was logged
+            logger.DidNotReceive().Log(
+                LogLevel.Warning,
+                Arg.Any<EventId>(),
+                Arg.Any<object>(),
+                Arg.Any<Exception>(),
+                Arg.Any<Func<object, Exception, string>>());
+        }
+        finally
+        {
+            // Cleanup
+            if (Directory.Exists(tempDir))
+            {
+                Directory.Delete(tempDir, true);
+            }
+        }
+    }
 }
diff --git a/test/Core.Test/Platform/Mailer/MailerTest.cs b/test/Core.Test/Platform/Mailer/MailerTest.cs
@@ -1,18 +1,24 @@
 ﻿using Bit.Core.Models.Mail;
 using Bit.Core.Platform.Mail.Delivery;
 using Bit.Core.Platform.Mail.Mailer;
+using Bit.Core.Settings;
 using Bit.Core.Test.Platform.Mailer.TestMail;
+using Microsoft.Extensions.Logging;
 using NSubstitute;
 using Xunit;
 
 namespace Bit.Core.Test.Platform.Mailer;
+
 public class MailerTest
 {
     [Fact]
     public async Task SendEmailAsync()
     {
+        var logger = Substitute.For<ILogger<HandlebarMailRenderer>>();
+        var globalSettings = new GlobalSettings { SelfHosted = false };
         var deliveryService = Substitute.For<IMailDeliveryService>();
-        var mailer = new Core.Platform.Mail.Mailer.Mailer(new HandlebarMailRenderer(), deliveryService);
+
+        var mailer = new Core.Platform.Mail.Mailer.Mailer(new HandlebarMailRenderer(logger, globalSettings), deliveryService);
 
         var mail = new TestMail.TestMail()
         {
