Merge origin/main into security/ai-agent-hardening

BenjaminMichaelis · BenjaminMichaelis · commit 74fce2b28ad8 · 2026-05-15T08:24:50.000-07:00
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -2,7 +2,7 @@
   <PropertyGroup>
     <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
     <CentralPackageTransitivePinningEnabled>false</CentralPackageTransitivePinningEnabled>
-    <ToolingPackagesVersion>1.1.1.19044</ToolingPackagesVersion>
+    <ToolingPackagesVersion>1.1.1.19071</ToolingPackagesVersion>
     <AccessToNugetFeed Condition="'$(AccessToNugetFeed)' == ''">false</AccessToNugetFeed>
     <!-- Disable NuGet vulnerability audit when the private feed is unavailable (e.g. CI without credentials).
          NuGet audit queries all sources in nuget.config regardless of RestoreSources, causing NU1900 which is
@@ -17,20 +17,22 @@
   </ItemGroup>
   <ItemGroup>
     <PackageVersion Include="Azure.Extensions.AspNetCore.DataProtection.Keys" Version="1.6.3" />
-    <PackageVersion Include="Microsoft.AspNetCore.DataProtection.EntityFrameworkCore" Version="10.0.7" />
+    <PackageVersion Include="Microsoft.AspNetCore.DataProtection.EntityFrameworkCore" Version="10.0.8" />
     <PackageVersion Include="AspNet.Security.OAuth.GitHub" Version="10.0.0" />
     <PackageVersion Include="Azure.Extensions.AspNetCore.Configuration.Secrets" Version="1.5.1" />
     <PackageVersion Include="Azure.Identity" Version="1.21.0" />
     <PackageVersion Include="Azure.Monitor.OpenTelemetry.AspNetCore" Version="1.5.0" />
+    <!-- TODO: update to stable release when Azure.Monitor.OpenTelemetry.Profiler reaches GA -->
+    <PackageVersion Include="Azure.Monitor.OpenTelemetry.Profiler" Version="1.0.0-beta9" />
     <PackageVersion Include="TUnit" Version="1.40.5" />
     <PackageVersion Include="EssentialCSharp.Shared.Models" Version="$(ToolingPackagesVersion)" />
     <PackageVersion Include="HtmlAgilityPack" Version="1.12.4" />
     <PackageVersion Include="IntelliTect.Multitool" Version="2.1.0" />
     <PackageVersion Include="Mailjet.Api" Version="4.0.0" />
     <PackageVersion Include="Microsoft.AspNetCore.Authentication.MicrosoftAccount" Version="10.0.8" />
-    <PackageVersion Include="Microsoft.AspNetCore.Identity.EntityFrameworkCore" Version="10.0.7" />
+    <PackageVersion Include="Microsoft.AspNetCore.Identity.EntityFrameworkCore" Version="10.0.8" />
     <PackageVersion Include="Microsoft.AspNetCore.Identity.UI" Version="10.0.7" />
-    <PackageVersion Include="Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation" Version="10.0.7" />
+    <PackageVersion Include="Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation" Version="10.0.8" />
     <PackageVersion Include="Microsoft.AspNetCore.Mvc.Testing" Version="10.0.7" />
     <PackageVersion Include="Microsoft.CodeAnalysis.Common" Version="5.0.0" />
     <PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="5.0.0" />
diff --git a/EssentialCSharp.Web.Tests/FunctionalTests.cs b/EssentialCSharp.Web.Tests/FunctionalTests.cs
@@ -51,5 +51,10 @@ public async Task WhenTheApplicationStarts_NonExistingPage_GivesCorrectStatusCod
         using HttpResponseMessage response = await client.GetAsync("/non-existing-page1234");
 
         await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NotFound);
+
+        string content = await response.Content.ReadAsStringAsync();
+        await Assert.That(content).Contains("Go Home");
+        await Assert.That(content).Contains("Go Back");
+        await Assert.That(content).Contains("Browse Chapters");
     }
