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] Prebuilt Rule Marked as Customized By Just Clicking "Edit rule settings" And Clicking Save Without Any Changes #203315

Open
Tracked by #201502
pborgonovi opened this issue Dec 6, 2024 · 13 comments
Assignees
Labels
8.18 candidate bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.17.1 v8.18.0 v9.0.0

Comments

@pborgonovi
Copy link
Contributor

pborgonovi commented Dec 6, 2024

Describe the bug:

When editing a prebuilt rule which has an update available through the Edit Rule Settings option without making any changes and clicking Save, the rule is incorrectly marked as “Customized.”

PS: Issue just happens when the rule has an update available

Kibana/Elasticsearch Stack version:

8.x

Functional Area (e.g. Endpoint management, timelines, resolver, etc.):

Prebuilt Rules

Pre requisites:

  1. prebuiltRulesCustomizationEnabled flag is enabled
  2. Prebuilt rules are installed
  3. Rule updates are available

Steps to reproduce:

  1. Navigate to the Rules Management page.
  2. Select a Prebuilt Rule which has an update available
  3. Click on Edit Rule Settings.
  4. Without making any changes, click Save.

Current behavior:

  • The rule is incorrectly marked as Customized.
  • The is_customized flag is set to true in the API response, even though no changes were made.

Expected behavior:

  • If no changes are made during the edit process, the rule should remain in its non-customized state (is_customized: false).
  • Saving without changes should not alter the rule’s customization status.

Evidences:

Rule has an update available: incorrectly set as is_customized=true

Screen.Recording.2024-12-06.at.10.29.50.AM.mov

Rule doesn't have an update available: correctly kept as is_customized=false

Screen.Recording.2024-12-06.at.10.30.48.AM.mov
@pborgonovi pborgonovi added bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team triage_needed labels Dec 6, 2024
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@banderror
Copy link
Contributor

@dplumlee Could you please check this one too?

@dplumlee
Copy link
Contributor

dplumlee commented Dec 6, 2024

Do you happen to know what rule package you have installed for this @pborgonovi? Going through these tickets, I'm pretty sure the bug is coming from this line of code when a rule doesn't have a base version to compare against. That's why it's only happening for certain rules - I'm not certain it's just due to the rule having an update. Some rules have updates AND the base version exists and this bug doesn't occur.

@banderror @xcrzx any thoughts on how to solve this? I would imagine it gets fixed when the full package history is implemented but maybe having our fallback be labeling the rule as modified needs additional edge cases? Still thinking through it.

@pborgonovi
Copy link
Contributor Author

@dplumlee When I ran these tests I had package version 8.15.2 installed

@banderror
Copy link
Contributor

I'm pretty sure the bug is coming from this line of code when a rule doesn't have a base version to compare against. That's why it's only happening for certain rules

@dplumlee Do we have evidence for this? Can we reliably reproduce this bug for such rules? Can we reliably see that it doesn't occur for the other group? We need to know for sure.

Also, we have a test 8.16.2-beta.1 package that contain all historical rule versions. It could be used to prove this theory.

@dplumlee
Copy link
Contributor

dplumlee commented Dec 9, 2024

@banderror yeah that's what I was using to get to this conclusion. The bug is definitely happening when rules don't have the base version, I've done some more thorough testing this morning.

But when I take the following steps

  1. Start kibana with lower package version than 8.16.2-beta.1 with prebuiltRulesCustomizationEnabled FF on
  2. Install all rules
  3. Update package version to 8.16.2-beta.1
  4. Follow the steps to repro this ticket

I am unable to find a rule that gets marked as "Modified". If I go from, say, package version 8.15.2 to 8.16.2, I am able to recreate this bug for certain rules including the specific ones in the ticket. That's why I'd imagine this will be solved when we switch to the beta package fully.

@banderror
Copy link
Contributor

@dplumlee The new release package with historical versions for Kibana 8.17.0 is going to be released very soon, probably today. This should fix this bug, in general.

But I would like us to also revisit this logic. Even when we have historical versions for all rules in general, what if for some reason (such as a bug in the package build process, or reaching the historical versions limit) some rule won't have a base version. Then, if user edits it as described in this ticket, or adds actions to it, it will be marked as customized, which IMO looks very confusing.

@jpdjere @xcrzx @maximpn What do you think about it? Should we change this logic to be marking rules as not customized when base version is missing? Should we mark them as customized only in some cases, but not when the user edits a rule? Or should we keep it as is?

The original reasoning from the RFC was:

Image

@maximpn
Copy link
Contributor

maximpn commented Dec 10, 2024

We can try to mitigate this issue. Rule's SO has immutable and since some moment rule_source fields. It gives us information about rule's source. It means we can use that fields as a hint when base version is missing.

My suggestion is extending is_customized calculation functionality for a missing base version case. When rule_source is external or immutable is true (when the first is missing) compare rule fields upon saving a rule to detect if something's changed.

@xcrzx
Copy link
Contributor

xcrzx commented Dec 12, 2024

My suggestion is extending is_customized calculation functionality for a missing base version case. When rule_source is external or immutable is true (when the first is missing) compare rule fields upon saving a rule to detect if something's changed.

This approach wouldn’t work in the general case. When the rule is not customized, we can determine its status as customized the first time it’s saved by comparing it to itself. However, in subsequent cases, such as saving it a second time after customization, calculating isCustomized would no longer be feasible.

@dplumlee
Copy link
Contributor

dplumlee commented Dec 13, 2024

However, in subsequent cases, such as saving it a second time after customization, calculating isCustomized would no longer be feasible

This is a good point.

There's a few options I can think of for how to deal with this:

  1. We can leave this logic as is and, depending on @xcrzx's testing of the new rules package, this could be a very rare situation for a user to find themselves in. Even if we fail to properly fetch the base version, the next time the rule is interacted with we'd probably successfully fetch it and it would fix itself. This option is obviously the easiest and depends on the rules package being very reliable, but the existing weird fallback would still exist.
  2. We can ignore the isCustomized calculation when the base version doesn't exist and simply keep whatever the rule had originally. This would also be pretty easy to implement but would go against some of the arguments made in the RFC @banderror posted above, most notably the likelihood that the rule being customized is far more likely than it being non-customized.
  3. We can keep the isCustomized calculation the same but in the update rule API endpoint, we simply exit early if the payload rule is the same as the saved object - skipping the extra calculations altogether. This would mainly be to fix this exact bug and wouldn't provide much other use for the amount of work it would take.

I'm leaning towards option 1 as it would take no work and I wasn't able to reproduce this bug at all in some, admittedly cursory, testing with the beta rule package. If @xcrzx isn't able to find any missing versions problems in his package testing I think I'd categorize it as an extremely niche edge case. I'm not sure if there's a better fallback option for the UI in this case short of also showing that the base version is missing which I don't think we'd want to expose to the user

@banderror
Copy link
Contributor

@approksiu I'd be interested in your opinion on the 3 options above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.18 candidate bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.17.1 v8.18.0 v9.0.0
Projects
None yet
Development

No branches or pull requests

6 participants