Repository navigation
exprhelpers: optional scenario argument for GetDecisionsCount, GetActiveDecisionsCount and GetActiveDecisionsTimeLeft - #4791
Conversation
…nd GetActiveDecisionsTimeLeft GetDecisionsSinceCount can filter by scenario since crowdsecurity#4756, but the two active-decision helpers still look at every decision on the IP, so a postoverflow can't ask "does this scenario's own ban still have time left". Give both the same optional trailing scenario argument, with the same exact match, and let GetActiveDecisionsTimeLeftByValue take the scenario pointer like CountDecisionsByValue does. Without it they behave as before.
The last of the four decision helpers without it. With a scenario it answers "has this scenario banned the IP before" without picking a window, the first rung of a per-scenario ban ladder. The query already took the filter; this only passes it through and registers the two-argument signature.
|
@tommy-ng0: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
|
@tommy-ng0: There are no area labels on this PR. You can add as many areas as you see fit.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
|
/kind enhancement |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4791 +/- ##
===========================================
+ Coverage 45.28% 71.82% +26.54%
===========================================
Files 518 545 +27
Lines 35942 37165 +1223
===========================================
+ Hits 16275 26694 +10419
+ Misses 19656 10470 -9186
+ Partials 11 1 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What and why
#4756 gave
GetDecisionsSinceCountan optional scenario argument so a profile can escalate ban length per scenario. I should have done the other three decision helpers in the same PR: this bringsGetDecisionsCount,GetActiveDecisionsCountandGetActiveDecisionsTimeLeftto parity, with the same optional trailingscenarioand the same exact match.@blotus please let me know if you have any questions/ reviews :)
How it was tested
Three new table tests in the shape of
TestGetDecisionsSinceCountWithScenario: scenario omitted, the same scenario (expired decision counted byGetDecisionsCount, not by the active helpers; own time left rather than the IP's longest), another scenario, an unknown scenario, an unknown IP, and one argument too many failing to compile. With the scenario filters removed, eight sub-tests fail.Live run: crowdsec built from this branch, in the v1.8.2 image,
crowdsecurity/sshdinstalled, with the postoverflow and profile above. A manual 2h ban on the IP carrying scenariocrowdsecurity/ssh-slow-bf, then six failed logins:ssh-bfwas not skipped (the IP had 2h left, but none fromssh-bf) and got a 1h ban:GetDecisionsCountwith the scenario saw 0, where the IP-wide count would have given 4h.Ban for 203.0.113.10 whitelisted, reason [this scenario already banned the source with >30m left]. Thessh-slow-bfoverflow from the same lines was skipped too, by the manual ban carrying its scenario.golangci-lint was not run locally.
Checklist
cscli -o json|rawoutput.AI assistance used: mostly