fix: address PR #1073 second-round feedback (security hardening iteration 4)

- Thread endUserId through GetChatCompletionCore, ProcessStreamingUpdatesAsync, and ExecuteFunctionCallAsync; include in tool-call and tool-rejected log messages for security telemetry
- Emit responseId early from StreamingResponseCreatedUpdate so ChatController can record ownership before stream completes (handles mid-stream client disconnects)
- Record responseId in streaming loop on first observation instead of post-loop, eliminating the disconnect race
- Add SizeLimit=10_000 to AddMemoryCache to bound memory growth; add SetSize(1) per entry in ResponseIdValidationService
- Fix MemoryCache disposal in ResponseIdValidationServiceTests: each test owns its cache via 'using var cache'
- Fix IServiceScope disposal in McpApiTokenServiceTests: collect scopes in _scopes list, dispose all via [After(Test)] teardown

Author: BenjaminMichaelis

EssentialCSharp.Chat.Shared/Services/AIChatService.cs

@@ -65,7 +65,7 @@ public AIChatService(IOptions<AIOptions> options, AISearchService searchService,
     {
         var responseOptions = await CreateResponseOptionsAsync(previousResponseId, tools, reasoningEffortLevel, mcpClient: mcpClient, endUserId: endUserId, cancellationToken: cancellationToken);
         var enrichedPrompt = await EnrichPromptWithContext(prompt, enableContextualSearch, cancellationToken);
-        return await GetChatCompletionCore(enrichedPrompt, responseOptions, systemPrompt, mcpClient, cancellationToken);
+        return await GetChatCompletionCore(enrichedPrompt, responseOptions, systemPrompt, mcpClient, endUserId, cancellationToken);
     }
 
     /// <summary>
@@ -111,7 +111,7 @@ public AIChatService(IOptions<AIOptions> options, AISearchService searchService,
             options: responseOptions,
             cancellationToken: cancellationToken);
 
-        await foreach (var result in ProcessStreamingUpdatesAsync(streamingUpdates, responseOptions, mcpClient, toolCallDepth: 0, cancellationToken))
+        await foreach (var result in ProcessStreamingUpdatesAsync(streamingUpdates, responseOptions, mcpClient, toolCallDepth: 0, endUserId: endUserId, cancellationToken: cancellationToken))
         {
             yield return result;
         }
@@ -185,6 +185,7 @@ private static string SanitizeForXmlContext(string? input) =>
 #pragma warning restore OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
         McpClient? mcpClient,
         int toolCallDepth = 0,
+        string? endUserId = null,
         [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken = default)
     {
         await foreach (var update in streamingUpdates.WithCancellation(cancellationToken))
@@ -193,8 +194,10 @@ private static string SanitizeForXmlContext(string? input) =>
 #pragma warning disable OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
             if (update is StreamingResponseCreatedUpdate created)
             {
-                // Remember the response ID for later function calls
+                // Emit the response ID early so the controller can record ownership
+                // before the stream completes — handles client disconnects mid-stream.
                 responseId = created.Response.Id;
+                yield return (string.Empty, responseId: responseId);
             }
             else if (update is StreamingResponseOutputItemDoneUpdate itemDone)
             {
@@ -205,7 +208,7 @@ private static string SanitizeForXmlContext(string? input) =>
                         throw new InvalidOperationException("Maximum tool call depth exceeded.");
 
                     // Execute the function call and stream its response
-                    await foreach (var functionResult in ExecuteFunctionCallAsync(functionCallItem, responseOptions, mcpClient, toolCallDepth + 1, cancellationToken))
+                    await foreach (var functionResult in ExecuteFunctionCallAsync(functionCallItem, responseOptions, mcpClient, toolCallDepth + 1, endUserId, cancellationToken))
                     {
                         if (functionResult.responseId != null)
                         {
@@ -237,12 +240,13 @@ private static string SanitizeForXmlContext(string? input) =>
 #pragma warning restore OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
         McpClient mcpClient,
         int toolCallDepth,
+        string? endUserId = null,
         [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken = default)
     {
         // Defense-in-depth: validate tool name against static allowlist before executing.
         if (!IsMcpToolAllowed(functionCallItem.FunctionName))
         {
-            LogMcpToolCallRejected(_Logger, functionCallItem.FunctionName);
+            LogMcpToolCallRejected(_Logger, functionCallItem.FunctionName, endUserId);
             // Feed a benign error back to the model so it can recover gracefully,
             // mirroring what GetChatCompletionCore does on the non-streaming path.
 #pragma warning disable OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
@@ -257,14 +261,14 @@ private static string SanitizeForXmlContext(string? input) =>
             var recoveryStream = _ResponseClient.CreateResponseStreamingAsync(
                 errorItems, responseOptions, cancellationToken);
             await foreach (var result in ProcessStreamingUpdatesAsync(
-                recoveryStream, responseOptions, mcpClient, toolCallDepth, cancellationToken))
+                recoveryStream, responseOptions, mcpClient, toolCallDepth, endUserId, cancellationToken))
             {
                 yield return result;
             }
             yield break;
         }
 
-        LogMcpToolCallInvokedStream(_Logger, functionCallItem.FunctionName, toolCallDepth);
+        LogMcpToolCallInvokedStream(_Logger, functionCallItem.FunctionName, toolCallDepth, endUserId);
         // A dictionary of arguments to pass to the tool. Each key represents a parameter name, and its associated value represents the argument value.
         Dictionary<string, object?> arguments = [];
         // example JsonResponse:
@@ -315,7 +319,7 @@ private static string SanitizeForXmlContext(string? input) =>
             responseOptions,
             cancellationToken);
 
-        await foreach (var result in ProcessStreamingUpdatesAsync(functionResponseStream, responseOptions, mcpClient, toolCallDepth, cancellationToken))
+        await foreach (var result in ProcessStreamingUpdatesAsync(functionResponseStream, responseOptions, mcpClient, toolCallDepth, endUserId, cancellationToken))
         {
             yield return result;
         }
@@ -402,6 +406,7 @@ private async Task<ResponseCreationOptions> CreateResponseOptionsAsync(
 #pragma warning restore OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
         string? systemPrompt = null,
         McpClient? mcpClient = null,
+        string? endUserId = null,
         CancellationToken cancellationToken = default)
     {
         // Construct the user input with system context if provided
@@ -434,7 +439,7 @@ private async Task<ResponseCreationOptions> CreateResponseOptionsAsync(
                     // This catches cases where the model hallucinates a tool name not on the list.
                     if (!IsMcpToolAllowed(functionCallItem.FunctionName))
                     {
-                        LogMcpToolCallRejected(_Logger, functionCallItem.FunctionName);
+                        LogMcpToolCallRejected(_Logger, functionCallItem.FunctionName, endUserId);
                         // Return a benign error to the model so it can respond gracefully
                         responseItems.Add(functionCallItem);
                         responseItems.Add(new FunctionCallOutputResponseItem(
@@ -443,7 +448,7 @@ private async Task<ResponseCreationOptions> CreateResponseOptionsAsync(
                         continue;
                     }
 
-                    LogMcpToolCallInvoked(_Logger, functionCallItem.FunctionName, iteration);
+                    LogMcpToolCallInvoked(_Logger, functionCallItem.FunctionName, iteration, endUserId);
                     var jsonResponse = functionCallItem.FunctionArguments.ToString();
                     var jsonArguments = System.Text.Json.JsonSerializer.Deserialize<Dictionary<string, object?>>(jsonResponse) ?? new Dictionary<string, object?>();
 
@@ -500,14 +505,14 @@ private bool IsMcpToolAllowed(string toolName)
         return _AllowedMcpTools.Contains(toolName);
     }
 
-    [LoggerMessage(Level = LogLevel.Information, Message = "AI tool call invoked: tool={ToolName} iteration={Iteration}")]
-    private static partial void LogMcpToolCallInvoked(ILogger logger, string toolName, int iteration);
+    [LoggerMessage(Level = LogLevel.Information, Message = "AI tool call invoked: tool={ToolName} iteration={Iteration} user={EndUserId}")]
+    private static partial void LogMcpToolCallInvoked(ILogger logger, string toolName, int iteration, string? endUserId);
 
-    [LoggerMessage(Level = LogLevel.Information, Message = "AI tool call invoked (streaming): tool={ToolName} depth={Depth}")]
-    private static partial void LogMcpToolCallInvokedStream(ILogger logger, string toolName, int depth);
+    [LoggerMessage(Level = LogLevel.Information, Message = "AI tool call invoked (streaming): tool={ToolName} depth={Depth} user={EndUserId}")]
+    private static partial void LogMcpToolCallInvokedStream(ILogger logger, string toolName, int depth, string? endUserId);
 
-    [LoggerMessage(Level = LogLevel.Warning, Message = "AI tool call rejected — not on allowlist: tool={ToolName}")]
-    private static partial void LogMcpToolCallRejected(ILogger logger, string toolName);
+    [LoggerMessage(Level = LogLevel.Warning, Message = "AI tool call rejected — not on allowlist: tool={ToolName} user={EndUserId}")]
+    private static partial void LogMcpToolCallRejected(ILogger logger, string toolName, string? endUserId);
 
     [LoggerMessage(Level = LogLevel.Warning, Message = "MCP tool skipped during option setup — not on allowlist: tool={ToolName}")]
     private static partial void LogMcpToolSkippedNotAllowed(ILogger logger, string toolName);

EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs

@@ -9,18 +9,30 @@ namespace EssentialCSharp.Web.Tests;
 [ClassDataSource<WebApplicationFactory>(Shared = SharedType.PerClass)]
 public class McpApiTokenServiceTests(WebApplicationFactory factory)
 {
+    private readonly List<IServiceScope> _scopes = [];
+
+    [After(Test)]
+    public void DisposeScopes()
+    {
+        foreach (var scope in _scopes)
+            scope.Dispose();
+        _scopes.Clear();
+    }
+
     private async Task<(string UserId, McpApiTokenService TokenService)> ArrangeAsync(string prefix)
     {
         string userId = await McpTestHelper.CreateUserAsync(factory, prefix);
-        var tokenService = factory.Services.CreateScope().ServiceProvider
-            .GetRequiredService<McpApiTokenService>();
+        var scope = factory.Services.CreateScope();
+        _scopes.Add(scope);
+        var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
         return (userId, tokenService);
     }
 
     private async Task<McpApiTokenService> FillToLimitAsync(string userId)
     {
-        var tokenService = factory.Services.CreateScope().ServiceProvider
-            .GetRequiredService<McpApiTokenService>();
+        var scope = factory.Services.CreateScope();
+        _scopes.Add(scope);
+        var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
         for (int i = 0; i < McpApiTokenService.MaxTokensPerUser; i++)
         {
             await tokenService.CreateTokenAsync(userId, $"token-{i}");

EssentialCSharp.Web.Tests/ResponseIdValidationServiceTests.cs

@@ -5,15 +5,15 @@ namespace EssentialCSharp.Web.Tests;
 
 public class ResponseIdValidationServiceTests
 {
-    private static ResponseIdValidationService CreateService()
-        => new(new MemoryCache(new MemoryCacheOptions()));
+    private static ResponseIdValidationService CreateService(MemoryCache cache) => new(cache);
 
     [Test]
     [Arguments(null)]
     [Arguments("")]
     public async Task ValidateResponseId_BlankResponseId_AllowsNewConversation(string? responseId)
     {
-        var service = CreateService();
+        using var cache = new MemoryCache(new MemoryCacheOptions());
+        var service = CreateService(cache);
 
         bool result = service.ValidateResponseId("user1", responseId);
 
@@ -25,7 +25,8 @@ public async Task ValidateResponseId_BlankResponseId_AllowsNewConversation(strin
     [Arguments("")]
     public async Task ValidateResponseId_BlankUserId_Rejects(string? userId)
     {
-        var service = CreateService();
+        using var cache = new MemoryCache(new MemoryCacheOptions());
+        var service = CreateService(cache);
 
         bool result = service.ValidateResponseId(userId, "resp_123");
 
@@ -35,7 +36,8 @@ public async Task ValidateResponseId_BlankUserId_Rejects(string? userId)
     [Test]
     public async Task ValidateResponseId_CacheMiss_AllowsGracefulDegradation()
     {
-        var service = CreateService();
+        using var cache = new MemoryCache(new MemoryCacheOptions());
+        var service = CreateService(cache);
         // No RecordResponseId call — simulate server restart / different instance
 
         bool result = service.ValidateResponseId("user1", "resp_unknown");
@@ -46,7 +48,8 @@ public async Task ValidateResponseId_CacheMiss_AllowsGracefulDegradation()
     [Test]
     public async Task ValidateResponseId_RecordedByOwner_Validates()
     {
-        var service = CreateService();
+        using var cache = new MemoryCache(new MemoryCacheOptions());
+        var service = CreateService(cache);
         service.RecordResponseId("user1", "resp_abc");
 
         bool result = service.ValidateResponseId("user1", "resp_abc");
@@ -57,7 +60,8 @@ public async Task ValidateResponseId_RecordedByOwner_Validates()
     [Test]
     public async Task ValidateResponseId_RecordedByDifferentUser_Rejects()
     {
-        var service = CreateService();
+        using var cache = new MemoryCache(new MemoryCacheOptions());
+        var service = CreateService(cache);
         service.RecordResponseId("user1", "resp_abc");
 
         bool result = service.ValidateResponseId("user2", "resp_abc");
@@ -68,7 +72,8 @@ public async Task ValidateResponseId_RecordedByDifferentUser_Rejects()
     [Test]
     public async Task RecordResponseId_NullInputs_DoesNotThrow()
     {
-        var service = CreateService();
+        using var cache = new MemoryCache(new MemoryCacheOptions());
+        var service = CreateService(cache);
 
         service.RecordResponseId(null, "resp_abc");
         service.RecordResponseId("user1", null);
@@ -82,7 +87,8 @@ public async Task RecordResponseId_NullInputs_DoesNotThrow()
     [Test]
     public async Task ValidateResponseId_MultipleResponseIds_EachValidatedIndependently()
     {
-        var service = CreateService();
+        using var cache = new MemoryCache(new MemoryCacheOptions());
+        var service = CreateService(cache);
         service.RecordResponseId("user1", "resp_001");
         service.RecordResponseId("user1", "resp_002");
 
@@ -95,7 +101,8 @@ public async Task ValidateResponseId_MultipleResponseIds_EachValidatedIndependen
     [Test]
     public async Task ValidateResponseId_TwoUsers_IsolatedFromEachOther()
     {
-        var service = CreateService();
+        using var cache = new MemoryCache(new MemoryCacheOptions());
+        var service = CreateService(cache);
         service.RecordResponseId("user1", "resp_A");
         service.RecordResponseId("user2", "resp_B");
 

EssentialCSharp.Web/Controllers/ChatController.cs

@@ -88,7 +88,7 @@ public async Task StreamMessage([FromBody] ChatMessageRequest request, Cancellat
 
         try
         {
-            string? finalResponseId = null;
+            bool responseIdRecorded = false;
 
             await foreach (var (text, responseId) in _AiChatService.GetChatCompletionStream(
                 prompt: request.Message,
@@ -106,18 +106,19 @@ public async Task StreamMessage([FromBody] ChatMessageRequest request, Cancellat
 
                 if (!string.IsNullOrEmpty(responseId))
                 {
-                    finalResponseId = responseId;
+                    // Record ownership as soon as the response ID is first observed —
+                    // if the client disconnects mid-stream the ownership is still cached.
+                    if (!responseIdRecorded)
+                    {
+                        _ResponseIdValidationService.RecordResponseId(userId, responseId);
+                        responseIdRecorded = true;
+                    }
                     var eventData = JsonSerializer.Serialize(new { type = "responseId", data = responseId });
                     await Response.WriteAsync($"data: {eventData}\n\n", cancellationToken);
                     await Response.Body.FlushAsync(cancellationToken);
                 }
             }
 
-            if (finalResponseId is not null)
-            {
-                _ResponseIdValidationService.RecordResponseId(userId, finalResponseId);
-            }
-
             await Response.WriteAsync("data: [DONE]\n\n", cancellationToken);
             await Response.Body.FlushAsync(cancellationToken);
         }

EssentialCSharp.Web/Program.cs

@@ -259,7 +259,7 @@ private static void Main(string[] args)
         // MCP server — always enabled, authenticated via opaque DB-backed tokens.
         builder.Services.AddScoped<McpApiTokenService>();
 
-        builder.Services.AddMemoryCache();
+        builder.Services.AddMemoryCache(options => options.SizeLimit = 10_000);
         builder.Services.AddSingleton<ResponseIdValidationService>();
 
         builder.Services.AddAuthentication()

EssentialCSharp.Web/Services/ResponseIdValidationService.cs

@@ -33,7 +33,10 @@ public void RecordResponseId(string? userId, string? responseId)
         // Create a fresh options instance per call — MemoryCacheEntryOptions has mutable
         // list properties (ExpirationTokens, PostEvictionCallbacks) and sharing a static
         // instance would cause future additions to affect all entries.
-        var entryOptions = new MemoryCacheEntryOptions().SetSlidingExpiration(TimeSpan.FromHours(2));
+        // Size = 1 unit per entry is required when AddMemoryCache sets a SizeLimit.
+        var entryOptions = new MemoryCacheEntryOptions()
+            .SetSlidingExpiration(TimeSpan.FromHours(2))
+            .SetSize(1);
         cache.Set(ResponseKey(responseId), userId, entryOptions);
     }