-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Fixing FP on plugins page #32
Conversation
@azurit LGTM, but do you have any example payloads that you can add to the tests file? |
@EsadCetiner I have an example data but not sure if it's suitable for a test for rule
Also, i'm not sure about rule
I would include it in the exclusion rule with a comment that it is not a case anymore on CRS 4 and can be removed after we drop support for CRS 3. What do you think? |
@azurit I don't see a reason why you shouldn't, it's legitimete data that's being wrongfully blocked. I'd add a test for 951240 as well even if the false positive doesn't exist with CRSv4. @theseion and I have been thinking of rewriting false positive tests for rule exclusion plugins to check if no rules are being triggered against a set of payloads known to cause false positives. The idea came from reviewing a PR but I haven't explored the idea just yet. |
Co-authored-by: Esad Cetiner <[email protected]>
Co-authored-by: Esad Cetiner <[email protected]>
Plugin descriptions contains lots of data (like changelogs, error messages and so on) that triggers our response body rules.