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() 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..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 @@ -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 } from './useMessageFeedback' import { useStatusMinDisplay } from './useStatusMinDisplay' import { EuiButtonIcon, @@ -186,62 +187,80 @@ 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 } = useMessageFeedback( + messageId, + conversationId + ) + + return ( + + + submitFeedback('thumbsUp')} /> - )} - - - {onRetry && ( + + - + submitFeedback('thumbsDown')} /> - )} - -) + + + {(copy) => ( + + )} + + + {onRetry && ( + + + + + + )} + + ) +} export const ChatMessage = ({ message, @@ -412,6 +431,7 @@ export const ChatMessage = ({ 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 new file mode 100644 index 000000000..076b2fb8b --- /dev/null +++ b/src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/useMessageFeedback.ts @@ -0,0 +1,113 @@ +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, useRef } from 'react' + +export type { Reaction } from './chat.store' + +const DEBOUNCE_MS = 500 + +interface MessageFeedbackRequest { + messageId: string + conversationId: string + reaction: Reaction +} + +interface UseMessageFeedbackReturn { + selectedReaction: Reaction | null + submitFeedback: (reaction: Reaction) => void +} + +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 = useMessageReaction(messageId) + const { setMessageFeedback } = useChatActions() + const debounceTimeoutRef = useRef | null>( + 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) { + logWarn('Cannot submit feedback without conversationId', { + 'ask_ai.message.id': messageId, + }) + return + } + + // Ignore if same reaction already selected + if (selectedReaction === reaction) { + return + } + + // Optimistic update - stored in Zustand so it persists across tab switches + setMessageFeedback(messageId, 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, + conversationId, + selectedReaction, + mutation, + setMessageFeedback, + ] + ) + + return { + selectedReaction, + submitFeedback, + } +} 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..127159ca0 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackRequest.cs @@ -0,0 +1,30 @@ +// 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.Text.Json.Serialization; + +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( + Guid MessageId, + Guid ConversationId, + 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 new file mode 100644 index 000000000..82235e392 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiMessageFeedbackUsecase.cs @@ -0,0 +1,43 @@ +// 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, 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 + + // MessageId and ConversationId are Guid types, so no sanitization needed + 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, + euid + ); + + await feedbackGateway.RecordFeedbackAsync(record, ctx); + } +} diff --git a/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/AskAiUsecase.cs index 43555d174..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}", 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/AskAi/IAskAiMessageFeedbackGateway.cs b/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs new file mode 100644 index 000000000..e3c52da60 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Core/AskAi/IAskAiMessageFeedbackGateway.cs @@ -0,0 +1,29 @@ +// 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( + Guid MessageId, + Guid ConversationId, + Reaction Reaction, + string? Euid = null +); 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..d03fc4544 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Core/LogSanitizer.cs @@ -0,0 +1,77 @@ +// 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 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: + /// - Newlines (\r, \n) + /// - 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) + { + if (string.IsNullOrEmpty(input)) + return string.Empty; + + 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 + var keepCount = 0; + foreach (var c in span) + { + if (!IsDangerousChar(c)) + keepCount++; + } + + return string.Create(keepCount, input, static (dest, src) => + { + var destIndex = 0; + foreach (var c in src) + { + if (!IsDangerousChar(c)) + dest[destIndex++] = c; + } + }); + } + + /// + /// Checks if a character is dangerous for logging (could enable log forging). + /// + private static bool IsDangerousChar(char c) => + 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 +} 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()) ); diff --git a/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs b/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs index 3081c418b..3b8344bfc 100644 --- a/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs +++ b/src/api/Elastic.Documentation.Api.Core/SerializationContext.cs @@ -18,6 +18,8 @@ public record InputMessage(string Role, MessagePart[] Parts); public record OutputMessage(string Role, MessagePart[] Parts, string FinishReason); [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.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/AgentBuilderAskAiGateway.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Adapters/AskAi/AgentBuilderAskAiGateway.cs index 456ec990d..e1ed540b1 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/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 sealed class ElasticsearchAskAiMessageFeedbackGateway : IAskAiMessageFeedbackGateway, IDisposable +{ + private readonly ElasticsearchClient _client; + private readonly string _indexName; + private readonly ILogger _logger; + private readonly SingleNodePool _nodePool; + private bool _disposed; + + public ElasticsearchAskAiMessageFeedbackGateway( + ElasticsearchOptions elasticsearchOptions, + AppEnvironment appEnvironment, + ILogger logger) + { + _logger = logger; + _indexName = $"ask-ai-message-feedback-{appEnvironment.Current.ToStringFast(true)}"; + + _nodePool = new SingleNodePool(new Uri(elasticsearchOptions.Url.Trim())); + using var clientSettings = new ElasticsearchClientSettings( + _nodePool, + sourceSerializer: (_, settings) => new DefaultSourceSerializer(settings, MessageFeedbackJsonContext.Default) + ) + .DefaultIndex(_indexName) + .Authentication(new ApiKey(elasticsearchOptions.ApiKey)); + _client = new ElasticsearchClient(clientSettings); + } + + public void Dispose() + { + if (_disposed) + return; + + _nodePool.Dispose(); + (_client.Transport as IDisposable)?.Dispose(); + _disposed = true; + } + + public async Task RecordFeedbackAsync(AskAiMessageFeedbackRecord record, CancellationToken ctx) + { + var feedbackId = Guid.NewGuid(); + var document = new MessageFeedbackDocument + { + FeedbackId = feedbackId.ToString(), + MessageId = record.MessageId.ToString(), + ConversationId = record.ConversationId.ToString(), + Reaction = record.Reaction.ToString().ToLowerInvariant(), + Euid = record.Euid, + Timestamp = DateTimeOffset.UtcNow + }; + + _logger.LogDebug("Indexing feedback with ID {FeedbackId} to index {IndexName}", feedbackId, _indexName); + + var response = await _client.IndexAsync(document, idx => idx + .Index(_indexName) + .Id(feedbackId.ToString()), ctx); + + // MessageId and ConversationId are Guid types, so no sanitization needed + 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}. ES _id: {EsId}, Index: {Index}", + record.Reaction, + record.MessageId, + 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; } + + [JsonPropertyName("conversation_id")] + public required string ConversationId { get; init; } + + [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; } +} + +[JsonSerializable(typeof(MessageFeedbackDocument))] +internal sealed partial class MessageFeedbackJsonContext : JsonSerializerContext; 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..3ea944d2d 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,8 +172,6 @@ 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); - const string preTag = ""; const string postTag = ""; @@ -252,17 +251,15 @@ 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", diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs index e336f23b2..c74c29fee 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/MappingsExtensions.cs @@ -35,6 +35,16 @@ private static void MapAskAiEndpoint(IEndpointRouteBuilder group) var stream = await askAiUsecase.AskAi(askAiRequest, ctx); 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) => + { + // 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) 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..b3ab979ca 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 (gateway is singleton for connection reuse) + _ = services.AddSingleton(); + _ = services.AddScoped(); + logger?.LogInformation("AskAiMessageFeedbackUsecase and Elasticsearch gateway registered successfully"); } catch (Exception ex) { @@ -227,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(); } 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..3be9601d7 --- /dev/null +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/LogSanitizerTests.cs @@ -0,0 +1,234 @@ +// 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 SanitizeWhenInputHasNoControlCharsReturnsSameString() + { + // 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 SanitizeWhenInputHasTabRemovesIt() + { + // Arrange + const string input = "Hello\tWorld"; + + // 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 SanitizeWhenInputHasMultipleControlCharsRemovesAll() + { + // 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("Line1Line2Line3Line4Line5"); + } + + [Fact] + public void SanitizeWhenInputIsOnlyControlCharsReturnsEmptyString() + { + // Arrange + const string input = "\r\n\t\x00\x1F"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + result.Should().BeEmpty(); + } + + [Fact] + public void SanitizeRemovesDelChar() + { + // Arrange - DEL is 0x7F + const string input = "Hello\u007FWorld"; + + // 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() + { + // Arrange - space (0x20) is NOT a control char + const string input = "Hello World! @#$%^&*() 123"; + + // Act + var result = LogSanitizer.Sanitize(input); + + // Assert + 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")] + [InlineData("unicode\u2028line\u2029separators", "unicodelineseparators")] + [InlineData("del\u007Fchar", "delchar")] + public void SanitizePreventsLogForging(string maliciousInput, string expected) + { + // Act + var result = LogSanitizer.Sanitize(maliciousInput); + + // Assert + result.Should().Be(expected); + } + + [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"); + } +}