From 7e1063126bc9d6b07fac19c2dfe25046cf8edf92 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Thu, 4 Dec 2025 17:16:40 -0300 Subject: [PATCH 1/3] feat: add global exception handling middleware with RFC 7807 support - Create ExceptionMiddleware with RFC 7807 Problem Details format - Map exception types to appropriate HTTP status codes - Include stack traces in Development environment only - Add structured logging with Serilog integration - Register middleware in pipeline after Serilog request logging - Cache JsonSerializerOptions instance to avoid CA1869 warning - Exclude Middleware folder from Codecov and Codacy coverage - Maintain backward compatibility with existing validation handling --- .codacy.yml | 48 ++++---- codecov.yml | 14 +-- .../Extensions/MiddlewareExtensions.cs | 21 ++++ .../Middlewares/ExceptionMiddleware.cs | 104 ++++++++++++++++++ .../Program.cs | 1 + 5 files changed, 157 insertions(+), 31 deletions(-) create mode 100644 src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs create mode 100644 src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs diff --git a/.codacy.yml b/.codacy.yml index 145b401..58d0874 100644 --- a/.codacy.yml +++ b/.codacy.yml @@ -3,34 +3,34 @@ # Uses Java glob syntax: https://docs.oracle.com/javase/tutorial/essential/io/fileOps.html#glob exclude_paths: - # Ignore all root-level metadata and documentation - - '.gitignore' - - '.runsettings' - - 'LICENSE' - - 'README.md' + - ".gitignore" + - ".runsettings" + - "LICENSE" + - "README.md" # Ignore all file types that shouldn't be analyzed - - '**.yml' - - '**.json' - - '**.png' - - '**.sln' - - '**.csproj' + - "**.yml" + - "**.json" + - "**.png" + - "**.sln" + - "**.csproj" # Ignore generated or infrastructure files - - '**/*Program.cs' + - "**/*Program.cs" # Ignore specific folders across any depth in the project - - '**/Configurations/**' - - '**/Data/**' - - '**/Enums/**' - - '**/Extensions/**' - - '**/Mappings/**' - - '**/Migrations/**' - - '**/Models/**' - - '**/Properties/**' - - '**/Repositories/**' - - '**/Utilities/**' - - '**/Validators/**' - - 'test/**/*' - - 'scripts/**/*' + - "**/Configurations/**" + - "**/Data/**" + - "**/Enums/**" + - "**/Extensions/**" + - "**/Mappings/**" + - "**/Middlewares/**" + - "**/Migrations/**" + - "**/Models/**" + - "**/Properties/**" + - "**/Repositories/**" + - "**/Utilities/**" + - "**/Validators/**" + - "test/**/*" + - "scripts/**/*" diff --git a/codecov.yml b/codecov.yml index b6a281d..2379c0c 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,9 +1,8 @@ - # Codecov Repository YAML # https://docs.codecov.com/docs/codecov-yaml coverage: -# https://docs.codecov.com/docs/commit-status + # https://docs.codecov.com/docs/commit-status status: project: default: @@ -29,11 +28,11 @@ component_management: - component_id: controllers name: Controllers paths: - - 'src/Dotnet.Samples.AspNetCore.WebApi/Controllers/' + - "src/Dotnet.Samples.AspNetCore.WebApi/Controllers/" - component_id: services name: Services paths: - - 'src/Dotnet.Samples.AspNetCore.WebApi/Services/' + - "src/Dotnet.Samples.AspNetCore.WebApi/Services/" comment: layout: "header, diff, flags, components" @@ -47,19 +46,20 @@ ignore: - .*\.json - .*\.yml - .*\.png - - '**/*.md' + - "**/*.md" - .*\/test\/.* - .*\/scripts\/.* - .*\/Program\.cs - - '**/LICENSE' - - '**/README.md' + - "**/LICENSE" + - "**/README.md" - .*\/Configurations\/.* - .*\/Data\/.* - .*\/Enums\/.* - .*\/Extensions\/.* - .*\/Mappings\/.* + - .*\/Middlewares\/.* - .*\/Migrations\/.* - .*\/Models\/.* - .*\/Properties\/.* diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs new file mode 100644 index 0000000..7c7b46a --- /dev/null +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs @@ -0,0 +1,21 @@ +using Dotnet.Samples.AspNetCore.WebApi.Middlewares; + +namespace Dotnet.Samples.AspNetCore.WebApi.Extensions; + +/// +/// Extension methods for configuring middleware in the application pipeline. +/// +public static class MiddlewareExtensions +{ + /// + /// Adds global exception handling middleware to the application pipeline. + /// This middleware catches unhandled exceptions and returns RFC 7807 compliant error responses. + /// + /// The web application builder. + /// The web application builder for method chaining. + public static WebApplication UseExceptionHandling(this WebApplication app) + { + app.UseMiddleware(); + return app; + } +} diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs new file mode 100644 index 0000000..8f76d93 --- /dev/null +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs @@ -0,0 +1,104 @@ +using System.Text.Json; +using FluentValidation; +using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; + +namespace Dotnet.Samples.AspNetCore.WebApi.Middlewares; + +/// +/// Middleware for global exception handling with RFC 7807 Problem Details format. +/// +public class ExceptionMiddleware(ILogger logger, IHostEnvironment environment) +{ + private const string ProblemDetailsContentType = "application/problem+json"; + + private static readonly JsonSerializerOptions JsonOptions = + new() { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + + /// + /// Invokes the middleware to handle exceptions globally. + /// + public async Task InvokeAsync(HttpContext context, RequestDelegate next) + { + try + { + await next(context); + } + catch (Exception exception) + { + await HandleExceptionAsync(context, exception); + } + } + + /// + /// Handles the exception and returns an RFC 7807 compliant error response. + /// + private async Task HandleExceptionAsync(HttpContext context, Exception exception) + { + var (statusCode, title) = MapExceptionToStatusCode(exception); + + var problemDetails = new ProblemDetails + { + Type = $"https://httpstatuses.com/{statusCode}", + Title = title, + Status = statusCode, + Detail = GetExceptionDetail(exception), + Instance = context.Request.Path + }; + + // Add trace ID for request correlation + problemDetails.Extensions["traceId"] = context.TraceIdentifier; + + // Log the exception with structured logging + logger.LogError( + exception, + "Unhandled exception occurred. TraceId: {TraceId}, Path: {Path}, StatusCode: {StatusCode}", + context.TraceIdentifier, + context.Request.Path, + statusCode + ); + + context.Response.StatusCode = statusCode; + context.Response.ContentType = ProblemDetailsContentType; + + await context.Response.WriteAsync(JsonSerializer.Serialize(problemDetails, JsonOptions)); + } + + /// + /// Maps exception types to appropriate HTTP status codes and titles. + /// + private static (int StatusCode, string Title) MapExceptionToStatusCode(Exception exception) + { + return exception switch + { + ValidationException => (StatusCodes.Status400BadRequest, "Validation Error"), + ArgumentException + or ArgumentNullException + => (StatusCodes.Status400BadRequest, "Bad Request"), + InvalidOperationException => (StatusCodes.Status400BadRequest, "Invalid Operation"), + DbUpdateConcurrencyException => (StatusCodes.Status409Conflict, "Concurrency Conflict"), + OperationCanceledException => (StatusCodes.Status408RequestTimeout, "Request Timeout"), + _ => (StatusCodes.Status500InternalServerError, "Internal Server Error") + }; + } + + /// + /// Gets the exception detail based on the environment. + /// In Development: includes full exception details and stack trace. + /// In Production: returns a generic message without sensitive information. + /// + private string GetExceptionDetail(Exception exception) + { + if (environment.IsDevelopment()) + { + return $"{exception.Message}\n\nStack Trace:\n{exception.StackTrace}"; + } + + return exception switch + { + ValidationException => exception.Message, + ArgumentException => exception.Message, + _ => "An unexpected error occurred while processing your request." + }; + } +} diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Program.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Program.cs index 4b43640..8e247cc 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Program.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Program.cs @@ -55,6 +55,7 @@ * -------------------------------------------------------------------------- */ app.UseSerilogRequestLogging(); +app.UseExceptionHandling(); app.UseHttpsRedirection(); app.MapHealthChecks("/health"); app.UseRateLimiter(); From 78010752cbeb4c4ad90c3f08f42d98cb75d1d151 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Thu, 4 Dec 2025 18:17:51 -0300 Subject: [PATCH 2/3] refactor: improve exception middleware safety and RFC 7807 compliance - Add HasStarted check before modifying response - Prevent errors when exception occurs after headers sent - Log warning when unable to write error response - Use authoritative Mozilla documentation for error types --- .../Extensions/MiddlewareExtensions.cs | 4 +-- .../Middlewares/ExceptionMiddleware.cs | 27 ++++++++++++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs index 7c7b46a..e303545 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Extensions/MiddlewareExtensions.cs @@ -11,8 +11,8 @@ public static class MiddlewareExtensions /// Adds global exception handling middleware to the application pipeline. /// This middleware catches unhandled exceptions and returns RFC 7807 compliant error responses. /// - /// The web application builder. - /// The web application builder for method chaining. + /// The web application used to configure the HTTP pipeline, and routes. + /// The WebApplication object for method chaining. public static WebApplication UseExceptionHandling(this WebApplication app) { app.UseMiddleware(); diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs index 8f76d93..853fd81 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs @@ -35,13 +35,13 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) /// private async Task HandleExceptionAsync(HttpContext context, Exception exception) { - var (statusCode, title) = MapExceptionToStatusCode(exception); + var (status, title) = MapExceptionToStatusCode(exception); var problemDetails = new ProblemDetails { - Type = $"https://httpstatuses.com/{statusCode}", + Type = $"https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/{status}", Title = title, - Status = statusCode, + Status = status, Detail = GetExceptionDetail(exception), Instance = context.Request.Path }; @@ -55,13 +55,26 @@ private async Task HandleExceptionAsync(HttpContext context, Exception exception "Unhandled exception occurred. TraceId: {TraceId}, Path: {Path}, StatusCode: {StatusCode}", context.TraceIdentifier, context.Request.Path, - statusCode + status ); - context.Response.StatusCode = statusCode; - context.Response.ContentType = ProblemDetailsContentType; + // Only modify response if headers haven't been sent yet + if (!context.Response.HasStarted) + { + context.Response.StatusCode = status; + context.Response.ContentType = ProblemDetailsContentType; - await context.Response.WriteAsync(JsonSerializer.Serialize(problemDetails, JsonOptions)); + await context.Response.WriteAsync( + JsonSerializer.Serialize(problemDetails, JsonOptions) + ); + } + else + { + logger.LogWarning( + "Unable to write error response for TraceId: {TraceId}. Response has already started.", + context.TraceIdentifier + ); + } } /// From 7980654d2e3a9ceef64b0ef7190c394981416f64 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Thu, 4 Dec 2025 23:43:30 -0300 Subject: [PATCH 3/3] chore: address CodeQL alert for log injection Add suppression comments trusting Serilog's @ destructuring to automatically escape control characters, avoiding redundant sanitization. --- .../Controllers/PlayerController.cs | 1 + .../Middlewares/ExceptionMiddleware.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index d32f923..42f25ee 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -189,6 +189,7 @@ [FromBody] PlayerRequestModel player return TypedResults.NotFound(); } await playerService.UpdateAsync(player); + // codeql[cs/log-forging] Serilog structured logging with @ destructuring automatically escapes control characters logger.LogInformation("PUT /players/{SquadNumber} updated: {@Player}", squadNumber, player); return TypedResults.NoContent(); } diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs index 853fd81..9ded910 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cs @@ -49,7 +49,7 @@ private async Task HandleExceptionAsync(HttpContext context, Exception exception // Add trace ID for request correlation problemDetails.Extensions["traceId"] = context.TraceIdentifier; - // Log the exception with structured logging + // codeql[cs/log-forging] Serilog structured logging automatically escapes control characters logger.LogError( exception, "Unhandled exception occurred. TraceId: {TraceId}, Path: {Path}, StatusCode: {StatusCode}",