-}
+}
diff --git a/EssentialCSharp.Web/Controllers/ChatController.cs b/EssentialCSharp.Web/Controllers/ChatController.cs
@@ -37,10 +37,10 @@ public ChatController(ILogger<ChatController> logger, AIChatService aiChatServic
     /// <summary>
     /// Validates the hCaptcha token when captcha is configured.
     /// Returns <c>true</c> when captcha is not configured (dev mode) or when the token is valid.
-    /// Fails open on hCaptcha service outages (null result) to avoid blocking legitimate users —
-    /// this is intentional given the existing [Authorize] + rate-limiting backstop.
+    /// Returns <c>false</c> for missing or invalid tokens.
+    /// Returns <c>null</c> when hCaptcha cannot be reached, so the caller can fail closed.
     /// </summary>
-    private async Task<bool> IsCaptchaValidAsync(string? token, string? remoteIp, CancellationToken ct)
+    private async Task<bool?> IsCaptchaValidAsync(string? token, string? remoteIp, CancellationToken ct)
     {
         if (string.IsNullOrWhiteSpace(_CaptchaOptions.SecretKey))
             return true; // captcha not configured — skip validation
@@ -51,8 +51,8 @@ private async Task<bool> IsCaptchaValidAsync(string? token, string? remoteIp, Ca
         HCaptchaResult? result = await _CaptchaService.VerifyAsync(token, remoteIp, ct);
         if (result is null)
         {
-            LogCaptchaServiceUnavailable(_Logger); // hCaptcha unreachable — fail open (intentional)
-            return true;
+            LogCaptchaServiceUnavailable(_Logger); // hCaptcha unreachable — fail closed
+            return null;
         }
 
         if (!result.Success)
@@ -73,7 +73,10 @@ public async Task<IActionResult> SendMessage([FromBody] ChatMessageRequest reque
         if (string.IsNullOrEmpty(userId))
             return Unauthorized();
 
-        if (!await IsCaptchaValidAsync(request.CaptchaResponse, HttpContext.Connection.RemoteIpAddress?.ToString(), cancellationToken))
+        bool? captchaValid = await IsCaptchaValidAsync(request.CaptchaResponse, HttpContext.Connection.RemoteIpAddress?.ToString(), cancellationToken);
+        if (captchaValid is null)
+            return StatusCode(503, new { error = "Human verification is temporarily unavailable. Please try again later.", errorCode = "captcha_unavailable" });
+        if (!captchaValid.Value)
             return StatusCode(403, new { error = "Human verification required.", errorCode = "captcha_failed" });
 
         var previousResponseId = string.IsNullOrWhiteSpace(request.PreviousResponseId)
@@ -126,7 +129,14 @@ public async Task StreamMessage([FromBody] ChatMessageRequest request, Cancellat
             return;
         }
 
-        if (!await IsCaptchaValidAsync(request.CaptchaResponse, HttpContext.Connection.RemoteIpAddress?.ToString(), cancellationToken))
+        bool? captchaValid = await IsCaptchaValidAsync(request.CaptchaResponse, HttpContext.Connection.RemoteIpAddress?.ToString(), cancellationToken);
+        if (captchaValid is null)
+        {
+            Response.StatusCode = 503;
+            await Response.WriteAsJsonAsync(new { error = "Human verification is temporarily unavailable. Please try again later.", errorCode = "captcha_unavailable" }, CancellationToken.None);
+            return;
+        }
+        if (!captchaValid.Value)
         {
             Response.StatusCode = 403;
             await Response.WriteAsJsonAsync(new { error = "Human verification required.", errorCode = "captcha_failed" }, CancellationToken.None);
diff --git a/EssentialCSharp.Web/EssentialCSharp.Web.csproj b/EssentialCSharp.Web/EssentialCSharp.Web.csproj
@@ -38,6 +38,7 @@
     <PackageReference Include="Azure.Extensions.AspNetCore.DataProtection.Keys" />
     <PackageReference Include="Azure.Identity" />
     <PackageReference Include="Azure.Monitor.OpenTelemetry.AspNetCore" />
+    <PackageReference Include="Azure.Monitor.OpenTelemetry.Profiler" />
     <PackageReference Include="EssentialCSharp.Shared.Models" />
     <PackageReference Include="HtmlAgilityPack" />
     <PackageReference Include="IntelliTect.Multitool" />
diff --git a/EssentialCSharp.Web/Program.cs b/EssentialCSharp.Web/Program.cs
@@ -19,6 +19,7 @@
 using Microsoft.AspNetCore.Identity.UI.Services;
 using Microsoft.AspNetCore.RateLimiting;
 using Azure.Monitor.OpenTelemetry.AspNetCore;
+using Azure.Monitor.OpenTelemetry.Profiler;
 using Microsoft.AspNetCore.DataProtection;
 using Microsoft.AspNetCore.Diagnostics;
 using Microsoft.AspNetCore.Diagnostics.HealthChecks;
@@ -90,7 +91,7 @@ private static void Main(string[] args)
             });
 
         if (useAzureMonitor)
-            otel.UseAzureMonitor();
+            otel.UseAzureMonitor().AddAzureMonitorProfiler();
         else if (useOtlp)
             otel.UseOtlpExporter();
 
@@ -105,32 +106,40 @@ private static void Main(string[] args)
             c.Timeout = TimeSpan.FromSeconds(3);
         });
 
-
+        bool hasTrustedProxyConfig = false;
 
         builder.Services.Configure<ForwardedHeadersOptions>(options =>
         {
             options.ForwardedHeaders =
                 ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto;
 
-            options.KnownIPNetworks.Clear();
-            options.KnownProxies.Clear();
-
-            // Restrict trusted proxy sources to configured CIDRs.
-            // SECURITY: Set ForwardedHeaders:TrustedProxyCidrs in production to your
-            // load-balancer IP range (e.g., the Azure Container Apps ingress CIDR) to
-            // prevent X-Forwarded-For spoofing. Without this, any client can fabricate
-            // a forwarded IP and bypass IP-partitioned rate limits.
             var trustedCidrs = builder.Configuration
                 .GetSection("ForwardedHeaders:TrustedProxyCidrs")
                 .Get<string[]>() ?? [];
 
+            List<System.Net.IPNetwork> trustedNetworks = [];
             foreach (var cidr in trustedCidrs)
             {
                 if (System.Net.IPNetwork.TryParse(cidr, out var network))
-                    options.KnownIPNetworks.Add(network);
+                {
+                    trustedNetworks.Add(network);
+                    hasTrustedProxyConfig = true;
+                }
                 else
                     Console.Error.WriteLine($"[WARN] ForwardedHeaders:TrustedProxyCidrs: could not parse '{cidr}' as an IP network — entry skipped. Check your configuration.");
             }
+
+            // Only replace the default loopback trust set when at least one valid
+            // proxy CIDR is configured. Otherwise forwarded headers remain limited to
+            // the default safe behavior instead of trusting arbitrary client-supplied
+            // X-Forwarded-* values.
+            if (hasTrustedProxyConfig)
+            {
+                options.KnownIPNetworks.Clear();
+                options.KnownProxies.Clear();
+                foreach (System.Net.IPNetwork network in trustedNetworks)
+                    options.KnownIPNetworks.Add(network);
+            }
         });
 
         ConfigurationManager configuration = builder.Configuration;
@@ -444,8 +453,7 @@ await context.HttpContext.Response.WriteAsync(
         // X-Forwarded-For spoofing protection (F4) is visibly inactive until properly configured.
         if (!app.Environment.IsDevelopment())
         {
-            var fwdOpts = app.Services.GetRequiredService<IOptions<ForwardedHeadersOptions>>().Value;
-            if (fwdOpts.KnownIPNetworks.Count == 0 && fwdOpts.KnownProxies.Count == 0)
+            if (!hasTrustedProxyConfig)
                 LogTrustedProxyCidrsNotConfigured(app.Logger);
         }
 
@@ -614,6 +622,16 @@ await McpJsonRpcResponseWriter.WriteErrorAsync(
             LogSitemapValidationFailed(logger, ex);
             // Continue startup even if sitemap validation fails
         }
+        catch (ArgumentException ex)
+        {
+            LogSitemapValidationFailed(logger, ex);
+            // Continue startup even if sitemap validation fails
+        }
+        catch (FormatException ex)
+        {
+            LogSitemapValidationFailed(logger, ex);
+            // Continue startup even if sitemap validation fails
+        }
 
         app.Run();
     }
diff --git a/EssentialCSharp.Web/Views/Home/Error.cshtml b/EssentialCSharp.Web/Views/Home/Error.cshtml
@@ -16,6 +16,7 @@
     }
     <div class="d-flex gap-2">
         <a href="/home" class="btn btn-primary">Go Home</a>
