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 9a5818818..3aa53020f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4256,7 +4256,7 @@ checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" [[package]] name = "windows_firewall" -version = "0.2.0" +version = "0.3.0" dependencies = [ "rust-i18n", "serde", diff --git a/dsc/tests/dsc_schema_default.tests.ps1 b/dsc/tests/dsc_schema_default.tests.ps1 new file mode 100644 index 000000000..286e76eda --- /dev/null +++ b/dsc/tests/dsc_schema_default.tests.ps1 @@ -0,0 +1,54 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'Synthetic test uses schema defaults' { + It 'Property matching schema default is not reported as differing' { + $out = '{"name":"test","enabled":true}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $out.inDesiredState | Should -Be $true + $out.differingProperties | Should -BeNullOrEmpty + } + + It 'Property differing from schema default is reported as differing' { + $out = '{"name":"test","enabled":false}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $out.inDesiredState | Should -Be $false + $out.differingProperties | Should -Contain 'enabled' + } + + It 'Integer property matching schema default is not reported as differing' { + $out = '{"name":"test","count":5}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $out.inDesiredState | Should -Be $true + $out.differingProperties | Should -BeNullOrEmpty + } + + It 'Integer property differing from schema default is reported as differing' { + $out = '{"name":"test","count":10}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $out.inDesiredState | Should -Be $false + $out.differingProperties | Should -Contain 'count' + } + + It 'Multiple properties matching schema defaults are not reported as differing' { + $out = '{"name":"test","enabled":true,"count":5}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $out.inDesiredState | Should -Be $true + $out.differingProperties | Should -BeNullOrEmpty + } + + It 'Mix of matching and non-matching defaults reports only non-matching' { + $out = '{"name":"test","enabled":true,"count":10}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $out.inDesiredState | Should -Be $false + $out.differingProperties | Should -Contain 'count' + $out.differingProperties | Should -Not -Contain 'enabled' + } + + It 'Property present in both expected and actual is compared normally' { + $out = '{"name":"test"}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $out.inDesiredState | Should -Be $true + $out.differingProperties | Should -BeNullOrEmpty + } +} diff --git a/lib/dsc-lib/src/dscresources/command_resource.rs b/lib/dsc-lib/src/dscresources/command_resource.rs index 6674d4924..5b4479b72 100644 --- a/lib/dsc-lib/src/dscresources/command_resource.rs +++ b/lib/dsc-lib/src/dscresources/command_resource.rs @@ -12,7 +12,7 @@ use crate::{configure::{config_doc::{ExecutionKind, SecurityContextKind}, config use crate::dscerror::DscError; use crate::locked_insert; use super::{ - dscresource::{get_diff, redact, DscResource}, + dscresource::{get_diff, get_diff_with_schema, redact, DscResource}, invoke_result::{ DeleteResult, DeleteResultKind, ExportResult, GetResult, ResolveResult, SetResult, TestResult, ValidateResult, @@ -454,7 +454,14 @@ fn invoke_synthetic_test(resource: &DscResource, expected: &str, target_resource } }; let expected_value: Value = serde_json::from_str(expected)?; - let diff_properties = get_diff(&expected_value, &actual_state); + let cached_resource = target_resource.unwrap_or(resource); + let schema: Option = get_resource_schema(&cached_resource.type_name, &cached_resource.version) + .or_else(|| { + // Populate the cache on a miss, then read from cache + get_schema(resource, target_resource).ok(); + get_resource_schema(&cached_resource.type_name, &cached_resource.version) + }); + let diff_properties = get_diff_with_schema(&expected_value, &actual_state, schema.as_ref()); Ok(TestResult::Resource(ResourceTestResponse { desired_state: expected_value, actual_state, diff --git a/lib/dsc-lib/src/dscresources/dscresource.rs b/lib/dsc-lib/src/dscresources/dscresource.rs index 8c2566610..29eda6a08 100644 --- a/lib/dsc-lib/src/dscresources/dscresource.rs +++ b/lib/dsc-lib/src/dscresources/dscresource.rs @@ -11,7 +11,7 @@ use rust_i18n::t; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use tracing::{debug, info, trace, warn}; @@ -470,7 +470,12 @@ impl Invoke for DscResource { response.actual_state } }; - let diff_properties = get_diff( &desired_state, &actual_state); + let schema: Option = if let Some(s) = &self.schema { + serde_json::to_value(s).ok() + } else { + self.schema().ok().and_then(|s| serde_json::from_str(&s).ok()) + }; + let diff_properties = get_diff_with_schema( &desired_state, &actual_state, schema.as_ref()); desired_state = redact(&desired_state); let test_result = TestResult::Resource(ResourceTestResponse { desired_state, @@ -647,6 +652,25 @@ pub fn get_adapter_input_kind(adapter: &DscResource) -> Result Vec { + get_diff_with_schema(expected, actual, None) +} + +#[must_use] +/// Performs a comparison of two JSON Values using an optional JSON Schema. +/// Properties whose schema sets `writeOnly` to `true` are ignored. If a property exists +/// in `expected` but not in `actual`, the schema's `default` value for that property is +/// used for comparison when available. +/// +/// # Arguments +/// +/// * `expected` - The expected value +/// * `actual` - The actual value +/// * `schema` - Optional JSON Schema to identify write-only properties and default values +/// +/// # Returns +/// +/// An array of top level properties that differ, if any +pub(crate) fn get_diff_with_schema(expected: &Value, actual: &Value, schema: Option<&Value>) -> Vec { let mut diff_properties: Vec = Vec::new(); if expected.is_null() { return diff_properties; @@ -668,6 +692,10 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { } for (key, value) in &*map { + if is_schema_write_only(schema, key) { + continue; + } + if is_secure_value(value) { // skip secure values as they are not comparable continue; @@ -702,8 +730,16 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { diff_properties.push(key.to_string()); } } else { - info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key)); - diff_properties.push(key.to_string()); + // Property not in actual - check schema for a default value + if let Some(default_value) = get_schema_default(schema, key) { + if value != &default_value { + info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key)); + diff_properties.push(key.to_string()); + } + } else { + info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key)); + diff_properties.push(key.to_string()); + } } } else { info!("{}", t!("dscresources.dscresource.diffKeyNotObject", key = key)); @@ -716,6 +752,59 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { diff_properties } +/// Looks up the default value for a property from a JSON Schema. +/// +/// # Arguments +/// +/// * `schema` - Optional JSON Schema value +/// * `property_name` - The property name to look up +/// +/// # Returns +/// +/// The default value if found in the schema's properties definition, otherwise None +fn get_schema_default(schema: Option<&Value>, property_name: &str) -> Option { + let schema = schema?; + let properties = schema.get("properties")?.as_object()?; + let property_schema = properties.get(property_name)?.as_object()?; + property_schema.get("default").cloned() +} + +/// Returns whether a property's JSON Schema sets `writeOnly` to `true`, directly or +/// through a local JSON Pointer reference. +fn is_schema_write_only(schema: Option<&Value>, property_name: &str) -> bool { + let Some(schema) = schema else { + return false; + }; + let Some(mut property_schema) = schema + .get("properties") + .and_then(Value::as_object) + .and_then(|properties| properties.get(property_name)) + else { + return false; + }; + let mut visited_references = HashSet::new(); + + loop { + if property_schema.get("writeOnly").and_then(Value::as_bool) == Some(true) { + return true; + } + + let Some(reference) = property_schema.get("$ref").and_then(Value::as_str) else { + return false; + }; + let Some(pointer) = reference.strip_prefix('#') else { + return false; + }; + if !visited_references.insert(pointer) { + return false; + } + let Some(resolved_schema) = schema.pointer(pointer) else { + return false; + }; + property_schema = resolved_schema; + } +} + /// Validates the properties of a resource against its schema. /// /// # Arguments @@ -926,3 +1015,143 @@ fn different_array_with_nested_array() { let array_two = vec![json!("a"), json!(1), json!({"a":"b"}), json!(vec![json!("a"), json!(2)])]; assert_eq!(is_same_array(&array_one, &array_two), false); } + +#[test] +fn diff_with_schema_default_matches_expected() { + use serde_json::json; + let expected = json!({"name": "test", "enabled": true}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "enabled": { "type": "boolean", "default": true } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert!(diff.is_empty(), "Expected no diff when expected matches schema default, got: {diff:?}"); +} + +#[test] +fn diff_with_schema_default_differs_from_expected() { + use serde_json::json; + let expected = json!({"name": "test", "enabled": false}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "enabled": { "type": "boolean", "default": true } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert_eq!(diff, vec!["enabled".to_string()]); +} + +#[test] +fn diff_with_schema_no_default_reports_missing_property() { + use serde_json::json; + let expected = json!({"name": "test", "enabled": true}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "enabled": { "type": "boolean" } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert_eq!(diff, vec!["enabled".to_string()]); +} + +#[test] +fn diff_with_schema_write_only_ignores_differing_property() { + use serde_json::json; + let expected = json!({"name": "test", "action": "remove"}); + let actual = json!({"name": "test", "action": "ignore"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "action": { "type": "string", "writeOnly": true } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert!(diff.is_empty(), "Expected write-only property to be ignored, got: {diff:?}"); +} + +#[test] +fn diff_with_schema_write_only_ignores_missing_property() { + use serde_json::json; + let expected = json!({"name": "test", "action": "remove"}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "action": { "type": "string", "writeOnly": true } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert!(diff.is_empty(), "Expected write-only property to be ignored, got: {diff:?}"); +} + +#[test] +fn diff_with_schema_write_only_local_ref_ignores_missing_property() { + use serde_json::json; + let expected = json!({"name": "test", "action": "remove"}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "action": { "$ref": "#/$defs/action" } + }, + "$defs": { + "action": { "type": "string", "writeOnly": true } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert!(diff.is_empty(), "Expected referenced write-only property to be ignored, got: {diff:?}"); +} + +#[test] +fn diff_with_schema_write_only_false_reports_missing_property() { + use serde_json::json; + let expected = json!({"name": "test", "action": "remove"}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "action": { "type": "string", "writeOnly": false } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert_eq!(diff, vec!["action".to_string()]); +} + +#[test] +fn diff_without_schema_reports_missing_property() { + use serde_json::json; + let expected = json!({"name": "test", "enabled": true}); + let actual = json!({"name": "test"}); + let diff = get_diff_with_schema(&expected, &actual, None); + assert_eq!(diff, vec!["enabled".to_string()]); +} + +#[test] +fn diff_with_schema_default_integer() { + use serde_json::json; + let expected = json!({"name": "test", "count": 5}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "count": { "type": "integer", "default": 5 } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert!(diff.is_empty(), "Expected no diff when expected matches schema default integer, got: {diff:?}"); +} 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..4815e0e3c 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: None }) } 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_schema_default.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 new file mode 100644 index 000000000..9c2279cb2 --- /dev/null +++ b/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 @@ -0,0 +1,113 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaults' -Skip:(!$canRunFirewallTests) { + BeforeDiscovery { + $canRunFirewallTests = $IsWindows -and + (Get-Command Get-NetFirewallRule -ErrorAction Ignore) -and + ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole( + [Security.Principal.WindowsBuiltInRole]::Administrator) + } + + BeforeAll { + $resourceType = 'Microsoft.Windows/FirewallRuleList' + $testRuleName = 'DSC-WindowsFirewall-SchemaDefault-Test' + + # Ensure a known rule exists for testing + $existing = Get-NetFirewallRule -Name $testRuleName -ErrorAction Ignore + if (-not $existing) { + New-NetFirewallRule -Name $testRuleName -DisplayName $testRuleName ` + -Direction Inbound -Action Allow -Protocol TCP -LocalPort 32921 ` + -Enabled True -PolicyStore PersistentStore | Out-Null + } + } + + AfterAll { + Remove-NetFirewallRule -Name $testRuleName -ErrorAction Ignore + } + + It 'unspecifiedRules action "ignore" does not report as differing' { + $json = @{ + unspecifiedRules = @{ + action = 'ignore' + } + rules = @(@{ + name = $testRuleName + direction = 'Inbound' + action = 'Allow' + protocol = 6 + localPorts = '32921' + enabled = $true + }) + } | ConvertTo-Json -Compress -Depth 5 + $out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + + $result = $out | ConvertFrom-Json + $result.inDesiredState | Should -Be $true + $result.differingProperties | Should -Not -Contain 'unspecifiedRules' + } + + It 'unspecifiedRules omitted does not report as differing' { + $json = @{ + rules = @(@{ + name = $testRuleName + direction = 'Inbound' + action = 'Allow' + protocol = 6 + localPorts = '32921' + enabled = $true + }) + } | ConvertTo-Json -Compress -Depth 5 + $out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + + $result = $out | ConvertFrom-Json + $result.inDesiredState | Should -Be $true + $result.differingProperties | Should -Not -Contain 'unspecifiedRules' + } + + It 'unspecifiedRules action "disable" is ignored for comparison' { + $json = @{ + unspecifiedRules = @{ + action = 'disable' + } + rules = @(@{ + name = $testRuleName + direction = 'Inbound' + action = 'Allow' + protocol = 6 + localPorts = '32921' + enabled = $true + }) + } | ConvertTo-Json -Compress -Depth 5 + $out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + + $result = $out | ConvertFrom-Json + $result.inDesiredState | Should -Be $true + $result.differingProperties | Should -Not -Contain 'unspecifiedRules' + } + + It 'unspecifiedRules action "remove" is ignored for comparison' { + $json = @{ + unspecifiedRules = @{ + action = 'remove' + } + rules = @(@{ + name = $testRuleName + direction = 'Inbound' + action = 'Allow' + protocol = 6 + localPorts = '32921' + enabled = $true + }) + } | ConvertTo-Json -Compress -Depth 5 + $out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + + $result = $out | ConvertFrom-Json + $result.inDesiredState | Should -Be $true + $result.differingProperties | Should -Not -Contain 'unspecifiedRules' + } +} diff --git a/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 index 607236491..453cd30c1 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,24 @@ 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 'does not return unspecifiedRules in the after state' -Skip:(!$isElevated) { + $json = @{ + rules = @() + unspecifiedRules = @{ + action = 'ignore' + } + } | ConvertTo-Json -Compress -Depth 5 + $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.PSObject.Properties.Name | Should -Not -Contain 'unspecifiedRules' } It 'updates an existing rule' -Skip:(!$isElevated) { @@ -146,18 +166,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 +183,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 +243,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 +264,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 +420,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 +430,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 +447,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 +455,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..d24b1eea9 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,51 @@ "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.", + "writeOnly": true, + "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", diff --git a/tools/dsctest/dsctest.dsc.manifests.json b/tools/dsctest/dsctest.dsc.manifests.json index 258bf0d17..e5bcc8a3c 100644 --- a/tools/dsctest/dsctest.dsc.manifests.json +++ b/tools/dsctest/dsctest.dsc.manifests.json @@ -280,6 +280,45 @@ } } }, + { + "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", + "type": "Test/SchemaDefault", + "version": "0.1.0", + "get": { + "executable": "dsctest", + "args": [ + "schema-default", + { + "jsonInputArg": "--input", + "mandatory": true + } + ] + }, + "schema": { + "embedded": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "required": ["name"], + "additionalProperties": false, + "properties": { + "name": { + "type": "string", + "description": "The name of the resource instance." + }, + "enabled": { + "type": "boolean", + "description": "Whether the resource is enabled.", + "default": true + }, + "count": { + "type": "integer", + "description": "The count value.", + "default": 5 + } + } + } + } + }, { "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", "type": "Test/InDesiredState", diff --git a/tools/dsctest/src/args.rs b/tools/dsctest/src/args.rs index 18287e764..135572bbe 100644 --- a/tools/dsctest/src/args.rs +++ b/tools/dsctest/src/args.rs @@ -20,6 +20,7 @@ pub enum Schemas { Operation, RefreshEnv, RestartRequired, + SchemaDefault, Set, Sleep, StateAndDiff, @@ -157,6 +158,12 @@ pub enum SubCommand { input: String, }, + #[clap(name = "schema-default", about = "Test resource for schema default values in synthetic test")] + SchemaDefault { + #[clap(name = "input", short, long, help = "The input to the schema-default command as JSON")] + input: String, + }, + #[clap(name = "schema", about = "Get the JSON schema for a subcommand")] Schema { #[clap(name = "subcommand", short, long, help = "The subcommand to get the schema for")] diff --git a/tools/dsctest/src/main.rs b/tools/dsctest/src/main.rs index dc5c56f7f..a7e8f4ebe 100644 --- a/tools/dsctest/src/main.rs +++ b/tools/dsctest/src/main.rs @@ -16,6 +16,7 @@ mod operation; mod adapter; mod refresh_env; mod restart_required; +mod schema_default; mod set; mod sleep; mod state_and_diff; @@ -41,6 +42,7 @@ use crate::metadata::Metadata; use crate::operation::Operation; use crate::refresh_env::RefreshEnv; use crate::restart_required::RestartRequired; +use crate::schema_default::SchemaDefault; use crate::set::{Set, invoke_set}; use crate::sleep::Sleep; use crate::state_and_diff::StateAndDiff; @@ -288,6 +290,18 @@ fn main() { }; serde_json::to_string(&restart_required).unwrap() }, + SubCommand::SchemaDefault { input } => { + let schema_default = match serde_json::from_str::(&input) { + Ok(sd) => sd, + Err(err) => { + eprintln!("Error JSON does not match schema: {err}"); + std::process::exit(1); + } + }; + // Only return 'name' in the output - omit 'enabled' and 'count' + // to test schema default comparison + serde_json::json!({"name": schema_default.name}).to_string() + }, SubCommand::Schema { subcommand } => { let schema = match subcommand { Schemas::Adapter => { @@ -335,6 +349,9 @@ fn main() { Schemas::RestartRequired => { schema_for!(RestartRequired) }, + Schemas::SchemaDefault => { + schema_for!(SchemaDefault) + }, Schemas::Set => { schema_for!(Set) }, diff --git a/tools/dsctest/src/schema_default.rs b/tools/dsctest/src/schema_default.rs new file mode 100644 index 000000000..6a661de48 --- /dev/null +++ b/tools/dsctest/src/schema_default.rs @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +pub struct SchemaDefault { + pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub enabled: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub count: Option, +}