From a9a92990faa6733d4354b90bfa7fe8701c5903e9 Mon Sep 17 00:00:00 2001 From: "Steve Lee (POWERSHELL HE/HIM) (from Dev Box)" Date: Thu, 13 Aug 2026 12:18:34 -0700 Subject: [PATCH 1/3] Ignore write-only properties in schema diffs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- lib/dsc-lib/src/dscresources/dscresource.rs | 73 ++++++++++++++++++- .../windows_firewall_schema_default.tests.ps1 | 10 ++- .../windows_firewall.dsc.resource.json | 1 + 3 files changed, 76 insertions(+), 8 deletions(-) diff --git a/lib/dsc-lib/src/dscresources/dscresource.rs b/lib/dsc-lib/src/dscresources/dscresource.rs index 84a6480b7..ff9a274f6 100644 --- a/lib/dsc-lib/src/dscresources/dscresource.rs +++ b/lib/dsc-lib/src/dscresources/dscresource.rs @@ -657,14 +657,15 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { #[must_use] /// Performs a comparison of two JSON Values using an optional JSON Schema. -/// If a property exists in `expected` but not in `actual`, the schema's `default` value -/// for that property is used for comparison when available. +/// 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 look up default values for missing properties +/// * `schema` - Optional JSON Schema to identify write-only properties and default values /// /// # Returns /// @@ -691,6 +692,10 @@ pub(crate) fn get_diff_with_schema(expected: &Value, actual: &Value, schema: Opt } 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; @@ -726,7 +731,7 @@ pub(crate) fn get_diff_with_schema(expected: &Value, actual: &Value, schema: Opt } } else { // Property not in actual - check schema for a default value - if let Some(default_value) = get_schema_default(schema, key) { + 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()); @@ -764,6 +769,18 @@ fn get_schema_default(schema: Option<&Value>, property_name: &str) -> Option, property_name: &str) -> bool { + schema + .and_then(|schema| schema.get("properties")) + .and_then(Value::as_object) + .and_then(|properties| properties.get(property_name)) + .and_then(Value::as_object) + .and_then(|property_schema| property_schema.get("writeOnly")) + .and_then(Value::as_bool) + .unwrap_or(false) +} + /// Validates the properties of a resource against its schema. /// /// # Arguments @@ -1023,6 +1040,54 @@ fn diff_with_schema_no_default_reports_missing_property() { 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_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; diff --git a/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 index fa0d6aa46..7cc45dfa0 100644 --- a/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 +++ b/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 @@ -65,7 +65,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaul $result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction' } - It 'non-default unspecifiedRulesAction "disable" is reported as differing' { + It 'non-default unspecifiedRulesAction "disable" is ignored for comparison' { $json = @{ unspecifiedRulesAction = 'disable' rules = @(@{ @@ -81,10 +81,11 @@ Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaul $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = $out | ConvertFrom-Json - $result.differingProperties | Should -Contain 'unspecifiedRulesAction' + $result.inDesiredState | Should -Be $true + $result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction' } - It 'non-default unspecifiedRulesAction "remove" is reported as differing' { + It 'non-default unspecifiedRulesAction "remove" is ignored for comparison' { $json = @{ unspecifiedRulesAction = 'remove' rules = @(@{ @@ -100,6 +101,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaul $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = $out | ConvertFrom-Json - $result.differingProperties | Should -Contain 'unspecifiedRulesAction' + $result.inDesiredState | Should -Be $true + $result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction' } } diff --git a/resources/windows_firewall/windows_firewall.dsc.resource.json b/resources/windows_firewall/windows_firewall.dsc.resource.json index 6a587fcd9..8aabc4123 100644 --- a/resources/windows_firewall/windows_firewall.dsc.resource.json +++ b/resources/windows_firewall/windows_firewall.dsc.resource.json @@ -61,6 +61,7 @@ "properties": { "unspecifiedRulesAction": { "type": "string", + "writeOnly": true, "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", From 2ced2fe9428fe3331e6f11d4d7fd74e133a38fdf Mon Sep 17 00:00:00 2001 From: "Steve Lee (POWERSHELL HE/HIM) (from Dev Box)" Date: Thu, 13 Aug 2026 13:01:27 -0700 Subject: [PATCH 2/3] Resolve local refs for write-only properties Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- lib/dsc-lib/src/dscresources/dscresource.rs | 59 ++++++++++++++++++--- 1 file changed, 51 insertions(+), 8 deletions(-) diff --git a/lib/dsc-lib/src/dscresources/dscresource.rs b/lib/dsc-lib/src/dscresources/dscresource.rs index ff9a274f6..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}; @@ -769,16 +769,40 @@ fn get_schema_default(schema: Option<&Value>, property_name: &str) -> Option, property_name: &str) -> bool { - schema - .and_then(|schema| schema.get("properties")) + 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)) - .and_then(Value::as_object) - .and_then(|property_schema| property_schema.get("writeOnly")) - .and_then(Value::as_bool) - .unwrap_or(false) + 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. @@ -1072,6 +1096,25 @@ fn diff_with_schema_write_only_ignores_missing_property() { 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; From e3f76e92e919851809fcc1493ba4c4a159731498 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:27:54 +0000 Subject: [PATCH 3/3] Omit write-only firewall instructions from set output Co-authored-by: SteveL-MSFT <11859881+SteveL-MSFT@users.noreply.github.com> --- resources/windows_firewall/src/firewall.rs | 2 +- .../tests/windows_firewall_set.tests.ps1 | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/resources/windows_firewall/src/firewall.rs b/resources/windows_firewall/src/firewall.rs index d825d8ffd..4815e0e3c 100644 --- a/resources/windows_firewall/src/firewall.rs +++ b/resources/windows_firewall/src/firewall.rs @@ -587,7 +587,7 @@ pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result {} // None or Ignore: no additional action. } - Ok(FirewallRuleList { rules: results, unspecified_rules: input.unspecified_rules.clone() }) + Ok(FirewallRuleList { rules: results, unspecified_rules: None }) } pub fn export_rules() -> Result { diff --git a/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 index dfa348901..453cd30c1 100644 --- a/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 +++ b/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 @@ -59,6 +59,19 @@ Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$isElevate ($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) { Initialize-TestFirewallRule $json = @{ rules = @(@{ name = $testRuleName; description = 'Updated by DSC test'; enabled = $false }) } | ConvertTo-Json -Compress -Depth 5