+        <a href="/hello-world" class="btn btn-outline-primary">Browse Chapters</a>
         <button type="button" class="btn btn-secondary"
                 onclick="if(history.length > 1){ history.back(); } else { window.location='/home'; }">
             Go Back
diff --git a/EssentialCSharp.Web/appsettings.Development.json b/EssentialCSharp.Web/appsettings.Development.json
@@ -3,7 +3,9 @@
   "Logging": {
     "LogLevel": {
       "Default": "Information",
-      "Microsoft.AspNetCore": "Warning"
+      "Microsoft.AspNetCore": "Warning",
+      "Microsoft.ServiceProfiler": "Debug",
+      "Azure.Monitor.OpenTelemetry.Profiler": "Debug"
     }
   },
   "ConnectionStrings": {
diff --git a/EssentialCSharp.Web/wwwroot/js/chat-module.js b/EssentialCSharp.Web/wwwroot/js/chat-module.js
@@ -19,6 +19,7 @@ let captchaWidgetId = null;
 let captchaTokenResolve = null;
 let captchaTokenReject = null;
 let captchaPending = false; // prevents concurrent token requests overwriting promise callbacks
+let captchaRequestGeneration = 0;
 const CAPTCHA_TIMEOUT_MS = 15_000;
 
 // Resolves once the widget has rendered. getCaptchaToken() awaits this so a user who
@@ -73,12 +74,17 @@ function getCaptchaToken() {
     if (!captchaSiteKey) return Promise.resolve(null);
     if (captchaPending) return Promise.reject(new Error('captcha-concurrent'));
     captchaPending = true;
+    const requestGeneration = ++captchaRequestGeneration;
 
     let timeoutId;
 
     // Chain onto captchaWidgetReady so calls made before the widget finishes loading
     // wait rather than immediately returning null and causing a 403.
     const tokenPromise = captchaWidgetReady.then(() => new Promise((resolve, reject) => {
+        if (requestGeneration !== captchaRequestGeneration) {
+            reject(new Error('captcha-stale'));
+            return;
+        }
         captchaTokenResolve = (token) => {
             captchaPending = false;
             clearTimeout(timeoutId); // cancel lingering timer so it can't corrupt the next call
@@ -95,6 +101,8 @@ function getCaptchaToken() {
     const timeoutPromise = new Promise((_, reject) => {
         // timeoutId is assigned synchronously here, before captchaTokenResolve can fire
         timeoutId = setTimeout(() => {
+            if (requestGeneration !== captchaRequestGeneration) return;
+            captchaRequestGeneration++;
             captchaPending = false;
             captchaTokenResolve = null;
             captchaTokenReject  = null;
@@ -106,6 +114,7 @@ function getCaptchaToken() {
 }
 
 function resetCaptchaWidget() {
+    captchaRequestGeneration++;
     captchaPending = false;
     captchaTokenResolve = null;
     captchaTokenReject = null;
@@ -339,6 +348,8 @@ export function useChatWidget() {
                     throw new Error('Authentication required');
                 } else if (response.status === 403) {
                     throw new Error('captcha-failed: Human verification failed. Please try again.');
+                } else if (response.status === 503) {
+                    throw new Error('captcha-unavailable: Human verification is temporarily unavailable. Please try again later.');
                 } else if (response.status === 429) {
                     // Handle rate limiting - simple error message without captcha
                     let errorData;
@@ -441,7 +452,9 @@ export function useChatWidget() {
                 errorType = 'auth-error';
                 isAuthenticated.value = false; // Update auth state
             } else if (error.message?.startsWith('captcha-')) {
-                errorMessage = 'Human verification failed. Please try again.';
+                errorMessage = error.message.includes('captcha-unavailable')
+                    ? 'Human verification is temporarily unavailable. Please try again later.'
+                    : 'Human verification failed. Please try again.';
                 errorType = 'captcha-error';
             } else if (error.message?.includes('Rate limit exceeded')) {
                 errorMessage = error.message; // Use the specific rate limit message with timing

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@`
`16`	`16`	`}`
`17`	`17`	`<div class="d-flex gap-2">`
`18`	`18`	`<a href="/home" class="btn btn-primary">Go Home</a>`
	`19`	`+ <a href="/hello-world" class="btn btn-outline-primary">Browse Chapters</a>`
`19`	`20`	`<button type="button" class="btn btn-secondary"`
`20`	`21`	`onclick="if(history.length > 1){ history.back(); } else { window.location='/home'; }">`
`21`	`22`	`Go Back`