[Security Solution][Endpoint] Wraps query in parentheses to avoid query exception lists #102612

dasansol92 · 2021-06-18T08:56:06Z

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

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2021-06-18T08:56:07Z

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

dasansol92 · 2021-06-21T08:15:12Z

@elasticmachine merge upstream

…_matching_list_names-102607

kibanamachine · 2021-06-21T10:22:26Z

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id	before	after	diff
`securitySolution`	6.9MB	6.9MB	+6.0B

History

💚 Build #132348 succeeded 0295649

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

ashokaditya

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.

dasansol92 · 2021-06-22T09:08:19Z

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?

ashokaditya · 2021-06-22T09:55:12Z

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.

dasansol92 · 2021-06-22T10:10:44Z

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).

paul-tavares

Just one questions - but other than that, LGTM 🚢

paul-tavares · 2021-06-22T11:52:43Z

x-pack/plugins/security_solution/public/management/common/utils.test.ts

@@ -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*))"


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?

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>

kibanamachine · 2021-06-22T15:28:51Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…#102941) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: David Sánchez <davidsansol92@gmail.com>

Wraps query in parentheses to avoid quering exception lists

0295649

dasansol92 added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v7.14.0 v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 18, 2021

dasansol92 requested a review from a team as a code owner June 18, 2021 08:56

dasansol92 mentioned this pull request Jun 18, 2021

[Security Solution] Trusted Apps and Event Filters search bar is returning lists when matching list names #102607

Closed

Merge branch 'master' into fix/olm-search_bar_is_returning_lists_when…

124e31b

…_matching_list_names-102607

ashokaditya approved these changes Jun 22, 2021

View reviewed changes

paul-tavares approved these changes Jun 22, 2021

View reviewed changes

dasansol92 removed the v7.13.0 label Jun 22, 2021

dasansol92 merged commit c940da4 into elastic:master Jun 22, 2021

dasansol92 added the v8.0.0 label Jun 22, 2021

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Jun 22, 2021

Wraps query in parentheses to avoid quering exception lists (elastic#…

d2f38d8

…102612) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine mentioned this pull request Jun 22, 2021

[7.x] Wraps query in parentheses to avoid quering exception lists (#102612) #102941

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[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

dasansol92 commented Jun 18, 2021

elasticmachine commented Jun 18, 2021

dasansol92 commented Jun 21, 2021

kibanamachine commented Jun 21, 2021

ashokaditya left a comment

dasansol92 commented Jun 22, 2021

ashokaditya commented Jun 22, 2021

dasansol92 commented Jun 22, 2021

paul-tavares left a comment

paul-tavares Jun 22, 2021

dasansol92 Jun 22, 2021

kibanamachine commented Jun 22, 2021

[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

dasansol92 commented Jun 18, 2021

Summary

For maintainers

elasticmachine commented Jun 18, 2021

dasansol92 commented Jun 21, 2021

kibanamachine commented Jun 21, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

ashokaditya left a comment

Choose a reason for hiding this comment

dasansol92 commented Jun 22, 2021

ashokaditya commented Jun 22, 2021

dasansol92 commented Jun 22, 2021

paul-tavares left a comment

Choose a reason for hiding this comment

paul-tavares Jun 22, 2021

Choose a reason for hiding this comment

dasansol92 Jun 22, 2021

Choose a reason for hiding this comment

kibanamachine commented Jun 22, 2021

💚 Backport successful