Feature: Firewall app by BornToBeRoot · Pull Request #3383 · BornToBeRoot/NETworkManager

BornToBeRoot · 2026-04-11T01:51:33Z

Changes proposed in this pull request

Add Firewall App

Related issue(s)

Fixes [New App] Firewall module/application #3259

To-Do

Update documentation to reflect this changes
Update changelog to reflect this changes

Copilot

Pull request overview

Adds an initial “Firewall” app surface to NETworkManager, including a WPF DataGrid-based UI and a new firewall model layer that loads NETworkManager-owned Windows Firewall rules via PowerShell.

Changes:

Introduce a new Firewall rules grid UI with row details, search, refresh, and context menus.
Add a new NETworkManager.Models.Firewall model layer (rule DTOs + PowerShell-backed rule loading/parsing).
Add supporting enums/models (protocols, directions, port specs, interface types, network profiles).

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
Source/NETworkManager/Views/FirewallView.xaml.cs	Adds click handler to toggle DataGrid row details.
Source/NETworkManager/Views/FirewallView.xaml	Reworks Firewall view to a searchable/refreshable MultiSelectDataGrid with row details.
Source/NETworkManager/ViewModels/FirewallViewModel.cs	Adds rule collection/view, filtering, refresh logic, and rule-related commands (mostly TODO).
Source/NETworkManager.Models/NETworkManager.Models.csproj	Adds Firewall folder entry to the Models project.
Source/NETworkManager.Models/Network/NetworkProfiles.cs	Adds enum representing Windows network profiles.
Source/NETworkManager.Models/Firewall/FirewallRuleProgram.cs	Adds program/path model for firewall rules.
Source/NETworkManager.Models/Firewall/FirewallRuleDirection.cs	Adds inbound/outbound direction enum.
Source/NETworkManager.Models/Firewall/FirewallRuleAction.cs	Adds allow/block action enum.
Source/NETworkManager.Models/Firewall/FirewallRule.cs	Adds firewall rule DTO + display helpers.
Source/NETworkManager.Models/Firewall/FirewallProtocol.cs	Adds protocol enum mapping (incl. Any/255).
Source/NETworkManager.Models/Firewall/FirewallPortSpecification.cs	Adds port range/single-port specification model.
Source/NETworkManager.Models/Firewall/FirewallPortLocation.cs	Adds local vs remote port location enum.
Source/NETworkManager.Models/Firewall/FirewallInterfaceType.cs	Adds interface type enum.
Source/NETworkManager.Models/Firewall/Firewall.cs	Adds PowerShell-based rule discovery + parsing helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Agent-Logs-Url: https://github.com/BornToBeRoot/NETworkManager/sessions/5215e397-497d-4847-a3f7-a7454cafebb3 Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 48 out of 52 changed files in this pull request and generated 11 comments.

Files not reviewed (2)

Source/NETworkManager.Localization/Resources/StaticStrings.Designer.cs: Language not supported
Source/NETworkManager.Localization/Resources/Strings.Designer.cs: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T22:08:32Z

+            var slashIndex = token.IndexOf('/');
+            var addressPart = slashIndex > 0 ? token[..slashIndex] : token;
+
+            if (!IPAddress.TryParse(addressPart, out _))
+                return new ValidationResult(false, Strings.EnterValidFirewallAddress);


EmptyOrFirewallAddressValidator accepts CIDR input but only validates the address portion before / and does not validate the prefix length (e.g., 10.0.0.0/999 would pass). This will allow invalid inputs through validation and then fail later when creating the firewall rule. Consider parsing/validating the CIDR suffix (IPv4: 0-32, IPv6: 0-128) when a / is present, and reject tokens with non-numeric/out-of-range prefix lengths.

Copilot · 2026-04-19T22:08:32Z

