From 773768035d96f95c0539ee1e15a37fa385b05b26 Mon Sep 17 00:00:00 2001 From: Reinier Date: Mon, 1 Jun 2026 17:56:24 +0000 Subject: [PATCH 01/12] Newsletter Handlebars templates, ACS custom domain, and web OTel fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Refactor newsletter emails to use Handlebars templating with hot-reload in dev, migrate ACS from managed to custom email domain, fix HEAD request handling on extension-less paths, and suppress Blazor ComponentHub noise in App Insights. Implementation: • Newsletter: Replaced 3 monolithic HTML templates with 6 modular Handlebars partials (shell, footer, account-action, weekly-content, daily-content, admin-content) for composability; NewsletterService now compiles templates via HandlebarsDotNet at startup, with live file reload in development mode • Newsletter: Added 429 and generic RequestFailedException error handling to AcsEmailSender.SendEmailAsync to prevent unhandled exceptions on rate limits • Newsletter: Added HandlebarsDotNet NuGet package and IHostEnvironment injection to NewsletterService; updated appsettings.json and tests accordingly • ACS infrastructure: Migrated communication.bicep from AzureManagedDomain to CustomerManaged custom domain; added newsletterSenderUsername resource and linkEmailDomain flag for safe post-DNS-verification linking • Infrastructure: Moved ACME DNS zone from rg-techhub-shared to rg-techhub-prod; updated Renew-WildcardCertificates.ps1, Deploy-Infrastructure.ps1, Bicep params, and documentation accordingly • Web: Added HeadRequestMiddleware that responds 200 immediately for extension-less HEAD requests (avoiding ~23s Blazor SSR timeouts) and rewrites HEAD to GET for static assets; registered via app.UseHeadRequestHandling() in Program.cs • Web: Added BlazorHubNoiseSuppressor (BaseProcessor) that clears ActivityTraceFlags.Recorded for failed ComponentHub activities (disconnected Blazor Server circuits showing success=false resultCode=0 in App Insights) • ServiceDefaults: Extended AddServiceDefaults/ConfigureOpenTelemetry with optional additionalActivityProcessor parameter for processor injection without requiring direct OTel package references in consuming projects • Tests: Added HeadRequestMiddlewareTests and BlazorHubNoiseSuppressorTests covering all branches; updated NewsletterServiceTests and E2E AboutPageTests --- .devcontainer/post-create.sh | 6 +- .github/workflows/cd.yml | 5 + Directory.Packages.props | 4 +- docs/wildcard-certificates.md | 10 +- infra/infrastructure.bicep | 8 + infra/modules/communication.bicep | 23 +- .../parameters/prod-infrastructure.bicepparam | 1 + scripts/Deploy-Infrastructure.ps1 | 34 ++- scripts/Renew-WildcardCertificates.ps1 | 8 +- .../Services/NewsletterBackgroundService.cs | 21 +- src/TechHub.Api/appsettings.json | 2 +- .../newsletter-account-action-content.html | 5 + .../Resources/newsletter-admin-content.html | 6 + .../newsletter-admin-status-template.html | 52 ---- .../Resources/newsletter-daily-content.html | 13 + .../newsletter-daily-overview-template.html | 33 --- .../newsletter-subscriber-footer.html | 5 + .../Data/Resources/newsletter-template.html | 63 +++++ .../Resources/newsletter-weekly-content.html | 26 ++ .../newsletter-weekly-digest-template.html | 32 --- .../Services/Newsletter/AcsEmailSender.cs | 17 +- .../Services/Newsletter/NewsletterService.cs | 245 +++++++++--------- .../TechHub.Infrastructure.csproj | 10 +- src/TechHub.ServiceDefaults/Extensions.cs | 23 +- .../Components/Layout/MainLayout.razor | 2 +- src/TechHub.Web/Components/Pages/About.razor | 22 -- .../Middleware/HeadRequestMiddleware.cs | 91 +++++++ src/TechHub.Web/Program.cs | 45 ++-- .../Telemetry/BlazorHubNoiseSuppressor.cs | 48 ++++ tests/TechHub.E2E.Tests/Web/AboutPageTests.cs | 10 - .../Services/NewsletterServiceTests.cs | 7 +- .../Middleware/HeadRequestMiddlewareTests.cs | 168 ++++++++++++ .../BlazorHubNoiseSuppressorTests.cs | 111 ++++++++ 33 files changed, 819 insertions(+), 337 deletions(-) create mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-account-action-content.html create mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html delete mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-admin-status-template.html create mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-daily-content.html delete mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-daily-overview-template.html create mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-subscriber-footer.html create mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-template.html create mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-content.html delete mode 100644 src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-digest-template.html create mode 100644 src/TechHub.Web/Middleware/HeadRequestMiddleware.cs create mode 100644 src/TechHub.Web/Telemetry/BlazorHubNoiseSuppressor.cs create mode 100644 tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs create mode 100644 tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh index b1840f77c..242943eec 100644 --- a/.devcontainer/post-create.sh +++ b/.devcontainer/post-create.sh @@ -144,11 +144,11 @@ fi echo "Installing PowerShell modules..." pwsh -Command 'Install-Module HtmlToMarkdown -AcceptLicense -Force' pwsh -Command 'Install-Module Pester -Force -SkipPublisherCheck -MinimumVersion "5.0.0" -Scope CurrentUser' -# Az.Accounts + Az.Resources are the only Az sub-modules needed for Deploy-Infrastructure.ps1 -# (New-AzDeployment, Test-AzDeployment, Get-AzContext). Installing just these avoids pulling -# the full Az bundle (~80 modules). +# Install only the Az sub-modules required by the deployment scripts. +# Az.PostgreSql is needed for Get-AzPostgreSqlFlexibleServerFirewallRule in Deploy-Infrastructure.ps1. pwsh -Command 'Install-Module Az.Accounts -Force -Scope CurrentUser' pwsh -Command 'Install-Module Az.Resources -Force -Scope CurrentUser' +pwsh -Command 'Install-Module Az.PostgreSql -Force -Scope CurrentUser' # ==================== PowerShell Profile ==================== echo "Setting up PowerShell profile..." diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 0660f498d..bbba5ee33 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -595,6 +595,11 @@ jobs: subscription-id: ${{ vars.AZURE_SUBSCRIPTION_ID }} enable-AzPSSession: true + - name: Install Az.PostgreSql module + if: steps.changes.outputs.infra-changed == 'true' + shell: pwsh + run: Install-Module Az.PostgreSql -Force -Scope CurrentUser -ErrorAction Stop + - name: Deploy infrastructure (Phase 1) if: steps.changes.outputs.infra-changed == 'true' shell: pwsh diff --git a/Directory.Packages.props b/Directory.Packages.props index 16e6ecf8a..b864a8923 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -2,7 +2,6 @@ true - @@ -11,6 +10,7 @@ + @@ -43,4 +43,4 @@ - + \ No newline at end of file diff --git a/docs/wildcard-certificates.md b/docs/wildcard-certificates.md index 5fc667065..9a1de9684 100644 --- a/docs/wildcard-certificates.md +++ b/docs/wildcard-certificates.md @@ -37,11 +37,11 @@ These are permanent — they never need updating. ### One-time GoDaddy NS delegation -After deploying the shared infrastructure (which creates the `acme.hub.ms` zone), add NS records for `acme` in the `hub.ms` zone at GoDaddy pointing to the Azure DNS nameservers: +After deploying the infrastructure (which creates the `acme.hub.ms` zone), add NS records for `acme` in the `hub.ms` zone at GoDaddy pointing to the Azure DNS nameservers: ```bash -# Get the nameservers after deploying shared infra -az network dns zone show --resource-group rg-techhub-shared --name acme.hub.ms --query nameServers -o tsv +# Get the nameservers after deploying infra +az network dns zone show --resource-group rg-techhub-prod --name acme.hub.ms --query nameServers -o tsv ``` Add each returned nameserver as an NS record for `acme` in GoDaddy's `hub.ms` zone. @@ -57,8 +57,8 @@ pip install certbot certbot-dns-azure The ACME DNS zone is deployed as part of shared infrastructure: - **Bicep module**: `infra/modules/acmeDnsZone.bicep` -- **Deployed by**: `infra/shared.bicep` → `acmeDnsZone` module -- **Resource**: `Microsoft.Network/dnsZones` — `acme.hub.ms` in `rg-techhub-shared` +- **Deployed by**: `infra/infrastructure.bicep` → `acmeDnsZone` module +- **Resource**: `Microsoft.Network/dnsZones` — `acme.hub.ms` in `rg-techhub-prod` Deploy with: diff --git a/infra/infrastructure.bicep b/infra/infrastructure.bicep index 6e74e00d4..c9c4876ef 100644 --- a/infra/infrastructure.bicep +++ b/infra/infrastructure.bicep @@ -80,6 +80,12 @@ param emailServiceName string = 'eml-techhub-prod' @description('Communication Service name') param communicationServiceName string = 'acs-techhub-prod' +@description('Custom email sending domain linked to ACS (e.g. mail.hub.ms). Must be verified in ACS after deploy.') +param customEmailDomain string = 'mail.hub.ms' + +@description('Set to true only after the custom domain DNS records have been added and verified in ACS. Redeploy after verification.') +param linkEmailDomain bool = false + @description('Common tags applied to all resources managed by this template') param commonTags object = { owner: 'techhub-maintainer' @@ -305,6 +311,8 @@ module communication './modules/communication.bicep' = { name: 'communication-${deploymentSuffix}' params: { emailServiceName: emailServiceName + customEmailDomain: customEmailDomain + linkEmailDomain: linkEmailDomain communicationServiceName: communicationServiceName tags: prodTags } diff --git a/infra/modules/communication.bicep b/infra/modules/communication.bicep index b414722d0..00dd0a813 100644 --- a/infra/modules/communication.bicep +++ b/infra/modules/communication.bicep @@ -1,6 +1,8 @@ param emailServiceName string -param managedDomainName string = 'AzureManagedDomain' +param customEmailDomain string param communicationServiceName string +@description('Set to true only after the custom domain DNS records have been added and verified in ACS.') +param linkEmailDomain bool = false param tags object = {} resource emailService 'Microsoft.Communication/emailServices@2026-03-18' = { @@ -14,26 +16,33 @@ resource emailService 'Microsoft.Communication/emailServices@2026-03-18' = { resource domain 'Microsoft.Communication/emailServices/domains@2026-03-18' = { parent: emailService - name: managedDomainName + name: customEmailDomain location: 'global' properties: { - domainManagement: 'AzureManaged' + domainManagement: 'CustomerManaged' userEngagementTracking: 'Disabled' } } +resource newsletterSenderUsername 'Microsoft.Communication/emailServices/domains/senderUsernames@2026-03-18' = { + parent: domain + name: 'newsletter' + properties: { + username: 'newsletter' + displayName: 'TechHub Newsletter' + } +} + resource communicationService 'Microsoft.Communication/communicationServices@2026-03-18' = { name: communicationServiceName location: 'global' tags: tags properties: { dataLocation: 'Europe' - linkedDomains: [ - domain.id - ] + linkedDomains: linkEmailDomain ? [domain.id] : [] } } output communicationServiceEndpoint string = 'https://${communicationService.name}.communication.azure.com/' output communicationServiceId string = communicationService.id -output senderAddress string = 'DoNotReply@${domain.properties.mailFromSenderDomain}' +output senderAddress string = 'newsletter@${domain.properties.mailFromSenderDomain}' diff --git a/infra/parameters/prod-infrastructure.bicepparam b/infra/parameters/prod-infrastructure.bicepparam index 1125b91f6..797bdbfdc 100644 --- a/infra/parameters/prod-infrastructure.bicepparam +++ b/infra/parameters/prod-infrastructure.bicepparam @@ -25,3 +25,4 @@ param adminIpAddresses = readEnvironmentVariable('ADMIN_IP_ADDRESSES') param alertEmailAddress = 'reinier.vanmaanen@xebia.com' param monthlyBudgetAmount = 250 param budgetStartDate = '2026-04-01' +param linkEmailDomain = true diff --git a/scripts/Deploy-Infrastructure.ps1 b/scripts/Deploy-Infrastructure.ps1 index 110f55f48..5490664a3 100644 --- a/scripts/Deploy-Infrastructure.ps1 +++ b/scripts/Deploy-Infrastructure.ps1 @@ -37,7 +37,10 @@ param( [string]$Mode = 'whatif', [Parameter(Mandatory = $false)] - [string]$Location = 'swedencentral' + [string]$Location = 'swedencentral', + + [Parameter(Mandatory = $false)] + [switch]$LinkEmailDomain ) $ErrorActionPreference = "Stop" @@ -222,18 +225,33 @@ if ($Mode -eq 'deploy') { $savedVerbose = $VerbosePreference $VerbosePreference = 'Continue' try { - New-AzDeployment ` - -Name $deploymentName ` - -Location $Location ` - -TemplateFile $infraTemplateFile ` - -TemplateParameterFile $infraParamsFile ` - -SkipTemplateParameterPrompt ` - -OutVariable infraResult | Out-Null + $effectiveParamsFile = $infraParamsFile + $tempParamsFile = $null + if ($LinkEmailDomain) { + Write-Warn "LinkEmailDomain is set — domain will be linked to ACS. Only use this after DNS verification." + # Write temp file to the same directory so the 'using' relative path remains valid + $tempParamsFile = Join-Path (Split-Path $infraParamsFile) "prod-infrastructure-linkdomain.bicepparam" + (Get-Content $infraParamsFile -Raw) ` + -replace 'param linkEmailDomain = false', 'param linkEmailDomain = true' | + Set-Content $tempParamsFile + $effectiveParamsFile = $tempParamsFile + } + $deployParams = @{ + Name = $deploymentName + Location = $Location + TemplateFile = $infraTemplateFile + TemplateParameterFile = $effectiveParamsFile + SkipTemplateParameterPrompt = $true + } + New-AzDeployment @deployParams -OutVariable infraResult | Out-Null } catch { Write-Fail "Infrastructure deployment failed: $_" exit 1 } finally { $VerbosePreference = $savedVerbose + if ($tempParamsFile -and (Test-Path $tempParamsFile)) { + Remove-Item $tempParamsFile -Force + } } Write-Ok "Base infrastructure deployed successfully" diff --git a/scripts/Renew-WildcardCertificates.ps1 b/scripts/Renew-WildcardCertificates.ps1 index c7d2f1efe..814f5d59c 100644 --- a/scripts/Renew-WildcardCertificates.ps1 +++ b/scripts/Renew-WildcardCertificates.ps1 @@ -12,8 +12,8 @@ Prerequisites: - certbot and certbot-dns-azure installed (pip install certbot certbot-dns-azure) - - Azure CLI authenticated with access to rg-techhub-shared - - ACME DNS zone deployed (acme.hub.ms in rg-techhub-shared) + - Azure CLI authenticated with access to rg-techhub-prod + - ACME DNS zone deployed (acme.hub.ms in rg-techhub-prod) - GoDaddy CNAME records configured (see docs/wildcard-certificates.md) .PARAMETER KeyVaultName @@ -51,7 +51,7 @@ param( [string]$KeyVaultName = 'kv-techhub-prod', [Parameter(Mandatory = $false)] - [string]$ResourceGroup = 'rg-techhub-shared', + [string]$ResourceGroup = 'rg-techhub-prod', [Parameter(Mandatory = $false)] [string]$AcmeDnsZone = 'acme.hub.ms', @@ -147,7 +147,7 @@ Write-Ok "certbot-dns-azure plugin found" az network dns zone show --resource-group $ResourceGroup --name $AcmeDnsZone -o json 2>&1 | Out-Null if ($LASTEXITCODE -ne 0) { Write-Fail "DNS zone '$AcmeDnsZone' not found in resource group '$ResourceGroup'." - Write-Detail "Deploy shared infrastructure first: ./scripts/Deploy-Infrastructure.ps1 -Environment shared -Mode deploy" + Write-Detail "Deploy infrastructure first: ./scripts/Deploy-Infrastructure.ps1 -Mode deploy" exit 1 } Write-Ok "ACME DNS zone '$AcmeDnsZone' exists" diff --git a/src/TechHub.Api/Services/NewsletterBackgroundService.cs b/src/TechHub.Api/Services/NewsletterBackgroundService.cs index cb3d049d3..0b122a130 100644 --- a/src/TechHub.Api/Services/NewsletterBackgroundService.cs +++ b/src/TechHub.Api/Services/NewsletterBackgroundService.cs @@ -76,7 +76,15 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) if (completed == manualTask) { _manualTrigger = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - await RunManualAsync(stoppingToken); + try + { + await RunManualAsync(stoppingToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogError(ex, "Unhandled exception in newsletter manual run — service will continue"); + } + continue; } @@ -87,8 +95,15 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) continue; } - await SendLatestRoundupsAsync(stoppingToken); - await SendScheduledDailyEmailsAsync(stoppingToken); + try + { + await SendLatestRoundupsAsync(stoppingToken); + await SendScheduledDailyEmailsAsync(stoppingToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogError(ex, "Unhandled exception in newsletter periodic run — service will continue"); + } } } diff --git a/src/TechHub.Api/appsettings.json b/src/TechHub.Api/appsettings.json index 59f1691d2..4a5b989f1 100644 --- a/src/TechHub.Api/appsettings.json +++ b/src/TechHub.Api/appsettings.json @@ -429,7 +429,7 @@ "AdminReportEmailAddress": "whistler112@gmail.com", "DailyDigestHourLocal": 9, "DailyDigestTimeZoneId": "Europe/Brussels", - "SendDelayMs": 200 + "SendDelayMs": 2500 }, "AiCategorization": { "Endpoint": "", diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-account-action-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-account-action-content.html new file mode 100644 index 000000000..dc724e890 --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-account-action-content.html @@ -0,0 +1,5 @@ +

