From e63b507ef8252cbf21d54e8d4e3a46638ba74351 Mon Sep 17 00:00:00 2001 From: JustAHubber Date: Thu, 11 Jun 2026 20:27:36 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20security=20&=20code=20audit=20=E2=80=94?= =?UTF-8?q?=20repojacking=20fix,=20mutex=20crash,=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security: - Auto-updater now targets the repository current name (Daolyap/Money-Shot). The old name only redirects until someone re-registers it, at which point they would own the update channel (repojacking). - Optional GitHub token properly wired to MONEYSHOT_GITHUB_TOKEN (the 401 guidance already referenced it; the read was a disabled empty-string stub). - SaveService system-directory check compares with a trailing separator so C:\Windows blocks C:\Windows\... but not C:\WindowsBackup siblings. - HistoryRetentionCount clamped to 0-500 during settings sanitization. Bugs: - Second app instance crashed on exit: OnExit called ReleaseMutex on the single-instance mutex it never owned (ApplicationException). - Hotkey actions that throw no longer propagate out of the WndProc hook (process-fatal); they are caught and logged. Registration failures (combination owned by another app) are now logged instead of silent. - Editor Save dialog now honours the configured default save folder and file format; previously both settings were ignored. - IsPointInElement used hand-rolled NaN guards instead of CanvasPosition (forbidden by the resize/drag design rules). Improvements: - MemoryTrimmer service shared by both editor-close paths; opening a capture from History now gets the same working-set trim as the capture flow (previously left ~hundreds of MB resident). - JPEG saves use QualityLevel 90 (default 75 smears screenshot text). - Logger includes full exception + stack trace at Error level. - Dead SetupToolbar() removed; App.xaml.cs unused usings dropped. - 14 new regression tests (SaveService path validation, retention clamp); 109/109 passing. Audit follow-ups recorded in Opus-Speaks.md: region-selector DPI accuracy at >100% scaling (E2), auto-update vs per-machine installs (E3). Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 4 +- MoneyShot.Tests/SaveServiceTests.cs | 64 +++++++++++++++++++++++++ MoneyShot.Tests/SettingsServiceTests.cs | 13 +++++ MoneyShot/App.xaml.cs | 23 +++++---- MoneyShot/MainWindow.xaml.cs | 29 +---------- MoneyShot/Services/AutoUpdateService.cs | 11 ++++- MoneyShot/Services/HotKeyService.cs | 15 +++++- MoneyShot/Services/Logger.cs | 6 ++- MoneyShot/Services/MemoryTrimmer.cs | 35 ++++++++++++++ MoneyShot/Services/SaveService.cs | 19 ++++++-- MoneyShot/Services/SettingsService.cs | 6 ++- MoneyShot/Views/EditorWindow.xaml.cs | 26 +++++----- MoneyShot/Views/HistoryWindow.xaml.cs | 13 ++++- Opus-Speaks.md | 12 +++++ 14 files changed, 212 insertions(+), 64 deletions(-) create mode 100644 MoneyShot.Tests/SaveServiceTests.cs create mode 100644 MoneyShot/Services/MemoryTrimmer.cs diff --git a/CLAUDE.md b/CLAUDE.md index da733b2..baeee9d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,7 +73,9 @@ Version comparison is **SemVer-only** (Major.Minor.Patch via `CompareSemVer`) If a release lacks `SHA256SUMS.txt` (older releases), the update is allowed through with a logged warning rather than refused. New releases generated by `release.yml` always include it. -The optional GitHub token read at line 50 is intentionally `Environment.GetEnvironmentVariable("")` — i.e. disabled. If you wire one up, do it through a real env-var name and treat it as a secret. +An optional GitHub token is read from the `MONEYSHOT_GITHUB_TOKEN` environment variable (higher API rate limit). Treat it as a secret — never log it. + +**The repo/owner constants must track the repository's current name** (`Daolyap/Money-Shot` since the rename). GitHub redirects an old repo name only until someone re-registers it — at which point they own the update channel (repojacking). Update the constants immediately on any rename. ### Editor undo model diff --git a/MoneyShot.Tests/SaveServiceTests.cs b/MoneyShot.Tests/SaveServiceTests.cs new file mode 100644 index 0000000..9a06ef3 --- /dev/null +++ b/MoneyShot.Tests/SaveServiceTests.cs @@ -0,0 +1,64 @@ +using System.IO; +using MoneyShot.Services; +using Xunit; + +namespace MoneyShot.Tests; + +public class SaveServiceTests +{ + private readonly SaveService _service = new(); + + // Path validation runs before the image is touched, so a null image never reaches the + // encoder for these rejection cases — no WPF/STA setup needed. + + [Fact] + public void SaveToFile_InsideWindowsDirectory_IsRejected() + { + var windows = Environment.GetFolderPath(Environment.SpecialFolder.Windows); + var path = Path.Combine(windows, "moneyshot-test.png"); + Assert.Throws(() => _service.SaveToFile(null!, path)); + } + + [Fact] + public void SaveToFile_NestedUnderSystemDirectory_IsRejected() + { + var system = Environment.GetFolderPath(Environment.SpecialFolder.System); + var path = Path.Combine(system, "drivers", "moneyshot-test.png"); + Assert.Throws(() => _service.SaveToFile(null!, path)); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void SaveToFile_EmptyPath_IsRejected(string path) + { + Assert.Throws(() => _service.SaveToFile(null!, path)); + } + + [Fact] + public void SaveToFile_BareDirectoryPath_IsRejected() + { + Assert.Throws(() => _service.SaveToFile(null!, Path.GetTempPath())); + } + + [Fact] + public void SaveToFile_UserWritablePath_PassesValidation() + { + // With a valid path, validation must NOT throw ArgumentException; the failure (if any) + // comes from the null image and is wrapped as InvalidOperationException. + var path = Path.Combine(Path.GetTempPath(), "moneyshot-test.png"); + var ex = Record.Exception(() => _service.SaveToFile(null!, path)); + Assert.IsType(ex); + } + + [Theory] + [InlineData("PNG", ".png")] + [InlineData("JPG", ".jpg")] + [InlineData("BMP", ".bmp")] + public void GenerateFileName_UsesLowercaseExtension(string format, string expectedExtension) + { + var name = _service.GenerateFileName(format); + Assert.StartsWith("Screenshot_", name); + Assert.EndsWith(expectedExtension, name); + } +} diff --git a/MoneyShot.Tests/SettingsServiceTests.cs b/MoneyShot.Tests/SettingsServiceTests.cs index f2d494b..be1118d 100644 --- a/MoneyShot.Tests/SettingsServiceTests.cs +++ b/MoneyShot.Tests/SettingsServiceTests.cs @@ -10,6 +10,19 @@ public class SettingsServiceTests private static readonly string MyPictures = Environment.GetFolderPath(Environment.SpecialFolder.MyPictures); + [Theory] + [InlineData(-5, 0)] + [InlineData(0, 0)] + [InlineData(50, 50)] + [InlineData(500, 500)] + [InlineData(100_000, 500)] + public void HistoryRetentionCount_IsClampedToUiRange(int input, int expected) + { + var settings = new AppSettings { HistoryRetentionCount = input }; + var sanitized = SettingsService.ValidateAndSanitizeSettings(settings); + Assert.Equal(expected, sanitized.HistoryRetentionCount); + } + [Fact] public void EmptyDefaultSavePath_FallsBackToMyPictures() { diff --git a/MoneyShot/App.xaml.cs b/MoneyShot/App.xaml.cs index dff182c..fb8a38c 100644 --- a/MoneyShot/App.xaml.cs +++ b/MoneyShot/App.xaml.cs @@ -1,5 +1,3 @@ -using System.Configuration; -using System.Data; using System.Windows; using System.Threading; @@ -11,14 +9,16 @@ namespace MoneyShot; public partial class App : Application { private static Mutex? _mutex; - + private static bool _ownsMutex; + protected override void OnStartup(StartupEventArgs e) { // Create a unique mutex name for the application const string mutexName = "MoneyShot_SingleInstance_Mutex_3E6F8A2D"; - + _mutex = new Mutex(true, mutexName, out bool createdNew); - + _ownsMutex = createdNew; + if (!createdNew) { // Another instance is already running @@ -30,15 +30,20 @@ protected override void OnStartup(StartupEventArgs e) Shutdown(); return; } - + base.OnStartup(e); } - + protected override void OnExit(ExitEventArgs e) { - _mutex?.ReleaseMutex(); + // Only the instance that actually acquired the mutex may release it — calling + // ReleaseMutex without ownership throws and would crash the "already running" + // second instance on its way out. + if (_ownsMutex) + { + _mutex?.ReleaseMutex(); + } _mutex?.Dispose(); base.OnExit(e); } } - diff --git a/MoneyShot/MainWindow.xaml.cs b/MoneyShot/MainWindow.xaml.cs index 51f6d72..32b8c66 100644 --- a/MoneyShot/MainWindow.xaml.cs +++ b/MoneyShot/MainWindow.xaml.cs @@ -392,36 +392,11 @@ private void OpenEditor(System.Windows.Media.Imaging.BitmapSource screenshot, st finally { // The editor's BitmapSource backings, RenderTargetBitmaps and pixelate brushes live in - // both managed and native heaps. Without forcing a collection plus a working-set trim, - // the process sits at several hundred MB until the next major GC — undesirable for a - // tray app that should hover near 80MB while idle. - ReleaseEditorMemory(); + // both managed and native heaps — see MemoryTrimmer for why this is forced here. + MemoryTrimmer.TrimAfterEditorClose(); } } - private static void ReleaseEditorMemory() - { - try - { - System.Runtime.GCSettings.LargeObjectHeapCompactionMode = - System.Runtime.GCLargeObjectHeapCompactionMode.CompactOnce; - GC.Collect(GC.MaxGeneration, GCCollectionMode.Aggressive, blocking: true, compacting: true); - GC.WaitForPendingFinalizers(); - GC.Collect(); - - // Ask Windows to trim the working set. -1, -1 is the documented "trim now" sentinel. - using var process = System.Diagnostics.Process.GetCurrentProcess(); - SetProcessWorkingSetSize(process.Handle, new IntPtr(-1), new IntPtr(-1)); - } - catch (Exception ex) - { - MoneyShot.Services.Logger.Warn("Could not release editor memory", ex); - } - } - - [System.Runtime.InteropServices.DllImport("kernel32.dll", SetLastError = true)] - private static extern bool SetProcessWorkingSetSize(IntPtr proc, IntPtr min, IntPtr max); - private void ShowSettings() { var settings = new SettingsWindow(); diff --git a/MoneyShot/Services/AutoUpdateService.cs b/MoneyShot/Services/AutoUpdateService.cs index 740943e..cc7f42b 100644 --- a/MoneyShot/Services/AutoUpdateService.cs +++ b/MoneyShot/Services/AutoUpdateService.cs @@ -16,8 +16,12 @@ namespace MoneyShot.Services; public sealed class AutoUpdateService { + // SECURITY: must track the repository's CURRENT name. The repo was renamed from + // "MoneyShot" to "Money-Shot"; GitHub redirects the old name only until someone + // re-registers it, at which point they would control this app's update channel + // (repojacking). If the repo is ever renamed again, update this immediately. private const string Owner = "Daolyap"; - private const string Repository = "MoneyShot"; + private const string Repository = "Money-Shot"; private const string LatestReleaseUrl = $"https://api.github.com/repos/{Owner}/{Repository}/releases/latest"; private const string AllReleasesUrl = $"https://api.github.com/repos/{Owner}/{Repository}/releases?per_page=1"; private static readonly Regex VersionNumberRegex = new(@"\d+", RegexOptions.Compiled); @@ -48,7 +52,10 @@ public AutoUpdateService(HttpClient? httpClient = null) _httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/vnd.github+json")); } - var token = Environment.GetEnvironmentVariable(""); // I can put a GitHub token in here if I want authenticated API requests + // Optional: authenticated API requests (higher rate limit). The 401 guidance message + // below already documents this variable name. Treat the value as a secret — it is only + // ever sent to api.github.com over HTTPS and never logged. + var token = Environment.GetEnvironmentVariable("MONEYSHOT_GITHUB_TOKEN"); if (!string.IsNullOrWhiteSpace(token) && _httpClient.DefaultRequestHeaders.Authorization == null) { _httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token.Trim()); diff --git a/MoneyShot/Services/HotKeyService.cs b/MoneyShot/Services/HotKeyService.cs index 7e8076e..da0b381 100644 --- a/MoneyShot/Services/HotKeyService.cs +++ b/MoneyShot/Services/HotKeyService.cs @@ -33,6 +33,10 @@ public int RegisterHotKey(uint modifiers, uint key, Action action) _hotKeyActions[_currentId] = action; return _currentId; } + + // Most often the combination is already claimed by another application (or by a + // still-running instance). Without this log the hotkey just silently does nothing. + Logger.Warn($"RegisterHotKey failed for modifiers=0x{modifiers:X} key=0x{key:X} — combination may be in use by another application."); return -1; } @@ -59,7 +63,16 @@ private IntPtr HwndHook(IntPtr hwnd, int msg, IntPtr wParam, IntPtr lParam, ref var id = wParam.ToInt32(); if (_hotKeyActions.TryGetValue(id, out var action)) { - action?.Invoke(); + try + { + action?.Invoke(); + } + catch (Exception ex) + { + // An exception escaping a window-procedure hook can take down the process; + // a failed capture should be logged, not fatal. + Logger.Error("Hotkey action threw", ex); + } handled = true; } } diff --git a/MoneyShot/Services/Logger.cs b/MoneyShot/Services/Logger.cs index 411eda5..78a88ff 100644 --- a/MoneyShot/Services/Logger.cs +++ b/MoneyShot/Services/Logger.cs @@ -31,9 +31,13 @@ internal static class Logger private static void Write(string level, string message, Exception? exception) { + // Errors get the full exception (incl. stack trace) — "ERR ... :: IOException: access + // denied" alone is rarely enough to diagnose a user report. Warnings stay single-line. var line = exception == null ? $"{DateTime.Now:yyyy-MM-dd HH:mm:ss.fff} [{level}] {message}" - : $"{DateTime.Now:yyyy-MM-dd HH:mm:ss.fff} [{level}] {message} :: {exception.GetType().Name}: {exception.Message}"; + : level == "ERR" + ? $"{DateTime.Now:yyyy-MM-dd HH:mm:ss.fff} [{level}] {message} :: {exception}" + : $"{DateTime.Now:yyyy-MM-dd HH:mm:ss.fff} [{level}] {message} :: {exception.GetType().Name}: {exception.Message}"; System.Diagnostics.Debug.WriteLine(line); diff --git a/MoneyShot/Services/MemoryTrimmer.cs b/MoneyShot/Services/MemoryTrimmer.cs new file mode 100644 index 0000000..85eda59 --- /dev/null +++ b/MoneyShot/Services/MemoryTrimmer.cs @@ -0,0 +1,35 @@ +using System.Runtime; +using System.Runtime.InteropServices; + +namespace MoneyShot.Services; + +/// +/// Releases the large native/managed bitmap backings the editor leaves behind and asks Windows +/// to trim the working set. Without this a tray app that should idle near ~80 MB sits at several +/// hundred MB after the editor closes, until the next major GC happens on its own schedule. +/// Shared by every code path that closes an EditorWindow (capture flow and history). +/// +internal static class MemoryTrimmer +{ + public static void TrimAfterEditorClose() + { + try + { + GCSettings.LargeObjectHeapCompactionMode = GCLargeObjectHeapCompactionMode.CompactOnce; + GC.Collect(GC.MaxGeneration, GCCollectionMode.Aggressive, blocking: true, compacting: true); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + // Ask Windows to trim the working set. -1, -1 is the documented "trim now" sentinel. + using var process = System.Diagnostics.Process.GetCurrentProcess(); + SetProcessWorkingSetSize(process.Handle, new IntPtr(-1), new IntPtr(-1)); + } + catch (Exception ex) + { + Logger.Warn("Could not release editor memory", ex); + } + } + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool SetProcessWorkingSetSize(IntPtr proc, IntPtr min, IntPtr max); +} diff --git a/MoneyShot/Services/SaveService.cs b/MoneyShot/Services/SaveService.cs index 3fe9121..e559889 100644 --- a/MoneyShot/Services/SaveService.cs +++ b/MoneyShot/Services/SaveService.cs @@ -27,10 +27,12 @@ public void SaveToFile(BitmapSource image, string filePath, string format = "PNG try { - BitmapEncoder? encoder = format.ToUpper() switch + BitmapEncoder encoder = format.ToUpper() switch { "PNG" => new PngBitmapEncoder(), - "JPG" or "JPEG" => new JpegBitmapEncoder(), + // Default JPEG quality (75) visibly smears text in screenshots; 90 keeps + // UI text legible at a still-reasonable file size. + "JPG" or "JPEG" => new JpegBitmapEncoder { QualityLevel = 90 }, "BMP" => new BmpBitmapEncoder(), "GIF" => new GifBitmapEncoder(), _ => new PngBitmapEncoder() @@ -120,9 +122,16 @@ private void ValidateFilePath(string filePath) foreach (var sysDir in systemDirs) { - if (!string.IsNullOrEmpty(sysDir) && - !string.IsNullOrEmpty(directory) && - directory.StartsWith(sysDir, StringComparison.OrdinalIgnoreCase)) + if (string.IsNullOrEmpty(sysDir) || string.IsNullOrEmpty(directory)) + { + continue; + } + + // Compare with a trailing separator so "C:\Windows" blocks "C:\Windows\..." + // and "C:\Windows" itself, but not sibling folders like "C:\WindowsBackup". + var sysDirWithSeparator = Path.TrimEndingDirectorySeparator(sysDir) + Path.DirectorySeparatorChar; + var directoryWithSeparator = Path.TrimEndingDirectorySeparator(directory) + Path.DirectorySeparatorChar; + if (directoryWithSeparator.StartsWith(sysDirWithSeparator, StringComparison.OrdinalIgnoreCase)) { throw new ArgumentException("Cannot save to system directories.", nameof(filePath)); } diff --git a/MoneyShot/Services/SettingsService.cs b/MoneyShot/Services/SettingsService.cs index 017fca8..f984ba9 100644 --- a/MoneyShot/Services/SettingsService.cs +++ b/MoneyShot/Services/SettingsService.cs @@ -152,7 +152,11 @@ internal static AppSettings ValidateAndSanitizeSettings(AppSettings settings) { settings.DefaultLineThickness = 3; } - + + // Clamp history retention to the range the UI offers (0 disables retention + // enforcement) so a hand-edited settings.json can't request absurd values. + settings.HistoryRetentionCount = Math.Clamp(settings.HistoryRetentionCount, 0, 500); + return settings; } diff --git a/MoneyShot/Views/EditorWindow.xaml.cs b/MoneyShot/Views/EditorWindow.xaml.cs index 166de3a..b61fd45 100644 --- a/MoneyShot/Views/EditorWindow.xaml.cs +++ b/MoneyShot/Views/EditorWindow.xaml.cs @@ -102,7 +102,6 @@ public EditorWindow(BitmapSource image) _originalImage = image; _saveService = new SaveService(); DisplayImage(); - SetupToolbar(); // Add keyboard event handler for Delete key KeyDown += EditorWindow_KeyDown; @@ -440,11 +439,6 @@ private void DisplayImage() DrawingCanvas.Height = _originalImage.PixelHeight; } - private void SetupToolbar() - { - // Tool buttons will be set up in XAML - } - private Point ClampToCanvasBounds(Point point) { var clampedX = Math.Max(0, Math.Min(point.X, DrawingCanvas.Width)); @@ -1093,12 +1087,8 @@ private void UpdateArrow(Point currentPoint) private bool IsPointInElement(UIElement element, Point point) { - var left = Canvas.GetLeft(element); - var top = Canvas.GetTop(element); - - // Handle NaN values (elements without explicit positioning) - if (double.IsNaN(left)) left = 0; - if (double.IsNaN(top)) top = 0; + var left = CanvasPosition.GetLeft(element); + var top = CanvasPosition.GetTop(element); if (element is Path path) { @@ -1947,12 +1937,18 @@ private void Save_Click(object sender, RoutedEventArgs e) try { var finalImage = CaptureCanvasAsImage(); - + + // Honour the user's configured save folder and default format — previously the + // dialog always opened wherever Windows last left it, defaulting to PNG. + var settings = new SettingsService().LoadSettings(); + var defaultFormat = settings.DefaultFileFormat.ToUpperInvariant(); var saveDialog = new Microsoft.Win32.SaveFileDialog { Filter = "PNG Image|*.png|JPEG Image|*.jpg|Bitmap Image|*.bmp", - DefaultExt = ".png", - FileName = _saveService.GenerateFileName("PNG") + FilterIndex = defaultFormat switch { "JPG" or "JPEG" => 2, "BMP" => 3, _ => 1 }, + DefaultExt = defaultFormat switch { "JPG" or "JPEG" => ".jpg", "BMP" => ".bmp", _ => ".png" }, + InitialDirectory = settings.DefaultSavePath, + FileName = _saveService.GenerateFileName(settings.DefaultFileFormat) }; if (saveDialog.ShowDialog() == true) diff --git a/MoneyShot/Views/HistoryWindow.xaml.cs b/MoneyShot/Views/HistoryWindow.xaml.cs index 7deb34c..26fdb78 100644 --- a/MoneyShot/Views/HistoryWindow.xaml.cs +++ b/MoneyShot/Views/HistoryWindow.xaml.cs @@ -112,8 +112,17 @@ private void OpenInEditor(HistoryEntry entry) MessageBoxButton.OK, MessageBoxImage.Warning); return; } - var editor = new EditorWindow(image); - editor.ShowDialog(); + try + { + var editor = new EditorWindow(image); + editor.ShowDialog(); + } + finally + { + // Same working-set trim the capture flow does — without it, opening a capture from + // history left hundreds of MB of bitmap backings resident after the editor closed. + MemoryTrimmer.TrimAfterEditorClose(); + } } private void CopyToClipboard(HistoryEntry entry) diff --git a/Opus-Speaks.md b/Opus-Speaks.md index 3f2974f..4bb05da 100644 --- a/Opus-Speaks.md +++ b/Opus-Speaks.md @@ -67,6 +67,18 @@ SHA-256 verification protects against tampering between GitHub Releases and the > > Until this lands, the SHA-256 check is the only integrity guarantee — that's still meaningfully better than nothing. +## E2. Region selector is only pixel-accurate at 100 % display scaling + +Found during the 2026-06 security/code audit. The app manifest declares `PerMonitorV2` DPI awareness, but `RegionSelector` sets its window `Width`/`Height` (DIPs) directly from physical pixel bounds and maps mouse DIP positions 1:1 onto frozen-bitmap pixel coordinates. At 100 % scaling DIP == pixel and everything lines up; at 125 %/150 % (or mixed-DPI multi-monitor) the window oversizes and crops drift. + +> Fixing this properly needs one selector window per monitor (each in its own DPI context) or explicit `CompositionTarget.TransformToDevice` math when converting selection rects to bitmap pixels. The mixed-DPI virtual-screen mapping is nonlinear — don't try to patch it with a single global scale factor. + +## E3. Auto-update can't swap a per-machine install + +`StageAndPrepareUpdateAsync` writes the swap script to user temp and runs it unelevated. For per-user installs that's fine, but the MSI installs to `C:\Program Files\Money Shot\` (per-machine), where `move /Y` will fail with access denied and the update silently doesn't apply (script exits 1, nothing re-launches). + +> Either: detect a Program Files install and direct the user to download the MSI instead of self-swapping; or run the script elevated (UAC prompt) via `UseShellExecute = true` + `Verb = "runas"`. Decide deliberately — silent elevation prompts from a background updater are hostile UX. + ## F. Migrate to `Microsoft.Extensions.Logging` if a real DI need ever appears The current `Logger` static facade closes the "no Release-build output" gap, but it doesn't enable structured logging, log levels controlled by config, or per-namespace filtering. If the project ever takes on a complexity that justifies DI (multiple loggers, multiple sinks, `ILogger` for testability), the right move is the originally-proposed migration from this document — replace `Logger` with `Microsoft.Extensions.Logging` + a logger factory in `App.xaml.cs`, then take constructor-injected `ILogger` in each service.