Skip to content

docs: add warnings and a use case on false positive management#247

Open
touchweb-vincent wants to merge 12 commits intocoreruleset:mainfrom
touchweb-vincent:patch-1
Open

docs: add warnings and a use case on false positive management#247
touchweb-vincent wants to merge 12 commits intocoreruleset:mainfrom
touchweb-vincent:patch-1

Conversation

@touchweb-vincent
Copy link
Copy Markdown
Contributor

@touchweb-vincent touchweb-vincent commented Nov 4, 2025

Hello,

what

This PR adds explicit warning notices to several rule-exclusion examples and documentation sections.
These notices remind users that:

  • Certain exclusions shown in examples are not safe to apply globally.
  • Such configurations must be used only in very specific contexts.
  • The examples are meant to illustrate syntax, not to recommend real-world use.
  • The goal is to make CRS documentation safer and more self-explanatory, reducing the risk of misconfiguration.

why

To help users avoid unsafe tuning practices that could compromise the effectiveness of CRS.

Some examples and documentation snippets may look harmless, but when reused without proper context they can lead to overly broad exclusions or rule bypasses.

Adding warnings provides clear guidance about when an example is for demonstration only, and when it should never be used in production.

@airween
Copy link
Copy Markdown
Contributor

airween commented Nov 4, 2025

@touchweb-vincent,

thanks for your PR - please make a bit more informative comment under the PR. For example, please see this. The two relevant questions are "why" and "what", with one-one sentences.

Thank you again.

@touchweb-vincent
Copy link
Copy Markdown
Contributor Author

@airween done, does this sound good to you ?

Copy link
Copy Markdown
Contributor

@airween airween left a comment

Choose a reason for hiding this comment

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

I'm not a native English speaker so it's not the best idea to give a review, but I made a suggestion.

Cc: @theseion, @RedXanadu.

Comment thread content/2-how-crs-works/2-3-false-positives-and-tuning.md Outdated
@airween
Copy link
Copy Markdown
Contributor

airween commented Nov 5, 2025

@airween done, does this sound good to you ?

Yes, excellent! Thank you!

Comment thread content/2-how-crs-works/2-3-false-positives-and-tuning.md Outdated
Comment thread content/2-how-crs-works/2-3-false-positives-and-tuning.md Outdated
```apache
# CRS Rule Exclusion: 941150 - XSS Filter - Category 5: Disallowed HTML Attributes
SecRule REQUEST_URI "@beginsWith /dynamic/new_post" \
SecRule REQUEST_URI "@rx ^/dynamic/new_post" \
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.

Why? @beginsWith is much easier to understand.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I’m not sure I understand your point - using ^ in a regex is just as straightforward as @beginswith, and it also provides a slightly different example. Since @beginswith is already used several times, it makes sense to vary the syntax a bit

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.

What is your reasoning behind changing the operator here? @rx is harder to understand.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I still don’t understand why you think @rx ^ is more complicated than beginsWith - and honestly, if they’re at the point of saying that, then mod_security2 is probably not the right tool for them, imo, since they’ll be writing regexes all day long just to deal with their false positives.

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.

Please explain why you've made this change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I replaced @beginswith with @rx because it adds a concrete, real-world example of a pattern they’ll use almost every day.

Using @rx (regular expression) shows a more flexible and practical case than @beginswith, since it can handle a wider variety of inputs - something they'll frequently need in production scenarios.

In short, this change makes the rule both more instructive and more representative of what developers will actually encounter.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@touchweb-vincent I see where your coming from, but I think kind of information is better suited in the section about creating rules. I think we should keep the false positive tuning doc simple to avoid overloading the reader with too much information.

Comment thread content/2-how-crs-works/2-3-false-positives-and-tuning.md Outdated
touchweb-vincent and others added 2 commits November 6, 2025 11:21
Co-authored-by: Ervin Hegedus <airween@gmail.com>
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@theseion
Copy link
Copy Markdown
Contributor

theseion commented Nov 8, 2025

Please work through the open comments @touchweb-vincent.

@touchweb-vincent
Copy link
Copy Markdown
Contributor Author

@theseion I was waiting for your point of view before committing the changes.

The changes have now been committed.

```apache
# CRS Rule Exclusion: 941150 - XSS Filter - Category 5: Disallowed HTML Attributes
SecRule REQUEST_URI "@beginsWith /dynamic/new_post" \
SecRule REQUEST_URI "@rx ^/dynamic/new_post" \
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.

What is your reasoning behind changing the operator here? @rx is harder to understand.

Comment thread content/2-how-crs-works/2-3-false-positives-and-tuning.md Outdated
@theseion
Copy link
Copy Markdown
Contributor

Please mark comments as resolved when you're done with them (two where still unresolved). That helps me, as a reviewer, to know that you've seen every comment. If you decide to not apply a suggestion or act on a comment (which is a valid choice), please provide a reason for why. Otherwise I have to assume you didn't read or understand the comment.

@theseion
Copy link
Copy Markdown
Contributor

Please resolve the open comments @touchweb-vincent.

@touchweb-vincent
Copy link
Copy Markdown
Contributor Author

@theseion All comments have been marked as resolved here.

```apache
# CRS Rule Exclusion: 941150 - XSS Filter - Category 5: Disallowed HTML Attributes
SecRule REQUEST_URI "@beginsWith /dynamic/new_post" \
SecRule REQUEST_URI "@rx ^/dynamic/new_post" \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@touchweb-vincent I see where your coming from, but I think kind of information is better suited in the section about creating rules. I think we should keep the false positive tuning doc simple to avoid overloading the reader with too much information.

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.

4 participants