diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index bc40ac391..ab97e1893 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -304,11 +304,16 @@ jobs: coverage-report: if: github.event_name == 'pull_request' - # Use Linux coverage only: merging all platforms inflates total line count - # because each platform has platform-specific source files (Windows adds ~4500 - # lines from registry/service/DISM resources). Single-platform coverage matches - # local `build.ps1 -codecoverage` results and avoids misleadingly low percentages. - needs: [linux-build, linux-pester] + # Use all platforms for changed-code coverage so platform-specific files are + # included. Keep full-codebase coverage Linux-only to avoid inflating its + # denominator with platform-specific sources. + needs: + - linux-build + - linux-pester + - macos-build + - macos-pester + - windows-build + - windows-pester runs-on: ubuntu-latest permissions: pull-requests: write @@ -321,7 +326,7 @@ jobs: - name: Download coverage artifacts uses: actions/download-artifact@v4 with: - pattern: 'linux*coverage' + pattern: '*coverage' path: coverage-data - name: Consolidate coverage data @@ -339,21 +344,33 @@ jobs: $baseSha = $mergeBase } - # Find all available lcov.info files from coverage artifacts + # Changed-code coverage uses every platform so platform-specific Rust + # files are analyzed. Full-codebase coverage remains Linux-only to + # avoid inflating its denominator with platform-specific sources. $lcovFiles = Get-ChildItem -Path 'coverage-data' -Filter 'lcov.info' -Recurse $pesterLcovFiles = Get-ChildItem -Path 'coverage-data' -Filter 'pester-lcov.info' -Recurse $allLcovFiles = @($lcovFiles) + @($pesterLcovFiles) | Where-Object { $_ } + $linuxLcovFiles = @($allLcovFiles | Where-Object { + ($_.FullName -match '[/\\]linux-[^/\\]+-coverage[/\\]') -or + ($_.FullName -match '[/\\]linux-coverage[/\\]') + }) if ($allLcovFiles.Count -eq 0) { Write-Warning 'No coverage data found from any platform.' "coverage_failed=true" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT return } + if ($linuxLcovFiles.Count -eq 0) { + Write-Warning 'No Linux coverage data found for the full-codebase report.' + "coverage_failed=true" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT + return + } "coverage_failed=false" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT - Write-Verbose -Verbose "Found $($allLcovFiles.Count) LCOV file(s) to merge" + Write-Verbose -Verbose "Found $($allLcovFiles.Count) cross-platform LCOV file(s)" + Write-Verbose -Verbose "Found $($linuxLcovFiles.Count) Linux LCOV file(s)" - # Merge all LCOV files into a single consolidated report + # Merge all platforms for changed-code coverage. $mergedLcovPath = Join-Path $PWD 'merged-lcov.info' if ($allLcovFiles.Count -eq 1) { Copy-Item -Path $allLcovFiles[0].FullName -Destination $mergedLcovPath @@ -361,8 +378,15 @@ jobs: Merge-LcovFile -Path ($allLcovFiles | ForEach-Object { $_.FullName }) -OutputPath $mergedLcovPath -Verbose } - # Full codebase coverage report (always computed) - $fullReport = Get-FullCodeCoverageReport -LcovPath $mergedLcovPath -Verbose + # Merge Linux coverage separately for the full-codebase report. + $linuxMergedLcovPath = Join-Path $PWD 'linux-merged-lcov.info' + if ($linuxLcovFiles.Count -eq 1) { + Copy-Item -Path $linuxLcovFiles[0].FullName -Destination $linuxMergedLcovPath + } else { + Merge-LcovFile -Path ($linuxLcovFiles | ForEach-Object { $_.FullName }) -OutputPath $linuxMergedLcovPath -Verbose + } + + $fullReport = Get-FullCodeCoverageReport -LcovPath $linuxMergedLcovPath -Verbose "full_percentage=$($fullReport.Percentage)" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT "full_covered=$($fullReport.CoveredLines)" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT diff --git a/Cargo.lock b/Cargo.lock index 80f251607..2056c7034 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4314,7 +4314,7 @@ checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" [[package]] name = "windows_firewall" -version = "0.2.0" +version = "0.3.0" dependencies = [ "rust-i18n", "serde", diff --git a/resources/windows_firewall/Cargo.toml b/resources/windows_firewall/Cargo.toml index 1230f4fe7..225b247ac 100644 --- a/resources/windows_firewall/Cargo.toml +++ b/resources/windows_firewall/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "windows_firewall" -version = "0.2.0" +version = "0.3.0" edition = "2024" [package.metadata.i18n] diff --git a/resources/windows_firewall/locales/en-us.toml b/resources/windows_firewall/locales/en-us.toml index d26b594be..3ed8e9e6f 100644 --- a/resources/windows_firewall/locales/en-us.toml +++ b/resources/windows_firewall/locales/en-us.toml @@ -9,11 +9,9 @@ invalidJson = "Invalid JSON input: %{error}" windowsOnly = "This resource is only supported on Windows" [get] -rulesArrayEmpty = "The rules array cannot be empty for get operations" selectorRequired = "Each firewall rule in a get request must include a name" [set] -rulesArrayEmpty = "The rules array cannot be empty for set operations" selectorRequired = "Each firewall rule in a set request must include a name" [firewall] @@ -28,8 +26,10 @@ ruleUpdateFailed = "Failed to update firewall rule '%{name}': %{error}" ruleReadFailed = "Failed to read firewall rule '%{name}': %{error}" portsNotAllowed = "Ports cannot be specified for firewall rule '%{name}' because protocol %{protocol} does not support ports" invalidProfiles = "Invalid profiles value '%{value}'. Valid values are Domain, Private, Public, or All" +emptyUnspecifiedProfiles = "The unspecified rules profiles filter cannot be empty" invalidInterfaceType = "Invalid interface type '%{value}'. Valid values are RemoteAccess, Wireless, Lan, or All" invalidProtocol = "Invalid protocol number '%{value}'. Must be between 0 and 256" +variantClearFailed = "Warning: VariantClear failed with HRESULT: %{hresult}" [firewall_helper] whatIfCreateRule = "Would create firewall rule '%{name}'" diff --git a/resources/windows_firewall/src/firewall.rs b/resources/windows_firewall/src/firewall.rs index f5c6714a6..d825d8ffd 100644 --- a/resources/windows_firewall/src/firewall.rs +++ b/resources/windows_firewall/src/firewall.rs @@ -10,7 +10,10 @@ use windows::Win32::System::Com::{CLSCTX_INPROC_SERVER, CoCreateInstance, CoInit use windows::Win32::System::Ole::IEnumVARIANT; use windows::Win32::System::Variant::{VARIANT, VariantClear}; -use crate::types::{FirewallError, FirewallRule, FirewallRuleList, Metadata, RuleAction, RuleDirection, UnspecifiedRulesAction}; +use crate::types::{ + FirewallError, FirewallRule, FirewallRuleList, Metadata, RuleAction, RuleDirection, + UnspecifiedRuleAction, UnspecifiedRules, +}; /// RAII wrapper for VARIANT that automatically calls VariantClear on drop struct SafeVariant(VARIANT); @@ -32,7 +35,13 @@ impl SafeVariant { impl Drop for SafeVariant { fn drop(&mut self) { if let Err(e) = unsafe { VariantClear(&mut self.0) } { - crate::write_error(&format!("Warning: VariantClear failed with HRESULT: {:#010x}", e.code().0 as u32)); + crate::write_error( + t!( + "firewall.variantClearFailed", + hresult = format!("{:#010x}", e.code().0 as u32) + ) + .as_ref(), + ); } } } @@ -212,6 +221,30 @@ fn profiles_to_mask(values: &[String]) -> Result { Ok(mask) } +fn rule_matches_unspecified_scope( + rule: &FirewallRule, + unspecified_rules: &UnspecifiedRules, +) -> Result { + if let Some(direction) = unspecified_rules.direction.as_ref() + && rule.direction.as_ref() != Some(direction) + { + return Ok(false); + } + + if let Some(profiles) = unspecified_rules.profiles.as_ref() { + if profiles.is_empty() { + return Err(t!("firewall.emptyUnspecifiedProfiles").to_string().into()); + } + let requested_mask = profiles_to_mask(profiles)?; + let rule_mask = profiles_to_mask(rule.profiles.as_deref().unwrap_or_default())?; + if requested_mask & rule_mask == 0 { + return Ok(false); + } + } + + Ok(true) +} + fn split_csv(value: Option) -> Option> { value.map(|raw| { raw.split(',') @@ -381,10 +414,6 @@ fn apply_rule_properties(rule: &INetFwRule, desired: &FirewallRule, existing_pro } pub fn get_rules(input: &FirewallRuleList) -> Result { - if input.rules.is_empty() { - return Err(t!("get.rulesArrayEmpty").to_string().into()); - } - let store = FirewallStore::open()?; let mut results = Vec::new(); @@ -399,7 +428,7 @@ pub fn get_rules(input: &FirewallRuleList) -> Result FirewallRule { @@ -426,10 +455,6 @@ fn project_rule(current: &FirewallRule, desired: &FirewallRule) -> FirewallRule } pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result { - if input.rules.is_empty() { - return Err(t!("set.rulesArrayEmpty").to_string().into()); - } - let store = FirewallStore::open()?; let mut results = Vec::new(); @@ -496,10 +521,14 @@ pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result { - let is_remove = matches!(&input.unspecified_rules_action, Some(UnspecifiedRulesAction::Remove)); + // Disable or remove rules that aren't explicitly listed and match the requested scope. + match &input.unspecified_rules { + Some(unspecified_rules) if matches!( + unspecified_rules.action, + UnspecifiedRuleAction::Disable | UnspecifiedRuleAction::Remove + ) => + { + let is_remove = unspecified_rules.action == UnspecifiedRuleAction::Remove; let specified_names: std::collections::HashSet = input.rules.iter() .filter_map(|r| r.selector_name().map(|n| n.to_ascii_lowercase())) .collect(); @@ -516,11 +545,15 @@ pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result Result {} // None or Ignore — no additional action + _ => {} // None or Ignore: no additional action. } - Ok(FirewallRuleList { rules: results, unspecified_rules_action: input.unspecified_rules_action.clone() }) + Ok(FirewallRuleList { rules: results, unspecified_rules: input.unspecified_rules.clone() }) } pub fn export_rules() -> Result { @@ -566,5 +599,74 @@ pub fn export_rules() -> Result { results.push(rule_to_model(&rule)?); } - Ok(FirewallRuleList { rules: results, unspecified_rules_action: None }) + Ok(FirewallRuleList { rules: results, unspecified_rules: None }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn rule(direction: RuleDirection, profiles: &[&str]) -> FirewallRule { + FirewallRule { + direction: Some(direction), + profiles: Some( + profiles + .iter() + .map(|profile| (*profile).to_string()) + .collect(), + ), + ..FirewallRule::default() + } + } + + fn scope(direction: Option, profiles: Option<&[&str]>) -> UnspecifiedRules { + UnspecifiedRules { + action: UnspecifiedRuleAction::Disable, + direction, + profiles: profiles.map(|values| { + values + .iter() + .map(|profile| (*profile).to_string()) + .collect() + }), + } + } + + #[test] + fn unspecified_rule_scope_combines_direction_and_profiles() { + let filter = scope(Some(RuleDirection::Inbound), Some(&["Domain"])); + + assert!( + rule_matches_unspecified_scope(&rule(RuleDirection::Inbound, &["Domain"]), &filter) + .unwrap() + ); + assert!( + !rule_matches_unspecified_scope(&rule(RuleDirection::Outbound, &["Domain"]), &filter) + .unwrap() + ); + assert!( + !rule_matches_unspecified_scope(&rule(RuleDirection::Inbound, &["Private"]), &filter) + .unwrap() + ); + } + + #[test] + fn unspecified_rule_profile_scope_intersects_all_profiles() { + let filter = scope(None, Some(&["Domain"])); + + assert!( + rule_matches_unspecified_scope(&rule(RuleDirection::Inbound, &["All"]), &filter) + .unwrap() + ); + } + + #[test] + fn unspecified_rule_profile_scope_rejects_empty_filter() { + let filter = scope(None, Some(&[])); + + assert!( + rule_matches_unspecified_scope(&rule(RuleDirection::Inbound, &["Domain"]), &filter) + .is_err() + ); + } } diff --git a/resources/windows_firewall/src/types.rs b/resources/windows_firewall/src/types.rs index bd0a3dd4d..1567c4650 100644 --- a/resources/windows_firewall/src/types.rs +++ b/resources/windows_firewall/src/types.rs @@ -23,17 +23,29 @@ pub enum RuleAction { #[derive(Debug, Serialize, Deserialize, PartialEq, Clone)] #[serde(rename_all = "camelCase")] -pub enum UnspecifiedRulesAction { +pub enum UnspecifiedRuleAction { Ignore, Disable, Remove, } +#[derive(Debug, Serialize, Deserialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct UnspecifiedRules { + pub action: UnspecifiedRuleAction, + + #[serde(skip_serializing_if = "Option::is_none")] + pub direction: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + pub profiles: Option>, +} + #[derive(Debug, Default, Serialize, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub struct FirewallRuleList { #[serde(skip_serializing_if = "Option::is_none")] - pub unspecified_rules_action: Option, + pub unspecified_rules: Option, pub rules: Vec, } @@ -135,6 +147,22 @@ impl From for FirewallError { #[cfg(windows)] impl From for FirewallError { fn from(error: windows::core::Error) -> Self { - Self { message: error.to_string() } + Self { + message: error.to_string(), + } + } +} + +#[cfg(test)] +mod tests { + use super::FirewallRuleList; + + #[test] + fn unspecified_rules_requires_action() { + let result = serde_json::from_str::( + r#"{"unspecifiedRules":{"direction":"Inbound"},"rules":[]}"#, + ); + + assert!(result.is_err()); } } diff --git a/resources/windows_firewall/tests/windows_firewall_get.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_get.tests.ps1 index daf95e307..0ca97677a 100644 --- a/resources/windows_firewall/tests/windows_firewall_get.tests.ps1 +++ b/resources/windows_firewall/tests/windows_firewall_get.tests.ps1 @@ -56,10 +56,11 @@ Describe 'Microsoft.Windows/FirewallRuleList - get operation' -Skip:(!$IsWindows $result.PSObject.Properties.Name | Should -Not -Contain 'direction' } - It 'fails when rules array is empty' { + It 'accepts an empty rules array' { $json = '{"rules":[]}' - $out = $json | dsc resource get -r $resourceType -f - 2>&1 - $LASTEXITCODE | Should -Not -Be 0 + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + ($out | ConvertFrom-Json).actualState.rules | Should -BeNullOrEmpty } It 'handles multiple rules in a single request' { diff --git a/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 index 607236491..dfa348901 100644 --- a/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 +++ b/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 @@ -1,16 +1,22 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$isElevated) { - BeforeDiscovery { - $isElevated = if ($IsWindows) { - ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( - [Security.Principal.WindowsBuiltInRole]::Administrator) - } else { - $false - } +BeforeDiscovery { + $isElevated = if ($IsWindows) { + ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( + [Security.Principal.WindowsBuiltInRole]::Administrator) + } else { + $false } + $hasNetSecurity = @( + 'Get-NetFirewallRule' + 'New-NetFirewallRule' + 'Remove-NetFirewallRule' + 'Set-NetFirewallRule' + ).Where({ $null -eq (Get-Command $_ -ErrorAction SilentlyContinue) }).Count -eq 0 +} +Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$isElevated -or !$hasNetSecurity) { BeforeAll { $resourceType = 'Microsoft.Windows/FirewallRuleList' $testRuleName = 'DSC-WindowsFirewall-Set-Test' @@ -46,10 +52,11 @@ Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$isElevate $LASTEXITCODE | Should -Not -Be 0 } - It 'fails when rules array is empty' -Skip:(!$isElevated) { + It 'accepts an empty rules array' -Skip:(!$isElevated) { $json = '{"rules":[]}' - $out = $json | dsc resource set -r $resourceType -f - 2>&1 - $LASTEXITCODE | Should -Not -Be 0 + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + ($out | ConvertFrom-Json).afterState.rules | Should -BeNullOrEmpty } It 'updates an existing rule' -Skip:(!$isElevated) { @@ -146,18 +153,13 @@ Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$isElevate } } -Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRulesAction (what-if)' -Skip:(!$isElevated) { - BeforeDiscovery { - $isElevated = if ($IsWindows) { - ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( - [Security.Principal.WindowsBuiltInRole]::Administrator) - } else { - $false - } - } - +Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRules (what-if)' -Skip:(!$isElevated -or !$hasNetSecurity) { BeforeAll { $testRuleName = 'DSC-WindowsFirewall-Unspecified-Test' + $inboundDomainRule = 'DSC-WindowsFirewall-Scope-Inbound-Domain' + $outboundDomainRule = 'DSC-WindowsFirewall-Scope-Outbound-Domain' + $inboundPrivateRule = 'DSC-WindowsFirewall-Scope-Inbound-Private' + $allProfilesRule = 'DSC-WindowsFirewall-Scope-Inbound-All' function Initialize-TestFirewallRule { $existing = Get-NetFirewallRule -Name $testRuleName -ErrorAction SilentlyContinue @@ -168,19 +170,49 @@ Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRulesAction (what-if)' } } + function Initialize-ScopeFirewallRules { + Remove-NetFirewallRule -Name 'DSC-WindowsFirewall-Scope-*' -ErrorAction SilentlyContinue + New-NetFirewallRule -Name $inboundDomainRule -DisplayName $inboundDomainRule -Direction Inbound -Profile Domain -Action Allow -Enabled True | Out-Null + New-NetFirewallRule -Name $outboundDomainRule -DisplayName $outboundDomainRule -Direction Outbound -Profile Domain -Action Allow -Enabled True | Out-Null + New-NetFirewallRule -Name $inboundPrivateRule -DisplayName $inboundPrivateRule -Direction Inbound -Profile Private -Action Allow -Enabled True | Out-Null + New-NetFirewallRule -Name $allProfilesRule -DisplayName $allProfilesRule -Direction Inbound -Profile Any -Action Allow -Enabled True | Out-Null + } + + function Get-UnspecifiedWhatIfRuleNames { + param( + [Parameter(Mandatory)] + [hashtable]$UnspecifiedRules, + + [array]$Rules = @() + ) + + $json = @{ + unspecifiedRules = $UnspecifiedRules + rules = $Rules + } | ConvertTo-Json -Compress -Depth 5 + + $result = windows_firewall set -w --input $json 2>$testdrive/error.log | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + + return @($result.rules | + Where-Object { $_._metadata.whatIf -match 'Would (disable|remove) unspecified firewall rule' } | + ForEach-Object { $_.name }) + } + Initialize-TestFirewallRule } AfterAll { Remove-NetFirewallRule -Name $testRuleName -ErrorAction SilentlyContinue + Remove-NetFirewallRule -Name 'DSC-WindowsFirewall-Scope-*' -ErrorAction SilentlyContinue } - It 'does not affect unspecified rules when unspecifiedRulesAction is ignore' -Skip:(!$isElevated) { + It 'does not affect unspecified rules when action is ignore' -Skip:(!$isElevated) { Initialize-TestFirewallRule # Specify a different rule name so $testRuleName is "unspecified" $json = @{ - unspecifiedRulesAction = 'ignore' + unspecifiedRules = @{ action = 'ignore' } rules = @(@{ name = 'SomeOtherRuleThatMayNotExist' direction = 'Inbound' @@ -198,7 +230,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRulesAction (what-if)' $unspecifiedEntries | Should -BeNullOrEmpty } - It 'does not affect unspecified rules when unspecifiedRulesAction is omitted' -Skip:(!$isElevated) { + It 'does not affect unspecified rules when unspecifiedRules is omitted' -Skip:(!$isElevated) { Initialize-TestFirewallRule $json = @{ @@ -219,12 +251,135 @@ Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRulesAction (what-if)' $unspecifiedEntries | Should -BeNullOrEmpty } - It 'reports would disable unspecified rules when unspecifiedRulesAction is disable' -Skip:(!$isElevated) { + It 'requires action when unspecifiedRules is used' -Skip:(!$isElevated) { + $json = @{ + unspecifiedRules = @{ direction = 'Inbound' } + rules = @() + } | ConvertTo-Json -Compress -Depth 5 + + $json | dsc resource set -r 'Microsoft.Windows/FirewallRuleList' -f - 2>$testdrive/error.log | Out-Null + $LASTEXITCODE | Should -Not -Be 0 + Get-Content -Raw $testdrive/error.log | Should -Match 'action' + } + + It 'rejects an empty unspecifiedRules profiles filter' -Skip:(!$isElevated) { + $json = @{ + unspecifiedRules = @{ + action = 'disable' + profiles = @() + } + rules = @() + } | ConvertTo-Json -Compress -Depth 5 + + $json | dsc resource set -r 'Microsoft.Windows/FirewallRuleList' -f - 2>$testdrive/error.log | Out-Null + $LASTEXITCODE | Should -Not -Be 0 + Get-Content -Raw $testdrive/error.log | Should -Match 'profiles' + } + + It 'filters unspecified rules by direction' -ForEach @( + @{ + Direction = 'Inbound' + IncludedRules = @( + 'DSC-WindowsFirewall-Scope-Inbound-Domain' + 'DSC-WindowsFirewall-Scope-Inbound-Private' + ) + ExcludedRule = 'DSC-WindowsFirewall-Scope-Outbound-Domain' + } + @{ + Direction = 'Outbound' + IncludedRules = @('DSC-WindowsFirewall-Scope-Outbound-Domain') + ExcludedRule = 'DSC-WindowsFirewall-Scope-Inbound-Domain' + } + ) -Skip:(!$isElevated) { + Initialize-ScopeFirewallRules + + $affectedNames = Get-UnspecifiedWhatIfRuleNames -UnspecifiedRules @{ + action = 'disable' + direction = $Direction + } + + foreach ($includedRule in $IncludedRules) { + $affectedNames | Should -Contain $includedRule + } + $affectedNames | Should -Not -Contain $ExcludedRule + } + + It 'filters unspecified rules by profiles and includes rules that apply to all profiles' -Skip:(!$isElevated) { + Initialize-ScopeFirewallRules + + $affectedNames = Get-UnspecifiedWhatIfRuleNames -UnspecifiedRules @{ + action = 'disable' + profiles = @('Domain') + } + + $affectedNames | Should -Contain $inboundDomainRule + $affectedNames | Should -Contain $outboundDomainRule + $affectedNames | Should -Contain $allProfilesRule + $affectedNames | Should -Not -Contain $inboundPrivateRule + } + + It 'matches any profile listed in the profiles filter' -Skip:(!$isElevated) { + Initialize-ScopeFirewallRules + + $affectedNames = Get-UnspecifiedWhatIfRuleNames -UnspecifiedRules @{ + action = 'disable' + profiles = @('Domain', 'Private') + } + + $affectedNames | Should -Contain $inboundDomainRule + $affectedNames | Should -Contain $outboundDomainRule + $affectedNames | Should -Contain $inboundPrivateRule + } + + It 'combines direction and profiles when filtering unspecified rules' -Skip:(!$isElevated) { + Initialize-ScopeFirewallRules + + $affectedNames = Get-UnspecifiedWhatIfRuleNames -UnspecifiedRules @{ + action = 'disable' + direction = 'Inbound' + profiles = @('Domain') + } + + $affectedNames | Should -Contain $inboundDomainRule + $affectedNames | Should -Contain $allProfilesRule + $affectedNames | Should -Not -Contain $outboundDomainRule + $affectedNames | Should -Not -Contain $inboundPrivateRule + } + + It 'applies remove to an empty rules list only within the filtered scope' -Skip:(!$isElevated) { + Initialize-ScopeFirewallRules + + $affectedNames = Get-UnspecifiedWhatIfRuleNames -UnspecifiedRules @{ + action = 'remove' + direction = 'Outbound' + profiles = @('Domain') + } + + $affectedNames | Should -Contain $outboundDomainRule + $affectedNames | Should -Not -Contain $inboundDomainRule + $affectedNames | Should -Not -Contain $inboundPrivateRule + $affectedNames | Should -Not -Contain $allProfilesRule + } + + It 'does not act on a declared rule that matches the unspecified rule scope' -Skip:(!$isElevated) { + Initialize-ScopeFirewallRules + + $affectedNames = Get-UnspecifiedWhatIfRuleNames -UnspecifiedRules @{ + action = 'disable' + direction = 'Inbound' + profiles = @('Domain') + } -Rules @(@{ name = $inboundDomainRule }) + + $affectedNames | Should -Not -Contain $inboundDomainRule + $affectedNames | Should -Contain $allProfilesRule + } + + It 'reports would disable unspecified rules when action is disable' -Skip:(!$isElevated) { Initialize-TestFirewallRule # Specify only testRuleName; all other rules are "unspecified" and should be disabled $json = @{ - unspecifiedRulesAction = 'disable' + unspecifiedRules = @{ action = 'disable' } rules = @(@{ name = $testRuleName enabled = $true @@ -252,7 +407,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRulesAction (what-if)' $actual.Enabled | Should -Be 'True' } - It 'skips already-disabled rules when unspecifiedRulesAction is disable' -Skip:(!$isElevated) { + It 'skips already-disabled rules when action is disable' -Skip:(!$isElevated) { Initialize-TestFirewallRule # Disable the test rule so it is already disabled Set-NetFirewallRule -Name $testRuleName -Enabled False @@ -262,7 +417,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRulesAction (what-if)' New-NetFirewallRule -Name $otherRuleName -DisplayName $otherRuleName -Direction Inbound -Action Allow -Protocol TCP -LocalPort 32790 -Enabled True -ErrorAction SilentlyContinue | Out-Null $json = @{ - unspecifiedRulesAction = 'disable' + unspecifiedRules = @{ action = 'disable' } rules = @(@{ name = $otherRuleName enabled = $true @@ -279,7 +434,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRulesAction (what-if)' Remove-NetFirewallRule -Name $otherRuleName -ErrorAction SilentlyContinue } - It 'reports would remove unspecified rules when unspecifiedRulesAction is remove' -Skip:(!$isElevated) { + It 'reports would remove unspecified rules when action is remove' -Skip:(!$isElevated) { Initialize-TestFirewallRule # Specify a different rule so testRuleName is "unspecified" @@ -287,7 +442,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - unspecifiedRulesAction (what-if)' $knownRule = (Get-NetFirewallRule | Select-Object -First 1).Name $json = @{ - unspecifiedRulesAction = 'remove' + unspecifiedRules = @{ action = 'remove' } rules = @(@{ name = $knownRule enabled = $true diff --git a/resources/windows_firewall/tests/windows_firewall_whatif.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_whatif.tests.ps1 index 5bd0f085e..d65bf934c 100644 --- a/resources/windows_firewall/tests/windows_firewall_whatif.tests.ps1 +++ b/resources/windows_firewall/tests/windows_firewall_whatif.tests.ps1 @@ -1,17 +1,21 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -Describe 'windows_firewall config whatif tests' -Skip:(!$isElevated -or !$hasNetSecurity) { - BeforeDiscovery { - $isElevated = if ($IsWindows) { - ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( - [Security.Principal.WindowsBuiltInRole]::Administrator) - } else { - $false - } - $hasNetSecurity = $null -ne (Get-Command 'Get-NetFirewallRule' -ErrorAction SilentlyContinue) +BeforeDiscovery { + $isElevated = if ($IsWindows) { + ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( + [Security.Principal.WindowsBuiltInRole]::Administrator) + } else { + $false } + $hasNetSecurity = @( + 'Get-NetFirewallRule' + 'New-NetFirewallRule' + 'Remove-NetFirewallRule' + ).Where({ $null -eq (Get-Command $_ -ErrorAction SilentlyContinue) }).Count -eq 0 +} +Describe 'windows_firewall config whatif tests' -Skip:(!$isElevated -or !$hasNetSecurity) { BeforeAll { $testRuleName = 'DSC-WindowsFirewall-WhatIf-Test' diff --git a/resources/windows_firewall/windows_firewall.dsc.resource.json b/resources/windows_firewall/windows_firewall.dsc.resource.json index 6a587fcd9..58c6f7dc5 100644 --- a/resources/windows_firewall/windows_firewall.dsc.resource.json +++ b/resources/windows_firewall/windows_firewall.dsc.resource.json @@ -6,7 +6,7 @@ "Windows", "Firewall" ], - "version": "0.2.1", + "version": "0.3.0", "get": { "executable": "windows_firewall", "args": [ @@ -59,16 +59,50 @@ "rules" ], "properties": { - "unspecifiedRulesAction": { - "type": "string", - "title": "Unspecified rules action", - "description": "The action to take on firewall rules not explicitly listed in the rules array. 'ignore' (default) leaves them unchanged, 'disable' disables them, and 'remove' deletes them.", - "default": "ignore", - "enum": [ - "ignore", - "disable", - "remove" - ] + "unspecifiedRules": { + "type": "object", + "title": "Unspecified rules", + "description": "Defines the action and optional scope for firewall rules not explicitly listed in the rules array. When both direction and profiles are specified, a rule must match both filters.", + "additionalProperties": false, + "required": [ + "action" + ], + "properties": { + "action": { + "type": "string", + "title": "Action", + "description": "The action to take on matching unspecified firewall rules. 'ignore' leaves them unchanged, 'disable' disables them, and 'remove' deletes them.", + "enum": [ + "ignore", + "disable", + "remove" + ] + }, + "direction": { + "type": "string", + "title": "Direction", + "description": "Limits the action to unspecified rules with this traffic direction.", + "enum": [ + "Inbound", + "Outbound" + ] + }, + "profiles": { + "type": "array", + "title": "Profiles", + "description": "Limits the action to unspecified rules that apply to any of these firewall profiles.", + "minItems": 1, + "items": { + "type": "string", + "enum": [ + "Domain", + "Private", + "Public", + "All" + ] + } + } + } }, "rules": { "type": "array",