Skip to content

[Security Solution][Detection Engine] Fix alert count so max alerts warning shows correctly#259199

Merged
marshallmain merged 6 commits intoelastic:mainfrom
marshallmain:fix-max-alerts-im
Mar 26, 2026
Merged

[Security Solution][Detection Engine] Fix alert count so max alerts warning shows correctly#259199
marshallmain merged 6 commits intoelastic:mainfrom
marshallmain:fix-max-alerts-im

Conversation

@marshallmain
Copy link
Copy Markdown
Contributor

@marshallmain marshallmain commented Mar 23, 2026

Fixes #259169

createEventSignal has a bug where it returns incorrect summary results for pages of source docs that matched no indicators. The calling code expects createEventSignal to return results pertaining only to the current page, but if no indicators are matched or an error is encountered, the function instead returns currentResults i.e. the sum of results from all prior pages. The effect is that each time a page matches no indicators the alert count we track in createThreatSignals doubles because we add currentResults to itself.

@marshallmain marshallmain force-pushed the fix-max-alerts-im branch 2 times, most recently from 1b8fad7 to af055cf Compare March 23, 2026 20:04
@elasticmachine

This comment was marked as outdated.

@marshallmain marshallmain marked this pull request as ready for review March 24, 2026 17:12
@marshallmain marshallmain requested a review from a team as a code owner March 24, 2026 17:12
@marshallmain marshallmain requested a review from rylnd March 24, 2026 17:12
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-detection-engine (Team:Detection Engine)

@marshallmain marshallmain added Feature:Indicator Match Rule Security Solution Indicator Match rule type backport:version Backport to applied version labels labels Mar 24, 2026
@nkhristinin nkhristinin self-requested a review March 25, 2026 15:18
Copy link
Copy Markdown
Contributor

@nkhristinin nkhristinin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fix, it LGTM!

But I have question about create_threat_signal.ts, we right now passing there currentResult and there case where we return it

    // empty threat list and we do not want to return everything as being
    // a hit so opt to return the existing result.
    ruleExecutionLogger.trace(
      'Indicator items are empty after filtering for missing data, returning without attempting a match'
    );
    return currentResult;
  }

Should we remove it, and add another use case for threat first search?

@marshallmain
Copy link
Copy Markdown
Contributor Author

Should we remove it, and add another use case for threat first search?

Good point, yeah I'll fix that one as well in this PR and add another test.

@marshallmain
Copy link
Copy Markdown
Contributor Author

@nkhristinin I added the suggested fix for threat-first search as well, and a similar test for that scenario. The test already passes without the fix that removes currentResults because we have an extra filter that removes indicator documents that are missing the required threat mapping fields, so (at least for this test case) it seems that we should never go down the path that returned currentResult. But if you remove that filter and run the test case on the pre-fix code then the test fails due to the max alerts warning being emitted.

@nkhristinin nkhristinin self-requested a review March 25, 2026 18:28
Copy link
Copy Markdown
Contributor

@nkhristinin nkhristinin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for fixing it!

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #181 / console app console autocomplete feature Autocomplete shouldnt trigger within a multiline block comment
  • [job] [logs] affected Scout: [ security / security_solution ] plugin / local-serverless-security_complete - Expandable flyout state sync - should test flyout url sync

Metrics [docs]

✅ unchanged

History

@marshallmain marshallmain merged commit 22c93a4 into elastic:main Mar 26, 2026
18 checks passed
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 9.2, 9.3