+    ///     Returns <c>null</c> when all three profiles are active so that a <c>TargetNullValue</c> binding
+    ///     can supply the translated "Any" label.
+    /// </summary>
+    /// <param name="value">A <see cref="bool" /> array with exactly three elements.</param>
+    /// <param name="targetType"></param>
+    /// <param name="parameter"></param>
+    /// <param name="culture"></param>
+    /// <returns>Localized, comma-separated profile list (e.g. "Domain, Private, Public").</returns>
+    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
+    {
+        if (value is not bool[] { Length: 3 } profiles)
+            return "-/-";
+
+        var names = new List<string>(3);
+        if (profiles[0]) names.Add(Strings.Domain);
+        if (profiles[1]) names.Add(Strings.Private);
+        if (profiles[2]) names.Add(Strings.Public);
+
+        return names.Count == 0 ? "-" : string.Join(", ", names);


The XML docs say this converter “Returns null when all three profiles are active … so that a TargetNullValue binding can supply the translated ‘Any’ label”, but the implementation never returns null and also returns "-" when no profiles are selected. Given the firewall PowerShell layer treats both all-true and all-false as Any, the converter should likely return a localized “Any” (or null to use TargetNullValue) for those cases to avoid misleading UI/exports.

Suggested change

/// Returns <c>null</c> when all three profiles are active so that a <c>TargetNullValue</c> binding

/// can supply the translated "Any" label.

/// </summary>

/// <param name="value">A <see cref="bool" /> array with exactly three elements.</param>

/// <param name="targetType"></param>

/// <param name="parameter"></param>

/// <param name="culture"></param>

/// <returns>Localized, comma-separated profile list (e.g. "Domain, Private, Public").</returns>

public object Convert(object value, Type targetType, object parameter, CultureInfo culture)

{

if (value is not bool[] { Length: 3 } profiles)

return "-/-";

var names = new List<string>(3);

if (profiles[0]) names.Add(Strings.Domain);

if (profiles[1]) names.Add(Strings.Private);

if (profiles[2]) names.Add(Strings.Public);

return names.Count == 0 ? "-" : string.Join(", ", names);

/// Returns <c>null</c> when all three profiles are active, or when none are active, so that a

/// <c>TargetNullValue</c> binding can supply the translated "Any" label.

/// </summary>

/// <param name="value">A <see cref="bool" /> array with exactly three elements.</param>

/// <param name="targetType"></param>

/// <param name="parameter"></param>

/// <param name="culture"></param>

/// <returns>

/// Localized, comma-separated profile list (e.g. "Domain, Private, Public"), or <c>null</c> for

/// profile combinations that represent "Any".

/// </returns>

public object Convert(object value, Type targetType, object parameter, CultureInfo culture)

{

if (value is not bool[] { Length: 3 } profiles)

return "-/-";

var allProfilesActive = profiles[0] && profiles[1] && profiles[2];

var noProfilesActive = !profiles[0] && !profiles[1] && !profiles[2];

if (allProfilesActive || noProfilesActive)

return null;

var names = new List<string>(3);

if (profiles[0]) names.Add(Strings.Domain);

if (profiles[1]) names.Add(Strings.Private);

if (profiles[2]) names.Add(Strings.Public);

return string.Join(", ", names);

Copilot · 2026-04-19T22:08:33Z

+                            <DataGridCheckBoxColumn Header="{x:Static localization:Strings.Enabled}"
+                                                    Binding="{Binding (firewall:FirewallRule.IsEnabled)}"
+                                                    SortMemberPath="IsEnabled"
+                                                    MinWidth="70" />
+
+                            <!-- Name -->
+                            <DataGridTextColumn Header="{x:Static localization:Strings.Name}"
+                                                Binding="{Binding (firewall:FirewallRule.Name)}"
+                                                SortMemberPath="Name"
+                                                MinWidth="150" />


The DataGrid bindings use attached-property syntax like Binding="{Binding (firewall:FirewallRule.IsEnabled)} / (firewall:FirewallRule.Name). FirewallRule is a plain CLR model (not a DependencyObject with attached DPs), so these paths will not resolve and will produce binding errors at runtime. Use normal property paths (e.g., IsEnabled, Name, Protocol, etc.) for these columns.

Copilot · 2026-04-19T22:08:33Z

+                                        <DataTemplate.Triggers>
+                                            <DataTrigger Binding="{Binding (firewall:FirewallRule.Action)}" Value="Allow">
+                                                <Setter TargetName="ActionIcon" Property="Fill" Value="#badc58" />
+                                                <Setter TargetName="ActionIcon" Property="OpacityMask">
+                                                    <Setter.Value>
+                                                        <VisualBrush Stretch="Uniform" Visual="{iconPacks:Material Kind=Check}" />
+                                                    </Setter.Value>
+                                                </Setter>
+                                            </DataTrigger>


The action-related DataTrigger binds via (firewall:FirewallRule.Action) and compares Value="Allow" as a string. Even after fixing the binding path, triggers on enum properties should compare to an enum constant (e.g., {x:Static firewall:FirewallRuleAction.Allow}), otherwise the trigger won’t fire reliably and the icon/text coloring may be wrong.

Copilot · 2026-04-19T22:08:33Z

+        try
+        {
+            using var ps = SMA.PowerShell.Create();
+            ps.AddScript("Get-NetConnectionProfile | Select-Object InterfaceAlias, NetworkCategory");
+
+            foreach (var result in ps.Invoke())
+            {
+                var alias = result.Properties["InterfaceAlias"]?.Value?.ToString();
+                var category = result.Properties["NetworkCategory"]?.Value?.ToString();
+
+                if (string.IsNullOrEmpty(alias))
+                    continue;
+
+                profileByAlias[alias] = category switch
+                {
+                    "DomainAuthenticated" => NetworkProfile.Domain,
+                    "Private"             => NetworkProfile.Private,
+                    "Public"              => NetworkProfile.Public,
+                    _                     => NetworkProfile.NotConfigured
+                };
+            }
+        }
+        catch
+        {
+            // Profile lookup is best-effort; proceed without profile information on error.
+        }


GetNetworkInterfaces() swallows all exceptions from the PowerShell Get-NetConnectionProfile call (catch { }). That makes failures (missing cmdlet/module, PowerShell issues, access problems) very hard to diagnose and can silently remove the new Network Profile feature. Consider at least logging the exception (best-effort) so issues can be traced in the field, while still continuing without profile info.

Copilot · 2026-04-19T22:08:33Z

+    static Firewall()
+    {
+        SharedRunspace = RunspaceFactory.CreateRunspace();
+        SharedRunspace.Open();
+
+        using var ps = SMA.PowerShell.Create();
+        ps.Runspace = SharedRunspace;
+        ps.AddScript(@"
+Set-ExecutionPolicy -ExecutionPolicy Bypass -Scope Process
+Import-Module NetSecurity -ErrorAction Stop").Invoke();
+    }


The Firewall static constructor opens a runspace and immediately runs Set-ExecutionPolicy Bypass + Import-Module NetSecurity. If either command fails (execution policy restricted, module missing/corrupt, runspace creation issue), the type initializer will throw and can crash the app with a TypeInitializationException before any UI can handle it. Consider lazy initialization with explicit error handling/logging (and surfacing a user-friendly error when the firewall feature is used), rather than doing PowerShell initialization unconditionally in a static ctor.

Copilot · 2026-04-19T22:08:34Z

+                                        <TextBox Grid.Row="0" Grid.Column="6"
+                                                 Text="{Binding (firewall:FirewallRule.InterfaceType), Mode=OneWay, Converter={StaticResource FirewallInterfaceTypeToStringConverter}}" />
+
+                                        <!-- Row 2: Network profiles | Description -->
+                                        <TextBlock Grid.Row="2" Grid.Column="0"
+                                                   Text="{x:Static localization:Strings.NetworkProfiles}"
+                                                   Style="{StaticResource InfoTextBlock}" />
+                                        <TextBox Grid.Row="2" Grid.Column="2"
+                                                 Text="{Binding (firewall:FirewallRule.NetworkProfiles), Mode=OneWay, Converter={StaticResource FirewallNetworkProfilesToStringConverter}}" />
+
+                                        <TextBlock Grid.Row="2" Grid.Column="4"
+                                                   Text="{x:Static localization:Strings.Description}"
+                                                   Style="{StaticResource InfoTextBlock}" />
+                                        <TextBox Grid.Row="2" Grid.Column="6"
+                                                 Text="{Binding (firewall:FirewallRule.Description), Mode=OneWay}" />


The row-details template also uses attached-property binding paths like (firewall:FirewallRule.InterfaceType) / (firewall:FirewallRule.NetworkProfiles) / (firewall:FirewallRule.Description). As with the main columns, these won’t bind on a plain CLR model. Use normal bindings (InterfaceType, NetworkProfiles, Description, etc.) so the details view actually shows values.

Copilot · 2026-04-19T22:08:34Z

+        /*
+        new(SettingsName.Firewall, ApplicationManager.GetIcon(ApplicationName.Firewall),
            SettingsGroup.Application),
-        new SettingsViewInfo(SettingsName.WakeOnLAN, ApplicationManager.GetIcon(ApplicationName.WakeOnLAN),
+        */
+        new(SettingsName.WakeOnLAN, ApplicationManager.GetIcon(ApplicationName.WakeOnLAN),


SettingsViewManager has the Firewall settings entry commented out. If this is unintentional, the Firewall app won’t have a settings page even though new firewall-related settings (e.g., export path/type) were added. If the Firewall app intentionally has no settings yet, consider removing the commented block to avoid dead code and reintroducing the entry when a settings view exists.

Copilot · 2026-04-19T22:08:34Z

+        /*
+        new(ProfileName.Firewall, ApplicationManager.GetIcon(ApplicationName.Firewall),
            ProfileGroup.Application),
-        new ProfileViewInfo(ProfileName.WakeOnLAN, ApplicationManager.GetIcon(ApplicationName.WakeOnLAN),
+        */
+        new(ProfileName.WakeOnLAN, ApplicationManager.GetIcon(ApplicationName.WakeOnLAN),


ProfileViewManager has the Firewall profile entry commented out, and FirewallView.xaml contains large commented-out profile UI blocks. If Firewall is not profile-aware yet, it would be cleaner to remove these commented sections (or gate them behind a feature flag) to reduce noise and avoid drifting/bitrot. If profiles are intended, re-enable the entry/UI so the feature is actually accessible.

Copilot · 2026-04-19T22:08:34Z

+            <!-- <ColumnDefinition Width="Auto" /> -->
+            <!-- <ColumnDefinition MinWidth="{x:Static settings:GlobalStaticConfiguration.Profile_WidthCollapsed}" -->
+            <!--                   Width="{Binding ProfileWidth, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}" -->
+            <!--                   MaxWidth="{x:Static settings:GlobalStaticConfiguration.Profile_MaxWidthExpanded}" /> -->


There are large blocks of commented-out layout/profile-manager UI (column definitions, splitter/expander, etc.). Keeping substantial commented code in the main view makes future maintenance harder and increases the risk of UI drift. Prefer removing these sections or reintroducing them via a configuration/feature toggle so the XAML stays authoritative.

Feature: Firewall app

ff7024e

Copilot AI review requested due to automatic review settings April 11, 2026 01:51

github-actions bot added this to the next-release milestone Apr 11, 2026

Copilot started reviewing on behalf of BornToBeRoot April 11, 2026 01:52 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

Comment thread Source/NETworkManager.Models/Firewall/FirewallRule.cs Outdated

Comment thread Source/NETworkManager.Models/Firewall/FirewallRule.cs Outdated

Comment thread Source/NETworkManager/Views/FirewallView.xaml Outdated

Update Source/NETworkManager.Models/Firewall/FirewallRule.cs

c3bccb0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of BornToBeRoot April 11, 2026 02:01 View session

Fix XML doc comments for Id/Name properties in FirewallRule.cs

7cf2ffd

Agent-Logs-Url: https://github.com/BornToBeRoot/NETworkManager/sessions/5215e397-497d-4847-a3f7-a7454cafebb3 Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com>

Copilot finished work on behalf of BornToBeRoot April 11, 2026 02:02

BornToBeRoot added 19 commits April 11, 2026 05:04

Feature: Add translation

0ea864c

Feature: Add translation and export

ff78d29

Merge branch 'main' into feature/firewall-app-base

271346e

Feature: Add network profile to networkinterfaceview

7058281

Docs: #3383

2252908

Merge branch 'main' into feature/firewall-app-base

e5d3ae4

Merge branch 'main' into feature/firewall-app-base

e04039e

Chore: Hide firewall settings/profile

77d086b

Chore: Refactor the UI

0761db4

Feature: UI & Copy data

ad07dc6

Chore: UI redesign

57a2863

Docs: Discovery Protocol

d90969f

Update package.json

00ed305

Update yarn.lock

aacad1a

Feature: Enable/Disable/Delete Firewall rule

70b404e

Feature: Open Hosts File Editor

a144800

Feature: Firewall add / edit firewall rule

b92a304

Chore: Adjust strings

fa13f29

Feature: Firewall edit/del + docs

6761e73

BornToBeRoot requested a review from Copilot April 19, 2026 22:02

Copilot started reviewing on behalf of BornToBeRoot April 19, 2026 22:02 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Conversation

BornToBeRoot commented Apr 11, 2026

Changes proposed in this pull request

Related issue(s)

To-Do

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants