Skip to content

Conversation

@arblade
Copy link
Contributor

@arblade arblade commented Apr 4, 2025

Following an issue on the splunk backend, I made a PR which requires to apply finalize_query on all rules, even those that are part of correlation rules, reversing the #278 PR on pysigma.

@thomaspatzke
Copy link
Member

Just checked, #278 is necessary to fix an issue in an Elasticsearch backend. So both behaviors are valid depending on the context. Therefore, I suggest to introduce a boolean backend class variable finalize_correlation_subqueries that controls the behavior.

@thomaspatzke thomaspatzke marked this pull request as ready for review April 6, 2025 00:17
@thomaspatzke thomaspatzke changed the base branch from main to release-0.11 April 12, 2025 09:49
@thomaspatzke thomaspatzke changed the base branch from release-0.11 to main April 12, 2025 09:49
@thomaspatzke thomaspatzke requested a review from Copilot April 12, 2025 09:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

tests/test_conversion_correlations.py:483

  • The expected output contains nested square brackets which may indicate double finalization. Please verify if the nested brackets are intentional and, if so, add a clarifying comment to explain the intended formatting.
        """[ [ EventID=4625 ]"""

sigma/conversion/base.py:229

  • Using a logical OR here causes all rules to be finalized when finalize_correlation_subqueries is true, regardless of backreferences. Consider adding an inline comment to clarify that this is intentional and document the rationale for finalizing correlation rules even when backreferences are present.
if self.finalize_correlation_subqueries or not rule._backreferences

@thomaspatzke thomaspatzke merged commit 1fbea68 into SigmaHQ:main Apr 12, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants