fix: removes multiline from default regex modifiers

[M4tteoP]

This PR evaluates the needed changes to align Coraza to Modsec v2 behavior rather than Modsec v3 in terms of default modifiers when compiling regexes. This necessity has been raised by coreruleset/coreruleset#3277.

We are currently running with two default modifiers:

• s (dotall)
• m (multiline mode)

While Modsec v2 (the reference implementation for the CRS) has:

• PCRE2_DOTALL
• PCRE2_DOLLAR_ENDONLY

Running the CRS rules with multiline may lead to false positives and lower performance.

Opening the PR to further discuss this, and whether we accept a breaking change for v3.* or not.

EDIT: The change has been implemented under the coraza.rule.no_regex_multiline build flag and is disabled by default.

[M4tteoP]

I was not able to find the DOLLAR_ENDONLY modifier for RE2, but it seems that re2 by default behaves like DOLLAR_ENDONLY enabled.
See also https://regex101.com/r/mUcy69/1 (Mind the regex option D (dollar end only) enabled).
Am I missing something?

[codecov]

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 81.66%. Comparing base (3bc5ef6) to head (3c50384).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/operators/rx.go	92.30%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #876   +/-   ##
=======================================
  Coverage   81.66%   81.66%           
=======================================
  Files         168      168           
  Lines        9647     9655    +8     
=======================================
+ Hits         7878     7885    +7     
  Misses       1519     1519           
- Partials      250      251    +1     

Flag	Coverage Δ
coraza.rule.case_sensitive_args_keys	81.62% <92.30%> (+<0.01%)	⬆️
coraza.rule.multiphase_valuation	81.66% <92.30%> (+<0.01%)	⬆️
coraza.rule.no_regex_multiline	81.60% <46.15%> (?)
default	81.66% <92.30%> (+<0.01%)	⬆️
examples+	16.45% <53.84%> (+<0.01%)	⬆️
examples+coraza.rule.case_sensitive_args_keys	81.62% <92.30%> (+<0.01%)	⬆️
examples+coraza.rule.multiphase_valuation	81.50% <53.84%> (-0.05%)	⬇️
examples+coraza.rule.no_regex_multiline	81.52% <46.15%> (?)
examples+memoize_builders	81.62% <53.84%> (-0.05%)	⬇️
examples+no_fs_access	80.94% <53.84%> (-0.05%)	⬇️
ftw	81.66% <92.30%> (+<0.01%)	⬆️
memoize_builders	81.76% <92.30%> (+<0.01%)	⬆️
no_fs_access	81.10% <92.30%> (+<0.01%)	⬆️
tinygo	81.63% <92.30%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[anuraaga]

Good to see ftw passing with this change. I wouldn't merge it until seeing the PR to ModSec v3 doing the same, otherwise someone will just ask in the future why we aren't aligned with ModSec v3.

[jcchavezs]

Maybe it is a good idea to coordinate this in @M4tteoP @airween

[M4tteoP]

Totally 💯

[M4tteoP]

Apart from aligning the behavior with ModSec, are we going to accept this breaking change and release it into a 3.x version or we aim to have it into 4.x? If the latter is the case, I would guard the feature with a build flag and merge this PR with the fix disabled by default

[jptosso]

We agreed to implement this as a build tag for v3 and mandatory for v4
Once merged, another PR has to be created to remove the flag and force it by default

[M4tteoP]

We agreed to implement this as a build tag for v3 and mandatory for v4

Done, added coraza.rule.no_regex_multiline build tag. It should be ready to be reviewed.

[jptosso]

can you add this to the testing matrix? within the github actions. As mentioned here #1182, we should start covering all permutations of flags

[jptosso]

Please add this build tag to magefile.go

[M4tteoP]

Done, let's if tests are happy ^_^

[M4tteoP]

Looks so 🚀
Note: codecov is failing because the new logic is tested on a new test file that has the conditional build, so we have coverage, but codecov does not get it.

[jptosso]

LGTM