doc(sigma): documented the use of _ws_ string by creating more tests

[rios0rios0]

Description

I initially removed the _ws_ replacement as I believed it was unnecessary. However, after further investigation, I realized the importance of this replacement in maintaining the functionality of various features. I have now added more detailed documentation to explain the use of _ws_ and its significance.

The updated JavaDoc provides a comprehensive explanation of why the _ws_ replacement is necessary and how it ensures the correct interpretation of whitespaces within Lucene queries. This documentation should help future developers understand the rationale behind this implementation.

For a conversion test demonstrating this feature, refer to QueryBackendTests#testConvertWhiteSpaceWithWsReplacement(). For an integration test, refer to DetectorRestApiIT#testCreateADetectorWithSigmaRulesUsingWhiteSpaces(). To ensure this feature operates correctly, configure OpenSearch with the settings in src/main/resources/mappings/detector-settings.json.

Related Issues

Resolves #1024

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR #1024.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[rios0rios0]

gradle build -x test -x check
BUILD SUCCESSFUL in 6s
11 actionable tasks: 7 executed, 4 up-to-date

[rios0rios0]

The test was breaking before my change. So, don't blame my change please.

gradle test
> Task :test

Tests with failures:
 - org.opensearch.securityanalytics.rules.backend.QueryBackendTests.testConvertNotComplicatedExpression

412 tests completed, 1 failed

> Task :test FAILED

FAILURE: Build failed with an exception.

BUILD FAILED in 18s
7 actionable tasks: 1 executed, 6 up-to-date

[tallyoh]

THANK YOU !
Really appreciate this. Now if they can merge the change for 2.19 🙏

[eirsep]

thanks for working on this bug

plz add unit tests verifying the queries for the failing scenarios are now working and integ test that does the following : ingest that query, run a detector and generate finding(rule match)

[rios0rios0]

thanks for working on this bug

plz add unit tests verifying the queries for the failing scenarios are now working and integ test that does the following : ingest that query, run a detector and generate finding(rule match)

Done!

gradle integTest --tests "org.opensearch.securityanalytics.DetectorSigmaRulesIT.testCreateDetectorSigmaRule_WithWindowsRules"

BUILD SUCCESSFUL in 22s
9 actionable tasks: 2 executed, 7 up-to-date

[rios0rios0]

gradle test
> Task :test

Tests with failures:
 - org.opensearch.securityanalytics.rules.backend.QueryBackendTests.testConvertNotComplicatedExpression

414 tests completed, 1 failed

> Task :test FAILED

FAILURE: Build failed with an exception.

BUILD FAILED in 20s
7 actionable tasks: 1 executed, 6 up-to-date

[sbcd90]

hi @rios0rios0 , can you please add just a new test with the Sigma Rule here? https://github.com/opensearch-project/security-analytics/blob/main/src/test/java/org/opensearch/securityanalytics/rules/backend/QueryBackendTests.java
a test similar to this one. https://github.com/opensearch-project/security-analytics/blob/main/src/test/java/org/opensearch/securityanalytics/rules/backend/QueryBackendTests.java#L1127
Also, please revert all the changes in the files except SigmaString.java.

[rios0rios0]

hi @rios0rios0 , can you please add just a new test with the Sigma Rule here? https://github.com/opensearch-project/security-analytics/blob/main/src/test/java/org/opensearch/securityanalytics/rules/backend/QueryBackendTests.java a test similar to this one. https://github.com/opensearch-project/security-analytics/blob/main/src/test/java/org/opensearch/securityanalytics/rules/backend/QueryBackendTests.java#L1127 Also, please revert all the changes in the files except SigmaString.java.

1 - "Also, please revert all the changes in the files except SigmaString.java."
Sorry, doesn't make sense your request.

Those files are needed to do my testing, did you see that? Could you please clarify?

2 - "can you please add just a new test with the Sigma Rule here?"
Ok on it.

[sbcd90]

hi @rios0rios0 , could you please copy the sigma rule from here #1024 (comment) & create a test out of it?

[rios0rios0]

1 - "Also, please revert all the changes in the files except SigmaString.java."
Sorry, doesn't make sense your request.

Of course. Just give me some minutes, on it.

[rios0rios0]

hi @rios0rios0 , could you please copy the sigma rule from here #1024 (comment) & create a test out of it?

Done! Working 100%

[rios0rios0]

@rios0rios0 , Thanks for the pr. Can you please address the review comments?

Hi, @sbcd90.
I have made all the changes requested by you. Could you please go ahead and release this PR?
There are lots of people needing this change, I'm empathizing with them.

[rios0rios0]

@sbcd90 without compromising the release of this PR, I'd like to point out something:

I have made all the changes requested by you. However, I don't agree with them, because instead of making the codebase easier to understand and modularized, we are just keeping huge and complicated files to understand and maintain. That's why I created this other PR, if you can look at the description, you will understand the rational and literary reason I followed. Could you explain yours?

[rios0rios0]

@sbcd90 @eirsep

1. gradle test
   

2. gradle spotlessApply
   

[sbcd90]

hi @rios0rios0 , can you please rebase your changes with the main branch & i can trigger the test workflow to verify if tests are passing?

[rios0rios0]

hi @rios0rios0 , can you please rebase your changes with the main branch & i can trigger the test workflow to verify if tests are passing?

Done!

[sbcd90]

@rios0rios0 , thanks a lot. i triggered the builds.

[sbcd90]

hi @rios0rios0 , could you check why tests are failing? they are passing in our main branch currently.

[rios0rios0]

hi @rios0rios0 , could you check why tests are failing? they are passing in our main branch currently.

Ok, on it.

[sbcd90]

@rios0rios0 , re-running the tests.

[rios0rios0]

@rios0rios0 , re-running the tests.

Wait a second please, I just fixed the unit test ones. The other integration tests, they are breaking since the logic is not right anymore. I have to change your pre-packaged rules as well, to match with empty space.

[sbcd90]

@rios0rios0 , re-running the tests.

Wait a second please, I just fixed the unit test ones. The other integration tests, they are breaking since the logic is not right anymore. I have to change your pre-packaged rules as well, to match with empty space.

Thanks @rios0rios0 .

[rios0rios0]

@tallyoh @givilleneuve @sbcd90 @eirsep

After further review, I have decided to pivot this PR from a fix change to a documentation update. The initial removal of the _ws_ replacement was based on the assumption that it was unnecessary. However, upon deeper analysis, I found that the _ws_ replacement is crucial for maintaining the predictability and functionality of various features. Please see my PR description for more information.

[rios0rios0]

1. gradle test
   

2. gradle integTest
   

[rios0rios0]

@sbcd90 @eirsep

Could you please advise on the best place to document this issue? It appears that users might be experiencing this problem because they are using an older version of Security Analytics that lacks the updated index template. As a result, the index template is no longer being created or overridden (because an old version is already in place), causing user queries to include _ws_ in the content, which prevents them from matching correctly. The solution is to edit the index template and change the configuration manually to match with src/main/resources/mappings/detector-settings.json.

[rios0rios0]

@sbcd90 @eirsep ?