Fixed mandatory check of rule ID.

[brijeshjvalera]

Fixed strict check: The id action is required for all SecRule/SecAction.

Make sure that you've checked the boxes below before you submit PR:

• My code includes positive and negative tests.
• I have an appropriate description with correct grammar.
• I have read Contribution guidelines, maintainers note and Quality standard.
• My code is properly linted and passes pre-commit tests.

Thanks for your contribution ❤️

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.98%. Comparing base (4722c9a) to head (20a10aa).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1325      +/-   ##
==========================================
+ Coverage   81.94%   81.98%   +0.03%     
==========================================
  Files         170      170              
  Lines        9777     9781       +4     
==========================================
+ Hits         8012     8019       +7     
+ Misses       1518     1516       -2     
+ Partials      247      246       -1     

Flag	Coverage Δ
coraza.rule.case_sensitive_args_keys	81.94% <100.00%> (+0.03%)	⬆️
coraza.rule.multiphase_valuation	81.98% <100.00%> (+0.03%)	⬆️
coraza.rule.no_regex_multiline	81.92% <100.00%> (+0.03%)	⬆️
default	81.98% <100.00%> (+0.03%)	⬆️
examples+	16.55% <14.28%> (+<0.01%)	⬆️
examples+coraza.rule.case_sensitive_args_keys	81.94% <100.00%> (+0.03%)	⬆️
examples+coraza.rule.multiphase_valuation	81.82% <100.00%> (+0.03%)	⬆️
examples+coraza.rule.no_regex_multiline	81.84% <100.00%> (+0.03%)	⬆️
examples+memoize_builders	81.94% <100.00%> (+0.03%)	⬆️
examples+no_fs_access	81.26% <100.00%> (+0.03%)	⬆️
ftw	81.98% <100.00%> (+0.03%)	⬆️
memoize_builders	82.07% <100.00%> (+0.03%)	⬆️
no_fs_access	81.43% <100.00%> (+0.03%)	⬆️
tinygo	81.95% <100.00%> (+0.03%)	⬆️

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.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jcchavezs]

curious why this empty string is needed.

[brijeshjvalera]

p.FromString() will fail as it has to follow the syntax: SecRule VARIABLES OPERATOR [ACTIONS]. Without empty string(""), "id:4" is treated as operator (will be treated as @rx id:3, @rx as default operator).
Testcase will fail with this as it will not find any ID action specified in the rule.

[airween]

A side note: we already started a discussion about this implicit @rx operator's role, see ModSecurity issue #3081. Feel free to leave a comment.

[M4tteoP]

Thanks for this!

[M4tteoP]

Let question from my side: here we are enforcing a check that, as per the documentation, has always been expected to work this way. However, it would be a breaking change for those not compliant with the documentation. Should we add a build flag to enable this check, or can we mention it in the release notes and require users to fix non-compliant rules when upgrading the minor version? @corazawaf/maintainers

[jcchavezs]

I would go for the build tag otherwise it can be a breaking change.