Skip to content

Commit cfcce5e

Browse files
rudolfcursoragent
andcommitted
Address review feedback: audit test + brand obs-ai namespace
- security/audit_service.test.ts: new test verifying that a spaceId on a fake KibanaRequest surfaces as `kibana.space_id` in the audit log when the audit service is wired with `getSpaceId: (req) => req.spaceId` (mirrors real wiring through spacesService). Addresses #266371 (comment)... by @jeramysoucy. - observability_ai_assistant: change the internal `namespace: string` field on the client/knowledge-base service dependency contracts to `namespace: SpaceId`. This was a real instance of the smell branded types are designed to flush out: `request.spaceId` (branded SpaceId) was being assigned into a `string` field, immediately losing the semantic distinction. Now the value stays branded all the way through the in-process API surface; the storage shape on ES (`Omit<KnowledgeBaseEntry, 'id'> & { namespace: string }`) stays a plain string because brands only exist in TypeScript and don't survive serialization. Use `getSpaceUrlPrefix` from `@kbn/core-spaces-common` for the space-aware URL, removing the local `DEFAULT_SPACE_ID` branching. Addresses #266371 (comment)... by @viduni94. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 0e814ab commit cfcce5e

4 files changed

Lines changed: 52 additions & 17 deletions

File tree