https://github.com/elastic/kibana/actions/runs/23601384010

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 26, 2026
…arning shows correctly (elastic#259199)

Fixes elastic#259169

`createEventSignal` has a bug where it returns incorrect summary results
for pages of source docs that matched no indicators. The calling code
expects `createEventSignal` to return results pertaining only to the
current page, but if no indicators are matched or an error is
encountered, the function instead returns `currentResults` i.e. the sum
of results from all prior pages. The effect is that each time a page
matches no indicators the alert count we track in `createThreatSignals`
_doubles_ because we add `currentResults` to itself.

(cherry picked from commit 22c93a4)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 26, 2026
…arning shows correctly (elastic#259199)

Fixes elastic#259169

`createEventSignal` has a bug where it returns incorrect summary results
for pages of source docs that matched no indicators. The calling code
expects `createEventSignal` to return results pertaining only to the
current page, but if no indicators are matched or an error is
encountered, the function instead returns `currentResults` i.e. the sum
of results from all prior pages. The effect is that each time a page
matches no indicators the alert count we track in `createThreatSignals`
_doubles_ because we add `currentResults` to itself.

(cherry picked from commit 22c93a4)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
9.2
9.3

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Mar 27, 2026
…erts warning shows correctly (#259199) (#259805)

# Backport

This will backport the following commits from `main` to `9.3`:
- [[Security Solution][Detection Engine] Fix alert count so max alerts
warning shows correctly
(#259199)](#259199)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Marshall
Main","email":"55718608+marshallmain@users.noreply.github.com"},"sourceCommit":{"committedDate":"2026-03-26T15:00:15Z","message":"[Security
Solution][Detection Engine] Fix alert count so max alerts warning shows
correctly (#259199)\n\nFixes
https://github.com/elastic/kibana/issues/259169\n\n`createEventSignal`
has a bug where it returns incorrect summary results\nfor pages of
source docs that matched no indicators. The calling code\nexpects
`createEventSignal` to return results pertaining only to the\ncurrent
page, but if no indicators are matched or an error is\nencountered, the
function instead returns `currentResults` i.e. the sum\nof results from
all prior pages. The effect is that each time a page\nmatches no
indicators the alert count we track in `createThreatSignals`\n_doubles_
because we add `currentResults` to
itself.","sha":"22c93a4b288217542614978c098814c6157f3b4d","branchLabelMapping":{"^v9.4.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Indicator
Match Rule","Team:Detection
Engine","backport:version","v9.4.0","9.4.0","v9.3.3","v9.2.8"],"title":"[Security
Solution][Detection Engine] Fix alert count so max alerts warning shows
correctly","number":259199,"url":"https://github.com/elastic/kibana/pull/259199","mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix alert count so max alerts warning shows
correctly (#259199)\n\nFixes
https://github.com/elastic/kibana/issues/259169\n\n`createEventSignal`
has a bug where it returns incorrect summary results\nfor pages of
source docs that matched no indicators. The calling code\nexpects
`createEventSignal` to return results pertaining only to the\ncurrent
page, but if no indicators are matched or an error is\nencountered, the
function instead returns `currentResults` i.e. the sum\nof results from
all prior pages. The effect is that each time a page\nmatches no
indicators the alert count we track in `createThreatSignals`\n_doubles_
because we add `currentResults` to
itself.","sha":"22c93a4b288217542614978c098814c6157f3b4d"}},"sourceBranch":"main","suggestedTargetBranches":["9.3","9.2"],"targetPullRequestStates":[{"branch":"main","label":"v9.4.0","branchLabelMappingKey":"^v9.4.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/259199","number":259199,"mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix alert count so max alerts warning shows
correctly (#259199)\n\nFixes
https://github.com/elastic/kibana/issues/259169\n\n`createEventSignal`
has a bug where it returns incorrect summary results\nfor pages of
source docs that matched no indicators. The calling code\nexpects
`createEventSignal` to return results pertaining only to the\ncurrent
page, but if no indicators are matched or an error is\nencountered, the
function instead returns `currentResults` i.e. the sum\nof results from
all prior pages. The effect is that each time a page\nmatches no
indicators the alert count we track in `createThreatSignals`\n_doubles_
because we add `currentResults` to
itself.","sha":"22c93a4b288217542614978c098814c6157f3b4d"}},{"branch":"9.3","label":"v9.3.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.2","label":"v9.2.8","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marshall Main <55718608+marshallmain@users.noreply.github.com>
kibanamachine added a commit that referenced this pull request Mar 27, 2026
…erts warning shows correctly (#259199) (#259804)

# Backport

This will backport the following commits from `main` to `9.2`:
- [[Security Solution][Detection Engine] Fix alert count so max alerts
warning shows correctly
(#259199)](#259199)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Marshall
Main","email":"55718608+marshallmain@users.noreply.github.com"},"sourceCommit":{"committedDate":"2026-03-26T15:00:15Z","message":"[Security
Solution][Detection Engine] Fix alert count so max alerts warning shows
correctly (#259199)\n\nFixes
https://github.com/elastic/kibana/issues/259169\n\n`createEventSignal`
has a bug where it returns incorrect summary results\nfor pages of
source docs that matched no indicators. The calling code\nexpects
`createEventSignal` to return results pertaining only to the\ncurrent
page, but if no indicators are matched or an error is\nencountered, the
function instead returns `currentResults` i.e. the sum\nof results from
all prior pages. The effect is that each time a page\nmatches no
indicators the alert count we track in `createThreatSignals`\n_doubles_
because we add `currentResults` to
itself.","sha":"22c93a4b288217542614978c098814c6157f3b4d","branchLabelMapping":{"^v9.4.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Indicator
Match Rule","Team:Detection
Engine","backport:version","v9.4.0","9.4.0","v9.3.3","v9.2.8"],"title":"[Security
Solution][Detection Engine] Fix alert count so max alerts warning shows
correctly","number":259199,"url":"https://github.com/elastic/kibana/pull/259199","mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix alert count so max alerts warning shows
correctly (#259199)\n\nFixes
https://github.com/elastic/kibana/issues/259169\n\n`createEventSignal`
has a bug where it returns incorrect summary results\nfor pages of
source docs that matched no indicators. The calling code\nexpects
`createEventSignal` to return results pertaining only to the\ncurrent
page, but if no indicators are matched or an error is\nencountered, the
function instead returns `currentResults` i.e. the sum\nof results from
all prior pages. The effect is that each time a page\nmatches no
indicators the alert count we track in `createThreatSignals`\n_doubles_
because we add `currentResults` to
itself.","sha":"22c93a4b288217542614978c098814c6157f3b4d"}},"sourceBranch":"main","suggestedTargetBranches":["9.3","9.2"],"targetPullRequestStates":[{"branch":"main","label":"v9.4.0","branchLabelMappingKey":"^v9.4.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/259199","number":259199,"mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix alert count so max alerts warning shows
correctly (#259199)\n\nFixes
https://github.com/elastic/kibana/issues/259169\n\n`createEventSignal`
has a bug where it returns incorrect summary results\nfor pages of
source docs that matched no indicators. The calling code\nexpects
`createEventSignal` to return results pertaining only to the\ncurrent
page, but if no indicators are matched or an error is\nencountered, the
function instead returns `currentResults` i.e. the sum\nof results from
all prior pages. The effect is that each time a page\nmatches no
indicators the alert count we track in `createThreatSignals`\n_doubles_
because we add `currentResults` to
itself.","sha":"22c93a4b288217542614978c098814c6157f3b4d"}},{"branch":"9.3","label":"v9.3.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.2","label":"v9.2.8","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marshall Main <55718608+marshallmain@users.noreply.github.com>
jeramysoucy pushed a commit to jeramysoucy/kibana that referenced this pull request Apr 1, 2026
…arning shows correctly (elastic#259199)

Fixes elastic#259169

`createEventSignal` has a bug where it returns incorrect summary results
for pages of source docs that matched no indicators. The calling code
expects `createEventSignal` to return results pertaining only to the
current page, but if no indicators are matched or an error is
encountered, the function instead returns `currentResults` i.e. the sum
of results from all prior pages. The effect is that each time a page
matches no indicators the alert count we track in `createThreatSignals`
_doubles_ because we add `currentResults` to itself.
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Apr 2, 2026
…arning shows correctly (elastic#259199)

Fixes elastic#259169

`createEventSignal` has a bug where it returns incorrect summary results
for pages of source docs that matched no indicators. The calling code
expects `createEventSignal` to return results pertaining only to the
current page, but if no indicators are matched or an error is
encountered, the function instead returns `currentResults` i.e. the sum
of results from all prior pages. The effect is that each time a page
matches no indicators the alert count we track in `createThreatSignals`
_doubles_ because we add `currentResults` to itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels Feature:Indicator Match Rule Security Solution Indicator Match rule type release_note:fix Team:Detection Engine Security Solution Detection Engine Area v9.2.8 v9.3.3 v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security Solution][Detection Engine] Indicator match rule reports max alerts warning but creates fewer alerts

4 participants