{{message}}

+

+ {{actionLabel}} → +

+

Or copy this link into your browser:
{{actionUrl}}

diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html new file mode 100644 index 000000000..7eb7bc02b --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html @@ -0,0 +1,6 @@ +

New content items (24h): {{newContentItems}}

+

Failed processed URLs (24h): {{failedProcessedUrls}}

+

Failed background jobs (24h): {{failedJobs}}

+

New newsletter subscribers (24h): {{newSubscribers}}

+

Active subscribers: {{activeSubscribers}}

+

Unconfirmed subscribers: {{unconfirmedSubscribers}}

diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-status-template.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-status-template.html deleted file mode 100644 index fc6759a5c..000000000 --- a/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-status-template.html +++ /dev/null @@ -1,52 +0,0 @@ - - - - - - - - - - - - - -
- - - - - - - -
TechHub Daily Status Report — {Date}
- - - - - - - - - - - - - - - - - - - - - - - - - -
New content items (24h){NewContentItems}
Failed processed URLs (24h){FailedUrls}
Failed background jobs (24h){FailedJobs}
New newsletter subscribers (24h){NewSubscribers}
Active subscribers{ActiveSubscribers}
Unconfirmed subscribers{UnconfirmedSubscribers}
-
-
- - diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-content.html new file mode 100644 index 000000000..fc427d5cd --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-content.html @@ -0,0 +1,13 @@ +

{{date}}

+{{#each sections}} +

{{title}}

+
    + {{#each items}} +
  • + {{title}} + ({{collectionName}}) +
    • + {{/each}} +
+{{/each}} +{{{footer}}} diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-overview-template.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-overview-template.html deleted file mode 100644 index 931974c31..000000000 --- a/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-overview-template.html +++ /dev/null @@ -1,33 +0,0 @@ - - - - - - - - - - - - - -
- - - - - - - -
TechHub Daily Overview
-

{Date}

- {Content} -

- You received this because you subscribed to TechHub newsletters.
- Manage subscription  ·  - Unsubscribe -

-
-
- - diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-subscriber-footer.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-subscriber-footer.html new file mode 100644 index 000000000..a3f4036df --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-subscriber-footer.html @@ -0,0 +1,5 @@ +

+ You received this because you subscribed to TechHub newsletters.
+ Manage subscription  ·  + Unsubscribe +

diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-template.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-template.html new file mode 100644 index 000000000..51c345601 --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-template.html @@ -0,0 +1,63 @@ + + + + + + + + + + + + + + +
+ + + + + + + + +
+ + diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-content.html new file mode 100644 index 000000000..0914b4dbc --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-content.html @@ -0,0 +1,26 @@ +{{#each roundups}} +{{#unless @first}}
{{/unless}} +
+

{{sectionTitle}}

+

{{title}}

+

{{introduction}}

+

In this roundup

+
+
    + {{#each links}} +
  • {{text}}
    • + {{/each}} +
+
+
+ + + + +
+ 📑 Read the full {{sectionTitle}} roundup → +
+
+
+{{/each}} +{{{footer}}} diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-digest-template.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-digest-template.html deleted file mode 100644 index bf4935043..000000000 --- a/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-digest-template.html +++ /dev/null @@ -1,32 +0,0 @@ - - - - - - - - - - - - - -
- - - - - - - -
TechHub Weekly Digest
- {Content} -

- You received this because you subscribed to TechHub newsletters.
- Manage subscription  ·  - Unsubscribe -

-
-
- - diff --git a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs index e4664e568..8059e18f7 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs @@ -45,7 +45,22 @@ public async Task SendAsync(string recipientEmail, string subject, string }, recipients: new EmailRecipients([new EmailAddress(recipientEmail)])); - var operation = await _emailClient.SendAsync(WaitUntil.Completed, emailMessage, ct); + EmailSendOperation operation; + try + { + operation = await _emailClient.SendAsync(WaitUntil.Completed, emailMessage, ct); + } + catch (RequestFailedException ex) when (ex.Status == 429) + { + _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429). Email to {RecipientEmail} was not sent", recipientEmail); + return false; + } + catch (RequestFailedException ex) + { + _logger.LogError(ex, "Newsletter email send failed with ACS error {Status} for {RecipientEmail}", ex.Status, recipientEmail); + return false; + } + if (operation.Value.Status != EmailSendStatus.Succeeded) { _logger.LogWarning("Newsletter email delivery completed with non-success status {Status}", operation.Value.Status); diff --git a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs index 3022ffe6d..0ea212a71 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs @@ -1,11 +1,12 @@ using System.Data; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.Net; using System.Reflection; using System.Security.Cryptography; using System.Text; using Dapper; +using HandlebarsDotNet; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using TechHub.Core.Configuration; @@ -27,9 +28,12 @@ public sealed class NewsletterService : INewsletterService private readonly AppSettings _appSettings; private readonly IEmailSender _emailSender; private readonly ILogger _logger; - private readonly string _weeklyDigestTemplate; - private readonly string _dailyOverviewTemplate; - private readonly string _adminStatusTemplate; + private readonly HandlebarsTemplate _shellTemplate; + private readonly HandlebarsTemplate _subscriberFooterTemplate; + private readonly HandlebarsTemplate _accountActionTemplate; + private readonly HandlebarsTemplate _weeklyContentTemplate; + private readonly HandlebarsTemplate _dailyContentTemplate; + private readonly HandlebarsTemplate _adminContentTemplate; public NewsletterService( IDbConnection connection, @@ -38,7 +42,8 @@ public NewsletterService( IOptions options, IOptions appSettings, IEmailSender emailSender, - ILogger logger) + ILogger logger, + IHostEnvironment env) { ArgumentNullException.ThrowIfNull(connection); ArgumentNullException.ThrowIfNull(subscriberRepository); @@ -47,6 +52,7 @@ public NewsletterService( ArgumentNullException.ThrowIfNull(appSettings); ArgumentNullException.ThrowIfNull(emailSender); ArgumentNullException.ThrowIfNull(logger); + ArgumentNullException.ThrowIfNull(env); _connection = connection; _subscriberRepository = subscriberRepository; @@ -55,9 +61,25 @@ public NewsletterService( _appSettings = appSettings.Value; _emailSender = emailSender; _logger = logger; - _weeklyDigestTemplate = LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-weekly-digest-template.html"); - _dailyOverviewTemplate = LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-daily-overview-template.html"); - _adminStatusTemplate = LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-admin-status-template.html"); + if (env.IsDevelopment()) + { + var resourcesPath = Path.GetFullPath(Path.Combine(env.ContentRootPath, "..", "TechHub.Infrastructure", "Data", "Resources")); + _shellTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-template.html"))); + _subscriberFooterTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-subscriber-footer.html"))); + _accountActionTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-account-action-content.html"))); + _weeklyContentTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-weekly-content.html"))); + _dailyContentTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-daily-content.html"))); + _adminContentTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-admin-content.html"))); + } + else + { + _shellTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-template.html")); + _subscriberFooterTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-subscriber-footer.html")); + _accountActionTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-account-action-content.html")); + _weeklyContentTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-weekly-content.html")); + _dailyContentTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-daily-content.html")); + _adminContentTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-admin-content.html")); + } } public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlugs, string? sendTargetKey = null, CancellationToken ct = default) @@ -297,14 +319,11 @@ public async Task SendManageLinkEmailAsync(string email, CancellationToken } var manageUrl = BuildManageUrl(email, secret); - var html = $""" - -

Manage your TechHub newsletter subscription

-

Click the button below to manage your subscription preferences.

-

Manage subscription

-

Or copy this link into your browser:
{WebUtility.HtmlEncode(manageUrl)}

- - """; + var html = BuildAccountActionHtml( + title: "Manage your TechHub newsletter subscription", + message: "Click the button below to manage your subscription preferences.", + actionLabel: "Manage subscription", + actionUrl: manageUrl); var text = $""" Manage your TechHub newsletter subscription @@ -465,20 +484,11 @@ public async Task SendConfirmationEmailAsync(string email, CancellationTok } var confirmUrl = BuildConfirmUrl(email); - var html = $""" - - - - - - - -

Confirm your TechHub newsletter subscription

-

Click the button below to confirm your subscription. If you did not sign up, you can safely ignore this email.

-

Confirm subscription

-

Or copy this link into your browser:
{WebUtility.HtmlEncode(confirmUrl)}

