From 2598a1b5e46aa6e35076372ebb98a65737a19e1b Mon Sep 17 00:00:00 2001 From: Bruno Farache Date: Fri, 24 Jul 2026 16:13:44 -0300 Subject: [PATCH 1/2] [Agent Builder] Prevent conversation overwrite via auto-create (#280189) Manual backport of #280189 adapted to the 9.4 hasAccess-based conversation client. Makes exists() report physical document existence and create() non-destructive via op_type: 'create'. --- .../conversation/client/client.test.ts | 166 ++++++++++++++++++ .../services/conversation/client/client.ts | 30 +++- 2 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.test.ts diff --git a/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.test.ts b/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.test.ts new file mode 100644 index 0000000000000..190b319eb2b31 --- /dev/null +++ b/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.test.ts @@ -0,0 +1,166 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { loggerMock } from '@kbn/logging-mocks'; +import { createClient, type ConversationClient } from './client'; +import type { Document } from './converters'; + +const testSpace = 'default'; + +interface MockEsClient { + search: jest.Mock; + index: jest.Mock; + delete: jest.Mock; +} + +const mockEsClient: MockEsClient = { + search: jest.fn(), + index: jest.fn(), + delete: jest.fn(), +}; + +jest.mock('./storage', () => ({ + createStorage: jest.fn(() => ({ + getClient: jest.fn(() => mockEsClient), + })), +})); + +describe('ConversationClient', () => { + let client: ConversationClient; + + const createConversationDocument = ({ + id = 'conversation-1', + agentId = 'agent-1', + userId = 'user-1', + username = 'test-user', + }: { + id?: string; + agentId?: string; + userId?: string; + username?: string; + } = {}): Document => + ({ + _id: id, + _seq_no: 1, + _primary_term: 1, + _source: { + agent_id: agentId, + user_id: userId, + user_name: username, + space: testSpace, + title: 'Conversation 1', + created_at: '2024-09-04T06:44:17.944Z', + updated_at: '2025-08-04T06:44:19.123Z', + conversation_rounds: [], + }, + } as Document); + + beforeEach(() => { + jest.clearAllMocks(); + + client = createClient({ + space: testSpace, + logger: loggerMock.create(), + esClient: {} as never, + user: { + id: 'user-1', + username: 'test-user', + }, + }); + }); + + describe('exists', () => { + it('returns true when the document exists, even when owned by another user', async () => { + mockEsClient.search.mockResolvedValue({ + hits: { + hits: [ + createConversationDocument({ + userId: 'other-user-id', + username: 'other-user', + }), + ], + }, + }); + + await expect(client.exists('conversation-1')).resolves.toBe(true); + }); + + it('returns false when no document exists', async () => { + mockEsClient.search.mockResolvedValue({ + hits: { + hits: [], + }, + }); + + await expect(client.exists('conversation-1')).resolves.toBe(false); + }); + + it('propagates search failures', async () => { + const error = new Error('search unavailable'); + mockEsClient.search.mockRejectedValue(error); + + await expect(client.exists('conversation-1')).rejects.toBe(error); + }); + }); + + describe('create', () => { + beforeEach(() => { + mockEsClient.index.mockResolvedValue({ result: 'created' }); + mockEsClient.search.mockResolvedValue({ + hits: { + hits: [createConversationDocument()], + }, + }); + }); + + it('indexes with op_type create so existing conversations are never overwritten', async () => { + await client.create({ + id: 'conversation-1', + title: 'Conversation 1', + agent_id: 'agent-1', + rounds: [], + }); + + expect(mockEsClient.index).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'conversation-1', + op_type: 'create', + }) + ); + }); + + it('throws a not found error when the id already exists', async () => { + const conflictError = Object.assign(new Error('version conflict'), { statusCode: 409 }); + mockEsClient.index.mockRejectedValueOnce(conflictError); + + await expect( + client.create({ + id: 'conversation-1', + title: 'Conversation 1', + agent_id: 'agent-1', + rounds: [], + }) + ).rejects.toMatchObject({ + message: 'Conversation conversation-1 not found', + }); + }); + + it('propagates non-conflict index failures', async () => { + const error = new Error('index unavailable'); + mockEsClient.index.mockRejectedValueOnce(error); + + await expect( + client.create({ + id: 'conversation-1', + title: 'Conversation 1', + agent_id: 'agent-1', + rounds: [], + }) + ).rejects.toBe(error); + }); + }); +}); diff --git a/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.ts b/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.ts index 2084b60d08176..a2b0c43f634a0 100644 --- a/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.ts +++ b/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.ts @@ -109,12 +109,16 @@ class ConversationClientImpl implements ConversationClient { return fromEs(document); } + /** + * Reports whether a conversation document physically exists in the index. + * + * This is an existence check only and intentionally does not enforce access + * control. Callers must not treat a `true` result as authorization to read or + * mutate the conversation; ownership is still validated by `get`/`update`/`delete`. + */ async exists(conversationId: string): Promise { const document = await this._get(conversationId); - if (!document) { - return false; - } - return hasAccess({ conversation: document, user: this.user }); + return document !== undefined; } async create(conversation: ConversationCreateRequest): Promise { @@ -128,10 +132,20 @@ class ConversationClientImpl implements ConversationClient { space: this.space, }); - await this.storage.getClient().index({ - id, - document: attributes, - }); + try { + await this.storage.getClient().index({ + id, + document: attributes, + // never overwrite an existing conversation, e.g. one owned by another user + op_type: 'create', + }); + } catch (error) { + if (error?.statusCode === 409) { + throw createConversationNotFoundError({ conversationId: id }); + } + + throw error; + } return this.get(id); } From d057e66af6fe4a36c7441e528fed0bd8ac4bfee0 Mon Sep 17 00:00:00 2001 From: Bruno Farache Date: Fri, 24 Jul 2026 20:45:21 -0300 Subject: [PATCH 2/2] [Agent Builder] Remove code comments --- .../server/services/conversation/client/client.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.ts b/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.ts index a2b0c43f634a0..60117e91e9450 100644 --- a/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.ts +++ b/x-pack/platform/plugins/shared/agent_builder/server/services/conversation/client/client.ts @@ -109,13 +109,6 @@ class ConversationClientImpl implements ConversationClient { return fromEs(document); } - /** - * Reports whether a conversation document physically exists in the index. - * - * This is an existence check only and intentionally does not enforce access - * control. Callers must not treat a `true` result as authorization to read or - * mutate the conversation; ownership is still validated by `get`/`update`/`delete`. - */ async exists(conversationId: string): Promise { const document = await this._get(conversationId); return document !== undefined; @@ -136,7 +129,6 @@ class ConversationClientImpl implements ConversationClient { await this.storage.getClient().index({ id, document: attributes, - // never overwrite an existing conversation, e.g. one owned by another user op_type: 'create', }); } catch (error) {