x-pack/platform/plugins/shared/observability_ai_assistant/server/service/client/index.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { isEmpty, last, merge, repeat, size } from 'lodash';
1818
import { Subject, Observable } from 'rxjs';
1919
import { EventEmitter, type Readable } from 'stream';
2020
import { finished } from 'stream/promises';
21+
import { asSpaceId, DEFAULT_SPACE_ID, type SpaceId } from '@kbn/core-spaces-common';
2122
import { ObservabilityAIAssistantClient } from '.';
2223
import { MessageRole, type Message, CONTEXT_FUNCTION_NAME } from '../../../common';
2324
import type {
@@ -146,7 +147,7 @@ describe('Observability AI Assistant client', () => {
146147

147148
let llmSimulator: LlmSimulator;
148149

149-
function createClient(namespace: string = 'default') {
150+
function createClient(namespace: SpaceId = DEFAULT_SPACE_ID) {
150151
jest.resetAllMocks();
151152

152153
// uncomment this line for debugging
@@ -219,7 +220,7 @@ describe('Observability AI Assistant client', () => {
219220
inferenceClient: inferenceClientMock,
220221
knowledgeBaseService: knowledgeBaseServiceMock,
221222
logger: loggerMock,
222-
namespace: 'default',
223+
namespace: DEFAULT_SPACE_ID,
223224
user: {
224225
name: 'johndoe',
225226
},
@@ -1757,7 +1758,7 @@ describe('Observability AI Assistant client', () => {
17571758
);
17581759
});
17591760

1760-
const runWithNamespace = async (namespace: string) => {
1761+
const runWithNamespace = async (namespace: SpaceId) => {
17611762
// client = createClient(namespace);
17621763
client = createClient();
17631764
(client as any).dependencies.namespace = namespace;
@@ -1777,7 +1778,7 @@ describe('Observability AI Assistant client', () => {
17771778
};
17781779

17791780
it('generates a link without space segment for default space', async () => {
1780-
await runWithNamespace('default');
1781+
await runWithNamespace(DEFAULT_SPACE_ID);
17811782

17821783
expect(functionClientMock.registerInstruction).toHaveBeenCalled();
17831784
expect(functionClientMock.registerInstruction).toHaveBeenCalledWith(
@@ -1790,7 +1791,7 @@ describe('Observability AI Assistant client', () => {
17901791

17911792
it('generates a link with space segment for non-default space', async () => {
17921793
const space = 'myspace';
1793-
await runWithNamespace('myspace');
1794+
await runWithNamespace(asSpaceId('myspace'));
17941795

17951796
expect(functionClientMock.registerInstruction).toHaveBeenCalled();
17961797
expect(functionClientMock.registerInstruction).toHaveBeenCalledWith(

x-pack/platform/plugins/shared/observability_ai_assistant/server/service/client/index.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import type { Logger } from '@kbn/logging';
1212
import type { PublicMethodsOf } from '@kbn/utility-types';
1313
import { last, merge, omit } from 'lodash';
1414
import type { Observable } from 'rxjs';
15-
import { DEFAULT_SPACE_ID } from '@kbn/core-spaces-common';
15+
import { getSpaceUrlPrefix } from '@kbn/core-spaces-common';
16+
import type { SpaceId } from '@kbn/core-spaces-common';
1617
import {
1718
catchError,
1819
defer,
@@ -95,7 +96,7 @@ export class ObservabilityAIAssistantClient {
9596
core: CoreSetup<ObservabilityAIAssistantPluginStartDependencies>;
9697
actionsClient: PublicMethodsOf<ActionsClient>;
9798
uiSettingsClient: IUiSettingsClient;
98-
namespace: string;
99+
namespace: SpaceId;
99100
esClient: {
100101
asInternalUser: ElasticsearchClient;
101102
asCurrentUser: ElasticsearchClient;
@@ -234,10 +235,7 @@ export class ObservabilityAIAssistantClient {
234235

235236
if (persist && !isConversationUpdate && kibanaPublicUrl) {
236237
const { namespace } = this.dependencies;
237-
const spaceAwarePath =
238-
namespace === DEFAULT_SPACE_ID
239-
? '/app/observabilityAIAssistant'
240-
: `/s/${namespace}/app/observabilityAIAssistant`;
238+
const spaceAwarePath = `${getSpaceUrlPrefix(namespace)}/app/observabilityAIAssistant`;
241239

242240
const conversationUrl = `${kibanaPublicUrl}${spaceAwarePath}/conversations/${conversationId}`;
243241

x-pack/platform/plugins/shared/observability_ai_assistant/server/service/knowledge_base_service/index.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { orderBy } from 'lodash';
1212
import { encode } from 'gpt-tokenizer';
1313
import { isLockAcquisitionError } from '@kbn/lock-manager';
1414
import type { DocumentationManagerAPI } from '@kbn/product-doc-base-plugin/server/services/doc_manager';
15+
import type { SpaceId } from '@kbn/core-spaces-common';
1516
import { resourceNames } from '..';
1617
import type {
1718
Instruction,
@@ -75,7 +76,7 @@ export class KnowledgeBaseService {
7576
}: {
7677
queries: Array<{ text: string; boost?: number }>;
7778
categories?: string[];
78-
namespace: string;
79+
namespace: SpaceId;
7980
user?: { name: string };
8081
}): Promise<RecalledEntry[]> {
8182
const response = await this.dependencies.esClient.asInternalUser.search<
@@ -188,7 +189,7 @@ export class KnowledgeBaseService {
188189
queries: Array<{ text: string; boost?: number }>;
189190
categories?: string[];
190191
user?: { name: string };
191-
namespace: string;
192+
namespace: SpaceId;
192193
esClient: { asCurrentUser: ElasticsearchClient; asInternalUser: ElasticsearchClient };
193194
uiSettingsClient: IUiSettingsClient;
194195
limit?: { tokens?: number; size?: number };
@@ -277,7 +278,7 @@ export class KnowledgeBaseService {
277278
};
278279

279280
getUserInstructions = async (
280-
namespace: string,
281+
namespace: SpaceId,
281282
user?: { name: string }
282283
): Promise<Array<Instruction & { public?: boolean }>> => {
283284
if (!this.dependencies.config.enableKnowledgeBase) {
@@ -330,7 +331,7 @@ export class KnowledgeBaseService {
330331
query?: string;
331332
sortBy?: string;
332333
sortDirection?: 'asc' | 'desc';
333-
namespace: string;
334+
namespace: SpaceId;
334335
}): Promise<{ entries: KnowledgeBaseEntry[] }> => {
335336
if (!this.dependencies.config.enableKnowledgeBase) {
336337
return { entries: [] };
@@ -478,7 +479,7 @@ export class KnowledgeBaseService {
478479
}: {
479480
entry: Omit<KnowledgeBaseEntry, '@timestamp'>;
480481
user?: { name: string; id?: string };
481-
namespace: string;
482+
namespace: SpaceId;
482483
}): Promise<void> => {
483484
if (!this.dependencies.config.enableKnowledgeBase) {
484485
return;
@@ -538,7 +539,7 @@ export class KnowledgeBaseService {
538539
}: {
539540
entries: Array<Omit<KnowledgeBaseEntry, '@timestamp'>>;
540541
user?: { name: string; id?: string };
541-
namespace: string;
542+
namespace: SpaceId;
542543
}): Promise<void> => {
543544
if (!this.dependencies.config.enableKnowledgeBase) {
544545
return;

x-pack/platform/plugins/shared/security/server/audit/audit_service.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type { FakeRawRequest } from '@kbn/core-http-server';
1313
import { httpServerMock, httpServiceMock } from '@kbn/core-http-server-mocks';
1414
import { kibanaRequestFactory } from '@kbn/core-http-server-utils';
1515
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
16+
import { asSpaceId } from '@kbn/core-spaces-common';
1617
import type { AuditEvent } from '@kbn/security-plugin-types-server';
1718

1819
import {
@@ -267,6 +268,40 @@ describe('#asScoped', () => {
267268
audit.stop();
268269
});
269270

271+
it('logs space_id from a fake request that carries a spaceId', async () => {
272+
const audit = new AuditService(logger);
273+
const auditSetup = audit.setup({
274+
license,
275+
config,
276+
logging,
277+
http,
278+
getCurrentUser,
279+
// Mirror real wiring (spacesService.getSpaceId) by sourcing the space id
280+
// directly from the request.
281+
getSpaceId: (req) => req.spaceId,
282+
getSID: () => Promise.resolve(undefined),
283+
recordAuditLoggingUsage,
284+
});
285+
286+
const fakeRawRequest: FakeRawRequest = {
287+
headers: {},
288+
spaceId: asSpaceId('my-space'),
289+
};
290+
const request = kibanaRequestFactory(fakeRawRequest);
291+
292+
await auditSetup.asScoped(request).log({
293+
message: 'MESSAGE',
294+
event: { action: 'ACTION' },
295+
});
296+
expect(logger.info).toHaveBeenLastCalledWith(
297+
'MESSAGE',
298+
expect.objectContaining({
299+
kibana: expect.objectContaining({ space_id: 'my-space' }),
300+
})
301+
);
302+
audit.stop();
303+
});
304+
270305
it('does not log to audit logger if event matches ignore filter', async () => {
271306
const audit = new AuditService(logger);
272307
const auditSetup = audit.setup({

0 commit comments

Comments
 (0)