Skip to content

Commit f550dbc

Browse files
BornToBeRootclaude
andauthored
Feature: Firewall code review and fixes (#3637)
* Feature: Firewall search, F5 and source sort improvements - 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> * Feature: Firewall keep selection, row details, export and program browse - 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> * Fix: Firewall load errors, runspace recovery and faster loading - 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> * Chore: Remove prepared Firewall profile code (not planned) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix: Firewall local address keywords 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> * Fix: Firewall don't restore selection of a rule hidden by the search Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Docs: Add PR link to changelog Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix: Copilot feedback for Firewall review - 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> * Chore: Remove obvious timer comment Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 35ebaea commit f550dbc

21 files changed

Lines changed: 565 additions & 124 deletions
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Globalization;
4+
using System.Windows.Data;
5+
using NETworkManager.Localization.Resources;
6+
using NETworkManager.Models.Firewall;
7+
8+
namespace NETworkManager.Converters;
9+
10+
/// <summary>
11+
/// Convert <see cref="FirewallRuleHiddenProperties" /> to a translated, comma-separated <see cref="string" />
12+
/// (e.g. "ICMP types, Service").
13+
/// </summary>
14+
public sealed class FirewallRuleHiddenPropertiesToStringConverter : IValueConverter
15+
{
16+
/// <summary>
17+
/// Convert <see cref="FirewallRuleHiddenProperties" /> to a translated, comma-separated <see cref="string" />.
18+
/// </summary>
19+
/// <param name="value">Object from type <see cref="FirewallRuleHiddenProperties" />.</param>
20+
/// <param name="targetType"></param>
21+
/// <param name="parameter"></param>
22+
/// <param name="culture"></param>
23+
/// <returns>Translated <see cref="FirewallRuleHiddenProperties" />.</returns>
24+
public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
25+
{
26+
return value is not FirewallRuleHiddenProperties properties
27+
? "-/-"
28+
: string.Join(", ", GetNames(properties));
29+
}
30+
31+
/// <summary>
32+
/// !!! Method not implemented !!!
33+
/// </summary>
34+
/// <param name="value"></param>
35+
/// <param name="targetType"></param>
36+
/// <param name="parameter"></param>
37+
/// <param name="culture"></param>
38+
/// <returns></returns>
39+
public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
40+
{
41+
throw new NotImplementedException();
42+
}
43+
44+
/// <summary>
45+
/// Gets the translated names of the hidden properties (e.g. "ICMP types", "Service").
46+
/// Also used for the warning shown before a rule is copied.
47+
/// </summary>
48+
/// <param name="properties">The hidden properties.</param>
49+
/// <returns>The translated names.</returns>
50+
public static List<string> GetNames(FirewallRuleHiddenProperties properties)
51+
{
52+
var names = new List<string>();
53+
54+
if (properties.HasFlag(FirewallRuleHiddenProperties.IcmpTypes))
55+
names.Add(Strings.IcmpTypes);
56+
57+
if (properties.HasFlag(FirewallRuleHiddenProperties.Service))
58+
names.Add(Strings.Service);
59+
60+
if (properties.HasFlag(FirewallRuleHiddenProperties.Package))
61+
names.Add(Strings.AppPackage);
62+
63+
if (properties.HasFlag(FirewallRuleHiddenProperties.InterfaceAlias))
64+
names.Add(Strings.NetworkAdapters);
65+
66+
if (properties.HasFlag(FirewallRuleHiddenProperties.Security))
67+
names.Add(Strings.UsersAndAuthentication);
68+
69+
return names;
70+
}
71+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
using System;
2+
using System.Globalization;
3+
using System.Windows.Data;
4+
using NETworkManager.Localization;
5+
using NETworkManager.Models.Firewall;
6+
7+
namespace NETworkManager.Converters;
8+
9+
/// <summary>
10+
/// Convert <see cref="FirewallRuleSource" /> to translated <see cref="string" /> or vice versa.
11+
/// </summary>
12+
public sealed class FirewallRuleSourceToStringConverter : IValueConverter
13+
{
14+
/// <summary>
15+
/// Convert <see cref="FirewallRuleSource" /> to translated <see cref="string" />.
16+
/// </summary>
17+
/// <param name="value">Object from type <see cref="FirewallRuleSource" />.</param>
18+
/// <param name="targetType"></param>
19+
/// <param name="parameter"></param>
20+
/// <param name="culture"></param>
21+
/// <returns>Translated <see cref="FirewallRuleSource" />.</returns>
22+
public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
23+
{
24+
return value is not FirewallRuleSource source
25+
? "-/-"
26+
: ResourceTranslator.Translate(ResourceIdentifier.FirewallRuleSource, source);
27+
}
28+
29+
/// <summary>
30+
/// !!! Method not implemented !!!
31+
/// </summary>
32+
/// <param name="value"></param>
33+
/// <param name="targetType"></param>
34+
/// <param name="parameter"></param>
35+
/// <param name="culture"></param>
36+
/// <returns></returns>
37+
public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
38+
{
39+
throw new NotImplementedException();
40+
}
41+
}

‎Source/NETworkManager.Localization/ResourceIdentifier.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,6 @@ public enum ResourceIdentifier
2828
FirewallInterfaceType,
2929
FirewallRuleDirection,
3030
FirewallRuleAction,
31+
FirewallRuleSource,
3132
NeighborState
3233
}

‎Source/NETworkManager.Localization/Resources/StaticStrings.Designer.cs‎

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Source/NETworkManager.Localization/Resources/StaticStrings.resx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,12 @@
351351
<data name="ExampleFirewallAddresses" xml:space="preserve">
352352
<value>10.0.0.0/8; LocalSubnet</value>
353353
</data>
354+
<data name="ExampleFirewallLocalAddresses" xml:space="preserve">
355+
<value>192.168.1.10; 10.0.0.0/8</value>
356+
</data>
357+
<data name="ExampleFirewallProgram" xml:space="preserve">
358+
<value>C:\Program Files\GameServer\server.exe</value>
359+
</data>
354360
<data name="ActiveDirectorySearchBase" xml:space="preserve">
355361
<value>OU=Computers,DC=borntoberoot,DC=net</value>
356362
</data>

‎Source/NETworkManager.Localization/Resources/Strings.Designer.cs‎

Lines changed: 63 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Source/NETworkManager.Localization/Resources/Strings.resx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4415,6 +4415,9 @@ The new rule may apply to more traffic than the original rule (e.g. to all servi
44154415
<data name="FailedToLoadFirewallRulesMessage" xml:space="preserve">
44164416
<value>Failed to load firewall rules. {0}</value>
44174417
</data>
4418+
<data name="FirewallRulesLoadedWithErrorsMessage" xml:space="preserve">
4419+
<value>Reloaded at {0}, but not all firewall rules or their properties could be loaded, so the list may be incomplete (see log file for details). {1}</value>
4420+
</data>
44184421
<data name="DeleteFirewallRuleMessage" xml:space="preserve">
44194422
<value>The selected firewall rule is permanently deleted:
44204423

@@ -4492,6 +4495,21 @@ The new rule may apply to more traffic than the original rule (e.g. to all servi
44924495
<data name="FirewallRuleAction_Allow" xml:space="preserve">
44934496
<value>Allow</value>
44944497
</data>
4498+
<data name="FirewallRuleSource_Local" xml:space="preserve">
4499+
<value>Local</value>
4500+
</data>
4501+
<data name="FirewallRuleSource_GroupPolicy" xml:space="preserve">
4502+
<value>Group Policy</value>
4503+
</data>
4504+
<data name="FirewallRuleSource_MDM" xml:space="preserve">
4505+
<value>Mobile Device Management (MDM)</value>
4506+
</data>
4507+
<data name="FirewallRuleSource_Other" xml:space="preserve">
4508+
<value>Windows (runtime)</value>
4509+
</data>
4510+
<data name="UnsupportedProperties" xml:space="preserve">
4511+
<value>Unsupported properties</value>
4512+
</data>
44954513
<data name="LocalPorts" xml:space="preserve">
44964514
<value>Local ports</value>
44974515
</data>
@@ -4504,6 +4522,9 @@ The new rule may apply to more traffic than the original rule (e.g. to all servi
45044522
<data name="RemoteAddresses" xml:space="preserve">
45054523
<value>Remote addresses</value>
45064524
</data>
4525+
<data name="EnterValidFirewallLocalAddress" xml:space="preserve">
4526+
<value>Enter a valid IP address, subnet (e.g. 10.0.0.0/8), range (e.g. 10.0.0.1-10.0.0.10), or the keyword Any. Other keywords (e.g. LocalSubnet) are only valid for remote addresses.</value>
4527+
</data>
45074528
<data name="EnterValidFirewallAddress" xml:space="preserve">
45084529
<value>Enter a valid IP address, subnet (e.g. 10.0.0.0/8), range (e.g. 10.0.0.1-10.0.0.10), or keyword (Any, LocalSubnet, DNS, DHCP, WINS, DefaultGateway, Internet, Intranet, IntranetRemoteAccess, PlayToDevice, CaptivePortal)</value>
45094530
</data>

‎Source/NETworkManager.Models/Export/ExportManager.FirewallRule.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ private static void CreateCsv(IEnumerable<FirewallRule> collection, string fileP
4747
sb.AppendLine(
4848
$"{nameof(FirewallRule.Name)}," +
4949
$"{nameof(FirewallRule.IsEnabled)}," +
50+
$"{nameof(FirewallRule.IsManaged)}," +
51+
$"{nameof(FirewallRule.Source)}," +
5052
$"{nameof(FirewallRule.Direction)}," +
5153
$"{nameof(FirewallRule.Action)}," +
5254
$"{nameof(FirewallRule.Protocol)}," +
@@ -63,6 +65,8 @@ private static void CreateCsv(IEnumerable<FirewallRule> collection, string fileP
6365
sb.AppendLine(
6466
$"{CsvHelper.QuoteString(rule.Name)}," +
6567
$"{rule.IsEnabled}," +
68+
$"{rule.IsManaged}," +
69+
$"{rule.Source}," +
6670
$"{rule.Direction}," +
6771
$"{rule.Action}," +
6872
$"{rule.Protocol}," +
@@ -90,6 +94,8 @@ from rule in collection
9094
select new XElement(nameof(FirewallRule),
9195
new XElement(nameof(FirewallRule.Name), rule.Name),
9296
new XElement(nameof(FirewallRule.IsEnabled), rule.IsEnabled),
97+
new XElement(nameof(FirewallRule.IsManaged), rule.IsManaged),
98+
new XElement(nameof(FirewallRule.Source), rule.Source),
9399
new XElement(nameof(FirewallRule.Direction), rule.Direction),
94100
new XElement(nameof(FirewallRule.Action), rule.Action),
95101
new XElement(nameof(FirewallRule.Protocol), rule.Protocol),
@@ -119,6 +125,8 @@ private static void CreateJson(IReadOnlyList<FirewallRule> collection, string fi
119125
{
120126
rule.Name,
121127
rule.IsEnabled,
128+
rule.IsManaged,
129+
Source = rule.Source.ToString(),
122130
Direction = rule.Direction.ToString(),
123131
Action = rule.Action.ToString(),
124132
Protocol = rule.Protocol.ToString(),

0 commit comments

Comments
 (0)