Skip to content

Commit b084eae

Browse files
kibanamachinegsoldevilacursoragent
authored
[9.4] [RAC] Disable _source for Analyzer alert stats query (#285953) (#291051)
# Backport This will backport the following commits from `main` to `9.4`: - [[RAC] Disable _source for Analyzer alert stats query (#285953)](#285953) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Gerard Soldevila","email":"gerard.soldevila@elastic.co"},"sourceCommit":{"committedDate":"2026-08-25T10:15:42Z","message":"[RAC] Disable _source for Analyzer alert stats query (#285953)\n\n## Summary\n\nThe Security Analyzer prevalence query (`StatsQuery.alertStatsQuery`)\nfetches up to `size: 5000` alert hits but only consumes aggregations and\nhit ids — it never reads `_source`. Because Elasticsearch defaults\n`_source` to `true`, the response could exceed\n`elasticsearch.maxResponseSize`, and Kibana aborted the request.\n\nThis sets `_source: false` on that one query. No default is changed, so\nevery other `find()` caller keeps its current behavior — the blast\nradius is limited to the Analyzer path.\n\nBecause `AlertsClient.find` runs a per-hit authorization check on every\nresult, disabling `_source` would otherwise leave `ensureAllAuthorized`\nunable to read the rule type id and consumer from the hits. So\n`ensureAllAuthorized` now reads those from the `fields` API (already\nrequested by `searchAlerts`), falling back to `_source` for `mget`\nresponses (which don't support `fields`). The same change:\n\n- Replaces the broken `reduce` — which rebuilt `ownersAndRuleTypeIds` as\na single-element array each iteration, so only the **last** hit was ever\nauthorized — with a `forEach` that collects every hit id and\ndeduplicates `(ruleTypeId, consumer)` pairs before calling\n`ensureAuthorized`.\n- Hardens `isValidAlert`'s `fields` access with optional chaining, since\nwith `_source: false` validation reads from `fields`.\n\n## Why not default `_source: false` globally\n\nAn earlier iteration flipped the `searchAlerts` default to `false` and\nmade each caller opt back in. That required touching ~7 consumers across\nsecurity/o11y/apm and risked silently breaking any `find()` / HTTP\n`/find` caller we didn't enumerate. Keeping the default as-is and opting\n**out** only where we know `_source` is unused is smaller, safer, and\nbackport-friendly.\n\n## Test plan\n\n- [ ] `AlertsClient` find/get/update/bulkUpdate unit tests pass (per-hit\nauthz now covers every unique rule type + consumer pair; dedup verified)\n- [ ] `StatsQuery` unit test asserts the alert query runs with `_source:\nfalse`\n- [ ] Analyzer prevalence (`resolver/tree` with `includeHits: true`) no\nlonger returns oversized responses\n\nCo-authored-by: Cursor <cursoragent@cursor.com>","sha":"f2748c6aaece272b694d05b66657ad7bd3cbb389","branchLabelMapping":{"^v9.6.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","release_note:skip","backport:all-open","v9.6.0"],"title":"[RAC] Disable _source for Analyzer alert stats query","number":285953,"url":"https://github.com/elastic/kibana/pull/285953","mergeCommit":{"message":"[RAC] Disable _source for Analyzer alert stats query (#285953)\n\n## Summary\n\nThe Security Analyzer prevalence query (`StatsQuery.alertStatsQuery`)\nfetches up to `size: 5000` alert hits but only consumes aggregations and\nhit ids — it never reads `_source`. Because Elasticsearch defaults\n`_source` to `true`, the response could exceed\n`elasticsearch.maxResponseSize`, and Kibana aborted the request.\n\nThis sets `_source: false` on that one query. No default is changed, so\nevery other `find()` caller keeps its current behavior — the blast\nradius is limited to the Analyzer path.\n\nBecause `AlertsClient.find` runs a per-hit authorization check on every\nresult, disabling `_source` would otherwise leave `ensureAllAuthorized`\nunable to read the rule type id and consumer from the hits. So\n`ensureAllAuthorized` now reads those from the `fields` API (already\nrequested by `searchAlerts`), falling back to `_source` for `mget`\nresponses (which don't support `fields`). The same change:\n\n- Replaces the broken `reduce` — which rebuilt `ownersAndRuleTypeIds` as\na single-element array each iteration, so only the **last** hit was ever\nauthorized — with a `forEach` that collects every hit id and\ndeduplicates `(ruleTypeId, consumer)` pairs before calling\n`ensureAuthorized`.\n- Hardens `isValidAlert`'s `fields` access with optional chaining, since\nwith `_source: false` validation reads from `fields`.\n\n## Why not default `_source: false` globally\n\nAn earlier iteration flipped the `searchAlerts` default to `false` and\nmade each caller opt back in. That required touching ~7 consumers across\nsecurity/o11y/apm and risked silently breaking any `find()` / HTTP\n`/find` caller we didn't enumerate. Keeping the default as-is and opting\n**out** only where we know `_source` is unused is smaller, safer, and\nbackport-friendly.\n\n## Test plan\n\n- [ ] `AlertsClient` find/get/update/bulkUpdate unit tests pass (per-hit\nauthz now covers every unique rule type + consumer pair; dedup verified)\n- [ ] `StatsQuery` unit test asserts the alert query runs with `_source:\nfalse`\n- [ ] Analyzer prevalence (`resolver/tree` with `includeHits: true`) no\nlonger returns oversized responses\n\nCo-authored-by: Cursor <cursoragent@cursor.com>","sha":"f2748c6aaece272b694d05b66657ad7bd3cbb389"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.6.0","branchLabelMappingKey":"^v9.6.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/285953","number":285953,"mergeCommit":{"message":"[RAC] Disable _source for Analyzer alert stats query (#285953)\n\n## Summary\n\nThe Security Analyzer prevalence query (`StatsQuery.alertStatsQuery`)\nfetches up to `size: 5000` alert hits but only consumes aggregations and\nhit ids — it never reads `_source`. Because Elasticsearch defaults\n`_source` to `true`, the response could exceed\n`elasticsearch.maxResponseSize`, and Kibana aborted the request.\n\nThis sets `_source: false` on that one query. No default is changed, so\nevery other `find()` caller keeps its current behavior — the blast\nradius is limited to the Analyzer path.\n\nBecause `AlertsClient.find` runs a per-hit authorization check on every\nresult, disabling `_source` would otherwise leave `ensureAllAuthorized`\nunable to read the rule type id and consumer from the hits. So\n`ensureAllAuthorized` now reads those from the `fields` API (already\nrequested by `searchAlerts`), falling back to `_source` for `mget`\nresponses (which don't support `fields`). The same change:\n\n- Replaces the broken `reduce` — which rebuilt `ownersAndRuleTypeIds` as\na single-element array each iteration, so only the **last** hit was ever\nauthorized — with a `forEach` that collects every hit id and\ndeduplicates `(ruleTypeId, consumer)` pairs before calling\n`ensureAuthorized`.\n- Hardens `isValidAlert`'s `fields` access with optional chaining, since\nwith `_source: false` validation reads from `fields`.\n\n## Why not default `_source: false` globally\n\nAn earlier iteration flipped the `searchAlerts` default to `false` and\nmade each caller opt back in. That required touching ~7 consumers across\nsecurity/o11y/apm and risked silently breaking any `find()` / HTTP\n`/find` caller we didn't enumerate. Keeping the default as-is and opting\n**out** only where we know `_source` is unused is smaller, safer, and\nbackport-friendly.\n\n## Test plan\n\n- [ ] `AlertsClient` find/get/update/bulkUpdate unit tests pass (per-hit\nauthz now covers every unique rule type + consumer pair; dedup verified)\n- [ ] `StatsQuery` unit test asserts the alert query runs with `_source:\nfalse`\n- [ ] Analyzer prevalence (`resolver/tree` with `includeHits: true`) no\nlonger returns oversized responses\n\nCo-authored-by: Cursor <cursoragent@cursor.com>","sha":"f2748c6aaece272b694d05b66657ad7bd3cbb389"}}]}] BACKPORT--> Co-authored-by: Gerard Soldevila <gerard.soldevila@elastic.co> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent f6a2688 commit b084eae

5 files changed

Lines changed: 159 additions & 39 deletions

File tree

‎x-pack/platform/plugins/shared/rule_registry/server/alert_data_client/alerts_client.ts‎

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,37 @@ const isValidAlert = (source?: estypes.SearchHit<ParsedTechnicalFields>): source
9595
(source?._source?.[ALERT_RULE_TYPE_ID] != null &&
9696
source?._source?.[ALERT_RULE_CONSUMER] != null &&
9797
source?._source?.[SPACE_IDS] != null) ||
98-
(source?.fields?.[ALERT_RULE_TYPE_ID][0] != null &&
99-
source?.fields?.[ALERT_RULE_CONSUMER][0] != null &&
100-
source?.fields?.[SPACE_IDS][0] != null)
98+
(source?.fields?.[ALERT_RULE_TYPE_ID]?.[0] != null &&
99+
source?.fields?.[ALERT_RULE_CONSUMER]?.[0] != null &&
100+
source?.fields?.[SPACE_IDS]?.[0] != null)
101101
);
102102
};
103103

