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] Blocking vs non-blocking query indices/fields validation #201095

Open
maximpn opened this issue Nov 21, 2024 · 5 comments
Open
Assignees
Labels
discuss Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team:Security Solution Platform Security Solution Platform Team

Comments

@maximpn
Copy link
Contributor

maximpn commented Nov 21, 2024

Related to: #180407

Summary

There is a question on how we should handle query validation in Prebuilt Rules Customization workflow. With lack of data in ES indices/fields validation would block saving a query in Prebuilt Rules Customization workflow.

  • Do we expect it to match rule creation/editing forms behavior? It means showing a confirmation modal in case of validation errors.
  • Do we expect treating indices/fields validation as warnings? It means we could treat indices/fields validation as warnings which don't block form submission. Such warnings could be shown in yellow in a similar way Required Fields editing component does. If we go this way it's easier to have the behavior for rule editing form as well.

Details

We have different types of queries kuery, lucene, EQL and ES|QL. Each one has validation specifics but generally speaking validation could be split in two logical parts

  • semantic validation
    It's something like validation via regex, linting or building an Abstract Syntax Tree. If it fails the string is never a valid query string. Rule execution with such a query string will fail.

  • indices/fields validation
    It's a validation of indices and/or fields used in a query. Such validation fails even for valid queries when relevant data is missing in Elasticsearch.

Considering each query type separately we have the following validations specifics of indices/fields validation

  • kuery and lucene has field names validation
  • EQL query has event category and field names validation, event category override field is also important here
  • ES|QL query has indices and columns (field names) validation

Rule editing form behavior

Currently rule editing page shows a modal when there are query validation errors. But the same approach might lead to worse UX for Prebuilt Rules Customization. Attached videos demonstrate ES|QL query UX

Screen.Recording.2024-11-20.at.10.53.09.mov

Current Prebuilt Rules Customization workflow UI

Screen.Recording.2024-11-20.at.10.52.30.mov
@maximpn maximpn added discuss Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team:Security Solution Platform Security Solution Platform Team labels Nov 21, 2024
@maximpn maximpn self-assigned this Nov 21, 2024
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-engine (Team:Security Solution Platform)

@elasticmachine
Copy link
Contributor

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

@maximpn
Copy link
Contributor Author

maximpn commented Nov 21, 2024

I transfer Slack discussion here

@ARWNightingale

I think it should match, what is the poor UX for having the same behaviour. What about the Ask AI assistant? that will open a flyout over the flyout?

@xcrzx

My 2c: we shouldn’t block the save button in the rule update flyout if there are field validation errors, as this could potentially block the entire upgrade workflow. For example, if a user doesn’t use AWS rules (and therefore has no AWS-related indices), it would be impossible to resolve conflicts in those rules. This could result in ever-lingering updates that users cannot address.

@approksiu

I think the behavior should be the same as in rule editing - we should allow saving fields with data/index-related errors, but the way we handle it in UI should be different because of the flyout.

@ARWNightingale
Copy link

@maximpn we do currently allow a overlay flyout over an existing flyout. This would be fine for this milestone until the design team has an update on this behaviour globally.
https://github.com/user-attachments/assets/3a2da5fd-7082-4718-b677-fdfa61e8f144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team:Security Solution Platform Security Solution Platform Team
Projects
None yet
Development

No branches or pull requests

3 participants