Skip to content

[medium] Keep eval-only regex condition variables out of tstats where - #80

Open
elhoim wants to merge 1 commit into
SigmaHQ:mainfrom
elhoim:fix/datamodel-or-regex-tstats-where
Open

elhoim wants to merge 1 commit into
SigmaHQ:mainfrom
elhoim:fix/datamodel-or-regex-tstats-where

Conversation

@elhoim

@elhoim elhoim commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

BLUF

  • In data_model output, an OR-ed regex is matched after tstats by rex/eval, but the eval-created <field>Condition variable was also referenced inside the tstats … where clause.
  • tstats does not know that variable, so a regex branch that is ORed with other conditions could never match there. Events that match only the regex were dropped before the rex/eval/search stage ran.
  • Fix: the tstats WHERE clause keeps only the top-level conjuncts that do not reference such a variable, and is omitted when none remain. The trailing | search still applies the full expression.
  • Detections become correct at the cost of a broader tstats pre-filter for these rules only. Rules without an OR-ed regex produce the same output as before.

Priority: medium

Details

finalize_query_data_model moves the rex/eval pipeline after tstats and re-applies the search expression with | search. The same expression was still used as the tstats WHERE clause. For example, CommandLine|re OR Image with splunk_cim_data_model() gives:

| tstats … from datamodel=Endpoint.Processes where processCondition="true" OR Processes.process_path="x.exe" by …
| rex field=Processes.process "(?<processMatch>A.*B)"
| eval processCondition=…
| search processCondition="true" OR Processes.process_path="x.exe"

processCondition only exists after the eval, so inside tstats the WHERE clause reduces to Processes.process_path="x.exe". An event whose process matches the regex but whose process_path is different matches the Sigma rule, yet it never leaves tstats.

finish_query already records the names of these variables in state.processing_state["deferred_or_condition_fields"]. The new helper _data_model_where_without_fields works as follows:

  • It splits the expression into top-level conjuncts, respecting quotes and parentheses. NOT x and field IN (…) each count as one conjunct. An expression with a top-level OR is treated as a single conjunct.
  • It drops every conjunct that references one of these variables.

The remaining conjuncts are implied by the full expression, so they are a safe pre-filter. When nothing remains, the where keyword is left out, because WHERE is optional in tstats.

After the fix, the example above has no tstats WHERE clause, and the existing test test_splunk_data_model_process_creation_with_or_regex now pre-filters on Processes.parent_process_path="*explorer.exe" only.

Changed test expectation

test_splunk_data_model_process_creation_with_or_regex: the tstats WHERE clause no longer contains (Processes.process="*test_value*" OR processCondition="true"). With that clause, only events containing test_value reached the regex stage, so events matching only foo.*bar were lost. The trailing | search is unchanged.

Testing

  • New tests:
    • a top-level regex OR field rule in data_model output (exact query, no WHERE clause)
    • a mixed rule where the IN and NOT conjuncts stay in tstats and the regex disjunction is removed
    • parametrised cases for the helper, including NOT … IN (…), numbered …Condition2 variables and quoted values
  • Without the fix, 10 tests fail. With it, the full suite passes: pytest 148 passed, with Python 3.12 and pySigma 1.2.0 from poetry.lock, run the same way as CI (poetry install + pytest).
  • black==24.1.1 (pre-commit pin) reports nothing on the changed code.

🤖 Generated with Claude Code

With data_model output, an OR-ed regex is evaluated after tstats by rex/eval, but the <field>Condition variable was also referenced in the tstats WHERE clause, where it does not exist. Events matching only the regex branch never left tstats. Only keep the top-level conjuncts that do not reference such a variable in the tstats WHERE clause (omitting it when none remain); the full expression is still applied by the trailing search.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The change is narrowly scoped, aligns with the stated failure mode, and is backed by focused regression tests covering both end-to-end query output and the new helper’s edge cases.

Review effort: Lite
Findings: None

What changed in this PR

Fixes Splunk data model (tstats) query generation for Sigma rules that use an OR-ed regex branch: eval-only <field>Condition… variables are no longer referenced inside the tstats … where clause (where they don’t exist), preventing regex-only-matching events from being dropped before the deferred rex/eval/search stage.

Changes:

  • Update finalize_query_data_model to strip tstats WHERE conjuncts that reference deferred OR-regex condition variables, and omit where entirely when nothing safe remains.
  • Add _data_model_where_without_fields helper to conservatively keep only safe top-level conjuncts for tstats pre-filtering.
  • Expand tests to cover top-level OR regex behavior, mixed conjunct retention (IN/NOT kept, regex disjunction dropped), and helper parsing cases.
File Description
sigma/​backends/​splunk/​splunk.py Ensures tstats WHERE does not reference eval-only *Condition* fields and omits WHERE when empty, while preserving full logic in the trailing `
tests/​test_backend_splunk.py Updates an existing expectation and adds targeted regression/unit tests for OR-regex data model query correctness and the new helper behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
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