Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Alerts] adds suppression for missing fields options for security rules #155055

Merged

Conversation

vitaliidm
Copy link
Contributor

@vitaliidm vitaliidm commented Apr 17, 2023

Summary

UX changes

Rule edit page

Accordion closed Screenshot 2023-04-21 at 16 09 44
Accordion opened Screenshot 2023-04-24 at 19 44 33

Rule Details page

Screenshot 2023-04-19 at 18 50 13

Checklist

Delete any items that are not applicable to this PR.

@vitaliidm vitaliidm self-assigned this Apr 20, 2023
@vitaliidm vitaliidm added backport:skip This commit does not require backporting Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Detection Alerts Security Solution Detection Alerts Feature release_note:feature Makes this part of the condensed release notes Team:Detection Alerts Security Detection Alerts Area Team v8.8.0 release_note:enhancement and removed release_note:feature Makes this part of the condensed release notes labels Apr 20, 2023
@vitaliidm vitaliidm marked this pull request as ready for review April 24, 2023 13:06
@vitaliidm vitaliidm requested review from a team as code owners April 24, 2023 13:06
@vitaliidm vitaliidm requested a review from maximpn April 24, 2023 13:06
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@vitaliidm vitaliidm requested a review from marshallmain April 24, 2023 13:07
@elastic elastic deleted a comment from kibana-ci Apr 24, 2023
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Rules are changes LGTM 👍 Left just a few nits.

@@ -32,3 +34,6 @@ export const PREBUILT_RULES_PACKAGE_NAME = 'security_detection_engine';
* Rule signature id (`rule.rule_id`) of the prebuilt "Endpoint Security" rule.
*/
export const ELASTIC_SECURITY_RULE_ID = '9a1a2dae-0b5f-4c3d-8305-a268d404c306';

export const DEFAULT_SUPPRESSION_MISSING_FIELDS_STRATEGY: AlertSuppressionMissingFields =
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We could improve cohesion by keeping it close to where the type is defined: in ./rule_schema/model/specific_attributes/query_attributes:

/**
 * describes how alerts will be generated for documents with missing suppress by fields
 */
export enum AlertSuppressionMissingFieldsStrategy {
  // per each document a separate alert will be created
  DoNotSuppress = 'doNotSuppress',
  // only alert will be created per suppress by bucket
  Suppress = 'suppress',
}

export const ALERT_SUPPRESSION_MISSING_FIELDS_DEFAULT_STRATEGY = AlertSuppressionMissingFields.Suppress;

Comment on lines 24 to 28
export type AlertSuppressionMissingFields = t.TypeOf<typeof AlertSuppressionMissingFields>;
export const AlertSuppressionMissingFields = t.union([
t.literal('doNotSuppress'),
t.literal('suppress'),
]);
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a handy helper for generating schemas from enums:

import { enumeration } from '@kbn/securitysolution-io-ts-types';

export type AlertSuppressionMissingFields = t.TypeOf<typeof AlertSuppressionMissingFields>;
export const AlertSuppressionMissingFields = enumeration('AlertSuppressionMissingFields', AlertSuppressionMissingFieldsStrategy);

@vitaliidm vitaliidm enabled auto-merge (squash) April 25, 2023 14:01
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3857 3858 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 9.1MB 9.1MB +2.7KB
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 17 19 +2
securitySolution 397 400 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 18 20 +2
securitySolution 477 480 +3
total +5

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @vitaliidm

Copy link
Contributor

@marshallmain marshallmain left a comment

Choose a reason for hiding this comment

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

