peter/SpamGuard
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: address code review findings (critical + important + minor)

Browse Source 
Critical fixes:
- C1: Fix GDI handle leak in IconGenerator (lazy singletons + DestroyIcon)
- C2: Add appsettings.json to .gitignore, provide template
- C3: Properly dispose IHost on exit

Important fixes:
- I1: Add SpamGuardOptionsValidator for startup config validation
- I3: Use UID range queries instead of SearchQuery.All
- I4: Only scan recent Sent messages after initial full scan
- I5: Atomic file writes (write-to-temp, then rename)
- I6: Set 30s HTTP timeout and base address on HttpClient
- I7: Remove duplicate AddSingleton<EmailClassifier> (DI conflict)
- I8: Better HTML stripping (script/style removal, entity decoding)

Minor fixes:
- M1: Delete placeholder UnitTest1.cs
- M2: Wire MaxActivityLogEntries config to ActivityLog constructor
- M4: Log warnings instead of bare catch blocks
- M5: Dispose JsonDocument instances
- M8: Use AppContext.BaseDirectory for appsettings.json path
- M9: Truncate NotifyIcon.Text to 127 chars

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
peter
2026-04-07 11:57:12 +01:00
parent cd9adc5a54
commit 66cca61b21
12 changed files with 181 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
.gitignore vendored
View File

@@ -482,3 +482,6 @@ $RECYCLE.BIN/
# Vim temporary swap files
*.swp
# User configuration with secrets
src/SpamGuard/appsettings.json

24
src/SpamGuard/Configuration/SpamGuardOptionsValidator.cs Normal file
View File

@@ -0,0 +1,24 @@
namespace SpamGuard.Configuration;
using Microsoft.Extensions.Options;
public sealed class SpamGuardOptionsValidator : IValidateOptions<SpamGuardOptions>
{
public ValidateOptionsResult Validate(string? name, SpamGuardOptions options)
{
var errors = new List<string>();
if (string.IsNullOrWhiteSpace(options.Imap.Host))
errors.Add("SpamGuard:Imap:Host is required.");
if (string.IsNullOrWhiteSpace(options.Imap.Username))
errors.Add("SpamGuard:Imap:Username is required.");
if (string.IsNullOrWhiteSpace(options.Imap.Password))
errors.Add("SpamGuard:Imap:Password is required.");
if (string.IsNullOrWhiteSpace(options.Claude.ApiKey))
errors.Add("SpamGuard:Claude:ApiKey is required.");
return errors.Count > 0
? ValidateOptionsResult.Fail(errors)
: ValidateOptionsResult.Success;
}
}

30
src/SpamGuard/Program.cs
View File

@@ -1,7 +1,7 @@
// src/SpamGuard/Program.cs
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Options;
using Serilog;
using SpamGuard.Configuration;
using SpamGuard.Services;
@@ -31,13 +31,16 @@ static class Program
retainedFileCountLimit: 7)
.CreateLogger();
IHost? host = null;
try
{
var host = Host.CreateDefaultBuilder()
host = Host.CreateDefaultBuilder()
.UseSerilog()
.ConfigureAppConfiguration((context, config) =>
{
config.AddJsonFile("appsettings.json", optional: false);
config.AddJsonFile(
Path.Combine(AppContext.BaseDirectory, "appsettings.json"),
optional: false);
config.AddEnvironmentVariables("SPAMGUARD_");
})
.ConfigureServices((context, services) =>
@@ -45,15 +48,28 @@ static class Program
services.Configure<SpamGuardOptions>(
context.Configuration.GetSection(SpamGuardOptions.SectionName));
// Validate required configuration on startup
services.AddSingleton<IValidateOptions<SpamGuardOptions>, SpamGuardOptionsValidator>();
services.AddOptionsWithValidateOnStart<SpamGuardOptions>();
// State stores
services.AddSingleton(new ProcessedUidStore(dataDir));
services.AddSingleton(new TrustedSenderStore(dataDir));
// Services
services.AddSingleton<ActivityLog>();
services.AddSingleton(sp =>
{
var opts = sp.GetRequiredService<IOptions<SpamGuardOptions>>().Value;
return new ActivityLog(opts.Monitoring.MaxActivityLogEntries);
});
services.AddSingleton<ImapClientFactory>();
services.AddSingleton<EmailClassifier>();
services.AddHttpClient<EmailClassifier>();
// EmailClassifier with managed HttpClient (timeout + base address)
services.AddHttpClient<EmailClassifier>(client =>
{
client.Timeout = TimeSpan.FromSeconds(30);
client.BaseAddress = new Uri("https://api.anthropic.com/");
});
// Background services
services.AddSingleton<InboxMonitorService>();
@@ -72,6 +88,8 @@ static class Program
}
finally
{
host?.StopAsync().GetAwaiter().GetResult();
host?.Dispose();
Log.CloseAndFlush();
}
}

