Repository navigation
Feature: Firewall code review and fixes - #3637
Merged
Merged
Conversation
- Search also covers program, ports, addresses, description and translated direction/action/protocol; filtered after a short delay while typing - F5 refreshes the rule list - Sort by rule source groups managed / not managed / policy rules - Program placeholder shows an example program path - Fix runspace comment Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Keep the selected rule selected and scrolled into view after the list is reloaded - Row details show the rule source and unsupported (hidden) properties - Export contains IsManaged and Source - Program can be selected with a file dialog or drag and drop Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Show an error if the rules could not be loaded and a message if they were only loaded partially, instead of an empty list - Recreate the PowerShell runspace if it could not be created or is broken (was cached until restart) - Query the filters of many managed rules at once instead of per rule (one script for both modes) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Windows only accepts the keyword Any for -LocalAddress, and Any4 / Any6 are not valid at all (tested with New-NetFirewallRule / Set-NetFirewallRule). Validate local addresses separately and use an example without keywords as placeholder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Filter-query errors can be hidden, and the advertised Explorer drag-and-drop workflow is blocked when elevated.
2 open issues
2 files not reviewed
Source/NETworkManager.Localization/Resources/StaticStrings.Designer.cs: Generated fileSource/NETworkManager.Localization/Resources/Strings.Designer.cs: Generated file
What changed in this PR
Improves the Firewall tool’s reliability and usability as part of #3609’s tool-review work.
Changes:
- Adds recoverable rule loading, partial-error reporting, and bulk filter queries.
- Improves search, selection retention, rule details, exports, and program selection.
- Tightens address validation and updates documentation and resources.
Windows build and manual verification remain necessary.
| File | Description |
|---|---|
| Website/docs/changelog/next-release.md | Records Firewall improvements and fixes. |
| Website/docs/application/firewall.md | Documents updated behavior and validation. |
| Source/NETworkManager/Views/FirewallView.xaml.cs | Scrolls restored selection into view. |
| Source/NETworkManager/Views/FirewallView.xaml | Adds F5 refresh and source details. |
| Source/NETworkManager/Views/FirewallRuleChildWindow.xaml.cs | Handles program file drops. |
| Source/NETworkManager/Views/FirewallRuleChildWindow.xaml | Adds program browsing and local-address validation. |
| Source/NETworkManager/ViewModels/FirewallViewModel.cs | Improves search, refresh feedback, and selection retention. |
| Source/NETworkManager/ViewModels/FirewallRuleViewModel.cs | Adds program file picker. |
| Source/NETworkManager.Validators/EmptyOrFirewallAddressValidator.cs | Restricts address keywords. |
| Source/NETworkManager.Profiles/ProfileViewManager.cs | Removes commented-out Firewall registration. |
| Source/NETworkManager.Profiles/ProfileName.cs | Removes unused Firewall enum member. |
| Source/NETworkManager.Profiles/ProfileInfo.cs | Removes commented-out import. |
| Source/NETworkManager.Models/Firewall/FirewallRule.cs | Adds source sorting order. |
| Source/NETworkManager.Models/Firewall/Firewall.cs | Updates runspace recovery, queries, and error reporting. |
| Source/NETworkManager.Models/Export/ExportManager.FirewallRule.cs | Exports rule ownership and source. |
| Source/NETworkManager.Localization/Resources/Strings.resx | Adds labels and error messages. |
| Source/NETworkManager.Localization/Resources/Strings.Designer.cs | Exposes new localized strings. |
| Source/NETworkManager.Localization/Resources/StaticStrings.resx | Adds address and program examples. |
| Source/NETworkManager.Localization/Resources/StaticStrings.Designer.cs | Exposes new examples. |
| Source/NETworkManager.Localization/ResourceIdentifier.cs | Registers rule-source translation. |
| Source/NETworkManager.Converters/FirewallRuleSourceToStringConverter.cs | Translates rule sources. |
| Source/NETworkManager.Converters/FirewallRuleHiddenPropertiesToStringConverter.cs | Formats unsupported-property labels. |
Review effort: Balanced
| } | ||
| // A query without a match (e.g. no rules managed by NETworkManager yet) is not an error | ||
| var errors = ps.Streams.Error | ||
| .Where(error => error.FullyQualifiedErrorId?.StartsWith("CmdletizationQuery_NotFound", StringComparison.Ordinal) != true) |
Comment on lines
+208
to
+209
| mah:TextBoxHelper.ButtonCommand="{Binding BrowseFileCommand}" AllowDrop="True" | ||
| Drop="TextBoxProgram_Drop" PreviewDragOver="TextBoxProgram_PreviewDragOver"> |
- Only ignore 'not found' errors of the rule query, not of the filter queries - Remove drag and drop for the program (blocked by Windows, the dialog requires admin rights) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Changes proposed in this pull request
Related issue(s)
To-Do
Contributing
By submitting this pull request, I confirm the following: