Add Microsoft.Windows/FirewallRuleList resource#1453
Add Microsoft.Windows/FirewallRuleList resource#1453SteveL-MSFT wants to merge 2 commits intoPowerShell:mainfrom
Microsoft.Windows/FirewallRuleList resource#1453Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new DSC resource, Microsoft.Windows/FirewallRuleList, implemented as a Rust-based command resource to manage Windows Firewall rules via the netfw.h COM APIs.
Changes:
- Introduces the
windows_firewallresource manifest and Rust implementation (get/set/export). - Adds PowerShell Pester tests covering get/set/export scenarios, including wildcard filtering for export.
- Registers the new crate in the workspace and enables the Windows Firewall feature in the
windowscrate feature set.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/windows_firewall/windows_firewall.dsc.resource.json | New resource manifest + embedded JSON schema for FirewallRuleList. |
| resources/windows_firewall/src/main.rs | Resource CLI entrypoint (get/set/export routing, input parsing, JSON output). |
| resources/windows_firewall/src/firewall.rs | Windows-only COM implementation for querying/updating/exporting firewall rules. |
| resources/windows_firewall/src/types.rs | Data model types for rules + error type used by the implementation. |
| resources/windows_firewall/src/util.rs | Filter/wildcard matching utilities + a small unit test. |
| resources/windows_firewall/locales/en-us.toml | i18n strings for CLI and firewall operation errors. |
| resources/windows_firewall/Cargo.toml | New Rust crate definition/dependencies for the resource. |
| resources/windows_firewall/.project.data.json | Build/packaging metadata for the new resource. |
| resources/windows_firewall/tests/windows_firewall_get.tests.ps1 | New Pester tests for get behavior. |
| resources/windows_firewall/tests/windows_firewall_set.tests.ps1 | New Pester tests for set behavior. |
| resources/windows_firewall/tests/windows_firewall_export.tests.ps1 | New Pester tests for export behavior + filtering semantics. |
| Cargo.toml | Adds resources/windows_firewall to workspace members/default-members/Windows group + enables WindowsFirewall feature. |
| Cargo.lock | Locks new crate/dependency graph entries. |
| lib/dsc-lib-jsonschema/.versions.json | Bumps latest patch version entry and adds V3_1_3 to the recognized list. |
| Describe 'Microsoft.Windows/FirewallRuleList - export operation' -Skip:(!$IsWindows) { | ||
| BeforeDiscovery { | ||
| $isElevated = if ($IsWindows) { | ||
| ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( | ||
| [Security.Principal.WindowsBuiltInRole]::Administrator) |
There was a problem hiding this comment.
BeforeAll performs dsc resource export -r ..., but the file only skips on !$IsWindows. Because BeforeAll executes even when each It is skipped, non-elevated runs will fail during setup. Skip the Describe when !$isElevated (or gate the BeforeAll export/setup) so the suite can run safely without admin rights.
| impl ComGuard { | ||
| fn new() -> Result<Self, FirewallError> { | ||
| unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) } | ||
| .ok() |
There was a problem hiding this comment.
CoInitializeEx(...) returns a Result, but this code calls .ok() (which turns it into an Option) and then calls map_err(...), which won’t compile. Drop the .ok() and map the error on the Result (or use the same CoInitializeEx(...).is_ok() pattern used elsewhere in the repo) so COM init failures are handled without a build break.
| .ok() |
| if let Some(protocol) = desired.protocol { | ||
| validate_protocol(protocol)?; | ||
|
|
||
| // Reject port specifications for protocols that don't support them (e.g. ICMP). | ||
| // On existing rules the ports are cleared automatically, but on new rules | ||
| // (existing_protocol == None) the conflicting SetLocalPorts/SetRemotePorts call | ||
| // would fail with a confusing COM error. | ||
| if !protocol_supports_ports(protocol) && existing_protocol.is_none() | ||
| && (desired.local_ports.is_some() || desired.remote_ports.is_some()) | ||
| { | ||
| return Err(t!("firewall.portsNotAllowed", name = name, protocol = protocol).to_string().into()); | ||
| } |
There was a problem hiding this comment.
When updating an existing rule, if desired.protocol is set to a value that doesn’t support ports (e.g., ICMP) and local_ports/remote_ports are also provided, later SetLocalPorts/SetRemotePorts calls will fail with a COM error. Consider validating and returning the portsNotAllowed error whenever the selected protocol doesn’t support ports (not just when creating new rules) so callers get a clear message.
| Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$IsWindows) { | ||
| BeforeDiscovery { | ||
| $isElevated = if ($IsWindows) { | ||
| ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( | ||
| [Security.Principal.WindowsBuiltInRole]::Administrator) |
There was a problem hiding this comment.
The suite only skips on !$IsWindows, but BeforeAll runs unconditionally and calls Get-/New-NetFirewallRule, which will fail when tests run non-elevated. Use -Skip:(!$IsWindows -or !$isElevated) at the Describe/Context level (or gate the BeforeAll setup on $isElevated) so non-admin runs don’t fail during discovery/setup.
| Describe 'Microsoft.Windows/FirewallRuleList - get operation' -Skip:(!$IsWindows) { | ||
| BeforeDiscovery { | ||
| $isElevated = if ($IsWindows) { | ||
| ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( | ||
| [Security.Principal.WindowsBuiltInRole]::Administrator) |
There was a problem hiding this comment.
BeforeAll runs even when each It is skipped, but it calls dsc resource export -r ... (and this resource requires an elevated security context). In non-elevated runs this will fail the whole file before the It-level -Skip applies. Consider skipping the entire Describe when !$isElevated (or moving the export/setup logic into elevated-only blocks).
PR Summary
Add resource to manage Windows Firewall rules