6
src/SpamGuard/Services/EmailClassifier.cs
View File

@@ -74,7 +74,7 @@ public sealed partial class EmailClassifier
};
var json = JsonSerializer.Serialize(requestBody);
var request = new HttpRequestMessage(HttpMethod.Post, "https://api.anthropic.com/v1/messages")
var request = new HttpRequestMessage(HttpMethod.Post, "v1/messages")
{
Content = new StringContent(json, Encoding.UTF8, "application/json")
};
@@ -85,7 +85,7 @@ public sealed partial class EmailClassifier
response.EnsureSuccessStatusCode();
var responseJson = await response.Content.ReadAsStringAsync(ct);
var doc = JsonDocument.Parse(responseJson);
using var doc = JsonDocument.Parse(responseJson);
var text = doc.RootElement
.GetProperty("content")[0]
.GetProperty("text")
@@ -110,7 +110,7 @@ public sealed partial class EmailClassifier
try
{
var doc = JsonDocument.Parse(cleaned);
using var doc = JsonDocument.Parse(cleaned);
var root = doc.RootElement;
return new ClassificationResult(

68
src/SpamGuard/Services/InboxMonitorService.cs
View File

@@ -11,7 +11,7 @@ using SpamGuard.Configuration;
using SpamGuard.Models;
using SpamGuard.State;
public sealed class InboxMonitorService : BackgroundService
public sealed partial class InboxMonitorService : BackgroundService
{
private readonly ImapClientFactory _imapFactory;
private readonly TrustedSenderStore _trustedSenders;
@@ -22,6 +22,7 @@ public sealed class InboxMonitorService : BackgroundService
private readonly ILogger<InboxMonitorService> _logger;
private volatile bool _paused;
private uint _lastSeenUid;
public bool IsPaused => _paused;
public void Pause() => _paused = true;
@@ -84,12 +85,25 @@ public sealed class InboxMonitorService : BackgroundService
var inbox = client.Inbox;
await inbox.OpenAsync(FolderAccess.ReadWrite, ct);
// Build search query: recent messages only
var since = _processedUids.Count == 0
? SearchQuery.DeliveredAfter(DateTime.UtcNow.AddDays(-_options.Monitoring.InitialScanDays))
: SearchQuery.All;
var uids = await inbox.SearchAsync(since, ct);
// Build search query: only fetch new messages
IList<UniqueId> uids;
if (_lastSeenUid > 0)
{
var range = new UniqueIdRange(new UniqueId(_lastSeenUid + 1), UniqueId.MaxValue);
uids = await inbox.SearchAsync(range, SearchQuery.All, ct);
}
else if (_processedUids.Count > 0)
{
// Resuming from persisted state -- scan recent messages only
uids = await inbox.SearchAsync(
SearchQuery.DeliveredAfter(DateTime.UtcNow.AddDays(-1)), ct);
}
else
{
// First ever run -- initial scan window
uids = await inbox.SearchAsync(
SearchQuery.DeliveredAfter(DateTime.UtcNow.AddDays(-_options.Monitoring.InitialScanDays)), ct);
}
_logger.LogDebug("Found {Count} messages in inbox", uids.Count);
// Find the spam/junk folder
@@ -98,7 +112,10 @@ public sealed class InboxMonitorService : BackgroundService
foreach (var uid in uids)
{
if (_processedUids.Contains(uid.Id))
{
if (uid.Id > _lastSeenUid) _lastSeenUid = uid.Id;
continue;
}
try
{
@@ -109,8 +126,10 @@ public sealed class InboxMonitorService : BackgroundService
_logger.LogError(ex, "Error processing UID={Uid}", uid.Id);
_activityLog.Add(new ActivityEntry(
DateTime.UtcNow, "", $"UID {uid.Id}", Verdict.Error, null, ex.Message));
_processedUids.Add(uid.Id); // Skip on next run to avoid infinite retry
_processedUids.Add(uid.Id);
}
if (uid.Id > _lastSeenUid) _lastSeenUid = uid.Id;
}
await client.DisconnectAsync(true, ct);
@@ -182,15 +201,30 @@ public sealed class InboxMonitorService : BackgroundService
private static string ExtractBodySnippet(MimeMessage message)
{
var text = message.TextBody ?? message.HtmlBody ?? "";
var text = message.TextBody;
// Strip HTML tags if we fell back to HTML body
if (message.TextBody == null && message.HtmlBody != null)
text = System.Text.RegularExpressions.Regex.Replace(text, "<[^>]+>", " ");
if (text == null && message.HtmlBody != null)
{
// Strip script and style blocks first, then remaining tags
text = StripScriptStyle().Replace(message.HtmlBody, " ");
text = StripHtmlTags().Replace(text, " ");
text = System.Net.WebUtility.HtmlDecode(text);
text = CollapseWhitespace().Replace(text, " ").Trim();
}
text ??= "";
return text.Length > 2000 ? text[..2000] : text;
}
[System.Text.RegularExpressions.GeneratedRegex(@"<(script|style)[^>]*>[\s\S]*?</\1>", System.Text.RegularExpressions.RegexOptions.IgnoreCase)]
private static partial System.Text.RegularExpressions.Regex StripScriptStyle();
[System.Text.RegularExpressions.GeneratedRegex(@"<[^>]+>")]
private static partial System.Text.RegularExpressions.Regex StripHtmlTags();
[System.Text.RegularExpressions.GeneratedRegex(@"\s{2,}")]
private static partial System.Text.RegularExpressions.Regex CollapseWhitespace();
private async Task<IMailFolder?> FindSpamFolderAsync(MailKit.Net.Imap.ImapClient client, CancellationToken ct)
{
// Try special folder first
@@ -199,7 +233,10 @@ public sealed class InboxMonitorService : BackgroundService
var junk = client.GetFolder(MailKit.SpecialFolder.Junk);
if (junk != null) return junk;
}
catch { }
catch (Exception ex)
{
_logger.LogDebug(ex, "Could not get Junk special folder");
}
// Fall back to configured folder name
try
@@ -209,7 +246,10 @@ public sealed class InboxMonitorService : BackgroundService
return folders.FirstOrDefault(f =>
f.Name.Equals(_options.Monitoring.SpamFolderName, StringComparison.OrdinalIgnoreCase));
}
catch { }
catch (Exception ex)
{
_logger.LogDebug(ex, "Could not find spam folder by name '{FolderName}'", _options.Monitoring.SpamFolderName);
}
return null;
}

11
src/SpamGuard/Services/TrustedSenderService.cs
View File

@@ -16,6 +16,7 @@ public sealed class TrustedSenderService : BackgroundService
private readonly TrustedSenderStore _store;
private readonly SpamGuardOptions _options;
private readonly ILogger<TrustedSenderService> _logger;
private bool _initialScanDone;
public TrustedSenderService(
ImapClientFactory imapFactory,
@@ -63,8 +64,13 @@ public sealed class TrustedSenderService : BackgroundService
await sentFolder.OpenAsync(FolderAccess.ReadOnly, ct);
var uids = await sentFolder.SearchAsync(SearchQuery.All, ct);
_logger.LogDebug("Found {Count} messages in Sent folder", uids.Count);
// After initial full scan, only check messages from the last refresh period
var query = _initialScanDone
? SearchQuery.DeliveredAfter(DateTime.UtcNow.AddMinutes(-_options.Monitoring.TrustedSenderRefreshMinutes))
: SearchQuery.All;
var uids = await sentFolder.SearchAsync(query, ct);
_logger.LogDebug("Found {Count} messages in Sent folder to scan", uids.Count);
var addresses = new List<string>();
foreach (var uid in uids)
@@ -74,6 +80,7 @@ public sealed class TrustedSenderService : BackgroundService
}
_store.AddRange(addresses);
_initialScanDone = true;
await client.DisconnectAsync(true, ct);
}

