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

[BUG] Changes doc level query name field from id to rule name and adds validation #972

Merged
merged 5 commits into from
Apr 13, 2024

Conversation

jowg-amazon
Copy link
Collaborator

@jowg-amazon jowg-amazon commented Apr 5, 2024

Description

Changes doc level query name field from id to rule name.

Adds validation check for Sigma rule title during rule creation/updation. Sigma rule title is restricted so that it must be 1 - 256 chars as per Sigma rule specifications.

https://github.com/SigmaHQ/sigma/wiki/Specification/10f4159a3ee87c2f322a137bad6cdad313a169ab#title

Issues Resolved

PR related to:

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

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.

engechas
engechas previously approved these changes Apr 5, 2024
Comment on lines 186 to 189
// allowed characters [- : , ( ) [ ] ' _]
String allowedChars = "-:,\\(\\)\\[\\]\'_";
// regex to restrict string to alphanumeric and allowed chars, must be between 0 - 256 characters
String regex = "[\\w\\s" + Pattern.quote(allowedChars) + "]{0,256}";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: since we use this pattern in both SA and alerting, we should move it to common utils so that it can be defined in one place. It would be easy to miss updating this pattern in either alerting or SA if it changes in the future

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved the validation to common utils inside ValidationHelpers.kt

engechas
engechas previously approved these changes Apr 10, 2024
AWSHurneyt
AWSHurneyt previously approved these changes Apr 10, 2024
@jowg-amazon jowg-amazon dismissed stale reviews from AWSHurneyt and engechas via a6d89aa April 12, 2024 18:28
Signed-off-by: Joanne Wang <[email protected]>
engechas
engechas previously approved these changes Apr 12, 2024
@riysaxen-amzn
Copy link
Collaborator

i see many tests are failing.. can we check that

@jowg-amazon
Copy link
Collaborator Author

i see many tests are failing.. can we check that

They're failing because of common util changes, will rerun the CIs

Signed-off-by: Joanne Wang <[email protected]>
@@ -105,6 +106,17 @@ protected static SigmaRule fromDict(Map<String, Object> rule, boolean collectErr
ruleId = null;
}

String title;
Copy link
Collaborator

Choose a reason for hiding this comment

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

better way to write

String title = rule.containsKey("title") ? rule.get("title").toString() : "";
validateTitle(title);

// Validation method
private void validateTitle(String title) {
    if (!title.matches("^.{1,256}$")) {
        errors.add(new SigmaTitleError("Sigma rule title can be max 256 characters"));
    }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can do this in a follow up PR, I did it this way to remain consistent with how we do the other validation checks right now.

Copy link
Collaborator

@riysaxen-amzn riysaxen-amzn left a comment

Choose a reason for hiding this comment

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

Approved with a minor comment. Also, let's make sure CI passes atleast once even if the tests are flaky

@jowg-amazon jowg-amazon merged commit b86ee63 into opensearch-project:main Apr 13, 2024
11 of 18 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 13, 2024
…s validation (#972)

* change doc level query name field to rule name and add validation

Signed-off-by: Joanne Wang <[email protected]>

* call validation from common utils

Signed-off-by: Joanne Wang <[email protected]>

* change sigma title validation

Signed-off-by: Joanne Wang <[email protected]>

* add name to doc level query

Signed-off-by: Joanne Wang <[email protected]>

* fix unit tests

Signed-off-by: Joanne Wang <[email protected]>

---------

Signed-off-by: Joanne Wang <[email protected]>
(cherry picked from commit b86ee63)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 13, 2024
…s validation (#972)

* change doc level query name field to rule name and add validation

Signed-off-by: Joanne Wang <[email protected]>

* call validation from common utils

Signed-off-by: Joanne Wang <[email protected]>

* change sigma title validation

Signed-off-by: Joanne Wang <[email protected]>

* add name to doc level query

Signed-off-by: Joanne Wang <[email protected]>

* fix unit tests

Signed-off-by: Joanne Wang <[email protected]>

---------

Signed-off-by: Joanne Wang <[email protected]>
(cherry picked from commit b86ee63)
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-972-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b86ee635b0d1c1039af69e2b0de6add1b3f63bc3
# Push it to GitHub
git push --set-upstream origin backport/backport-972-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-972-to-2.x.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-972-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b86ee635b0d1c1039af69e2b0de6add1b3f63bc3
# Push it to GitHub
git push --set-upstream origin backport/backport-972-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-972-to-2.x.

jowg-amazon added a commit that referenced this pull request Apr 13, 2024
…s validation (#972)

* change doc level query name field to rule name and add validation

Signed-off-by: Joanne Wang <[email protected]>

* call validation from common utils

Signed-off-by: Joanne Wang <[email protected]>

* change sigma title validation

Signed-off-by: Joanne Wang <[email protected]>

* add name to doc level query

Signed-off-by: Joanne Wang <[email protected]>

* fix unit tests

Signed-off-by: Joanne Wang <[email protected]>

---------

Signed-off-by: Joanne Wang <[email protected]>
(cherry picked from commit b86ee63)
jowg-amazon added a commit to jowg-amazon/security-analytics that referenced this pull request Apr 13, 2024
…s validation (opensearch-project#972)

* change doc level query name field to rule name and add validation

Signed-off-by: Joanne Wang <[email protected]>

* call validation from common utils

Signed-off-by: Joanne Wang <[email protected]>

* change sigma title validation

Signed-off-by: Joanne Wang <[email protected]>

* add name to doc level query

Signed-off-by: Joanne Wang <[email protected]>

* fix unit tests

Signed-off-by: Joanne Wang <[email protected]>

---------

Signed-off-by: Joanne Wang <[email protected]>
jowg-amazon added a commit that referenced this pull request Apr 13, 2024
jowg-amazon added a commit that referenced this pull request Apr 13, 2024
…s validation (#972) (#982)

* change doc level query name field to rule name and add validation

Signed-off-by: Joanne Wang <[email protected]>

* call validation from common utils

Signed-off-by: Joanne Wang <[email protected]>

* change sigma title validation

Signed-off-by: Joanne Wang <[email protected]>

* add name to doc level query

Signed-off-by: Joanne Wang <[email protected]>

* fix unit tests

Signed-off-by: Joanne Wang <[email protected]>

---------

Signed-off-by: Joanne Wang <[email protected]>
(cherry picked from commit b86ee63)

Co-authored-by: Joanne Wang <[email protected]>
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.11 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security-analytics/backport-2.11 2.11
# Navigate to the new working tree
pushd ../.worktrees/security-analytics/backport-2.11
# Create a new branch
git switch --create backport-972-to-2.11
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b86ee635b0d1c1039af69e2b0de6add1b3f63bc3
# Push it to GitHub
git push --set-upstream origin backport-972-to-2.11
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security-analytics/backport-2.11

Then, create a pull request where the base branch is 2.11 and the compare/head branch is backport-972-to-2.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants