bugfixes#155
Open
cmpl-ernestp wants to merge 2 commits into
Open
Conversation
…g added as "attempts"
There was a problem hiding this comment.
Pull request overview
This PR addresses three production-impacting issues in EvlWatcher’s IP-banning pipeline: whitelist evaluation during rule processing, installer behavior that overwrote configuration on reinstall/upgrade, and incorrect counting of “Audit Success” events as failed attempts.
Changes:
- Thread whitelist checks into
GenericIPBlockingTaskso whitelisted IPs are ignored during strike accumulation and permaban evaluation. - Preserve an existing
config.xmlduring NSIS installs to support GPO-friendly deployments. - Extend extracted event DTOs to carry
Keywords, enablingGenericIPBlockingTaskto skip Audit Success events during evaluation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Source/EvlWatcher/EvlWatcher/tasks/IGenericTaskFactory.cs | Adds/removes imports related to task factory changes. |
| Source/EvlWatcher/EvlWatcher/tasks/GenericIPBlockingTask.cs | Adds whitelist-aware evaluation and skips Audit Success events using Keywords. |
| Source/EvlWatcher/EvlWatcher/tasks/DefaultGenericTaskFactory.cs | Passes whitelist predicate into task creation. |
| Source/EvlWatcher/EvlWatcher/NSIS/make.nsi | Preserves existing config.xml during installation. |
| Source/EvlWatcher/EvlWatcher/EvlWatcher.cs | Wires default factory with whitelist predicate; includes Keywords in extracted events. |
| Source/EvlWatcher/EvlWatcher/DTOs/ExtractedEventRecord.cs | Adds Keywords field used by task evaluation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+110
to
+114
| if (_isWhiteListed != null && _isWhiteListed(kvp.Key)) | ||
| { | ||
| _logger.Dump($"IP {kvp.Key} reached permaban threshold but is whitelisted", SeverityLevel.Info); | ||
| continue; | ||
| } |
| _logger = logger; | ||
| _serviceconfiguration = configuration; | ||
| _genericTaskFactory = genericTaskFactory; | ||
| _genericTaskFactory = genericTaskFactory ?? new DefaultGenericTaskFactory(logger, IsWhiteListed); |
Comment on lines
436
to
+440
| ExtractedEventRecord eer = new ExtractedEventRecord() | ||
| { | ||
| TimeCreated = r.TimeCreated.Value, | ||
| Xml = r.ToXml() | ||
| Xml = r.ToXml(), | ||
| Keywords = r.Keywords |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed whitelist not being checked during rule evaluation
Fixed config file being overwritten with every install, complicating GPO deployments
Fixed Audit Success events being included in total attempts, resulting in legitimate users getting banned for frequent logins