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] Adapt special character escape according to kuery grammer #198288

Merged
merged 5 commits into from
Dec 3, 2024

Conversation

logeekal
Copy link
Contributor

@logeekal logeekal commented Oct 30, 2024

Summary

Fixes https://github.com/elastic/kibana-team/issues/1107

As mentioned in https://github.com/elastic/kibana/security/code-scanning/349, this PR resolves the escaping issue.

Additionally, it also adds more candiadates for escaping as mentioned in kuery grammar as shown below .

SpecialCharacter
= [\\():<>"*{}]

Solution

This PR replicates #128289 in 7.17 where escape_kquery was moved from data plugin to es-query package. This should have been backported to 7.17 but was not.

@logeekal logeekal added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Threat Hunting:Investigations Security Solution Investigations Team labels Oct 30, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

@logeekal logeekal requested a review from a team October 30, 2024 17:04
@logeekal logeekal force-pushed the fix/code_scanning_alert_349 branch from 3c7c527 to c0b7a1b Compare November 4, 2024 11:48
Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

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

LGTM!

@logeekal logeekal enabled auto-merge (squash) November 5, 2024 14:43
@legrego
Copy link
Member

legrego commented Nov 21, 2024

@logeekal is there anything we can do to assist with this PR? Do we "just" need a clean CI run?

@logeekal
Copy link
Contributor Author

logeekal commented Dec 3, 2024

/ci

@logeekal
Copy link
Contributor Author

logeekal commented Dec 3, 2024

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

There are no new commits on the base branch.

@elasticmachine
Copy link
Contributor

elasticmachine commented Dec 3, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Cypress Tests #7 / Detection rules, override Creates and activates a new custom rule with override option
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "false" and their admin callouts should not show up On Rule Details page "before each" hook for "We show 1 primary callout"
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "false" and their admin callouts should not show up On Rule Details page "before each" hook for "We show 1 primary callout"
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "null" and their admin callouts should not show up On Rule Details page "before each" hook for "We show 1 primary callout"
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "null" and their admin callouts should not show up On Rule Details page "before each" hook for "We show 1 primary callout"
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "true" and their admin callouts should show up On Detections home page We show the need admin primary callout
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "true" and their admin callouts should show up On Detections home page We show the need admin primary callout
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "true" and their admin callouts should show up On Rule Details page "before each" hook for "We show 1 primary callout"
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "true" and their admin callouts should show up On Rule Details page "before each" hook for "We show 1 primary callout"
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "true" and their admin callouts should show up On Rules Management page We show 1 primary callout of need admin
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #3 / Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set The users index_mapping_outdated is "true" and their admin callouts should show up On Rules Management page We show 1 primary callout of need admin

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
canvas 1063 1064 +1
dashboard 271 272 +1
dashboardEnhanced 92 93 +1
data 504 505 +1
dataVisualizer 321 322 +1
discover 461 462 +1
fleet 542 543 +1
infra 928 929 +1
inputControlVis 113 114 +1
lens 800 801 +1
lists 295 296 +1
maps 745 746 +1
ml 1598 1599 +1
observability 342 343 +1
securitySolution 2585 2586 +1
timelines 285 286 +1
upgradeAssistant 212 213 +1
uptime 626 627 +1
total +18

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/es-query 153 154 +1

Async chunks

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

id before after diff
canvas 1.0MB 1.0MB +278.0B
dashboard 181.3KB 181.6KB +278.0B
dataVisualizer 526.9KB 527.1KB +282.0B
discover 363.6KB 363.9KB +278.0B
fleet 630.6KB 630.9KB +278.0B
infra 941.0KB 941.3KB +282.0B
inputControlVis 83.6KB 83.9KB +278.0B
lens 1.0MB 1.0MB +282.0B
lists 149.4KB 149.7KB +279.0B
maps 2.7MB 2.7MB +278.0B
ml 3.3MB 3.3MB +282.0B
securitySolution 4.7MB 4.7MB +216.0B
upgradeAssistant 187.2KB 187.5KB +279.0B
uptime 578.9KB 579.2KB +282.0B
total +3.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dashboardEnhanced 44.7KB 44.9KB +278.0B
data 454.8KB 454.8KB +68.0B
observability 69.0KB 69.2KB +278.0B
timelines 311.2KB 311.4KB +169.0B
total +793.0B
Unknown metric groups

API count

id before after diff
@kbn/es-query 205 207 +2

History

@logeekal logeekal merged commit 38f1f6d into elastic:7.17 Dec 3, 2024
87 checks passed
@mistic mistic added the v7.17.27 label Dec 3, 2024
@mistic
Copy link
Member

mistic commented Dec 3, 2024

This PR didn't make it on time to the latest v7.17.26 BC. Updating the labels.

@logeekal
Copy link
Contributor Author

logeekal commented Dec 4, 2024

This PR didn't make it on time to the latest v7.17.26 BC. Updating the labels.

Thanks @mistic

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 release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v7.17.27
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants