-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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][Endpoint] Wraps query in parentheses to avoid query exception lists #102612
[Security Solution][Endpoint] Wraps query in parentheses to avoid query exception lists #102612
Conversation
Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt) |
@elasticmachine merge upstream |
…_matching_list_names-102607
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is quite a long query with many terms I would suggest to break this up into smaller clauses and then compose the query predicate. It would be easier to maintain and change in the future.
Are you talking about the test? |
No not the test. The actual query where the parantheses were missing. I presume this PR adds the test for a change that was done in an earlier PR. |
Maybe I misunderstood something but no, this test is because the changes on the current query (the ones in this PR). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one questions - but other than that, LGTM 🚢
@@ -35,7 +35,7 @@ describe('utils', () => { | |||
searchableFields | |||
) | |||
).toBe( | |||
"exception-list-agnostic.attributes.name:(*this'is%&query\\{\\}[]!¿?with.,-+`´special\\<\\>ºª@#|·chars*) OR exception-list-agnostic.attributes.description:(*this'is%&query\\{\\}[]!¿?with.,-+`´special\\<\\>ºª@#|·chars*) OR exception-list-agnostic.attributes.entries.value:(*this'is%&query\\{\\}[]!¿?with.,-+`´special\\<\\>ºª@#|·chars*) OR exception-list-agnostic.attributes.entries.entries.value:(*this'is%&query\\{\\}[]!¿?with.,-+`´special\\<\\>ºª@#|·chars*)" | |||
"(exception-list-agnostic.attributes.name:(*this'is%&query\\{\\}[]!¿?with.,-+`´special\\<\\>ºª@#|·chars*) OR exception-list-agnostic.attributes.description:(*this'is%&query\\{\\}[]!¿?with.,-+`´special\\<\\>ºª@#|·chars*) OR exception-list-agnostic.attributes.entries.value:(*this'is%&query\\{\\}[]!¿?with.,-+`´special\\<\\>ºª@#|·chars*) OR exception-list-agnostic.attributes.entries.entries.value:(*this'is%&query\\{\\}[]!¿?with.,-+`´special\\<\\>ºª@#|·chars*))" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. First: I'm dizzy looking at this. Second: 👏
Question: I don't see a (
or )
used as a value in the above test. Do we need to escape it if the user enters it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we don't need to escape it as it will be between **
inside each query for each field
…102612) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Summary
This was the query before the changes:
((exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "endpoint_trusted_apps") AND exception-list-agnostic.attributes.name:(*Eve*) OR exception-list-agnostic.attributes.description:(*Eve*) OR exception-list-agnostic.attributes.entries.value:(*Eve*) OR exception-list-agnostic.attributes.entries.entries.value:(*Eve*))
Without the parentheses we were filtering all that matches name, entries or description without the list_type and list_id filters (because the OR statement without parentheses).
This is the query after the changes:
((exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "endpoint_trusted_apps") AND (exception-list-agnostic.attributes.name:(*Eve*) OR exception-list-agnostic.attributes.description:(*Eve*) OR exception-list-agnostic.attributes.entries.value:(*Eve*) OR exception-list-agnostic.attributes.entries.entries.value:(*Eve*)))
Now there is no list/exception list returned on this query when filtering by list_id.
For maintainers