One minor change. Looking over the tests now, will update with any final thoughts but LGTM

}) => {
const bulkCreatedResult = await searchAfterAndBulkCreate({
tuple: { ...runOpts.tuple, maxSignals: size },
exceptionsList: runOpts.unprocessedExceptions,
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's drop the unprocessed exceptions from searchAfterBulkCreate for this case - unprocessed exceptions are only the value list exceptions where the list is too large to fit in the query. Long term we should look to remove the unlimited size value list exceptions so it's easier to just not support them at all for new features like suppression. The aggregation part of alert suppression already only supports list-based exceptions if the list is small enough for the exception to fit in the query.

Suggested change
exceptionsList: runOpts.unprocessedExceptions,
exceptionsList: [],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in this PR #155839

@vitaliidm vitaliidm merged commit 5fb93a1 into elastic:main Apr 25, 2023
const firstDoc = { id, '@timestamp': timestamp, agent: { name: 'agent-1' } };
const secondDoc = { id, '@timestamp': laterTimestamp, agent: { name: 'agent-1' } };
const thirdDoc = { id, '@timestamp': laterTimestamp, agent: { name: 'agent-2' } };
const missingFieldDoc1 = { id, '@timestamp': laterTimestamp };
Copy link
Contributor

Choose a reason for hiding this comment

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

Is one of these missingFieldDocs intended to have a different timestamp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in this PR #155839

vitaliidm added a commit that referenced this pull request May 9, 2023
…ck (#155839)

## Summary

- addresses review feedback on
#155055
- addresses UI changes from
#150101
  - removes accordion in favour of intended suppression components
  - adds popover with a link to documentation
  - changes wording
- addresses #156247

### Before
<img width="1017" alt="Screenshot 2023-04-24 at 19 44 33"
src="https://user-images.githubusercontent.com/92328789/234824612-b0ed2870-8aa0-44af-a37d-c061358c54a3.png">

### After

#### Intended Fields
<img width="1016" alt="Screenshot 2023-05-05 at 10 42 46"
src="https://user-images.githubusercontent.com/92328789/236426053-279d2f5b-46ea-434b-9cfa-696c71321661.png">

#### Tooltip
<img width="1016" alt="Screenshot 2023-05-05 at 10 43 41"
src="https://user-images.githubusercontent.com/92328789/236426061-1c39a5c2-63ca-4a36-b15e-2a1c1943481d.png">

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 9, 2023
…ck (elastic#155839)

## Summary

- addresses review feedback on
elastic#155055
- addresses UI changes from
elastic#150101
  - removes accordion in favour of intended suppression components
  - adds popover with a link to documentation
  - changes wording
- addresses elastic#156247

### Before
<img width="1017" alt="Screenshot 2023-04-24 at 19 44 33"
src="https://user-images.githubusercontent.com/92328789/234824612-b0ed2870-8aa0-44af-a37d-c061358c54a3.png">

### After

#### Intended Fields
<img width="1016" alt="Screenshot 2023-05-05 at 10 42 46"
src="https://user-images.githubusercontent.com/92328789/236426053-279d2f5b-46ea-434b-9cfa-696c71321661.png">

#### Tooltip
<img width="1016" alt="Screenshot 2023-05-05 at 10 43 41"
src="https://user-images.githubusercontent.com/92328789/236426061-1c39a5c2-63ca-4a36-b15e-2a1c1943481d.png">

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
(cherry picked from commit 31b6062)
kibanamachine added a commit that referenced this pull request May 10, 2023
…feedback (#155839) (#157192)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution][Alerts] resolves alerts suppression review
feedback (#155839)](#155839)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Vitalii
Dmyterko","email":"92328789+vitaliidm@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-05-09T16:12:20Z","message":"[Security
Solution][Alerts] resolves alerts suppression review feedback
(#155839)\n\n## Summary\r\n\r\n- addresses review feedback
on\r\nhttps://github.com//pull/155055\r\n- addresses UI
changes from\r\nhttps://github.com//issues/150101\r\n -
removes accordion in favour of intended suppression components\r\n -
adds popover with a link to documentation\r\n - changes wording\r\n-
addresses https://github.com/elastic/kibana/issues/156247\r\n\r\n###
Before\r\n<img width=\"1017\" alt=\"Screenshot 2023-04-24 at 19 44
33\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/234824612-b0ed2870-8aa0-44af-a37d-c061358c54a3.png\">\r\n\r\n###
After\r\n\r\n#### Intended Fields\r\n<img width=\"1016\"
alt=\"Screenshot 2023-05-05 at 10 42
46\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/236426053-279d2f5b-46ea-434b-9cfa-696c71321661.png\">\r\n\r\n####
Tooltip\r\n<img width=\"1016\" alt=\"Screenshot 2023-05-05 at 10 43
41\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/236426061-1c39a5c2-63ca-4a36-b15e-2a1c1943481d.png\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Garrett Spong
<spong@users.noreply.github.com>","sha":"31b6062148b55f712015fc9061172eca54c0acd4","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:
SecuritySolution","Team:Detection
Alerts","backport:prev-minor","v8.8.0","v8.9.0"],"number":155839,"url":"https://github.com/elastic/kibana/pull/155839","mergeCommit":{"message":"[Security
Solution][Alerts] resolves alerts suppression review feedback
(#155839)\n\n## Summary\r\n\r\n- addresses review feedback
on\r\nhttps://github.com//pull/155055\r\n- addresses UI
changes from\r\nhttps://github.com//issues/150101\r\n -
removes accordion in favour of intended suppression components\r\n -
adds popover with a link to documentation\r\n - changes wording\r\n-
addresses https://github.com/elastic/kibana/issues/156247\r\n\r\n###
Before\r\n<img width=\"1017\" alt=\"Screenshot 2023-04-24 at 19 44
33\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/234824612-b0ed2870-8aa0-44af-a37d-c061358c54a3.png\">\r\n\r\n###
After\r\n\r\n#### Intended Fields\r\n<img width=\"1016\"
alt=\"Screenshot 2023-05-05 at 10 42
46\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/236426053-279d2f5b-46ea-434b-9cfa-696c71321661.png\">\r\n\r\n####
Tooltip\r\n<img width=\"1016\" alt=\"Screenshot 2023-05-05 at 10 43
41\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/236426061-1c39a5c2-63ca-4a36-b15e-2a1c1943481d.png\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Garrett Spong
<spong@users.noreply.github.com>","sha":"31b6062148b55f712015fc9061172eca54c0acd4"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/155839","number":155839,"mergeCommit":{"message":"[Security
Solution][Alerts] resolves alerts suppression review feedback
(#155839)\n\n## Summary\r\n\r\n- addresses review feedback
on\r\nhttps://github.com//pull/155055\r\n- addresses UI
changes from\r\nhttps://github.com//issues/150101\r\n -
removes accordion in favour of intended suppression components\r\n -
adds popover with a link to documentation\r\n - changes wording\r\n-
addresses https://github.com/elastic/kibana/issues/156247\r\n\r\n###
Before\r\n<img width=\"1017\" alt=\"Screenshot 2023-04-24 at 19 44
33\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/234824612-b0ed2870-8aa0-44af-a37d-c061358c54a3.png\">\r\n\r\n###
After\r\n\r\n#### Intended Fields\r\n<img width=\"1016\"
alt=\"Screenshot 2023-05-05 at 10 42
46\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/236426053-279d2f5b-46ea-434b-9cfa-696c71321661.png\">\r\n\r\n####
Tooltip\r\n<img width=\"1016\" alt=\"Screenshot 2023-05-05 at 10 43
41\"\r\nsrc=\"https://user-images.githubusercontent.com/92328789/236426061-1c39a5c2-63ca-4a36-b15e-2a1c1943481d.png\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Garrett Spong
<spong@users.noreply.github.com>","sha":"31b6062148b55f712015fc9061172eca54c0acd4"}}]}]
BACKPORT-->

Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com>
Co-authored-by: Pedro Jaramillo <pedro.jaramillo@elastic.co>
@vitaliidm vitaliidm deleted the alerts_8_8/suppression_missing_fields branch March 4, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Detection Alerts Security Solution Detection Alerts Feature release_note:enhancement Team:Detection Alerts Security Detection Alerts Area Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security Solution][Alerts] Suppression rule fails after disabling and re-enabling
5 participants