Fix some issues from the code review

Author: CapulusCodeNinja

mcp_nexus/Controllers/McpController.cs

@@ -7,12 +7,13 @@
 using mcp_nexus.Infrastructure;
 using mcp_nexus.Session;
 using mcp_nexus.Models;
+using AspNetCoreRateLimit;
 
 namespace mcp_nexus.Controllers
 {
     [ApiController]
     [Route("mcp")]
-    public class McpController(McpProtocolService mcpProtocolService, ILogger<McpController> logger)
+    public class McpController(IMcpProtocolService mcpProtocolService, ILogger<McpController> logger)
         : ControllerBase
     {
         // PERFORMANCE: Reuse JSON options to avoid repeated allocations

mcp_nexus/Program.cs

@@ -2,6 +2,7 @@
 using System.Text.Json;
 using Microsoft.AspNetCore.Server.Kestrel.Core;
 using NLog.Web;
+using AspNetCoreRateLimit;
 
 using mcp_nexus.Constants;
 using mcp_nexus.Debugger;
@@ -551,7 +552,7 @@ private static async Task RunHttpServer(string[] args, CommandLineArguments comm
 
             ConfigureLogging(webBuilder.Logging, commandLineArgs.ServiceMode);
             RegisterServices(webBuilder.Services, webBuilder.Configuration, commandLineArgs.CustomCdbPath);
-            ConfigureHttpServices(webBuilder.Services);
+            ConfigureHttpServices(webBuilder.Services, webBuilder.Configuration);
 
             var app = webBuilder.Build();
             ConfigureHttpPipeline(app);
@@ -703,14 +704,14 @@ private static void RegisterServices(IServiceCollection services, IConfiguration
             Console.Error.WriteLine("Registered McpResourceService for MCP resources support");
         }
 
-        private static void ConfigureHttpServices(IServiceCollection services)
+        private static void ConfigureHttpServices(IServiceCollection services, IConfiguration configuration)
         {
             Console.WriteLine("Configuring MCP server for HTTP...");
 
             services.AddControllers()
                 .AddJsonOptions(options =>
                 {
-                    options.JsonSerializerOptions.PropertyNamingPolicy = JsonNamingPolicy.CamelCase;
+                    options.JsonSerializerOptions.PropertyNamingPolicy = null; // Don't change property names - MCP protocol requires exact field names
                     options.JsonSerializerOptions.WriteIndented = true;
                 });
 
@@ -738,14 +739,22 @@ private static void ConfigureHttpServices(IServiceCollection services)
                 });
             });
 
+            // Configure rate limiting
+            services.AddMemoryCache();
+            services.Configure<IpRateLimitOptions>(configuration.GetSection("IpRateLimiting"));
+            services.AddSingleton<IIpPolicyStore, MemoryCacheIpPolicyStore>();
+            services.AddSingleton<IRateLimitCounterStore, MemoryCacheRateLimitCounterStore>();
+            services.AddSingleton<IRateLimitConfiguration, RateLimitConfiguration>();
+            services.AddSingleton<IProcessingStrategy, AsyncKeyLockProcessingStrategy>();
+
             // MIGRATION: Register tool discovery without stdio transport for HTTP mode
             // Note: Tools are discovered automatically via [McpServerToolType] attribute
             // HTTP mode uses controllers instead of stdio transport
 
             // Register MCP services for HTTP endpoint compatibility
-            services.AddSingleton<McpToolDefinitionService>();
-            services.AddSingleton<McpToolExecutionService>();
-            services.AddSingleton<McpProtocolService>();
+            services.AddSingleton<IMcpToolDefinitionService, McpToolDefinitionService>();
+            services.AddSingleton<IMcpToolExecutionService, McpToolExecutionService>();
+            services.AddSingleton<IMcpProtocolService, McpProtocolService>();
             // Note: IMcpNotificationService now registered in shared RegisterServices() method
 
             Console.WriteLine("MCP server configured for HTTP with controllers, CORS, and tool discovery");
@@ -756,7 +765,7 @@ private static void ConfigureStdioServices(IServiceCollection services)
             Console.Error.WriteLine("Configuring MCP server for stdio...");
 
             // Add the MCP protocol service for logging comparison
-            services.AddSingleton<McpProtocolService>();
+            services.AddSingleton<IMcpProtocolService, McpProtocolService>();
 
             // Note: IMcpNotificationService is now registered in shared RegisterServices() method
 