- - """; + var html = BuildAccountActionHtml( + title: "Confirm your TechHub newsletter subscription", + message: "Click the button below to confirm your subscription. If you did not sign up, you can safely ignore this email.", + actionLabel: "Confirm subscription", + actionUrl: confirmUrl); var text = $""" Confirm your TechHub newsletter subscription @@ -682,66 +692,43 @@ FROM ranked return result; } - private string BuildSectionLinksHtml(RoundupRow roundup) + private IEnumerable GetRoundupLinks(RoundupRow roundup) { - var headers = roundup.Content + var baseRoundupUrl = BuildAbsoluteUrl($"/{roundup.SectionName}/roundups/{roundup.Slug}"); + return roundup.Content .Split('\n', StringSplitOptions.RemoveEmptyEntries) .Where(line => line.StartsWith("## ", StringComparison.Ordinal)) .Select(line => line[3..].Trim()) .Distinct(StringComparer.OrdinalIgnoreCase) - .ToList(); - - var baseRoundupUrl = BuildAbsoluteUrl($"/{roundup.SectionName}/roundups/{roundup.Slug}"); - var sb = new StringBuilder(); - sb.Append(""); - return sb.ToString(); + .Select(header => (object)new + { + href = $"{baseRoundupUrl}#{RoundupContentBuilder.BuildAnchor(header)}", + text = header + }); } private string RenderCombinedWeeklyHtml(IReadOnlyList roundups, string unsubscribeUrl, string manageUrl) { - var sb = new StringBuilder(); - - for (var i = 0; i < roundups.Count; i++) + var footer = _subscriberFooterTemplate(new { manageUrl, unsubscribeUrl }); + var content = _weeklyContentTemplate(new { - var roundup = roundups[i]; - var sectionTitle = GetSectionTitle(roundup.SectionName); - var fullRoundupUrl = BuildAbsoluteUrl($"/{roundup.SectionName}/roundups/{roundup.Slug}"); - var sectionLinksHtml = BuildSectionLinksHtml(roundup); - - if (i > 0) + roundups = roundups.Select(r => new { - sb.Append("
"); - } - - sb.Append(CultureInfo.InvariantCulture, $""" -
-

{WebUtility.HtmlEncode(sectionTitle)}

-

{WebUtility.HtmlEncode(roundup.Title)}

-

{WebUtility.HtmlEncode(roundup.Introduction)}

-

📑 In this roundup

-
{sectionLinksHtml}
-

- Read the full {WebUtility.HtmlEncode(sectionTitle)} roundup → -

-
- """); - } - - return _weeklyDigestTemplate - .Replace("{Content}", sb.ToString(), StringComparison.Ordinal) - .Replace("{ManageUrl}", WebUtility.HtmlEncode(manageUrl), StringComparison.Ordinal) - .Replace("{UnsubscribeUrl}", WebUtility.HtmlEncode(unsubscribeUrl), StringComparison.Ordinal); + sectionTitle = GetSectionTitle(r.SectionName), + title = StripRoundupPrefix(r.Title), + introduction = r.Introduction, + roundupUrl = BuildAbsoluteUrl($"/{r.SectionName}/roundups/{r.Slug}"), + links = GetRoundupLinks(r) + }), + footer + }); + return _shellTemplate(new + { + title = "TechHub Weekly Digest", + cardClass = "th-card--weekly", + maxWidth = "900px", + content + }); } private string BuildCombinedWeeklyPlainText(IReadOnlyList roundups, string unsubscribeUrl, string manageUrl) @@ -757,7 +744,7 @@ private string BuildCombinedWeeklyPlainText(IReadOnlyList roundups, var fullRoundupUrl = BuildAbsoluteUrl($"/{roundup.SectionName}/roundups/{roundup.Slug}"); sb.AppendLine(sectionTitle); sb.AppendLine(new string('-', sectionTitle.Length)); - sb.AppendLine(roundup.Title); + sb.AppendLine(StripRoundupPrefix(roundup.Title)); sb.AppendLine(); sb.AppendLine(roundup.Introduction); sb.AppendLine(); @@ -774,6 +761,12 @@ private string BuildCombinedWeeklyPlainText(IReadOnlyList roundups, private string GetSectionTitle(string slug) => _appSettings.Content.Sections.TryGetValue(slug, out var config) ? config.Title : slug; + private static string StripRoundupPrefix(string title) + { + var colonIndex = title.IndexOf(": ", StringComparison.Ordinal); + return colonIndex >= 0 ? title[(colonIndex + 2)..] : title; + } + private List OrderSectionsByWebsiteOrder(IEnumerable sectionNames) { var sectionOrder = _appSettings.Content.Sections @@ -813,35 +806,31 @@ private string BuildDailyOverviewHtml( { var unsubscribeUrl = BuildUnsubscribeUrl(email); var manageUrl = BuildManageUrl(email, _options.UnsubscribeSecret); - - var sb = new StringBuilder(); - foreach (var section in sections) + var footer = _subscriberFooterTemplate(new { manageUrl, unsubscribeUrl }); + var content = _dailyContentTemplate(new { - var items = itemsBySection.TryGetValue(section, out var rows) ? rows : []; - if (items.Count == 0) - { - continue; // Section has no news today — skip it - } - - sb.Append($"

{WebUtility.HtmlEncode(GetSectionTitle(section))}

"); - sb.Append("
    "); - foreach (var item in items) - { - var filteredUrl = BuildAbsoluteUrl($"/{section}/all?search={Uri.EscapeDataString(item.Title)}&exact=true"); - sb.Append("
  • "); - sb.Append($"{WebUtility.HtmlEncode(item.Title)}"); - sb.Append($" ({WebUtility.HtmlEncode(item.CollectionName)})"); - sb.Append("
    • "); - } - - sb.Append("
"); - } - - return _dailyOverviewTemplate - .Replace("{Date}", day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{Content}", sb.ToString(), StringComparison.Ordinal) - .Replace("{ManageUrl}", WebUtility.HtmlEncode(manageUrl), StringComparison.Ordinal) - .Replace("{UnsubscribeUrl}", WebUtility.HtmlEncode(unsubscribeUrl), StringComparison.Ordinal); + date = day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture), + sections = sections + .Where(s => itemsBySection.TryGetValue(s, out var rows) && rows.Count > 0) + .Select(s => new + { + title = GetSectionTitle(s), + items = itemsBySection[s].Select(item => new + { + url = BuildAbsoluteUrl($"/{s}/all?search={Uri.EscapeDataString(item.Title)}&exact=true"), + title = item.Title, + collectionName = item.CollectionName + }) + }), + footer + }); + return _shellTemplate(new + { + title = "TechHub Daily Overview", + cardClass = "th-card--daily", + maxWidth = "1100px", + content + }); } private string BuildDailyOverviewText( @@ -878,15 +867,37 @@ private string BuildDailyOverviewText( return sb.ToString(); } - private string BuildAdminStatusHtml(DateOnly day, NewsletterDailyReportStats stats) => - _adminStatusTemplate - .Replace("{Date}", day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{NewContentItems}", stats.NewContentItemsLast24Hours.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{FailedUrls}", stats.FailedProcessedUrlsLast24Hours.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{FailedJobs}", stats.FailedJobsLast24Hours.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{NewSubscribers}", stats.NewSubscribersLast24Hours.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{ActiveSubscribers}", stats.ActiveSubscribers.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{UnconfirmedSubscribers}", stats.UnconfirmedSubscribers.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal); + private string BuildAdminStatusHtml(DateOnly day, NewsletterDailyReportStats stats) + { + var content = _adminContentTemplate(new + { + newContentItems = stats.NewContentItemsLast24Hours.ToString(CultureInfo.InvariantCulture), + failedProcessedUrls = stats.FailedProcessedUrlsLast24Hours.ToString(CultureInfo.InvariantCulture), + failedJobs = stats.FailedJobsLast24Hours.ToString(CultureInfo.InvariantCulture), + newSubscribers = stats.NewSubscribersLast24Hours.ToString(CultureInfo.InvariantCulture), + activeSubscribers = stats.ActiveSubscribers.ToString(CultureInfo.InvariantCulture), + unconfirmedSubscribers = stats.UnconfirmedSubscribers.ToString(CultureInfo.InvariantCulture) + }); + return _shellTemplate(new + { + title = $"TechHub Daily Status Report \u2014 {day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture)}", + cardClass = "th-card--admin", + maxWidth = "600px", + content + }); + } + + private string BuildAccountActionHtml(string title, string message, string actionLabel, string actionUrl) + { + var content = _accountActionTemplate(new { message, actionLabel, actionUrl }); + return _shellTemplate(new + { + title, + cardClass = "th-card--account", + maxWidth = "640px", + content + }); + } private static string BuildAdminStatusText(DateOnly day, NewsletterDailyReportStats stats) => string.Create(CultureInfo.InvariantCulture, $""" diff --git a/src/TechHub.Infrastructure/TechHub.Infrastructure.csproj b/src/TechHub.Infrastructure/TechHub.Infrastructure.csproj index 56eb79816..065587791 100644 --- a/src/TechHub.Infrastructure/TechHub.Infrastructure.csproj +++ b/src/TechHub.Infrastructure/TechHub.Infrastructure.csproj @@ -14,6 +14,7 @@ + @@ -37,9 +38,12 @@ - - - + + + + + + diff --git a/src/TechHub.ServiceDefaults/Extensions.cs b/src/TechHub.ServiceDefaults/Extensions.cs index 9664017b9..410901411 100644 --- a/src/TechHub.ServiceDefaults/Extensions.cs +++ b/src/TechHub.ServiceDefaults/Extensions.cs @@ -39,14 +39,20 @@ public static class ServiceDefaultsExtensions /// Use this to enrich or suppress activities based on response properties /// (e.g. clear ActivityTraceFlags.Recorded for 4xx responses). /// + /// + /// Optional added to the tracing pipeline. + /// Use this to suppress or enrich activities that bypass the HTTP filter (e.g. SignalR + /// hub method invocations that have no ). + /// public static IHostApplicationBuilder AddServiceDefaults( this IHostApplicationBuilder builder, Func? additionalTraceFilter = null, - Action? additionalResponseEnricher = null) + Action? additionalResponseEnricher = null, + BaseProcessor? additionalActivityProcessor = null) { ArgumentNullException.ThrowIfNull(builder); - builder.ConfigureOpenTelemetry(additionalTraceFilter, additionalResponseEnricher); + builder.ConfigureOpenTelemetry(additionalTraceFilter, additionalResponseEnricher, additionalActivityProcessor); builder.AddDefaultHealthChecks(); @@ -78,10 +84,16 @@ public static IHostApplicationBuilder AddServiceDefaults( /// Optional callback invoked by EnrichWithHttpResponse after the response is written. /// Use this to enrich or suppress activities based on response properties. /// + /// + /// Optional added to the tracing pipeline. + /// Use this to suppress or enrich activities that bypass the HTTP filter (e.g. SignalR + /// hub method invocations that have no ). + /// public static IHostApplicationBuilder ConfigureOpenTelemetry( this IHostApplicationBuilder builder, Func? additionalTraceFilter = null, - Action? additionalResponseEnricher = null) + Action? additionalResponseEnricher = null, + BaseProcessor? additionalActivityProcessor = null) { ArgumentNullException.ThrowIfNull(builder); @@ -153,6 +165,11 @@ public static IHostApplicationBuilder ConfigureOpenTelemetry( }; }) .AddHttpClientInstrumentation(); + + if (additionalActivityProcessor != null) + { + tracing.AddProcessor(additionalActivityProcessor); + } }); builder.AddOpenTelemetryExporters(); diff --git a/src/TechHub.Web/Components/Layout/MainLayout.razor b/src/TechHub.Web/Components/Layout/MainLayout.razor index 868883ed6..bf1320b56 100644 --- a/src/TechHub.Web/Components/Layout/MainLayout.razor +++ b/src/TechHub.Web/Components/Layout/MainLayout.razor @@ -34,7 +34,7 @@ else diff --git a/src/TechHub.Web/Components/Pages/About.razor b/src/TechHub.Web/Components/Pages/About.razor index 46ae82176..257546776 100644 --- a/src/TechHub.Web/Components/Pages/About.razor +++ b/src/TechHub.Web/Components/Pages/About.razor @@ -77,28 +77,6 @@ - -
- Fokko Veegens -

Fokko Veegens

-

DevOps Consultant

-

- Fokko is a DevOps Consultant specializing in Azure, GitHub, and modern software - development practices. He helps organizations optimize their development workflows - and adopt cloud-native technologies effectively. -

-
- - 📘 GitHub - - - 💼 LinkedIn - - -
-

Why did we build this

diff --git a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs new file mode 100644 index 000000000..284da331d --- /dev/null +++ b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs @@ -0,0 +1,91 @@ +namespace TechHub.Web.Middleware; + +/// +/// Middleware that handles HEAD requests without engaging the Blazor SSR pipeline. +/// +/// +/// Two categories of HEAD request are handled differently: +/// +/// +/// Extension-less paths (Blazor page routes) — Returned immediately with +/// 200 OK and Content-Type: text/html, bypassing Blazor SSR entirely. +/// The SSR pipeline makes an API call for every page render; when the API is slow, +/// bots and crawlers (Slack link previews, SEO scanners) disconnect after their +/// 15–25 s timeout and record 499 (client closed request) in telemetry. Since HEAD +/// only requires response headers — not a body — returning 200 immediately is +/// semantically correct and eliminates the 499s with ~23 s duration. +/// File-extension paths (static assets, RSS feeds) — Rewrites the method to +/// GET, sets Response.Body = Stream.Null to suppress body output, and calls +/// the next middleware. MapStaticAssets serves the correct headers (ETag, +/// Content-Length, Cache-Control) without sending bytes; MapGet endpoints +/// (RSS feeds, /version, etc.) also work correctly this way. +/// +/// +/// +/// This middleware must be placed immediately before UseRouting() so that the method +/// rewrite (HEAD → GET) is visible to endpoint selection. UseInvalidRouteSegmentFilter +/// must run before this middleware so that invalid paths are already rejected with 404. +/// +/// +public class HeadRequestMiddleware +{ + private readonly RequestDelegate _next; + + public HeadRequestMiddleware(RequestDelegate next) + { + ArgumentNullException.ThrowIfNull(next); + _next = next; + } + + public async Task InvokeAsync(HttpContext context) + { + ArgumentNullException.ThrowIfNull(context); + + if (!HttpMethods.IsHead(context.Request.Method)) + { + await _next(context); + return; + } + + // Determine whether this is an extension-less path (Blazor page route) or a + // file-extension path (static asset, RSS feed, etc.) by inspecting the last segment. + var path = context.Request.Path.Value ?? "/"; + var lastSlash = path.LastIndexOf('/'); + var lastSegment = path.AsSpan()[(lastSlash + 1)..]; + var isExtensionless = !lastSegment.Contains('.'); + + if (isExtensionless) + { + // Extension-less path → Blazor page route. Short-circuit with 200 immediately. + // Calling next would trigger Blazor SSR → API call → potential slow response → + // bot/crawler timeout → 499. HEAD only needs headers, not a body. + context.Response.StatusCode = StatusCodes.Status200OK; + context.Response.ContentType = "text/html; charset=utf-8"; + return; + } + + // File-extension path: rewrite HEAD → GET so endpoint selection (which registers + // only GET handlers) can match. Stream.Null suppresses body output as HEAD requires. + context.Request.Method = HttpMethods.Get; + var originalBody = context.Response.Body; + context.Response.Body = Stream.Null; + try + { + await _next(context); + } + finally + { + context.Response.Body = originalBody; + context.Request.Method = HttpMethods.Head; + } + } +} + +/// +/// Extension method for registering . +/// +public static class HeadRequestMiddlewareExtensions +{ + public static IApplicationBuilder UseHeadRequestHandling(this IApplicationBuilder builder) + => builder.UseMiddleware(); +} diff --git a/src/TechHub.Web/Program.cs b/src/TechHub.Web/Program.cs index a2bc74bc6..b059678d8 100644 --- a/src/TechHub.Web/Program.cs +++ b/src/TechHub.Web/Program.cs @@ -25,9 +25,15 @@ // WebTelemetryFilters suppresses Blazor disconnect noise and bot-crawler 404s from traces. // SuppressIfClientError clears ActivityTraceFlags.Recorded for 4xx responses so they are // not exported to App Insights and do not inflate the requests/failed metric. +// additionalActivityProcessor: suppresses failed ComponentHub/UpdateRootComponents activities +// (Blazor Server push to disconnected circuits, success=false resultCode=0 in App Insights). +// CA2000 suppressed: TracerProviderBuilder.AddProcessor takes ownership and disposes the processor. +#pragma warning disable CA2000 // Dispose objects before losing scope builder.AddServiceDefaults( additionalTraceFilter: WebTelemetryFilters.ShouldTrace, - additionalResponseEnricher: WebTelemetryFilters.SuppressIfClientError); + additionalResponseEnricher: WebTelemetryFilters.SuppressIfClientError, + additionalActivityProcessor: new BlazorHubNoiseSuppressor()); +#pragma warning restore CA2000 // Dispose objects before losing scope // Log environment during startup for verification using (var loggerFactory = LoggerFactory.Create(b => b.AddConsole())) @@ -461,34 +467,15 @@ // reach the auth stack or endpoint selection. app.UseInvalidRouteSegmentFilter(); -// ── HEAD → GET rewrite (must be immediately before UseRouting) ──────────────── -// MapRazorComponents and MapGet only register GET endpoints. In WebApplication, -// UseRouting() is auto-inserted at the start of the pipeline (before user middleware), -// so it sees the original HEAD method and returns 405. To fix this, we place an -// explicit app.UseRouting() AFTER the rewrite so routing happens after the method -// is already GET. The body is suppressed via Stream.Null so HEAD responses are empty. -app.Use(async (context, next) => -{ - if (HttpMethods.IsHead(context.Request.Method)) - { - context.Request.Method = HttpMethods.Get; - var originalBody = context.Response.Body; - context.Response.Body = Stream.Null; - try - { - await next(); - } - finally - { - context.Response.Body = originalBody; - context.Request.Method = HttpMethods.Head; - } - } - else - { - await next(); - } -}); +// ── HEAD request handling (must be immediately before UseRouting) ─────────────── +// Extension-less HEAD requests (Blazor page routes) are short-circuited with 200 OK +// before Blazor SSR runs. SSR triggers an API call; when the API is slow, bots and +// crawlers disconnect after their timeout and generate 499s with ~23 s duration. +// File-extension HEAD requests (static assets, RSS) are rewritten to GET with +// Stream.Null to suppress the body — MapStaticAssets then serves correct headers. +// UseRouting() is placed explicitly after this middleware so endpoint selection sees +// GET rather than HEAD (MapRazorComponents only registers GET endpoints). +app.UseHeadRequestHandling(); // Explicit UseRouting() placement: must come AFTER the HEAD→GET rewrite so that // endpoint selection (including HTTP-method matching) sees GET rather than HEAD. diff --git a/src/TechHub.Web/Telemetry/BlazorHubNoiseSuppressor.cs b/src/TechHub.Web/Telemetry/BlazorHubNoiseSuppressor.cs new file mode 100644 index 000000000..c48935f11 --- /dev/null +++ b/src/TechHub.Web/Telemetry/BlazorHubNoiseSuppressor.cs @@ -0,0 +1,48 @@ +using System.Diagnostics; +using OpenTelemetry; + +namespace TechHub.Web.Telemetry; + +/// +/// OpenTelemetry activity processor that suppresses failed Blazor Server hub method +/// activities from the trace export pipeline. +/// +/// +/// When the client has already disconnected (navigated away, closed the tab), the server still +/// attempts to push a component update, which fails because the circuit is gone. This produces +/// a ComponentHub/UpdateRootComponents activity with +/// — no HTTP status code, so Application Insights +/// records it as success = false, resultCode = 0. These are structurally expected and +/// have no diagnostic value; they are a normal side-effect of Blazor Server's server-driven +/// rendering model. +/// +/// +/// +/// Successful hub invocations (circuit still connected, update delivered) are left untouched +/// so they remain visible in Application Insights for performance analysis. +/// +/// +/// +/// This processor clears in OnEnd only for +/// error-status activities, so the Azure Monitor BatchExportProcessor does not queue +/// them for export. Consistent with the approach in +/// for 404/405 HTTP responses. +/// +/// +internal sealed class BlazorHubNoiseSuppressor : BaseProcessor +{ + // All Blazor Server circuit hub invocations share this prefix in their display name. + // Examples: + // Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents + // Microsoft.AspNetCore.Components.Server.ComponentHub/BeginInvokeDotNetFromJS + private const string ComponentHubPrefix = "Microsoft.AspNetCore.Components.Server.ComponentHub"; + + public override void OnEnd(Activity activity) + { + if (activity.DisplayName.StartsWith(ComponentHubPrefix, StringComparison.Ordinal) + && activity.Status == ActivityStatusCode.Error) + { + activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; + } + } +} diff --git a/tests/TechHub.E2E.Tests/Web/AboutPageTests.cs b/tests/TechHub.E2E.Tests/Web/AboutPageTests.cs index 9662b4081..a8d915c3f 100644 --- a/tests/TechHub.E2E.Tests/Web/AboutPageTests.cs +++ b/tests/TechHub.E2E.Tests/Web/AboutPageTests.cs @@ -28,7 +28,6 @@ public async Task AboutPage_ShouldDisplay_TeamMembers() // Assert - Check for team member headings await Page.AssertElementVisibleByRoleAsync(AriaRole.Heading, "Reinier van Maanen"); await Page.AssertElementVisibleByRoleAsync(AriaRole.Heading, "Rob Bos"); - await Page.AssertElementVisibleByRoleAsync(AriaRole.Heading, "Fokko Veegens"); } [Fact] @@ -40,7 +39,6 @@ public async Task AboutPage_ShouldDisplay_TeamMemberPhotos() // Assert - Check for team member photos (exact alt text matching) await Page.AssertElementVisibleByAltTextAsync("Reinier van Maanen"); await Page.AssertElementVisibleByAltTextAsync("Rob Bos"); - await Page.AssertElementVisibleByAltTextAsync("Fokko Veegens"); } [Fact] @@ -62,14 +60,6 @@ public async Task AboutPage_ShouldDisplay_SocialLinks() await robSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "📘 GitHub"); await robSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "💼 LinkedIn"); await robSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "✍️ Blog"); - - // Fokko Veegens - should have GitHub and LinkedIn - var fokkoSection = Page.Locator("text=Fokko Veegens").Locator(".."); - await fokkoSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "📘 GitHub"); - await fokkoSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "💼 LinkedIn"); - - // Note: Fokko's blog link might not be visible or might be a placeholder - // This test verifies the presence of GitHub and LinkedIn only } [Fact] diff --git a/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs b/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs index e04b6d99c..2ace8575e 100644 --- a/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs +++ b/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs @@ -1,5 +1,6 @@ using Dapper; using FluentAssertions; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; @@ -346,6 +347,9 @@ private NewsletterService CreateService( } }); + var env = new Mock(); + env.Setup(e => e.EnvironmentName).Returns(Environments.Production); + return new NewsletterService( _fixture.Connection, new NewsletterSubscriberRepository(_fixture.Connection), @@ -353,7 +357,8 @@ private NewsletterService CreateService( options, appSettings, emailSender, - NullLogger.Instance); + NullLogger.Instance, + env.Object); } private static Section CreateSection(string name) => diff --git a/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs new file mode 100644 index 000000000..c856bd5db --- /dev/null +++ b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs @@ -0,0 +1,168 @@ +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using TechHub.Web.Middleware; + +namespace TechHub.Web.Tests.Middleware; + +public class HeadRequestMiddlewareTests +{ + // ─── Non-HEAD methods pass through unchanged ────────────────────────────── + + [Theory] + [InlineData("GET")] + [InlineData("POST")] + [InlineData("PUT")] + public async Task InvokeAsync_NonHeadMethod_CallsNextUnchanged(string method) + { + var (middleware, context, nextCalled) = CreateMiddleware(method, "/some/path"); + + await middleware.InvokeAsync(context); + + nextCalled().Should().BeTrue(); + context.Request.Method.Should().Be(method); + } + + // ─── Extension-less HEAD → 200 OK, no SSR ──────────────────────────────── + + [Theory] + [InlineData("/")] + [InlineData("/ai")] + [InlineData("/ai/videos/my-article-slug")] + [InlineData("/all/authors/john-doe")] + [InlineData("/about")] + [InlineData("/admin/login")] + [InlineData("/news/2024/some-post")] + public async Task InvokeAsync_HeadExtensionlessPath_Returns200WithoutCallingNext(string path) + { + var (middleware, context, nextCalled) = CreateMiddleware("HEAD", path); + + await middleware.InvokeAsync(context); + + nextCalled().Should().BeFalse("SSR must be bypassed for extension-less HEAD requests"); + context.Response.StatusCode.Should().Be(StatusCodes.Status200OK); + context.Response.ContentType.Should().Be("text/html; charset=utf-8"); + } + + [Fact] + public async Task InvokeAsync_HeadExtensionlessPath_MethodRemainsHead() + { + var (middleware, context, _) = CreateMiddleware("HEAD", "/ai/videos/article"); + + await middleware.InvokeAsync(context); + + // Method must not be mutated — HEAD remains HEAD so callers see the correct method. + context.Request.Method.Should().Be("HEAD"); + } + + // ─── File-extension HEAD → GET rewrite, body suppressed ────────────────── + + [Theory] + [InlineData("/favicon.ico")] + [InlineData("/robots.txt")] + [InlineData("/js/mobile-nav.abc123.js")] + [InlineData("/all/feed.xml")] + [InlineData("/sitemap.xml")] + [InlineData("/css/app.min.css")] + public async Task InvokeAsync_HeadFileExtensionPath_CallsNextWithGetMethod(string path) + { + string? methodSeenByNext = null; + RequestDelegate next = ctx => + { + methodSeenByNext = ctx.Request.Method; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = path; + + await middleware.InvokeAsync(context); + + methodSeenByNext.Should().Be(HttpMethods.Get, "next must see GET so endpoint selection matches MapStaticAssets/MapGet"); + } + + [Theory] + [InlineData("/favicon.ico")] + [InlineData("/robots.txt")] + [InlineData("/js/app.abc.js")] + [InlineData("/all/feed.xml")] + public async Task InvokeAsync_HeadFileExtensionPath_MethodRestoredToHeadAfterNext(string path) + { + var (middleware, context, nextCalled) = CreateMiddleware("HEAD", path); + + await middleware.InvokeAsync(context); + + nextCalled().Should().BeTrue(); + context.Request.Method.Should().Be("HEAD", "method must be restored to HEAD after next() returns"); + } + + [Fact] + public async Task InvokeAsync_HeadFileExtensionPath_BodyIsStreamNullDuringNextAndRestoredAfter() + { + using var originalBody = new MemoryStream(); + Stream? bodySeenByNext = null; + + RequestDelegate next = ctx => + { + bodySeenByNext = ctx.Response.Body; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = "/favicon.ico"; + context.Response.Body = originalBody; + + await middleware.InvokeAsync(context); + + bodySeenByNext.Should().BeSameAs(Stream.Null, "body must be Stream.Null while next executes so no bytes are written"); + context.Response.Body.Should().BeSameAs(originalBody, "original body must be restored after next() returns"); + } + + [Fact] + public async Task InvokeAsync_HeadFileExtensionPath_BodyRestoredEvenWhenNextThrows() + { + using var originalBody = new MemoryStream(); + + RequestDelegate next = _ => throw new InvalidOperationException("simulated error"); + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = "/favicon.ico"; + context.Response.Body = originalBody; + + await middleware.Invoking(m => m.InvokeAsync(context)).Should().ThrowAsync(); + + context.Response.Body.Should().BeSameAs(originalBody, "body must be restored even when next() throws"); + context.Request.Method.Should().Be("HEAD", "method must be restored even when next() throws"); + } + + // ─── Constructor guard ──────────────────────────────────────────────────── + + [Fact] + public void Constructor_NullNext_ThrowsArgumentNullException() + { + var act = () => new HeadRequestMiddleware(null!); + + act.Should().Throw().WithParameterName("next"); + } + + // ─── Helper ─────────────────────────────────────────────────────────────── + + private static (HeadRequestMiddleware middleware, HttpContext context, Func nextCalled) CreateMiddleware( + string method = "GET", + string path = "/") + { + var wasCalled = false; + RequestDelegate next = _ => + { + wasCalled = true; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = method; + context.Request.Path = path; + return (middleware, context, () => wasCalled); + } +} diff --git a/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs b/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs new file mode 100644 index 000000000..8c0d5fb34 --- /dev/null +++ b/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs @@ -0,0 +1,111 @@ +using System.Diagnostics; +using FluentAssertions; +using TechHub.Web.Telemetry; + +namespace TechHub.Web.Tests.Telemetry; + +public class BlazorHubNoiseSuppressorTests +{ + // ─── Failed ComponentHub activities are suppressed ─────────────────────── + + [Fact] + public void OnEnd_UpdateRootComponentsActivity_ErrorStatus_ClearsRecordedFlag() + { + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = CreateActivity( + "Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents", + ActivityStatusCode.Error); + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) + .Should().BeFalse("failed ComponentHub activities must not be exported to App Insights"); + } + + [Theory] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents")] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub/BeginInvokeDotNetFromJS")] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub/EndInvokeJSFromDotNet")] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub/DispatchBrowserEvent")] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub")] + public void OnEnd_AnyComponentHubActivity_ErrorStatus_ClearsRecordedFlag(string displayName) + { + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = CreateActivity(displayName, ActivityStatusCode.Error); + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) + .Should().BeFalse($"failed activity '{displayName}' must have Recorded flag cleared"); + } + + // ─── Successful ComponentHub activities pass through untouched ──────────── + + [Theory] + [InlineData(ActivityStatusCode.Ok)] + [InlineData(ActivityStatusCode.Unset)] + public void OnEnd_ComponentHubActivity_NonErrorStatus_LeavesRecordedFlagUnchanged(ActivityStatusCode statusCode) + { + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = CreateActivity( + "Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents", + statusCode); + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) + .Should().BeTrue("successful hub invocations must remain visible in App Insights"); + } + + // ─── Non-ComponentHub activities are left untouched ────────────────────── + + [Theory] + [InlineData("GET /ai/videos/my-article")] + [InlineData("POST /admin/login")] + [InlineData("Microsoft.AspNetCore.Hosting.HttpRequestIn")] + [InlineData("Microsoft.AspNetCore.SignalR.HubInvocation")] + [InlineData("SomeOtherHub/UpdateRootComponents")] + public void OnEnd_NonComponentHubActivity_LeavesRecordedFlagUnchanged(string displayName) + { + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = CreateActivity(displayName, ActivityStatusCode.Error); // even with Error status + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) + .Should().BeTrue($"activity '{displayName}' must not have its Recorded flag cleared"); + } + + [Fact] + public void OnEnd_ActivityAlreadyNotRecorded_RemainsNotRecorded() + { + // Verifies the bitwise operation does not inadvertently flip flags on unrecorded + // ComponentHub activities that were already suppressed upstream. + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = new ActivitySource("test").CreateActivity( + "Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents", + ActivityKind.Internal); + + // activity may be null when no listener is registered — skip the test in that case + if (activity is null) + return; + + activity.SetStatus(ActivityStatusCode.Error); + activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; // already not recorded + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded).Should().BeFalse(); + } + + // ─── Helper ─────────────────────────────────────────────────────────────── + + private static Activity CreateActivity(string displayName, ActivityStatusCode statusCode = ActivityStatusCode.Unset) + { + var activity = new Activity(displayName); + activity.ActivityTraceFlags |= ActivityTraceFlags.Recorded; + activity.Start(); + activity.SetStatus(statusCode); + return activity; + } +} From 0b9101a9cf2b2e71b57d85dd79e7e426f24b63ef Mon Sep 17 00:00:00 2001 From: Reinier Date: Mon, 1 Jun 2026 20:13:26 +0000 Subject: [PATCH 02/12] Standardize exception handling and refactor newsletter templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Extract Handlebars template compilation into a singleton, sanitize email logging, fix infra deploy script, and standardize all broad-catch patterns across background services and processing pipelines. Implementation: • Add NewsletterTemplateProvider singleton — compiles 6 Handlebars email templates once at startup (dev: disk, prod: embedded resources) instead of per-DI-scope in NewsletterService • Refactor NewsletterService to accept NewsletterTemplateProvider; remove per-scope template compilation and IHostEnvironment dependency • Sanitize recipientEmail in AcsEmailSender log statements (.Sanitize()) • Register NewsletterTemplateProvider as singleton in Program.cs • Fix Deploy-Infrastructure.ps1 linkEmailDomain regex to match true|false • Standardize all catch filters to 'when (ex is not OperationCanceledException)' across ContentFixerBackgroundService, ContentProcessingBackgroundService, RoundupGeneratorBackgroundService, ContentProcessingService, ArticleContentService, NewsletterService, DapperExtensions — removing redundant OutOfMemoryException/StackOverflowException exclusions (CLR terminates process for those regardless of catch blocks) • Document the accepted broad-catch pattern in src/AGENTS.md, src/TechHub.Api/AGENTS.md, and src/TechHub.Infrastructure/AGENTS.md • Update NewsletterServiceTests to pass NewsletterTemplateProvider • Fix BlazorHubNoiseSuppressorTests to use new Activity(...) directly --- scripts/Deploy-Infrastructure.ps1 | 2 +- src/AGENTS.md | 26 +++++++ src/TechHub.Api/AGENTS.md | 13 +++- src/TechHub.Api/Program.cs | 1 + .../Services/ContentFixerBackgroundService.cs | 4 +- .../ContentProcessingBackgroundService.cs | 2 +- .../RoundupGeneratorBackgroundService.cs | 2 +- src/TechHub.Infrastructure/AGENTS.md | 16 +++++ .../Data/DapperExtensions.cs | 2 +- .../ArticleContentService.cs | 2 +- .../ContentProcessingService.cs | 2 +- .../Services/Newsletter/AcsEmailSender.cs | 5 +- .../Services/Newsletter/NewsletterService.cs | 69 ++++--------------- .../Newsletter/NewsletterTemplateProvider.cs | 54 +++++++++++++++ .../Services/NewsletterServiceTests.cs | 2 +- .../BlazorHubNoiseSuppressorTests.cs | 9 +-- 16 files changed, 138 insertions(+), 73 deletions(-) create mode 100644 src/TechHub.Infrastructure/Services/Newsletter/NewsletterTemplateProvider.cs diff --git a/scripts/Deploy-Infrastructure.ps1 b/scripts/Deploy-Infrastructure.ps1 index 5490664a3..c380c7d4a 100644 --- a/scripts/Deploy-Infrastructure.ps1 +++ b/scripts/Deploy-Infrastructure.ps1 @@ -232,7 +232,7 @@ if ($Mode -eq 'deploy') { # Write temp file to the same directory so the 'using' relative path remains valid $tempParamsFile = Join-Path (Split-Path $infraParamsFile) "prod-infrastructure-linkdomain.bicepparam" (Get-Content $infraParamsFile -Raw) ` - -replace 'param linkEmailDomain = false', 'param linkEmailDomain = true' | + -replace 'param linkEmailDomain = (?:false|true)', 'param linkEmailDomain = true' | Set-Content $tempParamsFile $effectiveParamsFile = $tempParamsFile } diff --git a/src/AGENTS.md b/src/AGENTS.md index 014b0c1b1..b5b01d5ff 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -45,6 +45,32 @@ - **End users get friendly error messages** — But logs and Application Insights must contain full exception details, parameters, and context - **Log then rethrow or let it crash** — Prefer logging with context and rethrowing over catching and returning a default value +### Accepted Broad-Catch Pattern + +Catching all non-cancellation exceptions (`when (ex is not OperationCanceledException)`) is **accepted and preferred** in two scenarios: + +1. **Background service loops** — To prevent the entire service (and thus the application) from crashing on one unexpected error. The service logs, then continues processing on the next tick. + + ```csharp + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogError(ex, "Unhandled exception in … — service will continue"); + } + ``` + +2. **Per-item processing loops** — To skip a failing item and continue with the next (e.g., content processing, newsletter subscriber sending). Must be suppressed with `#pragma warning disable CA1031` and a comment explaining the intent. + + ```csharp + #pragma warning disable CA1031 // Best-effort: continue with other items if one fails + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "Failed to process {Item} — skipping", item); + } + #pragma warning restore CA1031 + ``` + +Do **not** add `OutOfMemoryException` or `StackOverflowException` exclusions — the CLR already terminates the process for those regardless of catch blocks. + ## Project Structure ```text diff --git a/src/TechHub.Api/AGENTS.md b/src/TechHub.Api/AGENTS.md index df7e5fdfe..faad254bf 100644 --- a/src/TechHub.Api/AGENTS.md +++ b/src/TechHub.Api/AGENTS.md @@ -6,7 +6,18 @@ REST API backend using ASP.NET Core Minimal APIs. Exposes endpoints for sections, content, filtering, RSS feeds, and structured data. -**Testing**: See [tests/TechHub.Api.Tests/AGENTS.md](../../tests/TechHub.Api.Tests/AGENTS.md) for integration testing patterns. +## Background Services + +Background services inherit from `BackgroundService` and are registered in `Program.cs` via `AddHostedService()`. The outer execution loop MUST catch all non-cancellation exceptions to prevent the entire application from crashing: + +```csharp +catch (Exception ex) when (ex is not OperationCanceledException) +{ + _logger.LogError(ex, "Unhandled exception in MyBackgroundService — service will continue"); +} +``` + +See [src/AGENTS.md](../AGENTS.md#accepted-broad-catch-pattern) for the accepted broad-catch pattern and rationale. ## Project Structure diff --git a/src/TechHub.Api/Program.cs b/src/TechHub.Api/Program.cs index 81d5a2ec0..3426669fb 100644 --- a/src/TechHub.Api/Program.cs +++ b/src/TechHub.Api/Program.cs @@ -261,6 +261,7 @@ builder.Configuration.GetSection(NewsletterOptions.SectionName)); builder.Services.AddScoped(); builder.Services.AddScoped(); +builder.Services.AddSingleton(); builder.Services.AddScoped(); builder.Services.AddAcsEmailClient(); builder.Services.AddScoped(); diff --git a/src/TechHub.Api/Services/ContentFixerBackgroundService.cs b/src/TechHub.Api/Services/ContentFixerBackgroundService.cs index b026acc8f..091bb006a 100644 --- a/src/TechHub.Api/Services/ContentFixerBackgroundService.cs +++ b/src/TechHub.Api/Services/ContentFixerBackgroundService.cs @@ -124,7 +124,7 @@ await jobRepo.AbortJobAsync(jobId, transcriptsSucceeded: 0, transcriptsFailed: 0, logOutput: "Content cleanup aborted by admin.", ct: CancellationToken.None); } - catch (Exception ex) when (ex is not OperationCanceledException and not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogError(ex, "Content fixer run failed (job {JobId})", jobId); await jobRepo.FailAsync(jobId, @@ -137,7 +137,7 @@ await jobRepo.FailAsync(jobId, { // Shutting down — expected } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogError(ex, "Unexpected exception in ContentFixerBackgroundService"); } diff --git a/src/TechHub.Api/Services/ContentProcessingBackgroundService.cs b/src/TechHub.Api/Services/ContentProcessingBackgroundService.cs index a89d6c308..235ae2253 100644 --- a/src/TechHub.Api/Services/ContentProcessingBackgroundService.cs +++ b/src/TechHub.Api/Services/ContentProcessingBackgroundService.cs @@ -172,7 +172,7 @@ private async Task RunOnceAsync(string triggerType, CancellationToken ct) { // Shutting down — expected } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { // Errors are already recorded by ContentProcessingService; log defensively here _logger.LogError(ex, "Unexpected exception in ContentProcessingBackgroundService"); diff --git a/src/TechHub.Api/Services/RoundupGeneratorBackgroundService.cs b/src/TechHub.Api/Services/RoundupGeneratorBackgroundService.cs index b6d857b54..5734ae44d 100644 --- a/src/TechHub.Api/Services/RoundupGeneratorBackgroundService.cs +++ b/src/TechHub.Api/Services/RoundupGeneratorBackgroundService.cs @@ -293,7 +293,7 @@ await jobRepo.AbortJobAsync(jobId, feedsProcessed: 0, itemsAdded: 0, itemsSkippe { // Shutting down — expected } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogError(ex, "Unexpected exception in RoundupGeneratorBackgroundService (job {JobId})", jobId); await jobRepo.AppendLogAsync(jobId, $"Roundup generation failed: {ex.Message}\n{ex.StackTrace}", CancellationToken.None); diff --git a/src/TechHub.Infrastructure/AGENTS.md b/src/TechHub.Infrastructure/AGENTS.md index 42438a8f4..cfdf6e01a 100644 --- a/src/TechHub.Infrastructure/AGENTS.md +++ b/src/TechHub.Infrastructure/AGENTS.md @@ -55,6 +55,22 @@ See [Data/Migrations/postgres/](Data/Migrations/postgres/) for complete schema. | Scoped | `IDbConnection` (per-request) | | Transient | All other services | +## Per-Item Processing Loops + +Processing pipelines (content ingestion, newsletter sending, roundup generation) iterate over collections and MUST NOT abort the entire run when one item fails. Use the broad-catch pattern with `#pragma warning disable CA1031`: + +```csharp +#pragma warning disable CA1031 // Best-effort: continue with other items if one fails +catch (Exception ex) when (ex is not OperationCanceledException) +{ + errorCount++; + _logger.LogWarning(ex, "Failed to process {Item} — skipping", item); +} +#pragma warning restore CA1031 +``` + +See [src/AGENTS.md](../AGENTS.md#accepted-broad-catch-pattern) for the full rationale. + ## Tag Cloud `TagCloudService` queries tags with section/collection title exclusion (repository filters these BEFORE counting). Quantile sizing: top 25% Large, middle 50% Medium, bottom 25% Small. Size group normalization: 1 group → all Medium, 2 → Medium + Small. diff --git a/src/TechHub.Infrastructure/Data/DapperExtensions.cs b/src/TechHub.Infrastructure/Data/DapperExtensions.cs index e20849e7e..bf8a7b055 100644 --- a/src/TechHub.Infrastructure/Data/DapperExtensions.cs +++ b/src/TechHub.Infrastructure/Data/DapperExtensions.cs @@ -151,7 +151,7 @@ private static void LogExplainPlan(IDbConnection connection, string sql, object? logger.LogWarning("{ExplainPlan}", sb.ToString().TrimEnd()); } - catch (Exception ex) when (ex is not OutOfMemoryException) + catch (Exception ex) when (ex is not OperationCanceledException) { logger.LogWarning(ex, "Failed to retrieve EXPLAIN plan for slow query"); } diff --git a/src/TechHub.Infrastructure/Services/ContentProcessing/ArticleContentService.cs b/src/TechHub.Infrastructure/Services/ContentProcessing/ArticleContentService.cs index bbe899df3..b9357a4b2 100644 --- a/src/TechHub.Infrastructure/Services/ContentProcessing/ArticleContentService.cs +++ b/src/TechHub.Infrastructure/Services/ContentProcessing/ArticleContentService.cs @@ -127,7 +127,7 @@ private static string ExtractMainContent(string html, string? url = null) return article.IsReadable ? article.Content : null; } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { return null; } diff --git a/src/TechHub.Infrastructure/Services/ContentProcessing/ContentProcessingService.cs b/src/TechHub.Infrastructure/Services/ContentProcessing/ContentProcessingService.cs index 8c24251ee..01273c707 100644 --- a/src/TechHub.Infrastructure/Services/ContentProcessing/ContentProcessingService.cs +++ b/src/TechHub.Infrastructure/Services/ContentProcessing/ContentProcessingService.cs @@ -444,7 +444,7 @@ await _jobRepo.CompleteAsync(jobId, feedsProcessed: 0, itemsAdded, itemsSkipped, await TryAbortJobAsync(jobId, 0, 0, 0, 0, 0, 0, log.ToString()); throw; } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { Log(string.Create(CultureInfo.InvariantCulture, $"FATAL ERROR: {ex.Message.Sanitize()}")); _logger.LogError(ex, "Ad-hoc processing job {JobId} failed with unhandled exception", jobId); diff --git a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs index 8059e18f7..f45f2a459 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Options; using TechHub.Core.Configuration; using TechHub.Core.Interfaces; +using TechHub.Core.Logging; namespace TechHub.Infrastructure.Services.Newsletter; @@ -52,12 +53,12 @@ public async Task SendAsync(string recipientEmail, string subject, string } catch (RequestFailedException ex) when (ex.Status == 429) { - _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429). Email to {RecipientEmail} was not sent", recipientEmail); + _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429). Email to {RecipientEmail} was not sent", recipientEmail.Sanitize()); return false; } catch (RequestFailedException ex) { - _logger.LogError(ex, "Newsletter email send failed with ACS error {Status} for {RecipientEmail}", ex.Status, recipientEmail); + _logger.LogError(ex, "Newsletter email send failed with ACS error {Status} for {RecipientEmail}", ex.Status, recipientEmail.Sanitize()); return false; } diff --git a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs index 0ea212a71..7fbb77470 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs @@ -1,12 +1,9 @@ using System.Data; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.Reflection; using System.Security.Cryptography; using System.Text; using Dapper; -using HandlebarsDotNet; -using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using TechHub.Core.Configuration; @@ -28,12 +25,7 @@ public sealed class NewsletterService : INewsletterService private readonly AppSettings _appSettings; private readonly IEmailSender _emailSender; private readonly ILogger _logger; - private readonly HandlebarsTemplate _shellTemplate; - private readonly HandlebarsTemplate _subscriberFooterTemplate; - private readonly HandlebarsTemplate _accountActionTemplate; - private readonly HandlebarsTemplate _weeklyContentTemplate; - private readonly HandlebarsTemplate _dailyContentTemplate; - private readonly HandlebarsTemplate _adminContentTemplate; + private readonly NewsletterTemplateProvider _templates; public NewsletterService( IDbConnection connection, @@ -43,7 +35,7 @@ public NewsletterService( IOptions appSettings, IEmailSender emailSender, ILogger logger, - IHostEnvironment env) + NewsletterTemplateProvider templates) { ArgumentNullException.ThrowIfNull(connection); ArgumentNullException.ThrowIfNull(subscriberRepository); @@ -52,7 +44,7 @@ public NewsletterService( ArgumentNullException.ThrowIfNull(appSettings); ArgumentNullException.ThrowIfNull(emailSender); ArgumentNullException.ThrowIfNull(logger); - ArgumentNullException.ThrowIfNull(env); + ArgumentNullException.ThrowIfNull(templates); _connection = connection; _subscriberRepository = subscriberRepository; @@ -61,25 +53,7 @@ public NewsletterService( _appSettings = appSettings.Value; _emailSender = emailSender; _logger = logger; - if (env.IsDevelopment()) - { - var resourcesPath = Path.GetFullPath(Path.Combine(env.ContentRootPath, "..", "TechHub.Infrastructure", "Data", "Resources")); - _shellTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-template.html"))); - _subscriberFooterTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-subscriber-footer.html"))); - _accountActionTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-account-action-content.html"))); - _weeklyContentTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-weekly-content.html"))); - _dailyContentTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-daily-content.html"))); - _adminContentTemplate = Handlebars.Compile(File.ReadAllText(Path.Combine(resourcesPath, "newsletter-admin-content.html"))); - } - else - { - _shellTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-template.html")); - _subscriberFooterTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-subscriber-footer.html")); - _accountActionTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-account-action-content.html")); - _weeklyContentTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-weekly-content.html")); - _dailyContentTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-daily-content.html")); - _adminContentTemplate = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-admin-content.html")); - } + _templates = templates; } public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlugs, string? sendTargetKey = null, CancellationToken ct = default) @@ -169,7 +143,7 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu } } } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { var status = successful > 0 ? "partial" : "failed"; await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, status, ex.Message, ct); @@ -709,8 +683,8 @@ private IEnumerable GetRoundupLinks(RoundupRow roundup) private string RenderCombinedWeeklyHtml(IReadOnlyList roundups, string unsubscribeUrl, string manageUrl) { - var footer = _subscriberFooterTemplate(new { manageUrl, unsubscribeUrl }); - var content = _weeklyContentTemplate(new + var footer = _templates.SubscriberFooter(new { manageUrl, unsubscribeUrl }); + var content = _templates.WeeklyContent(new { roundups = roundups.Select(r => new { @@ -722,7 +696,7 @@ private string RenderCombinedWeeklyHtml(IReadOnlyList roundups, stri }), footer }); - return _shellTemplate(new + return _templates.Shell(new { title = "TechHub Weekly Digest", cardClass = "th-card--weekly", @@ -806,8 +780,8 @@ private string BuildDailyOverviewHtml( { var unsubscribeUrl = BuildUnsubscribeUrl(email); var manageUrl = BuildManageUrl(email, _options.UnsubscribeSecret); - var footer = _subscriberFooterTemplate(new { manageUrl, unsubscribeUrl }); - var content = _dailyContentTemplate(new + var footer = _templates.SubscriberFooter(new { manageUrl, unsubscribeUrl }); + var content = _templates.DailyContent(new { date = day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture), sections = sections @@ -824,7 +798,7 @@ private string BuildDailyOverviewHtml( }), footer }); - return _shellTemplate(new + return _templates.Shell(new { title = "TechHub Daily Overview", cardClass = "th-card--daily", @@ -869,7 +843,7 @@ private string BuildDailyOverviewText( private string BuildAdminStatusHtml(DateOnly day, NewsletterDailyReportStats stats) { - var content = _adminContentTemplate(new + var content = _templates.AdminContent(new { newContentItems = stats.NewContentItemsLast24Hours.ToString(CultureInfo.InvariantCulture), failedProcessedUrls = stats.FailedProcessedUrlsLast24Hours.ToString(CultureInfo.InvariantCulture), @@ -878,7 +852,7 @@ private string BuildAdminStatusHtml(DateOnly day, NewsletterDailyReportStats sta activeSubscribers = stats.ActiveSubscribers.ToString(CultureInfo.InvariantCulture), unconfirmedSubscribers = stats.UnconfirmedSubscribers.ToString(CultureInfo.InvariantCulture) }); - return _shellTemplate(new + return _templates.Shell(new { title = $"TechHub Daily Status Report \u2014 {day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture)}", cardClass = "th-card--admin", @@ -889,8 +863,8 @@ private string BuildAdminStatusHtml(DateOnly day, NewsletterDailyReportStats sta private string BuildAccountActionHtml(string title, string message, string actionLabel, string actionUrl) { - var content = _accountActionTemplate(new { message, actionLabel, actionUrl }); - return _shellTemplate(new + var content = _templates.AccountAction(new { message, actionLabel, actionUrl }); + return _templates.Shell(new { title, cardClass = "th-card--account", @@ -952,19 +926,6 @@ private bool IsUnsubscribeSecretConfigured(string operationName) return false; } - private static string LoadEmbeddedHtml(string resourceName, string fallback = "") - { - var assembly = Assembly.GetExecutingAssembly(); - using var stream = assembly.GetManifestResourceStream(resourceName); - if (stream is null) - { - return fallback; - } - - using var reader = new StreamReader(stream); - return reader.ReadToEnd(); - } - private sealed class RoundupRow { public string Slug { get; init; } = string.Empty; diff --git a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterTemplateProvider.cs b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterTemplateProvider.cs new file mode 100644 index 000000000..39a5dcf18 --- /dev/null +++ b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterTemplateProvider.cs @@ -0,0 +1,54 @@ +using System.Reflection; +using HandlebarsDotNet; +using Microsoft.Extensions.Hosting; + +namespace TechHub.Infrastructure.Services.Newsletter; + +/// +/// Singleton service that compiles Handlebars email templates once at startup. +/// Injected into so templates are not recompiled +/// per DI scope. +/// +public sealed class NewsletterTemplateProvider +{ + public HandlebarsTemplate Shell { get; } + public HandlebarsTemplate SubscriberFooter { get; } + public HandlebarsTemplate AccountAction { get; } + public HandlebarsTemplate WeeklyContent { get; } + public HandlebarsTemplate DailyContent { get; } + public HandlebarsTemplate AdminContent { get; } + + public NewsletterTemplateProvider(IHostEnvironment env) + { + ArgumentNullException.ThrowIfNull(env); + + if (env.IsDevelopment()) + { + var resourcesPath = Path.GetFullPath(Path.Join(env.ContentRootPath, "..", "TechHub.Infrastructure", "Data", "Resources")); + Shell = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-template.html"))); + SubscriberFooter = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-subscriber-footer.html"))); + AccountAction = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-account-action-content.html"))); + WeeklyContent = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-weekly-content.html"))); + DailyContent = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-daily-content.html"))); + AdminContent = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-admin-content.html"))); + } + else + { + Shell = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-template.html")); + SubscriberFooter = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-subscriber-footer.html")); + AccountAction = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-account-action-content.html")); + WeeklyContent = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-weekly-content.html")); + DailyContent = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-daily-content.html")); + AdminContent = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-admin-content.html")); + } + } + + private static string LoadEmbeddedHtml(string resourceName) + { + var assembly = Assembly.GetExecutingAssembly(); + using var stream = assembly.GetManifestResourceStream(resourceName) + ?? throw new InvalidOperationException($"Embedded resource '{resourceName}' not found."); + using var reader = new StreamReader(stream); + return reader.ReadToEnd(); + } +} diff --git a/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs b/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs index 2ace8575e..54ad5e362 100644 --- a/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs +++ b/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs @@ -358,7 +358,7 @@ private NewsletterService CreateService( appSettings, emailSender, NullLogger.Instance, - env.Object); + new NewsletterTemplateProvider(env.Object)); } private static Section CreateSection(string name) => diff --git a/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs b/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs index 8c0d5fb34..cc204c899 100644 --- a/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs +++ b/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs @@ -82,13 +82,8 @@ public void OnEnd_ActivityAlreadyNotRecorded_RemainsNotRecorded() // Verifies the bitwise operation does not inadvertently flip flags on unrecorded // ComponentHub activities that were already suppressed upstream. var suppressor = new BlazorHubNoiseSuppressor(); - using var activity = new ActivitySource("test").CreateActivity( - "Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents", - ActivityKind.Internal); - - // activity may be null when no listener is registered — skip the test in that case - if (activity is null) - return; + using var activity = new Activity( + "Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents"); activity.SetStatus(ActivityStatusCode.Error); activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; // already not recorded From 51e7687f9241b7d948402c06c7a2a4ba66f6635e Mon Sep 17 00:00:00 2001 From: Reinier Date: Tue, 2 Jun 2026 06:56:35 +0000 Subject: [PATCH 03/12] Fixed email sanitizing --- src/TechHub.Core/Logging/InputSanitizer.cs | 23 +++++++++++ .../Services/Newsletter/AcsEmailSender.cs | 4 +- .../Logging/InputSanitizerTests.cs | 38 +++++++++++++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/TechHub.Core/Logging/InputSanitizer.cs b/src/TechHub.Core/Logging/InputSanitizer.cs index 126bdca65..8e3a94727 100644 --- a/src/TechHub.Core/Logging/InputSanitizer.cs +++ b/src/TechHub.Core/Logging/InputSanitizer.cs @@ -22,4 +22,27 @@ public static string Sanitize(this string? value) .Replace("\r", string.Empty, StringComparison.Ordinal) .Replace("\n", string.Empty, StringComparison.Ordinal); } + + /// + /// Masks an email address for safe logging, preserving only the first character of the local + /// part and the full domain so logs remain useful without exposing PII. + /// Examples: user@example.com → u***@example.com, + /// a@b.com → a***@b.com. + /// + public static string MaskEmail(this string? email) + { + if (string.IsNullOrEmpty(email)) + { + return string.Empty; + } + + var atIndex = email.IndexOf('@', StringComparison.Ordinal); + if (atIndex <= 0) + { + // Not a recognisable email — sanitize only. + return email.Sanitize(); + } + + return string.Concat(email.AsSpan(0, 1), "***", email.AsSpan(atIndex)); + } } diff --git a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs index f45f2a459..9432a93ee 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs @@ -53,12 +53,12 @@ public async Task SendAsync(string recipientEmail, string subject, string } catch (RequestFailedException ex) when (ex.Status == 429) { - _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429). Email to {RecipientEmail} was not sent", recipientEmail.Sanitize()); + _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429). Email to {RecipientEmail} was not sent", recipientEmail.MaskEmail()); return false; } catch (RequestFailedException ex) { - _logger.LogError(ex, "Newsletter email send failed with ACS error {Status} for {RecipientEmail}", ex.Status, recipientEmail.Sanitize()); + _logger.LogError(ex, "Newsletter email send failed with ACS error {Status} for {RecipientEmail}", ex.Status, recipientEmail.MaskEmail()); return false; } diff --git a/tests/TechHub.Core.Tests/Logging/InputSanitizerTests.cs b/tests/TechHub.Core.Tests/Logging/InputSanitizerTests.cs index 704500e06..3b0b0580f 100644 --- a/tests/TechHub.Core.Tests/Logging/InputSanitizerTests.cs +++ b/tests/TechHub.Core.Tests/Logging/InputSanitizerTests.cs @@ -102,4 +102,42 @@ public void Sanitize_TypicalLogValues_ReturnsUnchanged(string input, string expe // Assert result.Should().Be(expected); } + + // ─── MaskEmail ─────────────────────────────────────────────────────────── + + [Fact] + public void MaskEmail_NullValue_ReturnsEmpty() + { + InputSanitizer.MaskEmail(null).Should().BeEmpty(); + } + + [Fact] + public void MaskEmail_EmptyString_ReturnsEmpty() + { + InputSanitizer.MaskEmail(string.Empty).Should().BeEmpty(); + } + + [Theory] + [InlineData("user@example.com", "u***@example.com")] + [InlineData("reinier@xebia.com", "r***@xebia.com")] + [InlineData("a@b.com", "a***@b.com")] + [InlineData("newsletter@mail.hub.ms", "n***@mail.hub.ms")] + public void MaskEmail_ValidEmail_MasksLocalPart(string input, string expected) + { + InputSanitizer.MaskEmail(input).Should().Be(expected); + } + + [Fact] + public void MaskEmail_NoAtSign_ReturnsSanitizedValue() + { + // Not a recognisable email — falls back to Sanitize behaviour (no crash). + InputSanitizer.MaskEmail("notanemail").Should().Be("notanemail"); + } + + [Fact] + public void MaskEmail_EmailWithNewline_MasksAndSanitizes() + { + // Log-forging attempt embedded in an otherwise valid email. + InputSanitizer.MaskEmail("u\nser@example.com").Should().Be("u***@example.com"); + } } From c310d2131003d47ed1fccd3cd0f763bdc77a15be Mon Sep 17 00:00:00 2001 From: Reinier Date: Tue, 2 Jun 2026 07:13:40 +0000 Subject: [PATCH 04/12] Email masking --- .../Services/Newsletter/AcsEmailSender.cs | 2 ++ .../Services/Newsletter/NewsletterService.cs | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs index 9432a93ee..f9b1ff2ec 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs @@ -53,11 +53,13 @@ public async Task SendAsync(string recipientEmail, string subject, string } catch (RequestFailedException ex) when (ex.Status == 429) { + // codeql[cs/exposure-of-private-information] - email is masked via MaskEmail() before logging _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429). Email to {RecipientEmail} was not sent", recipientEmail.MaskEmail()); return false; } catch (RequestFailedException ex) { + // codeql[cs/exposure-of-private-information] - email is masked via MaskEmail() before logging _logger.LogError(ex, "Newsletter email send failed with ACS error {Status} for {RecipientEmail}", ex.Status, recipientEmail.MaskEmail()); return false; } diff --git a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs index 7fbb77470..5c67258eb 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs @@ -8,6 +8,7 @@ using Microsoft.Extensions.Options; using TechHub.Core.Configuration; using TechHub.Core.Interfaces; +using TechHub.Core.Logging; using TechHub.Core.Models; using TechHub.Core.Models.Admin; using TechHub.Infrastructure.Services.RoundupGeneration; @@ -222,7 +223,8 @@ await _subscriberRepository.LogSendAsync( logKey, sent ? 1 : 0, sent ? "sent" : "failed", - sent ? null : $"Delivery failed to {recipientEmail}", + // codeql[cs/exposure-of-private-information] - email is masked via MaskEmail() before writing to the log table + sent ? null : $"Delivery failed to {recipientEmail.MaskEmail()}", ct); return sent; } @@ -269,7 +271,8 @@ await _subscriberRepository.LogSendAsync( logKey, sent ? 1 : 0, sent ? "sent" : "failed", - sent ? null : $"Delivery failed to {recipientEmail}", + // codeql[cs/exposure-of-private-information] - email is masked via MaskEmail() before writing to the log table + sent ? null : $"Delivery failed to {recipientEmail.MaskEmail()}", ct); return sent; } From de70d8ea7b9e92c7d7ccb665d84bd1598bc37531 Mon Sep 17 00:00:00 2001 From: Reinier Date: Tue, 2 Jun 2026 10:12:44 +0000 Subject: [PATCH 05/12] Track per-send failed delivery counts in newsletter log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Remove email addresses from all logs and error messages to eliminate PII exposure, and add per-send failed delivery counts to the send log for observability in the admin UI and daily status email. Implementation: • Remove MaskEmail() from InputSanitizer.cs and all its tests (no longer needed) • Remove using TechHub.Core.Logging from AcsEmailSender.cs and NewsletterService.cs • AcsEmailSender: drop recipient email from rate-limit and ACS error log messages • Add failed_count column via migration 046_newsletter_send_log_failed_count.sql • Add failedCount parameter to INewsletterSubscriberRepository.LogSendAsync, repository impl, and all 8 call sites in NewsletterService • Add FailedCount to NewsletterSendLogEntry model and SELECT query • Add FailedNewsletterSendsLast24Hours to NewsletterDailyReportStats, DB stats query, admin email HTML template and plain-text builder • AdminNewsletter.razor: rename Recipients column to Sent, add Failed column (shows — when 0) • Update repository integration test to pass new failedCount arg and assert new stats field --- .../INewsletterSubscriberRepository.cs | 1 + src/TechHub.Core/Logging/InputSanitizer.cs | 22 ----------- .../Admin/NewsletterDailyReportStats.cs | 1 + .../Models/Admin/NewsletterSendLogEntry.cs | 1 + .../046_newsletter_send_log_failed_count.sql | 2 + .../Resources/newsletter-admin-content.html | 1 + .../NewsletterSubscriberRepository.cs | 11 ++++-- .../Services/Newsletter/AcsEmailSender.cs | 7 +--- .../Services/Newsletter/NewsletterService.cs | 23 +++++------ .../Pages/Admin/AdminNewsletter.razor | 4 +- .../Logging/InputSanitizerTests.cs | 38 ------------------- .../NewsletterSubscriberRepositoryTests.cs | 3 +- 12 files changed, 33 insertions(+), 81 deletions(-) create mode 100644 src/TechHub.Infrastructure/Data/Migrations/postgres/046_newsletter_send_log_failed_count.sql diff --git a/src/TechHub.Core/Interfaces/INewsletterSubscriberRepository.cs b/src/TechHub.Core/Interfaces/INewsletterSubscriberRepository.cs index 1916c6cd8..9fac6762a 100644 --- a/src/TechHub.Core/Interfaces/INewsletterSubscriberRepository.cs +++ b/src/TechHub.Core/Interfaces/INewsletterSubscriberRepository.cs @@ -52,6 +52,7 @@ Task LogSendAsync( string sendKind, string targetKey, int recipientCount, + int failedCount, string status, string? errorMessage, CancellationToken ct = default); diff --git a/src/TechHub.Core/Logging/InputSanitizer.cs b/src/TechHub.Core/Logging/InputSanitizer.cs index 8e3a94727..b13b9727f 100644 --- a/src/TechHub.Core/Logging/InputSanitizer.cs +++ b/src/TechHub.Core/Logging/InputSanitizer.cs @@ -23,26 +23,4 @@ public static string Sanitize(this string? value) .Replace("\n", string.Empty, StringComparison.Ordinal); } - /// - /// Masks an email address for safe logging, preserving only the first character of the local - /// part and the full domain so logs remain useful without exposing PII. - /// Examples: user@example.com → u***@example.com, - /// a@b.com → a***@b.com. - /// - public static string MaskEmail(this string? email) - { - if (string.IsNullOrEmpty(email)) - { - return string.Empty; - } - - var atIndex = email.IndexOf('@', StringComparison.Ordinal); - if (atIndex <= 0) - { - // Not a recognisable email — sanitize only. - return email.Sanitize(); - } - - return string.Concat(email.AsSpan(0, 1), "***", email.AsSpan(atIndex)); - } } diff --git a/src/TechHub.Core/Models/Admin/NewsletterDailyReportStats.cs b/src/TechHub.Core/Models/Admin/NewsletterDailyReportStats.cs index 443ef0ce7..aef4dbdb3 100644 --- a/src/TechHub.Core/Models/Admin/NewsletterDailyReportStats.cs +++ b/src/TechHub.Core/Models/Admin/NewsletterDailyReportStats.cs @@ -8,4 +8,5 @@ public sealed class NewsletterDailyReportStats public int NewSubscribersLast24Hours { get; init; } public int ActiveSubscribers { get; init; } public int UnconfirmedSubscribers { get; init; } + public int FailedNewsletterSendsLast24Hours { get; init; } } diff --git a/src/TechHub.Core/Models/Admin/NewsletterSendLogEntry.cs b/src/TechHub.Core/Models/Admin/NewsletterSendLogEntry.cs index 66b29ce78..cb931cf5b 100644 --- a/src/TechHub.Core/Models/Admin/NewsletterSendLogEntry.cs +++ b/src/TechHub.Core/Models/Admin/NewsletterSendLogEntry.cs @@ -7,6 +7,7 @@ public sealed class NewsletterSendLogEntry public string TargetKey { get; init; } = string.Empty; public DateTimeOffset SentAt { get; init; } public int RecipientCount { get; init; } + public int FailedCount { get; init; } public string Status { get; init; } = string.Empty; public string? ErrorMessage { get; init; } } diff --git a/src/TechHub.Infrastructure/Data/Migrations/postgres/046_newsletter_send_log_failed_count.sql b/src/TechHub.Infrastructure/Data/Migrations/postgres/046_newsletter_send_log_failed_count.sql new file mode 100644 index 000000000..371c1a1ce --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Migrations/postgres/046_newsletter_send_log_failed_count.sql @@ -0,0 +1,2 @@ +ALTER TABLE newsletter_send_log + ADD COLUMN IF NOT EXISTS failed_count INT NOT NULL DEFAULT 0; diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html index 7eb7bc02b..24eb03d60 100644 --- a/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html @@ -1,6 +1,7 @@

New content items (24h): {{newContentItems}}

Failed processed URLs (24h): {{failedProcessedUrls}}

Failed background jobs (24h): {{failedJobs}}

+

Failed newsletter sends (24h): {{failedNewsletterSends}}

New newsletter subscribers (24h): {{newSubscribers}}

Active subscribers: {{activeSubscribers}}

Unconfirmed subscribers: {{unconfirmedSubscribers}}

diff --git a/src/TechHub.Infrastructure/Repositories/NewsletterSubscriberRepository.cs b/src/TechHub.Infrastructure/Repositories/NewsletterSubscriberRepository.cs index 9032434d5..bedbe1e7f 100644 --- a/src/TechHub.Infrastructure/Repositories/NewsletterSubscriberRepository.cs +++ b/src/TechHub.Infrastructure/Repositories/NewsletterSubscriberRepository.cs @@ -271,6 +271,7 @@ public async Task LogSendAsync( string sendKind, string targetKey, int recipientCount, + int failedCount, string status, string? errorMessage, CancellationToken ct = default) @@ -281,13 +282,14 @@ public async Task LogSendAsync( const string Sql = """ INSERT INTO newsletter_send_log - (send_kind, target_key, recipient_count, status, error_message) + (send_kind, target_key, recipient_count, failed_count, status, error_message) VALUES - (@SendKind, @TargetKey, @RecipientCount, @Status, @Error) + (@SendKind, @TargetKey, @RecipientCount, @FailedCount, @Status, @Error) ON CONFLICT (send_kind, target_key) DO UPDATE SET sent_at = NOW(), recipient_count = EXCLUDED.recipient_count, + failed_count = EXCLUDED.failed_count, status = EXCLUDED.status, error_message = EXCLUDED.error_message """; @@ -299,6 +301,7 @@ await _connection.ExecuteAsync(new CommandDefinition( SendKind = sendKind, TargetKey = targetKey, RecipientCount = recipientCount, + FailedCount = failedCount, Status = status, Error = errorMessage }, @@ -314,6 +317,7 @@ public async Task> GetSendLogAsync(int cou target_key AS TargetKey, sent_at AS SentAt, recipient_count AS RecipientCount, + failed_count AS FailedCount, status AS Status, error_message AS ErrorMessage FROM newsletter_send_log @@ -395,7 +399,8 @@ public async Task GetDailyReportStatsAsync(Cancellat (SELECT COUNT(*) FROM content_processing_jobs WHERE status = 'failed' AND started_at >= NOW() - INTERVAL '24 hours') AS FailedJobsLast24Hours, (SELECT COUNT(*) FROM newsletter_subscribers WHERE subscribed_at >= NOW() - INTERVAL '24 hours') AS NewSubscribersLast24Hours, (SELECT COUNT(*) FROM newsletter_subscribers WHERE is_confirmed = TRUE AND unsubscribed_at IS NULL) AS ActiveSubscribers, - (SELECT COUNT(*) FROM newsletter_subscribers WHERE is_confirmed = FALSE AND unsubscribed_at IS NULL) AS UnconfirmedSubscribers + (SELECT COUNT(*) FROM newsletter_subscribers WHERE is_confirmed = FALSE AND unsubscribed_at IS NULL) AS UnconfirmedSubscribers, + (SELECT COALESCE(SUM(failed_count), 0) FROM newsletter_send_log WHERE sent_at >= NOW() - INTERVAL '24 hours') AS FailedNewsletterSendsLast24Hours """; return await _connection.QuerySingleAsync(new CommandDefinition(Sql, cancellationToken: ct)); diff --git a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs index f9b1ff2ec..8dda8112d 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs @@ -4,7 +4,6 @@ using Microsoft.Extensions.Options; using TechHub.Core.Configuration; using TechHub.Core.Interfaces; -using TechHub.Core.Logging; namespace TechHub.Infrastructure.Services.Newsletter; @@ -53,14 +52,12 @@ public async Task SendAsync(string recipientEmail, string subject, string } catch (RequestFailedException ex) when (ex.Status == 429) { - // codeql[cs/exposure-of-private-information] - email is masked via MaskEmail() before logging - _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429). Email to {RecipientEmail} was not sent", recipientEmail.MaskEmail()); + _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429)"); return false; } catch (RequestFailedException ex) { - // codeql[cs/exposure-of-private-information] - email is masked via MaskEmail() before logging - _logger.LogError(ex, "Newsletter email send failed with ACS error {Status} for {RecipientEmail}", ex.Status, recipientEmail.MaskEmail()); + _logger.LogError(ex, "Newsletter email send failed with ACS error {Status}", ex.Status); return false; } diff --git a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs index 5c67258eb..f5a7b3c6b 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs @@ -8,7 +8,6 @@ using Microsoft.Extensions.Options; using TechHub.Core.Configuration; using TechHub.Core.Interfaces; -using TechHub.Core.Logging; using TechHub.Core.Models; using TechHub.Core.Models.Admin; using TechHub.Infrastructure.Services.RoundupGeneration; @@ -92,7 +91,7 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu if (!IsUnsubscribeSecretConfigured("combined-weekly")) { - await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, 0, "failed", "Unsubscribe secret is not configured", ct); + await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, 0, 0, "failed", "Unsubscribe secret is not configured", ct); return false; } @@ -147,7 +146,7 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu catch (Exception ex) when (ex is not OperationCanceledException) { var status = successful > 0 ? "partial" : "failed"; - await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, status, ex.Message, ct); + await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, attempted - successful, status, ex.Message, ct); _logger.LogError(ex, "Failed sending combined weekly newsletter"); return false; } @@ -156,7 +155,7 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu var sendError = attempted == 0 || successful == attempted ? null : $"Delivered to {successful} of {attempted} subscribers"; - await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, sendStatus, sendError, ct); + await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, attempted - successful, sendStatus, sendError, ct); return attempted == 0 || successful > 0; } @@ -222,9 +221,9 @@ await _subscriberRepository.LogSendAsync( "test-send", logKey, sent ? 1 : 0, + sent ? 0 : 1, sent ? "sent" : "failed", - // codeql[cs/exposure-of-private-information] - email is masked via MaskEmail() before writing to the log table - sent ? null : $"Delivery failed to {recipientEmail.MaskEmail()}", + sent ? null : "Delivery failed", ct); return sent; } @@ -270,9 +269,9 @@ await _subscriberRepository.LogSendAsync( "test-daily", logKey, sent ? 1 : 0, + sent ? 0 : 1, sent ? "sent" : "failed", - // codeql[cs/exposure-of-private-information] - email is masked via MaskEmail() before writing to the log table - sent ? null : $"Delivery failed to {recipientEmail.MaskEmail()}", + sent ? null : "Delivery failed", ct); return sent; } @@ -389,7 +388,7 @@ public async Task SendDailyOverviewAsync(DateOnly day, CancellationToken c if (dailySubscribers.Count == 0) { - await _subscriberRepository.LogSendAsync(SendKind, targetKey, 0, "sent", null, ct); + await _subscriberRepository.LogSendAsync(SendKind, targetKey, 0, 0, "sent", null, ct); return true; } @@ -423,7 +422,7 @@ public async Task SendDailyOverviewAsync(DateOnly day, CancellationToken c : sent > 0 ? $"Delivered to {sent} of {eligible} eligible subscribers" : "Delivery failed for all eligible subscribers"; - await _subscriberRepository.LogSendAsync(SendKind, targetKey, sent, sendStatus, sendError, ct); + await _subscriberRepository.LogSendAsync(SendKind, targetKey, sent, eligible - sent, sendStatus, sendError, ct); return sent > 0; } @@ -446,7 +445,7 @@ public async Task SendAdminStatusReportAsync(DateOnly day, CancellationTok var text = BuildAdminStatusText(day, stats); var sent = await SendEmailAsync(_options.AdminReportEmailAddress, $"TechHub Daily Status Report — {targetKey}", html, text, ct); - await _subscriberRepository.LogSendAsync(SendKind, targetKey, sent ? 1 : 0, sent ? "sent" : "failed", sent ? null : "Unable to send admin status report", ct); + await _subscriberRepository.LogSendAsync(SendKind, targetKey, sent ? 1 : 0, sent ? 0 : 1, sent ? "sent" : "failed", sent ? null : "Unable to send admin status report", ct); return sent; } @@ -851,6 +850,7 @@ private string BuildAdminStatusHtml(DateOnly day, NewsletterDailyReportStats sta newContentItems = stats.NewContentItemsLast24Hours.ToString(CultureInfo.InvariantCulture), failedProcessedUrls = stats.FailedProcessedUrlsLast24Hours.ToString(CultureInfo.InvariantCulture), failedJobs = stats.FailedJobsLast24Hours.ToString(CultureInfo.InvariantCulture), + failedNewsletterSends = stats.FailedNewsletterSendsLast24Hours.ToString(CultureInfo.InvariantCulture), newSubscribers = stats.NewSubscribersLast24Hours.ToString(CultureInfo.InvariantCulture), activeSubscribers = stats.ActiveSubscribers.ToString(CultureInfo.InvariantCulture), unconfirmedSubscribers = stats.UnconfirmedSubscribers.ToString(CultureInfo.InvariantCulture) @@ -883,6 +883,7 @@ private static string BuildAdminStatusText(DateOnly day, NewsletterDailyReportSt New content items (24h): {stats.NewContentItemsLast24Hours} Failed processed URLs (24h): {stats.FailedProcessedUrlsLast24Hours} Failed background jobs (24h): {stats.FailedJobsLast24Hours} + Failed newsletter sends (24h): {stats.FailedNewsletterSendsLast24Hours} New newsletter subscribers (24h): {stats.NewSubscribersLast24Hours} Active subscribers: {stats.ActiveSubscribers} Unconfirmed subscribers: {stats.UnconfirmedSubscribers} diff --git a/src/TechHub.Web/Components/Pages/Admin/AdminNewsletter.razor b/src/TechHub.Web/Components/Pages/Admin/AdminNewsletter.razor index 822210d19..aef8d575d 100644 --- a/src/TechHub.Web/Components/Pages/Admin/AdminNewsletter.razor +++ b/src/TechHub.Web/Components/Pages/Admin/AdminNewsletter.razor @@ -118,7 +118,8 @@ When Kind Target - Recipients + Sent + Failed Status @@ -130,6 +131,7 @@ @log.SendKind @log.TargetKey @log.RecipientCount + @(log.FailedCount > 0 ? log.FailedCount.ToString() : "—") @log.Status } diff --git a/tests/TechHub.Core.Tests/Logging/InputSanitizerTests.cs b/tests/TechHub.Core.Tests/Logging/InputSanitizerTests.cs index 3b0b0580f..704500e06 100644 --- a/tests/TechHub.Core.Tests/Logging/InputSanitizerTests.cs +++ b/tests/TechHub.Core.Tests/Logging/InputSanitizerTests.cs @@ -102,42 +102,4 @@ public void Sanitize_TypicalLogValues_ReturnsUnchanged(string input, string expe // Assert result.Should().Be(expected); } - - // ─── MaskEmail ─────────────────────────────────────────────────────────── - - [Fact] - public void MaskEmail_NullValue_ReturnsEmpty() - { - InputSanitizer.MaskEmail(null).Should().BeEmpty(); - } - - [Fact] - public void MaskEmail_EmptyString_ReturnsEmpty() - { - InputSanitizer.MaskEmail(string.Empty).Should().BeEmpty(); - } - - [Theory] - [InlineData("user@example.com", "u***@example.com")] - [InlineData("reinier@xebia.com", "r***@xebia.com")] - [InlineData("a@b.com", "a***@b.com")] - [InlineData("newsletter@mail.hub.ms", "n***@mail.hub.ms")] - public void MaskEmail_ValidEmail_MasksLocalPart(string input, string expected) - { - InputSanitizer.MaskEmail(input).Should().Be(expected); - } - - [Fact] - public void MaskEmail_NoAtSign_ReturnsSanitizedValue() - { - // Not a recognisable email — falls back to Sanitize behaviour (no crash). - InputSanitizer.MaskEmail("notanemail").Should().Be("notanemail"); - } - - [Fact] - public void MaskEmail_EmailWithNewline_MasksAndSanitizes() - { - // Log-forging attempt embedded in an otherwise valid email. - InputSanitizer.MaskEmail("u\nser@example.com").Should().Be("u***@example.com"); - } } diff --git a/tests/TechHub.Infrastructure.Tests/Repositories/NewsletterSubscriberRepositoryTests.cs b/tests/TechHub.Infrastructure.Tests/Repositories/NewsletterSubscriberRepositoryTests.cs index fa855e87c..eee7a63ac 100644 --- a/tests/TechHub.Infrastructure.Tests/Repositories/NewsletterSubscriberRepositoryTests.cs +++ b/tests/TechHub.Infrastructure.Tests/Repositories/NewsletterSubscriberRepositoryTests.cs @@ -109,7 +109,7 @@ await _sut.UpsertSubscriberAsync( [Fact] public async Task LogSendAsync_AndHasBeenSentAsync_WorkAsExpected() { - await _sut.LogSendAsync("weekly-roundup", "weekly-ai-roundup-2026-05-18", 12, "sent", null, TestContext.Current.CancellationToken); + await _sut.LogSendAsync("weekly-roundup", "weekly-ai-roundup-2026-05-18", 12, 0, "sent", null, TestContext.Current.CancellationToken); var hasBeenSent = await _sut.HasBeenSentAsync("weekly-roundup", "weekly-ai-roundup-2026-05-18", TestContext.Current.CancellationToken); hasBeenSent.Should().BeTrue(); @@ -126,6 +126,7 @@ public async Task GetDailyReportStatsAsync_ReturnsNonNegativeCounts() stats.NewContentItemsLast24Hours.Should().BeGreaterThanOrEqualTo(0); stats.FailedProcessedUrlsLast24Hours.Should().BeGreaterThanOrEqualTo(0); stats.FailedJobsLast24Hours.Should().BeGreaterThanOrEqualTo(0); + stats.FailedNewsletterSendsLast24Hours.Should().BeGreaterThanOrEqualTo(0); stats.NewSubscribersLast24Hours.Should().BeGreaterThanOrEqualTo(0); stats.ActiveSubscribers.Should().BeGreaterThanOrEqualTo(0); stats.UnconfirmedSubscribers.Should().BeGreaterThanOrEqualTo(0); From 4220193322e57fa77d1643f915517d31c13240aa Mon Sep 17 00:00:00 2001 From: Reinier Date: Tue, 2 Jun 2026 10:25:31 +0000 Subject: [PATCH 06/12] Exclude minimal API endpoints from HEAD short-circuit in HeadRequestMiddleware --- src/TechHub.Core/Logging/InputSanitizer.cs | 1 - .../Middleware/HeadRequestMiddleware.cs | 22 ++++++++++++---- .../Middleware/HeadRequestMiddlewareTests.cs | 25 +++++++++++++++++++ 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/TechHub.Core/Logging/InputSanitizer.cs b/src/TechHub.Core/Logging/InputSanitizer.cs index b13b9727f..126bdca65 100644 --- a/src/TechHub.Core/Logging/InputSanitizer.cs +++ b/src/TechHub.Core/Logging/InputSanitizer.cs @@ -22,5 +22,4 @@ public static string Sanitize(this string? value) .Replace("\r", string.Empty, StringComparison.Ordinal) .Replace("\n", string.Empty, StringComparison.Ordinal); } - } diff --git a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs index 284da331d..3d6e4ba75 100644 --- a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs +++ b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs @@ -7,18 +7,20 @@ namespace TechHub.Web.Middleware; /// Two categories of HEAD request are handled differently: /// /// -/// Extension-less paths (Blazor page routes) — Returned immediately with +/// Extension-less Blazor page routes — Returned immediately with /// 200 OK and Content-Type: text/html, bypassing Blazor SSR entirely. /// The SSR pipeline makes an API call for every page render; when the API is slow, /// bots and crawlers (Slack link previews, SEO scanners) disconnect after their /// 15–25 s timeout and record 499 (client closed request) in telemetry. Since HEAD /// only requires response headers — not a body — returning 200 immediately is -/// semantically correct and eliminates the 499s with ~23 s duration. -/// File-extension paths (static assets, RSS feeds) — Rewrites the method to +/// semantically correct and eliminates the 499s with ~23 s duration. +/// Known minimal API endpoints (/version, /health, /alive) +/// are excluded from this path and fall through to the rewrite path below. +/// File-extension paths and minimal API endpoints — Rewrites the method to /// GET, sets Response.Body = Stream.Null to suppress body output, and calls /// the next middleware. MapStaticAssets serves the correct headers (ETag, /// Content-Length, Cache-Control) without sending bytes; MapGet endpoints -/// (RSS feeds, /version, etc.) also work correctly this way. +/// (RSS feeds, /version, /health, /alive, etc.) also work correctly this way. /// /// /// @@ -29,6 +31,16 @@ namespace TechHub.Web.Middleware; /// public class HeadRequestMiddleware { + // Extension-less minimal API endpoints that must NOT be short-circuited. + // They are not Blazor page routes so they must go through the HEAD→GET rewrite + // path so their handlers can run and return correct headers/status codes. + private static readonly HashSet _minimalApiPaths = new(StringComparer.OrdinalIgnoreCase) + { + "/version", + "/health", + "/alive", + }; + private readonly RequestDelegate _next; public HeadRequestMiddleware(RequestDelegate next) @@ -54,7 +66,7 @@ public async Task InvokeAsync(HttpContext context) var lastSegment = path.AsSpan()[(lastSlash + 1)..]; var isExtensionless = !lastSegment.Contains('.'); - if (isExtensionless) + if (isExtensionless && !_minimalApiPaths.Contains(path)) { // Extension-less path → Blazor page route. Short-circuit with 200 immediately. // Calling next would trigger Blazor SSR → API call → potential slow response → diff --git a/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs index c856bd5db..d146bbcd8 100644 --- a/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs +++ b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs @@ -43,6 +43,31 @@ public async Task InvokeAsync_HeadExtensionlessPath_Returns200WithoutCallingNext context.Response.ContentType.Should().Be("text/html; charset=utf-8"); } + // ─── Known minimal API endpoints must use the HEAD→GET rewrite path ─────── + + [Theory] + [InlineData("/version")] + [InlineData("/health")] + [InlineData("/alive")] + public async Task InvokeAsync_HeadMinimalApiPath_CallsNextWithGetMethod(string path) + { + string? methodSeenByNext = null; + RequestDelegate next = ctx => + { + methodSeenByNext = ctx.Request.Method; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = path; + + await middleware.InvokeAsync(context); + + methodSeenByNext.Should().Be(HttpMethods.Get, "minimal API endpoints must see GET so their MapGet handlers match"); + context.Request.Method.Should().Be("HEAD", "method must be restored to HEAD after next() returns"); + } + [Fact] public async Task InvokeAsync_HeadExtensionlessPath_MethodRemainsHead() { From d998f7bd094f4668ea733a87796fba559d81b4fa Mon Sep 17 00:00:00 2001 From: Reinier Date: Tue, 2 Jun 2026 14:33:42 +0000 Subject: [PATCH 07/12] Update review prompts --- .../address-th-review-comments.prompt.md | 223 ++++++++++++++++-- 1 file changed, 200 insertions(+), 23 deletions(-) diff --git a/.github/prompts/address-th-review-comments.prompt.md b/.github/prompts/address-th-review-comments.prompt.md index d1ddf585d..20508ac6d 100644 --- a/.github/prompts/address-th-review-comments.prompt.md +++ b/.github/prompts/address-th-review-comments.prompt.md @@ -1,10 +1,10 @@ --- name: address-th-review-comments -description: "Reviews all open review comment threads on the current branch's pull request, analyses each one, applies code fixes where needed, replies to each thread explaining what was done or why it was ignored, resolves each thread after replying, then commits and pushes directly." +description: "Reviews all open review comment threads, CodeQL code scanning alerts, and GitHub Advanced Security alerts on the current branch's pull request, analyses each one, applies code fixes where needed, replies to each thread explaining what was done or why it was ignored, resolves each thread after replying, then commits and pushes directly." model: Claude Sonnet 4.6 --- -# Address PR Review Comments +# Address PR Review Comments, CodeQL Alerts, and Advanced Security Alerts **🚨 CRITICAL**: Read this entire prompt from start to finish before executing any step. @@ -103,7 +103,9 @@ Store the result as `[REPO]` (format: `owner/repo`). --- -## Step 5 — Fetch all open (unresolved) review threads +## Step 5 — Fetch all open issues (review threads + security alerts) + +### 5a — Fetch unresolved review threads Run the following GraphQL query to retrieve all unresolved review threads and their comments. Replace `[OWNER]`, `[REPONAME]`, and `[PR_NUMBER]` with the actual values (split `[REPO]` on `/`): @@ -171,21 +173,55 @@ gh api graphql -f query=' ' | Out-File -FilePath ".tmp/pr-review-threads.json" -Encoding utf8 ``` -Parse the file. Filter to threads where `isResolved` is `false`. If there are no unresolved threads, inform the user: +Parse the file. Filter to threads where `isResolved` is `false`. Count them and store as `[THREAD_COUNT]`. + +### 5b — Fetch open code scanning (CodeQL) alerts + +Fetch all open code scanning alerts for the branch: + +```pwsh +gh api "repos/[REPO]/code-scanning/alerts?ref=refs/heads/[BRANCHNAME]&state=open&per_page=100" | Out-File -FilePath ".tmp/pr-codeql-alerts.json" -Encoding utf8 +``` + +Parse the file. Each alert has: `number`, `rule.id`, `rule.description`, `rule.severity`, `most_recent_instance.location` (`path`, `start_line`, `end_line`), `most_recent_instance.message.text`, and `html_url`. + +If the command fails (e.g., code scanning is not enabled), record 0 alerts and continue. + +Count the open alerts and store as `[CODEQL_COUNT]`. + +### 5c — Fetch open secret scanning alerts + +Fetch all open secret scanning alerts: + +```pwsh +gh api "repos/[REPO]/secret-scanning/alerts?state=open&per_page=100" | Out-File -FilePath ".tmp/pr-secret-alerts.json" -Encoding utf8 +``` + +Parse the file. Each alert has: `number`, `secret_type_display_name`, `resolution`, `html_url`, and `locations_url`. + +If the command fails (e.g., secret scanning is not enabled), record 0 alerts and continue. + +Count the open alerts and store as `[SECRET_COUNT]`. + +### 5d — Summarise what was found -> All review threads on PR #[PR_NUMBER] are already resolved. Nothing to do. +If ALL three counts are zero, inform the user: -Then skip directly to Step 9. +> No open review threads, CodeQL alerts, or secret scanning alerts found on PR #[PR_NUMBER]. Nothing to do. -Otherwise, count the unresolved threads and report: +Then skip directly to Step 10. -> Found [N] unresolved review thread(s) to address. +Otherwise report: -**CHECKPOINT**: "✅ Step 5 completed. Found [N] unresolved thread(s). Moving to Step 6." +> Found [THREAD_COUNT] unresolved review thread(s), [CODEQL_COUNT] open CodeQL alert(s), and [SECRET_COUNT] open secret scanning alert(s) to address. + +**CHECKPOINT**: "✅ Step 5 completed. [THREAD_COUNT] thread(s), [CODEQL_COUNT] CodeQL alert(s), [SECRET_COUNT] secret alert(s). Moving to Step 6." --- -## Step 6 — Analyse and address each open thread +## Step 6 — Analyse and address each open review thread + +If `[THREAD_COUNT]` is 0, skip this step entirely. **🚨 CRITICAL**: Steps 6d (reply) and 6e (resolve) are **MANDATORY** for **every single thread** — including threads where you made a code fix. You are not done with a thread until you have BOTH posted a reply AND resolved it on GitHub. Never skip 6d or 6e. Never batch them for later. @@ -262,11 +298,132 @@ Confirm both actions completed, then state: Repeat steps 6a–6f for every unresolved thread before moving on. -**CHECKPOINT**: "✅ Step 6 completed. All [N] threads replied to and resolved on GitHub. Moving to Step 7." +**CHECKPOINT**: "✅ Step 6 completed. All [THREAD_COUNT] threads replied to and resolved on GitHub. Moving to Step 7." + +--- + +## Step 7 — Analyse and address each open CodeQL alert + +If `[CODEQL_COUNT]` is 0, skip this step entirely. + +Work through each open CodeQL alert one at a time. **Complete all sub-steps before moving to the next alert.** + +### 7a — Read and understand the alert + +For each alert, read: + +- **Rule**: `rule.id` and `rule.description` — what vulnerability or code quality issue was detected +- **Severity**: `rule.severity` (e.g., `error`, `warning`, `note`) +- **Location**: `most_recent_instance.location.path` and `start_line` — the exact file and line +- **Message**: `most_recent_instance.message.text` — the specific diagnostic message + +Read the relevant file around the flagged line to understand the current code in context. + +### 7b — Decide: fix or dismiss + +**NEEDS A FIX** — The alert points to a genuine security issue, vulnerability, or code defect that should be corrected (e.g., SQL injection risk, unvalidated input, exposed secret, use of a deprecated insecure API). + +**DISMISS** — The alert is a false positive, the code path is unreachable, the risk is mitigated elsewhere, or the flagged pattern is an intentional and safe design choice. + +**🚨 CRITICAL**: For `error`-severity alerts, default to fixing unless there is a clear, well-reasoned case for dismissal. + +### 7c — If NEEDS A FIX: apply the fix + +Make the minimal correct change to resolve the CodeQL finding. Follow the conventions in the relevant `AGENTS.md` files. Run `get_errors` after editing to ensure no new errors were introduced. + +### 7d — **MANDATORY**: Dismiss or note the alert on GitHub + +**If you fixed the code**: The alert will auto-close when the fix is pushed. No API call needed here — just make a note that this alert was fixed in code. + +**If dismissing (false positive / won't fix)**: Dismiss the alert via the API: + +```pwsh +gh api repos/[REPO]/code-scanning/alerts/[ALERT_NUMBER] -X PATCH -f state="dismissed" -f dismissed_reason="false positive" -f dismissed_comment="[One or two sentences explaining why this is a false positive or won't be fixed, referencing the specific code path or mitigation]" +``` + +Valid `dismissed_reason` values: `"false positive"`, `"won't fix"`, `"used in tests"`. + +Verify the command exits 0. If it fails, stop and report the error. + +### 7e — Checkpoint for this alert + +State: + +"✅ CodeQL alert [N/TOTAL] (#[ALERT_NUMBER] — [rule.id]) — [FIXED in code / DISMISSED]. [Brief one-line summary of action taken]." --- -## Step 7 — Verify no new errors +Repeat steps 7a–7e for every open CodeQL alert before moving on. + +**CHECKPOINT**: "✅ Step 7 completed. All [CODEQL_COUNT] CodeQL alerts addressed. Moving to Step 8." + +--- + +## Step 8 — Analyse and address each open secret scanning alert + +If `[SECRET_COUNT]` is 0, skip this step entirely. + +Work through each open secret scanning alert one at a time. + +### 8a — Read and understand the alert + +For each alert: + +- Note the `secret_type_display_name` (e.g., "GitHub Personal Access Token") +- Fetch the alert locations to find where in the codebase the secret appears: + + ```pwsh + gh api [LOCATIONS_URL] + ``` + +- Identify the file(s) and line(s) involved. + +### 8b — Decide: rotate or dismiss + +**ROTATE / REMEDIATE** — The secret is a real credential or token that should not be in source code. The correct fix is to: + +1. Remove the secret from the file (replace with an environment variable reference, a secrets manager reference, or a placeholder) +2. Immediately rotate/revoke the actual secret in the relevant system (GitHub, Azure Key Vault, etc.) — **inform the user** that rotation is needed since you cannot do this on their behalf + +**DISMISS** — The value is a test fixture, a clearly fake/placeholder value, or is already rotated and the alert is stale. + +**🚨 CRITICAL**: Never leave a real secret in the codebase. If in doubt, treat it as real and remediate. + +### 8c — If ROTATE / REMEDIATE: remove the secret from code + +Replace the hardcoded secret with an appropriate reference (e.g., `Environment.GetEnvironmentVariable("SECRET_NAME")`, a config binding, or a Key Vault reference). Follow existing patterns in the codebase. Run `get_errors` after editing. + +**Then inform the user**: + +> ⚠️ Secret `[SECRET_TYPE]` was found at `[FILE]:[LINE]`. The hardcoded value has been removed from the code. **You must manually rotate this secret** in [the relevant system] to ensure the exposed value is no longer valid. + +### 8d — **MANDATORY**: Resolve or dismiss the alert on GitHub + +**If you removed the secret from code**: The alert will auto-close when the fix is pushed. No API call needed. + +**If dismissing**: Dismiss via the API: + +```pwsh +gh api repos/[REPO]/secret-scanning/alerts/[ALERT_NUMBER] -X PATCH -f resolution="used_in_tests" -f resolution_comment="[Brief explanation]" +``` + +Valid `resolution` values: `"false_positive"`, `"wont_fix"`, `"revoked"`, `"used_in_tests"`. + +### 8e — Checkpoint for this alert + +State: + +"✅ Secret alert [N/TOTAL] (#[ALERT_NUMBER] — [secret_type_display_name]) — [REMEDIATED in code / DISMISSED]. [Brief one-line summary]." + +--- + +Repeat steps 8a–8e for every open secret scanning alert before moving on. + +**CHECKPOINT**: "✅ Step 8 completed. All [SECRET_COUNT] secret scanning alerts addressed. Moving to Step 9." + +--- + +## Step 9 — Verify no new errors Run: @@ -274,35 +431,55 @@ Run: Run -Clean ``` -If there are build or test failures caused by changes made in Step 6, fix them before proceeding. +If there are build or test failures caused by changes made in Steps 6, 7, or 8, fix them before proceeding. -**CHECKPOINT**: "✅ Step 7 completed. No errors or failures. Moving to Step 8." +**CHECKPOINT**: "✅ Step 9 completed. No errors or failures. Moving to Step 10." --- -## Step 8 — Summarise changes for the user +## Step 10 — Summarise changes for the user + +Print a concise summary with three sections: -Print a concise table of every thread and what was done: +**Review threads:** | # | File | Line | Action | Summary | -|---|------|------|--------|---------| +|---|------|------|--------|------| | 1 | ... | ... | Fixed / No fix | ... | -**CHECKPOINT**: "✅ Step 8 completed. Summary provided. Moving to Step 9." +**CodeQL alerts:** + +| # | Alert # | Rule | Severity | Action | Summary | +|---|---------|------|----------|--------|------| +| 1 | ... | ... | ... | Fixed / Dismissed | ... | + +**Secret scanning alerts:** + +| # | Alert # | Type | Action | Summary | +|---|---------|------|--------|------| +| 1 | ... | ... | Remediated / Dismissed | ... | + +If a category had 0 items, omit its table and note "None found." + +**CHECKPOINT**: "✅ Step 10 completed. Summary provided. Moving to Step 11." --- -## Step 9 — Commit and push directly +## Step 11 — Commit and push directly -If **no code changes** were made (all threads received "no fix needed" replies), skip this step — no commit is necessary. +If **no code changes** were made (all issues received "no fix" / "dismiss" responses), skip this step — no commit is necessary. -Otherwise, stage and commit only the files changed in Step 6: +Otherwise, stage all changed files: ```pwsh git add -A ``` -Write a short, direct commit message summarising the fixes (no ticket numbers, no PR references). Use imperative mood. Example: `"Address PR review comments: [brief summary]"`. +Write a short, direct commit message summarising the fixes (no ticket numbers, no PR references). Use imperative mood. Include all relevant issue types. Examples: + +- `"Address PR review comments: [brief summary]"` +- `"Fix CodeQL alerts: [brief summary]"` +- `"Address review comments and fix CodeQL alerts: [brief summary]"` ```pwsh git commit -m "[COMMIT_MESSAGE]" @@ -317,4 +494,4 @@ git push origin [BRANCHNAME] If the push is rejected for any reason, stop and ask the user. -**CHECKPOINT**: "✅ Step 9 completed. Changes committed and pushed. Workflow complete." +**CHECKPOINT**: "✅ Step 11 completed. Changes committed and pushed. Workflow complete." From 7e95313c5787fc3d4077d94b6861a5ef95a5774f Mon Sep 17 00:00:00 2001 From: Reinier Date: Tue, 2 Jun 2026 15:08:07 +0000 Subject: [PATCH 08/12] Address PR review comments: HEAD route validation, fix doc wording, per-subscriber newsletter error handling - HeadRequestMiddleware: add IsValidSlug validation for all path segments before the 200 short-circuit so structurally invalid extension-less HEAD requests return 404 instead of a misleading 200 - HeadRequestMiddleware: update XML doc comment to describe HEAD/GET semantic difference as a deliberate performance trade-off, not 'semantically correct' - NewsletterService: move exception handling inside foreach loop so a failure for one subscriber does not abort delivery to remaining subscribers (CA1031) - HeadRequestMiddlewareTests: add 5 test cases for invalid-segment 404 behaviour --- .../Services/Newsletter/NewsletterService.cs | 41 +++++++++---------- .../Middleware/HeadRequestMiddleware.cs | 25 ++++++++++- .../Middleware/HeadRequestMiddlewareTests.cs | 18 ++++++++ 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs index f5a7b3c6b..f00bba2d4 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs @@ -111,24 +111,24 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu var attempted = 0; var successful = 0; - try + foreach (var subscriber in allSubscribers.Values) { - foreach (var subscriber in allSubscribers.Values) + // Find the roundups this subscriber cares about (intersection of their sections and new roundups) + var relevantRoundups = subscriber.WeeklySections + .Where(s => roundupsBySection.ContainsKey(s)) + .Select(s => roundupsBySection[s]) + .ToList(); + relevantRoundups = OrderRoundupsByWebsiteOrder(relevantRoundups); + + if (relevantRoundups.Count == 0) { - // Find the roundups this subscriber cares about (intersection of their sections and new roundups) - var relevantRoundups = subscriber.WeeklySections - .Where(s => roundupsBySection.ContainsKey(s)) - .Select(s => roundupsBySection[s]) - .ToList(); - relevantRoundups = OrderRoundupsByWebsiteOrder(relevantRoundups); - - if (relevantRoundups.Count == 0) - { - continue; - } + continue; + } - attempted++; + attempted++; + try + { var unsubscribeUrl = BuildUnsubscribeUrl(subscriber.Email); var manageUrl = BuildManageUrl(subscriber.Email, _options.UnsubscribeSecret); var subject = relevantRoundups.Count == 1 @@ -142,13 +142,12 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu successful++; } } - } - catch (Exception ex) when (ex is not OperationCanceledException) - { - var status = successful > 0 ? "partial" : "failed"; - await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, attempted - successful, status, ex.Message, ct); - _logger.LogError(ex, "Failed sending combined weekly newsletter"); - return false; +#pragma warning disable CA1031 // Best-effort: continue with other subscribers if one fails + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "Failed sending weekly newsletter to subscriber — skipping"); + } +#pragma warning restore CA1031 } var sendStatus = attempted == 0 || successful == attempted ? "sent" : successful > 0 ? "partial" : "failed"; diff --git a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs index 3d6e4ba75..4c78b2449 100644 --- a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs +++ b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs @@ -1,3 +1,5 @@ +using TechHub.Core.Validation; + namespace TechHub.Web.Middleware; /// @@ -12,8 +14,10 @@ namespace TechHub.Web.Middleware; /// The SSR pipeline makes an API call for every page render; when the API is slow, /// bots and crawlers (Slack link previews, SEO scanners) disconnect after their /// 15–25 s timeout and record 499 (client closed request) in telemetry. Since HEAD -/// only requires response headers — not a body — returning 200 immediately is -/// semantically correct and eliminates the 499s with ~23 s duration. +/// only requires response headers — not a body — returning 200 immediately avoids +/// these 499s; note that for unknown routes the response may not mirror a GET (which +/// would 404 after SSR slug validation), so this is a deliberate performance +/// trade-off rather than a strict semantic guarantee. /// Known minimal API endpoints (/version, /health, /alive) /// are excluded from this path and fall through to the rewrite path below. /// File-extension paths and minimal API endpoints — Rewrites the method to @@ -68,6 +72,23 @@ public async Task InvokeAsync(HttpContext context) if (isExtensionless && !_minimalApiPaths.Contains(path)) { + // Validate all path segments structurally before short-circuiting. + // InvalidRouteSegmentMiddleware validates only the first segment; segments + // beyond that (collection, slug, date-component, etc.) are checked here so + // that clearly invalid paths return 404 rather than a misleading 200. + // IsValidSlug covers all structurally valid URL segments (letters, digits, + // hyphens) and rejects dots, spaces, special characters, and other patterns + // that can never match a Blazor route. + var segments = path.Split('/', StringSplitOptions.RemoveEmptyEntries); + foreach (var segment in segments) + { + if (!RouteParameterValidator.IsValidSlug(segment)) + { + context.Response.StatusCode = StatusCodes.Status404NotFound; + return; + } + } + // Extension-less path → Blazor page route. Short-circuit with 200 immediately. // Calling next would trigger Blazor SSR → API call → potential slow response → // bot/crawler timeout → 499. HEAD only needs headers, not a body. diff --git a/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs index d146bbcd8..2543848bc 100644 --- a/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs +++ b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs @@ -162,6 +162,24 @@ public async Task InvokeAsync_HeadFileExtensionPath_BodyRestoredEvenWhenNextThro context.Request.Method.Should().Be("HEAD", "method must be restored even when next() throws"); } + // ─── Structurally invalid extension-less paths → 404, no SSR ───────────── + + [Theory] + [InlineData("/ai/invalid segment")] // space in segment + [InlineData("/section/../../etc/passwd")] // path traversal + [InlineData("/section/col%lection")] // percent + invalid chars + [InlineData("/section/has_underscore")] // underscore is not a valid slug character + [InlineData("/section/has.dot/slug")] // dot in non-terminal segment + public async Task InvokeAsync_HeadExtensionlessPath_InvalidSegment_Returns404WithoutCallingNext(string path) + { + var (middleware, context, nextCalled) = CreateMiddleware("HEAD", path); + + await middleware.InvokeAsync(context); + + nextCalled().Should().BeFalse("SSR must be bypassed for structurally invalid extension-less HEAD requests"); + context.Response.StatusCode.Should().Be(StatusCodes.Status404NotFound); + } + // ─── Constructor guard ──────────────────────────────────────────────────── [Fact] From cdc0c98325d2e7ce1d355c66e75d5b9e0432a6ad Mon Sep 17 00:00:00 2001 From: Reinier Date: Tue, 2 Jun 2026 16:00:15 +0000 Subject: [PATCH 09/12] Fixed review wcomment --- src/TechHub.Web/Middleware/HeadRequestMiddleware.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs index 4c78b2449..dca8ad647 100644 --- a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs +++ b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs @@ -80,13 +80,10 @@ public async Task InvokeAsync(HttpContext context) // hyphens) and rejects dots, spaces, special characters, and other patterns // that can never match a Blazor route. var segments = path.Split('/', StringSplitOptions.RemoveEmptyEntries); - foreach (var segment in segments) + if (segments.Any(segment => !RouteParameterValidator.IsValidSlug(segment))) { - if (!RouteParameterValidator.IsValidSlug(segment)) - { - context.Response.StatusCode = StatusCodes.Status404NotFound; - return; - } + context.Response.StatusCode = StatusCodes.Status404NotFound; + return; } // Extension-less path → Blazor page route. Short-circuit with 200 immediately. From 360e9ac32005394175b19dfd72182be6332daf05 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 16:16:36 +0000 Subject: [PATCH 10/12] Exclude auth and Blazor HEAD routes from short-circuit --- .../Middleware/HeadRequestMiddleware.cs | 25 ++++++++++++++++--- .../Middleware/HeadRequestMiddlewareTests.cs | 25 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs index dca8ad647..22a07b64e 100644 --- a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs +++ b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs @@ -35,16 +35,25 @@ namespace TechHub.Web.Middleware; /// public class HeadRequestMiddleware { - // Extension-less minimal API endpoints that must NOT be short-circuited. + // Exact extension-less endpoints that must NOT be short-circuited. // They are not Blazor page routes so they must go through the HEAD→GET rewrite // path so their handlers can run and return correct headers/status codes. - private static readonly HashSet _minimalApiPaths = new(StringComparer.OrdinalIgnoreCase) + private static readonly HashSet _excludedExactPaths = new(StringComparer.OrdinalIgnoreCase) { "/version", "/health", "/alive", + "/signin-oidc", + "/admin/logout", }; + // Prefix-based endpoints that must NOT be short-circuited. + private static readonly string[] _excludedPrefixes = + [ + "/_blazor", + "/MicrosoftIdentity", + ]; + private readonly RequestDelegate _next; public HeadRequestMiddleware(RequestDelegate next) @@ -70,7 +79,7 @@ public async Task InvokeAsync(HttpContext context) var lastSegment = path.AsSpan()[(lastSlash + 1)..]; var isExtensionless = !lastSegment.Contains('.'); - if (isExtensionless && !_minimalApiPaths.Contains(path)) + if (isExtensionless && !IsShortCircuitExcluded(context.Request.Path)) { // Validate all path segments structurally before short-circuiting. // InvalidRouteSegmentMiddleware validates only the first segment; segments @@ -109,6 +118,16 @@ public async Task InvokeAsync(HttpContext context) context.Request.Method = HttpMethods.Head; } } + + private static bool IsShortCircuitExcluded(PathString path) + { + if (_excludedExactPaths.Contains(path.Value ?? string.Empty)) + { + return true; + } + + return _excludedPrefixes.Any(prefix => path.StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase)); + } } /// diff --git a/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs index 2543848bc..7f6c29c5e 100644 --- a/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs +++ b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs @@ -68,6 +68,31 @@ public async Task InvokeAsync_HeadMinimalApiPath_CallsNextWithGetMethod(string p context.Request.Method.Should().Be("HEAD", "method must be restored to HEAD after next() returns"); } + [Theory] + [InlineData("/_blazor")] + [InlineData("/_blazor/negotiate")] + [InlineData("/MicrosoftIdentity/Account/SignIn")] + [InlineData("/signin-oidc")] + [InlineData("/admin/logout")] + public async Task InvokeAsync_HeadExcludedNonPagePath_CallsNextWithGetMethod(string path) + { + string? methodSeenByNext = null; + RequestDelegate next = ctx => + { + methodSeenByNext = ctx.Request.Method; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = path; + + await middleware.InvokeAsync(context); + + methodSeenByNext.Should().Be(HttpMethods.Get, "excluded non-page endpoints must use the HEAD→GET rewrite path"); + context.Request.Method.Should().Be("HEAD", "method must be restored to HEAD after next() returns"); + } + [Fact] public async Task InvokeAsync_HeadExtensionlessPath_MethodRemainsHead() { From 0930d0d1b621eebb37e2f44c321fc8f25490beb5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 16:19:54 +0000 Subject: [PATCH 11/12] Polish HEAD route exclusion helper --- src/TechHub.Web/Middleware/HeadRequestMiddleware.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs index 22a07b64e..b6559a998 100644 --- a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs +++ b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs @@ -121,12 +121,21 @@ public async Task InvokeAsync(HttpContext context) private static bool IsShortCircuitExcluded(PathString path) { - if (_excludedExactPaths.Contains(path.Value ?? string.Empty)) + var value = path.Value; + if (value is not null && _excludedExactPaths.Contains(value)) { return true; } - return _excludedPrefixes.Any(prefix => path.StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase)); + for (var i = 0; i < _excludedPrefixes.Length; i++) + { + if (path.StartsWithSegments(_excludedPrefixes[i], StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; } } From 3c35e9520ad602d7df5411292f7630744fbf5bf9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 17:23:20 +0000 Subject: [PATCH 12/12] Stabilize Copilot features sidebar E2E test --- tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs b/tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs index 22ac171fa..3c5c24442 100644 --- a/tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs +++ b/tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs @@ -89,9 +89,11 @@ public async Task GitHubCopilotFeatures_PaidTiers_ShouldBeStacked_InSidebar() // Assert - All tiers should be stacked vertically in the sidebar var sidebar = Page.Locator("aside.sidebar"); + var tiersSection = sidebar.Locator(".features-tiers-sidebar"); + await tiersSection.AssertElementVisibleAsync(); + var tierCards = sidebar.Locator(".features-tier-card"); - var count = await tierCards.CountAsync(); - count.Should().Be(6, "Expected 6 tier cards stacked in the sidebar"); + await tierCards.AssertCountAsync(6); } [Fact]