104+
/**
105+
* Reads an authorization field from an alert hit, preferring the `fields` API (which is
106+
* populated even when `_source` is disabled) and falling back to `_source` for `mget`
107+
* responses, which do not support `fields`.
108+
*/
109+
const getAlertAuthField = (
110+
hit:
111+
| {
112+
fields?: Record<string, unknown[]>;
113+
_source?: {
114+
[ALERT_RULE_TYPE_ID]?: string | null;
115+
[ALERT_RULE_CONSUMER]?: string | null;
116+
} | null;
117+
}
118+
| undefined,
119+
field: typeof ALERT_RULE_TYPE_ID | typeof ALERT_RULE_CONSUMER
120+
): string | undefined => {
121+
const fromFields = hit?.fields?.[field]?.[0];
122+
if (fromFields != null) {
123+
return String(fromFields);
124+
}
125+
const fromSource = hit?._source?.[field];
126+
return fromSource == null ? undefined : String(fromSource);
127+
};
128+
104129
export interface ConstructorOptions {
105130
logger: Logger;
106131
authorization: PublicMethodsOf<AlertingAuthorization>;
@@ -281,47 +306,39 @@ export class AlertsClient {
281306
items: Array<{
282307
_id: string;
283308
// this is typed kind of crazy to fit the output of es api response to this
309+
fields?: Record<string, unknown[]>;
284310
_source?: {
285311
[ALERT_RULE_TYPE_ID]?: string | null;
286312
[ALERT_RULE_CONSUMER]?: string | null;
287313
} | null;
288314
}>,
289315
operation: ReadOperations.Find | ReadOperations.Get | WriteOperations.Update
290316
) {
291-
const { hitIds, ownersAndRuleTypeIds } = items.reduce(
292-
(acc, hit) => ({
293-
hitIds: [hit._id, ...acc.hitIds],
294-
ownersAndRuleTypeIds: [
295-
{
296-
[ALERT_RULE_TYPE_ID]: hit?._source?.[ALERT_RULE_TYPE_ID],
297-
[ALERT_RULE_CONSUMER]: hit?._source?.[ALERT_RULE_CONSUMER],
298-
},
299-
],
300-
}),
301-
{ hitIds: [], ownersAndRuleTypeIds: [] } as {
302-
hitIds: string[];
303-
ownersAndRuleTypeIds: Array<{
304-
[ALERT_RULE_TYPE_ID]?: string | null;
305-
[ALERT_RULE_CONSUMER]?: string | null;
306-
}>;
307-
}
308-
);
317+
const hitIds: string[] = [];
318+
// Deduplicate authorization checks: authorization is granted per (ruleTypeId, consumer)
319+
// pair, so we only need to call `ensureAuthorized` once per unique pair.
320+
const ownersAndRuleTypeIds = new Map<string, { ruleTypeId: string; consumer: string }>();
309321

310-
const assertString = (hit: unknown): hit is string => hit !== null && hit !== undefined;
322+
items.forEach((hit) => {
323+
hitIds.push(hit._id);
324+
325+
const ruleTypeId = getAlertAuthField(hit, ALERT_RULE_TYPE_ID);
326+
const consumer = getAlertAuthField(hit, ALERT_RULE_CONSUMER);
327+
328+
if (ruleTypeId != null && consumer != null) {
329+
ownersAndRuleTypeIds.set(`${ruleTypeId}|${consumer}`, { ruleTypeId, consumer });
330+
}
331+
});
311332

312333
return Promise.all(
313-
ownersAndRuleTypeIds.map((hit) => {
314-
const alertOwner = hit?.[ALERT_RULE_CONSUMER];
315-
const ruleId = hit?.[ALERT_RULE_TYPE_ID];
316-
if (hit != null && assertString(alertOwner) && assertString(ruleId)) {
317-
return this.authorization.ensureAuthorized({
318-
ruleTypeId: ruleId,
319-
consumer: alertOwner,
320-
operation,
321-
entity: AlertingAuthorizationEntity.Alert,
322-
});
323-
}
324-
})
334+
Array.from(ownersAndRuleTypeIds.values()).map(({ ruleTypeId, consumer }) =>
335+
this.authorization.ensureAuthorized({
336+
ruleTypeId,
337+
consumer,
338+
operation,
339+
entity: AlertingAuthorizationEntity.Alert,
340+
})
341+
)
325342
).catch((error) => {
326343
for (const hitId of hitIds) {
327344
this.auditLogger?.log(

‎x-pack/platform/plugins/shared/rule_registry/server/alert_data_client/tests/bulk_update.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ describe('bulkUpdate()', () => {
213213
).rejects.toThrowErrorMatchingInlineSnapshot(`"Unauthorized for fake.rule and apm"`);
214214
expect(auditLogger.log).toHaveBeenCalledTimes(2);
215215
expect(auditLogger.log).toHaveBeenNthCalledWith(1, {
216-
message: `Failed attempt to update alert [id=${unsuccessfulAuthzHit}]`,
216+
message: `Failed attempt to update alert [id=${successfulAuthzHit}]`,
217217
event: {
218218
action: 'alert_update',
219219
category: ['database'],
@@ -226,7 +226,7 @@ describe('bulkUpdate()', () => {
226226
},
227227
});
228228
expect(auditLogger.log).toHaveBeenNthCalledWith(2, {
229-
message: `Failed attempt to update alert [id=${successfulAuthzHit}]`,
229+
message: `Failed attempt to update alert [id=${unsuccessfulAuthzHit}]`,
230230
event: {
231231
action: 'alert_update',
232232
category: ['database'],
@@ -410,7 +410,7 @@ describe('bulkUpdate()', () => {
410410

411411
expect(auditLogger.log).toHaveBeenCalledTimes(2);
412412
expect(auditLogger.log).toHaveBeenNthCalledWith(1, {
413-
message: `Failed attempt to update alert [id=${unsuccessfulAuthzHit}]`,
413+
message: `Failed attempt to update alert [id=${successfulAuthzHit}]`,
414414
event: {
415415
action: 'alert_update',
416416
category: ['database'],
@@ -423,7 +423,7 @@ describe('bulkUpdate()', () => {
423423
},
424424
});
425425
expect(auditLogger.log).toHaveBeenNthCalledWith(2, {
426-
message: `Failed attempt to update alert [id=${successfulAuthzHit}]`,
426+
message: `Failed attempt to update alert [id=${unsuccessfulAuthzHit}]`,
427427
event: {
428428
action: 'alert_update',
429429
category: ['database'],

‎x-pack/platform/plugins/shared/rule_registry/server/alert_data_client/tests/find_alerts.test.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,4 +778,65 @@ describe('find()', () => {
778778
`);
779779
});
780780
});
781+
782+
describe('per-hit authorization', () => {
783+
// Simulates an ES response for a `_source: false` search: authorization fields are only
784+
// available via the `fields` API, not `_source`.
785+
const buildFieldsHit = (id: string, ruleTypeId: string, consumer: string) => ({
786+
_index: '.alerts-observability.apm.alerts',
787+
_id: id,
788+
fields: {
789+
[ALERT_RULE_TYPE_ID]: [ruleTypeId],
790+
[ALERT_RULE_CONSUMER]: [consumer],
791+
[ALERT_WORKFLOW_STATUS]: ['open'],
792+
[SPACE_IDS]: [DEFAULT_SPACE],
793+
},
794+
});
795+
796+
const mockFieldsResponse = (hits: Array<ReturnType<typeof buildFieldsHit>>) =>
797+
esClientMock.search.mockResponseOnce({
798+
took: 5,
799+
timed_out: false,
800+
_shards: { total: 1, successful: 1, failed: 0, skipped: 0 },
801+
hits: { total: hits.length, max_score: 999, hits },
802+
});
803+
804+
test('authorizes every unique rule type and consumer pair read from the `fields` API', async () => {
805+
const alertsClient = new AlertsClient(alertsClientParams);
806+
mockFieldsResponse([
807+
buildFieldsHit('authorized', 'apm.error_rate', 'apm'),
808+
buildFieldsHit('unauthorized', 'fake.rule', 'apm'),
809+
]);
810+
811+
await expect(
812+
alertsClient.find({
813+
query: { match: { [ALERT_WORKFLOW_STATUS]: 'open' } },
814+
index: '.alerts-observability.apm.alerts',
815+
})
816+
).rejects.toThrow('Unauthorized for fake.rule and apm');
817+
818+
expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledTimes(2);
819+
expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith(
820+
expect.objectContaining({ ruleTypeId: 'apm.error_rate', consumer: 'apm' })
821+
);
822+
expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith(
823+
expect.objectContaining({ ruleTypeId: 'fake.rule', consumer: 'apm' })
824+
);
825+
});
826+
827+
test('deduplicates authorization checks for repeated rule type and consumer pairs', async () => {
828+
const alertsClient = new AlertsClient(alertsClientParams);
829+
mockFieldsResponse([
830+
buildFieldsHit('1', 'apm.error_rate', 'apm'),
831+
buildFieldsHit('2', 'apm.error_rate', 'apm'),
832+
]);
833+
834+
await alertsClient.find({
835+
query: { match: { [ALERT_WORKFLOW_STATUS]: 'open' } },
836+
index: '.alerts-observability.apm.alerts',
837+
});
838+
839+
expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledTimes(1);
840+
});
841+
});
781842
});

‎x-pack/solutions/security/plugins/security_solution/server/endpoint/routes/resolver/tree/queries/stats.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* 2.0.
66
*/
77

8+
import type { IScopedClusterClient } from '@kbn/core/server';
9+
import type { AlertsClient } from '@kbn/rule-registry-plugin/server';
810
import { StatsQuery } from './stats';
911

1012
describe('StatsQuery', () => {
@@ -83,4 +85,40 @@ describe('StatsQuery', () => {
8385

8486
expect(query.getRangeFilter()).toEqual([]);
8587
});
88+
89+
describe('search', () => {
90+
const buildScopedClient = () =>
91+
({
92+
asCurrentUser: {
93+
search: jest.fn().mockResolvedValue({ aggregations: { ids: { buckets: [] } } }),
94+
},
95+
asInternalUser: {
96+
search: jest.fn().mockResolvedValue({ aggregations: { ids: { buckets: [] } } }),
97+
},
98+
} as unknown as IScopedClusterClient);
99+
100+
const buildAlertsClient = () =>
101+
({
102+
find: jest
103+
.fn()
104+
.mockResolvedValue({ aggregations: { ids: { buckets: [] } }, hits: { hits: [] } }),
105+
} as unknown as jest.Mocked<AlertsClient>);
106+
107+
it('requests alerts without `_source` so large hit responses are not buffered', async () => {
108+
const query = new StatsQuery({
109+
schema,
110+
indexPatterns: ['.alerts-security.alerts-default'],
111+
timeRange: undefined,
112+
isInternalRequest: true,
113+
agentId: undefined,
114+
});
115+
const alertsClient = buildAlertsClient();
116+
117+
await query.search(buildScopedClient(), ['node-1'], alertsClient, true);
118+
119+
expect(alertsClient.find).toHaveBeenCalledWith(
120+
expect.objectContaining({ _source: false, size: 5000 })
121+
);
122+
});
123+
});
86124
});

‎x-pack/solutions/security/plugins/security_solution/server/endpoint/routes/resolver/tree/queries/stats.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,13 @@ export class StatsQuery extends BaseResolverQuery {
9595
nodes: NodeID[],
9696
index: string,
9797
includeHits: boolean
98-
): { size: number; query: object; index: string; aggs: object; fields?: string[] } {
98+
): { size: number; query: object; index: string; aggs: object; _source: false } {
9999
return {
100100
size: includeHits ? 5000 : 0,
101+
// Only alert counts (aggregations) and ids are consumed from this response, never the
102+
// document bodies. Disabling `_source` keeps the response small enough to avoid
103+
// exceeding `elasticsearch.maxResponseSize` when `includeHits` pulls up to 5000 hits.
104+
_source: false,
101105
query: {
102106
bool: {
103107
filter: [

0 commit comments

Comments
 (0)