Skip to content

fix(sigma): preserve ATR regex case semantics - #520

Merged
eeee2345 merged 1 commit into
Agent-Threat-Rule:mainfrom
dc-tw:fix/sigma-regex-case-semantics
Sep 5, 2026
Merged

eeee2345 merged 1 commit into
Agent-Threat-Rule:mainfrom
dc-tw:fix/sigma-regex-case-semantics

Conversation

@dc-tw

@dc-tw dc-tw commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #331. This intentionally does not close the issue: pyATR's silent regex compile failures and the remaining variable-width-lookbehind rules are separate follow-ups.

Context and scope

#509 fixed the JavaScript-only Unicode escapes. #510 and #511 preserve case semantics in the PyRIT digest, but do not change generate-sigma.py; this PR addresses only the Sigma-export subsection of #331.

The repository's rule-writing guide still recommends adding inline (?i), while the schema and runtime define regex matching as case-insensitive by default unless case_sensitive: true. This PR does not change that broader authoring guidance.

No rules, pyATR code, workflows, generated examples, or dependency files are changed.

Verification

  • python -m pytest scripts/test_generate_sigma.py -q -k 'not test_re2_dialect_rewrites_unicode_escapes': 18 passed, 1 deselected
  • full suite on this branch: 18 passed, 1 pre-existing failure
  • full suite on untouched upstream/main: 15 passed, the same 1 failure
  • regenerated the five tracked Sigma examples with both the upstream and changed exporters against the same rules: zero output diff
  • ATR-2026-02005 before/after: the only output change is content|re to content|re|i
  • git diff --check

The standalone Python converter suite is not currently wired into PR GitHub Actions; adding that CI integration is outside this focused change.

@eeee2345

eeee2345 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Reviewed. Approve on the substance — one follow-up below that is about the engine, not about this PR.

The premise checks out, and the blast radius is larger than the description says

I verified the claim independently rather than taking it:

  • src/engine.ts:933const rFlags = needsUnicodeFlag(normalized) ? 'iu' : 'i';. Unconditional. The engine never matches case-sensitively on this path.
  • spec/atr-schema.yaml:649 and docs/schema-spec.md:381 both define case_sensitive as an optional boolean defaulting to false.

So "case-insensitive by default, with an explicit opt-out" is the written contract, and the exporter was reading the absence of inline (?i) as the opt-out instead. That is the right diagnosis.

Worth putting a number on it, because the PR body undersells this. I counted rules whose conditions carry no inline (?i) anywhere:

835 rule files
290 with no inline (?i) anywhere   (34.7%)

Every downstream Sigma consumer has been getting case-sensitive matching on more than a third of the corpus, against an engine that is case-insensitive on all of it. That is not a cosmetic export detail; it is weaker detection than ATR advertises, in an artifact other projects consume. I would put that number in the PR body — it is the strongest argument for merging this.

One finding, for a follow-up rather than this PR

The engine does not honour case_sensitive uniformly. Two paths read it:

src/engine.ts:1125   const flags = cond.case_sensitive ? '' : 'i';
src/engine.ts:1543   const flags = caseSensitive ? '' : 'i';

and the compilePatterns path at :933 does not. ATR-2026-02557 documents this in its own false_positives, having been bitten by it:

"The engine compiles array-format regex conditions with flag 'i' unconditionally (src/engine.ts compilePatterns) and IGNORES case_sensitive on this condition format."

So on that condition format, case_sensitive: true is inert at runtime. Your exporter would faithfully emit field|re for it — case-sensitive Sigma against a case-insensitive engine, which is the same class of divergence you are fixing, pointing the other way.

This does not block the PR, for two reasons: no rule currently sets case_sensitive as a field (I checked — zero), so the branch is unexercised today; and the default path, which is 100% of the corpus, becomes correct. But it is worth a line in the doc caveat, and the real fix is one of:

  1. make compilePatterns honour case_sensitive, so the contract holds everywhere; or
  2. have the exporter warn when case_sensitive: true appears on a condition format the engine ignores.

(1) is the honest one and probably a small change. It is also the kind of thing better raised as its own issue than bolted onto this PR.

On the verification

Running the suite on untouched upstream/main to establish that the one failure is pre-existing is the part I want to call out. That distinction is usually skipped, and skipping it is how a pre-existing failure gets silently attributed to whoever touched the file last.

The conflict handling — case_sensitive: true plus inline i, schema wins, inline stripped, warning emitted — is the right resolution and the right place to be loud about it.

Merge

Holding for @eeee2345 on the merge itself, since rule-affecting exports are his call. Nothing in the code is blocking from my side.

@eeee2345
eeee2345 merged commit 057944a into Agent-Threat-Rule:main Sep 5, 2026
7 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