From ba8a7ef1b4f05dcc9c9727769763e16326ef2f84 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 11:51:21 +0100 Subject: [PATCH 01/28] Add AskAI message feedback endpoint --- .../AskAi/AskAiMessageFeedbackRequest.cs | 23 +++++ .../AskAi/AskAiMessageFeedbackUsecase.cs | 40 +++++++++ .../AskAi/IAskAiMessageFeedbackGateway.cs | 28 ++++++ .../SerializationContext.cs | 1 + .../TelemetryConstants.cs | 6 ++ .../AskAi/AgentBuilderStreamTransformer.cs | 9 +- ...lasticsearchAskAiMessageFeedbackGateway.cs | 89 +++++++++++++++++++ .../MappingsExtensions.cs | 6 ++ .../OpenTelemetry/OpenTelemetryExtensions.cs | 1 + .../ServicesExtension.cs | 5 ++ 10 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs create mode 100644 src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs create mode 100644 src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs create mode 100644 src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs new file mode 100644 index 000000000..bf244b665 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs @@ -0,0 +1,23 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +namespace Elastic.Documentation.Api.Core.AskAi; + +/// +/// Request model for submitting feedback on a specific Ask AI message. +/// +public record AskAiMessageFeedbackRequest( + string MessageId, + string ConversationId, + Reaction Reaction +); + +/// +/// The user's reaction to an Ask AI message. +/// +public enum Reaction +{ + ThumbsUp, + ThumbsDown +} diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs new file mode 100644 index 000000000..0d4302c08 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs @@ -0,0 +1,40 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Diagnostics; +using Microsoft.Extensions.Logging; + +namespace Elastic.Documentation.Api.Core.AskAi; + +/// +/// Use case for handling Ask AI message feedback submissions. +/// +public class AskAiMessageFeedbackUsecase( + IAskAiMessageFeedbackGateway feedbackGateway, + ILogger logger) +{ + private static readonly ActivitySource FeedbackActivitySource = new(TelemetryConstants.AskAiFeedbackSourceName); + + public async Task SubmitFeedback(AskAiMessageFeedbackRequest request, CancellationToken ctx) + { + using var activity = FeedbackActivitySource.StartActivity("ask-ai.message-feedback", ActivityKind.Internal); + _ = activity?.SetTag("gen_ai.conversation.id", request.ConversationId); + _ = activity?.SetTag("gen_ai.response.id", request.MessageId); + _ = activity?.SetTag("feedback.reaction", request.Reaction.ToString().ToLowerInvariant()); + + logger.LogInformation( + "Recording message feedback for message {MessageId} in conversation {ConversationId}: {Reaction}", + request.MessageId, + request.ConversationId, + request.Reaction); + + var record = new AskAiMessageFeedbackRecord( + request.MessageId, + request.ConversationId, + request.Reaction + ); + + await feedbackGateway.RecordFeedbackAsync(record, ctx); + } +} diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs new file mode 100644 index 000000000..de1aeeb9b --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs @@ -0,0 +1,28 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +namespace Elastic.Documentation.Api.Core.AskAi; + +/// +/// Gateway interface for recording Ask AI message feedback. +/// Infrastructure implementations may use different storage backends (Elasticsearch, database, etc.) +/// +public interface IAskAiMessageFeedbackGateway +{ + /// + /// Records feedback for a specific Ask AI message. + /// + /// The feedback record to store. + /// Cancellation token. + Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, CancellationToken ctx); +} + +/// +/// Internal record used to pass message feedback data to the gateway. +/// +public record AskAiMessageFeedbackRecord( + string MessageId, + string ConversationId, + Reaction Reaction +); diff --git a/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs b/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs index 3081c418b..57ba5811e 100644 --- a/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs +++ b/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs @@ -18,6 +18,7 @@ public record InputMessage(string Role, MessagePart[] Parts); public record OutputMessage(string Role, MessagePart[] Parts, string FinishReason); [JsonSerializable(typeof(AskAiRequest))] +[JsonSerializable(typeof(AskAiMessageFeedbackRequest))] [JsonSerializable(typeof(SearchRequest))] [JsonSerializable(typeof(SearchResponse))] [JsonSerializable(typeof(InputMessage))] diff --git a/src/api/Elastic.Documentation.Api.Core/TelemetryConstants.cs b/src/api/Elastic.Documentation.Api.Core/TelemetryConstants.cs index e6d8d4bd8..209ba5045 100644 --- a/src/api/Elastic.Documentation.Api.Core/TelemetryConstants.cs +++ b/src/api/Elastic.Documentation.Api.Core/TelemetryConstants.cs @@ -37,4 +37,10 @@ public static class TelemetryConstants /// Used to trace cache hits, misses, and performance. /// public const string CacheSourceName = "Elastic.Documentation.Api.Cache"; + + /// + /// ActivitySource name for Ask AI feedback operations. + /// Used to trace feedback submissions. + /// + public const string AskAiFeedbackSourceName = "Elastic.Documentation.Api.AskAiFeedback"; } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderStreamTransformer.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderStreamTransformer.cs index fd61fc9d9..9eed7d6e0 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderStreamTransformer.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderStreamTransformer.cs @@ -19,12 +19,12 @@ public class AgentBuilderStreamTransformer(ILogger +/// Records Ask AI message feedback to Elasticsearch. +/// +public class ElasticsearchAskAiMessageFeedbackGateway : IAskAiMessageFeedbackGateway +{ + private readonly ElasticsearchClient _client; + private readonly string _indexName; + private readonly ILogger _logger; + + public ElasticsearchAskAiMessageFeedbackGateway( + ElasticsearchOptions elasticsearchOptions, + AppEnvironment appEnvironment, + ILogger logger) + { + _logger = logger; + _indexName = $"ask-ai-message-feedback-{appEnvironment.Current.ToStringFast(true)}"; + + var nodePool = new SingleNodePool(new Uri(elasticsearchOptions.Url.Trim())); + var clientSettings = new ElasticsearchClientSettings( + nodePool, + sourceSerializer: (_, settings) => new DefaultSourceSerializer(settings, MessageFeedbackJsonContext.Default) + ) + .DefaultIndex(_indexName) + .Authentication(new ApiKey(elasticsearchOptions.ApiKey)); + + _client = new ElasticsearchClient(clientSettings); + } + + public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, CancellationToken ctx) + { + var document = new MessageFeedbackDocument + { + MessageId = record.MessageId, + ConversationId = record.ConversationId, + Reaction = record.Reaction.ToString().ToLowerInvariant(), + Timestamp = DateTimeOffset.UtcNow + }; + + var response = await _client.IndexAsync(document, _indexName, ctx); + + if (!response.IsValidResponse) + { + _logger.LogWarning( + "Failed to index message feedback for message {MessageId}: {Error}", + record.MessageId, + response.ElasticsearchServerError?.Error?.Reason ?? "Unknown error"); + } + else + { + _logger.LogInformation( + "Message feedback recorded: {Reaction} for message {MessageId} in conversation {ConversationId}", + record.Reaction, + record.MessageId, + record.ConversationId); + } + } +} + +internal sealed record MessageFeedbackDocument +{ + [JsonPropertyName("message_id")] + public required string MessageId { get; init; } + + [JsonPropertyName("conversation_id")] + public required string ConversationId { get; init; } + + [JsonPropertyName("reaction")] + public required string Reaction { get; init; } + + [JsonPropertyName("@timestamp")] + public required DateTimeOffset Timestamp { get; init; } +} + +[JsonSerializable(typeof(MessageFeedbackDocument))] +internal sealed partial class MessageFeedbackJsonContext : JsonSerializerContext; diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs index e336f23b2..693566b2f 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs @@ -35,6 +35,12 @@ private static void MapAskAiEndpoint(IEndpointRouteBuilder group) var stream = await askAiUsecase.AskAi(askAiRequest, ctx); await stream.CopyToAsync(context.Response.Body, ctx); }); + + _ = askAiGroup.MapPost("/message-feedback", async (AskAiMessageFeedbackRequest request, AskAiMessageFeedbackUsecase feedbackUsecase, Cancel ctx) => + { + await feedbackUsecase.SubmitFeedback(request, ctx); + return Results.NoContent(); + }); } private static void MapSearchEndpoint(IEndpointRouteBuilder group) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/OpenTelemetry/OpenTelemetryExtensions.cs b/src/api/Elastic.Documentation.Api.Infrastructure/OpenTelemetry/OpenTelemetryExtensions.cs index 26af4ee00..b7a1cf168 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/OpenTelemetry/OpenTelemetryExtensions.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/OpenTelemetry/OpenTelemetryExtensions.cs @@ -37,6 +37,7 @@ public static TracerProviderBuilder AddDocsApiTracing(this TracerProviderBuilder .AddSource(TelemetryConstants.StreamTransformerSourceName) .AddSource(TelemetryConstants.OtlpProxySourceName) .AddSource(TelemetryConstants.CacheSourceName) + .AddSource(TelemetryConstants.AskAiFeedbackSourceName) .AddAspNetCoreInstrumentation(aspNetCoreOptions => { // Don't trace root API endpoint (health check) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs b/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs index 7bcbfa3ad..a9eaa2095 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs @@ -216,6 +216,11 @@ private static void AddAskAiUsecase(IServiceCollection services, AppEnv appEnv) _ = services.AddScoped, AskAiGatewayFactory>(); _ = services.AddScoped(); logger?.LogInformation("Gateway and transformer factories registered successfully - provider switchable via X-AI-Provider header"); + + // Register message feedback components + _ = services.AddScoped(); + _ = services.AddScoped(); + logger?.LogInformation("AskAiMessageFeedbackUsecase and Elasticsearch gateway registered successfully"); } catch (Exception ex) { From 4ce3f48483ee9a274c2040c1c79ce31d164569b9 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 11:57:57 +0100 Subject: [PATCH 02/28] Adjust otel naming --- .../AskAi/AskAiMessageFeedbackUsecase.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs index 0d4302c08..36afcad19 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs @@ -18,10 +18,10 @@ public class AskAiMessageFeedbackUsecase( public async Task SubmitFeedback(AskAiMessageFeedbackRequest request, CancellationToken ctx) { - using var activity = FeedbackActivitySource.StartActivity("ask-ai.message-feedback", ActivityKind.Internal); - _ = activity?.SetTag("gen_ai.conversation.id", request.ConversationId); - _ = activity?.SetTag("gen_ai.response.id", request.MessageId); - _ = activity?.SetTag("feedback.reaction", request.Reaction.ToString().ToLowerInvariant()); + using var activity = FeedbackActivitySource.StartActivity("record message-feedback", ActivityKind.Internal); + _ = activity?.SetTag("gen_ai.conversation.id", request.ConversationId); // correlation with chat traces + _ = activity?.SetTag("ask_ai.message.id", request.MessageId); + _ = activity?.SetTag("ask_ai.feedback.reaction", request.Reaction.ToString().ToLowerInvariant()); logger.LogInformation( "Recording message feedback for message {MessageId} in conversation {ConversationId}: {Reaction}", From 1e52f4590bea41231573270d020cbbcb3668d5e2 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 14:17:00 +0100 Subject: [PATCH 03/28] Add UI onclick handler and other adjustments --- .../SearchOrAskAi/AskAi/ChatMessage.tsx | 114 +++++++++++------- .../SearchOrAskAi/AskAi/useMessageFeedback.ts | 100 +++++++++++++++ .../AskAi/AskAiMessageFeedbackRequest.cs | 6 + .../AskAi/AskAiMessageFeedbackUsecase.cs | 6 +- .../AskAi/IAskAiMessageFeedbackGateway.cs | 3 +- .../SerializationContext.cs | 1 + ...lasticsearchAskAiMessageFeedbackGateway.cs | 22 +++- .../MappingsExtensions.cs | 9 +- 8 files changed, 208 insertions(+), 53 deletions(-) create mode 100644 src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts diff --git a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx index ba2d5af15..ed08ac432 100644 --- a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx @@ -5,7 +5,8 @@ import { ApiError } from '../errorHandling' import { AskAiEvent, ChunkEvent, EventTypes } from './AskAiEvent' import { GeneratingStatus } from './GeneratingStatus' import { References } from './RelatedResources' -import { ChatMessage as ChatMessageType } from './chat.store' +import { ChatMessage as ChatMessageType, useConversationId } from './chat.store' +import { useMessageFeedback, Reaction } from './useMessageFeedback' import { useStatusMinDisplay } from './useStatusMinDisplay' import { EuiButtonIcon, @@ -186,62 +187,86 @@ const computeAiStatus = ( // Action bar for complete AI messages const ActionBar = ({ content, + messageId, onRetry, }: { content: string + messageId: string onRetry?: () => void -}) => ( - - - - - - - - - - - - - - {(copy) => ( +}) => { + const conversationId = useConversationId() + const { selectedReaction, submitFeedback, isPending } = useMessageFeedback( + messageId, + conversationId + ) + + const handleFeedback = (reaction: Reaction) => { + if (!isPending) { + submitFeedback(reaction) + } + } + + return ( + + + handleFeedback('thumbsUp')} /> - )} - - - {onRetry && ( + + - + handleFeedback('thumbsDown')} /> - )} - -) + + + {(copy) => ( + + )} + + + {onRetry && ( + + + + + + )} + + ) +} export const ChatMessage = ({ message, @@ -412,6 +437,7 @@ export const ChatMessage = ({ diff --git a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts new file mode 100644 index 000000000..667d59d47 --- /dev/null +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts @@ -0,0 +1,100 @@ +import { logWarn } from '../../../telemetry/logging' +import { traceSpan } from '../../../telemetry/tracing' +import { useMutation } from '@tanstack/react-query' +import { useState, useCallback } from 'react' + +export type Reaction = 'thumbsUp' | 'thumbsDown' + +interface MessageFeedbackRequest { + messageId: string + conversationId: string + reaction: Reaction +} + +interface UseMessageFeedbackReturn { + selectedReaction: Reaction | null + submitFeedback: (reaction: Reaction) => void + isPending: boolean +} + +const submitFeedbackToApi = async ( + payload: MessageFeedbackRequest +): Promise => { + await traceSpan('submit message-feedback', async (span) => { + span.setAttribute('gen_ai.conversation.id', payload.conversationId) // correlation with backend + span.setAttribute('ask_ai.message.id', payload.messageId) + span.setAttribute('ask_ai.feedback.reaction', payload.reaction) + + const response = await fetch('/docs/_api/v1/ask-ai/message-feedback', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(payload), + }) + + if (!response.ok) { + logWarn('Failed to submit feedback', { + 'http.status_code': response.status, + 'ask_ai.message.id': payload.messageId, + 'ask_ai.feedback.reaction': payload.reaction, + }) + } + }) +} + +export const useMessageFeedback = ( + messageId: string, + conversationId: string | null +): UseMessageFeedbackReturn => { + const [selectedReaction, setSelectedReaction] = useState( + null + ) + + const mutation = useMutation({ + mutationFn: submitFeedbackToApi, + onError: (error) => { + logWarn('Error submitting feedback', { + 'error.message': + error instanceof Error ? error.message : String(error), + }) + // Don't reset selection on error - user intent was clear + }, + }) + + const submitFeedback = useCallback( + (reaction: Reaction) => { + if (!conversationId) { + console.warn('Cannot submit feedback without conversationId') + return + } + + // Ignore if same reaction already selected + if (selectedReaction === reaction) { + return + } + + // Ignore if already submitting + if (mutation.isPending) { + return + } + + // Optimistic update + setSelectedReaction(reaction) + + // Submit to API + mutation.mutate({ + messageId, + conversationId, + reaction, + }) + }, + [messageId, conversationId, selectedReaction, mutation] + ) + + return { + selectedReaction, + submitFeedback, + isPending: mutation.isPending, + } +} diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs index bf244b665..d2d3b816e 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs @@ -2,6 +2,8 @@ // Elasticsearch B.V licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information +using System.Text.Json.Serialization; + namespace Elastic.Documentation.Api.Core.AskAi; /// @@ -16,8 +18,12 @@ Reaction Reaction /// /// The user's reaction to an Ask AI message. /// +[JsonConverter(typeof(JsonStringEnumConverter))] public enum Reaction { + [JsonStringEnumMemberName("thumbsUp")] ThumbsUp, + + [JsonStringEnumMemberName("thumbsDown")] ThumbsDown } diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs index 36afcad19..66957f876 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs @@ -16,12 +16,13 @@ public class AskAiMessageFeedbackUsecase( { private static readonly ActivitySource FeedbackActivitySource = new(TelemetryConstants.AskAiFeedbackSourceName); - public async Task SubmitFeedback(AskAiMessageFeedbackRequest request, CancellationToken ctx) + public async Task SubmitFeedback(AskAiMessageFeedbackRequest request, string? euid, CancellationToken ctx) { using var activity = FeedbackActivitySource.StartActivity("record message-feedback", ActivityKind.Internal); _ = activity?.SetTag("gen_ai.conversation.id", request.ConversationId); // correlation with chat traces _ = activity?.SetTag("ask_ai.message.id", request.MessageId); _ = activity?.SetTag("ask_ai.feedback.reaction", request.Reaction.ToString().ToLowerInvariant()); + // Note: user.euid is automatically added to spans by EuidSpanProcessor logger.LogInformation( "Recording message feedback for message {MessageId} in conversation {ConversationId}: {Reaction}", @@ -32,7 +33,8 @@ public async Task SubmitFeedback(AskAiMessageFeedbackRequest request, Cancellati var record = new AskAiMessageFeedbackRecord( request.MessageId, request.ConversationId, - request.Reaction + request.Reaction, + euid ); await feedbackGateway.RecordFeedbackAsync(record, ctx); diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs index de1aeeb9b..d384a2875 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs @@ -24,5 +24,6 @@ public interface IAskAiMessageFeedbackGateway public record AskAiMessageFeedbackRecord( string MessageId, string ConversationId, - Reaction Reaction + Reaction Reaction, + string? Euid = null ); diff --git a/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs b/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs index 57ba5811e..3b8344bfc 100644 --- a/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs +++ b/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs @@ -19,6 +19,7 @@ public record OutputMessage(string Role, MessagePart[] Parts, string FinishReaso [JsonSerializable(typeof(AskAiRequest))] [JsonSerializable(typeof(AskAiMessageFeedbackRequest))] +[JsonSerializable(typeof(Reaction))] [JsonSerializable(typeof(SearchRequest))] [JsonSerializable(typeof(SearchResponse))] [JsonSerializable(typeof(InputMessage))] diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs index c15dab029..9d9fb6927 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs @@ -42,15 +42,22 @@ public ElasticsearchAskAiMessageFeedbackGateway( public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, CancellationToken ctx) { + var feedbackId = Guid.NewGuid().ToString(); var document = new MessageFeedbackDocument { + FeedbackId = feedbackId, MessageId = record.MessageId, ConversationId = record.ConversationId, Reaction = record.Reaction.ToString().ToLowerInvariant(), + Euid = record.Euid, Timestamp = DateTimeOffset.UtcNow }; - var response = await _client.IndexAsync(document, _indexName, ctx); + _logger.LogDebug("Indexing feedback with ID {FeedbackId} to index {IndexName}", feedbackId, _indexName); + + var response = await _client.IndexAsync(document, idx => idx + .Index(_indexName) + .Id(feedbackId), ctx); if (!response.IsValidResponse) { @@ -62,16 +69,21 @@ public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, Cancell else { _logger.LogInformation( - "Message feedback recorded: {Reaction} for message {MessageId} in conversation {ConversationId}", + "Message feedback recorded: {Reaction} for message {MessageId} in conversation {ConversationId}. ES _id: {EsId}, Index: {Index}", record.Reaction, record.MessageId, - record.ConversationId); + record.ConversationId, + response.Id, + response.Index); } } } internal sealed record MessageFeedbackDocument { + [JsonPropertyName("feedback_id")] + public required string FeedbackId { get; init; } + [JsonPropertyName("message_id")] public required string MessageId { get; init; } @@ -81,6 +93,10 @@ internal sealed record MessageFeedbackDocument [JsonPropertyName("reaction")] public required string Reaction { get; init; } + [JsonPropertyName("euid")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string? Euid { get; init; } + [JsonPropertyName("@timestamp")] public required DateTimeOffset Timestamp { get; init; } } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs index 693566b2f..690e03c80 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs @@ -36,11 +36,14 @@ private static void MapAskAiEndpoint(IEndpointRouteBuilder group) await stream.CopyToAsync(context.Response.Body, ctx); }); - _ = askAiGroup.MapPost("/message-feedback", async (AskAiMessageFeedbackRequest request, AskAiMessageFeedbackUsecase feedbackUsecase, Cancel ctx) => + _ = askAiGroup.MapPost("/message-feedback", async (HttpContext context, AskAiMessageFeedbackRequest request, AskAiMessageFeedbackUsecase feedbackUsecase, Cancel ctx) => { - await feedbackUsecase.SubmitFeedback(request, ctx); + // Extract euid cookie for user tracking + _ = context.Request.Cookies.TryGetValue("euid", out var euid); + + await feedbackUsecase.SubmitFeedback(request, euid, ctx); return Results.NoContent(); - }); + }).DisableAntiforgery(); } private static void MapSearchEndpoint(IEndpointRouteBuilder group) From 264ac880204754b60beb1732d34f5b4cef96c428 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 14:27:44 +0100 Subject: [PATCH 04/28] Add log sanitization --- .../AskAi/AskAiMessageFeedbackUsecase.cs | 4 ++-- .../AskAi/AskAiUsecase.cs | 2 +- .../LogSanitizer.cs | 20 +++++++++++++++++++ .../AskAi/AgentBuilderAskAiGateway.cs | 5 +++-- ...lasticsearchAskAiMessageFeedbackGateway.cs | 7 ++++--- .../Adapters/Search/ElasticsearchGateway.cs | 9 +++++---- 6 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs index 66957f876..bac5d5580 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs @@ -26,8 +26,8 @@ public async Task SubmitFeedback(AskAiMessageFeedbackRequest request, string? eu logger.LogInformation( "Recording message feedback for message {MessageId} in conversation {ConversationId}: {Reaction}", - request.MessageId, - request.ConversationId, + LogSanitizer.Sanitize(request.MessageId), + LogSanitizer.Sanitize(request.ConversationId), request.Reaction); var record = new AskAiMessageFeedbackRecord( diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs index 43555d174..05af6417c 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs @@ -30,7 +30,7 @@ public async Task AskAi(AskAiRequest askAiRequest, Cancel ctx) }; var inputMessagesJson = JsonSerializer.Serialize(inputMessages, ApiJsonContext.Default.InputMessageArray); _ = activity?.SetTag("gen_ai.input.messages", inputMessagesJson); - logger.LogInformation("AskAI input message: {ask_ai.input.message}", askAiRequest.Message); + logger.LogInformation("AskAI input message: {ask_ai.input.message}", LogSanitizer.Sanitize(askAiRequest.Message)); logger.LogInformation("Streaming AskAI response"); var rawStream = await askAiGateway.AskAi(askAiRequest, ctx); // The stream transformer will handle disposing the activity when streaming completes diff --git a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs new file mode 100644 index 000000000..b915a0df7 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -0,0 +1,20 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +namespace Elastic.Documentation.Api.Core; + +/// +/// Utility for sanitizing user input before logging to prevent log forging attacks. +/// +public static class LogSanitizer +{ + /// + /// Sanitizes a string for safe logging by removing newline and carriage return characters. + /// This prevents log forging attacks where malicious input could inject fake log entries. + /// + /// The input string to sanitize. + /// The sanitized string with newlines removed, or empty string if input is null. + public static string Sanitize(string? input) => + input?.Replace("\r", "").Replace("\n", "") ?? string.Empty; +} diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs index 456ec990d..c0e1f821a 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs @@ -7,6 +7,7 @@ using System.Text; using System.Text.Json; using System.Text.Json.Serialization; +using Elastic.Documentation.Api.Core; using Elastic.Documentation.Api.Core.AskAi; using Elastic.Documentation.Api.Infrastructure.Aws; using Microsoft.Extensions.Logging; @@ -32,7 +33,7 @@ public async Task AskAi(AskAiRequest askAiRequest, Cancel ctx = default) askAiRequest.ConversationId); var requestBody = JsonSerializer.Serialize(agentBuilderPayload, AgentBuilderContext.Default.AgentBuilderPayload); - logger.LogInformation("Sending to Agent Builder with conversation_id: {ConversationId}", askAiRequest.ConversationId ?? "(null - first request)"); + logger.LogInformation("Sending to Agent Builder with conversation_id: {ConversationId}", LogSanitizer.Sanitize(askAiRequest.ConversationId) ?? "(null - first request)"); var kibanaUrl = await parameterProvider.GetParam("docs-kibana-url", false, ctx); var kibanaApiKey = await parameterProvider.GetParam("docs-kibana-apikey", true, ctx); @@ -48,7 +49,7 @@ public async Task AskAi(AskAiRequest askAiRequest, Cancel ctx = default) // Ensure the response is successful before streaming if (!response.IsSuccessStatusCode) { - logger.LogInformation("Body: {Body}", requestBody); + logger.LogInformation("Body: {Body}", LogSanitizer.Sanitize(requestBody)); var errorContent = await response.Content.ReadAsStringAsync(ctx); logger.LogInformation("Reason: {Reason}", response.ReasonPhrase); throw new HttpRequestException($"Agent Builder returned {response.StatusCode}: {errorContent}"); diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs index 9d9fb6927..8e641ef68 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs @@ -5,6 +5,7 @@ using System.Text.Json.Serialization; using Elastic.Clients.Elasticsearch; using Elastic.Clients.Elasticsearch.Serialization; +using Elastic.Documentation.Api.Core; using Elastic.Documentation.Api.Core.AskAi; using Elastic.Documentation.Api.Infrastructure.Adapters.Search; using Elastic.Transport; @@ -63,7 +64,7 @@ public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, Cancell { _logger.LogWarning( "Failed to index message feedback for message {MessageId}: {Error}", - record.MessageId, + LogSanitizer.Sanitize(record.MessageId), response.ElasticsearchServerError?.Error?.Reason ?? "Unknown error"); } else @@ -71,8 +72,8 @@ public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, Cancell _logger.LogInformation( "Message feedback recorded: {Reaction} for message {MessageId} in conversation {ConversationId}. ES _id: {EsId}, Index: {Index}", record.Reaction, - record.MessageId, - record.ConversationId, + LogSanitizer.Sanitize(record.MessageId), + LogSanitizer.Sanitize(record.ConversationId), response.Id, response.Index); } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs index 0f420ab09..6e59b1975 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs @@ -7,6 +7,7 @@ using Elastic.Clients.Elasticsearch.Core.Search; using Elastic.Clients.Elasticsearch.QueryDsl; using Elastic.Clients.Elasticsearch.Serialization; +using Elastic.Documentation.Api.Core; using Elastic.Documentation.Api.Core.Search; using Elastic.Documentation.AppliesTo; using Elastic.Transport; @@ -171,7 +172,7 @@ private static Query BuildFilter() => !(Query)new TermsQuery(Infer.Field Results)> HybridSearchWithRrfAsync(string query, int pageNumber, int pageSize, Cancel ctx = default) { - _logger.LogInformation("Starting RRF hybrid search for '{Query}' with pageNumber={PageNumber}, pageSize={PageSize}", query, pageNumber, pageSize); + _logger.LogInformation("Starting RRF hybrid search for '{Query}' with pageNumber={PageNumber}, pageSize={PageSize}", LogSanitizer.Sanitize(query), pageNumber, pageSize); const string preTag = ""; const string postTag = ""; @@ -256,13 +257,13 @@ private static Query BuildFilter() => !(Query)new TermsQuery(Infer.Field ExplainDocumentAsync(string query, string docum } catch (Exception ex) { - _logger.LogError(ex, "Error explaining document '{Url}' for query '{Query}'", documentUrl, query); + _logger.LogError(ex, "Error explaining document '{Url}' for query '{Query}'", LogSanitizer.Sanitize(documentUrl), LogSanitizer.Sanitize(query)); return new ExplainResult { SearchTitle = "N/A", From 2d08faa15d82c4d02933bb773cd6e67d07b91de2 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 14:34:27 +0100 Subject: [PATCH 05/28] Use spans and create tests --- .../LogSanitizer.cs | 32 ++++- .../LogSanitizerTests.cs | 136 ++++++++++++++++++ 2 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs diff --git a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs index b915a0df7..6ef4d5f27 100644 --- a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -12,9 +12,37 @@ public static class LogSanitizer /// /// Sanitizes a string for safe logging by removing newline and carriage return characters. /// This prevents log forging attacks where malicious input could inject fake log entries. + /// Uses span-based operations for optimal performance. /// /// The input string to sanitize. /// The sanitized string with newlines removed, or empty string if input is null. - public static string Sanitize(string? input) => - input?.Replace("\r", "").Replace("\n", "") ?? string.Empty; + public static string Sanitize(string? input) + { + if (string.IsNullOrEmpty(input)) + return string.Empty; + + var span = input.AsSpan(); + + // Fast path: no sanitization needed (common case) - zero allocations + if (!span.ContainsAny('\r', '\n')) + return input; + + // Slow path: count chars to keep, then create string with exact size + var keepCount = 0; + foreach (var c in span) + { + if (c is not '\r' and not '\n') + keepCount++; + } + + return string.Create(keepCount, input, static (dest, src) => + { + var destIndex = 0; + foreach (var c in src) + { + if (c is not '\r' and not '\n') + dest[destIndex++] = c; + } + }); + } } diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs new file mode 100644 index 000000000..976bfc086 --- /dev/null +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs @@ -0,0 +1,136 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using Elastic.Documentation.Api.Core; +using FluentAssertions; + +namespace Elastic.Documentation.Api.Infrastructure.Tests; + +public class LogSanitizerTests +{ + [Fact] + public void SanitizeWhenInputIsNullReturnsEmptyString() + { + // Act + var result = LogSanitizer.Sanitize(null); + + // Assert + result.Should().BeEmpty(); + } + + [Fact] + public void SanitizeWhenInputIsEmptyReturnsEmptyString() + { + // Act + var result = LogSanitizer.Sanitize(string.Empty); + + // Assert + result.Should().BeEmpty(); + } + + [Fact] + public void SanitizeWhenInputHasNoNewlinesReturnsSameString() + { + // Arrange + const string input = "Hello World"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().BeSameAs(input); // Same reference - no allocation + } + + [Fact] + public void SanitizeWhenInputHasCarriageReturnRemovesIt() + { + // Arrange + const string input = "Hello\rWorld"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("HelloWorld"); + } + + [Fact] + public void SanitizeWhenInputHasNewlineRemovesIt() + { + // Arrange + const string input = "Hello\nWorld"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("HelloWorld"); + } + + [Fact] + public void SanitizeWhenInputHasCrlfRemovesBoth() + { + // Arrange + const string input = "Hello\r\nWorld"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("HelloWorld"); + } + + [Fact] + public void SanitizeWhenInputHasMultipleNewlinesRemovesAll() + { + // Arrange + const string input = "Line1\nLine2\r\nLine3\rLine4"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("Line1Line2Line3Line4"); + } + + [Fact] + public void SanitizeWhenInputIsOnlyNewlinesReturnsEmptyString() + { + // Arrange + const string input = "\r\n\r\n"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().BeEmpty(); + } + + [Fact] + public void SanitizePreservesOtherSpecialCharacters() + { + // Arrange + const string input = "Hello\tWorld! @#$%^&*()"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be(input); + } + + [Theory] + [InlineData("fake-log-entry\nINFO: Injected message", "fake-log-entryINFO: Injected message")] + [InlineData("user-id-123\r\nERROR: Attack!", "user-id-123ERROR: Attack!")] + public void SanitizePreventsLogForging(string maliciousInput, string expected) + { + // Act + var result = LogSanitizer.Sanitize(maliciousInput); + + // Assert + result.Should().Be(expected); + result.Should().NotContain("\n"); + result.Should().NotContain("\r"); + } +} From 25eb256cf70f912811e05c36bb36a85ad2d24e7e Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 14:49:42 +0100 Subject: [PATCH 06/28] Remove all control characters as suggested by CodeQL --- .../LogSanitizer.cs | 30 +++++-- .../LogSanitizerTests.cs | 83 ++++++++++++++++--- 2 files changed, 93 insertions(+), 20 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs index 6ef4d5f27..5b8002664 100644 --- a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -10,12 +10,13 @@ namespace Elastic.Documentation.Api.Core; public static class LogSanitizer { /// - /// Sanitizes a string for safe logging by removing newline and carriage return characters. - /// This prevents log forging attacks where malicious input could inject fake log entries. + /// Sanitizes a string for safe logging by removing all ASCII control characters (0x00-0x1F). + /// This prevents log forging attacks where malicious input could inject fake log entries + /// via newlines, tabs, escape sequences, or other control characters. /// Uses span-based operations for optimal performance. /// /// The input string to sanitize. - /// The sanitized string with newlines removed, or empty string if input is null. + /// The sanitized string with control characters removed, or empty string if input is null. public static string Sanitize(string? input) { if (string.IsNullOrEmpty(input)) @@ -23,15 +24,25 @@ public static string Sanitize(string? input) var span = input.AsSpan(); - // Fast path: no sanitization needed (common case) - zero allocations - if (!span.ContainsAny('\r', '\n')) + // Fast path: check if any control characters exist (common case has none) - zero allocations + var hasControlChars = false; + foreach (var c in span) + { + if (IsControlChar(c)) + { + hasControlChars = true; + break; + } + } + + if (!hasControlChars) return input; // Slow path: count chars to keep, then create string with exact size var keepCount = 0; foreach (var c in span) { - if (c is not '\r' and not '\n') + if (!IsControlChar(c)) keepCount++; } @@ -40,9 +51,14 @@ public static string Sanitize(string? input) var destIndex = 0; foreach (var c in src) { - if (c is not '\r' and not '\n') + if (!IsControlChar(c)) dest[destIndex++] = c; } }); } + + /// + /// Checks if a character is an ASCII control character (0x00-0x1F). + /// + private static bool IsControlChar(char c) => c <= '\x1F'; } diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs index 976bfc086..5b0489b73 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs @@ -30,10 +30,10 @@ public void SanitizeWhenInputIsEmptyReturnsEmptyString() } [Fact] - public void SanitizeWhenInputHasNoNewlinesReturnsSameString() + public void SanitizeWhenInputHasNoControlCharsReturnsSameString() { // Arrange - const string input = "Hello World"; + const string input = "Hello World! @#$%^&*()"; // Act var result = LogSanitizer.Sanitize(input); @@ -68,6 +68,19 @@ public void SanitizeWhenInputHasNewlineRemovesIt() result.Should().Be("HelloWorld"); } + [Fact] + public void SanitizeWhenInputHasTabRemovesIt() + { + // Arrange + const string input = "Hello\tWorld"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("HelloWorld"); + } + [Fact] public void SanitizeWhenInputHasCrlfRemovesBoth() { @@ -82,23 +95,23 @@ public void SanitizeWhenInputHasCrlfRemovesBoth() } [Fact] - public void SanitizeWhenInputHasMultipleNewlinesRemovesAll() + public void SanitizeWhenInputHasMultipleControlCharsRemovesAll() { - // Arrange - const string input = "Line1\nLine2\r\nLine3\rLine4"; + // Arrange - includes \n, \r, \t, and escape char \x1B + const string input = "Line1\nLine2\r\nLine3\tLine4\x1BLine5"; // Act var result = LogSanitizer.Sanitize(input); // Assert - result.Should().Be("Line1Line2Line3Line4"); + result.Should().Be("Line1Line2Line3Line4Line5"); } [Fact] - public void SanitizeWhenInputIsOnlyNewlinesReturnsEmptyString() + public void SanitizeWhenInputIsOnlyControlCharsReturnsEmptyString() { // Arrange - const string input = "\r\n\r\n"; + const string input = "\r\n\t\x00\x1F"; // Act var result = LogSanitizer.Sanitize(input); @@ -108,10 +121,10 @@ public void SanitizeWhenInputIsOnlyNewlinesReturnsEmptyString() } [Fact] - public void SanitizePreservesOtherSpecialCharacters() + public void SanitizePreservesSpaceAndPrintableChars() { - // Arrange - const string input = "Hello\tWorld! @#$%^&*()"; + // Arrange - space (0x20) is NOT a control char + const string input = "Hello World! @#$%^&*() 123"; // Act var result = LogSanitizer.Sanitize(input); @@ -120,9 +133,37 @@ public void SanitizePreservesOtherSpecialCharacters() result.Should().Be(input); } + [Fact] + public void SanitizeRemovesNullChar() + { + // Arrange + const string input = "Hello\x00World"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("HelloWorld"); + } + + [Fact] + public void SanitizeRemovesEscapeSequence() + { + // Arrange - \x1B is ESC, commonly used in ANSI escape sequences + const string input = "Hello\x1B[31mRed\x1B[0mWorld"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("Hello[31mRed[0mWorld"); + } + [Theory] [InlineData("fake-log-entry\nINFO: Injected message", "fake-log-entryINFO: Injected message")] [InlineData("user-id-123\r\nERROR: Attack!", "user-id-123ERROR: Attack!")] + [InlineData("input\twith\ttabs", "inputwithtabs")] + [InlineData("escape\x1B[0msequence", "escape[0msequence")] public void SanitizePreventsLogForging(string maliciousInput, string expected) { // Act @@ -130,7 +171,23 @@ public void SanitizePreventsLogForging(string maliciousInput, string expected) // Assert result.Should().Be(expected); - result.Should().NotContain("\n"); - result.Should().NotContain("\r"); + } + + [Fact] + public void SanitizeRemovesAllAsciiControlChars() + { + // Arrange - all ASCII control chars 0x00-0x1F + var allControlChars = string.Create(32, 0, static (span, _) => + { + for (var i = 0; i < 32; i++) + span[i] = (char)i; + }); + var input = $"Before{allControlChars}After"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("BeforeAfter"); } } From 7926de7e134a35eb7dec92f5edd8205be1d0c2df Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 15:05:07 +0100 Subject: [PATCH 07/28] Validate input --- .../AskAi/AskAiMessageFeedbackUsecase.cs | 5 +++-- .../AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs | 7 ++++--- .../MappingsExtensions.cs | 7 +++++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs index bac5d5580..29ce73cb2 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs @@ -24,10 +24,11 @@ public async Task SubmitFeedback(AskAiMessageFeedbackRequest request, string? eu _ = activity?.SetTag("ask_ai.feedback.reaction", request.Reaction.ToString().ToLowerInvariant()); // Note: user.euid is automatically added to spans by EuidSpanProcessor + // MessageId and ConversationId are validated as UUIDs at the endpoint, so no sanitization needed logger.LogInformation( "Recording message feedback for message {MessageId} in conversation {ConversationId}: {Reaction}", - LogSanitizer.Sanitize(request.MessageId), - LogSanitizer.Sanitize(request.ConversationId), + request.MessageId, + request.ConversationId, request.Reaction); var record = new AskAiMessageFeedbackRecord( diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs index 8e641ef68..ca79d3d32 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs @@ -60,11 +60,12 @@ public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, Cancell .Index(_indexName) .Id(feedbackId), ctx); + // MessageId and ConversationId are validated as UUIDs at the endpoint, so no sanitization needed if (!response.IsValidResponse) { _logger.LogWarning( "Failed to index message feedback for message {MessageId}: {Error}", - LogSanitizer.Sanitize(record.MessageId), + record.MessageId, response.ElasticsearchServerError?.Error?.Reason ?? "Unknown error"); } else @@ -72,8 +73,8 @@ public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, Cancell _logger.LogInformation( "Message feedback recorded: {Reaction} for message {MessageId} in conversation {ConversationId}. ES _id: {EsId}, Index: {Index}", record.Reaction, - LogSanitizer.Sanitize(record.MessageId), - LogSanitizer.Sanitize(record.ConversationId), + record.MessageId, + record.ConversationId, response.Id, response.Index); } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs index 690e03c80..543da8be0 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs @@ -38,6 +38,13 @@ private static void MapAskAiEndpoint(IEndpointRouteBuilder group) _ = askAiGroup.MapPost("/message-feedback", async (HttpContext context, AskAiMessageFeedbackRequest request, AskAiMessageFeedbackUsecase feedbackUsecase, Cancel ctx) => { + // Validate UUIDs to prevent malformed input + if (!Guid.TryParse(request.MessageId, out _)) + return Results.BadRequest("Invalid MessageId format. Expected UUID."); + + if (!Guid.TryParse(request.ConversationId, out _)) + return Results.BadRequest("Invalid ConversationId format. Expected UUID."); + // Extract euid cookie for user tracking _ = context.Request.Cookies.TryGetValue("euid", out var euid); From 4df1e5ca87de17af9a32619cbe96941414691a61 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 15:11:03 +0100 Subject: [PATCH 08/28] Refactor --- .../SearchOrAskAi/AskAi/useMessageFeedback.ts | 4 +++- .../AskAi/AskAiMessageFeedbackRequest.cs | 5 +++-- .../AskAi/AskAiMessageFeedbackUsecase.cs | 2 +- .../AskAi/IAskAiMessageFeedbackGateway.cs | 4 ++-- .../ElasticsearchAskAiMessageFeedbackGateway.cs | 12 ++++++------ .../MappingsExtensions.cs | 8 +------- 6 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts index 667d59d47..5cf308ee4 100644 --- a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts @@ -65,7 +65,9 @@ export const useMessageFeedback = ( const submitFeedback = useCallback( (reaction: Reaction) => { if (!conversationId) { - console.warn('Cannot submit feedback without conversationId') + logWarn('Cannot submit feedback without conversationId', { + 'ask_ai.message.id': messageId, + }) return } diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs index d2d3b816e..127159ca0 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs @@ -8,10 +8,11 @@ namespace Elastic.Documentation.Api.Core.AskAi; /// /// Request model for submitting feedback on a specific Ask AI message. +/// Using Guid type ensures automatic validation during JSON deserialization. /// public record AskAiMessageFeedbackRequest( - string MessageId, - string ConversationId, + Guid MessageId, + Guid ConversationId, Reaction Reaction ); diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs index 29ce73cb2..82235e392 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs @@ -24,7 +24,7 @@ public async Task SubmitFeedback(AskAiMessageFeedbackRequest request, string? eu _ = activity?.SetTag("ask_ai.feedback.reaction", request.Reaction.ToString().ToLowerInvariant()); // Note: user.euid is automatically added to spans by EuidSpanProcessor - // MessageId and ConversationId are validated as UUIDs at the endpoint, so no sanitization needed + // MessageId and ConversationId are Guid types, so no sanitization needed logger.LogInformation( "Recording message feedback for message {MessageId} in conversation {ConversationId}: {Reaction}", request.MessageId, diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs index d384a2875..e3c52da60 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs @@ -22,8 +22,8 @@ public interface IAskAiMessageFeedbackGateway /// Internal record used to pass message feedback data to the gateway. /// public record AskAiMessageFeedbackRecord( - string MessageId, - string ConversationId, + Guid MessageId, + Guid ConversationId, Reaction Reaction, string? Euid = null ); diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs index ca79d3d32..a75d2fb97 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs @@ -43,12 +43,12 @@ public ElasticsearchAskAiMessageFeedbackGateway( public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, CancellationToken ctx) { - var feedbackId = Guid.NewGuid().ToString(); + var feedbackId = Guid.NewGuid(); var document = new MessageFeedbackDocument { - FeedbackId = feedbackId, - MessageId = record.MessageId, - ConversationId = record.ConversationId, + FeedbackId = feedbackId.ToString(), + MessageId = record.MessageId.ToString(), + ConversationId = record.ConversationId.ToString(), Reaction = record.Reaction.ToString().ToLowerInvariant(), Euid = record.Euid, Timestamp = DateTimeOffset.UtcNow @@ -58,9 +58,9 @@ public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, Cancell var response = await _client.IndexAsync(document, idx => idx .Index(_indexName) - .Id(feedbackId), ctx); + .Id(feedbackId.ToString()), ctx); - // MessageId and ConversationId are validated as UUIDs at the endpoint, so no sanitization needed + // MessageId and ConversationId are Guid types, so no sanitization needed if (!response.IsValidResponse) { _logger.LogWarning( diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs index 543da8be0..c74c29fee 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs @@ -36,15 +36,9 @@ private static void MapAskAiEndpoint(IEndpointRouteBuilder group) await stream.CopyToAsync(context.Response.Body, ctx); }); + // UUID validation is automatic via Guid type deserialization (returns 400 if invalid) _ = askAiGroup.MapPost("/message-feedback", async (HttpContext context, AskAiMessageFeedbackRequest request, AskAiMessageFeedbackUsecase feedbackUsecase, Cancel ctx) => { - // Validate UUIDs to prevent malformed input - if (!Guid.TryParse(request.MessageId, out _)) - return Results.BadRequest("Invalid MessageId format. Expected UUID."); - - if (!Guid.TryParse(request.ConversationId, out _)) - return Results.BadRequest("Invalid ConversationId format. Expected UUID."); - // Extract euid cookie for user tracking _ = context.Request.Cookies.TryGetValue("euid", out var euid); From d0883cf8b7a7b4b7b60366e2b231b7af1fa9253d Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 15:15:16 +0100 Subject: [PATCH 09/28] Persist reaction selection --- .../SearchOrAskAi/AskAi/chat.store.ts | 17 +++++++++++++- .../SearchOrAskAi/AskAi/useMessageFeedback.ts | 22 ++++++++++++------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/chat.store.ts b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/chat.store.ts index 1296bd511..3ddc4e119 100644 --- a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/chat.store.ts +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/chat.store.ts @@ -3,6 +3,7 @@ import { v4 as uuidv4 } from 'uuid' import { create } from 'zustand/react' export type AiProvider = 'AgentBuilder' | 'LlmGateway' +export type Reaction = 'thumbsUp' | 'thumbsDown' export interface ChatMessage { id: string @@ -22,6 +23,7 @@ interface ChatState { chatMessages: ChatMessage[] conversationId: string | null aiProvider: AiProvider + messageFeedback: Record // messageId -> reaction actions: { submitQuestion: (question: string) => void updateAiMessage: ( @@ -37,6 +39,7 @@ interface ChatState { hasMessageBeenSent: (id: string) => boolean markMessageAsSent: (id: string) => void cancelStreaming: () => void + setMessageFeedback: (messageId: string, reaction: Reaction) => void } } @@ -44,6 +47,7 @@ export const chatStore = create((set) => ({ chatMessages: [], conversationId: null, // Start with null - will be set by backend on first request aiProvider: 'LlmGateway', // Default to LLM Gateway + messageFeedback: {}, actions: { submitQuestion: (question: string) => { set((state) => { @@ -98,7 +102,7 @@ export const chatStore = create((set) => ({ clearChat: () => { sentAiMessageIds.clear() - set({ chatMessages: [], conversationId: null }) + set({ chatMessages: [], conversationId: null, messageFeedback: {} }) }, clearNon429Errors: () => { @@ -136,6 +140,15 @@ export const chatStore = create((set) => ({ ), })) }, + + setMessageFeedback: (messageId: string, reaction: Reaction) => { + set((state) => ({ + messageFeedback: { + ...state.messageFeedback, + [messageId]: reaction, + }, + })) + }, }, })) @@ -144,3 +157,5 @@ export const useConversationId = () => chatStore((state) => state.conversationId) export const useAiProvider = () => chatStore((state) => state.aiProvider) export const useChatActions = () => chatStore((state) => state.actions) +export const useMessageReaction = (messageId: string) => + chatStore((state) => state.messageFeedback[messageId] ?? null) diff --git a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts index 5cf308ee4..a2b890cac 100644 --- a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts @@ -1,9 +1,10 @@ import { logWarn } from '../../../telemetry/logging' import { traceSpan } from '../../../telemetry/tracing' +import { Reaction, useChatActions, useMessageReaction } from './chat.store' import { useMutation } from '@tanstack/react-query' -import { useState, useCallback } from 'react' +import { useCallback } from 'react' -export type Reaction = 'thumbsUp' | 'thumbsDown' +export type { Reaction } from './chat.store' interface MessageFeedbackRequest { messageId: string @@ -47,9 +48,8 @@ export const useMessageFeedback = ( messageId: string, conversationId: string | null ): UseMessageFeedbackReturn => { - const [selectedReaction, setSelectedReaction] = useState( - null - ) + const selectedReaction = useMessageReaction(messageId) + const { setMessageFeedback } = useChatActions() const mutation = useMutation({ mutationFn: submitFeedbackToApi, @@ -81,8 +81,8 @@ export const useMessageFeedback = ( return } - // Optimistic update - setSelectedReaction(reaction) + // Optimistic update - stored in Zustand so it persists across tab switches + setMessageFeedback(messageId, reaction) // Submit to API mutation.mutate({ @@ -91,7 +91,13 @@ export const useMessageFeedback = ( reaction, }) }, - [messageId, conversationId, selectedReaction, mutation] + [ + messageId, + conversationId, + selectedReaction, + mutation, + setMessageFeedback, + ] ) return { From 3dadebf240bab4ed6e80065f13af31216ed89ad9 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 15:21:33 +0100 Subject: [PATCH 10/28] Add debounce if user quickly changes selection --- .../SearchOrAskAi/AskAi/ChatMessage.tsx | 14 +++----- .../SearchOrAskAi/AskAi/useMessageFeedback.ts | 33 +++++++++++-------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx index ed08ac432..7954a2cdf 100644 --- a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx @@ -6,7 +6,7 @@ import { AskAiEvent, ChunkEvent, EventTypes } from './AskAiEvent' import { GeneratingStatus } from './GeneratingStatus' import { References } from './RelatedResources' import { ChatMessage as ChatMessageType, useConversationId } from './chat.store' -import { useMessageFeedback, Reaction } from './useMessageFeedback' +import { useMessageFeedback } from './useMessageFeedback' import { useStatusMinDisplay } from './useStatusMinDisplay' import { EuiButtonIcon, @@ -195,17 +195,11 @@ const ActionBar = ({ onRetry?: () => void }) => { const conversationId = useConversationId() - const { selectedReaction, submitFeedback, isPending } = useMessageFeedback( + const { selectedReaction, submitFeedback } = useMessageFeedback( messageId, conversationId ) - const handleFeedback = (reaction: Reaction) => { - if (!isPending) { - submitFeedback(reaction) - } - } - return ( @@ -218,7 +212,7 @@ const ActionBar = ({ display={ selectedReaction === 'thumbsUp' ? 'base' : 'empty' } - onClick={() => handleFeedback('thumbsUp')} + onClick={() => submitFeedback('thumbsUp')} /> @@ -232,7 +226,7 @@ const ActionBar = ({ display={ selectedReaction === 'thumbsDown' ? 'base' : 'empty' } - onClick={() => handleFeedback('thumbsDown')} + onClick={() => submitFeedback('thumbsDown')} /> diff --git a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts index a2b890cac..076b2fb8b 100644 --- a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts @@ -2,10 +2,12 @@ import { logWarn } from '../../../telemetry/logging' import { traceSpan } from '../../../telemetry/tracing' import { Reaction, useChatActions, useMessageReaction } from './chat.store' import { useMutation } from '@tanstack/react-query' -import { useCallback } from 'react' +import { useCallback, useRef } from 'react' export type { Reaction } from './chat.store' +const DEBOUNCE_MS = 500 + interface MessageFeedbackRequest { messageId: string conversationId: string @@ -15,7 +17,6 @@ interface MessageFeedbackRequest { interface UseMessageFeedbackReturn { selectedReaction: Reaction | null submitFeedback: (reaction: Reaction) => void - isPending: boolean } const submitFeedbackToApi = async ( @@ -50,6 +51,9 @@ export const useMessageFeedback = ( ): UseMessageFeedbackReturn => { const selectedReaction = useMessageReaction(messageId) const { setMessageFeedback } = useChatActions() + const debounceTimeoutRef = useRef | null>( + null + ) const mutation = useMutation({ mutationFn: submitFeedbackToApi, @@ -76,20 +80,22 @@ export const useMessageFeedback = ( return } - // Ignore if already submitting - if (mutation.isPending) { - return - } - // Optimistic update - stored in Zustand so it persists across tab switches setMessageFeedback(messageId, reaction) - // Submit to API - mutation.mutate({ - messageId, - conversationId, - reaction, - }) + // Cancel any pending debounced submission + if (debounceTimeoutRef.current) { + clearTimeout(debounceTimeoutRef.current) + } + + // Debounce the API call - only send the final selection + debounceTimeoutRef.current = setTimeout(() => { + mutation.mutate({ + messageId, + conversationId, + reaction, + }) + }, DEBOUNCE_MS) }, [ messageId, @@ -103,6 +109,5 @@ export const useMessageFeedback = ( return { selectedReaction, submitFeedback, - isPending: mutation.isPending, } } From b48be34c2db789ca88038ea9b2ac913fe80c1222 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 15:26:04 +0100 Subject: [PATCH 11/28] Fix other CodeQL errors regarind log sanitization --- .../AskAi/AskAiUsecase.cs | 2 +- .../LogSanitizer.cs | 31 +++++++++----- .../LogSanitizerTests.cs | 41 +++++++++++++++++++ 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs index 05af6417c..e0cd8227d 100644 --- a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs @@ -30,7 +30,7 @@ public async Task AskAi(AskAiRequest askAiRequest, Cancel ctx) }; var inputMessagesJson = JsonSerializer.Serialize(inputMessages, ApiJsonContext.Default.InputMessageArray); _ = activity?.SetTag("gen_ai.input.messages", inputMessagesJson); - logger.LogInformation("AskAI input message: {ask_ai.input.message}", LogSanitizer.Sanitize(askAiRequest.Message)); + logger.LogInformation("AskAI input message: <{ask_ai.input.message}>", LogSanitizer.Sanitize(askAiRequest.Message)); logger.LogInformation("Streaming AskAI response"); var rawStream = await askAiGateway.AskAi(askAiRequest, ctx); // The stream transformer will handle disposing the activity when streaming completes diff --git a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs index 5b8002664..d6c6678dc 100644 --- a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -10,11 +10,18 @@ namespace Elastic.Documentation.Api.Core; public static class LogSanitizer { /// - /// Sanitizes a string for safe logging by removing all ASCII control characters (0x00-0x1F). + /// Sanitizes a string for safe logging by removing dangerous control and separator characters. /// This prevents log forging attacks where malicious input could inject fake log entries /// via newlines, tabs, escape sequences, or other control characters. /// Uses span-based operations for optimal performance. /// + /// + /// Removes: + /// - ASCII control characters (0x00-0x1F) + /// - DEL character (0x7F) + /// - Unicode line separator (U+2028) + /// - Unicode paragraph separator (U+2029) + /// /// The input string to sanitize. /// The sanitized string with control characters removed, or empty string if input is null. public static string Sanitize(string? input) @@ -24,25 +31,25 @@ public static string Sanitize(string? input) var span = input.AsSpan(); - // Fast path: check if any control characters exist (common case has none) - zero allocations - var hasControlChars = false; + // Fast path: check if any dangerous characters exist (common case has none) - zero allocations + var hasDangerousChars = false; foreach (var c in span) { - if (IsControlChar(c)) + if (IsDangerousChar(c)) { - hasControlChars = true; + hasDangerousChars = true; break; } } - if (!hasControlChars) + if (!hasDangerousChars) return input; // Slow path: count chars to keep, then create string with exact size var keepCount = 0; foreach (var c in span) { - if (!IsControlChar(c)) + if (!IsDangerousChar(c)) keepCount++; } @@ -51,14 +58,18 @@ public static string Sanitize(string? input) var destIndex = 0; foreach (var c in src) { - if (!IsControlChar(c)) + if (!IsDangerousChar(c)) dest[destIndex++] = c; } }); } /// - /// Checks if a character is an ASCII control character (0x00-0x1F). + /// Checks if a character is dangerous for logging (could enable log forging). /// - private static bool IsControlChar(char c) => c <= '\x1F'; + private static bool IsDangerousChar(char c) => + c <= '\x1F' || // ASCII control characters (0x00-0x1F) + c == '\x7F' || // DEL character + c == '\u2028' || // Unicode line separator + c == '\u2029'; // Unicode paragraph separator } diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs index 5b0489b73..4ca3d6992 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs @@ -120,6 +120,45 @@ public void SanitizeWhenInputIsOnlyControlCharsReturnsEmptyString() result.Should().BeEmpty(); } + [Fact] + public void SanitizeRemovesDelChar() + { + // Arrange - DEL is 0x7F + const string input = "Hello\x7FWorld"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("HelloWorld"); + } + + [Fact] + public void SanitizeRemovesUnicodeLineSeparator() + { + // Arrange - Unicode line separator U+2028 + const string input = "Hello\u2028World"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("HelloWorld"); + } + + [Fact] + public void SanitizeRemovesUnicodeParagraphSeparator() + { + // Arrange - Unicode paragraph separator U+2029 + const string input = "Hello\u2029World"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().Be("HelloWorld"); + } + [Fact] public void SanitizePreservesSpaceAndPrintableChars() { @@ -164,6 +203,8 @@ public void SanitizeRemovesEscapeSequence() [InlineData("user-id-123\r\nERROR: Attack!", "user-id-123ERROR: Attack!")] [InlineData("input\twith\ttabs", "inputwithtabs")] [InlineData("escape\x1B[0msequence", "escape[0msequence")] + [InlineData("unicode\u2028line\u2029separators", "unicodelineseparators")] + [InlineData("del\x7Fchar", "delchar")] public void SanitizePreventsLogForging(string maliciousInput, string expected) { // Act From c2b7c3cd1e396d8dbb93db64fb2d749d6e1efd12 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 15:31:20 +0100 Subject: [PATCH 12/28] Fix formatting --- src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs index d6c6678dc..0b82c3e3c 100644 --- a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -68,8 +68,8 @@ public static string Sanitize(string? input) /// Checks if a character is dangerous for logging (could enable log forging). /// private static bool IsDangerousChar(char c) => - c <= '\x1F' || // ASCII control characters (0x00-0x1F) - c == '\x7F' || // DEL character - c == '\u2028' || // Unicode line separator - c == '\u2029'; // Unicode paragraph separator + c is <= '\x1F' or // ASCII control characters (0x00-0x1F) + '\x7F' or // DEL character + '\u2028' or // Unicode line separator + '\u2029'; // Unicode paragraph separator } From 79e4117916c3772f345df654b082964662ab2baa Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 16:20:34 +0100 Subject: [PATCH 13/28] Fix test --- .../LogSanitizerTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs index 4ca3d6992..3be9601d7 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs @@ -124,7 +124,7 @@ public void SanitizeWhenInputIsOnlyControlCharsReturnsEmptyString() public void SanitizeRemovesDelChar() { // Arrange - DEL is 0x7F - const string input = "Hello\x7FWorld"; + const string input = "Hello\u007FWorld"; // Act var result = LogSanitizer.Sanitize(input); @@ -204,7 +204,7 @@ public void SanitizeRemovesEscapeSequence() [InlineData("input\twith\ttabs", "inputwithtabs")] [InlineData("escape\x1B[0msequence", "escape[0msequence")] [InlineData("unicode\u2028line\u2029separators", "unicodelineseparators")] - [InlineData("del\x7Fchar", "delchar")] + [InlineData("del\u007Fchar", "delchar")] public void SanitizePreventsLogForging(string maliciousInput, string expected) { // Act From 5de5770274ce1cda4080a7afa4b43fc7bd9146d4 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 16:24:17 +0100 Subject: [PATCH 14/28] Dispose correctly --- .../ElasticsearchAskAiMessageFeedbackGateway.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs index a75d2fb97..582eb4e4a 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs @@ -16,11 +16,12 @@ namespace Elastic.Documentation.Api.Infrastructure.Adapters.AskAi; /// /// Records Ask AI message feedback to Elasticsearch. /// -public class ElasticsearchAskAiMessageFeedbackGateway : IAskAiMessageFeedbackGateway +public sealed class ElasticsearchAskAiMessageFeedbackGateway : IAskAiMessageFeedbackGateway, IDisposable { private readonly ElasticsearchClient _client; private readonly string _indexName; private readonly ILogger _logger; + private bool _disposed; public ElasticsearchAskAiMessageFeedbackGateway( ElasticsearchOptions elasticsearchOptions, @@ -41,6 +42,15 @@ public ElasticsearchAskAiMessageFeedbackGateway( _client = new ElasticsearchClient(clientSettings); } + public void Dispose() + { + if (_disposed) + return; + + (_client.Transport as IDisposable)?.Dispose(); + _disposed = true; + } + public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, CancellationToken ctx) { var feedbackId = Guid.NewGuid(); From a112b9898210999e3e5316708d9881379030e71e Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 16:26:23 +0100 Subject: [PATCH 15/28] Change scope of services --- .../ServicesExtension.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs b/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs index a9eaa2095..b3ab979ca 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs @@ -217,8 +217,8 @@ private static void AddAskAiUsecase(IServiceCollection services, AppEnv appEnv) _ = services.AddScoped(); logger?.LogInformation("Gateway and transformer factories registered successfully - provider switchable via X-AI-Provider header"); - // Register message feedback components - _ = services.AddScoped(); + // Register message feedback components (gateway is singleton for connection reuse) + _ = services.AddSingleton(); _ = services.AddScoped(); logger?.LogInformation("AskAiMessageFeedbackUsecase and Elasticsearch gateway registered successfully"); } @@ -232,7 +232,7 @@ private static void AddSearchUsecase(IServiceCollection services, AppEnv appEnv) { var logger = GetLogger(services); logger?.LogInformation("Configuring Search use case for environment {AppEnvironment}", appEnv); - _ = services.AddScoped(); + _ = services.AddSingleton(); _ = services.AddScoped(); _ = services.AddScoped(); } From 6eea0cb6aaedbca0ac037605af73d8ebe0ac64bb Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 21:03:28 +0100 Subject: [PATCH 16/28] Potential fix for pull request finding 'Missing Dispose call on local IDisposable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- .../AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs index 582eb4e4a..90f18784d 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs @@ -21,6 +21,7 @@ public sealed class ElasticsearchAskAiMessageFeedbackGateway : IAskAiMessageFeed private readonly ElasticsearchClient _client; private readonly string _indexName; private readonly ILogger _logger; + private readonly SingleNodePool _nodePool; private bool _disposed; public ElasticsearchAskAiMessageFeedbackGateway( @@ -31,9 +32,9 @@ public ElasticsearchAskAiMessageFeedbackGateway( _logger = logger; _indexName = $"ask-ai-message-feedback-{appEnvironment.Current.ToStringFast(true)}"; - var nodePool = new SingleNodePool(new Uri(elasticsearchOptions.Url.Trim())); + _nodePool = new SingleNodePool(new Uri(elasticsearchOptions.Url.Trim())); var clientSettings = new ElasticsearchClientSettings( - nodePool, + _nodePool, sourceSerializer: (_, settings) => new DefaultSourceSerializer(settings, MessageFeedbackJsonContext.Default) ) .DefaultIndex(_indexName) @@ -47,6 +48,7 @@ public void Dispose() if (_disposed) return; + _nodePool.Dispose(); (_client.Transport as IDisposable)?.Dispose(); _disposed = true; } From b6b2cbf55b54ebc5a69bd4aecacf7d512324e57e Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 21:09:21 +0100 Subject: [PATCH 17/28] Potential fix for pull request finding 'Missing Dispose call on local IDisposable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- .../AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs index 90f18784d..7cc224532 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs @@ -33,14 +33,15 @@ public ElasticsearchAskAiMessageFeedbackGateway( _indexName = $"ask-ai-message-feedback-{appEnvironment.Current.ToStringFast(true)}"; _nodePool = new SingleNodePool(new Uri(elasticsearchOptions.Url.Trim())); - var clientSettings = new ElasticsearchClientSettings( + using (var clientSettings = new ElasticsearchClientSettings( _nodePool, sourceSerializer: (_, settings) => new DefaultSourceSerializer(settings, MessageFeedbackJsonContext.Default) ) .DefaultIndex(_indexName) - .Authentication(new ApiKey(elasticsearchOptions.ApiKey)); - - _client = new ElasticsearchClient(clientSettings); + .Authentication(new ApiKey(elasticsearchOptions.ApiKey))) + { + _client = new ElasticsearchClient(clientSettings); + } } public void Dispose() From 5f6a852aa71400ea99c29b6bd588b9706233c602 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 21:21:11 +0100 Subject: [PATCH 18/28] Format --- .../AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs index 7cc224532..bfbe66e84 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs @@ -33,15 +33,13 @@ public ElasticsearchAskAiMessageFeedbackGateway( _indexName = $"ask-ai-message-feedback-{appEnvironment.Current.ToStringFast(true)}"; _nodePool = new SingleNodePool(new Uri(elasticsearchOptions.Url.Trim())); - using (var clientSettings = new ElasticsearchClientSettings( + using var clientSettings = new ElasticsearchClientSettings( _nodePool, sourceSerializer: (_, settings) => new DefaultSourceSerializer(settings, MessageFeedbackJsonContext.Default) ) .DefaultIndex(_indexName) - .Authentication(new ApiKey(elasticsearchOptions.ApiKey))) - { - _client = new ElasticsearchClient(clientSettings); - } + .Authentication(new ApiKey(elasticsearchOptions.ApiKey)); + _client = new ElasticsearchClient(clientSettings); } public void Dispose() From 07ff6e61b4d5bccd3987b51208ccceb088769fd8 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 21:25:43 +0100 Subject: [PATCH 19/28] Fix logging format for conversation_id in AgentBuilder --- .../Adapters/AskAi/AgentBuilderAskAiGateway.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs index c0e1f821a..e1ed540b1 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs @@ -33,7 +33,7 @@ public async Task AskAi(AskAiRequest askAiRequest, Cancel ctx = default) askAiRequest.ConversationId); var requestBody = JsonSerializer.Serialize(agentBuilderPayload, AgentBuilderContext.Default.AgentBuilderPayload); - logger.LogInformation("Sending to Agent Builder with conversation_id: {ConversationId}", LogSanitizer.Sanitize(askAiRequest.ConversationId) ?? "(null - first request)"); + logger.LogInformation("Sending to Agent Builder with conversation_id: \"{ConversationId}\"", LogSanitizer.Sanitize(askAiRequest.ConversationId) ?? "(null - first request)"); var kibanaUrl = await parameterProvider.GetParam("docs-kibana-url", false, ctx); var kibanaApiKey = await parameterProvider.GetParam("docs-kibana-apikey", true, ctx); From 01cd487920fb462d3aa4b3bd7c113a186ec9799f Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 22:17:18 +0100 Subject: [PATCH 20/28] Fix tests --- .../SearchOrAskAi/AskAi/ChatMessage.test.tsx | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.test.tsx b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.test.tsx index 7f4f5327b..7871ecfee 100644 --- a/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.test.tsx +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.test.tsx @@ -1,9 +1,27 @@ import { ApiError } from '../errorHandling' import { ChatMessage } from './ChatMessage' import { ChatMessage as ChatMessageType } from './chat.store' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { render, screen } from '@testing-library/react' import * as React from 'react' +// Create a fresh QueryClient for each test +const createTestQueryClient = () => + new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }) + +// Wrapper component for tests that need React Query +const renderWithQueryClient = (ui: React.ReactElement) => { + const testQueryClient = createTestQueryClient() + return render( + {ui} + ) +} + // Mock EuiCallOut and EuiSpacer for SearchOrAskAiErrorCallout jest.mock('@elastic/eui', () => { const actual = jest.requireActual('@elastic/eui') @@ -141,7 +159,7 @@ describe('ChatMessage Component', () => { it('should render AI message with correct content', () => { // Act - render() + renderWithQueryClient() // Assert expect( @@ -153,7 +171,7 @@ describe('ChatMessage Component', () => { it('should show feedback buttons', () => { // Act - render() + renderWithQueryClient() // Assert expect( @@ -170,7 +188,7 @@ describe('ChatMessage Component', () => { it('should display Elastic logo icon', () => { // Act - render() + renderWithQueryClient() // Assert const messageElement = screen @@ -199,7 +217,7 @@ describe('ChatMessage Component', () => { it('should show loading icon when streaming', () => { // Act - render() + renderWithQueryClient() // Assert // Loading elastic icon should be present @@ -211,7 +229,7 @@ describe('ChatMessage Component', () => { it('should not show feedback buttons when streaming', () => { // Act - render() + renderWithQueryClient() // Assert expect( @@ -244,7 +262,7 @@ describe('ChatMessage Component', () => { it('should show error message', () => { // Act - render() + renderWithQueryClient() // Assert const callout = screen.getByTestId('eui-callout') @@ -258,7 +276,7 @@ describe('ChatMessage Component', () => { it('should display previous content before error occurred', () => { // Act - render() + renderWithQueryClient() // Assert // When there's an error, the content is hidden, only the error callout is shown @@ -282,7 +300,7 @@ describe('ChatMessage Component', () => { it('should render markdown content', () => { // Act - render() + renderWithQueryClient() // Assert - EuiMarkdownFormat will render the markdown expect(screen.getByText(/Bold text/)).toBeInTheDocument() From ce47e5974661a24267bdf9b3ff7a77e5cdff23c1 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 22:19:54 +0100 Subject: [PATCH 21/28] Potential fix for code scanning alert no. 38: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../LogSanitizer.cs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs index 0b82c3e3c..cbcce536d 100644 --- a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -31,21 +31,7 @@ public static string Sanitize(string? input) var span = input.AsSpan(); - // Fast path: check if any dangerous characters exist (common case has none) - zero allocations - var hasDangerousChars = false; - foreach (var c in span) - { - if (IsDangerousChar(c)) - { - hasDangerousChars = true; - break; - } - } - - if (!hasDangerousChars) - return input; - - // Slow path: count chars to keep, then create string with exact size + // Always sanitize: remove all dangerous/control/log-forging characters var keepCount = 0; foreach (var c in span) { From e562c5cf9a99d91f148523d9d54f8dec3f587a63 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 22:31:37 +0100 Subject: [PATCH 22/28] Re-add fast path for common cases to avoid string allocation --- .../LogSanitizer.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs index cbcce536d..0b82c3e3c 100644 --- a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -31,7 +31,21 @@ public static string Sanitize(string? input) var span = input.AsSpan(); - // Always sanitize: remove all dangerous/control/log-forging characters + // Fast path: check if any dangerous characters exist (common case has none) - zero allocations + var hasDangerousChars = false; + foreach (var c in span) + { + if (IsDangerousChar(c)) + { + hasDangerousChars = true; + break; + } + } + + if (!hasDangerousChars) + return input; + + // Slow path: count chars to keep, then create string with exact size var keepCount = 0; foreach (var c in span) { From d31373166454dcd4bc2cdc0fb7e0e3fd181f7c3d Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Mon, 1 Dec 2025 22:41:59 +0100 Subject: [PATCH 23/28] Potential fix for code scanning alert no. 35: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../Adapters/Search/ElasticsearchGateway.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs index 6e59b1975..86a267f45 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs @@ -172,7 +172,7 @@ private static Query BuildFilter() => !(Query)new TermsQuery(Infer.Field Results)> HybridSearchWithRrfAsync(string query, int pageNumber, int pageSize, Cancel ctx = default) { - _logger.LogInformation("Starting RRF hybrid search for '{Query}' with pageNumber={PageNumber}, pageSize={PageSize}", LogSanitizer.Sanitize(query), pageNumber, pageSize); + _logger.LogInformation("Starting RRF hybrid search for '[{Query}]' with pageNumber={PageNumber}, pageSize={PageSize}", $"[{LogSanitizer.Sanitize(query)}]", pageNumber, pageSize); const string preTag = ""; const string postTag = ""; @@ -257,13 +257,13 @@ private static Query BuildFilter() => !(Query)new TermsQuery(Infer.Field Date: Mon, 1 Dec 2025 23:51:22 +0100 Subject: [PATCH 24/28] Potential fix for code scanning alert no. 39: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../Adapters/Search/ElasticsearchGateway.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs index 86a267f45..ef19d6cf0 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs @@ -172,7 +172,8 @@ private static Query BuildFilter() => !(Query)new TermsQuery(Infer.Field Results)> HybridSearchWithRrfAsync(string query, int pageNumber, int pageSize, Cancel ctx = default) { - _logger.LogInformation("Starting RRF hybrid search for '[{Query}]' with pageNumber={PageNumber}, pageSize={PageSize}", $"[{LogSanitizer.Sanitize(query)}]", pageNumber, pageSize); + var sanitizedQuery = LogSanitizer.Sanitize(query); + _logger.LogInformation("Starting RRF hybrid search. UserQuery='{SanitizedQuery}' pageNumber={PageNumber}, pageSize={PageSize}", sanitizedQuery, pageNumber, pageSize); const string preTag = ""; const string postTag = ""; @@ -257,13 +258,13 @@ private static Query BuildFilter() => !(Query)new TermsQuery(Infer.Field Date: Mon, 1 Dec 2025 23:57:32 +0100 Subject: [PATCH 25/28] Potential fix for code scanning alert no. 43: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../Elastic.Documentation.Api.Core/Search/SearchUsecase.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/api/Elastic.Documentation.Api.Core/Search/SearchUsecase.cs b/src/api/Elastic.Documentation.Api.Core/Search/SearchUsecase.cs index 20d6ed6f9..53cfd1330 100644 --- a/src/api/Elastic.Documentation.Api.Core/Search/SearchUsecase.cs +++ b/src/api/Elastic.Documentation.Api.Core/Search/SearchUsecase.cs @@ -25,11 +25,13 @@ public async Task Search(SearchRequest request, Cancel ctx = def PageSize = request.PageSize, }; + var sanitizedQuery = LogSanitizer.Sanitize(request.Query); + LogSearchResults( logger, response.PageSize, response.PageNumber, - request.Query, + sanitizedQuery, new SearchResultsLogProperties(results.Select(i => i.Url).ToArray()) ); From 65ad494e778d23c66453def34f762df8e29e7963 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Tue, 2 Dec 2025 00:16:49 +0100 Subject: [PATCH 26/28] Also remove newlines --- src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs index 0b82c3e3c..d03fc4544 100644 --- a/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -17,6 +17,7 @@ public static class LogSanitizer /// /// /// Removes: + /// - Newlines (\r, \n) /// - ASCII control characters (0x00-0x1F) /// - DEL character (0x7F) /// - Unicode line separator (U+2028) @@ -68,7 +69,8 @@ public static string Sanitize(string? input) /// Checks if a character is dangerous for logging (could enable log forging). /// private static bool IsDangerousChar(char c) => - c is <= '\x1F' or // ASCII control characters (0x00-0x1F) + c is '\r' or '\n' or // Newlines (carriage return, line feed) + <= '\x1F' or // ASCII control characters (0x00-0x1F) '\x7F' or // DEL character '\u2028' or // Unicode line separator '\u2029'; // Unicode paragraph separator From f6221915c45e6e7228b7b863457dd198555f5d3d Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Tue, 2 Dec 2025 00:29:03 +0100 Subject: [PATCH 27/28] Remove user input from logs --- .../Adapters/Search/ElasticsearchGateway.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs index ef19d6cf0..b5a48b66d 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs @@ -254,17 +254,15 @@ private static Query BuildFilter() => !(Query)new TermsQuery(Infer.Field Date: Tue, 2 Dec 2025 00:36:52 +0100 Subject: [PATCH 28/28] Remove this troublesome log for now --- .../Adapters/Search/ElasticsearchGateway.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs index b5a48b66d..3ea944d2d 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/Search/ElasticsearchGateway.cs @@ -172,9 +172,6 @@ private static Query BuildFilter() => !(Query)new TermsQuery(Infer.Field Results)> HybridSearchWithRrfAsync(string query, int pageNumber, int pageSize, Cancel ctx = default) { - var sanitizedQuery = LogSanitizer.Sanitize(query); - _logger.LogInformation("Starting RRF hybrid search. UserQuery='{SanitizedQuery}' pageNumber={PageNumber}, pageSize={PageSize}", sanitizedQuery, pageNumber, pageSize); - const string preTag = ""; const string postTag = "";