@@ -774,6 +783,7 @@ private static void ConfigureHttpPipeline(WebApplication app)
         {
             Console.WriteLine("Configuring HTTP request pipeline...");
 
+            app.UseIpRateLimiting();
             app.UseCors();
             app.UseRouting();
             app.MapControllers();

mcp_nexus/Protocol/IMcpProtocolService.cs

@@ -0,0 +1,17 @@
+using System.Text.Json;
+
+namespace mcp_nexus.Protocol
+{
+    /// <summary>
+    /// Interface for MCP protocol service to enable better testability
+    /// </summary>
+    public interface IMcpProtocolService
+    {
+        /// <summary>
+        /// Process an incoming MCP request
+        /// </summary>
+        /// <param name="requestElement">The JSON request element</param>
+        /// <returns>Response object or null for notifications</returns>
+        Task<object?> ProcessRequest(JsonElement requestElement);
+    }
+}

mcp_nexus/Protocol/IMcpToolDefinitionService.cs

@@ -0,0 +1,16 @@
+using mcp_nexus.Models;
+
+namespace mcp_nexus.Protocol
+{
+    /// <summary>
+    /// Interface for MCP tool definition service to enable better testability
+    /// </summary>
+    public interface IMcpToolDefinitionService
+    {
+        /// <summary>
+        /// Get all available MCP tools
+        /// </summary>
+        /// <returns>Array of tool definitions</returns>
+        McpToolSchema[] GetAllTools();
+    }
+}

mcp_nexus/Protocol/IMcpToolExecutionService.cs

@@ -0,0 +1,18 @@
+using System.Text.Json;
+
+namespace mcp_nexus.Protocol
+{
+    /// <summary>
+    /// Interface for MCP tool execution service to enable better testability
+    /// </summary>
+    public interface IMcpToolExecutionService
+    {
+        /// <summary>
+        /// Execute a specific MCP tool
+        /// </summary>
+        /// <param name="toolName">Name of the tool to execute</param>
+        /// <param name="arguments">Tool arguments</param>
+        /// <returns>Tool execution result</returns>
+        Task<object> ExecuteTool(string toolName, JsonElement arguments);
+    }
+}

mcp_nexus/Protocol/McpProtocolService.cs

@@ -6,10 +6,10 @@
 namespace mcp_nexus.Protocol
 {
     public class McpProtocolService(
-        McpToolDefinitionService m_toolDefinitionService,
-        McpToolExecutionService m_toolExecutionService,
+        IMcpToolDefinitionService m_toolDefinitionService,
+        IMcpToolExecutionService m_toolExecutionService,
         McpResourceService m_resourceService,
-        ILogger<McpProtocolService> m_logger)
+        ILogger<McpProtocolService> m_logger) : IMcpProtocolService
     {
         public async Task<object?> ProcessRequest(JsonElement requestElement)
         {
@@ -19,7 +19,11 @@ public class McpProtocolService(
                 var request = ParseRequest(requestElement);
                 if (request == null)
                 {
-                    return CreateErrorResponse(null, -32600, "Invalid Request - malformed JSON-RPC");
+                    return CreateErrorResponse(null, -32600, "Invalid Request: The request is not a valid JSON-RPC 2.0 request. Please ensure the request contains a 'method' field and follows the JSON-RPC 2.0 specification.", new {
+                        errorType = "InvalidRequest",
+                        requiredFields = new[] { "jsonrpc", "method" },
+                        specification = "JSON-RPC 2.0"
+                    });
                 }
 
                 requestId = request.Id;
@@ -42,8 +46,12 @@ public class McpProtocolService(
             }
             catch (Exception ex)
             {
-                m_logger.LogError(ex, "Error processing MCP request");
-                return CreateErrorResponse(requestId, -32603, "Internal error", ex.Message);
+                m_logger.LogError(ex, "Unexpected error processing MCP request: {RequestId}", requestId);
+                return CreateErrorResponse(requestId, -32603, "Internal server error occurred while processing your request. Please try again or contact support if the issue persists.", new { 
+                    errorType = "InternalError",
+                    timestamp = DateTime.UtcNow,
+                    requestId = requestId?.ToString() ?? "unknown"
+                });
             }
         }
 

mcp_nexus/Protocol/McpToolDefinitionService.cs

@@ -4,7 +4,7 @@
 
 namespace mcp_nexus.Protocol
 {
-    public class McpToolDefinitionService
+    public class McpToolDefinitionService : IMcpToolDefinitionService
     {
         private readonly IMcpNotificationService? m_notificationService;
 

mcp_nexus/Protocol/McpToolExecutionService.cs

@@ -7,7 +7,7 @@ namespace mcp_nexus.Protocol
 {
     public class McpToolExecutionService(
         SessionAwareWindbgTool sessionAwareWindbgTool,
-        ILogger<McpToolExecutionService> logger)
+        ILogger<McpToolExecutionService> logger) : IMcpToolExecutionService
     {
         // PERFORMANCE: Cache JsonSerializerOptions to avoid repeated allocation
         private static readonly JsonSerializerOptions s_jsonOptions = new()

mcp_nexus/appsettings.json

@@ -42,5 +42,18 @@
       "DefaultCommandTimeoutMinutes": 10,
       "MemoryCleanupThresholdMB": 1024
     }
+  },
+  "IpRateLimiting": {
+    "EnableEndpointRateLimiting": true,
+    "StackBlockedRequests": false,
+    "RealIpHeader": "X-Real-IP",
+    "ClientIdHeader": "X-ClientId",
+    "GeneralRules": [
+      {
+        "Endpoint": "*:/mcp",
+        "Period": "1m",
+        "Limit": 100
+      }
+    ]
   }
 }

mcp_nexus/mcp_nexus.csproj

@@ -20,6 +20,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <PackageReference Include="AspNetCoreRateLimit" Version="5.0.0" />
     <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
     <PackageReference Include="ModelContextProtocol" Version="0.3.0-preview.4" />
     <PackageReference Include="NLog.Extensions.Logging" Version="6.0.4" />