4
src/SpamGuard/State/ProcessedUidStore.cs
View File

@@ -52,7 +52,9 @@ public sealed class ProcessedUidStore
lock (_lock)
{
var json = JsonSerializer.Serialize(_uids);
File.WriteAllText(_filePath, json);
var tempPath = _filePath + ".tmp";
File.WriteAllText(tempPath, json);
File.Move(tempPath, _filePath, overwrite: true);
}
}

4
src/SpamGuard/State/TrustedSenderStore.cs
View File

@@ -50,7 +50,9 @@ public sealed class TrustedSenderStore
lock (_lock)
{
var json = JsonSerializer.Serialize(_senders);
File.WriteAllText(_filePath, json);
var tempPath = _filePath + ".tmp";
File.WriteAllText(tempPath, json);
File.Move(tempPath, _filePath, overwrite: true);
}
}

25
src/SpamGuard/Tray/IconGenerator.cs
View File

@@ -2,10 +2,23 @@ namespace SpamGuard.Tray;
using System.Drawing;
using System.Drawing.Drawing2D;
using System.Runtime.InteropServices;
public static class IconGenerator
{
public static Icon CreateCircleIcon(Color color, int size = 16)
private static readonly Lazy<Icon> _green = new(() => CreateCircleIcon(Color.FromArgb(76, 175, 80)));
private static readonly Lazy<Icon> _yellow = new(() => CreateCircleIcon(Color.FromArgb(255, 193, 7)));
private static readonly Lazy<Icon> _red = new(() => CreateCircleIcon(Color.FromArgb(244, 67, 54)));
public static Icon Green => _green.Value;
public static Icon Yellow => _yellow.Value;
public static Icon Red => _red.Value;
[DllImport("user32.dll", SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool DestroyIcon(IntPtr handle);
private static Icon CreateCircleIcon(Color color, int size = 16)
{
using var bitmap = new Bitmap(size, size);
using var graphics = Graphics.FromImage(bitmap);
@@ -15,14 +28,12 @@ public static class IconGenerator
using var brush = new SolidBrush(color);
graphics.FillEllipse(brush, 1, 1, size - 2, size - 2);
// Add a subtle border
using var pen = new Pen(Color.FromArgb(100, 0, 0, 0), 1);
graphics.DrawEllipse(pen, 1, 1, size - 3, size - 3);
return Icon.FromHandle(bitmap.GetHicon());
var hIcon = bitmap.GetHicon();
var icon = (Icon)Icon.FromHandle(hIcon).Clone();
DestroyIcon(hIcon);
return icon;
}
public static Icon Green => CreateCircleIcon(Color.FromArgb(76, 175, 80));
public static Icon Yellow => CreateCircleIcon(Color.FromArgb(255, 193, 7));
public static Icon Red => CreateCircleIcon(Color.FromArgb(244, 67, 54));
}

3
src/SpamGuard/Tray/TrayApplicationContext.cs
View File

@@ -48,7 +48,8 @@ public sealed class TrayApplicationContext : ApplicationContext
{
var checked_ = _activityLog.TodayChecked;
var spam = _activityLog.TodaySpam;
_notifyIcon.Text = $"SpamGuard - {checked_} checked, {spam} spam caught today";
var tooltip = $"SpamGuard - {checked_} checked, {spam} spam caught today";
_notifyIcon.Text = tooltip.Length > 127 ? tooltip[..127] : tooltip;
// Update icon based on state
if (!_monitor.IsPaused)

38
src/SpamGuard/appsettings.template.json Normal file
View File

@@ -0,0 +1,38 @@
{
"SpamGuard": {
"Imap": {
"Host": "imap.example.com",
"Port": 993,
"UseSsl": true,
"Username": "user@example.com",
"Password": ""
},
"Claude": {
"ApiKey": "",
"Model": "claude-sonnet-4-6",
"MaxBodyLength": 2000
},
"Monitoring": {
"PollIntervalSeconds": 60,
"TrustedSenderRefreshMinutes": 60,
"SpamConfidenceThreshold": 0.7,
"SpamFolderName": "Junk",
"InitialScanDays": 7,
"MaxActivityLogEntries": 500
}
},
"Serilog": {
"MinimumLevel": "Information",
"WriteTo": [
{ "Name": "Console" },
{
"Name": "File",
"Args": {
"path": "%APPDATA%/SpamGuard/logs/spamguard-.log",
"rollingInterval": "Day",
"retainedFileCountLimit": 7
}
}
]
}
}

10
tests/SpamGuard.Tests/UnitTest1.cs
View File

@@ -1,10 +0,0 @@
namespace SpamGuard.Tests;
public class UnitTest1
{
[Fact]
public void Test1()
{
}
}